fix(match-batch): cross-currency allocations + widened tolerance (#607)
* fix(match-batch): cross-currency allocations + widened tolerance Reported by jakob testing PR #603's MatchAllocationDialog with a SEK bank tx + a mix of SEK and USD invoices: 1. Tally rendered "1 USD + 1 SEK = 2 kr" — summing different currencies as if they were the same. 2. The 0.005 SEK tolerance blocked confirm on any FX rounding delta. ## What changed **UI (MatchAllocationDialog.tsx)** - Per-row amount input is explicitly in TRANSACTION currency (SEK for a Swedish bank import). Cross-currency rows show an "≈ X.XX (invoice currency)" hint under the input so the user can verify the FX result. - Default amount for a cross-currency allocation is `invoice.remaining × invoice.exchange_rate` (booked SEK), so the user doesn't have to mental-math the FX. - Overshoot tolerance widened from 0.005 SEK to `max(1 SEK, 0.5% × tx)` so bank-side FX rounding doesn't block confirm. A 2 400 kr tx now accepts ~12 kr of tolerance, a 100 kkr transfer accepts 500 kr. **RPC (match_batch_allocate cross_currency migration)** - BATCH_CURRENCY_MISMATCH dropped per-allocation. Mixed currencies now accepted with the convention that the cross-currency row pays the FULL invoice remaining (matches the single-tx match-supplier-invoice behavior). Partial cross-currency is out of scope for v1. - AR/AP line is booked at `invoice.remaining × invoice.exchange_rate` (the SEK that was originally on 1510/2440). FX residual is posted to 7960 (Valutakursförluster) or 3960 (Valutakursvinster) per BAS. - Sign conventions per direction documented inline: Customer: bank > booked → Cr 3960 (gain); bank < booked → Dr 7960 Supplier: bank < booked → Cr 3960 (gain); bank > booked → Dr 7960 - New BATCH_FX_RATE_MISSING when the cross-currency invoice has no exchange_rate on file (would otherwise silently book at 0). - New BATCH_FX_DEVIATION_TOO_LARGE when the user-entered amount deviates more than 10% from booked SEK — catches typos like "140" (USD invoice currency) when they meant "1390" (SEK equivalent) without rejecting genuine rate-day FX movement. RPC patched on remote via Supabase MCP. Same-currency path is byte-identical to the previous behavior. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(match-batch): PR review — strict sum, bank line = tx_abs, FX validation Round-1 review fixes on the cross-currency batch allocation flow: UI (MatchAllocationDialog): - Tighten tolerance to 0.005 SEK so the "balanced ✓" indicator matches what the server will accept. The previous widened tolerance (max 1 SEK or 0.5% × tx) created a reconciliation gap where the JE's bank line could legitimately disagree with the actual bank receipt. - Require balanced before confirm — undershoot is now a blocking state with an explicit warning, not a silent "leave unallocated". - Cross-currency default no longer caps at remainingTxBudget. Capping a USD invoice's default to the leftover SEK budget could silently trigger BATCH_FX_DEVIATION_TOO_LARGE on submit. The user re-balances the other rows to fit. - Add explicit FX-rate validation (bound check 0 < rate < 100000). - When a cross-currency invoice has no usable exchange_rate on file, leave the amount blank and surface a warning instead of guessing. RPC (match_batch_allocate): - New code BATCH_AMOUNT_BELOW_TX. Strict sum check on both sides means the server can't be coaxed by a direct API caller into the same broken state the UI now blocks. - Bank line credit/debit = v_tx_abs (the actual bank movement) instead of sum-of-allocations. Same value within rounding under the strict sum check, but it makes intent legible and lets per-row FX diff lines absorb rounding. - Defense-in-depth company_id filter on all re-queries / UPDATEs in the line-build + payment-row passes. - Drop the v_booked_sek-aliasing-for-invoice.total foot-gun. Use a dedicated v_inv_total var. - Truncate invoice_number to 32 chars in line_description. Tests: - pg-real: cross-currency happy path (USD invoice paid by SEK tx with FX loss to 7960, bank line = tx_abs). - pg-real: BATCH_AMOUNT_BELOW_TX rejection on undershoot. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(match-batch): PR review round 2 - caller user_id verification + FX bound Compliance-swarm + swedish-compliance findings on round 1: - CC6.3 (HIGH): p_user_id was caller-supplied and written into journal_entries.user_id / payment-row user_id without verifying it equals auth.uid(). Membership covered the company; nothing covered the user attribution. Two-layer fix: explicit guard rejects when p_user_id <> auth.uid(), and all writes now resolve v_caller = auth.uid() directly so the guard cant be silently bypassed. - A.8.28 (MED): server-side FX upper-bound (0 < rate < 100000) matches the UI. Previously RPC only checked > 0, allowing the UI guard to diverge. - V1.2.5 (LOW): truncate v_tx.date when concatenated into line_description (defense alongside round 1s invoice_number trunc). - Symmetry: populate supplier_invoice_payments.exchange_rate (column existed, INSERT omitted it). Customer side already populated. Matches swedish-compliances traceability note on AP rorelseskulder. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(match-batch): PR review round 3 - drop p_user_id, CHECK constraints, payment-day rate Genuine round-2 review findings (compliance-swarm + swedish-compliance): - V4.5: p_user_id dropped from RPC signature entirely. Round-2 added a guard; this removes the attack surface at the API boundary. Caller is resolved via auth.uid() inside the function. Route updated. - V2.2: CHECK constraint on invoices.exchange_rate and supplier_invoices.exchange_rate (0 < rate < 100000). Three layers now enforce the bound: schema, RPC, UI. - swedish-compliance traceability gap: payment_exchange_rate column on both invoice_payments and supplier_invoice_payments. Populated as v_alloc_amount / v_inv_remaining for cross-currency rows so FX diffs are reconstructible from the payment record alone (BFL 7 kap behandlingshistorik). NULL for same-currency. The existing exchange_rate column continues to store the invoicing rate. - CC6.1: extract isValidExchangeRate() to lib/utils.ts. UI's three inline bound checks now share one validator. - Dead code: drop unused leftover_note i18n key (sv + en). Tests: - pg-real signature updated (4-arg -> 3-arg) across all 9 call sites. - Added payment_exchange_rate assertion to cross-currency happy path (invoicing rate 10.0 stays, payment-day rate stored as 10.5). Migration applied to remote. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(test): missed 4th arg in BATCH_UNAUTHORIZED pg-real test Round-3 dropped p_user_id from match_batch_allocate. The replace_all caught the userId/companyId pattern but missed the BATCH_UNAUTHORIZED test which uses outsiderId instead of userId. CI failed with "bind message supplies 4 parameters, but prepared statement requires 3". Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Jakob Wennberg
co-authored by
Claude Opus 4.7
Claude Opus 4.7
parent
4da87e5e4c
commit
fc7a46c3f2
@@ -18,7 +18,7 @@ import { Skeleton } from '@/components/ui/skeleton'
|
||||
import { Badge } from '@/components/ui/badge'
|
||||
import { useToast } from '@/components/ui/use-toast'
|
||||
import { getErrorMessage } from '@/lib/errors/get-error-message'
|
||||
import { formatCurrency, formatDate, cn } from '@/lib/utils'
|
||||
import { formatCurrency, formatDate, cn, isValidExchangeRate } from '@/lib/utils'
|
||||
import { Loader2, Search, X, Plus, Check, AlertTriangle } from 'lucide-react'
|
||||
import type { Invoice, Customer, SupplierInvoice, Supplier } from '@/types'
|
||||
import type { TransactionWithInvoice } from './transaction-types'
|
||||
@@ -35,6 +35,11 @@ interface MatchAllocationDialogProps {
|
||||
* supplier invoices to the same shape so the row renderer + tally math stay
|
||||
* a single code path. The `kind` discriminator drives the underlying API
|
||||
* payload at submit time.
|
||||
*
|
||||
* `remaining` is in the invoice's own `currency` (USD, EUR, etc.).
|
||||
* `exchangeRate` is the invoice's SEK-per-foreign-unit at invoicing time
|
||||
* — used to compute the default SEK amount for cross-currency rows so the
|
||||
* user doesn't have to mental-math the FX (PR #607).
|
||||
*/
|
||||
interface AllocationCandidate {
|
||||
kind: 'customer_invoice' | 'supplier_invoice'
|
||||
@@ -44,6 +49,7 @@ interface AllocationCandidate {
|
||||
remaining: number
|
||||
total: number
|
||||
currency: string
|
||||
exchangeRate: number | null
|
||||
dueDate: string
|
||||
}
|
||||
|
||||
@@ -120,6 +126,7 @@ export default function MatchAllocationDialog({
|
||||
remaining: Number(r.remaining_amount ?? r.total ?? 0),
|
||||
total: Number(r.total ?? 0),
|
||||
currency: r.currency,
|
||||
exchangeRate: r.exchange_rate != null ? Number(r.exchange_rate) : null,
|
||||
dueDate: r.due_date,
|
||||
})),
|
||||
)
|
||||
@@ -142,6 +149,7 @@ export default function MatchAllocationDialog({
|
||||
remaining: Number(r.remaining_amount ?? r.total ?? 0),
|
||||
total: Number(r.total ?? 0),
|
||||
currency: r.currency,
|
||||
exchangeRate: r.exchange_rate != null ? Number(r.exchange_rate) : null,
|
||||
dueDate: r.due_date,
|
||||
})),
|
||||
)
|
||||
@@ -165,14 +173,27 @@ export default function MatchAllocationDialog({
|
||||
}, [open])
|
||||
|
||||
const txAmountAbs = transaction ? Math.abs(transaction.amount) : 0
|
||||
const txCurrency = transaction?.currency ?? 'SEK'
|
||||
|
||||
// Each draft's `amount` is the allocation in TRANSACTION currency (SEK
|
||||
// for a Swedish bank import). For cross-currency invoices the FX
|
||||
// rounding lives inside per-row FX diff lines (Dr 7960 / Cr 3960) — NOT
|
||||
// in the tolerance. So the sum must equal tx_abs exactly: anything
|
||||
// unallocated would leave the bank line on 1930 short of the actual
|
||||
// bank receipt and break reconciliation. (PR #607 round-1 review.)
|
||||
const allocated = useMemo(() => {
|
||||
return Object.values(drafts).reduce((sum, d) => sum + parseAmount(d.amount), 0)
|
||||
}, [drafts])
|
||||
|
||||
const leftover = round2(txAmountAbs - allocated)
|
||||
const overshoot = leftover < -0.005
|
||||
const balanced = Math.abs(leftover) < 0.005 && Object.keys(drafts).length > 0
|
||||
// 0.005 SEK matches the RPC's BATCH_AMOUNT_EXCEEDS_TX guard so the
|
||||
// "balanced ✓" indicator never lies to the user about what the server
|
||||
// will accept.
|
||||
const TOLERANCE = 0.005
|
||||
const overshoot = leftover < -TOLERANCE
|
||||
const balanced =
|
||||
Math.abs(leftover) < TOLERANCE && Object.keys(drafts).length > 0
|
||||
const undershoot = leftover > TOLERANCE
|
||||
|
||||
const filteredCandidates = useMemo(() => {
|
||||
const selectedIds = new Set(Object.keys(drafts))
|
||||
@@ -194,7 +215,29 @@ export default function MatchAllocationDialog({
|
||||
setDrafts((prev) => {
|
||||
if (prev[candidate.id]) return prev
|
||||
const remainingTxBudget = Math.max(0, round2(txAmountAbs - allocated))
|
||||
const defaultAmount = Math.min(candidate.remaining, remainingTxBudget)
|
||||
const sameCurrency = candidate.currency === txCurrency
|
||||
|
||||
// Same-currency: partial allowed, default to min(remaining, budget).
|
||||
// Cross-currency: full-payment-only, default to booked SEK (rate
|
||||
// sanity-checked). NOT capped to remainingTxBudget — the cross-
|
||||
// currency RPC guard requires the amount to be within ±10% of
|
||||
// booked_sek, so capping a USD invoice's default at the leftover
|
||||
// budget would silently trigger BATCH_FX_DEVIATION_TOO_LARGE on
|
||||
// submit. Instead, let the row default to the right amount and
|
||||
// the user re-balances the other rows to fit. PR #607 review fix.
|
||||
let defaultAmount: number
|
||||
if (sameCurrency) {
|
||||
defaultAmount = Math.min(candidate.remaining, remainingTxBudget)
|
||||
} else if (isValidExchangeRate(candidate.exchangeRate)) {
|
||||
defaultAmount = round2(candidate.remaining * candidate.exchangeRate)
|
||||
} else {
|
||||
// No (or out-of-range) FX rate. Leave the amount blank rather
|
||||
// than guessing a misleading default; the user must enter the
|
||||
// SEK amount the bank converted to manually. Blocked from
|
||||
// confirm via the per-row warning below.
|
||||
defaultAmount = 0
|
||||
}
|
||||
|
||||
return {
|
||||
...prev,
|
||||
[candidate.id]: {
|
||||
@@ -222,12 +265,10 @@ export default function MatchAllocationDialog({
|
||||
|
||||
async function handleConfirm() {
|
||||
if (!transaction) return
|
||||
if (!balanced && !overshoot) {
|
||||
// Allow undershoot — the tx keeps its leftover unallocated. But reject
|
||||
// a no-allocation submit.
|
||||
if (Object.keys(drafts).length === 0) return
|
||||
}
|
||||
if (overshoot) return
|
||||
// PR #607 round-1 review: require balanced. Undershoot is no longer
|
||||
// allowed because it leaves the bank line short of tx_abs and breaks
|
||||
// reconciliation.
|
||||
if (!balanced || overshoot) return
|
||||
|
||||
setSubmitting(true)
|
||||
try {
|
||||
@@ -384,24 +425,43 @@ export default function MatchAllocationDialog({
|
||||
</p>
|
||||
</div>
|
||||
{isSelected ? (
|
||||
<div className="flex items-center gap-2">
|
||||
<Input
|
||||
type="text"
|
||||
inputMode="decimal"
|
||||
value={draft.amount}
|
||||
onChange={(e) => setDraftAmount(c.id, e.target.value)}
|
||||
className="h-9 w-28 font-mono text-right tabular-nums"
|
||||
aria-label={t('amount_input_aria', { label: c.label })}
|
||||
/>
|
||||
<Button
|
||||
type="button"
|
||||
size="icon"
|
||||
variant="ghost"
|
||||
onClick={() => removeAllocation(c.id)}
|
||||
aria-label={t('remove_aria', { label: c.label })}
|
||||
>
|
||||
<X className="h-4 w-4" />
|
||||
</Button>
|
||||
<div className="flex flex-col items-end gap-1">
|
||||
<div className="flex items-center gap-2">
|
||||
<Input
|
||||
type="text"
|
||||
inputMode="decimal"
|
||||
value={draft.amount}
|
||||
onChange={(e) => setDraftAmount(c.id, e.target.value)}
|
||||
className="h-9 w-28 font-mono text-right tabular-nums"
|
||||
aria-label={t('amount_input_aria', { label: c.label })}
|
||||
/>
|
||||
<Button
|
||||
type="button"
|
||||
size="icon"
|
||||
variant="ghost"
|
||||
onClick={() => removeAllocation(c.id)}
|
||||
aria-label={t('remove_aria', { label: c.label })}
|
||||
>
|
||||
<X className="h-4 w-4" />
|
||||
</Button>
|
||||
</div>
|
||||
{/* FX hint — appears only for cross-currency rows
|
||||
so the user can see what their tx-currency
|
||||
input translates to in invoice currency.
|
||||
When the rate is missing or out of range, we
|
||||
warn instead of silently defaulting to a
|
||||
misleading number. PR #607 round-1 review. */}
|
||||
{c.currency !== txCurrency && (
|
||||
isValidExchangeRate(c.exchangeRate) ? (
|
||||
<p className="text-[11px] tabular-nums text-muted-foreground">
|
||||
≈ {formatCurrency(parseAmount(draft.amount) / c.exchangeRate, c.currency)}
|
||||
</p>
|
||||
) : (
|
||||
<p className="text-[11px] tabular-nums text-warning-foreground">
|
||||
{t('fx_rate_missing_warning', { currency: c.currency })}
|
||||
</p>
|
||||
)
|
||||
)}
|
||||
</div>
|
||||
) : (
|
||||
<Button
|
||||
@@ -450,12 +510,19 @@ export default function MatchAllocationDialog({
|
||||
<Check className="h-4 w-4 flex-shrink-0" />
|
||||
<p>{t('balanced_message')}</p>
|
||||
</div>
|
||||
) : leftover > 0.005 && Object.keys(drafts).length > 0 ? (
|
||||
<p className="text-xs text-muted-foreground">
|
||||
{t('leftover_note', {
|
||||
amount: formatCurrency(leftover, transaction.currency),
|
||||
})}
|
||||
</p>
|
||||
) : undershoot && Object.keys(drafts).length > 0 ? (
|
||||
// Undershoot is now a blocking state — the JE's 1930 line
|
||||
// must equal the bank's actual receipt or reconciliation
|
||||
// breaks. The user must allocate the full amount or remove
|
||||
// selections. PR #607 round-1 review fix.
|
||||
<div className="flex items-start gap-2 rounded-lg bg-warning/10 p-3 text-sm text-warning-foreground">
|
||||
<AlertTriangle className="h-4 w-4 flex-shrink-0 mt-0.5" />
|
||||
<p>
|
||||
{t('undershoot_warning', {
|
||||
amount: formatCurrency(leftover, transaction.currency),
|
||||
})}
|
||||
</p>
|
||||
</div>
|
||||
) : null}
|
||||
</div>
|
||||
</div>
|
||||
@@ -466,7 +533,10 @@ export default function MatchAllocationDialog({
|
||||
</Button>
|
||||
<Button
|
||||
onClick={handleConfirm}
|
||||
disabled={submitting || overshoot || Object.keys(drafts).length === 0}
|
||||
// Confirm requires sum == tx_abs exactly (within rounding).
|
||||
// Anything else lets the JE diverge from the bank line and
|
||||
// breaks reconciliation. PR #607 round-1 review fix.
|
||||
disabled={submitting || !balanced || overshoot}
|
||||
>
|
||||
{submitting && <Loader2 className="mr-2 h-4 w-4 animate-spin" />}
|
||||
{t('confirm')}
|
||||
|
||||
Reference in New Issue
Block a user