feat(notifications): Allow TEAM scope (#26756)

src/sentry/notifications/helpers.py

@@ -1,5 +1,17 @@
 from collections import defaultdict
-from typing import Any, Dict, Iterable, List, Mapping, MutableMapping, Optional, Set, Tuple, Union
+from typing import (
+    TYPE_CHECKING,
+    Any,
+    Dict,
+    Iterable,
+    List,
+    Mapping,
+    MutableMapping,
+    Optional,
+    Set,
+    Tuple,
+    Union,
+)
 
 from sentry.notifications.types import (
     NOTIFICATION_SCOPE_TYPE,
@@ -13,6 +25,18 @@ from sentry.notifications.types import (
 )
 from sentry.types.integrations import EXTERNAL_PROVIDERS, ExternalProviders
 
+if TYPE_CHECKING:
+    from sentry.models import (
+        Group,
+        GroupSubscription,
+        NotificationSetting,
+        Organization,
+        Project,
+        Team,
+        User,
+    )
+
+
 # This mapping represents how to interpret the absence of a DB row for a given
 # provider. For example, a user with no NotificationSettings should be opted
 # into receiving emails but no Slack messages.
@@ -32,10 +56,10 @@ NOTIFICATION_SETTING_DEFAULTS = {
 
 def _get_setting_mapping_from_mapping(
     notification_settings_by_user: Mapping[
-        Any,
+        "User",
         Mapping[NotificationScopeType, Mapping[ExternalProviders, NotificationSettingOptionValues]],
     ],
-    user: Any,
+    user: "User",
     type: NotificationSettingTypes,
 ) -> Mapping[ExternalProviders, NotificationSettingOptionValues]:
     # XXX(CEO): may not respect granularity of a setting for Slack a setting for email
@@ -57,10 +81,10 @@ def _get_setting_mapping_from_mapping(
 
 def where_should_user_be_notified(
     notification_settings_by_user: Mapping[
-        Any,
+        "User",
         Mapping[NotificationScopeType, Mapping[ExternalProviders, NotificationSettingOptionValues]],
     ],
-    user: Any,
+    user: "User",
 ) -> List[ExternalProviders]:
     """
     Given a mapping of default and specific notification settings by user,
@@ -79,8 +103,8 @@ def where_should_user_be_notified(
 
 
 def should_be_participating(
-    subscriptions_by_user_id: Mapping[int, Any],
-    user: Any,
+    subscriptions_by_user_id: Mapping[int, "GroupSubscription"],
+    user: "User",
     value: NotificationSettingOptionValues,
 ) -> bool:
     subscription = subscriptions_by_user_id.get(user.id)
@@ -90,10 +114,10 @@ def should_be_participating(
 
 
 def where_should_be_participating(
-    user: Any,
-    subscriptions_by_user_id: Mapping[int, Any],
+    user: "User",
+    subscriptions_by_user_id: Mapping[int, "GroupSubscription"],
     notification_settings_by_user: Mapping[
-        Any,
+        "User",
         Mapping[NotificationScopeType, Mapping[ExternalProviders, NotificationSettingOptionValues]],
     ],
 ) -> List[ExternalProviders]:
@@ -146,10 +170,11 @@ def get_values_by_provider_by_type(
 
 
 def transform_to_notification_settings_by_user(
-    notification_settings: Iterable[Any],
-    users: Iterable[Any],
+    notification_settings: Iterable["NotificationSetting"],
+    users: Iterable["User"],
 ) -> Mapping[
-    Any, Mapping[NotificationScopeType, Mapping[ExternalProviders, NotificationSettingOptionValues]]
+    "User",
+    Mapping[NotificationScopeType, Mapping[ExternalProviders, NotificationSettingOptionValues]],
 ]:
     """
     Given a unorganized list of notification settings, create a mapping of
@@ -157,7 +182,8 @@ def transform_to_notification_settings_by_user(
     """
     actor_mapping = {user.actor_id: user for user in users}
     notification_settings_by_user: Dict[
-        Any, Dict[NotificationScopeType, Dict[ExternalProviders, NotificationSettingOptionValues]]
+        "User",
+        Dict[NotificationScopeType, Dict[ExternalProviders, NotificationSettingOptionValues]],
     ] = defaultdict(lambda: defaultdict(dict))
     for notification_setting in notification_settings:
         user = actor_mapping.get(notification_setting.target_id)
@@ -169,7 +195,7 @@ def transform_to_notification_settings_by_user(
 
 
 def transform_to_notification_settings_by_parent_id(
-    notification_settings: Iterable[Any],
+    notification_settings: Iterable["NotificationSetting"],
     user_default: Optional[NotificationSettingOptionValues] = None,
 ) -> Tuple[
     Mapping[ExternalProviders, Mapping[int, NotificationSettingOptionValues]],
@@ -220,26 +246,31 @@ def get_scope_type(type: NotificationSettingTypes) -> NotificationScopeType:
 
 
 def get_scope(
-    user_id: int, project: Optional[Any] = None, organization: Optional[Any] = None
+    user: Optional["User"] = None,
+    team: Optional["Team"] = None,
+    project: Optional["Project"] = None,
+    organization: Optional["Organization"] = None,
 ) -> Tuple[NotificationScopeType, int]:
     """
     Figure out the scope from parameters and return it as a tuple.
-    TODO(mgaeta): Make sure user_id is in the project or organization.
+    TODO(mgaeta): Make sure the user/team is in the project/organization.
     """
-
     if project:
         return NotificationScopeType.PROJECT, project.id
 
     if organization:
         return NotificationScopeType.ORGANIZATION, organization.id
 
-    if user_id:
-        return NotificationScopeType.USER, user_id
+    if user:
+        return NotificationScopeType.USER, user.id
+
+    if team:
+        return NotificationScopeType.TEAM, team.id
 
-    raise Exception("scope must be either user, organization, or project")
+    raise Exception("scope must be either user, team, organization, or project")
 
 
-def get_target_id(user: Optional[Any] = None, team: Optional[Any] = None) -> int:
+def get_target_id(user: Optional["User"] = None, team: Optional["Team"] = None) -> int:
     """:returns the actor ID from a User or Team."""
     if user:
         return int(user.actor_id)
@@ -265,10 +296,10 @@ def get_subscription_from_attributes(
 
 
 def get_groups_for_query(
-    groups_by_project: Mapping[Any, Set[Any]],
+    groups_by_project: Mapping["Project", Set["Group"]],
     notification_settings_by_key: Mapping[int, NotificationSettingOptionValues],
     global_default_workflow_option: NotificationSettingOptionValues,
-) -> Set[Any]:
+) -> Set["Group"]:
     """
     If there is a subscription record associated with the group, we can just use
     that to know if a user is subscribed or not, as long as notifications aren't
@@ -283,7 +314,7 @@ def get_groups_for_query(
     return output
 
 
-def collect_groups_by_project(groups: Iterable[Any]) -> Mapping[Any, Set[Any]]:
+def collect_groups_by_project(groups: Iterable["Group"]) -> Mapping["Project", Set["Group"]]:
     """
     Collect all of the projects to look up, and keep a set of groups that are
     part of that project. (Note that the common -- but not only -- case here is
@@ -296,11 +327,11 @@ def collect_groups_by_project(groups: Iterable[Any]) -> Mapping[Any, Set[Any]]:
 
 
 def get_user_subscriptions_for_groups(
-    groups_by_project: Mapping[Any, Set[Any]],
+    groups_by_project: Mapping["Project", Set["Group"]],
     notification_settings_by_key: Mapping[int, NotificationSettingOptionValues],
-    subscriptions_by_group_id: Mapping[int, Any],
+    subscriptions_by_group_id: Mapping[int, "GroupSubscription"],
     global_default_workflow_option: NotificationSettingOptionValues,
-) -> Mapping[int, Tuple[bool, bool, Optional[Any]]]:
+) -> Mapping[int, Tuple[bool, bool, Optional["GroupSubscription"]]]:
     """
     Takes collected data and returns a mapping of group IDs to a two-tuple of
     (subscribed: bool, subscription: Optional[GroupSubscription]).
@@ -351,7 +382,7 @@ def get_fallback_settings(
     types_to_serialize: Iterable[NotificationSettingTypes],
     project_ids: Iterable[int],
     organization_ids: Iterable[int],
-    user: Optional[Any] = None,
+    user: Optional["User"] = None,
 ) -> MutableMapping[str, MutableMapping[str, MutableMapping[int, MutableMapping[str, str]]]]:
     """
     The API is responsible for calculating the implied setting values when a

src/sentry/notifications/manager.py

@@ -1,5 +1,5 @@
 from collections import defaultdict
-from typing import Any, Dict, Iterable, List, Mapping, Optional, Union
+from typing import TYPE_CHECKING, Dict, Iterable, List, Mapping, Optional, Sequence, Union
 
 from django.db import transaction
 from django.db.models import Q, QuerySet
@@ -20,6 +20,9 @@ from sentry.notifications.types import (
 )
 from sentry.types.integrations import ExternalProviders
 
+if TYPE_CHECKING:
+    from sentry.models import NotificationSetting, Organization, Project, Team, User
+
 
 class NotificationsManager(BaseManager):  # type: ignore
     """
@@ -30,10 +33,10 @@ class NotificationsManager(BaseManager):  # type: ignore
         self,
         provider: ExternalProviders,
         type: NotificationSettingTypes,
-        user: Optional[Any] = None,
-        team: Optional[Any] = None,
-        project: Optional[Any] = None,
-        organization: Optional[Any] = None,
+        user: Optional["User"] = None,
+        team: Optional["Team"] = None,
+        project: Optional["Project"] = None,
+        organization: Optional["Organization"] = None,
     ) -> NotificationSettingOptionValues:
         """
         One and only one of (user, team, project, or organization)
@@ -50,15 +53,37 @@ class NotificationsManager(BaseManager):  # type: ignore
             else NotificationSettingOptionValues.DEFAULT
         )
 
+    def _update_settings(
+        self,
+        provider: ExternalProviders,
+        type: NotificationSettingTypes,
+        value: NotificationSettingOptionValues,
+        scope_type: NotificationScopeType,
+        scope_identifier: int,
+        target_id: int,
+    ) -> None:
+        """Save a NotificationSettings row."""
+        with transaction.atomic():
+            setting, created = self.get_or_create(
+                provider=provider.value,
+                type=type.value,
+                scope_type=scope_type.value,
+                scope_identifier=scope_identifier,
+                target_id=target_id,
+                defaults={"value": value.value},
+            )
+            if not created and setting.value != value.value:
+                setting.update(value=value.value)
+
     def update_settings(
         self,
         provider: ExternalProviders,
         type: NotificationSettingTypes,
         value: NotificationSettingOptionValues,
-        user: Optional[Any] = None,
-        team: Optional[Any] = None,
-        project: Optional[Any] = None,
-        organization: Optional[Any] = None,
+        user: Optional["User"] = None,
+        team: Optional["Team"] = None,
+        project: Optional["Project"] = None,
+        organization: Optional["Organization"] = None,
     ) -> None:
         """
         Save a target's notification preferences.
@@ -81,32 +106,19 @@ class NotificationsManager(BaseManager):  # type: ignore
         if not validate(type, value):
             raise Exception(f"value '{value}' is not valid for type '{type}'")
 
-        user_id_option = getattr(user, "id", None)
-        scope_type, scope_identifier = get_scope(
-            user_id_option, project=project, organization=organization
-        )
+        scope_type, scope_identifier = get_scope(user, team, project, organization)
         target_id = get_target_id(user, team)
 
-        with transaction.atomic():
-            setting, created = self.get_or_create(
-                provider=provider.value,
-                type=type.value,
-                scope_type=scope_type.value,
-                scope_identifier=scope_identifier,
-                target_id=target_id,
-                defaults={"value": value.value},
-            )
-            if not created and setting.value != value.value:
-                setting.update(value=value.value)
+        self._update_settings(provider, type, value, scope_type, scope_identifier, target_id)
 
     def remove_settings(
         self,
         provider: ExternalProviders,
         type: NotificationSettingTypes,
-        user: Optional[Any] = None,
-        team: Optional[Any] = None,
-        project: Optional[Any] = None,
-        organization: Optional[Any] = None,
+        user: Optional["User"] = None,
+        team: Optional["Team"] = None,
+        project: Optional["Project"] = None,
+        organization: Optional["Organization"] = None,
     ) -> None:
         """
         We don't anticipate this function will be used by the API but is useful
@@ -142,16 +154,20 @@ class NotificationsManager(BaseManager):  # type: ignore
 
         return self.filter(**filters)
 
-    def remove_for_user(self, user: Any, type: Optional[NotificationSettingTypes] = None) -> None:
+    def remove_for_user(
+        self, user: "User", type: Optional[NotificationSettingTypes] = None
+    ) -> None:
         """Bulk delete all Notification Settings for a USER, optionally by type."""
         self._filter(target_ids=[user.actor_id], type=type).delete()
 
-    def remove_for_team(self, team: Any, type: Optional[NotificationSettingTypes] = None) -> None:
+    def remove_for_team(
+        self, team: "Team", type: Optional[NotificationSettingTypes] = None
+    ) -> None:
         """Bulk delete all Notification Settings for a TEAM, optionally by type."""
         self._filter(target_ids=[team.actor_id], type=type).delete()
 
     def remove_for_project(
-        self, project: Any, type: Optional[NotificationSettingTypes] = None
+        self, project: "Project", type: Optional[NotificationSettingTypes] = None
     ) -> None:
         """Bulk delete all Notification Settings for a PROJECT, optionally by type."""
         self._filter(
@@ -161,7 +177,7 @@ class NotificationsManager(BaseManager):  # type: ignore
         ).delete()
 
     def remove_for_organization(
-        self, organization: Any, type: Optional[NotificationSettingTypes] = None
+        self, organization: "Organization", type: Optional[NotificationSettingTypes] = None
     ) -> None:
         """Bulk delete all Notification Settings for an ENTIRE ORGANIZATION, optionally by type."""
         self._filter(
@@ -174,24 +190,21 @@ class NotificationsManager(BaseManager):  # type: ignore
         self,
         provider: ExternalProviders,
         type: NotificationSettingTypes,
-        user: Optional[Any] = None,
-        team: Optional[Any] = None,
-        project: Optional[Any] = None,
-        organization: Optional[Any] = None,
+        user: Optional["User"] = None,
+        team: Optional["Team"] = None,
+        project: Optional["Project"] = None,
+        organization: Optional["Organization"] = None,
     ) -> QuerySet:
         """Wrapper for .filter that translates object parameters to scopes and targets."""
-        user_id_option = getattr(user, "id", None)
-        scope_type, scope_identifier = get_scope(
-            user_id_option, project=project, organization=organization
-        )
+        scope_type, scope_identifier = get_scope(user, team, project, organization)
         target_id = get_target_id(user, team)
         return self._filter(provider, type, scope_type, scope_identifier, [target_id])
 
     def get_for_user_by_projects(
         self,
         type: NotificationSettingTypes,
-        user: Any,
-        parents: List[Any],
+        user: "User",
+        parents: List[Union["Organization", "Project"]],
     ) -> QuerySet:
         """
         Find all of a user's notification settings for a list of projects or
@@ -214,8 +227,8 @@ class NotificationsManager(BaseManager):  # type: ignore
     def get_for_users_by_parent(
         self,
         type: NotificationSettingTypes,
-        parent: Any,
-        users: List[Any],
+        parent: Union["Organization", "Project"],
+        users: Sequence["User"],
     ) -> QuerySet:
         """
         Find all of a project/organization's notification settings for a list of users.
@@ -238,8 +251,8 @@ class NotificationsManager(BaseManager):  # type: ignore
     def get_for_recipient_by_parent(
         self,
         type: NotificationSettingTypes,
-        parent: Any,
-        recipient: Any,
+        parent: Union["Organization", "Project"],
+        recipient: "User",
     ) -> QuerySet:
         """
         Find all of a project/organization's notification settings for a recipient.
@@ -255,9 +268,9 @@ class NotificationsManager(BaseManager):  # type: ignore
 
     def filter_to_subscribed_users(
         self,
-        project: Any,
-        users: List[Any],
-    ) -> Mapping[ExternalProviders, Iterable[Any]]:
+        project: "Project",
+        users: List["User"],
+    ) -> Mapping[ExternalProviders, Iterable["User"]]:
         """
         Filters a list of users down to the users by provider who are subscribed to alerts.
         We check both the project level settings and global default settings.
@@ -276,8 +289,8 @@ class NotificationsManager(BaseManager):  # type: ignore
         return mapping
 
     def get_notification_recipients(
-        self, project: Any
-    ) -> Mapping[ExternalProviders, Iterable[Any]]:
+        self, project: "Project"
+    ) -> Mapping[ExternalProviders, Iterable["User"]]:
         """
         Return a set of users that should receive Issue Alert emails for a given
         project. To start, we get the set of all users. Then we fetch all of
@@ -293,7 +306,7 @@ class NotificationsManager(BaseManager):  # type: ignore
 
     def update_settings_bulk(
         self,
-        notification_settings: Iterable[Any],
+        notification_settings: Sequence["NotificationSetting"],
         target_id: int,
     ) -> None:
         """
@@ -306,14 +319,6 @@ class NotificationsManager(BaseManager):  # type: ignore
             if value == NotificationSettingOptionValues.DEFAULT:
                 self._filter(provider, type, scope_type, scope_identifier, [target_id]).delete()
             else:
-                with transaction.atomic():
-                    setting, created = self.get_or_create(
-                        provider=provider.value,
-                        type=type.value,
-                        scope_type=scope_type.value,
-                        scope_identifier=scope_identifier,
-                        target_id=target_id,
-                        defaults={"value": value.value},
-                    )
-                    if not created and setting.value != value.value:
-                        setting.update(value=value.value)
+                self._update_settings(
+                    provider, type, value, scope_type, scope_identifier, target_id
+                )

tests/sentry/notifications/test_utils.py

@@ -4,7 +4,6 @@ from sentry.notifications.helpers import (
     collect_groups_by_project,
     get_fallback_settings,
     get_groups_for_query,
-    get_scope,
     get_scope_type,
     get_settings_by_provider,
     get_subscription_from_attributes,
@@ -30,7 +29,7 @@ from sentry.types.integrations import ExternalProviders
 
 class NotificationHelpersTest(TestCase):
     def setUp(self):
-        super(TestCase, self).setUp()
+        super().setUp()
 
         NotificationSetting.objects.update_settings(
             ExternalProviders.SLACK,
@@ -277,23 +276,6 @@ class NotificationHelpersTest(TestCase):
             == NotificationScopeType.ORGANIZATION
         )
 
-    def test_get_scope(self):
-        scope_type, scope_identifier = get_scope(self.user.id, project=None, organization=None)
-        assert scope_type == NotificationScopeType.USER
-        assert scope_identifier == self.user.id
-
-        scope_type, scope_identifier = get_scope(
-            self.user.id, project=self.project, organization=None
-        )
-        assert scope_type == NotificationScopeType.PROJECT
-        assert scope_identifier == self.project.id
-
-        scope_type, scope_identifier = get_scope(
-            self.user.id, project=None, organization=self.organization
-        )
-        assert scope_type == NotificationScopeType.ORGANIZATION
-        assert scope_identifier == self.organization.id
-
     def test_get_target_id(self):
         assert get_target_id(self.user) == self.user.actor_id
         assert get_target_id(self.team) == self.team.actor_id

tests/sentry/notifications/utils/test_get_scope.py

@@ -0,0 +1,33 @@
+from unittest import TestCase
+
+from sentry.models import Organization, Project, Team, User
+from sentry.notifications.helpers import get_scope
+from sentry.notifications.types import NotificationScopeType
+
+
+class GetScopeTestCase(TestCase):
+    def setUp(self) -> None:
+        self.user = User(id=1)
+
+    def test_get_scope_user(self):
+        scope_type, scope_identifier = get_scope(self.user)
+        assert scope_type == NotificationScopeType.USER
+        assert scope_identifier == self.user.id
+
+    def test_get_scope_team(self):
+        team = Team(id=1)
+        scope_type, scope_identifier = get_scope(team=team)
+        assert scope_type == NotificationScopeType.TEAM
+        assert scope_identifier == team.id
+
+    def test_get_scope_project(self):
+        project = Project(id=1)
+        scope_type, scope_identifier = get_scope(self.user, project=project)
+        assert scope_type == NotificationScopeType.PROJECT
+        assert scope_identifier == project.id
+
+    def test_get_scope_organization(self):
+        organization = Organization(id=1)
+        scope_type, scope_identifier = get_scope(self.user, organization=organization)
+        assert scope_type == NotificationScopeType.ORGANIZATION
+        assert scope_identifier == organization.id