From ff8eb6cdc7fba8b802df1cf731f84a08e9101390 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 5 Jun 2026 11:33:46 +0200 Subject: [PATCH] fix(frontend/tests): assign Purchaser group to admin mocks (carve-out follow-up) PR #924's test-fix wave updated history-approve-button.test.ts, history-retry-button.test.ts, recommendations-permissions.test.ts, and permissions.test.ts, but missed three suites that also broke under the #923 carve-out contract: - execute-mode-toggle.test.ts: mockUser('admin') produced { role: 'admin' } with no groups array. The toggle check calls canAccess('execute-any', 'purchases') which falls back to isAdmin() (not a carved-out verb), and isAdmin() requires ADMINISTRATORS_GROUP_ID in groups. Without groups the toggle was never rendered. - purchase-execution-toast.test.ts: the jest.mock('../recommendations') factory was missing getExecuteMode and clearExecuteMode, both of which app.ts calls in handleExecutePurchase (added in the #289 / #924 wave). Every single-record test threw TypeError at line 328 of app.ts before reaching the toast assertions. - recommendations.test.ts: the default state mock used groups: ['...000000000001'] (Administrators only). After #923, the execute-mode toggle checks canAccess('execute-any', 'purchases') which calls isAdmin() -- not a carved-out verb -- so admin-group membership alone caused the toggle to render. The three tests that assert the approval-required note ('shows purchase summary', 'modal body carries the approval-required explanation', 'approval-required note renders with its dedicated class') saw the toggle instead of the note. Fixes applied: 1. execute-mode-toggle: import ADMINISTRATORS_GROUP_ID + PURCHASER_GROUP_ID from permissions; mockUser('admin') now produces groups: [ADMINISTRATORS_GROUP_ID, PURCHASER_GROUP_ID]. 2. purchase-execution-toast: add getExecuteMode (returns '') and clearExecuteMode to the recommendations mock factory. 3. recommendations: the three approval-required note tests override getCurrentUser to a non-admin user (groups: []) and restore the admin user after each test to prevent mock state from leaking into sibling describe blocks. --- .../src/__tests__/execute-mode-toggle.test.ts | 14 +++++++-- .../purchase-execution-toast.test.ts | 6 ++++ .../src/__tests__/recommendations.test.ts | 29 +++++++++++++++++++ 3 files changed, 47 insertions(+), 2 deletions(-) diff --git a/frontend/src/__tests__/execute-mode-toggle.test.ts b/frontend/src/__tests__/execute-mode-toggle.test.ts index 5b67425df..dc1a1ab11 100644 --- a/frontend/src/__tests__/execute-mode-toggle.test.ts +++ b/frontend/src/__tests__/execute-mode-toggle.test.ts @@ -56,11 +56,21 @@ jest.mock('../toast', () => ({ })); import * as state from '../state'; - +import { ADMINISTRATORS_GROUP_ID, PURCHASER_GROUP_ID } from '../permissions'; + +// 'admin' represents a fully-capable admin: Administrators + Purchaser group +// membership (mirrors the auto-migration that adds existing admins to the +// Purchaser group on first deploy of issue #923). Both groups are needed: +// ADMINISTRATORS_GROUP_ID for isAdmin() (gates execute-any/execute-own), and +// PURCHASER_GROUP_ID for isPurchaser() (gates the carved-out execute:purchases +// / approve-any:purchases / retry-any:purchases verbs from issue #923). +// 'user' and 'readonly' have no group memberships and thus no execute access. type UserRole = 'admin' | 'user' | 'readonly'; const mockUser = (role: UserRole | null) => { (state.getCurrentUser as jest.Mock).mockReturnValue( - role === null ? null : { id: 'u', email: 'u@example.com', role }, + role === null + ? null + : { id: 'u', email: 'u@example.com', groups: role === 'admin' ? [ADMINISTRATORS_GROUP_ID, PURCHASER_GROUP_ID] : [] }, ); }; diff --git a/frontend/src/__tests__/purchase-execution-toast.test.ts b/frontend/src/__tests__/purchase-execution-toast.test.ts index 428fcc64b..356cc39de 100644 --- a/frontend/src/__tests__/purchase-execution-toast.test.ts +++ b/frontend/src/__tests__/purchase-execution-toast.test.ts @@ -46,6 +46,12 @@ jest.mock('../recommendations', () => ({ clearPurchaseModalRecommendations: jest.fn(), getFanOutBuckets: jest.fn(), clearFanOutBuckets: jest.fn(), + // app.ts calls getExecuteMode() and clearExecuteMode() in + // handleExecutePurchase (issue #289 / PR #924 carve-out). + // Default getExecuteMode to '' (approval path) so existing toast tests + // that don't exercise the direct-execute path are unaffected. + getExecuteMode: jest.fn().mockReturnValue(''), + clearExecuteMode: jest.fn(), })); jest.mock('../plans', () => ({ diff --git a/frontend/src/__tests__/recommendations.test.ts b/frontend/src/__tests__/recommendations.test.ts index bb9e659c7..d087cfac4 100644 --- a/frontend/src/__tests__/recommendations.test.ts +++ b/frontend/src/__tests__/recommendations.test.ts @@ -1393,12 +1393,21 @@ describe('Recommendations Module', () => { }); test('shows purchase summary', async () => { + // Use a non-admin session so the approval-required note renders instead + // of the execute-mode toggle (issue #923 carve-out: execute-any/own is + // gated on isAdmin(), which requires the Administrators group; a regular + // user without group membership sees the approval-required note). + // Restore the default admin mock after this test so sibling tests are + // unaffected; the outer beforeEach only calls clearAllMocks() which does + // not reset return values. + (state.getCurrentUser as jest.Mock).mockReturnValue({ id: 'u-reg', email: 'user@example.com', groups: [] }); const recommendations = [ { id: 'rec-2', provider: 'aws' as const, service: 'ec2', resource_type: 't3.medium', region: 'us-east-1', count: 5, term: 1, savings: 100, upfront_cost: 500 }, { id: 'rec-3', provider: 'aws' as const, service: 'rds', resource_type: 'db.r5.large', region: 'us-east-1', count: 2, term: 1, savings: 200, upfront_cost: 1000 } ]; await openPurchaseModal(recommendations); + (state.getCurrentUser as jest.Mock).mockReturnValue({ id: 'u-admin', email: 'admin@example.com', groups: ['00000000-0000-5000-8000-000000000001'] }); const details = document.getElementById('purchase-details'); // Issue #320: the modal now renders a full breakdown table with column @@ -1433,6 +1442,26 @@ describe('Recommendations Module', () => { // wording so a regression that reverts to the misleading "Execute // Purchase" framing fails this suite. describe('approval-required messaging (issue #288)', () => { + beforeEach(() => { + // These tests exercise the approval-required path that renders the + // explanatory note. After issue #923 / PR #924, the execute-mode + // toggle renders only for sessions with execute-any:purchases or + // execute-own:purchases (both gated on isAdmin()). Using a regular + // user here ensures the approval-required note is rendered and the + // execute-mode toggle is absent, which is what the assertions below + // expect. The admin execute-mode toggle path is covered in + // execute-mode-toggle.test.ts. + (state.getCurrentUser as jest.Mock).mockReturnValue({ id: 'u-reg', email: 'user@example.com', groups: [] }); + }); + + afterEach(() => { + // Restore the default admin mock so tests in sibling describe blocks + // are not affected by this scope's non-admin override. The outer + // beforeEach only calls clearAllMocks() which does not reset return + // values, so we must restore explicitly. + (state.getCurrentUser as jest.Mock).mockReturnValue({ id: 'u-admin', email: 'admin@example.com', groups: ['00000000-0000-5000-8000-000000000001'] }); + }); + const baseRec = { id: 'rec-288', provider: 'aws' as const,