fix(plans): show every service in multi-SP plan summary (closes #131)#156
Conversation
… first Closes #131. PR #123 (per-plan-type Savings Plans split) gave a single plan up to four `aws:savings-plans-<type>` entries, but `extractPlanInfo` in `frontend/src/plans.ts` rendered only `Object.values(services)[0]` — hiding all but one. The pre-existing "Multiple" placeholder for >1 service was an unhelpful stub: an operator looking at the card had no way to know which SP plan-types the plan actually covered. Fix: replace the "Multiple" branch with a comma-joined list of every service's label. SP slugs get an abbreviated label ("Compute SP", "SageMaker SP", "EC2 Instance SP", "Database SP") so a plan with all four still fits in the summary line. Non-SP slugs pass through unchanged so single-service plans render exactly as before (regression-safe). Tests: - `multi-SP plan summary lists every plan-type covered (issue #131)` pins the new behaviour: a plan with Compute SP + SageMaker SP shows both labels and the old "Multiple" placeholder is gone. - `single-service plan still renders one label (no regression)` pins the unchanged single-service path. Frontend-only change. Backend unchanged.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Summary
Closes #131.
PR #123 (per-plan-type Savings Plans split) gave a single plan up to four
aws:savings-plans-<type>entries, butextractPlanInfoinfrontend/src/plans.tsrendered onlyObject.values(services)[0]— hiding all but one. The pre-existing "Multiple" placeholder for >1 service was an unhelpful stub: operators looking at the card had no way to know which SP plan-types the plan covered.Fix
Replace the "Multiple" branch with a comma-joined list of every service's label. SP slugs get an abbreviated label ("Compute SP", "SageMaker SP", "EC2 Instance SP", "Database SP") so a plan with all four still fits in the summary line. Non-SP slugs pass through unchanged so single-service plans render exactly as before (regression-safe).
EC2EC2(unchanged)MultipleCompute SP, SageMaker SPMultipleCompute SP, EC2 Instance SP, SageMaker SP, Database SPMultipleEC2, Compute SPTests
multi-SP plan summary lists every plan-type covered (issue #131)— pins the new behaviour: Compute SP + SageMaker SP both appear, "Multiple" is gone.single-service plan still renders one label (no regression)— pins the unchanged single-service path.All 83 existing plan tests still pass.
Scope
Frontend-only. Backend unchanged.
🤖 Generated with claude-flow