fix(bank): keep other companies' accounts out of the EB account picker (#2141)
* fix(bank): keep other companies' accounts out of the EB account picker At one-session banks (SEB) a single BankID consent returns every account the signer can see across all their companies, so a reconnect from company A carries company B's accounts. PR #2116 made those arrive unchecked, labelled and unmirrored; they were still listed in company A's picker and in the connection's account list in settings, which read as "the wrong company's data in my books" (user report, Deepgrid group). - New lib/claimed-accounts.ts: partitionByClaim() splits a connection's accounts on claimed_by_company_id; describeClaimedElsewhere() renders the one-line Swedish summary. Unit-tested, including the legacy double-claim (no flag, stays own) and carried-deselection cases. - AccountPickerDialog: main list, "Markera alla" and the "x av y valda" counter cover own accounts only. Claimed accounts sit behind a collapsed "N konton synkas i <bolag>" disclosure (still tickable: a claim is a strong hint, not proof of ownership). Row markup extracted into renderAccountRow so both lists share it. - BankConnectionStatus: foreign rows dropped from the details list and the "x av y konton synkas" count; one muted summary line instead. No data or callback changes; brand-new never-claimed accounts still list unchecked, since Enable Banking's account resource carries no owner org number. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GY5eTAUFZDoCfbdWoERrsi * fix(bank): close the review and skeptic findings on the claimed-accounts picker Review (CodeRabbit): describeClaimedElsewhere decided "same claimant" on the display name; two companies can share a name. Now keyed on claimed_by_company_id as well, with a test. Skeptics (correctness + regression): - The empty-own-list message asserted "synkas redan i andra bolag" even for a consent with no accounts at all (failed connect, nothing ticked at the bank). Now only when claimed accounts exist; otherwise a plain "inga konton" message. - "Markera alla" stayed enabled but inert with zero own accounts: allSelected is now vacuously true there, so the button disables. - A claimed account ticked inside the disclosure kept counting after the disclosure was collapsed: the disclosure line now names the ticked count so the "x av y valda" counter never exceeds what is visible. - The pending_selection row in settings still counted foreign accounts ("3 konton tillgängliga" beside a picker saying none): now own accounts, with a dedicated line when everything is claimed elsewhere. - Claim flags did not survive an in-place renewal (accountsMetadata is rebuilt without them and the guard skipped seen-on-row accounts), so the sibling's accounts returned to the main list unlabeled on the next reconnect. The callback now re-derives the label from a fresh lookup for accounts that stay disabled here; released claims clear themselves. Two callback tests. Skeptic (compliance) hardening: - partitionByClaim requires enabled === false alongside the flag, so a flagged-but-enabled row (any future writer) can never hide a syncing account. - The sibling company's name is data-ph-masked on the settings summary line and the disclosure line, matching the row label. Declined: CodeRabbit docstring-coverage warning (repo has no docstring requirement; the touched functions carry inline rationale comments). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GY5eTAUFZDoCfbdWoERrsi * fix(bank): keep a re-stamped sibling claim out of the cash-account mirror CodeRabbit round 2: the renewal branch that re-derives the claim label left the account out of guardDisabledUids, so the mirror below still ran upsertFromPsd2 for it. The first connect never mirrored that account (#2116), so a renewal would have planted the sibling's IBAN in this company's cash_accounts and burned a 19xx slot for an account that stays off. Now excluded like a fresh claim; the renewal test asserts only the own account is mirrored. Declined (recorded for the summary): compliance-swarm advisory that the sibling's account metadata reaches the client. Both companies belong to the same signed-in user and the data arrives under that user's own PSD2 consent; the ownership decision is already made server-side in the callback, the picker only renders it. Non-blocking, no cross-user data. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GY5eTAUFZDoCfbdWoERrsi --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5.1
parent
11017ef9ee
commit
158108ec01
@@ -22,7 +22,8 @@ import {
|
||||
SelectValue,
|
||||
} from '@/components/ui/select'
|
||||
import { useToast } from '@/components/ui/use-toast'
|
||||
import { Loader2 } from 'lucide-react'
|
||||
import { ChevronRight, Loader2 } from 'lucide-react'
|
||||
import { cn } from '@/lib/utils'
|
||||
import { createClient } from '@/lib/supabase/client'
|
||||
import { useCompany } from '@/contexts/CompanyContext'
|
||||
import {
|
||||
@@ -34,6 +35,7 @@ import {
|
||||
resolveFiscalYearStart,
|
||||
resolveGapFillStart,
|
||||
} from '../lib/date-suggestions'
|
||||
import { describeClaimedElsewhere, partitionByClaim } from '../lib/claimed-accounts'
|
||||
import type { StoredAccount } from '../types'
|
||||
import {
|
||||
BankSyncProgressDialog,
|
||||
@@ -90,6 +92,15 @@ export function AccountPickerDialog({
|
||||
|
||||
const [selected, setSelected] = useState<Set<string>>(new Set())
|
||||
const [isSaving, setIsSaving] = useState(false)
|
||||
// Accounts the callback found booked by another of the user's companies
|
||||
// (one SEB consent covers every company the signer represents). They are
|
||||
// kept out of the main list so the picker shows THIS company's accounts,
|
||||
// and live behind a collapsed disclosure: still reachable, never pre-checked.
|
||||
const { own: ownAccounts, claimedElsewhere } = useMemo(
|
||||
() => partitionByClaim(accounts),
|
||||
[accounts],
|
||||
)
|
||||
const [claimedOpen, setClaimedOpen] = useState(false)
|
||||
// Server-side save rejection (validation / ledger conflict). Shown inline in
|
||||
// the dialog: a rejected save persisted nothing and started no sync, so the
|
||||
// user must see why and be able to correct the picks.
|
||||
@@ -166,6 +177,7 @@ export function AccountPickerDialog({
|
||||
)
|
||||
setSelected(initial)
|
||||
setSaveError(null)
|
||||
setClaimedOpen(false)
|
||||
setLookbackMode('fast')
|
||||
setCustomSubMode('date')
|
||||
setCustomDate('')
|
||||
@@ -300,13 +312,22 @@ export function AccountPickerDialog({
|
||||
return () => { cancelled = true }
|
||||
}, [open, isInitialSelection, company?.id, connectionId, supabase, accounts])
|
||||
|
||||
const allSelected = accounts.length > 0 && selected.size === accounts.length
|
||||
// "Alla" means this company's accounts: a claimed account is only ever
|
||||
// selected by an explicit tick inside the disclosure. Vacuously true when
|
||||
// there are no own accounts, so "Markera alla" is disabled instead of
|
||||
// being a live button that does nothing.
|
||||
const allSelected = ownAccounts.every((a) => selected.has(a.uid))
|
||||
const noneSelected = selected.size === 0
|
||||
// Claimed accounts the user deliberately ticked inside the disclosure. They
|
||||
// count in "x av y valda" and are named on the disclosure line even while
|
||||
// it is collapsed, so the counter never exceeds what the user can see.
|
||||
const selectedClaimedCount = claimedElsewhere.filter((a) => selected.has(a.uid)).length
|
||||
const selectableCount = ownAccounts.length + selectedClaimedCount
|
||||
|
||||
const sortedAccounts = useMemo(
|
||||
() => [...accounts].sort((a, b) => (a.name || a.iban || '').localeCompare(b.name || b.iban || '')),
|
||||
[accounts]
|
||||
)
|
||||
const byDisplayName = (a: StoredAccount, b: StoredAccount) =>
|
||||
(a.name || a.iban || '').localeCompare(b.name || b.iban || '')
|
||||
const sortedAccounts = useMemo(() => [...ownAccounts].sort(byDisplayName), [ownAccounts])
|
||||
const sortedClaimed = useMemo(() => [...claimedElsewhere].sort(byDisplayName), [claimedElsewhere])
|
||||
|
||||
// Detect cases where the user routed two enabled accounts with different
|
||||
// currencies to the same BAS account, usually a mistake, but allowed.
|
||||
@@ -334,7 +355,12 @@ export function AccountPickerDialog({
|
||||
}
|
||||
|
||||
function selectAll() {
|
||||
setSelected(new Set(accounts.map(a => a.uid)))
|
||||
// Own accounts only; a claimed account already ticked stays ticked.
|
||||
setSelected(prev => {
|
||||
const next = new Set(prev)
|
||||
for (const a of ownAccounts) next.add(a.uid)
|
||||
return next
|
||||
})
|
||||
}
|
||||
|
||||
function selectNone() {
|
||||
@@ -539,6 +565,102 @@ export function AccountPickerDialog({
|
||||
: lookback.days > daysBetween(gapFill.latestImportedDate)),
|
||||
)
|
||||
|
||||
// One account row; shared by the main list and the claimed-elsewhere
|
||||
// disclosure so the two can never drift apart.
|
||||
function renderAccountRow(account: StoredAccount) {
|
||||
const isChecked = selected.has(account.uid)
|
||||
const ledger = ledgerByUid[account.uid] || ''
|
||||
const ledgerExistsInChart = chartAccounts.some(c => c.account_number === ledger)
|
||||
return (
|
||||
<div
|
||||
key={account.uid}
|
||||
className="flex items-center gap-3 p-3 hover:bg-muted/50"
|
||||
>
|
||||
{/* Toggle area: label + Checkbox (a Radix Checkbox renders as
|
||||
its own <button role="checkbox">, so wrapping it in another
|
||||
<button> would be nested interactive elements: invalid HTML
|
||||
that browsers silently flatten and breaks event routing). */}
|
||||
<label className="flex flex-1 min-w-0 cursor-pointer items-center gap-3">
|
||||
<Checkbox
|
||||
checked={isChecked}
|
||||
onCheckedChange={() => toggle(account.uid)}
|
||||
disabled={isSaving}
|
||||
/>
|
||||
<div className="flex-1 min-w-0">
|
||||
<p className="text-sm font-medium truncate">
|
||||
{account.name || account.iban || 'Okänt konto'}
|
||||
<span className="ml-2 text-xs font-normal text-muted-foreground">
|
||||
{account.currency}
|
||||
</span>
|
||||
</p>
|
||||
{account.iban && (
|
||||
<p className="text-xs text-muted-foreground tabular-nums">
|
||||
{account.iban.replace(/(.{4})/g, '$1 ').trim()}
|
||||
</p>
|
||||
)}
|
||||
{/* The callback found this IBAN already booked by another
|
||||
of the user's companies (one consent can cover several
|
||||
companies' accounts at e.g. SEB). Unchecked by default;
|
||||
naming the claimant is what stops a reflexive
|
||||
select-all from booking it here too. */}
|
||||
{account.claimed_by_company_id && (
|
||||
<p className="text-xs text-muted-foreground">
|
||||
Synkas redan i{' '}
|
||||
<span data-ph-mask="">
|
||||
{account.claimed_by_company_name || 'ett annat bolag'}
|
||||
</span>
|
||||
</p>
|
||||
)}
|
||||
{/* Carried deselection: the user said "Synkas ej" to this
|
||||
IBAN on another connection. An unexplained unchecked
|
||||
box reads as a glitch; a silent one hides a sync gap. */}
|
||||
{!account.claimed_by_company_id && account.deselected_elsewhere && (
|
||||
<p className="text-xs text-muted-foreground">
|
||||
Tidigare bortvald: markera för att synka i detta bolag
|
||||
</p>
|
||||
)}
|
||||
</div>
|
||||
{account.balance !== undefined && (
|
||||
<p className="text-sm font-medium tabular-nums shrink-0">
|
||||
{new Intl.NumberFormat('sv-SE', {
|
||||
style: 'currency',
|
||||
currency: account.currency,
|
||||
}).format(account.balance)}
|
||||
</p>
|
||||
)}
|
||||
</label>
|
||||
{/* Ledger picker is a sibling of the label, not inside it:
|
||||
otherwise clicking the Select would also toggle the checkbox. */}
|
||||
<div className="w-44 shrink-0">
|
||||
{isChecked && (
|
||||
<Select
|
||||
value={ledger}
|
||||
onValueChange={(v) => setLedgerByUid(prev => ({ ...prev, [account.uid]: v }))}
|
||||
disabled={isSaving}
|
||||
>
|
||||
<SelectTrigger className="w-full">
|
||||
<SelectValue placeholder="Välj konto…" />
|
||||
</SelectTrigger>
|
||||
<SelectContent>
|
||||
{/* Surface a non-existent default so the user can see/correct it. */}
|
||||
{ledger && !ledgerExistsInChart && (
|
||||
<SelectItem value={ledger} disabled>
|
||||
{ledger}: finns ej i kontoplan
|
||||
</SelectItem>
|
||||
)}
|
||||
{chartAccounts.map(acc => (
|
||||
<SelectItem key={acc.account_number} value={acc.account_number}>
|
||||
<span className="tabular-nums">{acc.account_number}</span> {acc.account_name}
|
||||
</SelectItem>
|
||||
))}
|
||||
</SelectContent>
|
||||
</Select>
|
||||
)}
|
||||
</div>
|
||||
</div>
|
||||
)
|
||||
}
|
||||
|
||||
return (
|
||||
<>
|
||||
<BankSyncProgressDialog
|
||||
@@ -821,7 +943,7 @@ export function AccountPickerDialog({
|
||||
|
||||
<div className="flex items-center justify-between text-xs text-muted-foreground">
|
||||
<span>
|
||||
{selected.size} av {accounts.length} valda
|
||||
{selected.size} av {selectableCount} valda
|
||||
</span>
|
||||
<div className="flex gap-2">
|
||||
<button
|
||||
@@ -868,101 +990,50 @@ export function AccountPickerDialog({
|
||||
)}
|
||||
|
||||
<div className="max-h-[50vh] overflow-y-auto rounded-lg border border-border divide-y divide-border">
|
||||
{sortedAccounts.map(account => {
|
||||
const isChecked = selected.has(account.uid)
|
||||
const ledger = ledgerByUid[account.uid] || ''
|
||||
const ledgerExistsInChart = chartAccounts.some(c => c.account_number === ledger)
|
||||
return (
|
||||
<div
|
||||
key={account.uid}
|
||||
className="flex items-center gap-3 p-3 hover:bg-muted/50"
|
||||
>
|
||||
{/* Toggle area: label + Checkbox (a Radix Checkbox renders as
|
||||
its own <button role="checkbox">, so wrapping it in another
|
||||
<button> would be nested interactive elements: invalid HTML
|
||||
that browsers silently flatten and breaks event routing). */}
|
||||
<label className="flex flex-1 min-w-0 cursor-pointer items-center gap-3">
|
||||
<Checkbox
|
||||
checked={isChecked}
|
||||
onCheckedChange={() => toggle(account.uid)}
|
||||
disabled={isSaving}
|
||||
/>
|
||||
<div className="flex-1 min-w-0">
|
||||
<p className="text-sm font-medium truncate">
|
||||
{account.name || account.iban || 'Okänt konto'}
|
||||
<span className="ml-2 text-xs font-normal text-muted-foreground">
|
||||
{account.currency}
|
||||
</span>
|
||||
</p>
|
||||
{account.iban && (
|
||||
<p className="text-xs text-muted-foreground tabular-nums">
|
||||
{account.iban.replace(/(.{4})/g, '$1 ').trim()}
|
||||
</p>
|
||||
)}
|
||||
{/* The callback found this IBAN already booked by another
|
||||
of the user's companies (one consent can cover several
|
||||
companies' accounts at e.g. SEB). Unchecked by default;
|
||||
naming the claimant is what stops a reflexive
|
||||
select-all from booking it here too. */}
|
||||
{account.claimed_by_company_id && (
|
||||
<p className="text-xs text-muted-foreground">
|
||||
Synkas redan i{' '}
|
||||
<span data-ph-mask="">
|
||||
{account.claimed_by_company_name || 'ett annat bolag'}
|
||||
</span>
|
||||
</p>
|
||||
)}
|
||||
{/* Carried deselection: the user said "Synkas ej" to this
|
||||
IBAN on another connection. An unexplained unchecked
|
||||
box reads as a glitch; a silent one hides a sync gap. */}
|
||||
{!account.claimed_by_company_id && account.deselected_elsewhere && (
|
||||
<p className="text-xs text-muted-foreground">
|
||||
Tidigare bortvald: markera för att synka i detta bolag
|
||||
</p>
|
||||
)}
|
||||
</div>
|
||||
{account.balance !== undefined && (
|
||||
<p className="text-sm font-medium tabular-nums shrink-0">
|
||||
{new Intl.NumberFormat('sv-SE', {
|
||||
style: 'currency',
|
||||
currency: account.currency,
|
||||
}).format(account.balance)}
|
||||
</p>
|
||||
)}
|
||||
</label>
|
||||
{/* Ledger picker is a sibling of the label, not inside it:
|
||||
otherwise clicking the Select would also toggle the checkbox. */}
|
||||
<div className="w-44 shrink-0">
|
||||
{isChecked && (
|
||||
<Select
|
||||
value={ledger}
|
||||
onValueChange={(v) => setLedgerByUid(prev => ({ ...prev, [account.uid]: v }))}
|
||||
disabled={isSaving}
|
||||
>
|
||||
<SelectTrigger className="w-full">
|
||||
<SelectValue placeholder="Välj konto…" />
|
||||
</SelectTrigger>
|
||||
<SelectContent>
|
||||
{/* Surface a non-existent default so the user can see/correct it. */}
|
||||
{ledger && !ledgerExistsInChart && (
|
||||
<SelectItem value={ledger} disabled>
|
||||
{ledger}: finns ej i kontoplan
|
||||
</SelectItem>
|
||||
)}
|
||||
{chartAccounts.map(acc => (
|
||||
<SelectItem key={acc.account_number} value={acc.account_number}>
|
||||
<span className="tabular-nums">{acc.account_number}</span> {acc.account_name}
|
||||
</SelectItem>
|
||||
))}
|
||||
</SelectContent>
|
||||
</Select>
|
||||
)}
|
||||
</div>
|
||||
</div>
|
||||
)
|
||||
})}
|
||||
{sortedAccounts.map(renderAccountRow)}
|
||||
{/* Two distinct empty states: every account in the consent belongs
|
||||
to another company (the text below says so and points at the
|
||||
disclosure), or the consent simply carries no accounts (a failed
|
||||
connect, or nothing ticked at the bank), where a claim would be
|
||||
a false statement. */}
|
||||
{sortedAccounts.length === 0 && (
|
||||
<p className="p-3 text-xs text-muted-foreground">
|
||||
{claimedElsewhere.length > 0
|
||||
? 'Inga konton att välja: alla konton i den här bankkopplingen synkas redan i andra bolag.'
|
||||
: 'Bankkopplingen innehåller inga konton. Förnya anslutningen och välj konton hos banken.'}
|
||||
</p>
|
||||
)}
|
||||
</div>
|
||||
|
||||
{/* Accounts another of the user's companies already books (one SEB
|
||||
consent covers every company the signer represents). Collapsed by
|
||||
default so this company's picker shows this company's accounts;
|
||||
expandable because a claim is a strong hint, not proof, and an
|
||||
account that belongs here must stay reachable. */}
|
||||
{claimedElsewhere.length > 0 && (
|
||||
<div>
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => setClaimedOpen((v) => !v)}
|
||||
aria-expanded={claimedOpen}
|
||||
className="flex min-h-9 items-center gap-1 text-xs text-muted-foreground transition-colors duration-150 hover:text-foreground"
|
||||
>
|
||||
<ChevronRight
|
||||
className={cn('h-3.5 w-3.5 transition-transform duration-150', claimedOpen && 'rotate-90')}
|
||||
/>
|
||||
<span className="tabular-nums" data-ph-mask="">
|
||||
{describeClaimedElsewhere(claimedElsewhere)}
|
||||
{selectedClaimedCount > 0 && ` (${selectedClaimedCount} valt här)`}
|
||||
</span>
|
||||
</button>
|
||||
{claimedOpen && (
|
||||
<div className="max-h-[30vh] overflow-y-auto rounded-lg border border-border divide-y divide-border">
|
||||
{sortedClaimed.map(renderAccountRow)}
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
)}
|
||||
|
||||
<DialogFooter>
|
||||
<Button
|
||||
type="button"
|
||||
|
||||
@@ -15,6 +15,7 @@ import {
|
||||
import { cn, formatDate } from '@/lib/utils'
|
||||
import { ChevronRight, Loader2, MoreHorizontal } from 'lucide-react'
|
||||
import { getConnectionUiState } from '../lib/connection-state'
|
||||
import { describeClaimedElsewhere, partitionByClaim } from '../lib/claimed-accounts'
|
||||
import type { BankConnection } from '@/types'
|
||||
|
||||
interface BankConnectionStatusProps {
|
||||
@@ -56,8 +57,14 @@ export function BankConnectionStatus({
|
||||
balance?: number
|
||||
balance_updated_at?: string
|
||||
enabled?: boolean
|
||||
claimed_by_company_id?: string
|
||||
claimed_by_company_name?: string
|
||||
}>) || []
|
||||
const enabledCount = accounts.filter((a) => a.enabled !== false).length
|
||||
// Accounts another of the user's companies books (one SEB consent covers
|
||||
// every company the signer represents) are not this company's accounts:
|
||||
// they are left out of the list and the count, and summarised in one line.
|
||||
const { own: ownAccounts, claimedElsewhere } = partitionByClaim(accounts)
|
||||
const enabledCount = ownAccounts.filter((a) => a.enabled !== false).length
|
||||
|
||||
function formatBalanceAge(updatedAt: string): string {
|
||||
const hoursAgo = Math.floor((now - new Date(updatedAt).getTime()) / (1000 * 60 * 60))
|
||||
@@ -182,7 +189,11 @@ export function BankConnectionStatus({
|
||||
)}
|
||||
{uiState === 'pending_selection' ? (
|
||||
<span className="text-xs text-muted-foreground">
|
||||
{accounts.length} konton tillgängliga: inga transaktioner synkas ännu
|
||||
{ownAccounts.length > 0
|
||||
? `${ownAccounts.length} konton tillgängliga: inga transaktioner synkas ännu`
|
||||
: claimedElsewhere.length > 0
|
||||
? 'Alla konton i kopplingen synkas redan i andra bolag'
|
||||
: 'Inga konton i kopplingen: inga transaktioner synkas ännu'}
|
||||
</span>
|
||||
) : (
|
||||
<>
|
||||
@@ -290,7 +301,7 @@ export function BankConnectionStatus({
|
||||
className={cn('h-3.5 w-3.5 transition-transform duration-150', detailsOpen && 'rotate-90')}
|
||||
/>
|
||||
<span className="tabular-nums">
|
||||
{enabledCount} av {accounts.length} konton synkas
|
||||
{enabledCount} av {ownAccounts.length} konton synkas
|
||||
</span>
|
||||
</button>
|
||||
|
||||
@@ -331,7 +342,7 @@ export function BankConnectionStatus({
|
||||
)
|
||||
})()}
|
||||
|
||||
{accounts.map((account) => {
|
||||
{ownAccounts.map((account) => {
|
||||
const isDisabled = account.enabled === false
|
||||
return (
|
||||
<div
|
||||
@@ -372,6 +383,15 @@ export function BankConnectionStatus({
|
||||
</div>
|
||||
)
|
||||
})}
|
||||
{/* Sibling companies' accounts carried by this consent: one
|
||||
muted line, never rows. Moving one here is done in the
|
||||
account picker ("Hantera konton"), where it is a deliberate
|
||||
tick inside a disclosure. */}
|
||||
{claimedElsewhere.length > 0 && (
|
||||
<div className="py-2 text-xs text-muted-foreground tabular-nums" data-ph-mask="">
|
||||
{describeClaimedElsewhere(claimedElsewhere)}
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
|
||||
Reference in New Issue
Block a user