fix(ACI): Fix adding sentry app action to a workflow

src/sentry/notifications/notification_action/action_handler_registry/sentry_app_handler.py

@@ -33,6 +33,7 @@ class SentryAppActionHandler(ActionHandler):
 
     data_schema = {
         "$schema": "https://json-schema.org/draft/2020-12/schema",
+        "description": "The data schema for a Sentry App Action",
         "type": "object",
         "properties": {
             "settings": {"type": ["array", "object"]},

src/sentry/notifications/notification_action/action_validation.py

@@ -15,7 +15,7 @@
 from sentry.rules.actions.integrations.create_ticket.form import IntegrationNotifyServiceForm
 from sentry.rules.actions.notify_event_service import NotifyEventServiceForm
 from sentry.rules.actions.sentry_apps.utils import validate_sentry_app_action
-from sentry.sentry_apps.services.app.service import app_service
+from sentry.sentry_apps.services.app import RpcSentryAppInstallation, app_service
 from sentry.sentry_apps.utils.errors import SentryAppBaseError
 from sentry.utils import json
 from sentry.workflow_engine.models.action import Action
@@ -206,31 +206,74 @@ def __init__(self, validated_data: dict[str, Any], organization: Organization) -
         self.validated_data = validated_data
         self.organization = organization
 
+    def _get_sentry_app_installation(
+        self, sentry_app_identifier: SentryAppIdentifier, target_identifier: str
+    ) -> RpcSentryAppInstallation | None:
+        """
+        Get the sentry app installation based on whether the target identifier is an installation id or sentry app id
+        We do not want to accept SentryAppIdentifier.SENTRY_APP_INSTALLATION_UUID long term, this is temporary until we migrate the data over
+        """
+        installations = None
+        installation = None
+
+        if sentry_app_identifier == SentryAppIdentifier.SENTRY_APP_INSTALLATION_UUID:
+            installations = app_service.get_many(
+                filter=dict(uuids=[target_identifier], organization_id=self.organization.id)
+            )
+        else:
+            installations = app_service.get_many(
+                filter=dict(app_ids=[int(target_identifier)], organization_id=self.organization.id)
+            )
+        if installations:
+            installation = installations[0]
+
+        return installation
+
     def clean_data(self) -> dict[str, Any]:
-        is_sentry_app_installation = (
-            SentryAppIdentifier(self.validated_data["config"]["sentry_app_identifier"])
-            == SentryAppIdentifier.SENTRY_APP_INSTALLATION_UUID
+        sentry_app_identifier = SentryAppIdentifier(
+            self.validated_data["config"]["sentry_app_identifier"]
         )
+        target_identifier = self.validated_data["config"]["target_identifier"]
+        installation = self._get_sentry_app_installation(sentry_app_identifier, target_identifier)
+        if not installation:
+            raise ValidationError("Sentry app installation not found.")
+
+        if sentry_app_identifier == SentryAppIdentifier.SENTRY_APP_INSTALLATION_UUID:
+            # convert to use sentry_app_id until we can migrate all the data
+            self.validated_data["config"][
+                "sentry_app_identifier"
+            ] = SentryAppIdentifier.SENTRY_APP_ID
+            self.validated_data["config"]["target_identifier"] = str(installation.sentry_app.id)
+
+        settings = self.validated_data["data"].get("settings", [])
         action = {
-            "settings": self.validated_data["data"]["settings"],
-            "sentryAppInstallationUuid": (
-                self.validated_data["config"]["target_identifier"]
-                if is_sentry_app_installation
-                else None
-            ),
+            "settings": settings,
+            "sentryAppInstallationUuid": installation.uuid,
         }
-        try:
-            validate_sentry_app_action(action)
 
+        if not settings:
+            # XXX: it's only ok to not pass settings if there is no sentry app schema
+            # this means the app doesn't expect any settings
+            components = app_service.find_app_components(app_id=installation.sentry_app.id)
+            if any(
+                component.app_schema
+                for component in components
+                if component.type == "alert-rule-action"
+            ):
+                raise ValidationError("'settings' is a required property")
+
+        else:
             # Sentry app config blob expects value to be a string
-            settings = self.validated_data["data"]["settings"]
             for setting in settings:
                 if setting.get("value") is not None and not isinstance(setting["value"], str):
                     setting["value"] = json.dumps(setting["value"])
+            try:
+                # Only call creator for Sentry Apps with UI Components (settings) for actions
+                validate_sentry_app_action(action)
+            except SentryAppBaseError as e:
+                raise ValidationError(e.message) from e
 
-            return self.validated_data
-        except SentryAppBaseError as e:
-            raise ValidationError(e.message) from e
+        return self.validated_data
 
 
 @action_validator_registry.register(Action.Type.WEBHOOK)

tests/sentry/workflow_engine/endpoints/test_organization_workflow_details.py

@@ -1,5 +1,7 @@
 from contextlib import AbstractContextManager
 
+import responses
+
 from sentry import audit_log
 from sentry.api.serializers import serialize
 from sentry.constants import ObjectStatus
@@ -13,8 +15,20 @@
 from sentry.testutils.outbox import outbox_runner
 from sentry.testutils.silo import assume_test_silo_mode, region_silo_test
 from sentry.workflow_engine.endpoints.validators.base.workflow import WorkflowValidator
-from sentry.workflow_engine.models import Action, DataConditionGroup, Workflow
+from sentry.workflow_engine.models import (
+    Action,
+    Condition,
+    DataConditionGroup,
+    DataConditionGroupAction,
+    Workflow,
+    WorkflowDataConditionGroup,
+)
 from sentry.workflow_engine.models.detector_workflow import DetectorWorkflow
+from sentry.workflow_engine.typings.notification_action import (
+    ActionTarget,
+    ActionType,
+    SentryAppIdentifier,
+)
 from tests.sentry.workflow_engine.test_base import BaseWorkflowTest
 
 
@@ -54,7 +68,7 @@ def setUp(self) -> None:
             "enabled": True,
             "config": {},
             "triggers": {"logicType": "any", "conditions": []},
-            "action_filters": [],
+            "actionFilters": [],
         }
         validator = WorkflowValidator(
             data=self.valid_workflow,
@@ -63,6 +77,15 @@ def setUp(self) -> None:
         validator.is_valid(raise_exception=True)
         self.workflow = validator.create(validator.validated_data)
 
+        self.sentry_app, self.sentry_app_installation = self.create_sentry_app_with_schema()
+        self.sentry_app_settings = [
+            {"name": "alert_prefix", "value": "[Not Good]"},
+            {"name": "channel", "value": "#ignored-errors"},
+            {"name": "best_emoji", "value": ":fire:"},
+            {"name": "teamId", "value": "1"},
+            {"name": "assigneeId", "value": "3"},
+        ]
+
     def test_simple(self) -> None:
         self.valid_workflow["name"] = "Updated Workflow"
         response = self.get_success_response(
@@ -73,6 +96,116 @@ def test_simple(self) -> None:
         assert response.status_code == 200
         assert updated_workflow.name == "Updated Workflow"
 
+    @responses.activate
+    def test_update_add_sentry_app_action(self) -> None:
+        """
+        Test that adding a sentry app action to a workflow works as expected
+        """
+        responses.add(
+            method=responses.POST,
+            url="https://example.com/sentry/alert-rule",
+            status=200,
+        )
+        self.valid_workflow["actionFilters"] = [
+            {
+                "logicType": "any",
+                "conditions": [
+                    {
+                        "type": Condition.EQUAL.value,
+                        "comparison": 1,
+                        "conditionResult": True,
+                    }
+                ],
+                "actions": [
+                    {
+                        "config": {
+                            "sentryAppIdentifier": SentryAppIdentifier.SENTRY_APP_ID,
+                            "targetIdentifier": str(self.sentry_app.id),
+                            "targetType": ActionType.SENTRY_APP,
+                        },
+                        "data": {"settings": self.sentry_app_settings},
+                        "type": Action.Type.SENTRY_APP,
+                    },
+                ],
+            }
+        ]
+        response = self.get_success_response(
+            self.organization.slug, self.workflow.id, raw_data=self.valid_workflow
+        )
+        updated_workflow = Workflow.objects.get(id=response.data.get("id"))
+        action_filter = WorkflowDataConditionGroup.objects.get(workflow=updated_workflow)
+        dcga = DataConditionGroupAction.objects.get(condition_group=action_filter.condition_group)
+        action = dcga.action
+
+        assert response.status_code == 200
+        assert action.type == Action.Type.SENTRY_APP
+        assert action.config == {
+            "sentry_app_identifier": SentryAppIdentifier.SENTRY_APP_ID,
+            "target_identifier": str(self.sentry_app.id),
+            "target_type": ActionTarget.SENTRY_APP.value,
+        }
+        assert action.data["settings"] == self.sentry_app_settings
+
+    @responses.activate
+    def test_update_add_sentry_app_installation_uuid_action(self) -> None:
+        """
+        Test that adding a sentry app action to a workflow works as expected when we pass the installation UUID instead of the sentry app id.
+        We do not create new actions like this today, but we have data like this in the DB and need to make sure it still works until we can migrate away from it
+        to use the sentry app id
+        """
+        responses.add(
+            method=responses.POST,
+            url="https://example.com/sentry/alert-rule",
+            status=200,
+        )
+        # update the settings
+        updated_sentry_app_settings = [
+            {"name": "alert_prefix", "value": "[Very Good]"},
+            {"name": "channel", "value": "#pay-attention-to-errors"},
+            {"name": "best_emoji", "value": ":ice:"},
+            {"name": "teamId", "value": "1"},
+            {"name": "assigneeId", "value": "3"},
+        ]
+        self.valid_workflow["actionFilters"] = [
+            {
+                "logicType": "any",
+                "conditions": [
+                    {
+                        "type": Condition.EQUAL.value,
+                        "comparison": 1,
+                        "conditionResult": True,
+                    }
+                ],
+                "actions": [
+                    {
+                        "config": {
+                            "sentryAppIdentifier": SentryAppIdentifier.SENTRY_APP_INSTALLATION_UUID,
+                            "targetIdentifier": self.sentry_app_installation.uuid,
+                            "targetType": ActionType.SENTRY_APP,
+                        },
+                        "data": {"settings": updated_sentry_app_settings},
+                        "type": Action.Type.SENTRY_APP,
+                    },
+                ],
+            }
+        ]
+        response = self.get_success_response(
+            self.organization.slug, self.workflow.id, raw_data=self.valid_workflow
+        )
+        updated_workflow = Workflow.objects.get(id=response.data.get("id"))
+        action_filter = WorkflowDataConditionGroup.objects.get(workflow=updated_workflow)
+        dcga = DataConditionGroupAction.objects.get(condition_group=action_filter.condition_group)
+        action = dcga.action
+
+        assert response.status_code == 200
+        assert action.type == Action.Type.SENTRY_APP
+        assert action.config == {
+            "sentry_app_identifier": SentryAppIdentifier.SENTRY_APP_ID,
+            "target_identifier": str(self.sentry_app.id),
+            "target_type": ActionTarget.SENTRY_APP.value,
+        }
+        assert action.data["settings"] == updated_sentry_app_settings
+
     def test_update_triggers_with_empty_conditions(self) -> None:
         """Test that passing an empty list to triggers.conditions clears all conditions"""
         # Create a workflow with a trigger condition