Bug/document linking (#688)

* feat: enhance supplier invoice payment process and settings handling - Implemented linking of invoice documents to journal entries for cash payments in the supplier invoice payment process. - Refactored settings fetching logic to improve loading states and error handling across various settings components. - Introduced a new SettingsLoadError component to handle cases where settings fetch fails or returns no data. - Updated useSettings hook to manage loading and error states more effectively, allowing for retries on failure. - Enhanced tests for supplier invoice creation to ensure document IDs are persisted correctly for cash method payments. * feat(salary): enable monthly salary edits in draft runs and handle zero-total declarations
2026-06-08 07:37:24 +02:00
parent 32af88f9c4
commit 809120c4b8
24 changed files with 1244 additions and 131 deletions
@@ -17,7 +17,7 @@ import { roundOre } from '@/lib/money'
 import { formatVoucher } from '@/lib/bookkeeping/voucher-series-resolver'
 import { AccountNumber } from '@/components/ui/account-number'
 import { ReportExportMenu } from '@/components/reports/ReportExportMenu'
-import { useSettings } from '@/components/settings/useSettings'
+import { useCompanySettings } from '@/components/settings/useSettings'
 import { TrialBalanceChart } from '@/components/reports/TrialBalanceChart'
 import { VatCompositionChart } from '@/components/reports/VatCompositionChart'
 import { SkatteverketPanel } from '@/components/reports/SkatteverketPanel'
@@ -1011,10 +1011,10 @@ export function VatDeclarationView({
  // (moms_period in Inställningar) so the picker mirrors the setting instead of
  // always starting on quarterly. Applied once per company the first time its
  // settings load; a later manual change to the picker is preserved, and a
-  // company switch re-applies the new company's setting. `useSettings` only
-  // refetches when the active company changes, so this never clobbers a manual
-  // selection mid-session.
-  const { settings } = useSettings()
+  // company switch re-applies the new company's setting. `useCompanySettings`
+  // only refetches when the active company changes, so this never clobbers a
+  // manual selection mid-session.
+  const { settings } = useCompanySettings()
  const appliedForCompany = useRef<string | null>(null)
  useEffect(() => {
    const momsPeriod = settings?.moms_period
@@ -0,0 +1,24 @@
+'use client'
+
+import { useTranslations } from 'next-intl'
+import { AlertCircle } from 'lucide-react'
+import { Button } from '@/components/ui/button'
+
+/**
+ * Shown when a settings section's fetch settled without a row (or errored).
+ * Replaces the old behaviour where a null `settings` object left the loading
+ * skeleton on screen indefinitely — a settled fetch always resolves to either
+ * content or this retryable state.
+ */
+export function SettingsLoadError({ onRetry }: { onRetry: () => void }) {
+  const t = useTranslations('common')
+  return (
+    <div className="flex flex-col items-center justify-center gap-4 py-12 text-center">
+      <AlertCircle className="h-8 w-8 text-muted-foreground" aria-hidden />
+      <p className="text-sm text-muted-foreground">{t('load_error')}</p>
+      <Button variant="outline" size="sm" onClick={onRetry}>
+        {t('retry')}
+      </Button>
+    </div>
+  )
+}
@@ -1,25 +1,26 @@
 import { Skeleton } from '@/components/ui/skeleton'

+/**
+ * Placeholder shown while a settings section's data loads. Mirrors the real shape
+ * of the section forms — an uppercase section heading followed by stacked
+ * label/field rows, with a hairline divider between blocks — so the swap to live
+ * content doesn't jump from a mismatched layout.
+ */
 export function SettingsLoadingSkeleton() {
  return (
-    <div className="space-y-8 animate-in fade-in duration-300">
-      {[1, 2].map(i => (
-        <div key={i} className="space-y-4">
-          <Skeleton className="h-3.5 w-24" />
-          <div className="grid grid-cols-2 gap-4">
-            <div className="space-y-2">
-              <Skeleton className="h-3.5 w-20" />
-              <Skeleton className="h-10" />
+    <div className="space-y-8 animate-in fade-in duration-300" aria-busy="true">
+      {[0, 1].map((block) => (
+        <div
+          key={block}
+          className={block === 0 ? 'space-y-4' : 'space-y-4 border-t border-border pt-8'}
+        >
+          <Skeleton className="h-3.5 w-32" />
+          {[0, 1, 2].map((row) => (
+            <div key={row} className="space-y-2">
+              <Skeleton className="h-3.5 w-24" />
+              <Skeleton className="h-10 w-full max-w-md" />
            </div>
-            <div className="space-y-2">
-              <Skeleton className="h-3.5 w-28" />
-              <Skeleton className="h-10" />
-            </div>
-          </div>
-          <div className="space-y-2">
-            <Skeleton className="h-3.5 w-16" />
-            <Skeleton className="h-10" />
-          </div>
+          ))}
        </div>
      ))}
    </div>
@@ -3,6 +3,7 @@
 import { Suspense } from 'react'
 import { SettingsRail } from './SettingsRail'
 import { SettingsLoadingSkeleton } from './SettingsLoadingSkeleton'
+import { SettingsProvider } from './useSettings'
 import { SETTINGS_SECTIONS } from './sections'

 interface SettingsShellProps {
@@ -23,33 +24,39 @@ export function SettingsShell({ variant, activeSection, children }: SettingsShel
  if (variant === 'modal') {
    const Section = activeSection ? SETTINGS_SECTIONS[activeSection] : undefined
    return (
-      <div className="flex min-h-0 flex-1">
-        <aside className="hidden w-56 shrink-0 overflow-y-auto border-r border-border p-3 md:block">
-          <SettingsRail variant="modal" display="rail" activeId={activeSection} />
-        </aside>
-        <div className="min-h-0 flex-1 overflow-y-auto p-6">
-          <div className="mb-6 md:hidden">
-            <SettingsRail variant="modal" display="select" activeId={activeSection} />
+      // One provider per open shell: the section it wraps is swapped on tab change
+      // without remounting the shell, so the settings fetch is shared across tabs.
+      <SettingsProvider>
+        <div className="flex min-h-0 flex-1">
+          <aside className="hidden w-56 shrink-0 overflow-y-auto border-r border-border p-3 md:block">
+            <SettingsRail variant="modal" display="rail" activeId={activeSection} />
+          </aside>
+          <div className="min-h-0 flex-1 overflow-y-auto p-6">
+            <div className="mb-6 md:hidden">
+              <SettingsRail variant="modal" display="select" activeId={activeSection} />
+            </div>
+            <Suspense fallback={<SettingsLoadingSkeleton />}>
+              {Section ? <Section /> : null}
+            </Suspense>
          </div>
-          <Suspense fallback={<SettingsLoadingSkeleton />}>
-            {Section ? <Section /> : null}
-          </Suspense>
        </div>
-      </div>
+      </SettingsProvider>
    )
  }

  return (
-    <div className="grid gap-8 md:grid-cols-[220px_1fr]">
-      <aside className="md:sticky md:top-8 md:self-start">
-        <div className="mb-4 md:hidden">
-          <SettingsRail variant="page" display="select" />
-        </div>
-        <div className="hidden md:block">
-          <SettingsRail variant="page" display="rail" />
-        </div>
-      </aside>
-      <div className="min-w-0">{children}</div>
-    </div>
+    <SettingsProvider>
+      <div className="grid gap-8 md:grid-cols-[220px_1fr]">
+        <aside className="md:sticky md:top-8 md:self-start">
+          <div className="mb-4 md:hidden">
+            <SettingsRail variant="page" display="select" />
+          </div>
+          <div className="hidden md:block">
+            <SettingsRail variant="page" display="rail" />
+          </div>
+        </aside>
+        <div className="min-w-0">{children}</div>
+      </div>
+    </SettingsProvider>
  )
 }
@@ -4,6 +4,7 @@ import Link from 'next/link'
 import { useState } from 'react'
 import { useTranslations } from 'next-intl'
 import { SettingsFormWrapper } from '@/components/settings/SettingsFormWrapper'
+import { SettingsLoadError } from '@/components/settings/SettingsLoadError'
 import { SettingsLoadingSkeleton } from '@/components/settings/SettingsLoadingSkeleton'
 import { PeriodLockingSettings } from '@/components/settings/PeriodLockingSettings'
 import { VoucherSeriesManager } from '@/components/settings/VoucherSeriesManager'
@@ -20,7 +21,7 @@ const SERIES_OPTIONS = 'ABCDEFGHIJKLMNOPQRSTUVWXYZ'.split('')

 export function BookkeepingSettingsContent() {
  const t = useTranslations('settings_bookkeeping')
-  const { settings, isLoading, updateSettings } = useSettings()
+  const { settings, isLoading, updateSettings, refetch } = useSettings()
  const { company } = useCompany()
  // Local mirror of the company-level accounting_framework so the K2/K3
  // selector can reflect its own saves without waiting for the layout to
@@ -30,7 +31,8 @@ export function BookkeepingSettingsContent() {
    company?.accounting_framework ?? 'k2',
  )

-  if (isLoading || !settings) return <SettingsLoadingSkeleton />
+  if (isLoading) return <SettingsLoadingSkeleton />
+  if (!settings) return <SettingsLoadError onRetry={refetch} />

  function handleSave(formData: FormData) {
    const autoLockValue = formData.get('auto_lock_period_days') as string
@@ -8,15 +8,17 @@ import { CompanyProfileSection } from '@/components/settings/CompanyProfileSecti
 import { FiscalPeriodEditor } from '@/components/settings/FiscalPeriodEditor'
 import { LogoUpload } from '@/components/settings/LogoUpload'
 import { SettingsFormWrapper } from '@/components/settings/SettingsFormWrapper'
+import { SettingsLoadError } from '@/components/settings/SettingsLoadError'
 import { SettingsLoadingSkeleton } from '@/components/settings/SettingsLoadingSkeleton'
 import { useSettings } from '@/components/settings/useSettings'
 import type { CompanySettings } from '@/types'

 export function CompanySettingsContent() {
  const router = useRouter()
-  const { settings, isLoading, updateSettings } = useSettings()
+  const { settings, isLoading, updateSettings, refetch } = useSettings()

-  if (isLoading || !settings) return <SettingsLoadingSkeleton />
+  if (isLoading) return <SettingsLoadingSkeleton />
+  if (!settings) return <SettingsLoadError onRetry={refetch} />

  function handleSave(formData: FormData) {
    const updates: Record<string, unknown> = {
@@ -6,6 +6,7 @@ import { InvoiceSettingsForm } from '@/components/settings/InvoiceSettingsForm'
 import { InvoicePreviewCard } from '@/components/settings/InvoicePreviewCard'
 import { PdfPrintSettings } from '@/components/settings/PdfPrintSettings'
 import { SettingsFormWrapper } from '@/components/settings/SettingsFormWrapper'
+import { SettingsLoadError } from '@/components/settings/SettingsLoadError'
 import { SettingsLoadingSkeleton } from '@/components/settings/SettingsLoadingSkeleton'
 import { useSettings } from '@/components/settings/useSettings'
 import { useToast } from '@/components/ui/use-toast'
@@ -14,10 +15,11 @@ import type { CompanySettings } from '@/types'

 export function InvoicingSettingsContent() {
  const t = useTranslations('settings_invoicing')
-  const { settings, isLoading, updateSettings } = useSettings()
+  const { settings, isLoading, updateSettings, refetch } = useSettings()
  const { toast } = useToast()

-  if (isLoading || !settings) return <SettingsLoadingSkeleton />
+  if (isLoading) return <SettingsLoadingSkeleton />
+  if (!settings) return <SettingsLoadError onRetry={refetch} />

  function handleSave(formData: FormData) {
    const bankErrors = validateBankFields(formData)
@@ -1,46 +1,27 @@
 'use client'

-import { useEffect, useState } from 'react'
+import { useEffect } from 'react'
 import { useTranslations } from 'next-intl'
 import { useSearchParams, useRouter } from 'next/navigation'
 import { TaxSettingsForm } from '@/components/settings/TaxSettingsForm'
 import { SettingsFormWrapper } from '@/components/settings/SettingsFormWrapper'
+import { SettingsLoadError } from '@/components/settings/SettingsLoadError'
 import { SettingsLoadingSkeleton } from '@/components/settings/SettingsLoadingSkeleton'
 import { SkatteverketConnectPanel } from '@/components/settings/SkatteverketConnectPanel'
 import { useSettings } from '@/components/settings/useSettings'
 import { useToast } from '@/components/ui/use-toast'
-import { useCompany } from '@/contexts/CompanyContext'
-import { createClient } from '@/lib/supabase/client'
 import { ENABLED_EXTENSION_IDS } from '@/lib/extensions/_generated/enabled-extensions'
 import type { CompanySettings } from '@/types'

 export function TaxSettingsContent() {
-  const { settings, isLoading, updateSettings } = useSettings()
-  const { company } = useCompany()
+  const { settings, isLoading, updateSettings, refetch } = useSettings()
  const t = useTranslations('settings_skatteverket')
  const searchParams = useSearchParams()
  const router = useRouter()
  const { toast } = useToast()

-  const [isSandbox, setIsSandbox] = useState(false)
-
  const hasSkatteverketExtension = ENABLED_EXTENSION_IDS.has('skatteverket')

-  // Sandbox companies don't connect to the real Skatteverket — hide the panel,
-  // matching the old Skatteverket tab's visibility gate.
-  useEffect(() => {
-    if (!company?.id) return
-    const supabase = createClient()
-    supabase
-      .from('company_settings')
-      .select('is_sandbox')
-      .eq('company_id', company.id)
-      .single()
-      .then(({ data }) => {
-        if (data?.is_sandbox) setIsSandbox(true)
-      })
-  }, [company?.id])
-
  // Skatteverket OAuth callback — the connect flow returns to /settings/tax with
  // a status query param (returnTo set in SkatteverketConnectPanel).
  useEffect(() => {
@@ -61,7 +42,8 @@ export function TaxSettingsContent() {
    }
  }, [searchParams, router, toast, t])

-  if (isLoading || !settings) return <SettingsLoadingSkeleton />
+  if (isLoading) return <SettingsLoadingSkeleton />
+  if (!settings) return <SettingsLoadError onRetry={refetch} />

  function handleSave(formData: FormData) {
    const vatRegistered = formData.get('vat_registered') === 'true'
@@ -88,7 +70,10 @@ export function TaxSettingsContent() {
    }
  }

-  const showSkatteverket = hasSkatteverketExtension && !isSandbox
+  // Sandbox companies don't connect to the real Skatteverket — hide the panel,
+  // matching the old Skatteverket tab's visibility gate. Read straight off the
+  // already-loaded settings row (no separate query needed).
+  const showSkatteverket = hasSkatteverketExtension && !settings.is_sandbox

  return (
    <div className="space-y-8">
@@ -1,41 +1,107 @@
 'use client'

-import { useState, useEffect, useCallback } from 'react'
-import { useRouter } from 'next/navigation'
+import {
+  createContext,
+  createElement,
+  useCallback,
+  useContext,
+  useEffect,
+  useState,
+  type ReactNode,
+} from 'react'
 import { createClient } from '@/lib/supabase/client'
 import { useCompany } from '@/contexts/CompanyContext'
 import type { CompanySettings } from '@/types'

-export function useSettings() {
-  const router = useRouter()
+export interface SettingsState {
+  settings: CompanySettings | null
+  /** True while the fetch for the active company is in flight. */
+  isLoading: boolean
+  /** True once a fetch finished without a row (or errored) — distinct from loading. */
+  error: boolean
+  updateSettings: (updates: Partial<CompanySettings>) => void
+  refetch: () => Promise<void>
+}
+
+/**
+ * Standalone settings fetcher: loads `company_settings` for the active company
+ * (resolved from CompanyContext). Use this OUTSIDE the settings surface (e.g. the
+ * reports VAT view). Inside the settings surface, read the shared instance with
+ * `useSettings()` instead — `SettingsProvider` mounts exactly one of these so
+ * switching sections reuses the loaded data rather than refetching.
+ *
+ * Auth is already enforced by middleware before any authenticated page renders,
+ * so this no longer round-trips `auth.getUser()` — it gates purely on the
+ * resolved company id, removing a request from the path the skeleton waits on.
+ */
+export function useCompanySettings(): SettingsState {
  const { company } = useCompany()
  const [settings, setSettings] = useState<CompanySettings | null>(null)
  const [isLoading, setIsLoading] = useState(true)
+  const [error, setError] = useState(false)

  const fetchSettings = useCallback(async () => {
-    const supabase = createClient()
-    const { data: { user } } = await supabase.auth.getUser()
-    if (!user) { router.push('/login'); return }
-
-    if (company?.id) {
-      const { data } = await supabase
-        .from('company_settings')
-        .select('*')
-        .eq('company_id', company.id)
-        .single()
-      setSettings(data)
+    if (!company?.id) {
+      // No active company (the no-company escape hatch). Nothing to load; surface
+      // a settled empty state rather than a perpetual spinner.
+      setSettings(null)
+      setError(false)
+      setIsLoading(false)
+      return
    }

+    setIsLoading(true)
+    setError(false)
+
+    const supabase = createClient()
+    // maybeSingle() so a missing row resolves to { data: null } instead of
+    // throwing PGRST116 — a company created outside the onboarding flow may have
+    // no company_settings row yet, and that must not be treated as a hard error
+    // mid-query (it's surfaced as `error` below once the fetch settles).
+    const { data, error: queryError } = await supabase
+      .from('company_settings')
+      .select('*')
+      .eq('company_id', company.id)
+      .maybeSingle()
+
+    setSettings(data)
+    setError(Boolean(queryError) || !data)
    setIsLoading(false)
-  }, [company?.id, router])
+  }, [company?.id])

  useEffect(() => {
    fetchSettings()
  }, [fetchSettings])

  const updateSettings = useCallback((updates: Partial<CompanySettings>) => {
-    setSettings(prev => prev ? { ...prev, ...updates } as CompanySettings : null)
+    setSettings((prev) => (prev ? ({ ...prev, ...updates } as CompanySettings) : prev))
  }, [])

-  return { settings, isLoading, updateSettings, refetch: fetchSettings }
+  return { settings, isLoading, error, updateSettings, refetch: fetchSettings }
+}
+
+const SettingsContext = createContext<SettingsState | null>(null)
+
+/**
+ * Hosts one shared settings fetch for the whole settings surface. Mounted once by
+ * `SettingsShell`, it survives section swaps (the shell re-renders rather than
+ * remounting when the active section changes), so moving between settings tabs
+ * reuses the loaded data instead of refetching and re-flashing the skeleton.
+ */
+export function SettingsProvider({ children }: { children: ReactNode }) {
+  const value = useCompanySettings()
+  return createElement(SettingsContext.Provider, { value }, children)
+}
+
+/**
+ * Read the shared settings instance. Must be rendered within a `SettingsProvider`
+ * (every settings section is, via `SettingsShell`). Outside the settings surface,
+ * use `useCompanySettings()` instead.
+ */
+export function useSettings(): SettingsState {
+  const ctx = useContext(SettingsContext)
+  if (!ctx) {
+    throw new Error('useSettings must be used within a SettingsProvider')
+  }
+  return ctx
 }