-
Notifications
You must be signed in to change notification settings - Fork 58
INJIWEB-1616, A success message will display when a successful login happens and user redirects to home screen #490
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
Signed-off-by: Lkanath Panda <[email protected]>
WalkthroughAdds a login-success notification flow: new Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant PasscodePage as "PasscodePage\n(runs handleUnlockSuccess)"
participant Router as "Router\n(navigate)"
participant HomePage as "HomePage\n(useEffect)"
participant sessionStorage as "sessionStorage"
participant Toast as "Toast\n(showToast)"
User->>PasscodePage: Unlock with passcode
PasscodePage->>sessionStorage: setItem('showLoginSuccessToast','true') (try)
PasscodePage->>Router: navigate('/user/home', { state: { loginSuccess: true } })
Router->>HomePage: new location (with state/key)
HomePage->>HomePage: useEffect on location.key
HomePage->>HomePage: check location.state.loginSuccess OR sessionStorage flag
HomePage->>HomePage: delay 100ms
HomePage->>Toast: showToast(t('Home.loginSuccess'), type: success)
Toast->>User: display success notification
HomePage->>sessionStorage: removeItem('showLoginSuccessToast')
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
INJIWEB-1616.mp4 |
| setDisplayName(userDisplayName); | ||
| }, [userDisplayName]); | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we please add unit tests for this code ?
| AppStorage.removeItem(KEYS.REDIRECT_TO, true); | ||
| } | ||
| navigate(redirectPath); | ||
| // Set sessionStorage flag as backup in case navigation state is lost |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we please add unit tests for this code ?
|
@iamlokanath unit tests are failing on the PR, please check the same |
Signed-off-by: Lkanath Panda <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@inji-web/src/__tests__/pages/User/HomePage.test.tsx`:
- Around line 95-116: The test asserts Storage.prototype.removeItem too early
and can race with HomePage's async effect; update the test so the assertion for
removeItem (the spy created via jest.spyOn(Storage.prototype, 'removeItem')) is
performed inside the same waitFor that checks showToast (or in a separate await
waitFor) after rendering HomePage, ensuring the call with
'showLoginSuccessToast' is awaited before calling removeItemSpy.mockRestore();
keep the rest of the setup (mocking useUser/useLocation and setting
sessionStorage) unchanged.
In `@inji-web/src/__tests__/pages/User/PasscodePage.test.tsx`:
- Around line 181-195: Add an afterEach cleanup block so spies and shared mocks
don't leak between tests: add afterEach(() => { jest.restoreAllMocks();
jest.clearAllMocks(); if (typeof mockNavigate?.mockClear === "function")
mockNavigate.mockClear(); }); This ensures any Storage.prototype.setItem spy
(e.g., setItemSpy created in the test) is restored and global mocks like
mockNavigate (declared in the describe scope) are cleared between tests to avoid
accumulated call counts.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
inji-web/src/__tests__/pages/User/PasscodePage.test.tsx (1)
375-403: Remove duplicate test.This test is identical to the one at lines 311-339 (same name: "should clear passcode input fields when wrong passcode is entered during unlock wallet", same logic, same assertions). This appears to be a copy-paste error.
🔧 Proposed fix
- test("should clear passcode input fields when wrong passcode is entered during unlock wallet", async () => { - const expectedErrorMsg = "The passcode doesn't seem right. Please try again, or tap 'Forgot Passcode' if you need help resetting it"; - // Mock wallet exists - mockApiResponseSequence([ - { data: successWalletResponse }, // fetchWallets - { error: { response: { data: { errorCode: "incorrect_passcode" } } }, status: 400 } // unlockWallet - ]); - - renderWithProviders(<PasscodePage />); - - await waitFor(() => { - expect(mockUseApi.fetchData).toHaveBeenCalledTimes(1); - }); - - // Enter passcode - await enterPasscode(); - const inputs = screen.getAllByTestId('input-passcode'); - inputs.forEach(input => expect(input).toHaveValue('1')); - - // Submit - userEvent.click(screen.getByTestId("btn-submit-passcode")); - - // Wait for error and check inputs are cleared - await waitFor(() => { - inputs.forEach(input => expect(input).toHaveValue('')); - }); - - await verifyPasscodeErrorAndInteractiveElementStatus(expectedErrorMsg, false, "", true, "error-msg-passcode"); - }); - describe('Passcode mismatch validation', () => {
Signed-off-by: Lkanath Panda <[email protected]>



Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.