diff --git a/frontend/src/__tests__/purchase-compatibility.test.ts b/frontend/src/__tests__/purchase-compatibility.test.ts index 15da73f8a..2f28a5a80 100644 --- a/frontend/src/__tests__/purchase-compatibility.test.ts +++ b/frontend/src/__tests__/purchase-compatibility.test.ts @@ -79,6 +79,13 @@ describe('isSavingsPlanService (issue #132)', () => { expect(isSavingsPlanService(SAVINGS_PLANS_BUCKET_KEY)).toBe(true); }); + // Issue #658: Azure SP client reports service = "savingsplans" (no hyphen), + // mirroring common.ServiceSavingsPlans. The Go IsSavingsPlan function + // handles this via an explicit equality check; we mirror it here. + test('matches the Azure SP umbrella slug "savingsplans" (issue #658)', () => { + expect(isSavingsPlanService('savingsplans')).toBe(true); + }); + test('rejects non-SP slugs', () => { expect(isSavingsPlanService('ec2')).toBe(false); expect(isSavingsPlanService('rds')).toBe(false); @@ -142,4 +149,18 @@ describe('savingsPlansBucketLabel (issue #132)', () => { savingsPlansBucketLabel([SAVINGS_PLANS_BUCKET_KEY, 'savings-plans-compute']), ).toBe('Savings Plans (Compute)'); }); + + // Issue #658: Azure SP rows use the umbrella slug "savingsplans" (no + // hyphen). It is treated as a family marker (like SAVINGS_PLANS_BUCKET_KEY), + // so it is excluded from the parenthetical plan-type list and the label + // gracefully falls back to "Savings Plans". + test('skips the Azure SP umbrella slug "savingsplans" and falls back (issue #658)', () => { + expect(savingsPlansBucketLabel(['savingsplans'])).toBe('Savings Plans'); + }); + + test('mixed Azure SP and named AWS SP slugs: umbrella excluded from label', () => { + expect( + savingsPlansBucketLabel(['savingsplans', 'savings-plans-compute']), + ).toBe('Savings Plans (Compute)'); + }); }); diff --git a/frontend/src/__tests__/recommendations.test.ts b/frontend/src/__tests__/recommendations.test.ts index e0ab8dbf8..455a8b359 100644 --- a/frontend/src/__tests__/recommendations.test.ts +++ b/frontend/src/__tests__/recommendations.test.ts @@ -2421,6 +2421,100 @@ describe('Bundle B: column header filter triggers', () => { expect((lastCall[1]?.values as string[]).sort()).toEqual(['savings-plans-compute', 'savings-plans-ec2instance']); }); }); + + // Issue #658: Azure SP rows use service = "savingsplans" (no hyphen). Verify + // they appear in the rendered table and are recognized by the SP group toggle. + describe('Issue #658: Azure Savings Plans row rendering and SP group toggle', () => { + const azureSpRecs = [ + { id: 'rec-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: 'rec-az-sp-1', provider: 'azure', cloud_account_id: 'sub1', service: 'savingsplans', resource_type: 'Compute', region: 'eastus', count: 2, term: 1, savings: 300, upfront_cost: 1200 }, + { id: 'rec-sp-c', provider: 'aws', cloud_account_id: 'a1', service: 'savings-plans-compute', resource_type: 'sp', region: 'us-east-1', count: 1, term: 1, savings: 200, upfront_cost: 800 }, + ]; + + beforeEach(() => { + (api.getRecommendations as jest.Mock).mockResolvedValue({ + summary: {}, + recommendations: azureSpRecs, + regions: [], + }); + (state.getRecommendations as jest.Mock).mockReturnValue(azureSpRecs); + (state.getVisibleRecommendations as jest.Mock).mockReturnValue(azureSpRecs); + (state.getRecommendationsColumnFilters as jest.Mock).mockReturnValue({}); + }); + + test('Azure SP rows (service="savingsplans") appear in the rendered table', async () => { + await loadRecommendations(); + // The table should contain a row with service badge "savingsplans". + const serviceBadges = Array.from( + document.querySelectorAll('td .service-badge'), + ).map((el) => el.textContent ?? ''); + expect(serviceBadges).toContain('savingsplans'); + }); + + test('service popover renders All Savings Plans toggle when Azure SP + AWS SP slugs are present', async () => { + // "savingsplans" + "savings-plans-compute" = 2 distinct SP slugs -> toggle should appear. + await loadRecommendations(); + const serviceBtn = document.querySelector( + 'th .column-filter-btn[data-column="service"]', + ); + serviceBtn?.click(); + const groupBox = document.querySelector( + '.column-filter-popover input[data-role="sp-group"]', + ); + expect(groupBox).not.toBeNull(); + const groupLabel = groupBox?.closest('label'); + expect(groupLabel?.textContent).toContain('All Savings Plans'); + }); + + test('clicking All Savings Plans toggle selects both Azure SP and AWS SP slugs', async () => { + // Use 4 distinct service values (ec2, rds, savingsplans, savings-plans-compute) + // so that selecting ec2+rds (2 of 4) gives an active filter, and then toggling + // the SP group on (adding savingsplans + savings-plans-compute) yields 4 of 4 = all, + // which we avoid by keeping ec2 only (1 of 4 + 2 SPs = 3 of 4, stays partial). + const widerRecs = [ + { id: 'rec-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: 'rec-rds', provider: 'aws', cloud_account_id: 'a1', service: 'rds', resource_type: 'db.t3', region: 'us-east-1', count: 1, term: 1, savings: 120, upfront_cost: 600 }, + { id: 'rec-az-sp', provider: 'azure', cloud_account_id: 'sub1', service: 'savingsplans', resource_type: 'Compute', region: 'eastus', count: 2, term: 1, savings: 300, upfront_cost: 1200 }, + { id: 'rec-aws-sp', provider: 'aws', cloud_account_id: 'a1', service: 'savings-plans-compute', resource_type: 'sp', region: 'us-east-1', count: 1, term: 1, savings: 200, upfront_cost: 800 }, + ]; + (api.getRecommendations as jest.Mock).mockResolvedValue({ summary: {}, recommendations: widerRecs, regions: [] }); + (state.getRecommendations as jest.Mock).mockReturnValue(widerRecs); + (state.getVisibleRecommendations as jest.Mock).mockReturnValue(widerRecs); + // Start with ec2 selected only; SP slugs are not in the active filter. + (state.getRecommendationsColumnFilters as jest.Mock).mockReturnValue({ + service: { kind: 'set', values: ['ec2'] }, + }); + await loadRecommendations(); + const serviceBtn = document.querySelector( + 'th .column-filter-btn[data-column="service"]', + ); + serviceBtn?.click(); + + // SP group toggle should be unchecked (no SP slugs are in the active filter). + const groupBox = document.querySelector( + '.column-filter-popover input[data-role="sp-group"]', + ); + expect(groupBox).not.toBeNull(); + expect(groupBox?.checked).toBe(false); + + // Tick the SP group toggle (browser flips -> checked on click). + groupBox!.checked = true; + groupBox!.dispatchEvent(new Event('change')); + + // Commit includes ec2 (already checked) + savingsplans + savings-plans-compute + // = 3 of 4 distinct values -> persisted as a set (not collapsed to null). + const calls = (state.setRecommendationsColumnFilter as jest.Mock).mock.calls; + const lastCall = calls[calls.length - 1]; + expect(lastCall[0]).toBe('service'); + expect(lastCall[1]?.kind).toBe('set'); + const values = (lastCall[1]?.values as string[]).sort(); + // Both the Azure SP umbrella slug and the AWS SP plan-type slug should be selected. + expect(values).toContain('savingsplans'); + expect(values).toContain('savings-plans-compute'); + // ec2 remains in the selection. + expect(values).toContain('ec2'); + }); + }); }); describe('Bundle B: sticky bottom action box', () => { @@ -3117,6 +3211,84 @@ describe('Issue #132: bulk-buy collapses SP plan types into one bucket', () => { }); }); +// Issue #658: Azure SP rows (service="savingsplans") must collapse into +// the same bulk-buy bucket as other SP types (savings-plans-*), matching +// the Go IsSavingsPlan umbrella semantics. +describe('Issue #658: Azure SP bulk-buy bucketing', () => { + beforeEach(async () => { + document.body.replaceChildren(); + const recsTab = document.createElement('div'); + recsTab.id = 'opportunities-tab'; + recsTab.className = 'tab-content active'; + const summary = document.createElement('div'); + summary.id = 'recommendations-summary'; + const list = document.createElement('div'); + list.id = 'recommendations-list'; + recsTab.appendChild(summary); + recsTab.appendChild(list); + document.body.appendChild(recsTab); + const purchaseModal = document.createElement('div'); + purchaseModal.id = 'purchase-modal'; + purchaseModal.className = 'hidden'; + const purchaseDetails = document.createElement('div'); + purchaseDetails.id = 'purchase-details'; + purchaseModal.appendChild(purchaseDetails); + document.body.appendChild(purchaseModal); + jest.clearAllMocks(); + (api.listAccountServiceOverrides as jest.Mock).mockResolvedValue([]); + const { clearFanOutBuckets, clearPurchaseModalRecommendations } = await import('../recommendations'); + clearFanOutBuckets(); + clearPurchaseModalRecommendations(); + }); + + test('single Azure SP rec at term=1 lands in the single-bucket happy path', async () => { + const recs = [ + { id: 'az-sp-1', provider: 'azure', cloud_account_id: 'sub1', service: 'savingsplans', resource_type: 'Compute', region: 'eastus', count: 2, term: 1, savings: 300, upfront_cost: 1200, payment: 'monthly' }, + ]; + (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(recs.map((r) => r.id as string))); + + await loadRecommendations(); + (document.getElementById('bulk-purchase-btn') as HTMLButtonElement).click(); + + const { getFanOutBuckets, getPurchaseModalRecommendations } = await import('../recommendations'); + // Single bucket -> happy path (no fan-out modal). + expect(getFanOutBuckets()).toBeNull(); + const modalRecs = getPurchaseModalRecommendations(); + expect(modalRecs).toHaveLength(1); + expect(modalRecs[0]!.service).toBe('savingsplans'); + }); + + test('Azure SP + AWS SP-compute at same term collapse into one bucket', async () => { + const recs = [ + { id: 'az-sp-1', provider: 'azure', cloud_account_id: 'sub1', service: 'savingsplans', resource_type: 'Compute', region: 'eastus', count: 2, term: 1, savings: 300, upfront_cost: 1200, payment: 'monthly' }, + { id: 'aws-sp-1', provider: 'aws', cloud_account_id: 'a1', service: 'savings-plans-compute', resource_type: 'sp', region: 'us-east-1', count: 1, term: 1, savings: 100, upfront_cost: 500, payment: 'no-upfront' }, + ]; + (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({}); + // Both selected — but different providers mean different bucket keys + // (bucket key includes provider), so they land in separate buckets. + (state.getSelectedRecommendationIDs as jest.Mock).mockReturnValue(new Set(recs.map((r) => r.id as string))); + + await loadRecommendations(); + (document.getElementById('bulk-purchase-btn') as HTMLButtonElement).click(); + await Promise.resolve(); await Promise.resolve(); await Promise.resolve(); + + const { getFanOutBuckets } = await import('../recommendations'); + const buckets = getFanOutBuckets(); + expect(buckets).not.toBeNull(); + // Two separate providers -> two buckets even though both are SP service types. + expect(buckets!.length).toBe(2); + // Both buckets use the canonical SP bucket service key. + expect(buckets!.every((b) => b.service === 'savings-plans')).toBe(true); + }); +}); + // Issue #224: at most one (term, payment) variant per physical-resource // cell can be selected at any time. After PR #195's per-cell fan-out (2 // terms × 3 payments per cell), naive selection produces wrong purchase diff --git a/frontend/src/lib/purchase-compatibility.ts b/frontend/src/lib/purchase-compatibility.ts index 57245339d..005ea1b7b 100644 --- a/frontend/src/lib/purchase-compatibility.ts +++ b/frontend/src/lib/purchase-compatibility.ts @@ -74,8 +74,14 @@ export function paymentOptionsFor(provider: Provider, service: string, term: Ter // hardcoded set so a future plan type added on the backend // (`common.IsSavingsPlan` is also `HasPrefix`) is picked up without a // frontend edit. +// +// Issue #658: Azure Savings Plans use the umbrella slug "savingsplans" +// (no hyphen) — the same constant as common.ServiceSavingsPlans. The +// Go IsSavingsPlan function handles this via an explicit equality check +// (`string(s) == "savingsplans"`); we mirror that here so Azure SP rows +// are grouped correctly in the filter popover and bulk-buy bucketing. export function isSavingsPlanService(service: string): boolean { - return service.startsWith('savings-plans'); + return service.startsWith('savings-plans') || service === 'savingsplans'; } // SAVINGS_PLANS_BUCKET_KEY is the canonical service slug used in the @@ -94,11 +100,22 @@ const SP_SHORT_LABEL: Record = { 'savings-plans-database': 'Database', }; +// UMBRELLA_SLUGS are SP slugs that identify the family as a whole rather +// than a specific plan type; they are excluded from the parenthetical +// plan-type list in savingsPlansBucketLabel so a bucket of Azure SP recs +// renders as "Savings Plans" rather than the raw slug. +// +// Issue #658: "savingsplans" (no hyphen) is the Azure SP client's service +// slug and the legacy AWS SP umbrella (common.ServiceSavingsPlans). It is +// a family marker, not a plan-type label, so it is treated the same way +// as SAVINGS_PLANS_BUCKET_KEY. +const UMBRELLA_SLUGS = new Set([SAVINGS_PLANS_BUCKET_KEY, 'savingsplans']); + // savingsPlansBucketLabel formats the bulk-buy bucket title for one // or more SP plan types. Returns: -// - 'Savings Plans (Compute)' for a single plan type +// - 'Savings Plans (Compute)' for a single named plan type // - 'Savings Plans (Compute + SageMaker)' for a mixed bucket -// - 'Savings Plans' if no plan types resolve (defensive fallback) +// - 'Savings Plans' for umbrella/Azure SP slugs or when no types resolve // Plan-type order in the output follows insertion order of the input // slugs — caller controls the order. export function savingsPlansBucketLabel(serviceSlugs: readonly string[]): string { @@ -106,10 +123,11 @@ export function savingsPlansBucketLabel(serviceSlugs: readonly string[]): string const parts: string[] = []; for (const slug of serviceSlugs) { if (!isSavingsPlanService(slug) || seen.has(slug)) continue; - // Skip the canonical bucket key itself if it shows up in the input - // — it's a marker, not a plan-type label, so rendering it as - // "savings-plans" inside the parentheses would be ugly. - if (slug === SAVINGS_PLANS_BUCKET_KEY) continue; + // Skip umbrella slugs (bucket-key marker and the "savingsplans" Azure + // SP / legacy AWS SP identifier) — they represent the family, not a + // specific plan type, so rendering them in the parentheses would be + // misleading. + if (UMBRELLA_SLUGS.has(slug)) continue; seen.add(slug); parts.push(SP_SHORT_LABEL[slug] ?? slug); }