feat(deletions) Move organizationintegration deletes to ScheduledDeletion (#28803)

I missed this one in my previous inventory of custom deletion tasks.
I've also expanded the Repository relations to include code mappings as
there is another foreign key there that will block deletions.

src/sentry/api/bases/organization_integrations.py

@@ -33,7 +33,7 @@ class OrganizationIntegrationBaseEndpoint(IntegrationEndpoint):
     def get_organization_integration(organization, integration_id):
         """
         Get just the cross table entry.
-        Note: This will still return migrations that are pending deletion.
+        Note: This will still return organization integrations that are pending deletion.
 
         :param organization:
         :param integration_id:
@@ -58,7 +58,6 @@ class OrganizationIntegrationBaseEndpoint(IntegrationEndpoint):
         :return:
         """
         try:
-
             return Integration.objects.get(id=integration_id, organizations=organization)
         except Integration.DoesNotExist:
             raise Http404

src/sentry/api/endpoints/organization_integration_details.py

@@ -1,14 +1,18 @@
-from uuid import uuid4
-
+from django.db import transaction
 from rest_framework import serializers
 
 from sentry.api.bases.organization_integrations import OrganizationIntegrationBaseEndpoint
 from sentry.api.serializers import serialize
 from sentry.api.serializers.models.integration import OrganizationIntegrationSerializer
 from sentry.features.helpers import requires_feature
-from sentry.models import AuditLogEntryEvent, Integration, ObjectStatus, OrganizationIntegration
+from sentry.models import (
+    AuditLogEntryEvent,
+    Integration,
+    ObjectStatus,
+    OrganizationIntegration,
+    ScheduledDeletion,
+)
 from sentry.shared_integrations.exceptions import IntegrationError
-from sentry.tasks.deletion import delete_organization_integration
 from sentry.utils.audit import create_audit_entry
 
 
@@ -73,26 +77,20 @@ class OrganizationIntegrationDetailsEndpoint(OrganizationIntegrationBaseEndpoint
         # do any integration specific deleting steps
         integration.get_installation(organization.id).uninstall()
 
-        updated = OrganizationIntegration.objects.filter(
-            id=org_integration.id, status=ObjectStatus.VISIBLE
-        ).update(status=ObjectStatus.PENDING_DELETION)
-
-        if updated:
-            delete_organization_integration.apply_async(
-                kwargs={
-                    "object_id": org_integration.id,
-                    "transaction_id": uuid4().hex,
-                    "actor_id": request.user.id,
-                },
-                countdown=0,
-            )
-            create_audit_entry(
-                request=request,
-                organization=organization,
-                target_object=integration.id,
-                event=AuditLogEntryEvent.INTEGRATION_REMOVE,
-                data={"provider": integration.provider, "name": integration.name},
-            )
+        with transaction.atomic():
+            updated = OrganizationIntegration.objects.filter(
+                id=org_integration.id, status=ObjectStatus.VISIBLE
+            ).update(status=ObjectStatus.PENDING_DELETION)
+
+            if updated:
+                ScheduledDeletion.schedule(org_integration, days=0, actor=request.user)
+                create_audit_entry(
+                    request=request,
+                    organization=organization,
+                    target_object=integration.id,
+                    event=AuditLogEntryEvent.INTEGRATION_REMOVE,
+                    data={"provider": integration.provider, "name": integration.name},
+                )
 
         return self.respond(status=204)
 

src/sentry/deletions/defaults/organizationintegration.py

@@ -1,16 +1,48 @@
+from sentry.constants import ObjectStatus
+
 from ..base import ModelDeletionTask, ModelRelation
 
 
 class OrganizationIntegrationDeletionTask(ModelDeletionTask):
+    def should_proceed(self, instance):
+        return instance.status in {ObjectStatus.DELETION_IN_PROGRESS, ObjectStatus.PENDING_DELETION}
+
     def get_child_relations(self, instance):
-        from sentry.models import ExternalIssue
+        from sentry.models import (
+            ExternalIssue,
+            Identity,
+            IntegrationExternalProject,
+            PagerDutyService,
+            RepositoryProjectPathConfig,
+        )
 
-        return [
+        relations = [
             ModelRelation(
                 ExternalIssue,
                 {
                     "integration_id": instance.integration_id,
                     "organization_id": instance.organization_id,
                 },
-            )
+            ),
+            ModelRelation(IntegrationExternalProject, {"organization_integration_id": instance.id}),
+            ModelRelation(PagerDutyService, {"organization_integration_id": instance.id}),
+            ModelRelation(
+                RepositoryProjectPathConfig, {"organization_integration_id": instance.id}
+            ),
         ]
+
+        # delete the identity attached through the default_auth_id
+        if instance.default_auth_id:
+            relations.append(ModelRelation(Identity, {"id": instance.default_auth_id}))
+
+        return relations
+
+    def delete_instance(self, instance):
+        from sentry.models import Repository
+
+        # Dissociate repos from the integration being deleted. integration
+        Repository.objects.filter(
+            organization_id=instance.organization_id, integration_id=instance.integration_id
+        ).update(integration_id=None)
+
+        return super().delete_instance(instance)

src/sentry/deletions/defaults/repository.py

@@ -12,11 +12,12 @@ class RepositoryDeletionTask(ModelDeletionTask):
         return instance.status in {ObjectStatus.PENDING_DELETION, ObjectStatus.DELETION_IN_PROGRESS}
 
     def get_child_relations(self, instance):
-        from sentry.models import Commit, PullRequest
+        from sentry.models import Commit, PullRequest, RepositoryProjectPathConfig
 
         return [
             ModelRelation(Commit, {"repository_id": instance.id}),
             ModelRelation(PullRequest, {"repository_id": instance.id}),
+            ModelRelation(RepositoryProjectPathConfig, {"repository_id": instance.id}),
         ]
 
     def delete_instance(self, instance):

src/sentry/integrations/base.py

@@ -383,7 +383,7 @@ class IntegrationInstallation:
     def uninstall(self) -> None:
         """
         For integrations that need additional steps for uninstalling
-        that are not covered in the `delete_organization_integration`
+        that are not covered by the deletion task for OrganizationIntegration
         task.
         """
         pass

src/sentry/testutils/fixtures.py

@@ -357,6 +357,12 @@ class Fixtures:
 
         return integration
 
+    def create_identity(self, *args, **kwargs):
+        return Factories.create_identity(*args, **kwargs)
+
+    def create_identity_provider(self, *args, **kwargs):
+        return Factories.create_identity_provider(*args, **kwargs)
+
     @pytest.fixture(autouse=True)
     def _init_insta_snapshot(self, insta_snapshot):
         self.insta_snapshot = insta_snapshot

tests/sentry/api/endpoints/test_organization_integration_details.py

@@ -4,6 +4,7 @@ from sentry.models import (
     Integration,
     OrganizationIntegration,
     Repository,
+    ScheduledDeletion,
 )
 from sentry.testutils import APITestCase
 from sentry.testutils.helpers import with_feature
@@ -61,30 +62,15 @@ class OrganizationIntegrationDetailsDeleteTest(OrganizationIntegrationDetailsTes
     method = "delete"
 
     def test_removal(self):
-        with self.tasks():
-            self.get_success_response(self.organization.slug, self.integration.id)
-            assert Integration.objects.filter(id=self.integration.id).exists()
-
-            # Ensure Organization integrations are removed
-            assert not OrganizationIntegration.objects.filter(
-                integration=self.integration, organization=self.organization
-            ).exists()
-            assert not Identity.objects.filter(user=self.user).exists()
-
-            # make sure repo is dissociated from integration
-            assert Repository.objects.get(id=self.repo.id).integration_id is None
-
-    def test_removal_default_identity_already_removed(self):
-        with self.tasks():
-            self.identity.delete()
-            self.get_success_response(self.organization.slug, self.integration.id)
-
-            assert Integration.objects.filter(id=self.integration.id).exists()
-
-            # Ensure Organization integrations are removed
-            assert not OrganizationIntegration.objects.filter(
-                integration=self.integration, organization=self.organization
-            ).exists()
+        self.get_success_response(self.organization.slug, self.integration.id)
+        assert Integration.objects.filter(id=self.integration.id).exists()
+
+        org_integration = OrganizationIntegration.objects.get(
+            integration=self.integration, organization=self.organization
+        )
+        assert ScheduledDeletion.objects.filter(
+            model_name="OrganizationIntegration", object_id=org_integration.id
+        )
 
 
 class OrganizationIntegrationDetailsPutTest(OrganizationIntegrationDetailsTest):

tests/sentry/deletions/test_organizationintegration.py

@@ -1,4 +1,13 @@
-from sentry.models import ExternalIssue, Integration, OrganizationIntegration, ScheduledDeletion
+from sentry.constants import ObjectStatus
+from sentry.models import (
+    ExternalIssue,
+    Identity,
+    Integration,
+    OrganizationIntegration,
+    Project,
+    Repository,
+    ScheduledDeletion,
+)
 from sentry.tasks.deletion import run_deletion
 from sentry.testutils import TransactionTestCase
 
@@ -7,14 +16,12 @@ class DeleteOrganizationIntegrationTest(TransactionTestCase):
     def test_simple(self):
         org = self.create_organization()
         integration = Integration.objects.create(provider="example", name="Example")
-        integration.add_organization(org, self.user)
-        organization_integration = OrganizationIntegration.objects.get(
-            integration_id=integration.id, organization_id=org.id
-        )
+        organization_integration = integration.add_organization(org, self.user)
         external_issue = ExternalIssue.objects.create(
             organization_id=org.id, integration_id=integration.id, key="ABC-123"
         )
 
+        organization_integration.update(status=ObjectStatus.PENDING_DELETION)
         deletion = ScheduledDeletion.schedule(organization_integration, days=0)
         deletion.update(in_progress=True)
 
@@ -23,3 +30,48 @@ class DeleteOrganizationIntegrationTest(TransactionTestCase):
 
         assert not OrganizationIntegration.objects.filter(id=organization_integration.id).exists()
         assert not ExternalIssue.objects.filter(id=external_issue.id).exists()
+
+    def test_skip_on_undelete(self):
+        org = self.create_organization()
+        integration = Integration.objects.create(provider="example", name="Example")
+        organization_integration = integration.add_organization(org, self.user)
+
+        deletion = ScheduledDeletion.schedule(organization_integration, days=0)
+        deletion.update(in_progress=True)
+
+        with self.tasks():
+            run_deletion(deletion.id)
+
+        assert OrganizationIntegration.objects.filter(id=organization_integration.id).exists()
+
+    def test_repository_and_identity(self):
+        org = self.create_organization()
+        project = self.create_project(organization=org)
+        integration = Integration.objects.create(provider="example", name="Example")
+        provider = self.create_identity_provider(integration)
+        identity = self.create_identity(
+            user=self.user, identity_provider=provider, external_id="abc123"
+        )
+        organization_integration = integration.add_organization(org, self.user, identity.id)
+        repository = self.create_repo(
+            project=project, name="testrepo", provider="gitlab", integration_id=integration.id
+        )
+
+        external_issue = ExternalIssue.objects.create(
+            organization_id=org.id, integration_id=integration.id, key="ABC-123"
+        )
+        organization_integration.update(status=ObjectStatus.PENDING_DELETION)
+        deletion = ScheduledDeletion.schedule(organization_integration, days=0)
+        deletion.update(in_progress=True)
+
+        with self.tasks():
+            run_deletion(deletion.id)
+
+        assert Integration.objects.filter(id=integration.id).exists()
+        assert Project.objects.filter(id=project.id).exists()
+        assert not OrganizationIntegration.objects.filter(id=organization_integration.id).exists()
+        assert not ExternalIssue.objects.filter(id=external_issue.id).exists()
+        assert not Identity.objects.filter(id=identity.id).exists()
+
+        repo = Repository.objects.get(id=repository.id)
+        assert repo.integration_id is None

tests/sentry/integrations/slack/test_uninstall.py

@@ -1,6 +1,14 @@
 from typing import Optional
 
-from sentry.models import Integration, NotificationSetting, OrganizationIntegration, Project, User
+from sentry.constants import ObjectStatus
+from sentry.models import (
+    Integration,
+    NotificationSetting,
+    OrganizationIntegration,
+    Project,
+    ScheduledDeletion,
+    User,
+)
 from sentry.notifications.helpers import NOTIFICATION_SETTING_DEFAULTS
 from sentry.notifications.types import NotificationSettingOptionValues, NotificationSettingTypes
 from sentry.testutils import APITestCase
@@ -18,9 +26,9 @@ class SlackUninstallTest(APITestCase):
         self.login_as(self.user)
 
     def uninstall(self) -> None:
-        assert OrganizationIntegration.objects.filter(
+        org_integration = OrganizationIntegration.objects.get(
             integration=self.integration, organization=self.organization
-        ).exists()
+        )
 
         with self.tasks():
             self.get_success_response(self.organization.slug, self.integration.id)
@@ -28,7 +36,12 @@ class SlackUninstallTest(APITestCase):
         assert Integration.objects.filter(id=self.integration.id).exists()
 
         assert not OrganizationIntegration.objects.filter(
-            integration=self.integration, organization=self.organization
+            integration=self.integration,
+            organization=self.organization,
+            status=ObjectStatus.VISIBLE,
+        ).exists()
+        assert ScheduledDeletion.objects.filter(
+            model_name="OrganizationIntegration", object_id=org_integration.id
         ).exists()
 
     def get_setting(

tests/sentry/integrations/vercel/test_integration.py

@@ -3,6 +3,7 @@ from urllib.parse import parse_qs
 import responses
 from rest_framework.serializers import ValidationError
 
+from sentry.constants import ObjectStatus
 from sentry.integrations.vercel import VercelIntegrationProvider
 from sentry.models import (
     Integration,
@@ -10,6 +11,7 @@ from sentry.models import (
     Project,
     ProjectKey,
     ProjectKeyStatus,
+    ScheduledDeletion,
     SentryAppInstallation,
     SentryAppInstallationForProvider,
 )
@@ -395,4 +397,10 @@ class VercelIntegrationTest(IntegrationTestCase):
 
         # deleting the integration only happens when we get the Vercel webhook
         integration = Integration.objects.get(provider=self.provider.key)
-        assert not OrganizationIntegration.objects.filter(integration_id=integration.id).exists()
+        org_integration = OrganizationIntegration.objects.get(
+            integration_id=integration.id, organization_id=self.organization.id
+        )
+        assert org_integration.status == ObjectStatus.PENDING_DELETION
+        assert ScheduledDeletion.objects.filter(
+            model_name="OrganizationIntegration", object_id=org_integration.id
+        ).exists()