From 8684a3efc3da03ce1f8dfc3ab014e5e2e90c89a7 Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Tue, 30 Jul 2024 21:22:46 -0500 Subject: [PATCH 1/5] delete note, add trait to other request --- app/Http/Requests/Traits/MayContainCustomFields.php | 1 - app/Http/Requests/UpdateAssetRequest.php | 2 ++ 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/app/Http/Requests/Traits/MayContainCustomFields.php b/app/Http/Requests/Traits/MayContainCustomFields.php index 50b239b60eca..1dd25dbe0369 100644 --- a/app/Http/Requests/Traits/MayContainCustomFields.php +++ b/app/Http/Requests/Traits/MayContainCustomFields.php @@ -15,7 +15,6 @@ public function withValidator($validator) $asset_model = AssetModel::find($this->model_id); } if ($this->method() == 'PATCH' || $this->method() == 'PUT') { - // this is dependent on the asset update request PR $asset_model = $this->asset; } // collect the custom fields in the request diff --git a/app/Http/Requests/UpdateAssetRequest.php b/app/Http/Requests/UpdateAssetRequest.php index cc23a65c4074..a749e5816bd6 100644 --- a/app/Http/Requests/UpdateAssetRequest.php +++ b/app/Http/Requests/UpdateAssetRequest.php @@ -2,12 +2,14 @@ namespace App\Http\Requests; +use App\Http\Requests\Traits\MayContainCustomFields; use App\Models\Asset; use Illuminate\Support\Facades\Gate; use Illuminate\Validation\Rule; class UpdateAssetRequest extends ImageUploadRequest { + use MayContainCustomFields; /** * Determine if the user is authorized to make this request. * From 9b80843c7759bd145ca2f516499af36648607ca4 Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Tue, 30 Jul 2024 21:44:22 -0500 Subject: [PATCH 2/5] tests a little broken, added some nullchecks --- app/Http/Requests/Traits/MayContainCustomFields.php | 2 +- tests/Feature/Assets/Api/UpdateAssetTest.php | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/app/Http/Requests/Traits/MayContainCustomFields.php b/app/Http/Requests/Traits/MayContainCustomFields.php index 1dd25dbe0369..2feada17742e 100644 --- a/app/Http/Requests/Traits/MayContainCustomFields.php +++ b/app/Http/Requests/Traits/MayContainCustomFields.php @@ -24,7 +24,7 @@ public function withValidator($validator) }); // if there are custom fields, find the one's that don't exist on the model's fieldset and add an error to the validator's error bag if (count($request_fields) > 0) { - $request_fields->diff($asset_model->fieldset->fields->pluck('db_column')) + $request_fields->diff($asset_model?->fieldset?->fields?->pluck('db_column')) ->each(function ($request_field_name) use ($request_fields, $validator) { if (CustomField::where('db_column', $request_field_name)->exists()) { $validator->errors()->add($request_field_name, trans('validation.custom.custom_field_not_found_on_model')); diff --git a/tests/Feature/Assets/Api/UpdateAssetTest.php b/tests/Feature/Assets/Api/UpdateAssetTest.php index db5893b4dcf3..2a57d3839176 100644 --- a/tests/Feature/Assets/Api/UpdateAssetTest.php +++ b/tests/Feature/Assets/Api/UpdateAssetTest.php @@ -269,6 +269,8 @@ public function testEncryptedCustomFieldCanBeUpdated() { $this->markIncompleteIfMySQL('Custom Fields tests do not work on MySQL'); + // hmm, for some reason this customfield isn't attached to a fieldset + // need to check out these factory methods... $field = CustomField::factory()->testEncrypted()->create(); $asset = Asset::factory()->hasEncryptedCustomField($field)->create(); $superuser = User::factory()->superuser()->create(); From 0941c0944a68f1043b43325f47cd51bd88fa6111 Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Tue, 30 Jul 2024 22:55:15 -0500 Subject: [PATCH 3/5] ok, found issue, but need to test some things now... --- app/Http/Requests/Traits/MayContainCustomFields.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/Http/Requests/Traits/MayContainCustomFields.php b/app/Http/Requests/Traits/MayContainCustomFields.php index 2feada17742e..9a7f85e3a268 100644 --- a/app/Http/Requests/Traits/MayContainCustomFields.php +++ b/app/Http/Requests/Traits/MayContainCustomFields.php @@ -15,7 +15,7 @@ public function withValidator($validator) $asset_model = AssetModel::find($this->model_id); } if ($this->method() == 'PATCH' || $this->method() == 'PUT') { - $asset_model = $this->asset; + $asset_model = $this->asset->model; } // collect the custom fields in the request $validator->after(function ($validator) use ($asset_model) { From b0063b1d4a0af2f78c3b7c4334256a89cfa661b0 Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Wed, 31 Jul 2024 11:57:35 -0500 Subject: [PATCH 4/5] test written --- tests/Feature/Assets/Api/UpdateAssetTest.php | 25 ++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/tests/Feature/Assets/Api/UpdateAssetTest.php b/tests/Feature/Assets/Api/UpdateAssetTest.php index 2a57d3839176..499ea61f28b9 100644 --- a/tests/Feature/Assets/Api/UpdateAssetTest.php +++ b/tests/Feature/Assets/Api/UpdateAssetTest.php @@ -269,8 +269,6 @@ public function testEncryptedCustomFieldCanBeUpdated() { $this->markIncompleteIfMySQL('Custom Fields tests do not work on MySQL'); - // hmm, for some reason this customfield isn't attached to a fieldset - // need to check out these factory methods... $field = CustomField::factory()->testEncrypted()->create(); $asset = Asset::factory()->hasEncryptedCustomField($field)->create(); $superuser = User::factory()->superuser()->create(); @@ -456,4 +454,27 @@ public function testAssetCannotBeUpdatedByUserInSeparateCompany() ]) ->assertStatusMessageIs('success'); } + + public function testCustomFieldCannotBeUpdatedIfNotOnCurrentAssetModel() + { + $customField = CustomField::factory()->create(); + $customField2 = CustomField::factory()->create(); + $asset = Asset::factory()->hasMultipleCustomFields([$customField])->create(); + $user = User::factory()->editAssets()->create(); + + // successful + $this->actingAsForApi($user)->patchJson(route('api.assets.update', $asset->id), [ + $customField->db_column_name() => 'test attribute', + ])->assertStatusMessageIs('success'); + + // custom field exists, but not on this asset model + $this->actingAsForApi($user)->patchJson(route('api.assets.update', $asset->id), [ + $customField2->db_column_name() => 'test attribute', + ])->assertStatusMessageIs('error'); + + // custom field does not exist + $this->actingAsForApi($user)->patchJson(route('api.assets.update', $asset->id), [ + '_snipeit_non_existent_custom_field_50' => 'test attribute', + ])->assertStatusMessageIs('error'); + } } From 61312c2eecf92951e63ea76a474b12e9b0ff2de5 Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Wed, 31 Jul 2024 12:07:50 -0500 Subject: [PATCH 5/5] oops, mysql --- tests/Feature/Assets/Api/UpdateAssetTest.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/Feature/Assets/Api/UpdateAssetTest.php b/tests/Feature/Assets/Api/UpdateAssetTest.php index 499ea61f28b9..300d06ae50d1 100644 --- a/tests/Feature/Assets/Api/UpdateAssetTest.php +++ b/tests/Feature/Assets/Api/UpdateAssetTest.php @@ -457,6 +457,8 @@ public function testAssetCannotBeUpdatedByUserInSeparateCompany() public function testCustomFieldCannotBeUpdatedIfNotOnCurrentAssetModel() { + $this->markIncompleteIfMySQL('Custom Field Tests do not work in MySQL'); + $customField = CustomField::factory()->create(); $customField2 = CustomField::factory()->create(); $asset = Asset::factory()->hasMultipleCustomFields([$customField])->create();