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 (