Browse Source

fix(workflow): resolve threshold cannot be cleared bug (#16741)

* Fixing bug with clearing resolve thresholds
* Adding test to make sure unchanged field is not reset
Chris Fuller 5 years ago
parent
commit
c37aed2b58

+ 2 - 2
src/sentry/incidents/logic.py

@@ -977,8 +977,8 @@ def update_alert_rule_trigger(
         updated_fields["threshold_type"] = threshold_type.value
     if alert_threshold is not None:
         updated_fields["alert_threshold"] = alert_threshold
-    if resolve_threshold is not None:
-        updated_fields["resolve_threshold"] = resolve_threshold
+    # We set resolve_threshold to None as a 'reset', in case it was previously a value and we're removing it here.
+    updated_fields["resolve_threshold"] = resolve_threshold
 
     deleted_exclusion_ids = []
     new_subs = []

+ 54 - 0
tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py

@@ -222,6 +222,60 @@ class AlertRuleDetailsPutEndpointTest(AlertRuleDetailsBase, APITestCase):
         assert resp.data["name"] == "AUniqueName"
         assert resp.data["triggers"][1]["alertThreshold"] == 125
 
+    def test_delete_resolve_alert_threshold(self):
+        # This is a test to make sure we can remove a resolveThreshold after it has been set.
+        self.create_member(
+            user=self.user, organization=self.organization, role="owner", teams=[self.team]
+        )
+
+        self.login_as(self.user)
+        alert_rule = self.alert_rule
+        # We need the IDs to force update instead of create, so we just get the rule using our own API. Like frontend would.
+        serialized_alert_rule = self.get_serialized_alert_rule()
+
+        serialized_alert_rule["triggers"][1]["resolveThreshold"] = None
+        serialized_alert_rule["name"] = "AUniqueName"
+
+        with self.feature("organizations:incidents"):
+            resp = self.get_valid_response(
+                self.organization.slug, alert_rule.id, **serialized_alert_rule
+            )
+
+        assert resp.data["name"] == "AUniqueName"
+        assert resp.data["triggers"][1]["resolveThreshold"] is None
+
+    def test_update_resolve_alert_threshold(self):
+        # This is a test to make sure we can remove a resolveThreshold after it has been set.
+        self.create_member(
+            user=self.user, organization=self.organization, role="owner", teams=[self.team]
+        )
+
+        self.login_as(self.user)
+        alert_rule = self.alert_rule
+        # We need the IDs to force update instead of create, so we just get the rule using our own API. Like frontend would.
+        serialized_alert_rule = self.get_serialized_alert_rule()
+
+        serialized_alert_rule["triggers"][1]["resolveThreshold"] = 75
+        serialized_alert_rule["name"] = "AUniqueName"
+
+        with self.feature("organizations:incidents"):
+            resp = self.get_valid_response(
+                self.organization.slug, alert_rule.id, **serialized_alert_rule
+            )
+        assert resp.data["name"] == "AUniqueName"
+        assert resp.data["triggers"][1]["resolveThreshold"] == 75
+
+        # We had/have logic to remove unchanged fields from the serializer before passing them to update
+        # It is possible with an error here that sending an update without changing the field
+        # could nullify it. So we make sure it's not getting nullified here.
+        # we can an update with all the same fields, and it comes back as 125 instead of None.
+        with self.feature("organizations:incidents"):
+            resp = self.get_valid_response(
+                self.organization.slug, alert_rule.id, **serialized_alert_rule
+            )
+        assert resp.data["name"] == "AUniqueName"
+        assert resp.data["triggers"][1]["resolveThreshold"] == 75
+
     def test_delete_trigger(self):
         self.create_member(
             user=self.user, organization=self.organization, role="owner", teams=[self.team]