fix(transactions): block invalid invoice match targets (#1294)
Classify customer and supplier invoice targets as matchable, settled, or otherwise not open. Block invalid targets with localized guidance while retaining the valid partial-payment flow and add focused regression coverage. Fixes #1260
This commit is contained in:
@@ -10,8 +10,8 @@ import { formatCurrency, formatDate, cn } from '@/lib/utils'
|
||||
import { getErrorMessage } from '@/lib/errors/get-error-message'
|
||||
import { isInvoiceBookingRateMissing, previewedFxGainSek } from './invoice-match-fx'
|
||||
import {
|
||||
isMatchableInvoice,
|
||||
isMatchableSupplierInvoice,
|
||||
getInvoiceMatchTargetState,
|
||||
getSupplierInvoiceMatchTargetState,
|
||||
} from '@/lib/invoices/matchable-statuses'
|
||||
import { CheckCircle2, AlertTriangle, Trash2, Plus, Pencil } from 'lucide-react'
|
||||
import type { TransactionWithInvoice } from './transaction-types'
|
||||
@@ -153,14 +153,16 @@ export default function InvoiceMatchDialog({
|
||||
// still be stale (fetched before the other match, or settled in another
|
||||
// tab), so re-check here rather than trust the pointer.
|
||||
//
|
||||
// This is not an advisory guard: both match routes reject a settled target
|
||||
// outright (MATCH_INVOICE_ALREADY_PAID / MATCH_SI_ALREADY_PAID), so there is
|
||||
// no "match anyway" that could succeed. Say so and block, instead of
|
||||
// computing a diff against a 0 kr remaining balance and calling the result
|
||||
// a partial payment.
|
||||
const targetSettled =
|
||||
(isSupplierInvoice && !isMatchableSupplierInvoice(transaction!.potential_supplier_invoice)) ||
|
||||
(isCustomerInvoice && !isMatchableInvoice(transaction!.potential_invoice))
|
||||
// This is not an advisory guard: the match routes reject any target outside
|
||||
// their open-status CAS lists, so there is no "match anyway" that could
|
||||
// succeed. Distinguish a paid or zero-balance target from a different
|
||||
// non-open status so the blocking copy explains the actual problem.
|
||||
const targetMatchState = isSupplierInvoice
|
||||
? getSupplierInvoiceMatchTargetState(transaction!.potential_supplier_invoice)
|
||||
: isCustomerInvoice
|
||||
? getInvoiceMatchTargetState(transaction!.potential_invoice)
|
||||
: null
|
||||
const targetBlocked = targetMatchState !== null && targetMatchState !== 'matchable'
|
||||
|
||||
const [candidate, setCandidate] = useState<DuplicateCandidate | null>(null)
|
||||
const [isCheckingDuplicate, setIsCheckingDuplicate] = useState(false)
|
||||
@@ -200,7 +202,7 @@ export default function InvoiceMatchDialog({
|
||||
}, [open])
|
||||
|
||||
useEffect(() => {
|
||||
if (!open || !transactionId) {
|
||||
if (!open || !transactionId || targetBlocked) {
|
||||
setPreview(null)
|
||||
setPreviewFailure(null)
|
||||
setIsEditing(false)
|
||||
@@ -260,11 +262,27 @@ export default function InvoiceMatchDialog({
|
||||
return () => {
|
||||
cancelled = true
|
||||
}
|
||||
}, [open, transactionId, isCustomerInvoice, isSupplierInvoice, invoiceId, supplierInvoiceId, uiLocale])
|
||||
}, [
|
||||
open,
|
||||
transactionId,
|
||||
isCustomerInvoice,
|
||||
isSupplierInvoice,
|
||||
invoiceId,
|
||||
supplierInvoiceId,
|
||||
targetBlocked,
|
||||
uiLocale,
|
||||
])
|
||||
|
||||
useEffect(() => {
|
||||
if (!open || !transactionId || !isCustomerInvoice || !onLinkToExisting) {
|
||||
if (
|
||||
!open ||
|
||||
!transactionId ||
|
||||
!isCustomerInvoice ||
|
||||
!onLinkToExisting ||
|
||||
targetBlocked
|
||||
) {
|
||||
setCandidate(null)
|
||||
setIsCheckingDuplicate(false)
|
||||
return
|
||||
}
|
||||
let cancelled = false
|
||||
@@ -285,7 +303,7 @@ export default function InvoiceMatchDialog({
|
||||
return () => {
|
||||
cancelled = true
|
||||
}
|
||||
}, [open, transactionId, isCustomerInvoice, onLinkToExisting])
|
||||
}, [open, transactionId, isCustomerInvoice, onLinkToExisting, targetBlocked])
|
||||
|
||||
// Live balance + validity. The dialog disables Confirm while edit mode is
|
||||
// active and the entry is invalid; an out-of-balance entry can't be sent.
|
||||
@@ -368,9 +386,11 @@ export default function InvoiceMatchDialog({
|
||||
}
|
||||
|
||||
const matchTitle = isSupplierInvoice ? t('title_supplier') : t('title_customer')
|
||||
const matchDescription = isSupplierInvoice
|
||||
? t('description_supplier')
|
||||
: t('description_customer')
|
||||
const matchDescription = targetBlocked
|
||||
? t('description_blocked')
|
||||
: isSupplierInvoice
|
||||
? t('description_supplier')
|
||||
: t('description_customer')
|
||||
|
||||
return (
|
||||
<Dialog open={open} onOpenChange={onOpenChange}>
|
||||
@@ -383,7 +403,7 @@ export default function InvoiceMatchDialog({
|
||||
{transaction && (isCustomerInvoice || isSupplierInvoice) && (
|
||||
<div className="space-y-4">
|
||||
{/* Duplicate-payment warning: customer-side only, only when a candidate exists */}
|
||||
{candidate && isCustomerInvoice && (
|
||||
{!targetBlocked && candidate && isCustomerInvoice && (
|
||||
<div className="rounded-lg border border-warning/40 bg-warning/10 p-4 space-y-3">
|
||||
<div className="flex items-start gap-2">
|
||||
<AlertTriangle className="h-4 w-4 flex-shrink-0 mt-0.5 text-warning-foreground" />
|
||||
@@ -528,17 +548,24 @@ export default function InvoiceMatchDialog({
|
||||
The customer branch previously fell back to .total; both
|
||||
branches now mirror the supplier branch's correct logic. */}
|
||||
{(() => {
|
||||
// Settled target: the amount comparison below would be
|
||||
// meaningless (it measures against a 0 kr remaining balance and
|
||||
// reports the whole transaction as a "differens"), and no
|
||||
// outcome it describes is reachable. Replace it outright.
|
||||
if (targetSettled) {
|
||||
// A blocked target makes the amount comparison below
|
||||
// meaningless, and no outcome it describes is reachable.
|
||||
if (targetBlocked) {
|
||||
const isSettled = targetMatchState === 'settled'
|
||||
return (
|
||||
<div className="flex items-start gap-2 p-3 rounded-lg bg-warning/10 text-warning-foreground">
|
||||
<AlertTriangle className="h-4 w-4 flex-shrink-0 mt-0.5" />
|
||||
<div className="text-sm">
|
||||
<p className="font-medium">{t('target_settled_title')}</p>
|
||||
<p>{t('target_settled_description')}</p>
|
||||
<p className="font-medium">
|
||||
{t(isSettled ? 'target_settled_title' : 'target_not_open_title')}
|
||||
</p>
|
||||
<p>
|
||||
{t(
|
||||
isSettled
|
||||
? 'target_settled_description'
|
||||
: 'target_not_open_description',
|
||||
)}
|
||||
</p>
|
||||
</div>
|
||||
</div>
|
||||
)
|
||||
@@ -624,7 +651,7 @@ export default function InvoiceMatchDialog({
|
||||
front instead of showing a confident zero. Rendered on its own
|
||||
rather than inside the Valutaomräkning card below, because in
|
||||
this state the preview 400s and that card never renders. */}
|
||||
{invoiceRateMissing && (
|
||||
{!targetBlocked && invoiceRateMissing && (
|
||||
<div className="rounded-lg border border-warning/40 bg-warning/5 p-4">
|
||||
<div className="flex items-start gap-2">
|
||||
<AlertTriangle className="h-4 w-4 mt-0.5 text-warning-foreground flex-shrink-0" />
|
||||
@@ -651,7 +678,7 @@ export default function InvoiceMatchDialog({
|
||||
When the payment-date rate lookup failed, swaps in a
|
||||
manual-rate input so the user can type the rate from their
|
||||
bank statement and retry. */}
|
||||
{preview?.fx_conversion?.required && (() => {
|
||||
{!targetBlocked && preview?.fx_conversion?.required && (() => {
|
||||
const fx = preview.fx_conversion
|
||||
if (!fx?.required) return null
|
||||
// fx_conversion is only produced by the customer-invoice preview
|
||||
@@ -755,7 +782,7 @@ export default function InvoiceMatchDialog({
|
||||
blocked the preview: the ochre panel above already owns that
|
||||
story, and an empty "Bokföring" card with a second phrasing of
|
||||
the same refusal reads as two separate problems. */}
|
||||
{(preview || (previewFailure && !invoiceRateMissing)) && (
|
||||
{!targetBlocked && (preview || (previewFailure && !invoiceRateMissing)) && (
|
||||
<div className="rounded-lg border p-4 space-y-3">
|
||||
<div className="flex items-center justify-between">
|
||||
<p className="text-sm font-medium">{t('booking_title')}</p>
|
||||
@@ -928,15 +955,16 @@ export default function InvoiceMatchDialog({
|
||||
</div>
|
||||
)}
|
||||
|
||||
{/* What will happen */}
|
||||
<div className="rounded-lg bg-muted/50 p-4 space-y-2">
|
||||
<p className="text-sm font-medium">{t('on_confirm_title')}</p>
|
||||
<ul className="text-sm text-muted-foreground space-y-1">
|
||||
<li>• {isSupplierInvoice ? t('on_confirm_link_supplier') : t('on_confirm_link_customer')}</li>
|
||||
<li>• {isSupplierInvoice ? t('on_confirm_mark_paid_supplier') : t('on_confirm_mark_paid_customer')}</li>
|
||||
<li>• {t('on_confirm_voucher')}</li>
|
||||
</ul>
|
||||
</div>
|
||||
{!targetBlocked && (
|
||||
<div className="rounded-lg bg-muted/50 p-4 space-y-2">
|
||||
<p className="text-sm font-medium">{t('on_confirm_title')}</p>
|
||||
<ul className="text-sm text-muted-foreground space-y-1">
|
||||
<li>• {isSupplierInvoice ? t('on_confirm_link_supplier') : t('on_confirm_link_customer')}</li>
|
||||
<li>• {isSupplierInvoice ? t('on_confirm_mark_paid_supplier') : t('on_confirm_mark_paid_customer')}</li>
|
||||
<li>• {t('on_confirm_voucher')}</li>
|
||||
</ul>
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
)}
|
||||
|
||||
@@ -949,9 +977,9 @@ export default function InvoiceMatchDialog({
|
||||
disabled={
|
||||
isConfirming ||
|
||||
isCheckingDuplicate ||
|
||||
// Settled target: the route rejects this unconditionally, so the
|
||||
// Blocked target: the route rejects this unconditionally, so the
|
||||
// button has no reachable success path.
|
||||
targetSettled ||
|
||||
targetBlocked ||
|
||||
(isEditing && !editValidation.isValid) ||
|
||||
// Block confirm when cross-currency lookup failed and the user
|
||||
// hasn't typed a manual rate yet. Same-currency and auto-rate
|
||||
|
||||
Reference in New Issue
Block a user