Polish/polish codebase - #18
Conversation
… into new component folder structure
|
Warning Review limit reached
Next review available in: 54 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAuthentication routes were moved or added under ChangesAuthentication route handlers
Client auth endpoint migration
Navbar component composition
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant useAuth
participant AuthMeRoute
participant verifyJWT
participant dashboardUserModel
Browser->>useAuth: initialize authentication
useAuth->>AuthMeRoute: GET /api/auth/me
AuthMeRoute->>verifyJWT: verify auth token
AuthMeRoute->>dashboardUserModel: find user by token userId
dashboardUserModel-->>AuthMeRoute: user data without passwordHash
AuthMeRoute-->>useAuth: authentication response
useAuth-->>Browser: render authenticated or Login state
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
app/api/auth/me/route.ts (1)
28-35: 🩺 Stability & Availability | 🔵 TrivialLog ordinary database errors too.
Mongoose errors will not normally be
AppErrorinstances, so this condition suppresses diagnostics while still returning a 500. Log or wrap unknown errors before responding.🤖 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 `@app/api/auth/me/route.ts` around lines 28 - 35, Update the catch block in the /api/auth/me route to log ordinary database errors as well as AppError instances before returning the 500 response. Ensure unknown errors are logged or wrapped with the existing error context, while preserving the current client-facing response.components/features/navbar/ScrollController.tsx (1)
14-16: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winReduce effect-driven state churn in scroll bookkeeping.
lastScrollYis only a previous-value cache, but storing it in state and updating it on every scroll causes an additional render/effect pass. Likewise,setPrefersReducedMotion(mediaQuery.matches)synchronously schedules an initialization render. Keep the previous scroll position in a ref and lazily initialize the media-query state.Proposed refactor
- const [lastScrollY, setLastScrollY] = useState(0); + const lastScrollYRef = useRef(0); - const [prefersReducedMotion, setPrefersReducedMotion] = useState(false); + const [prefersReducedMotion, setPrefersReducedMotion] = useState( + () => + typeof window !== "undefined" && + window.matchMedia("(prefers-reduced-motion: reduce)").matches, + ); useEffect(() => { const mediaQuery = window.matchMedia("(prefers-reduced-motion: reduce)"); - setPrefersReducedMotion(mediaQuery.matches); const handler = (e: MediaQueryListEvent) => setPrefersReducedMotion(e.matches); @@ - } else if (currentScrollY > lastScrollY && currentScrollY > HIDE_DELAY_PX) { + } else if ( + currentScrollY > lastScrollYRef.current && + currentScrollY > HIDE_DELAY_PX + ) { @@ - currentScrollY < lastScrollY && - Math.abs(currentScrollY - lastScrollY) > SCROLL_THRESHOLD + currentScrollY < lastScrollYRef.current && + Math.abs(currentScrollY - lastScrollYRef.current) > SCROLL_THRESHOLD @@ - setLastScrollY(currentScrollY); + lastScrollYRef.current = currentScrollY;Also applies to: 19-26, 30-49
🤖 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 `@components/features/navbar/ScrollController.tsx` around lines 14 - 16, Update the scroll bookkeeping in the component using lastScrollY to store the previous position in a ref rather than state, mutating its current value without triggering renders. Lazily initialize prefersReducedMotion from the media-query match during useState initialization, and remove the synchronous initialization update while preserving subsequent media-query change handling.Source: Linters/SAST tools
🤖 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 `@__tests__/unit/auth-routes.test.ts`:
- Around line 42-56: Update the test case “connects to the database before
checking for an existing user” to assert invocation order using the mock state:
verify connectDB.mock.invocationCallOrder[0] is less than
findOne.mock.invocationCallOrder[0], while preserving the existing call-count,
arguments, and response assertions.
In `@app/api/auth/me/route.ts`:
- Line 27: Update the `/api/auth/me` handler to return the same explicit user
payload shape used by sign-in and sign-up, mapping the authenticated Mongoose
user’s `_id` to `id` instead of serializing the raw document through
`NextResponse.json({ user })`. Preserve the existing response behavior and
fields expected by `useAuth`.
- Around line 18-21: Call connectDB() at the start of the route’s try block,
before the dashboardUserModel.findById query, so database connectivity is
established before fetching the authenticated user. Reuse the existing connectDB
symbol and preserve the current query and passwordHash projection.
In `@components/features/navbar/AuthButton.tsx`:
- Around line 7-12: Update AuthButton’s authenticated branch to destructure and
invoke the signOut function from useAuth when the control is activated. Replace
the non-interactive div with an accessible, keyboard-focusable control that
includes the user’s username in its label, while preserving the existing
authenticated styling and logout behavior.
In `@proxy.ts`:
- Around line 8-9: Update the public-route matching logic in proxy.ts to use
exact matches or a slash boundary instead of startsWith(route). Ensure
/api/auth/sign-in and /api/auth/sign-up remain public, while similarly prefixed
paths such as -foo or nested endpoints do not bypass authentication; apply the
same change to the additional public-route entries noted in the comment.
- Around line 8-9: Update the proxy authentication allowlist alongside
"/api/auth/sign-in" and "/api/auth/sign-up" to include "/api/auth/sign-out",
allowing sign-out requests without a valid token to reach the sign-out route and
clear stale cookies.
---
Nitpick comments:
In `@app/api/auth/me/route.ts`:
- Around line 28-35: Update the catch block in the /api/auth/me route to log
ordinary database errors as well as AppError instances before returning the 500
response. Ensure unknown errors are logged or wrapped with the existing error
context, while preserving the current client-facing response.
In `@components/features/navbar/ScrollController.tsx`:
- Around line 14-16: Update the scroll bookkeeping in the component using
lastScrollY to store the previous position in a ref rather than state, mutating
its current value without triggering renders. Lazily initialize
prefersReducedMotion from the media-query match during useState initialization,
and remove the synchronous initialization update while preserving subsequent
media-query change handling.
🪄 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: ebb4e92a-dbab-4ecb-b9d8-8a9647923c5a
📒 Files selected for processing (18)
__tests__/unit/auth-routes.test.tsapp/(root)/layout.tsxapp/api/auth/me/route.tsapp/api/auth/sign-in/route.tsapp/api/auth/sign-out/route.tsapp/api/auth/sign-up/route.tsapp/api/me/route.tscomponents/Navbar.tsxcomponents/features/navbar/AuthButton.tsxcomponents/features/navbar/Logo.tsxcomponents/features/navbar/NavLinks.tsxcomponents/features/navbar/ScrollController.tsxcomponents/features/navbar/UserSection.tsxcomponents/features/navbar/index.tscomponents/layout/Navbar.tsxhooks/useAuth.tslib/auth/jwt.tsproxy.ts
💤 Files with no reviewable changes (2)
- app/api/me/route.ts
- components/Navbar.tsx
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…sify into polish/polish-codebase
1 participant
Summary by CodeRabbit
New Features
Bug Fixes