refactor: Extract send_peer_remove_event().

This prevents leaking some variables into an already
cluttered function.

We also add test coverage for what's now an
early-exit condition in the new function--we exempt
public MIT streams from these events.
This commit is contained in:
Steve Howell
2018-08-21 17:20:54 +00:00
committed by Tim Abbott
parent 092bb6a728
commit a6bc3886e6
2 changed files with 15 additions and 2 deletions

View File

@@ -2790,9 +2790,9 @@ def bulk_remove_subscriptions(users: Iterable[UserProfile],
all_subscribers_by_stream = get_user_ids_for_streams(streams=streams) all_subscribers_by_stream = get_user_ids_for_streams(streams=streams)
for stream in streams: def send_peer_remove_event(stream: Stream) -> None:
if stream.is_in_zephyr_realm and not stream.invite_only: if stream.is_in_zephyr_realm and not stream.invite_only:
continue return
altered_users = altered_user_dict[stream.id] altered_users = altered_user_dict[stream.id]
altered_user_ids = [u.id for u in altered_users] altered_user_ids = [u.id for u in altered_users]
@@ -2813,6 +2813,9 @@ def bulk_remove_subscriptions(users: Iterable[UserProfile],
user_id=removed_user.id) user_id=removed_user.id)
send_event(event, peer_user_ids) send_event(event, peer_user_ids)
for stream in streams:
send_peer_remove_event(stream=stream)
new_vacant_streams = [stream for stream in new_vacant_streams = [stream for stream in
set(occupied_streams_before) - set(occupied_streams_after)] set(occupied_streams_before) - set(occupied_streams_after)]
new_vacant_private_streams = [stream for stream in new_vacant_streams new_vacant_private_streams = [stream for stream in new_vacant_streams

View File

@@ -2426,6 +2426,16 @@ class SubscriptionAPITest(ZulipTestCase):
self.assert_length(events, 0) self.assert_length(events, 0)
self.assert_length(queries, 9) self.assert_length(queries, 9)
events = []
with tornado_redirected_to_list(events):
bulk_remove_subscriptions(
users=[mit_user],
streams=streams,
acting_client=get_client('website'),
)
self.assert_length(events, 0)
def test_bulk_subscribe_many(self) -> None: def test_bulk_subscribe_many(self) -> None:
# Create a whole bunch of streams # Create a whole bunch of streams