Skip to content

Fix logout redirect to landing page#84

Closed
jordansilly77-stack wants to merge 1 commit into
imuniqueshiv:mainfrom
jordansilly77-stack:fix/logout-redirect-home
Closed

Fix logout redirect to landing page#84
jordansilly77-stack wants to merge 1 commit into
imuniqueshiv:mainfrom
jordansilly77-stack:fix/logout-redirect-home

Conversation

@jordansilly77-stack

@jordansilly77-stack jordansilly77-stack commented Jul 7, 2026

Copy link
Copy Markdown

Summary\n- Clear logged-in state after a successful logout\n- Redirect users to the landing page instead of forcing a hard navigation to /login\n- Keep users on the current page and show an error toast if logout fails\n\n## Validation\n- npm run lint --prefix client\n- npm run build --prefix client\n\nCloses #83

Summary by CodeRabbit

  • Bug Fixes
    • Improved logout behavior so users are fully signed out, with login state and saved data cleared more reliably.
    • Added clearer success and error notifications during logout.
    • Updated post-logout navigation to return users to the home page instead of forcing a login redirect on failure.

@vercel

vercel Bot commented Jul 7, 2026

Copy link
Copy Markdown

@jordansilly77-stack is attempting to deploy a commit to the Shiv Raj Singh's projects Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Jul 7, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Navbar.jsx's logout handler was modified to import toast from react-toastify, add setIsLoggedin to the AppContent context destructuring, and change post-logout behavior to clear state and toast-notify with navigation to "/" on success, or show an error toast on failure instead of redirecting.

Changes

Logout flow fix

Layer / File(s) Summary
Logout state, toast, and navigation update
client/src/components/Navbar.jsx
Adds toast import and setIsLoggedin destructuring; on successful logout, clears login state, userData, and localStorage, closes open menus, shows a success toast, and navigates to /; on failure, shows an error toast instead of redirecting via window.location.href.

Estimated code review effort: 2 (Simple) | ~10 minutes

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant Navbar
  participant API
  participant Toast

  User->>Navbar: Click Logout
  Navbar->>API: Call logout endpoint
  alt Success
    API-->>Navbar: 200 OK
    Navbar->>Navbar: setIsLoggedin(false), clear userData, clear localStorage
    Navbar->>Toast: Show success toast
    Navbar->>Navbar: Navigate to "/"
  else Failure
    API-->>Navbar: Error response
    Navbar->>Toast: Show error toast
  end
Loading

Related Issues: #83

Suggested labels: bug, frontend

Suggested reviewers: imuniqueshiv

🐰 A logout that once led astray,
Now hops back home without delay,
No 404, no lonely fright,
Just a friendly toast, "Goodbye, alright!"
Carrots stored, the state cleared clean —
This bunny's flow is smooth and keen.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main logout redirect fix.
Linked Issues check ✅ Passed The changes match #83 by clearing logout state, using router navigation to '/', showing success/error toasts, and avoiding redirects on failure.
Out of Scope Changes check ✅ Passed The PR stays focused on Navbar logout behavior and does not introduce unrelated feature changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@jordansilly77-stack

Copy link
Copy Markdown
Author

Local validation passed with:\n\n- npm run lint --prefix client\n- npm run build --prefix client\n\nThe Vercel preview check currently points to a Vercel authorization URL, so it looks like a preview deployment authorization issue rather than a client build failure.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
client/src/components/Navbar.jsx (1)

135-155: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consolidate logout through AppContext.logoutUser
handleLogout duplicates the shared auth reset and bypasses isLoggingOut, so this flow can drift. Have logoutUser return success/failure (or throw), then keep only the menu-closing and navigate("/") steps in Navbar.

🤖 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 `@client/src/components/Navbar.jsx` around lines 135 - 155, The logout flow in
handleLogout duplicates auth reset logic instead of using the shared
AppContext.logoutUser path, which can drift and ignores isLoggingOut. Update
AppContext.logoutUser to return success/failure or throw after doing the auth
reset, then change Navbar.handleLogout to call logoutUser and keep only the UI
cleanup steps like closing menus, showing the toast, and navigating to "/".
🤖 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 `@client/src/components/Navbar.jsx`:
- Around line 135-155: The logout flow in handleLogout duplicates auth reset
logic instead of using the shared AppContext.logoutUser path, which can drift
and ignores isLoggingOut. Update AppContext.logoutUser to return success/failure
or throw after doing the auth reset, then change Navbar.handleLogout to call
logoutUser and keep only the UI cleanup steps like closing menus, showing the
toast, and navigating to "/".

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f8189478-68d6-47ac-b61f-de2098a5ca08

📥 Commits

Reviewing files that changed from the base of the PR and between 22c0f25 and e2f8aec.

📒 Files selected for processing (1)
  • client/src/components/Navbar.jsx

@imuniqueshiv

Copy link
Copy Markdown
Owner

@jordansilly77-stack Thank you for your contribution and for fixing the logout redirect issue! 🚀

The implementation looks good and aligns well with the scope of Issue #83.

Before I merge this PR, could you please comment /claim on Issue #83 so the issue is properly assigned according to the repository contribution workflow?

Once you've claimed the issue, I'll be happy to merge your PR. Thanks again for your contribution!

@imuniqueshiv imuniqueshiv added bug Something isn't working good first issue Good for newcomers ECSoC26 Eligible for Elite Coders Summer of Code 2026 automated scoring. good-issue +10 XP labels Jul 7, 2026
@imuniqueshiv

imuniqueshiv commented Jul 8, 2026

Copy link
Copy Markdown
Owner

@jordansilly77-stack Thank you for your contribution! 🚀

Your implementation looks good and successfully addresses the logout redirect issue. I really appreciate the time and effort you put into this fix.

However, this issue had already been claimed by another contributor according to our repository's contribution workflow. To keep the process fair and consistent for everyone, I'll be moving forward with the PR from the contributor who claimed the issue first.

Thank you again for your contribution, and I hope you'll continue contributing to MeetOnMemory. There are many other open issues where your help would be greatly appreciated. Happy coding! 💙

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working ECSoC26 Eligible for Elite Coders Summer of Code 2026 automated scoring. good first issue Good for newcomers good-issue +10 XP

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants