fix(invoices): Ej skickade view and a prompt before downloading a draft-stamped PDF (#2401)
* fix(invoices): show finalized-but-unsent invoices as their own view and ask before downloading a draft-stamped PDF
Two user reports, one hidden state: an invoice that went through Granska
& skapa has an F-number but its DB status is still 'draft' until it is
marked as sent (and booked). The list lumped those rows under Utkast, and
"Ladda ner PDF" handed out the UTKAST-stamped render with no warning, which
users then mailed to customers.
Why it occurred: the status column carries two meanings for 'draft'
(unnumbered draft vs numbered, unsent invoice) and every surface decided on
its own how to read it. The list badge knew the difference ("Ej skickad"),
the tab predicate and the PDF download did not.
What was simplified: the tab predicate moved out of the page into
lib/invoices/invoice-list-tabs.ts as one function used by the rows, the
per-view counts, the status sections and the row badge, so the four cannot
drift. The ?status= alias parsing collapsed into the same module.
Why this and not the proposed shapes: a real 'issued' status in the DB
would touch MCP, the v1 API, reports and SIE for a distinction that
invoice_number already carries. Removing the UTKAST stamp from numbered
drafts would be wrong: an unbooked invoice is not issued. So the UI splits
the state (Ej skickade view, ?status=unsent, ?status=godkanda alias) and the
download asks first: "Bokför och ladda ner" (or "Markera som skickad och
ladda ner" for cash-method companies and offerter) runs the existing manual
mark-sent dialog and then downloads the issued document; "Ladda ner utkast"
still works. The manual mark-sent toast now offers "Ladda ner PDF" too.
Fixes #2399
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RZiqSg2v6XrtaFZyyRK88b
* fix(invoices): skeptic round on the draft-download prompt
Four refutations on 352d4bafc, all confirmed in code:
- Proformas (and any non-faktura) in an accrual company were told "Fakturan
är inte bokförd" and offered "Bokför och ladda ner"; mark-sent never books
a proforma. The label predicate is now issuesByBooking = booksOnIssue &&
isRealInvoice, shared with the existing primary button, which carried the
same "och bokför" promise on proformas.
- A partial mark-sent success (PDF archive, periodisering or delivery history
failed) still ran the chained download, and its toast evicted the warning
(one toast at a time). onSuccess now carries `partial`; the chained
download is dropped on a partial result, matching the toast action.
- Numbered följesedlar are stamped UTKAST too (pdf-template does not exclude
them) but the decision skipped the prompt. They now get the prompt; the
issue action is their own status flip, so updateStatus reports success and
the download is queued only after it.
- The download queue was a boolean bound to "whatever invoice is mounted";
the detail pager keeps the page mounted across ArrowLeft/ArrowRight, so a
step could download the neighbour or leave the queue armed. The queue now
holds the invoice id and is dropped when a different invoice is shown.
Refs #2399
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RZiqSg2v6XrtaFZyyRK88b
* fix(invoices): review pass on PR #2401
CodeRabbit, all confirmed against the code:
- The PDF preview (Visa PDF) bypassed the draft prompt; the browser viewer
has a save button, so an unwarned preview is an unwarned download. The
preview now runs the same decision; the prompt's draft button honours the
original intent ("Visa utkast" opens the viewer, "Ladda ner utkast" saves).
- updateStatus lost its isUpdating reset when both branches started
returning; moved to finally so a failed refetch after mark-sent does not
leave the page's buttons disabled.
- A cancelled credit note matched both the Kreditfakturor and Makulerade
views; the credit view now excludes cancelled rows like every other view.
Declined: the DECISIONS.md date (2026-09-08 is the local date the decision
was recorded; the bot compared against UTC) and the docstring-coverage
warning (not a repo gate).
Refs #2399
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RZiqSg2v6XrtaFZyyRK88b
---------
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5.1
parent
477b59453f
commit
4e8d649b2f
@@ -0,0 +1,38 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { draftDownloadDecision } from '../draft-download-decision'
|
||||
|
||||
describe('draftDownloadDecision', () => {
|
||||
it('downloads issued documents without asking', () => {
|
||||
expect(draftDownloadDecision({ status: 'sent', invoice_number: 'F-1' })).toBe('download')
|
||||
expect(draftDownloadDecision({ status: 'paid', invoice_number: 'F-1' })).toBe('download')
|
||||
expect(draftDownloadDecision({ status: 'overdue', invoice_number: 'F-1' })).toBe('download')
|
||||
})
|
||||
|
||||
it('offers to issue any numbered draft: the renderer stamps every draft kind', () => {
|
||||
expect(draftDownloadDecision({ status: 'draft', invoice_number: 'F-1' })).toBe('offer_issue')
|
||||
expect(
|
||||
draftDownloadDecision({ status: 'draft', invoice_number: 'O-1', document_type: 'quote' }),
|
||||
).toBe('offer_issue')
|
||||
expect(
|
||||
draftDownloadDecision({ status: 'draft', invoice_number: 'P-1', document_type: 'proforma' }),
|
||||
).toBe('offer_issue')
|
||||
expect(
|
||||
draftDownloadDecision({
|
||||
status: 'draft',
|
||||
invoice_number: 'FS-1',
|
||||
document_type: 'delivery_note',
|
||||
}),
|
||||
).toBe('offer_issue')
|
||||
})
|
||||
|
||||
it('only warns for an unnumbered draft: there is nothing to issue yet', () => {
|
||||
expect(draftDownloadDecision({ status: 'draft', invoice_number: null })).toBe('confirm_draft')
|
||||
expect(draftDownloadDecision({ status: 'draft', invoice_number: '' })).toBe('confirm_draft')
|
||||
})
|
||||
|
||||
it('leaves self-billed invoices alone: there is no own PDF to issue', () => {
|
||||
expect(
|
||||
draftDownloadDecision({ status: 'draft', invoice_number: null, is_self_billed: true }),
|
||||
).toBe('download')
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,118 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import {
|
||||
INVOICE_LIST_TABS,
|
||||
isUnsentNumberedInvoice,
|
||||
matchesInvoiceListTab,
|
||||
parseInvoiceListTab,
|
||||
type InvoiceListTab,
|
||||
} from '../invoice-list-tabs'
|
||||
|
||||
type Row = Parameters<typeof matchesInvoiceListTab>[0]
|
||||
|
||||
function row(overrides: Partial<Row> & { status: Row['status'] }): Row {
|
||||
return {
|
||||
invoice_number: null,
|
||||
credited_invoice_id: null,
|
||||
document_type: 'invoice',
|
||||
is_self_billed: false,
|
||||
...overrides,
|
||||
}
|
||||
}
|
||||
|
||||
function tabsFor(invoice: Row): InvoiceListTab[] {
|
||||
return INVOICE_LIST_TABS.filter((tab) => matchesInvoiceListTab(invoice, tab))
|
||||
}
|
||||
|
||||
describe('isUnsentNumberedInvoice', () => {
|
||||
it('is the numbered draft faktura only', () => {
|
||||
expect(isUnsentNumberedInvoice(row({ status: 'draft', invoice_number: 'F-12' }))).toBe(true)
|
||||
expect(isUnsentNumberedInvoice(row({ status: 'draft' }))).toBe(false)
|
||||
expect(isUnsentNumberedInvoice(row({ status: 'sent', invoice_number: 'F-12' }))).toBe(false)
|
||||
})
|
||||
|
||||
it('excludes credit notes, self-billed invoices and other document kinds', () => {
|
||||
expect(
|
||||
isUnsentNumberedInvoice(
|
||||
row({ status: 'draft', invoice_number: 'F-13', credited_invoice_id: 'orig' }),
|
||||
),
|
||||
).toBe(false)
|
||||
expect(
|
||||
isUnsentNumberedInvoice(row({ status: 'draft', invoice_number: 'F-14', is_self_billed: true })),
|
||||
).toBe(false)
|
||||
expect(
|
||||
isUnsentNumberedInvoice(row({ status: 'draft', invoice_number: 'O-1', document_type: 'quote' })),
|
||||
).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe('matchesInvoiceListTab', () => {
|
||||
it('splits DB status draft into unsent (numbered) and draft (unnumbered)', () => {
|
||||
expect(tabsFor(row({ status: 'draft', invoice_number: 'F-12' }))).toEqual(['all', 'unsent'])
|
||||
expect(tabsFor(row({ status: 'draft' }))).toEqual(['all', 'draft'])
|
||||
})
|
||||
|
||||
it('keeps sent and overdue invoices in the unpaid view, never in unsent', () => {
|
||||
expect(tabsFor(row({ status: 'sent', invoice_number: 'F-12' }))).toEqual(['all', 'unpaid'])
|
||||
expect(tabsFor(row({ status: 'overdue', invoice_number: 'F-12' }))).toEqual([
|
||||
'all',
|
||||
'unpaid',
|
||||
'overdue',
|
||||
])
|
||||
})
|
||||
|
||||
it('does not put draft credit notes or drafts of other kinds in unsent or draft', () => {
|
||||
expect(
|
||||
tabsFor(row({ status: 'draft', invoice_number: 'F-13', credited_invoice_id: 'orig' })),
|
||||
).toEqual(['all', 'credit'])
|
||||
expect(tabsFor(row({ status: 'draft', document_type: 'quote' }))).toEqual(['all', 'quote'])
|
||||
expect(tabsFor(row({ status: 'draft', document_type: 'proforma' }))).toEqual([
|
||||
'all',
|
||||
'proforma',
|
||||
])
|
||||
expect(tabsFor(row({ status: 'draft', document_type: 'delivery_note' }))).toEqual([
|
||||
'all',
|
||||
'delivery_note',
|
||||
])
|
||||
})
|
||||
|
||||
it('treats a missing document_type as a faktura', () => {
|
||||
expect(
|
||||
tabsFor({ status: 'draft', invoice_number: 'F-12', credited_invoice_id: null }),
|
||||
).toEqual(['all', 'unsent'])
|
||||
})
|
||||
|
||||
it('keeps cancelled rows out of every view but cancelled', () => {
|
||||
expect(tabsFor(row({ status: 'cancelled', invoice_number: 'F-12' }))).toEqual(['cancelled'])
|
||||
expect(
|
||||
tabsFor(row({ status: 'cancelled', invoice_number: 'F-13', credited_invoice_id: 'orig' })),
|
||||
).toEqual(['cancelled'])
|
||||
})
|
||||
|
||||
it('counts every non-cancelled faktura in exactly one of unsent, draft, unpaid, paid', () => {
|
||||
const rows = [
|
||||
row({ status: 'draft' }),
|
||||
row({ status: 'draft', invoice_number: 'F-1' }),
|
||||
row({ status: 'sent', invoice_number: 'F-2' }),
|
||||
row({ status: 'overdue', invoice_number: 'F-3' }),
|
||||
row({ status: 'paid', invoice_number: 'F-4' }),
|
||||
]
|
||||
const buckets: InvoiceListTab[] = ['unsent', 'draft', 'unpaid', 'paid']
|
||||
for (const invoice of rows) {
|
||||
expect(buckets.filter((tab) => matchesInvoiceListTab(invoice, tab))).toHaveLength(1)
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
describe('parseInvoiceListTab', () => {
|
||||
it('accepts tab ids and the documented aliases', () => {
|
||||
expect(parseInvoiceListTab('unsent')).toBe('unsent')
|
||||
expect(parseInvoiceListTab('drafts')).toBe('draft')
|
||||
expect(parseInvoiceListTab('godkanda')).toBe('unsent')
|
||||
expect(parseInvoiceListTab('approved')).toBe('unsent')
|
||||
})
|
||||
|
||||
it('rejects unknown values', () => {
|
||||
expect(parseInvoiceListTab('bogus')).toBeNull()
|
||||
expect(parseInvoiceListTab(null)).toBeNull()
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,33 @@
|
||||
import type { Invoice } from '@/types'
|
||||
|
||||
/**
|
||||
* What "Ladda ner PDF" should do for a document that is not yet issued.
|
||||
*
|
||||
* The renderer stamps every status='draft' document "UTKAST: inte en giltig
|
||||
* faktura". That stamp is correct: an unbooked invoice is not issued. The
|
||||
* problem (#2399) is that nothing said so before the file was on disk, and
|
||||
* the stamped PDF was mailed to customers by mistake. So the download asks
|
||||
* first, and offers the path that produces the real document.
|
||||
*
|
||||
* - `download`: issued (or a document kind with no issue step); save as is.
|
||||
* - `offer_issue`: numbered, not sent: offer "Markera som skickad (och
|
||||
* bokför) och ladda ner", the same action as the page's primary button.
|
||||
* - `confirm_draft`: no number yet: nothing to issue, only a warning that the
|
||||
* file is a stamped draft. Granska & skapa is the way to a number.
|
||||
*/
|
||||
export type DraftDownloadDecision = 'download' | 'offer_issue' | 'confirm_draft'
|
||||
|
||||
type DecisionInvoice = Pick<Invoice, 'status' | 'invoice_number'> & {
|
||||
document_type?: string | null
|
||||
is_self_billed?: boolean | null
|
||||
}
|
||||
|
||||
export function draftDownloadDecision(invoice: DecisionInvoice): DraftDownloadDecision {
|
||||
if (invoice.status !== 'draft') return 'download'
|
||||
// Self-billed: the counterparty's document, no own PDF to issue.
|
||||
if (invoice.is_self_billed) return 'download'
|
||||
// Every other kind (faktura, kreditfaktura, offert, proforma, följesedel)
|
||||
// is stamped while status is draft; the page picks the issue action per
|
||||
// kind (send dialog, or the plain status flip for följesedlar).
|
||||
return invoice.invoice_number ? 'offer_issue' : 'confirm_draft'
|
||||
}
|
||||
@@ -0,0 +1,95 @@
|
||||
import type { Invoice } from '@/types'
|
||||
|
||||
/**
|
||||
* The invoice list's status views. One predicate for the visible rows, the
|
||||
* per-view counts and the status sections, so the three can never drift.
|
||||
*
|
||||
* `unsent` and `draft` split the DB status 'draft' in two: an invoice that
|
||||
* went through Granska & skapa carries an F-number and is waiting to be sent
|
||||
* or booked (#2399), while an unnumbered draft is still being written.
|
||||
*/
|
||||
export const INVOICE_LIST_MAIN_TABS = ['all', 'unpaid', 'overdue', 'unsent', 'draft'] as const
|
||||
export const INVOICE_LIST_MORE_TABS = [
|
||||
'paid',
|
||||
'proforma',
|
||||
'quote',
|
||||
'delivery_note',
|
||||
'credit',
|
||||
'cancelled',
|
||||
] as const
|
||||
export const INVOICE_LIST_TABS = [...INVOICE_LIST_MAIN_TABS, ...INVOICE_LIST_MORE_TABS] as const
|
||||
export type InvoiceListTab = (typeof INVOICE_LIST_TABS)[number]
|
||||
|
||||
/** ?status= values that are not tab ids: older bookmarks and the words users
|
||||
* reach for ("godkända" is what Visma calls a finalized, unsent invoice). */
|
||||
export const INVOICE_LIST_TAB_ALIASES: Record<string, InvoiceListTab> = {
|
||||
drafts: 'draft',
|
||||
godkanda: 'unsent',
|
||||
approved: 'unsent',
|
||||
}
|
||||
|
||||
export function parseInvoiceListTab(param: string | null): InvoiceListTab | null {
|
||||
if (!param) return null
|
||||
const candidate = INVOICE_LIST_TAB_ALIASES[param] ?? param
|
||||
return (INVOICE_LIST_TABS as readonly string[]).includes(candidate)
|
||||
? (candidate as InvoiceListTab)
|
||||
: null
|
||||
}
|
||||
|
||||
type ListInvoice = Pick<Invoice, 'status' | 'invoice_number' | 'credited_invoice_id'> & {
|
||||
document_type?: string | null
|
||||
is_self_billed?: boolean | null
|
||||
}
|
||||
|
||||
function documentTypeOf(invoice: ListInvoice): string {
|
||||
return invoice.document_type || 'invoice'
|
||||
}
|
||||
|
||||
/**
|
||||
* A faktura with an F-number whose DB status is still 'draft': finalized but
|
||||
* neither sent nor booked. Self-billed invoices arrive already issued and
|
||||
* credit notes have their own view, so both are excluded.
|
||||
*/
|
||||
export function isUnsentNumberedInvoice(invoice: ListInvoice): boolean {
|
||||
return (
|
||||
invoice.status === 'draft' &&
|
||||
!!invoice.invoice_number &&
|
||||
documentTypeOf(invoice) === 'invoice' &&
|
||||
!invoice.credited_invoice_id &&
|
||||
!invoice.is_self_billed
|
||||
)
|
||||
}
|
||||
|
||||
export function matchesInvoiceListTab(invoice: ListInvoice, tab: InvoiceListTab): boolean {
|
||||
const isCreditNote = !!invoice.credited_invoice_id
|
||||
const docType = documentTypeOf(invoice)
|
||||
switch (tab) {
|
||||
case 'all':
|
||||
return invoice.status !== 'cancelled'
|
||||
case 'unpaid':
|
||||
return ['sent', 'overdue'].includes(invoice.status) && !isCreditNote && docType === 'invoice'
|
||||
case 'overdue':
|
||||
return invoice.status === 'overdue' && !isCreditNote && docType === 'invoice'
|
||||
case 'unsent':
|
||||
return isUnsentNumberedInvoice(invoice)
|
||||
case 'draft':
|
||||
return (
|
||||
invoice.status === 'draft' &&
|
||||
docType === 'invoice' &&
|
||||
!isCreditNote &&
|
||||
!isUnsentNumberedInvoice(invoice)
|
||||
)
|
||||
case 'paid':
|
||||
return invoice.status === 'paid'
|
||||
case 'credit':
|
||||
return isCreditNote && invoice.status !== 'cancelled'
|
||||
case 'proforma':
|
||||
return docType === 'proforma' && invoice.status !== 'cancelled'
|
||||
case 'quote':
|
||||
return docType === 'quote' && invoice.status !== 'cancelled'
|
||||
case 'delivery_note':
|
||||
return docType === 'delivery_note' && invoice.status !== 'cancelled'
|
||||
case 'cancelled':
|
||||
return invoice.status === 'cancelled'
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user