fix: article ordering, free-text rows, invoice back-nav, onboarding resilience (#1053) (#1056)

* fix: article number ordering, free-text rows, invoice back-nav, onboarding resilience (#1053)

Four fixes from Discord feedback in issue #1053:

- Articles now order by article number with numeric-aware comparison
  ('2' before '10', unnumbered last, name tiebreak) in the invoice
  editor's article picker and as the register's default sort, via a
  shared lib/articles/sort.ts. Name order put article "1" last.

- Invoice rows with no amounts (quantity 0, unit price 0) render as
  pure text rows on the PDF, the invoice detail page, and the review
  step via shared isTextLikeLine(), instead of printing
  "0 / 0,00 SEK / 0,00 SEK". Display-only; booking untouched.

- The invoice editor navigates with router.replace after saving, so
  the detail page's back arrow returns to the list instead of
  reopening a fresh editor from history.

- A transient query failure no longer reads as "no companies" /
  "onboarding not done": getActiveCompanyId throws
  CompanyContextError('resolution_failed') instead of returning null,
  the Edge middleware fails open on a degraded resolution (no
  onboarding redirect, no cookie clearing, no locale overwrite), and
  the dashboard page only redirects to /onboarding on a positively
  read incomplete/missing settings row. This is the likely cause of
  the completed onboarding wizard reappearing.

Fixes #1053

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* docs: CLAUDE.md tenancy line matches actual resolution order (prefs-first, cookie not read)

The middleware stopped reading the gnubok-company-id cookie when
user_preferences became authoritative (RLS parity); the stale doc line
still described cookie-first order and misled review tooling.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

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)
+  })
+})

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<T extends SortableArticle>(articles: T[]): T[] {
+  return [...articles].sort(compareArticles)
+}

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', () => {

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
   }
 

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)
+  })
+})

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)
+}

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.
                 <View key={index} style={styles.tableRow}>

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<typeof createServerClient>,
   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 }
 }