From 996fe3fb48a1c9249dfbc8a7c6d7b94e417341a3 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Wed, 4 Nov 2026 18:01:20 +0100 Subject: [PATCH 1/2] test(frontend): cover Savings Plans child rows and tiny payback bounds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - A jest case expands a Savings Plans group with a single-plan child and a multi-variant child. The single plan keeps its resource type, the child summary skips its blank resource type, and the child's variant rows leave the shared identity columns blank. Mutating any of those three call sites now fails a test. - The Payback Period range switches to "Up to X months" on the rounded lower bound, so a positive bound under 0.05 months no longer prints "0.0 – X months". Closes #1705 Co-Authored-By: Claude Opus 5.5 --- .../src/__tests__/recommendations.test.ts | 37 +++++++++++++++++++ frontend/src/recommendations.ts | 3 +- 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/frontend/src/__tests__/recommendations.test.ts b/frontend/src/__tests__/recommendations.test.ts index 76dabb420..f3c5067b6 100644 --- a/frontend/src/__tests__/recommendations.test.ts +++ b/frontend/src/__tests__/recommendations.test.ts @@ -3564,6 +3564,43 @@ describe('Bundle B: column header filter triggers', () => { await load(variants, ['v1684-au']); expect(document.getElementById('recommendations-action-summary')?.textContent).toMatch(/across 1 row\)$/); }); + + // Issue #1705: the Savings Plans child rows use the same nesting rules. + test('Savings Plans children: a single plan keeps its type, variants of a multi-variant plan stay blank', async () => { + const sp = { provider: 'aws', cloud_account_id: 'a1705', region: 'us-east-1', count: 1, upfront_cost: 0 }; + await load([ + { ...sp, id: 'sp1705-compute', service: 'savings-plans-compute', resource_type: 'Compute SP', term: 1, payment: 'no-upfront', savings: 40 }, + { ...sp, id: 'sp1705-ec2-1', service: 'savings-plans-ec2instance', resource_type: '', term: 1, payment: 'no-upfront', savings: 30 }, + { ...sp, id: 'sp1705-ec2-3', service: 'savings-plans-ec2instance', resource_type: '', term: 3, payment: 'no-upfront', savings: 50 }, + ]); + const spChevron = document.querySelector('.rec-sp-group-chevron'); + if (spChevron?.getAttribute('aria-expanded') !== 'true') spChevron!.click(); + + // The single-plan child is not a variant: it keeps its resource type. + const single = document.querySelector('tr.recommendation-row[data-rec-id="sp1705-compute"]')!; + expect(single.textContent).toContain('Compute SP'); + + // The multi-variant child's summary skips its blank resource type. + const childSummary = document.querySelector('tr.rec-sp-group-child')!; + expect(childSummary.querySelector('.rec-cell-identity')?.textContent?.trim()).toBe('us-east-1 \u2014 2 variants'); + + const childChevron = childSummary.querySelector('.rec-cell-chevron')!; + if (childChevron.getAttribute('aria-expanded') !== 'true') childChevron.click(); + const variant = document.querySelector('tr.rec-variant-row[data-rec-id="sp1705-ec2-3"]')!; + expect(Array.from(variant.cells).slice(1, 5).map((td) => td.textContent?.trim())).toEqual(['', '', '', '']); + expect(variant.textContent).not.toContain('us-east-1'); + }); + + test('payback card treats a lower bound that rounds to 0.0 as no payback', async () => { + const tiny = { ...base, resource_type: 'db.r6g.large' }; + // Best end: 1 / 100 = 0.01 months, which rounds to "0.0"; worst end: 600 / 100 = 6 months. + await load([ + { ...tiny, id: 'v1705-tiny', term: 1, payment: 'partial-upfront', savings: 100, upfront_cost: 1 }, + { ...tiny, id: 'v1705-full', term: 1, payment: 'all-upfront', savings: 100, upfront_cost: 600 }, + ]); + expect(paybackText()).toContain('Up to 6.0 months'); + expect(paybackText()).not.toContain('0.0 \u2013'); + }); }); // Issue #164: cross-column-aware (cascading) distinct values. diff --git a/frontend/src/recommendations.ts b/frontend/src/recommendations.ts index b87e3a7de..51c7661dc 100644 --- a/frontend/src/recommendations.ts +++ b/frontend/src/recommendations.ts @@ -1607,7 +1607,8 @@ function formatPaybackRange(min: number, max: number): string { const lo = min.toFixed(1); const hi = max.toFixed(1); if (lo === hi) return `${lo} months`; - return min > 0 ? `${lo} – ${hi} months` : `Up to ${hi} months`; + // Decide on the rounded value: a positive bound under 0.05 also prints "0.0" (#1705). + return lo === '0.0' ? `Up to ${hi} months` : `${lo} – ${hi} months`; } /** Format a term range, collapsing "1 yr – 1 yr" to "1 yr". */ From bfe543b30b78e5d5947d3806d47639388e214592 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Wed, 4 Nov 2026 19:29:36 +0100 Subject: [PATCH 2/2] fix(frontend): treat a negative payback lower bound as no payback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rounded-string check printed "-0.0 – X months" for a lower bound just below zero (a negative upfront), where the previous min > 0 check printed "Up to X months". Compare the rounded value numerically. Refs #1705 Co-Authored-By: Claude Opus 5.5 --- frontend/src/__tests__/recommendations.test.ts | 11 +++++++++++ frontend/src/recommendations.ts | 5 +++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/frontend/src/__tests__/recommendations.test.ts b/frontend/src/__tests__/recommendations.test.ts index f3c5067b6..678e7587b 100644 --- a/frontend/src/__tests__/recommendations.test.ts +++ b/frontend/src/__tests__/recommendations.test.ts @@ -3601,6 +3601,17 @@ describe('Bundle B: column header filter triggers', () => { expect(paybackText()).toContain('Up to 6.0 months'); expect(paybackText()).not.toContain('0.0 \u2013'); }); + + test('payback card treats a negative lower bound as no payback', async () => { + const credit = { ...base, resource_type: 'db.r6g.xlarge' }; + // A negative upfront (a credit) gives -1 / 100 = -0.01 months, which rounds to "-0.0". + await load([ + { ...credit, id: 'v1705-credit', term: 1, payment: 'partial-upfront', savings: 100, upfront_cost: -1 }, + { ...credit, id: 'v1705-paid', term: 1, payment: 'all-upfront', savings: 100, upfront_cost: 600 }, + ]); + expect(paybackText()).toContain('Up to 6.0 months'); + expect(paybackText()).not.toContain('-0.0'); + }); }); // Issue #164: cross-column-aware (cascading) distinct values. diff --git a/frontend/src/recommendations.ts b/frontend/src/recommendations.ts index 51c7661dc..352362b5b 100644 --- a/frontend/src/recommendations.ts +++ b/frontend/src/recommendations.ts @@ -1607,8 +1607,9 @@ function formatPaybackRange(min: number, max: number): string { const lo = min.toFixed(1); const hi = max.toFixed(1); if (lo === hi) return `${lo} months`; - // Decide on the rounded value: a positive bound under 0.05 also prints "0.0" (#1705). - return lo === '0.0' ? `Up to ${hi} months` : `${lo} – ${hi} months`; + // Decide on the rounded value: a positive bound under 0.05 prints "0.0" and a + // negative one "-0.0" or below, and none of them is a payback period (#1705). + return Number(lo) <= 0 ? `Up to ${hi} months` : `${lo} – ${hi} months`; } /** Format a term range, collapsing "1 yr – 1 yr" to "1 yr". */