Merge pull request #2750 from johannaengland/bugfix/alertprofiles/add…

…-group-to-expression

Fix form validation with equal and in operator for adding expression with group to filter

python/nav/web/alertprofiles/forms.py

@@ -650,29 +650,31 @@ def clean(self) -> Dict[str, Any]:
         value = validated_data["value"]
 
         if match_field.data_type == MatchField.IP:
-            if operator_type == Operator.IN:
-                ip_list = value.split()
-            else:
-                ip_list = [value]
-            validated_ip_addresses = []
-            for ip in ip_list:
-                if not is_valid_ip(ip=ip, strict=True) and not is_valid_cidr(cidr=ip):
-                    self.add_error(
-                        field="value",
-                        error=forms.ValidationError(("Invalid IP address: %s" % ip)),
-                    )
-                else:
-                    validated_ip_addresses.append(str(ip))
-            # Bring ip address back into original format to be processed below
-            value = " ".join(validated_ip_addresses)
+            validated_data["value"] = self._clean_ip_addresses(
+                operator_type=operator_type, value=value
+            )
+            return validated_data
 
         if operator_type == Operator.IN:
-            """If input was a multiple choice list we have to join each option in one
-            string, where each option is separated by a | (pipe).
-            If input was a IP adress we should replace space with | (pipe)."""
-            if match_field.data_type == MatchField.IP:
-                validated_data["value"] = value.replace(' ', '|')
-            else:
-                validated_data["value"] = "|".join(value)
+            validated_data["value"] = "|".join(value)
+        elif operator_type == Operator.EQUALS:
+            validated_data["value"] = value[0]
 
         return validated_data
+
+    def _clean_ip_addresses(self, operator_type, value):
+        if operator_type == Operator.IN:
+            ip_list = value.split()
+        else:
+            ip_list = [value]
+        validated_ip_addresses = []
+        for ip in ip_list:
+            if not is_valid_ip(ip=ip, strict=True) and not is_valid_cidr(cidr=ip):
+                self.add_error(
+                    field="value",
+                    error=forms.ValidationError(("Invalid IP address: %s" % ip)),
+                )
+            else:
+                validated_ip_addresses.append(str(ip))
+
+        return "|".join(validated_ip_addresses)

tests/integration/web/alertprofiles_test.py

@@ -405,10 +405,12 @@ def test_alertprofiles_add_expression_with_multiple_non_valid_ip_addresses_shoul
         ).exists()
         assert f"Invalid IP address: {invalid_ip}" in smart_str(response.content)
 
-    def test_alertprofiles_add_expression_with_multiple_alert_types_should_succeed(
+    def test_alertprofiles_add_expression_with_in_condition_should_succeed(
         self, client, dummy_filter
     ):
-        """Tests that an expression with multiple alert types can be added"""
+        """Tests that an expression with an in condition can be added, alert type is
+        just an example
+        """
         string_match_field = MatchField.objects.get(name="Alert type")
         url = reverse("alertprofiles-filters-saveexpression")
         data = {
@@ -429,6 +431,32 @@ def test_alertprofiles_add_expression_with_multiple_alert_types_should_succeed(
             response.content
         )
 
+    def test_alertprofiles_add_expression_with_equals_condition_should_succeed(
+        self, client, dummy_filter
+    ):
+        """Tests that an expression with an equals condition can be added, group is
+        just an example
+        """
+        group_match_field = MatchField.objects.get(name="Group")
+        url = reverse("alertprofiles-filters-saveexpression")
+        data = {
+            "filter": dummy_filter.pk,
+            "match_field": group_match_field.pk,
+            "operator": Operator.EQUALS,
+            "value": "AD",
+        }
+        response = client.post(url, data=data, follow=True)
+        assert response.status_code == 200
+        assert Expression.objects.filter(
+            filter=dummy_filter,
+            match_field=group_match_field,
+            operator=Operator.EQUALS,
+            value=data["value"],
+        ).exists()
+        assert f"Added expression to filter {dummy_filter}" in smart_str(
+            response.content
+        )
+
 
 class TestsPermissions:
     def test_set_accountgroup_permissions_should_not_crash(self, db, client):