diff --git a/src/sentry/notifications/notification_action/activity_registry/email.py b/src/sentry/notifications/notification_action/activity_registry/email.py index f18c61ffc0ef..73f82c45df0d 100644 --- a/src/sentry/notifications/notification_action/activity_registry/email.py +++ b/src/sentry/notifications/notification_action/activity_registry/email.py @@ -1,12 +1,21 @@ from sentry.models.activity import Activity +from sentry.notifications.models.notificationaction import ActionTarget from sentry.notifications.notification_action.activity_registry.base import ( NOTIFICATION_PLATFORM_COMPATIBLE_ACTIVITIES, + build_activity_data, require_config, send_activity_notification, ) from sentry.notifications.notification_action.registry import activity_handler_registry from sentry.notifications.notification_action.types import ActivityHandler +from sentry.notifications.platform.service import NotificationService +from sentry.notifications.platform.strategies.issue_owners import ( + IssueOwnersActivityAlertStrategy, +) from sentry.notifications.platform.target import GenericNotificationTarget +from sentry.notifications.platform.templates.workflow_engine import ( + WorkflowEngineActivityAction, +) from sentry.notifications.platform.types import ( NotificationProviderKey, NotificationTargetResourceType, @@ -19,14 +28,24 @@ class EmailActivityHandler(ActivityHandler): compatible_activity_types = NOTIFICATION_PLATFORM_COMPATIBLE_ACTIVITIES + @classmethod + def invoke_using_issue_owners_strategy( + cls, invocation: ActionInvocation, activity: Activity + ) -> None: + if activity.group is None: + return + strategy = IssueOwnersActivityAlertStrategy(group=activity.group) + data = build_activity_data(invocation, activity) + NotificationService[WorkflowEngineActivityAction](data=data).notify_sync(strategy=strategy) + @classmethod def invoke_action(cls, invocation: ActionInvocation, activity: Activity) -> None: - action = invocation.action - # TODO(leander): handle ISSUE_OWNERS target_type — requires resolving - # issue owners and creating a target per owner. + if invocation.action.config.get("target_type") == ActionTarget.ISSUE_OWNERS: + return cls.invoke_using_issue_owners_strategy(invocation, activity) + target = GenericNotificationTarget( provider_key=NotificationProviderKey.EMAIL, resource_type=NotificationTargetResourceType.EMAIL, - resource_id=require_config(action, "target_identifier"), + resource_id=require_config(invocation.action, "target_identifier"), ) send_activity_notification(invocation, activity, target) diff --git a/src/sentry/notifications/platform/strategies/__init__.py b/src/sentry/notifications/platform/strategies/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/src/sentry/notifications/platform/strategies/issue_owners.py b/src/sentry/notifications/platform/strategies/issue_owners.py new file mode 100644 index 000000000000..f148847292f4 --- /dev/null +++ b/src/sentry/notifications/platform/strategies/issue_owners.py @@ -0,0 +1,84 @@ +from __future__ import annotations + +from dataclasses import dataclass + +from sentry.models.group import Group +from sentry.models.groupassignee import GroupAssignee +from sentry.models.groupowner import GroupOwner +from sentry.models.organizationmemberteam import OrganizationMemberTeam +from sentry.notifications.platform.target import GenericNotificationTarget +from sentry.notifications.platform.types import ( + NotificationProviderKey, + NotificationStrategy, + NotificationTarget, + NotificationTargetResourceType, +) +from sentry.users.services.user.service import user_service + + +@dataclass(frozen=True) +class IssueOwnersActivityAlertStrategy(NotificationStrategy): + """ + Strategy for sending activity alerts to Issue Owners. + A bit different than that used for GroupEvent alerts, since determine_eligible_recipients + cannot be used here, we don't have an event to parse. + """ + + group: Group + + def get_targets(self) -> list[NotificationTarget]: + user_ids, team_ids = self.get_issue_owner_ids() + + if team_ids: + members = OrganizationMemberTeam.objects.filter(team_id__in=team_ids).select_related( + "organizationmember" + ) + for member in members: + uid = member.organizationmember.user_id + if uid is not None: + user_ids.add(uid) + + if not user_ids: + return [] + + users = user_service.get_many_by_id(ids=list(user_ids)) + targets: list[NotificationTarget] = [] + for user in users: + if not user.email: + continue + targets.append( + GenericNotificationTarget( + provider_key=NotificationProviderKey.EMAIL, + resource_type=NotificationTargetResourceType.EMAIL, + resource_id=user.email, + ) + ) + return targets + + def get_issue_owner_ids(self) -> tuple[set[int], set[int]]: + """ + Returns a tuple of (user_ids, team_ids) based on the issue owners. + If a GroupAssignee is set, it will be the only identifier returned. + If not, it will return the identifiers of the GroupOwners. + """ + user_ids: set[int] = set() + team_ids: set[int] = set() + + assignee = GroupAssignee.objects.filter(group=self.group).first() + if assignee is not None: + if assignee.user_id is not None: + user_ids.add(assignee.user_id) + elif assignee.team_id is not None: + team_ids.add(assignee.team_id) + return user_ids, team_ids + + owners = GroupOwner.objects.filter(group=self.group).exclude( + user_id__isnull=True, team_id__isnull=True + ) + for owner in owners: + if owner.user_id is not None: + user_ids.add(owner.user_id) + elif owner.team_id is not None: + team_ids.add(owner.team_id) + + return user_ids, team_ids diff --git a/src/sentry/testutils/factories.py b/src/sentry/testutils/factories.py index 32cd258e137a..4ed178c18060 100644 --- a/src/sentry/testutils/factories.py +++ b/src/sentry/testutils/factories.py @@ -102,6 +102,7 @@ from sentry.models.grouphistory import GroupHistory from sentry.models.grouplink import GroupLink from sentry.models.groupopenperiod import GroupOpenPeriod +from sentry.models.groupowner import GroupOwner, GroupOwnerType from sentry.models.grouprelease import GroupRelease from sentry.models.organization import Organization from sentry.models.organizationmapping import OrganizationMapping @@ -1270,6 +1271,25 @@ def create_group(project, create_open_period=True, **kwargs): def create_group_activity(group, *args, **kwargs): return Activity.objects.create(group=group, project=group.project, *args, **kwargs) + @staticmethod + @assume_test_silo_mode(SiloMode.CELL) + def create_group_owner( + group, + type=GroupOwnerType.OWNERSHIP_RULE.value, + user_id=None, + team=None, + **kwargs, + ) -> GroupOwner: + return GroupOwner.objects.create( + group=group, + project=group.project, + organization=group.project.organization, + type=type, + user_id=user_id, + team=team, + **kwargs, + ) + @staticmethod @assume_test_silo_mode(SiloMode.CELL) def create_group_action_log_entry( diff --git a/src/sentry/testutils/fixtures.py b/src/sentry/testutils/fixtures.py index f9a34bb038fd..40ca57aeea6b 100644 --- a/src/sentry/testutils/fixtures.py +++ b/src/sentry/testutils/fixtures.py @@ -361,6 +361,11 @@ def create_group_activity(self, group=None, *args, **kwargs): group = self.group return Factories.create_group_activity(group, *args, **kwargs) + def create_group_owner(self, group=None, **kwargs): + if group is None: + group = self.group + return Factories.create_group_owner(group, **kwargs) + def create_group_action_log_entry(self, group=None, *args, **kwargs) -> GroupActionLogEntry: if group is None: group = self.group diff --git a/tests/sentry/notifications/notification_action/activity_registry/test_email.py b/tests/sentry/notifications/notification_action/activity_registry/test_email.py index e38a0965959f..64b417e99f16 100644 --- a/tests/sentry/notifications/notification_action/activity_registry/test_email.py +++ b/tests/sentry/notifications/notification_action/activity_registry/test_email.py @@ -1,7 +1,11 @@ from unittest import mock +from sentry.notifications.models.notificationaction import ActionTarget from sentry.notifications.notification_action.activity_registry.email import EmailActivityHandler from sentry.notifications.notification_action.registry import activity_handler_registry +from sentry.notifications.platform.strategies.issue_owners import ( + IssueOwnersActivityAlertStrategy, +) from sentry.notifications.platform.target import GenericNotificationTarget from sentry.notifications.platform.types import ( NotificationProviderKey, @@ -25,7 +29,7 @@ def setUp(self) -> None: self.action = self.create_action( type=Action.Type.EMAIL, config={ - "target_type": 1, + "target_type": ActionTarget.USER, "target_identifier": str(self.user.id), }, ) @@ -58,3 +62,41 @@ def test_invoke_action(self, mock_send: mock.MagicMock) -> None: assert target.provider_key == NotificationProviderKey.EMAIL assert target.resource_type == NotificationTargetResourceType.EMAIL assert target.resource_id == str(self.user.id) + + @mock.patch( + "sentry.notifications.notification_action.activity_registry.email.NotificationService" + ) + @mock.patch( + "sentry.notifications.notification_action.activity_registry.email.build_activity_data" + ) + def test_invoke_action_issue_owners( + self, + mock_build_data: mock.MagicMock, + mock_notification_service: mock.MagicMock, + ) -> None: + self.action.config = { + "target_type": ActionTarget.ISSUE_OWNERS, + } + self.action.save() + + activity = self.create_group_activity( + group=self.group, + type=ActivityType.SEER_RCA_STARTED.value, + ) + invocation = self.create_action_invocation( + event=activity, + group=self.group, + action=self.action, + detector=self.detector, + workflow_id=self.workflow.id, + ) + + EmailActivityHandler.invoke_action(invocation=invocation, activity=activity) + + mock_build_data.assert_called_once_with(invocation, activity) + mock_service_instance = mock_notification_service.__getitem__.return_value.return_value + mock_service_instance.notify_sync.assert_called_once() + call_kwargs = mock_service_instance.notify_sync.call_args[1] + strategy = call_kwargs["strategy"] + assert isinstance(strategy, IssueOwnersActivityAlertStrategy) + assert strategy.group == activity.group diff --git a/tests/sentry/notifications/platform/strategies/__init__.py b/tests/sentry/notifications/platform/strategies/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/tests/sentry/notifications/platform/strategies/test_issue_owners.py b/tests/sentry/notifications/platform/strategies/test_issue_owners.py new file mode 100644 index 000000000000..69b967c78e52 --- /dev/null +++ b/tests/sentry/notifications/platform/strategies/test_issue_owners.py @@ -0,0 +1,139 @@ +from sentry.models.groupassignee import GroupAssignee +from sentry.models.groupowner import GroupOwnerType +from sentry.notifications.platform.strategies.issue_owners import ( + IssueOwnersActivityAlertStrategy, +) +from sentry.notifications.platform.target import GenericNotificationTarget +from sentry.notifications.platform.types import ( + NotificationProviderKey, + NotificationTargetResourceType, +) +from sentry.testutils.cases import TestCase + + +class TestIssueOwnersActivityAlertStrategy(TestCase): + def setUp(self) -> None: + super().setUp() + self.group = self.create_group(project=self.project) + + def test_assignee_user_returns_single_target(self) -> None: + GroupAssignee.objects.assign(self.group, self.user) + + strategy = IssueOwnersActivityAlertStrategy(group=self.group) + targets = strategy.get_targets() + + assert len(targets) == 1 + assert targets[0].resource_id == self.user.email + + def test_assignee_team_resolves_to_member_emails(self) -> None: + user_a = self.create_user(email="a@example.com") + user_b = self.create_user(email="b@example.com") + team = self.create_team(organization=self.organization) + self.create_team_membership(team=team, user=user_a) + self.create_team_membership(team=team, user=user_b) + GroupAssignee.objects.assign(self.group, assigned_to=team) + + strategy = IssueOwnersActivityAlertStrategy(group=self.group) + targets = strategy.get_targets() + + emails = {t.resource_id for t in targets} + assert "a@example.com" in emails + assert "b@example.com" in emails + + def test_assignee_takes_precedence_over_owners(self) -> None: + other_user = self.create_user(email="owner@example.com") + self.create_member(organization=self.organization, user=other_user) + self.create_group_owner( + group=self.group, + user_id=other_user.id, + ) + GroupAssignee.objects.assign(self.group, self.user) + + strategy = IssueOwnersActivityAlertStrategy(group=self.group) + targets = strategy.get_targets() + + emails = {t.resource_id for t in targets} + assert self.user.email in emails + assert "owner@example.com" not in emails + + def test_owners_used_when_no_assignee(self) -> None: + other_user = self.create_user(email="owner@example.com") + self.create_member(organization=self.organization, user=other_user) + self.create_group_owner( + group=self.group, + user_id=other_user.id, + ) + + strategy = IssueOwnersActivityAlertStrategy(group=self.group) + targets = strategy.get_targets() + + assert len(targets) == 1 + assert targets[0].resource_id == "owner@example.com" + + def test_multiple_owners_all_notified(self) -> None: + user_a = self.create_user(email="a@example.com") + user_b = self.create_user(email="b@example.com") + self.create_member(organization=self.organization, user=user_a) + self.create_member(organization=self.organization, user=user_b) + for u in (user_a, user_b): + self.create_group_owner( + group=self.group, + type=GroupOwnerType.SUSPECT_COMMIT.value, + user_id=u.id, + ) + + strategy = IssueOwnersActivityAlertStrategy(group=self.group) + targets = strategy.get_targets() + + emails = {t.resource_id for t in targets} + assert "a@example.com" in emails + assert "b@example.com" in emails + + def test_team_owner_resolves_to_members(self) -> None: + team_user = self.create_user(email="team-member@example.com") + team = self.create_team(organization=self.organization) + self.create_team_membership(team=team, user=team_user) + self.create_group_owner( + group=self.group, + type=GroupOwnerType.CODEOWNERS.value, + team=team, + ) + + strategy = IssueOwnersActivityAlertStrategy(group=self.group) + targets = strategy.get_targets() + + emails = {t.resource_id for t in targets} + assert "team-member@example.com" in emails + + def test_deduplicates_across_owner_types(self) -> None: + self.create_group_owner( + group=self.group, + type=GroupOwnerType.SUSPECT_COMMIT.value, + user_id=self.user.id, + ) + self.create_group_owner( + group=self.group, + type=GroupOwnerType.OWNERSHIP_RULE.value, + user_id=self.user.id, + ) + + strategy = IssueOwnersActivityAlertStrategy(group=self.group) + targets = strategy.get_targets() + + emails = [t.resource_id for t in targets] + assert emails.count(self.user.email) == 1 + + def test_no_assignee_no_owners_returns_empty(self) -> None: + strategy = IssueOwnersActivityAlertStrategy(group=self.group) + assert strategy.get_targets() == [] + + def test_all_targets_are_email_type(self) -> None: + GroupAssignee.objects.assign(self.group, self.user) + + strategy = IssueOwnersActivityAlertStrategy(group=self.group) + targets = strategy.get_targets() + + for target in targets: + assert isinstance(target, GenericNotificationTarget) + assert target.provider_key == NotificationProviderKey.EMAIL + assert target.resource_type == NotificationTargetResourceType.EMAIL