diff --git a/frontend/src/__tests__/recommendations.test.ts b/frontend/src/__tests__/recommendations.test.ts index a0a1bc07c..8a01d8879 100644 --- a/frontend/src/__tests__/recommendations.test.ts +++ b/frontend/src/__tests__/recommendations.test.ts @@ -5170,6 +5170,38 @@ describe('Issues #225 + #226: cell grouping with savings range and collapse/expa const variantRows = document.querySelectorAll('.rec-variant-row'); expect(variantRows.length).toBe(2); }); + + // QA 4.13: expand/collapse state must be reset when the data source + // reloads due to a provider/account Global filter change. Before the fix, + // stale expandedCells keys from the prior provider desynchronised the + // Expand-All button label and left newly-shown groups collapsed. + test('QA 4.13: expand state clears on loadRecommendations (provider filter change)', async () => { + const recs = multiVariantRecs(); + (api.getRecommendations as jest.Mock).mockResolvedValue({ summary: {}, recommendations: recs }); + (state.getRecommendations as jest.Mock).mockReturnValue(recs); + + // 1. Load and expand all groups. + await loadRecommendations(); + const expandAllBtn = document.querySelector('.expand-all-toggle'); + expect(expandAllBtn).not.toBeNull(); + expandAllBtn!.click(); + + // Confirm we are now in "Collapse all" state. + expect(expandAllBtn!.textContent).toMatch(/Collapse all/); + + // 2. Simulate a provider filter change by calling loadRecommendations() + // again (this is exactly what the subscribeProvider callback invokes). + await loadRecommendations(); + + // 3. Expand state must have been cleared: button label reverts to + // "Expand all" and no variant rows are visible (groups are collapsed). + const expandAllBtn2 = document.querySelector('.expand-all-toggle'); + expect(expandAllBtn2).not.toBeNull(); + expect(expandAllBtn2!.textContent).toMatch(/Expand all/); + + const variantRows = document.querySelectorAll('.rec-variant-row'); + expect(variantRows.length).toBe(0); + }); }); }); @@ -6478,6 +6510,70 @@ describe('Issue #494: deterministic group sort on multi-variant cells', () => { expect(first.length).toBe(3); } }); + + // ------------------------------------------------------------------------- + // QA 4.13 - Monthly Cost: zero-cost (all-upfront) variants must not prevent + // sort-direction toggle from reordering rows. + // + // Before the fix, cells with at least one all-upfront variant (monthly_cost=0) + // all scored 0 via Math.min(...finite), tying every such cell so the direction + // multiplier had nothing to act on and subsequent sort clicks were no-ops. + // The fix uses the minimum NON-ZERO recurring cost, falling back to 0 only + // when all finite values are 0 (pure all-upfront cell). + // ------------------------------------------------------------------------- + test('Monthly Cost: mixed cell (all-upfront + no-upfront) sorts by non-zero recurring cost, not 0', async () => { + // cellLow: all-upfront (monthly=0) + no-upfront (monthly=20) -> score = 20 + const cellLow = multiVariantCell({ + resourceType: 'low-mixed', payment1y: 'all-upfront', payment3y: 'no-upfront', + upfront1y: 500, upfront3y: 0, monthly1y: 0, monthly3y: 20, + }); + // cellHigh: all-upfront (monthly=0) + no-upfront (monthly=80) -> score = 80 + const cellHigh = multiVariantCell({ + resourceType: 'high-mixed', payment1y: 'all-upfront', payment3y: 'no-upfront', + upfront1y: 500, upfront3y: 0, monthly1y: 0, monthly3y: 80, + }); + const recs = [...cellHigh, ...cellLow]; // wrong insertion order + + // Ascending: cellLow (score=20) must precede cellHigh (score=80). + setupTestFixture(recs, { column: 'monthly_cost', direction: 'asc' }); + await loadRecommendations(); + const ascOrder = renderedCellOrder(); + expect(indexOrFail(ascOrder, 'low-mixed')) + .toBeLessThan(indexOrFail(ascOrder, 'high-mixed')); + + // Descending: cellHigh (score=80) must now precede cellLow (score=20). + // This is the toggle that was broken before the fix -- both cells scored 0 + // so direction-flip was a no-op. + setupTestFixture(recs, { column: 'monthly_cost', direction: 'desc' }); + await loadRecommendations(); + const descOrder = renderedCellOrder(); + expect(indexOrFail(descOrder, 'high-mixed')) + .toBeLessThan(indexOrFail(descOrder, 'low-mixed')); + + // The two directions must produce OPPOSITE orderings (the toggle is live). + expect(ascOrder).not.toEqual(descOrder); + }); + + test('Monthly Cost: pure all-upfront cell scores 0 (not POSITIVE_INFINITY) and sorts before all-null cells', async () => { + // cellPureAllUpfront: both variants have monthly_cost=0 (all-upfront) -> score=0 + const cellPureAllUpfront = multiVariantCell({ + resourceType: 'pure-upfront', payment1y: 'all-upfront', payment3y: 'all-upfront', + upfront1y: 600, upfront3y: 1800, monthly1y: 0, monthly3y: 0, + }); + // cellAllNull: both variants have monthly_cost=null -> score=POSITIVE_INFINITY + const cellAllNull = multiVariantCell({ + resourceType: 'pure-null', payment1y: 'all-upfront', payment3y: 'all-upfront', + upfront1y: 1000, upfront3y: 3000, monthly1y: null, monthly3y: null, + }); + const recs = [...cellAllNull, ...cellPureAllUpfront]; + + // Ascending: pure-upfront (score=0) must come before all-null (score=+Inf). + setupTestFixture(recs, { column: 'monthly_cost', direction: 'asc' }); + await loadRecommendations(); + const order = renderedCellOrder(); + expect(indexOrFail(order, 'pure-upfront')) + .toBeLessThan(indexOrFail(order, 'pure-null')); + }); }); // Helper used by the issue-#479/#480/#481/#482/#483/#484 describes below to diff --git a/frontend/src/recommendations.ts b/frontend/src/recommendations.ts index e7c60611e..10c4fd749 100644 --- a/frontend/src/recommendations.ts +++ b/frontend/src/recommendations.ts @@ -62,7 +62,11 @@ export function getAccountName(accountId: string): string { // issues #225 + #226: expand/collapse state for cell grouping. // Contains the cellKey strings of cells the user has explicitly expanded. -// Cleared on page load / full refresh; survives per-column filter/sort re-renders. +// Cleared on every loadRecommendations() entry (page load, tab switch back +// to Opportunities, provider/account Global filter change, manual refresh, +// lookback change, stale auto-refresh — see resetExpandedCells() call at +// the top of loadRecommendations). Survives per-column filter/sort/period +// re-renders, which go through rerenderRecommendations() instead. const expandedCells = new Set(); // Last computed group keys for the visible filtered set — used by the // Expand-All button handler to populate expandedCells without re-computing. @@ -70,7 +74,8 @@ let lastVisibleGroupKeys: string[] = []; // issue #135: expand/collapse state for SP plan-type group rows. // Contains the spGroupKey strings the user has explicitly expanded. -// Cleared together with expandedCells on page load / full refresh. +// Cleared together with expandedCells on every loadRecommendations() entry +// (see resetExpandedCells); survives column-filter/sort/period re-renders. const expandedSpGroups = new Set(); // #272 (CR follow-up): cache of the most-recent API-derived summary so @@ -462,6 +467,13 @@ export async function loadRecommendations(): Promise { // the module default ("savings desc"). Idempotent on subsequent reloads. readSortFromUrl(); + // QA 4.13 (expand/collapse desync): clear expand state whenever the data + // source is reloaded due to a provider/account Global filter change. + // Column-filter and sort re-renders go through rerenderRecommendations() + // instead, so they intentionally do NOT reach this path -- preserving the + // "expand survives column-filter/sort" behavior documented near line 63. + resetExpandedCells(); + // Issue #344 T3: skeleton rows for the recommendations table so the // panel reads as "loading" instead of staying blank while the // (potentially multi-second) Promise.all resolves. 5 rows ≈ above- @@ -1187,14 +1199,25 @@ function cellScoreFor( } if (column === 'monthly_cost') { const period = state.getCostPeriod(); - const finite: number[] = []; + // #494 best-case framing: prefer the smallest NON-ZERO recurring cost so + // that all-upfront variants (monthly_cost=0, which is a real value not a + // null sentinel) do not tie every cell at 0 and make direction-toggling a + // no-op. Tiers: + // 1. any non-zero finite value exists -> Math.min of those + // 2. all finite values are 0 (pure all-upfront cell) -> 0 + // 3. no finite value at all (all null) -> POSITIVE_INFINITY (sink to bottom) + const nonZero: number[] = []; + const anyFinite: number[] = []; for (const v of variants) { - const scaled = scaleCost(v.monthly_cost, period); - if (scaled != null) finite.push(scaled); + const s = scaleCost(v.monthly_cost, period); + if (s != null) { + anyFinite.push(s); + if (s > 0) nonZero.push(s); + } } - // Best-case framing: lowest recurring cost wins. All-null cells sink to - // the bottom via the existing POSITIVE_INFINITY sentinel logic. - return finite.length === 0 ? Number.POSITIVE_INFINITY : Math.min(...finite); + if (nonZero.length) return Math.min(...nonZero); + if (anyFinite.length) return 0; + return Number.POSITIVE_INFINITY; } if (column === 'effective_savings_pct') { const finite: number[] = [];