INJIWEB-1682, allow switching account from passcode screen when wallet is locked #512
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (7)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the
WalkthroughAdds a "Log in with different account" flow for temporarily locked wallets: UI, logout+cleanup+redirect logic in PasscodePage, new translation keys across seven locales, and expanded tests including a mocked Changes
Sequence DiagramsequenceDiagram
actor User
participant PasscodePage as PasscodePage Component
participant API as Logout API
participant UserState as User State
participant Storage as Session Storage
participant Router as Router
User->>PasscodePage: Click "Log in with different account"
PasscodePage->>PasscodePage: handleLoginWithDifferentAccount()
PasscodePage->>API: logoutApi()
API-->>PasscodePage: logout response
PasscodePage->>Storage: remove NAV_GUARD_SESSION_KEY
PasscodePage->>UserState: removeUser()
UserState-->>PasscodePage: confirm removal
PasscodePage->>Router: navigate to "/"
Router-->>User: navigate home
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
Signed-off-by: Lkanath Panda <[email protected]>
INJIWEB-1682.mp4 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
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)
439-467: Duplicate test detected.This test
"should clear passcode input fields when wrong passcode is entered during unlock wallet"is an exact duplicate of the test defined at lines 375-403. Please remove this duplicate to avoid redundant test execution and maintenance confusion.🗑️ Proposed fix: Remove duplicate test
- 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"); - }); -
🤖 Fix all issues with AI agents
In `@inji-web/src/__tests__/pages/User/PasscodePage.test.tsx`:
- Around line 260-261: The failing test is due to double-prefixing of the
testId: TertiaryButton prepends "btn-" to the passed testId
(data-testid={`btn-${testId}`}), but PasscodePage currently passes
testId="btn-login-different-account"; update the prop passed from PasscodePage
(where it sets testId on the TertiaryButton) to "login-different-account" so the
rendered data-testid becomes "btn-login-different-account" and the test
(expecting btn-login-different-account) will pass.
In `@inji-web/src/pages/User/Passcode/PasscodePage.tsx`:
- Around line 296-301: The testId passed to TertiaryButton is already prefixed
inside the component, so remove the duplicated "btn-" prefix: update the
TertiaryButton prop in PasscodePage (where TertiaryButton is used with
onClick={handleLoginWithDifferentAccount} and
className={PasscodePageStyles.forgotPasscodeButton}) to pass
testId="login-different-account" (or another id without the "btn-" prefix) so
the rendered data-testid matches expected "btn-login-different-account".
- Around line 265-266: The code assumes response.error is a ResponseTypeObject
by directly reading (response.error as ApiError)?.response?.data?.errors and
then errorCode; change this to safely narrow or guard the shape before access:
add a type guard or runtime check around response.error (e.g., verify
response.error?.response?.data?.errors is an array via Array.isArray) before
assigning parsedErrors and deriving errorCode in the logout handling logic
(references: response.error, parsedErrors, errorCode, ApiError) and provide a
clear fallback path when the errors array is absent.
🧹 Nitpick comments (2)
inji-web/src/pages/User/Passcode/PasscodePage.tsx (2)
264-270: Empty conditional block serves no purpose.The
if (errorCode !== 'user_logout_error')block is empty and has no effect on execution flow since thefinallyblock always runs regardless. Either remove this check entirely or implement the intended behavior (e.g., logging the non-fatal error).♻️ Option 1: Remove empty check
try { const response = await logoutApi.fetchData({ apiConfig: api.userLogout, }); - if (!response.ok()) { - const parsedErrors = (response.error as ApiError)?.response?.data?.errors; - const errorCode = parsedErrors?.[0]?.errorCode; - if (errorCode !== 'user_logout_error') { - // Non-fatal logout error; still clear session and redirect - } - } + // Logout errors are non-fatal; cleanup proceeds regardless } finally { loginWithDifferentAccountCleanup(); }♻️ Option 2: Log non-fatal errors
if (!response.ok()) { const parsedErrors = (response.error as ApiError)?.response?.data?.errors; const errorCode = parsedErrors?.[0]?.errorCode; if (errorCode !== 'user_logout_error') { - // Non-fatal logout error; still clear session and redirect + console.warn('Non-fatal logout error during account switch:', errorCode); } }
253-257: Consider adding error handling for removeUser.The
loginWithDifferentAccountCleanupfunction callsremoveUser?.()with optional chaining, which is good. However, ifremoveUserthrows, the redirect toROUTES.ROOTwould not execute. Consider wrapping in try-catch ifremoveUserinvolves async operations or could throw.♻️ Optional: Add defensive error handling
const loginWithDifferentAccountCleanup = () => { window.sessionStorage.removeItem(NAV_GUARD_SESSION_KEY); - removeUser?.(); + try { + removeUser?.(); + } catch (e) { + console.error('Error during user removal:', e); + } window.location.replace(ROUTES.ROOT); };
iamlokanath
changed the title
…-account Signed-off-by: Lkanath Panda <[email protected]>
There was a problem hiding this comment.
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/pages/User/Passcode/PasscodePage.tsx (1)
50-66: Reset lock-related UI state when status is not temporarily locked.
isTemporarilyLocked(andcanUnlockWallet/testIdSuffix) can remain stale if the wallet status changes away from a lock state, leaving the “login with different account” button visible and inputs disabled after the lock clears. Consider explicitly resetting these flags in the non-lock path.🔧 Suggested fix
const handleWalletStatusError = (errorCode: string, fallBackError: string | undefined = undefined, httpStatusCode: number | null = null) => { if ( errorCode === WalletLockStatus.TEMPORARILY_LOCKED || errorCode === WalletLockStatus.PERMANENTLY_LOCKED || errorCode === WalletLockStatus.LAST_ATTEMPT_BEFORE_LOCKOUT ) { setTestIdSuffix(`-${walletStatusToTestIdSuffix[errorCode]}`); setError(t(`error.walletStatus.${errorCode}`)); - if (errorCode === WalletLockStatus.TEMPORARILY_LOCKED) { - setIsTemporarilyLocked(true); - } + setIsTemporarilyLocked(errorCode === WalletLockStatus.TEMPORARILY_LOCKED); if (errorCode !== WalletLockStatus.LAST_ATTEMPT_BEFORE_LOCKOUT) { setCanUnlockWallet(false); setPasscode(initialPasscodeArray); setConfirmPasscode(initialPasscodeArray); } } else if (fallBackError) { + setIsTemporarilyLocked(false); + setTestIdSuffix(""); + setCanUnlockWallet(true); if (httpStatusCode === HTTP_STATUS_CODES.BAD_REQUEST) { setError(t(fallBackError)); } } }
|
Signed-off-by: Lkanath Panda <[email protected]>
1 participant
Summary by CodeRabbit
Release Notes
New Features
Localization
Tests
✏️ Tip: You can customize this high-level summary in your review settings.