From a17f68e97bc21a4682045ce22848a464ea948b0e Mon Sep 17 00:00:00 2001 From: Michelle Fu Date: Thu, 6 Nov 2025 13:50:26 -0800 Subject: [PATCH 1/2] check for resolution condition in validator --- src/sentry/incidents/metric_issue_detector.py | 14 ++++++- .../endpoints/validators/test_validators.py | 39 ++++++++++++++++++- 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/src/sentry/incidents/metric_issue_detector.py b/src/sentry/incidents/metric_issue_detector.py index 0fdb63cce3a807..c3efde8b550b5b 100644 --- a/src/sentry/incidents/metric_issue_detector.py +++ b/src/sentry/incidents/metric_issue_detector.py @@ -82,7 +82,13 @@ def schedule_update_project_config(detector: Detector) -> None: class MetricIssueComparisonConditionValidator(BaseDataConditionValidator): supported_conditions = frozenset( - (Condition.GREATER, Condition.LESS, Condition.ANOMALY_DETECTION) + ( + Condition.GREATER, + Condition.LESS, + Condition.GREATER_OR_EQUAL, + Condition.LESS_OR_EQUAL, + Condition.ANOMALY_DETECTION, + ) ) supported_condition_results = frozenset( (DetectorPriorityLevel.HIGH, DetectorPriorityLevel.MEDIUM, DetectorPriorityLevel.OK) @@ -132,6 +138,12 @@ def validate_conditions(self, value): MetricIssueComparisonConditionValidator(data=value, many=True).is_valid( raise_exception=True ) + if not any( + condition["condition_result"] == DetectorPriorityLevel.OK for condition in value + ) and not any(condition["type"] == Condition.ANOMALY_DETECTION for condition in value): + raise serializers.ValidationError( + "Resolution condition required for metric issue detector." + ) return value diff --git a/tests/sentry/incidents/endpoints/validators/test_validators.py b/tests/sentry/incidents/endpoints/validators/test_validators.py index 562b3d97dbfc8b..02e2e85e38bc49 100644 --- a/tests/sentry/incidents/endpoints/validators/test_validators.py +++ b/tests/sentry/incidents/endpoints/validators/test_validators.py @@ -161,6 +161,12 @@ def setUp(self) -> None: "conditionResult": DetectorPriorityLevel.HIGH, "conditionGroupId": self.data_condition_group.id, }, + { + "type": Condition.LESS_OR_EQUAL, + "comparison": 100, + "conditionResult": DetectorPriorityLevel.OK, + "conditionGroupId": self.data_condition_group.id, + }, ], }, "config": { @@ -244,7 +250,7 @@ def test_create_with_valid_data( # Verify conditions in DB conditions = list(DataCondition.objects.filter(condition_group=condition_group)) - assert len(conditions) == 1 + assert len(conditions) == 2 condition = conditions[0] assert condition.type == Condition.GREATER assert condition.comparison == 100 @@ -399,6 +405,31 @@ def test_invalid_detector_type(self) -> None: ) ] + def test_no_resolution_condition(self) -> None: + data = { + **self.valid_data, + "conditionGroup": { + "id": self.data_condition_group.id, + "organizationId": self.organization.id, + "logicType": self.data_condition_group.logic_type, + "conditions": [ + { + "type": Condition.GREATER, + "comparison": 100, + "conditionResult": DetectorPriorityLevel.HIGH, + "conditionGroupId": self.data_condition_group.id, + }, + ], + }, + } + validator = MetricIssueDetectorValidator(data=data, context=self.context) + assert not validator.is_valid() + assert validator.errors.get("conditionGroup", {}).get("conditions") == [ + ErrorDetail( + string="Resolution condition required for metric issue detector.", code="invalid" + ) + ] + def test_too_many_conditions(self) -> None: data = { **self.valid_data, @@ -425,6 +456,12 @@ def test_too_many_conditions(self) -> None: "conditionResult": DetectorPriorityLevel.HIGH, "conditionGroupId": self.data_condition_group.id, }, + { + "type": Condition.LESS_OR_EQUAL, + "comparison": 100, + "conditionResult": DetectorPriorityLevel.OK, + "conditionGroupId": self.data_condition_group.id, + }, ], }, } From 54f99e6e5753221c82610d9c4712a22efeb9daa9 Mon Sep 17 00:00:00 2001 From: Michelle Fu Date: Thu, 6 Nov 2025 15:13:17 -0800 Subject: [PATCH 2/2] some more tests + a little fix --- src/sentry/incidents/metric_issue_detector.py | 2 +- .../test_organization_detector_details.py | 17 +++++++++++++++-- .../test_organization_detector_index.py | 10 ++++++++-- 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/src/sentry/incidents/metric_issue_detector.py b/src/sentry/incidents/metric_issue_detector.py index c3efde8b550b5b..841ec889b8f6e0 100644 --- a/src/sentry/incidents/metric_issue_detector.py +++ b/src/sentry/incidents/metric_issue_detector.py @@ -158,7 +158,7 @@ def validate(self, attrs): if "condition_group" in attrs: conditions = attrs.get("condition_group", {}).get("conditions") - if len(conditions) > 2: + if len(conditions) > 3: raise serializers.ValidationError("Too many conditions") return attrs diff --git a/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py b/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py index 10bdcd72030358..6e04e28a474229 100644 --- a/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py +++ b/tests/sentry/workflow_engine/endpoints/test_organization_detector_details.py @@ -79,6 +79,12 @@ def setUp(self) -> None: comparison=50, condition_result=DetectorPriorityLevel.LOW, ) + self.resolve_condition = self.create_data_condition( + condition_group=self.data_condition_group, + type=Condition.GREATER_OR_EQUAL, + comparison=50, + condition_result=DetectorPriorityLevel.OK, + ) self.detector = self.create_detector( project_id=self.project.id, name="Test Detector", @@ -180,6 +186,13 @@ def setUp(self) -> None: "conditionResult": DetectorPriorityLevel.HIGH, "conditionGroupId": self.condition.condition_group.id, }, + { + "id": self.resolve_condition.id, + "comparison": 100, + "type": Condition.LESS_OR_EQUAL, + "conditionResult": DetectorPriorityLevel.OK, + "conditionGroupId": self.condition.condition_group.id, + }, ], }, "config": self.detector.config, @@ -223,7 +236,7 @@ def test_update(self, mock_schedule_update_project_config: mock.MagicMock) -> No self.assert_condition_group_updated(condition_group) conditions = list(DataCondition.objects.filter(condition_group=condition_group)) - assert len(conditions) == 1 + assert len(conditions) == 2 condition = conditions[0] self.assert_data_condition_updated(condition) @@ -276,7 +289,7 @@ def test_update_add_data_condition(self) -> None: condition_group = detector.workflow_condition_group assert condition_group conditions = list(DataCondition.objects.filter(condition_group=condition_group)) - assert len(conditions) == 2 + assert len(conditions) == 3 def test_update_bad_schema(self) -> None: """ diff --git a/tests/sentry/workflow_engine/endpoints/test_organization_detector_index.py b/tests/sentry/workflow_engine/endpoints/test_organization_detector_index.py index d43fd3226a0905..d985e472665abc 100644 --- a/tests/sentry/workflow_engine/endpoints/test_organization_detector_index.py +++ b/tests/sentry/workflow_engine/endpoints/test_organization_detector_index.py @@ -727,7 +727,13 @@ def setUp(self) -> None: "comparison": 100, "conditionResult": DetectorPriorityLevel.HIGH, "conditionGroupId": self.data_condition_group.id, - } + }, + { + "type": Condition.LESS_OR_EQUAL, + "comparison": 100, + "conditionResult": DetectorPriorityLevel.OK, + "conditionGroupId": self.data_condition_group.id, + }, ], }, "config": { @@ -868,7 +874,7 @@ def test_valid_creation( assert condition_group.organization_id == self.organization.id conditions = list(DataCondition.objects.filter(condition_group=condition_group)) - assert len(conditions) == 1 + assert len(conditions) == 2 condition = conditions[0] assert condition.type == Condition.GREATER assert condition.comparison == 100