From 22f0647d6cef56a97d7cd9ff6d6d9df8fd444fd1 Mon Sep 17 00:00:00 2001 From: Jakob Wennberg Date: Fri, 28 Aug 2026 17:15:25 +0200 Subject: [PATCH] feat(bookkeeping): make inline rattelse discoverable on the verifikat page (#1554) (#2011) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(bookkeeping): make inline rättelse discoverable on the verifikat page (#1554) A user who wanted Fortnox-style "stryk rader" went looking on the verifikat page and concluded the feature did not exist: since #1739 every correction action sits behind an icon-only ⋯ menu, nothing says which correction track applies when, and the struck-line marker showed only a date. - Promote "Stryk rader i verifikatet" to a visible outline button for a posted, non-structural entry whose period the period-status endpoint reports as open; the ⋯ item stays so the menu remains the complete list. - Add the convention-7 "?" after the H1 with the two-sentence track rule: inline rättelse while the period is open and unlocked, storno once it is locked, closed or declared. - The rattelse-log route now returns an additive actor_label resolved from profiles via the service client (same precedent as behandlingshistorik); struck rows read "Struken {date} av {actor}" and the Rättelsehistorik rows carry the actor beside the date. No change to the RPCs, the log table, or which corrections are legal. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015nAd8XJ2RPCmG2eKoLBdna * fix(bookkeeping): address review findings on inline rättelse discoverability (#1554) - Tie the un-awaited period-status fetch to the fetchData run that issued it (monotonic request ref), so an earlier response resolving last can no longer set periodStatus='open' for an entry in a locked period and promote the "Stryk rader" button the RPC would refuse. - Align the "?" help copy with what the system enforces: storno is the only path once the period is locked or closed; a VAT-declared month is stated as a caveat (same wording as the StrikeLinesDialog explainer), not as a gate the product does not apply. Both sv and en. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015nAd8XJ2RPCmG2eKoLBdna --------- Co-authored-by: Jakob Wennberg <311770904+jakobwennberg-oss@users.noreply.github.com> Co-authored-by: Claude Fable 5 --- DECISIONS.md | 1 + app/(dashboard)/bookkeeping/[id]/page.tsx | 81 ++++++++++++-- .../[id]/rattelse-log/__tests__/route.test.ts | 102 ++++++++++++++---- .../[id]/rattelse-log/route.ts | 29 ++++- messages/en.json | 3 + messages/sv.json | 3 + 6 files changed, 191 insertions(+), 28 deletions(-) diff --git a/DECISIONS.md b/DECISIONS.md index c930ee88..e98329f2 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -1323,6 +1323,7 @@ One line per decision: `[YYYY-MM-DD] : `. Appended by agents and [2026-08-28] AR-PDF minus fix uses ASCII hyphen formatting, not font embedding: registering a Unicode TTF for react-pdf would change the whole document's typography and bundle size to fix one glyph; formatPdfKronor keeps built-in Helvetica and sidesteps WinAnsi's missing U+2212. [2026-08-28] Same-bank warning limited to observed one-session banks (SEB only): prod shows Handelsbanken tolerates 4 concurrent sessions, and the generic warning made a user abandon a legitimate renewal. Planned sync-death visibility work was dropped: already shipped via #1271 (health probe), #1727 (stale state), #1969 (cron unstarve). [2026-08-28] Same-bank warning revised to three tiers after skeptic refutation: hard warn SEB, silent/calm only for verified multi-session banks (Handelsbanken, 4 distinct session_ids observed), legacy hedged warning for unknown banks (fail closed), shared-session siblings exempt (fan-out carries them). +[2026-08-28] Verifikat page promotes "Stryk rader i verifikatet" to a visible outline button only when the period-status endpoint answers open (anything else or unknown keeps it in the ⋯ menu only), and the ⋯ item stays: inline rättelse is the normal path in an open period (#1554), the promoted button must never invite an action the RPC will refuse, and the menu remains the complete action list per the 2026-08-20 detail-page grammar. Struck-line actor labels are resolved server-side via resolveUserLabelsFromProfiles (profiles RLS is self-only) rather than a new column on the log. [2026-08-28] Employee-save failure reported inline (role=alert in the dialog footer, carrying the requestId) in addition to the single destructive toast, and a missing PERSONNUMMER_ENCRYPTION_KEY typed as 503 PERSONNUMMER_ENCRYPTION_NOT_CONFIGURED rather than INTERNAL_ERROR (#1996): the Radix modal aria-hides the root-layout Toaster while the dialog is open, so the toast is invisible to assistive tech and E2E drivers; TOAST_LIMIT is 1, so a second toast is not an option; and the missing key is a permanent configuration gap where "try again later" is wrong and "contact support" is right (same reasoning as CUSTOMER_PERSONAL_NUMBER_UNREADABLE and INVOICE_SEND_EMAIL_NOT_CONFIGURED). The shared postAction helper was not extended (it takes no body and exposes no requestId): keeping the change local to the dialog avoids widening a helper other panels rely on. [2026-08-28] /migrate SIE guard extended to every provider (Fortnox exemption removed) as "a completed SIE import must exist for the company", not "must be part of this run", plus a wizard hint that disables Start when SIE is unchecked and never imported; chose this over forcing the checkbox on because the route is the only seam a direct API call or a stale client cannot bypass, and "must exist" keeps entities-only re-runs after a full migration working (#2000). [2026-08-28] /migrate SIE guard skips company-info-only runs (all entity flags false) and the wizard derives "SIE already imported" from the preview OR this session's successful /import-sie results: company info writes no accounts, balances or subledger rows, so the BFL rationale does not apply; and the one-shot preview went stale after phase 1 succeeded and phase 2 failed, falsely blocking an entities-only retry (#2000 review). diff --git a/app/(dashboard)/bookkeeping/[id]/page.tsx b/app/(dashboard)/bookkeeping/[id]/page.tsx index 5621a83b..3a309469 100644 --- a/app/(dashboard)/bookkeeping/[id]/page.tsx +++ b/app/(dashboard)/bookkeeping/[id]/page.tsx @@ -1,6 +1,6 @@ 'use client' -import { useState, useEffect, useCallback, use } from 'react' +import { useState, useEffect, useCallback, useRef, use } from 'react' import { useRouter } from 'next/navigation' import Link from 'next/link' import { useTranslations } from 'next-intl' @@ -82,9 +82,14 @@ type RattelseLogRow = { struck_lines: StruckLineSnapshot[] | null added_lines: StruckLineSnapshot[] | null actor: string | null + // Resolved server-side from profiles (rattelse-log route); null when the + // actor is unknown or the lookup failed. + actor_label: string | null created_at: string } +type PeriodStatus = 'open' | 'locked' | 'closed' + /** * Human "who committed this" line from the committed_actor_* snapshot * (WHO relayed the commit; commit_method records HOW). The claude.ai MCP @@ -137,6 +142,16 @@ export default function JournalEntryDetailPage({ params }: { params: Promise<{ i const [showStrikeLines, setShowStrikeLines] = useState(false) const [showCorrectMetadata, setShowCorrectMetadata] = useState(false) const [rattelseLog, setRattelseLog] = useState([]) + // Lock state of the period covering entry_date, from the period-status + // preview endpoint. Gates the visible "Stryk rader" button: inline rättelse + // only applies while the period is open, so the promoted button hides + // (and the ⋯ item stays) whenever the status is anything else or unknown. + const [periodStatus, setPeriodStatus] = useState(null) + // Monotonic id of the latest fetchData run. The period-status fetch is not + // awaited, and fetchData re-runs on every id change and after every dialog + // success, so an earlier response can resolve last; only the response that + // belongs to the most recent run may set periodStatus. + const periodStatusRequestRef = useRef(0) const [showEdit, setShowEdit] = useState(false) const [showRecordate, setShowRecordate] = useState(false) const [showDeleteConfirm, setShowDeleteConfirm] = useState(false) @@ -201,6 +216,26 @@ export default function JournalEntryDetailPage({ params }: { params: Promise<{ i setEntry(data.entry) setChain(data.chain) setIsLastInSeries(data.is_last_in_series ?? false) + // Period lock state for the entry's date, best-effort and not awaited + // (the page paints without it): the endpoint fails closed (a lookup + // failure reads as 'locked'), and a missing answer just keeps the + // strike button in the ⋯ menu only. + setPeriodStatus(null) + const periodRequest = ++periodStatusRequestRef.current + void fetch( + `/api/bookkeeping/fiscal-periods/period-status?date=${encodeURIComponent(data.entry.entry_date)}`, + ) + .then(async (periodRes) => { + if (!periodRes.ok) return + const { data: period } = await periodRes.json() + // A newer fetchData has run since; its own response owns the state. + if (periodRequest !== periodStatusRequestRef.current) return + const status = period?.status + setPeriodStatus(status === 'open' || status === 'locked' || status === 'closed' ? status : null) + }) + .catch(() => { + if (periodRequest === periodStatusRequestRef.current) setPeriodStatus(null) + }) // Underlag references (linked invoices), best-effort; the verifikat still // renders if this fails, it just falls back to documents-only. if (refsRes.ok) { @@ -397,13 +432,20 @@ export default function JournalEntryDetailPage({ params }: { params: Promise<{ i const struckDisplayLines = rattelseLog .filter((r) => r.rattelse_type === 'lines') .flatMap((r) => - (r.struck_lines ?? []).map((s) => ({ ...s, struck_at: r.created_at })) + (r.struck_lines ?? []).map((s) => ({ ...s, struck_at: r.created_at, struck_by: r.actor_label })) ) + // The struck marker beside a struck row: who and when at a glance, the + // date alone when the actor could not be resolved. + const struckMarker = (s: { struck_at: string; struck_by: string | null }) => + s.struck_by + ? t('struck_marker_by', { date: formatDate(s.struck_at), actor: s.struck_by }) + : t('struck_marker', { date: formatDate(s.struck_at) }) + // Live and struck lines interleaved by original position. const displayRows: Array< | { kind: 'live'; line: JournalEntryLine } - | { kind: 'struck'; line: StruckLineSnapshot & { struck_at: string } } + | { kind: 'struck'; line: StruckLineSnapshot & { struck_at: string; struck_by: string | null } } > = [ ...lines.map((l) => ({ kind: 'live' as const, line: l })), ...struckDisplayLines.map((s) => ({ kind: 'struck' as const, line: s })), @@ -490,6 +532,11 @@ export default function JournalEntryDetailPage({ params }: { params: Promise<{ i const showActions = entry.status === 'posted' || entry.status === 'draft' const showDelete = entry.status === 'draft' || isLastInSeries const showRattelseGroup = canCorrect && !isOpeningBalance + // Inline rättelse is the normal correction path while the period is open, + // so "Stryk rader" is promoted to a visible secondary button there (#1554: + // a user who never opens the ⋯ menu concluded the feature did not exist). + // The ⋯ item stays, so the menu remains the complete action list. + const showStrikeButton = showRattelseGroup && canInlineRattelse && periodStatus === 'open' const underlagAside = (() => { if (attachmentCount === 0 && references.length === 0) { @@ -531,6 +578,11 @@ export default function JournalEntryDetailPage({ params }: { params: Promise<{ i
{/* data-ph-mask: the title carries the voucher number */}

{title}

+ {/* Convention 7: which correction track applies when, behind the "?" */} + +

{t('help_rattelse_tracks_open')}

+

{t('help_rattelse_tracks_locked')}

+ {rattelseLog.length > 0 && ( @@ -576,6 +628,17 @@ export default function JournalEntryDetailPage({ params }: { params: Promise<{ i {t('correct_opening_balances')} )} + {showStrikeButton && ( + + )} @@ -774,9 +837,7 @@ export default function JournalEntryDetailPage({ params }: { params: Promise<{ i {s.line_description || ''} - - {t('struck_marker', { date: formatDate(s.struck_at) })} - + {struckMarker(s)} {Number(s.debit_amount) > 0 && fmtAmount(s.debit_amount)} @@ -852,7 +913,7 @@ export default function JournalEntryDetailPage({ params }: { params: Promise<{ i {s.line_description && (

{s.line_description}

)} -

{t('struck_marker', { date: formatDate(s.struck_at) })}

+

{struckMarker(s)}

{Number(s.debit_amount) > 0 &&

{fmtAmount(s.debit_amount)} D

} @@ -963,7 +1024,11 @@ export default function JournalEntryDetailPage({ params }: { params: Promise<{ i {rattelseLog.map((row) => (
  • - {formatDate(row.created_at)} + {/* data-ph-mask: the actor label is a person's e-mail or name */} + + {formatDate(row.created_at)} + {row.actor_label ? ` · ${row.actor_label}` : ''} + {row.rattelse_type === 'metadata' ? t('rattelse_kind_metadata') : t('rattelse_kind_lines')} diff --git a/app/api/bookkeeping/journal-entries/[id]/rattelse-log/__tests__/route.test.ts b/app/api/bookkeeping/journal-entries/[id]/rattelse-log/__tests__/route.test.ts index 7a6d9dee..14ee7ae7 100644 --- a/app/api/bookkeeping/journal-entries/[id]/rattelse-log/__tests__/route.test.ts +++ b/app/api/bookkeeping/journal-entries/[id]/rattelse-log/__tests__/route.test.ts @@ -12,6 +12,8 @@ import { } from '@/tests/helpers' const { supabase, enqueue, reset } = createQueuedMockSupabase() +// Service-role client for the profiles lookup (profiles RLS is self-only). +const service = createQueuedMockSupabase() const requireAuthMock = vi.fn() vi.mock('@/lib/auth/require-auth', () => ({ @@ -25,17 +27,44 @@ vi.mock('@/lib/company/context', () => ({ vi.mock('@/lib/init', () => ({ ensureInitialized: vi.fn() })) +const createServiceClientMock = vi.fn() +vi.mock('@/lib/supabase/server', () => ({ + createClient: vi.fn(), + createServiceClient: (...args: unknown[]) => createServiceClientMock(...args), +})) + import { GET } from '../route' const params = () => createMockRouteParams({ id: 'entry-1' }) const makeGet = () => createMockRequest('/api/bookkeeping/journal-entries/entry-1/rattelse-log', { method: 'GET' }) +const linesRow = (id: string, actor: string | null, created_at: string) => ({ + id, + rattelse_type: 'lines', + old_description: null, + new_description: null, + old_entry_date: null, + new_entry_date: null, + struck_lines: [ + { id: `${id}-struck`, account_number: '5410', debit_amount: 500, credit_amount: 0, line_description: null, sort_order: 1 }, + ], + added_lines: [ + { id: `${id}-added`, account_number: '5420', debit_amount: 500, credit_amount: 0, line_description: null, sort_order: 3 }, + ], + actor, + created_at, +}) + +type LogRow = { id: string; rattelse_type: string; actor: string | null; actor_label: string | null } + describe('GET /api/bookkeeping/journal-entries/[id]/rattelse-log', () => { beforeEach(() => { vi.clearAllMocks() reset() + service.reset() requireAuthMock.mockResolvedValue({ user: { id: 'user-1' }, supabase }) + createServiceClientMock.mockReturnValue(service.supabase) }) it('returns 401 when not authenticated', async () => { @@ -54,28 +83,14 @@ describe('GET /api/bookkeeping/journal-entries/[id]/rattelse-log', () => { expect(response.status).toBe(404) expect(body.error).toContain('hittades inte') + expect(createServiceClientMock).not.toHaveBeenCalled() }) - it('returns the rättelse rows newest first', async () => { + it('returns the rättelse rows newest first with the actor resolved from profiles', async () => { enqueue({ data: { id: 'entry-1' }, error: null }) // ownership check enqueue({ data: [ - { - id: 'log-2', - rattelse_type: 'lines', - old_description: null, - new_description: null, - old_entry_date: null, - new_entry_date: null, - struck_lines: [ - { id: 'line-1', account_number: '5410', debit_amount: 500, credit_amount: 0, line_description: null, sort_order: 1 }, - ], - added_lines: [ - { id: 'line-9', account_number: '5420', debit_amount: 500, credit_amount: 0, line_description: null, sort_order: 3 }, - ], - actor: 'user-1', - created_at: '2026-07-23T12:00:00Z', - }, + linesRow('log-2', 'user-1', '2026-07-23T12:00:00Z'), { id: 'log-1', rattelse_type: 'metadata', @@ -91,14 +106,65 @@ describe('GET /api/bookkeeping/journal-entries/[id]/rattelse-log', () => { ], error: null, }) + service.enqueue({ + data: [{ id: 'user-1', email: 'anna@example.se', full_name: null }], + error: null, + }) const response = await GET(makeGet(), params()) - const { body } = await parseJsonResponse<{ data: { id: string; rattelse_type: string }[] }>(response) + const { body } = await parseJsonResponse<{ data: LogRow[] }>(response) expect(response.status).toBe(200) expect(body.data).toHaveLength(2) expect(body.data[0].id).toBe('log-2') expect(body.data[0].rattelse_type).toBe('lines') + // Raw actor uuid is kept; the label is additive. + expect(body.data[0].actor).toBe('user-1') + expect(body.data[0].actor_label).toBe('anna@example.se') + expect(body.data[1].actor_label).toBe('anna@example.se') + // One lookup, scoped to exactly the distinct actor ids in the log rows. + expect(service.findCalls('profiles', 'in')).toEqual([['id', ['user-1']]]) + }) + + it('leaves actor_label null and skips the profiles lookup when no row has an actor', async () => { + enqueue({ data: { id: 'entry-1' }, error: null }) // ownership check + enqueue({ data: [linesRow('log-3', null, '2026-07-23T12:00:00Z')], error: null }) + + const response = await GET(makeGet(), params()) + const { body } = await parseJsonResponse<{ data: LogRow[] }>(response) + + expect(response.status).toBe(200) + expect(body.data).toHaveLength(1) + expect(body.data[0].actor_label).toBeNull() + expect(createServiceClientMock).not.toHaveBeenCalled() + expect(service.findCalls('profiles', 'in')).toEqual([]) + }) + + it('still returns 200 with actor_label null when the profiles lookup fails', async () => { + enqueue({ data: { id: 'entry-1' }, error: null }) // ownership check + enqueue({ data: [linesRow('log-4', 'user-2', '2026-07-23T12:00:00Z')], error: null }) + service.enqueue({ data: null, error: { message: 'permission denied' } }) + + const response = await GET(makeGet(), params()) + const { body } = await parseJsonResponse<{ data: LogRow[] }>(response) + + expect(response.status).toBe(200) + expect(body.data[0].actor).toBe('user-2') + expect(body.data[0].actor_label).toBeNull() + }) + + it('still returns 200 with actor_label null when the service client cannot be created', async () => { + enqueue({ data: { id: 'entry-1' }, error: null }) // ownership check + enqueue({ data: [linesRow('log-5', 'user-2', '2026-07-23T12:00:00Z')], error: null }) + createServiceClientMock.mockImplementation(() => { + throw new Error('SUPABASE_SERVICE_ROLE_KEY missing') + }) + + const response = await GET(makeGet(), params()) + const { body } = await parseJsonResponse<{ data: LogRow[] }>(response) + + expect(response.status).toBe(200) + expect(body.data[0].actor_label).toBeNull() }) it('returns 500 with a Swedish message when the query fails', async () => { diff --git a/app/api/bookkeeping/journal-entries/[id]/rattelse-log/route.ts b/app/api/bookkeeping/journal-entries/[id]/rattelse-log/route.ts index 914ff175..887521c2 100644 --- a/app/api/bookkeeping/journal-entries/[id]/rattelse-log/route.ts +++ b/app/api/bookkeeping/journal-entries/[id]/rattelse-log/route.ts @@ -1,5 +1,7 @@ import { NextResponse } from 'next/server' import { withRouteContext } from '@/lib/api/with-route-context' +import { createServiceClient } from '@/lib/supabase/server' +import { resolveUserLabelsFromProfiles } from '@/lib/reports/behandlingshistorik' /** * GET /api/bookkeeping/journal-entries/[id]/rattelse-log @@ -7,7 +9,9 @@ import { withRouteContext } from '@/lib/api/with-route-context' * The entry's inline rättelse history (BFL 5 kap 5 § / 9 §): the immutable * who/when trail behind every metadata edit and line strike, newest first. * Struck lines render with strikethrough in the verifikat detail view from - * the struck_lines snapshots here. + * the struck_lines snapshots here. Each row also carries `actor_label`, the + * actor's profile label, so the page can say who struck a line without the + * reader opening a log panel; the raw `actor` uuid is kept unchanged. */ export const GET = withRouteContext<{ params: Promise<{ id: string }> }>( 'bookkeeping.journal_entry.rattelse_log', @@ -41,6 +45,27 @@ export const GET = withRouteContext<{ params: Promise<{ id: string }> }>( return NextResponse.json({ error: 'Kunde inte hämta rättelsehistorik' }, { status: 500 }) } - return NextResponse.json({ data: data ?? [] }) + const rows = (data ?? []) as ({ actor: string | null } & Record)[] + + // Who: `profiles` RLS is self-only, so the label lookup goes through the + // service client, scoped to exactly the actor ids that already appear in + // this company's own log rows (same precedent as behandlingshistorik). + // Best-effort: a failed lookup leaves the label null, never the response. + const actorIds = Array.from(new Set(rows.map((r) => r.actor).filter((a): a is string => !!a))) + let labels = new Map() + if (actorIds.length > 0) { + try { + labels = await resolveUserLabelsFromProfiles(createServiceClient(), actorIds) + } catch { + labels = new Map() + } + } + + return NextResponse.json({ + data: rows.map((row) => ({ + ...row, + actor_label: row.actor ? (labels.get(row.actor) ?? null) : null, + })), + }) }, ) diff --git a/messages/en.json b/messages/en.json index 1526ed5d..dc7695ba 100644 --- a/messages/en.json +++ b/messages/en.json @@ -5236,6 +5236,9 @@ "strike_lines": "Strike lines in the voucher", "correct_metadata": "Change text or date", "struck_marker": "Struck {date}", + "struck_marker_by": "Struck {date} by {actor}", + "help_rattelse_tracks_open": "While the period is open and unlocked, a posted voucher is corrected inside the voucher itself: strike lines or change the text and date. The original stays visible with a strikethrough and every correction is logged with who and when.", + "help_rattelse_tracks_locked": "Once the period is locked or closed, storno is the only path: the voucher is reversed and, if needed, replaced by a correction voucher. If the month has already been VAT-declared, correcting VAT accounts can affect the filed declaration.", "rattelse_history_title": "Correction history", "rattelse_kind_metadata": "Text/date corrected", "rattelse_kind_lines": "Lines struck and replaced", diff --git a/messages/sv.json b/messages/sv.json index e8820502..62751c51 100644 --- a/messages/sv.json +++ b/messages/sv.json @@ -5236,6 +5236,9 @@ "strike_lines": "Stryk rader i verifikatet", "correct_metadata": "Ändra text eller datum", "struck_marker": "Struken {date}", + "struck_marker_by": "Struken {date} av {actor}", + "help_rattelse_tracks_open": "I en öppen, olåst period rättar du ett bokfört verifikat direkt i verifikatet: stryk rader eller ändra text och datum. Originalet förblir synligt överstruket och varje rättelse loggas med vem och när.", + "help_rattelse_tracks_locked": "När perioden är låst eller stängd är storno den enda vägen: verifikatet återförs och ersätts vid behov av ett ändringsverifikat. Om månaden redan är momsdeklarerad kan en rättelse av momskonton påverka den inlämnade deklarationen.", "rattelse_history_title": "Rättelsehistorik", "rattelse_kind_metadata": "Text/datum rättat", "rattelse_kind_lines": "Rader strukna och ersatta",