Fixed bug where memberships remained after a team is deleted #5928
Conversation
|
Hey @stnguyen90 just pushed a commit to this PR. Since there aren't any more API calls being made than there were originally, I think it's good. Regardless, I'm willing to learn and if you think I could have found a better solution, let me know!! |
- cache caused stale data in memberships
|
Just pushed a fix that definitely works 😶🌫️ HUGE thanks to @stnguyen90 for the absolutely amazing support he provided till now. I have learnt so much more about the system design of Appwrite, and was mesmerized to say the least. That being said, the fix was actually really simple, and I just needed to implement cache invalidation when a team is deleted. According to network timings in the dev tools, the cache invalidation has added ~0.4ms per user to the response time. With 7 users, it added 2.7ms, and with 2 users, it took less than 1ms. Feedback is appreciated! |
Will do that now |
|
Added a test to check the same and it was a success on localhost |
This commit contains changes in 3 places. - First I changed the placement of an informative comment in the DELETE TEAM controller, and moved it outside of the loop. I did this when Steven pointed out that the behaviour I describe in the comment is for the whole loop. - The second change is the removal of my first test. I was facing quite a few issues with creating users in the test, and ended up using the CREATE TEAM MEMBERSHIP to perform 2 actions at once -> create a new user if one doesn't exist with the provided email, and create a membership for the user. Before this approach, I had quite a bit of code that didn't work, and it seems like I removed some things that weren't supposed to be removed, and didn't change variable names where necessary. Anyway, I figured that the problem has something to do with the user being created on the client side, so I moved the test to the server side. - The new test I implemented does the same thing as my previous failed test, but in more detailed and distinct steps. The test first creates 5 new users inside of a loop, and pushes each new user's ID to an array called 'new_users' if the response is as expected. Then a new team is created. The next step is to create memberships for all 5 users. If all these steps pass, the new team that was just created, is deleted, and we check to make sure the new users have 0 team memberships each. Formatter and linter showed no errors. Tests were successful on localhost.
|
Re-requested review. Thanks for the helpful resources Steven, I honestly used to try and write descriptive commit messages, but that habit died down slowly lol. Will definitely start writing more descriptive commits from now on! |
The CREATE TEAM MEMBERSHIP endpoint requires the email of the user to be added to the team. If the user does not exist in the project, a new user is created with the specified email and added to the team. The first version of the test creates 5 users, and then adds them to the newly created team. This process is more streamlined now, by using the CREATE TEAM MEMBERSHIPS behaviour to create a user on the go and create a membership for them immediately after. I also change the way I add user IDs to the array, by using the shorthand notation instead of the `array_push` function.
Run the composer format and lint commands, which I forgot to run before.
Honestly forgot to run the formatter and linter. Fixed 1/1 issue and pushed. |
Huge thanks to you Steven! Really appreciate all the help! |
|
Thank you @safwanyp! |
What does this PR do?
This PR closes #5517.
Test Plan
Did a couple of tests by manually creating teams and memberships randomly. Kept track of the memberships and made sure they persisted and/or deleted as needed.
Related PRs and Issues
Checklist