diff --git a/src/sentry/models/groupsubscription.py b/src/sentry/models/groupsubscription.py index 6e80981fdedf93..c06acf63b08f61 100644 --- a/src/sentry/models/groupsubscription.py +++ b/src/sentry/models/groupsubscription.py @@ -97,6 +97,8 @@ def bulk_subscribe( Subscribe a list of user ids and/or teams to an issue, but only if the users/teams are not explicitly unsubscribed. """ + from sentry import features + # Unique the IDs. user_ids = set(user_ids) if user_ids else set() @@ -124,24 +126,25 @@ def bulk_subscribe( for user_id in user_ids.difference(existing_subscriptions) ] - existing_team_subscriptions = set( - GroupSubscription.objects.filter( - team_id__in=team_ids, group=group, project=group.project - ).values_list("team_id", flat=True) - ) + if features.has("organizations:team-workflow-notifications", group.organization): + existing_team_subscriptions = set( + GroupSubscription.objects.filter( + team_id__in=team_ids, group=group, project=group.project + ).values_list("team_id", flat=True) + ) - subscriptions.extend( - [ - GroupSubscription( - team_id=team_id, - group=group, - project=group.project, - is_active=True, - reason=reason, - ) - for team_id in team_ids.difference(existing_team_subscriptions) - ] - ) + subscriptions.extend( + [ + GroupSubscription( + team_id=team_id, + group=group, + project=group.project, + is_active=True, + reason=reason, + ) + for team_id in team_ids.difference(existing_team_subscriptions) + ] + ) try: with transaction.atomic(router.db_for_write(GroupSubscription)): diff --git a/tests/sentry/models/test_groupsubscription.py b/tests/sentry/models/test_groupsubscription.py index 9530b9420ff231..b1c44682155705 100644 --- a/tests/sentry/models/test_groupsubscription.py +++ b/tests/sentry/models/test_groupsubscription.py @@ -42,26 +42,21 @@ def test_bulk(self): group = self.create_group() user_ids = [] - team_ids = [] for _ in range(20): user = self.create_user() user_ids.append(user.id) - team = self.create_team() - team_ids.append(team.id) - GroupSubscription.objects.bulk_subscribe(group=group, user_ids=user_ids, team_ids=team_ids) + GroupSubscription.objects.bulk_subscribe(group=group, user_ids=user_ids) - assert len(GroupSubscription.objects.filter(group=group)) == 40 + assert len(GroupSubscription.objects.filter(group=group)) == 20 one_more = self.create_user() user_ids.append(one_more.id) - team_ids.append(self.create_team().id) - # should not error - GroupSubscription.objects.bulk_subscribe(group=group, user_ids=user_ids, team_ids=team_ids) + GroupSubscription.objects.bulk_subscribe(group=group, user_ids=user_ids) - assert len(GroupSubscription.objects.filter(group=group)) == 42 + assert len(GroupSubscription.objects.filter(group=group)) == 21 def test_bulk_dupes(self): group = self.create_group() @@ -72,16 +67,63 @@ def test_bulk_dupes(self): user_ids.append(user.id) user_ids.append(user.id) + GroupSubscription.objects.bulk_subscribe(group=group, user_ids=user_ids) + + assert len(GroupSubscription.objects.filter(group=group)) == 1 + + @with_feature("organizations:team-workflow-notifications") + def test_bulk_teams(self): + group = self.create_group() + + team_ids = [] + for _ in range(20): + team = self.create_team() + team_ids.append(team.id) + + GroupSubscription.objects.bulk_subscribe(group=group, team_ids=team_ids) + + assert len(GroupSubscription.objects.filter(group=group)) == 20 + + one_more = self.create_team() + team_ids.append(one_more.id) + + # should not error + GroupSubscription.objects.bulk_subscribe(group=group, team_ids=team_ids) + + assert len(GroupSubscription.objects.filter(group=group)) == 21 + + @with_feature("organizations:team-workflow-notifications") + def test_bulk_teams_dupes(self): + group = self.create_group() + team_ids = [] team = self.create_team() team_ids.append(team.id) team_ids.append(team.id) + GroupSubscription.objects.bulk_subscribe(group=group, team_ids=team_ids) + + assert len(GroupSubscription.objects.filter(group=group)) == 1 + + @with_feature("organizations:team-workflow-notifications") + def test_bulk_users_and_teams(self): + group = self.create_group() + + user_ids = [] + team_ids = [] + + for _ in range(10): + user = self.create_user() + user_ids.append(user.id) + team = self.create_team() + team_ids.append(team.id) + GroupSubscription.objects.bulk_subscribe(group=group, user_ids=user_ids, team_ids=team_ids) - assert len(GroupSubscription.objects.filter(group=group)) == 2 + assert len(GroupSubscription.objects.filter(group=group)) == 20 + @with_feature("organizations:team-workflow-notifications") def test_bulk_user_on_team(self): """ Test that ensures bulk_subscribe subscribes users and teams individually, even if one of those users is part of one of those teams. @@ -98,23 +140,6 @@ def test_bulk_user_on_team(self): assert len(GroupSubscription.objects.filter(group=group)) == 2 - def test_bulk_users_xor_team(self): - """ - Test that ensures bulk_subscribe can be called with just a list of user IDs or just a list of teams; it does not need both. - """ - group = self.create_group() - user = self.create_user() - team = self.create_team() - - user_ids = [user.id] - team_ids = [team.id] - - # should not error - GroupSubscription.objects.bulk_subscribe(group=group, user_ids=user_ids) - - # should not error - GroupSubscription.objects.bulk_subscribe(group=group, team_ids=team_ids) - def test_actor_user(self): group = self.create_group() user = self.create_user()