From 98323185de298a32df9f545ea5ebacc6a7bc42cd Mon Sep 17 00:00:00 2001 From: snipe Date: Wed, 11 Sep 2024 16:43:33 +0100 Subject: [PATCH 1/4] Null and warn if editing asset to a non-deployabe state Signed-off-by: snipe --- app/Http/Controllers/Assets/AssetsController.php | 9 ++++++++- resources/lang/en-US/admin/hardware/form.php | 1 + resources/views/hardware/edit.blade.php | 2 +- 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/app/Http/Controllers/Assets/AssetsController.php b/app/Http/Controllers/Assets/AssetsController.php index 59b22b386d..d162b9a98d 100755 --- a/app/Http/Controllers/Assets/AssetsController.php +++ b/app/Http/Controllers/Assets/AssetsController.php @@ -2,6 +2,7 @@ namespace App\Http\Controllers\Assets; +use App\Events\CheckoutableCheckedIn; use App\Helpers\Helper; use App\Http\Controllers\Controller; use App\Http\Requests\ImageUploadRequest; @@ -336,8 +337,14 @@ class AssetsController extends Controller $status = Statuslabel::find($asset->status_id); - if ($status && $status->archived) { + // This is a non-deployable status label - we should check the asset back in. + if (($status && $status->getStatuslabelType() != 'deployable') && (is_null($target = $asset->assignedTo))) { + + $originalValues = $asset->getRawOriginal(); $asset->assigned_to = null; + $asset->assigned_type = null; + + event(new CheckoutableCheckedIn($asset, $target, auth()->user(), 'Checkin on asset update', date('Y-m-d H:i:s'), $originalValues)); } if ($asset->assigned_to == '') { diff --git a/resources/lang/en-US/admin/hardware/form.php b/resources/lang/en-US/admin/hardware/form.php index edec543637..03b8f04add 100644 --- a/resources/lang/en-US/admin/hardware/form.php +++ b/resources/lang/en-US/admin/hardware/form.php @@ -55,6 +55,7 @@ return [ 'asset_location_update_default' => 'Update only default location', 'asset_location_update_actual' => 'Update only actual location', 'asset_not_deployable' => 'That asset status is not deployable. This asset cannot be checked out.', + 'asset_not_deployable_checkin' => 'That asset status is not deployable. Using this status label will checkin the asset.', 'asset_deployable' => 'That status is deployable. This asset can be checked out.', 'processing_spinner' => 'Processing... (This might take a bit of time on large files)', 'optional_infos' => 'Optional Information', diff --git a/resources/views/hardware/edit.blade.php b/resources/views/hardware/edit.blade.php index bdbc3c3c2e..b756187fdd 100755 --- a/resources/views/hardware/edit.blade.php +++ b/resources/views/hardware/edit.blade.php @@ -263,7 +263,7 @@ $("#assignto_selector").hide(); $("#selected_status_status").removeClass('text-success'); $("#selected_status_status").addClass('text-danger'); - $("#selected_status_status").html(' {{ trans('admin/hardware/form.asset_not_deployable')}} '); + $("#selected_status_status").html(' {{ (($item->assignedTo) && ($item->deleted_at == '')) ? trans('admin/hardware/form.asset_not_deployable_checkin') : trans('admin/hardware/form.asset_not_deployable') }} '); } } }); From 04e96b8f20095568aa37dad8eca2d0b1effff07e Mon Sep 17 00:00:00 2001 From: snipe Date: Wed, 11 Sep 2024 16:43:41 +0100 Subject: [PATCH 2/4] Added tests Signed-off-by: snipe --- tests/Feature/Assets/Ui/EditAssetTest.php | 43 ++++++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/tests/Feature/Assets/Ui/EditAssetTest.php b/tests/Feature/Assets/Ui/EditAssetTest.php index befe64a864..c17366bed5 100644 --- a/tests/Feature/Assets/Ui/EditAssetTest.php +++ b/tests/Feature/Assets/Ui/EditAssetTest.php @@ -2,16 +2,20 @@ namespace Tests\Feature\Assets\Ui; +use App\Events\CheckoutableCheckedIn; use App\Models\Asset; use App\Models\AssetModel; +use App\Models\Location; use App\Models\StatusLabel; use App\Models\User; +use Illuminate\Support\Carbon; +use Illuminate\Support\Facades\Event; use Tests\TestCase; class EditAssetTest extends TestCase { - public function testPermissionRequiredToViewLicense() + public function testPermissionRequiredToViewAsset() { $asset = Asset::factory()->create(); $this->actingAs(User::factory()->create()) @@ -64,4 +68,41 @@ class EditAssetTest extends TestCase $this->assertDatabaseHas('assets', ['asset_tag' => 'New Asset Tag']); } + public function testNewCheckinIsLoggedIfStatusChangedToUndeployable() + { + Event::fake([CheckoutableCheckedIn::class]); + + $user = User::factory()->create(); + $location = Location::factory()->create(); + $deployable_status = Statuslabel::factory()->rtd()->create(); + $achived_status = Statuslabel::factory()->archived()->create(); + + $asset = Asset::factory()->assignedToUser($user)->create(['status_id' => $deployable_status->id]); + + $this->assertTrue($asset->assignedTo->is($user)); + + $currentTimestamp = now(); + + $this->actingAs(User::factory()->viewAssets()->editAssets()->create()) + ->post( + route('hardware.update', ['hardware' => $asset->id]), + [ + 'status_id' => $achived_status->id, + ], + ); + + $asset->refresh(); + \Log::error('AssignedTo: '.$asset->refresh()->assignedTo); + \Log::error('Assigned_to: '.$asset->refresh()->assigned_to); + \Log::error('Assigned_type: '.$asset->refresh()->assigned_type); + $this->assertNull($asset->refresh()->assignedTo); + $this->assertNull($asset->refresh()->assigned_type); + $this->assertEquals($achived_status->id, $asset->refresh()->status_id); + + 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); + } + } From 43250ae88142d3168a009457c54d4eb01bbe261a Mon Sep 17 00:00:00 2001 From: snipe Date: Wed, 11 Sep 2024 17:04:09 +0100 Subject: [PATCH 3/4] Updated tests Signed-off-by: snipe --- .../Controllers/Assets/AssetsController.php | 1 + tests/Feature/Assets/Ui/EditAssetTest.php | 22 +++++++------------ 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/app/Http/Controllers/Assets/AssetsController.php b/app/Http/Controllers/Assets/AssetsController.php index d162b9a98d..2bb248b993 100755 --- a/app/Http/Controllers/Assets/AssetsController.php +++ b/app/Http/Controllers/Assets/AssetsController.php @@ -343,6 +343,7 @@ class AssetsController extends Controller $originalValues = $asset->getRawOriginal(); $asset->assigned_to = null; $asset->assigned_type = null; + $asset->accepted = null; event(new CheckoutableCheckedIn($asset, $target, auth()->user(), 'Checkin on asset update', date('Y-m-d H:i:s'), $originalValues)); } diff --git a/tests/Feature/Assets/Ui/EditAssetTest.php b/tests/Feature/Assets/Ui/EditAssetTest.php index c17366bed5..c7aa85478c 100644 --- a/tests/Feature/Assets/Ui/EditAssetTest.php +++ b/tests/Feature/Assets/Ui/EditAssetTest.php @@ -73,34 +73,28 @@ class EditAssetTest extends TestCase Event::fake([CheckoutableCheckedIn::class]); $user = User::factory()->create(); - $location = Location::factory()->create(); $deployable_status = Statuslabel::factory()->rtd()->create(); $achived_status = Statuslabel::factory()->archived()->create(); - $asset = Asset::factory()->assignedToUser($user)->create(['status_id' => $deployable_status->id]); - $this->assertTrue($asset->assignedTo->is($user)); $currentTimestamp = now(); $this->actingAs(User::factory()->viewAssets()->editAssets()->create()) - ->post( - route('hardware.update', ['hardware' => $asset->id]), - [ + ->from(route('hardware.edit', $asset)) + ->patch(route('hardware.update', $asset), [ 'status_id' => $achived_status->id, + 'model_id' => $asset->model_id, + 'asset_tag' => $asset->asset_tag, ], - ); + ) + ->assertStatus(302); $asset->refresh(); - \Log::error('AssignedTo: '.$asset->refresh()->assignedTo); - \Log::error('Assigned_to: '.$asset->refresh()->assigned_to); - \Log::error('Assigned_type: '.$asset->refresh()->assigned_type); - $this->assertNull($asset->refresh()->assignedTo); - $this->assertNull($asset->refresh()->assigned_type); - $this->assertEquals($achived_status->id, $asset->refresh()->status_id); + $this->assertNull($asset->assigned_type); + $this->assertEquals($achived_status->id, $asset->status_id); 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); } From a97530367d6013a05c5a99341087dd2f8002343b Mon Sep 17 00:00:00 2001 From: snipe Date: Wed, 11 Sep 2024 17:29:22 +0100 Subject: [PATCH 4/4] Fixed tests again Signed-off-by: snipe --- app/Http/Controllers/Assets/AssetsController.php | 6 +++--- resources/views/hardware/edit.blade.php | 2 +- tests/Feature/Assets/Ui/EditAssetTest.php | 11 +++++++---- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/app/Http/Controllers/Assets/AssetsController.php b/app/Http/Controllers/Assets/AssetsController.php index 2bb248b993..5f944c386e 100755 --- a/app/Http/Controllers/Assets/AssetsController.php +++ b/app/Http/Controllers/Assets/AssetsController.php @@ -331,14 +331,14 @@ class AssetsController extends Controller $asset->expected_checkin = $request->input('expected_checkin', null); // If the box isn't checked, it's not in the request at all. - $asset->requestable = $request->filled('requestable'); + $asset->requestable = $request->filled('requestable', 0); $asset->rtd_location_id = $request->input('rtd_location_id', null); $asset->byod = $request->input('byod', 0); - $status = Statuslabel::find($asset->status_id); + $status = Statuslabel::find($request->input('status_id')); // This is a non-deployable status label - we should check the asset back in. - if (($status && $status->getStatuslabelType() != 'deployable') && (is_null($target = $asset->assignedTo))) { + if (($status && $status->getStatuslabelType() != 'deployable') && ($target = $asset->assignedTo)) { $originalValues = $asset->getRawOriginal(); $asset->assigned_to = null; diff --git a/resources/views/hardware/edit.blade.php b/resources/views/hardware/edit.blade.php index b756187fdd..4c14b2ea02 100755 --- a/resources/views/hardware/edit.blade.php +++ b/resources/views/hardware/edit.blade.php @@ -263,7 +263,7 @@ $("#assignto_selector").hide(); $("#selected_status_status").removeClass('text-success'); $("#selected_status_status").addClass('text-danger'); - $("#selected_status_status").html(' {{ (($item->assignedTo) && ($item->deleted_at == '')) ? trans('admin/hardware/form.asset_not_deployable_checkin') : trans('admin/hardware/form.asset_not_deployable') }} '); + $("#selected_status_status").html(' {{ (($item->assigned_to!='') && ($item->assigned_type!='') && ($item->deleted_at == '')) ? trans('admin/hardware/form.asset_not_deployable_checkin') : trans('admin/hardware/form.asset_not_deployable') }} '); } } }); diff --git a/tests/Feature/Assets/Ui/EditAssetTest.php b/tests/Feature/Assets/Ui/EditAssetTest.php index c7aa85478c..27f00b5313 100644 --- a/tests/Feature/Assets/Ui/EditAssetTest.php +++ b/tests/Feature/Assets/Ui/EditAssetTest.php @@ -81,16 +81,19 @@ class EditAssetTest extends TestCase $currentTimestamp = now(); $this->actingAs(User::factory()->viewAssets()->editAssets()->create()) - ->from(route('hardware.edit', $asset)) - ->patch(route('hardware.update', $asset), [ + ->from(route('hardware.edit', $asset->id)) + ->put(route('hardware.update', $asset->id), [ 'status_id' => $achived_status->id, 'model_id' => $asset->model_id, - 'asset_tag' => $asset->asset_tag, + 'asset_tags' => $asset->asset_tag, ], ) ->assertStatus(302); + //->assertRedirect(route('hardware.show', ['hardware' => $asset->id]));; - $asset->refresh(); + // $asset->refresh(); + $asset = Asset::find($asset->id); + $this->assertNull($asset->assigned_to); $this->assertNull($asset->assigned_type); $this->assertEquals($achived_status->id, $asset->status_id);