From fa928d5cd168ef4cd4216f2680b97abeebc77356 Mon Sep 17 00:00:00 2001 From: Ganesh Pawar Date: Sat, 27 Mar 2021 10:18:37 +0530 Subject: [PATCH] streams: Split setting for stream creation policy. Users wanted a feature where they could specify which users can create public streams and which users can create private streams. This splits stream creation code into two parts, public and private stream creation. Fixes #17009. --- frontend_tests/node_tests/dispatch.js | 7 ++- frontend_tests/node_tests/hotkey.js | 6 +- frontend_tests/node_tests/lib/events.js | 11 +++- frontend_tests/node_tests/settings_data.js | 11 +++- frontend_tests/node_tests/settings_org.js | 30 ++++++---- frontend_tests/puppeteer_tests/admin.ts | 3 +- static/js/hotkey.js | 3 +- static/js/page_params.ts | 3 +- static/js/popover_menus.js | 4 +- static/js/server_events_dispatch.js | 3 +- static/js/settings_data.ts | 8 ++- static/js/settings_org.js | 3 +- static/js/stream_settings_ui.js | 23 ++++++- static/styles/settings.css | 3 +- .../organization_permissions_admin.hbs | 10 +++- .../stream_settings/stream_creation_form.hbs | 1 - templates/zerver/api/changelog.md | 11 ++++ version.py | 2 +- zerver/lib/events.py | 22 ++++++- zerver/lib/streams.py | 13 ++-- .../0358_split_create_stream_policy.py | 46 ++++++++++++++ zerver/models.py | 18 ++++-- zerver/openapi/zulip.yaml | 60 ++++++++++++++++++- zerver/tests/test_events.py | 3 +- zerver/tests/test_home.py | 5 +- zerver/tests/test_realm.py | 6 +- zerver/tests/test_subs.py | 60 ++++++++++++++----- zerver/views/realm.py | 5 +- 28 files changed, 309 insertions(+), 71 deletions(-) create mode 100644 zerver/migrations/0358_split_create_stream_policy.py diff --git a/frontend_tests/node_tests/dispatch.js b/frontend_tests/node_tests/dispatch.js index b76d65d257..ee3d362f61 100644 --- a/frontend_tests/node_tests/dispatch.js +++ b/frontend_tests/node_tests/dispatch.js @@ -344,8 +344,11 @@ run_test("realm settings", ({override}) => { assert.equal(page_params[parameter_name], 1); } - let event = event_fixtures.realm__update__create_stream_policy; - test_realm_integer(event, "realm_create_stream_policy"); + let event = event_fixtures.realm__update__create_private_stream_policy; + test_realm_integer(event, "realm_create_private_stream_policy"); + + event = event_fixtures.realm__update__create_public_stream_policy; + test_realm_integer(event, "realm_create_public_stream_policy"); event = event_fixtures.realm__update__invite_to_stream_policy; test_realm_integer(event, "realm_invite_to_stream_policy"); diff --git a/frontend_tests/node_tests/hotkey.js b/frontend_tests/node_tests/hotkey.js index 7595e2e3d9..6f9dc58585 100644 --- a/frontend_tests/node_tests/hotkey.js +++ b/frontend_tests/node_tests/hotkey.js @@ -275,13 +275,15 @@ run_test("allow normal typing when processing text", ({override}) => { }); run_test("streams", ({override}) => { - settings_data.user_can_create_streams = () => true; + settings_data.user_can_create_private_streams = () => true; + settings_data.user_can_create_public_streams = () => true; override(overlays, "streams_open", () => true); override(overlays, "is_active", () => true); assert_mapping("S", stream_settings_ui, "keyboard_sub"); assert_mapping("V", stream_settings_ui, "view_stream"); assert_mapping("n", stream_settings_ui, "open_create_stream"); - settings_data.user_can_create_streams = () => false; + settings_data.user_can_create_private_streams = () => false; + settings_data.user_can_create_public_streams = () => false; assert_unmapped("n"); }); diff --git a/frontend_tests/node_tests/lib/events.js b/frontend_tests/node_tests/lib/events.js index cc59199414..177ad51451 100644 --- a/frontend_tests/node_tests/lib/events.js +++ b/frontend_tests/node_tests/lib/events.js @@ -240,10 +240,17 @@ exports.fixtures = { value: 1, }, - realm__update__create_stream_policy: { + realm__update__create_private_stream_policy: { type: "realm", op: "update", - property: "create_stream_policy", + property: "create_private_stream_policy", + value: 2, + }, + + realm__update__create_public_stream_policy: { + type: "realm", + op: "update", + property: "create_public_stream_policy", value: 2, }, diff --git a/frontend_tests/node_tests/settings_data.js b/frontend_tests/node_tests/settings_data.js index d28ac3c0fb..d32e704ce2 100644 --- a/frontend_tests/node_tests/settings_data.js +++ b/frontend_tests/node_tests/settings_data.js @@ -168,9 +168,14 @@ function test_policy(label, policy, validation_func) { } test_policy( - "user_can_create_streams", - "realm_create_stream_policy", - settings_data.user_can_create_streams, + "user_can_create_private_streams", + "realm_create_private_stream_policy", + settings_data.user_can_create_private_streams, +); +test_policy( + "user_can_create_public_streams", + "realm_create_public_stream_policy", + settings_data.user_can_create_public_streams, ); test_policy( "user_can_subscribe_other_users", diff --git a/frontend_tests/node_tests/settings_org.js b/frontend_tests/node_tests/settings_org.js index 2e05efe694..787f0ebf84 100644 --- a/frontend_tests/node_tests/settings_org.js +++ b/frontend_tests/node_tests/settings_org.js @@ -166,11 +166,11 @@ function test_submit_settings_form(override, submit_form) { realm_email_address_visibility: settings_config.email_address_visibility_values.admins_only.code, realm_add_custom_emoji_policy: settings_config.common_policy_values.by_admins_only.code, - realm_create_stream_by_admins_only: true, realm_waiting_period_threshold: 1, realm_default_language: '"es"', realm_invite_to_stream_policy: settings_config.common_policy_values.by_admins_only.code, - realm_create_stream_policy: settings_config.common_policy_values.by_members.code, + realm_create_private_stream_policy: settings_config.common_policy_values.by_members.code, + realm_create_public_stream_policy: settings_config.common_policy_values.by_members.code, realm_invite_to_realm_policy: settings_config.common_policy_values.by_members.code, }); @@ -204,10 +204,15 @@ function test_submit_settings_form(override, submit_form) { invite_to_stream_policy_elem.attr("id", "id_realm_invite_to_stream_policy"); invite_to_stream_policy_elem.data = () => "number"; - const create_stream_policy_elem = $("#id_realm_create_stream_policy"); - create_stream_policy_elem.val("2"); - create_stream_policy_elem.attr("id", "id_realm_create_stream_policy"); - create_stream_policy_elem.data = () => "number"; + const create_public_stream_policy_elem = $("#id_realm_create_public_stream_policy"); + create_public_stream_policy_elem.val("2"); + create_public_stream_policy_elem.attr("id", "id_realm_create_public_stream_policy"); + create_public_stream_policy_elem.data = () => "number"; + + const create_private_stream_policy_elem = $("#id_realm_create_private_stream_policy"); + create_private_stream_policy_elem.val("2"); + create_private_stream_policy_elem.attr("id", "id_realm_create_private_stream_policy"); + create_private_stream_policy_elem.data = () => "number"; const add_custom_emoji_policy_elem = $("#id_realm_add_custom_emoji_policy"); add_custom_emoji_policy_elem.val("1"); @@ -234,7 +239,8 @@ function test_submit_settings_form(override, submit_form) { bot_creation_policy_elem, email_address_visibility_elem, add_custom_emoji_policy_elem, - create_stream_policy_elem, + create_public_stream_policy_elem, + create_private_stream_policy_elem, invite_to_stream_policy_elem, ]); @@ -247,7 +253,8 @@ function test_submit_settings_form(override, submit_form) { invite_to_stream_policy: 1, email_address_visibility: 1, add_custom_emoji_policy: 1, - create_stream_policy: 2, + create_public_stream_policy: 2, + create_private_stream_policy: 2, }; assert.deepEqual(data, expected_value); @@ -457,7 +464,8 @@ function test_sync_realm_settings() { } } - test_common_policy("create_stream_policy"); + test_common_policy("create_private_stream_policy"); + test_common_policy("create_public_stream_policy"); test_common_policy("invite_to_stream_policy"); test_common_policy("invite_to_realm_policy"); @@ -467,7 +475,7 @@ function test_sync_realm_settings() { property_elem.length = 1; property_elem.attr("id", "id_realm_message_content_edit_limit_minutes"); - page_params.realm_create_stream_policy = 1; + page_params.realm_create_public_stream_policy = 1; page_params.realm_message_content_edit_limit_seconds = 120; settings_org.sync_realm_settings("message_content_edit_limit_seconds"); @@ -502,7 +510,7 @@ function test_sync_realm_settings() { property_elem.length = 1; property_elem.attr("id", "id_realm_message_content_edit_limit_minutes"); - page_params.realm_create_stream_policy = 1; + page_params.realm_create_public_stream_policy = 1; page_params.realm_message_content_edit_limit_seconds = 120; settings_org.sync_realm_settings("message_content_edit_limit_seconds"); diff --git a/frontend_tests/puppeteer_tests/admin.ts b/frontend_tests/puppeteer_tests/admin.ts index 1296804567..284031fbee 100644 --- a/frontend_tests/puppeteer_tests/admin.ts +++ b/frontend_tests/puppeteer_tests/admin.ts @@ -103,7 +103,8 @@ async function test_changing_create_streams_and_invite_to_stream_policies( page: Page, ): Promise { const policies = { - "create stream": "#id_realm_create_stream_policy", + "create private stream": "#id_realm_create_private_stream_policy", + "create public stream": "#id_realm_create_public_stream_policy", "invite to stream": "#id_realm_invite_to_stream_policy", }; const policy_values = { diff --git a/static/js/hotkey.js b/static/js/hotkey.js index 55ae334f16..b457f9d8b9 100644 --- a/static/js/hotkey.js +++ b/static/js/hotkey.js @@ -738,7 +738,8 @@ export function process_hotkey(e, hotkey) { if ( event_name === "n_key" && overlays.streams_open() && - settings_data.user_can_create_streams() + (settings_data.user_can_create_private_streams() || + settings_data.user_can_create_public_streams()) ) { stream_settings_ui.open_create_stream(); return true; diff --git a/static/js/page_params.ts b/static/js/page_params.ts index 05fb418c3f..87af33abb7 100644 --- a/static/js/page_params.ts +++ b/static/js/page_params.ts @@ -15,7 +15,8 @@ export const page_params: { is_spectator: boolean; realm_add_custom_emoji_policy: number; realm_avatar_changes_disabled: boolean; - realm_create_stream_policy: number; + realm_create_private_stream_policy: number; + realm_create_public_stream_policy: number; realm_delete_own_message_policy: number; realm_edit_topic_policy: number; realm_email_address_visibility: number; diff --git a/static/js/popover_menus.js b/static/js/popover_menus.js index ce3eb97401..3478de8fc3 100644 --- a/static/js/popover_menus.js +++ b/static/js/popover_menus.js @@ -46,7 +46,9 @@ export function initialize() { popovers.hide_all_except_sidebars(instance); instance.setContent( render_left_sidebar_stream_setting_popover({ - can_create_streams: settings_data.user_can_create_streams(), + can_create_streams: + settings_data.user_can_create_private_streams() || + settings_data.user_can_create_public_streams(), }), ); left_sidebar_stream_setting_popover_displayed = true; diff --git a/static/js/server_events_dispatch.js b/static/js/server_events_dispatch.js index fb388a2a30..f522cf3985 100644 --- a/static/js/server_events_dispatch.js +++ b/static/js/server_events_dispatch.js @@ -188,7 +188,8 @@ export function dispatch_normal_event(event) { user_group_edit_policy: noop, avatar_changes_disabled: settings_account.update_avatar_change_display, bot_creation_policy: settings_bots.update_bot_permissions_ui, - create_stream_policy: noop, + create_public_stream_policy: noop, + create_private_stream_policy: noop, invite_to_stream_policy: noop, default_code_block_language: noop, default_language: noop, diff --git a/static/js/settings_data.ts b/static/js/settings_data.ts index 44ab565837..394671591e 100644 --- a/static/js/settings_data.ts +++ b/static/js/settings_data.ts @@ -173,8 +173,12 @@ export function user_can_unsubscribe_other_users(): boolean { return page_params.is_admin; } -export function user_can_create_streams(): boolean { - return user_has_permission(page_params.realm_create_stream_policy); +export function user_can_create_private_streams(): boolean { + return user_has_permission(page_params.realm_create_private_stream_policy); +} + +export function user_can_create_public_streams(): boolean { + return user_has_permission(page_params.realm_create_public_stream_policy); } export function user_can_move_messages_between_streams(): boolean { diff --git a/static/js/settings_org.js b/static/js/settings_org.js index 22b4d2bbbb..75a8c7f1c1 100644 --- a/static/js/settings_org.js +++ b/static/js/settings_org.js @@ -193,7 +193,8 @@ function get_subsection_property_elements(element) { } const simple_dropdown_properties = [ - "realm_create_stream_policy", + "realm_create_private_stream_policy", + "realm_create_public_stream_policy", "realm_invite_to_stream_policy", "realm_user_group_edit_policy", "realm_private_message_policy", diff --git a/static/js/stream_settings_ui.js b/static/js/stream_settings_ui.js index 6db6bc9797..b9678bcd9b 100644 --- a/static/js/stream_settings_ui.js +++ b/static/js/stream_settings_ui.js @@ -601,13 +601,29 @@ export function setup_page(callback) { function populate_and_fill() { $("#subscriptions_table").empty(); + // Show only stream types the user is allowed to create. + const stream_privacy_policy_values = _.pickBy( + stream_data.stream_privacy_policy_values, + (value, key) => + (key === "public" && settings_data.user_can_create_public_streams()) || + (key !== "public" && settings_data.user_can_create_private_streams()), + ); + + // Required to mark the first item in the list of stream types as checked in stream_types.hbs. + const stream_privacy_policy = settings_data.user_can_create_public_streams() + ? stream_privacy_policy_values.public.code + : stream_privacy_policy_values.private_with_public_history.code; + const template_data = { - can_create_streams: settings_data.user_can_create_streams(), + can_create_streams: + settings_data.user_can_create_private_streams() || + settings_data.user_can_create_public_streams(), hide_all_streams: !should_list_all_streams(), max_name_length: page_params.max_stream_name_length, max_description_length: page_params.max_stream_description_length, is_owner: page_params.is_owner, - stream_privacy_policy_values: stream_data.stream_privacy_policy_values, + stream_privacy_policy_values, + stream_privacy_policy, stream_post_policy_values: stream_data.stream_post_policy_values, zulip_plan_is_not_limited: page_params.zulip_plan_is_not_limited, org_level_message_retention_setting: @@ -643,7 +659,8 @@ export function setup_page(callback) { } if ( - settings_data.user_can_create_streams() || + settings_data.user_can_create_private_streams() || + settings_data.user_can_create_public_streams() || page_params.realm_is_zephyr_mirror_realm ) { open_create_stream(); diff --git a/static/styles/settings.css b/static/styles/settings.css index 25d6e5f18d..09fdb85b52 100644 --- a/static/styles/settings.css +++ b/static/styles/settings.css @@ -1559,7 +1559,8 @@ input[type="checkbox"] { /* These have enough space for all the options in German. */ .setting_desktop_icon_count_display, #id_realm_waiting_period_setting, -#id_realm_create_stream_policy, +#id_realm_create_public_stream_policy, +#id_realm_create_private_stream_policy, #id_realm_invite_to_stream_policy, #id_realm_private_message_policy, #id_realm_add_custom_emoji_policy, diff --git a/static/templates/settings/organization_permissions_admin.hbs b/static/templates/settings/organization_permissions_admin.hbs index ee81f18d73..ec4c069473 100644 --- a/static/templates/settings/organization_permissions_admin.hbs +++ b/static/templates/settings/organization_permissions_admin.hbs @@ -98,8 +98,14 @@
- - + {{> dropdown_options_widget option_values=common_policy_values}} + +
+
+ +
diff --git a/static/templates/stream_settings/stream_creation_form.hbs b/static/templates/stream_settings/stream_creation_form.hbs index 7f5e720a42..4f5505bd9c 100644 --- a/static/templates/stream_settings/stream_creation_form.hbs +++ b/static/templates/stream_settings/stream_creation_form.hbs @@ -31,7 +31,6 @@
{{> stream_types - stream_privacy_policy=stream_privacy_policy_values.public.code stream_post_policy=stream_post_policy_values.everyone.code is_stream_edit=false }} diff --git a/templates/zerver/api/changelog.md b/templates/zerver/api/changelog.md index 9aff225324..017cd4a4a9 100644 --- a/templates/zerver/api/changelog.md +++ b/templates/zerver/api/changelog.md @@ -11,6 +11,17 @@ below features are supported. ## Changes in Zulip 5.0 +**Feature level 102** + +* [`POST /register`](/api/register-queue), `PATCH /realm`: The + `create_stream_policy` setting was split into two settings for + different types of streams: `create_private_stream_policy` and + `create_public_stream_policy`. +* [`POST /register`](/api/register-queue): The `create_stream_policy` + property was deprecated in favor of the + `create_private_stream_policy` and `create_public_stream_policy` + properties, but it still available for backwards-compatibility. + **Feature level 101** * [`POST /register`](/api/register-queue), `PATCH /realm`: Replaced diff --git a/version.py b/version.py index 74f538e9fb..13cd497d9f 100644 --- a/version.py +++ b/version.py @@ -33,7 +33,7 @@ DESKTOP_WARNING_VERSION = "5.4.3" # Changes should be accompanied by documentation explaining what the # new level means in templates/zerver/api/changelog.md, as well as # "**Changes**" entries in the endpoint's documentation in `zulip.yaml`. -API_FEATURE_LEVEL = 101 +API_FEATURE_LEVEL = 102 # 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/zerver/lib/events.py b/zerver/lib/events.py index 7194b81e71..678f209591 100644 --- a/zerver/lib/events.py +++ b/zerver/lib/events.py @@ -411,7 +411,14 @@ def fetch_initial_state_data( client_gravatar=False, ) - state["can_create_streams"] = settings_user.can_create_streams() + state["can_create_private_streams"] = settings_user.can_create_private_streams() + state["can_create_public_streams"] = settings_user.can_create_public_streams() + # TODO/compatibility: Deprecated in Zulip 5.0 (feature level + # 102); we can remove this once we no longer need to support + # legacy mobile app versions that read the old property. + state["can_create_streams"] = ( + settings_user.can_create_private_streams() or settings_user.can_create_public_streams() + ) state["can_subscribe_other_users"] = settings_user.can_subscribe_other_users() state["can_invite_others_to_realm"] = settings_user.can_invite_others_to_realm() state["is_admin"] = settings_user.is_realm_admin @@ -743,7 +750,11 @@ def apply_event( state["is_moderator"] = person["role"] == UserProfile.ROLE_MODERATOR state["is_guest"] = person["role"] == UserProfile.ROLE_GUEST # Recompute properties based on is_admin/is_guest - state["can_create_streams"] = user_profile.can_create_streams() + state["can_create_private_streams"] = user_profile.can_create_private_streams() + state["can_create_public_streams"] = user_profile.can_create_public_streams() + state["can_create_streams"] = ( + state["can_create_private_streams"] or state["can_create_public_streams"] + ) state["can_subscribe_other_users"] = user_profile.can_subscribe_other_users() state["can_invite_others_to_realm"] = user_profile.can_invite_others_to_realm() @@ -924,7 +935,8 @@ def apply_event( state["realm_upload_quota_mib"] = event["extra_data"]["upload_quota"] policy_permission_dict = { - "create_stream_policy": "can_create_streams", + "create_public_stream_policy": "can_create_public_streams", + "create_private_stream_policy": "can_create_private_streams", "invite_to_stream_policy": "can_subscribe_other_users", "invite_to_realm_policy": "can_invite_others_to_realm", } @@ -943,6 +955,10 @@ def apply_event( event["property"] ) + # Finally, we need to recompute this value from its inputs. + state["can_create_streams"] = ( + state["can_create_private_streams"] or state["can_create_public_streams"] + ) elif event["op"] == "update_dict": for key, value in event["data"].items(): state["realm_" + key] = value diff --git a/zerver/lib/streams.py b/zerver/lib/streams.py index fd1265eec8..0038ef6111 100644 --- a/zerver/lib/streams.py +++ b/zerver/lib/streams.py @@ -149,10 +149,11 @@ def create_streams_if_needed( added_streams: List[Stream] = [] existing_streams: List[Stream] = [] for stream_dict in stream_dicts: + invite_only = stream_dict.get("invite_only", False) stream, created = create_stream_if_needed( realm, stream_dict["name"], - invite_only=stream_dict.get("invite_only", False), + invite_only=invite_only, is_web_public=stream_dict.get("is_web_public", False), stream_post_policy=stream_dict.get( "stream_post_policy", Stream.STREAM_POST_POLICY_EVERYONE @@ -671,10 +672,12 @@ def list_to_streams( created_streams: List[Stream] = [] else: # autocreate=True path starts here - if not user_profile.can_create_streams(): - # Guest users case will not be handled here as it will be - # handled by the decorator in add_subscriptions_backend. - raise JsonableError(_("Insufficient permission")) + for stream_dict in missing_stream_dicts: + invite_only = stream_dict.get("invite_only", False) + if invite_only and not user_profile.can_create_private_streams(): + raise JsonableError(_("Insufficient permission")) + if not invite_only and not user_profile.can_create_public_streams(): + raise JsonableError(_("Insufficient permission")) if not autocreate: raise JsonableError( diff --git a/zerver/migrations/0358_split_create_stream_policy.py b/zerver/migrations/0358_split_create_stream_policy.py new file mode 100644 index 0000000000..c420a7f875 --- /dev/null +++ b/zerver/migrations/0358_split_create_stream_policy.py @@ -0,0 +1,46 @@ +from django.db import migrations, models +from django.db.backends.postgresql.schema import DatabaseSchemaEditor +from django.db.migrations.state import StateApps +from django.db.models import F + + +def copy_stream_policy_field(apps: StateApps, schema_editor: DatabaseSchemaEditor) -> None: + Realm = apps.get_model("zerver", "Realm") + Realm.objects.all().update(create_public_stream_policy=F("create_stream_policy")) + Realm.objects.all().update(create_private_stream_policy=F("create_stream_policy")) + + +# When reversing the migration, we have to pick one of the new fields +# to store in the original field name. This does destroy information, +# but in most cases downgrades that would reverse migrations happen +# before any real usage, so it's very likely that both values are +# identical. +def reverse_code(apps: StateApps, schema_editor: DatabaseSchemaEditor) -> None: + Realm = apps.get_model("zerver", "Realm") + Realm.objects.all().update(create_stream_policy=F("create_public_stream_policy")) + + +class Migration(migrations.Migration): + atomic = False + + dependencies = [ + ("zerver", "0357_remove_realm_allow_message_deleting"), + ] + + operations = [ + migrations.AddField( + model_name="realm", + name="create_private_stream_policy", + field=models.PositiveSmallIntegerField(default=1), + ), + migrations.AddField( + model_name="realm", + name="create_public_stream_policy", + field=models.PositiveSmallIntegerField(default=1), + ), + migrations.RunPython(copy_stream_policy_field, reverse_code=reverse_code, elidable=True), + migrations.RemoveField( + model_name="realm", + name="create_stream_policy", + ), + ] diff --git a/zerver/models.py b/zerver/models.py index e92815d00b..f45dceb5b4 100644 --- a/zerver/models.py +++ b/zerver/models.py @@ -292,7 +292,10 @@ class Realm(models.Model): add_custom_emoji_policy: int = models.PositiveSmallIntegerField(default=POLICY_MEMBERS_ONLY) # Who in the organization is allowed to create streams. - create_stream_policy: int = models.PositiveSmallIntegerField(default=POLICY_MEMBERS_ONLY) + create_public_stream_policy: int = models.PositiveSmallIntegerField(default=POLICY_MEMBERS_ONLY) + create_private_stream_policy: int = models.PositiveSmallIntegerField( + default=POLICY_MEMBERS_ONLY + ) # Who in the organization is allowed to delete messages they themselves sent. delete_own_message_policy: bool = models.PositiveSmallIntegerField(default=POLICY_ADMINS_ONLY) @@ -605,7 +608,8 @@ class Realm(models.Model): add_custom_emoji_policy=int, allow_edit_history=bool, bot_creation_policy=int, - create_stream_policy=int, + create_public_stream_policy=int, + create_private_stream_policy=int, invite_to_stream_policy=int, move_messages_between_streams_policy=int, default_language=str, @@ -1834,7 +1838,8 @@ class UserProfile(AbstractBaseUser, PermissionsMixin, UserBaseSettings): def has_permission(self, policy_name: str) -> bool: if policy_name not in [ "add_custom_emoji_policy", - "create_stream_policy", + "create_private_stream_policy", + "create_public_stream_policy", "delete_own_message_policy", "edit_topic_policy", "invite_to_stream_policy", @@ -1872,8 +1877,11 @@ class UserProfile(AbstractBaseUser, PermissionsMixin, UserBaseSettings): assert policy_value == Realm.POLICY_FULL_MEMBERS_ONLY return not self.is_provisional_member - def can_create_streams(self) -> bool: - return self.has_permission("create_stream_policy") + def can_create_public_streams(self) -> bool: + return self.has_permission("create_public_stream_policy") + + def can_create_private_streams(self) -> bool: + return self.has_permission("create_private_stream_policy") def can_subscribe_other_users(self) -> bool: return self.has_permission("invite_to_stream_policy") diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index bfaa554558..3ea8929cee 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -10359,12 +10359,34 @@ paths: Present if `realm` is present in `fetch_event_types`. The policy for which users can create bot users in this organization. - realm_create_stream_policy: + realm_create_public_stream_policy: type: integer description: | Present if `realm` is present in `fetch_event_types`. - The policy for which users can create streams in this organization. + The policy for which users can create public streams in this organization. + + - 1 = members only + - 2 = admins only + - 3 = full members only + - 4 = admins and moderators only + + **Changes**: Before Zulip 5.0 (feature level 102), permission to + create streams was controlled by the `realm_create_stream_policy` setting. + realm_create_private_stream_policy: + type: integer + description: | + Present if `realm` is present in `fetch_event_types`. + + The policy for which users can create private streams in this organization. + + - 1 = members only + - 2 = admins only + - 3 = full members only + - 4 = admins and moderators only + + **Changes**: Before Zulip 5.0 (feature level 102), permission to + create streams was controlled by the `realm_create_stream_policy` setting. realm_invite_to_stream_policy: type: integer description: | @@ -11366,11 +11388,43 @@ paths: resolution. See also `avatar_url_medium`. can_create_streams: type: boolean + deprecated: true description: | Present if `realm_user` is present in `fetch_event_types`. - Whether the current user is allowed to create streams with + Whether the current user is allowed to create at least one type + of stream with the organization's [stream creation + policy](/help/configure-who-can-create-streams). Its value will + always equal `can_create_public_streams || can_create_private_streams`. + + **Changes**: Deprecated in Zulip 5.0 (feature level 102), when + the new `create_private_stream_policy` and + `create_public_stream_policy` properties introduced the + possibility that a user could only create one type of stream. + + This field will be removed in a future release. + can_create_public_streams: + type: boolean + description: | + Present if `realm_user` is present in `fetch_event_types`. + + Whether the current user is allowed to create public streams with the organization's [stream creation policy](/help/configure-who-can-create-streams). + + **Changes**: New in Zulip 5.0 (feature level 102). In older + versions, the deprecated `can_create_streams` property should be + used to determine whether the user can create public streams. + can_create_private_streams: + type: boolean + description: | + Present if `realm_user` is present in `fetch_event_types`. + + Whether the current user is allowed to create private streams with + the organization's [stream creation policy](/help/configure-who-can-create-streams). + + **Changes**: New in Zulip 5.0 (feature level 102). In older + versions, the deprecated `can_create_streams` property should be + used to determine whether the user can create private streams. can_subscribe_other_users: type: boolean description: | diff --git a/zerver/tests/test_events.py b/zerver/tests/test_events.py index d77776c178..b4d80454f4 100644 --- a/zerver/tests/test_events.py +++ b/zerver/tests/test_events.py @@ -2127,7 +2127,8 @@ class RealmPropertyActionTest(BaseAction): message_retention_days=[10, 20], name=["Zulip", "New Name"], waiting_period_threshold=[10, 20], - create_stream_policy=[4, 3, 2, 1], + create_public_stream_policy=[4, 3, 2, 1], + create_private_stream_policy=[4, 3, 2, 1], invite_to_stream_policy=[4, 3, 2, 1], private_message_policy=[2, 1], user_group_edit_policy=[1, 2, 3, 4], diff --git a/zerver/tests/test_home.py b/zerver/tests/test_home.py index 5712e084b5..ea1b148bbc 100644 --- a/zerver/tests/test_home.py +++ b/zerver/tests/test_home.py @@ -50,6 +50,8 @@ class HomeTest(ZulipTestCase): "avatar_url", "avatar_url_medium", "bot_types", + "can_create_private_streams", + "can_create_public_streams", "can_create_streams", "can_invite_others_to_realm", "can_subscribe_other_users", @@ -112,7 +114,8 @@ class HomeTest(ZulipTestCase): "realm_bot_domain", "realm_bots", "realm_community_topic_editing_limit_seconds", - "realm_create_stream_policy", + "realm_create_private_stream_policy", + "realm_create_public_stream_policy", "realm_default_code_block_language", "realm_default_external_accounts", "realm_default_language", diff --git a/zerver/tests/test_realm.py b/zerver/tests/test_realm.py index 3d40998173..1d69f32d95 100644 --- a/zerver/tests/test_realm.py +++ b/zerver/tests/test_realm.py @@ -467,7 +467,8 @@ class RealmTest(ZulipTestCase): invalid_values = dict( bot_creation_policy=10, - create_stream_policy=10, + create_public_stream_policy=10, + create_private_stream_policy=10, invite_to_stream_policy=10, email_address_visibility=10, message_retention_days=10, @@ -817,7 +818,8 @@ class RealmAPITest(ZulipTestCase): message_retention_days=[10, 20], name=["Zulip", "New Name"], waiting_period_threshold=[10, 20], - create_stream_policy=Realm.COMMON_POLICY_TYPES, + create_private_stream_policy=Realm.COMMON_POLICY_TYPES, + create_public_stream_policy=Realm.COMMON_POLICY_TYPES, user_group_edit_policy=Realm.COMMON_POLICY_TYPES, private_message_policy=Realm.PRIVATE_MESSAGE_POLICY_TYPES, invite_to_stream_policy=Realm.COMMON_POLICY_TYPES, diff --git a/zerver/tests/test_subs.py b/zerver/tests/test_subs.py index 1c770c2ce2..a4be7b4825 100644 --- a/zerver/tests/test_subs.py +++ b/zerver/tests/test_subs.py @@ -3226,25 +3226,35 @@ class SubscriptionAPITest(ZulipTestCase): ) def _test_user_settings_for_adding_streams(self, stream_policy: str, invite_only: bool) -> None: + # TODO: This test makes excessive use of mocking and should be + # rewritten to be more similar to _test_user_settings_for_creating_streams. + method = "can_create_private_streams" if invite_only else "can_create_public_streams" do_set_realm_property( self.test_user.realm, stream_policy, Realm.POLICY_ADMINS_ONLY, acting_user=None ) - with mock.patch("zerver.models.UserProfile.can_create_streams", return_value=False): + with mock.patch(f"zerver.models.UserProfile.{method}", return_value=False): result = self.common_subscribe_to_streams( self.test_user, ["stream1"], invite_only=invite_only, allow_fail=True ) self.assert_json_error(result, "Insufficient permission") - with mock.patch("zerver.models.UserProfile.can_create_streams", return_value=True): + with mock.patch(f"zerver.models.UserProfile.{method}", return_value=True): self.common_subscribe_to_streams(self.test_user, ["stream2"], invite_only=invite_only) # User should still be able to subscribe to an existing stream - with mock.patch("zerver.models.UserProfile.can_create_streams", return_value=False): + with mock.patch(f"zerver.models.UserProfile.{method}", return_value=False): self.common_subscribe_to_streams(self.test_user, ["stream2"], invite_only=invite_only) - def test_user_settings_for_adding_streams(self) -> None: - self._test_user_settings_for_adding_streams("create_stream_policy", invite_only=False) + def test_user_settings_for_adding_private_streams(self) -> None: + self._test_user_settings_for_adding_streams( + "create_private_stream_policy", invite_only=True + ) + + def test_user_settings_for_adding_public_streams(self) -> None: + self._test_user_settings_for_adding_streams( + "create_public_stream_policy", invite_only=False + ) def _test_user_settings_for_creating_streams( self, stream_policy: str, invite_only: bool @@ -3274,11 +3284,12 @@ class SubscriptionAPITest(ZulipTestCase): user_profile, ["new_stream2"], allow_fail=True, + invite_only=invite_only, ) self.assert_json_error(result, "Insufficient permission") do_change_user_role(user_profile, UserProfile.ROLE_MODERATOR, acting_user=None) - self.common_subscribe_to_streams(user_profile, ["new_stream2"]) + self.common_subscribe_to_streams(user_profile, ["new_stream2"], invite_only=invite_only) do_set_realm_property(realm, stream_policy, Realm.POLICY_MEMBERS_ONLY, acting_user=None) do_change_user_role(user_profile, UserProfile.ROLE_GUEST, acting_user=None) @@ -3312,15 +3323,36 @@ class SubscriptionAPITest(ZulipTestCase): do_set_realm_property(realm, "waiting_period_threshold", 0, acting_user=None) self.common_subscribe_to_streams(user_profile, ["new_stream3"], invite_only=invite_only) - def test_user_settings_for_creating_streams(self) -> None: - self._test_user_settings_for_creating_streams("create_stream_policy", invite_only=False) + def test_user_settings_for_creating_private_streams(self) -> None: + self._test_user_settings_for_creating_streams( + "create_private_stream_policy", invite_only=True + ) - def test_can_create_streams(self) -> None: - def validation_func(user_profile: UserProfile) -> bool: - user_profile.refresh_from_db() - return user_profile.can_create_streams() + def test_user_settings_for_creating_public_streams(self) -> None: + self._test_user_settings_for_creating_streams( + "create_public_stream_policy", invite_only=False + ) - self.check_has_permission_policies("create_stream_policy", validation_func) + def _test_can_create_streams(self, stream_policy: str, invite_only: bool) -> None: + if invite_only: + + def validation_func(user_profile: UserProfile) -> bool: + user_profile.refresh_from_db() + return user_profile.can_create_private_streams() + + else: + + def validation_func(user_profile: UserProfile) -> bool: + user_profile.refresh_from_db() + return user_profile.can_create_public_streams() + + self.check_has_permission_policies(stream_policy, validation_func) + + def test_can_create_private_streams(self) -> None: + self._test_can_create_streams("create_private_stream_policy", invite_only=True) + + def test_can_create_public_streams(self) -> None: + self._test_can_create_streams("create_public_stream_policy", invite_only=False) def test_user_settings_for_subscribing_other_users(self) -> None: """ @@ -3332,7 +3364,7 @@ class SubscriptionAPITest(ZulipTestCase): realm = user_profile.realm do_set_realm_property( - realm, "create_stream_policy", Realm.POLICY_MEMBERS_ONLY, acting_user=None + realm, "create_public_stream_policy", Realm.POLICY_MEMBERS_ONLY, acting_user=None ) do_set_realm_property( realm, "invite_to_stream_policy", Realm.POLICY_ADMINS_ONLY, acting_user=None diff --git a/zerver/views/realm.py b/zerver/views/realm.py index 5da4d8e7d8..5a704f37c3 100644 --- a/zerver/views/realm.py +++ b/zerver/views/realm.py @@ -100,7 +100,10 @@ def update_realm( bot_creation_policy: Optional[int] = REQ( json_validator=check_int_in(Realm.BOT_CREATION_POLICY_TYPES), default=None ), - create_stream_policy: Optional[int] = REQ( + create_public_stream_policy: Optional[int] = REQ( + json_validator=check_int_in(Realm.COMMON_POLICY_TYPES), default=None + ), + create_private_stream_policy: Optional[int] = REQ( json_validator=check_int_in(Realm.COMMON_POLICY_TYPES), default=None ), invite_to_stream_policy: Optional[int] = REQ(