idek#97
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Note Reviews pausedUse the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR integrates Google OAuth authentication, adds per-list import workflows, implements map theme toggling, and refactors macro-route coordinate handling. Six complementary features span OAuth setup, auth page flows, list import UI, map theming, routing parameter expansion, and visual marker/tile updates. ChangesGoogle OAuth and Enhanced List/Map Features
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ 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 |
Member
|
@coderabbitai cancel |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/UnifiedMap/UnifiedMap.tsx (1)
1563-1570:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPass target coordinates during session restore route fetch.
The restore path still calls
fetchMacroRoute(targetStoreId)without coordinates even thoughtargetStoreLocationis already available. That leaves this path out of the coordinate-aware refactor and can fail to restore custom/candidate-store routes reliably after refresh.Suggested fix
- if ( - navigationMode === "city" && - targetStoreLocation && - targetStoreId && - macroRouteGeometry.length === 0 - ) { - void fetchMacroRoute(targetStoreId); - } + if ( + navigationMode === "city" && + targetStoreLocation && + macroRouteGeometry.length === 0 + ) { + void fetchMacroRoute( + targetStoreId ?? null, + targetStoreLocation.lat, + targetStoreLocation.lng, + ); + }🤖 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/pages/UnifiedMap/UnifiedMap.tsx` around lines 1563 - 1570, The restore path calls fetchMacroRoute without coordinates which breaks the coordinate-aware refactor; update the conditional branch that checks navigationMode, targetStoreLocation, targetStoreId, and macroRouteGeometry to call fetchMacroRoute with the target coordinates (e.g., pass targetStoreLocation or its lat/lng) instead of only targetStoreId, and ensure the fetchMacroRoute function signature (and any callers) accept and use those coordinates so restored routes for custom/candidate stores rebuild correctly.
🧹 Nitpick comments (2)
src/pages/RegistrationPage/RegistrationPage.tsx (1)
7-8: ⚡ Quick winConsolidate Google auth handling to one source of truth.
This flow duplicates auth-store mutation after
googleLoginRequestand relies onas string. Let the service own auth-setting, then remove localsetAuthwiring from this page.Proposed refactor
-import { useStore } from "../../context/useStore"; import { googleLoginRequest, registerRequest } from "../../services/authService"; ... - const setAuth = useStore((state) => state.setAuth); ... onSuccess: async (tokenResponse) => { try { - const result = await googleLoginRequest(tokenResponse.access_token); - setAuth(result, result.token as string); + await googleLoginRequest(tokenResponse.access_token); toast.success("Welcome!"); navigate("/dashboard"); } catch { setError("Google login failed. Please try again."); }Also applies to: 22-22, 27-30
🤖 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/pages/RegistrationPage/RegistrationPage.tsx` around lines 7 - 8, The page currently duplicates auth mutations by calling setAuth from useStore after googleLoginRequest/registerRequest and uses unsafe casts (as string); instead, move auth-setting into the service and remove local wiring: update googleLoginRequest and registerRequest to be the single source of truth that sets auth state (or return a fully typed auth result), stop calling setAuth inside RegistrationPage (remove setAuth usage and the as string casts), and clean up imports (remove useStore if no longer used); locate references to googleLoginRequest, registerRequest, and setAuth in RegistrationPage.tsx and remove the local post-request setAuth logic so the component relies on the service-managed auth state.src/pages/LoginPage/LoginPage.tsx (1)
20-23: ⚡ Quick winRemove duplicate auth-state write in Google success handler.
googleLoginRequestalready updates auth store; re-callingsetAuthhere withresult.token as stringduplicates side effects and keeps an unsafe cast on this path.Proposed refactor
const handleGoogleSuccess = async (credential: string) => { try { - const result = await googleLoginRequest(credential); - setAuth(result, result.token as string); + await googleLoginRequest(credential); toast.success("Welcome!"); navigate("/dashboard"); } catch { setError("Google login failed. Please try again."); } };🤖 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/pages/LoginPage/LoginPage.tsx` around lines 20 - 23, The googleLoginRequest success handler currently calls setAuth(result, result.token as string) which duplicates auth-store writes and uses an unsafe cast; remove the duplicate setAuth(...) call and the cast, and rely on googleLoginRequest to update auth state; keep the toast.success("Welcome!") and navigate("/dashboard") behavior, and if you need the token later use a safe access (e.g., check result.token) instead of casting.
🤖 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/main.tsx`:
- Line 8: Replace the hardcoded GOOGLE_CLIENT_ID constant with the
environment-configured value so the Google auth provider uses
import.meta.env.VITE_GOOGLE_CLIENT_ID (or the existing VITE_GOOGLE_CLIENT_ID
reference) at runtime; update the GOOGLE_CLIENT_ID declaration and the
GoogleProvider/clientId usage (look for GOOGLE_CLIENT_ID and GoogleProvider) to
read the env variable instead of the literal string and guard/throw an error if
the env value is missing.
In `@src/services/authService.ts`:
- Around line 64-75: The function googleLoginRequest should fail fast when the
Google auth response lacks a usable token: after extracting token from
response.data, verify token is a non-empty string and if not, throw an Error (or
return a rejected Promise) with a clear message so callers don’t treat this as
success; only call useStore.getState().setAuth(userData, token) when the token
check passes and then return response.data on success.
---
Outside diff comments:
In `@src/pages/UnifiedMap/UnifiedMap.tsx`:
- Around line 1563-1570: The restore path calls fetchMacroRoute without
coordinates which breaks the coordinate-aware refactor; update the conditional
branch that checks navigationMode, targetStoreLocation, targetStoreId, and
macroRouteGeometry to call fetchMacroRoute with the target coordinates (e.g.,
pass targetStoreLocation or its lat/lng) instead of only targetStoreId, and
ensure the fetchMacroRoute function signature (and any callers) accept and use
those coordinates so restored routes for custom/candidate stores rebuild
correctly.
---
Nitpick comments:
In `@src/pages/LoginPage/LoginPage.tsx`:
- Around line 20-23: The googleLoginRequest success handler currently calls
setAuth(result, result.token as string) which duplicates auth-store writes and
uses an unsafe cast; remove the duplicate setAuth(...) call and the cast, and
rely on googleLoginRequest to update auth state; keep the
toast.success("Welcome!") and navigate("/dashboard") behavior, and if you need
the token later use a safe access (e.g., check result.token) instead of casting.
In `@src/pages/RegistrationPage/RegistrationPage.tsx`:
- Around line 7-8: The page currently duplicates auth mutations by calling
setAuth from useStore after googleLoginRequest/registerRequest and uses unsafe
casts (as string); instead, move auth-setting into the service and remove local
wiring: update googleLoginRequest and registerRequest to be the single source of
truth that sets auth state (or return a fully typed auth result), stop calling
setAuth inside RegistrationPage (remove setAuth usage and the as string casts),
and clean up imports (remove useStore if no longer used); locate references to
googleLoginRequest, registerRequest, and setAuth in RegistrationPage.tsx and
remove the local post-request setAuth logic so the component relies on the
service-managed auth state.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d292965c-0af7-4e4a-9483-3e5d354796e8
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (9)
package.jsonsrc/components/Card/ListCard.tsxsrc/main.tsxsrc/pages/Dashboard/Dashboard.tsxsrc/pages/LoginPage/LoginPage.tsxsrc/pages/RegistrationPage/RegistrationPage.tsxsrc/pages/UnifiedMap/UnifiedMap.tsxsrc/services/authService.tssrc/services/routingService.ts
| import "./index.css"; | ||
| import App from "./App.tsx"; | ||
|
|
||
| const GOOGLE_CLIENT_ID = "1011433342347-l9445a7d8f14e06rcog3npas9uvsos96.apps.googleusercontent.com" |
Contributor
There was a problem hiding this comment.
Use environment-configured Google client ID for the provider.
Line 8 hardcodes the client ID while Line 16 reads VITE_GOOGLE_CLIENT_ID; the provider (Line 20) should use the env value to avoid environment-specific OAuth breakage.
Proposed fix
-const GOOGLE_CLIENT_ID = "1011433342347-l9445a7d8f14e06rcog3npas9uvsos96.apps.googleusercontent.com"
+const GOOGLE_CLIENT_ID = import.meta.env.VITE_GOOGLE_CLIENT_ID;
+if (!GOOGLE_CLIENT_ID) {
+ throw new Error("Missing VITE_GOOGLE_CLIENT_ID");
+}
...
-console.log("CLIENT ID:", import.meta.env.VITE_GOOGLE_CLIENT_ID);Also applies to: 16-16, 20-20
🤖 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/main.tsx` at line 8, Replace the hardcoded GOOGLE_CLIENT_ID constant with
the environment-configured value so the Google auth provider uses
import.meta.env.VITE_GOOGLE_CLIENT_ID (or the existing VITE_GOOGLE_CLIENT_ID
reference) at runtime; update the GOOGLE_CLIENT_ID declaration and the
GoogleProvider/clientId usage (look for GOOGLE_CLIENT_ID and GoogleProvider) to
read the env variable instead of the literal string and guard/throw an error if
the env value is missing.
Comment on lines
+64
to
+75
| export const googleLoginRequest = async (credential: string) => { | ||
| const response = await api.post<AuthResponse>( | ||
| "/api/auth/google", | ||
| { credential }, | ||
| { headers: { "X-Return-Token": "true" } }, | ||
| ); | ||
| const { token, ...userData } = response.data; | ||
| if (token) { | ||
| useStore.getState().setAuth(userData, token); | ||
| } | ||
| return response.data; | ||
| }; |
Contributor
There was a problem hiding this comment.
Fail fast when Google auth response has no usable token.
googleLoginRequest currently returns success even if token is absent/empty, which can push callers down a success navigation path without a valid authenticated session.
Proposed fix
export const googleLoginRequest = async (credential: string) => {
const response = await api.post<AuthResponse>(
"/api/auth/google",
{ credential },
{ headers: { "X-Return-Token": "true" } },
);
const { token, ...userData } = response.data;
- if (token) {
- useStore.getState().setAuth(userData, token);
- }
+ if (typeof token !== "string" || token.trim().length === 0) {
+ throw new Error("Invalid response from server. No token received.");
+ }
+ useStore.getState().setAuth(userData, token);
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/services/authService.ts` around lines 64 - 75, The function
googleLoginRequest should fail fast when the Google auth response lacks a usable
token: after extracting token from response.data, verify token is a non-empty
string and if not, throw an Error (or return a rejected Promise) with a clear
message so callers don’t treat this as success; only call
useStore.getState().setAuth(userData, token) when the token check passes and
then return response.data on success.
Contributor
|
✅ Action performedReviews paused. |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit