Fix deletion logic by ItzNotABug · Pull Request #9938 · appwrite/appwrite

ItzNotABug · 2025-05-30T12:37:16Z

What does this PR do?

Fix the deletion logic, for the last time I hope!

Test Plan

(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work. Screenshots may also be helpful.)

Related PRs and Issues

Checklist

Have you read the Contributing Guidelines on issues?
If the PR includes a change to an API's metadata (desc, label, params, etc.), does it also include updated API specs and example docs?

coderabbitai · 2025-05-30T12:37:23Z

Walkthrough

The changes refine the logic for deleting team memberships by ensuring that only users who are both the membership owner and acting on their own membership are restricted from deletion if they are the sole owner. Corresponding end-to-end tests are updated to reflect the revised behavior, particularly for non-owner users.

Changes

File(s)	Change Summary
app/controllers/api/teams.php	Updated logic to restrict deletion only if the user is both the sole owner and deleting their own owner membership. Variable renamed for clarity.
tests/e2e/Services/Teams/TeamsConsoleClientTest.php	Adjusted test expectations for non-owner user deletion: now expects success (204) and decremented count.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant API
    participant TeamMembership

    User->>API: Request to delete membership
    API->>TeamMembership: Check if user is sole owner and deleting own owner membership
    alt User is sole owner and deleting own owner membership
        API-->>User: Throw exception (deletion not allowed)
    else
        API->>TeamMembership: Proceed with deletion
        API-->>User: Respond with 204 No Content
    end

Possibly related PRs

Fix teams deletion #9888: Refines the condition for preventing deletion of a team membership by requiring the user to be both the membership user and an owner, similar to the changes in this PR.

Suggested reviewers

lohanidamodar
abnegate

Poem

A rabbit hopped through code so neat,
Tweaked the rules for teams to meet:
Owners stay when they're the last,
But non-owners now delete fast.
Tests were changed, the logic clean—
The happiest hop you’ve ever seen! 🐇

📜 Recent review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2af8b30 and 67cb3aa.

📒 Files selected for processing (2)

app/controllers/api/teams.php (1 hunks)
tests/e2e/Services/Teams/TeamsConsoleClientTest.php (1 hunks)

⏰ Context from checks skipped due to timeout of 90000ms (4)

GitHub Check: Setup & Build Appwrite Image
GitHub Check: Setup & Build Appwrite Image
GitHub Check: Linter
GitHub Check: scan

🔇 Additional comments (3)

app/controllers/api/teams.php (1)

1362-1367: LGTM! Improved deletion logic with better variable naming.

The refined logic is more precise and permissive in the right way:

Better variable naming: $isSelfOwner is more descriptive than $isCurrentUserAnOwner

More specific restriction: Now only prevents deletion when a user is trying to delete their own owner membership AND they're the sole owner

Allows valid use cases:

Owners can delete other members' memberships (including other owners if multiple exist)

Non-owners can delete their own memberships

Still prevents organizational lockout by protecting the sole owner

This change aligns well with typical team management expectations.

tests/e2e/Services/Teams/TeamsConsoleClientTest.php (2)

222-222: Test correctly updated to reflect the new deletion logic.

The status code change from 400 to 204 properly reflects that non-owner users can now successfully delete their own memberships, aligning with the refined business logic in the controller.

230-230: Membership count assertion updated correctly.

The expected total changing from 3 to 2 correctly reflects the successful deletion of the non-owner user's own membership, maintaining test accuracy with the updated deletion behavior.

✨ Finishing Touches

📝 Generate Docstrings

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.

❤️ Share

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Explain this complex logic.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai explain this code block.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read src/utils.ts and explain its main purpose.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai generate docstrings to generate docstrings for this PR.
@coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

github-actions · 2025-05-30T12:39:33Z

Security Scan Results for PR

Docker Image Scan Results

Package	Version	Vulnerability	Severity
binutils	2.42-r0	CVE-2025-0840	HIGH
libexpat	2.6.4-r0	CVE-2024-8176	HIGH
libxml2	2.12.7-r0	CVE-2024-56171	HIGH
libxml2	2.12.7-r0	CVE-2025-24928	HIGH
libxml2	2.12.7-r0	CVE-2025-27113	HIGH
libxml2	2.12.7-r0	CVE-2025-32414	HIGH
libxml2	2.12.7-r0	CVE-2025-32415	HIGH
sqlite-libs	3.45.3-r1	CVE-2025-29087	HIGH
xz	5.6.2-r0	CVE-2025-31115	HIGH
xz-libs	5.6.2-r0	CVE-2025-31115	HIGH
golang.org/x/crypto	v0.31.0	CVE-2025-22869	HIGH

Source Code Scan Results

🎉 No vulnerabilities found!

github-actions · 2025-05-30T12:55:48Z

✨ Benchmark results

Requests per second: 889
Requests with 200 status code: 160,128
P99 latency: 0.210066148

⚡ Benchmark Comparison

Metric	This PR	Latest version
RPS	889	1,067
200	160,128	192,132
P99	0.210066148	0.176829327

ItzNotABug added 2 commits May 30, 2025 18:04

fix: logic.

e1ee9ba

fix: test.

67cb3aa

ItzNotABug self-assigned this May 30, 2025

abnegate approved these changes May 30, 2025

View reviewed changes

abnegate merged commit 7e4a3bb into 1.7.x May 30, 2025
39 of 40 checks passed

abnegate deleted the fix-deletion-logic branch May 30, 2025 12:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix deletion logic#9938

Fix deletion logic#9938
abnegate merged 2 commits into1.7.xfrom
fix-deletion-logic

ItzNotABug commented May 30, 2025 •

edited

Loading

Uh oh!

coderabbitai bot commented May 30, 2025 •

edited

Loading

Chat

Support

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

Documentation and Community

Uh oh!

github-actions bot commented May 30, 2025

Uh oh!

github-actions bot commented May 30, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

ItzNotABug commented May 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Test Plan

Related PRs and Issues

Checklist

Uh oh!

coderabbitai bot commented May 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Possibly related PRs

Suggested reviewers

Poem

Chat

Support

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

Documentation and Community

Uh oh!

github-actions bot commented May 30, 2025

Security Scan Results for PR

Docker Image Scan Results

Source Code Scan Results

Uh oh!

github-actions bot commented May 30, 2025

✨ Benchmark results

⚡ Benchmark Comparison

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ItzNotABug commented May 30, 2025 •

edited

Loading

coderabbitai bot commented May 30, 2025 •

edited

Loading