From f0c270aa3d9473ec06b5f23f057f93520165ad2d Mon Sep 17 00:00:00 2001 From: Justin Gasper Date: Fri, 18 Sep 2026 12:12:58 +1000 Subject: [PATCH] Sales: auto-apply date range, header inside panel, simpler totals - Apply the date range filter automatically (debounced) as its controls change, with a Clear link, matching the report search and filters - Render the date range filter as a panel with its heading inside the box, like the report panel - Drop the per-tile record-count line from the summary tiles - Show an uncoded single-currency total in US dollars, matching the converted columns - Hide a total whose converted counterpart the report also provides, such as Amount beside Amount (converted) Co-Authored-By: Claude Fable 5.1 --- src/apps/sales/README.md | 23 ++-- src/apps/sales/src/SalesPage.module.scss | 9 +- src/apps/sales/src/SalesPage.spec.tsx | 68 ++++++++--- src/apps/sales/src/SalesPage.tsx | 146 +++++++++++------------ src/apps/sales/src/sales.utils.spec.ts | 27 ++++- src/apps/sales/src/sales.utils.ts | 43 ++++++- 6 files changed, 207 insertions(+), 109 deletions(-) diff --git a/src/apps/sales/README.md b/src/apps/sales/README.md index 5f9bd5c6b..5bf0f858c 100644 --- a/src/apps/sales/README.md +++ b/src/apps/sales/README.md @@ -19,18 +19,19 @@ with its Close button or the X icon; obsolete lookups are aborted. ## Date range filter (PM-6364) -A date range section at the top of the page filters the report by **Created -Date** for pipeline generation, or by **Close Date** for revenue projection. +A date range panel at the top of the page, headed inside its box like the +report panel, filters the report by **Created Date** for pipeline generation, +or by **Close Date** for revenue projection. The Filter type dropdown lists the report's own `date`/`datetime` columns rather than hard-coded Salesforce field IDs, and opens on the Created Date column when the report has one. From date and To date are inclusive and either may be left empty for an open-ended range. -Unlike the search and column filters, the range is only sent when **Apply -filter** is pressed, and **Reset filter** clears it without disturbing search, -column filters or sorting. Clearing the report filters likewise leaves the range -intact. An inverted range is reported inline and never sent. A report with no -date columns disables the section. +Like the search and column filters, the range applies automatically (debounced) +as the Filter type, From date or To date change. **Clear** empties it without +disturbing search, column filters or sorting, and clearing the report filters +likewise leaves the range intact. An inverted range is reported inline and never +sent. A report with no date columns disables the panel. The Reports API applies the range across the whole received snapshot before paginating, so a filtered count is the real matching count and not a per-page @@ -42,8 +43,12 @@ Summary tiles above the report show the metrics the current filters produce over every matching record: opportunity count, a total per numeric column (pipeline value and revenue projections), and a breakdown per category column such as Stage. The API computes them, so they never describe only the visible page. -Totals show their shared currency; a total that sums different currencies is -rendered as a plain number and labelled as mixed. Tiles are hidden when the API +Each tile shows only its label and value. A total whose converted counterpart +the report also provides, such as Amount beside Amount (converted), is hidden +for now as redundant. Totals show their shared currency, and a single-currency +total the report leaves uncoded is shown in US dollars to match the converted +columns; a total that sums different currencies is rendered as a plain number +and labelled as mixed. Tiles are hidden when the API returns no `summary`, which keeps the page working against an API that predates this feature. diff --git a/src/apps/sales/src/SalesPage.module.scss b/src/apps/sales/src/SalesPage.module.scss index 251a3cd03..f651d12ac 100644 --- a/src/apps/sales/src/SalesPage.module.scss +++ b/src/apps/sales/src/SalesPage.module.scss @@ -42,11 +42,8 @@ .filterField input:disabled { background: $tc-2026-border; cursor: not-allowed; } .filterActions { min-height: 48px; } .dateFilters { margin-bottom: 24px; } -.dateFieldset { background: $tc-2026-surface; border: 1px solid $tc-2026-border; border-radius: $tc-2026-radius-lg; box-shadow: $tc-2026-shadow-card; padding: 24px; margin: 0; } -.dateLegend { font-size: $tc-2026-h4-size; line-height: 32px; font-weight: 700; padding: 0; } -.dateHint { margin-top: 4px; color: $tc-2026-muted; max-width: 820px; } -.dateControls { display: flex; align-items: flex-end; flex-wrap: wrap; gap: 16px; margin-top: 20px; } -.dateStatus { margin-top: 16px; font-size: 13px; color: $tc-2026-muted; } +.dateHint { max-width: 820px; } +.dateStatus { flex-basis: 100%; font-size: 13px; color: $tc-2026-muted; } .dateError { color: $tc-2026-danger; font-weight: 700; } .summary { margin-bottom: 24px; display: flex; flex-direction: column; gap: 16px; } .metrics { display: grid; grid-template-columns: repeat(auto-fit, minmax(220px, 1fr)); gap: 16px; } @@ -92,7 +89,7 @@ .page { padding: 20px 12px 40px; } .header h1 { font-size: 36px; line-height: 44px; } .headerActions { width: 100%; justify-content: space-between; } - .panelHeader, .filters, .pagination, .dateFieldset, .metric, .breakdown { padding: 16px; } + .panelHeader, .filters, .pagination, .metric, .breakdown { padding: 16px; } .panelHeader { align-items: flex-start; flex-direction: column; } .filterField { flex-basis: 100%; } .filterActions { width: 100%; } diff --git a/src/apps/sales/src/SalesPage.spec.tsx b/src/apps/sales/src/SalesPage.spec.tsx index d7b652ae8..781e4be8a 100644 --- a/src/apps/sales/src/SalesPage.spec.tsx +++ b/src/apps/sales/src/SalesPage.spec.tsx @@ -1,6 +1,6 @@ /* eslint-disable import/no-extraneous-dependencies, ordered-imports/ordered-imports */ import '@testing-library/jest-dom' -import { act, fireEvent, render, screen, waitFor } from '@testing-library/react' +import { act, fireEvent, render, screen, waitFor, within } from '@testing-library/react' import { ButtonHTMLAttributes, ReactNode } from 'react' import SalesPage from './SalesPage' @@ -71,12 +71,30 @@ function fixture(): SalesReport { sourceRowCount: 30, summary: { amounts: [{ - columnId: 'AMOUNT', + columnId: 'AMOUNT_CONVERTED', count: 28, currencyCode: 'USD', - label: 'Amount', + label: 'Amount (converted)', mixedCurrency: false, total: 1234567, + }, { + columnId: 'AMOUNT', + count: 28, + label: 'Amount', + mixedCurrency: false, + total: 987654, + }, { + columnId: 'EXP_AMOUNT', + count: 28, + label: 'Expected Revenue', + mixedCurrency: false, + total: 555555, + }, { + columnId: 'LOCAL_FEE', + count: 3, + label: 'Local fee', + mixedCurrency: true, + total: 2468, }], groups: [{ amountColumnId: 'AMOUNT', @@ -94,6 +112,12 @@ function fixture(): SalesReport { } } +/** @returns The Clear control of the named section, keeping the two Clear buttons apart. Does not throw. */ +function clearButton(section: string): HTMLElement { + return within(screen.getByRole('region', { name: section })) + .getByRole('button', { name: 'Clear' }) +} + describe('Sales page', () => { beforeEach(() => { fetchOpportunityDetails.mockReset() @@ -213,7 +237,7 @@ describe('Sales page', () => { jest.useRealTimers() }) - it('offers the report date fields, defaults to Created Date and applies an inclusive range', async () => { + it('offers the report date fields, defaults to Created Date and applies a range as it changes', async () => { render() await screen.findByText('Example opportunity') const field = screen.getByLabelText('Filter type') as HTMLSelectElement @@ -226,37 +250,39 @@ describe('Sales page', () => { fireEvent.change(screen.getByLabelText('To date'), { target: { value: '2026-09-30' } }) expect(fetchReport) .toHaveBeenCalledTimes(1) - fireEvent.click(screen.getByRole('button', { name: 'Apply filter' })) + expect(screen.queryByRole('button', { name: 'Apply filter' })).not.toBeInTheDocument() await waitFor(() => expect(fetchReport) .toHaveBeenLastCalledWith(expect.objectContaining({ dateColumn: 'CLOSE_DATE', dateFrom: '2026-07-01', dateTo: '2026-09-30', page: 1, }), expect.any(AbortSignal))) + // The three changes within one pause are sent as a single request. + expect(fetchReport) + .toHaveBeenCalledTimes(2) await screen.findByText(/Showing records by Close Date from 2026-07-01 through 2026-09-30/) }) - it('refuses an inverted range without sending a request and clears the error on reset', async () => { + it('refuses an inverted range without sending a request and clears the error on Clear', async () => { render() await screen.findByText('Example opportunity') fireEvent.change(screen.getByLabelText('From date'), { target: { value: '2026-09-30' } }) fireEvent.change(screen.getByLabelText('To date'), { target: { value: '2026-09-01' } }) - fireEvent.click(screen.getByRole('button', { name: 'Apply filter' })) await screen.findByText('The From date must be on or before the To date.') expect(fetchReport) .toHaveBeenCalledTimes(1) - fireEvent.click(screen.getByRole('button', { name: 'Reset filter' })) + fireEvent.click(clearButton('Date range filter')) await waitFor(() => expect(screen.queryByText('The From date must be on or before the To date.')) .not.toBeInTheDocument()) expect(screen.getByLabelText('From date')) .toHaveValue('') + await screen.findByText('No date range applied.') expect(fetchReport) .toHaveBeenCalledTimes(1) }) - it('resets an applied range and keeps the range when report filters are cleared', async () => { + it('clears an applied range and keeps the range when report filters are cleared', async () => { render() await screen.findByText('Example opportunity') fireEvent.change(screen.getByLabelText('From date'), { target: { value: '2026-09-01' } }) - fireEvent.click(screen.getByRole('button', { name: 'Apply filter' })) await waitFor(() => expect(fetchReport) .toHaveBeenLastCalledWith( expect.objectContaining({ dateColumn: 'CREATED_DATE', dateFrom: '2026-09-01' }), @@ -269,7 +295,7 @@ describe('Sales page', () => { expect.any(AbortSignal), )) // Clearing the report filters must not silently empty the separate date range. - fireEvent.click(screen.getByRole('button', { name: 'Clear' })) + fireEvent.click(clearButton('Sales report')) await waitFor(() => expect(fetchReport) .toHaveBeenLastCalledWith( expect.not.objectContaining({ search: 'Example' }), @@ -280,7 +306,7 @@ describe('Sales page', () => { expect.objectContaining({ dateColumn: 'CREATED_DATE', dateFrom: '2026-09-01' }), expect.any(AbortSignal), ) - fireEvent.click(screen.getByRole('button', { name: 'Reset filter' })) + fireEvent.click(clearButton('Date range filter')) await waitFor(() => expect(fetchReport) .toHaveBeenLastCalledWith( expect.objectContaining({ dateColumn: undefined, dateFrom: undefined, dateTo: undefined }), @@ -289,13 +315,25 @@ describe('Sales page', () => { await screen.findByText('No date range applied.') }) - it('shows totals for every matching record rather than the returned page', async () => { + it('shows totals for every matching record without per-tile record counts', async () => { render() await screen.findByText('Example opportunity') expect(screen.getByText('$1,234,567')) .toBeInTheDocument() - expect(screen.getByText('28 of 30 records with a value')) + // The plain Amount is redundant beside Amount (converted), so its tile is hidden. + expect(screen.queryByText('$987,654')) + .not.toBeInTheDocument() + // An uncoded single-currency total reads in dollars like the converted columns. + expect(screen.getByText('$555,555')) + .toBeInTheDocument() + expect(screen.getByText('2,468')) .toBeInTheDocument() + expect(screen.getByText('Totals mix currencies.')) + .toBeInTheDocument() + expect(screen.queryByText('Matching records')) + .not.toBeInTheDocument() + expect(screen.queryByText(/records with a value/)) + .not.toBeInTheDocument() expect(screen.getByText('Stage breakdown')) .toBeInTheDocument() expect(screen.getByText('18 records')) @@ -315,7 +353,7 @@ describe('Sales page', () => { await screen.findByText('Example opportunity') expect(screen.getByLabelText('Filter type')) .toBeDisabled() - expect(screen.getByRole('button', { name: 'Apply filter' })) + expect(screen.getByLabelText('From date')) .toBeDisabled() expect(screen.queryByText('Opportunities')) .not.toBeInTheDocument() diff --git a/src/apps/sales/src/SalesPage.tsx b/src/apps/sales/src/SalesPage.tsx index adb9e0a9b..40c5b6f60 100644 --- a/src/apps/sales/src/SalesPage.tsx +++ b/src/apps/sales/src/SalesPage.tsx @@ -13,6 +13,7 @@ import { dateColumns, dateRangeError, defaultDateColumn, + displayedAmounts, formatSummaryAmount, withDateRange, } from './sales.utils' @@ -180,13 +181,25 @@ const SalesPage: FC = () => { setDateColumn(defaultDateColumn(availableDates)) }, [availableDates, dateColumn]) - /** @param event Date range submission. @returns Nothing; applies a valid range. Does not throw. */ - function applyDateRange(event: FormEvent): void { - event.preventDefault() + /** Applies the pending range when it is valid, otherwise shows why it cannot be sent. Does not throw. */ + const applyDateRange = useCallback((): void => { const invalid = dateRangeError(dateColumn, dateFrom, dateTo) setDateError(invalid) if (invalid) return setQuery(current => withDateRange(current, dateColumn, dateFrom, dateTo)) + }, [dateColumn, dateFrom, dateTo]) + + useEffect(() => { + // The range applies as its controls change, after the same pause as the + // report filters, so a date typed segment by segment is not sent per keystroke. + const timer = window.setTimeout(applyDateRange, filterDebounceMs) + return () => window.clearTimeout(timer) + }, [applyDateRange]) + + /** @param event Date range submission. @returns Nothing; applies a valid range at once. Does not throw. */ + function submitDateRange(event: FormEvent): void { + event.preventDefault() + applyDateRange() } /** Clears the date range without disturbing search, column filters or sorting. Does not throw. */ @@ -200,6 +213,7 @@ const SalesPage: FC = () => { const rangeApplied = !!query.dateColumn const summary = report?.summary + const shownAmounts = displayedAmounts(summary?.amounts ?? []) const firstRow = report?.total ? (report.page - 1) * report.perPage + 1 : 0 const lastRow = report ? Math.min(report.page * report.perPage, report.total) : 0 const updatedAt = report ? new Date(report.refreshedAt) @@ -257,68 +271,55 @@ const SalesPage: FC = () => { )} -
-
- Date range filter -

- Filter by Created Date for pipeline generation, or by Close Date for revenue - projections. Counts and totals below cover every matching record, not just this page. -

-
-
- - -
-
- - setDateFrom(event.target.value)} - type='date' - value={dateFrom} - /> -
-
- - setDateTo(event.target.value)} - type='date' - value={dateTo} - /> -
-
- - -
+
+
+
+

Date range filter

+

+ Filter by Created Date for pipeline generation, or by Close Date for revenue + projections. Counts and totals below cover every matching record, not just this page. +

+
+
+ +
+ + +
+
+ + setDateFrom(event.target.value)} + type='date' + value={dateFrom} + /> +
+
+ + setDateTo(event.target.value)} + type='date' + value={dateTo} + /> +
+
+

{dateError && {dateError}} @@ -333,8 +334,8 @@ const SalesPage: FC = () => { )} {!dateError && !rangeApplied && No date range applied.}

-
-
+ + {summary && (
@@ -342,17 +343,14 @@ const SalesPage: FC = () => {

Opportunities

{summary.recordCount.toLocaleString()}

-

Matching records

- {summary.amounts.map(amount => ( + {shownAmounts.map(amount => (

{amount.label}

{formatSummaryAmount(amount)}

-

- {`${amount.count.toLocaleString()} of `} - {`${summary.recordCount.toLocaleString()} records with a value`} - {amount.mixedCurrency ? ' ยท totals mixed currencies' : ''} -

+ {amount.mixedCurrency && ( +

Totals mix currencies.

+ )}
))} diff --git a/src/apps/sales/src/sales.utils.spec.ts b/src/apps/sales/src/sales.utils.spec.ts index 9c9e11790..6d00584bb 100644 --- a/src/apps/sales/src/sales.utils.spec.ts +++ b/src/apps/sales/src/sales.utils.spec.ts @@ -1,8 +1,9 @@ -import { SalesReport } from './sales.models' +import { SalesReport, SalesSummaryAmount } from './sales.models' import { dateColumns, dateRangeError, defaultDateColumn, + displayedAmounts, formatSummaryAmount, withDateRange, } from './sales.utils' @@ -73,11 +74,33 @@ describe('Sales date range utilities', () => { .toMatchObject({ dateColumn: undefined, dateFrom: undefined, dateTo: undefined, page: 1 }) }) - it('labels a total with its shared currency and leaves a mixed sum unlabelled', () => { + it('hides a total whose converted counterpart the report also provides', () => { + /** @returns A single-currency total for the given column. Does not throw. */ + const amount = (columnId: string, label: string): SalesSummaryAmount => ({ + columnId, count: 1, label, mixedCurrency: false, total: 1, + }) + + const converted = amount('AMOUNT_CONVERTED', 'Amount (converted)') + const plain = amount('AMOUNT', 'Amount') + const revenue = amount('EXP_AMOUNT', 'Expected Revenue') + const revenueConverted = amount('CONVERTED_REVENUE', 'Expected Revenue (Converted)') + expect(displayedAmounts([converted, plain, revenue, revenueConverted])) + .toEqual([converted, revenueConverted]) + expect(displayedAmounts([plain, revenue])) + .toEqual([plain, revenue]) + expect(displayedAmounts([])) + .toEqual([]) + }) + + it('labels a total with its shared currency, shows an uncoded one in dollars and leaves a mixed sum bare', () => { expect(formatSummaryAmount({ columnId: 'AMOUNT', count: 2, currencyCode: 'USD', label: 'Amount', mixedCurrency: false, total: 1234.56, })) .toBe('$1,235') + expect(formatSummaryAmount({ + columnId: 'EXPECTED_REVENUE', count: 2, label: 'Expected Revenue', mixedCurrency: false, total: 1234.56, + })) + .toBe('$1,235') expect(formatSummaryAmount({ columnId: 'AMOUNT', count: 2, label: 'Amount', mixedCurrency: true, total: 1234.56, })) diff --git a/src/apps/sales/src/sales.utils.ts b/src/apps/sales/src/sales.utils.ts index 6fe8ea710..0f4317f37 100644 --- a/src/apps/sales/src/sales.utils.ts +++ b/src/apps/sales/src/sales.utils.ts @@ -70,16 +70,53 @@ export function withDateRange(current: SalesQuery, column: string, from: string, return { ...current, ...next, page: 1 } } +/** + * Currency shown for a single-currency total the report leaves uncoded. The converted + * columns arrive coded in US dollars while Amount and Expected Revenue arrive uncoded, + * so this keeps every tile reading the same way. + */ +const defaultCurrencyCode = 'USD' + +/** Column ID and label suffixes Salesforce gives a currency column converted to the corporate currency. */ +const convertedIdPattern = /_CONVERTED$/i +const convertedLabelPattern = /\s*\(converted\)$/i + +/** + * Drops each total whose converted counterpart the report also provides, such as Amount beside + * Amount (converted), because the converted total already states the figure in one currency. + * @param amounts Summary totals in report order. + * @returns The totals to show, in the same order; all of them when none has a converted counterpart. + * @throws Does not throw. + */ +export function displayedAmounts(amounts: SalesSummaryAmount[]): SalesSummaryAmount[] { + const converted = new Set() + amounts.forEach(amount => { + if (convertedIdPattern.test(amount.columnId)) { + converted.add(amount.columnId.replace(convertedIdPattern, '') + .toLowerCase()) + } + + if (convertedLabelPattern.test(amount.label)) { + converted.add(amount.label.replace(convertedLabelPattern, '') + .toLowerCase()) + } + }) + return amounts.filter(amount => !converted.has(amount.columnId.toLowerCase()) + && !converted.has(amount.label.toLowerCase())) +} + /** * Formats a snapshot-wide total for display, using the currency the matching rows agree on. * @param amount Summary entry for one numeric column. - * @returns A localized currency amount, or a plain number when the rows mix currencies. + * @returns A localized currency amount, in the shared currency or in US dollars when the + * report leaves a single-currency total uncoded; a plain number when the rows mix currencies. * @throws Does not throw for an unexpected currency code; falls back to a plain number. */ export function formatSummaryAmount(amount: SalesSummaryAmount): string { + const currency = amount.currencyCode ?? (amount.mixedCurrency ? undefined : defaultCurrencyCode) try { - return amount.total.toLocaleString(undefined, amount.currencyCode - ? { currency: amount.currencyCode, maximumFractionDigits: 0, style: 'currency' } + return amount.total.toLocaleString(undefined, currency + ? { currency, maximumFractionDigits: 0, style: 'currency' } : { maximumFractionDigits: 0 }) } catch { return amount.total.toLocaleString(undefined, { maximumFractionDigits: 0 })