diff --git a/packages/desktop-client/src/components/modals/SelectLinkedAccountsModal.test.ts b/packages/desktop-client/src/components/modals/SelectLinkedAccountsModal.test.ts new file mode 100644 index 0000000000..d8b5fc3f4a --- /dev/null +++ b/packages/desktop-client/src/components/modals/SelectLinkedAccountsModal.test.ts @@ -0,0 +1,99 @@ +import type { + AccountEntity, + SyncServerSimpleFinAccount, +} from '@actual-app/core/types/models'; +import { describe, expect, it } from 'vitest'; + +import { computeInitialLinkState } from './SelectLinkedAccountsModal'; + +function makeLocalAccount( + overrides: Partial & { id: string }, +): AccountEntity { + return { + name: overrides.id, + offbudget: 0, + closed: 0, + sort_order: 0, + last_reconciled: null, + tombstone: 0, + account_id: null, + bank: null, + bankName: null, + bankId: null, + mask: null, + official_name: null, + balance_current: null, + balance_available: null, + balance_limit: null, + account_sync_source: null, + last_sync: null, + bank_sync_status: null, + ...overrides, + }; +} + +function makeExternalAccount(accountId: string): SyncServerSimpleFinAccount { + return { account_id: accountId, name: accountId, balance: 0 }; +} + +describe('computeInitialLinkState', () => { + it('preselects the upgrading account when there is exactly one unmatched external account', () => { + const localAccounts = [makeLocalAccount({ id: 'local-1' })]; + const externalAccounts = [makeExternalAccount('ext-1')]; + + const { initiallyChosenAccounts } = computeInitialLinkState( + localAccounts, + externalAccounts, + 'local-1', + ); + + expect(initiallyChosenAccounts['ext-1']).toBe('local-1'); + }); + + it('does not preselect when there are multiple unmatched external accounts (regression for #8518)', () => { + const localAccounts = [makeLocalAccount({ id: 'local-1' })]; + const externalAccounts = [ + makeExternalAccount('ext-1'), + makeExternalAccount('ext-2'), + ]; + + const { initiallyChosenAccounts } = computeInitialLinkState( + localAccounts, + externalAccounts, + 'local-1', + ); + + expect(Object.values(initiallyChosenAccounts)).not.toContain('local-1'); + }); + + it('does not preselect when upgradingAccountId is not set', () => { + const localAccounts = [makeLocalAccount({ id: 'local-1' })]; + const externalAccounts = [makeExternalAccount('ext-1')]; + + const { initiallyChosenAccounts } = computeInitialLinkState( + localAccounts, + externalAccounts, + undefined, + ); + + expect(Object.values(initiallyChosenAccounts)).not.toContain('local-1'); + }); + + it('leaves an already-linked upgrading account untouched', () => { + const localAccounts = [ + makeLocalAccount({ id: 'local-1', account_id: 'ext-1' }), + ]; + const externalAccounts = [ + makeExternalAccount('ext-1'), + makeExternalAccount('ext-2'), + ]; + + const { initiallyChosenAccounts } = computeInitialLinkState( + localAccounts, + externalAccounts, + 'local-1', + ); + + expect(initiallyChosenAccounts).toEqual({ 'ext-1': 'local-1' }); + }); +}); diff --git a/packages/desktop-client/src/components/modals/SelectLinkedAccountsModal.tsx b/packages/desktop-client/src/components/modals/SelectLinkedAccountsModal.tsx index 8807c53e0e..2183e9c1f9 100644 --- a/packages/desktop-client/src/components/modals/SelectLinkedAccountsModal.tsx +++ b/packages/desktop-client/src/components/modals/SelectLinkedAccountsModal.tsx @@ -73,6 +73,57 @@ function isNewAccountOption( ); } +/** + * Computes which local accounts should be pre-linked to which external + * accounts when the modal first opens. When `upgradingAccountId` is set and + * there's exactly one external account that doesn't already match a known + * local account, it's unambiguous which row it belongs to, so it's + * preselected. If there's more than one unmatched external account, guessing + * would risk claiming the wrong row (and hiding the account from every other + * row's picker), so it's left for the user to pick manually. + */ +export function computeInitialLinkState( + localAccounts: AccountEntity[], + externalAccounts: ExternalAccount[], + upgradingAccountId: string | undefined, +): { + initialDraftLinkAccounts: Map; + initiallyChosenAccounts: Record; +} { + const externalAccountIds = new Set(externalAccounts.map(a => a.account_id)); + const initialDraftLinkAccounts = new Map(); + for (const acc of localAccounts) { + if (acc.account_id && externalAccountIds.has(acc.account_id)) { + initialDraftLinkAccounts.set(acc.account_id, 'linking'); + } + } + + const initiallyChosenAccounts: Record = Object.fromEntries( + localAccounts + .filter(acc => acc.account_id) + .map(acc => [acc.account_id, acc.id]), + ); + + const unmatchedExternalAccounts = externalAccounts.filter( + account => initiallyChosenAccounts[account.account_id] == null, + ); + + if ( + upgradingAccountId && + unmatchedExternalAccounts.length === 1 && + !Object.values(initiallyChosenAccounts).includes(upgradingAccountId) + ) { + initiallyChosenAccounts[unmatchedExternalAccounts[0].account_id] = + upgradingAccountId; + initialDraftLinkAccounts.set( + unmatchedExternalAccounts[0].account_id, + 'linking', + ); + } + + return { initialDraftLinkAccounts, initiallyChosenAccounts }; +} + export type SelectLinkedAccountsModalProps = | { requisitionId: string; @@ -161,48 +212,19 @@ export function SelectLinkedAccountsModal({ const dispatch = useDispatch(); const { data: allAccounts = [] } = useAccounts(); const localAccounts = allAccounts.filter(a => a.closed === 0); - const { initialDraftLinkAccounts, initiallyChosenAccounts } = useMemo(() => { - const externalAccountIds = new Set( - externalAccounts?.map(a => a.account_id) || [], - ); - const initialDraftLinkAccounts = new Map(); - for (const acc of localAccounts) { - if (acc.account_id && externalAccountIds.has(acc.account_id)) { - initialDraftLinkAccounts.set(acc.account_id, 'linking'); - } - } - - const initiallyChosenAccounts = Object.fromEntries( - localAccounts - .filter(acc => acc.account_id) - .map(acc => [acc.account_id, acc.id]), - ); - - const preselectedExternalAccount = - propsWithSortedExternalAccounts.externalAccounts.find( - account => initiallyChosenAccounts[account.account_id] == null, - ); - - if ( - upgradingAccountId && - preselectedExternalAccount && - !Object.values(initiallyChosenAccounts).includes(upgradingAccountId) - ) { - initiallyChosenAccounts[preselectedExternalAccount.account_id] = - upgradingAccountId; - initialDraftLinkAccounts.set( - preselectedExternalAccount.account_id, - 'linking', - ); - } - - return { initialDraftLinkAccounts, initiallyChosenAccounts }; - }, [ - localAccounts, - externalAccounts, - propsWithSortedExternalAccounts.externalAccounts, - upgradingAccountId, - ]); + const { initialDraftLinkAccounts, initiallyChosenAccounts } = useMemo( + () => + computeInitialLinkState( + localAccounts, + propsWithSortedExternalAccounts.externalAccounts, + upgradingAccountId, + ), + [ + localAccounts, + propsWithSortedExternalAccounts.externalAccounts, + upgradingAccountId, + ], + ); const [draftLinkAccounts, setDraftLinkAccounts] = useState< Map diff --git a/upcoming-release-notes/fix-relink-account-picker.md b/upcoming-release-notes/fix-relink-account-picker.md new file mode 100644 index 0000000000..aa56f6a57c --- /dev/null +++ b/upcoming-release-notes/fix-relink-account-picker.md @@ -0,0 +1,6 @@ +--- +category: Bugfixes +authors: [youngcw] +--- + +Fix relinking a bank account that had been unlinked.