From a3b7d956bc1c548346deed4e2d0882339fa15ed9 Mon Sep 17 00:00:00 2001
From: Sahil Batra <sahil@zulip.com>
Date: Wed, 16 Oct 2024 14:47:58 +0530
Subject: [PATCH] user_groups: Allow adding users who are members of subgroups.

This commit updates the code, which checks if user is member of
the group before adding them to the group, to consider only
direct members and now allows members of subgroups to be added
as direct members of the group.
---
 web/src/user_group_edit_members.ts |  4 ++--
 web/src/user_groups.ts             | 10 +++++++++-
 web/tests/user_groups.test.js      | 12 ++++++++++++
 3 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/web/src/user_group_edit_members.ts b/web/src/user_group_edit_members.ts
index 90110d81d5..67bc19fddd 100644
--- a/web/src/user_group_edit_members.ts
+++ b/web/src/user_group_edit_members.ts
@@ -220,7 +220,7 @@ function add_new_members({pill_user_ids}: {pill_user_ids: number[]}): void {
             deactivated_users.add(user_id);
             return false;
         }
-        if (user_groups.is_user_in_group(group.id, user_id)) {
+        if (user_groups.is_user_in_group(group.id, user_id, true)) {
             // we filter out already added users before sending
             // add member request as the endpoint is not so robust and
             // fails complete request if any already added member
@@ -235,7 +235,7 @@ function add_new_members({pill_user_ids}: {pill_user_ids: number[]}): void {
 
     if (
         user_id_set.has(current_user.user_id) &&
-        user_groups.is_user_in_group(group.id, current_user.user_id)
+        user_groups.is_user_in_group(group.id, current_user.user_id, true)
     ) {
         // We don't want to send a request to add ourselves if we
         // are already added to this group. This case occurs
diff --git a/web/src/user_groups.ts b/web/src/user_groups.ts
index 5c4ccdfe6e..2172384c00 100644
--- a/web/src/user_groups.ts
+++ b/web/src/user_groups.ts
@@ -311,7 +311,11 @@ export function get_recursive_group_members(target_user_group: UserGroup): Set<n
     return members;
 }
 
-export function is_user_in_group(user_group_id: number, user_id: number): boolean {
+export function is_user_in_group(
+    user_group_id: number,
+    user_id: number,
+    direct_member_only = false,
+): boolean {
     const user_group = user_group_by_id_dict.get(user_group_id);
     if (user_group === undefined) {
         blueslip.error("Could not find user group", {user_group_id});
@@ -321,6 +325,10 @@ export function is_user_in_group(user_group_id: number, user_id: number): boolea
         return true;
     }
 
+    if (direct_member_only) {
+        return false;
+    }
+
     const subgroup_ids = get_recursive_subgroups(user_group);
     if (subgroup_ids === undefined) {
         return false;
diff --git a/web/tests/user_groups.test.js b/web/tests/user_groups.test.js
index 668c87c197..51825e8112 100644
--- a/web/tests/user_groups.test.js
+++ b/web/tests/user_groups.test.js
@@ -349,6 +349,18 @@ run_test("is_user_in_group", () => {
     assert.equal(user_groups.is_user_in_group(foo.id, 6), true);
     assert.equal(user_groups.is_user_in_group(foo.id, 3), false);
 
+    // Check when passing direct_member_only as true.
+    assert.equal(user_groups.is_user_in_group(admins.id, 1, true), true);
+    assert.equal(user_groups.is_user_in_group(admins.id, 6, true), false);
+
+    assert.equal(user_groups.is_user_in_group(all.id, 2, true), true);
+    assert.equal(user_groups.is_user_in_group(all.id, 1, true), false);
+    assert.equal(user_groups.is_user_in_group(all.id, 6, true), false);
+
+    assert.equal(user_groups.is_user_in_group(test.id, 4, true), true);
+    assert.equal(user_groups.is_user_in_group(test.id, 1, true), false);
+    assert.equal(user_groups.is_user_in_group(test.id, 6, true), false);
+
     assert.equal(user_groups.is_user_in_setting_group(test.id, 4), true);
     assert.equal(user_groups.is_user_in_setting_group(test.id, 1), true);
     assert.equal(user_groups.is_user_in_setting_group(test.id, 6), true);