From 9886997a7fb735c34aa5dd443bfbfcfbba3549ea Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 28 Sep 2026 21:37:21 +0200 Subject: [PATCH] fix(frontend): discard stale purchase modal opens so Execute submits what is shown openPurchaseModal did not clear fan-out buckets and openFanOutModal had no staleness check after its override fetch, so interleaved opens could render the single-row modal while Execute submitted fan-out buckets, or let an older open render over a newer one. Both openers now take a shared open token and drop their result when a newer open started; openPurchaseModal also clears fan-out state up front. Closes #430 --- .../__tests__/purchase-modal-submit.test.ts | 81 +++++++++++++++++++ frontend/src/recommendations.ts | 10 ++- 2 files changed, 90 insertions(+), 1 deletion(-) diff --git a/frontend/src/__tests__/purchase-modal-submit.test.ts b/frontend/src/__tests__/purchase-modal-submit.test.ts index b7a688179..4aa76c955 100644 --- a/frontend/src/__tests__/purchase-modal-submit.test.ts +++ b/frontend/src/__tests__/purchase-modal-submit.test.ts @@ -1653,6 +1653,87 @@ describe('Issue #331: Escape discards purchase state like the explicit close but }); }); +// Issue #430: openPurchaseModal and openFanOutModal both await an override +// fetch before rendering. Without a shared open token, an older open that +// resolved last could render over the newer one, or the single-row modal +// could render while fan-out buckets from the other open were still set, so +// Execute submitted buckets the dialog did not show. +describe('Issue #430: interleaved purchase modal opens submit what the newest open displays', () => { + type Opener = 'single' | 'fanout'; + const cases: Array<[Opener[], Opener[]]> = [ + [['fanout', 'single'], ['fanout', 'single']], + [['fanout', 'single'], ['single', 'fanout']], + [['single', 'fanout'], ['single', 'fanout']], + [['single', 'fanout'], ['fanout', 'single']], + ]; + + test.each(cases)('opens %j resolving %j', async (starts, resolves) => { + const fanOutRows: LocalRecommendation[] = [ + { + id: 'fo-ec2', provider: 'aws', cloud_account_id: 'fo-acct', service: 'ec2', + region: 'us-east-1', resource_type: 'm5.large', term: 1, payment: 'no-upfront', + count: 1, upfront_cost: 0, monthly_cost: 100, savings: 50, + }, + { + id: 'fo-rds', provider: 'aws', cloud_account_id: 'fo-acct', service: 'rds', + region: 'us-east-1', resource_type: 'db.r5.large', term: 3, payment: 'all-upfront', + count: 1, upfront_cost: 1000, monthly_cost: 0, savings: 200, + }, + ]; + const singleRec: LocalRecommendation = { + id: 'single-only', provider: 'aws', cloud_account_id: 'single-acct', service: 'ec2', + region: 'us-west-2', resource_type: 'm6i.large', term: 3, payment: 'all-upfront', + count: 1, upfront_cost: 4000, monthly_cost: 0, savings: 300, + }; + (api.getConfig as jest.Mock).mockResolvedValue({ global: { default_payment: 'no-upfront' } }); + (api.getRecommendations as jest.Mock).mockResolvedValue({ + summary: {}, recommendations: fanOutRows, regions: [], + }); + (state.getRecommendations as jest.Mock).mockReturnValue([...fanOutRows, singleRec]); + (state.getVisibleRecommendations as jest.Mock).mockReturnValue(fanOutRows); + (state.getSelectedRecommendationIDs as jest.Mock).mockReturnValue(new Set(['fo-ec2', 'fo-rds'])); + await loadRecommendations(); + + const fetches: Record>>>> = { + single: deferred(), + fanout: deferred(), + }; + (api.listAccountServiceOverrides as jest.Mock).mockImplementation((id: string) => + (id === 'single-acct' ? fetches.single : fetches.fanout).promise); + + const pending: Array> = []; + for (const opener of starts) { + if (opener === 'single') { + pending.push(openPurchaseModal([singleRec])); + } else { + (document.getElementById('bulk-purchase-btn') as HTMLButtonElement).click(); + await flush(); + } + } + // Both opens must be parked on their fetch before either resolves, or + // the interleaving under test never happens. + expect(api.listAccountServiceOverrides).toHaveBeenCalledWith('single-acct'); + expect(api.listAccountServiceOverrides).toHaveBeenCalledWith('fo-acct'); + + for (const opener of resolves) { + fetches[opener].resolve([]); + await flush(); + } + await Promise.all(pending); + + const newest = starts[starts.length - 1]!; + expect(document.getElementById('fanout-summary') !== null).toBe(newest === 'fanout'); + + (document.getElementById('execute-purchase-btn') as HTMLButtonElement).click(); + await flush(); + + const submittedIDs = (api.executePurchase as jest.Mock).mock.calls + .flatMap(([payload]) => (payload as Array<{ id: string }>).map((rec) => rec.id)) + .sort(); + expect(submittedIDs).toEqual(newest === 'fanout' ? ['fo-ec2', 'fo-rds'] : ['single-only']); + }); +}); + // Issue #333: the frontend grouped bulk purchases without account identity, // so two recommendations sharing (provider, service, term, payment) but // belonging to different cloud accounts landed in ONE bucket and were sent diff --git a/frontend/src/recommendations.ts b/frontend/src/recommendations.ts index 7b1d9fffb..9424f4eb2 100644 --- a/frontend/src/recommendations.ts +++ b/frontend/src/recommendations.ts @@ -54,6 +54,10 @@ let checkedPurchaseIndices: Set = new Set(); // checkedPurchaseIndices. Used by getPurchaseModalRecommendations to // distinguish "all deselected by user" from "modal never opened". let checkedPurchaseModalInitialised = false; +// Bumped by every openPurchaseModal/openFanOutModal call. Each opener checks +// it after its override fetch so only the newest open renders and sets +// submit state (issue #430). +let purchaseModalOpenToken = 0; // Cache of account ID → name for column display let accountNamesCache: Map = new Map(); @@ -4362,6 +4366,7 @@ async function openFanOutModal( bucketEntries: Array<[string, LocalRecommendation[]]>, toolbar: BulkPurchaseToolbarState, ): Promise { + const openToken = ++purchaseModalOpenToken; // Pre-fetch service-overrides for every distinct account referenced by // any rec in any bucket; overridesByAccount seeds the bucket-level // payment (issue #111). One fetch per distinct accountID; cached for the lifetime of this @@ -4374,6 +4379,7 @@ async function openFanOutModal( } } const overridesByAccount = await fetchOverridesForAccounts(allAccountIDs); + if (openToken !== purchaseModalOpenToken) return; const buckets: FanOutBucket[] = bucketEntries .filter(([_key, recs]) => recs.length > 0) @@ -5111,6 +5117,8 @@ function renderDirectExecuteWarning(): void { * payments and loaded priced alternatives. */ export async function openPurchaseModal(recommendations: LocalRecommendation[], capacityPercent = 100): Promise { + const openToken = ++purchaseModalOpenToken; + clearFanOutBuckets(); currentPurchaseCapacityPercent = capacityPercent; currentPurchaseRecommendations = []; const pendingRows = currentPurchaseRecommendations; @@ -5129,7 +5137,7 @@ export async function openPurchaseModal(recommendations: LocalRecommendation[], } const overridesByAccount = await fetchOverridesForAccounts(accountIDs); - if (currentPurchaseRecommendations !== pendingRows) return; + if (openToken !== purchaseModalOpenToken || currentPurchaseRecommendations !== pendingRows) return; const seeds = recommendations.map((r) => resolvePerRecPaymentSeed(r, overridesByAccount, capacityPercent)); const resolvedSeeds = seeds.filter((seed) => seed !== null);