Browse Source

Revert "feat(active-release) cleanup feature flags for active release notifications (#38058)"

This reverts commit 4fec9177ff456e01b7a5a021351a7c5d7b55b6a6.

Co-authored-by: snigdha.sharma via Slack <snigdha.sharma@sentry.io>
Sentry Bot 2 years ago
parent
commit
ea4ee012bd

+ 4 - 2
src/sentry/conf/server.py

@@ -949,8 +949,8 @@ SENTRY_FEATURES = {
     "auth:register": True,
     # Workflow 2.0 Alpha Functionality for sentry users only
     "organizations:active-release-monitor-alpha": False,
-    # Workflow 2.0 Active Release Notifications
-    "organizations:active-release-notifications-enable": False,
+    # Workflow 2.0 Experimental ReleaseMembers who opt-in to get notified as a release committer
+    "organizations:active-release-notification-opt-in": False,
     # Enable advanced search features, like negation and wildcard matching.
     "organizations:advanced-search": True,
     # Use metrics as the dataset for crash free metric alerts
@@ -1166,6 +1166,8 @@ SENTRY_FEATURES = {
     "organizations:team-insights": True,
     # Enable setting team-level roles and receiving permissions from them
     "organizations:team-roles": False,
+    # Enable sending Active release notifications without creating an explicit rule in DB
+    "projects:active-release-monitor-default-on": False,
     # Adds additional filters and a new section to issue alert rules.
     "projects:alert-filters": True,
     # Enable functionality to specify custom inbound filters on events.

+ 2 - 1
src/sentry/features/__init__.py

@@ -58,7 +58,7 @@ default_manager.add("organizations:create")
 
 # Organization scoped features that are in development or in customer trials.
 default_manager.add("organizations:active-release-monitor-alpha", OrganizationFeature, True)
-default_manager.add("organizations:active-release-notifications-enable", OrganizationFeature)
+default_manager.add("organizations:active-release-notification-opt-in", OrganizationFeature, True)
 default_manager.add("organizations:alert-filters", OrganizationFeature)
 default_manager.add("organizations:alert-crash-free-metrics", OrganizationFeature, True)
 default_manager.add("organizations:api-keys", OrganizationFeature)
@@ -193,6 +193,7 @@ default_manager.add("organizations:sso-saml2", OrganizationFeature)
 default_manager.add("organizations:team-insights", OrganizationFeature)
 
 # Project scoped features
+default_manager.add("projects:active-release-monitor-default-on", ProjectFeature)
 default_manager.add("projects:alert-filters", ProjectFeature)
 default_manager.add("projects:custom-inbound-filters", ProjectFeature)
 default_manager.add("projects:data-forwarding", ProjectFeature)

+ 21 - 0
src/sentry/notifications/manager.py

@@ -303,6 +303,27 @@ class NotificationsManager(BaseManager["NotificationSetting"]):
         notification_settings_by_recipient = transform_to_notification_settings_by_recipient(
             notification_settings, recipients
         )
+        # XXX(gilbert): remove this when organizations:active-release-notification-opt-in is removed
+        # injects the notification settings since no Notification settings will exist in the db
+        from sentry import features
+        from sentry.models import Organization, User
+
+        if isinstance(parent, Organization):
+            notification_settings_by_recipient = dict(notification_settings_by_recipient)
+            for user_recipient in filter(lambda x: isinstance(x, User), recipients):
+                if type == NotificationSettingTypes.ACTIVE_RELEASE and features.has(
+                    "organizations:active-release-notification-opt-in",
+                    parent,
+                    actor=user_recipient,
+                ):
+                    notification_settings_by_recipient[user_recipient] = {
+                        {
+                            NotificationScopeType.USER: {
+                                ExternalProviders.EMAIL: NotificationSettingOptionValues.ALWAYS,
+                                ExternalProviders.SLACK: NotificationSettingOptionValues.ALWAYS,
+                            }
+                        }
+                    }
 
         mapping = defaultdict(set)
         for recipient in recipients:

+ 18 - 7
src/sentry/notifications/utils/participants.py

@@ -279,16 +279,27 @@ def _get_release_committers(
     # commit_author_id : Author
     author_users: Mapping[str, Author] = get_users_for_commits(commits)
 
-    release_committers = list(
-        User.objects.filter(id__in={au["id"] for au in author_users.values() if au.get("id")})
-    )
     # TODO(scttcper): Remove this after the experiment
     if release_dry_run:
-        return release_committers
+        return list(
+            User.objects.filter(id__in={au["id"] for au in author_users.values() if au.get("id")})
+        )
 
-    if features.has("organizations:active-release-notifications-enable", release.organization):
-        return release_committers
-    return []
+    # XXX(gilbert): this is inefficient since this evaluates flagr once per user
+    # it should be ok since this method should only be called for projects within sentry
+    # do not copy this unless you know the risk; you've been warned!
+    return list(
+        filter(
+            lambda u: features.has(
+                "organizations:active-release-notification-opt-in", release.organization, actor=u
+            ),
+            list(
+                User.objects.filter(
+                    id__in={au["id"] for au in author_users.values() if au.get("id")}
+                )
+            ),
+        )
+    )
 
 
 def get_send_to(

+ 1 - 3
src/sentry/rules/processor.py

@@ -340,9 +340,7 @@ class RuleProcessor:
             self._get_active_release_rule_actions(),
             1,
             1,
-            not features.has(
-                "organizations:active-release-notifications-enable", self.project.organization
-            ),
+            not features.has("projects:active-release-monitor-default-on", self.project),
         )
 
         return self.grouped_futures.values()

+ 5 - 3
src/sentry/tasks/release_summary.py

@@ -7,6 +7,7 @@ from sentry.models import Activity, Release, ReleaseActivity, ReleaseCommit
 from sentry.notifications.notifications.activity.release_summary import (
     ReleaseSummaryActivityNotification,
 )
+from sentry.notifications.utils.participants import _get_release_committers
 from sentry.tasks.base import instrumented_task
 from sentry.types.activity import ActivityType
 from sentry.types.releaseactivity import ReleaseActivityType
@@ -33,9 +34,10 @@ def prepare_release_summary():
         if not release_has_commits:
             continue
 
-        if not features.has(
-            "organizations:active-release-notifications-enable", release.organization
-        ):
+        # Check if any participants are members of the feature flag
+        # TODO(workflow): can remove with active-release-notification-opt-in
+        participants = _get_release_committers(release)
+        if not participants:
             continue
 
         # Find the activity created in Deploy.notify_if_ready

+ 1 - 1
tests/sentry/integrations/slack/notifications/test_active_release.py

@@ -62,7 +62,7 @@ class SlackIssueAlertNotificationTest(SlackActivityNotificationTest):
     @mock.patch("sentry.notifications.notify.notify", side_effect=send_notification)
     def test_notify_user(self, mock_func, mock_get_release_committers):
         mock_get_release_committers.return_value = [self.user]
-        with self.tasks(), self.feature("organizations:active-release-notifications-enable"):
+        with self.tasks(), self.feature("projects:active-release-monitor-default-on"):
             rp = RuleProcessor(
                 self.event,
                 is_new=True,

+ 1 - 1
tests/sentry/mail/activity/test_release_summary.py

@@ -43,7 +43,7 @@ class ReleaseSummaryTestCase(ActivityTestCase):
         self.commit2 = self.another_commit(1, "b", self.user2, repository)
 
     def test_simple(self):
-        with self.feature("organizations:active-release-notifications-enable"):
+        with self.feature("organizations:active-release-notification-opt-in"):
             release_summary = ReleaseSummaryActivityNotification(
                 Activity(
                     project=self.project,

+ 1 - 1
tests/sentry/notifications/utils/test_participants.py

@@ -197,7 +197,7 @@ class GetSentToReleaseMembersTest(TestCase):
     def test_default_committer(self, spy_get_release_committers):
         event = self.store_event("empty.lol")
         event.group = self.group
-        with self.feature("organizations:active-release-notifications-enable"):
+        with self.feature("organizations:active-release-notification-opt-in"):
             assert self.get_send_to_release_members(event) == {
                 ExternalProviders.EMAIL: {self.user},
                 ExternalProviders.SLACK: {self.user},

+ 5 - 7
tests/sentry/rules/test_processor.py

@@ -483,7 +483,7 @@ class RuleProcessorActiveReleaseTest(TestCase):
     @mock.patch("sentry.notifications.utils.participants.get_release_committers")
     def test_default_notification_setting_off(self, mock_get_release_committers):
         mock_get_release_committers.return_value = [self.user]
-        with self.tasks(), self.feature("organizations:active-release-notifications-enable"):
+        with self.tasks(), self.feature("projects:active-release-monitor-default-on"):
             mail.outbox = []
             rp = RuleProcessor(
                 self.event,
@@ -506,7 +506,7 @@ class RuleProcessorActiveReleaseTest(TestCase):
             user=self.user,
             project=self.project,
         )
-        with self.tasks(), self.feature("organizations:active-release-notifications-enable"):
+        with self.tasks(), self.feature("projects:active-release-monitor-default-on"):
             mail.outbox = []
             rp = RuleProcessor(
                 self.event,
@@ -545,7 +545,7 @@ class RuleProcessorActiveReleaseTest(TestCase):
                 ],
             },
         )
-        with self.tasks(), self.feature("organizations:active-release-notifications-enable"):
+        with self.tasks(), self.feature("projects:active-release-monitor-default-on"):
             mail.outbox = []
             rp = RuleProcessor(
                 self.event,
@@ -583,7 +583,7 @@ class RuleProcessorActiveReleaseTest(TestCase):
         )
 
         mock_get_release_committers.return_value = [self.user, user2]
-        with self.tasks(), self.feature("organizations:active-release-notifications-enable"):
+        with self.tasks(), self.feature("projects:active-release-monitor-default-on"):
             mail.outbox = []
             rp = RuleProcessor(
                 self.event,
@@ -603,9 +603,7 @@ class RuleProcessorActiveReleaseTest(TestCase):
     @mock.patch("sentry.analytics.record")
     def test_active_release_disabled(self, mock_record, mock_get_release_committers):
         mock_get_release_committers.return_value = [self.user]
-        with self.tasks(), self.feature(
-            {"organizations:active-release-notifications-enable": False}
-        ):
+        with self.tasks(), self.feature({"projects:active-release-monitor-default-on": False}):
             mail.outbox = []
             rp = RuleProcessor(
                 self.event,

Some files were not shown because too many files changed in this diff