From 02adb1a5454a28842ebfaee605d6cc9b06dfb154 Mon Sep 17 00:00:00 2001 From: Teisha McRae Date: Wed, 21 Oct 2026 16:35:29 -0400 Subject: [PATCH 1/7] Reconcile imported assignment rules and UI warnings This change rebinds imported rule_expression assignee records to valid users/groups in the target environment, clearing stale unresolved assignees and marking them for re-selection. It also adds guarded user/group fetches in the picker to avoid noisy 404s when stale assignees remain, and surfaces explicit warnings in the assignment expression inspector when an imported assignee cannot be resolved. --- .../AssignmentRulesReconciliation.php | 222 ++++++++++++++++++ .../Exporters/ProcessExporter.php | 11 + ProcessMaker/Jobs/ImportProcess.php | 4 + resources/js/components/SelectUserGroup.vue | 54 +++-- .../components/inspector/AssignExpression.vue | 37 ++- 5 files changed, 311 insertions(+), 17 deletions(-) create mode 100644 ProcessMaker/ImportExport/AssignmentRulesReconciliation.php diff --git a/ProcessMaker/ImportExport/AssignmentRulesReconciliation.php b/ProcessMaker/ImportExport/AssignmentRulesReconciliation.php new file mode 100644 index 0000000000..f7a66067fa --- /dev/null +++ b/ProcessMaker/ImportExport/AssignmentRulesReconciliation.php @@ -0,0 +1,222 @@ +> + */ + public static function decodeRules(?string $json): array + { + if ($json === null || $json === '') { + return []; + } + + $list = json_decode($json); + if (!is_array($list)) { + return []; + } + + return $list; + } + + /** + * @param array> $rules + */ + public static function encodeRules(array $rules): string + { + return json_encode(array_values($rules), JSON_UNESCAPED_UNICODE); + } + + /** + * @param array> $rules + * @return array> + */ + public static function reconcileRulesAfterImport(array $rules): array + { + $reconciled = []; + foreach ($rules as $rule) { + $reconciled[] = self::reconcileRuleAfterImport($rule); + } + + return $reconciled; + } + + /** + * @param object|array $rule + * @return object|array + */ + public static function reconcileRuleAfterImport(object|array $rule): object|array + { + $type = self::ruleGet($rule, 'type'); + $assignee = null; + + if ($type === 'user') { + $username = self::usernameFromAssignmentName(self::ruleGet($rule, 'assignmentName')); + // username is unique in the users table and is the same key + // UserExporter's fallback matching already trusts on import. + $assignee = $username ? User::where('username', $username)->value('id') : null; + } elseif ($type === 'group') { + $name = self::ruleGet($rule, 'assignmentName'); + $name = is_string($name) ? trim($name) : ''; + // Group names are not unique in the schema, so only an + // unambiguous match may be rebound. + $groups = $name !== '' ? Group::where('name', $name)->get() : collect(); + $assignee = $groups->count() === 1 ? $groups->first()->id : null; + } else { + return $rule; + } + + return $assignee === null + ? self::markRuleUnresolved($rule) + : self::markRuleResolved($rule, $assignee); + } + + /** + * Extract the username from "Full Name (username)", tolerating the + * " - Inactive" suffix SelectUserGroup appends for inactive users. + */ + public static function usernameFromAssignmentName(mixed $assignmentName): ?string + { + if (!is_string($assignmentName) || !preg_match('/\(([^()]*)\)\s*$/', trim($assignmentName), $matches)) { + return null; + } + + $username = trim(preg_replace('/\s+-\s+Inactive$/i', '', trim($matches[1]))); + + return $username !== '' ? $username : null; + } + + /** + * @param object|array $rule + * @return object|array + */ + public static function markRuleUnresolved(object|array $rule): object|array + { + $rule = self::ruleSet($rule, 'assignee', null); + $rule = self::ruleSet($rule, self::KEY_UNRESOLVED, true); + + return $rule; + } + + /** + * @return iterable>}> + */ + public static function collectAssignmentRulesFromDocument(\DOMDocument $document): iterable + { + $ns = WorkflowServiceProvider::PROCESS_MAKER_NS; + foreach (self::HUMAN_TASK_TAGS as $tag) { + foreach ($document->getElementsByTagName($tag) as $element) { + if (!$element instanceof DOMElement) { + continue; + } + if ($element->getAttributeNS($ns, 'assignment') !== 'rule_expression') { + continue; + } + $raw = $element->getAttributeNS($ns, self::ATTRIBUTE); + if ($raw === '') { + continue; + } + yield [ + 'element' => $element, + 'path' => $element->getNodePath(), + 'rules' => self::decodeRules($raw), + ]; + } + } + } + + public static function writeRulesOnElement(DOMElement $element, array $rules): void + { + $ns = WorkflowServiceProvider::PROCESS_MAKER_NS; + $element->setAttributeNS($ns, self::ATTRIBUTE, self::encodeRules($rules)); + } + + /** + * Reconcile every rule_expression assignmentRules block on a BPMN document. + */ + public static function reconcileAssignmentRulesInDocument(\DOMDocument $document): bool + { + $updated = false; + + foreach (self::collectAssignmentRulesFromDocument($document) as $item) { + $reconciled = self::reconcileRulesAfterImport($item['rules']); + self::writeRulesOnElement($item['element'], $reconciled); + $updated = true; + } + + return $updated; + } + + /** + * @param object|array $rule + * @return object|array + */ + private static function markRuleResolved(object|array $rule, mixed $assignee): object|array + { + $rule = self::ruleSet($rule, 'assignee', $assignee); + + if (is_array($rule)) { + unset($rule[self::KEY_UNRESOLVED]); + + return $rule; + } + + if (property_exists($rule, self::KEY_UNRESOLVED)) { + unset($rule->{self::KEY_UNRESOLVED}); + } + + return $rule; + } + + /** + * @param object|array $rule + */ + private static function ruleGet(object|array $rule, string $key): mixed + { + if (is_array($rule)) { + return $rule[$key] ?? null; + } + + return $rule->$key ?? null; + } + + /** + * @param object|array $rule + * @return object|array + */ + private static function ruleSet(object|array $rule, string $key, mixed $value): object|array + { + if (is_array($rule)) { + $rule[$key] = $value; + + return $rule; + } + + $rule->$key = $value; + + return $rule; + } +} diff --git a/ProcessMaker/ImportExport/Exporters/ProcessExporter.php b/ProcessMaker/ImportExport/Exporters/ProcessExporter.php index 9cf72a5b96..fc6cc88724 100644 --- a/ProcessMaker/ImportExport/Exporters/ProcessExporter.php +++ b/ProcessMaker/ImportExport/Exporters/ProcessExporter.php @@ -5,6 +5,7 @@ use DOMXPath; use Illuminate\Support\Arr; use Illuminate\Support\Facades\Log; +use ProcessMaker\ImportExport\AssignmentRulesReconciliation; use ProcessMaker\ImportExport\DependentType; use ProcessMaker\ImportExport\Psudomodels\Signal; use ProcessMaker\ImportExport\Utils; @@ -314,6 +315,15 @@ private function exportAssignments() } } + private function importAssignmentRules(): void + { + $document = $this->model->getDefinitions(true); + + if (AssignmentRulesReconciliation::reconcileAssignmentRulesInDocument($document)) { + $this->model->bpmn = $document->saveXML(); + } + } + private function exportScreens() { $tags = [ @@ -480,6 +490,7 @@ private function importAssetsByMode(): void $this->importScripts(); $this->importSubprocesses(); $this->importAssignments(); + $this->importAssignmentRules(); $this->importProcessLaunchpad(); $this->importElementDestination(); } diff --git a/ProcessMaker/Jobs/ImportProcess.php b/ProcessMaker/Jobs/ImportProcess.php index 03b4768916..49dd7cb5c1 100644 --- a/ProcessMaker/Jobs/ImportProcess.php +++ b/ProcessMaker/Jobs/ImportProcess.php @@ -14,6 +14,7 @@ use Illuminate\Support\Facades\Log; use Illuminate\Support\Facades\Storage; use ProcessMaker\Events\ImportedScreenSaved; +use ProcessMaker\ImportExport\AssignmentRulesReconciliation; use ProcessMaker\Managers\ExportManager; use ProcessMaker\Models\AnonymousUser; use ProcessMaker\Models\Process; @@ -518,6 +519,7 @@ protected function saveScreen($screen) return $new; } catch (\Exception $e) { Log::error('Import Screen: ' . $e->getMessage()); + return false; } } @@ -781,6 +783,8 @@ private function validatePackages($process) */ private function saveBpmn($process) { + AssignmentRulesReconciliation::reconcileAssignmentRulesInDocument($this->definitions); + $this->new['process']->bpmn = $this->definitions->saveXML(); if (isset($process->cancel_screen_id)) { $this->new['process']->cancel_screen_id = $process->cancel_screen_id; diff --git a/resources/js/components/SelectUserGroup.vue b/resources/js/components/SelectUserGroup.vue index d6390f66e4..7cbc68a230 100644 --- a/resources/js/components/SelectUserGroup.vue +++ b/resources/js/components/SelectUserGroup.vue @@ -165,45 +165,71 @@ this.loading = true; let results = []; + // showAlert: false + local catch: a stale/missing assignee must not + // surface as a global 404 toast or leave the picker loading forever. let usersPromise = Promise.all( value.users.map(item => { + let id = null; if (typeof item == 'number' || typeof item == 'string') { - return ProcessMaker.apiClient.get("users/" + item); - } else { - if (item.assignee) { - let id = item.assignee; - return ProcessMaker.apiClient.get("users/" + id); - } + id = item; + } else if (item && item.assignee) { + id = item.assignee; + } + if (!id) { + return Promise.resolve(null); } + return ProcessMaker.apiClient.get("users/" + id, { showAlert: false }) + .then(response => this.addUsernameToFullName(response.data)) + .catch(error => { + if (error?.config) { + error.config._defaultErrorShown = true; + } + return null; + }); }) ) .then(items => { items.forEach(item => { - results.push(this.addUsernameToFullName(item.data)); + if (item) { + results.push(item); + } }); }); let groupsPromise = Promise.all( value.groups.map(item => { + let id = null; if (typeof item == 'number' || typeof item == 'string' ) { - return ProcessMaker.apiClient.get("groups/" + item); - } else { - if (item.assignee) { - let id = this.unformatGroup(item.assignee); - return ProcessMaker.apiClient.get("groups/" + id); - } + id = item; + } else if (item && item.assignee) { + id = this.unformatGroup(item.assignee); + } + if (!id) { + return Promise.resolve(null); } + return ProcessMaker.apiClient.get("groups/" + id, { showAlert: false }) + .then(response => this.formatGroup(response.data)) + .catch(error => { + if (error?.config) { + error.config._defaultErrorShown = true; + } + return null; + }); }) ) .then(items => { items.forEach(item => { - results.push(this.formatGroup(item.data)); + if (item) { + results.push(item); + } }); }); Promise.all([usersPromise, groupsPromise]) .then(() => { this.content = results; + }) + .finally(() => { this.loading = false; }); } diff --git a/resources/js/processes/modeler/components/inspector/AssignExpression.vue b/resources/js/processes/modeler/components/inspector/AssignExpression.vue index d0b09abd22..bb249ac58b 100644 --- a/resources/js/processes/modeler/components/inspector/AssignExpression.vue +++ b/resources/js/processes/modeler/components/inspector/AssignExpression.vue @@ -26,6 +26,7 @@ :hide-users="false" :multiple="false" :activeTasksCount="true" + :error="editAssignmentUnresolvedError" /> @@ -87,6 +88,7 @@ :multiple="false" :helper="$t('If no evaluations are true')" :activeTasksCount="true" + :error="defaultAssignmentUnresolvedError" /> @@ -146,7 +148,21 @@ export default { }); let index = this.specialAssignments.indexOf(defaultAssignment[0]); return index >= 0 ? index : null; - } + }, + defaultAssignmentRule() { + return this.specialAssignments.find(assignment => assignment.default) || null; + }, + defaultAssignmentUnresolvedError() { + return this.defaultAssignmentRule && this.defaultAssignmentRule.unresolved + ? this.$t('This assignee was not found in this environment. Select a user or group.') + : null; + }, + editAssignmentUnresolvedError() { + const rule = this.editIndex !== null ? this.specialAssignments[this.editIndex] : null; + return rule && rule.unresolved + ? this.$t('This assignee was not found in this environment. Select a user or group.') + : null; + }, }, watch: { specialAssignments: { @@ -223,6 +239,10 @@ export default { methods: { addSpecialAssignment(editIndex = null) { let field; + if (!this.assignedExpression) { + this.hideAddCard(); + return; + } if (this.assignedExpression.users.length) { field = { "type" : "user", @@ -240,6 +260,10 @@ export default { "id": id }; } + if (!field) { + this.hideAddCard(); + return; + } let byExpression = { type: field.type, assignee: field.id, @@ -274,7 +298,10 @@ export default { users: [], groups: [] }; - if (this.specialAssignments[index].type == 'user') { + if (this.specialAssignments[index].unresolved) { + // Leave the picker empty; the warning above it explains why and + // saving requires picking a user/group that exists here. + } else if (this.specialAssignments[index].type == 'user') { assignee.users.push(this.specialAssignments[index].assignee); } else if (this.specialAssignments[index].type == 'group') { assignee.groups.push(parseInt(this.specialAssignments[index].assignee.replace("group-", ""))); @@ -314,7 +341,11 @@ export default { let defaultAssignment = this.specialAssignments.filter(assignment => { return assignment.default;}); if (defaultAssignment.length == 0) { return; - } + } + if (defaultAssignment[0].unresolved) { + // Keep the picker empty so it never fetches the cleared assignee + return; + } if (defaultAssignment[0].type == 'user') { this.defaultAssignment.users.push(defaultAssignment[0]); } else if (defaultAssignment[0].type == 'group') { From 6c4f6569bf4fa9cd5ea9754f472b48c83d8f1324 Mon Sep 17 00:00:00 2001 From: Teisha McRae Date: Wed, 21 Oct 2026 16:41:27 -0400 Subject: [PATCH 2/7] Add tests for reconciling assignment rules --- .../AssignmentRulesReconciliationTest.php | 186 ++++++++++++++++++ .../Jobs/ImportProcessAssignmentRulesTest.php | 132 +++++++++++++ 2 files changed, 318 insertions(+) create mode 100644 tests/Feature/ImportExport/AssignmentRulesReconciliationTest.php create mode 100644 tests/Feature/Jobs/ImportProcessAssignmentRulesTest.php diff --git a/tests/Feature/ImportExport/AssignmentRulesReconciliationTest.php b/tests/Feature/ImportExport/AssignmentRulesReconciliationTest.php new file mode 100644 index 0000000000..bf99615025 --- /dev/null +++ b/tests/Feature/ImportExport/AssignmentRulesReconciliationTest.php @@ -0,0 +1,186 @@ +create(['username' => 'ignacio']); + + $rule = (object) [ + 'type' => 'user', + 'assignee' => 999, + 'expression' => 'x==true', + 'assignmentName' => 'ignacio aranda (ignacio)', + ]; + + $reconciled = AssignmentRulesReconciliation::reconcileRuleAfterImport($rule); + + $this->assertSame($target->id, $reconciled->assignee); + $this->assertFalse(property_exists($reconciled, 'unresolved')); + } + + public function testReconcileDoesNotBindWhenStaleIdBelongsToDifferentUser(): void + { + $wrongIdUser = User::factory()->create(['username' => 'wronguser']); + $correctUser = User::factory()->create(['username' => 'ignacio']); + + $rule = (object) [ + 'type' => 'user', + 'assignee' => $wrongIdUser->id, + 'expression' => 'x==true', + 'assignmentName' => 'ignacio aranda (ignacio)', + ]; + + $reconciled = AssignmentRulesReconciliation::reconcileRuleAfterImport($rule); + + $this->assertSame($correctUser->id, $reconciled->assignee); + } + + public function testReconcileUserWithInactiveSuffixInAssignmentName(): void + { + $target = User::factory()->create(['username' => 'ignacio', 'status' => 'INACTIVE']); + + $rule = (object) [ + 'type' => 'user', + 'assignee' => 999, + 'expression' => 'x==true', + 'assignmentName' => 'ignacio aranda (ignacio - Inactive)', + ]; + + $reconciled = AssignmentRulesReconciliation::reconcileRuleAfterImport($rule); + + $this->assertSame($target->id, $reconciled->assignee); + } + + public function testReconcileMarksUnresolvedWhenUsernameNotFound(): void + { + $rule = (object) [ + 'type' => 'user', + 'assignee' => 6, + 'expression' => 'x==true', + 'assignmentName' => 'ignacio aranda (ignacio)', + ]; + + $reconciled = AssignmentRulesReconciliation::reconcileRuleAfterImport($rule); + + $this->assertNull($reconciled->assignee); + $this->assertTrue($reconciled->unresolved); + } + + public function testReconcileMarksUnresolvedWhenAssignmentNameHasNoUsername(): void + { + User::factory()->create(['username' => 'ignacio']); + + foreach ([null, 'Ignacio Aranda', '()'] as $assignmentName) { + $rule = (object) [ + 'type' => 'user', + 'assignee' => 6, + 'expression' => 'x==true', + 'assignmentName' => $assignmentName, + ]; + + $reconciled = AssignmentRulesReconciliation::reconcileRuleAfterImport($rule); + + $this->assertNull($reconciled->assignee); + $this->assertTrue($reconciled->unresolved); + } + } + + public function testReconcileGroupByAssignmentName(): void + { + $group = Group::factory()->create(['name' => 'Accounting']); + + $rule = (object) [ + 'type' => 'group', + 'assignee' => 999, + 'expression' => 'x==true', + 'assignmentName' => 'Accounting', + ]; + + $reconciled = AssignmentRulesReconciliation::reconcileRuleAfterImport($rule); + + $this->assertSame($group->id, $reconciled->assignee); + $this->assertFalse(property_exists($reconciled, 'unresolved')); + } + + public function testReconcileGroupUnresolvedWhenNameIsAmbiguousOrMissing(): void + { + Group::factory()->create(['name' => 'Accounting']); + Group::factory()->create(['name' => 'Accounting']); + + $rule = (object) [ + 'type' => 'group', + 'assignee' => 999, + 'expression' => 'x==true', + 'assignmentName' => 'Accounting', + ]; + + $reconciled = AssignmentRulesReconciliation::reconcileRuleAfterImport($rule); + + $this->assertNull($reconciled->assignee); + $this->assertTrue($reconciled->unresolved); + } + + public function testUsernameFromAssignmentName(): void + { + $this->assertSame('ignacio', AssignmentRulesReconciliation::usernameFromAssignmentName('ignacio aranda (ignacio)')); + $this->assertSame('agus', AssignmentRulesReconciliation::usernameFromAssignmentName('agus ara pelle (agus)')); + $this->assertNull(AssignmentRulesReconciliation::usernameFromAssignmentName('Ignacio Aranda')); + $this->assertNull(AssignmentRulesReconciliation::usernameFromAssignmentName(null)); + } + + public function testProcessExporterImportRebindsAssignmentRulesByUsername(): void + { + $sourceUser = User::factory()->create(['username' => 'ignacio']); + $process = $this->createProcess('process-with-different-kinds-of-assignments', [ + 'name' => 'V2AssignmentRulesImport', + ]); + Utils::setAttributeAtXPath($process, self::TASK_1, 'pm:assignment', 'rule_expression'); + Utils::setAttributeAtXPath($process, self::TASK_1, 'pm:assignmentRules', json_encode([ + [ + 'type' => 'user', + 'assignee' => 999, + 'expression' => 'form_checkbox_1==true', + 'assignmentName' => 'ignacio aranda (ignacio)', + ], + [ + 'type' => 'user', + 'assignee' => 47, + 'expression' => null, + 'assignmentName' => 'missing user (missinguser)', + 'default' => true, + ], + ])); + $process->save(); + + $targetUser = null; + $this->runExportAndImport($process, ProcessExporter::class, function () use ($process, $sourceUser, &$targetUser) { + $process->forceDelete(); + $sourceUser->forceDelete(); + $targetUser = User::factory()->create(['username' => 'ignacio']); + }); + + $imported = Process::where('name', 'V2AssignmentRulesImport')->firstOrFail(); + $raw = Utils::getAttributeAtXPath($imported, self::TASK_1, 'pm:assignmentRules'); + $rules = AssignmentRulesReconciliation::decodeRules($raw); + + $this->assertSame($targetUser->id, $rules[0]->assignee); + $this->assertFalse(property_exists($rules[0], 'unresolved')); + $this->assertNull($rules[1]->assignee); + $this->assertTrue($rules[1]->unresolved); + } +} diff --git a/tests/Feature/Jobs/ImportProcessAssignmentRulesTest.php b/tests/Feature/Jobs/ImportProcessAssignmentRulesTest.php new file mode 100644 index 0000000000..5cc57d80d7 --- /dev/null +++ b/tests/Feature/Jobs/ImportProcessAssignmentRulesTest.php @@ -0,0 +1,132 @@ +createAdminUser(); + } + + public function testV1ImportRebindsAssignmentRulesByUsername(): void + { + $targetUser = User::factory()->create(['username' => 'ignacio']); + + $payload = $this->buildV1ProcessPackage([ + [ + 'type' => 'user', + 'assignee' => 999, + 'expression' => 'form_checkbox_1==true', + 'assignmentName' => 'ignacio aranda (ignacio)', + ], + ]); + + $result = (new ImportProcess($payload))->handle(); + $imported = Process::findOrFail($result->process->id); + $rules = $this->assignmentRulesOnProcess($imported); + + $this->assertSame($targetUser->id, $rules[0]->assignee); + $this->assertFalse(property_exists($rules[0], 'unresolved')); + } + + public function testV1ImportDoesNotBindByStaleAssigneeId(): void + { + $wrongUser = User::factory()->create(['username' => 'wronguser']); + $correctUser = User::factory()->create(['username' => 'ignacio']); + + $payload = $this->buildV1ProcessPackage([ + [ + 'type' => 'user', + 'assignee' => $wrongUser->id, + 'expression' => 'form_checkbox_1==true', + 'assignmentName' => 'ignacio aranda (ignacio)', + ], + ]); + + $result = (new ImportProcess($payload))->handle(); + $imported = Process::findOrFail($result->process->id); + $rules = $this->assignmentRulesOnProcess($imported); + + $this->assertSame($correctUser->id, $rules[0]->assignee); + } + + public function testV1ImportMarksUnresolvedWhenUsernameNotFound(): void + { + $payload = $this->buildV1ProcessPackage([ + [ + 'type' => 'user', + 'assignee' => 6, + 'expression' => 'form_checkbox_1==true', + 'assignmentName' => 'ignacio aranda (ignacio)', + ], + ]); + + $result = (new ImportProcess($payload))->handle(); + $imported = Process::findOrFail($result->process->id); + $rules = $this->assignmentRulesOnProcess($imported); + + $this->assertNull($rules[0]->assignee); + $this->assertTrue($rules[0]->unresolved); + } + + /** + * @param array> $rules + */ + private function buildV1ProcessPackage(array $rules): string + { + $process = $this->createProcess('process-with-different-kinds-of-assignments', [ + 'name' => 'V1AssignmentRulesImport', + ]); + + Utils::setAttributeAtXPath($process, self::TASK_1, 'pm:assignment', 'rule_expression'); + Utils::setAttributeAtXPath($process, self::TASK_1, 'pm:assignmentRules', json_encode($rules)); + $process->save(); + + return json_encode([ + 'type' => 'process_package', + 'version' => '1', + 'process' => (object) [ + 'name' => 'V1AssignmentRulesImport', + 'description' => '', + 'status' => 'ACTIVE', + 'bpmn' => $process->bpmn, + 'created_at' => now()->toIso8601String(), + 'deleted_at' => null, + 'notifications' => (object) [], + ], + 'process_categories' => [ + (object) [ + 'name' => 'Assignment Rules Import Category', + 'status' => 'ACTIVE', + 'created_at' => now()->toIso8601String(), + ], + ], + 'screens' => [], + 'scripts' => [], + ]); + } + + /** + * @return array + */ + private function assignmentRulesOnProcess(Process $process): array + { + $raw = Utils::getAttributeAtXPath($process, self::TASK_1, 'pm:assignmentRules'); + + return AssignmentRulesReconciliation::decodeRules($raw); + } +} From 6386cf3eb0658e3a858f6dfb3049726aa94560ed Mon Sep 17 00:00:00 2001 From: Teisha McRae Date: Wed, 21 Oct 2026 17:19:56 -0400 Subject: [PATCH 3/7] Normalize group assignment IDs Group assignees can be stored as either numeric IDs or strings like group-. This change centralizes normalization so default and special assignments consistently use the plain ID value before string operations, comparisons, and persisted assignment data. --- .../components/inspector/AssignExpression.vue | 34 +++++++------------ 1 file changed, 13 insertions(+), 21 deletions(-) diff --git a/resources/js/processes/modeler/components/inspector/AssignExpression.vue b/resources/js/processes/modeler/components/inspector/AssignExpression.vue index bb249ac58b..8f4dd7276e 100644 --- a/resources/js/processes/modeler/components/inspector/AssignExpression.vue +++ b/resources/js/processes/modeler/components/inspector/AssignExpression.vue @@ -194,25 +194,13 @@ export default { "id": id, }; } else if (this.defaultAssignment.groups.length && Object.keys(this.defaultAssignment.groups[0]).length) { - let name = this.defaultAssignment.groups[0].name ? this.defaultAssignment.groups[0].name : this.defaultAssignment.groups[0].assignmentName; - let id; - if (this.defaultAssignment.groups[0].id) { - if (this.defaultAssignment.groups[0].id.includes("group")){ - id = this.defaultAssignment.groups[0].id.replace("group-", ""); - } else { - id = this.defaultAssignment.groups[0].id; - } - } else { - if (this.defaultAssignment.groups[0].assignee.includes("group")) { - id = this.defaultAssignment.groups[0].assignee.replace("group-", ""); - } else { - id = this.defaultAssignment.groups[0].assignee; - } - } + let group = this.defaultAssignment.groups[0]; + let name = group.name ? group.name : group.assignmentName; + let rawId = group.id ? group.id : group.assignee; field = { "type" : "group", "name": name, - "id": id, + "id": this.normalizeGroupId(rawId), }; } @@ -237,6 +225,12 @@ export default { } }, methods: { + // Group assignees may be numeric (e.g. after import reconciliation + // rebinds a rule) or "group-" strings from the picker; normalize + // both to the plain id string before any string operations. + normalizeGroupId(groupId) { + return String(groupId).replace("group-", ""); + }, addSpecialAssignment(editIndex = null) { let field; if (!this.assignedExpression) { @@ -252,7 +246,7 @@ export default { } else if (this.assignedExpression.groups.length) { let id = this.assignedExpression.groups[0].id; if (this.assignedExpression.groups[0].id) { - id = this.assignedExpression.groups[0].id.replace('group-', ''); + id = this.normalizeGroupId(id); } field = { "type" : "group", @@ -304,7 +298,7 @@ export default { } else if (this.specialAssignments[index].type == 'user') { assignee.users.push(this.specialAssignments[index].assignee); } else if (this.specialAssignments[index].type == 'group') { - assignee.groups.push(parseInt(this.specialAssignments[index].assignee.replace("group-", ""))); + assignee.groups.push(parseInt(this.normalizeGroupId(this.specialAssignments[index].assignee))); } this.assignedExpression = assignee; @@ -349,9 +343,7 @@ export default { if (defaultAssignment[0].type == 'user') { this.defaultAssignment.users.push(defaultAssignment[0]); } else if (defaultAssignment[0].type == 'group') { - if (typeof defaultAssignment[0].assignee != 'number') { - defaultAssignment[0].assignee = defaultAssignment[0].assignee.replace("group-", ""); - } + defaultAssignment[0].assignee = this.normalizeGroupId(defaultAssignment[0].assignee); this.defaultAssignment.groups.push(defaultAssignment[0]); } }, From 4ed176b5228d2cbef3b63b678f7c33d49b758012 Mon Sep 17 00:00:00 2001 From: Teisha McRae Date: Mon, 26 Oct 2026 10:41:50 -0400 Subject: [PATCH 4/7] Guard unresolved assignment rules Block unresolved user/group assignment rules after import and during task evaluation so they cannot assign tasks or fall through to the Process Manager. --- .../AssignmentRulesReconciliation.php | 22 ++++ ProcessMaker/Models/Process.php | 11 ++ ProcessMaker/Models/ProcessRequestToken.php | 10 ++ .../ProcessExecutionRawRepository.php | 7 ++ .../validateUnresolvedAssignmentRules.js | 42 +++++++ .../AssignmentRulesReconciliationTest.php | 21 ++++ tests/Model/ProcessRequestTokenTest.php | 105 ++++++++++++++++++ 7 files changed, 218 insertions(+) create mode 100644 resources/js/processes/modeler/validateUnresolvedAssignmentRules.js diff --git a/ProcessMaker/ImportExport/AssignmentRulesReconciliation.php b/ProcessMaker/ImportExport/AssignmentRulesReconciliation.php index f7a66067fa..74bb9c9d81 100644 --- a/ProcessMaker/ImportExport/AssignmentRulesReconciliation.php +++ b/ProcessMaker/ImportExport/AssignmentRulesReconciliation.php @@ -121,6 +121,28 @@ public static function markRuleUnresolved(object|array $rule): object|array return $rule; } + /** + * Whether a rule was left unbound after import (or equivalent). + * Matching unresolved rules must not assign or fall through to the Process Manager. + * + * @param object|array $rule + */ + public static function isUnresolvedRule(object|array $rule): bool + { + if (self::ruleGet($rule, self::KEY_UNRESOLVED) === true) { + return true; + } + + $type = self::ruleGet($rule, 'type'); + if ($type !== 'user' && $type !== 'group') { + return false; + } + + $assignee = self::ruleGet($rule, 'assignee'); + + return $assignee === null || $assignee === ''; + } + /** * @return iterable>}> */ diff --git a/ProcessMaker/Models/Process.php b/ProcessMaker/Models/Process.php index f7390f1aaa..913e672c25 100644 --- a/ProcessMaker/Models/Process.php +++ b/ProcessMaker/Models/Process.php @@ -22,6 +22,7 @@ use ProcessMaker\Exception\TaskDoesNotHaveUsersException; use ProcessMaker\Exception\ThereIsNoProcessManagerAssignedException; use ProcessMaker\Facades\WorkflowUserManager; +use ProcessMaker\ImportExport\AssignmentRulesReconciliation; use ProcessMaker\Managers\DataManager; use ProcessMaker\Nayra\Bpmn\Models\Activity; use ProcessMaker\Nayra\Contracts\Bpmn\ActivityInterface; @@ -928,6 +929,10 @@ private function getNextUserByRule($activity, $token) $formalExp->setBody($item->expression); $eval = $formalExp($instanceData); if ($eval) { + // Unresolved import assignees must not assign or escalate to the Process Manager. + if (AssignmentRulesReconciliation::isUnresolvedRule($item)) { + throw new TaskDoesNotHaveUsersException($activity->getName()); + } switch ($item->type) { case 'user_group': $users = []; @@ -1005,6 +1010,9 @@ public function getAssigneesFromExpressionRules($activity, $token) $formalExp->setBody($item->expression); $eval = $formalExp($instanceData); if ($eval) { + if (AssignmentRulesReconciliation::isUnresolvedRule($item)) { + throw new TaskDoesNotHaveUsersException($activity->getName()); + } if ($item->type === 'group') { $groups[] = $item->assignee; } elseif ($item->type === 'user') { @@ -1016,6 +1024,9 @@ public function getAssigneesFromExpressionRules($activity, $token) // If no rule was applied, use the default configured user/group if (empty($users) && empty($groups)) { foreach ($default as $item) { + if (AssignmentRulesReconciliation::isUnresolvedRule($item)) { + throw new TaskDoesNotHaveUsersException($activity->getName()); + } if ($item->type === 'group') { $groups[] = $item->assignee; } elseif ($item->type === 'user') { diff --git a/ProcessMaker/Models/ProcessRequestToken.php b/ProcessMaker/Models/ProcessRequestToken.php index baca50850f..866ab7c3a6 100644 --- a/ProcessMaker/Models/ProcessRequestToken.php +++ b/ProcessMaker/Models/ProcessRequestToken.php @@ -16,7 +16,9 @@ use ProcessMaker\Contracts\ConditionalRedirectServiceInterface; use ProcessMaker\Events\ActivityAssigned; use ProcessMaker\Events\ActivityReassignment; +use ProcessMaker\Exception\TaskDoesNotHaveUsersException; use ProcessMaker\Facades\WorkflowUserManager; +use ProcessMaker\ImportExport\AssignmentRulesReconciliation; use ProcessMaker\Managers\DataManager; use ProcessMaker\Models\MustacheExpressionEvaluator; use ProcessMaker\Nayra\Bpmn\TokenTrait; @@ -1106,6 +1108,10 @@ public function getAssigneesFromExpression(string|array|null $form_data = []): a continue; } + if (AssignmentRulesReconciliation::isUnresolvedRule($assignment)) { + throw new TaskDoesNotHaveUsersException($this->element_name ?: $this->element_id); + } + $hasExpressionMatch = true; $this->appendAssignmentAssignees($assignment, $userIds); } @@ -1116,6 +1122,10 @@ public function getAssigneesFromExpression(string|array|null $form_data = []): a continue; } + if (AssignmentRulesReconciliation::isUnresolvedRule($assignment)) { + throw new TaskDoesNotHaveUsersException($this->element_name ?: $this->element_id); + } + $this->appendAssignmentAssignees($assignment, $userIds); } } diff --git a/ProcessMaker/Repositories/ProcessExecutionRawRepository.php b/ProcessMaker/Repositories/ProcessExecutionRawRepository.php index cf7685c7bc..07c6ebf6a4 100644 --- a/ProcessMaker/Repositories/ProcessExecutionRawRepository.php +++ b/ProcessMaker/Repositories/ProcessExecutionRawRepository.php @@ -10,7 +10,9 @@ use ProcessMaker\AssignmentRules\ProcessManagerAssigned; use ProcessMaker\Exception\InvalidUserAssignmentException; use ProcessMaker\Exception\TaskDoesNotHaveRequesterException; +use ProcessMaker\Exception\TaskDoesNotHaveUsersException; use ProcessMaker\Exception\ThereIsNoProcessManagerAssignedException; +use ProcessMaker\ImportExport\AssignmentRulesReconciliation; use ProcessMaker\Managers\DataManager; use ProcessMaker\Models\FormalExpression; use ProcessMaker\Models\Group; @@ -604,6 +606,11 @@ private function getNextUserByRuleRaw( continue; } + // Unresolved import assignees must not assign or escalate to the Process Manager. + if (AssignmentRulesReconciliation::isUnresolvedRule($item)) { + throw new TaskDoesNotHaveUsersException($activity->getName()); + } + switch ($item->type) { case 'user_group': $users = []; diff --git a/resources/js/processes/modeler/validateUnresolvedAssignmentRules.js b/resources/js/processes/modeler/validateUnresolvedAssignmentRules.js new file mode 100644 index 0000000000..b4be65b76d --- /dev/null +++ b/resources/js/processes/modeler/validateUnresolvedAssignmentRules.js @@ -0,0 +1,42 @@ +/** + * Design-time validation for Rule Expression tasks whose assignees were + * left unresolved after import (or otherwise have no user/group selected). + */ +const validateUnresolvedAssignmentRules = () => ProcessMaker.EventBus.$on("modeler-validate", (node, reporter) => { + if (!["bpmn:Task", "bpmn:ManualTask", "bpmn:UserTask"].includes(node.$type)) { + return; + } + + if (node.assignment !== "rule_expression") { + return; + } + + let rules = []; + try { + rules = typeof node.assignmentRules === "string" + ? JSON.parse(node.assignmentRules || "[]") + : (node.assignmentRules || []); + } catch (e) { + return; + } + + if (!Array.isArray(rules) || rules.length === 0) { + return; + } + + const hasUnresolved = rules.some((rule) => { + if (rule && rule.unresolved === true) { + return true; + } + if (!rule || (rule.type !== "user" && rule.type !== "group")) { + return false; + } + return rule.assignee === null || rule.assignee === undefined || rule.assignee === ""; + }); + + if (hasUnresolved) { + reporter.report(node.id, "This task has an incomplete assignment. Select a user or group for each unresolved rule."); + } +}); + +export default validateUnresolvedAssignmentRules; diff --git a/tests/Feature/ImportExport/AssignmentRulesReconciliationTest.php b/tests/Feature/ImportExport/AssignmentRulesReconciliationTest.php index bf99615025..fe9c5c10fe 100644 --- a/tests/Feature/ImportExport/AssignmentRulesReconciliationTest.php +++ b/tests/Feature/ImportExport/AssignmentRulesReconciliationTest.php @@ -143,6 +143,27 @@ public function testUsernameFromAssignmentName(): void $this->assertNull(AssignmentRulesReconciliation::usernameFromAssignmentName(null)); } + public function testIsUnresolvedRule(): void + { + $this->assertTrue(AssignmentRulesReconciliation::isUnresolvedRule((object) [ + 'type' => 'user', + 'assignee' => null, + 'unresolved' => true, + ])); + $this->assertTrue(AssignmentRulesReconciliation::isUnresolvedRule([ + 'type' => 'group', + 'assignee' => null, + ])); + $this->assertFalse(AssignmentRulesReconciliation::isUnresolvedRule((object) [ + 'type' => 'user', + 'assignee' => 5, + ])); + $this->assertFalse(AssignmentRulesReconciliation::isUnresolvedRule((object) [ + 'type' => 'self_service', + 'assignee' => null, + ])); + } + public function testProcessExporterImportRebindsAssignmentRulesByUsername(): void { $sourceUser = User::factory()->create(['username' => 'ignacio']); diff --git a/tests/Model/ProcessRequestTokenTest.php b/tests/Model/ProcessRequestTokenTest.php index baa5b9bc79..d791072107 100644 --- a/tests/Model/ProcessRequestTokenTest.php +++ b/tests/Model/ProcessRequestTokenTest.php @@ -714,4 +714,109 @@ public function testGetAssigneesFromExpressionEvaluatesFeelFunctionsLikeAssignme $this->assertEquals([$matchedUser->id], $result); } + + public function testGetAssigneesFromExpressionThrowsWhenMatchingRuleIsUnresolved(): void + { + $defaultUser = User::factory()->create(['status' => 'ACTIVE']); + $process = Process::factory()->create(); + $request = ProcessRequest::factory()->create(['process_id' => $process->id]); + + $rules = [ + [ + 'type' => 'user', + 'assignee' => null, + 'expression' => 'TestVar<10', + 'assignmentName' => 'missing user (missing)', + 'unresolved' => true, + ], + [ + 'type' => 'user', + 'assignee' => $defaultUser->id, + 'expression' => null, + 'assignmentName' => 'default user', + 'default' => true, + ], + ]; + + $activity = $this->createMock(\ProcessMaker\Nayra\Contracts\Bpmn\ActivityInterface::class); + $activity->method('getProperty') + ->willReturnCallback(function ($key, $default) use ($rules) { + if ($key === 'assignmentRules') { + return json_encode($rules); + } + + return $default; + }); + + $bpmnDefinition = $this->createMock(\ProcessMaker\Nayra\Storage\BpmnElement::class); + $bpmnDefinition->method('getBpmnElementInstance') + ->willReturn($activity); + + $token = $this->getMockBuilder(ProcessRequestToken::class) + ->onlyMethods(['getBpmnDefinition']) + ->getMock(); + + $token->process_id = $process->id; + $token->process_request_id = $request->id; + $token->process = $process; + $token->element_name = 'Form Task'; + $token->setRelation('processRequest', $request); + + $token->expects($this->atLeastOnce()) + ->method('getBpmnDefinition') + ->willReturn($bpmnDefinition); + + $this->expectException(\ProcessMaker\Exception\TaskDoesNotHaveUsersException::class); + + $token->getAssigneesFromExpression(['TestVar' => 5]); + } + + public function testGetAssigneesFromExpressionThrowsWhenDefaultRuleIsUnresolved(): void + { + $process = Process::factory()->create(); + $request = ProcessRequest::factory()->create(['process_id' => $process->id]); + + $rules = [ + [ + 'type' => 'user', + 'assignee' => null, + 'expression' => null, + 'assignmentName' => 'agus ara pelle (agus)', + 'default' => true, + 'unresolved' => true, + ], + ]; + + $activity = $this->createMock(\ProcessMaker\Nayra\Contracts\Bpmn\ActivityInterface::class); + $activity->method('getProperty') + ->willReturnCallback(function ($key, $default) use ($rules) { + if ($key === 'assignmentRules') { + return json_encode($rules); + } + + return $default; + }); + + $bpmnDefinition = $this->createMock(\ProcessMaker\Nayra\Storage\BpmnElement::class); + $bpmnDefinition->method('getBpmnElementInstance') + ->willReturn($activity); + + $token = $this->getMockBuilder(ProcessRequestToken::class) + ->onlyMethods(['getBpmnDefinition']) + ->getMock(); + + $token->process_id = $process->id; + $token->process_request_id = $request->id; + $token->process = $process; + $token->element_name = 'Form Task'; + $token->setRelation('processRequest', $request); + + $token->expects($this->atLeastOnce()) + ->method('getBpmnDefinition') + ->willReturn($bpmnDefinition); + + $this->expectException(\ProcessMaker\Exception\TaskDoesNotHaveUsersException::class); + + $token->getAssigneesFromExpression(['TestVar' => 5]); + } } From bcf3f58a257e5a4bba6ec475411238beb67e1303 Mon Sep 17 00:00:00 2001 From: Teisha McRae Date: Mon, 26 Oct 2026 10:55:43 -0400 Subject: [PATCH 5/7] Reconcile assignment rules in model-only imports Move assignment-rule reconciliation into the standard import flow so BPMN-embedded rules are re-bound during saveModelOnly template imports as well as saveAllAssets imports. --- .../Exporters/ProcessExporter.php | 6 ++- resources/js/processes/modeler/initialLoad.js | 2 + .../AssignmentRulesReconciliationTest.php | 51 +++++++++++++++++++ 3 files changed, 58 insertions(+), 1 deletion(-) diff --git a/ProcessMaker/ImportExport/Exporters/ProcessExporter.php b/ProcessMaker/ImportExport/Exporters/ProcessExporter.php index fc6cc88724..8e32b543e4 100644 --- a/ProcessMaker/ImportExport/Exporters/ProcessExporter.php +++ b/ProcessMaker/ImportExport/Exporters/ProcessExporter.php @@ -128,6 +128,11 @@ public function import($existingAssetInDatabase = null, $importingFromTemplate = $this->importAssetsByMode(); + // Assignment rules are embedded in BPMN (not separate assets), so they + // must be reconciled for saveModelOnly template editing as well as + // saveAllAssets imports. + $this->importAssignmentRules(); + $process->save(); $process->notification_settings()->delete(); @@ -490,7 +495,6 @@ private function importAssetsByMode(): void $this->importScripts(); $this->importSubprocesses(); $this->importAssignments(); - $this->importAssignmentRules(); $this->importProcessLaunchpad(); $this->importElementDestination(); } diff --git a/resources/js/processes/modeler/initialLoad.js b/resources/js/processes/modeler/initialLoad.js index 36822328d6..e2883d900f 100644 --- a/resources/js/processes/modeler/initialLoad.js +++ b/resources/js/processes/modeler/initialLoad.js @@ -46,6 +46,7 @@ import Interstitial from "./components/inspector/Interstitial"; import TaskInterstitial from "./components/inspector/TaskInterstitial"; import SelectUserGroup from "../../components/SelectUserGroup"; import validateScreenRef from "./validateScreenRef"; +import validateUnresolvedAssignmentRules from "./validateUnresolvedAssignmentRules"; import validateFlowGenieRef from "./validateFlowGenieRef"; import ErrorHandlingTimeout from "./components/inspector/ErrorHandlingTimeout"; import ErrorHandlingRetryAttempts from "./components/inspector/ErrorHandlingRetryAttempts"; @@ -523,3 +524,4 @@ ProcessMaker.EventBus.$on( validateScreenRef(); validateFlowGenieRef(); +validateUnresolvedAssignmentRules(); diff --git a/tests/Feature/ImportExport/AssignmentRulesReconciliationTest.php b/tests/Feature/ImportExport/AssignmentRulesReconciliationTest.php index fe9c5c10fe..f69c6b3dd3 100644 --- a/tests/Feature/ImportExport/AssignmentRulesReconciliationTest.php +++ b/tests/Feature/ImportExport/AssignmentRulesReconciliationTest.php @@ -4,6 +4,7 @@ use ProcessMaker\ImportExport\AssignmentRulesReconciliation; use ProcessMaker\ImportExport\Exporters\ProcessExporter; +use ProcessMaker\ImportExport\Options; use ProcessMaker\ImportExport\Utils; use ProcessMaker\Models\Group; use ProcessMaker\Models\Process; @@ -204,4 +205,54 @@ public function testProcessExporterImportRebindsAssignmentRulesByUsername(): voi $this->assertNull($rules[1]->assignee); $this->assertTrue($rules[1]->unresolved); } + + public function testSaveModelOnlyTemplateImportRebindsAssignmentRulesByUsername(): void + { + $this->addGlobalSignalProcess(); + + $sourceUser = User::factory()->create(['username' => 'ignacio']); + // Local user whose ID collides with the exported assignee — must not + // win over username matching when editing a saveModelOnly template. + $wrongUserAtSameId = User::factory()->create(['username' => 'wronguser']); + + $process = $this->createProcess('process-with-different-kinds-of-assignments', [ + 'name' => 'TemplateAssignmentRulesImport', + ]); + Utils::setAttributeAtXPath($process, self::TASK_1, 'pm:assignment', 'rule_expression'); + Utils::setAttributeAtXPath($process, self::TASK_1, 'pm:assignmentRules', json_encode([ + [ + 'type' => 'user', + 'assignee' => $wrongUserAtSameId->id, + 'expression' => 'form_checkbox_1==true', + 'assignmentName' => 'ignacio aranda (ignacio)', + ], + ])); + $process->save(); + + $payload = $this->export($process, ProcessExporter::class, null, true); + + $process->forceDelete(); + $sourceUser->forceDelete(); + // Keep $wrongUserAtSameId so an unreconciled import would bind them. + + $targetUser = User::factory()->create(['username' => 'ignacio']); + + // Per-process options: global saveModelOnly would also hit non-BPMN + // assets and break Manifest::updateBPMNDefinitions. + $this->import($payload, new Options([ + $payload['root'] => [ + 'mode' => 'copy', + 'saveAssetsMode' => 'saveModelOnly', + 'isTemplate' => true, + ], + ])); + + $imported = Process::where('name', 'TemplateAssignmentRulesImport')->firstOrFail(); + $raw = Utils::getAttributeAtXPath($imported, self::TASK_1, 'pm:assignmentRules'); + $rules = AssignmentRulesReconciliation::decodeRules($raw); + + $this->assertSame($targetUser->id, $rules[0]->assignee); + $this->assertNotSame($wrongUserAtSameId->id, $rules[0]->assignee); + $this->assertFalse(property_exists($rules[0], 'unresolved')); + } } From 653c95b43fff54d28026923f0e73774ab31ac0e6 Mon Sep 17 00:00:00 2001 From: Teisha McRae Date: Mon, 26 Oct 2026 11:56:27 -0400 Subject: [PATCH 6/7] Handle translated inactive user suffixes Import/export reconciliation now recognizes SelectUserGroup's inactive-user suffix across locales, not just the English "Inactive" label. --- .../AssignmentRulesReconciliation.php | 59 ++++++++++++++++++- .../AssignmentRulesReconciliationTest.php | 40 ++++++++++--- 2 files changed, 89 insertions(+), 10 deletions(-) diff --git a/ProcessMaker/ImportExport/AssignmentRulesReconciliation.php b/ProcessMaker/ImportExport/AssignmentRulesReconciliation.php index 74bb9c9d81..56c4816341 100644 --- a/ProcessMaker/ImportExport/AssignmentRulesReconciliation.php +++ b/ProcessMaker/ImportExport/AssignmentRulesReconciliation.php @@ -25,6 +25,13 @@ class AssignmentRulesReconciliation private const HUMAN_TASK_TAGS = ['task', 'userTask', 'manualTask']; + /** + * Cached labels for SelectUserGroup's inactive suffix ($t('Inactive')). + * + * @var array|null + */ + private static ?array $inactiveStatusLabels = null; + /** * @return array> */ @@ -96,7 +103,9 @@ public static function reconcileRuleAfterImport(object|array $rule): object|arra /** * Extract the username from "Full Name (username)", tolerating the - * " - Inactive" suffix SelectUserGroup appends for inactive users. + * inactive suffix SelectUserGroup appends via $t('Inactive') + * The assignmentName was written in whatever locale the designer used, + * so import must recognize every available translation of that key. */ public static function usernameFromAssignmentName(mixed $assignmentName): ?string { @@ -104,11 +113,57 @@ public static function usernameFromAssignmentName(mixed $assignmentName): ?strin return null; } - $username = trim(preg_replace('/\s+-\s+Inactive$/i', '', trim($matches[1]))); + $username = self::stripInactiveStatusSuffix(trim($matches[1])); return $username !== '' ? $username : null; } + /** + * Labels used by SelectUserGroup for inactive users: " - " + $t('Inactive'). + * + * @return array + */ + public static function inactiveStatusLabels(): array + { + if (self::$inactiveStatusLabels !== null) { + return self::$inactiveStatusLabels; + } + + $labels = ['Inactive']; + foreach (glob(resource_path('lang/*.json')) ?: [] as $file) { + $locale = basename($file, '.json'); + $translated = trans('Inactive', [], $locale); + if (is_string($translated) && $translated !== '') { + $labels[] = $translated; + } + } + + self::$inactiveStatusLabels = array_values(array_unique($labels)); + + return self::$inactiveStatusLabels; + } + + /** + * Reset cached inactive labels (for tests). + */ + public static function clearInactiveStatusLabelCache(): void + { + self::$inactiveStatusLabels = null; + } + + private static function stripInactiveStatusSuffix(string $username): string + { + foreach (self::inactiveStatusLabels() as $label) { + $quoted = preg_quote($label, '/'); + $stripped = preg_replace('/\s+-\s+' . $quoted . '$/iu', '', $username); + if (is_string($stripped) && $stripped !== $username) { + return trim($stripped); + } + } + + return $username; + } + /** * @param object|array $rule * @return object|array diff --git a/tests/Feature/ImportExport/AssignmentRulesReconciliationTest.php b/tests/Feature/ImportExport/AssignmentRulesReconciliationTest.php index f69c6b3dd3..2915026603 100644 --- a/tests/Feature/ImportExport/AssignmentRulesReconciliationTest.php +++ b/tests/Feature/ImportExport/AssignmentRulesReconciliationTest.php @@ -55,16 +55,28 @@ public function testReconcileUserWithInactiveSuffixInAssignmentName(): void { $target = User::factory()->create(['username' => 'ignacio', 'status' => 'INACTIVE']); - $rule = (object) [ - 'type' => 'user', - 'assignee' => 999, - 'expression' => 'x==true', - 'assignmentName' => 'ignacio aranda (ignacio - Inactive)', - ]; + foreach ([ + 'ignacio aranda (ignacio - Inactive)', + 'ignacio aranda (ignacio - inactivo)', + 'ignacio aranda (ignacio - inactif)', + 'ignacio aranda (ignacio - Inaktiv)', + ] as $assignmentName) { + $rule = (object) [ + 'type' => 'user', + 'assignee' => 999, + 'expression' => 'x==true', + 'assignmentName' => $assignmentName, + ]; - $reconciled = AssignmentRulesReconciliation::reconcileRuleAfterImport($rule); + $reconciled = AssignmentRulesReconciliation::reconcileRuleAfterImport($rule); - $this->assertSame($target->id, $reconciled->assignee); + $this->assertSame( + $target->id, + $reconciled->assignee, + "Failed to rebind from assignmentName: {$assignmentName}" + ); + $this->assertFalse(property_exists($reconciled, 'unresolved')); + } } public function testReconcileMarksUnresolvedWhenUsernameNotFound(): void @@ -138,10 +150,22 @@ public function testReconcileGroupUnresolvedWhenNameIsAmbiguousOrMissing(): void public function testUsernameFromAssignmentName(): void { + AssignmentRulesReconciliation::clearInactiveStatusLabelCache(); + $this->assertSame('ignacio', AssignmentRulesReconciliation::usernameFromAssignmentName('ignacio aranda (ignacio)')); $this->assertSame('agus', AssignmentRulesReconciliation::usernameFromAssignmentName('agus ara pelle (agus)')); + $this->assertSame('ignacio', AssignmentRulesReconciliation::usernameFromAssignmentName('ignacio aranda (ignacio - Inactive)')); + // SelectUserGroup uses $t('Inactive'); Spanish/French/German bake the + // translated suffix into assignmentName at design time. + $this->assertSame('ignacio', AssignmentRulesReconciliation::usernameFromAssignmentName('ignacio aranda (ignacio - inactivo)')); + $this->assertSame('ignacio', AssignmentRulesReconciliation::usernameFromAssignmentName('ignacio aranda (ignacio - inactif)')); + $this->assertSame('ignacio', AssignmentRulesReconciliation::usernameFromAssignmentName('ignacio aranda (ignacio - Inaktiv)')); $this->assertNull(AssignmentRulesReconciliation::usernameFromAssignmentName('Ignacio Aranda')); $this->assertNull(AssignmentRulesReconciliation::usernameFromAssignmentName(null)); + + $labels = AssignmentRulesReconciliation::inactiveStatusLabels(); + $this->assertContains('Inactive', $labels); + $this->assertContains('inactivo', $labels); } public function testIsUnresolvedRule(): void From 9152e0ae4973e1ee5d53b303a71771f66382e8ae Mon Sep 17 00:00:00 2001 From: Teisha McRae Date: Mon, 26 Oct 2026 16:02:39 -0400 Subject: [PATCH 7/7] Skip unresolved assignment rules The reassign assignee lookup no longer throws on unresolved expression/default assignment rules. These rules are skipped during picker resolution so the UI does not fall through to Default Assignment, while runtime assignment behavior remains unchanged and still resolves via getNextUserByRule. Updated tests cover the skip behavior for matching and default unresolved rules. --- ProcessMaker/Models/ProcessRequestToken.php | 10 ++++++---- tests/Model/ProcessRequestTokenTest.php | 19 +++++++++++-------- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/ProcessMaker/Models/ProcessRequestToken.php b/ProcessMaker/Models/ProcessRequestToken.php index 866ab7c3a6..3f09ddb2da 100644 --- a/ProcessMaker/Models/ProcessRequestToken.php +++ b/ProcessMaker/Models/ProcessRequestToken.php @@ -16,7 +16,6 @@ use ProcessMaker\Contracts\ConditionalRedirectServiceInterface; use ProcessMaker\Events\ActivityAssigned; use ProcessMaker\Events\ActivityReassignment; -use ProcessMaker\Exception\TaskDoesNotHaveUsersException; use ProcessMaker\Facades\WorkflowUserManager; use ProcessMaker\ImportExport\AssignmentRulesReconciliation; use ProcessMaker\Managers\DataManager; @@ -1108,11 +1107,14 @@ public function getAssigneesFromExpression(string|array|null $form_data = []): a continue; } + // In reassign picker, skip unresolved rules without + // throwing. Runtime assignment still fails via getNextUserByRule. + // Mark as matched so we do not fall through to Default Assignment. + $hasExpressionMatch = true; if (AssignmentRulesReconciliation::isUnresolvedRule($assignment)) { - throw new TaskDoesNotHaveUsersException($this->element_name ?: $this->element_id); + continue; } - $hasExpressionMatch = true; $this->appendAssignmentAssignees($assignment, $userIds); } @@ -1123,7 +1125,7 @@ public function getAssigneesFromExpression(string|array|null $form_data = []): a } if (AssignmentRulesReconciliation::isUnresolvedRule($assignment)) { - throw new TaskDoesNotHaveUsersException($this->element_name ?: $this->element_id); + continue; } $this->appendAssignmentAssignees($assignment, $userIds); diff --git a/tests/Model/ProcessRequestTokenTest.php b/tests/Model/ProcessRequestTokenTest.php index d791072107..dc27c88be6 100644 --- a/tests/Model/ProcessRequestTokenTest.php +++ b/tests/Model/ProcessRequestTokenTest.php @@ -715,10 +715,10 @@ public function testGetAssigneesFromExpressionEvaluatesFeelFunctionsLikeAssignme $this->assertEquals([$matchedUser->id], $result); } - public function testGetAssigneesFromExpressionThrowsWhenMatchingRuleIsUnresolved(): void + public function testGetAssigneesFromExpressionSkipsUnresolvedMatchingRuleWithoutFallingThroughToDefault(): void { $defaultUser = User::factory()->create(['status' => 'ACTIVE']); - $process = Process::factory()->create(); + $process = Process::factory()->create(['manager_id' => []]); $request = ProcessRequest::factory()->create(['process_id' => $process->id]); $rules = [ @@ -766,14 +766,16 @@ public function testGetAssigneesFromExpressionThrowsWhenMatchingRuleIsUnresolved ->method('getBpmnDefinition') ->willReturn($bpmnDefinition); - $this->expectException(\ProcessMaker\Exception\TaskDoesNotHaveUsersException::class); + $result = $token->getAssigneesFromExpression(['TestVar' => 5]); - $token->getAssigneesFromExpression(['TestVar' => 5]); + // Listing must not 500, and must not surface the Default Assignment + // as if the unresolved expression rule had been skipped. + $this->assertNotContains($defaultUser->id, $result); } - public function testGetAssigneesFromExpressionThrowsWhenDefaultRuleIsUnresolved(): void + public function testGetAssigneesFromExpressionSkipsUnresolvedDefaultRule(): void { - $process = Process::factory()->create(); + $process = Process::factory()->create(['manager_id' => []]); $request = ProcessRequest::factory()->create(['process_id' => $process->id]); $rules = [ @@ -815,8 +817,9 @@ public function testGetAssigneesFromExpressionThrowsWhenDefaultRuleIsUnresolved( ->method('getBpmnDefinition') ->willReturn($bpmnDefinition); - $this->expectException(\ProcessMaker\Exception\TaskDoesNotHaveUsersException::class); + $result = $token->getAssigneesFromExpression(['TestVar' => 5]); - $token->getAssigneesFromExpression(['TestVar' => 5]); + $this->assertIsArray($result); + $this->assertSame([], $result); } }