feat: Org Owners/Admins unable to access their own "Edit Member" menu#24407
feat: Org Owners/Admins unable to access their own "Edit Member" menu#24407Vibgitcode27 wants to merge 6 commits intocalcom:mainfrom
Conversation
|
@Vibgitcode27 is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
github-actions
bot
added
Low priority
WalkthroughMemberList.tsx updates adjust action availability and labels based on whether the target member is the current user and on permission flags. Edit mode now depends solely on canChangeRole/canRemove/canImpersonate/canInvite. Impersonation excludes self. Resend invitation is hidden for self. Remove/leave text and confirmation dialog content switch based on self-targeting. Mobile action menus mirror desktop logic. The role column’s faceted value switch adds explicit block braces. A hook result variable was renamed to _fetchMoreOnBottomReached, replacing the previous reference. Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
Vibgitcode27
changed the title
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/features/ee/teams/components/MemberList.tsx (1)
594-630: Gate mobile edit action behindcanChangeRole.The mobile view shows the edit action whenever
editModeis true (line 596), buteditModecan be true even whencanChangeRoleis false (e.g., if onlycanRemoveis true). This is inconsistent with the desktop view (lines 498-515), which properly gates the edit action behindcanChangeRole.Apply this diff to gate the edit action behind
canChangeRole:<DropdownMenuContent> <DropdownMenuItem className="outline-none"> <DropdownItem disabled={!user.accepted} href={!user.accepted ? undefined : `/${user.username}`} target="_blank" type="button" StartIcon="external-link"> {t("view_public_page")} </DropdownItem> </DropdownMenuItem> - {editMode && ( + {canChangeRole && ( + <DropdownMenuItem> + <DropdownItem + type="button" + onClick={() => + dispatch({ + type: "EDIT_USER_SHEET", + payload: { + user, + showModal: true, + }, + }) + } + StartIcon="pencil"> + {t("edit")} + </DropdownItem> + </DropdownMenuItem> + )} + {canRemove && ( - <> - <DropdownMenuItem> - <DropdownItem - type="button" - onClick={() => - dispatch({ - type: "EDIT_USER_SHEET", - payload: { - user, - showModal: true, - }, - }) - } - StartIcon="pencil"> - {t("edit")} - </DropdownItem> - </DropdownMenuItem> <DropdownMenuItem> <DropdownItem type="button" color="destructive" onClick={() => dispatch({ type: "SET_DELETE_ID", payload: { user, showModal: true, }, }) } StartIcon="user-x"> {isSelf ? t("leave") : t("remove")} </DropdownItem> </DropdownMenuItem> - </> )} </DropdownMenuContent>
🧹 Nitpick comments (1)
packages/features/ee/teams/components/MemberList.tsx (1)
168-174: Consider using named export instead of default export.Per coding guidelines, prefer named exports for better tree-shaking and easier refactoring. While this is a main component that might be exempt, named exports provide clearer imports.
As per coding guidelines.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/features/ee/teams/components/MemberList.tsx(7 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Always use
t()for text localization in frontend code; direct text embedding should trigger a warning
Files:
packages/features/ee/teams/components/MemberList.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/features/ee/teams/components/MemberList.tsx
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/features/ee/teams/components/MemberList.tsx
🧬 Code graph analysis (1)
packages/features/ee/teams/components/MemberList.tsx (1)
packages/features/auth/lib/next-auth-options.ts (1)
session(746-771)
🔇 Additional comments (7)
packages/features/ee/teams/components/MemberList.tsx (7)
435-435: LGTM! Key fix for the reported issue.The
editModecalculation now correctly allows org owners/admins to access the triple-dot menu on their own member card. Removing theisSelfexclusion enables self-editing while maintaining proper permission checks.
437-442: LGTM! Correct security logic.Properly excludes self-impersonation by adding
!isSelfto the conditions. Users should not be able to impersonate themselves.
537-552: LGTM! Correct business logic.Properly excludes the resend invitation option for self by adding
!isSelf. It doesn't make sense to resend an invitation to yourself.
671-679: LGTM! Good practice for block scoping.Adding explicit block braces to the
case "role"statement prevents potential variable scoping issues in switch statements.
689-694: LGTM! Correct pattern for side-effect hooks.The underscore prefix correctly indicates the return value is intentionally unused. The
useFetchMoreOnBottomReachedhook likely sets up scroll listeners as a side effect, so the call must be preserved even though the return value isn't needed.
568-568: Translation keys "leave" and "remove" verified in all locale files.
772-781: Approve confirmation dialog changes
All new translation keys are present in the i18n files across locales.
|
@dhairyashiil I believe the other PR doesn’t fully address the issue, so I’ve created this. Could you please review it? |
Everything looks good to me so far, but could you please check the following?
Record both scenarios and please attach a video here. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
@dhairyashiil I’ve added the video to the main PR README under By the way, slightly off topic, I noticed I have around 5–6 PRs that are approved but not yet merged. Just wanted to check if there’s any update or if the merges are being held for some reason. |
|
@dhairyashiil Also owner can change role to admin or member. But the owner cannot leave team, that is default logic of the app(I didn't want to change that), owner has to first change role then only he would be able to leave. I have updated the video to show you just that. Let me know if you need any changes here! |
|
@dhairyashiil Can you take a look? |
|
This PR has been marked as stale due to inactivity. If you're still working on it or need any help, please let us know or update the PR to keep it active. |
Yes, checked, thanks. |
|
This PR has been marked as stale due to inactivity. If you're still working on it or need any help, please let us know or update the PR to keep it active. |
Devin AI is resolving merge conflictsThis PR has merge conflicts with the Devin will:
If you prefer to resolve conflicts manually, you can close the Devin session and handle it yourself. |
Devin AI is resolving merge conflictsThis PR has merge conflicts with the Devin will:
If you prefer to resolve conflicts manually, you can close the Devin session and handle it yourself. |
Devin AI is resolving merge conflictsThis PR has merge conflicts with the Devin will:
If you prefer to resolve conflicts manually, you can close the Devin session and handle it yourself. |
Resolved conflicts in apps/web/modules/ee/teams/components/MemberList.tsx: - Combined usePathname (from main) and useRouter (from PR) imports - Used main's approach for useFetchMoreOnBottomReached (no variable assignment) Co-Authored-By: unknown <>
…update related logic * Simplified permission checks in UserListTable by removing unnecessary self-check condition for editing permissions.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
sahitya-chandra
added
ready-for-e2e
run-ci
sahitya-chandra
left a comment
sahitya-chandra
left a comment
There was a problem hiding this comment.
Screencast.from.2026-02-19.15-29-49.webm
6 participants
What does this PR do?
This PR fixes an issue where Org Owners and Admins could not access the "Edit Member" menu for their own profiles.
Now, when viewing their own member card, the triple-dot menu shows only two options:
Visual Demo (For contributors especially)
Before this feature :-
After this fix :-
2025-10-10.19-09-01.mp4
CAL_PR_24407.-.Made.with.Clipchamp.mp4
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Checklist