user_group_pill: Add support for subgroups.

While the support to include all members of a subgroup is needed only in
the `stream_create` context for now, we have added the support for
subgroups to `user_group_pill` for all cases. We have done this because
that is still going to be the correct behaviour if we add similar
support to other pill inputs.

In terms of calculating and populating the recursive member list, it was
decided not to do it when initializing the user_groups data. One reason
for that was it would introduce a lot more complexity when adding or
removing members from any of the subgroups to keep the recursive member
list updated. Keeping in line with the general pattern of calculating
recursive subgroups on the fly too, it was decided to calculate the
recursive list of members on the fly too.

Also changes the `get_group_ids` tests to make sure that subgroup ids
are not part of the result of `get_group_ids`. Since it is used to
calculated taken_groups, we don't want to filter out subgroups as part
of taken_groups and their typeahead should still be visible.

web/src/user_group_pill.ts

@@ -17,7 +17,8 @@ export type UserGroupPillData = UserGroup & {
 };
 
 function display_pill(group: UserGroup): string {
-    return `${group.name}: ${group.members.size} users`;
+    const group_members = user_groups.get_recursive_group_members(group);
+    return `${group.name}: ${group_members.size} users`;
 }
 
 export function create_item_from_group_name(
@@ -47,13 +48,15 @@ export function get_group_name_from_item(item: InputPillItem<UserGroupPill>): st
 }
 
 export function get_user_ids(pill_widget: UserGroupPillWidget | CombinedPillContainer): number[] {
-    let user_ids = pill_widget
-        .items()
-        .flatMap((item) =>
-            item.type === "user_group"
-                ? [...user_groups.get_user_group_from_id(item.group_id).members]
-                : [],
-        );
+    let user_ids: number[] = [];
+    for (const user_group_item of pill_widget.items()) {
+        if (user_group_item.type === "user_group") {
+            const user_group = user_groups.get_user_group_from_id(user_group_item.group_id);
+            const group_members = user_groups.get_recursive_group_members(user_group);
+            user_ids.push(...group_members);
+        }
+    }
+
     user_ids = [...new Set(user_ids)];
     user_ids.sort((a, b) => a - b);
 

web/tests/user_group_pill.test.js

@@ -20,6 +20,13 @@ const testers = {
     id: 102,
     members: [20, 50, 30, 40],
 };
+const everyone = {
+    name: "role:everyone",
+    description: "Everyone",
+    id: 103,
+    members: [],
+    direct_subgroup_ids: [101, 102],
+};
 
 const admins_pill = {
     group_id: admins.id,
@@ -33,8 +40,17 @@ const testers_pill = {
     type: "user_group",
     display_value: testers.name + ": " + testers.members.length + " users",
 };
+const everyone_pill = {
+    group_id: everyone.id,
+    group_name: everyone.name,
+    type: "user_group",
+    // While we can programmatically set the user count below,
+    // calculating it would almost mimic the entire display function
+    // here, reducing the usefulness of the test.
+    display_value: everyone.name + ": 5 users",
+};
 
-const groups = [admins, testers];
+const groups = [admins, testers, everyone];
 for (const group of groups) {
     user_groups.add(group);
 }
@@ -49,6 +65,7 @@ run_test("create_item", () => {
     test_create_item("admins", [testers_pill], admins_pill);
     test_create_item("admins", [admins_pill], undefined);
     test_create_item("unknown", [], undefined);
+    test_create_item("role:everyone", [], everyone_pill);
 });
 
 run_test("get_stream_id", () => {
@@ -56,19 +73,25 @@ run_test("get_stream_id", () => {
 });
 
 run_test("get_user_ids", () => {
-    const items = [admins_pill, testers_pill];
+    let items = [admins_pill, testers_pill];
     const widget = {items: () => items};
 
-    const user_ids = user_group_pill.get_user_ids(widget);
+    let user_ids = user_group_pill.get_user_ids(widget);
+    assert.deepEqual(user_ids, [10, 20, 30, 40, 50]);
+
+    // Test whether subgroup members are included or not.
+    items = [everyone_pill];
+    user_ids = user_group_pill.get_user_ids(widget);
     assert.deepEqual(user_ids, [10, 20, 30, 40, 50]);
 });
 
 run_test("get_group_ids", () => {
-    const items = [admins_pill, testers_pill];
+    const items = [admins_pill, everyone_pill];
     const widget = {items: () => items};
 
+    // Subgroups should not be part of the results, we use `everyone_pill` to test that.
     const group_ids = user_group_pill.get_group_ids(widget);
-    assert.deepEqual(group_ids, [101, 102]);
+    assert.deepEqual(group_ids, [101, 103]);
 });
 
 run_test("append_user_group", () => {