diff --git a/.changeset/charges-record-design-refinement.md b/.changeset/charges-record-design-refinement.md new file mode 100644 index 0000000000..f617fa2035 --- /dev/null +++ b/.changeset/charges-record-design-refinement.md @@ -0,0 +1,52 @@ +--- +'@accounter/client': patch +--- + +Adopt the imported charges-list design refinement: per-type colour chips, a selection accent rail, +one unified card, an anchored expansion panel, and a real drop-target state. + +**Per-type colour chip.** `CHARGE_TYPE_COLOR` in `helpers/charges.tsx` gives each of the eleven +charge types its own hue, rendered by `ChargeTypeBadge` as a tinted, ringed icon chip beside the type +name. Colour is what lets you pick every Salary charge out of a hundred rows without reading, which a +uniformly grey icon could not do. + +Three of the designer's hues were reassigned because they collided with the status vocabulary the +record already uses — amber means "needs attention", emerald "accept / positive amount", red +"error / negative amount", and all three appear on the same row as the chip. Salary moved amber → +purple, Monthly VAT emerald → sky, Dividend rose → pink; the other eight are unchanged. A test +asserts the reservation, so a future palette edit cannot quietly put a status colour back on a type. +The hue class lists are complete literal strings rather than interpolated from the hue name, since +Tailwind only ever sees literals in source. + +**Selection and drop feedback.** A selected record now carries a 2px left accent rail in addition to +its background tint. `DragFile` names its dropzone group so the record can style itself off Mantine's +`data-accept`, lighting the whole row on drag — previously the only sign a drag had registered was the +cursor, over a target with no visible edges. The drop ring is `primary`, not the `ring` token: +`--color-accent` and `--color-muted` hold the same value, so the tint alone was indistinguishable from +the selected background, and a grey ring at 40% did not separate them either. + +**One surface.** Toolbar and record list now share a single bordered, rounded card instead of the +toolbar floating above a separately bordered list; its select-all checkbox in particular looked +disconnected from the rows it governs. The expansion panel is indented under its record behind a rail, +so it reads as belonging to that charge rather than as the next item in the list. + +**Smaller fixes carried from the same design pass.** The CSV export gains a visible label — icon-only, +it had no accessible name at all, since a tooltip is a description rather than a name. Count chips gain +hover hints naming what they count and what is wrong when something is. "Delete Charge" is finally +styled destructive; it was the only irreversible item in the menu and sat in the same weight as "Copy +Charge Link" directly above it. The accountant-status control gains a section label, a dot and a +current-value checkmark per option, an `aria-label` on its trigger (it previously offered a hundred +identically unnamed buttons down a list), a note that Pending means a *downgrade* rather than a step +toward approval, and `dark:` variants it had none of — the VAT and business-trip reports share this +control and inherit all of it. + +Charges-surface neutrals now use the colour tokens activated in the previous release; the amber, +emerald, red status accents and the eleven type hues stay literal, since no token expresses them. The +duplicated "absent value" placeholder, defined identically in two files, is now one shared component. + +Two things in the imported mockup were deliberately **not** adopted, because the shipped code is +better: `role="status"` on the needs badge (a hundred rows would mean a hundred live regions +announcing on every refetch) and a count chip whose state never reaches its accessible name. Its +compact row also dropped the approval control and the date and reflowed the column spans; approval is +the primary triage action and the date the second-most-scanned field, and holding spans stable across +densities means toggling density does not reflow the columns. diff --git a/.changeset/charges-record-list.md b/.changeset/charges-record-list.md new file mode 100644 index 0000000000..18920e7c8d --- /dev/null +++ b/.changeset/charges-record-list.md @@ -0,0 +1,52 @@ +--- +'@accounter/client': minor +--- + +Replace the charges table with a charge record list, driven by a per-type field spec. + +Charges are a union of 11 types with genuinely different attributes, so a shared column set never +fitted them: of 14 display attributes, VAT and business trip each apply to exactly one type, which +left those columns blank on ten rows out of eleven. Rows also measured 166px, because `ListCapsule` +rendered tags and each metadata count as its own bordered box while the select and actions cells +stacked their controls vertically. + +Each charge now renders as a record built from six regions at fixed horizontal positions — manage, +identity, meaning, health, money, actions. Region placement is identical on every record, which keeps +the vertical scanning a table gave you; which fields appear inside a region comes from a declarative +matrix in `charge-fields.ts`, so a record's shape is a pure function of its `__typename`. Records +measure ~85px, or 50px with the new density toggle. + +The list keeps TanStack Table as its headless engine, so sorting, selection (keyed by charge id) and +expansion are unchanged, and every call site keeps the same props. + +Other user-visible changes: + +- Missing info is summarised once per record as a badge, counting only fields that charge type + actually displays, instead of up to six unlabelled red dots scattered across cells. +- Suggested descriptions and tags are offered inline with a one-click accept, replacing a solid + yellow background that read as an error rather than an offer. +- Sorting moved from column headers to a list toolbar bound to the server's `sortBy`, so it orders + every matching charge rather than only the loaded page. Select-all, batch actions and CSV export + moved to the same toolbar, which now announces the selection count. +- A document can be dropped anywhere on a record, not just onto the narrow More Info cell. +- Row density is togglable and remembered. + +Bug fixes: + +- The Date column never sorted. Its accessor returned the whole date object, so TanStack's automatic + sort fell back to comparing `"[object Object]"` against itself and returned the same answer for + every pair, in both directions. +- Lists silently truncated at 100 rows. The rendered row model sits at the end of a pipeline ending + in pagination, so the charges-ledger-validation screen (which streams without a limit) and the + unbounded VAT report sections could never show anything past row 100. +- Nothing inside a charge row was clickable when wrapped for drag-and-drop upload, because Mantine's + dropzone disables pointer events on its content. This went unnoticed while it only wrapped inert + text. +- The header row rendered one more cell than the body, adding a phantom column. +- CSV export could include charges selected in a different table, since the VAT report shares one + selection map across three lists. +- Batch "refresh selected" silently did nothing for selected charges that were not currently + rendered. +- Per-record validation state was conveyed by colour alone with no accessible name; count chips and + the missing-info badge now carry text, and the expand control, which was a nameless chevron, is + labelled. diff --git a/packages/client/src/components/business/charges-section.tsx b/packages/client/src/components/business/charges-section.tsx index 136fb4a30d..7752bfab78 100644 --- a/packages/client/src/components/business/charges-section.tsx +++ b/packages/client/src/components/business/charges-section.tsx @@ -98,13 +98,12 @@ export function ChargesSection({ businessId }: Props) { -
- -
+ {/* No border wrapper: the record list draws its own, and two nested ones read as a seam. */} + ); diff --git a/packages/client/src/components/charges-ledger-validation.tsx b/packages/client/src/components/charges-ledger-validation.tsx index 0b37f048ab..ad9515c57b 100644 --- a/packages/client/src/components/charges-ledger-validation.tsx +++ b/packages/client/src/components/charges-ledger-validation.tsx @@ -4,7 +4,11 @@ import { useQuery } from 'urql'; import { Loader, Progress, ThemeIcon } from '@mantine/core'; import type { RowSelectionState } from '@tanstack/react-table'; import { encodeFilters, ROUTES } from '@/router/routes.js'; -import { ChargesLedgerValidationDocument, type ChargeFilter } from '../gql/graphql.js'; +import { + ChargesLedgerValidationDocument, + type ChargeFilter, + type ChargeSortBy, +} from '../gql/graphql.js'; import { useUrlQuery } from '../hooks/use-url-query.js'; import { FiltersContext } from '../providers/filters-context.js'; import { ChargesFilters } from './charges/charges-filters.js'; @@ -85,6 +89,11 @@ export const ChargesLedgerValidation = (): ReactElement => { [setFilter], ); + // Server-side sort, so it lives on this screen's filter rather than in the table. + const setSortBy = useCallback((sortBy: ChargeSortBy): void => { + setFilter(current => ({ ...current, sortBy })); + }, []); + useEffect(() => { if (filter) { validateLedger(); @@ -166,6 +175,7 @@ export const ChargesLedgerValidation = (): ReactElement => { [] } isAllOpened={isAllOpened} + sort={{ value: filter?.sortBy, onChange: setSortBy }} />
{progress > 0 && progress < 100 && } diff --git a/packages/client/src/components/charges/__tests__/charge-fields.test.ts b/packages/client/src/components/charges/__tests__/charge-fields.test.ts new file mode 100644 index 0000000000..fa931d4c3d --- /dev/null +++ b/packages/client/src/components/charges/__tests__/charge-fields.test.ts @@ -0,0 +1,152 @@ +import { describe, expect, it } from 'vitest'; +import { MissingChargeInfo } from '../../../gql/graphql.js'; +import { CHARGE_TYPE_NAME, type ChargeType } from '../../../helpers/index.js'; +import { + CHARGE_FIELDS, + isFieldVisible, + isMissing, + isSpecialField, + relevantMissingInfo, + visibleFields, + type ChargeField, +} from '../charge-fields.js'; + +const ALL_TYPES = Object.keys(CHARGE_TYPE_NAME) as ChargeType[]; + +/** + * These assertions describe the *shape* of the spec sheet rather than restating it cell by cell, so + * they catch the mistakes a hand-transcribed matrix actually makes: a transposed row, a digit in the + * wrong column, a row of the wrong length. A deliberate spec change will fail them, which is the + * point — it forces a second look at the sheet. + */ +describe('charge field matrix', () => { + it('covers every charge type the client knows about', () => { + expect(ALL_TYPES).toHaveLength(11); + for (const type of ALL_TYPES) { + // A type missing from the matrix would silently fall back to "show everything". + expect(visibleFields(type).length, `${type} has no matrix row`).toBeGreaterThan(0); + } + }); + + it('gives every type a full-width row', () => { + // Rows are positional against CHARGE_FIELDS; a short row would make later fields read as hidden. + for (const type of ALL_TYPES) { + const decided = CHARGE_FIELDS.filter( + field => isFieldVisible(type, field) || !isFieldVisible(type, field), + ); + expect(decided).toHaveLength(CHARGE_FIELDS.length); + } + }); + + it.each(['type', 'mainDate', 'description', 'tags', 'ledgerCount'])( + 'shows %s on all 11 types', + field => { + for (const type of ALL_TYPES) { + expect(isFieldVisible(type, field), `${type}.${field}`).toBe(true); + } + }, + ); + + /** + * The heart of why the record composes fields per type instead of sharing table columns: these two + * attributes apply to exactly one charge type each, so as columns they were blank 10 rows out of 11. + */ + it('shows vat and businessTrip on exactly one type each', () => { + expect(ALL_TYPES.filter(type => isFieldVisible(type, 'vat'))).toEqual(['CommonCharge']); + expect(ALL_TYPES.filter(type => isFieldVisible(type, 'businessTrip'))).toEqual([ + 'BusinessTripCharge', + ]); + }); + + it('hides amount only on FinancialCharge and transactions only on FinancialCharge', () => { + expect(ALL_TYPES.filter(type => !isFieldVisible(type, 'amount'))).toEqual(['FinancialCharge']); + expect(ALL_TYPES.filter(type => !isFieldVisible(type, 'transactionsCount'))).toEqual([ + 'FinancialCharge', + ]); + }); + + it('matches the spec on how many types show each optional field', () => { + const count = (field: ChargeField): number => + ALL_TYPES.filter(type => isFieldVisible(type, field)).length; + + expect({ + dateRange: count('dateRange'), + mainCounterparty: count('mainCounterparty'), + mainTaxCategory: count('mainTaxCategory'), + documentsCount: count('documentsCount'), + miscExpensesCount: count('miscExpensesCount'), + }).toEqual({ + dateRange: 7, + mainCounterparty: 4, + mainTaxCategory: 5, + documentsCount: 5, + miscExpensesCount: 7, + }); + }); + + it('special-cases exactly the two fields the spec footnotes call out', () => { + const specials = ALL_TYPES.flatMap(type => + CHARGE_FIELDS.filter(field => isSpecialField(type, field)).map(field => `${type}.${field}`), + ); + expect(specials.sort()).toEqual([ + 'ConversionCharge.amount', + 'InternalTransferCharge.mainCounterparty', + ]); + }); + + it('treats a special-cased field as visible', () => { + expect(isFieldVisible('ConversionCharge', 'amount')).toBe(true); + expect(isFieldVisible('InternalTransferCharge', 'mainCounterparty')).toBe(true); + }); + + it('shows everything for an unknown charge type rather than rendering an empty record', () => { + const unknown = 'FutureCharge' as ChargeType; + expect(visibleFields(unknown)).toEqual([...CHARGE_FIELDS]); + }); +}); + +describe('relevantMissingInfo', () => { + it('drops missing info the charge type has nowhere to show', () => { + // The server flags a missing counterparty on charges whose spec row hides the field — without + // filtering, the record would advertise a need the user cannot act on. + expect(isFieldVisible('SalaryCharge', 'mainCounterparty')).toBe(false); + expect(relevantMissingInfo('SalaryCharge', [MissingChargeInfo.Counterparty])).toEqual([]); + }); + + it('keeps missing info the charge type does show', () => { + expect(relevantMissingInfo('CommonCharge', [MissingChargeInfo.Counterparty])).toEqual([ + MissingChargeInfo.Counterparty, + ]); + }); + + it('keeps description and tags for every type, since all 11 display them', () => { + for (const type of ALL_TYPES) { + expect( + relevantMissingInfo(type, [MissingChargeInfo.Description, MissingChargeInfo.Tags]), + type, + ).toEqual([MissingChargeInfo.Description, MissingChargeInfo.Tags]); + } + }); + + it('drops VAT for the ten types that do not display it', () => { + for (const type of ALL_TYPES.filter(t => t !== 'CommonCharge')) { + expect(relevantMissingInfo(type, [MissingChargeInfo.Vat]), type).toEqual([]); + } + expect(relevantMissingInfo('CommonCharge', [MissingChargeInfo.Vat])).toEqual([ + MissingChargeInfo.Vat, + ]); + }); + + it('handles an absent validationData', () => { + expect(relevantMissingInfo('CommonCharge', undefined)).toEqual([]); + }); +}); + +describe('isMissing', () => { + it('requires both that the server reported it and that the type displays it', () => { + expect(isMissing('CommonCharge', [MissingChargeInfo.Vat], MissingChargeInfo.Vat)).toBe(true); + expect(isMissing('SalaryCharge', [MissingChargeInfo.Vat], MissingChargeInfo.Vat)).toBe(false); + expect(isMissing('CommonCharge', [], MissingChargeInfo.Vat)).toBe(false); + expect(isMissing('CommonCharge', undefined, MissingChargeInfo.Vat)).toBe(false); + }); +}); diff --git a/packages/client/src/components/charges/__tests__/charge-indicators.test.tsx b/packages/client/src/components/charges/__tests__/charge-indicators.test.tsx new file mode 100644 index 0000000000..2275ad1828 --- /dev/null +++ b/packages/client/src/components/charges/__tests__/charge-indicators.test.tsx @@ -0,0 +1,279 @@ +// @vitest-environment happy-dom + +import React, { act, type ReactElement } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import { describe, expect, it } from 'vitest'; +import { Currency, LedgerValidationStatus, MissingChargeInfo } from '../../../gql/graphql.js'; +import { CHARGE_TYPE_COLOR, type ChargeType } from '../../../helpers/index.js'; +import { + amountState, + AmountText, + ChargeTypeBadge, + chargeTypeChipClass, + CountChip, + ledgerState, + NeedsBadge, + StatusDot, + vatState, +} from '../charge-indicators.js'; + +( + globalThis as typeof globalThis & { + IS_REACT_ACT_ENVIRONMENT?: boolean; + } +).IS_REACT_ACT_ENVIRONMENT = true; + +async function render(element: ReactElement) { + const container = document.createElement('div'); + document.body.append(container); + + let root: Root | null = null; + await act(async () => { + root = createRoot(container); + root.render(element); + await Promise.resolve(); + }); + + const cleanup = async () => { + await act(async () => { + root?.unmount(); + await Promise.resolve(); + }); + container.remove(); + }; + + return { container, cleanup }; +} + +describe('ledgerState', () => { + it('reads an absent (still deferred) value as pending rather than fine', () => { + // `metadata.invalidLedger` arrives in a later @defer patch; treating undefined as VALID would + // flash a healthy ledger before the real answer lands. + expect(ledgerState(undefined)).toBe('pending'); + }); + + it.each([ + [LedgerValidationStatus.Valid, 'ok'], + [LedgerValidationStatus.Diff, 'warning'], + [LedgerValidationStatus.Invalid, 'error'], + ] as const)('maps %s to %s', (status, expected) => { + expect(ledgerState(status)).toBe(expected); + }); +}); + +describe('amountState', () => { + it('is ok for the ten types that do not validate their amount', () => { + expect(amountState(false, undefined)).toBe('ok'); + // Even a stale `false` must not raise an error on a type that never validates. + expect(amountState(false, false)).toBe('ok'); + }); + + it('is pending until CreditcardBankCharge validity arrives, then reflects it', () => { + expect(amountState(true, undefined)).toBe('pending'); + expect(amountState(true, true)).toBe('ok'); + expect(amountState(true, false)).toBe('error'); + }); +}); + +describe('vatState', () => { + const base = { value: 180, currency: Currency.Ils, amountValue: 1180, isMissingInfo: false }; + + it('is ok for a consistent VAT', () => { + expect(vatState(base)).toBe('ok'); + }); + + it('flags a missing VAT on a local-currency charge', () => { + expect(vatState({ ...base, value: undefined })).toBe('error'); + }); + + it('does not flag an absent VAT on a foreign-currency charge', () => { + expect(vatState({ ...base, value: undefined, currency: Currency.Usd })).toBe('ok'); + }); + + it('flags a VAT whose sign disagrees with the charge amount', () => { + expect(vatState({ ...base, value: 180, amountValue: -1180 })).toBe('error'); + expect(vatState({ ...base, value: -180, amountValue: 1180 })).toBe('error'); + }); + + it('flags server-reported missing VAT regardless of the local checks', () => { + expect(vatState({ ...base, isMissingInfo: true })).toBe('error'); + }); +}); + +describe('StatusDot', () => { + it('renders nothing when healthy, so a complete record stays visually quiet', async () => { + const { container, cleanup } = await render(); + expect(container.innerHTML).toBe(''); + await cleanup(); + }); + + it.each(['pending', 'warning', 'error'] as const)('renders a dot for %s', async state => { + const { container, cleanup } = await render(); + expect(container.querySelector('span')).not.toBeNull(); + await cleanup(); + }); +}); + +describe('CountChip', () => { + it('names the count for assistive tech without repeating a state when healthy', async () => { + const { container, cleanup } = await render(); + const chip = container.querySelector('[aria-label]'); + expect(chip?.getAttribute('aria-label')).toBe('ledger 4'); + expect(container.textContent).toContain('4'); + await cleanup(); + }); + + it('puts the state in the accessible name, so it is not conveyed by color alone', async () => { + const { container, cleanup } = await render( + , + ); + expect(container.querySelector('[aria-label]')?.getAttribute('aria-label')).toBe( + 'ledger 4, has differences', + ); + await cleanup(); + }); + + it('still names a zero count', async () => { + const { container, cleanup } = await render( + , + ); + expect(container.querySelector('[aria-label]')?.getAttribute('aria-label')).toBe( + 'documents 0, has issues', + ); + await cleanup(); + }); +}); + +describe('NeedsBadge', () => { + it('renders nothing for a complete charge', async () => { + const { container, cleanup } = await render( + , + ); + expect(container.innerHTML).toBe(''); + await cleanup(); + }); + + it('renders nothing when validationData never arrived', async () => { + const { container, cleanup } = await render( + , + ); + expect(container.innerHTML).toBe(''); + await cleanup(); + }); + + it('counts the missing info and names each item', async () => { + const { container, cleanup } = await render( + , + ); + expect(container.textContent).toContain('2'); + expect(container.querySelector('[aria-label]')?.getAttribute('aria-label')).toBe( + '2 details missing: description, tags', + ); + expect(container.querySelector('[title]')?.getAttribute('title')).toBe( + 'Missing: description, tags', + ); + await cleanup(); + }); + + it('does not count missing info the charge type has nowhere to show', async () => { + // A SalaryCharge hides both counterparty and VAT, so a badge here would point at fields the + // user cannot see, let alone fix. + const { container, cleanup } = await render( + , + ); + expect(container.innerHTML).toBe(''); + await cleanup(); + }); + + it('counts only the displayable subset when the two are mixed', async () => { + const { container, cleanup } = await render( + , + ); + expect(container.querySelector('[aria-label]')?.getAttribute('aria-label')).toBe( + '1 detail missing: description', + ); + await cleanup(); + }); +}); + +describe('AmountText', () => { + it('colors income and expense differently and keeps digits tabular', async () => { + const income = await render(); + const incomeClass = income.container.querySelector('span')?.className ?? ''; + expect(incomeClass).toContain('text-emerald-700'); + expect(incomeClass).toContain('dark:text-emerald-400'); + expect(incomeClass).toContain('tabular-nums'); + await income.cleanup(); + + const expense = await render(); + const expenseClass = expense.container.querySelector('span')?.className ?? ''; + expect(expenseClass).toContain('text-red-600'); + expect(expenseClass).toContain('dark:text-red-400'); + await expense.cleanup(); + }); + + it('formats with the charge currency', async () => { + const { container, cleanup } = await render( + , + ); + expect(container.textContent).toContain('1,180'); + await cleanup(); + }); +}); + +describe('charge type palette', () => { + const ALL_TYPES = Object.keys(CHARGE_TYPE_COLOR) as ChargeType[]; + + it('covers all eleven charge types', () => { + expect(ALL_TYPES).toHaveLength(11); + for (const type of ALL_TYPES) { + expect(chargeTypeChipClass(type), type).toBeTruthy(); + } + }); + + it('gives every type its own hue, so two types are never confusable by colour', () => { + const hues = ALL_TYPES.map(type => CHARGE_TYPE_COLOR[type]); + expect(new Set(hues).size).toBe(hues.length); + }); + + /** + * The reservation this whole palette is built around. Amber means "needs attention", emerald + * "accept / positive", red "error / negative" — all three appear on the same record as the type + * chip, so a type wearing one of them would make that colour stop meaning state. Asserted rather + * than merely commented, because the cost of a future palette edit reintroducing it is that the + * needs badge quietly stops reading as a warning. + */ + it('never spends a status colour on a type', () => { + const RESERVED = ['amber', 'emerald', 'red', 'green', 'yellow', 'rose']; + for (const type of ALL_TYPES) { + expect(RESERVED, type).not.toContain(CHARGE_TYPE_COLOR[type]); + // Also guard the emitted classes, in case a hue name and its classes ever drift apart. + for (const reserved of RESERVED) { + expect(chargeTypeChipClass(type), `${type} chip uses ${reserved}`).not.toContain( + `-${reserved}-`, + ); + } + } + }); + + it('renders the type name alongside the chip, so colour is never the only signal', async () => { + const { container, cleanup } = await render(); + expect(container.textContent).toContain('Salary'); + // The hue is on the chip, and the name is readable independently of it. + expect(container.querySelector('.bg-purple-50')).not.toBeNull(); + await cleanup(); + }); + + it('falls back to a neutral hue for an unrecognised __typename', () => { + expect(chargeTypeChipClass('NotARealCharge' as ChargeType)).toContain('-slate-'); + }); +}); diff --git a/packages/client/src/components/charges/__tests__/charges-table-refetch.test.tsx b/packages/client/src/components/charges/__tests__/charges-table-refetch.test.tsx index d939a4f393..821be3899b 100644 --- a/packages/client/src/components/charges/__tests__/charges-table-refetch.test.tsx +++ b/packages/client/src/components/charges/__tests__/charges-table-refetch.test.tsx @@ -103,9 +103,14 @@ describe('charges table row refetch', () => { }); } - /** The green "confirm suggestion" mini button rendered next to a missing-info cell. */ + /** + * The "accept suggestion" control rendered beside a suggested description or tags. Matched on its + * accessible name rather than a colour class: the record's accept button replaced the old + * `ConfirmMiniButton`, and a name is what the affordance actually promises a user. + */ function confirmButtons(): HTMLElement[] { - return [...container.querySelectorAll('button.text-green-500')]; + // Description reads `Accept suggested description "..."`, tags `Accept N suggested tag(s)`. + return [...container.querySelectorAll('[aria-label^="Accept"]')]; } async function click(element: HTMLElement): Promise { diff --git a/packages/client/src/components/charges/__tests__/charges-toolbar.test.tsx b/packages/client/src/components/charges/__tests__/charges-toolbar.test.tsx new file mode 100644 index 0000000000..fda79a88f3 --- /dev/null +++ b/packages/client/src/components/charges/__tests__/charges-toolbar.test.tsx @@ -0,0 +1,320 @@ +// @vitest-environment happy-dom + +import React, { act, type ReactElement } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import { Provider, createClient } from 'urql'; +import { describe, expect, it, vi } from 'vitest'; +import { useTable, type RowSelectionState, type SortingState } from '@tanstack/react-table'; +import { tableFeaturesConfig } from '@/lib/table-features.js'; +import { + AccountantStatus, + ChargeSortByField, + type ChargeSortBy, +} from '../../../gql/graphql.js'; +import type { ChargeRow } from '../charges-table.js'; +import { ChargesToolbar, SERVER_SORT_OPTIONS } from '../charges-toolbar.js'; +import { CLIENT_SORT_OPTIONS } from '../charges-sort-menu.js'; +import { columns } from '../columns.js'; +import type { ChargeDensity } from '../use-charge-density.js'; + +( + globalThis as typeof globalThis & { + IS_REACT_ACT_ENVIRONMENT?: boolean; + } +).IS_REACT_ACT_ENVIRONMENT = true; + +// The toolbar's children (batch-actions menu, CSV export) hold urql hooks. They never fire during +// these tests but need a client in context to mount, so this one carries no exchanges — nothing can +// reach the network even if an operation were dispatched. +const client = createClient({ url: '/graphql', exchanges: [] }); + +function makeRow(id: string, description: string): ChargeRow { + return { + id, + type: 'CommonCharge', + description, + tags: [], + suggestedTags: [], + counts: { transactions: 0, documents: 0, ledger: 0, miscExpenses: 0 }, + missingInfo: [], + accountantApproval: AccountantStatus.Unapproved, + }; +} + +/** Drives a real table instance so the toolbar is exercised against the same API the screen uses. */ +function Harness({ + rows, + rowSelection, + onRowSelectionChange, + showExport = false, + sort, + sorting = [], + onSortingChange = () => {}, + density = 'comfortable', + onDensityChange = () => {}, +}: { + rows: ChargeRow[]; + rowSelection: RowSelectionState; + onRowSelectionChange: (updater: unknown) => void; + showExport?: boolean; + sort?: { value?: ChargeSortBy | null; onChange: (next: ChargeSortBy) => void }; + sorting?: SortingState; + onSortingChange?: (updater: unknown) => void; + density?: ChargeDensity; + onDensityChange?: (next: ChargeDensity) => void; +}): ReactElement { + const table = useTable({ + features: tableFeaturesConfig, + data: rows, + columns, + getRowId: row => row.id, + enableRowSelection: true, + onRowSelectionChange, + state: { rowSelection }, + }); + return ( + row.id)} + sort={sort} + sorting={sorting} + onSortingChange={onSortingChange as never} + density={density} + onDensityChange={onDensityChange} + /> + ); +} + +async function render(element: ReactElement) { + const container = document.createElement('div'); + document.body.append(container); + + let root: Root | null = null; + await act(async () => { + root = createRoot(container); + root.render({element}); + await Promise.resolve(); + }); + + const cleanup = async () => { + await act(async () => { + root?.unmount(); + await Promise.resolve(); + }); + container.remove(); + }; + + return { container, cleanup }; +} + +const ROWS = [makeRow('c1', 'Cloud spend'), makeRow('c2', 'Office chairs'), makeRow('c3', 'Coffee')]; + +describe('ChargesToolbar', () => { + it('reports the row count when nothing is selected', async () => { + const { container, cleanup } = await render( + {}} />, + ); + + expect(container.textContent).toContain('3 charges'); + await cleanup(); + }); + + it('singularises a one-charge list', async () => { + const { container, cleanup } = await render( + {}} />, + ); + + expect(container.textContent).toContain('1 charge'); + expect(container.textContent).not.toContain('1 charges'); + await cleanup(); + }); + + it('announces the selection, replacing what a bare header checkbox never conveyed', async () => { + const { container, cleanup } = await render( + {}} />, + ); + + const live = container.querySelector('[aria-live="polite"]'); + expect(live?.textContent).toBe('2 of 3 selected'); + await cleanup(); + }); + + it('drives selection through the table when select-all is clicked', async () => { + const onRowSelectionChange = vi.fn(); + const { container, cleanup } = await render( + , + ); + + const checkbox = container.querySelector('[aria-label="Select all charges"]'); + expect(checkbox).not.toBeNull(); + + await act(async () => { + checkbox!.click(); + await Promise.resolve(); + }); + + expect(onRowSelectionChange).toHaveBeenCalled(); + // Selection is keyed by charge id (via `getRowId`), not row index, so it survives sorting. + const updater = onRowSelectionChange.mock.calls[0]![0] as + | RowSelectionState + | ((old: RowSelectionState) => RowSelectionState); + const next = typeof updater === 'function' ? updater({}) : updater; + expect(Object.keys(next).sort()).toEqual(['c1', 'c2', 'c3']); + + await cleanup(); + }); + + it('hides the CSV export unless the screen asks for it', async () => { + // `DownloadCSVButton` is icon-only and keeps its label in a tooltip, so there is no text to + // match — `aria-busy` is the attribute unique to it within this toolbar. + const without = await render( + {}} />, + ); + expect(without.container.querySelector('[aria-busy]')).toBeNull(); + await without.cleanup(); + + const withExport = await render( + {}} showExport />, + ); + expect(withExport.container.querySelector('[aria-busy]')).not.toBeNull(); + await withExport.cleanup(); + }); + + it('shows the server sort field and scopes it to all matching charges', async () => { + const { container, cleanup } = await render( + {}} + sort={{ value: { field: ChargeSortByField.AbsAmount, asc: false }, onChange: () => {} }} + />, + ); + + const trigger = container.querySelector('[aria-label="Sort charges"]'); + expect(trigger).not.toBeNull(); + expect(trigger!.textContent).toContain('Absolute amount'); + await cleanup(); + }); + + it('falls back to the loaded-charges sort when the screen owns no filter', async () => { + const { container, cleanup } = await render( + {}} + sorting={[{ id: 'counterparty', desc: false }]} + />, + ); + + // Reads the label for the explicit column id, not tanstack's derived + // `counterparty_counterparty_name`. + expect( + container.querySelector('[aria-label="Sort charges"]')?.textContent, + ).toContain('Counterparty'); + await cleanup(); + }); + + it('hides the sort control when there is only one charge to order', async () => { + // The single-charge screen renders exactly one record; a sort menu there read "None" and did + // nothing. + const { container, cleanup } = await render( + {}} />, + ); + expect(container.querySelector('[aria-label="Sort charges"]')).toBeNull(); + await cleanup(); + }); + + it('offers exactly the fields the server can sort by, and no others', async () => { + // Guards against the menu drifting from the GraphQL enum — offering a field the server rejects + // would fail at query time, and omitting one silently loses a capability. + expect(SERVER_SORT_OPTIONS.map(option => option.value).sort()).toEqual( + Object.values(ChargeSortByField).sort(), + ); + }); + + it('offers only client-sortable column ids in the fallback menu', async () => { + // Every fallback option must name a real sortable column, or selecting it would set `sorting` + // to an id no column answers to and quietly do nothing — the Date-column bug in another guise. + const sortableIds = columns + .filter(column => column.enableSorting !== false) + .map(column => column.id); + for (const option of CLIENT_SORT_OPTIONS) { + expect(sortableIds, option.value).toContain(option.value); + } + }); + + it('never renders both bindings at once, so the two sorts cannot fight', async () => { + const { container, cleanup } = await render( + {}} + sort={{ value: { field: ChargeSortByField.Date, asc: true }, onChange: () => {} }} + sorting={[{ id: 'amount', desc: true }]} + />, + ); + + expect(container.querySelectorAll('[aria-label="Sort charges"]')).toHaveLength(1); + // The server binding wins; the stale client `sorting` is not surfaced. + const label = container.querySelector('[aria-label="Sort charges"]')?.textContent ?? ''; + expect(label).toContain('Date'); + expect(label).not.toContain('Amount'); + await cleanup(); + }); + + it('toggles density, and reports the current mode as a pressed state', async () => { + const onDensityChange = vi.fn(); + const { container, cleanup } = await render( + {}} + density="comfortable" + onDensityChange={onDensityChange} + />, + ); + + const toggle = container.querySelector('[aria-label="Compact density"]'); + expect(toggle).not.toBeNull(); + expect(toggle!.getAttribute('aria-pressed')).toBe('false'); + + await act(async () => { + toggle!.click(); + await Promise.resolve(); + }); + expect(onDensityChange).toHaveBeenCalledWith('compact'); + await cleanup(); + }); + + it('offers the way back out of compact', async () => { + const onDensityChange = vi.fn(); + const { container, cleanup } = await render( + {}} + density="compact" + onDensityChange={onDensityChange} + />, + ); + + const toggle = container.querySelector('[aria-label="Comfortable density"]'); + expect(toggle!.getAttribute('aria-pressed')).toBe('true'); + await act(async () => { + toggle!.click(); + await Promise.resolve(); + }); + expect(onDensityChange).toHaveBeenCalledWith('comfortable'); + await cleanup(); + }); + + it('exposes the batch-actions menu', async () => { + const { container, cleanup } = await render( + {}} />, + ); + + expect(container.querySelector('[aria-label="Batch charge actions"]')).not.toBeNull(); + await cleanup(); + }); +}); diff --git a/packages/client/src/components/charges/__tests__/columns.test.tsx b/packages/client/src/components/charges/__tests__/columns.test.tsx new file mode 100644 index 0000000000..b666e51306 --- /dev/null +++ b/packages/client/src/components/charges/__tests__/columns.test.tsx @@ -0,0 +1,120 @@ +// @vitest-environment happy-dom + +import React, { act, type ReactElement } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import { describe, expect, it } from 'vitest'; +import { useTable, type SortingState } from '@tanstack/react-table'; +import { tableFeaturesConfig } from '@/lib/table-features.js'; +import { AccountantStatus, Currency } from '../../../gql/graphql.js'; +import type { ChargeRow } from '../charges-table.js'; +import { columns } from '../columns.js'; + +( + globalThis as typeof globalThis & { + IS_REACT_ACT_ENVIRONMENT?: boolean; + } +).IS_REACT_ACT_ENVIRONMENT = true; + +function makeRow(id: string, date: string | undefined, amount: number | undefined): ChargeRow { + return { + id, + type: 'CommonCharge', + dates: date ? { date: new Date(date) } : undefined, + amount: + amount === undefined + ? undefined + : { value: amount, currency: Currency.Ils, shouldValidate: false }, + description: `charge ${id}`, + tags: [], + suggestedTags: [], + counts: { transactions: 0, documents: 0, ledger: 0, miscExpenses: 0 }, + missingInfo: [], + accountantApproval: AccountantStatus.Unapproved, + }; +} + +/** Reports the id order the table's sorted row model produces, so we assert on real sort output. */ +function SortProbe({ + rows, + sorting, + onOrder, +}: { + rows: ChargeRow[]; + sorting: SortingState; + onOrder: (ids: string[]) => void; +}): ReactElement { + const table = useTable({ + features: tableFeaturesConfig, + data: rows, + columns, + getRowId: row => row.id, + state: { sorting }, + }); + onOrder(table.getRowModel().rows.map(row => row.original.id)); + return
; +} + +async function sortedIds(rows: ChargeRow[], sorting: SortingState): Promise { + const container = document.createElement('div'); + document.body.append(container); + let ids: string[] = []; + let root: Root | null = null; + await act(async () => { + root = createRoot(container); + root.render( (ids = next)} />); + await Promise.resolve(); + }); + await act(async () => { + root?.unmount(); + await Promise.resolve(); + }); + container.remove(); + return ids; +} + +// Deliberately not in date order, so a no-op sort is distinguishable from a working one. +const ROWS = [ + makeRow('mar', '2026-03-15', -100), + makeRow('jan', '2026-01-05', -3000), + makeRow('feb', '2026-02-20', 250), +]; + +describe('date column sorting', () => { + /** + * Regression test for a sort that silently did nothing. `accessorKey: 'date'` yielded the whole + * `DateProps` object; tanstack's `auto` sort saw neither a Date nor a string, fell back to + * `sortFn_basic`, and compared `"[object Object]" > "[object Object]"` — returning -1 for every + * pair, so the input order survived untouched. Against these rows that looked like + * ['mar','jan','feb'] for both directions. + */ + it('orders ascending by date', async () => { + expect(await sortedIds(ROWS, [{ id: 'date', desc: false }])).toEqual(['jan', 'feb', 'mar']); + }); + + it('orders descending by date', async () => { + expect(await sortedIds(ROWS, [{ id: 'date', desc: true }])).toEqual(['mar', 'feb', 'jan']); + }); + + it('actually reorders — the unsorted order is not already correct', async () => { + expect(await sortedIds(ROWS, [])).toEqual(['mar', 'jan', 'feb']); + }); + + it('keeps charges without a date from breaking the order', async () => { + const withUndated = [...ROWS, makeRow('none', undefined, -5)]; + const ids = await sortedIds(withUndated, [{ id: 'date', desc: false }]); + expect(ids).toHaveLength(4); + // The three dated charges keep their relative order regardless of where the undated one lands. + expect(ids.filter(id => id !== 'none')).toEqual(['jan', 'feb', 'mar']); + }); +}); + +describe('amount column sorting', () => { + it('orders by magnitude, ignoring income/expense sign', async () => { + // -100, 250, -3000 → 100, 250, 3000 + expect(await sortedIds(ROWS, [{ id: 'amount', desc: false }])).toEqual([ + 'mar', + 'feb', + 'jan', + ]); + }); +}); diff --git a/packages/client/src/components/charges/__tests__/pagination.test.tsx b/packages/client/src/components/charges/__tests__/pagination.test.tsx new file mode 100644 index 0000000000..ca9e02cce6 --- /dev/null +++ b/packages/client/src/components/charges/__tests__/pagination.test.tsx @@ -0,0 +1,75 @@ +// @vitest-environment happy-dom + +import React, { act, type ReactElement } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import { describe, expect, it } from 'vitest'; +import { useTable } from '@tanstack/react-table'; +import { tableFeaturesConfig } from '@/lib/table-features.js'; +import { AccountantStatus } from '../../../gql/graphql.js'; +import type { ChargeRow } from '../charges-table.js'; +import { columns } from '../columns.js'; + +( + globalThis as typeof globalThis & { IS_REACT_ACT_ENVIRONMENT?: boolean } +).IS_REACT_ACT_ENVIRONMENT = true; + +function makeRow(id: string): ChargeRow { + return { + id, + type: 'CommonCharge', + tags: [], + suggestedTags: [], + counts: { transactions: 0, documents: 0, ledger: 0, miscExpenses: 0 }, + missingInfo: [], + accountantApproval: AccountantStatus.Unapproved, + }; +} + +/** Mirrors the pagination options `ChargesTable` passes, so the rendered row count is asserted. */ +function Probe({ rows, onCount }: { rows: ChargeRow[]; onCount: (n: number) => void }): ReactElement { + const table = useTable({ + features: tableFeaturesConfig, + data: rows, + columns, + getRowId: row => row.id, + initialState: { pagination: { pageIndex: 0, pageSize: Number.MAX_SAFE_INTEGER } }, + }); + onCount(table.getRowModel().rows.length); + return
; +} + +async function renderedRowCount(rows: ChargeRow[]): Promise { + const container = document.createElement('div'); + document.body.append(container); + let count = -1; + let root: Root | null = null; + await act(async () => { + root = createRoot(container); + root.render( (count = n)} />); + await Promise.resolve(); + }); + await act(async () => { + root?.unmount(); + await Promise.resolve(); + }); + container.remove(); + return count; +} + +describe('charges list pagination', () => { + /** + * `rowPaginationFeature` is registered in the shared feature set and `getRowModel()` ends in + * pagination, so the list silently truncated: at the previous `pageSize: 100`, the two screens that + * fetch without a limit (ledger validation streams; the VAT report sections are unbounded) could + * never show anything past row 100. Leaving `pageSize` unset would be worse — tanstack defaults to 10. + */ + it('renders every charge it is handed, past the old 100-row ceiling', async () => { + const rows = Array.from({ length: 250 }, (_, i) => makeRow(`c${i}`)); + expect(await renderedRowCount(rows)).toBe(250); + }); + + it('renders an exactly-100 list unchanged', async () => { + const rows = Array.from({ length: 100 }, (_, i) => makeRow(`c${i}`)); + expect(await renderedRowCount(rows)).toBe(100); + }); +}); diff --git a/packages/client/src/components/charges/cells/amount.tsx b/packages/client/src/components/charges/cells/amount.tsx deleted file mode 100644 index fba53d1233..0000000000 --- a/packages/client/src/components/charges/cells/amount.tsx +++ /dev/null @@ -1,38 +0,0 @@ -import { type ReactElement } from 'react'; -import { Indicator } from '@mantine/core'; -import { Currency } from '../../../gql/graphql.js'; -import { formatAmountWithCurrency } from '../../../helpers/index.js'; - -export type AmountProps = { - amount?: { - value: number; - currency: Currency; - shouldValidate: boolean; - isValid?: boolean; - }; -}; - -export const Amount = ({ amount }: AmountProps): ReactElement | null => { - if (!amount) { - return null; - } - const { value, currency, shouldValidate, isValid } = amount; - return ( - -

0 ? 'whitespace-nowrap text-green-700' : 'whitespace-nowrap text-red-500' - } - > - {formatAmountWithCurrency(value, currency)} -

- - ); -}; diff --git a/packages/client/src/components/charges/cells/business-trip.tsx b/packages/client/src/components/charges/cells/business-trip.tsx deleted file mode 100644 index 0b47fa6951..0000000000 --- a/packages/client/src/components/charges/cells/business-trip.tsx +++ /dev/null @@ -1,22 +0,0 @@ -import { type ReactElement } from 'react'; -import { Link } from 'react-router-dom'; -import { ROUTES } from '@/router/routes.js'; - -export type BusinessTripProps = { - id: string; - name: string; -}; - -export const BusinessTrip = ({ id, name }: BusinessTripProps): ReactElement => { - return ( - event.stopPropagation()} - className="inline-flex items-center font-semibold" - > - {name} - - ); -}; diff --git a/packages/client/src/components/charges/cells/counterparty.tsx b/packages/client/src/components/charges/cells/counterparty.tsx deleted file mode 100644 index d6078f6493..0000000000 --- a/packages/client/src/components/charges/cells/counterparty.tsx +++ /dev/null @@ -1,45 +0,0 @@ -import { type ReactElement } from 'react'; -import { Link } from 'react-router-dom'; -import { Indicator } from '@mantine/core'; -import { ROUTES } from '@/router/routes.js'; -import type { ChargeType } from '../../../helpers/index.js'; -import { shouldHaveCounterparty } from '../utils.js'; - -export type CounterpartyProps = { - counterparty: - | { - name: string; - id: string; - } - | undefined; - type: ChargeType; - isMissing?: boolean; -}; - -export const Counterparty = ({ - counterparty, - type, - isMissing, -}: CounterpartyProps): ReactElement => { - const isError = shouldHaveCounterparty(type) && !!isMissing; - const { name, id } = counterparty ?? { name: 'Missing', id: undefined }; - - return ( -
- - {!isError && id && ( - event.stopPropagation()} - className="inline-flex items-center font-semibold" - > - {name} - - )} - {isError && name} - -
- ); -}; diff --git a/packages/client/src/components/charges/cells/description.tsx b/packages/client/src/components/charges/cells/description.tsx deleted file mode 100644 index ba4e19c03b..0000000000 --- a/packages/client/src/components/charges/cells/description.tsx +++ /dev/null @@ -1,88 +0,0 @@ -import { useCallback, useMemo, useState, type ReactElement } from 'react'; -import { Indicator } from '@mantine/core'; -import { useUpdateCharge } from '../../../hooks/use-update-charge.js'; -import { ConfirmMiniButton, SimilarChargesByIdModal } from '../../common/index.js'; - -export type DescriptionProps = { - chargeId: string; - value?: string; - isMissing?: boolean; - suggestedDescription?: string; - onChange: () => void; -}; - -export const Description = ({ - chargeId, - value, - isMissing, - suggestedDescription, - onChange, -}: DescriptionProps): ReactElement => { - const [similarChargesOpen, setSimilarChargesOpen] = useState(false); - // The description the similar-charges follow-up compares against, captured when it was applied. - // Reading it off the props instead would make the criteria vanish the moment `onChange` below - // refreshes the row (the suggestion is gone once accepted), closing the dialog immediately. - const [appliedDescription, setAppliedDescription] = useState(undefined); - const { updateCharge, fetching } = useUpdateCharge(); - - const updateUserDescription = useCallback( - async (value?: string) => { - if (value !== undefined) { - const updated = await updateCharge({ - chargeId, - fields: { userDescription: value }, - }); - if (!updated) { - return; - } - // Refresh the row on the mutation itself. Hanging it off the follow-up dialog's close made - // the update invisible whenever that dialog resolved without closing. - onChange(); - setAppliedDescription(value); - setSimilarChargesOpen(true); - } - }, - [chargeId, updateCharge, onChange], - ); - - const cellText = useMemo(() => { - if (value && value !== '') { - return value; - } - if (suggestedDescription) { - return suggestedDescription; - } - return 'Missing'; - }, [value, suggestedDescription]); - - const hasAlternative = useMemo( - () => isMissing && !!suggestedDescription?.length, - [isMissing, suggestedDescription], - ); - - return ( - <> -
- -

{cellText}

- - {hasAlternative && ( - { - event.stopPropagation(); - updateUserDescription(suggestedDescription); - }} - disabled={fetching} - /> - )} -
- - - - ); -}; diff --git a/packages/client/src/components/charges/cells/index.ts b/packages/client/src/components/charges/cells/index.ts deleted file mode 100644 index 5ffa210117..0000000000 --- a/packages/client/src/components/charges/cells/index.ts +++ /dev/null @@ -1,10 +0,0 @@ -export { Amount } from './amount.js'; -export { BusinessTrip } from './business-trip.js'; -export { Counterparty } from './counterparty.js'; -export { DateCell } from './date.js'; -export { Description } from './description.js'; -export { MoreInfo } from './more-info.js'; -export { Tags } from './tags.js'; -export { TaxCategory } from './tax-category.js'; -export { TypeCell } from './type.js'; -export { Vat } from './vat.js'; diff --git a/packages/client/src/components/charges/cells/more-info.tsx b/packages/client/src/components/charges/cells/more-info.tsx deleted file mode 100644 index 44cef7b822..0000000000 --- a/packages/client/src/components/charges/cells/more-info.tsx +++ /dev/null @@ -1,140 +0,0 @@ -import { useMemo, type ReactElement } from 'react'; -import { Indicator } from '@mantine/core'; -import type { ChargeType } from '../../../helpers/index.js'; -import { DragFile, ListCapsule } from '../../common/index.js'; - -export type MoreInfoProps = { - chargeId: string; - type: ChargeType; - isTransactionsMissing?: boolean; - isDocumentsMissing?: boolean; - info?: { - transactionsCount: number; - documentsCount: number; - ledgerCount: number; - miscExpensesCount: number; - invalidLedger?: 'VALID' | 'DIFF' | 'INVALID'; - }; -}; - -export const MoreInfo = ({ - chargeId, - type, - isTransactionsMissing, - isDocumentsMissing, - info, -}: MoreInfoProps): ReactElement => { - const shouldHaveDocuments = useMemo((): boolean => { - switch (type) { - case 'BusinessTripCharge': - case 'ConversionCharge': - case 'DividendCharge': - case 'InternalTransferCharge': - case 'SalaryCharge': - case 'MonthlyVatCharge': - case 'BankDepositCharge': - case 'ForeignSecuritiesCharge': - case 'CreditcardBankCharge': - case 'FinancialCharge': - return false; - default: - return true; - } - }, [type]); - - const shouldHaveTransactions = useMemo((): boolean => { - switch (type) { - case 'FinancialCharge': - return false; - default: - return true; - } - }, [type]); - - const isTransactionsError = useMemo( - () => shouldHaveTransactions && isTransactionsMissing, - [shouldHaveTransactions, isTransactionsMissing], - ); - - const isDocumentsError = useMemo( - () => shouldHaveDocuments && isDocumentsMissing, - [shouldHaveDocuments, isDocumentsMissing], - ); - - const ledgerStatus = useMemo(() => info?.invalidLedger, [info?.invalidLedger]); - - const list: ( - | React.ReactNode - | { - content: React.ReactNode; - extraClassName?: string; - } - )[] = []; - - if (isTransactionsError || info?.transactionsCount || shouldHaveTransactions) { - list.push({ - extraClassName: - info?.transactionsCount || !shouldHaveTransactions ? undefined : 'bg-yellow-400', - content: ( - -
Transactions: {info?.transactionsCount ?? 0}
- - ), - }); - } - - list.push({ - content: ( - -
Ledger Records: {info?.ledgerCount ?? 0}
- - ), - }); - - if (isDocumentsError || info?.documentsCount) { - list.push({ - content: ( - -
Documents: {info?.documentsCount ?? 0}
- - ), - extraClassName: !isDocumentsMissing || !shouldHaveDocuments ? undefined : 'bg-yellow-400', - }); - } - - if (info?.miscExpensesCount) { - list.push({ - content: ( -
Misc Expenses: {info.miscExpensesCount ?? 0}
- ), - }); - } - - return ( - - - - ); -}; diff --git a/packages/client/src/components/charges/cells/tags.tsx b/packages/client/src/components/charges/cells/tags.tsx deleted file mode 100644 index 47a24c13a0..0000000000 --- a/packages/client/src/components/charges/cells/tags.tsx +++ /dev/null @@ -1,89 +0,0 @@ -import { useCallback, useState, type ReactElement } from 'react'; -import { Group, Indicator, Text } from '@mantine/core'; -import { useUpdateCharge } from '../../../hooks/use-update-charge.js'; -import { ConfirmMiniButton, ListCapsule, SimilarChargesByIdModal } from '../../common/index.js'; - -export type TagsProps = { - chargeId: string; - tags: { id: string; name: string; namePath?: string[] }[]; - suggestedTags: { id: string; name: string; namePath?: string[] }[]; - isMissing?: boolean; - onChange: () => void; -}; - -export const Tags = ({ - chargeId, - tags: originalTags, - suggestedTags, - isMissing, - onChange, -}: TagsProps): ReactElement => { - const { updateCharge, fetching } = useUpdateCharge(); - - const [similarChargesOpen, setSimilarChargesOpen] = useState(false); - // The tags the similar-charges follow-up compares against, captured when they were applied. - // Reading them off the props instead would make the criteria vanish the moment `onChange` below - // refreshes the row (the suggestion is gone once accepted), closing the dialog immediately. - const [appliedTagIds, setAppliedTagIds] = useState<{ id: string }[] | undefined>(undefined); - - const hasAlternative = isMissing && !!suggestedTags?.length; - - const tags = originalTags?.length ? originalTags : hasAlternative ? suggestedTags : []; - - const updateTag = useCallback( - async (tags?: Array<{ id: string }>) => { - const appliedTags = tags?.map(t => ({ id: t.id })); - const updated = await updateCharge({ - chargeId, - fields: { tags: appliedTags }, - }); - if (!updated) { - return; - } - // Refresh the row on the mutation itself. Hanging it off the follow-up dialog's close made - // the update invisible whenever that dialog resolved without closing. - onChange(); - setAppliedTagIds(appliedTags); - setSimilarChargesOpen(true); - }, - [chargeId, updateCharge, onChange], - ); - - return ( - <> - - ( - -
- {t.namePath && ( - - {`${t.namePath.join(' > ')} >`} - - )} - {t.name} -
- - ))} - extraClassName={hasAlternative ? 'bg-yellow-400' : undefined} - /> - - {hasAlternative && ( - { - event.stopPropagation(); - updateTag(suggestedTags); - }} - disabled={fetching} - /> - )} - - - - ); -}; diff --git a/packages/client/src/components/charges/cells/tax-category.tsx b/packages/client/src/components/charges/cells/tax-category.tsx deleted file mode 100644 index c748a90018..0000000000 --- a/packages/client/src/components/charges/cells/tax-category.tsx +++ /dev/null @@ -1,18 +0,0 @@ -import { type ReactElement } from 'react'; -import { Indicator } from '@mantine/core'; - -export type TaxCategoryProps = { - taxCategory?: { - id: string; - name: string; - }; - isMissing?: boolean; -}; - -export const TaxCategory = ({ taxCategory, isMissing }: TaxCategoryProps): ReactElement => { - return ( - - {taxCategory?.name ?? 'N/A'} - - ); -}; diff --git a/packages/client/src/components/charges/cells/type.tsx b/packages/client/src/components/charges/cells/type.tsx deleted file mode 100644 index 9523532fcc..0000000000 --- a/packages/client/src/components/charges/cells/type.tsx +++ /dev/null @@ -1,28 +0,0 @@ -import { useMemo, type ReactElement } from 'react'; -import { ThemeIcon } from '@mantine/core'; -import { getChargeTypeIcon, getChargeTypeName, type ChargeType } from '../../../helpers/index.js'; -import { Tooltip } from '../../common/index.js'; - -type Props = { - type: ChargeType; -}; - -export const TypeCell = ({ type }: Props): ReactElement => { - const { text, icon } = useMemo( - (): { - text: string; - icon: ReactElement; - } => ({ - text: getChargeTypeName(type), - icon: getChargeTypeIcon(type), - }), - [type], - ); - return ( - - - {icon} - - - ); -}; diff --git a/packages/client/src/components/charges/cells/vat.tsx b/packages/client/src/components/charges/cells/vat.tsx deleted file mode 100644 index 9f013ca3ca..0000000000 --- a/packages/client/src/components/charges/cells/vat.tsx +++ /dev/null @@ -1,28 +0,0 @@ -import { type ReactElement } from 'react'; -import { Indicator } from '@mantine/core'; -import { Currency } from '../../../gql/graphql.js'; -import { formatAmountWithCurrency } from '../../../helpers/index.js'; - -export type VatProps = { - value?: number; - currency?: Currency; - missingInfo?: boolean; - amountValue?: number; -}; - -export const Vat = ({ value, currency, amountValue, missingInfo }: VatProps): ReactElement => { - const isLocalCurrencyButNoVat = value == null && currency === Currency.Ils; - const vatIsNegativeToAmount = - ((value ?? 0) > 0 && (amountValue ?? 0) < 0) || ((value ?? 0) < 0 && (amountValue ?? 0) > 0); - const isError = isLocalCurrencyButNoVat || vatIsNegativeToAmount; - - return ( -
- - {value != null && currency ? formatAmountWithCurrency(value, currency) : null} - -
- ); -}; diff --git a/packages/client/src/components/charges/charge-actions-menu.tsx b/packages/client/src/components/charges/charge-actions-menu.tsx index 26d4861397..d4eaa98a3e 100644 --- a/packages/client/src/components/charges/charge-actions-menu.tsx +++ b/packages/client/src/components/charges/charge-actions-menu.tsx @@ -111,7 +111,9 @@ export function ChargeActionsMenu({ Copy Charge Link - setConfirmDeleteOpen(true)}> + {/* The only irreversible item in the menu, and it sat in the same weight as "Copy Charge + Link" directly above it. */} + setConfirmDeleteOpen(true)}> Delete Charge diff --git a/packages/client/src/components/charges/cells/date.tsx b/packages/client/src/components/charges/charge-dates.ts similarity index 57% rename from packages/client/src/components/charges/cells/date.tsx rename to packages/client/src/components/charges/charge-dates.ts index cd4476a793..be59806abe 100644 --- a/packages/client/src/components/charges/cells/date.tsx +++ b/packages/client/src/components/charges/charge-dates.ts @@ -1,7 +1,19 @@ -import { type ReactElement } from 'react'; -import { format } from 'date-fns'; +/** + * A charge has three date families — documents, events and debits — each with a min and a max. The + * record shows one primary date plus, when the charge actually spans a range, the outer bounds. + * + * Extracted from the old `cells/date.tsx` so the logic survives the presentational cell's removal. + */ +export type ChargeDates = { + /** The date shown as the charge's own. */ + date?: Date; + /** Earliest date across every source, for the range line. */ + mostMinDate?: Date; + /** Latest date across every source, for the range line. */ + mostMaxDate?: Date; +}; -export function getDateProps({ +export function getChargeDates({ minDebitDate, minEventDate, minDocumentsDate, @@ -17,7 +29,7 @@ export function getDateProps({ maxDebitDate: string | Date | null; maxEventDate: string | Date | null; maxDocumentsDate: string | Date | null; -}): DateProps | undefined { +}): ChargeDates | undefined { if (!minDocumentsDate && !minEventDate && !minDebitDate) { return undefined; } @@ -30,6 +42,7 @@ export function getDateProps({ const mostMinDate = minTimestamps.length > 0 ? new Date(Math.min(...minTimestamps)) : undefined; const mostMaxDate = maxTimestamps.length > 0 ? new Date(Math.max(...maxTimestamps)) : undefined; + // Documents date wins, then event, then debit — the charge's most meaningful date first. const displayDate = minDocumentsDate || minEventDate || minDebitDate; return { @@ -39,21 +52,11 @@ export function getDateProps({ }; } -export type DateProps = { - date?: Date; - mostMinDate?: Date; - mostMaxDate?: Date; -}; - -export const DateCell = ({ date, mostMinDate, mostMaxDate }: DateProps): ReactElement => { +/** Whether the charge spans more than a single day, and so has a range worth showing. */ +export function hasDateRange(dates: ChargeDates | undefined): boolean { return ( - <> -
{date && format(date, 'dd/MM/yy')}
- {mostMinDate && mostMaxDate && mostMinDate.getTime() !== mostMaxDate.getTime() ? ( -
- ({format(mostMinDate, 'dd/MM/yy')} - {format(mostMaxDate, 'dd/MM/yy')}) -
- ) : null} - + !!dates?.mostMinDate && + !!dates.mostMaxDate && + dates.mostMinDate.getTime() !== dates.mostMaxDate.getTime() ); -}; +} diff --git a/packages/client/src/components/charges/charge-fields.stories.tsx b/packages/client/src/components/charges/charge-fields.stories.tsx new file mode 100644 index 0000000000..0d96630b86 --- /dev/null +++ b/packages/client/src/components/charges/charge-fields.stories.tsx @@ -0,0 +1,142 @@ +import type { ReactElement } from 'react'; +import type { Meta, StoryObj } from '@storybook/react-vite'; +import { CHARGE_TYPE_NAME, getChargeTypeIcon, type ChargeType } from '../../helpers/index.js'; +import { + CHARGE_FIELD_LABEL, + CHARGE_FIELDS, + isFieldVisible, + isSpecialField, + visibleFields, +} from './charge-fields.js'; + +/** Row order copied from the spec spreadsheet, so the two can be scanned side by side. */ +const SHEET_ORDER: ChargeType[] = [ + 'BankDepositCharge', + 'BusinessTripCharge', + 'CommonCharge', + 'ConversionCharge', + 'CreditcardBankCharge', + 'DividendCharge', + 'FinancialCharge', + 'ForeignSecuritiesCharge', + 'InternalTransferCharge', + 'MonthlyVatCharge', + 'SalaryCharge', +]; + +/** + * A read-back of the spec matrix, laid out the same way as the source spreadsheet so the two can be + * compared cell by cell. This is genuinely tabular reference data with a uniform attribute set, so + * unlike the charge record itself, a table is the right shape for it. + */ +function SpecMatrix(): ReactElement { + return ( +
+
+

Charge field spec — as transcribed

+

+ Compare against the source sheet. ✓ shown · · hidden ·{' '} + ★ shown, special-cased. +

+
+ +
+ + + + + {CHARGE_FIELDS.map(field => ( + + ))} + + + + + {SHEET_ORDER.map(type => ( + + + {CHARGE_FIELDS.map(field => { + const special = isSpecialField(type, field); + const shown = isFieldVisible(type, field); + return ( + + ); + })} + + + ))} + +
+ type + + {/* Plain horizontal headings, narrow enough to wrap onto two lines. The table + scrolls inside its own container when 14 of them exceed the viewport. */} + + {CHARGE_FIELD_LABEL[field]} + + + shown +
+ + {getChargeTypeIcon(type)} + {CHARGE_TYPE_NAME[type]} + + + {special ? '★' : shown ? '✓' : '·'} + + {visibleFields(type).length} +
+
+ +
+

Special cases

+
    +
  • + Conversion · amount — show the base and quote amounts, not one total. +
    • +
  • + Internal transfer · main counterparty — show both sides of the + transfer. +
    • +
  • + Charge management — selection, accountant-approval status and button, + expansion button, charge menu. Universal across all 11 types, so not a column here. +
    • +
+
+ +
+

Why the record composes fields per type

+

+ VAT and business trip each apply to exactly one of + eleven types — as table columns they were blank on ten rows out of eleven. Counterparty + applies to four, tax category to five. +

+
+
+ ); +} + +const meta = { + title: 'Charges/Spec Matrix', + component: SpecMatrix, + parameters: { layout: 'fullscreen' }, +} satisfies Meta; + +export default meta; +type Story = StoryObj; + +export const Default: Story = {}; diff --git a/packages/client/src/components/charges/charge-fields.ts b/packages/client/src/components/charges/charge-fields.ts new file mode 100644 index 0000000000..adb0b71623 --- /dev/null +++ b/packages/client/src/components/charges/charge-fields.ts @@ -0,0 +1,151 @@ +import { MissingChargeInfo } from '../../gql/graphql.js'; +import type { ChargeType } from '../../helpers/index.js'; + +/** + * Display attributes of a collapsed charge record, in the same order as the columns of the spec + * spreadsheet. `MATRIX` rows below are positional against this list, so the two can be read + * side by side against the sheet. + * + * "Charge management" from the spec — selection checkbox, accountant-approval status button, + * expansion toggle, charge menu — is universal across all 11 types and so is deliberately not a + * field here. It is always rendered. + */ +export const CHARGE_FIELDS = [ + 'type', + 'mainDate', + 'dateRange', + 'amount', + 'vat', + 'mainCounterparty', + 'description', + 'tags', + 'mainTaxCategory', + 'businessTrip', + 'transactionsCount', + 'documentsCount', + 'miscExpensesCount', + 'ledgerCount', +] as const; + +export type ChargeField = (typeof CHARGE_FIELDS)[number]; + +/** Human wording for each attribute, matching the spec sheet's column headings. */ +export const CHARGE_FIELD_LABEL: Record = { + type: 'type', + mainDate: 'main date', + dateRange: 'date range', + amount: 'amount', + vat: 'VAT', + mainCounterparty: 'main counterparty', + description: 'description', + tags: 'tags', + mainTaxCategory: 'main tax category', + businessTrip: 'business trip', + transactionsCount: 'transactions', + documentsCount: 'documents', + miscExpensesCount: 'misc expenses', + ledgerCount: 'ledger', +}; + +/** + * Per-field visibility: + * - `0` — never rendered for this charge type, *regardless of what the data holds*. The matrix is + * authoritative, so a record's shape is a pure function of its `__typename` and is directly + * assertable in a test. + * - `1` — rendered. + * - `2` — rendered, special-cased. See {@link isSpecialField}. + */ +type Visibility = 0 | 1 | 2; + +/** + * The spec matrix. One row per charge type, positional against {@link CHARGE_FIELDS}, mirroring the + * spec spreadsheet 1:1 so a cell can be diffed against it by eye. + * + * Note how little of this a table could express: `vat` and `businessTrip` each apply to exactly one + * of eleven types, which is why the record composes fields per type rather than sharing columns. + */ +// prettier-ignore +const MATRIX: Record = { + // ty md dr am vat cp de tg tax bt tx doc me ldg + BankDepositCharge: [ 1, 1, 0, 1, 0, 0, 1, 1, 0, 0, 1, 0, 0, 1 ], + BusinessTripCharge: [ 1, 1, 1, 1, 0, 0, 1, 1, 1, 1, 1, 1, 1, 1 ], + CommonCharge: [ 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 1, 1, 1, 1 ], + ConversionCharge: [ 1, 1, 0, 2, 0, 0, 1, 1, 1, 0, 1, 0, 0, 1 ], + CreditcardBankCharge: [ 1, 1, 0, 1, 0, 1, 1, 1, 1, 0, 1, 0, 0, 1 ], + DividendCharge: [ 1, 1, 1, 1, 0, 0, 1, 1, 0, 0, 1, 0, 1, 1 ], + FinancialCharge: [ 1, 1, 1, 0, 0, 0, 1, 1, 1, 0, 0, 1, 1, 1 ], + ForeignSecuritiesCharge: [ 1, 1, 1, 1, 0, 1, 1, 1, 0, 0, 1, 0, 0, 1 ], + InternalTransferCharge: [ 1, 1, 1, 1, 0, 2, 1, 1, 0, 0, 1, 0, 1, 1 ], + MonthlyVatCharge: [ 1, 1, 0, 1, 0, 0, 1, 1, 0, 0, 1, 1, 1, 1 ], + SalaryCharge: [ 1, 1, 1, 1, 0, 0, 1, 1, 0, 0, 1, 1, 1, 1 ], +}; + +const FIELD_INDEX = new Map(CHARGE_FIELDS.map((field, i) => [field, i])); + +function visibility(type: ChargeType, field: ChargeField): Visibility { + const row = MATRIX[type]; + if (!row) { + // An unknown `__typename` (a charge type added server-side before the client caught up) should + // degrade to showing what it can rather than rendering an empty record. + return 1; + } + return row[FIELD_INDEX.get(field)!] ?? 0; +} + +/** Whether `field` appears on a collapsed record of `type`. Consults only the matrix, never the data. */ +export function isFieldVisible(type: ChargeType, field: ChargeField): boolean { + return visibility(type, field) !== 0; +} + +/** + * Whether `field` needs this type's special-cased rendering (the spec's footnotes): + * - `ConversionCharge.amount` — show the base *and* quote amounts, not one total. + * - `InternalTransferCharge.mainCounterparty` — show both sides of the transfer. + */ +export function isSpecialField(type: ChargeType, field: ChargeField): boolean { + return visibility(type, field) === 2; +} + +/** The fields of `type`, in spec order. Handy for tests and for iterating a region's contents. */ +export function visibleFields(type: ChargeType): ChargeField[] { + return CHARGE_FIELDS.filter(field => isFieldVisible(type, field)); +} + +/** + * The record field each {@link MissingChargeInfo} value would be reported against. Used to drop + * missing-info the record has no place to show — see {@link relevantMissingInfo}. + */ +const MISSING_INFO_FIELD: Record = { + [MissingChargeInfo.Counterparty]: 'mainCounterparty', + [MissingChargeInfo.Description]: 'description', + [MissingChargeInfo.Documents]: 'documentsCount', + [MissingChargeInfo.Tags]: 'tags', + [MissingChargeInfo.TaxCategory]: 'mainTaxCategory', + [MissingChargeInfo.Transactions]: 'transactionsCount', + [MissingChargeInfo.Vat]: 'vat', +}; + +/** + * Server-reported missing info, filtered to what this charge type actually displays. + * + * The server's own validation rules and this spec disagree in places — `validate.helper.ts` excludes + * a required counterparty for InternalTransfer/Salary/Financial, while the spec hides the + * counterparty field for BusinessTrip/Dividend/Conversion/Salary and more. Without this filter a + * record would advertise a need for a field it never shows, and the needs badge would count + * something the user cannot act on. + */ +export function relevantMissingInfo( + type: ChargeType, + missingInfo: readonly MissingChargeInfo[] | undefined, +): MissingChargeInfo[] { + return (missingInfo ?? []).filter(info => isFieldVisible(type, MISSING_INFO_FIELD[info])); +} + +/** Whether `info` is both reported missing and displayable on `type` — for per-field indicators. */ +export function isMissing( + type: ChargeType, + missingInfo: readonly MissingChargeInfo[] | undefined, + info: MissingChargeInfo, +): boolean { + return !!missingInfo?.includes(info) && isFieldVisible(type, MISSING_INFO_FIELD[info]); +} diff --git a/packages/client/src/components/charges/charge-indicators.stories.tsx b/packages/client/src/components/charges/charge-indicators.stories.tsx new file mode 100644 index 0000000000..3639e8e80c --- /dev/null +++ b/packages/client/src/components/charges/charge-indicators.stories.tsx @@ -0,0 +1,223 @@ +import type { ReactElement, ReactNode } from 'react'; +import type { Meta, StoryObj } from '@storybook/react-vite'; +import { Currency, LedgerValidationStatus, MissingChargeInfo } from '../../gql/graphql.js'; +import { CHARGE_TYPE_COLOR, CHARGE_TYPE_NAME, type ChargeType } from '../../helpers/index.js'; +import { + amountState, + AmountText, + ChargeTypeBadge, + CountChip, + ledgerState, + NeedsBadge, + StatusDot, + vatState, + type IndicatorState, +} from './charge-indicators.js'; +import { ChargeDescriptionField, ChargeTagsField } from './charge-suggestion-field.js'; + +const ALL_TYPES = Object.keys(CHARGE_TYPE_NAME) as ChargeType[]; +const ALL_STATES: IndicatorState[] = ['ok', 'pending', 'warning', 'error']; + +function Row({ label, children }: { label: string; children: ReactNode }): ReactElement { + return ( +
+ {label} +
{children}
+
+ ); +} + +function Section({ title, children }: { title: string; children: ReactNode }): ReactElement { + return ( +
+

{title}

+ {children} +
+ ); +} + +function Gallery(): ReactElement { + const tags = [ + { id: 't1', name: 'saas', namePath: ['expenses', 'software'] }, + { id: 't2', name: 'infra' }, + ]; + + return ( +
+
+

Charge record indicators

+

+ Every non-ok state contributes text to its accessible name — hover or inspect to confirm + nothing is conveyed by color alone. ok renders no dot, so a complete record + stays visually quiet. +

+
+ +
+ + {ALL_TYPES.map(type => ( + + ))} + + + {ALL_TYPES.map(type => ( + + {CHARGE_TYPE_COLOR[type]} + + ))} + + + + Those three mean needs-attention, accept and error respectively, and all three appear on + the same record as the chip. Salary, Monthly VAT and Dividend were moved off amber, + emerald and rose for exactly that reason — see the invariant in + charge-indicators.test.tsx. + + +
+ +
+ + {ALL_STATES.map(state => ( + + + {state} + + ))} + +
+ +
+ + + + + + + + + + + + + + + +
+ +
+ + + + + + +
+ + + +
+ + + {[undefined, true, false].map((isValid, i) => ( + + + + + ))} + +
+ +
+ + {[ + { value: 180, currency: Currency.Ils, amountValue: 1180, isMissingInfo: false }, + { value: undefined, currency: Currency.Ils, amountValue: 1180, isMissingInfo: false }, + { value: 180, currency: Currency.Ils, amountValue: -1180, isMissingInfo: false }, + ].map((props, i) => ( + + + + VAT {props.value ?? '—'} on {props.amountValue} + + + ))} + +
+ +
+ + + (empty) + + + + + + {ALL_TYPES.map(type => ( + + + + {CHARGE_TYPE_NAME[type]} + + + ))} + +
+ +
+ + {}} + /> + {}} + /> + {}} + /> + + + {}} /> + {}} /> + {}} /> + +
+
+ ); +} + +const meta = { + title: 'Charges/Indicators', + component: Gallery, + parameters: { layout: 'fullscreen' }, +} satisfies Meta; + +export default meta; +type Story = StoryObj; + +export const Default: Story = {}; diff --git a/packages/client/src/components/charges/charge-indicators.tsx b/packages/client/src/components/charges/charge-indicators.tsx new file mode 100644 index 0000000000..937f4e6f49 --- /dev/null +++ b/packages/client/src/components/charges/charge-indicators.tsx @@ -0,0 +1,289 @@ +import type { ReactElement } from 'react'; +import { TriangleAlert } from 'lucide-react'; +import { cn } from '@/lib/utils.js'; +import { Currency, LedgerValidationStatus, MissingChargeInfo } from '../../gql/graphql.js'; +import { + formatAmountWithCurrency, + getChargeTypeColor, + getChargeTypeIcon, + getChargeTypeName, + type ChargeType, + type ChargeTypeColor, +} from '../../helpers/index.js'; +import { relevantMissingInfo } from './charge-fields.js'; + +/** + * Health of a single record field or count. + * + * `pending` covers a `@defer`red value that has not arrived yet (notably + * `metadata.invalidLedger`), which must read as "not known" rather than "fine". + */ +export type IndicatorState = 'ok' | 'pending' | 'warning' | 'error'; + +const DOT_CLASS: Record, string> = { + pending: 'bg-muted-foreground/40 animate-pulse', + warning: 'bg-amber-500 dark:bg-amber-400', + error: 'bg-red-500 dark:bg-red-400', +}; + +/** Human wording for an indicator, used as the accessible name so state is never color-only. */ +const STATE_LABEL: Record = { + ok: 'ok', + pending: 'checking', + warning: 'has differences', + error: 'has issues', +}; + +/** + * A displayed field that has no value. + * + * Deliberately quiet: the needs badge in the manage region carries the alarm for anything actually + * missing, so repeating it per field would make a record read as full of errors when it is merely + * incomplete. Was defined identically in two places before this. + */ +export function AbsentValue({ children }: { children: string }): ReactElement { + return {children}; +} + +/** + * Replaces Mantine's `Indicator`. Two differences that matter: `ok` renders nothing (so a healthy + * record is visually quiet), and every non-ok state contributes text to its container's accessible + * name — the old corner dot conveyed validation state by color alone. + */ +export function StatusDot({ + state, + className, +}: { + state: IndicatorState; + className?: string; +}): ReactElement | null { + if (state === 'ok') { + return null; + } + return ( + + ); +} + +/** + * Complete literal class list per hue. Written out rather than assembled from the hue name because + * Tailwind only ever sees literals in source — `bg-${color}-50` would compile to nothing, which is + * precisely the failure this codebase spent a release with across its whole token layer. + */ +const TYPE_CHIP_CLASS: Record = { + indigo: + 'bg-indigo-50 text-indigo-700 ring-indigo-200 dark:bg-indigo-950/40 dark:text-indigo-300 dark:ring-indigo-900', + violet: + 'bg-violet-50 text-violet-700 ring-violet-200 dark:bg-violet-950/40 dark:text-violet-300 dark:ring-violet-900', + teal: 'bg-teal-50 text-teal-700 ring-teal-200 dark:bg-teal-950/40 dark:text-teal-300 dark:ring-teal-900', + cyan: 'bg-cyan-50 text-cyan-700 ring-cyan-200 dark:bg-cyan-950/40 dark:text-cyan-300 dark:ring-cyan-900', + blue: 'bg-blue-50 text-blue-700 ring-blue-200 dark:bg-blue-950/40 dark:text-blue-300 dark:ring-blue-900', + orange: + 'bg-orange-50 text-orange-700 ring-orange-200 dark:bg-orange-950/40 dark:text-orange-300 dark:ring-orange-900', + fuchsia: + 'bg-fuchsia-50 text-fuchsia-700 ring-fuchsia-200 dark:bg-fuchsia-950/40 dark:text-fuchsia-300 dark:ring-fuchsia-900', + slate: + 'bg-slate-100 text-slate-700 ring-slate-200 dark:bg-slate-800/60 dark:text-slate-300 dark:ring-slate-700', + purple: + 'bg-purple-50 text-purple-700 ring-purple-200 dark:bg-purple-950/40 dark:text-purple-300 dark:ring-purple-900', + sky: 'bg-sky-50 text-sky-700 ring-sky-200 dark:bg-sky-950/40 dark:text-sky-300 dark:ring-sky-900', + pink: 'bg-pink-50 text-pink-700 ring-pink-200 dark:bg-pink-950/40 dark:text-pink-300 dark:ring-pink-900', +}; + +/** The hue class list for a charge type. Exported for the palette invariant test. */ +export function chargeTypeChipClass(type: ChargeType): string { + return TYPE_CHIP_CLASS[getChargeTypeColor(type)]; +} + +/** + * The record's type token: a tinted icon chip beside the full type name. + * + * The type is the key to interpreting every other field, so it reads as a name rather than hiding an + * icon behind a tooltip. Colour makes it findable down a long list, and is never the only signal — + * the name always reads, which matters both for colourblind users and for the eleven hues that are + * only a step apart from each other. + */ +export function ChargeTypeBadge({ + type, + className, +}: { + type: ChargeType; + className?: string; +}): ReactElement { + return ( + + + {getChargeTypeIcon(type)} + + {getChargeTypeName(type)} + + ); +} + +/** Maps `metadata.invalidLedger` — absent while deferred — onto an indicator state. */ +export function ledgerState(invalidLedger: LedgerValidationStatus | undefined): IndicatorState { + switch (invalidLedger) { + case undefined: + return 'pending'; + case LedgerValidationStatus.Valid: + return 'ok'; + case LedgerValidationStatus.Diff: + return 'warning'; + case LedgerValidationStatus.Invalid: + return 'error'; + } +} + +/** + * Maps `CreditcardBankCharge.validCreditCardAmount` onto an indicator state. Only that type + * validates its amount; every other type passes `undefined` for `shouldValidate` and gets `ok`. + */ +export function amountState(shouldValidate: boolean, isValid: boolean | undefined): IndicatorState { + if (!shouldValidate) { + return 'ok'; + } + if (isValid === undefined) { + return 'pending'; + } + return isValid ? 'ok' : 'error'; +} + +/** + * One count from the record's health region — `⬤ 4 ledger`. The dot and the count share a single + * accessible name so a screen reader hears "ledger 4, has differences" rather than just "4". + */ +export function CountChip({ + label, + count, + state = 'ok', + hint, +}: { + label: string; + count: number; + state?: IndicatorState; + /** Hover text. Supplements the accessible name below; never a substitute for it. */ + hint?: string; +}): ReactElement { + return ( + + + + {count} + + {label} + + ); +} + +/** Human wording for each missing-info kind, for the needs badge's accessible name. */ +const MISSING_INFO_LABEL: Record = { + [MissingChargeInfo.Counterparty]: 'counterparty', + [MissingChargeInfo.Description]: 'description', + [MissingChargeInfo.Documents]: 'documents', + [MissingChargeInfo.Tags]: 'tags', + [MissingChargeInfo.TaxCategory]: 'tax category', + [MissingChargeInfo.Transactions]: 'transactions', + [MissingChargeInfo.Vat]: 'VAT', +}; + +/** + * Row-level roll-up of what a charge is missing, sitting in the record's manage region so a single + * column answers "what needs work" down a long list. Renders nothing when the charge is complete. + * + * Counts only missing info the record actually displays (see `relevantMissingInfo`) — otherwise a + * type would advertise a need for a field it never shows. + */ +export function NeedsBadge({ + type, + missingInfo, +}: { + type: ChargeType; + missingInfo: readonly MissingChargeInfo[] | undefined; +}): ReactElement | null { + const relevant = relevantMissingInfo(type, missingInfo); + if (relevant.length === 0) { + return null; + } + const names = relevant.map(info => MISSING_INFO_LABEL[info]).join(', '); + return ( + + + + {relevant.length} + + + ); +} + +/** + * A signed money value. `tabular-nums` is the point: it is what lets amounts line up down the + * record list, which a plain table cell never did. + */ +export function AmountText({ + value, + currency, + className, +}: { + value: number; + currency: Currency; + className?: string; +}): ReactElement { + return ( + 0 ? 'text-emerald-700 dark:text-emerald-400' : 'text-red-600 dark:text-red-400', + className, + )} + > + {formatAmountWithCurrency(value, currency)} + + ); +} + +/** + * VAT alongside its own health. Keeps the two local consistency checks the old `Vat` cell made — + * a missing VAT on an ILS charge, and a VAT whose sign disagrees with the charge amount. + */ +export function vatState({ + value, + currency, + amountValue, + isMissingInfo, +}: { + value: number | undefined; + currency: Currency | undefined; + amountValue: number | undefined; + isMissingInfo: boolean; +}): IndicatorState { + if (isMissingInfo) { + return 'error'; + } + const localCurrencyButNoVat = value == null && currency === Currency.Ils; + const signDisagreesWithAmount = + ((value ?? 0) > 0 && (amountValue ?? 0) < 0) || ((value ?? 0) < 0 && (amountValue ?? 0) > 0); + return localCurrencyButNoVat || signDisagreesWithAmount ? 'error' : 'ok'; +} diff --git a/packages/client/src/components/charges/charge-record-regions.tsx b/packages/client/src/components/charges/charge-record-regions.tsx new file mode 100644 index 0000000000..ea1e382f91 --- /dev/null +++ b/packages/client/src/components/charges/charge-record-regions.tsx @@ -0,0 +1,386 @@ +import type { ReactElement } from 'react'; +import { format } from 'date-fns'; +import { ArrowRight, ChevronDown, ChevronUp } from 'lucide-react'; +import { Link } from 'react-router-dom'; +import { ROUTES } from '@/router/routes.js'; +import { MissingChargeInfo } from '../../gql/graphql.js'; +import { Tooltip, UpdateAccountantStatus } from '../common/index.js'; +import { Button } from '../ui/button.js'; +import { Checkbox } from '../ui/checkbox.js'; +import { ChargeActionsMenu } from './charge-actions-menu.js'; +import { hasDateRange } from './charge-dates.js'; +import { isFieldVisible, isMissing, isSpecialField } from './charge-fields.js'; +import type { IndicatorState } from './charge-indicators.js'; +import { + AbsentValue as Absent, + amountState, + AmountText, + ChargeTypeBadge, + CountChip, + ledgerState, + NeedsBadge, + StatusDot, + vatState, +} from './charge-indicators.js'; +import { ChargeDescriptionField, ChargeTagsField } from './charge-suggestion-field.js'; +import type { ChargeRow } from './charges-table.js'; +import type { ChargeDensity } from './use-charge-density.js'; + +/** + * The record is six regions at fixed horizontal positions. Region placement is identical on every + * charge — that is what preserves the vertical scanning a table gave us — while *which fields* appear + * inside a region varies by type, per the spec matrix. + * + * Every region therefore renders its grid cell unconditionally, even when the matrix hides all of its + * fields. Returning `null` instead removes a grid child, which slides every later region one column + * left: a `FinancialCharge` has neither amount nor VAT, and its actions column ended up misaligned + * with every other record's. + */ + +type RegionProps = { + row: ChargeRow; + onChange: () => void; +}; + +/** + * Compact mode keeps the fields that answer "which charge is this and does it need me" — type, date, + * description, amount, and the needs badge — and drops the supporting detail (date range, counterparty + * → tax category, tags, count chips). It is a display choice only: nothing about the matrix changes, + * so a field hidden by compact was still going to be hidden by the matrix if the type excluded it. + */ +function isCompact(density: ChargeDensity): boolean { + return density === 'compact'; +} + +/** Hover wording for the ledger chip, whose state is the only one with four possibilities. */ +const LEDGER_HINT: Record = { + ok: 'Ledger is valid', + pending: 'Validating the ledger…', + warning: 'Ledger differs from the expected records', + error: 'Ledger is invalid', +}; + +/** A — selection, accountant approval, and the row-level roll-up of what this charge is missing. */ +export function ManageRegion({ + row, + onChange, + isSelected, + onSelectedChange, + onStatusChange, +}: RegionProps & { + isSelected: boolean; + onSelectedChange: (selected: boolean) => void; + onStatusChange: () => void; +}): ReactElement { + return ( + // Laid out in a row, not a column. Stacked, these three controls were ~70px tall and set the + // record's height regardless of how much content it had — the same mistake the old table's + // `select` cell made, where a checkbox over a status button padded every row out. +
+ onSelectedChange(!!value)} + // 100 identical "Select row" labels are useless to a screen reader; name the charge. + aria-label={`Select charge ${row.description ?? row.id}`} + /> + + +
+ ); +} + +/** B — what kind of charge this is, and when. */ +export function IdentityRegion({ + row, + density, +}: { + row: ChargeRow; + density: ChargeDensity; +}): ReactElement { + const showRange = + !isCompact(density) && isFieldVisible(row.type, 'dateRange') && hasDateRange(row.dates); + + return ( +
+ + {isFieldVisible(row.type, 'mainDate') && ( + + {row.dates?.date ? format(row.dates.date, 'dd MMM yy') : No date} + + )} + {showRange && ( + + {format(row.dates!.mostMinDate!, 'dd MMM')} –{' '} + {format(row.dates!.mostMaxDate!, 'dd MMM yy')} + + )} +
+ ); +} + +/** C — what the charge means: description, who it is with, how it is classified, and its tags. */ +export function MeaningRegion({ + row, + onChange, + density, +}: RegionProps & { density: ChargeDensity }): ReactElement { + const compact = isCompact(density); + const showCounterparty = !compact && isFieldVisible(row.type, 'mainCounterparty'); + const showTaxCategory = !compact && isFieldVisible(row.type, 'mainTaxCategory'); + const showBusinessTrip = !compact && isFieldVisible(row.type, 'businessTrip'); + // The spec's footnote: an internal transfer moves money between two of your own accounts, so a + // single "counterparty" would be a half-truth. + const bothSides = isSpecialField(row.type, 'mainCounterparty'); + + return ( +
+ {isFieldVisible(row.type, 'description') && ( + + + + )} + + {(showCounterparty || showTaxCategory || showBusinessTrip) && ( + + {showCounterparty && + (row.counterparty ? ( + <> + event.stopPropagation()} + className="font-medium underline-offset-2 hover:underline" + > + {row.counterparty.name} + + {bothSides && ( + + + + both sides + + + )} + + ) : ( + No counterparty + ))} + + {showCounterparty && showTaxCategory && ( + + )} + + {showTaxCategory && + (row.taxCategory ? ( + {row.taxCategory.name} + ) : ( + No tax category + ))} + + {showBusinessTrip && row.businessTrip && (showCounterparty || showTaxCategory) && ( + + · + + )} + + {showBusinessTrip && row.businessTrip && ( + event.stopPropagation()} + className="font-medium underline-offset-2 hover:underline" + > + {row.businessTrip.name} + + )} + + )} + + {!compact && isFieldVisible(row.type, 'tags') && ( + + + + )} +
+ ); +} + +/** D — the money. Right-aligned and tabular so amounts compare down the list. */ +export function MoneyRegion({ + row, + density, +}: { + row: ChargeRow; + density: ChargeDensity; +}): ReactElement { + const showAmount = isFieldVisible(row.type, 'amount'); + const showVat = !isCompact(density) && isFieldVisible(row.type, 'vat'); + + // The spec's footnote: a conversion has a base and a quote amount, so one total would hide half of + // what happened. The base/quote pair itself lives on the expansion panel; the record shows the + // amount it has plus a marker that there are two sides. + const isConversion = isSpecialField(row.type, 'amount'); + + return ( +
+ {showAmount && + (row.amount ? ( + + + + + ) : ( + No amount + ))} + {isConversion && base → quote} + {showVat && ( + + + {row.vat?.value == null ? ( + No VAT + ) : ( + VAT {row.vat.value.toLocaleString()} + )} + + )} +
+ ); +} + +/** E — the charge's health: what is attached to it, and whether any of it is wrong. */ +export function HealthRegion({ + row, + density, +}: { + row: ChargeRow; + density: ChargeDensity; +}): ReactElement { + const chips: ReactElement[] = []; + const compact = isCompact(density); + + const txMissing = isMissing(row.type, row.missingInfo, MissingChargeInfo.Transactions); + const docMissing = isMissing(row.type, row.missingInfo, MissingChargeInfo.Documents); + + if (!compact && isFieldVisible(row.type, 'transactionsCount')) { + chips.push( + , + ); + } + if (!compact && isFieldVisible(row.type, 'documentsCount')) { + chips.push( + , + ); + } + if (!compact && isFieldVisible(row.type, 'miscExpensesCount')) { + chips.push( + , + ); + } + if (!compact && isFieldVisible(row.type, 'ledgerCount')) { + chips.push( + , + ); + } + + return ( +
+ {chips} +
+ ); +} + +/** F — per-charge actions and the expansion toggle. */ +export function ActionsRegion({ + row, + onChange, + onDelete, + isExpanded, + onToggleExpanded, + panelId, +}: RegionProps & { + onDelete: () => void; + isExpanded: boolean; + onToggleExpanded: () => void; + panelId: string; +}): ReactElement { + return ( +
+ 0} + /> + + + +
+ ); +} diff --git a/packages/client/src/components/charges/charge-record.stories.tsx b/packages/client/src/components/charges/charge-record.stories.tsx new file mode 100644 index 0000000000..920e3c408b --- /dev/null +++ b/packages/client/src/components/charges/charge-record.stories.tsx @@ -0,0 +1,272 @@ +import { useState, type ReactElement } from 'react'; +import { MemoryRouter } from 'react-router-dom'; +import type { Meta, StoryObj } from '@storybook/react-vite'; +import { + AccountantStatus, + Currency, + LedgerValidationStatus, + MissingChargeInfo, +} from '../../gql/graphql.js'; +import { CHARGE_TYPE_NAME, type ChargeType } from '../../helpers/index.js'; +import { visibleFields } from './charge-fields.js'; +import { ChargeRecord } from './charge-record.js'; +import type { ChargeRow } from './charges-table.js'; +import type { ChargeDensity } from './use-charge-density.js'; + +/** Row order copied from the spec spreadsheet so this reads alongside the Spec Matrix story. */ +const SHEET_ORDER: ChargeType[] = [ + 'BankDepositCharge', + 'BusinessTripCharge', + 'CommonCharge', + 'ConversionCharge', + 'CreditcardBankCharge', + 'DividendCharge', + 'FinancialCharge', + 'ForeignSecuritiesCharge', + 'InternalTransferCharge', + 'MonthlyVatCharge', + 'SalaryCharge', +]; + +/** + * Every field populated. Because the matrix decides visibility rather than the data, a fully-loaded + * fixture is exactly what proves suppression: whatever a record omits, it omits by spec. + */ +function makeRow(type: ChargeType, overrides: Partial = {}): ChargeRow { + return { + id: `charge-${type}`, + type, + dates: { + date: new Date('2026-03-03'), + mostMinDate: new Date('2026-03-01'), + mostMaxDate: new Date('2026-03-05'), + }, + amount: { value: -1180.5, currency: Currency.Ils, shouldValidate: false }, + vat: { value: 180.5, currency: Currency.Ils }, + counterparty: { id: 'biz-1', name: 'Google Cloud EMEA' }, + description: 'Monthly cloud spend', + tags: [ + { id: 't1', name: 'saas', namePath: ['expenses', 'software'] }, + { id: 't2', name: 'infra' }, + ], + suggestedTags: [], + taxCategory: { id: 'tc-1', name: 'Cloud Infrastructure' }, + businessTrip: { id: 'trip-1', name: 'Berlin Q1 Summit' }, + counts: { + transactions: 2, + documents: 1, + ledger: 4, + miscExpenses: 0, + invalidLedger: LedgerValidationStatus.Valid, + }, + missingInfo: [], + accountantApproval: AccountantStatus.Unapproved, + ...overrides, + }; +} + +/** Renders records in a list, the way `ChargesTable` does, so alignment across them is visible. */ +function RecordList({ + rows, + density = 'comfortable', +}: { + rows: ChargeRow[]; + density?: ChargeDensity; +}): ReactElement { + const [selected, setSelected] = useState>({}); + const [expanded, setExpanded] = useState>({}); + return ( +
    + {rows.map(row => ( + setSelected(s => ({ ...s, [row.id]: value }))} + onToggleExpanded={() => setExpanded(e => ({ ...e, [row.id]: !e[row.id] }))} + onCollapse={() => setExpanded(e => ({ ...e, [row.id]: false }))} + registerRefetch={() => () => {}} + updateCharge={() => {}} + removeCharge={() => {}} + /> + ))} +
+ ); +} + +function Section({ + title, + note, + rows, + density, +}: { + title: string; + note?: string; + rows: ChargeRow[]; + density?: ChargeDensity; +}) { + return ( +
+
+

{title}

+ {note &&

{note}

} +
+ +
+ ); +} + +function AllTypes(): ReactElement { + return ( +
+
+

Charge record — all 11 types

+

+ Every fixture below has every field populated. Anything a record does not + show, it omits because the spec matrix says so — not because the data was missing. Region + positions are identical on every row; the fields inside them are not. +

+
+ +
makeRow(type))} + /> + +
+ +
+ +
+ +
makeRow(type, { id: `compact-${type}` }))} + /> + +
+ makeRow('CommonCharge', { id: `status-${status}`, accountantApproval: status }), + )} + /> + +
+

Field counts per type, from the matrix

+
    + {SHEET_ORDER.map(type => ( +
  • + {CHARGE_TYPE_NAME[type]} + {visibleFields(type).length} +
    • + ))} +
+
+
+ ); +} + +const meta = { + title: 'Charges/Charge Record', + component: AllTypes, + parameters: { layout: 'fullscreen' }, + decorators: [ + Story => ( + + + + ), + ], +} satisfies Meta; + +export default meta; +type Story = StoryObj; + +export const Default: Story = {}; diff --git a/packages/client/src/components/charges/charge-record.tsx b/packages/client/src/components/charges/charge-record.tsx new file mode 100644 index 0000000000..d414c3b78d --- /dev/null +++ b/packages/client/src/components/charges/charge-record.tsx @@ -0,0 +1,207 @@ +import { memo, useCallback, useEffect, useMemo, type ReactElement } from 'react'; +import { useQuery } from 'urql'; +import { + ChargeForChargesTableFieldsFragmentDoc, + RefetchChargeForChargesTableDocument, +} from '@/gql/graphql.js'; +import { getFragmentData } from '@/gql/index.js'; +import { cn } from '@/lib/utils.js'; +import { DragFile } from '../common/index.js'; +import { Card } from '../ui/card.js'; +import { ChargeExtendedInfo } from './charge-extended-info.js'; +import { + ActionsRegion, + HealthRegion, + IdentityRegion, + ManageRegion, + MeaningRegion, + MoneyRegion, +} from './charge-record-regions.js'; +import { convertChargeFragmentToTableRow, type ChargeRow } from './charges-table.js'; +import type { ChargeDensity } from './use-charge-density.js'; + +// eslint-disable-next-line @typescript-eslint/no-unused-expressions -- used by codegen +/* GraphQL */ ` + query RefetchChargeForChargesTable($chargeId: UUID!) { + charge(chargeId: $chargeId) { + id + ...ChargeForChargesTableFields + } + } +`; + +type Props = { + row: ChargeRow; + /** + * Selection and expansion arrive as plain booleans rather than as the tanstack row. + * + * `row.getIsExpanded()` reads live table state, so a retained row object returns the *current* + * value — which made a `memo` comparator that called it useless: after a toggle both the previous + * and next props reported the new value, the comparator saw equality, and the record never + * re-rendered. Primitives snapshot correctly. + */ + isSelected: boolean; + isExpanded: boolean; + density: ChargeDensity; + onSelectedChange: (selected: boolean) => void; + onToggleExpanded: () => void; + onCollapse: () => void; + /** Registers this charge's refetch so batch actions can refresh it without it being rendered. */ + registerRefetch: (chargeId: string, refetch: () => void) => () => void; + updateCharge: (charge: ChargeRow) => void; + removeCharge: (chargeId: string) => void; +}; + +function ChargeRecordImpl({ + row, + isSelected, + isExpanded, + density, + onSelectedChange, + onToggleExpanded, + onCollapse, + registerRefetch, + updateCharge, + removeCharge, +}: Props): ReactElement { + const [{ data: newData, fetching }, fetchCharge] = useQuery({ + query: RefetchChargeForChargesTableDocument, + pause: true, + variables: { chargeId: row.id }, + }); + + // Handlers are ordinary props now, not properties written onto the row model during render. + useEffect(() => registerRefetch(row.id, fetchCharge), [registerRefetch, row.id, fetchCharge]); + + // `network-only` (#4239): this always runs right after a mutation, so a replayed or cached result + // would silently re-apply the pre-mutation charge. It also swallows its argument — `onChange` is + // handed to plain callbacks and DOM handlers alike, and forwarding their argument would land it in + // urql's `OperationContext`. + const onChange = useCallback((): void => { + fetchCharge({ requestPolicy: 'network-only' }); + }, [fetchCharge]); + const onDelete = useCallback(() => removeCharge(row.id), [removeCharge, row.id]); + + const originalStringified = useMemo(() => JSON.stringify(row), [row]); + const newRow = useMemo( + () => + newData?.charge + ? convertChargeFragmentToTableRow( + getFragmentData(ChargeForChargesTableFieldsFragmentDoc, newData.charge), + ) + : null, + [newData], + ); + const newStringified = useMemo(() => (newRow ? JSON.stringify(newRow) : null), [newRow]); + + useEffect(() => { + if (newRow && newStringified && !fetching && newStringified !== originalStringified) { + updateCharge(newRow); + } + }, [newRow, newStringified, originalStringified, fetching, updateCharge]); + + const titleId = `charge-${row.id}-title`; + const panelId = `charge-${row.id}-panel`; + + return ( +
  • + {/* The whole record is the drop target. Previously only the narrow More Info cell accepted a + dropped document, so filing a PDF meant hitting one column. */} + +
    + + + {/* The description doubles as the record's accessible name. */} + + {row.description ?? row.suggestedDescription ?? `Charge ${row.id}`} + + + {/* Health before money, so the amount lands hard against the right edge next to the + actions — the strongest position for the number the eye scans for. Ordered the other + way round, a right-aligned amount butted straight into the left-aligned count chips and + the two read as one run-on string. */} + + + +
    +     + + {/* Inside the same `
  • `, not a sibling — otherwise the list count and keyboard order break. */} + {isExpanded && ( +
    + {/* The rail indents the panel under its record so it reads as belonging to it rather than + as the next thing in the list — full-bleed, the two were only distinguishable by + background tint. */} +
    + {/* `w-0 min-w-full overflow-x-auto` keeps the (wide) extended info from stretching the + page: it renders at the record's width and anything wider scrolls in here. The panel + still contains genuinely wide nested tables (transactions, documents, ledger). */} +
    + + + +
    +
    +
    + )} +
    • + ); +} + +/** + * Memoized on the fields that actually change what is drawn. Worth it at 100 records — and only safe + * because the refetch handler is no longer written onto `row` during render (see the registry in + * `ChargesTable`); with the old mutation a memoized record would have kept a no-op handler. + */ +export const ChargeRecord = memo( + ChargeRecordImpl, + (prev, next) => + prev.row === next.row && + prev.isSelected === next.isSelected && + prev.isExpanded === next.isExpanded && + prev.density === next.density, + // The callbacks are deliberately not compared: they are fresh closures on every render but each + // one calls through to the live table, so a retained closure still acts on current state. +); diff --git a/packages/client/src/components/charges/charge-suggestion-field.tsx b/packages/client/src/components/charges/charge-suggestion-field.tsx new file mode 100644 index 0000000000..a04e75abdb --- /dev/null +++ b/packages/client/src/components/charges/charge-suggestion-field.tsx @@ -0,0 +1,197 @@ +import { useCallback, useState, type ReactElement, type ReactNode } from 'react'; +import { Check } from 'lucide-react'; +import { useUpdateCharge } from '../../hooks/use-update-charge.js'; +import { SimilarChargesByIdModal } from '../common/index.js'; +import { Badge } from '../ui/badge.js'; +import { Button } from '../ui/button.js'; +import { AbsentValue } from './charge-indicators.js'; + +export type SuggestedTag = { id: string; name: string; namePath?: string[] }; + +/** What the similar-charges follow-up matches on, once a suggestion has been applied. */ +type SimilarCriteria = { description?: string; tagIds?: { id: string }[] }; + +/** + * Shared behavior behind the description and tags fields: offer a server-suggested value inline, + * apply it in one click, then offer to apply the same value to similar charges. + * + * Replaces the old `bg-yellow-400` block, which read as an error rather than an offer. A pending + * suggestion is instead marked with a dashed amber underline — visible, but not alarming. + */ +function SuggestionAffordance({ + chargeId, + label, + onAccept, + fetching, + onChange, + children, +}: { + chargeId: string; + /** Names the accept control for assistive tech, e.g. "Accept suggested description". */ + label: string; + /** + * Applies the suggestion. Resolves to the criteria the similar-charges follow-up should compare + * against, or `null`/`undefined` if the mutation failed. + */ + onAccept: () => Promise; + /** + * In-flight state of the caller's own `useUpdateCharge`. Passed down rather than read from a + * second `useUpdateCharge()` here — each call is an independent `useMutation`, so a local one + * would never observe the parent's mutation and the button would never disable. + */ + fetching: boolean; + onChange: () => void; + children: ReactNode; +}): ReactElement { + const [similarOpen, setSimilarOpen] = useState(false); + /** + * What the follow-up dialog compares against, captured at the moment it was applied (#4239). + * Reading it off props instead would make the criteria vanish as soon as `onChange` refreshes the + * record — the suggestion is gone once accepted — closing the dialog immediately. + */ + const [applied, setApplied] = useState(undefined); + + const accept = useCallback( + async (event: { stopPropagation: () => void }): Promise => { + event.stopPropagation(); + // Only proceed once the charge actually took the value — `updateCharge` resolves to `void` on + // failure, having already toasted the error. + const criteria = await onAccept(); + if (!criteria) { + return; + } + // Refresh on the mutation itself, not on the dialog's close (#4239): hanging it off the + // follow-up made the update invisible whenever that dialog resolved without closing. + onChange(); + setApplied(criteria); + setSimilarOpen(true); + }, + [onAccept, onChange], + ); + + return ( + <> + + + {children} + + + + + + + ); +} + +export function ChargeDescriptionField({ + chargeId, + value, + suggestion, + onChange, +}: { + chargeId: string; + value: string | undefined; + suggestion: string | undefined; + onChange: () => void; +}): ReactElement { + const { updateCharge, fetching } = useUpdateCharge(); + const trimmed = value?.trim(); + + const accept = useCallback(async () => { + const value = suggestion!; + const updated = await updateCharge({ chargeId, fields: { userDescription: value } }); + return updated ? { description: value } : null; + }, [chargeId, suggestion, updateCharge]); + + if (trimmed) { + return {trimmed}; + } + if (!suggestion) { + return No description; + } + return ( + + {suggestion} + + ); +} + +/** A tag chip. The `namePath` breadcrumb goes in the tooltip rather than inline — the record is three + * lines tall, and a full `a > b > c >` prefix on every tag would dominate it. */ +function TagChip({ tag }: { tag: SuggestedTag }): ReactElement { + const path = tag.namePath?.length ? `${tag.namePath.join(' > ')} > ${tag.name}` : undefined; + return ( + + {tag.name} + + ); +} + +export function ChargeTagsField({ + chargeId, + tags, + suggestedTags, + onChange, +}: { + chargeId: string; + tags: readonly SuggestedTag[]; + suggestedTags: readonly SuggestedTag[]; + onChange: () => void; +}): ReactElement { + const { updateCharge, fetching } = useUpdateCharge(); + + const accept = useCallback(async () => { + const tagIds = suggestedTags.map(tag => ({ id: tag.id })); + const updated = await updateCharge({ chargeId, fields: { tags: tagIds } }); + return updated ? { tagIds } : null; + }, [chargeId, suggestedTags, updateCharge]); + + if (tags.length > 0) { + return ( + + {tags.map(tag => ( + + ))} + + ); + } + if (suggestedTags.length === 0) { + return No tags; + } + return ( + + + {suggestedTags.map(tag => ( + + ))} + + + ); +} diff --git a/packages/client/src/components/charges/charges-batch-actions-menu.tsx b/packages/client/src/components/charges/charges-batch-actions-menu.tsx index 97f7b3bb6c..251f71261b 100644 --- a/packages/client/src/components/charges/charges-batch-actions-menu.tsx +++ b/packages/client/src/components/charges/charges-batch-actions-menu.tsx @@ -16,6 +16,13 @@ import type { ChargeRow } from './charges-table.js'; interface Props { table: Table; + /** + * Refreshes the given charges. Goes through `ChargesTable`'s refetch registry rather than calling a + * handler hung off each row, which silently did nothing for selected charges that were not + * currently rendered — `getSelectedRowModel()` is built from the core row model and so is not + * limited to the paginated rows on screen. + */ + onRefreshCharges?: (chargeIds: string[]) => void; } /** @@ -24,7 +31,7 @@ interface Props { * {@link RegenerateLedgerRecordsButton}, confirmation modal included) and "Change tags" (add/remove * tags across all selected charges), each in a single request. */ -export function ChargesBatchActionsMenu({ table }: Props): ReactElement { +export function ChargesBatchActionsMenu({ table, onRefreshCharges }: Props): ReactElement { const { regenerateLedgerRecords } = useRegenerateLedgerRecords(); const [confirmOpen, setConfirmOpen] = useState(false); const [tagsOpen, setTagsOpen] = useState(false); @@ -33,14 +40,11 @@ export function ChargesBatchActionsMenu({ table }: Props): ReactElement { const selectedCount = rows.length; const selectedIds = rows.map(row => row.original.id); - // Refresh each selected row so the table reflects the applied change. The rows are re-read from - // the table rather than closed over: this runs after an awaited mutation, by which point a row - // refreshed in the meantime has been swapped for a new object and the captured one's `onChange` - // is an inert stub. + // Refresh each selected charge so the list reflects the applied change. The ids are re-read from + // the table rather than closed over (#4239): this runs after an awaited mutation, by which point + // the selection may have moved on, and the captured list would refresh the wrong charges. function refreshSelected(): void { - for (const row of table.getSelectedRowModel().rows) { - row.original.onChange(); - } + onRefreshCharges?.(table.getSelectedRowModel().rows.map(row => row.original.id)); } function onRegenerate(): void { diff --git a/packages/client/src/components/charges/charges-filters.tsx b/packages/client/src/components/charges/charges-filters.tsx index ab0f36ff58..cb33c4cf1d 100644 --- a/packages/client/src/components/charges/charges-filters.tsx +++ b/packages/client/src/components/charges/charges-filters.tsx @@ -48,20 +48,11 @@ interface ChargesFiltersFormProps { withDefaultDateRange?: boolean; } -const fieldsToSort: { label: string; value: ChargeSortByField }[] = [ - { - value: ChargeSortByField.AbsAmount, - label: 'Abs Amount', - }, - { - value: ChargeSortByField.Amount, - label: 'Amount', - }, - { - value: ChargeSortByField.Date, - label: 'Date', - }, -]; +/** + * Sort applied when a filter carries none. Shared by `defaultValues` and `onSubmit` — the sort field + * itself now lives in the list toolbar, not in this form. + */ +const DEFAULT_SORT_BY = { field: ChargeSortByField.Date, asc: false }; export const chargesTypeFilterOptions: Array<{ label: string; value: ChargeFilterType }> = [ { label: 'All', value: ChargeFilterType.All }, @@ -95,10 +86,7 @@ function ChargesFiltersForm({ byOwners: userContext?.context.adminBusinessId ? [userContext.context.adminBusinessId] : undefined, - sortBy: { - field: ChargeSortByField.Date, - asc: false, - }, + sortBy: DEFAULT_SORT_BY, // Screens where the date range is optional (e.g. missing-info charges) // start unbounded, so old unresolved charges aren't hidden by a default // "last year" window. @@ -111,36 +99,23 @@ function ChargesFiltersForm({ ...filter, }, }); - const { control, handleSubmit, watch } = form; - const [asc, setAsc] = useState(filter.sortBy?.asc ?? false); - const [enableAsc, setEnableAsc] = useState(!!filter.sortBy?.field); + const { control, handleSubmit } = form; const { selectableFinancialEntities: financialEntities, fetching: financialEntitiesFetching } = useGetFinancialEntities(); const { selectableTags: tags, fetching: tagsFetching } = useGetTags(); const { selectableBusinessTrips: businessTrips, fetching: businessTripsFetching } = useGetBusinessTrips(); - const sortByField = watch('sortBy.field'); - - useEffect(() => { - if (sortByField && !enableAsc) { - setEnableAsc(true); - } else if (!sortByField && enableAsc) { - setEnableAsc(false); - } - }, [sortByField, enableAsc]); - const onSubmit: SubmitHandler = data => { - if (asc != null && data.sortBy?.field) { - data.sortBy.asc = asc; - } - setFilter(data); + // `sortBy` is no longer a field in this form, but react-hook-form still surfaces it in `data` + // from `defaultValues` — so submitting a filter change would silently reset whatever sort the + // toolbar had applied. Carry the live value through explicitly instead. + setFilter({ ...data, sortBy: filter.sortBy ?? DEFAULT_SORT_BY }); closeModal(); }; function clearFilter(): void { - setFilter({}); - setAsc(false); + setFilter({ sortBy: DEFAULT_SORT_BY }); closeModal(); } @@ -345,39 +320,6 @@ function ChargesFiltersForm({ )} /> - ( - - Field to sort by - -