From 4525308a548673ab390f7f2ca570fcd7c871e75a Mon Sep 17 00:00:00 2001 From: Gil Gardosh Date: Mon, 17 Aug 2026 19:01:28 +0300 Subject: [PATCH 1/2] feat(client): replace the charges table with a per-type charge record list 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, leaving those columns blank on ten rows out of eleven. Rows also measured 166px. Each charge now renders as a record of six regions at fixed horizontal positions. Region placement is identical on every record, preserving the vertical scanning a table gave; 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 compact. TanStack Table stays as the headless engine, so sorting, selection (keyed by charge id) and expansion are unchanged and every call site keeps the same props. columns.tsx becomes an accessor-only sort schema; the presentational cells and utils.ts (whose per-type rules had drifted from the server's) are removed. Bug fixes found along the way: - The Date column never sorted: its accessor returned the whole date object, so the automatic sort compared "[object Object]" with itself and returned the same answer for every pair, in both directions. - Lists silently truncated at 100 rows, hiding everything past row 100 on the screens that fetch without a limit. - Nothing inside a row was clickable when wrapped for drag-and-drop, because Mantine's dropzone disables pointer events on its content. - The header row rendered one more cell than the body. - CSV export could include charges selected in a sibling table. - Batch refresh did nothing for selected but unrendered charges. - Validation state was colour-only with no accessible name, and the expand control was an unlabelled chevron. Co-Authored-By: Claude Opus 5 (1M context) --- .changeset/charges-record-list.md | 52 +++ .../components/business/charges-section.tsx | 13 +- .../components/charges-ledger-validation.tsx | 12 +- .../charges/__tests__/charge-fields.test.ts | 152 +++++++ .../__tests__/charge-indicators.test.tsx | 226 +++++++++++ .../__tests__/charges-table-refetch.test.tsx | 9 +- .../__tests__/charges-toolbar.test.tsx | 320 +++++++++++++++ .../charges/__tests__/columns.test.tsx | 120 ++++++ .../charges/__tests__/pagination.test.tsx | 75 ++++ .../src/components/charges/cells/amount.tsx | 38 -- .../charges/cells/business-trip.tsx | 22 - .../components/charges/cells/counterparty.tsx | 45 --- .../components/charges/cells/description.tsx | 88 ---- .../src/components/charges/cells/index.ts | 10 - .../components/charges/cells/more-info.tsx | 140 ------- .../src/components/charges/cells/tags.tsx | 89 ---- .../components/charges/cells/tax-category.tsx | 18 - .../src/components/charges/cells/type.tsx | 28 -- .../src/components/charges/cells/vat.tsx | 28 -- .../{cells/date.tsx => charge-dates.ts} | 43 +- .../charges/charge-fields.stories.tsx | 142 +++++++ .../src/components/charges/charge-fields.ts | 151 +++++++ .../charges/charge-indicators.stories.tsx | 199 +++++++++ .../components/charges/charge-indicators.tsx | 207 ++++++++++ .../charges/charge-record-regions.tsx | 379 ++++++++++++++++++ .../charges/charge-record.stories.tsx | 272 +++++++++++++ .../src/components/charges/charge-record.tsx | 190 +++++++++ .../charges/charge-suggestion-field.tsx | 202 ++++++++++ .../charges/charges-batch-actions-menu.tsx | 20 +- .../components/charges/charges-filters.tsx | 82 +--- .../src/components/charges/charges-row.tsx | 124 ------ .../components/charges/charges-sort-menu.tsx | 88 ++++ .../src/components/charges/charges-table.tsx | 356 ++++++++-------- .../components/charges/charges-toolbar.tsx | 130 ++++++ .../client/src/components/charges/columns.ts | 39 ++ .../client/src/components/charges/columns.tsx | 160 -------- .../components/charges/use-charge-density.ts | 49 +++ .../client/src/components/charges/utils.ts | 40 -- .../components/common/inputs/drag-file.tsx | 7 + .../screens/charges/all-charges.tsx | 9 +- .../src/components/screens/charges/charge.tsx | 2 +- .../screens/charges/missing-info-charges.tsx | 19 +- 42 files changed, 3281 insertions(+), 1114 deletions(-) create mode 100644 .changeset/charges-record-list.md create mode 100644 packages/client/src/components/charges/__tests__/charge-fields.test.ts create mode 100644 packages/client/src/components/charges/__tests__/charge-indicators.test.tsx create mode 100644 packages/client/src/components/charges/__tests__/charges-toolbar.test.tsx create mode 100644 packages/client/src/components/charges/__tests__/columns.test.tsx create mode 100644 packages/client/src/components/charges/__tests__/pagination.test.tsx delete mode 100644 packages/client/src/components/charges/cells/amount.tsx delete mode 100644 packages/client/src/components/charges/cells/business-trip.tsx delete mode 100644 packages/client/src/components/charges/cells/counterparty.tsx delete mode 100644 packages/client/src/components/charges/cells/description.tsx delete mode 100644 packages/client/src/components/charges/cells/index.ts delete mode 100644 packages/client/src/components/charges/cells/more-info.tsx delete mode 100644 packages/client/src/components/charges/cells/tags.tsx delete mode 100644 packages/client/src/components/charges/cells/tax-category.tsx delete mode 100644 packages/client/src/components/charges/cells/type.tsx delete mode 100644 packages/client/src/components/charges/cells/vat.tsx rename packages/client/src/components/charges/{cells/date.tsx => charge-dates.ts} (57%) create mode 100644 packages/client/src/components/charges/charge-fields.stories.tsx create mode 100644 packages/client/src/components/charges/charge-fields.ts create mode 100644 packages/client/src/components/charges/charge-indicators.stories.tsx create mode 100644 packages/client/src/components/charges/charge-indicators.tsx create mode 100644 packages/client/src/components/charges/charge-record-regions.tsx create mode 100644 packages/client/src/components/charges/charge-record.stories.tsx create mode 100644 packages/client/src/components/charges/charge-record.tsx create mode 100644 packages/client/src/components/charges/charge-suggestion-field.tsx delete mode 100644 packages/client/src/components/charges/charges-row.tsx create mode 100644 packages/client/src/components/charges/charges-sort-menu.tsx create mode 100644 packages/client/src/components/charges/charges-toolbar.tsx create mode 100644 packages/client/src/components/charges/columns.ts delete mode 100644 packages/client/src/components/charges/columns.tsx create mode 100644 packages/client/src/components/charges/use-charge-density.ts delete mode 100644 packages/client/src/components/charges/utils.ts 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..7c2060bbb5 --- /dev/null +++ b/packages/client/src/components/charges/__tests__/charge-indicators.test.tsx @@ -0,0 +1,226 @@ +// @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 { + amountState, + AmountText, + 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(); + }); +}); 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/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..48d780aaa0 --- /dev/null +++ b/packages/client/src/components/charges/charge-indicators.stories.tsx @@ -0,0 +1,199 @@ +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_NAME, type ChargeType } from '../../helpers/index.js'; +import { + amountState, + AmountText, + 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_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..3d176529ca --- /dev/null +++ b/packages/client/src/components/charges/charge-indicators.tsx @@ -0,0 +1,207 @@ +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, type ChargeType } 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-gray-300 dark:bg-gray-600 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', +}; + +/** + * 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 ( + + ); +} + +/** 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', +}: { + label: string; + count: number; + state?: IndicatorState; +}): 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..58eba0a9e1 --- /dev/null +++ b/packages/client/src/components/charges/charge-record-regions.tsx @@ -0,0 +1,379 @@ +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 { getChargeTypeIcon, getChargeTypeName } from '../../helpers/index.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 { + amountState, + AmountText, + 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'; +} + +/** Quiet placeholder for a displayed field with no value. The needs badge carries the alarm. */ +function Absent({ children }: { children: string }): ReactElement { + return {children}; +} + +/** 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 ( +
+ + {/* The type used to be an icon behind a tooltip. It is the key to interpreting everything + else in the record, so it carries its name. */} + + {getChargeTypeIcon(row.type)} + + {getChargeTypeName(row.type)} + + {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); + + 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..7ac92b8922 --- /dev/null +++ b/packages/client/src/components/charges/charge-record.tsx @@ -0,0 +1,190 @@ +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 && ( +
    + {/* `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..91469b9332 --- /dev/null +++ b/packages/client/src/components/charges/charge-suggestion-field.tsx @@ -0,0 +1,202 @@ +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'; + +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} + + + + + + + ); +} + +/** Shown where a displayed field has no value and no suggestion. The row-level needs badge carries + * the alarm, so this stays quiet rather than shouting a second time. */ +function MissingValue({ children }: { children: string }): ReactElement { + 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 - -