Browse Source

fix(active-release): change over ReleaseSummaryActivityNotification to use the new notification settings (#37316)

* extract out ActiveReleaseAlertNotification into its own file.
introduce special codepath for NotifyActiveReleaseEmailAction(NotifyEmailAction)

* undo filter on rule futures

* add simple test, make apply_active_release_rule less parameterized

* add one more test. decouple adapter to Notifications

* parameterize notification_type to be passed down for checking NotificationSettingTypes
add more email tests
add slack tests

* default value should be ISSUE_ALERTS

* ellaborate more on NotifyActiveReleaseEmailAction

* comment formatting

* throttle predicate evaluation based on PR. remove IssueOccurrencesFilter - if
we're already evaluating, the issue already is occurring.

* clean-up logging statements with accurate references

* extract out ActiveReleaseAlertNotification into its own file.
introduce special codepath for NotifyActiveReleaseEmailAction(NotifyEmailAction)

* release summary should respect the new notification setting instead of a deploy setting

* fix typing

* NotificationSettingTypes.ACTIVE_RELEASE should be project-level setting

* fix typing, update comments

* refactor class name ReleaseSummaryActivityNotification -> ActiveReleaseSummaryNotification

* style(lint): Auto commit lint changes

* revert filtering

* add types for clarity

Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com>
Gilbert Szeto 2 years ago
parent
commit
8e0f5c1478

+ 1 - 0
src/sentry/models/notificationsetting.py

@@ -73,6 +73,7 @@ class NotificationSetting(Model):
             (NotificationSettingTypes.DEPLOY, "deploy"),
             (NotificationSettingTypes.ISSUE_ALERTS, "issue"),
             (NotificationSettingTypes.WORKFLOW, "workflow"),
+            (NotificationSettingTypes.ACTIVE_RELEASE, "activeRelease"),
             (NotificationSettingTypes.APPROVAL, "approval"),
             (NotificationSettingTypes.QUOTA, "quota"),
             (NotificationSettingTypes.QUOTA_ERRORS, "quotaErrors"),

+ 1 - 2
src/sentry/notifications/notifications/active_release.py

@@ -113,7 +113,7 @@ class ActiveReleaseIssueNotification(AlertRuleNotification):
             "users_seen": self.group.count_users_seen(),
             "event": self.event,
             "link": get_group_settings_link(
-                self.group, environment, rule_details=None, referrer="alert_email_release"
+                group, environment, rule_details=None, referrer="alert_email_release"
             ),
             "has_integrations": has_integrations(self.organization, self.project),
             "enhanced_privacy": enhanced_privacy,
@@ -127,7 +127,6 @@ class ActiveReleaseIssueNotification(AlertRuleNotification):
             if self.event_state and self.event_state.is_regression
             else False,
         }
-
         # if the organization has enabled enhanced privacy controls we don't send
         # data which may show PII or source code
         if not enhanced_privacy:

+ 8 - 3
src/sentry/notifications/notifications/activity/release_summary.py

@@ -31,9 +31,9 @@ from sentry.utils.http import absolute_uri
 from .base import ActivityNotification
 
 
-class ReleaseSummaryActivityNotification(ActivityNotification):
+class ActiveReleaseSummaryNotification(ActivityNotification):
     metrics_key = "release_summary"
-    notification_setting_type = NotificationSettingTypes.DEPLOY
+    notification_setting_type = NotificationSettingTypes.ACTIVE_RELEASE
     template_path = "sentry/emails/activity/release_summary"
 
     def __init__(self, activity: Activity) -> None:
@@ -76,7 +76,12 @@ class ReleaseSummaryActivityNotification(ActivityNotification):
     def get_participants_with_group_subscription_reason(
         self,
     ) -> Mapping[ExternalProviders, Mapping[Team | User, int]]:
-        return get_participants_for_release(self.projects, self.organization, self.user_ids)
+        return get_participants_for_release(
+            self.projects,
+            self.organization,
+            self.user_ids,
+            self.notification_setting_type,
+        )
 
     def get_users_by_teams(self) -> Mapping[int, list[int]]:
         if not self.user_id_team_lookup:

+ 63 - 23
src/sentry/notifications/utils/participants.py

@@ -2,7 +2,7 @@ from __future__ import annotations
 
 import logging
 from collections import defaultdict
-from typing import TYPE_CHECKING, Any, Iterable, Mapping, MutableMapping, Sequence
+from typing import TYPE_CHECKING, Any, Iterable, Mapping, MutableMapping, Sequence, Union
 
 from sentry import features
 from sentry.models import (
@@ -20,6 +20,7 @@ from sentry.models import (
     UserOption,
 )
 from sentry.notifications.helpers import (
+    get_scope_type,
     get_settings_by_provider,
     get_values_by_provider_by_type,
     transform_to_notification_settings_by_recipient,
@@ -85,34 +86,51 @@ def get_participants_for_group(
     return participants_by_provider
 
 
-def get_reason(
-    user: User, value: NotificationSettingOptionValues, user_ids: set[int]
-) -> int | None:
-    # Members who opt into all deploy emails.
-    if value == NotificationSettingOptionValues.ALWAYS:
-        return GroupSubscriptionReason.deploy_setting
+ProviderNotificationValues = Mapping[ExternalProviders, NotificationSettingOptionValues]
 
-    # Members which have been seen in the commit log.
-    elif value == NotificationSettingOptionValues.COMMITTED_ONLY and user.id in user_ids:
-        return GroupSubscriptionReason.committed
-    return None
+ScopedProviderNotificationValues = Mapping[NotificationScopeType, ProviderNotificationValues]
+
+RecipientScopedProviderNotificationValues = Mapping[
+    Union[Team, User], ScopedProviderNotificationValues
+]
 
 
 def get_participants_for_release(
-    projects: Iterable[Project], organization: Organization, user_ids: set[int]
+    projects: Iterable[Project],
+    organization: Organization,
+    user_ids: set[int],
+    notification_setting_type: NotificationSettingTypes = NotificationSettingTypes.DEPLOY,
 ) -> Mapping[ExternalProviders, Mapping[Team | User, int]]:
     # Collect all users with verified emails on a team in the related projects.
     users = set(User.objects.get_team_members_with_verified_email_for_projects(projects))
 
-    # Get all the involved users' settings for deploy-emails (including
-    # users' organization-independent settings.)
-    notification_settings = NotificationSetting.objects.get_for_recipient_by_parent(
-        NotificationSettingTypes.DEPLOY,
-        recipients=users,
-        parent=organization,
-    )
-    notification_settings_by_recipient = transform_to_notification_settings_by_recipient(
-        notification_settings, users
+    parent_specific_scope_type = get_scope_type(notification_setting_type)
+
+    def parent_notification_settings_by_recipient(
+        setting_type: NotificationSettingTypes,
+        parent: Iterable[Union[Project, Organization]],
+        recipients: Iterable[Team | User],
+    ) -> RecipientScopedProviderNotificationValues:
+        by_recipient: MutableMapping[Team | User, ScopedProviderNotificationValues] = {}
+        for project_or_org in parent:
+            notification_settings = NotificationSetting.objects.get_for_recipient_by_parent(
+                setting_type,
+                recipients=recipients,
+                parent=project_or_org,
+            )
+            by_recipient.update(
+                transform_to_notification_settings_by_recipient(notification_settings, recipients)
+            )
+        return by_recipient
+
+    # Get all the involved users' settings for deploy-emails or active-release emails
+    # (including users' organization-independent settings.)
+    notification_settings_by_recipient = (
+        parent_notification_settings_by_recipient(notification_setting_type, projects, users)
+        if parent_specific_scope_type == NotificationScopeType.PROJECT
+        else parent_notification_settings_by_recipient(
+            notification_setting_type, [organization], users
+        )
     )
 
     # Map users to their setting value. Prioritize user/org specific, then
@@ -120,16 +138,38 @@ def get_participants_for_release(
     users_to_reasons_by_provider: MutableMapping[
         ExternalProviders, MutableMapping[Team | User, int]
     ] = defaultdict(dict)
+
+    def get_reason(
+        user: User,
+        value: NotificationSettingOptionValues,
+        user_ids: set[int],
+        notification_setting_type: NotificationSettingTypes,
+    ) -> int | None:
+        if notification_setting_type == NotificationSettingTypes.DEPLOY:
+            # Members who opt into all deploy emails.
+            if value == NotificationSettingOptionValues.ALWAYS:
+                return GroupSubscriptionReason.deploy_setting
+
+            # Members which have been seen in the commit log.
+            elif value == NotificationSettingOptionValues.COMMITTED_ONLY and user.id in user_ids:
+                return GroupSubscriptionReason.committed
+        elif notification_setting_type == NotificationSettingTypes.ACTIVE_RELEASE:
+            if value == NotificationSettingOptionValues.ALWAYS:
+                # user must be a committer and a part of this release
+                return GroupSubscriptionReason.committed
+
+        return None
+
     for user in users:
         notification_settings_by_scope = notification_settings_by_recipient.get(user, {})
         values_by_provider = get_values_by_provider_by_type(
             notification_settings_by_scope,
             notification_providers(),
-            NotificationSettingTypes.DEPLOY,
+            notification_setting_type,
             user,
         )
         for provider, value in values_by_provider.items():
-            reason_option = get_reason(user, value, user_ids)
+            reason_option = get_reason(user, value, user_ids, notification_setting_type)
             if reason_option:
                 users_to_reasons_by_provider[provider][user] = reason_option
     return users_to_reasons_by_provider

+ 2 - 2
src/sentry/tasks/release_summary.py

@@ -5,7 +5,7 @@ from django.utils import timezone
 from sentry import features
 from sentry.models import Activity, Release, ReleaseActivity, ReleaseCommit
 from sentry.notifications.notifications.activity.release_summary import (
-    ReleaseSummaryActivityNotification,
+    ActiveReleaseSummaryNotification,
 )
 from sentry.notifications.utils.participants import _get_release_committers
 from sentry.tasks.base import instrumented_task
@@ -55,7 +55,7 @@ def prepare_release_summary():
         if not activity:
             continue
 
-        release_summary = ReleaseSummaryActivityNotification(activity)
+        release_summary = ActiveReleaseSummaryNotification(activity)
         release_summary.send()
         if features.has("organizations:active-release-monitor-alpha", release.organization):
             ReleaseActivity.objects.create(

+ 31 - 9
tests/sentry/mail/activity/test_release_summary.py

@@ -2,11 +2,15 @@ from urllib.parse import quote
 
 from django.core import mail
 
-from sentry.models import Activity, Environment, Repository
+from sentry.models import Activity, Environment, NotificationSetting, Repository
 from sentry.notifications.notifications.activity.release_summary import (
-    ReleaseSummaryActivityNotification,
+    ActiveReleaseSummaryNotification,
+)
+from sentry.notifications.types import (
+    GroupSubscriptionReason,
+    NotificationSettingOptionValues,
+    NotificationSettingTypes,
 )
-from sentry.notifications.types import GroupSubscriptionReason
 from sentry.testutils.cases import ActivityTestCase
 from sentry.types.activity import ActivityType
 from sentry.types.integrations import ExternalProviders
@@ -44,7 +48,7 @@ class ReleaseSummaryTestCase(ActivityTestCase):
 
     def test_simple(self):
         with self.feature("organizations:active-release-notification-opt-in"):
-            release_summary = ReleaseSummaryActivityNotification(
+            release_summary = ActiveReleaseSummaryNotification(
                 Activity(
                     project=self.project,
                     user=self.user1,
@@ -53,12 +57,30 @@ class ReleaseSummaryTestCase(ActivityTestCase):
                 )
             )
 
+        assert NotificationSetting.objects.all().count() == 0
+
+        # opt-in to getting active_release notifications
+        NotificationSetting.objects.update_settings(
+            ExternalProviders.EMAIL,
+            NotificationSettingTypes.ACTIVE_RELEASE,
+            NotificationSettingOptionValues.ALWAYS,
+            user=self.user1,
+            project=self.project,
+        )
+        assert (
+            NotificationSetting.objects.all()[0].type
+            == NotificationSettingTypes.ACTIVE_RELEASE.value
+        )
+
+        recipient_by_parent = NotificationSetting.objects.get_for_recipient_by_parent(
+            NotificationSettingTypes.ACTIVE_RELEASE, self.project, [self.user1, self.user2]
+        )
+        assert len(recipient_by_parent) == 1
+
         # user1 is included because they committed
-        participants = release_summary.get_participants_with_group_subscription_reason()[
-            ExternalProviders.EMAIL
-        ]
-        assert len(participants) == 1
-        assert participants == {
+        participants = release_summary.get_participants_with_group_subscription_reason()
+        assert len(participants[ExternalProviders.EMAIL]) == 1
+        assert participants[ExternalProviders.EMAIL] == {
             self.user1: GroupSubscriptionReason.committed,
         }
 

+ 2 - 2
tests/sentry/tasks/test_release_summary.py

@@ -45,7 +45,7 @@ class SendReleaseSummaryTest(ActivityTestCase):
         self.commit1 = self.another_commit(0, "a", self.user1, repository)
 
     @patch(
-        "sentry.notifications.notifications.activity.release_summary.ReleaseSummaryActivityNotification.send"
+        "sentry.notifications.notifications.activity.release_summary.ActiveReleaseSummaryNotification.send"
     )
     def test_simple(self, mock_release_summary_send):
         with self.feature("organizations:active-release-notification-opt-in"):
@@ -54,7 +54,7 @@ class SendReleaseSummaryTest(ActivityTestCase):
         assert len(mock_release_summary_send.mock_calls) == 1
 
     @patch(
-        "sentry.notifications.notifications.activity.release_summary.ReleaseSummaryActivityNotification.send"
+        "sentry.notifications.notifications.activity.release_summary.ActiveReleaseSummaryNotification.send"
     )
     def test_without_feature_flag(self, mock_release_summary_send):
         prepare_release_summary()