refactor(inertia): group the shared user props and name the inline conditions (#773)

## 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<int, array{…}>` 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.
This commit is contained in:
Víctor Falcón 2026-08-11 15:09:55 +02:00 committed by GitHub
parent a912fb2f45
commit 6b441a0f02
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
1 changed files with 99 additions and 39 deletions

View File

@ -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<string, Closure>
*/
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<string, bool>
*/