subs: Send create event to new subscribers of invite-only streams.

This fixes a regression introduced by our migration to track
subscribers for all public streams, where now users who are added to
an invite-only stream were receiving a mark_subscribed event
for a stream their browser didn't know existed, causing an exception.

To fix this, we now send a stream create event to the browser just
before the user receives the notification that it was added to the
invite-only stream.
This commit is contained in:
Tim Abbott
2017-01-28 16:21:31 -08:00
parent f665980079
commit 153418de38
2 changed files with 63 additions and 1 deletions

View File

@@ -1669,12 +1669,31 @@ def bulk_add_subscriptions(streams, users):
sub_tuples_by_user[sub.user_profile.id].append((sub, stream)) sub_tuples_by_user[sub.user_profile.id].append((sub, stream))
new_streams.add((sub.user_profile.id, stream.id)) 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: for user_profile in users:
if len(sub_tuples_by_user[user_profile.id]) == 0: if len(sub_tuples_by_user[user_profile.id]) == 0:
continue continue
sub_pairs = sub_tuples_by_user[user_profile.id] sub_pairs = sub_tuples_by_user[user_profile.id]
notify_subscriptions_added(user_profile, sub_pairs, fetch_stream_subscriber_emails) 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: for stream in streams:
if stream.realm.is_zephyr_mirror_realm and not stream.invite_only: if stream.realm.is_zephyr_mirror_realm and not stream.invite_only:
continue continue

View File

@@ -40,7 +40,7 @@ from zerver.lib.actions import (
gather_subscriptions_helper, bulk_add_subscriptions, bulk_remove_subscriptions, gather_subscriptions_helper, bulk_add_subscriptions, bulk_remove_subscriptions,
gather_subscriptions, get_default_streams_for_realm, get_realm, get_stream, gather_subscriptions, get_default_streams_for_realm, get_realm, get_stream,
get_user_profile_by_email, set_default_streams, get_subscription, 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 ( 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']['op'], 'peer_add')
self.assertEqual(add_peer_event['event']['user_id'], user_profile.id) 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): def test_users_getting_add_peer_event(self):
# type: () -> None # type: () -> None
""" """