From 49f201f0e568a55030a0b9727f0e081a2ee00ed9 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 28 May 2026 20:50:51 +0200 Subject: [PATCH 1/3] ux(recommendations): cascading categorical filter distinct values (#164) When building a categorical filter popover for column X, apply all OTHER active column filters to the rec set first. This means selecting Provider=AWS before opening the Service popover now shows only AWS services instead of the full cross-provider list, eliminating ghost values that produce zero rows. Mitigation for the "broaden a column" UX: any value that is part of the column's own currently-active filter is always included in the distinct list even if the cross-filtered set would omit it, so the user can change or deselect the existing value without first clearing every other filter. Algorithm: full re-scan of the cross-filtered rec set on each popover open via the existing applyColumnFilters path. Tests: 2 new assertions -- service popover narrows to AWS-only services when Provider=AWS is active; provider popover still shows all providers when Provider=AWS is its own active filter (own-filter exclusion). --- .../src/__tests__/recommendations.test.ts | 57 ++++++++++++++++++- frontend/src/recommendations.ts | 35 +++++++++++- 2 files changed, 89 insertions(+), 3 deletions(-) diff --git a/frontend/src/__tests__/recommendations.test.ts b/frontend/src/__tests__/recommendations.test.ts index fc0dc04ac..3558240f7 100644 --- a/frontend/src/__tests__/recommendations.test.ts +++ b/frontend/src/__tests__/recommendations.test.ts @@ -2357,7 +2357,7 @@ describe('Bundle B: column header filter triggers', () => { expect(document.querySelector('.column-filter-popover')).toBeNull(); }); - test('categorical popover lists distinct values from the unfiltered rec set', async () => { + test('categorical popover lists distinct values from the rec set (no other active filters)', async () => { await loadRecommendations(); const providerBtn = document.querySelector('th .column-filter-btn[data-column="provider"]'); providerBtn?.click(); @@ -2819,6 +2819,61 @@ describe('Bundle B: column header filter triggers', () => { expect(values).toContain('ec2'); }); }); + + // Issue #164: cross-column-aware (cascading) distinct values. + // Each column popover must show only values from rows that pass all + // OTHER active filters -- values that produce non-empty results when + // combined with the existing selection. + describe('Issue #164: cross-column-aware categorical filter distinct values', () => { + const multiProviderRecs = [ + { id: 'r-aws-ec2', provider: 'aws', cloud_account_id: 'a1', service: 'ec2', resource_type: 't3.medium', region: 'us-east-1', count: 1, term: 1, savings: 100, upfront_cost: 500 }, + { id: 'r-aws-rds', provider: 'aws', cloud_account_id: 'a1', service: 'rds', resource_type: 'db.t3', region: 'us-east-1', count: 1, term: 1, savings: 80, upfront_cost: 400 }, + { id: 'r-az-vm', provider: 'azure', cloud_account_id: 'a2', service: 'vm', resource_type: 'D2s', region: 'eastus', count: 2, term: 3, savings: 200, upfront_cost: 800 }, + ]; + + beforeEach(() => { + (api.getRecommendations as jest.Mock).mockResolvedValue({ + summary: {}, + recommendations: multiProviderRecs, + regions: [], + }); + (state.getRecommendations as jest.Mock).mockReturnValue(multiProviderRecs); + (state.getVisibleRecommendations as jest.Mock).mockReturnValue(multiProviderRecs); + }); + + test('service popover shows only AWS services when provider=aws filter is active', async () => { + // Provider=aws is set; opening the service popover must list only + // services from aws rows (ec2, rds), not the azure service (vm). + (state.getRecommendationsColumnFilters as jest.Mock).mockReturnValue({ + provider: { kind: 'set', values: ['aws'] }, + }); + await loadRecommendations(); + const serviceBtn = document.querySelector('th .column-filter-btn[data-column="service"]'); + serviceBtn?.click(); + const values = Array.from( + document.querySelectorAll('.column-filter-popover .column-filter-item input[type="checkbox"]'), + ).map((cb) => cb.dataset['value']).sort(); + expect(values).toEqual(['ec2', 'rds']); + expect(values).not.toContain('vm'); + }); + + test('provider popover shows all providers when opening the provider column (own filter excluded)', async () => { + // Provider=aws is active, but opening the provider popover must still + // show all 3 providers because the column's own filter is excluded from + // the cross-filter narrowing -- allowing the user to switch providers. + (state.getRecommendationsColumnFilters as jest.Mock).mockReturnValue({ + provider: { kind: 'set', values: ['aws'] }, + }); + await loadRecommendations(); + const providerBtn = document.querySelector('th .column-filter-btn[data-column="provider"]'); + providerBtn?.click(); + const values = Array.from( + document.querySelectorAll('.column-filter-popover .column-filter-item input[type="checkbox"]'), + ).map((cb) => cb.dataset['value']).sort(); + // All providers must be visible so the user can switch from aws to azure. + expect(values).toEqual(['aws', 'azure']); + }); + }); }); describe('Bundle B: sticky bottom action box', () => { diff --git a/frontend/src/recommendations.ts b/frontend/src/recommendations.ts index 6cf8b6f46..9b9f2c5d3 100644 --- a/frontend/src/recommendations.ts +++ b/frontend/src/recommendations.ts @@ -1928,6 +1928,10 @@ function positionPopover(popover: HTMLElement, anchor: HTMLElement): void { function distinctValuesForColumn( recs: readonly LocalRecommendation[], column: state.RecommendationsColumnId, + // Values to include regardless of what appears in `recs` (used so the + // currently-active filter selection for the column being edited remains + // visible even when cross-column filtering would otherwise hide it). + alwaysInclude?: ReadonlySet, ): string[] { // Numeric columns don't get a checkbox list, but we still call this for // categorical columns only. @@ -1935,6 +1939,9 @@ function distinctValuesForColumn( for (const r of recs) { seen.add(categoricalCellValue(r, column)); } + if (alwaysInclude) { + for (const v of alwaysInclude) seen.add(v); + } return Array.from(seen).sort((a, b) => { if (a === '' && b !== '') return -1; // (empty) first if (a !== '' && b === '') return 1; @@ -1963,9 +1970,15 @@ function categoricalDisplayLabel( // Build the popover DOM for a given column. Categorical: checkbox list with // (All) tri-state + Clear footer. Numeric: free-text expression input with // inline error and Clear footer. +// +// `recs` should already be pre-filtered to the cross-column narrowed set +// (all filters except this column's own applied). `alwaysInclude` pins any +// currently-active values for this column so the user can still deselect +// them even when the cross-filtered set would otherwise omit them. function buildPopoverContent( column: state.RecommendationsColumnId, recs: readonly LocalRecommendation[], + alwaysInclude?: ReadonlySet, ): { el: HTMLDivElement; checkboxes: Map; input: HTMLInputElement | null; errorEl: HTMLElement | null } { const popover = document.createElement('div'); popover.className = 'column-filter-popover'; @@ -2034,7 +2047,7 @@ function buildPopoverContent( } }); } else { - const distinct = distinctValuesForColumn(recs, column); + const distinct = distinctValuesForColumn(recs, column, alwaysInclude); // (All) tri-state checkbox at the top. const allLabel = document.createElement('label'); @@ -2337,7 +2350,25 @@ function openColumnPopover(column: state.RecommendationsColumnId, anchor: HTMLEl if (openPopover) closePopover(); const recs = state.getRecommendations() as unknown as LocalRecommendation[]; - const built = buildPopoverContent(column, recs); + // Cross-column-aware distinct values (issue #164): build the checkbox list + // from rows that pass all OTHER active filters, not the full unfiltered set. + // This way, selecting Provider=AWS first narrows the Service popover to only + // AWS services -- values that produce non-empty results. + // + // Mitigation for the "broaden a single column" UX: any value that is part of + // the column's own currently-active filter is always included even if the + // cross-filtered set would omit it (e.g. opening Provider popover while + // Provider=AWS + Service=ec2 are both active still shows AWS so the user can + // deselect it without first clearing the Service filter). + const allFilters = state.getRecommendationsColumnFilters(); + const filtersExceptThisColumn: state.RecommendationsColumnFilters = { ...allFilters }; + delete filtersExceptThisColumn[column]; + const recsForDistinct = applyColumnFilters(recs, filtersExceptThisColumn); + const ownFilter = allFilters[column]; + const alwaysInclude: ReadonlySet = ownFilter && ownFilter.kind === 'set' + ? new Set(ownFilter.values) + : new Set(); + const built = buildPopoverContent(column, recsForDistinct, alwaysInclude); document.body.appendChild(built.el); openPopover = { column, From ac25809d17a143dff6cfcee3dc3fee831043d639 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Sat, 6 Jun 2026 08:19:03 +0200 Subject: [PATCH 2/3] test(recommendations): regression for alwaysInclude in cascading filter popover Add a contradictory-filter test (provider=azure + service=ec2) that verifies values from a column's own active filter remain visible in the popover even when cross-filtering by other columns would omit them. This pins the alwaysInclude behavior added in the cascading-filter implementation: without it, opening the service popover while provider=azure is active would drop ec2 from the list entirely, making it impossible to deselect without first clearing the provider filter. --- .../src/__tests__/recommendations.test.ts | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/frontend/src/__tests__/recommendations.test.ts b/frontend/src/__tests__/recommendations.test.ts index 3558240f7..5a05ec37a 100644 --- a/frontend/src/__tests__/recommendations.test.ts +++ b/frontend/src/__tests__/recommendations.test.ts @@ -2873,6 +2873,28 @@ describe('Bundle B: column header filter triggers', () => { // All providers must be visible so the user can switch from aws to azure. expect(values).toEqual(['aws', 'azure']); }); + + test('service popover pins active-filter values even when cross-filtering would omit them (alwaysInclude)', async () => { + // Contradictory filter state: provider=azure (only has vm) + service=ec2 (no azure rows). + // Opening the service popover: cross-filtering by provider=azure yields only vm. + // But ec2 is in the service column's own active filter, so alwaysInclude must keep it + // visible so the user can deselect it without first clearing the provider filter. + (state.getRecommendationsColumnFilters as jest.Mock).mockReturnValue({ + provider: { kind: 'set', values: ['azure'] }, + service: { kind: 'set', values: ['ec2'] }, + }); + await loadRecommendations(); + const serviceBtn = document.querySelector('th .column-filter-btn[data-column="service"]'); + serviceBtn?.click(); + const values = Array.from( + document.querySelectorAll('.column-filter-popover .column-filter-item input[type="checkbox"]'), + ).map((cb) => cb.dataset['value']).sort(); + // vm is visible because provider=azure cross-filter leaves only that azure row. + expect(values).toContain('vm'); + // ec2 must also be visible even though no azure row has service=ec2: + // the alwaysInclude set pins values from the column's own active filter. + expect(values).toContain('ec2'); + }); }); }); From 4d043d8cd83ef34daaa9dc55c2c75fbc86c5e0a4 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 19 Jun 2026 23:05:06 +0200 Subject: [PATCH 3/3] test(recommendations): assert hostile payload escaping in cascading filter popover Add a regression test that verifies a service value containing an HTML injection string () is stored in dataset and rendered via textContent -- never interpreted as markup -- when the cascading filter popover builds its checkbox list. --- .../src/__tests__/recommendations.test.ts | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/frontend/src/__tests__/recommendations.test.ts b/frontend/src/__tests__/recommendations.test.ts index 5a05ec37a..f95dfc281 100644 --- a/frontend/src/__tests__/recommendations.test.ts +++ b/frontend/src/__tests__/recommendations.test.ts @@ -2895,6 +2895,44 @@ describe('Bundle B: column header filter triggers', () => { // the alwaysInclude set pins values from the column's own active filter. expect(values).toContain('ec2'); }); + + test('hostile payload in service value is not interpreted as HTML in the popover (XSS guard)', async () => { + // A recommendation whose `service` field contains an HTML injection string + // must not be parsed as markup in the filter popover -- the value goes through + // textContent and dataset, never innerHTML. + const hostileService = ''; + const recsWithHostile = [ + { id: 'r-aws-ec2', provider: 'aws', cloud_account_id: 'a1', service: hostileService, resource_type: 't3.medium', region: 'us-east-1', count: 1, term: 1, savings: 100, upfront_cost: 500 }, + { id: 'r-aws-rds', provider: 'aws', cloud_account_id: 'a1', service: 'rds', resource_type: 'db.t3', region: 'us-east-1', count: 1, term: 1, savings: 80, upfront_cost: 400 }, + ]; + (api.getRecommendations as jest.Mock).mockResolvedValue({ + summary: {}, + recommendations: recsWithHostile, + regions: [], + }); + (state.getRecommendations as jest.Mock).mockReturnValue(recsWithHostile); + (state.getVisibleRecommendations as jest.Mock).mockReturnValue(recsWithHostile); + (state.getRecommendationsColumnFilters as jest.Mock).mockReturnValue({}); + await loadRecommendations(); + const serviceBtn = document.querySelector('th .column-filter-btn[data-column="service"]'); + serviceBtn?.click(); + + // The popover must not contain a parsed