From fca57dc47054458bfddf151ed176250a2bb22670 Mon Sep 17 00:00:00 2001 From: Mattsson <111893710+mattssonn@users.noreply.github.com> Date: Thu, 27 Aug 2026 19:09:47 +0200 Subject: [PATCH] fix(vat): momsdeklaration defaults respect the configured cadence and persist manual changes (#1998) * fix(vat): momsdeklaration defaults respect the configured cadence and persist manual changes The period picker re-seeded from scratch on every visit: an arsmoms user whose moms_period was never set landed on a silently guessed quarterly declaration (companies without a company_settings row bypassed every gate), and a manually chosen cadence evaporated on the next visit. - Gate the view when no company_settings row exists, matching the existing "registered but no period" gate: a declaration for the wrong period type is a compliance hazard, not a convenience. - Persist the manually chosen cadence per company (localStorage, FyPicker pattern) and restore it while moms_period is unchanged; the concrete period still re-seeds to the most recently ended one, and a changed setting discards the stored cadence. - Extract the seeding decision into lib/vat/period-selection.ts with unit tests. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_012pQn9kC742B9R7Ggi8wdn9 * fix(vat): drop cadence persistence; the moms_period re-seed is the control Skeptic review refuted the persistence half of the previous commit twice: the render-phase localStorage restore diverged from SSR (hydration error on every visit once a cadence was stored), and restoring a manually chosen cadence that deviates from moms_period kept the filing pipeline open on the wrong period type across visits, with no downstream path validating period type against the setting. The redovisningsperiod has exactly one lawful value per company, so the mount-time re-seed from company_settings.moms_period is the self-healing control, not a bug. The settings-row gate and the extracted, tested seeding resolver stay. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_012pQn9kC742B9R7Ggi8wdn9 --------- Co-authored-by: Claude Fable 5 --- DECISIONS.md | 1 + components/reports/views/index.tsx | 57 +++++++++++++++------- lib/vat/__tests__/period-selection.test.ts | 55 +++++++++++++++++++++ lib/vat/period-selection.ts | 51 +++++++++++++++++++ 4 files changed, 147 insertions(+), 17 deletions(-) create mode 100644 lib/vat/__tests__/period-selection.test.ts create mode 100644 lib/vat/period-selection.ts diff --git a/DECISIONS.md b/DECISIONS.md index 2b48de9a..d30f940e 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -1301,4 +1301,5 @@ One line per decision: `[YYYY-MM-DD] : `. Appended by agents and [2026-08-27] Brand domains gate signup server-side (brands.signup_mode + brand_signup_allowlist), not in the register page: the browser used to call supabase.auth.signUp directly, so any client-side host check would be cosmetic. Email signup moved to POST /api/auth/signup on ALL hosts (byte-identical GoTrue call for open hosts); BankID gates in /bankid/complete; Google gates via the dashboard layout's brand-domain bounce (the account exists after OAuth, but gets no branded experience). Company invites bypass the allowlist because the invite is the authorization. Allowlisted signups' companies attach to the brand's byrå team via create_company_for_brand_signup (allowlist entry = the byrå's standing WL-15 authorization, recorded by an owner/admin); without the attach, WL-01 would home the company on the canonical domain, invisible on the very domain the user signed up on. Rejected a Supabase before-user-created hook: it does not reliably see the originating host and adds dashboard config coupling. [2026-08-27] New `tool-pg` vitest project: MCP tools driven through a REAL supabase-js client against a REAL PostgREST (tests/tool-pg/, scripts/tool-pg/reset.sh, `npm run tools:pg:reset` + `npm run test:tools`, plus a tool-pg CI job). NOT a duplicate of pg-real: that project holds a `pg` Pool and writes SQL, which structurally cannot see the half of a tool that PostgREST resolves at request time (the `.select()` column strings, the resource embeds, the `or=(...)` grammar, `.contains()` operand types). Before this, all 100 files in extensions/general/mcp-server/__tests__ faked supabase and query-journal.test.ts deferred its query chain to "the live MCP smoke test", which does not exist in CI: the PostgREST grammar of 157 tools was gated by nothing. Three findings worth keeping. (1) supabase-js hard-codes a `/rest/v1` prefix that a bare PostgREST does not serve, so the first version of the harness 404'd all 55 sweep queries, the tools reported the empty response as "Database error: undefined", and the suite passed GREEN while exercising nothing; fixed with a URL-rewriting `global.fetch` in createToolPgClient, and a permanent self-test now injects a bad column and asserts the harness detects 42703, so a green sweep means something. (2) Errors are captured at the TRANSPORT, not from the thrown Error: the tools wrap failures in their own prose and lose the payload, so a real 42703 arrives as an unclassifiable string. (3) The reset recreates the CONTAINER rather than dropping schemas: `storage` is owned by supabase_storage_admin so `DROP SCHEMA storage` fails as postgres, and dropping only `public` leaves the storage RLS policies migration 20240101000024 creates unconditionally, aborting the next replay partway and leaving a half-migrated database that looks like a migration bug. CI runs PostgREST via `docker run --network host` rather than a service container, because service containers on a non-containerized job are reachable from the runner but not from each other by name. Current coverage is honest and partial: 74 read tools, 87 real requests, 0 malformed queries, 4 failures all 22P02 from the empty argument set. Per-tool argument fixtures are what deepen it, and the harness is the thing that makes writing them worthwhile. [2026-08-27] Added `npm run check:types`, a typecheck ratchet (scripts/checks/no-new-type-errors.mjs + typecheck-baseline.json), wired into the core-build `checks` job next to check:lint. Reason: `npm test` does NOT typecheck. Vitest transpiles and discards types, so a type error passes all 18 000 tests and only surfaces in `npm run build` minutes later; that happened TWICE on 2026-08-27 (a widened errorKind union in the MCP server that lib/events/types.ts still contradicted, and an `interface` that would not assign into `Record[]` because interfaces have no implicit index signature). It is not merely a faster copy of the build job: `tsc --noEmit` also covers `__tests__` files, which the Next.js build never compiles, and that is where all 539 baseline errors live. Baseline is keyed per FILE, deliberately unlike the per-RULE lint ratchet: the legacy errors are concentrated in a handful of old test files and TS2322 is common enough that a code-keyed budget would silently absorb a real regression somewhere else, whereas per-file trips the moment a previously-clean file gains an error. Verified the gate actually fires by introducing a deliberate `const x: number = 'str'` and watching it fail with the exact location, then restoring. Cost measured: 36 s cold (what CI pays, since tsconfig.tsbuildinfo is gitignored) and 4.4 s warm locally via the existing `incremental: true`. The script sets NODE_OPTIONS=--max-old-space-size=8192 because a bare tsc dies with "Ineffective mark-compacts near heap limit" on this graph after about two minutes, which reads like a hang rather than a misconfiguration; it also detects that OOM string and exits 2 with a "raise HEAP_MB" message rather than silently reporting zero errors. NOT changed: Definition of Done item 1 still says only lint + test. CI enforcement is the stronger mechanism and does not need the policy edit; adding it to DoD is a founder call. +[2026-08-27] Dropped VAT cadence localStorage persistence from PR #1998 (kept the settings-row gate): skeptic pass refuted it twice (SSR hydration mismatch from render-phase localStorage read; persisting a cadence that deviates from moms_period keeps the filing pipeline open on the wrong period type with no downstream period-type validation). The mount-time re-seed from moms_period is the self-healing control; FyPicker already persists the rakenskapsar pick. [2026-08-27] Invite-only brand signup ships accepting a low-severity allowlist enumeration residual: POST /api/auth/signup returns 403 for a non-allowlisted email vs 200/400 for an allowlisted one, and the 403 short-circuits before GoTrue, so it is captcha-free and unthrottled: someone with candidate emails can test which are on a brand's allowlist. Not closed because (a) the app deliberately never holds the Turnstile secret (it lives in Supabase/GoTrue; a repo test forbids TURNSTILE_SECRET_KEY in app env), and (b) the clear "you're not invited, go to Accounted" redirect UX inherently reveals the verdict. It leaks membership of guessed emails, not the list, and no ledger/credential data. Follow-up option if it matters later: add signup-endpoint rate limiting. The related fail-OPEN (a brands-table error was read as unbranded, opening invite-only signup during a DB blip) WAS fixed: the gate now returns lookupFailed and both signup routes answer 503. diff --git a/components/reports/views/index.tsx b/components/reports/views/index.tsx index aaaba646..4740e8b7 100644 --- a/components/reports/views/index.tsx +++ b/components/reports/views/index.tsx @@ -19,6 +19,7 @@ import { SegmentedControl } from '@/components/ui/segmented-control' import { EmptyState } from '@/components/ui/empty-state' import { FyPicker } from '@/components/common/FyPicker' import { mostRecentEndedVatPeriod } from '@/lib/vat/period-defaults' +import { resolveInitialVatPeriodSelection } from '@/lib/vat/period-selection' import { ContextPicker } from '@/components/common/ContextPicker' import { cn, formatDate } from '@/lib/utils' import { getErrorMessage } from '@/lib/errors/get-error-message' @@ -1517,8 +1518,12 @@ export function VatDeclarationView({ pageTitle }: { pageTitle?: string } = {}) { // Company settings drive both the momsregistrerad gate and the default // periodicity (moms_period in Inställningar). Applied once per company the // first time its settings settle — as a render-phase adjustment, not an - // effect. A later manual change to the picker is preserved, and a company - // switch re-applies the new company's setting. `useCompanySettings` only + // effect. A later manual change to the picker is preserved for the session, + // and a company switch re-applies the new company's setting. The cadence is + // deliberately NOT persisted across visits: the redovisningsperiod is fixed + // by the company's Skatteverket registration, so this mount-time re-seed is + // the control that self-heals an in-session detour to the wrong period + // type (see lib/vat/period-selection.ts). `useCompanySettings` only // refetches when the active company changes, so this never clobbers a // manual selection mid-session. const { settings, isLoading: settingsLoading, refetch: refetchSettings } = useCompanySettings() @@ -1526,25 +1531,24 @@ export function VatDeclarationView({ pageTitle }: { pageTitle?: string } = {}) { const companyKey = settingsLoading ? null : (settings?.company_id ?? 'none') if (companyKey !== null && appliedCompany !== companyKey) { setAppliedCompany(companyKey) - const configured = settings?.moms_period ?? 'quarterly' - setPeriodType(configured) - if (configured === 'monthly' || configured === 'quarterly') { - // Default to the period whose declaration is actually open: the current - // one can never be filed, so seeding it forced a step-back click on - // every filing visit (and a year-boundary trap in January). - const ended = mostRecentEndedVatPeriod(configured, new Date(), { - over40m: settings?.vat_taxable_base_over_40m === true, - }) - setYear(ended.year) - setPeriod(ended.period) - } else { - setPeriod(1) - } + const initial = resolveInitialVatPeriodSelection({ + momsPeriod: settings?.moms_period ?? null, + over40m: settings?.vat_taxable_base_over_40m === true, + }) + setPeriodType(initial.periodType) + setYear(initial.year) + setPeriod(initial.period) } // Settings row present and the company answered "not VAT-registered" — // the declaration is meaningless, so the whole view is gated below. const notVatRegistered = !settingsLoading && settings !== null && !settings.vat_registered + // No company_settings row at all (company created outside onboarding): + // VAT registration AND periodicity are both unknown. This used to fall + // through every gate and render a silently guessed quarterly declaration; + // the wrong period type for an årsmoms company is a compliance hazard, so + // it now gates like the other unknowns. + const settingsRowMissing = !settingsLoading && settings === null // Registered but never picked a redovisningsperiod (rare — onboarding // requires it, but companies created outside that flow can miss it). const momsPeriodMissing = settings?.vat_registered === true && !settings.moms_period @@ -1586,7 +1590,11 @@ export function VatDeclarationView({ pageTitle }: { pageTitle?: string } = {}) { // gated, or no redovisningsperiod configured); any change to it triggers a // refetch and stale responses are discarded. const fetchKey = - periodType === null || notVatRegistered || momsPeriodMissing || awaitingFiscalPeriod + periodType === null || + notVatRegistered || + settingsRowMissing || + momsPeriodMissing || + awaitingFiscalPeriod ? null : `${periodType}:${year}:${period}:${isYearly ? fiscalPeriodId : ''}:${retryKey}` @@ -1797,6 +1805,21 @@ export function VatDeclarationView({ pageTitle }: { pageTitle?: string } = {}) { ) } + if (settingsRowMissing) { + return ( +
+ {bareHeader} + +
+ ) + } + if (notVatRegistered) { return (
diff --git a/lib/vat/__tests__/period-selection.test.ts b/lib/vat/__tests__/period-selection.test.ts new file mode 100644 index 00000000..9b3f957c --- /dev/null +++ b/lib/vat/__tests__/period-selection.test.ts @@ -0,0 +1,55 @@ +import { describe, it, expect } from 'vitest' +import { resolveInitialVatPeriodSelection } from '../period-selection' + +describe('resolveInitialVatPeriodSelection', () => { + const today = new Date(2026, 7, 27) // 2026-08-27 + + it('seeds yearly cadence from a yearly setting', () => { + expect( + resolveInitialVatPeriodSelection({ momsPeriod: 'yearly', over40m: false, today }), + ).toEqual({ periodType: 'yearly', year: 2026, period: 1 }) + }) + + it('seeds the most recently ended quarter for a quarterly setting', () => { + expect( + resolveInitialVatPeriodSelection({ momsPeriod: 'quarterly', over40m: false, today }), + ).toEqual({ periodType: 'quarterly', year: 2026, period: 2 }) + }) + + it('rolls a quarterly seed back across the year boundary in Q1', () => { + expect( + resolveInitialVatPeriodSelection({ + momsPeriod: 'quarterly', + over40m: false, + today: new Date(2026, 1, 10), + }), + ).toEqual({ periodType: 'quarterly', year: 2025, period: 4 }) + }) + + it('seeds monthly filers from the deadline-aware default', () => { + // 2026-08-06: June's declaration is due 17 Aug, so June is the open one. + expect( + resolveInitialVatPeriodSelection({ + momsPeriod: 'monthly', + over40m: false, + today: new Date(2026, 7, 6), + }), + ).toEqual({ periodType: 'monthly', year: 2026, period: 6 }) + }) + + it('honors the over-40M monthly rule (always the most recently ended month)', () => { + expect( + resolveInitialVatPeriodSelection({ + momsPeriod: 'monthly', + over40m: true, + today: new Date(2026, 7, 6), + }), + ).toEqual({ periodType: 'monthly', year: 2026, period: 7 }) + }) + + it('falls back to quarterly when moms_period is unset (state is gated, never rendered)', () => { + expect( + resolveInitialVatPeriodSelection({ momsPeriod: null, over40m: false, today }), + ).toEqual({ periodType: 'quarterly', year: 2026, period: 2 }) + }) +}) diff --git a/lib/vat/period-selection.ts b/lib/vat/period-selection.ts new file mode 100644 index 00000000..f31dc777 --- /dev/null +++ b/lib/vat/period-selection.ts @@ -0,0 +1,51 @@ +/** + * Initial period selection for the VAT declaration view. + * + * The view seeds its cadence (month/quarter/räkenskapsår) from the company's + * configured redovisningsperiod (company_settings.moms_period) and its + * concrete period from the most recently ended one (period-defaults.ts), the + * only one that can actually be filed. + * + * The cadence is deliberately NOT persisted across visits: a company's + * redovisningsperiod is fixed by its Skatteverket registration (SFL 26 kap), + * so the setting has exactly one lawful value and the mount-time re-seed is + * the control that self-heals a temporary in-session detour (e.g. a + * helårsmoms user peeking at quarterly figures). Restoring such a detour + * would keep the filing pipeline open on the wrong period type. The yearly + * cadence's räkenskapsår pick is persisted by FyPicker, which is fine: every + * fiscal year is a legitimate target. + */ + +import { mostRecentEndedVatPeriod } from './period-defaults' +import type { MomsPeriod, VatPeriodType } from '@/types' + +export interface VatPeriodSelection { + periodType: VatPeriodType + year: number + /** 1-12 for monthly, 1-4 for quarterly, always 1 for yearly. */ + period: number +} + +/** + * Decide the view's initial period from the company's moms_period setting: + * the setting's cadence, seeded to the most recently ended period in it. + */ +export function resolveInitialVatPeriodSelection(opts: { + momsPeriod: MomsPeriod | null + over40m: boolean + today?: Date +}): VatPeriodSelection { + const { momsPeriod, over40m } = opts + const today = opts.today ?? new Date() + + // The 'quarterly' fallback only shapes state that is never shown: the view + // gates on a missing moms_period (and on a missing settings row) before + // rendering a declaration. + const cadence = momsPeriod ?? 'quarterly' + + if (cadence === 'yearly') { + return { periodType: 'yearly', year: today.getFullYear(), period: 1 } + } + const ended = mostRecentEndedVatPeriod(cadence, today, { over40m }) + return { periodType: cadence, year: ended.year, period: ended.period } +}