diff --git a/frontend/src/__tests__/plans.test.ts b/frontend/src/__tests__/plans.test.ts index d35465c18..097679e4c 100644 --- a/frontend/src/__tests__/plans.test.ts +++ b/frontend/src/__tests__/plans.test.ts @@ -3070,4 +3070,183 @@ describe('Plans Module', () => { expect(paymentSelect.value).toBe('all-upfront'); }); }); + + // Issue #340 follow-up (PR #376): per-plan health-score badge. + describe('plan health-score badge', () => { + // Mirror of the band thresholds in plans.ts. Declared here rather than + // imported because plans.ts does not export them; if they diverge the + // boundary cases below fail, which is the point. + const HEALTH_SCORE_GOOD_BOUNDARY = 80; + const HEALTH_SCORE_WARN_BOUNDARY = 50; + + const basePlan = { + id: 'plan-1', + name: 'Test Plan', + enabled: true, + auto_purchase: true, + services: { + ec2: { provider: 'aws', service: 'ec2', enabled: true, term: 1, payment: 'all-upfront', coverage: 80 } + }, + ramp_schedule: { type: 'immediate', percent_per_step: 100, step_interval_days: 0 } + }; + + test('renders a green badge for a healthy score (>= 80)', async () => { + (api.getPlans as jest.Mock).mockResolvedValue({ + plans: [{ ...basePlan, health_score: 90, health_factors: [] }] + }); + (api.getPlannedPurchases as jest.Mock).mockResolvedValue({ purchases: [] }); + + await loadPlans(); + + const list = document.getElementById('plans-list'); + expect(list?.innerHTML).toContain('badge-success'); + expect(list?.innerHTML).toContain('Health: 90'); + }); + + test('renders an amber badge for a medium score (50-79)', async () => { + (api.getPlans as jest.Mock).mockResolvedValue({ + plans: [{ + ...basePlan, + health_score: 65, + health_factors: [{ code: 'behind_schedule', penalty: 20, note: 'on step 1, expected step 3 by now' }] + }] + }); + (api.getPlannedPurchases as jest.Mock).mockResolvedValue({ purchases: [] }); + + await loadPlans(); + + const list = document.getElementById('plans-list'); + expect(list?.innerHTML).toContain('badge-warning'); + expect(list?.innerHTML).toContain('Health: 65'); + }); + + test('renders a red badge for a poor score (< 50)', async () => { + (api.getPlans as jest.Mock).mockResolvedValue({ + plans: [{ + ...basePlan, + health_score: 30, + health_factors: [ + { code: 'overdue', penalty: 30, note: 'next purchase date has passed' }, + { code: 'failed_executions', penalty: 40, note: '4 failed execution(s)' } + ] + }] + }); + (api.getPlannedPurchases as jest.Mock).mockResolvedValue({ purchases: [] }); + + await loadPlans(); + + const list = document.getElementById('plans-list'); + expect(list?.innerHTML).toContain('badge-danger'); + expect(list?.innerHTML).toContain('Health: 30'); + }); + + test('omits the badge entirely when health_score is absent (pre-feature API response)', async () => { + (api.getPlans as jest.Mock).mockResolvedValue({ plans: [{ ...basePlan }] }); + (api.getPlannedPurchases as jest.Mock).mockResolvedValue({ purchases: [] }); + + await loadPlans(); + + const list = document.getElementById('plans-list'); + expect(list?.innerHTML).not.toContain('Health:'); + }); + + // Regression guard for the fabricated-score bug: when the backend can't + // compute a score it sends null, and the UI must say so. Pre-fix the + // backend substituted 100 on a store failure, painting every plan with a + // confident green "healthy" badge built from data it never read. + test('renders an explicit unknown badge when health_score is null', async () => { + (api.getPlans as jest.Mock).mockResolvedValue({ + plans: [{ ...basePlan, health_score: null }] + }); + (api.getPlannedPurchases as jest.Mock).mockResolvedValue({ purchases: [] }); + + await loadPlans(); + + const list = document.getElementById('plans-list'); + expect(list?.innerHTML).toContain('Health: unknown'); + expect(list?.innerHTML).toContain('badge-muted'); + // Critically, it must NOT be dressed up as a healthy score. + expect(list?.innerHTML).not.toContain('badge-success'); + expect(list?.innerHTML).not.toContain('Health: 100'); + }); + + // Band boundaries, not just mid-band samples: with only 90/65/30 the + // suite cannot tell `>=` from `>`, so flipping either comparison would + // silently reclassify every plan sitting exactly on a threshold. + test.each([ + [HEALTH_SCORE_GOOD_BOUNDARY, 'badge-success'], + [HEALTH_SCORE_GOOD_BOUNDARY - 1, 'badge-warning'], + [HEALTH_SCORE_WARN_BOUNDARY, 'badge-warning'], + [HEALTH_SCORE_WARN_BOUNDARY - 1, 'badge-danger'], + ])('score %i sits in the %s band', async (score, expectedClass) => { + (api.getPlans as jest.Mock).mockResolvedValue({ + plans: [{ ...basePlan, health_score: score, health_factors: [] }] + }); + (api.getPlannedPurchases as jest.Mock).mockResolvedValue({ purchases: [] }); + + await loadPlans(); + + const list = document.getElementById('plans-list'); + expect(list?.innerHTML).toContain(expectedClass); + expect(list?.innerHTML).toContain(`Health: ${score}`); + }); + + // A score outside the 0-100 the API contracts to send is no more + // trustworthy than a NaN. Banding it would paint -1 red and 101 green as + // though both had been measured. + test.each([-1, 101, 1000])('renders unknown for out-of-range score %i', async (score) => { + (api.getPlans as jest.Mock).mockResolvedValue({ + plans: [{ ...basePlan, health_score: score }] + }); + (api.getPlannedPurchases as jest.Mock).mockResolvedValue({ purchases: [] }); + + await loadPlans(); + + const list = document.getElementById('plans-list'); + expect(list?.innerHTML).toContain('Health: unknown'); + expect(list?.innerHTML).toContain('badge-muted'); + expect(list?.innerHTML).not.toContain(`Health: ${score}`); + }); + + // A garbled score off the wire is an uncomputable score, so it must + // reach the same "unknown" badge rather than silently vanishing (which + // reads as "this deploy predates the feature"). + test('renders unknown rather than dropping the badge for a non-finite score', async () => { + (api.getPlans as jest.Mock).mockResolvedValue({ + plans: [{ ...basePlan, health_score: NaN }] + }); + (api.getPlannedPurchases as jest.Mock).mockResolvedValue({ purchases: [] }); + + await loadPlans(); + + const list = document.getElementById('plans-list'); + expect(list?.innerHTML).toContain('Health: unknown'); + expect(list?.innerHTML).not.toContain('Health: NaN'); + }); + + test('escapes health factor notes in the tooltip (XSS regression)', async () => { + const maliciousNote = '">'; + (api.getPlans as jest.Mock).mockResolvedValue({ + plans: [{ + ...basePlan, + health_score: 40, + health_factors: [{ code: 'stalled', penalty: 15, note: maliciousNote }] + }] + }); + (api.getPlannedPurchases as jest.Mock).mockResolvedValue({ purchases: [] }); + + await loadPlans(); + + const list = document.getElementById('plans-list'); + // Pre-fix, the unescaped `"` would close the title attribute early and + // the rest would parse as a live element. Post-fix the + // quote is entity-encoded so the whole malicious string stays inert + // text inside the title attribute -- assert on the parsed DOM (not the + // raw HTML string, since HTML serializers are free to leave `<`/`>` + // un-re-escaped inside an already-safe attribute value). + expect(list?.querySelector('img')).toBeNull(); + const badge = list?.querySelector('.badge-danger[title]'); + expect(badge?.getAttribute('title')).toContain(maliciousNote); + }); + }); }); diff --git a/frontend/src/api/index.ts b/frontend/src/api/index.ts index e59ac15d9..8b5c3e22f 100644 --- a/frontend/src/api/index.ts +++ b/frontend/src/api/index.ts @@ -17,6 +17,7 @@ export type { Recommendation, RecommendationFilters, Plan, + PlanHealthFactor, CreatePlanRequest, PurchaseHistory, HistoryFilters, diff --git a/frontend/src/api/types.ts b/frontend/src/api/types.ts index ec7dd3786..84a69737e 100644 --- a/frontend/src/api/types.ts +++ b/frontend/src/api/types.ts @@ -227,6 +227,24 @@ export interface Plan { // Such plans are surfaced under an "Unassigned" section in the Plans UI // so operators can find and re-scope them (issue #973). unassigned?: boolean; + // Health score/factors are computed at read time on GET /plans (never + // persisted) by computePlanHealth server-side (issue #340 follow-up). + // Optional so an older cached response served during a partial deploy + // still renders the card cleanly -- the badge just doesn't appear. + // Explicitly null when the backend could not compute a score; clients + // must render that as "unknown" rather than substituting a default. + health_score?: number | null; + health_factors?: PlanHealthFactor[]; +} + +// PlanHealthFactor is one penalty subtracted from a plan's health score +// (issue #340 follow-up). `code` is a fixed vocabulary from the backend +// (see internal/api/plan_health.go); `note` is a human-readable reason +// suitable for the health-badge tooltip. +export interface PlanHealthFactor { + code: string; + penalty: number; + note: string; } export interface CreatePlanRequest { diff --git a/frontend/src/plans.ts b/frontend/src/plans.ts index 369f99078..ae02d4d3a 100644 --- a/frontend/src/plans.ts +++ b/frontend/src/plans.ts @@ -861,6 +861,78 @@ interface BackendPlan { // (issue #973). The backend sets this flag when an account filter is // active so the frontend can bucket them under an "Unassigned" section. unassigned?: boolean; + // Health score/factors are computed at read time on GET /plans (never + // persisted) -- see computePlanHealth in internal/api/plan_health.go + // (issue #340 follow-up). Optional so an older cached response served + // during a partial deploy still renders the card cleanly; explicitly + // null when the backend could not compute the score. + health_score?: number | null; + health_factors?: api.PlanHealthFactor[]; +} + +// Score bands for the per-plan health badge (issue #340 follow-up). Green +// >= 80 (healthy), amber 50-79 (needs attention), red < 50 (action needed). +// Reuses the existing status-badge palette (badge-success/badge-warning/ +// badge-danger) already used elsewhere on this card rather than introducing +// new CSS. +const HEALTH_SCORE_GOOD_THRESHOLD = 80; +const HEALTH_SCORE_WARN_THRESHOLD = 50; + +// Inclusive bounds of the score the API contracts to send (openapi.yaml +// PlanWithHealth.health_score: minimum 0, maximum 100). Anything outside is +// treated as unusable rather than banded. +const HEALTH_SCORE_MIN = 0; +const HEALTH_SCORE_MAX = 100; + +function healthBadgeClass(score: number): string { + if (score >= HEALTH_SCORE_GOOD_THRESHOLD) return 'badge-success'; + if (score >= HEALTH_SCORE_WARN_THRESHOLD) return 'badge-warning'; + return 'badge-danger'; +} + +// healthBadgeHtml renders the per-plan health-score badge, distinguishing +// three states the API can report: +// +// - field absent: an older API response served during a partial deploy. +// Render nothing so the card still looks intentional. +// - field null: the backend could not compute the score (execution counts +// unavailable). Render an explicit "unknown" badge -- never a stand-in +// number, which an operator could not tell apart from a measured score. +// - a number within the contract's 0-100 range: the score, banded into the +// green/amber/red palette. +// - anything else (NaN, Infinity, out of range, a non-number off the wire): +// also "unknown". A score outside 0-100 violates the range the API +// declares, so it is no more trustworthy than a NaN, and banding it would +// paint -1 red and 101 green as though both were measured. Silently +// dropping the badge instead would read as "this deploy predates the +// feature". +// +// The tooltip enumerates every penalty factor so a bad score is actionable +// instead of opaque; every factor note is escaped since it renders inside an +// HTML attribute (title) via innerHTML (feedback_innerhtml_xss: all +// API-sourced values must be escaped here even though the backend generates +// these notes itself). +function healthBadgeHtml(plan: BackendPlan): string { + if (plan.health_score === undefined) return ''; + // The typeof/isFinite check is defensive: the type says number, but this + // value comes off the wire and is interpolated into innerHTML below. + if (plan.health_score === null + || typeof plan.health_score !== 'number' + || !Number.isFinite(plan.health_score) + || plan.health_score < HEALTH_SCORE_MIN + || plan.health_score > HEALTH_SCORE_MAX) { + // Neutral wording on purpose: null means the backend could not read the + // execution history, while a non-finite value means the number itself + // arrived unusable. Naming only the first would send an operator + // chasing a database problem that may not exist. + return `Health: unknown`; + } + const factors = plan.health_factors || []; + const factorLines = factors.length > 0 + ? factors.map(f => `-${f.penalty}: ${f.note}`) + : ['No issues detected']; + const tooltip = [`Plan health: ${plan.health_score}/100`, ...factorLines].join('\n'); + return `Health: ${plan.health_score}`; } // Pretty label for a service slug used inside the plan card. @@ -992,6 +1064,7 @@ function renderPlanCard(plan: BackendPlan, canManagePlan: boolean, canDeletePlan

${escapeHtml(plan.name)}

${status.label} + ${healthBadgeHtml(plan)} ${overdueBadge} ${canManagePlan && !isUnassigned ? `