feat(notifications): replace logic for bulk settings update (#60502)

Trying to remove all dependencies on `NotificationSetting`. We have a
`bulk_update_settings` that's only used to enable all settings for a
provider so I'm making the function do that instead without updating
`NotificationSetting` rows.

Author: Stephen Cefali

src/sentry/integrations/slack/webhooks/action.py

@@ -28,13 +28,12 @@
 from sentry.models.activity import ActivityIntegration
 from sentry.models.group import Group
 from sentry.models.organizationmember import InviteStatus, OrganizationMember
-from sentry.notifications.defaults import NOTIFICATION_SETTINGS_ALL_SOMETIMES
 from sentry.notifications.utils.actions import MessageAction
 from sentry.services.hybrid_cloud.integration import integration_service
 from sentry.services.hybrid_cloud.notifications import notifications_service
 from sentry.services.hybrid_cloud.user import RpcUser
 from sentry.shared_integrations.exceptions import ApiError
-from sentry.types.integrations import ExternalProviders
+from sentry.types.integrations import ExternalProviderEnum
 from sentry.utils import json
 from sentry.web.decorators import transaction_start
 
@@ -467,10 +466,9 @@ def handle_enable_notifications(self, slack_request: SlackActionRequest) -> Resp
         if not identity_user:
             return self.respond_with_text(NO_IDENTITY_MESSAGE)
 
-        notifications_service.bulk_update_settings(
-            external_provider=ExternalProviders.SLACK,
+        notifications_service.enable_all_settings_for_provider(
+            external_provider=ExternalProviderEnum.SLACK,
             user_id=identity_user.id,
-            notification_type_to_value_map=NOTIFICATION_SETTINGS_ALL_SOMETIMES,
         )
         return self.respond_with_text(ENABLE_SLACK_SUCCESS_MESSAGE)
 

src/sentry/notifications/manager.py

@@ -521,41 +521,3 @@ def disable_settings_for_users(
                     user_id=user.id,
                     defaults={"value": NotificationSettingOptionValues.NEVER.value},
                 )
-
-    def has_any_provider_settings(
-        self, recipient: RpcActor | Team | User, provider: ExternalProviders
-    ) -> bool:
-        from sentry.models.team import Team
-        from sentry.models.user import User
-
-        key_field = None
-        if isinstance(recipient, RpcActor):
-            key_field = "user_id" if recipient.actor_type == ActorType.USER else "team_id"
-        if isinstance(recipient, (RpcUser, User)):
-            key_field = "user_id"
-        if isinstance(recipient, Team):
-            key_field = "team_id"
-
-        assert key_field, "Could not resolve key_field"
-
-        team_ids: Set[int] = set()
-        user_ids: Set[int] = set()
-        if isinstance(recipient, RpcActor):
-            (team_ids if recipient.actor_type == ActorType.TEAM else user_ids).add(recipient.id)
-        elif isinstance(recipient, Team):
-            team_ids.add(recipient.id)
-        elif isinstance(recipient, User):
-            user_ids.add(recipient.id)
-
-        return (
-            self._filter(provider=provider, team_ids=team_ids, user_ids=user_ids)
-            .filter(
-                value__in={
-                    NotificationSettingOptionValues.ALWAYS.value,
-                    NotificationSettingOptionValues.COMMITTED_ONLY.value,
-                    NotificationSettingOptionValues.SUBSCRIBE_ONLY.value,
-                },
-                **{key_field: recipient.id},
-            )
-            .exists()
-        )

src/sentry/services/hybrid_cloud/notifications/impl.py

@@ -13,6 +13,7 @@
 from sentry.models.user import User
 from sentry.notifications.notificationcontroller import NotificationController
 from sentry.notifications.types import (
+    NOTIFICATION_SETTING_TYPES,
     NotificationScopeEnum,
     NotificationSettingEnum,
     NotificationSettingOptionValues,
@@ -30,7 +31,7 @@
 from sentry.services.hybrid_cloud.notifications.serial import serialize_notification_setting
 from sentry.services.hybrid_cloud.user import RpcUser
 from sentry.services.hybrid_cloud.user.service import user_service
-from sentry.types.integrations import ExternalProviders
+from sentry.types.integrations import ExternalProviderEnum, ExternalProviders
 
 
 class DatabaseBackedNotificationsService(NotificationsService):
@@ -67,26 +68,24 @@ def update_settings(
             skip_provider_updates=skip_provider_updates,
         )
 
-    def bulk_update_settings(
+    def enable_all_settings_for_provider(
         self,
         *,
-        notification_type_to_value_map: Mapping[
-            NotificationSettingTypes, NotificationSettingOptionValues
-        ],
-        external_provider: ExternalProviders,
+        external_provider: ExternalProviderEnum,
         user_id: int,
     ) -> None:
-        with transaction.atomic(router.db_for_write(NotificationSetting)):
-            for notification_type, setting_option in notification_type_to_value_map.items():
-                self.update_settings(
-                    external_provider=external_provider,
-                    actor=RpcActor(id=user_id, actor_type=ActorType.USER),
-                    notification_type=notification_type,
-                    setting_option=setting_option,
-                    skip_provider_updates=True,
+        with transaction.atomic(router.db_for_write(NotificationSettingProvider)):
+            for type_str in NOTIFICATION_SETTING_TYPES.values():
+                NotificationSettingProvider.objects.create_or_update(
+                    provider=external_provider.value,
+                    user_id=user_id,
+                    scope_type=NotificationScopeEnum.USER.value,
+                    scope_identifier=user_id,
+                    type=type_str,
+                    values={
+                        "value": NotificationSettingsOptionEnum.ALWAYS.value,
+                    },
                 )
-            # update the providers at the end
-            NotificationSetting.objects.update_provider_settings(user_id, None)
 
     def update_notification_options(
         self,

src/sentry/services/hybrid_cloud/notifications/service.py

@@ -20,7 +20,7 @@
 from sentry.services.hybrid_cloud.rpc import RpcService, rpc_method
 from sentry.services.hybrid_cloud.user import RpcUser
 from sentry.silo import SiloMode
-from sentry.types.integrations import ExternalProviders
+from sentry.types.integrations import ExternalProviderEnum, ExternalProviders
 
 
 class NotificationsService(RpcService):
@@ -53,13 +53,10 @@ def update_settings(
 
     @rpc_method
     @abstractmethod
-    def bulk_update_settings(
+    def enable_all_settings_for_provider(
         self,
         *,
-        notification_type_to_value_map: Mapping[
-            NotificationSettingTypes, NotificationSettingOptionValues
-        ],
-        external_provider: ExternalProviders,
+        external_provider: ExternalProviderEnum,
         user_id: int,
     ) -> None:
         pass

tests/sentry/integrations/slack/webhooks/actions/test_enable_notifications.py

@@ -3,10 +3,8 @@
     NO_IDENTITY_MESSAGE,
 )
 from sentry.models.identity import Identity
-from sentry.models.notificationsetting import NotificationSetting
+from sentry.models.notificationsettingprovider import NotificationSettingProvider
 from sentry.models.user import User
-from sentry.notifications.types import NotificationSettingOptionValues, NotificationSettingTypes
-from sentry.types.integrations import ExternalProviders
 
 from . import BaseEventTest
 
@@ -27,28 +25,34 @@ def test_enable_all_slack_no_identity(self):
         assert response.data["text"] == NO_IDENTITY_MESSAGE
 
     def test_enable_all_slack_already_enabled(self):
-        NotificationSetting.objects.update_settings(
-            ExternalProviders.EMAIL,
-            NotificationSettingTypes.ISSUE_ALERTS,
-            NotificationSettingOptionValues.NEVER,
+        NotificationSettingProvider.objects.create(
             user_id=self.user.id,
+            scope_type="user",
+            scope_identifier=self.user.id,
+            type="alerts",
+            provider="slack",
+            value="never",
         )
-
         response = self.post_webhook(
             action_data=[{"name": "enable_notifications", "value": "all_slack"}]
         )
         self.user = User.objects.get(id=self.user.id)  # Reload to fetch actor
         assert response.status_code == 200, response.content
         assert response.data["text"] == ENABLE_SLACK_SUCCESS_MESSAGE
 
-        assert NotificationSetting.objects.has_any_provider_settings(
-            self.user, ExternalProviders.SLACK
+        assert (
+            NotificationSettingProvider.objects.get(
+                user_id=self.user.id,
+                scope_type="user",
+                scope_identifier=self.user.id,
+                type="alerts",
+                provider="slack",
+            ).value
+            == "always"
         )
 
     def test_enable_all_slack(self):
-        assert not NotificationSetting.objects.has_any_provider_settings(
-            self.user, ExternalProviders.SLACK
-        )
+        assert not NotificationSettingProvider.objects.all().exists()
 
         response = self.post_webhook(
             action_data=[{"name": "enable_notifications", "value": "all_slack"}]
@@ -57,6 +61,13 @@ def test_enable_all_slack(self):
         assert response.status_code == 200, response.content
         assert response.data["text"] == ENABLE_SLACK_SUCCESS_MESSAGE
 
-        assert NotificationSetting.objects.has_any_provider_settings(
-            self.user, ExternalProviders.SLACK
+        assert (
+            NotificationSettingProvider.objects.get(
+                user_id=self.user.id,
+                scope_type="user",
+                scope_identifier=self.user.id,
+                type="alerts",
+                provider="slack",
+            ).value
+            == "always"
         )