Browse Source

fix(deletion) Handle delete, undelete, delete flow better (#28508)

Because organization deletion can be undone we have some users that are
either spamming the org deleteion endpoint or are deleting, undeleting
and then redeleting their organization. Using create_or_update should
make the integrity errors go away and do what customers were hoping to
do.

Fixes SENTRY-S54
Mark Story 3 years ago
parent
commit
8e61512d1b

+ 15 - 5
src/sentry/models/scheduledeletion.py

@@ -40,14 +40,24 @@ class ScheduledDeletion(Model):
 
     @classmethod
     def schedule(cls, instance, days=30, hours=0, data=None, actor=None):
-        record = cls.objects.create(
+        model_name = type(instance).__name__
+        record, created = cls.objects.create_or_update(
             app_label=instance._meta.app_label,
-            model_name=type(instance).__name__,
+            model_name=model_name,
             object_id=instance.pk,
-            date_scheduled=timezone.now() + timedelta(days=days, hours=hours),
-            data=data or {},
-            actor_id=actor.id if actor else None,
+            values={
+                "date_scheduled": timezone.now() + timedelta(days=days, hours=hours),
+                "data": data or {},
+                "actor_id": actor.id if actor else None,
+            },
         )
+        if not created:
+            record = cls.objects.get(
+                app_label=instance._meta.app_label,
+                model_name=model_name,
+                object_id=instance.pk,
+            )
+
         delete_logger.info(
             "object.delete.queued",
             extra={

+ 14 - 1
tests/sentry/api/endpoints/test_organization_details.py

@@ -701,7 +701,6 @@ class OrganizationDeleteTest(OrganizationDetailsTestBase):
     method = "delete"
 
     def test_can_remove_as_owner(self):
-
         owners = self.organization.get_owners()
         assert len(owners) > 0
 
@@ -745,6 +744,20 @@ class OrganizationDeleteTest(OrganizationDetailsTestBase):
         with self.settings(SENTRY_SINGLE_ORGANIZATION=True):
             self.get_error_response(org.slug, status_code=400)
 
+    def test_redo_deletion(self):
+        # Orgs can delete, undelete, delete within a day
+        org = self.create_organization(owner=self.user)
+        ScheduledDeletion.schedule(org, days=1)
+
+        self.get_success_response(org.slug)
+
+        org = Organization.objects.get(id=org.id)
+        assert org.status == OrganizationStatus.PENDING_DELETION
+
+        assert ScheduledDeletion.objects.filter(
+            object_id=org.id, model_name="Organization"
+        ).exists()
+
 
 class OrganizationSettings2FATest(TwoFactorAPITestCase):
     endpoint = "sentry-api-0-organization-details"

+ 12 - 0
tests/sentry/tasks/test_deletion.py

@@ -49,6 +49,18 @@ from sentry.testutils.helpers.datetime import before_now, iso_format
 
 
 class RunScheduledDeletionTest(TestCase):
+    def test_duplicate_schedule(self):
+        org = self.create_organization(name="test")
+        team = self.create_team(organization=org, name="delete")
+
+        first = ScheduledDeletion.schedule(team, days=0)
+        second = ScheduledDeletion.schedule(team, days=1)
+        # Should get the same record.
+        assert first.id == second.id
+        assert first.guid == second.guid
+        # Date should be updated
+        assert second.date_scheduled - first.date_scheduled >= timedelta(days=1)
+
     def test_simple(self):
         org = self.create_organization(name="test")
         team = self.create_team(organization=org, name="delete")