diff --git a/frontend/src/__tests__/api-inventory.test.ts b/frontend/src/__tests__/api-inventory.test.ts index 0585be734..af6672b0e 100644 --- a/frontend/src/__tests__/api-inventory.test.ts +++ b/frontend/src/__tests__/api-inventory.test.ts @@ -7,7 +7,7 @@ */ import { apiRequest } from '../api/client'; -import { listActiveCommitments } from '../api/inventory'; +import { listActiveCommitments, getCoverageBreakdown } from '../api/inventory'; jest.mock('../api/client', () => ({ apiRequest: jest.fn(), @@ -68,3 +68,33 @@ describe('listActiveCommitments', () => { expect(result).toEqual([]); }); }); + +describe('getCoverageBreakdown', () => { + beforeEach(() => { + (apiRequest as jest.Mock).mockReset(); + }); + + test('calls /inventory/coverage', async () => { + (apiRequest as jest.Mock).mockResolvedValue({ providers: [] }); + + await getCoverageBreakdown(); + + expect(apiRequest).toHaveBeenCalledWith('/inventory/coverage'); + }); + + test('returns the full response envelope including providers array', async () => { + const payload = { + providers: [ + { provider: 'aws', services: null, overall_coverage_pct: null }, + { provider: 'azure', services: null, overall_coverage_pct: null }, + { provider: 'gcp', services: null, overall_coverage_pct: null }, + ], + }; + (apiRequest as jest.Mock).mockResolvedValue(payload); + + const result = await getCoverageBreakdown(); + + expect(result).toEqual(payload); + expect(result.providers).toHaveLength(3); + }); +}); diff --git a/frontend/src/__tests__/inventory.test.ts b/frontend/src/__tests__/inventory.test.ts index 60d818962..fe3f71044 100644 --- a/frontend/src/__tests__/inventory.test.ts +++ b/frontend/src/__tests__/inventory.test.ts @@ -1,10 +1,8 @@ /** - * Inventory & Coverage section tests (issue #340 T4 + deferred sub-task - * for the Active commitments table). + * Inventory & Coverage section tests (issue #340 T4, #754). * * Verifies the sub-tab switching machinery for the umbrella section AND - * the per-commitment table fetch+render flow (skeleton → table | empty | - * error). The coverage sub-section remains an intentional placeholder. + * the per-commitment / coverage fetch+render flows. */ // loadRIExchange is a side-effect import from a module that touches the @@ -14,16 +12,18 @@ jest.mock('../riexchange', () => ({ loadRIExchange: jest.fn(), })); -// The active-commitments load path hits the API. Mock the entire api -// barrel so we don't need to stand up fetch — tests exercise the render -// machinery, not the network shape. +// The active-commitments and coverage load paths hit the API. Mock the +// entire api barrel so we don't need to stand up fetch — tests exercise +// the render machinery, not the network shape. jest.mock('../api', () => ({ listActiveCommitments: jest.fn(), + getCoverageBreakdown: jest.fn(), })); -import { loadInventory, switchInventorySubSection, loadActiveCommitments } from '../inventory'; +import { loadInventory, switchInventorySubSection, loadActiveCommitments, loadCoverageBreakdown } from '../inventory'; import { loadRIExchange } from '../riexchange'; import * as api from '../api'; +import type { ProviderCoverageSection } from '../api'; function buildInventoryDOM(): void { // Build the inventory tab + sub-nav via DOM methods rather than an @@ -66,16 +66,24 @@ function buildInventoryDOM(): void { ac.appendChild(list); tab.appendChild(ac); - for (const [id, hidden, body] of [ - ['inventory-coverage', true, 'coverage'], - ['inventory-ri-exchange', false, 'ri-exchange'], - ] as const) { - const section = document.createElement('section'); - section.id = id; - if (hidden) section.classList.add('hidden'); - section.textContent = body; - tab.appendChild(section); - } + // coverage section: matches the real HTML structure with refresh button + + // providers container that loadCoverageBreakdown renders into. + const coverageSection = document.createElement('section'); + coverageSection.id = 'inventory-coverage'; + coverageSection.classList.add('hidden'); + const coverageRefresh = document.createElement('button'); + coverageRefresh.id = 'coverage-refresh-btn'; + coverageRefresh.textContent = 'Refresh'; + coverageSection.appendChild(coverageRefresh); + const coverageProviders = document.createElement('div'); + coverageProviders.id = 'coverage-providers'; + coverageSection.appendChild(coverageProviders); + tab.appendChild(coverageSection); + + const riSection = document.createElement('section'); + riSection.id = 'inventory-ri-exchange'; + riSection.textContent = 'ri-exchange'; + tab.appendChild(riSection); document.body.appendChild(tab); } @@ -115,6 +123,9 @@ describe('Inventory & Coverage sub-section switching', () => { // switching tests don't need to care about the fetch outcome. (api.listActiveCommitments as jest.Mock).mockReset(); (api.listActiveCommitments as jest.Mock).mockResolvedValue([]); + // getCoverageBreakdown is invoked when switching to the coverage sub-tab. + (api.getCoverageBreakdown as jest.Mock).mockReset(); + (api.getCoverageBreakdown as jest.Mock).mockResolvedValue({ providers: [] }); }); afterEach(() => { @@ -264,3 +275,138 @@ describe('loadActiveCommitments — fetch + render flow', () => { expect(accountCell?.querySelector('.monospace')).not.toBeNull(); }); }); + +// ────────────────────────────────────────────── +// loadCoverageBreakdown — fetch + render flow (issue #754) +// ────────────────────────────────────────────── + +function makeProviderSection( + provider: string, + services: ProviderCoverageSection['services'], + overallPct: number | null +): ProviderCoverageSection { + return { provider, services, overall_coverage_pct: overallPct }; +} + +describe('loadCoverageBreakdown — fetch + render flow', () => { + beforeEach(() => { + buildInventoryDOM(); + (api.getCoverageBreakdown as jest.Mock).mockReset(); + }); + + afterEach(() => { + clearDOM(); + }); + + test('renders per-provider sections with service rows', async () => { + (api.getCoverageBreakdown as jest.Mock).mockResolvedValue({ + providers: [ + makeProviderSection('aws', [ + { service: 'ec2', covered_monthly: 200, on_demand_monthly: 300, coverage_pct: 40 }, + { service: 'rds', covered_monthly: 100, on_demand_monthly: 0, coverage_pct: 100 }, + ], 50), + makeProviderSection('azure', null, null), + makeProviderSection('gcp', null, null), + ], + }); + + await loadCoverageBreakdown(); + + const container = document.getElementById('coverage-providers')!; + const cards = container.querySelectorAll('.coverage-provider-card'); + expect(cards.length).toBe(3); + + // AWS card: has service table rows. + const awsCard = cards[0]!; + expect(awsCard.textContent).toContain('AWS'); + expect(awsCard.textContent).toContain('50.0% covered'); + const rows = awsCard.querySelectorAll('tbody tr'); + expect(rows.length).toBe(2); + expect(rows[0]!.textContent).toContain('ec2'); + expect(rows[0]!.textContent).toContain('40.0%'); + expect(rows[1]!.textContent).toContain('rds'); + expect(rows[1]!.textContent).toContain('100.0%'); + }); + + test('renders "No usage detected" for providers with null services', async () => { + (api.getCoverageBreakdown as jest.Mock).mockResolvedValue({ + providers: [ + makeProviderSection('aws', null, null), + makeProviderSection('azure', null, null), + makeProviderSection('gcp', null, null), + ], + }); + + await loadCoverageBreakdown(); + + const container = document.getElementById('coverage-providers')!; + const empties = container.querySelectorAll('.empty'); + expect(empties.length).toBe(3); + expect(empties[0]!.textContent).toContain('AWS'); + }); + + test('renders N/A for null coverage_pct (no usage signal on that service)', async () => { + (api.getCoverageBreakdown as jest.Mock).mockResolvedValue({ + providers: [ + makeProviderSection('aws', [ + { service: 'ec2', covered_monthly: 0, on_demand_monthly: 0, coverage_pct: null }, + ], null), + makeProviderSection('azure', null, null), + makeProviderSection('gcp', null, null), + ], + }); + + await loadCoverageBreakdown(); + + const container = document.getElementById('coverage-providers')!; + const row = container.querySelector('tbody tr'); + expect(row).not.toBeNull(); + expect(row!.textContent).toContain('N/A'); + }); + + test('renders an error paragraph when the API rejects', async () => { + (api.getCoverageBreakdown as jest.Mock).mockRejectedValue(new Error('network failure')); + + await loadCoverageBreakdown(); + + const container = document.getElementById('coverage-providers')!; + const err = container.querySelector('.error'); + expect(err).not.toBeNull(); + expect(err!.textContent).toContain('network failure'); + }); + + test('refresh button re-invokes the fetch', async () => { + (api.getCoverageBreakdown as jest.Mock).mockResolvedValue({ providers: [] }); + + await loadCoverageBreakdown(); + expect(api.getCoverageBreakdown).toHaveBeenCalledTimes(1); + + const btn = document.getElementById('coverage-refresh-btn')!; + btn.click(); + await Promise.resolve(); + expect(api.getCoverageBreakdown).toHaveBeenCalledTimes(2); + }); + + test('coverage bar has non-empty text and aria-label for screen readers', async () => { + (api.getCoverageBreakdown as jest.Mock).mockResolvedValue({ + providers: [ + makeProviderSection('aws', [ + { service: 'ec2', covered_monthly: 100, on_demand_monthly: 100, coverage_pct: 50 }, + ], 50), + makeProviderSection('azure', null, null), + makeProviderSection('gcp', null, null), + ], + }); + + await loadCoverageBreakdown(); + + const container = document.getElementById('coverage-providers')!; + const headers = Array.from(container.querySelectorAll('thead th')); + // The last header column is the coverage bar — it must have a visible + // label (not empty string) so screen readers announce the column purpose. + const barTh = headers[headers.length - 1]; + expect(barTh).not.toBeNull(); + expect(barTh!.textContent).toBe('Coverage bar'); + expect(barTh!.getAttribute('aria-label')).toBe('Coverage bar'); + }); +}); diff --git a/frontend/src/api/index.ts b/frontend/src/api/index.ts index 82802abd7..c9c985346 100644 --- a/frontend/src/api/index.ts +++ b/frontend/src/api/index.ts @@ -54,7 +54,10 @@ export type { ExchangeResult, RIExchangeConfig, RIExchangeHistoryRecord, - InventoryCommitment + InventoryCommitment, + CoverageServiceRow, + ProviderCoverageSection, + CoverageBreakdownResponse } from './types'; // Re-export client functions @@ -170,7 +173,8 @@ export { // Re-export inventory functions export { - listActiveCommitments + listActiveCommitments, + getCoverageBreakdown } from './inventory'; // Re-export RI exchange functions diff --git a/frontend/src/api/inventory.ts b/frontend/src/api/inventory.ts index a43322948..288f9cf85 100644 --- a/frontend/src/api/inventory.ts +++ b/frontend/src/api/inventory.ts @@ -8,7 +8,7 @@ */ import { apiRequest } from './client'; -import type { InventoryCommitment } from './types'; +import type { CoverageBreakdownResponse, InventoryCommitment } from './types'; /** * List active (non-expired) commitments across the user's accessible @@ -21,3 +21,12 @@ export async function listActiveCommitments(accountID?: string): Promise(`/inventory/commitments${qs}`); return resp.commitments ?? []; } + +/** + * Fetch per-provider, per-service coverage breakdowns. + * Returns one section per known provider (aws, azure, gcp). A provider + * with no usage data has services=null and overall_coverage_pct=null. + */ +export async function getCoverageBreakdown(): Promise { + return apiRequest('/inventory/coverage'); +} diff --git a/frontend/src/api/types.ts b/frontend/src/api/types.ts index 78bf5861d..e051f26ac 100644 --- a/frontend/src/api/types.ts +++ b/frontend/src/api/types.ts @@ -670,6 +670,36 @@ export interface InventoryCommitment { status: string; } +// Coverage breakdown types (issue #754) + +/** + * One service row within a provider's coverage section. + * coverage_pct is null when both covered_monthly and on_demand_monthly + * are zero (no usage detected) -- do not coerce null to 0. + */ +export interface CoverageServiceRow { + service: string; + covered_monthly: number; + on_demand_monthly: number; + coverage_pct: number | null; +} + +/** + * Per-provider coverage section returned by GET /api/inventory/coverage. + * services is null when the provider has no usage data. Render "No usage + * detected" rather than an empty table in that case. + */ +export interface ProviderCoverageSection { + provider: string; + services: CoverageServiceRow[] | null; + overall_coverage_pct: number | null; +} + +/** Envelope returned by GET /api/inventory/coverage. */ +export interface CoverageBreakdownResponse { + providers: ProviderCoverageSection[]; +} + // Internal types export interface ApiError extends Error { status?: number; diff --git a/frontend/src/index.html b/frontend/src/index.html index 59813a16c..b1f88a322 100644 --- a/frontend/src/index.html +++ b/frontend/src/index.html @@ -239,12 +239,18 @@

Active commitments

- + diff --git a/frontend/src/inventory.ts b/frontend/src/inventory.ts index 810229041..487f78d85 100644 --- a/frontend/src/inventory.ts +++ b/frontend/src/inventory.ts @@ -1,19 +1,17 @@ /** - * Inventory & Coverage section (issue #340 T4). + * Inventory & Coverage section (issue #340 T4, #754). * * Umbrella section that folds the former top-level "RI Exchange" tab into * a sub-section of a broader Inventory & Coverage view. Sub-sections: * - active-commitments — per-commitment list backed by * /api/inventory/commitments - * - coverage — placeholder until per-provider donuts land + * - coverage — per-provider coverage breakdowns backed by + * /api/inventory/coverage (issue #754) * - ri-exchange — hosts the existing RI Exchange UI unchanged - * - * The coverage sub-section is still an intentional empty state — its - * backend endpoint isn't in scope for #340 and remains a deferred - * sub-task. */ import * as api from './api'; +import type { ProviderCoverageSection, CoverageServiceRow } from './api'; import { loadRIExchange } from './riexchange'; import { showSkeletonRows, teardownSkeleton } from './lib/skeleton'; import { formatCurrency, formatDate } from './utils'; @@ -58,6 +56,8 @@ export function switchInventorySubSection(name: string): void { void loadRIExchange(); } else if (target === 'active-commitments') { void loadActiveCommitments(); + } else if (target === 'coverage') { + void loadCoverageBreakdown(); } currentSubSection = target; @@ -211,6 +211,158 @@ function buildAccountCell(c: api.InventoryCommitment): HTMLTableCellElement { return td; } +// ────────────────────────────────────────────── +// Coverage breakdown +// ────────────────────────────────────────────── + +const COVERAGE_CONTAINER_ID = 'coverage-providers'; +const COVERAGE_REFRESH_BTN_ID = 'coverage-refresh-btn'; + +const PROVIDER_DISPLAY_NAMES: Record = { + aws: 'AWS', + azure: 'Azure', + gcp: 'GCP', +}; + +/** + * Fetch and render per-provider coverage breakdowns into #coverage-providers. + * Shows a skeleton on entry, then either the rendered sections or an error. + * Idempotent — safe to call on every sub-tab switch and on every refresh click. + */ +export async function loadCoverageBreakdown(): Promise { + const container = document.getElementById(COVERAGE_CONTAINER_ID); + if (!container) return; + + wireCoverageRefreshButton(); + + // One skeleton row per known provider while loading. + showSkeletonRows(container, 3, 1); + + try { + const data = await api.getCoverageBreakdown(); + renderCoverageBreakdown(container, data.providers); + } catch (error) { + teardownSkeleton(container); + const err = error as Error; + renderErrorParagraph(container, `Failed to load coverage data: ${err.message}`); + } +} + +function wireCoverageRefreshButton(): void { + const btn = document.getElementById(COVERAGE_REFRESH_BTN_ID); + if (!btn) return; + if (btn.dataset['wired'] === '1') return; + btn.addEventListener('click', () => { + void loadCoverageBreakdown(); + }); + btn.dataset['wired'] = '1'; +} + +/** + * Render coverage sections. Each provider gets its own card. Providers + * with services=null show an empty-state paragraph. All text is set via + * textContent -- no innerHTML -- so no escaping helper is needed (XSS posture + * matches the active-commitments section per issue #340). + */ +function renderCoverageBreakdown(container: HTMLElement, providers: ProviderCoverageSection[]): void { + clearChildren(container); + + if (!providers || providers.length === 0) { + renderEmptyParagraph(container, 'No coverage data available.'); + return; + } + + for (const section of providers) { + container.appendChild(buildProviderSection(section)); + } +} + +function buildProviderSection(section: ProviderCoverageSection): HTMLElement { + const card = document.createElement('section'); + card.className = 'card coverage-provider-card'; + + // Header row: provider name + overall coverage badge. + const header = document.createElement('div'); + header.className = 'section-header'; + + const title = document.createElement('h3'); + title.textContent = PROVIDER_DISPLAY_NAMES[section.provider] ?? section.provider.toUpperCase(); + header.appendChild(title); + + if (section.overall_coverage_pct !== null && section.overall_coverage_pct !== undefined) { + const badge = document.createElement('span'); + badge.className = 'coverage-overall-badge'; + badge.textContent = `Overall: ${section.overall_coverage_pct.toFixed(1)}% covered`; + header.appendChild(badge); + } + card.appendChild(header); + + // Body: empty-state or per-service table. + if (!section.services || section.services.length === 0) { + const empty = document.createElement('p'); + empty.className = 'empty'; + empty.textContent = `No usage detected for ${PROVIDER_DISPLAY_NAMES[section.provider] ?? section.provider}.`; + card.appendChild(empty); + return card; + } + + card.appendChild(buildServiceTable(section.services)); + return card; +} + +function buildServiceTable(rows: CoverageServiceRow[]): HTMLTableElement { + const table = document.createElement('table'); + table.className = 'coverage-service-table'; + + const thead = document.createElement('thead'); + const headerRow = document.createElement('tr'); + for (const label of ['Service', 'Covered/mo', 'On-demand gap/mo', 'Coverage %', 'Coverage bar']) { + const th = document.createElement('th'); + th.textContent = label; + if (label === 'Coverage bar') { + th.setAttribute('aria-label', 'Coverage bar'); + } + headerRow.appendChild(th); + } + thead.appendChild(headerRow); + table.appendChild(thead); + + const tbody = document.createElement('tbody'); + for (const row of rows) { + tbody.appendChild(buildServiceRow(row)); + } + table.appendChild(tbody); + return table; +} + +function buildServiceRow(row: CoverageServiceRow): HTMLTableRowElement { + const tr = document.createElement('tr'); + + appendCell(tr, row.service); + appendCell(tr, formatCurrency(row.covered_monthly)); + appendCell(tr, formatCurrency(row.on_demand_monthly)); + appendCell(tr, row.coverage_pct !== null && row.coverage_pct !== undefined + ? `${row.coverage_pct.toFixed(1)}%` + : 'N/A'); + // Bar cell: visual coverage indicator. + const barTd = document.createElement('td'); + barTd.className = 'coverage-bar-cell'; + if (row.coverage_pct !== null && row.coverage_pct !== undefined) { + const bar = document.createElement('div'); + bar.className = 'coverage-bar'; + const fill = document.createElement('div'); + fill.className = 'coverage-bar-fill'; + // Clamp to [0, 100] so a misconfigured value can't overflow. + const pct = Math.min(100, Math.max(0, row.coverage_pct)); + fill.style.width = `${pct}%`; + bar.appendChild(fill); + barTd.appendChild(bar); + } + tr.appendChild(barTd); + + return tr; +} + /** * Wire sub-nav button clicks. Idempotent — calling this more than once * doesn't double-bind handlers. diff --git a/internal/api/handler_inventory.go b/internal/api/handler_inventory.go index d5713eb64..6323ffdaa 100644 --- a/internal/api/handler_inventory.go +++ b/internal/api/handler_inventory.go @@ -10,6 +10,12 @@ import ( "github.com/aws/aws-lambda-go/events" ) +// knownProviders is the ordered list of providers we always include in +// the coverage response so the frontend can rely on a stable ordering. +// Providers with no usage data get an empty Services slice and a nil +// OverallCoveragePct — the frontend renders "No usage detected" for them. +var knownProviders = []string{"aws", "azure", "gcp"} + // listActiveCommitments handles GET /api/inventory/commitments. // // Returns one row per *active* (non-expired) PurchaseHistoryRecord, with @@ -100,3 +106,172 @@ func buildInventoryCommitment(p config.PurchaseHistoryRecord, accountName string Status: "active", } } + +// getCoverageBreakdown handles GET /api/inventory/coverage. +// +// Returns per-provider, per-service coverage breakdowns computed from +// two data sources already available in the system: +// - Active commitments (purchase history): their MonthlyCost is the +// "covered" portion of monthly spend. +// - Recommendations (scheduler): their Savings represent the remaining +// on-demand gap that could still be committed. +// +// Coverage% per service = covered / (covered + on_demand). +// A provider with no data in either source gets Services=nil and +// OverallCoveragePct=nil — the frontend renders "No usage detected". +// +// Auth: `view:purchases`. Same gate as /api/inventory/commitments; +// both pull from purchase history so the role is consistent. +func (h *Handler) getCoverageBreakdown(ctx context.Context, req *events.LambdaFunctionURLRequest, params map[string]string) (any, error) { + session, err := h.requirePermission(ctx, req, "view", "purchases") + if err != nil { + return nil, err + } + + // --- covered: active commitments ---------------------------------------- + purchases, err := h.fetchCommitmentRecords(ctx, params) + if err != nil { + return nil, err + } + purchases, err = h.filterPurchaseHistoryByAllowedAccounts(ctx, session, purchases) + if err != nil { + return nil, err + } + + now := time.Now() + // coveredByKey accumulates MonthlyCost by "provider:service". + coveredByKey := make(map[string]float64) + for _, p := range purchases { + if !isActiveCommitment(p, now) { + continue + } + coveredByKey[p.Provider+":"+p.Service] += p.MonthlyCost + } + + // --- on-demand gap: recommendations ------------------------------------- + // Recommendations represent uncommitted demand that could be purchased. + // Their Savings field is the monthly on-demand cost of the uncovered gap. + recs, err := h.scheduler.ListRecommendations(ctx, config.RecommendationFilter{}) + if err != nil { + // Non-fatal: recommendations are best-effort for coverage display. + // An empty rec list is treated as "no uncovered gap" — coverage + // will show covered-only totals rather than failing the whole page. + recs = nil + } + // Apply the same allowed-accounts scope as the commitments query above so + // restricted users cannot infer data from accounts they are not entitled to. + if recs != nil { + recs, err = h.filterRecommendationsByAllowedAccounts(ctx, session, recs) + if err != nil { + return nil, err + } + } + onDemandByKey := make(map[string]float64) + for _, rec := range recs { + onDemandByKey[rec.Provider+":"+rec.Service] += rec.Savings + } + + return buildCoverageBreakdown(coveredByKey, onDemandByKey), nil +} + +// buildCoverageBreakdown constructs the CoverageBreakdownResponse from +// the two pre-aggregated maps. Extracted so it can be unit-tested without +// requiring a full Handler. +// +// coveredByKey and onDemandByKey are both keyed by "provider:service". +// A provider that appears in neither map gets Services=nil and +// OverallCoveragePct=nil (no usage detected). +func buildCoverageBreakdown( + coveredByKey map[string]float64, + onDemandByKey map[string]float64, +) CoverageBreakdownResponse { + // Collect all (provider, service) keys appearing in either map. + type providerService struct{ provider, service string } + keySet := make(map[providerService]struct{}) + for k := range coveredByKey { + provider, service := splitProviderService(k) + keySet[providerService{provider, service}] = struct{}{} + } + for k := range onDemandByKey { + provider, service := splitProviderService(k) + keySet[providerService{provider, service}] = struct{}{} + } + + // Group by provider, then build per-service rows. + providerSvcMap := make(map[string][]CoverageServiceRow) + for ps := range keySet { + key := ps.provider + ":" + ps.service + covered := coveredByKey[key] + onDemand := onDemandByKey[key] + row := CoverageServiceRow{ + Service: ps.service, + CoveredMonthly: covered, + OnDemandMonthly: onDemand, + CoveragePct: coveragePct(covered, onDemand), + } + providerSvcMap[ps.provider] = append(providerSvcMap[ps.provider], row) + } + + // Sort service rows within each provider for stable output. + for p := range providerSvcMap { + rows := providerSvcMap[p] + sort.SliceStable(rows, func(i, j int) bool { + return rows[i].Service < rows[j].Service + }) + providerSvcMap[p] = rows + } + + // Emit one section per known provider in canonical order; providers + // with no data get nil Services and nil OverallCoveragePct so the + // frontend can show "No usage detected" rather than an empty table. + sections := make([]ProviderCoverageSection, 0, len(knownProviders)) + for _, p := range knownProviders { + rows := providerSvcMap[p] + if len(rows) == 0 { + sections = append(sections, ProviderCoverageSection{ + Provider: p, + Services: nil, + OverallCoveragePct: nil, + }) + continue + } + + var totalCovered, totalOnDemand float64 + for _, r := range rows { + totalCovered += r.CoveredMonthly + totalOnDemand += r.OnDemandMonthly + } + sections = append(sections, ProviderCoverageSection{ + Provider: p, + Services: rows, + OverallCoveragePct: coveragePct(totalCovered, totalOnDemand), + }) + } + + return CoverageBreakdownResponse{Providers: sections} +} + +// coveragePct returns covered / (covered + onDemand) * 100 as a *float64. +// Returns nil when both inputs are zero (no usage signal) to preserve the +// "absent" semantic per feedback_nullable_not_zero — callers should render +// nil as "N/A", not "0%". +func coveragePct(covered, onDemand float64) *float64 { + total := covered + onDemand + if total == 0 { + return nil + } + pct := covered / total * 100 + return &pct +} + +// splitProviderService splits a "provider:service" key back into its two +// parts. The separator is always the first colon so service names +// containing colons (unlikely but possible) are preserved. +func splitProviderService(key string) (provider, service string) { + for i := 0; i < len(key); i++ { + if key[i] == ':' { + return key[:i], key[i+1:] + } + } + return key, "" +} diff --git a/internal/api/handler_inventory_test.go b/internal/api/handler_inventory_test.go index 3cef99275..76eeb5c1c 100644 --- a/internal/api/handler_inventory_test.go +++ b/internal/api/handler_inventory_test.go @@ -265,3 +265,145 @@ func TestHandler_isActiveCommitment_Predicate(t *testing.T) { assert.False(t, isActiveCommitment(expired, now), "a commitment whose term ended a year ago must be inactive") } + +// ────────────────────────────────────────────── +// buildCoverageBreakdown unit tests (issue #754) +// ────────────────────────────────────────────── + +// TestBuildCoverageBreakdown_SingleProvider verifies that covered/on-demand +// sums are correctly attributed per service and the overall coverage% is +// computed across all services in the provider. +func TestBuildCoverageBreakdown_SingleProvider(t *testing.T) { + covered := map[string]float64{ + "aws:ec2": 200.0, + "aws:rds": 100.0, + } + onDemand := map[string]float64{ + "aws:ec2": 300.0, // ec2 coverage = 200/(200+300) = 40% + // rds has no on-demand gap, so rds coverage = 100% + } + + resp := buildCoverageBreakdown(covered, onDemand) + + require.Len(t, resp.Providers, 3, "always 3 known providers") + aws := resp.Providers[0] + assert.Equal(t, "aws", aws.Provider) + require.NotNil(t, aws.Services, "AWS has usage so Services must be non-nil") + require.Len(t, aws.Services, 2) + + // Services are sorted alphabetically; ec2 < rds. + ec2 := aws.Services[0] + assert.Equal(t, "ec2", ec2.Service) + assert.Equal(t, 200.0, ec2.CoveredMonthly) + assert.Equal(t, 300.0, ec2.OnDemandMonthly) + require.NotNil(t, ec2.CoveragePct) + assert.InDelta(t, 40.0, *ec2.CoveragePct, 0.001, "ec2 coverage = 200/500 * 100") + + rds := aws.Services[1] + assert.Equal(t, "rds", rds.Service) + assert.Equal(t, 100.0, rds.CoveredMonthly) + assert.Equal(t, 0.0, rds.OnDemandMonthly) + require.NotNil(t, rds.CoveragePct) + assert.InDelta(t, 100.0, *rds.CoveragePct, 0.001, "rds coverage = 100/100 * 100") + + // Overall: (200+100) / (200+100+300+0) * 100 = 300/600 = 50% + require.NotNil(t, aws.OverallCoveragePct) + assert.InDelta(t, 50.0, *aws.OverallCoveragePct, 0.001, "AWS overall coverage = 300/600 * 100") +} + +// TestBuildCoverageBreakdown_EmptyProvider verifies that a provider with no +// data in either map gets Services=nil and OverallCoveragePct=nil — not +// a zero — per feedback_nullable_not_zero. +func TestBuildCoverageBreakdown_EmptyProvider(t *testing.T) { + covered := map[string]float64{"aws:ec2": 100.0} + onDemand := map[string]float64{"aws:ec2": 100.0} + + resp := buildCoverageBreakdown(covered, onDemand) + + require.Len(t, resp.Providers, 3) + for _, p := range resp.Providers { + if p.Provider == "aws" { + continue + } + assert.Nil(t, p.Services, "provider %s has no data, Services must be nil", p.Provider) + assert.Nil(t, p.OverallCoveragePct, "provider %s has no data, OverallCoveragePct must be nil", p.Provider) + } +} + +// TestBuildCoverageBreakdown_ZeroBothSides verifies that a service with +// both covered=0 and on_demand=0 produces a nil CoveragePct, not 0. +func TestBuildCoverageBreakdown_ZeroBothSides(t *testing.T) { + assert.Nil(t, coveragePct(0, 0), "no usage: coverage% must be nil, not 0") +} + +// TestBuildCoverageBreakdown_OnlyOnDemand verifies that a provider with +// recommendations but no commitments shows 0% coverage (not nil). +func TestBuildCoverageBreakdown_OnlyOnDemand(t *testing.T) { + covered := map[string]float64{} + onDemand := map[string]float64{"azure:compute": 500.0} + + resp := buildCoverageBreakdown(covered, onDemand) + + var azure *ProviderCoverageSection + for i := range resp.Providers { + if resp.Providers[i].Provider == "azure" { + azure = &resp.Providers[i] + break + } + } + require.NotNil(t, azure) + require.NotNil(t, azure.Services) + require.Len(t, azure.Services, 1) + require.NotNil(t, azure.Services[0].CoveragePct) + assert.InDelta(t, 0.0, *azure.Services[0].CoveragePct, 0.001, "0 covered / 500 on-demand = 0%") +} + +// TestHandler_getCoverageBreakdown_Integration exercises the full handler +// path including auth and purchase-history filtering. +func TestHandler_getCoverageBreakdown_Integration(t *testing.T) { + ctx := context.Background() + mockStore := new(MockConfigStore) + mockScheduler := new(MockScheduler) + t.Cleanup(func() { + mockStore.AssertExpectations(t) + mockScheduler.AssertExpectations(t) + }) + + now := time.Now() + purchases := []config.PurchaseHistoryRecord{ + { + AccountID: "acc-1", + PurchaseID: "p-1", + Provider: "aws", + Service: "ec2", + Timestamp: now.AddDate(-1, 0, 1), // active: 1y term started ~1y ago + Term: 1, + MonthlyCost: 150.0, + }, + } + recs := []config.RecommendationRecord{ + {Provider: "aws", Service: "ec2", Savings: 350.0}, + } + + mockStore.On("GetAllPurchaseHistory", ctx, config.MaxListLimit).Return(purchases, nil) + mockStore.ListCloudAccountsFn = func(_ context.Context, _ config.CloudAccountFilter) ([]config.CloudAccount, error) { + return []config.CloudAccount{}, nil + } + mockScheduler.On("ListRecommendations", ctx, config.RecommendationFilter{}).Return(recs, nil) + + mockAuth, req := adminInventoryReq(ctx) + handler := &Handler{auth: mockAuth, config: mockStore, scheduler: mockScheduler} + + result, err := handler.getCoverageBreakdown(ctx, req, map[string]string{}) + require.NoError(t, err) + + resp, ok := result.(CoverageBreakdownResponse) + require.True(t, ok) + require.Len(t, resp.Providers, 3) + + aws := resp.Providers[0] + assert.Equal(t, "aws", aws.Provider) + require.NotNil(t, aws.OverallCoveragePct) + // coverage = 150 / (150+350) * 100 = 30% + assert.InDelta(t, 30.0, *aws.OverallCoveragePct, 0.001) +} diff --git a/internal/api/handler_per_account_perms_test.go b/internal/api/handler_per_account_perms_test.go index 84e1ad7ca..eff74f43f 100644 --- a/internal/api/handler_per_account_perms_test.go +++ b/internal/api/handler_per_account_perms_test.go @@ -10,7 +10,7 @@ package api // minimal: just enough to confirm the filter passes when accounts match so we // know the test would fail for the right reason if the enforcement were removed. // -// Endpoints covered (8 total): +// Endpoints covered (9 total): // 1. GET /recommendations (filterRecommendationsByAllowedAccounts) // 2. GET /recommendations/:id (getRecommendationDetail — cross-account rejection) // 3. GET /history (filterPurchaseHistoryByAllowedAccounts) @@ -19,6 +19,7 @@ package api // 6. GET /dashboard/summary (filterDashboardRecommendations — aggregate subset) // 7. POST /purchases/execute (validatePurchaseRecommendationScope — 403) // 8. GET /purchases/planned list (requireExecutionAccess / requirePlanAccess — 404) +// 9. GET /inventory/coverage (filterRecommendationsByAllowedAccounts on recs leg) import ( "context" @@ -726,6 +727,161 @@ func TestPerAccountPerms_PlannedPurchase_CrossAccountPlanRejected404(t *testing. mockStore.AssertNotCalled(t, "TransitionExecutionStatus") } +// ─── 9. GET /inventory/coverage ────────────────────────────────────────────── + +// TestPerAccountPerms_CoverageBreakdown_RecsFilteredByAllowedAccounts asserts +// that a scoped user's coverage response aggregates on-demand savings only from +// account-A recommendations — account-B savings must be excluded from the +// onDemandByKey accumulation so the coverage% is not inflated by inaccessible data. +// +// Regression: if filterRecommendationsByAllowedAccounts is removed from +// getCoverageBreakdown's recommendations leg, the account-B savings (200) +// pollute onDemandByKey and the aws:ec2 on_demand_monthly in the response rises +// from 200 to 400, lowering the coverage% from 50% to 33%. +func TestPerAccountPerms_CoverageBreakdown_RecsFilteredByAllowedAccounts(t *testing.T) { + ctx := context.Background() + + now := time.Now() + // Active commitment for account A: contributes to covered side. + purchaseA := config.PurchaseHistoryRecord{ + AccountID: permsAccA, + PurchaseID: "p-cov-a", + Provider: "aws", + Service: "ec2", + Timestamp: now.AddDate(0, -6, 0), + Term: 1, + MonthlyCost: 200.0, + } + + // Recommendation for account A: on-demand gap the scoped user is allowed to see. + recA := config.RecommendationRecord{ + ID: "rec-cov-a", + Provider: "aws", + Service: "ec2", + CloudAccountID: permsPtr(permsAccA), + Savings: 200.0, + } + // Recommendation for account B: must be excluded for the scoped user. + recB := config.RecommendationRecord{ + ID: "rec-cov-b", + Provider: "aws", + Service: "ec2", + CloudAccountID: permsPtr(permsAccB), + Savings: 200.0, + } + + mockSched := new(MockScheduler) + mockSched.On("ListRecommendations", ctx, config.RecommendationFilter{}). + Return([]config.RecommendationRecord{recA, recB}, nil) + + mockStore := new(MockConfigStore) + mockStore.On("GetAllPurchaseHistory", ctx, config.MaxListLimit). + Return([]config.PurchaseHistoryRecord{purchaseA}, nil) + mockStore.ListCloudAccountsFn = func(_ context.Context, _ config.CloudAccountFilter) ([]config.CloudAccount, error) { + return permsAccountList(), nil + } + + handler := &Handler{ + auth: scopedAuthMock(ctx), + scheduler: mockSched, + config: mockStore, + } + + result, err := handler.getCoverageBreakdown(ctx, scopedReq(), map[string]string{}) + require.NoError(t, err) + require.NotNil(t, result) + + resp, ok := result.(CoverageBreakdownResponse) + require.True(t, ok) + + var aws *ProviderCoverageSection + for i := range resp.Providers { + if resp.Providers[i].Provider == "aws" { + aws = &resp.Providers[i] + break + } + } + require.NotNil(t, aws, "aws provider section must be present") + require.NotNil(t, aws.Services, "aws must have service rows") + require.Len(t, aws.Services, 1) + + ec2 := aws.Services[0] + assert.Equal(t, "ec2", ec2.Service) + assert.Equal(t, 200.0, ec2.CoveredMonthly, "covered side must reflect account-A commitment") + assert.Equal(t, 200.0, ec2.OnDemandMonthly, + "on-demand side must include only account-A rec (200); account-B rec (200) must be excluded") + // coverage = 200/(200+200) * 100 = 50% + require.NotNil(t, ec2.CoveragePct) + assert.InDelta(t, 50.0, *ec2.CoveragePct, 0.001, + "coverage must be computed from account-A data only; account-B leak would give ~33%%") +} + +// TestPerAccountPerms_CoverageBreakdown_AdminSeesAll confirms that an +// unrestricted session aggregates both accounts — the positive path through +// IsUnrestrictedAccess so the test above cannot pass by dropping all records. +func TestPerAccountPerms_CoverageBreakdown_AdminSeesAll(t *testing.T) { + ctx := context.Background() + + now := time.Now() + purchaseA := config.PurchaseHistoryRecord{ + AccountID: permsAccA, + PurchaseID: "p-cov-admin-a", + Provider: "aws", + Service: "ec2", + Timestamp: now.AddDate(0, -6, 0), + Term: 1, + MonthlyCost: 200.0, + } + recA := config.RecommendationRecord{ + Provider: "aws", Service: "ec2", + CloudAccountID: permsPtr(permsAccA), Savings: 200.0, + } + recB := config.RecommendationRecord{ + Provider: "aws", Service: "ec2", + CloudAccountID: permsPtr(permsAccB), Savings: 200.0, + } + + mockSched := new(MockScheduler) + mockSched.On("ListRecommendations", ctx, config.RecommendationFilter{}). + Return([]config.RecommendationRecord{recA, recB}, nil) + + mockStore := new(MockConfigStore) + mockStore.On("GetAllPurchaseHistory", ctx, config.MaxListLimit). + Return([]config.PurchaseHistoryRecord{purchaseA}, nil) + mockStore.ListCloudAccountsFn = func(_ context.Context, _ config.CloudAccountFilter) ([]config.CloudAccount, error) { + return permsAccountList(), nil + } + + mockAuth, req := adminDashboardReq(ctx) + handler := &Handler{ + auth: mockAuth, + scheduler: mockSched, + config: mockStore, + } + + result, err := handler.getCoverageBreakdown(ctx, req, map[string]string{}) + require.NoError(t, err) + + resp, ok := result.(CoverageBreakdownResponse) + require.True(t, ok) + + var aws *ProviderCoverageSection + for i := range resp.Providers { + if resp.Providers[i].Provider == "aws" { + aws = &resp.Providers[i] + break + } + } + require.NotNil(t, aws) + require.Len(t, aws.Services, 1) + // Admin sees both recs: on_demand = 200+200 = 400; coverage = 200/600 * 100 = 33.3% + assert.Equal(t, 400.0, aws.Services[0].OnDemandMonthly, + "admin must see both accounts' on-demand gap (200+200)") + require.NotNil(t, aws.Services[0].CoveragePct) + assert.InDelta(t, 33.333, *aws.Services[0].CoveragePct, 0.01, + "admin coverage = 200/(200+400) * 100 = 33.3%%") +} + // TestPerAccountPerms_PlannedPurchase_AllowedAccountPlanSucceeds is the paired // positive case confirming the filter passes for account-A plans. func TestPerAccountPerms_PlannedPurchase_AllowedAccountPlanSucceeds(t *testing.T) { diff --git a/internal/api/router.go b/internal/api/router.go index 0447a89a2..1438f78fe 100644 --- a/internal/api/router.go +++ b/internal/api/router.go @@ -261,6 +261,10 @@ func (r *Router) registerRoutes() { // handler also filters by the session's allowed_accounts list // so a restricted-access user sees only their entitled rows. {ExactPath: "/api/inventory/commitments", Method: "GET", Handler: r.listInventoryCommitmentsHandler, Auth: AuthUser}, + // Per-provider, per-service coverage breakdown for the Coverage + // sub-tab (issue #754). AuthUser + allowed_accounts filter applied + // inside the handler, matching commitments endpoint precedent. + {ExactPath: "/api/inventory/coverage", Method: "GET", Handler: r.getCoverageBreakdownHandler, Auth: AuthUser}, // RI Exchange endpoints — GETs are AuthUser (Convertible RIs, // Reshape Recommendations, Exchange History pages all need this). @@ -683,6 +687,10 @@ func (r *Router) listInventoryCommitmentsHandler(ctx context.Context, req *event return r.h.listActiveCommitments(ctx, req, req.QueryStringParameters) } +func (r *Router) getCoverageBreakdownHandler(ctx context.Context, req *events.LambdaFunctionURLRequest, params map[string]string) (any, error) { + return r.h.getCoverageBreakdown(ctx, req, req.QueryStringParameters) +} + func (r *Router) listExchangeableAzureRIsHandler(ctx context.Context, req *events.LambdaFunctionURLRequest, params map[string]string) (any, error) { return r.h.listExchangeableAzureRIs(ctx, req) } diff --git a/internal/api/types.go b/internal/api/types.go index 067a4e2d2..3cce623d2 100644 --- a/internal/api/types.go +++ b/internal/api/types.go @@ -522,6 +522,38 @@ type InventoryCommitmentsResponse struct { Commitments []InventoryCommitment `json:"commitments"` } +// CoverageServiceRow is one service row within a provider's coverage +// section. CoveredMonthly is the sum of active-commitment MonthlyCost +// values for the (provider, service) pair. OnDemandMonthly is the sum +// of recommendation Savings values — i.e. the portion of on-demand +// spend that is NOT yet committed. CoveragePct is nil when both sums +// are zero (no usage detected), not 0, to preserve the "absent" +// semantic per feedback_nullable_not_zero. +type CoverageServiceRow struct { + Service string `json:"service"` + CoveredMonthly float64 `json:"covered_monthly"` + OnDemandMonthly float64 `json:"on_demand_monthly"` + CoveragePct *float64 `json:"coverage_pct"` +} + +// ProviderCoverageSection is the per-provider block returned by +// GET /api/inventory/coverage. Services is nil (not []) when the +// provider has no usage data, which the frontend uses to distinguish +// "no usage detected" from "usage exists but all services are 0%". +// OverallCoveragePct follows the same null-vs-zero contract as +// CoverageServiceRow.CoveragePct. +type ProviderCoverageSection struct { + Provider string `json:"provider"` + Services []CoverageServiceRow `json:"services"` + OverallCoveragePct *float64 `json:"overall_coverage_pct"` +} + +// CoverageBreakdownResponse is the envelope returned by +// GET /api/inventory/coverage. +type CoverageBreakdownResponse struct { + Providers []ProviderCoverageSection `json:"providers"` +} + // UpcomingPurchaseResponse holds upcoming purchase data type UpcomingPurchaseResponse struct { Purchases []UpcomingPurchase `json:"purchases"`