Fix callback getting appended to the url when signin button is clicked on login page #1178
Conversation
Added guard clause to prevent signin() method from being called on login page as it tries to navigate to login page and causing callback url getting appended in the url bar issue. Signed-off-by: idrisGit <idris.gadi01@gmail.com>
|
@CarolinaCobo is attempting to deploy a commit to the Codú Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe pull request includes modifications to three components: Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Hello @IdrisGit, thanks for opening your first Pull Request. The maintainers will review this Pull Request and provide feedback as soon as possible. Keep up the great work! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
components/ArticleMenu/ArticleMenu.tsx (2)
Line range hint
138-141: Consider preventing like action for unauthenticated users.The current implementation attempts the like action before checking authentication, which could lead to unnecessary API calls. Consider checking authentication first:
onClick={() => { - if (data?.currentUserLiked) return likePost(postId, false); - likePost(postId); if (!session) { - signIn(); + return signIn(); } + if (data?.currentUserLiked) { + return likePost(postId, false); + } + likePost(postId); }}
Line range hint
161-164: Consider preventing bookmark action for unauthenticated users.Similar to the like button, the current implementation attempts the bookmark action before checking authentication. Consider checking authentication first:
onClick={() => { if (!session) { - signIn(); + return signIn(); } if (data?.currentUserBookmarked) return bookmarkPost(postId, false); bookmarkPost(postId); }}components/Nav/Nav.tsx (1)
61-71: LGTM: Effective solution with room for minor improvement.The guard clause effectively prevents the callback URL from being appended multiple times. The implementation is clean and the comments clearly explain the NextAuth.js behavior being addressed.
Consider extracting the path string to a constant to improve maintainability:
+const LOGIN_PAGE_PATH = "/get-started"; + const handleSignInPageNavigation = () => { - if (pathname === "/get-started") { + if (pathname === LOGIN_PAGE_PATH) { return; } signIn(); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- components/ArticleMenu/ArticleMenu.tsx (2 hunks)
- components/ArticlePreview/ArticlePreview.tsx (1 hunks)
- components/Nav/Nav.tsx (4 hunks)
✅ Files skipped from review due to trivial changes (1)
- components/ArticlePreview/ArticlePreview.tsx
🔇 Additional comments (5)
components/ArticleMenu/ArticleMenu.tsx (3)
Line range hint
142-147: LGTM! Proper spacing in className template literal.The addition of space in the className template literal ensures reliable class concatenation.
Line range hint
165-170: LGTM! Proper spacing in className template literal.The addition of space in the className template literal ensures reliable class concatenation.
Line range hint
138-164: Verify consistent sign-in behavior across components.While this component uses NextAuth's
signIn(), ensure that the fix for URL callback appending in the Nav component (PR #1178) is consistently applied here if needed.components/Nav/Nav.tsx (2)
3-3: LGTM: Clean hook implementation.The usePathname hook is correctly imported and implemented to access the current route.
Also applies to: 44-45
135-138: LGTM: Consistent implementation across authentication buttons.Both sign-in and sign-up buttons now use the same handler, ensuring consistent behavior and preventing the callback URL issue.
Let's verify there are no other direct signIn() calls that might need similar protection:
Also applies to: 143-143
✅ Verification successful
The previous command failed because the file type was not recognized. Let's try with file extension patterns instead to find all signIn calls and their context.
All signIn() calls appear to be properly guarded or have callback URLs
The search results show that all
signIn()calls in the codebase fall into three categories:
- Protected with session checks (e.g., in ReportModal, CommentsArea)
- Used with proper callback URLs (in get-started/_client.tsx)
- Server-side auth handling (in server/auth.ts)
The changes in Nav.tsx align with existing patterns, and no other components need similar protection.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other direct signIn() calls that might need the same guard clause rg "signIn\(" --type ts --type tsxLength of output: 64
Script:
#!/bin/bash # Search for signIn calls with ripgrep, using just file extensions rg "signIn\(" -g "*.ts" -g "*.tsx" -A 2Length of output: 4409
✨ Codu Pull Request 💻
Fixes #777
Pull Request details
singin()method redirects to the login page and appends the current page URL to the callback.signin()method.Any Breaking changes
Associated Screenshots