From 054f08c6dfef6a4a4ea94d31719e8c5fb9efd2ae Mon Sep 17 00:00:00 2001
From: Shubham Padia <shubham@zulip.com>
Date: Thu, 11 Jul 2024 08:45:15 +0000
Subject: [PATCH] user_group_pill: Exclude inactive members from pill.

While using the pill in `stream_create`, it was noticed that deactivated
users were also part of the user_ids returned by the user_group_pill,
which we do not want to do.
This pill is used in two areas: compose box and stream create/edit.
This commit will only affect stream create/edit since compose box was
just using the typeahead data from user_group_pill and nothing else.
Stream and user pill handle inactive users already, so no change is
needed there.
---
 web/src/user_group_pill.ts        | 12 ++++++---
 web/tests/user_group_pill.test.js | 42 +++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+), 3 deletions(-)

diff --git a/web/src/user_group_pill.ts b/web/src/user_group_pill.ts
index 89ba603734..0b697ac915 100644
--- a/web/src/user_group_pill.ts
+++ b/web/src/user_group_pill.ts
@@ -1,5 +1,6 @@
 import {$t_html} from "./i18n";
 import type {InputPillContainer, InputPillItem} from "./input_pill";
+import * as people from "./people";
 import type {CombinedPillContainer, CombinedPillItem} from "./typeahead_helper";
 import type {UserGroup} from "./user_groups";
 import * as user_groups from "./user_groups";
@@ -18,10 +19,10 @@ export type UserGroupPillData = UserGroup & {
 };
 
 function display_pill(group: UserGroup): string {
-    const group_members = user_groups.get_recursive_group_members(group);
+    const group_members = get_group_members(group);
     return $t_html(
         {defaultMessage: "{group_name}: {group_size, plural, one {# user} other {# users}}"},
-        {group_name: user_groups.get_display_group_name(group), group_size: group_members.size},
+        {group_name: user_groups.get_display_group_name(group), group_size: group_members.length},
     );
 }
 
@@ -56,7 +57,7 @@ export function get_user_ids(pill_widget: UserGroupPillWidget | CombinedPillCont
     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);
+            const group_members = get_group_members(user_group);
             user_ids.push(...group_members);
         }
     }
@@ -67,6 +68,11 @@ export function get_user_ids(pill_widget: UserGroupPillWidget | CombinedPillCont
     return user_ids;
 }
 
+function get_group_members(user_group: UserGroup): number[] {
+    const user_ids = [...user_groups.get_recursive_group_members(user_group)];
+    return user_ids.filter((user_id) => people.is_person_active(user_id));
+}
+
 export function append_user_group(group: UserGroup, pill_widget: CombinedPillContainer): void {
     pill_widget.appendValidatedData({
         type: "user_group",
diff --git a/web/tests/user_group_pill.test.js b/web/tests/user_group_pill.test.js
index 150ea6a295..0e79048bca 100644
--- a/web/tests/user_group_pill.test.js
+++ b/web/tests/user_group_pill.test.js
@@ -7,6 +7,42 @@ const {run_test} = require("./lib/test");
 
 const user_groups = zrequire("user_groups");
 const user_group_pill = zrequire("user_group_pill");
+const people = zrequire("people");
+
+const user1 = {
+    user_id: 10,
+    email: "user1@example.com",
+    full_name: "User One",
+};
+people.add_active_user(user1);
+
+const user2 = {
+    user_id: 20,
+    email: "user2@example.com",
+    full_name: "User Two",
+};
+people.add_active_user(user2);
+
+const user3 = {
+    user_id: 30,
+    email: "user3@example.com",
+    full_name: "User Three",
+};
+people.add_active_user(user3);
+
+const user4 = {
+    user_id: 40,
+    email: "user4@example.com",
+    full_name: "User Four",
+};
+people.add_active_user(user4);
+
+const user5 = {
+    user_id: 50,
+    email: "user5@example.com",
+    full_name: "User Five",
+};
+people.add_active_user(user5);
 
 const admins = {
     name: "Admins",
@@ -83,6 +119,12 @@ run_test("get_user_ids", () => {
     items = [everyone_pill];
     user_ids = user_group_pill.get_user_ids(widget);
     assert.deepEqual(user_ids, [10, 20, 30, 40, 50]);
+
+    // Deactivated users should be excluded.
+    people.deactivate(user5);
+    user_ids = user_group_pill.get_user_ids(widget);
+    assert.deepEqual(user_ids, [10, 20, 30, 40]);
+    people.add_active_user(user5);
 });
 
 run_test("get_group_ids", () => {