streams: Send stream creation events when subscribing guests.

We did not send the stream creation events when subscribing
guests to public streams while we do send them when subscribing
non-admin users to private streams.

This commit adds code to send the stream creation events when
subscribing guests to public streams, so the clients can know
that the stream exists and fixes the bug where client tries
to process a subscription add event for a stream which it does
not know about.
This commit is contained in:
Sahil Batra 2023-06-30 16:44:49 +05:30 committed by Tim Abbott
parent 2c02d94b85
commit 75b61a8261
6 changed files with 70 additions and 8 deletions

View File

@ -20,6 +20,13 @@ format used by the Zulip server that they are interacting with.
## Changes in Zulip 8.0
**Feature level 192**
* [`GET /events`](/api/get-events): Stream creation events are now
sent when guest users gain access to a public stream by being
subscribed. Guest users previously only received these events when
subscribed to private streams.
**Feature level 191**
* [`GET /events`](/api/get-events), [`POST /register`](/api/register-queue),

View File

@ -33,7 +33,7 @@ DESKTOP_WARNING_VERSION = "5.9.3"
# Changes should be accompanied by documentation explaining what the
# new level means in api_docs/changelog.md, as well as "**Changes**"
# entries in the endpoint's documentation in `zulip.yaml`.
API_FEATURE_LEVEL = 191
API_FEATURE_LEVEL = 192
# Bump the minor PROVISION_VERSION to indicate that folks should provision
# only when going from an old version of the code to a newer version. Bump

View File

@ -345,14 +345,16 @@ def bulk_add_subs_to_db_with_logging(
RealmAuditLog.objects.bulk_create(all_subscription_logs)
def send_stream_creation_events_for_private_streams(
def send_stream_creation_events_for_previously_inaccessible_streams(
realm: Realm,
stream_dict: Dict[int, Stream],
altered_user_dict: Dict[int, Set[int]],
altered_guests: Set[int],
) -> None:
for stream_id, stream_users_ids in altered_user_dict.items():
stream = stream_dict[stream_id]
notify_user_ids = []
if not stream.is_public():
# Users newly added to invite-only streams
# need a `create` notification. The former, because
@ -362,6 +364,11 @@ def send_stream_creation_events_for_private_streams(
# Realm admins already have all created private streams.
realm_admin_ids = {user.id for user in realm.get_admin_users_and_bots()}
notify_user_ids = list(stream_users_ids - realm_admin_ids)
else:
# Guese users need a `create` notification for
# public streams as well because they need the stream
# to exist before they get the "subscribe" notification.
notify_user_ids = list(stream_users_ids & altered_guests)
if notify_user_ids:
send_stream_creation_event(realm, stream, notify_user_ids)
@ -538,8 +545,11 @@ def bulk_add_subscriptions(
)
altered_user_dict: Dict[int, Set[int]] = defaultdict(set)
altered_guests: Set[int] = set()
for sub_info in subs_to_add + subs_to_activate:
altered_user_dict[sub_info.stream.id].add(sub_info.user.id)
if sub_info.user.is_guest:
altered_guests.add(sub_info.user.id)
stream_dict = {stream.id: stream for stream in streams}
@ -555,10 +565,11 @@ def bulk_add_subscriptions(
# being subscribed; we can skip these notifications when this is
# being called from the new user creation flow.
if not from_user_creation:
send_stream_creation_events_for_private_streams(
send_stream_creation_events_for_previously_inaccessible_streams(
realm=realm,
stream_dict=stream_dict,
altered_user_dict=altered_user_dict,
altered_guests=altered_guests,
)
send_subscription_add_events(

View File

@ -1170,11 +1170,15 @@ paths:
This event is also sent when a user gains access to a stream they
previously [could not access](/help/stream-permissions), such as
when a private stream is made public, or when a user is subscribed
to a private stream.
when a private stream is made public, or when a [guest user](/help/roles-and-permissions)
is subscribed to a public (or private) stream.
Note that organization administrators who are not subscribed will
not be able to see content on the stream; just that it exists.
**Changes**: Prior to Zulip 8.0 (feature level 192), this event was
not sent when guest users gained access to a public stream by being
subscribed.
properties:
id:
$ref: "#/components/schemas/EventIdSchema"

View File

@ -3261,6 +3261,18 @@ class SubscribeActionTest(BaseAction):
10,
)
stream.invite_only = False
stream.save()
# Subscribe as a guest to a public stream.
self.user_profile = self.example_user("polonius")
action = lambda: bulk_add_subscriptions(
user_profile.realm, [stream], [self.user_profile], acting_user=None
)
events = self.verify_action(action, include_subscribers=include_subscribers, num_events=2)
check_stream_create("events[0]", events[0])
check_subscription_add("events[1]", events[1])
class DraftActionTest(BaseAction):
def do_enable_drafts_synchronization(self, user_profile: UserProfile) -> None:

View File

@ -4656,6 +4656,34 @@ class SubscriptionAPITest(ZulipTestCase):
([web_public_stream], [public_stream, private_stream]),
)
# Guest can be subscribed by other users.
normal_user = self.example_user("aaron")
with self.capture_send_event_calls(expected_num_events=6) as events:
self.common_subscribe_to_streams(
self.example_user("hamlet"),
["Denmark"],
dict(principals=orjson.dumps([guest_user.id, normal_user.id]).decode()),
)
# Verify that stream creation event is sent to guest user only.
stream_create_events = [
event
for event in events
if event["event"]["type"] == "stream" and event["event"]["op"] == "create"
]
self.assert_length(stream_create_events, 1)
self.assertEqual(stream_create_events[0]["users"], [guest_user.id])
# Verify that subscription add event is sent to both the users.
subscription_add_events = [
event
for event in events
if event["event"]["type"] == "subscription" and event["event"]["op"] == "add"
]
self.assert_length(subscription_add_events, 2)
self.assertEqual(subscription_add_events[0]["users"], [guest_user.id])
self.assertEqual(subscription_add_events[1]["users"], [normal_user.id])
def test_users_getting_add_peer_event(self) -> None:
"""
Check users getting add_peer_event is correct
@ -4781,7 +4809,7 @@ class SubscriptionAPITest(ZulipTestCase):
# Verify that peer_event events are never sent in Zephyr
# realm. This does generate stream creation events from
# send_stream_creation_events_for_private_streams.
# send_stream_creation_events_for_previously_inaccessible_streams.
with self.capture_send_event_calls(expected_num_events=num_streams + 1) as events:
with self.assert_database_query_count(num_streams + 12):
self.common_subscribe_to_streams(