Skip to content

Commit

Permalink
fix(aci): handle cases found in dry run (#85600)
Browse files Browse the repository at this point in the history
  • Loading branch information
cathteng authored Feb 20, 2025
1 parent 58ce000 commit 99e4e28
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ def create_new_high_priority_issue_data_condition(
@data_condition_translator_registry.register(LevelCondition.id)
@data_condition_translator_registry.register(LevelFilter.id)
def create_level_data_condition(data: dict[str, Any], dcg: DataConditionGroup) -> DataCondition:
comparison = {"match": data["match"], "level": data["level"]}
comparison = {"match": data["match"], "level": int(data["level"])}

return DataCondition(
type=Condition.LEVEL,
Expand Down Expand Up @@ -204,7 +204,7 @@ def create_issue_category_data_condition(
data: dict[str, Any], dcg: DataConditionGroup
) -> DataCondition:
comparison = {
"value": data["value"],
"value": int(data["value"]),
}

return DataCondition(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import logging
from typing import Any

from sentry.constants import ObjectStatus
from sentry.grouping.grouptype import ErrorGroupType
from sentry.models.rule import Rule
from sentry.models.rulesnooze import RuleSnooze
Expand Down Expand Up @@ -56,11 +57,11 @@ def run(self) -> None:
workflow = self._create_workflow_and_lookup(
conditions=conditions,
filters=filters,
action_match=self.data["action_match"],
action_match=self.data.get("action_match", Rule.DEFAULT_CONDITION_MATCH),
detector=error_detector,
)
if_dcg = self._create_if_dcg(
filter_match=self.data.get("filter_match", "all"),
filter_match=self.data.get("filter_match", Rule.DEFAULT_FILTER_MATCH),
workflow=workflow,
conditions=conditions,
filters=filters,
Expand Down Expand Up @@ -162,6 +163,8 @@ def _create_workflow_and_lookup(
rule_snooze = RuleSnooze.objects.filter(rule=self.rule, user_id=None).first()
if rule_snooze and rule_snooze.until is None:
enabled = False
if self.rule.status == ObjectStatus.DISABLED:
enabled = False

config = {"frequency": self.rule.data.get("frequency") or Workflow.DEFAULT_FREQUENCY}
kwargs = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class TestIssueCategoryCondition(ConditionTestCase):
condition = Condition.ISSUE_CATEGORY
payload = {
"id": IssueCategoryFilter.id,
"value": 1,
"value": "1",
}

def setUp(self):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class TestLevelCondition(ConditionTestCase):
payload = {
"id": LevelCondition.id,
"match": MatchType.EQUAL,
"level": 20,
"level": "20",
}

def setup_group_event_and_job(self):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import pytest
from jsonschema.exceptions import ValidationError

from sentry.constants import ObjectStatus
from sentry.grouping.grouptype import ErrorGroupType
from sentry.models.rulesnooze import RuleSnooze
from sentry.rules.age import AgeComparisonType
Expand Down Expand Up @@ -406,7 +407,9 @@ def assert_nothing_migrated(self, issue_alert):
assert DataCondition.objects.all().count() == 0
assert Action.objects.all().count() == 0

def assert_issue_alert_migrated(self, issue_alert, is_enabled=True):
def assert_issue_alert_migrated(
self, issue_alert, is_enabled=True, logic_type=DataConditionGroup.Type.ANY_SHORT_CIRCUIT
):
issue_alert_workflow = AlertRuleWorkflow.objects.get(rule=issue_alert)
issue_alert_detector = AlertRuleDetector.objects.get(rule=issue_alert)

Expand All @@ -431,7 +434,7 @@ def assert_issue_alert_migrated(self, issue_alert, is_enabled=True):
assert detector_workflow.workflow == workflow

assert workflow.when_condition_group
assert workflow.when_condition_group.logic_type == DataConditionGroup.Type.ANY_SHORT_CIRCUIT
assert workflow.when_condition_group.logic_type == logic_type
conditions = DataCondition.objects.filter(condition_group=workflow.when_condition_group)
assert conditions.count() == 2
assert conditions.filter(
Expand All @@ -442,7 +445,7 @@ def assert_issue_alert_migrated(self, issue_alert, is_enabled=True):
).exists()

if_dcg = WorkflowDataConditionGroup.objects.get(workflow=workflow).condition_group
assert if_dcg.logic_type == DataConditionGroup.Type.ANY_SHORT_CIRCUIT
assert if_dcg.logic_type == logic_type
filters = DataCondition.objects.filter(condition_group=if_dcg)
assert filters.count() == 1
assert filters.filter(
Expand All @@ -464,6 +467,28 @@ def test_run(self):
action = dcg_actions.action
assert action.type == Action.Type.SLACK

def test_run__missing_matches(self):
data = self.issue_alert.data
del data["action_match"]
del data["filter_match"]
IssueAlertMigrator(self.issue_alert, self.user.id).run()

self.assert_issue_alert_migrated(self.issue_alert, logic_type=DataConditionGroup.Type.ALL)

dcg_actions = DataConditionGroupAction.objects.all()[0]
action = dcg_actions.action
assert action.type == Action.Type.SLACK

def test_run__disabled_rule(self):
self.issue_alert.update(status=ObjectStatus.DISABLED)
IssueAlertMigrator(self.issue_alert, self.user.id).run()

self.assert_issue_alert_migrated(self.issue_alert, is_enabled=False)

dcg_actions = DataConditionGroupAction.objects.all()[0]
action = dcg_actions.action
assert action.type == Action.Type.SLACK

def test_run__snoozed_rule(self):
RuleSnooze.objects.create(rule=self.issue_alert)
IssueAlertMigrator(self.issue_alert, self.user.id).run()
Expand Down

0 comments on commit 99e4e28

Please sign in to comment.