From 4313648ca560fd69ad3327d61af6434c95d88f80 Mon Sep 17 00:00:00 2001 From: Evy Kassirer Date: Fri, 6 Jun 2025 20:59:54 -0700 Subject: [PATCH] streams: Add subscriber_count to page load data. --- api_docs/changelog.md | 9 ++++++ version.py | 2 +- zerver/actions/streams.py | 1 + zerver/lib/events.py | 5 ++++ zerver/lib/streams.py | 1 + zerver/lib/subscription_info.py | 6 ++++ zerver/lib/types.py | 4 +++ zerver/models/streams.py | 1 + zerver/openapi/zulip.yaml | 51 +++++++++++++++++++++++++++++++++ zerver/tests/test_events.py | 12 ++++++++ 10 files changed, 91 insertions(+), 1 deletion(-) diff --git a/api_docs/changelog.md b/api_docs/changelog.md index dd2f90cd82..4641270d89 100644 --- a/api_docs/changelog.md +++ b/api_docs/changelog.md @@ -20,6 +20,15 @@ format used by the Zulip server that they are interacting with. ## Changes in Zulip 11.0 +**Feature level 394** + +* [`POST /register`](/api/register-queue), [`GET + /events`](/api/get-events), [`GET /streams`](/api/get-streams), + [`GET /streams/{stream_id}`](/api/get-stream-by-id):: Added a new + field `subscriber_count` to Stream and Subscription objects with the + total number of non-deactivated users who are subscribed to the + channel. + **Feature level 393** * [`PATCH /messages/{message_id}`](/api/delete-message), diff --git a/version.py b/version.py index 7358ba9fda..b0908a0544 100644 --- a/version.py +++ b/version.py @@ -34,7 +34,7 @@ DESKTOP_WARNING_VERSION = "5.9.3" # new level means in api_docs/changelog.md, as well as "**Changes**" # entries in the endpoint's documentation in `zulip.yaml`. -API_FEATURE_LEVEL = 393 +API_FEATURE_LEVEL = 394 # 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 diff --git a/zerver/actions/streams.py b/zerver/actions/streams.py index 1f06f3004e..fca4b9a327 100644 --- a/zerver/actions/streams.py +++ b/zerver/actions/streams.py @@ -475,6 +475,7 @@ def send_subscription_add_events( rendered_description=stream_dict["rendered_description"], stream_id=stream_dict["stream_id"], stream_post_policy=stream_dict["stream_post_policy"], + subscriber_count=stream_dict["subscriber_count"], topics_policy=stream_dict["topics_policy"], # Computed fields not present in Stream.API_FIELDS is_announcement_only=stream_dict["is_announcement_only"], diff --git a/zerver/lib/events.py b/zerver/lib/events.py index 75d7c87925..08ac9f6733 100644 --- a/zerver/lib/events.py +++ b/zerver/lib/events.py @@ -1630,6 +1630,10 @@ def apply_event( if sub["stream_id"] == event["stream_id"]: sub[event["property"]] = event["value"] elif event["op"] == "peer_add": + # Note: We don't update subscriber_count here, since we + # have no way to know whether the added subscriber is + # already in our count or not. The opposite decision would + # be defensible, but this is less code. if include_subscribers: stream_ids = set(event["stream_ids"]) user_ids = set(event["user_ids"]) @@ -1644,6 +1648,7 @@ def apply_event( subscribers = set(sub["subscribers"]) | user_ids sub["subscribers"] = sorted(subscribers) elif event["op"] == "peer_remove": + # Note: We don't update subscriber_count here, as with peer_add. if include_subscribers: stream_ids = set(event["stream_ids"]) user_ids = set(event["user_ids"]) diff --git a/zerver/lib/streams.py b/zerver/lib/streams.py index 3459725221..f856d0b1ea 100644 --- a/zerver/lib/streams.py +++ b/zerver/lib/streams.py @@ -1538,6 +1538,7 @@ def stream_to_dict( stream_post_policy=stream_post_policy, is_announcement_only=stream_post_policy == Stream.STREAM_POST_POLICY_ADMINS, stream_weekly_traffic=stream_weekly_traffic, + subscriber_count=stream.subscriber_count, topics_policy=StreamTopicsPolicyEnum(stream.topics_policy).name, ) diff --git a/zerver/lib/subscription_info.py b/zerver/lib/subscription_info.py index 36bdb15a98..a4dd828263 100644 --- a/zerver/lib/subscription_info.py +++ b/zerver/lib/subscription_info.py @@ -144,6 +144,7 @@ def get_web_public_subs( stream_id=stream_id, stream_post_policy=stream_post_policy, stream_weekly_traffic=stream_weekly_traffic, + subscriber_count=stream.subscriber_count, topics_policy=StreamTopicsPolicyEnum(topics_policy).name, wildcard_mentions_notify=wildcard_mentions_notify, ) @@ -219,6 +220,7 @@ def build_stream_api_dict( stream_id=raw_stream_dict["id"], stream_post_policy=raw_stream_dict["stream_post_policy"], stream_weekly_traffic=stream_weekly_traffic, + subscriber_count=raw_stream_dict["subscriber_count"], topics_policy=raw_stream_dict["topics_policy"], is_announcement_only=is_announcement_only, is_recently_active=raw_stream_dict["is_recently_active"], @@ -251,6 +253,7 @@ def build_stream_dict_for_sub( stream_id = stream_dict["stream_id"] stream_post_policy = stream_dict["stream_post_policy"] stream_weekly_traffic = stream_dict["stream_weekly_traffic"] + subscriber_count = stream_dict["subscriber_count"] topics_policy = stream_dict["topics_policy"] is_announcement_only = stream_dict["is_announcement_only"] is_recently_active = stream_dict["is_recently_active"] @@ -301,6 +304,7 @@ def build_stream_dict_for_sub( stream_id=stream_id, stream_post_policy=stream_post_policy, stream_weekly_traffic=stream_weekly_traffic, + subscriber_count=subscriber_count, topics_policy=topics_policy, wildcard_mentions_notify=wildcard_mentions_notify, ) @@ -326,6 +330,7 @@ def build_stream_dict_for_never_sub( rendered_description = raw_stream_dict["rendered_description"] stream_id = raw_stream_dict["id"] stream_post_policy = raw_stream_dict["stream_post_policy"] + subscriber_count = raw_stream_dict["subscriber_count"] topics_policy = raw_stream_dict["topics_policy"] if recent_traffic is not None: @@ -378,6 +383,7 @@ def build_stream_dict_for_never_sub( stream_id=stream_id, stream_post_policy=stream_post_policy, stream_weekly_traffic=stream_weekly_traffic, + subscriber_count=subscriber_count, topics_policy=topics_policy, ) diff --git a/zerver/lib/types.py b/zerver/lib/types.py index 65d64a081b..59fb642586 100644 --- a/zerver/lib/types.py +++ b/zerver/lib/types.py @@ -178,6 +178,7 @@ class RawStreamDict(TypedDict): name: str rendered_description: str stream_post_policy: int + subscriber_count: int topics_policy: str @@ -235,6 +236,7 @@ class SubscriptionStreamDict(TypedDict): stream_id: int stream_post_policy: int stream_weekly_traffic: int | None + subscriber_count: int subscribers: NotRequired[list[int]] partial_subscribers: NotRequired[list[int]] topics_policy: str @@ -264,6 +266,7 @@ class NeverSubscribedStreamDict(TypedDict): stream_id: int stream_post_policy: int stream_weekly_traffic: int | None + subscriber_count: int subscribers: NotRequired[list[int]] partial_subscribers: NotRequired[list[int]] topics_policy: str @@ -295,6 +298,7 @@ class DefaultStreamDict(TypedDict): rendered_description: str stream_id: int # `stream_id` represents `id` of the `Stream` object in `API_FIELDS` stream_post_policy: int + subscriber_count: int topics_policy: str # Computed fields not specified in `Stream.API_FIELDS` is_announcement_only: bool diff --git a/zerver/models/streams.py b/zerver/models/streams.py index 6fb9ac9a1d..bc1fb1c3d1 100644 --- a/zerver/models/streams.py +++ b/zerver/models/streams.py @@ -258,6 +258,7 @@ class Stream(models.Model): "message_retention_days", "name", "rendered_description", + "subscriber_count", "can_add_subscribers_group_id", "can_administer_channel_group_id", "can_send_message_group_id", diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index c75e557025..1ad1ef8ce7 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -1367,6 +1367,7 @@ paths: "can_remove_subscribers_group": 2, "can_subscribe_group": 2, "stream_weekly_traffic": null, + "subscriber_count": 0, }, ], "id": 0, @@ -16231,6 +16232,7 @@ paths: can_administer_channel_group: {} can_send_message_group: {} can_subscribe_group: {} + subscriber_count: {} stream_weekly_traffic: type: integer nullable: true @@ -21470,6 +21472,7 @@ paths: can_administer_channel_group: {} can_send_message_group: {} can_subscribe_group: {} + subscriber_count: {} stream_weekly_traffic: type: integer nullable: true @@ -21515,6 +21518,7 @@ paths: - can_subscribe_group - stream_weekly_traffic - is_recently_active + - subscriber_count example: { "msg": "", @@ -21543,6 +21547,7 @@ paths: "stream_id": 2, "stream_post_policy": 1, "stream_weekly_traffic": null, + subscriber_count: 20, }, { "can_add_subscribers_group": 9, @@ -21566,6 +21571,7 @@ paths: "stream_id": 1, "stream_post_policy": 1, "stream_weekly_traffic": null, + subscriber_count: 10, }, ], } @@ -21621,6 +21627,7 @@ paths: "can_remove_subscribers_group": 2, "can_subscribe_group": 2, "stream_weekly_traffic": null, + subscriber_count: 12, }, } "400": @@ -23939,6 +23946,7 @@ components: can_administer_channel_group: {} can_send_message_group: {} can_subscribe_group: {} + subscriber_count: {} stream_weekly_traffic: type: integer nullable: true @@ -23965,6 +23973,7 @@ components: - rendered_description - is_web_public - stream_post_policy + - subscriber_count - message_retention_days - history_public_to_subscribers - first_message_id @@ -24134,6 +24143,27 @@ components: $ref: "#/components/schemas/CanSendMessageGroup" can_subscribe_group: $ref: "#/components/schemas/CanSubscribeGroup" + subscriber_count: + type: number + description: | + The total number of non-deactivated users (including bots) who + are subscribed to the channel. Clients are responsible for updating + this value using `peer_add` and `peer_remove` events. + + The server's internals cannot guarantee this value is correctly + synced with `peer_add` and `peer_remove` events for the channel. As + a result, if a (rare) race occurs between a change in the channel's + subscribers and fetching this value, it is possible for a client + that is correctly following the events protocol to end up with a + permanently off-by-one error in the channel's subscriber count. + + Clients are recommended to fetch full subscriber data for a channel + in contexts where it is important to avoid this risk. The official + web application, for example, uses this field primarily while + waiting to fetch a given channel's full subscriber list from the + server. + + **Changes**: New in Zulip 11.0 (feature level 394). BasicBot: allOf: - $ref: "#/components/schemas/BasicBotBase" @@ -25214,6 +25244,27 @@ components: channels. Note that some endpoints will never return archived channels unless the client declares explicit support for them via the `archived_channels` client capability. + subscriber_count: + type: number + description: | + The total number of non-deactivated users (including bots) who + are subscribed to the channel. Clients are responsible for updating + this value using `peer_add` and `peer_remove` events. + + The server's internals cannot guarantee this value is correctly + synced with `peer_add` and `peer_remove` events for the channel. As + a result, if a (rare) race occurs between a change in the channel's + subscribers and fetching this value, it is possible for a client + that is correctly following the events protocol to end up with a + permanently off-by-one error in the channel's subscriber count. + + Clients are recommended to fetch full subscriber data for a channel + in contexts where it is important to avoid this risk. The official + web application, for example, uses this field primarily while + waiting to fetch a given channel's full subscriber list from the + server. + + **Changes**: New in Zulip 11.0 (feature level 394). DefaultChannelGroup: type: object description: | diff --git a/zerver/tests/test_events.py b/zerver/tests/test_events.py index 82e8a73745..9b08e6c112 100644 --- a/zerver/tests/test_events.py +++ b/zerver/tests/test_events.py @@ -496,13 +496,25 @@ class BaseAction(ZulipTestCase): for u in state["never_subscribed"]: if "subscribers" in u: u["subscribers"].sort() + # this isn't guaranteed to match + del u["subscriber_count"] if "subscriptions" in state: for u in state["subscriptions"]: if "subscribers" in u: u["subscribers"].sort() + # this isn't guaranteed to match + del u["subscriber_count"] state["subscriptions"] = {u["name"]: u for u in state["subscriptions"]} if "unsubscribed" in state: + for u in state["unsubscribed"]: + # this isn't guaranteed to match + del u["subscriber_count"] state["unsubscribed"] = {u["name"]: u for u in state["unsubscribed"]} + if "streams" in state: + for stream in state["streams"]: + if "subscriber_count" in stream: + # this isn't guaranteed to match + del stream["subscriber_count"] if "realm_bots" in state: state["realm_bots"] = {u["email"]: u for u in state["realm_bots"]} # Since time is different for every call, just fix the value