From 3e3f98a38923596a52c4e33386c922ea07d58cc5 Mon Sep 17 00:00:00 2001 From: Casey Locker Date: Thu, 20 Aug 2026 09:57:13 -0500 Subject: [PATCH 01/16] fix(sponsor-reports): keep Payment Method applied across grain switches buildPurchaseLinesQuery destructured paymentMethod away and the control was hidden on the line views, so filtering to invoiced orders and clicking Line Items silently widened 22 items to 250. The lines endpoint now supports the filter, so it is carried through at every grain. ref: https://app.clickup.com/t/86bbh5vat --- .../__tests__/sponsor-reports-actions.test.js | 7 +-- .../__tests__/sponsor-reports-query.test.js | 20 ++++++--- src/actions/sponsor-reports-actions.js | 12 +++-- .../__tests__/index.test.js | 18 ++++++-- .../purchase-details-report-page/index.js | 44 +++++++++---------- 5 files changed, 58 insertions(+), 43 deletions(-) diff --git a/src/actions/__tests__/sponsor-reports-actions.test.js b/src/actions/__tests__/sponsor-reports-actions.test.js index 0b5493eba..bb0678ccd 100644 --- a/src/actions/__tests__/sponsor-reports-actions.test.js +++ b/src/actions/__tests__/sponsor-reports-actions.test.js @@ -1233,7 +1233,7 @@ describe("sponsor-reports-actions", () => { const rowC = { item_code: "C1", quantity: 3 }; const page1Summary = { total_orders: 11 }; - it("records the active filters on REQUEST and hits the lines endpoint without paymentMethod", async () => { + it("records the active filters on REQUEST and hits the lines endpoint with paymentMethod", async () => { const store = mockStore(MOCK_STATE); const filters = { sponsorIds: [17], paymentMethod: "Card" }; await store.dispatch(getPurchaseDetailsByItemRows(filters)); @@ -1250,11 +1250,12 @@ describe("sponsor-reports-actions", () => { expect(capturedUrl).toContain( "/summits/42/reports/purchase-details/lines" ); - // buildPurchaseLinesQuery drops paymentMethod (order-level attribute). + // buildPurchaseLinesQuery now carries paymentMethod through (declared on + // the lines filter set via the parent hop). const filterClauses = capturedParams["filter[]"] || []; expect( filterClauses.some((c) => String(c).includes("payment_method")) - ).toBe(false); + ).toBe(true); }); it("bulk-loads all pages (page 1, then the rest in parallel) into one atomic RECEIVE_PURCHASE_DETAILS_BY_ITEM_ROWS", async () => { diff --git a/src/actions/__tests__/sponsor-reports-query.test.js b/src/actions/__tests__/sponsor-reports-query.test.js index 0831d5af7..ee5b2a9fb 100644 --- a/src/actions/__tests__/sponsor-reports-query.test.js +++ b/src/actions/__tests__/sponsor-reports-query.test.js @@ -132,13 +132,19 @@ describe("buildPurchaseLinesQuery", () => { expect(q).not.toHaveProperty("order"); }); - it("drops payment_method — the lines filter set does not support it", () => { - const q = buildPurchaseLinesQuery( - { status: "Paid", paymentMethod: "Invoice" }, - { page: 1, perPage: 10 } - ); - expect(q["filter[]"]).toEqual(["status==Paid"]); - expect(q["filter[]"]).not.toContain("payment_method==Invoice"); + it("carries paymentMethod through to the lines query", () => { + const query = buildPurchaseLinesQuery({ paymentMethod: "invoice" }); + expect(query["filter[]"]).toContain("payment_method==invoice"); + }); + + it("builds the same payment_method clause at both grains", () => { + // the regression: applying a filter and switching grain must not change the set + const orders = buildPurchaseQuery({ paymentMethod: "invoice" })["filter[]"]; + const lines = buildPurchaseLinesQuery({ paymentMethod: "invoice" })[ + "filter[]" + ]; + const clause = (f) => f.find((c) => c.startsWith("payment_method")); + expect(clause(lines)).toEqual(clause(orders)); }); }); diff --git a/src/actions/sponsor-reports-actions.js b/src/actions/sponsor-reports-actions.js index 3fa35df8d..84a0dd0c5 100644 --- a/src/actions/sponsor-reports-actions.js +++ b/src/actions/sponsor-reports-actions.js @@ -259,13 +259,11 @@ export const buildPurchaseQuery = ( // Lines grain: same date expansion, NO order (manifest relies on backend default // ordering). Used by the on-screen lines fetch AND exportPurchaseDetailsLinesCsv. -// The lines endpoint's filter set omits payment_method (it's an order-level -// attribute), so drop it here rather than emit a clause BaseFilter silently -// ignores. The UI also hides the Payment Method control in the Line Items view. -export const buildPurchaseLinesQuery = ( - { paymentMethod: _paymentMethod, ...filters } = {}, - { page, perPage } = {} -) => buildReportQuery({ ...expandDates(filters), page, perPage }); +// payment_method IS supported at line grain (declared on PurchaseLineDetailsFilter +// via the parent hop). It used to be dropped here, which is why applying it and +// switching grain silently widened the result set instead of narrowing it. +export const buildPurchaseLinesQuery = (filters = {}, { page, perPage } = {}) => + buildReportQuery({ ...expandDates(filters), page, perPage }); export const getPurchaseDetailsReport = (filters = {}, pagination = {}) => diff --git a/src/pages/sponsors/sponsor-reports/purchase-details-report-page/__tests__/index.test.js b/src/pages/sponsors/sponsor-reports/purchase-details-report-page/__tests__/index.test.js index 4a7b0f662..d844433bb 100644 --- a/src/pages/sponsors/sponsor-reports/purchase-details-report-page/__tests__/index.test.js +++ b/src/pages/sponsors/sponsor-reports/purchase-details-report-page/__tests__/index.test.js @@ -373,6 +373,17 @@ describe("PurchaseDetailsReportPage", () => { ); }); + it("keeps the Payment Method control visible on the line views", async () => { + renderPage(); + await act(async () => { + fireEvent.click(screen.getByText("sponsor_reports_page.view_line_items")); + }); + // aria-label is T.translate(...), which renders as the raw KEY under Jest + expect( + screen.getByLabelText("sponsor_reports_page.filter_payment_method") + ).toBeInTheDocument(); + }); + it("Line Items CSV export passes the lines slice filters to exportPurchaseDetailsLinesCsv", async () => { // Export reads the applied filters from the lines slice (recorded on REQUEST // in production); seed them directly since the mock store is inert. @@ -575,7 +586,7 @@ describe("PurchaseDetailsReportPage", () => { }); }); - it("hides the Payment Method filter in the By Item view (lines filter set omits it)", async () => { + it("keeps the Payment Method filter visible in the By Item view (lines endpoint honors it via the parent hop)", async () => { const history = createMemoryHistory({ initialEntries: [PAGE_URL] }); renderWithRedux( @@ -587,9 +598,10 @@ describe("PurchaseDetailsReportPage", () => { await act(async () => { fireEvent.click(screen.getByText("sponsor_reports_page.view_by_item")); }); + // aria-label is T.translate(...), which renders as the raw KEY under Jest expect( - document.querySelector("#pd-filter-payment-method") - ).not.toBeInTheDocument(); + screen.getByLabelText("sponsor_reports_page.filter_payment_method") + ).toBeInTheDocument(); }); describe("validation error — snackbar hook", () => { diff --git a/src/pages/sponsors/sponsor-reports/purchase-details-report-page/index.js b/src/pages/sponsors/sponsor-reports/purchase-details-report-page/index.js index aa6df674c..a9afaab62 100644 --- a/src/pages/sponsors/sponsor-reports/purchase-details-report-page/index.js +++ b/src/pages/sponsors/sponsor-reports/purchase-details-report-page/index.js @@ -366,30 +366,28 @@ const PurchaseDetailsReportPage = ({ onChange={(e) => update({ formCode: e.target.value || undefined })} /> - {/* Payment Method is an order-level attribute; only the orders endpoint - filters on it (the lines filter set omits payment_method), so surface - it in the orders view only — mirrors search being view-specific. */} - {view === "orders" && ( - - + - update({ paymentMethod: e.target.value || undefined }) - } - /> - - )} + ) + }} + value={draft.paymentMethod || ""} + options={paymentMethodSelectOptions} + onChange={(e) => + update({ paymentMethod: e.target.value || undefined }) + } + /> + {/* Date pickers keep draft.dateFrom/dateTo as "YYYY-MM-DD"; buildQuery expands them to UTC ISO datetimes. Convert the picker's moment back to "YYYY-MM-DD" at the update boundary so the query contract is unchanged. */} From d545ce5977123143e0480dc96c3b5063c893ae56 Mon Sep 17 00:00:00 2001 From: Casey Locker Date: Thu, 20 Aug 2026 10:16:47 -0500 Subject: [PATCH 02/16] docs(sponsor-reports): fix second stale paymentMethod-drop comment getPurchaseDetailsByItemRows still claimed the lines-grain query drops paymentMethod, but it calls buildPurchaseLinesQuery, which now carries it through. Same false claim as the one already fixed at the query builder itself, missed on the first sweep. ref: https://app.clickup.com/t/86bbh5vat --- src/actions/sponsor-reports-actions.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/actions/sponsor-reports-actions.js b/src/actions/sponsor-reports-actions.js index 84a0dd0c5..57beb45c2 100644 --- a/src/actions/sponsor-reports-actions.js +++ b/src/actions/sponsor-reports-actions.js @@ -514,7 +514,7 @@ export const getPurchaseDetailsByItemRows = guardedDispatch( createAction(REQUEST_PURCHASE_DETAILS_BY_ITEM)({ filters }) ); - // Lines-grain query (drops paymentMethod); one arg → no page/per_page emitted. + // Lines-grain query (carries paymentMethod through); one arg → no page/per_page emitted. const baseQuery = buildPurchaseLinesQuery(filters); const url = `${base(currentSummit.id)}/purchase-details/lines`; const fetchPage = (page) => From 552b4696224f61a050f0fe34cfe876c35298b9aa Mon Sep 17 00:00:00 2001 From: Casey Locker Date: Thu, 20 Aug 2026 10:23:19 -0500 Subject: [PATCH 03/16] fix(sponsor-reports): show the line's own state in the manifest A soft-canceled line leaves its parent order Paid, so the Status column read Paid on a dead row and the per-sponsor chip counted it. Both now read the line. Adds the two freshness columns. ref: https://app.clickup.com/t/86bbh5vat --- .../sponsors/reports/LinesManifestView.js | 43 +++++++++++++++---- .../__tests__/LinesManifestView.test.js | 38 ++++++++++++++++ src/i18n/en.json | 3 ++ 3 files changed, 76 insertions(+), 8 deletions(-) diff --git a/src/components/sponsors/reports/LinesManifestView.js b/src/components/sponsors/reports/LinesManifestView.js index 04a875e34..a4d061a6e 100644 --- a/src/components/sponsors/reports/LinesManifestView.js +++ b/src/components/sponsors/reports/LinesManifestView.js @@ -83,10 +83,16 @@ const bucketLinesBySponsor = (rows = []) => { groups.push({ sponsorId: id, sponsorName: row.sponsor?.name ?? "", - lines: [] + lines: [], + // Canceled lines still RENDER (struck through) but must not be counted — + // the chip means live lines, matching the By Item units chip on the same + // screen, which already excludes them. + liveLineCount: 0 }); } - groups[indexByKey.get(key)].lines.push(row); + const group = groups[indexByKey.get(key)]; + group.lines.push(row); + if (!row.is_canceled) group.liveLineCount += 1; }); return groups; }; @@ -102,7 +108,9 @@ const HEADERS = [ { key: "col_quantity", align: "right" }, { key: "col_used_rate" }, { key: "col_status" }, - { key: "col_line_total", align: "right" } + { key: "col_line_total", align: "right" }, + { key: "col_synced_at" }, + { key: "col_source_updated" } ]; const LinesManifestView = ({ @@ -126,7 +134,7 @@ const LinesManifestView = ({ size="small" sx={{ ml: 1.5 }} label={T.translate("sponsor_reports_page.lines_count", { - count: group.lines.length + count: group.liveLineCount })} /> @@ -178,16 +186,35 @@ const LinesManifestView = ({ {line.quantity} {line.rate_name} - + {/* The LINE's state, not the parent order's. A soft-canceled + line leaves its order Paid, so rendering purchase.status + printed "Paid" on a dead row — and the strikethrough that + was the only other signal does not survive CSV export. */} + {line.is_canceled ? ( + + ) : ( + + )} {line.line_total == null ? "—" : currencyAmountFromCents(line.line_total)} + + {formatCheckoutTime(line.synced_at)} + + + {formatCheckoutTime(line.source_updated_at)} + ))} diff --git a/src/components/sponsors/reports/__tests__/LinesManifestView.test.js b/src/components/sponsors/reports/__tests__/LinesManifestView.test.js index 81ded0b53..ccd9cc7f5 100644 --- a/src/components/sponsors/reports/__tests__/LinesManifestView.test.js +++ b/src/components/sponsors/reports/__tests__/LinesManifestView.test.js @@ -1,5 +1,6 @@ import "@testing-library/jest-dom"; import React from "react"; +import moment from "moment-timezone"; import { render, screen, within } from "@testing-library/react"; import LinesManifestView from "../LinesManifestView"; @@ -56,6 +57,43 @@ describe("LinesManifestView", () => { expect(row).toHaveAttribute("data-canceled", "true"); }); + it("renders the LINE's own state, not the parent order's status", () => { + // line() defaults to a Paid parent: the exact trap. A soft-canceled line leaves + // its order Paid, so rendering purchase.status printed "Paid" on a dead row. + renderView({ rows: [line({ is_canceled: true })] }); + expect( + screen.getByText("sponsor_reports_page.status_canceled") + ).toBeInTheDocument(); + expect(screen.queryByText("Paid")).not.toBeInTheDocument(); + }); + + it("renders both freshness timestamps", () => { + // formatCheckoutTime is moment.unix(v).utc().format("YYYY-MM-DD h:mm A") + const synced = 1755561600; + renderView({ + rows: [line({ synced_at: synced, source_updated_at: synced })] + }); + expect( + screen.getAllByText(moment.unix(synced).utc().format("YYYY-MM-DD h:mm A")) + ).not.toHaveLength(0); + }); + + it("counts only live lines per sponsor group", () => { + // Canceled lines still RENDER (struck through); the chip means LIVE lines, + // matching the By Item units chip on the same screen. The module's local + // i18n mock interpolates {count}, so the chip text carries the number. + renderView({ + rows: [ + line({ is_canceled: false }), + line({ is_canceled: true, item_code: "AV2" }) + ] + }); + expect(screen.getAllByRole("row").length).toBeGreaterThan(2); // both lines render + expect( + screen.getByText("sponsor_reports_page.lines_count:1") + ).toBeInTheDocument(); + }); + // Sponsor bucketing (formerly bucketLinesBySponsor, now a private helper). describe("sponsor bucketing", () => { it("groups by sponsor.id preserving first-seen order", () => { diff --git a/src/i18n/en.json b/src/i18n/en.json index 58eb3a2c0..4f9115a20 100644 --- a/src/i18n/en.json +++ b/src/i18n/en.json @@ -4360,6 +4360,7 @@ "status_completed": "Completed", "status_in_progress": "In Progress", "status_pending": "Pending", + "status_canceled": "Canceled", "filter_asset_status": "Status", "group_by": "Group by", "report_filters": "Report Filters", @@ -4394,6 +4395,8 @@ "col_invoice_sub_status": "Invoice Status", "col_invoice_due_date": "Invoice Due", "col_line_total": "Line Total", + "col_synced_at": "Synced", + "col_source_updated": "Source Updated", "byitem_col_orders": "Orders", "byitem_col_total": "Total", "byitem_sponsor_items_chip": "{purchased} of {items} items purchased", From dbc55211f32f279cf02e0f56ccecacef4c5774c2 Mon Sep 17 00:00:00 2001 From: Casey Locker Date: Thu, 20 Aug 2026 10:31:53 -0500 Subject: [PATCH 04/16] test(sponsor-reports): strengthen manifest freshness and live-count assertions The timestamps test used the same epoch for both fields and asserted only "at least one match", so it couldn't catch a swapped or missing cell. Now uses two distinct values and pins them to the row's final two cells. The live-count test asserted row count loosely (>2) without proving which rows rendered; now pins the exact count and asserts the canceled row's identity. ref: https://app.clickup.com/t/86bbh5vat --- .../__tests__/LinesManifestView.test.js | 28 +++++++++++++------ 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/src/components/sponsors/reports/__tests__/LinesManifestView.test.js b/src/components/sponsors/reports/__tests__/LinesManifestView.test.js index ccd9cc7f5..d840a2bc1 100644 --- a/src/components/sponsors/reports/__tests__/LinesManifestView.test.js +++ b/src/components/sponsors/reports/__tests__/LinesManifestView.test.js @@ -67,15 +67,24 @@ describe("LinesManifestView", () => { expect(screen.queryByText("Paid")).not.toBeInTheDocument(); }); - it("renders both freshness timestamps", () => { - // formatCheckoutTime is moment.unix(v).utc().format("YYYY-MM-DD h:mm A") - const synced = 1755561600; + it("renders both freshness timestamps, in the row's final two cells", () => { + // formatCheckoutTime is moment.unix(v).utc().format("YYYY-MM-DD h:mm A"). + // Distinct values (not the same epoch for both fields) so a swap or a + // missing cell can't hide behind a shared string. + const synced = 1755561600; // 2025-08-19 + const sourceUpdated = 1755648000; // 2025-08-20 renderView({ - rows: [line({ synced_at: synced, source_updated_at: synced })] + rows: [line({ synced_at: synced, source_updated_at: sourceUpdated })] }); - expect( - screen.getAllByText(moment.unix(synced).utc().format("YYYY-MM-DD h:mm A")) - ).not.toHaveLength(0); + const syncedText = moment.unix(synced).utc().format("YYYY-MM-DD h:mm A"); + const sourceUpdatedText = moment + .unix(sourceUpdated) + .utc() + .format("YYYY-MM-DD h:mm A"); + const row = screen.getByText("AV1").closest("tr"); + const cells = within(row).getAllByRole("cell"); + expect(cells[cells.length - 2]).toHaveTextContent(syncedText); + expect(cells[cells.length - 1]).toHaveTextContent(sourceUpdatedText); }); it("counts only live lines per sponsor group", () => { @@ -88,7 +97,10 @@ describe("LinesManifestView", () => { line({ is_canceled: true, item_code: "AV2" }) ] }); - expect(screen.getAllByRole("row").length).toBeGreaterThan(2); // both lines render + // header row + the 2 line rows, exactly — proves neither line was dropped. + expect(screen.getAllByRole("row")).toHaveLength(3); + const canceledRow = screen.getByText("AV2").closest("tr"); + expect(canceledRow).toHaveAttribute("data-canceled", "true"); expect( screen.getByText("sponsor_reports_page.lines_count:1") ).toBeInTheDocument(); From 504627574a317a254e42a92166069d87f86ad222 Mon Sep 17 00:00:00 2001 From: Casey Locker Date: Thu, 20 Aug 2026 10:50:27 -0500 Subject: [PATCH 05/16] fix(sponsor-reports): By Item drill-down shows line state and freshness The drill-down rendered the parent order's status on a canceled line, and the group chip described deliberately-filtered canceled orders as 'purchased'. Aggregates are unchanged (decision 8) - the wording is not. ref: https://app.clickup.com/t/86bbh5vat Co-Authored-By: Claude --- src/components/sponsors/reports/ByItemView.js | 34 ++++++-- .../reports/__tests__/ByItemView.test.js | 78 ++++++++++++++++++- src/i18n/en.json | 2 +- 3 files changed, 106 insertions(+), 8 deletions(-) diff --git a/src/components/sponsors/reports/ByItemView.js b/src/components/sponsors/reports/ByItemView.js index 1f644ef72..4c867a36e 100644 --- a/src/components/sponsors/reports/ByItemView.js +++ b/src/components/sponsors/reports/ByItemView.js @@ -119,7 +119,10 @@ const accumulateRow = (itemMap, row) => { status: row.purchase?.status ?? "", qty: row.quantity ?? 0, lineTotalCents: row.line_total ?? null, - isCanceled: Boolean(row.is_canceled) + isCanceled: Boolean(row.is_canceled), + // line-grain freshness (decision 1): the contributor row IS a line + syncedAt: row.synced_at ?? null, + sourceUpdatedAt: row.source_updated_at ?? null }); }; @@ -250,7 +253,9 @@ const CONTRIB_HEADERS = [ { key: "col_used_rate" }, { key: "col_status" }, { key: "col_quantity", align: "right" }, - { key: "col_line_total", align: "right" } + { key: "col_line_total", align: "right" }, + { key: "col_synced_at" }, + { key: "col_source_updated" } ]; // One expansion key per (sponsor, item) so the same item code under two @@ -457,10 +462,21 @@ const ItemTable = ({ {c.rateName} - + {/* the line's own state — its parent order can be Paid + while this specific line is canceled */} + {c.isCanceled ? ( + + ) : ( + + )} {c.qty} @@ -468,6 +484,12 @@ const ItemTable = ({ ? "—" : currencyAmountFromCents(c.lineTotalCents)} + + {formatCheckoutTime(c.syncedAt)} + + + {formatCheckoutTime(c.sourceUpdatedAt)} + ))} diff --git a/src/components/sponsors/reports/__tests__/ByItemView.test.js b/src/components/sponsors/reports/__tests__/ByItemView.test.js index 666df5c5b..7d01e3348 100644 --- a/src/components/sponsors/reports/__tests__/ByItemView.test.js +++ b/src/components/sponsors/reports/__tests__/ByItemView.test.js @@ -1,5 +1,6 @@ import "@testing-library/jest-dom"; import React from "react"; +import moment from "moment-timezone"; import { render, screen, fireEvent, within } from "@testing-library/react"; import ByItemView, { groupLinesByItem, @@ -160,10 +161,21 @@ describe("groupLinesBySponsorItem", () => { status: "Paid", qty: 2, lineTotalCents: 100000, - isCanceled: true + isCanceled: true, + syncedAt: null, + sourceUpdatedAt: null }); }); + it("carries line-grain freshness into the contributor", () => { + const [g] = groupLinesBySponsorItem([ + line({ synced_at: 1755561600, source_updated_at: 1755558000 }) + ]); + const [c] = g.items[0].contributors; + expect(c.syncedAt).toBe(1755561600); + expect(c.sourceUpdatedAt).toBe(1755558000); + }); + it("EXCLUDES canceled lines from qty/money/purchasedCount/Σqty but keeps them as contributors", () => { const rows = [ line({ @@ -415,6 +427,52 @@ describe("ByItemView", () => { expect(screen.queryByText("OCP-1")).not.toBeInTheDocument(); }); + it("renders the line's own state in the drill-down, not the parent order's", () => { + // Distinct epochs (not the same value for both fields) so a swap or a + // missing cell can't hide behind a shared string. + const synced = 1755561600; // 2025-08-19 + const sourceUpdated = 1755648000; // 2025-08-20 + renderView({ + groups: [ + group({ + items: [ + item({ + contributors: [ + { + sponsorName: "FNTECH", + number: "OCP-1", + formCode: "AV", + addOnName: null, + checkoutAt: null, + rateName: "Early", + status: "Paid", // parent order is Paid... + qty: 1, + lineTotalCents: 1000, + isCanceled: true, // ...but THIS line is canceled + syncedAt: synced, + sourceUpdatedAt: sourceUpdated + } + ] + }) + ] + }) + ] + }); + fireEvent.click(screen.getByText("AV1")); // expand the item + expect( + screen.getByText("sponsor_reports_page.status_canceled") + ).toBeInTheDocument(); + const syncedText = moment.unix(synced).utc().format("YYYY-MM-DD h:mm A"); + const sourceUpdatedText = moment + .unix(sourceUpdated) + .utc() + .format("YYYY-MM-DD h:mm A"); + const row = screen.getByText("OCP-1").closest("tr"); + const cells = within(row).getAllByRole("cell"); + expect(cells[cells.length - 2]).toHaveTextContent(syncedText); + expect(cells[cells.length - 1]).toHaveTextContent(sourceUpdatedText); + }); + it("expand button toggles the drill-down and reflects aria-expanded", () => { renderView(); const toggle = screen.getByRole("button", { @@ -484,6 +542,24 @@ describe("ByItemView", () => { }); }); +describe("byitem_sponsor_items_chip copy", () => { + it("does not describe canceled orders as purchased", () => { + // The chip read "13 of 13 items purchased" over a set of deliberately-filtered + // CANCELED orders. Under Jest the chip renders as its i18n key, so the wording + // is only checkable in the catalog. + // eslint-disable-next-line global-require + const en = require("../../../../i18n/en.json"); + // Not a bare /purchased/: the template's own {purchased} interpolation + // key would self-match that. Target the retired descriptive phrase. + expect(en.sponsor_reports_page.byitem_sponsor_items_chip).not.toMatch( + /items purchased/ + ); + expect(en.sponsor_reports_page.byitem_sponsor_items_chip).toContain( + "{purchased} of {items}" + ); + }); +}); + describe("sortItems", () => { const row = (over) => item(over); diff --git a/src/i18n/en.json b/src/i18n/en.json index 4f9115a20..2cfdb0bbc 100644 --- a/src/i18n/en.json +++ b/src/i18n/en.json @@ -4399,7 +4399,7 @@ "col_source_updated": "Source Updated", "byitem_col_orders": "Orders", "byitem_col_total": "Total", - "byitem_sponsor_items_chip": "{purchased} of {items} items purchased", + "byitem_sponsor_items_chip": "{purchased} of {items} items with purchases", "byitem_sum_qty": "{qty} units", "byitem_contributing_orders": "Contributing orders", "byitem_sponsors_per_page": "Sponsors per page:", From d7885eaabef1e8f3297de98ebc8a2f9e474a54f2 Mon Sep 17 00:00:00 2001 From: Casey Locker Date: Thu, 20 Aug 2026 11:01:28 -0500 Subject: [PATCH 06/16] test(sponsor-reports): pin By Item chip copy and drill-down column parity Exact-equality on the chip catalog value closes the gap where a substring guard could pass a reworded defect or unrelated copy. A new header/cell cardinality check on the nested contributor table catches a header array desync that a positional last-two-cells check alone would miss, in both the by-sponsor (10 cols) and all-sponsors (11 cols) layouts. ref: https://app.clickup.com/t/86bbh5vat --- .../reports/__tests__/ByItemView.test.js | 42 +++++++++++++++---- 1 file changed, 33 insertions(+), 9 deletions(-) diff --git a/src/components/sponsors/reports/__tests__/ByItemView.test.js b/src/components/sponsors/reports/__tests__/ByItemView.test.js index 7d01e3348..137672440 100644 --- a/src/components/sponsors/reports/__tests__/ByItemView.test.js +++ b/src/components/sponsors/reports/__tests__/ByItemView.test.js @@ -545,17 +545,16 @@ describe("ByItemView", () => { describe("byitem_sponsor_items_chip copy", () => { it("does not describe canceled orders as purchased", () => { // The chip read "13 of 13 items purchased" over a set of deliberately-filtered - // CANCELED orders. Under Jest the chip renders as its i18n key, so the wording - // is only checkable in the catalog. + // CANCELED orders. Under Jest the chip renders as its i18n key, so the + // wording is only checkable in the catalog — and only by exact equality: + // the template's own {purchased} interpolation key contains the substring + // "purchased", so a substring/regex guard either self-matches the + // placeholder or is too loose to pin the actual sentence (e.g. it would + // let "{purchased} of {items} purchased" or "...items shipped" through). // eslint-disable-next-line global-require const en = require("../../../../i18n/en.json"); - // Not a bare /purchased/: the template's own {purchased} interpolation - // key would self-match that. Target the retired descriptive phrase. - expect(en.sponsor_reports_page.byitem_sponsor_items_chip).not.toMatch( - /items purchased/ - ); - expect(en.sponsor_reports_page.byitem_sponsor_items_chip).toContain( - "{purchased} of {items}" + expect(en.sponsor_reports_page.byitem_sponsor_items_chip).toBe( + "{purchased} of {items} items with purchases" ); }); }); @@ -918,6 +917,31 @@ describe("ByItemView all-sponsors layout", () => { ); expect(onLayoutChange).not.toHaveBeenCalled(); }); + + // A positional last-two-cells check (as used elsewhere for the freshness + // columns) only proves a cell isn't missing — it says nothing about the + // HEADER row, which is a separate array (CONTRIB_HEADERS, optionally + // prepended with col_sponsor). If the two desync by one, every column right + // of the break silently misaligns and stays green. Assert exact + // header/cell cardinality on the NESTED contributor table specifically — + // the outer item table has its own separate ITEM_HEADERS and expansion + // colSpan that must not be counted here. + it.each([ + ["by-sponsor", 10, () => renderView()], + ["all-sponsors", 11, () => renderAll()] + ])( + "the %s drill-down has %i headers matching %i cells per row", + (_name, count, mount) => { + mount(); + fireEvent.click(screen.getByText("AV1")); + const contribTable = screen.getAllByRole("table").at(-1); + expect(within(contribTable).getAllByRole("columnheader")).toHaveLength( + count + ); + const row = within(contribTable).getByText("OCP-1").closest("tr"); + expect(within(row).getAllByRole("cell")).toHaveLength(count); + } + ); }); describe("Destination booth fallback (By Item drill-down)", () => { From 1d95ea38a65a61720fa640e43b688c1aa09e1059 Mon Sep 17 00:00:00 2001 From: Casey Locker Date: Thu, 20 Aug 2026 11:26:11 -0500 Subject: [PATCH 07/16] feat(sponsor-reports): surface refunded amount and freshness on Orders A refund never changes purchase status, so a fully refunded order read as a clean Paid on screen; the CSV already carried the field. ref: https://app.clickup.com/t/86bbh5vat Co-Authored-By: Claude --- .../sponsors/reports/OrdersTable.js | 23 ++++++++ .../reports/__tests__/OrdersTable.test.js | 54 +++++++++++++++++-- src/i18n/en.json | 1 + 3 files changed, 73 insertions(+), 5 deletions(-) diff --git a/src/components/sponsors/reports/OrdersTable.js b/src/components/sponsors/reports/OrdersTable.js index bb0d6b08b..c049eec50 100644 --- a/src/components/sponsors/reports/OrdersTable.js +++ b/src/components/sponsors/reports/OrdersTable.js @@ -135,6 +135,29 @@ const columns = [ header: T.translate("sponsor_reports_page.col_sponsor_note"), sortable: false // not a backend ordering field // No render — MuiTable fallback reads row["sponsor_note"] directly. + }, + { + columnKey: "refunded_amount", + header: T.translate("sponsor_reports_page.col_refunded"), + sortable: false, // not a backend ordering field + // Refunds never touch purchase status — a fully refunded order reads Paid + // forever, so this column is the only on-screen signal. The CSV already had it. + render: (row) => + row.refunded_amount == null + ? "—" + : currencyAmountFromCents(row.refunded_amount) + }, + { + columnKey: "synced_at", + header: T.translate("sponsor_reports_page.col_synced_at"), + sortable: false, // not a backend ordering field + render: (row) => formatCheckoutTime(row.synced_at) + }, + { + columnKey: "source_updated_at", + header: T.translate("sponsor_reports_page.col_source_updated"), + sortable: false, // not a backend ordering field + render: (row) => formatCheckoutTime(row.source_updated_at) } ]; diff --git a/src/components/sponsors/reports/__tests__/OrdersTable.test.js b/src/components/sponsors/reports/__tests__/OrdersTable.test.js index 42851715d..c83ba5acb 100644 --- a/src/components/sponsors/reports/__tests__/OrdersTable.test.js +++ b/src/components/sponsors/reports/__tests__/OrdersTable.test.js @@ -1,7 +1,8 @@ // src/components/sponsors/reports/__tests__/OrdersTable.test.js import "@testing-library/jest-dom"; import React from "react"; -import { render, screen, fireEvent } from "@testing-library/react"; +import moment from "moment-timezone"; +import { render, screen, fireEvent, within } from "@testing-library/react"; import OrdersTable from "../OrdersTable"; // MuiTable uses i18n-react internally (no-items message, pagination labels). @@ -70,12 +71,55 @@ describe("OrdersTable finance columns", () => { payment_method: null, invoice_reference: null, invoice_sub_status: null, - invoice_due_date: null + invoice_due_date: null, + refunded_amount: null }; renderTable([nullRow]); - // invoice_total (25000) still renders $250.00; the four finance cells render —. - // Exactly four em-dash cells appear (one per null finance column). - expect(screen.getAllByText("—")).toHaveLength(4); + // invoice_total (25000) still renders $250.00; the five finance cells render —. + // Exactly five em-dash cells appear (one per null finance column, now + // including refunded_amount added in Task 10). + expect(screen.getAllByText("—")).toHaveLength(5); + }); +}); + +describe("OrdersTable refund and freshness columns", () => { + it("surfaces refunded_amount, which never changes purchase status", () => { + // a refund leaves the order Paid forever; refunded_amount is the only signal + renderTable([{ ...sampleRow, refunded_amount: 100 }]); + expect(screen.getByText("$1.00")).toBeInTheDocument(); + }); + + it("renders both freshness timestamps, in the row's final two cells", () => { + // formatCheckoutTime is moment.unix(v).utc().format("YYYY-MM-DD h:mm A"). + // Distinct values (not the same epoch for both fields) so a swap or a + // missing cell can't hide behind a shared string. + const synced = 1755561600; // 2025-08-19 + const sourceUpdated = 1755648000; // 2025-08-20 + renderTable([ + { ...sampleRow, synced_at: synced, source_updated_at: sourceUpdated } + ]); + const syncedText = moment.unix(synced).utc().format("YYYY-MM-DD h:mm A"); + const sourceUpdatedText = moment + .unix(sourceUpdated) + .utc() + .format("YYYY-MM-DD h:mm A"); + const row = screen.getByText(sampleRow.purchase_number).closest("tr"); + const cells = within(row).getAllByRole("cell"); + expect(cells[cells.length - 2]).toHaveTextContent(syncedText); + expect(cells[cells.length - 1]).toHaveTextContent(sourceUpdatedText); + }); + + it("appends the new columns without disturbing the existing header order", () => { + renderTable(); + const headers = screen + .getAllByRole("columnheader") + .map((h) => h.textContent); + expect(headers).toHaveLength(14); + expect(headers.slice(-3)).toEqual([ + "sponsor_reports_page.col_refunded", + "sponsor_reports_page.col_synced_at", + "sponsor_reports_page.col_source_updated" + ]); }); }); diff --git a/src/i18n/en.json b/src/i18n/en.json index 2cfdb0bbc..62138d4dd 100644 --- a/src/i18n/en.json +++ b/src/i18n/en.json @@ -4395,6 +4395,7 @@ "col_invoice_sub_status": "Invoice Status", "col_invoice_due_date": "Invoice Due", "col_line_total": "Line Total", + "col_refunded": "Refunded", "col_synced_at": "Synced", "col_source_updated": "Source Updated", "byitem_col_orders": "Orders", From 3afa8b0d57b6d6d613c62b72858315739f7d5283 Mon Sep 17 00:00:00 2001 From: Casey Locker Date: Thu, 20 Aug 2026 11:38:48 -0500 Subject: [PATCH 08/16] fix(sponsor-reports): right-align refunded amount, pin full Orders header order refunded_amount rendered left-aligned while invoice_total (the table's other currency column) is right-aligned. Also strengthened the header-order test to assert the complete 14-key sequence instead of only the appended tail, so a reorder among the original 11 columns is caught too. ref: https://app.clickup.com/t/86bbh5vat Co-Authored-By: Claude --- src/components/sponsors/reports/OrdersTable.js | 1 + .../reports/__tests__/OrdersTable.test.js | 16 ++++++++++++++-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/components/sponsors/reports/OrdersTable.js b/src/components/sponsors/reports/OrdersTable.js index c049eec50..6cacd9ded 100644 --- a/src/components/sponsors/reports/OrdersTable.js +++ b/src/components/sponsors/reports/OrdersTable.js @@ -140,6 +140,7 @@ const columns = [ columnKey: "refunded_amount", header: T.translate("sponsor_reports_page.col_refunded"), sortable: false, // not a backend ordering field + align: "right", // Refunds never touch purchase status — a fully refunded order reads Paid // forever, so this column is the only on-screen signal. The CSV already had it. render: (row) => diff --git a/src/components/sponsors/reports/__tests__/OrdersTable.test.js b/src/components/sponsors/reports/__tests__/OrdersTable.test.js index c83ba5acb..94c089dcc 100644 --- a/src/components/sponsors/reports/__tests__/OrdersTable.test.js +++ b/src/components/sponsors/reports/__tests__/OrdersTable.test.js @@ -114,8 +114,20 @@ describe("OrdersTable refund and freshness columns", () => { const headers = screen .getAllByRole("columnheader") .map((h) => h.textContent); - expect(headers).toHaveLength(14); - expect(headers.slice(-3)).toEqual([ + // Full 14-key sequence, not just the appended tail — a reorder or swap + // among the original 11 (e.g. col_order/col_sponsor) must fail this too. + expect(headers).toEqual([ + "sponsor_reports_page.col_order", + "sponsor_reports_page.col_sponsor", + "sponsor_reports_page.col_checkout_time", + "sponsor_reports_page.col_type", + "sponsor_reports_page.col_status", + "sponsor_reports_page.col_invoice_total", + "sponsor_reports_page.col_payment_method", + "sponsor_reports_page.col_invoice_reference", + "sponsor_reports_page.col_invoice_sub_status", + "sponsor_reports_page.col_invoice_due_date", + "sponsor_reports_page.col_sponsor_note", "sponsor_reports_page.col_refunded", "sponsor_reports_page.col_synced_at", "sponsor_reports_page.col_source_updated" From c8562c96613fff665503c4301f1caaea2db142d5 Mon Sep 17 00:00:00 2001 From: Casey Locker Date: Thu, 20 Aug 2026 11:56:58 -0500 Subject: [PATCH 09/16] fix(sponsor-reports): label the gross total and the canceled default Total Paid is gross and sat unlabelled beside Total Refunded, whose dollar is inside it. Canceled orders are excluded by default at both grains and nothing in the UI said so. ref: https://app.clickup.com/t/86bbh5vat --- src/i18n/en.json | 3 +- .../__tests__/index.test.js | 31 +++++++++++++++++++ .../purchase-details-report-page/index.js | 7 ++++- 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/src/i18n/en.json b/src/i18n/en.json index 62138d4dd..11e5c0ccb 100644 --- a/src/i18n/en.json +++ b/src/i18n/en.json @@ -4336,10 +4336,11 @@ "download_csv": "Download CSV", "total_orders": "Total Sales", "total_items": "Total Items", - "total_paid": "Total Paid", + "total_paid": "Total Paid (before refunds)", "total_pending": "Total Pending", "total_refunded": "Total Refunded", "filter_status": "Purchase Status", + "filter_status_canceled_note": "Canceled orders are excluded unless you select Canceled", "filter_form": "Type", "filter_payment_method": "Payment Method", "any": "Any", diff --git a/src/pages/sponsors/sponsor-reports/purchase-details-report-page/__tests__/index.test.js b/src/pages/sponsors/sponsor-reports/purchase-details-report-page/__tests__/index.test.js index d844433bb..54801a322 100644 --- a/src/pages/sponsors/sponsor-reports/purchase-details-report-page/__tests__/index.test.js +++ b/src/pages/sponsors/sponsor-reports/purchase-details-report-page/__tests__/index.test.js @@ -611,4 +611,35 @@ describe("PurchaseDetailsReportPage", () => { expect(mockErrorMessage).toHaveBeenCalledWith("Too many filters"); }); }); + + describe("self-describing chrome — gross Total Paid label + canceled default note", () => { + it("labels Total Paid as gross", () => { + // Decision 7 keeps the figure gross; the label is the whole fix. Under Jest the + // tile renders "sponsor_reports_page.total_paid" (the i18n mock is the identity + // function), so the catalog value is the only place this copy is checkable — + // and exact equality (not a substring match) is the only assertion that can + // both catch a wrong replacement string and notice "Total Paid" going missing. + const en = require("../../../../../i18n/en.json"); + expect(en.sponsor_reports_page.total_paid).toBe( + "Total Paid (before refunds)" + ); + }); + + it("still renders the gross figure unchanged beside Total Refunded", () => { + // currencyAmountFromCents (openstack-uicore-foundation) has no thousands + // separator — verified directly against the installed lib, so this is + // "$13297.00", not "$13,297.00". Out of scope to change here; decision 7 + // only requires the gross figure itself stay untouched. + renderPage({ total_paid: 1329700, total_refunded: 100 }); + expect(screen.getByText("$13297.00")).toBeInTheDocument(); + expect(screen.getByText("$1.00")).toBeInTheDocument(); + }); + + it("states the silent canceled default next to the status control", () => { + renderPage(); + expect( + screen.getByText("sponsor_reports_page.filter_status_canceled_note") + ).toBeInTheDocument(); + }); + }); }); diff --git a/src/pages/sponsors/sponsor-reports/purchase-details-report-page/index.js b/src/pages/sponsors/sponsor-reports/purchase-details-report-page/index.js index a9afaab62..7666ba271 100644 --- a/src/pages/sponsors/sponsor-reports/purchase-details-report-page/index.js +++ b/src/pages/sponsors/sponsor-reports/purchase-details-report-page/index.js @@ -16,7 +16,7 @@ import { connect } from "react-redux"; import { withRouter } from "react-router-dom"; import moment from "moment-timezone"; import T from "i18n-react/dist/i18n-react"; -import { Alert, Box, Button } from "@mui/material"; +import { Alert, Box, Button, FormHelperText } from "@mui/material"; import PrintIcon from "@mui/icons-material/Print"; import DownloadIcon from "@mui/icons-material/Download"; import ShoppingCartOutlinedIcon from "@mui/icons-material/ShoppingCartOutlined"; @@ -352,6 +352,11 @@ const PurchaseDetailsReportPage = ({ options={statusSelectOptions} onChange={(e) => update({ status: e.target.value || undefined })} /> + {/* The default excludes canceled orders at BOTH grains and nothing said so, + so "Any" silently means paid plus pending. */} + + {T.translate("sponsor_reports_page.filter_status_canceled_note")} + Date: Thu, 20 Aug 2026 12:09:22 -0500 Subject: [PATCH 10/16] fix(sponsor-reports): flow the status helper text in-page instead of drifting CustomTheme's global MuiFormHelperText override makes helper text position: absolute, anchored to the nearest positioned ancestor. The new note is a sibling of MuiDropdown, not a child of its internal FormControl, so it had no positioned ancestor and drifted to the initial containing block instead of sitting under the dropdown. Override back to static on this instance so it renders in normal flow and reserves its own height. ref: https://app.clickup.com/t/86bbh5vat --- .../purchase-details-report-page/index.js | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/pages/sponsors/sponsor-reports/purchase-details-report-page/index.js b/src/pages/sponsors/sponsor-reports/purchase-details-report-page/index.js index 7666ba271..b2bb6c040 100644 --- a/src/pages/sponsors/sponsor-reports/purchase-details-report-page/index.js +++ b/src/pages/sponsors/sponsor-reports/purchase-details-report-page/index.js @@ -353,8 +353,15 @@ const PurchaseDetailsReportPage = ({ onChange={(e) => update({ status: e.target.value || undefined })} /> {/* The default excludes canceled orders at BOTH grains and nothing said so, - so "Any" silently means paid plus pending. */} - + so "Any" silently means paid plus pending. + CustomTheme's global MuiFormHelperText override makes this + position: absolute anchored to the nearest positioned ancestor — + MuiDropdown's internal FormControl (position: relative), not this + Box. Since this FormHelperText is a sibling, not a FormControl + child, it has no positioned ancestor and drifts to the initial + containing block. Override back to static so it flows in normal + layout right under the dropdown, in-flow (no overlap/spill). */} + {T.translate("sponsor_reports_page.filter_status_canceled_note")} From 5eb2fa97d39efc170b285d9ff3a019575d370984 Mon Sep 17 00:00:00 2001 From: Casey Locker Date: Thu, 20 Aug 2026 12:36:13 -0500 Subject: [PATCH 11/16] fix(sponsor-reports): reword lines chip, add header/cell guard, fix test title MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The lines_count chip still said "{count} lines" even though canceled lines render (struck through) but aren't counted, so a group showing 2 rows could read "1 lines" — reworded to "{count} live lines" to match the byitem chip's phrasing. LinesManifestView had no columnheader-count-vs-cell-count assertion, so a HEADERS/TableCell desync would misalign columns without failing any test; added the same guard ByItemView already has for its contributor table. The ByItemView it.each drill-down test title formatted the mount function as %i, rendering "NaN cells per row" in CI output; reworded to use only the arguments actually referenced. ref: https://app.clickup.com/t/86bbh5vat Co-Authored-By: Claude --- .../sponsors/reports/__tests__/ByItemView.test.js | 2 +- .../reports/__tests__/LinesManifestView.test.js | 12 ++++++++++++ src/i18n/en.json | 2 +- 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/components/sponsors/reports/__tests__/ByItemView.test.js b/src/components/sponsors/reports/__tests__/ByItemView.test.js index 137672440..dcd806eba 100644 --- a/src/components/sponsors/reports/__tests__/ByItemView.test.js +++ b/src/components/sponsors/reports/__tests__/ByItemView.test.js @@ -930,7 +930,7 @@ describe("ByItemView all-sponsors layout", () => { ["by-sponsor", 10, () => renderView()], ["all-sponsors", 11, () => renderAll()] ])( - "the %s drill-down has %i headers matching %i cells per row", + "the %s drill-down has %i headers matching that many cells per row", (_name, count, mount) => { mount(); fireEvent.click(screen.getByText("AV1")); diff --git a/src/components/sponsors/reports/__tests__/LinesManifestView.test.js b/src/components/sponsors/reports/__tests__/LinesManifestView.test.js index d840a2bc1..be8f62f6e 100644 --- a/src/components/sponsors/reports/__tests__/LinesManifestView.test.js +++ b/src/components/sponsors/reports/__tests__/LinesManifestView.test.js @@ -106,6 +106,18 @@ describe("LinesManifestView", () => { ).toBeInTheDocument(); }); + // A positional last-two-cells check (as used above for the freshness + // columns) only proves a cell isn't missing — it says nothing about the + // HEADER row, which is a separate array (HEADERS). If the two desync by + // one, every column right of the break silently misaligns and stays + // green. Assert exact header/cell cardinality directly. + it("has exactly 13 column headers matching 13 cells per row", () => { + renderView(); + expect(screen.getAllByRole("columnheader")).toHaveLength(13); + const row = screen.getByText("AV1").closest("tr"); + expect(within(row).getAllByRole("cell")).toHaveLength(13); + }); + // Sponsor bucketing (formerly bucketLinesBySponsor, now a private helper). describe("sponsor bucketing", () => { it("groups by sponsor.id preserving first-seen order", () => { diff --git a/src/i18n/en.json b/src/i18n/en.json index 11e5c0ccb..ebfc14f35 100644 --- a/src/i18n/en.json +++ b/src/i18n/en.json @@ -4374,7 +4374,7 @@ "view_orders": "Orders", "view_line_items": "Line Items", "view_by_item": "By Item", - "lines_count": "{count} lines", + "lines_count": "{count} live lines", "destination_booth_fallback": "Booth", "col_checkout_time": "Checkout Time", "col_invoice_total": "Invoice Total", From e34b5a69530414895864fee331b715488c64059a Mon Sep 17 00:00:00 2001 From: Casey Locker Date: Thu, 20 Aug 2026 12:54:43 -0500 Subject: [PATCH 12/16] test(sponsor-reports): pin the lines_count copy in the catalog The chip is fed liveLineCount while canceled lines still render, so the copy has to say which number it is. This module's i18n mock renders the chip from the key and count only, so the existing count assertion is value-independent: the English could regress to "{count} lines" and every DOM test would still pass. Mirrors the By Item chip's catalog copy test. ref: https://app.clickup.com/t/86bbh5vat --- .../reports/__tests__/LinesManifestView.test.js | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/components/sponsors/reports/__tests__/LinesManifestView.test.js b/src/components/sponsors/reports/__tests__/LinesManifestView.test.js index be8f62f6e..4eb0e2569 100644 --- a/src/components/sponsors/reports/__tests__/LinesManifestView.test.js +++ b/src/components/sponsors/reports/__tests__/LinesManifestView.test.js @@ -200,3 +200,17 @@ describe("Destination booth fallback", () => { ).toBeInTheDocument(); }); }); + +describe("lines_count copy", () => { + it("says the count is of LIVE lines, not all rendered lines", () => { + // The chip is fed liveLineCount, but canceled lines still RENDER, so a group + // showing two rows reports one. The copy has to say which number it is. + // This module's i18n mock renders the chip from the KEY and COUNT only, so + // the count assertion above is value-independent: the English could regress + // to "{count} lines" and every DOM test would still pass. Pin it in the + // catalog by exact equality, mirroring the By Item chip's copy test. + // eslint-disable-next-line global-require + const en = require("../../../../i18n/en.json"); + expect(en.sponsor_reports_page.lines_count).toBe("{count} live lines"); + }); +}); From 0fab48b67721e40e4fccb2a11422941148a9b45d Mon Sep 17 00:00:00 2001 From: Casey Locker Date: Thu, 20 Aug 2026 13:42:58 -0500 Subject: [PATCH 13/16] fix(sponsor-reports): announce the canceled-default note with its control The note explaining that canceled orders are excluded by default rendered next to the Purchase Status dropdown but was not associated with it, so screen readers never announced the two together. The note is a sibling rather than a FormControl child, so MUI cannot wire this up itself; the dropdown spreads unrecognised props onto the Select, so aria-describedby reaches the rendered control. ref: https://app.clickup.com/t/86bbh5vat --- .../__tests__/index.test.js | 16 ++++++++++++++++ .../purchase-details-report-page/index.js | 10 +++++++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/pages/sponsors/sponsor-reports/purchase-details-report-page/__tests__/index.test.js b/src/pages/sponsors/sponsor-reports/purchase-details-report-page/__tests__/index.test.js index 54801a322..256a9174d 100644 --- a/src/pages/sponsors/sponsor-reports/purchase-details-report-page/__tests__/index.test.js +++ b/src/pages/sponsors/sponsor-reports/purchase-details-report-page/__tests__/index.test.js @@ -641,5 +641,21 @@ describe("PurchaseDetailsReportPage", () => { screen.getByText("sponsor_reports_page.filter_status_canceled_note") ).toBeInTheDocument(); }); + + it("associates that note with the status control for screen readers", () => { + // Rendering the note NEAR the control is not the same as announcing it + // WITH the control. The note is a sibling, not a FormControl child, so + // MUI cannot wire this up itself — assert the aria-describedby actually + // resolves to the note's id rather than trusting visual adjacency. + renderPage(); + const control = screen.getByLabelText( + "sponsor_reports_page.filter_status" + ); + const noteId = control.getAttribute("aria-describedby"); + expect(noteId).toBeTruthy(); + expect(document.getElementById(noteId)).toHaveTextContent( + "sponsor_reports_page.filter_status_canceled_note" + ); + }); }); }); diff --git a/src/pages/sponsors/sponsor-reports/purchase-details-report-page/index.js b/src/pages/sponsors/sponsor-reports/purchase-details-report-page/index.js index b2bb6c040..7c52df27d 100644 --- a/src/pages/sponsors/sponsor-reports/purchase-details-report-page/index.js +++ b/src/pages/sponsors/sponsor-reports/purchase-details-report-page/index.js @@ -56,6 +56,9 @@ import { DEFAULT_CURRENT_PAGE } from "../../../../utils/constants"; // backend query contract is untouched. const REPORT_DATE_TZ = "UTC"; const REPORT_DATE_FORMAT = "YYYY-MM-DD"; +// Shared by the Purchase Status helper text and the aria-describedby that points +// at it, so the two cannot drift apart. +const STATUS_NOTE_ID = "pd-filter-status-note"; // The uicore picker emits moment(0) (epoch) on a CLEAR, not null — treat that // sentinel as "no date" so clearing removes the filter instead of sending // date>=1970-01-01. This matches the house filter convention (`.unix() || null` @@ -351,6 +354,11 @@ const PurchaseDetailsReportPage = ({ value={draft.status || ""} options={statusSelectOptions} onChange={(e) => update({ status: e.target.value || undefined })} + // The note below is a sibling, not a FormControl child, so MUI cannot + // wire it up itself. MuiDropdown spreads unrecognised props onto the + // Select, so this reaches the rendered control and screen readers + // announce the note with it instead of only near it. + aria-describedby={STATUS_NOTE_ID} /> {/* The default excludes canceled orders at BOTH grains and nothing said so, so "Any" silently means paid plus pending. @@ -361,7 +369,7 @@ const PurchaseDetailsReportPage = ({ child, it has no positioned ancestor and drifts to the initial containing block. Override back to static so it flows in normal layout right under the dropdown, in-flow (no overlap/spill). */} - + {T.translate("sponsor_reports_page.filter_status_canceled_note")} From e077974406e3f5b476b613052ce06a2513e85618 Mon Sep 17 00:00:00 2001 From: Casey Locker Date: Thu, 20 Aug 2026 16:47:45 -0500 Subject: [PATCH 14/16] fix(sponsor-reports): carry the canceled-default note as a hover info icon Helper text under the Purchase Status dropdown made that filter twice the height of its siblings, and the center-aligned filter row rendered it out of line with them. Switch to the repo's existing hover-info idiom, an aria-hidden fa-info-circle carrying the copy in its title, so every filter is the same height again. That idiom is sighted-only, so the same copy is repeated in a visually-hidden span the Select points at via aria-describedby, keeping the note announced with its control. Renames the key to filter_status_info, matching the label_key + _info convention the rest of the app uses for hover info. ref: https://app.clickup.com/t/86bbh5vat --- src/i18n/en.json | 2 +- .../__tests__/index.test.js | 19 ++++++--- .../purchase-details-report-page/index.js | 41 ++++++++++--------- 3 files changed, 36 insertions(+), 26 deletions(-) diff --git a/src/i18n/en.json b/src/i18n/en.json index ebfc14f35..af2aff963 100644 --- a/src/i18n/en.json +++ b/src/i18n/en.json @@ -4340,7 +4340,7 @@ "total_pending": "Total Pending", "total_refunded": "Total Refunded", "filter_status": "Purchase Status", - "filter_status_canceled_note": "Canceled orders are excluded unless you select Canceled", + "filter_status_info": "Canceled orders are excluded unless you select Canceled", "filter_form": "Type", "filter_payment_method": "Payment Method", "any": "Any", diff --git a/src/pages/sponsors/sponsor-reports/purchase-details-report-page/__tests__/index.test.js b/src/pages/sponsors/sponsor-reports/purchase-details-report-page/__tests__/index.test.js index 256a9174d..5fce190d3 100644 --- a/src/pages/sponsors/sponsor-reports/purchase-details-report-page/__tests__/index.test.js +++ b/src/pages/sponsors/sponsor-reports/purchase-details-report-page/__tests__/index.test.js @@ -635,11 +635,20 @@ describe("PurchaseDetailsReportPage", () => { expect(screen.getByText("$1.00")).toBeInTheDocument(); }); - it("states the silent canceled default next to the status control", () => { + it("states the silent canceled default on the status control's info icon", () => { + // Carried as the repo's hover-info idiom rather than helper text under the + // control: helper text made this filter twice the height of its siblings, + // and the center-aligned filter row then rendered it out of line with + // them. jsdom has no layout, so the alignment itself is not assertable — + // what IS assertable is that the note is no longer a block under the + // control, which is the thing that changed the height. renderPage(); - expect( - screen.getByText("sponsor_reports_page.filter_status_canceled_note") - ).toBeInTheDocument(); + const icon = document.querySelector("i.fa-info-circle"); + expect(icon).toBeInTheDocument(); + expect(icon).toHaveAttribute( + "title", + "sponsor_reports_page.filter_status_info" + ); }); it("associates that note with the status control for screen readers", () => { @@ -654,7 +663,7 @@ describe("PurchaseDetailsReportPage", () => { const noteId = control.getAttribute("aria-describedby"); expect(noteId).toBeTruthy(); expect(document.getElementById(noteId)).toHaveTextContent( - "sponsor_reports_page.filter_status_canceled_note" + "sponsor_reports_page.filter_status_info" ); }); }); diff --git a/src/pages/sponsors/sponsor-reports/purchase-details-report-page/index.js b/src/pages/sponsors/sponsor-reports/purchase-details-report-page/index.js index 7c52df27d..9c77f983e 100644 --- a/src/pages/sponsors/sponsor-reports/purchase-details-report-page/index.js +++ b/src/pages/sponsors/sponsor-reports/purchase-details-report-page/index.js @@ -16,7 +16,8 @@ import { connect } from "react-redux"; import { withRouter } from "react-router-dom"; import moment from "moment-timezone"; import T from "i18n-react/dist/i18n-react"; -import { Alert, Box, Button, FormHelperText } from "@mui/material"; +import { Alert, Box, Button } from "@mui/material"; +import { visuallyHidden } from "@mui/utils"; import PrintIcon from "@mui/icons-material/Print"; import DownloadIcon from "@mui/icons-material/Download"; import ShoppingCartOutlinedIcon from "@mui/icons-material/ShoppingCartOutlined"; @@ -56,8 +57,8 @@ import { DEFAULT_CURRENT_PAGE } from "../../../../utils/constants"; // backend query contract is untouched. const REPORT_DATE_TZ = "UTC"; const REPORT_DATE_FORMAT = "YYYY-MM-DD"; -// Shared by the Purchase Status helper text and the aria-describedby that points -// at it, so the two cannot drift apart. +// Shared by the visually-hidden copy of the Purchase Status note and the +// aria-describedby that points at it, so the two cannot drift apart. const STATUS_NOTE_ID = "pd-filter-status-note"; // The uicore picker emits moment(0) (epoch) on a CLEAR, not null — treat that // sentinel as "no date" so clearing removes the filter instead of sending @@ -343,7 +344,15 @@ const PurchaseDetailsReportPage = ({ const extraControls = (draft, update) => ( <> - + {/* The default excludes canceled orders at BOTH grains and nothing said so, + so "Any" silently means paid plus pending. Carried as the repo's + hover-info idiom (fa-info-circle + title) rather than helper text + under the control, so this filter stays the same height as its + siblings and the row keeps its alignment. The icon is aria-hidden + like every other use of it, so the same copy is repeated in a + visually-hidden span that the Select points at via aria-describedby; + otherwise the note would be sighted-only. */} + update({ status: e.target.value || undefined })} - // The note below is a sibling, not a FormControl child, so MUI cannot - // wire it up itself. MuiDropdown spreads unrecognised props onto the - // Select, so this reaches the rendered control and screen readers - // announce the note with it instead of only near it. aria-describedby={STATUS_NOTE_ID} /> - {/* The default excludes canceled orders at BOTH grains and nothing said so, - so "Any" silently means paid plus pending. - CustomTheme's global MuiFormHelperText override makes this - position: absolute anchored to the nearest positioned ancestor — - MuiDropdown's internal FormControl (position: relative), not this - Box. Since this FormHelperText is a sibling, not a FormControl - child, it has no positioned ancestor and drifts to the initial - containing block. Override back to static so it flows in normal - layout right under the dropdown, in-flow (no overlap/spill). */} - - {T.translate("sponsor_reports_page.filter_status_canceled_note")} - +