fix(transactions): persist the source filter per company and stop the reset race (#1726)
The source filter was persisted under a browser-wide v1 key (#1105), but the stale-filter guard added in #1124 compared the restored value against sourceItems before cash accounts, skattekonto rows, and transactions had loaded, so every mount reset the in-memory filter back to 'Alla källor' while storage kept the old choice: restore-then-reset on every visit. - New pure helper components/transactions/source-filter-storage.ts: per-company v2 key, isSourceFilter moved out of the page, read/write helpers (read removes the retired v1 key once), and resolveEffectiveSourceFilter. - page.tsx keeps sourceFilter as the WANTED filter, restored per company (with a state-only ?source= URL override that is never written to storage); the guard effect is replaced by a derived effectiveSourceFilter memo used by every consumer, so a source that is still loading or went stale shows 'all' without destroying the choice. - ?highlight= deep links widen to 'all' in memory when the wanted filter would hide the highlighted row. - Unit tests for the helper; no i18n changes, no migrations. Co-authored-by: Jakob Wennberg <311770904+jakobwennberg-oss@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Jakob Wennberg
Claude Fable 5
parent
3de5dee553
commit
b77af371c4
@@ -0,0 +1,122 @@
|
||||
import { describe, it, expect, vi, afterEach } from 'vitest'
|
||||
import {
|
||||
SOURCE_FILTER_STORAGE_PREFIX,
|
||||
isSourceFilter,
|
||||
readStoredSourceFilter,
|
||||
writeStoredSourceFilter,
|
||||
resolveEffectiveSourceFilter,
|
||||
} from '@/components/transactions/source-filter-storage'
|
||||
|
||||
const LEGACY_KEY = 'Accounted:transaction-source-filter:v1'
|
||||
|
||||
function stubLocalStorage(initial: Record<string, string> = {}) {
|
||||
const store = new Map<string, string>(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)
|
||||
})
|
||||
})
|
||||
@@ -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:<id> 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:<id> 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'
|
||||
}
|
||||
Reference in New Issue
Block a user