From 71b8f4961423786ff4d5357ae688264bbabbe986 Mon Sep 17 00:00:00 2001
From: Sahil Batra <sahil@zulip.com>
Date: Fri, 27 Oct 2023 12:21:43 +0530
Subject: [PATCH] streams: Return early if there is no change in subscriptions.

This commit updates code in bulk_remove_subscriptions and
bulk_add_subscriptions to return early if there are no
subscribers to remove or add to the streams.

This change helps us in avoiding unnecessary queries like the
one used to get subscribers list of streams, which is then used
to send events but we would not send any events if no subscribers
are added or removed and some more similar queries.
---
 zerver/actions/streams.py |  9 +++++++++
 zerver/tests/test_subs.py | 15 ++++++++++++++-
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/zerver/actions/streams.py b/zerver/actions/streams.py
index 09297d521f..d3d280ca5c 100644
--- a/zerver/actions/streams.py
+++ b/zerver/actions/streams.py
@@ -639,6 +639,10 @@ def bulk_add_subscriptions(
             sub_info = SubInfo(user_profile, sub, stream)
             subs_to_add.append(sub_info)
 
+    if len(subs_to_add) == 0 and len(subs_to_activate) == 0:
+        # We can return early if users are already subscribed to all the streams.
+        return ([], already_subscribed)
+
     bulk_add_subs_to_db_with_logging(
         realm=realm,
         acting_user=acting_user,
@@ -807,6 +811,11 @@ def bulk_remove_subscriptions(
     subs_to_deactivate = [
         sub_info for sub_infos in existing_subs_by_user.values() for sub_info in sub_infos
     ]
+
+    if len(subs_to_deactivate) == 0:
+        # We can return early if users are not subscribed to any of the streams.
+        return ([], not_subscribed)
+
     sub_ids_to_deactivate = [sub_info.sub.id for sub_info in subs_to_deactivate]
     streams_to_unsubscribe = [sub_info.stream for sub_info in subs_to_deactivate]
     # We do all the database changes in a transaction to ensure
diff --git a/zerver/tests/test_subs.py b/zerver/tests/test_subs.py
index fa9ef0f97e..9857129b32 100644
--- a/zerver/tests/test_subs.py
+++ b/zerver/tests/test_subs.py
@@ -2712,13 +2712,26 @@ class StreamAdminTest(ZulipTestCase):
         self.assert_length(json["removed"], 2)
         self.assert_length(json["not_removed"], 0)
 
+    def test_remove_unsubbed_user_along_with_subbed(self) -> None:
+        result = self.attempt_unsubscribe_of_principal(
+            query_count=17,
+            target_users=[self.example_user("cordelia"), self.example_user("iago")],
+            is_realm_admin=True,
+            is_subbed=True,
+            invite_only=False,
+            target_users_subbed=False,
+        )
+        json = self.assert_json_success(result)
+        self.assert_length(json["removed"], 1)
+        self.assert_length(json["not_removed"], 1)
+
     def test_remove_already_not_subbed(self) -> None:
         """
         Trying to unsubscribe someone who already isn't subscribed to a stream
         fails gracefully.
         """
         result = self.attempt_unsubscribe_of_principal(
-            query_count=11,
+            query_count=9,
             target_users=[self.example_user("cordelia")],
             is_realm_admin=True,
             is_subbed=False,