refactor: Pass realm to bulk_remove_subscriptions.

We made a very similar change to bulk_add_subscriptions
earlier in the year.

Author: showell

zerver/lib/actions.py

@@ -4184,6 +4184,7 @@ def notify_subscriptions_removed(
 
 
 def bulk_remove_subscriptions(
+    realm: Realm,
     users: Iterable[UserProfile],
     streams: Iterable[Stream],
     *,
@@ -4193,6 +4194,13 @@ def bulk_remove_subscriptions(
     users = list(users)
     streams = list(streams)
 
+    # Sanity check our callers
+    for stream in streams:
+        assert stream.realm_id == realm.id
+
+    for user in users:
+        assert user.realm_id == realm.id
+
     stream_dict = {stream.id: stream for stream in streams}
 
     existing_subs_by_user = get_bulk_stream_subscriber_info(users, streams)
@@ -4226,16 +4234,14 @@ def get_non_subscribed_subs() -> List[Tuple[UserProfile, Stream]]:
             subs_to_deactivate.append(sub_info)
             sub_ids_to_deactivate.append(sub_info.sub.id)
 
-    our_realm = users[0].realm
-
     # We do all the database changes in a transaction to ensure
     # RealmAuditLog entries are atomically created when making changes.
     with transaction.atomic():
-        occupied_streams_before = list(get_occupied_streams(our_realm))
+        occupied_streams_before = list(get_occupied_streams(realm))
         Subscription.objects.filter(
             id__in=sub_ids_to_deactivate,
         ).update(active=False)
-        occupied_streams_after = list(get_occupied_streams(our_realm))
+        occupied_streams_after = list(get_occupied_streams(realm))
 
         # Log subscription activities in RealmAuditLog
         event_time = timezone_now()
@@ -4266,7 +4272,7 @@ def get_non_subscribed_subs() -> List[Tuple[UserProfile, Stream]]:
     for user_profile in users:
         if len(streams_by_user[user_profile.id]) == 0:
             continue
-        notify_subscriptions_removed(our_realm, user_profile, streams_by_user[user_profile.id])
+        notify_subscriptions_removed(realm, user_profile, streams_by_user[user_profile.id])
 
         event = {
             "type": "mark_stream_messages_as_read",
@@ -4276,7 +4282,7 @@ def get_non_subscribed_subs() -> List[Tuple[UserProfile, Stream]]:
         queue_json_publish("deferred_work", event)
 
     send_peer_remove_events(
-        realm=our_realm,
+        realm=realm,
         streams=streams,
         altered_user_dict=altered_user_dict,
     )

zerver/lib/test_classes.py

@@ -1076,8 +1076,9 @@ def subscribe(self, user_profile: UserProfile, stream_name: str) -> Stream:
         return stream
 
     def unsubscribe(self, user_profile: UserProfile, stream_name: str) -> None:
+        realm = user_profile.realm
         stream = get_stream(stream_name, user_profile.realm)
-        bulk_remove_subscriptions([user_profile], [stream], acting_user=None)
+        bulk_remove_subscriptions(realm, [user_profile], [stream], acting_user=None)
 
     # Subscribe to a stream by making an API request
     def common_subscribe_to_streams(

zerver/management/commands/merge_streams.py

@@ -74,6 +74,7 @@ def handle(self, *args: Any, **options: str) -> None:
         if len(subs_to_deactivate) > 0:
             print(f"Deactivating {len(subs_to_deactivate)} subscriptions")
             bulk_remove_subscriptions(
+                realm,
                 [sub.user_profile for sub in subs_to_deactivate],
                 [stream_to_destroy],
                 acting_user=None,

zerver/management/commands/remove_users_from_stream.py

@@ -25,7 +25,7 @@ def handle(self, *args: Any, **options: Any) -> None:
         stream_name = options["stream"].strip()
         stream = get_stream(stream_name, realm)
 
-        result = bulk_remove_subscriptions(user_profiles, [stream], acting_user=None)
+        result = bulk_remove_subscriptions(realm, user_profiles, [stream], acting_user=None)
         not_subscribed = result[1]
         not_subscribed_users = {tup[0] for tup in not_subscribed}
 

zerver/tests/test_audit_log.py

@@ -277,6 +277,7 @@ def test_subscriptions(self) -> None:
         now = timezone_now()
 
         user = self.example_user("hamlet")
+        realm = user.realm
         stream = self.make_stream("test_stream")
         acting_user = self.example_user("iago")
         bulk_add_subscriptions(user.realm, [stream], [user], acting_user=acting_user)
@@ -293,7 +294,7 @@ def test_subscriptions(self) -> None:
         self.assertEqual(modified_stream.id, stream.id)
         self.assertEqual(subscription_creation_logs[0].modified_user, user)
 
-        bulk_remove_subscriptions([user], [stream], acting_user=acting_user)
+        bulk_remove_subscriptions(realm, [user], [stream], acting_user=acting_user)
         subscription_deactivation_logs = RealmAuditLog.objects.filter(
             event_type=RealmAuditLog.SUBSCRIPTION_DEACTIVATED,
             event_time__gte=now,

zerver/tests/test_events.py

@@ -1920,12 +1920,12 @@ def test_subscribe_other_user_never_subscribed(self) -> None:
             check_subscription_peer_add("events[1]", events[1])
 
     def test_remove_other_user_never_subscribed(self) -> None:
-        self.subscribe(self.example_user("othello"), "test_stream")
+        othello = self.example_user("othello")
+        realm = othello.realm
+        self.subscribe(othello, "test_stream")
         stream = get_stream("test_stream", self.user_profile.realm)
 
-        action = lambda: bulk_remove_subscriptions(
-            [self.example_user("othello")], [stream], acting_user=None
-        )
+        action = lambda: bulk_remove_subscriptions(realm, [othello], [stream], acting_user=None)
         events = self.verify_action(action)
         check_subscription_peer_remove("events[0]", events[0])
 
@@ -2476,13 +2476,15 @@ def do_test_subscribe_events(self, include_subscribers: bool) -> None:
         )
         check_subscription_peer_add("events[0]", events[0])
 
+        hamlet = self.example_user("hamlet")
+        iago = self.example_user("iago")
+        othello = self.example_user("othello")
+        realm = othello.realm
         stream = get_stream("test_stream", self.user_profile.realm)
 
         # Now remove the first user, to test the normal unsubscribe flow and
         # 'peer_remove' event for subscribed streams.
-        action = lambda: bulk_remove_subscriptions(
-            [self.example_user("othello")], [stream], acting_user=None
-        )
+        action = lambda: bulk_remove_subscriptions(realm, [othello], [stream], acting_user=None)
         events = self.verify_action(
             action,
             include_subscribers=include_subscribers,
@@ -2491,9 +2493,7 @@ def do_test_subscribe_events(self, include_subscribers: bool) -> None:
         check_subscription_peer_remove("events[0]", events[0])
 
         # Now remove the user himself, to test the 'remove' event flow
-        action = lambda: bulk_remove_subscriptions(
-            [self.example_user("hamlet")], [stream], acting_user=None
-        )
+        action = lambda: bulk_remove_subscriptions(realm, [hamlet], [stream], acting_user=None)
         events = self.verify_action(
             action, include_subscribers=include_subscribers, include_streams=False, num_events=2
         )
@@ -2515,9 +2515,7 @@ def do_test_subscribe_events(self, include_subscribers: bool) -> None:
         check_subscription_peer_add("events[0]", events[0])
 
         # Remove the user to test 'peer_remove' event flow for unsubscribed stream.
-        action = lambda: bulk_remove_subscriptions(
-            [self.example_user("iago")], [stream], acting_user=None
-        )
+        action = lambda: bulk_remove_subscriptions(realm, [iago], [stream], acting_user=None)
         events = self.verify_action(
             action,
             include_subscribers=include_subscribers,

zerver/tests/test_subs.py

@@ -3976,6 +3976,8 @@ def test_users_getting_remove_peer_event(self) -> None:
         user5 = self.example_user("AARON")
         guest = self.example_user("polonius")
 
+        realm = user1.realm
+
         stream1 = self.make_stream("stream1")
         stream2 = self.make_stream("stream2")
         stream3 = self.make_stream("stream3")
@@ -4000,6 +4002,7 @@ def test_users_getting_remove_peer_event(self) -> None:
             with queries_captured() as query_count:
                 with cache_tries_captured() as cache_count:
                     bulk_remove_subscriptions(
+                        realm,
                         [user1, user2],
                         [stream1, stream2, stream3, private],
                         acting_user=None,
@@ -4067,6 +4070,7 @@ def test_bulk_subscribe_MIT(self) -> None:
 
         with self.tornado_redirected_to_list(events, expected_num_events=0):
             bulk_remove_subscriptions(
+                realm,
                 users=[mit_user],
                 streams=streams,
                 acting_user=None,

zerver/views/streams.py

@@ -405,6 +405,7 @@ def remove_subscriptions_backend(
     ),
 ) -> HttpResponse:
 
+    realm = user_profile.realm
     removing_someone_else = check_if_removing_someone_else(user_profile, principals)
 
     streams_as_dict: List[StreamDict] = []
@@ -424,7 +425,7 @@ def remove_subscriptions_backend(
 
     result: Dict[str, List[str]] = dict(removed=[], not_removed=[])
     (removed, not_subscribed) = bulk_remove_subscriptions(
-        people_to_unsub, streams, acting_user=user_profile
+        realm, people_to_unsub, streams, acting_user=user_profile
     )
 
     for (subscriber, removed_stream) in removed: