From 7161b6416e3ac31d00062574b23e5205d50d8355 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 24 Jul 2024 14:16:42 -0700 Subject: [PATCH 01/31] Add failing test for accessories and consumables checkin --- .../Feature/Users/Ui/BulkDeleteUsersTest.php | 110 ++++++++++++++++++ 1 file changed, 110 insertions(+) create mode 100644 tests/Feature/Users/Ui/BulkDeleteUsersTest.php diff --git a/tests/Feature/Users/Ui/BulkDeleteUsersTest.php b/tests/Feature/Users/Ui/BulkDeleteUsersTest.php new file mode 100644 index 000000000..96798aae1 --- /dev/null +++ b/tests/Feature/Users/Ui/BulkDeleteUsersTest.php @@ -0,0 +1,110 @@ +count(2)->create(); + [$userA, $userB] = User::factory()->count(2)->create(); + + // Add checkouts for multiple accessories to multiple users to get different ids in the mix + $this->attachAccessoryToUser($accessoryA, $userA); + $this->attachAccessoryToUser($accessoryA, $userB); + $this->attachAccessoryToUser($accessoryB, $userA); + $this->attachAccessoryToUser($accessoryB, $userB); + + $this->actingAs(User::factory()->editUsers()->create()) + ->post(route('users/bulksave'), [ + 'ids' => [ + $userA->id, + ], + 'status_id' => Statuslabel::factory()->create()->id, + ]) + ->assertRedirect(); + + // These assertions check against a bug where the wrong value from + // accessories_users was being populated in action_logs.item_id. + $this->assertDatabaseHas('action_logs', [ + 'action_type' => 'checkin from', + 'target_id' => $userA->id, + 'target_type' => User::class, + 'note' => 'Bulk checkin items', + 'item_type' => Accessory::class, + 'item_id' => $accessoryA->id, + ]); + + $this->assertDatabaseHas('action_logs', [ + 'action_type' => 'checkin from', + 'target_id' => $userA->id, + 'target_type' => User::class, + 'note' => 'Bulk checkin items', + 'item_type' => Accessory::class, + 'item_id' => $accessoryB->id, + ]); + } + + public function testConsumableCheckinsAreProperlyLogged() + { + [$consumableA, $consumableB] = Consumable::factory()->count(2)->create(); + [$userA, $userB] = User::factory()->count(2)->create(); + + // Add checkouts for multiple consumables to multiple users to get different ids in the mix + $this->attachConsumableToUser($consumableA, $userA); + $this->attachConsumableToUser($consumableA, $userB); + $this->attachConsumableToUser($consumableB, $userA); + $this->attachConsumableToUser($consumableB, $userB); + + $this->actingAs(User::factory()->editUsers()->create()) + ->post(route('users/bulksave'), [ + 'ids' => [ + $userA->id, + ], + 'status_id' => Statuslabel::factory()->create()->id, + ]) + ->assertRedirect(); + + // These assertions check against a bug where the wrong value from + // consumables_users was being populated in action_logs.item_id. + $this->assertDatabaseHas('action_logs', [ + 'action_type' => 'checkin from', + 'target_id' => $userA->id, + 'target_type' => User::class, + 'note' => 'Bulk checkin items', + 'item_type' => Consumable::class, + 'item_id' => $consumableA->id, + ]); + + $this->assertDatabaseHas('action_logs', [ + 'action_type' => 'checkin from', + 'target_id' => $userA->id, + 'target_type' => User::class, + 'note' => 'Bulk checkin items', + 'item_type' => Consumable::class, + 'item_id' => $consumableB->id, + ]); + } + + private function attachAccessoryToUser(Accessory $accessory, User $user): void + { + $accessory->users()->attach($accessory->id, [ + 'accessory_id' => $accessory->id, + 'assigned_to' => $user->id, + ]); + } + + private function attachConsumableToUser(Consumable $consumable, User $user): void + { + $consumable->users()->attach($consumable->id, [ + 'consumable_id' => $consumable->id, + 'assigned_to' => $user->id, + ]); + } +} From 44dbbeb608fe2cf8eb6dd514577ebc86f88da7e7 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 24 Jul 2024 14:17:49 -0700 Subject: [PATCH 02/31] Add accessory and consumable specific checkin methods --- .../Controllers/Users/BulkUsersController.php | 35 +++++++++++++++++-- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/app/Http/Controllers/Users/BulkUsersController.php b/app/Http/Controllers/Users/BulkUsersController.php index b0683e2cb..fab13f3fa 100644 --- a/app/Http/Controllers/Users/BulkUsersController.php +++ b/app/Http/Controllers/Users/BulkUsersController.php @@ -16,6 +16,7 @@ use App\Models\Consumable; use App\Models\User; use Carbon\Carbon; use Illuminate\Http\Request; +use Illuminate\Support\Collection; use Illuminate\Support\Facades\Auth; use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Password; @@ -229,9 +230,9 @@ class BulkUsersController extends Controller $this->logItemCheckinAndDelete($assets, Asset::class); - $this->logItemCheckinAndDelete($accessories, Accessory::class); + $this->logAccessoriesCheckin($accessories); $this->logItemCheckinAndDelete($licenses, License::class); - $this->logItemCheckinAndDelete($consumables, Consumable::class); + $this->logConsumablesCheckin($consumables); Asset::whereIn('id', $assets->pluck('id'))->update([ @@ -279,7 +280,7 @@ class BulkUsersController extends Controller if ($itemType == License::class){ $item_id = $item->license_id; } - + $logAction->item_id = $item_id; // We can't rely on get_class here because the licenses/accessories fetched above are not eloquent models, but simply arrays. $logAction->item_type = $itemType; @@ -291,6 +292,34 @@ class BulkUsersController extends Controller } } + private function logAccessoriesCheckin(Collection $accessories): void + { + foreach ($accessories as $accessory) { + $logAction = new Actionlog(); + $logAction->item_id = $accessory->accessory_id; + $logAction->item_type = Accessory::class; + $logAction->target_id = $accessory->assigned_to; + $logAction->target_type = User::class; + $logAction->user_id = Auth::id(); + $logAction->note = 'Bulk checkin items'; + $logAction->logaction('checkin from'); + } + } + + private function logConsumablesCheckin(Collection $consumables): void + { + foreach ($consumables as $consumable) { + $logAction = new Actionlog(); + $logAction->item_id = $consumable->consumable_id; + $logAction->item_type = Consumable::class; + $logAction->target_id = $consumable->assigned_to; + $logAction->target_type = User::class; + $logAction->user_id = Auth::id(); + $logAction->note = 'Bulk checkin items'; + $logAction->logaction('checkin from'); + } + } + /** * Save bulk-edited users * From 78a0417ee9ae656afd37ef69c86b7ac368fbe9c4 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Mon, 5 Aug 2024 15:11:18 -0700 Subject: [PATCH 03/31] Add another user into the mix --- .../Feature/Users/Ui/BulkDeleteUsersTest.php | 28 +++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/tests/Feature/Users/Ui/BulkDeleteUsersTest.php b/tests/Feature/Users/Ui/BulkDeleteUsersTest.php index 96798aae1..a88334e37 100644 --- a/tests/Feature/Users/Ui/BulkDeleteUsersTest.php +++ b/tests/Feature/Users/Ui/BulkDeleteUsersTest.php @@ -13,11 +13,13 @@ class BulkDeleteUsersTest extends TestCase public function testAccessoryCheckinsAreProperlyLogged() { [$accessoryA, $accessoryB] = Accessory::factory()->count(2)->create(); - [$userA, $userB] = User::factory()->count(2)->create(); + [$userA, $userB, $userC] = User::factory()->count(3)->create(); // Add checkouts for multiple accessories to multiple users to get different ids in the mix $this->attachAccessoryToUser($accessoryA, $userA); $this->attachAccessoryToUser($accessoryA, $userB); + $this->attachAccessoryToUser($accessoryA, $userC); + $this->attachAccessoryToUser($accessoryB, $userA); $this->attachAccessoryToUser($accessoryB, $userB); @@ -25,6 +27,7 @@ class BulkDeleteUsersTest extends TestCase ->post(route('users/bulksave'), [ 'ids' => [ $userA->id, + $userC->id, ], 'status_id' => Statuslabel::factory()->create()->id, ]) @@ -49,16 +52,27 @@ class BulkDeleteUsersTest extends TestCase 'item_type' => Accessory::class, 'item_id' => $accessoryB->id, ]); + + $this->assertDatabaseHas('action_logs', [ + 'action_type' => 'checkin from', + 'target_id' => $userC->id, + 'target_type' => User::class, + 'note' => 'Bulk checkin items', + 'item_type' => Accessory::class, + 'item_id' => $accessoryA->id, + ]); } public function testConsumableCheckinsAreProperlyLogged() { [$consumableA, $consumableB] = Consumable::factory()->count(2)->create(); - [$userA, $userB] = User::factory()->count(2)->create(); + [$userA, $userB, $userC] = User::factory()->count(3)->create(); // Add checkouts for multiple consumables to multiple users to get different ids in the mix $this->attachConsumableToUser($consumableA, $userA); $this->attachConsumableToUser($consumableA, $userB); + $this->attachConsumableToUser($consumableA, $userC); + $this->attachConsumableToUser($consumableB, $userA); $this->attachConsumableToUser($consumableB, $userB); @@ -66,6 +80,7 @@ class BulkDeleteUsersTest extends TestCase ->post(route('users/bulksave'), [ 'ids' => [ $userA->id, + $userC->id, ], 'status_id' => Statuslabel::factory()->create()->id, ]) @@ -90,6 +105,15 @@ class BulkDeleteUsersTest extends TestCase 'item_type' => Consumable::class, 'item_id' => $consumableB->id, ]); + + $this->assertDatabaseHas('action_logs', [ + 'action_type' => 'checkin from', + 'target_id' => $userC->id, + 'target_type' => User::class, + 'note' => 'Bulk checkin items', + 'item_type' => Consumable::class, + 'item_id' => $consumableA->id, + ]); } private function attachAccessoryToUser(Accessory $accessory, User $user): void From 480e4f3a691e9ccc6ea7159c632e2062299a0c1c Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Mon, 5 Aug 2024 15:16:40 -0700 Subject: [PATCH 04/31] Improve readability --- .../Feature/Users/Ui/BulkDeleteUsersTest.php | 40 +++++++++---------- 1 file changed, 18 insertions(+), 22 deletions(-) diff --git a/tests/Feature/Users/Ui/BulkDeleteUsersTest.php b/tests/Feature/Users/Ui/BulkDeleteUsersTest.php index a88334e37..36526318e 100644 --- a/tests/Feature/Users/Ui/BulkDeleteUsersTest.php +++ b/tests/Feature/Users/Ui/BulkDeleteUsersTest.php @@ -16,12 +16,8 @@ class BulkDeleteUsersTest extends TestCase [$userA, $userB, $userC] = User::factory()->count(3)->create(); // Add checkouts for multiple accessories to multiple users to get different ids in the mix - $this->attachAccessoryToUser($accessoryA, $userA); - $this->attachAccessoryToUser($accessoryA, $userB); - $this->attachAccessoryToUser($accessoryA, $userC); - - $this->attachAccessoryToUser($accessoryB, $userA); - $this->attachAccessoryToUser($accessoryB, $userB); + $this->attachAccessoryToUsers($accessoryA, [$userA, $userB, $userC]); + $this->attachAccessoryToUsers($accessoryB, [$userA, $userB]); $this->actingAs(User::factory()->editUsers()->create()) ->post(route('users/bulksave'), [ @@ -69,12 +65,8 @@ class BulkDeleteUsersTest extends TestCase [$userA, $userB, $userC] = User::factory()->count(3)->create(); // Add checkouts for multiple consumables to multiple users to get different ids in the mix - $this->attachConsumableToUser($consumableA, $userA); - $this->attachConsumableToUser($consumableA, $userB); - $this->attachConsumableToUser($consumableA, $userC); - - $this->attachConsumableToUser($consumableB, $userA); - $this->attachConsumableToUser($consumableB, $userB); + $this->attachConsumableToUsers($consumableA, [$userA, $userB, $userC]); + $this->attachConsumableToUsers($consumableB, [$userA, $userB]); $this->actingAs(User::factory()->editUsers()->create()) ->post(route('users/bulksave'), [ @@ -116,19 +108,23 @@ class BulkDeleteUsersTest extends TestCase ]); } - private function attachAccessoryToUser(Accessory $accessory, User $user): void + private function attachAccessoryToUsers(Accessory $accessory, array $users): void { - $accessory->users()->attach($accessory->id, [ - 'accessory_id' => $accessory->id, - 'assigned_to' => $user->id, - ]); + foreach ($users as $user) { + $accessory->users()->attach($accessory->id, [ + 'accessory_id' => $accessory->id, + 'assigned_to' => $user->id, + ]); + } } - private function attachConsumableToUser(Consumable $consumable, User $user): void + private function attachConsumableToUsers(Consumable $consumable, array $users): void { - $consumable->users()->attach($consumable->id, [ - 'consumable_id' => $consumable->id, - 'assigned_to' => $user->id, - ]); + foreach ($users as $user) { + $consumable->users()->attach($consumable->id, [ + 'consumable_id' => $consumable->id, + 'assigned_to' => $user->id, + ]); + } } } From bfebcdc7ed3e6b126259d09999ff78a9eb253c07 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Mon, 5 Aug 2024 15:22:35 -0700 Subject: [PATCH 05/31] Improve variable name --- .../Controllers/Users/BulkUsersController.php | 28 +++++++++---------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/app/Http/Controllers/Users/BulkUsersController.php b/app/Http/Controllers/Users/BulkUsersController.php index fab13f3fa..81b19b1c5 100644 --- a/app/Http/Controllers/Users/BulkUsersController.php +++ b/app/Http/Controllers/Users/BulkUsersController.php @@ -220,20 +220,18 @@ class BulkUsersController extends Controller $users = User::whereIn('id', $user_raw_array)->get(); $assets = Asset::whereIn('assigned_to', $user_raw_array)->where('assigned_type', \App\Models\User::class)->get(); - $accessories = DB::table('accessories_users')->whereIn('assigned_to', $user_raw_array)->get(); + $accessoryUserRows = DB::table('accessories_users')->whereIn('assigned_to', $user_raw_array)->get(); $licenses = DB::table('license_seats')->whereIn('assigned_to', $user_raw_array)->get(); - $consumables = DB::table('consumables_users')->whereIn('assigned_to', $user_raw_array)->get(); + $consumableUserRows = DB::table('consumables_users')->whereIn('assigned_to', $user_raw_array)->get(); if ((($assets->count() > 0) && ((!$request->filled('status_id')) || ($request->input('status_id') == '')))) { return redirect()->route('users.index')->with('error', 'No status selected'); } - $this->logItemCheckinAndDelete($assets, Asset::class); - $this->logAccessoriesCheckin($accessories); + $this->logAccessoriesCheckin($accessoryUserRows); $this->logItemCheckinAndDelete($licenses, License::class); - $this->logConsumablesCheckin($consumables); - + $this->logConsumablesCheckin($consumableUserRows); Asset::whereIn('id', $assets->pluck('id'))->update([ 'status_id' => e(request('status_id')), @@ -244,7 +242,7 @@ class BulkUsersController extends Controller LicenseSeat::whereIn('id', $licenses->pluck('id'))->update(['assigned_to' => null]); - ConsumableAssignment::whereIn('id', $consumables->pluck('id'))->delete(); + ConsumableAssignment::whereIn('id', $consumableUserRows->pluck('id'))->delete(); foreach ($users as $user) { @@ -292,13 +290,13 @@ class BulkUsersController extends Controller } } - private function logAccessoriesCheckin(Collection $accessories): void + private function logAccessoriesCheckin(Collection $accessoryUserRows): void { - foreach ($accessories as $accessory) { + foreach ($accessoryUserRows as $accessoryUserRow) { $logAction = new Actionlog(); - $logAction->item_id = $accessory->accessory_id; + $logAction->item_id = $accessoryUserRow->accessory_id; $logAction->item_type = Accessory::class; - $logAction->target_id = $accessory->assigned_to; + $logAction->target_id = $accessoryUserRow->assigned_to; $logAction->target_type = User::class; $logAction->user_id = Auth::id(); $logAction->note = 'Bulk checkin items'; @@ -306,13 +304,13 @@ class BulkUsersController extends Controller } } - private function logConsumablesCheckin(Collection $consumables): void + private function logConsumablesCheckin(Collection $consumableUserRows): void { - foreach ($consumables as $consumable) { + foreach ($consumableUserRows as $consumableUserRow) { $logAction = new Actionlog(); - $logAction->item_id = $consumable->consumable_id; + $logAction->item_id = $consumableUserRow->consumable_id; $logAction->item_type = Consumable::class; - $logAction->target_id = $consumable->assigned_to; + $logAction->target_id = $consumableUserRow->assigned_to; $logAction->target_type = User::class; $logAction->user_id = Auth::id(); $logAction->note = 'Bulk checkin items'; From 364775dcfe8a4e9fa8d4908456cf71fc49462354 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Mon, 5 Aug 2024 15:42:03 -0700 Subject: [PATCH 06/31] Improve readability --- .../Feature/Users/Ui/BulkDeleteUsersTest.php | 71 +++++-------------- 1 file changed, 19 insertions(+), 52 deletions(-) diff --git a/tests/Feature/Users/Ui/BulkDeleteUsersTest.php b/tests/Feature/Users/Ui/BulkDeleteUsersTest.php index 36526318e..2da5cd907 100644 --- a/tests/Feature/Users/Ui/BulkDeleteUsersTest.php +++ b/tests/Feature/Users/Ui/BulkDeleteUsersTest.php @@ -6,6 +6,7 @@ use App\Models\Accessory; use App\Models\Consumable; use App\Models\Statuslabel; use App\Models\User; +use Illuminate\Database\Eloquent\Model; use Tests\TestCase; class BulkDeleteUsersTest extends TestCase @@ -31,32 +32,9 @@ class BulkDeleteUsersTest extends TestCase // These assertions check against a bug where the wrong value from // accessories_users was being populated in action_logs.item_id. - $this->assertDatabaseHas('action_logs', [ - 'action_type' => 'checkin from', - 'target_id' => $userA->id, - 'target_type' => User::class, - 'note' => 'Bulk checkin items', - 'item_type' => Accessory::class, - 'item_id' => $accessoryA->id, - ]); - - $this->assertDatabaseHas('action_logs', [ - 'action_type' => 'checkin from', - 'target_id' => $userA->id, - 'target_type' => User::class, - 'note' => 'Bulk checkin items', - 'item_type' => Accessory::class, - 'item_id' => $accessoryB->id, - ]); - - $this->assertDatabaseHas('action_logs', [ - 'action_type' => 'checkin from', - 'target_id' => $userC->id, - 'target_type' => User::class, - 'note' => 'Bulk checkin items', - 'item_type' => Accessory::class, - 'item_id' => $accessoryA->id, - ]); + $this->assertActionLogCheckInEntryFor($userA, $accessoryA); + $this->assertActionLogCheckInEntryFor($userA, $accessoryB); + $this->assertActionLogCheckInEntryFor($userC, $accessoryA); } public function testConsumableCheckinsAreProperlyLogged() @@ -80,32 +58,9 @@ class BulkDeleteUsersTest extends TestCase // These assertions check against a bug where the wrong value from // consumables_users was being populated in action_logs.item_id. - $this->assertDatabaseHas('action_logs', [ - 'action_type' => 'checkin from', - 'target_id' => $userA->id, - 'target_type' => User::class, - 'note' => 'Bulk checkin items', - 'item_type' => Consumable::class, - 'item_id' => $consumableA->id, - ]); - - $this->assertDatabaseHas('action_logs', [ - 'action_type' => 'checkin from', - 'target_id' => $userA->id, - 'target_type' => User::class, - 'note' => 'Bulk checkin items', - 'item_type' => Consumable::class, - 'item_id' => $consumableB->id, - ]); - - $this->assertDatabaseHas('action_logs', [ - 'action_type' => 'checkin from', - 'target_id' => $userC->id, - 'target_type' => User::class, - 'note' => 'Bulk checkin items', - 'item_type' => Consumable::class, - 'item_id' => $consumableA->id, - ]); + $this->assertActionLogCheckInEntryFor($userA, $consumableA); + $this->assertActionLogCheckInEntryFor($userA, $consumableB); + $this->assertActionLogCheckInEntryFor($userC, $consumableA); } private function attachAccessoryToUsers(Accessory $accessory, array $users): void @@ -127,4 +82,16 @@ class BulkDeleteUsersTest extends TestCase ]); } } + + private function assertActionLogCheckInEntryFor(User $user, Model $model): void + { + $this->assertDatabaseHas('action_logs', [ + 'action_type' => 'checkin from', + 'target_id' => $user->id, + 'target_type' => User::class, + 'note' => 'Bulk checkin items', + 'item_type' => get_class($model), + 'item_id' => $model->id, + ]); + } } From 4ed3347f52295b73876bfb6a103d987956b5224c Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Mon, 5 Aug 2024 15:43:38 -0700 Subject: [PATCH 07/31] Add permission check --- tests/Feature/Users/Ui/BulkDeleteUsersTest.php | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/Feature/Users/Ui/BulkDeleteUsersTest.php b/tests/Feature/Users/Ui/BulkDeleteUsersTest.php index 2da5cd907..cf283ad4c 100644 --- a/tests/Feature/Users/Ui/BulkDeleteUsersTest.php +++ b/tests/Feature/Users/Ui/BulkDeleteUsersTest.php @@ -11,6 +11,18 @@ use Tests\TestCase; class BulkDeleteUsersTest extends TestCase { + public function testRequiresCorrectPermission() + { + $this->actingAs(User::factory()->create()) + ->post(route('users/bulksave'), [ + 'ids' => [ + User::factory()->create()->id, + ], + 'status_id' => Statuslabel::factory()->create()->id, + ]) + ->assertForbidden(); + } + public function testAccessoryCheckinsAreProperlyLogged() { [$accessoryA, $accessoryB] = Accessory::factory()->count(2)->create(); From 5ecdb7e07c1727af1aa5ceb377d4918e2ffdcf54 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Mon, 5 Aug 2024 15:50:09 -0700 Subject: [PATCH 08/31] Add validation test --- .../Feature/Users/Ui/BulkDeleteUsersTest.php | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/Feature/Users/Ui/BulkDeleteUsersTest.php b/tests/Feature/Users/Ui/BulkDeleteUsersTest.php index cf283ad4c..01e16d502 100644 --- a/tests/Feature/Users/Ui/BulkDeleteUsersTest.php +++ b/tests/Feature/Users/Ui/BulkDeleteUsersTest.php @@ -3,6 +3,7 @@ namespace Tests\Feature\Users\Ui; use App\Models\Accessory; +use App\Models\Asset; use App\Models\Consumable; use App\Models\Statuslabel; use App\Models\User; @@ -23,6 +24,31 @@ class BulkDeleteUsersTest extends TestCase ->assertForbidden(); } + public function testValidation() + { + // $this->markTestIncomplete(); + $user = User::factory()->create(); + Asset::factory()->assignedToUser($user)->create(); + + $actor = $this->actingAs(User::factory()->editUsers()->create()); + + // "ids" required + $actor->post(route('users/bulksave'), [ + // 'ids' => [ + // $user->id, + // ], + 'status_id' => Statuslabel::factory()->create()->id, + ])->assertSessionHas('error')->assertRedirect(); + + // "status_id" needed when provided users have assets associated + $actor->post(route('users/bulksave'), [ + 'ids' => [ + $user->id, + ], + // 'status_id' => Statuslabel::factory()->create()->id, + ])->assertSessionHas('error')->assertRedirect(); + } + public function testAccessoryCheckinsAreProperlyLogged() { [$accessoryA, $accessoryB] = Accessory::factory()->count(2)->create(); From e3049fffd42baf54f8cbaad29933e45a398ce4b2 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Mon, 5 Aug 2024 15:50:53 -0700 Subject: [PATCH 09/31] Remove comment --- tests/Feature/Users/Ui/BulkDeleteUsersTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/Feature/Users/Ui/BulkDeleteUsersTest.php b/tests/Feature/Users/Ui/BulkDeleteUsersTest.php index 01e16d502..6b4621daf 100644 --- a/tests/Feature/Users/Ui/BulkDeleteUsersTest.php +++ b/tests/Feature/Users/Ui/BulkDeleteUsersTest.php @@ -26,7 +26,6 @@ class BulkDeleteUsersTest extends TestCase public function testValidation() { - // $this->markTestIncomplete(); $user = User::factory()->create(); Asset::factory()->assignedToUser($user)->create(); From b06501dd0288938e5533fd7950603a837678a558 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Mon, 5 Aug 2024 16:02:59 -0700 Subject: [PATCH 10/31] Add assertions --- .../Feature/Users/Ui/BulkDeleteUsersTest.php | 20 +++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/tests/Feature/Users/Ui/BulkDeleteUsersTest.php b/tests/Feature/Users/Ui/BulkDeleteUsersTest.php index 6b4621daf..be5af79e0 100644 --- a/tests/Feature/Users/Ui/BulkDeleteUsersTest.php +++ b/tests/Feature/Users/Ui/BulkDeleteUsersTest.php @@ -48,7 +48,7 @@ class BulkDeleteUsersTest extends TestCase ])->assertSessionHas('error')->assertRedirect(); } - public function testAccessoryCheckinsAreProperlyLogged() + public function testAccessoriesCanBeBulkCheckedIn() { [$accessoryA, $accessoryB] = Accessory::factory()->count(2)->create(); [$userA, $userB, $userC] = User::factory()->count(3)->create(); @@ -57,6 +57,10 @@ class BulkDeleteUsersTest extends TestCase $this->attachAccessoryToUsers($accessoryA, [$userA, $userB, $userC]); $this->attachAccessoryToUsers($accessoryB, [$userA, $userB]); + $this->assertTrue($userA->accessories->isNotEmpty()); + $this->assertTrue($userB->accessories->isNotEmpty()); + $this->assertTrue($userC->accessories->isNotEmpty()); + $this->actingAs(User::factory()->editUsers()->create()) ->post(route('users/bulksave'), [ 'ids' => [ @@ -67,6 +71,10 @@ class BulkDeleteUsersTest extends TestCase ]) ->assertRedirect(); + $this->assertTrue($userA->fresh()->accessories->isEmpty()); + $this->assertTrue($userB->fresh()->accessories->isNotEmpty()); + $this->assertTrue($userC->fresh()->accessories->isEmpty()); + // These assertions check against a bug where the wrong value from // accessories_users was being populated in action_logs.item_id. $this->assertActionLogCheckInEntryFor($userA, $accessoryA); @@ -74,7 +82,7 @@ class BulkDeleteUsersTest extends TestCase $this->assertActionLogCheckInEntryFor($userC, $accessoryA); } - public function testConsumableCheckinsAreProperlyLogged() + public function testConsumablesCanBeBulkCheckedIn() { [$consumableA, $consumableB] = Consumable::factory()->count(2)->create(); [$userA, $userB, $userC] = User::factory()->count(3)->create(); @@ -83,6 +91,10 @@ class BulkDeleteUsersTest extends TestCase $this->attachConsumableToUsers($consumableA, [$userA, $userB, $userC]); $this->attachConsumableToUsers($consumableB, [$userA, $userB]); + $this->assertTrue($userA->consumables->isNotEmpty()); + $this->assertTrue($userB->consumables->isNotEmpty()); + $this->assertTrue($userC->consumables->isNotEmpty()); + $this->actingAs(User::factory()->editUsers()->create()) ->post(route('users/bulksave'), [ 'ids' => [ @@ -93,6 +105,10 @@ class BulkDeleteUsersTest extends TestCase ]) ->assertRedirect(); + $this->assertTrue($userA->fresh()->consumables->isEmpty()); + $this->assertTrue($userB->fresh()->consumables->isNotEmpty()); + $this->assertTrue($userC->fresh()->consumables->isEmpty()); + // These assertions check against a bug where the wrong value from // consumables_users was being populated in action_logs.item_id. $this->assertActionLogCheckInEntryFor($userA, $consumableA); From 67b3ab820fbebd16e40661654a98e4d60d3f2c46 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Mon, 5 Aug 2024 16:10:08 -0700 Subject: [PATCH 11/31] Add todo comments --- app/Http/Controllers/Users/BulkUsersController.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/Http/Controllers/Users/BulkUsersController.php b/app/Http/Controllers/Users/BulkUsersController.php index 81b19b1c5..c2cc8607f 100644 --- a/app/Http/Controllers/Users/BulkUsersController.php +++ b/app/Http/Controllers/Users/BulkUsersController.php @@ -247,7 +247,9 @@ class BulkUsersController extends Controller foreach ($users as $user) { + // @todo: This can be deleted since we have ConsumableAssignment above right? $user->consumables()->sync([]); + // @todo: Can this be removed if we introduce AccessoryAssignment? $user->accessories()->sync([]); if ($request->input('delete_user')=='1') { $user->delete(); From 8a206a6d92441873085f0d112856c79748655275 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Mon, 5 Aug 2024 16:10:12 -0700 Subject: [PATCH 12/31] Add assertion --- .../Feature/Users/Ui/BulkDeleteUsersTest.php | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/Feature/Users/Ui/BulkDeleteUsersTest.php b/tests/Feature/Users/Ui/BulkDeleteUsersTest.php index be5af79e0..8a9d24bae 100644 --- a/tests/Feature/Users/Ui/BulkDeleteUsersTest.php +++ b/tests/Feature/Users/Ui/BulkDeleteUsersTest.php @@ -116,6 +116,25 @@ class BulkDeleteUsersTest extends TestCase $this->assertActionLogCheckInEntryFor($userC, $consumableA); } + public function testUsersCanBeDeletedInBulk() + { + [$userA, $userB, $userC] = User::factory()->count(3)->create(); + + $this->actingAs(User::factory()->editUsers()->create()) + ->post(route('users/bulksave'), [ + 'ids' => [ + $userA->id, + $userC->id, + ], + 'delete_user' => '1', + ]) + ->assertRedirect(); + + $this->assertSoftDeleted($userA); + $this->assertNotSoftDeleted($userB); + $this->assertSoftDeleted($userC); + } + private function attachAccessoryToUsers(Accessory $accessory, array $users): void { foreach ($users as $user) { From 5786ff5035a7fa48648dd70d26c655f9635124ee Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Mon, 5 Aug 2024 16:11:07 -0700 Subject: [PATCH 13/31] Add assertion for success message --- tests/Feature/Users/Ui/BulkDeleteUsersTest.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/Feature/Users/Ui/BulkDeleteUsersTest.php b/tests/Feature/Users/Ui/BulkDeleteUsersTest.php index 8a9d24bae..f321e011f 100644 --- a/tests/Feature/Users/Ui/BulkDeleteUsersTest.php +++ b/tests/Feature/Users/Ui/BulkDeleteUsersTest.php @@ -128,7 +128,8 @@ class BulkDeleteUsersTest extends TestCase ], 'delete_user' => '1', ]) - ->assertRedirect(); + ->assertRedirect() + ->assertSessionHas('success', trans('general.bulk_checkin_delete_success')); $this->assertSoftDeleted($userA); $this->assertNotSoftDeleted($userB); From fc405d9d730c949ea368fc7c192cb992b17fbebc Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Mon, 5 Aug 2024 16:14:01 -0700 Subject: [PATCH 14/31] Assert redirected to correct place --- tests/Feature/Users/Ui/BulkDeleteUsersTest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/Feature/Users/Ui/BulkDeleteUsersTest.php b/tests/Feature/Users/Ui/BulkDeleteUsersTest.php index f321e011f..8d6ca61d5 100644 --- a/tests/Feature/Users/Ui/BulkDeleteUsersTest.php +++ b/tests/Feature/Users/Ui/BulkDeleteUsersTest.php @@ -69,7 +69,7 @@ class BulkDeleteUsersTest extends TestCase ], 'status_id' => Statuslabel::factory()->create()->id, ]) - ->assertRedirect(); + ->assertRedirect(route('users.index')); $this->assertTrue($userA->fresh()->accessories->isEmpty()); $this->assertTrue($userB->fresh()->accessories->isNotEmpty()); @@ -103,7 +103,7 @@ class BulkDeleteUsersTest extends TestCase ], 'status_id' => Statuslabel::factory()->create()->id, ]) - ->assertRedirect(); + ->assertRedirect(route('users.index')); $this->assertTrue($userA->fresh()->consumables->isEmpty()); $this->assertTrue($userB->fresh()->consumables->isNotEmpty()); @@ -128,7 +128,7 @@ class BulkDeleteUsersTest extends TestCase ], 'delete_user' => '1', ]) - ->assertRedirect() + ->assertRedirect(route('users.index')) ->assertSessionHas('success', trans('general.bulk_checkin_delete_success')); $this->assertSoftDeleted($userA); From 5cd9dd4a679ec0f809aa0cadcbe4b5b809af60fe Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Mon, 5 Aug 2024 16:17:24 -0700 Subject: [PATCH 15/31] Add assertion to ensure user cannot perform bulk actions on self --- tests/Feature/Users/Ui/BulkDeleteUsersTest.php | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/Feature/Users/Ui/BulkDeleteUsersTest.php b/tests/Feature/Users/Ui/BulkDeleteUsersTest.php index 8d6ca61d5..fad4dee83 100644 --- a/tests/Feature/Users/Ui/BulkDeleteUsersTest.php +++ b/tests/Feature/Users/Ui/BulkDeleteUsersTest.php @@ -48,6 +48,23 @@ class BulkDeleteUsersTest extends TestCase ])->assertSessionHas('error')->assertRedirect(); } + public function testCannotPerformBulkActionsOnSelf() + { + $actor = User::factory()->editUsers()->create(); + + $this->actingAs($actor) + ->post(route('users/bulksave'), [ + 'ids' => [ + $actor->id, + ], + 'delete_user' => '1', + ]) + ->assertRedirect(route('users.index')) + ->assertSessionHas('success', trans('general.bulk_checkin_delete_success')); + + $this->assertNotSoftDeleted($actor); + } + public function testAccessoriesCanBeBulkCheckedIn() { [$accessoryA, $accessoryB] = Accessory::factory()->count(2)->create(); From d1bb3ef6bf96babf845c44c6bb33284eacd70305 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Mon, 5 Aug 2024 16:19:09 -0700 Subject: [PATCH 16/31] Scaffold two tests --- tests/Feature/Users/Ui/BulkDeleteUsersTest.php | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/Feature/Users/Ui/BulkDeleteUsersTest.php b/tests/Feature/Users/Ui/BulkDeleteUsersTest.php index fad4dee83..4048f50b4 100644 --- a/tests/Feature/Users/Ui/BulkDeleteUsersTest.php +++ b/tests/Feature/Users/Ui/BulkDeleteUsersTest.php @@ -99,6 +99,14 @@ class BulkDeleteUsersTest extends TestCase $this->assertActionLogCheckInEntryFor($userC, $accessoryA); } + public function testAssetsCanBeBulkCheckedIn() + { + $this->markTestIncomplete(); + + // @todo: + // $this->assertActionLogCheckInEntryFor(); ... + } + public function testConsumablesCanBeBulkCheckedIn() { [$consumableA, $consumableB] = Consumable::factory()->count(2)->create(); @@ -133,6 +141,14 @@ class BulkDeleteUsersTest extends TestCase $this->assertActionLogCheckInEntryFor($userC, $consumableA); } + public function testLicenseSeatsCanBeBulkCheckedIn() + { + $this->markTestIncomplete(); + + // @todo: + // $this->assertActionLogCheckInEntryFor(); ... + } + public function testUsersCanBeDeletedInBulk() { [$userA, $userB, $userC] = User::factory()->count(3)->create(); From 16aa47509b3f655f6a9e2497ed38278c4fd7a702 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Mon, 5 Aug 2024 16:24:52 -0700 Subject: [PATCH 17/31] Implement test for assets --- .../Feature/Users/Ui/BulkDeleteUsersTest.php | 35 +++++++++++++++++-- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/tests/Feature/Users/Ui/BulkDeleteUsersTest.php b/tests/Feature/Users/Ui/BulkDeleteUsersTest.php index 4048f50b4..8787e849e 100644 --- a/tests/Feature/Users/Ui/BulkDeleteUsersTest.php +++ b/tests/Feature/Users/Ui/BulkDeleteUsersTest.php @@ -101,10 +101,34 @@ class BulkDeleteUsersTest extends TestCase public function testAssetsCanBeBulkCheckedIn() { - $this->markTestIncomplete(); + // $this->markTestIncomplete(); - // @todo: - // $this->assertActionLogCheckInEntryFor(); ... + [$userA, $userB, $userC] = User::factory()->count(3)->create(); + + $assetA = $this->assignAssetToUser($userA); + $assetB = $this->assignAssetToUser($userB); + $assetC = $this->assignAssetToUser($userC); + + $this->assertTrue($userA->assets->isNotEmpty()); + $this->assertTrue($userB->assets->isNotEmpty()); + $this->assertTrue($userC->assets->isNotEmpty()); + + $this->actingAs(User::factory()->editUsers()->create()) + ->post(route('users/bulksave'), [ + 'ids' => [ + $userA->id, + $userC->id, + ], + 'status_id' => Statuslabel::factory()->create()->id, + ]) + ->assertRedirect(route('users.index')); + + $this->assertTrue($userA->fresh()->assets->isEmpty()); + $this->assertTrue($userB->fresh()->assets->isNotEmpty()); + $this->assertTrue($userC->fresh()->assets->isEmpty()); + + $this->assertActionLogCheckInEntryFor($userA, $assetA); + $this->assertActionLogCheckInEntryFor($userC, $assetC); } public function testConsumablesCanBeBulkCheckedIn() @@ -200,4 +224,9 @@ class BulkDeleteUsersTest extends TestCase 'item_id' => $model->id, ]); } + + private function assignAssetToUser(User $user): Asset + { + return Asset::factory()->assignedToUser($user)->create(); + } } From 59b7d5db4b47a9f5c74312c9a79cf3317cbcebd6 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Mon, 5 Aug 2024 16:25:14 -0700 Subject: [PATCH 18/31] Remove comment --- tests/Feature/Users/Ui/BulkDeleteUsersTest.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/Feature/Users/Ui/BulkDeleteUsersTest.php b/tests/Feature/Users/Ui/BulkDeleteUsersTest.php index 8787e849e..e17ee770b 100644 --- a/tests/Feature/Users/Ui/BulkDeleteUsersTest.php +++ b/tests/Feature/Users/Ui/BulkDeleteUsersTest.php @@ -101,8 +101,6 @@ class BulkDeleteUsersTest extends TestCase public function testAssetsCanBeBulkCheckedIn() { - // $this->markTestIncomplete(); - [$userA, $userB, $userC] = User::factory()->count(3)->create(); $assetA = $this->assignAssetToUser($userA); From 1acd24fdbe81dbabe547ef41899a109fd3879155 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Mon, 5 Aug 2024 16:26:25 -0700 Subject: [PATCH 19/31] Re-order helpers --- tests/Feature/Users/Ui/BulkDeleteUsersTest.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/Feature/Users/Ui/BulkDeleteUsersTest.php b/tests/Feature/Users/Ui/BulkDeleteUsersTest.php index e17ee770b..4fab86d46 100644 --- a/tests/Feature/Users/Ui/BulkDeleteUsersTest.php +++ b/tests/Feature/Users/Ui/BulkDeleteUsersTest.php @@ -191,6 +191,11 @@ class BulkDeleteUsersTest extends TestCase $this->assertSoftDeleted($userC); } + private function assignAssetToUser(User $user): Asset + { + return Asset::factory()->assignedToUser($user)->create(); + } + private function attachAccessoryToUsers(Accessory $accessory, array $users): void { foreach ($users as $user) { @@ -222,9 +227,4 @@ class BulkDeleteUsersTest extends TestCase 'item_id' => $model->id, ]); } - - private function assignAssetToUser(User $user): Asset - { - return Asset::factory()->assignedToUser($user)->create(); - } } From a55693211f8f5ea1e3dd9046bd0f0be184a2afad Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Mon, 5 Aug 2024 16:27:04 -0700 Subject: [PATCH 20/31] Move test class --- .../Feature/Users/Ui/{ => BulkActions}/BulkDeleteUsersTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename tests/Feature/Users/Ui/{ => BulkActions}/BulkDeleteUsersTest.php (99%) diff --git a/tests/Feature/Users/Ui/BulkDeleteUsersTest.php b/tests/Feature/Users/Ui/BulkActions/BulkDeleteUsersTest.php similarity index 99% rename from tests/Feature/Users/Ui/BulkDeleteUsersTest.php rename to tests/Feature/Users/Ui/BulkActions/BulkDeleteUsersTest.php index 4fab86d46..bc5e50b5f 100644 --- a/tests/Feature/Users/Ui/BulkDeleteUsersTest.php +++ b/tests/Feature/Users/Ui/BulkActions/BulkDeleteUsersTest.php @@ -1,6 +1,6 @@ Date: Mon, 5 Aug 2024 16:36:29 -0700 Subject: [PATCH 21/31] Improve readability --- .../Ui/BulkActions/BulkDeleteUsersTest.php | 22 +++++-------------- 1 file changed, 5 insertions(+), 17 deletions(-) diff --git a/tests/Feature/Users/Ui/BulkActions/BulkDeleteUsersTest.php b/tests/Feature/Users/Ui/BulkActions/BulkDeleteUsersTest.php index bc5e50b5f..f9f1c27de 100644 --- a/tests/Feature/Users/Ui/BulkActions/BulkDeleteUsersTest.php +++ b/tests/Feature/Users/Ui/BulkActions/BulkDeleteUsersTest.php @@ -74,10 +74,6 @@ class BulkDeleteUsersTest extends TestCase $this->attachAccessoryToUsers($accessoryA, [$userA, $userB, $userC]); $this->attachAccessoryToUsers($accessoryB, [$userA, $userB]); - $this->assertTrue($userA->accessories->isNotEmpty()); - $this->assertTrue($userB->accessories->isNotEmpty()); - $this->assertTrue($userC->accessories->isNotEmpty()); - $this->actingAs(User::factory()->editUsers()->create()) ->post(route('users/bulksave'), [ 'ids' => [ @@ -103,13 +99,9 @@ class BulkDeleteUsersTest extends TestCase { [$userA, $userB, $userC] = User::factory()->count(3)->create(); - $assetA = $this->assignAssetToUser($userA); - $assetB = $this->assignAssetToUser($userB); - $assetC = $this->assignAssetToUser($userC); - - $this->assertTrue($userA->assets->isNotEmpty()); - $this->assertTrue($userB->assets->isNotEmpty()); - $this->assertTrue($userC->assets->isNotEmpty()); + $assetForUserA = $this->assignAssetToUser($userA); + $lonelyAsset = $this->assignAssetToUser($userB); + $assetForUserC = $this->assignAssetToUser($userC); $this->actingAs(User::factory()->editUsers()->create()) ->post(route('users/bulksave'), [ @@ -125,8 +117,8 @@ class BulkDeleteUsersTest extends TestCase $this->assertTrue($userB->fresh()->assets->isNotEmpty()); $this->assertTrue($userC->fresh()->assets->isEmpty()); - $this->assertActionLogCheckInEntryFor($userA, $assetA); - $this->assertActionLogCheckInEntryFor($userC, $assetC); + $this->assertActionLogCheckInEntryFor($userA, $assetForUserA); + $this->assertActionLogCheckInEntryFor($userC, $assetForUserC); } public function testConsumablesCanBeBulkCheckedIn() @@ -138,10 +130,6 @@ class BulkDeleteUsersTest extends TestCase $this->attachConsumableToUsers($consumableA, [$userA, $userB, $userC]); $this->attachConsumableToUsers($consumableB, [$userA, $userB]); - $this->assertTrue($userA->consumables->isNotEmpty()); - $this->assertTrue($userB->consumables->isNotEmpty()); - $this->assertTrue($userC->consumables->isNotEmpty()); - $this->actingAs(User::factory()->editUsers()->create()) ->post(route('users/bulksave'), [ 'ids' => [ From 35e7a3163cb5e36153dd4a02b43f398e8361f4dd Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Mon, 5 Aug 2024 16:54:31 -0700 Subject: [PATCH 22/31] Implement test case --- .../Ui/BulkActions/BulkDeleteUsersTest.php | 49 +++++++++++++++++-- 1 file changed, 46 insertions(+), 3 deletions(-) diff --git a/tests/Feature/Users/Ui/BulkActions/BulkDeleteUsersTest.php b/tests/Feature/Users/Ui/BulkActions/BulkDeleteUsersTest.php index f9f1c27de..e6a83e936 100644 --- a/tests/Feature/Users/Ui/BulkActions/BulkDeleteUsersTest.php +++ b/tests/Feature/Users/Ui/BulkActions/BulkDeleteUsersTest.php @@ -5,6 +5,8 @@ namespace Feature\Users\Ui\BulkActions; use App\Models\Accessory; use App\Models\Asset; use App\Models\Consumable; +use App\Models\License; +use App\Models\LicenseSeat; use App\Models\Statuslabel; use App\Models\User; use Illuminate\Database\Eloquent\Model; @@ -153,10 +155,51 @@ class BulkDeleteUsersTest extends TestCase public function testLicenseSeatsCanBeBulkCheckedIn() { - $this->markTestIncomplete(); + [$userA, $userB, $userC] = User::factory()->count(3)->create(); - // @todo: - // $this->assertActionLogCheckInEntryFor(); ... + $licenseSeatForUserA = LicenseSeat::factory()->assignedToUser($userA)->create(); + $lonelyLicenseSeat = LicenseSeat::factory()->assignedToUser($userB)->create(); + $licenseSeatForUserC = LicenseSeat::factory()->assignedToUser($userC)->create(); + + $this->actingAs(User::factory()->editUsers()->create()) + ->post(route('users/bulksave'), [ + 'ids' => [ + $userA->id, + $userC->id, + ], + ]) + ->assertRedirect(route('users.index')) + ->assertSessionHas('success', trans('general.bulk_checkin_success')); + + $this->assertDatabaseMissing('license_seats', [ + 'license_id' => $licenseSeatForUserA->license->id, + 'assigned_to' => $userA->id, + ]); + + $this->assertDatabaseMissing('license_seats', [ + 'license_id' => $licenseSeatForUserC->license->id, + 'assigned_to' => $userC->id, + ]); + + // Slightly different from the other assertions since we use + // the license and not the license seat in this case. + $this->assertDatabaseHas('action_logs', [ + 'action_type' => 'checkin from', + 'target_id' => $userA->id, + 'target_type' => User::class, + 'note' => 'Bulk checkin items', + 'item_type' => License::class, + 'item_id' => $licenseSeatForUserA->license->id, + ]); + + $this->assertDatabaseHas('action_logs', [ + 'action_type' => 'checkin from', + 'target_id' => $userC->id, + 'target_type' => User::class, + 'note' => 'Bulk checkin items', + 'item_type' => License::class, + 'item_id' => $licenseSeatForUserC->license->id, + ]); } public function testUsersCanBeDeletedInBulk() From 392d34422ace4f4401d997aea5326db627bb6b1c Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Mon, 5 Aug 2024 16:57:40 -0700 Subject: [PATCH 23/31] Remove code handled by ConsumableAssignment:: call above --- app/Http/Controllers/Users/BulkUsersController.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/app/Http/Controllers/Users/BulkUsersController.php b/app/Http/Controllers/Users/BulkUsersController.php index c2cc8607f..388760742 100644 --- a/app/Http/Controllers/Users/BulkUsersController.php +++ b/app/Http/Controllers/Users/BulkUsersController.php @@ -246,9 +246,6 @@ class BulkUsersController extends Controller foreach ($users as $user) { - - // @todo: This can be deleted since we have ConsumableAssignment above right? - $user->consumables()->sync([]); // @todo: Can this be removed if we introduce AccessoryAssignment? $user->accessories()->sync([]); if ($request->input('delete_user')=='1') { From 1c664af3260f269810d9735bb9060dc3f242d963 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Mon, 5 Aug 2024 16:58:08 -0700 Subject: [PATCH 24/31] Remove todo outside of scope --- app/Http/Controllers/Users/BulkUsersController.php | 1 - 1 file changed, 1 deletion(-) diff --git a/app/Http/Controllers/Users/BulkUsersController.php b/app/Http/Controllers/Users/BulkUsersController.php index 388760742..899728139 100644 --- a/app/Http/Controllers/Users/BulkUsersController.php +++ b/app/Http/Controllers/Users/BulkUsersController.php @@ -246,7 +246,6 @@ class BulkUsersController extends Controller foreach ($users as $user) { - // @todo: Can this be removed if we introduce AccessoryAssignment? $user->accessories()->sync([]); if ($request->input('delete_user')=='1') { $user->delete(); From 01e4382d20ba36cb0811300f4b5c551c978249c3 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Mon, 5 Aug 2024 16:58:17 -0700 Subject: [PATCH 25/31] Formatting --- app/Http/Controllers/Users/BulkUsersController.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/Http/Controllers/Users/BulkUsersController.php b/app/Http/Controllers/Users/BulkUsersController.php index 899728139..c1a0dfab6 100644 --- a/app/Http/Controllers/Users/BulkUsersController.php +++ b/app/Http/Controllers/Users/BulkUsersController.php @@ -244,13 +244,11 @@ class BulkUsersController extends Controller LicenseSeat::whereIn('id', $licenses->pluck('id'))->update(['assigned_to' => null]); ConsumableAssignment::whereIn('id', $consumableUserRows->pluck('id'))->delete(); - foreach ($users as $user) { $user->accessories()->sync([]); if ($request->input('delete_user')=='1') { $user->delete(); } - } $msg = trans('general.bulk_checkin_success'); From 17eccfcd8b840e2394ac19abe4acac9c7242f4d2 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Mon, 5 Aug 2024 16:58:27 -0700 Subject: [PATCH 26/31] Formatting --- app/Http/Controllers/Users/BulkUsersController.php | 1 - 1 file changed, 1 deletion(-) diff --git a/app/Http/Controllers/Users/BulkUsersController.php b/app/Http/Controllers/Users/BulkUsersController.php index c1a0dfab6..a3bf2ad0c 100644 --- a/app/Http/Controllers/Users/BulkUsersController.php +++ b/app/Http/Controllers/Users/BulkUsersController.php @@ -240,7 +240,6 @@ class BulkUsersController extends Controller 'expected_checkin' => null, ]); - LicenseSeat::whereIn('id', $licenses->pluck('id'))->update(['assigned_to' => null]); ConsumableAssignment::whereIn('id', $consumableUserRows->pluck('id'))->delete(); From 94e00b8a3e663d970d4035542cb7156700a95a67 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Mon, 5 Aug 2024 17:26:11 -0700 Subject: [PATCH 27/31] Use new accessory checkout relationship --- .../Feature/Users/Ui/BulkActions/BulkDeleteUsersTest.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/Feature/Users/Ui/BulkActions/BulkDeleteUsersTest.php b/tests/Feature/Users/Ui/BulkActions/BulkDeleteUsersTest.php index e6a83e936..d84135cce 100644 --- a/tests/Feature/Users/Ui/BulkActions/BulkDeleteUsersTest.php +++ b/tests/Feature/Users/Ui/BulkActions/BulkDeleteUsersTest.php @@ -3,6 +3,7 @@ namespace Feature\Users\Ui\BulkActions; use App\Models\Accessory; +use App\Models\AccessoryCheckout; use App\Models\Asset; use App\Models\Consumable; use App\Models\License; @@ -230,10 +231,9 @@ class BulkDeleteUsersTest extends TestCase private function attachAccessoryToUsers(Accessory $accessory, array $users): void { foreach ($users as $user) { - $accessory->users()->attach($accessory->id, [ - 'accessory_id' => $accessory->id, - 'assigned_to' => $user->id, - ]); + $a = $accessory->checkouts()->make(); + $a->assignedTo()->associate($user); + $a->save(); } } From 2a8ac81eeddeae13f12963973bf63f81bc30b37b Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Tue, 6 Aug 2024 15:22:13 -0700 Subject: [PATCH 28/31] Fix namespace --- tests/Feature/Users/Ui/BulkActions/BulkDeleteUsersTest.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/Feature/Users/Ui/BulkActions/BulkDeleteUsersTest.php b/tests/Feature/Users/Ui/BulkActions/BulkDeleteUsersTest.php index d84135cce..4a97fe30d 100644 --- a/tests/Feature/Users/Ui/BulkActions/BulkDeleteUsersTest.php +++ b/tests/Feature/Users/Ui/BulkActions/BulkDeleteUsersTest.php @@ -1,9 +1,8 @@ Date: Tue, 6 Aug 2024 16:13:51 -0700 Subject: [PATCH 29/31] Switch to snake case --- .../Users/Ui/BulkActions/BulkDeleteUsersTest.php | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/Feature/Users/Ui/BulkActions/BulkDeleteUsersTest.php b/tests/Feature/Users/Ui/BulkActions/BulkDeleteUsersTest.php index 4a97fe30d..016d28bcd 100644 --- a/tests/Feature/Users/Ui/BulkActions/BulkDeleteUsersTest.php +++ b/tests/Feature/Users/Ui/BulkActions/BulkDeleteUsersTest.php @@ -14,7 +14,7 @@ use Tests\TestCase; class BulkDeleteUsersTest extends TestCase { - public function testRequiresCorrectPermission() + public function test_requires_correct_permission() { $this->actingAs(User::factory()->create()) ->post(route('users/bulksave'), [ @@ -26,7 +26,7 @@ class BulkDeleteUsersTest extends TestCase ->assertForbidden(); } - public function testValidation() + public function test_validation() { $user = User::factory()->create(); Asset::factory()->assignedToUser($user)->create(); @@ -50,7 +50,7 @@ class BulkDeleteUsersTest extends TestCase ])->assertSessionHas('error')->assertRedirect(); } - public function testCannotPerformBulkActionsOnSelf() + public function test_cannot_perform_bulk_actions_on_self() { $actor = User::factory()->editUsers()->create(); @@ -67,7 +67,7 @@ class BulkDeleteUsersTest extends TestCase $this->assertNotSoftDeleted($actor); } - public function testAccessoriesCanBeBulkCheckedIn() + public function test_accessories_can_be_bulk_checked_in() { [$accessoryA, $accessoryB] = Accessory::factory()->count(2)->create(); [$userA, $userB, $userC] = User::factory()->count(3)->create(); @@ -97,7 +97,7 @@ class BulkDeleteUsersTest extends TestCase $this->assertActionLogCheckInEntryFor($userC, $accessoryA); } - public function testAssetsCanBeBulkCheckedIn() + public function test_assets_can_be_bulk_checked_in() { [$userA, $userB, $userC] = User::factory()->count(3)->create(); @@ -123,7 +123,7 @@ class BulkDeleteUsersTest extends TestCase $this->assertActionLogCheckInEntryFor($userC, $assetForUserC); } - public function testConsumablesCanBeBulkCheckedIn() + public function test_consumables_can_be_bulk_checked_in() { [$consumableA, $consumableB] = Consumable::factory()->count(2)->create(); [$userA, $userB, $userC] = User::factory()->count(3)->create(); @@ -153,7 +153,7 @@ class BulkDeleteUsersTest extends TestCase $this->assertActionLogCheckInEntryFor($userC, $consumableA); } - public function testLicenseSeatsCanBeBulkCheckedIn() + public function test_license_seats_can_be_bulk_checked_in() { [$userA, $userB, $userC] = User::factory()->count(3)->create(); @@ -202,7 +202,7 @@ class BulkDeleteUsersTest extends TestCase ]); } - public function testUsersCanBeDeletedInBulk() + public function test_users_can_be_deleted_in_bulk() { [$userA, $userB, $userC] = User::factory()->count(3)->create(); From d03baa4613154f9e42487c265b8f01feb455ac82 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 7 Aug 2024 13:29:45 -0700 Subject: [PATCH 30/31] Make diff cleaner --- app/Http/Controllers/Users/BulkUsersController.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/Http/Controllers/Users/BulkUsersController.php b/app/Http/Controllers/Users/BulkUsersController.php index a48207030..1a8f84b7a 100644 --- a/app/Http/Controllers/Users/BulkUsersController.php +++ b/app/Http/Controllers/Users/BulkUsersController.php @@ -220,7 +220,7 @@ class BulkUsersController extends Controller $users = User::whereIn('id', $user_raw_array)->get(); $assets = Asset::whereIn('assigned_to', $user_raw_array)->where('assigned_type', User::class)->get(); - $accessoryUserRows = DB::table('accessories_checkout')->whereIn('assigned_to', $user_raw_array)->where('assigned_type', User::class)->get(); + $accessoryUserRows = DB::table('accessories_checkout')->where('assigned_type', User::class)->whereIn('assigned_to', $user_raw_array)->get(); $licenses = DB::table('license_seats')->whereIn('assigned_to', $user_raw_array)->get(); $consumableUserRows = DB::table('consumables_users')->whereIn('assigned_to', $user_raw_array)->get(); From 4a2d2f733691819b400d929f953b40ac724e8b7e Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 7 Aug 2024 13:32:30 -0700 Subject: [PATCH 31/31] Improve variable name --- tests/Feature/Users/Ui/BulkActions/BulkDeleteUsersTest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/Feature/Users/Ui/BulkActions/BulkDeleteUsersTest.php b/tests/Feature/Users/Ui/BulkActions/BulkDeleteUsersTest.php index 016d28bcd..46ba023c1 100644 --- a/tests/Feature/Users/Ui/BulkActions/BulkDeleteUsersTest.php +++ b/tests/Feature/Users/Ui/BulkActions/BulkDeleteUsersTest.php @@ -230,9 +230,9 @@ class BulkDeleteUsersTest extends TestCase private function attachAccessoryToUsers(Accessory $accessory, array $users): void { foreach ($users as $user) { - $a = $accessory->checkouts()->make(); - $a->assignedTo()->associate($user); - $a->save(); + $accessoryCheckout = $accessory->checkouts()->make(); + $accessoryCheckout->assignedTo()->associate($user); + $accessoryCheckout->save(); } }