From b612351e48405f4c8ebc8a0d92b500d358898547 Mon Sep 17 00:00:00 2001 From: Vector73 Date: Tue, 24 Jun 2025 06:55:35 +0000 Subject: [PATCH] stream_setting: Add setting for who can move messages out of channel. Adds `can_move_messages_out_of_channel_group` channel-level permission for who can move messages out of the channel. Fixes #34243. --- api_docs/changelog.md | 10 ++ version.py | 2 +- web/src/group_permission_settings.ts | 1 + web/src/message_edit.ts | 4 +- web/src/popover_menus_data.ts | 3 +- web/src/settings_components.ts | 1 + web/src/settings_config.ts | 4 + web/src/stream_create.ts | 1 + web/src/stream_data.ts | 29 ++++ web/src/stream_popover.ts | 3 +- web/src/stream_types.ts | 2 + .../stream_settings/stream_types.hbs | 5 + web/tests/compose_validate.test.cjs | 1 + web/tests/lib/events.cjs | 2 + web/tests/message_edit.test.cjs | 7 +- web/tests/stream_data.test.cjs | 35 +++++ zerver/actions/message_edit.py | 3 +- zerver/actions/streams.py | 3 + zerver/lib/event_types.py | 2 + zerver/lib/streams.py | 27 ++++ zerver/lib/subscription_info.py | 14 ++ zerver/lib/types.py | 4 + ..._can_move_messages_out_of_channel_group.py | 23 +++ ..._can_move_messages_out_of_channel_group.py | 56 ++++++++ ..._can_move_messages_out_of_channel_group.py | 22 +++ zerver/models/streams.py | 9 ++ zerver/openapi/zulip.yaml | 59 ++++++++ zerver/tests/test_message_move_stream.py | 135 +++++++++++++++++- zerver/tests/test_message_move_topic.py | 8 +- zerver/views/streams.py | 5 + 30 files changed, 460 insertions(+), 20 deletions(-) create mode 100644 zerver/migrations/0722_stream_can_move_messages_out_of_channel_group.py create mode 100644 zerver/migrations/0723_set_default_value_for_can_move_messages_out_of_channel_group.py create mode 100644 zerver/migrations/0724_alter_stream_can_move_messages_out_of_channel_group.py diff --git a/api_docs/changelog.md b/api_docs/changelog.md index 2259ffa039..e70872b683 100644 --- a/api_docs/changelog.md +++ b/api_docs/changelog.md @@ -32,6 +32,16 @@ format used by the Zulip server that they are interacting with. `can_move_messages_within_channel_group` parameter to support setting and changing the user group whose members can move messages within the specified channel. +* [`GET /users/me/subscriptions`](/api/get-subscriptions), + [`GET /streams`](/api/get-streams), [`GET /events`](/api/get-events), + [`POST /register`](/api/register-queue): Added `can_move_messages_out_of_channel_group` + field which is a [group-setting value](/api/group-setting-values) describing the + set of users with permissions to move messages out of the channel. +* [`POST /users/me/subscriptions`](/api/subscribe), + [`PATCH /streams/{stream_id}`](/api/update-stream): Added + `can_move_messages_out_of_channel_group` parameter to support setting and + changing the user group whose members can move messages out of the specified + channel. **Feature level 395** diff --git a/version.py b/version.py index fe96788ebe..fac3cbfc6d 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 = 395 +API_FEATURE_LEVEL = 396 # 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/group_permission_settings.ts b/web/src/group_permission_settings.ts index 3aa64619a1..e91f2db46e 100644 --- a/web/src/group_permission_settings.ts +++ b/web/src/group_permission_settings.ts @@ -71,6 +71,7 @@ export type RealmGroupSettingName = z.infer 0; const has_unread_messages = num_unread_for_topic(sub.stream_id, topic_name) > 0; - const can_move_topic = - !sub.is_archived && settings_data.user_can_move_messages_between_streams(); + const can_move_topic = stream_data.user_can_move_messages_out_of_channel(sub); const can_rename_topic = stream_data.user_can_move_messages_within_channel(sub); const can_resolve_topic = !sub.is_archived && settings_data.user_can_resolve_topic(); const visibility_policy = user_topics.get_topic_visibility_policy(sub.stream_id, topic_name); diff --git a/web/src/settings_components.ts b/web/src/settings_components.ts index d96a90d146..53423fd383 100644 --- a/web/src/settings_components.ts +++ b/web/src/settings_components.ts @@ -1575,6 +1575,7 @@ export const group_setting_widget_map = new Map([ ["can_add_subscribers_group", null], ["can_administer_channel_group", null], + ["can_move_messages_out_of_channel_group", null], ["can_move_messages_within_channel_group", null], ["can_remove_subscribers_group", null], ["can_send_message_group", null], diff --git a/web/src/stream_data.ts b/web/src/stream_data.ts index ac9029eb75..9162e23a20 100644 --- a/web/src/stream_data.ts +++ b/web/src/stream_data.ts @@ -825,6 +825,35 @@ export function rewire_can_post_messages_in_stream( can_post_messages_in_stream = value; } +export function user_can_move_messages_out_of_channel(stream: StreamSubscription): boolean { + if (page_params.is_spectator) { + return false; + } + + if (stream.is_archived) { + return false; + } + + const user_can_administer_channel = settings_data.user_has_permission_for_group_setting( + stream.can_administer_channel_group, + "can_administer_channel_group", + "stream", + ); + + if (user_can_administer_channel) { + return true; + } + + return ( + settings_data.user_can_move_messages_between_streams() || + settings_data.user_has_permission_for_group_setting( + stream.can_move_messages_out_of_channel_group, + "can_move_messages_out_of_channel_group", + "stream", + ) + ); +} + export function user_can_move_messages_within_channel(stream: StreamSubscription): boolean { if (page_params.is_spectator) { return false; diff --git a/web/src/stream_popover.ts b/web/src/stream_popover.ts index a0822c78d1..c08da7b883 100644 --- a/web/src/stream_popover.ts +++ b/web/src/stream_popover.ts @@ -32,7 +32,6 @@ import * as people from "./people.ts"; import * as popover_menus from "./popover_menus.ts"; import {left_sidebar_tippy_options} from "./popover_menus.ts"; import {web_channel_default_view_values} from "./settings_config.ts"; -import * as settings_data from "./settings_data.ts"; import {realm} from "./state_data.ts"; import * as stream_data from "./stream_data.ts"; import * as stream_settings_api from "./stream_settings_api.ts"; @@ -387,7 +386,7 @@ export async function build_move_topic_to_stream_popover( // input based on can_move_messages_between_topics_group. In other cases, message object is // available and thus we check the time-based permissions as well in the // below if block to enable or disable the stream and topic input. - let disable_stream_input = !settings_data.user_can_move_messages_between_streams(); + let disable_stream_input = !stream_data.user_can_move_messages_out_of_channel(stream); args.disable_topic_input = !stream_data.user_can_move_messages_within_channel(stream); let modal_heading; diff --git a/web/src/stream_types.ts b/web/src/stream_types.ts index fac12bae9e..438dd78a45 100644 --- a/web/src/stream_types.ts +++ b/web/src/stream_types.ts @@ -13,6 +13,7 @@ export type StreamPostPolicy = (typeof StreamPostPolicy)[keyof typeof StreamPost export const stream_permission_group_settings_schema = z.enum([ "can_add_subscribers_group", "can_administer_channel_group", + "can_move_messages_out_of_channel_group", "can_move_messages_within_channel_group", "can_remove_subscribers_group", "can_send_message_group", @@ -31,6 +32,7 @@ export type StreamTopicsPolicy = z.infer; export const stream_schema = z.object({ can_add_subscribers_group: group_setting_value_schema, can_administer_channel_group: group_setting_value_schema, + can_move_messages_out_of_channel_group: group_setting_value_schema, can_move_messages_within_channel_group: group_setting_value_schema, can_remove_subscribers_group: group_setting_value_schema, can_send_message_group: group_setting_value_schema, diff --git a/web/templates/stream_settings/stream_types.hbs b/web/templates/stream_settings/stream_types.hbs index d1292fe823..0fb71e9020 100644 --- a/web/templates/stream_settings/stream_types.hbs +++ b/web/templates/stream_settings/stream_types.hbs @@ -131,6 +131,11 @@ label=group_setting_labels.can_move_messages_within_channel_group prefix=prefix }} + {{> ../settings/group_setting_value_pill_input + setting_name="can_move_messages_out_of_channel_group" + label=group_setting_labels.can_move_messages_out_of_channel_group + prefix=prefix }} + {{#if (or is_owner is_stream_edit)}}
diff --git a/web/tests/compose_validate.test.cjs b/web/tests/compose_validate.test.cjs index 9b30674cd7..844317c278 100644 --- a/web/tests/compose_validate.test.cjs +++ b/web/tests/compose_validate.test.cjs @@ -785,6 +785,7 @@ test_ui("test warn_if_topic_resolved", ({override, mock_template}) => { stream_id: 111, name: "random", can_administer_channel_group: nobody.id, + can_move_messages_out_of_channel_group: nobody.id, can_move_messages_within_channel_group: nobody.id, }; stream_data.add_sub(sub); diff --git a/web/tests/lib/events.cjs b/web/tests/lib/events.cjs index 9281d572a0..c280db54e7 100644 --- a/web/tests/lib/events.cjs +++ b/web/tests/lib/events.cjs @@ -56,6 +56,7 @@ exports.test_streams = { stream_post_policy: 1, topics_policy: "inherit", can_administer_channel_group: 2, + can_move_messages_out_of_channel_group: 2, can_move_messages_within_channel_group: 2, can_send_message_group: 2, can_remove_subscribers_group: 2, @@ -78,6 +79,7 @@ exports.test_streams = { stream_post_policy: 1, topics_policy: "inherit", can_administer_channel_group: 2, + can_move_messages_out_of_channel_group: 2, can_move_messages_within_channel_group: 2, can_send_message_group: 2, can_remove_subscribers_group: 2, diff --git a/web/tests/message_edit.test.cjs b/web/tests/message_edit.test.cjs index 70f0b35905..64a77b1b5e 100644 --- a/web/tests/message_edit.test.cjs +++ b/web/tests/message_edit.test.cjs @@ -151,7 +151,8 @@ run_test("is_stream_editable", ({override}) => { type: "stream", }; override(realm, "realm_allow_message_editing", true); - override(settings_data, "user_can_move_messages_between_streams", () => true); + override(stream_data, "user_can_move_messages_out_of_channel", () => true); + override(stream_data, "get_sub_by_id", () => ({})); override(current_user, "is_moderator", true); override(stream_data, "is_stream_archived", () => false); @@ -167,7 +168,7 @@ run_test("is_stream_editable", ({override}) => { message.sent_by_me = false; assert.equal(message_edit.is_stream_editable(message), true); - override(settings_data, "user_can_move_messages_between_streams", () => false); + override(stream_data, "user_can_move_messages_out_of_channel", () => false); assert.equal(message_edit.is_stream_editable(message), false); override(current_user, "is_moderator", false); @@ -176,7 +177,7 @@ run_test("is_stream_editable", ({override}) => { override(realm, "realm_move_messages_between_streams_limit_seconds", 259200); message.timestamp = current_timestamp - 60; - override(settings_data, "user_can_move_messages_between_streams", () => true); + override(stream_data, "user_can_move_messages_out_of_channel", () => true); assert.equal(message_edit.is_stream_editable(message), true); message.timestamp = current_timestamp - 600000; diff --git a/web/tests/stream_data.test.cjs b/web/tests/stream_data.test.cjs index 4c62c07a90..238d7acbcd 100644 --- a/web/tests/stream_data.test.cjs +++ b/web/tests/stream_data.test.cjs @@ -1518,6 +1518,41 @@ test("can_post_messages_in_stream", ({override}) => { assert.equal(stream_data.can_post_messages_in_stream(social), false); }); +test("can_move_messages_out_of_channel", ({override}) => { + const social = { + subscribed: true, + name: "social", + stream_id: 10, + can_administer_channel_group: nobody_group.id, + can_move_messages_out_of_channel_group: nobody_group.id, + }; + const scotland = { + subscribed: true, + name: "scotland", + stream_id: 11, + can_administer_channel_group: nobody_group.id, + can_move_messages_out_of_channel_group: everyone_group.id, + }; + + override(realm, "realm_can_move_messages_between_channels_group", nobody_group.id); + override(current_user, "user_id", admin_user_id); + assert.equal(stream_data.user_can_move_messages_out_of_channel(social), false); + social.can_administer_channel_group = admins_group.id; + assert.equal(stream_data.user_can_move_messages_out_of_channel(social), true); + + override(current_user, "user_id", moderator_user_id); + assert.equal(stream_data.user_can_move_messages_out_of_channel(social), false); + + assert.equal(stream_data.user_can_move_messages_out_of_channel(scotland), true); + + page_params.is_spectator = true; + assert.equal(stream_data.user_can_move_messages_out_of_channel(scotland), false); + + page_params.is_spectator = false; + scotland.is_archived = true; + assert.equal(stream_data.user_can_move_messages_out_of_channel(scotland), false); +}); + test("can_move_messages_within_channel", ({override}) => { const social = { subscribed: true, diff --git a/zerver/actions/message_edit.py b/zerver/actions/message_edit.py index 892a91a7a5..e6b4f325cb 100644 --- a/zerver/actions/message_edit.py +++ b/zerver/actions/message_edit.py @@ -56,6 +56,7 @@ from zerver.lib.streams import ( access_stream_by_id_for_message, can_access_stream_history, can_edit_topic, + can_move_messages_out_of_channel, check_stream_access_based_on_can_send_message_group, get_stream_topics_policy, notify_stream_is_recently_active_update, @@ -1561,7 +1562,7 @@ def check_update_message( if isinstance(message_edit_request, StreamMessageEditRequest): if message_edit_request.is_stream_edited: assert message.is_stream_message() - if not user_profile.can_move_messages_between_streams(): + if not can_move_messages_out_of_channel(user_profile, message_edit_request.orig_stream): raise JsonableError(_("You don't have permission to move this message")) check_stream_access_based_on_can_send_message_group( diff --git a/zerver/actions/streams.py b/zerver/actions/streams.py index adb1cc2f39..a00bbaa09e 100644 --- a/zerver/actions/streams.py +++ b/zerver/actions/streams.py @@ -458,6 +458,9 @@ def send_subscription_add_events( is_archived=stream_dict["is_archived"], can_add_subscribers_group=stream_dict["can_add_subscribers_group"], can_administer_channel_group=stream_dict["can_administer_channel_group"], + can_move_messages_out_of_channel_group=stream_dict[ + "can_move_messages_out_of_channel_group" + ], can_move_messages_within_channel_group=stream_dict[ "can_move_messages_within_channel_group" ], diff --git a/zerver/lib/event_types.py b/zerver/lib/event_types.py index e9090a6de0..b1807f90b7 100644 --- a/zerver/lib/event_types.py +++ b/zerver/lib/event_types.py @@ -824,6 +824,7 @@ class EventScheduledMessagesUpdate(BaseEvent): class BasicStreamFields(BaseModel): is_archived: bool can_administer_channel_group: int | UserGroupMembersDict + can_move_messages_out_of_channel_group: int | UserGroupMembersDict can_move_messages_within_channel_group: int | UserGroupMembersDict can_remove_subscribers_group: int | UserGroupMembersDict can_send_message_group: int | UserGroupMembersDict @@ -888,6 +889,7 @@ class EventSubmessage(BaseEvent): class SingleSubscription(BaseModel): is_archived: bool can_administer_channel_group: int | UserGroupMembersDict + can_move_messages_out_of_channel_group: int | UserGroupMembersDict can_move_messages_within_channel_group: int | UserGroupMembersDict can_remove_subscribers_group: int | UserGroupMembersDict can_send_message_group: int | UserGroupMembersDict diff --git a/zerver/lib/streams.py b/zerver/lib/streams.py index a536e033d0..02703a072b 100644 --- a/zerver/lib/streams.py +++ b/zerver/lib/streams.py @@ -88,6 +88,7 @@ class StreamDict(TypedDict, total=False): topics_policy: int | None can_add_subscribers_group: UserGroup | None can_administer_channel_group: UserGroup | None + can_move_messages_out_of_channel_group: UserGroup | None can_move_messages_within_channel_group: UserGroup | None can_send_message_group: UserGroup | None can_remove_subscribers_group: UserGroup | None @@ -274,6 +275,7 @@ def create_stream_if_needed( topics_policy: int | None = None, can_add_subscribers_group: UserGroup | None = None, can_administer_channel_group: UserGroup | None = None, + can_move_messages_out_of_channel_group: UserGroup | None = None, can_move_messages_within_channel_group: UserGroup | None = None, can_send_message_group: UserGroup | None = None, can_remove_subscribers_group: UserGroup | None = None, @@ -400,6 +402,9 @@ def create_streams_if_needed( topics_policy=stream_dict.get("topics_policy", None), can_add_subscribers_group=stream_dict.get("can_add_subscribers_group", None), can_administer_channel_group=stream_dict.get("can_administer_channel_group", None), + can_move_messages_out_of_channel_group=stream_dict.get( + "can_move_messages_out_of_channel_group", None + ), can_move_messages_within_channel_group=stream_dict.get( "can_move_messages_within_channel_group", None ), @@ -1080,6 +1085,24 @@ def can_access_stream_history_by_id(user_profile: UserProfile, stream_id: int) - return can_access_stream_history(user_profile, stream) +def can_move_messages_out_of_channel(user_profile: UserProfile, stream: Stream) -> bool: + if user_profile.is_realm_admin: + return True + + if user_profile.can_move_messages_between_streams(): + return True + + if can_administer_accessible_channel(stream, user_profile): + return True + + return user_has_permission_for_group_setting( + stream.can_move_messages_out_of_channel_group_id, + user_profile, + Stream.stream_permission_group_settings["can_move_messages_out_of_channel_group"], + direct_member_only=False, + ) + + def can_move_messages_within_channel(user_profile: UserProfile, stream: Stream) -> bool: if user_profile.is_realm_admin or can_administer_accessible_channel(stream, user_profile): return True @@ -1538,6 +1561,9 @@ def stream_to_dict( can_administer_channel_group = get_group_setting_value_for_register_api( stream.can_administer_channel_group_id, anonymous_group_membership ) + can_move_messages_out_of_channel_group = get_group_setting_value_for_register_api( + stream.can_move_messages_out_of_channel_group_id, anonymous_group_membership + ) can_move_messages_within_channel_group = get_group_setting_value_for_register_api( stream.can_move_messages_within_channel_group_id, anonymous_group_membership ) @@ -1559,6 +1585,7 @@ def stream_to_dict( is_archived=stream.deactivated, can_add_subscribers_group=can_add_subscribers_group, can_administer_channel_group=can_administer_channel_group, + can_move_messages_out_of_channel_group=can_move_messages_out_of_channel_group, can_move_messages_within_channel_group=can_move_messages_within_channel_group, can_send_message_group=can_send_message_group, can_remove_subscribers_group=can_remove_subscribers_group, diff --git a/zerver/lib/subscription_info.py b/zerver/lib/subscription_info.py index 7e252a5723..b7dce876c2 100644 --- a/zerver/lib/subscription_info.py +++ b/zerver/lib/subscription_info.py @@ -70,6 +70,9 @@ def get_web_public_subs( can_administer_channel_group = get_group_setting_value_for_register_api( stream.can_administer_channel_group_id, anonymous_group_membership ) + can_move_messages_out_of_channel_group = get_group_setting_value_for_register_api( + stream.can_move_messages_out_of_channel_group_id, anonymous_group_membership + ) can_move_messages_within_channel_group = get_group_setting_value_for_register_api( stream.can_move_messages_within_channel_group_id, anonymous_group_membership ) @@ -121,6 +124,7 @@ def get_web_public_subs( audible_notifications=audible_notifications, can_add_subscribers_group=can_add_subscribers_group, can_administer_channel_group=can_administer_channel_group, + can_move_messages_out_of_channel_group=can_move_messages_out_of_channel_group, can_move_messages_within_channel_group=can_move_messages_within_channel_group, can_send_message_group=can_send_message_group, can_remove_subscribers_group=can_remove_subscribers_group, @@ -193,6 +197,9 @@ def build_stream_api_dict( can_administer_channel_group = get_group_setting_value_for_register_api( raw_stream_dict["can_administer_channel_group_id"], anonymous_group_membership ) + can_move_messages_out_of_channel_group = get_group_setting_value_for_register_api( + raw_stream_dict["can_move_messages_out_of_channel_group_id"], anonymous_group_membership + ) can_move_messages_within_channel_group = get_group_setting_value_for_register_api( raw_stream_dict["can_move_messages_within_channel_group_id"], anonymous_group_membership ) @@ -210,6 +217,7 @@ def build_stream_api_dict( is_archived=raw_stream_dict["deactivated"], can_add_subscribers_group=can_add_subscribers_group, can_administer_channel_group=can_administer_channel_group, + can_move_messages_out_of_channel_group=can_move_messages_out_of_channel_group, can_move_messages_within_channel_group=can_move_messages_within_channel_group, can_send_message_group=can_send_message_group, can_remove_subscribers_group=can_remove_subscribers_group, @@ -244,6 +252,7 @@ def build_stream_dict_for_sub( is_archived = stream_dict["is_archived"] can_add_subscribers_group = stream_dict["can_add_subscribers_group"] can_administer_channel_group = stream_dict["can_administer_channel_group"] + can_move_messages_out_of_channel_group = stream_dict["can_move_messages_out_of_channel_group"] can_move_messages_within_channel_group = stream_dict["can_move_messages_within_channel_group"] can_send_message_group = stream_dict["can_send_message_group"] can_remove_subscribers_group = stream_dict["can_remove_subscribers_group"] @@ -287,6 +296,7 @@ def build_stream_dict_for_sub( audible_notifications=audible_notifications, can_add_subscribers_group=can_add_subscribers_group, can_administer_channel_group=can_administer_channel_group, + can_move_messages_out_of_channel_group=can_move_messages_out_of_channel_group, can_move_messages_within_channel_group=can_move_messages_within_channel_group, can_send_message_group=can_send_message_group, can_remove_subscribers_group=can_remove_subscribers_group, @@ -356,6 +366,9 @@ def build_stream_dict_for_never_sub( can_administer_channel_group_value = get_group_setting_value_for_register_api( raw_stream_dict["can_administer_channel_group_id"], anonymous_group_membership ) + can_move_messages_out_of_channel_group_value = get_group_setting_value_for_register_api( + raw_stream_dict["can_move_messages_out_of_channel_group_id"], anonymous_group_membership + ) can_move_messages_within_channel_group_value = get_group_setting_value_for_register_api( raw_stream_dict["can_move_messages_within_channel_group_id"], anonymous_group_membership ) @@ -377,6 +390,7 @@ def build_stream_dict_for_never_sub( is_archived=is_archived, can_add_subscribers_group=can_add_subscribers_group_value, can_administer_channel_group=can_administer_channel_group_value, + can_move_messages_out_of_channel_group=can_move_messages_out_of_channel_group_value, can_move_messages_within_channel_group=can_move_messages_within_channel_group_value, can_send_message_group=can_send_message_group_value, can_remove_subscribers_group=can_remove_subscribers_group_value, diff --git a/zerver/lib/types.py b/zerver/lib/types.py index 13cec15f32..4262f0dfa4 100644 --- a/zerver/lib/types.py +++ b/zerver/lib/types.py @@ -160,6 +160,7 @@ class RawStreamDict(TypedDict): can_add_subscribers_group_id: int can_administer_channel_group_id: int + can_move_messages_out_of_channel_group_id: int can_move_messages_within_channel_group_id: int can_send_message_group_id: int can_remove_subscribers_group_id: int @@ -210,6 +211,7 @@ class SubscriptionStreamDict(TypedDict): audible_notifications: bool | None can_add_subscribers_group: int | UserGroupMembersDict can_administer_channel_group: int | UserGroupMembersDict + can_move_messages_out_of_channel_group: int | UserGroupMembersDict can_move_messages_within_channel_group: int | UserGroupMembersDict can_send_message_group: int | UserGroupMembersDict can_remove_subscribers_group: int | UserGroupMembersDict @@ -249,6 +251,7 @@ class NeverSubscribedStreamDict(TypedDict): is_archived: bool can_add_subscribers_group: int | UserGroupMembersDict can_administer_channel_group: int | UserGroupMembersDict + can_move_messages_out_of_channel_group: int | UserGroupMembersDict can_move_messages_within_channel_group: int | UserGroupMembersDict can_send_message_group: int | UserGroupMembersDict can_remove_subscribers_group: int | UserGroupMembersDict @@ -284,6 +287,7 @@ class DefaultStreamDict(TypedDict): is_archived: bool can_add_subscribers_group: int | UserGroupMembersDict can_administer_channel_group: int | UserGroupMembersDict + can_move_messages_out_of_channel_group: int | UserGroupMembersDict can_move_messages_within_channel_group: int | UserGroupMembersDict can_send_message_group: int | UserGroupMembersDict can_remove_subscribers_group: int | UserGroupMembersDict diff --git a/zerver/migrations/0722_stream_can_move_messages_out_of_channel_group.py b/zerver/migrations/0722_stream_can_move_messages_out_of_channel_group.py new file mode 100644 index 0000000000..37dc44fe40 --- /dev/null +++ b/zerver/migrations/0722_stream_can_move_messages_out_of_channel_group.py @@ -0,0 +1,23 @@ +# Generated by Django 5.1.8 on 2025-06-09 07:43 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("zerver", "0721_alter_stream_can_move_messages_within_channel_group"), + ] + + operations = [ + migrations.AddField( + model_name="stream", + name="can_move_messages_out_of_channel_group", + field=models.ForeignKey( + null=True, + on_delete=django.db.models.deletion.RESTRICT, + related_name="+", + to="zerver.usergroup", + ), + ), + ] diff --git a/zerver/migrations/0723_set_default_value_for_can_move_messages_out_of_channel_group.py b/zerver/migrations/0723_set_default_value_for_can_move_messages_out_of_channel_group.py new file mode 100644 index 0000000000..df4441455c --- /dev/null +++ b/zerver/migrations/0723_set_default_value_for_can_move_messages_out_of_channel_group.py @@ -0,0 +1,56 @@ +from django.db import migrations, transaction +from django.db.backends.base.schema import BaseDatabaseSchemaEditor +from django.db.migrations.state import StateApps +from django.db.models import Max, Min, OuterRef + + +def set_default_value_for_can_move_messages_out_of_channel_group( + apps: StateApps, schema_editor: BaseDatabaseSchemaEditor +) -> None: + Stream = apps.get_model("zerver", "stream") + NamedUserGroup = apps.get_model("zerver", "NamedUserGroup") + BATCH_SIZE = 1000 + + max_id = Stream.objects.filter(can_move_messages_out_of_channel_group=None).aggregate( + Max("id") + )["id__max"] + if max_id is None: + # Do nothing if there are no channels on the server. + return + + lower_bound = Stream.objects.filter(can_move_messages_out_of_channel_group=None).aggregate( + Min("id") + )["id__min"] + while lower_bound <= max_id + BATCH_SIZE / 2: + upper_bound = lower_bound + BATCH_SIZE - 1 + print(f"Processing batch {lower_bound} to {upper_bound} for Stream") + + with transaction.atomic(): + Stream.objects.filter( + id__range=(lower_bound, upper_bound), + can_move_messages_out_of_channel_group=None, + ).update( + can_move_messages_out_of_channel_group=NamedUserGroup.objects.filter( + name="role:nobody", + realm_for_sharding=OuterRef("realm_id"), + is_system_group=True, + ).values("pk") + ) + + lower_bound += BATCH_SIZE + + +class Migration(migrations.Migration): + atomic = False + + dependencies = [ + ("zerver", "0722_stream_can_move_messages_out_of_channel_group"), + ] + + operations = [ + migrations.RunPython( + set_default_value_for_can_move_messages_out_of_channel_group, + elidable=True, + reverse_code=migrations.RunPython.noop, + ) + ] diff --git a/zerver/migrations/0724_alter_stream_can_move_messages_out_of_channel_group.py b/zerver/migrations/0724_alter_stream_can_move_messages_out_of_channel_group.py new file mode 100644 index 0000000000..243a6ad86d --- /dev/null +++ b/zerver/migrations/0724_alter_stream_can_move_messages_out_of_channel_group.py @@ -0,0 +1,22 @@ +# Generated by Django 5.1.8 on 2025-06-10 04:53 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("zerver", "0723_set_default_value_for_can_move_messages_out_of_channel_group"), + ] + + operations = [ + migrations.AlterField( + model_name="stream", + name="can_move_messages_out_of_channel_group", + field=models.ForeignKey( + on_delete=django.db.models.deletion.RESTRICT, + related_name="+", + to="zerver.usergroup", + ), + ), + ] diff --git a/zerver/models/streams.py b/zerver/models/streams.py index a9087a1fa5..044a087ab9 100644 --- a/zerver/models/streams.py +++ b/zerver/models/streams.py @@ -146,6 +146,9 @@ class Stream(models.Model): can_administer_channel_group = models.ForeignKey( UserGroup, on_delete=models.RESTRICT, related_name="+" ) + can_move_messages_out_of_channel_group = models.ForeignKey( + UserGroup, on_delete=models.RESTRICT, related_name="+" + ) can_move_messages_within_channel_group = models.ForeignKey( UserGroup, on_delete=models.RESTRICT, related_name="+" ) @@ -178,6 +181,11 @@ class Stream(models.Model): allow_everyone_group=False, default_group_name="stream_creator_or_nobody", ), + "can_move_messages_out_of_channel_group": GroupPermissionSetting( + allow_nobody_group=True, + allow_everyone_group=True, + default_group_name=SystemGroups.NOBODY, + ), "can_move_messages_within_channel_group": GroupPermissionSetting( allow_nobody_group=True, allow_everyone_group=True, @@ -259,6 +267,7 @@ class Stream(models.Model): "subscriber_count", "can_add_subscribers_group_id", "can_administer_channel_group_id", + "can_move_messages_out_of_channel_group_id", "can_move_messages_within_channel_group_id", "can_send_message_group_id", "can_remove_subscribers_group_id", diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index 41e210f8d5..9170d71214 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -11577,6 +11577,8 @@ paths: $ref: "#/components/schemas/CanRemoveSubscribersGroup" can_administer_channel_group: $ref: "#/components/schemas/CanAdministerChannelGroup" + can_move_messages_out_of_channel_group: + $ref: "#/components/schemas/CanMoveMessagesOutOfChannelGroup" can_move_messages_within_channel_group: $ref: "#/components/schemas/CanMoveMessagesWithinChannelGroup" can_send_message_group: @@ -11621,6 +11623,8 @@ paths: contentType: application/json can_administer_channel_group: contentType: application/json + can_move_messages_out_of_channel_group: + contentType: application/json can_move_messages_within_channel_group: contentType: application/json can_send_message_group: @@ -16237,6 +16241,7 @@ paths: can_add_subscribers_group: {} can_remove_subscribers_group: {} can_administer_channel_group: {} + can_move_messages_out_of_channel_group: {} can_move_messages_within_channel_group: {} can_send_message_group: {} can_subscribe_group: {} @@ -21478,6 +21483,7 @@ paths: can_add_subscribers_group: {} can_remove_subscribers_group: {} can_administer_channel_group: {} + can_move_messages_out_of_channel_group: {} can_move_messages_within_channel_group: {} can_send_message_group: {} can_subscribe_group: {} @@ -21902,6 +21908,32 @@ paths: "old": 15, } - $ref: "#/components/schemas/GroupSettingValueUpdate" + can_move_messages_out_of_channel_group: + allOf: + - description: | + The set of users who have permission to move messages out of this channel + expressed as an [update to a group-setting value][update-group-setting]. + + [update-group-setting]: /api/group-setting-values#updating-group-setting-values + + Note that a user must [have content access](/help/channel-permissions) to a + channel in order to move messages out of the channel. + + Channel administrators and users present in the organization-level + `can_move_messages_between_channels_group` setting can always move messages + out of the channel if they [have content access](/help/channel-permissions) to + the channel. + + **Changes**: New in Zulip 11.0 (feature level 396). Prior to this + change, only the users in `can_move_messages_between_channels_group` were able + move messages between channels. + example: + { + "new": + {"direct_members": [10], "direct_subgroups": [11]}, + "old": 15, + } + - $ref: "#/components/schemas/GroupSettingValueUpdate" can_move_messages_within_channel_group: allOf: - description: | @@ -21993,6 +22025,8 @@ paths: contentType: application/json can_administer_channel_group: contentType: application/json + can_move_messages_out_of_channel_group: + contentType: application/json can_move_messages_within_channel_group: contentType: application/json can_send_message_group: @@ -23981,6 +24015,7 @@ components: can_add_subscribers_group: {} can_remove_subscribers_group: {} can_administer_channel_group: {} + can_move_messages_out_of_channel_group: {} can_move_messages_within_channel_group: {} can_send_message_group: {} can_subscribe_group: {} @@ -24177,6 +24212,8 @@ components: $ref: "#/components/schemas/CanRemoveSubscribersGroup" can_administer_channel_group: $ref: "#/components/schemas/CanAdministerChannelGroup" + can_move_messages_out_of_channel_group: + $ref: "#/components/schemas/CanMoveMessagesOutOfChannelGroup" can_move_messages_within_channel_group: $ref: "#/components/schemas/CanMoveMessagesWithinChannelGroup" can_send_message_group: @@ -25270,6 +25307,8 @@ components: $ref: "#/components/schemas/CanRemoveSubscribersGroup" can_administer_channel_group: $ref: "#/components/schemas/CanAdministerChannelGroup" + can_move_messages_out_of_channel_group: + $ref: "#/components/schemas/CanMoveMessagesOutOfChannelGroup" can_move_messages_within_channel_group: $ref: "#/components/schemas/CanMoveMessagesWithinChannelGroup" can_send_message_group: @@ -27135,6 +27174,26 @@ components: change, the permission to administer channels was limited to realm administrators. + [setting-values]: /api/group-setting-values + CanMoveMessagesOutOfChannelGroup: + allOf: + - $ref: "#/components/schemas/GroupSettingValue" + - description: | + A [group-setting value][setting-values] defining the set of users + who have permission to move messages out of this channel. + + Note that a user must [have content access](/help/channel-permissions) to a + channel in order to move messages out of the channel. + + Channel administrators and users present in the organization-level + `can_move_messages_between_channels_group` setting can always move messages + out of the channel if they [have content access](/help/channel-permissions) to + the channel. + + **Changes**: New in Zulip 11.0 (feature level 396). Prior to this + change, only the users in `can_move_messages_between_channels_group` were able + move messages between channels. + [setting-values]: /api/group-setting-values CanMoveMessagesWithinChannelGroup: allOf: diff --git a/zerver/tests/test_message_move_stream.py b/zerver/tests/test_message_move_stream.py index 3e60a3b62c..5ffbb83ed4 100644 --- a/zerver/tests/test_message_move_stream.py +++ b/zerver/tests/test_message_move_stream.py @@ -46,6 +46,34 @@ class MessageMoveStreamTest(ZulipTestCase): False, ) + def assert_move_message( + self, + user: str, + orig_stream: Stream, + stream_id: int | None = None, + topic_name: str | None = None, + expected_error: str | None = None, + ) -> None: + user_profile = self.example_user(user) + self.subscribe(user_profile, orig_stream.name) + message_id = self.send_stream_message(user_profile, orig_stream.name) + + params_dict: dict[str, str | int] = {} + if stream_id is not None: + params_dict["stream_id"] = stream_id + if topic_name is not None: + params_dict["topic"] = topic_name + + result = self.api_patch( + user_profile, + "/api/v1/messages/" + str(message_id), + params_dict, + ) + if expected_error is not None: + self.assert_json_error(result, expected_error) + else: + self.assert_json_success(result) + def prepare_move_topics( self, user_email: str, @@ -864,8 +892,10 @@ class MessageMoveStreamTest(ZulipTestCase): nobody_system_group, acting_user=None, ) - check_move_message_according_to_permission("desdemona", expect_fail=True) - check_move_message_according_to_permission("iago", expect_fail=True) + check_move_message_according_to_permission("shiva", expect_fail=True) + # Iago can move messages between channels via channel-level + # `can_move_messages_out_of_channel_group` permission. + check_move_message_according_to_permission("iago") # Check sending messages when only administrators are allowed. do_change_realm_permission_group_setting( @@ -922,8 +952,8 @@ class MessageMoveStreamTest(ZulipTestCase): check_move_message_according_to_permission("othello") check_move_message_according_to_permission("cordelia") - # Iago is not in the allowed user group, so cannot move messages. - check_move_message_according_to_permission("iago", expect_fail=True) + # Shiva is not in the allowed user group, so cannot move messages. + check_move_message_according_to_permission("shiva", expect_fail=True) # Test for checking the setting for anonymous user group. anonymous_user_group = self.create_or_update_anonymous_group_for_setting( @@ -1154,6 +1184,99 @@ class MessageMoveStreamTest(ZulipTestCase): ) check_move_message_to_stream(hamlet) + def test_can_move_messages_out_of_channel_group(self) -> None: + hamlet = self.example_user("hamlet") + cordelia = self.example_user("cordelia") + iago = self.example_user("iago") + realm = hamlet.realm + + members_system_group = NamedUserGroup.objects.get( + name=SystemGroups.MEMBERS, realm=realm, is_system_group=True + ) + nobody_system_group = NamedUserGroup.objects.get( + name=SystemGroups.NOBODY, realm=realm, is_system_group=True + ) + + expected_error = "You don't have permission to move this message" + + do_change_realm_permission_group_setting( + realm, + "can_move_messages_between_topics_group", + nobody_system_group, + acting_user=None, + ) + do_change_realm_permission_group_setting( + realm, + "can_move_messages_between_channels_group", + nobody_system_group, + acting_user=None, + ) + + stream_1 = get_stream("Denmark", realm) + stream_2 = get_stream("Verona", realm) + + # Nobody is allowed to move messages. + self.assert_move_message( + "hamlet", stream_1, stream_id=stream_2.id, expected_error=expected_error + ) + # Realm admin can always move messages out of the channel. + self.assert_move_message("iago", stream_1, stream_id=stream_2.id) + + do_change_stream_group_based_setting( + stream_1, + "can_move_messages_out_of_channel_group", + members_system_group, + acting_user=iago, + ) + # Only members are allowed to move messages out of the channel. + self.assert_move_message("hamlet", stream_1, stream_id=stream_2.id) + self.assert_move_message("cordelia", stream_1, stream_id=stream_2.id) + # Guests are not allowed. + self.assert_move_message( + "polonius", stream_1, stream_id=stream_2.id, expected_error=expected_error + ) + + # Nobody is allowed to edit the topics when moving messages between the channels. + self.assert_move_message( + "hamlet", + stream_1, + stream_id=stream_2.id, + topic_name="new topic", + expected_error="You don't have permission to edit this message", + ) + + do_change_realm_permission_group_setting( + realm, + "can_move_messages_between_topics_group", + members_system_group, + acting_user=None, + ) + # Now Hamlet is in `can_move_messages_between_topics_group`, so can edit topics. + self.assert_move_message("hamlet", stream_1, stream_id=stream_2.id, topic_name="new topic") + + user_group = check_add_user_group( + realm, "new_group", [hamlet, cordelia], acting_user=hamlet + ) + do_change_stream_group_based_setting( + stream_1, "can_move_messages_out_of_channel_group", user_group, acting_user=iago + ) + + # Hamlet and Cordelia are in the `can_move_messages_out_of_channel_group`, + # so they can move messages out of the channel. + self.assert_move_message("cordelia", stream_1, stream_id=stream_2.id) + self.assert_move_message("hamlet", stream_1, stream_id=stream_2.id) + # But Shiva is not, so he can't. + self.assert_move_message( + "shiva", stream_1, stream_id=stream_2.id, expected_error=expected_error + ) + + do_change_stream_group_based_setting( + stream_1, "can_administer_channel_group", members_system_group, acting_user=iago + ) + # Channel administrators with content access can always move messages out of + # the channel even if they are not in `can_move_messages_out_of_channel_group`. + self.assert_move_message("shiva", stream_1, stream_id=stream_2.id) + def test_move_message_to_stream_with_topic_editing_not_allowed(self) -> None: (user_profile, old_stream, new_stream, msg_id, msg_id_later) = self.prepare_move_topics( "othello", "old_stream_1", "new_stream_1", "test" @@ -1212,7 +1335,7 @@ class MessageMoveStreamTest(ZulipTestCase): "iago", "test move stream", "new stream", "test" ) - with self.assert_database_query_count(61), self.assert_memcached_count(14): + with self.assert_database_query_count(59), self.assert_memcached_count(14): result = self.client_patch( f"/json/messages/{msg_id}", { @@ -1536,7 +1659,7 @@ class MessageMoveStreamTest(ZulipTestCase): second_stream, msg_id, msg_id_later, - ) = self.prepare_move_topics("iago", "first stream", "second stream", "test") + ) = self.prepare_move_topics("shiva", "first stream", "second stream", "test") # 'prepare_move_topics' sends 3 messages in the first_stream messages = get_topic_messages(user_profile, first_stream, "test") diff --git a/zerver/tests/test_message_move_topic.py b/zerver/tests/test_message_move_topic.py index 6e8f141893..7a79d265a2 100644 --- a/zerver/tests/test_message_move_topic.py +++ b/zerver/tests/test_message_move_topic.py @@ -514,7 +514,7 @@ class MessageMoveTopicTest(ZulipTestCase): set_topic_visibility_policy(desdemona, muted_topics, UserTopic.VisibilityPolicy.MUTED) set_topic_visibility_policy(cordelia, muted_topics, UserTopic.VisibilityPolicy.MUTED) - with self.assert_database_query_count(31): + with self.assert_database_query_count(29): check_update_message( user_profile=desdemona, message_id=message_id, @@ -544,7 +544,7 @@ class MessageMoveTopicTest(ZulipTestCase): ] set_topic_visibility_policy(desdemona, muted_topics, UserTopic.VisibilityPolicy.MUTED) set_topic_visibility_policy(cordelia, muted_topics, UserTopic.VisibilityPolicy.MUTED) - with self.assert_database_query_count(36): + with self.assert_database_query_count(35): check_update_message( user_profile=desdemona, message_id=message_id, @@ -577,7 +577,7 @@ class MessageMoveTopicTest(ZulipTestCase): set_topic_visibility_policy(desdemona, muted_topics, UserTopic.VisibilityPolicy.MUTED) set_topic_visibility_policy(cordelia, muted_topics, UserTopic.VisibilityPolicy.MUTED) - with self.assert_database_query_count(33): + with self.assert_database_query_count(31): check_update_message( user_profile=desdemona, message_id=message_id, @@ -600,7 +600,7 @@ class MessageMoveTopicTest(ZulipTestCase): second_message_id = self.send_stream_message( hamlet, stream_name, topic_name="changed topic name", content="Second message" ) - with self.assert_database_query_count(26): + with self.assert_database_query_count(25): check_update_message( user_profile=desdemona, message_id=second_message_id, diff --git a/zerver/views/streams.py b/zerver/views/streams.py index 4115a5e289..d79a34bbda 100644 --- a/zerver/views/streams.py +++ b/zerver/views/streams.py @@ -289,6 +289,7 @@ def update_stream_backend( ] = None, can_add_subscribers_group: Json[GroupSettingChangeRequest] | None = None, can_administer_channel_group: Json[GroupSettingChangeRequest] | None = None, + can_move_messages_out_of_channel_group: Json[GroupSettingChangeRequest] | None = None, can_move_messages_within_channel_group: Json[GroupSettingChangeRequest] | None = None, can_send_message_group: Json[GroupSettingChangeRequest] | None = None, can_remove_subscribers_group: Json[GroupSettingChangeRequest] | None = None, @@ -660,6 +661,7 @@ def add_subscriptions_backend( ] = None, can_add_subscribers_group: Json[int | UserGroupMembersData] | None = None, can_administer_channel_group: Json[int | UserGroupMembersData] | None = None, + can_move_messages_out_of_channel_group: Json[int | UserGroupMembersData] | None = None, can_move_messages_within_channel_group: Json[int | UserGroupMembersData] | None = None, can_send_message_group: Json[int | UserGroupMembersData] | None = None, can_remove_subscribers_group: Json[int | UserGroupMembersData] | None = None, @@ -748,6 +750,9 @@ def add_subscriptions_backend( stream_dict_copy["can_administer_channel_group"] = group_settings_map[ "can_administer_channel_group" ] + stream_dict_copy["can_move_messages_out_of_channel_group"] = group_settings_map[ + "can_move_messages_out_of_channel_group" + ] stream_dict_copy["can_move_messages_within_channel_group"] = group_settings_map[ "can_move_messages_within_channel_group" ]