diff --git a/netbox/dcim/tests/test_views.py b/netbox/dcim/tests/test_views.py index cf5d8ce20..c4f5c80a9 100644 --- a/netbox/dcim/tests/test_views.py +++ b/netbox/dcim/tests/test_views.py @@ -3463,6 +3463,38 @@ class InterfaceTestCase(ViewTestCases.DeviceComponentViewTestCase): self.assertHttpStatus(response, 302) self.assertEqual(Interface.objects.filter(device=device, name__startswith='xe').count(), 37) + @override_settings(EXEMPT_VIEW_PERMISSIONS=['*']) + def test_bulk_import_omitted_field_validation_error(self): + """Surface omitted-field validation errors during bulk updates.""" + device = Device.objects.first() + wireless_interface = Interface.objects.create( + device=device, + name='Wireless-22683', + type=InterfaceTypeChoices.TYPE_80211AC, + rf_channel_width=Decimal('20.0'), + ) + self.add_permissions('dcim.add_interface', 'dcim.change_interface') + csv_data = '\n'.join([ + 'id,type', + f'{wireless_interface.pk},{InterfaceTypeChoices.TYPE_1GE_GBIC}', + ]) + response = self.client.post( + self._get_url('bulk_import'), + data={ + 'data': csv_data, + 'format': ImportFormatChoices.CSV, + 'csv_delimiter': CSVDelimiterChoices.AUTO, + }, + ) + self.assertHttpStatus(response, 200) + self.assertContains( + response, + 'rf_channel_width: Channel width may be set only on wireless interfaces.', + ) + wireless_interface.refresh_from_db() + self.assertEqual(wireless_interface.type, InterfaceTypeChoices.TYPE_80211AC) + self.assertEqual(wireless_interface.rf_channel_width, Decimal('20.0')) + class FrontPortTestCase(ViewTestCases.DeviceComponentViewTestCase): model = FrontPort diff --git a/netbox/netbox/forms/bulk_import.py b/netbox/netbox/forms/bulk_import.py index 6bd50442b..730d28084 100644 --- a/netbox/netbox/forms/bulk_import.py +++ b/netbox/netbox/forms/bulk_import.py @@ -1,4 +1,5 @@ from django import forms +from django.core.exceptions import NON_FIELD_ERRORS, ValidationError from django.db import models from django.utils.translation import gettext_lazy as _ @@ -70,6 +71,31 @@ class NetBoxModelImportForm(CSVModelForm, NetBoxModelForm): return cleaned + def _update_errors(self, errors): + """Convert errors for fields absent from the form to prefixed non-field errors.""" + if hasattr(errors, 'error_dict'): + remapped = [] + passthrough = {} + for field, error_list in errors.error_dict.items(): + if field == NON_FIELD_ERRORS or field in self.fields: + passthrough[field] = error_list + else: + for error in error_list: + message = next(iter(error)) + if error.params: + message = message.replace('%', '%%') + remapped.append(ValidationError( + '{field}: {message}'.format(field=field, message=message), + code=error.code, + params=error.params, + )) + if passthrough: + super()._update_errors(ValidationError(passthrough)) + for error in remapped: + self.add_error(None, error) + else: + super()._update_errors(errors) + class OwnerCSVMixin(forms.Form): owner = CSVModelChoiceField( diff --git a/netbox/netbox/tests/test_forms.py b/netbox/netbox/tests/test_forms.py index 8a01c393a..ecc2b343f 100644 --- a/netbox/netbox/tests/test_forms.py +++ b/netbox/netbox/tests/test_forms.py @@ -1,3 +1,8 @@ +from unittest.mock import patch + +from django.core.exceptions import NON_FIELD_ERRORS +from django.core.exceptions import ValidationError as DjangoValidationError +from django.core.validators import MaxLengthValidator from django.test import TestCase from dcim.choices import InterfaceTypeChoices @@ -5,12 +10,8 @@ from dcim.forms import InterfaceImportForm from dcim.models import Device, DeviceRole, DeviceType, Interface, Manufacturer, Site -class NetBoxModelImportFormCleanTestCase(TestCase): - """ - Test the clean() method of NetBoxModelImportForm to ensure it properly converts - empty strings to None for nullable fields during CSV import. - Uses InterfaceImportForm as the concrete implementation to test. - """ +class NetBoxModelImportFormTestCase(TestCase): + """Test NetBoxModelImportForm.""" @classmethod def setUpTestData(cls): @@ -301,3 +302,99 @@ class NetBoxModelImportFormCleanTestCase(TestCase): ) self.assertTrue(form.is_valid(), f'Form errors: {form.errors}') self.assertIsNone(form.cleaned_data['wwn']) + + def test_missing_field_validation_error_becomes_non_field_error(self): + """Convert validation errors for absent fields to non-field errors.""" + form = InterfaceImportForm( + data={ + 'device': self.device, + 'name': 'Test Interface', + 'type': InterfaceTypeChoices.TYPE_1GE_GBIC, + } + ) + with patch.object( + form.instance, + 'full_clean', + side_effect=DjangoValidationError({'absent_field': ['Field error.']}), + ): + result = form.is_valid() + + self.assertFalse(result) + self.assertIn('absent_field: Field error.', form.non_field_errors()) + + def test_non_field_error_not_overwritten_by_remapped_missing_field_error(self): + """Preserve remapped and existing non-field errors.""" + form = InterfaceImportForm( + data={ + 'device': self.device, + 'name': 'Test Interface Mixed', + 'type': InterfaceTypeChoices.TYPE_1GE_GBIC, + } + ) + self.assertTrue(form.is_valid(), f'Form errors: {form.errors}') + # absent_field appears first to expose the former overwrite bug + form._update_errors(DjangoValidationError({ + 'absent_field': ['Field error.'], + NON_FIELD_ERRORS: ['A general error.'], + })) + non_field_errors = form.non_field_errors() + self.assertIn('A general error.', non_field_errors) + self.assertIn('absent_field: Field error.', non_field_errors) + + def test_remapped_error_preserves_code_and_params(self): + """Preserve the original ValidationError code and params when remapping.""" + form = InterfaceImportForm( + data={ + 'device': self.device, + 'name': 'Test Interface Params', + 'type': InterfaceTypeChoices.TYPE_1GE_GBIC, + } + ) + self.assertTrue(form.is_valid(), f'Form errors: {form.errors}') + form._update_errors(DjangoValidationError({ + 'absent_field': [ + DjangoValidationError( + '%(value)s is not a valid value.', + code='invalid_value', + params={'value': '100%'}, + ), + ], + })) + non_field_errors = form.non_field_errors() + self.assertIn( + 'absent_field: 100% is not a valid value.', + non_field_errors, + ) + error_data = form.errors[NON_FIELD_ERRORS].as_data() + matching = [error for error in error_data if error.code == 'invalid_value'] + self.assertEqual(len(matching), 1) + self.assertEqual(matching[0].params, {'value': '100%'}) + + def test_remapped_error_preserves_pluralized_message(self): + """Preserve pluralization order when remapping an ngettext_lazy message.""" + form = InterfaceImportForm( + data={ + 'device': self.device, + 'name': 'Test Interface Plural', + 'type': InterfaceTypeChoices.TYPE_1GE_GBIC, + } + ) + self.assertTrue(form.is_valid(), f'Form errors: {form.errors}') + form._update_errors(DjangoValidationError({ + 'absent_field': [ + DjangoValidationError( + MaxLengthValidator.message, + code=MaxLengthValidator.code, + params={'limit_value': 20, 'show_value': 25}, + ), + ], + })) + non_field_errors = form.non_field_errors() + self.assertIn( + 'absent_field: Ensure this value has at most 20 characters (it has 25).', + non_field_errors, + ) + error_data = form.errors[NON_FIELD_ERRORS].as_data() + matching = [error for error in error_data if error.code == MaxLengthValidator.code] + self.assertEqual(len(matching), 1) + self.assertEqual(matching[0].params, {'limit_value': 20, 'show_value': 25})