Merge pull request #15377 from marcusmoore/fixes/custom-field-values-on-validation-error

Fixed custom field defaults being prematurely updated
This commit is contained in:
snipe 2024-08-23 07:20:23 +01:00 committed by GitHub
commit 78d355f136
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 81 additions and 13 deletions

View file

@ -151,17 +151,17 @@ class AssetModelsController extends Controller
$model->notes = $request->input('notes'); $model->notes = $request->input('notes');
$model->requestable = $request->input('requestable', '0'); $model->requestable = $request->input('requestable', '0');
$this->removeCustomFieldsDefaultValues($model);
$model->fieldset_id = $request->input('fieldset_id'); $model->fieldset_id = $request->input('fieldset_id');
if ($model->save()) {
$this->removeCustomFieldsDefaultValues($model);
if ($this->shouldAddDefaultValues($request->input())) { if ($this->shouldAddDefaultValues($request->input())) {
if (!$this->assignCustomFieldsDefaultValues($model, $request->input('default_values'))) { if (!$this->assignCustomFieldsDefaultValues($model, $request->input('default_values'))) {
return redirect()->back()->withInput()->with('error', trans('admin/custom_fields/message.fieldset_default_value.error')); return redirect()->back()->withInput()->with('error', trans('admin/custom_fields/message.fieldset_default_value.error'));
} }
} }
if ($model->save()) {
if ($model->wasChanged('eol')) { if ($model->wasChanged('eol')) {
if ($model->eol > 0) { if ($model->eol > 0) {
$newEol = $model->eol; $newEol = $model->eol;

View file

@ -4,6 +4,8 @@ namespace Tests\Feature\AssetModels\Ui;
use App\Models\AssetModel; use App\Models\AssetModel;
use App\Models\Category; use App\Models\Category;
use App\Models\CustomField;
use App\Models\CustomFieldset;
use App\Models\User; use App\Models\User;
use Tests\TestCase; use Tests\TestCase;
@ -12,11 +14,10 @@ class UpdateAssetModelsTest extends TestCase
public function testPermissionRequiredToStoreAssetModel() public function testPermissionRequiredToStoreAssetModel()
{ {
$this->actingAs(User::factory()->create()) $this->actingAs(User::factory()->create())
->post(route('models.store'), [ ->put(route('models.update', ['model' => AssetModel::factory()->create()]), [
'name' => 'Test Model', 'name' => 'Changed Name',
'category_id' => Category::factory()->create()->id 'category_id' => Category::factory()->create()->id,
]) ])
->assertStatus(403)
->assertForbidden(); ->assertForbidden();
} }
@ -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);
}
} }