diff --git a/frontend/src/__tests__/recommendations.test.ts b/frontend/src/__tests__/recommendations.test.ts index 1abe0edf..135f458b 100644 --- a/frontend/src/__tests__/recommendations.test.ts +++ b/frontend/src/__tests__/recommendations.test.ts @@ -238,6 +238,270 @@ describe('Recommendations Module', () => { (state.getRecommendationsColumnFilters as jest.Mock).mockReturnValue({}); }); + test('summary cards narrow to selection in real time (closes #279)', async () => { + // 4 recs / 2 cells. With no selection: cards reflect the full + // visible set (savings $300–$400, upfront $0–$3000). After ticking + // cell1's pricey variant, cards narrow to that single rec + // ($150 savings / $1000 upfront). Tick the second cell's cheap + // variant too: cards sum the two selected variants ($350 savings / + // $1000 upfront). Cards must update on every selection toggle — + // the selection-toggle handler triggers a list rerender, which + // calls renderRecommendationsSummary with the new selection. + const recs = [ + { id: 'cell1-cheap', provider: 'aws', cloud_account_id: 'a1', service: 'ec2', resource_type: 't3.medium', region: 'us-east-1', count: 1, term: 1, payment_option: 'no-upfront', savings: 100, upfront_cost: 0 }, + { id: 'cell1-pricey', provider: 'aws', cloud_account_id: 'a1', service: 'ec2', resource_type: 't3.medium', region: 'us-east-1', count: 1, term: 1, payment_option: 'all-upfront', savings: 150, upfront_cost: 1000 }, + { id: 'cell2-cheap', provider: 'aws', cloud_account_id: 'a1', service: 'rds', resource_type: 'db.t3', region: 'us-east-1', count: 1, term: 1, payment_option: 'no-upfront', savings: 200, upfront_cost: 0 }, + { id: 'cell2-pricey', provider: 'aws', cloud_account_id: 'a1', service: 'rds', resource_type: 'db.t3', region: 'us-east-1', count: 1, term: 1, payment_option: 'all-upfront', savings: 250, upfront_cost: 2000 }, + ]; + (api.getRecommendations as jest.Mock).mockResolvedValue({ + summary: { total_count: 4, total_monthly_savings: 700, total_upfront_cost: 3000, avg_payback_months: 2 }, + recommendations: recs, + regions: [], + }); + (state.getRecommendations as jest.Mock).mockReturnValue(recs); + (state.getVisibleRecommendations as jest.Mock).mockReturnValue(recs); + (state.getRecommendationsColumnFilters as jest.Mock).mockReturnValue({}); + + const cardValue = (titlePattern: RegExp): string => { + const summary = document.getElementById('recommendations-summary'); + const card = Array.from(summary?.querySelectorAll('.card') ?? []) + .find((c) => titlePattern.test(c.querySelector('h3')?.textContent ?? '')); + return card?.querySelector('.value')?.textContent ?? ''; + }; + const cardTitle = (valuePattern: RegExp): string => { + const summary = document.getElementById('recommendations-summary'); + const card = Array.from(summary?.querySelectorAll('.card') ?? []) + .find((c) => valuePattern.test(c.querySelector('.value')?.textContent ?? '')); + return card?.querySelector('h3')?.textContent ?? ''; + }; + + // No selection: cards reflect the full visible set. + (state.getSelectedRecommendationIDs as jest.Mock).mockReturnValue(new Set()); + await loadRecommendations(); + expect(cardValue(/Recommendations/)).toBe('2'); // 2 cells, not 4 variants + expect(cardValue(/Monthly Savings/)).toMatch(/\$300\b/); + expect(cardValue(/Monthly Savings/)).toMatch(/\$400\b/); + expect(cardValue(/Upfront/)).toMatch(/\$0\b/); + expect(cardValue(/Upfront/)).toMatch(/\$3,?000\b/); + // Card titles reflect the "all visible" mode. + expect(cardTitle(/^2$/)).toBe('Total Recommendations'); + + // Tick one cell's pricey variant: cards narrow to that one rec. + (state.getSelectedRecommendationIDs as jest.Mock).mockReturnValue(new Set(['cell1-pricey'])); + await loadRecommendations(); + expect(cardValue(/Recommendations/)).toBe('1'); + // Single rec → range collapses to a single value. + expect(cardValue(/Monthly Savings/)).toMatch(/^\$150$/); + expect(cardValue(/Upfront/)).toMatch(/^\$1,?000$/); + // Title switches to "Selected ..." form. + expect(cardTitle(/^1$/)).toBe('Selected Recommendations'); + + // Tick a second cell: cards sum the two selected variants. + (state.getSelectedRecommendationIDs as jest.Mock).mockReturnValue(new Set(['cell1-pricey', 'cell2-cheap'])); + await loadRecommendations(); + expect(cardValue(/Recommendations/)).toBe('2'); + expect(cardValue(/Monthly Savings/)).toMatch(/^\$350$/); // 150 + 200 + expect(cardValue(/Upfront/)).toMatch(/^\$1,?000$/); // 1000 + 0 + }); + + test('row checkbox change event updates summary cards in place (PR #283 CR pass-2)', async () => { + // Regression guard for the real DOM event path: verifies that dispatching + // a `change` event on a row checkbox updates summary cards WITHOUT a + // second loadRecommendations() call. The earlier mock-and-reload test + // above confirms the rendering math; this test confirms the event + // handler itself triggers renderRecommendationsList. + // + // Each rec is a DISTINCT cell (different resource_type) so they render as + // flat rows — not grouped summary rows — making their checkboxes directly + // queryable from the DOM. Multi-variant cells collapse into a summary row + // with a chevron button; variant-level checkboxes are only in the DOM when + // the cell is expanded. + // + // Mock coordination: addSelectedRecommendation / removeSelectedRecommendation + // mutate a shared Set so getSelectedRecommendationIDs reflects the toggle + // immediately — the same pattern used by checkboxes-evict-siblings tests. + const recs = [ + { id: 'rec-a', provider: 'aws', cloud_account_id: 'a1', service: 'ec2', resource_type: 't3.medium', region: 'us-east-1', count: 1, term: 1, savings: 150, upfront_cost: 1000 }, + { id: 'rec-b', provider: 'aws', cloud_account_id: 'a1', service: 'rds', resource_type: 'db.t3', region: 'us-east-1', count: 1, term: 1, savings: 200, upfront_cost: 0 }, + ]; + (api.getRecommendations as jest.Mock).mockResolvedValue({ + summary: { total_count: 2, total_monthly_savings: 350, total_upfront_cost: 1000, avg_payback_months: 1 }, + recommendations: recs, + regions: [], + }); + (state.getRecommendations as jest.Mock).mockReturnValue(recs); + (state.getVisibleRecommendations as jest.Mock).mockReturnValue(recs); + (state.getRecommendationsColumnFilters as jest.Mock).mockReturnValue({}); + + // Wire add/remove to a shared Set so getSelectedRecommendationIDs + // reflects in-flight selection state after every checkbox event. + const selectedIds = new Set(); + (state.getSelectedRecommendationIDs as jest.Mock).mockImplementation(() => new Set(selectedIds)); + (state.addSelectedRecommendation as jest.Mock).mockImplementation((id: string) => { selectedIds.add(id); }); + (state.removeSelectedRecommendation as jest.Mock).mockImplementation((id: string) => { selectedIds.delete(id); }); + (state.clearSelectedRecommendations as jest.Mock).mockImplementation(() => { selectedIds.clear(); }); + + // Initial render: no selection, cards show full-set values. + await loadRecommendations(); + + const cardValue = (titlePattern: RegExp): string => { + const summary = document.getElementById('recommendations-summary'); + const card = Array.from(summary?.querySelectorAll('.card') ?? []) + .find((c) => titlePattern.test(c.querySelector('h3')?.textContent ?? '')); + return card?.querySelector('.value')?.textContent ?? ''; + }; + + // Each rec is its own cell → 2 cells, each with 1 variant → range = single value. + expect(cardValue(/Recommendations/)).toBe('2'); + + // Fire the real DOM change event on rec-a's checkbox — do NOT + // call loadRecommendations() again. The handler inside recommendations.ts + // calls renderRecommendationsList() which updates the cards in place. + const checkbox = document.querySelector('input[data-rec-id="rec-a"]'); + expect(checkbox).not.toBeNull(); + checkbox!.checked = true; + checkbox!.dispatchEvent(new Event('change')); + + // Cards must now reflect only rec-a ($150 savings / $1000 upfront). + expect(cardValue(/Monthly Savings/)).toMatch(/^\$150$/); + expect(cardValue(/Upfront/)).toMatch(/^\$1,?000$/); + // Recommendations card must show "1 selected". + expect(cardValue(/Recommendations/)).toBe('1'); + }); + + test('select-all change event updates summary cards in place (PR #283 CR pass-2)', async () => { + // Verifies the select-all checkbox event path: after checking select-all, + // summary cards reflect the picked-best-per-cell selection WITHOUT a + // second loadRecommendations() call. + // + // pickBestVariantPerCell picks one variant per (provider, account, service, + // region, resource_type, engine) group — for these 2 cells it picks one + // from cell1 and one from cell2, so both cards must show 2 rows selected. + const recs = [ + { id: 'cell1-cheap', provider: 'aws', cloud_account_id: 'a1', service: 'ec2', resource_type: 't3.medium', region: 'us-east-1', count: 1, term: 1, payment_option: 'no-upfront', savings: 100, upfront_cost: 0 }, + { id: 'cell1-pricey', provider: 'aws', cloud_account_id: 'a1', service: 'ec2', resource_type: 't3.medium', region: 'us-east-1', count: 1, term: 1, payment_option: 'all-upfront', savings: 150, upfront_cost: 1000 }, + { id: 'cell2-cheap', provider: 'aws', cloud_account_id: 'a1', service: 'rds', resource_type: 'db.t3', region: 'us-east-1', count: 1, term: 1, payment_option: 'no-upfront', savings: 200, upfront_cost: 0 }, + { id: 'cell2-pricey', provider: 'aws', cloud_account_id: 'a1', service: 'rds', resource_type: 'db.t3', region: 'us-east-1', count: 1, term: 1, payment_option: 'all-upfront', savings: 250, upfront_cost: 2000 }, + ]; + (api.getRecommendations as jest.Mock).mockResolvedValue({ + summary: { total_count: 4, total_monthly_savings: 700, total_upfront_cost: 3000, avg_payback_months: 2 }, + recommendations: recs, + regions: [], + }); + (state.getRecommendations as jest.Mock).mockReturnValue(recs); + (state.getVisibleRecommendations as jest.Mock).mockReturnValue(recs); + (state.getRecommendationsColumnFilters as jest.Mock).mockReturnValue({}); + + // Wire add/remove/clear to a shared Set (same coordination pattern as the + // row-checkbox test above and the evict-siblings test). + const selectedIds = new Set(); + (state.getSelectedRecommendationIDs as jest.Mock).mockImplementation(() => new Set(selectedIds)); + (state.addSelectedRecommendation as jest.Mock).mockImplementation((id: string) => { selectedIds.add(id); }); + (state.removeSelectedRecommendation as jest.Mock).mockImplementation((id: string) => { selectedIds.delete(id); }); + (state.clearSelectedRecommendations as jest.Mock).mockImplementation(() => { selectedIds.clear(); }); + + await loadRecommendations(); + + const cardValue = (titlePattern: RegExp): string => { + const summary = document.getElementById('recommendations-summary'); + const card = Array.from(summary?.querySelectorAll('.card') ?? []) + .find((c) => titlePattern.test(c.querySelector('h3')?.textContent ?? '')); + return card?.querySelector('.value')?.textContent ?? ''; + }; + + // Fire the real select-all change event — do NOT call loadRecommendations(). + const selectAll = document.getElementById('select-all-recs') as HTMLInputElement; + expect(selectAll).not.toBeNull(); + selectAll.checked = true; + selectAll.dispatchEvent(new Event('change')); + + // addSelectedRecommendation was called once per cell (pickBestVariantPerCell + // picks one per cell → 2 cells → 2 adds). + expect((state.addSelectedRecommendation as jest.Mock).mock.calls.length).toBe(2); + + // Recommendations card shows "2" (2 selected cells, not 4 raw variants). + expect(cardValue(/Recommendations/)).toBe('2'); + }); + + test('"Showing X of Y" surfaces selection count (closes #279)', async () => { + const recs = [ + { id: 'r1', provider: 'aws', service: 'ec2', resource_type: 't3.medium', region: 'us-east-1', count: 1, term: 1, savings: 100, upfront_cost: 0 }, + { id: 'r2', provider: 'aws', service: 'rds', resource_type: 'db.t3', region: 'us-east-1', count: 1, term: 1, savings: 200, upfront_cost: 0 }, + ]; + (api.getRecommendations as jest.Mock).mockResolvedValue({ summary: {}, recommendations: recs, regions: [] }); + (state.getRecommendations as jest.Mock).mockReturnValue(recs); + (state.getVisibleRecommendations as jest.Mock).mockReturnValue(recs); + + // No selection: line just shows "Showing X of Y". + (state.getSelectedRecommendationIDs as jest.Mock).mockReturnValue(new Set()); + await loadRecommendations(); + const liveText = (): string => + document.querySelector('.recommendations-filter-live')?.textContent ?? ''; + expect(liveText()).toMatch(/^Showing 2 of 2 recommendations$/); + + // ≥1 selected: line prepends the selection count so the user has + // visible feedback that their selection is influencing the cards. + (state.getSelectedRecommendationIDs as jest.Mock).mockReturnValue(new Set(['r1'])); + await loadRecommendations(); + expect(liveText()).toMatch(/^1 selected · Showing 2 of 2 recommendations$/); + }); + + test('selection filtered out of view: cards and live line treat it as no selection (PR #283 CR)', async () => { + // 4 recs / 2 cells. The user has selected cell2's variant ('r3'), but + // then applies a column filter that hides cell2. The visible set is + // cell1 only (r1, r2). The selected row (r3) is NOT in the visible set. + // + // Expected behaviour: + // - Summary cards reflect the visible (unselected-from-visible) set: + // cell1 range $100–$150, title "Total Recommendations" not "Selected". + // - Live status line reads "Showing 2 of 4 recommendations" — no + // "1 selected" prefix, because r3 is hidden. + const allRecs = [ + { id: 'r1', provider: 'aws', cloud_account_id: 'a1', service: 'ec2', resource_type: 't3.medium', region: 'us-east-1', count: 1, term: 1, payment_option: 'no-upfront', savings: 100, upfront_cost: 0 }, + { id: 'r2', provider: 'aws', cloud_account_id: 'a1', service: 'ec2', resource_type: 't3.medium', region: 'us-east-1', count: 1, term: 1, payment_option: 'all-upfront', savings: 150, upfront_cost: 1000 }, + { id: 'r3', provider: 'aws', cloud_account_id: 'a1', service: 'rds', resource_type: 'db.t3', region: 'us-east-1', count: 1, term: 1, payment_option: 'no-upfront', savings: 200, upfront_cost: 0 }, + { id: 'r4', provider: 'aws', cloud_account_id: 'a1', service: 'rds', resource_type: 'db.t3', region: 'us-east-1', count: 1, term: 1, payment_option: 'all-upfront', savings: 250, upfront_cost: 2000 }, + ]; + const visibleRecs = allRecs.filter((r) => r.service === 'ec2'); // cell1 only + + (api.getRecommendations as jest.Mock).mockResolvedValue({ + summary: {}, recommendations: allRecs, regions: [], + }); + (state.getRecommendations as jest.Mock).mockReturnValue(allRecs); + // Visible set is cell1 only — column filter hides rds rows. + (state.getVisibleRecommendations as jest.Mock).mockReturnValue(visibleRecs); + (state.getRecommendationsColumnFilters as jest.Mock).mockReturnValue({ + service: { kind: 'set', values: ['ec2'] }, + }); + // r3 is selected, but it is NOT in the visible set. + (state.getSelectedRecommendationIDs as jest.Mock).mockReturnValue(new Set(['r3'])); + + await loadRecommendations(); + + const summary = document.getElementById('recommendations-summary'); + const savingsCard = Array.from(summary?.querySelectorAll('.card') ?? []) + .find((c) => /Monthly Savings/.test(c.querySelector('h3')?.textContent ?? '')); + const savingsValue = savingsCard?.querySelector('.value')?.textContent ?? ''; + const savingsTitle = savingsCard?.querySelector('h3')?.textContent ?? ''; + + // Cards reflect the visible set (cell1: $100–$150), not the hidden selection. + expect(savingsValue).toContain('$100'); + expect(savingsValue).toContain('$150'); + expect(savingsValue).not.toContain('$200'); + // Title must not flip to "Selected …" since no visible row is selected. + expect(savingsTitle).toBe('Potential Monthly Savings'); + + // Live line must not prefix "1 selected" because r3 is hidden. + const liveText = document.querySelector('.recommendations-filter-live')?.textContent ?? ''; + expect(liveText).not.toMatch(/selected/); + expect(liveText).toMatch(/Showing/); + + // Reset mocks so this filter doesn't leak into subsequent tests. + (state.getRecommendationsColumnFilters as jest.Mock).mockReturnValue({}); + (state.getVisibleRecommendations as jest.Mock).mockReturnValue([]); + (state.getSelectedRecommendationIDs as jest.Mock).mockReturnValue(new Set()); + }); + test('renders recommendations list', async () => { (api.getRecommendations as jest.Mock).mockResolvedValue({ summary: {}, @@ -601,15 +865,15 @@ describe('Recommendations Module', () => { }); }); - test('renders sortable column headers with indicators (Bundle B: 11 columns)', async () => { + test('renders sortable column headers with indicators (Bundle B + #282: 12 columns)', async () => { await loadRecommendations(); const list = document.getElementById('recommendations-list'); - // Bundle B: every data column is sortable. 11 sortable data columns: - // provider, account, service, resource_type, region, count, term, + // Bundle B + issue #282: every data column is sortable. 12 sortable data columns: + // provider, account, service, resource_type, region, count, term, payment, // savings, upfront_cost, monthly_cost, effective_savings_pct. // The leading checkbox column is not sortable. const sortables = list?.querySelectorAll('th.sortable'); - expect(sortables?.length).toBe(11); + expect(sortables?.length).toBe(12); // The default sort is savings desc → that header shows an active ▼. const savingsHeader = list?.querySelector('th[data-sort="savings"]'); expect(savingsHeader?.innerHTML).toContain('active'); @@ -622,6 +886,73 @@ describe('Recommendations Module', () => { expect(state.setRecommendationsSort).toHaveBeenCalledWith({ column: 'upfront_cost', direction: 'desc' }); }); + test('Payment column header renders and is sortable (#282)', async () => { + await loadRecommendations(); + const list = document.getElementById('recommendations-list'); + const paymentTh = list?.querySelector('th[data-sort="payment"]'); + expect(paymentTh).not.toBeNull(); + expect(paymentTh?.textContent).toContain('Payment'); + // Clicking the Payment header should call setRecommendationsSort with 'payment'. + (paymentTh as HTMLElement)?.click(); + expect(state.setRecommendationsSort).toHaveBeenCalledWith({ column: 'payment', direction: 'desc' }); + }); + + test('Payment column cell renders human-readable labels (#282)', async () => { + const recs = [ + { id: 'p1', provider: 'aws', cloud_account_id: 'a1', service: 'ec2', resource_type: 't3.small', region: 'us-east-1', count: 1, term: 1, payment: 'all-upfront', savings: 100, upfront_cost: 1000 }, + { id: 'p2', provider: 'aws', cloud_account_id: 'a1', service: 'rds', resource_type: 'db.t3', region: 'us-east-1', count: 1, term: 1, payment: 'partial-upfront', savings: 200, upfront_cost: 500 }, + { id: 'p3', provider: 'aws', cloud_account_id: 'a1', service: 'ec2', resource_type: 't3.medium', region: 'us-west-2', count: 1, term: 1, payment: 'no-upfront', savings: 300, upfront_cost: 0 }, + { id: 'p4', provider: 'aws', cloud_account_id: 'a1', service: 'rds', resource_type: 'db.m5', region: 'us-west-2', count: 1, term: 3, payment: 'monthly', savings: 400, upfront_cost: 0 }, + ]; + (api.getRecommendations as jest.Mock).mockResolvedValue({ summary: {}, recommendations: recs, regions: [] }); + (state.getRecommendations as jest.Mock).mockReturnValue(recs); + (state.getVisibleRecommendations as jest.Mock).mockReturnValue(recs); + (state.getRecommendationsColumnFilters as jest.Mock).mockReturnValue({}); + (state.getSelectedRecommendationIDs as jest.Mock).mockReturnValue(new Set()); + await loadRecommendations(); + + const rows = document.querySelectorAll('tr.recommendation-row'); + const paymentCells = Array.from(rows).map((row) => { + // Payment column is the 9th (0-indexed: 0=checkbox, 1=provider, + // 2=account, 3=service, 4=resource_type, 5=region, 6=count, 7=term, 8=payment) + return row.querySelectorAll('td')[8]?.textContent?.trim() ?? ''; + }); + expect(paymentCells).toContain('All Upfront'); + expect(paymentCells).toContain('Partial Upfront'); + expect(paymentCells).toContain('No Upfront'); + expect(paymentCells).toContain('Monthly'); + // Em-dash rendered for missing payment (no dedicated test rec here, + // but the helper returns '—' for undefined — covered by formatPayment unit test). + }); + + test('Payment column filter narrows visible rows (#282)', async () => { + const recs = [ + { id: 'f1', provider: 'aws', cloud_account_id: 'a1', service: 'ec2', resource_type: 't3.small', region: 'us-east-1', count: 1, term: 1, payment: 'all-upfront', savings: 100, upfront_cost: 1000 }, + { id: 'f2', provider: 'aws', cloud_account_id: 'a1', service: 'rds', resource_type: 'db.t3', region: 'us-east-1', count: 1, term: 1, payment: 'no-upfront', savings: 200, upfront_cost: 0 }, + ]; + (api.getRecommendations as jest.Mock).mockResolvedValue({ summary: {}, recommendations: recs, regions: [] }); + (state.getRecommendations as jest.Mock).mockReturnValue(recs); + (state.getVisibleRecommendations as jest.Mock).mockReturnValue(recs); + (state.getSelectedRecommendationIDs as jest.Mock).mockReturnValue(new Set()); + + // Apply a payment filter so only the all-upfront rec is visible. + (state.getRecommendationsColumnFilters as jest.Mock).mockReturnValue({ + payment: { kind: 'set', values: ['all-upfront'] }, + }); + + await loadRecommendations(); + + const rows = document.querySelectorAll('tr.recommendation-row'); + // Only the all-upfront rec should be rendered. + expect(rows.length).toBe(1); + // Payment cell text should be "All Upfront". + const paymentCell = rows[0]?.querySelectorAll('td')[8]; + expect(paymentCell?.textContent?.trim()).toBe('All Upfront'); + + // Restore filter mock so it doesn't leak into subsequent tests. + (state.getRecommendationsColumnFilters as jest.Mock).mockReturnValue({}); + }); + test('bottom action box selection summary reflects current selection (Bundle B)', async () => { // Bundle B replaced the floating .recommendations-bulk-toolbar with // selection-summary text inside the sticky bottom action box. @@ -640,9 +971,14 @@ describe('Recommendations Module', () => { await loadRecommendations(); const summary = document.getElementById('recommendations-action-summary'); - // #273: summary text shows just the selection count (drops the - // "of N cells visible" suffix — the visible fallback is gone). - expect(summary?.textContent).toContain('1 selected'); + // #281: summary text now surfaces the financial impact of the + // current action target (savings/mo + upfront across N cells) + // instead of just selection counts. The user can already see + // selection state from row checkboxes; the action box is prime + // real estate for the dollar figures they're authorising. + expect(summary?.textContent).toMatch(/\$100\/mo/); + expect(summary?.textContent).toMatch(/\$500 upfront/); + expect(summary?.textContent).toMatch(/1 cell\b/); // Old bulk-toolbar surface is gone. expect(document.querySelector('.recommendations-bulk-toolbar')).toBeNull(); }); @@ -1248,7 +1584,7 @@ describe('Bundle B: column header filter triggers', () => { const buttons = document.querySelectorAll('th .column-filter-btn[data-column]'); const cols = Array.from(buttons).map((b) => b.dataset['column']); expect(cols.sort()).toEqual( - ['account', 'count', 'effective_savings_pct', 'monthly_cost', 'provider', 'region', 'resource_type', 'savings', 'service', 'term', 'upfront_cost'].sort(), + ['account', 'count', 'effective_savings_pct', 'monthly_cost', 'payment', 'provider', 'region', 'resource_type', 'savings', 'service', 'term', 'upfront_cost'].sort(), ); }); @@ -1573,9 +1909,12 @@ describe('Bundle B: sticky bottom action box', () => { (state.getSelectedRecommendationIDs as jest.Mock).mockReturnValue(new Set()); }); - test('bottom box exposes Payment / Capacity % / Purchase / Create Plan; no Term selector', async () => { + test('bottom box no longer exposes the bulk Payment dropdown (#282)', async () => { await loadRecommendations(); - expect(document.getElementById('bulk-purchase-payment')).not.toBeNull(); + // Issue #282: global Payment dropdown removed — each rec carries its own + // payment_option from the API fan-out; a global override was misleading. + expect(document.getElementById('bulk-purchase-payment')).toBeNull(); + // Other controls remain. expect(document.getElementById('bulk-purchase-capacity')).not.toBeNull(); expect(document.getElementById('bulk-purchase-btn')).not.toBeNull(); expect(document.getElementById('create-plan-btn')).not.toBeNull(); @@ -2908,6 +3247,51 @@ describe('Issues #225 + #226: cell grouping with savings range and collapse/expa expect(plr.savingsMin).toBe(0); expect(plr.savingsMax).toBe(0); }); + + test('paybackMonthsMin uses per-cell paired variants, not cross-extrema (PR #283 CR)', () => { + // Cell A has two variants: + // v1: savings=50, upfront=0 → ratio = 0 (best payback: 0 months) + // v2: savings=200, upfront=300 → ratio = 1.5 (1.5 months) + // + // Cell B has two variants: + // v3: savings=100, upfront=200 → ratio = 2 (2 months) + // v4: savings=150, upfront=600 → ratio = 4 (4 months) + // + // Attainable min-payback selection: cell A picks v1 (ratio 0), cell B + // picks v3 (ratio 2). Total upfront=0+200=200, total savings=50+100=150. + // paybackMonthsMin = 200/150 ≈ 1.333... + // + // Cross-extrema (old code) would compute: + // upfrontMin = 0 (from v1) + 200 (from v3) = 200 + // savingsMax = 200 (from v2) + 150 (from v4) = 350 + // paybackMonthsMin = 200/350 ≈ 0.571 + // That's not attainable because it mixes upfront from v1 with savings + // from v2 (different variants of cell A). + // + // The new per-cell paired logic must produce 200/150 ≈ 1.333, not 0.571. + const cellA = [ + mkRec({ id: 'a-v1', resource_type: 'm5.large', savings: 50, upfront_cost: 0 }), + mkRec({ id: 'a-v2', resource_type: 'm5.large', savings: 200, upfront_cost: 300 }), + ]; + const cellB = [ + mkRec({ id: 'b-v3', resource_type: 't3.medium', savings: 100, upfront_cost: 200 }), + mkRec({ id: 'b-v4', resource_type: 't3.medium', savings: 150, upfront_cost: 600 }), + ]; + const groups = groupRecsByCell([...cellA, ...cellB]); + const plr = pageLevelRange(groups); + + // Per-cell paired: best ratio for A is v1 (0/50=0), best for B is v3 (200/100=2). + // Combined: upfront=0+200=200, savings=50+100=150 → paybackMin=200/150≈1.333 + expect(plr.paybackMonthsMin).toBeCloseTo(200 / 150, 5); + + // Cross-extrema would give 200/350≈0.571 — must NOT be the result. + expect(plr.paybackMonthsMin).not.toBeCloseTo(200 / 350, 5); + + // paybackMonthsMax: worst payback per cell. A picks v2 (300/200=1.5), + // B picks v4 (600/150=4). Combined: upfront=300+600=900, savings=200+150=350. + // paybackMax = 900/350 ≈ 2.571 + expect(plr.paybackMonthsMax).toBeCloseTo(900 / 350, 5); + }); }); describe('DOM rendering (cell grouping integrated)', () => { @@ -2952,6 +3336,20 @@ describe('Issues #225 + #226: cell grouping with savings range and collapse/expa expect(variantRows.length).toBe(0); }); + test('chevron button lives inside td.checkbox-col of the summary row (closes #280)', async () => { + const recs = multiVariantRecs(); + (api.getRecommendations as jest.Mock).mockResolvedValue({ summary: {}, recommendations: recs }); + (state.getRecommendations as jest.Mock).mockReturnValue(recs); + await loadRecommendations(); + + const summaryRow = document.querySelector('.rec-cell-summary-row'); + expect(summaryRow).not.toBeNull(); + const checkboxCell = summaryRow!.querySelector('td.checkbox-col'); + expect(checkboxCell).not.toBeNull(); + const chevron = checkboxCell!.querySelector('.rec-cell-chevron'); + expect(chevron).not.toBeNull(); + }); + test('clicking chevron expands the cell and shows variant rows', async () => { const recs = multiVariantRecs(); (api.getRecommendations as jest.Mock).mockResolvedValue({ summary: {}, recommendations: recs }); @@ -2960,22 +3358,35 @@ describe('Issues #225 + #226: cell grouping with savings range and collapse/expa const chevron = document.querySelector('.rec-cell-chevron'); expect(chevron).not.toBeNull(); + // aria-expanded starts false (collapsed) + expect(chevron!.getAttribute('aria-expanded')).toBe('false'); chevron!.click(); - // After expand: variant rows should appear + // After expand: variant rows should appear and aria-expanded flips const variantRows = document.querySelectorAll('.rec-variant-row'); expect(variantRows.length).toBe(2); + const updatedChevron = document.querySelector('.rec-cell-chevron'); + expect(updatedChevron!.getAttribute('aria-expanded')).toBe('true'); }); - test('page-level range banner appears when multi-variant cells exist', async () => { + test('page-level range banner is removed (closes #278) — savings card is the canonical surface', async () => { + // The "Recommended range" banner under the table was redundant with + // the Potential Monthly Savings card after #272 / #279 brought the + // same range to the summary header. Closing #278 removed the + // banner; the card is the single source of truth. const recs = multiVariantRecs(); (api.getRecommendations as jest.Mock).mockResolvedValue({ summary: {}, recommendations: recs }); (state.getRecommendations as jest.Mock).mockReturnValue(recs); + (state.getVisibleRecommendations as jest.Mock).mockReturnValue(recs); await loadRecommendations(); - const banner = document.querySelector('.rec-range-banner'); - expect(banner).not.toBeNull(); - expect(banner!.textContent).toMatch(/Recommended range/); + // Banner gone. + expect(document.querySelector('.rec-range-banner')).toBeNull(); + // Card carries the range. + const summary = document.getElementById('recommendations-summary'); + const savingsCard = Array.from(summary?.querySelectorAll('.card') ?? []) + .find((c) => /Monthly Savings/.test(c.querySelector('h3')?.textContent ?? '')); + expect(savingsCard?.querySelector('.value.savings')?.textContent).toMatch(/\$\d/); }); test('single-variant cell renders a flat row, no summary row', async () => { diff --git a/frontend/src/recommendations.ts b/frontend/src/recommendations.ts index 46669137..60eaa774 100644 --- a/frontend/src/recommendations.ts +++ b/frontend/src/recommendations.ts @@ -131,15 +131,9 @@ export async function loadRecommendations(): Promise { if (g.default_payment && (validPayments as string[]).includes(g.default_payment)) { cachedGlobalDefaultPayment = g.default_payment as CompatPayment; } - // Seed the bulk-toolbar default with the resolved value so that - // loadBulkPurchaseState()'s first-visit fallback uses it. - // BulkPurchasePayment is a subset of CompatPayment ('upfront' is - // Azure-only and not a valid bulk-toolbar value); skip the seed - // if the global default is outside that subset. - const validBulkPayments: BulkPurchasePayment[] = ['all-upfront', 'partial-upfront', 'no-upfront', 'monthly']; - if ((validBulkPayments as string[]).includes(cachedGlobalDefaultPayment)) { - defaultBulkPurchaseState.payment = cachedGlobalDefaultPayment as BulkPurchasePayment; - } + // cachedGlobalDefaultPayment is now read directly by loadBulkPurchaseState() + // (issue #282 dropped the toolbar dropdown; no longer need to seed + // defaultBulkPurchaseState — the module-level cache is the source of truth). } accountNamesCache = new Map(accounts.map(a => [a.id, a.name])); state.setRecommendations((data.recommendations || []) as unknown as api.Recommendation[]); @@ -170,42 +164,74 @@ export async function loadRecommendations(): Promise { } function renderRecommendationsSummary( - summary: RecommendationsSummary, + _summary: RecommendationsSummary, recommendations: readonly LocalRecommendation[], ): void { const container = document.getElementById('recommendations-summary'); if (!container) return; - // Potential Monthly Savings is computed from the same source as the - // "Recommended range" banner under the table (closes #272). The previous - // implementation rendered `summary.total_monthly_savings` from the API — - // a flat sum across every (term, payment) variant of every cell — so a - // typical 12-cell page with 2 terms × 3 payments per cell showed up to - // ~6× the achievable savings (the user can only buy one variant per - // cell). pageLevelRange sums per-cell min/max and stays consistent with - // the banner. - const groups = groupRecsByCell(recommendations); + // All four summary cards (Total Recommendations, Potential Monthly + // Savings, Total Upfront Cost, Payback Period) recompute client-side + // from the same source on every list rerender (closes #279). The API- + // derived `summary` is no longer read — it summed every (term, payment) + // variant of every cell and overstated achievable totals by ~6× on a + // typical fan-out, the same bug class #272 closed for the savings card + // alone. Now extended to the full header. + // + // Selection narrowing: when the user has ticked ≥1 checkbox visible in + // the table, the cards narrow to `selected ∩ visible`. Cleared selection + // → cards snap back to the full visible set. The cards therefore reflect + // exactly what a Purchase / Plan click would commit to. + // + // The unused `_summary` arg is retained so the call sites in + // loadRecommendations / renderRecommendationsList don't have to change + // shape; it can be removed when the field is also dropped from + // RecommendationsSummary. + const selected = state.getSelectedRecommendationIDs(); + // Narrow selection to visible rows only — if the user has ticked rows that + // are currently hidden by a column filter, those rows must not drive the + // summary cards (the cards should reflect what the user can actually see + // and act on). An empty visible-intersection means no effective selection. + const selectedVisible = recommendations.filter((r) => selected.has(r.id)); + const target: readonly LocalRecommendation[] = selectedVisible.length > 0 + ? selectedVisible + : recommendations; + const groups = groupRecsByCell(target); const plr = pageLevelRange(groups); + const isSelectionView = selectedVisible.length > 0 && plr.cellCount > 0; + const savingsText = plr.cellCount > 0 && plr.savingsMax > 0 ? formatSavingsRange(plr.savingsMin, plr.savingsMax) : formatCurrency(0); + const upfrontText = plr.cellCount > 0 && plr.upfrontMax > 0 + ? formatSavingsRange(plr.upfrontMin, plr.upfrontMax) + : formatCurrency(0); + const paybackText = plr.cellCount > 0 && plr.paybackMonthsMax > 0 + ? formatPaybackRange(plr.paybackMonthsMin, plr.paybackMonthsMax) + : '0 months'; + // Total Recommendations counts CELLS not variants — each cell is the + // user's actual decision unit. + const countLabel = isSelectionView ? 'Selected Recommendations' : 'Total Recommendations'; + const savingsLabel = isSelectionView ? 'Selected Monthly Savings' : 'Potential Monthly Savings'; + const upfrontLabel = isSelectionView ? 'Selected Upfront Cost' : 'Total Upfront Cost'; + const paybackLabel = 'Payback Period'; container.innerHTML = `
-

Total Recommendations

-

${summary.total_count || 0}

+

${countLabel}

+

${plr.cellCount}

-

Potential Monthly Savings

+

${savingsLabel}

${savingsText}

-

Total Upfront Cost

-

${formatCurrency(summary.total_upfront_cost)}

+

${upfrontLabel}

+

${upfrontText}

-

Payback Period

-

${summary.avg_payback_months ? summary.avg_payback_months.toFixed(1) : 0} months

+

${paybackLabel}

+

${paybackText}

`; } @@ -237,6 +263,7 @@ const SORTABLE_STRING_COLUMNS: Record string service: (r) => r.service ?? '', resource_type: (r) => r.resource_type ?? '', region: (r) => r.region ?? '', + payment: (r) => r.payment ?? '', }; // cellKey identifies the physical-resource cell a rec belongs to. @@ -311,21 +338,105 @@ export function cellSummary(variants: readonly LocalRecommendation[]): CellRange return { savingsMin, savingsMax, upfrontMin, upfrontMax, termMin, termMax }; } +/** Page-level totals for the summary header (closes #279). All ranges are + * the cell-by-cell min/max sums — the user buys at most one variant per + * cell, so the achievable totals are bounded by `sum(cell.{min,max})` not + * by `sum(every variant)`. */ +export interface PageLevelRange { + savingsMin: number; + savingsMax: number; + upfrontMin: number; + upfrontMax: number; + /** payback months at the min-savings end of the range; clamped to 0 + * when both savings and upfront are 0 (nothing to pay back). */ + paybackMonthsMin: number; + /** payback months at the max-savings end. */ + paybackMonthsMax: number; + cellCount: number; +} + /** - * Compute the page-level savings range by summing per-cell min/max savings. - * overallMin = sum of savingsMin per cell - * overallMax = sum of savingsMax per cell + * Compute the page-level summary by summing per-cell min/max envelopes. + * Used by both the summary cards and the (now-removed) banner; lives at + * module scope so dashboard.ts can reuse it for the cross-page parity in + * #279. + * + * savingsMin = sum of cellSummary.savingsMin + * savingsMax = sum of cellSummary.savingsMax + * upfrontMin = sum of cellSummary.upfrontMin (matches the variant + * whose savingsMin was contributed) + * upfrontMax = sum of cellSummary.upfrontMax + * paybackMonthsMin = sum of per-cell best-payback variant upfronts / + * sum of their savings. Each cell independently + * picks the variant with the lowest upfront/savings + * ratio. Using independent cross-extrema + * (upfrontMin / savingsMax) is NOT attainable when a + * cell's lowest-upfront variant ≠ its highest-savings + * variant — it would imply buying one impossible + * combination. Full convolution (trying every + * cross-cell combination) is O(variants^cells) and + * impractical for pages with 30+ cells. Per-cell + * paired choice is O(cells × variants) and produces + * bounds that match what a min-payback / max-payback + * per-cell selection would actually commit to. + * paybackMonthsMax = same logic, each cell picks the worst-payback + * variant. + * * Exported for tests. */ -export function pageLevelRange(groups: Map): { savingsMin: number; savingsMax: number; cellCount: number } { +export function pageLevelRange(groups: Map): PageLevelRange { let savingsMin = 0; let savingsMax = 0; + let upfrontMin = 0; + let upfrontMax = 0; + // Payback accumulators: per-cell paired variant sums. + let paybackBestUpfront = 0; + let paybackBestSavings = 0; + let paybackWorstUpfront = 0; + let paybackWorstSavings = 0; for (const variants of groups.values()) { const s = cellSummary(variants); savingsMin += s.savingsMin; savingsMax += s.savingsMax; + upfrontMin += s.upfrontMin; + upfrontMax += s.upfrontMax; + + // For payback: pick the variant with the best (lowest) payback ratio for + // the min-end, and the worst (highest) for the max-end. Treat savings ≤ 0 + // as Infinity payback so zero-savings variants sort to the worst end. + let bestRatio = Infinity; + let bestUpfront = 0; + let bestSavings = 0; + let worstRatio = -Infinity; + let worstUpfront = 0; + let worstSavings = 0; + for (const v of variants) { + const ratio = v.savings > 0 ? v.upfront_cost / v.savings : Infinity; + if (ratio < bestRatio) { + bestRatio = ratio; + bestUpfront = v.upfront_cost; + bestSavings = v.savings; + } + if (ratio > worstRatio) { + worstRatio = ratio; + worstUpfront = v.upfront_cost; + worstSavings = v.savings; + } + } + paybackBestUpfront += bestUpfront; + paybackBestSavings += bestSavings; + paybackWorstUpfront += worstUpfront; + paybackWorstSavings += worstSavings; } - return { savingsMin, savingsMax, cellCount: groups.size }; + // Clamp to 0 when total savings is non-positive to avoid Infinity / NaN. + const paybackMonthsMin = paybackBestSavings > 0 ? paybackBestUpfront / paybackBestSavings : 0; + const paybackMonthsMax = paybackWorstSavings > 0 ? paybackWorstUpfront / paybackWorstSavings : 0; + return { + savingsMin, savingsMax, + upfrontMin, upfrontMax, + paybackMonthsMin, paybackMonthsMax, + cellCount: groups.size, + }; } // Fixed payment ordering for within-cell variant sort. @@ -393,6 +504,15 @@ function formatSavingsRange(min: number, max: number): string { return lo === hi ? lo : `${lo} – ${hi}`; } +/** Format a payback-months range, collapsing identical endpoints to a + * single value and rounding to 1 decimal. Used by the Payback Period + * summary card after #279 broadened it to a range. */ +function formatPaybackRange(min: number, max: number): string { + const lo = min.toFixed(1); + const hi = max.toFixed(1); + return lo === hi ? `${lo} months` : `${lo} – ${hi} months`; +} + /** Format a term range, collapsing "1yr – 1yr" to "1yr". */ function formatTermRange(min: number, max: number): string { const lo = formatTerm(min); @@ -527,6 +647,7 @@ const SORT_HEADER_LABELS: Record = { region: 'Region', count: 'Count', term: 'Term', + payment: 'Payment', savings: 'Monthly Savings', upfront_cost: 'Upfront Cost', monthly_cost: 'Monthly Cost', @@ -644,6 +765,7 @@ function categoricalCellValue(r: LocalRecommendation, col: state.Recommendations case 'resource_type': return r.resource_type ?? ''; case 'region': return r.region ?? ''; case 'term': return r.term == null ? '' : String(r.term); + case 'payment': return r.payment ?? ''; // Numeric columns shouldn't reach this branch; return empty for type-safety. case 'count': case 'savings': @@ -671,7 +793,8 @@ function numericCellValue(r: LocalRecommendation, col: state.RecommendationsColu case 'service': case 'resource_type': case 'region': - case 'term': return Number.NaN; + case 'term': + case 'payment': return Number.NaN; } } @@ -1215,8 +1338,17 @@ function renderFilterStatusBar(loadedCount: number, visibleCount: number): void bar.appendChild(live); } // Always update the live region (even when no filters active) so the - // spoken count reflects state after every render. - live.textContent = `Showing ${visibleCount} of ${loadedCount} recommendations`; + // spoken count reflects state after every render. #279: when the user + // has ticked ≥1 visible row, the line surfaces the selection count too — + // same narrowing source-of-truth as the summary cards. Selections that + // are filtered out of view are NOT counted here, matching the card + // narrowing behaviour (only selected ∩ visible drives the UI). + const selectedIDs = state.getSelectedRecommendationIDs(); + const visibleRecs = state.getVisibleRecommendations(); + const selectedVisibleCount = visibleRecs.filter((r) => selectedIDs.has(r.id)).length; + live.textContent = selectedVisibleCount > 0 + ? `${selectedVisibleCount} selected · Showing ${visibleCount} of ${loadedCount} recommendations` + : `Showing ${visibleCount} of ${loadedCount} recommendations`; const filters = state.getRecommendationsColumnFilters(); const activeCount = Object.keys(filters).length; @@ -1530,9 +1662,22 @@ function providerBadgeClass(provider: string): string { // neither sortable nor filterable). const FILTERABLE_COLUMNS: readonly state.RecommendationsColumnId[] = [ 'provider', 'account', 'service', 'resource_type', 'region', - 'count', 'term', 'savings', 'upfront_cost', 'monthly_cost', 'effective_savings_pct', + 'count', 'term', 'payment', 'savings', 'upfront_cost', 'monthly_cost', 'effective_savings_pct', ]; +// Map raw payment_option values to display labels for the Payment column. +const PAYMENT_DISPLAY_LABELS: Record = { + 'all-upfront': 'All Upfront', + 'partial-upfront': 'Partial Upfront', + 'no-upfront': 'No Upfront', + 'monthly': 'Monthly', +}; + +function formatPayment(payment: string | undefined): string { + if (!payment) return '\u2014'; + return PAYMENT_DISPLAY_LABELS[payment] ?? payment; +} + // Helper: render one variant row (a single LocalRecommendation) with optional // indentation styling for multi-variant cells. function buildVariantRowMarkup(rec: LocalRecommendation, selectedRecs: ReadonlySet, isNested: boolean): string { @@ -1557,6 +1702,7 @@ function buildVariantRowMarkup(rec: LocalRecommendation, selectedRecs: ReadonlyS ${escapeHtml(rec.region)} ${rec.count} ${formatTerm(rec.term)} + ${formatPayment(rec.payment)} ${formatCurrency(rec.savings)} ${formatCurrency(rec.upfront_cost)} ${rec.monthly_cost != null ? formatCurrency(rec.monthly_cost) : '—'} @@ -1564,8 +1710,8 @@ function buildVariantRowMarkup(rec: LocalRecommendation, selectedRecs: ReadonlyS `; } -// The total column count for colspan on summary rows: checkbox col + 11 data cols = 12. -const TABLE_COL_COUNT = 12; +// The total column count for colspan on summary rows: checkbox col + 12 data cols = 13. +const TABLE_COL_COUNT = 13; function buildListMarkup(recommendations: LocalRecommendation[], selectedRecs: ReadonlySet): string { const sort = state.getRecommendationsSort(); @@ -1586,11 +1732,9 @@ function buildListMarkup(recommendations: LocalRecommendation[], selectedRecs: R // Only multi-variant cells count — single-variant cells render flat with no chevron. lastVisibleGroupKeys = sortedKeys.filter((k) => (groups.get(k)?.length ?? 0) > 1); - // Page-level range banner (summed across all visible cells). - const plr = pageLevelRange(groups); - const bannerHtml = plr.cellCount > 0 && plr.savingsMax > 0 - ? `
Recommended range: ${formatSavingsRange(plr.savingsMin, plr.savingsMax)}/mo across ${plr.cellCount} cell${plr.cellCount === 1 ? '' : 's'}
` - : ''; + // The "Recommended range" banner that used to live here was redundant + // with the Potential Monthly Savings card after #272 / #279 brought + // the same range to the summary header. Removed (closes #278). // Build tbody rows: grouped for multi-variant cells, flat for single-variant. const rows: string[] = []; @@ -1629,14 +1773,15 @@ function buildListMarkup(recommendations: LocalRecommendation[], selectedRecs: R rows.push(` - + + + ${escapeHtml(providerDisplayName(rep.provider))} ${escapeHtml(accountName)} ${escapeHtml(rep.service)} - ${escapeHtml(rep.resource_type)}${rep.engine ? ` (${escapeHtml(rep.engine)})` : ''} — ${escapeHtml(rep.region)} — ${variants.length} variants ${savingsDisplay} · upfront: ${upfrontDisplay} · term: ${termDisplay} @@ -1651,7 +1796,6 @@ function buildListMarkup(recommendations: LocalRecommendation[], selectedRecs: R } return ` - ${bannerHtml} @@ -1703,9 +1847,13 @@ const BULK_PURCHASE_LS_KEY = 'cudly.recommendations.bulkPurchase.v1'; // BulkPurchaseToolbarState used to carry a `term` field that overrode each // row's recommended term at API-call time. Bundle B drops it: each rec is // purchased with its own per-row term (see term-aware bucketing in -// handleBulkPurchaseClick). loadBulkPurchaseState explicitly picks known -// fields so any legacy `term` from older localStorage values is silently -// ignored on read — no migration shim needed. +// handleBulkPurchaseClick). Issue #282 drops the global Payment dropdown +// from the toolbar: the `payment` field is kept internally (seeded from +// GlobalConfig or 'all-upfront') so the fan-out modal's override/fallback +// logic continues to work, but it is no longer exposed in the UI or +// persisted to localStorage. loadBulkPurchaseState explicitly picks known +// fields so any legacy `term` or `payment` from older localStorage values +// is silently ignored on read — no migration shim needed. type BulkPurchasePayment = 'all-upfront' | 'partial-upfront' | 'no-upfront' | 'monthly'; // Centralized bucket-level payment compatibility check. A bucket is @@ -1737,11 +1885,11 @@ function loadBulkPurchaseState(): BulkPurchaseToolbarState { if (!raw) return { ...defaultBulkPurchaseState }; const parsed = JSON.parse(raw) as Partial & { term?: unknown }; // Explicit field-pick rather than spread-and-omit — avoids leaking a - // legacy `term` value into the returned object even at runtime. + // legacy `term` or `payment` from older localStorage values at runtime. + // Payment is seeded from GlobalConfig only (issue #282 drops the toolbar + // dropdown; the field is internal-only, not persisted). return { - // issue #223: fall back to resolved GlobalConfig.DefaultPayment rather - // than the literal 'all-upfront' when localStorage has no stored value. - payment: (parsed.payment || cachedGlobalDefaultPayment) as BulkPurchasePayment, + payment: cachedGlobalDefaultPayment as BulkPurchasePayment, capacity: Math.max(1, Math.min(100, Number(parsed.capacity) || 100)), }; } catch { @@ -1751,7 +1899,9 @@ function loadBulkPurchaseState(): BulkPurchaseToolbarState { function saveBulkPurchaseState(s: BulkPurchaseToolbarState): void { try { - localStorage.setItem(BULK_PURCHASE_LS_KEY, JSON.stringify(s)); + // Only persist capacity — payment is dropped from the toolbar (issue #282) + // and is now session-only, seeded from GlobalConfig. + localStorage.setItem(BULK_PURCHASE_LS_KEY, JSON.stringify({ capacity: s.capacity })); } catch { // Private-browsing / quota-exceeded — non-fatal, just lose the // sticky choice. The bottom box still works in-session. @@ -1765,11 +1915,15 @@ function saveBulkPurchaseState(s: BulkPurchaseToolbarState): void { // leaving the input/select elements (and their in-progress values) alone. // // IDs preserved for backward compatibility: -// #bulk-purchase-payment (Payment dropdown) // #bulk-purchase-capacity (Capacity % input — read by app.ts:307) // #bulk-purchase-btn (Purchase one-off button) // #create-plan-btn (Create Purchase Plan button — relocated from // the old top filter bar) +// +// Issue #282: the bulk Payment dropdown (#bulk-purchase-payment) is removed. +// Each rec carries its own payment_option from the API fan-out; the per-cell +// radio enforcement caps purchase to one variant per cell. A global override +// was misleading and is redundant. function mountBottomActionBox(): HTMLElement | null { const recsTab = document.getElementById('recommendations-tab'); if (!recsTab) return null; @@ -1791,21 +1945,6 @@ function mountBottomActionBox(): HTMLElement | null { summary.className = 'recommendations-action-summary'; box.appendChild(summary); - // Payment dropdown — preserved ID - const paymentLabel = document.createElement('label'); - paymentLabel.textContent = 'Payment '; - const paymentSelect = document.createElement('select'); - paymentSelect.id = 'bulk-purchase-payment'; - [['all-upfront', 'All Upfront'], ['partial-upfront', 'Partial Upfront'], ['no-upfront', 'No Upfront'], ['monthly', 'Monthly']].forEach(([v, t]) => { - const opt = document.createElement('option'); - opt.value = v as string; - opt.textContent = t as string; - if (v === tbState.payment) opt.selected = true; - paymentSelect.appendChild(opt); - }); - paymentLabel.appendChild(paymentSelect); - box.appendChild(paymentLabel); - // Capacity % input — preserved ID (app.ts:307 reads this) const capacityLabel = document.createElement('label'); capacityLabel.textContent = 'Capacity % '; @@ -1854,11 +1993,10 @@ function mountBottomActionBox(): HTMLElement | null { const persist = (): void => { saveBulkPurchaseState({ - payment: paymentSelect.value as BulkPurchaseToolbarState['payment'], + payment: tbState.payment, capacity: Math.max(1, Math.min(100, parseInt(capacityInput.value, 10) || 100)), }); }; - paymentSelect.addEventListener('change', persist); capacityInput.addEventListener('change', persist); purchaseBtn.addEventListener('click', () => { @@ -1917,21 +2055,39 @@ function updateBottomActionBox(visibleCount: number, loadedCount: number): void 0, ); - // Action buttons require an explicit selection (closes #273): a misclick - // on "Purchase visible" / "Plan from visible" with no checkboxes ticked - // could trigger an irreversible bulk purchase across every visible row, - // and the visible set silently changes when the user clicks Refresh or - // adjusts filters. Only the "selected" form of the action buttons remains. + // Action-box summary line surfaces the *financial* impact of the + // current action target, not just selection counts (closes #281). The + // selected-vs-visible count is the least useful info at this point — + // the user can already see selection state from row checkboxes — and + // the action box is prime real estate for the dollar figures the user + // is about to authorise. Source-of-truth matches the summary cards + // above: selection ∩ visible if ≥1 selected, else the visible set. + const target: readonly LocalRecommendation[] = selectedVisibleCount > 0 + ? visible.filter((r) => selected.has(r.id)) + : visible; + const targetGroups = groupRecsByCell(target); + const targetRange = pageLevelRange(targetGroups); + const summary = document.getElementById('recommendations-action-summary'); if (summary) { if (loadedCount === 0) { summary.textContent = '(No recommendations loaded)'; } else if (visibleCount === 0) { summary.textContent = '(0 visible — adjust filters)'; - } else if (selectedVisibleCount > 0) { - summary.textContent = `(${selectedVisibleCount} selected)`; - } else { + } else if (selectedVisibleCount === 0) { summary.textContent = '(Select cells to act on)'; + } else if (targetRange.cellCount > 0) { + const savingsText = targetRange.savingsMax > 0 + ? formatSavingsRange(targetRange.savingsMin, targetRange.savingsMax) + : formatCurrency(0); + const upfrontText = targetRange.upfrontMax > 0 + ? formatSavingsRange(targetRange.upfrontMin, targetRange.upfrontMax) + : formatCurrency(0); + const cellWord = targetRange.cellCount === 1 ? 'cell' : 'cells'; + summary.textContent = `(${savingsText}/mo · ${upfrontText} upfront across ${targetRange.cellCount} ${cellWord})`; + } else { + // Shouldn't happen given the gating above, but defensive. + summary.textContent = `(${selectedVisibleCount} selected)`; } } diff --git a/frontend/src/state.ts b/frontend/src/state.ts index af8f3166..b79415e4 100644 --- a/frontend/src/state.ts +++ b/frontend/src/state.ts @@ -5,19 +5,23 @@ import type { AppState } from './types'; import type { Recommendation } from './api/types'; -export type RecommendationsSortColumn = 'savings' | 'upfront_cost' | 'count' | 'term' | 'payback' | 'monthly_cost' | 'effective_savings_pct'; -export interface RecommendationsSort { - column: RecommendationsSortColumn; - direction: 'asc' | 'desc'; -} - // Closed enumeration of column ids the per-column filters target. // Typo-safety: misspellings at call sites become compile errors. export type RecommendationsColumnId = | 'provider' | 'account' | 'service' | 'resource_type' | 'region' - | 'count' | 'term' | 'savings' | 'upfront_cost' + | 'count' | 'term' | 'payment' | 'savings' | 'upfront_cost' | 'monthly_cost' | 'effective_savings_pct'; +// Every visible column is sortable, so the sort column type is exactly the +// column id set. Aliasing here keeps the two in sync automatically — adding +// a future column to RecommendationsColumnId automatically makes it sortable +// without a separate edit to this type. +export type RecommendationsSortColumn = RecommendationsColumnId; +export interface RecommendationsSort { + column: RecommendationsSortColumn; + direction: 'asc' | 'desc'; +} + export type RecommendationsColumnFilter = | { kind: 'set'; values: string[] } // categorical — values always string-form | { kind: 'expr'; expr: string }; // numeric — parsed on apply diff --git a/frontend/src/styles/components.css b/frontend/src/styles/components.css index 5fff9fc4..b4775761 100644 --- a/frontend/src/styles/components.css +++ b/frontend/src/styles/components.css @@ -913,10 +913,15 @@ tr.rec-cell-summary-row:hover { background: transparent; border: none; cursor: pointer; - font-size: 0.8em; + font-size: 1.2em; color: #555; - padding: 0 0.25rem; + padding: 4px; + min-width: 24px; + min-height: 24px; line-height: 1; + display: inline-flex; + align-items: center; + justify-content: center; } .rec-cell-chevron:hover { color: #1a73e8;