Repository navigation
fix(frontend/tests): assign Purchaser group to admin mocks (carve-out follow-up) - #972
Conversation
… 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.
|
Warning Review limit reached
More reviews will be available in 3 minutes and 27 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
cristim
added
priority/p1
|
@coderabbitai review |
✅ Action performedReview finished.
|
1 participant
Summary
feat/multicloud-web-frontendPURCHASER_GROUP_IDto admin mocks + add missinggetExecuteMode/clearExecuteModestubs to the recommendations mock factoryRoot cause (points to #924)
PR #924 carved out
execute:purchases,approve-any:purchases, andretry-any:purchasesfromadmin:*and required Purchaser group membership for those verbs. It fixed four test suites but missed these three:execute-mode-toggle.test.ts:mockUser('admin')produced{ role: 'admin' }with nogroupsarray.canAccess('execute-any', 'purchases')falls back toisAdmin(), which requiresADMINISTRATORS_GROUP_IDinuser.groups. Without a groups array,isAdmin()returned false and the execute-mode toggle never rendered (3 tests failed).purchase-execution-toast.test.ts: thejest.mock('../recommendations')factory was missinggetExecuteModeandclearExecuteMode.app.tscalls both inhandleExecutePurchase(introduced in the feat(api,recs): permission-gated direct purchase execute (execute-{any,own}) bypassing approval email, with cost warning + confirm gate #289/feat(auth): add Purchaser group + carve execute/approve-any/retry-any out of admin wildcard (closes #923) #924 wave). Every single-record test threwTypeError: getExecuteMode is not a functionat line 328 ofapp.tsbefore reaching the toast assertions (12 tests failed).recommendations.test.ts: the default state mock hadgroups: ['00000000-0000-5000-8000-000000000001'](Administrators only).canAccess('execute-any', 'purchases')is NOT a carved-out verb so it callsisAdmin()(notisPurchaser()), meaning admin-group membership alone caused the execute-mode toggle to render. Three tests expecting the approval-required note saw the toggle text instead.Per-suite fix
execute-mode-toggle.test.ts: Import
ADMINISTRATORS_GROUP_IDandPURCHASER_GROUP_IDfrom../permissions;mockUser('admin')now producesgroups: [ADMINISTRATORS_GROUP_ID, PURCHASER_GROUP_ID](mirrors the auto-migration pattern from feat(auth): add Purchaser group + carve execute/approve-any/retry-any out of admin wildcard (closes #923) #924). Non-admin roles producegroups: []. 3 tests updated.purchase-execution-toast.test.ts: Add
getExecuteMode: jest.fn().mockReturnValue('')andclearExecuteMode: jest.fn()to the recommendations mock factory. The approval path default ('') leaves all existing toast assertions unaffected. 1 mock factory updated, 12 previously-failing tests now pass.recommendations.test.ts: The three approval-required note tests override
getCurrentUserto a non-admin user (groups: []) so the approval note renders instead of the execute-mode toggle. A paired restore call /afterEachrestores the admin user to prevent mock state from leaking into sibling describe blocks. 3 tests updated.Test plan
npx jest src/__tests__/execute-mode-toggle.test.ts- 6/6 passnpx jest src/__tests__/purchase-execution-toast.test.ts- 21/21 passnpx jest src/__tests__/recommendations.test.ts- 367/367 passnpx tsc --noEmit- cleanfeat/multicloud-web-frontendafter merge