Browse Source

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
Martin Hauser 1 day ago
parent
commit
b8a540bf0f

+ 2 - 2
netbox/core/forms/bulk_edit.py

@@ -41,8 +41,8 @@ class DataSourceBulkEditForm(PrimaryModelBulkEditForm):
 
 
     model = DataSource
     model = DataSource
     fieldsets = (
     fieldsets = (
-        FieldSet('type', 'enabled', 'description', 'sync_interval', 'parameters', 'ignore_rules', 'comments'),
+        FieldSet('type', 'enabled', 'description', 'sync_interval', 'parameters', 'ignore_rules'),
     )
     )
     nullable_fields = (
     nullable_fields = (
-        'description', 'description', 'sync_interval', 'parameters', 'parameters', 'ignore_rules' 'comments',
+        'description', 'sync_interval', 'parameters', 'ignore_rules', 'comments',
     )
     )

+ 15 - 1
netbox/dcim/forms/bulk_edit.py

@@ -989,7 +989,7 @@ class PowerFeedBulkEditForm(PrimaryModelBulkEditForm):
         FieldSet('power_panel', 'rack', 'status', 'type', 'mark_connected', 'description', 'tenant'),
         FieldSet('power_panel', 'rack', 'status', 'type', 'mark_connected', 'description', 'tenant'),
         FieldSet('supply', 'phase', 'voltage', 'amperage', 'max_utilization', name=_('Power'))
         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),
         choices=add_blank_choice(ConsolePortTypeChoices),
         required=False
         required=False
     )
     )
+    description = forms.CharField(
+        label=_('Description'),
+        required=False
+    )
 
 
     nullable_fields = ('label', 'type', 'description')
     nullable_fields = ('label', 'type', 'description')
 
 
@@ -1247,6 +1251,11 @@ class ModuleBayTemplateBulkEditForm(ComponentTemplateBulkEditForm):
         max_length=64,
         max_length=64,
         required=False
         required=False
     )
     )
+    position = forms.CharField(
+        label=_('Position'),
+        max_length=30,
+        required=False
+    )
     description = forms.CharField(
     description = forms.CharField(
         label=_('Description'),
         label=_('Description'),
         required=False
         required=False
@@ -1307,6 +1316,11 @@ class InventoryItemTemplateBulkEditForm(ComponentTemplateBulkEditForm):
         queryset=Manufacturer.objects.all(),
         queryset=Manufacturer.objects.all(),
         required=False
         required=False
     )
     )
+    part_id = forms.CharField(
+        label=_('Part ID'),
+        max_length=50,
+        required=False
+    )
 
 
     nullable_fields = ('label', 'role', 'manufacturer', 'part_id', 'description')
     nullable_fields = ('label', 'role', 'manufacturer', 'part_id', 'description')
 
 

+ 3 - 0
netbox/dcim/tests/test_views.py

@@ -1763,6 +1763,7 @@ class ConsolePortTemplateTestCase(ViewTestCases.DeviceComponentTemplateViewTestC
 
 
         cls.bulk_edit_data = {
         cls.bulk_edit_data = {
             'type': ConsolePortTypeChoices.TYPE_RJ45,
             'type': ConsolePortTypeChoices.TYPE_RJ45,
+            'description': 'Foo bar',
         }
         }
 
 
 
 
@@ -2027,6 +2028,7 @@ class ModuleBayTemplateTestCase(ViewTestCases.DeviceComponentTemplateViewTestCas
 
 
         cls.bulk_edit_data = {
         cls.bulk_edit_data = {
             'description': 'Foo bar',
             'description': 'Foo bar',
+            'position': 'A1',
         }
         }
 
 
 
 
@@ -2108,6 +2110,7 @@ class InventoryItemTemplateTestCase(ViewTestCases.DeviceComponentTemplateViewTes
 
 
         cls.bulk_edit_data = {
         cls.bulk_edit_data = {
             'description': 'Foo bar',
             'description': 'Foo bar',
+            'part_id': 'PN-1',
         }
         }
 
 
 
 

+ 7 - 2
netbox/extras/forms/bulk_edit.py

@@ -102,7 +102,7 @@ class CustomFieldBulkEditForm(ChangelogMessageMixin, OwnerMixin, BulkEditForm):
             name=_('Validation')
             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):
 class CustomFieldChoiceSetBulkEditForm(ChangelogMessageMixin, OwnerMixin, BulkEditForm):
@@ -300,6 +300,11 @@ class EventRuleBulkEditForm(OwnerMixin, NetBoxModelBulkEditForm):
         max_length=200,
         max_length=200,
         required=False
         required=False
     )
     )
+    conditions = JSONField(
+        label=_('Conditions'),
+        required=False,
+        help_text=_('Enter conditions in <a href="https://json.org/">JSON</a> format.')
+    )
 
 
     nullable_fields = ('description', 'conditions')
     nullable_fields = ('description', 'conditions')
 
 
@@ -367,7 +372,7 @@ class ConfigContextBulkEditForm(ChangelogMessageMixin, OwnerMixin, BulkEditForm)
     fieldsets = (
     fieldsets = (
         FieldSet('weight', 'profile', 'is_active', 'description'),
         FieldSet('weight', 'profile', 'is_active', 'description'),
     )
     )
-    nullable_fields = ('profile', 'description')
+    nullable_fields = ('profile', 'description', 'owner')
 
 
 
 
 class ConfigTemplateBulkEditForm(ChangelogMessageMixin, OwnerMixin, BulkEditForm):
 class ConfigTemplateBulkEditForm(ChangelogMessageMixin, OwnerMixin, BulkEditForm):

+ 5 - 1
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)
         return customfield.to_form_field(set_initial=False, enforce_required=False)
 
 
     def _extend_nullable_fields(self):
     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 = [
         nullable_custom_fields = [
             name for name, customfield in self.custom_fields.items()
             name for name, customfield in self.custom_fields.items()
             if (not customfield.required and customfield.ui_editable == CustomFieldUIEditableChoices.YES)
             if (not customfield.required and customfield.ui_editable == CustomFieldUIEditableChoices.YES)

+ 73 - 0
netbox/netbox/tests/test_base_classes.py

@@ -1,4 +1,5 @@
 from django.apps import apps
 from django.apps import apps
+from django.core.exceptions import FieldDoesNotExist
 from django.test import TestCase
 from django.test import TestCase
 from django.utils.module_loading import import_string
 from django.utils.module_loading import import_string
 
 
@@ -134,6 +135,16 @@ class FormClassesTestCase(TestCase):
             return NetBoxModelFilterSetForm
             return NetBoxModelFilterSetForm
         return None
         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):
     def test_model_form_base_classes(self):
         """
         """
         Check that each model form inherits from the appropriate base class.
         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')
                 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}")
                 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):
     def test_import_form_base_classes(self):
         """
         """
         Check that each bulk import form inherits from the appropriate base class.
         Check that each bulk import form inherits from the appropriate base class.

+ 1 - 1
netbox/tenancy/forms/bulk_edit.py

@@ -122,7 +122,7 @@ class ContactBulkEditForm(PrimaryModelBulkEditForm):
     )
     )
 
 
     nullable_fields = (
     nullable_fields = (
-        'add_groups', 'remove_groups', 'title', 'phone', 'email', 'address', 'link', 'description', 'comments'
+        'title', 'phone', 'email', 'address', 'link', 'description', 'comments'
     )
     )