From eb60f2eb30933ee103d67eb85a364fcfd1036e58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Falc=C3=B3n?= Date: Sun, 9 Aug 2026 18:41:35 +0200 Subject: [PATCH] fix(open-banking): unblock account mapping when the bank reports accounts without a uid (#746) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem A paying user reported that "Save & Sync" on the account mapping screen did nothing — no error, no navigation, nothing. EnableBanking returned their Société Générale card (`CB Visa`) with `uid: null`: ```json {"uid": null, "name": "CB Visa", "cash_account_type": "CARD", "account_id": {"iban": null, "other": {"scheme_name": "CPAN", "identification": "************8210"}}} ``` The page renders one mapping row per pending account, so the form posted `bank_account_uid: null`. `MapAccountsRequest` requires it, the request 422'd, and since the page never rendered validation errors the button looked dead. The user retried the connection six times. A second problem kept them from doing what they actually wanted: both accounts reported `currency: "XXX"` — ISO 4217 for "no currency". `getCompatibleAccounts` filtered their existing EUR accounts against `"XXX"`, matched nothing, and the "Link to existing account" option was never rendered. The backend already knew `XXX` isn't a currency (`AccountUserCurrencyService::resolveImportedCurrency`); the frontend didn't. ## Fix - Accounts without a uid are filtered out of the mapping screen. They can never be synced anyway — every sync service early-returns on an empty `external_account_id` — and `CreatesAccountsFromPending` already skipped them during onboarding. That rule now lives in one place, `BankingConnection::mappablePendingAccounts()`. - The skipped accounts are named on the page, so users don't go hunting for an account they can see in their bank app. - A connection where *no* account has a uid is closed out instead of parking on a mapping page that could only ever 422. - Validation errors surface as a toast, with app-authored messages in `MapAccountsRequest::messages()` instead of raw field paths. - `XXX` (and a blank/lowercase variant) is treated as "unknown currency": the code isn't displayed, and it no longer filters out every linkable account. ## Scope One user affected in production (the reporter). Their connection has one valid account alongside the card, so this fix unblocks them on deploy — no data change needed. ## Testing - `tests/Feature/OpenBanking` — 315 passed, including two new cases: uid-less accounts are hidden and named, and an all-uid-less connection is closed rather than left awaiting mapping. - Browser QA against a local reproduction of the exact production payload: only `Compte Bancaire` is offered, `CB Visa` is named as skipped, "Link to existing account" appears despite the `XXX` currency, submitting without picking an account toasts the error, and picking `Dany SG` links it (`external_account_id` set, connection `active`). ## Demo https://github.com/user-attachments/assets/6275834a-a518-47b9-9015-a698e5926d71 ## Not in this PR `AuthorizationController::refreshAccountIds()` consumes the raw pending list on reconnect; its positional fallback can pair a legacy account with a uid-less entry. Not reachable for any account in production today, it belongs to a different flow, and it deserves its own test — filed separately rather than smuggled in here. --- .../OpenBanking/AccountMappingController.php | 15 ++++- .../Concerns/CreatesAccountsFromPending.php | 8 +-- .../OpenBanking/MapAccountsRequest.php | 13 ++++ app/Models/BankingConnection.php | 34 ++++++++++- lang/es.json | 2 + lang/fr.json | 2 + .../js/pages/open-banking/map-accounts.tsx | 61 +++++++++++++++---- .../OpenBanking/AccountMappingTest.php | 60 ++++++++++++++++++ 8 files changed, 174 insertions(+), 21 deletions(-) diff --git a/app/Http/Controllers/OpenBanking/AccountMappingController.php b/app/Http/Controllers/OpenBanking/AccountMappingController.php index bf65449e..0c0dd024 100644 --- a/app/Http/Controllers/OpenBanking/AccountMappingController.php +++ b/app/Http/Controllers/OpenBanking/AccountMappingController.php @@ -42,6 +42,18 @@ class AccountMappingController extends Controller ->with('success', 'Bank account connected successfully.'); } + $mappableAccounts = $connection->mappablePendingAccounts(); + + // Nothing the bank gave us can be mapped, so there is no decision left for the + // user to make. Close the connection instead of leaving it awaiting a mapping + // that can never happen. + if ($mappableAccounts === []) { + $this->createAccountsFromPending($user, $connection, $accountUserCurrencyService); + + return redirect()->route('settings.connections.index') + ->with('error', __('Your bank did not provide an identifier for any of its accounts, so they cannot be synced.')); + } + $existingAccounts = $user ->accounts() ->whereNull('banking_connection_id') @@ -50,8 +62,9 @@ class AccountMappingController extends Controller return Inertia::render('open-banking/map-accounts', [ 'connection' => $connection, - 'bankAccounts' => $connection->pending_accounts_data, + 'bankAccounts' => $mappableAccounts, 'existingAccounts' => $existingAccounts, + 'unmappableAccountNames' => $connection->unmappablePendingAccountNames(), ]); } diff --git a/app/Http/Controllers/OpenBanking/Concerns/CreatesAccountsFromPending.php b/app/Http/Controllers/OpenBanking/Concerns/CreatesAccountsFromPending.php index 49ae2f45..6785d703 100644 --- a/app/Http/Controllers/OpenBanking/Concerns/CreatesAccountsFromPending.php +++ b/app/Http/Controllers/OpenBanking/Concerns/CreatesAccountsFromPending.php @@ -30,12 +30,8 @@ trait CreatesAccountsFromPending $accountType = $connection->provider->defaultAccountType(); - foreach ($connection->pending_accounts_data ?? [] as $accountData) { - $uid = $accountData['uid'] ?? null; - - if (! $uid) { - continue; - } + foreach ($connection->mappablePendingAccounts() as $accountData) { + $uid = $accountData['uid']; $currency = $accountUserCurrencyService->resolveImportedCurrency($accountData['currency'] ?? null, $user); $name = $accountData['name'] diff --git a/app/Http/Requests/OpenBanking/MapAccountsRequest.php b/app/Http/Requests/OpenBanking/MapAccountsRequest.php index 41b71c34..fa54100e 100644 --- a/app/Http/Requests/OpenBanking/MapAccountsRequest.php +++ b/app/Http/Requests/OpenBanking/MapAccountsRequest.php @@ -31,4 +31,17 @@ class MapAccountsRequest extends FormRequest ], ]; } + + /** + * @return array + */ + public function messages(): array + { + return [ + 'mappings.required' => 'There are no accounts to map.', + 'mappings.min' => 'There are no accounts to map.', + 'mappings.*.bank_account_uid.required' => 'This account cannot be mapped because your bank did not provide an identifier for it.', + 'mappings.*.existing_account_id.required_if' => 'Choose the account you want to link to.', + ]; + } } diff --git a/app/Models/BankingConnection.php b/app/Models/BankingConnection.php index eea236cb..3c948bc3 100644 --- a/app/Models/BankingConnection.php +++ b/app/Models/BankingConnection.php @@ -24,7 +24,7 @@ use Illuminate\Database\Eloquent\SoftDeletes; * @property Carbon|null $bank_transactions_email_cutoff_at * @property Carbon|null $rate_limited_until * @property int $consecutive_sync_failures - * @property array|null $pending_accounts_data + * @property array>|null $pending_accounts_data */ class BankingConnection extends Model { @@ -152,6 +152,38 @@ class BankingConnection extends Model return ! empty($this->pending_accounts_data); } + /** + * Pending accounts the user can actually map. Providers sometimes report accounts + * without a uid (EnableBanking does it for some French cards); those can never be + * synced, so offering them for mapping only produces a validation error. + * + * @return array> + */ + public function mappablePendingAccounts(): array + { + return array_values(array_filter( + $this->pending_accounts_data ?? [], + fn (array $account): bool => ! empty($account['uid']), + )); + } + + /** + * Names of the pending accounts left out of the mapping screen, so the user is told + * why an account they can see in their bank never shows up here. + * + * @return array + */ + public function unmappablePendingAccountNames(): array + { + return array_values(array_map( + fn (array $account): string => $account['name'] ?? $account['account_id']['iban'] ?? __('Bank Account'), + array_filter( + $this->pending_accounts_data ?? [], + fn (array $account): bool => empty($account['uid']), + ), + )); + } + public function isExpired(): bool { return $this->status === BankingConnectionStatus::Expired diff --git a/lang/es.json b/lang/es.json index 7fe492b8..6863a6d8 100644 --- a/lang/es.json +++ b/lang/es.json @@ -1150,6 +1150,8 @@ "No balance records found.": "No se encontraron registros de balance.", "No bank connections yet. Connect a bank to automatically sync your transactions.": "Aún no hay conexiones bancarias. Conecta un banco para sincronizar automáticamente tus transacciones.", "No bank found.": "No se encontró ningún banco.", + "Not shown: :accounts. Your bank did not provide an identifier, so syncing is not possible.": "No se muestran: :accounts. Tu banco no proporcionó un identificador, así que no es posible sincronizarlas.", + "Your bank did not provide an identifier for any of its accounts, so they cannot be synced.": "Tu banco no proporcionó un identificador para ninguna de sus cuentas, así que no se pueden sincronizar.", "No banks found.": "No se encontraron bancos.", "No cashflow data for this period": "Sin datos de flujo de dinero para este período", "No categories found.": "No se encontraron categorías.", diff --git a/lang/fr.json b/lang/fr.json index caf09934..e91be8d3 100644 --- a/lang/fr.json +++ b/lang/fr.json @@ -1111,6 +1111,8 @@ "No balance records found.": "Aucun enregistrement de solde trouvé.", "No bank connections yet. Connect a bank to automatically sync your transactions.": "Il n'y a pas encore de connexions bancaires. Connectez une banque pour synchroniser automatiquement vos transactions.", "No bank found.": "Aucun banc n'a été trouvé.", + "Not shown: :accounts. Your bank did not provide an identifier, so syncing is not possible.": "Non affichés : :accounts. Votre banque n'a pas fourni d'identifiant, la synchronisation est donc impossible.", + "Your bank did not provide an identifier for any of its accounts, so they cannot be synced.": "Votre banque n'a fourni d'identifiant pour aucun de ses comptes, ils ne peuvent donc pas être synchronisés.", "No banks found.": "Aucun banc n'a été trouvé.", "No cashflow data for this period": "Aucune donnée sur les flux monétaires pour cette période", "No categories found.": "Aucune catégorie trouvée.", diff --git a/resources/js/pages/open-banking/map-accounts.tsx b/resources/js/pages/open-banking/map-accounts.tsx index 55007ba5..70052643 100644 --- a/resources/js/pages/open-banking/map-accounts.tsx +++ b/resources/js/pages/open-banking/map-accounts.tsx @@ -21,6 +21,17 @@ import type { BankingConnection, PendingBankAccount } from '@/types/banking'; import { __ } from '@/utils/i18n'; import { Head, Link, router } from '@inertiajs/react'; import { useState } from 'react'; +import { toast } from 'sonner'; + +/** + * Banks don't always report a usable currency: EnableBanking sends XXX, the ISO 4217 + * code for "no currency". Treat that as unknown rather than as a real code. + */ +function usableCurrency(reported: string | null | undefined): string | null { + const currency = (reported ?? '').trim().toUpperCase(); + + return currency === '' || currency === 'XXX' ? null : currency; +} interface Mapping { bank_account_uid: string; @@ -32,12 +43,14 @@ interface Props { connection: BankingConnection; bankAccounts: PendingBankAccount[]; existingAccounts: Account[]; + unmappableAccountNames: string[]; } export default function MapAccountsPage({ connection, bankAccounts, existingAccounts, + unmappableAccountNames, }: Props) { const [mappings, setMappings] = useState( bankAccounts.map((ba) => ({ @@ -56,7 +69,12 @@ export default function MapAccountsPage({ ); } - function getCompatibleAccounts(currency: string) { + /** With no currency reported there is nothing to match on, so anything is linkable. */ + function getLinkableAccounts(currency: string | null) { + if (!currency) { + return existingAccounts; + } + return existingAccounts.filter((a) => a.currency_code === currency); } @@ -67,6 +85,11 @@ export default function MapAccountsPage({ `/open-banking/connections/${connection.id}/map-accounts`, { mappings }, { + onError: (errors) => { + toast.error( + Object.values(errors)[0] ?? __('Something went wrong.'), + ); + }, onFinish: () => setProcessing(false), }, ); @@ -94,13 +117,17 @@ export default function MapAccountsPage({ const mapping = mappings.find( (m) => m.bank_account_uid === bankAccount.uid, ); - const compatibleAccounts = getCompatibleAccounts( - bankAccount.currency, - ); + const currency = usableCurrency(bankAccount.currency); + const compatibleAccounts = + getLinkableAccounts(currency); + const iban = bankAccount.account_id?.iban; const displayName = - bankAccount.name || - bankAccount.account_id?.iban || - __('Bank Account'); + bankAccount.name || iban || __('Bank Account'); + const details = [ + currency, + // Don't repeat the IBAN when it is already the title. + displayName === iban ? null : iban, + ].filter(Boolean); return ( @@ -108,12 +135,11 @@ export default function MapAccountsPage({ {displayName} - - {bankAccount.currency} - {bankAccount.account_id?.iban && - bankAccount.name && - ` \u00b7 ${bankAccount.account_id.iban}`} - + {details.length > 0 && ( + + {details.join(' \u00b7 ')} + + )} 0 && ( +

+ {__( + 'Not shown: :accounts. Your bank did not provide an identifier, so syncing is not possible.', + { accounts: unmappableAccountNames.join(', ') }, + )} +

+ )} +