diff --git a/src/sentry/notifications/utils/links.py b/src/sentry/notifications/utils/links.py index 1a5138c76e5c..598256b2d229 100644 --- a/src/sentry/notifications/utils/links.py +++ b/src/sentry/notifications/utils/links.py @@ -107,29 +107,8 @@ def get_rules( project: Project, type_id: int | None = None, ) -> list[NotificationRuleDetails]: - legacy_rules = [rule for rule in rules if rule.legacy_rule_id is not None] - workflow_rules = [rule for rule in rules if rule.legacy_rule_id is None] - - return get_workflow_links(workflow_rules, organization, project) + get_rules_with_legacy_ids( - legacy_rules, organization, project - ) - - -def get_rules_with_legacy_ids( - rules: Sequence[NotificationOrigin], organization: Organization, project: Project -) -> list[NotificationRuleDetails]: - rules_with_legacy_ids = [] - for rule in rules: - rule_id = rule.legacy_rule_id - assert rule_id is not None - rules_with_legacy_ids.append( - NotificationRuleDetails( - rule_id, - rule.label, - f"/organizations/{organization.slug}/issues/alerts/rules/{project.slug}/{rule_id}/details/", - ) - ) - return rules_with_legacy_ids + workflow_rules = [rule for rule in rules if rule.workflow_id is not None] + return get_workflow_links(workflow_rules, organization, project) def get_workflow_links( diff --git a/src/sentry/templates/sentry/emails/digests/body.html b/src/sentry/templates/sentry/emails/digests/body.html index fd6bace074ca..a5d6ca1ca5fe 100644 --- a/src/sentry/templates/sentry/emails/digests/body.html +++ b/src/sentry/templates/sentry/emails/digests/body.html @@ -51,7 +51,7 @@

{{ counts|length }} new alert{{ counts|pluralize }} from
- {% with rule_details=rules_details|get_item:rule.link_id %} + {% with rule_details=rules_details|get_item:rule.workflow_id %} This email was triggered by {{ rule_details.label }} {% endwith %}
@@ -69,7 +69,7 @@

{{ counts|length }} new alert{{ counts|pluralize }} from {{ group.get_level_display }} - {% include "sentry/emails/_group.html" with rule_id=rule.link_id %} + {% include "sentry/emails/_group.html" with rule_id=rule.workflow_id %}
{% if clock_24_hours %} diff --git a/src/sentry/web/frontend/debug/debug_feedback_issue.py b/src/sentry/web/frontend/debug/debug_feedback_issue.py index 35816c1d44f3..7dff398da923 100644 --- a/src/sentry/web/frontend/debug/debug_feedback_issue.py +++ b/src/sentry/web/frontend/debug/debug_feedback_issue.py @@ -27,8 +27,8 @@ def get(self, request: HttpRequest) -> HttpResponse: origin = NotificationOrigin( label="An example rule", environment_id=None, - workflow_id=None, - legacy_rule_id=1, + workflow_id=1, + legacy_rule_id=None, ) rules = get_rules([origin], org, project, group.type) diff --git a/src/sentry/web/frontend/debug/debug_generic_issue.py b/src/sentry/web/frontend/debug/debug_generic_issue.py index e145d0686bd6..ab550a0efe4f 100644 --- a/src/sentry/web/frontend/debug/debug_generic_issue.py +++ b/src/sentry/web/frontend/debug/debug_generic_issue.py @@ -28,8 +28,8 @@ def get(self, request: HttpRequest) -> HttpResponse: origin = NotificationOrigin( label="An example rule", environment_id=None, - workflow_id=None, - legacy_rule_id=1, + workflow_id=1, + legacy_rule_id=None, ) rules = get_rules([origin], org, project, group.type) diff --git a/src/sentry/web/frontend/debug/debug_performance_issue.py b/src/sentry/web/frontend/debug/debug_performance_issue.py index 3c6918bb1948..4da3dfa49b5b 100644 --- a/src/sentry/web/frontend/debug/debug_performance_issue.py +++ b/src/sentry/web/frontend/debug/debug_performance_issue.py @@ -30,8 +30,8 @@ def get( origin = NotificationOrigin( label="Example performance rule", environment_id=None, - workflow_id=None, - legacy_rule_id=1, + workflow_id=1, + legacy_rule_id=None, ) transaction_data = get_transaction_data(perf_event) diff --git a/src/sentry/web/frontend/debug/mail.py b/src/sentry/web/frontend/debug/mail.py index bbfd8401ba26..9b6def061060 100644 --- a/src/sentry/web/frontend/debug/mail.py +++ b/src/sentry/web/frontend/debug/mail.py @@ -480,8 +480,8 @@ def alert(request: HttpRequest) -> HttpResponse: origin = NotificationOrigin( label="An example rule", environment_id=None, - workflow_id=None, - legacy_rule_id=1, + workflow_id=1, + legacy_rule_id=None, ) notification_reason = ( random.randint(0, 1) > 0 @@ -523,8 +523,8 @@ def digest(request: HttpRequest) -> HttpResponse: i: NotificationOrigin( label=f"Rule #{i}", environment_id=None, - workflow_id=None, - legacy_rule_id=i, + workflow_id=i, + legacy_rule_id=None, ) for i in range(1, random.randint(2, 4)) } diff --git a/tests/sentry/notifications/notifications/test_digests.py b/tests/sentry/notifications/notifications/test_digests.py index 7acb7a16ef33..0e17aacdf10c 100644 --- a/tests/sentry/notifications/notifications/test_digests.py +++ b/tests/sentry/notifications/notifications/test_digests.py @@ -199,6 +199,23 @@ def test_sends_digest_with_workflow_engine_ui(self) -> None: self.run_test(event_count=2) assert "new alerts since" in mail.outbox[0].subject + def test_sends_digest_with_workflow_link_for_dual_written_rule(self) -> None: + rule = self.create_project_rule(project=self.project) + self.create_alert_rule_workflow(rule_id=rule.id, workflow=self.workflow) + + self.run_test(event_count=2) + + message = mail.outbox[0] + assert isinstance(message, EmailMultiAlternatives) + html = str(message.alternatives[0][0]) + assert ( + f"/organizations/{self.organization.slug}/monitors/alerts/{self.workflow.id}/" in html + ) + assert ( + f"/organizations/{self.organization.slug}/issues/alerts/rules/" + f"{self.project.slug}/{rule.id}/details/" not in html + ) + def test_digest_email_uses_user_timezone(self) -> None: self.organization.member_set.exclude(user_id=self.user.id).delete() diff --git a/tests/sentry/notifications/test_helpers.py b/tests/sentry/notifications/test_helpers.py index db8cd7aa215d..6912a9ad5b55 100644 --- a/tests/sentry/notifications/test_helpers.py +++ b/tests/sentry/notifications/test_helpers.py @@ -21,7 +21,10 @@ ) from sentry.silo.base import SiloMode from sentry.testutils.cases import TestCase -from sentry.testutils.helpers.notifications import create_notification_origin +from sentry.testutils.helpers.notifications import ( + create_notification_origin, + create_workflow_notification_origin_for_rule, +) from sentry.testutils.silo import assume_test_silo_mode @@ -92,14 +95,14 @@ def test_get_subscription_from_attributes(self) -> None: def test_collect_groups_by_project(self) -> None: assert collect_groups_by_project([self.group]) == {self.project.id: {self.group}} - def test_get_group_settings_link(self) -> None: + def test_get_group_settings_link_uses_workflow(self) -> None: rule: Rule = self.create_project_rule(self.project) - origin = create_notification_origin(rule) + origin = create_workflow_notification_origin_for_rule(rule) rule_details = get_rules([origin], self.organization, self.project, self.group.type) - assert rule_details[0].id == rule.id + assert origin.workflow_id is not None + assert rule_details[0].id == origin.workflow_id assert rule_details[0].status_url == ( - f"/organizations/{self.organization.slug}/issues/alerts/rules/" - f"{self.project.slug}/{rule.id}/details/" + f"/organizations/{self.organization.slug}/monitors/alerts/{origin.workflow_id}/" ) link = get_group_settings_link( self.group, self.environment.name, rule_details, 1337, extra="123" @@ -134,17 +137,11 @@ def test_get_rules_uses_workflow_identity_without_legacy_rule(self) -> None: f"/organizations/{self.organization.slug}/monitors/alerts/{workflow_id}/" ) - def test_get_rules_falls_back_to_rule_id_without_embedded_identity(self) -> None: + def test_get_rules_omits_origin_without_workflow_identity(self) -> None: rule = self.create_project_rule(self.project) origin = create_notification_origin(rule) - [rule_details] = get_rules([origin], self.organization, self.project) - - assert rule_details.id == rule.id - assert rule_details.status_url == ( - f"/organizations/{self.organization.slug}/issues/alerts/rules/" - f"{self.project.slug}/{rule.id}/details/" - ) + assert get_rules([origin], self.organization, self.project) == [] def test_notification_origin_identifies_test_notification(self) -> None: origin = NotificationOrigin( @@ -176,8 +173,8 @@ def test_get_email_link_extra_params(self) -> None: project2 = self.create_project() rule2 = self.create_project_rule(project2) origins = [ - create_notification_origin(rule), - create_notification_origin(rule2), + create_workflow_notification_origin_for_rule(rule), + create_workflow_notification_origin_for_rule(rule2), ] rule_details = get_rules(origins, self.organization, self.project, self.group.type)