streams: Use bulk function in can_access_stream_metadata_user_ids.

Performance remains the same whether we're using the bulk function
underneath the function in question or not, this helps us avoid
duplication.

(cherry picked from commit 2e48293e4b)
This commit is contained in:
Shubham Padia
2025-03-25 09:54:13 +00:00
committed by Tim Abbott
parent e6f3c15a92
commit ac9a97e1a7
2 changed files with 8 additions and 37 deletions

View File

@@ -17,7 +17,6 @@ from zerver.lib.exceptions import (
OrganizationOwnerRequiredError,
)
from zerver.lib.stream_subscription import (
get_active_subscriptions_for_stream_id,
get_guest_user_ids_for_streams,
get_subscribed_stream_ids_for_user,
get_user_ids_for_streams,
@@ -959,40 +958,11 @@ def access_stream_to_remove_visibility_policy_by_id(
return stream
def private_stream_user_ids(stream_id: int) -> set[int]:
subscriptions = get_active_subscriptions_for_stream_id(
stream_id, include_deactivated_users=False
)
return {sub["user_profile_id"] for sub in subscriptions.values("user_profile_id")}
def public_stream_user_ids(stream: Stream) -> set[int]:
guest_subscriptions = get_active_subscriptions_for_stream_id(
stream.id, include_deactivated_users=False
).filter(user_profile__role=UserProfile.ROLE_GUEST)
guest_subscriptions_ids = {
sub["user_profile_id"] for sub in guest_subscriptions.values("user_profile_id")
}
return set(active_non_guest_user_ids(stream.realm_id)) | guest_subscriptions_ids
def can_access_stream_metadata_user_ids(stream: Stream) -> set[int]:
# return user ids of users who can access the attributes of a
# stream, such as its name/description. Useful for sending events
# to all users with access to a stream's attributes.
if stream.is_public():
# For a public stream, this is everyone in the realm
# except unsubscribed guest users
return public_stream_user_ids(stream)
else:
# for a private stream, it's subscribers plus channel admins
# and users belonging to `can_add_subscribers_group` or
# `can_subscribe_group`.
return (
private_stream_user_ids(stream.id)
| {user.id for user in stream.realm.get_admin_users_and_bots()}
| get_user_ids_with_metadata_access_via_permission_groups(stream)
)
return bulk_can_access_stream_metadata_user_ids([stream])[stream.id]
def bulk_can_access_stream_metadata_user_ids(streams: list[Stream]) -> dict[int, set[int]]:

View File

@@ -83,7 +83,6 @@ from zerver.lib.streams import (
ensure_stream,
filter_stream_authorization_for_adding_subscribers,
list_to_streams,
public_stream_user_ids,
user_has_content_access,
)
from zerver.lib.subscription_info import (
@@ -1975,9 +1974,9 @@ class StreamAdminTest(ZulipTestCase):
self.assertEqual(events[0]["event"]["streams"][0]["name"], "new_stream")
self.assertEqual(events[0]["event"]["streams"][0]["stream_id"], stream.id)
notified_user_ids = set(events[0]["users"])
self.assertEqual(
self.assertCountEqual(
notified_user_ids,
public_stream_user_ids(stream),
set(active_non_guest_user_ids(stream.realm_id)),
)
# Guest user should not be notified.
self.assertNotIn(self.example_user("polonius").id, notified_user_ids)
@@ -2123,7 +2122,7 @@ class StreamAdminTest(ZulipTestCase):
stream_name_1 = get_stream("stream_name1", user_profile.realm)
notified_user_ids = get_notified_user_ids()
self.assertEqual(notified_user_ids, set(public_stream_user_ids(stream_name_1)))
self.assertEqual(notified_user_ids, set(active_non_guest_user_ids(realm.id)))
self.assertIn(user_profile.id, notified_user_ids)
self.assertIn(self.example_user("prospero").id, notified_user_ids)
self.assertNotIn(self.example_user("polonius").id, notified_user_ids)
@@ -2142,7 +2141,7 @@ class StreamAdminTest(ZulipTestCase):
acting_user=self.example_user("polonius"),
)
notified_user_ids = get_notified_user_ids()
self.assertEqual(notified_user_ids, set(public_stream_user_ids(stream_name_1)))
self.assertEqual(notified_user_ids, set(active_non_guest_user_ids(realm.id)))
self.assertIn(user_profile.id, notified_user_ids)
self.assertIn(self.example_user("prospero").id, notified_user_ids)
self.assertNotIn(self.example_user("polonius").id, notified_user_ids)
@@ -2159,7 +2158,9 @@ class StreamAdminTest(ZulipTestCase):
# Subscribed guest user should be notified.
self.subscribe(self.example_user("polonius"), stream_name_1.name)
notified_user_ids = get_notified_user_ids()
self.assertEqual(notified_user_ids, set(public_stream_user_ids(stream_name_1)))
expected_notified_user_ids = set(active_non_guest_user_ids(realm.id))
expected_notified_user_ids.add(self.example_user("polonius").id)
self.assertEqual(notified_user_ids, expected_notified_user_ids)
self.assertIn(user_profile.id, notified_user_ids)
self.assertIn(self.example_user("prospero").id, notified_user_ids)
self.assertIn(self.example_user("polonius").id, notified_user_ids)