Merge pull request #14458 from spencerrlongg/bug/sc-24884

Add Form Request and Tests for Update Asset API Method

app/Http/Controllers/Api/AssetsController.php

@@ -4,6 +4,7 @@
 
 use App\Events\CheckoutableCheckedIn;
 use App\Http\Requests\StoreAssetRequest;
+use App\Http\Requests\UpdateAssetRequest;
 use App\Http\Traits\MigratesLegacyAssetLocations;
 use App\Models\CheckoutAcceptance;
 use App\Models\LicenseSeat;
@@ -651,36 +652,35 @@ public function store(StoreAssetRequest $request): JsonResponse
      * Accepts a POST request to update an asset
      *
      * @author [A. Gianotto] [<[email protected]>]
-     * @param \App\Http\Requests\ImageUploadRequest $request
      * @since [v4.0]
      */
-    public function update(ImageUploadRequest $request, $id) : JsonResponse
+    public function update(UpdateAssetRequest $request, Asset $asset): JsonResponse
     {
-        $this->authorize('update', Asset::class);
-
-        if ($asset = Asset::find($id)) {
-            $asset->fill($request->all());
+        $asset->fill($request->validated());
 
-            ($request->filled('model_id')) ?
-                $asset->model()->associate(AssetModel::find($request->get('model_id'))) : null;
-            ($request->filled('rtd_location_id')) ?
-                $asset->location_id = $request->get('rtd_location_id') : '';
-            ($request->filled('company_id')) ?
-                $asset->company_id = Company::getIdForCurrentUser($request->get('company_id')) : '';
+        if ($request->has('model_id')) {
+            $asset->model()->associate(AssetModel::find($request->validated()['model_id']));
+        }
+        if ($request->has('company_id')) {
+            $asset->company_id = Company::getIdForCurrentUser($request->validated()['company_id']);
+        }
+        if ($request->has('rtd_location_id') && !$request->has('location_id')) {
+            $asset->location_id = $request->validated()['rtd_location_id'];
+        }
+        if ($request->input('last_audit_date')) {
+            $asset->last_audit_date = Carbon::parse($request->input('last_audit_date'))->startOfDay()->format('Y-m-d H:i:s');
+        }
 
-            ($request->filled('rtd_location_id')) ?
-                $asset->location_id = $request->get('rtd_location_id') : null;
+        /**
+        * this is here just legacy reasons. Api\AssetController
+        * used image_source  once to allow encoded image uploads.
+        */
+        if ($request->has('image_source')) {
+            $request->offsetSet('image', $request->offsetGet('image_source'));
+        }
 
-            /**
-            * this is here just legacy reasons. Api\AssetController
-            * used image_source  once to allow encoded image uploads.
-            */
-            if ($request->has('image_source')) {
-                $request->offsetSet('image', $request->offsetGet('image_source'));
-            }     
-
-            $asset = $request->handleImages($asset);
-            $model = AssetModel::find($asset->model_id);
+        $asset = $request->handleImages($asset);
+        $model = $asset->model;
 
             // Update custom fields
             $problems_updating_encrypted_custom_fields = false;
@@ -706,15 +706,13 @@ public function update(ImageUploadRequest $request, $id) : JsonResponse
                     }
                 }
             }
-
-
             if ($asset->save()) {
                 if (($request->filled('assigned_user')) && ($target = User::find($request->get('assigned_user')))) {
                         $location = $target->location_id;
                 } elseif (($request->filled('assigned_asset')) && ($target = Asset::find($request->get('assigned_asset')))) {
                     $location = $target->location_id;
 
-                    Asset::where('assigned_type', \App\Models\Asset::class)->where('assigned_to', $id)
+                    Asset::where('assigned_type', \App\Models\Asset::class)->where('assigned_to', $asset->id)
                         ->update(['location_id' => $target->location_id]);
                 } elseif (($request->filled('assigned_location')) && ($target = Location::find($request->get('assigned_location')))) {
                     $location = $target->id;
@@ -728,17 +726,13 @@ public function update(ImageUploadRequest $request, $id) : JsonResponse
                     $asset->image = $asset->getImageUrl();
                 }
 
-                if ($problems_updating_encrypted_custom_fields) {
-                    return response()->json(Helper::formatStandardApiResponse('success', $asset, trans('admin/hardware/message.update.encrypted_warning')));
-                } else {
-                    return response()->json(Helper::formatStandardApiResponse('success', $asset, trans('admin/hardware/message.update.success')));
-                }
+            if ($problems_updating_encrypted_custom_fields) {
+                return response()->json(Helper::formatStandardApiResponse('success', $asset, trans('admin/hardware/message.update.encrypted_warning')));
+            } else {
+                return response()->json(Helper::formatStandardApiResponse('success', $asset, trans('admin/hardware/message.update.success')));
             }
-
-            return response()->json(Helper::formatStandardApiResponse('error', null, $asset->getErrors()), 200);
         }
-
-        return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.does_not_exist')), 200);
+        return response()->json(Helper::formatStandardApiResponse('error', null, $asset->getErrors()), 200);
     }
 
 

app/Http/Requests/UpdateAssetRequest.php

@@ -0,0 +1,44 @@
+<?php
+
+namespace App\Http\Requests;
+
+use App\Models\Asset;
+use Illuminate\Support\Facades\Gate;
+use Illuminate\Validation\Rule;
+
+class UpdateAssetRequest extends ImageUploadRequest
+{
+    /**
+     * Determine if the user is authorized to make this request.
+     *
+     * @return bool
+     */
+    public function authorize()
+    {
+        return Gate::allows('update', $this->asset);
+    }
+
+    /**
+     * Get the validation rules that apply to the request.
+     *
+     * @return array
+     */
+    public function rules()
+    {
+        $rules = array_merge(
+            parent::rules(),
+            (new Asset)->getRules(),
+            // this is to overwrite rulesets that include required, and rewrite unique_undeleted
+            [
+                'model_id'  => ['integer', 'exists:models,id,deleted_at,NULL', 'not_array'],
+                'status_id' => ['integer', 'exists:status_labels,id'],
+                'asset_tag' => [
+                    'min:1', 'max:255', 'not_array',
+                    Rule::unique('assets', 'asset_tag')->ignore($this->asset)->withoutTrashed()
+                ],
+            ],
+        );
+
+        return $rules;
+    }
+}

app/Models/Asset.php

@@ -97,35 +97,38 @@ public function declinedCheckout(User $declinedBy, $signature)
     ];
 
     protected $rules = [
-        'model_id'         => 'required|integer|exists:models,id,deleted_at,NULL|not_array',
-        'status_id'        => 'required|integer|exists:status_labels,id',
-        'asset_tag'        => 'required|min:1|max:255|unique_undeleted:assets,asset_tag|not_array',
-        'name'             => 'nullable|max:255',
-        'company_id'       => 'nullable|integer|exists:companies,id',
-        'warranty_months'  => 'nullable|numeric|digits_between:0,240',
-        'last_checkout'    => 'nullable|date_format:Y-m-d H:i:s',
-        'last_checkin'     => 'nullable|date_format:Y-m-d H:i:s',
-        'expected_checkin' => 'nullable|date',
-        'last_audit_date'  => 'nullable|date_format:Y-m-d H:i:s',
-        // 'next_audit_date'  => 'nullable|date|after:last_audit_date',
-        'next_audit_date'  => 'nullable|date',
-        'location_id'      => 'nullable|exists:locations,id',
-        'rtd_location_id'  => 'nullable|exists:locations,id',
-        'purchase_date'    => 'nullable|date|date_format:Y-m-d',
-        'serial'           => 'nullable|unique_undeleted:assets,serial',
-        'purchase_cost'    => 'nullable|numeric|gte:0',
-        'supplier_id'      => 'nullable|exists:suppliers,id',
-        'asset_eol_date'   => 'nullable|date',
-        'eol_explicit'     => 'nullable|boolean',
-        'byod'             => 'nullable|boolean',
-        'order_number'     => 'nullable|string|max:191',
-        'notes'            => 'nullable|string|max:65535',
-        'assigned_to'      => 'nullable|integer',
-        'requestable'      => 'nullable|boolean',
+        'model_id'          => ['required', 'integer', 'exists:models,id,deleted_at,NULL', 'not_array'],
+        'status_id'         => ['required', 'integer', 'exists:status_labels,id'],
+        'asset_tag'         => ['required', 'min:1', 'max:255', 'unique_undeleted:assets,asset_tag', 'not_array'],
+        'name'              => ['nullable', 'max:255'],
+        'company_id'        => ['nullable', 'integer', 'exists:companies,id'],
+        'warranty_months'   => ['nullable', 'numeric', 'digits_between:0,240'],
+        'last_checkout'     => ['nullable', 'date_format:Y-m-d H:i:s'],
+        'last_checkin'      => ['nullable', 'date_format:Y-m-d H:i:s'],
+        'expected_checkin'  => ['nullable', 'date'],
+        'last_audit_date'   => ['nullable', 'date_format:Y-m-d H:i:s'],
+        'next_audit_date'   => ['nullable', 'date'],
+        //'after:last_audit_date'],
+        'location_id'       => ['nullable', 'exists:locations,id'],
+        'rtd_location_id'   => ['nullable', 'exists:locations,id'],
+        'purchase_date'     => ['nullable', 'date', 'date_format:Y-m-d'],
+        'serial'            => ['nullable', 'unique_undeleted:assets,serial'],
+        'purchase_cost'     => ['nullable', 'numeric', 'gte:0'],
+        'supplier_id'       => ['nullable', 'exists:suppliers,id'],
+        'asset_eol_date'    => ['nullable', 'date'],
+        'eol_explicit'      => ['nullable', 'boolean'],
+        'byod'              => ['nullable', 'boolean'],
+        'order_number'      => ['nullable', 'string', 'max:191'],
+        'notes'             => ['nullable', 'string', 'max:65535'],
+        'assigned_to'       => ['nullable', 'integer'],
+        'requestable'       => ['nullable', 'boolean'],
+        'assigned_user'     => ['nullable', 'exists:users,id,deleted_at,NULL'],
+        'assigned_location' => ['nullable', 'exists:locations,id,deleted_at,NULL'],
+        'assigned_asset'    => ['nullable', 'exists:assets,id,deleted_at,NULL']
     ];
 
 
-  /**
+    /**
    * The attributes that are mass assignable.
    *
    * @var array

app/Providers/ValidationServiceProvider.php

@@ -66,7 +66,6 @@ public function boot()
          * `unique_undeleted:table,fieldname` in your rules out of the box
          */
         Validator::extend('unique_undeleted', function ($attribute, $value, $parameters, $validator) {
-
             if (count($parameters)) {
 
                 // This is a bit of a shim, but serial doesn't have any other rules around it other than that it's nullable

database/factories/AssetFactory.php

@@ -48,6 +48,7 @@ public function definition()
             'assigned_type' => null,
             'next_audit_date' => null,
             'last_checkout' => null,
+            'asset_eol_date' => null
         ];
     }
 
@@ -354,6 +355,17 @@ public function nonrequestable()
         return $this->state(['requestable' => false]);
     }
 
+    public function noPurchaseOrEolDate()
+    {
+        return $this->afterCreating(function (Asset $asset) {
+            $asset->update([
+                'purchase_date' => null,
+                'asset_eol_date' => null
+            ]);
+        });
+    }
+
+
     public function hasEncryptedCustomField(CustomField $field = null)
     {
         return $this->state(function () use ($field) {
@@ -372,7 +384,6 @@ public function hasMultipleCustomFields(array $fields = null): self
         });
     }
 
-
     /**
      * This allows bypassing model level validation if you want to purposefully
      * create an asset in an invalid state. Validation is turned back on

database/factories/LocationFactory.php

@@ -25,7 +25,13 @@ public function definition()
             'image' => rand(1, 9).'.jpg',
         ];
     }
-
+
+    // one of these can eventuall go away - left temporarily for conflict resolution
+    public function deleted(): self
+    {
+        return $this->state(['deleted_at' => $this->faker->dateTime()]);
+    }
+
     public function deletedLocation()
     {
         return $this->state(function () {

database/factories/UserFactory.php

@@ -309,4 +309,9 @@ private function appendPermission(array $permission)
             ];
         });
     }
+
+    public function deleted(): self
+    {
+        return $this->state(['deleted_at' => $this->faker->dateTime()]);
+    }
 }

routes/api.php

@@ -1,7 +1,6 @@
 <?php
 
 use App\Http\Controllers\Api;
-// use Illuminate\Http\Request;
 use Illuminate\Support\Facades\Route;
 
 
@@ -571,19 +570,22 @@
               'destroy'
           ]
         )->name('api.assets.files.destroy');
-
       });
 
-        Route::resource('hardware', 
+    // pulling this out of resource route group to begin normalizing for route-model binding.
+    // this would probably keep working with the resource route group, but the general practice is for
+    // the model name to be the parameter - and i think it's a good differentiation in the code while we convert the others.
+    Route::patch('/hardware/{asset}', [Api\AssetsController::class, 'update'])->name('api.assets.update');
+
+    Route::resource('hardware',
         Api\AssetsController::class,
         ['names' => [
                 'index' => 'api.assets.index',
                 'show' => 'api.assets.show',
-                'update' => 'api.assets.update',
                 'store' => 'api.assets.store',
                 'destroy' => 'api.assets.destroy',
             ],
-        'except' => ['create', 'edit'],
+            'except' => ['create', 'edit', 'update'],
         'parameters' => ['asset' => 'asset_id'],
         ]
         ); // end assets API routes