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
This commit is contained in:
parent
eab6b42659
commit
1077ff4a33
|
|
@ -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',
|
||||
)
|
||||
|
|
|
|||
|
|
@ -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')
|
||||
|
||||
|
|
|
|||
|
|
@ -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',
|
||||
}
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -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 <a href="https://json.org/">JSON</a> 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):
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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'
|
||||
)
|
||||
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue