From 425484812fc3cc8460b1892a774cfcd43fb73b27 Mon Sep 17 00:00:00 2001 From: Rodrigo Date: Mon, 31 Aug 2026 14:03:00 -0400 Subject: [PATCH] FOUR-31145: Self Service tasks not visible to users in assigned subgroups --- ProcessMaker/Models/Group.php | 61 +++++++ ProcessMaker/Models/ProcessMakerModel.php | 5 +- ProcessMaker/Models/User.php | 44 ++++- .../Observers/GroupMemberObserver.php | 22 +++ tests/Feature/SelfServiceSubgroupTest.php | 162 ++++++++++++++++++ 5 files changed, 285 insertions(+), 9 deletions(-) create mode 100644 tests/Feature/SelfServiceSubgroupTest.php diff --git a/ProcessMaker/Models/Group.php b/ProcessMaker/Models/Group.php index 6d7948debe..f30797b719 100644 --- a/ProcessMaker/Models/Group.php +++ b/ProcessMaker/Models/Group.php @@ -2,6 +2,8 @@ namespace ProcessMaker\Models; +use Illuminate\Support\Collection; +use Illuminate\Support\Facades\Cache; use Illuminate\Validation\Rule; use ProcessMaker\Models\EmptyModel; use ProcessMaker\Query\Traits\PMQL; @@ -156,4 +158,63 @@ public function assigned() { return $this->morphMany(ProcessTaskAssignment::class, 'assigned', 'assignment_type', 'assignment_id'); } + + /** + * Parent group IDs for the given groups (one walk up, not one query per group). + */ + public static function ancestorIdsFor(iterable $groupIds): Collection + { + $ids = collect($groupIds)->filter()->map(fn ($id) => (int) $id)->unique()->values(); + if ($ids->isEmpty()) { + return collect(); + } + + return collect(static::computeAncestorIds($ids->all())); + } + + public const SELF_SERVICE_HIERARCHY_VERSION_KEY = 'self_service:hierarchy_version'; + + public static function selfServiceHierarchyVersion(): int + { + return (int) Cache::get(self::SELF_SERVICE_HIERARCHY_VERSION_KEY, 1); + } + + public static function bumpSelfServiceHierarchyVersion(): void + { + if (!Cache::has(self::SELF_SERVICE_HIERARCHY_VERSION_KEY)) { + Cache::forever(self::SELF_SERVICE_HIERARCHY_VERSION_KEY, 1); + } + Cache::increment(self::SELF_SERVICE_HIERARCHY_VERSION_KEY); + } + + /** + * @return array + */ + private static function computeAncestorIds(array $groupIds): array + { + $ancestors = []; + $queue = $groupIds; + $visited = array_fill_keys($groupIds, true); + + while ($queue) { + $parents = GroupMember::query() + ->where('member_type', self::class) + ->whereIn('member_id', $queue) + ->pluck('group_id') + ->all(); + + $queue = []; + foreach ($parents as $parentId) { + $parentId = (int) $parentId; + if (isset($visited[$parentId])) { + continue; + } + $visited[$parentId] = true; + $ancestors[] = $parentId; + $queue[] = $parentId; + } + } + + return $ancestors; + } } diff --git a/ProcessMaker/Models/ProcessMakerModel.php b/ProcessMaker/Models/ProcessMakerModel.php index 1fb77d2728..38e4bee6f0 100644 --- a/ProcessMaker/Models/ProcessMakerModel.php +++ b/ProcessMaker/Models/ProcessMakerModel.php @@ -35,8 +35,9 @@ public function scopeExclude($query, array $columns) } $columnsToShow = array_diff($this->getTableColumns(), $columns); - $columnsToShow = array_map(function ($column) { - return $this->table . '.' . $column; + $table = $this->getTable(); + $columnsToShow = array_map(function ($column) use ($table) { + return $table . '.' . $column; }, $columnsToShow); return $query->select($columnsToShow); diff --git a/ProcessMaker/Models/User.php b/ProcessMaker/Models/User.php index c13c133659..b10522badc 100644 --- a/ProcessMaker/Models/User.php +++ b/ProcessMaker/Models/User.php @@ -48,6 +48,10 @@ class User extends Authenticatable implements HasMedia // Session key to save request ids that the user started public const REQUESTS_SESSION_KEY = 'web-entry-request-ids'; + public const SELF_SERVICE_GROUP_IDS_CACHE_PREFIX = 'self_service:user_group_ids:'; + + public const SELF_SERVICE_GROUP_IDS_CACHE_TTL_MINUTES = 10; + /** * The attributes that are mass assignable. * @@ -449,15 +453,13 @@ public function canSelfServe(ProcessRequestToken $task) return true; } elseif (array_key_exists('groups', $task->self_service_groups)) { return collect($task->self_service_groups['groups']) - ->intersect( - $this->groups()->pluck('groups.id') - )->count() > 0; + ->intersect($this->selfServiceGroupIds()) + ->count() > 0; } else { // For older processes return collect($task->self_service_groups) - ->intersect( - $this->groups()->pluck('groups.id') - )->count() > 0; + ->intersect($this->selfServiceGroupIds()) + ->count() > 0; } } @@ -466,9 +468,37 @@ public function removeFromGroups() $this->groups()->detach(); } + /** + * Direct group IDs plus ancestor groups. Cached per user; skipped on inbox repeats. + */ + public function selfServiceGroupIds() + { + $key = $this->selfServiceGroupIdsCacheKey(); + + return collect(Cache::remember($key, now()->addMinutes(self::SELF_SERVICE_GROUP_IDS_CACHE_TTL_MINUTES), function () { + $direct = $this->groups()->pluck('groups.id'); + if ($direct->isEmpty()) { + return []; + } + + return $direct->merge(Group::ancestorIdsFor($direct))->unique()->values()->all(); + })); + } + + public static function flushSelfServiceGroupIdsCache(int $userId): void + { + $version = Group::selfServiceHierarchyVersion(); + Cache::forget(self::SELF_SERVICE_GROUP_IDS_CACHE_PREFIX . $userId . ':' . $version); + } + + private function selfServiceGroupIdsCacheKey(): string + { + return self::SELF_SERVICE_GROUP_IDS_CACHE_PREFIX . $this->id . ':' . Group::selfServiceHierarchyVersion(); + } + public function availableSelfServiceTasksQuery() { - $groupIds = $this->groups()->pluck('groups.id'); + $groupIds = $this->selfServiceGroupIds(); $taskQuery = ProcessRequestToken::select(['process_request_tokens.id']) ->where([ diff --git a/ProcessMaker/Observers/GroupMemberObserver.php b/ProcessMaker/Observers/GroupMemberObserver.php index 05e763f458..405b1ce759 100644 --- a/ProcessMaker/Observers/GroupMemberObserver.php +++ b/ProcessMaker/Observers/GroupMemberObserver.php @@ -6,6 +6,7 @@ use ProcessMaker\Events\GroupMembershipChanged; use ProcessMaker\Models\Group; use ProcessMaker\Models\GroupMember; +use ProcessMaker\Models\User; class GroupMemberObserver { @@ -14,6 +15,8 @@ class GroupMemberObserver */ public function created(GroupMember $groupMember): void { + $this->invalidateSelfServiceGroupCache($groupMember); + // Only handle group-to-group relationships, not user-to-group if ($groupMember->member_type === Group::class) { $group = Group::find($groupMember->member_id); @@ -32,6 +35,8 @@ public function created(GroupMember $groupMember): void */ public function updated(GroupMember $groupMember): void { + $this->invalidateSelfServiceGroupCache($groupMember); + // Only handle group-to-group relationships, not user-to-group if ($groupMember->member_type === Group::class) { $group = Group::find($groupMember->member_id); @@ -50,6 +55,8 @@ public function updated(GroupMember $groupMember): void */ public function deleted(GroupMember $groupMember): void { + $this->invalidateSelfServiceGroupCache($groupMember); + // Only handle group-to-group relationships, not user-to-group if ($groupMember->member_type === Group::class) { $group = Group::find($groupMember->member_id); @@ -68,6 +75,8 @@ public function deleted(GroupMember $groupMember): void */ public function restored(GroupMember $groupMember): void { + $this->invalidateSelfServiceGroupCache($groupMember); + // Only handle group-to-group relationships, not user-to-group if ($groupMember->member_type === Group::class) { $group = Group::find($groupMember->member_id); @@ -80,4 +89,17 @@ public function restored(GroupMember $groupMember): void } } } + + private function invalidateSelfServiceGroupCache(GroupMember $groupMember): void + { + if ($groupMember->member_type === Group::class) { + Group::bumpSelfServiceHierarchyVersion(); + + return; + } + + if ($groupMember->member_type === User::class) { + User::flushSelfServiceGroupIdsCache((int) $groupMember->member_id); + } + } } diff --git a/tests/Feature/SelfServiceSubgroupTest.php b/tests/Feature/SelfServiceSubgroupTest.php new file mode 100644 index 0000000000..2ab055332a --- /dev/null +++ b/tests/Feature/SelfServiceSubgroupTest.php @@ -0,0 +1,162 @@ +createNestedGroups(); + + $task = ProcessRequestToken::factory()->create([ + 'is_self_service' => true, + 'status' => 'ACTIVE', + 'user_id' => null, + 'self_service_groups' => ['groups' => [$parent->id]], + ]); + + $this->assertContains($task->id, $nestedUser->availableSelfServiceTaskIds()); + $this->assertContains($task->id, $directUser->availableSelfServiceTaskIds()); + $this->assertNotContains($task->id, $outsider->availableSelfServiceTaskIds()); + + Auth::login($nestedUser); + $this->assertTrue($nestedUser->canSelfServe($task)); + + Auth::login($directUser); + $this->assertTrue($directUser->canSelfServe($task)); + + Auth::login($outsider); + $this->assertFalse($outsider->canSelfServe($task)); + + $this->assertContains($parent->id, $nestedUser->selfServiceGroupIds()); + $this->assertNotContains($subgroup->id, $outsider->selfServiceGroupIds()); + } + + public function test_two_level_nested_subgroup_member_sees_self_service_task(): void + { + $parent = Group::factory()->create(); + $child = Group::factory()->create(); + $grandchild = Group::factory()->create(); + $user = User::factory()->create(); + + GroupMember::factory()->create([ + 'group_id' => $parent->id, + 'member_id' => $child->id, + 'member_type' => Group::class, + ]); + GroupMember::factory()->create([ + 'group_id' => $child->id, + 'member_id' => $grandchild->id, + 'member_type' => Group::class, + ]); + $user->groups()->attach($grandchild); + + $task = ProcessRequestToken::factory()->create([ + 'is_self_service' => true, + 'status' => 'ACTIVE', + 'user_id' => null, + 'self_service_groups' => ['groups' => [(string) $parent->id]], + ]); + + $this->assertContains($task->id, $user->availableSelfServiceTaskIds()); + Auth::login($user); + $this->assertTrue($user->canSelfServe($task)); + } + + public function test_removing_subgroup_from_parent_drops_cached_self_service_visibility(): void + { + [$parent, $subgroup, $nestedUser] = $this->createNestedGroups(); + + $task = ProcessRequestToken::factory()->create([ + 'is_self_service' => true, + 'status' => 'ACTIVE', + 'user_id' => null, + 'self_service_groups' => ['groups' => [$parent->id]], + ]); + + $this->assertContains($task->id, $nestedUser->availableSelfServiceTaskIds()); + + GroupMember::where('group_id', $parent->id) + ->where('member_id', $subgroup->id) + ->where('member_type', Group::class) + ->first() + ->delete(); + + $this->assertNotContains($task->id, $nestedUser->fresh()->availableSelfServiceTaskIds()); + } + + public function test_exclude_qualifies_columns_with_the_model_table(): void + { + $task = ProcessRequestToken::factory()->create([ + 'element_type' => 'task', + ]); + + $found = ProcessRequestToken::exclude(['data'])->find($task->id); + + $this->assertNotNull($found); + $this->assertSame($task->id, $found->id); + $this->assertStringContainsString( + '`process_request_tokens`.`id`', + ProcessRequestToken::exclude(['data'])->toSql() + ); + } + + public function test_subgroup_member_sees_parent_self_service_task_on_tasks_index(): void + { + [$parent, , $nestedUser] = $this->createNestedGroups(); + + $task = ProcessRequestToken::factory()->create([ + 'is_self_service' => true, + 'status' => 'ACTIVE', + 'user_id' => null, + 'element_type' => 'task', + 'self_service_groups' => ['groups' => [(string) $parent->id], 'users' => []], + ]); + + $response = $this->actingAs($nestedUser, 'api')->getJson(route('api.tasks.index', [ + 'pmql' => '(status = "Self Service")', + 'per_page' => 15, + 'order_by' => 'ID', + 'order_direction' => 'DESC', + 'non_system' => true, + 'processesIManage' => false, + ])); + + $response->assertOk(); + $this->assertContains($task->id, collect($response->json('data'))->pluck('id')); + } + + /** + * @return array{0: Group, 1: Group, 2: User, 3: User, 4: User} + */ + private function createNestedGroups(): array + { + $parent = Group::factory()->create(['name' => 'Main']); + $subgroup = Group::factory()->create(['name' => 'Sub']); + $nestedUser = User::factory()->create(); + $directUser = User::factory()->create(); + $outsider = User::factory()->create(); + + GroupMember::factory()->create([ + 'group_id' => $parent->id, + 'member_id' => $subgroup->id, + 'member_type' => Group::class, + ]); + $nestedUser->groups()->attach($subgroup); + $directUser->groups()->attach($parent); + + return [$parent, $subgroup, $nestedUser, $directUser, $outsider]; + } +}