diff --git a/DECISIONS.md b/DECISIONS.md index 34f42ba9..b66b1e4d 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -1092,3 +1092,4 @@ One line per decision: `[YYYY-MM-DD] : `. Appended by agents and [2026-08-19] Hem build-assistant hero downgraded to the quiet-sentence pattern (AgentPromo, matches SkatteverketPromoCard; founder direction 2026-08-18 'redesign first, maybe remove later'): dismissal is per-company localStorage (erp_agent_promo_dismissed:) like the SKV promo, gate and hasAi/billing routing unchanged. [2026-08-19] Banking settings UI state derives from a pure helper (extensions/general/enable-banking/lib/connection-state.ts), not inline JSX conditions: sort precedence, the single page-level .attn sentence, and each row's one primary action must agree on which state a connection is in, and only a pure module can unit-test that. The same-bank connect intercept excludes 'pending' rows (an in-flight authorization is not a renewable connection) and the fresh-connect body sends force_new: true after the intercept so the parallel 409 server guard can distinguish deliberate second connections; 'pending' rows now render as a spinner row ("Väntar på banken") for their whole lifetime instead of only locking the connect button for 30 s, since an invisible in-flight row was the confusion. [2026-08-19] Bank reconnect supersede reuses status 'revoked' plus a new superseded_by column instead of a new status value, and re-points transactions.bank_connection_id to the superseding row: every existing filter, ledger-claim release, and cron skip already handles 'revoked' correctly (no CHECK-constraint migration on a live table), superseded_by disambiguates a supersede from a user disconnect, and re-pointing the feed rows (plain FK metadata, never journal tables) is what makes the picker's gap-fill probe and per-connection scoping survive a renewal. +[2026-08-19] /transactions source filter persists in per-company localStorage (v2 key) rather than user_preferences.ui_state, matching the FyPicker/JournalEntryList page-scoped filter idiom (design rule 9 reserves ui_state for split-button modes/nav), and a derived effectiveSourceFilter memo replaces the stale-filter reset-guard effect: the guard raced the async cashAccounts/skvRows/transactions loads on every mount and wiped the restored choice back to 'Alla källor'. diff --git a/app/(dashboard)/transactions/page.tsx b/app/(dashboard)/transactions/page.tsx index a912819e..85d20313 100644 --- a/app/(dashboard)/transactions/page.tsx +++ b/app/(dashboard)/transactions/page.tsx @@ -47,6 +47,12 @@ import type { PotentialVoucher, } from '@/components/transactions/transaction-types' import { SuggestionReviewList } from '@/components/transactions/SuggestionReviewList' +import { + isSourceFilter, + readStoredSourceFilter, + resolveEffectiveSourceFilter, + writeStoredSourceFilter, +} from '@/components/transactions/source-filter-storage' import type { SkattekontoBatchResult, SkattekontoBatchRowResult, @@ -124,8 +130,6 @@ const TemplatePicker = dynamic(() => import('@/components/transactions/TemplateP type InvoiceWithCustomer = Invoice & { customer?: Customer } type SupplierInvoiceWithSupplier = SupplierInvoice & { supplier?: Supplier } -const SOURCE_FILTER_STORAGE_KEY = 'Accounted:transaction-source-filter:v1' - // Page-local fiscal-year scope (FyPicker appends the company id). Deliberately // NOT the shared report scope (Accounted:fiscal-year:): a year picked on a // report page must never silently hide pending inbox rows here, and vice versa. @@ -137,18 +141,6 @@ const PERIOD_FILTER_STORAGE_PREFIX = 'Accounted:transactions-fy-scope:v1:' // /api/documents/counts. const UUID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i -// Validates a persisted value. Stale acct: entries (account removed or -// disabled) are caught later by the sourceItems stale-filter guard. -function isSourceFilter(value: string | null): value is SourceFilter { - return ( - value === 'all' || - value === 'bank' || - value === 'bank:other' || - value === 'skatteverket' || - (value?.startsWith('acct:') ?? false) - ) -} - // A skattekonto row qualifies for bulk booking when the outcome is fully // deterministic: a rule matched (booking_suggestion), it is not a likely // duplicate (match_suggestion), it is unbooked, and it has actually happened @@ -305,6 +297,7 @@ interface QuickReviewState { export default function TransactionsPage() { const { company } = useCompany() const companyId = company?.id ?? null + const searchParams = useSearchParams() const t = useTranslations('transactions') const [transactions, setTransactions] = useState([]) const [isLoading, setIsLoading] = useState(true) @@ -477,28 +470,27 @@ export default function TransactionsPage() { // ever visit the settings panel where the reconnect prompt lives. const [skvNeedsReconnect, setSkvNeedsReconnect] = useState(false) - // One browser-wide source filter, persisted (#1105) so the choice - // survives reloads. Defaults to 'all'. + // The user's WANTED source filter, persisted per company (#1105, per-company + // since v2). What the page actually filters by is the derived + // effectiveSourceFilter below, which falls back to 'all' while the wanted + // source's items are still loading or when the source went stale. const [sourceFilter, setSourceFilter] = useState('all') useEffect(() => { - try { - const stored = window.localStorage.getItem(SOURCE_FILTER_STORAGE_KEY) - if (isSourceFilter(stored)) setSourceFilter(stored) - } catch { - // localStorage may be unavailable. Keep the default in-memory state. - } - }, []) + if (!companyId) return + // A valid ?source= deep link overrides the remembered choice for this + // visit only: it is applied to state, never written to storage. + const urlSource = searchParams.get('source') + setSourceFilter(isSourceFilter(urlSource) ? urlSource : readStoredSourceFilter(companyId)) + }, [companyId, searchParams]) - const handleSourceFilterChange = useCallback((next: SourceFilter) => { - setSourceFilter(next) - - try { - window.localStorage.setItem(SOURCE_FILTER_STORAGE_KEY, next) - } catch { - // localStorage may be unavailable. The in-memory filter still works. - } - }, []) + const handleSourceFilterChange = useCallback( + (next: SourceFilter) => { + setSourceFilter(next) + if (companyId) writeStoredSourceFilter(companyId, next) + }, + [companyId], + ) // Period filter (rakenskapsar). FyPicker owns the persistence under the // page-local key. Quarter chips existed briefly (#1545) but were dropped: @@ -543,7 +535,6 @@ export default function TransactionsPage() { // Bank transaction being moved to another cash account (null = dialog closed). const [moveAccountTarget, setMoveAccountTarget] = useState(null) const supabase = useRealtimeSupabase() - const searchParams = useSearchParams() const highlightId = searchParams.get('highlight') // Tracks the last highlight target we acted on so re-renders don't re-trigger // the auto-open every time the user closes the categorize panel. @@ -642,59 +633,6 @@ export default function TransactionsPage() { [skvUnmatched, uncategorizedTransactions], ) - const inboxItems = useMemo(() => { - const items: InboxItem[] = [] - const query = searchTerm.trim().toLowerCase() - if (sourceFilter !== 'skatteverket') { - for (const tx of uncategorizedTransactions) { - // The refetch already narrows state server-side; this check makes the - // filter correct immediately on change, before the refetch lands. - if (!isWithinBounds(tx.date, periodBounds)) continue - if ( - sourceFilter.startsWith('acct:') && - tx.cash_account_id !== sourceFilter.slice('acct:'.length) - ) { - continue - } - if (sourceFilter === 'bank:other' && tx.cash_account_id != null) continue - if ( - query && - !tx.description?.toLowerCase().includes(query) && - !tx.date.includes(query) && - !String(tx.amount).includes(query) - ) { - continue - } - items.push({ source: 'bank', date: tx.date, data: tx }) - } - } - if (sourceFilter === 'all' || sourceFilter === 'skatteverket') { - // Inbox only shows SKV rows that need action (no verifikat yet). - for (const r of skvRows) { - // Exiting rows stay rendered for the exit animation, even if a - // refetch already gave them a journal_entry_id mid-window (see above). - if (r.journal_entry_id && !exitingIds.has(r.id)) continue - // SKV rows live client-side only, so the period filter applies here. - if (!isWithinBounds(r.transaktionsdatum, periodBounds)) continue - if ( - query && - !r.transaktionstext?.toLowerCase().includes(query) && - !r.transaktionsdatum.includes(query) && - !String(r.belopp_skatteverket).includes(query) - ) { - continue - } - items.push({ source: 'skatteverket', date: r.transaktionsdatum, data: r }) - } - } - return items.sort((a, b) => { - if (a.date !== b.date) return b.date.localeCompare(a.date) - // Same date → bank first so invoice-match cards lead. - if (a.source !== b.source) return a.source === 'bank' ? -1 : 1 - return 0 - }) - }, [exitingIds, periodBounds, searchTerm, skvRows, sourceFilter, uncategorizedTransactions]) - // History shows only the contiguous newest-first window: the older pending // rows merged in for the inbox would otherwise render as sparse, gap-ridden // months below the window and read as missing bookkeeping. Same-date rows at @@ -793,13 +731,76 @@ export default function TransactionsPage() { return items }, [cashAccounts, hasUnassignedBankRows, skvNeedsReconnect, skvRows.length, t, totalSourceBalance]) - // A narrowed filter can go stale (account disabled, skv rows drained, - // "övriga" bucket emptied): fall back to everything rather than filtering - // the inbox down to an invisible source. - useEffect(() => { - if (sourceFilter === 'all') return - if (!sourceItems.some((item) => item.id === sourceFilter)) setSourceFilter('all') - }, [sourceFilter, sourceItems]) + // What the page actually filters by. A narrowed filter whose source is not + // (yet) among the items, because cash accounts / skv rows / transactions + // are still loading, or because the source went stale (account disabled, + // skv rows drained, "övriga" bucket emptied), resolves to 'all' instead of + // filtering the inbox down to an invisible source. Deriving this (rather + // than resetting state, as the old guard effect did) removes the mount-time + // race that wiped the persisted choice before the async sources arrived. + const effectiveSourceFilter = useMemo( + () => + resolveEffectiveSourceFilter( + sourceFilter, + sourceItems.map((item) => item.id), + ), + [sourceFilter, sourceItems], + ) + + // Declared after sourceItems/effectiveSourceFilter because the inbox must + // filter by the EFFECTIVE source, never the raw wanted one. + const inboxItems = useMemo(() => { + const items: InboxItem[] = [] + const query = searchTerm.trim().toLowerCase() + if (effectiveSourceFilter !== 'skatteverket') { + for (const tx of uncategorizedTransactions) { + // The refetch already narrows state server-side; this check makes the + // filter correct immediately on change, before the refetch lands. + if (!isWithinBounds(tx.date, periodBounds)) continue + if ( + effectiveSourceFilter.startsWith('acct:') && + tx.cash_account_id !== effectiveSourceFilter.slice('acct:'.length) + ) { + continue + } + if (effectiveSourceFilter === 'bank:other' && tx.cash_account_id != null) continue + if ( + query && + !tx.description?.toLowerCase().includes(query) && + !tx.date.includes(query) && + !String(tx.amount).includes(query) + ) { + continue + } + items.push({ source: 'bank', date: tx.date, data: tx }) + } + } + if (effectiveSourceFilter === 'all' || effectiveSourceFilter === 'skatteverket') { + // Inbox only shows SKV rows that need action (no verifikat yet). + for (const r of skvRows) { + // Exiting rows stay rendered for the exit animation, even if a + // refetch already gave them a journal_entry_id mid-window (see above). + if (r.journal_entry_id && !exitingIds.has(r.id)) continue + // SKV rows live client-side only, so the period filter applies here. + if (!isWithinBounds(r.transaktionsdatum, periodBounds)) continue + if ( + query && + !r.transaktionstext?.toLowerCase().includes(query) && + !r.transaktionsdatum.includes(query) && + !String(r.belopp_skatteverket).includes(query) + ) { + continue + } + items.push({ source: 'skatteverket', date: r.transaktionsdatum, data: r }) + } + } + return items.sort((a, b) => { + if (a.date !== b.date) return b.date.localeCompare(a.date) + // Same date → bank first so invoice-match cards lead. + if (a.source !== b.source) return a.source === 'bank' ? -1 : 1 + return 0 + }) + }, [effectiveSourceFilter, exitingIds, periodBounds, searchTerm, skvRows, uncategorizedTransactions]) // Rows the bulkbar's "Markera alla" can select: the visible bank rows // (they feed the /api/transactions/* batch handlers) ... @@ -1409,6 +1410,21 @@ export default function TransactionsPage() { if (!tx) return handledHighlightRef.current = highlightId + // A deep link must land on a visible row: when the remembered source + // filter would hide the highlighted transaction, widen to 'all' in + // memory only (storage keeps the user's choice for the next visit). + // Checked against the WANTED filter, not the effective one: transactions + // can load before cash accounts, when the effective filter is still 'all' + // but the wanted one would hide the row the moment the accounts arrive. + // 'all' rather than acct: because it also covers rows with a null + // cash_account_id. + const hiddenByFilter = + sourceFilter === 'skatteverket' || + (sourceFilter.startsWith('acct:') && + tx.cash_account_id !== sourceFilter.slice('acct:'.length)) || + (sourceFilter === 'bank:other' && tx.cash_account_id != null) + if (hiddenByFilter) setSourceFilter('all') + // Defer the scroll until React has committed the list to the DOM. // Without rAF the data-tx-id node may not exist yet when this fires // immediately after fetchTransactions resolves. @@ -1420,7 +1436,7 @@ export default function TransactionsPage() { } }) }) - }, [highlightId, transactions]) + }, [highlightId, sourceFilter, transactions]) // Auto-fetch suggestions when transactions load useEffect(() => { @@ -3470,7 +3486,7 @@ export default function TransactionsPage() { setIsDialogOpen(true)} /> - {skvNeedsReconnect && sourceFilter === 'skatteverket' ? ( + {skvNeedsReconnect && effectiveSourceFilter === 'skatteverket' ? ( // Only when the user is actually looking at skattekonto rows: as a // permanent page-wide line it read as noise (feedback 2026-08-14). // The skattekonto page keeps its own reconnect line. @@ -3534,11 +3550,11 @@ export default function TransactionsPage() { /> {sourceItems.length > 1 && ( handleSourceFilterChange(id as SourceFilter)} triggerLabel={(() => { const active = - sourceItems.find((item) => item.id === sourceFilter) ?? sourceItems[0] + sourceItems.find((item) => item.id === effectiveSourceFilter) ?? sourceItems[0] return active.annotation ? `${active.label} · ${active.annotation}` : active.label })()} items={sourceItems} @@ -3572,13 +3588,13 @@ export default function TransactionsPage() { /> ) : mode === 'inbox' ? ( inboxItems.length === 0 ? ( - searchTerm || sourceFilter !== 'all' || periodBounds ? ( + searchTerm || effectiveSourceFilter !== 'all' || periodBounds ? ( = {}) { + const store = new Map(Object.entries(initial)) + const localStorage = { + getItem: (key: string) => store.get(key) ?? null, + setItem: (key: string, value: string) => { + store.set(key, value) + }, + removeItem: (key: string) => { + store.delete(key) + }, + } + vi.stubGlobal('window', { localStorage }) + return store +} + +afterEach(() => { + vi.unstubAllGlobals() +}) + +describe('isSourceFilter', () => { + it('accepts every member of the SourceFilter union', () => { + expect(isSourceFilter('all')).toBe(true) + expect(isSourceFilter('bank')).toBe(true) + expect(isSourceFilter('bank:other')).toBe(true) + expect(isSourceFilter('skatteverket')).toBe(true) + expect(isSourceFilter('acct:2f8d1c3a')).toBe(true) + }) + + it('rejects null and unknown values', () => { + expect(isSourceFilter(null)).toBe(false) + expect(isSourceFilter('')).toBe(false) + expect(isSourceFilter('everything')).toBe(false) + expect(isSourceFilter('account:123')).toBe(false) + expect(isSourceFilter('ALL')).toBe(false) + }) +}) + +describe('resolveEffectiveSourceFilter', () => { + it('keeps the wanted filter when its id is among the items', () => { + expect(resolveEffectiveSourceFilter('acct:a1', ['all', 'acct:a1', 'skatteverket'])).toBe( + 'acct:a1', + ) + expect(resolveEffectiveSourceFilter('skatteverket', ['all', 'skatteverket'])).toBe( + 'skatteverket', + ) + }) + + it('falls back to all when the wanted id is missing (items still loading or source stale)', () => { + expect(resolveEffectiveSourceFilter('acct:a1', ['all'])).toBe('all') + expect(resolveEffectiveSourceFilter('skatteverket', ['all', 'acct:a1'])).toBe('all') + expect(resolveEffectiveSourceFilter('bank:other', ['all', 'acct:a1'])).toBe('all') + }) + + it('passes all through regardless of items', () => { + expect(resolveEffectiveSourceFilter('all', [])).toBe('all') + expect(resolveEffectiveSourceFilter('all', ['all', 'acct:a1'])).toBe('all') + }) +}) + +describe('readStoredSourceFilter / writeStoredSourceFilter', () => { + it('round-trips a filter under the per-company key', () => { + stubLocalStorage() + writeStoredSourceFilter('company-a', 'acct:a1') + expect(readStoredSourceFilter('company-a')).toBe('acct:a1') + }) + + it('keeps companies separate: one company never reads another company value', () => { + stubLocalStorage() + writeStoredSourceFilter('company-a', 'acct:a1') + writeStoredSourceFilter('company-b', 'skatteverket') + expect(readStoredSourceFilter('company-a')).toBe('acct:a1') + expect(readStoredSourceFilter('company-b')).toBe('skatteverket') + expect(readStoredSourceFilter('company-c')).toBe('all') + }) + + it('returns all when the stored value is invalid', () => { + stubLocalStorage({ [SOURCE_FILTER_STORAGE_PREFIX + 'company-a']: 'garbage' }) + expect(readStoredSourceFilter('company-a')).toBe('all') + }) + + it('returns all when localStorage throws', () => { + vi.stubGlobal('window', { + localStorage: { + getItem: () => { + throw new Error('denied') + }, + setItem: () => { + throw new Error('denied') + }, + removeItem: () => { + throw new Error('denied') + }, + }, + }) + expect(readStoredSourceFilter('company-a')).toBe('all') + // Write must swallow the failure too. + expect(() => writeStoredSourceFilter('company-a', 'bank')).not.toThrow() + }) + + it('returns all when window is unavailable (SSR/node)', () => { + // No stub: vitest runs in the node environment, so window is undefined. + expect(readStoredSourceFilter('company-a')).toBe('all') + expect(() => writeStoredSourceFilter('company-a', 'bank')).not.toThrow() + }) + + it('removes the retired browser-wide v1 key on read and ignores its value', () => { + const store = stubLocalStorage({ [LEGACY_KEY]: 'skatteverket' }) + expect(readStoredSourceFilter('company-a')).toBe('all') + expect(store.has(LEGACY_KEY)).toBe(false) + }) +}) diff --git a/components/transactions/source-filter-storage.ts b/components/transactions/source-filter-storage.ts new file mode 100644 index 00000000..86d2e153 --- /dev/null +++ b/components/transactions/source-filter-storage.ts @@ -0,0 +1,58 @@ +import type { SourceFilter } from '@/components/transactions/transaction-types' + +// Per-company key (v2). The v1 key was browser-wide, so an acct: picked +// under one company leaked into every other company in the same browser; +// v2 scopes the memory per company like the FyPicker/JournalEntryList idiom. +export const SOURCE_FILTER_STORAGE_PREFIX = 'Accounted:transaction-source-filter:v2:' + +// The retired browser-wide key from #1105. Removed once on read so it does +// not linger in users' storage forever. +const LEGACY_SOURCE_FILTER_STORAGE_KEY = 'Accounted:transaction-source-filter:v1' + +// Validates a persisted or URL-provided value. Stale acct: entries +// (account removed or disabled) are handled by resolveEffectiveSourceFilter, +// which falls back to 'all' whenever the id is not among the picker items. +export function isSourceFilter(value: string | null): value is SourceFilter { + return ( + value === 'all' || + value === 'bank' || + value === 'bank:other' || + value === 'skatteverket' || + (value?.startsWith('acct:') ?? false) + ) +} + +export function readStoredSourceFilter(companyId: string): SourceFilter { + try { + // One-time cleanup of the legacy browser-wide key; v2 ignores its value. + window.localStorage.removeItem(LEGACY_SOURCE_FILTER_STORAGE_KEY) + const stored = window.localStorage.getItem(SOURCE_FILTER_STORAGE_PREFIX + companyId) + if (isSourceFilter(stored)) return stored + } catch { + // localStorage may be unavailable. Fall through to the default. + } + return 'all' +} + +export function writeStoredSourceFilter(companyId: string, next: SourceFilter): void { + try { + window.localStorage.setItem(SOURCE_FILTER_STORAGE_PREFIX + companyId, next) + } catch { + // localStorage may be unavailable. The in-memory filter still works. + } +} + +// The wanted filter (persisted choice or URL override) applies only while its +// source actually exists among the picker items. While async sources +// (cash accounts, skv rows, transactions) are still loading, or when a source +// went stale (account disabled, skattekonto drained), the page shows 'all'; +// the wanted value stays intact so the choice springs back when the source +// reappears. This derivation replaces the old reset-guard effect, which raced +// the loads and permanently reset the in-memory filter on every mount. +export function resolveEffectiveSourceFilter( + wanted: SourceFilter, + itemIds: readonly string[], +): SourceFilter { + if (wanted === 'all') return 'all' + return itemIds.includes(wanted) ? wanted : 'all' +}