diff --git a/ProcessMaker/Http/Controllers/Api/TaskController.php b/ProcessMaker/Http/Controllers/Api/TaskController.php index 2c2647cc40..af2948abf8 100644 --- a/ProcessMaker/Http/Controllers/Api/TaskController.php +++ b/ProcessMaker/Http/Controllers/Api/TaskController.php @@ -134,6 +134,18 @@ public function index(Request $request, $getTotal = false, User $user = null) $query = $this->indexBaseQuery($request); + // Get fields from request (sent by frontend) + // If not provided, don't apply select() to maintain backward compatibility (returns all columns) + $fields = $request->input('fields', ''); + if ($fields) { + $selectedFields = explode(',', $fields); + // Ensure 'id' is always included for internal logic (e.g., inOverdueQuery at line ~186) + if (!in_array('id', $selectedFields)) { + $selectedFields[] = 'id'; + } + $query = $query->select($selectedFields); + } + $this->applyFilters($query, $request); $this->excludeNonVisibleTasks($query, $request); diff --git a/ProcessMaker/Models/ProcessRequestToken.php b/ProcessMaker/Models/ProcessRequestToken.php index a32cf13dab..5b0aabf342 100644 --- a/ProcessMaker/Models/ProcessRequestToken.php +++ b/ProcessMaker/Models/ProcessRequestToken.php @@ -782,8 +782,7 @@ public function valueAliasStatus($value, $expression, $callback = null, User $us $query->whereIn('process_request_tokens.id', $selfServiceTaskIds); } elseif ($user) { - $taskIds = $user->availableSelfServiceTaskIds(); - $query->whereIn('process_request_tokens.id', $taskIds); + $query->whereIn('process_request_tokens.id', $user->availableSelfServiceTasksQuery()); } else { $query->where('process_request_tokens.is_self_service', 1); } diff --git a/ProcessMaker/Models/User.php b/ProcessMaker/Models/User.php index aeec77ed1f..3eb46dfc3a 100644 --- a/ProcessMaker/Models/User.php +++ b/ProcessMaker/Models/User.php @@ -466,11 +466,11 @@ public function removeFromGroups() $this->groups()->detach(); } - public function availableSelfServiceTaskIds() + public function availableSelfServiceTasksQuery() { $groupIds = $this->groups()->pluck('groups.id'); - $taskQuery = ProcessRequestToken::select(['id']) + $taskQuery = ProcessRequestToken::select(['process_request_tokens.id']) ->where([ 'is_self_service' => true, 'status' => 'ACTIVE', @@ -490,7 +490,12 @@ public function availableSelfServiceTaskIds() $query->orWhereJsonContains('self_service_groups->users', (string) $this->id); }); - return $taskQuery->pluck('id'); + return $taskQuery; + } + + public function availableSelfServiceTaskIds() + { + return $this->availableSelfServiceTasksQuery()->pluck('id'); } /** diff --git a/ProcessMaker/Traits/TaskControllerIndexMethods.php b/ProcessMaker/Traits/TaskControllerIndexMethods.php index 1789267a9b..be27fa8804 100644 --- a/ProcessMaker/Traits/TaskControllerIndexMethods.php +++ b/ProcessMaker/Traits/TaskControllerIndexMethods.php @@ -383,7 +383,7 @@ private function applyForCurrentUser($query, $user) $query->where(function ($query) use ($user) { $query->where('user_id', $user->id) - ->orWhereIn('id', $user->availableSelfServiceTaskIds()); + ->orWhereIn('id', $user->availableSelfServiceTasksQuery()); }); } diff --git a/resources/js/requests/components/RequestDetail.vue b/resources/js/requests/components/RequestDetail.vue index 853f777424..b784450e33 100644 --- a/resources/js/requests/components/RequestDetail.vue +++ b/resources/js/requests/components/RequestDetail.vue @@ -184,13 +184,33 @@ export default { this.status }&per_page=${ this.perPage - }${this.getSortParam()}`, + }${this.getSortParam()}${this.getColumnsParam()}`, ) .then((response) => { this.data = this.transform(response.data); this.loading = false; }); }, + /** + * Get the fields parameter for the API request + * @returns {string} The fields parameter for the API request + */ + getColumnsParam() { + const fields = [ + 'id', + 'element_id', // Required by assignableUsers relationship (TokenAssignableUsers::match uses element_id) + 'element_name', + 'user_id', + 'process_id', + 'process_request_id', + 'status', + 'due_at', + 'is_self_service', + 'is_actionbyemail', + 'self_service_groups', // Required by Task resource's addAssignableUsers() method when recalculating assignable users + ]; + return `&fields=${fields.join(',')}`; + }, getSortParam() { if (this.sortOrder instanceof Array && this.sortOrder.length > 0) { return ( diff --git a/tests/Feature/SelfServiceOptimizationTest.php b/tests/Feature/SelfServiceOptimizationTest.php new file mode 100644 index 0000000000..e7d440e258 --- /dev/null +++ b/tests/Feature/SelfServiceOptimizationTest.php @@ -0,0 +1,181 @@ +create(); + $groupA = Group::factory()->create(['name' => 'Group A']); + $groupB = Group::factory()->create(['name' => 'Group B']); + $user->groups()->attach([$groupA->id, $groupB->id]); + + $otherUser = User::factory()->create(); + $otherGroup = Group::factory()->create(['name' => 'Other Group']); + + // 2. CREATE SCENARIOS + + // Scenario 1: New Format - Int ID in groups array + $t1 = $this->createSelfServiceTask(['groups' => [$groupA->id]]); + + // Scenario 2: New Format - String ID in groups array (Legacy/JSON inconsistency) + $t2 = $this->createSelfServiceTask(['groups' => [(string) $groupB->id]]); + + // Scenario 3: Old Format - Direct ID in array (Very old processes) + $t3 = $this->createSelfServiceTask([$groupA->id]); + + // Scenario 4: Direct User Assignment (Int) + $t4 = $this->createSelfServiceTask(['users' => [$user->id]]); + + // Scenario 5: Direct User Assignment (String) + $t5 = $this->createSelfServiceTask(['users' => [(string) $user->id]]); + + // --- NEGATIVE SCENARIOS (Should NEVER be returned) --- + + // Scenario 6: Task for another group + $t6 = $this->createSelfServiceTask(['groups' => [$otherGroup->id]]); + + // Scenario 7: Task for another user + $t7 = $this->createSelfServiceTask(['users' => [$otherUser->id]]); + + // Scenario 8: Task is not ACTIVE + $t8 = $this->createSelfServiceTask(['users' => [$user->id]], 'COMPLETED'); + + // Scenario 9: Task is already assigned to someone + $t9 = $this->createSelfServiceTask(['users' => [$user->id]], 'ACTIVE', $otherUser->id); + + // 3. THE COMPARISON ENGINE + + // Method A: Array Pluck (Memory intensive, prone to crash) + $oldWayIds = $user->availableSelfServiceTaskIds()->sort()->values()->toArray(); + + // Method B: Subquery (Optimized, safe) + $newWayQuery = $user->availableSelfServiceTasksQuery(); + $resultsNewWay = ProcessRequestToken::whereIn('id', $newWayQuery) + ->orderBy('id') + ->pluck('id') + ->toArray(); + + // 4. ASSERTIONS + + // A. Integrity check: Both lists must be identical + $this->assertEquals($oldWayIds, $resultsNewWay, 'FATAL: Subquery results differ from Array results!'); + + // B. Coverage check: Ensure all positive scenarios are present + $expectedIds = [$t1->id, $t2->id, $t3->id, $t4->id, $t5->id]; + sort($expectedIds); + $this->assertEquals($expectedIds, $resultsNewWay, 'Subquery missed one of the valid scenarios.'); + + // C. Exclusion check: Ensure none of the negative scenarios leaked in + $forbiddenIds = [$t6->id, $t7->id, $t8->id, $t9->id]; + foreach ($forbiddenIds as $id) { + $this->assertNotContains($id, $resultsNewWay, "Security breach: Task $id should not be visible."); + } + + // D. Performance Logic check: Subquery must be an instance of Eloquent Builder + $this->assertInstanceOf(\Illuminate\Database\Eloquent\Builder::class, $newWayQuery); + } + + /** + * STRESS TEST: Demonstrates the performance and stability gap. + * This test creates 10,000 tasks to show how the old way struggles vs the new way. + */ + public function test_large_data_performance_and_stability() + { + $user = User::factory()->create(); + $group = Group::factory()->create(); + $user->groups()->attach($group); + + // Crear dependencias reales para evitar errores de Foreign Key + $process = \ProcessMaker\Models\Process::factory()->create(); + $request = \ProcessMaker\Models\ProcessRequest::factory()->create([ + 'process_id' => $process->id, + ]); + + echo "\n--- STRESS TEST (10,000 Self-Service Tasks) ---\n"; + + // 1. Seed 10,000 tasks efficiently using bulk insert + $count = 10000; + $now = now()->toDateTimeString(); + $chunkSize = 1000; + + for ($i = 0; $i < $count / $chunkSize; $i++) { + $tasks = []; + for ($j = 0; $j < $chunkSize; $j++) { + $tasks[] = [ + 'process_id' => $process->id, + 'process_request_id' => $request->id, + 'element_id' => 'node_1', + 'element_type' => 'task', + 'status' => 'ACTIVE', + 'is_self_service' => 1, + 'self_service_groups' => json_encode(['groups' => [$group->id]]), + 'created_at' => $now, + 'updated_at' => $now, + ]; + } + DB::table('process_request_tokens')->insert($tasks); + } + + // 2. Measure OLD WAY (Array of IDs) + $startMemOld = memory_get_usage(); + $startTimeOld = microtime(true); + + $ids = $user->availableSelfServiceTaskIds(); + $resultOld = ProcessRequestToken::whereIn('id', $ids)->count(); + + $timeOld = microtime(true) - $startTimeOld; + $memOld = (memory_get_usage() - $startMemOld) / 1024 / 1024; + + // 3. Measure NEW WAY (Subquery) + $startMemNew = memory_get_usage(); + $startTimeNew = microtime(true); + + $query = $user->availableSelfServiceTasksQuery(); + $resultNew = ProcessRequestToken::whereIn('id', $query)->count(); + + $timeNew = microtime(true) - $startTimeNew; + $memNew = (memory_get_usage() - $startMemNew) / 1024 / 1024; + + // OUTPUT RESULTS + echo 'OLD WAY (Array): Time: ' . number_format($timeOld, 4) . 's | Mem: ' . number_format($memOld, 2) . "MB | Found: $resultOld\n"; + echo 'NEW WAY (Subquery): Time: ' . number_format($timeNew, 4) . 's | Mem: ' . number_format($memNew, 2) . "MB | Found: $resultNew\n"; + + // ASSERTIONS + $this->assertEquals($resultOld, $resultNew, 'Results must be identical!'); + + // En base de datos reales (no en memoria), la subconsulta suele ser más rápida + // Pero lo más importante es que no tiene límites de placeholders + $this->assertTrue($resultNew > 0); + + echo "----------------------------------------------\n"; + if ($timeNew > 0) { + echo 'Optimization: ' . number_format(($timeOld / $timeNew), 1) . "x faster\n"; + } + } + + private function createSelfServiceTask($groups, $status = 'ACTIVE', $userId = null) + { + return ProcessRequestToken::factory()->create([ + 'is_self_service' => true, + 'status' => $status, + 'user_id' => $userId, + 'self_service_groups' => $groups, + ]); + } +} diff --git a/tests/unit/ProcessMaker/FilterTest.php b/tests/unit/ProcessMaker/FilterTest.php index 70e0269fa5..25f2d2ef8f 100644 --- a/tests/unit/ProcessMaker/FilterTest.php +++ b/tests/unit/ProcessMaker/FilterTest.php @@ -227,10 +227,10 @@ public function testTaskStatusSelfservice() ], ], ProcessRequestToken::class); - $this->assertEquals( - "select * from `process_request_tokens` where ((`process_request_tokens`.`id` in ({$selfServiceTask->id})))", - $sql - ); + $this->assertStringContainsString('select * from `process_request_tokens` where ((`process_request_tokens`.`id` in (select `process_request_tokens`.`id` from `process_request_tokens` where', $sql); + $this->assertStringContainsString('`is_self_service` = 1', $sql); + $this->assertStringContainsString("`status` = 'ACTIVE'", $sql); + $this->assertStringContainsString('json_contains(`self_service_groups`', $sql); } public function testTaskStatusActive() diff --git a/tests/unit/ProcessMaker/Models/SelfServiceComparisonTest.php b/tests/unit/ProcessMaker/Models/SelfServiceComparisonTest.php new file mode 100644 index 0000000000..6666b7e6b4 --- /dev/null +++ b/tests/unit/ProcessMaker/Models/SelfServiceComparisonTest.php @@ -0,0 +1,94 @@ +create(); + $group = Group::factory()->create(); + $user->groups()->attach($group); + + // Task 1: Available by GROUP (Should appear) + $task1 = ProcessRequestToken::factory()->create([ + 'is_self_service' => true, + 'status' => 'ACTIVE', + 'user_id' => null, + 'self_service_groups' => ['groups' => [$group->id]], + ]); + + // Task 2: Available by direct USER (Should appear) + $task2 = ProcessRequestToken::factory()->create([ + 'is_self_service' => true, + 'status' => 'ACTIVE', + 'user_id' => null, + 'self_service_groups' => ['users' => [$user->id]], + ]); + + // Task 3: NOT available (Another group) + ProcessRequestToken::factory()->create([ + 'is_self_service' => true, + 'status' => 'ACTIVE', + 'user_id' => null, + 'self_service_groups' => ['groups' => [9999]], + ]); + + // Task 4: NOT available (Already completed) + ProcessRequestToken::factory()->create([ + 'is_self_service' => true, + 'status' => 'COMPLETED', + 'user_id' => null, + 'self_service_groups' => ['groups' => [$group->id]], + ]); + + // 2. Execution of both methods + + // Method A: Using the IDs array (Preserved behavior) + $idsArray = $user->availableSelfServiceTaskIds(); + $resultsFromArray = ProcessRequestToken::whereIn('id', $idsArray) + ->pluck('id') + ->sort() + ->values() + ->toArray(); + + // Method B: Using the subquery (New optimization) + $subqueryBuilder = $user->availableSelfServiceTasksQuery(); + $resultsFromSubquery = ProcessRequestToken::whereIn('id', $subqueryBuilder) + ->pluck('id') + ->sort() + ->values() + ->toArray(); + + // 3. Verification + + // Check that tasks were found + $this->assertCount(2, $resultsFromArray, 'Exactly 2 tasks should have been found with the old method.'); + + // Check that both methods return exactly the same results + $this->assertEquals( + $resultsFromArray, + $resultsFromSubquery, + 'Task IDs found by both methods MUST be identical.' + ); + + // Verify specific IDs are correct + $this->assertContains($task1->id, $resultsFromSubquery); + $this->assertContains($task2->id, $resultsFromSubquery); + } +}