Skip to content

Commit 66ba35e

Browse files
committed
Revert "fix(hc): Silo fixes for OrganizationAlertRuleAvailableActionIndexEndpoint (#57949)"
This reverts commit 53edebb.
1 parent 7748425 commit 66ba35e

File tree

9 files changed

+50
-92
lines changed

9 files changed

+50
-92
lines changed

Diff for: src/sentry/incidents/endpoints/organization_alert_rule_available_action_index.py

+15-22
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,4 @@
1-
from __future__ import annotations
2-
31
from collections import defaultdict
4-
from typing import Any, DefaultDict, List, Mapping
52

63
from rest_framework import status
74
from rest_framework.request import Request
@@ -12,24 +9,20 @@
129
from sentry.api.base import region_silo_endpoint
1310
from sentry.api.bases.organization import OrganizationEndpoint
1411
from sentry.api.exceptions import ResourceDoesNotExist
12+
from sentry.constants import SentryAppStatus
1513
from sentry.incidents.logic import (
1614
get_available_action_integrations_for_org,
1715
get_opsgenie_teams,
1816
get_pagerduty_services,
1917
)
2018
from sentry.incidents.models import AlertRuleTriggerAction
2119
from sentry.incidents.serializers import ACTION_TARGET_TYPE_TO_STRING
22-
from sentry.models.organization import Organization
23-
from sentry.services.hybrid_cloud.app import RpcSentryAppInstallation, app_service
24-
from sentry.services.hybrid_cloud.integration import RpcIntegration
20+
from sentry.models.integrations.sentry_app_installation import SentryAppInstallation
2521

2622

2723
def build_action_response(
28-
registered_type,
29-
integration: RpcIntegration | None = None,
30-
organization: Organization | None = None,
31-
sentry_app_installation: RpcSentryAppInstallation | None = None,
32-
) -> Mapping[str, Any]:
24+
registered_type, integration=None, organization=None, sentry_app_installation=None
25+
):
3326
"""
3427
Build the "available action" objects for the API. Each one can have different fields.
3528
@@ -64,14 +57,14 @@ def build_action_response(
6457

6558
elif sentry_app_installation:
6659
action_response["sentryAppName"] = sentry_app_installation.sentry_app.name
67-
action_response["sentryAppId"] = sentry_app_installation.sentry_app.id
60+
action_response["sentryAppId"] = sentry_app_installation.sentry_app_id
6861
action_response["sentryAppInstallationUuid"] = sentry_app_installation.uuid
69-
action_response["status"] = sentry_app_installation.sentry_app.status
62+
action_response["status"] = SentryAppStatus.as_str(
63+
sentry_app_installation.sentry_app.status
64+
)
7065

7166
# Sentry Apps can be alertable but not have an Alert Rule UI Component
72-
component = app_service.prepare_sentry_app_components(
73-
installation_id=sentry_app_installation.id, component_type="alert-rule-action"
74-
)
67+
component = sentry_app_installation.prepare_sentry_app_components("alert-rule-action")
7568
if component:
7669
action_response["settings"] = component.schema.get("settings", {})
7770

@@ -94,7 +87,7 @@ def get(self, request: Request, organization) -> Response:
9487
actions = []
9588

9689
# Cache Integration objects in this data structure to save DB calls.
97-
provider_integrations: DefaultDict[str, List[RpcIntegration]] = defaultdict(list)
90+
provider_integrations = defaultdict(list)
9891
for integration in get_available_action_integrations_for_org(organization):
9992
provider_integrations[integration.provider].append(integration)
10093

@@ -110,13 +103,13 @@ def get(self, request: Request, organization) -> Response:
110103

111104
# Add all alertable SentryApps to the list.
112105
elif registered_type.type == AlertRuleTriggerAction.Type.SENTRY_APP:
113-
installs = app_service.get_installed_for_organization(
114-
organization_id=organization.id
115-
)
116106
actions += [
117107
build_action_response(registered_type, sentry_app_installation=install)
118-
for install in installs
119-
if install.sentry_app.is_alertable
108+
for install in SentryAppInstallation.objects.get_installed_for_organization(
109+
organization.id
110+
).filter(
111+
sentry_app__is_alertable=True,
112+
)
120113
]
121114

122115
else:

Diff for: src/sentry/incidents/logic.py

+5-7
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515

1616
from sentry import analytics, audit_log, features, quotas
1717
from sentry.auth.access import SystemAccess
18-
from sentry.constants import CRASH_RATE_ALERT_AGGREGATE_ALIAS, ObjectStatus
18+
from sentry.constants import CRASH_RATE_ALERT_AGGREGATE_ALIAS
1919
from sentry.incidents import tasks
2020
from sentry.incidents.models import (
2121
AlertRule,
@@ -38,6 +38,7 @@
3838
TriggerStatus,
3939
)
4040
from sentry.models.actor import Actor
41+
from sentry.models.integrations.integration import Integration
4142
from sentry.models.integrations.organization_integration import OrganizationIntegration
4243
from sentry.models.notificationaction import ActionService, ActionTarget
4344
from sentry.models.project import Project
@@ -1457,7 +1458,7 @@ def get_actions_for_trigger(trigger):
14571458
return AlertRuleTriggerAction.objects.filter(alert_rule_trigger=trigger)
14581459

14591460

1460-
def get_available_action_integrations_for_org(organization) -> List[RpcIntegration]:
1461+
def get_available_action_integrations_for_org(organization):
14611462
"""
14621463
Returns a list of integrations that the organization has installed. Integrations are
14631464
filtered by the list of registered providers.
@@ -1471,11 +1472,8 @@ def get_available_action_integrations_for_org(organization) -> List[RpcIntegrati
14711472
if registration.type != AlertRuleTriggerAction.Type.DISCORD
14721473
or features.has("organizations:integrations-discord-metric-alerts", organization)
14731474
]
1474-
return integration_service.get_integrations(
1475-
status=ObjectStatus.ACTIVE,
1476-
org_integration_status=ObjectStatus.ACTIVE,
1477-
organization_id=organization.id,
1478-
providers=providers,
1475+
return Integration.objects.get_active_integrations(organization.id).filter(
1476+
provider__in=providers
14791477
)
14801478

14811479

Diff for: src/sentry/models/integrations/sentry_app_installation.py

+17-11
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
import uuid
44
from itertools import chain
5-
from typing import TYPE_CHECKING, Any, List, Mapping
5+
from typing import TYPE_CHECKING, Any, List
66

77
from django.db import models, router, transaction
88
from django.db.models import OuterRef, QuerySet, Subquery
@@ -19,7 +19,6 @@
1919
)
2020
from sentry.db.models.fields.hybrid_cloud_foreign_key import HybridCloudForeignKey
2121
from sentry.services.hybrid_cloud.auth import AuthenticatedToken
22-
from sentry.services.hybrid_cloud.project import RpcProject
2322
from sentry.types.region import find_regions_for_orgs
2423

2524
if TYPE_CHECKING:
@@ -196,12 +195,19 @@ def outboxes_for_update(self) -> List[ControlOutbox]:
196195
for region_name in find_regions_for_orgs([self.organization_id])
197196
]
198197

199-
def prepare_ui_component(
200-
self,
201-
component: SentryAppComponent,
202-
project: Project | RpcProject | None = None,
203-
values: Any = None,
204-
) -> SentryAppComponent | None:
198+
def prepare_sentry_app_components(self, component_type, project=None, values=None):
199+
from sentry.models.integrations.sentry_app_component import SentryAppComponent
200+
201+
try:
202+
component = SentryAppComponent.objects.get(
203+
sentry_app_id=self.sentry_app_id, type=component_type
204+
)
205+
except SentryAppComponent.DoesNotExist:
206+
return None
207+
208+
return self.prepare_ui_component(component, project, values)
209+
210+
def prepare_ui_component(self, component, project=None, values=None):
205211
return prepare_ui_component(
206212
self, component, project_slug=project.slug if project else None, values=values
207213
)
@@ -211,8 +217,8 @@ def prepare_sentry_app_components(
211217
installation: SentryAppInstallation,
212218
component_type: str,
213219
project_slug: str | None = None,
214-
values: List[Mapping[str, Any]] | None = None,
215-
) -> SentryAppComponent | None:
220+
values: Any = None,
221+
):
216222
from sentry.models.integrations.sentry_app_component import SentryAppComponent
217223

218224
try:
@@ -229,7 +235,7 @@ def prepare_ui_component(
229235
installation: SentryAppInstallation,
230236
component: SentryAppComponent,
231237
project_slug: str | None = None,
232-
values: List[Mapping[str, Any]] | None = None,
238+
values: Any = None,
233239
) -> SentryAppComponent | None:
234240
from sentry.coreapi import APIError
235241
from sentry.sentry_apps.components import SentryAppComponentPreparer

Diff for: src/sentry/services/hybrid_cloud/app/impl.py

+6-11
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
)
2929
from sentry.services.hybrid_cloud.app.serial import (
3030
serialize_sentry_app,
31-
serialize_sentry_app_component,
3231
serialize_sentry_app_installation,
3332
)
3433
from sentry.services.hybrid_cloud.auth import AuthenticationContext
@@ -56,7 +55,12 @@ def get_many(
5655

5756
def find_app_components(self, *, app_id: int) -> List[RpcSentryAppComponent]:
5857
return [
59-
serialize_sentry_app_component(c)
58+
RpcSentryAppComponent(
59+
uuid=str(c.uuid),
60+
sentry_app_id=c.sentry_app_id,
61+
type=c.type,
62+
app_schema=c.schema,
63+
)
6064
for c in SentryAppComponent.objects.filter(sentry_app_id=app_id)
6165
]
6266

@@ -249,12 +253,3 @@ def create_internal_integration_for_channel_request(
249253
installation = SentryAppInstallation.objects.get(sentry_app=sentry_app)
250254

251255
return serialize_sentry_app_installation(installation=installation, app=sentry_app)
252-
253-
def prepare_sentry_app_components(
254-
self, *, installation_id: int, component_type: str, project_slug: Optional[str] = None
255-
) -> Optional[RpcSentryAppComponent]:
256-
from sentry.models.integrations.sentry_app_installation import prepare_sentry_app_components
257-
258-
installation = SentryAppInstallation.objects.get(id=installation_id)
259-
component = prepare_sentry_app_components(installation, component_type, project_slug)
260-
return serialize_sentry_app_component(component) if component else None

Diff for: src/sentry/services/hybrid_cloud/app/model.py

-1
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@ class RpcSentryApp(RpcModel):
4444
uuid: str = ""
4545
events: List[str] = Field(default_factory=list)
4646
webhook_url: Optional[str] = None
47-
is_alertable: bool = False
4847
is_published: bool = False
4948
is_unpublished: bool = False
5049
is_internal: bool = True

Diff for: src/sentry/services/hybrid_cloud/app/serial.py

+1-13
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,11 @@
22

33
from sentry.constants import SentryAppStatus
44
from sentry.models.apiapplication import ApiApplication
5-
from sentry.models.integrations import SentryAppComponent
65
from sentry.models.integrations.sentry_app import SentryApp
76
from sentry.models.integrations.sentry_app_installation import SentryAppInstallation
87
from sentry.services.hybrid_cloud.app import (
98
RpcApiApplication,
109
RpcSentryApp,
11-
RpcSentryAppComponent,
1210
RpcSentryAppInstallation,
1311
)
1412

@@ -36,12 +34,11 @@ def serialize_sentry_app(app: SentryApp) -> RpcSentryApp:
3634
uuid=app.uuid,
3735
events=app.events,
3836
webhook_url=app.webhook_url,
39-
is_alertable=app.is_alertable,
4037
is_published=app.status == SentryAppStatus.PUBLISHED,
4138
is_unpublished=app.status == SentryAppStatus.UNPUBLISHED,
4239
is_internal=app.status == SentryAppStatus.INTERNAL,
4340
is_publish_request_inprogress=app.status == SentryAppStatus.PUBLISH_REQUEST_INPROGRESS,
44-
status=SentryAppStatus.as_str(app.status),
41+
status=app.status,
4542
)
4643

4744

@@ -61,12 +58,3 @@ def serialize_sentry_app_installation(
6158
uuid=installation.uuid,
6259
api_token=installation.api_token.token if installation.api_token else None,
6360
)
64-
65-
66-
def serialize_sentry_app_component(component: SentryAppComponent) -> RpcSentryAppComponent:
67-
return RpcSentryAppComponent(
68-
uuid=str(component.uuid),
69-
sentry_app_id=component.sentry_app_id,
70-
type=component.type,
71-
app_schema=component.schema,
72-
)

Diff for: src/sentry/services/hybrid_cloud/app/service.py

-7
Original file line numberDiff line numberDiff line change
@@ -140,12 +140,5 @@ def create_internal_integration_for_channel_request(
140140
) -> RpcSentryAppInstallation:
141141
pass
142142

143-
@rpc_method
144-
@abc.abstractmethod
145-
def prepare_sentry_app_components(
146-
self, *, installation_id: int, component_type: str, project_slug: Optional[str] = None
147-
) -> Optional[RpcSentryAppComponent]:
148-
pass
149-
150143

151144
app_service = cast(AppService, AppService.create_delegation())

Diff for: tests/sentry/incidents/endpoints/test_organization_alert_rule_available_action_index.py

+4-13
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
from sentry.incidents.models import AlertRuleTriggerAction
66
from sentry.models.integrations.integration import Integration
77
from sentry.models.integrations.organization_integration import OrganizationIntegration
8-
from sentry.services.hybrid_cloud.app.serial import serialize_sentry_app_installation
98
from sentry.silo import SiloMode
109
from sentry.testutils.cases import APITestCase
1110
from sentry.testutils.silo import assume_test_silo_mode, region_silo_test
@@ -26,7 +25,7 @@
2625
}
2726

2827

29-
@region_silo_test(stable=True)
28+
@region_silo_test
3029
class OrganizationAlertRuleAvailableActionIndexEndpointTest(APITestCase):
3130
endpoint = "sentry-api-0-organization-alert-rule-available-actions"
3231
email = AlertRuleTriggerAction.get_registered_type(AlertRuleTriggerAction.Type.EMAIL)
@@ -110,9 +109,7 @@ def test_build_action_response_pagerduty(self):
110109
def test_build_action_response_sentry_app(self):
111110
installation = self.install_new_sentry_app("foo")
112111

113-
data = build_action_response(
114-
self.sentry_app, sentry_app_installation=serialize_sentry_app_installation(installation)
115-
)
112+
data = build_action_response(self.sentry_app, sentry_app_installation=installation)
116113

117114
assert data["type"] == "sentry_app"
118115
assert data["allowedTargetTypes"] == ["sentry_app"]
@@ -183,10 +180,7 @@ def test_sentry_apps(self):
183180
assert len(response.data) == 2
184181
assert build_action_response(self.email) in response.data
185182
assert (
186-
build_action_response(
187-
self.sentry_app,
188-
sentry_app_installation=serialize_sentry_app_installation(installation),
189-
)
183+
build_action_response(self.sentry_app, sentry_app_installation=installation)
190184
in response.data
191185
)
192186

@@ -199,10 +193,7 @@ def test_published_sentry_apps(self):
199193

200194
assert len(response.data) == 2
201195
assert (
202-
build_action_response(
203-
self.sentry_app,
204-
sentry_app_installation=serialize_sentry_app_installation(installation),
205-
)
196+
build_action_response(self.sentry_app, sentry_app_installation=installation)
206197
in response.data
207198
)
208199

Diff for: tests/sentry/incidents/test_logic.py

+2-7
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,6 @@
6969
from sentry.models.actor import ActorTuple, get_actor_id_for_user
7070
from sentry.models.integrations.integration import Integration
7171
from sentry.models.integrations.organization_integration import OrganizationIntegration
72-
from sentry.services.hybrid_cloud.integration.serial import serialize_integration
7372
from sentry.shared_integrations.exceptions import ApiError, ApiRateLimitedError, ApiTimeoutError
7473
from sentry.snuba.dataset import Dataset
7574
from sentry.snuba.models import QuerySubscription, SnubaQuery, SnubaQueryEventType
@@ -1998,18 +1997,14 @@ def test_unregistered(self):
19981997
def test_registered(self):
19991998
integration = Integration.objects.create(external_id="1", provider="slack")
20001999
integration.add_organization(self.organization)
2001-
assert list(get_available_action_integrations_for_org(self.organization)) == [
2002-
serialize_integration(integration)
2003-
]
2000+
assert list(get_available_action_integrations_for_org(self.organization)) == [integration]
20042001

20052002
def test_mixed(self):
20062003
integration = Integration.objects.create(external_id="1", provider="slack")
20072004
integration.add_organization(self.organization)
20082005
other_integration = Integration.objects.create(external_id="12345", provider="random")
20092006
other_integration.add_organization(self.organization)
2010-
assert list(get_available_action_integrations_for_org(self.organization)) == [
2011-
serialize_integration(integration)
2012-
]
2007+
assert list(get_available_action_integrations_for_org(self.organization)) == [integration]
20132008

20142009
def test_disabled_integration(self):
20152010
integration = Integration.objects.create(

0 commit comments

Comments
 (0)