From 9326d8fd2fd3c9739cffa7d0f2f384fa49a623c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Falc=C3=B3n?= Date: Fri, 3 Jul 2026 17:40:08 +0200 Subject: [PATCH] perf(accounts): defer the account ledger prop on show (#632) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem `AccountController@show` (added in #631) loaded the **full transaction ledger** for an account into the initial Inertia payload on every visit — `transactions()->with(['category','labels'])->get()`. For accounts with years of movements that's several MB, all on the critical render path, blocking first paint. The `ponytail:` comment flagged the deferred/cursor move as a follow-up; this PR does it. ## Fix Wrap the `transactions` prop in `Inertia::defer(...)` so the page shell (header, chart, actions) paints immediately and the ledger arrives in a follow-up request behind a skeleton. Mirrors how `index()` already defers `accountMetrics` and how `dashboard.tsx` consumes ``. The ledger **stays the whole set on purpose** — search and filtering run client-side over *decrypted* rows (privacy-first design), so the client needs every row. Cursor/offset pagination isn't viable without breaking search over encrypted fields, so deferred is the right lever: it moves the cost off the critical path rather than reducing bytes. Non-transactional account types (`loan`/`investment`/`retirement`/`real_estate`, i.e. `!hasTransactionLedger()`) return a plain `[]` and never render ``, so they take no extra round-trip. ## Testing - `AccountControllerTest`: the "includes transactions" case migrated from `->has('transactions', 2)` to `->missing('transactions')->loadDeferredProps(...)`, proving the prop is deferred yet resolves. - `Show.test.tsx`: added coverage that creating a transaction reloads **only** the deferred prop (`router.reload({ only: ['transactions'] })`) — this refresh became load-bearing on deferred-prop semantics once the remount `key` was dropped in #631. - `php artisan test` (AccountController + PageQueryCount): 45 passed. Vitest Show: 3 passed. Pint + ESLint clean. ## Reviewer notes - **Known minor (not fixed):** brief loading-skeleton shape change on first load — the `` fallback (plain bars) differs from `TransactionList`'s own internal table skeleton, so there's a ~1-frame visual jump. Aligning them would mean either duplicating the table skeleton or adding a prop to the shared component; judged not worth it for a sub-frame cosmetic nit. - **Out of scope (pre-existing, from #631, flagged during review):** since #631 removed the remount `key`, an active filter/search now persists across a create-reload (a new non-matching row can look "not created"); and in-memory search/sort currently operates on still-encrypted rows pending the plaintext migration. Neither is introduced or worsened here. --- app/Http/Controllers/AccountController.php | 24 +++++------ resources/js/pages/Accounts/Show.test.tsx | 15 ++++++- resources/js/pages/Accounts/Show.tsx | 46 ++++++++++++++-------- tests/Feature/AccountControllerTest.php | 5 ++- 4 files changed, 59 insertions(+), 31 deletions(-) diff --git a/app/Http/Controllers/AccountController.php b/app/Http/Controllers/AccountController.php index 0bb56165..9e42d3f1 100644 --- a/app/Http/Controllers/AccountController.php +++ b/app/Http/Controllers/AccountController.php @@ -120,20 +120,20 @@ class AccountController extends Controller } } - // ponytail: load-all transactions for the account (server-side, replaces - // the old client-side sync-all-then-filter). Fine for a single account; - // switch to a deferred/cursor-paginated prop if accounts grow huge. - $transactions = $account->type->hasTransactionLedger() - ? $account->transactions() - ->with(['category', 'labels']) - ->orderBy('transaction_date', 'desc') - ->orderBy('id', 'desc') - ->get() - : []; - return Inertia::render('Accounts/Show', [ 'account' => $data, - 'transactions' => $transactions, + // Deferred so the page shell paints without blocking on the ledger + // query/serialization. It stays the whole set because search and + // filtering run client-side over decrypted rows. ponytail: window it + // server-side only if one account's history gets big enough that the + // transfer itself hurts. + 'transactions' => $account->type->hasTransactionLedger() + ? Inertia::defer(fn () => $account->transactions() + ->with(['category', 'labels']) + ->orderBy('transaction_date', 'desc') + ->orderBy('id', 'desc') + ->get()) + : [], ]); } diff --git a/resources/js/pages/Accounts/Show.test.tsx b/resources/js/pages/Accounts/Show.test.tsx index e1de798e..22d435bf 100644 --- a/resources/js/pages/Accounts/Show.test.tsx +++ b/resources/js/pages/Accounts/Show.test.tsx @@ -1,4 +1,5 @@ -import { fireEvent, render, screen } from '@testing-library/react'; +import { router } from '@inertiajs/react'; +import { act, fireEvent, render, screen } from '@testing-library/react'; import { type ReactNode } from 'react'; import { describe, expect, it, vi } from 'vitest'; @@ -7,6 +8,7 @@ import AccountShow from './Show'; vi.mock('@inertiajs/react', () => ({ Head: () => null, router: { reload: vi.fn() }, + Deferred: ({ children }: { children: ReactNode }) => <>{children}, })); vi.mock('@/actions/App/Http/Controllers/AccountController', () => ({ @@ -115,6 +117,17 @@ describe('AccountShow', () => { ); }); + it('reloads only the deferred transactions prop after a transaction is created', () => { + renderPage(); + + const { onSuccess } = editTransactionDialog.mock.calls.at(-1)![0] as { + onSuccess: () => void; + }; + act(() => onSuccess()); + + expect(router.reload).toHaveBeenCalledWith({ only: ['transactions'] }); + }); + it('hides transaction action for connected accounts', () => { renderPage({ ...baseAccount, diff --git a/resources/js/pages/Accounts/Show.tsx b/resources/js/pages/Accounts/Show.tsx index b1f0ed03..c8ded843 100644 --- a/resources/js/pages/Accounts/Show.tsx +++ b/resources/js/pages/Accounts/Show.tsx @@ -39,6 +39,7 @@ import { SelectTrigger, SelectValue, } from '@/components/ui/select'; +import { Skeleton } from '@/components/ui/skeleton'; import { Textarea } from '@/components/ui/textarea'; import { useChartColors } from '@/hooks/use-chart-color-scheme'; import AppSidebarLayout from '@/layouts/app/app-sidebar-layout'; @@ -63,7 +64,7 @@ import { Label as LabelType } from '@/types/label'; import { type Transaction } from '@/types/transaction'; import { formatDateMedium } from '@/utils/date'; import { __ } from '@/utils/i18n'; -import { Head, router } from '@inertiajs/react'; +import { Deferred, Head, router } from '@inertiajs/react'; import { ChevronDown, Pencil, Plus } from 'lucide-react'; import { useCallback, useMemo, useState } from 'react'; import { Line, LineChart, ResponsiveContainer, Tooltip } from 'recharts'; @@ -382,23 +383,34 @@ export default function AccountShow({ )} {isTransactionalAccount(account) && ( - - setChartRefreshKey((key) => key + 1) + + {Array.from({ length: 8 }).map((_, i) => ( + + ))} + } - /> + > + + setChartRefreshKey((key) => key + 1) + } + /> + )} diff --git a/tests/Feature/AccountControllerTest.php b/tests/Feature/AccountControllerTest.php index 68e5db3f..3220e2d1 100644 --- a/tests/Feature/AccountControllerTest.php +++ b/tests/Feature/AccountControllerTest.php @@ -238,7 +238,10 @@ test('account show includes the account transactions and excludes other accounts ->assertOk() ->assertInertia(fn ($page) => $page ->component('Accounts/Show') - ->has('transactions', 2) + ->missing('transactions') + ->loadDeferredProps(fn ($reload) => $reload + ->has('transactions', 2) + ) ); });