Replace Role Selection with Organization Hub (#152)#155
Conversation
- Remove /select-role page and related logic - Create Organization Hub at /organizations route - Add reusable organization components (Card, Grid, EmptyState, Header) - Update authentication flow to redirect to Organization Hub - Update navigation and routing to use /organizations instead of /select-role - Add getUserOrganizations API endpoint - Update CreateOrganizationPage and JoinOrganizationPage to redirect to /organizations
|
@mrunmayeekokitkar 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. |
|
Warning Review limit reached
Next review available in: 13 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe pull request replaces role selection with an authenticated Organization Hub, adds organization listing, joining, and switching flows, updates onboarding redirects, and introduces supporting client hook memoization and server-local environment loading. ChangesOrganization Hub
Client hook stability
Server environment loading
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant Login
participant ProtectedRoute
participant OrganizationHub
participant organizationApi
participant organizationController
User->>Login: submit credentials
Login->>ProtectedRoute: navigate to /organizations
ProtectedRoute->>OrganizationHub: render authenticated hub
OrganizationHub->>organizationApi: getUserOrganizations()
organizationApi->>organizationController: GET /api/organizations/user
organizationController-->>OrganizationHub: organization list
User->>OrganizationHub: select organization card
OrganizationHub->>organizationApi: selectOrganization({ organizationId })
organizationApi->>organizationController: POST /api/organizations/select
organizationController-->>OrganizationHub: updated user
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
|
Hi @mrunmayeekokitkar, thank you so much for your PR! 🎉 📋 PR Review Checklist
We appreciate the effort you've put in — let us know if you need any help getting this across the finish line! 💪 |
- Remove unused variables from OrganizationHub.jsx - Wrap scrollToSectionIndex in useCallback in ScrollNavigator.jsx - Add scrollToSectionIndex to useEffect dependency array - Wrap toggleTheme in useCallback in ThemeContext.jsx
imuniqueshiv
left a comment
imuniqueshiv
left a comment
There was a problem hiding this comment.
@mrunmayeekokitkar Thanks for your contribution! I appreciate the effort you put into implementing the Organization Hub and refactoring the onboarding flow. The overall structure and component organization look good.
Before this PR can be merged, there are a few items that still need to be addressed to fully satisfy the requirements of #152:
- Please verify that the authentication and onboarding flow behaves correctly for both new and existing users after replacing
/select-rolewith/organizations. The redirect flow should match the expected behavior described in the issue. - Organization cards should serve as the entry point into the selected organization, as described in the issue. Please ensure the current navigation aligns with that expected behavior.
- The "Browse Organizations" action should match the onboarding experience described in the issue. Please verify that it leads users through the intended browse/join flow.
- Please include the requested screenshots (Desktop, Tablet, and Mobile) in the PR description as specified in the issue.
- Please expand the testing section to briefly mention the scenarios you verified (organization list, empty state, onboarding flow, navigation, and responsive layouts).
Once these items have been addressed and the implementation aligns with the acceptance criteria of #152, I'd be happy to review it again.
Thanks again for your contribution!
imuniqueshiv
added
enhancement
…g flow - Updated ProtectedRoute to redirect new users to /organizations instead of /select-role - Modified Login page to redirect to /organizations after successful authentication - Organization cards now serve as entry point to selected organization via dashboard - Browse Organizations button leads to /join-organization for onboarding flow - Organization Hub displays user's organizations with responsive grid layout - Added organization selection API endpoint and card click handler - Verified authentication flow for both new and existing users
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (3)
client/src/components/ScrollNavigator.jsx (1)
38-73: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider moving
getSectionElementoutside the component.
getSectionElement(lines 29–35) is a pure function (only usesdocument.getElementById) but is defined inside the component, making it a missing dependency in both theuseCallback([]deps, line 73) and theuseEffect([scrollToSectionIndex]deps, line 167). This will triggerreact-hooks/exhaustive-depslint warnings. Since the function has no reactive dependencies, moving it to module scope eliminates the issue without behavior change.♻️ Proposed refactor
// Move outside component, above export default function ScrollNavigator() const getSectionElement = (id) => { return ( document.getElementById(id) || document.getElementById(id.replace(/-/g, "")) || document.getElementById(id.replace(/([A-Z])/g, "-$1").toLowerCase()) ); }; export default function ScrollNavigator() { // ... refs and state ... - const getSectionElement = (id) => { - return ( - document.getElementById(id) || - document.getElementById(id.replace(/-/g, "")) || - document.getElementById(id.replace(/([A-Z])/g, "-$1").toLowerCase()) - ); - }; - // Linear Element Navigation (With Dynamic Header Height Calculations) const scrollToSectionIndex = useCallback((targetIndex) => {Also applies to: 167-167
🤖 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/ScrollNavigator.jsx` around lines 38 - 73, Move the pure getSectionElement function from inside the component to module scope, preserving its existing document.getElementById behavior. This removes it from the reactive dependency requirements for scrollToSectionIndex and the related useEffect without changing scrolling behavior.client/src/components/organization/OrganizationCard.jsx (1)
65-68: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated user-data refresh-and-persist logic across two files.
Both
OrganizationCard.jsxandJoinOrganizationPage.jsxinline the samegetUserData() → setUserData() → localStorage.setItem("userData", …)sequence after a successful API call. Extract this into a shared helper — ideally arefreshUserDatamethod on theAppContentcontext — so the two call sites can't drift apart.
client/src/components/organization/OrganizationCard.jsx#L65-L68: replace the inline sequence with a call to the shared helper.client/src/pages/JoinOrganizationPage.jsx#L38-L42: replace the inline sequence with a call to the same shared helper.🤖 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/organization/OrganizationCard.jsx` around lines 65 - 68, Extract the duplicated getUserData, setUserData, and localStorage persistence sequence into a shared refreshUserData helper on the AppContent context. Replace the inline logic at client/src/components/organization/OrganizationCard.jsx lines 65-68 and client/src/pages/JoinOrganizationPage.jsx lines 38-42 with calls to that helper, preserving the existing successful-refresh behavior at both sites.server/controllers/organizationController.js (1)
232-291: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
selectOrganizationduplicates validation and lookup logic fromjoinOrganization.The auth check,
organizationIdvalidation,Organization.findById, and membership check are nearly identical tojoinOrganization(lines 154-186). Consider extracting a shared helper for the common prefix (auth → validate → find org → check membership) to reduce duplication and ensure both paths stay in sync.♻️ Proposed refactor: extract shared org lookup helper
+// Shared helper: validate request, find org, and check membership +async function validateAndFindOrg(req, res, { requireMember = true } = {}) { + if (!req.user || !req.user.id) { + return { error: res.status(401).json({ success: false, message: "Authentication failed." }) }; + } + const { organizationId } = req.body; + if (!organizationId) { + return { error: res.status(400).json({ success: false, message: "organizationId is required." }) }; + } + const userId = req.user.id; + const organization = await Organization.findById(organizationId); + if (!organization) { + return { error: res.status(404).json({ success: false, message: "Organization not found." }) }; + } + const isMember = organization.members.some((m) => m.toString() === userId.toString()); + if (requireMember && !isMember) { + return { error: res.status(403).json({ success: false, message: "You are not a member of this organization." }) }; + } + return { userId, organization, isMember }; +} + export const selectOrganization = async (req, res) => { try { - const { organizationId } = req.body; - - if (!req.user || !req.user.id) { - return res - .status(401) - .json({ success: false, message: "Authentication failed." }); - } - - if (!organizationId) { - return res - .status(400) - .json({ success: false, message: "organizationId is required." }); - } - - const userId = req.user.id; - - const organization = await Organization.findById(organizationId); - if (!organization) { - return res - .status(404) - .json({ success: false, message: "Organization not found." }); - } - - const isMember = organization.members.some( - (m) => m.toString() === userId.toString(), - ); - - if (!isMember) { - return res - .status(403) - .json({ success: false, message: "You are not a member of this organization." }); - } + const result = await validateAndFindOrg(req, res, { requireMember: true }); + if (result.error) return; + const { userId, organization } = result; // Update user's selected organization🤖 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 `@server/controllers/organizationController.js` around lines 232 - 291, Extract the duplicated authentication, organizationId validation, organization lookup, and membership-check flow from joinOrganization and selectOrganization into a shared helper. Update both functions to reuse that helper while preserving their existing status responses and subsequent operation-specific behavior, including selectOrganization’s user update and response.
🤖 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 `@client/src/components/organization/OrganizationCard.jsx`:
- Around line 65-68: Extract the duplicated getUserData, setUserData, and
localStorage update sequence from OrganizationCard and JoinOrganizationPage into
a shared refreshUserData helper on AppContent. Update both call sites to use the
helper while preserving the existing conditional update behavior.
- Around line 82-84: Update the root card div in OrganizationCard to expose
button semantics with role="button", tabIndex={0}, and an aria-label describing
the organization-switch action. Add an onKeyDown handler that invokes
handleCardClick for Enter and Space, preventing Space’s default scrolling
behavior while preserving the existing click interaction.
In `@client/src/components/ProtectedRoute.jsx`:
- Around line 24-29: Remove "/organizations" from the onboardingPages array used
by ProtectedRoute, leaving only the create-organization and join-organization
routes restricted to incomplete onboarding users. Preserve the existing redirect
behavior for those two onboarding pages so completed users can access the
Organization Hub and post-create/join navigation to "/organizations" remains
functional.
In `@client/src/pages/JoinOrganizationPage.jsx`:
- Around line 38-42: Extract the duplicated getUserData → setUserData →
localStorage.setItem("userData", …) sequence from JoinOrganizationPage and
OrganizationCard into a shared helper. Update both call sites to use that helper
while preserving the existing conditional update behavior and storage key.
- Around line 117-122: Add a joiningId state in JoinOrganizationPage and set it
around the async handleJoin request, clearing it when the request completes or
fails. Update the organization button to disable only when its org._id matches
joiningId and provide loading feedback while disabled, preventing duplicate
submissions.
In `@client/src/pages/OrganizationHub.jsx`:
- Around line 18-20: Update the organization-loading logic in OrganizationHub so
non-success responses explicitly show an error using the existing toast pattern,
passing data.message as the error text. Only set organizations from
data.organizations on successful responses, preventing failed requests from
displaying the empty-state message.
In `@client/src/services/organizationApi.js`:
- Around line 9-10: Add a server-side GET handler for /api/organizations/user in
the organization routes so it matches getUserOrganizations in organizationApi.js
and returns the authenticated user’s organizations. If an equivalent existing
route already provides this data, update getUserOrganizations to use that route
instead; preserve OrganizationHub.jsx loading behavior without a 404.
In `@server/server.js`:
- Around line 32-34: Update the dotenv loading flow in server.js so the existing
fallback dotenv.config() resolves .env relative to __dirname, matching the
envPath resolution used for .env.local; preserve the current .env.local-first
behavior and fallback semantics.
---
Nitpick comments:
In `@client/src/components/organization/OrganizationCard.jsx`:
- Around line 65-68: Extract the duplicated getUserData, setUserData, and
localStorage persistence sequence into a shared refreshUserData helper on the
AppContent context. Replace the inline logic at
client/src/components/organization/OrganizationCard.jsx lines 65-68 and
client/src/pages/JoinOrganizationPage.jsx lines 38-42 with calls to that helper,
preserving the existing successful-refresh behavior at both sites.
In `@client/src/components/ScrollNavigator.jsx`:
- Around line 38-73: Move the pure getSectionElement function from inside the
component to module scope, preserving its existing document.getElementById
behavior. This removes it from the reactive dependency requirements for
scrollToSectionIndex and the related useEffect without changing scrolling
behavior.
In `@server/controllers/organizationController.js`:
- Around line 232-291: Extract the duplicated authentication, organizationId
validation, organization lookup, and membership-check flow from joinOrganization
and selectOrganization into a shared helper. Update both functions to reuse that
helper while preserving their existing status responses and subsequent
operation-specific behavior, including selectOrganization’s user update and
response.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4b7e58b4-f90f-4940-a97b-4105d2190a09
📒 Files selected for processing (18)
client/src/App.jsxclient/src/components/Navbar.jsxclient/src/components/ProtectedRoute.jsxclient/src/components/ScrollNavigator.jsxclient/src/components/organization/OrganizationCard.jsxclient/src/components/organization/OrganizationEmptyState.jsxclient/src/components/organization/OrganizationGrid.jsxclient/src/components/organization/OrganizationHeader.jsxclient/src/context/ThemeContext.jsxclient/src/pages/CreateOrganizationPage.jsxclient/src/pages/JoinOrganizationPage.jsxclient/src/pages/Login.jsxclient/src/pages/OrganizationHub.jsxclient/src/pages/SelectRolePage.jsxclient/src/services/organizationApi.jsserver/controllers/organizationController.jsserver/routes/organizationRoutes.jsserver/server.js
💤 Files with no reviewable changes (1)
- client/src/pages/SelectRolePage.jsx
…endpoint - Add error handling to MongoDB connection to prevent server crash - Add error handling to Redis connection to allow server to run without Redis - Fix CSRF token endpoint to handle missing req.csrfToken function - Server now runs in development mode even without database dependencies - Login functionality restored for testing purposes
- Resolved conflicts in JoinOrganizationPage.jsx - Kept our modern Organization Hub design - Removed SelectRolePage.jsx (replaced by Organization Hub) - Preserved improved UI/UX from our implementation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
… NoSQL injection - Add mongoose import for ObjectId validation - Validate organizationId format in joinOrganization and selectOrganization - Prevent NoSQL injection attacks by checking ObjectId validity before database queries - Addresses high severity security alert on line 254
|
Hi! I have completed the review checklist, please check. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Hi @mrunmayeekokitkar, thank you so much for this solid contribution! 🎉🚀 Your PR #155 (Replace Role Selection with Organization Hub (#152)) has been merged. ✅ Related issue: #152 (🏢 Replace Role Selection with a Production Organization Hub). This update fits nicely into the project and is genuinely appreciated — great work! We'd love to see you contribute again soon. 💙 ⭐ If you're enjoying MeetOnMemory, please consider starring the repository — it really helps us out! |
3 participants
Pull Request
Description
Replace the role selection flow with a centralized Organization Hub. This introduces a new
/organizationsroute, reusable organization UI components, updated authentication and navigation flows, and a new API endpoint for fetching a user's organizations.Type of Change
Related Issue
Closes #152
Testing
Verified the new organization onboarding flow, routing, and navigation.
Changes
Organization Hub
OrganizationHub.jsxat/organizationsReusable Components
Added reusable organization UI components:
OrganizationCard.jsxOrganizationGrid.jsxOrganizationEmptyState.jsxOrganizationHeader.jsxRouting & Navigation
/select-roleroute/organizationsroute/organizationsProtectedRouteonboarding redirect to/organizations/organizationsSelectRolePage.jsxAPI
getUserOrganizationsendpoint toorganizationApi.jsPage Updates
CreateOrganizationPageto redirect to/organizationsJoinOrganizationPageto redirect to/organizationsUI
Screenshots
N/A
Checklist
Summary by CodeRabbit
New Features
Bug Fixes