-
Notifications
You must be signed in to change notification settings - Fork 5
fix: user-redirection-when-voting #750
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughParses and validates a Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant LoginPage
participant SessionStorage
participant AuthService
participant Router
User->>LoginPage: Open /login?redirect=/vote/123
LoginPage->>LoginPage: parse & validate `redirect` → `redirectTo`
LoginPage->>SessionStorage: set `postLoginRedirect` = "/vote/123"
Note over AuthService,LoginPage: Authentication attempt (auto-login or SSE)
AuthService-->>LoginPage: auth success
LoginPage->>SessionStorage: read `postLoginRedirect`
alt postLoginRedirect exists
LoginPage->>Router: navigate to "/vote/123"
LoginPage->>SessionStorage: clear `postLoginRedirect`
else fallback to redirectTo or "/"
LoginPage->>Router: navigate to `redirectTo` or "/"
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
platforms/eVoting/src/app/(auth)/login/page.tsx (2)
17-47:⚠️ Potential issue | 🟠 MajorPersist
redirectbefore the auto-login early return.Right now the effect returns early when
ename/session/signatureexist, so theredirectparam is never stored. That means deep-link auto-login will still fall back to/even if a redirect was supplied.✅ Suggested fix (store redirect before the early return)
- if (ename && session && signature) { - // Clean up URL - window.history.replaceState({}, '', window.location.pathname); - - // Auto-submit login - handleAutoLogin(ename, session, signature, appVersion || '0.4.0'); - return; - } - - if (redirect && redirect.startsWith("/") && !redirect.startsWith("//")) { - setRedirectTo(redirect); - sessionStorage.setItem("postLoginRedirect", redirect); - } + if (redirect && redirect.startsWith("/") && !redirect.startsWith("//")) { + setRedirectTo(redirect); + sessionStorage.setItem("postLoginRedirect", redirect); + } + + if (ename && session && signature) { + // Clean up URL + window.history.replaceState({}, '', window.location.pathname); + + // Auto-submit login + handleAutoLogin(ename, session, signature, appVersion || '0.4.0'); + return; + }
184-193:⚠️ Potential issue | 🟡 MinorFix the mobile prompt typo.
“using you” should be “using your”.
✏️ Proposed edit
- <span>Click the button below using you</span> + <span>Click the button below using your</span>
Description of change
fixed the issue when user cast its vote if not loggedin to the evoting it will take the user to the root page after successful login instead of navigating the user to that particular vote page (voteid).
Issue Number
Closes #703
Type of change
How the change has been tested
Manual
Change checklist
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.