mirror of
https://github.com/zulip/zulip.git
synced 2025-10-23 04:52:12 +00:00
get_streams: Add include_can_access_content
.
Also add some query count checks. See https://chat.zulip.org/#narrow/channel/378-api-design/topic/GET.20.2Fstreams.20with.20new.20permissions/with/2096944 for API design discussion.
This commit is contained in:
committed by
Tim Abbott
parent
19b96ce9a8
commit
6dde44cf37
@@ -20,6 +20,12 @@ format used by the Zulip server that they are interacting with.
|
||||
|
||||
## Changes in Zulip 10.0
|
||||
|
||||
**Feature level 356**
|
||||
|
||||
* [`GET /streams`](/api/get-streams): The new parameter
|
||||
`include_can_access_content`, if set to True, returns all the
|
||||
channels that the user making the request has content access to.
|
||||
|
||||
**Feature level 355**
|
||||
|
||||
* [`POST /messages/flags/narrow`](/api/update-message-flags-for-narrow),
|
||||
|
@@ -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 = 355 # Last bumped for adding 'ignored_because_not_subscribed_channels'
|
||||
API_FEATURE_LEVEL = 356 # Last bumped for adding `include_can_access_content` to get-streams.
|
||||
|
||||
# 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
|
||||
|
@@ -1443,6 +1443,7 @@ def get_streams_for_user(
|
||||
exclude_archived: bool = True,
|
||||
include_all_active: bool = False,
|
||||
include_owner_subscribed: bool = False,
|
||||
include_can_access_content: bool = False,
|
||||
) -> list[Stream]:
|
||||
if include_all_active and not user_profile.is_realm_admin:
|
||||
raise JsonableError(_("User not authorized for this query"))
|
||||
@@ -1473,14 +1474,53 @@ def get_streams_for_user(
|
||||
else:
|
||||
query_filter |= option
|
||||
|
||||
if include_subscribed:
|
||||
should_add_owner_subscribed_filter = include_owner_subscribed and user_profile.is_bot
|
||||
|
||||
if include_can_access_content:
|
||||
all_streams = list(
|
||||
query.only(
|
||||
*Stream.API_FIELDS,
|
||||
"can_send_message_group",
|
||||
"can_send_message_group__named_user_group",
|
||||
# Both of these fields are need for get_content_access_streams.
|
||||
"is_in_zephyr_realm",
|
||||
"recipient_id",
|
||||
)
|
||||
)
|
||||
user_group_membership_details = UserGroupMembershipDetails(
|
||||
user_recursive_group_ids=None
|
||||
)
|
||||
content_access_streams = get_content_access_streams(
|
||||
user_profile, all_streams, user_group_membership_details
|
||||
)
|
||||
# Optimization: Currently, only include_owner_subscribed
|
||||
# has the ability to add additional results to
|
||||
# content_access_streams. We return early to save us a
|
||||
# database query down the line if we do not need to add
|
||||
# include_owner_subscribed filter.
|
||||
if not should_add_owner_subscribed_filter:
|
||||
return content_access_streams
|
||||
|
||||
content_access_stream_ids = [stream.id for stream in content_access_streams]
|
||||
content_access_stream_check = Q(id__in=set(content_access_stream_ids))
|
||||
add_filter_option(content_access_stream_check)
|
||||
|
||||
# Subscribed channels will already have been included if
|
||||
# include_can_access_content is True.
|
||||
if not include_can_access_content and include_subscribed:
|
||||
subscribed_stream_ids = get_subscribed_stream_ids_for_user(user_profile)
|
||||
recipient_check = Q(id__in=set(subscribed_stream_ids))
|
||||
add_filter_option(recipient_check)
|
||||
if include_public:
|
||||
|
||||
# All accessible public channels will already have been
|
||||
# included if include_can_access_content is True.
|
||||
if not include_can_access_content and include_public:
|
||||
invite_only_check = Q(invite_only=False)
|
||||
add_filter_option(invite_only_check)
|
||||
if include_web_public:
|
||||
|
||||
# All accessible web-public channels will already have been
|
||||
# included if include_can_access_content is True.
|
||||
if not include_can_access_content and include_web_public:
|
||||
# This should match get_web_public_streams_queryset
|
||||
web_public_check = Q(
|
||||
is_web_public=True,
|
||||
@@ -1489,7 +1529,8 @@ def get_streams_for_user(
|
||||
deactivated=False,
|
||||
)
|
||||
add_filter_option(web_public_check)
|
||||
if include_owner_subscribed and user_profile.is_bot:
|
||||
|
||||
if should_add_owner_subscribed_filter:
|
||||
bot_owner = user_profile.bot_owner
|
||||
assert bot_owner is not None
|
||||
owner_stream_ids = get_subscribed_stream_ids_for_user(bot_owner)
|
||||
@@ -1576,6 +1617,7 @@ def do_get_streams(
|
||||
include_all_active: bool = False,
|
||||
include_default: bool = False,
|
||||
include_owner_subscribed: bool = False,
|
||||
include_can_access_content: bool = False,
|
||||
) -> list[APIStreamDict]:
|
||||
# This function is only used by API clients now.
|
||||
|
||||
@@ -1587,6 +1629,7 @@ def do_get_streams(
|
||||
exclude_archived,
|
||||
include_all_active,
|
||||
include_owner_subscribed,
|
||||
include_can_access_content,
|
||||
)
|
||||
|
||||
stream_ids = {stream.id for stream in streams}
|
||||
|
@@ -20363,6 +20363,16 @@ paths:
|
||||
type: boolean
|
||||
default: false
|
||||
example: true
|
||||
- name: include_can_access_content
|
||||
in: query
|
||||
description: |
|
||||
Include all the channels that the user has content access to.
|
||||
|
||||
**Changes**: New in Zulip 10.0 (feature level 356).
|
||||
schema:
|
||||
type: boolean
|
||||
default: false
|
||||
example: true
|
||||
responses:
|
||||
"200":
|
||||
description: Success.
|
||||
|
@@ -6761,7 +6761,8 @@ class GetStreamsTest(ZulipTestCase):
|
||||
include_public="false",
|
||||
include_subscribed="false",
|
||||
)
|
||||
result = self.api_get(test_bot, "/api/v1/streams", filters)
|
||||
with self.assert_database_query_count(7):
|
||||
result = self.api_get(test_bot, "/api/v1/streams", filters)
|
||||
owner_subs = self.api_get(hamlet, "/api/v1/users/me/subscriptions")
|
||||
|
||||
json = self.assert_json_success(result)
|
||||
@@ -6784,7 +6785,8 @@ class GetStreamsTest(ZulipTestCase):
|
||||
include_public="false",
|
||||
include_subscribed="true",
|
||||
)
|
||||
result = self.api_get(test_bot, "/api/v1/streams", filters)
|
||||
with self.assert_database_query_count(8):
|
||||
result = self.api_get(test_bot, "/api/v1/streams", filters)
|
||||
|
||||
json = self.assert_json_success(result)
|
||||
self.assertIn("streams", json)
|
||||
@@ -6800,15 +6802,16 @@ class GetStreamsTest(ZulipTestCase):
|
||||
# Check it correctly lists the bot owner's subs + all public streams
|
||||
self.make_stream("private_stream", realm=realm, invite_only=True)
|
||||
self.subscribe(test_bot, "private_stream")
|
||||
result = self.api_get(
|
||||
test_bot,
|
||||
"/api/v1/streams",
|
||||
{
|
||||
"include_owner_subscribed": "true",
|
||||
"include_public": "true",
|
||||
"include_subscribed": "false",
|
||||
},
|
||||
)
|
||||
with self.assert_database_query_count(7):
|
||||
result = self.api_get(
|
||||
test_bot,
|
||||
"/api/v1/streams",
|
||||
{
|
||||
"include_owner_subscribed": "true",
|
||||
"include_public": "true",
|
||||
"include_subscribed": "false",
|
||||
},
|
||||
)
|
||||
|
||||
json = self.assert_json_success(result)
|
||||
self.assertIn("streams", json)
|
||||
@@ -6823,15 +6826,16 @@ class GetStreamsTest(ZulipTestCase):
|
||||
|
||||
# Check it correctly lists the bot owner's subs + all public streams +
|
||||
# the bot's subs
|
||||
result = self.api_get(
|
||||
test_bot,
|
||||
"/api/v1/streams",
|
||||
{
|
||||
"include_owner_subscribed": "true",
|
||||
"include_public": "true",
|
||||
"include_subscribed": "true",
|
||||
},
|
||||
)
|
||||
with self.assert_database_query_count(8):
|
||||
result = self.api_get(
|
||||
test_bot,
|
||||
"/api/v1/streams",
|
||||
{
|
||||
"include_owner_subscribed": "true",
|
||||
"include_public": "true",
|
||||
"include_subscribed": "true",
|
||||
},
|
||||
)
|
||||
|
||||
json = self.assert_json_success(result)
|
||||
self.assertIn("streams", json)
|
||||
@@ -6844,6 +6848,39 @@ class GetStreamsTest(ZulipTestCase):
|
||||
|
||||
self.assertEqual(actual, expected)
|
||||
|
||||
private_stream_2 = self.make_stream("private_stream_2", realm=realm, invite_only=True)
|
||||
private_stream_3 = self.make_stream("private_stream_3", realm=realm, invite_only=True)
|
||||
self.make_stream("private_stream_4", realm=realm, invite_only=True)
|
||||
test_bot_group = self.create_or_update_anonymous_group_for_setting([test_bot], [])
|
||||
do_change_stream_group_based_setting(
|
||||
private_stream_2, "can_add_subscribers_group", test_bot_group, acting_user=None
|
||||
)
|
||||
do_change_stream_group_based_setting(
|
||||
private_stream_3, "can_administer_channel_group", test_bot_group, acting_user=None
|
||||
)
|
||||
# Check it correctly lists the bot owner's subs + the channels
|
||||
# bot has content access to.
|
||||
with self.assert_database_query_count(10):
|
||||
result = self.api_get(
|
||||
test_bot,
|
||||
"/api/v1/streams",
|
||||
{
|
||||
"include_owner_subscribed": "true",
|
||||
"include_can_access_content": "true",
|
||||
},
|
||||
)
|
||||
|
||||
json = self.assert_json_success(result)
|
||||
self.assertIn("streams", json)
|
||||
self.assertIsInstance(json["streams"], list)
|
||||
|
||||
actual = sorted(s["name"] for s in json["streams"])
|
||||
expected = [s["name"] for s in owner_subs_json["subscriptions"]]
|
||||
expected.extend(["Rome", "Venice", "Scotland", "private_stream", "private_stream_2"])
|
||||
expected.sort()
|
||||
|
||||
self.assertEqual(actual, expected)
|
||||
|
||||
def test_all_active_streams_api(self) -> None:
|
||||
url = "/api/v1/streams"
|
||||
data = {"include_all_active": "true"}
|
||||
@@ -6915,6 +6952,42 @@ class GetStreamsTest(ZulipTestCase):
|
||||
]
|
||||
self.assertEqual(sorted(s["name"] for s in json["streams"]), sorted(all_streams))
|
||||
|
||||
def test_include_can_access_content_streams_api(self) -> None:
|
||||
"""
|
||||
Ensure that the query we use to get public streams successfully returns
|
||||
a list of streams
|
||||
"""
|
||||
# Cordelia is not subscribed to private stream `core team`.
|
||||
user = self.example_user("cordelia")
|
||||
realm = get_realm("zulip")
|
||||
self.login_user(user)
|
||||
user_group = self.create_or_update_anonymous_group_for_setting([user], [])
|
||||
|
||||
private_stream_1 = self.make_stream("private_stream_1", realm=realm, invite_only=True)
|
||||
private_stream_2 = self.make_stream("private_stream_2", realm=realm, invite_only=True)
|
||||
private_stream_3 = self.make_stream("private_stream_3", realm=realm, invite_only=True)
|
||||
self.make_stream("private_stream_4", realm=realm, invite_only=True)
|
||||
|
||||
self.subscribe(user, private_stream_1.name)
|
||||
do_change_stream_group_based_setting(
|
||||
private_stream_2, "can_add_subscribers_group", user_group, acting_user=user
|
||||
)
|
||||
do_change_stream_group_based_setting(
|
||||
private_stream_3, "can_administer_channel_group", user_group, acting_user=user
|
||||
)
|
||||
|
||||
# Check it correctly lists all content access streams with
|
||||
# include_can_access_content=false
|
||||
filters = dict(include_can_access_content="true")
|
||||
with self.assert_database_query_count(8):
|
||||
result = self.api_get(user, "/api/v1/streams", filters)
|
||||
json = self.assert_json_success(result)
|
||||
result_streams = [
|
||||
stream.name for stream in Stream.objects.filter(realm=realm, invite_only=False)
|
||||
]
|
||||
result_streams.extend([private_stream_1.name, private_stream_2.name])
|
||||
self.assertEqual(sorted(s["name"] for s in json["streams"]), sorted(result_streams))
|
||||
|
||||
def test_get_single_stream_api(self) -> None:
|
||||
self.login("hamlet")
|
||||
realm = get_realm("zulip")
|
||||
|
@@ -928,6 +928,7 @@ def get_streams_backend(
|
||||
include_all_active: Json[bool] = False,
|
||||
include_default: Json[bool] = False,
|
||||
include_owner_subscribed: Json[bool] = False,
|
||||
include_can_access_content: Json[bool] = False,
|
||||
) -> HttpResponse:
|
||||
streams = do_get_streams(
|
||||
user_profile,
|
||||
@@ -938,6 +939,7 @@ def get_streams_backend(
|
||||
include_all_active=include_all_active,
|
||||
include_default=include_default,
|
||||
include_owner_subscribed=include_owner_subscribed,
|
||||
include_can_access_content=include_can_access_content,
|
||||
)
|
||||
return json_success(request, data={"streams": streams})
|
||||
|
||||
|
Reference in New Issue
Block a user