diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py index c108b9e171..19f357b2a2 100644 --- a/zerver/lib/actions.py +++ b/zerver/lib/actions.py @@ -1669,12 +1669,31 @@ def bulk_add_subscriptions(streams, users): sub_tuples_by_user[sub.user_profile.id].append((sub, stream)) new_streams.add((sub.user_profile.id, stream.id)) + # We now send several types of events to notify browsers. The + # first batch is notifications to users on invite-only streams + # that the stream exists. + for stream in streams: + new_users = [user for user in users if (user.id, stream.id) in new_streams] + + # Users newly added to invite-only streams need a `create` + # notification, since they didn't have the invite-only stream + # in their browser yet. + if stream.invite_only: + event = dict(type="stream", op="create", + streams=[stream.to_dict()]) + send_event(event, [user.id for user in new_users]) + + # The second batch is events for the users themselves that they + # were subscribed to the new streams. for user_profile in users: if len(sub_tuples_by_user[user_profile.id]) == 0: continue sub_pairs = sub_tuples_by_user[user_profile.id] notify_subscriptions_added(user_profile, sub_pairs, fetch_stream_subscriber_emails) + # The second batch is events for other users who are tracking the + # subscribers lists of streams in their browser; everyone for + # public streams and only existing subscribers for private streams. for stream in streams: if stream.realm.is_zephyr_mirror_realm and not stream.invite_only: continue diff --git a/zerver/tests/test_subs.py b/zerver/tests/test_subs.py index 7287c39295..5e5b8ed43e 100644 --- a/zerver/tests/test_subs.py +++ b/zerver/tests/test_subs.py @@ -40,7 +40,7 @@ from zerver.lib.actions import ( gather_subscriptions_helper, bulk_add_subscriptions, bulk_remove_subscriptions, gather_subscriptions, get_default_streams_for_realm, get_realm, get_stream, get_user_profile_by_email, set_default_streams, get_subscription, - create_streams_if_needed, active_user_ids + create_stream_if_needed, create_streams_if_needed, active_user_ids ) from zerver.views.streams import ( @@ -1548,6 +1548,49 @@ class SubscriptionAPITest(ZulipTestCase): self.assertEqual(add_peer_event['event']['op'], 'peer_add') self.assertEqual(add_peer_event['event']['user_id'], user_profile.id) + def test_private_stream_subscription(self): + # type: () -> None + realm = get_realm("zulip") + + # Create a private stream with Hamlet subscribed + stream_name = "private" + (stream, _) = create_stream_if_needed(realm, stream_name, invite_only=True) + + existing_email = "hamlet@zulip.com" + existing_user_profile = get_user_profile_by_email(existing_email) + bulk_add_subscriptions([stream], [existing_user_profile]) + + # Now subscribe Cordelia to the stream, capturing events + email = 'cordelia@zulip.com' + user_profile = get_user_profile_by_email(email) + + events = [] + with tornado_redirected_to_list(events): + bulk_add_subscriptions([stream], [user_profile]) + + self.assert_length(events, 3) + create_event, add_event, add_peer_event = events + + self.assertEqual(create_event['event']['type'], 'stream') + self.assertEqual(create_event['event']['op'], 'create') + self.assertEqual(create_event['users'], [user_profile.id]) + self.assertEqual(create_event['event']['streams'][0]['name'], stream_name) + + self.assertEqual(add_event['event']['type'], 'subscription') + self.assertEqual(add_event['event']['op'], 'add') + self.assertEqual(add_event['users'], [user_profile.id]) + self.assertEqual( + set(add_event['event']['subscriptions'][0]['subscribers']), + set([user_profile.email, existing_user_profile.email]) + ) + + # We don't send a peer_add event to othello + self.assertNotIn(user_profile.id, add_peer_event['users']) + self.assertEqual(len(add_peer_event['users']), 1) + self.assertEqual(add_peer_event['event']['type'], 'subscription') + self.assertEqual(add_peer_event['event']['op'], 'peer_add') + self.assertEqual(add_peer_event['event']['user_id'], user_profile.id) + def test_users_getting_add_peer_event(self): # type: () -> None """