Skip to content

Commit

Permalink
feat(team-workflow): Add subscription for team in GroupSubscription (
Browse files Browse the repository at this point in the history
…#55825)

Updates `subscribe_actor` to no longer read all the team's users and
instead subscribe the Team itself if the team-workflow-notifications
feature flag is enabled. Also updates `subscribe` so that it can accept
a Team as a subscriber, not just Users or RpcUsers.

---------

Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com>
  • Loading branch information
isabellaenriquez and getsantry[bot] committed Sep 12, 2023
1 parent 930abe1 commit e69b158
Show file tree
Hide file tree
Showing 6 changed files with 60 additions and 19 deletions.
2 changes: 1 addition & 1 deletion src/sentry/api/endpoints/group_notes.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ def post(self, request: Request, group) -> Response:
)

GroupSubscription.objects.subscribe(
group=group, user=request.user, reason=GroupSubscriptionReason.comment
group=group, subscriber=request.user, reason=GroupSubscriptionReason.comment
)

mentioned_users = extract_user_ids_from_mentions(group.organization.id, mentions)
Expand Down
4 changes: 2 additions & 2 deletions src/sentry/api/helpers/group_index/update.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ def self_subscribe_and_assign_issue(
# representation of current user
if acting_user:
GroupSubscription.objects.subscribe(
user=acting_user, group=group, reason=GroupSubscriptionReason.status_change
subscriber=acting_user, group=group, reason=GroupSubscriptionReason.status_change
)

if self_assign_issue == "1" and not group.assignee_set.exists():
Expand Down Expand Up @@ -736,7 +736,7 @@ def handle_is_bookmarked(
user_id=acting_user.id if acting_user else None,
)
GroupSubscription.objects.subscribe(
user=acting_user, group=group, reason=GroupSubscriptionReason.bookmark
subscriber=acting_user, group=group, reason=GroupSubscriptionReason.bookmark
)
elif is_bookmarked is False:
GroupBookmark.objects.filter(
Expand Down
2 changes: 1 addition & 1 deletion src/sentry/issues/status_change.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ def handle_status_update(
if not is_bulk:
if acting_user:
GroupSubscription.objects.subscribe(
user=acting_user,
subscriber=acting_user,
group=group,
reason=GroupSubscriptionReason.status_change,
)
Expand Down
39 changes: 27 additions & 12 deletions src/sentry/models/groupsubscription.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,22 +34,33 @@ class GroupSubscriptionManager(BaseManager):
def subscribe(
self,
group: Group,
user: User | RpcUser,
subscriber: User | RpcUser | Team,
reason: int = GroupSubscriptionReason.unknown,
) -> bool:
"""
Subscribe a user to an issue, but only if the user has not explicitly
Subscribe a user or team to an issue, but only if that user or team has not explicitly
unsubscribed.
"""
from sentry.models import Team, User

try:
with transaction.atomic(router.db_for_write(GroupSubscription)):
self.create(
user_id=user.id,
group=group,
project=group.project,
is_active=True,
reason=reason,
)
if isinstance(subscriber, User) or isinstance(subscriber, RpcUser):
self.create(
user_id=subscriber.id,
group=group,
project=group.project,
is_active=True,
reason=reason,
)
elif isinstance(subscriber, Team):
self.create(
team=subscriber,
group=group,
project=group.project,
is_active=True,
reason=reason,
)
except IntegrityError:
pass
return True
Expand All @@ -60,14 +71,18 @@ def subscribe_actor(
actor: Union[Team, User, RpcUser],
reason: int = GroupSubscriptionReason.unknown,
) -> Optional[bool]:
from sentry import features
from sentry.models import Team, User

if isinstance(actor, RpcUser) or isinstance(actor, User):
return self.subscribe(group, actor, reason)
if isinstance(actor, Team):
# subscribe the members of the team
team_users_ids = list(actor.member_set.values_list("user_id", flat=True))
return self.bulk_subscribe(group, team_users_ids, reason)
if features.has("organizations:team-workflow-notifications", group.organization, actor):
return self.subscribe(group, actor, reason)
else:
# subscribe the members of the team
team_users_ids = list(actor.member_set.values_list("user_id", flat=True))
return self.bulk_subscribe(group, team_users_ids, reason)

raise NotImplementedError("Unknown actor type: %r" % type(actor))

Expand Down
2 changes: 1 addition & 1 deletion src/sentry/receivers/releases.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ def resolved_in_commit(instance, created, **kwargs):
# subscribe every user
for user in user_list:
GroupSubscription.objects.subscribe(
user=user,
subscriber=user,
group=group,
reason=GroupSubscriptionReason.status_change,
)
Expand Down
30 changes: 28 additions & 2 deletions tests/sentry/models/test_groupsubscription.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
from sentry.services.hybrid_cloud.user.service import user_service
from sentry.silo import SiloMode
from sentry.testutils.cases import TestCase
from sentry.testutils.helpers.features import with_feature
from sentry.testutils.silo import assume_test_silo_mode, region_silo_test
from sentry.types.integrations import ExternalProviders

Expand All @@ -21,13 +22,21 @@ class SubscribeTest(TestCase):
def test_simple(self):
group = self.create_group()
user = self.create_user()
team = self.create_team()

GroupSubscription.objects.subscribe(group=group, user=user)
GroupSubscription.objects.subscribe(group=group, subscriber=user)

assert GroupSubscription.objects.filter(group=group, user_id=user.id).exists()

# should not error
GroupSubscription.objects.subscribe(group=group, user=user)
GroupSubscription.objects.subscribe(group=group, subscriber=user)

GroupSubscription.objects.subscribe(group=group, subscriber=team)

assert GroupSubscription.objects.filter(group=group, team=team).exists()

# should not error
GroupSubscription.objects.subscribe(group=group, subscriber=team)

def test_bulk(self):
group = self.create_group()
Expand Down Expand Up @@ -88,6 +97,23 @@ def test_actor_team(self):
# should not error
GroupSubscription.objects.subscribe_actor(group=group, actor=team)

@with_feature("organizations:team-workflow-notifications")
def test_subscribe_team(self):
org = self.create_organization()
group = self.create_group()
user = self.create_user(email="foo@example.com")
team = self.create_team(organization=org)
self.create_member(user=user, organization=org, role="owner", teams=[team])

GroupSubscription.objects.subscribe_actor(group=group, actor=team)

assert not GroupSubscription.objects.filter(group=group, user_id=user.id).exists()

assert GroupSubscription.objects.filter(group=group, team=team).exists()

# should not error
GroupSubscription.objects.subscribe_actor(group=group, actor=team)


@region_silo_test(stable=True)
class GetParticipantsTest(TestCase):
Expand Down

0 comments on commit e69b158

Please sign in to comment.