Browse Source

ref(notifications): change how we enable slack settings for linking team (#60623)

This PR stops using `notifications_service.update_settings` in Slack
team linking and removes that function since we don't need it anymore.
Stephen Cefali 1 year ago
parent
commit
a91b3477e9

+ 6 - 8
src/sentry/integrations/slack/views/link_team.py

@@ -13,12 +13,11 @@ from sentry.models.integrations.external_actor import ExternalActor
 from sentry.models.integrations.integration import Integration
 from sentry.models.organizationmember import OrganizationMember
 from sentry.models.team import Team
-from sentry.notifications.types import NotificationSettingOptionValues, NotificationSettingTypes
-from sentry.services.hybrid_cloud.actor import ActorType, RpcActor
+from sentry.notifications.types import NotificationSettingEnum
 from sentry.services.hybrid_cloud.identity import identity_service
 from sentry.services.hybrid_cloud.integration import RpcIntegration, integration_service
 from sentry.services.hybrid_cloud.notifications import notifications_service
-from sentry.types.integrations import ExternalProviders
+from sentry.types.integrations import ExternalProviderEnum, ExternalProviders
 from sentry.utils.signing import unsign
 from sentry.web.decorators import transaction_start
 from sentry.web.frontend.base import BaseView, region_silo_view
@@ -198,11 +197,10 @@ class SlackLinkTeamView(BaseView):
         # Turn on notifications for all of a team's projects.
         # TODO(jangjodi): Remove this once the flag is removed
         if not has_team_workflow:
-            notifications_service.update_settings(
-                external_provider=ExternalProviders.SLACK,
-                notification_type=NotificationSettingTypes.ISSUE_ALERTS,
-                setting_option=NotificationSettingOptionValues.ALWAYS,
-                actor=RpcActor(id=team.id, actor_type=ActorType.TEAM),
+            notifications_service.enable_all_settings_for_provider(
+                external_provider=ExternalProviderEnum.SLACK,
+                team_id=team.id,
+                types=[NotificationSettingEnum.ISSUE_ALERTS],
             )
         message = SUCCESS_LINKED_MESSAGE.format(
             slug=team.slug,

+ 22 - 28
src/sentry/services/hybrid_cloud/notifications/impl.py

@@ -16,9 +16,7 @@ from sentry.notifications.types import (
     NOTIFICATION_SETTING_TYPES,
     NotificationScopeEnum,
     NotificationSettingEnum,
-    NotificationSettingOptionValues,
     NotificationSettingsOptionEnum,
-    NotificationSettingTypes,
 )
 from sentry.services.hybrid_cloud.actor import ActorType, RpcActor
 from sentry.services.hybrid_cloud.auth.model import AuthenticationContext
@@ -35,41 +33,37 @@ from sentry.types.integrations import ExternalProviderEnum, ExternalProviders
 
 
 class DatabaseBackedNotificationsService(NotificationsService):
-    def update_settings(
-        self,
-        *,
-        external_provider: ExternalProviders,
-        notification_type: NotificationSettingTypes,
-        setting_option: NotificationSettingOptionValues,
-        actor: RpcActor,
-        project_id: Optional[int] = None,
-        organization_id: Optional[int] = None,
-        skip_provider_updates: bool = False,
-        organization_id_for_team: Optional[int] = None,
-    ) -> None:
-        NotificationSetting.objects.update_settings(
-            provider=external_provider,
-            type=notification_type,
-            value=setting_option,
-            project=project_id,
-            organization=organization_id,
-            actor=actor,
-            skip_provider_updates=skip_provider_updates,
-        )
-
     def enable_all_settings_for_provider(
         self,
         *,
         external_provider: ExternalProviderEnum,
-        user_id: int,
+        user_id: Optional[int] = None,
+        team_id: Optional[int] = None,
+        types: Optional[List[NotificationSettingEnum]] = None,
     ) -> None:
+        assert (team_id and not user_id) or (
+            user_id and not team_id
+        ), "Can only enable settings for team or user"
+
+        kwargs: MutableMapping[str, str | int] = {}
+        if user_id:
+            kwargs["user_id"] = user_id
+            kwargs["scope_type"] = NotificationScopeEnum.USER.value
+            kwargs["scope_identifier"] = user_id
+        elif team_id:
+            kwargs["team_id"] = team_id
+            kwargs["scope_type"] = NotificationScopeEnum.TEAM.value
+            kwargs["scope_identifier"] = team_id
+
+        type_str_list = list(map(lambda t: t.value, types)) if types else None
         with transaction.atomic(router.db_for_write(NotificationSettingProvider)):
             for type_str in NOTIFICATION_SETTING_TYPES.values():
+                # check the type if it's an input
+                if type_str_list and type_str not in type_str_list:
+                    continue
                 NotificationSettingProvider.objects.create_or_update(
+                    **kwargs,
                     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,

+ 3 - 19
src/sentry/services/hybrid_cloud/notifications/service.py

@@ -8,9 +8,7 @@ from typing import List, Mapping, MutableMapping, Optional, Set, Tuple
 from sentry.notifications.types import (
     NotificationScopeEnum,
     NotificationSettingEnum,
-    NotificationSettingOptionValues,
     NotificationSettingsOptionEnum,
-    NotificationSettingTypes,
 )
 from sentry.services.hybrid_cloud.actor import ActorType, RpcActor
 from sentry.services.hybrid_cloud.auth.model import AuthenticationContext
@@ -35,29 +33,15 @@ class NotificationsService(RpcService):
 
         return DatabaseBackedNotificationsService()
 
-    @rpc_method
-    @abstractmethod
-    def update_settings(
-        self,
-        *,
-        external_provider: ExternalProviders,
-        notification_type: NotificationSettingTypes,
-        setting_option: NotificationSettingOptionValues,
-        actor: RpcActor,
-        project_id: Optional[int] = None,
-        organization_id: Optional[int] = None,
-        skip_provider_updates: bool = False,
-        organization_id_for_team: Optional[int] = None,
-    ) -> None:
-        pass
-
     @rpc_method
     @abstractmethod
     def enable_all_settings_for_provider(
         self,
         *,
         external_provider: ExternalProviderEnum,
-        user_id: int,
+        user_id: Optional[int] = None,
+        team_id: Optional[int] = None,
+        types: Optional[List[NotificationSettingEnum]] = None,
     ) -> None:
         pass
 

+ 8 - 2
tests/sentry/integrations/slack/test_link_team.py

@@ -11,6 +11,7 @@ from sentry.integrations.slack.views.unlink_team import build_team_unlinking_url
 from sentry.models.integrations.external_actor import ExternalActor
 from sentry.models.integrations.organization_integration import OrganizationIntegration
 from sentry.models.notificationsetting import NotificationSetting
+from sentry.models.notificationsettingprovider import NotificationSettingProvider
 from sentry.models.organization import Organization
 from sentry.models.team import Team
 from sentry.notifications.types import NotificationScopeType
@@ -161,8 +162,13 @@ class SlackIntegrationLinkTeamTest(SlackIntegrationLinkTeamTestBase):
         )
 
         with assume_test_silo_mode(SiloMode.CONTROL):
-            team_settings = NotificationSetting.objects.filter(
-                scope_type=NotificationScopeType.TEAM.value, team_id=self.team.id
+            team_settings = NotificationSettingProvider.objects.filter(
+                team_id=self.team.id,
+                provider="slack",
+                type="alerts",
+                scope_type="team",
+                scope_identifier=self.team.id,
+                value="always",
             )
             assert len(team_settings) == 1