users: Send stream creation/deletion events on role change.

We now send stream creation and stream deletion events on
changing a user's role because a user can gain or lose
access to some streams on changing their role.

api_docs/changelog.md

@@ -24,6 +24,9 @@ format used by the Zulip server that they are interacting with.
 
 * [`POST /register`](/api/register-queue): `streams` field in the response
   now included web-public streams as well.
+* [`GET /events`](/api/get-events): Events for stream creation and deletion
+  are now sent to the user if they gain or lose access to some streams due
+  to change in their role.
 
 **Feature level 204**
 

version.py

@@ -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 = 204
+API_FEATURE_LEVEL = 205
 
 # 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

zerver/actions/users.py

@@ -19,6 +19,9 @@ from zerver.lib.cache import bot_dict_fields
 from zerver.lib.create_user import create_user
 from zerver.lib.send_email import clear_scheduled_emails
 from zerver.lib.sessions import delete_user_sessions
+from zerver.lib.stream_subscription import bulk_get_subscriber_peer_info
+from zerver.lib.stream_traffic import get_streams_traffic
+from zerver.lib.streams import get_streams_for_user, stream_to_dict
 from zerver.lib.user_counts import realm_user_count_by_role
 from zerver.lib.user_groups import get_system_user_group_for_user
 from zerver.lib.users import get_active_bots_owned_by_user
@@ -28,6 +31,7 @@ from zerver.models import (
     RealmAuditLog,
     Recipient,
     Service,
+    Stream,
     Subscription,
     UserGroupMembership,
     UserProfile,
@@ -309,6 +313,59 @@ def do_deactivate_user(
             )
 
 
+def send_stream_events_for_role_update(
+    user_profile: UserProfile, old_accessible_streams: List[Stream]
+) -> None:
+    current_accessible_streams = get_streams_for_user(
+        user_profile,
+        include_all_active=user_profile.is_realm_admin,
+        include_web_public=True,
+    )
+
+    old_accessible_stream_ids = {stream.id for stream in old_accessible_streams}
+    current_accessible_stream_ids = {stream.id for stream in current_accessible_streams}
+
+    now_accessible_stream_ids = current_accessible_stream_ids - old_accessible_stream_ids
+    if now_accessible_stream_ids:
+        recent_traffic = get_streams_traffic(now_accessible_stream_ids, user_profile.realm)
+
+        now_accessible_streams = [
+            stream
+            for stream in current_accessible_streams
+            if stream.id in now_accessible_stream_ids
+        ]
+        event = dict(
+            type="stream",
+            op="create",
+            streams=[stream_to_dict(stream, recent_traffic) for stream in now_accessible_streams],
+        )
+        send_event_on_commit(user_profile.realm, event, [user_profile.id])
+
+        subscriber_peer_info = bulk_get_subscriber_peer_info(
+            user_profile.realm, now_accessible_streams
+        )
+        for stream_id, stream_subscriber_set in subscriber_peer_info.subscribed_ids.items():
+            peer_add_event = dict(
+                type="subscription",
+                op="peer_add",
+                stream_ids=[stream_id],
+                user_ids=sorted(stream_subscriber_set),
+            )
+            send_event_on_commit(user_profile.realm, peer_add_event, [user_profile.id])
+
+    now_inaccessible_stream_ids = old_accessible_stream_ids - current_accessible_stream_ids
+    if now_inaccessible_stream_ids:
+        now_inaccessible_streams = [
+            stream for stream in old_accessible_streams if stream.id in now_inaccessible_stream_ids
+        ]
+        event = dict(
+            type="stream",
+            op="delete",
+            streams=[stream_to_dict(stream) for stream in now_inaccessible_streams],
+        )
+        send_event_on_commit(user_profile.realm, event, [user_profile.id])
+
+
 @transaction.atomic(durable=True)
 def do_change_user_role(
     user_profile: UserProfile, value: int, *, acting_user: Optional[UserProfile]
@@ -316,6 +373,12 @@ def do_change_user_role(
     old_value = user_profile.role
     old_system_group = get_system_user_group_for_user(user_profile)
 
+    previously_accessible_streams = get_streams_for_user(
+        user_profile,
+        include_web_public=True,
+        include_all_active=user_profile.is_realm_admin,
+    )
+
     user_profile.role = value
     user_profile.save(update_fields=["role"])
     RealmAuditLog.objects.create(
@@ -372,6 +435,8 @@ def do_change_user_role(
             user_profile.realm, [user_profile.id], acting_user=acting_user
         )
 
+    send_stream_events_for_role_update(user_profile, previously_accessible_streams)
+
 
 def do_make_user_billing_admin(user_profile: UserProfile) -> None:
     user_profile.is_billing_admin = True

zerver/lib/events.py

@@ -882,26 +882,6 @@ def apply_event(
                     state["can_subscribe_other_users"] = user_profile.can_subscribe_other_users()
                     state["can_invite_others_to_realm"] = user_profile.can_invite_others_to_realm()
 
-                    # TODO: Probably rather than writing the perfect
-                    # live-update code for the case of racing with the
-                    # current user changing roles, we should just do a
-                    # full refetch.
-                    if "never_subscribed" in state:
-                        sub_info = gather_subscriptions_helper(
-                            user_profile,
-                            include_subscribers=include_subscribers,
-                        )
-                        state["subscriptions"] = sub_info.subscriptions
-                        state["unsubscribed"] = sub_info.unsubscribed
-                        state["never_subscribed"] = sub_info.never_subscribed
-
-                    if "streams" in state:
-                        state["streams"] = do_get_streams(
-                            user_profile,
-                            include_web_public=True,
-                            include_all_active=user_profile.is_realm_admin,
-                        )
-
                     if state["is_guest"]:
                         state["realm_default_streams"] = []
                     else:

zerver/openapi/zulip.yaml

@@ -1165,12 +1165,16 @@ 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 [guest user](/help/roles-and-permissions)
+                                when a private stream is made public, their role changes or when
-                                is subscribed to a public (or private) stream.
+                                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 205), this event was
+                                not sent when a user gained access to a stream due to role change.
+
                                 **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.
@@ -1222,6 +1226,13 @@ paths:
                             - type: object
                               description: |
                                 Event sent to all users who can see a stream when it is deactivated.
+
+                                This event is also sent when a user loses access to a stream they
+                                previously [could access](/help/stream-permissions) when their roles
+                                changes.
+
+                                **Changes**: Prior to Zulip 8.0 (feature level 205), this event was
+                                not sent when a user lost access to a stream due to role change.
                               properties:
                                 id:
                                   $ref: "#/components/schemas/EventIdSchema"

zerver/tests/test_events.py

@@ -1859,10 +1859,19 @@ class NormalActionsTest(BaseAction):
         self.user_profile.refresh_from_db()
 
         do_change_user_role(self.user_profile, UserProfile.ROLE_MEMBER, acting_user=None)
+
+        self.make_stream("Test private stream", invite_only=True)
+        self.subscribe(self.example_user("othello"), "Test private stream")
+
         for role in [UserProfile.ROLE_REALM_ADMINISTRATOR, UserProfile.ROLE_MEMBER]:
+            if role == UserProfile.ROLE_REALM_ADMINISTRATOR:
+                num_events = 6
+            else:
+                num_events = 5
+
             events = self.verify_action(
                 lambda: do_change_user_role(self.user_profile, role, acting_user=None),
-                num_events=4,
+                num_events=num_events,
             )
             check_realm_user_update("events[0]", events[0], "role")
             self.assertEqual(events[0]["person"]["role"], role)
@@ -1872,8 +1881,11 @@ class NormalActionsTest(BaseAction):
 
             if role == UserProfile.ROLE_REALM_ADMINISTRATOR:
                 check_user_group_remove_members("events[3]", events[3])
+                check_stream_create("events[4]", events[4])
+                check_subscription_peer_add("events[5]", events[5])
             else:
                 check_user_group_add_members("events[3]", events[3])
+                check_stream_delete("events[4]", events[4])
 
     def test_change_is_billing_admin(self) -> None:
         reset_email_visibility_to_everyone_in_zulip_realm()
@@ -1896,10 +1908,18 @@ class NormalActionsTest(BaseAction):
         self.user_profile.refresh_from_db()
 
         do_change_user_role(self.user_profile, UserProfile.ROLE_MEMBER, acting_user=None)
+
+        self.make_stream("Test private stream", invite_only=True)
+        self.subscribe(self.example_user("othello"), "Test private stream")
+
         for role in [UserProfile.ROLE_REALM_OWNER, UserProfile.ROLE_MEMBER]:
+            if role == UserProfile.ROLE_REALM_OWNER:
+                num_events = 6
+            else:
+                num_events = 5
             events = self.verify_action(
                 lambda: do_change_user_role(self.user_profile, role, acting_user=None),
-                num_events=4,
+                num_events=num_events,
             )
             check_realm_user_update("events[0]", events[0], "role")
             self.assertEqual(events[0]["person"]["role"], role)
@@ -1909,8 +1929,11 @@ class NormalActionsTest(BaseAction):
 
             if role == UserProfile.ROLE_REALM_OWNER:
                 check_user_group_remove_members("events[3]", events[3])
+                check_stream_create("events[4]", events[4])
+                check_subscription_peer_add("events[5]", events[5])
             else:
                 check_user_group_add_members("events[3]", events[3])
+                check_stream_delete("events[4]", events[4])
 
     def test_change_is_moderator(self) -> None:
         reset_email_visibility_to_everyone_in_zulip_realm()
@@ -1950,9 +1973,16 @@ class NormalActionsTest(BaseAction):
 
         do_change_user_role(self.user_profile, UserProfile.ROLE_MEMBER, acting_user=None)
         for role in [UserProfile.ROLE_GUEST, UserProfile.ROLE_MEMBER]:
+            if role == UserProfile.ROLE_MEMBER:
+                # When changing role from guest to member, peer_add events are also sent
+                # to make sure the subscribers info is provided to the clients for the
+                # streams added by stream creation event.
+                num_events = 7
+            else:
+                num_events = 5
             events = self.verify_action(
                 lambda: do_change_user_role(self.user_profile, role, acting_user=None),
-                num_events=4,
+                num_events=num_events,
             )
             check_realm_user_update("events[0]", events[0], "role")
             self.assertEqual(events[0]["person"]["role"], role)
@@ -1962,8 +1992,12 @@ class NormalActionsTest(BaseAction):
 
             if role == UserProfile.ROLE_GUEST:
                 check_user_group_remove_members("events[3]", events[3])
+                check_stream_delete("events[4]", events[4])
             else:
                 check_user_group_add_members("events[3]", events[3])
+                check_stream_create("events[4]", events[4])
+                check_subscription_peer_add("events[5]", events[5])
+                check_subscription_peer_add("events[6]", events[6])
 
     def test_change_notification_settings(self) -> None:
         for notification_setting in self.user_profile.notification_setting_types:

zerver/tests/test_users.py

@@ -220,7 +220,7 @@ class PermissionTest(ZulipTestCase):
         self.assertFalse(othello_dict["is_owner"])
 
         req = dict(role=UserProfile.ROLE_REALM_OWNER)
-        with self.capture_send_event_calls(expected_num_events=4) as events:
+        with self.capture_send_event_calls(expected_num_events=6) as events:
             result = self.client_patch(f"/json/users/{othello.id}", req)
         self.assert_json_success(result)
         owner_users = realm.get_human_owner_users()
@@ -230,7 +230,7 @@ class PermissionTest(ZulipTestCase):
         self.assertEqual(person["role"], UserProfile.ROLE_REALM_OWNER)
 
         req = dict(role=UserProfile.ROLE_MEMBER)
-        with self.capture_send_event_calls(expected_num_events=4) as events:
+        with self.capture_send_event_calls(expected_num_events=5) as events:
             result = self.client_patch(f"/json/users/{othello.id}", req)
         self.assert_json_success(result)
         owner_users = realm.get_human_owner_users()
@@ -281,7 +281,7 @@ class PermissionTest(ZulipTestCase):
         # Giveth
         req = dict(role=orjson.dumps(UserProfile.ROLE_REALM_ADMINISTRATOR).decode())
 
-        with self.capture_send_event_calls(expected_num_events=4) as events:
+        with self.capture_send_event_calls(expected_num_events=6) as events:
             result = self.client_patch(f"/json/users/{othello.id}", req)
         self.assert_json_success(result)
         admin_users = realm.get_human_admin_users()
@@ -292,7 +292,7 @@ class PermissionTest(ZulipTestCase):
 
         # Taketh away
         req = dict(role=orjson.dumps(UserProfile.ROLE_MEMBER).decode())
-        with self.capture_send_event_calls(expected_num_events=4) as events:
+        with self.capture_send_event_calls(expected_num_events=5) as events:
             result = self.client_patch(f"/json/users/{othello.id}", req)
         self.assert_json_success(result)
         admin_users = realm.get_human_admin_users()
@@ -518,9 +518,42 @@ class PermissionTest(ZulipTestCase):
         )
 
         req = dict(role=orjson.dumps(new_role).decode())
+
+        # The basic events sent in all cases on changing role are - one event
+        # for changing role and one event each for adding and removing user
+        # from system user group.
         num_events = 3
+
         if UserProfile.ROLE_MEMBER in [old_role, new_role]:
-            num_events = 4
+            # There is one additional event for adding/removing user from
+            # the "Full members" group as well.
+            num_events += 1
+
+        if new_role == UserProfile.ROLE_GUEST:
+            # There is one additional event deleting the unsubscribed public
+            # streams that the user will not able to access after becoming guest.
+            num_events += 1
+
+        if old_role == UserProfile.ROLE_GUEST:
+            # User will receive one event for creation of unsubscribed public
+            # (and private, if the new role is owner or admin) streams that
+            # they did not have access to previously and 3 more peer_add
+            # events for each of the public stream.
+            if new_role in [UserProfile.ROLE_REALM_ADMINISTRATOR, UserProfile.ROLE_REALM_OWNER]:
+                # If the new role is owner or admin, the peer_add event will be
+                # sent for one private stream as well.
+                num_events += 5
+            else:
+                num_events += 4
+        elif new_role in [
+            UserProfile.ROLE_REALM_ADMINISTRATOR,
+            UserProfile.ROLE_REALM_OWNER,
+        ] and old_role not in [UserProfile.ROLE_REALM_ADMINISTRATOR, UserProfile.ROLE_REALM_OWNER]:
+            # If old_role is not guest and user's role is changed to admin or owner from moderator
+            # or member, then the user gains access to unsubscribed private streams and thus
+            # receives one event for stream creation and one peer_add event for it.
+            num_events += 2
+
         with self.capture_send_event_calls(expected_num_events=num_events) as events:
             result = self.client_patch(f"/json/users/{user_profile.id}", req)
         self.assert_json_success(result)