streams: Send message when a user is unsubscribed from a channel.

Previously, Notification Bot only sent messages to notify users when they were added to channels by others. However, there was no corresponding notification when they were removed from channels. This commit ensures that when a user is unsubscribed from a channel by another user, Notification Bot sends a message to inform them, using a silent mention of the acting user. The message follows this format: "@_**username** unsubscribed you from the channel #channel_name." Fixes: #29272
zulip · Nov 4, 2024 · 6e060c2 · 6e060c2
1 parent 7384e07
commit 6e060c2
Show file tree

Hide file tree

Showing 8 changed files with 149 additions and 16 deletions.
diff --git a/help/add-or-remove-users-from-a-channel.md b/help/add-or-remove-users-from-a-channel.md
@@ -146,6 +146,8 @@ subscribe the user.
 
 {end_tabs}
 
+{!automated-dm-channel-remove.md!}
+
 ## Related articles
 
 * [Introduction to channels](/help/introduction-to-channels)

diff --git a/help/configure-automated-notices.md b/help/configure-automated-notices.md
@@ -54,7 +54,7 @@ automated notices to help others understand how content was moved.
 
 ## Notices about users
 
-You will be notified if someone [subscribes you to a
+You will be notified if someone [subscribes you to or removes you from a
 channel](/help/add-or-remove-users-from-a-channel#add-users-to-a-channel), or
 changes your [group](/help/user-groups) membership.
 

diff --git a/help/include/automated-dm-channel-remove.md b/help/include/automated-dm-channel-remove.md
@@ -0,0 +1,6 @@
+!!! warn ""
+
+    **Note**: Removing someone else from a channel sends them an
+    automated direct message from the [Notification Bot][notification-bot].
+
+[notification-bot]: /help/configure-automated-notices
diff --git a/zerver/openapi/curl_param_value_generators.py b/zerver/openapi/curl_param_value_generators.py
@@ -126,7 +126,7 @@ def add_emoji_to_message() -> dict[str, object]:
 
     # The message ID here is hardcoded based on the corresponding value
     # for the example message IDs we use in zulip.yaml.
-    message_id = 48
+    message_id = 49
     emoji_name = "octopus"
     emoji_code = "1f419"
     reaction_type = "unicode_emoji"

diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml
@@ -24644,7 +24644,7 @@ components:
         The target message's ID.
       schema:
         type: integer
-      example: 43
+      example: 44
       required: true
     UserId:
       name: user_id

diff --git a/zerver/tests/test_openapi.py b/zerver/tests/test_openapi.py
@@ -88,7 +88,7 @@ def test_get_openapi_parameters(self) -> None:
             description="The target message's ID.\n",
             json_encoded=False,
             value_schema={"type": "integer"},
-            example=43,
+            example=44,
             required=True,
             deprecated=False,
         )

diff --git a/zerver/tests/test_subs.py b/zerver/tests/test_subs.py
@@ -2551,6 +2551,20 @@ def test_archive_invite_only_stream_youre_not_on(self) -> None:
         )
         self.archive_stream(priv_stream)
 
+    def assert_user_got_unsubscription_notification(
+        self, user: UserProfile, expected_msg: str
+    ) -> None:
+        # verify that the user was sent a message informing them about the subscription
+        realm = user.realm
+        msg = most_recent_message(user)
+        self.assertEqual(msg.recipient.type, msg.recipient.PERSONAL)
+        self.assertEqual(msg.sender_id, self.notification_bot(realm).id)
+
+        def non_ws(s: str) -> str:
+            return s.replace("\n", "").replace(" ", "")
+
+        self.assertEqual(non_ws(msg.content), non_ws(expected_msg))
+
     def attempt_unsubscribe_of_principal(
         self,
         target_users: list[UserProfile],
@@ -2630,16 +2644,60 @@ def test_realm_admin_remove_others_from_public_stream(self) -> None:
         those you aren't on.
         """
         result = self.attempt_unsubscribe_of_principal(
-            query_count=17,
+            query_count=33,
             target_users=[self.example_user("cordelia")],
             is_realm_admin=True,
             is_subbed=True,
             invite_only=False,
             target_users_subbed=True,
         )
+        user_profile = self.example_user("iago")
+        unsubscription_notification_message = f"""
+        @_**{user_profile.full_name}|{user_profile.id}** unsubscribed you from the channel #**hümbüǵ**.
+        """
         json = self.assert_json_success(result)
         self.assert_length(json["removed"], 1)
         self.assert_length(json["not_removed"], 0)
+        self.assert_user_got_unsubscription_notification(
+            self.example_user("cordelia"), unsubscription_notification_message
+        )
+
+    def test_realm_admin_remove_others_from_multiple_public_streams(self) -> None:
+        user_profile = self.example_user("iago")
+        self.login_user(user_profile)
+
+        target_users = [self.example_user(name) for name in ["cordelia", "prospero", "hamlet"]]
+        principals = [user.id for user in target_users]
+        public_channels_to_unsubscribe = []
+
+        for i in range(1, 5):
+            channel_name = f"channel_{i}"
+            self.make_stream(channel_name, invite_only=False)
+            public_channels_to_unsubscribe.append(channel_name)
+            self.subscribe(user_profile, channel_name)
+            for user in target_users:
+                self.subscribe(user, channel_name)
+
+        result = self.client_delete(
+            "/json/users/me/subscriptions",
+            {
+                "subscriptions": orjson.dumps(public_channels_to_unsubscribe).decode(),
+                "principals": orjson.dumps(principals).decode(),
+            },
+        )
+        self.assert_json_success(result)
+
+        unsubscription_notification_message = f"""
+        @_**{user_profile.full_name}|{user_profile.id}** unsubscribed you from the following channels:
+        """
+        unsubscription_notification_message += "\n\n"
+        for channel_name in public_channels_to_unsubscribe:
+            unsubscription_notification_message += f"* #**{channel_name}**\n"
+
+        for target_user in target_users:
+            self.assert_user_got_unsubscription_notification(
+                target_user, unsubscription_notification_message
+            )
 
     def test_realm_admin_remove_multiple_users_from_stream(self) -> None:
         """
@@ -2657,8 +2715,8 @@ def test_realm_admin_remove_multiple_users_from_stream(self) -> None:
             for name in ["cordelia", "prospero", "iago", "hamlet", "outgoing_webhook_bot"]
         ]
         result = self.attempt_unsubscribe_of_principal(
-            query_count=24,
-            cache_count=8,
+            query_count=56,
+            cache_count=27,
             target_users=target_users,
             is_realm_admin=True,
             is_subbed=True,
@@ -2675,7 +2733,7 @@ def test_realm_admin_remove_others_from_subbed_private_stream(self) -> None:
         are on.
         """
         result = self.attempt_unsubscribe_of_principal(
-            query_count=17,
+            query_count=33,
             target_users=[self.example_user("cordelia")],
             is_realm_admin=True,
             is_subbed=True,
@@ -2692,7 +2750,7 @@ def test_realm_admin_remove_others_from_unsubbed_private_stream(self) -> None:
         streams you aren't on.
         """
         result = self.attempt_unsubscribe_of_principal(
-            query_count=17,
+            query_count=33,
             target_users=[self.example_user("cordelia")],
             is_realm_admin=True,
             is_subbed=False,
@@ -2718,7 +2776,7 @@ def test_cant_remove_others_from_stream_legacy_emails(self) -> None:
 
     def test_admin_remove_others_from_stream_legacy_emails(self) -> None:
         result = self.attempt_unsubscribe_of_principal(
-            query_count=17,
+            query_count=33,
             target_users=[self.example_user("cordelia")],
             is_realm_admin=True,
             is_subbed=True,
@@ -2732,7 +2790,7 @@ def test_admin_remove_others_from_stream_legacy_emails(self) -> None:
 
     def test_admin_remove_multiple_users_from_stream_legacy_emails(self) -> None:
         result = self.attempt_unsubscribe_of_principal(
-            query_count=19,
+            query_count=43,
             target_users=[self.example_user("cordelia"), self.example_user("prospero")],
             is_realm_admin=True,
             is_subbed=True,
@@ -2746,7 +2804,7 @@ def test_admin_remove_multiple_users_from_stream_legacy_emails(self) -> None:
 
     def test_remove_unsubbed_user_along_with_subbed(self) -> None:
         result = self.attempt_unsubscribe_of_principal(
-            query_count=16,
+            query_count=17,
             target_users=[self.example_user("cordelia"), self.example_user("iago")],
             is_realm_admin=True,
             is_subbed=True,
@@ -2763,7 +2821,7 @@ def test_remove_already_not_subbed(self) -> None:
         fails gracefully.
         """
         result = self.attempt_unsubscribe_of_principal(
-            query_count=9,
+            query_count=10,
             target_users=[self.example_user("cordelia")],
             is_realm_admin=True,
             is_subbed=False,
@@ -2779,7 +2837,7 @@ def test_bot_owner_can_remove_bot_from_stream(self) -> None:
         webhook_bot = self.example_user("webhook_bot")
         do_change_bot_owner(webhook_bot, bot_owner=user_profile, acting_user=user_profile)
         result = self.attempt_unsubscribe_of_principal(
-            query_count=14,
+            query_count=15,
             target_users=[webhook_bot],
             is_realm_admin=False,
             is_subbed=True,

diff --git a/zerver/views/streams.py b/zerver/views/streams.py
@@ -475,6 +475,59 @@ def compose_views(thunks: list[Callable[[], HttpResponse]]) -> dict[str, Any]:
     return json_dict
 
 
+def get_just_unsubscribed_message_content(acting_user: UserProfile, channel_names: set[str]) -> str:
+    subscriptions = sorted(channel_names)
+    if len(subscriptions) == 1:
+        return _("{user_full_name} unsubscribed you from the channel {channel_name}.").format(
+            user_full_name=f"@_**{acting_user.full_name}|{acting_user.id}**",
+            channel_name=f"#**{subscriptions[0]}**",
+        )
+
+    message = _("{user_full_name} unsubscribed you from the following channels:").format(
+        user_full_name=f"@_**{acting_user.full_name}|{acting_user.id}**",
+    )
+    message += "\n\n"
+    for channel_name in subscriptions:
+        message += f"* #**{channel_name}**\n"
+    return message
+
+
+def send_messages_for_unsubscribers(
+    acting_user: UserProfile,
+    unsubscribed_users: set[UserProfile],
+    unsubscribed_channels_by_user: dict[str, list[str]],
+    id_to_user_profile: dict[str, UserProfile],
+) -> None:
+    """
+    Send notifications to users when they are unsubscribed from channels.
+    """
+    bot_ids = {str(user.id) for user in unsubscribed_users if user.is_bot}
+    notifications = []
+
+    sender = get_system_bot(settings.NOTIFICATION_BOT, acting_user.realm_id)
+    if unsubscribed_channels_by_user:
+        for user_id, unsubscribed_channel_names in unsubscribed_channels_by_user.items():
+            if user_id == str(acting_user.id) or user_id in bot_ids:
+                continue
+
+            recipient_user = id_to_user_profile[user_id]
+            msg = get_just_unsubscribed_message_content(
+                acting_user=acting_user,
+                channel_names=set(unsubscribed_channel_names),
+            )
+
+            notifications.append(
+                internal_prep_private_message(
+                    sender=sender,
+                    recipient_user=recipient_user,
+                    content=msg,
+                    mention_backend=MentionBackend(acting_user.realm.id),
+                )
+            )
+    if notifications:
+        do_send_messages(notifications, mark_as_read=[acting_user.id])
+
+
 @typed_endpoint
 def remove_subscriptions_backend(
     request: HttpRequest,
@@ -505,16 +558,30 @@ def remove_subscriptions_backend(
         unsubscribing_others=unsubscribing_others,
     )
 
+    id_to_user_profile: dict[str, UserProfile] = {}
     result: dict[str, list[str]] = dict(removed=[], not_removed=[])
+    unsubscribed_channels_by_user_dict: dict[str, list[str]] = defaultdict(list)
     (removed, not_subscribed) = bulk_remove_subscriptions(
         realm, people_to_unsub, streams, acting_user=user_profile
     )
 
-    for subscriber, removed_stream in removed:
-        result["removed"].append(removed_stream.name)
+    for subscriber, removed_channel in removed:
+        user_id = str(subscriber.id)
+        result["removed"].append(removed_channel.name)
+        unsubscribed_channels_by_user_dict[user_id].append(removed_channel.name)
+        id_to_user_profile[user_id] = subscriber
     for subscriber, not_subscribed_stream in not_subscribed:
         result["not_removed"].append(not_subscribed_stream.name)
 
+    for user_id in unsubscribed_channels_by_user_dict:
+        unsubscribed_channels_by_user_dict[user_id].sort()
+
+    send_messages_for_unsubscribers(
+        acting_user=user_profile,
+        unsubscribed_users=people_to_unsub,
+        unsubscribed_channels_by_user=unsubscribed_channels_by_user_dict,
+        id_to_user_profile=id_to_user_profile,
+    )
     return json_success(request, data=result)