diff --git a/.compliance/ropa.yaml b/.compliance/ropa.yaml index 6fef2aff..95368450 100644 --- a/.compliance/ropa.yaml +++ b/.compliance/ropa.yaml @@ -60,7 +60,12 @@ processing_activities: purpose: >- Identifiera en privatkund när företagets avtal, fakturering eller kundadministration kräver en entydig identitet. Personnumret får endast - sparas på kundtypen privatkund och exponeras endast maskerat i API och UI. + sparas på kundtypen privatkund. Samtliga listnings- och exportytor i API + och UI exponerar det endast maskerat (fyra sista siffrorna). Undantaget är + en enskild uppslagsyta (GET /api/customers/{id}/personal-number) som + returnerar hela värdet för en kund i taget, kräver skrivroll och loggas + med aktör men aldrig med värdet, så att den som registrerat ett + personnummer kan kontrollera vad som faktiskt sparats. lawful_basis: art_6_1_b special_category_basis: null controller: gnubok-tenant @@ -88,7 +93,15 @@ processing_activities: - masked_api_and_ui_output_last4_only - write_role_required - rls_company_scoped - - no_full_value_read_endpoint + # Replaces the previous `no_full_value_read_endpoint`. A single explicit + # drill-in now exists (GET /api/customers/{id}/personal-number): the + # value was otherwise write-only, so a user could store a personnummer + # and never verify what had been stored. Every listing surface still + # masks; the drill-in requires the write role, returns one customer at a + # time, and logs actor + customer id without the value. + - full_value_read_only_via_single_customer_drill_in + - full_value_read_requires_write_role + - full_value_read_logged_with_actor_never_value - id: agi.submit name: AGI inlämning till Skatteverket diff --git a/DECISIONS.md b/DECISIONS.md index 118fe166..c836255e 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -632,3 +632,15 @@ One line per decision: `[YYYY-MM-DD] : `. Appended by agents and [2026-07-27] Bolagsskatt add-back (#1051): sumPostedYearEndDispositions now adds back 78xx planenlig avskrivning alongside 88xx and 7533, and excludes fiscal_periods.closing_entry_id from its fetch. Shipping only this "Stage 1" half of the issue: it corrects the tax base and the periodiseringsfond 25 % cap with no migration and no displayed-figure change. The issue's other half (making /rapporter show bokslut entries by moving generateIncomeStatement to excludeFinalClosingEntry) is deliberately NOT done here: it duplicates the exclusion in the kpi_report_aggregates RPC (so it needs a migration plus a pg test), it changes displayed profit for every company that ran the bokslut flow, and it requires removing the add-back at four call sites, including the one that caused the original too-high-tax customer bug. The closing-entry exclusion is part of Stage 1 rather than a follow-up because closing verifikat do carry 78xx/88xx/7533 reversal lines on production, so without it the new add-back silently cancels itself once the year is closed. The issue's stated constraint that source_type='year_end' is load-bearing for the iXBRL RR/BR split is stale: build-input.ts and arsredovisning/build-data.ts already moved to excludeFinalClosingEntry. [2026-07-27] Documents bucket WORM (#1208): dropped the production-only `users_delete_own_documents` DELETE policy on storage.objects and pinned the invariant with a name-agnostic pg-real test, rather than adding a storage DELETE policy for authenticated users or building the orphan-cleanup script the issue asks for. The policy existed in no migration (dashboard drift, alongside `users_read_own_documents` / `users_upload_own_documents`, which production has INSTEAD of this repo's `documents_select_own` / `documents_insert_own`) and let any user delete, with a normal browser token, the storage bytes of documents linked to posted verifikat: rakenskapsinformation under BFL 7 kap 2 §. Neither deleteDocument()'s linked-check nor block_document_deletion() reaches that far; both protect the row, and the row survives pointing at nothing. Verified reproducible against a local replay of the full migration stream: with the policy present the uploader's own DELETE removes a legacy-layout object, with it dropped the DELETE matches zero rows. Dropping it breaks nothing because every in-app remove() on this bucket has run service-role since #1215. The two legacy read/insert policies are deliberately left alone: the Phase B backfill from 20260726092000 has not run, so dropping the legacy SELECT would make most existing documents unreadable. That is Phase C. The 326 orphan objects (49.5 MB) the issue also describes are NOT cleaned up here: irreversible deletion against 7-year-retention data is not worth 49 MB without a separate report-only pass. The test asserts no DELETE and no UPDATE policy over the bucket under ANY name, because the hole arrived under a name this repo never used. [2026-07-27] Foreign 0 % supplier invoices (#1042): shipped as an advisory banner keyed on supplier_type + reverse_charge, NOT as the "confirm the reason for 0 % VAT (exempt vs import vs other)" picker the issue asks for. Two reasons. First, supplier_invoices.vat_treatment is pure metadata: createSupplierInvoiceRegistrationEntry branches on reverse_charge + supplier_type + per-line vat_rate and never reads it, and get_vat_declaration_totals maps rutor from journal account numbers alone, so a stored reason would change no accounting output. Second, an "import" option would be false confidence: nothing in the codebase can book import VAT (no generator emits 2615/2625/2635 or the 4545-4547 basis accounts), so offering it would imply ruta 50/60 were handled when they are not. The real defect underneath the issue is narrower and is what this fixes: a foreign supplier at 0 % with reverse charge left off books no 26x4 leg and no 44xx/45xx basis, emptying ruta 20-24/30-32/48. The check is deliberately silent for swedish_business, where 0 % is a genuine exemption belonging in no ruta, and never blocks submission, because a non-EU goods purchase cleared at customs is legitimately 0 % without reverse charge and forcing the switch there would book a wrong verifikat. Import VAT support stays a separate, larger issue. + +[2026-07-28] /submit-pr removed at Emil's request; /resolve-pr is now the single PR skill and absorbed submit-pr's full contents (publish stage included, skipped when a PR already exists). Folding rather than plain deletion was necessary because /fix lane mode and /resolve-pr both called into it, so deleting alone would have left two skills pointing at nothing and lanes unable to push. All rules survive verbatim, including the one-fix-commit discipline and the bots-edit-comments-in-place trap. Backup of the old file kept in the session scratchpad only. + +[2026-07-28] /orchestrate deleted the same day it was built; parallel work is now one /fix per terminal, each isolated by EnterWorktree. Two things killed the coordinator: a Claude instance cannot observe another instance's terminal, so it could only ever track lanes via a file the lanes themselves wrote (making it a reader, not a controller), and Emil says "it works" in the lane's own terminal, so the coordinator was never in the core loop. EnterWorktree then removed its last real job. The merge lock, shared board and file-scope collision holds went with it: /fix now stops at the commit and suggests /resolve-pr, so Emil approves each merge by hand and serialises them himself. Residual hazard (another session merged while this branch was tested) is handled where it already was, in /resolve-pr Stage 0: fetch, merge origin/main, migration-collision check, re-verify, and void the certification if the merge touched the feature's own files. Trade accepted knowingly: this drops the full-autonomy-including-merge grant from earlier the same day, in exchange for a much leaner /fix. Branch protection on main is OFF; enable it when a second human joins. + +[2026-07-28] /fix reverted to its 2026-07-07 shape and trimmed further: issue -> understand -> plan card gate -> implement -> self-verify -> test card -> "it works" -> commit, and it ends there. Same-day additions removed: the Stage 0 worktree/bun-install/port-probing block, the `git merge origin/main` sync step, the tone-of-voice reply draft, and the closing /resolve-pr suggestion; the CLAUDE.md "Worktrees" section went with them, since it re-injected EnterWorktree into every /fix run. Emil's call, verbatim: an end-to-end skill that takes an issue, runs the steps, and commits when he says it works; nothing else. This reverses the /orchestrate post-mortem entry above, which assumed worktree isolation would stay. Parallel /fix sessions now share whatever branch their terminal is on, which is the cost accepted for the leaner skill; reply drafts move to a separate /tone-of-voice invocation. + +[2026-07-28] Issue #1258 (supplier invoice line description): chose Option A, keep the fixed invoice-level verifikat text and fix only the preview, over Option B, propagating each item's typed description to its journal line. Deciding fact: the customer-invoice side already writes invoice-level descriptions too ("Forsaljning faktura {tag}", "Kreditfaktura {tag}" in lib/bookkeeping/invoice-entries.ts), with the only per-line variation being a voucher cross-reference suffix. Invoice-level text is therefore a system-wide convention, not a supplier-side oversight, so B would have created an inconsistency between the two invoice sides rather than removing one, while landing in the journal engine and needing an aggregation-collision policy for items that share an (account, dimensions) bucket. Accepted cost, stated plainly: the per-item description the user types stays UI-only and never reaches the books. Revisit if users ask for per-line ledger detail; the collision policy is the only hard part. + +[2026-07-28] Preview honesty over prettier labels in the supplier-invoice voucher preview: the BESKRIVNING column now renders the exact line_description the engine will post, and the hardcoded ACCOUNT_LABELS map (11 accounts) was removed. That map made the column silently mix "friendly account label" (for its 11 entries) with "raw account number" (every expense account, the reported bug), and neither was the posted text. Account identity was not lost: AccountNumber already shows the BAS name on its hover card. The ankomstnummer suffix the engine appends is absent from the preview because it is assigned on save and does not exist yet at preview time. + +[2026-07-28] The "senaste bokförda verifikat" line in the balans-/resultatrapport header (#1267) reads MAX(voucher_number) over posted entries, never voucher_sequences.last_number. The sequence counter is an allocation high-water mark that provably drifts from the books in both directions: next_voucher_number burns a number when the follow-up insert fails (the reversal path in engine.ts does exactly that), delete_last_voucher decrements blindly by one instead of resetting to the new MAX, and pre-RPC SIE imports left it behind MAX. Since the whole point of the line is avstämning, printing an allocated number would send a reconciler chasing a gap that does not exist, so the label states plainly that the number is the last posted one. Scoped to the report's own date range rather than the fiscal year, so a Q1 report printed in November says something true about Q1; the balansrapport keeps the fiscal-year start as its lower bound because it accumulates. Skipped entirely on a dimension-filtered resultatrapport: that report already discloses it is partial, and an unfiltered voucher range beside a filtered result invites the wrong conclusion. No new i18n keys: both report views and the PDF template are hard-coded Swedish, per the "stays Swedish" report surfaces in .claude/rules/i18n.md, so the issue's acceptance criterion asking for sv+en strings does not apply here. diff --git a/app/(dashboard)/bookkeeping/[id]/page.tsx b/app/(dashboard)/bookkeeping/[id]/page.tsx index f3ab0f93..09a5a536 100644 --- a/app/(dashboard)/bookkeeping/[id]/page.tsx +++ b/app/(dashboard)/bookkeeping/[id]/page.tsx @@ -520,19 +520,20 @@ export default function JournalEntryDetailPage({ params }: { params: Promise<{ i {t('correct_opening_balances')} )} - {entry.status === 'posted' && ( - - )} + {/* Copy is not status-gated: it only prefills a fresh manual draft + (no voucher number, date or attachments carried over), so it is + offered on drafts too, matching the list surfaces. */} + )} diff --git a/app/(dashboard)/customers/[id]/page.tsx b/app/(dashboard)/customers/[id]/page.tsx index 25200b2b..1d3a7c2d 100644 --- a/app/(dashboard)/customers/[id]/page.tsx +++ b/app/(dashboard)/customers/[id]/page.tsx @@ -10,7 +10,11 @@ import { Button } from '@/components/ui/button' import { Badge } from '@/components/ui/badge' import { Dialog, DialogContent, DialogHeader, DialogTitle } from '@/components/ui/dialog' import { useToast } from '@/components/ui/use-toast' -import { maskCustomerPersonalNumber } from '@/lib/customers/mask-personal-number' +import { + UNDECRYPTABLE_PERSONAL_NUMBER_MASK, + maskCustomerPersonalNumber, +} from '@/lib/customers/mask-personal-number' +import { AttnLine } from '@/components/ui/attn-line' import CustomerForm from '@/components/customers/CustomerForm' import { DestructiveConfirmDialog, useDestructiveConfirm } from '@/components/ui/destructive-confirm-dialog' import { @@ -26,8 +30,12 @@ import { Loader2, ReceiptText, Lock, + Eye, + EyeOff, } from 'lucide-react' +import { useLocale } from 'next-intl' import { useCanWrite } from '@/lib/hooks/use-can-write' +import { getErrorMessage, type ErrorLocale } from '@/lib/errors/get-error-message' import { cn, formatDate } from '@/lib/utils' import { invoiceNumberDisplay } from '@/lib/invoices/display' import type { Customer, CustomerType, CreateCustomerInput } from '@/types' @@ -71,18 +79,57 @@ export default function CustomerDetailPage({ const { toast } = useToast() const { canWrite } = useCanWrite() const t = useTranslations('customer_detail') + const errorLocale = useLocale() as ErrorLocale const [customer, setCustomer] = useState(null) const [isLoading, setIsLoading] = useState(true) const [isEditOpen, setIsEditOpen] = useState(false) const [isUpdating, setIsUpdating] = useState(false) + // Full personnummer, fetched on demand and held only for this view. Cleared + // whenever the customer is refetched so it can never outlive the row it + // belongs to. + const [revealedPersonalNumber, setRevealedPersonalNumber] = useState(null) + const [isRevealing, setIsRevealing] = useState(false) const { dialogProps: confirmDialogProps, confirm: confirmAction } = useDestructiveConfirm() + const isUnreadablePersonalNumber = + customer?.personal_number === UNDECRYPTABLE_PERSONAL_NUMBER_MASK + + async function togglePersonalNumber() { + if (revealedPersonalNumber) { + setRevealedPersonalNumber(null) + return + } + setIsRevealing(true) + try { + const response = await fetch(`/api/customers/${id}/personal-number`) + const result = await response.json() + if (!response.ok) { + toast({ + title: t('personal_number_reveal_failed_title'), + description: getErrorMessage(result, { context: 'customer', locale: errorLocale }), + variant: 'destructive', + }) + return + } + setRevealedPersonalNumber(result.data.personal_number) + } catch { + toast({ + title: t('personal_number_reveal_failed_title'), + description: t('retry'), + variant: 'destructive', + }) + } finally { + setIsRevealing(false) + } + } + useEffect(() => { fetchCustomer() }, [id]) async function fetchCustomer() { setIsLoading(true) + setRevealedPersonalNumber(null) try { const response = await fetch(`/api/customers/${id}`) if (!response.ok) { @@ -298,8 +345,38 @@ export default function CustomerDetailPage({
{t('label_personal_number')} - {maskCustomerPersonalNumber(customer.personal_number || customer.org_number)} + {revealedPersonalNumber ?? + maskCustomerPersonalNumber(customer.personal_number || customer.org_number)} + {/* Viewers keep the mask: the endpoint refuses them anyway. */} + {canWrite && customer.personal_number && !isUnreadablePersonalNumber && ( + + )} + {isUnreadablePersonalNumber && ( + setIsEditOpen(true) }} + > + {t('personal_number_unreadable')} + + )}
)} {customer.vat_number && ( diff --git a/app/(dashboard)/customers/page.tsx b/app/(dashboard)/customers/page.tsx index 7b6540fb..03147911 100644 --- a/app/(dashboard)/customers/page.tsx +++ b/app/(dashboard)/customers/page.tsx @@ -4,7 +4,6 @@ import { useState, useEffect, useMemo, useCallback, Suspense } from 'react' import dynamic from 'next/dynamic' import { useLocale, useTranslations } from 'next-intl' import { useSearchParams, useRouter, usePathname } from 'next/navigation' -import { createClient } from '@/lib/supabase/client' import { Button } from '@/components/ui/button' import { Badge } from '@/components/ui/badge' import { Input } from '@/components/ui/input' @@ -18,7 +17,6 @@ import { EmptyCustomers, EmptyState } from '@/components/ui/empty-state' import { ReportExportMenu } from '@/components/reports/ReportExportMenu' import { cn } from '@/lib/utils' import Link from 'next/link' -import { useCompany } from '@/contexts/CompanyContext' import { useCanWrite } from '@/lib/hooks/use-can-write' import type { Customer, CustomerType, CreateCustomerInput } from '@/types' @@ -64,7 +62,6 @@ function compareStrings(a: string, b: string): number { } function CustomersPageInner() { - const { company } = useCompany() const { canWrite } = useCanWrite() const [customers, setCustomers] = useState([]) const [isLoading, setIsLoading] = useState(true) @@ -73,7 +70,6 @@ function CustomersPageInner() { const [isDialogOpen, setIsDialogOpen] = useState(false) const [isCreating, setIsCreating] = useState(false) const { toast } = useToast() - const supabase = createClient() const t = useTranslations('customers') const tCommon = useTranslations('common') const errorLocale = useLocale() as ErrorLocale @@ -104,25 +100,38 @@ function CustomersPageInner() { [searchParams, sortColumn, sortDir, router, pathname] ) + /** + * Read the roster through the API, not straight from Supabase. + * + * personal_number holds AES-256-GCM ciphertext (migration 20260726110000). + * A browser-side select('*') handed this page 76 to 82 hex characters and + * getIdentifier() rendered them into the nowrap identifier cell, which is + * what shredded the table layout for companies with private customers. + * GET /api/customers maps every row through maskCustomerRow, so the + * ciphertext now never leaves the server and the column shows the same + * '********-1234' the detail view does. + * + * No `company` guard: the route resolves the active company server-side, so + * the fetch no longer has to wait for CompanyContext to hydrate. The old + * guard could leave the list empty on a slow context load, because the + * effect below runs once and never retries. + */ async function fetchCustomers() { - if (!company) return setIsLoading(true) - const { data, error } = await supabase - .from('customers') - .select('*') - .eq('company_id', company.id) - .order('name', { ascending: true }) - - if (error) { + try { + const response = await fetch('/api/customers') + if (!response.ok) throw new Error('Failed to load customers') + const { data } = await response.json() + setCustomers(data || []) + } catch { toast({ title: t('load_failed_title'), description: t('load_failed_description'), variant: 'destructive', }) - } else { - setCustomers(data || []) + } finally { + setIsLoading(false) } - setIsLoading(false) } useEffect(() => { @@ -167,6 +176,9 @@ function CustomersPageInner() { c.name.toLowerCase().includes(term) || c.email?.toLowerCase().includes(term) || c.org_number?.includes(term) || + // The masked form, so this matches the last four digits. Against the + // raw ciphertext it matched nothing, which read as "search is broken" + // for anyone looking up a private customer by personnummer. c.personal_number?.includes(term) || c.city?.toLowerCase().includes(term) || c.notes?.toLowerCase().includes(term) diff --git a/app/(dashboard)/transactions/page.tsx b/app/(dashboard)/transactions/page.tsx index 48f681ba..44b102de 100644 --- a/app/(dashboard)/transactions/page.tsx +++ b/app/(dashboard)/transactions/page.tsx @@ -43,6 +43,10 @@ import type { StoredSkattekontoTransaction, } from '@/types/skatteverket' import { findBankSkvCounterparts } from '@/lib/skatteverket/bank-counterpart' +import { + MATCHABLE_INVOICE_STATUSES, + MATCHABLE_SUPPLIER_INVOICE_STATUSES, +} from '@/lib/invoices/matchable-statuses' import { useCompany } from '@/contexts/CompanyContext' import { useRealtimeSupabase } from '@/lib/hooks/use-realtime-supabase' import { getErrorMessage } from '@/lib/errors/get-error-message' @@ -155,12 +159,27 @@ async function fetchPotentialMatches( .filter((t) => t.potential_supplier_invoice_id) .map((t) => t.potential_supplier_invoice_id) + // The hint columns are never revisited once written, so an invoice settled + // by a different transaction leaves a stale pointer behind. Revalidate here: + // an unmatchable candidate must not reach the row or the match dialog, which + // would otherwise compare the transaction against a 0 kr remaining balance + // and call it a partial payment. const [invoiceResult, supplierInvoiceResult] = await Promise.all([ potentialInvoiceIds.length > 0 - ? supabase.from('invoices').select('*, customer:customers(*)').in('id', potentialInvoiceIds) + ? supabase + .from('invoices') + .select('*, customer:customers(*)') + .in('id', potentialInvoiceIds) + .in('status', [...MATCHABLE_INVOICE_STATUSES]) + .gt('remaining_amount', 0) : Promise.resolve({ data: null, error: null }), potentialSupplierInvoiceIds.length > 0 - ? supabase.from('supplier_invoices').select('*, supplier:suppliers(*)').in('id', potentialSupplierInvoiceIds) + ? supabase + .from('supplier_invoices') + .select('*, supplier:suppliers(*)') + .in('id', potentialSupplierInvoiceIds) + .in('status', [...MATCHABLE_SUPPLIER_INVOICE_STATUSES]) + .gt('remaining_amount', 0) : Promise.resolve({ data: null, error: null }), ]) diff --git a/app/api/bookkeeping/accounts/__tests__/accounts.test.ts b/app/api/bookkeeping/accounts/__tests__/accounts.test.ts index a272cb64..9f3190ec 100644 --- a/app/api/bookkeeping/accounts/__tests__/accounts.test.ts +++ b/app/api/bookkeeping/accounts/__tests__/accounts.test.ts @@ -193,6 +193,58 @@ describe('POST /api/bookkeeping/accounts', () => { expect(body.error).toContain('5010') }) + it('keeps the plain 409 when the colliding account is active', async () => { + const { supabase } = createCapturingSupabase([ + { error: { code: '23505', message: 'dup' } }, + { data: { is_active: true } }, + ]) + auth(supabase) + const req = createMockRequest('/api/bookkeeping/accounts', { + method: 'POST', + body: { + account_number: '5010', + account_name: 'Lokalhyra', + account_type: 'expense', + normal_balance: 'debit', + }, + }) + const { status, body } = await parseJsonResponse<{ error: string }>( + await createPOST(req, routeParams) + ) + expect(status).toBe(409) + expect(typeof body.error).toBe('string') + }) + + it('returns ACCOUNT_EXISTS_INACTIVE when the colliding account is deactivated', async () => { + const { supabase, calls } = createCapturingSupabase([ + { error: { code: '23505', message: 'dup' } }, + { data: { is_active: false } }, + ]) + auth(supabase) + const req = createMockRequest('/api/bookkeeping/accounts', { + method: 'POST', + body: { + account_number: '3910', + account_name: 'Hyresintäkter egen', + account_type: 'revenue', + normal_balance: 'credit', + }, + }) + const { status, body } = await parseJsonResponse<{ + error: { code: string; message: string; details?: { account_number?: string } } + }>(await createPOST(req, routeParams)) + + expect(status).toBe(409) + expect(body.error.code).toBe('ACCOUNT_EXISTS_INACTIVE') + expect(body.error.message).toContain('3910') + expect(body.error.details?.account_number).toBe('3910') + // The is_active lookup must be company-scoped, not a bare account_number + // match: the same number exists under every other company too. + const eqArgs = calls.filter((c) => c.method === 'eq').map((c) => c.args) + expect(eqArgs).toContainEqual(['company_id', 'company-1']) + expect(eqArgs).toContainEqual(['account_number', '3910']) + }) + it('forwards default_vat_rate into the insert', async () => { const { supabase, calls } = createCapturingSupabase([ { data: { account_number: '3740', default_vat_rate: 0 } }, @@ -420,4 +472,54 @@ describe('POST /api/bookkeeping/accounts/activate', () => { expect(body.activated).toBe(1) expect(body.unknown).toEqual(['0000']) }) + + // The only route back for a deactivated account: it is in the chart, so the + // insert path would hit the unique constraint. It must be flipped back on + // instead, including for custom numbers the BAS reference has never heard of. + it('reactivates an existing inactive account instead of inserting it', async () => { + const { supabase, calls } = createCapturingSupabase([ + { data: [{ account_number: '3910', is_active: false }] }, // existing lookup + { data: [{ account_number: '3910' }] }, // update result + ]) + auth(supabase) + const req = createMockRequest('/api/bookkeeping/accounts/activate', { + method: 'POST', + body: { account_numbers: ['3910'] }, + }) + const { status, body } = await parseJsonResponse<{ + activated: number + reactivated: number + skipped: number + unknown: string[] + }>(await activatePOST(req, routeParams)) + + expect(status).toBe(200) + expect(body.reactivated).toBe(1) + expect(body.activated).toBe(0) + expect(body.unknown).toEqual([]) + expect(calls.find((c) => c.method === 'update')?.args[0]).toEqual({ is_active: true }) + expect(calls.some((c) => c.method === 'insert')).toBe(false) + }) + + it('skips an account that is already active', async () => { + const { supabase, calls } = createCapturingSupabase([ + { data: [{ account_number: '1930', is_active: true }] }, + ]) + auth(supabase) + const req = createMockRequest('/api/bookkeeping/accounts/activate', { + method: 'POST', + body: { account_numbers: ['1930'] }, + }) + const { status, body } = await parseJsonResponse<{ + activated: number + reactivated: number + skipped: number + }>(await activatePOST(req, routeParams)) + + expect(status).toBe(200) + expect(body.skipped).toBe(1) + expect(body.reactivated).toBe(0) + expect(calls.some((c) => c.method === 'update')).toBe(false) + expect(calls.some((c) => c.method === 'insert')).toBe(false) + }) }) diff --git a/app/api/bookkeeping/accounts/__tests__/reference.test.ts b/app/api/bookkeeping/accounts/__tests__/reference.test.ts index 81d15083..70803a9f 100644 --- a/app/api/bookkeeping/accounts/__tests__/reference.test.ts +++ b/app/api/bookkeeping/accounts/__tests__/reference.test.ts @@ -31,7 +31,7 @@ function createCapturingSupabase(results: { data?: unknown; error?: unknown }[]) const result = results[idx++] ?? { data: null, error: null } // eslint-disable-next-line @typescript-eslint/no-explicit-any const b: any = {} - for (const m of ['select', 'eq', 'order', 'range', 'maybeSingle', 'single']) { + for (const m of ['select', 'eq', 'in', 'order', 'range', 'maybeSingle', 'single']) { b[m] = (...args: unknown[]) => { calls.push({ method: m, args }) return b @@ -85,9 +85,21 @@ describe('GET /api/bookkeeping/accounts/reference', () => { }) }) +/** + * bas-lookup answers "can this number be activated at all?", which is what + * gates ActivateAccountsDialog's confirm button. It consults the company's own + * chart first, so a custom (non-BAS) account the company deactivated still + * comes back known — before that it read as unknown and the button stayed dead. + */ describe('GET /api/bookkeeping/accounts/bas-lookup', () => { + function authWith(chartRows: unknown[]) { + const { supabase, calls } = createCapturingSupabase([{ data: chartRows }]) + requireAuthMock.mockResolvedValue({ user: { id: 'user-1' }, supabase, error: null }) + return calls + } + beforeEach(() => { - requireAuthMock.mockResolvedValue({ user: { id: 'user-1' }, supabase: {}, error: null }) + authWith([]) }) it('returns 401 when not authenticated', async () => { @@ -96,7 +108,10 @@ describe('GET /api/bookkeeping/accounts/bas-lookup', () => { supabase: {}, error: NextResponse.json({ error: 'Unauthorized' }, { status: 401 }), }) - const res = await basLookupGET(createMockRequest('/api/bookkeeping/accounts/bas-lookup')) + const res = await basLookupGET( + createMockRequest('/api/bookkeeping/accounts/bas-lookup'), + routeParams + ) expect(res.status).toBe(401) }) @@ -105,20 +120,82 @@ describe('GET /api/bookkeeping/accounts/bas-lookup', () => { searchParams: { numbers: '1930,0000' }, }) const { status, body } = await parseJsonResponse<{ - data: Array<{ account_number: string; known: boolean }> - }>(await basLookupGET(req)) + data: Array<{ account_number: string; known: boolean; in_chart: boolean }> + }>(await basLookupGET(req, routeParams)) expect(status).toBe(200) - expect(body.data.find((a) => a.account_number === '1930')?.known).toBe(true) + const bas = body.data.find((a) => a.account_number === '1930') + expect(bas?.known).toBe(true) + // Known from the static catalog, not held by the company: activating it + // inserts a new row rather than reviving one. + expect(bas?.in_chart).toBe(false) expect(body.data.find((a) => a.account_number === '0000')?.known).toBe(false) }) + it('reports a deactivated custom account as known and in the chart', async () => { + const calls = authWith([ + { + account_number: '3910', + account_name: 'Hyresintäkter egen', + account_class: 3, + account_type: 'revenue', + is_active: false, + }, + ]) + const req = createMockRequest('/api/bookkeeping/accounts/bas-lookup', { + searchParams: { numbers: '3910' }, + }) + const { status, body } = await parseJsonResponse<{ + data: Array<{ + account_number: string + account_name: string | null + known: boolean + in_chart: boolean + is_active: boolean + }> + }>(await basLookupGET(req, routeParams)) + + expect(status).toBe(200) + // 3910 is not in the BAS catalog: without the chart read this was known:false. + const row = body.data[0] + expect(row.known).toBe(true) + expect(row.in_chart).toBe(true) + expect(row.is_active).toBe(false) + expect(row.account_name).toBe('Hyresintäkter egen') + // Defense in depth alongside RLS: another company's chart must not answer. + expect(calls.filter((c) => c.method === 'eq').map((c) => c.args)).toContainEqual([ + 'company_id', + 'company-1', + ]) + }) + + it("prefers the company's own account name over the BAS catalog name", async () => { + authWith([ + { + account_number: '1930', + account_name: 'Företagskonto SEB', + account_class: 1, + account_type: 'asset', + is_active: true, + }, + ]) + const req = createMockRequest('/api/bookkeeping/accounts/bas-lookup', { + searchParams: { numbers: '1930' }, + }) + const { body } = await parseJsonResponse<{ + data: Array<{ account_name: string | null; in_chart: boolean }> + }>(await basLookupGET(req, routeParams)) + + expect(body.data[0].account_name).toBe('Företagskonto SEB') + expect(body.data[0].in_chart).toBe(true) + }) + it('rejects an oversized numbers list with 400', async () => { const many = Array.from({ length: 2001 }, (_, i) => String(10000 + i)).join(',') const req = createMockRequest('/api/bookkeeping/accounts/bas-lookup', { searchParams: { numbers: many }, }) - const { status } = await parseJsonResponse(await basLookupGET(req)) + const { status } = await parseJsonResponse(await basLookupGET(req, routeParams)) expect(status).toBe(400) }) }) diff --git a/app/api/bookkeeping/accounts/bas-lookup/route.ts b/app/api/bookkeeping/accounts/bas-lookup/route.ts index cb35ac8e..3fa49679 100644 --- a/app/api/bookkeeping/accounts/bas-lookup/route.ts +++ b/app/api/bookkeeping/accounts/bas-lookup/route.ts @@ -1,21 +1,40 @@ import { NextResponse } from 'next/server' -import { requireAuth } from '@/lib/auth/require-auth' +import { withRouteContext } from '@/lib/api/with-route-context' import { getBASReference } from '@/lib/bookkeeping/bas-reference' +import { fetchAllRows } from '@/lib/supabase/fetch-all' +import { getErrorMessage as getUserErrorMessage } from '@/lib/errors/get-error-message' + +interface ChartRow { + account_number: string + account_name: string + account_class: number | null + account_type: string | null + is_active: boolean | null +} /** * GET /api/bookkeeping/accounts/bas-lookup?numbers=5010,2641 * - * Returns BAS reference metadata (name, class, type) for a list of account - * numbers. Used by ActivateAccountsDialog to render human-readable labels - * before the user confirms activation. Unknown numbers are returned with - * account_name=null so the UI can flag them as non-BAS. + * Resolves account numbers to a human-readable label plus whether they can be + * activated at all. Used by ActivateAccountsDialog to render the list before + * the user confirms. * - * Pure in-memory reference lookup — no tenant data, so no company context is - * resolved; requireAuth() keeps it behind auth (MFA on hosted). + * Two sources, company first: + * 1. The company's own chart_of_accounts. A row here is activatable even if + * it isn't a standard BAS account (custom accounts) and even if it is + * currently deactivated: POST /accounts/activate reactivates it. The + * company's own account_name wins, since it may have been renamed. + * 2. The static BAS reference, for accounts not yet added to the chart. + * + * Numbers in neither source are returned with account_name=null and + * known=false so the UI can flag them as non-BAS and offer "create". + * + * `in_chart` / `is_active` let callers tell "will be added" from "will be + * reactivated". Consulting the company chart is why this route resolves a + * company context (it was a pure in-memory reference lookup before). */ -export async function GET(request: Request) { - const auth = await requireAuth() - if (auth.error) return auth.error +export const GET = withRouteContext('bookkeeping.accounts.bas-lookup', async (request, ctx) => { + const { supabase, companyId } = ctx const { searchParams } = new URL(request.url) const raw = searchParams.get('numbers') || '' @@ -28,10 +47,47 @@ export async function GET(request: Request) { return NextResponse.json({ error: 'Too many account numbers' }, { status: 400 }) } + // Paginated: `numbers` can hold up to 2000 entries and PostgREST silently + // caps an unranged select at 1000 rows. A truncated chart read would report + // accounts that ARE in the chart as known:false / in_chart:false, so the + // dialog would offer "create" for an account that already exists. + let chartRows: ChartRow[] + try { + chartRows = await fetchAllRows( + ({ from, to }) => + supabase + .from('chart_of_accounts') + .select('account_number, account_name, account_class, account_type, is_active') + .eq('company_id', companyId) + .in('account_number', numbers) + // Paging is only stable under a unique total order. + .order('account_number', { ascending: true }) + .range(from, to), + { dedupeBy: (row) => row.account_number }, + ) + } catch (error) { + return NextResponse.json({ error: getUserErrorMessage(error) }, { status: 500 }) + } + + const inChart = new Map(chartRows.map((row) => [row.account_number, row])) + const data = numbers.map((num) => { + const own = inChart.get(num) + if (own) { + return { + account_number: own.account_number, + account_name: own.account_name, + account_class: own.account_class, + account_type: own.account_type, + known: true, + in_chart: true, + is_active: own.is_active, + } + } + const ref = getBASReference(num) if (!ref) { - return { account_number: num, account_name: null, known: false } + return { account_number: num, account_name: null, known: false, in_chart: false, is_active: false } } return { account_number: ref.account_number, @@ -39,8 +95,10 @@ export async function GET(request: Request) { account_class: ref.account_class, account_type: ref.account_type, known: true, + in_chart: false, + is_active: false, } }) return NextResponse.json({ data }) -} +}) diff --git a/app/api/bookkeeping/accounts/route.ts b/app/api/bookkeeping/accounts/route.ts index 94efdb86..979a6cdd 100644 --- a/app/api/bookkeeping/accounts/route.ts +++ b/app/api/bookkeeping/accounts/route.ts @@ -5,6 +5,7 @@ import { withRouteContext } from '@/lib/api/with-route-context' import { validateBody, validateQuery } from '@/lib/api/validate' import { CreateAccountSchema } from '@/lib/api/schemas' import { getErrorMessage as getUserErrorMessage } from '@/lib/errors/get-error-message' +import { errorResponseFromCode } from '@/lib/errors/get-structured-error' // Response shapes are legacy `{ data }` / `{ error: string }` — several pages // (import, supplier-invoices, article form) consume the list directly. @@ -111,6 +112,27 @@ export const POST = withRouteContext( if (error) { if (error.code === '23505') { + // The unique constraint counts deactivated rows, so "already exists" + // covers two very different situations. Only look up which one it is + // on the failing path: the happy path stays a single insert. + const { data: existing } = await supabase + .from('chart_of_accounts') + .select('is_active') + .eq('company_id', companyId) + .eq('account_number', body.account_number) + .maybeSingle() + + if (existing && existing.is_active === false) { + // Re-creating can never succeed here; the caller must reactivate + // instead. The distinct code is what AddAccountDialog keys on to + // offer that as a one-click action rather than a dead end. + return errorResponseFromCode('ACCOUNT_EXISTS_INACTIVE', log, { + status: 409, + messageSv: `Kontonummer ${body.account_number} finns redan i din kontoplan men är inaktiverat.`, + details: { account_number: body.account_number }, + }) + } + return NextResponse.json( { error: `Kontonummer ${body.account_number} finns redan i din kontoplan.` }, { status: 409 }, diff --git a/app/api/customers/[id]/personal-number/route.ts b/app/api/customers/[id]/personal-number/route.ts new file mode 100644 index 00000000..723c04af --- /dev/null +++ b/app/api/customers/[id]/personal-number/route.ts @@ -0,0 +1,82 @@ +/** + * GET /api/customers/{id}/personal-number + * + * The deliberate drill-in behind the mask: returns the full personnummer for + * one individual customer. + * + * Every other customer read surface (list, detail, export) returns + * '********-1234'. Without this endpoint the value was write-only by + * construction: a user could store a personnummer and then never verify what + * had actually been stored, which is the failure this exists to close. It + * mirrors the employee convention, where the list masks and the master GET + * returns all 12 digits (app/api/v1/companies/[companyId]/employees/[id]). + * + * Gated on the write role even though it only reads. .compliance/ropa.yaml + * listed `no_full_value_read_endpoint` among the safeguards for + * customers.personal_number; this endpoint retires that measure, so it keeps + * the exposure as narrow as the purpose allows. The person who needs to verify + * a stored personnummer is the one who typed it and can correct it, which is + * exactly the non-viewer role. A viewer (typically an external consultant with + * read-only access) keeps seeing the mask. + * + * The read is logged with the actor, never with the value, so a reveal is + * attributable. audit_log is written by DB triggers only, so this is a + * structured application log rather than an audit row. + */ + +import { NextResponse } from 'next/server' +import { withRouteContext } from '@/lib/api/with-route-context' +import { errorResponseFromCode } from '@/lib/errors/get-structured-error' +import { revealStoredCustomerPersonalNumber } from '@/lib/customers/protect-personal-number' +import { getErrorMessage as getUserErrorMessage } from '@/lib/errors/get-error-message' + +export const GET = withRouteContext( + 'customer.personal_number.reveal', + async (_request, ctx, { params }: { params: Promise<{ id: string }> }) => { + const { id } = await params + const { supabase, companyId, user, log, requestId } = ctx + const opLog = log.child({ customerId: id }) + + const { data, error } = await supabase + .from('customers') + .select('id, personal_number') + .eq('id', id) + .eq('company_id', companyId) + .single() + + if (error) { + if (error.code === 'PGRST116') { + return errorResponseFromCode('CUSTOMER_NOT_FOUND', opLog, { requestId }) + } + opLog.error('customer fetch before personal number reveal failed', error) + return errorResponseFromCode('INTERNAL_ERROR', opLog, { + requestId, + details: { reason: getUserErrorMessage(error) }, + }) + } + + if (!data.personal_number) { + return errorResponseFromCode('CUSTOMER_NO_PERSONAL_NUMBER', opLog, { requestId }) + } + + let personalNumber: string | null + try { + personalNumber = revealStoredCustomerPersonalNumber(data.personal_number) + } catch (err) { + // Same row state the mask renders as '********-????'. Answer with the + // specific code so the UI can tell the user to retype it, rather than + // with a 500 that reads as "try again later". + opLog.error('customer personal_number decrypt failed on reveal', { + reason: err instanceof Error ? err.message : String(err), + }) + return errorResponseFromCode('CUSTOMER_PERSONAL_NUMBER_UNREADABLE', opLog, { requestId }) + } + + // Attributable without being a leak: who revealed which customer, never + // the number itself. + opLog.info('customer personal number revealed', { userId: user.id }) + + return NextResponse.json({ data: { personal_number: personalNumber } }) + }, + { requireWrite: true }, +) diff --git a/app/api/customers/[id]/route.ts b/app/api/customers/[id]/route.ts index 1824b802..645f7318 100644 --- a/app/api/customers/[id]/route.ts +++ b/app/api/customers/[id]/route.ts @@ -5,19 +5,9 @@ import { validateVatNumber } from '@/lib/vat/vies-client' import { withRouteContext } from '@/lib/api/with-route-context' import { errorResponseFromCode } from '@/lib/errors/get-structured-error' import { encryptCustomerPersonalNumber, maskCustomerRow } from '@/lib/customers/protect-personal-number' +import { isMaskedPersonalNumber } from '@/lib/customers/mask-personal-number' import { getErrorMessage as getUserErrorMessage } from '@/lib/errors/get-error-message' -/** - * Shape produced by maskCustomerPersonalNumber: no read path ever returns the - * stored personnummer, only '********-1234'. A client that PATCHes back a - * customer it just read therefore submits the mask, which must mean "leave the - * stored value alone", never "store this literally" and never "clear it". - * components/customers/CustomerForm.tsx strips it before sending, but that - * guard belongs here too: any other client (script, agent, future UI) that - * skips it would otherwise destroy the value. - */ -const MASKED_PERSONAL_NUMBER = /^\*{8}-\d{4}$/ - export const GET = withRouteContext( 'customer.get', async (_request, ctx, { params }: { params: Promise<{ id: string }> }) => { @@ -83,11 +73,22 @@ export const PATCH = withRouteContext( return errorResponseFromCode('CUSTOMER_UPDATE_FAILED', opLog, { requestId }) } - // The masked sentinel counts as "field not supplied": it carries no new + // No ordinary read returns the stored personnummer, only '********-1234', + // or '********-????' when the stored value could not be decrypted. A + // client that PATCHes back a customer it just read therefore submits one + // of those, and it counts as "field not supplied": it carries no new // value, so it must not be validated, stored or treated as a clear. + // CustomerForm strips it before sending, but the guard belongs here too: + // any other client (script, agent, future UI) that skips it would + // otherwise destroy the value. + // + // Both forms are recognized via lib/customers/mask-personal-number.ts so + // this route, UpdateCustomerSchema and the form cannot disagree about what + // counts as a mask. They previously each carried their own '-1234'-only + // copy, which made an undecryptable row uneditable in every field, not + // just this one. const personalNumberSubmitted = - body.personal_number !== undefined && - !(typeof body.personal_number === 'string' && MASKED_PERSONAL_NUMBER.test(body.personal_number)) + body.personal_number !== undefined && !isMaskedPersonalNumber(body.personal_number) const effectiveType = body.customer_type ?? existing.customer_type if (personalNumberSubmitted && body.personal_number && effectiveType !== 'individual') { diff --git a/app/api/customers/__tests__/personal-number.test.ts b/app/api/customers/__tests__/personal-number.test.ts index 8d301180..98c67193 100644 --- a/app/api/customers/__tests__/personal-number.test.ts +++ b/app/api/customers/__tests__/personal-number.test.ts @@ -54,6 +54,8 @@ type CustomerWrite = { personal_number?: string | null } // Synthetic personnummer, never a real one. const PERSONAL_NUMBER = '19900101-1234' const MASKED = '********-1234' +// What a row whose stored ciphertext cannot be decrypted reads back as. +const UNDECRYPTABLE_MASK = '********-????' /** * The shape customers_personal_number_check accepts as of 20260726110000: @@ -211,6 +213,88 @@ describe('personal_number on customer routes', () => { expect(body.data.personal_number).toBe(MASKED) }) + it('keeps the stored value when the undecryptable placeholder is sent back', async () => { + // A row whose ciphertext cannot be decrypted reads back as + // '********-????'. That is still a mask, so PATCHing it must leave the + // column alone. When only '********-1234' was recognized, this 400'd and + // took the whole edit with it: the customer's name and address could not + // be saved either, over a field the user had no way to correct. + queryResult = { + data: { + id: 'customer-1', + customer_type: 'individual', + name: 'Anna Andersson', + personal_number: 'ab'.repeat(40), + }, + error: null, + } + + const response = await PATCH( + createMockRequest('/api/customers/customer-1', { + method: 'PATCH', + body: { + name: 'Anna Andersson', + city: 'Göteborg', + personal_number: UNDECRYPTABLE_MASK, + }, + }), + routeParams, + ) + + const { status, body } = await parseJsonResponse<{ data: { personal_number: string } }>(response) + expect(status).toBe(200) + // The rest of the edit went through... + expect(captured.update[0]).toMatchObject({ name: 'Anna Andersson', city: 'Göteborg' }) + // ...and the unreadable ciphertext was neither stored over nor cleared. + expect(captured.update[0]).not.toHaveProperty('personal_number') + expect(body.data.personal_number).toBe(UNDECRYPTABLE_MASK) + }) + + it('replaces an undecryptable value when the user types a real personnummer', async () => { + // The repair path, and the only "backfill" that can exist: nothing can + // recover the unreadable ciphertext, but the user can overwrite it. + queryResult = { + data: { + id: 'customer-1', + customer_type: 'individual', + personal_number: encryptPersonnummer(PERSONAL_NUMBER), + }, + error: null, + } + + const response = await PATCH( + createMockRequest('/api/customers/customer-1', { + method: 'PATCH', + body: { personal_number: PERSONAL_NUMBER }, + }), + routeParams, + ) + + expect(response.status).toBe(200) + const written = (captured.update[0] as CustomerWrite).personal_number as string + expect(written).toMatch(CIPHERTEXT_SHAPE) + expect(decryptPersonnummer(written)).toBe(PERSONAL_NUMBER) + }) + + it('rejects the undecryptable placeholder on create', async () => { + // Same rule as the '-1234' mask: on create there is no stored value to + // preserve, so a mask is a client error. + const response = await POST( + createMockRequest('/api/customers', { + method: 'POST', + body: { + name: 'Anna Andersson', + customer_type: 'individual', + personal_number: UNDECRYPTABLE_MASK, + }, + }), + { params: Promise.resolve({}) }, + ) + + expect(response.status).toBe(400) + expect(captured.insert).toHaveLength(0) + }) + it('does not treat a masked value as a personal number on a corporate customer', async () => { queryResult = { data: { id: 'customer-1', customer_type: 'individual', name: 'Anna A' }, diff --git a/app/api/customers/__tests__/reveal-personal-number.test.ts b/app/api/customers/__tests__/reveal-personal-number.test.ts new file mode 100644 index 00000000..805f61f0 --- /dev/null +++ b/app/api/customers/__tests__/reveal-personal-number.test.ts @@ -0,0 +1,171 @@ +/** + * GET /api/customers/[id]/personal-number: the drill-in behind the mask. + * + * Every other customer read surface returns '********-1234'. Without this + * endpoint the field was write-only by construction: a user could store a + * personnummer and had no way to check what had actually been stored, which is + * what made an unreadable value indistinguishable from a rendering fault. + * + * What these tests pin: + * - auth and tenancy run before anything is decrypted + * - a stored ciphertext comes back as the full personnummer + * - an undecryptable row answers with its own code, not a 500, because the + * user can fix it in one step by typing the number in again + */ +import { NextResponse } from 'next/server' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { createMockRequest, parseJsonResponse } from '@/tests/helpers' +import { encryptPersonnummer } from '@/lib/salary/personnummer' + +let queryResult: { data: unknown; error: unknown } = { data: null, error: null } + +const buildChain = (): unknown => + new Proxy( + {}, + { + get(_target, prop) { + if (prop === 'then') { + return (resolve: (value: unknown) => void) => resolve(queryResult) + } + return () => buildChain() + }, + }, + ) + +const supabase = { from: vi.fn(() => buildChain()) } + +const requireAuthMock = vi.fn() +vi.mock('@/lib/auth/require-auth', () => ({ + requireAuth: (...args: unknown[]) => requireAuthMock(...args), +})) + +vi.mock('@/lib/company/context', () => ({ + getActiveCompanyId: vi.fn().mockResolvedValue('company-1'), + requireCompanyId: vi.fn().mockResolvedValue('company-1'), +})) + +const requireWriteMock = vi.fn() +vi.mock('@/lib/auth/require-write', () => ({ + requireWritePermission: (...args: unknown[]) => requireWriteMock(...args), +})) + +vi.mock('@/lib/init', () => ({ ensureInitialized: vi.fn() })) + +import { GET } from '../[id]/personal-number/route' + +// Synthetic personnummer, never a real one. +const PERSONAL_NUMBER = '19900101-1234' + +// Hex of the shape customers_personal_number_check accepts that is NOT valid +// ciphertext: the GCM auth tag can never verify. +const GARBAGE_HEX = 'ab'.repeat(40) + +describe('GET /api/customers/[id]/personal-number', () => { + const routeParams = { params: Promise.resolve({ id: 'customer-1' }) } + const request = () => createMockRequest('/api/customers/customer-1/personal-number') + + beforeEach(() => { + vi.clearAllMocks() + queryResult = { data: null, error: null } + requireAuthMock.mockResolvedValue({ user: { id: 'user-1' }, supabase }) + requireWriteMock.mockResolvedValue({ ok: true }) + }) + + it('returns 403 for a viewer, who keeps seeing the mask', async () => { + // The drill-in retires the `no_full_value_read_endpoint` safeguard in + // .compliance/ropa.yaml, so it stays as narrow as the purpose allows: the + // person who needs to verify a personnummer is the one who can correct it. + requireWriteMock.mockResolvedValue({ + ok: false, + response: NextResponse.json( + { error: 'Du har endast läsbehörighet i detta företag.' }, + { status: 403 }, + ), + }) + queryResult = { + data: { id: 'customer-1', personal_number: encryptPersonnummer(PERSONAL_NUMBER) }, + error: null, + } + + const response = await GET(request(), routeParams) + + expect(response.status).toBe(403) + expect(await response.text()).not.toContain(PERSONAL_NUMBER) + }) + + it('returns 401 when unauthenticated, before touching the row', async () => { + requireAuthMock.mockResolvedValue({ + error: NextResponse.json({ error: 'Unauthorized' }, { status: 401 }), + }) + + const response = await GET(request(), routeParams) + + expect(response.status).toBe(401) + expect(supabase.from).not.toHaveBeenCalled() + }) + + it('returns the full personnummer for a stored ciphertext', async () => { + queryResult = { + data: { id: 'customer-1', personal_number: encryptPersonnummer(PERSONAL_NUMBER) }, + error: null, + } + + const response = await GET(request(), routeParams) + const { status, body } = await parseJsonResponse<{ data: { personal_number: string } }>(response) + + expect(status).toBe(200) + expect(body.data.personal_number).toBe(PERSONAL_NUMBER) + }) + + it('returns a legacy plaintext value unchanged', async () => { + queryResult = { data: { id: 'customer-1', personal_number: '900101-1234' }, error: null } + + const { status, body } = await parseJsonResponse<{ data: { personal_number: string } }>( + await GET(request(), routeParams), + ) + + expect(status).toBe(200) + expect(body.data.personal_number).toBe('900101-1234') + }) + + it('returns 404 when the customer does not exist in the active company', async () => { + queryResult = { data: null, error: { code: 'PGRST116', message: 'No rows returned' } } + + const response = await GET(request(), { params: Promise.resolve({ id: 'missing' }) }) + + expect(response.status).toBe(404) + }) + + it('returns 404 when the customer has no stored personnummer', async () => { + queryResult = { data: { id: 'customer-1', personal_number: null }, error: null } + + const { status, body } = await parseJsonResponse<{ error: { code: string } }>( + await GET(request(), routeParams), + ) + + expect(status).toBe(404) + expect(body.error.code).toBe('CUSTOMER_NO_PERSONAL_NUMBER') + }) + + it('answers with a specific code, not a 500, when the value cannot be decrypted', async () => { + // The row that renders as '********-????'. Retrying never helps, so this + // must not look transient: the UI turns this code into "type it in again". + queryResult = { data: { id: 'customer-1', personal_number: GARBAGE_HEX }, error: null } + + const { status, body } = await parseJsonResponse<{ error: { code: string } }>( + await GET(request(), routeParams), + ) + + expect(status).toBe(422) + expect(body.error.code).toBe('CUSTOMER_PERSONAL_NUMBER_UNREADABLE') + }) + + it('never leaks the stored ciphertext, whatever the outcome', async () => { + const stored = encryptPersonnummer(PERSONAL_NUMBER) + queryResult = { data: { id: 'customer-1', personal_number: stored }, error: null } + + const raw = await (await GET(request(), routeParams)).text() + + expect(raw).not.toContain(stored) + }) +}) diff --git a/app/api/customers/route.ts b/app/api/customers/route.ts index 9cf07b2f..b033666f 100644 --- a/app/api/customers/route.ts +++ b/app/api/customers/route.ts @@ -8,6 +8,7 @@ import { withRouteContext } from '@/lib/api/with-route-context' import { errorResponse, errorResponseFromCode } from '@/lib/errors/get-structured-error' import type { Customer } from '@/types' import { encryptCustomerPersonalNumber, maskCustomerRow } from '@/lib/customers/protect-personal-number' +import { fetchAllRows } from '@/lib/supabase/fetch-all' import { getErrorMessage as getUserErrorMessage } from '@/lib/errors/get-error-message' ensureInitialized() @@ -17,18 +18,30 @@ export const GET = withRouteContext( async (_request, ctx) => { const { supabase, companyId, log, requestId } = ctx - const { data, error } = await supabase - .from('customers') - .select('*') - .eq('company_id', companyId) - .order('name', { ascending: true }) - - if (error) { - log.error('customer list failed', error) + // Paginated: PostgREST caps an unranged select at 1000 rows, which would + // hand the roster page a silently truncated customer list. Ordered on the + // PK because paging is only stable under a unique total order; the + // name sort callers expect is re-applied below. + let rows: Customer[] + try { + rows = await fetchAllRows( + ({ from, to }) => + supabase + .from('customers') + .select('*') + .eq('company_id', companyId) + .order('id', { ascending: true }) + .range(from, to), + { dedupeBy: (row) => row.id }, + ) + } catch (error) { + log.error('customer list failed', error as Error) return errorResponse(error, log, { requestId }) } - return NextResponse.json({ data: (data ?? []).map(maskCustomerRow) }) + rows.sort((a, b) => (a.name ?? '').localeCompare(b.name ?? '', 'sv')) + + return NextResponse.json({ data: rows.map(maskCustomerRow) }) }, ) diff --git a/app/api/extensions/enable-banking/callback/__tests__/route.test.ts b/app/api/extensions/enable-banking/callback/__tests__/route.test.ts index 08f4a05f..45745431 100644 --- a/app/api/extensions/enable-banking/callback/__tests__/route.test.ts +++ b/app/api/extensions/enable-banking/callback/__tests__/route.test.ts @@ -31,7 +31,18 @@ const CURRENCY_DEFAULTS: Record = { vi.mock('@/lib/cash-accounts/service', () => ({ upsertFromPsd2: (...args: unknown[]) => mockUpsertFromPsd2(...args), - allocatePsd2LedgerAccount: (...args: unknown[]) => mockAllocate(...args), + // The route resolves ledgers through resolvePsd2LedgerAccount (IBAN match + // first, allocation second). mockAllocate remains the allocation stand-in; + // the wrapper puts its answer in the resolver's envelope so the existing + // "did we allocate?" assertions keep their meaning. Tests that exercise the + // IBAN path override resolvePsd2LedgerAccount's outcome via mockAllocate's + // own implementation. + resolvePsd2LedgerAccount: async (...args: unknown[]) => { + const ledgerAccount = await mockAllocate(...args) + if (!ledgerAccount) return null + if (typeof ledgerAccount === 'object') return ledgerAccount + return { ledgerAccount, reuseCashAccountId: null, source: 'allocated' } + }, defaultLedgerForCurrency: (currency: string) => CURRENCY_DEFAULTS[currency.toUpperCase()] ?? '1930', })) @@ -251,6 +262,70 @@ describe('GET /api/extensions/enable-banking/callback', () => { ).toBe('1935') }) + it('reuses the mapping of a known IBAN when the bank returns a new account uid', async () => { + // The reconnect case behind the reported bug: the ASPSP minted a fresh + // account uid, so the (connection, uid) lookup finds nothing and the old + // behavior allocated an overflow slot, silently moving the user's 1930 + // mapping. Matching on IBAN has to bring both the ledger and the existing + // row along. + mockFrom.mockImplementation((table: string) => { + if (table === 'cash_accounts') { + // Nothing mirrored under the NEW uid. + return mockChain({ data: [], error: null }) + } + const chain: Record = {} + chain.update = vi.fn(() => chain) + chain.eq = vi.fn().mockReturnValue(chain) + chain.select = vi.fn().mockReturnValue(chain) + chain.in = vi.fn().mockReturnValue(chain) + chain.single = vi.fn().mockResolvedValue({ + data: { + id: 'conn-1', + bank_name: 'TestBank', + company_id: 'company-1', + user_id: 'user-1', + status: 'expired', + }, + error: null, + }) + chain.then = (resolve: (v: unknown) => void) => resolve({ data: null, error: null }) + return chain + }) + + mockAllocate.mockResolvedValue({ + ledgerAccount: '1930', + reuseCashAccountId: 'cash-row-1', + source: 'iban', + }) + + mockCreateSession.mockResolvedValue({ + session_id: 'sess-2', + accounts: [ + { uid: 'acc-new', account_id: { iban: 'SE1234' }, name: 'Företagskonto', currency: 'SEK' }, + ], + access: { valid_until: '2024-12-31T00:00:00Z' }, + aspsp: { name: 'TestBank', country: 'SE' }, + }) + + const response = await GET(makeRequest({ code: 'auth-code', state: 'valid-state' })) + expect(response.status).toBe(200) + // Reading the body drives the stream, which is what awaits the finalize + // work the assertions below inspect. + await response.text() + + expect(mockUpsertFromPsd2).toHaveBeenCalledTimes(1) + const mirrored = mockUpsertFromPsd2.mock.calls[0][2] as { + ledger_account: string + reuse_cash_account_id: string | null + external_uid: string + } + expect(mirrored.ledger_account).toBe('1930') + // The existing row is promoted, not duplicated: it keeps its linked + // transactions and picks up the new uid. + expect(mirrored.reuse_cash_account_id).toBe('cash-row-1') + expect(mirrored.external_uid).toBe('acc-new') + }) + it('deletes the fresh row and streams an error redirect when the session exchange fails', async () => { const deleteCalls: unknown[] = [] const updateCalls: unknown[] = [] diff --git a/app/api/extensions/enable-banking/callback/route.ts b/app/api/extensions/enable-banking/callback/route.ts index 33f520a2..dbd0b91b 100644 --- a/app/api/extensions/enable-banking/callback/route.ts +++ b/app/api/extensions/enable-banking/callback/route.ts @@ -8,7 +8,7 @@ import type { StoredAccount } from '@/extensions/general/enable-banking/types' import { eventBus } from '@/lib/events/bus' import { upsertFromPsd2, - allocatePsd2LedgerAccount, + resolvePsd2LedgerAccount, defaultLedgerForCurrency, } from '@/lib/cash-accounts/service' import { renderFinalizeShell, renderFinalizeRedirect } from './finalize-page' @@ -340,11 +340,13 @@ async function finalizeConnection( } // Mirror each PSD2 account into cash_accounts so routing decisions read - // from the canonical entity table. Accounts already mirrored (reconnect) - // keep their ledger_account — re-deriving it here would clobber the - // user's remaps. New accounts each get a free BAS class-19 slot: a bank - // returning N same-currency accounts must not collide on the UNIQUE - // (company_id, ledger_account) constraint by all defaulting to 1930. + // from the canonical entity table. Accounts already mirrored under the same + // (connection, uid) keep their ledger_account — re-deriving it here would + // clobber the user's remaps. Everything else goes through + // resolvePsd2LedgerAccount, which matches on IBAN before allocating: a + // re-authorization that mints new account uids, and a fresh connect that + // mints a whole new connection row, both have to land back on the mapping + // the user already chose instead of overflowing into the next free slots. const { data: mirroredRows } = await supabase .from('cash_accounts') .select('external_uid, ledger_account') @@ -360,13 +362,28 @@ async function finalizeConnection( for (const account of accountsMetadata) { let targetLedger = existingLedgerByUid.get(account.uid) + let reuseCashAccountId: string | null = null if (!targetLedger) { - targetLedger = - (await allocatePsd2LedgerAccount(supabase, updatedConnection.company_id, updatedConnection.user_id, { + const resolved = await resolvePsd2LedgerAccount( + supabase, + updatedConnection.company_id, + updatedConnection.user_id, + { + iban: account.iban, currency: account.currency, accountName: account.name, exclude: assignedLedgers, - })) ?? defaultLedgerForCurrency(account.currency) + }, + ) + targetLedger = resolved?.ledgerAccount ?? defaultLedgerForCurrency(account.currency) + reuseCashAccountId = resolved?.reuseCashAccountId ?? null + if (resolved?.source === 'iban') { + console.log('[enable-banking] Reused existing ledger mapping for known IBAN', { + connectionId: updatedConnection.id, + uid: account.uid, + ledgerAccount: targetLedger, + }) + } } assignedLedgers.add(targetLedger) if (account.ledger_account !== targetLedger) { @@ -382,6 +399,7 @@ async function finalizeConnection( iban: account.iban ?? null, name: account.name ?? null, enabled: account.enabled ?? true, + reuse_cash_account_id: reuseCashAccountId, }) } catch (cashErr) { const reason = cashErr instanceof Error ? cashErr.message : String(cashErr) diff --git a/app/api/extensions/enable-banking/sync/cron/__tests__/route.test.ts b/app/api/extensions/enable-banking/sync/cron/__tests__/route.test.ts new file mode 100644 index 00000000..792d79fa --- /dev/null +++ b/app/api/extensions/enable-banking/sync/cron/__tests__/route.test.ts @@ -0,0 +1,261 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +/** + * Covers the session health probe added to the daily bank sync. + * + * Before it, a connection only ever left 'active' by failing a transaction + * fetch, so a session killed bank-side (several ASPSPs drop the previous AIS + * session when the same PSU authorizes again) kept rendering as healthy with a + * stale last_synced_at. Connections the sync loop skips (capability gate, every + * account deselected) and connections parked in 'pending_selection' were never + * checked at all. + */ + +interface ClientState { + active: Record[] + probeCandidates: Record[] + updates: { id: unknown; payload: Record }[] +} + +const mocks = vi.hoisted(() => ({ + createClient: vi.fn(), + probeSessionHealth: vi.fn(), + syncAccountTransactions: vi.fn(), + hasCapability: vi.fn(), + runReconciliation: vi.fn(), +})) + +vi.mock('@supabase/supabase-js', () => ({ + createClient: mocks.createClient, +})) + +vi.mock('@/lib/auth/cron', () => ({ + verifyCronSecret: vi.fn(() => null), +})) + +vi.mock('@/lib/init', () => ({ + ensureInitialized: vi.fn(), +})) + +vi.mock('@/extensions/general/enable-banking/lib/sync', () => ({ + syncAccountTransactions: (...args: unknown[]) => mocks.syncAccountTransactions(...args), +})) + +vi.mock('@/lib/entitlements/has-capability', () => ({ + hasCapability: (...args: unknown[]) => mocks.hasCapability(...args), +})) + +vi.mock('@/lib/reconciliation/bank-reconciliation', () => ({ + runReconciliation: (...args: unknown[]) => mocks.runReconciliation(...args), + DEFAULT_UNATTENDED_CONFIDENCE_THRESHOLD: 0.9, +})) + +vi.mock('@/lib/email/service', () => ({ + getEmailService: () => ({ isConfigured: () => false, sendEmail: vi.fn() }), +})) + +vi.mock('@/lib/branding/service', () => ({ + getBranding: () => ({ appName: 'Accounted' }), +})) + +// Partial mock: the route also imports the real message constants and the +// consent-expiry helpers, and only the probe needs stubbing. +vi.mock('@/extensions/general/enable-banking/lib/api-client', async () => { + const actual = await vi.importActual< + typeof import('@/extensions/general/enable-banking/lib/api-client') + >('@/extensions/general/enable-banking/lib/api-client') + return { + ...actual, + probeSessionHealth: (...args: unknown[]) => mocks.probeSessionHealth(...args), + } +}) + +import { REAUTH_REQUIRED_MESSAGE } from '@/extensions/general/enable-banking/lib/api-client' +import { GET } from '../route' + +function makeClient(state: ClientState) { + return { + from: () => { + const filters: Record = {} + let isDelete = false + let updatePayload: Record | null = null + + function result() { + if (isDelete) return { data: [], error: null } + if (updatePayload) { + state.updates.push({ id: filters.id, payload: updatePayload }) + return { data: null, error: null } + } + // The sync loop asks for status = 'active'; the probe pass asks for + // status IN ('active','pending_selection'). + if (filters['in:status']) return { data: state.probeCandidates, error: null } + if (filters.status === 'active') return { data: state.active, error: null } + return { data: null, error: null } + } + + const chain: Record = {} + const passthrough = ['select', 'not', 'lt', 'gte', 'order', 'limit'] + for (const method of passthrough) chain[method] = vi.fn(() => chain) + chain.eq = vi.fn((col: string, value: unknown) => { + filters[col] = value + return chain + }) + chain.in = vi.fn((col: string, values: unknown) => { + filters[`in:${col}`] = values + return chain + }) + chain.delete = vi.fn(() => { + isDelete = true + return chain + }) + chain.update = vi.fn((payload: Record) => { + updatePayload = payload + return chain + }) + chain.maybeSingle = vi.fn(() => Promise.resolve({ data: null, error: null })) + chain.then = (onFulfilled: (value: unknown) => unknown) => + Promise.resolve(result()).then(onFulfilled) + return chain + }, + auth: { admin: { getUserById: vi.fn().mockResolvedValue({ data: { user: null } }) } }, + } +} + +function connection(overrides: Record = {}) { + return { + id: 'conn-1', + company_id: 'company-1', + user_id: 'user-1', + bank_name: 'TestBank', + session_id: 'sess-1', + status: 'active', + consent_expires: '2099-01-01T00:00:00Z', + accounts_data: [{ uid: 'acc-1', currency: 'SEK', enabled: true }], + initial_sync_completed_at: '2026-01-01T00:00:00Z', + last_expiry_notification_at: null, + error_message: null, + ...overrides, + } +} + +let state: ClientState + +const originalUrl = process.env.NEXT_PUBLIC_SUPABASE_URL +const originalServiceKey = process.env.SUPABASE_SERVICE_ROLE_KEY + +beforeEach(() => { + vi.clearAllMocks() + process.env.NEXT_PUBLIC_SUPABASE_URL = 'https://example.supabase.co' + process.env.SUPABASE_SERVICE_ROLE_KEY = 'service-key' + state = { active: [], probeCandidates: [], updates: [] } + mocks.createClient.mockImplementation(() => makeClient(state)) + mocks.hasCapability.mockResolvedValue(true) + mocks.syncAccountTransactions.mockResolvedValue({ imported: 0, duplicates: 0, errors: 0 }) + mocks.probeSessionHealth.mockResolvedValue('unknown') +}) + +afterEach(() => { + process.env.NEXT_PUBLIC_SUPABASE_URL = originalUrl + process.env.SUPABASE_SERVICE_ROLE_KEY = originalServiceKey +}) + +function cronRequest(): Request { + return new Request('http://localhost:3000/api/extensions/enable-banking/sync/cron') +} + +describe('GET /api/extensions/enable-banking/sync/cron: session health probe', () => { + it('expires a connection whose session the bank has killed', async () => { + state.probeCandidates = [connection()] + mocks.probeSessionHealth.mockResolvedValue('dead') + + const response = await GET(cronRequest()) + + expect(response.status).toBe(200) + await expect(response.json()).resolves.toMatchObject({ probedDead: 1 }) + expect(state.updates).toEqual([ + { + id: 'conn-1', + payload: { status: 'expired', error_message: REAUTH_REQUIRED_MESSAGE }, + }, + ]) + }) + + it('probes a connection parked in pending_selection, which the sync loop never touches', async () => { + state.probeCandidates = [connection({ status: 'pending_selection', last_synced_at: null })] + mocks.probeSessionHealth.mockResolvedValue('dead') + + await GET(cronRequest()) + + expect(mocks.probeSessionHealth).toHaveBeenCalledWith('sess-1') + expect(state.updates[0].payload).toMatchObject({ status: 'expired' }) + }) + + it('leaves the connection alone when the probe is inconclusive', async () => { + // Flipping a live connection to expired costs the user a full BankID + // re-authorization, so only a definite 'dead' may act. + state.probeCandidates = [connection()] + mocks.probeSessionHealth.mockResolvedValue('unknown') + + await GET(cronRequest()) + + expect(state.updates).toHaveLength(0) + }) + + it('leaves the connection alone when the session is alive', async () => { + state.probeCandidates = [connection()] + mocks.probeSessionHealth.mockResolvedValue('alive') + + await GET(cronRequest()) + + expect(state.updates).toHaveLength(0) + }) + + it('does not probe a connection the sync loop just proved alive', async () => { + // A successful transaction fetch is stronger evidence than the probe, and + // the extra call would burn the ASPSP's per-consent request budget. + state.active = [connection()] + state.probeCandidates = [connection()] + + await GET(cronRequest()) + + expect(mocks.syncAccountTransactions).toHaveBeenCalledTimes(1) + expect(mocks.probeSessionHealth).not.toHaveBeenCalled() + }) + + it('probes a connection the capability gate skipped instead of leaving it "Aktiv"', async () => { + // The silent skip that let a dead connection sit at 'active' for days. + state.active = [connection()] + state.probeCandidates = [connection()] + mocks.hasCapability.mockResolvedValue(false) + mocks.probeSessionHealth.mockResolvedValue('dead') + + await GET(cronRequest()) + + expect(mocks.syncAccountTransactions).not.toHaveBeenCalled() + expect(mocks.probeSessionHealth).toHaveBeenCalledWith('sess-1') + expect(state.updates[0].payload).toMatchObject({ status: 'expired' }) + }) + + it('probes a connection whose accounts are all deselected', async () => { + // This branch reports 'synced' without writing last_synced_at, so the row + // looks fresh forever. + state.active = [connection({ accounts_data: [{ uid: 'acc-1', currency: 'SEK', enabled: false }] })] + state.probeCandidates = [connection()] + mocks.probeSessionHealth.mockResolvedValue('dead') + + await GET(cronRequest()) + + expect(mocks.syncAccountTransactions).not.toHaveBeenCalled() + expect(state.updates[0].payload).toMatchObject({ status: 'expired' }) + }) + + it('runs the probe even when there is nothing to sync', async () => { + state.active = [] + state.probeCandidates = [connection({ status: 'pending_selection' })] + mocks.probeSessionHealth.mockResolvedValue('dead') + + const response = await GET(cronRequest()) + + await expect(response.json()).resolves.toMatchObject({ processed: 0, probedDead: 1 }) + }) +}) diff --git a/app/api/extensions/enable-banking/sync/cron/route.ts b/app/api/extensions/enable-banking/sync/cron/route.ts index c3cd9509..6dc88e93 100644 --- a/app/api/extensions/enable-banking/sync/cron/route.ts +++ b/app/api/extensions/enable-banking/sync/cron/route.ts @@ -8,6 +8,7 @@ import { import { isConsentExpiringSoon, getDaysUntilExpiry, + probeSessionHealth, SessionExpiredError, REAUTH_REQUIRED_MESSAGE, SYNC_FAILED_MESSAGE, @@ -78,10 +79,9 @@ export const GET = withCronContext('cron.bank_sync', async (_request, ctx) => { return errorResponse(connError, ctx.log, { requestId: ctx.requestId }) } - if (!connections || connections.length === 0) { - return NextResponse.json({ message: 'No active connections to sync', processed: 0 }) - } - + // No early return on an empty set: the health probe below still has work to + // do (a company whose only connection is parked in 'pending_selection' has + // nothing to sync but can absolutely have a dead session). const startTime = Date.now() const TIME_BUDGET_MS = 50_000 // 50s: leave 10s margin for Vercel timeout const baseUrl = process.env.NEXT_PUBLIC_APP_URL || 'http://localhost:3000' @@ -93,11 +93,14 @@ export const GET = withCronContext('cron.bank_sync', async (_request, ctx) => { imported: number duplicates: number errors: number - status: 'synced' | 'expired' | 'expiring_soon' | 'error' + // 'skipped' = nothing was fetched from the bank (every account deselected), + // so this run proves nothing about whether the session is still alive. Kept + // distinct from 'synced' because the health probe below keys on it. + status: 'synced' | 'skipped' | 'expired' | 'expiring_soon' | 'error' daysUntilExpiry?: number | null }[] = [] - for (const connection of connections) { + for (const connection of connections ?? []) { if (Date.now() - startTime > TIME_BUDGET_MS) { ctx.log.info('time budget reached', { processedSoFar: results.length }) break @@ -180,7 +183,7 @@ export const GET = withCronContext('cron.bank_sync', async (_request, ctx) => { imported: 0, duplicates: 0, errors: 0, - status: 'synced', + status: 'skipped', daysUntilExpiry: daysLeft, }) continue @@ -321,6 +324,81 @@ export const GET = withCronContext('cron.bank_sync', async (_request, ctx) => { } } + // Health probe for connections this run did NOT prove alive by syncing them. + // + // A sync failure is the only thing that used to move a connection off + // 'active', which leaves two silent holes: connections the loop skipped + // (capability not entitled, every account deselected, time budget reached) + // and connections that never sync at all because they are still parked in + // 'pending_selection'. Both kept rendering as healthy with a stale + // last_synced_at while their PSD2 session was already dead bank-side, so the + // user read old balances as current. Probing costs one cheap session call + // per connection and only ever acts on a definite 'dead'. + const probeResults: { connectionId: string; bankName: string }[] = [] + const PROBE_BUDGET_MS = 100_000 + const provenAlive = new Set( + results.filter(r => r.status === 'synced' || r.status === 'expiring_soon').map(r => r.connectionId) + ) + + const { data: unverified, error: unverifiedError } = await supabase + .from('bank_connections') + .select('id, company_id, user_id, bank_name, session_id, status, last_expiry_notification_at') + .in('status', ['active', 'pending_selection']) + .not('session_id', 'is', null) + .order('last_synced_at', { ascending: true, nullsFirst: true }) + .limit(100) + + if (unverifiedError) { + ctx.log.error('failed to fetch connections for health probe', unverifiedError, { + message: unverifiedError.message, + }) + } + + for (const connection of unverified ?? []) { + if (Date.now() - startTime > PROBE_BUDGET_MS) { + ctx.log.info('probe budget reached', { probedSoFar: probeResults.length }) + break + } + if (provenAlive.has(connection.id)) continue + + // Per-connection isolation, matching the sync loop above: without it a + // single network blip aborts probing for every remaining candidate in the + // batch and the coverage gap stays silent until tomorrow's run. + try { + const health = await probeSessionHealth(connection.session_id as string) + if (health !== 'dead') continue + + const { error: updateError } = await supabase + .from('bank_connections') + .update({ status: 'expired', error_message: REAUTH_REQUIRED_MESSAGE }) + .eq('id', connection.id) + + // Only claim the connection was marked dead once the write landed. + // Notifying (and counting) on an unpersisted update would tell the user + // to re-authorize while the row still reads 'active'. + if (updateError) { + ctx.log.error('failed to mark probed-dead connection as expired', updateError, { + connectionId: connection.id, + }) + continue + } + + await sendConsentExpiryNotification(supabase, connection, 0, true, baseUrl) + + ctx.log.info('health probe found a dead session', { + connectionId: connection.id, + bankName: connection.bank_name, + previousStatus: connection.status, + }) + probeResults.push({ connectionId: connection.id, bankName: connection.bank_name }) + } catch (err) { + ctx.log.error('health probe failed for connection', err as Error, { + connectionId: connection.id, + bankName: connection.bank_name, + }) + } + } + const totalImported = results.reduce((sum, r) => sum + r.imported, 0) const totalExpired = results.filter(r => r.status === 'expired').length const totalExpiringSoon = results.filter(r => r.status === 'expiring_soon').length @@ -332,6 +410,7 @@ export const GET = withCronContext('cron.bank_sync', async (_request, ctx) => { totalExpired, totalExpiringSoon, totalFailed, + probedDead: probeResults.length, }) return NextResponse.json({ @@ -340,6 +419,8 @@ export const GET = withCronContext('cron.bank_sync', async (_request, ctx) => { totalExpired, totalExpiringSoon, totalFailed, + probedDead: probeResults.length, + probeResults, results, }) }) diff --git a/app/api/invoices/__tests__/route.test.ts b/app/api/invoices/__tests__/route.test.ts index 44d8e012..b424a83e 100644 --- a/app/api/invoices/__tests__/route.test.ts +++ b/app/api/invoices/__tests__/route.test.ts @@ -80,6 +80,43 @@ describe('GET /api/invoices', () => { expect(body.count).toBe(2) }) + it('masks the embedded customer personnummer in the list', async () => { + // The customer:customers(*) join carries the stored personal_number out to + // the browser. A legacy plaintext value is used here so the assertion does + // not depend on PERSONNUMMER_ENCRYPTION_KEY being set in the test env; the + // ciphertext path lands on the same masked shape. + const invoices = [ + { ...makeInvoice(), customer: { id: 'cust-1', name: 'Test', personal_number: '19900101-1234' } }, + ] + enqueue({ data: invoices, error: null, count: 1 }) + + const request = createMockRequest('/api/invoices') + const response = await GET(request) + const { status, body } = await parseJsonResponse<{ + data: { customer: { personal_number: string | null; name: string } }[] + }>(response) + + expect(status).toBe(200) + expect(body.data[0].customer.personal_number).toBe('********-1234') + expect(JSON.stringify(body)).not.toContain('19900101-1234') + // Masking must not strip the rest of the embed. + expect(body.data[0].customer.name).toBe('Test') + }) + + it('leaves an invoice without an embedded customer untouched', async () => { + // PostgREST returns customer: null when the customer was removed; the mask + // must be null-safe rather than 500 the whole list. + const invoices = [{ ...makeInvoice(), customer: null }] + enqueue({ data: invoices, error: null, count: 1 }) + + const request = createMockRequest('/api/invoices') + const response = await GET(request) + const { status, body } = await parseJsonResponse<{ data: { customer: null }[] }>(response) + + expect(status).toBe(200) + expect(body.data[0].customer).toBeNull() + }) + it('applies status filter', async () => { enqueue({ data: [], error: null, count: 0 }) diff --git a/app/api/invoices/route.ts b/app/api/invoices/route.ts index 4a05758f..85068cee 100644 --- a/app/api/invoices/route.ts +++ b/app/api/invoices/route.ts @@ -11,6 +11,7 @@ import { withRouteContext } from '@/lib/api/with-route-context' import { errorResponse, errorResponseFromCode } from '@/lib/errors/get-structured-error' import type { Logger } from '@/lib/logger' import { getErrorMessage as getUserErrorMessage } from '@/lib/errors/get-error-message' +import { maskEmbeddedCustomer } from '@/lib/customers/protect-personal-number' ensureInitialized() @@ -42,7 +43,9 @@ export const GET = withRouteContext( return errorResponse(error, log, { requestId }) } - return NextResponse.json({ data, count }) + // Mask the embedded customer's personnummer: the customers(*) join + // carries the stored ciphertext, which has no business reaching a client. + return NextResponse.json({ data: (data ?? []).map(maskEmbeddedCustomer), count }) }, ) @@ -240,7 +243,7 @@ export const POST = withRouteContext( }) } - return NextResponse.json({ data: completeInvoice }) + return NextResponse.json({ data: maskEmbeddedCustomer(completeInvoice) }) }, { requireWrite: true }, ) @@ -346,9 +349,9 @@ async function createCreditNote( requestId, }) } - return NextResponse.json({ data: reopenedCreditNote }) + return NextResponse.json({ data: maskEmbeddedCustomer(reopenedCreditNote) }) } - return NextResponse.json({ data: existingCreditNote }) + return NextResponse.json({ data: maskEmbeddedCustomer(existingCreditNote) }) } const creditNoteNumber = `KR-${originalInvoice.invoice_number}` @@ -403,7 +406,7 @@ async function createCreditNote( .eq('company_id', companyId) .eq('creation_complete', true) .maybeSingle() - if (racedCreditNote) return NextResponse.json({ data: racedCreditNote }) + if (racedCreditNote) return NextResponse.json({ data: maskEmbeddedCustomer(racedCreditNote) }) } log.error('credit note insert failed', creditNoteError) return errorResponseFromCode('INVOICE_CREATE_INSERT_FAILED', log, { @@ -469,5 +472,5 @@ async function createCreditNote( // A credit note is only issued when the user sends it or marks it as sent. // Until then it is a non-editable draft: no journal entry is created and // the original invoice remains in its current state. - return NextResponse.json({ data: completeCreditNote }) + return NextResponse.json({ data: maskEmbeddedCustomer(completeCreditNote) }) } diff --git a/app/api/reports/balansrapport/xlsx/route.ts b/app/api/reports/balansrapport/xlsx/route.ts index 1118bd39..36c525e6 100644 --- a/app/api/reports/balansrapport/xlsx/route.ts +++ b/app/api/reports/balansrapport/xlsx/route.ts @@ -8,6 +8,7 @@ import { currencyColumn, xlsxFilename, } from '@/lib/reports/xlsx-export' +import { formatLatestVouchers, LATEST_VOUCHERS_LABEL } from '@/lib/reports/latest-vouchers-format' import { getErrorMessage as getUserErrorMessage } from '@/lib/errors/get-error-message' interface FlatRow { @@ -83,6 +84,21 @@ export const GET = withRouteContext('report.balansrapport.xlsx', async (request, ub: report.beraknat_resultat, }) + // Reconciliation aid (#1267). reportToWorkbook has no preamble concept, so + // this rides as a first body row, the same way the dimension disclosure + // does on the other report exports. + const vouchersLabel = formatLatestVouchers(report.latest_vouchers) + if (vouchersLabel) { + rows.unshift({ + group: `${LATEST_VOUCHERS_LABEL}: ${vouchersLabel}`, + account_number: '', + account_name: '', + ib: null as unknown as number, + period_change: null as unknown as number, + ub: null as unknown as number, + }) + } + const buffer = reportToWorkbook([ { name: 'Balansrapport', diff --git a/app/api/reports/resultatrapport/xlsx/route.ts b/app/api/reports/resultatrapport/xlsx/route.ts index 7fd221c3..5ff599f5 100644 --- a/app/api/reports/resultatrapport/xlsx/route.ts +++ b/app/api/reports/resultatrapport/xlsx/route.ts @@ -9,6 +9,7 @@ import { currencyColumn, xlsxFilename, } from '@/lib/reports/xlsx-export' +import { formatLatestVouchers, LATEST_VOUCHERS_LABEL } from '@/lib/reports/latest-vouchers-format' import { getErrorMessage as getUserErrorMessage } from '@/lib/errors/get-error-message' interface FlatRow { @@ -88,6 +89,20 @@ export const GET = withRouteContext('report.resultatrapport.xlsx', async (reques prior_period: report.net_result_prior, }) + // Reconciliation aid (#1267). reportToWorkbook has no preamble concept, so + // this rides as a first body row, the same way the disclosure below does. + // Unshifted first so the disclosure, when present, still ends up on top. + const vouchersLabel = formatLatestVouchers(report.latest_vouchers) + if (vouchersLabel) { + rows.unshift({ + group: `${LATEST_VOUCHERS_LABEL}: ${vouchersLabel}`, + account_number: '', + account_name: '', + current_period: null as unknown as number, + prior_period: null as unknown as number, + }) + } + // Partial-view disclosure survives the file boundary: a filtered export // must never be mistakable for the authoritative report (BFNAR 2013:2). const disclosure = dimensionFilterDisclosure(dimFilter.dimensions) diff --git a/app/api/reports/vat-declaration/__tests__/route.test.ts b/app/api/reports/vat-declaration/__tests__/route.test.ts index 6bb5542b..59e8acdd 100644 --- a/app/api/reports/vat-declaration/__tests__/route.test.ts +++ b/app/api/reports/vat-declaration/__tests__/route.test.ts @@ -47,6 +47,20 @@ function makeRequest(query: string) { return new Request(`http://localhost/api/reports/vat-declaration${query}`) } +/** + * chart_of_accounts builder for fetchDynamicRuta05Accounts: which of the + * company's own class 3 accounts carry a "Standard moms" and therefore belong + * in ruta 05. Empty by default, i.e. a plain BAS chart. + */ +function chartBuilder(accounts: Array<{ account_number: string; default_vat_rate: number }> = []) { + const result = { data: accounts, error: null } + const b: Record = {} + for (const m of ['select', 'eq', 'in', 'not', 'order']) b[m] = vi.fn().mockReturnValue(b) + b.range = vi.fn().mockResolvedValue(result) + b.then = (resolve: (v: unknown) => void) => resolve(result) + return b +} + describe('GET /api/reports/vat-declaration', () => { beforeEach(() => { vi.clearAllMocks() @@ -56,6 +70,7 @@ describe('GET /api/reports/vat-declaration', () => { error: null, }) mockSupabase.rpc.mockResolvedValue({ data: rpcPayload(), error: null }) + mockSupabase.from.mockImplementation(() => chartBuilder()) }) it('returns 401 when not authenticated', async () => { @@ -131,9 +146,10 @@ describe('GET /api/reports/vat-declaration', () => { expect(body.data.periodLabel).toBe('Kvartal 3 2026') // Regression guard: the dead company_settings round trip is gone and - // resolvePeriodDates makes no DB call for calendar quarters, so the - // handler issues exactly one PostgREST call: the totals RPC. - expect(mockSupabase.from).not.toHaveBeenCalled() + // resolvePeriodDates makes no DB call for calendar quarters. The only + // table read left is chart_of_accounts, for the company's own ruta 05 + // accounts (#1261). + expect(mockSupabase.from.mock.calls.map(([t]) => t)).toEqual(['chart_of_accounts']) expect(mockSupabase.rpc).toHaveBeenCalledTimes(1) expect(mockSupabase.rpc).toHaveBeenCalledWith( 'get_vat_declaration_totals', @@ -145,7 +161,7 @@ describe('GET /api/reports/vat-declaration', () => { ) }) - it('happy path monthly: no table queries, one RPC', async () => { + it('happy path monthly: no period lookup, one RPC', async () => { const res = await GET( makeRequest('?periodType=monthly&year=2026&period=7'), { params: Promise.resolve({}) }, @@ -155,7 +171,7 @@ describe('GET /api/reports/vat-declaration', () => { expect(body.data.rutor.ruta49).toBe(21800) expect(body.data.periodLabel).toBe('Juli 2026') - expect(mockSupabase.from).not.toHaveBeenCalled() + expect(mockSupabase.from).not.toHaveBeenCalledWith('fiscal_periods') expect(mockSupabase.rpc).toHaveBeenCalledTimes(1) expect(mockSupabase.rpc).toHaveBeenCalledWith( 'get_vat_declaration_totals', diff --git a/app/api/reports/vat-declaration/ruta/[ruta]/sources/__tests__/route.test.ts b/app/api/reports/vat-declaration/ruta/[ruta]/sources/__tests__/route.test.ts index 261969e8..6923d2d4 100644 --- a/app/api/reports/vat-declaration/ruta/[ruta]/sources/__tests__/route.test.ts +++ b/app/api/reports/vat-declaration/ruta/[ruta]/sources/__tests__/route.test.ts @@ -27,23 +27,40 @@ interface SupabaseShape { */ function buildSupabase( linesResult: { data: unknown; error: unknown }, - fiscalPeriodResult: { data: unknown; error: unknown } = { data: null, error: null } + fiscalPeriodResult: { data: unknown; error: unknown } = { data: null, error: null }, + chartAccounts: Array<{ account_number: string; default_vat_rate: number }> = [] ): SupabaseShape { + const chartResult = { data: chartAccounts, error: null } return { rpc: vi.fn().mockResolvedValue(linesResult), - from: vi.fn().mockImplementation(() => ({ - select: vi.fn().mockReturnThis(), - eq: vi.fn().mockReturnThis(), - in: vi.fn().mockReturnThis(), - gte: vi.fn().mockReturnThis(), - lte: vi.fn().mockReturnThis(), - order: vi.fn().mockReturnThis(), - limit: vi.fn().mockReturnThis(), - or: vi.fn().mockReturnThis(), - maybeSingle: vi.fn().mockResolvedValue(fiscalPeriodResult), - range: vi.fn().mockResolvedValue(linesResult), - then: (resolve: (v: unknown) => void) => resolve(linesResult), - })), + from: vi.fn().mockImplementation((table: string) => { + // Ruta 05 also collects the company's own momspliktiga intäktskonton, + // read off chart_of_accounts rather than the fixed ACCOUNT_RUTA map. + if (table === 'chart_of_accounts') { + return { + select: vi.fn().mockReturnThis(), + eq: vi.fn().mockReturnThis(), + in: vi.fn().mockReturnThis(), + not: vi.fn().mockReturnThis(), + order: vi.fn().mockReturnThis(), + range: vi.fn().mockResolvedValue(chartResult), + then: (resolve: (v: unknown) => void) => resolve(chartResult), + } + } + return { + select: vi.fn().mockReturnThis(), + eq: vi.fn().mockReturnThis(), + in: vi.fn().mockReturnThis(), + gte: vi.fn().mockReturnThis(), + lte: vi.fn().mockReturnThis(), + order: vi.fn().mockReturnThis(), + limit: vi.fn().mockReturnThis(), + or: vi.fn().mockReturnThis(), + maybeSingle: vi.fn().mockResolvedValue(fiscalPeriodResult), + range: vi.fn().mockResolvedValue(linesResult), + then: (resolve: (v: unknown) => void) => resolve(linesResult), + } + }), } } @@ -207,6 +224,50 @@ describe('GET /api/reports/vat-declaration/ruta/[ruta]/sources', () => { }) }) +describe('GET /api/reports/vat-declaration/ruta/[ruta]/sources: ruta 05 accounts', () => { + /** The p_accounts array the route handed to get_vat_ruta_source_lines. */ + function rpcAccounts(supabase: SupabaseShape): string[] { + return (supabase.rpc.mock.calls[0][1] as { p_accounts: string[] }).p_accounts + } + + function get(ruta: string) { + const req = createMockRequest( + `/api/reports/vat-declaration/ruta/${ruta}/sources`, + { searchParams: { periodType: 'monthly', year: '2026', period: '5' } } + ) + return GET(req, createMockRouteParams({ ruta })) + } + + it('drills into the company own revenue accounts too (#1261)', async () => { + // Without this the drill-down would list a smaller sum than the ruta 05 + // figure it drills into: the konto feeds the total but not the source list. + const supabase = buildSupabase({ data: [], error: null }, { data: null, error: null }, [ + { account_number: '3013', default_vat_rate: 0.06 }, + ]) + authOk(supabase) + + expect((await get('05')).status).toBe(200) + + const accounts = rpcAccounts(supabase) + expect(accounts).toContain('3013') + expect(accounts).toContain('3001') // static mapping still there + }) + + it('leaves other rutor on the static mapping alone', async () => { + const supabase = buildSupabase({ data: [], error: null }, { data: null, error: null }, [ + { account_number: '3013', default_vat_rate: 0.06 }, + ]) + authOk(supabase) + + expect((await get('10')).status).toBe(200) + + const accounts = rpcAccounts(supabase) + expect(accounts).toContain('2611') + expect(accounts).not.toContain('3013') + expect(supabase.from).not.toHaveBeenCalledWith('chart_of_accounts') + }) +}) + describe('GET /api/reports/vat-declaration/ruta/[ruta]/sources: period resolution', () => { // A first räkenskapsår may run up to 18 months (BFL 3 kap 3 §), and // helårsmoms is filed per räkenskapsår, not per calendar year diff --git a/app/api/reports/vat-declaration/ruta/[ruta]/sources/route.ts b/app/api/reports/vat-declaration/ruta/[ruta]/sources/route.ts index d643fe7e..ce99ae5e 100644 --- a/app/api/reports/vat-declaration/ruta/[ruta]/sources/route.ts +++ b/app/api/reports/vat-declaration/ruta/[ruta]/sources/route.ts @@ -4,6 +4,7 @@ import { ACCOUNT_RUTA, resolvePeriodDates, } from '@/lib/reports/vat-declaration' +import { fetchDynamicRuta05Accounts } from '@/lib/reports/vat-revenue-accounts' import type { ReportSourceLine } from '@/lib/reports/source-lines' import type { VatDeclarationRutor, VatPeriodType } from '@/types' @@ -44,6 +45,14 @@ export const GET = withRouteContext<{ params: Promise<{ ruta: string }> }>( .filter(([, m]) => m.box === rutaKey) .map(([acc]) => acc) + // Ruta 05 also collects the company's own momspliktiga intäktskonton, which + // ACCOUNT_RUTA cannot know about (#1261). Without them the drill-down would + // list a smaller sum than the figure it drills into. + if (rutaKey === 'ruta05') { + const { accounts } = await fetchDynamicRuta05Accounts(supabase, companyId) + accountsForRuta.push(...accounts) + } + if (accountsForRuta.length === 0) { return NextResponse.json( { error: `Ruta ${rutaParam} har inga underliggande konton` }, diff --git a/app/api/transactions/[id]/match-supplier-invoice/__tests__/route.test.ts b/app/api/transactions/[id]/match-supplier-invoice/__tests__/route.test.ts index 0e5d6450..c4d487eb 100644 --- a/app/api/transactions/[id]/match-supplier-invoice/__tests__/route.test.ts +++ b/app/api/transactions/[id]/match-supplier-invoice/__tests__/route.test.ts @@ -16,7 +16,7 @@ vi.mock('@/lib/logger', () => ({ }), })) -const { supabase: mockSupabase, enqueue, reset } = createQueuedMockSupabase() +const { supabase: mockSupabase, enqueue, reset, findCalls } = createQueuedMockSupabase() vi.mock('@/lib/supabase/server', () => ({ createClient: () => Promise.resolve(mockSupabase), })) @@ -454,6 +454,25 @@ describe('POST /api/transactions/[id]/match-supplier-invoice: non-FX paths', () expect(body.remaining_amount).toBe(0) }) + // The suggestion pointer must not survive the match that consumes it: this + // request marks the invoice paid, so a surviving hint would point at a + // settled invoice. The customer-invoice route has always cleared its own + // field; this one did not. + it('clears potential_supplier_invoice_id when it links the matched transaction', async () => { + enqueueHappyPath({ + transaction: { amount: -1000, currency: 'SEK' }, + invoice: { currency: 'SEK', remaining_amount: 1000 }, + }) + await POST(makeReq(), createMockRouteParams({ id: TX_UUID })) + + const txUpdate = findCalls('transactions', 'update').at(-1)?.[0] + expect(txUpdate).toMatchObject({ + supplier_invoice_id: SI_UUID, + potential_supplier_invoice_id: null, + is_business: true, + }) + }) + it('öresavrundning: a whole-krona Bankgiro payment settles an öre-bearing invoice in full via 3740', async () => { // The reported bug: invoice 11 231,25, bank paid 11 231 → previously left // 0,25 stranded as partially_paid. Now → paid, with 0,25 booked to 3740. diff --git a/app/api/transactions/[id]/match-supplier-invoice/route.ts b/app/api/transactions/[id]/match-supplier-invoice/route.ts index 6441e5ce..241dd28c 100644 --- a/app/api/transactions/[id]/match-supplier-invoice/route.ts +++ b/app/api/transactions/[id]/match-supplier-invoice/route.ts @@ -416,6 +416,11 @@ export const POST = withRouteContext( .from('transactions') .update({ supplier_invoice_id, + // Clear the suggestion now that it is a confirmed link, mirroring the + // customer-invoice route. Leaving it set kept a pointer at an invoice + // this very request just marked paid, i.e. the stale-pointer state + // every read path now has to defend against. + potential_supplier_invoice_id: null, journal_entry_id: journalEntryId, is_business: true, }) diff --git a/components/bookkeeping/ActivateAccountsDialog.tsx b/components/bookkeeping/ActivateAccountsDialog.tsx index 597556e1..f009efa2 100644 --- a/components/bookkeeping/ActivateAccountsDialog.tsx +++ b/components/bookkeeping/ActivateAccountsDialog.tsx @@ -30,6 +30,10 @@ interface BasLookupRow { account_number: string account_name: string | null known: boolean + // Present since the lookup learned about the company's own chart: an account + // that is in_chart but not is_active is being reactivated, not added. + in_chart?: boolean + is_active?: boolean } export function ActivateAccountsDialog({ @@ -56,7 +60,15 @@ export function ActivateAccountsDialog({ }) .catch(() => { if (cancelled) return - setRows(accountNumbers.map((n) => ({ account_number: n, account_name: null, known: false }))) + setRows( + accountNumbers.map((n) => ({ + account_number: n, + account_name: null, + known: false, + in_chart: false, + is_active: false, + })), + ) }) .finally(() => { if (!cancelled) setLoading(false) @@ -107,6 +119,11 @@ export function ActivateAccountsDialog({
  • {r.account_number} {r.account_name} + {r.in_chart && !r.is_active && ( + + Aktiveras igen + + )}
    • ))} diff --git a/components/bookkeeping/AddAccountDialog.tsx b/components/bookkeeping/AddAccountDialog.tsx index b29df357..31bc4fc0 100644 --- a/components/bookkeeping/AddAccountDialog.tsx +++ b/components/bookkeeping/AddAccountDialog.tsx @@ -20,10 +20,18 @@ import { classifyAccount } from '@/lib/bookkeeping/account-classifier' import type { BASAccount } from '@/types' import { getErrorMessage as getUserErrorMessage } from '@/lib/errors/get-error-message' +/** + * The create path hands back the full row the API inserted. The reactivate + * path only learns the account number back from /accounts/activate, and the + * stored account is deliberately left untouched, so the rest is unknown here. + * Every host refetches its own list and reads only account_number. + */ +type CreatedAccount = Partial & { account_number: string } + interface AddAccountDialogProps { open: boolean onOpenChange: (open: boolean) => void - onCreated: (account: BASAccount) => void + onCreated: (account: CreatedAccount) => void initialAccountNumber?: string initialAccountName?: string } @@ -45,6 +53,10 @@ export function AddAccountDialog({ const [normalBalance, setNormalBalance] = useState<'debit' | 'credit'>('debit') const [isSaving, setIsSaving] = useState(false) const [error, setError] = useState('') + // Set when the create failed because the number belongs to a deactivated + // account. Creating it can never succeed (the unique constraint counts + // inactive rows), so the dialog offers reactivation instead of a dead end. + const [inactiveConflict, setInactiveConflict] = useState(false) // Apply prefill values whenever the dialog opens. Resetting on close happens // implicitly after a successful create; here we only need to seed inputs so @@ -55,6 +67,7 @@ export function AddAccountDialog({ setAccountNumber(num) setAccountName(initialAccountName ?? '') setError('') + setInactiveConflict(false) if (num.length === 4) { setNormalBalance(classifyAccount(num).normal_balance) } @@ -65,6 +78,7 @@ export function AddAccountDialog({ async function handleCreate() { setError('') + setInactiveConflict(false) if (!/^\d{4}$/.test(accountNumber)) { setError('Kontonumret måste vara exakt 4 siffror') @@ -100,6 +114,8 @@ export function AddAccountDialog({ // route's own Swedish reason. Passing the parsed body plus the status // resolves all three shapes (envelope, bare string, no body). const body = await response.json().catch(() => null) + const code = (body as { error?: { code?: string } } | null)?.error?.code + setInactiveConflict(code === 'ACCOUNT_EXISTS_INACTIVE') setError(getUserErrorMessage(body, { statusCode: response.status })) return } @@ -121,6 +137,41 @@ export function AddAccountDialog({ } } + // Recovery for ACCOUNT_EXISTS_INACTIVE: flip the existing account back on + // instead of trying to insert a second row. The values typed into this form + // are intentionally dropped — the account comes back exactly as it was, and + // renaming it is the kontoplan's job, not a side effect of a failed create. + async function handleReactivate() { + setError('') + setIsSaving(true) + try { + const response = await fetch('/api/bookkeeping/accounts/activate', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ account_numbers: [accountNumber] }), + }) + + if (!response.ok) { + const body = await response.json().catch(() => null) + setError(getUserErrorMessage(body, { statusCode: response.status })) + return + } + + setInactiveConflict(false) + setAccountNumber('') + setAccountName('') + setDescription('') + setDefaultVatRate('none') + setSruCode('') + onCreated({ account_number: accountNumber }) + onOpenChange(false) + } catch (err) { + setError(err instanceof Error ? getUserErrorMessage(err) : 'Något gick fel') + } finally { + setIsSaving(false) + } + } + return ( @@ -149,6 +200,10 @@ export function AddAccountDialog({ onChange={(e) => { const v = e.target.value.replace(/\D/g, '').slice(0, 4) setAccountNumber(v) + // The conflict is about a specific number; editing it makes + // the reactivate offer stale. + setInactiveConflict(false) + setError('') if (v.length === 4) { setNormalBalance(classifyAccount(v).normal_balance) } @@ -220,6 +275,12 @@ export function AddAccountDialog({ 6 % + {accountNumber.length === 4 && accountNumber.startsWith('3') && ( +

    + På intäktskonton avgör satsen också om kontot räknas som + momspliktig försäljning i ruta 05 i momsdeklarationen. +

    + )}
    @@ -231,16 +292,33 @@ export function AddAccountDialog({
    - {error && ( + {error && !inactiveConflict && (

    {error}

    )} + + {inactiveConflict && ( +
    +

    {error}

    + +
    + )} - diff --git a/components/bookkeeping/ChartOfAccountsManager.tsx b/components/bookkeeping/ChartOfAccountsManager.tsx index d56dd349..23bc0db1 100644 --- a/components/bookkeeping/ChartOfAccountsManager.tsx +++ b/components/bookkeeping/ChartOfAccountsManager.tsx @@ -1,6 +1,6 @@ 'use client' -import { Fragment, useState, useEffect, useCallback, useMemo } from 'react' +import { Fragment, useState, useEffect, useCallback, useMemo, useRef } from 'react' import { useTranslations } from 'next-intl' import { Badge } from '@/components/ui/badge' import { Button } from '@/components/ui/button' @@ -74,6 +74,10 @@ export default function ChartOfAccountsManager() { const [collapsedMyClasses, setCollapsedMyClasses] = useState>(new Set()) const [expandedCatalogClasses, setExpandedCatalogClasses] = useState>(new Set()) const [hideK2Excluded, setHideK2Excluded] = useState(null) + // Off by default: the list endpoint's `?active=false` means "no filter", so + // leaving it off keeps first paint on the smaller active-only payload. + // A deactivated account is otherwise invisible everywhere and unrecoverable. + const [showInactive, setShowInactive] = useState(false) // Data state const [accounts, setAccounts] = useState([]) @@ -99,8 +103,19 @@ export default function ChartOfAccountsManager() { // Data fetching // ------------------------------------------- + // Read through a ref, not the state value: making fetchAccounts depend on + // showInactive would put it in the mount effect's dep list and re-run the + // whole blocking load on every toggle (the same double-fetch the comment + // below warns about for hideK2Excluded). + const showInactiveRef = useRef(showInactive) + showInactiveRef.current = showInactive + const fetchAccounts = useCallback(async () => { - const res = await fetch('/api/bookkeeping/accounts') + // `?active=false` disables the filter entirely (active + inactive), it does + // not select inactive rows only — see list_company_accounts. + const res = await fetch( + showInactiveRef.current ? '/api/bookkeeping/accounts?active=false' : '/api/bookkeeping/accounts', + ) const { data } = await res.json() setAccounts(data || []) }, []) @@ -167,6 +182,18 @@ export default function ChartOfAccountsManager() { } }, [fetchAccounts, fetchUsage]) + // Refetch when the inactive filter flips. Skips the first run (the mount + // effect above already fetched) and deliberately does not raise `loading`: + // swapping a filter should not blank the table back to the skeleton. + const showInactiveInitial = useRef(true) + useEffect(() => { + if (showInactiveInitial.current) { + showInactiveInitial.current = false + return + } + void fetchAccounts() + }, [showInactive, fetchAccounts]) + // Loads the BAS catalog and the K2 default on demand, once, the first time // the user opens the "BAS-katalog" tab. const ensureReferenceLoaded = useCallback(async () => { @@ -211,6 +238,22 @@ export default function ChartOfAccountsManager() { // ------------------------------------------- async function toggleActive(account: BASAccount) { + // Deactivating an account that carries postings hides it from the + // kontoplan and from new verifikat while its balances stay in the books. + // That is legal and reversible, but it should never happen silently: it is + // what made a used account unreachable in the first place. The count is + // already in memory from fetchUsage, so no extra request. + const usage = usageCounts.get(account.account_number) ?? 0 + if (account.is_active && usage > 0) { + const confirmed = await confirm({ + title: t('deactivate_confirm_title', { number: account.account_number }), + description: t('deactivate_confirm', { count: usage }), + confirmLabel: t('deactivate_confirm_action'), + variant: 'warning', + }) + if (!confirmed) return + } + setTogglingAccount(account.account_number) try { const res = await fetch(`/api/bookkeeping/accounts/${account.account_number}`, { @@ -273,9 +316,13 @@ export default function ChartOfAccountsManager() { body: JSON.stringify({ account_numbers: [accountNumber] }), }) if (!res.ok) throw new Error(t('toast_activate_failed')) - const { activated } = await res.json() + // `reactivated` covers an account the company already had but had turned + // off. Without it the catalog tab flipped the row back on in silence. + const { activated, reactivated } = await res.json() if (activated > 0) { toast({ title: t('toast_activated_title'), description: t('toast_activated_description', { number: accountNumber }) }) + } else if (reactivated > 0) { + toast({ title: t('toast_reactivated_title'), description: t('toast_reactivated_description', { number: accountNumber }) }) } await refreshAll() } catch { @@ -458,6 +505,16 @@ export default function ChartOfAccountsManager() { className="h-9 pl-10" /> + {view === 'my-accounts' && ( + + )} {view === 'bas-catalog' && (