fix: prevent auto-approval banner from showing before eligibility check#1387
Open
fix: prevent auto-approval banner from showing before eligibility check#1387
Conversation
The auto-approval banner was shown immediately on page load because useState(true) was used as the initial value. This caused the "eligible for automatic approval" banner to flash for all rooms before the useEffect ran to check actual eligibility. Changed to useState(false) so the banner only appears after eligibility is confirmed. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a UI flash in the booking flow where the “eligible for automatic approval” banner could briefly render before auto-approval eligibility is actually computed.
Changes:
- Initialize
isAutoApprovaltofalseinuseCheckAutoApprovalto avoid showing the success banner on first render. - Add a unit test intended to validate the hook’s initial
isAutoApprovalstate.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
booking-app/components/src/client/routes/booking/hooks/useCheckAutoApproval.tsx |
Changes the initial isAutoApproval state to false so the success banner can’t appear pre-check. |
booking-app/tests/unit/auto-approval-initial-state.unit.test.tsx |
Adds a regression test for the hook’s initial auto-approval state (needs adjustment to truly validate pre-effect initial render). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
booking-app/tests/unit/auto-approval-initial-state.unit.test.tsx
Outdated
Show resolved
Hide resolved
booking-app/tests/unit/auto-approval-initial-state.unit.test.tsx
Outdated
Show resolved
Hide resolved
Address Copilot review: - Use BookingContext.Provider wrapper instead of _currentValue mock - Capture renderValues[] to assert the first render is false, not just the final state (verified: test fails with useState(true)) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
booking-app/tests/unit/auto-approval-initial-state.unit.test.tsx
Outdated
Show resolved
Hide resolved
booking-app/components/src/client/routes/booking/hooks/useCheckAutoApproval.tsx
Show resolved
Hide resolved
Add an explicit "checking" state so BookingStatusBar does not render approval/rejection banners before the eligibility check completes. Also remove unused imports (act, beforeEach) from the test file. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary of Changes
The "Yay! This request is eligible for automatic approval" banner was briefly shown for all rooms regardless of auto-approval eligibility. The root cause was
useState(true)as the initial value forisAutoApprovalinuseCheckAutoApproval. This meant the banner displayed immediately on render, before theuseEffectcould check actual room eligibility.Changed the initial state to
useState(false)so the banner only appears after the eligibility check confirms auto-approval.Checklist
Screenshots / Video
N/A — One-line state initialization fix. Before: banner flashes on load for all rooms. After: banner only appears when eligibility is confirmed.