diff --git a/client/src/components/Budget/CostsPanel.helpers.test.ts b/client/src/components/Budget/CostsPanel.helpers.test.ts index 80ac00812..ec2d15cb6 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 { splitCents, payerSum, payersBalanced, rebalancePayers } from './CostsPanel.helpers' +import { hasTicketSplit, payerSum, payersBalanced, readTicketItems, readUserNote, rebalancePayers, splitCents, writeTicketItems } from './CostsPanel.helpers' describe('splitCents', () => { it('splits evenly when it divides cleanly', () => { @@ -76,3 +76,59 @@ describe('rebalancePayers', () => { expect(payersBalanced(next, new Set([1, 2, 3]), 100)).toBe(true) }) }) + +// ── Notes vs. itemized receipts (#1658) ────────────────────────────────────── + +describe('readTicketItems', () => { + const receipt = '{"items":[{"name":"Bread","price":"3.50","parts":[1,2]},{"name":"Wine","price":"12","parts":[2]}]}' + + it('reads the receipt out of its own column', () => { + const items = readTicketItems({ ticket_json: receipt }) + expect(items.map(i => i.name)).toEqual(['Bread', 'Wine']) + expect(items[0].participants).toEqual(new Set([1, 2])) + expect(items[1].price).toBe('12') + }) + + it('still reads a response cached before the column existed', () => { + const items = readTicketItems({ note: 'TICKETJSON:' + receipt }) + expect(items).toHaveLength(2) + expect(items[1].participants).toEqual(new Set([2])) + }) + + it('prefers the column over the legacy note when both are present', () => { + const items = readTicketItems({ ticket_json: '{"items":[{"name":"Only","price":"1","parts":[]}]}', note: 'TICKETJSON:' + receipt }) + expect(items.map(i => i.name)).toEqual(['Only']) + }) + + it('is empty for an expense with neither, and for malformed JSON', () => { + expect(readTicketItems({ note: 'just a note' })).toEqual([]) + expect(readTicketItems(null)).toEqual([]) + expect(readTicketItems({ ticket_json: '{oops' })).toEqual([]) + }) + + it('round-trips through writeTicketItems', () => { + const items = readTicketItems({ ticket_json: receipt }) + expect(readTicketItems({ ticket_json: writeTicketItems(items) })).toEqual(items) + }) +}) + +describe('hasTicketSplit', () => { + it('recognises both storage shapes and nothing else', () => { + expect(hasTicketSplit({ ticket_json: '{"items":[]}' })).toBe(true) + expect(hasTicketSplit({ note: 'TICKETJSON:{"items":[]}' })).toBe(true) + expect(hasTicketSplit({ note: 'dinner with Ben' })).toBe(false) + expect(hasTicketSplit(undefined)).toBe(false) + }) +}) + +describe('readUserNote', () => { + it('returns what the user typed', () => { + expect(readUserNote({ note: 'Lisa pays half back' })).toBe('Lisa pays half back') + }) + + it('never returns a receipt blob as if it were a note', () => { + expect(readUserNote({ note: 'TICKETJSON:{"items":[]}' })).toBe('') + expect(readUserNote({ note: null })).toBe('') + expect(readUserNote(null)).toBe('') + }) +}) diff --git a/client/src/components/Budget/CostsPanel.helpers.ts b/client/src/components/Budget/CostsPanel.helpers.ts index 8e5c94042..74e6d631e 100644 --- a/client/src/components/Budget/CostsPanel.helpers.ts +++ b/client/src/components/Budget/CostsPanel.helpers.ts @@ -88,6 +88,53 @@ export interface TicketItem { participants: Set } +/** + * Read the itemized receipt off an expense (#1658). + * + * It lives in `ticket_json` since migration 186. Before that it was smuggled + * through `note` behind a `TICKETJSON:` prefix, which is why an expense split by + * receipt could never carry a written note. The old shape is still read here so + * a response cached before the migration keeps rendering its split. + */ +export function readTicketItems(item: { ticket_json?: string | null; note?: string | null } | null | undefined): TicketItem[] { + const raw = item?.ticket_json ?? (item?.note?.startsWith('TICKETJSON:') ? item.note.slice(11) : null) + if (!raw) return [] + try { + const parsed = JSON.parse(raw) + return (parsed.items || []).map((line: { name?: string; price?: unknown; parts?: number[] }, i: number) => ({ + id: `${i}`, + name: String(line.name ?? ''), + price: String(line.price ?? ''), + participants: new Set(line.parts || []), + })) + } catch { + return [] + } +} + +/** True when this expense is split line by line rather than equally or by amount. */ +export function hasTicketSplit(item: { ticket_json?: string | null; note?: string | null } | null | undefined): boolean { + return Boolean(item?.ticket_json || item?.note?.startsWith('TICKETJSON:')) +} + +/** Serialize the receipt lines for storage in `ticket_json`. */ +export function writeTicketItems(items: TicketItem[]): string { + return JSON.stringify({ items: items.map(i => ({ name: i.name, price: i.price, parts: [...i.participants] })) }) +} + +/** Long enough for a receipt discrepancy or a reimbursement reminder, short + * enough that the row stays a row. */ +export const NOTE_MAX = 500 + +/** + * The note a user actually typed, which is never the receipt blob. Only matters + * for data written before migration 186 moved the receipt out of the field. + */ +export function readUserNote(item: { note?: string | null } | null | undefined): string { + const note = item?.note + return note && !note.startsWith('TICKETJSON:') ? note : '' +} + /** Per-person totals for a receipt split line by line, plus the receipt total. */ export function calculateTicketShares(items: TicketItem[]): { shares: Record; total: number } { const shares: Record = {} diff --git a/client/src/components/Budget/CostsPanel.test.tsx b/client/src/components/Budget/CostsPanel.test.tsx index 6e42b259c..6dd9ec25c 100644 --- a/client/src/components/Budget/CostsPanel.test.tsx +++ b/client/src/components/Budget/CostsPanel.test.tsx @@ -171,6 +171,79 @@ describe('CostsPanel — settlements in the ledger', () => { expect(screen.getByText('Unfinished')).toBeInTheDocument() }) + // ── Notes on an expense (#1658) ──────────────────────────────────────────── + + it('shows a note on the row and expands it on click', async () => { + const long = 'Bought the whole week of breakfasts here, Ben pays back his half once the card statement lands' + const item = { + ...buildBudgetItem({ trip_id: 1, category: 'food', name: 'Supermarket' }), + total_price: 60, + note: long, + payers: [{ user_id: 1, amount: 60, username: 'alice' }], + } + server.use( + http.get('/api/trips/1/budget', () => HttpResponse.json({ items: [item] })), + http.get('/api/trips/1/budget/settlement', () => HttpResponse.json({ balances: [], flows: [], settlements: [] })), + ) + const user = userEvent.setup() + render() + + // Re-queried after each click: the row is re-created on every render of the + // panel, so a handle taken before the click points at a detached node. + const noteToggle = () => screen.getByRole('button', { name: new RegExp(long.slice(0, 20)) }) + await screen.findByText('Supermarket') + expect(noteToggle()).toHaveAttribute('aria-expanded', 'false') + await user.click(noteToggle()) + expect(noteToggle()).toHaveAttribute('aria-expanded', 'true') + await user.click(noteToggle()) + expect(noteToggle()).toHaveAttribute('aria-expanded', 'false') + }) + + it('never renders a receipt blob as a note', async () => { + const item = { + ...buildBudgetItem({ trip_id: 1, category: 'food', name: 'Market' }), + total_price: 20, + // How the receipt was stored before it got its own column. + note: 'TICKETJSON:{"items":[{"name":"Cheese","price":"20","parts":[1]}]}', + payers: [{ user_id: 1, amount: 20, username: 'alice' }], + } + server.use( + http.get('/api/trips/1/budget', () => HttpResponse.json({ items: [item] })), + http.get('/api/trips/1/budget/settlement', () => HttpResponse.json({ balances: [], flows: [], settlements: [] })), + ) + render() + + await screen.findByText('Market') + expect(screen.queryByText(/TICKETJSON/)).toBeNull() + }) + + it('keeps an existing note when the expense is saved again', async () => { + const item = { + ...buildBudgetItem({ trip_id: 1, category: 'food', name: 'Lunch' }), + total_price: 30, + note: 'split with the neighbours', + payers: [{ user_id: 1, amount: 30, username: 'alice' }], + } + let patched: Record | null = null + server.use( + http.get('/api/trips/1/budget', () => HttpResponse.json({ items: [item] })), + http.get('/api/trips/1/budget/settlement', () => HttpResponse.json({ balances: [], flows: [], settlements: [] })), + http.put('/api/trips/1/budget/:id', async ({ request }) => { + patched = (await request.json()) as Record + return HttpResponse.json({ item: { ...item, ...patched } }) + }), + ) + const user = userEvent.setup() + render() + + await screen.findByText('Lunch') + await user.click(screen.getAllByTitle('Edit')[0]) + await user.click(await screen.findByRole('button', { name: 'Save' })) + + await waitFor(() => expect(patched).toBeTruthy()) + expect(patched!.note).toBe('split with the neighbours') + }) + it('shows the net hint on a settled expense row and hides it while unfinished', async () => { seedStore(useAuthStore, { user: buildUser({ id: 1, username: 'alice' }), isAuthenticated: true }) seedStore(useSettingsStore, { settings: { ...useSettingsStore.getState().settings, default_currency: 'EUR' } }) @@ -534,7 +607,9 @@ describe('CostsPanel — settlements in the ledger', () => { expect.objectContaining({ user_id: 1, amount: 75 }), expect.objectContaining({ user_id: 2, amount: 25 }), ])) - expect(posted!.note).toContain('TICKETJSON:') + // The receipt has its own column since #1658; `note` is the user's text. + expect(JSON.parse(posted!.ticket_json as string).items).toHaveLength(3) + expect(posted!.note).toBeNull() }) // ── Display currency ─────────────────────────────────────────────────────── diff --git a/client/src/components/Budget/CostsPanel.tsx b/client/src/components/Budget/CostsPanel.tsx index d4440824e..4c10528cd 100644 --- a/client/src/components/Budget/CostsPanel.tsx +++ b/client/src/components/Budget/CostsPanel.tsx @@ -1,6 +1,6 @@ import { useState, useEffect, useMemo, useCallback } from 'react' import { useSearchParams } from 'react-router' -import { ArrowDown, ArrowUp, BarChart3, Plus, Search, ArrowRight, ArrowLeftRight, Check, RotateCcw, Pencil, Trash2, AlertCircle, Download } from 'lucide-react' +import { ArrowDown, ArrowUp, BarChart3, Plus, Search, ArrowRight, ArrowLeftRight, Check, RotateCcw, Pencil, Trash2, AlertCircle, Download, StickyNote, ChevronDown } from 'lucide-react' import { useTripStore } from '../../store/tripStore' import { useAuthStore } from '../../store/authStore' import { useSettingsStore } from '../../store/settingsStore' @@ -16,7 +16,7 @@ import Modal from '../shared/Modal' import CustomSelect from '../shared/CustomSelect' import { CustomDatePicker } from '../shared/CustomDateTimePicker' import { SYMBOLS, currenciesWith, SPLIT_COLORS } from './BudgetPanel.constants' -import { calculateTicketShares, payersBalanced, rebalancePayers, splitEqualShares, type TicketItem } from './CostsPanel.helpers' +import { calculateTicketShares, hasTicketSplit, NOTE_MAX, payersBalanced, readTicketItems, readUserNote, rebalancePayers, splitEqualShares, writeTicketItems, type TicketItem } from './CostsPanel.helpers' import { COST_CATEGORY_LIST, catMeta } from './costsCategories' import type { BudgetItem } from '../../types' import type { TripMember } from './BudgetPanelMemberChips' @@ -80,6 +80,9 @@ export default function CostsPanel({ tripId, tripMembers = [] }: CostsPanelProps const [dayFilter, setDayFilter] = useState('') // '' = all days, else YYYY-MM-DD const [modalOpen, setModalOpen] = useState(false) const [editing, setEditing] = useState(null) + // One note open at a time: two expanded rows next to each other read as a mess, + // and the point of the collapse is that the list stays scannable. + const [expandedNoteId, setExpandedNoteId] = useState(null) const [editingSettlement, setEditingSettlement] = useState(null) const [addingPayment, setAddingPayment] = useState(false) @@ -246,8 +249,7 @@ export default function CostsPanel({ tripId, tripMembers = [] }: CostsPanelProps const items = budgetItems.slice().sort((a, b) => (a.expense_date || '').localeCompare(b.expense_date || '')) for (const e of items) { const cur = curOf(e) - // Ticket notes carry the itemized-receipt JSON, not a human note. - const note = e.note && !e.note.startsWith('TICKETJSON:') ? e.note : '' + const note = readUserNote(e) rows.push([ esc(fmtDate(e.expense_date || '')), esc(e.name), esc(t(catMeta(e.category).labelKey)), (e.total_price || 0).toFixed(currencyDecimals(cur)), cur, @@ -502,6 +504,11 @@ export default function CostsPanel({ tripId, tripMembers = [] }: CostsPanelProps .costs-root .text-content-muted { color: var(--c-ink2) !important; } .costs-root .text-content-faint { color: var(--c-ink3) !important; } .costs-root .exp-actions { opacity: 1; } + .costs-root .exp-actions button { background: none; } + .costs-root .exp-actions button:hover { background: var(--bg-secondary); color: var(--c-ink); } + /* Destructive actions keep their own tint: the neutral one would read as + "safe" on the button that deletes the expense. */ + .costs-root .exp-actions .exp-action-danger:hover { background: rgba(220,38,38,0.14); color: #dc2626; } @media (max-width: 1100px) { .costs-root .costs-summary { grid-template-columns: 1fr 1fr !important; } .costs-root .costs-grid { grid-template-columns: 1fr !important; } @@ -653,6 +660,28 @@ export default function CostsPanel({ tripId, tripMembers = [] }: CostsPanelProps } // ── inline subcomponents (close over helpers) ──────────────────────────── + /** + * Edit and delete, in a capsule beside the row rather than inside it — the + * mobile card has always done it this way, and out here the buttons stop + * competing with the amount for the right edge of the card. + */ + function RowActions({ onEdit, onDelete, deleteLabel }: { onEdit: () => void; onDelete: () => void; deleteLabel: string }) { + // No `background` here on purpose: an inline value would beat the :hover + // rule in the stylesheet block below, which is how the first attempt at + // this silently did nothing. + const btn: React.CSSProperties = { + display: 'inline-flex', alignItems: 'center', justifyContent: 'center', + width: 30, height: 30, borderRadius: 999, border: 0, + cursor: 'pointer', padding: 0, + } + return ( +
+ + +
+ ) + } + function ExpenseRow({ e }: { e: BudgetItem }) { const c = catMeta(e.category) const Icon = c.Icon @@ -660,17 +689,38 @@ export default function CostsPanel({ tripId, tripMembers = [] }: CostsPanelProps const payers = (e.payers || []).filter(p => p.amount > 0) const net = round2(myPaidOf(e) - myShareOf(e)) const unfinished = isUnfinished(e) + const note = readUserNote(e) + const open = expandedNoteId === e.id + + // Fixed columns, identical on every row, so the eye can run down the list + // instead of re-finding each field on each line. A row without a note leaves + // its column empty rather than letting everything after it drift left, which + // is what made the list read as unstructured. + const cols = isMobile ? '46px 1fr auto' : '46px minmax(200px, 1fr) minmax(0, 1.5fr) auto' + return ( -
+
+
+ {/* The category rides the card edge as a tab, the way the mobile card + does, so it stops taking up a slot inside the row itself. */} + {!isMobile && ( + + )} + {isMobile && unfinished && ( ! )} + + {/* What it was. */}
-
- {e.name} +
+ {e.name} {unfinished && !isMobile && ( ! @@ -678,8 +728,15 @@ export default function CostsPanel({ tripId, tripMembers = [] }: CostsPanelProps )}
+ {cur !== base && ( +
+ {fmt(e.total_price, cur)} {'→'} {fmt(baseTotal(e))} +
+ )} + {/* Under the name, because a chip reading "Y 122,00" says nothing on + its own — the name above it is what gives it a meaning. */} {payers.length > 0 && ( -
+
{payers.map(p => ( @@ -688,29 +745,38 @@ export default function CostsPanel({ tripId, tripMembers = [] }: CostsPanelProps ))}
)} - {!isMobile && ( -
- {t(c.labelKey)}{cur !== base ? ` · ${fmt(e.total_price, cur)} → ${fmt(baseTotal(e))}` : ''} -
- )}
+ + {/* The note, marked by a rule in the category colour instead of a box, so + it reads as part of the row rather than something dropped onto it. */} + {!isMobile && ( + note ? ( + + ) : + )} + + {/* The money, and what to do with it. */}
-
-
{fmt(baseTotal(e))}
+
+ {fmt(baseTotal(e))} {!unfinished && (e.members || []).length > 0 && Math.abs(net) > 0.01 && ( -
0 ? '#16a34a' : '#dc2626' }}> + 0 ? 'rgba(22,163,74,0.13)' : 'rgba(220,38,38,0.13)'), fontSize: 'calc(11.5px * var(--fs-scale-caption, 1))', fontWeight: 600, color: net > 0 ? '#16a34a' : '#dc2626' }}> {net > 0 ? t('costs.youLent', { amount: fmt(net) }) : t('costs.youBorrowed', { amount: fmt(-net) })} -
+ )}
- {canEdit && ( -
- - -
- )}
+ {canEdit && { setEditing(e); setModalOpen(true) }} onDelete={() => handleDelete(e.id)} deleteLabel={t('common.delete')} />} +
) } @@ -720,7 +786,8 @@ export default function CostsPanel({ tripId, tripMembers = [] }: CostsPanelProps // Legacy transfers carry no currency and were entered in the display base. const cur = (s.currency || base).toUpperCase() return ( -
+
+
@@ -732,16 +799,20 @@ export default function CostsPanel({ tripId, tripMembers = [] }: CostsPanelProps {personName(s.from_user_id)} → {personName(s.to_user_id)}
+ {/* A payment carries no note, but it keeps the column so its amount lands + on the same axis as every expense above it. */} + {!isMobile && }
-
{fmt(convert(s.amount, cur))}
- {canEdit && ( -
- - -
- )} + {fmt(convert(s.amount, cur))}
+ {canEdit && ( +
+ + +
+ )} +
) } @@ -936,6 +1007,7 @@ export function ExpenseModal({ tripId, base, people, me, editing, prefill, onClo }) { const { t, locale } = useTranslation() const toast = useToast() + const isMobile = useIsMobile() const { addBudgetItem, updateBudgetItem } = useTripStore() const { convert } = useExchangeRates(base) const sym = (c: string) => SYMBOLS[c] || (c + ' ') @@ -944,6 +1016,7 @@ export function ExpenseModal({ tripId, base, people, me, editing, prefill, onClo const [cat, setCat] = useState(editing ? catMeta(editing.category).key : (prefill?.category || 'food')) const [currency, setCurrency] = useState((editing?.currency || base).toUpperCase()) const [day, setDay] = useState(editing?.expense_date || new Date().toISOString().slice(0, 10)) + const [note, setNote] = useState(() => readUserNote(editing)) const [total, setTotal] = useState(() => { if (editing) return editing.total_price ? String(editing.total_price) : '' if (prefill?.amount != null) return String(prefill.amount) @@ -977,7 +1050,7 @@ export function ExpenseModal({ tripId, base, people, me, editing, prefill, onClo const [pinnedPayers, setPinnedPayers] = useState>(() => new Set(initialPayers.map(p => p.user_id))) const [splitMode, setSplitMode] = useState<'equally' | 'custom' | 'ticket'>(() => { - if (editing?.note && editing.note.startsWith('TICKETJSON:')) { + if (hasTicketSplit(editing)) { return 'ticket' } if (editing && editing.members && editing.members.length > 0) { @@ -987,22 +1060,7 @@ export function ExpenseModal({ tripId, base, people, me, editing, prefill, onClo return 'equally' }) - const [ticketItems, setTicketItems] = useState(() => { - if (editing?.note && editing.note.startsWith('TICKETJSON:')) { - try { - const parsed = JSON.parse(editing.note.slice(11)) - return (parsed.items || []).map((item: any) => ({ - id: String(Math.random()), - name: item.name, - price: String(item.price), - participants: new Set(item.parts || []) - })) - } catch { - return [] - } - } - return [] - }) + const [ticketItems, setTicketItems] = useState(() => readTicketItems(editing)) const [customAmounts, setCustomAmounts] = useState>(() => { const m: Record = {} @@ -1191,13 +1249,8 @@ export function ExpenseModal({ tripId, base, people, me, editing, prefill, onClo member_ids: [...participants], expense_date: day || null, total_price: totalNum, - note: splitMode === 'ticket' ? 'TICKETJSON:' + JSON.stringify({ - items: ticketItems.map(item => ({ - name: item.name, - price: item.price, - parts: [...item.participants] - })) - }) : null, + note: note.trim() || null, + ticket_json: splitMode === 'ticket' ? writeTicketItems(ticketItems) : null, ...(!editing && prefill?.reservationId ? { reservation_id: prefill.reservationId } : {}), } try { @@ -1212,17 +1265,25 @@ export function ExpenseModal({ tripId, base, people, me, editing, prefill, onClo } const inputCls = 'w-full bg-surface-input border border-edge text-content' - const labelCls = 'block text-[11px] font-semibold uppercase tracking-[0.08em] text-content-faint mb-[6px]' + const labelCls = 'block text-caption font-semibold uppercase tracking-[0.14em] text-content-faint mb-2' + // Borrowed from the trip dialog (TripFormModal): related fields sit together in + // a panel instead of running as one undifferentiated column of inputs. + const panelCls = 'rounded-2xl border border-edge bg-surface-secondary p-4' return ( -
}> -
+ {/* Two columns on desktop: what the expense was on the left, how it is + shared on the right. The split follows the two questions the dialog + actually asks, so neither side is a leftovers pile. */} +
+
+
setName(e.target.value)} placeholder={t('costs.namePlaceholder')} className={inputCls} style={{ borderRadius: 10, padding: '11px 13px', fontSize: 'calc(14px * var(--fs-scale-body, 1))', outline: 'none' }} /> @@ -1260,7 +1321,9 @@ export function ExpenseModal({ tripId, base, people, me, editing, prefill, onClo
)} -
+
+ +
{COST_CATEGORY_LIST.map(c => { @@ -1277,8 +1340,11 @@ export function ExpenseModal({ tripId, base, people, me, editing, prefill, onClo
-
-
+
+ +
+
+
-
+
- -
+ +
@@ -1491,6 +1557,14 @@ export function ExpenseModal({ tripId, base, people, me, editing, prefill, onClo )}
+ +
+ +