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/src/sentry/workflow_engine/endpoints/serializers/action_handler_serializer.py b/src/sentry/workflow_engine/endpoints/serializers/action_handler_serializer.py index fb278cde57badf..00693373cc76dd 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 @@ -76,16 +75,9 @@ def serialize( "status": installation.sentry_app.status, } 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") + 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") 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()