fix(deletions) Only fire pending_delete signal if deletion will happen

src/sentry/api/endpoints/organization_details.py

@@ -110,8 +110,6 @@
     ("apdexThreshold", "sentry:apdex_threshold", int, None),
 )
 
-delete_logger = logging.getLogger("sentry.deletions.api")
-
 DELETION_STATUSES = frozenset(
     [OrganizationStatus.PENDING_DELETION, OrganizationStatus.DELETION_IN_PROGRESS]
 )
@@ -503,10 +501,7 @@ def put(self, request, organization):
                     event=AuditLogEntryEvent.ORG_RESTORE,
                     data=organization.get_audit_log_data(),
                 )
-                delete_logger.info(
-                    "object.delete.canceled",
-                    extra={"object_id": organization.id, "model": Organization.__name__},
-                )
+                ScheduledDeletion.cancel(organization)
             elif changed_data:
                 self.create_audit_entry(
                     request=request,

src/sentry/models/scheduledeletion.py

@@ -67,6 +67,26 @@ def schedule(cls, instance, days=30, hours=0, data=None, actor=None):
         )
         return record
 
+    @classmethod
+    def cancel(cls, instance):
+        model_name = type(instance).__name__
+        try:
+            deletion = cls.objects.get(
+                model_name=model_name, object_id=instance.pk, in_progress=False
+            )
+        except cls.DoesNotExist:
+            delete_logger.info(
+                "object.delete.canceled.failed",
+                extra={"object_id": instance.pk, "model": model_name},
+            )
+            return
+
+        deletion.delete()
+        delete_logger.info(
+            "object.delete.canceled",
+            extra={"object_id": instance.pk, "model": model_name},
+        )
+
     def get_model(self):
         return apps.get_model(self.app_label, self.model_name)
 

src/sentry/tasks/deletion.py

@@ -84,10 +84,6 @@ def run_deletion(deletion_id, first_pass=True):
         deletion.delete()
         return
 
-    if first_pass:
-        actor = deletion.get_actor()
-        pending_delete.send(sender=type(instance), instance=instance, actor=actor)
-
     task = deletions.get(
         model=deletion.get_model(),
         query={"id": deletion.object_id},
@@ -106,6 +102,10 @@ def run_deletion(deletion_id, first_pass=True):
         deletion.delete()
         return
 
+    if first_pass:
+        actor = deletion.get_actor()
+        pending_delete.send(sender=type(instance), instance=instance, actor=actor)
+
     has_more = task.chunk()
     if has_more:
         run_deletion.apply_async(

tests/sentry/api/endpoints/test_organization_details.py

@@ -602,11 +602,15 @@ def test_empty_sensitive_fields(self):
 
     def test_cancel_delete(self):
         org = self.create_organization(owner=self.user, status=OrganizationStatus.PENDING_DELETION)
+        ScheduledDeletion.schedule(org, days=1)
 
         self.get_success_response(org.slug, **{"cancelDeletion": True})
 
         org = Organization.objects.get(id=org.id)
         assert org.status == OrganizationStatus.VISIBLE
+        assert not ScheduledDeletion.objects.filter(
+            model_name="Organization", object_id=org.id
+        ).exists()
 
     def test_relay_pii_config(self):
         value = '{"applications": {"freeform": []}}'

tests/sentry/tasks/test_deletion.py

@@ -23,6 +23,17 @@
 
 
 class RunScheduledDeletionTest(TestCase):
+    def test_schedule_and_cancel(self):
+        org = self.create_organization(name="test")
+        team = self.create_team(organization=org, name="delete")
+
+        schedule = ScheduledDeletion.schedule(team, days=0)
+        ScheduledDeletion.cancel(team)
+        assert not ScheduledDeletion.objects.filter(id=schedule.id).exists()
+
+        # No errors if we cancel a delete that wasn't started.
+        assert ScheduledDeletion.cancel(team) is None
+
     def test_duplicate_schedule(self):
         org = self.create_organization(name="test")
         team = self.create_team(organization=org, name="delete")
@@ -100,6 +111,22 @@ def test_triggers_pending_delete_signal(self):
         assert args["actor"] == self.user
         pending_delete.disconnect(signal_handler)
 
+    def test_no_pending_delete_trigger_on_skipped_delete(self):
+        org = self.create_organization(name="test")
+        project = self.create_project(organization=org)
+        repo = self.create_repo(project=project, name="example/example")
+
+        signal_handler = Mock()
+        pending_delete.connect(signal_handler)
+
+        ScheduledDeletion.schedule(instance=repo, actor=self.user, days=0)
+
+        with self.tasks():
+            run_scheduled_deletions()
+
+        pending_delete.disconnect(signal_handler)
+        assert signal_handler.call_count == 0
+
     def test_handle_missing_record(self):
         org = self.create_organization(name="test")
         team = self.create_team(organization=org, name="delete")