diff --git a/frontend/src/__tests__/recommendations.test.ts b/frontend/src/__tests__/recommendations.test.ts index 4ae91b917..5a10e695c 100644 --- a/frontend/src/__tests__/recommendations.test.ts +++ b/frontend/src/__tests__/recommendations.test.ts @@ -1,7 +1,7 @@ /** * Recommendations module tests */ -import { loadRecommendations, openPurchaseModal, getPurchaseModalRecommendations, clearPurchaseModalRecommendations, refreshRecommendations, setupRecommendationsHandlers, pickBestVariantPerCell, seedGlobalDefaults, effectiveMonthlySavings, effectiveSavingsPct, onDemandMonthly, groupRecsByCell, cellSummary, pageLevelRange, resetExpandedCells, resetAutoRefreshInFlight, scaleCost, formatCostForPeriod, periodSuffix, loadColumnVisibility, saveColumnVisibility, resetColumnVisibilityState, TOGGLEABLE_COLUMNS, COLUMN_DEFS, isHomogeneousSelection, renderUsageSparkline } from '../recommendations'; +import { loadRecommendations, openPurchaseModal, getPurchaseModalRecommendations, clearPurchaseModalRecommendations, refreshRecommendations, setupRecommendationsHandlers, pickBestVariantPerCell, seedGlobalDefaults, effectiveMonthlySavings, effectiveSavingsPct, onDemandMonthly, groupRecsByCell, cellSummary, pageLevelRange, resetExpandedCells, resetAutoRefreshInFlight, scaleCost, formatCostForPeriod, periodSuffix, loadColumnVisibility, saveColumnVisibility, resetColumnVisibilityState, TOGGLEABLE_COLUMNS, COLUMN_DEFS, isHomogeneousSelection, renderUsageSparkline, loadColumnFilters, saveColumnFilters, resetColumnFiltersState } from '../recommendations'; import type { CostPeriod } from '../state'; // Mock the api module @@ -6678,3 +6678,184 @@ describe('renderUsageSparkline (issue #239)', () => { expect(pairs[6]).toMatch(/^56\.0,/); }); }); + +// ============================================================================ +// Issue #163: Column filter localStorage persistence +// ============================================================================ + +describe('Column filters localStorage persistence (issue #163)', () => { + beforeEach(() => { + resetColumnFiltersState(); + // localStorageMock is reset by jest.clearAllMocks() in the global beforeEach + }); + + // --- loadColumnFilters --- + + describe('loadColumnFilters', () => { + test('returns empty object when localStorage key is absent', () => { + // Default mock: getItem returns null + const result = loadColumnFilters(); + expect(Object.keys(result)).toHaveLength(0); + }); + + test('returns empty object on JSON parse error', () => { + localStorageMock.getItem.mockReturnValue('{not valid json}'); + const result = loadColumnFilters(); + expect(Object.keys(result)).toHaveLength(0); + }); + + test('returns empty object when schemaVersion is wrong', () => { + localStorageMock.getItem.mockReturnValue( + JSON.stringify({ schemaVersion: 99, filters: { region: { kind: 'set', values: ['us-east-1'] } } }), + ); + const result = loadColumnFilters(); + expect(Object.keys(result)).toHaveLength(0); + }); + + test('returns empty object when filters is not an object', () => { + localStorageMock.getItem.mockReturnValue( + JSON.stringify({ schemaVersion: 1, filters: 'not-an-object' }), + ); + const result = loadColumnFilters(); + expect(Object.keys(result)).toHaveLength(0); + }); + + test('restores a categorical (set) filter correctly', () => { + localStorageMock.getItem.mockReturnValue( + JSON.stringify({ + schemaVersion: 1, + filters: { region: { kind: 'set', values: ['us-east-1', 'eu-west-1'] } }, + }), + ); + const result = loadColumnFilters(); + expect(result.region).toEqual({ kind: 'set', values: ['us-east-1', 'eu-west-1'] }); + expect(Object.keys(result)).toHaveLength(1); + }); + + test('restores a numeric (expr) filter correctly', () => { + localStorageMock.getItem.mockReturnValue( + JSON.stringify({ + schemaVersion: 1, + filters: { savings: { kind: 'expr', expr: '>100' } }, + }), + ); + const result = loadColumnFilters(); + expect(result.savings).toEqual({ kind: 'expr', expr: '>100' }); + }); + + test('silently drops unknown column keys', () => { + localStorageMock.getItem.mockReturnValue( + JSON.stringify({ + schemaVersion: 1, + filters: { + region: { kind: 'set', values: ['us-east-1'] }, + future_unknown_column: { kind: 'set', values: ['foo'] }, + }, + }), + ); + const result = loadColumnFilters(); + expect(result.region).toBeDefined(); + expect((result as Record)['future_unknown_column']).toBeUndefined(); + expect(Object.keys(result)).toHaveLength(1); + }); + + test('silently drops filters with malformed shape', () => { + localStorageMock.getItem.mockReturnValue( + JSON.stringify({ + schemaVersion: 1, + filters: { + region: { kind: 'set', values: 'not-an-array' }, + savings: { kind: 'expr', expr: 42 }, + count: { kind: 'unknown-kind', data: 'x' }, + }, + }), + ); + const result = loadColumnFilters(); + expect(Object.keys(result)).toHaveLength(0); + }); + + test('silently drops expr filter with empty string expr', () => { + localStorageMock.getItem.mockReturnValue( + JSON.stringify({ + schemaVersion: 1, + filters: { savings: { kind: 'expr', expr: '' } }, + }), + ); + const result = loadColumnFilters(); + expect(Object.keys(result)).toHaveLength(0); + }); + }); + + // --- saveColumnFilters --- + + describe('saveColumnFilters', () => { + test('writes correct JSON shape to localStorage', () => { + saveColumnFilters({ region: { kind: 'set', values: ['us-east-1'] } }); + expect(localStorageMock.setItem).toHaveBeenCalledWith( + 'cudly.recs.columnFilters.v1', + expect.stringContaining('"schemaVersion":1'), + ); + const callArg = localStorageMock.setItem.mock.calls[0]?.[1] as string; + const parsed = JSON.parse(callArg); + expect(parsed.schemaVersion).toBe(1); + expect(parsed.filters.region).toEqual({ kind: 'set', values: ['us-east-1'] }); + }); + + test('writes empty filters object when no filters active', () => { + saveColumnFilters({}); + const callArg = localStorageMock.setItem.mock.calls[0]?.[1] as string; + const parsed = JSON.parse(callArg); + expect(parsed.filters).toEqual({}); + }); + + test('round-trips a numeric expr filter', () => { + saveColumnFilters({ savings: { kind: 'expr', expr: '>500' } }); + const callArg = localStorageMock.setItem.mock.calls[0]?.[1] as string; + const payload = JSON.parse(callArg); + expect(payload.filters.savings).toEqual({ kind: 'expr', expr: '>500' }); + }); + }); + + // --- round-trip and guard tests --- + + describe('persistence round-trip', () => { + test('save then load restores same filter state', () => { + const filters = { + region: { kind: 'set' as const, values: ['ap-southeast-1'] }, + savings: { kind: 'expr' as const, expr: '>200' }, + }; + const store = new Map(); + localStorageMock.getItem.mockImplementation((k: string) => store.get(k) ?? null); + localStorageMock.setItem.mockImplementation((k: string, v: string) => { store.set(k, v); }); + saveColumnFilters(filters); + const restored = loadColumnFilters(); + expect(restored.region).toEqual({ kind: 'set', values: ['ap-southeast-1'] }); + expect(restored.savings).toEqual({ kind: 'expr', expr: '>200' }); + expect(Object.keys(restored)).toHaveLength(2); + }); + + test('stored key with unknown column is silently dropped, no error', () => { + const store = new Map([ + ['cudly.recs.columnFilters.v1', JSON.stringify({ + schemaVersion: 1, + filters: { + region: { kind: 'set', values: ['us-east-1'] }, + deleted_account_col: { kind: 'set', values: ['123456789012'] }, + }, + })], + ]); + localStorageMock.getItem.mockImplementation((k: string) => store.get(k) ?? null); + expect(() => loadColumnFilters()).not.toThrow(); + const result = loadColumnFilters(); + expect(result.region).toBeDefined(); + expect((result as Record)['deleted_account_col']).toBeUndefined(); + }); + + test('corrupted JSON in localStorage falls back to empty object, no error', () => { + localStorageMock.getItem.mockReturnValue('}{invalid json]['); + expect(() => loadColumnFilters()).not.toThrow(); + const result = loadColumnFilters(); + expect(Object.keys(result)).toHaveLength(0); + }); + }); +}); diff --git a/frontend/src/recommendations.ts b/frontend/src/recommendations.ts index 83f412b72..b5890002f 100644 --- a/frontend/src/recommendations.ts +++ b/frontend/src/recommendations.ts @@ -1842,6 +1842,7 @@ function buildPopoverContent( const expr = input!.value.trim(); if (expr === '') { state.setRecommendationsColumnFilter(column, null); + saveColumnFilters(state.getRecommendationsColumnFilters()); errorEl!.textContent = ''; rerenderRecommendations(); return; @@ -1853,6 +1854,7 @@ function buildPopoverContent( } errorEl!.textContent = ''; state.setRecommendationsColumnFilter(column, { kind: 'expr', expr }); + saveColumnFilters(state.getRecommendationsColumnFilters()); rerenderRecommendations(); }; input.addEventListener('blur', commit); @@ -1955,6 +1957,7 @@ function buildPopoverContent( } else { state.setRecommendationsColumnFilter(column, { kind: 'set', values: selected }); } + saveColumnFilters(state.getRecommendationsColumnFilters()); updateAllTriState(); updateSPTriState(); rerenderRecommendations(); @@ -1973,6 +1976,7 @@ function buildPopoverContent( } else { state.setRecommendationsColumnFilter(column, { kind: 'set', values: [] }); } + saveColumnFilters(state.getRecommendationsColumnFilters()); updateAllTriState(); updateSPTriState(); rerenderRecommendations(); @@ -2016,6 +2020,7 @@ function buildPopoverContent( if (input) { // Numeric column: Clear drops the expression entirely (no filter). state.setRecommendationsColumnFilter(column, null); + saveColumnFilters(state.getRecommendationsColumnFilters()); input.value = ''; if (errorEl) errorEl.textContent = ''; rerenderRecommendations(); @@ -2454,6 +2459,7 @@ function renderFilterStatusBar(loadedCount: number, visibleCount: number): void badge.className = 'clear-filters'; badge.addEventListener('click', () => { state.clearAllRecommendationsColumnFilters(); + saveColumnFilters(state.getRecommendationsColumnFilters()); rerenderRecommendations(); }); bar.insertBefore(badge, live); @@ -3100,6 +3106,79 @@ export function saveColumnVisibility(hidden: ReadonlySet([ + 'provider', 'account', 'service', 'resource_type', 'region', + 'count', 'term', 'payment', 'savings', 'upfront_cost', + 'monthly_cost', 'on_demand_monthly', 'effective_savings_pct', +]); + +interface ColumnFiltersSchema { + schemaVersion: number; + filters: Record; +} + +/** Load column filter state from localStorage. Returns empty object on any error. Exported for tests. */ +export function loadColumnFilters(): state.RecommendationsColumnFilters { + try { + const raw = localStorage.getItem(COLUMN_FILTERS_LS_KEY); + if (!raw) return {}; + const parsed = JSON.parse(raw) as Partial; + if (parsed.schemaVersion !== COLUMN_FILTERS_SCHEMA_VERSION) return {}; + if (!parsed.filters || typeof parsed.filters !== 'object' || Array.isArray(parsed.filters)) return {}; + const result: state.RecommendationsColumnFilters = {}; + for (const [key, value] of Object.entries(parsed.filters)) { + // Drop unknown column ids (stale after a column rename or deletion). + if (!VALID_COLUMN_IDS.has(key as state.RecommendationsColumnId)) continue; + // Validate filter shape: must be kind:'set' with string[] or kind:'expr' with string. + if (value && value.kind === 'set' && Array.isArray(value.values) + && value.values.every((v) => typeof v === 'string')) { + result[key as state.RecommendationsColumnId] = { kind: 'set', values: value.values }; + } else if (value && value.kind === 'expr' && typeof value.expr === 'string' && value.expr !== '') { + result[key as state.RecommendationsColumnId] = { kind: 'expr', expr: value.expr }; + } + // Anything else (malformed, hand-edited) is silently dropped. + } + return result; + } catch { + return {}; + } +} + +/** Persist column filter state to localStorage. Exported for tests. */ +export function saveColumnFilters(filters: state.RecommendationsColumnFilters): void { + try { + const payload: ColumnFiltersSchema = { + schemaVersion: COLUMN_FILTERS_SCHEMA_VERSION, + filters: filters as Record, + }; + localStorage.setItem(COLUMN_FILTERS_LS_KEY, JSON.stringify(payload)); + } catch { + // Private-browsing / quota-exceeded — non-fatal. + } +} + +// Seed flag: set to true once column filters are loaded from localStorage +// on first render so subsequent renders don't overwrite in-session changes. +let columnFiltersSeeded = false; + +/** + * Reset column-filter seeded state. Exported for tests only — not part of + * the public API. Call in beforeEach to ensure tests don't share seeding state. + */ +export function resetColumnFiltersState(): void { + columnFiltersSeeded = false; + state.clearAllRecommendationsColumnFilters(); +} + // Seed flag: set to true once column visibility is loaded from localStorage // on first render so subsequent renders don't overwrite in-session toggles. let columnVisibilitySeeded = false; @@ -3859,6 +3938,20 @@ function renderRecommendationsList(loadedRecs: LocalRecommendation[]): void { const container = document.getElementById('recommendations-list'); if (!container) return; + // Seed column filters from localStorage on the first render (issue #163). + // columnFiltersSeeded stays true for the rest of the session so in-session + // changes are not overwritten on subsequent rerenders. + if (!columnFiltersSeeded) { + const persisted = loadColumnFilters(); + for (const [col, filter] of Object.entries(persisted)) { + state.setRecommendationsColumnFilter( + col as state.RecommendationsColumnId, + filter, + ); + } + columnFiltersSeeded = true; + } + // Seed column visibility from localStorage on the first render. // columnVisibilitySeeded stays true for the rest of the session so // in-session toggles (via the "Columns ▾" popover) are not overwritten.