diff --git a/ProcessMaker/Http/Controllers/Api/UserController.php b/ProcessMaker/Http/Controllers/Api/UserController.php index e7e62248ee..351a1c84ab 100644 --- a/ProcessMaker/Http/Controllers/Api/UserController.php +++ b/ProcessMaker/Http/Controllers/Api/UserController.php @@ -8,6 +8,7 @@ use Illuminate\Support\Facades\Auth; use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\Hash; +use Illuminate\Validation\Rule; use ProcessMaker\Events\UserCreated; use ProcessMaker\Events\UserDeleted; use ProcessMaker\Events\UserGroupMembershipUpdated; @@ -63,6 +64,7 @@ class UserController extends Controller 'timezone', 'datetime_format', 'status', + 'delegation_user_id', 'avatar', 'preferences_2fa', 'connected_accounts', @@ -79,6 +81,17 @@ class UserController extends Controller 'disableRecommendations', ]; + /** + * Statuses a personal-profile user may set on their own account. + * + * @var array + */ + private const SELF_SERVICE_STATUSES = [ + 'ACTIVE', + 'INACTIVE', + 'OUT_OF_OFFICE', + ]; + /** * Display a listing of the resource. * @@ -503,8 +516,23 @@ public function update(User $user, Request $request) $isSelfServiceUpdate = $this->authorizeSelfServiceUpdate($authenticatedUser, $user, $fields); $rules = User::rules($user); if ($isSelfServiceUpdate) { + $fields = $this->forgetUnassignableDelegation($fields); + if (array_key_exists('delegation_user_id', $fields)) { + $request->json()->set('delegation_user_id', $fields['delegation_user_id']); + } $rules['meta'] = ['sometimes', 'array']; $rules['meta.disableRecommendations'] = ['sometimes', 'boolean']; + $rules['status'] = ['required', Rule::in($this->selfServiceStatuses($user))]; + $rules['delegation_user_id'] = [ + 'nullable', + 'integer', + Rule::exists('users', 'id')->where(function ($query) { + $query->where('is_system', false) + ->whereNull('deleted_at') + ->where('status', 'ACTIVE'); + }), + Rule::notIn([(string) $user->id]), + ]; } $request->validate($rules); if ($isSelfServiceUpdate) { @@ -646,6 +674,46 @@ private function authorizeSelfServiceUpdate(User $authenticatedUser, User $targe return true; } + /** + * Statuses this user may submit. An administrator-assigned status that is + * not otherwise available stays valid so saving the profile does not + * reject the account's current value. + * + * @return array + */ + private function selfServiceStatuses(User $user): array + { + $statuses = self::SELF_SERVICE_STATUSES; + if ($user->status && !in_array($user->status, $statuses, true)) { + $statuses[] = $user->status; + } + + return $statuses; + } + + /** + * A replacement the profile picker can no longer load should not fail the + * rest of the save. Clear missing, deleted, system, and non-ACTIVE users. + */ + private function forgetUnassignableDelegation(array $fields): array + { + if (empty($fields['delegation_user_id'])) { + return $fields; + } + + $delegate = User::withTrashed()->find($fields['delegation_user_id']); + if ( + !$delegate + || $delegate->trashed() + || $delegate->is_system + || $delegate->status !== 'ACTIVE' + ) { + $fields['delegation_user_id'] = null; + } + + return $fields; + } + /** * Merge self-service metadata into the persisted server-managed values. */ diff --git a/ProcessMaker/Http/Controllers/ProfileController.php b/ProcessMaker/Http/Controllers/ProfileController.php index 0fdfae2e84..a0d5a206b3 100644 --- a/ProcessMaker/Http/Controllers/ProfileController.php +++ b/ProcessMaker/Http/Controllers/ProfileController.php @@ -61,11 +61,13 @@ function ($result, $item) { $currentUser->preferences_2fa = $currentUser->getValid2FAPreferences(); $is2FAEnabledForGroup = $currentUser->in2FAGroupOrIndependent(); - $addons = $this->getPluginAddons('edit', []); + $addons = $this->getPluginAddons('edit', ['user' => $currentUser]); + $addonsSettings = $this->getPluginAddons('edit.settings', ['user' => $currentUser]); return view('profile.edit', compact('currentUser', 'states', 'timezones', 'countries', 'datetimeFormats', - 'status', 'enabled2FA', 'global2FAEnabled', 'is2FAEnabledForGroup', 'addons', 'ssoUser')); + 'status', 'enabled2FA', 'global2FAEnabled', 'is2FAEnabledForGroup', 'addons', + 'addonsSettings', 'ssoUser')); } /** diff --git a/ProcessMaker/Models/Process.php b/ProcessMaker/Models/Process.php index f7390f1aaa..b87cde0286 100644 --- a/ProcessMaker/Models/Process.php +++ b/ProcessMaker/Models/Process.php @@ -724,6 +724,7 @@ public function scalateToManagerIfEnabled($user, $activity, $token, $assignmentT if (app()->bound('workflow.UserManager') && $assignmentProcess) { $config = json_decode($activity->getProperty('config', '{}'), true); $escalateToManager = $config['escalateToManager'] ?? false; + $assignee = User::find($user); if ($escalateToManager) { $user = WorkflowUserManager::escalateToManager($token, $user); } else { @@ -732,7 +733,10 @@ public function scalateToManagerIfEnabled($user, $activity, $token, $assignmentT 'process_id' => $this->id, 'request_id' => $token->getInstance()->getId(), ]); - $user = $res['assign_to']; + $assignedUserId = $res['assign_to'] ?? null; + $user = $assignee + ? $assignee->acceptResolvedAssignee($assignedUserId) + : $assignedUserId; } } } @@ -806,9 +810,9 @@ private function getNextUserFromProcessVariable($activity, $token) $outOfOffice = User::whereIn('id', array_unique($assignedUsers))->where('status', 'OUT_OF_OFFICE')->get(); foreach ($outOfOffice as $user) { - $delegation = $user->delegationUser()->pluck('id')->toArray(); - if ($delegation) { - $users[] = $delegation[0]; + $delegationId = $user->activeDelegationUserId(); + if ($delegationId) { + $users[] = $delegationId; } } diff --git a/ProcessMaker/Models/ProcessRequestToken.php b/ProcessMaker/Models/ProcessRequestToken.php index b65c1b0a06..178e166db0 100644 --- a/ProcessMaker/Models/ProcessRequestToken.php +++ b/ProcessMaker/Models/ProcessRequestToken.php @@ -12,6 +12,7 @@ use Illuminate\Support\Facades\Notification; use Laravel\Scout\Searchable; use Log; +use ProcessMaker\AssignmentRules\ProcessManagerAssigned; use ProcessMaker\Casts\MillisecondsToDateCast; use ProcessMaker\Contracts\ConditionalRedirectServiceInterface; use ProcessMaker\Events\ActivityAssigned; @@ -1323,12 +1324,43 @@ public function reassignTo($userId) 'process_id' => $this->process_id, 'request_id' => $this->process_request_id, ]); - $this->user_id = $res['assign_to']; + $assignedUserId = $res['assign_to'] ?? null; + $this->user_id = $this->resolveAssignedUserId($userId, $assignedUserId); + $this->unsetRelation('user'); } return $this; } + /** + * Assignee to store after the assignment process runs. When nobody can + * take the task, it falls back to the Process Manager instead of being + * left unassigned. + * + * @param mixed $requestedUserId + * @param mixed $assignedUserId + * @return mixed + */ + public function resolveAssignedUserId($requestedUserId, $assignedUserId) + { + $assignee = User::find($requestedUserId); + $resolvedUserId = $assignee + ? $assignee->acceptResolvedAssignee($assignedUserId) + : $assignedUserId; + + return $resolvedUserId ?: $this->processManagerId(); + } + + /** + * @return int + */ + private function processManagerId(): int + { + $request = $this->processRequest; + + return (new ProcessManagerAssigned())->getNextUser($this->getDefinition(true), $this, $request->process, $request); + } + /** * Returns True is the tokens belongs to a MultiInstance Task * @@ -1449,8 +1481,11 @@ public function reassign($toUserId, User $requestingUser, $comments = '') } // Send a notification to the user - $notification = new TaskReassignmentNotification($this); - $this->user->notify($notification); + $this->unsetRelation('user'); + if ($this->user) { + $notification = new TaskReassignmentNotification($this); + $this->user->notify($notification); + } if ($this->element_type === 'task') { event(new ActivityAssigned($this)); } diff --git a/ProcessMaker/Models/User.php b/ProcessMaker/Models/User.php index 5f15ac4e36..d48c2baad6 100644 --- a/ProcessMaker/Models/User.php +++ b/ProcessMaker/Models/User.php @@ -510,6 +510,37 @@ public function delegationUser() return $this->belongsTo(self::class); } + /** + * Replacement who can take a task right now. A stored replacement can + * become inactive or out of office after it was selected. + */ + public function activeDelegationUserId(): ?int + { + $delegateId = $this->delegationUser()->where('status', 'ACTIVE')->value('id'); + + return $delegateId ? (int) $delegateId : null; + } + + /** + * Keep a resolved assignee unless it is this user's replacement and that + * replacement cannot take a task. + * + * @param mixed $resolvedUserId + * @return mixed + */ + public function acceptResolvedAssignee($resolvedUserId) + { + if (!$resolvedUserId || (int) $resolvedUserId === (int) $this->id) { + return $resolvedUserId; + } + + if ((int) $this->delegation_user_id !== (int) $resolvedUserId) { + return $resolvedUserId; + } + + return $this->activeDelegationUserId() ? $resolvedUserId : null; + } + /** * User's Manager are user associations that allow for automatic reassignment based on specific rules in the task assignment. * diff --git a/ProcessMaker/Repositories/ProcessExecutionRawRepository.php b/ProcessMaker/Repositories/ProcessExecutionRawRepository.php index cf7685c7bc..dbb1d1fb66 100644 --- a/ProcessMaker/Repositories/ProcessExecutionRawRepository.php +++ b/ProcessMaker/Repositories/ProcessExecutionRawRepository.php @@ -350,7 +350,10 @@ public function getUserByIdRaw(?int $userId): ?User return null; } - $row = DB::selectOne('SELECT * FROM users WHERE id = ? LIMIT 1', [$userId]); + $row = DB::selectOne( + 'SELECT * FROM users WHERE id = ? AND deleted_at IS NULL LIMIT 1', + [$userId] + ); if (!$row) { return null; } @@ -566,8 +569,15 @@ private function getNextUserFromProcessVariableRaw( $oooPlaceholders = implode(',', array_fill(0, count($uniqueUsers), '?')); $oooRows = DB::select( - "SELECT delegation_user_id FROM users WHERE id IN ($oooPlaceholders) AND status = ? AND delegation_user_id IS NOT NULL", - array_merge($uniqueUsers, ['OUT_OF_OFFICE']) + "SELECT delegate.id AS delegation_user_id + FROM users + INNER JOIN users AS delegate ON delegate.id = users.delegation_user_id + WHERE users.id IN ($oooPlaceholders) + AND users.status = ? + AND users.delegation_user_id IS NOT NULL + AND delegate.status = ? + AND delegate.deleted_at IS NULL", + array_merge($uniqueUsers, ['OUT_OF_OFFICE', 'ACTIVE']) ); foreach ($oooRows as $row) { $users[(int) $row->delegation_user_id] = (int) $row->delegation_user_id; diff --git a/resources/views/profile/edit.blade.php b/resources/views/profile/edit.blade.php index 2a067be1c9..6f022f02f1 100644 --- a/resources/views/profile/edit.blade.php +++ b/resources/views/profile/edit.blade.php @@ -148,11 +148,13 @@ 'timezone', 'datetime_format', 'status', + 'delegation_user_id', 'avatar', 'preferences_2fa', 'connected_accounts', 'valpassword', ]; + const isSelfServiceProfile = @json(!(Auth::user()->is_administrator || Auth::user()->hasPermission('edit-users'))); let formVueInstance = new Vue({ el: '#editProfile', mixins:addons, @@ -312,7 +314,8 @@ if (this.image === false) { this.formData.avatar = false; } - ProcessMaker.apiClient.put('users/' + this.formData.id, this.formData) + const payload = isSelfServiceProfile ? this.profilePayload() : this.formData; + ProcessMaker.apiClient.put('users/' + this.formData.id, payload) .then((response) => { // reset the slack configuration error this.slackConfigurationError = false; diff --git a/tests/Feature/Api/TaskAssignmentByVariableTest.php b/tests/Feature/Api/TaskAssignmentByVariableTest.php index 1d62397894..ea75afe1a1 100644 --- a/tests/Feature/Api/TaskAssignmentByVariableTest.php +++ b/tests/Feature/Api/TaskAssignmentByVariableTest.php @@ -250,6 +250,39 @@ public function testProcessVariableAssignmentWithObjectVariable() $this->assertEquals($users->first()->id, $task->user_id); } + public function testOutOfOfficeAssignmentUsesOnlyAnActiveReplacement() + { + $activeReplacement = User::factory()->create(['status' => 'ACTIVE']); + $outOfOffice = User::factory()->create([ + 'status' => 'OUT_OF_OFFICE', + 'delegation_user_id' => $activeReplacement->id, + ]); + $process = $this->createProcess('process_variable', 'usersVariable', '', '', false); + + $response = $this->startTestProcess($process, ['usersVariable' => $outOfOffice->id]); + $task = ProcessRequestToken::where([ + 'process_request_id' => $response['id'], + 'status' => 'ACTIVE', + ])->firstOrFail(); + $this->assertEquals($activeReplacement->id, $task->user_id); + + $manager = User::factory()->create(['status' => 'ACTIVE']); + foreach (['INACTIVE', 'OUT_OF_OFFICE'] as $status) { + $unavailableReplacement = User::factory()->create(['status' => $status]); + $outOfOffice->delegation_user_id = $unavailableReplacement->id; + $outOfOffice->save(); + $process = $this->createProcess('process_variable', 'usersVariable', '', '', false); + $process->manager_id = [$manager->id]; + $process->save(); + + $response = $this->startTestProcess($process, ['usersVariable' => $outOfOffice->id]); + $assignedToReplacement = ProcessRequestToken::where('process_request_id', $response['id']) + ->where('user_id', $unavailableReplacement->id) + ->exists(); + $this->assertFalse($assignedToReplacement); + } + } + public function testProcessVariableAssignmentWithLiteralId() { // Create users of a group and a user without group diff --git a/tests/Feature/Api/TaskAssignmentTest.php b/tests/Feature/Api/TaskAssignmentTest.php index 58f2ec7f3e..1c1544d1a2 100644 --- a/tests/Feature/Api/TaskAssignmentTest.php +++ b/tests/Feature/Api/TaskAssignmentTest.php @@ -130,6 +130,42 @@ public function testUpdateTaskAssignment() $this->assertEquals('ProcessMaker\Models\Group', $assignment->assignment_type); } + /** + * An unavailable replacement is resolved to the process manager + */ + public function testUnavailableReplacementResolvesToProcessManager() + { + $manager = User::factory()->create(['status' => 'ACTIVE']); + $process = Process::factory()->create([ + 'status' => 'ACTIVE', + 'bpmn' => file_get_contents(__DIR__ . '/processes/InvalidUserAssignment.bpmn'), + ]); + $process->manager_id = $manager->id; + $process->save(); + $instance = $this->startProcess($process, 'node_1'); + $token = $instance->tokens()->where('status', 'ACTIVE')->firstOrFail(); + + $replacement = User::factory()->create(['status' => 'INACTIVE']); + $outOfOffice = User::factory()->create([ + 'status' => 'OUT_OF_OFFICE', + 'delegation_user_id' => $replacement->id, + ]); + + $this->assertSame( + $manager->id, + $token->resolveAssignedUserId($outOfOffice->id, $replacement->id) + ); + + $activeReplacement = User::factory()->create(['status' => 'ACTIVE']); + $outOfOffice->delegation_user_id = $activeReplacement->id; + $outOfOffice->save(); + $this->assertSame( + $activeReplacement->id, + $token->resolveAssignedUserId($outOfOffice->id, $activeReplacement->id) + ); + $this->assertSame($manager->id, $token->resolveAssignedUserId($outOfOffice->id, null)); + } + /** * Invalid user assignment is reassigned to the process manager */ diff --git a/tests/Feature/Api/UsersTest.php b/tests/Feature/Api/UsersTest.php index 5d3ade9fb1..4205ef609c 100644 --- a/tests/Feature/Api/UsersTest.php +++ b/tests/Feature/Api/UsersTest.php @@ -960,7 +960,6 @@ public function testSelfServiceUpdateRejectsDisallowedFieldsWithoutPersistingCha $originalFirstname = $this->user->firstname; $disallowedFields = [ 'manager_id' => User::factory()->create()->id, - 'delegation_user_id' => User::factory()->create()->id, 'schedule' => ['monday' => []], 'force_change_password' => true, 'is_administrator' => true, @@ -989,6 +988,82 @@ public function testSelfServiceUpdateRejectsDisallowedFieldsWithoutPersistingCha ]); } + public function testSelfServiceUpdateClearsStaleInactiveDelegationOnSave(): void + { + $this->user = User::factory()->create(['is_administrator' => false, 'status' => 'ACTIVE']); + $this->user->giveDirectPermission('edit-personal-profile'); + $inactiveDelegate = User::factory()->create(['status' => 'INACTIVE']); + $this->user->delegation_user_id = $inactiveDelegate->id; + $this->user->status = 'OUT_OF_OFFICE'; + $this->user->save(); + $this->user->refresh(); + $this->flushSession(); + + $response = $this->apiCall( + 'PUT', + self::API_TEST_URL . '/' . $this->user->id, + $this->getSelfServiceUpdateData($this->user, [ + 'status' => 'OUT_OF_OFFICE', + 'delegation_user_id' => $inactiveDelegate->id, + ]) + ); + + $response->assertStatus(204); + $this->assertDatabaseHas('users', [ + 'id' => $this->user->id, + 'status' => 'OUT_OF_OFFICE', + 'delegation_user_id' => null, + ]); + } + + public function testSelfServiceUpdateCanSetOutOfOfficeAndDelegation(): void + { + $this->user = User::factory()->create(['is_administrator' => false, 'status' => 'ACTIVE']); + $this->user->giveDirectPermission('edit-personal-profile'); + $this->user->refresh(); + $this->flushSession(); + $delegate = User::factory()->create(['status' => 'ACTIVE']); + + $response = $this->apiCall( + 'PUT', + self::API_TEST_URL . '/' . $this->user->id, + $this->getSelfServiceUpdateData($this->user, [ + 'status' => 'OUT_OF_OFFICE', + 'delegation_user_id' => $delegate->id, + ]) + ); + + $response->assertStatus(204); + $this->assertDatabaseHas('users', [ + 'id' => $this->user->id, + 'status' => 'OUT_OF_OFFICE', + 'delegation_user_id' => $delegate->id, + ]); + } + + public function testSelfServiceUpdateRejectsBlockedAndScheduledStatuses(): void + { + $this->user = User::factory()->create(['is_administrator' => false, 'status' => 'ACTIVE']); + $this->user->giveDirectPermission('edit-personal-profile'); + $this->user->refresh(); + $this->flushSession(); + + foreach (['BLOCKED', 'SCHEDULED'] as $status) { + $response = $this->apiCall( + 'PUT', + self::API_TEST_URL . '/' . $this->user->id, + $this->getSelfServiceUpdateData($this->user, ['status' => $status]) + ); + + $response->assertStatus(422)->assertJsonValidationErrors('status'); + } + + $this->assertDatabaseHas('users', [ + 'id' => $this->user->id, + 'status' => 'ACTIVE', + ]); + } + public function testSelfServiceUpdateRejectsServerManagedMetadataWithoutPersistingChanges(): void { $this->user = User::factory()->create([ diff --git a/tests/Feature/ProfileTest.php b/tests/Feature/ProfileTest.php index 666ee2b569..3f498e1559 100644 --- a/tests/Feature/ProfileTest.php +++ b/tests/Feature/ProfileTest.php @@ -3,6 +3,7 @@ namespace Tests\Feature; use Illuminate\Support\Facades\Artisan; +use ProcessMaker\Http\Controllers\ProfileController; use ProcessMaker\Models\Group; use ProcessMaker\Models\GroupMember; use ProcessMaker\Models\Permission; @@ -91,4 +92,40 @@ public function testEditProfileGroupPermission(): void $response = $this->actingAs($user, 'web')->call('GET', $url); $response->assertStatus(200); } + + public function testEditProfilePassesSignedInUserToSettingsAddons(): void + { + Artisan::call('db:seed', ['class' => 'PermissionSeeder']); + $user = User::factory()->create([ + 'is_administrator' => false, + 'delegation_user_id' => User::factory()->create()->id, + ]); + $user->permissions()->attach(Permission::byName('edit-personal-profile')->id); + $user->refresh(); + $user->invalidatePermissionCache(); + + $addons = new \ReflectionProperty(ProfileController::class, 'addons'); + $originalAddons = $addons->getValue(); + $captured = null; + + try { + ProfileController::registerAddon([ + 'method' => 'edit.settings', + 'data' => function (array $data) use (&$captured) { + $captured = $data; + + return $data; + }, + ]); + + $response = $this->actingAs($user)->get(route('profile.edit')); + } finally { + $addons->setValue(null, $originalAddons); + } + + $response->assertStatus(200); + $this->assertInstanceOf(User::class, $captured['user'] ?? null); + $this->assertSame($user->id, $captured['user']->id); + $this->assertSame($user->delegation_user_id, $captured['user']->delegation_user_id); + } } diff --git a/tests/Model/UserTest.php b/tests/Model/UserTest.php index d7249f51b6..fe4103a078 100644 --- a/tests/Model/UserTest.php +++ b/tests/Model/UserTest.php @@ -143,4 +143,27 @@ public function testRefreshInvalidatesBothPermissionCacheFamilies() $this->assertNull(Cache::get("user_{$user->id}_permissions")); $this->assertSame('keep-me', Cache::get('unrelated-cache-key')); } + + public function testOnlyAnActiveReplacementCanReceiveADelegatedTask(): void + { + $activeReplacement = User::factory()->create(['status' => 'ACTIVE']); + $owner = User::factory()->create([ + 'status' => 'OUT_OF_OFFICE', + 'delegation_user_id' => $activeReplacement->id, + ]); + + $this->assertSame($activeReplacement->id, $owner->activeDelegationUserId()); + $this->assertSame($activeReplacement->id, $owner->acceptResolvedAssignee($activeReplacement->id)); + + foreach (['INACTIVE', 'OUT_OF_OFFICE', 'BLOCKED', 'SCHEDULED'] as $status) { + $unavailableReplacement = User::factory()->create(['status' => $status]); + $owner->delegation_user_id = $unavailableReplacement->id; + $owner->save(); + + $this->assertNull($owner->activeDelegationUserId()); + $this->assertNull($owner->acceptResolvedAssignee($unavailableReplacement->id)); + } + + $this->assertSame($owner->id, $owner->acceptResolvedAssignee($owner->id)); + } }