fix(web): handle expired session profile fetch (#1448)#1449
Conversation
|
You have run out of free Bugbot PR reviews for this billing cycle. This will reset on March 3. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
There was a problem hiding this comment.
Pull request overview
Fixes the “silent 401” UX by surfacing an immediate toast when the initial /user/profile fetch fails due to an expired/unauthorized session, aligning behavior with expected login prompting for previously-authenticated users.
Changes:
- Show a
react-toastifyerror toast whenUserApi.getProfile()fails with 401/403. - Reuse shared toast options and add a stable
toastIdto avoid duplicate toasts. - Update
UserProvidertests to assert the unauthorized toast behavior.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
packages/web/src/auth/context/UserProvider.tsx |
Adds unauthorized handling for profile fetch to display a session-expired toast. |
packages/web/src/auth/context/UserProvider.test.tsx |
Updates tests to validate toast behavior on 401 profile fetch responses. |
| expect(screen.getAllByTestId("n-icon").length).toBeGreaterThan(0); | ||
| expect(screen.getAllByTestId("d-icon").length).toBeGreaterThan(0); | ||
| expect(screen.getAllByTestId("w-icon").length).toBeGreaterThan(0); |
There was a problem hiding this comment.
AGENTS.md explicitly asks tests in packages/web to avoid using data- attributes/test IDs and prefer semantic queries and user-visible behavior. This new assertion relies on getAllByTestId("n-icon"/"d-icon"/"w-icon"), which makes the test more implementation-coupled. Consider asserting the shortcut keys via accessible text/roles within the shortcut overlay (e.g., locate the Global shortcuts section and assert it contains the expected key labels).
| useKeyUpEvent({ | ||
| combination: ["2"], | ||
| combination: ["d"], | ||
| deps: [navigate], | ||
| handler: () => navigate(ROOT_ROUTES.DAY), | ||
| }); |
There was a problem hiding this comment.
The new global navigation shortcut for "d" conflicts with an existing Now view shortcut: useNowShortcuts already registers combination: ["d"] to focus the task description (packages/web/src/views/Now/shortcuts/useNowShortcuts.ts:48-52). Since useGlobalShortcuts is mounted in the app layout, pressing "d" on the Now page will trigger both handlers (navigation + focus), which is likely unintended. Consider choosing non-conflicting global keys (e.g., keep numeric keys, use a modifier like Shift, or change the Now-view description shortcut) or adding route/context gating so only one handler runs.
packages/web/src/views/Calendar/hooks/shortcuts/useGlobalShortcuts.ts
Outdated
Show resolved
Hide resolved
tyler-dane
force-pushed
the
codex-bug-fix-401-1448
branch
from
77bdb0e to
68e97db
Compare
| expect(mockToastError).toHaveBeenCalledWith( | ||
| "Session expired. Please log in again to reconnect Google Calendar.", | ||
| expect.objectContaining({ | ||
| toastId: "profile-session-expired", | ||
| }), | ||
| ); |
There was a problem hiding this comment.
The test verifies the toast message for unauthorized responses but doesn't cover the FORBIDDEN (403) status code path. Add a test case for status 403 to ensure both unauthorized conditions trigger the toast.
| }); | ||
|
|
||
| it("should handle session fetch errors gracefully", async () => { | ||
| it("shows a login toast when profile fetch returns unauthorized", async () => { |
There was a problem hiding this comment.
The test doesn't verify that console.error is NOT called for unauthorized responses. Add an assertion to confirm the error logging path is skipped when isUnauthorized is true.
… reauthentication
| expect(mockToastError).toHaveBeenCalled(); | ||
| const latestToastCall = mockToastError.mock.calls.at(-1); | ||
| expect(latestToastCall?.[1]).toEqual( | ||
| expect.objectContaining({ | ||
| toastId: "session-expired-api", | ||
| autoClose: false, | ||
| closeOnClick: false, | ||
| draggable: false, |
There was a problem hiding this comment.
The test assertion is checking the wrong parameter. toast.error is called with two parameters: the content (React element) as the first parameter, and options object as the second parameter. The assertion should check both parameters separately like in compass.api.test.ts (lines 112-120), not just the first parameter.
Closes #1448