test(frontend): pin allowed_accounts UI enforcement#587
Conversation
Add allowed-accounts.test.ts covering the three frontend surfaces that rely on the backend's allowed_accounts filtering: 1. Account chip: when listAccounts returns a filtered subset the chip option list contains exactly those accounts plus the All Accounts sentinel. Disallowed accounts never appear. Zero-account case yields only the sentinel. 2. History list: the UI renders exactly the rows returned by getHistory with no additional client-side account filtering. Empty response yields zero rendered rows. 3. 403 on Cancel: when cancelPurchase rejects with a 403 (disallowed account accessed via deep link or stale tab) the handler surfaces a user-friendly error toast instead of an unhandled exception.
cristim
added
triaged
|
Warning Review limit reached
More reviews will be available in 42 minutes and 52 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 (1)
📝 WalkthroughWalkthroughThis PR adds a comprehensive Jest test suite ( ChangesAllowed Accounts UI Enforcement Tests
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
frontend/src/__tests__/allowed-accounts.test.ts (1)
259-285: 💤 Low valueConsider clarifying the console.error spy comment and async timing.
The test validates the 403 error handling requirement correctly. Two minor observations:
Comment clarity (line 260-261): The comment mentions "history.ts's catch block" but the 403 error originates from the cancel button handler, not from the
loadHistory()catch block (which succeeds on line 273). Consider clarifying which error path is being silenced.Async timing (line 277): The 10ms
setTimeoutwait is somewhat arbitrary. While it works in practice, consider usingwaitForutilities or Jest fake timers for more deterministic async handling in future tests.Neither issue affects correctness, and the test successfully validates that a 403 from
cancelPurchasetriggers an error toast rather than an unhandled exception.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/__tests__/allowed-accounts.test.ts` around lines 259 - 285, Update the test comment to accurately state that the console.error spy is silencing the error emitted by the cancel button handler (the rejection from api.cancelPurchase), not from loadHistory(), so replace "history.ts's catch block" with a phrase referencing the cancel handler/cancelPurchase rejection; additionally, replace the arbitrary await new Promise((r) => setTimeout(r, 10)) with a deterministic async wait (e.g., use Jest DOM/Testing Library's waitFor or proper Jest fake timers) to await the UI/toast update after invoking the button so the expectation on showToast is stable (references: loadHistory, api.cancelPurchase, showToast, the console.error spy).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@frontend/src/__tests__/allowed-accounts.test.ts`:
- Around line 259-285: Update the test comment to accurately state that the
console.error spy is silencing the error emitted by the cancel button handler
(the rejection from api.cancelPurchase), not from loadHistory(), so replace
"history.ts's catch block" with a phrase referencing the cancel
handler/cancelPurchase rejection; additionally, replace the arbitrary await new
Promise((r) => setTimeout(r, 10)) with a deterministic async wait (e.g., use
Jest DOM/Testing Library's waitFor or proper Jest fake timers) to await the
UI/toast update after invoking the button so the expectation on showToast is
stable (references: loadHistory, api.cancelPurchase, showToast, the
console.error spy).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 52956104-32ff-4309-be14-3ffd6e3af5a9
📒 Files selected for processing (1)
frontend/src/__tests__/allowed-accounts.test.ts
… per CR Replace the arbitrary 10ms setTimeout with waitFor from @testing-library/dom so the 403-cancel toast assertion waits deterministically for the mock to be called. Also correct the comment from "history.ts's catch block" to "cancel button handler's catch block" which accurately identifies the code path.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
1 participant
Summary
Add
allowed-accounts.test.tscovering the three frontend surfaces that rely on the backend'sallowed_accountsfiltering (issue #313):listAccountsreturns a filtered subset the chip option list contains exactly those accounts plus the All Accounts sentinel. Disallowed accounts never appear. Zero-account case yields only the sentinel.getHistorywith no additional client-side account filtering. Empty response yields zero rendered rows.cancelPurchaserejects with a 403 (disallowed account accessed via deep link or stale tab) the handler surfaces a user-friendly error toast instead of an unhandled exception.Closes #313
Test plan
jest src/__tests__/allowed-accounts.test.ts)Summary by CodeRabbit