diff --git a/api_docs/changelog.md b/api_docs/changelog.md index 9517b8ce82..f67c8da02c 100644 --- a/api_docs/changelog.md +++ b/api_docs/changelog.md @@ -20,6 +20,12 @@ format used by the Zulip server that they are interacting with. ## Changes in Zulip 10.0 +**Feature level 289** + +* [`POST /users/{user_id}/subscription`](/api/subscribe): In the response, + users are identified by their numeric user ID rather than by their + Zulip API email address. + **Feature level 288** * [`POST /register`](/api/register-queue): diff --git a/version.py b/version.py index c41b8f6bbf..95b21659f6 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 = 288 # Last bumped for presence_history_limit_days +API_FEATURE_LEVEL = 289 # return ID as key while subscribing to a stream. # 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/web/src/stream_edit_subscribers.ts b/web/src/stream_edit_subscribers.ts index b38f69b3ef..cba3fd1a5a 100644 --- a/web/src/stream_edit_subscribers.ts +++ b/web/src/stream_edit_subscribers.ts @@ -226,11 +226,11 @@ function subscribe_new_users({pill_user_ids}: {pill_user_ids: number[]}): void { function invite_success(raw_data: unknown): void { const data = add_user_ids_api_response_schema.parse(raw_data); pill_widget.clear(); - const subscribed_users = Object.keys(data.subscribed).map( - (email) => people.get_by_email(email)!, + const subscribed_users = Object.keys(data.subscribed).map((user_id) => + people.get_by_user_id(Number(user_id)), ); - const already_subscribed_users = Object.keys(data.already_subscribed).map( - (email) => people.get_by_email(email)!, + const already_subscribed_users = Object.keys(data.already_subscribed).map((user_id) => + people.get_by_user_id(Number(user_id)), ); show_stream_subscription_request_result({ diff --git a/zerver/openapi/python_examples.py b/zerver/openapi/python_examples.py index cca31a7580..479c0218f1 100644 --- a/zerver/openapi/python_examples.py +++ b/zerver/openapi/python_examples.py @@ -129,7 +129,7 @@ def add_subscriptions(client: Client) -> None: # {code_example|end} assert_success_response(result) validate_against_openapi_schema(result, "/users/me/subscriptions", "post", "200") - assert "newbie@zulip.com" in result["subscribed"] + assert str(user_id) in result["subscribed"] def test_add_subscriptions_already_subscribed(client: Client) -> None: diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index 833f28cba8..b25f9a4650 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -9916,12 +9916,15 @@ paths: subscribed: type: object description: | - A dictionary where the key is the Zulip API email address of the user/bot - and the value is a list of the names of the channels that were subscribed - to as a result of the query. + A dictionary where the key is the ID of the user and the value + is a list of the names of the channels that user was subscribed + to as a result of the request. + + **Changes**: Before Zulip 10.0 (feature level 289), the user + keys were Zulip API email addresses, not user ID. additionalProperties: description: | - `{email_address}`: List of the names of the channels that were subscribed + `{id}`: List of the names of the channels that were subscribed to as a result of the query. type: array items: @@ -9929,12 +9932,16 @@ paths: already_subscribed: type: object description: | - A dictionary where the key is the Zulip API email address of the user/bot - and the value is a list of the names of the channels that the user/bot is - already subscribed to. + A dictionary where the key is the ID of the user and the value + is a list of the names of the channels that where the user was + not added as a subscriber in this request, because they were + already a subscriber. + + **Changes**: Before Zulip 10.0 (feature level 289), the user + keys were Zulip API email addresses, not user IDs. additionalProperties: description: | - `{email_address}`: List of the names of the channels that the user is + `{id}`: List of the names of the channels that the user is already subscribed to. type: array items: @@ -9948,11 +9955,10 @@ paths: authorized to subscribe to. Only present if `"authorization_errors_fatal": false`. example: { - "already_subscribed": - {"iago@zulip.com": ["testing-help"]}, + "already_subscribed": {"1": ["testing-help"]}, "msg": "", "result": "success", - "subscribed": {"newbie@zulip.com": ["testing-help"]}, + "subscribed": {"2": ["testing-help"]}, } "400": description: Bad request. diff --git a/zerver/tests/test_subs.py b/zerver/tests/test_subs.py index 9c52cfc540..72f017e3ac 100644 --- a/zerver/tests/test_subs.py +++ b/zerver/tests/test_subs.py @@ -495,10 +495,10 @@ class TestCreateStreams(ZulipTestCase): "result": "success", "msg": "", "subscribed": { - "AARON@zulip.com": ["brand new stream"], - "cordelia@zulip.com": ["brand new stream"], - "hamlet@zulip.com": ["brand new stream"], - "iago@zulip.com": ["brand new stream"], + "10": ["brand new stream"], + "11": ["brand new stream"], + "6": ["brand new stream"], + "8": ["brand new stream"], }, "already_subscribed": {}, } @@ -4062,6 +4062,7 @@ class SubscriptionAPITest(ZulipTestCase): self.user_profile = self.example_user("hamlet") self.test_email = self.user_profile.email self.test_user = self.user_profile + self.test_id = str(self.user_profile.id) self.login_user(self.user_profile) self.test_realm = self.user_profile.realm self.streams = self.get_streams(self.user_profile) @@ -4170,29 +4171,30 @@ class SubscriptionAPITest(ZulipTestCase): email: str, new_subs: list[str], realm: Realm, + id: str, invite_only: bool = False, ) -> None: """ Check result of adding subscriptions. You can add subscriptions for yourself or possibly many - principals, which is why e-mails map to subscriptions in the + principals, which is why user ID map to subscriptions in the result. The result json is of the form {"msg": "", "result": "success", - "already_subscribed": {self.example_email("iago"): ["Venice", "Verona"]}, - "subscribed": {self.example_email("iago"): ["Venice8"]}} + "already_subscribed": {self.example_user("iago").id: ["Venice", "Verona"]}, + "subscribed": {self.example_user("iago").id: ["Venice8"]}} """ result = self.common_subscribe_to_streams( self.test_user, subscriptions, other_params, invite_only=invite_only ) json = self.assert_json_success(result) - self.assertEqual(sorted(subscribed), sorted(json["subscribed"][email])) - self.assertEqual(sorted(already_subscribed), sorted(json["already_subscribed"][email])) - user = get_user(email, realm) + self.assertEqual(sorted(subscribed), sorted(json["subscribed"][id])) + self.assertEqual(sorted(already_subscribed), sorted(json["already_subscribed"][id])) + user = get_user_profile_by_id_in_realm(int(id), realm) new_streams = self.get_streams(user) self.assertEqual(sorted(new_streams), sorted(new_subs)) @@ -4217,6 +4219,7 @@ class SubscriptionAPITest(ZulipTestCase): self.test_email, self.streams + add_streams, self.test_realm, + self.test_id, ) def test_successful_subscriptions_add_with_announce(self) -> None: @@ -4246,6 +4249,7 @@ class SubscriptionAPITest(ZulipTestCase): self.test_email, self.streams + add_streams, self.test_realm, + self.test_id, ) expected_stream_ids = {get_stream(stream, self.test_realm).id for stream in add_streams} @@ -4392,6 +4396,7 @@ class SubscriptionAPITest(ZulipTestCase): self.test_email, [*self.streams, "hümbüǵ"], self.test_realm, + self.test_id, ) def test_subscriptions_add_too_long(self) -> None: @@ -4703,6 +4708,7 @@ class SubscriptionAPITest(ZulipTestCase): other_profile.email, streams_to_sub, invitee_realm, + str(other_profile.id), invite_only=invite_only, ) @@ -4875,7 +4881,7 @@ class SubscriptionAPITest(ZulipTestCase): do_change_stream_post_policy(stream, Stream.STREAM_POST_POLICY_ADMINS, acting_user=member) result = self.common_subscribe_to_streams(member, ["stream1"]) json = self.assert_json_success(result) - self.assertEqual(json["subscribed"], {member.email: ["stream1"]}) + self.assertEqual(json["subscribed"], {str(member.id): ["stream1"]}) self.assertEqual(json["already_subscribed"], {}) def test_subscribe_to_stream_post_policy_restrict_new_members_stream(self) -> None: @@ -4895,7 +4901,7 @@ class SubscriptionAPITest(ZulipTestCase): ) result = self.common_subscribe_to_streams(new_member, ["stream1"]) json = self.assert_json_success(result) - self.assertEqual(json["subscribed"], {new_member.email: ["stream1"]}) + self.assertEqual(json["subscribed"], {str(new_member.id): ["stream1"]}) self.assertEqual(json["already_subscribed"], {}) def test_subscribe_to_stream_post_policy_moderators_stream(self) -> None: @@ -4912,7 +4918,7 @@ class SubscriptionAPITest(ZulipTestCase): ) result = self.common_subscribe_to_streams(member, ["stream1"]) json = self.assert_json_success(result) - self.assertEqual(json["subscribed"], {member.email: ["stream1"]}) + self.assertEqual(json["subscribed"], {str(member.id): ["stream1"]}) self.assertEqual(json["already_subscribed"], {}) def test_guest_user_subscribe(self) -> None: @@ -5897,7 +5903,7 @@ class InviteOnlyStreamTest(ZulipTestCase): result = self.common_subscribe_to_streams(hamlet, [stream_name], invite_only=True) json = self.assert_json_success(result) - self.assertEqual(json["subscribed"], {hamlet.email: [stream_name]}) + self.assertEqual(json["subscribed"], {str(hamlet.id): [stream_name]}) self.assertEqual(json["already_subscribed"], {}) # Subscribing oneself to an invite-only stream is not allowed @@ -5925,7 +5931,7 @@ class InviteOnlyStreamTest(ZulipTestCase): extra_post_data={"principals": orjson.dumps([othello.id]).decode()}, ) json = self.assert_json_success(result) - self.assertEqual(json["subscribed"], {othello.email: [stream_name]}) + self.assertEqual(json["subscribed"], {str(othello.id): [stream_name]}) self.assertEqual(json["already_subscribed"], {}) # Make sure both users are subscribed to this stream diff --git a/zerver/views/streams.py b/zerver/views/streams.py index 9d83a8ef28..70a0f02819 100644 --- a/zerver/views/streams.py +++ b/zerver/views/streams.py @@ -664,9 +664,7 @@ def add_subscriptions_backend( realm, streams, subscribers, acting_user=user_profile, color_map=color_map ) - # We can assume unique emails here for now, but we should eventually - # convert this function to be more id-centric. - email_to_user_profile: dict[str, UserProfile] = {} + id_to_user_profile: dict[str, UserProfile] = {} result: dict[str, Any] = dict( subscribed=defaultdict(list), already_subscribed=defaultdict(list) @@ -674,12 +672,14 @@ def add_subscriptions_backend( for sub_info in subscribed: subscriber = sub_info.user stream = sub_info.stream - result["subscribed"][subscriber.email].append(stream.name) - email_to_user_profile[subscriber.email] = subscriber + user_id = str(subscriber.id) + result["subscribed"][user_id].append(stream.name) + id_to_user_profile[user_id] = subscriber for sub_info in already_subscribed: subscriber = sub_info.user stream = sub_info.stream - result["already_subscribed"][subscriber.email].append(stream.name) + user_id = str(subscriber.id) + result["already_subscribed"][user_id].append(stream.name) result["subscribed"] = dict(result["subscribed"]) result["already_subscribed"] = dict(result["already_subscribed"]) @@ -688,7 +688,7 @@ def add_subscriptions_backend( user_profile=user_profile, subscribers=subscribers, new_subscriptions=result["subscribed"], - email_to_user_profile=email_to_user_profile, + id_to_user_profile=id_to_user_profile, created_streams=created_streams, announce=announce, ) @@ -704,7 +704,7 @@ def send_messages_for_new_subscribers( user_profile: UserProfile, subscribers: set[UserProfile], new_subscriptions: dict[str, list[str]], - email_to_user_profile: dict[str, UserProfile], + id_to_user_profile: dict[str, UserProfile], created_streams: list[Stream], announce: bool, ) -> None: @@ -716,7 +716,7 @@ def send_messages_for_new_subscribers( excessive query counts by mocking this function so that it doesn't drown out query counts from other code. """ - bots = {subscriber.email: subscriber.is_bot for subscriber in subscribers} + bots = {str(subscriber.id): subscriber.is_bot for subscriber in subscribers} newly_created_stream_names = {s.name for s in created_streams} @@ -727,11 +727,11 @@ def send_messages_for_new_subscribers( # or if a new stream was created with the "announce" option. notifications = [] if new_subscriptions: - for email, subscribed_stream_names in new_subscriptions.items(): - if email == user_profile.email: + for id, subscribed_stream_names in new_subscriptions.items(): + if id == str(user_profile.id): # Don't send a Zulip if you invited yourself. continue - if bots[email]: + if bots[id]: # Don't send invitation Zulips to bots continue @@ -742,7 +742,7 @@ def send_messages_for_new_subscribers( if not notify_stream_names: continue - recipient_user = email_to_user_profile[email] + recipient_user = id_to_user_profile[id] sender = get_system_bot(settings.NOTIFICATION_BOT, recipient_user.realm_id) msg = you_were_just_subscribed_message(