From 14c4598cda6989017af9dd8551791d5a2c456a9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Falc=C3=B3n?= Date: Sat, 20 Jun 2026 13:09:04 +0200 Subject: [PATCH] fix(open-banking): only block re-adding a bank when a live connection exists (#569) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What When creating a new connection / connected account, the bank list filtered out any bank the user already had a connection to. The check was too broad: - It blocked a bank whenever a **non-pending** connection existed, so **expired, errored and revoked-but-not-deleted** connections kept blocking re-connection. - The inline variant was even broader — it blocked on **stale pending** attempts too. ## Fix A bank is now treated as "already connected" only when a connection is genuinely **live**, applying three checks: - **Right provider** — only `enablebanking` connections gate EnableBanking banks; the exact provider gates Binance/Bitpanda/Coinbase/Indexa Capital. - **Active** — only `active` or `awaiting_mapping` statuses block. `expired`, `error`, `revoked` and `pending` no longer block, so the user can start a fresh connection. - **Not deleted** — soft-deleted connections never reach the frontend (no query uses `withTrashed`), so a deleted connection never blocks. The two divergent filter implementations (`connect-account-dialog.tsx` and `connect-account-inline.tsx`) are unified into a single helper in `resources/js/lib/banking-connections.ts`. ### Covered scenarios - Manual BBVA account → not a `BankingConnection`, never blocked → can connect BBVA. ✓ - Deleted BBVA connection → soft-deleted, absent from the frontend → can connect Enable Banking + BBVA. ✓ ## Tests - New `resources/js/lib/banking-connections.test.ts` (replaces the old `connect-account-dialog.test.tsx`). ## Note `awaiting_mapping` is treated as live (bank just authorized, accounts not yet mapped) to avoid duplicate connections. If only `active` should block, drop it from `LIVE_STATUSES`. --- .../connect-account-dialog.test.tsx | 50 ----------- .../open-banking/connect-account-dialog.tsx | 25 ++---- .../open-banking/connect-account-inline.tsx | 13 +-- resources/js/lib/banking-connections.test.ts | 82 +++++++++++++++++++ resources/js/lib/banking-connections.ts | 47 +++++++++++ 5 files changed, 141 insertions(+), 76 deletions(-) delete mode 100644 resources/js/components/open-banking/connect-account-dialog.test.tsx create mode 100644 resources/js/lib/banking-connections.test.ts create mode 100644 resources/js/lib/banking-connections.ts diff --git a/resources/js/components/open-banking/connect-account-dialog.test.tsx b/resources/js/components/open-banking/connect-account-dialog.test.tsx deleted file mode 100644 index a27afcb6..00000000 --- a/resources/js/components/open-banking/connect-account-dialog.test.tsx +++ /dev/null @@ -1,50 +0,0 @@ -import type { BankingConnection } from '@/types/banking'; -import { describe, expect, it } from 'vitest'; -import { alreadyConnectedBankNames } from './connect-account-dialog'; - -function connection( - overrides: Partial = {}, -): BankingConnection { - return { - id: crypto.randomUUID(), - provider: 'enablebanking', - aspsp_name: 'Bankinter', - aspsp_country: 'ES', - status: 'active', - valid_until: null, - last_synced_at: null, - error_message: null, - accounts_count: 1, - created_at: '2026-01-01T00:00:00Z', - updated_at: '2026-01-01T00:00:00Z', - ...overrides, - }; -} - -describe('alreadyConnectedBankNames', () => { - it('includes active, error and expired EnableBanking banks', () => { - const names = alreadyConnectedBankNames([ - connection({ aspsp_name: 'Bankinter', status: 'active' }), - connection({ aspsp_name: 'BBVA', status: 'error' }), - connection({ aspsp_name: 'ING', status: 'expired' }), - ]); - - expect(names).toEqual(new Set(['Bankinter', 'BBVA', 'ING'])); - }); - - it('excludes pending connections so a stale attempt does not block re-adding', () => { - const names = alreadyConnectedBankNames([ - connection({ aspsp_name: 'Bankinter', status: 'pending' }), - ]); - - expect(names.has('Bankinter')).toBe(false); - }); - - it('ignores non-EnableBanking providers', () => { - const names = alreadyConnectedBankNames([ - connection({ provider: 'binance', aspsp_name: 'Binance' }), - ]); - - expect(names.has('Binance')).toBe(false); - }); -}); diff --git a/resources/js/components/open-banking/connect-account-dialog.tsx b/resources/js/components/open-banking/connect-account-dialog.tsx index 67e27d27..0b0609e0 100644 --- a/resources/js/components/open-banking/connect-account-dialog.tsx +++ b/resources/js/components/open-banking/connect-account-dialog.tsx @@ -23,6 +23,10 @@ import { TooltipContent, TooltipTrigger, } from '@/components/ui/tooltip'; +import { + alreadyConnectedBankNames, + hasLiveConnectionForProvider, +} from '@/lib/banking-connections'; import { getCsrfToken } from '@/lib/csrf'; import type { BankingConnection, @@ -87,25 +91,6 @@ const WISE_INSTITUTION: EnableBankingInstitution = { maximum_consent_validity: null, }; -/** - * Names of EnableBanking ASPSPs the user already has a connection to. - * - * Pending connections are excluded: they are throwaway, mid-flow attempts, so a - * stale one must not block re-adding the bank. Any other status (active, error, - * expired, …) counts as already connected and should be re-used via reconnect. - */ -export function alreadyConnectedBankNames( - connections: BankingConnection[], -): Set { - return new Set( - connections - .filter( - (c) => c.provider === 'enablebanking' && c.status !== 'pending', - ) - .map((c) => c.aspsp_name), - ); -} - interface ConnectAccountDialogProps { open: boolean; onOpenChange: (open: boolean) => void; @@ -228,7 +213,7 @@ export function ConnectAccountDialog({ const data = await response.json(); const hasProvider = (provider: string) => - connections.some((c) => c.provider === provider); + hasLiveConnectionForProvider(connections, provider); const extraInstitutions = [ BINANCE_INSTITUTION, diff --git a/resources/js/components/open-banking/connect-account-inline.tsx b/resources/js/components/open-banking/connect-account-inline.tsx index ea585db6..7d6f9a57 100644 --- a/resources/js/components/open-banking/connect-account-inline.tsx +++ b/resources/js/components/open-banking/connect-account-inline.tsx @@ -11,6 +11,10 @@ import { } from '@/components/ui/select'; import { Textarea } from '@/components/ui/textarea'; import { useWebHaptics } from '@/hooks/use-web-haptics'; +import { + alreadyConnectedBankNames, + hasLiveConnectionForProvider, +} from '@/lib/banking-connections'; import { getCsrfToken } from '@/lib/csrf'; import type { BankingConnection, @@ -166,13 +170,10 @@ export function ConnectAccountInline({ const data = await response.json(); - const connectedEnableBankingNames = new Set( - connections - .filter((c) => c.provider === 'enablebanking') - .map((c) => c.aspsp_name), - ); + const connectedEnableBankingNames = + alreadyConnectedBankNames(connections); const hasProvider = (provider: string) => - connections.some((c) => c.provider === provider); + hasLiveConnectionForProvider(connections, provider); const extraInstitutions = [ BINANCE_INSTITUTION, diff --git a/resources/js/lib/banking-connections.test.ts b/resources/js/lib/banking-connections.test.ts new file mode 100644 index 00000000..d86e2c20 --- /dev/null +++ b/resources/js/lib/banking-connections.test.ts @@ -0,0 +1,82 @@ +import type { BankingConnection } from '@/types/banking'; +import { describe, expect, it } from 'vitest'; +import { + alreadyConnectedBankNames, + hasLiveConnectionForProvider, +} from './banking-connections'; + +function connection( + overrides: Partial = {}, +): BankingConnection { + return { + id: crypto.randomUUID(), + provider: 'enablebanking', + aspsp_name: 'Bankinter', + aspsp_country: 'ES', + status: 'active', + valid_until: null, + last_synced_at: null, + error_message: null, + accounts_count: 1, + created_at: '2026-01-01T00:00:00Z', + updated_at: '2026-01-01T00:00:00Z', + ...overrides, + }; +} + +describe('alreadyConnectedBankNames', () => { + it('includes live EnableBanking banks (active and awaiting_mapping)', () => { + const names = alreadyConnectedBankNames([ + connection({ aspsp_name: 'Bankinter', status: 'active' }), + connection({ aspsp_name: 'BBVA', status: 'awaiting_mapping' }), + ]); + + expect(names).toEqual(new Set(['Bankinter', 'BBVA'])); + }); + + it('excludes expired, revoked, error and pending so the bank can be re-added', () => { + const names = alreadyConnectedBankNames([ + connection({ aspsp_name: 'Bankinter', status: 'pending' }), + connection({ aspsp_name: 'BBVA', status: 'expired' }), + connection({ aspsp_name: 'ING', status: 'revoked' }), + connection({ aspsp_name: 'Santander', status: 'error' }), + ]); + + expect(names).toEqual(new Set()); + }); + + it('ignores non-EnableBanking providers', () => { + const names = alreadyConnectedBankNames([ + connection({ provider: 'binance', aspsp_name: 'Binance' }), + ]); + + expect(names.has('Binance')).toBe(false); + }); +}); + +describe('hasLiveConnectionForProvider', () => { + it('is true only when a live connection for the provider exists', () => { + const connections = [ + connection({ provider: 'binance', status: 'active' }), + ]; + + expect(hasLiveConnectionForProvider(connections, 'binance')).toBe(true); + expect(hasLiveConnectionForProvider(connections, 'coinbase')).toBe( + false, + ); + }); + + it('ignores non-live connections so the provider can be re-added', () => { + const connections = [ + connection({ provider: 'binance', status: 'error' }), + connection({ provider: 'coinbase', status: 'expired' }), + ]; + + expect(hasLiveConnectionForProvider(connections, 'binance')).toBe( + false, + ); + expect(hasLiveConnectionForProvider(connections, 'coinbase')).toBe( + false, + ); + }); +}); diff --git a/resources/js/lib/banking-connections.ts b/resources/js/lib/banking-connections.ts new file mode 100644 index 00000000..fa034bf2 --- /dev/null +++ b/resources/js/lib/banking-connections.ts @@ -0,0 +1,47 @@ +import type { BankingConnection } from '@/types/banking'; + +/** + * Statuses that count as a live connection. Only these block re-adding the same + * bank: the connection is either usable (active) or freshly authorized and + * awaiting account mapping. Pending (abandoned mid-flow), expired, revoked and + * error connections never block, so the user can always start a fresh one. + * + * Soft-deleted connections never reach the frontend, so a deleted connection + * never blocks either. + */ +const LIVE_STATUSES: ReadonlySet = new Set([ + 'active', + 'awaiting_mapping', +]); + +function isLiveConnection(connection: BankingConnection): boolean { + return LIVE_STATUSES.has(connection.status); +} + +/** + * Names of EnableBanking ASPSPs the user has a live connection to. + */ +export function alreadyConnectedBankNames( + connections: BankingConnection[], +): Set { + return new Set( + connections + .filter( + (c) => c.provider === 'enablebanking' && isLiveConnection(c), + ) + .map((c) => c.aspsp_name), + ); +} + +/** + * Whether the user already has a live connection for a single-connection + * provider (Binance, Bitpanda, Coinbase, Indexa Capital, …). + */ +export function hasLiveConnectionForProvider( + connections: BankingConnection[], + provider: string, +): boolean { + return connections.some( + (c) => c.provider === provider && isLiveConnection(c), + ); +}