fix(open-banking): only block re-adding a bank when a live connection exists (#569)
## 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`.
This commit is contained in:
parent
52708f940d
commit
14c4598cda
|
|
@ -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> = {},
|
||||
): 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);
|
||||
});
|
||||
});
|
||||
|
|
@ -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<string> {
|
||||
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,
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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> = {},
|
||||
): 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,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
@ -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<BankingConnection['status']> = 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<string> {
|
||||
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),
|
||||
);
|
||||
}
|
||||
Loading…
Reference in New Issue