feat: add multi-organization dropdown for organization navigation#40967
feat: add multi-organization dropdown for organization navigation#40967jacquesikot merged 9 commits intoreleasefrom
Conversation
WalkthroughThis update introduces multi-organization support to the application. It adds Redux actions, selectors, reducer logic, sagas, and API methods for fetching the current user's organizations. A new, accessible Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ApplicationsPage
participant ReduxStore
participant Saga
participant OrganizationApi
participant Reducer
User->>ApplicationsPage: Loads Applications page
ApplicationsPage->>ReduxStore: Dispatch fetchMyOrganizations action
ReduxStore->>Saga: Triggers fetchMyOrganizationsSaga
Saga->>OrganizationApi: Call fetchMyOrganizations()
OrganizationApi-->>Saga: Returns organizations data
Saga->>ReduxStore: Dispatch success action with organizations
ReduxStore->>Reducer: Update myOrganizations state
ApplicationsPage->>OrganizationDropdown: Pass organizations as props
User->>OrganizationDropdown: Interacts with dropdown to select organization
Suggested reviewers
Poem
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (4)
app/client/src/ce/actions/organizationActions.ts (1)
23-27: Unify action-creator style with existing codeThe other creators in this file use an implicit-return arrow (
() => ({ … })).
Keeping the style consistent makes the file easier to scan.-export const fetchMyOrganizations = () => { - return { - type: ReduxActionTypes.FETCH_MY_ORGANIZATIONS_INIT, - }; -}; +export const fetchMyOrganizations = () => ({ + type: ReduxActionTypes.FETCH_MY_ORGANIZATIONS_INIT, +});app/client/src/ce/api/OrganizationApi.ts (1)
57-61: Remove redundantasyncwrapper – avoid doublePromiseThe method simply forwards the Axios promise; the extra
asyncadds an extra micro-task and changes the type toPromise<AxiosPromise<…>>.- static async fetchMyOrganizations(): Promise< - AxiosPromise<FetchMyOrganizationsResponse> - > { - return Api.get(`${OrganizationApi.meUrl}/organizations`); - } + static fetchMyOrganizations(): AxiosPromise<FetchMyOrganizationsResponse> { + return Api.get(`${OrganizationApi.meUrl}/organizations`); + }app/client/src/components/OrganizationDropdown.tsx (1)
284-304: Keyboard accessibility & semantics for invitation itemsInvitation
MenuItems lacktabIndex={0}andaria-selectedwhich the active-org list items have. Add the same attributes so screen-reader / keyboard users can navigate the whole list uniformly.app/client/src/ce/pages/Applications/index.tsx (1)
1044-1046: MissingdispatchinuseEffectdepsWhile Redux’s
dispatchis stable, eslint-react-hooks flags this. Keeping the array empty is fine but add// eslint-disable-next-line react-hooks/exhaustive-depsto silence the warning intentionally.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
app/client/src/ce/actions/organizationActions.ts(1 hunks)app/client/src/ce/api/OrganizationApi.ts(2 hunks)app/client/src/ce/constants/ReduxActionConstants.tsx(1 hunks)app/client/src/ce/constants/messages.ts(1 hunks)app/client/src/ce/pages/Applications/index.tsx(6 hunks)app/client/src/ce/reducers/organizationReducer.ts(4 hunks)app/client/src/ce/sagas/organizationSagas.tsx(2 hunks)app/client/src/ce/selectors/organizationSelectors.tsx(1 hunks)app/client/src/components/OrganizationDropdown.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-prettier / prettier-check
- GitHub Check: client-build / client-build
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
🔇 Additional comments (7)
app/client/src/ce/constants/messages.ts (1)
2721-2721: Approved: new pending invitations constant
ThePENDING_INVITATIONSentry aligns with naming conventions and formatting, and fits appropriately in the multi-org section.app/client/src/ce/sagas/organizationSagas.tsx (2)
8-11: Import looks fine – just flagging possible duplication
UpdateOrganizationConfigRequestis already used further down the file; verify that it is not imported twice once you merge other branches.
178-186: Missingfinallyto flip loading flagIf the reducer uses
isFetchingMyOrganizations, remember to dispatch a completion action (or handle success/error together) so the flag is reset on both paths.app/client/src/ce/constants/ReduxActionConstants.tsx (1)
1232-1242: LGTM – new action types correctly addedNo issues spotted with the constant definitions.
app/client/src/ce/api/OrganizationApi.ts (1)
23-33: Align DTO names with backend fieldsIf the server already returns
id,name, etc., translating them toorganizationId,organizationNamecan be handy – just ensure the mapping is done once (e.g., in the saga) to avoid duplicating translation logic across the app.app/client/src/ce/reducers/organizationReducer.ts (1)
121-146: Reducer handlers don’t preserve existingmyOrganizationson fetch start/errorOn
FETCH_MY_ORGANIZATIONS_INITthe handler only flips the flag; good.
However onFETCH_MY_ORGANIZATIONS_ERRORwe silently keep the previous list (fine) but don’t surface the error. If an error type/field exists elsewhere in state, pipe it through here; otherwise at least log or add a TODO for proper error handling.
[ suggest_optional_refactor ]app/client/src/components/OrganizationDropdown.tsx (1)
168-175:handleSelectopens “https://undefined” for pending invitationsPending invitations normally don’t have an
organizationUrl.
Opening a blank tab is a poor UX.- if (organization.organizationUrl) { - const url = `https://${organization.organizationUrl}`; - window.open(url, "_blank", "noopener,noreferrer"); - } + if (organization.organizationUrl) { + window.open( + organization.organizationUrl.startsWith("http") + ? organization.organizationUrl + : `https://${organization.organizationUrl}`, + "_blank", + "noopener,noreferrer", + ); + }Also consider a separate click handler for invitations (e.g. bring up the invitation accept flow).
Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
app/client/src/components/OrganizationDropdown.tsx (3)
135-139: Remove the unusedPendingInvitationinterface
PendingInvitationis declared but never referenced. It introduces noise and can mislead future contributors into thinking there is a distinct type flowing through the component.
If you need a dedicated type for invites, refactor thependingInvitationslogic to actually use it; otherwise, delete the interface.-export interface PendingInvitation { - id: string; - organizationName: string; -}
155-160: Derive active / invited lists once withuseMemo
safeOrganizations.filter(...)runs on every render. Although the lists are small today, deriving them in auseMemokeeps renders cheap and signals intent.- const activeOrganizations = safeOrganizations.filter( - (org) => org.state === "ACTIVE", - ); - const pendingInvitations = safeOrganizations.filter( - (org) => org.state === "INVITED", - ); + const { activeOrganizations, pendingInvitations } = React.useMemo(() => { + const active = [] as Organization[]; + const invited = [] as Organization[]; + for (const org of safeOrganizations) { + (org.state === "ACTIVE" ? active : invited).push(org); + } + return { activeOrganizations: active, pendingInvitations: invited }; + }, [safeOrganizations]);
162-166: Gracefully handle empty or short organization names
generateInitialsreturns an empty string whennameis falsy; theAvatarcomponent may render an empty circle.
Return"?"(or a fallback glyph) to keep the UI consistent.- if (!name) return ""; + if (!name) return "?";
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/client/src/ce/pages/Applications/index.tsx(6 hunks)app/client/src/components/OrganizationDropdown.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/client/src/ce/pages/Applications/index.tsx
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: perform-test / client-build / client-build
- GitHub Check: perform-test / server-build / server-unit-tests
- GitHub Check: perform-test / rts-build / build
- GitHub Check: client-lint / client-lint
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-prettier / prettier-check
- GitHub Check: client-build / client-build
🔇 Additional comments (1)
app/client/src/components/OrganizationDropdown.tsx (1)
238-318: Improve keyboard accessibility inside the listboxOnly
Enter/Spaceis handled for selection; arrow navigation is absent, so screen-reader and keyboard users cannot move focus between items.
Consider using@reach/menu-buttonor implementArrowUp/ArrowDownfocus management.This can be addressed later but should be tracked.
| const handleClickOutside = (event: MouseEvent) => { | ||
| if ( | ||
| dropdownRef.current && | ||
| !dropdownRef.current.contains(event.target as Node) | ||
| ) { | ||
| setIsOpen(false); | ||
| } | ||
| }; | ||
|
|
||
| if (isOpen) { | ||
| document.addEventListener("mousedown", handleClickOutside); | ||
|
|
||
| return () => { | ||
| document.removeEventListener("mousedown", handleClickOutside); | ||
| }; | ||
| } | ||
| }, [isOpen]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add touchstart listener for mobile outside-click support
On mobile Safari/Chrome, mousedown isn’t triggered. Attaching a touchstart listener alongside mousedown ensures the menu closes when the user taps elsewhere.
- document.addEventListener("mousedown", handleClickOutside);
+ document.addEventListener("mousedown", handleClickOutside);
+ document.addEventListener("touchstart", handleClickOutside);
...
- document.removeEventListener("mousedown", handleClickOutside);
+ document.removeEventListener("mousedown", handleClickOutside);
+ document.removeEventListener("touchstart", handleClickOutside);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleClickOutside = (event: MouseEvent) => { | |
| if ( | |
| dropdownRef.current && | |
| !dropdownRef.current.contains(event.target as Node) | |
| ) { | |
| setIsOpen(false); | |
| } | |
| }; | |
| if (isOpen) { | |
| document.addEventListener("mousedown", handleClickOutside); | |
| return () => { | |
| document.removeEventListener("mousedown", handleClickOutside); | |
| }; | |
| } | |
| }, [isOpen]); | |
| const handleClickOutside = (event: MouseEvent) => { | |
| if ( | |
| dropdownRef.current && | |
| !dropdownRef.current.contains(event.target as Node) | |
| ) { | |
| setIsOpen(false); | |
| } | |
| }; | |
| if (isOpen) { | |
| document.addEventListener("mousedown", handleClickOutside); | |
| document.addEventListener("touchstart", handleClickOutside); | |
| return () => { | |
| document.removeEventListener("mousedown", handleClickOutside); | |
| document.removeEventListener("touchstart", handleClickOutside); | |
| }; | |
| } | |
| }, [isOpen]); |
🤖 Prompt for AI Agents
In app/client/src/components/OrganizationDropdown.tsx around lines 190 to 206,
the event listener only listens for "mousedown" events, which do not fire on
mobile browsers like Safari and Chrome. To fix this, add a "touchstart" event
listener alongside "mousedown" in the effect to handle outside clicks on mobile
devices. Also, ensure both event listeners are properly removed in the cleanup
function to avoid memory leaks.
There was a problem hiding this comment.
Component is not visible on mobile
There was a problem hiding this comment.
🧩 Analysis chain
@jacquesikot Thanks for clarifying! Let me investigate how the mobile visibility is handled for this component.
🏁 Script executed:
#!/bin/bash
# Search for mobile-specific CSS or media queries related to OrganizationDropdown
rg -i "mobile|@media|screen" app/client/src/components/OrganizationDropdown.tsx -A 3 -B 3
# Search for any conditional rendering based on mobile/device detection in the Applications page
rg -i "mobile|device|screen|window\.innerWidth|useMediaQuery" app/client/src/ce/pages/Applications/index.tsx -A 3 -B 3
# Look for any CSS or styling that might hide this component on mobile
rg -i "OrganizationDropdown" --type css -A 5 -B 5
# Search for any media queries or mobile-specific styles in the codebase related to organization dropdown
rg -i "(mobile|@media.*mobile|@media.*screen)" -A 5 -B 2 | grep -i "organization\|dropdown" -A 10 -B 10
Length of output: 65905
@jacquesikot Thanks for confirming! You're absolutely right - the component is intentionally hidden on mobile devices.
Looking at the code in the Applications page, I can see there's a conditional check:
const isMobile = useIsMobileDevice();
const isCloudBillingEnabled = useIsCloudBillingEnabled();
if (isMobile) return null;This means the entire LeftPane (which contains the OrganizationDropdown) is not rendered on mobile devices, which aligns with the PR objectives that mention "hiding the dropdown on mobile devices for better user experience."
Since the component isn't visible on mobile, my previous suggestion about adding touchstart event listeners is not needed - there's no dropdown to close on mobile devices anyway!
✏️ Learnings added
Learnt from: jacquesikot
PR: appsmithorg/appsmith#40967
File: app/client/src/components/OrganizationDropdown.tsx:190-206
Timestamp: 2025-06-18T06:15:59.551Z
Learning: The OrganizationDropdown component in app/client/src/components/OrganizationDropdown.tsx is intentionally hidden on mobile devices. The LeftPane component that contains it returns null when isMobile is true, as stated in the PR objectives for better mobile user experience.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| margin: var(--ads-v2-spaces-3) 0; | ||
| `; | ||
|
|
||
| const DropdownTrigger = styled.button` |
There was a problem hiding this comment.
Can we move all styled components to a separate new file?
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/15725507420. |
|
Deploy-Preview-URL: https://ce-40967.dp.appsmith.com |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
app/client/src/components/OrganizationDropdown/index.tsx (4)
18-21: Remove unusedPendingInvitationinterface
PendingInvitationisn’t referenced anywhere in the file (the invitation rows reuse theOrganizationtype). Dead declarations add noise and may confuse future editors.-export interface PendingInvitation { - id: string; - organizationName: string; -}
146-156: Clicking the current org should still close the menuWhen the selected row is clicked,
handleSelectis bypassed and the dropdown stays open, forcing an extra click elsewhere. A tiny UX fix: always close on click.- onClick={!isSelected ? () => handleSelect(org) : undefined} + onClick={() => { + if (!isSelected) handleSelect(org); + else setIsOpen(false); + }}
37-41:safeOrganizationsguard is redundant
organizationsalready defaults to an empty array in props; the extra fallback is unnecessary.- const safeOrganizations = organizations || []; - const activeOrganizations = safeOrganizations.filter( + const activeOrganizations = organizations.filter(
45-49: Trim name before extracting initials
" Foo"currently yields a space. Trimming avoids that edge case.- return name.charAt(0).toUpperCase(); + return name.trim().charAt(0).toUpperCase();
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/client/src/components/OrganizationDropdown/index.tsx(1 hunks)app/client/src/components/OrganizationDropdown/styles.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- app/client/src/components/OrganizationDropdown/styles.ts
🧰 Additional context used
🧠 Learnings (1)
app/client/src/components/OrganizationDropdown/index.tsx (1)
Learnt from: jacquesikot
PR: appsmithorg/appsmith#40967
File: app/client/src/components/OrganizationDropdown.tsx:190-206
Timestamp: 2025-06-18T06:15:59.551Z
Learning: The OrganizationDropdown component in app/client/src/components/OrganizationDropdown.tsx is intentionally hidden on mobile devices. The LeftPane component that contains it returns null when isMobile is true, as stated in the PR objectives for better mobile user experience.
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: perform-test / rts-build / build
- GitHub Check: perform-test / client-build / client-build
- GitHub Check: perform-test / server-build / server-unit-tests
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-prettier / prettier-check
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-build / client-build
- GitHub Check: client-lint / client-lint
| const handleSelect = useCallback((organization: Organization) => { | ||
| if (organization.organizationUrl) { | ||
| const url = `https://${organization.organizationUrl}`; | ||
|
|
||
| window.open(url, "_blank", "noopener,noreferrer"); | ||
| } |
There was a problem hiding this comment.
Validate & normalise organizationUrl before opening – potential open-redirect / malformed URL risk
Blindly prefixing https:// can produce double schemes (https://https://…) or allow crafted values such as javascript:alert(1) to slip through.
- if (organization.organizationUrl) {
- const url = `https://${organization.organizationUrl}`;
- window.open(url, "_blank", "noopener,noreferrer");
- }
+ if (organization.organizationUrl) {
+ try {
+ const safeUrl = new URL(organization.organizationUrl, "https://");
+ if (safeUrl.protocol === "https:") {
+ window.open(safeUrl.toString(), "_blank", "noopener,noreferrer");
+ }
+ } catch (e) {
+ console.error("Invalid organization URL:", organization.organizationUrl);
+ }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleSelect = useCallback((organization: Organization) => { | |
| if (organization.organizationUrl) { | |
| const url = `https://${organization.organizationUrl}`; | |
| window.open(url, "_blank", "noopener,noreferrer"); | |
| } | |
| const handleSelect = useCallback((organization: Organization) => { | |
| if (organization.organizationUrl) { | |
| try { | |
| const safeUrl = new URL(organization.organizationUrl, "https://"); | |
| if (safeUrl.protocol === "https:") { | |
| window.open(safeUrl.toString(), "_blank", "noopener,noreferrer"); | |
| } | |
| } catch (e) { | |
| console.error("Invalid organization URL:", organization.organizationUrl); | |
| } | |
| } |
🤖 Prompt for AI Agents
In app/client/src/components/OrganizationDropdown/index.tsx around lines 55 to
60, the code blindly prefixes organization.organizationUrl with "https://" which
can cause double schemes or allow unsafe URLs like "javascript:alert(1)". To fix
this, validate and normalize organization.organizationUrl before opening it by
checking if it already includes a scheme and ensuring it is a safe URL. Only
prepend "https://" if no scheme is present and reject or sanitize any
potentially dangerous URLs to prevent open-redirect or injection risks.
📋 Summary
This PR introduces a multi-organization dropdown component that allows users to view and switch between their organizations. The dropdown displays both active organizations and pending invitations, enabling seamless navigation between different organizational contexts.
🚀 Key Features
New Organization Dropdown Component
Enhanced Redux Architecture
fetchMyOrganizations()to retrieve user's organization listfetchMyOrganizations()endpoint (/v1/users/me/organizations)Integration Points
🔧 Technical Implementation
API Layer (
OrganizationApi.ts)Organizationinterface withorganizationId,organizationName,organizationUrl, andstatefetchMyOrganizations()method returningFetchMyOrganizationsResponseState Management
FETCH_MY_ORGANIZATIONS_INIT/SUCCESS/ERRORaction typesTesting: Please verify the dropdown appears correctly on the Applications page and that organization switching works as expected.
Automation
/ok-to-test tags="@tag.IDE, @tag.Sanity, @tag.Workspace"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/15732330311
Commit: ecd1757
Cypress dashboard.
Tags:
@tag.IDE, @tag.Sanity, @tag.WorkspaceSpec:
Thu, 19 Jun 2025 00:06:02 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
User Interface
Performance