From 13adeb79e4c97b0ad1bd6674093db5998d1c04c7 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 28 May 2026 00:34:11 +0200 Subject: [PATCH 1/2] fix(opportunities): disable 'Plan from X selected' on heterogeneous selection (closes #769) Plans require a uniform provider/service/term/payment combination. Add isHomogeneousSelection() helper that checks all four axes; gate the "Plan from N selected" button on the result. The Purchase button is unaffected. The existing a11y hint span surfaces the explanation when the selection is heterogeneous. Eight new tests cover all four axes, the single-row pass-through, and the DOM-level button/hint state. --- .../src/__tests__/recommendations.test.ts | 93 ++++++++++++++++++- frontend/src/recommendations.ts | 64 +++++++++---- 2 files changed, 139 insertions(+), 18 deletions(-) diff --git a/frontend/src/__tests__/recommendations.test.ts b/frontend/src/__tests__/recommendations.test.ts index 2c521a04f..57c3429f4 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 } 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 } from '../recommendations'; import type { CostPeriod } from '../state'; // Mock the api module @@ -2715,6 +2715,25 @@ describe('Bundle B: sticky bottom action box', () => { expect(planBtn.disabled).toBe(true); }); + test('plan button disabled on heterogeneous selection; purchase button still enabled (#769)', async () => { + // The fixture recs differ in term (1 vs 3), so selecting both makes the + // selection heterogeneous for the plan button but not the purchase button. + (state.getSelectedRecommendationIDs as jest.Mock).mockReturnValue(new Set(['r1', 'r2'])); + await loadRecommendations(); + const purchaseBtn = document.getElementById('bulk-purchase-btn') as HTMLButtonElement; + const planBtn = document.getElementById('create-plan-btn') as HTMLButtonElement; + const hint = document.getElementById('recommendations-action-disabled-hint') as HTMLSpanElement; + // Purchase button is unaffected by homogeneity. + expect(purchaseBtn.disabled).toBe(false); + expect(purchaseBtn.textContent).toBe('Purchase 2 selected'); + // Plan button must be disabled. + expect(planBtn.disabled).toBe(true); + expect(planBtn.textContent).toBe('Plan from 2 selected'); + // Hint is visible with the heterogeneous explanation. + expect(hint.hidden).toBe(false); + expect(hint.textContent).toContain('Plans require one provider, service, term, and payment'); + }); + test('Capacity input value persists across re-render (mount-once-then-update)', async () => { await loadRecommendations(); const cap = document.getElementById('bulk-purchase-capacity') as HTMLInputElement; @@ -6402,3 +6421,75 @@ describe('Issue #484: numeric filter matches the displayed rounded value', () => }); }); }); + +// Helpers shared by the isHomogeneousSelection describe block below. +function makeRec(overrides: Partial<{ + id: string; + provider: string; + service: string; + term: number; + payment: string; +}>): { id: string; provider: string; service: string; resource_type: string; region: string; count: number; term: number; payment: string; savings: number; upfront_cost: number } { + return { + id: overrides.id ?? 'r1', + provider: overrides.provider ?? 'aws', + service: overrides.service ?? 'ec2', + resource_type: 't3.medium', + region: 'us-east-1', + count: 1, + term: overrides.term ?? 1, + payment: overrides.payment ?? 'all_upfront', + savings: 100, + upfront_cost: 500, + }; +} + +describe('isHomogeneousSelection (#769)', () => { + test('empty slice is homogeneous', () => { + expect(isHomogeneousSelection([])).toBe(true); + }); + + test('single-item slice is always homogeneous', () => { + expect(isHomogeneousSelection([makeRec({}) as never])).toBe(true); + }); + + test('two recs with identical provider/service/term/payment are homogeneous', () => { + const recs = [ + makeRec({ id: 'r1' }), + makeRec({ id: 'r2' }), + ]; + expect(isHomogeneousSelection(recs as never[])).toBe(true); + }); + + test('heterogeneous on provider axis', () => { + const recs = [ + makeRec({ id: 'r1', provider: 'aws' }), + makeRec({ id: 'r2', provider: 'azure' }), + ]; + expect(isHomogeneousSelection(recs as never[])).toBe(false); + }); + + test('heterogeneous on service axis', () => { + const recs = [ + makeRec({ id: 'r1', service: 'ec2' }), + makeRec({ id: 'r2', service: 'rds' }), + ]; + expect(isHomogeneousSelection(recs as never[])).toBe(false); + }); + + test('heterogeneous on term axis', () => { + const recs = [ + makeRec({ id: 'r1', term: 1 }), + makeRec({ id: 'r2', term: 3 }), + ]; + expect(isHomogeneousSelection(recs as never[])).toBe(false); + }); + + test('heterogeneous on payment axis', () => { + const recs = [ + makeRec({ id: 'r1', payment: 'all_upfront' }), + makeRec({ id: 'r2', payment: 'no_upfront' }), + ]; + expect(isHomogeneousSelection(recs as never[])).toBe(false); + }); +}); diff --git a/frontend/src/recommendations.ts b/frontend/src/recommendations.ts index f540fddb4..dac11dfd2 100644 --- a/frontend/src/recommendations.ts +++ b/frontend/src/recommendations.ts @@ -3034,6 +3034,23 @@ function resolvePurchaseTarget(): LocalRecommendation[] { return visible.filter((r) => selected.has(r.id)); } +// isHomogeneousSelection returns true iff every recommendation in the slice +// shares the same (provider, service, term, payment). A single-item slice +// always passes. An empty slice returns true (vacuously homogeneous). +// Plans require a homogeneous selection because the plan's scheduling +// parameters (provider, service, term, payment) must be unambiguous. +// Exported so unit tests can cover it directly without a full DOM setup. +export function isHomogeneousSelection(recs: readonly LocalRecommendation[]): boolean { + if (recs.length <= 1) return true; + // recs is non-empty here; the non-null assertion is safe. + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + const first = recs[0]!; + const { provider, service, term, payment } = first; + return recs.every( + (r) => r.provider === provider && r.service === service && r.term === term && r.payment === payment, + ); +} + // updateBottomActionBox refreshes labels and disabled state on every // renderRecommendationsList call without rebuilding the input/select DOM, // preserving any in-progress typing in the Capacity input. @@ -3095,21 +3112,11 @@ function updateBottomActionBox(visibleCount: number, loadedCount: number): void ? 'No rows visible — adjust filters' : 'Select at least one cell to enable'; - // a11y: the disabled-state explanation lives on a sibling hint span, - // not on the buttons' `title` attribute. Disabled