From ba69259f2d2181d9bad2f9fb33d2e46c5137a8f8 Mon Sep 17 00:00:00 2001 From: snipe Date: Tue, 24 Sep 2024 15:35:00 +0100 Subject: [PATCH] Fixed #13396 - do not allow checkout to undeployable status types Signed-off-by: snipe --- app/Http/Requests/StoreAssetRequest.php | 2 + app/Models/Asset.php | 13 ++- ...annotBeCheckedOutToNondeployableStatus.php | 51 +++++++++++ resources/views/hardware/view.blade.php | 4 +- tests/Feature/Assets/Api/StoreAssetTest.php | 90 ++++++++++++++----- 5 files changed, 135 insertions(+), 25 deletions(-) create mode 100644 app/Rules/AssetCannotBeCheckedOutToNondeployableStatus.php diff --git a/app/Http/Requests/StoreAssetRequest.php b/app/Http/Requests/StoreAssetRequest.php index b2feb72f79..e1665e2136 100644 --- a/app/Http/Requests/StoreAssetRequest.php +++ b/app/Http/Requests/StoreAssetRequest.php @@ -9,6 +9,7 @@ use App\Models\Setting; use Carbon\Carbon; use Carbon\Exceptions\InvalidFormatException; use Illuminate\Support\Facades\Gate; +use App\Rules\AssetCannotBeCheckedOutToNondeployableStatus; class StoreAssetRequest extends ImageUploadRequest { @@ -61,6 +62,7 @@ class StoreAssetRequest extends ImageUploadRequest return array_merge( $modelRules, + ['status_id' => [new AssetCannotBeCheckedOutToNondeployableStatus()]], parent::rules(), ); } diff --git a/app/Models/Asset.php b/app/Models/Asset.php index d4ecc6bf32..bd0578fc2e 100644 --- a/app/Models/Asset.php +++ b/app/Models/Asset.php @@ -930,9 +930,20 @@ class Asset extends Depreciable * */ public function checkInvalidNextAuditDate() { - if (($this->last_audit_date) && ($this->next_audit_date) && ($this->last_audit_date > $this->next_audit_date)) { + + // Deliberately parse the dates as Y-m-d (without H:i:s) to compare them + if ($this->last_audit_date) { + $last = Carbon::parse($this->last_audit_date)->format('Y-m-d'); + } + + if ($this->next_audit_date) { + $next = Carbon::parse($this->next_audit_date)->format('Y-m-d'); + } + + if ((isset($last) && (isset($next))) && ($last > $next)) { return true; } + return false; } diff --git a/app/Rules/AssetCannotBeCheckedOutToNondeployableStatus.php b/app/Rules/AssetCannotBeCheckedOutToNondeployableStatus.php new file mode 100644 index 0000000000..c2c451b82b --- /dev/null +++ b/app/Rules/AssetCannotBeCheckedOutToNondeployableStatus.php @@ -0,0 +1,51 @@ + + */ + protected $data = []; + + + /** + * Set the data under validation. + * + * @param array $data + */ + public function setData(array $data): static + { + $this->data = $data; + return $this; + } + + /** + * Run the validation rule. + * + * @param \Closure(string): \Illuminate\Translation\PotentiallyTranslatedString $fail + */ + public function validate(string $attribute, mixed $value, Closure $fail): void + { + // Check to see if any of the assign-ish fields are set + if ((isset($this->data['assigned_to'])) || (isset($this->data['assigned_user'])) || (isset($this->data['assigned_location'])) || (isset($this->data['assigned_asset'])) || (isset($this->data['assigned_type']))) { + + if (($value) && ($label = Statuslabel::find($value)) && ($label->getStatuslabelType()!='deployable')) { + $fail(trans('admin/hardware/form.asset_not_deployable')); + } + + } + + + } +} diff --git a/resources/views/hardware/view.blade.php b/resources/views/hardware/view.blade.php index 92e4b9313f..0dd2d75c46 100755 --- a/resources/views/hardware/view.blade.php +++ b/resources/views/hardware/view.blade.php @@ -27,7 +27,7 @@ [ 'warning' => trans('admin/hardware/message.warning_audit_date_mismatch', [ - 'last_audit_date' => Helper::getFormattedDateObject($asset->last_audit_date, 'date', false), + 'last_audit_date' => Helper::getFormattedDateObject($asset->last_audit_date, 'datetime', false), 'next_audit_date' => Helper::getFormattedDateObject($asset->next_audit_date, 'date', false) ] ) @@ -493,7 +493,7 @@
{!! $asset->checkInvalidNextAuditDate() ? '' : '' !!} - {{ Helper::getFormattedDateObject($audit_log->created_at, 'date', false) }} + {{ Helper::getFormattedDateObject($audit_log->created_at, 'datetime', false) }} @if ($audit_log->user) (by {{ link_to_route('users.show', $audit_log->user->present()->fullname(), [$audit_log->user->id]) }}) @endif diff --git a/tests/Feature/Assets/Api/StoreAssetTest.php b/tests/Feature/Assets/Api/StoreAssetTest.php index a3f5981635..a147504519 100644 --- a/tests/Feature/Assets/Api/StoreAssetTest.php +++ b/tests/Feature/Assets/Api/StoreAssetTest.php @@ -29,7 +29,7 @@ class StoreAssetTest extends TestCase $location = Location::factory()->create(); $model = AssetModel::factory()->create(); $rtdLocation = Location::factory()->create(); - $status = Statuslabel::factory()->create(); + $status = Statuslabel::factory()->readyToDeploy()->create(); $supplier = Supplier::factory()->create(); $user = User::factory()->createAssets()->create(); $userAssigned = User::factory()->create(); @@ -90,7 +90,7 @@ class StoreAssetTest extends TestCase 'last_audit_date' => '2023-09-03', 'asset_tag' => '1234', 'model_id' => AssetModel::factory()->create()->id, - 'status_id' => Statuslabel::factory()->create()->id, + 'status_id' => Statuslabel::factory()->readyToDeploy()->create()->id, ]) ->assertOk() ->assertStatusMessageIs('success'); @@ -106,7 +106,7 @@ class StoreAssetTest extends TestCase // 'last_audit_date' => '2023-09-03 12:23:45', 'asset_tag' => '1234', 'model_id' => AssetModel::factory()->create()->id, - 'status_id' => Statuslabel::factory()->create()->id, + 'status_id' => Statuslabel::factory()->readyToDeploy()->create()->id, ]) ->assertOk() ->assertStatusMessageIs('success'); @@ -122,17 +122,63 @@ class StoreAssetTest extends TestCase 'last_audit_date' => 'this-is-not-valid', 'asset_tag' => '1234', 'model_id' => AssetModel::factory()->create()->id, - 'status_id' => Statuslabel::factory()->create()->id, + 'status_id' => Statuslabel::factory()->readyToDeploy()->create()->id, ]) + ->assertOk() ->assertStatusMessageIs('error'); $this->assertNotNull($response->json('messages.last_audit_date')); } + public function testSaveWithArchivedStatusAndUserReturnsValidationError() + { + $response = $this->actingAsForApi(User::factory()->superuser()->create()) + ->postJson(route('api.assets.store'), [ + 'assigned_to' => '1', + 'assigned_type' => User::class, + 'model_id' => AssetModel::factory()->create()->id, + 'status_id' => Statuslabel::factory()->archived()->create()->id, + ]) + ->assertOk() + ->assertStatusMessageIs('error'); + + $this->assertNotNull($response->json('messages.status_id')); + } + + public function testSaveWithPendingStatusAndUserReturnsValidationError() + { + $response = $this->actingAsForApi(User::factory()->superuser()->create()) + ->postJson(route('api.assets.store'), [ + 'assigned_to' => '1', + 'assigned_type' => User::class, + 'model_id' => AssetModel::factory()->create()->id, + 'status_id' => Statuslabel::factory()->pending()->create()->id, + ]) + ->assertOk() + ->assertJson([ + 'messages' => ['status_id' => [trans('admin/hardware/form.asset_not_deployable')]] + ]); + + $this->assertNotNull($response->json('messages.status_id')); + } + + public function testSaveWithPendingStatusWithoutUserIsSuccessful() + { + $response = $this->actingAsForApi(User::factory()->superuser()->create()) + ->postJson(route('api.assets.store'), [ + 'asset_tag' => '1234', + 'model_id' => AssetModel::factory()->create()->id, + 'status_id' => Statuslabel::factory()->pending()->create()->id, + ]) + ->assertOk() + ->assertStatusMessageIs('success'); + } + + public function testArchivedDepreciateAndPhysicalCanBeNull() { $model = AssetModel::factory()->ipadModel()->create(); - $status = Statuslabel::factory()->create(); + $status = Statuslabel::factory()->readyToDeploy()->create(); $this->settings->enableAutoIncrement(); @@ -157,7 +203,7 @@ class StoreAssetTest extends TestCase public function testArchivedDepreciateAndPhysicalCanBeEmpty() { $model = AssetModel::factory()->ipadModel()->create(); - $status = Statuslabel::factory()->create(); + $status = Statuslabel::factory()->readyToDeploy()->create(); $this->settings->enableAutoIncrement(); @@ -182,7 +228,7 @@ class StoreAssetTest extends TestCase public function testAssetEolDateIsCalculatedIfPurchaseDateSet() { $model = AssetModel::factory()->mbp13Model()->create(); - $status = Statuslabel::factory()->create(); + $status = Statuslabel::factory()->readyToDeploy()->create(); $this->settings->enableAutoIncrement(); @@ -203,7 +249,7 @@ class StoreAssetTest extends TestCase public function testAssetEolDateIsNotCalculatedIfPurchaseDateNotSet() { $model = AssetModel::factory()->mbp13Model()->create(); - $status = Statuslabel::factory()->create(); + $status = Statuslabel::factory()->readyToDeploy()->create(); $this->settings->enableAutoIncrement(); @@ -223,7 +269,7 @@ class StoreAssetTest extends TestCase public function testAssetEolExplicitIsSetIfAssetEolDateIsExplicitlySet() { $model = AssetModel::factory()->mbp13Model()->create(); - $status = Statuslabel::factory()->create(); + $status = Statuslabel::factory()->readyToDeploy()->create(); $this->settings->enableAutoIncrement(); @@ -245,7 +291,7 @@ class StoreAssetTest extends TestCase public function testAssetGetsAssetTagWithAutoIncrement() { $model = AssetModel::factory()->create(); - $status = Statuslabel::factory()->create(); + $status = Statuslabel::factory()->readyToDeploy()->create(); $this->settings->enableAutoIncrement(); @@ -265,7 +311,7 @@ class StoreAssetTest extends TestCase public function testAssetCreationFailsWithNoAssetTagOrAutoIncrement() { $model = AssetModel::factory()->create(); - $status = Statuslabel::factory()->create(); + $status = Statuslabel::factory()->readyToDeploy()->create(); $this->settings->disableAutoIncrement(); @@ -289,7 +335,7 @@ class StoreAssetTest extends TestCase ->postJson(route('api.assets.store'), [ 'asset_tag' => 'random-string', 'model_id' => AssetModel::factory()->create()->id, - 'status_id' => Statuslabel::factory()->create()->id, + 'status_id' => Statuslabel::factory()->readyToDeploy()->create()->id, // API accepts float 'purchase_cost' => 12.34, ]) @@ -311,7 +357,7 @@ class StoreAssetTest extends TestCase ->postJson(route('api.assets.store'), [ 'asset_tag' => 'random-string', 'model_id' => AssetModel::factory()->create()->id, - 'status_id' => Statuslabel::factory()->create()->id, + 'status_id' => Statuslabel::factory()->readyToDeploy()->create()->id, // API also accepts string for comma separated values 'purchase_cost' => '12,34', ]) @@ -325,7 +371,7 @@ class StoreAssetTest extends TestCase public function testUniqueSerialNumbersIsEnforcedWhenEnabled() { $model = AssetModel::factory()->create(); - $status = Statuslabel::factory()->create(); + $status = Statuslabel::factory()->readyToDeploy()->create(); $serial = '1234567890'; $this->settings->enableAutoIncrement(); @@ -353,7 +399,7 @@ class StoreAssetTest extends TestCase public function testUniqueSerialNumbersIsNotEnforcedWhenDisabled() { $model = AssetModel::factory()->create(); - $status = Statuslabel::factory()->create(); + $status = Statuslabel::factory()->readyToDeploy()->create(); $serial = '1234567890'; $this->settings->enableAutoIncrement(); @@ -381,7 +427,7 @@ class StoreAssetTest extends TestCase public function testAssetTagsMustBeUniqueWhenUndeleted() { $model = AssetModel::factory()->create(); - $status = Statuslabel::factory()->create(); + $status = Statuslabel::factory()->readyToDeploy()->create(); $asset_tag = '1234567890'; $this->settings->disableAutoIncrement(); @@ -408,7 +454,7 @@ class StoreAssetTest extends TestCase public function testAssetTagsCanBeDuplicatedIfDeleted() { $model = AssetModel::factory()->create(); - $status = Statuslabel::factory()->create(); + $status = Statuslabel::factory()->readyToDeploy()->create(); $asset_tag = '1234567890'; $this->settings->disableAutoIncrement(); @@ -438,7 +484,7 @@ class StoreAssetTest extends TestCase public function testAnAssetCanBeCheckedOutToUserOnStore() { $model = AssetModel::factory()->create(); - $status = Statuslabel::factory()->create(); + $status = Statuslabel::factory()->readyToDeploy()->create(); $user = User::factory()->createAssets()->create(); $userAssigned = User::factory()->create(); @@ -464,7 +510,7 @@ class StoreAssetTest extends TestCase public function testAnAssetCanBeCheckedOutToLocationOnStore() { $model = AssetModel::factory()->create(); - $status = Statuslabel::factory()->create(); + $status = Statuslabel::factory()->readyToDeploy()->create(); $location = Location::factory()->create(); $user = User::factory()->createAssets()->create(); @@ -490,7 +536,7 @@ class StoreAssetTest extends TestCase public function testAnAssetCanBeCheckedOutToAssetOnStore() { $model = AssetModel::factory()->create(); - $status = Statuslabel::factory()->create(); + $status = Statuslabel::factory()->readyToDeploy()->create(); $asset = Asset::factory()->create(); $user = User::factory()->createAssets()->create(); @@ -530,7 +576,7 @@ class StoreAssetTest extends TestCase { $this->markIncompleteIfMySQL('Custom Fields tests do not work on MySQL'); - $status = Statuslabel::factory()->create(); + $status = Statuslabel::factory()->readyToDeploy()->create(); $field = CustomField::factory()->testEncrypted()->create(); $superuser = User::factory()->superuser()->create(); $assetData = Asset::factory()->hasEncryptedCustomField($field)->make(); @@ -555,7 +601,7 @@ class StoreAssetTest extends TestCase // @todo: $this->markTestIncomplete(); - $status = Statuslabel::factory()->create(); + $status = Statuslabel::factory()->readyToDeploy()->create(); $field = CustomField::factory()->testEncrypted()->create(); $normal_user = User::factory()->editAssets()->create(); $assetData = Asset::factory()->hasEncryptedCustomField($field)->make();