appwrite icon indicating copy to clipboard operation
appwrite copied to clipboard

Published 20 hours ago • verified publisher icon appwrite

Feat Sync team deletion

Open byawitz opened this issue 1 year ago • 6 comments

What does this PR do?

Changed team and organization deleting logic to the API instead of queuing it to the deleting worker

Test Plan

Tested against local 1.5.5 instance,

  1. All of the projects were pulled using the CLI and pushed into a new Organization with different project IDs.
  2. A few resources and files were added manually
  3. Delete organization, delete everything.

Related PRs and Issues

Close #7939

Checklist

  • [x] Have you read the Contributing Guidelines on issues?
  • [x] 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?

byawitz avatar May 29 '24 19:05 byawitz

@Meldiron, checked, the test is without sleep

https://github.com/appwrite/appwrite/blob/main/tests/e2e/Services/Teams/TeamsBase.php#L371-L388

byawitz avatar May 31 '24 15:05 byawitz

@Meldiron, checked, the test is without sleep

https://github.com/appwrite/appwrite/blob/main/tests/e2e/Services/Teams/TeamsBase.php#L371-L388

@byawitz, but that tests doesn't verify the memberships are deleted synchronously. Maybe you can add a users list memberships? Or at least manually test?

stnguyen90 avatar Jun 05 '24 00:06 stnguyen90

The test returns 404, which means the teams were deleted, and there's no way to delete them in an async way.

What should the test be different?

byawitz avatar Jun 05 '24 00:06 byawitz

The test returns 404, which means the teams were deleted, and there's no way to delete them in an async way.

Even without your change, the current test returns 404 so the current test doesn't actually test anything about memberships?

What should the test be different?

A test that verifies the memberships are deleted would be great. I'm not totally sure on how to test, though. Maybe users.listMemberships()?

stnguyen90 avatar Jun 05 '24 23:06 stnguyen90

The tests check if the team for a given ID exists. When the endpoint returns 404, it means the team no longer exists.

Do you mean to add a whole new test for userMemberships?

byawitz avatar Jun 25 '24 13:06 byawitz

Test added here

byawitz avatar Jun 26 '24 17:06 byawitz

:sparkles: Benchmark results

  • Requests per second: 1,681
  • Requests with 200 status code: 302,570
  • P99 latency: 0.079768253

:zap: Benchmark Comparison

Metric This PR Latest version
RPS 1,681 2,149
200 302,570 386,812
P99 0.079768253 0.075311548

github-actions[bot] avatar Aug 08 '24 14:08 github-actions[bot]