From c81bc1d2ee853fa8ab70238a0da1986dbc28e100 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Mon, 12 Feb 2024 17:54:22 -0800 Subject: [PATCH 01/31] Scaffold tests around asset check in --- .../{Assets => Checkins}/AssetCheckinTest.php | 26 ++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) rename tests/Feature/{Assets => Checkins}/AssetCheckinTest.php (53%) diff --git a/tests/Feature/Assets/AssetCheckinTest.php b/tests/Feature/Checkins/AssetCheckinTest.php similarity index 53% rename from tests/Feature/Assets/AssetCheckinTest.php rename to tests/Feature/Checkins/AssetCheckinTest.php index 059fb1294f..2ec5f78318 100644 --- a/tests/Feature/Assets/AssetCheckinTest.php +++ b/tests/Feature/Checkins/AssetCheckinTest.php @@ -1,6 +1,6 @@ actingAs(User::factory()->create()) + ->post(route('hardware.checkin.store', [ + 'assetId' => Asset::factory()->assignedToUser()->create()->id, + ])) + ->assertForbidden(); + } + + public function testAssetCanBeCheckedIn() + { + $this->markTestIncomplete(); + } + + public function testCheckInEmailSentToUserIfSettingEnabled() + { + $this->markTestIncomplete(); + } + + public function testCheckInEmailNotSentToUserIfSettingDisabled() + { + $this->markTestIncomplete(); + } + public function testLastCheckInFieldIsSetOnCheckin() { $admin = User::factory()->superuser()->create(); From 307b39bd38956a2b441d9d09f1fec8fe78b84fd3 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Tue, 13 Feb 2024 12:03:27 -0800 Subject: [PATCH 02/31] Implement tests around asset check in --- database/factories/AssetFactory.php | 6 +- tests/Feature/Checkins/AssetCheckinTest.php | 64 +++++++++++++++++++-- 2 files changed, 62 insertions(+), 8 deletions(-) diff --git a/database/factories/AssetFactory.php b/database/factories/AssetFactory.php index 432495bcc7..3fa0ae1d40 100644 --- a/database/factories/AssetFactory.php +++ b/database/factories/AssetFactory.php @@ -289,11 +289,11 @@ class AssetFactory extends Factory }); } - public function assignedToUser() + public function assignedToUser(User $user = null) { - return $this->state(function () { + return $this->state(function () use ($user) { return [ - 'assigned_to' => User::factory(), + 'assigned_to' => $user->id ?? User::factory(), 'assigned_type' => User::class, ]; }); diff --git a/tests/Feature/Checkins/AssetCheckinTest.php b/tests/Feature/Checkins/AssetCheckinTest.php index 2ec5f78318..d43cdfb24f 100644 --- a/tests/Feature/Checkins/AssetCheckinTest.php +++ b/tests/Feature/Checkins/AssetCheckinTest.php @@ -2,8 +2,12 @@ namespace Tests\Feature\Checkins; +use App\Events\CheckoutableCheckedIn; use App\Models\Asset; use App\Models\User; +use App\Notifications\CheckinAssetNotification; +use Illuminate\Support\Facades\Event; +use Illuminate\Support\Facades\Notification; use Tests\Support\InteractsWithSettings; use Tests\TestCase; @@ -22,17 +26,68 @@ class AssetCheckinTest extends TestCase public function testAssetCanBeCheckedIn() { - $this->markTestIncomplete(); + Event::fake([CheckoutableCheckedIn::class]); + + $admin = User::factory()->checkinAssets()->create(); + $user = User::factory()->create(); + $asset = Asset::factory()->assignedToUser($user)->create(['last_checkin' => null]); + + $this->assertTrue($asset->assignedTo->is($user)); + + $this->actingAs($admin) + ->post(route('hardware.checkin.store', ['assetId' => $asset->id])) + ->assertRedirect(); + + $this->assertNull($asset->fresh()->assignedTo); + Event::assertDispatched(CheckoutableCheckedIn::class, 1); } public function testCheckInEmailSentToUserIfSettingEnabled() { - $this->markTestIncomplete(); + Notification::fake(); + + $user = User::factory()->create(); + $asset = Asset::factory()->assignedToUser($user)->create(); + + $asset->model->category->update(['checkin_email' => true]); + + event(new CheckoutableCheckedIn( + $asset, + $user, + User::factory()->checkinAssets()->create(), + '' + )); + + Notification::assertSentTo( + [$user], + function (CheckinAssetNotification $notification, $channels) { + return in_array('mail', $channels); + }, + ); } public function testCheckInEmailNotSentToUserIfSettingDisabled() { - $this->markTestIncomplete(); + Notification::fake(); + + $user = User::factory()->create(); + $asset = Asset::factory()->assignedToUser($user)->create(); + + $asset->model->category->update(['checkin_email' => false]); + + event(new CheckoutableCheckedIn( + $asset, + $user, + User::factory()->checkinAssets()->create(), + '' + )); + + Notification::assertNotSentTo( + [$user], + function (CheckinAssetNotification $notification, $channels) { + return in_array('mail', $channels); + } + ); } public function testLastCheckInFieldIsSetOnCheckin() @@ -45,8 +100,7 @@ class AssetCheckinTest extends TestCase $this->actingAs($admin) ->post(route('hardware.checkin.store', [ 'assetId' => $asset->id, - ])) - ->assertRedirect(); + ])); $this->assertNotNull( $asset->fresh()->last_checkin, From 852b0b3f11c170d1d99a0efc43e4d1460d92ecd1 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Tue, 13 Feb 2024 12:15:59 -0800 Subject: [PATCH 03/31] Scaffold additional tests --- tests/Feature/Api/Assets/AssetCheckinTest.php | 30 +++++++++++++++++++ tests/Feature/Checkins/AssetCheckinTest.php | 14 +++++++-- 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/tests/Feature/Api/Assets/AssetCheckinTest.php b/tests/Feature/Api/Assets/AssetCheckinTest.php index f71191d80c..7e5098bcef 100644 --- a/tests/Feature/Api/Assets/AssetCheckinTest.php +++ b/tests/Feature/Api/Assets/AssetCheckinTest.php @@ -11,6 +11,36 @@ class AssetCheckinTest extends TestCase { use InteractsWithSettings; + public function testCheckingInAssetRequiresCorrectPermission() + { + $this->markTestIncomplete(); + } + + public function testAssetCheckedOutToAssetCanBeCheckedIn() + { + $this->markTestIncomplete(); + } + + public function testAssetCheckedOutToLocationCanBeCheckedIn() + { + $this->markTestIncomplete(); + } + + public function testAssetCheckedOutToUserCanBeCheckedIn() + { + $this->markTestIncomplete(); + } + + public function testCheckInEmailSentToUserIfSettingEnabled() + { + $this->markTestIncomplete(); + } + + public function testCheckInEmailNotSentToUserIfSettingDisabled() + { + $this->markTestIncomplete(); + } + public function testLastCheckInFieldIsSetOnCheckin() { $admin = User::factory()->superuser()->create(); diff --git a/tests/Feature/Checkins/AssetCheckinTest.php b/tests/Feature/Checkins/AssetCheckinTest.php index d43cdfb24f..92a8c29615 100644 --- a/tests/Feature/Checkins/AssetCheckinTest.php +++ b/tests/Feature/Checkins/AssetCheckinTest.php @@ -24,13 +24,23 @@ class AssetCheckinTest extends TestCase ->assertForbidden(); } - public function testAssetCanBeCheckedIn() + public function testAssetCheckedOutToAssetCanBeCheckedIn() + { + $this->markTestIncomplete(); + } + + public function testAssetCheckedOutToLocationCanBeCheckedIn() + { + $this->markTestIncomplete(); + } + + public function testAssetCheckedOutToUserCanBeCheckedIn() { Event::fake([CheckoutableCheckedIn::class]); $admin = User::factory()->checkinAssets()->create(); $user = User::factory()->create(); - $asset = Asset::factory()->assignedToUser($user)->create(['last_checkin' => null]); + $asset = Asset::factory()->assignedToUser($user)->create(); $this->assertTrue($asset->assignedTo->is($user)); From 0506f3bef978437278697e1b69770e1731691bb9 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Tue, 13 Feb 2024 12:35:31 -0800 Subject: [PATCH 04/31] Scaffold additional tests --- tests/Feature/Api/Assets/AssetCheckinTest.php | 35 ++++++++---- tests/Feature/Checkins/AssetCheckinTest.php | 55 ++++++++++++------- 2 files changed, 60 insertions(+), 30 deletions(-) diff --git a/tests/Feature/Api/Assets/AssetCheckinTest.php b/tests/Feature/Api/Assets/AssetCheckinTest.php index 7e5098bcef..85104052c1 100644 --- a/tests/Feature/Api/Assets/AssetCheckinTest.php +++ b/tests/Feature/Api/Assets/AssetCheckinTest.php @@ -16,6 +16,16 @@ class AssetCheckinTest extends TestCase $this->markTestIncomplete(); } + public function testCannotCheckInNonExistentAsset() + { + $this->markTestIncomplete(); + } + + public function testCannotCheckInAssetThatIsNotCheckedOut() + { + $this->markTestIncomplete(); + } + public function testAssetCheckedOutToAssetCanBeCheckedIn() { $this->markTestIncomplete(); @@ -31,16 +41,6 @@ class AssetCheckinTest extends TestCase $this->markTestIncomplete(); } - public function testCheckInEmailSentToUserIfSettingEnabled() - { - $this->markTestIncomplete(); - } - - public function testCheckInEmailNotSentToUserIfSettingDisabled() - { - $this->markTestIncomplete(); - } - public function testLastCheckInFieldIsSetOnCheckin() { $admin = User::factory()->superuser()->create(); @@ -57,4 +57,19 @@ class AssetCheckinTest extends TestCase 'last_checkin field should be set on checkin' ); } + + public function testPendingCheckoutAcceptancesAreClearedUponCheckin() + { + $this->markTestIncomplete(); + } + + public function testCheckInEmailSentToUserIfSettingEnabled() + { + $this->markTestIncomplete(); + } + + public function testCheckInEmailNotSentToUserIfSettingDisabled() + { + $this->markTestIncomplete(); + } } diff --git a/tests/Feature/Checkins/AssetCheckinTest.php b/tests/Feature/Checkins/AssetCheckinTest.php index 92a8c29615..95cf639f82 100644 --- a/tests/Feature/Checkins/AssetCheckinTest.php +++ b/tests/Feature/Checkins/AssetCheckinTest.php @@ -24,6 +24,16 @@ class AssetCheckinTest extends TestCase ->assertForbidden(); } + public function testCannotCheckInNonExistentAsset() + { + $this->markTestIncomplete(); + } + + public function testCannotCheckInAssetThatIsNotCheckedOut() + { + $this->markTestIncomplete(); + } + public function testAssetCheckedOutToAssetCanBeCheckedIn() { $this->markTestIncomplete(); @@ -45,13 +55,36 @@ class AssetCheckinTest extends TestCase $this->assertTrue($asset->assignedTo->is($user)); $this->actingAs($admin) - ->post(route('hardware.checkin.store', ['assetId' => $asset->id])) - ->assertRedirect(); + ->post(route('hardware.checkin.store', ['assetId' => $asset->id, 'backto' => 'user'])) + ->assertRedirect(route('users.show', $user)); $this->assertNull($asset->fresh()->assignedTo); Event::assertDispatched(CheckoutableCheckedIn::class, 1); } + public function testLastCheckInFieldIsSetOnCheckin() + { + $admin = User::factory()->superuser()->create(); + $asset = Asset::factory()->create(['last_checkin' => null]); + + $asset->checkOut(User::factory()->create(), $admin, now()); + + $this->actingAs($admin) + ->post(route('hardware.checkin.store', [ + 'assetId' => $asset->id, + ])); + + $this->assertNotNull( + $asset->fresh()->last_checkin, + 'last_checkin field should be set on checkin' + ); + } + + public function testPendingCheckoutAcceptancesAreClearedUponCheckin() + { + $this->markTestIncomplete(); + } + public function testCheckInEmailSentToUserIfSettingEnabled() { Notification::fake(); @@ -99,22 +132,4 @@ class AssetCheckinTest extends TestCase } ); } - - public function testLastCheckInFieldIsSetOnCheckin() - { - $admin = User::factory()->superuser()->create(); - $asset = Asset::factory()->create(['last_checkin' => null]); - - $asset->checkOut(User::factory()->create(), $admin, now()); - - $this->actingAs($admin) - ->post(route('hardware.checkin.store', [ - 'assetId' => $asset->id, - ])); - - $this->assertNotNull( - $asset->fresh()->last_checkin, - 'last_checkin field should be set on checkin' - ); - } } From 31a75bd252ccd1747b77a538e5e995c68391490f Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Tue, 13 Feb 2024 13:17:02 -0800 Subject: [PATCH 05/31] Add some assertions --- database/factories/AssetFactory.php | 1 + tests/Feature/Checkins/AssetCheckinTest.php | 56 ++++++++++++++------- 2 files changed, 39 insertions(+), 18 deletions(-) diff --git a/database/factories/AssetFactory.php b/database/factories/AssetFactory.php index 3fa0ae1d40..e6108ce943 100644 --- a/database/factories/AssetFactory.php +++ b/database/factories/AssetFactory.php @@ -295,6 +295,7 @@ class AssetFactory extends Factory return [ 'assigned_to' => $user->id ?? User::factory(), 'assigned_type' => User::class, + 'last_checkout' => now()->subDay(), ]; }); } diff --git a/tests/Feature/Checkins/AssetCheckinTest.php b/tests/Feature/Checkins/AssetCheckinTest.php index 95cf639f82..6b0fdd0e7a 100644 --- a/tests/Feature/Checkins/AssetCheckinTest.php +++ b/tests/Feature/Checkins/AssetCheckinTest.php @@ -4,6 +4,7 @@ namespace Tests\Feature\Checkins; use App\Events\CheckoutableCheckedIn; use App\Models\Asset; +use App\Models\Statuslabel; use App\Models\User; use App\Notifications\CheckinAssetNotification; use Illuminate\Support\Facades\Event; @@ -34,6 +35,43 @@ class AssetCheckinTest extends TestCase $this->markTestIncomplete(); } + public function testAssetCheckedOutToUserCanBeCheckedIn() + { + Event::fake([CheckoutableCheckedIn::class]); + + $admin = User::factory()->checkinAssets()->create(); + $user = User::factory()->create(); + $status = Statuslabel::first() ?? Statuslabel::factory()->create(); + $asset = Asset::factory()->assignedToUser($user)->create([ + 'expected_checkin' => now()->addDay(), + 'last_checkin' => null, + 'accepted' => 'accepted', + ]); + + $this->assertTrue($asset->assignedTo->is($user)); + + $this->actingAs($admin) + ->post( + route('hardware.checkin.store', ['assetId' => $asset->id, 'backto' => 'user']), + [ + 'name' => 'Changed Name', + 'status_id' => $status->id, + ], + ) + ->assertRedirect(route('users.show', $user)); + + Event::assertDispatched(CheckoutableCheckedIn::class, 1); + $this->assertNull($asset->refresh()->assignedTo); + $this->assertNull($asset->expected_checkin); + $this->assertNull($asset->last_checkout); + $this->assertNotNull($asset->last_checkin); + $this->assertNull($asset->assignedTo); + $this->assertNull($asset->assigned_type); + $this->assertNull($asset->accepted); + $this->assertEquals('Changed Name', $asset->name); + $this->assertEquals($status->id, $asset->status_id); + } + public function testAssetCheckedOutToAssetCanBeCheckedIn() { $this->markTestIncomplete(); @@ -44,24 +82,6 @@ class AssetCheckinTest extends TestCase $this->markTestIncomplete(); } - public function testAssetCheckedOutToUserCanBeCheckedIn() - { - Event::fake([CheckoutableCheckedIn::class]); - - $admin = User::factory()->checkinAssets()->create(); - $user = User::factory()->create(); - $asset = Asset::factory()->assignedToUser($user)->create(); - - $this->assertTrue($asset->assignedTo->is($user)); - - $this->actingAs($admin) - ->post(route('hardware.checkin.store', ['assetId' => $asset->id, 'backto' => 'user'])) - ->assertRedirect(route('users.show', $user)); - - $this->assertNull($asset->fresh()->assignedTo); - Event::assertDispatched(CheckoutableCheckedIn::class, 1); - } - public function testLastCheckInFieldIsSetOnCheckin() { $admin = User::factory()->superuser()->create(); From b653d19579416d9a003827a0714b15727fe5167c Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Tue, 13 Feb 2024 13:29:54 -0800 Subject: [PATCH 06/31] Implement test --- tests/Feature/Checkins/AssetCheckinTest.php | 22 ++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/tests/Feature/Checkins/AssetCheckinTest.php b/tests/Feature/Checkins/AssetCheckinTest.php index 6b0fdd0e7a..f3518d11ba 100644 --- a/tests/Feature/Checkins/AssetCheckinTest.php +++ b/tests/Feature/Checkins/AssetCheckinTest.php @@ -35,7 +35,7 @@ class AssetCheckinTest extends TestCase $this->markTestIncomplete(); } - public function testAssetCheckedOutToUserCanBeCheckedIn() + public function testAssetCanBeCheckedIn() { Event::fake([CheckoutableCheckedIn::class]); @@ -72,14 +72,22 @@ class AssetCheckinTest extends TestCase $this->assertEquals($status->id, $asset->status_id); } - public function testAssetCheckedOutToAssetCanBeCheckedIn() + public function testCheckinTimeAndActionLogNoteCanBeSet() { - $this->markTestIncomplete(); - } + Event::fake(); - public function testAssetCheckedOutToLocationCanBeCheckedIn() - { - $this->markTestIncomplete(); + $this->actingAs(User::factory()->checkinAssets()->create()) + ->post(route( + 'hardware.checkin.store', + ['assetId' => Asset::factory()->assignedToUser()->create()->id] + ), [ + 'checkin_at' => '2023-01-02 12:45:56', + 'note' => 'hello' + ]); + + Event::assertDispatched(function (CheckoutableCheckedIn $event) { + return $event->action_date === '2023-01-02 12:45:56' && $event->note === 'hello'; + }, 1); } public function testLastCheckInFieldIsSetOnCheckin() From 391b832613fda4077a56c25ea752dad65d2d7a7d Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Tue, 13 Feb 2024 13:34:55 -0800 Subject: [PATCH 07/31] Implement test --- tests/Feature/Checkins/AssetCheckinTest.php | 46 ++++++++++----------- 1 file changed, 22 insertions(+), 24 deletions(-) diff --git a/tests/Feature/Checkins/AssetCheckinTest.php b/tests/Feature/Checkins/AssetCheckinTest.php index f3518d11ba..76e1c7bffb 100644 --- a/tests/Feature/Checkins/AssetCheckinTest.php +++ b/tests/Feature/Checkins/AssetCheckinTest.php @@ -25,14 +25,12 @@ class AssetCheckinTest extends TestCase ->assertForbidden(); } - public function testCannotCheckInNonExistentAsset() - { - $this->markTestIncomplete(); - } - public function testCannotCheckInAssetThatIsNotCheckedOut() { - $this->markTestIncomplete(); + $this->actingAs(User::factory()->checkinAssets()->create()) + ->post(route('hardware.checkin.store', ['assetId' => Asset::factory()->create()->id])) + ->assertSessionHas('error') + ->assertRedirect(route('hardware.index')); } public function testAssetCanBeCheckedIn() @@ -72,24 +70,6 @@ class AssetCheckinTest extends TestCase $this->assertEquals($status->id, $asset->status_id); } - public function testCheckinTimeAndActionLogNoteCanBeSet() - { - Event::fake(); - - $this->actingAs(User::factory()->checkinAssets()->create()) - ->post(route( - 'hardware.checkin.store', - ['assetId' => Asset::factory()->assignedToUser()->create()->id] - ), [ - 'checkin_at' => '2023-01-02 12:45:56', - 'note' => 'hello' - ]); - - Event::assertDispatched(function (CheckoutableCheckedIn $event) { - return $event->action_date === '2023-01-02 12:45:56' && $event->note === 'hello'; - }, 1); - } - public function testLastCheckInFieldIsSetOnCheckin() { $admin = User::factory()->superuser()->create(); @@ -113,6 +93,24 @@ class AssetCheckinTest extends TestCase $this->markTestIncomplete(); } + public function testCheckinTimeAndActionLogNoteCanBeSet() + { + Event::fake(); + + $this->actingAs(User::factory()->checkinAssets()->create()) + ->post(route( + 'hardware.checkin.store', + ['assetId' => Asset::factory()->assignedToUser()->create()->id] + ), [ + 'checkin_at' => '2023-01-02 12:45:56', + 'note' => 'hello' + ]); + + Event::assertDispatched(function (CheckoutableCheckedIn $event) { + return $event->action_date === '2023-01-02 12:45:56' && $event->note === 'hello'; + }, 1); + } + public function testCheckInEmailSentToUserIfSettingEnabled() { Notification::fake(); From f708b8b29937adce722c841c8faeed4a465bffe4 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Tue, 13 Feb 2024 14:30:26 -0800 Subject: [PATCH 08/31] Implement test --- tests/Feature/Checkins/AssetCheckinTest.php | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/tests/Feature/Checkins/AssetCheckinTest.php b/tests/Feature/Checkins/AssetCheckinTest.php index 76e1c7bffb..4af5512087 100644 --- a/tests/Feature/Checkins/AssetCheckinTest.php +++ b/tests/Feature/Checkins/AssetCheckinTest.php @@ -4,6 +4,7 @@ namespace Tests\Feature\Checkins; use App\Events\CheckoutableCheckedIn; use App\Models\Asset; +use App\Models\CheckoutAcceptance; use App\Models\Statuslabel; use App\Models\User; use App\Notifications\CheckinAssetNotification; @@ -73,9 +74,7 @@ class AssetCheckinTest extends TestCase public function testLastCheckInFieldIsSetOnCheckin() { $admin = User::factory()->superuser()->create(); - $asset = Asset::factory()->create(['last_checkin' => null]); - - $asset->checkOut(User::factory()->create(), $admin, now()); + $asset = Asset::factory()->assignedToUser()->create(['last_checkin' => null]); $this->actingAs($admin) ->post(route('hardware.checkin.store', [ @@ -90,7 +89,14 @@ class AssetCheckinTest extends TestCase public function testPendingCheckoutAcceptancesAreClearedUponCheckin() { - $this->markTestIncomplete(); + $asset = Asset::factory()->assignedToUser()->create(); + + $acceptance = CheckoutAcceptance::factory()->for($asset, 'checkoutable')->pending()->create(); + + $this->actingAs(User::factory()->checkinAssets()->create()) + ->post(route('hardware.checkin.store', ['assetId' => $asset->id])); + + $this->assertFalse($acceptance->exists(), 'Acceptance was not deleted'); } public function testCheckinTimeAndActionLogNoteCanBeSet() From 9ab56fe9caee4522a758bcdb43d8c172f7f8deb5 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Tue, 13 Feb 2024 17:04:53 -0800 Subject: [PATCH 09/31] Implement tests --- database/factories/AssetFactory.php | 16 ++++- database/factories/LicenseSeatFactory.php | 10 +++ tests/Feature/Checkins/AssetCheckinTest.php | 72 ++++++++++++++++++++- 3 files changed, 96 insertions(+), 2 deletions(-) diff --git a/database/factories/AssetFactory.php b/database/factories/AssetFactory.php index e6108ce943..5461303c88 100644 --- a/database/factories/AssetFactory.php +++ b/database/factories/AssetFactory.php @@ -8,7 +8,6 @@ use App\Models\Location; use App\Models\Statuslabel; use App\Models\Supplier; use App\Models\User; -use Carbon\Carbon; use Carbon\CarbonImmutable; use Illuminate\Database\Eloquent\Factories\Factory; @@ -353,4 +352,19 @@ class AssetFactory extends Factory { return $this->state(['requestable' => false]); } + + /** + * This allows bypassing model level validation if you want to purposefully + * create an asset in an invalid state. Validation is turned back on + * after the model is created via the factory. + * @return AssetFactory + */ + public function canBeInvalidUponCreation() + { + return $this->afterMaking(function (Asset $asset) { + $asset->setValidating(false); + })->afterCreating(function (Asset $asset) { + $asset->setValidating(true); + }); + } } diff --git a/database/factories/LicenseSeatFactory.php b/database/factories/LicenseSeatFactory.php index 3c6cc4246b..cd9acfee1b 100644 --- a/database/factories/LicenseSeatFactory.php +++ b/database/factories/LicenseSeatFactory.php @@ -3,6 +3,7 @@ namespace Database\Factories; use App\Models\License; +use App\Models\User; use Illuminate\Database\Eloquent\Factories\Factory; class LicenseSeatFactory extends Factory @@ -13,4 +14,13 @@ class LicenseSeatFactory extends Factory 'license_id' => License::factory(), ]; } + + public function assignedToUser(User $user = null) + { + return $this->state(function () use ($user) { + return [ + 'assigned_to' => $user->id ?? User::factory(), + ]; + }); + } } diff --git a/tests/Feature/Checkins/AssetCheckinTest.php b/tests/Feature/Checkins/AssetCheckinTest.php index 4af5512087..433e824ddc 100644 --- a/tests/Feature/Checkins/AssetCheckinTest.php +++ b/tests/Feature/Checkins/AssetCheckinTest.php @@ -5,6 +5,8 @@ namespace Tests\Feature\Checkins; use App\Events\CheckoutableCheckedIn; use App\Models\Asset; use App\Models\CheckoutAcceptance; +use App\Models\LicenseSeat; +use App\Models\Location; use App\Models\Statuslabel; use App\Models\User; use App\Notifications\CheckinAssetNotification; @@ -71,6 +73,47 @@ class AssetCheckinTest extends TestCase $this->assertEquals($status->id, $asset->status_id); } + public function testLocationIsSetToRTDLocationByDefaultUponCheckin() + { + $rtdLocation = Location::factory()->create(); + $asset = Asset::factory()->assignedToUser()->create([ + 'location_id' => Location::factory()->create()->id, + 'rtd_location_id' => $rtdLocation->id, + ]); + + $this->actingAs(User::factory()->checkinAssets()->create()) + ->post(route('hardware.checkin.store', ['assetId' => $asset->id])); + + $this->assertTrue($asset->refresh()->location()->is($rtdLocation)); + } + + public function testLocationCanBeSetUponCheckin() + { + $location = Location::factory()->create(); + $asset = Asset::factory()->assignedToUser()->create(); + + $this->actingAs(User::factory()->checkinAssets()->create()) + ->post(route('hardware.checkin.store', ['assetId' => $asset->id]), [ + 'location_id' => $location->id, + ]); + + $this->assertTrue($asset->refresh()->location()->is($location)); + } + + public function testDefaultLocationCanBeUpdatedUponCheckin() + { + $location = Location::factory()->create(); + $asset = Asset::factory()->assignedToUser()->create(); + + $this->actingAs(User::factory()->checkinAssets()->create()) + ->post(route('hardware.checkin.store', ['assetId' => $asset->id]), [ + 'location_id' => $location->id, + 'update_default_location' => 0 + ]); + + $this->assertTrue($asset->refresh()->defaultLoc()->is($location)); + } + public function testLastCheckInFieldIsSetOnCheckin() { $admin = User::factory()->superuser()->create(); @@ -82,11 +125,38 @@ class AssetCheckinTest extends TestCase ])); $this->assertNotNull( - $asset->fresh()->last_checkin, + $asset->refresh()->last_checkin, 'last_checkin field should be set on checkin' ); } + public function testAssetsLicenseSeatsAreClearedUponCheckin() + { + $asset = Asset::factory()->assignedToUser()->create(); + LicenseSeat::factory()->assignedToUser()->for($asset)->create(); + + $this->assertNotNull($asset->licenseseats->first()->assigned_to); + + $this->actingAs(User::factory()->checkinAssets()->create()) + ->post(route('hardware.checkin.store', ['assetId' => $asset->id])); + + $this->assertNull($asset->refresh()->licenseseats->first()->assigned_to); + } + + public function testLegacyLocationValuesSetToZeroAreUpdated() + { + $asset = Asset::factory()->canBeInvalidUponCreation()->assignedToUser()->create([ + 'rtd_location_id' => 0, + 'location_id' => 0, + ]); + + $this->actingAs(User::factory()->checkinAssets()->create()) + ->post(route('hardware.checkin.store', ['assetId' => $asset->id])); + + $this->assertNull($asset->refresh()->rtd_location_id); + $this->assertNull($asset->location_id); + } + public function testPendingCheckoutAcceptancesAreClearedUponCheckin() { $asset = Asset::factory()->assignedToUser()->create(); From ad1846fed6da53a8827c506402d39f6b2e088b14 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Tue, 13 Feb 2024 17:50:26 -0800 Subject: [PATCH 10/31] Implement tests --- tests/Feature/Api/Assets/AssetCheckinTest.php | 30 ++++++++----------- tests/Feature/Checkins/AssetCheckinTest.php | 7 ++--- 2 files changed, 14 insertions(+), 23 deletions(-) diff --git a/tests/Feature/Api/Assets/AssetCheckinTest.php b/tests/Feature/Api/Assets/AssetCheckinTest.php index 85104052c1..77b64a3f8c 100644 --- a/tests/Feature/Api/Assets/AssetCheckinTest.php +++ b/tests/Feature/Api/Assets/AssetCheckinTest.php @@ -13,40 +13,34 @@ class AssetCheckinTest extends TestCase public function testCheckingInAssetRequiresCorrectPermission() { - $this->markTestIncomplete(); + $this->actingAsForApi(User::factory()->create()) + ->postJson(route('api.asset.checkin', Asset::factory()->assignedToUser()->create())) + ->assertForbidden(); } public function testCannotCheckInNonExistentAsset() { - $this->markTestIncomplete(); + $this->actingAsForApi(User::factory()->checkinAssets()->create()) + ->postJson(route('api.asset.checkin', ['id' => 'does-not-exist'])) + ->assertStatusMessageIs('error'); } public function testCannotCheckInAssetThatIsNotCheckedOut() { - $this->markTestIncomplete(); + $this->actingAsForApi(User::factory()->checkinAssets()->create()) + ->postJson(route('api.asset.checkin', Asset::factory()->create()->id)) + ->assertStatusMessageIs('error'); } - public function testAssetCheckedOutToAssetCanBeCheckedIn() - { - $this->markTestIncomplete(); - } - - public function testAssetCheckedOutToLocationCanBeCheckedIn() - { - $this->markTestIncomplete(); - } - - public function testAssetCheckedOutToUserCanBeCheckedIn() + public function testAssetCanBeCheckedIn() { $this->markTestIncomplete(); } public function testLastCheckInFieldIsSetOnCheckin() { - $admin = User::factory()->superuser()->create(); - $asset = Asset::factory()->create(['last_checkin' => null]); - - $asset->checkOut(User::factory()->create(), $admin, now()); + $admin = User::factory()->checkinAssets()->create(); + $asset = Asset::factory()->assignedToUser()->create(['last_checkin' => null]); $this->actingAsForApi($admin) ->postJson(route('api.asset.checkin', $asset)) diff --git a/tests/Feature/Checkins/AssetCheckinTest.php b/tests/Feature/Checkins/AssetCheckinTest.php index 433e824ddc..f951ede70a 100644 --- a/tests/Feature/Checkins/AssetCheckinTest.php +++ b/tests/Feature/Checkins/AssetCheckinTest.php @@ -116,13 +116,10 @@ class AssetCheckinTest extends TestCase public function testLastCheckInFieldIsSetOnCheckin() { - $admin = User::factory()->superuser()->create(); + $admin = User::factory()->checkinAssets()->create(); $asset = Asset::factory()->assignedToUser()->create(['last_checkin' => null]); - $this->actingAs($admin) - ->post(route('hardware.checkin.store', [ - 'assetId' => $asset->id, - ])); + $this->actingAs($admin)->post(route('hardware.checkin.store', ['assetId' => $asset->id])); $this->assertNotNull( $asset->refresh()->last_checkin, From 7bfd02054b5926461a016c43332db662d8de6190 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 14 Feb 2024 10:48:41 -0800 Subject: [PATCH 11/31] Remove duplicate authorization check --- app/Http/Controllers/Api/AssetsController.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/Http/Controllers/Api/AssetsController.php b/app/Http/Controllers/Api/AssetsController.php index f5168a5914..71bdff8095 100644 --- a/app/Http/Controllers/Api/AssetsController.php +++ b/app/Http/Controllers/Api/AssetsController.php @@ -864,11 +864,9 @@ class AssetsController extends Controller */ public function checkin(Request $request, $asset_id) { - $this->authorize('checkin', Asset::class); $asset = Asset::with('model')->findOrFail($asset_id); $this->authorize('checkin', $asset); - $target = $asset->assignedTo; if (is_null($target)) { return response()->json(Helper::formatStandardApiResponse('error', [ From af513946a2c59f87f725a0aeda41bc3c9f77011a Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 14 Feb 2024 10:48:49 -0800 Subject: [PATCH 12/31] Implement test --- tests/Feature/Api/Assets/AssetCheckinTest.php | 33 ++++++++++++++++++- tests/Feature/Checkins/AssetCheckinTest.php | 3 +- 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/tests/Feature/Api/Assets/AssetCheckinTest.php b/tests/Feature/Api/Assets/AssetCheckinTest.php index 77b64a3f8c..b068d49491 100644 --- a/tests/Feature/Api/Assets/AssetCheckinTest.php +++ b/tests/Feature/Api/Assets/AssetCheckinTest.php @@ -2,8 +2,11 @@ namespace Tests\Feature\Api\Assets; +use App\Events\CheckoutableCheckedIn; use App\Models\Asset; +use App\Models\Statuslabel; use App\Models\User; +use Illuminate\Support\Facades\Event; use Tests\Support\InteractsWithSettings; use Tests\TestCase; @@ -34,7 +37,35 @@ class AssetCheckinTest extends TestCase public function testAssetCanBeCheckedIn() { - $this->markTestIncomplete(); + Event::fake([CheckoutableCheckedIn::class]); + + $user = User::factory()->create(); + $status = Statuslabel::factory()->create(); + $asset = Asset::factory()->assignedToUser($user)->create([ + 'expected_checkin' => now()->addDay(), + 'last_checkin' => null, + 'accepted' => 'accepted', + ]); + + $this->assertTrue($asset->assignedTo->is($user)); + + $this->actingAsForApi(User::factory()->checkinAssets()->create()) + ->postJson(route('api.asset.checkin', $asset->id), [ + 'name' => 'Changed Name', + 'status_id' => $status->id, + ]) + ->assertOk(); + + Event::assertDispatched(CheckoutableCheckedIn::class, 1); + $this->assertNull($asset->refresh()->assignedTo); + $this->assertNull($asset->expected_checkin); + $this->assertNull($asset->last_checkout); + $this->assertNotNull($asset->last_checkin); + $this->assertNull($asset->assignedTo); + $this->assertNull($asset->assigned_type); + $this->assertNull($asset->accepted); + $this->assertEquals('Changed Name', $asset->name); + $this->assertEquals($status->id, $asset->status_id); } public function testLastCheckInFieldIsSetOnCheckin() diff --git a/tests/Feature/Checkins/AssetCheckinTest.php b/tests/Feature/Checkins/AssetCheckinTest.php index f951ede70a..ce4cb6f22e 100644 --- a/tests/Feature/Checkins/AssetCheckinTest.php +++ b/tests/Feature/Checkins/AssetCheckinTest.php @@ -40,7 +40,6 @@ class AssetCheckinTest extends TestCase { Event::fake([CheckoutableCheckedIn::class]); - $admin = User::factory()->checkinAssets()->create(); $user = User::factory()->create(); $status = Statuslabel::first() ?? Statuslabel::factory()->create(); $asset = Asset::factory()->assignedToUser($user)->create([ @@ -51,7 +50,7 @@ class AssetCheckinTest extends TestCase $this->assertTrue($asset->assignedTo->is($user)); - $this->actingAs($admin) + $this->actingAs(User::factory()->checkinAssets()->create()) ->post( route('hardware.checkin.store', ['assetId' => $asset->id, 'backto' => 'user']), [ From d7aed2edc998afbbb3cf13c20541b8ad979b07d5 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 14 Feb 2024 11:10:45 -0800 Subject: [PATCH 13/31] Remove unneeded code --- app/Http/Controllers/Api/AssetsController.php | 1 - app/Http/Controllers/Assets/AssetCheckinController.php | 2 -- 2 files changed, 3 deletions(-) diff --git a/app/Http/Controllers/Api/AssetsController.php b/app/Http/Controllers/Api/AssetsController.php index 71bdff8095..ecbbe7a78c 100644 --- a/app/Http/Controllers/Api/AssetsController.php +++ b/app/Http/Controllers/Api/AssetsController.php @@ -879,7 +879,6 @@ class AssetsController extends Controller $asset->expected_checkin = null; $asset->last_checkout = null; $asset->last_checkin = now(); - $asset->assigned_to = null; $asset->assignedTo()->disassociate($asset); $asset->accepted = null; diff --git a/app/Http/Controllers/Assets/AssetCheckinController.php b/app/Http/Controllers/Assets/AssetCheckinController.php index 4fe10e895f..891480f570 100644 --- a/app/Http/Controllers/Assets/AssetCheckinController.php +++ b/app/Http/Controllers/Assets/AssetCheckinController.php @@ -69,9 +69,7 @@ class AssetCheckinController extends Controller $asset->expected_checkin = null; $asset->last_checkout = null; $asset->last_checkin = now(); - $asset->assigned_to = null; $asset->assignedTo()->disassociate($asset); - $asset->assigned_type = null; $asset->accepted = null; $asset->name = $request->get('name'); From 02f39472f9f8d8b566e2840198b55bdd7be9c441 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 14 Feb 2024 11:10:59 -0800 Subject: [PATCH 14/31] Remove duplicate test --- tests/Feature/Checkins/AssetCheckinTest.php | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/tests/Feature/Checkins/AssetCheckinTest.php b/tests/Feature/Checkins/AssetCheckinTest.php index ce4cb6f22e..7a4074370c 100644 --- a/tests/Feature/Checkins/AssetCheckinTest.php +++ b/tests/Feature/Checkins/AssetCheckinTest.php @@ -113,19 +113,6 @@ class AssetCheckinTest extends TestCase $this->assertTrue($asset->refresh()->defaultLoc()->is($location)); } - public function testLastCheckInFieldIsSetOnCheckin() - { - $admin = User::factory()->checkinAssets()->create(); - $asset = Asset::factory()->assignedToUser()->create(['last_checkin' => null]); - - $this->actingAs($admin)->post(route('hardware.checkin.store', ['assetId' => $asset->id])); - - $this->assertNotNull( - $asset->refresh()->last_checkin, - 'last_checkin field should be set on checkin' - ); - } - public function testAssetsLicenseSeatsAreClearedUponCheckin() { $asset = Asset::factory()->assignedToUser()->create(); From 4354e126b1d002b69242b34eb072af22b9642487 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 14 Feb 2024 11:11:08 -0800 Subject: [PATCH 15/31] Scaffold tests --- tests/Feature/Api/Assets/AssetCheckinTest.php | 55 +++++++++++++++---- 1 file changed, 44 insertions(+), 11 deletions(-) diff --git a/tests/Feature/Api/Assets/AssetCheckinTest.php b/tests/Feature/Api/Assets/AssetCheckinTest.php index b068d49491..8f806ada93 100644 --- a/tests/Feature/Api/Assets/AssetCheckinTest.php +++ b/tests/Feature/Api/Assets/AssetCheckinTest.php @@ -4,6 +4,7 @@ namespace Tests\Feature\Api\Assets; use App\Events\CheckoutableCheckedIn; use App\Models\Asset; +use App\Models\Location; use App\Models\Statuslabel; use App\Models\User; use Illuminate\Support\Facades\Event; @@ -50,7 +51,7 @@ class AssetCheckinTest extends TestCase $this->assertTrue($asset->assignedTo->is($user)); $this->actingAsForApi(User::factory()->checkinAssets()->create()) - ->postJson(route('api.asset.checkin', $asset->id), [ + ->postJson(route('api.asset.checkin', $asset), [ 'name' => 'Changed Name', 'status_id' => $status->id, ]) @@ -68,22 +69,54 @@ class AssetCheckinTest extends TestCase $this->assertEquals($status->id, $asset->status_id); } - public function testLastCheckInFieldIsSetOnCheckin() + public function testLocationIsSetToRTDLocationByDefaultUponCheckin() { - $admin = User::factory()->checkinAssets()->create(); - $asset = Asset::factory()->assignedToUser()->create(['last_checkin' => null]); + $rtdLocation = Location::factory()->create(); + $asset = Asset::factory()->assignedToUser()->create([ + 'location_id' => Location::factory()->create()->id, + 'rtd_location_id' => $rtdLocation->id, + ]); - $this->actingAsForApi($admin) - ->postJson(route('api.asset.checkin', $asset)) - ->assertOk(); + $this->actingAsForApi(User::factory()->checkinAssets()->create()) + ->postJson(route('api.asset.checkin', $asset->id)); - $this->assertNotNull( - $asset->fresh()->last_checkin, - 'last_checkin field should be set on checkin' - ); + $this->assertTrue($asset->refresh()->location()->is($rtdLocation)); + } + + public function testLocationCanBeSetUponCheckin() + { + $location = Location::factory()->create(); + $asset = Asset::factory()->assignedToUser()->create(); + + $this->actingAsForApi(User::factory()->checkinAssets()->create()) + ->postJson(route('api.asset.checkin', $asset->id), [ + 'location_id' => $location->id, + ]); + + $this->assertTrue($asset->refresh()->location()->is($location)); + } + + public function testDefaultLocationCanBeUpdatedUponCheckin() + { + $this->markTestIncomplete(); + } + + public function testAssetsLicenseSeatsAreClearedUponCheckin() + { + $this->markTestIncomplete(); + } + + public function testLegacyLocationValuesSetToZeroAreUpdated() + { + $this->markTestIncomplete(); } public function testPendingCheckoutAcceptancesAreClearedUponCheckin() + { + $this->markTestIncomplete('Not currently in controller'); + } + + public function testCheckinTimeAndActionLogNoteCanBeSet() { $this->markTestIncomplete(); } From 3cc72021b681e7c1acf3ce07da74134fe79eaf86 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 14 Feb 2024 11:16:31 -0800 Subject: [PATCH 16/31] Move notification test to notifications test suite --- tests/Feature/Api/Assets/AssetCheckinTest.php | 10 ---- tests/Feature/Checkins/AssetCheckinTest.php | 50 ------------------- .../Notifications/AssetWebhookTest.php | 48 ++++++++++++++++++ 3 files changed, 48 insertions(+), 60 deletions(-) diff --git a/tests/Feature/Api/Assets/AssetCheckinTest.php b/tests/Feature/Api/Assets/AssetCheckinTest.php index 8f806ada93..32aed3616e 100644 --- a/tests/Feature/Api/Assets/AssetCheckinTest.php +++ b/tests/Feature/Api/Assets/AssetCheckinTest.php @@ -120,14 +120,4 @@ class AssetCheckinTest extends TestCase { $this->markTestIncomplete(); } - - public function testCheckInEmailSentToUserIfSettingEnabled() - { - $this->markTestIncomplete(); - } - - public function testCheckInEmailNotSentToUserIfSettingDisabled() - { - $this->markTestIncomplete(); - } } diff --git a/tests/Feature/Checkins/AssetCheckinTest.php b/tests/Feature/Checkins/AssetCheckinTest.php index 7a4074370c..5bd19175bf 100644 --- a/tests/Feature/Checkins/AssetCheckinTest.php +++ b/tests/Feature/Checkins/AssetCheckinTest.php @@ -9,9 +9,7 @@ use App\Models\LicenseSeat; use App\Models\Location; use App\Models\Statuslabel; use App\Models\User; -use App\Notifications\CheckinAssetNotification; use Illuminate\Support\Facades\Event; -use Illuminate\Support\Facades\Notification; use Tests\Support\InteractsWithSettings; use Tests\TestCase; @@ -169,52 +167,4 @@ class AssetCheckinTest extends TestCase return $event->action_date === '2023-01-02 12:45:56' && $event->note === 'hello'; }, 1); } - - public function testCheckInEmailSentToUserIfSettingEnabled() - { - Notification::fake(); - - $user = User::factory()->create(); - $asset = Asset::factory()->assignedToUser($user)->create(); - - $asset->model->category->update(['checkin_email' => true]); - - event(new CheckoutableCheckedIn( - $asset, - $user, - User::factory()->checkinAssets()->create(), - '' - )); - - Notification::assertSentTo( - [$user], - function (CheckinAssetNotification $notification, $channels) { - return in_array('mail', $channels); - }, - ); - } - - public function testCheckInEmailNotSentToUserIfSettingDisabled() - { - Notification::fake(); - - $user = User::factory()->create(); - $asset = Asset::factory()->assignedToUser($user)->create(); - - $asset->model->category->update(['checkin_email' => false]); - - event(new CheckoutableCheckedIn( - $asset, - $user, - User::factory()->checkinAssets()->create(), - '' - )); - - Notification::assertNotSentTo( - [$user], - function (CheckinAssetNotification $notification, $channels) { - return in_array('mail', $channels); - } - ); - } } diff --git a/tests/Feature/Notifications/AssetWebhookTest.php b/tests/Feature/Notifications/AssetWebhookTest.php index ae45a4caa4..f7df7bf356 100644 --- a/tests/Feature/Notifications/AssetWebhookTest.php +++ b/tests/Feature/Notifications/AssetWebhookTest.php @@ -108,6 +108,54 @@ class AssetWebhookTest extends TestCase Notification::assertNotSentTo(new AnonymousNotifiable, CheckinAssetNotification::class); } + public function testCheckInEmailSentToUserIfSettingEnabled() + { + Notification::fake(); + + $user = User::factory()->create(); + $asset = Asset::factory()->assignedToUser($user)->create(); + + $asset->model->category->update(['checkin_email' => true]); + + event(new CheckoutableCheckedIn( + $asset, + $user, + User::factory()->checkinAssets()->create(), + '' + )); + + Notification::assertSentTo( + [$user], + function (CheckinAssetNotification $notification, $channels) { + return in_array('mail', $channels); + }, + ); + } + + public function testCheckInEmailNotSentToUserIfSettingDisabled() + { + Notification::fake(); + + $user = User::factory()->create(); + $asset = Asset::factory()->assignedToUser($user)->create(); + + $asset->model->category->update(['checkin_email' => false]); + + event(new CheckoutableCheckedIn( + $asset, + $user, + User::factory()->checkinAssets()->create(), + '' + )); + + Notification::assertNotSentTo( + [$user], + function (CheckinAssetNotification $notification, $channels) { + return in_array('mail', $channels); + } + ); + } + private function createAsset() { return Asset::factory()->laptopMbp()->create(); From bacfdc5049220cd8a142c974e10b16eb4ff95c9d Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 14 Feb 2024 11:33:03 -0800 Subject: [PATCH 17/31] Scaffold additional tests --- tests/Feature/Api/Assets/AssetCheckinTest.php | 44 +++++++++++++++++-- tests/Feature/Checkins/AssetCheckinTest.php | 4 +- 2 files changed, 42 insertions(+), 6 deletions(-) diff --git a/tests/Feature/Api/Assets/AssetCheckinTest.php b/tests/Feature/Api/Assets/AssetCheckinTest.php index 32aed3616e..83bbdab084 100644 --- a/tests/Feature/Api/Assets/AssetCheckinTest.php +++ b/tests/Feature/Api/Assets/AssetCheckinTest.php @@ -4,6 +4,7 @@ namespace Tests\Feature\Api\Assets; use App\Events\CheckoutableCheckedIn; use App\Models\Asset; +use App\Models\LicenseSeat; use App\Models\Location; use App\Models\Statuslabel; use App\Models\User; @@ -98,17 +99,38 @@ class AssetCheckinTest extends TestCase public function testDefaultLocationCanBeUpdatedUponCheckin() { - $this->markTestIncomplete(); + $this->markTestIncomplete('Not currently in controller'); + + $location = Location::factory()->create(); + $asset = Asset::factory()->assignedToUser()->create(); + + $this->actingAsForApi(User::factory()->checkinAssets()->create()) + ->postJson(route('api.asset.checkin', $asset), [ + 'location_id' => $location->id, + 'update_default_location' => 0 + ]); + + $this->assertTrue($asset->refresh()->defaultLoc()->is($location)); } public function testAssetsLicenseSeatsAreClearedUponCheckin() { - $this->markTestIncomplete(); + $this->markTestIncomplete('Not currently in controller'); + + $asset = Asset::factory()->assignedToUser()->create(); + LicenseSeat::factory()->assignedToUser()->for($asset)->create(); + + $this->assertNotNull($asset->licenseseats->first()->assigned_to); + + $this->actingAsForApi(User::factory()->checkinAssets()->create()) + ->postJson(route('api.asset.checkin', $asset)); + + $this->assertNull($asset->refresh()->licenseseats->first()->assigned_to); } public function testLegacyLocationValuesSetToZeroAreUpdated() { - $this->markTestIncomplete(); + $this->markTestIncomplete('Not currently in controller'); } public function testPendingCheckoutAcceptancesAreClearedUponCheckin() @@ -118,6 +140,20 @@ class AssetCheckinTest extends TestCase public function testCheckinTimeAndActionLogNoteCanBeSet() { - $this->markTestIncomplete(); + $this->markTestIncomplete( + 'checkin_at currently takes a date and applies a time which is not inline with what the web controller does.' + ); + + Event::fake(); + + $this->actingAsForApi(User::factory()->checkinAssets()->create()) + ->postJson(route('api.asset.checkin', Asset::factory()->assignedToUser()->create()), [ + 'checkin_at' => '2023-01-02 12:34:56', + 'note' => 'hi there', + ]); + + Event::assertDispatched(function (CheckoutableCheckedIn $event) { + return $event->action_date === '2023-01-02 12:34:56' && $event->note === 'hi there'; + }, 1); } } diff --git a/tests/Feature/Checkins/AssetCheckinTest.php b/tests/Feature/Checkins/AssetCheckinTest.php index 5bd19175bf..925078ddfb 100644 --- a/tests/Feature/Checkins/AssetCheckinTest.php +++ b/tests/Feature/Checkins/AssetCheckinTest.php @@ -159,12 +159,12 @@ class AssetCheckinTest extends TestCase 'hardware.checkin.store', ['assetId' => Asset::factory()->assignedToUser()->create()->id] ), [ - 'checkin_at' => '2023-01-02 12:45:56', + 'checkin_at' => '2023-01-02 12:34:56', 'note' => 'hello' ]); Event::assertDispatched(function (CheckoutableCheckedIn $event) { - return $event->action_date === '2023-01-02 12:45:56' && $event->note === 'hello'; + return $event->action_date === '2023-01-02 12:34:56' && $event->note === 'hello'; }, 1); } } From 905df5ec258846cfa5c22c5894e0b14abc7d9cbd Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 14 Feb 2024 12:14:27 -0800 Subject: [PATCH 18/31] Consolidate test cases --- tests/Feature/Api/Assets/AssetCheckinTest.php | 16 +++------------- tests/Feature/Checkins/AssetCheckinTest.php | 16 +++------------- 2 files changed, 6 insertions(+), 26 deletions(-) diff --git a/tests/Feature/Api/Assets/AssetCheckinTest.php b/tests/Feature/Api/Assets/AssetCheckinTest.php index 83bbdab084..947828f550 100644 --- a/tests/Feature/Api/Assets/AssetCheckinTest.php +++ b/tests/Feature/Api/Assets/AssetCheckinTest.php @@ -42,6 +42,7 @@ class AssetCheckinTest extends TestCase Event::fake([CheckoutableCheckedIn::class]); $user = User::factory()->create(); + $location = Location::factory()->create(); $status = Statuslabel::factory()->create(); $asset = Asset::factory()->assignedToUser($user)->create([ 'expected_checkin' => now()->addDay(), @@ -55,6 +56,7 @@ class AssetCheckinTest extends TestCase ->postJson(route('api.asset.checkin', $asset), [ 'name' => 'Changed Name', 'status_id' => $status->id, + 'location_id' => $location->id, ]) ->assertOk(); @@ -68,6 +70,7 @@ class AssetCheckinTest extends TestCase $this->assertNull($asset->accepted); $this->assertEquals('Changed Name', $asset->name); $this->assertEquals($status->id, $asset->status_id); + $this->assertTrue($asset->location()->is($location)); } public function testLocationIsSetToRTDLocationByDefaultUponCheckin() @@ -84,19 +87,6 @@ class AssetCheckinTest extends TestCase $this->assertTrue($asset->refresh()->location()->is($rtdLocation)); } - public function testLocationCanBeSetUponCheckin() - { - $location = Location::factory()->create(); - $asset = Asset::factory()->assignedToUser()->create(); - - $this->actingAsForApi(User::factory()->checkinAssets()->create()) - ->postJson(route('api.asset.checkin', $asset->id), [ - 'location_id' => $location->id, - ]); - - $this->assertTrue($asset->refresh()->location()->is($location)); - } - public function testDefaultLocationCanBeUpdatedUponCheckin() { $this->markTestIncomplete('Not currently in controller'); diff --git a/tests/Feature/Checkins/AssetCheckinTest.php b/tests/Feature/Checkins/AssetCheckinTest.php index 925078ddfb..ad4d9074b7 100644 --- a/tests/Feature/Checkins/AssetCheckinTest.php +++ b/tests/Feature/Checkins/AssetCheckinTest.php @@ -39,6 +39,7 @@ class AssetCheckinTest extends TestCase Event::fake([CheckoutableCheckedIn::class]); $user = User::factory()->create(); + $location = Location::factory()->create(); $status = Statuslabel::first() ?? Statuslabel::factory()->create(); $asset = Asset::factory()->assignedToUser($user)->create([ 'expected_checkin' => now()->addDay(), @@ -54,6 +55,7 @@ class AssetCheckinTest extends TestCase [ 'name' => 'Changed Name', 'status_id' => $status->id, + 'location_id' => $location->id, ], ) ->assertRedirect(route('users.show', $user)); @@ -68,6 +70,7 @@ class AssetCheckinTest extends TestCase $this->assertNull($asset->accepted); $this->assertEquals('Changed Name', $asset->name); $this->assertEquals($status->id, $asset->status_id); + $this->assertTrue($asset->location()->is($location)); } public function testLocationIsSetToRTDLocationByDefaultUponCheckin() @@ -84,19 +87,6 @@ class AssetCheckinTest extends TestCase $this->assertTrue($asset->refresh()->location()->is($rtdLocation)); } - public function testLocationCanBeSetUponCheckin() - { - $location = Location::factory()->create(); - $asset = Asset::factory()->assignedToUser()->create(); - - $this->actingAs(User::factory()->checkinAssets()->create()) - ->post(route('hardware.checkin.store', ['assetId' => $asset->id]), [ - 'location_id' => $location->id, - ]); - - $this->assertTrue($asset->refresh()->location()->is($location)); - } - public function testDefaultLocationCanBeUpdatedUponCheckin() { $location = Location::factory()->create(); From aec59f2da6c16501109769fcd44393de08f915dc Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 14 Feb 2024 12:27:42 -0800 Subject: [PATCH 19/31] Update assertion to be more correct --- tests/Feature/Checkins/AssetCheckinTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Feature/Checkins/AssetCheckinTest.php b/tests/Feature/Checkins/AssetCheckinTest.php index ad4d9074b7..fb60b61fce 100644 --- a/tests/Feature/Checkins/AssetCheckinTest.php +++ b/tests/Feature/Checkins/AssetCheckinTest.php @@ -125,7 +125,7 @@ class AssetCheckinTest extends TestCase ->post(route('hardware.checkin.store', ['assetId' => $asset->id])); $this->assertNull($asset->refresh()->rtd_location_id); - $this->assertNull($asset->location_id); + $this->assertEquals($asset->location_id, $asset->rtd_location_id); } public function testPendingCheckoutAcceptancesAreClearedUponCheckin() From 3ae8adfbf959ab6e1544b61a1aee83ee7df68235 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 21 Feb 2024 12:33:32 -0800 Subject: [PATCH 20/31] Remove incomplete flag on test case --- tests/Feature/Api/Assets/AssetCheckinTest.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/Feature/Api/Assets/AssetCheckinTest.php b/tests/Feature/Api/Assets/AssetCheckinTest.php index 947828f550..3eabdd076f 100644 --- a/tests/Feature/Api/Assets/AssetCheckinTest.php +++ b/tests/Feature/Api/Assets/AssetCheckinTest.php @@ -105,8 +105,6 @@ class AssetCheckinTest extends TestCase public function testAssetsLicenseSeatsAreClearedUponCheckin() { - $this->markTestIncomplete('Not currently in controller'); - $asset = Asset::factory()->assignedToUser()->create(); LicenseSeat::factory()->assignedToUser()->for($asset)->create(); From 2df026bcb546002381592c51c939b3979bb920a6 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Thu, 22 Feb 2024 12:40:14 -0800 Subject: [PATCH 21/31] Allow updating asset default location when checking in asset via api --- app/Http/Controllers/Api/AssetsController.php | 4 ++++ tests/Feature/Api/Assets/AssetCheckinTest.php | 2 -- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/app/Http/Controllers/Api/AssetsController.php b/app/Http/Controllers/Api/AssetsController.php index cabf063f3d..2967471501 100644 --- a/app/Http/Controllers/Api/AssetsController.php +++ b/app/Http/Controllers/Api/AssetsController.php @@ -890,6 +890,10 @@ class AssetsController extends Controller if ($request->filled('location_id')) { $asset->location_id = $request->input('location_id'); + + if ($request->get('update_default_location') == 0){ + $asset->rtd_location_id = $request->get('location_id'); + } } if ($request->has('status_id')) { diff --git a/tests/Feature/Api/Assets/AssetCheckinTest.php b/tests/Feature/Api/Assets/AssetCheckinTest.php index 3eabdd076f..b7dda0d4b8 100644 --- a/tests/Feature/Api/Assets/AssetCheckinTest.php +++ b/tests/Feature/Api/Assets/AssetCheckinTest.php @@ -89,8 +89,6 @@ class AssetCheckinTest extends TestCase public function testDefaultLocationCanBeUpdatedUponCheckin() { - $this->markTestIncomplete('Not currently in controller'); - $location = Location::factory()->create(); $asset = Asset::factory()->assignedToUser()->create(); From 714fc630506aff802b326326dbc47bfabc3bfa05 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Thu, 22 Feb 2024 13:14:30 -0800 Subject: [PATCH 22/31] Have legacy locations updated upon api asset checkin --- app/Http/Controllers/Api/AssetsController.php | 19 +++++++++++++++++++ tests/Feature/Api/Assets/AssetCheckinTest.php | 11 ++++++++++- 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/app/Http/Controllers/Api/AssetsController.php b/app/Http/Controllers/Api/AssetsController.php index 2967471501..3d223248ab 100644 --- a/app/Http/Controllers/Api/AssetsController.php +++ b/app/Http/Controllers/Api/AssetsController.php @@ -886,6 +886,25 @@ class AssetsController extends Controller $asset->name = $request->input('name'); } + // This is just meant to correct legacy issues where some user data would have 0 + // as a location ID, which isn't valid. Later versions of Snipe-IT have stricter validation + // rules, so it's necessary to fix this for long-time users. It's kinda gross, but will help + // people (and their data) in the long run + + if ($asset->rtd_location_id == '0') { + \Log::debug('Manually override the RTD location IDs'); + \Log::debug('Original RTD Location ID: '.$asset->rtd_location_id); + $asset->rtd_location_id = ''; + \Log::debug('New RTD Location ID: '.$asset->rtd_location_id); + } + + if ($asset->location_id == '0') { + \Log::debug('Manually override the location IDs'); + \Log::debug('Original Location ID: '.$asset->location_id); + $asset->location_id = ''; + \Log::debug('New Location ID: '.$asset->location_id); + } + $asset->location_id = $asset->rtd_location_id; if ($request->filled('location_id')) { diff --git a/tests/Feature/Api/Assets/AssetCheckinTest.php b/tests/Feature/Api/Assets/AssetCheckinTest.php index b7dda0d4b8..befac2e4ce 100644 --- a/tests/Feature/Api/Assets/AssetCheckinTest.php +++ b/tests/Feature/Api/Assets/AssetCheckinTest.php @@ -116,7 +116,16 @@ class AssetCheckinTest extends TestCase public function testLegacyLocationValuesSetToZeroAreUpdated() { - $this->markTestIncomplete('Not currently in controller'); + $asset = Asset::factory()->canBeInvalidUponCreation()->assignedToUser()->create([ + 'rtd_location_id' => 0, + 'location_id' => 0, + ]); + + $this->actingAsForApi(User::factory()->checkinAssets()->create()) + ->postJson(route('api.asset.checkin', $asset)); + + $this->assertNull($asset->refresh()->rtd_location_id); + $this->assertEquals($asset->location_id, $asset->rtd_location_id); } public function testPendingCheckoutAcceptancesAreClearedUponCheckin() From dba837b1d2253896a2226cf965f9f8b7899b0ce2 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Thu, 22 Feb 2024 13:21:52 -0800 Subject: [PATCH 23/31] Move location migration logic to trait --- app/Http/Controllers/Api/AssetsController.php | 22 +++---------- .../Assets/AssetCheckinController.php | 22 +++---------- app/Http/Traits/MigratesLegacyLocations.php | 33 +++++++++++++++++++ 3 files changed, 41 insertions(+), 36 deletions(-) create mode 100644 app/Http/Traits/MigratesLegacyLocations.php diff --git a/app/Http/Controllers/Api/AssetsController.php b/app/Http/Controllers/Api/AssetsController.php index 3d223248ab..f64e68621d 100644 --- a/app/Http/Controllers/Api/AssetsController.php +++ b/app/Http/Controllers/Api/AssetsController.php @@ -4,6 +4,7 @@ namespace App\Http\Controllers\Api; use App\Events\CheckoutableCheckedIn; use App\Http\Requests\StoreAssetRequest; +use App\Http\Traits\MigratesLegacyLocations; use Illuminate\Http\JsonResponse; use Illuminate\Support\Facades\Crypt; use Illuminate\Support\Facades\Gate; @@ -45,6 +46,8 @@ use Route; */ class AssetsController extends Controller { + use MigratesLegacyLocations; + /** * Returns JSON listing of all assets * @@ -886,24 +889,7 @@ class AssetsController extends Controller $asset->name = $request->input('name'); } - // This is just meant to correct legacy issues where some user data would have 0 - // as a location ID, which isn't valid. Later versions of Snipe-IT have stricter validation - // rules, so it's necessary to fix this for long-time users. It's kinda gross, but will help - // people (and their data) in the long run - - if ($asset->rtd_location_id == '0') { - \Log::debug('Manually override the RTD location IDs'); - \Log::debug('Original RTD Location ID: '.$asset->rtd_location_id); - $asset->rtd_location_id = ''; - \Log::debug('New RTD Location ID: '.$asset->rtd_location_id); - } - - if ($asset->location_id == '0') { - \Log::debug('Manually override the location IDs'); - \Log::debug('Original Location ID: '.$asset->location_id); - $asset->location_id = ''; - \Log::debug('New Location ID: '.$asset->location_id); - } + $this->migrateLegacyLocations($asset); $asset->location_id = $asset->rtd_location_id; diff --git a/app/Http/Controllers/Assets/AssetCheckinController.php b/app/Http/Controllers/Assets/AssetCheckinController.php index 891480f570..0a54a070f7 100644 --- a/app/Http/Controllers/Assets/AssetCheckinController.php +++ b/app/Http/Controllers/Assets/AssetCheckinController.php @@ -6,6 +6,7 @@ use App\Events\CheckoutableCheckedIn; use App\Helpers\Helper; use App\Http\Controllers\Controller; use App\Http\Requests\AssetCheckinRequest; +use App\Http\Traits\MigratesLegacyLocations; use App\Models\Asset; use App\Models\CheckoutAcceptance; use Illuminate\Database\Eloquent\Builder; @@ -15,6 +16,8 @@ use Illuminate\Support\Facades\View; class AssetCheckinController extends Controller { + use MigratesLegacyLocations; + /** * Returns a view that presents a form to check an asset back into inventory. * @@ -77,24 +80,7 @@ class AssetCheckinController extends Controller $asset->status_id = e($request->get('status_id')); } - // This is just meant to correct legacy issues where some user data would have 0 - // as a location ID, which isn't valid. Later versions of Snipe-IT have stricter validation - // rules, so it's necessary to fix this for long-time users. It's kinda gross, but will help - // people (and their data) in the long run - - if ($asset->rtd_location_id == '0') { - \Log::debug('Manually override the RTD location IDs'); - \Log::debug('Original RTD Location ID: '.$asset->rtd_location_id); - $asset->rtd_location_id = ''; - \Log::debug('New RTD Location ID: '.$asset->rtd_location_id); - } - - if ($asset->location_id == '0') { - \Log::debug('Manually override the location IDs'); - \Log::debug('Original Location ID: '.$asset->location_id); - $asset->location_id = ''; - \Log::debug('New Location ID: '.$asset->location_id); - } + $this->migrateLegacyLocations($asset); $asset->location_id = $asset->rtd_location_id; diff --git a/app/Http/Traits/MigratesLegacyLocations.php b/app/Http/Traits/MigratesLegacyLocations.php new file mode 100644 index 0000000000..68d170f080 --- /dev/null +++ b/app/Http/Traits/MigratesLegacyLocations.php @@ -0,0 +1,33 @@ +rtd_location_id == '0') { + \Log::debug('Manually override the RTD location IDs'); + \Log::debug('Original RTD Location ID: ' . $asset->rtd_location_id); + $asset->rtd_location_id = ''; + \Log::debug('New RTD Location ID: ' . $asset->rtd_location_id); + } + + if ($asset->location_id == '0') { + \Log::debug('Manually override the location IDs'); + \Log::debug('Original Location ID: ' . $asset->location_id); + $asset->location_id = ''; + \Log::debug('New Location ID: ' . $asset->location_id); + } + } +} From 4caadcfa191d068d862c94f03336dbea8d8dc102 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Thu, 22 Feb 2024 13:33:16 -0800 Subject: [PATCH 24/31] Clear pending checkout acceptances when checking in asset via api --- app/Http/Controllers/Api/AssetsController.php | 15 +++++++++++++++ tests/Feature/Api/Assets/AssetCheckinTest.php | 10 +++++++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/app/Http/Controllers/Api/AssetsController.php b/app/Http/Controllers/Api/AssetsController.php index f64e68621d..997b27a413 100644 --- a/app/Http/Controllers/Api/AssetsController.php +++ b/app/Http/Controllers/Api/AssetsController.php @@ -5,6 +5,8 @@ namespace App\Http\Controllers\Api; use App\Events\CheckoutableCheckedIn; use App\Http\Requests\StoreAssetRequest; use App\Http\Traits\MigratesLegacyLocations; +use App\Models\CheckoutAcceptance; +use Illuminate\Database\Eloquent\Builder; use Illuminate\Http\JsonResponse; use Illuminate\Support\Facades\Crypt; use Illuminate\Support\Facades\Gate; @@ -919,6 +921,19 @@ class AssetsController extends Controller } } + // Get all pending Acceptances for this asset and delete them + CheckoutAcceptance::pending() + ->whereHasMorph( + 'checkoutable', + [Asset::class], + function (Builder $query) use ($asset) { + $query->where('id', $asset->id); + }) + ->get() + ->map(function ($acceptance) { + $acceptance->delete(); + }); + if ($asset->save()) { event(new CheckoutableCheckedIn($asset, $target, Auth::user(), $request->input('note'), $checkin_at, $originalValues)); diff --git a/tests/Feature/Api/Assets/AssetCheckinTest.php b/tests/Feature/Api/Assets/AssetCheckinTest.php index befac2e4ce..e4f443ce30 100644 --- a/tests/Feature/Api/Assets/AssetCheckinTest.php +++ b/tests/Feature/Api/Assets/AssetCheckinTest.php @@ -4,6 +4,7 @@ namespace Tests\Feature\Api\Assets; use App\Events\CheckoutableCheckedIn; use App\Models\Asset; +use App\Models\CheckoutAcceptance; use App\Models\LicenseSeat; use App\Models\Location; use App\Models\Statuslabel; @@ -130,7 +131,14 @@ class AssetCheckinTest extends TestCase public function testPendingCheckoutAcceptancesAreClearedUponCheckin() { - $this->markTestIncomplete('Not currently in controller'); + $asset = Asset::factory()->assignedToUser()->create(); + + $acceptance = CheckoutAcceptance::factory()->for($asset, 'checkoutable')->pending()->create(); + + $this->actingAsForApi(User::factory()->checkinAssets()->create()) + ->postJson(route('api.asset.checkin', $asset)); + + $this->assertFalse($acceptance->exists(), 'Acceptance was not deleted'); } public function testCheckinTimeAndActionLogNoteCanBeSet() From b55a19cebb58dd340d04039f0eae920d1d74fbe7 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Thu, 22 Feb 2024 13:50:46 -0800 Subject: [PATCH 25/31] Add assertion event is dispatched with correct timestamp --- tests/Feature/Api/Assets/AssetCheckinTest.php | 9 ++++++++- tests/Feature/Checkins/AssetCheckinTest.php | 9 ++++++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/tests/Feature/Api/Assets/AssetCheckinTest.php b/tests/Feature/Api/Assets/AssetCheckinTest.php index e4f443ce30..2957a6f0ba 100644 --- a/tests/Feature/Api/Assets/AssetCheckinTest.php +++ b/tests/Feature/Api/Assets/AssetCheckinTest.php @@ -9,6 +9,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\Support\InteractsWithSettings; use Tests\TestCase; @@ -53,6 +54,8 @@ class AssetCheckinTest extends TestCase $this->assertTrue($asset->assignedTo->is($user)); + $currentTimestamp = now(); + $this->actingAsForApi(User::factory()->checkinAssets()->create()) ->postJson(route('api.asset.checkin', $asset), [ 'name' => 'Changed Name', @@ -61,7 +64,6 @@ class AssetCheckinTest extends TestCase ]) ->assertOk(); - Event::assertDispatched(CheckoutableCheckedIn::class, 1); $this->assertNull($asset->refresh()->assignedTo); $this->assertNull($asset->expected_checkin); $this->assertNull($asset->last_checkout); @@ -72,6 +74,11 @@ class AssetCheckinTest extends TestCase $this->assertEquals('Changed Name', $asset->name); $this->assertEquals($status->id, $asset->status_id); $this->assertTrue($asset->location()->is($location)); + + Event::assertDispatched(function (CheckoutableCheckedIn $event) use ($currentTimestamp) { + // this could be better mocked but is ok for now. + return Carbon::parse($event->action_date)->diffInSeconds($currentTimestamp) < 2; + }, 1); } public function testLocationIsSetToRTDLocationByDefaultUponCheckin() diff --git a/tests/Feature/Checkins/AssetCheckinTest.php b/tests/Feature/Checkins/AssetCheckinTest.php index fb60b61fce..3492cb6ae0 100644 --- a/tests/Feature/Checkins/AssetCheckinTest.php +++ b/tests/Feature/Checkins/AssetCheckinTest.php @@ -9,6 +9,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\Support\InteractsWithSettings; use Tests\TestCase; @@ -49,6 +50,8 @@ class AssetCheckinTest extends TestCase $this->assertTrue($asset->assignedTo->is($user)); + $currentTimestamp = now(); + $this->actingAs(User::factory()->checkinAssets()->create()) ->post( route('hardware.checkin.store', ['assetId' => $asset->id, 'backto' => 'user']), @@ -60,7 +63,6 @@ class AssetCheckinTest extends TestCase ) ->assertRedirect(route('users.show', $user)); - Event::assertDispatched(CheckoutableCheckedIn::class, 1); $this->assertNull($asset->refresh()->assignedTo); $this->assertNull($asset->expected_checkin); $this->assertNull($asset->last_checkout); @@ -71,6 +73,11 @@ class AssetCheckinTest extends TestCase $this->assertEquals('Changed Name', $asset->name); $this->assertEquals($status->id, $asset->status_id); $this->assertTrue($asset->location()->is($location)); + + Event::assertDispatched(function (CheckoutableCheckedIn $event) use ($currentTimestamp) { + // this could be better mocked but is ok for now. + return Carbon::parse($event->action_date)->diffInSeconds($currentTimestamp) < 2; + }, 1); } public function testLocationIsSetToRTDLocationByDefaultUponCheckin() From c401c88702684b73209c250279861ee4fc522ee7 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Thu, 22 Feb 2024 16:19:33 -0800 Subject: [PATCH 26/31] Scope event fake --- tests/Feature/Checkins/AssetCheckinTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Feature/Checkins/AssetCheckinTest.php b/tests/Feature/Checkins/AssetCheckinTest.php index 3492cb6ae0..3452b14c10 100644 --- a/tests/Feature/Checkins/AssetCheckinTest.php +++ b/tests/Feature/Checkins/AssetCheckinTest.php @@ -149,7 +149,7 @@ class AssetCheckinTest extends TestCase public function testCheckinTimeAndActionLogNoteCanBeSet() { - Event::fake(); + Event::fake([CheckoutableCheckedIn::class]); $this->actingAs(User::factory()->checkinAssets()->create()) ->post(route( From 29d729171c4517e0c1c2cb66cc60be82b1b3d680 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Mon, 26 Feb 2024 11:13:39 -0800 Subject: [PATCH 27/31] Align test with actual values passed from the web --- tests/Feature/Checkins/AssetCheckinTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Feature/Checkins/AssetCheckinTest.php b/tests/Feature/Checkins/AssetCheckinTest.php index 3452b14c10..a8e26312be 100644 --- a/tests/Feature/Checkins/AssetCheckinTest.php +++ b/tests/Feature/Checkins/AssetCheckinTest.php @@ -156,12 +156,12 @@ class AssetCheckinTest extends TestCase 'hardware.checkin.store', ['assetId' => Asset::factory()->assignedToUser()->create()->id] ), [ - 'checkin_at' => '2023-01-02 12:34:56', + 'checkin_at' => '2023-01-02', 'note' => 'hello' ]); Event::assertDispatched(function (CheckoutableCheckedIn $event) { - return $event->action_date === '2023-01-02 12:34:56' && $event->note === 'hello'; + return $event->action_date === '2023-01-02' && $event->note === 'hello'; }, 1); } } From 69022bb8b6817e638e69bee3089c55e908a76e64 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Tue, 27 Feb 2024 11:12:35 -0800 Subject: [PATCH 28/31] Implement test --- tests/Feature/Api/Assets/AssetCheckinTest.php | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/tests/Feature/Api/Assets/AssetCheckinTest.php b/tests/Feature/Api/Assets/AssetCheckinTest.php index 2957a6f0ba..060d0aacd1 100644 --- a/tests/Feature/Api/Assets/AssetCheckinTest.php +++ b/tests/Feature/Api/Assets/AssetCheckinTest.php @@ -150,20 +150,18 @@ class AssetCheckinTest extends TestCase public function testCheckinTimeAndActionLogNoteCanBeSet() { - $this->markTestIncomplete( - 'checkin_at currently takes a date and applies a time which is not inline with what the web controller does.' - ); - Event::fake(); $this->actingAsForApi(User::factory()->checkinAssets()->create()) ->postJson(route('api.asset.checkin', Asset::factory()->assignedToUser()->create()), [ - 'checkin_at' => '2023-01-02 12:34:56', + // time is appended to the provided date in controller + 'checkin_at' => '2023-01-02', 'note' => 'hi there', ]); Event::assertDispatched(function (CheckoutableCheckedIn $event) { - return $event->action_date === '2023-01-02 12:34:56' && $event->note === 'hi there'; + return Carbon::parse('2023-01-02')->isSameDay(Carbon::parse($event->action_date)) + && $event->note === 'hi there'; }, 1); } } From 0e460baf829d93db82452d18343246423ea5815e Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Tue, 27 Feb 2024 12:03:36 -0800 Subject: [PATCH 29/31] Improve readability --- app/Http/Controllers/Api/AssetsController.php | 10 ++++------ app/Http/Controllers/Assets/AssetCheckinController.php | 10 ++++------ 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/app/Http/Controllers/Api/AssetsController.php b/app/Http/Controllers/Api/AssetsController.php index 997b27a413..2587d6ce4c 100644 --- a/app/Http/Controllers/Api/AssetsController.php +++ b/app/Http/Controllers/Api/AssetsController.php @@ -6,6 +6,7 @@ use App\Events\CheckoutableCheckedIn; use App\Http\Requests\StoreAssetRequest; use App\Http\Traits\MigratesLegacyLocations; use App\Models\CheckoutAcceptance; +use App\Models\LicenseSeat; use Illuminate\Database\Eloquent\Builder; use Illuminate\Http\JsonResponse; use Illuminate\Support\Facades\Crypt; @@ -914,12 +915,9 @@ class AssetsController extends Controller $originalValues['action_date'] = $checkin_at; } - if(!empty($asset->licenseseats->all())){ - foreach ($asset->licenseseats as $seat){ - $seat->assigned_to = null; - $seat->save(); - } - } + $asset->licenseseats->each(function (LicenseSeat $seat) { + $seat->update(['assigned_to' => null]); + }); // Get all pending Acceptances for this asset and delete them CheckoutAcceptance::pending() diff --git a/app/Http/Controllers/Assets/AssetCheckinController.php b/app/Http/Controllers/Assets/AssetCheckinController.php index 0a54a070f7..e224522fa7 100644 --- a/app/Http/Controllers/Assets/AssetCheckinController.php +++ b/app/Http/Controllers/Assets/AssetCheckinController.php @@ -9,6 +9,7 @@ use App\Http\Requests\AssetCheckinRequest; use App\Http\Traits\MigratesLegacyLocations; use App\Models\Asset; use App\Models\CheckoutAcceptance; +use App\Models\LicenseSeat; use Illuminate\Database\Eloquent\Builder; use Illuminate\Support\Facades\Auth; use Illuminate\Support\Facades\Redirect; @@ -101,12 +102,9 @@ class AssetCheckinController extends Controller $checkin_at = $request->get('checkin_at'); } - if(!empty($asset->licenseseats->all())){ - foreach ($asset->licenseseats as $seat){ - $seat->assigned_to = null; - $seat->save(); - } - } + $asset->licenseseats->each(function (LicenseSeat $seat) { + $seat->update(['assigned_to' => null]); + }); // Get all pending Acceptances for this asset and delete them $acceptances = CheckoutAcceptance::pending()->whereHasMorph('checkoutable', From a5516e35115f925ef529a6b4028f8c6d8c16279c Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Tue, 27 Feb 2024 12:06:29 -0800 Subject: [PATCH 30/31] Improve trait name --- app/Http/Controllers/Api/AssetsController.php | 4 ++-- app/Http/Controllers/Assets/AssetCheckinController.php | 4 ++-- ...esLegacyLocations.php => MigratesLegacyAssetLocations.php} | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) rename app/Http/Traits/{MigratesLegacyLocations.php => MigratesLegacyAssetLocations.php} (97%) diff --git a/app/Http/Controllers/Api/AssetsController.php b/app/Http/Controllers/Api/AssetsController.php index 2587d6ce4c..5f010bdbb7 100644 --- a/app/Http/Controllers/Api/AssetsController.php +++ b/app/Http/Controllers/Api/AssetsController.php @@ -4,7 +4,7 @@ namespace App\Http\Controllers\Api; use App\Events\CheckoutableCheckedIn; use App\Http\Requests\StoreAssetRequest; -use App\Http\Traits\MigratesLegacyLocations; +use App\Http\Traits\MigratesLegacyAssetLocations; use App\Models\CheckoutAcceptance; use App\Models\LicenseSeat; use Illuminate\Database\Eloquent\Builder; @@ -49,7 +49,7 @@ use Route; */ class AssetsController extends Controller { - use MigratesLegacyLocations; + use MigratesLegacyAssetLocations; /** * Returns JSON listing of all assets diff --git a/app/Http/Controllers/Assets/AssetCheckinController.php b/app/Http/Controllers/Assets/AssetCheckinController.php index e224522fa7..222c7a67b0 100644 --- a/app/Http/Controllers/Assets/AssetCheckinController.php +++ b/app/Http/Controllers/Assets/AssetCheckinController.php @@ -6,7 +6,7 @@ use App\Events\CheckoutableCheckedIn; use App\Helpers\Helper; use App\Http\Controllers\Controller; use App\Http\Requests\AssetCheckinRequest; -use App\Http\Traits\MigratesLegacyLocations; +use App\Http\Traits\MigratesLegacyAssetLocations; use App\Models\Asset; use App\Models\CheckoutAcceptance; use App\Models\LicenseSeat; @@ -17,7 +17,7 @@ use Illuminate\Support\Facades\View; class AssetCheckinController extends Controller { - use MigratesLegacyLocations; + use MigratesLegacyAssetLocations; /** * Returns a view that presents a form to check an asset back into inventory. diff --git a/app/Http/Traits/MigratesLegacyLocations.php b/app/Http/Traits/MigratesLegacyAssetLocations.php similarity index 97% rename from app/Http/Traits/MigratesLegacyLocations.php rename to app/Http/Traits/MigratesLegacyAssetLocations.php index 68d170f080..13b464d0ca 100644 --- a/app/Http/Traits/MigratesLegacyLocations.php +++ b/app/Http/Traits/MigratesLegacyAssetLocations.php @@ -4,7 +4,7 @@ namespace App\Http\Traits; use App\Models\Asset; -trait MigratesLegacyLocations +trait MigratesLegacyAssetLocations { /** * This is just meant to correct legacy issues where some user data would have 0 From 5084e5d3ef8fe8629c2d44fad87ccdbe80ee1538 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Tue, 27 Feb 2024 12:23:26 -0800 Subject: [PATCH 31/31] Improve variable type --- app/Http/Controllers/Api/AssetsController.php | 4 ++-- tests/Feature/Api/Assets/AssetCheckinTest.php | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/Http/Controllers/Api/AssetsController.php b/app/Http/Controllers/Api/AssetsController.php index 5f010bdbb7..d78a2074f9 100644 --- a/app/Http/Controllers/Api/AssetsController.php +++ b/app/Http/Controllers/Api/AssetsController.php @@ -899,8 +899,8 @@ class AssetsController extends Controller if ($request->filled('location_id')) { $asset->location_id = $request->input('location_id'); - if ($request->get('update_default_location') == 0){ - $asset->rtd_location_id = $request->get('location_id'); + if ($request->input('update_default_location')){ + $asset->rtd_location_id = $request->input('location_id'); } } diff --git a/tests/Feature/Api/Assets/AssetCheckinTest.php b/tests/Feature/Api/Assets/AssetCheckinTest.php index 060d0aacd1..27d5e6de80 100644 --- a/tests/Feature/Api/Assets/AssetCheckinTest.php +++ b/tests/Feature/Api/Assets/AssetCheckinTest.php @@ -103,7 +103,7 @@ class AssetCheckinTest extends TestCase $this->actingAsForApi(User::factory()->checkinAssets()->create()) ->postJson(route('api.asset.checkin', $asset), [ 'location_id' => $location->id, - 'update_default_location' => 0 + 'update_default_location' => true, ]); $this->assertTrue($asset->refresh()->defaultLoc()->is($location));