diff --git a/src/sentry/deletions/defaults/sentry_app_installation.py b/src/sentry/deletions/defaults/sentry_app_installation.py index 410555ceff4445..8fe7906f714795 100644 --- a/src/sentry/deletions/defaults/sentry_app_installation.py +++ b/src/sentry/deletions/defaults/sentry_app_installation.py @@ -1,11 +1,8 @@ from collections.abc import Sequence -from sentry.constants import ObjectStatus from sentry.deletions.base import BaseRelation, ModelDeletionTask, ModelRelation from sentry.deletions.defaults.apigrant import ModelApiGrantDeletionTask from sentry.sentry_apps.models.sentry_app_installation import SentryAppInstallation -from sentry.types.region import RegionMappingNotFound -from sentry.workflow_engine.service.action import action_service class SentryAppInstallationDeletionTask(ModelDeletionTask[SentryAppInstallation]): @@ -28,15 +25,3 @@ def get_child_relations(self, instance: SentryAppInstallation) -> list[BaseRelat def mark_deletion_in_progress(self, instance_list: Sequence[SentryAppInstallation]) -> None: pass - - def delete_instance(self, instance: SentryAppInstallation) -> None: - try: - action_service.update_action_status_for_sentry_app_via_uuid( - organization_id=instance.organization_id, - status=ObjectStatus.DISABLED, - sentry_app_install_uuid=instance.uuid, - ) - except RegionMappingNotFound: - pass - - return super().delete_instance(instance) diff --git a/src/sentry/hybridcloud/outbox/category.py b/src/sentry/hybridcloud/outbox/category.py index 163db57dfb8f76..469caf0832f777 100644 --- a/src/sentry/hybridcloud/outbox/category.py +++ b/src/sentry/hybridcloud/outbox/category.py @@ -62,6 +62,7 @@ class OutboxCategory(IntEnum): SERVICE_HOOK_UPDATE = 40 SENTRY_APP_DELETE = 41 + SENTRY_APP_INSTALLATION_DELETE = 42 @classmethod def as_choices(cls) -> Sequence[tuple[int, int]]: @@ -304,6 +305,7 @@ class OutboxScope(IntEnum): OutboxCategory.SENTRY_APP_UPDATE, OutboxCategory.SERVICE_HOOK_UPDATE, OutboxCategory.SENTRY_APP_DELETE, + OutboxCategory.SENTRY_APP_INSTALLATION_DELETE, }, ) # No longer in use diff --git a/src/sentry/receivers/outbox/control.py b/src/sentry/receivers/outbox/control.py index b737918d27b38a..8693b3dc2be6c7 100644 --- a/src/sentry/receivers/outbox/control.py +++ b/src/sentry/receivers/outbox/control.py @@ -83,6 +83,31 @@ def process_sentry_app_deletes( # TODO: also update webhook actions using object identifier (sentry app slug) +@receiver(process_control_outbox, sender=OutboxCategory.SENTRY_APP_INSTALLATION_DELETE) +def process_sentry_app_installation_deletes( + shard_identifier: int, + object_identifier: int, + region_name: str, + payload: Mapping[str, Any], + **kwds: Any, +): + # This function should only be used when the sentry app is being deleted. + # Currently this receiver is only used for deletion. + if options.get("workflow_engine.sentry-app-actions-outbox"): + logger.info( + "sentry_app_installation_delete.update_action_status", + extra={ + "region_name": region_name, + "sentry_app_install_uuid": payload["uuid"], + }, + ) + action_service.update_action_status_for_sentry_app_via_uuid__region( + region_name=region_name, + status=ObjectStatus.DISABLED, + sentry_app_install_uuid=payload["uuid"], + ) + + @receiver(process_control_outbox, sender=OutboxCategory.API_APPLICATION_UPDATE) def process_api_application_updates(object_identifier: int, region_name: str, **kwds: Any): if ( diff --git a/src/sentry/sentry_apps/models/sentry_app.py b/src/sentry/sentry_apps/models/sentry_app.py index 547925b80c0d21..90fc208f69dffd 100644 --- a/src/sentry/sentry_apps/models/sentry_app.py +++ b/src/sentry/sentry_apps/models/sentry_app.py @@ -241,8 +241,8 @@ def delete(self, *args, **kwargs): for outbox in self.outboxes_for_delete(): outbox.save() - SentryAppAvatar.objects.filter(sentry_app=self).delete() - return super().delete(*args, **kwargs) + SentryAppAvatar.objects.filter(sentry_app=self).delete() + return super().delete(*args, **kwargs) def _disable(self): self.events = [] diff --git a/src/sentry/sentry_apps/models/sentry_app_installation.py b/src/sentry/sentry_apps/models/sentry_app_installation.py index 08091c6451de41..dde49caabf974c 100644 --- a/src/sentry/sentry_apps/models/sentry_app_installation.py +++ b/src/sentry/sentry_apps/models/sentry_app_installation.py @@ -4,7 +4,7 @@ from collections.abc import Collection, Mapping from typing import TYPE_CHECKING, Any, ClassVar, overload -from django.db import models +from django.db import models, router, transaction from django.utils import timezone from jsonschema import ValidationError @@ -16,9 +16,9 @@ from sentry.db.models.fields.hybrid_cloud_foreign_key import HybridCloudForeignKey from sentry.db.models.manager.base_query_set import BaseQuerySet from sentry.db.models.paranoia import ParanoidManager, ParanoidModel -from sentry.hybridcloud.models.outbox import ControlOutboxBase, outbox_context +from sentry.hybridcloud.models.outbox import ControlOutbox, ControlOutboxBase, outbox_context from sentry.hybridcloud.outbox.base import ReplicatedControlModel -from sentry.hybridcloud.outbox.category import OutboxCategory +from sentry.hybridcloud.outbox.category import OutboxCategory, OutboxScope from sentry.projects.services.project import RpcProject from sentry.sentry_apps.services.app.model import RpcSentryAppComponent, RpcSentryAppInstallation from sentry.sentry_apps.utils.errors import ( @@ -26,7 +26,7 @@ SentryAppIntegratorError, SentryAppSentryError, ) -from sentry.types.region import find_regions_for_orgs +from sentry.types.region import find_all_region_names, find_regions_for_orgs if TYPE_CHECKING: from sentry.models.project import Project @@ -124,6 +124,12 @@ def save(self, *args, **kwargs): self.date_updated = timezone.now() return super().save(*args, **kwargs) + def delete(self, *args, **kwargs): + with outbox_context(transaction.atomic(using=router.db_for_write(SentryAppInstallation))): + for outbox in self.outboxes_for_delete(): + outbox.save() + return super().delete(*args, **kwargs) + @property def api_application_id(self) -> int | None: from sentry.sentry_apps.models.sentry_app import SentryApp @@ -141,6 +147,19 @@ def outboxes_for_update(self, shard_identifier: int | None = None) -> list[Contr # these isn't so important in that case. return super().outboxes_for_update(shard_identifier=self.api_application_id or 0) + def outboxes_for_delete(self) -> list[ControlOutbox]: + return [ + ControlOutbox( + shard_scope=OutboxScope.APP_SCOPE, + shard_identifier=self.id, + object_identifier=self.id, + category=OutboxCategory.SENTRY_APP_INSTALLATION_DELETE, + region_name=region_name, + payload={"uuid": self.uuid}, + ) + for region_name in find_all_region_names() + ] + def prepare_ui_component( self, component: SentryAppComponent, diff --git a/tests/sentry/deletions/test_sentry_app_installations.py b/tests/sentry/deletions/test_sentry_app_installations.py index db78466beea6e6..33f3276950a735 100644 --- a/tests/sentry/deletions/test_sentry_app_installations.py +++ b/tests/sentry/deletions/test_sentry_app_installations.py @@ -17,6 +17,7 @@ from sentry.silo.base import SiloMode from sentry.silo.safety import unguarded_write from sentry.testutils.cases import TestCase +from sentry.testutils.helpers.options import override_options from sentry.testutils.outbox import outbox_runner from sentry.testutils.silo import assume_test_silo_mode, control_silo_test from sentry.workflow_engine.models import Action @@ -111,6 +112,7 @@ def test_soft_deletes_installation(self) -> None: assert c.fetchone()[0] == 1 + @override_options({"workflow_engine.sentry-app-actions-outbox": True}) def test_disables_actions(self) -> None: action = self.create_action( type=Action.Type.SENTRY_APP, @@ -129,6 +131,8 @@ def test_disables_actions(self) -> None: }, ) deletions.exec_sync(self.install) + with outbox_runner(): + pass action.refresh_from_db() assert action.status == ObjectStatus.DISABLED diff --git a/tests/sentry/sentry_apps/api/endpoints/test_organization_sentry_app_installation_details.py b/tests/sentry/sentry_apps/api/endpoints/test_organization_sentry_app_installation_details.py index 2ab3093073c185..fccd17bc5fe3a9 100644 --- a/tests/sentry/sentry_apps/api/endpoints/test_organization_sentry_app_installation_details.py +++ b/tests/sentry/sentry_apps/api/endpoints/test_organization_sentry_app_installation_details.py @@ -8,16 +8,21 @@ SentryAppInstallationUpdatedEvent, ) from sentry.analytics.events.sentry_app_uninstalled import SentryAppUninstalledEvent -from sentry.constants import SentryAppInstallationStatus +from sentry.constants import ObjectStatus, SentryAppInstallationStatus from sentry.deletions.tasks.scheduled import run_scheduled_deletions_control from sentry.models.auditlogentry import AuditLogEntry +from sentry.notifications.models.notificationaction import ActionTarget from sentry.sentry_apps.models.sentry_app_installation import SentryAppInstallation from sentry.sentry_apps.token_exchange.grant_exchanger import GrantExchanger from sentry.testutils.cases import APITestCase from sentry.testutils.helpers.analytics import assert_last_analytics_event +from sentry.testutils.helpers.options import override_options +from sentry.testutils.outbox import outbox_runner from sentry.testutils.silo import control_silo_test from sentry.users.services.user.service import user_service from sentry.utils import json +from sentry.workflow_engine.models.action import Action +from sentry.workflow_engine.typings.notification_action import SentryAppIdentifier class SentryAppInstallationDetailsTest(APITestCase): @@ -103,6 +108,7 @@ def test_no_access_outside_install_organization(self) -> None: class DeleteSentryAppInstallationDetailsTest(SentryAppInstallationDetailsTest): @responses.activate @patch("sentry.analytics.record") + @override_options({"workflow_engine.sentry-app-actions-outbox": True}) def test_delete_install(self, record: MagicMock) -> None: responses.add(url="https://example.com/webhook", method=responses.POST, body=b"") self.login_as(user=self.user) @@ -122,9 +128,24 @@ def test_delete_install(self, record: MagicMock) -> None: ), ) + action = self.create_action( + type=Action.Type.SENTRY_APP, + config={ + "target_identifier": self.installation2.uuid, + "sentry_app_identifier": SentryAppIdentifier.SENTRY_APP_INSTALLATION_UUID, + "target_type": ActionTarget.SENTRY_APP, + }, + ) + with self.tasks(): run_scheduled_deletions_control() + with outbox_runner(): + pass + + action.refresh_from_db() + assert action.status == ObjectStatus.DISABLED + assert not SentryAppInstallation.objects.filter(id=self.installation2.id).exists() response_body = json.loads(responses.calls[0].request.body)