From e70fc334bdf129a9c1496942d52bc470e38b38ff Mon Sep 17 00:00:00 2001 From: Maurice Date: Mon, 12 Oct 2026 13:22:32 +0200 Subject: [PATCH] fix(costs): people who left the trip are named and keep their balance in the costs lists Someone removed from a trip stays on the expenses they paid for or shared, but the costs views only knew the current members. They showed up as "?" in the expense chips and the settle-up suggestions, and the balances and final budget lists left them out, so the listed balances no longer added up to zero. The names now come from what the server already sends with the expenses and the settlement, and anyone off the trip who still carries money gets a row after the members, on desktop and on the phone. The expense form is unchanged. Refs #2604 --- .../Budget/CostsPanel.helpers.test.ts | 38 ++++++++++++++++++- .../components/Budget/CostsPanel.helpers.ts | 32 ++++++++++++++++ .../src/components/Budget/CostsPanel.test.tsx | 29 ++++++++++++++ client/src/components/Budget/CostsPanel.tsx | 20 +++++++--- .../src/components/Budget/useCostsLedger.ts | 4 ++ .../mobile/screens/trip/tabs/MCostsTab.tsx | 16 +++++--- 6 files changed, 127 insertions(+), 12 deletions(-) diff --git a/client/src/components/Budget/CostsPanel.helpers.test.ts b/client/src/components/Budget/CostsPanel.helpers.test.ts index 1586c3739..043b90fd0 100644 --- a/client/src/components/Budget/CostsPanel.helpers.test.ts +++ b/client/src/components/Budget/CostsPanel.helpers.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest' -import { calculateTicketShares, expenseEditorFor, newExpenseSeed, receiptToPrefill, finalBudgetFor, finalBudgetSources, hasTicketSplit, paidByUser, payerSum, payersBalanced, readTicketItems, readUserNote, rebalancePayers, settlementDate, splitCents, splitEqualShares, writeTicketItems, type TicketItem } from './CostsPanel.helpers' +import { calculateTicketShares, expenseEditorFor, newExpenseSeed, receiptToPrefill, finalBudgetFor, finalBudgetSources, formerParticipantRows, hasTicketSplit, knownPersonNames, paidByUser, payerSum, payersBalanced, readTicketItems, readUserNote, rebalancePayers, settlementDate, splitCents, splitEqualShares, writeTicketItems, type TicketItem } from './CostsPanel.helpers' describe('splitCents', () => { it('splits evenly when it divides cleanly', () => { @@ -360,3 +360,39 @@ describe('expenseEditorFor', () => { .toEqual({ key: 'booking', editing: null, prefill: { amount: 3 }, close: closeBooking }) }) }) + +// #2604: someone removed from the trip keeps their rows on the expenses, so the +// views name them from what the server sent and keep their money in the lists. +describe('knownPersonNames', () => { + it('names a former member from the expenses and the settlement', () => { + const names = knownPersonNames( + [{ payers: [{ user_id: 4, username: 'Lena' }], members: [{ user_id: 1, username: 'Jonas' }, { user_id: 4, username: 'Lena' }] }], + { balances: [{ user_id: 5, username: 'Tom' }], finalBudgets: [], flows: [{ from: { user_id: 5, username: 'Tom' }, to: { user_id: 6, username: 'Mia' } }] }, + ) + expect(Object.fromEntries(names)).toEqual({ 1: 'Jonas', 4: 'Lena', 5: 'Tom', 6: 'Mia' }) + }) + + it('copes with no settlement and rows without a name', () => { + expect(knownPersonNames([{ payers: [{ user_id: 2 }], members: null }], null).size).toBe(0) + }) +}) + +describe('formerParticipantRows', () => { + const balances = [ + { user_id: 1, balance: 30 }, + { user_id: 4, balance: 60 }, + { user_id: 5, balance: -90 }, + { user_id: 7, balance: 0 }, + ] + + it('lists the people off the trip that still carry money, so the balances add up to zero', () => { + const extra = formerParticipantRows([1, 5], balances, b => Math.abs(b.balance) >= 0.005) + expect(extra).toEqual([{ user_id: 4, balance: 60 }]) + const listed = [...balances.filter(b => [1, 5].includes(b.user_id)), ...extra] + expect(listed.reduce((a, b) => a + b.balance, 0)).toBe(0) + }) + + it('adds nobody when everyone is on the trip', () => { + expect(formerParticipantRows([1, 4, 5, 7], balances, () => true)).toEqual([]) + }) +}) diff --git a/client/src/components/Budget/CostsPanel.helpers.ts b/client/src/components/Budget/CostsPanel.helpers.ts index 4c15a587c..c8b6e290d 100644 --- a/client/src/components/Budget/CostsPanel.helpers.ts +++ b/client/src/components/Budget/CostsPanel.helpers.ts @@ -75,6 +75,38 @@ export function finalBudgetFor(finals: BudgetParticipantFinal[], member: { id: n } } +/** + * Every name the server sent along with the expenses and the settlement, by user + * id. Someone removed from the trip keeps their payer and split rows (#2604) but + * is no longer among the trip members, so the views fall back to these instead + * of printing "?" for them. + */ +export function knownPersonNames( + items: { payers?: { user_id: number; username?: string }[] | null; members?: { user_id: number; username: string }[] | null }[], + settlement: { balances?: { user_id: number; username: string }[]; finalBudgets?: { user_id: number; username: string }[]; flows?: { from: { user_id: number; username: string }; to: { user_id: number; username: string } }[] } | null | undefined, +): Map { + const names = new Map() + const add = (p: { user_id: number; username?: string | null }) => { if (p.username && !names.has(p.user_id)) names.set(p.user_id, p.username) } + for (const item of items) { + for (const p of item.payers || []) add(p) + for (const m of item.members || []) add(m) + } + for (const b of settlement?.balances || []) add(b) + for (const f of settlement?.finalBudgets || []) add(f) + for (const f of settlement?.flows || []) { add(f.from); add(f.to) } + return names +} + +/** + * The server's rows for people no longer on the trip that still carry money, to + * list after the members' own. The balances net to zero across everyone the + * expenses name, so leaving a former member out made the list stop adding up. + */ +export function formerParticipantRows(memberIds: number[], rows: T[], carriesMoney: (row: T) => boolean): T[] { + const onTrip = new Set(memberIds) + return rows.filter(r => !onTrip.has(r.user_id) && carriesMoney(r)) +} + /** * The rows behind one traveler's final budget, ready to print. They arrive as * ids and display cents: the server spreads each of the three figures over its diff --git a/client/src/components/Budget/CostsPanel.test.tsx b/client/src/components/Budget/CostsPanel.test.tsx index a0d8479f5..3d53b216a 100644 --- a/client/src/components/Budget/CostsPanel.test.tsx +++ b/client/src/components/Budget/CostsPanel.test.tsx @@ -2777,4 +2777,33 @@ describe('CostsPanel: a bill the server cannot convert', () => { // currency when it has no quote of its own. expect(asked.every(a => a.base === 'IRR' && a.baseRate === null)).toBe(true) }) + + it('FE-W5COSTS-097: someone who left the trip is named and keeps their balance in the list (#2604)', async () => { + // Carol was removed from the trip; her payer row stays on the expense and the + // server still nets her into the balances. + const item = { + ...buildBudgetItem({ trip_id: 1, category: 'food', name: 'Boat' }), total_price: 90, + payers: [{ user_id: 3, amount: 90, username: 'carol' }], + members: [{ user_id: 1, paid: 0, amount: null, username: 'alice' }, { user_id: 2, paid: 0, amount: null, username: 'bob' }, { user_id: 3, paid: 0, amount: null, username: 'carol' }], + } + server.use( + http.get('/api/trips/1/budget', () => HttpResponse.json({ items: [item] })), + http.get('/api/trips/1/budget/settlement', () => + HttpResponse.json({ + balances: [ + { user_id: 1, username: 'alice', avatar_url: null, balance: -30 }, + { user_id: 2, username: 'bob', avatar_url: null, balance: -30 }, + { user_id: 3, username: 'carol', avatar_url: null, balance: 60 }, + ], + flows: [], + settlements: [], + }) + ), + ) + render() + + await screen.findByText('Boat') + await waitFor(() => expect(screen.getAllByText('carol').length).toBeGreaterThan(0)) + expect(screen.queryByText('?')).not.toBeInTheDocument() + }) }) diff --git a/client/src/components/Budget/CostsPanel.tsx b/client/src/components/Budget/CostsPanel.tsx index d36bd2564..67c12365a 100644 --- a/client/src/components/Budget/CostsPanel.tsx +++ b/client/src/components/Budget/CostsPanel.tsx @@ -14,7 +14,7 @@ import { CustomDatePicker } from '../shared/CustomDateTimePicker' import { useReceiptScan } from './useReceiptScan' import { ReceiptScanModal } from './ReceiptScanModal' import { SYMBOLS, SPLIT_COLORS } from './BudgetPanel.constants' -import { finalBudgetFor, finalBudgetSources, NOTE_MAX, paidByUser, readUserNote, settlementDate } from './CostsPanel.helpers' +import { finalBudgetFor, finalBudgetSources, formerParticipantRows, NOTE_MAX, paidByUser, readUserNote, settlementDate } from './CostsPanel.helpers' import { COST_CAT_META, COST_CATEGORY_LIST, catMeta } from './costsCategories' import { baseTotal as baseTotalOf, booked as bookedAmount, currencyOptions, isUnfinished as isUnfinishedModel, lineOf as lineOfModel, memberShareOf, @@ -69,7 +69,7 @@ export default function CostsPanel({ tripId, tripMembers = [] }: CostsPanelProps // Display/base currency, the trip's own, the settlement, totals, filters and // the ledger writes: the same ledger the phone tab runs on. const { - me, base, tripCurrency, ctx, settlement, settlementError, loadSettlement, + me, base, tripCurrency, ctx, settlement, settlementError, loadSettlement, knownNames, search, setSearch, segment: filter, setSegment: setFilter, catFilter, setCatFilter, dayFilter, setDayFilter, totals, filtered, filteredSettlements, catBreakdown, exportCsv: handleExportCsv, deleteExpense: handleDelete, undoSettlement, settleFlow, settleAll, @@ -102,12 +102,14 @@ export default function CostsPanel({ tripId, tripMembers = [] }: CostsPanelProps const people = tripMembers const personById = useCallback((id: number) => people.find(p => p.id === id), [people]) - const personName = useCallback((id: number) => id === me ? t('costs.you') : (personById(id)?.username || '?'), [me, personById, t]) + // Someone who left the trip is no longer in `people`, but the expenses still name them (#2604). + const nameOf = useCallback((id: number) => personById(id)?.username || knownNames.get(id) || '?', [personById, knownNames]) + const personName = useCallback((id: number) => id === me ? t('costs.you') : nameOf(id), [me, nameOf, t]) const colorFor = useCallback((id: number) => { const idx = people.findIndex(p => p.id === id) return SPLIT_COLORS[(idx >= 0 ? idx : 0) % SPLIT_COLORS.length].gradient }, [people]) - const initial = useCallback((id: number) => id === me ? t('costs.youShort') : (personById(id)?.username || '?').charAt(0).toUpperCase(), [me, personById, t]) + const initial = useCallback((id: number) => id === me ? t('costs.youShort') : nameOf(id).charAt(0).toUpperCase(), [me, nameOf, t]) const fmt = useCallback((v: number, c = base) => formatMoney(v, c, locale), [base, locale]) const fmt0 = useCallback((v: number, c = base) => formatMoney(v, c, locale, { decimals: 0 }), [base, locale]) @@ -840,7 +842,10 @@ export default function CostsPanel({ tripId, tripMembers = [] }: CostsPanelProps function BalancesList({ balances }: { balances: CostsSettlementResponse['balances'] }) { if (settlementError) return loadFailed() - const rows = people.map(p => balances.find(b => b.user_id === p.id) || { user_id: p.id, username: p.username, avatar_url: null, balance: 0 }) + const rows = [ + ...people.map(p => balances.find(b => b.user_id === p.id) || { user_id: p.id, username: p.username, avatar_url: null, balance: 0 }), + ...formerParticipantRows(people.map(p => p.id), balances, b => Math.abs(b.balance) >= 0.005), + ] const max = Math.max(1, ...rows.map(r => Math.abs(r.balance))) return (
@@ -881,7 +886,10 @@ export default function CostsPanel({ tripId, tripMembers = [] }: CostsPanelProps function FinalBudgetList() { if (settlementError) return loadFailed() const finals = settlement?.finalBudgets || [] - const rows = people.map(p => finalBudgetFor(finals, p)) + const rows = [ + ...people.map(p => finalBudgetFor(finals, p)), + ...formerParticipantRows(people.map(p => p.id), finals, f => Math.abs(f.final) >= 0.005 || Math.abs(f.expenses) >= 0.005), + ] return (
{rows.map(r => { diff --git a/client/src/components/Budget/useCostsLedger.ts b/client/src/components/Budget/useCostsLedger.ts index fdc106a68..7d703b7fc 100644 --- a/client/src/components/Budget/useCostsLedger.ts +++ b/client/src/components/Budget/useCostsLedger.ts @@ -20,6 +20,7 @@ import { type CostsSegment, type CostsSettlementResponse, } from './costsModel'; +import { knownPersonNames } from './CostsPanel.helpers'; import { useFreezeMissingRates } from './useFreezeMissingRates'; type Translate = (key: string, params?: Record) => string; @@ -129,6 +130,8 @@ export function useCostsLedger({ const shareOf = skipUnfinishedShares ? settledShareOf : myShareOf; const flows = useMemo(() => settlement?.flows || [], [settlement]); + // Names for people the expenses still name after they left the trip (#2604). + const knownNames = useMemo(() => knownPersonNames(budgetItems, settlement), [budgetItems, settlement]); // `ctx` carries the rates. They can land after the expenses, and without it in the // deps the cards kept the sums they were first added up with while the rows moved on. const totals = useMemo(() => computeTotals(budgetItems, flows, ctx, shareOf), [budgetItems, flows, ctx, shareOf]); @@ -224,6 +227,7 @@ export function useCostsLedger({ settlementError, loadSettlement, flows, + knownNames, search, setSearch, segment, diff --git a/client/src/mobile/screens/trip/tabs/MCostsTab.tsx b/client/src/mobile/screens/trip/tabs/MCostsTab.tsx index 0a3314bf3..8d82e2006 100644 --- a/client/src/mobile/screens/trip/tabs/MCostsTab.tsx +++ b/client/src/mobile/screens/trip/tabs/MCostsTab.tsx @@ -11,7 +11,7 @@ import MCostSheet from '../sheets/MCostSheet' import { ReceiptPreviewModal } from '../../../../components/Budget/ReceiptPreviewModal' import { useCostsLedger } from '../../../../components/Budget/useCostsLedger' import { useSettlementForm } from '../../../../components/Budget/useSettlementForm' -import { finalBudgetFor, finalBudgetSources, NOTE_MAX, paidByUser, readUserNote } from '../../../../components/Budget/CostsPanel.helpers' +import { finalBudgetFor, finalBudgetSources, formerParticipantRows, NOTE_MAX, paidByUser, readUserNote } from '../../../../components/Budget/CostsPanel.helpers' import { catMeta, COST_CAT_META } from '../../../../components/Budget/costsCategories' import CustomSelect from '../../../../components/shared/CustomSelect' import { CustomDatePicker } from '../../../../components/shared/CustomDateTimePicker' @@ -48,7 +48,7 @@ export default function MCostsTab({ planner, shell }: MTabScreenProps) { // The display and trip currencies, the server-computed settlement, totals, // filters and the ledger writes: the same ledger the desktop panel runs on. const { - me, base, tripCurrency, ctx, settlement, settlementError, loadSettlement, flows, + me, base, tripCurrency, ctx, settlement, settlementError, loadSettlement, flows, knownNames, search, setSearch, segment, setSegment, catFilter, setCatFilter, dayFilter, setDayFilter, totals, filtered, filteredSettlements, catBreakdown, exportCsv: handleExportCsv, deleteExpense, undoSettlement: handleUndoSettlement, @@ -70,7 +70,13 @@ export default function MCostsTab({ planner, shell }: MTabScreenProps) { const catKeys = useMemo(() => categoryFilterKeys(budgetItems), [budgetItems]) const dayKeys = useMemo(() => dayFilterKeys(budgetItems), [budgetItems]) - const personName = (id: number) => (id === me ? t('costs.you') : tripMembers.find(p => p.id === id)?.username || '?') + // Someone who left the trip is no longer a member, but the expenses still name them (#2604). + const personName = (id: number) => (id === me ? t('costs.you') : tripMembers.find(p => p.id === id)?.username || knownNames.get(id) || '?') + // The members, then whoever left the trip with money still on their name, so the rows add up. + const memberIds = tripMembers.map(p => p.id) + const asPerson = (r: { user_id: number; username: string; avatar_url?: string | null }) => ({ id: r.user_id, username: r.username, avatar_url: r.avatar_url ?? null }) + const balancePeople = [...tripMembers, ...formerParticipantRows(memberIds, settlement?.balances || [], b => Math.abs(b.balance) >= 0.005).map(asPerson)] + const finalPeople = [...tripMembers, ...formerParticipantRows(memberIds, settlement?.finalBudgets || [], f => Math.abs(f.final) >= 0.005 || Math.abs(f.expenses) >= 0.005).map(asPerson)] const shortDate = (dateIso: string) => { try { @@ -232,7 +238,7 @@ export default function MCostsTab({ planner, shell }: MTabScreenProps) { )}
{t('costs.balances')}
- {tripMembers.map(p => { + {balancePeople.map(p => { const balance = settlement?.balances.find(b => b.user_id === p.id)?.balance ?? 0 const pos = balance > 0.01 const neg = balance < -0.01 @@ -256,7 +262,7 @@ export default function MCostsTab({ planner, shell }: MTabScreenProps) {
{t('costs.finalBudget')}
{settlementError ? (

{t('common.unknownError')}

- ) : tripMembers.map(p => { + ) : finalPeople.map(p => { const row = finalBudgetFor(settlement?.finalBudgets || [], p) const open = expandedFinalId === p.id return (