Mirror
/
zulip
mirror of https://github.com/zulip/zulip.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

channel_subscribe: Use IDs instead of emails when processing results. 

As part of our todo in the code, we want to use the unique user IDs
instead of emails when processing the results of subscribing users to a
channel. These changes apply those changes and streamlines the use of IDs.
This commit is contained in:
joseph 2024-08-03 17:41:35 +00:00 committed by Tim Abbott
parent 62f74fcb83
commit 65893292b5
7 changed files with 63 additions and 45 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
api_docs/changelog.md
View File

@ -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):

2
version.py
View File

@ -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

8
web/src/stream_edit_subscribers.ts
View File

@ -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({

2
zerver/openapi/python_examples.py
View File

@ -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:

28
zerver/openapi/zulip.yaml
View File

@ -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.

36
zerver/tests/test_subs.py
View File

@ -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

26
zerver/views/streams.py
View File

@ -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(