Merge pull request #15547 from snipe/disallow_checkout_with_nondeployable_status

Fixed #13396 - do not allow checkout to undeployable status types
2024-12-23 12:44:12 -08:00 · 2024-09-25 18:32:59 +01:00 · 2024-09-25 18:32:59 +01:00 · cb6e5042d9
parent c1a887b48b ba69259f2d
commit cb6e5042d9
5 changed files with 135 additions and 25 deletions
--- 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(),
        );
    }
--- 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;
    }

--- a/app/Rules/AssetCannotBeCheckedOutToNondeployableStatus.php
+++ b/app/Rules/AssetCannotBeCheckedOutToNondeployableStatus.php
@ -0,0 +1,51 @@
+<?php
+
+namespace App\Rules;
+
+use App\Models\Statuslabel;
+use Closure;
+use Illuminate\Contracts\Validation\ValidationRule;
+use Illuminate\Contracts\Validation\DataAwareRule;
+
+
+class AssetCannotBeCheckedOutToNondeployableStatus implements DataAwareRule, ValidationRule
+{
+
+    /**
+     * All of the data under validation.
+     *
+     * @var array<string, mixed>
+     */
+    protected $data = [];
+
+
+    /**
+     * Set the data under validation.
+     *
+     * @param  array<string, mixed>  $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'));
+            }
+
+        }
+
+
+    }
+}
--- 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 @@
                                            </div>
                                            <div class="col-md-9">
                                                {!! $asset->checkInvalidNextAuditDate() ? '<i class="fas fa-exclamation-triangle text-orange" aria-hidden="true"></i>' : '' !!}
-                                                {{ 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
--- 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();