From 69c0a772cc96148eb659e82f59ed13fd09889435 Mon Sep 17 00:00:00 2001 From: Sahil Batra Date: Fri, 25 Oct 2024 18:57:18 +0530 Subject: [PATCH] user_groups: Do not allow manually entering pills of invalid subgroups. Even though we do not show groups that cannot be used as subgroups in typeahead, user can still type the complete to get the pill and make the request to the server which currently returns not so good error message. This commit fixes it to not create the pill for such cases and hence not making any request to server. --- web/src/add_group_members_pill.ts | 79 ++++++++++++++++++++++++++++++ web/src/add_subscribers_pill.ts | 24 +-------- web/src/user_group_edit_members.ts | 5 +- 3 files changed, 83 insertions(+), 25 deletions(-) diff --git a/web/src/add_group_members_pill.ts b/web/src/add_group_members_pill.ts index b6aa065a2e..bb85c00e20 100644 --- a/web/src/add_group_members_pill.ts +++ b/web/src/add_group_members_pill.ts @@ -1,11 +1,16 @@ +import assert from "minimalistic-assert"; + import * as add_subscribers_pill from "./add_subscribers_pill"; import * as input_pill from "./input_pill"; import * as keydown_util from "./keydown_util"; import type {User} from "./people"; import * as stream_pill from "./stream_pill"; import type {CombinedPill, CombinedPillContainer} from "./typeahead_helper"; +import * as user_group_components from "./user_group_components"; import * as user_group_create_members_data from "./user_group_create_members_data"; import * as user_group_pill from "./user_group_pill"; +import * as user_groups from "./user_groups"; +import type {UserGroup} from "./user_groups"; import * as user_pill from "./user_pill"; function get_pill_user_ids(pill_widget: CombinedPillContainer): number[] { @@ -19,6 +24,80 @@ function get_pill_group_ids(pill_widget: CombinedPillContainer): number[] { return group_user_ids; } +export function create_item_from_text( + text: string, + current_items: CombinedPill[], +): CombinedPill | undefined { + const funcs = [ + stream_pill.create_item_from_stream_name, + user_group_pill.create_item_from_group_name, + user_pill.create_item_from_email, + ]; + + const stream_item = stream_pill.create_item_from_stream_name(text, current_items); + if (stream_item) { + return stream_item; + } + + const group_item = user_group_pill.create_item_from_group_name(text, current_items); + if (group_item) { + const subgroup = user_groups.get_user_group_from_id(group_item.group_id); + const current_group_id = user_group_components.active_group_id; + assert(current_group_id !== undefined); + const current_group = user_groups.get_user_group_from_id(current_group_id); + if (user_groups.check_group_can_be_subgroup(subgroup, current_group)) { + return group_item; + } + + return undefined; + } + for (const func of funcs) { + const item = func(text, current_items); + if (item) { + return item; + } + } + return undefined; +} + +export function create({ + $pill_container, + get_potential_members, + get_potential_groups, +}: { + $pill_container: JQuery; + get_potential_members: () => User[]; + get_potential_groups: () => UserGroup[]; +}): CombinedPillContainer { + const pill_widget = input_pill.create({ + $container: $pill_container, + create_item_from_text, + get_text_from_item: add_subscribers_pill.get_text_from_item, + get_display_value_from_item: add_subscribers_pill.get_display_value_from_item, + generate_pill_html: add_subscribers_pill.generate_pill_html, + }); + function get_users(): User[] { + const potential_members = get_potential_members(); + return user_pill.filter_taken_users(potential_members, pill_widget); + } + + function get_user_groups(): UserGroup[] { + const potential_groups = get_potential_groups(); + return user_group_pill.filter_taken_groups(potential_groups, pill_widget); + } + + add_subscribers_pill.set_up_pill_typeahead({ + pill_widget, + $pill_container, + get_users, + get_user_groups, + }); + + add_subscribers_pill.set_up_handlers_for_add_button_state(pill_widget, $pill_container); + + return pill_widget; +} + export function create_without_add_button({ $pill_container, onPillCreateAction, diff --git a/web/src/add_subscribers_pill.ts b/web/src/add_subscribers_pill.ts index cc6b36545c..46743ab8c6 100644 --- a/web/src/add_subscribers_pill.ts +++ b/web/src/add_subscribers_pill.ts @@ -100,7 +100,7 @@ export function generate_pill_html(item: CombinedPill): string { return stream_pill.generate_pill_html(item); } -function set_up_handlers_for_add_button_state( +export function set_up_handlers_for_add_button_state( pill_widget: CombinedPillContainer, $pill_container: JQuery, ): void { @@ -127,11 +127,9 @@ function set_up_handlers_for_add_button_state( export function create({ $pill_container, get_potential_subscribers, - get_potential_groups, }: { $pill_container: JQuery; get_potential_subscribers: () => User[]; - get_potential_groups?: () => UserGroup[]; }): CombinedPillContainer { const pill_widget = input_pill.create({ $container: $pill_container, @@ -144,26 +142,8 @@ export function create({ const potential_subscribers = get_potential_subscribers(); return user_pill.filter_taken_users(potential_subscribers, pill_widget); } - const opts: { - pill_widget: CombinedPillContainer; - $pill_container: JQuery; - get_users: () => User[]; - get_user_groups?: () => UserGroup[]; - } = { - pill_widget, - $pill_container, - get_users, - }; - if (get_potential_groups !== undefined) { - function get_user_groups(): UserGroup[] { - assert(get_potential_groups !== undefined); - const potential_groups = get_potential_groups(); - return user_group_pill.filter_taken_groups(potential_groups, pill_widget); - } - opts.get_user_groups = get_user_groups; - } - set_up_pill_typeahead(opts); + set_up_pill_typeahead({pill_widget, $pill_container, get_users}); set_up_handlers_for_add_button_state(pill_widget, $pill_container); diff --git a/web/src/user_group_edit_members.ts b/web/src/user_group_edit_members.ts index d87e8ab3e6..b447113285 100644 --- a/web/src/user_group_edit_members.ts +++ b/web/src/user_group_edit_members.ts @@ -9,7 +9,6 @@ import render_user_group_membership_request_result from "../templates/user_group import render_user_group_subgroup_entry from "../templates/user_group_settings/user_group_subgroup_entry.hbs"; import * as add_group_members_pill from "./add_group_members_pill"; -import * as add_subscribers_pill from "./add_subscribers_pill"; import * as blueslip from "./blueslip"; import * as channel from "./channel"; import * as confirm_dialog from "./confirm_dialog"; @@ -142,9 +141,9 @@ export function enable_member_management({ // current_group_id and pill_widget are module-level variables current_group_id = group_id; - pill_widget = add_subscribers_pill.create({ + pill_widget = add_group_members_pill.create({ $pill_container, - get_potential_subscribers: get_potential_members, + get_potential_members, get_potential_groups: get_potential_subgroups, });