Przeglądaj źródła

fix(circuits): Require termination object for selected type

Require a Circuit Termination target when a termination type has been
selected, so blank dynamic target fields surface an inline form error
instead of falling through to generic GFK validation for termination_id.

Add a model-level guard for the same invalid state before generic
GenericForeignKey validation runs.

Fixes #22163
Martin Hauser 1 dzień temu
rodzic
commit
d25e2d43d0

+ 16 - 6
netbox/circuits/forms/model_forms.py

@@ -1,6 +1,6 @@
 from django import forms
 from django import forms
 from django.contrib.contenttypes.models import ContentType
 from django.contrib.contenttypes.models import ContentType
-from django.core.exceptions import ObjectDoesNotExist
+from django.core.exceptions import ObjectDoesNotExist, ValidationError
 from django.utils.translation import gettext_lazy as _
 from django.utils.translation import gettext_lazy as _
 
 
 from circuits.choices import (
 from circuits.choices import (
@@ -24,7 +24,7 @@ from utilities.forms.fields import (
 from utilities.forms.mixins import DistanceValidationMixin
 from utilities.forms.mixins import DistanceValidationMixin
 from utilities.forms.rendering import FieldSet, InlineFields, M2MAddRemoveFields
 from utilities.forms.rendering import FieldSet, InlineFields, M2MAddRemoveFields
 from utilities.forms.widgets import DatePicker, HTMXSelect, NumberWithOptions
 from utilities.forms.widgets import DatePicker, HTMXSelect, NumberWithOptions
-from utilities.templatetags.builtins.filters import bettertitle
+from utilities.string import title
 
 
 __all__ = (
 __all__ = (
     'CircuitForm',
     'CircuitForm',
@@ -195,13 +195,11 @@ class CircuitTerminationForm(NetBoxModelForm):
     termination_type = ContentTypeChoiceField(
     termination_type = ContentTypeChoiceField(
         queryset=ContentType.objects.filter(model__in=CIRCUIT_TERMINATION_TERMINATION_TYPES),
         queryset=ContentType.objects.filter(model__in=CIRCUIT_TERMINATION_TERMINATION_TYPES),
         widget=HTMXSelect(),
         widget=HTMXSelect(),
-        required=False,
         label=_('Termination type')
         label=_('Termination type')
     )
     )
     termination = DynamicModelChoiceField(
     termination = DynamicModelChoiceField(
         label=_('Termination'),
         label=_('Termination'),
         queryset=Site.objects.none(),  # Initial queryset
         queryset=Site.objects.none(),  # Initial queryset
-        required=False,
         disabled=True,
         disabled=True,
         selector=True
         selector=True
     )
     )
@@ -247,16 +245,28 @@ class CircuitTerminationForm(NetBoxModelForm):
                 self.fields['termination'].queryset = model.objects.all()
                 self.fields['termination'].queryset = model.objects.all()
                 self.fields['termination'].widget.attrs['selector'] = model._meta.label_lower
                 self.fields['termination'].widget.attrs['selector'] = model._meta.label_lower
                 self.fields['termination'].disabled = False
                 self.fields['termination'].disabled = False
-                self.fields['termination'].label = _(bettertitle(model._meta.verbose_name))
+                self.fields['termination'].label = _(title(model._meta.verbose_name))
             except ObjectDoesNotExist:
             except ObjectDoesNotExist:
                 pass
                 pass
 
 
             if self.instance and termination_type_id != self.instance.termination_type_id:
             if self.instance and termination_type_id != self.instance.termination_type_id:
                 self.initial['termination'] = None
                 self.initial['termination'] = None
+        else:
+            # Clear the initial termination value if termination_type is not set
+            self.initial['termination'] = None
 
 
     def clean(self):
     def clean(self):
         super().clean()
         super().clean()
 
 
+        termination = self.cleaned_data.get('termination')
+        termination_type = self.cleaned_data.get('termination_type')
+        if termination_type and not termination:
+            raise ValidationError({
+                'termination': _('Please select a {termination_type}.').format(
+                    termination_type=_(title(termination_type.model_class()._meta.verbose_name))
+                )
+            })
+
         # Assign the selected termination (if any)
         # Assign the selected termination (if any)
         self.instance.termination = self.cleaned_data.get('termination')
         self.instance.termination = self.cleaned_data.get('termination')
 
 
@@ -319,7 +329,7 @@ class CircuitGroupAssignmentForm(NetBoxModelForm):
                 self.fields['member'].queryset = model.objects.all()
                 self.fields['member'].queryset = model.objects.all()
                 self.fields['member'].widget.attrs['selector'] = model._meta.label_lower
                 self.fields['member'].widget.attrs['selector'] = model._meta.label_lower
                 self.fields['member'].disabled = False
                 self.fields['member'].disabled = False
-                self.fields['member'].label = _(bettertitle(model._meta.verbose_name))
+                self.fields['member'].label = _(title(model._meta.verbose_name))
             except ObjectDoesNotExist:
             except ObjectDoesNotExist:
                 pass
                 pass
 
 

+ 8 - 0
netbox/circuits/models/circuits.py

@@ -17,6 +17,7 @@ from netbox.models.features import (
     TagsMixin,
     TagsMixin,
 )
 )
 from netbox.models.mixins import DistanceMixin
 from netbox.models.mixins import DistanceMixin
+from utilities.string import title
 
 
 from .base import BaseCircuitType
 from .base import BaseCircuitType
 
 
@@ -367,6 +368,13 @@ class CircuitTermination(
         return reverse('circuits:circuittermination', args=[self.pk])
         return reverse('circuits:circuittermination', args=[self.pk])
 
 
     def clean(self):
     def clean(self):
+        if self.termination_type and not (self.termination or self.termination_id):
+            termination_type = self.termination_type.model_class()
+            raise ValidationError(
+                _("Please select a {termination_type}.").format(
+                    termination_type=_(title(termination_type._meta.verbose_name))
+                )
+            )
         super().clean()
         super().clean()
 
 
         if self.termination is None:
         if self.termination is None:

+ 43 - 0
netbox/circuits/tests/test_forms.py

@@ -0,0 +1,43 @@
+from django.contrib.contenttypes.models import ContentType
+from django.test import TestCase
+
+from circuits.forms import CircuitTerminationForm
+from circuits.models import Circuit, CircuitType, Provider, ProviderNetwork
+
+
+class CircuitTerminationFormTestCase(TestCase):
+    @classmethod
+    def setUpTestData(cls):
+        provider = Provider.objects.create(name='Provider 1', slug='provider-1')
+        circuit_type = CircuitType.objects.create(name='Circuit Type 1', slug='circuit-type-1')
+
+        cls.circuit = Circuit.objects.create(
+            cid='Circuit 1',
+            provider=provider,
+            type=circuit_type,
+        )
+        cls.provider_network = ProviderNetwork.objects.create(
+            name='Provider Network 1',
+            provider=provider,
+        )
+
+    def test_termination_required_when_termination_type_is_selected(self):
+        """
+        Selecting a termination type without a target object should report a
+        validation error against the visible form field.
+        """
+        provider_network_type = ContentType.objects.get_for_model(ProviderNetwork)
+
+        form = CircuitTerminationForm(
+            data={
+                'circuit': self.circuit.pk,
+                'term_side': 'A',
+                'termination_type': provider_network_type.pk,
+                'termination': '',
+            }
+        )
+
+        self.assertFalse(form.is_valid())
+        self.assertIn('termination', form.errors)
+        self.assertIn('Please select a Provider Network.', form.errors['termination'])
+        self.assertNotIn('termination_id', form.errors)

+ 20 - 0
netbox/circuits/tests/test_models.py

@@ -1,3 +1,5 @@
+from django.contrib.contenttypes.models import ContentType
+from django.core.exceptions import NON_FIELD_ERRORS, ValidationError
 from django.test import TestCase
 from django.test import TestCase
 
 
 from circuits.models import Circuit, CircuitTermination, CircuitType, Provider, ProviderNetwork
 from circuits.models import Circuit, CircuitTermination, CircuitType, Provider, ProviderNetwork
@@ -146,3 +148,21 @@ class CircuitTerminationTestCase(TestCase):
 
 
         # Cache should be cleared (SET_NULL behavior)
         # Cache should be cleared (SET_NULL behavior)
         self.assertIsNone(self.circuits[0].termination_a)
         self.assertIsNone(self.circuits[0].termination_a)
+
+    def test_termination_required_when_termination_type_is_selected(self):
+        """Model rejects type-without-target before generic GFK validation hits termination_id."""
+        provider_network_type = ContentType.objects.get_for_model(ProviderNetwork)
+
+        termination = CircuitTermination(
+            circuit=self.circuits[0],
+            term_side='A',
+            termination_type=provider_network_type,
+        )
+
+        with self.assertRaises(ValidationError) as cm:
+            termination.full_clean()
+
+        errors = cm.exception.message_dict
+        self.assertIn(NON_FIELD_ERRORS, errors)
+        self.assertIn('Please select a Provider Network.', errors[NON_FIELD_ERRORS])
+        self.assertNotIn('termination_id', errors)