Skip to content

Commit 3159b63

Browse files
nolanpronolanpro
authored
Merge pull request #9066 from ProcessMaker/FOUR-33374
FOUR-33374 Fix issue qualifing columns in queries
2 parents 41da510 + 4486e09 commit 3159b63

2 files changed

Lines changed: 237 additions & 62 deletions

File tree

‎ProcessMaker/Traits/TaskControllerIndexMethods.php‎

Lines changed: 44 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
use Illuminate\Database\QueryException;
77
use Illuminate\Support\Arr;
88
use Illuminate\Support\Facades\Cache;
9+
use Illuminate\Support\Facades\DB;
910
use ProcessMaker\Filters\Filter;
1011
use ProcessMaker\Managers\DataManager;
1112
use ProcessMaker\Models\Process;
@@ -69,7 +70,7 @@ private function applyFilters($query, $request)
6970
}
7071

7172
$filterByFields = [
72-
'process_id',
73+
'process_request_tokens.process_id' => 'process_id',
7374
'process_request_tokens.user_id' => 'user_id',
7475
'process_request_tokens.status' => 'status',
7576
'element_id',
@@ -247,60 +248,6 @@ private function applyColumnOrdering($query, $request)
247248
'is_priority',
248249
];
249250

250-
$hasProcessRequestOrdering = false;
251-
$hasUserOrdering = false;
252-
foreach ($orderColumns as $column) {
253-
$normalizedColumn = preg_replace(
254-
'/^(process_request|processRequests)\./',
255-
'process_requests.',
256-
$column
257-
);
258-
259-
if (in_array($normalizedColumn, array_map(
260-
fn ($name) => 'process_requests.' . $name,
261-
$processRequestColumns
262-
), true) || preg_match('/^data\.[A-Za-z0-9_-]+(?:\.[A-Za-z0-9_-]+)*$/', $column)) {
263-
$hasProcessRequestOrdering = true;
264-
}
265-
if ($column === 'user.name') {
266-
$hasUserOrdering = true;
267-
}
268-
}
269-
270-
if ($hasProcessRequestOrdering) {
271-
$query->leftJoin(
272-
'process_requests',
273-
'process_requests.id',
274-
'=',
275-
'process_request_tokens.process_request_id'
276-
);
277-
}
278-
if ($hasUserOrdering) {
279-
$query->leftJoin(
280-
'users',
281-
'users.id',
282-
'=',
283-
'process_request_tokens.user_id'
284-
);
285-
}
286-
if ($hasProcessRequestOrdering || $hasUserOrdering) {
287-
if ($query->getQuery()->columns === null) {
288-
$query->select('process_request_tokens.*');
289-
} else {
290-
$query->select(array_map(function ($column) {
291-
if (!is_string($column)) {
292-
return $column;
293-
}
294-
295-
$column = ltrim($column, '.');
296-
297-
return str_contains($column, '.')
298-
? $column
299-
: 'process_request_tokens.' . $column;
300-
}, $query->getQuery()->columns));
301-
}
302-
}
303-
304251
$hasValidOrdering = false;
305252
foreach ($orderColumns as $index => $column) {
306253
$direction = strtolower($orderDirections[$index] ?? $orderDirections[0] ?? 'asc');
@@ -312,17 +259,44 @@ private function applyColumnOrdering($query, $request)
312259
);
313260

314261
if ($column === 'user.name') {
315-
$query->orderBy('users.firstname', $direction)
316-
->orderBy('users.lastname', $direction);
262+
$query->orderBy(
263+
$this->relatedOrderSubquery('users', 'firstname', 'users.id', 'process_request_tokens.user_id'),
264+
$direction
265+
)->orderBy(
266+
$this->relatedOrderSubquery('users', 'lastname', 'users.id', 'process_request_tokens.user_id'),
267+
$direction
268+
);
317269
$hasValidOrdering = true;
318270
} elseif (preg_match('/^data\.([A-Za-z0-9_-]+(?:\.[A-Za-z0-9_-]+)*)$/', $column, $matches)) {
319-
$query->orderBy('process_requests.data->' . str_replace('.', '->', $matches[1]), $direction);
271+
$jsonColumn = 'data->' . str_replace('.', '->', $matches[1]);
272+
$query->orderBy(
273+
$this->relatedOrderSubquery(
274+
'process_requests',
275+
$jsonColumn,
276+
'process_requests.id',
277+
'process_request_tokens.process_request_id'
278+
),
279+
$direction
280+
);
320281
$hasValidOrdering = true;
321282
} elseif (in_array($normalizedColumn, array_map(
322283
fn ($name) => 'process_requests.' . $name,
323284
$processRequestColumns
324285
), true)) {
325-
$query->orderBy($normalizedColumn, $direction);
286+
$columnName = substr($normalizedColumn, strlen('process_requests.'));
287+
if ($columnName === 'id') {
288+
$query->orderBy('process_request_tokens.process_request_id', $direction);
289+
} else {
290+
$query->orderBy(
291+
$this->relatedOrderSubquery(
292+
'process_requests',
293+
$columnName,
294+
'process_requests.id',
295+
'process_request_tokens.process_request_id'
296+
),
297+
$direction
298+
);
299+
}
326300
$hasValidOrdering = true;
327301
} elseif (in_array($column, $tokenColumns, true)) {
328302
$query->orderBy('process_request_tokens.' . $column, $direction);
@@ -335,14 +309,22 @@ private function applyColumnOrdering($query, $request)
335309
}
336310
}
337311

312+
private function relatedOrderSubquery(string $table, string $column, string $localKey, string $foreignKey)
313+
{
314+
return DB::table($table)
315+
->select($column)
316+
->whereColumn($localKey, $foreignKey)
317+
->limit(1);
318+
}
319+
338320
private function applyStatusFilter($query, $request)
339321
{
340322
$statusFilter = $request->input('statusfilter', '');
341323
if ($statusFilter) {
342324
$statusFilter = array_map(function ($value) {
343325
return mb_strtoupper(trim($value));
344326
}, explode(',', $statusFilter));
345-
$query->whereIn('status', $statusFilter);
327+
$query->whereIn('process_request_tokens.status', $statusFilter);
346328
}
347329
}
348330

@@ -544,8 +526,8 @@ private function applyForCurrentUser($query, $user)
544526
}
545527

546528
$query->where(function ($query) use ($user) {
547-
$query->where('user_id', $user->id)
548-
->orWhereIn('id', $user->availableSelfServiceTasksQuery());
529+
$query->where('process_request_tokens.user_id', $user->id)
530+
->orWhereIn('process_request_tokens.id', $user->availableSelfServiceTasksQuery());
549531
});
550532
}
551533

‎tests/Feature/Api/TaskListNonAdminCaseTitleOrderTest.php‎

Lines changed: 193 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,193 @@
1+
<?php
2+
3+
namespace Tests\Feature\Api;
4+
5+
use PHPUnit\Framework\Attributes\Group as TestGroup;
6+
use ProcessMaker\Models\ProcessRequest;
7+
use ProcessMaker\Models\ProcessRequestToken;
8+
use ProcessMaker\Models\User;
9+
use Tests\Feature\Shared\RequestHelper;
10+
use Tests\TestCase;
11+
12+
/**
13+
* Regression for tasks index 1052 (ambiguous user_id) when a non-admin
14+
* lists tasks ordered by process_requests.case_title.
15+
*/
16+
#[TestGroup('process_tests')]
17+
class TaskListNonAdminCaseTitleOrderTest extends TestCase
18+
{
19+
use RequestHelper;
20+
21+
public function testNonAdminCanListTasksOrderedByCaseTitleDesc()
22+
{
23+
$this->user = User::factory()->create([
24+
'is_administrator' => false,
25+
'status' => 'ACTIVE',
26+
]);
27+
$otherUser = User::factory()->create([
28+
'is_administrator' => false,
29+
'status' => 'ACTIVE',
30+
]);
31+
32+
$zetaRequest = ProcessRequest::factory()->create();
33+
$alphaRequest = ProcessRequest::factory()->create();
34+
$otherRequest = ProcessRequest::factory()->create();
35+
$zetaRequest->forceFill(['case_title' => 'Zeta mandate'])->saveQuietly();
36+
$alphaRequest->forceFill(['case_title' => 'Alpha mandate'])->saveQuietly();
37+
$otherRequest->forceFill(['case_title' => 'Omega mandate'])->saveQuietly();
38+
39+
$zetaTask = ProcessRequestToken::factory()->create([
40+
'user_id' => $this->user->id,
41+
'process_id' => $zetaRequest->process_id,
42+
'process_request_id' => $zetaRequest->id,
43+
'element_type' => 'task',
44+
'status' => 'ACTIVE',
45+
'is_self_service' => false,
46+
'is_priority' => true,
47+
'completed_at' => null,
48+
]);
49+
$alphaTask = ProcessRequestToken::factory()->create([
50+
'user_id' => $this->user->id,
51+
'process_id' => $alphaRequest->process_id,
52+
'process_request_id' => $alphaRequest->id,
53+
'element_type' => 'task',
54+
'status' => 'ACTIVE',
55+
'is_self_service' => false,
56+
'is_priority' => true,
57+
'completed_at' => null,
58+
]);
59+
ProcessRequestToken::factory()->create([
60+
'user_id' => $otherUser->id,
61+
'process_id' => $otherRequest->process_id,
62+
'process_request_id' => $otherRequest->id,
63+
'element_type' => 'task',
64+
'status' => 'ACTIVE',
65+
'is_self_service' => false,
66+
'is_priority' => true,
67+
'completed_at' => null,
68+
]);
69+
70+
$response = $this->apiCall('GET', '/tasks', [
71+
'page' => 1,
72+
'include' => 'process,processRequest,processRequest.user,user',
73+
'pmql' => '(user_id = ' . $this->user->id . ')',
74+
'per_page' => 15,
75+
'order_by' => 'process_requests.case_title',
76+
'order_direction' => 'desc',
77+
'non_system' => true,
78+
'processesIManage' => 'false',
79+
'advanced_filter' => json_encode([
80+
[
81+
'subject' => ['type' => 'Status'],
82+
'operator' => '=',
83+
'value' => 'In Progress',
84+
],
85+
[
86+
'subject' => ['type' => 'Field', 'value' => 'is_priority'],
87+
'operator' => '=',
88+
'value' => true,
89+
],
90+
]),
91+
]);
92+
93+
$response->assertStatus(200);
94+
95+
$rows = $response->json('data');
96+
$this->assertCount(2, $rows);
97+
$this->assertSame(
98+
[$zetaTask->id, $alphaTask->id],
99+
array_column($rows, 'id')
100+
);
101+
$this->assertSame(
102+
['Zeta mandate', 'Alpha mandate'],
103+
array_map(fn ($row) => $row['process_request']['case_title'] ?? null, $rows)
104+
);
105+
}
106+
107+
public function testNonAdminCanListTasksOrderedByCaseTitleDescAndFulltextSearch()
108+
{
109+
$this->user = User::factory()->create([
110+
'is_administrator' => false,
111+
'status' => 'ACTIVE',
112+
]);
113+
$otherUser = User::factory()->create([
114+
'is_administrator' => false,
115+
'status' => 'ACTIVE',
116+
]);
117+
118+
$zetaRequest = ProcessRequest::factory()->create();
119+
$alphaRequest = ProcessRequest::factory()->create();
120+
$otherRequest = ProcessRequest::factory()->create();
121+
$zetaRequest->forceFill(['case_title' => 'Zeta mandate'])->saveQuietly();
122+
$alphaRequest->forceFill(['case_title' => 'Alpha mandate'])->saveQuietly();
123+
$otherRequest->forceFill(['case_title' => 'Omega mandate'])->saveQuietly();
124+
125+
$zetaTask = ProcessRequestToken::factory()->create([
126+
'user_id' => $this->user->id,
127+
'process_id' => $zetaRequest->process_id,
128+
'process_request_id' => $zetaRequest->id,
129+
'element_name' => 'Review of the client name',
130+
'element_type' => 'task',
131+
'status' => 'ACTIVE',
132+
'is_self_service' => false,
133+
'is_priority' => true,
134+
'completed_at' => null,
135+
]);
136+
$alphaTask = ProcessRequestToken::factory()->create([
137+
'user_id' => $this->user->id,
138+
'process_id' => $alphaRequest->process_id,
139+
'process_request_id' => $alphaRequest->id,
140+
'element_type' => 'task',
141+
'status' => 'ACTIVE',
142+
'is_self_service' => false,
143+
'is_priority' => true,
144+
'completed_at' => null,
145+
]);
146+
ProcessRequestToken::factory()->create([
147+
'user_id' => $otherUser->id,
148+
'process_id' => $otherRequest->process_id,
149+
'process_request_id' => $otherRequest->id,
150+
'element_type' => 'task',
151+
'status' => 'ACTIVE',
152+
'is_self_service' => false,
153+
'is_priority' => true,
154+
'completed_at' => null,
155+
]);
156+
157+
$response = $this->apiCall('GET', '/tasks', [
158+
'page' => 1,
159+
'include' => 'process,processRequest,processRequest.user,user',
160+
'pmql' => '(user_id = ' . $this->user->id . ') AND (fulltext LIKE "%Review of the client name%") ',
161+
'per_page' => 15,
162+
'order_by' => 'process_requests.case_title',
163+
'order_direction' => 'desc',
164+
'non_system' => true,
165+
'processesIManage' => 'false',
166+
'advanced_filter' => json_encode([
167+
[
168+
'subject' => ['type' => 'Status'],
169+
'operator' => '=',
170+
'value' => 'In Progress',
171+
],
172+
[
173+
'subject' => ['type' => 'Field', 'value' => 'is_priority'],
174+
'operator' => '=',
175+
'value' => true,
176+
],
177+
]),
178+
]);
179+
180+
$response->assertStatus(200);
181+
182+
$rows = $response->json('data');
183+
$this->assertCount(1, $rows);
184+
$this->assertSame(
185+
[$zetaTask->id],
186+
array_column($rows, 'id')
187+
);
188+
$this->assertSame(
189+
['Zeta mandate'],
190+
array_map(fn ($row) => $row['process_request']['case_title'] ?? null, $rows)
191+
);
192+
}
193+
}

0 commit comments

Comments
 (0)