From b3a30d99f0fa54670dedde47bfb16dfd2f19bb11 Mon Sep 17 00:00:00 2001 From: Edward Hibbert Date: Mon, 5 Jun 2023 09:42:16 +0100 Subject: [PATCH 1/5] Demote network coordinator. --- app/Http/Controllers/UserController.php | 10 ++++- .../Feature/Events/AddRemoveVolunteerTest.php | 2 +- tests/Feature/Networks/NetworkTest.php | 40 ++++++++++++++++--- tests/TestCase.php | 2 + 4 files changed, 47 insertions(+), 7 deletions(-) diff --git a/app/Http/Controllers/UserController.php b/app/Http/Controllers/UserController.php index 7995c57cd2..51cbedd665 100644 --- a/app/Http/Controllers/UserController.php +++ b/app/Http/Controllers/UserController.php @@ -373,11 +373,19 @@ public function postAdminEdit(Request $request) $user = User::find($user_id); + $oldRole = $user->role; + // Set role for User $user->update([ - 'role' => $request->input('user_role'), + 'role' => $request->input('user_role'), ]); + // If we are demoting from NetworkCoordinator to host, remove them from the list of coordinators for + // any networks they are currently coordinating. + if ($oldRole == Role::NETWORK_COORDINATOR && $user->role == Role::HOST) { + $user->networks()->detach(); + } + // The user may have previously been removed from the group, which will mean they have an entry in // users_groups with deleted_at set. Zap that if present so that sync() then works. sync() doesn't // handle soft deletes itself. diff --git a/tests/Feature/Events/AddRemoveVolunteerTest.php b/tests/Feature/Events/AddRemoveVolunteerTest.php index 88d34800f9..8f6588a0b3 100644 --- a/tests/Feature/Events/AddRemoveVolunteerTest.php +++ b/tests/Feature/Events/AddRemoveVolunteerTest.php @@ -198,7 +198,7 @@ public function testAdminRemoveReaddHost() { $response = $this->post('/profile/edit-admin-settings', [ '_token' => $tokenValue, 'id' => $host->id, - 'user_role' => 2, + 'user_role' => Role::ADMINISTRATOR, 'assigned_groups' => [ $idgroups ], diff --git a/tests/Feature/Networks/NetworkTest.php b/tests/Feature/Networks/NetworkTest.php index f563f4cd97..6aedc154cf 100644 --- a/tests/Feature/Networks/NetworkTest.php +++ b/tests/Feature/Networks/NetworkTest.php @@ -19,11 +19,6 @@ class NetworkTest extends TestCase protected function setUp(): void { parent::setUp(); - DB::statement('SET foreign_key_checks=0'); - Network::truncate(); - DB::delete('delete from user_network'); - DB::statement('SET foreign_key_checks=1'); - $this->networkService = new RepairNetworkService(); } @@ -330,4 +325,39 @@ public function admins_can_edit() $this->assertTrue($network->containsGroup($group)); $this->assertTrue($group->isMemberOf($network)); } + + public function testRemoveNetworkCoordinatorByRole() { + $this->withoutExceptionHandling(); + + $network = Network::factory()->create(); + + $admin = User::factory()->administrator()->create(); + $coordinator = User::factory()->networkCoordinator()->create(); + $network->addCoordinator($coordinator); + + $this->actingAs($admin); + + $response = $this->get('/user/edit/' . $coordinator->id); + $response->assertStatus(200); + + $crawler = new Crawler($response->getContent()); + + $tokens = $crawler->filter('input[name=_token]')->each(function (Crawler $node, $i) { + return $node; + }); + + $tokenValue = $tokens[0]->attr('value'); + + $response = $this->post('/profile/edit-admin-settings', [ + '_token' => $tokenValue, + 'id' => $coordinator->id, + 'assigned_groups' => [], + 'user_role' => Role::HOST, + ]); + $response->assertSessionHas('message'); + $this->assertTrue($response->isRedirection()); + + // Demoting to host should remove as a network coordinator. + $this->assertFalse($network->coordinators->contains($coordinator)); + } } diff --git a/tests/TestCase.php b/tests/TestCase.php index cb82ed3650..4fe386f7fd 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -94,6 +94,8 @@ protected function setUp(): void $network->name = 'Restarters'; $network->shortname = 'restarters'; $network->save(); + } else { + error_log("Got network"); } $this->withoutExceptionHandling(); From 6c845be9a8f1a133caf83ec9de458fe98dc8f3cc Mon Sep 17 00:00:00 2001 From: Edward Hibbert Date: Mon, 5 Jun 2023 09:43:33 +0100 Subject: [PATCH 2/5] Demote network coordinator. --- app/Http/Controllers/UserController.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/Http/Controllers/UserController.php b/app/Http/Controllers/UserController.php index 51cbedd665..8b2ce89958 100644 --- a/app/Http/Controllers/UserController.php +++ b/app/Http/Controllers/UserController.php @@ -380,9 +380,9 @@ public function postAdminEdit(Request $request) 'role' => $request->input('user_role'), ]); - // If we are demoting from NetworkCoordinator to host, remove them from the list of coordinators for + // If we are demoting from NetworkCoordinator, remove them from the list of coordinators for // any networks they are currently coordinating. - if ($oldRole == Role::NETWORK_COORDINATOR && $user->role == Role::HOST) { + if ($oldRole == Role::NETWORK_COORDINATOR && ($user->role == Role::HOST) || $user->role == Role::RESTARTER) { $user->networks()->detach(); } From 33e8166377a661925775759ebb43f63118d8d3f8 Mon Sep 17 00:00:00 2001 From: Edward Hibbert Date: Mon, 5 Jun 2023 10:20:09 +0100 Subject: [PATCH 3/5] Test fixes. --- tests/Feature/Events/AddRemoveVolunteerTest.php | 1 + tests/Feature/Networks/NetworkTest.php | 5 +---- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/Feature/Events/AddRemoveVolunteerTest.php b/tests/Feature/Events/AddRemoveVolunteerTest.php index 8f6588a0b3..5a047e91bd 100644 --- a/tests/Feature/Events/AddRemoveVolunteerTest.php +++ b/tests/Feature/Events/AddRemoveVolunteerTest.php @@ -9,6 +9,7 @@ use App\Notifications\AdminModerationEvent; use App\Notifications\NotifyRestartersOfNewEvent; use App\Party; +use App\Role; use App\User; use DB; use Faker\Generator as Faker; diff --git a/tests/Feature/Networks/NetworkTest.php b/tests/Feature/Networks/NetworkTest.php index 6aedc154cf..9b31092927 100644 --- a/tests/Feature/Networks/NetworkTest.php +++ b/tests/Feature/Networks/NetworkTest.php @@ -300,10 +300,7 @@ public function admins_can_edit() $admin = User::factory()->administrator()->create(); $this->actingAs($admin); - $network = new Network(); - $network->name = 'Restarters'; - $network->shortname = 'restarters'; - $network->save(); + $network = Network::where('shortname', 'restarters')->first(); $response = $this->get('/networks/' . $network->id . '/edit', $network->attributesToArray()); $response->assertSuccessful(); From e276705859e7729cfbbab14cb069fe310e05dd93 Mon Sep 17 00:00:00 2001 From: Edward Hibbert Date: Mon, 5 Jun 2023 10:59:00 +0100 Subject: [PATCH 4/5] Test change to help debug intermittent test failure. --- tests/Feature/Events/InviteEventTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Feature/Events/InviteEventTest.php b/tests/Feature/Events/InviteEventTest.php index cf8fe315e9..390f5e18f1 100644 --- a/tests/Feature/Events/InviteEventTest.php +++ b/tests/Feature/Events/InviteEventTest.php @@ -146,8 +146,8 @@ public function testInviteReal() // ...should show up in the list of events with an invitation as we have not yet accepted. $response3 = $this->get('/party'); $events = $this->getVueProperties($response3)[1][':initial-events']; - $this->assertNotFalse(strpos($events, '"attending":false')); - $this->assertNotFalse(strpos($events, '"invitation"')); + $this->assertStringContainsString('"attending":false', $events); + $this->assertStringContainsString('"invitation"', $events); // Now accept the invitation. $response4 = $this->get($invitation); From 77120b0fce6751263d37d7cdf84191ebec6f4fc1 Mon Sep 17 00:00:00 2001 From: Edward Hibbert Date: Mon, 12 Jun 2023 12:12:02 +0100 Subject: [PATCH 5/5] Review feedback. --- app/Http/Controllers/UserController.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/Http/Controllers/UserController.php b/app/Http/Controllers/UserController.php index 8b2ce89958..d52114b66e 100644 --- a/app/Http/Controllers/UserController.php +++ b/app/Http/Controllers/UserController.php @@ -382,7 +382,7 @@ public function postAdminEdit(Request $request) // If we are demoting from NetworkCoordinator, remove them from the list of coordinators for // any networks they are currently coordinating. - if ($oldRole == Role::NETWORK_COORDINATOR && ($user->role == Role::HOST) || $user->role == Role::RESTARTER) { + if ($oldRole == Role::NETWORK_COORDINATOR && ($user->role == Role::HOST || $user->role == Role::RESTARTER)) { $user->networks()->detach(); }