From cac692e2934f2262d39da8cebce2a722c165ec54 Mon Sep 17 00:00:00 2001 From: Jakob Wennberg <149234542+jakobwennberg@users.noreply.github.com> Date: Fri, 5 Jun 2026 09:37:26 +0200 Subject: [PATCH] fix(ux): book documents directly from inbox + attach existing underlag when booking transactions (#670) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(inbox): re-add Bokför manuellt on unmatched documents Pilot feedback: a document in Dokumentinkorg could not be booked without first matching it to a bank transaction, which is impossible for cash expenses and other entries with no bank movement. The backend (/items/:id/book-direct) and BookDirectlyDialog already support standalone booking — re-expose the button in the unmatched state. The dialog still offers optional transaction selection inside. Co-Authored-By: Claude Opus 4.8 (1M context) * feat(transactions): pick existing inbox document when booking manually Pilot feedback: "Bokför manuellt" from a transaction only allowed uploading new files — an already-uploaded underlag from the inbox could not be attached. Add a select mode to InboxDocumentPicker (onSelect prop; journalEntryId now optional) and mount it in TransactionBookingDialog: picked documents are linked after the journal entry is created via /api/documents/{id}/link with inbox_item_id, which also stamps the inbox item as consumed so it drops out of every pending surface. Non-ok link responses now count toward the failure toast (previously only network errors did). Co-Authored-By: Claude Opus 4.8 (1M context) * fix(documents): address PR #670 review — stale preview dialog, JE tenancy check Review findings: - InboxDocumentPicker left the preview dialog floating open when a pick was confirmed from inside it (previewItem was never cleared before onClose; the component stays mounted, so the on-open reset never ran). Clear it in both select and link mode. (greptile) - linkToJournalEntry verified the document's company but trusted the client-supplied journal_entry_id (FK only requires existence). Add an explicit company-scoped journal entry lookup; misses map to the existing DOC_LINK_ENTRY_NOT_FOUND envelope. RLS prevented any data leak either way — this makes the rejection explicit. New regression test covers the cross-tenant case. (compliance-swarm A.8.28) Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Claude Opus 4.8 (1M context) --- .../[id]/link/__tests__/route.test.ts | 18 +++ .../bookkeeping/InboxDocumentPicker.tsx | 46 +++++-- .../general/InvoiceInboxWorkspace.tsx | 21 ++- .../transactions/TransactionBookingDialog.tsx | 126 ++++++++++++++---- lib/core/documents/document-service.ts | 15 +++ messages/en.json | 2 + messages/sv.json | 2 + 7 files changed, 188 insertions(+), 42 deletions(-) diff --git a/app/api/documents/[id]/link/__tests__/route.test.ts b/app/api/documents/[id]/link/__tests__/route.test.ts index 497c0341..993c1234 100644 --- a/app/api/documents/[id]/link/__tests__/route.test.ts +++ b/app/api/documents/[id]/link/__tests__/route.test.ts @@ -74,6 +74,7 @@ describe('POST /api/documents/[id]/link', () => { }) it('links the document and stamps the inbox item when inbox_item_id is given', async () => { + enqueue({ data: { id: 'je-1' } }) // journal entry company check enqueue({ data: { id: 'doc-1', journal_entry_id: 'je-1', file_name: 'x.pdf' } }) // link update enqueue({ data: null }) // inbox stamp update @@ -90,6 +91,7 @@ describe('POST /api/documents/[id]/link', () => { }) it('does not touch the inbox when no inbox_item_id is given', async () => { + enqueue({ data: { id: 'je-1' } }) // journal entry company check enqueue({ data: { id: 'doc-1', journal_entry_id: 'je-1', file_name: 'x.pdf' } }) // link update const res = await POST( @@ -103,6 +105,7 @@ describe('POST /api/documents/[id]/link', () => { }) it('maps a period-lock trigger error to PERIOD_LOCKED', async () => { + enqueue({ data: { id: 'je-1' } }) // journal entry company check enqueue({ data: null, error: { message: 'new row violates ... locked/closed fiscal period' }, @@ -118,6 +121,7 @@ describe('POST /api/documents/[id]/link', () => { }) it('maps an already-linked error to DOC_LINK_ALREADY_LINKED', async () => { + enqueue({ data: { id: 'je-1' } }) // journal entry company check enqueue({ data: null, error: { message: 'document already linked to another entry' }, @@ -129,4 +133,18 @@ describe('POST /api/documents/[id]/link', () => { const { body } = await parseJsonResponse<{ error: { code: string } }>(res) expect(body.error.code).toBe('DOC_LINK_ALREADY_LINKED') }) + + it('rejects a journal entry outside the active company with DOC_LINK_ENTRY_NOT_FOUND', async () => { + // The company-scoped lookup finds no row — same result whether the id is + // bogus or belongs to another tenant. The document must never be updated. + enqueue({ data: null }) // journal entry company check → no match + const res = await POST( + makeReq({ journal_entry_id: 'je-other-company' }), + createMockRouteParams({ id: 'doc-1' }), + ) + const { body } = await parseJsonResponse<{ error: { code: string } }>(res) + expect(body.error.code).toBe('DOC_LINK_ENTRY_NOT_FOUND') + expect(mockSupabase.from).not.toHaveBeenCalledWith('document_attachments') + expect(mockSupabase.from).not.toHaveBeenCalledWith('invoice_inbox_items') + }) }) diff --git a/components/bookkeeping/InboxDocumentPicker.tsx b/components/bookkeeping/InboxDocumentPicker.tsx index be674ea9..7a9dd647 100644 --- a/components/bookkeeping/InboxDocumentPicker.tsx +++ b/components/bookkeeping/InboxDocumentPicker.tsx @@ -20,19 +20,25 @@ import { FileText, ImageIcon, Loader2, Search, Inbox, Eye } from 'lucide-react' // InboxDocumentPicker // -// Opens from JournalEntryAttachments ("Välj från inkorgen"). Lists invoice-inbox -// documents that have not yet been consumed (no supplier invoice, no journal -// entry, not matched to a transaction, document not already linked) so the user -// can attach one as underlag to the current verifikat. Picking one links the -// document to the journal entry AND stamps the inbox item so it drops out of the -// active inbox — see app/api/documents/[id]/link/route.ts. +// Lists invoice-inbox documents that have not yet been consumed (no supplier +// invoice, no journal entry, not matched to a transaction, document not +// already linked) so the user can attach one as underlag to a verifikat. +// +// Two modes: +// - Link mode (JournalEntryAttachments, "Välj från inkorgen"): `journalEntryId` +// is set and picking a document immediately links it to the journal entry AND +// stamps the inbox item so it drops out of the active inbox — see +// app/api/documents/[id]/link/route.ts. +// - Select mode (TransactionBookingDialog, "Välj befintligt underlag"): the +// journal entry does not exist yet, so `onSelect` is provided instead and the +// pick is reported to the parent, which links after the entry is created. // // Each row carries a preview button (eye) that opens a quick dialog rendering // the document inline, so the user can confirm the right file before attaching. // Attaching is the row's primary click (fast path) and is also offered from // inside the preview dialog (preview → confirm). -interface AvailableInboxDoc { +export interface AvailableInboxDoc { inbox_item_id: string document_id: string file_name: string @@ -49,9 +55,12 @@ interface AvailableInboxDoc { interface Props { open: boolean onClose: () => void - journalEntryId: string - /** Called after a successful link so the parent can refresh its document list. */ - onLinked: () => void + /** Link mode: the journal entry to link the picked document to. */ + journalEntryId?: string + /** Link mode: called after a successful link so the parent can refresh its document list. */ + onLinked?: () => void + /** Select mode: report the picked document to the parent instead of linking. */ + onSelect?: (doc: AvailableInboxDoc) => void } function isImageType(type: string | null): boolean { @@ -69,7 +78,7 @@ function DocIcon({ mime }: { mime: string | null }) { return } -export default function InboxDocumentPicker({ open, onClose, journalEntryId, onLinked }: Props) { +export default function InboxDocumentPicker({ open, onClose, journalEntryId, onLinked, onSelect }: Props) { const t = useTranslations('journal_attachments') const { toast } = useToast() @@ -113,6 +122,18 @@ export default function InboxDocumentPicker({ open, onClose, journalEntryId, onL }, [items, search]) async function handlePick(item: AvailableInboxDoc) { + // Select mode: the journal entry doesn't exist yet — hand the pick to the + // parent and let it link after creation. Clear the preview first: the + // preview dialog's open state is `previewItem !== null`, so leaving it set + // would strand a floating preview after the picker closes (the component + // stays mounted; the on-open reset only runs on the next open). + if (onSelect) { + setPreviewItem(null) + onSelect(item) + onClose() + return + } + if (!journalEntryId) return setLinkingId(item.document_id) try { const res = await fetch(`/api/documents/${item.document_id}/link`, { @@ -133,7 +154,8 @@ export default function InboxDocumentPicker({ open, onClose, journalEntryId, onL return } toast({ title: t('picker_linked') }) - onLinked() + setPreviewItem(null) + onLinked?.() onClose() } catch { toast({ title: t('picker_link_failed'), variant: 'destructive' }) diff --git a/components/extensions/general/InvoiceInboxWorkspace.tsx b/components/extensions/general/InvoiceInboxWorkspace.tsx index 9e8c3bcf..b78e406c 100644 --- a/components/extensions/general/InvoiceInboxWorkspace.tsx +++ b/components/extensions/general/InvoiceInboxWorkspace.tsx @@ -1630,13 +1630,12 @@ function FieldsRail({ ) : ( <> {/* Unmatched state: the canonical next step is to find the bank - transaction this underlag belongs to. "Skapa leverantörs- - faktura" stays as an escape hatch for users who want - supplier-invoice tracking (accrual flow). The old "Bokför - direkt" escape hatch was removed — its label was unclear - and the deterministic-book-without-bank-tx use case is - covered by "Matcha mot transaktion" → "Bokför manuellt" - (matched state). */} + transaction this underlag belongs to. "Bokför manuellt" opens + BookDirectlyDialog without a transaction — for underlag with + no bank movement (cash expenses, private outlays); the dialog + still offers optional transaction selection inside. "Skapa + leverantörsfaktura" stays as an escape hatch for users who + want supplier-invoice tracking (accrual flow). */} + {showUploadZone && ( -
+
+ {pickedInboxDocs.length > 0 && ( +
+ {pickedInboxDocs.map((doc) => ( +
+ + + {doc.supplier_name ?? doc.file_name} + + {doc.amount != null && ( + + {formatCurrency(doc.amount, doc.currency ?? 'SEK')} + + )} + +
+ ))} +
+ )} +
)}
@@ -226,6 +296,16 @@ export default function TransactionBookingDialog({ sourceId={transaction.id} onEntryCreated={(entryId) => handleBooked(transaction.id, entryId)} /> + + setInboxPickerOpen(false)} + onSelect={(doc) => + setPickedInboxDocs((prev) => + prev.some((d) => d.document_id === doc.document_id) ? prev : [...prev, doc], + ) + } + /> ) diff --git a/lib/core/documents/document-service.ts b/lib/core/documents/document-service.ts index eb87fe5e..5bb3b6e9 100644 --- a/lib/core/documents/document-service.ts +++ b/lib/core/documents/document-service.ts @@ -305,6 +305,21 @@ export async function linkToJournalEntry( journalEntryId: string, journalEntryLineId?: string ): Promise { + // The document is company-filtered below, but the journal entry id arrives + // from the client and the FK only requires existence — verify it belongs to + // the same company so a crafted id can't anchor a document to another + // tenant's verifikation. (RLS hides foreign rows either way; this makes the + // rejection explicit instead of a confusing downstream state.) + const { data: entry, error: entryError } = await supabase + .from('journal_entries') + .select('id') + .eq('id', journalEntryId) + .eq('company_id', companyId) + .maybeSingle() + + if (entryError || !entry) { + throw new Error('Failed to link document: journal entry not found') + } const { data, error } = await supabase .from('document_attachments') diff --git a/messages/en.json b/messages/en.json index 796f80f7..46aedaa7 100644 --- a/messages/en.json +++ b/messages/en.json @@ -1694,6 +1694,8 @@ "description": "Create a journal entry for the transaction", "doc_label": "Receipt (optional)", "doc_attached_count": "{count} attached", + "doc_pick_existing": "Choose existing document", + "doc_picked_remove": "Remove document", "doc_link_failed_title": "Receipt could not be attached", "doc_link_failed_description": "{count} file(s) could not be linked to the journal entry. Try again from the bookkeeping page.", "bank_line_description": "Business account" diff --git a/messages/sv.json b/messages/sv.json index 655bd0ed..3ea9dd34 100644 --- a/messages/sv.json +++ b/messages/sv.json @@ -1694,6 +1694,8 @@ "description": "Skapa en verifikation för transaktionen", "doc_label": "Underlag (valfritt)", "doc_attached_count": "{count} bifogade", + "doc_pick_existing": "Välj befintligt underlag", + "doc_picked_remove": "Ta bort underlag", "doc_link_failed_title": "Underlag kunde inte bifogas", "doc_link_failed_description": "{count} fil(er) kunde inte länkas till verifikationen. Försök igen via bokföringssidan.", "bank_line_description": "Företagskonto"