From 6b441a0f029da01e6801b3cffaa2db3faa3aece0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Falc=C3=B3n?= Date: Tue, 11 Aug 2026 15:09:55 +0200 Subject: [PATCH] refactor(inertia): group the shared user props and name the inline conditions (#773) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Why `HandleInertiaRequests::share` was at cyclomatic complexity **18** — the highest left after #771. Almost all of it was one shape repeated seven times: ```php 'bankingConnections' => fn () => $user ? $user->bankingConnections()->get(...)->map(...) : [], 'accounts' => fn () => $user ? $user->accounts()->...->get() : [], 'categories' => fn () => $user ? $user->categories()->forDisplay()->get() : [], // ...four more ``` Each of those ternaries is a branch, and the guest answer is the same every time. ## What changed **`userCollectionProps()`** owns the seven deferred props. The guest case is answered once, up front: ```php if ($user === null) { return array_fill_keys(['expiredBankingConnections', 'bankingConnections', /* ... */], fn () => []); } ``` …so the seven queries below read as queries rather than as ternaries. `automationRules` was the odd one out (a full closure with an early `return []`) and now matches its siblings. **Three conditions got names** instead of living inline in the props array: | method | was | |---|---| | `isDemoAccount()` | `$user?->isDemoAccount() && ! app()->environment('local')` | | `demoCredentials()` | `config(...) && ($isDemoQuery \|\| $isDemoAccount) ? [...] : null` | | `hasResidualEncryptionArtifacts()` | the four-way `&&` chain gating the cleanup job | The comments now say *why* each one is what it is — that a demo account is only treated as one outside local, and that the cleanup job fires when the salt outlived the encrypted rows. ## Behaviour One deliberate change: `expiredBankingConnections` and `bankingConnections` end in `->all()`, returning a plain list instead of a `Collection`. The JSON is identical (a Collection of arrays serializes to the same array), and it keeps the prop's type expressible outside `share()` — `Collection`'s `TValue` is invariant, so phpstan cannot match `Closure(): Collection` against itself once the array literal lives in its own method. Everything else is a move. ## Metrics | | before | after | |---|---|---| | `share` | 18 | 4 | | `userCollectionProps` (new) | — | 2 | | `demoCredentials` / `hasResidualEncryptionArtifacts` / `isDemoAccount` (new) | — | 3 / 4 / 2 | ## Testing This middleware runs on every page, so: **the whole Feature suite — 1958 tests, 1957 passed, 1 skipped.** PHPStan clean. `InertiaSharedDataTest` covers exactly what moved: the guest path, the authenticated path, both encryption-cleanup branches (queued when the salt is residual, not queued when there is no salt), the expired-connection reconnect links and the connections prop — the two props whose return type changed. --- app/Http/Middleware/HandleInertiaRequests.php | 138 +++++++++++++----- 1 file changed, 99 insertions(+), 39 deletions(-) diff --git a/app/Http/Middleware/HandleInertiaRequests.php b/app/Http/Middleware/HandleInertiaRequests.php index d39085cd..382421f2 100644 --- a/app/Http/Middleware/HandleInertiaRequests.php +++ b/app/Http/Middleware/HandleInertiaRequests.php @@ -8,7 +8,9 @@ use App\Features\CalculateBalancesOnImport; use App\Features\Mcp; use App\Jobs\PurgeResidualEncryptionArtifactsJob; use App\Models\BankingConnection; +use App\Models\User; use App\Services\CurrencyOptions; +use Closure; use Illuminate\Foundation\Inspiring; use Illuminate\Http\Request; use Inertia\Middleware; @@ -49,8 +51,6 @@ class HandleInertiaRequests extends Middleware [$message, $author] = str(Inspiring::quotes()->random())->explode('-'); $user = $request->user(); - $isDemoAccount = $user?->isDemoAccount() && ! app()->environment('local'); - $isDemoQuery = $request->query('demo') === '1'; // Cache encryption checks to avoid duplicate queries $hasEncryptedAccounts = $user?->accounts()->where('encrypted', true)->exists() ?? false; @@ -62,7 +62,7 @@ class HandleInertiaRequests extends Middleware // encryption cleanup off to a queued job instead of mutating the user // inline during the render. The job re-checks the condition and is // idempotent, so dispatching it on repeat requests is harmless. - if (! $request->is('api/*') && $user?->encryption_salt !== null && ! $hasEncryptedAccounts && ! $hasEncryptedTransactions) { + if ($this->hasResidualEncryptionArtifacts($request, $user, $hasEncryptedAccounts, $hasEncryptedTransactions)) { PurgeResidualEncryptionArtifactsJob::dispatch($user); } @@ -81,17 +81,14 @@ class HandleInertiaRequests extends Middleware 'auth' => [ 'user' => $user, 'hasProPlan' => $user?->hasProPlan() ?? false, - 'isDemoAccount' => $isDemoAccount, + 'isDemoAccount' => $this->isDemoAccount($user), ], 'subscriptionPaymentIssue' => $user?->hasPastDueSubscription() ? [ 'status' => 'past_due', 'action_url' => route('settings.billing.portal'), ] : null, 'demoEnabled' => (bool) config('app.demo.enabled'), - 'demoCredentials' => config('app.demo.enabled') && ($isDemoQuery || $isDemoAccount) ? [ - 'email' => config('app.demo.email'), - 'password' => config('app.demo.password'), - ] : null, + 'demoCredentials' => $this->demoCredentials($request, $user), 'subscriptionsEnabled' => config('subscriptions.enabled', false), 'aiCategorizationUpsellRate' => (int) config('ai_categorization.upsell_sample_rate'), 'pricing' => [ @@ -106,7 +103,41 @@ class HandleInertiaRequests extends Middleware 'includeRealEstateInNetWorthChart' => $user?->setting->include_real_estate_in_net_worth_chart ?? true, 'sidebarOpen' => ! $request->hasCookie('sidebar_state') || $request->cookie('sidebar_state') === 'true', 'features' => $this->resolveFeatureFlags(), - 'expiredBankingConnections' => fn () => $user ? $user->bankingConnections() + ...$this->userCollectionProps($user), + 'hasEncryptedAccounts' => $hasEncryptedAccounts, + 'hasEncryptionSetup' => $user?->encryption_salt !== null, + 'hasEncryptedTransactions' => $hasEncryptedTransactions, + 'locale' => app()->getLocale(), + 'translations' => $this->getTranslations(), + 'currencies' => [ + 'profile' => $this->currencyOptions->primaryOptions(), + 'accounts' => $this->currencyOptions->accountOptions(), + ], + ]; + } + + /** + * The deferred props that list the user's own records. Guests get empty lists + * so the frontend can read them unconditionally. + * + * @return array + */ + private function userCollectionProps(?User $user): array + { + if ($user === null) { + return array_fill_keys([ + 'expiredBankingConnections', + 'bankingConnections', + 'accounts', + 'categories', + 'banks', + 'automationRules', + 'labels', + ], fn () => []); + } + + return [ + 'expiredBankingConnections' => fn () => $user->bankingConnections() ->where('provider', BankingProvider::EnableBanking) ->where(function ($query) { $query->where('status', BankingConnectionStatus::Expired) @@ -124,51 +155,80 @@ class HandleInertiaRequests extends Middleware 'provider' => $connection->provider->value, 'valid_until' => $connection->valid_until?->toIso8601String(), 'reconnect_url' => route('open-banking.reconnect', $connection), - ]) : [], - 'bankingConnections' => fn () => $user ? $user->bankingConnections() + ]) + ->all(), + 'bankingConnections' => fn () => $user->bankingConnections() ->get(['id', 'aspsp_name', 'provider', 'status']) ->map(fn (BankingConnection $connection): array => [ 'id' => $connection->id, 'aspsp_name' => $connection->aspsp_name, 'provider' => $connection->provider->value, 'status' => $connection->status->value, - ]) : [], - 'accounts' => fn () => $user ? $user->accounts() + ]) + ->all(), + 'accounts' => fn () => $user->accounts() ->with(['bank', 'realEstateDetail:id,account_id,linked_loan_account_id']) ->orderBy('name') ->get() - ->makeHidden('realEstateDetail') : [], - 'categories' => fn () => $user ? $user->categories() + ->makeHidden('realEstateDetail'), + 'categories' => fn () => $user->categories() ->forDisplay() - ->get() : [], - 'banks' => fn () => $user ? $user->banks() + ->get(), + 'banks' => fn () => $user->banks() ->orderBy('name') - ->get() : [], - 'automationRules' => function () use ($user) { - if (! $user) { - return []; - } - - return $user->automationRules() - ->with(['category', 'labels']) - ->orderBy('priority') - ->get(); - }, - 'labels' => fn () => $user ? $user->labels() + ->get(), + 'automationRules' => fn () => $user->automationRules() + ->with(['category', 'labels']) + ->orderBy('priority') + ->get(), + 'labels' => fn () => $user->labels() ->orderBy('name') - ->get() : [], - 'hasEncryptedAccounts' => $hasEncryptedAccounts, - 'hasEncryptionSetup' => $user?->encryption_salt !== null, - 'hasEncryptedTransactions' => $hasEncryptedTransactions, - 'locale' => app()->getLocale(), - 'translations' => $this->getTranslations(), - 'currencies' => [ - 'profile' => $this->currencyOptions->primaryOptions(), - 'accounts' => $this->currencyOptions->accountOptions(), - ], + ->get(), ]; } + /** + * The demo account is only treated as one outside local, where a developer + * signed in as it should get the normal app. + */ + private function isDemoAccount(?User $user): bool + { + return ($user?->isDemoAccount() ?? false) && ! app()->environment('local'); + } + + /** + * The demo login is prefilled for whoever asked for it (?demo=1) and for the + * demo account itself, so it can sign back in after logging out. + * + * @return array{email: ?string, password: ?string}|null + */ + private function demoCredentials(Request $request, ?User $user): ?array + { + $wantsDemo = $request->query('demo') === '1' || $this->isDemoAccount($user); + + if (! config('app.demo.enabled') || ! $wantsDemo) { + return null; + } + + return [ + 'email' => config('app.demo.email'), + 'password' => config('app.demo.password'), + ]; + } + + /** + * True when the user finished decrypting their data but the encryption salt + * and other artifacts are still on the row, so the cleanup job has work. + * Skipped for API requests, which are not a render path. + */ + private function hasResidualEncryptionArtifacts(Request $request, ?User $user, bool $hasEncryptedAccounts, bool $hasEncryptedTransactions): bool + { + return ! $request->is('api/*') + && $user?->encryption_salt !== null + && ! $hasEncryptedAccounts + && ! $hasEncryptedTransactions; + } + /** * @return array */