From 54661115477577ba135329ee5c6ff22bcc9b0a7a Mon Sep 17 00:00:00 2001 From: Aditya Kumar Kasaudhan <74228301+Aditya8840@users.noreply.github.com> Date: Sat, 5 Apr 2025 11:24:42 +0530 Subject: [PATCH 1/4] streams: Separate bulk_check_basic_stream_access function. This commit extracts the common logic for bulk-checking basic stream access into a separate function, reducing redundancy when introducing can_unsubscribe_group. --- zerver/lib/streams.py | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/zerver/lib/streams.py b/zerver/lib/streams.py index ccf561da81ac9..8207c5b5b9b6a 100644 --- a/zerver/lib/streams.py +++ b/zerver/lib/streams.py @@ -1254,6 +1254,23 @@ def can_resolve_topics(user: UserProfile, orig_stream: Stream, target_stream: St return False +def bulk_check_basic_stream_access(user_profile: UserProfile, streams: list[Stream]) -> bool: + existing_recipient_ids = [stream.recipient_id for stream in streams] + sub_recipient_ids = Subscription.objects.filter( + user_profile=user_profile, recipient_id__in=existing_recipient_ids, active=True + ).values_list("recipient_id", flat=True) + + for stream in streams: + assert stream.recipient_id is not None + is_subscribed = stream.recipient_id in sub_recipient_ids + if not check_basic_stream_access( + user_profile, stream, is_subscribed=is_subscribed, require_content_access=False + ): + return False + + return True + + def bulk_can_remove_subscribers_from_streams( streams: list[Stream], user_profile: UserProfile ) -> bool: @@ -1285,18 +1302,8 @@ def bulk_can_remove_subscribers_from_streams( if not bool(permission_failure_streams): return True - existing_recipient_ids = [stream.recipient_id for stream in streams] - sub_recipient_ids = Subscription.objects.filter( - user_profile=user_profile, recipient_id__in=existing_recipient_ids, active=True - ).values_list("recipient_id", flat=True) - - for stream in streams: - assert stream.recipient_id is not None - is_subscribed = stream.recipient_id in sub_recipient_ids - if not check_basic_stream_access( - user_profile, stream, is_subscribed=is_subscribed, require_content_access=False - ): - return False + if not bulk_check_basic_stream_access(user_profile, streams): + return False for stream in streams: if not is_user_in_can_remove_subscribers_group(stream, user_recursive_group_ids): From 433405c8bda811f2665176d7cb8c1db99d7d474a Mon Sep 17 00:00:00 2001 From: Aditya Kumar Kasaudhan <74228301+Aditya8840@users.noreply.github.com> Date: Thu, 29 May 2025 01:04:22 +0530 Subject: [PATCH 2/4] streams: Rename popover for subscription toggle restrictions. This commit makes the function reusable, helping reduce code duplication. --- web/src/stream_edit.ts | 2 +- web/src/stream_ui_updates.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/web/src/stream_edit.ts b/web/src/stream_edit.ts index 630a1d09359b4..8362a9eccd8f8 100644 --- a/web/src/stream_edit.ts +++ b/web/src/stream_edit.ts @@ -172,7 +172,7 @@ function show_subscription_settings(sub: SettingsSubscription): void { stream_ui_updates.update_add_subscriptions_elements(sub); if (!stream_data.can_toggle_subscription(sub)) { - stream_ui_updates.initialize_cant_subscribe_popover(); + stream_ui_updates.initialize_subscription_toggle_disabled_popover(); } const $subscriber_container = $edit_container.find(".edit_subscribers_for_stream"); diff --git a/web/src/stream_ui_updates.ts b/web/src/stream_ui_updates.ts index defcc5370ee6a..f7822dfec3761 100644 --- a/web/src/stream_ui_updates.ts +++ b/web/src/stream_ui_updates.ts @@ -139,7 +139,7 @@ export function update_private_stream_privacy_option_state( .toggleClass("default_stream_private_tooltip", is_default_stream); } -export function initialize_cant_subscribe_popover(): void { +export function initialize_subscription_toggle_disabled_popover(): void { const $button_wrapper = $(".settings .stream_settings_header .sub_unsub_button_wrapper"); settings_components.initialize_disable_button_hint_popover($button_wrapper, undefined); } @@ -220,7 +220,7 @@ export function update_settings_button_for_sub(sub: StreamSubscription): void { $settings_button.addClass("toggle-subscription-tooltip"); } else { $settings_button.attr("title", ""); - initialize_cant_subscribe_popover(); + initialize_subscription_toggle_disabled_popover(); $settings_button.prop("disabled", true); $settings_button.removeClass("toggle-subscription-tooltip"); } From bc0efba8406b5d430f4ace83780748d230999c06 Mon Sep 17 00:00:00 2001 From: Aditya Kumar Kasaudhan <74228301+Aditya8840@users.noreply.github.com> Date: Fri, 21 Mar 2025 04:41:02 +0530 Subject: [PATCH 3/4] streams: Add new can_unsubscribe_group permission setting. Fixes: #33413. --- api_docs/unmerged.d/ZF-1abcca.md | 9 ++ web/src/group_permission_settings.ts | 1 + web/src/settings_config.ts | 4 + web/src/stream_data.ts | 20 ++- web/src/stream_events.ts | 8 +- web/src/stream_popover.ts | 5 +- web/src/stream_types.ts | 2 + web/src/stream_ui_updates.ts | 6 + web/src/user_profile.ts | 3 +- web/styles/subscriptions.css | 8 ++ .../left_sidebar_stream_actions_popover.hbs | 2 + .../browse_streams_list_item.hbs | 22 ++- .../stream_settings/stream_permissions.hbs | 5 + .../stream_settings/stream_settings.hbs | 9 +- web/tests/lib/example_settings.cjs | 8 ++ web/tests/stream_data.test.cjs | 45 +++++- web/tests/stream_events.test.cjs | 35 +++++ web/tests/stream_settings_ui.test.cjs | 8 ++ zerver/actions/streams.py | 1 + zerver/lib/streams.py | 48 +++++++ zerver/lib/subscription_info.py | 14 ++ zerver/lib/types.py | 4 + .../0755_stream_can_unsubscribe_group.py | 23 +++ ...efault_for_stream_can_unsubscribe_group.py | 54 +++++++ ...0757_alter_stream_can_unsubscribe_group.py | 22 +++ zerver/models/streams.py | 9 ++ zerver/openapi/zulip.yaml | 92 ++++++++++-- zerver/tests/test_channel_creation.py | 3 + zerver/tests/test_channel_permissions.py | 134 ++++++++++++++++++ zerver/views/streams.py | 8 ++ 30 files changed, 588 insertions(+), 24 deletions(-) create mode 100644 api_docs/unmerged.d/ZF-1abcca.md create mode 100644 zerver/migrations/0755_stream_can_unsubscribe_group.py create mode 100644 zerver/migrations/0756_set_default_for_stream_can_unsubscribe_group.py create mode 100644 zerver/migrations/0757_alter_stream_can_unsubscribe_group.py diff --git a/api_docs/unmerged.d/ZF-1abcca.md b/api_docs/unmerged.d/ZF-1abcca.md new file mode 100644 index 0000000000000..57897c514155b --- /dev/null +++ b/api_docs/unmerged.d/ZF-1abcca.md @@ -0,0 +1,9 @@ +* [`GET /users/me/subscriptions`](/api/get-subscriptions), + [`GET /streams`](/api/get-streams), [`GET /events`](/api/get-events), + [`POST /register`](/api/register-queue): Added `can_unsubscribe_group` + field to Stream and Subscription objects. +* [`POST /users/me/subscriptions`](/api/subscribe), + [`PATCH /streams/{stream_id}`](/api/update-stream): Added `can_unsubscribe_group` + parameter for setting the user group whose members can unsubscribe themselves from channel. +* [`DELETE /users/me/subscriptions`](/api/unsubscribe): Unsubscription is allowed for organization administrators, + users who can administer the channel or remove other subscribers, and members of the channel's `can_unsubscribe_group`. diff --git a/web/src/group_permission_settings.ts b/web/src/group_permission_settings.ts index f89e543ce491c..0beaf1541c964 100644 --- a/web/src/group_permission_settings.ts +++ b/web/src/group_permission_settings.ts @@ -90,6 +90,7 @@ export const stream_group_setting_name_schema = z.enum([ "can_remove_subscribers_group", "can_send_message_group", "can_subscribe_group", + "can_unsubscribe_group", "can_resolve_topics_group", ]); export type StreamGroupSettingName = z.infer; diff --git a/web/src/settings_config.ts b/web/src/settings_config.ts index 0ef5443732e02..8f3ba65dce52d 100644 --- a/web/src/settings_config.ts +++ b/web/src/settings_config.ts @@ -786,6 +786,9 @@ export const all_group_setting_labels = { can_send_message_group: $t({defaultMessage: "Who can post to this channel"}), can_administer_channel_group: $t({defaultMessage: "Who can administer this channel"}), can_subscribe_group: $t({defaultMessage: "Who can subscribe to this channel"}), + can_unsubscribe_group: $t({ + defaultMessage: "Who can unsubscribe from this channel", + }), can_remove_subscribers_group: $t({ defaultMessage: "Who can unsubscribe anyone from this channel", }), @@ -888,6 +891,7 @@ export const stream_group_permission_settings: StreamGroupSettingName[] = [ "can_move_messages_out_of_channel_group", "can_move_messages_within_channel_group", "can_subscribe_group", + "can_unsubscribe_group", "can_add_subscribers_group", "can_remove_subscribers_group", "can_resolve_topics_group", diff --git a/web/src/stream_data.ts b/web/src/stream_data.ts index 6562a483529f2..41b801f6a6475 100644 --- a/web/src/stream_data.ts +++ b/web/src/stream_data.ts @@ -690,14 +690,30 @@ export function user_can_set_topics_policy(sub?: StreamSubscription): boolean { return user_can_set_topics_policy && can_administer_channel(sub); } +export function can_unsubscribe(sub: StreamSubscription): boolean { + // Handles if the user is an organization admin, or in one of these groups: + // can_administer_channel_group or can_remove_subscribers_group. + if (can_unsubscribe_others(sub)) { + return true; + } + + return settings_data.user_has_permission_for_group_setting( + sub.can_unsubscribe_group, + "can_unsubscribe_group", + "stream", + ); +} + export function can_toggle_subscription(sub: StreamSubscription): boolean { if (page_params.is_spectator) { return false; } - // Currently, you can always remove your subscription if you're subscribed. + // If the user is subscribed, they can unsubscribe themselves only if they are + // an organization admin, or in one of these groups: can_administer_channel_group, + // can_remove_subscribers_group, or can_unsubscribe_group. if (sub.subscribed) { - return true; + return can_unsubscribe(sub); } if (has_content_access(sub)) { diff --git a/web/src/stream_events.ts b/web/src/stream_events.ts index 5868ecd31f41c..80fcc0a524328 100644 --- a/web/src/stream_events.ts +++ b/web/src/stream_events.ts @@ -108,7 +108,13 @@ export function update_property

( sub, group_setting_value_schema.parse(value), ); - if (property === "can_subscribe_group" || property === "can_add_subscribers_group") { + if ( + property === "can_subscribe_group" || + property === "can_add_subscribers_group" || + property === "can_unsubscribe_group" || + property === "can_remove_subscribers_group" || + property === "can_administer_channel_group" + ) { stream_settings_ui.update_subscription_elements(sub); } if (property === "can_administer_channel_group") { diff --git a/web/src/stream_popover.ts b/web/src/stream_popover.ts index ab344e294360c..62e1668b50bb6 100644 --- a/web/src/stream_popover.ts +++ b/web/src/stream_popover.ts @@ -121,12 +121,15 @@ function build_stream_popover(opts: {elt: HTMLElement; stream_id: number}): void const stream_unread = unread.unread_count_info_for_stream(stream_id); const stream_unread_count = stream_unread.unmuted_count + stream_unread.muted_count; const has_unread_messages = stream_unread_count > 0; + const stream_subscription = sub_store.get(stream_id); + assert(stream_subscription !== undefined); const content = render_left_sidebar_stream_actions_popover({ stream: { - ...sub_store.get(stream_id), + ...stream_subscription, url: browser_history.get_full_url(stream_hash), list_of_topics_view_url: hash_util.by_channel_topic_list_url(stream_id), }, + should_display_unsubscribe_button: stream_data.can_unsubscribe(stream_subscription), has_unread_messages, show_go_to_channel_feed, show_go_to_list_of_topics, diff --git a/web/src/stream_types.ts b/web/src/stream_types.ts index ce62c4f45c1ff..3418ed5d7fd59 100644 --- a/web/src/stream_types.ts +++ b/web/src/stream_types.ts @@ -21,6 +21,7 @@ export const stream_permission_group_settings_schema = z.enum([ "can_resolve_topics_group", "can_send_message_group", "can_subscribe_group", + "can_unsubscribe_group", ]); export type StreamPermissionGroupSetting = z.infer; @@ -44,6 +45,7 @@ export const stream_schema = z.object({ can_resolve_topics_group: group_setting_value_schema, can_send_message_group: group_setting_value_schema, can_subscribe_group: group_setting_value_schema, + can_unsubscribe_group: group_setting_value_schema, creator_id: z.nullable(z.number()), date_created: z.number(), description: z.string(), diff --git a/web/src/stream_ui_updates.ts b/web/src/stream_ui_updates.ts index f7822dfec3761..8bef79ae511cc 100644 --- a/web/src/stream_ui_updates.ts +++ b/web/src/stream_ui_updates.ts @@ -210,6 +210,7 @@ export function update_settings_button_for_sub(sub: StreamSubscription): void { .addClass("unsubscribed action-button-quiet-brand") .removeClass("action-button-quiet-neutral"); } + const $parent = $settings_button.parent(); if (stream_data.can_toggle_subscription(sub)) { $settings_button.prop("disabled", false); const $parent_element: tippy.ReferenceElement & HTMLElement = util.the( @@ -220,6 +221,11 @@ export function update_settings_button_for_sub(sub: StreamSubscription): void { $settings_button.addClass("toggle-subscription-tooltip"); } else { $settings_button.attr("title", ""); + if (sub.subscribed) { + $parent.attr("data-tooltip-template-id", "cannot-unsubscribe-tooltip-template"); + } else { + $parent.attr("data-tooltip-template-id", "cannot-subscribe-tooltip-template"); + } initialize_subscription_toggle_disabled_popover(); $settings_button.prop("disabled", true); $settings_button.removeClass("toggle-subscription-tooltip"); diff --git a/web/src/user_profile.ts b/web/src/user_profile.ts index e73e962229d5d..dffaedb1653d8 100644 --- a/web/src/user_profile.ts +++ b/web/src/user_profile.ts @@ -283,7 +283,8 @@ export function get_user_unsub_streams(user_id: number): dropdown_widget.Option[ function format_user_stream_list_item_html(stream: StreamSubscription, user: User): string { const show_unsubscribe_button = - people.can_admin_user(user) || stream_data.can_unsubscribe_others(stream); + stream_data.can_unsubscribe_others(stream) || + (people.is_my_user_id(user.user_id) && stream_data.can_unsubscribe(stream)); const show_private_stream_unsub_tooltip = people.is_my_user_id(user.user_id) && stream.invite_only; const show_last_user_in_private_stream_unsub_tooltip = diff --git a/web/styles/subscriptions.css b/web/styles/subscriptions.css index 5dc3b14699349..30c9aa7e18965 100644 --- a/web/styles/subscriptions.css +++ b/web/styles/subscriptions.css @@ -780,6 +780,14 @@ h4.user_group_setting_subsection_title { color: var(--color-stream-group-row-plus-icon-disabled); } + &.checked svg { + opacity: 0.5; + } + + &.checked svg { + opacity: 0.5; + } + .sub-unsub-icon { color: var(--color-stream-group-row-checked-icon-disabled); cursor: not-allowed; diff --git a/web/templates/popovers/left_sidebar/left_sidebar_stream_actions_popover.hbs b/web/templates/popovers/left_sidebar/left_sidebar_stream_actions_popover.hbs index 6c421ccb3196b..26cf391483aaf 100644 --- a/web/templates/popovers/left_sidebar/left_sidebar_stream_actions_popover.hbs +++ b/web/templates/popovers/left_sidebar/left_sidebar_stream_actions_popover.hbs @@ -77,12 +77,14 @@ {{#unless stream.is_archived}} + {{#if should_display_unsubscribe_button}}

  • {{t "Unsubscribe"}}
    • + {{/if}}
  • diff --git a/web/templates/stream_settings/browse_streams_list_item.hbs b/web/templates/stream_settings/browse_streams_list_item.hbs index b9acac3cbc936..71667e8dec1f6 100644 --- a/web/templates/stream_settings/browse_streams_list_item.hbs +++ b/web/templates/stream_settings/browse_streams_list_item.hbs @@ -2,10 +2,10 @@
    {{#if subscribed}} -
    +
    -
    -