diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py index 1a9a48c4f1..d02500b1d5 100644 --- a/zerver/lib/actions.py +++ b/zerver/lib/actions.py @@ -4184,6 +4184,7 @@ SubAndRemovedT = Tuple[List[Tuple[UserProfile, Stream]], List[Tuple[UserProfile, 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 bulk_remove_subscriptions( 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 bulk_remove_subscriptions( 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 bulk_remove_subscriptions( queue_json_publish("deferred_work", event) send_peer_remove_events( - realm=our_realm, + realm=realm, streams=streams, altered_user_dict=altered_user_dict, ) diff --git a/zerver/lib/test_classes.py b/zerver/lib/test_classes.py index b285888e88..aa3214d299 100644 --- a/zerver/lib/test_classes.py +++ b/zerver/lib/test_classes.py @@ -1076,8 +1076,9 @@ Output: 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( diff --git a/zerver/management/commands/merge_streams.py b/zerver/management/commands/merge_streams.py index 97222d3804..1d39610d25 100644 --- a/zerver/management/commands/merge_streams.py +++ b/zerver/management/commands/merge_streams.py @@ -74,6 +74,7 @@ class Command(ZulipBaseCommand): 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, diff --git a/zerver/management/commands/remove_users_from_stream.py b/zerver/management/commands/remove_users_from_stream.py index a90ead0c2f..f1875982c7 100644 --- a/zerver/management/commands/remove_users_from_stream.py +++ b/zerver/management/commands/remove_users_from_stream.py @@ -25,7 +25,7 @@ class Command(ZulipBaseCommand): 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} diff --git a/zerver/tests/test_audit_log.py b/zerver/tests/test_audit_log.py index 8988d1d3ec..c45c2297ea 100644 --- a/zerver/tests/test_audit_log.py +++ b/zerver/tests/test_audit_log.py @@ -277,6 +277,7 @@ class TestRealmAuditLog(ZulipTestCase): 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 @@ class TestRealmAuditLog(ZulipTestCase): 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, diff --git a/zerver/tests/test_events.py b/zerver/tests/test_events.py index bcd83b4d65..30de1f3016 100644 --- a/zerver/tests/test_events.py +++ b/zerver/tests/test_events.py @@ -1920,12 +1920,12 @@ class NormalActionsTest(BaseAction): 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 @@ class SubscribeActionTest(BaseAction): ) 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 @@ class SubscribeActionTest(BaseAction): 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 @@ class SubscribeActionTest(BaseAction): 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, diff --git a/zerver/tests/test_subs.py b/zerver/tests/test_subs.py index d012b6a9b3..43b30ef2a3 100644 --- a/zerver/tests/test_subs.py +++ b/zerver/tests/test_subs.py @@ -3976,6 +3976,8 @@ class SubscriptionAPITest(ZulipTestCase): 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 @@ class SubscriptionAPITest(ZulipTestCase): 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 @@ class SubscriptionAPITest(ZulipTestCase): with self.tornado_redirected_to_list(events, expected_num_events=0): bulk_remove_subscriptions( + realm, users=[mit_user], streams=streams, acting_user=None, diff --git a/zerver/views/streams.py b/zerver/views/streams.py index 3da57633f7..f79ef1eab9 100644 --- a/zerver/views/streams.py +++ b/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: