From 24981bcdbfc77c58241149cc9f46c7fcfeb3de74 Mon Sep 17 00:00:00 2001 From: snipe Date: Wed, 12 Aug 2026 12:19:28 +0100 Subject: [PATCH] Asset Impoter: Fixed #19468 - removed requiredness for custom fields --- app/Livewire/Importer.php | 26 +++++++++------------ tests/Feature/Livewire/ImporterTest.php | 30 +++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 15 deletions(-) diff --git a/app/Livewire/Importer.php b/app/Livewire/Importer.php index cd96777369..bc09c83716 100644 --- a/app/Livewire/Importer.php +++ b/app/Livewire/Importer.php @@ -1026,21 +1026,17 @@ class Importer extends Component } } - // Asset imports let users map custom fields on top of the built-in - // ones. A custom field marked required in ANY fieldset should be - // flagged as required at the wizard level - we can't know per-row - // which fieldset each asset will land in, so we treat the union - // across all fieldsets as the safe requirement set. Users see the - // strictest possible bar and can back out if their CSV doesn't - // cover it. - if ($type === 'asset') { - $requiredCustomFields = CustomField::whereHas( - 'fieldset', - fn ($q) => $q->where('custom_field_custom_fieldset.required', 1), - )->get()->map->db_column_name()->all(); - - $required = array_values(array_unique(array_merge($required, $requiredCustomFields))); - } + // Custom fields are intentionally NOT flagged as required at the + // wizard level for asset imports. Required-ness varies per fieldset, + // and a CSV can span multiple asset models pointing at different + // fieldsets, so a field required in Fieldset A may be irrelevant + // for rows destined for Fieldset B (issue #19468). Server-side + // validation on Asset::save() enforces the correct per-asset rule + // via customFieldValidationRules() -> $model->fieldset->validation_rules(), + // which reads the pivot->required flag for the specific fieldset + // attached to the row's model. Rows that legitimately need the + // field will still fail at save-time and surface in the import + // error output. return $required; } diff --git a/tests/Feature/Livewire/ImporterTest.php b/tests/Feature/Livewire/ImporterTest.php index 4ec21a46ac..70cd4eeefa 100644 --- a/tests/Feature/Livewire/ImporterTest.php +++ b/tests/Feature/Livewire/ImporterTest.php @@ -3,6 +3,8 @@ namespace Tests\Feature\Livewire; use App\Livewire\Importer; +use App\Models\CustomField; +use App\Models\CustomFieldset; use App\Models\Import; use App\Models\User; use Illuminate\Support\Facades\Storage; @@ -508,6 +510,34 @@ class ImporterTest extends TestCase }); } + public function test_asset_required_fields_do_not_include_custom_fields_required_in_some_fieldsets_only(): void + { + // Regression for #19468. A CSV of assets can span multiple asset + // models with different fieldsets. A custom field required in one + // fieldset but not in another (or not attached to another) must not + // be flagged as required at the wizard level, because rows destined + // for the other fieldset don't need it. Per-asset server-side + // validation on Asset::save() enforces the correct rule at save-time. + $customField = CustomField::factory()->create(['name' => 'Priority']); + + $laptopFieldset = CustomFieldset::factory()->create(); + $laptopFieldset->fields()->attach($customField, ['required' => 1, 'order' => 1]); + + // Second fieldset that does NOT include the custom field at all. + CustomFieldset::factory()->create(); + + Livewire::actingAs(User::factory()->canImport()->create()) + ->test(Importer::class) + ->tap(function ($c) use ($customField) { + $required = $c->instance()->requiredForType('asset'); + $this->assertNotContains( + $customField->db_column_name(), + $required, + 'Custom field required in only some fieldsets should not be flagged as required at the wizard level.', + ); + }); + } + public function test_next_step_auto_maps_fields_even_when_type_was_preselected(): void { // Regression: updatingTypeOfImport only fires on wire-model changes.