fix(transactions): prevent crash when sorting by nullable column (#501)
## Problem `GET /transactions` throws `InvalidArgumentException: Illegal operator and value combination` (Sentry [PHP-LARAVEL-34](https://whisper-money.sentry.io/issues/PHP-LARAVEL-34) — escalating, 76 occurrences, 2 users). ## Root cause Laravel's `cursorPaginate()` builds `where(orderColumn, '>', cursorValue)` for the next page. When sorting by a **nullable** column (`creditor_name` / `debtor_name`) and the boundary row's value is `null`, the cursor encodes `null` → `prepareValueAndOperator` rejects `where(col, '>', null)`. Default sort (`transaction_date`, NOT NULL) never hits it. ## Fix When sorting by a nullable column, select `COALESCE(col, '') as col_sort` and order by the alias. Cursor reads the coalesced (never-null) value; Laravel rewrites the WHERE to the `COALESCE` expression. Alias is hidden from serialization. ## Tests - Reproduces cross-page cursor pagination with null values in the sort column (500 → 200). - Asserts the `*_sort` alias is not leaked to the frontend. Fixes PHP-LARAVEL-34
This commit is contained in:
parent
a7dde5fbc5
commit
a69c602366
|
|
@ -48,15 +48,29 @@ class TransactionController extends Controller
|
|||
'search' => $validated['search'] ?? null,
|
||||
], fn ($value) => $value !== null);
|
||||
|
||||
$transactions = Transaction::query()
|
||||
$query = Transaction::query()
|
||||
->where('user_id', $user->id)
|
||||
->with(['account.bank', 'category', 'labels'])
|
||||
->applyFilters($filters)
|
||||
->orderBy($sortColumn, $sortDirection)
|
||||
->applyFilters($filters);
|
||||
|
||||
$nullableSortColumns = ['creditor_name', 'debtor_name'];
|
||||
|
||||
if (in_array($sortColumn, $nullableSortColumns, true)) {
|
||||
$sortAlias = $sortColumn.'_sort';
|
||||
$query->select('transactions.*')
|
||||
->selectRaw("COALESCE({$sortColumn}, '') as {$sortAlias}")
|
||||
->orderBy($sortAlias, $sortDirection);
|
||||
} else {
|
||||
$query->orderBy($sortColumn, $sortDirection);
|
||||
}
|
||||
|
||||
$transactions = $query
|
||||
->orderBy('id', 'desc')
|
||||
->cursorPaginate($perPage)
|
||||
->withQueryString();
|
||||
|
||||
$transactions->getCollection()->each(fn (Transaction $transaction) => $transaction->makeHidden(['creditor_name_sort', 'debtor_name_sort']));
|
||||
|
||||
$appliedFilters = [
|
||||
'date_from' => $validated['date_from'] ?? null,
|
||||
'date_to' => $validated['date_to'] ?? null,
|
||||
|
|
|
|||
|
|
@ -614,3 +614,47 @@ test('filter by multiple categories including uncategorized', function () {
|
|||
->has('transactions.data', 2)
|
||||
);
|
||||
});
|
||||
|
||||
test('paginates across pages when sorting by a nullable column with null values', function () {
|
||||
Transaction::factory()->plaintext()->count(12)->create([
|
||||
'user_id' => $this->user->id,
|
||||
'account_id' => $this->account->id,
|
||||
'creditor_name' => null,
|
||||
]);
|
||||
|
||||
Transaction::factory()->plaintext()->count(5)->create([
|
||||
'user_id' => $this->user->id,
|
||||
'account_id' => $this->account->id,
|
||||
'creditor_name' => 'ACME',
|
||||
]);
|
||||
|
||||
$firstPage = actingAs($this->user)->get(route('transactions.index', [
|
||||
'sort' => 'creditor_name',
|
||||
'per_page' => 10,
|
||||
]));
|
||||
|
||||
$firstPage->assertSuccessful();
|
||||
|
||||
$nextPageUrl = $firstPage->getOriginalContent()->getData()['page']['props']['transactions']['next_page_url'];
|
||||
|
||||
expect($nextPageUrl)->not->toBeNull();
|
||||
|
||||
actingAs($this->user)->get($nextPageUrl)->assertSuccessful();
|
||||
});
|
||||
|
||||
test('does not expose the sort alias attribute when sorting by a nullable column', function () {
|
||||
Transaction::factory()->plaintext()->create([
|
||||
'user_id' => $this->user->id,
|
||||
'account_id' => $this->account->id,
|
||||
'creditor_name' => null,
|
||||
]);
|
||||
|
||||
$response = actingAs($this->user)->get(route('transactions.index', [
|
||||
'sort' => 'creditor_name',
|
||||
]));
|
||||
|
||||
$response->assertInertia(fn ($page) => $page
|
||||
->has('transactions.data', 1)
|
||||
->missing('transactions.data.0.creditor_name_sort')
|
||||
);
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in New Issue