From 44436b9d3c06c283847bcc77ddd2cc19c18a479c Mon Sep 17 00:00:00 2001 From: Mia Hsu Date: Tue, 21 Oct 2025 10:15:49 -0700 Subject: [PATCH 1/3] fix(aci): filter out non-alertable and not-broken sentry app for available actions --- .../organization_available_action_index.py | 50 +++++++++++-------- ...est_organization_available_action_index.py | 48 ++++++++++++++++++ 2 files changed, 78 insertions(+), 20 deletions(-) diff --git a/src/sentry/workflow_engine/endpoints/organization_available_action_index.py b/src/sentry/workflow_engine/endpoints/organization_available_action_index.py index fdcfb9ebb092b7..0969750453ed8d 100644 --- a/src/sentry/workflow_engine/endpoints/organization_available_action_index.py +++ b/src/sentry/workflow_engine/endpoints/organization_available_action_index.py @@ -20,6 +20,7 @@ from sentry.apidocs.utils import inline_sentry_response_serializer from sentry.integrations.services.integration import RpcIntegration from sentry.rules.actions.services import PluginService, SentryAppService +from sentry.sentry_apps.models.sentry_app_installation import prepare_ui_component from sentry.sentry_apps.services.app import app_service from sentry.workflow_engine.endpoints.serializers.action_handler_serializer import ( ActionHandlerSerializer, @@ -77,19 +78,29 @@ def get(self, request, organization): AvailableIntegration(integration=integration, services=services) ) - sentry_app_component_contexts = app_service.get_installation_component_contexts( + all_sentry_app_contexts = app_service.get_installation_component_contexts( filter={"organization_id": organization.id}, component_type="alert-rule-action", include_contexts_without_component=True, ) - # Split contexts into those with and without components - sentry_app_contexts_with_components = [ - context for context in sentry_app_component_contexts if context.component - ] - sentry_app_contexts_without_components = [ - context for context in sentry_app_component_contexts if not context.component - ] + # filter for alertable apps and split contexts into those with and without components + alertable_apps_with_components = [] + alertable_apps_without_components = [] + for context in all_sentry_app_contexts: + if not context.installation.sentry_app.is_alertable: + continue + + if context.component: + # filter out broken apps by checking if prepare_ui_component succeeds + prepared_component = prepare_ui_component( + installation=context.installation, + component=context.component, + ) + if prepared_component is not None: + alertable_apps_with_components.append(context) + else: + alertable_apps_without_components.append(context) actions = [] for action_type, handler in action_handler_registry.registrations.items(): @@ -112,19 +123,18 @@ def get(self, request, organization): ) # add alertable sentry app actions - # sentry app actions are only for sentry apps with components + # sentry app actions are only for alertable sentry apps with components elif action_type == Action.Type.SENTRY_APP: - for context in sentry_app_contexts_with_components: - if context.installation.sentry_app.is_alertable: - actions.append( - serialize( - handler, - request.user, - ActionHandlerSerializer(), - action_type=action_type, - sentry_app_context=context, - ) + for context in alertable_apps_with_components: + actions.append( + serialize( + handler, + request.user, + ActionHandlerSerializer(), + action_type=action_type, + sentry_app_context=context, ) + ) # add webhook action # service options include plugins and sentry apps without components @@ -132,7 +142,7 @@ def get(self, request, organization): plugins = get_notification_plugins_for_org(organization) sentry_apps: list[PluginService] = [ SentryAppService(context.installation.sentry_app) - for context in sentry_app_contexts_without_components + for context in alertable_apps_without_components ] available_services: list[PluginService] = plugins + sentry_apps diff --git a/tests/sentry/workflow_engine/endpoints/test_organization_available_action_index.py b/tests/sentry/workflow_engine/endpoints/test_organization_available_action_index.py index ca463b7d6bcaff..491eaad8e57dea 100644 --- a/tests/sentry/workflow_engine/endpoints/test_organization_available_action_index.py +++ b/tests/sentry/workflow_engine/endpoints/test_organization_available_action_index.py @@ -206,6 +206,32 @@ class SentryAppActionHandler(ActionHandler): slug=self.sentry_app.slug, organization=self.organization ) + # should not return sentry apps that are not alertable + self.not_alertable_sentry_app = self.create_sentry_app( + name="Not Alertable Sentry App", + organization=self.organization, + is_alertable=False, + ) + self.not_alertable_sentry_app_installation = self.create_sentry_app_installation( + slug=self.not_alertable_sentry_app.slug, organization=self.organization + ) + + self.not_alertable_sentry_app = self.create_sentry_app( + name="Not Alertable Sentry App With Component", + organization=self.organization, + schema={ + "elements": [ + self.sentry_app_settings_schema, + ] + }, + is_alertable=False, + ) + self.not_alertable_sentry_app_with_component_installation = ( + self.create_sentry_app_installation( + slug=self.not_alertable_sentry_app.slug, organization=self.organization + ) + ) + # should not return sentry apps that are not installed self.create_sentry_app( name="Bad Sentry App", @@ -388,6 +414,28 @@ def test_sentry_apps(self, mock_sentry_app_component_preparer: MagicMock) -> Non }, ] + @patch( + "sentry.workflow_engine.endpoints.organization_available_action_index.prepare_ui_component" + ) + def test_sentry_apps_filters_failed_component_preparation( + self, mock_prepare_ui_component: MagicMock + ) -> None: + """Test that sentry apps whose components fail to prepare are filtered out""" + self.setup_sentry_apps() + + # make prepare_ui_component return None to simulate a broken app + mock_prepare_ui_component.return_value = None + + response = self.get_success_response( + self.organization.slug, + status_code=200, + ) + + # verify prepare_ui_component was called + assert mock_prepare_ui_component.called + # should return no sentry apps since component preparation failed + assert len(response.data) == 0 + def test_webhooks(self) -> None: self.setup_webhooks() From f98f986ce9d870edb0a414ed1d1c0f7c6e51cc0d Mon Sep 17 00:00:00 2001 From: Mia Hsu Date: Tue, 28 Oct 2025 16:52:32 -0700 Subject: [PATCH 2/3] remove prepare_ui_component from serializer --- .../serializers/action_handler_serializer.py | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/src/sentry/workflow_engine/endpoints/serializers/action_handler_serializer.py b/src/sentry/workflow_engine/endpoints/serializers/action_handler_serializer.py index fb278cde57badf..2400372f32a6a5 100644 --- a/src/sentry/workflow_engine/endpoints/serializers/action_handler_serializer.py +++ b/src/sentry/workflow_engine/endpoints/serializers/action_handler_serializer.py @@ -3,7 +3,6 @@ from sentry.api.serializers import Serializer, register from sentry.rules.actions.notify_event_service import PLUGINS_WITH_FIRST_PARTY_EQUIVALENTS -from sentry.sentry_apps.models.sentry_app_installation import prepare_ui_component from sentry.workflow_engine.types import ActionHandler @@ -74,18 +73,10 @@ def serialize( "installationId": str(installation.id), "installationUuid": str(installation.uuid), "status": installation.sentry_app.status, + "settings": component.app_schema.get("settings", {}) if component else None, } - if component: - prepared_component = prepare_ui_component( - installation=installation, - component=component, - project_slug=None, - values=None, - ) - if prepared_component: - sentry_app["settings"] = prepared_component.app_schema.get("settings", {}) - if prepared_component.app_schema.get("title"): - sentry_app["title"] = prepared_component.app_schema.get("title") + if component.app_schema.get("title"): + sentry_app["title"] = component.app_schema.get("title") result["sentryApp"] = sentry_app services = kwargs.get("services") From a01c534c1a064d0cb409992f425882b29d29724e Mon Sep 17 00:00:00 2001 From: Mia Hsu Date: Tue, 28 Oct 2025 17:01:24 -0700 Subject: [PATCH 3/3] mypy --- .../endpoints/serializers/action_handler_serializer.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/sentry/workflow_engine/endpoints/serializers/action_handler_serializer.py b/src/sentry/workflow_engine/endpoints/serializers/action_handler_serializer.py index 2400372f32a6a5..00693373cc76dd 100644 --- a/src/sentry/workflow_engine/endpoints/serializers/action_handler_serializer.py +++ b/src/sentry/workflow_engine/endpoints/serializers/action_handler_serializer.py @@ -73,10 +73,11 @@ def serialize( "installationId": str(installation.id), "installationUuid": str(installation.uuid), "status": installation.sentry_app.status, - "settings": component.app_schema.get("settings", {}) if component else None, } - if component.app_schema.get("title"): - sentry_app["title"] = component.app_schema.get("title") + if component: + sentry_app["settings"] = component.app_schema.get("settings", {}) + if component.app_schema.get("title"): + sentry_app["title"] = component.app_schema.get("title") result["sentryApp"] = sentry_app services = kwargs.get("services")