From bae14944adb29fe077eb8c79fb4392eb17c3cec8 Mon Sep 17 00:00:00 2001
From: Sahil Batra <sahil@zulip.com>
Date: Tue, 28 Jan 2025 15:14:34 +0530
Subject: [PATCH] group_permission_settings: Refactor
 get_assigned_permission_object.

- Function now returns early if can_edit_settings is false since
we do not want details if the group has direct permission or not.
- Added function to get the tooltip text if group has permission
due to being subgroup of a group that has permission.
- Added comments for some explaination around different blocks.
---
 web/src/group_permission_settings.ts | 75 ++++++++++++++++++----------
 1 file changed, 49 insertions(+), 26 deletions(-)

diff --git a/web/src/group_permission_settings.ts b/web/src/group_permission_settings.ts
index de00102991..a91cfcd347 100644
--- a/web/src/group_permission_settings.ts
+++ b/web/src/group_permission_settings.ts
@@ -137,71 +137,94 @@ export type AssignedGroupPermission = {
     tooltip_message?: string;
 };
 
+export function get_tooltip_for_group_without_direct_permission(supergroup_id: number): string {
+    const supergroup = user_groups.get_user_group_from_id(supergroup_id);
+    return $t(
+        {
+            defaultMessage:
+                "This group has this permission because it's a subgroup of {supergroup_name}.",
+        },
+        {
+            supergroup_name: user_groups.get_display_group_name(supergroup.name),
+        },
+    );
+}
+
 export function get_assigned_permission_object(
     setting_value: GroupSettingValue,
     setting_name: RealmGroupSettingName | StreamGroupSettingName | GroupGroupSettingName,
     group_id: number,
     can_edit_settings: boolean,
 ): AssignedGroupPermission | undefined {
+    // This function returns an object of type AssignedGroupPermission
+    // containing details about whether the user can edit the setting,
+    // if the group has the permission, and returns undefined otherwise.
     const assigned_permission_object: AssignedGroupPermission = {
         setting_name,
         can_edit: can_edit_settings,
     };
 
     if (!can_edit_settings) {
+        if (!user_groups.group_has_permission(setting_value, group_id)) {
+            // The group does not have permission.
+            return undefined;
+        }
+
+        // Since user cannot change this setting, the tooltip is
+        // the same whether the group has direct permission or has
+        // permission due to being subgroup of a group with permission.
         assigned_permission_object.tooltip_message = $t({
             defaultMessage: "You are not allowed to remove this permission.",
         });
+        return assigned_permission_object;
     }
 
+    // The user has permission to change the setting, but whether the user
+    // will be able to remove the permission for this particular group
+    // depends on whether the group has the permission directly or not.
     if (typeof setting_value === "number") {
         if (setting_value === group_id) {
+            // The group has permission directly, so the user can remove
+            // the permission for this particular group, and there is no
+            // need to show a tooltip.
             return assigned_permission_object;
         }
 
         if (user_groups.is_subgroup_of_target_group(setting_value, group_id)) {
-            if (can_edit_settings) {
-                assigned_permission_object.can_edit = false;
-                const supergroup = user_groups.get_user_group_from_id(setting_value);
-                assigned_permission_object.tooltip_message = $t(
-                    {
-                        defaultMessage:
-                            "This group has this permission because it's a subgroup of {supergroup_name}.",
-                    },
-                    {
-                        supergroup_name: user_groups.get_display_group_name(supergroup.name),
-                    },
-                );
-            }
+            // The group has permission because it is one of the subgroups of
+            // the group that has permission. Therefore, the user cannot remove
+            // the permission for this group, and the UI should show a disabled
+            // checkbox with an appropriate tooltip.
+            assigned_permission_object.can_edit = false;
+            assigned_permission_object.tooltip_message =
+                get_tooltip_for_group_without_direct_permission(setting_value);
             return assigned_permission_object;
         }
 
+        // The group does not have permission.
         return undefined;
     }
 
+    // Setting is set to an anonymous group.
     const direct_subgroup_ids = setting_value.direct_subgroups;
     if (direct_subgroup_ids.includes(group_id)) {
+        // The group is one of the groups that has permission and can be
+        // changed to not have permission.
         return assigned_permission_object;
     }
 
     for (const direct_subgroup_id of direct_subgroup_ids) {
         if (user_groups.is_subgroup_of_target_group(direct_subgroup_id, group_id)) {
-            if (can_edit_settings) {
-                assigned_permission_object.can_edit = false;
-                const supergroup = user_groups.get_user_group_from_id(direct_subgroup_id);
-                assigned_permission_object.tooltip_message = $t(
-                    {
-                        defaultMessage:
-                            "This group has this permission because it's a subgroup of {supergroup_name}.",
-                    },
-                    {
-                        supergroup_name: user_groups.get_display_group_name(supergroup.name),
-                    },
-                );
-            }
+            // The group has permission because it is a subgroup of one of the
+            // groups that has permission. Therefore, the user cannot remove the
+            // permission for this group.
+            assigned_permission_object.can_edit = false;
+            assigned_permission_object.tooltip_message =
+                get_tooltip_for_group_without_direct_permission(direct_subgroup_id);
             return assigned_permission_object;
         }
     }
 
+    // The group does not have permission.
     return undefined;
 }