From 2c273b9f728023a1c15ebbea2df13650b19e0009 Mon Sep 17 00:00:00 2001 From: Gary Snider <75227981+gsnider2195@users.noreply.github.com> Date: Thu, 3 Oct 2024 15:36:00 -0700 Subject: [PATCH] fix multiple bugs --- changes/222.fixed | 1 + changes/233.fixed | 2 + changes/245.fixed | 2 + nautobot_firewall_models/forms.py | 3 +- .../0022_fix_policy_natpolicy_m2ms.py | 54 +++++++++++++++++++ nautobot_firewall_models/models/nat_policy.py | 10 +++- .../models/security_policy.py | 10 +++- .../inc/nat_policy_device_weight.html | 2 +- .../inc/nat_policy_dynamic_group_weight.html | 2 +- .../inc/policy_dynamic_group_weight.html | 2 +- .../viewsets/nat_policy.py | 8 +-- 11 files changed, 83 insertions(+), 13 deletions(-) create mode 100644 changes/222.fixed create mode 100644 changes/233.fixed create mode 100644 changes/245.fixed create mode 100644 nautobot_firewall_models/migrations/0022_fix_policy_natpolicy_m2ms.py diff --git a/changes/222.fixed b/changes/222.fixed new file mode 100644 index 00000000..34368dd3 --- /dev/null +++ b/changes/222.fixed @@ -0,0 +1 @@ +Fixed server error when navigating to Policy detail view. diff --git a/changes/233.fixed b/changes/233.fixed new file mode 100644 index 00000000..0785601d --- /dev/null +++ b/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. diff --git a/changes/245.fixed b/changes/245.fixed new file mode 100644 index 00000000..7357fa0e --- /dev/null +++ b/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. diff --git a/nautobot_firewall_models/forms.py b/nautobot_firewall_models/forms.py index e0d9768f..348010b2 100644 --- a/nautobot_firewall_models/forms.py +++ b/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,7 @@ class NATPolicyRuleForm(LocalContextModelForm, NautobotModelForm): """NATPolicyRule creation/edit form.""" # Metadata - name = forms.CharField(required=False, label="Name") + name = forms.CharField(label="Name") tags = DynamicModelMultipleChoiceField(queryset=Tag.objects.all(), required=False) request_id = forms.CharField(required=False, label="Optional field for request ticket identifier.") diff --git a/nautobot_firewall_models/migrations/0022_fix_policy_natpolicy_m2ms.py b/nautobot_firewall_models/migrations/0022_fix_policy_natpolicy_m2ms.py new file mode 100644 index 00000000..b94468e5 --- /dev/null +++ b/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", "0062_module_data_migration"), + ("extras", "0114_computedfield_grouping"), + ("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", + ), + ), + ] diff --git a/nautobot_firewall_models/models/nat_policy.py b/nautobot_firewall_models/models/nat_policy.py index da4c5285..6b1c2e95 100644 --- a/nautobot_firewall_models/models/nat_policy.py +++ b/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, diff --git a/nautobot_firewall_models/models/security_policy.py b/nautobot_firewall_models/models/security_policy.py index 356d6a29..e5fbd711 100644 --- a/nautobot_firewall_models/models/security_policy.py +++ b/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, diff --git a/nautobot_firewall_models/templates/nautobot_firewall_models/inc/nat_policy_device_weight.html b/nautobot_firewall_models/templates/nautobot_firewall_models/inc/nat_policy_device_weight.html index 9f6d2cde..a1065717 100644 --- a/nautobot_firewall_models/templates/nautobot_firewall_models/inc/nat_policy_device_weight.html +++ b/nautobot_firewall_models/templates/nautobot_firewall_models/inc/nat_policy_device_weight.html @@ -6,7 +6,7 @@ Assign NAT Policy Weight to Device {% if object.natpolicydevicem2m_set.all %} -
+ {% csrf_token %} diff --git a/nautobot_firewall_models/templates/nautobot_firewall_models/inc/nat_policy_dynamic_group_weight.html b/nautobot_firewall_models/templates/nautobot_firewall_models/inc/nat_policy_dynamic_group_weight.html index 53ccbb3d..4c2c6088 100644 --- a/nautobot_firewall_models/templates/nautobot_firewall_models/inc/nat_policy_dynamic_group_weight.html +++ b/nautobot_firewall_models/templates/nautobot_firewall_models/inc/nat_policy_dynamic_group_weight.html @@ -6,7 +6,7 @@ Assign NAT Policy Weight to Dynamic Group {% if object.natpolicydynamicgroupm2m_set.all %} - + {% csrf_token %}
diff --git a/nautobot_firewall_models/templates/nautobot_firewall_models/inc/policy_dynamic_group_weight.html b/nautobot_firewall_models/templates/nautobot_firewall_models/inc/policy_dynamic_group_weight.html index de3c1a57..f8aee545 100644 --- a/nautobot_firewall_models/templates/nautobot_firewall_models/inc/policy_dynamic_group_weight.html +++ b/nautobot_firewall_models/templates/nautobot_firewall_models/inc/policy_dynamic_group_weight.html @@ -6,7 +6,7 @@ Assign Policy Weight to Dynamic Group {% if object.policydynamicgroupm2m_set.all %} - + {% csrf_token %}
diff --git a/nautobot_firewall_models/viewsets/nat_policy.py b/nautobot_firewall_models/viewsets/nat_policy.py index 1d437aa2..306b91b2 100644 --- a/nautobot_firewall_models/viewsets/nat_policy.py +++ b/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."""