From 1077ff4a3321f350082d12abfc9bcab58bc9e345 Mon Sep 17 00:00:00 2001 From: Martin Hauser Date: Thu, 20 Aug 2026 21:40:51 +0200 Subject: [PATCH] fix(forms): Correct nullable_fields in bulk edit forms `nullable_fields` only marks declared form fields as clearable. Several bulk edit forms referenced missing, stale, duplicate, or non-model fields. This left intended fields unavailable, rendered ineffective Set Null controls, and could trigger a server error for Contact bulk edits. Declare the intended fields, correct the PowerFeed and DataSource entries, and remove invalid Contact group fields. Align owner and comments nullification on forms that do not inherit the common bulk-edit fields, and prevent DataSource comments from rendering twice. Add regression coverage to keep nullable declarations aligned with their form and model fields. Fixes #22990 --- netbox/core/forms/bulk_edit.py | 4 +- netbox/dcim/forms/bulk_edit.py | 16 +++++- netbox/dcim/tests/test_views.py | 3 + netbox/extras/forms/bulk_edit.py | 9 ++- netbox/netbox/forms/bulk_edit.py | 6 +- netbox/netbox/tests/test_base_classes.py | 73 ++++++++++++++++++++++++ netbox/tenancy/forms/bulk_edit.py | 2 +- 7 files changed, 106 insertions(+), 7 deletions(-) diff --git a/netbox/core/forms/bulk_edit.py b/netbox/core/forms/bulk_edit.py index 3111ac268..eb748e716 100644 --- a/netbox/core/forms/bulk_edit.py +++ b/netbox/core/forms/bulk_edit.py @@ -41,8 +41,8 @@ class DataSourceBulkEditForm(PrimaryModelBulkEditForm): model = DataSource fieldsets = ( - FieldSet('type', 'enabled', 'description', 'sync_interval', 'parameters', 'ignore_rules', 'comments'), + FieldSet('type', 'enabled', 'description', 'sync_interval', 'parameters', 'ignore_rules'), ) nullable_fields = ( - 'description', 'description', 'sync_interval', 'parameters', 'parameters', 'ignore_rules' 'comments', + 'description', 'sync_interval', 'parameters', 'ignore_rules', 'comments', ) diff --git a/netbox/dcim/forms/bulk_edit.py b/netbox/dcim/forms/bulk_edit.py index d9ef8600f..7b580af36 100644 --- a/netbox/dcim/forms/bulk_edit.py +++ b/netbox/dcim/forms/bulk_edit.py @@ -989,7 +989,7 @@ class PowerFeedBulkEditForm(PrimaryModelBulkEditForm): FieldSet('power_panel', 'rack', 'status', 'type', 'mark_connected', 'description', 'tenant'), FieldSet('supply', 'phase', 'voltage', 'amperage', 'max_utilization', name=_('Power')) ) - nullable_fields = ('location', 'tenant', 'description', 'comments') + nullable_fields = ('rack', 'tenant', 'description', 'comments') # @@ -1015,6 +1015,10 @@ class ConsolePortTemplateBulkEditForm(ComponentTemplateBulkEditForm): choices=add_blank_choice(ConsolePortTypeChoices), required=False ) + description = forms.CharField( + label=_('Description'), + required=False + ) nullable_fields = ('label', 'type', 'description') @@ -1247,6 +1251,11 @@ class ModuleBayTemplateBulkEditForm(ComponentTemplateBulkEditForm): max_length=64, required=False ) + position = forms.CharField( + label=_('Position'), + max_length=30, + required=False + ) description = forms.CharField( label=_('Description'), required=False @@ -1307,6 +1316,11 @@ class InventoryItemTemplateBulkEditForm(ComponentTemplateBulkEditForm): queryset=Manufacturer.objects.all(), required=False ) + part_id = forms.CharField( + label=_('Part ID'), + max_length=50, + required=False + ) nullable_fields = ('label', 'role', 'manufacturer', 'part_id', 'description') diff --git a/netbox/dcim/tests/test_views.py b/netbox/dcim/tests/test_views.py index 9d50dfdd5..244f61f9f 100644 --- a/netbox/dcim/tests/test_views.py +++ b/netbox/dcim/tests/test_views.py @@ -1763,6 +1763,7 @@ class ConsolePortTemplateTestCase(ViewTestCases.DeviceComponentTemplateViewTestC cls.bulk_edit_data = { 'type': ConsolePortTypeChoices.TYPE_RJ45, + 'description': 'Foo bar', } @@ -2027,6 +2028,7 @@ class ModuleBayTemplateTestCase(ViewTestCases.DeviceComponentTemplateViewTestCas cls.bulk_edit_data = { 'description': 'Foo bar', + 'position': 'A1', } @@ -2108,6 +2110,7 @@ class InventoryItemTemplateTestCase(ViewTestCases.DeviceComponentTemplateViewTes cls.bulk_edit_data = { 'description': 'Foo bar', + 'part_id': 'PN-1', } diff --git a/netbox/extras/forms/bulk_edit.py b/netbox/extras/forms/bulk_edit.py index abfe22f5a..348218f1e 100644 --- a/netbox/extras/forms/bulk_edit.py +++ b/netbox/extras/forms/bulk_edit.py @@ -102,7 +102,7 @@ class CustomFieldBulkEditForm(ChangelogMessageMixin, OwnerMixin, BulkEditForm): name=_('Validation') ), ) - nullable_fields = ('group_name', 'description', 'choice_set', 'validation_schema') + nullable_fields = ('group_name', 'description', 'choice_set', 'validation_schema', 'owner', 'comments') class CustomFieldChoiceSetBulkEditForm(ChangelogMessageMixin, OwnerMixin, BulkEditForm): @@ -300,6 +300,11 @@ class EventRuleBulkEditForm(OwnerMixin, NetBoxModelBulkEditForm): max_length=200, required=False ) + conditions = JSONField( + label=_('Conditions'), + required=False, + help_text=_('Enter conditions in JSON format.') + ) nullable_fields = ('description', 'conditions') @@ -367,7 +372,7 @@ class ConfigContextBulkEditForm(ChangelogMessageMixin, OwnerMixin, BulkEditForm) fieldsets = ( FieldSet('weight', 'profile', 'is_active', 'description'), ) - nullable_fields = ('profile', 'description') + nullable_fields = ('profile', 'description', 'owner') class ConfigTemplateBulkEditForm(ChangelogMessageMixin, OwnerMixin, BulkEditForm): diff --git a/netbox/netbox/forms/bulk_edit.py b/netbox/netbox/forms/bulk_edit.py index 2e4e59982..0098a5133 100644 --- a/netbox/netbox/forms/bulk_edit.py +++ b/netbox/netbox/forms/bulk_edit.py @@ -59,7 +59,11 @@ class NetBoxModelBulkEditForm(ChangelogMessageMixin, CustomFieldsMixin, BulkEdit return customfield.to_form_field(set_initial=False, enforce_required=False) def _extend_nullable_fields(self): - nullable_common_fields = ['owner'] + # The bulk edit template always renders a Set Null control for these + nullable_common_fields = [ + name for name in ('owner', 'comments') + if name in self.fields and name not in self.nullable_fields + ] nullable_custom_fields = [ name for name, customfield in self.custom_fields.items() if (not customfield.required and customfield.ui_editable == CustomFieldUIEditableChoices.YES) diff --git a/netbox/netbox/tests/test_base_classes.py b/netbox/netbox/tests/test_base_classes.py index cc1ab1de5..eb3ea81ee 100644 --- a/netbox/netbox/tests/test_base_classes.py +++ b/netbox/netbox/tests/test_base_classes.py @@ -1,4 +1,5 @@ from django.apps import apps +from django.core.exceptions import FieldDoesNotExist from django.test import TestCase from django.utils.module_loading import import_string @@ -134,6 +135,16 @@ class FormClassesTestCase(TestCase): return NetBoxModelFilterSetForm return None + @classmethod + def get_bulk_edit_form_for_model(cls, model): + """ + Return the bulk edit form class for a given model, or None if it has none. + """ + try: + return cls.get_form_for_model(model, prefix='BulkEdit') + except ImportError: + return None + def test_model_form_base_classes(self): """ Check that each model form inherits from the appropriate base class. @@ -152,6 +163,68 @@ class FormClassesTestCase(TestCase): form_class = self.get_form_for_model(model, prefix='BulkEdit') self.assertTrue(issubclass(form_class, base_class), f"{form_class} does not inherit from {base_class}") + def test_bulk_edit_nullable_fields(self): + """ + Check that every name in a bulk edit form's nullable_fields is a field on the form, and that no + name is listed twice. A name with no matching field is inert: neither the rendered form nor the + update handler acts on it. + """ + for model in apps.get_models(): + if (form_class := self.get_bulk_edit_form_for_model(model)) is None: + continue + # Read the class attribute, which excludes the fields added per instance at runtime + declared = tuple(form_class.nullable_fields) + for name in declared: + self.assertIn( + name, + form_class.base_fields, + f"{form_class.__name__}.nullable_fields lists '{name}', which is not a field on the form", + ) + # The update handler reads model_field.null when nullifying, so a form-only field crashes + try: + model._meta.get_field(name) + except FieldDoesNotExist: + self.fail( + f"{form_class.__name__}.nullable_fields lists '{name}', " + f"which is not a field on {model.__name__}" + ) + duplicates = sorted({name for name in declared if declared.count(name) > 1}) + self.assertEqual( + duplicates, + [], + f"{form_class.__name__}.nullable_fields lists duplicate entries: {duplicates}", + ) + + def test_bulk_edit_hardcoded_nullable_fields(self): + """ + Check that forms which declare fieldsets mark their owner and comments fields as nullable. The + bulk edit template renders a Set Null control for both outside the declared fieldsets, so a form + which omits them offers a control that does nothing. + """ + for model in apps.get_models(): + if (form_class := self.get_bulk_edit_form_for_model(model)) is None: + continue + if not getattr(form_class, 'fieldsets', None): + continue + # Instantiate so that fields added per instance by _extend_nullable_fields() are included + form = form_class({'pk': []}, initial={}) + declared_in_fieldsets = { + item for fieldset in form_class.fieldsets for item in fieldset.items + } + for name in ('owner', 'comments'): + if name not in form.fields: + continue + self.assertIn( + name, + form.nullable_fields, + f"{form_class.__name__} renders a Set Null control for '{name}' without marking it nullable", + ) + self.assertNotIn( + name, + declared_in_fieldsets, + f"{form_class.__name__} lists '{name}' in a fieldset, which renders the field twice", + ) + def test_import_form_base_classes(self): """ Check that each bulk import form inherits from the appropriate base class. diff --git a/netbox/tenancy/forms/bulk_edit.py b/netbox/tenancy/forms/bulk_edit.py index 3565e7d13..5a3ae7963 100644 --- a/netbox/tenancy/forms/bulk_edit.py +++ b/netbox/tenancy/forms/bulk_edit.py @@ -122,7 +122,7 @@ class ContactBulkEditForm(PrimaryModelBulkEditForm): ) nullable_fields = ( - 'add_groups', 'remove_groups', 'title', 'phone', 'email', 'address', 'link', 'description', 'comments' + 'title', 'phone', 'email', 'address', 'link', 'description', 'comments' )