diff --git a/src/sentry/digests/notifications.py b/src/sentry/digests/notifications.py index c12eebe5531c..0568b708ed83 100644 --- a/src/sentry/digests/notifications.py +++ b/src/sentry/digests/notifications.py @@ -3,17 +3,15 @@ import logging from collections import defaultdict from collections.abc import Mapping, Sequence +from dataclasses import replace from typing import Any, NamedTuple, TypeAlias -import sentry_sdk - from sentry import tsdb from sentry.digests.types import IdentifierKey, Notification, Record, RecordWithRuleObjects from sentry.models.group import Group, GroupStatus from sentry.models.project import Project from sentry.models.rule import Rule -from sentry.notifications.types import ActionTargetType, FallthroughChoiceType -from sentry.notifications.utils.rules import get_rule_or_workflow_id +from sentry.notifications.types import ActionTargetType, FallthroughChoiceType, NotificationRule from sentry.services.eventstore.models import Event, GroupEvent from sentry.tsdb.base import TSDBModel from sentry.workflow_engine.models import Workflow @@ -21,7 +19,7 @@ logger = logging.getLogger("sentry.digests") -Digest: TypeAlias = dict[Rule, dict[Group, list[RecordWithRuleObjects]]] +Digest: TypeAlias = dict[NotificationRule, dict[Group, list[RecordWithRuleObjects]]] class DigestInfo(NamedTuple): @@ -78,7 +76,7 @@ def unsplit_key( def event_to_record( event: Event | GroupEvent, - rules: Sequence[Rule], + rules: Sequence[NotificationRule], notification_uuid: str | None = None, identifier_key: IdentifierKey = IdentifierKey.RULE, ) -> Record: @@ -88,7 +86,11 @@ def event_to_record( # TODO(iamrajjoshi): The typing on this function is wrong, the type should be GroupEvent # TODO(iamrajjoshi): Creating a PR to fix this assert event.group is not None - rule_ids = [int(get_rule_or_workflow_id(rule)[1]) for rule in rules] + rule_ids = [] + for rule in rules: + rule_id = rule.legacy_rule_id if identifier_key == IdentifierKey.RULE else rule.workflow_id + assert rule_id is not None + rule_ids.append(rule_id) return Record( event.event_id, Notification(event, rule_ids, notification_uuid, identifier_key), @@ -97,7 +99,7 @@ def event_to_record( def _bind_records( - records: Sequence[Record], groups: dict[int, Group], rules: dict[int, Rule] + records: Sequence[Record], groups: dict[int, Group], rules: dict[int, NotificationRule] ) -> list[RecordWithRuleObjects]: ret = [] for record in records: @@ -123,7 +125,9 @@ def _bind_records( def _group_records( - records: Sequence[RecordWithRuleObjects], groups: dict[int, Group], rules: dict[int, Rule] + records: Sequence[RecordWithRuleObjects], + groups: dict[int, Group], + rules: dict[int, NotificationRule], ) -> Digest: grouped: Digest = defaultdict(lambda: defaultdict(list)) for record in records: @@ -161,7 +165,7 @@ def _sort_digest( def _build_digest_impl( records: Sequence[Record], groups: dict[int, Group], - rules: dict[int, Rule], + rules: dict[int, NotificationRule], event_counts: dict[int, int], user_counts: Mapping[Any, int], ) -> Digest: @@ -171,8 +175,10 @@ def _build_digest_impl( return _sort_digest(grouped, event_counts=event_counts, user_counts=user_counts) -def get_rules_from_workflows(project: Project, workflow_ids: set[int]) -> dict[int, Rule]: - rules: dict[int, Rule] = {} +def get_rules_from_workflows( + project: Project, workflow_ids: set[int] +) -> dict[int, NotificationRule]: + rules: dict[int, NotificationRule] = {} if not workflow_ids: return rules @@ -194,27 +200,22 @@ def get_rules_from_workflows(project: Project, workflow_ids: set[int]) -> dict[i if alert_workflow: if rule := bulk_rules.get(alert_workflow.rule_id): assert rule.project_id == project.id, "Rule must belong to Project" - rule.environment_id = workflow.environment_id - try: - rule.data["actions"][0]["legacy_rule_id"] = rule.id - rule.data["actions"][0]["workflow_id"] = workflow_id - except KeyError: - # This shouldn't happen, but isn't a deal breaker if it does - sentry_sdk.capture_exception( - Exception(f"Rule {rule.id} does not have a legacy_rule_id"), - level="warning", - ) - rules[workflow_id] = rule + rules[workflow_id] = replace( + NotificationRule.from_deprecated_legacy_rule( + rule, project=project, workflow_id=workflow_id + ), + environment_id=workflow.environment_id, + ) continue - # Create synthetic Rule when no AlertRuleWorkflow or no Rule found - rules[workflow_id] = Rule( + rules[workflow_id] = NotificationRule( label=workflow.name, - id=workflow_id, - project_id=project.id, + action_id=None, + project=project, environment_id=workflow.environment_id, - # We need to do this so that the links are built correctly downstream data={"actions": [{"workflow_id": workflow_id}]}, + workflow_id=workflow_id, + legacy_rule_id=None, ) return rules @@ -245,17 +246,10 @@ def build_digest(project: Project, records: Sequence[Record]) -> DigestInfo: groups = Group.objects.in_bulk(record.value.event.group_id for record in records) group_ids = list(groups) - rules = Rule.objects.in_bulk(rule_ids) - - for rule in rules.values(): - try: - rule.data["actions"][0]["legacy_rule_id"] = rule.id - except KeyError: - # This shouldn't happen, but isn't a deal breaker if it does - sentry_sdk.capture_exception( - Exception(f"Rule {rule.id} does not have a legacy_rule_id"), - level="warning", - ) + rules = { + rule_id: NotificationRule.from_deprecated_legacy_rule(rule, project=project) + for rule_id, rule in Rule.objects.in_bulk(rule_ids).items() + } rules.update(get_rules_from_workflows(project, workflow_ids)) diff --git a/src/sentry/digests/types.py b/src/sentry/digests/types.py index c1c1aa5c0d56..dc3ed112cd66 100644 --- a/src/sentry/digests/types.py +++ b/src/sentry/digests/types.py @@ -8,7 +8,7 @@ from sentry.utils.dates import to_datetime if TYPE_CHECKING: - from sentry.models.rule import Rule + from sentry.notifications.types import NotificationRule from sentry.services.eventstore.models import Event, GroupEvent @@ -23,7 +23,7 @@ class Notification(NamedTuple): notification_uuid: str | None = None identifier_key: IdentifierKey = IdentifierKey.RULE - def with_rules(self, rules: list[Rule]) -> NotificationWithRuleObjects: + def with_rules(self, rules: list[NotificationRule]) -> NotificationWithRuleObjects: return NotificationWithRuleObjects( event=self.event, rules=rules, @@ -41,7 +41,7 @@ class Record(NamedTuple): def datetime(self) -> datetime_mod.datetime: return to_datetime(self.timestamp) - def with_rules(self, rules: list[Rule]) -> RecordWithRuleObjects: + def with_rules(self, rules: list[NotificationRule]) -> RecordWithRuleObjects: return RecordWithRuleObjects( key=self.key, value=self.value.with_rules(rules), @@ -51,7 +51,7 @@ def with_rules(self, rules: list[Rule]) -> RecordWithRuleObjects: class NotificationWithRuleObjects(NamedTuple): event: Event | GroupEvent - rules: list[Rule] + rules: list[NotificationRule] notification_uuid: str | None diff --git a/src/sentry/digests/utils.py b/src/sentry/digests/utils.py index 811358ccfcd4..4d44265daddd 100644 --- a/src/sentry/digests/utils.py +++ b/src/sentry/digests/utils.py @@ -11,8 +11,7 @@ from sentry.models.group import Group from sentry.models.project import Project from sentry.models.projectownership import ProjectOwnership -from sentry.models.rule import Rule -from sentry.notifications.types import ActionTargetType, FallthroughChoiceType +from sentry.notifications.types import ActionTargetType, FallthroughChoiceType, NotificationRule from sentry.notifications.utils.participants import get_send_to from sentry.services.eventstore.models import Event, GroupEvent from sentry.types.actor import Actor @@ -166,7 +165,7 @@ def sort_func(record: Record) -> datetime: return sorted(records, key=sort_func, reverse=True) -def get_groups(digest: Digest) -> Sequence[tuple[Rule, Group, Event | GroupEvent]]: +def get_groups(digest: Digest) -> Sequence[tuple[NotificationRule, Group, Event | GroupEvent]]: """ Split a digest into groups and return it as a tuple of: the applicable rule, the group, and the group's first event. diff --git a/src/sentry/integrations/discord/message_builder/issues.py b/src/sentry/integrations/discord/message_builder/issues.py index 452f963907ce..e06191aec11c 100644 --- a/src/sentry/integrations/discord/message_builder/issues.py +++ b/src/sentry/integrations/discord/message_builder/issues.py @@ -23,9 +23,9 @@ from sentry.integrations.types import ExternalProviders from sentry.models.group import Group, GroupStatus from sentry.models.project import Project -from sentry.models.rule import Rule from sentry.notifications.notifications.base import ProjectNotification -from sentry.notifications.utils.rules import RuleIdType, get_rule_or_workflow_id +from sentry.notifications.types import NotificationRule +from sentry.notifications.utils.rules import get_rule_or_workflow_id from sentry.services.eventstore.models import GroupEvent from ..message_builder.base.component import DiscordComponentCustomIds as CustomIds @@ -37,7 +37,7 @@ def __init__( group: Group, event: GroupEvent | None = None, tags: set[str] | None = None, - rules: list[Rule] | None = None, + rules: list[NotificationRule] | None = None, link_to_event: bool = False, issue_details: bool = False, notification: ProjectNotification | None = None, @@ -57,39 +57,40 @@ def build(self, notification_uuid: str | None = None) -> DiscordMessage: max(self.group.last_seen, self.event.datetime) if self.event else self.group.last_seen ) obj: Group | GroupEvent = self.event if self.event is not None else self.group - rule_id = None + rule_id: int | None = None rule_environment_id = None - key: RuleIdType = "legacy_rule_id" + is_workflow = False if self.rules: rule_environment_id = self.rules[0].environment_id - key, rule_id = get_rule_or_workflow_id(self.rules[0], prefer="workflow_id") + key, rule_id_value = get_rule_or_workflow_id(self.rules[0], prefer="workflow_id") + is_workflow = key == "workflow_id" + rule_id = int(rule_id_value) url = None - match key: - case "workflow_id": - url = get_title_link_workflow_engine_ui( - self.group, - self.event, - self.link_to_event, - self.issue_details, - self.notification, - ExternalProviders.DISCORD, - int(rule_id) if rule_id else None, - rule_environment_id, - notification_uuid=notification_uuid, - ) - case "legacy_rule_id": - url = get_title_link( - self.group, - self.event, - self.link_to_event, - self.issue_details, - self.notification, - ExternalProviders.DISCORD, - int(rule_id) if rule_id else None, - rule_environment_id, - notification_uuid=notification_uuid, - ) + if is_workflow: + url = get_title_link_workflow_engine_ui( + self.group, + self.event, + self.link_to_event, + self.issue_details, + self.notification, + ExternalProviders.DISCORD, + rule_id, + rule_environment_id, + notification_uuid=notification_uuid, + ) + else: + url = get_title_link( + self.group, + self.event, + self.link_to_event, + self.issue_details, + self.notification, + ExternalProviders.DISCORD, + rule_id, + rule_environment_id, + notification_uuid=notification_uuid, + ) embeds = [ DiscordMessageEmbed( diff --git a/src/sentry/integrations/github/actions/create_ticket.py b/src/sentry/integrations/github/actions/create_ticket.py index 585c1334a20c..578b5aa2ade1 100644 --- a/src/sentry/integrations/github/actions/create_ticket.py +++ b/src/sentry/integrations/github/actions/create_ticket.py @@ -14,4 +14,4 @@ class GitHubCreateTicketAction(TicketEventAction): provider = IntegrationProviderSlug.GITHUB.value def generate_footer(self, rule_url: str) -> str: - return f"\nThis issue was automatically created by Sentry via [{self.rule.label}]({absolute_uri(rule_url)})" + return f"\nThis issue was automatically created by Sentry via [{self.rule_context.label}]({absolute_uri(rule_url)})" diff --git a/src/sentry/integrations/github_enterprise/actions/create_ticket.py b/src/sentry/integrations/github_enterprise/actions/create_ticket.py index b3848dee32e9..ca7fe0a342a1 100644 --- a/src/sentry/integrations/github_enterprise/actions/create_ticket.py +++ b/src/sentry/integrations/github_enterprise/actions/create_ticket.py @@ -14,4 +14,4 @@ class GitHubEnterpriseCreateTicketAction(TicketEventAction): provider = IntegrationProviderSlug.GITHUB_ENTERPRISE.value def generate_footer(self, rule_url: str) -> str: - return f"\nThis issue was automatically created by Sentry via [{self.rule.label}]({absolute_uri(rule_url)})" + return f"\nThis issue was automatically created by Sentry via [{self.rule_context.label}]({absolute_uri(rule_url)})" diff --git a/src/sentry/integrations/jira/actions/create_ticket.py b/src/sentry/integrations/jira/actions/create_ticket.py index 68170054e498..c43dd1acf7f1 100644 --- a/src/sentry/integrations/jira/actions/create_ticket.py +++ b/src/sentry/integrations/jira/actions/create_ticket.py @@ -24,7 +24,7 @@ def __init__(self, *args: Any, **kwargs: Any) -> None: self.data["fixVersions"] = [fix_versions] def generate_footer(self, rule_url: str) -> str: - return f"This ticket was automatically created by Sentry via [{self.rule.label}|{absolute_uri(rule_url)}]" + return f"This ticket was automatically created by Sentry via [{self.rule_context.label}|{absolute_uri(rule_url)}]" def translate_integration(self, integration: RpcIntegration) -> str: name = integration.metadata.get("domain_name", integration.name) diff --git a/src/sentry/integrations/jira_server/actions/create_ticket.py b/src/sentry/integrations/jira_server/actions/create_ticket.py index 5a7b87448658..0b436a36ec52 100644 --- a/src/sentry/integrations/jira_server/actions/create_ticket.py +++ b/src/sentry/integrations/jira_server/actions/create_ticket.py @@ -24,7 +24,7 @@ def __init__(self, *args: Any, **kwargs: Any) -> None: self.data["fixVersions"] = [fix_versions] def generate_footer(self, rule_url: str) -> str: - return f"This ticket was automatically created by Sentry via [{self.rule.label}|{absolute_uri(rule_url)}]" + return f"This ticket was automatically created by Sentry via [{self.rule_context.label}|{absolute_uri(rule_url)}]" def translate_integration(self, integration: RpcIntegration) -> str: return integration.metadata.get("domain_name", integration.name) diff --git a/src/sentry/integrations/messaging/message_builder.py b/src/sentry/integrations/messaging/message_builder.py index 2d2fad7e921d..21cae0889c66 100644 --- a/src/sentry/integrations/messaging/message_builder.py +++ b/src/sentry/integrations/messaging/message_builder.py @@ -9,12 +9,12 @@ from sentry.models.environment import Environment from sentry.models.group import Group from sentry.models.project import Project -from sentry.models.rule import Rule from sentry.models.team import Team from sentry.notifications.notifications.base import BaseNotification from sentry.notifications.notifications.rules import AlertRuleNotification +from sentry.notifications.types import NotificationRule from sentry.notifications.utils.links import create_link_to_workflow -from sentry.notifications.utils.rules import get_key_from_rule_data, get_rule_or_workflow_id +from sentry.notifications.utils.rules import get_rule_or_workflow_id from sentry.services.eventstore.models import Event, GroupEvent from sentry.users.services.user import RpcUser from sentry.utils.http import absolute_uri @@ -250,10 +250,11 @@ def build_attachment_replay_link( return None -def build_rule_url(rule: Any, group: Group, project: Project) -> str: +def build_rule_url(rule: NotificationRule, group: Group, project: Project) -> str: org_slug = group.organization.slug project_slug = project.slug - rule_id = get_key_from_rule_data(rule, "legacy_rule_id") + rule_id = rule.legacy_rule_id + assert rule_id is not None rule_url = f"/organizations/{org_slug}/issues/alerts/rules/{project_slug}/{rule_id}/details/" return absolute_uri(rule_url) @@ -263,7 +264,7 @@ def build_footer( group: Group, project: Project, url_format: str, - rules: Sequence[Rule] | None = None, + rules: Sequence[NotificationRule] | None = None, ) -> str: footer = f"{group.qualified_short_id}" if rules: diff --git a/src/sentry/integrations/msteams/actions/notification.py b/src/sentry/integrations/msteams/actions/notification.py index a437aaa2df46..8008e2d90479 100644 --- a/src/sentry/integrations/msteams/actions/notification.py +++ b/src/sentry/integrations/msteams/actions/notification.py @@ -1,5 +1,7 @@ from __future__ import annotations +from collections.abc import Generator, Sequence + from sentry.integrations.messaging.metrics import ( MessagingInteractionEvent, MessagingInteractionType, @@ -11,7 +13,9 @@ from sentry.integrations.msteams.spec import MsTeamsMessagingSpec from sentry.integrations.services.integration import RpcIntegration from sentry.integrations.types import IntegrationProviderSlug +from sentry.notifications.types import RuleFuture from sentry.rules.actions import IntegrationEventAction +from sentry.rules.base import CallbackFuture from sentry.services.eventstore.models import GroupEvent from sentry.shared_integrations.exceptions import ApiError, IntegrationError from sentry.utils import metrics @@ -41,14 +45,16 @@ def get_integrations(self) -> list[RpcIntegration]: a for a in super().get_integrations() if a.metadata.get("installation_type") != "tenant" ] - def after(self, event: GroupEvent, notification_uuid: str | None = None): + def after( + self, event: GroupEvent, notification_uuid: str | None = None + ) -> Generator[CallbackFuture]: channel = self.get_option("channel_id") integration = self.get_integration() if not integration: return - def send_notification(event, futures): + def send_notification(event: GroupEvent, futures: Sequence[RuleFuture]) -> None: rules = [f.rule for f in futures] card = MSTeamsIssueMessageBuilder( event.group, event, rules, integration diff --git a/src/sentry/integrations/msteams/card_builder/issues.py b/src/sentry/integrations/msteams/card_builder/issues.py index 0f1e28f15705..b9bc43d762f7 100644 --- a/src/sentry/integrations/msteams/card_builder/issues.py +++ b/src/sentry/integrations/msteams/card_builder/issues.py @@ -29,7 +29,7 @@ from sentry.integrations.types import IntegrationProviderSlug from sentry.models.group import Group, GroupStatus from sentry.models.project import Project -from sentry.models.rule import Rule +from sentry.notifications.types import NotificationRule from sentry.services.eventstore.models import Event, GroupEvent from .base import MSTeamsMessageBuilder @@ -52,16 +52,8 @@ logger = logging.getLogger(__name__) -def get_workflow_ids(rules: Sequence[Rule]) -> list[int]: - workflow_ids = [] - for rule in rules: - action = rule.data.get("actions", [{}])[0] - workflow_id = action.get("workflow_id") - - if workflow_id is not None: - workflow_ids.append(int(workflow_id)) - - return workflow_ids +def get_workflow_ids(rules: Sequence[NotificationRule]) -> list[int]: + return [rule.workflow_id for rule in rules if rule.workflow_id is not None] class MSTeamsIssueMessageBuilder(MSTeamsMessageBuilder): @@ -69,7 +61,7 @@ def __init__( self, group: Group, event: Event | GroupEvent | None, - rules: Sequence[Rule], + rules: Sequence[NotificationRule], integration: RpcIntegration, workflow_ids: Sequence[int] = (), ): @@ -87,7 +79,7 @@ def generate_action_payload(self, action_type: ACTION_TYPE) -> Any: "actionType": action_type, "groupId": self.group.id, "eventId": self.event.event_id if self.event else None, - "rules": [rule.id for rule in self.rules], + "rules": [rule.broken_rule_id for rule in self.rules], "workflows": list(dict.fromkeys([*workflow_ids, *self.workflow_ids])), "integrationId": self.integration.id, } diff --git a/src/sentry/integrations/msteams/webhook.py b/src/sentry/integrations/msteams/webhook.py index 777d0ea8ca3b..301e7ab01fc8 100644 --- a/src/sentry/integrations/msteams/webhook.py +++ b/src/sentry/integrations/msteams/webhook.py @@ -51,6 +51,7 @@ from sentry.models.apikey import ApiKey from sentry.models.group import Group from sentry.models.rule import Rule +from sentry.notifications.types import NotificationRule from sentry.services import eventstore from sentry.silo.base import SiloMode from sentry.users.services.user.service import user_service @@ -653,7 +654,10 @@ def _handle_action_submitted(self, request: Request) -> Response: # get the rules from the payload rule_ids = payload.get("rules", []) workflow_ids = payload.get("workflows", []) - rules = tuple(Rule.objects.filter(id__in=rule_ids, project_id=group.project_id)) + rules = tuple( + NotificationRule.from_deprecated_legacy_rule(rule, project=group.project) + for rule in Rule.objects.filter(id__in=rule_ids, project_id=group.project_id) + ) metrics.incr( "integrations.msteams.action.rule_lookup", tags={ diff --git a/src/sentry/integrations/opsgenie/actions/notification.py b/src/sentry/integrations/opsgenie/actions/notification.py index fde38da45ce4..686d650153e5 100644 --- a/src/sentry/integrations/opsgenie/actions/notification.py +++ b/src/sentry/integrations/opsgenie/actions/notification.py @@ -1,6 +1,7 @@ from __future__ import annotations import logging +from collections.abc import Generator, Sequence from typing import cast import sentry_sdk @@ -14,7 +15,10 @@ from sentry.integrations.opsgenie.utils import get_team from sentry.integrations.services.integration import integration_service from sentry.integrations.types import IntegrationProviderSlug +from sentry.notifications.types import RuleFuture from sentry.rules.actions import IntegrationEventAction +from sentry.rules.base import CallbackFuture +from sentry.services.eventstore.models import GroupEvent from sentry.shared_integrations.exceptions import ApiError logger = logging.getLogger("sentry.integrations.opsgenie") @@ -43,7 +47,9 @@ def __init__(self, *args, **kwargs): }, } - def after(self, event, notification_uuid: str | None = None): + def after( + self, event: GroupEvent, notification_uuid: str | None = None + ) -> Generator[CallbackFuture]: integration = self.get_integration() if not integration: logger.warning("Integration removed, but the rule still refers to it") @@ -66,7 +72,7 @@ def after(self, event, notification_uuid: str | None = None): ) return - def send_notification(event, futures): + def send_notification(event: GroupEvent, futures: Sequence[RuleFuture]) -> None: installation = integration.get_installation(self.project.organization_id) try: client: OpsgenieClient = installation.get_keyring_client(self.get_option("team")) diff --git a/src/sentry/integrations/opsgenie/client.py b/src/sentry/integrations/opsgenie/client.py index 937c47b2e91c..d397d1e7fb58 100644 --- a/src/sentry/integrations/opsgenie/client.py +++ b/src/sentry/integrations/opsgenie/client.py @@ -1,6 +1,7 @@ from __future__ import annotations -from typing import Literal +from collections.abc import Sequence +from typing import Any, Literal from sentry.integrations.client import ApiClient from sentry.integrations.models.integration import Integration @@ -9,8 +10,9 @@ from sentry.integrations.services.integration.model import RpcIntegration from sentry.integrations.types import IntegrationProviderSlug from sentry.models.group import Group +from sentry.notifications.types import NotificationRule from sentry.notifications.utils.links import create_link_to_workflow -from sentry.notifications.utils.rules import get_key_from_rule_data, split_rules_by_rule_workflow_id +from sentry.notifications.utils.rules import split_rules_by_rule_workflow_id from sentry.services.eventstore.models import Event, GroupEvent from sentry.shared_integrations.exceptions import ApiError @@ -41,35 +43,38 @@ def get_alerts(self, limit: int | None = 1) -> object | None: path = f"/alerts?limit={limit}" return self.get(path=path, headers=self._get_auth_headers()) - def _get_workflow_urls(self, group, rules): + def _get_workflow_urls(self, group: Group, rules: Sequence[NotificationRule]) -> list[str]: organization = group.project.organization workflow_urls = [] for rule in rules: - # fetch the workflow_id from the rule.data - workflow_id = get_key_from_rule_data(rule, "workflow_id") + workflow_id = rule.workflow_id + assert workflow_id is not None workflow_urls.append( - organization.absolute_url(create_link_to_workflow(organization.slug, workflow_id)) + organization.absolute_url( + create_link_to_workflow(organization.slug, str(workflow_id)) + ) ) return workflow_urls - def _get_rule_urls(self, group, rules): + def _get_rule_urls(self, group: Group, rules: Sequence[NotificationRule]) -> list[str]: organization = group.project.organization rule_urls = [] for rule in rules: - rule_id = get_key_from_rule_data(rule, "legacy_rule_id") + rule_id = rule.legacy_rule_id + assert rule_id is not None path = f"/organizations/{organization.slug}/issues/alerts/rules/{group.project.slug}/{rule_id}/details/" rule_urls.append(organization.absolute_url(path)) return rule_urls def build_issue_alert_payload( self, - data, - rules, + data: Any, + rules: Sequence[NotificationRule], event: Event | GroupEvent, group: Group | None, priority: OpsgeniePriority | None = "P3", notification_uuid: str | None = None, - ): + ) -> dict[str, Any]: payload = { "message": event.message or event.title, "source": "Sentry", diff --git a/src/sentry/integrations/pagerduty/actions/notification.py b/src/sentry/integrations/pagerduty/actions/notification.py index ffbe449773ef..d1c144faef1a 100644 --- a/src/sentry/integrations/pagerduty/actions/notification.py +++ b/src/sentry/integrations/pagerduty/actions/notification.py @@ -14,8 +14,7 @@ build_pagerduty_event_payload, ) from sentry.integrations.types import IntegrationProviderSlug -from sentry.models.rule import Rule -from sentry.notifications.types import RuleFuture +from sentry.notifications.types import NotificationRule, RuleFuture from sentry.rules.actions import IntegrationEventAction from sentry.rules.base import CallbackFuture from sentry.services.eventstore.models import GroupEvent @@ -106,7 +105,7 @@ def send_notification(event: GroupEvent, futures: Sequence[RuleFuture]) -> None: severity=severity, ) - rules: list[Rule] = [f.rule for f in futures] + rules: list[NotificationRule] = [f.rule for f in futures] rule = rules[0] if rules else None if rule and rule.label: diff --git a/src/sentry/integrations/slack/actions/notification.py b/src/sentry/integrations/slack/actions/notification.py index a97c05e28a42..6aa4446aadae 100644 --- a/src/sentry/integrations/slack/actions/notification.py +++ b/src/sentry/integrations/slack/actions/notification.py @@ -29,9 +29,8 @@ from sentry.integrations.slack.utils.threads import NotificationActionThreadUtils from sentry.integrations.types import IntegrationProviderSlug from sentry.integrations.utils.metrics import EventLifecycle -from sentry.models.rule import Rule from sentry.notifications.additional_attachment_manager import get_additional_attachment -from sentry.notifications.types import RuleFuture +from sentry.notifications.types import NotificationRule, RuleFuture from sentry.notifications.utils.open_period import open_period_start_for_group from sentry.rules.actions import IntegrationEventAction from sentry.rules.base import CallbackFuture @@ -77,7 +76,7 @@ def _should_send_nudge(self, channel_id: str | None) -> bool: def _build_notification_blocks( self, event: GroupEvent, - rules: Sequence[Rule], + rules: Sequence[NotificationRule], tags: set, integration: RpcIntegration, notification_uuid: str | None = None, @@ -241,7 +240,13 @@ def _send_notification_action_notification( rule = rules[0] if rules else None rule_to_use = self.rule if self.rule else rule # In the NOA, we will store the action id in the rule id field - action_id = rule_to_use.id if rule_to_use else None + action_id = ( + rule_to_use.broken_rule_id + if isinstance(rule_to_use, NotificationRule) + else rule_to_use.id + if rule_to_use + else None + ) if not action_id: # We are logging because this should never happen, all actions should have an uuid diff --git a/src/sentry/integrations/slack/message_builder/issues.py b/src/sentry/integrations/slack/message_builder/issues.py index a219941ae9d1..046f90960ec5 100644 --- a/src/sentry/integrations/slack/message_builder/issues.py +++ b/src/sentry/integrations/slack/message_builder/issues.py @@ -48,10 +48,10 @@ from sentry.models.project import Project from sentry.models.projectownership import ProjectOwnership from sentry.models.release import Release -from sentry.models.rule import Rule from sentry.models.team import Team from sentry.notifications.notifications.base import ProjectNotification from sentry.notifications.platform.slack.renderers.seer import SeerSlackRenderer +from sentry.notifications.types import NotificationRule from sentry.notifications.utils.actions import BlockKitMessageAction, MessageAction from sentry.notifications.utils.participants import ( dedupe_suggested_assignees, @@ -65,7 +65,6 @@ from sentry.types.actor import Actor from sentry.types.group import SUBSTATUS_TO_STR from sentry.users.services.user.model import RpcUser -from sentry.workflow_engine.models import Workflow STATUSES = {"resolved": "resolved", "ignored": "ignored", "unresolved": "re-opened"} MAX_BLOCK_TEXT_LENGTH = 256 @@ -73,7 +72,7 @@ MAX_SUGGESTED_ASSIGNEES = 3 -def get_group_users_count(group: Group, rules: list[Rule] | None = None) -> int: +def get_group_users_count(group: Group, rules: list[NotificationRule] | None = None) -> int: environment_ids: list[int] | None = None if rules: environment_ids = [rule.environment_id for rule in rules if rule.environment_id is not None] @@ -195,7 +194,7 @@ def get_tags( return fields -def get_context(group: Group, rules: list[Rule] | None = None) -> str: +def get_context(group: Group, rules: list[NotificationRule] | None = None) -> str: context_text = "" context = group.issue_type.notification_config.context.copy() @@ -416,7 +415,7 @@ def __init__( tags: set[str] | None = None, identity: RpcIdentity | None = None, actions: Sequence[MessageAction | BlockKitMessageAction] | None = None, - rules: list[Rule] | None = None, + rules: list[NotificationRule] | None = None, link_to_event: bool = False, issue_details: bool = False, notification: ProjectNotification | None = None, @@ -613,18 +612,10 @@ def build(self, notification_uuid: str | None = None) -> SlackBlock: # handler, so it keeps preferring the legacy rule id. _, value = get_rule_or_workflow_id(self.rules[0]) rule_id = int(value) - action = self.rules[0].data.get("actions", [{}])[0] - if action.get("workflow_id") is not None: - workflow_id = int(action["workflow_id"]) - + workflow_id = self.rules[0].workflow_id or workflow_id link_key, link_value = get_rule_or_workflow_id(self.rules[0], prefer="workflow_id") link_id = int(link_value) - match link_key: - case "workflow_id": - workflow = Workflow.objects.filter(id=link_id).first() - rule_environment_id = workflow.environment_id if workflow else None - case "legacy_rule_id": - rule_environment_id = self.rules[0].environment_id + rule_environment_id = self.rules[0].environment_id # build up actions text if self.actions and self.identity and not action_text: diff --git a/src/sentry/integrations/slack/message_builder/util.py b/src/sentry/integrations/slack/message_builder/util.py index 2c302ecb4914..9ba5d49dbcd9 100644 --- a/src/sentry/integrations/slack/message_builder/util.py +++ b/src/sentry/integrations/slack/message_builder/util.py @@ -4,7 +4,7 @@ from sentry.integrations.slack.message_builder.types import SLACK_URL_FORMAT from sentry.models.group import Group from sentry.models.project import Project -from sentry.models.rule import Rule +from sentry.notifications.types import NotificationRule from sentry.notifications.utils.links import create_link_to_workflow from sentry.notifications.utils.rules import get_rule_or_workflow_id from sentry.utils.http import absolute_uri @@ -13,7 +13,7 @@ def build_slack_footer( group: Group, project: Project, - rules: Sequence[Rule] | None = None, + rules: Sequence[NotificationRule] | None = None, ) -> str: footer = f"{group.qualified_short_id}" diff --git a/src/sentry/integrations/slack/webhooks/action.py b/src/sentry/integrations/slack/webhooks/action.py index d3f8c1c8a8f7..a9ba7986aba5 100644 --- a/src/sentry/integrations/slack/webhooks/action.py +++ b/src/sentry/integrations/slack/webhooks/action.py @@ -59,6 +59,7 @@ from sentry.models.organizationmember import InviteStatus, OrganizationMember from sentry.models.rule import Rule from sentry.notifications.services import notifications_service +from sentry.notifications.types import NotificationRule from sentry.notifications.utils.actions import BlockKitMessageAction, MessageAction from sentry.seer.entrypoints.operator import SeerAutofixOperator from sentry.seer.entrypoints.slack.entrypoint import SlackAutofixEntrypoint @@ -136,18 +137,18 @@ def update_group( return resp -def get_rule(rule_id: int | None, organization_id: int) -> Rule | None: +def get_rule(rule_id: int | None, organization_id: int) -> NotificationRule | None: """Get the rule that fired""" if not rule_id: return None try: - # Scope the callback-provided rule ID to the integration-validated organization + # Scope the callback-provided rule ID to the integration-validated organization. rule = Rule.objects.get(id=rule_id, project__organization_id=organization_id) - # We need to add the legacy_rule_id field to the rule data since the message builder will use it to build the link to the rule + # The callback contract puns Rule and Workflow IDs, so preserve Rule.id here. rule.data["actions"][0]["legacy_rule_id"] = rule.id except Rule.DoesNotExist: return None - return rule + return NotificationRule.from_deprecated_legacy_rule(rule) def get_group(slack_request: SlackActionRequest) -> Group | None: diff --git a/src/sentry/integrations/vsts/actions/create_ticket.py b/src/sentry/integrations/vsts/actions/create_ticket.py index 100fd6d3ce9d..4b5e7c96829c 100644 --- a/src/sentry/integrations/vsts/actions/create_ticket.py +++ b/src/sentry/integrations/vsts/actions/create_ticket.py @@ -15,4 +15,4 @@ class AzureDevopsCreateTicketAction(TicketEventAction): provider = IntegrationProviderSlug.AZURE_DEVOPS.value def generate_footer(self, rule_url: str) -> str: - return f"\nThis work item was automatically created by Sentry via [{self.rule.label}]({absolute_uri(rule_url)})" + return f"\nThis work item was automatically created by Sentry via [{self.rule_context.label}]({absolute_uri(rule_url)})" diff --git a/src/sentry/mail/adapter.py b/src/sentry/mail/adapter.py index dfeb6e731737..999aa965fab8 100644 --- a/src/sentry/mail/adapter.py +++ b/src/sentry/mail/adapter.py @@ -65,7 +65,7 @@ def rule_notify( log_event = "dispatched" for future in futures: rules.append(future.rule) - extra["rule_id"] = future.rule.id + extra["rule_id"] = future.rule.broken_rule_id if not future.kwargs: continue raise NotImplementedError( diff --git a/src/sentry/notifications/notification_action/types.py b/src/sentry/notifications/notification_action/types.py index 2500403bb124..6aca62dd26ec 100644 --- a/src/sentry/notifications/notification_action/types.py +++ b/src/sentry/notifications/notification_action/types.py @@ -2,7 +2,7 @@ from abc import ABC, abstractmethod from collections.abc import Callable, Collection, Sequence from dataclasses import asdict -from typing import Any, ClassVar, NotRequired, Protocol, TypedDict +from typing import Any, ClassVar, Protocol from django.core.exceptions import ValidationError from taskbroker_client.retry import RetryTaskError @@ -23,8 +23,13 @@ from sentry.models.activity import Activity from sentry.models.organization import Organization from sentry.models.project import Project -from sentry.models.rule import Rule, RuleSource -from sentry.notifications.types import TEST_NOTIFICATION_ID, RuleFuture +from sentry.models.rule import Rule +from sentry.notifications.types import ( + TEST_NOTIFICATION_ID, + NotificationRule, + NotificationRuleData, + RuleFuture, +) from sentry.notifications.utils.issue_notification_context import IssueNotificationContext from sentry.rules.processing.processor import activate_downstream_actions from sentry.services.eventstore.models import GroupEvent @@ -52,11 +57,6 @@ FutureCallback = Callable[[GroupEvent, Sequence[RuleFuture]], Any] -class RuleData(TypedDict): - actions: list[dict[str, Any]] - legacy_rule_id: NotRequired[int] - - class LegacyRegistryHandler(ABC): """ Abstract base class that defines the interface for notification handlers. @@ -206,7 +206,7 @@ def create_rule_instance_from_action( detector: Detector, event_data: WorkflowEventData, workflow_id: WorkflowId, - ) -> Rule: + ) -> NotificationRule: """ Creates a Rule instance from the Action model. :param action: Action @@ -217,7 +217,7 @@ def create_rule_instance_from_action( """ environment_id = event_data.workflow_env.id if event_data.workflow_env else None - data: RuleData = { + data: NotificationRuleData = { "actions": [ cls.build_rule_action_blob(action, detector.linked_project.organization.id) ], @@ -271,14 +271,14 @@ def create_rule_instance_from_action( # mail action needs to have skipDigests set to True data["actions"][0]["skipDigests"] = True - rule = Rule( - id=action.id, + rule = NotificationRule( + action_id=action.id, project=detector.linked_project, environment_id=environment_id, label=label, - data=dict(data), - status=ObjectStatus.ACTIVE, - source=RuleSource.ISSUE, + data=data, + workflow_id=(None if workflow_id == TEST_NOTIFICATION_ID else workflow_id), + legacy_rule_id=(data["actions"][0].get("legacy_rule_id")), ) return rule @@ -286,7 +286,7 @@ def create_rule_instance_from_action( @staticmethod def get_rule_futures( event_data: WorkflowEventData, - rule: Rule, + rule: NotificationRule, notification_uuid: str, ) -> Collection[tuple[Callable[[GroupEvent, Sequence[RuleFuture]], None], list[RuleFuture]]]: """ @@ -360,7 +360,7 @@ def invoke_legacy_registry(cls, invocation: ActionInvocation) -> None: "action_id": invocation.action.id, "detector_id": invocation.detector.id, "event_data": asdict(invocation.event_data), - "rule_id": rule.id, + "rule_id": rule.broken_rule_id, "rule_project_id": rule.project.id, "rule_environment_id": rule.environment_id, "rule_label": rule.label, @@ -372,7 +372,7 @@ def invoke_legacy_registry(cls, invocation: ActionInvocation) -> None: # Execute the futures # If the rule id is -1, we are sending a test notification - if rule.id == TEST_NOTIFICATION_ID: + if rule.is_test_notification: cls.send_test_notification(invocation.event_data, futures) else: cls.execute_futures(invocation.event_data, futures) diff --git a/src/sentry/notifications/notifications/digest.py b/src/sentry/notifications/notifications/digest.py index 1cb14dfc3e03..85ddf3eaa4b8 100644 --- a/src/sentry/notifications/notifications/digest.py +++ b/src/sentry/notifications/notifications/digest.py @@ -265,7 +265,7 @@ def send(self) -> None: def get_log_params(self, recipient: Actor) -> Mapping[str, Any]: try: - alert_id = list(self.digest.digest)[0].id + alert_id = list(self.digest.digest)[0].broken_rule_id except Exception: alert_id = None diff --git a/src/sentry/notifications/notifications/rules.py b/src/sentry/notifications/notifications/rules.py index 5606c67321a1..63f03b82399e 100644 --- a/src/sentry/notifications/notifications/rules.py +++ b/src/sentry/notifications/notifications/rules.py @@ -28,6 +28,7 @@ from sentry.notifications.types import ( ActionTargetType, FallthroughChoiceType, + NotificationRule, NotificationSettingEnum, ) from sentry.notifications.utils import ( @@ -97,7 +98,12 @@ def __init__( self.target_type = target_type self.target_identifier = target_identifier self.fallthrough_choice = fallthrough_choice - self.rules = notification.rules + self.rules = [ + rule + if isinstance(rule, NotificationRule) + else NotificationRule.from_deprecated_legacy_rule(rule, project=project) + for rule in notification.rules + ] if ( event.group.issue_category in GROUP_CATEGORIES_CUSTOM_EMAIL @@ -355,7 +361,7 @@ def get_log_params(self, recipient: Actor) -> Mapping[str, Any]: return { "target_type": self.target_type, "target_identifier": self.target_identifier, - "alert_id": self.rules[0].id if self.rules else None, + "alert_id": self.rules[0].broken_rule_id if self.rules else None, **super().get_log_params(recipient), } diff --git a/src/sentry/notifications/platform/discord/renderers/issue.py b/src/sentry/notifications/platform/discord/renderers/issue.py index e1dfe0a31aa7..70fc2a1ff3a9 100644 --- a/src/sentry/notifications/platform/discord/renderers/issue.py +++ b/src/sentry/notifications/platform/discord/renderers/issue.py @@ -50,7 +50,7 @@ def render[DataT: NotificationData]( except Exception: raise NotificationRenderError(f"Failed to retrieve event {data.event_id}") - rules = [data.rule.to_rule()] if data.rule else [] + rules = [data.rule.to_notification_rule(group.project)] if data.rule else [] return DiscordIssuesMessageBuilder( group=group, diff --git a/src/sentry/notifications/platform/msteams/renderers/issue.py b/src/sentry/notifications/platform/msteams/renderers/issue.py index 29a3974e207b..9e92eec0c33a 100644 --- a/src/sentry/notifications/platform/msteams/renderers/issue.py +++ b/src/sentry/notifications/platform/msteams/renderers/issue.py @@ -8,7 +8,6 @@ from sentry.integrations.types import IntegrationProviderSlug from sentry.models.group import Group, GroupStatus from sentry.models.project import Project -from sentry.models.rule import Rule from sentry.notifications.platform.msteams.provider import MSTeamsRenderable from sentry.notifications.platform.registry import renderer_registry from sentry.notifications.platform.renderer import NotificationRenderer @@ -20,6 +19,7 @@ NotificationRenderedTemplate, NotificationSource, ) +from sentry.notifications.types import NotificationRule from sentry.services.eventstore.models import Event, GroupEvent from sentry.types.actor import Actor @@ -65,7 +65,7 @@ def render[DataT: NotificationData]( except Exception: raise NotificationRenderError(f"Failed to retrieve event {data.event_id}") - rules = [data.rule.to_rule()] if data.rule else [] + rules = [data.rule.to_notification_rule(group.project)] if data.rule else [] issue_url = cls.build_issue_url(group=group, notification_uuid=data.notification_uuid) fields: list[Block | None] = [ @@ -125,7 +125,7 @@ def build_footer( *, group: Group, event: Event | GroupEvent | None, - rules: Sequence[Rule], + rules: Sequence[NotificationRule], ) -> ColumnSetBlock: from sentry.integrations.messaging.message_builder import build_footer from sentry.integrations.msteams.card_builder import MSTEAMS_URL_FORMAT @@ -186,7 +186,11 @@ def build_assignee_note(cls, group: Group) -> TextBlock | None: @classmethod def build_action_payload( - cls, *, action_type: ACTION_TYPE, data: IssueNotificationData, rules: Sequence[Rule] + cls, + *, + action_type: ACTION_TYPE, + data: IssueNotificationData, + rules: Sequence[NotificationRule], ) -> dict[str, Any]: # Keep this lazy to avoid initializing the msteams package during notifications app startup. from sentry.integrations.msteams.card_builder.issues import get_workflow_ids @@ -198,7 +202,7 @@ def build_action_payload( "actionType": action_type, "groupId": data.group_id, "eventId": data.event_id, - "rules": [rule.id for rule in rules], + "rules": [rule.broken_rule_id for rule in rules], "workflows": get_workflow_ids(rules), } } @@ -213,7 +217,7 @@ def build_action( reverse_action: ACTION_TYPE, reverse_action_title: str, data: IssueNotificationData, - rules: Sequence[Rule], + rules: Sequence[NotificationRule], **card_kwargs: Any, ) -> Action: """ @@ -253,7 +257,7 @@ def build_assignee_choices(cls, group: Group) -> Sequence[tuple[str, str]]: @classmethod def build_actions( - cls, *, group: Group, data: IssueNotificationData, rules: Sequence[Rule] + cls, *, group: Group, data: IssueNotificationData, rules: Sequence[NotificationRule] ) -> ContainerBlock: from sentry.integrations.msteams.card_builder import ME from sentry.integrations.msteams.card_builder.block import ( diff --git a/src/sentry/notifications/platform/slack/renderers/issue.py b/src/sentry/notifications/platform/slack/renderers/issue.py index 76642c29fb1f..1447ba55c90b 100644 --- a/src/sentry/notifications/platform/slack/renderers/issue.py +++ b/src/sentry/notifications/platform/slack/renderers/issue.py @@ -34,7 +34,7 @@ def render[DataT: NotificationData]( group=group, event=event, tags=set(data.tags) if data.tags else None, - rules=[data.rule.to_rule()] if data.rule else None, + rules=[data.rule.to_notification_rule(group.project)] if data.rule else None, notes=data.notes, link_to_event=True, ).build(notification_uuid=data.notification_uuid) diff --git a/src/sentry/notifications/platform/templates/issue.py b/src/sentry/notifications/platform/templates/issue.py index 1873bd6c9a3e..af3de9d4e7a3 100644 --- a/src/sentry/notifications/platform/templates/issue.py +++ b/src/sentry/notifications/platform/templates/issue.py @@ -4,7 +4,7 @@ from pydantic import BaseModel, ConfigDict -from sentry.models.rule import Rule +from sentry.models.project import Project from sentry.notifications.platform.registry import template_registry from sentry.notifications.platform.types import ( NotificationCategory, @@ -13,6 +13,7 @@ NotificationSource, NotificationTemplate, ) +from sentry.notifications.types import TEST_NOTIFICATION_ID, NotificationRule, NotificationRuleData class SerializableRuleProxy(BaseModel): @@ -23,36 +24,59 @@ class SerializableRuleProxy(BaseModel): model_config = ConfigDict(frozen=True) id: int + action_id: int | None = None label: str data: dict[str, Any] environment_id: int | None = None project_id: int + workflow_id: int | None = None + legacy_rule_id: int | None = None @classmethod - def from_rule(cls, rule: Rule) -> SerializableRuleProxy: - """ - Temporary method to convert a Rule to a NotificationRuleInfo. This will - be removed once we no longer rely on the Rule ORM model. - """ + def from_rule(cls, rule: NotificationRule) -> SerializableRuleProxy: + """Create a serializable representation of a notification rule.""" return cls( - id=rule.id, + id=rule.broken_rule_id, + action_id=rule.action_id, label=rule.label, data=rule.data, environment_id=rule.environment_id, project_id=rule.project.id, + workflow_id=rule.workflow_id, + legacy_rule_id=rule.legacy_rule_id, ) - def to_rule(self) -> Rule: - """ - Temporary method to convert a NotificationRuleInfo to a Rule. This will - be removed once we no longer rely on the Rule ORM model. - """ - return Rule( - id=self.id, + def to_notification_rule(self, project: Project) -> NotificationRule: + workflow_id = self.workflow_id + legacy_rule_id = self.legacy_rule_id + actions = self.data.get("actions") + if ( + not isinstance(actions, list) + or not actions + or not all(isinstance(action, dict) for action in actions) + ): + actions = [{}] + data: NotificationRuleData = {"actions": [dict(action) for action in actions]} + if workflow_id is None and legacy_rule_id is None: + # Compatibility for payloads serialized before identities became top-level fields. + action = actions[0] + workflow_id = action.get("workflow_id") + legacy_rule_id = action.get("legacy_rule_id") + workflow_id = int(workflow_id) if workflow_id is not None else None + legacy_rule_id = int(legacy_rule_id) if legacy_rule_id is not None else None + if workflow_id == TEST_NOTIFICATION_ID or legacy_rule_id == TEST_NOTIFICATION_ID: + workflow_id = None + legacy_rule_id = TEST_NOTIFICATION_ID + elif workflow_id is None and legacy_rule_id is None: + legacy_rule_id = self.id + return NotificationRule( + action_id=self.action_id if "action_id" in self.__fields_set__ else self.id, label=self.label, - data=self.data, + data=data, environment_id=self.environment_id, - project_id=self.project_id, + project=project, + workflow_id=workflow_id, + legacy_rule_id=legacy_rule_id, ) @@ -83,6 +107,8 @@ class IssueNotificationTemplate(NotificationTemplate[IssueNotificationData]): data={ "actions": [{"workflow_id": 3}], }, + workflow_id=3, + legacy_rule_id=None, ), ) hide_from_debugger = True diff --git a/src/sentry/notifications/types.py b/src/sentry/notifications/types.py index c76f0f41f638..56ea8b547e54 100644 --- a/src/sentry/notifications/types.py +++ b/src/sentry/notifications/types.py @@ -2,17 +2,156 @@ from dataclasses import dataclass from enum import Enum, StrEnum -from typing import TYPE_CHECKING, Any, NamedTuple +from typing import TYPE_CHECKING, Any, NamedTuple, TypedDict from sentry.hybridcloud.rpc import ValueEqualityEnum if TYPE_CHECKING: from sentry.models.organization import Organization + from sentry.models.project import Project from sentry.models.rule import Rule +class NotificationRuleData(TypedDict): + """Configuration for legacy action instantiation, not notification identity.""" + + actions: list[dict[str, Any]] + + +@dataclass(eq=False, frozen=True) +class NotificationRule: + """Rule-like notification context for the legacy action registry. + + ``workflow_id`` and ``legacy_rule_id`` are the canonical notification identities. + Notification code must not recover identity from ``data["actions"]``. Action data + exists only to configure the legacy action registry. Parsing identity from action + data is restricted to explicit compatibility boundaries for deprecated Rule rows + and payloads serialized before these top-level fields existed. + """ + + action_id: int | None + label: str + data: NotificationRuleData + project: Project + environment_id: int | None + workflow_id: int | None + legacy_rule_id: int | None + + @classmethod + def from_deprecated_legacy_rule( + cls, + rule: Rule, + *, + project: Project | None = None, + workflow_id: int | None = None, + ) -> NotificationRule: + actions = rule.data.get("actions") + if ( + not isinstance(actions, list) + or not actions + or not all(isinstance(action, dict) for action in actions) + ): + # Deprecated rules can reach render-only paths without action data. + actions = [{}] + + first_action = actions[0] + embedded_workflow_id = first_action.get("workflow_id") + embedded_legacy_rule_id = first_action.get("legacy_rule_id") + if embedded_workflow_id is not None: + embedded_workflow_id = int(embedded_workflow_id) + if embedded_legacy_rule_id is not None: + embedded_legacy_rule_id = int(embedded_legacy_rule_id) + if embedded_legacy_rule_id == TEST_NOTIFICATION_ID: + effective_workflow_id = None + legacy_rule_id = TEST_NOTIFICATION_ID + elif workflow_id is not None: + effective_workflow_id = workflow_id + legacy_rule_id = rule.id + elif embedded_legacy_rule_id is not None: + effective_workflow_id = embedded_workflow_id + legacy_rule_id = embedded_legacy_rule_id + elif embedded_workflow_id is not None: + effective_workflow_id = embedded_workflow_id + legacy_rule_id = None + else: + effective_workflow_id = None + legacy_rule_id = rule.id + + return cls( + action_id=None, + label=rule.label, + data={"actions": [dict(action) for action in actions]}, + project=project or rule.project, + environment_id=rule.environment_id, + workflow_id=effective_workflow_id, + legacy_rule_id=legacy_rule_id, + ) + + def __post_init__(self) -> None: + if not self.data["actions"]: + raise ValueError("NotificationRule requires at least one action") + + if self.legacy_rule_id == TEST_NOTIFICATION_ID: + if self.workflow_id is not None: + raise ValueError("Test notification cannot have a workflow ID") + elif self.workflow_id == TEST_NOTIFICATION_ID: + raise ValueError("Workflow ID cannot be the test notification ID") + elif self.workflow_id is None and self.legacy_rule_id is None: + raise ValueError("NotificationRule requires a workflow or legacy rule ID") + + @property + def identifier(self) -> str: + if self.is_test_notification and self.action_id is not None: + return f"test:{self.action_id}" + if self.workflow_id is not None: + return f"workflow:{self.workflow_id}" + assert self.legacy_rule_id is not None + return f"legacy:{self.legacy_rule_id}" + + @property + def broken_rule_id(self) -> int: + """Preserve callers that historically treated several ID domains as Rule.id.""" + if self.action_id is not None: + return self.action_id + if self.legacy_rule_id is not None: + return self.legacy_rule_id + assert self.workflow_id is not None + return self.workflow_id + + def __eq__(self, other: object) -> bool: + if not isinstance(other, NotificationRule): + return NotImplemented + return self.identifier == other.identifier + + def __hash__(self) -> int: + return hash(self.identifier) + + @property + def is_test_notification(self) -> bool: + return self.legacy_rule_id == TEST_NOTIFICATION_ID + + @property + def is_workflow_only(self) -> bool: + return self.workflow_id is not None and self.legacy_rule_id is None + + @property + def is_workflow_with_legacy_rule(self) -> bool: + return self.workflow_id is not None and self.legacy_rule_id is not None + + @property + def is_legacy_rule_only(self) -> bool: + return self.workflow_id is None and self.legacy_rule_id not in ( + None, + TEST_NOTIFICATION_ID, + ) + + @property + def project_id(self) -> int: + return self.project.id + + class RuleFuture(NamedTuple): - rule: Rule + rule: NotificationRule kwargs: dict[str, Any] diff --git a/src/sentry/notifications/utils/links.py b/src/sentry/notifications/utils/links.py index ce3785371ce1..c014759b922c 100644 --- a/src/sentry/notifications/utils/links.py +++ b/src/sentry/notifications/utils/links.py @@ -8,11 +8,8 @@ from sentry.models.group import Group from sentry.models.organization import Organization from sentry.models.project import Project -from sentry.models.rule import Rule -from sentry.notifications.utils.rules import ( - get_key_from_rule_data, - split_rules_by_rule_workflow_id, -) +from sentry.notifications.types import NotificationRule +from sentry.notifications.utils.rules import split_rules_by_rule_workflow_id from sentry.types.rules import NotificationRuleDetails """ @@ -106,7 +103,10 @@ def get_issue_replay_link(group: Group, sentry_query_params: str = "") -> str: def get_rules( - rules: Sequence[Rule], organization: Organization, project: Project, type_id: int | None = None + rules: Sequence[NotificationRule], + organization: Organization, + project: Project, + type_id: int | None = None, ) -> list[NotificationRuleDetails]: rules_and_workflows = split_rules_by_rule_workflow_id(rules) @@ -115,21 +115,13 @@ def get_rules( ) + get_rules_with_legacy_ids(rules_and_workflows.rules, organization, project) -def _fetch_rule_id(rule: Rule, type_id: int | None = None) -> int: - # Try to fetch the legacy rule id, if it fails, return the rule id - # This allows us to support both legacy and new rule ids - try: - return int(get_key_from_rule_data(rule, "legacy_rule_id")) - except AssertionError: - return rule.id - - def get_rules_with_legacy_ids( - rules: Sequence[Rule], organization: Organization, project: Project + rules: Sequence[NotificationRule], organization: Organization, project: Project ) -> list[NotificationRuleDetails]: rules_with_legacy_ids = [] for rule in rules: - rule_id = _fetch_rule_id(rule) + rule_id = rule.legacy_rule_id + assert rule_id is not None rules_with_legacy_ids.append( NotificationRuleDetails( rule_id, @@ -141,16 +133,17 @@ def get_rules_with_legacy_ids( def get_workflow_links( - rules: Sequence[Rule], organization: Organization, project: Project + rules: Sequence[NotificationRule], organization: Organization, project: Project ) -> list[NotificationRuleDetails]: workflow_links = [] for rule in rules: - workflow_id = get_key_from_rule_data(rule, "workflow_id") + workflow_id = rule.workflow_id + assert workflow_id is not None workflow_links.append( NotificationRuleDetails( - int(workflow_id), + workflow_id, rule.label, - create_link_to_workflow(organization.slug, workflow_id), + create_link_to_workflow(organization.slug, str(workflow_id)), ) ) return workflow_links diff --git a/src/sentry/notifications/utils/rules.py b/src/sentry/notifications/utils/rules.py index 39de8a922488..93e39dba09fc 100644 --- a/src/sentry/notifications/utils/rules.py +++ b/src/sentry/notifications/utils/rules.py @@ -2,24 +2,20 @@ from dataclasses import dataclass from typing import Literal -from sentry.models.rule import Rule +from sentry.notifications.types import NotificationRule RuleIdType = Literal["workflow_id", "legacy_rule_id"] -def get_key_from_rule_data(rule: Rule, key: str) -> str: - value = rule.data.get("actions", [{}])[0].get(key) - assert value is not None - return value - - @dataclass class RulesAndWorkflows: - rules: list[Rule] - workflow_rules: list[Rule] # workflows as fake Rules + rules: list[NotificationRule] + workflow_rules: list[NotificationRule] -def split_rules_by_rule_workflow_id(rules: Sequence[Rule]) -> RulesAndWorkflows: +def split_rules_by_rule_workflow_id( + rules: Sequence[NotificationRule], +) -> RulesAndWorkflows: parsed_rules = [] workflow_rules = [] for rule in rules: @@ -33,7 +29,7 @@ def split_rules_by_rule_workflow_id(rules: Sequence[Rule]) -> RulesAndWorkflows: def get_rule_or_workflow_id( - rule: Rule, *, prefer: RuleIdType = "legacy_rule_id" + rule: NotificationRule, *, prefer: RuleIdType = "legacy_rule_id" ) -> tuple[RuleIdType, str]: """ Returns which id the rule data carries, and its value. When both a legacy @@ -45,8 +41,7 @@ def get_rule_or_workflow_id( else ("legacy_rule_id", "workflow_id") ) for key in keys: - try: - return (key, get_key_from_rule_data(rule, key)) - except AssertionError: - pass - return ("legacy_rule_id", str(rule.id)) + value = rule.workflow_id if key == "workflow_id" else rule.legacy_rule_id + if value is not None: + return (key, str(value)) + raise AssertionError("NotificationRule must have a workflow or legacy rule ID") diff --git a/src/sentry/rules/actions/base.py b/src/sentry/rules/actions/base.py index 4ac119e9c094..0b6ad05ecc30 100644 --- a/src/sentry/rules/actions/base.py +++ b/src/sentry/rules/actions/base.py @@ -2,16 +2,19 @@ import abc import logging -from collections.abc import Generator +from collections.abc import Generator, MutableMapping +from typing import Any -from sentry.models.rule import Rule +from sentry.notifications.types import NotificationRule from sentry.rules.base import CallbackFuture, RuleBase from sentry.services.eventstore.models import GroupEvent logger = logging.getLogger("sentry.rules") -def instantiate_action(rule: Rule, action): +def instantiate_action( + rule: NotificationRule, action: MutableMapping[str, Any] +) -> EventAction | None: from sentry.rules import rules action_id = action["id"] diff --git a/src/sentry/rules/actions/integrations/base.py b/src/sentry/rules/actions/integrations/base.py index b88b38de1b42..ee0a0aadbd89 100644 --- a/src/sentry/rules/actions/integrations/base.py +++ b/src/sentry/rules/actions/integrations/base.py @@ -15,8 +15,7 @@ ) from sentry.mail.analytics import EmailNotificationSent from sentry.models.organization import OrganizationStatus -from sentry.models.rule import Rule -from sentry.notifications.types import RuleFuture +from sentry.notifications.types import NotificationRule, RuleFuture from sentry.rules.actions import EventAction from sentry.rules.base import CallbackFuture from sentry.services.eventstore.models import GroupEvent @@ -110,7 +109,7 @@ def record_notification_sent( self, event: GroupEvent, external_id: str, - rule: Rule | None = None, + rule: NotificationRule | None = None, notification_uuid: str | None = None, ) -> None: from sentry.integrations.discord.analytics import DiscordIntegrationNotificationSent @@ -129,6 +128,10 @@ def record_notification_sent( "slack": SlackIntegrationNotificationSent, "email": EmailNotificationSent, } + alert_id = None + if rule is not None: + alert_id = rule.broken_rule_id + try: if event_class := PROVIDER_TO_EVENT_CLASS.get(self.provider): analytics.record( @@ -137,7 +140,7 @@ def record_notification_sent( project_id=event.project_id, group_id=event.group_id, notification_uuid=notification_uuid if notification_uuid else "", - alert_id=rule.id if rule else None, + alert_id=alert_id, category="issue_alert", ) ) @@ -148,7 +151,7 @@ def record_notification_sent( analytics.record( AlertSentEvent( provider=self.provider, - alert_id=rule.id if rule else "", + alert_id=alert_id if alert_id is not None else "", alert_type="issue_alert", organization_id=event.organization.id, project_id=event.project_id, diff --git a/src/sentry/rules/actions/integrations/create_ticket/base.py b/src/sentry/rules/actions/integrations/create_ticket/base.py index 5a5a630a2f3b..a303de710311 100644 --- a/src/sentry/rules/actions/integrations/create_ticket/base.py +++ b/src/sentry/rules/actions/integrations/create_ticket/base.py @@ -5,7 +5,7 @@ from typing import Any from sentry.integrations.services.integration import RpcIntegration -from sentry.models.rule import Rule +from sentry.notifications.types import NotificationRule from sentry.rules.actions.integrations.base import IntegrationEventAction from sentry.rules.actions.integrations.create_ticket.form import IntegrationNotifyServiceForm from sentry.rules.actions.integrations.create_ticket.utils import create_issue @@ -18,7 +18,6 @@ class TicketEventAction(IntegrationEventAction, abc.ABC): integration_key = "integration" link: str | None - rule: Rule def __init__(self, *args: Any, **kwargs: Any) -> None: super(IntegrationEventAction, self).__init__(*args, **kwargs) @@ -47,6 +46,12 @@ def render_label(self) -> str: label: str = self.label.format(integration=self.get_integration_name()) return label + @property + def rule_context(self) -> NotificationRule: + if self.rule is None: + raise TypeError("Ticket delivery requires a rule context") + return self.rule + @property @abc.abstractmethod def ticket_type(self) -> str: diff --git a/src/sentry/rules/actions/integrations/create_ticket/utils.py b/src/sentry/rules/actions/integrations/create_ticket/utils.py index d0f385a73f9b..5a2345edae54 100644 --- a/src/sentry/rules/actions/integrations/create_ticket/utils.py +++ b/src/sentry/rules/actions/integrations/create_ticket/utils.py @@ -138,7 +138,7 @@ def create_issue(event: GroupEvent, futures: Sequence[RuleFuture]) -> None: organization = event.group.project.organization for future in futures: - rule_id = future.rule.id + action_id = future.rule.broken_rule_id data: dict[str, Any] = future.kwargs["data"] provider = future.kwargs.get("provider") integration_id = future.kwargs.get("integration_id") @@ -146,8 +146,7 @@ def create_issue(event: GroupEvent, futures: Sequence[RuleFuture]) -> None: # If we invoked this handler from the notification action, we need to replace the rule_id with the legacy_rule_id, so we link notifications correctly # In the Notification Action, we store the rule_id in the action_id field - action_id = rule_id - rule_id = data.get("legacy_rule_id", rule_id) + legacy_rule_id = future.rule.legacy_rule_id integration = integration_service.get_integration( integration_id=integration_id, @@ -166,13 +165,16 @@ def create_issue(event: GroupEvent, futures: Sequence[RuleFuture]) -> None: ) data["title"] = installation.get_group_title(event.group, event) - workflow_id = data.get("workflow_id") + workflow_id = future.rule.workflow_id if workflow_id is not None: data["description"] = build_description_workflow_engine_ui( event, workflow_id, installation, generate_footer ) else: - data["description"] = build_description(event, rule_id, installation, generate_footer) + assert isinstance(legacy_rule_id, int) + data["description"] = build_description( + event, legacy_rule_id, installation, generate_footer + ) if data.get("dynamic_form_fields"): del data["dynamic_form_fields"] @@ -182,7 +184,7 @@ def create_issue(event: GroupEvent, futures: Sequence[RuleFuture]) -> None: "%s.rule_trigger.link_already_exists", provider, extra={ - "rule_id": rule_id, + "rule_id": legacy_rule_id, "project_id": event.group.project.id, "group_id": event.group.id, }, @@ -195,7 +197,7 @@ def create_issue(event: GroupEvent, futures: Sequence[RuleFuture]) -> None: ).capture() as lifecycle: lifecycle.add_extra("provider", provider) lifecycle.add_extra("integration_id", integration.id) - lifecycle.add_extra("rule_id", rule_id) + lifecycle.add_extra("rule_id", legacy_rule_id) if action_id: lifecycle.add_extra("action_id", action_id) diff --git a/src/sentry/rules/actions/notify_event.py b/src/sentry/rules/actions/notify_event.py index 650a81ab67aa..dca5b998e109 100644 --- a/src/sentry/rules/actions/notify_event.py +++ b/src/sentry/rules/actions/notify_event.py @@ -23,10 +23,14 @@ class NotifyEventAction(EventAction): def after( self, event: GroupEvent, notification_uuid: str | None = None ) -> Generator[CallbackFuture]: + rule_id = None + if self.rule is not None: + rule_id = self.rule.broken_rule_id + logger.info( "notify_event.legacy_plugin_action_noop", extra={ - "rule_id": self.rule.id if self.rule else None, + "rule_id": rule_id, "event_id": event.event_id, "action": self.id, }, diff --git a/src/sentry/rules/base.py b/src/sentry/rules/base.py index 97ecbdf94aad..8137ae93eaa5 100644 --- a/src/sentry/rules/base.py +++ b/src/sentry/rules/base.py @@ -2,19 +2,15 @@ import abc import logging -from collections import namedtuple from collections.abc import Callable, MutableMapping, Sequence -from typing import TYPE_CHECKING, Any, ClassVar +from typing import Any, ClassVar, NamedTuple from django import forms from sentry.models.project import Project -from sentry.notifications.types import RuleFuture +from sentry.notifications.types import NotificationRule, RuleFuture from sentry.services.eventstore.models import GroupEvent -if TYPE_CHECKING: - from sentry.models.rule import Rule - """ Rules apply either before an event gets stored, or immediately after. @@ -45,10 +41,14 @@ - [ACTION:I want to group events when] [RULE:an event matches [FORM]] """ + # Encapsulates a reference to the callback, including arguments. The `key` # attribute may be specifically used to key the callbacks when they are # collated during rule processing. -CallbackFuture = namedtuple("CallbackFuture", ["callback", "kwargs", "key"]) +class CallbackFuture(NamedTuple): + callback: Callable[[GroupEvent, Sequence[RuleFuture]], None] + kwargs: dict[str, Any] + key: str | None class RuleBase(abc.ABC): @@ -58,7 +58,7 @@ def __init__( self, project: Project, data: MutableMapping[str, Any] | None = None, - rule: Rule | None = None, + rule: NotificationRule | None = None, ) -> None: self.project = project self.data = data or {} diff --git a/src/sentry/rules/processing/processor.py b/src/sentry/rules/processing/processor.py index ce43c1f8b625..ca978af417eb 100644 --- a/src/sentry/rules/processing/processor.py +++ b/src/sentry/rules/processing/processor.py @@ -4,8 +4,7 @@ from collections.abc import Callable, Mapping, MutableMapping, Sequence from typing import Any -from sentry.models.rule import Rule -from sentry.notifications.types import RuleFuture +from sentry.notifications.types import NotificationRule, RuleFuture from sentry.rules import rules from sentry.rules.actions.base import instantiate_action from sentry.services.eventstore.models import GroupEvent @@ -64,14 +63,16 @@ def split_conditions_and_filters( def activate_downstream_actions( - rule: Rule, + rule: NotificationRule, event: GroupEvent, notification_uuid: str | None = None, ) -> MutableMapping[ - str, tuple[Callable[[GroupEvent, Sequence[RuleFuture]], None], list[RuleFuture]] + str | Callable[[GroupEvent, Sequence[RuleFuture]], None], + tuple[Callable[[GroupEvent, Sequence[RuleFuture]], None], list[RuleFuture]], ]: grouped_futures: MutableMapping[ - str, tuple[Callable[[GroupEvent, Sequence[RuleFuture]], None], list[RuleFuture]] + str | Callable[[GroupEvent, Sequence[RuleFuture]], None], + tuple[Callable[[GroupEvent, Sequence[RuleFuture]], None], list[RuleFuture]], ] = {} instantiated_actions = 0 diff --git a/src/sentry/sentry_apps/tasks/sentry_apps.py b/src/sentry/sentry_apps/tasks/sentry_apps.py index 059299a46842..f8742488ff2d 100644 --- a/src/sentry/sentry_apps/tasks/sentry_apps.py +++ b/src/sentry/sentry_apps/tasks/sentry_apps.py @@ -46,7 +46,6 @@ from sentry.models.organizationmapping import OrganizationMapping from sentry.models.project import Project from sentry.notifications.types import RuleFuture -from sentry.notifications.utils.rules import get_rule_or_workflow_id from sentry.sentry_apps.api.serializers.app_platform_event import AppPlatformEvent from sentry.sentry_apps.event_types import SentryAppEventType from sentry.sentry_apps.metrics import ( @@ -808,18 +807,16 @@ def notify_sentry_app(event: GroupEvent, futures: Sequence[RuleFuture]) -> None: # If the future comes from a rule with a UI component form in the schema, append the issue alert payload # TODO(ecosystem): We need to change this payload format after alerts create issues - id = f.rule.id - - # if we are using the new workflow engine, we need to use the legacy rule id - # Ignore test notifications - if int(id) != -1: - _, id = get_rule_or_workflow_id(f.rule) + rule_or_workflow_id = ( + f.rule.legacy_rule_id if f.rule.legacy_rule_id is not None else f.rule.workflow_id + ) + assert rule_or_workflow_id is not None settings = f.kwargs.get("schema_defined_settings") if settings: extra_kwargs["additional_payload_key"] = "issue_alert" extra_kwargs["additional_payload"] = { - "id": int(id), + "id": int(rule_or_workflow_id), "title": f.rule.label, "sentry_app_id": f.kwargs["sentry_app"].id, "settings": settings, diff --git a/src/sentry/templates/sentry/emails/_group.html b/src/sentry/templates/sentry/emails/_group.html index c8cebdc850a1..651760e1d251 100644 --- a/src/sentry/templates/sentry/emails/_group.html +++ b/src/sentry/templates/sentry/emails/_group.html @@ -6,7 +6,7 @@

{% if metadata.type %} {%if rule %} - {{metadata.type|truncatechars:40}} + {{metadata.type|truncatechars:40}} {% else %} {{metadata.type|truncatechars:40}} {% endif %} @@ -19,7 +19,7 @@

{% endif %} {% else %} {%if rule %} - {{metadata.value|truncatechars:40}} + {{metadata.value|truncatechars:40}} {% else %} {{metadata.value|truncatechars:40}} {% endif %} @@ -34,7 +34,7 @@

{%if rule %} - {{metadata.directive|truncatechars:40}} + {{metadata.directive|truncatechars:40}} {% else %} {{metadata.directive|truncatechars:40}} {% endif %} @@ -48,7 +48,7 @@

{%if rule %} - {{group.title|truncatechars:40}} + {{group.title|truncatechars:40}} {% else %} {{group.title|truncatechars:40}} {% endif %} diff --git a/src/sentry/templates/sentry/emails/digests/body.html b/src/sentry/templates/sentry/emails/digests/body.html index 6cb5f84b11c3..4358707a5763 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.id snooze_alert_url=snooze_alert_urls|get_item:rule.id %} + {% with rule_details=rules_details|get_item:rule.broken_rule_id snooze_alert_url=snooze_alert_urls|get_item:rule.broken_rule_id %} {% if snooze_alert %} Mute this alert {% endif %} diff --git a/src/sentry/web/frontend/debug/debug_feedback_issue.py b/src/sentry/web/frontend/debug/debug_feedback_issue.py index a481156357d6..d0e259296501 100644 --- a/src/sentry/web/frontend/debug/debug_feedback_issue.py +++ b/src/sentry/web/frontend/debug/debug_feedback_issue.py @@ -6,6 +6,7 @@ from sentry.models.organization import Organization from sentry.models.project import Project from sentry.models.rule import Rule +from sentry.notifications.types import NotificationRule from sentry.notifications.utils import get_generic_data from sentry.notifications.utils.links import get_group_settings_link, get_rules from sentry.utils import json @@ -25,6 +26,7 @@ def get(self, request: HttpRequest) -> HttpResponse: group = event.group rule = Rule(id=1, label="An example rule") + notification_rule = NotificationRule.from_deprecated_legacy_rule(rule, project=project) generic_issue_data_html = get_generic_data(event) section_header = "Issue Data" if generic_issue_data_html else "" @@ -33,14 +35,14 @@ def get(self, request: HttpRequest) -> HttpResponse: text_template="sentry/emails/feedback.txt", context={ "rule": rule, - "rules": get_rules([rule], org, project, group.type), + "rules": get_rules([notification_rule], org, project, group.type), "group": group, "event": event, "timezone": settings.SENTRY_DEFAULT_TIME_ZONE, "link": get_group_settings_link( group, None, - get_rules([rule], org, project, group.type), + get_rules([notification_rule], org, project, group.type), ), "generic_issue_data": [(section_header, mark_safe(generic_issue_data_html), None)], "tags": event.tags, diff --git a/src/sentry/web/frontend/debug/debug_generic_issue.py b/src/sentry/web/frontend/debug/debug_generic_issue.py index f29747f956d8..daf0ffa14039 100644 --- a/src/sentry/web/frontend/debug/debug_generic_issue.py +++ b/src/sentry/web/frontend/debug/debug_generic_issue.py @@ -7,6 +7,7 @@ from sentry.models.organization import Organization from sentry.models.project import Project from sentry.models.rule import Rule +from sentry.notifications.types import NotificationRule from sentry.notifications.utils import get_generic_data from sentry.notifications.utils.links import get_group_settings_link, get_rules from sentry.utils import json @@ -26,6 +27,7 @@ def get(self, request: HttpRequest) -> HttpResponse: group = event.group rule = Rule(id=1, label="An example rule") + notification_rule = NotificationRule.from_deprecated_legacy_rule(rule, project=project) generic_issue_data_html = get_generic_data(event) section_header = "Issue Data" if generic_issue_data_html else "" @@ -35,7 +37,7 @@ def get(self, request: HttpRequest) -> HttpResponse: text_template="sentry/emails/generic.txt", context={ "rule": rule, - "rules": get_rules([rule], org, project, group.type), + "rules": get_rules([notification_rule], org, project, group.type), "group": group, "event": event, "timezone": zoneinfo.ZoneInfo("Europe/Vienna"), @@ -44,7 +46,7 @@ def get(self, request: HttpRequest) -> HttpResponse: "link": get_group_settings_link( group, None, - get_rules([rule], org, project, group.type), + get_rules([notification_rule], org, project, group.type), ), "generic_issue_data": [(section_header, mark_safe(generic_issue_data_html), None)], "tags": event.tags, diff --git a/src/sentry/web/frontend/debug/mail.py b/src/sentry/web/frontend/debug/mail.py index c4bc9c6ae7dc..9db82a158d01 100644 --- a/src/sentry/web/frontend/debug/mail.py +++ b/src/sentry/web/frontend/debug/mail.py @@ -44,7 +44,7 @@ from sentry.notifications.notifications.base import BaseNotification from sentry.notifications.notifications.digest import DigestNotification from sentry.notifications.notifications.rules import get_group_substatus_text -from sentry.notifications.types import GroupSubscriptionReason +from sentry.notifications.types import GroupSubscriptionReason, NotificationRule from sentry.notifications.utils import get_interface_list from sentry.notifications.utils.links import ( get_group_settings_link, @@ -282,7 +282,8 @@ def make_feedback_issue(project: Project) -> GroupEvent: def get_shared_context( rule: Rule, org: Organization, project: Project, group: Group, event: BaseEvent ) -> dict[str, Any]: - rules = get_rules([rule], org, project, group.type) + notification_rule = NotificationRule.from_deprecated_legacy_rule(rule, project=project) + rules = get_rules([notification_rule], org, project, group.type) snooze_alert = len(rules) > 0 snooze_alert_url = rules[0].status_url + urlencode({"mute": "1"}) if snooze_alert else "" return { @@ -517,7 +518,16 @@ def digest(request: HttpRequest) -> HttpResponse: org = Organization(id=1, slug="example", name="Example Organization") project = Project(id=1, slug="example", name="Example Project", organization=org) rules = { - i: Rule(id=i, project=project, label=f"Rule #{i}") for i in range(1, random.randint(2, 4)) + i: NotificationRule( + action_id=None, + label=f"Rule #{i}", + data={"actions": [{}]}, + project=project, + environment_id=None, + workflow_id=None, + legacy_rule_id=i, + ) + for i in range(1, random.randint(2, 4)) } groups = {} event_counts = {} diff --git a/tests/sentry/digests/test_notifications.py b/tests/sentry/digests/test_notifications.py index 92e5c505aa19..a9311a186749 100644 --- a/tests/sentry/digests/test_notifications.py +++ b/tests/sentry/digests/test_notifications.py @@ -1,6 +1,7 @@ from __future__ import annotations import uuid +from dataclasses import replace from functools import cached_property from sentry.digests.notifications import ( @@ -16,7 +17,7 @@ from sentry.models.group import Group from sentry.models.project import Project from sentry.models.rule import Rule -from sentry.notifications.types import ActionTargetType, FallthroughChoiceType +from sentry.notifications.types import ActionTargetType, FallthroughChoiceType, NotificationRule from sentry.testutils.cases import TestCase from sentry.testutils.skips import requires_snuba @@ -38,19 +39,23 @@ def rule(self) -> Rule: @cached_property def record(self) -> Record: - return event_to_record(self.event, (self.rule,), self.notification_uuid) + return event_to_record(self.event, (self.notification_rule,), self.notification_uuid) + + @cached_property + def notification_rule(self) -> NotificationRule: + return NotificationRule.from_deprecated_legacy_rule(self.rule) @property def group_mapping(self) -> dict[int, Group]: return {self.event.group.id: self.event.group} @property - def rule_mapping(self) -> dict[int, Rule]: - return {self.rule.id: self.rule} + def rule_mapping(self) -> dict[int, NotificationRule]: + return {self.rule.id: self.notification_rule} def test_success(self) -> None: (record,) = _bind_records([self.record], self.group_mapping, self.rule_mapping) - assert record == self.record.with_rules([self.rule]) + assert record == self.record.with_rules([self.notification_rule]) def test_without_group(self) -> None: # If the record can't be associated with a group, it should be dropped @@ -70,8 +75,10 @@ def project(self) -> Project: return self.create_project(fire_project_created=True) @cached_property - def rule(self) -> Rule: - return self.create_project_rule(project=self.project) + def rule(self) -> NotificationRule: + return NotificationRule.from_deprecated_legacy_rule( + self.create_project_rule(project=self.project) + ) def test_success(self) -> None: events = [ @@ -80,15 +87,23 @@ def test_success(self) -> None: ] group = events[0].group assert group is not None + equivalent_rule = replace(self.rule) + assert equivalent_rule is not self.rule records = [ RecordWithRuleObjects( event.event_id, - NotificationWithRuleObjects(event, [self.rule], self.notification_uuid), + NotificationWithRuleObjects( + event, + [equivalent_rule if index == 1 else self.rule], + self.notification_uuid, + ), event.datetime.timestamp(), ) - for event in events + for index, event in enumerate(events) ] - ret = _group_records(records, {group.id: group}, {self.rule.id: self.rule}) + legacy_rule_id = self.rule.legacy_rule_id + assert legacy_rule_id is not None + ret = _group_records(records, {group.id: group}, {legacy_rule_id: self.rule}) assert ret == {self.rule: {group: records}} @@ -98,21 +113,25 @@ def project(self) -> Project: return self.create_project(fire_project_created=True) def test_success(self) -> None: - first_rule = self.create_project_rule( - project=self.project, - name="Send a notification for new issues", - condition_data=[ - {"id": "sentry.rules.conditions.first_seen_event.FirstSeenEventCondition"} - ], - action_data=[{"id": "sentry.rules.actions.notify_event.NotifyEventAction"}], + first_rule = NotificationRule.from_deprecated_legacy_rule( + self.create_project_rule( + project=self.project, + name="Send a notification for new issues", + condition_data=[ + {"id": "sentry.rules.conditions.first_seen_event.FirstSeenEventCondition"} + ], + action_data=[{"id": "sentry.rules.actions.notify_event.NotifyEventAction"}], + ) ) - second_rule = self.create_project_rule( - project=self.project, - name="Send a notification for regressions", - condition_data=[ - {"id": "sentry.rules.conditions.regression_event.RegressionEventCondition"} - ], - action_data=[{"id": "sentry.rules.actions.notify_event.NotifyEventAction"}], + second_rule = NotificationRule.from_deprecated_legacy_rule( + self.create_project_rule( + project=self.project, + name="Send a notification for regressions", + condition_data=[ + {"id": "sentry.rules.conditions.regression_event.RegressionEventCondition"} + ], + action_data=[{"id": "sentry.rules.actions.notify_event.NotifyEventAction"}], + ) ) rules = [first_rule, second_rule] diff --git a/tests/sentry/digests/test_utilities.py b/tests/sentry/digests/test_utilities.py index 8398948fb440..8a8b9a053554 100644 --- a/tests/sentry/digests/test_utilities.py +++ b/tests/sentry/digests/test_utilities.py @@ -20,7 +20,7 @@ from sentry.models.project import Project from sentry.models.projectownership import ProjectOwnership from sentry.models.rule import Rule as RuleModel -from sentry.notifications.types import ActionTargetType, FallthroughChoiceType +from sentry.notifications.types import ActionTargetType, FallthroughChoiceType, NotificationRule from sentry.notifications.utils.rules import split_rules_by_rule_workflow_id from sentry.services.eventstore.models import Event from sentry.testutils.cases import SnubaTestCase, TestCase @@ -30,7 +30,10 @@ def _get_records(project: Project, rules: Collection[RuleModel], event: Event) -> list[Record]: - rules_and_workflows = split_rules_by_rule_workflow_id(list(rules)) + notification_rules = [ + NotificationRule.from_deprecated_legacy_rule(rule, project=project) for rule in rules + ] + rules_and_workflows = split_rules_by_rule_workflow_id(notification_rules) rules_by_identifier_key = { IdentifierKey.RULE: rules_and_workflows.rules, IdentifierKey.WORKFLOW: rules_and_workflows.workflow_rules, @@ -74,7 +77,8 @@ def test_get_event_from_groups_in_digest(self) -> None: ), ] - records = [event_to_record(event, (rule,)) for event in events] + notification_rule = NotificationRule.from_deprecated_legacy_rule(rule, project=project) + records = [event_to_record(event, (notification_rule,)) for event in events] digest = build_digest(project, sort_records(records))[0] @@ -127,7 +131,8 @@ def test_get_rules_from_workflows_uses_workflow_environment_for_linked_rule(self rendered_rule = get_rules_from_workflows(project, {workflow_id})[workflow_id] - assert rendered_rule.id == rule.id + assert rendered_rule.legacy_rule_id == rule.id + assert rendered_rule.workflow_id == workflow.id assert rendered_rule.environment_id == production.id def test_get_rules_from_workflows_uses_workflow_environment_for_synthetic_rule(self) -> None: @@ -139,13 +144,14 @@ def test_get_rules_from_workflows_uses_workflow_environment_for_synthetic_rule(s rendered_rule = get_rules_from_workflows(project, {workflow.id})[workflow.id] - assert rendered_rule.id == workflow.id + assert rendered_rule.legacy_rule_id is None + assert rendered_rule.workflow_id == workflow.id assert rendered_rule.environment_id == environment.id def assert_rule_ids(digest: Digest, expected_rule_ids: list[int]) -> None: for rule, groups in digest.items(): - assert rule.id in expected_rule_ids + assert rule.legacy_rule_id in expected_rule_ids def assert_get_personalized_digests( @@ -246,6 +252,9 @@ def setUp(self) -> None: ) self.rule = self.create_project_rule() + self.notification_rule = NotificationRule.from_deprecated_legacy_rule( + self.rule, project=self.project + ) self.rule_with_workflow_id = self.create_project_rule(include_legacy_rule_id=False) self.rule_with_legacy_rule_id = self.create_project_rule(include_workflow_id=False) @@ -268,7 +277,7 @@ def create_events_from_filenames( def test_simple(self) -> None: records = [ - event_to_record(event, (self.rule,)) + event_to_record(event, (self.notification_rule,)) for event in self.team1_events + self.team2_events + self.user4_events ] digest = build_digest(self.project, sort_records(records))[0] @@ -317,7 +326,7 @@ def test_simple_with_legacy_rule_id(self) -> None: def test_direct_email(self) -> None: """When the action type is not Issue Owners, then the target actor gets a digest.""" self.project_ownership.update(fallthrough=False) - records = [event_to_record(event, (self.rule,)) for event in self.team1_events] + records = [event_to_record(event, (self.notification_rule,)) for event in self.team1_events] digest = build_digest(self.project, sort_records(records))[0] expected_result = {self.user1.id: set(self.team1_events)} @@ -443,7 +452,10 @@ def test_everyone_with_owners(self) -> None: events = self.create_events_from_filenames( self.project, ["hello.moz", "goodbye.moz", "hola.moz", "adios.moz"] ) - records = [event_to_record(event, (self.rule,)) for event in events + self.team1_events] + records = [ + event_to_record(event, (self.notification_rule,)) + for event in events + self.team1_events + ] digest = build_digest(self.project, sort_records(records))[0] expected_result = { self.user1.id: set(events), diff --git a/tests/sentry/integrations/github/test_ticket_action.py b/tests/sentry/integrations/github/test_ticket_action.py index 95db76d8dce5..9c9edb2a1c07 100644 --- a/tests/sentry/integrations/github/test_ticket_action.py +++ b/tests/sentry/integrations/github/test_ticket_action.py @@ -14,6 +14,7 @@ from sentry.issues.action_log.types import CreateExternalIssueAction from sentry.models.activity import Activity from sentry.models.repository import Repository +from sentry.notifications.types import NotificationRule, RuleFuture from sentry.services.eventstore.models import GroupEvent from sentry.silo.base import SiloMode from sentry.testutils.cases import RuleTestCase @@ -22,7 +23,6 @@ from sentry.testutils.silo import assume_test_silo_mode from sentry.testutils.skips import requires_snuba from sentry.types.activity import ActivityType -from sentry.types.rules import RuleFuture pytestmark = [requires_snuba] @@ -67,11 +67,20 @@ def stub_get_jwt(self): def trigger(self, event, rule_object): action = rule_object.data.get("actions", ())[0] - action_inst = self.get_rule(data=action, rule=rule_object) + notification_rule = NotificationRule( + action_id=rule_object.id, + label=rule_object.label, + data={"actions": [action]}, + project=rule_object.project, + environment_id=rule_object.environment_id, + workflow_id=123, + legacy_rule_id=rule_object.id, + ) + action_inst = self.get_rule(data=action, rule=notification_rule) results = list(action_inst.after(event=event)) assert len(results) == 1 - rule_future = RuleFuture(rule=rule_object, kwargs=results[0].kwargs) + rule_future = RuleFuture(rule=notification_rule, kwargs=results[0].kwargs) return results[0].callback(event, futures=[rule_future]) def get_key(self, event: GroupEvent): diff --git a/tests/sentry/integrations/github_enterprise/test_ticket_action.py b/tests/sentry/integrations/github_enterprise/test_ticket_action.py index 3ee2f5836c5e..299a24bfa633 100644 --- a/tests/sentry/integrations/github_enterprise/test_ticket_action.py +++ b/tests/sentry/integrations/github_enterprise/test_ticket_action.py @@ -14,6 +14,7 @@ from sentry.integrations.models.external_issue import ExternalIssue from sentry.models.activity import Activity from sentry.models.repository import Repository +from sentry.notifications.types import NotificationRule, RuleFuture from sentry.rules import rules from sentry.services.eventstore.models import GroupEvent from sentry.silo.base import SiloMode @@ -22,7 +23,6 @@ from sentry.testutils.silo import assume_test_silo_mode from sentry.testutils.skips import requires_snuba from sentry.types.activity import ActivityType -from sentry.types.rules import RuleFuture pytestmark = [requires_snuba] @@ -78,11 +78,20 @@ def stub_get_jwt(self): def trigger(self, event, rule_object): action = rule_object.data.get("actions", ())[0] - action_inst = self.get_rule(data=action, rule=rule_object) + notification_rule = NotificationRule( + action_id=rule_object.id, + label=rule_object.label, + data={"actions": [action]}, + project=rule_object.project, + environment_id=rule_object.environment_id, + workflow_id=123, + legacy_rule_id=rule_object.id, + ) + action_inst = self.get_rule(data=action, rule=notification_rule) results = list(action_inst.after(event=event)) assert len(results) == 1 - rule_future = RuleFuture(rule=rule_object, kwargs=results[0].kwargs) + rule_future = RuleFuture(rule=notification_rule, kwargs=results[0].kwargs) return results[0].callback(event, futures=[rule_future]) def get_key(self, event: GroupEvent): diff --git a/tests/sentry/integrations/jira/test_notify_action.py b/tests/sentry/integrations/jira/test_notify_action.py index 21b4636cd66a..6c476eee1ab7 100644 --- a/tests/sentry/integrations/jira/test_notify_action.py +++ b/tests/sentry/integrations/jira/test_notify_action.py @@ -4,12 +4,12 @@ from sentry.integrations.jira import JiraCreateTicketAction from sentry.integrations.models.external_issue import ExternalIssue from sentry.models.grouplink import GroupLink +from sentry.notifications.types import NotificationRule, RuleFuture from sentry.silo.base import SiloMode from sentry.testutils.cases import PerformanceIssueTestCase, RuleTestCase from sentry.testutils.helpers.notifications import TEST_ISSUE_OCCURRENCE from sentry.testutils.silo import assume_test_silo_mode from sentry.testutils.skips import requires_snuba -from sentry.types.rules import RuleFuture from sentry.utils import json pytestmark = [requires_snuba] @@ -84,7 +84,12 @@ def create_issue_base(self, event): assert len(results) == 1 # Trigger rule callback - rule_future = RuleFuture(rule=self.jira_rule, kwargs=results[0].kwargs) + persisted_rule = self.jira_rule.rule + assert persisted_rule is not None + rule_future = RuleFuture( + rule=NotificationRule.from_deprecated_legacy_rule(persisted_rule), + kwargs=results[0].kwargs, + ) results[0].callback(event, futures=[rule_future]) return json.loads(responses.calls[1].request.body) diff --git a/tests/sentry/integrations/jira/test_ticket_action.py b/tests/sentry/integrations/jira/test_ticket_action.py index 1643eda5fa2b..5e9d10a598ae 100644 --- a/tests/sentry/integrations/jira/test_ticket_action.py +++ b/tests/sentry/integrations/jira/test_ticket_action.py @@ -10,6 +10,7 @@ from sentry.integrations.models.external_issue import ExternalIssue from sentry.integrations.types import EventLifecycleOutcome from sentry.models.activity import Activity +from sentry.notifications.types import NotificationRule, RuleFuture from sentry.services.eventstore.models import GroupEvent from sentry.shared_integrations.exceptions import ( ApiInvalidRequestError, @@ -20,7 +21,6 @@ from sentry.testutils.cases import RuleTestCase from sentry.testutils.skips import requires_snuba from sentry.types.activity import ActivityType -from sentry.types.rules import RuleFuture from sentry.utils import json pytestmark = [requires_snuba] @@ -50,11 +50,20 @@ def setUp(self) -> None: def trigger(self, event, rule_object): action = rule_object.data.get("actions", ())[0] - action_inst = self.get_rule(data=action, rule=rule_object) + notification_rule = NotificationRule( + action_id=rule_object.id, + label=rule_object.label, + data={"actions": [action]}, + project=rule_object.project, + environment_id=rule_object.environment_id, + workflow_id=123, + legacy_rule_id=rule_object.id, + ) + action_inst = self.get_rule(data=action, rule=notification_rule) results = list(action_inst.after(event=event)) assert len(results) == 1 - rule_future = RuleFuture(rule=rule_object, kwargs=results[0].kwargs) + rule_future = RuleFuture(rule=notification_rule, kwargs=results[0].kwargs) return results[0].callback(event, futures=[rule_future]) def get_key(self, event: GroupEvent): diff --git a/tests/sentry/integrations/jira_server/test_ticket_action.py b/tests/sentry/integrations/jira_server/test_ticket_action.py index 022fe880efcb..85ca3aec6820 100644 --- a/tests/sentry/integrations/jira_server/test_ticket_action.py +++ b/tests/sentry/integrations/jira_server/test_ticket_action.py @@ -7,12 +7,12 @@ from sentry.integrations.jira_server import JiraServerCreateTicketAction, JiraServerIntegration from sentry.integrations.models.external_issue import ExternalIssue from sentry.models.rule import Rule +from sentry.notifications.types import NotificationRule, RuleFuture from sentry.services.eventstore.models import GroupEvent from sentry.silo.base import SiloMode from sentry.testutils.cases import RuleTestCase from sentry.testutils.silo import assume_test_silo_mode from sentry.testutils.skips import requires_snuba -from sentry.types.rules import RuleFuture from tests.sentry.integrations.jira_server import EXAMPLE_PRIVATE_KEY pytestmark = [requires_snuba] @@ -50,11 +50,20 @@ def setUp(self) -> None: def trigger(self, event: GroupEvent, rule_object: Rule) -> object: action = rule_object.data.get("actions", ())[0] - action_inst = self.get_rule(data=action, rule=rule_object) + notification_rule = NotificationRule( + action_id=rule_object.id, + label=rule_object.label, + data={"actions": [action]}, + project=rule_object.project, + environment_id=rule_object.environment_id, + workflow_id=123, + legacy_rule_id=rule_object.id, + ) + action_inst = self.get_rule(data=action, rule=notification_rule) results = list(action_inst.after(event=event)) assert len(results) == 1 - rule_future = RuleFuture(rule=rule_object, kwargs=results[0].kwargs) + rule_future = RuleFuture(rule=notification_rule, kwargs=results[0].kwargs) return results[0].callback(event, futures=[rule_future]) def get_key(self, event: GroupEvent) -> str: diff --git a/tests/sentry/integrations/msteams/test_message_builder.py b/tests/sentry/integrations/msteams/test_message_builder.py index 9f736e01d6ec..f16e34ec13fb 100644 --- a/tests/sentry/integrations/msteams/test_message_builder.py +++ b/tests/sentry/integrations/msteams/test_message_builder.py @@ -1,6 +1,7 @@ from __future__ import annotations import re +from dataclasses import replace from typing import TypeGuard import orjson @@ -49,6 +50,7 @@ from sentry.models.group import GroupStatus from sentry.models.groupassignee import GroupAssignee from sentry.models.organization import Organization +from sentry.notifications.types import NotificationRule from sentry.testutils.cases import TestCase from sentry.testutils.helpers.notifications import ( DummyNotification, @@ -117,10 +119,13 @@ def setUp(self) -> None: assert self.event1.group is not None self.group1 = self.event1.group - self.rules = [ + persisted_rules = [ self.create_project_rule(name="rule1"), self.create_project_rule(name="rule2"), ] + self.rules = [ + NotificationRule.from_deprecated_legacy_rule(rule) for rule in persisted_rules + ] def test_simple(self) -> None: card = MSTeamsMessageBuilder().build( @@ -422,18 +427,18 @@ def test_issue_message_builder(self) -> None: assert card_json[0] == "{" and card_json[-1] == "}" def test_issue_action_payload_includes_rule_and_workflow_ids(self) -> None: - self.rules[0].data["actions"][0].update( - {"legacy_rule_id": self.rules[0].id, "workflow_id": 123} - ) + legacy_rule_id = self.rules[0].legacy_rule_id + assert legacy_rule_id is not None + rule = replace(self.rules[0], legacy_rule_id=legacy_rule_id, workflow_id=123) payload = MSTeamsIssueMessageBuilder( group=self.group1, event=self.event1, - rules=[self.rules[0]], + rules=[rule], integration=self.integration, ).generate_action_payload(ACTION_TYPE.RESOLVE)["payload"] - assert payload["rules"] == [self.rules[0].id] + assert payload["rules"] == [legacy_rule_id] assert payload["workflows"] == [123] def test_issue_without_description(self) -> None: @@ -443,6 +448,42 @@ def test_issue_without_description(self) -> None: assert 3 == len(issue_card["body"]) + def test_action_payload_preserves_rule_id_contract(self) -> None: + legacy_rule = self.rules[0] + legacy_rule_id = legacy_rule.legacy_rule_id + assert legacy_rule_id is not None + rules = [ + NotificationRule( + action_id=legacy_rule.action_id, + label="Workflow with legacy rule", + data={"actions": [{"legacy_rule_id": legacy_rule_id}]}, + project=self.project1, + environment_id=None, + workflow_id=123, + legacy_rule_id=legacy_rule_id, + ), + NotificationRule( + action_id=None, + label="Workflow only", + data={"actions": [{"workflow_id": 123}]}, + project=self.project1, + environment_id=None, + workflow_id=123, + legacy_rule_id=None, + ), + ] + builder = MSTeamsIssueMessageBuilder( + group=self.group1, + event=self.event1, + rules=rules, + integration=self.integration, + ) + + payload = builder.generate_action_payload(ACTION_TYPE.RESOLVE) + + assert payload["payload"]["rules"] == [legacy_rule_id, 123] + assert payload["payload"]["workflows"] == [123] + def test_issue_with_only_one_rule(self) -> None: one_rule = self.rules[:1] issue_card = MSTeamsIssueMessageBuilder( diff --git a/tests/sentry/integrations/opsgenie/test_client.py b/tests/sentry/integrations/opsgenie/test_client.py index 761bf858d714..edf22f04e880 100644 --- a/tests/sentry/integrations/opsgenie/test_client.py +++ b/tests/sentry/integrations/opsgenie/test_client.py @@ -6,6 +6,7 @@ from sentry.integrations.opsgenie.client import OpsgenieClient from sentry.integrations.types import EventLifecycleOutcome +from sentry.notifications.types import NotificationRule from sentry.shared_integrations.exceptions import ApiError, ApiUnauthorized from sentry.testutils.asserts import ( assert_count_of_metric, @@ -82,7 +83,7 @@ def test_send_notification(self, mock_record: MagicMock) -> None: with self.options({"system.url-prefix": "http://example.com"}): payload = client.build_issue_alert_payload( data=event, - rules=[rule], + rules=[NotificationRule.from_deprecated_legacy_rule(rule)], event=event, group=group, priority="P2", @@ -149,7 +150,7 @@ def test_send_notification_with_workflow_engine_trigger_actions( with self.options({"system.url-prefix": "http://example.com"}): payload = client.build_issue_alert_payload( data=event, - rules=[rule], + rules=[NotificationRule.from_deprecated_legacy_rule(rule)], event=event, group=group, priority="P2", @@ -215,7 +216,7 @@ def test_send_notification_with_workflow_engine_ui_links(self, mock_record: Magi with self.options({"system.url-prefix": "http://example.com"}): payload = client.build_issue_alert_payload( data=event, - rules=[rule], + rules=[NotificationRule.from_deprecated_legacy_rule(rule)], event=event, group=group, priority="P2", @@ -282,7 +283,7 @@ def test_send_notification_unauthorized_errors(self, mock_record: MagicMock) -> with self.options({"system.url-prefix": "http://example.com"}): payload = client.build_issue_alert_payload( data=event, - rules=[rule], + rules=[NotificationRule.from_deprecated_legacy_rule(rule)], event=event, group=group, priority="P2", diff --git a/tests/sentry/integrations/pagerduty/test_notification.py b/tests/sentry/integrations/pagerduty/test_notification.py index dfe2a4c22809..180b01435e0a 100644 --- a/tests/sentry/integrations/pagerduty/test_notification.py +++ b/tests/sentry/integrations/pagerduty/test_notification.py @@ -11,6 +11,7 @@ from sentry.integrations.pagerduty.client import PAGERDUTY_SUMMARY_MAX_LENGTH from sentry.integrations.pagerduty.utils import add_service from sentry.integrations.types import EventLifecycleOutcome +from sentry.notifications.types import NotificationRule, RuleFuture from sentry.silo.base import SiloMode from sentry.testutils.asserts import assert_halt_metric, assert_slo_metric from sentry.testutils.cases import PerformanceIssueTestCase, RuleTestCase @@ -22,7 +23,6 @@ from sentry.testutils.helpers.notifications import TEST_ISSUE_OCCURRENCE from sentry.testutils.silo import assume_test_silo_mode from sentry.testutils.skips import requires_snuba -from sentry.types.rules import RuleFuture pytestmark = [requires_snuba] @@ -101,7 +101,10 @@ def test_applies_correctly(self, mock_record: MagicMock) -> None: ) # Trigger rule callback - rule_future = RuleFuture(rule=self.project_rule, kwargs=results[0].kwargs) + rule_future = RuleFuture( + rule=NotificationRule.from_deprecated_legacy_rule(self.project_rule), + kwargs=results[0].kwargs, + ) results[0].callback(event, futures=[rule_future]) data = orjson.loads(responses.calls[0].request.body) @@ -152,7 +155,10 @@ def test_applies_correctly_performance_issue(self) -> None: ) # Trigger rule callback - rule_future = RuleFuture(rule=self.project_rule, kwargs=results[0].kwargs) + rule_future = RuleFuture( + rule=NotificationRule.from_deprecated_legacy_rule(self.project_rule), + kwargs=results[0].kwargs, + ) results[0].callback(event, futures=[rule_future]) data = orjson.loads(responses.calls[0].request.body) @@ -188,7 +194,10 @@ def test_applies_correctly_generic_issue(self) -> None: ) # Trigger rule callback - rule_future = RuleFuture(rule=self.project_rule, kwargs=results[0].kwargs) + rule_future = RuleFuture( + rule=NotificationRule.from_deprecated_legacy_rule(self.project_rule), + kwargs=results[0].kwargs, + ) results[0].callback(group_event, futures=[rule_future]) data = orjson.loads(responses.calls[0].request.body) @@ -228,7 +237,10 @@ def test_truncates_summary(self) -> None: ) # Trigger rule callback - rule_future = RuleFuture(rule=self.project_rule, kwargs=results[0].kwargs) + rule_future = RuleFuture( + rule=NotificationRule.from_deprecated_legacy_rule(self.project_rule), + kwargs=results[0].kwargs, + ) results[0].callback(event, futures=[rule_future]) data = orjson.loads(responses.calls[0].request.body) diff --git a/tests/sentry/integrations/slack/actions/notification/test_slack_notify_service_action.py b/tests/sentry/integrations/slack/actions/notification/test_slack_notify_service_action.py index fed5f6d9e6d9..996e1be021b9 100644 --- a/tests/sentry/integrations/slack/actions/notification/test_slack_notify_service_action.py +++ b/tests/sentry/integrations/slack/actions/notification/test_slack_notify_service_action.py @@ -1,4 +1,5 @@ from copy import deepcopy +from dataclasses import replace from unittest.mock import MagicMock, patch import orjson @@ -7,13 +8,18 @@ from sentry.integrations.slack import SlackNotifyServiceAction from sentry.integrations.types import EventLifecycleOutcome +from sentry.models.rule import Rule from sentry.notifications.models.notificationmessage import NotificationMessage +from sentry.notifications.types import NotificationRule, RuleFuture from sentry.shared_integrations.exceptions import IntegrationError from sentry.silo.base import SiloMode from sentry.testutils.asserts import assert_failure_metric from sentry.testutils.cases import RuleTestCase from sentry.testutils.silo import assume_test_silo_mode -from sentry.types.rules import RuleFuture + + +def notification_rule_for_action(rule: Rule) -> NotificationRule: + return replace(NotificationRule.from_deprecated_legacy_rule(rule), action_id=rule.id) class TestInit(RuleTestCase): @@ -78,7 +84,10 @@ def test_after_noa( results = list(rule_cls_instance.after(event=self.event)) assert len(results) == 1 - results[0].callback(self.event, futures=[RuleFuture(rule=rule, kwargs={})]) + results[0].callback( + self.event, + futures=[RuleFuture(rule=notification_rule_for_action(rule), kwargs={})], + ) blocks = mock_post.call_args.kwargs["blocks"] blocks = orjson.loads(blocks) @@ -123,7 +132,10 @@ def test_after_noa_slo_halt(self, mock_post: MagicMock, mock_record: MagicMock) results = list(rule_cls_instance.after(event=self.event)) assert len(results) == 1 - results[0].callback(self.event, futures=[RuleFuture(rule=rule, kwargs={})]) + results[0].callback( + self.event, + futures=[RuleFuture(rule=notification_rule_for_action(rule), kwargs={})], + ) blocks = mock_post.call_args.kwargs["blocks"] blocks = orjson.loads(blocks) @@ -174,7 +186,10 @@ def test_after_noa_error( results = list(rule_cls_instance.after(event=self.event)) assert len(results) == 1 - results[0].callback(self.event, futures=[RuleFuture(rule=rule, kwargs={})]) + results[0].callback( + self.event, + futures=[RuleFuture(rule=notification_rule_for_action(rule), kwargs={})], + ) assert NotificationMessage.objects.all().count() == 1 @@ -210,7 +225,10 @@ def test_after_noa_test_action( results = list(rule_cls_instance.after(event=self.event)) assert len(results) == 1 - results[0].callback(self.event, futures=[RuleFuture(rule=rule, kwargs={})]) + results[0].callback( + self.event, + futures=[RuleFuture(rule=notification_rule_for_action(rule), kwargs={})], + ) blocks = mock_post.call_args.kwargs["blocks"] blocks = orjson.loads(blocks) @@ -246,12 +264,16 @@ def test_after_noa_new_ui( rule.id = self.action.id rule.environment_id = None - rule_cls_instance = self.get_rule(data=rule.data["actions"][0], rule=rule) + notification_rule = notification_rule_for_action(rule) + rule_cls_instance = self.get_rule(data=rule.data["actions"][0], rule=notification_rule) results = list(rule_cls_instance.after(event=self.event)) assert len(results) == 1 - results[0].callback(self.event, futures=[RuleFuture(rule=rule, kwargs={})]) + results[0].callback( + self.event, + futures=[RuleFuture(rule=notification_rule, kwargs={})], + ) blocks = mock_post.call_args.kwargs["blocks"] blocks = orjson.loads(blocks) @@ -292,7 +314,10 @@ def test_after_with_threads_noa( rule.id = self.action.id rule.environment_id = None - results[0].callback(self.event, futures=[RuleFuture(rule=rule, kwargs={})]) + results[0].callback( + self.event, + futures=[RuleFuture(rule=notification_rule_for_action(rule), kwargs={})], + ) blocks = mock_post.call_args.kwargs["blocks"] blocks = orjson.loads(blocks) @@ -349,7 +374,10 @@ def test_after_reply_in_thread_noa( rule.id = self.action.id rule.environment_id = None - results[0].callback(self.event, futures=[RuleFuture(rule=rule, kwargs={})]) + results[0].callback( + self.event, + futures=[RuleFuture(rule=notification_rule_for_action(rule), kwargs={})], + ) blocks = mock_post.call_args.kwargs["blocks"] blocks = orjson.loads(blocks) diff --git a/tests/sentry/integrations/slack/notifications/test_issue_alert.py b/tests/sentry/integrations/slack/notifications/test_issue_alert.py index e2371eb3e263..51523c6c4c5d 100644 --- a/tests/sentry/integrations/slack/notifications/test_issue_alert.py +++ b/tests/sentry/integrations/slack/notifications/test_issue_alert.py @@ -24,7 +24,12 @@ from sentry.notifications.models.notificationsettingoption import NotificationSettingOption from sentry.notifications.models.notificationsettingprovider import NotificationSettingProvider from sentry.notifications.notifications.rules import AlertRuleNotification -from sentry.notifications.types import ActionTargetType, FallthroughChoiceType, FineTuningAPIKey +from sentry.notifications.types import ( + ActionTargetType, + FallthroughChoiceType, + FineTuningAPIKey, + NotificationRule, +) from sentry.plugins.base import Notification from sentry.silo.base import SiloMode from sentry.tasks.digests import deliver_digest @@ -329,7 +334,15 @@ def test_issue_alert_issue_owners_block(self) -> None: == f"{event.project.slug} | " ) - def _assert_issue_owners_env_block(self, rule: Rule, environment: Environment) -> None: + def _assert_issue_owners_env_block( + self, rule: Rule | NotificationRule, environment: Environment + ) -> None: + workflow_id = ( + rule.workflow_id + if isinstance(rule, NotificationRule) + else rule.data["actions"][0]["workflow_id"] + ) + assert workflow_id is not None event = self.store_event( data={"message": "Hello world", "level": "error", "environment": environment.name}, project_id=self.project.id, @@ -350,13 +363,13 @@ def _assert_issue_owners_env_block(self, rule: Rule, environment: Environment) - notification_uuid = notification.notification_uuid assert ( fallback_text - == f"Alert triggered " + == f"Alert triggered " ) assert blocks[0]["text"]["text"] == fallback_text assert event.group assert ( blocks[1]["text"]["text"] - == f":red_circle: " + == f":red_circle: " ) assert ( blocks[4]["elements"][0]["text"] @@ -649,9 +662,12 @@ def test_issue_alert_team_issue_owners_user_settings_off_digests( name="ja rule", action_data=[action_data], ) + notification_rule = NotificationRule.from_deprecated_legacy_rule(rule) key = f"mail:p:{self.project.id}" - backend.add(key, event_to_record(event, [rule]), increment_delay=0, maximum_delay=0) + backend.add( + key, event_to_record(event, [notification_rule]), increment_delay=0, maximum_delay=0 + ) with self.tasks(): deliver_digest(key) @@ -951,13 +967,16 @@ def test_digest_enabled_block(self, digests: MagicMock) -> None: digests.enabled.return_value = True rule = self.create_project_rule(project=self.project) + notification_rule = NotificationRule.from_deprecated_legacy_rule(rule) ProjectOwnership.objects.create(project_id=self.project.id) event = self.store_event( data={"message": "Hello world", "level": "error"}, project_id=self.project.id ) key = f"mail:p:{self.project.id}:IssueOwners::AllMembers" - backend.add(key, event_to_record(event, [rule]), increment_delay=0, maximum_delay=0) + backend.add( + key, event_to_record(event, [notification_rule]), increment_delay=0, maximum_delay=0 + ) with self.tasks(): deliver_digest(key) diff --git a/tests/sentry/integrations/slack/test_message_builder.py b/tests/sentry/integrations/slack/test_message_builder.py index 00c80ca60577..be72b1c291fe 100644 --- a/tests/sentry/integrations/slack/test_message_builder.py +++ b/tests/sentry/integrations/slack/test_message_builder.py @@ -38,6 +38,7 @@ from sentry.models.rule import Rule as IssueAlertRule from sentry.models.team import Team from sentry.monitors.grouptype import MonitorIncidentType +from sentry.notifications.types import NotificationRule from sentry.notifications.utils.actions import MessageAction from sentry.services.eventstore.models import Event from sentry.silo.base import SiloMode @@ -388,7 +389,10 @@ def test_build_group_block_noa(self) -> None: more_tags = {"escape": "`room`", "foo": "bar", "release": release.version} notes = "hey @colleen fix it" - assert SlackIssuesMessageBuilder(group, rules=[rule]).build() == build_test_message_blocks( + notification_rule = NotificationRule.from_deprecated_legacy_rule(rule) + assert SlackIssuesMessageBuilder( + group, rules=[notification_rule] + ).build() == build_test_message_blocks( teams={self.team}, users={self.user}, group=group, @@ -461,8 +465,9 @@ def test_build_group_block_with_workflow_only(self) -> None: rule = self.create_project_rule(project=self.project) workflow_id = rule.data["actions"][0]["workflow_id"] rule.data["actions"][0].pop("legacy_rule_id") + notification_rule = NotificationRule.from_deprecated_legacy_rule(rule) - blocks = SlackIssuesMessageBuilder(self.group, rules=[rule]).build()["blocks"] + blocks = SlackIssuesMessageBuilder(self.group, rules=[notification_rule]).build()["blocks"] assert orjson.loads(blocks[0]["block_id"]) == { "issue": self.group.id, @@ -1317,7 +1322,7 @@ def test_get_context_users_affected(self) -> None: assert group group.update(type=1, substatus=GroupSubStatus.ONGOING, times_seen=3) - context = get_context(group, [rule]) + context = get_context(group, [NotificationRule.from_deprecated_legacy_rule(rule)]) assert ( context == f"Events: *3* Users Affected: *7* State: *Ongoing* First Seen: *{time_since(group.first_seen)}*" @@ -1325,7 +1330,7 @@ def test_get_context_users_affected(self) -> None: # filter users affected by env rule.update(environment_id=env.id) - context = get_context(group, [rule]) + context = get_context(group, [NotificationRule.from_deprecated_legacy_rule(rule)]) assert ( context == f"Events: *3* Users Affected: *5* State: *Ongoing* First Seen: *{time_since(group.first_seen)}*" diff --git a/tests/sentry/integrations/vsts/test_notify_action.py b/tests/sentry/integrations/vsts/test_notify_action.py index 4e72b40ef11e..9ec7c030404e 100644 --- a/tests/sentry/integrations/vsts/test_notify_action.py +++ b/tests/sentry/integrations/vsts/test_notify_action.py @@ -8,12 +8,12 @@ from sentry.integrations.vsts import AzureDevopsCreateTicketAction from sentry.integrations.vsts.integration import VstsIntegration from sentry.models.grouplink import GroupLink +from sentry.notifications.types import NotificationRule, RuleFuture from sentry.silo.base import SiloMode from sentry.testutils.cases import RuleTestCase from sentry.testutils.helpers.datetime import freeze_time from sentry.testutils.silo import assume_test_silo_mode from sentry.testutils.skips import requires_snuba -from sentry.types.rules import RuleFuture from .test_issues import VstsIssueBase @@ -71,7 +71,20 @@ def test_create_issue(self) -> None: assert len(results) == 1 # Trigger rule callback - rule_future = RuleFuture(rule=azuredevops_rule, kwargs=results[0].kwargs) + persisted_rule = azuredevops_rule.rule + assert persisted_rule is not None + rule_future = RuleFuture( + rule=NotificationRule( + action_id=persisted_rule.id, + label=persisted_rule.label, + data={"actions": [azuredevops_rule.data]}, + project=persisted_rule.project, + environment_id=persisted_rule.environment_id, + workflow_id=123, + legacy_rule_id=persisted_rule.id, + ), + kwargs=results[0].kwargs, + ) results[0].callback(event, futures=[rule_future]) data = orjson.loads(responses.calls[0].response.text) diff --git a/tests/sentry/mail/test_adapter.py b/tests/sentry/mail/test_adapter.py index f0d9e4129eb0..f84649e46811 100644 --- a/tests/sentry/mail/test_adapter.py +++ b/tests/sentry/mail/test_adapter.py @@ -44,6 +44,7 @@ from sentry.notifications.types import ( ActionTargetType, FallthroughChoiceType, + NotificationRule, RuleFuture, ) from sentry.notifications.utils.digest import get_digest_subject @@ -1398,9 +1399,14 @@ def test_notify_digest(self, notify: MagicMock) -> None: ) rule = self.create_project_rule(project=project) + notification_rule = NotificationRule.from_deprecated_legacy_rule(rule) ProjectOwnership.objects.create(project_id=self.project.id, fallthrough=True) digest = build_digest( - project, (event_to_record(event, (rule,)), event_to_record(event2, (rule,))) + project, + ( + event_to_record(event, (notification_rule,)), + event_to_record(event2, (notification_rule,)), + ), ) with self.tasks(): @@ -1454,9 +1460,14 @@ def test_notify_digest_replay_id(self, notify: MagicMock) -> None: ) rule = self.create_project_rule(project=project) + notification_rule = NotificationRule.from_deprecated_legacy_rule(rule) ProjectOwnership.objects.create(project_id=self.project.id, fallthrough=True) digest = build_digest( - project, (event_to_record(event, (rule,)), event_to_record(event2, (rule,))) + project, + ( + event_to_record(event, (notification_rule,)), + event_to_record(event2, (notification_rule,)), + ), ) features = ["organizations:session-replay"] @@ -1482,8 +1493,9 @@ def test_notify_digest_replay_id(self, notify: MagicMock) -> None: def test_notify_digest_single_record(self, send_async: MagicMock, notify: MagicMock) -> None: event = self.store_event(data={}, project_id=self.project.id) rule = self.create_project_rule(project=self.project) + notification_rule = NotificationRule.from_deprecated_legacy_rule(rule) ProjectOwnership.objects.create(project_id=self.project.id, fallthrough=True) - digest = build_digest(self.project, (event_to_record(event, (rule,)),)) + digest = build_digest(self.project, (event_to_record(event, (notification_rule,)),)) self.adapter.notify_digest( self.project, digest, @@ -1509,9 +1521,14 @@ def test_notify_digest_subject_prefix(self) -> None: ) rule = self.create_project_rule(project=self.project) + notification_rule = NotificationRule.from_deprecated_legacy_rule(rule) digest = build_digest( - self.project, (event_to_record(event, (rule,)), event_to_record(event2, (rule,))) + self.project, + ( + event_to_record(event, (notification_rule,)), + event_to_record(event2, (notification_rule,)), + ), ) with self.tasks(): @@ -1550,9 +1567,14 @@ def test_notify_digest_user_does_not_exist(self, notify: MagicMock) -> None: "targetIdentifier": str(444), } rule = self.create_project_rule(name="a rule", action_data=[action_data]) + notification_rule = NotificationRule.from_deprecated_legacy_rule(rule) digest = build_digest( - project, (event_to_record(event, (rule,)), event_to_record(event2, (rule,))) + project, + ( + event_to_record(event, (notification_rule,)), + event_to_record(event2, (notification_rule,)), + ), ) with self.tasks(): @@ -1567,7 +1589,8 @@ class MailAdapterRuleNotifyTest(BaseMailAdapterTest): def test_normal(self, mock_logger: MagicMock) -> None: event = self.store_event(data={}, project_id=self.project.id) rule = self.create_project_rule(name="my rule") - futures = [RuleFuture(rule, {})] + notification_rule = NotificationRule.from_deprecated_legacy_rule(rule) + futures = [RuleFuture(notification_rule, {})] with mock.patch.object(self.adapter, "notify") as notify: self.adapter.rule_notify(event, futures, ActionTargetType.ISSUE_OWNERS) assert notify.call_count == 1 @@ -1583,7 +1606,7 @@ def test_normal(self, mock_logger: MagicMock) -> None: "target_identifier": None, "fallthrough_choice": None, "notification_uuid": mock.ANY, - "rule_id": rule.id, + "rule_id": notification_rule.broken_rule_id, "project_id": event.group.project.id, }, ) @@ -1596,7 +1619,8 @@ def test_digest(self, mock_logger: MagicMock, digests: MagicMock) -> None: event = self.store_event(data={}, project_id=self.project.id) rule = self.create_project_rule(project=self.project) - futures = [RuleFuture(rule, {})] + notification_rule = NotificationRule.from_deprecated_legacy_rule(rule) + futures = [RuleFuture(notification_rule, {})] self.adapter.rule_notify(event, futures, ActionTargetType.ISSUE_OWNERS) assert digests.backend.add.call_count == 1 assert event.group @@ -1611,7 +1635,7 @@ def test_digest(self, mock_logger: MagicMock, digests: MagicMock) -> None: "target_identifier": None, "fallthrough_choice": None, "notification_uuid": mock.ANY, - "rule_id": rule.id, + "rule_id": notification_rule.broken_rule_id, "project_id": event.group.project.id, "digest_key": mock.ANY, }, @@ -1623,14 +1647,14 @@ def test_digest_with_perf_issue(self, digests: MagicMock) -> None: event = self.create_performance_issue() rule = self.create_project_rule(project=self.project) - futures = [RuleFuture(rule, {})] + futures = [RuleFuture(NotificationRule.from_deprecated_legacy_rule(rule), {})] self.adapter.rule_notify(event, futures, ActionTargetType.ISSUE_OWNERS) assert digests.backend.add.call_count == 1 def test_notify_includes_uuid(self) -> None: event = self.store_event(data={}, project_id=self.project.id) rule = self.create_project_rule(name="my rule") - futures = [RuleFuture(rule, {})] + futures = [RuleFuture(NotificationRule.from_deprecated_legacy_rule(rule), {})] notification_uuid = str(uuid.uuid4()) with mock.patch.object(self.adapter, "notify") as notify: self.adapter.rule_notify( diff --git a/tests/sentry/notifications/notification_action/test_issue_alert_registry_handlers.py b/tests/sentry/notifications/notification_action/test_issue_alert_registry_handlers.py index 91cb71725df1..bce1edcd76f2 100644 --- a/tests/sentry/notifications/notification_action/test_issue_alert_registry_handlers.py +++ b/tests/sentry/notifications/notification_action/test_issue_alert_registry_handlers.py @@ -3,8 +3,7 @@ import pytest -from sentry.constants import ObjectStatus -from sentry.models.rule import Rule, RuleSource +from sentry.models.rule import Rule from sentry.notifications.models.notificationaction import ActionTarget from sentry.notifications.notification_action.issue_alert_registry import ( AzureDevopsIssueAlertHandler, @@ -25,7 +24,13 @@ BaseIssueAlertHandler, TicketingIssueAlertHandler, ) -from sentry.notifications.types import TEST_NOTIFICATION_ID, ActionTargetType, FallthroughChoiceType +from sentry.notifications.types import ( + TEST_NOTIFICATION_ID, + ActionTargetType, + FallthroughChoiceType, + NotificationRule, + NotificationRuleData, +) from sentry.testutils.helpers.data_blobs import ( AZURE_DEVOPS_ACTION_DATA_BLOBS, EMAIL_ACTION_DATA_BLOBS, @@ -112,18 +117,23 @@ def get_additional_fields(cls, action: Action, mapping: ActionFieldMapping): ) def test_create_rule_instance_from_action(self) -> None: - """Test that create_rule_instance_from_action creates a Rule with correct attributes""" + """Test that create_rule_instance_from_action creates a notification rule.""" rule = self.handler.create_rule_instance_from_action( self.action, self.detector, self.event_data, workflow_id=self.workflow.id ) - assert isinstance(rule, Rule) - assert rule.id == self.action.id + assert isinstance(rule, NotificationRule) + assert rule.action_id == self.action.id assert rule.project == self.detector.project assert rule.environment_id is not None assert self.workflow.environment is not None assert rule.environment_id == self.workflow.environment.id assert rule.label == self.workflow.name + assert rule.workflow_id == self.workflow.id + assert rule.legacy_rule_id == self.rule.id + assert rule.is_workflow_with_legacy_rule + assert not rule.is_workflow_only + assert not rule.is_test_notification assert rule.data == { "actions": [ { @@ -136,23 +146,45 @@ def test_create_rule_instance_from_action(self) -> None: } ], } - assert rule.status == ObjectStatus.ACTIVE - assert rule.source == RuleSource.ISSUE + assert rule.project_id == self.detector.project.id + + other_rule = self.handler.create_rule_instance_from_action( + self.action, self.detector, self.event_data, workflow_id=self.workflow.id + ) + assert rule.identifier == f"workflow:{self.workflow.id}" + assert rule == other_rule + assert hash(rule) == hash(other_rule) + assert {rule: "first", other_rule: "second"} == {rule: "second"} + + same_numeric_legacy_rule = NotificationRule( + action_id=None, + label="Legacy rule", + data={"actions": [{"id": "test-action"}]}, + project=self.project, + environment_id=None, + workflow_id=None, + legacy_rule_id=self.workflow.id, + ) + assert same_numeric_legacy_rule.identifier == f"legacy:{self.workflow.id}" + assert same_numeric_legacy_rule != rule def test_create_rule_instance_from_action_with_workflow_only(self) -> None: - """Test that create_rule_instance_from_action creates a Rule with correct attributes""" + """Test that create_rule_instance_from_action creates a notification rule.""" self.rule.delete() rule = self.handler.create_rule_instance_from_action( self.action, self.detector, self.event_data, workflow_id=self.workflow.id ) - assert isinstance(rule, Rule) - assert rule.id == self.action.id + assert isinstance(rule, NotificationRule) + assert rule.action_id == self.action.id assert rule.project == self.detector.project assert rule.environment_id is not None assert self.workflow.environment is not None assert rule.environment_id == self.workflow.environment.id assert rule.label == self.workflow.name + assert rule.workflow_id == self.workflow.id + assert rule.legacy_rule_id is None + assert rule.is_workflow_only assert rule.data == { "actions": [ { @@ -164,8 +196,6 @@ def test_create_rule_instance_from_action_with_workflow_only(self) -> None: } ] } - assert rule.status == ObjectStatus.ACTIVE - assert rule.source == RuleSource.ISSUE def test_create_rule_instance_from_action_deleted_workflow_falls_back_to_detector_name( self, @@ -177,8 +207,11 @@ def test_create_rule_instance_from_action_deleted_workflow_falls_back_to_detecto self.action, self.detector, self.event_data, workflow_id=workflow_id ) - assert isinstance(rule, Rule) + assert isinstance(rule, NotificationRule) assert rule.label == self.detector.name + assert rule.workflow_id == workflow_id + assert rule.legacy_rule_id is None + assert rule.is_workflow_only assert rule.data == { "actions": [ { @@ -198,7 +231,7 @@ def test_rule_instance_from_action_uses_workflow_name_not_stale_rule_label( rule = self.handler.create_rule_instance_from_action( self.action, self.detector, self.event_data, workflow_id=self.workflow.id ) - assert isinstance(rule, Rule) + assert isinstance(rule, NotificationRule) assert rule.label == "Renamed Alert Name" assert rule.label != self.rule.label # legacy rule label is still "Test Alert" @@ -208,8 +241,11 @@ def test_create_rule_instance_from_action_with_test_notification_id(self) -> Non self.action, self.detector, self.event_data, workflow_id=TEST_NOTIFICATION_ID ) - assert isinstance(rule, Rule) + assert isinstance(rule, NotificationRule) assert rule.label == self.detector.name + assert rule.workflow_id is None + assert rule.legacy_rule_id == TEST_NOTIFICATION_ID + assert rule.is_test_notification assert rule.data == { "actions": [ { @@ -222,16 +258,120 @@ def test_create_rule_instance_from_action_with_test_notification_id(self) -> Non ], } + def test_notification_rule_rejects_invalid_identity(self) -> None: + data: NotificationRuleData = {"actions": [{"id": "test-action"}]} + + with pytest.raises(ValueError, match="requires at least one action"): + NotificationRule( + action_id=self.action.id, + label="Invalid", + data={"actions": []}, + project=self.project, + environment_id=None, + workflow_id=self.workflow.id, + legacy_rule_id=None, + ) + + with pytest.raises(ValueError, match="requires a workflow or legacy rule ID"): + NotificationRule( + action_id=self.action.id, + label="Invalid", + data=data, + project=self.project, + environment_id=None, + workflow_id=None, + legacy_rule_id=None, + ) + + with pytest.raises(ValueError, match="cannot have a workflow ID"): + NotificationRule( + action_id=self.action.id, + label="Invalid", + data=data, + project=self.project, + environment_id=None, + workflow_id=self.workflow.id, + legacy_rule_id=TEST_NOTIFICATION_ID, + ) + + def test_from_deprecated_legacy_rule(self) -> None: + legacy_rule = self.create_project_rule(project=self.project, include_workflow_id=False) + notification_rule = NotificationRule.from_deprecated_legacy_rule(legacy_rule) + + assert notification_rule.action_id is None + assert notification_rule.legacy_rule_id == legacy_rule.id + assert notification_rule.legacy_rule_id == legacy_rule.id + assert notification_rule.workflow_id is None + assert notification_rule.is_legacy_rule_only + assert notification_rule.data == {"actions": legacy_rule.data["actions"]} + + def test_from_deprecated_legacy_rule_normalizes_string_identity(self) -> None: + legacy_rule = self.create_project_rule(project=self.project) + legacy_rule.data["actions"][0]["legacy_rule_id"] = str(legacy_rule.id) + legacy_rule.data["actions"][0]["workflow_id"] = str(self.workflow.id) + + notification_rule = NotificationRule.from_deprecated_legacy_rule(legacy_rule) + + assert notification_rule.legacy_rule_id == legacy_rule.id + assert notification_rule.workflow_id == self.workflow.id + + def test_explicit_workflow_uses_authoritative_legacy_rule_id(self) -> None: + legacy_rule = self.create_project_rule(project=self.project) + legacy_rule.data["actions"][0]["legacy_rule_id"] = legacy_rule.id + 1 + + notification_rule = NotificationRule.from_deprecated_legacy_rule( + legacy_rule, workflow_id=self.workflow.id + ) + + assert notification_rule.legacy_rule_id == legacy_rule.id + assert notification_rule.workflow_id == self.workflow.id + + def test_test_notification_identity_uses_action_id(self) -> None: + data: NotificationRuleData = {"actions": [{"id": "test-action"}]} + first = NotificationRule( + action_id=1, + label="First", + data=data, + project=self.project, + environment_id=None, + workflow_id=None, + legacy_rule_id=TEST_NOTIFICATION_ID, + ) + second = NotificationRule( + action_id=2, + label="Second", + data=data, + project=self.project, + environment_id=None, + workflow_id=None, + legacy_rule_id=TEST_NOTIFICATION_ID, + ) + + assert first != second + + def test_from_deprecated_legacy_rule_without_actions(self) -> None: + legacy_rule = self.create_project_rule(project=self.project) + legacy_rule.update(data={}) + legacy_rule = Rule.objects.get(id=legacy_rule.id) + + with self.assertNumQueries(0): + notification_rule = NotificationRule.from_deprecated_legacy_rule( + legacy_rule, project=self.project + ) + + assert notification_rule.is_legacy_rule_only + assert notification_rule.data == {"actions": [{}]} + def test_create_rule_instance_from_action_no_environment(self) -> None: - """Test that create_rule_instance_from_action creates a Rule with correct attributes""" + """Test that create_rule_instance_from_action creates a notification rule.""" self.create_workflow() job = WorkflowEventData(event=self.group_event, workflow_env=None, group=self.group) rule = self.handler.create_rule_instance_from_action( self.action, self.detector, job, workflow_id=self.workflow.id ) - assert isinstance(rule, Rule) - assert rule.id == self.action.id + assert isinstance(rule, NotificationRule) + assert rule.action_id == self.action.id assert rule.project == self.detector.project assert rule.environment_id is None assert rule.label == self.workflow.name @@ -247,8 +387,6 @@ def test_create_rule_instance_from_action_no_environment(self) -> None: } ], } - assert rule.status == ObjectStatus.ACTIVE - assert rule.source == RuleSource.ISSUE @mock.patch("sentry.notifications.notification_action.types.invoke_future_with_error_handling") @mock.patch("sentry.notifications.notification_action.types.activate_downstream_actions") diff --git a/tests/sentry/notifications/notifications/test_digests.py b/tests/sentry/notifications/notifications/test_digests.py index e50538402a6a..303d2d56b46e 100644 --- a/tests/sentry/notifications/notifications/test_digests.py +++ b/tests/sentry/notifications/notifications/test_digests.py @@ -14,6 +14,7 @@ from sentry.digests.notifications import event_to_record from sentry.mail.analytics import EmailNotificationSent from sentry.models.projectownership import ProjectOwnership +from sentry.notifications.types import NotificationRule from sentry.services.eventstore.models import Event, GroupEvent from sentry.tasks.digests import deliver_digest from sentry.testutils.cases import PerformanceIssueTestCase, SlackActivityNotificationTest, TestCase @@ -61,7 +62,10 @@ def add_event(self, fingerprint: str, backend: Backend, event_type: str = "error assert event is not None backend.add( - self.key, event_to_record(event, [self.rule]), increment_delay=0, maximum_delay=0 + self.key, + event_to_record(event, [self.notification_rule]), + increment_delay=0, + maximum_delay=0, ) def run_test( @@ -91,6 +95,7 @@ def run_test( def setUp(self) -> None: super().setUp() self.rule = self.create_project_rule(project=self.project) + self.notification_rule = NotificationRule.from_deprecated_legacy_rule(self.rule) self.key = f"mail:p:{self.project.id}:IssueOwners::AllMembers" ProjectOwnership.objects.create(project_id=self.project.id, fallthrough=True) for i in range(USER_COUNT - 1): @@ -269,6 +274,7 @@ def test_slack_digest_notification_block(self, digests: MagicMock) -> None: timestamp = timestamp_raw.isoformat() key = f"slack:p:{self.project.id}:IssueOwners::AllMembers" rule = self.create_project_rule(project=self.project) + notification_rule = NotificationRule.from_deprecated_legacy_rule(rule) event1 = self.store_event( data={ "timestamp": timestamp, @@ -291,13 +297,13 @@ def test_slack_digest_notification_block(self, digests: MagicMock) -> None: notification_uuid = str(uuid.uuid4()) backend.add( key, - event_to_record(event1, [rule], notification_uuid), + event_to_record(event1, [notification_rule], notification_uuid), increment_delay=0, maximum_delay=0, ) backend.add( key, - event_to_record(event2, [rule], notification_uuid), + event_to_record(event2, [notification_rule], notification_uuid), increment_delay=0, maximum_delay=0, ) @@ -350,6 +356,7 @@ def test_slack_digest_notification_truncates_at_48_blocks(self, digests: MagicMo timestamp = before_now(days=1).isoformat() key = f"slack:p:{self.project.id}:IssueOwners::AllMembers" rule = self.create_project_rule(project=self.project) + notification_rule = NotificationRule.from_deprecated_legacy_rule(rule) notification_uuid = str(uuid.uuid4()) # Create 17 events to exceed 48 blocks (each event generates ~3 blocks) @@ -365,7 +372,7 @@ def test_slack_digest_notification_truncates_at_48_blocks(self, digests: MagicMo ) backend.add( key, - event_to_record(event, [rule], notification_uuid), + event_to_record(event, [notification_rule], notification_uuid), increment_delay=0, maximum_delay=0, ) diff --git a/tests/sentry/notifications/platform/discord/renderers/test_issue.py b/tests/sentry/notifications/platform/discord/renderers/test_issue.py index 3fea480a9d8d..850c8b9c53bc 100644 --- a/tests/sentry/notifications/platform/discord/renderers/test_issue.py +++ b/tests/sentry/notifications/platform/discord/renderers/test_issue.py @@ -49,6 +49,8 @@ def _create_data( "actions": [{"workflow_id": 1}], }, project_id=self.project.id, + workflow_id=1, + legacy_rule_id=None, ), ) @@ -119,7 +121,12 @@ def test_source(self) -> None: data = IssueNotificationData( group_id=self.group.id, rule=SerializableRuleProxy( - id=1, label="Test Detector", data={}, project_id=self.project.id + id=1, + label="Test Detector", + data={"actions": [{"workflow_id": 1}]}, + project_id=self.project.id, + workflow_id=1, + legacy_rule_id=None, ), ) assert data.source == NotificationSource.ISSUE @@ -130,7 +137,12 @@ def test_provider_returns_issue_renderer(self) -> None: data = IssueNotificationData( group_id=self.group.id, rule=SerializableRuleProxy( - id=1, label="Test Detector", data={}, project_id=self.project.id + id=1, + label="Test Detector", + data={"actions": [{"workflow_id": 1}]}, + project_id=self.project.id, + workflow_id=1, + legacy_rule_id=None, ), ) renderer = DiscordNotificationProvider.get_renderer(data=data) diff --git a/tests/sentry/notifications/platform/msteams/renderers/test_issue.py b/tests/sentry/notifications/platform/msteams/renderers/test_issue.py index 79ab27fc6926..9baa2218e7b0 100644 --- a/tests/sentry/notifications/platform/msteams/renderers/test_issue.py +++ b/tests/sentry/notifications/platform/msteams/renderers/test_issue.py @@ -83,6 +83,8 @@ def _create_data( "actions": [{"workflow_id": 1}], }, project_id=self.project.id, + workflow_id=1, + legacy_rule_id=None, ), ) @@ -115,7 +117,9 @@ def _build_expected_card( label="Test Detector", data={"actions": [{"workflow_id": 1}]}, project_id=self.project.id, - ).to_rule() + workflow_id=1, + legacy_rule_id=None, + ).to_notification_rule(project) ] footer_text = build_footer( group=group, project=project, url_format=MSTEAMS_URL_FORMAT, rules=rules @@ -420,7 +424,12 @@ def test_render_group_not_found(self) -> None: group_id=999999999, notification_uuid="test-uuid", rule=SerializableRuleProxy( - id=1, label="Test Detector", data={}, project_id=self.project.id + id=1, + label="Test Detector", + data={"actions": [{"workflow_id": 1}]}, + project_id=self.project.id, + workflow_id=1, + legacy_rule_id=None, ), ) rendered_template = NotificationRenderedTemplate(subject="Issue Alert", body=[]) @@ -435,7 +444,12 @@ def test_source(self) -> None: data = IssueNotificationData( group_id=self.group.id, rule=SerializableRuleProxy( - id=1, label="Test Detector", data={}, project_id=self.project.id + id=1, + label="Test Detector", + data={"actions": [{"workflow_id": 1}]}, + project_id=self.project.id, + workflow_id=1, + legacy_rule_id=None, ), ) assert data.source == NotificationSource.ISSUE @@ -446,7 +460,12 @@ def test_provider_returns_issue_renderer(self) -> None: data = IssueNotificationData( group_id=self.group.id, rule=SerializableRuleProxy( - id=1, label="Test Detector", data={}, project_id=self.project.id + id=1, + label="Test Detector", + data={"actions": [{"workflow_id": 1}]}, + project_id=self.project.id, + workflow_id=1, + legacy_rule_id=None, ), ) renderer = MSTeamsNotificationProvider.get_renderer(data=data) diff --git a/tests/sentry/notifications/platform/msteams/renderers/test_issue_parity.py b/tests/sentry/notifications/platform/msteams/renderers/test_issue_parity.py index abb0e75bbf87..1e1c2069e358 100644 --- a/tests/sentry/notifications/platform/msteams/renderers/test_issue_parity.py +++ b/tests/sentry/notifications/platform/msteams/renderers/test_issue_parity.py @@ -13,6 +13,7 @@ SerializableRuleProxy, ) from sentry.notifications.platform.types import NotificationRenderedTemplate +from sentry.notifications.types import NotificationRule from sentry.testutils.cases import TestCase @@ -56,7 +57,10 @@ def legacy_card(self) -> AdaptiveCard: rpc_integration = integration_service.get_integration(integration_id=self.integration.id) assert rpc_integration is not None return MSTeamsIssueMessageBuilder( - self.issue_group, self.event, [self.rule], rpc_integration + self.issue_group, + self.event, + [NotificationRule.from_deprecated_legacy_rule(self.rule)], + rpc_integration, ).build_group_card() def platform_card(self) -> AdaptiveCard: @@ -64,7 +68,9 @@ def platform_card(self) -> AdaptiveCard: group_id=self.issue_group.id, event_id=self.event.event_id, notification_uuid="", - rule=SerializableRuleProxy.from_rule(self.rule), + rule=SerializableRuleProxy.from_rule( + NotificationRule.from_deprecated_legacy_rule(self.rule) + ), ) return IssueMSTeamsRenderer.render( data=data, diff --git a/tests/sentry/notifications/platform/slack/renderers/test_issue.py b/tests/sentry/notifications/platform/slack/renderers/test_issue.py index be01e9e4fa55..84c60af5b2a4 100644 --- a/tests/sentry/notifications/platform/slack/renderers/test_issue.py +++ b/tests/sentry/notifications/platform/slack/renderers/test_issue.py @@ -75,11 +75,80 @@ def _create_invocation( class IssueNotificationDataTest(IssueAlertInvocationMixin): + def test_deserializes_legacy_rule_proxy(self) -> None: + proxy = SerializableRuleProxy.parse_obj( + { + "id": 1, + "label": "Legacy payload", + "data": {"actions": [{"workflow_id": 2}]}, + "project_id": self.project.id, + } + ) + + rule = proxy.to_notification_rule(self.project) + + assert rule.action_id == 1 + assert rule.workflow_id == 2 + assert rule.legacy_rule_id is None + + def test_deserializes_legacy_rule_proxy_without_action_data(self) -> None: + proxy = SerializableRuleProxy.parse_obj( + { + "id": 1, + "label": "Legacy payload", + "data": {}, + "project_id": self.project.id, + } + ) + + rule = proxy.to_notification_rule(self.project) + + assert rule.action_id == 1 + assert rule.workflow_id is None + assert rule.legacy_rule_id == 1 + assert rule.data == {"actions": [{}]} + + def test_deserializes_string_identity(self) -> None: + proxy = SerializableRuleProxy.parse_obj( + { + "id": 1, + "label": "Legacy payload", + "data": {"actions": [{"workflow_id": "2"}]}, + "project_id": self.project.id, + } + ) + + rule = proxy.to_notification_rule(self.project) + + assert rule.workflow_id == 2 + assert rule.legacy_rule_id is None + + def test_deserializes_workflow_rule_without_action(self) -> None: + proxy = SerializableRuleProxy( + id=2, + action_id=None, + label="Workflow payload", + data={"actions": [{"workflow_id": 2}]}, + project_id=self.project.id, + workflow_id=2, + legacy_rule_id=None, + ) + + rule = proxy.to_notification_rule(self.project) + + assert rule.action_id is None + assert rule.workflow_id == 2 + def test_source(self) -> None: data = IssueNotificationData( group_id=self.group.id, rule=SerializableRuleProxy( - id=1, label="Test Detector", data={}, project_id=self.project.id + id=1, + label="Test Detector", + data={"actions": [{"workflow_id": 1}]}, + project_id=self.project.id, + workflow_id=1, + legacy_rule_id=None, ), ) assert data.source == NotificationSource.ISSUE @@ -97,8 +166,10 @@ def test_from_action_invocation(self) -> None: assert result.event_id == invocation.event_data.event.event_id assert result.notification_uuid == "test-uuid-123" assert isinstance(result.rule, SerializableRuleProxy) - assert result.rule.id == invocation.action.id + assert result.rule.action_id == invocation.action.id assert result.rule.label == "Test Workflow" + assert result.rule.workflow_id == invocation.workflow_id + assert result.rule.legacy_rule_id is None assert result.tags == ["environment", "level"] assert result.notes == "test note" assert len(result.rule.data["actions"]) == 1 @@ -337,7 +408,12 @@ def test_provider_returns_issue_renderer(self) -> None: data = IssueNotificationData( group_id=self.group.id, rule=SerializableRuleProxy( - id=1, label="Test Detector", data={}, project_id=self.project.id + id=1, + label="Test Detector", + data={"actions": [{"workflow_id": 1}]}, + project_id=self.project.id, + workflow_id=1, + legacy_rule_id=None, ), ) renderer = SlackNotificationProvider.get_renderer(data=data) diff --git a/tests/sentry/notifications/test_helpers.py b/tests/sentry/notifications/test_helpers.py index 4974531e1b2d..0f2fe6fc5de4 100644 --- a/tests/sentry/notifications/test_helpers.py +++ b/tests/sentry/notifications/test_helpers.py @@ -7,7 +7,11 @@ validate, ) from sentry.notifications.models.notificationsettingoption import NotificationSettingOption -from sentry.notifications.types import NotificationSettingEnum, NotificationSettingsOptionEnum +from sentry.notifications.types import ( + NotificationRule, + NotificationSettingEnum, + NotificationSettingsOptionEnum, +) from sentry.notifications.utils.links import ( get_email_link_extra_params, get_group_settings_link, @@ -87,7 +91,10 @@ def test_collect_groups_by_project(self) -> None: def test_get_group_settings_link(self) -> None: rule: Rule = self.create_project_rule(self.project) - rule_details = get_rules([rule], self.organization, self.project, self.group.type) + notification_rule = NotificationRule.from_deprecated_legacy_rule(rule) + rule_details = get_rules( + [notification_rule], self.organization, self.project, self.group.type + ) link = get_group_settings_link( self.group, self.environment.name, rule_details, 1337, extra="123" ) @@ -109,7 +116,13 @@ def test_get_email_link_extra_params(self) -> None: project2 = self.create_project() rule2 = self.create_project_rule(project2) - rule_details = get_rules([rule, rule2], self.organization, self.project, self.group.type) + notification_rules = [ + NotificationRule.from_deprecated_legacy_rule(rule), + NotificationRule.from_deprecated_legacy_rule(rule2), + ] + rule_details = get_rules( + notification_rules, self.organization, self.project, self.group.type + ) extra_params = { k: dict(map(lambda x: (x[0], x[1][0]), parse_qs(v.strip("?")).items())) for k, v in get_email_link_extra_params( diff --git a/tests/sentry/notifications/utils/test_rules.py b/tests/sentry/notifications/utils/test_rules.py index f85f26d03136..397fdab3b844 100644 --- a/tests/sentry/notifications/utils/test_rules.py +++ b/tests/sentry/notifications/utils/test_rules.py @@ -1,28 +1,38 @@ -from sentry.models.rule import Rule +from sentry.models.project import Project +from sentry.notifications.types import NotificationRule from sentry.notifications.utils.rules import get_rule_or_workflow_id -def _rule(action: dict[str, str]) -> Rule: - return Rule(id=99, data={"actions": [action]}) +def _rule(*, legacy_rule_id: int | None, workflow_id: int | None) -> NotificationRule: + return NotificationRule( + action_id=None, + label="Test rule", + data={"actions": [{}]}, + project=Project(id=1), + environment_id=None, + workflow_id=workflow_id, + legacy_rule_id=legacy_rule_id, + ) def test_get_rule_or_workflow_id_prefers_legacy_rule_id_by_default() -> None: - rule = _rule({"legacy_rule_id": "1", "workflow_id": "2"}) + rule = _rule(legacy_rule_id=1, workflow_id=2) assert get_rule_or_workflow_id(rule) == ("legacy_rule_id", "1") def test_get_rule_or_workflow_id_prefer_workflow() -> None: - rule = _rule({"legacy_rule_id": "1", "workflow_id": "2"}) + rule = _rule(legacy_rule_id=1, workflow_id=2) assert get_rule_or_workflow_id(rule, prefer="workflow_id") == ("workflow_id", "2") def test_get_rule_or_workflow_id_falls_back_to_available_id() -> None: - assert get_rule_or_workflow_id(_rule({"legacy_rule_id": "1"}), prefer="workflow_id") == ( + assert get_rule_or_workflow_id( + _rule(legacy_rule_id=1, workflow_id=None), prefer="workflow_id" + ) == ( "legacy_rule_id", "1", ) - assert get_rule_or_workflow_id(_rule({"workflow_id": "2"})) == ("workflow_id", "2") - - -def test_get_rule_or_workflow_id_falls_back_to_rule_id() -> None: - assert get_rule_or_workflow_id(_rule({}), prefer="workflow_id") == ("legacy_rule_id", "99") + assert get_rule_or_workflow_id(_rule(legacy_rule_id=None, workflow_id=2)) == ( + "workflow_id", + "2", + ) diff --git a/tests/sentry/sentry_apps/tasks/test_sentry_apps.py b/tests/sentry/sentry_apps/tasks/test_sentry_apps.py index 42f66d26d498..47e7686be609 100644 --- a/tests/sentry/sentry_apps/tasks/test_sentry_apps.py +++ b/tests/sentry/sentry_apps/tasks/test_sentry_apps.py @@ -22,6 +22,7 @@ from sentry.issues.grouptype import FeedbackGroup from sentry.issues.ingest import save_issue_occurrence from sentry.models.activity import Activity +from sentry.notifications.types import NotificationRule, RuleFuture from sentry.sentry_apps.metrics import SentryAppWebhookFailureReason, SentryAppWebhookHaltReason from sentry.sentry_apps.models.sentry_app import SentryApp from sentry.sentry_apps.models.sentry_app_installation import SentryAppInstallation @@ -56,7 +57,6 @@ from sentry.testutils.silo import assume_test_silo_mode, assume_test_silo_mode_of, control_silo_test from sentry.testutils.skips import requires_snuba from sentry.types.activity import ActivityType -from sentry.types.rules import RuleFuture from sentry.users.services.user.service import user_service from sentry.utils import json from sentry.utils.http import absolute_uri @@ -123,6 +123,15 @@ class TestSendAlertEvent(TestCase, OccurrenceTestMixin): def setUp(self) -> None: self.sentry_app = self.create_sentry_app(organization=self.organization) self.rule = self.create_project_rule(name="Issa Rule") + self.notification_rule = NotificationRule( + action_id=self.rule.id, + label=self.rule.label, + data={"actions": self.rule.data["actions"]}, + project=self.rule.project, + environment_id=self.rule.environment_id, + workflow_id=123, + legacy_rule_id=self.rule.id, + ) self.install = self.create_sentry_app_installation( organization=self.organization, slug=self.sentry_app.slug ) @@ -195,7 +204,7 @@ def test_no_sentry_app_in_future(self, safe_urlopen: MagicMock) -> None: event = self.store_event(data={}, project_id=self.project.id) assert event.group is not None group_event = GroupEvent.from_event(event, event.group) - rule_future = RuleFuture(rule=self.rule, kwargs={}) + rule_future = RuleFuture(rule=self.notification_rule, kwargs={}) with self.tasks(): notify_sentry_app(group_event, [rule_future]) @@ -209,7 +218,7 @@ def test_no_installation(self, mock_record: MagicMock, safe_urlopen: MagicMock) event = self.store_event(data={}, project_id=self.project.id) assert event.group is not None group_event = GroupEvent.from_event(event, event.group) - rule_future = RuleFuture(rule=self.rule, kwargs={"sentry_app": sentry_app}) + rule_future = RuleFuture(rule=self.notification_rule, kwargs={"sentry_app": sentry_app}) with self.tasks(): notify_sentry_app(group_event, [rule_future]) @@ -236,7 +245,9 @@ def test_send_alert_event(self, mock_record: MagicMock, safe_urlopen: MagicMock) assert event.group is not None group = event.group group_event = GroupEvent.from_event(event, group) - rule_future = RuleFuture(rule=self.rule, kwargs={"sentry_app": self.sentry_app}) + rule_future = RuleFuture( + rule=self.notification_rule, kwargs={"sentry_app": self.sentry_app} + ) with self.tasks(): notify_sentry_app(group_event, [rule_future]) @@ -314,7 +325,7 @@ def test_send_alert_event_with_additional_payload( ] rule_future = RuleFuture( - rule=self.rule, + rule=self.notification_rule, kwargs={"sentry_app": self.sentry_app, "schema_defined_settings": settings}, ) @@ -375,7 +386,15 @@ def test_send_alert_event_with_additional_payload_legacy_rule_id( ] rule_future = RuleFuture( - rule=rule, + rule=NotificationRule( + action_id=rule.id, + label=rule.label, + data={"actions": rule.data["actions"]}, + project=rule.project, + environment_id=rule.environment_id, + workflow_id=123, + legacy_rule_id=rule.id, + ), kwargs={"sentry_app": self.sentry_app, "schema_defined_settings": settings}, ) @@ -426,7 +445,9 @@ def test_send_alert_event_with_groupevent( group_event = event.for_group(group_info.group) group_event.occurrence = occurrence - rule_future = RuleFuture(rule=self.rule, kwargs={"sentry_app": self.sentry_app}) + rule_future = RuleFuture( + rule=self.notification_rule, kwargs={"sentry_app": self.sentry_app} + ) with self.tasks(): notify_sentry_app(group_event, [rule_future]) @@ -515,7 +536,9 @@ def test_feedback_alert_webhook_includes_message_in_metadata_value( group_event = event.for_group(group_info.group) group_event.occurrence = occurrence - rule_future = RuleFuture(rule=self.rule, kwargs={"sentry_app": self.sentry_app}) + rule_future = RuleFuture( + rule=self.notification_rule, kwargs={"sentry_app": self.sentry_app} + ) with self.tasks(): notify_sentry_app(group_event, [rule_future]) @@ -554,7 +577,7 @@ def test_send_alert_event_with_3p_failure( ] rule_future = RuleFuture( - rule=self.rule, + rule=self.notification_rule, kwargs={"sentry_app": self.sentry_app, "schema_defined_settings": settings}, ) diff --git a/tests/sentry/tasks/test_digests.py b/tests/sentry/tasks/test_digests.py index ad0f473e5905..c70f98c4c822 100644 --- a/tests/sentry/tasks/test_digests.py +++ b/tests/sentry/tasks/test_digests.py @@ -10,6 +10,7 @@ from sentry.digests.backends.redis import RedisBackend from sentry.digests.notifications import event_to_record from sentry.models.projectownership import ProjectOwnership +from sentry.notifications.types import NotificationRule from sentry.tasks.digests import deliver_digest from sentry.testutils.cases import TestCase from sentry.testutils.helpers.datetime import before_now @@ -27,6 +28,7 @@ def run_test(self, key: str) -> None: digests.backend.digest = backend.digest rule = self.create_project_rule(project=self.project) + notification_rule = NotificationRule.from_deprecated_legacy_rule(rule) ProjectOwnership.objects.create(project_id=self.project.id, fallthrough=True) event = self.store_event( data={"timestamp": before_now(days=1).isoformat(), "fingerprint": ["group-1"]}, @@ -39,13 +41,13 @@ def run_test(self, key: str) -> None: notification_uuid = str(uuid.uuid4()) backend.add( key, - event_to_record(event, [rule], notification_uuid), + event_to_record(event, [notification_rule], notification_uuid), increment_delay=0, maximum_delay=0, ) backend.add( key, - event_to_record(event_2, [rule], notification_uuid), + event_to_record(event_2, [notification_rule], notification_uuid), increment_delay=0, maximum_delay=0, )