Browse Source

Revert "fix(hc): Silo fixes for OrganizationAlertRuleAvailableActionI… (#58091)

Rolling back https://github.com/getsentry/sentry/pull/57949 to address
SENTRY-16NH (https://sentry.sentry.io/issues/4544284664/)

The original fix should be mostly okay but `RpcSentryAppComponent`
should not have `Mapping[str, Any]` as a field, which seems to be the
root of the problem.
Ryan Skonnord 1 year ago
parent
commit
bdc6554002

+ 0 - 1
pyproject.toml

@@ -384,7 +384,6 @@ module = [
     "sentry.identity.vsts.provider",
     "sentry.incidents.action_handlers",
     "sentry.incidents.endpoints.bases",
-    "sentry.incidents.endpoints.organization_alert_rule_available_action_index",
     "sentry.incidents.endpoints.organization_alert_rule_details",
     "sentry.incidents.endpoints.organization_alert_rule_index",
     "sentry.incidents.endpoints.organization_incident_comment_details",

+ 15 - 22
src/sentry/incidents/endpoints/organization_alert_rule_available_action_index.py

@@ -1,7 +1,4 @@
-from __future__ import annotations
-
 from collections import defaultdict
-from typing import Any, DefaultDict, List, Mapping
 
 from rest_framework import status
 from rest_framework.request import Request
@@ -12,6 +9,7 @@ from sentry.api.api_publish_status import ApiPublishStatus
 from sentry.api.base import region_silo_endpoint
 from sentry.api.bases.organization import OrganizationEndpoint
 from sentry.api.exceptions import ResourceDoesNotExist
+from sentry.constants import SentryAppStatus
 from sentry.incidents.logic import (
     get_available_action_integrations_for_org,
     get_opsgenie_teams,
@@ -19,17 +17,12 @@ from sentry.incidents.logic import (
 )
 from sentry.incidents.models import AlertRuleTriggerAction
 from sentry.incidents.serializers import ACTION_TARGET_TYPE_TO_STRING
-from sentry.models.organization import Organization
-from sentry.services.hybrid_cloud.app import RpcSentryAppInstallation, app_service
-from sentry.services.hybrid_cloud.integration import RpcIntegration
+from sentry.models.integrations.sentry_app_installation import SentryAppInstallation
 
 
 def build_action_response(
-    registered_type,
-    integration: RpcIntegration | None = None,
-    organization: Organization | None = None,
-    sentry_app_installation: RpcSentryAppInstallation | None = None,
-) -> Mapping[str, Any]:
+    registered_type, integration=None, organization=None, sentry_app_installation=None
+):
     """
     Build the "available action" objects for the API. Each one can have different fields.
 
@@ -64,14 +57,14 @@ def build_action_response(
 
     elif sentry_app_installation:
         action_response["sentryAppName"] = sentry_app_installation.sentry_app.name
-        action_response["sentryAppId"] = sentry_app_installation.sentry_app.id
+        action_response["sentryAppId"] = sentry_app_installation.sentry_app_id
         action_response["sentryAppInstallationUuid"] = sentry_app_installation.uuid
-        action_response["status"] = sentry_app_installation.sentry_app.status
+        action_response["status"] = SentryAppStatus.as_str(
+            sentry_app_installation.sentry_app.status
+        )
 
         # Sentry Apps can be alertable but not have an Alert Rule UI Component
-        component = app_service.prepare_sentry_app_components(
-            installation_id=sentry_app_installation.id, component_type="alert-rule-action"
-        )
+        component = sentry_app_installation.prepare_sentry_app_components("alert-rule-action")
         if component:
             action_response["settings"] = component.schema.get("settings", {})
 
@@ -94,7 +87,7 @@ class OrganizationAlertRuleAvailableActionIndexEndpoint(OrganizationEndpoint):
         actions = []
 
         # Cache Integration objects in this data structure to save DB calls.
-        provider_integrations: DefaultDict[str, List[RpcIntegration]] = defaultdict(list)
+        provider_integrations = defaultdict(list)
         for integration in get_available_action_integrations_for_org(organization):
             provider_integrations[integration.provider].append(integration)
 
@@ -110,13 +103,13 @@ class OrganizationAlertRuleAvailableActionIndexEndpoint(OrganizationEndpoint):
 
             # Add all alertable SentryApps to the list.
             elif registered_type.type == AlertRuleTriggerAction.Type.SENTRY_APP:
-                installs = app_service.get_installed_for_organization(
-                    organization_id=organization.id
-                )
                 actions += [
                     build_action_response(registered_type, sentry_app_installation=install)
-                    for install in installs
-                    if install.sentry_app.is_alertable
+                    for install in SentryAppInstallation.objects.get_installed_for_organization(
+                        organization.id
+                    ).filter(
+                        sentry_app__is_alertable=True,
+                    )
                 ]
 
             else:

+ 5 - 7
src/sentry/incidents/logic.py

@@ -15,7 +15,7 @@ from snuba_sdk import Column, Condition, Limit, Op
 
 from sentry import analytics, audit_log, features, quotas
 from sentry.auth.access import SystemAccess
-from sentry.constants import CRASH_RATE_ALERT_AGGREGATE_ALIAS, ObjectStatus
+from sentry.constants import CRASH_RATE_ALERT_AGGREGATE_ALIAS
 from sentry.incidents import tasks
 from sentry.incidents.models import (
     AlertRule,
@@ -38,6 +38,7 @@ from sentry.incidents.models import (
     TriggerStatus,
 )
 from sentry.models.actor import Actor
+from sentry.models.integrations.integration import Integration
 from sentry.models.integrations.organization_integration import OrganizationIntegration
 from sentry.models.notificationaction import ActionService, ActionTarget
 from sentry.models.project import Project
@@ -1457,7 +1458,7 @@ def get_actions_for_trigger(trigger):
     return AlertRuleTriggerAction.objects.filter(alert_rule_trigger=trigger)
 
 
-def get_available_action_integrations_for_org(organization) -> List[RpcIntegration]:
+def get_available_action_integrations_for_org(organization):
     """
     Returns a list of integrations that the organization has installed. Integrations are
     filtered by the list of registered providers.
@@ -1471,11 +1472,8 @@ def get_available_action_integrations_for_org(organization) -> List[RpcIntegrati
         if registration.type != AlertRuleTriggerAction.Type.DISCORD
         or features.has("organizations:integrations-discord-metric-alerts", organization)
     ]
-    return integration_service.get_integrations(
-        status=ObjectStatus.ACTIVE,
-        org_integration_status=ObjectStatus.ACTIVE,
-        organization_id=organization.id,
-        providers=providers,
+    return Integration.objects.get_active_integrations(organization.id).filter(
+        provider__in=providers
     )
 
 

+ 17 - 11
src/sentry/models/integrations/sentry_app_installation.py

@@ -2,7 +2,7 @@ from __future__ import annotations
 
 import uuid
 from itertools import chain
-from typing import TYPE_CHECKING, Any, List, Mapping
+from typing import TYPE_CHECKING, Any, List
 
 from django.db import models, router, transaction
 from django.db.models import OuterRef, QuerySet, Subquery
@@ -19,7 +19,6 @@ from sentry.db.models import (
 )
 from sentry.db.models.fields.hybrid_cloud_foreign_key import HybridCloudForeignKey
 from sentry.services.hybrid_cloud.auth import AuthenticatedToken
-from sentry.services.hybrid_cloud.project import RpcProject
 from sentry.types.region import find_regions_for_orgs
 
 if TYPE_CHECKING:
@@ -196,12 +195,19 @@ class SentryAppInstallation(ParanoidModel):
             for region_name in find_regions_for_orgs([self.organization_id])
         ]
 
-    def prepare_ui_component(
-        self,
-        component: SentryAppComponent,
-        project: Project | RpcProject | None = None,
-        values: Any = None,
-    ) -> SentryAppComponent | None:
+    def prepare_sentry_app_components(self, component_type, project=None, values=None):
+        from sentry.models.integrations.sentry_app_component import SentryAppComponent
+
+        try:
+            component = SentryAppComponent.objects.get(
+                sentry_app_id=self.sentry_app_id, type=component_type
+            )
+        except SentryAppComponent.DoesNotExist:
+            return None
+
+        return self.prepare_ui_component(component, project, values)
+
+    def prepare_ui_component(self, component, project=None, values=None):
         return prepare_ui_component(
             self, component, project_slug=project.slug if project else None, values=values
         )
@@ -211,8 +217,8 @@ def prepare_sentry_app_components(
     installation: SentryAppInstallation,
     component_type: str,
     project_slug: str | None = None,
-    values: List[Mapping[str, Any]] | None = None,
-) -> SentryAppComponent | None:
+    values: Any = None,
+):
     from sentry.models.integrations.sentry_app_component import SentryAppComponent
 
     try:
@@ -229,7 +235,7 @@ def prepare_ui_component(
     installation: SentryAppInstallation,
     component: SentryAppComponent,
     project_slug: str | None = None,
-    values: List[Mapping[str, Any]] | None = None,
+    values: Any = None,
 ) -> SentryAppComponent | None:
     from sentry.coreapi import APIError
     from sentry.sentry_apps.components import SentryAppComponentPreparer

+ 6 - 11
src/sentry/services/hybrid_cloud/app/impl.py

@@ -28,7 +28,6 @@ from sentry.services.hybrid_cloud.app import (
 )
 from sentry.services.hybrid_cloud.app.serial import (
     serialize_sentry_app,
-    serialize_sentry_app_component,
     serialize_sentry_app_installation,
 )
 from sentry.services.hybrid_cloud.auth import AuthenticationContext
@@ -56,7 +55,12 @@ class DatabaseBackedAppService(AppService):
 
     def find_app_components(self, *, app_id: int) -> List[RpcSentryAppComponent]:
         return [
-            serialize_sentry_app_component(c)
+            RpcSentryAppComponent(
+                uuid=str(c.uuid),
+                sentry_app_id=c.sentry_app_id,
+                type=c.type,
+                app_schema=c.schema,
+            )
             for c in SentryAppComponent.objects.filter(sentry_app_id=app_id)
         ]
 
@@ -249,12 +253,3 @@ class DatabaseBackedAppService(AppService):
             installation = SentryAppInstallation.objects.get(sentry_app=sentry_app)
 
         return serialize_sentry_app_installation(installation=installation, app=sentry_app)
-
-    def prepare_sentry_app_components(
-        self, *, installation_id: int, component_type: str, project_slug: Optional[str] = None
-    ) -> Optional[RpcSentryAppComponent]:
-        from sentry.models.integrations.sentry_app_installation import prepare_sentry_app_components
-
-        installation = SentryAppInstallation.objects.get(id=installation_id)
-        component = prepare_sentry_app_components(installation, component_type, project_slug)
-        return serialize_sentry_app_component(component) if component else None

+ 0 - 1
src/sentry/services/hybrid_cloud/app/model.py

@@ -44,7 +44,6 @@ class RpcSentryApp(RpcModel):
     uuid: str = ""
     events: List[str] = Field(default_factory=list)
     webhook_url: Optional[str] = None
-    is_alertable: bool = False
     is_published: bool = False
     is_unpublished: bool = False
     is_internal: bool = True

+ 1 - 13
src/sentry/services/hybrid_cloud/app/serial.py

@@ -2,13 +2,11 @@ from typing import Optional
 
 from sentry.constants import SentryAppStatus
 from sentry.models.apiapplication import ApiApplication
-from sentry.models.integrations import SentryAppComponent
 from sentry.models.integrations.sentry_app import SentryApp
 from sentry.models.integrations.sentry_app_installation import SentryAppInstallation
 from sentry.services.hybrid_cloud.app import (
     RpcApiApplication,
     RpcSentryApp,
-    RpcSentryAppComponent,
     RpcSentryAppInstallation,
 )
 
@@ -36,12 +34,11 @@ def serialize_sentry_app(app: SentryApp) -> RpcSentryApp:
         uuid=app.uuid,
         events=app.events,
         webhook_url=app.webhook_url,
-        is_alertable=app.is_alertable,
         is_published=app.status == SentryAppStatus.PUBLISHED,
         is_unpublished=app.status == SentryAppStatus.UNPUBLISHED,
         is_internal=app.status == SentryAppStatus.INTERNAL,
         is_publish_request_inprogress=app.status == SentryAppStatus.PUBLISH_REQUEST_INPROGRESS,
-        status=SentryAppStatus.as_str(app.status),
+        status=app.status,
     )
 
 
@@ -61,12 +58,3 @@ def serialize_sentry_app_installation(
         uuid=installation.uuid,
         api_token=installation.api_token.token if installation.api_token else None,
     )
-
-
-def serialize_sentry_app_component(component: SentryAppComponent) -> RpcSentryAppComponent:
-    return RpcSentryAppComponent(
-        uuid=str(component.uuid),
-        sentry_app_id=component.sentry_app_id,
-        type=component.type,
-        app_schema=component.schema,
-    )

+ 0 - 7
src/sentry/services/hybrid_cloud/app/service.py

@@ -140,12 +140,5 @@ class AppService(RpcService):
     ) -> RpcSentryAppInstallation:
         pass
 
-    @rpc_method
-    @abc.abstractmethod
-    def prepare_sentry_app_components(
-        self, *, installation_id: int, component_type: str, project_slug: Optional[str] = None
-    ) -> Optional[RpcSentryAppComponent]:
-        pass
-
 
 app_service = cast(AppService, AppService.create_delegation())

+ 4 - 13
tests/sentry/incidents/endpoints/test_organization_alert_rule_available_action_index.py

@@ -5,7 +5,6 @@ from sentry.incidents.endpoints.organization_alert_rule_available_action_index i
 from sentry.incidents.models import AlertRuleTriggerAction
 from sentry.models.integrations.integration import Integration
 from sentry.models.integrations.organization_integration import OrganizationIntegration
-from sentry.services.hybrid_cloud.app.serial import serialize_sentry_app_installation
 from sentry.silo import SiloMode
 from sentry.testutils.cases import APITestCase
 from sentry.testutils.silo import assume_test_silo_mode, region_silo_test
@@ -26,7 +25,7 @@ METADATA = {
 }
 
 
-@region_silo_test(stable=True)
+@region_silo_test
 class OrganizationAlertRuleAvailableActionIndexEndpointTest(APITestCase):
     endpoint = "sentry-api-0-organization-alert-rule-available-actions"
     email = AlertRuleTriggerAction.get_registered_type(AlertRuleTriggerAction.Type.EMAIL)
@@ -110,9 +109,7 @@ class OrganizationAlertRuleAvailableActionIndexEndpointTest(APITestCase):
     def test_build_action_response_sentry_app(self):
         installation = self.install_new_sentry_app("foo")
 
-        data = build_action_response(
-            self.sentry_app, sentry_app_installation=serialize_sentry_app_installation(installation)
-        )
+        data = build_action_response(self.sentry_app, sentry_app_installation=installation)
 
         assert data["type"] == "sentry_app"
         assert data["allowedTargetTypes"] == ["sentry_app"]
@@ -183,10 +180,7 @@ class OrganizationAlertRuleAvailableActionIndexEndpointTest(APITestCase):
         assert len(response.data) == 2
         assert build_action_response(self.email) in response.data
         assert (
-            build_action_response(
-                self.sentry_app,
-                sentry_app_installation=serialize_sentry_app_installation(installation),
-            )
+            build_action_response(self.sentry_app, sentry_app_installation=installation)
             in response.data
         )
 
@@ -199,10 +193,7 @@ class OrganizationAlertRuleAvailableActionIndexEndpointTest(APITestCase):
 
         assert len(response.data) == 2
         assert (
-            build_action_response(
-                self.sentry_app,
-                sentry_app_installation=serialize_sentry_app_installation(installation),
-            )
+            build_action_response(self.sentry_app, sentry_app_installation=installation)
             in response.data
         )
 

+ 2 - 7
tests/sentry/incidents/test_logic.py

@@ -69,7 +69,6 @@ from sentry.incidents.models import (
 from sentry.models.actor import ActorTuple, get_actor_id_for_user
 from sentry.models.integrations.integration import Integration
 from sentry.models.integrations.organization_integration import OrganizationIntegration
-from sentry.services.hybrid_cloud.integration.serial import serialize_integration
 from sentry.shared_integrations.exceptions import ApiError, ApiRateLimitedError, ApiTimeoutError
 from sentry.snuba.dataset import Dataset
 from sentry.snuba.models import QuerySubscription, SnubaQuery, SnubaQueryEventType
@@ -1998,18 +1997,14 @@ class GetAvailableActionIntegrationsForOrgTest(TestCase):
     def test_registered(self):
         integration = Integration.objects.create(external_id="1", provider="slack")
         integration.add_organization(self.organization)
-        assert list(get_available_action_integrations_for_org(self.organization)) == [
-            serialize_integration(integration)
-        ]
+        assert list(get_available_action_integrations_for_org(self.organization)) == [integration]
 
     def test_mixed(self):
         integration = Integration.objects.create(external_id="1", provider="slack")
         integration.add_organization(self.organization)
         other_integration = Integration.objects.create(external_id="12345", provider="random")
         other_integration.add_organization(self.organization)
-        assert list(get_available_action_integrations_for_org(self.organization)) == [
-            serialize_integration(integration)
-        ]
+        assert list(get_available_action_integrations_for_org(self.organization)) == [integration]
 
     def test_disabled_integration(self):
         integration = Integration.objects.create(