feat(forgot-password): setup ui and logic#117
Conversation
Summary of ChangesHello @Cysteine12, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces the foundational elements for a password recovery system within the application. It provides users with a dedicated interface to initiate a password reset, ensuring a smoother experience by pre-filling known information and guiding them through the process. The changes lay the groundwork for future password management features. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
📝 WalkthroughWalkthroughAdds a forgot-password flow: new ForgotPassword screen and wrapper, a ResetPassword wrapper, a useForgotPassword hook that calls the API and navigates to reset-password on success, and a login change that navigates to the forgot-password screen with the current email. Changes
Sequence DiagramsequenceDiagram
participant User as "User"
participant Login as "LoginScreen"
participant Forgot as "ForgotPasswordScreen"
participant Hook as "useForgotPassword"
participant API as "Auth API"
participant Nav as "Navigation"
participant Alert as "AlertService"
User->>Login: tap "Forgot Password"
Login->>Nav: navigate to /forgot-password?email=...
Nav->>Forgot: show ForgotPasswordScreen (prefilled email)
User->>Forgot: submit email
Forgot->>Hook: call forgotPassword mutation
Hook->>API: POST /forgot-password
alt success
API-->>Hook: 200 OK
Hook->>Nav: navigate to /reset-password
Nav-->>User: show ResetPasswordScreen
else error
API-->>Hook: error
Hook->>Alert: show error
Alert-->>User: display error message
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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)
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 |
Cysteine12
added
enhancement
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the forgot password functionality, including the UI screen and API integration. The implementation is solid, but I've identified a few issues in the new ForgotPassword screen. Specifically, there's an incorrect title, unsafe handling of a route parameter that could lead to a crash, and a potential UI bug with the loading indicator's color. I've provided specific suggestions to address these points.
![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
🧹 Nitpick comments (2)
mobile/app/(auth)/reset-password.tsx (1)
4-9: Placeholder screen noted — follow-up needed for issue#78.This is a minimal placeholder as mentioned in the PR description. However, the linked issue
#78specifies that the Reset Password screen should accept an email parameter and provide OTP and new password form fields.Would you like me to help scaffold the full Reset Password screen with OTP input, new password fields, and form validation to complete the requirements from issue
#78?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mobile/app/`(auth)/reset-password.tsx around lines 4 - 9, The ResetPasswordScreen currently is a placeholder; update the ResetPasswordScreen component to accept an optional email parameter from navigation/route props (e.g., route.params.email), and replace the placeholder UI with a form containing: an OTP input (one field or grouped inputs), "New Password" and "Confirm Password" inputs, and Submit button; add local state for email, otp, newPassword, confirmPassword, and form validation that checks OTP presence, password strength, and password match (show inline errors), and implement a submit handler (e.g., handleResetPassword) that validates the form, disables the button while submitting, calls the existing password-reset API or auth service, and navigates or shows success/error messages; ensure to keep the component name ResetPasswordScreen and export default unchanged so imports remain valid.mobile/features/auth/hook.ts (1)
111-112: Consider passing the email to the reset-password route.Per issue
#78, the Reset Password screen should accept an email parameter. Currently, navigation does not forward the email, which may be needed for the OTP verification flow.♻️ Suggested change to pass email parameter
- onSuccess: (data) => { - router.replace('/reset-password'); + onSuccess: (data, variables) => { + router.replace({ pathname: '/reset-password', params: { email: variables.email } }); },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mobile/features/auth/hook.ts` around lines 111 - 112, The onSuccess handler currently calls router.replace('/reset-password') without forwarding the user's email; update the onSuccess callback in the auth hook so it extracts the email from the response (e.g., data.email or the form values available in the handler) and passes it to the Reset Password route (for example by using router.replace with a query param or route param carrying the email). Target the onSuccess function and the router.replace call and ensure you URL-encode the email when constructing the route/query.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@mobile/features/auth/hook.ts`:
- Around line 139-140: Prettier formatting is failing in this file; run the code
formatter or reformat the export line that exports useForgotPassword, useLogin,
useLogout, useRegister, useRequestOtp, and useVerifyEmail so it matches project
Prettier settings—e.g., run `npx prettier --write mobile/features/auth/hook.ts`
or apply the same formatting rules (line breaks/commas/spacing) to the export
statement exporting those hooks.
In `@mobile/screens/auth/forgot-password.tsx`:
- Line 1: The file mobile/screens/auth/forgot-password.tsx fails Prettier
formatting; run the formatter (npx prettier --write
mobile/screens/auth/forgot-password.tsx) to fix spacing/line breaks and ensure
imports like the Button import (import { Button } from
'@/components/reusables/ui/button';) and the rest of the file conform to the
project's Prettier rules, then stage the updated file and push the changes.
- Line 48: The CardTitle in mobile/screens/auth/forgot-password.tsx is incorrect
— it currently reads "Sign in to your app" (likely copy-pasted); update the
CardTitle element (the CardTitle component instance) to a Forgot Password
appropriate string such as "Forgot your password" or "Reset your password" so
the heading matches the screen's purpose (ensure the change is made inside the
CardTitle usage in this file).
---
Nitpick comments:
In `@mobile/app/`(auth)/reset-password.tsx:
- Around line 4-9: The ResetPasswordScreen currently is a placeholder; update
the ResetPasswordScreen component to accept an optional email parameter from
navigation/route props (e.g., route.params.email), and replace the placeholder
UI with a form containing: an OTP input (one field or grouped inputs), "New
Password" and "Confirm Password" inputs, and Submit button; add local state for
email, otp, newPassword, confirmPassword, and form validation that checks OTP
presence, password strength, and password match (show inline errors), and
implement a submit handler (e.g., handleResetPassword) that validates the form,
disables the button while submitting, calls the existing password-reset API or
auth service, and navigates or shows success/error messages; ensure to keep the
component name ResetPasswordScreen and export default unchanged so imports
remain valid.
In `@mobile/features/auth/hook.ts`:
- Around line 111-112: The onSuccess handler currently calls
router.replace('/reset-password') without forwarding the user's email; update
the onSuccess callback in the auth hook so it extracts the email from the
response (e.g., data.email or the form values available in the handler) and
passes it to the Reset Password route (for example by using router.replace with
a query param or route param carrying the email). Target the onSuccess function
and the router.replace call and ensure you URL-encode the email when
constructing the route/query.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
mobile/app/(auth)/forgot-password.tsxmobile/app/(auth)/reset-password.tsxmobile/features/auth/hook.tsmobile/screens/auth/forgot-password.tsxmobile/screens/auth/login.tsx
mobile/features/auth/hook.ts
Outdated
| export { useForgotPassword, useLogin, useLogout, useRegister, useRequestOtp, useVerifyEmail }; | ||
|
|
There was a problem hiding this comment.
Fix Prettier formatting issues.
The CI pipeline reports Prettier formatting issues in this file. Run npx prettier --write mobile/features/auth/hook.ts to fix.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mobile/features/auth/hook.ts` around lines 139 - 140, Prettier formatting is
failing in this file; run the code formatter or reformat the export line that
exports useForgotPassword, useLogin, useLogout, useRegister, useRequestOtp, and
useVerifyEmail so it matches project Prettier settings—e.g., run `npx prettier
--write mobile/features/auth/hook.ts` or apply the same formatting rules (line
breaks/commas/spacing) to the export statement exporting those hooks.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
mobile/app/(auth)/_layout.tsx (1)
19-24:⚠️ Potential issue | 🟡 MinorAdd explicit
reset-passwordStack.Screen registration for consistent header configuration.The screen exists at
mobile/app/(auth)/reset-password.tsxand is navigated to from the forgot-password flow (mobile/features/auth/hook.ts:112), but it's not explicitly declared in the Stack. Add it alongside the other screens to ensure predictable header behavior:Suggested addition
<Stack.Screen name="reset-password" options={{ headerTitle: '', headerShown: true }} />All other screens in this flow (
login,register,verify-email,forgot-password) are explicitly registered;reset-passwordshould follow the same pattern.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mobile/app/`(auth)/_layout.tsx around lines 19 - 24, The auth navigation Stack is missing an explicit registration for the "reset-password" route which causes inconsistent header behavior; add a <Stack.Screen> entry for the "reset-password" screen (name "reset-password") alongside the existing <Stack.Screen> entries (login, register, verify-email, forgot-password) and give it the same header options as forgot-password (e.g., headerTitle: '' and headerShown: true) so the reset-password component is explicitly declared and gets predictable header configuration.
🧹 Nitpick comments (1)
mobile/features/auth/hook.ts (1)
114-116: Harden forgot-password error messaging to avoid account-enumeration hints.This path currently displays raw backend messages. For password recovery, prefer a fixed user-facing message and keep detailed error text internal/logged.
Suggested change
onError: (data: AxiosError<ErrorResponse>) => { - Alert.alert('Request Failed', data.response?.data?.message || data.message); + Alert.alert( + 'Request Failed', + 'If an account exists for this email, reset instructions have been sent.' + ); },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mobile/features/auth/hook.ts` around lines 114 - 116, The onError handler in mobile/features/auth/hook.ts currently shows the backend error message directly (onError: (data: AxiosError<ErrorResponse>) => { ... }), which can leak account-enumeration info; change it to display a fixed, generic user-facing Alert.alert message like "If an account with that email exists, you will receive recovery instructions" and stop exposing data.response?.data?.message to the UI, while still recording the detailed error internally (e.g., console.error or the app logger) including data.response and data.message for debugging; keep the AxiosError parameter and only swap the Alert text to the fixed string and add a log call to capture the original error.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@mobile/app/`(auth)/_layout.tsx:
- Around line 19-24: The auth navigation Stack is missing an explicit
registration for the "reset-password" route which causes inconsistent header
behavior; add a <Stack.Screen> entry for the "reset-password" screen (name
"reset-password") alongside the existing <Stack.Screen> entries (login,
register, verify-email, forgot-password) and give it the same header options as
forgot-password (e.g., headerTitle: '' and headerShown: true) so the
reset-password component is explicitly declared and gets predictable header
configuration.
---
Nitpick comments:
In `@mobile/features/auth/hook.ts`:
- Around line 114-116: The onError handler in mobile/features/auth/hook.ts
currently shows the backend error message directly (onError: (data:
AxiosError<ErrorResponse>) => { ... }), which can leak account-enumeration info;
change it to display a fixed, generic user-facing Alert.alert message like "If
an account with that email exists, you will receive recovery instructions" and
stop exposing data.response?.data?.message to the UI, while still recording the
detailed error internally (e.g., console.error or the app logger) including
data.response and data.message for debugging; keep the AxiosError parameter and
only swap the Alert text to the fixed string and add a log call to capture the
original error.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
mobile/app/(auth)/_layout.tsxmobile/features/auth/hook.tsmobile/screens/auth/forgot-password.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- mobile/screens/auth/forgot-password.tsx
1 participant
📌 Summary
🎯 Why is this change needed?
Closes #78
🧠 What was changed?
🏗️ Type of Change
🧪 How was this tested?
Test details:
📸 Screenshots / API Samples (if applicable)
Summary by CodeRabbit