diff --git a/app/Http/Controllers/AssetModelsController.php b/app/Http/Controllers/AssetModelsController.php index b6bb34738..60b1a3980 100755 --- a/app/Http/Controllers/AssetModelsController.php +++ b/app/Http/Controllers/AssetModelsController.php @@ -151,17 +151,17 @@ class AssetModelsController extends Controller $model->notes = $request->input('notes'); $model->requestable = $request->input('requestable', '0'); - $this->removeCustomFieldsDefaultValues($model); - $model->fieldset_id = $request->input('fieldset_id'); - if ($this->shouldAddDefaultValues($request->input())) { - if (!$this->assignCustomFieldsDefaultValues($model, $request->input('default_values'))){ - return redirect()->back()->withInput()->with('error', trans('admin/custom_fields/message.fieldset_default_value.error')); - } - } - if ($model->save()) { + $this->removeCustomFieldsDefaultValues($model); + + if ($this->shouldAddDefaultValues($request->input())) { + if (!$this->assignCustomFieldsDefaultValues($model, $request->input('default_values'))) { + return redirect()->back()->withInput()->with('error', trans('admin/custom_fields/message.fieldset_default_value.error')); + } + } + if ($model->wasChanged('eol')) { if ($model->eol > 0) { $newEol = $model->eol; diff --git a/tests/Feature/AssetModels/Ui/UpdateAssetModelsTest.php b/tests/Feature/AssetModels/Ui/UpdateAssetModelsTest.php index 423eaad57..95eb592b6 100644 --- a/tests/Feature/AssetModels/Ui/UpdateAssetModelsTest.php +++ b/tests/Feature/AssetModels/Ui/UpdateAssetModelsTest.php @@ -4,6 +4,8 @@ namespace Tests\Feature\AssetModels\Ui; use App\Models\AssetModel; use App\Models\Category; +use App\Models\CustomField; +use App\Models\CustomFieldset; use App\Models\User; use Tests\TestCase; @@ -12,14 +14,13 @@ class UpdateAssetModelsTest extends TestCase public function testPermissionRequiredToStoreAssetModel() { $this->actingAs(User::factory()->create()) - ->post(route('models.store'), [ - 'name' => 'Test Model', - 'category_id' => Category::factory()->create()->id + ->put(route('models.update', ['model' => AssetModel::factory()->create()]), [ + 'name' => 'Changed Name', + 'category_id' => Category::factory()->create()->id, ]) - ->assertStatus(403) ->assertForbidden(); } - + public function testUserCanEditAssetModels() { $category = Category::factory()->forAssets()->create(); @@ -62,4 +63,71 @@ class UpdateAssetModelsTest extends TestCase } + public function test_default_values_remain_unchanged_after_validation_error_occurs() + { + $this->markIncompleteIfMySQL('Custom Field Tests do not work in MySQL'); + + $assetModel = AssetModel::factory()->create(); + $customFieldset = CustomFieldset::factory()->create(); + [$customFieldOne, $customFieldTwo] = CustomField::factory()->count(2)->create(); + + $customFieldset->fields()->attach($customFieldOne, ['order' => 1, 'required' => false]); + $customFieldset->fields()->attach($customFieldTwo, ['order' => 2, 'required' => false]); + + $assetModel->fieldset()->associate($customFieldset); + + $assetModel->defaultValues()->attach($customFieldOne, ['default_value' => 'first default value']); + $assetModel->defaultValues()->attach($customFieldTwo, ['default_value' => 'second default value']); + + $this->actingAs(User::factory()->superuser()->create()) + ->put(route('models.update', ['model' => $assetModel]), [ + // should trigger validation error without name, etc, and NOT remove or change default values + 'add_default_values' => '1', + 'fieldset_id' => $customFieldset->id, + 'default_values' => [ + $customFieldOne->id => 'first changed value', + $customFieldTwo->id => 'second changed value', + ], + ]); + + $potentiallyChangedDefaultValues = $assetModel->defaultValues->pluck('pivot.default_value'); + $this->assertCount(2, $potentiallyChangedDefaultValues); + $this->assertContains('first default value', $potentiallyChangedDefaultValues); + $this->assertContains('second default value', $potentiallyChangedDefaultValues); + } + + public function test_default_values_can_be_updated() + { + $this->markIncompleteIfMySQL('Custom Field Tests do not work in MySQL'); + + $assetModel = AssetModel::factory()->create(); + $customFieldset = CustomFieldset::factory()->create(); + [$customFieldOne, $customFieldTwo] = CustomField::factory()->count(2)->create(); + + $customFieldset->fields()->attach($customFieldOne, ['order' => 1, 'required' => false]); + $customFieldset->fields()->attach($customFieldTwo, ['order' => 2, 'required' => false]); + + $assetModel->fieldset()->associate($customFieldset); + + $assetModel->defaultValues()->attach($customFieldOne, ['default_value' => 'first default value']); + $assetModel->defaultValues()->attach($customFieldTwo, ['default_value' => 'second default value']); + + $this->actingAs(User::factory()->superuser()->create()) + ->put(route('models.update', ['model' => $assetModel]), [ + // should trigger validation error without name, etc, and NOT remove or change default values + 'name' => 'Test Model Edited', + 'category_id' => $assetModel->category_id, + 'add_default_values' => '1', + 'fieldset_id' => $customFieldset->id, + 'default_values' => [ + $customFieldOne->id => 'first changed value', + $customFieldTwo->id => 'second changed value', + ], + ]); + + $potentiallyChangedDefaultValues = $assetModel->defaultValues->pluck('pivot.default_value'); + $this->assertCount(2, $potentiallyChangedDefaultValues); + $this->assertContains('first changed value', $potentiallyChangedDefaultValues); + $this->assertContains('second changed value', $potentiallyChangedDefaultValues); + } }