Fixes #11460 - Fix unterminated cable exception when editing cable (#15813)

* Fix cable edit form with single unterminated cable

* Minor tweaks

* Instead of skipping HTMX, override the template & move form template to an "htmx" template

* Use HTMXSelect widget for A/B type selection

* Infer A/B termination types from POST data

* Fix saving cable which results in resetting of the termination type fields

* Condense view logic

---------

Co-authored-by: Jeremy Stretch <jstretch@netboxlabs.com>

netbox/dcim/forms/connections.py

@@ -1,4 +1,5 @@
 from django import forms
+from django.contrib.contenttypes.models import ContentType
 from django.utils.translation import gettext_lazy as _
 
 from circuits.models import Circuit, CircuitTermination
@@ -82,14 +83,22 @@ def get_cable_form(a_type, b_type):
 
     class _CableForm(CableForm, metaclass=FormMetaclass):
 
-        def __init__(self, *args, **kwargs):
+        def __init__(self, *args, initial=None, **kwargs):
+
+            initial = initial or {}
+            if a_type:
+                ct = ContentType.objects.get_for_model(a_type)
+                initial['a_terminations_type'] = f'{ct.app_label}.{ct.model}'
+            if b_type:
+                ct = ContentType.objects.get_for_model(b_type)
+                initial['b_terminations_type'] = f'{ct.app_label}.{ct.model}'
 
             # TODO: Temporary hack to work around list handling limitations with utils.normalize_querydict()
             for field_name in ('a_terminations', 'b_terminations'):
-                if field_name in kwargs.get('initial', {}) and type(kwargs['initial'][field_name]) is not list:
+                if field_name in initial and type(initial[field_name]) is not list:
-                    kwargs['initial'][field_name] = [kwargs['initial'][field_name]]
+                    initial[field_name] = [initial[field_name]]
 
-            super().__init__(*args, **kwargs)
+            super().__init__(*args, initial=initial, **kwargs)
 
             if self.instance and self.instance.pk:
                 # Initialize A/B terminations when modifying an existing Cable instance
@@ -100,7 +109,7 @@ def get_cable_form(a_type, b_type):
             super().clean()
 
             # Set the A/B terminations on the Cable instance
-            self.instance.a_terminations = self.cleaned_data['a_terminations']
+            self.instance.a_terminations = self.cleaned_data.get('a_terminations', [])
-            self.instance.b_terminations = self.cleaned_data['b_terminations']
+            self.instance.b_terminations = self.cleaned_data.get('b_terminations', [])
 
     return _CableForm

netbox/dcim/forms/model_forms.py

@@ -13,8 +13,7 @@ from netbox.forms import NetBoxModelForm
 from tenancy.forms import TenancyForm
 from utilities.forms import BootstrapMixin, add_blank_choice
 from utilities.forms.fields import (
-    CommentField, ContentTypeChoiceField, DynamicModelChoiceField, DynamicModelMultipleChoiceField, JSONField,
+    CommentField, DynamicModelChoiceField, DynamicModelMultipleChoiceField, JSONField, NumericArrayField, SlugField,
-    NumericArrayField, SlugField,
 )
 from utilities.forms.widgets import APISelect, ClearableFileInput, HTMXSelect, NumberWithOptions, SelectWithPK
 from virtualization.models import Cluster
@@ -616,14 +615,33 @@ class ModuleForm(ModuleCommonForm, NetBoxModelForm):
             self.fields['adopt_components'].disabled = True
 
 
+def get_termination_type_choices():
+    return add_blank_choice([
+        (f'{ct.app_label}.{ct.model}', ct.model_class()._meta.verbose_name.title())
+        for ct in ContentType.objects.filter(CABLE_TERMINATION_MODELS)
+    ])
+
+
 class CableForm(TenancyForm, NetBoxModelForm):
+    a_terminations_type = forms.ChoiceField(
+        choices=get_termination_type_choices,
+        required=False,
+        widget=HTMXSelect(),
+        label=_('Type')
+    )
+    b_terminations_type = forms.ChoiceField(
+        choices=get_termination_type_choices,
+        required=False,
+        widget=HTMXSelect(),
+        label=_('Type')
+    )
     comments = CommentField()
 
     class Meta:
         model = Cable
         fields = [
-            'type', 'status', 'tenant_group', 'tenant', 'label', 'color', 'length', 'length_unit', 'description',
+            'a_terminations_type', 'b_terminations_type', 'type', 'status', 'tenant_group', 'tenant', 'label', 'color',
-            'comments', 'tags',
+            'length', 'length_unit', 'description', 'comments', 'tags',
         ]
         error_messages = {
             'length': {

netbox/dcim/views.py

@@ -3183,34 +3183,29 @@ class CableView(generic.ObjectView):
 class CableEditView(generic.ObjectEditView):
     queryset = Cable.objects.all()
     template_name = 'dcim/cable_edit.html'
+    htmx_template_name = 'dcim/htmx/cable_edit.html'
 
-    def dispatch(self, request, *args, **kwargs):
+    def alter_object(self, obj, request, url_args, url_kwargs):
-
-        # If creating a new Cable, initialize the form class using URL query params
-        if 'pk' not in kwargs:
-            self.form = forms.get_cable_form(
-                a_type=CABLE_TERMINATION_TYPES.get(request.GET.get('a_terminations_type')),
-                b_type=CABLE_TERMINATION_TYPES.get(request.GET.get('b_terminations_type'))
-            )
-
-        return super().dispatch(request, *args, **kwargs)
-
-    def get_object(self, **kwargs):
         """
-        Hack into get_object() to set the form class when editing an existing Cable, since ObjectEditView
+        Hack into alter_object() to set the form class when editing an existing Cable, since ObjectEditView
         doesn't currently provide a hook for dynamic class resolution.
         """
-        obj = super().get_object(**kwargs)
+        a_terminations_type = CABLE_TERMINATION_TYPES.get(
+            request.GET.get('a_terminations_type') or request.POST.get('a_terminations_type')
+        )
+        b_terminations_type = CABLE_TERMINATION_TYPES.get(
+            request.GET.get('b_terminations_type') or request.POST.get('b_terminations_type')
+        )
 
         if obj.pk:
-            # TODO: Optimize this logic
+            if not a_terminations_type and (termination_a := obj.terminations.filter(cable_end='A').first()):
-            termination_a = obj.terminations.filter(cable_end='A').first()
+                a_terminations_type = termination_a.termination._meta.model
-            a_type = termination_a.termination._meta.model if termination_a else None
+            if not b_terminations_type and (termination_b := obj.terminations.filter(cable_end='B').first()):
-            termination_b = obj.terminations.filter(cable_end='B').first()
+                b_terminations_type = termination_b.termination._meta.model
-            b_type = termination_b.termination._meta.model if termination_b else None
-            self.form = forms.get_cable_form(a_type, b_type)
 
-        return obj
+        self.form = forms.get_cable_form(a_terminations_type, b_terminations_type)
+
+        return super().alter_object(obj, request, url_args, url_kwargs)
 
     def get_extra_addanother_params(self, request):
 

netbox/netbox/views/generic/object_views.py

@@ -167,6 +167,7 @@ class ObjectEditView(GetReturnURLMixin, BaseObjectView):
     """
     template_name = 'generic/object_edit.html'
     form = None
+    htmx_template_name = 'htmx/form.html'
 
     def dispatch(self, request, *args, **kwargs):
         # Determine required permission based on whether we are editing an existing object
@@ -228,7 +229,7 @@ class ObjectEditView(GetReturnURLMixin, BaseObjectView):
 
         # If this is an HTMX request, return only the rendered form HTML
         if is_htmx(request):
-            return render(request, 'htmx/form.html', {
+            return render(request, self.htmx_template_name, {
                 'form': form,
             })
 

netbox/templates/dcim/cable_edit.html

@@ -1,90 +1,5 @@
 {% extends 'generic/object_edit.html' %}
-{% load static %}
-{% load helpers %}
-{% load form_helpers %}
-{% load i18n %}
 
 {% block form %}
-
+  {%  include 'dcim/htmx/cable_edit.html' %}
-  {# A side termination #}
-  <div class="field-group mb-5">
-    <div class="row mb-2">
-      <h5 class="offset-sm-3">{% trans "A Side" %}</h5>
-    </div>
-    {% if 'termination_a_device' in form.fields %}
-      {% render_field form.termination_a_device %}
-    {% endif %}
-    {% if 'termination_a_powerpanel' in form.fields %}
-      {% render_field form.termination_a_powerpanel %}
-    {% endif %}
-    {% if 'termination_a_circuit' in form.fields %}
-      {% render_field form.termination_a_circuit %}
-    {% endif %}
-    {% render_field form.a_terminations %}
-  </div>
-
-  {# B side termination #}
-  <div class="field-group mb-5">
-    <div class="row mb-2">
-      <h5 class="offset-sm-3">{% trans "B Side" %}</h5>
-    </div>
-    {% if 'termination_b_device' in form.fields %}
-      {% render_field form.termination_b_device %}
-    {% endif %}
-    {% if 'termination_b_powerpanel' in form.fields %}
-      {% render_field form.termination_b_powerpanel %}
-    {% endif %}
-    {% if 'termination_b_circuit' in form.fields %}
-      {% render_field form.termination_b_circuit %}
-    {% endif %}
-    {% render_field form.b_terminations %}
-  </div>
-
-  {# Cable attributes #}
-  <div class="field-group mb-5">
-    <div class="row mb-2">
-      <h5 class="offset-sm-3">{% trans "Cable" %}</h5>
-    </div>
-    {% render_field form.status %}
-    {% render_field form.type %}
-    {% render_field form.label %}
-    {% render_field form.description %}
-    {% render_field form.color %}
-    <div class="row mb-3">
-      <label class="col-sm-3 col-form-label text-lg-end">{{ form.length.label }}</label>
-      <div class="col-md-5">
-        {{ form.length }}
-      </div>
-      <div class="col-md-4">
-        {{ form.length_unit }}
-      </div>
-      <div class="invalid-feedback"></div>
-    </div>
-    {% render_field form.tags %}
-  </div>
-
-  <div class="field-group mb-5">
-    <div class="row mb-2">
-      <h5 class="offset-sm-3">{% trans "Tenancy" %}</h5>
-    </div>
-    {% render_field form.tenant_group %}
-    {% render_field form.tenant %}
-  </div>
-
-  {% if form.custom_fields %}
-    <div class="field-group mb-5">
-      <div class="row mb-2">
-        <h5 class="offset-sm-3">{% trans "Custom Fields" %}</h5>
-      </div>
-      {% render_custom_fields form %}
-    </div>
-  {% endif %}
-
-  {% if form.comments %}
-    <div class="field-group mb-5">
-      <h5 class="text-center">{% trans "Comments" %}</h5>
-      {% render_field form.comments %}
-    </div>
-  {% endif %}
-
 {% endblock %}

netbox/templates/dcim/htmx/cable_edit.html

@@ -0,0 +1,92 @@
+{% load static %}
+{% load helpers %}
+{% load form_helpers %}
+{% load i18n %}
+
+
+{# A side termination #}
+<div id="a_termination_block" class="field-group mb-5">
+  <div class="row mb-2">
+    <h5 class="offset-sm-3">{% trans "A Side" %}</h5>
+  </div>
+  {% render_field form.a_terminations_type %}
+  {% if 'termination_a_device' in form.fields %}
+    {% render_field form.termination_a_device %}
+  {% endif %}
+  {% if 'termination_a_powerpanel' in form.fields %}
+    {% render_field form.termination_a_powerpanel %}
+  {% endif %}
+  {% if 'termination_a_circuit' in form.fields %}
+    {% render_field form.termination_a_circuit %}
+  {% endif %}
+  {% if 'a_terminations' in form.fields %}
+    {% render_field form.a_terminations %}
+  {% endif %}
+</div>
+
+{# B side termination #}
+<div id="b_termination_block" class="field-group mb-5">
+  <div class="row mb-2">
+    <h5 class="offset-sm-3">{% trans "B Side" %}</h5>
+  </div>
+  {% render_field form.b_terminations_type %}
+  {% if 'termination_b_device' in form.fields %}
+    {% render_field form.termination_b_device %}
+  {% endif %}
+  {% if 'termination_b_powerpanel' in form.fields %}
+    {% render_field form.termination_b_powerpanel %}
+  {% endif %}
+  {% if 'termination_b_circuit' in form.fields %}
+    {% render_field form.termination_b_circuit %}
+  {% endif %}
+  {% if 'b_terminations' in form.fields %}
+    {% render_field form.b_terminations %}
+  {% endif %}
+</div>
+
+{# Cable attributes #}
+<div class="field-group mb-5">
+  <div class="row mb-2">
+    <h5 class="offset-sm-3">{% trans "Cable" %}</h5>
+  </div>
+  {% render_field form.status %}
+  {% render_field form.type %}
+  {% render_field form.label %}
+  {% render_field form.description %}
+  {% render_field form.color %}
+  <div class="row mb-3">
+    <label class="col-sm-3 col-form-label text-lg-end">{{ form.length.label }}</label>
+    <div class="col-md-5">
+      {{ form.length }}
+    </div>
+    <div class="col-md-4">
+      {{ form.length_unit }}
+    </div>
+    <div class="invalid-feedback"></div>
+  </div>
+  {% render_field form.tags %}
+</div>
+
+<div class="field-group mb-5">
+  <div class="row mb-2">
+    <h5 class="offset-sm-3">{% trans "Tenancy" %}</h5>
+  </div>
+  {% render_field form.tenant_group %}
+  {% render_field form.tenant %}
+</div>
+
+{% if form.custom_fields %}
+  <div class="field-group mb-5">
+    <div class="row mb-2">
+      <h5 class="offset-sm-3">{% trans "Custom Fields" %}</h5>
+    </div>
+    {% render_custom_fields form %}
+  </div>
+{% endif %}
+
+{% if form.comments %}
+  <div class="field-group mb-5">
+    <h5 class="text-center">{% trans "Comments" %}</h5>
+    {% render_field form.comments %}
+  </div>
+{% endif %}