From dad4ce8fe6dd4a0d60aa310bf00c6b015e1800ea Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 28 May 2026 16:15:48 +0200 Subject: [PATCH 1/9] refactor(frontend/state): add History/ApprovalQueue column-filter slices (refs #166) Adds two new closed column-id enums and matching filter slices for the History page tables: PurchaseHistoryColumnId for the completed-purchases table and ApprovalQueueColumnId for the pending-approvals card. The two tables share Provider/Service/Term/Count/UpfrontCost/MonthlySavings columns but diverge on the queue's Account/Payment/MonthlyCost/CreatedBy vs Purchase History's ResourceType/Region, so each gets its own in-memory slice with set/clear/getAll accessors mirroring the existing recommendations equivalents. In-memory only on this iteration; localStorage persistence stays a follow-up under the same umbrella as the recommendations equivalent. --- frontend/src/state.ts | 87 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+) diff --git a/frontend/src/state.ts b/frontend/src/state.ts index 9a9e55505..d87b56626 100644 --- a/frontend/src/state.ts +++ b/frontend/src/state.ts @@ -334,6 +334,93 @@ export function clearAllActiveRiColumnFilters(): void { activeRiColumnFilters = {}; } +// --------------------------------------------------------------------------- +// History per-column filters (issue #166 follow-up). +// +// The History page renders two tables: Purchase History (completed-and-final +// rows) and Approval Queue (pending/in-flight rows). Their column shapes +// overlap only partially (Account / Payment / Monthly Cost / Created By +// appear only on the queue; Resource Type / Region appear only on Purchase +// History), so each table gets its own closed column-id enum and its own +// in-memory filter slice. Both reuse `applyColumnFilters` from +// lib/column-filters.ts via the same `kind: 'set' | 'expr'` shape used by +// the recommendations table -- no new lib-level abstractions needed. +// +// In-memory only on this iteration; persistence (localStorage) is a +// follow-up tracked under the same umbrella as the recommendations +// equivalent. +// --------------------------------------------------------------------------- + +export type PurchaseHistoryColumnId = + | 'provider' | 'service' | 'resource_type' | 'region' | 'term' + | 'count' | 'upfront_cost' | 'savings'; + +export type ApprovalQueueColumnId = + | 'provider' | 'account' | 'service' | 'term' | 'payment' | 'created_by' + | 'count' | 'monthly_cost' | 'upfront_cost' | 'savings'; + +export type HistoryColumnFilter = + | { kind: 'set'; values: string[] } + | { kind: 'expr'; expr: string }; + +export type PurchaseHistoryColumnFilters = Partial< + Record +>; +export type ApprovalQueueColumnFilters = Partial< + Record +>; + +let purchaseHistoryColumnFilters: PurchaseHistoryColumnFilters = {}; +let approvalQueueColumnFilters: ApprovalQueueColumnFilters = {}; + +export function getPurchaseHistoryColumnFilters(): PurchaseHistoryColumnFilters { + return { ...purchaseHistoryColumnFilters }; +} + +export function setPurchaseHistoryColumnFilter( + column: PurchaseHistoryColumnId, + filter: HistoryColumnFilter | null, +): void { + if (filter === null) { + const next = { ...purchaseHistoryColumnFilters }; + delete next[column]; + purchaseHistoryColumnFilters = next; + return; + } + purchaseHistoryColumnFilters = { + ...purchaseHistoryColumnFilters, + [column]: filter, + }; +} + +export function clearAllPurchaseHistoryColumnFilters(): void { + purchaseHistoryColumnFilters = {}; +} + +export function getApprovalQueueColumnFilters(): ApprovalQueueColumnFilters { + return { ...approvalQueueColumnFilters }; +} + +export function setApprovalQueueColumnFilter( + column: ApprovalQueueColumnId, + filter: HistoryColumnFilter | null, +): void { + if (filter === null) { + const next = { ...approvalQueueColumnFilters }; + delete next[column]; + approvalQueueColumnFilters = next; + return; + } + approvalQueueColumnFilters = { + ...approvalQueueColumnFilters, + [column]: filter, + }; +} + +export function clearAllApprovalQueueColumnFilters(): void { + approvalQueueColumnFilters = {}; +} + // --------------------------------------------------------------------------- // Per-column visibility state (issue #318). // A column id in this set is HIDDEN; an absent id is visible (default visible). From d964915e34f7233a7a76083f2e436fda87b24818 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 28 May 2026 16:18:34 +0200 Subject: [PATCH 2/9] =?UTF-8?q?feat(frontend/history):=20inline=20column?= =?UTF-8?q?=20filters=20via=20shared=20lib=20=E2=80=94=20Purchase=20Histor?= =?UTF-8?q?y=20table=20(refs=20#166)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds per-column filter buttons to the Purchase History table headers. Filter columns: Provider, Service, Type (resource_type), Region, Term (categorical), and Count, Upfront Cost, Monthly Savings (numeric). Status is excluded — the existing status chip-row is the canonical filter for that column. Wires `applyColumnFilters` from lib/column-filters.ts through extractors that match the rendered cell shape: categorical extractors return the raw field; numeric extractors return the value rounded to display precision so typed numbers match the rendered cells (issue #484 contract). Filters compose with the existing status chip filter, and the column-filter popover lists distinct values from rows that survived the status filter (so picking "Failed" then opening Provider only lists providers with failed rows). Introduces a small lib/history-filter-popover.ts module shared by both History tables — keeps the popover DOM/teardown/keyboard wiring out of history.ts itself and reuses the existing .column-filter-popover CSS so the visual matches the recommendations equivalent. --- frontend/src/history.ts | 188 ++++++++++- frontend/src/lib/history-filter-popover.ts | 355 +++++++++++++++++++++ 2 files changed, 534 insertions(+), 9 deletions(-) create mode 100644 frontend/src/lib/history-filter-popover.ts diff --git a/frontend/src/history.ts b/frontend/src/history.ts index 0e68ae0b1..79d4d90bc 100644 --- a/frontend/src/history.ts +++ b/frontend/src/history.ts @@ -14,6 +14,16 @@ import { getCurrentUser } from './state'; import { canAccess } from './permissions'; import { showSkeletonRows, teardownSkeleton } from './lib/skeleton'; import { getAccountName } from './recommendations'; +import { applyColumnFilters } from './lib/column-filters'; +import { + openHistoryColumnPopover, + renderHistoryFilterButton, + closeOpenHistoryPopover, +} from './lib/history-filter-popover'; +import type { + PurchaseHistoryColumnId, + ApprovalQueueColumnId, +} from './state'; const VALID_PROVIDERS: api.Provider[] = ['aws', 'azure', 'gcp']; @@ -275,6 +285,12 @@ export async function loadHistory(): Promise { const queueEl = document.getElementById('purchases-approval-queue'); if (queueEl) showSkeletonRows(queueEl, 3, 12); + // Close any open History column-filter popover before re-rendering so the + // popover doesn't sit anchored to a stale button after the table is + // innerHTML-rewritten. The next render with active filters rebinds fresh + // triggers; the popover stays opt-in (user clicks again to re-open). + closeOpenHistoryPopover(); + try { // Provider/account filters live in state.ts now (mutated by topbar chips). const rawProvider = state.getCurrentProvider(); @@ -820,6 +836,149 @@ function renderActionCell(p: HistoryPurchase): string { return escapeHtml(p.plan_name || '-'); } +// --------------------------------------------------------------------------- +// Per-column filter wiring for the Purchase History table. +// +// The Status chip-row above stays as-is — it's an enum-driven filter with no +// natural fit for the generic set/expr column-filter shape, and the chip-row +// is the more discoverable affordance for status anyway. Column filters here +// cover the row attributes (provider/service/type/region/term/count/upfront/ +// savings) — Status is intentionally excluded. +// +// Numeric extractors round to 0 decimal places to match formatCurrency's +// default (CURRENCY_DEFAULT_DIGITS), so a user typing the visible "$123" +// matches the row that displays that exact value (issue #484 contract on +// the recommendations table). +// --------------------------------------------------------------------------- + +const PURCHASE_HISTORY_NUMERIC_COLUMNS: ReadonlySet = new Set([ + 'count', 'upfront_cost', 'savings', +]); + +function purchaseHistoryCategoricalCellValue( + p: HistoryPurchase, + col: PurchaseHistoryColumnId, +): string { + switch (col) { + case 'provider': return p.provider ?? ''; + case 'service': return p.service ?? ''; + case 'resource_type': return p.resource_type ?? ''; + case 'region': return p.region ?? ''; + case 'term': return p.term == null ? '' : String(p.term); + case 'count': + case 'upfront_cost': + case 'savings': return ''; + } +} + +function purchaseHistoryNumericCellValue( + p: HistoryPurchase, + col: PurchaseHistoryColumnId, +): number { + switch (col) { + case 'count': return p.count ?? 0; + case 'upfront_cost': return p.upfront_cost ?? 0; + case 'savings': return p.estimated_savings ?? 0; + case 'provider': + case 'service': + case 'resource_type': + case 'region': + case 'term': return Number.NaN; + } +} + +// Round to display precision so typed values match the rendered cell value +// (formatCurrency default of 0 fraction digits, formatTerm renders the +// integer term unchanged). +function roundForDisplay(n: number): number { + if (!Number.isFinite(n)) return n; + return Number(n.toFixed(0)); +} + +export function applyPurchaseHistoryColumnFilters( + purchases: readonly HistoryPurchase[], + filters: state.PurchaseHistoryColumnFilters, +): HistoryPurchase[] { + return applyColumnFilters( + purchases, + filters, + { + categorical: purchaseHistoryCategoricalCellValue, + numeric: (p, col) => roundForDisplay(purchaseHistoryNumericCellValue(p, col)), + }, + ); +} + +const PURCHASE_HISTORY_LABELS: Record = { + provider: 'Provider', + service: 'Service', + resource_type: 'Type', + region: 'Region', + term: 'Term', + count: 'Count', + upfront_cost: 'Upfront Cost', + savings: 'Monthly Savings', +}; + +function purchaseHistoryDistinctValues( + purchases: readonly HistoryPurchase[], + column: PurchaseHistoryColumnId, +): string[] { + const seen = new Set(); + for (const p of purchases) { + seen.add(purchaseHistoryCategoricalCellValue(p, column)); + } + return Array.from(seen).sort((a, b) => { + if (a === '' && b !== '') return -1; + if (a !== '' && b === '') return 1; + return a.localeCompare(b); + }); +} + +function purchaseHistoryDisplayLabel( + column: PurchaseHistoryColumnId, + value: string, +): string { + if (value === '') return '(empty)'; + if (column === 'term') { + const n = Number(value); + return Number.isFinite(n) ? formatTerm(n) : value; + } + return value; +} + +function wirePurchaseHistoryFilterButtons( + container: HTMLElement, + // The pre-column-filter slice — popover lists distinct values from + // every row that survived the status chip, NOT the further-narrowed + // visible slice (otherwise the popover would lose values the user just + // unchecked). + sourceRows: readonly HistoryPurchase[], +): void { + container.querySelectorAll('.history-column-filter-btn').forEach((btn) => { + const column = btn.dataset['column'] as PurchaseHistoryColumnId | undefined; + if (!column) return; + btn.addEventListener('click', (e) => { + e.stopPropagation(); + const isNumeric = PURCHASE_HISTORY_NUMERIC_COLUMNS.has(column); + const filters = state.getPurchaseHistoryColumnFilters(); + openHistoryColumnPopover({ + column, + anchor: btn, + currentFilter: filters[column], + headerLabel: PURCHASE_HISTORY_LABELS[column], + kind: isNumeric ? 'numeric' : 'categorical', + distinctValues: isNumeric ? undefined : purchaseHistoryDistinctValues(sourceRows, column), + displayLabel: (v) => purchaseHistoryDisplayLabel(column, v), + onCommit: (filter) => { + state.setPurchaseHistoryColumnFilter(column, filter); + renderHistoryList(lastPurchases); + }, + }); + }); + }); +} + function renderHistoryList(purchases: HistoryPurchase[]): void { const container = document.getElementById('history-list'); if (!container) return; @@ -872,7 +1031,7 @@ function renderHistoryList(purchases: HistoryPurchase[]): void { return; } - const visible = purchases.filter(p => { + const statusFiltered = purchases.filter(p => { if (activeStatusFilter === 'all') return true; const s = normalizeStatus(p).toLowerCase(); if (activeStatusFilter === 'pending') return s === 'pending' || s === 'notified' || isInFlightStatus(s); @@ -883,6 +1042,13 @@ function renderHistoryList(purchases: HistoryPurchase[]): void { return s === activeStatusFilter; }); + // Apply the per-column filters AFTER the status chip filter so the + // categorical popover lists only values present in the active status + // slice (e.g. filtering by "Failed" only shows providers/services that + // have failed rows). + const colFilters = state.getPurchaseHistoryColumnFilters(); + const visible = applyPurchaseHistoryColumnFilters(statusFiltered, colFilters); + const tableRows = visible.map(p => { const statusCell = (() => { const badge = statusBadgeHTML(normalizeStatus(p)); @@ -925,6 +1091,9 @@ function renderHistoryList(purchases: HistoryPurchase[]): void { const amortize = state.getAmortizeUpfront(); const monthlyColHeader = amortize ? 'Monthly Cost (amortized)' : 'Monthly Cost'; + const fbtn = (col: PurchaseHistoryColumnId): string => renderHistoryFilterButton( + col, PURCHASE_HISTORY_LABELS[col], colFilters[col] != null, + ); const markup = ` ${buildStatusChipRowHTML(purchases, activeStatusFilter)} @@ -932,15 +1101,15 @@ function renderHistoryList(purchases: HistoryPurchase[]): void { - - - - - - - + + + + + + + - + @@ -953,6 +1122,7 @@ function renderHistoryList(purchases: HistoryPurchase[]): void { // Mount the amortize checkbox into the controls area (idempotent). mountAmortizeCheckbox('history-controls', 'history-amortize-checkbox'); + wirePurchaseHistoryFilterButtons(container, statusFiltered); container.querySelectorAll('.status-chip[data-history-status]').forEach(btn => { btn.addEventListener('click', () => { diff --git a/frontend/src/lib/history-filter-popover.ts b/frontend/src/lib/history-filter-popover.ts new file mode 100644 index 000000000..4fedce514 --- /dev/null +++ b/frontend/src/lib/history-filter-popover.ts @@ -0,0 +1,355 @@ +/** + * Lightweight column-filter popover shared by the two History tables + * (Purchase History + Approval Queue) — issue #166 follow-up. + * + * Mirrors the visual shape of recommendations.ts's popover (so the same + * CSS in styles/components.css applies) but is intentionally simpler: + * - One popover open at a time per `openColumnPopover` call. + * - The caller owns the column-id enum, the filter state slice, and + * the re-render callback; this module only renders the popover DOM, + * wires inputs to commit, and manages anchor/global-listener teardown. + * - No "All Savings Plans" group affordance (recs-specific). + * + * The popover is appended to `document.body` (portal pattern) so it + * survives the table's `innerHTML` rewrite on every commit; the next + * render is expected to rebind the trigger button by `[data-column=…]`. + */ + +import { + parseNumericFilter, + type ColumnFilterKind, +} from './column-filters'; + +export interface PopoverConfig { + column: TColumnId; + anchor: HTMLElement; + // Current filter for this column (null = not narrowed). + currentFilter: ColumnFilterKind | undefined; + // Header label rendered inside the popover ("Filter
Status DateProviderServiceTypeRegionCountTermUpfront CostProvider${fbtn('provider')}Service${fbtn('service')}Type${fbtn('resource_type')}Region${fbtn('region')}Count${fbtn('count')}Term${fbtn('term')}Upfront Cost${fbtn('upfront_cost')} ${escapeHtml(monthlyColHeader)}Monthly SavingsMonthly Savings${fbtn('savings')} Plan
` cell. The caller + * binds the click handler against `.history-column-filter-btn` after the + * table is innerHTML-rewritten. + */ +export function renderHistoryFilterButton( + column: TColumnId, + label: string, + active: boolean, +): string { + const cls = `history-column-filter-btn${active ? ' active' : ''}`; + const ariaLabel = active ? `Filter ${label} — currently active` : `Filter ${label}`; + // Use the same gear-style glyph (⛛) the recommendations popover + // uses so the two surfaces are visually identical to the user. + return ``; +} From 159cd61e3a3a2eecb22044e47f585817755ee0dc Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 28 May 2026 16:24:21 +0200 Subject: [PATCH 3/9] =?UTF-8?q?feat(frontend/history):=20inline=20column?= =?UTF-8?q?=20filters=20via=20shared=20lib=20=E2=80=94=20Approval=20Queue?= =?UTF-8?q?=20table=20(refs=20#166)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds per-column filter buttons to the Approval Queue table headers. Filter columns: Provider, Account, Service, Term, Payment, Created by (categorical), and Count, Monthly Cost, Upfront Cost, Monthly Savings (numeric). Status is excluded — the queue scope is already pending| notified by definition; the broader Status chip-row above is the authoritative status filter for the page. Uses the same lib/history-filter-popover.ts helper introduced for the Purchase History table. Numeric extractors round to display precision (CURRENCY_DEFAULT_DIGITS) so a "$X" filter matches the rendered cell; monthly_cost returns NaN for null so a "= 0" predicate doesn't match rows where the provider didn't report a value. Categorical extractors mirror the cell rendering: account uses account_id with getAccountName as the display label, created_by prefers email then falls back to UUID. Test-suite mocks (history-* + allowed-accounts + xss-provider-class) extended with the new state accessors so they keep passing against the expanded state surface. --- .../src/__tests__/allowed-accounts.test.ts | 6 + .../__tests__/history-approval-queue.test.ts | 6 + .../__tests__/history-approve-button.test.ts | 6 + .../__tests__/history-cancel-button.test.ts | 6 + .../__tests__/history-retry-button.test.ts | 6 + frontend/src/__tests__/history.test.ts | 8 + .../src/__tests__/xss-provider-class.test.ts | 6 + frontend/src/history.ts | 179 ++++++++++++++++-- 8 files changed, 212 insertions(+), 11 deletions(-) diff --git a/frontend/src/__tests__/allowed-accounts.test.ts b/frontend/src/__tests__/allowed-accounts.test.ts index 20cddb667..06f9e27c7 100644 --- a/frontend/src/__tests__/allowed-accounts.test.ts +++ b/frontend/src/__tests__/allowed-accounts.test.ts @@ -73,6 +73,12 @@ jest.mock('../state', () => ({ getAmortizeUpfront: jest.fn().mockReturnValue(false), setAmortizeUpfront: jest.fn(), subscribeAmortizeUpfront: jest.fn().mockReturnValue(() => {}), + getPurchaseHistoryColumnFilters: jest.fn().mockReturnValue({}), + setPurchaseHistoryColumnFilter: jest.fn(), + clearAllPurchaseHistoryColumnFilters: jest.fn(), + getApprovalQueueColumnFilters: jest.fn().mockReturnValue({}), + setApprovalQueueColumnFilter: jest.fn(), + clearAllApprovalQueueColumnFilters: jest.fn(), })); // --------------------------------------------------------------------------- diff --git a/frontend/src/__tests__/history-approval-queue.test.ts b/frontend/src/__tests__/history-approval-queue.test.ts index 3bc03f3ba..762a0fb09 100644 --- a/frontend/src/__tests__/history-approval-queue.test.ts +++ b/frontend/src/__tests__/history-approval-queue.test.ts @@ -63,6 +63,12 @@ jest.mock('../state', () => ({ getAmortizeUpfront: jest.fn().mockReturnValue(false), setAmortizeUpfront: jest.fn(), subscribeAmortizeUpfront: jest.fn().mockReturnValue(() => {}), + getPurchaseHistoryColumnFilters: jest.fn().mockReturnValue({}), + setPurchaseHistoryColumnFilter: jest.fn(), + clearAllPurchaseHistoryColumnFilters: jest.fn(), + getApprovalQueueColumnFilters: jest.fn().mockReturnValue({}), + setApprovalQueueColumnFilter: jest.fn(), + clearAllApprovalQueueColumnFilters: jest.fn(), })); jest.mock('../recommendations', () => ({ diff --git a/frontend/src/__tests__/history-approve-button.test.ts b/frontend/src/__tests__/history-approve-button.test.ts index cbf1bda90..e60bf12cd 100644 --- a/frontend/src/__tests__/history-approve-button.test.ts +++ b/frontend/src/__tests__/history-approve-button.test.ts @@ -64,6 +64,12 @@ jest.mock('../state', () => ({ getAmortizeUpfront: jest.fn().mockReturnValue(false), setAmortizeUpfront: jest.fn(), subscribeAmortizeUpfront: jest.fn().mockReturnValue(() => {}), + getPurchaseHistoryColumnFilters: jest.fn().mockReturnValue({}), + setPurchaseHistoryColumnFilter: jest.fn(), + clearAllPurchaseHistoryColumnFilters: jest.fn(), + getApprovalQueueColumnFilters: jest.fn().mockReturnValue({}), + setApprovalQueueColumnFilter: jest.fn(), + clearAllApprovalQueueColumnFilters: jest.fn(), })); import * as api from '../api'; diff --git a/frontend/src/__tests__/history-cancel-button.test.ts b/frontend/src/__tests__/history-cancel-button.test.ts index 492bf47dd..1ed6d3259 100644 --- a/frontend/src/__tests__/history-cancel-button.test.ts +++ b/frontend/src/__tests__/history-cancel-button.test.ts @@ -61,6 +61,12 @@ jest.mock('../state', () => ({ getAmortizeUpfront: jest.fn().mockReturnValue(false), setAmortizeUpfront: jest.fn(), subscribeAmortizeUpfront: jest.fn().mockReturnValue(() => {}), + getPurchaseHistoryColumnFilters: jest.fn().mockReturnValue({}), + setPurchaseHistoryColumnFilter: jest.fn(), + clearAllPurchaseHistoryColumnFilters: jest.fn(), + getApprovalQueueColumnFilters: jest.fn().mockReturnValue({}), + setApprovalQueueColumnFilter: jest.fn(), + clearAllApprovalQueueColumnFilters: jest.fn(), })); import * as api from '../api'; diff --git a/frontend/src/__tests__/history-retry-button.test.ts b/frontend/src/__tests__/history-retry-button.test.ts index 72d86282a..a96b1f366 100644 --- a/frontend/src/__tests__/history-retry-button.test.ts +++ b/frontend/src/__tests__/history-retry-button.test.ts @@ -70,6 +70,12 @@ jest.mock('../state', () => ({ getAmortizeUpfront: jest.fn().mockReturnValue(false), setAmortizeUpfront: jest.fn(), subscribeAmortizeUpfront: jest.fn().mockReturnValue(() => {}), + getPurchaseHistoryColumnFilters: jest.fn().mockReturnValue({}), + setPurchaseHistoryColumnFilter: jest.fn(), + clearAllPurchaseHistoryColumnFilters: jest.fn(), + getApprovalQueueColumnFilters: jest.fn().mockReturnValue({}), + setApprovalQueueColumnFilter: jest.fn(), + clearAllApprovalQueueColumnFilters: jest.fn(), })); import * as api from '../api'; diff --git a/frontend/src/__tests__/history.test.ts b/frontend/src/__tests__/history.test.ts index ecca147c0..0b47ff45f 100644 --- a/frontend/src/__tests__/history.test.ts +++ b/frontend/src/__tests__/history.test.ts @@ -44,6 +44,14 @@ jest.mock('../state', () => ({ getAmortizeUpfront: jest.fn().mockReturnValue(false), setAmortizeUpfront: jest.fn(), subscribeAmortizeUpfront: jest.fn().mockReturnValue(() => {}), + // History per-column filter accessors (issue #166): tests only exercise + // empty filter state, so each getter returns {} and each setter is a no-op. + getPurchaseHistoryColumnFilters: jest.fn().mockReturnValue({}), + setPurchaseHistoryColumnFilter: jest.fn(), + clearAllPurchaseHistoryColumnFilters: jest.fn(), + getApprovalQueueColumnFilters: jest.fn().mockReturnValue({}), + setApprovalQueueColumnFilter: jest.fn(), + clearAllApprovalQueueColumnFilters: jest.fn(), })); import * as api from '../api'; diff --git a/frontend/src/__tests__/xss-provider-class.test.ts b/frontend/src/__tests__/xss-provider-class.test.ts index 3dad85b7b..d1accefea 100644 --- a/frontend/src/__tests__/xss-provider-class.test.ts +++ b/frontend/src/__tests__/xss-provider-class.test.ts @@ -46,6 +46,12 @@ jest.mock('../state', () => ({ getAmortizeUpfront: jest.fn().mockReturnValue(false), setAmortizeUpfront: jest.fn(), subscribeAmortizeUpfront: jest.fn().mockReturnValue(() => {}), + getPurchaseHistoryColumnFilters: jest.fn().mockReturnValue({}), + setPurchaseHistoryColumnFilter: jest.fn(), + clearAllPurchaseHistoryColumnFilters: jest.fn(), + getApprovalQueueColumnFilters: jest.fn().mockReturnValue({}), + setApprovalQueueColumnFilter: jest.fn(), + clearAllApprovalQueueColumnFilters: jest.fn(), })); import * as api from '../api'; diff --git a/frontend/src/history.ts b/frontend/src/history.ts index 79d4d90bc..c17980ecb 100644 --- a/frontend/src/history.ts +++ b/frontend/src/history.ts @@ -1505,18 +1505,171 @@ function isPendingRow(p: HistoryPurchase): boolean { // from the history table (confirmDialog → API → toast → reload). The // reload re-renders both views from one fetch, which removes the // approved row from BOTH lists in one shot. +// --------------------------------------------------------------------------- +// Per-column filter wiring for the Approval Queue table. +// +// The queue scope is already narrow (pending|notified rows only); column +// filters add inline narrowing on the queue's own columns. As with the +// Purchase History wiring, Status is excluded because the queue's row set +// is status-defined and the parent loadHistory loop is the authoritative +// status source. +// +// Numeric extractors round to 0 decimal places (CURRENCY_DEFAULT_DIGITS) +// so a "$X" filter targets the same value the cell renders. +// --------------------------------------------------------------------------- + +const APPROVAL_QUEUE_NUMERIC_COLUMNS: ReadonlySet = new Set([ + 'count', 'monthly_cost', 'upfront_cost', 'savings', +]); + +const APPROVAL_QUEUE_LABELS: Record = { + provider: 'Provider', + account: 'Account', + service: 'Service', + term: 'Term', + payment: 'Payment', + created_by: 'Created by', + count: 'Count', + monthly_cost: 'Monthly Cost', + upfront_cost: 'Upfront Cost', + savings: 'Monthly Savings', +}; + +function approvalQueueCategoricalCellValue( + p: HistoryPurchase, + col: ApprovalQueueColumnId, +): string { + switch (col) { + case 'provider': return p.provider ?? ''; + case 'account': return p.account_id ?? ''; + case 'service': return p.service ?? ''; + case 'term': return p.term == null ? '' : String(p.term); + case 'payment': return p.payment ?? ''; + case 'created_by': return p.created_by_user_email ?? p.created_by_user_id ?? ''; + case 'count': + case 'monthly_cost': + case 'upfront_cost': + case 'savings': return ''; + } +} + +function approvalQueueNumericCellValue( + p: HistoryPurchase, + col: ApprovalQueueColumnId, +): number { + switch (col) { + case 'count': return p.count ?? 0; + // Return NaN for null monthly_cost so numeric predicates (e.g. "= 0") + // don't match rows where the provider didn't report a monthly cost. + case 'monthly_cost': return p.monthly_cost == null ? Number.NaN : p.monthly_cost; + case 'upfront_cost': return p.upfront_cost ?? 0; + case 'savings': return p.estimated_savings ?? 0; + case 'provider': + case 'account': + case 'service': + case 'term': + case 'payment': + case 'created_by': return Number.NaN; + } +} + +export function applyApprovalQueueColumnFilters( + purchases: readonly HistoryPurchase[], + filters: state.ApprovalQueueColumnFilters, +): HistoryPurchase[] { + return applyColumnFilters( + purchases, + filters, + { + categorical: approvalQueueCategoricalCellValue, + numeric: (p, col) => roundForDisplay(approvalQueueNumericCellValue(p, col)), + }, + ); +} + +function approvalQueueDistinctValues( + purchases: readonly HistoryPurchase[], + column: ApprovalQueueColumnId, +): string[] { + const seen = new Set(); + for (const p of purchases) { + seen.add(approvalQueueCategoricalCellValue(p, column)); + } + return Array.from(seen).sort((a, b) => { + if (a === '' && b !== '') return -1; + if (a !== '' && b === '') return 1; + return a.localeCompare(b); + }); +} + +function approvalQueueDisplayLabel( + column: ApprovalQueueColumnId, + value: string, +): string { + if (value === '') return '(empty)'; + if (column === 'term') { + const n = Number(value); + return Number.isFinite(n) ? formatTerm(n) : value; + } + if (column === 'account') { + // Account cells render via getAccountName() — mirror the same display. + return getAccountName(value); + } + return value; +} + +function wireApprovalQueueFilterButtons( + container: HTMLElement, + // Source for the popover's distinct-values list — the pre-column-filter + // pending slice. Same reasoning as Purchase History: the popover must + // list every value that exists in the broader (un-narrowed) set so + // the user can re-check a value after unchecking it. + sourceRows: readonly HistoryPurchase[], +): void { + container.querySelectorAll('.history-column-filter-btn').forEach((btn) => { + const column = btn.dataset['column'] as ApprovalQueueColumnId | undefined; + if (!column) return; + btn.addEventListener('click', (e) => { + e.stopPropagation(); + const isNumeric = APPROVAL_QUEUE_NUMERIC_COLUMNS.has(column); + const filters = state.getApprovalQueueColumnFilters(); + openHistoryColumnPopover({ + column, + anchor: btn, + currentFilter: filters[column], + headerLabel: APPROVAL_QUEUE_LABELS[column], + kind: isNumeric ? 'numeric' : 'categorical', + distinctValues: isNumeric ? undefined : approvalQueueDistinctValues(sourceRows, column), + displayLabel: (v) => approvalQueueDisplayLabel(column, v), + onCommit: (filter) => { + state.setApprovalQueueColumnFilter(column, filter); + renderApprovalQueue(lastPendingForQueue); + }, + }); + }); + }); +} + +// Cache of the last pre-column-filter pending list so the popover-driven +// re-render path can rebuild the table without re-fetching. +let lastPendingForQueue: HistoryPurchase[] = []; + export function renderApprovalQueue(purchases: HistoryPurchase[]): void { const container = document.getElementById('purchases-approval-queue'); if (!container) return; const pending = (purchases || []).filter(isPendingRow); + lastPendingForQueue = pending; if (pending.length === 0) { container.innerHTML = '

No pending approvals.

'; return; } - const rows = pending.map(p => { + const colFilters = state.getApprovalQueueColumnFilters(); + const visible = applyApprovalQueueColumnFilters(pending, colFilters); + + const rows = visible.map(p => { const actions = renderPendingActionButtons(p); const actionsCell = actions || '-'; // Show email when resolved; fall back to UUID so the cancel-own gate still @@ -1562,21 +1715,24 @@ export function renderApprovalQueue(purchases: HistoryPurchase[]): void { const monthlyColHeader = amortize ? 'Monthly Cost (amortized)' : 'Monthly Cost'; // monthlyColHeader is a hardcoded constant string (no user data), so // interpolating it directly into the template is safe. + const fbtn = (col: ApprovalQueueColumnId): string => renderHistoryFilterButton( + col, APPROVAL_QUEUE_LABELS[col], colFilters[col] != null, + ); container.innerHTML = ` - - - - - - - - - - + + + + + + + + + + @@ -1590,4 +1746,5 @@ export function renderApprovalQueue(purchases: HistoryPurchase[]): void { mountAmortizeCheckbox('purchases-approval-queue-section', 'approval-queue-amortize-checkbox'); wireRowActionHandlers(container); + wireApprovalQueueFilterButtons(container, pending); } From b144b2f473c7d027632724f4f52033faf2213e47 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 28 May 2026 16:26:13 +0200 Subject: [PATCH 4/9] test(frontend/history): column-filter regression suites (refs #166) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds two test files exercising the new History column-filter wiring: * history-column-filters.test.ts — Purchase History table: numeric expr, categorical set, stacked AND, invalid expr (no-op), clear, and the term-as-stringified-categorical case. * approval-queue-column-filters.test.ts — Approval Queue table: numeric expr (monthly_cost >= N), categorical set (payment in {…}), stacked AND across provider+created_by, invalid expr (no-op), the NaN-as-missing contract for null monthly_cost (so "= 0" and "> 0" don't coincidentally match unreported rows), and clear. Both suites mock the heavy module transitive deps (api / navigation / utils / state / confirmDialog / approval-details / toast / skeleton / recommendations) so the pure column-filter helpers can be exercised without standing up a DOM. --- .../approval-queue-column-filters.test.ts | 125 ++++++++++++++++++ .../__tests__/history-column-filters.test.ts | 117 ++++++++++++++++ 2 files changed, 242 insertions(+) create mode 100644 frontend/src/__tests__/approval-queue-column-filters.test.ts create mode 100644 frontend/src/__tests__/history-column-filters.test.ts diff --git a/frontend/src/__tests__/approval-queue-column-filters.test.ts b/frontend/src/__tests__/approval-queue-column-filters.test.ts new file mode 100644 index 000000000..cf3140544 --- /dev/null +++ b/frontend/src/__tests__/approval-queue-column-filters.test.ts @@ -0,0 +1,125 @@ +/** + * Approval Queue column-filter regression suite (issue #166). + * + * Covers the inline per-column filters wired onto the Approval Queue + * table headers. Filterable columns: Provider, Account, Service, Term, + * Payment, Created by (categorical), Count, Monthly Cost, Upfront Cost, + * Monthly Savings (numeric). Status is excluded by design — the queue + * scope is already pending|notified. + * + * Tested matrix: + * 1. Numeric expr filter narrows by predicate (monthly_cost >= N). + * 2. Categorical set filter narrows by membership (payment in set). + * 3. Multiple filters AND together across columns. + * 4. Invalid numeric expression is skipped (no exception, no narrowing). + * 5. NaN-as-missing contract: monthly_cost == null produces NaN, which + * fails every numeric predicate (not coincidentally matches "= 0"). + * 6. Clearing returns a fresh clone of the input. + */ + +import { applyApprovalQueueColumnFilters } from '../history'; +import type { HistoryPurchase } from '../types'; +import type { ApprovalQueueColumnFilters } from '../state'; + +jest.mock('../api', () => ({})); +jest.mock('../navigation', () => ({ switchTab: jest.fn() })); +jest.mock('../utils', () => ({ + formatCurrency: jest.fn((v) => `$${v ?? 0}`), + formatDate: jest.fn((v) => v), + formatTerm: jest.fn((y) => `${y} Year${y === 1 ? '' : 's'}`), + escapeHtml: jest.fn((s) => s ?? ''), +})); +jest.mock('../state', () => ({ + subscribeProvider: jest.fn().mockReturnValue(() => {}), + subscribeAccount: jest.fn().mockReturnValue(() => {}), +})); +jest.mock('../confirmDialog', () => ({ confirmDialog: jest.fn() })); +jest.mock('../approval-details', () => ({ buildApprovalDetailsBody: jest.fn() })); +jest.mock('../toast', () => ({ showToast: jest.fn() })); +jest.mock('../lib/skeleton', () => ({ showSkeletonRows: jest.fn(), teardownSkeleton: jest.fn() })); +jest.mock('../recommendations', () => ({ getAccountName: jest.fn((id: string) => id) })); + +function mkRow(overrides: Partial): HistoryPurchase { + return { + purchase_id: 'p', + timestamp: '2024-01-01T00:00:00Z', + provider: 'aws', + service: 'ec2', + resource_type: 'reserved-instance', + region: 'us-east-1', + count: 1, + term: 1, + payment: 'all_upfront', + upfront_cost: 100, + monthly_cost: 50, + estimated_savings: 30, + account_id: 'acct-1', + created_by_user_email: 'alice@example.com', + status: 'pending', + ...overrides, + }; +} + +const rows: HistoryPurchase[] = [ + mkRow({ purchase_id: 'a', provider: 'aws', account_id: 'acct-1', payment: 'all_upfront', monthly_cost: 50, estimated_savings: 30, created_by_user_email: 'alice@example.com' }), + mkRow({ purchase_id: 'b', provider: 'aws', account_id: 'acct-2', payment: 'no_upfront', monthly_cost: 200, estimated_savings: 100, created_by_user_email: 'bob@example.com' }), + mkRow({ purchase_id: 'c', provider: 'azure', account_id: 'acct-2', payment: 'partial_upfront', monthly_cost: null as unknown as number | undefined, estimated_savings: 60, created_by_user_email: 'alice@example.com' }), + mkRow({ purchase_id: 'd', provider: 'gcp', account_id: 'acct-3', payment: 'all_upfront', monthly_cost: 500, estimated_savings: 250, created_by_user_email: 'carol@example.com' }), +]; + +describe('applyApprovalQueueColumnFilters', () => { + test('numeric expr: monthly_cost >= 200 narrows to expensive rows', () => { + const filters: ApprovalQueueColumnFilters = { + monthly_cost: { kind: 'expr', expr: '>=200' }, + }; + const out = applyApprovalQueueColumnFilters(rows, filters); + expect(out.map((r) => r.purchase_id)).toEqual(['b', 'd']); + }); + + test('categorical set: payment in {all_upfront} narrows to those rows', () => { + const filters: ApprovalQueueColumnFilters = { + payment: { kind: 'set', values: ['all_upfront'] }, + }; + const out = applyApprovalQueueColumnFilters(rows, filters); + expect(out.map((r) => r.purchase_id)).toEqual(['a', 'd']); + }); + + test('multiple filters AND together (provider=aws + created_by=alice)', () => { + const filters: ApprovalQueueColumnFilters = { + provider: { kind: 'set', values: ['aws'] }, + created_by: { kind: 'set', values: ['alice@example.com'] }, + }; + const out = applyApprovalQueueColumnFilters(rows, filters); + expect(out.map((r) => r.purchase_id)).toEqual(['a']); + }); + + test('invalid numeric expression is skipped (filter is a no-op)', () => { + const filters: ApprovalQueueColumnFilters = { + savings: { kind: 'expr', expr: 'not-a-num' }, + }; + const out = applyApprovalQueueColumnFilters(rows, filters); + expect(out).toHaveLength(rows.length); + }); + + test('null monthly_cost fails every numeric predicate (NaN contract)', () => { + // Row c has monthly_cost: null. A "= 0" predicate must NOT match it, + // and a ">0" predicate must NOT match it either. + const eqZero: ApprovalQueueColumnFilters = { + monthly_cost: { kind: 'expr', expr: '0' }, + }; + expect(applyApprovalQueueColumnFilters(rows, eqZero).map((r) => r.purchase_id)) + .not.toContain('c'); + + const gtZero: ApprovalQueueColumnFilters = { + monthly_cost: { kind: 'expr', expr: '>0' }, + }; + expect(applyApprovalQueueColumnFilters(rows, gtZero).map((r) => r.purchase_id)) + .not.toContain('c'); + }); + + test('clearing filters via empty record returns a fresh clone', () => { + const out = applyApprovalQueueColumnFilters(rows, {}); + expect(out).toEqual(rows); + expect(out).not.toBe(rows); + }); +}); diff --git a/frontend/src/__tests__/history-column-filters.test.ts b/frontend/src/__tests__/history-column-filters.test.ts new file mode 100644 index 000000000..4de38746b --- /dev/null +++ b/frontend/src/__tests__/history-column-filters.test.ts @@ -0,0 +1,117 @@ +/** + * Purchase History column-filter regression suite (issue #166). + * + * Covers the inline per-column filters wired onto the Purchase History + * table headers. The existing Status chip-row remains the canonical + * filter for status — these tests exercise the new column-filter slice + * (provider/service/type/region/term + count/upfront_cost/savings). + * + * Tested matrix: + * 1. Numeric expr filter narrows by predicate (savings > N). + * 2. Categorical set filter narrows by membership (provider in set). + * 3. Multiple filters AND together across columns. + * 4. Invalid numeric expression is skipped (no exception, no narrowing). + * 5. Clearing a filter via setPurchaseHistoryColumnFilter(col, null) + * restores the full slice. + * 6. Term column treats absent vs zero correctly — categorical-empty + * filtering. + */ + +import { applyPurchaseHistoryColumnFilters } from '../history'; +import type { HistoryPurchase } from '../types'; +import type { PurchaseHistoryColumnFilters } from '../state'; + +// history.ts pulls in api/state/navigation transitively; the column-filter +// helper is pure (operates on the passed-in rows + filter record), but the +// module import path still resolves those — stub them out so the test runs +// without an apiBase / DOM context. +jest.mock('../api', () => ({})); +jest.mock('../navigation', () => ({ switchTab: jest.fn() })); +jest.mock('../utils', () => ({ + formatCurrency: jest.fn((v) => `$${v ?? 0}`), + formatDate: jest.fn((v) => v), + formatTerm: jest.fn((y) => `${y} Year${y === 1 ? '' : 's'}`), + escapeHtml: jest.fn((s) => s ?? ''), +})); +jest.mock('../state', () => ({ + subscribeProvider: jest.fn().mockReturnValue(() => {}), + subscribeAccount: jest.fn().mockReturnValue(() => {}), +})); +jest.mock('../confirmDialog', () => ({ confirmDialog: jest.fn() })); +jest.mock('../approval-details', () => ({ buildApprovalDetailsBody: jest.fn() })); +jest.mock('../toast', () => ({ showToast: jest.fn() })); +jest.mock('../lib/skeleton', () => ({ showSkeletonRows: jest.fn(), teardownSkeleton: jest.fn() })); +jest.mock('../recommendations', () => ({ getAccountName: jest.fn((id: string) => id) })); + +function mkRow(overrides: Partial): HistoryPurchase { + return { + purchase_id: 'p', + timestamp: '2024-01-01T00:00:00Z', + provider: 'aws', + service: 'ec2', + resource_type: 'reserved-instance', + region: 'us-east-1', + count: 1, + term: 1, + upfront_cost: 100, + estimated_savings: 50, + ...overrides, + }; +} + +const rows: HistoryPurchase[] = [ + mkRow({ purchase_id: 'a', provider: 'aws', service: 'ec2', region: 'us-east-1', count: 1, term: 1, upfront_cost: 100, estimated_savings: 50 }), + mkRow({ purchase_id: 'b', provider: 'aws', service: 'rds', region: 'us-west-2', count: 3, term: 3, upfront_cost: 500, estimated_savings: 200 }), + mkRow({ purchase_id: 'c', provider: 'azure', service: 'ec2', region: 'eu-west-1', count: 5, term: 1, upfront_cost: 1000, estimated_savings: 400 }), + mkRow({ purchase_id: 'd', provider: 'gcp', service: 'ec2', region: 'us-east-1', count: 2, term: 1, upfront_cost: 250, estimated_savings: 80 }), +]; + +describe('applyPurchaseHistoryColumnFilters', () => { + test('numeric expr: savings > 100 narrows to high-saving rows', () => { + const filters: PurchaseHistoryColumnFilters = { + savings: { kind: 'expr', expr: '>100' }, + }; + const out = applyPurchaseHistoryColumnFilters(rows, filters); + expect(out.map((r) => r.purchase_id)).toEqual(['b', 'c']); + }); + + test('categorical set: provider in {aws, gcp} excludes azure', () => { + const filters: PurchaseHistoryColumnFilters = { + provider: { kind: 'set', values: ['aws', 'gcp'] }, + }; + const out = applyPurchaseHistoryColumnFilters(rows, filters); + expect(out.map((r) => r.purchase_id)).toEqual(['a', 'b', 'd']); + }); + + test('multiple filters AND together (provider=aws + savings >= 100)', () => { + const filters: PurchaseHistoryColumnFilters = { + provider: { kind: 'set', values: ['aws'] }, + savings: { kind: 'expr', expr: '>=100' }, + }; + const out = applyPurchaseHistoryColumnFilters(rows, filters); + expect(out.map((r) => r.purchase_id)).toEqual(['b']); + }); + + test('invalid numeric expression is skipped (filter is a no-op)', () => { + const filters: PurchaseHistoryColumnFilters = { + savings: { kind: 'expr', expr: '>>nope' }, + }; + const out = applyPurchaseHistoryColumnFilters(rows, filters); + // Parse failure → filter ignored; full slice passes. + expect(out).toHaveLength(rows.length); + }); + + test('clearing filters via empty record returns a fresh clone', () => { + const out = applyPurchaseHistoryColumnFilters(rows, {}); + expect(out).toEqual(rows); + expect(out).not.toBe(rows); + }); + + test('term filter uses categorical-set semantics with stringified values', () => { + const filters: PurchaseHistoryColumnFilters = { + term: { kind: 'set', values: ['3'] }, + }; + const out = applyPurchaseHistoryColumnFilters(rows, filters); + expect(out.map((r) => r.purchase_id)).toEqual(['b']); + }); +}); From c4cfbe21d450e188c3f3aacb93cdfede8ecd3f8f Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 8 Jun 2026 12:58:16 -0700 Subject: [PATCH 5/9] test(frontend/history): add column-filter state mocks to cancel-permissions suite Rebasing onto feat/multicloud-web-frontend surfaced a gap: the new History/ApprovalQueue per-column-filter accessors added to ../state by this PR (issue #166) were missing from the ../state mock in history-cancel-permissions.test.ts. Without them renderApprovalQueue threw "getApprovalQueueColumnFilters is not a function", suppressing the whole approval-queue render and zeroing out the cancel buttons the permission-gating assertions depend on. Add the same six getter/setter/clear mocks already present in the other history test suites so the render path completes and the cancel-gating assertions exercise real button output again. refs #166 --- frontend/src/__tests__/history-cancel-permissions.test.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/frontend/src/__tests__/history-cancel-permissions.test.ts b/frontend/src/__tests__/history-cancel-permissions.test.ts index 515f355d3..99021e82c 100644 --- a/frontend/src/__tests__/history-cancel-permissions.test.ts +++ b/frontend/src/__tests__/history-cancel-permissions.test.ts @@ -60,6 +60,12 @@ jest.mock('../state', () => ({ getAmortizeUpfront: jest.fn().mockReturnValue(false), setAmortizeUpfront: jest.fn(), subscribeAmortizeUpfront: jest.fn().mockReturnValue(() => {}), + getPurchaseHistoryColumnFilters: jest.fn().mockReturnValue({}), + setPurchaseHistoryColumnFilter: jest.fn(), + clearAllPurchaseHistoryColumnFilters: jest.fn(), + getApprovalQueueColumnFilters: jest.fn().mockReturnValue({}), + setApprovalQueueColumnFilter: jest.fn(), + clearAllApprovalQueueColumnFilters: jest.fn(), })); // Mock permissions so we can inject arbitrary permission sets, including From 58c36cb2ec0f1489ba75af703500efac18e19b23 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 8 Jun 2026 15:54:47 -0700 Subject: [PATCH 6/9] fix(frontend/history): reset trigger aria-expanded on popover close closeOpenHistoryPopover removed the popover DOM but never restored the trigger button's aria-expanded to "false", leaving stale expanded accessibility state after outside-click, Escape, or toggle-close. Reset it on every close path, independent of focus restoration. --- frontend/src/lib/history-filter-popover.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/frontend/src/lib/history-filter-popover.ts b/frontend/src/lib/history-filter-popover.ts index 4fedce514..3a21b429a 100644 --- a/frontend/src/lib/history-filter-popover.ts +++ b/frontend/src/lib/history-filter-popover.ts @@ -74,10 +74,12 @@ export function closeOpenHistoryPopover(restoreFocus = false): void { el.remove(); openPopover = null; detachGlobalListeners(); + const trigger = document.querySelector( + `.history-column-filter-btn[data-column="${CSS.escape(column)}"]`, + ); + // Always clear the stale expanded state, regardless of focus restoration. + trigger?.setAttribute('aria-expanded', 'false'); if (restoreFocus) { - const trigger = document.querySelector( - `.history-column-filter-btn[data-column="${CSS.escape(column)}"]`, - ); trigger?.focus(); } lastAnchorSelector = null; From 3f0da167842d7b107b0062894ed077c9a43488ee Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 19 Jun 2026 17:45:14 +0200 Subject: [PATCH 7/9] sec(frontend/history): escapeHtmlAttr on filter button label+column attrs renderHistoryFilterButton injected `column` and `label` raw into aria-label, title, and data-column attributes via innerHTML template literal. Apply escapeHtmlAttr to both before interpolation and replace the em-dash in the active-state aria-label with a hyphen. Add history-filter-popover-xss.test.ts to assert hostile payloads in label and column are entity-encoded, not executed. --- .../history-filter-popover-xss.test.ts | 63 +++++++++++++++++++ frontend/src/lib/history-filter-popover.ts | 8 ++- 2 files changed, 69 insertions(+), 2 deletions(-) create mode 100644 frontend/src/__tests__/history-filter-popover-xss.test.ts diff --git a/frontend/src/__tests__/history-filter-popover-xss.test.ts b/frontend/src/__tests__/history-filter-popover-xss.test.ts new file mode 100644 index 000000000..9d84b8cf9 --- /dev/null +++ b/frontend/src/__tests__/history-filter-popover-xss.test.ts @@ -0,0 +1,63 @@ +/** + * XSS regression for renderHistoryFilterButton (issue #166 follow-up). + * + * renderHistoryFilterButton injects `column` and `label` into HTML attribute + * values (data-column, aria-label, title) via template literal. Any caller + * passing a user-controlled string without escaping would produce a stored-XSS + * vector. escapeHtmlAttr must be applied to both values before interpolation. + */ + +import { renderHistoryFilterButton } from '../lib/history-filter-popover'; + +// Use the real escapeHtmlAttr so DOM-based escaping is exercised, not a +// pass-through stub. +jest.mock('../lib/column-filters', () => ({ + parseNumericFilter: jest.fn(), +})); + +describe('renderHistoryFilterButton: attribute-injection XSS guard', () => { + // This payload attempts to break out of the aria-label attribute and inject + // a script element. In raw form it would produce: + // aria-label=""> tag into attribute context', () => { + const html = renderHistoryFilterButton('provider', SCRIPT_PAYLOAD, false); + // The raw < > and " chars must be entity-encoded; no unescaped angle brackets. + expect(html).not.toContain('">'); + // The encoded form is present (attribute value is escaped, not stripped). + expect(html).toContain('<script>'); + }); + + test('hostile label does not inject event handler attribute', () => { + const html = renderHistoryFilterButton('provider', EVENT_PAYLOAD, false); + // The injected " must be encoded; no raw onmouseover= outside an attribute value. + expect(html).not.toContain('" onmouseover='); + // The encoded form must appear inside the attribute value. + expect(html).toContain('" onmouseover='); + }); + + test('hostile column id does not inject raw markup into data-column', () => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const html = renderHistoryFilterButton(SCRIPT_PAYLOAD as any, 'Provider', false); + expect(html).not.toContain('>'); + expect(html).toContain('<script>'); + }); + + test('safe column and label pass through correctly', () => { + const html = renderHistoryFilterButton('provider', 'Provider', false); + expect(html).toContain('data-column="provider"'); + expect(html).toContain('aria-label="Filter Provider"'); + expect(html).toContain('class="history-column-filter-btn"'); + }); + + test('active flag adds "active" class and updates aria-label', () => { + const html = renderHistoryFilterButton('savings', 'Monthly Savings', true); + expect(html).toContain('class="history-column-filter-btn active"'); + expect(html).toContain('aria-label="Filter Monthly Savings - currently active"'); + }); +}); diff --git a/frontend/src/lib/history-filter-popover.ts b/frontend/src/lib/history-filter-popover.ts index 3a21b429a..7c7a48a65 100644 --- a/frontend/src/lib/history-filter-popover.ts +++ b/frontend/src/lib/history-filter-popover.ts @@ -19,6 +19,7 @@ import { parseNumericFilter, type ColumnFilterKind, } from './column-filters'; +import { escapeHtmlAttr } from '../utils'; export interface PopoverConfig { column: TColumnId; @@ -350,8 +351,11 @@ export function renderHistoryFilterButton( active: boolean, ): string { const cls = `history-column-filter-btn${active ? ' active' : ''}`; - const ariaLabel = active ? `Filter ${label} — currently active` : `Filter ${label}`; + const ariaLabel = escapeHtmlAttr( + active ? `Filter ${label} - currently active` : `Filter ${label}`, + ); + const safeColumn = escapeHtmlAttr(String(column)); // Use the same gear-style glyph (⛛) the recommendations popover // uses so the two surfaces are visually identical to the user. - return ``; + return ``; } From e198591a1e7e48da1dae4c6b071a71c01f312f51 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 10 Jul 2026 16:39:18 +0200 Subject: [PATCH 8/9] test(frontend/history): add column-filter state mocks to revoke-button suite The #290 revoke-button suite landed on main after this branch added the column-filter state accessors to the other history test suites, so its ../state mock lacked getPurchaseHistoryColumnFilters and the five sibling accessors. After rebasing onto main the render path in history.ts calls them, throws in the mocked suite, and suppresses the whole history-list render, zeroing out the inline Revoke buttons the tests assert on. Add the same six getter/setter/clear mocks already present in the other history suites so the render path completes and the revoke-gating assertions exercise real button output again. refs #166 --- frontend/src/__tests__/history-revoke-button.test.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/frontend/src/__tests__/history-revoke-button.test.ts b/frontend/src/__tests__/history-revoke-button.test.ts index 6f8740fc5..325e7b788 100644 --- a/frontend/src/__tests__/history-revoke-button.test.ts +++ b/frontend/src/__tests__/history-revoke-button.test.ts @@ -60,6 +60,12 @@ jest.mock('../state', () => ({ getAmortizeUpfront: jest.fn().mockReturnValue(false), setAmortizeUpfront: jest.fn(), subscribeAmortizeUpfront: jest.fn().mockReturnValue(() => {}), + getPurchaseHistoryColumnFilters: jest.fn().mockReturnValue({}), + setPurchaseHistoryColumnFilter: jest.fn(), + clearAllPurchaseHistoryColumnFilters: jest.fn(), + getApprovalQueueColumnFilters: jest.fn().mockReturnValue({}), + setApprovalQueueColumnFilter: jest.fn(), + clearAllApprovalQueueColumnFilters: jest.fn(), })); import * as api from '../api'; From 51af0ff26fc4720436e46500e186d3f5e8a2961e Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 17 Jul 2026 19:35:24 +0300 Subject: [PATCH 9/9] test(frontend/history): add History column-filter mocks to marketplace-sell suite loadHistory() now calls state.getPurchaseHistoryColumnFilters() and state.getApprovalQueueColumnFilters() (added by this PR). The history-marketplace-sell-button test mocked state but omitted these new functions, causing the two sell-button-shown assertions to receive [] instead of the expected button IDs. Add the six new state stubs (get/set/clearAll for both slices) matching the pattern already established in the revoke-button and cancel-permissions suites. --- .../src/__tests__/history-marketplace-sell-button.test.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/frontend/src/__tests__/history-marketplace-sell-button.test.ts b/frontend/src/__tests__/history-marketplace-sell-button.test.ts index 9da8ae1b1..77d10e108 100644 --- a/frontend/src/__tests__/history-marketplace-sell-button.test.ts +++ b/frontend/src/__tests__/history-marketplace-sell-button.test.ts @@ -65,6 +65,12 @@ jest.mock('../state', () => ({ getAmortizeUpfront: jest.fn().mockReturnValue(false), setAmortizeUpfront: jest.fn(), subscribeAmortizeUpfront: jest.fn().mockReturnValue(() => {}), + getPurchaseHistoryColumnFilters: jest.fn().mockReturnValue({}), + setPurchaseHistoryColumnFilter: jest.fn(), + clearAllPurchaseHistoryColumnFilters: jest.fn(), + getApprovalQueueColumnFilters: jest.fn().mockReturnValue({}), + setApprovalQueueColumnFilter: jest.fn(), + clearAllApprovalQueueColumnFilters: jest.fn(), })); import * as api from '../api';
DateAccountProviderServiceCountTermPayment${monthlyColHeader}Upfront CostMonthly SavingsCreated byAccount${fbtn('account')}Provider${fbtn('provider')}Service${fbtn('service')}Count${fbtn('count')}Term${fbtn('term')}Payment${fbtn('payment')}${monthlyColHeader}${fbtn('monthly_cost')}Upfront Cost${fbtn('upfront_cost')}Monthly Savings${fbtn('savings')}Created by${fbtn('created_by')} Actions