From 451f4bee19d31a1408049885a2f8fe6d3fc3faa5 Mon Sep 17 00:00:00 2001 From: Safwan Parkar Date: Fri, 4 Aug 2023 15:40:42 +0400 Subject: [PATCH] Fixed an incorrect test for team deletion 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. --- app/controllers/api/teams.php | 2 +- tests/e2e/Services/Teams/TeamsBase.php | 63 ------------- tests/e2e/Services/Teams/TeamsBaseServer.php | 98 ++++++++++++++++++++ 3 files changed, 99 insertions(+), 64 deletions(-) diff --git a/app/controllers/api/teams.php b/app/controllers/api/teams.php index 2b2f3b6920..5c5c128f57 100644 --- a/app/controllers/api/teams.php +++ b/app/controllers/api/teams.php @@ -343,8 +343,8 @@ App::delete('/v1/teams/:teamId') Query::limit(2000), // TODO fix members limit ]); + // Memberships are deleted here instead of in the worker to make sure user permisions are updated instantly foreach ($memberships as $membership) { - // Memberships are deleted here instead of in the worker to make sure user permisions are updated instantly if (!$dbForProject->deleteDocument('memberships', $membership->getId())) { throw new Exception(Exception::GENERAL_SERVER_ERROR, 'Failed to remove membership for team from DB'); } diff --git a/tests/e2e/Services/Teams/TeamsBase.php b/tests/e2e/Services/Teams/TeamsBase.php index 7d1e51c08c..8e19bede17 100644 --- a/tests/e2e/Services/Teams/TeamsBase.php +++ b/tests/e2e/Services/Teams/TeamsBase.php @@ -428,67 +428,4 @@ trait TeamsBase $this->assertEquals($user['headers']['status-code'], 400); } - - public function testTeamDeleteUpdatesUserMembership() - { - $users = []; - $team = null; - - $team = $this->client->call(Client::METHOD_POST, '/teams', array_merge([ - 'content-type' => 'application/json', - 'x-appwrite-project' => $this->getProject()['$id'], - ], $this->getHeaders()), [ - 'teamId' => ID::unique(), - 'name' => 'Demo' - ]); - - $this->assertEquals(201, $team['headers']['status-code']); - $this->assertNotEmpty($team['body']['$id']); - $this->assertEquals('Demo', $team['body']['name']); - $this->assertGreaterThan(-1, $team['body']['total']); - $this->assertIsInt($team['body']['total']); - - for ($i = 0; $i < 5; $i++) { - $mem = $this->client->call(Client::METHOD_POST, '/teams/' . $team['body']['$id'] . '/memberships', array_merge([ - 'content-type' => 'application/json', - 'x-appwrite-project' => $this->getProject()['$id'], - ], $this->getHeaders()), [ - 'email' => 'email' . $i . '@example.com', - 'roles' => ['admin', 'editor'], - 'name' => 'User ' . $i, - 'url' => 'http://localhost:5000/join-us#title' - ]); - - $this->assertEquals(201, $mem['headers']['status-code']); - $this->assertNotEmpty($mem['body']['$id']); - $this->assertNotEmpty($mem['body']['userId']); - $this->assertEquals('User ' . $i, $mem['body']['userName']); - $this->assertEquals('email' . $i . '@example.com', $mem['body']['userEmail']); - $this->assertNotEmpty($mem['body']['teamId']); - $this->assertCount(2, $mem['body']['roles']); - } - - $this->client->call(Client::METHOD_DELETE, '/teams/' . $team['body']['$id'], array_merge([ - 'content-type' => 'application/json', - 'x-appwrite-project' => $this->getProject()['$id'], - ], $this->getHeaders())); - - foreach ($users as $user) { - $user = $this->client->call(Client::METHOD_GET, '/users/' . $user['body']['$id'], array_merge([ - 'content-type' => 'application/json', - 'x-appwrite-project' => $this->getProject()['$id'], - ], $this->getHeaders())); - - $this->assertEquals(200, $user['headers']['status-code']); - $this->assertEquals(0, $user['body']['total']); - $this->assertEquals([], $user['body']['memberships']); - } - - $team = $this->client->call(Client::METHOD_GET, '/teams/' . $team['body']['$id'], array_merge([ - 'content-type' => 'application/json', - 'x-appwrite-project' => $this->getProject()['$id'], - ], $this->getHeaders())); - - $this->assertEquals(404, $team['headers']['status-code']); - } } diff --git a/tests/e2e/Services/Teams/TeamsBaseServer.php b/tests/e2e/Services/Teams/TeamsBaseServer.php index aa1be49f41..569170f890 100644 --- a/tests/e2e/Services/Teams/TeamsBaseServer.php +++ b/tests/e2e/Services/Teams/TeamsBaseServer.php @@ -4,6 +4,7 @@ namespace Tests\E2E\Services\Teams; use Tests\E2E\Client; use Utopia\Database\Validator\Datetime as DatetimeValidator; +use Utopia\Database\Helpers\ID; trait TeamsBaseServer { @@ -281,4 +282,101 @@ trait TeamsBaseServer $this->assertIsInt($response['body']['total']); $this->assertEquals(true, $dateValidator->isValid($response['body']['$createdAt'])); } + + public function testTeamDeleteUpdatesUserMembership() + { + $new_users = []; + + /** + * Create 5 new users and add their IDs to an array + */ + for ($i = 0; $i < 5; $i++) { + $user = $this->client->call(Client::METHOD_POST, '/users', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'userId' => ID::unique(), + 'email' => 'newuser' . $i . '@localhost.test', + 'password' => 'password', + 'name' => 'New User ' . $i, + ], false); + + $user_body = json_decode($user['body'], true); + + $this->assertEquals(201, $user['headers']['status-code']); + $this->assertEquals('newuser' . $i . '@localhost.test', $user_body['email']); + $this->assertEquals('New User ' . $i, $user_body['name']); + $this->assertEquals($user_body['status'], true); + + array_push($new_users, $user_body['$id']); + } + + /** + * Create a new team + */ + $new_team = $this->client->call(Client::METHOD_POST, '/teams', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'teamId' => ID::unique(), + 'name' => 'New Team Test', + 'roles' => ['admin', 'editor'], + ]); + + $this->assertEquals(201, $new_team['headers']['status-code']); + $this->assertNotEmpty($new_team['body']['$id']); + $this->assertEquals('New Team Test', $new_team['body']['name']); + $this->assertGreaterThan(-1, $new_team['body']['total']); + $this->assertIsInt($new_team['body']['total']); + $this->assertArrayHasKey('prefs', $new_team['body']); + + /** + * Create team memberships for each of the new users + */ + for ($i = 0; $i < 5; $i++) { + $new_membership = $this->client->call(Client::METHOD_POST, '/teams/' . $new_team['body']['$id'] . '/memberships', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'email' => 'newuser' . $i . '@localhost.test', + 'name' => 'New User ' . $i, + 'roles' => ['admin', 'editor'], + 'url' => 'http://localhost:5000/join-us#title' + ]); + + $this->assertEquals(201, $new_membership['headers']['status-code']); + $this->assertNotEmpty($new_membership['body']['$id']); + $this->assertNotEmpty($new_membership['body']['userId']); + $this->assertEquals('New User ' . $i, $new_membership['body']['userName']); + $this->assertEquals('newuser' . $i . '@localhost.test', $new_membership['body']['userEmail']); + $this->assertNotEmpty($new_membership['body']['teamId']); + $this->assertCount(2, $new_membership['body']['roles']); + $dateValidator = new DatetimeValidator(); + $this->assertEquals(true, $dateValidator->isValid($new_membership['body']['joined'])); + $this->assertEquals(true, $new_membership['body']['confirm']); + } + + /** + * Delete the team + */ + $team_del_response = $this->client->call(Client::METHOD_DELETE, '/teams/' . $new_team['body']['$id'], array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders())); + + $this->assertEquals(204, $team_del_response['headers']['status-code']); + + /** + * Check that the team memberships for each of the new users has been deleted + */ + for ($i = 0; $i < 5; $i++) { + $membership = $this->client->call(Client::METHOD_GET, '/users/' . $new_users[$i] . '/memberships', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders())); + + $this->assertEquals(200, $membership['headers']['status-code']); + $this->assertEquals(0, $membership['body']['total']); + } + } }