Conversation
|
Warning Review limit reached
More reviews will be available in 4 minutes and 6 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (18)
WalkthroughThe PR refactors authentication state management from token-based persistence to direct session storage with localStorage, updates the auth API surface to expose only login/logout functions, reorganizes UI components by replacing AuthControls with an enhanced OrgSwitcher dropdown menu, introduces responsive sidebar rendering based on a desktop media query, and removes the session restoration startup flow. ChangesAuth State Simplification & Responsive UI Refactoring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
Fallow audit reportNo GitHub PR/MR findings. Generated by fallow. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ac010afc6d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
This PR refactors authentication/UI wiring by removing the AuthControls sidebar component, expanding OrgSwitcher into a menu that includes sign-out, and simplifying auth session persistence by storing the full session in localStorage. It also introduces a shared isDesktopAtom media-query atom and uses it to avoid rendering duplicate sidebar content for mobile vs desktop.
Changes:
- Refactor login/auth flow to use
reatomForm+loginAction(withAsync)and persistauthSessionAtomviawithLocalStorage, removingrestoreSessionand/auth/me. - Replace
AuthControlswith an expandedOrgSwitchermenu (org list + sign out) and update integration stories accordingly. - Add
isDesktopAtomand use it inAppShellto conditionally render sidebar content based on viewport.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| vite.config.ts | Removes PluginOption casts and relies on inferred Vite plugin types. |
| src/widgets/app-shell/ui/AppShell.tsx | Uses new isDesktopAtom to render only the relevant sidebar variant. |
| src/shared/model/viewport.ts | Adds isDesktopAtom via reatomMediaQuery. |
| src/shared/model/index.ts | Re-exports isDesktopAtom from shared model index. |
| src/pages/login/ui/LoginPage.tsx | Refactors login UI to consume a reatomForm model and bindField inputs. |
| src/pages/login/model/routes.tsx | Creates/login form model via route loader and passes it into LoginPage. |
| src/main.tsx | Removes restoreSession() bootstrap call (session now comes from localStorage). |
| src/entities/auth/model/auth.ts | Simplifies auth model: persist session, remove error/pending atoms, add withAsync to login. |
| src/entities/auth/mocks/handlers.ts | Removes /auth/me mock handler and updates auth mock exports. |
| src/entities/auth/index.ts | Updates public exports to match new auth model (drops removed atoms/actions). |
| src/entities/auth/api/authApi.ts | Removes /auth/me endpoint helpers. |
| src/app/OrgSwitcher.tsx | Expands OrgSwitcher into a menu with org items and sign-out. |
| src/app/mocks/handlers.ts | Removes registration of the deleted authMe mock handler. |
| src/app/integration/Auth.stories.tsx | Updates auth integration tests for new login error behavior and sign-out UI path. |
| src/app/AuthControls.tsx | Deletes the AuthControls component (sign-out is now in OrgSwitcher). |
| src/app/App.tsx | Removes AuthControls from the sidebar footer layout. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export const logoutAction = action(() => { | ||
| authSessionAtom.set(null) | ||
| wrap(logout()) | ||
| }, 'logoutAction') |
| @@ -1,12 +1,21 @@ | |||
| import { urlAtom } from '@reatom/core' | |||
| import { urlAtom, reatomForm } from '@reatom/core' | |||
| import { Fragment } from 'react/jsx-runtime' | |||
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/shared/model/viewport.ts (1)
3-3: ⚡ Quick winKeep the
isDesktopAtombreakpoint aligned with themdresponsive token
src/shared/model/viewport.tshardcodesreatomMediaQuery('(min-width: 768px)')forisDesktopAtom, while sidebar/layout visibility is controlled viamdresponsive props elsewhere (e.g.,src/widgets/app-shell/ui/sidebar.tsx,src/widgets/app-shell/ui/AppShell.tsx,src/widgets/master-details/ui/MasterDetails.tsx). If themdbreakpoint value changes, these can diverge—derive the query from the same shared breakpoint source/constant asmd.🤖 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 `@src/shared/model/viewport.ts` at line 3, Replace the hardcoded '(min-width: 768px)' in export const isDesktopAtom = reatomMediaQuery(...) with the shared `md` breakpoint constant used across the app: import the project’s responsive token (the shared `md` breakpoint) and construct the media query dynamically (e.g., reatomMediaQuery(`(min-width: ${md})`)) so `isDesktopAtom` always follows the same `md` token as components like Sidebar/AppShell/MasterDetails.src/pages/login/model/routes.tsx (1)
13-13: ⚡ Quick winAvoid hardcoded default credentials in initial form state.
Line 13 pre-fills real-looking credentials by default. Prefer empty defaults and let stories/tests fill values explicitly.
♻️ Suggested change
- { email: 'alex@example.com', password: 'password' }, + { email: '', password: '' },🤖 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 `@src/pages/login/model/routes.tsx` at line 13, The initial form state in src/pages/login/model/routes.tsx currently hardcodes realistic credentials ({ email: 'alex@example.com', password: 'password' }); replace those defaults with empty values (e.g., email: '' and password: '') so no real-looking credentials are stored in source, and ensure tests/stories explicitly set any needed example values instead of relying on this file.
🤖 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.
Inline comments:
In `@src/entities/auth/model/auth.ts`:
- Around line 17-20: logoutAction currently clears authSessionAtom too early and
fire-forgets logout() via wrap(logout()), causing unhandled rejections and
possible client/server divergence; change logoutAction to perform the logout
call as an async/await flow (use wrap(logout()).extend(withAsync()) or await the
wrapped promise inside the action), move authSessionAtom.set(null) to after a
successful logout, and add try/catch around the await to handle/log failures and
avoid unhandled rejections (reference logoutAction, logout(),
authSessionAtom.set, wrap).
---
Nitpick comments:
In `@src/pages/login/model/routes.tsx`:
- Line 13: The initial form state in src/pages/login/model/routes.tsx currently
hardcodes realistic credentials ({ email: 'alex@example.com', password:
'password' }); replace those defaults with empty values (e.g., email: '' and
password: '') so no real-looking credentials are stored in source, and ensure
tests/stories explicitly set any needed example values instead of relying on
this file.
In `@src/shared/model/viewport.ts`:
- Line 3: Replace the hardcoded '(min-width: 768px)' in export const
isDesktopAtom = reatomMediaQuery(...) with the shared `md` breakpoint constant
used across the app: import the project’s responsive token (the shared `md`
breakpoint) and construct the media query dynamically (e.g.,
reatomMediaQuery(`(min-width: ${md})`)) so `isDesktopAtom` always follows the
same `md` token as components like Sidebar/AppShell/MasterDetails.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 070f45a2-6463-428c-8ef4-4efef85ec19b
📒 Files selected for processing (16)
src/app/App.tsxsrc/app/AuthControls.tsxsrc/app/OrgSwitcher.tsxsrc/app/integration/Auth.stories.tsxsrc/app/mocks/handlers.tssrc/entities/auth/api/authApi.tssrc/entities/auth/index.tssrc/entities/auth/mocks/handlers.tssrc/entities/auth/model/auth.tssrc/main.tsxsrc/pages/login/model/routes.tsxsrc/pages/login/ui/LoginPage.tsxsrc/shared/model/index.tssrc/shared/model/viewport.tssrc/widgets/app-shell/ui/AppShell.tsxvite.config.ts
💤 Files with no reviewable changes (6)
- src/entities/auth/index.ts
- src/app/mocks/handlers.ts
- src/main.tsx
- src/app/AuthControls.tsx
- src/app/App.tsx
- src/entities/auth/api/authApi.ts
…proved organization handling
Summary by CodeRabbit
New Features
Refactor
Tests