Repository: getsentry/sentry
Pull request: feat(participants): Unsubscribe participants when reassigned and unassigned
When an issue is unassigned or reassigned, we should remove them from being a participant and notify them that the issue was assigned to someone else (unless it was unassigned, then they are notified that it was unassigned aka assigned to nobody). 

Closes #57719 

Problem statement:
[BE] Unsubscribe previous group assignees
If a user or team was assigned an issue they become participants, but if the issue is unassigned or re-assigned to another user or team, we do not remove the subscription for the previous assignees. We should remove previous assignees _but_ ensure they receive a workflow notification that they are unassigned/it was reassigned.

Hints from issue discussion:

Commit message: add notification tests

Patch to review:
diff --git a/src/sentry/api/helpers/group_index/update.py b/src/sentry/api/helpers/group_index/update.py
index de6bcde9fa15a2..b2459f52f8dac5 100644
--- a/src/sentry/api/helpers/group_index/update.py
+++ b/src/sentry/api/helpers/group_index/update.py
@@ -836,6 +836,9 @@ def handle_assigned_to(
     )
     if assigned_actor:
         for group in group_list:
+            if features.has("organizations:participants-purge", group.organization):
+                GroupAssignee.objects.deassign(group, acting_user)
+
             resolved_actor: RpcUser | Team = assigned_actor.resolve()
 
             assignment = GroupAssignee.objects.assign(
diff --git a/src/sentry/models/groupassignee.py b/src/sentry/models/groupassignee.py
index 8e1e74cd85df2e..f99796ed89e1d2 100644
--- a/src/sentry/models/groupassignee.py
+++ b/src/sentry/models/groupassignee.py
@@ -18,6 +18,7 @@
 from sentry.db.models.fields.hybrid_cloud_foreign_key import HybridCloudForeignKey
 from sentry.models.grouphistory import GroupHistoryStatus, record_group_history
 from sentry.models.groupowner import GroupOwner
+from sentry.models.groupsubscription import GroupSubscription
 from sentry.notifications.types import GroupSubscriptionReason
 from sentry.signals import issue_assigned, issue_unassigned
 from sentry.types.activity import ActivityType
@@ -121,6 +122,19 @@ def deassign(self, group: Group, acting_user: User | RpcUser | None = None) -> N
         from sentry.integrations.utils import sync_group_assignee_outbound
         from sentry.models.activity import Activity
         from sentry.models.projectownership import ProjectOwnership
+        from sentry.models.team import Team
+        from sentry.services.hybrid_cloud.user.service import user_service
+
+        previous_assignee: User | RpcUser | Team | None = None
+        try:
+            previous_groupassignee = self.get(group=group)
+        except GroupAssignee.DoesNotExist:
+            previous_groupassignee = None
+        else:
+            if previous_groupassignee.user_id:
+                previous_assignee = user_service.get_user(previous_groupassignee.user_id)
+            else:
+                previous_assignee = Team.objects.get(id=previous_groupassignee.team.id)
 
         affected = self.filter(group=group)[:1].count()
         self.filter(group=group).delete()
@@ -141,6 +155,33 @@ def deassign(self, group: Group, acting_user: User | RpcUser | None = None) -> N
             GroupOwner.invalidate_assignee_exists_cache(group.project.id, group.id)
             GroupOwner.invalidate_debounce_issue_owners_evaluation_cache(group.project.id, group.id)
 
+            if (
+                features.has("organizations:participants-purge", group.organization)
+                and previous_assignee
+            ):
+                if (
+                    features.has("organizations:team-workflow-notifications", group.organization)
+                    and type(previous_assignee) is Team
+                ):
+                    GroupSubscription.objects.filter(
+                        group=group,
+                        project=group.project,
+                        team=previous_assignee,
+                        reason=GroupSubscriptionReason.assigned,
+                    ).delete()
+                elif type(previous_assignee) is Team:
+                    team_members = list(
+                        previous_assignee.member_set.values_list("user_id", flat=True)
+                    )
+                    for member in team_members:
+                        GroupSubscription.objects.filter(
+                            group=group, project=group.project, user_id=member
+                        ).delete()
+                else:
+                    GroupSubscription.objects.filter(
+                        group=group, project=group.project, user_id=previous_assignee.id
+                    ).delete()
+
             metrics.incr("group.assignee.change", instance="deassigned", skip_internal=True)
             # sync Sentry assignee to external issues
             if features.has(
diff --git a/src/sentry/testutils/helpers/slack.py b/src/sentry/testutils/helpers/slack.py
index 9b91fb9d7c593f..81c8f64660d463 100644
--- a/src/sentry/testutils/helpers/slack.py
+++ b/src/sentry/testutils/helpers/slack.py
@@ -97,9 +97,9 @@ def send_notification(provider, *args_list):
         send_notification_as_slack(*args_list, {})
 
 
-def get_attachment():
+def get_attachment(index=0):
     assert len(responses.calls) >= 1
-    data = parse_qs(responses.calls[0].request.body)
+    data = parse_qs(responses.calls[index].request.body)
     assert "text" in data
     assert "attachments" in data
     attachments = json.loads(data["attachments"][0])
diff --git a/tests/sentry/api/helpers/test_group_index.py b/tests/sentry/api/helpers/test_group_index.py
index ff7f9c734afe37..7bf600a38fc111 100644
--- a/tests/sentry/api/helpers/test_group_index.py
+++ b/tests/sentry/api/helpers/test_group_index.py
@@ -523,6 +523,12 @@ def test_assigned_to(self, mock_record: Mock) -> None:
         )
 
         assert GroupAssignee.objects.filter(group=self.group, user_id=self.user.id).exists()
+        assert GroupSubscription.objects.filter(
+            group=self.group,
+            project=self.group.project,
+            user_id=self.user.id,
+            reason=GroupSubscriptionReason.assigned,
+        ).exists()
 
         assert assigned_to == {
             "email": self.user.email,
@@ -556,3 +562,204 @@ def test_unassign(self, mock_record: Mock) -> None:
             assigned_by=None,
             had_to_deassign=True,
         )
+
+    @patch("sentry.analytics.record")
+    def test_unassign_with_feature_flag(self, mock_record: Mock) -> None:
+        # first assign the issue
+        assigned_to = handle_assigned_to(
+            ActorTuple.from_actor_identifier(self.user.id),
+            None,
+            None,
+            self.group_list,
+            self.project_lookup,
+            self.user,
+        )
+
+        assert GroupAssignee.objects.filter(group=self.group, user_id=self.user.id).exists()
+        assert GroupSubscription.objects.filter(
+            group=self.group,
+            project=self.group.project,
+            user_id=self.user.id,
+            reason=GroupSubscriptionReason.assigned,
+        ).exists()
+
+        # then unassign it
+        assigned_to = handle_assigned_to(
+            None, None, None, self.group_list, self.project_lookup, self.user
+        )
+
+        assert not GroupAssignee.objects.filter(group=self.group, user_id=self.user.id).exists()
+        assert not GroupSubscription.objects.filter(
+            group=self.group,
+            project=self.group.project,
+            user_id=self.user.id,
+            reason=GroupSubscriptionReason.assigned,
+        ).exists()
+
+        assert assigned_to is None
+        mock_record.assert_called_with(
+            "manual.issue_assignment",
+            group_id=self.group.id,
+            organization_id=self.group.project.organization_id,
+            project_id=self.group.project_id,
+            assigned_by=None,
+            had_to_deassign=True,
+        )
+
+    @patch("sentry.analytics.record")
+    def test_reassign_user(self, mock_record: Mock) -> None:
+        user2 = self.create_user(email="meow@meow.meow")
+
+        # first assign the issue
+        assigned_to = handle_assigned_to(
+            ActorTuple.from_actor_identifier(self.user.id),
+            None,
+            None,
+            self.group_list,
+            self.project_lookup,
+            self.user,
+        )
+
+        assert GroupAssignee.objects.filter(group=self.group, user_id=self.user.id).exists()
+        assert GroupSubscription.objects.filter(
+            group=self.group,
+            project=self.group.project,
+            user_id=self.user.id,
+            reason=GroupSubscriptionReason.assigned,
+        ).exists()
+
+        # then assign it to someone else
+        with self.feature("organizations:participants-purge"):
+            assigned_to = handle_assigned_to(
+                ActorTuple.from_actor_identifier(user2.id),
+                None,
+                None,
+                self.group_list,
+                self.project_lookup,
+                self.user,
+            )
+
+        assert not GroupAssignee.objects.filter(group=self.group, user_id=self.user.id).exists()
+        assert not GroupSubscription.objects.filter(
+            group=self.group,
+            project=self.group.project,
+            user_id=self.user.id,
+            reason=GroupSubscriptionReason.assigned,
+        ).exists()
+        assert GroupAssignee.objects.filter(group=self.group, user_id=user2.id).exists()
+        assert GroupSubscription.objects.filter(
+            group=self.group,
+            project=self.group.project,
+            user_id=user2.id,
+            reason=GroupSubscriptionReason.assigned,
+        ).exists()
+
+        assert assigned_to == {
+            "email": user2.email,
+            "id": str(user2.id),
+            "name": user2.username,
+            "type": "user",
+        }
+        mock_record.assert_called_with(
+            "manual.issue_assignment",
+            group_id=self.group.id,
+            organization_id=self.group.project.organization_id,
+            project_id=self.group.project_id,
+            assigned_by=None,
+            had_to_deassign=False,
+        )
+
+    @patch("sentry.analytics.record")
+    def test_reassign_team(self, mock_record: Mock) -> None:
+        user1 = self.create_user("foo@example.com")
+        user2 = self.create_user("bar@example.com")
+        team1 = self.create_team()
+        member1 = self.create_member(user=user1, organization=self.organization, role="member")
+        member2 = self.create_member(user=user2, organization=self.organization, role="member")
+        self.create_team_membership(team1, member1, role="admin")
+        self.create_team_membership(team1, member2, role="admin")
+
+        user3 = self.create_user("baz@example.com")
+        user4 = self.create_user("boo@example.com")
+        team2 = self.create_team()
+        member3 = self.create_member(user=user3, organization=self.organization, role="member")
+        member4 = self.create_member(user=user4, organization=self.organization, role="member")
+        self.create_team_membership(team2, member3, role="admin")
+        self.create_team_membership(team2, member4, role="admin")
+
+        # first assign the issue to team1
+        assigned_to = handle_assigned_to(
+            (ActorTuple.from_actor_identifier(f"team:{team1.id}")),
+            None,
+            None,
+            self.group_list,
+            self.project_lookup,
+            self.user,
+        )
+
+        assert GroupAssignee.objects.filter(group=self.group, team=team1.id).exists()
+        assert GroupSubscription.objects.filter(
+            group=self.group,
+            project=self.group.project,
+            user_id=user1.id,
+            reason=GroupSubscriptionReason.assigned,
+        ).exists()
+        assert GroupSubscription.objects.filter(
+            group=self.group,
+            project=self.group.project,
+            user_id=user2.id,
+            reason=GroupSubscriptionReason.assigned,
+        ).exists()
+
+        # then assign it to team2
+        with self.feature("organizations:participants-purge"):
+            assigned_to = handle_assigned_to(
+                ActorTuple.from_actor_identifier(f"team:{team2.id}"),
+                None,
+                None,
+                self.group_list,
+                self.project_lookup,
+                self.user,
+            )
+
+        assert not GroupAssignee.objects.filter(group=self.group, team=team1.id).exists()
+        assert not GroupSubscription.objects.filter(
+            group=self.group,
+            project=self.group.project,
+            user_id=user1.id,
+            reason=GroupSubscriptionReason.assigned,
+        ).exists()
+        assert not GroupSubscription.objects.filter(
+            group=self.group,
+            project=self.group.project,
+            user_id=user2.id,
+            reason=GroupSubscriptionReason.assigned,
+        ).exists()
+
+        assert GroupAssignee.objects.filter(group=self.group, team=team2.id).exists()
+        assert GroupSubscription.objects.filter(
+            group=self.group,
+            project=self.group.project,
+            user_id=user3.id,
+            reason=GroupSubscriptionReason.assigned,
+        ).exists()
+        assert GroupSubscription.objects.filter(
+            group=self.group,
+            project=self.group.project,
+            user_id=user4.id,
+            reason=GroupSubscriptionReason.assigned,
+        ).exists()
+
+        assert assigned_to == {
+            "id": str(team2.id),
+            "name": team2.slug,
+            "type": "team",
+        }
+        mock_record.assert_called_with(
+            "manual.issue_assignment",
+            group_id=self.group.id,
+            organization_id=self.group.project.organization_id,
+            project_id=self.group.project_id,
+            assigned_by=None,
+            had_to_deassign=False,
+        )
diff --git a/tests/sentry/notifications/notifications/test_assigned.py b/tests/sentry/notifications/notifications/test_assigned.py
index d6b576a9e19925..55331b3c3137d4 100644
--- a/tests/sentry/notifications/notifications/test_assigned.py
+++ b/tests/sentry/notifications/notifications/test_assigned.py
@@ -2,7 +2,6 @@
 from django.core import mail
 from django.core.mail.message import EmailMultiAlternatives
 
-from sentry.models.identity import Identity, IdentityProvider, IdentityStatus
 from sentry.models.notificationsetting import NotificationSetting
 from sentry.models.options.user_option import UserOption
 from sentry.notifications.types import NotificationSettingOptionValues, NotificationSettingTypes
@@ -15,10 +14,44 @@
 
 
 class AssignedNotificationAPITest(APITestCase):
+    def validate_email(self, outbox, index, email, txt_msg, html_msg):
+        msg = outbox[index]
+        assert msg.to == [email]
+        assert isinstance(msg, EmailMultiAlternatives)
+        # check the txt version
+        assert txt_msg in msg.body
+        # check the html version
+        assert isinstance(msg.alternatives[0][0], str)
+        assert html_msg in msg.alternatives[0][0]
+
+    def validate_slack_message(self, msg, group, project, index=0):
+        attachment, text = get_attachment(index)
+        assert text == msg
+        assert attachment["title"] == group.title
+        assert project.slug in attachment["footer"]
+
+    def setup_user(self, user, team):
+        member = self.create_member(user=user, organization=self.organization, role="member")
+        self.create_team_membership(team, member, role="admin")
+
+        UserOption.objects.create(user=user, key="self_notifications", value="1")
+
+        NotificationSetting.objects.update_settings(
+            ExternalProviders.SLACK,
+            NotificationSettingTypes.WORKFLOW,
+            NotificationSettingOptionValues.ALWAYS,
+            user_id=user.id,
+        )
+        self.access_token = "xoxb-access-token"
+        self.identity = self.create_identity(
+            user=user, identity_provider=self.provider, external_id=user.id
+        )
+
     def setUp(self):
         super().setUp()
 
         self.integration = install_slack(self.organization)
+        self.provider = self.create_identity_provider(integration=self.integration)
 
         self.login_as(self.user)
 
@@ -28,26 +61,13 @@ def test_sends_assignment_notification(self):
         Test that an email AND Slack notification are sent with
         the expected values when an issue is assigned.
         """
-
-        UserOption.objects.create(user=self.user, key="self_notifications", value="1")
-        NotificationSetting.objects.update_settings(
-            ExternalProviders.SLACK,
-            NotificationSettingTypes.WORKFLOW,
-            NotificationSettingOptionValues.ALWAYS,
-            user_id=self.user.id,
-        )
-
-        Identity.objects.create(
-            external_id="UXXXXXXX1",
-            idp=IdentityProvider.objects.create(type="slack", external_id="TXXXXXXX1", config={}),
-            user=self.user,
-            status=IdentityStatus.VALID,
-            scopes=[],
-        )
+        user = self.create_user()
+        self.setup_user(user, self.team)
+        self.login_as(user)
 
         url = f"/api/0/issues/{self.group.id}/"
         with self.tasks():
-            response = self.client.put(url, format="json", data={"assignedTo": self.user.username})
+            response = self.client.put(url, format="json", data={"assignedTo": user.username})
         assert response.status_code == 200, response.content
 
         msg = mail.outbox[0]
@@ -60,7 +80,7 @@ def test_sends_assignment_notification(self):
 
         attachment, text = get_attachment()
 
-        assert text == f"Issue assigned to {self.user.get_display_name()} by themselves"
+        assert text == f"Issue assigned to {user.get_display_name()} by themselves"
         assert attachment["title"] == self.group.title
         assert self.project.slug in attachment["footer"]
 
@@ -97,3 +117,153 @@ def test_sends_assignment_notification_team(self):
         )
         assert attachment["title"] == self.group.title
         assert self.project.slug in attachment["footer"]
+
+    @responses.activate
+    def test_sends_reassignment_notification_user(self):
+        """Test that if a user is assigned to an issue and then the issue is reassigned to a different user
+        that the original assignee receives an unassignment notification as well as the new assignee
+        receiving an assignment notification"""
+        user1 = self.create_user()
+        user2 = self.create_user()
+        self.setup_user(user1, self.team)
+        self.setup_user(user2, self.team)
+
+        self.login_as(user1)
+
+        url = f"/api/0/issues/{self.group.id}/"
+        with self.tasks():
+            response = self.client.put(
+                url,
+                format="json",
+                data={"assignedTo": user1.username, "assignedBy": user1.username},
+            )
+        assert response.status_code == 200, response.content
+
+        with self.tasks(), self.feature("organizations:participants-purge"):
+            response = self.client.put(
+                url,
+                format="json",
+                data={"assignedTo": user2.username, "assignedBy": user1.username},
+            )
+        assert response.status_code == 200, response.content
+
+        assert len(mail.outbox) == 3
+
+        txt_msg = f"assigned {self.group.qualified_short_id} to themselves"
+        html_msg = f"{self.group.qualified_short_id}</a> to themselves</p>"
+        self.validate_email(mail.outbox, 0, user1.email, txt_msg, html_msg)
+
+        txt_msg = f"{user1.email} unassigned {self.group.qualified_short_id}"
+        html_msg = f"{user1.email}</strong> unassigned "
+        self.validate_email(mail.outbox, 1, user1.email, txt_msg, html_msg)
+
+        txt_msg = f"assigned {self.group.qualified_short_id} to {user2.email}"
+        html_msg = f"{self.group.qualified_short_id}</a> to {user2.email}</p>"
+        self.validate_email(mail.outbox, 2, user2.email, txt_msg, html_msg)
+
+        msg = f"Issue assigned to {user1.get_display_name()} by themselves"
+        self.validate_slack_message(msg, self.group, self.project, index=0)
+        self.validate_slack_message(msg, self.group, self.project, index=1)
+
+        msg = f"Issue unassigned by {user1.get_display_name()}"
+        self.validate_slack_message(msg, self.group, self.project, index=2)
+        self.validate_slack_message(msg, self.group, self.project, index=3)
+
+        msg = f"Issue assigned to {user2.get_display_name()} by {user1.get_display_name()}"
+        self.validate_slack_message(msg, self.group, self.project, index=4)
+        self.validate_slack_message(msg, self.group, self.project, index=5)
+
+    @responses.activate
+    def test_sends_reassignment_notification_team(self):
+        """Test that if a team is assigned to an issue and then the issue is reassigned to a different team
+        that the originally assigned team receives an unassignment notification as well as the new assigned
+        team receiving an assignment notification"""
+        user1 = self.create_user("foo@example.com")
+        user2 = self.create_user("bar@example.com")
+        user3 = self.create_user("baz@example.com")
+        user4 = self.create_user("boo@example.com")
+        team1 = self.create_team()
+        team2 = self.create_team()
+        project = self.create_project(teams=[team1, team2])
+        group = self.create_group(project=project)
+        self.setup_user(user1, team1)
+        self.setup_user(user2, team1)
+        self.setup_user(user3, team2)
+        self.setup_user(user4, team2)
+
+        link_team(
+            team=team1,
+            integration=self.integration,
+            channel_id="CXXXXXXX1",
+            channel_name="#python",
+        )
+        NotificationSetting.objects.update_settings(
+            ExternalProviders.SLACK,
+            NotificationSettingTypes.WORKFLOW,
+            NotificationSettingOptionValues.ALWAYS,
+            team_id=team1.id,
+            organization_id_for_team=self.organization.id,
+        )
+        link_team(
+            team=team2,
+            integration=self.integration,
+            channel_id="CXXXXXXX2",
+            channel_name="#javascript",
+        )
+        NotificationSetting.objects.update_settings(
+            ExternalProviders.SLACK,
+            NotificationSettingTypes.WORKFLOW,
+            NotificationSettingOptionValues.ALWAYS,
+            team_id=team2.id,
+            organization_id_for_team=self.organization.id,
+        )
+
+        self.login_as(user1)
+
+        url = f"/api/0/issues/{group.id}/"
+        with self.tasks():
+            response = self.client.put(url, format="json", data={"assignedTo": f"team:{team1.id}"})
+        assert response.status_code == 200, response.content
+
+        with self.tasks(), self.feature("organizations:participants-purge"):
+            response = self.client.put(
+                url,
+                format="json",
+                data={"assignedTo": f"team:{team2.id}", "assignedBy": self.user.username},
+            )
+        assert response.status_code == 200, response.content
+
+        assert len(mail.outbox) == 6
+
+        txt_msg = f"assigned {group.qualified_short_id} to the {team1.slug} team"
+        html_msg = f"{group.qualified_short_id}</a> to the {team1.slug} team</p>"
+        self.validate_email(mail.outbox, 0, user2.email, txt_msg, html_msg)
+
+        txt_msg = f"{user1.email} unassigned {group.qualified_short_id}"
+        html_msg = f"{user1.email}</strong> unassigned "
+        self.validate_email(mail.outbox, 2, user2.email, txt_msg, html_msg)
+
+        txt_msg = f"assigned {group.qualified_short_id} to the {team2.slug} team"
+        html_msg = f"to the {team2.slug} team</p>"
+        self.validate_email(mail.outbox, 4, user3.email, txt_msg, html_msg)
+
+        msg = f"Issue assigned to the {team1.slug} team by {user1.email}"
+        self.validate_slack_message(msg, group, project, index=0)
+
+        msg = f"Issue assigned to the {team1.slug} team by {user1.email}"
+        self.validate_slack_message(msg, group, project, index=2)
+
+        msg = f"Issue assigned to the {team1.slug} team by {user1.email}"
+        self.validate_slack_message(msg, group, project, index=4)
+
+        msg = f"Issue unassigned by {user1.get_display_name()}"
+        self.validate_slack_message(msg, group, project, index=6)
+
+        msg = f"Issue unassigned by {user1.get_display_name()}"
+        self.validate_slack_message(msg, group, project, index=8)
+
+        msg = f"Issue assigned to the {team2.slug} team by {user1.email}"
+        self.validate_slack_message(msg, group, project, index=10)
+
+        msg = f"Issue assigned to the {team2.slug} team by {user1.email}"
+        self.validate_slack_message(msg, group, project, index=12)


Code review comments:
diff_hunk: @@ -836,6 +836,9 @@ def handle_assigned_to(
     )
     if assigned_actor:
         for group in group_list:
+            if features.has("organizations:participants-purge", group.organization):
+                GroupAssignee.objects.deassign(group, acting_user)
path: src/sentry/api/helpers/group_index/update.py
text: @author:
Passing through `deassign` before `assign` (when reassigning) neatly sends the unassignment notification 

@user1:
Not sure if it's actually a problem, but this also creates extra activities for the group, so that there will always be a "unassigned" then "assigned to" message