From d25e2d43d0f0451e09e1671af1b7e600addba676 Mon Sep 17 00:00:00 2001 From: Martin Hauser Date: Wed, 20 May 2026 15:47:00 +0200 Subject: [PATCH] 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 --- netbox/circuits/forms/model_forms.py | 22 ++++++++++---- netbox/circuits/models/circuits.py | 8 ++++++ netbox/circuits/tests/test_forms.py | 43 ++++++++++++++++++++++++++++ netbox/circuits/tests/test_models.py | 20 +++++++++++++ 4 files changed, 87 insertions(+), 6 deletions(-) create mode 100644 netbox/circuits/tests/test_forms.py diff --git a/netbox/circuits/forms/model_forms.py b/netbox/circuits/forms/model_forms.py index 446ec0eed..94e43f0ce 100644 --- a/netbox/circuits/forms/model_forms.py +++ b/netbox/circuits/forms/model_forms.py @@ -1,6 +1,6 @@ from django import forms 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 circuits.choices import ( @@ -24,7 +24,7 @@ from utilities.forms.fields import ( from utilities.forms.mixins import DistanceValidationMixin from utilities.forms.rendering import FieldSet, InlineFields, M2MAddRemoveFields from utilities.forms.widgets import DatePicker, HTMXSelect, NumberWithOptions -from utilities.templatetags.builtins.filters import bettertitle +from utilities.string import title __all__ = ( 'CircuitForm', @@ -195,13 +195,11 @@ class CircuitTerminationForm(NetBoxModelForm): termination_type = ContentTypeChoiceField( queryset=ContentType.objects.filter(model__in=CIRCUIT_TERMINATION_TERMINATION_TYPES), widget=HTMXSelect(), - required=False, label=_('Termination type') ) termination = DynamicModelChoiceField( label=_('Termination'), queryset=Site.objects.none(), # Initial queryset - required=False, disabled=True, selector=True ) @@ -247,16 +245,28 @@ class CircuitTerminationForm(NetBoxModelForm): self.fields['termination'].queryset = model.objects.all() self.fields['termination'].widget.attrs['selector'] = model._meta.label_lower 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: pass if self.instance and termination_type_id != self.instance.termination_type_id: self.initial['termination'] = None + else: + # Clear the initial termination value if termination_type is not set + self.initial['termination'] = None def clean(self): 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) 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'].widget.attrs['selector'] = model._meta.label_lower 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: pass diff --git a/netbox/circuits/models/circuits.py b/netbox/circuits/models/circuits.py index 0e2d808c4..10d621092 100644 --- a/netbox/circuits/models/circuits.py +++ b/netbox/circuits/models/circuits.py @@ -17,6 +17,7 @@ from netbox.models.features import ( TagsMixin, ) from netbox.models.mixins import DistanceMixin +from utilities.string import title from .base import BaseCircuitType @@ -367,6 +368,13 @@ class CircuitTermination( return reverse('circuits:circuittermination', args=[self.pk]) 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() if self.termination is None: diff --git a/netbox/circuits/tests/test_forms.py b/netbox/circuits/tests/test_forms.py new file mode 100644 index 000000000..7745526fb --- /dev/null +++ b/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) diff --git a/netbox/circuits/tests/test_models.py b/netbox/circuits/tests/test_models.py index f837cab65..a3c4a54ca 100644 --- a/netbox/circuits/tests/test_models.py +++ b/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 circuits.models import Circuit, CircuitTermination, CircuitType, Provider, ProviderNetwork @@ -146,3 +148,21 @@ class CircuitTerminationTestCase(TestCase): # Cache should be cleared (SET_NULL behavior) 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)