some more cleanup + tests

app/Http/Controllers/Api/AssetsController.php

@@ -626,23 +626,22 @@ class AssetsController extends Controller
      *
      * @author [A. Gianotto] [<snipe@snipe.net>]
      * @since [v4.0]
-     * @return \Illuminate\Http\JsonResponse
      */
-    public function update(UpdateAssetRequest $request, Asset $asset)
+    public function update(UpdateAssetRequest $request, Asset $asset): JsonResponse
     {
         $asset->fill($request->validated());
 
-        // TODO: how much of this can go in the validator?
+
-        // this is _always_ filled now, see UpdateAssetRequest
+        if ($request->has('model_id')) {
-        // i'm leaving it like this for now, but when would we ever want model_id to be `null`??
+            $asset->model()->associate(AssetModel::find($request->validated()['model_id']));
-        // it actually breaks at the model validation if it gets to null...
+        }
-        ($request->has('model_id')) ?
+        if ($request->has('company_id')) {
-            $asset->model()->associate(AssetModel::find($request->validated()['model_id'])) : null;
+            $asset->company_id = Company::getIdForCurrentUser($request->validated()['company_id']);
-        ($request->has('company_id')) ?
+        }
-            $asset->company_id = Company::getIdForCurrentUser($request->validated()['company_id']) : null;
+        if ($request->has('rtd_location_id') && !$request->has('location_id')) {
-        // TODO: this seems like bad logic maybe? it means that if you submit an rtd_location_id and a location_id in the same request location_id is overwritten with rtd_location_id. seems wrong.
+            $asset->location_id = $request->validated()['rtd_location_id'];
-        //($request->has('rtd_location_id')) ?
+        }
-        //    $asset->location_id = $request->validated()['rtd_location_id'] : null;
+
 
         /**
          * this is here just legacy reasons. Api\AssetController
@@ -695,8 +694,6 @@ class AssetsController extends Controller
         }
 
         return response()->json(Helper::formatStandardApiResponse('error', null, $asset->getErrors()), 200);
-
-        // TODO: confirm that everything expects a _200_ model not found exception
     }
 
 

app/Http/Requests/UpdateAssetRequest.php

@@ -18,17 +18,6 @@ class UpdateAssetRequest extends ImageUploadRequest
         return Gate::allows('update', new Asset);
     }
 
-    public function prepareForValidation()
-    {
-        // the following are 'required' attributes that may or may not be present on an patch request
-        // so supplying them here instead of doing funky array modification to the rules
-        return $this->merge([
-            //'asset_tag' => $this->asset_tag ?? $this->asset->asset_tag,
-            //'model_id' => $this->model_id ?? $this->asset->model_id,
-            //'status_id' => $this->status_id ?? $this->asset->status_id,
-        ]);
-    }
-
     /**
      * Get the validation rules that apply to the request.
      *

tests/Feature/Api/Assets/AssetUpdateTest.php

@@ -9,8 +9,6 @@ use App\Models\Location;
 use App\Models\Statuslabel;
 use App\Models\Supplier;
 use App\Models\User;
-use Carbon\Carbon;
-use Illuminate\Testing\Fluent\AssertableJson;
 
 // TODO: DELETE INTERACTSWITHSETTINGS BEFORE FINAL PR
 use Tests\Support\InteractsWithSettings;
@@ -239,4 +237,41 @@ class AssetUpdateTest extends TestCase
             ->assertOk()
             ->assertStatusMessageIs('error');
     }
+
+    public function testIfRtdLocationIdIsSetWithoutLocationIdAssetReturnsToDefault()
+    {
+        $location = Location::factory()->create();
+        $asset = Asset::factory()->laptopMbp()->create([
+            'location_id' => $location->id
+        ]);
+        $rtdLocation = Location::factory()->create();
+
+        $this->actingAsForApi(User::factory()->editAssets()->create())
+            ->patchJson(route('api.assets.update', $asset->id), [
+                'rtd_location_id' => $rtdLocation->id
+            ]);
+
+        $asset->refresh();
+
+        $this->assertTrue($asset->defaultLoc->is($rtdLocation));
+        $this->assertTrue($asset->location->is($rtdLocation));
+    }
+
+    public function testIfLocationAndRtdLocationAreSetLocationIdIsLocation()
+    {
+        $location = Location::factory()->create();
+        $asset = Asset::factory()->laptopMbp()->create();
+        $rtdLocation = Location::factory()->create();
+
+        $this->actingAsForApi(User::factory()->editAssets()->create())
+            ->patchJson(route('api.assets.update', $asset->id), [
+                'rtd_location_id' => $rtdLocation->id,
+                'location_id' => $location->id
+            ]);
+
+        $asset->refresh();
+
+        $this->assertTrue($asset->defaultLoc->is($rtdLocation));
+        $this->assertTrue($asset->location->is($location));
+    }
 }