fix(banking): stop Wise appearing multiple times in the connect list (#589)
## Problem
In production, searching for a bank like **Wise** in the *Connect Bank
Account* picker showed it many times, and **the count grew every time
the search box changed** (4 → 6 → …). Only one entry — the real native
integration — should appear.
## Root cause
Two bugs stacked on top of each other:
1. **Non-unique React key.** The list rendered each institution with
`key={institution.name}`. Wise is offered both natively (our own
API-token integration) and by the Enable Banking aggregator, so two rows
shared the key `"Wise"`. On every re-render of the filtered list, React
couldn't reconcile the duplicate keys and **leaked orphaned DOM nodes**
instead of replacing them — so the entry multiplied with each keystroke
(and rendered out of alphabetical order). React even warned: *"two
children with the same key, Wise … may cause children to be
duplicated"*.
2. **No dedup against native integrations.** Even without the growth,
Wise showed twice (aggregator + native). It should only surface through
our own integration.
The bank list is built from the Enable Banking API merged with our
native providers — it does **not** read from the `banks` table, which
was a red herring (only 2 non-duplicated Wise rows there).
## Changes
- Unique key (`name-country-index`) in both the dialog and inline
pickers.
- Drop aggregator institutions we already integrate natively, so Wise
only surfaces through its own integration (general — also covers
Binance, Coinbase, etc.).
- Point the native Wise logo at the official brand mark; remove the
now-unused local asset.
## Tests
Added regression tests covering dedup and repeated filtering. Reproduced
the bug first (counts grew `[3,4,5,6,7]`), then confirmed the fix holds
it at `1`. Full JS suite green (242 tests).
This commit is contained in:
Víctor Falcón
GitHub
parent
619ed0f1db
commit
ed5aac0c4a
Binary file not shown.
|
Before Width: | Height: | Size: 6.8 KiB |
|
|
@ -70,23 +70,27 @@ function liveBbvaConnection(): BankingConnection {
|
|||
};
|
||||
}
|
||||
|
||||
async function reachBankStep(connections: BankingConnection[]) {
|
||||
type Institution = {
|
||||
name: string;
|
||||
country: string;
|
||||
logo: string;
|
||||
maximum_consent_validity: null;
|
||||
};
|
||||
|
||||
function institution(name: string, logo = ''): Institution {
|
||||
return { name, country: 'ES', logo, maximum_consent_validity: null };
|
||||
}
|
||||
|
||||
async function reachBankStep(
|
||||
connections: BankingConnection[],
|
||||
institutions: Institution[] = [
|
||||
institution('BBVA'),
|
||||
institution('CaixaBank'),
|
||||
],
|
||||
) {
|
||||
global.fetch = vi.fn().mockResolvedValue({
|
||||
ok: true,
|
||||
json: async () => [
|
||||
{
|
||||
name: 'BBVA',
|
||||
country: 'ES',
|
||||
logo: '',
|
||||
maximum_consent_validity: null,
|
||||
},
|
||||
{
|
||||
name: 'CaixaBank',
|
||||
country: 'ES',
|
||||
logo: '',
|
||||
maximum_consent_validity: null,
|
||||
},
|
||||
],
|
||||
json: async () => institutions,
|
||||
}) as unknown as typeof fetch;
|
||||
|
||||
render(
|
||||
|
|
@ -104,7 +108,7 @@ async function reachBankStep(connections: BankingConnection[]) {
|
|||
|
||||
await waitFor(() =>
|
||||
expect(
|
||||
screen.getByRole('button', { name: /BBVA/ }),
|
||||
screen.getByPlaceholderText('Search banks...'),
|
||||
).toBeInTheDocument(),
|
||||
);
|
||||
}
|
||||
|
|
@ -192,4 +196,34 @@ describe('ConnectAccountDialog', () => {
|
|||
).not.toBeInTheDocument();
|
||||
expect(screen.getByRole('button', { name: 'Connect' })).toBeEnabled();
|
||||
});
|
||||
|
||||
it('shows Wise once, preferring the native integration over the aggregator', async () => {
|
||||
await reachBankStep(
|
||||
[],
|
||||
[institution('Abanca'), institution('Wise', 'aggregator-logo')],
|
||||
);
|
||||
|
||||
expect(screen.getAllByText('Wise')).toHaveLength(1);
|
||||
// The surviving entry is the native provider, with its own logo —
|
||||
// not the aggregator's.
|
||||
const wiseButton = screen.getByRole('button', { name: 'Wise' });
|
||||
expect(wiseButton.querySelector('img')).toHaveAttribute(
|
||||
'src',
|
||||
'https://enablebanking.com/brands/BE/Wise/',
|
||||
);
|
||||
});
|
||||
|
||||
it('does not duplicate banks when the search is filtered repeatedly', async () => {
|
||||
await reachBankStep(
|
||||
[],
|
||||
[institution('Abanca'), institution('Wise', 'aggregator-logo')],
|
||||
);
|
||||
const search = screen.getByPlaceholderText('Search banks...');
|
||||
|
||||
for (const query of ['wise', '', 'wise', '']) {
|
||||
fireEvent.change(search, { target: { value: query } });
|
||||
}
|
||||
|
||||
expect(screen.getAllByText('Wise')).toHaveLength(1);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -149,41 +149,46 @@ export function ConnectAccountDialog({
|
|||
/>
|
||||
|
||||
<div className="max-h-[300px] space-y-1 overflow-y-auto">
|
||||
{filteredInstitutions.map((institution) => {
|
||||
const isConnected = connectedBankNames.has(
|
||||
institution.name,
|
||||
);
|
||||
{filteredInstitutions.map(
|
||||
(institution, index) => {
|
||||
const isConnected =
|
||||
connectedBankNames.has(
|
||||
institution.name,
|
||||
);
|
||||
|
||||
return (
|
||||
<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>
|
||||
{isConnected && (
|
||||
<Badge
|
||||
variant="secondary"
|
||||
className="ml-auto"
|
||||
>
|
||||
{__('Already connected')}
|
||||
</Badge>
|
||||
)}
|
||||
</button>
|
||||
);
|
||||
})}
|
||||
return (
|
||||
<button
|
||||
key={`${institution.name}-${institution.country}-${index}`}
|
||||
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>
|
||||
{isConnected && (
|
||||
<Badge
|
||||
variant="secondary"
|
||||
className="ml-auto"
|
||||
>
|
||||
{__(
|
||||
'Already connected',
|
||||
)}
|
||||
</Badge>
|
||||
)}
|
||||
</button>
|
||||
);
|
||||
},
|
||||
)}
|
||||
{filteredInstitutions.length === 0 && (
|
||||
<p className="py-4 text-center text-sm text-muted-foreground">
|
||||
{__('No banks found.')}
|
||||
|
|
|
|||
|
|
@ -131,9 +131,9 @@ export function ConnectAccountInline({
|
|||
/>
|
||||
|
||||
<div className="max-h-[300px] space-y-1 overflow-y-auto rounded-lg border p-1">
|
||||
{filteredInstitutions.map((institution) => (
|
||||
{filteredInstitutions.map((institution, index) => (
|
||||
<button
|
||||
key={institution.name}
|
||||
key={`${institution.name}-${institution.country}-${index}`}
|
||||
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
|
||||
|
|
|
|||
|
|
@ -152,9 +152,19 @@ export function useConnectFlow(connections: BankingConnection[]) {
|
|||
),
|
||||
).map((p) => p.institution);
|
||||
|
||||
const allInstitutions = [...extraInstitutions, ...data].sort(
|
||||
(a, b) => a.name.localeCompare(b.name),
|
||||
// A provider we integrate natively (e.g. Wise) must surface only
|
||||
// through its own entry, never the bank-aggregator's duplicate.
|
||||
const nativeNames = new Set(
|
||||
CONNECT_PROVIDERS.map((p) => p.institution.name),
|
||||
);
|
||||
const fromProvider = (
|
||||
data as EnableBankingInstitution[]
|
||||
).filter((institution) => !nativeNames.has(institution.name));
|
||||
|
||||
const allInstitutions = [
|
||||
...extraInstitutions,
|
||||
...fromProvider,
|
||||
].sort((a, b) => a.name.localeCompare(b.name));
|
||||
|
||||
setInstitutions(allInstitutions);
|
||||
setFilteredInstitutions(allInstitutions);
|
||||
|
|
|
|||
|
|
@ -184,7 +184,7 @@ export const CONNECT_PROVIDERS: ConnectProvider[] = [
|
|||
institution: {
|
||||
name: 'Wise',
|
||||
country: 'ALL',
|
||||
logo: '/images/banks/logos/wise.png',
|
||||
logo: 'https://enablebanking.com/brands/BE/Wise/',
|
||||
maximum_consent_validity: null,
|
||||
},
|
||||
endpoint: '/open-banking/wise/connect',
|
||||
|
|
|
|||
Loading…
Reference in New Issue