fix: Group Members fixes #1445
fix: Group Members fixes #1445
Conversation
Fixed usage of permission policies Updated tests Setup logic to automaitcally do the only action sheet option if available
WalkthroughThe pull request introduces several modifications across multiple files, focusing on group management and action handling. The changes primarily involve refactoring the Changes
Suggested Reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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
CodeRabbit Configuration File (
|
Performance Comparison ReportSignificant Changes To DurationThere are no entries Meaningless Changes To DurationShow entries
Show details
Render Count ChangesThere are no entries Render IssuesThere are no entries Added ScenariosThere are no entries Removed ScenariosThere are no entries |
![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
🧹 Nitpick comments (1)
containers/GroupScreenMembersTable.tsx (1)
112-119: Ensure consistent parameter naming ingetGroupMemberActions.The function call includes multiple booleans (
isAdmin,isSuperAdmin,currentAccountIsAdmin,currentAccountIsSuperAdmin). Consider aligning naming or grouping these flags for clarity, especially within the called function. This will simplify reasoning about permission levels.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
components/StateHandlers/ActionSheetStateHandler.tsx(1 hunks)containers/GroupScreenMembersTable.tsx(4 hunks)features/conversation/conversation-message/conversation-message-content-types/conversation-message-chat-group-update.tsx(1 hunks)utils/groupUtils/getGroupMemberActions.test.ts(1 hunks)utils/groupUtils/getGroupMemberActions.ts(2 hunks)utils/groupUtils/userCanDoGroupActions.ts(3 hunks)
🔇 Additional comments (19)
components/StateHandlers/ActionSheetStateHandler.tsx (1)
18-21: Ensure desired UX when there is only one option.
Short-circuiting the action sheet when options.options.length === 1 is helpful to reduce user friction. However, consider how you handle special cases (e.g., a destructive-only option). Ensure this scenario aligns with your expected UX before automatically calling the callback.
utils/groupUtils/userCanDoGroupActions.ts (2)
14-14: Consistent logic when returning "superAdmin".
The condition properly returns "superAdmin" if isSuperAdmin is true. This looks good.
32-32: Matching new role check for "superAdmin".
This change correctly checks for both "admin" and "superAdmin" in the policy.
utils/groupUtils/getGroupMemberActions.ts (7)
5-12: Refactor to a single object parameter enhances clarity.
Defining GetGroupMemberActionsProps and consolidating inputs into a single object is a clean design that improves maintainability.
14-21: Destructured props increase readability.
Using object destructuring is a best practice that clarifies parameter usage.
27-28: Pass consistent role flags to userCanDoGroupActions.
Replacing older flags with currentAccountIsSuperAdmin and currentAccountIsAdmin ensures consistency with the new naming convention.
31-41: Appropriate checks before promoting to admin or super admin.
The conditions logically guard against self-modification or reassigning existing roles. Ensure tests cover edge cases (e.g., can a super admin promote another super admin?).
44-52: Revoke checks appear correct.
Verifying that it’s not the current user and roles are set correctly before revoking. This is consistent with the rest of your role logic.
55-55: canRevokeSuperAdmin logic.
The additional check that ensures isSuperAdmin and currentAccountIsSuperAdmin are both true is sensible.
84-84: Naming suggests "remove" but double-check edge cases.
“Remove from group” seems correct. Just confirm that super admins can or cannot remove themselves, if that scenario is valid.
utils/groupUtils/getGroupMemberActions.test.ts (6)
6-15: Single object parameter in tests improves maintainability.
This approach is more explicit, making it easier to see how booleans affect outcomes.
21-30: Promote to super admin scenario.
Ensures correct logic flow. Good coverage of the super admin case.
36-45: Revoke admin tests.
Properly checks the new parameter structure and ensures the correct result.
51-60: Revoke super admin coverage.
Verifies that “Revoke super admin” is displayed under proper conditions.
66-75: Remove from group scenario.
Covers a key permission boundary. This test ensures consistency with group removal logic.
81-90: Excluding admin actions for the current user.
Clear test ensuring that self-targeted promotions/removals do not appear, aligning with the logic in getGroupMemberActions.
features/conversation/conversation-message/conversation-message-content-types/conversation-message-chat-group-update.tsx (1)
115-115: Keep translations consistent with updated semantics.
Using "group_member_left" clarifies that a member has left, matching the action more closely than "group_member_joined." This improvement helps communicate the event accurately.
containers/GroupScreenMembersTable.tsx (2)
81-85: Validate currentAccountIsAdmin logic when group membership changes.
The useMemo hook is correct for performance, but ensure that getAddressIsAdmin is updated/invalidated whenever group membership or permissions change. This helps avoid stale values if membership updates occur quickly.
211-211: Include currentAccountIsAdmin in effect or dependency checks if needed.
You have added currentAccountIsAdmin to the dependencies, which is correct for recalculating tableViewItems. Confirm that any side-effects or triggered reactions also handle the updated admin state.
| @@ -1,6 +1,6 @@ | |||
| import type { PermissionPolicySet } from "@xmtp/react-native-sdk/build/lib/types/PermissionPolicySet"; | |||
|
|
|||
| type MemberRole = "admin" | "super_admin" | "member"; | |||
| type MemberRole = "admin" | "superAdmin" | "member"; | |||
There was a problem hiding this comment.
💡 Codebase verification
Update required: Multiple references to "super_admin" need to be changed
The codebase still contains multiple references to "super_admin" that need to be updated to maintain consistency with the new "superAdmin" naming convention. Here are the locations that require updates:
utils/groupUtils/getGroupMemberActions.ts: Translation key referencesutils/groupUtils/adminUtils.ts: Permission level checksutils/groupUtils/adminUtils.test.ts: Test casequeries/usePromoteToSuperAdminMutation.ts: Permission level assignmentscreens/Profile.tsx: Translation key referencescontainers/GroupScreenMembersTable.tsx: Translation key referencei18n/translations/en.tsandi18n/translations/fr.ts: Translation keys and values
🔗 Analysis chain
Renaming "super_admin" to "superAdmin" aligns with camelCase conventions.
This update maintains consistency. Ensure all references are updated.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify no lingering references to "super_admin" remain in the codebase.
rg "super_admin"
Length of output: 2014
Fixed usage of permission policies
Updated tests
Setup logic to automaitcally do the only action sheet option if available
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Refactor
Style