fix UI and model bugs (#274)

changes/222.fixed

@@ -0,0 +1 @@
+Fixed server error when navigating to Policy detail view.

changes/233.fixed

@@ -0,0 +1,2 @@
+Fixed name fields being optional on multiple forms.
+Fixed assigned devices and assigned dynamic groups fields not marked as optional on NATPolicy and Policy.

changes/245.fixed

@@ -0,0 +1,2 @@
+Fixed server error when navigating to NATPolicy detail view.
+Fixed server error when updating device/dynamic group weights on NATPolicy.

changes/275.fixed

@@ -0,0 +1 @@
+Fixed capirca failures with Nautobot v2.3.3 or higher.

nautobot_firewall_models/filters.py

@@ -163,6 +163,7 @@ def search(self, queryset, name, value):  # pylint: disable=unused-argument
         """Construct Q filter for filterset."""
         if not value.strip():
             return queryset
+        # pylint: disable=unsupported-binary-operation
         return queryset.filter(
             Q(name__icontains=value) | Q(description__icontains=value) | Q(request_id__icontains=value)
         )
@@ -181,6 +182,7 @@ def search(self, queryset, name, value):  # pylint: disable=unused-argument
         """Construct Q filter for filterset."""
         if not value.strip():
             return queryset
+        # pylint: disable=unsupported-binary-operation
         return queryset.filter(
             Q(name__icontains=value) | Q(description__icontains=value) | Q(request_id__icontains=value)
         )

nautobot_firewall_models/forms.py

@@ -538,7 +538,6 @@ class PolicyRuleFilterForm(LocalContextFilterForm, NautobotFilterForm):
 class PolicyRuleForm(LocalContextModelForm, NautobotModelForm):
     """PolicyRule creation/edit form."""
 
-    name = forms.CharField(required=False, label="Name")
     tags = DynamicModelMultipleChoiceField(queryset=Tag.objects.all(), required=False)
     source_users = DynamicModelMultipleChoiceField(
         queryset=models.UserObject.objects.all(), label="Source User Objects", required=False
@@ -710,7 +709,6 @@ class NATPolicyRuleForm(LocalContextModelForm, NautobotModelForm):
     """NATPolicyRule creation/edit form."""
 
     # Metadata
-    name = forms.CharField(required=False, label="Name")
     tags = DynamicModelMultipleChoiceField(queryset=Tag.objects.all(), required=False)
     request_id = forms.CharField(required=False, label="Optional field for request ticket identifier.")
 

nautobot_firewall_models/migrations/0022_fix_policy_natpolicy_m2ms.py

@@ -0,0 +1,54 @@
+# Generated by Django 4.2.16 on 2024-10-03 21:29
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("dcim", "0014_location_status_data_migration"),
+        ("extras", "0047_enforce_custom_field_slug"),
+        ("nautobot_firewall_models", "0021_alter_addressobject_status_and_more"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="natpolicy",
+            name="assigned_devices",
+            field=models.ManyToManyField(
+                blank=True,
+                related_name="nat_policies",
+                through="nautobot_firewall_models.NATPolicyDeviceM2M",
+                to="dcim.device",
+            ),
+        ),
+        migrations.AlterField(
+            model_name="natpolicy",
+            name="assigned_dynamic_groups",
+            field=models.ManyToManyField(
+                blank=True,
+                related_name="nat_policies",
+                through="nautobot_firewall_models.NATPolicyDynamicGroupM2M",
+                to="extras.dynamicgroup",
+            ),
+        ),
+        migrations.AlterField(
+            model_name="policy",
+            name="assigned_devices",
+            field=models.ManyToManyField(
+                blank=True,
+                related_name="firewall_policies",
+                through="nautobot_firewall_models.PolicyDeviceM2M",
+                to="dcim.device",
+            ),
+        ),
+        migrations.AlterField(
+            model_name="policy",
+            name="assigned_dynamic_groups",
+            field=models.ManyToManyField(
+                blank=True,
+                related_name="firewall_policies",
+                through="nautobot_firewall_models.PolicyDynamicGroupM2M",
+                to="extras.dynamicgroup",
+            ),
+        ),
+    ]

nautobot_firewall_models/models/nat_policy.py

@@ -252,10 +252,16 @@ class NATPolicy(PrimaryModel):
         to="nautobot_firewall_models.NATPolicyRule", blank=True, related_name="nat_policies"
     )
     assigned_devices = models.ManyToManyField(
-        to="dcim.Device", through="NATPolicyDeviceM2M", related_name="nat_policies"
+        to="dcim.Device",
+        through="NATPolicyDeviceM2M",
+        related_name="nat_policies",
+        blank=True,
     )
     assigned_dynamic_groups = models.ManyToManyField(
-        to="extras.DynamicGroup", through="NATPolicyDynamicGroupM2M", related_name="nat_policies"
+        to="extras.DynamicGroup",
+        through="NATPolicyDynamicGroupM2M",
+        related_name="nat_policies",
+        blank=True,
     )
     status = StatusField(
         on_delete=models.PROTECT,

nautobot_firewall_models/models/security_policy.py

@@ -183,10 +183,16 @@ class Policy(PrimaryModel):
     name = models.CharField(max_length=100, unique=True)
     policy_rules = models.ManyToManyField(to=PolicyRule, blank=True, related_name="policies")
     assigned_devices = models.ManyToManyField(
-        to="dcim.Device", through="PolicyDeviceM2M", related_name="firewall_policies"
+        to="dcim.Device",
+        through="PolicyDeviceM2M",
+        related_name="firewall_policies",
+        blank=True,
     )
     assigned_dynamic_groups = models.ManyToManyField(
-        to="extras.DynamicGroup", through="PolicyDynamicGroupM2M", related_name="firewall_policies"
+        to="extras.DynamicGroup",
+        through="PolicyDynamicGroupM2M",
+        related_name="firewall_policies",
+        blank=True,
     )
     status = StatusField(
         on_delete=models.PROTECT,

nautobot_firewall_models/templates/nautobot_firewall_models/inc/nat_policy_device_weight.html

@@ -6,7 +6,7 @@
                 <strong>Assign NAT Policy Weight to Device</strong>
             </div>
             {% if object.natpolicydevicem2m_set.all %}
-                <form action="{% url 'plugins:nautobot_firewall_models:natpolicy_set_device_weight' pk=object.id %}" method="post" enctype="multipart/form-data" class="form form-horizontal">
+                <form action="{% url 'plugins:nautobot_firewall_models:natpolicy_devices' pk=object.id %}" method="post" enctype="multipart/form-data" class="form form-horizontal">
                     {% csrf_token %}
                     <table class="table table-hover panel-body">
                         <tr>

nautobot_firewall_models/templates/nautobot_firewall_models/inc/nat_policy_dynamic_group_weight.html

@@ -6,7 +6,7 @@
                 <strong>Assign NAT Policy Weight to Dynamic Group</strong>
             </div>
             {% if object.natpolicydynamicgroupm2m_set.all %}
-                <form action="{% url 'plugins:nautobot_firewall_models:natpolicy_set_dynamic_group_weight' pk=object.id %}" method="post" enctype="multipart/form-data" class="form form-horizontal">
+                <form action="{% url 'plugins:nautobot_firewall_models:natpolicy_dynamic-groups' pk=object.id %}" method="post" enctype="multipart/form-data" class="form form-horizontal">
                     {% csrf_token %}
                     <table class="table table-hover panel-body">
                         <tr>

nautobot_firewall_models/templates/nautobot_firewall_models/inc/policy_dynamic_group_weight.html

@@ -6,7 +6,7 @@
                 <strong>Assign Policy Weight to Dynamic Group</strong>
             </div>
             {% if object.policydynamicgroupm2m_set.all %}
-                <form action="{% url 'plugins:nautobot_firewall_models:policy_set_dynamic_group_weight' pk=object.id %}" method="post" enctype="multipart/form-data" class="form form-horizontal">
+                <form action="{% url 'plugins:nautobot_firewall_models:policy_dynamic-groups' pk=object.id %}" method="post" enctype="multipart/form-data" class="form form-horizontal">
                     {% csrf_token %}
                     <table class="table table-hover panel-body">
                         <tr>

nautobot_firewall_models/utils/capirca.py

@@ -133,18 +133,29 @@ def _check_for_empty(_type, start1, start2, end1, end2):
                 "Capirca does not support the value you provided, and was removed from consideration"
             )
 
-    def _format_data(self, data, _type):
+    def _format_data(self, data, _type):  # pylint: disable=too-many-statements
         def format_address(data, name):
             """Format address objects, looking for the address type."""
-            keys = ["ip_range", "fqdn", "prefix", "ip_address"]
-            for key in keys:
-                if data.get(key):
-                    value = data[key]["display"]
-                    if not self.address.get(name):
-                        self.address[name] = {key: value}
-                    break
+            if data.get("ip_range"):
+                value = data["ip_range"]["display"]
+                if not self.address.get(name):
+                    self.address[name] = {"ip_range": value}
+            elif data.get("fqdn"):
+                value = data["fqdn"]["display"]
+                if not self.address.get(name):
+                    self.address[name] = {"fqdn": value}
+            elif data.get("prefix"):
+                value = data["prefix"]["prefix"]
+                if not self.address.get(name):
+                    self.address[name] = {"prefix": value}
+            elif data.get("ip_address"):
+                value = data["ip_address"]["address"]
+                if not self.address.get(name):
+                    self.address[name] = {"ip_address": value}
             else:
-                raise ValidationError(f"Address object: `{name}` does not have a valid `{str(keys)}` object applied.")
+                raise ValidationError(
+                    f"Address object: `{name}` does not have a valid `ip_range`, `fqdn`, `prefix`, or `ip_address` object applied."
+                )
 
         def format_address_group(data, name):
             """Format address group objects, also adding the address objects as new ones are found."""

nautobot_firewall_models/viewsets/nat_policy.py

@@ -78,10 +78,10 @@ def devices(self, request, pk, *args, **kwargs):
         form_data = dict(request.POST)
         form_data.pop("csrfmiddlewaretoken", None)
         for device, weight in form_data.items():
-            m2m = NATPolicyDeviceM2M.objects.get(device=device, policy=pk)
+            m2m = NATPolicyDeviceM2M.objects.get(device=device, nat_policy=pk)
             m2m.weight = weight[0]
             m2m.validated_save()
-        return redirect(reverse("plugins:nautobot_firewall_models:nat_policy", kwargs={"pk": pk}))
+        return redirect(reverse("plugins:nautobot_firewall_models:natpolicy", kwargs={"pk": pk}))
 
     @action(detail=True, methods=["post"])
     def dynamic_groups(self, request, pk, *args, **kwargs):
@@ -90,10 +90,10 @@ def dynamic_groups(self, request, pk, *args, **kwargs):
         form_data = dict(request.POST)
         form_data.pop("csrfmiddlewaretoken", None)
         for group, weight in form_data.items():
-            m2m = NATPolicyDynamicGroupM2M.objects.get(dynamic_group=group, policy=pk)
+            m2m = NATPolicyDynamicGroupM2M.objects.get(dynamic_group=group, nat_policy=pk)
             m2m.weight = weight[0]
             m2m.validated_save()
-        return redirect(reverse("plugins:nautobot_firewall_models:nat_policy", kwargs={"pk": pk}))
+        return redirect(reverse("plugins:nautobot_firewall_models:natpolicy", kwargs={"pk": pk}))
 
     def get_queryset(self):
         """Overload to overwrite permissiosn action map."""