chore:configured frontend to backend connection#2
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/Login.tsx (1)
60-66: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winResend UI visibility is coupled to error-message text instead of the actual error type.
errorMsg.toLowerCase().includes('verify')(Line 101) decides whether to render the resend button, but the actual 403 status is already known at the point of catching (Line 60). If the backend'sdetailfor a 403 doesn't contain "verify" (e.g., "Account not confirmed"), the resend option won't show even though it's exactly the right scenario; a coincidental "verify" substring in an unrelated message would also incorrectly show it.♻️ Suggested fix
const [errorMsg, setErrorMsg] = useState<string | null>(null); + const [needsVerification, setNeedsVerification] = useState(false); ... } catch (err: any) { console.error('Login failed', err); if (err.response?.status === 403) { setErrorMsg(err.response.data?.detail || 'Please verify your email before logging in.'); + setNeedsVerification(true); } else if (err.response?.status === 401 || err.response?.status === 400) { setErrorMsg('Invalid email or password.'); + setNeedsVerification(false); } else { setErrorMsg('An unexpected error occurred. Please try again.'); + setNeedsVerification(false); } } finally { ... - {errorMsg.toLowerCase().includes('verify') && ( + {needsVerification && (Also applies to: 97-132
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Login.tsx` around lines 60 - 66, The resend button logic in Login should not depend on matching text in errorMsg, because the 403 case is already known in the catch block. Update Login’s error handling so the component stores or derives the actual auth/error status from the catch path (especially the 403 branch) and use that state in the resend-button render condition instead of errorMsg.toLowerCase().includes('verify'). Keep the existing user-facing messages, but make the resend UI decision based on the real error type/status coming from the login request handling.
🧹 Nitpick comments (1)
src/features/auth/auth.service.ts (1)
79-90: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd explicit generic to
resendVerification'sapiClient.postfor type-safety parity.
verifyEmail(Line 80) types itsapiClient.postcall explicitly, butresendVerification(Line 88) does not, soresponse.data's shape isn't actually checked against the declaredPromise<{ message: string }>return type — a backend response-shape mismatch would go undetected by the compiler.♻️ Suggested fix
- async resendVerification(username_or_email: string): Promise<{ message: string }> { - const response = await apiClient.post('/auth/resend-verification', { username_or_email }); + async resendVerification(username_or_email: string): Promise<{ message: string }> { + const response = await apiClient.post<{ message: string }>('/auth/resend-verification', { username_or_email }); return response.data; },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/auth/auth.service.ts` around lines 79 - 90, The `resendVerification` method is missing the same explicit `apiClient.post` generic used by `verifyEmail`, so its returned `response.data` is not type-checked against the declared `Promise<{ message: string }>` shape. Update `resendVerification` to pass an explicit response generic to `apiClient.post`, matching the method’s return type and keeping type-safety consistent with `verifyEmail` in `auth.service.ts`.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/App.tsx`:
- Around line 15-18: The redirect logic in App.tsx is running at module
evaluation time, which triggers window.location.replace during import and breaks
jsdom tests on /verify-email. Move the pathname check and redirect into a
runtime effect in App or, preferably, into an earlier routing/bootstrap layer so
it only runs after the app starts, not while the module is being imported.
In `@src/components/VerifyEmail.tsx`:
- Around line 1-144: The VerifyEmail component has formatting that doesn’t match
Prettier, causing CI to fail. Update the JSX/TypeScript in VerifyEmail and
reformat it with Prettier so the import/order, multiline props, and inline style
objects match the project’s formatting rules. Focus on the VerifyEmail React
component and its useEffect/render blocks; no logic changes are needed.
- Around line 27-51: The shared catch in VerifyEmail’s effect is conflating two
different failures: `authService.verifyEmail(token)` and the follow-up
`authService.getMe()` profile fetch. Split the error handling in `VerifyEmail`
so a successful verification with a later `getMe()` failure still shows
verification success (or a profile-loading error) instead of “Invalid or expired
email verification token.”; keep the invalid-token message only for
`verifyEmail` failures and use `setStatus`, `setErrorMessage`, and the
`navigate(ROUTES.DASHBOARD)` flow accordingly.
- Around line 38-40: The redirect scheduled in VerifyEmail’s timeout is not
cleaned up, so a later navigation can fire after the component is gone. Update
the VerifyEmail component to store the timer returned by the setTimeout that
calls navigate(ROUTES.DASHBOARD), and clear it in the component’s
cleanup/unmount path (for example in the same effect that sets it up). Make sure
the fix is tied to the VerifyEmail component and the navigate redirect logic so
the timeout cannot run after unmount.
---
Outside diff comments:
In `@src/components/Login.tsx`:
- Around line 60-66: The resend button logic in Login should not depend on
matching text in errorMsg, because the 403 case is already known in the catch
block. Update Login’s error handling so the component stores or derives the
actual auth/error status from the catch path (especially the 403 branch) and use
that state in the resend-button render condition instead of
errorMsg.toLowerCase().includes('verify'). Keep the existing user-facing
messages, but make the resend UI decision based on the real error type/status
coming from the login request handling.
---
Nitpick comments:
In `@src/features/auth/auth.service.ts`:
- Around line 79-90: The `resendVerification` method is missing the same
explicit `apiClient.post` generic used by `verifyEmail`, so its returned
`response.data` is not type-checked against the declared `Promise<{ message:
string }>` shape. Update `resendVerification` to pass an explicit response
generic to `apiClient.post`, matching the method’s return type and keeping
type-safety consistent with `verifyEmail` in `auth.service.ts`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 87e20114-baee-47c2-9978-619494e7672b
📒 Files selected for processing (6)
src/App.tsxsrc/components/Login.tsxsrc/components/VerifyEmail.tsxsrc/constants/routes.constants.tssrc/features/auth/auth.service.tsvite.config.ts
| import React, { useEffect, useState } from 'react'; | ||
| import { useSearchParams, useNavigate } from 'react-router-dom'; | ||
| import { Loader2, CheckCircle2, XCircle, ArrowRight } from 'lucide-react'; | ||
| import { AuthLayout } from '@/components/ui/AuthLayout/AuthLayout'; | ||
| import { authService } from '@/features/auth/auth.service'; | ||
| import { useUserStore } from '@/stores/userStore'; | ||
| import { ROUTES } from '@/constants/routes.constants'; | ||
|
|
||
| export const VerifyEmail: React.FC = () => { | ||
| const [searchParams] = useSearchParams(); | ||
| const navigate = useNavigate(); | ||
| const { setAccessToken, setUser } = useUserStore(); | ||
|
|
||
| const [status, setStatus] = useState<'loading' | 'success' | 'error'>('loading'); | ||
| const [errorMessage, setErrorMessage] = useState<string | null>(null); | ||
|
|
||
| useEffect(() => { | ||
| const token = searchParams.get('token'); | ||
|
|
||
| if (!token) { | ||
| setStatus('error'); | ||
| setErrorMessage('Missing verification token. Please verify the URL.'); | ||
| return; | ||
| } | ||
|
|
||
| const performVerification = async () => { | ||
| try { | ||
| const response = await authService.verifyEmail(token); | ||
|
|
||
| // If the backend returns access token on successful verification, auto-login | ||
| if (response.access_token) { | ||
| setAccessToken(response.access_token); | ||
| const userProfile = await authService.getMe(); | ||
| setUser(userProfile); | ||
| setStatus('success'); | ||
|
|
||
| // Redirect to dashboard after a short delay | ||
| setTimeout(() => { | ||
| navigate(ROUTES.DASHBOARD); | ||
| }, 2000); | ||
| } else { | ||
| // Fallback if no token returned (require user to sign in manually) | ||
| setStatus('success'); | ||
| } | ||
| } catch (err: any) { | ||
| console.error('Verification failed', err); | ||
| setStatus('error'); | ||
| setErrorMessage( | ||
| err.response?.data?.detail || 'Invalid or expired email verification token.' | ||
| ); | ||
| } | ||
| }; | ||
|
|
||
| performVerification(); | ||
| }, [searchParams, setAccessToken, setUser, navigate]); | ||
|
|
||
| return ( | ||
| <AuthLayout> | ||
| <div | ||
| className="auth-form-card no-card" | ||
| style={{ alignItems: 'center', textAlign: 'center', maxWidth: '380px' }} | ||
| > | ||
| {status === 'loading' && ( | ||
| <> | ||
| <div style={{ marginBottom: '16px', color: 'var(--color-brand-teal)' }}> | ||
| <Loader2 size={48} className="animate-spin" /> | ||
| </div> | ||
| <div className="form-header" style={{ alignItems: 'center' }}> | ||
| <h2 className="form-title">Verifying Email</h2> | ||
| <p className="form-subtitle">Please wait while we secure your account...</p> | ||
| </div> | ||
| </> | ||
| )} | ||
|
|
||
| {status === 'success' && ( | ||
| <> | ||
| <div | ||
| style={{ | ||
| width: '64px', | ||
| height: '64px', | ||
| borderRadius: '50%', | ||
| backgroundColor: '#f0fdf4', | ||
| display: 'flex', | ||
| alignItems: 'center', | ||
| justifyContent: 'center', | ||
| marginBottom: '8px', | ||
| }} | ||
| > | ||
| <CheckCircle2 size={36} color="var(--color-brand-teal)" strokeWidth={2.5} /> | ||
| </div> | ||
| <div className="form-header" style={{ alignItems: 'center' }}> | ||
| <h2 className="form-title">Email Verified!</h2> | ||
| <p className="form-subtitle"> | ||
| Your account is verified. Logging you in and redirecting to the dashboard... | ||
| </p> | ||
| </div> | ||
| </> | ||
| )} | ||
|
|
||
| {status === 'error' && ( | ||
| <> | ||
| <div | ||
| style={{ | ||
| width: '64px', | ||
| height: '64px', | ||
| borderRadius: '50%', | ||
| backgroundColor: '#fef2f2', | ||
| display: 'flex', | ||
| alignItems: 'center', | ||
| justifyContent: 'center', | ||
| marginBottom: '8px', | ||
| }} | ||
| > | ||
| <XCircle size={36} color="#ef4444" strokeWidth={2.5} /> | ||
| </div> | ||
| <div className="form-header" style={{ alignItems: 'center' }}> | ||
| <h2 className="form-title" style={{ color: '#ef4444' }}>Verification Failed</h2> | ||
| <p className="form-subtitle">{errorMessage}</p> | ||
| </div> | ||
|
|
||
| <div | ||
| style={{ | ||
| width: '100%', | ||
| display: 'flex', | ||
| flexDirection: 'column', | ||
| gap: '16px', | ||
| marginTop: '16px', | ||
| }} | ||
| > | ||
| <button | ||
| className="auth-submit-btn" | ||
| onClick={() => navigate(ROUTES.LOGIN)} | ||
| style={{ width: '100%' }} | ||
| > | ||
| <span>Back to Sign In</span> | ||
| <ArrowRight size={18} /> | ||
| </button> | ||
| </div> | ||
| </> | ||
| )} | ||
| </div> | ||
| </AuthLayout> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
CI Prettier check is failing on this file.
Pipeline logs report Prettier formatting issues for this file; run prettier --write before merge.
🧰 Tools
🪛 GitHub Actions: CI / 0_Type-check · Lint · Format · Test · Build.txt
[warning] 1-1: Prettier --check reported formatting issues in this file.
🪛 GitHub Actions: CI / Type-check · Lint · Format · Test · Build
[warning] 1-1: Prettier --check reported formatting/style issues in this file.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/VerifyEmail.tsx` around lines 1 - 144, The VerifyEmail
component has formatting that doesn’t match Prettier, causing CI to fail. Update
the JSX/TypeScript in VerifyEmail and reformat it with Prettier so the
import/order, multiline props, and inline style objects match the project’s
formatting rules. Focus on the VerifyEmail React component and its
useEffect/render blocks; no logic changes are needed.
Source: Pipeline failures
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src/components/Login.tsx (2)
238-243: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCentralize the API base URL instead of inlining the fallback.
import.meta.env.VITE_API_BASE_URL || 'http://localhost:8000'duplicates whatever base-URL logic already backsauthService/apiClient. Keeping two independent fallback literals risks drift if one is updated and the other isn't.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Login.tsx` around lines 238 - 243, Replace the inline API URL fallback in the Google login button’s onClick handler with the existing centralized base-URL configuration used by authService or apiClient, importing or exposing that shared value as needed so all authentication requests use the same API endpoint.
60-68: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winResend-verification UI relies on message-text matching instead of explicit state.
Whether the resend button/section renders is derived from
errorMsg.toLowerCase().includes('verify')(Line 103). This only works because the fallback default text ("Please verify your email...") happens to contain "verify". If the backend'serr.response.data?.detail(Line 63) ever returns a different phrase for the 403 case (e.g. "Account not activated"), the resend UI silently disappears even though the underlying condition (unverified email) is unchanged.Track this explicitly, e.g. a boolean set alongside
errorMsgwhen the 403 branch is hit, instead of inferring intent from copy.♻️ Proposed refactor
const [errorMsg, setErrorMsg] = useState<string | null>(null); + const [isUnverified, setIsUnverified] = useState(false); ... if (err.response?.status === 403) { setErrorMsg(err.response.data?.detail || 'Please verify your email before logging in.'); + setIsUnverified(true); } else if (err.response?.status === 401 || err.response?.status === 400) { setErrorMsg('Invalid email or password.'); + setIsUnverified(false); } else { setErrorMsg('An unexpected error occurred. Please try again.'); + setIsUnverified(false); } ... - {errorMsg.toLowerCase().includes('verify') && ( + {isUnverified && (Also applies to: 99-143
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Login.tsx` around lines 60 - 68, Track unverified-email state explicitly in the Login component instead of deriving resend UI visibility from error-message text. Add a boolean state updated in the 403 branch of the login error handler, reset for other outcomes or new login attempts, and use it to control the resend section currently gated by errorMsg matching in the render logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/App.tsx`:
- Around line 29-33: Fix the OAuth callback redirect in the pathname handling
block of App so window.location.hash is not appended with a second “#”; remove
its leading hash before concatenating it with /#/auth/callback, ensuring the
resulting URL matches ROUTES.AUTH_CALLBACK.
- Around line 23-27: Limit access-token extraction in App initialization to the
OAuth callback route only, excluding the `#/verify-email` flow. Update the
token-matching logic near initAuth so it validates the hash pathname or
otherwise requires the expected OAuth callback pattern before setting
accessToken, preventing verification tokens from being used as bearer tokens.
In `@src/components/AuthCallback.tsx`:
- Line 5: Remove the unused authService import from AuthCallback.tsx, ensuring
no references or dependent logic require it.
In `@src/components/VerifyEmail.tsx`:
- Line 28: Update the VerifyEmail component’s verification flow so a successful
verifyEmail followed by a failed getMe() does not set the generic error status
or display “Verification Failed” while the access token remains persisted; use a
distinct partial-success state or tailored success UI, and align the follow-up
action with the authenticated state. Remove the unused isVerified variable and
adjust the logic around verifyEmail/getMe and status rendering to use the
selected outcome consistently.
---
Nitpick comments:
In `@src/components/Login.tsx`:
- Around line 238-243: Replace the inline API URL fallback in the Google login
button’s onClick handler with the existing centralized base-URL configuration
used by authService or apiClient, importing or exposing that shared value as
needed so all authentication requests use the same API endpoint.
- Around line 60-68: Track unverified-email state explicitly in the Login
component instead of deriving resend UI visibility from error-message text. Add
a boolean state updated in the 403 branch of the login error handler, reset for
other outcomes or new login attempts, and use it to control the resend section
currently gated by errorMsg matching in the render logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2e08a00c-f4f9-41ba-9fee-f653a209da5d
📒 Files selected for processing (6)
src/App.tsxsrc/components/AuthCallback.tsxsrc/components/Login.tsxsrc/components/VerifyEmail.tsxsrc/constants/routes.constants.tssrc/features/auth/auth.service.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/constants/routes.constants.ts
- src/features/auth/auth.service.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/App.tsx (1)
45-49: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winGuard against an empty token in the path-form redirect.
startsWith('/reset-password/')also matches the bare/reset-password/(trailing slash, no token). In that casesubstring(16)yields''and you redirect to/#/reset-password/, which won't match the:tokensegment inROUTES.RESET_PASSWORDand lands on an unmatched route. Add a truthiness guard mirroring the query-form block above, and prefer the constant length over the magic16.♻️ Proposed guard
- if (window.location.pathname.startsWith('/reset-password/')) { - const token = window.location.pathname.substring(16); // '/reset-password/'.length === 16 - window.location.replace(window.location.origin + '/#/reset-password/' + token); - return; - } + const RESET_PREFIX = '/reset-password/'; + if (window.location.pathname.startsWith(RESET_PREFIX)) { + const token = window.location.pathname.substring(RESET_PREFIX.length); + if (token) { + window.location.replace(window.location.origin + '/#/reset-password/' + token); + return; + } + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/App.tsx` around lines 45 - 49, In the path-form redirect in App, guard against an empty token before redirecting, matching the query-form handling above. Derive the token using the length of the '/reset-password/' prefix instead of the magic number 16, and only call window.location.replace when the token is truthy.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/App.tsx`:
- Around line 45-49: In the path-form redirect in App, guard against an empty
token before redirecting, matching the query-form handling above. Derive the
token using the length of the '/reset-password/' prefix instead of the magic
number 16, and only call window.location.replace when the token is truthy.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4772fac6-1c3d-4320-82be-c7a5efa82a15
📒 Files selected for processing (2)
src/App.tsxsrc/test/Login.test.tsx
✅ Files skipped from review due to trivial changes (1)
- src/test/Login.test.tsx
Summary by CodeRabbit