From f51bc0cf79ad6e281bed57eaf6a3536783cac554 Mon Sep 17 00:00:00 2001 From: Josh Callender <1569818+saponifi3d@users.noreply.github.com> Date: Mon, 16 Dec 2024 18:34:46 -0800 Subject: [PATCH 1/3] update the type to be the specific type of notification, this will help UI / notfication action --- src/sentry/testutils/helpers/backups.py | 2 +- src/sentry/workflow_engine/handlers/action/notification.py | 4 +++- src/sentry/workflow_engine/models/action.py | 4 +++- tests/sentry/workflow_engine/models/test_action.py | 6 +++--- tests/sentry/workflow_engine/test_base.py | 2 +- 5 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/sentry/testutils/helpers/backups.py b/src/sentry/testutils/helpers/backups.py index 76c8c2f48a1585..7a4fb90c175f55 100644 --- a/src/sentry/testutils/helpers/backups.py +++ b/src/sentry/testutils/helpers/backups.py @@ -667,7 +667,7 @@ def create_exhaustive_organization( organization=org, ) - send_notification_action = self.create_action(type=Action.Type.NOTIFICATION, data="") + send_notification_action = self.create_action(type=Action.Type.SLACK, data="") self.create_data_condition_group_action( action=send_notification_action, condition_group=notification_condition_group, diff --git a/src/sentry/workflow_engine/handlers/action/notification.py b/src/sentry/workflow_engine/handlers/action/notification.py index 97e8cf84f39818..c9637f32466442 100644 --- a/src/sentry/workflow_engine/handlers/action/notification.py +++ b/src/sentry/workflow_engine/handlers/action/notification.py @@ -4,7 +4,9 @@ from sentry.workflow_engine.types import ActionHandler -@action_handler_registry.register(Action.Type.NOTIFICATION) +# TODO - Enable once the PR to allow for multiple of the same funcs is merged +# @action_handler_registry.register(Action.Type.PAGERDUTY) +@action_handler_registry.register(Action.Type.SLACK) class NotificationActionHandler(ActionHandler): @staticmethod def execute( diff --git a/src/sentry/workflow_engine/models/action.py b/src/sentry/workflow_engine/models/action.py index 027ef5cf11c933..1d27615c8b0f9d 100644 --- a/src/sentry/workflow_engine/models/action.py +++ b/src/sentry/workflow_engine/models/action.py @@ -30,7 +30,9 @@ class Action(DefaultFieldsModel): __repr__ = sane_repr("id", "type") class Type(models.TextChoices): - NOTIFICATION = "notification" + EMAIL = "email" + SLACK = "slack" + PAGERDUTY = "pagerduty" WEBHOOK = "webhook" # The type field is used to denote the type of action we want to trigger diff --git a/tests/sentry/workflow_engine/models/test_action.py b/tests/sentry/workflow_engine/models/test_action.py index 32251b47c2937a..9f8983c75ad642 100644 --- a/tests/sentry/workflow_engine/models/test_action.py +++ b/tests/sentry/workflow_engine/models/test_action.py @@ -13,7 +13,7 @@ class TestAction(TestCase): def setUp(self): self.mock_event = Mock(spec=GroupEvent) self.mock_detector = Mock(name="detector") - self.action = Action(type=Action.Type.NOTIFICATION) + self.action = Action(type=Action.Type.SLACK) def test_get_handler_notification_type(self): with patch("sentry.workflow_engine.registry.action_handler_registry.get") as mock_get: @@ -22,7 +22,7 @@ def test_get_handler_notification_type(self): handler = self.action.get_handler() - mock_get.assert_called_once_with(Action.Type.NOTIFICATION) + mock_get.assert_called_once_with(Action.Type.SLACK) assert handler == mock_handler def test_get_handler_webhook_type(self): @@ -49,7 +49,7 @@ def test_get_handler_unregistered_type(self): self.action.get_handler() # Verify the registry was queried with the correct action type - mock_get.assert_called_once_with(Action.Type.NOTIFICATION) + mock_get.assert_called_once_with(Action.Type.SLACK) def test_trigger_calls_handler_execute(self): mock_handler = Mock(spec=ActionHandler) diff --git a/tests/sentry/workflow_engine/test_base.py b/tests/sentry/workflow_engine/test_base.py index edf4cb531bc4b2..3543af270134c5 100644 --- a/tests/sentry/workflow_engine/test_base.py +++ b/tests/sentry/workflow_engine/test_base.py @@ -108,7 +108,7 @@ def create_workflow_action( action_group = self.create_data_condition_group(logic_type="any-short") action = self.create_action( - type=Action.Type.NOTIFICATION, + type=Action.Type.SLACK, data={"message": "test"}, **kwargs, ) From 409617664dea3dfa72390b6a2cddef5905b57440 Mon Sep 17 00:00:00 2001 From: Josh Callender <1569818+saponifi3d@users.noreply.github.com> Date: Mon, 16 Dec 2024 19:25:13 -0800 Subject: [PATCH 2/3] Add legacy_notification_type to denote if it's metric or issue alerts. --- src/sentry/workflow_engine/models/action.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/sentry/workflow_engine/models/action.py b/src/sentry/workflow_engine/models/action.py index 1d27615c8b0f9d..d6b90a7fa8846d 100644 --- a/src/sentry/workflow_engine/models/action.py +++ b/src/sentry/workflow_engine/models/action.py @@ -35,6 +35,10 @@ class Type(models.TextChoices): PAGERDUTY = "pagerduty" WEBHOOK = "webhook" + class LegacyNotificationType(models.TextChoices): + ISSUE_ALERT = "issue" + METRIC_ALERT = "metric" + # The type field is used to denote the type of action we want to trigger type = models.TextField(choices=Type.choices) data = models.JSONField(default=dict) @@ -48,6 +52,13 @@ class Type(models.TextChoices): "sentry.Integration", blank=True, null=True, on_delete="CASCADE" ) + # LEGACY: The legacy_notification_type is used to denote if this notification was for an issue alert, metric alert, etc. + # We need this because of how tightly coupled the notification system is with the legacy alert models + legacy_notification_type = models.TextField( + null=True, + choices=LegacyNotificationType.choices, + ) + # LEGACY: The target_display is used to display the target's name in notifications target_display = models.TextField(null=True) From 2b2c12bef9ac9561bcb86f7ce63802a43fe685b2 Mon Sep 17 00:00:00 2001 From: Josh Callender <1569818+saponifi3d@users.noreply.github.com> Date: Mon, 16 Dec 2024 19:32:38 -0800 Subject: [PATCH 3/3] SQL Migration --- migrations_lockfile.txt | 2 +- .../migrations/0016_refactor_action_model.py | 33 +++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 src/sentry/workflow_engine/migrations/0016_refactor_action_model.py diff --git a/migrations_lockfile.txt b/migrations_lockfile.txt index ef62e79b102a35..81b3b06b3c3397 100644 --- a/migrations_lockfile.txt +++ b/migrations_lockfile.txt @@ -21,4 +21,4 @@ social_auth: 0002_default_auto_field uptime: 0018_add_trace_sampling_field_to_uptime -workflow_engine: 0015_create_rule_lookup_tables +workflow_engine: 0016_refactor_action_model diff --git a/src/sentry/workflow_engine/migrations/0016_refactor_action_model.py b/src/sentry/workflow_engine/migrations/0016_refactor_action_model.py new file mode 100644 index 00000000000000..15511e36ffd139 --- /dev/null +++ b/src/sentry/workflow_engine/migrations/0016_refactor_action_model.py @@ -0,0 +1,33 @@ +# Generated by Django 5.1.4 on 2024-12-17 03:31 + +from django.db import migrations, models + +from sentry.new_migrations.migrations import CheckedMigration + + +class Migration(CheckedMigration): + # This flag is used to mark that a migration shouldn't be automatically run in production. + # This should only be used for operations where it's safe to run the migration after your + # code has deployed. So this should not be used for most operations that alter the schema + # of a table. + # Here are some things that make sense to mark as post deployment: + # - Large data migrations. Typically we want these to be run manually so that they can be + # monitored and not block the deploy for a long period of time while they run. + # - Adding indexes to large tables. Since this can take a long time, we'd generally prefer to + # run this outside deployments so that we don't block them. Note that while adding an index + # is a schema change, it's completely safe to run the operation after the code has deployed. + # Once deployed, run these manually via: https://develop.sentry.dev/database-migrations/#migration-deployment + + is_post_deployment = False + + dependencies = [ + ("workflow_engine", "0015_create_rule_lookup_tables"), + ] + + operations = [ + migrations.AddField( + model_name="action", + name="legacy_notification_type", + field=models.TextField(null=True), + ), + ]