ref(notification settings): Read team settings

src/sentry/integrations/slack/views/link_team.py

@@ -155,7 +155,7 @@ def handle(self, request, signed_params):
         allowed_roles = ["admin", "manager", "owner"]
         if not (
             org_member.role in allowed_roles
-            and (organization.flags.allow_joinleave or team in [org_member.teams])
+            and (organization.flags.allow_joinleave or team in org_member.teams.all())
         ):
             return self.send_slack_message(
                 request,
@@ -191,7 +191,6 @@ def handle(self, request, signed_params):
         )
 
         if created:
-            # turn on notifications for all of a team's projects
             NotificationSetting.objects.update_settings(
                 ExternalProviders.SLACK,
                 NotificationSettingTypes.ISSUE_ALERTS,

src/sentry/mail/adapter.py

@@ -152,8 +152,13 @@ def should_notify(self, target_type, group):
         metrics.incr("mail_adapter.should_notify")
         # only notify if we have users to notify. We always want to notify if targeting
         # a member directly.
-        return target_type == ActionTargetType.MEMBER or self.get_sendable_user_objects(
-            group.project
+        return (
+            target_type
+            in [
+                ActionTargetType.MEMBER,
+                ActionTargetType.TEAM,
+            ]
+            or self.get_sendable_user_objects(group.project)
         )
 
     def add_unsubscribe_link(self, context, user_id, project, referrer):

src/sentry/models/groupsubscription.py

@@ -93,9 +93,9 @@ def get_participants(self, group) -> Mapping[ExternalProviders, Mapping[Any, int
         users = User.objects.get_from_group(group)
         user_ids = [user.id for user in users]
         subscriptions = self.filter(group=group, user_id__in=user_ids)
-        notification_settings = NotificationSetting.objects.get_for_users_by_parent(
+        notification_settings = NotificationSetting.objects.get_for_recipient_by_parent(
             NotificationSettingTypes.WORKFLOW,
-            users=users,
+            recipients=users,
             parent=group.project,
         )
         subscriptions_by_user_id = {

src/sentry/notifications/helpers.py

@@ -255,18 +255,19 @@ def get_scope(
     Figure out the scope from parameters and return it as a tuple.
     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:
-        return NotificationScopeType.USER, user.id
-
     if team:
         return NotificationScopeType.TEAM, team.id
 
+    if user:
+        return NotificationScopeType.USER, user.id
+
     raise Exception("scope must be either user, team, organization, or project")
 
 

src/sentry/notifications/manager.py

@@ -1,5 +1,15 @@
 from collections import defaultdict
-from typing import TYPE_CHECKING, Dict, Iterable, List, Mapping, Optional, Sequence, Union
+from typing import (
+    TYPE_CHECKING,
+    Dict,
+    Iterable,
+    List,
+    Mapping,
+    MutableSet,
+    Optional,
+    Sequence,
+    Union,
+)
 
 from django.db import transaction
 from django.db.models import Q, QuerySet
@@ -224,46 +234,49 @@ def get_for_user_by_projects(
             target=user.actor,
         )
 
-    def get_for_users_by_parent(
+    def get_for_recipient_by_parent(
         self,
-        type: NotificationSettingTypes,
+        type_: NotificationSettingTypes,
         parent: Union["Organization", "Project"],
-        users: Sequence["User"],
+        recipients: Sequence[Union["Team", "User"]],
     ) -> QuerySet:
+        from sentry.models import Team, User
+
         """
-        Find all of a project/organization's notification settings for a list of users.
-        This will include each user's project/organization-independent settings.
+        Find all of a project/organization's notification settings for a list of
+        users or teams. Note that this WILL work with a mixed list. This will
+        include each user or team's project/organization-independent settings.
         """
-        scope_type = get_scope_type(type)
+        user_ids: MutableSet[int] = set()
+        team_ids: MutableSet[int] = set()
+        actor_ids: MutableSet[int] = set()
+        for recipient in recipients:
+            if type(recipient) == Team:
+                team_ids.add(recipient.id)
+            if type(recipient) == User:
+                user_ids.add(recipient.id)
+            actor_ids.add(recipient.actor.id)
+
+        # If the list would be empty, don't bother querying.
+        if not actor_ids:
+            return self.none()
+
+        parent_specific_scope_type = get_scope_type(type_)
         return self.filter(
             Q(
-                scope_type=scope_type.value,
+                scope_type=parent_specific_scope_type.value,
                 scope_identifier=parent.id,
             )
             | Q(
                 scope_type=NotificationScopeType.USER.value,
-                scope_identifier__in=[user.id for user in users],
+                scope_identifier__in=user_ids,
+            )
+            | Q(
+                scope_type=NotificationScopeType.TEAM.value,
+                scope_identifier__in=team_ids,
             ),
-            type=type.value,
-            target__in=[user.actor.id for user in users],
-        )
-
-    def get_for_recipient_by_parent(
-        self,
-        type: NotificationSettingTypes,
-        parent: Union["Organization", "Project"],
-        recipient: "User",
-    ) -> QuerySet:
-        """
-        Find all of a project/organization's notification settings for a recipient.
-        This will include each recipient's project/organization-independent settings.
-        """
-        scope_type = get_scope_type(type)
-        return self.filter(
-            scope_type=scope_type.value,
-            scope_identifier=parent.id,
-            type=type.value,
-            target=recipient.actor_id,
+            type=type_.value,
+            target__in=actor_ids,
         )
 
     def filter_to_subscribed_users(
@@ -275,8 +288,8 @@ def filter_to_subscribed_users(
         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.
         """
-        notification_settings = self.get_for_users_by_parent(
-            NotificationSettingTypes.ISSUE_ALERTS, parent=project, users=users
+        notification_settings = self.get_for_recipient_by_parent(
+            NotificationSettingTypes.ISSUE_ALERTS, parent=project, recipients=users
         )
         notification_settings_by_user = transform_to_notification_settings_by_user(
             notification_settings, users

src/sentry/notifications/utils/participants.py

@@ -101,9 +101,9 @@ def get_participants_for_release(
 
     # Get all the involved users' settings for deploy-emails (including
     # users' organization-independent settings.)
-    notification_settings = NotificationSetting.objects.get_for_users_by_parent(
+    notification_settings = NotificationSetting.objects.get_for_recipient_by_parent(
         NotificationSettingTypes.DEPLOY,
-        users=users,
+        recipients=users,
         parent=organization,
     )
     notification_settings_by_user = transform_to_notification_settings_by_user(
@@ -225,10 +225,10 @@ def disabled_users_from_project(project: Project) -> Mapping[ExternalProviders,
     """Get a set of users that have disabled Issue Alert notifications for a given project."""
     user_ids = project.member_set.values_list("user", flat=True)
     users = User.objects.filter(id__in=user_ids)
-    notification_settings = NotificationSetting.objects.get_for_users_by_parent(
+    notification_settings = NotificationSetting.objects.get_for_recipient_by_parent(
         type=NotificationSettingTypes.ISSUE_ALERTS,
         parent=project,
-        users=users,
+        recipients=users,
     )
     notification_settings_by_user = transform_to_notification_settings_by_user(
         notification_settings, users
@@ -267,9 +267,8 @@ def get_send_to_team(
         return {}
 
     team_notification_settings = NotificationSetting.objects.get_for_recipient_by_parent(
-        NotificationSettingTypes.ISSUE_ALERTS, parent=project, recipient=team
+        NotificationSettingTypes.ISSUE_ALERTS, parent=project, recipients=[team]
     )
-
     if team_notification_settings:
         team_mapping = {
             ExternalProviders(notification_setting.provider): {team}
@@ -312,8 +311,8 @@ def get_send_to_member(
         )
     except User.DoesNotExist:
         return {}
-    notification_settings = NotificationSetting.objects.get_for_users_by_parent(
-        NotificationSettingTypes.ISSUE_ALERTS, parent=project, users=[user]
+    notification_settings = NotificationSetting.objects.get_for_recipient_by_parent(
+        NotificationSettingTypes.ISSUE_ALERTS, parent=project, recipients=[user]
     )
     if notification_settings:
         return {

tests/sentry/api/endpoints/test_integrations_slack_commands.py

@@ -10,7 +10,15 @@
 from sentry.integrations.slack.endpoints.command import LINK_USER_MESSAGE
 from sentry.integrations.slack.util.auth import set_signing_secret
 from sentry.integrations.slack.views.link_team import build_linking_url
-from sentry.models import ExternalActor, Identity, IdentityProvider, IdentityStatus, Integration
+from sentry.models import (
+    ExternalActor,
+    Identity,
+    IdentityProvider,
+    IdentityStatus,
+    Integration,
+    NotificationSetting,
+)
+from sentry.notifications.types import NotificationScopeType
 from sentry.testutils import APITestCase, TestCase
 from sentry.types.integrations import ExternalProviders
 from sentry.utils import json
@@ -148,6 +156,11 @@ def test_link_team_command(self):
             in data["text"]
         )
 
+        team_settings = NotificationSetting.objects.filter(
+            scope_type=NotificationScopeType.TEAM.value, target=self.team.actor.id
+        )
+        assert len(team_settings) == 1
+
     def test_link_team_idp_does_not_exist(self):
         """Test that get_identity fails if we cannot find a matching idp"""
         self.get_slack_response(

tests/sentry/integrations/slack/test_notifications.py

@@ -555,7 +555,6 @@ def test_issue_alert_team(self, mock_func):
             ExternalProviders.SLACK,
             NotificationSettingTypes.ISSUE_ALERTS,
             NotificationSettingOptionValues.ALWAYS,
-            project=self.project,
             team=self.team,
         )
 
@@ -594,6 +593,130 @@ def test_issue_alert_team(self, mock_func):
         assert attachments[0]["text"] == ""
         assert attachments[0]["footer"] == event.group.qualified_short_id
 
+    @responses.activate
+    @mock.patch("sentry.notifications.notify.notify", side_effect=send_notification)
+    def test_issue_alert_team_new_project(self, mock_func):
+        """Test that issue alerts are sent to a team in Slack when the team has added a new project"""
+
+        # add a second user to the team so we can be sure it's only
+        # sent once (to the team, and not to each individual user)
+        user2 = self.create_user(is_superuser=False)
+        self.create_member(teams=[self.team], user=user2, organization=self.organization)
+        self.idp = IdentityProvider.objects.create(type="slack", external_id="TXXXXXXX2", config={})
+        self.identity = Identity.objects.create(
+            external_id="UXXXXXXX2",
+            idp=self.idp,
+            user=user2,
+            status=IdentityStatus.VALID,
+            scopes=[],
+        )
+        NotificationSetting.objects.update_settings(
+            ExternalProviders.SLACK,
+            NotificationSettingTypes.ISSUE_ALERTS,
+            NotificationSettingOptionValues.ALWAYS,
+            user=user2,
+        )
+        # update the team's notification settings
+        ExternalActor.objects.create(
+            actor=self.team.actor,
+            organization=self.organization,
+            integration=self.integration,
+            provider=ExternalProviders.SLACK.value,
+            external_name="goma",
+            external_id="CXXXXXXX2",
+        )
+        NotificationSetting.objects.update_settings(
+            ExternalProviders.SLACK,
+            NotificationSettingTypes.ISSUE_ALERTS,
+            NotificationSettingOptionValues.ALWAYS,
+            team=self.team,
+        )
+        # add a new project
+        project2 = self.create_project(
+            name="hellboy", organization=self.organization, teams=[self.team]
+        )
+
+        event = self.store_event(
+            data={"message": "Hello world", "level": "error"}, project_id=project2.id
+        )
+        action_data = {
+            "id": "sentry.mail.actions.NotifyEmailAction",
+            "targetType": "Team",
+            "targetIdentifier": str(self.team.id),
+        }
+        rule = Rule.objects.create(
+            project=project2,
+            label="ja rule",
+            data={
+                "match": "all",
+                "actions": [action_data],
+            },
+        )
+        notification = Notification(event=event, rule=rule)
+
+        with self.options({"system.url-prefix": "http://example.com"}), self.tasks():
+            self.adapter.notify(notification, ActionTargetType.TEAM, self.team.id)
+
+        # check that only one was sent out - more would mean each user is being notified
+        # rather than the team
+        assert len(responses.calls) == 1
+
+        # check that the team got a notification
+        data = parse_qs(responses.calls[0].request.body)
+        assert data["channel"] == ["CXXXXXXX2"]
+        assert "attachments" in data
+        attachments = json.loads(data["attachments"][0])
+        assert len(attachments) == 1
+        assert attachments[0]["title"] == "Hello world"
+        assert attachments[0]["text"] == ""
+        assert attachments[0]["footer"] == event.group.qualified_short_id
+
+    @responses.activate
+    @mock.patch("sentry.notifications.notify.notify", side_effect=send_notification)
+    def test_not_issue_alert_team_removed_project(self, mock_func):
+        """Test that issue alerts are not sent to a team in Slack when the team has removed the project the issue belongs to"""
+
+        # create the team's notification settings
+        ExternalActor.objects.create(
+            actor=self.team.actor,
+            organization=self.organization,
+            integration=self.integration,
+            provider=ExternalProviders.SLACK.value,
+            external_name="goma",
+            external_id="CXXXXXXX2",
+        )
+        NotificationSetting.objects.update_settings(
+            ExternalProviders.SLACK,
+            NotificationSettingTypes.ISSUE_ALERTS,
+            NotificationSettingOptionValues.ALWAYS,
+            team=self.team,
+        )
+        # remove the project from the team
+        self.project.remove_team(self.team)
+
+        event = self.store_event(
+            data={"message": "Hello world", "level": "error"}, project_id=self.project.id
+        )
+        action_data = {
+            "id": "sentry.mail.actions.NotifyEmailAction",
+            "targetType": "Team",
+            "targetIdentifier": str(self.team.id),
+        }
+        rule = Rule.objects.create(
+            project=self.project,
+            label="ja rule",
+            data={
+                "match": "all",
+                "actions": [action_data],
+            },
+        )
+        notification = Notification(event=event, rule=rule)
+
+        with self.options({"system.url-prefix": "http://example.com"}), self.tasks():
+            self.adapter.notify(notification, ActionTargetType.TEAM, self.team.id)
+
+        assert len(responses.calls) == 0
+
     @responses.activate
     @mock.patch("sentry.notifications.notify.notify", side_effect=send_notification)
     def test_issue_alert_team_fallback(self, mock_func):