wrap deletion in transaction

getsentry · Mar 3, 2025 · ef10021 · ef10021
1 parent 5975097
commit ef10021
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 14 deletions.
diff --git a/src/sentry/api/endpoints/project_rule_details.py b/src/sentry/api/endpoints/project_rule_details.py
@@ -2,6 +2,7 @@
 
 import logging
 
+from django.db import router, transaction
 from drf_spectacular.utils import extend_schema
 from rest_framework import serializers, status
 from rest_framework.request import Request
@@ -31,7 +32,7 @@
 from sentry.integrations.jira_server.actions.create_ticket import JiraServerCreateTicketAction
 from sentry.integrations.slack.tasks.find_channel_id_for_rule import find_channel_id_for_rule
 from sentry.integrations.slack.utils.rule_status import RedisRuleStatus
-from sentry.models.rule import NeglectedRule, RuleActivity, RuleActivityType
+from sentry.models.rule import NeglectedRule, Rule, RuleActivity, RuleActivityType
 from sentry.projects.project_rules.updater import ProjectRuleUpdater
 from sentry.rules.actions import trigger_sentry_app_action_creators_for_issues
 from sentry.rules.actions.utils import get_changed_data, get_updated_rule_data
@@ -367,20 +368,22 @@ def delete(self, request: Request, project, rule) -> Response:
         - Actions: specify what should happen when the trigger conditions are met and the filters match.
         """
         rule_id = rule.id
-        rule.update(status=ObjectStatus.PENDING_DELETION)
-        RuleActivity.objects.create(
-            rule=rule, user_id=request.user.id, type=RuleActivityType.DELETED.value
-        )
-        scheduled = RegionScheduledDeletion.schedule(rule, days=0, actor=request.user)
-
-        if features.has(
-            "organizations:workflow-engine-issue-alert-dual-write", project.organization
-        ):
-            workflow_id = delete_migrated_issue_alert(rule)
-            logger.info(
-                "workflow_engine.issue_alert.deleted",
-                extra={"rule_id": rule_id, "workflow_id": workflow_id},
+
+        with transaction.atomic(router.db_for_write(Rule)):
+            rule.update(status=ObjectStatus.PENDING_DELETION)
+            RuleActivity.objects.create(
+                rule=rule, user_id=request.user.id, type=RuleActivityType.DELETED.value
             )
+            scheduled = RegionScheduledDeletion.schedule(rule, days=0, actor=request.user)
+
+            if features.has(
+                "organizations:workflow-engine-issue-alert-dual-write", project.organization
+            ):
+                workflow_id = delete_migrated_issue_alert(rule)
+                logger.info(
+                    "workflow_engine.issue_alert.deleted",
+                    extra={"rule_id": rule_id, "workflow_id": workflow_id},
+                )
 
         self.create_audit_entry(
             request=request,

diff --git a/tests/sentry/api/endpoints/test_project_rule_details.py b/tests/sentry/api/endpoints/test_project_rule_details.py
@@ -1703,6 +1703,32 @@ def test_dual_delete_workflow_engine(self):
         assert not DataCondition.objects.filter(condition_group=when_dcg).exists()
         assert not DataCondition.objects.filter(condition_group=if_dcg).exists()
 
+    @with_feature("organizations:workflow-engine-issue-alert-dual-write")
+    @patch("sentry.api.endpoints.project_rule_details.delete_migrated_issue_alert")
+    def test_dual_delete_workflow_engine__fail(self, mock_delete):
+        rule = self.create_project_rule(
+            self.project,
+            condition_data=[
+                {
+                    "id": "sentry.rules.conditions.first_seen_event.FirstSeenEventCondition",
+                    "name": "A new issue is created",
+                },
+                {
+                    "id": "sentry.rules.filters.latest_release.LatestReleaseFilter",
+                    "name": "The event occurs",
+                },
+            ],
+        )
+        IssueAlertMigrator(rule, user_id=self.user.id).run()
+
+        mock_delete.side_effect = Exception("oh no")
+
+        # failed workflow engine deletion should stop the rule from being deleted
+        self.get_error_response(self.organization.slug, rule.project.slug, rule.id, status_code=500)
+
+        rule.refresh_from_db()
+        assert rule
+
     def test_dual_delete_workflow_engine_no_migrated_models(self):
         rule = self.create_project_rule(self.project)
         self.get_success_response(