From 984cc7a4f2d80faded881447bf429120b4d6b63a Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 20 Mar 2024 13:29:25 -0700 Subject: [PATCH 01/33] Scaffold tests around asset checkout via web --- database/factories/StatuslabelFactory.php | 5 ++ tests/Feature/Checkouts/AssetCheckoutTest.php | 73 +++++++++++++++++++ 2 files changed, 78 insertions(+) create mode 100644 tests/Feature/Checkouts/AssetCheckoutTest.php diff --git a/database/factories/StatuslabelFactory.php b/database/factories/StatuslabelFactory.php index 0b8359dd5..fa2e5d5e1 100644 --- a/database/factories/StatuslabelFactory.php +++ b/database/factories/StatuslabelFactory.php @@ -46,6 +46,11 @@ class StatuslabelFactory extends Factory }); } + public function readyToDeploy() + { + return $this->rtd(); + } + public function pending() { return $this->state(function () { diff --git a/tests/Feature/Checkouts/AssetCheckoutTest.php b/tests/Feature/Checkouts/AssetCheckoutTest.php new file mode 100644 index 000000000..92291cea1 --- /dev/null +++ b/tests/Feature/Checkouts/AssetCheckoutTest.php @@ -0,0 +1,73 @@ +actingAs(User::factory()->create()) + ->post(route('hardware.checkout.store', Asset::factory()->create()), [ + 'checkout_to_type' => 'user', + 'assigned_user' => User::factory()->create()->id, + ]) + ->assertForbidden(); + } + + public function testValidationWhenCheckingOutAsset() + { + $this->actingAs(User::factory()->create()) + ->post(route('hardware.checkout.store', Asset::factory()->create()), [ + // + ]) + ->assertSessionHasErrors(); + } + + public function testAnAssetCanBeCheckedOutToAUser() + { + $this->markTestIncomplete(); + + Event::fake([CheckoutableCheckedOut::class]); + + $status = Statuslabel::factory()->readyToDeploy()->create(); + $asset = Asset::factory()->create(); + $admin = User::factory()->create(); + $user = User::factory()->create(); + + $this->actingAs($admin) + ->post(route('hardware.checkout.store', $asset), [ + 'checkout_to_type' => 'user', + 'assigned_user' => $user->id, + 'name' => 'Changed Name', + 'status_id' => $status->id, + 'checkout_at' => '2024-03-18', + 'expected_checkin' => '2024-03-28', + 'note' => 'An awesome note', + ]); + + // @todo: assert CheckoutableCheckedOut dispatched with correct data + Event::assertDispatched(CheckoutableCheckedOut::class); + + // @todo: assert action log entry created + } + + public function testAnAssetCanBeCheckedOutToAnAsset() + { + $this->markTestIncomplete(); + } + + public function testAnAssetCanBeCheckedOutToALocation() + { + $this->markTestIncomplete(); + } +} From 393dc51167cee3d4c3037f362080667a1573f550 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 20 Mar 2024 13:44:26 -0700 Subject: [PATCH 02/33] Add assertions around the event dispatch --- tests/Feature/Checkouts/AssetCheckoutTest.php | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/tests/Feature/Checkouts/AssetCheckoutTest.php b/tests/Feature/Checkouts/AssetCheckoutTest.php index 92291cea1..1f67422c6 100644 --- a/tests/Feature/Checkouts/AssetCheckoutTest.php +++ b/tests/Feature/Checkouts/AssetCheckoutTest.php @@ -35,13 +35,13 @@ class AssetCheckoutTest extends TestCase public function testAnAssetCanBeCheckedOutToAUser() { - $this->markTestIncomplete(); + // $this->markTestIncomplete(); Event::fake([CheckoutableCheckedOut::class]); $status = Statuslabel::factory()->readyToDeploy()->create(); $asset = Asset::factory()->create(); - $admin = User::factory()->create(); + $admin = User::factory()->checkoutAssets()->create(); $user = User::factory()->create(); $this->actingAs($admin) @@ -55,8 +55,14 @@ class AssetCheckoutTest extends TestCase 'note' => 'An awesome note', ]); - // @todo: assert CheckoutableCheckedOut dispatched with correct data - Event::assertDispatched(CheckoutableCheckedOut::class); + // @todo: ensure asset updated + + Event::assertDispatched(function (CheckoutableCheckedOut $event) use ($admin, $asset, $user) { + return $event->checkoutable->is($asset) + && $event->checkedOutTo->is($user) + && $event->checkedOutBy->is($admin) + && $event->note === 'An awesome note'; + }); // @todo: assert action log entry created } From 530291f81c2b5418cdb5fc110e44150c86a4ff0a Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 20 Mar 2024 14:28:27 -0700 Subject: [PATCH 03/33] Implement some tests --- .../Assets/AssetCheckoutController.php | 2 + tests/Feature/Checkouts/AssetCheckoutTest.php | 68 +++++++++++++++++-- 2 files changed, 65 insertions(+), 5 deletions(-) diff --git a/app/Http/Controllers/Assets/AssetCheckoutController.php b/app/Http/Controllers/Assets/AssetCheckoutController.php index a096f1667..46f8c5b8b 100644 --- a/app/Http/Controllers/Assets/AssetCheckoutController.php +++ b/app/Http/Controllers/Assets/AssetCheckoutController.php @@ -64,6 +64,7 @@ class AssetCheckoutController extends Controller $target = $this->determineCheckoutTarget($asset); + // this seems to be handled by the checkOut method $asset = $this->updateAssetLocation($asset, $target); $checkout_at = date('Y-m-d H:i:s'); @@ -92,6 +93,7 @@ class AssetCheckoutController extends Controller $settings = \App\Models\Setting::getSettings(); // We have to check whether $target->company_id is null here since locations don't have a company yet + // @todo: test this if (($settings->full_multiple_companies_support) && ((!is_null($target->company_id)) && (!is_null($asset->company_id)))) { if ($target->company_id != $asset->company_id){ return redirect()->to("hardware/$assetId/checkout")->with('error', trans('general.error_user_company')); diff --git a/tests/Feature/Checkouts/AssetCheckoutTest.php b/tests/Feature/Checkouts/AssetCheckoutTest.php index 1f67422c6..b24d83dbf 100644 --- a/tests/Feature/Checkouts/AssetCheckoutTest.php +++ b/tests/Feature/Checkouts/AssetCheckoutTest.php @@ -4,6 +4,7 @@ namespace Tests\Feature\Checkouts; use App\Events\CheckoutableCheckedOut; use App\Models\Asset; +use App\Models\Location; use App\Models\Statuslabel; use App\Models\User; use Illuminate\Support\Facades\Event; @@ -24,6 +25,46 @@ class AssetCheckoutTest extends TestCase ->assertForbidden(); } + public function testNonExistentAssetCannotBeCheckedOut() + { + Event::fake([CheckoutableCheckedOut::class]); + + $this->actingAs(User::factory()->checkoutAssets()->create()) + ->post(route('hardware.checkout.store', 1000), [ + 'checkout_to_type' => 'user', + 'assigned_user' => User::factory()->create()->id, + 'name' => 'Changed Name', + 'status_id' => Statuslabel::factory()->readyToDeploy()->create()->id, + 'checkout_at' => '2024-03-18', + 'expected_checkin' => '2024-03-28', + 'note' => 'An awesome note', + ]) + ->assertSessionHas('error'); + + Event::assertNotDispatched(CheckoutableCheckedOut::class); + } + + public function testAssetNotAvailableForCheckoutCannotBeCheckedOut() + { + Event::fake([CheckoutableCheckedOut::class]); + + $asset = Asset::factory()->assignedToUser()->create(); + + $this->actingAs(User::factory()->checkoutAssets()->create()) + ->post(route('hardware.checkout.store', $asset), [ + 'checkout_to_type' => 'user', + 'assigned_user' => User::factory()->create()->id, + 'name' => 'Changed Name', + 'status_id' => Statuslabel::factory()->readyToDeploy()->create()->id, + 'checkout_at' => '2024-03-18', + 'expected_checkin' => '2024-03-28', + 'note' => 'An awesome note', + ]) + ->assertSessionHas('error'); + + Event::assertNotDispatched(CheckoutableCheckedOut::class); + } + public function testValidationWhenCheckingOutAsset() { $this->actingAs(User::factory()->create()) @@ -35,27 +76,34 @@ class AssetCheckoutTest extends TestCase public function testAnAssetCanBeCheckedOutToAUser() { - // $this->markTestIncomplete(); + $this->markTestIncomplete(); Event::fake([CheckoutableCheckedOut::class]); - $status = Statuslabel::factory()->readyToDeploy()->create(); - $asset = Asset::factory()->create(); + $originalStatus = Statuslabel::factory()->readyToDeploy()->create(); + $updatedStatus = Statuslabel::factory()->readyToDeploy()->create(); + $asset = Asset::factory()->create(['status_id' => $originalStatus->id]); $admin = User::factory()->checkoutAssets()->create(); - $user = User::factory()->create(); + $userLocation = Location::factory()->create(); + + $user = User::factory()->for($userLocation)->create(); $this->actingAs($admin) ->post(route('hardware.checkout.store', $asset), [ 'checkout_to_type' => 'user', 'assigned_user' => $user->id, 'name' => 'Changed Name', - 'status_id' => $status->id, + 'status_id' => $updatedStatus->id, 'checkout_at' => '2024-03-18', 'expected_checkin' => '2024-03-28', 'note' => 'An awesome note', ]); // @todo: ensure asset updated + $asset->refresh(); + $this->assertTrue($asset->location->is($userLocation)); + $this->assertEquals('2024-03-28 00:00:00', (string) $asset->expected_checkin); + $this->assertTrue($asset->assetstatus->is($updatedStatus)); Event::assertDispatched(function (CheckoutableCheckedOut $event) use ($admin, $asset, $user) { return $event->checkoutable->is($asset) @@ -67,6 +115,11 @@ class AssetCheckoutTest extends TestCase // @todo: assert action log entry created } + public function testLicenseSeatsAreAssignedToUserUponCheckout() + { + $this->markTestIncomplete(); + } + public function testAnAssetCanBeCheckedOutToAnAsset() { $this->markTestIncomplete(); @@ -76,4 +129,9 @@ class AssetCheckoutTest extends TestCase { $this->markTestIncomplete(); } + + public function testLastCheckoutUsesCurrentDateIfNotProvided() + { + $this->markTestIncomplete(); + } } From b368acf94194ee7f56e12d8aac6e8c4c25f4ccf0 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 20 Mar 2024 17:46:09 -0700 Subject: [PATCH 04/33] Implement test case --- database/factories/LicenseSeatFactory.php | 10 ++++++++++ tests/Feature/Checkouts/AssetCheckoutTest.php | 19 ++++++++++++++++--- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/database/factories/LicenseSeatFactory.php b/database/factories/LicenseSeatFactory.php index cd9acfee1..d3283d839 100644 --- a/database/factories/LicenseSeatFactory.php +++ b/database/factories/LicenseSeatFactory.php @@ -2,6 +2,7 @@ namespace Database\Factories; +use App\Models\Asset; use App\Models\License; use App\Models\User; use Illuminate\Database\Eloquent\Factories\Factory; @@ -15,6 +16,15 @@ class LicenseSeatFactory extends Factory ]; } + public function assignedToAsset(Asset $asset = null) + { + return $this->state(function () use ($asset) { + return [ + 'asset_id' => $asset->id, + ]; + }); + } + public function assignedToUser(User $user = null) { return $this->state(function () use ($user) { diff --git a/tests/Feature/Checkouts/AssetCheckoutTest.php b/tests/Feature/Checkouts/AssetCheckoutTest.php index b24d83dbf..974473a08 100644 --- a/tests/Feature/Checkouts/AssetCheckoutTest.php +++ b/tests/Feature/Checkouts/AssetCheckoutTest.php @@ -4,6 +4,7 @@ namespace Tests\Feature\Checkouts; use App\Events\CheckoutableCheckedOut; use App\Models\Asset; +use App\Models\LicenseSeat; use App\Models\Location; use App\Models\Statuslabel; use App\Models\User; @@ -102,9 +103,9 @@ class AssetCheckoutTest extends TestCase // @todo: ensure asset updated $asset->refresh(); $this->assertTrue($asset->location->is($userLocation)); - $this->assertEquals('2024-03-28 00:00:00', (string) $asset->expected_checkin); + $this->assertEquals('2024-03-28 00:00:00', (string)$asset->expected_checkin); $this->assertTrue($asset->assetstatus->is($updatedStatus)); - + Event::assertDispatched(function (CheckoutableCheckedOut $event) use ($admin, $asset, $user) { return $event->checkoutable->is($asset) && $event->checkedOutTo->is($user) @@ -117,7 +118,19 @@ class AssetCheckoutTest extends TestCase public function testLicenseSeatsAreAssignedToUserUponCheckout() { - $this->markTestIncomplete(); + $asset = Asset::factory()->create(); + $seat = LicenseSeat::factory()->assignedToAsset($asset)->create(); + $user = User::factory()->create(); + + $this->assertFalse($user->licenses->contains($seat->license)); + + $this->actingAs(User::factory()->checkoutAssets()->create()) + ->post(route('hardware.checkout.store', $asset), [ + 'checkout_to_type' => 'user', + 'assigned_user' => $user->id, + ]); + + $this->assertTrue($user->fresh()->licenses->contains($seat->license)); } public function testAnAssetCanBeCheckedOutToAnAsset() From 6f53f2ac64d83ace72fcd0422ef881b4fc268c9b Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 10 Apr 2024 11:43:50 -0700 Subject: [PATCH 05/33] Finish implementing test case --- tests/Feature/Checkouts/AssetCheckoutTest.php | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/tests/Feature/Checkouts/AssetCheckoutTest.php b/tests/Feature/Checkouts/AssetCheckoutTest.php index 974473a08..b04a44cdb 100644 --- a/tests/Feature/Checkouts/AssetCheckoutTest.php +++ b/tests/Feature/Checkouts/AssetCheckoutTest.php @@ -9,13 +9,10 @@ use App\Models\Location; use App\Models\Statuslabel; use App\Models\User; use Illuminate\Support\Facades\Event; -use Tests\Support\InteractsWithSettings; use Tests\TestCase; class AssetCheckoutTest extends TestCase { - use InteractsWithSettings; - public function testCheckingOutAssetRequiresCorrectPermission() { $this->actingAs(User::factory()->create()) @@ -77,8 +74,6 @@ class AssetCheckoutTest extends TestCase public function testAnAssetCanBeCheckedOutToAUser() { - $this->markTestIncomplete(); - Event::fake([CheckoutableCheckedOut::class]); $originalStatus = Statuslabel::factory()->readyToDeploy()->create(); @@ -100,11 +95,13 @@ class AssetCheckoutTest extends TestCase 'note' => 'An awesome note', ]); - // @todo: ensure asset updated $asset->refresh(); + $this->assertTrue($asset->assignedTo()->is($user)); $this->assertTrue($asset->location->is($userLocation)); - $this->assertEquals('2024-03-28 00:00:00', (string)$asset->expected_checkin); + $this->assertEquals('Changed Name', $asset->name); $this->assertTrue($asset->assetstatus->is($updatedStatus)); + $this->assertEquals('2024-03-18 00:00:00', $asset->last_checkout); + $this->assertEquals('2024-03-28 00:00:00', (string)$asset->expected_checkin); Event::assertDispatched(function (CheckoutableCheckedOut $event) use ($admin, $asset, $user) { return $event->checkoutable->is($asset) @@ -112,8 +109,6 @@ class AssetCheckoutTest extends TestCase && $event->checkedOutBy->is($admin) && $event->note === 'An awesome note'; }); - - // @todo: assert action log entry created } public function testLicenseSeatsAreAssignedToUserUponCheckout() From e65252725a993a86404c67c9d1152451b889dbbb Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 10 Apr 2024 11:52:59 -0700 Subject: [PATCH 06/33] Add simple test for LogListener --- tests/Feature/Checkouts/AssetCheckoutTest.php | 1 + tests/Unit/Listeners/LogListenerTest.php | 37 +++++++++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 tests/Unit/Listeners/LogListenerTest.php diff --git a/tests/Feature/Checkouts/AssetCheckoutTest.php b/tests/Feature/Checkouts/AssetCheckoutTest.php index b04a44cdb..fb72b741b 100644 --- a/tests/Feature/Checkouts/AssetCheckoutTest.php +++ b/tests/Feature/Checkouts/AssetCheckoutTest.php @@ -103,6 +103,7 @@ class AssetCheckoutTest extends TestCase $this->assertEquals('2024-03-18 00:00:00', $asset->last_checkout); $this->assertEquals('2024-03-28 00:00:00', (string)$asset->expected_checkin); + Event::assertDispatched(CheckoutableCheckedOut::class, 1); Event::assertDispatched(function (CheckoutableCheckedOut $event) use ($admin, $asset, $user) { return $event->checkoutable->is($asset) && $event->checkedOutTo->is($user) diff --git a/tests/Unit/Listeners/LogListenerTest.php b/tests/Unit/Listeners/LogListenerTest.php new file mode 100644 index 000000000..ca20bc0a9 --- /dev/null +++ b/tests/Unit/Listeners/LogListenerTest.php @@ -0,0 +1,37 @@ +create(); + $checkedOutTo = User::factory()->create(); + $checkedOutBy = User::factory()->create(); + $this->actingAs($checkedOutBy); + + (new LogListener())->onCheckoutableCheckedOut(new CheckoutableCheckedOut( + $asset, + $checkedOutTo, + $checkedOutBy, + 'A simple note...', + )); + + $this->assertDatabaseHas('action_logs', [ + 'action_type' => 'checkout', + 'user_id' => $checkedOutBy->id, + 'target_id' => $checkedOutTo->id, + 'target_type' => User::class, + 'item_id' => $asset->id, + 'item_type' => Asset::class, + 'note' => 'A simple note...', + ]); + } +} From 37ebf1827fb0321e27b23dc9dcb733d179034964 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 10 Apr 2024 12:12:44 -0700 Subject: [PATCH 07/33] Organization --- tests/Feature/Checkouts/AssetCheckoutTest.php | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/Feature/Checkouts/AssetCheckoutTest.php b/tests/Feature/Checkouts/AssetCheckoutTest.php index fb72b741b..63a610186 100644 --- a/tests/Feature/Checkouts/AssetCheckoutTest.php +++ b/tests/Feature/Checkouts/AssetCheckoutTest.php @@ -112,6 +112,16 @@ class AssetCheckoutTest extends TestCase }); } + public function testAnAssetCanBeCheckedOutToAnAsset() + { + $this->markTestIncomplete(); + } + + public function testAnAssetCanBeCheckedOutToALocation() + { + $this->markTestIncomplete(); + } + public function testLicenseSeatsAreAssignedToUserUponCheckout() { $asset = Asset::factory()->create(); @@ -129,16 +139,6 @@ class AssetCheckoutTest extends TestCase $this->assertTrue($user->fresh()->licenses->contains($seat->license)); } - public function testAnAssetCanBeCheckedOutToAnAsset() - { - $this->markTestIncomplete(); - } - - public function testAnAssetCanBeCheckedOutToALocation() - { - $this->markTestIncomplete(); - } - public function testLastCheckoutUsesCurrentDateIfNotProvided() { $this->markTestIncomplete(); From 7dbf8a8a8e5d98924532c937d64cee23162779b8 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 10 Apr 2024 13:28:52 -0700 Subject: [PATCH 08/33] Add tests for asset and location check out --- .../Assets/AssetCheckoutController.php | 1 - tests/Feature/Checkouts/AssetCheckoutTest.php | 76 ++++++++++++++----- 2 files changed, 56 insertions(+), 21 deletions(-) diff --git a/app/Http/Controllers/Assets/AssetCheckoutController.php b/app/Http/Controllers/Assets/AssetCheckoutController.php index 46f8c5b8b..84033c231 100644 --- a/app/Http/Controllers/Assets/AssetCheckoutController.php +++ b/app/Http/Controllers/Assets/AssetCheckoutController.php @@ -64,7 +64,6 @@ class AssetCheckoutController extends Controller $target = $this->determineCheckoutTarget($asset); - // this seems to be handled by the checkOut method $asset = $this->updateAssetLocation($asset, $target); $checkout_at = date('Y-m-d H:i:s'); diff --git a/tests/Feature/Checkouts/AssetCheckoutTest.php b/tests/Feature/Checkouts/AssetCheckoutTest.php index 63a610186..ac28a3154 100644 --- a/tests/Feature/Checkouts/AssetCheckoutTest.php +++ b/tests/Feature/Checkouts/AssetCheckoutTest.php @@ -72,22 +72,68 @@ class AssetCheckoutTest extends TestCase ->assertSessionHasErrors(); } - public function testAnAssetCanBeCheckedOutToAUser() + public function assetCheckoutTargets(): array + { + return [ + 'User' => [function () { + $userLocation = Location::factory()->create(); + $user = User::factory()->for($userLocation)->create(); + + return [ + 'checkout_type' => 'user', + 'target' => $user, + 'expected_location' => $userLocation, + ]; + }], + 'Asset without location set' => [function () { + $rtdLocation = Location::factory()->create(); + $asset = Asset::factory()->for($rtdLocation, 'defaultLoc')->create(['location_id' => null]); + + return [ + 'checkout_type' => 'asset', + 'target' => $asset, + 'expected_location' => $rtdLocation, + ]; + }], + 'Asset with location set' => [function () { + $rtdLocation = Location::factory()->create(); + $location = Location::factory()->create(); + $asset = Asset::factory()->for($location)->for($rtdLocation, 'defaultLoc')->create(); + + return [ + 'checkout_type' => 'asset', + 'target' => $asset, + 'expected_location' => $location, + ]; + }], + 'Location' => [function () { + $location = Location::factory()->create(); + + return [ + 'checkout_type' => 'location', + 'target' => $location, + 'expected_location' => $location, + ]; + }], + ]; + } + + /** @dataProvider assetCheckoutTargets */ + public function testAnAssetCanBeCheckedOutToAUser($data) { Event::fake([CheckoutableCheckedOut::class]); + ['checkout_type' => $type, 'target' => $target, 'expected_location' => $expectedLocation] = $data(); + $originalStatus = Statuslabel::factory()->readyToDeploy()->create(); $updatedStatus = Statuslabel::factory()->readyToDeploy()->create(); $asset = Asset::factory()->create(['status_id' => $originalStatus->id]); $admin = User::factory()->checkoutAssets()->create(); - $userLocation = Location::factory()->create(); - - $user = User::factory()->for($userLocation)->create(); $this->actingAs($admin) ->post(route('hardware.checkout.store', $asset), [ - 'checkout_to_type' => 'user', - 'assigned_user' => $user->id, + 'checkout_to_type' => $type, + 'assigned_' . $type => $target->id, 'name' => 'Changed Name', 'status_id' => $updatedStatus->id, 'checkout_at' => '2024-03-18', @@ -96,32 +142,22 @@ class AssetCheckoutTest extends TestCase ]); $asset->refresh(); - $this->assertTrue($asset->assignedTo()->is($user)); - $this->assertTrue($asset->location->is($userLocation)); + $this->assertTrue($asset->assignedTo()->is($target)); + $this->assertTrue($asset->location->is($expectedLocation)); $this->assertEquals('Changed Name', $asset->name); $this->assertTrue($asset->assetstatus->is($updatedStatus)); $this->assertEquals('2024-03-18 00:00:00', $asset->last_checkout); $this->assertEquals('2024-03-28 00:00:00', (string)$asset->expected_checkin); Event::assertDispatched(CheckoutableCheckedOut::class, 1); - Event::assertDispatched(function (CheckoutableCheckedOut $event) use ($admin, $asset, $user) { + Event::assertDispatched(function (CheckoutableCheckedOut $event) use ($admin, $asset, $target) { return $event->checkoutable->is($asset) - && $event->checkedOutTo->is($user) + && $event->checkedOutTo->is($target) && $event->checkedOutBy->is($admin) && $event->note === 'An awesome note'; }); } - public function testAnAssetCanBeCheckedOutToAnAsset() - { - $this->markTestIncomplete(); - } - - public function testAnAssetCanBeCheckedOutToALocation() - { - $this->markTestIncomplete(); - } - public function testLicenseSeatsAreAssignedToUserUponCheckout() { $asset = Asset::factory()->create(); From 72eda1e9094e364cb96ba0011020aa2fe72efe61 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 10 Apr 2024 13:29:31 -0700 Subject: [PATCH 09/33] Improve naming --- tests/Feature/Checkouts/AssetCheckoutTest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/Feature/Checkouts/AssetCheckoutTest.php b/tests/Feature/Checkouts/AssetCheckoutTest.php index ac28a3154..f01b62cee 100644 --- a/tests/Feature/Checkouts/AssetCheckoutTest.php +++ b/tests/Feature/Checkouts/AssetCheckoutTest.php @@ -72,7 +72,7 @@ class AssetCheckoutTest extends TestCase ->assertSessionHasErrors(); } - public function assetCheckoutTargets(): array + public function checkoutTargets(): array { return [ 'User' => [function () { @@ -118,8 +118,8 @@ class AssetCheckoutTest extends TestCase ]; } - /** @dataProvider assetCheckoutTargets */ - public function testAnAssetCanBeCheckedOutToAUser($data) + /** @dataProvider checkoutTargets */ + public function testAnAssetCanBeCheckedOut($data) { Event::fake([CheckoutableCheckedOut::class]); From d371d14c1f0ebf6be375d90a8369fc1b93b5561a Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 10 Apr 2024 13:49:07 -0700 Subject: [PATCH 10/33] Implement test --- tests/Feature/Checkouts/AssetCheckoutTest.php | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/tests/Feature/Checkouts/AssetCheckoutTest.php b/tests/Feature/Checkouts/AssetCheckoutTest.php index f01b62cee..b933ec1c9 100644 --- a/tests/Feature/Checkouts/AssetCheckoutTest.php +++ b/tests/Feature/Checkouts/AssetCheckoutTest.php @@ -8,6 +8,7 @@ use App\Models\LicenseSeat; use App\Models\Location; use App\Models\Statuslabel; use App\Models\User; +use Illuminate\Support\Carbon; use Illuminate\Support\Facades\Event; use Tests\TestCase; @@ -177,6 +178,18 @@ class AssetCheckoutTest extends TestCase public function testLastCheckoutUsesCurrentDateIfNotProvided() { - $this->markTestIncomplete(); + $asset = Asset::factory()->create(); + + $this->actingAs(User::factory()->checkoutAssets()->create()) + ->post(route('hardware.checkout.store', $asset), [ + 'checkout_to_type' => 'user', + 'assigned_user' => User::factory()->create()->id, + ]); + + $asset->refresh(); + + // It's possible that this can be properly mocked in the future: + // https://laravel.com/docs/8.x/mocking#interacting-with-time + $this->assertTrue(Carbon::parse($asset->last_checkout)->diffInSeconds(now()) < 2); } } From 6d572424ac91095615f5eb28d12bc3437bceaaa1 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 10 Apr 2024 14:02:25 -0700 Subject: [PATCH 11/33] Add validation around dates --- app/Http/Requests/AssetCheckoutRequest.php | 8 ++++++++ tests/Feature/Checkouts/AssetCheckoutTest.php | 14 ++++++++++++-- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/app/Http/Requests/AssetCheckoutRequest.php b/app/Http/Requests/AssetCheckoutRequest.php index a4c6f8e00..f48a7d5e5 100644 --- a/app/Http/Requests/AssetCheckoutRequest.php +++ b/app/Http/Requests/AssetCheckoutRequest.php @@ -27,6 +27,14 @@ class AssetCheckoutRequest extends Request 'assigned_location' => 'required_without_all:assigned_user,assigned_asset', 'status_id' => 'exists:status_labels,id,deployable,1', 'checkout_to_type' => 'required|in:asset,location,user', + 'checkout_at' => [ + 'nullable', + 'date', + ], + 'expected_checkin' => [ + 'nullable', + 'date' + ], ]; return $rules; diff --git a/tests/Feature/Checkouts/AssetCheckoutTest.php b/tests/Feature/Checkouts/AssetCheckoutTest.php index b933ec1c9..6f2460e55 100644 --- a/tests/Feature/Checkouts/AssetCheckoutTest.php +++ b/tests/Feature/Checkouts/AssetCheckoutTest.php @@ -68,9 +68,19 @@ class AssetCheckoutTest extends TestCase { $this->actingAs(User::factory()->create()) ->post(route('hardware.checkout.store', Asset::factory()->create()), [ - // + 'status_id' => 'does-not-exist', + 'checkout_at' => 'invalid-date', + 'expected_checkin' => 'invalid-date', ]) - ->assertSessionHasErrors(); + ->assertSessionHasErrors([ + 'assigned_user', + 'assigned_asset', + 'assigned_location', + 'status_id', + 'checkout_to_type', + 'checkout_at', + 'expected_checkin', + ]); } public function checkoutTargets(): array From 4434de6241ed616e967bd212ad0d75007f78449d Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 10 Apr 2024 15:47:26 -0700 Subject: [PATCH 12/33] Add test case --- .../Assets/AssetCheckoutController.php | 1 - tests/Feature/Checkouts/AssetCheckoutTest.php | 23 +++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/app/Http/Controllers/Assets/AssetCheckoutController.php b/app/Http/Controllers/Assets/AssetCheckoutController.php index 84033c231..a096f1667 100644 --- a/app/Http/Controllers/Assets/AssetCheckoutController.php +++ b/app/Http/Controllers/Assets/AssetCheckoutController.php @@ -92,7 +92,6 @@ class AssetCheckoutController extends Controller $settings = \App\Models\Setting::getSettings(); // We have to check whether $target->company_id is null here since locations don't have a company yet - // @todo: test this if (($settings->full_multiple_companies_support) && ((!is_null($target->company_id)) && (!is_null($asset->company_id)))) { if ($target->company_id != $asset->company_id){ return redirect()->to("hardware/$assetId/checkout")->with('error', trans('general.error_user_company')); diff --git a/tests/Feature/Checkouts/AssetCheckoutTest.php b/tests/Feature/Checkouts/AssetCheckoutTest.php index 6f2460e55..8577f638e 100644 --- a/tests/Feature/Checkouts/AssetCheckoutTest.php +++ b/tests/Feature/Checkouts/AssetCheckoutTest.php @@ -4,6 +4,7 @@ namespace Tests\Feature\Checkouts; use App\Events\CheckoutableCheckedOut; use App\Models\Asset; +use App\Models\Company; use App\Models\LicenseSeat; use App\Models\Location; use App\Models\Statuslabel; @@ -169,6 +170,28 @@ class AssetCheckoutTest extends TestCase }); } + public function testCannotCheckoutAcrossCompaniesWhenFullCompanySupportEnabled() + { + Event::fake([CheckoutableCheckedOut::class]); + + $this->settings->enableMultipleFullCompanySupport(); + + $assetCompany = Company::factory()->create(); + $userCompany = Company::factory()->create(); + + $user = User::factory()->for($userCompany)->create(); + $asset = Asset::factory()->for($assetCompany)->create(); + + $this->actingAs(User::factory()->superuser()->create()) + ->post(route('hardware.checkout.store', $asset), [ + 'checkout_to_type' => 'user', + 'assigned_user' => $user->id, + ]) + ->assertRedirect(route('hardware.checkout.store', $asset)); + + Event::assertNotDispatched(CheckoutableCheckedOut::class); + } + public function testLicenseSeatsAreAssignedToUserUponCheckout() { $asset = Asset::factory()->create(); From b63962e90b454fc799d882c3f789499fee729a9b Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 10 Apr 2024 16:01:15 -0700 Subject: [PATCH 13/33] Add additional assertions --- tests/Feature/Checkouts/AssetCheckoutTest.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/Feature/Checkouts/AssetCheckoutTest.php b/tests/Feature/Checkouts/AssetCheckoutTest.php index 8577f638e..c2e428878 100644 --- a/tests/Feature/Checkouts/AssetCheckoutTest.php +++ b/tests/Feature/Checkouts/AssetCheckoutTest.php @@ -39,7 +39,8 @@ class AssetCheckoutTest extends TestCase 'expected_checkin' => '2024-03-28', 'note' => 'An awesome note', ]) - ->assertSessionHas('error'); + ->assertSessionHas('error') + ->assertRedirect(route('hardware.index')); Event::assertNotDispatched(CheckoutableCheckedOut::class); } @@ -60,7 +61,8 @@ class AssetCheckoutTest extends TestCase 'expected_checkin' => '2024-03-28', 'note' => 'An awesome note', ]) - ->assertSessionHas('error'); + ->assertSessionHas('error') + ->assertRedirect(route('hardware.index')); Event::assertNotDispatched(CheckoutableCheckedOut::class); } From 6b6e18695f9e3578c60a1aee1a4855deec493885 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 10 Apr 2024 16:12:47 -0700 Subject: [PATCH 14/33] Account for null asset in factory state --- database/factories/LicenseSeatFactory.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/database/factories/LicenseSeatFactory.php b/database/factories/LicenseSeatFactory.php index d3283d839..f9560af47 100644 --- a/database/factories/LicenseSeatFactory.php +++ b/database/factories/LicenseSeatFactory.php @@ -20,7 +20,7 @@ class LicenseSeatFactory extends Factory { return $this->state(function () use ($asset) { return [ - 'asset_id' => $asset->id, + 'asset_id' => $asset->id ?? Asset::factory(), ]; }); } From bbb2cdcceb27f521177bdff31d9e260576b29930 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 10 Apr 2024 16:15:43 -0700 Subject: [PATCH 15/33] Add note --- tests/Unit/Listeners/LogListenerTest.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/Unit/Listeners/LogListenerTest.php b/tests/Unit/Listeners/LogListenerTest.php index ca20bc0a9..011a5c51a 100644 --- a/tests/Unit/Listeners/LogListenerTest.php +++ b/tests/Unit/Listeners/LogListenerTest.php @@ -15,6 +15,8 @@ class LogListenerTest extends TestCase $asset = Asset::factory()->create(); $checkedOutTo = User::factory()->create(); $checkedOutBy = User::factory()->create(); + + // Simply to ensure `user_id` is set in the action log $this->actingAs($checkedOutBy); (new LogListener())->onCheckoutableCheckedOut(new CheckoutableCheckedOut( From 6666a78936113eeecfd0656c6bd962c9389f067f Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 10 Apr 2024 16:19:04 -0700 Subject: [PATCH 16/33] Organization --- tests/Feature/Checkouts/AssetCheckoutTest.php | 59 ++++++++++--------- 1 file changed, 31 insertions(+), 28 deletions(-) diff --git a/tests/Feature/Checkouts/AssetCheckoutTest.php b/tests/Feature/Checkouts/AssetCheckoutTest.php index c2e428878..c1c1bb869 100644 --- a/tests/Feature/Checkouts/AssetCheckoutTest.php +++ b/tests/Feature/Checkouts/AssetCheckoutTest.php @@ -15,6 +15,13 @@ use Tests\TestCase; class AssetCheckoutTest extends TestCase { + protected function setUp(): void + { + parent::setUp(); + + Event::fake([CheckoutableCheckedOut::class]); + } + public function testCheckingOutAssetRequiresCorrectPermission() { $this->actingAs(User::factory()->create()) @@ -23,12 +30,12 @@ class AssetCheckoutTest extends TestCase 'assigned_user' => User::factory()->create()->id, ]) ->assertForbidden(); + + Event::assertNotDispatched(CheckoutableCheckedOut::class); } public function testNonExistentAssetCannotBeCheckedOut() { - Event::fake([CheckoutableCheckedOut::class]); - $this->actingAs(User::factory()->checkoutAssets()->create()) ->post(route('hardware.checkout.store', 1000), [ 'checkout_to_type' => 'user', @@ -47,8 +54,6 @@ class AssetCheckoutTest extends TestCase public function testAssetNotAvailableForCheckoutCannotBeCheckedOut() { - Event::fake([CheckoutableCheckedOut::class]); - $asset = Asset::factory()->assignedToUser()->create(); $this->actingAs(User::factory()->checkoutAssets()->create()) @@ -84,6 +89,28 @@ class AssetCheckoutTest extends TestCase 'checkout_at', 'expected_checkin', ]); + + Event::assertNotDispatched(CheckoutableCheckedOut::class); + } + + public function testCannotCheckoutAcrossCompaniesWhenFullCompanySupportEnabled() + { + $this->settings->enableMultipleFullCompanySupport(); + + $assetCompany = Company::factory()->create(); + $userCompany = Company::factory()->create(); + + $user = User::factory()->for($userCompany)->create(); + $asset = Asset::factory()->for($assetCompany)->create(); + + $this->actingAs(User::factory()->superuser()->create()) + ->post(route('hardware.checkout.store', $asset), [ + 'checkout_to_type' => 'user', + 'assigned_user' => $user->id, + ]) + ->assertRedirect(route('hardware.checkout.store', $asset)); + + Event::assertNotDispatched(CheckoutableCheckedOut::class); } public function checkoutTargets(): array @@ -135,8 +162,6 @@ class AssetCheckoutTest extends TestCase /** @dataProvider checkoutTargets */ public function testAnAssetCanBeCheckedOut($data) { - Event::fake([CheckoutableCheckedOut::class]); - ['checkout_type' => $type, 'target' => $target, 'expected_location' => $expectedLocation] = $data(); $originalStatus = Statuslabel::factory()->readyToDeploy()->create(); @@ -172,28 +197,6 @@ class AssetCheckoutTest extends TestCase }); } - public function testCannotCheckoutAcrossCompaniesWhenFullCompanySupportEnabled() - { - Event::fake([CheckoutableCheckedOut::class]); - - $this->settings->enableMultipleFullCompanySupport(); - - $assetCompany = Company::factory()->create(); - $userCompany = Company::factory()->create(); - - $user = User::factory()->for($userCompany)->create(); - $asset = Asset::factory()->for($assetCompany)->create(); - - $this->actingAs(User::factory()->superuser()->create()) - ->post(route('hardware.checkout.store', $asset), [ - 'checkout_to_type' => 'user', - 'assigned_user' => $user->id, - ]) - ->assertRedirect(route('hardware.checkout.store', $asset)); - - Event::assertNotDispatched(CheckoutableCheckedOut::class); - } - public function testLicenseSeatsAreAssignedToUserUponCheckout() { $asset = Asset::factory()->create(); From f6ad27503099639965a228fc6ae01e02ab055f48 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 10 Apr 2024 16:28:48 -0700 Subject: [PATCH 17/33] Clean ups --- tests/Feature/Checkouts/AssetCheckoutTest.php | 28 +++++++------------ 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/tests/Feature/Checkouts/AssetCheckoutTest.php b/tests/Feature/Checkouts/AssetCheckoutTest.php index c1c1bb869..8396fd372 100644 --- a/tests/Feature/Checkouts/AssetCheckoutTest.php +++ b/tests/Feature/Checkouts/AssetCheckoutTest.php @@ -30,8 +30,6 @@ class AssetCheckoutTest extends TestCase 'assigned_user' => User::factory()->create()->id, ]) ->assertForbidden(); - - Event::assertNotDispatched(CheckoutableCheckedOut::class); } public function testNonExistentAssetCannotBeCheckedOut() @@ -41,10 +39,6 @@ class AssetCheckoutTest extends TestCase 'checkout_to_type' => 'user', 'assigned_user' => User::factory()->create()->id, 'name' => 'Changed Name', - 'status_id' => Statuslabel::factory()->readyToDeploy()->create()->id, - 'checkout_at' => '2024-03-18', - 'expected_checkin' => '2024-03-28', - 'note' => 'An awesome note', ]) ->assertSessionHas('error') ->assertRedirect(route('hardware.index')); @@ -54,17 +48,12 @@ class AssetCheckoutTest extends TestCase public function testAssetNotAvailableForCheckoutCannotBeCheckedOut() { - $asset = Asset::factory()->assignedToUser()->create(); + $assetAlreadyCheckedOut = Asset::factory()->assignedToUser()->create(); $this->actingAs(User::factory()->checkoutAssets()->create()) - ->post(route('hardware.checkout.store', $asset), [ + ->post(route('hardware.checkout.store', $assetAlreadyCheckedOut), [ 'checkout_to_type' => 'user', 'assigned_user' => User::factory()->create()->id, - 'name' => 'Changed Name', - 'status_id' => Statuslabel::factory()->readyToDeploy()->create()->id, - 'checkout_at' => '2024-03-18', - 'expected_checkin' => '2024-03-28', - 'note' => 'An awesome note', ]) ->assertSessionHas('error') ->assertRedirect(route('hardware.index')); @@ -113,6 +102,10 @@ class AssetCheckoutTest extends TestCase Event::assertNotDispatched(CheckoutableCheckedOut::class); } + /** + * This data provider contains checkout targets along with the + * asset's expected location after the checkout process. + */ public function checkoutTargets(): array { return [ @@ -164,9 +157,8 @@ class AssetCheckoutTest extends TestCase { ['checkout_type' => $type, 'target' => $target, 'expected_location' => $expectedLocation] = $data(); - $originalStatus = Statuslabel::factory()->readyToDeploy()->create(); - $updatedStatus = Statuslabel::factory()->readyToDeploy()->create(); - $asset = Asset::factory()->create(['status_id' => $originalStatus->id]); + $newStatus = Statuslabel::factory()->readyToDeploy()->create(); + $asset = Asset::factory()->create(); $admin = User::factory()->checkoutAssets()->create(); $this->actingAs($admin) @@ -174,7 +166,7 @@ class AssetCheckoutTest extends TestCase 'checkout_to_type' => $type, 'assigned_' . $type => $target->id, 'name' => 'Changed Name', - 'status_id' => $updatedStatus->id, + 'status_id' => $newStatus->id, 'checkout_at' => '2024-03-18', 'expected_checkin' => '2024-03-28', 'note' => 'An awesome note', @@ -184,7 +176,7 @@ class AssetCheckoutTest extends TestCase $this->assertTrue($asset->assignedTo()->is($target)); $this->assertTrue($asset->location->is($expectedLocation)); $this->assertEquals('Changed Name', $asset->name); - $this->assertTrue($asset->assetstatus->is($updatedStatus)); + $this->assertTrue($asset->assetstatus->is($newStatus)); $this->assertEquals('2024-03-18 00:00:00', $asset->last_checkout); $this->assertEquals('2024-03-28 00:00:00', (string)$asset->expected_checkin); From 70934e54cf7f6d459c4d63e0a0c3da4a1da07229 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 10 Apr 2024 16:34:14 -0700 Subject: [PATCH 18/33] Remove unneeded comment --- tests/Feature/Checkouts/AssetCheckoutTest.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/Feature/Checkouts/AssetCheckoutTest.php b/tests/Feature/Checkouts/AssetCheckoutTest.php index 8396fd372..25ff69fa9 100644 --- a/tests/Feature/Checkouts/AssetCheckoutTest.php +++ b/tests/Feature/Checkouts/AssetCheckoutTest.php @@ -218,8 +218,6 @@ class AssetCheckoutTest extends TestCase $asset->refresh(); - // It's possible that this can be properly mocked in the future: - // https://laravel.com/docs/8.x/mocking#interacting-with-time $this->assertTrue(Carbon::parse($asset->last_checkout)->diffInSeconds(now()) < 2); } } From 7c2fae7b9d6b8caa827d291775ad3af74df3d71d Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 10 Apr 2024 17:08:45 -0700 Subject: [PATCH 19/33] Scaffold api test cases --- app/Http/Controllers/Api/AssetsController.php | 10 +++ .../Feature/Api/Assets/AssetCheckoutTest.php | 81 +++++++++++++++++++ 2 files changed, 91 insertions(+) create mode 100644 tests/Feature/Api/Assets/AssetCheckoutTest.php diff --git a/app/Http/Controllers/Api/AssetsController.php b/app/Http/Controllers/Api/AssetsController.php index 92f1038cb..f436990e7 100644 --- a/app/Http/Controllers/Api/AssetsController.php +++ b/app/Http/Controllers/Api/AssetsController.php @@ -823,12 +823,15 @@ class AssetsController extends Controller // This item is checked out to a location if (request('checkout_to_type') == 'location') { + // @todo: test this + dd('asdfasdf'); $target = Location::find(request('assigned_location')); $asset->location_id = ($target) ? $target->id : ''; $error_payload['target_id'] = $request->input('assigned_location'); $error_payload['target_type'] = 'location'; } elseif (request('checkout_to_type') == 'asset') { + // @todo: test this $target = Asset::where('id', '!=', $asset_id)->find(request('assigned_asset')); // Override with the asset's location_id if it has one $asset->location_id = (($target) && (isset($target->location_id))) ? $target->location_id : ''; @@ -836,6 +839,7 @@ class AssetsController extends Controller $error_payload['target_type'] = 'asset'; } elseif (request('checkout_to_type') == 'user') { + // @todo: test this // Fetch the target and set the asset's new location_id $target = User::find(request('assigned_user')); $asset->location_id = (($target) && (isset($target->location_id))) ? $target->location_id : ''; @@ -844,20 +848,26 @@ class AssetsController extends Controller } if ($request->filled('status_id')) { + // @todo: test this $asset->status_id = $request->get('status_id'); } if (! isset($target)) { + // @todo: test this return response()->json(Helper::formatStandardApiResponse('error', $error_payload, 'Checkout target for asset '.e($asset->asset_tag).' is invalid - '.$error_payload['target_type'].' does not exist.')); } + // @todo: test this $checkout_at = request('checkout_at', date('Y-m-d H:i:s')); + // @todo: test this $expected_checkin = request('expected_checkin', null); + // @todo: test this $note = request('note', null); // Using `->has` preserves the asset name if the name parameter was not included in request. + // @todo: test this $asset_name = request()->has('name') ? request('name') : $asset->name; // Set the location ID to the RTD location id if there is one diff --git a/tests/Feature/Api/Assets/AssetCheckoutTest.php b/tests/Feature/Api/Assets/AssetCheckoutTest.php new file mode 100644 index 000000000..6fa578a71 --- /dev/null +++ b/tests/Feature/Api/Assets/AssetCheckoutTest.php @@ -0,0 +1,81 @@ +actingAsForApi(User::factory()->create()) + ->postJson(route('api.asset.checkout', Asset::factory()->create()), [ + 'checkout_to_type' => 'user', + 'assigned_user' => User::factory()->create()->id, + ]) + ->assertForbidden(); + } + + public function testNonExistentAssetCannotBeCheckedOut() + { + $this->actingAsForApi(User::factory()->checkoutAssets()->create()) + ->postJson(route('api.asset.checkout', 1000), [ + 'checkout_to_type' => 'user', + 'assigned_user' => User::factory()->create()->id, + ]) + ->assertStatusMessageIs('error'); + } + + public function testAssetNotAvailableForCheckoutCannotBeCheckedOut() + { + $assetAlreadyCheckedOut = Asset::factory()->assignedToUser()->create(); + + $this->actingAsForApi(User::factory()->checkoutAssets()->create()) + ->postJson(route('api.asset.checkout', $assetAlreadyCheckedOut), [ + 'checkout_to_type' => 'user', + 'assigned_user' => User::factory()->create()->id, + ]) + ->assertStatusMessageIs('error'); + } + + public function testValidationWhenCheckingOutAsset() + { + $this->actingAsForApi(User::factory()->checkoutAssets()->create()) + ->postJson(route('api.asset.checkout', Asset::factory()->create()), []) + ->assertStatusMessageIs('error'); + + Event::assertNotDispatched(CheckoutableCheckedOut::class); + } + + public function testCannotCheckoutAcrossCompaniesWhenFullCompanySupportEnabled() + { + $this->markTestIncomplete('This is not implemented'); + } + + public function testAssetCanBeCheckedOut() + { + $this->markTestIncomplete(); + } + + public function testLicenseSeatsAreAssignedToUserUponCheckout() + { + $this->markTestIncomplete('This is not implemented'); + } + + public function testLastCheckoutUsesCurrentDateIfNotProvided() + { + $this->markTestIncomplete(); + } +} From f28a82de71873e054c9e40410afbab5b83ac28af Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 10 Apr 2024 17:27:07 -0700 Subject: [PATCH 20/33] Implement some tests, scaffold others --- app/Http/Controllers/Api/AssetsController.php | 14 +-- .../Feature/Api/Assets/AssetCheckoutTest.php | 108 +++++++++++++++++- tests/Feature/Checkouts/AssetCheckoutTest.php | 7 +- 3 files changed, 116 insertions(+), 13 deletions(-) diff --git a/app/Http/Controllers/Api/AssetsController.php b/app/Http/Controllers/Api/AssetsController.php index f436990e7..f1f0018a8 100644 --- a/app/Http/Controllers/Api/AssetsController.php +++ b/app/Http/Controllers/Api/AssetsController.php @@ -823,15 +823,14 @@ class AssetsController extends Controller // This item is checked out to a location if (request('checkout_to_type') == 'location') { - // @todo: test this - dd('asdfasdf'); $target = Location::find(request('assigned_location')); + // @todo: test the setting of '' $asset->location_id = ($target) ? $target->id : ''; $error_payload['target_id'] = $request->input('assigned_location'); $error_payload['target_type'] = 'location'; } elseif (request('checkout_to_type') == 'asset') { - // @todo: test this + // @todo: test this code path $target = Asset::where('id', '!=', $asset_id)->find(request('assigned_asset')); // Override with the asset's location_id if it has one $asset->location_id = (($target) && (isset($target->location_id))) ? $target->location_id : ''; @@ -839,28 +838,25 @@ class AssetsController extends Controller $error_payload['target_type'] = 'asset'; } elseif (request('checkout_to_type') == 'user') { - // @todo: test this + // @todo: test this code path // Fetch the target and set the asset's new location_id $target = User::find(request('assigned_user')); + // @todo: test if this is needed or already handled in checkOut method $asset->location_id = (($target) && (isset($target->location_id))) ? $target->location_id : ''; $error_payload['target_id'] = $request->input('assigned_user'); $error_payload['target_type'] = 'user'; } if ($request->filled('status_id')) { - // @todo: test this $asset->status_id = $request->get('status_id'); } if (! isset($target)) { - // @todo: test this + // @todo: test this code path return response()->json(Helper::formatStandardApiResponse('error', $error_payload, 'Checkout target for asset '.e($asset->asset_tag).' is invalid - '.$error_payload['target_type'].' does not exist.')); } - - - // @todo: test this $checkout_at = request('checkout_at', date('Y-m-d H:i:s')); // @todo: test this $expected_checkin = request('expected_checkin', null); diff --git a/tests/Feature/Api/Assets/AssetCheckoutTest.php b/tests/Feature/Api/Assets/AssetCheckoutTest.php index 6fa578a71..b902ba418 100644 --- a/tests/Feature/Api/Assets/AssetCheckoutTest.php +++ b/tests/Feature/Api/Assets/AssetCheckoutTest.php @@ -4,7 +4,10 @@ namespace Tests\Feature\Api\Assets; use App\Events\CheckoutableCheckedOut; use App\Models\Asset; +use App\Models\Location; +use App\Models\Statuslabel; use App\Models\User; +use Illuminate\Support\Carbon; use Illuminate\Support\Facades\Event; use Tests\TestCase; @@ -50,6 +53,11 @@ class AssetCheckoutTest extends TestCase ->assertStatusMessageIs('error'); } + public function testAssetCannotBeCheckedOutToItself() + { + $this->markTestIncomplete(); + } + public function testValidationWhenCheckingOutAsset() { $this->actingAsForApi(User::factory()->checkoutAssets()->create()) @@ -64,9 +72,93 @@ class AssetCheckoutTest extends TestCase $this->markTestIncomplete('This is not implemented'); } - public function testAssetCanBeCheckedOut() + /** + * This data provider contains checkout targets along with the + * asset's expected location after the checkout process. + */ + public function checkoutTargets(): array { - $this->markTestIncomplete(); + return [ + 'User' => [function () { + $userLocation = Location::factory()->create(); + $user = User::factory()->for($userLocation)->create(); + + return [ + 'checkout_type' => 'user', + 'target' => $user, + 'expected_location' => $userLocation, + ]; + }], + 'Asset without location set' => [function () { + $rtdLocation = Location::factory()->create(); + $asset = Asset::factory()->for($rtdLocation, 'defaultLoc')->create(['location_id' => null]); + + return [ + 'checkout_type' => 'asset', + 'target' => $asset, + 'expected_location' => $rtdLocation, + ]; + }], + 'Asset with location set' => [function () { + $rtdLocation = Location::factory()->create(); + $location = Location::factory()->create(); + $asset = Asset::factory()->for($location)->for($rtdLocation, 'defaultLoc')->create(); + + return [ + 'checkout_type' => 'asset', + 'target' => $asset, + 'expected_location' => $location, + ]; + }], + 'Location' => [function () { + $location = Location::factory()->create(); + + return [ + 'checkout_type' => 'location', + 'target' => $location, + 'expected_location' => $location, + ]; + }], + ]; + } + + /** @dataProvider checkoutTargets */ + public function testAssetCanBeCheckedOut($data) + { + // $this->markTestIncomplete(); + + ['checkout_type' => $type, 'target' => $target, 'expected_location' => $expectedLocation] = $data(); + + $newStatus = Statuslabel::factory()->readyToDeploy()->create(); + $asset = Asset::factory()->create(); + $admin = User::factory()->checkoutAssets()->create(); + + $this->actingAsForApi($admin) + ->postJson(route('api.asset.checkout', $asset), [ + 'checkout_to_type' => $type, + 'assigned_' . $type => $target->id, + 'status_id' => $newStatus->id, + 'checkout_at' => '2024-04-01', + 'expected_checkin' => '2024-04-08', + 'name' => 'Changed Name', + 'note' => 'Here is a cool note!', + ]); + + $asset->refresh(); + $this->assertTrue($asset->assignedTo()->is($target)); + $this->assertTrue($asset->location->is($expectedLocation)); + $this->assertEquals('Changed Name', $asset->name); + $this->assertTrue($asset->assetstatus->is($newStatus)); + $this->assertEquals('2024-04-01 00:00:00', $asset->last_checkout); + $this->assertEquals('2024-04-08 00:00:00', (string)$asset->expected_checkin); + + Event::assertDispatched(CheckoutableCheckedOut::class, 1); + Event::assertDispatched(function (CheckoutableCheckedOut $event) use ($admin, $asset, $target) { + return $event->checkoutable->is($asset) + && $event->checkedOutTo->is($target) + && $event->checkedOutBy->is($admin) + && $event->note === 'Here is a cool note!'; + }); } public function testLicenseSeatsAreAssignedToUserUponCheckout() @@ -76,6 +168,16 @@ class AssetCheckoutTest extends TestCase public function testLastCheckoutUsesCurrentDateIfNotProvided() { - $this->markTestIncomplete(); + $asset = Asset::factory()->create(['last_checkout' => now()->subMonth()]); + + $this->actingAsForApi(User::factory()->checkoutAssets()->create()) + ->postJson(route('api.asset.checkout', $asset), [ + 'checkout_to_type' => 'user', + 'assigned_user' => User::factory()->create()->id, + ]); + + $asset->refresh(); + + $this->assertTrue(Carbon::parse($asset->last_checkout)->diffInSeconds(now()) < 2); } } diff --git a/tests/Feature/Checkouts/AssetCheckoutTest.php b/tests/Feature/Checkouts/AssetCheckoutTest.php index 25ff69fa9..b3f8daab9 100644 --- a/tests/Feature/Checkouts/AssetCheckoutTest.php +++ b/tests/Feature/Checkouts/AssetCheckoutTest.php @@ -61,6 +61,11 @@ class AssetCheckoutTest extends TestCase Event::assertNotDispatched(CheckoutableCheckedOut::class); } + public function testAssetCannotBeCheckedOutToItself() + { + $this->markTestIncomplete(); + } + public function testValidationWhenCheckingOutAsset() { $this->actingAs(User::factory()->create()) @@ -208,7 +213,7 @@ class AssetCheckoutTest extends TestCase public function testLastCheckoutUsesCurrentDateIfNotProvided() { - $asset = Asset::factory()->create(); + $asset = Asset::factory()->create(['last_checkout' => now()->subMonth()]); $this->actingAs(User::factory()->checkoutAssets()->create()) ->post(route('hardware.checkout.store', $asset), [ From fa5016713fadb10ad64e1a055c02161b8e73bb73 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 10 Apr 2024 17:29:44 -0700 Subject: [PATCH 21/33] Implement test --- tests/Feature/Api/Assets/AssetCheckoutTest.php | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/tests/Feature/Api/Assets/AssetCheckoutTest.php b/tests/Feature/Api/Assets/AssetCheckoutTest.php index b902ba418..0918f050b 100644 --- a/tests/Feature/Api/Assets/AssetCheckoutTest.php +++ b/tests/Feature/Api/Assets/AssetCheckoutTest.php @@ -55,7 +55,14 @@ class AssetCheckoutTest extends TestCase public function testAssetCannotBeCheckedOutToItself() { - $this->markTestIncomplete(); + $asset = Asset::factory()->create(); + + $this->actingAsForApi(User::factory()->checkoutAssets()->create()) + ->postJson(route('api.asset.checkout', $asset), [ + 'checkout_to_type' => 'asset', + 'assigned_asset' => $asset->id, + ]) + ->assertStatusMessageIs('error'); } public function testValidationWhenCheckingOutAsset() @@ -125,8 +132,6 @@ class AssetCheckoutTest extends TestCase /** @dataProvider checkoutTargets */ public function testAssetCanBeCheckedOut($data) { - // $this->markTestIncomplete(); - ['checkout_type' => $type, 'target' => $target, 'expected_location' => $expectedLocation] = $data(); $newStatus = Statuslabel::factory()->readyToDeploy()->create(); From 5567a1e9ac196fc9f229f8b203401967fe5eb149 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 10 Apr 2024 17:37:11 -0700 Subject: [PATCH 22/33] Formatting --- app/Http/Controllers/Api/AssetsController.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/app/Http/Controllers/Api/AssetsController.php b/app/Http/Controllers/Api/AssetsController.php index f1f0018a8..74ba2cf6d 100644 --- a/app/Http/Controllers/Api/AssetsController.php +++ b/app/Http/Controllers/Api/AssetsController.php @@ -820,7 +820,6 @@ class AssetsController extends Controller 'asset_tag' => $asset->asset_tag, ]; - // This item is checked out to a location if (request('checkout_to_type') == 'location') { $target = Location::find(request('assigned_location')); @@ -851,7 +850,6 @@ class AssetsController extends Controller $asset->status_id = $request->get('status_id'); } - if (! isset($target)) { // @todo: test this code path return response()->json(Helper::formatStandardApiResponse('error', $error_payload, 'Checkout target for asset '.e($asset->asset_tag).' is invalid - '.$error_payload['target_type'].' does not exist.')); @@ -875,8 +873,6 @@ class AssetsController extends Controller // $asset->location_id = $target->rtd_location_id; // } - - if ($asset->checkOut($target, Auth::user(), $checkout_at, $expected_checkin, $note, $asset_name, $asset->location_id)) { return response()->json(Helper::formatStandardApiResponse('success', ['asset'=> e($asset->asset_tag)], trans('admin/hardware/message.checkout.success'))); } From 1935a4aca3a039e505b9a42d77a22bb1420a27ae Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 22 May 2024 12:47:33 -0700 Subject: [PATCH 23/33] Improve scenario naming --- .../Feature/Api/Assets/AssetCheckoutTest.php | 81 ++++++++++--------- 1 file changed, 44 insertions(+), 37 deletions(-) diff --git a/tests/Feature/Api/Assets/AssetCheckoutTest.php b/tests/Feature/Api/Assets/AssetCheckoutTest.php index 0918f050b..a73bbc708 100644 --- a/tests/Feature/Api/Assets/AssetCheckoutTest.php +++ b/tests/Feature/Api/Assets/AssetCheckoutTest.php @@ -13,7 +13,6 @@ use Tests\TestCase; class AssetCheckoutTest extends TestCase { - protected function setUp(): void { parent::setUp(); @@ -86,46 +85,54 @@ class AssetCheckoutTest extends TestCase public function checkoutTargets(): array { return [ - 'User' => [function () { - $userLocation = Location::factory()->create(); - $user = User::factory()->for($userLocation)->create(); + 'Checkout to User' => [ + function () { + $userLocation = Location::factory()->create(); + $user = User::factory()->for($userLocation)->create(); - return [ - 'checkout_type' => 'user', - 'target' => $user, - 'expected_location' => $userLocation, - ]; - }], - 'Asset without location set' => [function () { - $rtdLocation = Location::factory()->create(); - $asset = Asset::factory()->for($rtdLocation, 'defaultLoc')->create(['location_id' => null]); + return [ + 'checkout_type' => 'user', + 'target' => $user, + 'expected_location' => $userLocation, + ]; + } + ], + 'Checkout to Asset without location set' => [ + function () { + $rtdLocation = Location::factory()->create(); + $asset = Asset::factory()->for($rtdLocation, 'defaultLoc')->create(['location_id' => null]); - return [ - 'checkout_type' => 'asset', - 'target' => $asset, - 'expected_location' => $rtdLocation, - ]; - }], - 'Asset with location set' => [function () { - $rtdLocation = Location::factory()->create(); - $location = Location::factory()->create(); - $asset = Asset::factory()->for($location)->for($rtdLocation, 'defaultLoc')->create(); + return [ + 'checkout_type' => 'asset', + 'target' => $asset, + 'expected_location' => $rtdLocation, + ]; + } + ], + 'Checkout to Asset with location set' => [ + function () { + $rtdLocation = Location::factory()->create(); + $location = Location::factory()->create(); + $asset = Asset::factory()->for($location)->for($rtdLocation, 'defaultLoc')->create(); - return [ - 'checkout_type' => 'asset', - 'target' => $asset, - 'expected_location' => $location, - ]; - }], - 'Location' => [function () { - $location = Location::factory()->create(); + return [ + 'checkout_type' => 'asset', + 'target' => $asset, + 'expected_location' => $location, + ]; + } + ], + 'Checkout to Location' => [ + function () { + $location = Location::factory()->create(); - return [ - 'checkout_type' => 'location', - 'target' => $location, - 'expected_location' => $location, - ]; - }], + return [ + 'checkout_type' => 'location', + 'target' => $location, + 'expected_location' => $location, + ]; + } + ], ]; } From 5d368990dc3ee6ba9f2dc99c893397e824113c4c Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 22 May 2024 13:15:51 -0700 Subject: [PATCH 24/33] Fix assertion --- tests/Feature/Api/Assets/AssetCheckoutTest.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/Feature/Api/Assets/AssetCheckoutTest.php b/tests/Feature/Api/Assets/AssetCheckoutTest.php index a73bbc708..22afb966b 100644 --- a/tests/Feature/Api/Assets/AssetCheckoutTest.php +++ b/tests/Feature/Api/Assets/AssetCheckoutTest.php @@ -105,7 +105,7 @@ class AssetCheckoutTest extends TestCase return [ 'checkout_type' => 'asset', 'target' => $asset, - 'expected_location' => $rtdLocation, + 'expected_location' => null, ]; } ], @@ -158,12 +158,15 @@ class AssetCheckoutTest extends TestCase $asset->refresh(); $this->assertTrue($asset->assignedTo()->is($target)); - $this->assertTrue($asset->location->is($expectedLocation)); $this->assertEquals('Changed Name', $asset->name); $this->assertTrue($asset->assetstatus->is($newStatus)); $this->assertEquals('2024-04-01 00:00:00', $asset->last_checkout); $this->assertEquals('2024-04-08 00:00:00', (string)$asset->expected_checkin); + $expectedLocation + ? $this->assertTrue($asset->location->is($expectedLocation)) + : $this->assertNull($asset->location); + Event::assertDispatched(CheckoutableCheckedOut::class, 1); Event::assertDispatched(function (CheckoutableCheckedOut $event) use ($admin, $asset, $target) { return $event->checkoutable->is($asset) From 8ca882d1c8b8495b2c0f7c6de445eee4931d274a Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 22 May 2024 13:23:49 -0700 Subject: [PATCH 25/33] Complete a scenario --- app/Http/Controllers/Api/AssetsController.php | 1 - tests/Feature/Api/Assets/AssetCheckoutTest.php | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/app/Http/Controllers/Api/AssetsController.php b/app/Http/Controllers/Api/AssetsController.php index 74ba2cf6d..b1697d7c6 100644 --- a/app/Http/Controllers/Api/AssetsController.php +++ b/app/Http/Controllers/Api/AssetsController.php @@ -829,7 +829,6 @@ class AssetsController extends Controller $error_payload['target_type'] = 'location'; } elseif (request('checkout_to_type') == 'asset') { - // @todo: test this code path $target = Asset::where('id', '!=', $asset_id)->find(request('assigned_asset')); // Override with the asset's location_id if it has one $asset->location_id = (($target) && (isset($target->location_id))) ? $target->location_id : ''; diff --git a/tests/Feature/Api/Assets/AssetCheckoutTest.php b/tests/Feature/Api/Assets/AssetCheckoutTest.php index 22afb966b..34183650c 100644 --- a/tests/Feature/Api/Assets/AssetCheckoutTest.php +++ b/tests/Feature/Api/Assets/AssetCheckoutTest.php @@ -142,7 +142,7 @@ class AssetCheckoutTest extends TestCase ['checkout_type' => $type, 'target' => $target, 'expected_location' => $expectedLocation] = $data(); $newStatus = Statuslabel::factory()->readyToDeploy()->create(); - $asset = Asset::factory()->create(); + $asset = Asset::factory()->forLocation()->create(); $admin = User::factory()->checkoutAssets()->create(); $this->actingAsForApi($admin) From c7fa2c04ad1685e372ed0b37bfb19ae478232fbd Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 22 May 2024 13:33:12 -0700 Subject: [PATCH 26/33] Add scenario --- app/Http/Controllers/Api/AssetsController.php | 3 --- tests/Feature/Api/Assets/AssetCheckoutTest.php | 12 ++++++++++++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/app/Http/Controllers/Api/AssetsController.php b/app/Http/Controllers/Api/AssetsController.php index b1697d7c6..bfdc0255e 100644 --- a/app/Http/Controllers/Api/AssetsController.php +++ b/app/Http/Controllers/Api/AssetsController.php @@ -823,7 +823,6 @@ class AssetsController extends Controller // This item is checked out to a location if (request('checkout_to_type') == 'location') { $target = Location::find(request('assigned_location')); - // @todo: test the setting of '' $asset->location_id = ($target) ? $target->id : ''; $error_payload['target_id'] = $request->input('assigned_location'); $error_payload['target_type'] = 'location'; @@ -836,10 +835,8 @@ class AssetsController extends Controller $error_payload['target_type'] = 'asset'; } elseif (request('checkout_to_type') == 'user') { - // @todo: test this code path // Fetch the target and set the asset's new location_id $target = User::find(request('assigned_user')); - // @todo: test if this is needed or already handled in checkOut method $asset->location_id = (($target) && (isset($target->location_id))) ? $target->location_id : ''; $error_payload['target_id'] = $request->input('assigned_user'); $error_payload['target_type'] = 'user'; diff --git a/tests/Feature/Api/Assets/AssetCheckoutTest.php b/tests/Feature/Api/Assets/AssetCheckoutTest.php index 34183650c..1199f8c6d 100644 --- a/tests/Feature/Api/Assets/AssetCheckoutTest.php +++ b/tests/Feature/Api/Assets/AssetCheckoutTest.php @@ -85,6 +85,18 @@ class AssetCheckoutTest extends TestCase public function checkoutTargets(): array { return [ + 'Checkout to User without location set' => [ + function () { + $userLocation = Location::factory()->create(); + $user = User::factory()->for($userLocation)->create(['location_id' => null]); + + return [ + 'checkout_type' => 'user', + 'target' => $user, + 'expected_location' => null, + ]; + } + ], 'Checkout to User' => [ function () { $userLocation = Location::factory()->create(); From 1fe22e4b7bc34a8a59329f33cc4c620d83014f34 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 22 May 2024 13:33:41 -0700 Subject: [PATCH 27/33] Re-order scenarios --- .../Feature/Api/Assets/AssetCheckoutTest.php | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/tests/Feature/Api/Assets/AssetCheckoutTest.php b/tests/Feature/Api/Assets/AssetCheckoutTest.php index 1199f8c6d..471b67a00 100644 --- a/tests/Feature/Api/Assets/AssetCheckoutTest.php +++ b/tests/Feature/Api/Assets/AssetCheckoutTest.php @@ -85,18 +85,6 @@ class AssetCheckoutTest extends TestCase public function checkoutTargets(): array { return [ - 'Checkout to User without location set' => [ - function () { - $userLocation = Location::factory()->create(); - $user = User::factory()->for($userLocation)->create(['location_id' => null]); - - return [ - 'checkout_type' => 'user', - 'target' => $user, - 'expected_location' => null, - ]; - } - ], 'Checkout to User' => [ function () { $userLocation = Location::factory()->create(); @@ -109,14 +97,14 @@ class AssetCheckoutTest extends TestCase ]; } ], - 'Checkout to Asset without location set' => [ + 'Checkout to User without location set' => [ function () { - $rtdLocation = Location::factory()->create(); - $asset = Asset::factory()->for($rtdLocation, 'defaultLoc')->create(['location_id' => null]); + $userLocation = Location::factory()->create(); + $user = User::factory()->for($userLocation)->create(['location_id' => null]); return [ - 'checkout_type' => 'asset', - 'target' => $asset, + 'checkout_type' => 'user', + 'target' => $user, 'expected_location' => null, ]; } @@ -134,6 +122,18 @@ class AssetCheckoutTest extends TestCase ]; } ], + 'Checkout to Asset without location set' => [ + function () { + $rtdLocation = Location::factory()->create(); + $asset = Asset::factory()->for($rtdLocation, 'defaultLoc')->create(['location_id' => null]); + + return [ + 'checkout_type' => 'asset', + 'target' => $asset, + 'expected_location' => null, + ]; + } + ], 'Checkout to Location' => [ function () { $location = Location::factory()->create(); From f16c79bb9a74b44c4e5bca6de01923ee627fc81a Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 22 May 2024 15:38:23 -0700 Subject: [PATCH 28/33] Improve event assertions --- app/Http/Controllers/Api/AssetsController.php | 3 --- tests/Feature/Api/Assets/AssetCheckoutTest.php | 17 ++++++++++------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/app/Http/Controllers/Api/AssetsController.php b/app/Http/Controllers/Api/AssetsController.php index bfdc0255e..b5ef2e43c 100644 --- a/app/Http/Controllers/Api/AssetsController.php +++ b/app/Http/Controllers/Api/AssetsController.php @@ -852,12 +852,9 @@ class AssetsController extends Controller } $checkout_at = request('checkout_at', date('Y-m-d H:i:s')); - // @todo: test this $expected_checkin = request('expected_checkin', null); - // @todo: test this $note = request('note', null); // Using `->has` preserves the asset name if the name parameter was not included in request. - // @todo: test this $asset_name = request()->has('name') ? request('name') : $asset->name; // Set the location ID to the RTD location id if there is one diff --git a/tests/Feature/Api/Assets/AssetCheckoutTest.php b/tests/Feature/Api/Assets/AssetCheckoutTest.php index 471b67a00..82d572ef5 100644 --- a/tests/Feature/Api/Assets/AssetCheckoutTest.php +++ b/tests/Feature/Api/Assets/AssetCheckoutTest.php @@ -160,20 +160,21 @@ class AssetCheckoutTest extends TestCase $this->actingAsForApi($admin) ->postJson(route('api.asset.checkout', $asset), [ 'checkout_to_type' => $type, - 'assigned_' . $type => $target->id, + 'assigned_'.$type => $target->id, 'status_id' => $newStatus->id, 'checkout_at' => '2024-04-01', 'expected_checkin' => '2024-04-08', 'name' => 'Changed Name', 'note' => 'Here is a cool note!', - ]); + ]) + ->assertOk(); $asset->refresh(); $this->assertTrue($asset->assignedTo()->is($target)); $this->assertEquals('Changed Name', $asset->name); $this->assertTrue($asset->assetstatus->is($newStatus)); $this->assertEquals('2024-04-01 00:00:00', $asset->last_checkout); - $this->assertEquals('2024-04-08 00:00:00', (string)$asset->expected_checkin); + $this->assertEquals('2024-04-08 00:00:00', (string) $asset->expected_checkin); $expectedLocation ? $this->assertTrue($asset->location->is($expectedLocation)) @@ -181,10 +182,12 @@ class AssetCheckoutTest extends TestCase Event::assertDispatched(CheckoutableCheckedOut::class, 1); Event::assertDispatched(function (CheckoutableCheckedOut $event) use ($admin, $asset, $target) { - return $event->checkoutable->is($asset) - && $event->checkedOutTo->is($target) - && $event->checkedOutBy->is($admin) - && $event->note === 'Here is a cool note!'; + $this->assertTrue($event->checkoutable->is($asset)); + $this->assertTrue($event->checkedOutTo->is($target)); + $this->assertTrue($event->checkedOutBy->is($admin)); + $this->assertEquals('Here is a cool note!', $event->note); + + return true; }); } From 6d104251b3193f99062ce3b84841cc45bbaad38a Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 22 May 2024 15:40:54 -0700 Subject: [PATCH 29/33] Remove todo --- app/Http/Controllers/Api/AssetsController.php | 1 - 1 file changed, 1 deletion(-) diff --git a/app/Http/Controllers/Api/AssetsController.php b/app/Http/Controllers/Api/AssetsController.php index b5ef2e43c..7e941c342 100644 --- a/app/Http/Controllers/Api/AssetsController.php +++ b/app/Http/Controllers/Api/AssetsController.php @@ -847,7 +847,6 @@ class AssetsController extends Controller } if (! isset($target)) { - // @todo: test this code path return response()->json(Helper::formatStandardApiResponse('error', $error_payload, 'Checkout target for asset '.e($asset->asset_tag).' is invalid - '.$error_payload['target_type'].' does not exist.')); } From a3389a31cd3eb6f6cf280acfcbdcbbf7143d2724 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 22 May 2024 17:10:16 -0700 Subject: [PATCH 30/33] Update test name and add todo --- tests/Feature/Checkouts/AssetCheckoutTest.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/Feature/Checkouts/AssetCheckoutTest.php b/tests/Feature/Checkouts/AssetCheckoutTest.php index b3f8daab9..96752537e 100644 --- a/tests/Feature/Checkouts/AssetCheckoutTest.php +++ b/tests/Feature/Checkouts/AssetCheckoutTest.php @@ -63,6 +63,7 @@ class AssetCheckoutTest extends TestCase public function testAssetCannotBeCheckedOutToItself() { + // @todo: $this->markTestIncomplete(); } @@ -158,7 +159,7 @@ class AssetCheckoutTest extends TestCase } /** @dataProvider checkoutTargets */ - public function testAnAssetCanBeCheckedOut($data) + public function testAssetCanBeCheckedOut($data) { ['checkout_type' => $type, 'target' => $target, 'expected_location' => $expectedLocation] = $data(); From 67c4fa296671e45546fdfcb510845a9241cbbb09 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 22 May 2024 17:14:10 -0700 Subject: [PATCH 31/33] Improve event assertions --- tests/Feature/Checkouts/AssetCheckoutTest.php | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/Feature/Checkouts/AssetCheckoutTest.php b/tests/Feature/Checkouts/AssetCheckoutTest.php index 96752537e..7f3de6bf1 100644 --- a/tests/Feature/Checkouts/AssetCheckoutTest.php +++ b/tests/Feature/Checkouts/AssetCheckoutTest.php @@ -188,10 +188,12 @@ class AssetCheckoutTest extends TestCase Event::assertDispatched(CheckoutableCheckedOut::class, 1); Event::assertDispatched(function (CheckoutableCheckedOut $event) use ($admin, $asset, $target) { - return $event->checkoutable->is($asset) - && $event->checkedOutTo->is($target) - && $event->checkedOutBy->is($admin) - && $event->note === 'An awesome note'; + $this->assertTrue($event->checkoutable->is($asset)); + $this->assertTrue($event->checkedOutTo->is($target)); + $this->assertTrue($event->checkedOutBy->is($admin)); + $this->assertEquals('An awesome note', $event->note); + + return true; }); } From 482ebfbb68bd6cc762cc34b0256b55777c97870d Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 22 May 2024 17:21:29 -0700 Subject: [PATCH 32/33] Implement test --- tests/Feature/Checkouts/AssetCheckoutTest.php | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/tests/Feature/Checkouts/AssetCheckoutTest.php b/tests/Feature/Checkouts/AssetCheckoutTest.php index 7f3de6bf1..fd0408b45 100644 --- a/tests/Feature/Checkouts/AssetCheckoutTest.php +++ b/tests/Feature/Checkouts/AssetCheckoutTest.php @@ -63,8 +63,16 @@ class AssetCheckoutTest extends TestCase public function testAssetCannotBeCheckedOutToItself() { - // @todo: - $this->markTestIncomplete(); + $asset = Asset::factory()->create(); + + $this->actingAs(User::factory()->checkoutAssets()->create()) + ->post(route('hardware.checkout.store', $asset), [ + 'checkout_to_type' => 'asset', + 'assigned_asset' => $asset->id, + ]) + ->assertSessionHas('error'); + + Event::assertNotDispatched(CheckoutableCheckedOut::class); } public function testValidationWhenCheckingOutAsset() From 28b450fd3cb11857235540447abd15b37bdc6ed4 Mon Sep 17 00:00:00 2001 From: Brady Wetherington Date: Mon, 27 May 2024 15:38:42 +0100 Subject: [PATCH 33/33] Add index to 'parent_id' for users with large number of locations --- ...43554_add_parent_id_index_to_locations.php | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 database/migrations/2024_05_27_143554_add_parent_id_index_to_locations.php diff --git a/database/migrations/2024_05_27_143554_add_parent_id_index_to_locations.php b/database/migrations/2024_05_27_143554_add_parent_id_index_to_locations.php new file mode 100644 index 000000000..2aef11918 --- /dev/null +++ b/database/migrations/2024_05_27_143554_add_parent_id_index_to_locations.php @@ -0,0 +1,33 @@ +index('parent_id'); + }); + } + + /** + * Reverse the migrations. + * + * @return void + */ + public function down() + { + Schema::table('locations', function (Blueprint $table) { + // + $table->dropIndex('locations_parent_id_index'); + }); + } +}