diff --git a/frontend/src/__tests__/plans-overdue-181.test.ts b/frontend/src/__tests__/plans-overdue-181.test.ts new file mode 100644 index 000000000..a07989459 --- /dev/null +++ b/frontend/src/__tests__/plans-overdue-181.test.ts @@ -0,0 +1,123 @@ +/** + * Issue #181: a scheduled purchase that is past its date yet still pending + * read as an on-schedule "pending" row. The Planned Purchases table now adds + * an Overdue badge and names the next action the user can take. + * + * Drives the real loadPlans() render path. Dates are far in the past / + * future so the test does not depend on today's date. + */ +import { loadPlans } from '../plans'; + +jest.mock('../api', () => ({ + getPlans: jest.fn(), + getPlannedPurchases: jest.fn(), + listPlanAccounts: jest.fn().mockResolvedValue([]), +})); + +jest.mock('../state', () => ({ + getRecommendations: jest.fn().mockReturnValue([]), + getSelectedRecommendationIDs: jest.fn().mockReturnValue(new Set()), + getVisibleRecommendations: jest.fn().mockReturnValue([]), + setVisibleRecommendations: jest.fn(), + getCurrentProvider: jest.fn().mockReturnValue(''), + setCurrentProvider: jest.fn(), + getCurrentAccountIDs: jest.fn().mockReturnValue([]), + setCurrentAccountIDs: jest.fn(), + subscribeProvider: jest.fn().mockReturnValue(() => {}), + subscribeAccount: jest.fn().mockReturnValue(() => {}), + getCurrentUser: jest.fn(), + getPlansColumnFilters: jest.fn().mockReturnValue({}), + setPlansColumnFilter: jest.fn(), + clearAllPlansColumnFilters: jest.fn(), +})); + +jest.mock('../history', () => ({ viewPlanHistory: jest.fn() })); + +import * as api from '../api'; +import * as state from '../state'; + +const USER_ID = 'user-1'; + +const plan = { + id: 'plan-1', + name: 'Sample 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, current_step: 1, total_steps: 1 }, +}; + +const purchase = (id: string, scheduled_date: string, status: string) => ({ + id, + plan_id: 'plan-1', + plan_name: 'Sample Plan', + scheduled_date, + provider: 'aws', + service: 'ec2', + resource_type: 't3.medium', + region: 'us-east-1', + count: 2, + term: 1, + payment: 'all-upfront', + upfront_cost: 100, + estimated_savings: 50, + step_number: 1, + total_steps: 1, + status, + created_by_user_id: USER_ID, + has_recommendations: true, +}); + +const setUser = (canRun: boolean) => { + const effectivePermissions = [{ action: 'update', resource: 'plans' }]; + if (canRun) effectivePermissions.push({ action: 'execute', resource: 'purchases' }, { action: 'update', resource: 'purchases' }); + (state.getCurrentUser as jest.Mock).mockReturnValue({ id: USER_ID, email: 'u@example.com', groups: [], effectivePermissions }); +}; + +// Rows are located through their action buttons' data-id (run perms needed). +const row = (id: string): HTMLTableRowElement => { + const tr = document.querySelector(`#planned-purchases-list [data-id="${id}"]`)?.closest('tr'); + if (!tr) throw new Error(`row ${id} not found`); + return tr; +}; + +describe('Planned Purchases: overdue pending rows (#181)', () => { + beforeEach(() => { + jest.clearAllMocks(); + const list = document.createElement('div'); + list.id = 'plans-list'; + const planned = document.createElement('div'); + planned.id = 'planned-purchases-list'; + document.body.replaceChildren(list, planned); + (api.getPlans as jest.Mock).mockResolvedValue({ plans: [plan] }); + }); + + test('a pending row past its date gets an Overdue badge and a next action', async () => { + setUser(true); + (api.getPlannedPurchases as jest.Mock).mockResolvedValue({ + purchases: [purchase('pp-late', '2000-01-03', 'pending'), purchase('pp-future', '2999-01-03', 'pending')], + }); + await loadPlans(); + const late = row('pp-late'); + expect(late.querySelector('.badge-danger')?.textContent).toBe('Overdue'); + expect(late.querySelector('.overdue-hint')?.textContent).toBe('Not run on its date. Run now or pause.'); + const future = row('pp-future'); + expect(future.querySelector('.badge-danger')).toBeNull(); + expect(future.querySelector('.overdue-hint')).toBeNull(); + }); + + test('without run permission the hint does not offer an action the user cannot take', async () => { + setUser(false); + (api.getPlannedPurchases as jest.Mock).mockResolvedValue({ purchases: [purchase('pp-late', '2000-01-03', 'notified')] }); + await loadPlans(); + const hint = document.querySelector('#planned-purchases-list .overdue-hint'); + expect(hint?.textContent).toBe('Not run on its date.'); + }); + + test('a paused row past its date is not flagged (pausing is the user\'s choice)', async () => { + setUser(true); + (api.getPlannedPurchases as jest.Mock).mockResolvedValue({ purchases: [purchase('pp-paused', '2000-01-03', 'paused')] }); + await loadPlans(); + expect(document.querySelector('#planned-purchases-list .badge-danger')).toBeNull(); + }); +}); diff --git a/frontend/src/__tests__/purchases-placeholders-181.test.ts b/frontend/src/__tests__/purchases-placeholders-181.test.ts new file mode 100644 index 000000000..bd8718f9b --- /dev/null +++ b/frontend/src/__tests__/purchases-placeholders-181.test.ts @@ -0,0 +1,169 @@ +/** + * Issue #181: Purchases (Approval queue / Purchase History) must never + * render impossible values. An execution with no recommendations comes back + * as count 0, term 0, "0 commitment(s)", provider/region "multiple" and $0 + * costs; a mixed-term basket comes back as term 0. Before the fix these + * rendered as "0 Years", "0", "0 commitment(s)", "Multiple" and "$0.00". + * + * Drives loadHistory() with the real utils formatters. + */ + +import { loadHistory } from '../history'; +import { hasNoCommitments, isRealTerm, isPastScheduledDate } from '../lib/purchase-row-guards'; + +jest.mock('../api', () => ({ + getHistory: jest.fn(), + getConfig: jest.fn().mockResolvedValue({ global: {} }), + approvePurchase: jest.fn(), + cancelPurchase: jest.fn(), + getPurchaseDetails: jest.fn().mockResolvedValue({ + total_upfront_cost: 100, + estimated_savings: 10, + recommendations: [{ provider: 'aws', service: 'ec2', region: 'us-east-1', count: 1, term: 1, upfront_cost: 100, savings: 10 }], + }), + listAccounts: jest.fn().mockResolvedValue([]), + getDeploymentInfo: jest.fn().mockResolvedValue({}), +})); + +jest.mock('../navigation', () => ({ + switchTab: jest.fn(), +})); + +jest.mock('../confirmDialog', () => ({ + confirmDialog: jest.fn(), +})); + +jest.mock('../toast', () => ({ + showToast: jest.fn(), +})); + +jest.mock('../state', () => ({ + getCurrentUser: jest.fn(), + getCurrentProvider: jest.fn().mockReturnValue(''), + setCurrentProvider: jest.fn(), + getCurrentAccountIDs: jest.fn().mockReturnValue([]), + setCurrentAccountIDs: jest.fn(), + subscribeProvider: jest.fn().mockReturnValue(() => {}), + subscribeAccount: jest.fn().mockReturnValue(() => {}), + getAmortizeUpfront: jest.fn().mockReturnValue(false), + setAmortizeUpfront: jest.fn(), + subscribeAmortizeUpfront: jest.fn().mockReturnValue(() => {}), + getPurchaseHistoryColumnFilters: jest.fn().mockReturnValue({}), + setPurchaseHistoryColumnFilter: jest.fn(), + clearAllPurchaseHistoryColumnFilters: jest.fn(), + getApprovalQueueColumnFilters: jest.fn().mockReturnValue({}), + setApprovalQueueColumnFilter: jest.fn(), + clearAllApprovalQueueColumnFilters: jest.fn(), +})); + +jest.mock('../recommendations', () => ({ + getAccountName: jest.fn((id: string) => id), +})); + +import * as api from '../api'; +import { getCurrentUser } from '../state'; +import { ADMINISTRATORS_GROUP_ID } from '../permissions'; + +const ADMIN_USER = { id: 'admin-uuid', email: 'admin@example.com', groups: [ADMINISTRATORS_GROUP_ID] }; + +function setupDOM(): void { + document.body.innerHTML = ` + +
+ `; +} + +// The shape internal/api/handler_history.go produces for a pending +// execution that carries no recommendations. +const EMPTY_PENDING = { + purchase_id: 'exec-empty', + timestamp: '2026-09-01T00:00:00Z', + provider: 'multiple', + service: 'multiple', + resource_type: '0 commitment(s)', + region: 'multiple', + count: 0, + term: 0, + payment: '', + upfront_cost: 0, + monthly_cost: 0, + estimated_savings: 0, + plan_name: 'Full Coverage Purchase', + status: 'pending', + created_by_user_id: ADMIN_USER.id, +}; + +// A real two-commitment basket whose terms differ: term collapses to 0. +const MIXED_TERM_COMPLETED = { + ...EMPTY_PENDING, + purchase_id: 'exec-mixed', + provider: 'aws', + service: 'ec2', + resource_type: '2 commitment(s)', + count: 2, + upfront_cost: 1200, + monthly_cost: 40, + estimated_savings: 90, + status: 'completed', +}; + +const cells = (root: string, id: string): string[] => + Array.from(document.querySelectorAll(`${root} tr[data-execution-id="${id}"] td`)).map(td => (td.textContent || '').trim()); + +describe('purchase-row-guards helpers (#181)', () => { + test('hasNoCommitments is true only for a zero count', () => { + expect(hasNoCommitments({ count: 0 })).toBe(true); + expect(hasNoCommitments({ count: 1 })).toBe(false); + }); + + test('isRealTerm rejects 0, negatives, NaN and nullish', () => { + expect(isRealTerm(1)).toBe(true); + expect(isRealTerm(3)).toBe(true); + for (const t of [0, -1, NaN, Infinity, null, undefined]) expect(isRealTerm(t)).toBe(false); + }); + + test('isPastScheduledDate compares calendar days, not instants', () => { + const now = new Date(2026, 9, 23, 0, 30); // local 2026-10-23 00:30 + expect(isPastScheduledDate('2026-10-22', now)).toBe(true); + expect(isPastScheduledDate('2026-10-22T23:59:59Z', now)).toBe(true); + expect(isPastScheduledDate('2026-10-23', now)).toBe(false); + expect(isPastScheduledDate('2026-11-01', now)).toBe(false); + expect(isPastScheduledDate('', now)).toBe(false); + expect(isPastScheduledDate(undefined, now)).toBe(false); + expect(isPastScheduledDate('not-a-date', now)).toBe(false); + }); +}); + +describe('Purchases tables never render impossible values (#181)', () => { + beforeEach(() => { + setupDOM(); + jest.clearAllMocks(); + (getCurrentUser as jest.Mock).mockReturnValue(ADMIN_USER); + (api.getHistory as jest.Mock).mockResolvedValue({ summary: {}, purchases: [EMPTY_PENDING, MIXED_TERM_COMPLETED] }); + }); + + test('history row with no commitments shows dashes and says why', async () => { + await loadHistory(); + const row = cells('#history-list', 'exec-empty'); + // Status, Date, Provider, Service, Type, Region, Count, Term, Upfront, Monthly, Savings, Plan + expect(row.slice(2, 11)).toEqual(['-', '-', 'No recommendations attached', '-', '-', '-', '-', '-', '-']); + const text = row.join(' '); + expect(text).not.toMatch(/0 Years|0 commitment\(s\)|Multiple|\$0/); + }); + + test('approval queue row with no commitments shows dashes and says why', async () => { + await loadHistory(); + const row = cells('#purchases-approval-queue', 'exec-empty'); + // Date, Account, Provider, Service, Count, Term, Payment, Monthly, Upfront, Savings, Created by, Actions + expect(row.slice(2, 10)).toEqual(['-', 'No recommendations attached', '-', '-', '-', '-', '-', '-']); + }); + + test('a real mixed-term basket keeps its values but shows a dash, not "0 Years", for the term', async () => { + await loadHistory(); + const row = cells('#history-list', 'exec-mixed'); + expect(row[6]).toBe('2'); + expect(row[7]).toBe('-'); + expect(row[8]).toBe('$1,200'); + expect(row.join(' ')).not.toContain('0 Years'); + }); +}); diff --git a/frontend/src/history.ts b/frontend/src/history.ts index dede41d37..f68d7062d 100644 --- a/frontend/src/history.ts +++ b/frontend/src/history.ts @@ -17,6 +17,7 @@ import { canAccess } from './permissions'; import { showSkeletonRows, teardownSkeleton } from './lib/skeleton'; import { getAccountName } from './recommendations'; import { applyColumnFilters } from './lib/column-filters'; +import { hasNoCommitments, isRealTerm, NO_RECOMMENDATIONS_LABEL, NO_RECOMMENDATIONS_TITLE } from './lib/purchase-row-guards'; import { openHistoryColumnPopover, renderHistoryFilterButton, @@ -558,7 +559,18 @@ function buildStatusChipRowHTML(purchases: HistoryPurchase[], active: StatusFilt `; } +const MUTED_DASH = '-'; +const NO_RECS_HTML = `${NO_RECOMMENDATIONS_LABEL}`; + +// #181: a row with no commitments has placeholder count/term/costs (0, "0 +// Years", $0) and a "0 commitment(s)" type; render dashes and say why +// instead. A 0 term on a real row (a mixed-term basket) is also a dash. +function termCell(p: HistoryPurchase): string { + return isRealTerm(p.term) ? escapeHtml(formatTerm(p.term)) : MUTED_DASH; +} + function providerCell(p: HistoryPurchase): string { + if (hasNoCommitments(p)) return MUTED_DASH; if (!p.provider || p.provider === 'multiple') return 'Multiple'; // Whitelist provider to prevent stored XSS via class attribute injection (#443). const safeProvider = (VALID_PROVIDERS as string[]).includes(p.provider) ? p.provider : 'unknown'; @@ -1129,8 +1141,9 @@ function purchaseHistoryDisplayLabel( ): string { if (value === '') return '(empty)'; if (column === 'term') { + // Match the cell: a 0 term renders as a dash, never "0 Years" (#181). const n = Number(value); - return Number.isFinite(n) ? formatTerm(n) : value; + return isRealTerm(n) ? formatTerm(n) : Number.isFinite(n) ? '-' : value; } return value; } @@ -1268,19 +1281,21 @@ function renderHistoryList(purchases: HistoryPurchase[]): void { const monthlyCostCell = displayMonthly != null ? formatCurrency(displayMonthly) : '-'; + const empty = hasNoCommitments(p); + const dashOr = (html: string): string => (empty ? MUTED_DASH : html); return ` ${statusCell} ${formatDate(p.timestamp)} ${providerCell(p)} - ${escapeHtml(p.service)} - ${escapeHtml(p.resource_type)} - ${escapeHtml(p.region)} - ${escapeHtml(String(p.count ?? ''))} - ${formatTerm(p.term)} - ${upfrontCell(p, amortize)} - ${monthlyCostCell} - ${formatCurrency(p.estimated_savings)} + ${dashOr(escapeHtml(p.service))} + ${empty ? NO_RECS_HTML : escapeHtml(p.resource_type)} + ${dashOr(escapeHtml(p.region))} + ${dashOr(escapeHtml(String(p.count ?? '')))} + ${termCell(p)} + ${empty ? `${MUTED_DASH}` : upfrontCell(p, amortize)} + ${dashOr(monthlyCostCell)} + ${dashOr(formatCurrency(p.estimated_savings))} ${planCellContent} `; @@ -1813,8 +1828,9 @@ function approvalQueueDisplayLabel( ): string { if (value === '') return '(empty)'; if (column === 'term') { + // Match the cell: a 0 term renders as a dash, never "0 Years" (#181). const n = Number(value); - return Number.isFinite(n) ? formatTerm(n) : value; + return isRealTerm(n) ? formatTerm(n) : Number.isFinite(n) ? '-' : value; } if (column === 'account') { // Account cells render via getAccountName() — mirror the same display. @@ -1889,7 +1905,8 @@ export function renderApprovalQueue(purchases: HistoryPurchase[]): void { const accountCell = p.account_id ? escapeHtml(getAccountName(p.account_id)) : '-'; - const termCell = p.term ? escapeHtml(formatTerm(p.term)) : '-'; + const empty = hasNoCommitments(p); + const dashOr = (html: string): string => (empty ? MUTED_DASH : html); const paymentCell = p.payment ? escapeHtml(p.payment) : '-'; const rawMonthly = p.monthly_cost != null ? p.monthly_cost : null; const displayMonthly = (rawMonthly != null && amortize) @@ -1904,13 +1921,13 @@ export function renderApprovalQueue(purchases: HistoryPurchase[]): void { ${formatDate(p.timestamp)} ${accountCell} ${providerCell(p)} - ${escapeHtml(p.service)} - ${escapeHtml(String(p.count ?? ''))} - ${termCell} - ${paymentCell} - ${monthlyCostCell} - ${upfrontCell(p, amortize)} - ${formatCurrency(p.estimated_savings)} + ${empty ? NO_RECS_HTML : escapeHtml(p.service)} + ${dashOr(escapeHtml(String(p.count ?? '')))} + ${termCell(p)} + ${dashOr(paymentCell)} + ${dashOr(monthlyCostCell)} + ${empty ? `${MUTED_DASH}` : upfrontCell(p, amortize)} + ${dashOr(formatCurrency(p.estimated_savings))} ${createdBy} ${actionsCell} diff --git a/frontend/src/lib/purchase-row-guards.ts b/frontend/src/lib/purchase-row-guards.ts new file mode 100644 index 000000000..fbc90fc66 --- /dev/null +++ b/frontend/src/lib/purchase-row-guards.ts @@ -0,0 +1,46 @@ +/** + * Guards against rendering impossible values on scheduled and pending + * purchase rows (issue #181): a "0 Years" term, a zero count, "0 + * commitment(s)" and all-$0 costs for an execution that carries no + * recommendations, and an overdue scheduled purchase that still reads as a + * plain "pending". + */ + +/** Visible label for a row with nothing to buy. Matches the Plans copy (#609). */ +export const NO_RECOMMENDATIONS_LABEL = 'No recommendations attached'; +export const NO_RECOMMENDATIONS_TITLE = + 'This purchase has no recommendations attached, so it cannot buy anything.'; + +/** + * True for a purchase-history / approval-queue row that carries no + * commitments. The API sets count to the number of recommendations on a + * synthetic execution row (internal/api/handler_history.go), so 0 means the + * row's type, term and costs are placeholders, not data. + */ +export function hasNoCommitments(row: { count: number }): boolean { + return row.count === 0; +} + +/** A term in years that can actually be bought (1, 3, ...). */ +export function isRealTerm(term: number | null | undefined): term is number { + return typeof term === 'number' && Number.isFinite(term) && term > 0; +} + +/** Local calendar date as YYYY-MM-DD. */ +function localISODate(d: Date): string { + const mm = String(d.getMonth() + 1).padStart(2, '0'); + const dd = String(d.getDate()).padStart(2, '0'); + return `${d.getFullYear()}-${mm}-${dd}`; +} + +/** + * True when a YYYY-MM-DD (or ISO timestamp) scheduled date is strictly + * before today in the user's calendar. Compares the date part as text so a + * date-only string is not shifted a day by UTC parsing. + */ +export function isPastScheduledDate(scheduled: string | null | undefined, now: Date = new Date()): boolean { + if (!scheduled) return false; + const day = scheduled.slice(0, 10); + if (!/^\d{4}-\d{2}-\d{2}$/.test(day)) return false; + return day < localISODate(now); +} diff --git a/frontend/src/plans.ts b/frontend/src/plans.ts index 294e60521..bf62c7a39 100644 --- a/frontend/src/plans.ts +++ b/frontend/src/plans.ts @@ -22,6 +22,7 @@ import { sortedDistinctValues, } from './lib/column-filter-popover'; import { suggestedPurchaseStartDate } from './lib/calendar-date'; +import { isPastScheduledDate } from './lib/purchase-row-guards'; import { mountPlanEditorPanel } from './plan-editor-panel'; // pendingPlanRecommendations holds the resolved plan target captured at @@ -434,6 +435,15 @@ function renderPlannedPurchaseRow(purchase: PlannedPurchase): string { canAccess('cancel-own', 'purchases') ); + // #181: a pending step whose date has passed did not run; "pending" alone + // reads as on schedule. Flag it and name the next action the user can take. + const overdue = isPending && isPastScheduledDate(purchase.scheduled_date); + const overdueHint = canRunPurchase ? 'Not run on its date. Run now or pause.' : 'Not run on its date.'; + const overdueBadge = overdue + ? 'Overdue' + : ''; + const overdueHintHtml = overdue ? `
${overdueHint}
` : ''; + return ` @@ -448,7 +458,7 @@ function renderPlannedPurchaseRow(purchase: PlannedPurchase): string { ${termCell} ${upfrontShown} ${savingsCell} - ${escapeHtml(purchase.status)} + ${escapeHtml(purchase.status)}${overdueBadge}${overdueHintHtml} ${canRunPurchase ? `` : ''} ${canPauseOrResumePurchase && isPending ? `` : ''} diff --git a/frontend/src/styles/tables.css b/frontend/src/styles/tables.css index 305c05cb0..03edd832e 100644 --- a/frontend/src/styles/tables.css +++ b/frontend/src/styles/tables.css @@ -211,3 +211,17 @@ tr.history-row-highlight:hover { color: var(--cudly-text-subtle); font-size: var(--cudly-fs-xs); } + +/* Status + Overdue badges side by side, and the next-action hint under an + * overdue planned purchase's status (#181). */ +.planned-purchase-row .status-badges { + display: inline-flex; + gap: 0.25rem; + white-space: nowrap; +} + +.planned-purchase-row .overdue-hint { + margin-top: 0.25rem; + font-size: 0.8rem; + color: var(--cudly-text-muted); +}