From 3b9daf2606ef2a6dedb30e2c57fa07e30e69d34b Mon Sep 17 00:00:00 2001 From: Jakob Wennberg Date: Sun, 6 Sep 2026 20:19:18 +0200 Subject: [PATCH] fix(expenses): review follow-ups from #2333 (#2352) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - The owner's claimant key is trimmed and lower-cased, the same rule the payout RPC applies, so "Jakob" and "jakob " are one person with one Att göra row and one exact-amount match. - The inbox pages through every registered claim (fetchAllRows) before pairing, so a long backlog can never understate a person's debt. - A foreign receipt's VAT field is locked at 0 and 0 is what is submitted. - Transport failures in the one-click and picker confirms show the destructive toast instead of failing silently. - The open-claims flag is set only after the stale-fetch guard, and a payout match decrements the inbox count like every other row exit. - Test: reset the live-link mock before the bank_line junction case. - Wording: "Återbetalning av utlägg". Co-authored-by: Jakob Wennberg <311770904+jakobwennberg-oss@users.noreply.github.com> Co-authored-by: Claude Fable 5.1 --- app/(dashboard)/transactions/page.tsx | 35 +++++++++++++------ .../__tests__/route.test.ts | 1 + .../general/RegisterExpenseDialog.tsx | 8 +++-- .../transactions/ExpenseClaimPickerDialog.tsx | 10 ++++-- .../__tests__/booking-feedback-parity.test.ts | 9 ++--- .../expense-payout-candidates.test.ts | 9 +++++ lib/expenses/expense-payout-candidates.ts | 12 ++++++- lib/worklist/__tests__/categories.test.ts | 2 +- lib/worklist/types.ts | 2 +- messages/sv.json | 2 +- 10 files changed, 66 insertions(+), 24 deletions(-) diff --git a/app/(dashboard)/transactions/page.tsx b/app/(dashboard)/transactions/page.tsx index ece913b8..363d6206 100644 --- a/app/(dashboard)/transactions/page.tsx +++ b/app/(dashboard)/transactions/page.tsx @@ -212,19 +212,25 @@ async function fetchExpensePayoutMatches( ): Promise<{ byTransaction: Map; people: ExpensePayoutDue[] }> { const out = new Map() if (!companyId || rows.length === 0) return { byTransaction: out, people: [] } - const { data, error } = await supabase - .from('expense_claims') - .select('id, employee_id, claimant_name, liability_account, amount_sek, expense_date') - .eq('company_id', companyId) - .eq('status', 'registered') - .order('expense_date', { ascending: true }) - if (error) { + // Every registered claim, paged past the PostgREST row cap: a truncated set + // would understate a person's debt and suggest a payout for part of it. + let claimRows: Parameters[0] + try { + claimRows = await fetchAllRows(({ from, to }) => + supabase + .from('expense_claims') + .select('id, employee_id, claimant_name, liability_account, amount_sek, expense_date') + .eq('company_id', companyId) + .eq('status', 'registered') + .order('expense_date', { ascending: true }) + .order('id', { ascending: true }) + .range(from, to), + ) + } catch (error) { console.error('[fetchExpensePayoutMatches] expense_claims query failed', error) return { byTransaction: out, people: [] } } - const people = groupExpenseClaimsByPerson( - (data ?? []) as Parameters[0], - ) + const people = groupExpenseClaimsByPerson(claimRows) for (const [txId, m] of matchTransactionsToExpensePayouts(rows, people)) out.set(txId, m.person) return { byTransaction: out, people } } @@ -1170,11 +1176,11 @@ export default function TransactionsPage() { fetchExpensePayoutMatches(supabase, companyId, allRows), ]) const expensePayoutMap = expensePayouts.byTransaction - setHasOpenExpenseClaims(expensePayouts.people.length > 0) // Re-check after the second await: a scope change during the match // enrichment must also discard this response. if (fetchGenerationRef.current !== generation) return + setHasOpenExpenseClaims(expensePayouts.people.length > 0) const transactionsWithInvoices: TransactionWithInvoice[] = allRows.map((t) => ({ ...t, @@ -2329,6 +2335,12 @@ export default function TransactionsPage() { }) setExpensePayoutDialogOpen(false) applyExpensePayoutBooked(selectedTransaction.id, result.journal_entry_id, person.key) + } catch (error) { + toast({ + title: t('expense_payout_match_failed_title'), + description: getErrorMessage(error, { context: 'transaction' }), + variant: 'destructive', + }) } finally { setIsConfirmingMatch(false) } @@ -2339,6 +2351,7 @@ export default function TransactionsPage() { // same person drops its suggestion, since that person is now paid. function applyExpensePayoutBooked(transactionId: string, journalEntryId: string, personKey: string) { setExitingIds((prev) => new Set(prev).add(transactionId)) + setTotalUncategorizedCount((prev) => Math.max(0, (prev ?? 1) - 1)) setTimeout(() => { setTransactions((prev) => prev.map((tx) => { diff --git a/app/api/transactions/[id]/match-expense-payout/__tests__/route.test.ts b/app/api/transactions/[id]/match-expense-payout/__tests__/route.test.ts index 1299b885..e692f975 100644 --- a/app/api/transactions/[id]/match-expense-payout/__tests__/route.test.ts +++ b/app/api/transactions/[id]/match-expense-payout/__tests__/route.test.ts @@ -137,6 +137,7 @@ describe('POST /api/transactions/[id]/match-expense-payout', () => { expect(parsed.body.error.code).toBe('EXPENSE_PAYOUT_MATCH_TX_ALREADY_LINKED') reset() + mockHasLiveLink.mockResolvedValue(false) enqueue({ data: makeTxRow({ transaction_voucher_links: [{ journal_entry_id: 'je-bulk', role: 'bank_line' }], diff --git a/components/extensions/general/RegisterExpenseDialog.tsx b/components/extensions/general/RegisterExpenseDialog.tsx index cab5b7b4..bd1f4c18 100644 --- a/components/extensions/general/RegisterExpenseDialog.tsx +++ b/components/extensions/general/RegisterExpenseDialog.tsx @@ -132,7 +132,9 @@ export default function RegisterExpenseDialog({ open, onOpenChange, item, payer, }, [open, payer, employeesLoaded]) const amount = parseAmount(amountInput) - const vatAmount = parseAmount(vatInput) + // Foreign VAT is never deductible here: the field is locked and 0 is what + // gets submitted, whatever the extraction said. + const vatAmount = isForeign ? 0 : parseAmount(vatInput) const net = roundOre(amount - vatAmount) const employee = employees.find((e) => e.id === employeeId) ?? null const claimantName = @@ -302,9 +304,9 @@ export default function RegisterExpenseDialog({ open, onOpenChange, item, payer, setVatInput(e.target.value)} - disabled={isSubmitting} + disabled={isSubmitting || isForeign} className="tabular-nums" /> diff --git a/components/transactions/ExpenseClaimPickerDialog.tsx b/components/transactions/ExpenseClaimPickerDialog.tsx index a7f63e6b..d750b63f 100644 --- a/components/transactions/ExpenseClaimPickerDialog.tsx +++ b/components/transactions/ExpenseClaimPickerDialog.tsx @@ -25,7 +25,7 @@ import { useToast } from '@/components/ui/use-toast' import { getErrorMessage } from '@/lib/errors/get-error-message' import { formatCurrency, formatDate } from '@/lib/utils' import { roundOre } from '@/lib/money' -import { groupExpenseClaimsByPerson } from '@/lib/expenses/expense-payout-candidates' +import { expenseClaimantKey, groupExpenseClaimsByPerson } from '@/lib/expenses/expense-payout-candidates' import type { ExpensePayoutDue } from '@/lib/worklist/types' import type { TransactionWithInvoice } from './transaction-types' @@ -98,7 +98,7 @@ export default function ExpenseClaimPickerDialog({ open, onOpenChange, transacti const personClaims = useMemo( () => claims - .filter((c) => (c.employee_id ?? `owner:${c.claimant_name}`) === personKey) + .filter((c) => expenseClaimantKey(c) === personKey) .sort((a, b) => a.expense_date.localeCompare(b.expense_date)), [claims, personKey], ) @@ -136,6 +136,12 @@ export default function ExpenseClaimPickerDialog({ open, onOpenChange, transacti return } onMatched(transaction.id, result.journal_entry_id, personKey) + } catch (error) { + toast({ + title: t('failed_title'), + description: getErrorMessage(error, { context: 'transaction' }), + variant: 'destructive', + }) } finally { setSubmitting(false) } diff --git a/components/transactions/__tests__/booking-feedback-parity.test.ts b/components/transactions/__tests__/booking-feedback-parity.test.ts index dca76168..62fee2bc 100644 --- a/components/transactions/__tests__/booking-feedback-parity.test.ts +++ b/components/transactions/__tests__/booking-feedback-parity.test.ts @@ -83,12 +83,13 @@ describe('transactions page booking feedback', () => { it('decrements the unbooked count on every path that removes a row', () => { // finishBooking, handleTransactionBooked (manual booking dialog / voucher // match), the three other single-row exits already on the page, the - // duplicate-dialog "Ignorera transaktionen" tail, and - // handleDeleteTransaction (deleting a pending row must not leave the - // inbox badge stale: the realtime echo is not guaranteed for DELETE). + // duplicate-dialog "Ignorera transaktionen" tail, handleDeleteTransaction + // (deleting a pending row must not leave the inbox badge stale: the + // realtime echo is not guaranteed for DELETE), and applyExpensePayoutBooked + // (a transfer booked as the repayment of utlägg leaves the inbox too). expect( PAGE_SRC.match(/setTotalUncategorizedCount\(\(prev\) => Math\.max\(0, \(prev \?\? 1\) - 1\)\)/g) ?? [], - ).toHaveLength(7) + ).toHaveLength(8) }) it('ships the undo strings it renders in both locales', () => { diff --git a/lib/expenses/__tests__/expense-payout-candidates.test.ts b/lib/expenses/__tests__/expense-payout-candidates.test.ts index f0a510cc..ea0e8bf6 100644 --- a/lib/expenses/__tests__/expense-payout-candidates.test.ts +++ b/lib/expenses/__tests__/expense-payout-candidates.test.ts @@ -18,6 +18,15 @@ describe('groupExpenseClaimsByPerson', () => { expect(out[0].claim_ids).toEqual(['a', 'b']) }) + it('treats "Jakob" and "jakob " as one owner, the way the payout RPC does', () => { + const out = groupExpenseClaimsByPerson([ + { id: 'a', employee_id: null, claimant_name: 'Jakob', liability_account: '2893', amount_sek: 100, expense_date: '2026-09-01' }, + { id: 'b', employee_id: null, claimant_name: 'jakob ', liability_account: '2893', amount_sek: 50, expense_date: '2026-09-02' }, + ]) + expect(out).toHaveLength(1) + expect(out[0]).toMatchObject({ key: 'owner:jakob', claimant_name: 'Jakob', total_sek: 150, claim_ids: ['a', 'b'] }) + }) + it('leaves an enskild firma owner out: egen insättning is not a debt', () => { const out = groupExpenseClaimsByPerson([ { id: 'a', employee_id: null, claimant_name: 'Sara', liability_account: '2018', amount_sek: 500, expense_date: '2026-09-01' }, diff --git a/lib/expenses/expense-payout-candidates.ts b/lib/expenses/expense-payout-candidates.ts index b11b0a83..9f93371a 100644 --- a/lib/expenses/expense-payout-candidates.ts +++ b/lib/expenses/expense-payout-candidates.ts @@ -22,6 +22,16 @@ export interface ExpenseClaimRowForGrouping { expense_date: string } +/** + * The person behind a claim: the employee id, or for the owner the trimmed, + * lower-cased name. Same rule as create_expense_payout_batch's claimant check + * (lower(btrim(claimant_name))), so "Jakob" and "jakob " are one person here + * exactly when the RPC would accept their claims in one payout. + */ +export function expenseClaimantKey(row: { employee_id: string | null; claimant_name: string }): string { + return row.employee_id ?? `owner:${row.claimant_name.trim().toLowerCase()}` +} + /** * Group registered claims into one item per person, oldest debt first. * @@ -34,7 +44,7 @@ export function groupExpenseClaimsByPerson(rows: ExpenseClaimRowForGrouping[]): const byPerson = new Map() for (const row of rows) { if (row.liability_account === '2018') continue - const key = row.employee_id ?? `owner:${row.claimant_name}` + const key = expenseClaimantKey(row) const amount = Number(row.amount_sek) || 0 const existing = byPerson.get(key) if (existing) { diff --git a/lib/worklist/__tests__/categories.test.ts b/lib/worklist/__tests__/categories.test.ts index 17149af0..c23ee645 100644 --- a/lib/worklist/__tests__/categories.test.ts +++ b/lib/worklist/__tests__/categories.test.ts @@ -568,7 +568,7 @@ describe('listExpensePayoutsDue', () => { oldest_expense_date: '2026-09-02', }, { - key: 'owner:Jakob', + key: 'owner:jakob', employee_id: null, claimant_name: 'Jakob', liability_account: '2893', diff --git a/lib/worklist/types.ts b/lib/worklist/types.ts index 66b8f7dd..9c8ffade 100644 --- a/lib/worklist/types.ts +++ b/lib/worklist/types.ts @@ -126,7 +126,7 @@ export type WorklistCategory = (typeof WORKLIST_CATEGORIES)[number] * (or claimant_name for the owner, who has no employee row). */ export interface ExpensePayoutDue { - /** employee_id, or `owner:` for claims without one. */ + /** employee_id, or `owner:` for claims without one. */ key: string employee_id: string | null claimant_name: string diff --git a/messages/sv.json b/messages/sv.json index 24706908..798ccf55 100644 --- a/messages/sv.json +++ b/messages/sv.json @@ -6729,7 +6729,7 @@ "dismiss": "Dölj" }, "dashboard": { - "suggested_kind_expense_payout": "Återbetalning utlägg", + "suggested_kind_expense_payout": "Återbetalning av utlägg", "band_betala": "Betala", "row_expense_payout": "Betala ut utlägg till {name}", "row_expense_payout_detail_one": "1 kvitto · {date}",