fix: group-charter-redirect #288
Conversation
WalkthroughPost-authentication behavior in the login screen now uses a client-side redirect to /charter via router.push instead of a full page reload. Token/state handling and event source cleanup remain unchanged. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant L as LoginScreen
participant ES as EventSource
participant R as Router
U->>L: Complete authentication
ES-->>L: Auth data (id, token)
L->>L: setAuthId, setAuthToken
L->>ES: close()
L->>L: setIsAuthenticating(false)
L->>R: push("/charter")
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Poem
✨ Finishing Touches
🧪 Generate 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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 comments (3)
platforms/group-charter-manager/src/components/auth/login-screen.tsx (3)
37-43: Close EventSource on unmount to prevent leaksIf the component unmounts during auth, the SSE remains open. Store the instance in a ref and close it in a cleanup.
Example (outside the changed lines):
import { useEffect, useRef, useState } from "react"; const eventSourceRef = useRef<EventSource | null>(null); const watchEventStream = (sessionId: string) => { const sseUrl = `${baseUrl}/api/auth/sessions/${sessionId}`; const es = (eventSourceRef.current = new EventSource(sseUrl)); es.onopen = () => console.log("Successfully connected to auth stream."); es.onmessage = /* ... */; es.onerror = (err) => { console.error("EventSource error:", err); es.close(); }; }; useEffect(() => { initializeAuth(); return () => { eventSourceRef.current?.close(); eventSourceRef.current = null; }; }, []);
48-51: Harden onmessage against malformed JSONA bad event payload will throw and skip cleanup. Add a small guard.
- const data = JSON.parse(e.data); - console.log("Auth data received:", data); + let data; + try { + data = JSON.parse(e.data); + } catch (err) { + console.error("Invalid auth event payload:", err, e.data); + return; + } + console.log("Auth data received:", data);
61-69: Verify AuthProvider re‐initialization on login without full reloadAuthProvider’s client‐side
useEffect(insrc/components/auth/auth-provider.tsx) only runs once on mount and never observes subsequent changes tolocalStorage. When LoginScreen writes the token and owner ID (inlogin-screen.tsxlines 55–61) and then navigates to “/charter”, AuthProvider still has its old state and won’t fetch/api/users/meagain.Actionable fixes:
• In LoginScreen (
src/components/auth/login-screen.tsxlines 61–69), aftersetAuthId(user.id); setAuthToken(token);instead of relying on a full page reload, invoke your auth context to update state. For example:
import { useAuth } from "@/components/auth/auth-provider"; // … const { login } = useAuth(); // … login(token, user); // then navigate router.push("/charter");• Or export and call the internal
initializeAuth()method (fromauth-provider.tsx) immediately after setting the token, or listen for the browserstorageevent inside AuthProvider to trigger a re-fetch.• Alternatively, enhance AuthProvider (
src/components/auth/auth-provider.tsxaround lines 45–53) to watch for storage changes or expose a public API that re-runs the token check and/api/users/mecall.
🧹 Nitpick comments (2)
platforms/group-charter-manager/src/components/auth/login-screen.tsx (2)
67-68: Use replace() to avoid leaving login in historyAfter successful login, replace the history entry so Back doesn’t return to the login screen.
- router.push("/charter"); + router.replace("/charter");
65-66: Remove dead commented-out reloadAvoid keeping commented code; it adds noise. If you need a fallback, guard it behind a feature flag.
- // window.location.reload();
Description of change
fixed the redirect issue in the group-charter-manger
Issue Number
closes N/A
Type of change
How the change has been tested
CI and Manual
Change checklist
Summary by CodeRabbit