-
-
Notifications
You must be signed in to change notification settings - Fork 888
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature Added: An option for users to leave the Organization (Fixes PalisadoesFoundation/talawa-admin#1873) #2753
base: develop
Are you sure you want to change the base?
Conversation
WalkthroughThe changes made in this pull request involve modifications to the Changes
Assessment against linked issues
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
Documentation and Community
|
Our Pull Request Approval ProcessThanks for contributing! Testing Your CodeRemember, your PRs won't be reviewed until these criteria are met:
Our policies make our code better. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
CONTRIBUTING.mdRead our CONTRIBUTING.md file. Most importantly:
Other
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Congratulations on making your first PR! 🎊 If you haven't already, check out our Contributing Guidelines and PR Reporting Guidelines to ensure that you are following our guidelines for contributing and creating PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
tests/resolvers/Mutation/removeMember.spec.ts (1)
Line range hint
1-400
: Critical: Missing test coverage for new "Leave Organization" functionality.The test suite is missing essential test cases for the new feature that allows users to leave organizations without admin permissions. Please add test cases to cover:
- A regular user successfully leaving an organization
- Error cases specific to the leave organization functionality (e.g., attempting to leave an organization they're not part of)
- Edge cases (e.g., last member leaving an organization)
This is particularly important as it's the core functionality being added in PR #2753 (PalisadoesFoundation/talawa-admin#1873).
Would you like me to help generate the missing test cases? Here's a suggested structure:
it("should allow a regular user to leave an organization", async () => { const args: MutationRemoveMemberArgs = { data: { organizationId: testOrganization?.id, userId: testUsers[2]?._id, // Regular member }, }; const context = { userId: testUsers[2]?.id, // Same user (leaving themselves) }; // ... test implementation }); it("should throw error when user attempts to leave an organization they're not part of", async () => { // ... test implementation }); it("should handle edge case when the last member leaves an organization", async () => { // ... test implementation });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
src/resolvers/Mutation/removeMember.ts
(0 hunks)tests/resolvers/Mutation/removeMember.spec.ts
(1 hunks)
💤 Files with no reviewable changes (1)
- src/resolvers/Mutation/removeMember.ts
🔇 Additional comments (2)
tests/resolvers/Mutation/removeMember.spec.ts (2)
145-145
: LGTM: Error message translation looks correct.
The error message translation for organization not found error is properly implemented.
Line range hint 219-247
: Critical: Test case contradicts new feature requirements.
The test case "should throw admin cannot remove self error" contradicts the PR objective of allowing users to leave organizations. This test enforces that users cannot remove themselves, which is exactly what the new feature should allow.
This test case should either be:
- Removed if self-removal is now allowed for all users, or
- Modified to specifically test admin-specific scenarios if there are special cases for admins leaving organizations
Please clarify the intended behavior and update the test accordingly.
Let's verify the implementation in the resolver:
@raggettii Please fix the failing tests. |
What kind of change does this PR introduce?
This PR adds API functionality for allowing users to leave organizations without requiring admin permissions.
Issue Number: PalisadoesFoundation/talawa-admin#1873
Fixes PalisadoesFoundation/talawa-admin#1873
Did you add tests for your changes?
Yes, unit tests have been added to ensure that:
-The user can remove themselves from the organization without requiring admin privileges.
-The mutation correctly handles the leave organization functionality.
Snapshots/Videos:
Snapshots for the admin PR have been added; these are relevant to the backend functionality of the "Leave Organization" feature.
If relevant, did you update the documentation?
Summary
This change removes the requirement for admin privileges when a user tries to leave an organization, allowing users to leave without needing admin approval. This feature improves user autonomy and ensures that they can manage their membership within organizations. Additionally, tests have been added to verify that this functionality works as expected.
Does this PR introduce a breaking change?
No, this change does not introduce a breaking change. Existing functionality where an admin is required to remove users from an organization is preserved.
Other information
Have you read the contributing guide?
Yes
Summary by CodeRabbit
Bug Fixes
Tests