3
0
mirror of https://github.com/snipe/snipe-it.git synced 2026-08-18 11:15:42 +00:00

Asset Impoter: Fixed #19468 - removed requiredness for custom fields

This commit is contained in:
snipe
2026-08-12 12:19:28 +01:00
parent 34e60c64e3
commit 24981bcdbf
2 changed files with 41 additions and 15 deletions

View File

@ -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;
}

View File

@ -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.