From eb70c29369acc2cfa334d9a34407f95b9d4fd19b Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 28 May 2026 00:35:03 +0200 Subject: [PATCH 1/2] fix(planned-purchases): Edit on scheduled purchase loads plan details The Edit button on a scheduled-purchase row surfaced "Failed to load plan details" because handlePlannedPurchaseAction passed the purchase's own id to editPlan, which then called GET /plans/ and got a 404. Fixed by adding a data-plan-id attribute to the Edit button and passing it as a third argument through the handler to editPlan, so the correct GET /plans/ call is made. Closes #773. --- frontend/src/__tests__/plans.test.ts | 30 ++++++++++++++++++++++++++++ frontend/src/plans.ts | 12 ++++++----- 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/frontend/src/__tests__/plans.test.ts b/frontend/src/__tests__/plans.test.ts index 333805fbe..2898a9267 100644 --- a/frontend/src/__tests__/plans.test.ts +++ b/frontend/src/__tests__/plans.test.ts @@ -838,6 +838,36 @@ describe('Plans Module', () => { expect(mockShowToast).toHaveBeenCalledWith(expect.objectContaining({ message: 'Failed to pause purchase: API Error' })); }); + + test('edit action calls getPlan with plan_id, not the purchase id (#773)', async () => { + // The purchase row has id="purchase-1" and plan_id="plan-1". + // Before the fix, editPlan received "purchase-1", causing GET /plans/purchase-1 + // to return 404 and surfacing "Failed to load plan details". + (api.getPlan as jest.Mock).mockResolvedValue({ + id: 'plan-1', + name: 'Test Plan', + enabled: true, + auto_purchase: false, + notification_days_before: 3, + 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 }, + }); + + const editBtn = document.querySelector('[data-action="edit"]') as HTMLButtonElement; + editBtn?.click(); + + await new Promise(resolve => setTimeout(resolve, 50)); + + // Must use the plan FK, not the purchase PK. + expect(api.getPlan).toHaveBeenCalledWith('plan-1'); + expect(api.getPlan).not.toHaveBeenCalledWith('purchase-1'); + // No error toast should fire. + expect(mockShowToast).not.toHaveBeenCalledWith( + expect.objectContaining({ message: 'Failed to load plan details' }), + ); + }); }); describe('resume action for paused purchase', () => { diff --git a/frontend/src/plans.ts b/frontend/src/plans.ts index 39a9cb395..2b3cff48d 100644 --- a/frontend/src/plans.ts +++ b/frontend/src/plans.ts @@ -148,7 +148,8 @@ function renderPlannedPurchases(purchases: PlannedPurchase[]): void { container.querySelectorAll('[data-action]').forEach(btn => { btn.addEventListener('click', () => void handlePlannedPurchaseAction( btn.dataset['action'] || '', - btn.dataset['id'] || '' + btn.dataset['id'] || '', + btn.dataset['planId'] || '' )); }); } @@ -197,7 +198,7 @@ function renderPlannedPurchaseRow(purchase: PlannedPurchase): string { ${canManagePlan && canRun ? `` : ''} ${canManagePlan && isPending ? `` : ''} ${canManagePlan && isPaused ? `` : ''} - ${canManagePlan ? `` : ''} + ${canManagePlan ? `` : ''} ${canDisablePlan ? `` : ''} @@ -221,7 +222,7 @@ function getPlannedPurchaseStatusClass(status: string): string { /** * Handle planned purchase action */ -async function handlePlannedPurchaseAction(action: string, purchaseId: string): Promise { +async function handlePlannedPurchaseAction(action: string, purchaseId: string, planId = ''): Promise { try { switch (action) { case 'run': @@ -237,8 +238,9 @@ async function handlePlannedPurchaseAction(action: string, purchaseId: string): await api.resumePlannedPurchase(purchaseId); break; case 'edit': - // Open edit modal for the plan - await editPlan(purchaseId); + // Open edit modal for the parent plan using plan_id, not the purchase id. + // The purchase row's data-plan-id attribute carries the plan FK (#773). + await editPlan(planId); return; case 'disable': if (confirm('Disable this plan? The plan will be paused and no purchases will be scheduled. You can re-enable it later from the Plans list.')) { From 4a44f1e73c7219fbed52e0fcbd5a6d15435691d4 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 28 May 2026 14:43:12 +0200 Subject: [PATCH 2/2] fix(plans): defensive guard against empty plan id in edit action Add an early-return guard in the edit case of handlePlannedPurchaseAction so that a missing or empty data-plan-id attribute produces a console.warn and a no-op instead of forwarding an empty string to editPlan / the API. Covers the defensive path with a focused unit test. --- frontend/src/__tests__/plans.test.ts | 17 +++++++++++++++++ frontend/src/plans.ts | 4 ++++ 2 files changed, 21 insertions(+) diff --git a/frontend/src/__tests__/plans.test.ts b/frontend/src/__tests__/plans.test.ts index 2898a9267..ec2cfe64a 100644 --- a/frontend/src/__tests__/plans.test.ts +++ b/frontend/src/__tests__/plans.test.ts @@ -868,6 +868,23 @@ describe('Plans Module', () => { expect.objectContaining({ message: 'Failed to load plan details' }), ); }); + + test('edit action with empty plan id is a no-op (defensive guard)', async () => { + // Simulate a button whose data-plan-id attribute is missing/empty by + // directly injecting a button without the attribute and clicking it. + const container = document.getElementById('planned-purchases-list'); + const btn = document.createElement('button'); + btn.dataset.action = 'edit'; + btn.dataset.id = 'purchase-1'; + // intentionally omit data-plan-id so planId defaults to '' + container?.appendChild(btn); + btn.click(); + + await new Promise(resolve => setTimeout(resolve, 50)); + + // getPlan must NOT be called when planId is empty. + expect(api.getPlan).not.toHaveBeenCalled(); + }); }); describe('resume action for paused purchase', () => { diff --git a/frontend/src/plans.ts b/frontend/src/plans.ts index 2b3cff48d..b3572038c 100644 --- a/frontend/src/plans.ts +++ b/frontend/src/plans.ts @@ -240,6 +240,10 @@ async function handlePlannedPurchaseAction(action: string, purchaseId: string, p case 'edit': // Open edit modal for the parent plan using plan_id, not the purchase id. // The purchase row's data-plan-id attribute carries the plan FK (#773). + if (!planId) { + console.warn('edit action ignored: missing plan id'); + return; + } await editPlan(planId); return; case 'disable':