diff --git a/CLAUDE.md b/CLAUDE.md index 7eb409a4..836e5722 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -67,7 +67,7 @@ npm run skills:generate # Regenerate agent_atom_registry seed after editing an ## Architecture - **Journal entry lifecycle**: `createDraftEntry()` → `commitEntry()` (atomic voucher via `commit_journal_entry` RPC); `createJournalEntry()` does both. Everything accounting-shaped routes through this engine. -- **Tenancy**: every business table has `company_id`. Active company resolves in `lib/supabase/middleware.ts`: `gnubok-company-id` cookie → `user_preferences.active_company_id` → first membership. RLS uses `user_company_ids()`; queries still filter by `company_id` explicitly (defense in depth: service-role paths have no RLS). +- **Tenancy**: every business table has `company_id`. Active company resolves in `lib/supabase/middleware.ts` from `user_preferences.active_company_id` (authoritative: RLS reads the same value via `current_active_company_id()`), falling back to first non-archived membership. The `gnubok-company-id` cookie is written as a hint for legacy read paths but deliberately no longer read: letting it override the DB would desync Next.js from RLS. RLS uses `user_company_ids()`; queries still filter by `company_id` explicitly (defense in depth: service-role paths have no RLS). - **Auth**: Supabase email+password + TOTP MFA, enforced **application-side**, not in RLS. `NEXT_PUBLIC_REQUIRE_MFA=true` on hosted; `NEXT_PUBLIC_SELF_HOSTED=true` disables MFA. API routes wrap `withRouteContext`: it is the only path that enforces MFA, so never hand-roll `supabase.auth.getUser()` in a route. - **Events**: `lib/events/bus.ts` is a module-level singleton. Any route that emits events must call `ensureInitialized()` (`lib/init.ts`) at module level: otherwise extension handlers are never wired and events silently go nowhere. - **Supabase clients**: browser `client.ts`, server `createClient()`, service role `createServiceClient()`, cookieless service role `createServiceClientNoCookies()` (lives in `lib/auth/api-keys.ts`; for API-key/MCP paths). Paginate with `fetchAllRows()`: PostgREST silently caps at 1000 rows. diff --git a/DECISIONS.md b/DECISIONS.md index 8ecb550b..058be807 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -206,3 +206,6 @@ One line per decision: `[YYYY-MM-DD] : `. Appended by agents and [2026-07-15] Issue #735 result-appropriation: did NOT run the mass 2099-to-2098 backfill. Prod audit showed the script read the FROZEN opening-balance 2099, not the CURRENT posted balance: 202 of 360 planned periods already carried their own disposition (mostly via SIE import) and 43 would have double-moved equity (worst case tens of MSEK on a single company), while periods without an OB entry fabricated balances via the cumulative-history fallback. Posted ONE verified entry for the reporting company (unblocking their arsredovisning) and rewrote the script to a current-balance-safe backfill: eligibility decided from current posted 2099, explicit OB entry required, already-disposed and ambiguous periods skipped to a manual-review report. planResultAppropriation/generateResultAppropriation untouched (steady-state year-end path, covered by pg-real tests); the safety lives in the sweep. [2026-07-17] create_account now enforces first-digit vs account_type consistency (superRefine at the commit boundary + fail-fast in the MCP tool): class 8 legitimately allows both revenue and expense (financial items per the BAS catalog), classes 0/9 stay unconstrained (free-use per the BAS standard); prevents contradictory rows like 2999+expense whose derived account_class would misclassify balance sheet vs income statement. PR-review findings on set_voucher_note (posted-entry immutability, BFL 5:5 audit trail) were refuted, not fixed: the notes-only carve-out is migration 20260608120000's whole-row to_jsonb diff and audit_journal_entries already records old/new on every UPDATE. [2026-07-17] Assistenten settings opens on Kunskap (konteringskarta) instead of Minne; dropped the nested Kompetens/Minne tab row inside Kunskap: it duplicated the top-level tabs one row above. +[2026-07-17] getActiveCompanyId now throws CompanyContextError('resolution_failed') on query failure instead of returning null (issue #1053): null was indistinguishable from "no companies" and every caller redirects that state to /onboarding, so a transient DB failure showed onboarded users the wizard. Chose throw-at-the-source over a degraded-flag return so all redirect sites are fixed at once; withRouteContext already try/catches the call. The Edge middleware copy keeps a degraded flag instead (middleware cannot throw usefully) and fails open. +[2026-07-17] Amount-less invoice rows (quantity 0 and unit price 0) render as text rows on PDF/detail/review via shared isTextLikeLine() instead of printing "0 / 0,00 SEK / 0,00 SEK" (issue #1053): users write free-text lines through the article picker's "Egen rad (fri text)" product row, not only the dedicated textrad button. Display-only; booking and validation semantics untouched. +[2026-07-17] Articles default sort is article_number (numeric-aware via Intl.Collator numeric, unnumbered last, name tiebreak) in both the register and the invoice editor picker, replacing name order (issue #1053): users number articles precisely to control listing order, matching Fortnox convention. diff --git a/app/(dashboard)/articles/page.tsx b/app/(dashboard)/articles/page.tsx index d46bff55..56270a30 100644 --- a/app/(dashboard)/articles/page.tsx +++ b/app/(dashboard)/articles/page.tsx @@ -31,6 +31,7 @@ import { EmptyState } from '@/components/ui/empty-state' import { PageHeader } from '@/components/ui/page-header' import { ReportExportMenu } from '@/components/reports/ReportExportMenu' import { formatCurrency } from '@/lib/utils' +import { compareArticles } from '@/lib/articles/sort' import Link from 'next/link' import { useCompany } from '@/contexts/CompanyContext' import { useCanWrite } from '@/lib/hooks/use-can-write' @@ -76,9 +77,10 @@ function ArticlesPageInner() { const sortParam = searchParams.get('sort') const dirParam = searchParams.get('dir') + // Default to the user's own article numbering (numeric-aware), issue #1053. const sortColumn: SortColumn = (SORTABLE_COLUMNS as ReadonlyArray).includes(sortParam ?? '') ? (sortParam as SortColumn) - : 'name' + : 'article_number' const sortDir: SortDir = dirParam === 'desc' ? 'desc' : 'asc' const updateSort = useCallback( @@ -193,7 +195,7 @@ function ArticlesPageInner() { cmp = compareStrings(a.name || '', b.name || '') break case 'article_number': - cmp = compareStrings(a.article_number || '', b.article_number || '') + cmp = compareArticles(a, b) break case 'type': cmp = compareStrings(a.type || '', b.type || '') diff --git a/app/(dashboard)/invoices/[id]/page.tsx b/app/(dashboard)/invoices/[id]/page.tsx index 9f36f7c2..9579a60f 100644 --- a/app/(dashboard)/invoices/[id]/page.tsx +++ b/app/(dashboard)/invoices/[id]/page.tsx @@ -12,7 +12,7 @@ import { Separator } from '@/components/ui/separator' import { useToast } from '@/components/ui/use-toast' import { formatCurrency, formatDate, cn } from '@/lib/utils' import { getVatTreatmentLabel } from '@/lib/invoices/vat-rules' -import { invoiceDisplayNumber } from '@/lib/invoices/display' +import { invoiceDisplayNumber, isTextLikeLine } from '@/lib/invoices/display' import { getDisplayTotal } from '@/lib/invoices/rounding' import { isEditableInvoiceDraft } from '@/lib/invoices/is-editable-draft' import { creditNoteNeedsJournalEntry } from '@/lib/invoices/issue-credit-note' @@ -820,7 +820,7 @@ export default function InvoiceDetailPage({ params }: { params: Promise<{ id: st numeric columns; a blank one renders as a spacer. */}
{invoice.items.map((item) => - item.line_type === 'text' ? ( + isTextLikeLine(item) ? (
{item.description || ' '}
@@ -857,7 +857,7 @@ export default function InvoiceDetailPage({ params }: { params: Promise<{ id: st {/* Items, mobile cards */}
{invoice.items.map((item) => - item.line_type === 'text' ? ( + isTextLikeLine(item) ? (

{item.description || ' '}

) : (
diff --git a/app/(dashboard)/page.tsx b/app/(dashboard)/page.tsx index b45798b7..30536fb3 100644 --- a/app/(dashboard)/page.tsx +++ b/app/(dashboard)/page.tsx @@ -29,16 +29,20 @@ export default async function DashboardPage() { const rawCompanyId = cookieStore.get('gnubok-company-id')?.value ?? await getActiveCompanyId(supabase, user.id) - // Validate the cookie/preference points to a company the user can access + // Validate the cookie/preference points to a company the user can access. + // Only a positive "no membership row" clears it: a FAILED query means the + // membership is unknown, and treating that as absent bounced onboarded + // users to the wizard on transient failures (issue #1053). RLS still + // guards every downstream query if the cookie is stale. let companyId = rawCompanyId if (companyId) { - const { data: membership } = await supabase + const { data: membership, error: membershipError } = await supabase .from('company_members') .select('company_id') .eq('company_id', companyId) .eq('user_id', user.id) .maybeSingle() - if (!membership) companyId = null + if (!membership && !membershipError) companyId = null } if (!companyId) { @@ -54,7 +58,7 @@ export default async function DashboardPage() { // Fetch all data in parallel const [ - { data: settings }, + settingsRes, { count: customerCount }, { count: invoiceCount }, { count: receiptCount }, @@ -71,7 +75,7 @@ export default async function DashboardPage() { worklist, suggestedMatches, ] = await Promise.all([ - supabase.from('company_settings').select('*').eq('company_id', companyId).single(), + supabase.from('company_settings').select('*').eq('company_id', companyId).maybeSingle(), supabase.from('customers').select('*', { count: 'exact', head: true }).eq('company_id', companyId), supabase.from('invoices').select('*', { count: 'exact', head: true }).eq('company_id', companyId), supabase.from('receipts').select('*', { count: 'exact', head: true }).eq('company_id', companyId), @@ -100,6 +104,15 @@ export default async function DashboardPage() { listSuggestedMatches(supabase, companyId, 5), ]) + // A FAILED settings read must not masquerade as "onboarding not done": + // that sent fully onboarded users back to the wizard on a transient query + // failure (issue #1053). Throw to the error boundary (retryable) and only + // redirect on a genuinely incomplete or missing settings row. + const { data: settings, error: settingsError } = settingsRes + if (settingsError) { + throw new Error(`company_settings fetch failed: ${settingsError.message}`) + } + // If onboarding is not complete, redirect to onboarding if (!settings?.onboarding_complete) { redirect('/onboarding') diff --git a/components/invoices/InvoiceEditor.tsx b/components/invoices/InvoiceEditor.tsx index 84c0103a..09a0e736 100644 --- a/components/invoices/InvoiceEditor.tsx +++ b/components/invoices/InvoiceEditor.tsx @@ -23,6 +23,7 @@ import { Tabs, TabsList, TabsTrigger } from '@/components/ui/tabs' import { useToast } from '@/components/ui/use-toast' import { formatCurrency } from '@/lib/utils' import { getVatRules, getAvailableVatRates } from '@/lib/invoices/vat-rules' +import { sortArticles } from '@/lib/articles/sort' import { getAmountToPay } from '@/lib/invoices/rounding' import { Dialog, DialogContent, DialogHeader, DialogTitle, DialogDescription, DialogFooter } from '@/components/ui/dialog' import { Loader2, Plus, Trash2, ArrowLeft, Send, Eye, Landmark, Lock, AlertTriangle, MoreVertical, CalendarClock, Tags, Copy } from 'lucide-react' @@ -479,8 +480,9 @@ export default function InvoiceEditor(props: InvoiceEditorProps = { mode: 'creat .select('id, article_number, name, unit, price_excl_vat, vat_rate, revenue_account, currency') .eq('company_id', company.id) .eq('active', true) - .order('name') - setArticles((data ?? []) as ArticleOption[]) + // Numeric-aware order by article number ('2' before '10', unnumbered last): + // the picker should follow the user's own numbering, not the alphabet. + setArticles(sortArticles((data ?? []) as ArticleOption[])) } async function fetchRevenueAccounts() { @@ -566,7 +568,7 @@ export default function InvoiceEditor(props: InvoiceEditorProps = { mode: 'creat throw new Error(getErrorMessage(result, { context: 'article', statusCode: response.status })) } const created = result.data as ArticleOption - setArticles((prev) => [...prev, created].sort((a, b) => a.name.localeCompare(b.name, 'sv'))) + setArticles((prev) => sortArticles([...prev, created])) setValue(`items.${index}.article_id`, created.id, { shouldDirty: true }) toast({ title: t('article_saved_title'), description: created.name }) } catch (error) { @@ -956,7 +958,7 @@ export default function InvoiceEditor(props: InvoiceEditorProps = { mode: 'creat title: ts('created_title'), description: ts('created_description', { number: data.external_invoice_number ?? '' }), }) - router.push(`/invoices/${result.data.id}`) + router.replace(`/invoices/${result.data.id}`) } catch (error) { toast({ title: ts('create_failed_title'), @@ -1033,7 +1035,7 @@ export default function InvoiceEditor(props: InvoiceEditorProps = { mode: 'creat if (selectedCustomer?.email && createdInvoiceId && hasEmailSend) { setShowSendPrompt(true) } else if (createdInvoiceId) { - router.push(`/invoices/${createdInvoiceId}`) + router.replace(`/invoices/${createdInvoiceId}`) } } @@ -1103,7 +1105,7 @@ export default function InvoiceEditor(props: InvoiceEditorProps = { mode: 'creat } else if (selectedCustomer?.email && hasEmailSend) { setShowSendPrompt(true) } else { - router.push(`/invoices/${result.data.id}`) + router.replace(`/invoices/${result.data.id}`) } } catch (error) { toast({ @@ -1162,7 +1164,10 @@ export default function InvoiceEditor(props: InvoiceEditorProps = { mode: 'creat title: t('toast_draft_saved_title'), description: t('toast_draft_saved_description'), }) - router.push(`/invoices/${result.data.id}`) + // replace (here and in every post-save navigation): the editor page must + // drop out of history, or the detail page's back arrow reopens a fresh + // editor instead of returning to the list (issue #1053). + router.replace(`/invoices/${result.data.id}`) } catch (error) { toast({ title: t('save_draft_failed_title'), @@ -1220,7 +1225,7 @@ export default function InvoiceEditor(props: InvoiceEditorProps = { mode: 'creat title: t('toast_draft_updated_title'), description: t('toast_draft_updated_description'), }) - router.push(`/invoices/${initial.id}`) + router.replace(`/invoices/${initial.id}`) } catch (error) { toast({ title: t('update_failed_title'), @@ -1259,7 +1264,7 @@ export default function InvoiceEditor(props: InvoiceEditorProps = { mode: 'creat } finally { setIsSending(false) setShowSendPrompt(false) - router.push(`/invoices/${createdInvoiceId}`) + router.replace(`/invoices/${createdInvoiceId}`) } } @@ -2504,7 +2509,7 @@ export default function InvoiceEditor(props: InvoiceEditorProps = { mode: 'creat { if (!open && createdInvoiceId) { setShowSendPrompt(false) - router.push(`/invoices/${createdInvoiceId}`) + router.replace(`/invoices/${createdInvoiceId}`) } }}> @@ -2519,7 +2524,7 @@ export default function InvoiceEditor(props: InvoiceEditorProps = { mode: 'creat variant="outline" onClick={() => { setShowSendPrompt(false) - if (createdInvoiceId) router.push(`/invoices/${createdInvoiceId}`) + if (createdInvoiceId) router.replace(`/invoices/${createdInvoiceId}`) }} disabled={isSending} > diff --git a/components/invoices/InvoiceReviewContent.tsx b/components/invoices/InvoiceReviewContent.tsx index 36e02fa7..f0e3f63b 100644 --- a/components/invoices/InvoiceReviewContent.tsx +++ b/components/invoices/InvoiceReviewContent.tsx @@ -6,6 +6,7 @@ import { Badge } from '@/components/ui/badge' import { Separator } from '@/components/ui/separator' import { formatCurrency, formatDate } from '@/lib/utils' import { getDisplayTotal } from '@/lib/invoices/rounding' +import { isTextLikeLine } from '@/lib/invoices/display' import { itemHasAccrual } from '@/lib/bookkeeping/accruals/account-suggestions' import type { Customer, Currency } from '@/types' @@ -73,10 +74,11 @@ export function InvoiceReviewContent({ non_eu_business: t('customer_type_non_eu_business'), } - // Calculate per-rate VAT breakdown (free-text rows carry no amounts). + // Calculate per-rate VAT breakdown (free-text and amount-less rows carry + // no amounts and must not seed an empty rate group). const vatByRate = new Map() for (const item of items) { - if (item.line_type === 'text') continue + if (isTextLikeLine(item)) continue const rate = item.vat_rate ?? 0 const lineTotal = item.quantity * item.unit_price const lineVat = Math.round(lineTotal * rate / 100 * 100) / 100 @@ -131,7 +133,7 @@ export function InvoiceReviewContent({ {items.map((item, index) => - item.line_type === 'text' ? ( + isTextLikeLine(item) ? ( {item.description || ' '} @@ -170,7 +172,7 @@ export function InvoiceReviewContent({
{items.map((item, index) => - item.line_type === 'text' ? ( + isTextLikeLine(item) ? (

{item.description || ' '}

) : (
diff --git a/lib/articles/__tests__/sort.test.ts b/lib/articles/__tests__/sort.test.ts new file mode 100644 index 00000000..7addecf7 --- /dev/null +++ b/lib/articles/__tests__/sort.test.ts @@ -0,0 +1,71 @@ +import { describe, it, expect } from 'vitest' +import { compareArticles, sortArticles } from '@/lib/articles/sort' + +describe('sortArticles', () => { + it('orders by article number numerically, not alphabetically', () => { + const sorted = sortArticles([ + { article_number: '10', name: 'Tio' }, + { article_number: '2', name: 'Två' }, + { article_number: '1', name: 'Ett' }, + ]) + expect(sorted.map((a) => a.article_number)).toEqual(['1', '2', '10']) + }) + + it('reproduces issue #1053: numbered register no longer sorts by name', () => { + const sorted = sortArticles([ + { article_number: '2', name: 'Delbehandling' }, + { article_number: '4', name: 'Fotvårdsremiss 85+' }, + { article_number: '5', name: 'Fotvårdsremiss ej frikort' }, + { article_number: '3', name: 'Fotvårdsremiss frikort' }, + { article_number: '1', name: 'Medicinsk fotvård' }, + ]) + expect(sorted.map((a) => a.article_number)).toEqual(['1', '2', '3', '4', '5']) + }) + + it('puts articles without a number last, sorted by name', () => { + const sorted = sortArticles([ + { article_number: null, name: 'Zeta' }, + { article_number: '7', name: 'Sju' }, + { article_number: null, name: 'Alfa' }, + { article_number: undefined, name: 'Beta' }, + ]) + expect(sorted.map((a) => a.name)).toEqual(['Sju', 'Alfa', 'Beta', 'Zeta']) + }) + + it('treats blank article numbers as unnumbered', () => { + const sorted = sortArticles([ + { article_number: ' ', name: 'Blank' }, + { article_number: '1', name: 'Ett' }, + ]) + expect(sorted.map((a) => a.name)).toEqual(['Ett', 'Blank']) + }) + + it('breaks ties on equal numbers by name', () => { + const sorted = sortArticles([ + { article_number: '1', name: 'B' }, + { article_number: '1', name: 'A' }, + ]) + expect(sorted.map((a) => a.name)).toEqual(['A', 'B']) + }) + + it('handles alphanumeric numbers with embedded digits', () => { + const sorted = sortArticles([ + { article_number: 'A10', name: 'x' }, + { article_number: 'A2', name: 'y' }, + ]) + expect(sorted.map((a) => a.article_number)).toEqual(['A2', 'A10']) + }) + + it('does not mutate the input array', () => { + const input = [ + { article_number: '2', name: 'b' }, + { article_number: '1', name: 'a' }, + ] + sortArticles(input) + expect(input.map((a) => a.article_number)).toEqual(['2', '1']) + }) + + it('compareArticles is exported for column sorters', () => { + expect(compareArticles({ article_number: '2', name: 'x' }, { article_number: '10', name: 'y' })).toBeLessThan(0) + }) +}) diff --git a/lib/articles/sort.ts b/lib/articles/sort.ts new file mode 100644 index 00000000..ae7af359 --- /dev/null +++ b/lib/articles/sort.ts @@ -0,0 +1,31 @@ +/** + * Canonical ordering for article lists and pickers: by article number with + * numeric-aware comparison ('2' before '10'), articles without a number last, + * name as tie-breaker. Users number their articles to control this order + * (issue #1053), so a plain string sort ('10' < '2') or name sort breaks it. + */ + +interface SortableArticle { + article_number?: string | null + name: string +} + +const numberCollator = new Intl.Collator('sv', { numeric: true, sensitivity: 'base' }) + +export function compareArticles(a: SortableArticle, b: SortableArticle): number { + const aNum = a.article_number?.trim() + const bNum = b.article_number?.trim() + if (aNum && bNum) { + const byNumber = numberCollator.compare(aNum, bNum) + if (byNumber !== 0) return byNumber + } else if (aNum) { + return -1 + } else if (bNum) { + return 1 + } + return numberCollator.compare(a.name, b.name) +} + +export function sortArticles(articles: T[]): T[] { + return [...articles].sort(compareArticles) +} diff --git a/lib/company/__tests__/context.test.ts b/lib/company/__tests__/context.test.ts index 8117bf54..b768ce8a 100644 --- a/lib/company/__tests__/context.test.ts +++ b/lib/company/__tests__/context.test.ts @@ -183,6 +183,52 @@ describe('getActiveCompanyId', () => { expect(await getActiveCompanyId(supabase as never, 'user-1')).toBeNull() }) + + // A failed query must throw, never read as "no companies": callers redirect + // the null state to the onboarding wizard, and a transient failure was + // enough to show onboarding to a fully onboarded user (issue #1053). + it('throws resolution_failed when the preferences query fails', async () => { + const { supabase } = buildSupabase({ + user_preferences: { maybeSingle: { data: null, error: { message: 'fetch failed' } } }, + company_members: { maybeSingle: { data: { company_id: 'company-1' } } }, + }) + + const err = await getActiveCompanyId(supabase as never, 'user-1').catch((e) => e) + + expect(err).toBeInstanceOf(CompanyContextError) + expect(err.code).toBe('resolution_failed') + }) + + it('throws resolution_failed when the membership query fails', async () => { + const { supabase } = buildSupabase({ + user_preferences: { maybeSingle: { data: null } }, + company_members: { maybeSingle: { data: null, error: { message: 'timeout' } } }, + }) + + const err = await getActiveCompanyId(supabase as never, 'user-1').catch((e) => e) + + expect(err).toBeInstanceOf(CompanyContextError) + expect(err.code).toBe('resolution_failed') + }) + + it('throws instead of silently switching company when preference validation fails', async () => { + const { supabase } = buildSupabase({ + user_preferences: { maybeSingle: { data: { active_company_id: 'company-2' } } }, + company_members: { + maybeSingle: [ + { data: { company_id: 'company-1' } }, // first membership (parallel fetch) + { data: null, error: { message: 'connection reset' } }, // validation FAILS + ], + }, + }) + + const err = await getActiveCompanyId(supabase as never, 'user-1').catch((e) => e) + + // Falling back to company-1 here would silently flip a consultant onto + // the wrong company's books. + expect(err).toBeInstanceOf(CompanyContextError) + expect(err.code).toBe('resolution_failed') + }) }) describe('getCompanyDisplayName', () => { diff --git a/lib/company/context.ts b/lib/company/context.ts index 0adfd9e7..4c13c2cc 100644 --- a/lib/company/context.ts +++ b/lib/company/context.ts @@ -8,12 +8,14 @@ const COMPANY_COOKIE = 'gnubok-company-id' /** * Thrown by setActiveCompany so callers can tell a permissions problem * ('not_member') apart from a failed/unverified database write - * ('persist_failed') and surface the right message to the user. + * ('persist_failed'), and by getActiveCompanyId when a resolution query + * fails ('resolution_failed': the active company is unknown right now, + * which is NOT the same as the user having no companies). */ export class CompanyContextError extends Error { constructor( message: string, - readonly code: 'not_member' | 'persist_failed' + readonly code: 'not_member' | 'persist_failed' | 'resolution_failed' ) { super(message) this.name = 'CompanyContextError' @@ -32,7 +34,10 @@ export class CompanyContextError extends Error { * Having Next.js and RLS both read from `user_preferences` keeps them * perfectly in sync. * - * Returns null if the user has no non-archived companies. + * Returns null only when the user positively has no non-archived companies. + * Throws CompanyContextError('resolution_failed') when a query fails: a + * transient failure must never read as "no companies", because callers + * redirect that state to the onboarding wizard (issue #1053). */ export async function getActiveCompanyId( supabase: SupabaseClient, @@ -46,7 +51,7 @@ export async function getActiveCompanyId( // every dashboard layout render, so the sequential version was pure // wall-clock cost. Mirrors resolveCompanyForMiddleware, minus the // write-back (read paths shouldn't write). - const [{ data: prefs }, { data: firstCompany }] = await Promise.all([ + const [prefsRes, firstRes] = await Promise.all([ supabase .from('user_preferences') .select('active_company_id') @@ -62,6 +67,17 @@ export async function getActiveCompanyId( .maybeSingle(), ]) + const resolutionError = prefsRes.error ?? firstRes.error + if (resolutionError) { + throw new CompanyContextError( + `Active company resolution failed: ${resolutionError.message}`, + 'resolution_failed' + ) + } + + const prefs = prefsRes.data + const firstCompany = firstRes.data + if (prefs?.active_company_id) { if (firstCompany && prefs.active_company_id === firstCompany.company_id) { return firstCompany.company_id @@ -70,7 +86,7 @@ export async function getActiveCompanyId( // Preference points at a different company than the first membership: // validate it still resolves to a non-archived company the user is a // member of before trusting it. - const { data: membership } = await supabase + const { data: membership, error: membershipError } = await supabase .from('company_members') .select('company_id, companies!inner(archived_at)') .eq('company_id', prefs.active_company_id) @@ -78,6 +94,15 @@ export async function getActiveCompanyId( .is('companies.archived_at', null) .maybeSingle() + // Falling back to the first membership on a FAILED validation would + // silently switch a multi-company user's active company: fail loudly. + if (membershipError) { + throw new CompanyContextError( + `Active company validation failed: ${membershipError.message}`, + 'resolution_failed' + ) + } + if (membership) return membership.company_id } diff --git a/lib/invoices/__tests__/display.test.ts b/lib/invoices/__tests__/display.test.ts new file mode 100644 index 00000000..093db3b0 --- /dev/null +++ b/lib/invoices/__tests__/display.test.ts @@ -0,0 +1,23 @@ +import { describe, it, expect } from 'vitest' +import { isTextLikeLine } from '@/lib/invoices/display' + +describe('isTextLikeLine', () => { + it('is true for explicit text rows regardless of amounts', () => { + expect(isTextLikeLine({ line_type: 'text', quantity: 0, unit_price: 0 })).toBe(true) + expect(isTextLikeLine({ line_type: 'text', quantity: 2, unit_price: 100 })).toBe(true) + }) + + it('is true for product rows with no amounts (issue #1053 free-text line)', () => { + expect(isTextLikeLine({ line_type: 'product', quantity: 0, unit_price: 0 })).toBe(true) + expect(isTextLikeLine({ quantity: 0, unit_price: 0 })).toBe(true) + expect(isTextLikeLine({ quantity: null, unit_price: null })).toBe(true) + expect(isTextLikeLine({})).toBe(true) + }) + + it('is false as soon as the row carries a quantity or a price', () => { + expect(isTextLikeLine({ line_type: 'product', quantity: 1, unit_price: 0 })).toBe(false) + expect(isTextLikeLine({ line_type: 'product', quantity: 0, unit_price: 250 })).toBe(false) + expect(isTextLikeLine({ line_type: 'product', quantity: 0, unit_price: -250 })).toBe(false) + expect(isTextLikeLine({ quantity: 3, unit_price: 99.5 })).toBe(false) + }) +}) diff --git a/lib/invoices/display.ts b/lib/invoices/display.ts index 5b7589ba..6a9f7aba 100644 --- a/lib/invoices/display.ts +++ b/lib/invoices/display.ts @@ -16,3 +16,19 @@ export function invoiceDisplayNumber(invoice: { }): string { return invoice.invoice_number ?? invoice.external_invoice_number ?? INVOICE_NUMBER_DRAFT_LABEL } + +/** + * True when an invoice line should render as a pure text row: description + * only, no quantity/unit/price/amount columns. Explicit text rows + * (line_type 'text') always qualify; so do product rows carrying no amounts + * at all (quantity and unit price both zero/absent). Users write free-text + * lines via the article picker's "Egen rad (fri text)" and leave antal/pris + * at zero; printing "0 / 0,00 SEK / 0,00 SEK" on those is noise (issue #1053). + */ +export function isTextLikeLine(item: { + line_type?: 'product' | 'text' | null + quantity?: number | null + unit_price?: number | null +}): boolean { + return item.line_type === 'text' || (!item.quantity && !item.unit_price) +} diff --git a/lib/invoices/pdf-template.tsx b/lib/invoices/pdf-template.tsx index 5dc645bc..b311ca4e 100644 --- a/lib/invoices/pdf-template.tsx +++ b/lib/invoices/pdf-template.tsx @@ -10,6 +10,7 @@ import { import type { Invoice, InvoiceItem, Customer, CompanySettings, InvoiceDocumentType } from '@/types' import { generateOcrReference } from '@/lib/bankgiro/luhn' import { getAmountToPay } from '@/lib/invoices/rounding' +import { isTextLikeLine } from '@/lib/invoices/display' type PdfLang = 'sv' | 'en' @@ -657,7 +658,9 @@ export function InvoicePDF({ invoice, customer, items, company, originalInvoiceN // Free-text / blank rows carry no amounts: exclude them from every VAT // calculation. They still render as their own row in the line-items table. - const billableItems = items.filter((item) => item.line_type !== 'text') + // Amount-less product rows count as text too (isTextLikeLine), so they + // neither print zeros nor seed an empty per-rate VAT group. + const billableItems = items.filter((item) => !isTextLikeLine(item)) // Check if items have mixed VAT rates const hasPerLineVat = billableItems.some((item) => item.vat_rate !== undefined && item.vat_rate !== null) @@ -857,7 +860,7 @@ export function InvoicePDF({ invoice, customer, items, company, originalInvoiceN {/* Table rows */} {items.map((item, index) => - item.line_type === 'text' ? ( + isTextLikeLine(item) ? ( // Free-text / blank row: description spans the full width, no // numeric columns. An empty description renders as a spacer. diff --git a/lib/supabase/middleware.ts b/lib/supabase/middleware.ts index ccb6bcdc..50869f13 100644 --- a/lib/supabase/middleware.ts +++ b/lib/supabase/middleware.ts @@ -174,7 +174,11 @@ export async function updateSession(request: NextRequest) { // Resolve the active company at most once per request: both the MFA // enrollment gate and the company-context block below need it, and the // resolution costs DB round trips. - let resolvedCompany: { companyId: string | null; locale: string | null } | null = null + let resolvedCompany: { + companyId: string | null + locale: string | null + degraded: boolean + } | null = null const resolveCompanyOnce = async () => (resolvedCompany ??= await resolveCompanyForMiddleware(supabase, user.id, request)) @@ -206,11 +210,12 @@ export async function updateSession(request: NextRequest) { // Company context resolution const cookieCompanyId = request.cookies.get('gnubok-company-id')?.value - const { companyId, locale: dbLocale } = await resolveCompanyOnce() + const { companyId, locale: dbLocale, degraded } = await resolveCompanyOnce() // If the cookie pointed at a company we can no longer resolve (e.g. - // archived), clear it so the browser stops sending it. - if (cookieCompanyId && cookieCompanyId !== companyId) { + // archived), clear it so the browser stops sending it. Never on degraded + // resolution: a transient query failure must not wipe a valid cookie. + if (!degraded && cookieCompanyId && cookieCompanyId !== companyId) { supabaseResponse.cookies.set('gnubok-company-id', '', { path: '/', maxAge: 0 }) } @@ -219,7 +224,7 @@ export async function updateSession(request: NextRequest) { // without forcing every RSC render to query the database itself. const cookieLocale = request.cookies.get(LOCALE_COOKIE)?.value const effectiveLocale = isLocale(dbLocale) ? dbLocale : DEFAULT_LOCALE - if (cookieLocale !== effectiveLocale) { + if (!degraded && cookieLocale !== effectiveLocale) { supabaseResponse.cookies.set(LOCALE_COOKIE, effectiveLocale, { path: '/', sameSite: 'lax', @@ -246,6 +251,15 @@ export async function updateSession(request: NextRequest) { return supabaseResponse } + // Degraded resolution (a query FAILED, as opposed to returning no rows) + // means the user's companies are unknown, not absent. Fail open: pass + // the request through and let the layout's own resolution retry or + // surface an error. Redirecting here showed fully onboarded users the + // onboarding wizard again on a transient failure (issue #1053). + if (degraded) { + return supabaseResponse + } + // Enrichment lives in the user-keyed `bankid_enrichment` table (migration // 20260506160000), it cannot live in extension_data, which is // company-scoped, and the user has no company yet on this path. @@ -299,13 +313,13 @@ async function resolveCompanyForMiddleware( supabase: ReturnType, userId: string, _request: NextRequest -): Promise<{ companyId: string | null; locale: string | null }> { +): Promise<{ companyId: string | null; locale: string | null; degraded: boolean }> { // 1. user_preferences (authoritative) + first membership, fetched in // parallel: the fallback query result doubles as validation when the // preferred company happens to be the first membership, which is the // common single-company case, so most requests pay one round trip // instead of two sequential ones. - const [{ data: prefs }, { data: firstCompany }] = await Promise.all([ + const [prefsRes, firstRes] = await Promise.all([ supabase .from('user_preferences') .select('active_company_id, locale') @@ -321,14 +335,28 @@ async function resolveCompanyForMiddleware( .maybeSingle(), ]) + const prefs = prefsRes.data + const firstCompany = firstRes.data const locale = (prefs?.locale as string | undefined) ?? null + // A FAILED query (as opposed to one returning no rows) means the user's + // companies are unknown right now, not absent: flag it so the caller + // fails open instead of redirecting to onboarding or clearing cookies + // (issue #1053). Middleware cannot throw usefully, hence a flag. + if (prefsRes.error || firstRes.error) { + console.error( + '[middleware] company resolution query failed', + prefsRes.error ?? firstRes.error + ) + return { companyId: null, locale, degraded: true } + } + if (prefs?.active_company_id) { if (prefs.active_company_id === firstCompany?.company_id) { - return { companyId: firstCompany.company_id, locale } + return { companyId: firstCompany.company_id, locale, degraded: false } } - const { data: membership } = await supabase + const { data: membership, error: membershipError } = await supabase .from('company_members') .select('company_id, companies!inner(archived_at)') .eq('company_id', prefs.active_company_id) @@ -336,11 +364,18 @@ async function resolveCompanyForMiddleware( .is('companies.archived_at', null) .maybeSingle() - if (membership) return { companyId: membership.company_id, locale } + // A failed validation must not silently switch the user onto their + // first membership (wrong company for consultants): degrade instead. + if (membershipError) { + console.error('[middleware] company preference validation failed', membershipError) + return { companyId: null, locale, degraded: true } + } + + if (membership) return { companyId: membership.company_id, locale, degraded: false } } // 2. Fallback: first non-archived membership (already fetched above) - if (!firstCompany) return { companyId: null, locale } + if (!firstCompany) return { companyId: null, locale, degraded: false } // Write the fallback back to user_preferences so future RLS lookups // see the same active company without needing this fallback scan. @@ -358,5 +393,5 @@ async function resolveCompanyForMiddleware( console.error('[middleware] active company write-back failed', writeBackError) } - return { companyId: firstCompany.company_id, locale } + return { companyId: firstCompany.company_id, locale, degraded: false } }