diff --git a/web/src/add_subscribers_pill.ts b/web/src/add_subscribers_pill.ts index 452085007d..46f7544330 100644 --- a/web/src/add_subscribers_pill.ts +++ b/web/src/add_subscribers_pill.ts @@ -11,6 +11,7 @@ import * as stream_pill from "./stream_pill"; import type {CombinedPill, CombinedPillContainer} from "./typeahead_helper"; 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"; export function create_item_from_text( @@ -51,17 +52,28 @@ export function set_up_pill_typeahead({ pill_widget, $pill_container, get_users, + get_user_groups, }: { pill_widget: CombinedPillContainer; $pill_container: JQuery; get_users: () => User[]; + get_user_groups?: () => UserGroup[]; }): void { - const opts = { + const opts: { + user_source: () => User[]; + stream: boolean; + user_group: boolean; + user: boolean; + user_group_source?: () => UserGroup[]; + } = { user_source: get_users, stream: true, user_group: true, user: true, }; + if (get_user_groups !== undefined) { + opts.user_group_source = get_user_groups; + } pill_typeahead.set_up_combined($pill_container.find(".input"), pill_widget, opts); } @@ -91,9 +103,11 @@ export function generate_pill_html(item: CombinedPill): string { 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, @@ -106,8 +120,26 @@ 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({pill_widget, $pill_container, get_users}); + set_up_pill_typeahead(opts); const $pill_widget_input = $pill_container.find(".input"); const $pill_widget_button = $pill_container.parent().find(".add-users-button"); diff --git a/web/src/pill_typeahead.ts b/web/src/pill_typeahead.ts index ee0eb356ec..fad05a283f 100644 --- a/web/src/pill_typeahead.ts +++ b/web/src/pill_typeahead.ts @@ -222,6 +222,7 @@ export function set_up_combined( user_group?: boolean; stream?: boolean; user_source?: () => User[]; + user_group_source?: () => UserGroup[]; exclude_bots?: boolean; update_func?: () => void; }, @@ -252,7 +253,14 @@ export function set_up_combined( } if (include_user_groups) { - source = [...source, ...user_group_pill.typeahead_source(pills)]; + if (opts.user_group_source !== undefined) { + const groups: UserGroupPillData[] = opts + .user_group_source() + .map((user_group) => ({type: "user_group", ...user_group})); + source = [...source, ...groups]; + } else { + source = [...source, ...user_group_pill.typeahead_source(pills)]; + } } if (include_users) { diff --git a/web/src/user_group_edit_members.ts b/web/src/user_group_edit_members.ts index 9a6b12b0c0..d87e8ab3e6 100644 --- a/web/src/user_group_edit_members.ts +++ b/web/src/user_group_edit_members.ts @@ -44,6 +44,10 @@ function get_potential_members(): User[] { return people.filter_all_users(is_potential_member); } +function get_potential_subgroups(): UserGroup[] { + return user_groups.get_potential_subgroups(current_group_id); +} + function get_user_group_members(group: UserGroup): (User | UserGroup)[] { const member_ids = [...group.members]; const member_users = people.get_users_from_ids(member_ids); @@ -141,6 +145,7 @@ export function enable_member_management({ pill_widget = add_subscribers_pill.create({ $pill_container, get_potential_subscribers: get_potential_members, + get_potential_groups: get_potential_subgroups, }); $pill_container.find(".input").on("input", () => { diff --git a/web/src/user_groups.ts b/web/src/user_groups.ts index 2172384c00..492b5afd39 100644 --- a/web/src/user_groups.ts +++ b/web/src/user_groups.ts @@ -311,6 +311,30 @@ export function get_recursive_group_members(target_user_group: UserGroup): Set { + if (user_group.id === target_user_group.id) { + return false; + } + + if (already_subgroup_ids.has(user_group.id)) { + return false; + } + + const recursive_subgroup_ids = get_recursive_subgroups(user_group); + assert(recursive_subgroup_ids !== undefined); + if (recursive_subgroup_ids.has(target_user_group.id)) { + return false; + } + return true; + }); +} + export function is_user_in_group( user_group_id: number, user_id: number, diff --git a/web/tests/pill_typeahead.test.js b/web/tests/pill_typeahead.test.js index b8723fa3a3..33fb250c8f 100644 --- a/web/tests/pill_typeahead.test.js +++ b/web/tests/pill_typeahead.test.js @@ -465,7 +465,11 @@ run_test("set_up_combined", ({mock_template, override, override_rewire}) => { }) .filter(Boolean); if (opts.user_group) { - expected_result = [...expected_result, ...group_items]; + if (opts.user_group_source) { + expected_result = [...expected_result, ...opts.user_group_source()]; + } else { + expected_result = [...expected_result, ...group_items]; + } } if (opts.user) { if (opts.user_source) { @@ -530,6 +534,8 @@ run_test("set_up_combined", ({mock_template, override, override_rewire}) => { {user: true, user_source: () => [fred_item, mark_item]}, {stream: true}, {user_group: true}, + // user and custom user group source. + {user_group: true, user_group_source: () => [admins_item]}, {user_group: true, stream: true}, {user_group: true, user: true}, {user: true, stream: true}, diff --git a/web/tests/user_groups.test.js b/web/tests/user_groups.test.js index 51825e8112..48a6121eb6 100644 --- a/web/tests/user_groups.test.js +++ b/web/tests/user_groups.test.js @@ -583,3 +583,74 @@ run_test("get_display_group_name", () => { assert.equal(user_groups.get_display_group_name(all.name), "translated: Everyone"); assert.equal(user_groups.get_display_group_name(students.name), "Students"); }); + +run_test("get_potential_subgroups", () => { + // Remove existing groups. + user_groups.init(); + + const admins = { + name: "Administrators", + id: 1, + members: new Set([1]), + is_system_group: false, + direct_subgroup_ids: new Set([4]), + }; + const all = { + name: "Everyone", + id: 2, + members: new Set([2, 3]), + is_system_group: false, + direct_subgroup_ids: new Set([1, 3]), + }; + const students = { + name: "Students", + id: 3, + members: new Set([4, 5]), + is_system_group: false, + direct_subgroup_ids: new Set([]), + }; + const teachers = { + name: "Teachers", + id: 4, + members: new Set([6, 7, 8]), + is_system_group: false, + direct_subgroup_ids: new Set([]), + }; + const science = { + name: "Science", + id: 5, + members: new Set([9]), + is_system_group: false, + direct_subgroup_ids: new Set([]), + }; + + user_groups.initialize({ + realm_user_groups: [admins, all, students, teachers, science], + }); + + function get_potential_subgroup_ids(group_id) { + return user_groups + .get_potential_subgroups(group_id) + .map((subgroup) => subgroup.id) + .sort(); + } + + assert.deepEqual(get_potential_subgroup_ids(all.id), [teachers.id, science.id]); + assert.deepEqual(get_potential_subgroup_ids(admins.id), [students.id, science.id]); + assert.deepEqual(get_potential_subgroup_ids(teachers.id), [students.id, science.id]); + assert.deepEqual(get_potential_subgroup_ids(students.id), [admins.id, teachers.id, science.id]); + assert.deepEqual(get_potential_subgroup_ids(science.id), [ + admins.id, + all.id, + students.id, + teachers.id, + ]); + + user_groups.add_subgroups(all.id, [teachers.id]); + user_groups.add_subgroups(teachers.id, [science.id]); + assert.deepEqual(get_potential_subgroup_ids(all.id), [science.id]); + assert.deepEqual(get_potential_subgroup_ids(admins.id), [students.id, science.id]); + assert.deepEqual(get_potential_subgroup_ids(teachers.id), [students.id]); + assert.deepEqual(get_potential_subgroup_ids(students.id), [admins.id, teachers.id, science.id]); + assert.deepEqual(get_potential_subgroup_ids(science.id), [students.id]); +});