refactor(models): Centralize ACL rule validation

Moves ACL rule validation logic from forms to model methods to ensure
consistent enforcement across all usages. This improves maintainability
and eliminates redundant validation code.

Author: pheus

netbox_acls/forms/models.py

@@ -43,13 +43,6 @@
 # Sets a standard help_text value to be used by the various classes for acl index
 help_text_acl_rule_index = "Determines the order of the rule in the ACL processing. AKA Sequence Number."
 
-# Sets a standard error message for ACL rules with an action of remark, but no remark set.
-error_message_no_remark = "Action is set to remark, you MUST add a remark."
-# Sets a standard error message for ACL rules with an action of remark, but no source_prefix is set.
-error_message_action_remark_source_prefix_set = "Action is set to remark, Source Prefix CANNOT be set."
-# Sets a standard error message for ACL rules with an action not set to remark, but no remark is set.
-error_message_remark_without_action_remark = "CANNOT set remark unless action is set to remark."
-
 
 class AccessListForm(NetBoxModelForm):
     """
@@ -545,35 +538,6 @@ class Meta:
             ),
         }
 
-    def clean(self):
-        """
-        Validates form inputs before submitting:
-          - Check if action set to remark, but no remark set.
-          - Check if action set to remark, but source_prefix set.
-          - Check remark set, but action not set to remark.
-        """
-        super().clean()
-        cleaned_data = self.cleaned_data
-        error_message = {}
-
-        action = cleaned_data.get("action")
-        remark = cleaned_data.get("remark")
-        source_prefix = cleaned_data.get("source_prefix")
-
-        if action == "remark":
-            # Check if action set to remark, but no remark set.
-            if not remark:
-                error_message["remark"] = [error_message_no_remark]
-            # Check if action set to remark, but source_prefix set.
-            if source_prefix:
-                error_message["source_prefix"] = [error_message_action_remark_source_prefix_set]
-        # Check remark set, but action not set to remark.
-        elif remark:
-            error_message["remark"] = [error_message_remark_without_action_remark]
-
-        if error_message:
-            raise ValidationError(error_message)
-
 
 class ACLExtendedRuleForm(NetBoxModelForm):
     """
@@ -651,45 +615,3 @@ class Meta:
             ),
             "source_ports": help_text_acl_rule_logic,
         }
-
-    def clean(self):
-        """
-        Validates form inputs before submitting:
-        - Check if action set to remark, but no remark set.
-        - Check if action set to remark, but source_prefix set.
-        - Check if action set to remark, but source_ports set.
-        - Check if action set to remark, but destination_prefix set.
-        - Check if action set to remark, but destination_ports set.
-        - Check if action set to remark, but protocol set.
-        - Check remark set, but action not set to remark.
-        """
-        super().clean()
-        cleaned_data = self.cleaned_data
-        error_message = {}
-
-        action = cleaned_data.get("action")
-        remark = cleaned_data.get("remark")
-        source_prefix = cleaned_data.get("source_prefix")
-        source_ports = cleaned_data.get("source_ports")
-        destination_prefix = cleaned_data.get("destination_prefix")
-        destination_ports = cleaned_data.get("destination_ports")
-        protocol = cleaned_data.get("protocol")
-
-        if action == "remark":
-            if not remark:
-                error_message["remark"] = [error_message_no_remark]
-            if source_prefix:
-                error_message["source_prefix"] = [error_message_action_remark_source_prefix_set]
-            if source_ports:
-                error_message["source_ports"] = ["Action is set to remark, Source Ports CANNOT be set."]
-            if destination_prefix:
-                error_message["destination_prefix"] = ["Action is set to remark, Destination Prefix CANNOT be set."]
-            if destination_ports:
-                error_message["destination_ports"] = ["Action is set to remark, Destination Ports CANNOT be set."]
-            if protocol:
-                error_message["protocol"] = ["Action is set to remark, Protocol CANNOT be set."]
-        elif remark:
-            error_message["remark"] = [error_message_remark_without_action_remark]
-
-        if error_message:
-            raise ValidationError(error_message)

netbox_acls/models/access_list_rules.py

@@ -3,6 +3,7 @@
 """
 
 from django.contrib.postgres.fields import ArrayField
+from django.core.exceptions import ValidationError
 from django.db import models
 from django.urls import reverse
 from django.utils.translation import gettext_lazy as _
@@ -17,6 +18,29 @@
     "ACLExtendedRule",
 )
 
+# Error message when the action is 'remark', but no remark is provided.
+ERROR_MESSAGE_NO_REMARK = _("When the action is 'remark', a remark is required.")
+
+# Error message when the action is 'remark', but the source_prefix is set.
+ERROR_MESSAGE_ACTION_REMARK_SOURCE_PREFIX_SET = _("When the action is 'remark', the Source Prefix must not be set.")
+
+# Error message when the action is 'remark', but the source_ports are set.
+ERROR_MESSAGE_ACTION_REMARK_SOURCE_PORTS_SET = _("When the action is 'remark', Source Ports must not be set.")
+
+# Error message when the action is 'remark', but the destination_prefix is set.
+ERROR_MESSAGE_ACTION_REMARK_DESTINATION_PREFIX_SET = _(
+    "When the action is 'remark', the Destination Prefix must not be set."
+)
+
+# Error message when the action is 'remark', but the destination_ports are set.
+ERROR_MESSAGE_ACTION_REMARK_DESTINATION_PORTS_SET = _("When the action is 'remark', Destination Ports must not be set.")
+
+# Error message when the action is 'remark', but the protocol is set.
+ERROR_MESSAGE_ACTION_REMARK_PROTOCOL_SET = _("When the action is 'remark', Protocol must not be set.")
+
+# Error message when a remark is provided, but the action is not set to 'remark'.
+ERROR_MESSAGE_REMARK_WITHOUT_ACTION_REMARK = _("A remark cannot be set unless the action is 'remark'.")
+
 
 class ACLRule(NetBoxModel):
     """
@@ -111,6 +135,31 @@ class Meta(ACLRule.Meta):
         verbose_name = _("ACL Standard Rule")
         verbose_name_plural = _("ACL Standard Rules")
 
+    def clean(self):
+        """
+        Validate the ACL Standard Rule inputs.
+
+        If the action is 'remark', then the remark field must be provided (non-empty),
+        and the source_prefix field must be empty.
+        Conversely, if the remark field is provided, the action must be set to 'remark'.
+        """
+
+        super().clean()
+        errors = {}
+
+        # Validate that only the remark field is filled
+        if self.action == ACLRuleActionChoices.ACTION_REMARK:
+            if not self.remark:
+                errors["remark"] = ERROR_MESSAGE_NO_REMARK
+            if self.source_prefix:
+                errors["source_prefix"] = ERROR_MESSAGE_ACTION_REMARK_SOURCE_PREFIX_SET
+        # Validate that the action is "remark", when the remark field is provided
+        elif self.remark:
+            errors["remark"] = ERROR_MESSAGE_REMARK_WITHOUT_ACTION_REMARK
+
+        if errors:
+            raise ValidationError(errors)
+
 
 class ACLExtendedRule(ACLRule):
     """
@@ -174,5 +223,43 @@ class Meta(ACLRule.Meta):
         verbose_name = _("ACL Extended Rule")
         verbose_name_plural = _("ACL Extended Rules")
 
+    def clean(self):
+        """
+        Validate the ACL Extended Rule inputs.
+
+        When the action is 'remark', the remark field must be provided (non-empty),
+        and the following fields must be empty:
+          - source_prefix
+          - source_ports
+          - destination_prefix
+          - destination_ports
+          - protocol
+
+        Conversely, if a remark is provided, the action must be set to 'remark'.
+        """
+        super().clean()
+        errors = {}
+
+        # Validate that only the remark field is filled
+        if self.action == ACLRuleActionChoices.ACTION_REMARK:
+            if not self.remark:
+                errors["remark"] = ERROR_MESSAGE_NO_REMARK
+            if self.source_prefix:
+                errors["source_prefix"] = ERROR_MESSAGE_ACTION_REMARK_SOURCE_PREFIX_SET
+            if self.source_ports:
+                errors["source_ports"] = ERROR_MESSAGE_ACTION_REMARK_SOURCE_PORTS_SET
+            if self.destination_prefix:
+                errors["destination_prefix"] = ERROR_MESSAGE_ACTION_REMARK_DESTINATION_PREFIX_SET
+            if self.destination_ports:
+                errors["destination_ports"] = ERROR_MESSAGE_ACTION_REMARK_DESTINATION_PORTS_SET
+            if self.protocol:
+                errors["protocol"] = ERROR_MESSAGE_ACTION_REMARK_PROTOCOL_SET
+        # Validate that the action is "remark", when the remark field is provided
+        elif self.remark:
+            errors["remark"] = ERROR_MESSAGE_REMARK_WITHOUT_ACTION_REMARK
+
+        if errors:
+            raise ValidationError(errors)
+
     def get_protocol_color(self):
         return ACLProtocolChoices.colors.get(self.protocol)