fix(ACI): Accept fallthrough type in actions

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

@@ -42,7 +42,13 @@ class EmailActionHandler(ActionHandler):
         "$schema": "https://json-schema.org/draft/2020-12/schema",
         "type": "object",
         "properties": {
-            "fallthroughType": {  # TODO: migrate to snake_case
+            "fallthrough_type": {
+                "type": "string",
+                "description": "The fallthrough type for issue owners email notifications",
+                "enum": [*FallthroughChoiceType],
+            },
+            # XXX(CEO): temporarily support this incorrect camel case
+            "fallthroughType": {
                 "type": "string",
                 "description": "The fallthrough type for issue owners email notifications",
                 "enum": [*FallthroughChoiceType],

src/sentry/notifications/notification_action/issue_alert_registry/handlers/email_issue_alert_handler.py

@@ -51,7 +51,13 @@ def get_additional_fields(cls, action: Action, mapping: ActionFieldMapping) -> d
         }
 
         if target_type == ActionTarget.ISSUE_OWNERS.value:
-            blob = EmailDataBlob(**action.data)
-            final_blob[EmailFieldMappingKeys.FALLTHROUGH_TYPE_KEY.value] = blob.fallthroughType
+            # XXX(CEO): temporarily handle both fallthroughType and fallthrough_type
+            action_data = action.data.copy()
+            if action.data.get("fallthroughType"):
+                del action_data["fallthroughType"]
+                action_data["fallthrough_type"] = action.data["fallthroughType"]
+
+            blob = EmailDataBlob(**action_data)
+            final_blob[EmailFieldMappingKeys.FALLTHROUGH_TYPE_KEY.value] = blob.fallthrough_type
 
         return final_blob

src/sentry/testutils/helpers/data_blobs.py

@@ -761,37 +761,37 @@
         "targetType": "IssueOwners",
         "id": "sentry.mail.actions.NotifyEmailAction",
         "targetIdentifier": "None",
-        "fallthroughType": "ActiveMembers",
+        "fallthrough_type": "ActiveMembers",
         "uuid": "2e8847d7-8fe4-44d2-8a16-e25040329790",
     },
     # NoOne Fallthrough (targetIdentifier is "")
     {
         "targetType": "IssueOwners",
         "targetIdentifier": "",
         "id": "sentry.mail.actions.NotifyEmailAction",
-        "fallthroughType": "NoOne",
+        "fallthrough_type": "NoOne",
         "uuid": "fb039430-0848-4fc4-89b4-bc7689a9f851",
     },
     # AllMembers Fallthrough (targetIdentifier is None)
     {
         "targetType": "IssueOwners",
         "id": "sentry.mail.actions.NotifyEmailAction",
         "targetIdentifier": None,
-        "fallthroughType": "AllMembers",
+        "fallthrough_type": "AllMembers",
         "uuid": "41f13756-8f90-4afe-b162-55268c6e3cdb",
     },
     # NoOne Fallthrough (targetIdentifier is "None")
     {
         "targetType": "IssueOwners",
         "id": "sentry.mail.actions.NotifyEmailAction",
         "targetIdentifier": "None",
-        "fallthroughType": "NoOne",
+        "fallthrough_type": "NoOne",
         "uuid": "99c9b517-0a0f-47f0-b3ff-2a9cd2fd9c49",
     },
     # ActiveMembers Fallthrough
     {
         "targetType": "Member",
-        "fallthroughType": "ActiveMembers",
+        "fallthrough_type": "ActiveMembers",
         "id": "sentry.mail.actions.NotifyEmailAction",
         "targetIdentifier": 3234013,
         "uuid": "6e83337b-9561-4167-a208-27d6bdf5e613",
@@ -807,7 +807,7 @@
     {
         "targetType": "Team",
         "id": "sentry.mail.actions.NotifyEmailAction",
-        "fallthroughType": "AllMembers",
+        "fallthrough_type": "AllMembers",
         "uuid": "71b445cf-573b-4e0c-86bc-8dfbad93c480",
         "targetIdentifier": 188022,
     },

src/sentry/workflow_engine/typings/notification_action.py

@@ -98,7 +98,7 @@ class EmailFieldMappingKeys(StrEnum):
     EmailFieldMappingKeys is an enum that represents the keys of an email field mapping.
     """
 
-    FALLTHROUGH_TYPE_KEY = "fallthroughType"
+    FALLTHROUGH_TYPE_KEY = "fallthrough_type"
     TARGET_TYPE_KEY = "targetType"
 
 
@@ -577,7 +577,7 @@ def get_sanitized_data(self) -> dict[str, Any]:
         ):
             return dataclasses.asdict(
                 EmailDataBlob(
-                    fallthroughType=self.action.get(
+                    fallthrough_type=self.action.get(
                         EmailFieldMappingKeys.FALLTHROUGH_TYPE_KEY.value,
                         FallthroughChoiceType.ACTIVE_MEMBERS.value,
                     ),
@@ -740,7 +740,7 @@ class EmailDataBlob(DataBlob):
     EmailDataBlob represents the data blob for an email notification action.
     """
 
-    fallthroughType: str = ""
+    fallthrough_type: str = ""
 
 
 issue_alert_action_translator_mapping: dict[str, type[BaseActionTranslator]] = {

tests/sentry/notifications/notification_action/test_issue_alert_registry_handlers.py

@@ -25,6 +25,7 @@
     BaseIssueAlertHandler,
     TicketingIssueAlertHandler,
 )
+from sentry.notifications.types import ActionTargetType, FallthroughChoiceType
 from sentry.testutils.helpers.data_blobs import (
     AZURE_DEVOPS_ACTION_DATA_BLOBS,
     EMAIL_ACTION_DATA_BLOBS,
@@ -538,47 +539,47 @@ def setUp(self) -> None:
         self.detector = self.create_detector(project=self.project)
         # These are the actions that are healed from the old email action data blobs
         # It removes targetIdentifier for IssueOwner targets (since that shouldn't be set for those)
-        # It also removes the fallthroughType for Team and Member targets (since that shouldn't be set for those)
+        # It also removes the fallthrough_type for Team and Member targets (since that shouldn't be set for those)
         self.HEALED_EMAIL_ACTION_DATA_BLOBS = [
             # IssueOwners (targetIdentifier is "None")
             {
-                "targetType": "IssueOwners",
+                "targetType": ActionTargetType.ISSUE_OWNERS.value,
                 "id": "sentry.mail.actions.NotifyEmailAction",
-                "fallthroughType": "ActiveMembers",
+                "fallthrough_type": FallthroughChoiceType.ACTIVE_MEMBERS,
             },
             # NoOne Fallthrough (targetIdentifier is "")
             {
-                "targetType": "IssueOwners",
+                "targetType": ActionTargetType.ISSUE_OWNERS.value,
                 "id": "sentry.mail.actions.NotifyEmailAction",
-                "fallthroughType": "NoOne",
+                "fallthrough_type": FallthroughChoiceType.NO_ONE,
             },
             # AllMembers Fallthrough (targetIdentifier is None)
             {
-                "targetType": "IssueOwners",
+                "targetType": ActionTargetType.ISSUE_OWNERS.value,
                 "id": "sentry.mail.actions.NotifyEmailAction",
-                "fallthroughType": "AllMembers",
+                "fallthrough_type": "AllMembers",
             },
             # NoOne Fallthrough (targetIdentifier is "None")
             {
-                "targetType": "IssueOwners",
+                "targetType": ActionTargetType.ISSUE_OWNERS.value,
                 "id": "sentry.mail.actions.NotifyEmailAction",
-                "fallthroughType": "NoOne",
+                "fallthrough_type": FallthroughChoiceType.NO_ONE,
             },
             # ActiveMembers Fallthrough
             {
-                "targetType": "Member",
+                "targetType": ActionTargetType.MEMBER.value,
                 "id": "sentry.mail.actions.NotifyEmailAction",
                 "targetIdentifier": "3234013",
             },
             # Member Email
             {
                 "id": "sentry.mail.actions.NotifyEmailAction",
                 "targetIdentifier": "2160509",
-                "targetType": "Member",
+                "targetType": ActionTargetType.MEMBER.value,
             },
             # Team Email
             {
-                "targetType": "Team",
+                "targetType": ActionTargetType.TEAM.value,
                 "id": "sentry.mail.actions.NotifyEmailAction",
                 "targetIdentifier": "188022",
             },
@@ -587,10 +588,8 @@ def setUp(self) -> None:
     def test_build_rule_action_blob(self) -> None:
         for expected, healed in zip(EMAIL_ACTION_DATA_BLOBS, self.HEALED_EMAIL_ACTION_DATA_BLOBS):
             action_data = pop_keys_from_data_blob(expected, Action.Type.EMAIL)
-
             # pop the targetType from the action_data
             target_type = EmailActionHelper.get_target_type_object(action_data.pop("targetType"))
-
             # Handle all possible targetIdentifier formats
             target_identifier: str | None = str(expected["targetIdentifier"])
             if target_identifier in ("None", "", None):
@@ -605,9 +604,28 @@ def test_build_rule_action_blob(self) -> None:
                 },
             )
             blob = self.handler.build_rule_action_blob(action, self.organization.id)
-
             assert blob == healed
 
+    def test_build_rule_action_blob_fallthrough_type_camel_case(self) -> None:
+        """
+        Test that while we are temporarily allowing both fallthroughType and fallthrough_type in Action.data
+        that both work when building the action data blob and result in the snake case version
+        """
+        action = Action.objects.create(
+            type=Action.Type.EMAIL,
+            data={"fallthroughType": FallthroughChoiceType.ACTIVE_MEMBERS},
+            config={
+                "target_type": ActionTarget.ISSUE_OWNERS,
+                "target_identifier": None,
+            },
+        )
+        blob = self.handler.build_rule_action_blob(action, self.organization.id)
+        assert blob == {
+            "fallthrough_type": FallthroughChoiceType.ACTIVE_MEMBERS,
+            "id": "sentry.mail.actions.NotifyEmailAction",
+            "targetType": "IssueOwners",
+        }
+
 
 class TestPluginIssueAlertHandler(BaseWorkflowTest):
     def setUp(self) -> None:

tests/sentry/workflow_engine/endpoints/test_organization_workflow_index.py

@@ -10,17 +10,20 @@
 from sentry.grouping.grouptype import ErrorGroupType
 from sentry.incidents.grouptype import MetricIssue
 from sentry.notifications.models.notificationaction import ActionTarget
+from sentry.notifications.types import FallthroughChoiceType
 from sentry.testutils.asserts import assert_org_audit_log_exists
 from sentry.testutils.cases import APITestCase
 from sentry.testutils.outbox import outbox_runner
 from sentry.testutils.silo import region_silo_test
 from sentry.workflow_engine.models import (
     Action,
+    DataConditionGroupAction,
     DetectorWorkflow,
     Workflow,
     WorkflowDataConditionGroup,
     WorkflowFireHistory,
 )
+from sentry.workflow_engine.models.data_condition import Condition
 from tests.sentry.workflow_engine.test_base import MockActionValidatorTranslator
 
 
@@ -394,6 +397,13 @@ def setUp(self) -> None:
             role="member",
             organization=self.organization,
         )
+        self.basic_condition = [
+            {
+                "type": Condition.EQUAL.value,
+                "comparison": 1,
+                "conditionResult": True,
+            }
+        ]
 
     @mock.patch("sentry.workflow_engine.endpoints.validators.base.workflow.create_audit_entry")
     def test_create_workflow__basic(self, mock_audit: mock.MagicMock) -> None:
@@ -428,13 +438,7 @@ def test_create_workflow__with_config(self) -> None:
     def test_create_workflow__with_triggers(self) -> None:
         self.valid_workflow["triggers"] = {
             "logicType": "any",
-            "conditions": [
-                {
-                    "type": "eq",
-                    "comparison": 1,
-                    "conditionResult": True,
-                }
-            ],
+            "conditions": self.basic_condition,
         }
 
         response = self.get_success_response(
@@ -457,13 +461,7 @@ def test_create_workflow__with_actions(self, mock_action_validator: mock.MagicMo
         self.valid_workflow["actionFilters"] = [
             {
                 "logicType": "any",
-                "conditions": [
-                    {
-                        "type": "eq",
-                        "comparison": 1,
-                        "conditionResult": True,
-                    }
-                ],
+                "conditions": self.basic_condition,
                 "actions": [
                     {
                         "type": Action.Type.SLACK,
@@ -492,6 +490,55 @@ def test_create_workflow__with_actions(self, mock_action_validator: mock.MagicMo
             "actionFilters", []
         )[0].get("id")
 
+    @mock.patch(
+        "sentry.notifications.notification_action.registry.action_validator_registry.get",
+        return_value=MockActionValidatorTranslator,
+    )
+    def test_create_workflow__with_fallthrough_type_action(
+        self, mock_action_validator: mock.MagicMock
+    ) -> None:
+        self.valid_workflow["actionFilters"] = [
+            {
+                "logicType": "any",
+                "conditions": self.basic_condition,
+                "actions": [
+                    {
+                        "type": Action.Type.EMAIL,
+                        "config": {
+                            "targetType": "issue_owners",
+                        },
+                        "data": {"fallthroughType": "ActiveMembers"},
+                    },
+                ],
+            }
+        ]
+
+        response = self.get_success_response(
+            self.organization.slug,
+            raw_data=self.valid_workflow,
+        )
+
+        assert response.status_code == 201
+        new_workflow = Workflow.objects.get(id=response.data["id"])
+        new_action_filters = WorkflowDataConditionGroup.objects.filter(workflow=new_workflow)
+        assert len(new_action_filters) == len(response.data.get("actionFilters", []))
+        dcga = DataConditionGroupAction.objects.filter(
+            condition_group=new_action_filters[0].condition_group
+        ).first()
+        assert dcga
+        assert str(new_action_filters[0].condition_group.id) == response.data.get(
+            "actionFilters", []
+        )[0].get("id")
+        assert (
+            response.data.get("actionFilters")[0]
+            .get("actions")[0]
+            .get("data")
+            .get("fallthrough_type")
+            == FallthroughChoiceType.ACTIVE_MEMBERS.value
+        )
+        assert dcga.action.type == Action.Type.EMAIL
+        assert dcga.action.data == {"fallthrough_type": "ActiveMembers"}
+
     def test_create_invalid_workflow(self) -> None:
         self.valid_workflow["name"] = ""
         response = self.get_response(

tests/sentry/workflow_engine/migration_helpers/test_migrate_rule_action.py

@@ -114,8 +114,8 @@ def assert_action_data_blob(
                         assert action.data.get(field) == source_value
                     else:
                         # For unmapped fields, check directly with empty string default
-                        if action.type == Action.Type.EMAIL and field == "fallthroughType":
-                            # for email actions, the default value for fallthroughType should be "ActiveMembers"
+                        if action.type == Action.Type.EMAIL and field == "fallthrough_type":
+                            # for email actions, the default value for fallthrough_type should be "ActiveMembers"
                             assert action.data.get(field) == compare_dict.get(
                                 field, "ActiveMembers"
                             )
@@ -131,10 +131,10 @@ def assert_action_data_blob(
                 if key not in exclude_keys:
                     if (
                         action.type == Action.Type.EMAIL
-                        and key == "fallthroughType"
+                        and key == "fallthrough_type"
                         and action.config.get("target_type") != ActionTarget.ISSUE_OWNERS
                     ):
-                        # for email actions, fallthroughType should only be set for when targetType is ISSUE_OWNERS
+                        # for email actions, fallthrough_type should only be set for when targetType is ISSUE_OWNERS
                         continue
                     else:
                         assert compare_dict[key] == action.data[key]
@@ -626,9 +626,9 @@ def test_email_migration_malformed(self) -> None:
             {
                 "uuid": "12345678-90ab-cdef-0123-456789abcdef",
                 "id": "sentry.mail.actions.NotifyEmailAction",
-                "fallthroughType": "NoOne",
+                "fallthrough_type": "NoOne",
             },
-            # This should be ok since we have a default value for fallthroughType
+            # This should be ok since we have a default value for fallthrough_type
             {
                 "uuid": "12345678-90ab-cdef-0123-456789abcdef",
                 "id": "sentry.mail.actions.NotifyEmailAction",