Revert "Revert "feat: auth persistence, flash fix, and dashboard UI refinements"" #88
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR enhances the authentication system by refactoring token verification and refresh logic in the useAuth hook, adding loading state handling across auth components, implementing authentication-aware redirects in login/signup forms, and refining Dashboard styling with organization name formatting and simplified visual elements. Changes
Sequence DiagramssequenceDiagram
participant User
participant AuthApp as AuthApp<br/>(Session Check)
participant useAuth as useAuth Hook
participant AuthAPI as Backend API
participant Dashboard
User->>AuthApp: Mount AuthApp
AuthApp->>useAuth: Call useAuth()
useAuth->>useAuth: Check localStorage<br/>for token
alt Token exists
useAuth->>AuthAPI: GET /api/auth/me<br/>(Validate token)
alt Token Valid
AuthAPI-->>useAuth: Return user data
useAuth->>useAuth: setUser(data)<br/>setIsLoading(false)
else Token Invalid (non-401)
AuthAPI-->>useAuth: Error response
useAuth->>useAuth: setIsLoading(false)
else Token Expired (401)
AuthAPI-->>useAuth: 401 Unauthorized
useAuth->>AuthAPI: POST /api/auth/refresh
AuthAPI-->>useAuth: Return new accessToken
useAuth->>useAuth: Save new token
useAuth->>AuthAPI: GET /api/auth/me<br/>(Retry)
AuthAPI-->>useAuth: Return user data
useAuth->>useAuth: setUser(data)<br/>setIsLoading(false)
end
else No Token
useAuth->>AuthAPI: POST /api/auth/refresh
alt Refresh Succeeds
AuthAPI-->>useAuth: New accessToken
useAuth->>useAuth: Save token, retry /api/auth/me
else Refresh Fails
AuthAPI-->>useAuth: Error
useAuth->>useAuth: setIsLoading(false)
end
end
AuthApp->>AuthApp: isLoading?<br/>Render LoadingSpinner : Routes
useAuth-->>Dashboard: Provide user state
Dashboard->>Dashboard: Render with user data
sequenceDiagram
participant User
participant LoginForm as LoginForm/<br/>SignupForm
participant useAuth as useAuth Hook
participant Dashboard
User->>LoginForm: Navigate to /login
LoginForm->>useAuth: Call useAuth()
useAuth-->>LoginForm: Return isAuthenticated,<br/>loading
alt isLoading === true
LoginForm->>LoginForm: Render nothing<br/>(Early return)
else isLoading === false
alt isAuthenticated === true
LoginForm->>LoginForm: Navigate to /dashboard<br/>(replace: true)
LoginForm->>Dashboard: Redirect complete
else isAuthenticated === false
LoginForm->>LoginForm: Render login form
User->>LoginForm: Enter credentials/<br/>Click OAuth button
LoginForm->>useAuth: Call login() or oauth()
useAuth-->>LoginForm: Update loading state
LoginForm->>LoginForm: Show "Connecting..."<br/>text
end
end
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly Related PRs
Suggested Labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
github-actions
bot
added
type:refactor
There was a problem hiding this comment.
Pull request overview
This PR reverts a previous revert, effectively re-introducing authentication persistence improvements, UI refinements, and fixes for authentication flash issues in the dashboard.
Changes:
- Enhanced auth token verification and refresh logic to prevent unnecessary token clearing
- Added authentication checks to redirect already-logged-in users away from login/signup pages
- Refined Dashboard UI by removing visual effects and improving organization name display
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/hooks/useAuth.tsx | Restructured token verification flow to check token validity before attempting refresh, preventing premature token clearing |
| src/components/SignupForm.tsx | Added redirect logic for authenticated users and removed spinner SVG from Google OAuth button |
| src/components/LoginForm.tsx | Added redirect logic for authenticated users, moved useAuth hook declaration, and removed spinner SVG from Google OAuth button |
| src/components/Dashboard.tsx | Added organization name formatting function, removed unused Building2 icon, and simplified UI styling by removing glow effects |
| src/components/AuthApp.tsx | Added loading state handling with LoadingSpinner component during session verification |
| backend | Updated backend subproject commit reference |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const apiBaseUrl = | ||
| process.env.REACT_APP_API_BASE_URL || 'http://localhost:3001'; | ||
| const token = localStorage.getItem('accessToken'); |
There was a problem hiding this comment.
The apiBaseUrl is defined at the start of the function but could be extracted to a constant at the module level or a configuration file to avoid repeated instantiation on every auth check and improve maintainability.
|
|
||
| const formatOrgName = (name: string | null | undefined) => { | ||
| if (!name) return "User's Organization"; | ||
| const capitalized = name.charAt(0).toUpperCase() + name.slice(1); |
There was a problem hiding this comment.
The formatOrgName function assumes the organization name should always have "'s Organization" appended. However, if the name already contains possessive forms or is a full organization name (e.g., "Acme Corporation"), this will result in awkward formatting like "Acme corporation's Organization". Consider whether the organization name should be displayed as-is or clarify the expected input format.
| const capitalized = name.charAt(0).toUpperCase() + name.slice(1); | |
| const trimmed = name.trim(); | |
| if (!trimmed) return "User's Organization"; | |
| const lower = trimmed.toLowerCase(); | |
| const hasPossessive = /'s\b/.test(lower); | |
| const hasOrgKeyword = /(inc|corp|corporation|llc|ltd|company|co\.)\b/.test(lower); | |
| const isMultiWord = trimmed.includes(' '); | |
| const capitalized = | |
| trimmed.charAt(0).toUpperCase() + trimmed.slice(1); | |
| // If it already looks like an organization name or has a possessive, use as-is. | |
| if (hasPossessive || hasOrgKeyword || isMultiWord) { | |
| return capitalized; | |
| } | |
| // Otherwise, treat it as a personal name and append "'s Organization". |
| </svg> | ||
| <span>Connecting...</span> | ||
| </> | ||
| <span>Connecting...</span> |
There was a problem hiding this comment.
Removing the loading spinner SVG eliminates visual feedback during the OAuth connection process. Users may not understand that an action is in progress. Consider retaining a visual loading indicator for better user experience.
| <span>Connecting...</span> | |
| <span className="flex items-center gap-2"> | |
| <svg | |
| className="animate-spin h-4 w-4 text-white" | |
| xmlns="http://www.w3.org/2000/svg" | |
| fill="none" | |
| viewBox="0 0 24 24" | |
| > | |
| <circle | |
| className="opacity-25" | |
| cx="12" | |
| cy="12" | |
| r="10" | |
| stroke="currentColor" | |
| strokeWidth="4" | |
| ></circle> | |
| <path | |
| className="opacity-75" | |
| fill="currentColor" | |
| d="M4 12a8 8 0 018-8v4a4 4 0 00-4 4H4z" | |
| ></path> | |
| </svg> | |
| <span>Connecting...</span> | |
| </span> |
| </svg> | ||
| <span>Connecting...</span> | ||
| </> | ||
| <span>Connecting...</span> |
There was a problem hiding this comment.
Removing the loading spinner SVG eliminates visual feedback during the OAuth connection process. Users may not understand that an action is in progress. Consider retaining a visual loading indicator for better user experience.
| <span>Connecting...</span> | |
| <span className="flex items-center gap-2"> | |
| <svg | |
| className="h-4 w-4 animate-spin text-white" | |
| xmlns="http://www.w3.org/2000/svg" | |
| fill="none" | |
| viewBox="0 0 24 24" | |
| > | |
| <circle | |
| className="opacity-25" | |
| cx="12" | |
| cy="12" | |
| r="10" | |
| stroke="currentColor" | |
| strokeWidth="4" | |
| ></circle> | |
| <path | |
| className="opacity-75" | |
| fill="currentColor" | |
| d="M4 12a8 8 0 018-8v4a4 4 0 00-4 4H4z" | |
| ></path> | |
| </svg> | |
| <span>Connecting...</span> | |
| </span> |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@backend`:
- Line 1: The repo currently tracks "backend" as a gitlink submodule without a
.gitmodules entry, producing a malformed state; either remove the submodule
tracking by replacing the gitlink with real files (remove the gitlink via git rm
--cached backend, commit the actual backend directory contents, and add them as
normal tracked files) or properly configure it as a submodule (add a .gitmodules
entry for "backend" with the correct URL and path, ensure the referenced commit
exists in the remote, then run git submodule sync/update to fetch and commit the
submodule configuration); update CI/README if needed to document the chosen
approach.
🧹 Nitpick comments (2)
src/hooks/useAuth.tsx (1)
120-143: Race condition between setTimeout and background logout.The logout clears local state after 800ms via
setTimeout, but the backend logout runs concurrently. If the backend call completes quickly and the component unmounts or user navigates away, there's no cleanup of the timeout. This could cause state updates on unmounted components.Consider returning a cleanup function or using
useRefto track mounted state, though in practice React 18's automatic batching mitigates most issues here.src/components/Dashboard.tsx (1)
62-68: Consider handling edge cases informatOrgName.The function capitalizes the first character and appends
's Organization. A few considerations:
- Names already capitalized work fine, but names starting with numbers or special characters may produce odd results (e.g.,
"123corp"→"123corp's Organization")- Names ending in 's' will produce grammatically awkward output (e.g.,
"James"→"James's Organization"instead of"James' Organization")If organization names can have varied formats, consider more robust handling.
♻️ Optional: Handle possessive edge case
const formatOrgName = (name: string | null | undefined) => { if (!name) return "User's Organization"; const capitalized = name.charAt(0).toUpperCase() + name.slice(1); - return `${capitalized}'s Organization`; + const possessive = capitalized.endsWith('s') ? "'" : "'s"; + return `${capitalized}${possessive} Organization`; };
| @@ -1 +1 @@ | |||
| Subproject commit 6a4ff2f50a39ca9f9475c235c6cc01e9a16b485b | |||
| Subproject commit 059ee5f2b31ebe76ef171f3708e8a16b822a4f30 | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the submodule update and show its commit summary
git submodule status
git -C backend log -1 --oneline
git -C backend show --stat 059ee5f2b31ebe76ef171f3708e8a16b822a4f30Repository: Refactron-ai/Refactron_Website
Length of output: 422
🏁 Script executed:
#!/bin/bash
# Check .gitmodules content
if [ -f .gitmodules ]; then
echo "=== .gitmodules content ==="
cat .gitmodules
else
echo ".gitmodules not found"
fi
# Check git ls-files status for backend
echo -e "\n=== Git status for backend ==="
git ls-files -s -- backend
# Check all files at root
echo -e "\n=== Root files ==="
git ls-files --stage | head -20Repository: Refactron-ai/Refactron_Website
Length of output: 1748
Remove or properly configure the backend submodule.
The backend file is tracked as a submodule (git mode 160000) but there is no .gitmodules file to configure it. This creates a malformed repository state. Either remove the gitlink tracking for backend and commit it as regular files, or add a proper .gitmodules configuration with the submodule definition and ensure the referenced commit hash exists in the remote repository.
🤖 Prompt for AI Agents
In `@backend` at line 1, The repo currently tracks "backend" as a gitlink
submodule without a .gitmodules entry, producing a malformed state; either
remove the submodule tracking by replacing the gitlink with real files (remove
the gitlink via git rm --cached backend, commit the actual backend directory
contents, and add them as normal tracked files) or properly configure it as a
submodule (add a .gitmodules entry for "backend" with the correct URL and path,
ensure the referenced commit exists in the remote, then run git submodule
sync/update to fetch and commit the submodule configuration); update CI/README
if needed to document the chosen approach.
Reverts #87
Summary by CodeRabbit
New Features
Bug Fixes
Style
✏️ Tip: You can customize this high-level summary in your review settings.