stream_settings: Use can_remove_subscribers_group setting in webapp.

This commit updates the frontend to show or hide the "Unsubscribe"
button in subscribers list in stream settings as per the
can_remove_subscribers_group setting for the stream.
This commit is contained in:
Sahil Batra
2022-12-29 22:19:38 +05:30
committed by Tim Abbott
parent 73f11853ec
commit 73a378d23f
7 changed files with 151 additions and 6 deletions

View File

@@ -21,6 +21,7 @@ const sub_store = zrequire("sub_store");
const stream_data = zrequire("stream_data"); const stream_data = zrequire("stream_data");
const stream_settings_data = zrequire("stream_settings_data"); const stream_settings_data = zrequire("stream_settings_data");
const settings_config = zrequire("settings_config"); const settings_config = zrequire("settings_config");
const user_groups = zrequire("user_groups");
const me = { const me = {
email: "me@zulip.com", email: "me@zulip.com",
@@ -39,6 +40,14 @@ function contains_sub(subs, sub) {
return subs.some((s) => s.name === sub.name); return subs.some((s) => s.name === sub.name);
} }
const admins_group = {
name: "Admins",
id: 1,
members: new Set([1]),
is_system_group: true,
direct_subgroup_ids: new Set([]),
};
function test(label, f) { function test(label, f) {
run_test(label, (helpers) => { run_test(label, (helpers) => {
page_params.is_admin = false; page_params.is_admin = false;
@@ -48,6 +57,7 @@ function test(label, f) {
people.add_active_user(me); people.add_active_user(me);
people.initialize_current_user(me.user_id); people.initialize_current_user(me.user_id);
stream_data.clear_subscriptions(); stream_data.clear_subscriptions();
user_groups.initialize({realm_user_groups: [admins_group]});
f(helpers); f(helpers);
}); });
} }
@@ -333,6 +343,7 @@ test("admin_options", () => {
stream_id: 1, stream_id: 1,
is_muted: true, is_muted: true,
invite_only: false, invite_only: false,
can_remove_subscribers_group_id: admins_group.id,
}; };
stream_data.add_sub(sub); stream_data.add_sub(sub);
return sub; return sub;
@@ -385,6 +396,7 @@ test("stream_settings", () => {
color: "cinnamon", color: "cinnamon",
subscribed: true, subscribed: true,
invite_only: false, invite_only: false,
can_remove_subscribers_group_id: admins_group.id,
}; };
const blue = { const blue = {
@@ -393,6 +405,7 @@ test("stream_settings", () => {
color: "blue", color: "blue",
subscribed: false, subscribed: false,
invite_only: false, invite_only: false,
can_remove_subscribers_group_id: admins_group.id,
}; };
const amber = { const amber = {
@@ -404,6 +417,7 @@ test("stream_settings", () => {
history_public_to_subscribers: true, history_public_to_subscribers: true,
stream_post_policy: stream_data.stream_post_policy_values.admins.code, stream_post_policy: stream_data.stream_post_policy_values.admins.code,
message_retention_days: 10, message_retention_days: 10,
can_remove_subscribers_group_id: admins_group.id,
}; };
stream_data.add_sub(cinnamon); stream_data.add_sub(cinnamon);
stream_data.add_sub(amber); stream_data.add_sub(amber);
@@ -996,3 +1010,85 @@ test("can_post_messages_in_stream", () => {
page_params.is_spectator = true; page_params.is_spectator = true;
assert.equal(stream_data.can_post_messages_in_stream(social), false); assert.equal(stream_data.can_post_messages_in_stream(social), false);
}); });
test("can_unsubscribe_others", () => {
const admin_user_id = 1;
const moderator_user_id = 2;
const member_user_id = 3;
const admins = {
name: "Admins",
id: 1,
members: new Set([admin_user_id]),
is_system_group: true,
direct_subgroup_ids: new Set([]),
};
const moderators = {
name: "Moderators",
id: 2,
members: new Set([moderator_user_id]),
is_system_group: true,
direct_subgroup_ids: new Set([1]),
};
const all = {
name: "Everyone",
id: 3,
members: new Set([member_user_id]),
is_system_group: true,
direct_subgroup_ids: new Set([2]),
};
const nobody = {
name: "Nobody",
id: 4,
members: new Set([]),
is_system_group: true,
direct_subgroup_ids: new Set([]),
};
user_groups.initialize({realm_user_groups: [admins, moderators, all, nobody]});
const sub = {
name: "Denmark",
subscribed: true,
color: "red",
stream_id: 1,
can_remove_subscribers_group_id: admins.id,
};
stream_data.add_sub(sub);
people.initialize_current_user(admin_user_id);
assert.equal(stream_data.can_unsubscribe_others(sub), true);
people.initialize_current_user(moderator_user_id);
assert.equal(stream_data.can_unsubscribe_others(sub), false);
sub.can_remove_subscribers_group_id = moderators.id;
people.initialize_current_user(admin_user_id);
assert.equal(stream_data.can_unsubscribe_others(sub), true);
people.initialize_current_user(moderator_user_id);
assert.equal(stream_data.can_unsubscribe_others(sub), true);
people.initialize_current_user(member_user_id);
assert.equal(stream_data.can_unsubscribe_others(sub), false);
sub.can_remove_subscribers_group_id = all.id;
people.initialize_current_user(admin_user_id);
assert.equal(stream_data.can_unsubscribe_others(sub), true);
people.initialize_current_user(moderator_user_id);
assert.equal(stream_data.can_unsubscribe_others(sub), true);
people.initialize_current_user(member_user_id);
assert.equal(stream_data.can_unsubscribe_others(sub), true);
// Even with the nobody system group, admins can still unsubscribe others.
sub.can_remove_subscribers_group_id = nobody.id;
page_params.is_admin = true;
assert.equal(stream_data.can_unsubscribe_others(sub), true);
page_params.is_admin = false;
assert.equal(stream_data.can_unsubscribe_others(sub), false);
// This isn't a real state, but we want coverage on !can_view_subscribers.
sub.subscribed = false;
sub.invite_only = true;
page_params.is_admin = true;
assert.equal(stream_data.can_unsubscribe_others(sub), true);
page_params.is_admin = false;
assert.equal(stream_data.can_unsubscribe_others(sub), false);
});

View File

@@ -21,8 +21,18 @@ set_global("page_params", {});
const stream_data = zrequire("stream_data"); const stream_data = zrequire("stream_data");
const stream_settings_ui = zrequire("stream_settings_ui"); const stream_settings_ui = zrequire("stream_settings_ui");
const user_groups = zrequire("user_groups");
run_test("redraw_left_panel", ({mock_template}) => { run_test("redraw_left_panel", ({mock_template}) => {
const admins_group = {
name: "Admins",
id: 1,
members: new Set([1]),
is_system_group: true,
direct_subgroup_ids: new Set([]),
};
user_groups.initialize({realm_user_groups: [admins_group]});
// set-up sub rows stubs // set-up sub rows stubs
const denmark = { const denmark = {
elem: "denmark", elem: "denmark",
@@ -33,6 +43,7 @@ run_test("redraw_left_panel", ({mock_template}) => {
subscribers: [1], subscribers: [1],
stream_weekly_traffic: null, stream_weekly_traffic: null,
color: "red", color: "red",
can_remove_subscribers_group_id: admins_group.id,
}; };
const poland = { const poland = {
elem: "poland", elem: "poland",
@@ -43,6 +54,7 @@ run_test("redraw_left_panel", ({mock_template}) => {
subscribers: [1, 2, 3], subscribers: [1, 2, 3],
stream_weekly_traffic: 13, stream_weekly_traffic: 13,
color: "red", color: "red",
can_remove_subscribers_group_id: admins_group.id,
}; };
const pomona = { const pomona = {
elem: "pomona", elem: "pomona",
@@ -53,6 +65,7 @@ run_test("redraw_left_panel", ({mock_template}) => {
subscribers: [], subscribers: [],
stream_weekly_traffic: 0, stream_weekly_traffic: 0,
color: "red", color: "red",
can_remove_subscribers_group_id: admins_group.id,
}; };
const cpp = { const cpp = {
elem: "cpp", elem: "cpp",
@@ -63,6 +76,7 @@ run_test("redraw_left_panel", ({mock_template}) => {
subscribers: [1, 2], subscribers: [1, 2],
stream_weekly_traffic: 6, stream_weekly_traffic: 6,
color: "red", color: "red",
can_remove_subscribers_group_id: admins_group.id,
}; };
const zzyzx = { const zzyzx = {
elem: "zzyzx", elem: "zzyzx",
@@ -73,6 +87,7 @@ run_test("redraw_left_panel", ({mock_template}) => {
subscribers: [1, 2], subscribers: [1, 2],
stream_weekly_traffic: 6, stream_weekly_traffic: 6,
color: "red", color: "red",
can_remove_subscribers_group_id: admins_group.id,
}; };
const sub_row_data = [denmark, poland, pomona, cpp, zzyzx]; const sub_row_data = [denmark, poland, pomona, cpp, zzyzx];

View File

@@ -8,6 +8,7 @@ import * as people from "./people";
import * as settings_config from "./settings_config"; import * as settings_config from "./settings_config";
import * as stream_topic_history from "./stream_topic_history"; import * as stream_topic_history from "./stream_topic_history";
import * as sub_store from "./sub_store"; import * as sub_store from "./sub_store";
import * as user_groups from "./user_groups";
import {user_settings} from "./user_settings"; import {user_settings} from "./user_settings";
import * as util from "./util"; import * as util from "./util";
@@ -556,6 +557,36 @@ export function can_subscribe_others(sub) {
return !page_params.is_guest && (!sub.invite_only || sub.subscribed); return !page_params.is_guest && (!sub.invite_only || sub.subscribed);
} }
export function can_unsubscribe_others(sub) {
// Whether the current user has permission to remove other users
// from the stream. Organization administrators can remove users
// from any stream; additionally, users who can access the stream
// and are in the stream's can_remove_subscribers_group can do so
// as well.
//
// TODO: The API allows the current user to remove bots that it
// administers from streams; so we might need to refactor this
// logic to accept a target_user_id parameter in order to support
// that in the UI.
// A user must be able to view subscribers in a stream in order to
// remove them. This check may never fire in practice, since the
// UI for removing subscribers generally is a list of the stream's
// subscribers.
if (!can_view_subscribers(sub)) {
return false;
}
if (page_params.is_admin) {
return true;
}
return user_groups.is_user_in_group(
sub.can_remove_subscribers_group_id,
people.my_current_user_id(),
);
}
export function can_post_messages_in_stream(stream) { export function can_post_messages_in_stream(stream) {
if (page_params.is_spectator) { if (page_params.is_spectator) {
return false; return false;

View File

@@ -23,13 +23,13 @@ export let pill_widget;
let current_stream_id; let current_stream_id;
let subscribers_list_widget; let subscribers_list_widget;
function format_member_list_elem(person) { function format_member_list_elem(person, user_can_remove_subscribers) {
return render_stream_member_list_entry({ return render_stream_member_list_entry({
name: person.full_name, name: person.full_name,
user_id: person.user_id, user_id: person.user_id,
is_current_user: person.user_id === page_params.user_id, is_current_user: person.user_id === page_params.user_id,
email: settings_data.email_for_user_settings(person), email: settings_data.email_for_user_settings(person),
can_edit_subscribers: page_params.is_admin, can_remove_subscribers: user_can_remove_subscribers,
show_email: settings_data.show_email(), show_email: settings_data.show_email(),
}); });
} }
@@ -87,6 +87,7 @@ export function enable_subscriber_management({sub, $parent_container}) {
}); });
const user_ids = peer_data.get_subscribers(stream_id); const user_ids = peer_data.get_subscribers(stream_id);
const user_can_remove_subscribers = stream_data.can_unsubscribe_others(sub);
// We track a single subscribers_list_widget for this module, since we // We track a single subscribers_list_widget for this module, since we
// only ever have one list of subscribers visible at a time. // only ever have one list of subscribers visible at a time.
@@ -94,10 +95,11 @@ export function enable_subscriber_management({sub, $parent_container}) {
$parent_container, $parent_container,
name: "stream_subscribers", name: "stream_subscribers",
user_ids, user_ids,
user_can_remove_subscribers,
}); });
} }
function make_list_widget({$parent_container, name, user_ids}) { function make_list_widget({$parent_container, name, user_ids, user_can_remove_subscribers}) {
const users = people.get_users_from_ids(user_ids); const users = people.get_users_from_ids(user_ids);
people.sort_but_pin_current_user_on_top(users); people.sort_but_pin_current_user_on_top(users);
@@ -109,7 +111,7 @@ function make_list_widget({$parent_container, name, user_ids}) {
return ListWidget.create($list_container, users, { return ListWidget.create($list_container, users, {
name, name,
modifier(item) { modifier(item) {
return format_member_list_elem(item); return format_member_list_elem(item, user_can_remove_subscribers);
}, },
filter: { filter: {
$element: $parent_container.find(".search"), $element: $parent_container.find(".search"),

View File

@@ -46,6 +46,7 @@ export function add_settings_fields(sub) {
sub.can_change_stream_permissions = stream_data.can_change_permissions(sub); sub.can_change_stream_permissions = stream_data.can_change_permissions(sub);
sub.can_access_subscribers = stream_data.can_view_subscribers(sub); sub.can_access_subscribers = stream_data.can_view_subscribers(sub);
sub.can_add_subscribers = stream_data.can_subscribe_others(sub); sub.can_add_subscribers = stream_data.can_subscribe_others(sub);
sub.can_remove_subscribers = stream_data.can_unsubscribe_others(sub);
sub.preview_url = hash_util.by_stream_url(sub.stream_id); sub.preview_url = hash_util.by_stream_url(sub.stream_id);
sub.is_old_stream = sub.stream_weekly_traffic !== null; sub.is_old_stream = sub.stream_weekly_traffic !== null;

View File

@@ -9,7 +9,7 @@
<td class="hidden-subscriber-email">{{t "(hidden)"}}</td> <td class="hidden-subscriber-email">{{t "(hidden)"}}</td>
{{/if}} {{/if}}
<td class="subscriber-user-id">{{user_id}}</td> <td class="subscriber-user-id">{{user_id}}</td>
{{#if can_edit_subscribers}} {{#if can_remove_subscribers}}
<td class="unsubscribe"> <td class="unsubscribe">
<div class="subscriber_list_remove"> <div class="subscriber_list_remove">
<form class="remove-subscriber-form"> <form class="remove-subscriber-form">

View File

@@ -24,7 +24,7 @@
<th>{{t "Name" }}</th> <th>{{t "Name" }}</th>
<th>{{t "Email" }}</th> <th>{{t "Email" }}</th>
<th>{{t "User ID" }}</th> <th>{{t "User ID" }}</th>
{{#if is_realm_admin}} {{#if can_remove_subscribers}}
<th class="actions">{{t "Actions" }}</th> <th class="actions">{{t "Actions" }}</th>
{{/if}} {{/if}}
</thead> </thead>