diff --git a/frontend/src/__tests__/plans.test.ts b/frontend/src/__tests__/plans.test.ts index 333805fbe..afe3ebda3 100644 --- a/frontend/src/__tests__/plans.test.ts +++ b/frontend/src/__tests__/plans.test.ts @@ -19,7 +19,8 @@ jest.mock('../api', () => ({ createPlannedPurchases: jest.fn(), listPlanAccounts: jest.fn().mockResolvedValue([]), setPlanAccounts: jest.fn().mockResolvedValue(undefined), - listAccounts: jest.fn().mockResolvedValue([]) + listAccounts: jest.fn().mockResolvedValue([]), + getAccount: jest.fn().mockResolvedValue(null) })); // Mock state module @@ -1264,6 +1265,139 @@ describe('Plans Module', () => { expect((document.getElementById('plan-id') as HTMLInputElement).value).toBe(''); }); + + // #770: Purchase Configuration section should be prefilled from the + // selected commitment when exactly one commitment is passed in the snapshot. + describe('prefill from single selected commitment (#770)', () => { + const fixture: api.Recommendation = { + id: 'rec-770', + provider: 'aws', + service: 'ec2', + region: 'us-east-1', + resource_type: 't3.medium', + count: 1, + term: 1, + payment: 'partial-upfront', + upfront_cost: 100, + monthly_cost: 20, + savings: 15, + selected: true, + purchased: false, + cloud_account_id: 'acct-uuid-123', + }; + + test('prefills provider and service selects', () => { + openCreatePlanModal([fixture]); + + expect((document.getElementById('plan-provider') as HTMLSelectElement).value).toBe('aws'); + expect((document.getElementById('plan-service') as HTMLSelectElement).value).toBe('ec2'); + }); + + test('prefills term and payment selects', () => { + openCreatePlanModal([fixture]); + + expect((document.getElementById('plan-term') as HTMLSelectElement).value).toBe('1'); + expect((document.getElementById('plan-payment') as HTMLSelectElement).value).toBe('partial-upfront'); + }); + + test('calls populateTermSelect and populatePaymentSelect with provider+service', () => { + openCreatePlanModal([fixture]); + + expect(populateTermSelect).toHaveBeenCalledWith( + expect.any(HTMLSelectElement), 'aws', 'ec2' + ); + expect(populatePaymentSelect).toHaveBeenCalledWith( + expect.any(HTMLSelectElement), 'aws', 'ec2' + ); + }); + + test('fetches account by cloud_account_id to prefill chip', async () => { + (api.getAccount as jest.Mock).mockResolvedValueOnce({ + id: 'acct-uuid-123', + name: 'Prod AWS', + external_id: '123456789012', + }); + + openCreatePlanModal([fixture]); + + // Wait for the async prefillAccountChipFromId to settle + await Promise.resolve(); + await Promise.resolve(); + + expect(api.getAccount).toHaveBeenCalledWith('acct-uuid-123'); + const chips = document.getElementById('plan-accounts-selected'); + expect(chips?.textContent).toContain('Prod AWS'); + const hiddenIds = (document.getElementById('plan-account-ids') as HTMLInputElement).value; + expect(hiddenIds).toContain('acct-uuid-123'); + }); + + test('does not throw and leaves account section empty when getAccount fails', async () => { + (api.getAccount as jest.Mock).mockRejectedValueOnce(new Error('network error')); + + openCreatePlanModal([fixture]); + + await Promise.resolve(); + await Promise.resolve(); + + // Account section should be empty -- no chip added + const hiddenIds = (document.getElementById('plan-account-ids') as HTMLInputElement).value; + expect(hiddenIds).toBe(''); + }); + + test('skips prefill when no cloud_account_id present', async () => { + const noAccount: api.Recommendation = { ...fixture, cloud_account_id: undefined }; + openCreatePlanModal([noAccount]); + + await Promise.resolve(); + + expect(api.getAccount).not.toHaveBeenCalled(); + }); + + test('discards stale getAccount result when modal is reopened before promise resolves', async () => { + // Simulate a slow first getAccount call that resolves after the modal + // is closed and a new modal session starts (the race condition fixed + // by the planModalSession guard — #770 CR Major). + let resolveFirstCall!: (value: api.CloudAccount) => void; + const firstCallPromise = new Promise(resolve => { + resolveFirstCall = resolve; + }); + + (api.getAccount as jest.Mock) + .mockReturnValueOnce(firstCallPromise) // first open: hangs + .mockResolvedValueOnce(null); // second open: no account + + // First modal open with cloud_account_id + openCreatePlanModal([fixture]); + + // Close and reopen — this increments planModalSession + closePlanModal(); + const noAccount: api.Recommendation = { ...fixture, cloud_account_id: undefined }; + openCreatePlanModal([noAccount]); + + // Now resolve the stale first promise — should be discarded + resolveFirstCall({ id: 'acct-uuid-123', name: 'Prod AWS', external_id: '123456789012' } as api.CloudAccount); + await Promise.resolve(); + await Promise.resolve(); + + // The stale chip must NOT have been added to the new modal session + const hiddenIds = (document.getElementById('plan-account-ids') as HTMLInputElement).value; + expect(hiddenIds).toBe(''); + const chips = document.getElementById('plan-accounts-selected'); + expect(chips?.textContent).not.toContain('Prod AWS'); + }); + + test('does not prefill when snapshot has more than one commitment', () => { + const second: api.Recommendation = { ...fixture, id: 'rec-771', service: 'rds' }; + openCreatePlanModal([fixture, second]); + + // populateTermSelect is called by setupRampScheduleHandlers / updateCommitmentOptions + // but NOT by prefillPurchaseConfigFromCommitment (which only runs for length===1) + // The provider/service select should NOT be forced to either rec's values + // We assert that the service select was not forced to 'ec2' alone + // (a multi-commitment plan requires manual selection) + expect(api.getAccount).not.toHaveBeenCalled(); + }); + }); }); describe('openNewPlanModal', () => { diff --git a/frontend/src/plans.ts b/frontend/src/plans.ts index 39a9cb395..fc232d5ca 100644 --- a/frontend/src/plans.ts +++ b/frontend/src/plans.ts @@ -701,6 +701,12 @@ export function closePlanModal(): void { // Selected accounts for the plan modal let planSelectedAccounts: Array<{ id: string; name: string; external_id: string }> = []; +// Monotonically incrementing counter scoped to the plan modal lifecycle. +// Incremented each time the create modal opens so that async callbacks +// from a previous session (stale promises) can detect they're out-of-date +// and discard their results rather than mutating state in the new session. +let planModalSession = 0; + /** * Render selected account chips in the plan modal */ @@ -822,6 +828,68 @@ async function setupPlanAccountsSection(planId?: string): Promise { } } +/** + * Prefill the Purchase Configuration section (provider / service / term / + * payment) from a single selected commitment. Called after form.reset() so + * the defaults are already in place; each field is still editable. (#770) + */ +function prefillPurchaseConfigFromCommitment(rec: api.Recommendation): void { + const providerSelect = document.getElementById('plan-provider') as HTMLSelectElement | null; + const serviceSelect = document.getElementById('plan-service') as HTMLSelectElement | null; + const termSelect = document.getElementById('plan-term') as HTMLSelectElement | null; + const paymentSelect = document.getElementById('plan-payment') as HTMLSelectElement | null; + + if (!providerSelect || !serviceSelect || !termSelect || !paymentSelect) return; + + const provider = rec.provider ?? ''; + const service = rec.service ?? ''; + + if (provider) providerSelect.value = provider; + if (service) serviceSelect.value = service; + + // Repopulate term/payment options for the chosen provider+service, then + // apply the commitment's own values so the dropdowns are consistent. + if (provider && service) { + populateTermSelect(termSelect, provider, service); + populatePaymentSelect(paymentSelect, provider, service); + } + + if (rec.term != null) termSelect.value = String(rec.term); + const normalizedPayment = rec.payment ? normalizePaymentValue(rec.payment, provider) : ''; + if (normalizedPayment) paymentSelect.value = normalizedPayment; +} + +/** + * Fetch the account with the given internal UUID and add it as a pre-selected + * chip in the plan modal accounts section. Runs after setupPlanAccountsSection + * has reset the chip list for the create flow. Silently no-ops on failure so + * the user can still pick the account manually. (#770) + * + * @param accountId - Internal UUID of the account to prefill. + * @param session - planModalSession value captured at call time. If the + * modal is closed and reopened before this promise resolves, + * the counter will have advanced and the stale result is + * discarded to prevent wrong-modal pollution. (#770 CR) + */ +async function prefillAccountChipFromId(accountId: string, session: number): Promise { + try { + const account = await api.getAccount(accountId); + // Session guard: discard the result if the modal was closed and reopened + // while this promise was in-flight. planModalSession is incremented on + // each new modal open, so a mismatch means this callback is stale. + if (session !== planModalSession) return; + // Guard: only add if the chip is not already present (e.g. a concurrent + // edit flow somehow set it) and the account record is usable. + if (account && account.id && !planSelectedAccounts.some(a => a.id === account.id)) { + planSelectedAccounts.push({ id: account.id, name: account.name, external_id: account.external_id }); + renderPlanAccountChips(); + updatePlanAccountIdsField(); + } + } catch { + // Non-critical: the user can still search and add the account manually. + } +} + /** * Open create plan modal with selected recommendations. * @@ -847,6 +915,13 @@ export function openCreatePlanModal(snapshot?: readonly api.Recommendation[]): v (document.getElementById('plan-id') as HTMLInputElement).value = ''; (document.getElementById('plan-form') as HTMLFormElement | null)?.reset(); + // When exactly one commitment is selected, prefill the Purchase + // Configuration fields so the user does not have to re-enter them. + // Fields are still fully editable after prefill. (#770) + if (pendingPlanRecommendations.length === 1) { + prefillPurchaseConfigFromCommitment(pendingPlanRecommendations[0]!); + } + // Set up ramp schedule change handlers for dynamic plan name setupRampScheduleHandlers(); @@ -856,7 +931,21 @@ export function openCreatePlanModal(snapshot?: readonly api.Recommendation[]): v // Generate initial plan name updatePlanNameFromSchedule(); + // Stamp a new session so any in-flight prefillAccountChipFromId promise + // from a prior modal open can detect it belongs to a stale session and + // discard its result without mutating planSelectedAccounts. (#770 CR) + planModalSession += 1; + + // setupPlanAccountsSection clears planSelectedAccounts and re-renders. + // When a single commitment carries a cloud_account_id, we look up that + // account after the section has reset and add it as a pre-selected chip. void setupPlanAccountsSection(); + if (pendingPlanRecommendations.length === 1) { + const accountId = pendingPlanRecommendations[0]!.cloud_account_id; + if (accountId) { + void prefillAccountChipFromId(accountId, planModalSession); + } + } const planModal = document.getElementById('plan-modal'); if (planModal) {