Repository: getsentry/sentry
Pull request: feat(participants): Remove some comment participants
When a user and/or team are mentioned in a comment, they will no longer be subscribed to the issue. Also, if a user deletes their only comment on an issue, they will be unsubscribed from the issue (unless they were initially subscribed for some other reason like being assigned, bookmarking, explicitly subscribing, etc). 

Closes #57720 

Problem statement:
[BE] Unsubscribe commenters/mentioned users and teams
Users and teams who are mentioned in a comment should no longer be automatically subscribed to be a participant.

When a user deletes their own comment and their subscription reason was commenting, they should be unsubscribed.

Hints from issue discussion:

Commit message: add reason to filter

Patch to review:
diff --git a/src/sentry/api/endpoints/group_notes.py b/src/sentry/api/endpoints/group_notes.py
index bdbe9887a1096f..8dfb3e79f29eba 100644
--- a/src/sentry/api/endpoints/group_notes.py
+++ b/src/sentry/api/endpoints/group_notes.py
@@ -70,24 +70,29 @@ def post(self, request: Request, group) -> Response:
         GroupSubscription.objects.subscribe(
             group=group, subscriber=request.user, reason=GroupSubscriptionReason.comment
         )
-
-        mentioned_users = extract_user_ids_from_mentions(group.organization.id, mentions)
-        GroupSubscription.objects.bulk_subscribe(
-            group=group, user_ids=mentioned_users["users"], reason=GroupSubscriptionReason.mentioned
-        )
-
-        if features.has("organizations:team-workflow-notifications", group.organization):
+        if not features.has("organizations:participants-purge", group.organization):
+            mentioned_users = extract_user_ids_from_mentions(group.organization.id, mentions)
             GroupSubscription.objects.bulk_subscribe(
                 group=group,
-                team_ids=mentioned_users["teams"],
-                reason=GroupSubscriptionReason.team_mentioned,
+                user_ids=mentioned_users["users"],
+                reason=GroupSubscriptionReason.mentioned,
             )
-        else:
+
+        if features.has(
+            "organizations:team-workflow-notifications", group.organization
+        ) and not features.has("organizations:participants-purge", group.organization):
             GroupSubscription.objects.bulk_subscribe(
                 group=group,
-                user_ids=mentioned_users["team_users"],
+                team_ids=mentioned_users["teams"],
                 reason=GroupSubscriptionReason.team_mentioned,
             )
+        else:
+            if not features.has("organizations:participants-purge", group.organization):
+                GroupSubscription.objects.bulk_subscribe(
+                    group=group,
+                    user_ids=mentioned_users["team_users"],
+                    reason=GroupSubscriptionReason.team_mentioned,
+                )
 
         activity = Activity.objects.create_group_activity(
             group, ActivityType.NOTE, user_id=request.user.id, data=data
diff --git a/src/sentry/api/endpoints/group_notes_details.py b/src/sentry/api/endpoints/group_notes_details.py
index a8fe1f22b69b15..ee1f8e1b5f2a9c 100644
--- a/src/sentry/api/endpoints/group_notes_details.py
+++ b/src/sentry/api/endpoints/group_notes_details.py
@@ -3,6 +3,7 @@
 from rest_framework.request import Request
 from rest_framework.response import Response
 
+from sentry import features
 from sentry.api.api_publish_status import ApiPublishStatus
 from sentry.api.base import region_silo_endpoint
 from sentry.api.bases.group import GroupEndpoint
@@ -10,6 +11,8 @@
 from sentry.api.serializers import serialize
 from sentry.api.serializers.rest_framework.group_notes import NoteSerializer
 from sentry.models.activity import Activity
+from sentry.models.groupsubscription import GroupSubscription
+from sentry.notifications.types import GroupSubscriptionReason
 from sentry.signals import comment_deleted, comment_updated
 from sentry.types.activity import ActivityType
 
@@ -29,11 +32,19 @@ def delete(self, request: Request, group, note_id) -> Response:
         if not request.user.is_authenticated:
             raise PermissionDenied(detail="Key doesn't have permission to delete Note")
 
-        try:
-            note = Activity.objects.get(
-                group=group, type=ActivityType.NOTE.value, user_id=request.user.id, id=note_id
-            )
-        except Activity.DoesNotExist:
+        notes_by_user = Activity.objects.filter(
+            group=group, type=ActivityType.NOTE.value, user_id=request.user.id
+        )
+        if not len(notes_by_user):
+            raise ResourceDoesNotExist
+
+        note = None
+        for note in notes_by_user:
+            if note.id == note_id:
+                note = note
+                break
+
+        if not note:
             raise ResourceDoesNotExist
 
         webhook_data = {
@@ -52,6 +63,15 @@ def delete(self, request: Request, group, note_id) -> Response:
             data=webhook_data,
             sender="delete",
         )
+        if features.has("organizations:participants-purge", group.organization):
+            # if the user left more than one comment, we want to keep the subscription
+            if len(notes_by_user) == 1:
+                GroupSubscription.objects.filter(
+                    user_id=request.user.id,
+                    group=group,
+                    project=group.project,
+                    reason=GroupSubscriptionReason.comment,
+                ).delete()
 
         return Response(status=204)
 
diff --git a/tests/sentry/api/endpoints/test_group_notes.py b/tests/sentry/api/endpoints/test_group_notes.py
index 9c6d666cc1f975..8ab0eaa5818cee 100644
--- a/tests/sentry/api/endpoints/test_group_notes.py
+++ b/tests/sentry/api/endpoints/test_group_notes.py
@@ -129,15 +129,19 @@ def test_simple(self):
         assert response.status_code == 400, response.content
 
     def test_with_mentions(self):
-        user = self.create_user(email="hello@meow.com")
+        user_not_on_team = self.create_user(email="hello@meow.com")
+        user_on_team = self.create_user(email="hello@woof.com")
 
         self.org = self.create_organization(name="Gnarly Org", owner=None)
         self.team = self.create_team(organization=self.org, name="Ultra Rad Team")
+        self.create_member(user=self.user, organization=self.org, role="member", teams=[self.team])
 
         # member that IS NOT part of the team
-        self.create_member(user=user, organization=self.org, role="member", teams=[])
+        self.create_member(user=user_not_on_team, organization=self.org, role="member", teams=[])
         # member that IS part of the team
-        self.create_member(user=self.user, organization=self.org, role="member", teams=[self.team])
+        self.create_member(
+            user=user_on_team, organization=self.org, role="member", teams=[self.team]
+        )
         group = self.group
 
         self.login_as(user=self.user)
@@ -148,32 +152,100 @@ def test_with_mentions(self):
         response = self.client.post(
             url,
             format="json",
-            data={"text": "**meredith@getsentry.com** is fun", "mentions": ["8"]},
+            data={"text": "**meredith@getsentry.com** is fun", "mentions": ["8888"]},
         )
         assert response.status_code == 400, response.content
 
-        user_id = str(self.user.id)
-
         # mentioning a member in the correct team returns 201
         response = self.client.post(
             url,
             format="json",
-            data={"text": "**meredith@getsentry.com** is so fun", "mentions": ["%s" % user_id]},
+            data={"text": "**hello@woof.com** is so fun", "mentions": [f"{user_on_team.id}"]},
         )
         assert response.status_code == 201, response.content
-
-        user_id = str(user.id)
+        assert GroupSubscription.objects.get(
+            user_id=self.user.id,
+            group=group,
+            project=group.project,
+            reason=GroupSubscriptionReason.comment,
+        )
+        assert GroupSubscription.objects.get(
+            user_id=user_on_team.id,
+            group=group,
+            project=group.project,
+            reason=GroupSubscriptionReason.mentioned,
+        )
 
         # mentioning a member that exists but NOT in the team returns
         # validation error
         response = self.client.post(
             url,
             format="json",
-            data={"text": "**hello@meow.com** is not so fun", "mentions": ["%s" % user_id]},
+            data={
+                "text": "**hello@meow.com** is not so fun",
+                "mentions": [f"{user_not_on_team.id}"],
+            },
         )
 
         assert response.data == {"mentions": ["Cannot mention a non team member"]}
 
+    @with_feature("organizations:participants-purge")
+    @with_feature("organizations:team-workflow-notifications")
+    def test_mentions_with_participants_purge_flag(self):
+        self.org = self.create_organization(name="Gnarly Org", owner=None)
+        self.team = self.create_team(organization=self.org, name="Ultra Rad Team")
+        user_on_team = self.create_user(email="hello@woof.com")
+        self.create_member(user=self.user, organization=self.org, role="member", teams=[self.team])
+        # member that IS part of the team
+        self.create_member(
+            user=user_on_team, organization=self.org, role="member", teams=[self.team]
+        )
+        group = self.group
+
+        self.login_as(user=self.user)
+
+        url = f"/api/0/issues/{group.id}/comments/"
+
+        # mentioning a member does NOT subscribe them to the issue
+        response = self.client.post(
+            url,
+            format="json",
+            data={"text": "**hello@woof.com** is so fun", "mentions": [f"{user_on_team.id}"]},
+        )
+        assert response.status_code == 201, response.content
+        assert GroupSubscription.objects.get(
+            user_id=self.user.id,
+            group=group,
+            project=group.project,
+            reason=GroupSubscriptionReason.comment,
+        )
+        assert not GroupSubscription.objects.filter(
+            user_id=user_on_team.id,
+            group=group,
+            project=group.project,
+            reason=GroupSubscriptionReason.mentioned,
+        ).exists()
+
+        # mentioning a team does NOT subscribe the team to the issue
+        response = self.client.post(
+            url,
+            format="json",
+            data={"text": "**ultra-rad-team** is so rad", "mentions": [f"team:{self.team.id}"]},
+        )
+        assert response.status_code == 201, response.content
+        assert GroupSubscription.objects.get(
+            user_id=self.user.id,
+            group=group,
+            project=group.project,
+            reason=GroupSubscriptionReason.comment,
+        )
+        assert not GroupSubscription.objects.filter(
+            team=self.team.id,
+            group=group,
+            project=group.project,
+            reason=GroupSubscriptionReason.mentioned,
+        ).exists()
+
     def test_with_team_user_mentions(self):
         user = self.create_user(email="redTeamUser@example.com")
 
diff --git a/tests/sentry/api/endpoints/test_group_notes_details.py b/tests/sentry/api/endpoints/test_group_notes_details.py
index c6619364b29937..c9dabd639a5b96 100644
--- a/tests/sentry/api/endpoints/test_group_notes_details.py
+++ b/tests/sentry/api/endpoints/test_group_notes_details.py
@@ -6,11 +6,14 @@
 from sentry.models.activity import Activity
 from sentry.models.group import Group
 from sentry.models.grouplink import GroupLink
+from sentry.models.groupsubscription import GroupSubscription
 from sentry.models.integrations.external_issue import ExternalIssue
 from sentry.models.integrations.integration import Integration
+from sentry.notifications.types import GroupSubscriptionReason
 from sentry.silo import SiloMode
 from sentry.testutils.cases import APITestCase
 from sentry.testutils.silo import assume_test_silo_mode, region_silo_test
+from sentry.types.activity import ActivityType
 
 
 @region_silo_test(stable=True)
@@ -55,6 +58,75 @@ def test_delete(self):
 
         assert Group.objects.get(id=self.group.id).num_comments == 0
 
+    def test_delete_with_participants_flag(self):
+        """Test that if a user deletes their comment on an issue, we delete the subscription too"""
+        self.login_as(user=self.user)
+        event = self.store_event(data={}, project_id=self.project.id)
+        group = event.group
+
+        # create a comment
+        comment_url = f"/api/0/issues/{group.id}/comments/"
+        response = self.client.post(comment_url, format="json", data={"text": "hi haters"})
+        assert response.status_code == 201, response.content
+        assert GroupSubscription.objects.filter(
+            group=group,
+            project=group.project,
+            user_id=self.user.id,
+            reason=GroupSubscriptionReason.comment,
+        ).exists()
+        activity = Activity.objects.get(
+            group=group, type=ActivityType.NOTE.value, user_id=self.user.id
+        )
+
+        with self.feature("organizations:participants-purge"):
+            url = f"/api/0/issues/{group.id}/comments/{activity.id}/"
+            response = self.client.delete(url, format="json")
+
+        assert response.status_code == 204, response.status_code
+        assert not GroupSubscription.objects.filter(
+            group=group,
+            project=self.group.project,
+            user_id=self.user.id,
+            reason=GroupSubscriptionReason.comment,
+        ).exists()
+
+    def test_delete_with_participants_flag_multiple_comments(self):
+        """Test that if a user has commented multiple times on an issue and deletes one, we don't remove the subscription"""
+        self.login_as(user=self.user)
+        event = self.store_event(data={}, project_id=self.project.id)
+        group = event.group
+
+        # create a comment
+        comment_url = f"/api/0/issues/{group.id}/comments/"
+        response = self.client.post(comment_url, format="json", data={"text": "hi haters"})
+        assert response.status_code == 201, response.content
+        assert GroupSubscription.objects.filter(
+            group=group,
+            project=group.project,
+            user_id=self.user.id,
+            reason=GroupSubscriptionReason.comment,
+        ).exists()
+
+        # create another comment that we'll delete
+        response = self.client.post(comment_url, format="json", data={"text": "bye haters"})
+        assert response.status_code == 201, response.content
+
+        activity = Activity.objects.filter(
+            group=group, type=ActivityType.NOTE.value, user_id=self.user.id
+        ).first()
+
+        with self.feature("organizations:participants-purge"):
+            url = f"/api/0/issues/{group.id}/comments/{activity.id}/"
+            response = self.client.delete(url, format="json")
+
+        assert response.status_code == 204, response.status_code
+        assert GroupSubscription.objects.filter(
+            group=group,
+            project=self.group.project,
+            user_id=self.user.id,
+            reason=GroupSubscriptionReason.comment,
+        ).exists()
+
     @patch("sentry.integrations.mixins.IssueBasicMixin.update_comment")
     @responses.activate
     def test_put(self, mock_update_comment):


Code review comments:
diff_hunk: @@ -29,11 +32,19 @@ def delete(self, request: Request, group, note_id) -> Response:
         if not request.user.is_authenticated:
             raise PermissionDenied(detail="Key doesn't have permission to delete Note")
 
-        try:
-            note = Activity.objects.get(
-                group=group, type=ActivityType.NOTE.value, user_id=request.user.id, id=note_id
-            )
-        except Activity.DoesNotExist:
+        notes_by_user = Activity.objects.filter(
+            group=group, type=ActivityType.NOTE.value, user_id=request.user.id
+        )
+        if not len(notes_by_user):
+            raise ResourceDoesNotExist
+
+        note = None
+        for note in notes_by_user:
+            if note.id == note_id:
+                note = note
path: src/sentry/api/endpoints/group_notes_details.py
text: @user1:
@author what are you doing here lol

@author:
I need to know if the user had left other comments on the issue and then also grab the one comment that matches the `note_id`