feat: optimize get-groups API and GroupListPage (#3518)

* fix: optimize get-groups api and GroupListPage

* fix: fix linter issue

controllers/group.go

@@ -70,15 +70,33 @@ func (c *ApiController) GetGroups() {
 		if err != nil {
 			c.ResponseError(err.Error())
 			return
-		} else {
+		}
-			err = object.ExtendGroupsWithUsers(groups)
+		groupsHaveChildrenMap, err := object.GetGroupsHaveChildrenMap(groups)
-			if err != nil {
+		if err != nil {
-				c.ResponseError(err.Error())
+			c.ResponseError(err.Error())
-				return
+			return
+		}
+
+		for _, group := range groups {
+			_, ok := groupsHaveChildrenMap[group.Name]
+			if ok {
+				group.HaveChildren = true
 			}
 
-			c.ResponseOk(groups, paginator.Nums())
+			parent, ok := groupsHaveChildrenMap[group.ParentId]
+			if ok {
+				group.ParentName = parent.DisplayName
+			}
 		}
+
+		err = object.ExtendGroupsWithUsers(groups)
+		if err != nil {
+			c.ResponseError(err.Error())
+			return
+		}
+
+		c.ResponseOk(groups, paginator.Nums())
+
 	}
 }
 

object/group.go

@@ -17,7 +17,6 @@ package object
 import (
 	"errors"
 	"fmt"
-	"sync"
 
 	"github.com/casdoor/casdoor/conf"
 	"github.com/casdoor/casdoor/util"
@@ -36,12 +35,14 @@ type Group struct {
 	ContactEmail string   `xorm:"varchar(100)" json:"contactEmail"`
 	Type         string   `xorm:"varchar(100)" json:"type"`
 	ParentId     string   `xorm:"varchar(100)" json:"parentId"`
+	ParentName   string   `xorm:"-" json:"parentName"`
 	IsTopGroup   bool     `xorm:"bool" json:"isTopGroup"`
 	Users        []string `xorm:"-" json:"users"`
 
-	Title    string   `json:"title,omitempty"`
+	Title        string   `json:"title,omitempty"`
-	Key      string   `json:"key,omitempty"`
+	Key          string   `json:"key,omitempty"`
-	Children []*Group `json:"children,omitempty"`
+	HaveChildren bool     `xorm:"-" json:"haveChildren"`
+	Children     []*Group `json:"children,omitempty"`
 
 	IsEnabled bool `json:"isEnabled"`
 }
@@ -79,6 +80,26 @@ func GetPaginationGroups(owner string, offset, limit int, field, value, sortFiel
 	return groups, nil
 }
 
+func GetGroupsHaveChildrenMap(groups []*Group) (map[string]*Group, error) {
+	groupsHaveChildren := []*Group{}
+	resultMap := make(map[string]*Group)
+
+	groupIds := []string{}
+	for _, group := range groups {
+		groupIds = append(groupIds, group.Name)
+		groupIds = append(groupIds, group.ParentId)
+	}
+
+	err := ormer.Engine.Cols("owner", "name", "parent_id", "display_name").Distinct("parent_id").In("parent_id", groupIds).Find(&groupsHaveChildren)
+	if err != nil {
+		return nil, err
+	}
+	for _, group := range groups {
+		resultMap[group.Name] = group
+	}
+	return resultMap, nil
+}
+
 func getGroup(owner string, name string) (*Group, error) {
 	if owner == "" || name == "" {
 		return nil, nil
@@ -298,17 +319,11 @@ func ExtendGroupWithUsers(group *Group) error {
 		return nil
 	}
 
-	users, err := GetUsers(group.Owner)
-	if err != nil {
-		return err
-	}
-
 	groupId := group.GetId()
 	userIds := []string{}
-	for _, user := range users {
+	userIds, err := userEnforcer.GetAllUsersByGroup(groupId)
-		if util.InSlice(user.Groups, groupId) {
+	if err != nil {
-			userIds = append(userIds, user.GetId())
+		return err
-		}
 	}
 
 	group.Users = userIds
@@ -316,29 +331,14 @@ func ExtendGroupWithUsers(group *Group) error {
 }
 
 func ExtendGroupsWithUsers(groups []*Group) error {
-	var wg sync.WaitGroup
-	errChan := make(chan error, len(groups))
-
 	for _, group := range groups {
-		wg.Add(1)
+		users, err := userEnforcer.GetAllUsersByGroup(group.GetId())
-		go func(group *Group) {
-			defer wg.Done()
-			err := ExtendGroupWithUsers(group)
-			if err != nil {
-				errChan <- err
-			}
-		}(group)
-	}
-
-	wg.Wait()
-	close(errChan)
-
-	for err := range errChan {
 		if err != nil {
 			return err
 		}
-	}
 
+		group.Users = users
+	}
 	return nil
 }
 

web/src/GroupListPage.js

@@ -33,18 +33,6 @@ class GroupListPage extends BaseListPage {
   }
   UNSAFE_componentWillMount() {
     super.UNSAFE_componentWillMount();
-    this.getGroups(this.state.owner);
-  }
-
-  getGroups(organizationName) {
-    GroupBackend.getGroups(organizationName)
-      .then((res) => {
-        if (res.status === "ok") {
-          this.setState({
-            groups: res.data,
-          });
-        }
-      });
   }
 
   newGroup() {
@@ -188,12 +176,8 @@ class GroupListPage extends BaseListPage {
               {record.parentId}
             </Link>;
           }
-          const parentGroup = this.state.groups.find((group) => group.name === text);
+          return <Link to={`/groups/${record.owner}/${record.parentId}`}>
-          if (parentGroup === undefined) {
+            {record?.parentName}
-            return "";
-          }
-          return <Link to={`/groups/${parentGroup.owner}/${parentGroup.name}`}>
-            {parentGroup?.displayName}
           </Link>;
         },
       },
@@ -215,12 +199,11 @@ class GroupListPage extends BaseListPage {
         width: "180px",
         fixed: (Setting.isMobile()) ? "false" : "right",
         render: (text, record, index) => {
-          const haveChildren = this.state.groups.find((group) => group.parentId === record.id) !== undefined;
           return (
             <div>
               <Button style={{marginTop: "10px", marginBottom: "10px", marginRight: "10px"}} type="primary" onClick={() => this.props.history.push(`/groups/${record.owner}/${record.name}`)}>{i18next.t("general:Edit")}</Button>
               <PopconfirmModal
-                disabled={haveChildren}
+                disabled={record.haveChildren}
                 title={i18next.t("general:Sure to delete") + `: ${record.name} ?`}
                 onConfirm={() => this.deleteGroup(index)}
               >