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,11 +15,10 @@
 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 app_service
 from sentry.sentry_apps.utils.errors import SentryAppBaseError
 from sentry.workflow_engine.models.action import Action
 from sentry.workflow_engine.processors.action import get_notification_plugins_for_org
-from sentry.workflow_engine.typings.notification_action import SentryAppIdentifier
 
 from .types import BaseActionValidatorHandler
 
@@ -206,18 +205,21 @@ def __init__(self, validated_data: dict[str, Any], organization: Organization) -
         self.organization = organization
 
     def clean_data(self) -> dict[str, Any]:
-        is_sentry_app_installation = (
-            SentryAppIdentifier(self.validated_data["config"]["sentry_app_identifier"])
-            == SentryAppIdentifier.SENTRY_APP_INSTALLATION_UUID
+        installation = app_service.get_installation_by_id(
+            id=int(self.validated_data["config"]["target_identifier"])
         )
+        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,
         }
+        # 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
+        if not settings:
+            components = app_service.find_app_components(app_id=installation.sentry_app.id)
+            if any(component.app_schema for component in components):
+                raise ValidationError("'settings' is a required property")
+
         try:
             validate_sentry_app_action(action)
             return self.validated_data

tests/sentry/deletions/test_sentry_app.py

@@ -67,6 +67,11 @@ def test_disables_actions(self) -> None:
                 "sentry_app_identifier": SentryAppIdentifier.SENTRY_APP_ID,
                 "target_type": ActionTarget.SENTRY_APP,
             },
+            data={
+                "settings": [
+                    {"name": "best_emoji", "value": ":fire:"},
+                ]
+            },
         )
         webhook_action = self.create_action(
             type=Action.Type.WEBHOOK,
@@ -81,6 +86,11 @@ def test_disables_actions(self) -> None:
                 "sentry_app_identifier": SentryAppIdentifier.SENTRY_APP_ID,
                 "target_type": ActionTarget.SENTRY_APP,
             },
+            data={
+                "settings": [
+                    {"name": "best_emoji", "value": ":fire:"},
+                ]
+            },
         )
         deletions.exec_sync(self.sentry_app)
 

tests/sentry/deletions/test_sentry_app_installations.py

@@ -119,6 +119,11 @@ def test_disables_actions(self) -> None:
                 "sentry_app_identifier": SentryAppIdentifier.SENTRY_APP_INSTALLATION_UUID,
                 "target_type": ActionTarget.SENTRY_APP,
             },
+            data={
+                "settings": [
+                    {"name": "best_emoji", "value": ":fire:"},
+                ]
+            },
         )
         other_action = self.create_action(
             type=Action.Type.SENTRY_APP,
@@ -127,6 +132,11 @@ def test_disables_actions(self) -> None:
                 "sentry_app_identifier": SentryAppIdentifier.SENTRY_APP_INSTALLATION_UUID,
                 "target_type": ActionTarget.SENTRY_APP,
             },
+            data={
+                "settings": [
+                    {"name": "best_emoji", "value": ":fire:"},
+                ]
+            },
         )
         deletions.exec_sync(self.install)
         with outbox_runner():

tests/sentry/notifications/notification_action/metric_alert_registry/test_sentry_app_metric_alert_handler.py

@@ -45,6 +45,11 @@ def setUp(self) -> None:
                 "target_type": ActionTarget.SENTRY_APP.value,
                 "sentry_app_identifier": SentryAppIdentifier.SENTRY_APP_ID,
             },
+            data={
+                "settings": [
+                    {"name": "best_emoji", "value": ":fire:"},
+                ]
+            },
         )
 
         self.handler = SentryAppMetricAlertHandler()
@@ -112,7 +117,7 @@ def test_invoke_legacy_registry(self, mock_send_alert: mock.MagicMock) -> None:
             integration_id=None,
             target_identifier=None,
             target_display=None,
-            sentry_app_config=None,
+            sentry_app_config=self.action.data.get("settings"),
             sentry_app_id=str(self.sentry_app.id),
         )
 
@@ -188,7 +193,7 @@ def test_invoke_legacy_registry_with_activity(self, mock_send_alert: mock.MagicM
             integration_id=None,
             target_identifier=None,
             target_display=None,
-            sentry_app_config=None,
+            sentry_app_config=self.action.data.get("settings"),
             sentry_app_id=str(self.sentry_app.id),
         )
 

tests/sentry/sentry_apps/api/endpoints/test_sentry_app_details.py

@@ -858,6 +858,11 @@ def test_disables_actions(self) -> None:
                 "sentry_app_identifier": SentryAppIdentifier.SENTRY_APP_ID,
                 "target_type": ActionTarget.SENTRY_APP,
             },
+            data={
+                "settings": [
+                    {"name": "best_emoji", "value": ":fire:"},
+                ]
+            },
         )
         webhook_action = self.create_action(
             type=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,
@@ -73,6 +87,64 @@ 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.sentry_app = 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},
+        ]
+        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
+
     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