feat(open-banking): disable already-connected banks in the connect picker (#556)
## Why Several production users ended up with **two connections to the same bank** (or duplicate accounts for the same IBAN) with EnableBanking. The auto-invalidation we assumed exists only happens on the dedicated *reconnect* flow, which reuses the same `BankingConnection` row. `AuthorizationController::store()` always creates a brand-new connection and never checks for an existing one, so re-adding an already-connected bank from scratch silently duplicates it. ## What - In the connect picker, already-connected EnableBanking banks are no longer **hidden** — they are shown **disabled with a tooltip**: *"You already have a connection with this bank. Reconnect it."* This both prevents the duplicate and guides the user to the right action. - Connections in `pending` state are excluded, so a stale/abandoned attempt no longer hides a bank forever (a latent bug in the old hide-based filter). - The same dialog opened from the **create-account flow** (`settings/accounts`) received no `connections`, so nothing was deduplicated there. The user's banking connections are now shared as a lightweight global Inertia prop (`bankingConnections`) and fed into both entry points, so they behave identically. As a bonus, crypto providers (Binance, etc.) are now also de-duplicated in that flow. ## Notes / follow-ups - Existing dirty data (the ~10 affected users) is intentionally left as-is per product decision. - The onboarding inline variant (`connect-account-inline.tsx`) still hides rather than disables; unifying the two near-duplicate connect components is a deliberate follow-up. - No backend guard was added to `store()`; this is UI-level prevention. A server-side guard is the robust follow-up if needed. ## Tests - `tests/Feature/InertiaSharedDataTest.php` — new shared prop is present and shaped correctly. - `resources/js/components/open-banking/connect-account-dialog.test.tsx` — `alreadyConnectedBankNames` includes active/error/expired, excludes pending and non-EnableBanking providers.
This commit is contained in:
Víctor Falcón
GitHub
parent
89c1ab1ca8
commit
6e6433c6ad
|
|
@ -122,6 +122,14 @@ class HandleInertiaRequests extends Middleware
|
|||
'valid_until' => $connection->valid_until?->toIso8601String(),
|
||||
'reconnect_url' => route('open-banking.reconnect', $connection),
|
||||
]) : [],
|
||||
'bankingConnections' => fn () => $user ? $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()
|
||||
->with(['bank', 'realEstateDetail:id,account_id,linked_loan_account_id'])
|
||||
->orderBy('name')
|
||||
|
|
|
|||
|
|
@ -1834,6 +1834,7 @@
|
|||
"You agree to indemnify, defend, and hold\\n harmless Whisper Money and its officers,\\n directors, employees, and agents from any\\n claims, liabilities, damages, losses, and\\n expenses, including reasonable legal fees,\\n arising out of or related to your use of the\\n service, violation of these Terms, or violation\\n of any rights of another party.": "Aceptas indemnizar, defender y eximir de responsabilidad a Whisper Money y sus directivos, directores, empleados y agentes de cualquier reclamación, responsabilidad, daño, pérdida y gasto, incluidos los honorarios legales razonables, derivados de o relacionados con tu uso del servicio, la violación de estos Términos o la violación de los derechos de otra parte.",
|
||||
"You agree to pay all applicable fees as described at the time of purchase": "Aceptas pagar todas las tarifas aplicables según se describan en el momento de la compra",
|
||||
"You agree to pay all applicable fees as\\n described at the time of purchase": "Aceptas pagar todas las tarifas aplicables tal como se describen en el momento de la compra",
|
||||
"You already have a connection with this bank. Reconnect it.": "Ya tienes una conexión con este banco, reconéctala.",
|
||||
"You already have accounts set up. Let's continue with the onboarding.": "Ya tienes cuentas configuradas. Continuemos con la configuración.",
|
||||
"You are always the owner of your data": "Siempre eres el dueño de tus datos",
|
||||
"You are being redirected in 3 seconds.": "Serás redirigido en 3 segundos.",
|
||||
|
|
|
|||
|
|
@ -16,6 +16,7 @@ import {
|
|||
import { getCsrfToken } from '@/lib/csrf';
|
||||
import { SharedData } from '@/types';
|
||||
import { Account } from '@/types/account';
|
||||
import type { BankingConnection } from '@/types/banking';
|
||||
import { __ } from '@/utils/i18n';
|
||||
import { router, usePage } from '@inertiajs/react';
|
||||
import { Link2, PenLine } from 'lucide-react';
|
||||
|
|
@ -35,12 +36,17 @@ export function CreateAccountDialog({
|
|||
auth,
|
||||
subscriptionsEnabled,
|
||||
accounts: sharedAccounts,
|
||||
bankingConnections: sharedConnections,
|
||||
} = usePage<SharedData>().props;
|
||||
const isFreePlan = subscriptionsEnabled && !auth?.hasProPlan;
|
||||
const sharedAccountsList = useMemo(
|
||||
() => (sharedAccounts as Account[]) || [],
|
||||
[sharedAccounts],
|
||||
);
|
||||
const connections = useMemo(
|
||||
() => (sharedConnections as BankingConnection[]) || [],
|
||||
[sharedConnections],
|
||||
);
|
||||
const availableLoanAccounts = useMemo(
|
||||
() => sharedAccountsList.filter((a) => a.type === 'loan'),
|
||||
[sharedAccountsList],
|
||||
|
|
@ -333,6 +339,7 @@ export function CreateAccountDialog({
|
|||
<ConnectAccountDialog
|
||||
open={connectDialogOpen}
|
||||
onOpenChange={setConnectDialogOpen}
|
||||
connections={connections}
|
||||
/>
|
||||
|
||||
<UpgradeConnectionDialog
|
||||
|
|
|
|||
|
|
@ -0,0 +1,50 @@
|
|||
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);
|
||||
});
|
||||
});
|
||||
|
|
@ -18,6 +18,11 @@ import {
|
|||
SelectValue,
|
||||
} from '@/components/ui/select';
|
||||
import { Textarea } from '@/components/ui/textarea';
|
||||
import {
|
||||
Tooltip,
|
||||
TooltipContent,
|
||||
TooltipTrigger,
|
||||
} from '@/components/ui/tooltip';
|
||||
import { getCsrfToken } from '@/lib/csrf';
|
||||
import type {
|
||||
BankingConnection,
|
||||
|
|
@ -82,6 +87,25 @@ 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;
|
||||
|
|
@ -140,6 +164,11 @@ export function ConnectAccountDialog({
|
|||
|
||||
const isWise = useMemo(() => selectedBank?.name === 'Wise', [selectedBank]);
|
||||
|
||||
const connectedBankNames = useMemo(
|
||||
() => alreadyConnectedBankNames(connections),
|
||||
[connections],
|
||||
);
|
||||
|
||||
const resetState = useCallback(() => {
|
||||
setStep('country');
|
||||
setCountry('');
|
||||
|
|
@ -198,11 +227,6 @@ export function ConnectAccountDialog({
|
|||
|
||||
const data = await response.json();
|
||||
|
||||
const connectedEnableBankingNames = new Set(
|
||||
connections
|
||||
.filter((c) => c.provider === 'enablebanking')
|
||||
.map((c) => c.aspsp_name),
|
||||
);
|
||||
const hasProvider = (provider: string) =>
|
||||
connections.some((c) => c.provider === provider);
|
||||
|
||||
|
|
@ -230,7 +254,7 @@ export function ConnectAccountDialog({
|
|||
if (institution.name === 'Indexa Capital') {
|
||||
return !hasProvider('indexacapital');
|
||||
}
|
||||
return !connectedEnableBankingNames.has(institution.name);
|
||||
return true;
|
||||
})
|
||||
.sort((a, b) => a.name.localeCompare(b.name));
|
||||
|
||||
|
|
@ -419,27 +443,58 @@ export function ConnectAccountDialog({
|
|||
/>
|
||||
|
||||
<div className="max-h-[300px] space-y-1 overflow-y-auto">
|
||||
{filteredInstitutions.map((institution) => (
|
||||
<button
|
||||
key={institution.name}
|
||||
type="button"
|
||||
className={`flex w-full items-center gap-3 rounded-md px-3 py-2 text-left text-sm transition-colors hover:bg-accent ${
|
||||
selectedBank?.name ===
|
||||
institution.name
|
||||
? 'bg-accent'
|
||||
: ''
|
||||
}`}
|
||||
onClick={() =>
|
||||
setSelectedBank(institution)
|
||||
}
|
||||
>
|
||||
<BankLogo
|
||||
src={institution.logo}
|
||||
className="h-6 w-6"
|
||||
/>
|
||||
<span>{institution.name}</span>
|
||||
</button>
|
||||
))}
|
||||
{filteredInstitutions.map((institution) => {
|
||||
const isConnected = connectedBankNames.has(
|
||||
institution.name,
|
||||
);
|
||||
|
||||
const item = (
|
||||
<button
|
||||
key={institution.name}
|
||||
type="button"
|
||||
aria-disabled={isConnected}
|
||||
className={`flex w-full items-center gap-3 rounded-md px-3 py-2 text-left text-sm transition-colors ${
|
||||
isConnected
|
||||
? 'cursor-not-allowed opacity-50'
|
||||
: 'hover:bg-accent'
|
||||
} ${
|
||||
selectedBank?.name ===
|
||||
institution.name
|
||||
? 'bg-accent'
|
||||
: ''
|
||||
}`}
|
||||
onClick={() => {
|
||||
if (isConnected) {
|
||||
return;
|
||||
}
|
||||
setSelectedBank(institution);
|
||||
}}
|
||||
>
|
||||
<BankLogo
|
||||
src={institution.logo}
|
||||
className="h-6 w-6"
|
||||
/>
|
||||
<span>{institution.name}</span>
|
||||
</button>
|
||||
);
|
||||
|
||||
if (!isConnected) {
|
||||
return item;
|
||||
}
|
||||
|
||||
return (
|
||||
<Tooltip key={institution.name}>
|
||||
<TooltipTrigger asChild>
|
||||
{item}
|
||||
</TooltipTrigger>
|
||||
<TooltipContent>
|
||||
{__(
|
||||
'You already have a connection with this bank. Reconnect it.',
|
||||
)}
|
||||
</TooltipContent>
|
||||
</Tooltip>
|
||||
);
|
||||
})}
|
||||
{filteredInstitutions.length === 0 && (
|
||||
<p className="py-4 text-center text-sm text-muted-foreground">
|
||||
{__('No banks found.')}
|
||||
|
|
|
|||
|
|
@ -109,6 +109,25 @@ test('authenticated users receive expired banking connection reconnect links', f
|
|||
);
|
||||
});
|
||||
|
||||
test('authenticated users receive their banking connections in shared props', function () {
|
||||
$user = User::factory()->onboarded()->create();
|
||||
$connection = BankingConnection::factory()->create([
|
||||
'user_id' => $user->id,
|
||||
'aspsp_name' => 'Bankinter',
|
||||
'status' => BankingConnectionStatus::Active,
|
||||
]);
|
||||
|
||||
$response = actingAs($user)->withoutVite()->get(route('dashboard'));
|
||||
|
||||
$response->assertInertia(fn (Assert $page) => $page
|
||||
->has('bankingConnections', 1)
|
||||
->where('bankingConnections.0.id', $connection->id)
|
||||
->where('bankingConnections.0.aspsp_name', 'Bankinter')
|
||||
->where('bankingConnections.0.provider', $connection->provider->value)
|
||||
->where('bankingConnections.0.status', 'active')
|
||||
);
|
||||
});
|
||||
|
||||
test('shared currency options split profile and account currencies', function () {
|
||||
$response = $this->withoutVite()->get(route('home'));
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue