fix(ux): book documents directly from inbox + attach existing underlag when booking transactions (#670)
* 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Jakob Wennberg
co-authored by
Claude Opus 4.8
Claude Opus 4.8
parent
f7cd1b86e7
commit
cac692e293
@@ -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')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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 <FileText className="h-4 w-4 text-muted-foreground shrink-0" />
|
||||
}
|
||||
|
||||
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' })
|
||||
|
||||
@@ -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). */}
|
||||
<Button
|
||||
variant="default"
|
||||
size="sm"
|
||||
@@ -1645,6 +1644,14 @@ function FieldsRail({
|
||||
>
|
||||
Matcha mot transaktion
|
||||
</Button>
|
||||
<Button
|
||||
variant="outline"
|
||||
size="sm"
|
||||
className="w-full"
|
||||
onClick={onBookDirect}
|
||||
>
|
||||
Bokför manuellt
|
||||
</Button>
|
||||
<Link href={`/supplier-invoices/new?inbox_item_id=${item.id}`} className="block">
|
||||
<Button variant="outline" size="sm" className="w-full">
|
||||
Skapa leverantörsfaktura
|
||||
|
||||
@@ -7,10 +7,12 @@ import { Button } from '@/components/ui/button'
|
||||
import { Label } from '@/components/ui/label'
|
||||
import { useToast } from '@/components/ui/use-toast'
|
||||
import { formatCurrency, formatDate } from '@/lib/utils'
|
||||
import { ArrowUpRight, ArrowDownRight, ChevronDown, ChevronUp, Paperclip } from 'lucide-react'
|
||||
import { ArrowUpRight, ArrowDownRight, ChevronDown, ChevronUp, FileText, Inbox, Paperclip, X } from 'lucide-react'
|
||||
import JournalEntryForm from '@/components/bookkeeping/JournalEntryForm'
|
||||
import DocumentUploadZone from '@/components/bookkeeping/DocumentUploadZone'
|
||||
import type { UploadedFile } from '@/components/bookkeeping/DocumentUploadZone'
|
||||
import InboxDocumentPicker from '@/components/bookkeeping/InboxDocumentPicker'
|
||||
import type { AvailableInboxDoc } from '@/components/bookkeeping/InboxDocumentPicker'
|
||||
import type { FormLine } from '@/components/bookkeeping/JournalEntryForm'
|
||||
import { resolveSekAmount, buildCurrencyMetadata } from '@/lib/bookkeeping/currency-utils'
|
||||
import { applyTemplate } from '@/lib/bookkeeping/template-library'
|
||||
@@ -101,47 +103,72 @@ export default function TransactionBookingDialog({
|
||||
const t = useTranslations('tx_booking_dialog')
|
||||
const { toast } = useToast()
|
||||
const [uploadedFiles, setUploadedFiles] = useState<UploadedFile[]>([])
|
||||
const [pickedInboxDocs, setPickedInboxDocs] = useState<AvailableInboxDoc[]>([])
|
||||
const [showUploadZone, setShowUploadZone] = useState(false)
|
||||
const [inboxPickerOpen, setInboxPickerOpen] = useState(false)
|
||||
|
||||
if (!transaction) return null
|
||||
|
||||
const isIncome = transaction.amount > 0
|
||||
|
||||
const handleBooked = async (transactionId: string, journalEntryId: string) => {
|
||||
// Link any uploaded documents to the new journal entry
|
||||
// Link any attached documents to the new journal entry: freshly uploaded
|
||||
// files, and existing inbox documents picked via InboxDocumentPicker. For
|
||||
// picked docs, inbox_item_id stamps the inbox item as consumed so it drops
|
||||
// out of the active inbox — see app/api/documents/[id]/link/route.ts.
|
||||
const filesToLink = uploadedFiles.filter((f) => f.status === 'uploaded' && f.id)
|
||||
if (filesToLink.length > 0) {
|
||||
let linkFailCount = 0
|
||||
for (const file of filesToLink) {
|
||||
try {
|
||||
await fetch(`/api/documents/${file.id}/link`, {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ journal_entry_id: journalEntryId }),
|
||||
})
|
||||
} catch {
|
||||
linkFailCount++
|
||||
}
|
||||
}
|
||||
if (linkFailCount > 0) {
|
||||
toast({
|
||||
title: t('doc_link_failed_title'),
|
||||
description: t('doc_link_failed_description', { count: linkFailCount }),
|
||||
variant: 'destructive',
|
||||
let linkFailCount = 0
|
||||
for (const file of filesToLink) {
|
||||
try {
|
||||
const res = await fetch(`/api/documents/${file.id}/link`, {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ journal_entry_id: journalEntryId }),
|
||||
})
|
||||
if (!res.ok) linkFailCount++
|
||||
} catch {
|
||||
linkFailCount++
|
||||
}
|
||||
}
|
||||
for (const doc of pickedInboxDocs) {
|
||||
try {
|
||||
const res = await fetch(`/api/documents/${doc.document_id}/link`, {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({
|
||||
journal_entry_id: journalEntryId,
|
||||
inbox_item_id: doc.inbox_item_id,
|
||||
}),
|
||||
})
|
||||
if (!res.ok) linkFailCount++
|
||||
} catch {
|
||||
linkFailCount++
|
||||
}
|
||||
}
|
||||
if (linkFailCount > 0) {
|
||||
toast({
|
||||
title: t('doc_link_failed_title'),
|
||||
description: t('doc_link_failed_description', { count: linkFailCount }),
|
||||
variant: 'destructive',
|
||||
})
|
||||
}
|
||||
|
||||
setUploadedFiles([])
|
||||
setPickedInboxDocs([])
|
||||
setShowUploadZone(false)
|
||||
onBooked(transactionId, journalEntryId)
|
||||
}
|
||||
|
||||
const attachedCount =
|
||||
uploadedFiles.filter((f) => f.status === 'uploaded').length + pickedInboxDocs.length
|
||||
|
||||
return (
|
||||
<Dialog open={open} onOpenChange={(o) => {
|
||||
if (!o) {
|
||||
setUploadedFiles([])
|
||||
setPickedInboxDocs([])
|
||||
setShowUploadZone(false)
|
||||
setInboxPickerOpen(false)
|
||||
}
|
||||
onOpenChange(o)
|
||||
}}>
|
||||
@@ -188,9 +215,9 @@ export default function TransactionBookingDialog({
|
||||
<div className="flex items-center gap-2">
|
||||
<Paperclip className="h-4 w-4 text-muted-foreground" />
|
||||
<span className="font-medium">{t('doc_label')}</span>
|
||||
{uploadedFiles.filter((f) => f.status === 'uploaded').length > 0 && (
|
||||
{attachedCount > 0 && (
|
||||
<span className="text-xs text-muted-foreground">
|
||||
{t('doc_attached_count', { count: uploadedFiles.filter((f) => f.status === 'uploaded').length })}
|
||||
{t('doc_attached_count', { count: attachedCount })}
|
||||
</span>
|
||||
)}
|
||||
</div>
|
||||
@@ -201,12 +228,55 @@ export default function TransactionBookingDialog({
|
||||
)}
|
||||
</button>
|
||||
{showUploadZone && (
|
||||
<div className="px-3 pb-3">
|
||||
<div className="px-3 pb-3 space-y-2">
|
||||
<DocumentUploadZone
|
||||
files={uploadedFiles}
|
||||
onFilesChange={setUploadedFiles}
|
||||
compact
|
||||
/>
|
||||
{pickedInboxDocs.length > 0 && (
|
||||
<div className="space-y-1">
|
||||
{pickedInboxDocs.map((doc) => (
|
||||
<div
|
||||
key={doc.document_id}
|
||||
className="flex items-center gap-2 text-sm py-1.5 px-2 rounded bg-muted/50"
|
||||
>
|
||||
<FileText className="h-4 w-4 text-muted-foreground shrink-0" />
|
||||
<span className="truncate flex-1">
|
||||
{doc.supplier_name ?? doc.file_name}
|
||||
</span>
|
||||
{doc.amount != null && (
|
||||
<span className="text-xs text-muted-foreground tabular-nums shrink-0">
|
||||
{formatCurrency(doc.amount, doc.currency ?? 'SEK')}
|
||||
</span>
|
||||
)}
|
||||
<Button
|
||||
variant="ghost"
|
||||
size="sm"
|
||||
className="h-6 w-6 p-0 shrink-0"
|
||||
aria-label={t('doc_picked_remove')}
|
||||
onClick={() =>
|
||||
setPickedInboxDocs((prev) =>
|
||||
prev.filter((d) => d.document_id !== doc.document_id),
|
||||
)
|
||||
}
|
||||
>
|
||||
<X className="h-3 w-3" />
|
||||
</Button>
|
||||
</div>
|
||||
))}
|
||||
</div>
|
||||
)}
|
||||
<Button
|
||||
type="button"
|
||||
variant="outline"
|
||||
size="sm"
|
||||
className="w-full"
|
||||
onClick={() => setInboxPickerOpen(true)}
|
||||
>
|
||||
<Inbox className="h-4 w-4 mr-2" />
|
||||
{t('doc_pick_existing')}
|
||||
</Button>
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
@@ -226,6 +296,16 @@ export default function TransactionBookingDialog({
|
||||
sourceId={transaction.id}
|
||||
onEntryCreated={(entryId) => handleBooked(transaction.id, entryId)}
|
||||
/>
|
||||
|
||||
<InboxDocumentPicker
|
||||
open={inboxPickerOpen}
|
||||
onClose={() => setInboxPickerOpen(false)}
|
||||
onSelect={(doc) =>
|
||||
setPickedInboxDocs((prev) =>
|
||||
prev.some((d) => d.document_id === doc.document_id) ? prev : [...prev, doc],
|
||||
)
|
||||
}
|
||||
/>
|
||||
</DialogContent>
|
||||
</Dialog>
|
||||
)
|
||||
|
||||
@@ -305,6 +305,21 @@ export async function linkToJournalEntry(
|
||||
journalEntryId: string,
|
||||
journalEntryLineId?: string
|
||||
): Promise<DocumentAttachment> {
|
||||
// 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')
|
||||
|
||||
@@ -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"
|
||||
|
||||
@@ -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"
|
||||
|
||||
Reference in New Issue
Block a user