From e3081c667a44db469fac1e1de2d03b2d3106f100 Mon Sep 17 00:00:00 2001
From: Lunny Xiao <xiaolunwen@gmail.com>
Date: Fri, 6 Dec 2019 13:34:54 +0800
Subject: [PATCH] Only show part of members on orgnization dashboard and add
 paging for orgnization members page (#9092)

* Only show part of members on orgnization dashboard and add paging for orgnization members page

* fix test

* fix typo
---
 .../doc/advanced/config-cheat-sheet.en-us.md  |  1 +
 .../doc/advanced/config-cheat-sheet.zh-cn.md  |  1 +
 models/org.go                                 | 57 +++++++++++++++----
 models/org_test.go                            |  4 +-
 modules/setting/setting.go                    |  2 +
 routers/api/v1/org/member.go                  | 31 +++-------
 routers/org/members.go                        | 40 +++++++++++--
 routers/user/home.go                          | 29 +++++++++-
 templates/org/member/members.tmpl             |  2 +
 9 files changed, 121 insertions(+), 46 deletions(-)

diff --git a/docs/content/doc/advanced/config-cheat-sheet.en-us.md b/docs/content/doc/advanced/config-cheat-sheet.en-us.md
index 4fc8511b8..9f02e888c 100644
--- a/docs/content/doc/advanced/config-cheat-sheet.en-us.md
+++ b/docs/content/doc/advanced/config-cheat-sheet.en-us.md
@@ -113,6 +113,7 @@ Values containing `#` or `;` must be quoted using `` ` `` or `"""`.
 
 - `EXPLORE_PAGING_NUM`: **20**: Number of repositories that are shown in one explore page.
 - `ISSUE_PAGING_NUM`: **10**: Number of issues that are shown in one page (for all pages that list issues).
+- `MEMBERS_PAGING_NUM`: **20**: Number of members that are shown in organization members.
 - `FEED_MAX_COMMIT_NUM`: **5**: Number of maximum commits shown in one activity feed.
 - `GRAPH_MAX_COMMIT_NUM`: **100**: Number of maximum commits shown in the commit graph.
 - `DEFAULT_THEME`: **gitea**: \[gitea, arc-green\]: Set the default theme for the Gitea install.
diff --git a/docs/content/doc/advanced/config-cheat-sheet.zh-cn.md b/docs/content/doc/advanced/config-cheat-sheet.zh-cn.md
index 7940e8e1f..0f5d300e3 100644
--- a/docs/content/doc/advanced/config-cheat-sheet.zh-cn.md
+++ b/docs/content/doc/advanced/config-cheat-sheet.zh-cn.md
@@ -37,6 +37,7 @@ menu:
 
 - `EXPLORE_PAGING_NUM`: 探索页面每页显示的仓库数量。
 - `ISSUE_PAGING_NUM`: 工单页面每页显示的工单数量。
+- `MEMBERS_PAGING_NUM`: **20**: 组织成员页面每页显示的成员数量。
 - `FEED_MAX_COMMIT_NUM`: 活动流页面显示的最大提交数量。
 
 ### UI - Admin (`ui.admin`)
diff --git a/models/org.go b/models/org.go
index f14dad1db..dbc71761f 100644
--- a/models/org.go
+++ b/models/org.go
@@ -68,10 +68,35 @@ func (org *User) GetTeams() error {
 }
 
 // GetMembers returns all members of organization.
-func (org *User) GetMembers() error {
-	ous, err := GetOrgUsersByOrgID(org.ID)
+func (org *User) GetMembers() (err error) {
+	org.Members, org.MembersIsPublic, err = FindOrgMembers(FindOrgMembersOpts{
+		OrgID: org.ID,
+	})
+	return
+}
+
+// FindOrgMembersOpts represensts find org members condtions
+type FindOrgMembersOpts struct {
+	OrgID      int64
+	PublicOnly bool
+	Start      int
+	Limit      int
+}
+
+// CountOrgMembers counts the organization's members
+func CountOrgMembers(opts FindOrgMembersOpts) (int64, error) {
+	sess := x.Where("org_id=?", opts.OrgID)
+	if opts.PublicOnly {
+		sess.And("is_public = ?", true)
+	}
+	return sess.Count(new(OrgUser))
+}
+
+// FindOrgMembers loads organization members according conditions
+func FindOrgMembers(opts FindOrgMembersOpts) (UserList, map[int64]bool, error) {
+	ous, err := GetOrgUsersByOrgID(opts.OrgID, opts.PublicOnly, opts.Start, opts.Limit)
 	if err != nil {
-		return err
+		return nil, nil, err
 	}
 
 	var ids = make([]int64, len(ous))
@@ -80,9 +105,12 @@ func (org *User) GetMembers() error {
 		ids[i] = ou.UID
 		idsIsPublic[ou.UID] = ou.IsPublic
 	}
-	org.MembersIsPublic = idsIsPublic
-	org.Members, err = GetUsersByIDs(ids)
-	return err
+
+	users, err := GetUsersByIDs(ids)
+	if err != nil {
+		return nil, nil, err
+	}
+	return users, idsIsPublic, nil
 }
 
 // AddMember adds new member to organization.
@@ -467,15 +495,20 @@ func GetOrgUsersByUserID(uid int64, all bool) ([]*OrgUser, error) {
 }
 
 // GetOrgUsersByOrgID returns all organization-user relations by organization ID.
-func GetOrgUsersByOrgID(orgID int64) ([]*OrgUser, error) {
-	return getOrgUsersByOrgID(x, orgID)
+func GetOrgUsersByOrgID(orgID int64, publicOnly bool, start, limit int) ([]*OrgUser, error) {
+	return getOrgUsersByOrgID(x, orgID, publicOnly, start, limit)
 }
 
-func getOrgUsersByOrgID(e Engine, orgID int64) ([]*OrgUser, error) {
+func getOrgUsersByOrgID(e Engine, orgID int64, publicOnly bool, start, limit int) ([]*OrgUser, error) {
 	ous := make([]*OrgUser, 0, 10)
-	err := e.
-		Where("org_id=?", orgID).
-		Find(&ous)
+	sess := e.Where("org_id=?", orgID)
+	if publicOnly {
+		sess.And("is_public = ?", true)
+	}
+	if limit > 0 {
+		sess.Limit(limit, start)
+	}
+	err := sess.Find(&ous)
 	return ous, err
 }
 
diff --git a/models/org_test.go b/models/org_test.go
index 1a6b288dc..ac1a23991 100644
--- a/models/org_test.go
+++ b/models/org_test.go
@@ -395,7 +395,7 @@ func TestGetOrgUsersByUserID(t *testing.T) {
 func TestGetOrgUsersByOrgID(t *testing.T) {
 	assert.NoError(t, PrepareTestDatabase())
 
-	orgUsers, err := GetOrgUsersByOrgID(3)
+	orgUsers, err := GetOrgUsersByOrgID(3, false, 0, 0)
 	assert.NoError(t, err)
 	if assert.Len(t, orgUsers, 3) {
 		assert.Equal(t, OrgUser{
@@ -410,7 +410,7 @@ func TestGetOrgUsersByOrgID(t *testing.T) {
 			IsPublic: false}, *orgUsers[1])
 	}
 
-	orgUsers, err = GetOrgUsersByOrgID(NonexistentID)
+	orgUsers, err = GetOrgUsersByOrgID(NonexistentID, false, 0, 0)
 	assert.NoError(t, err)
 	assert.Len(t, orgUsers, 0)
 }
diff --git a/modules/setting/setting.go b/modules/setting/setting.go
index a97ab9467..c08621df5 100644
--- a/modules/setting/setting.go
+++ b/modules/setting/setting.go
@@ -159,6 +159,7 @@ var (
 		ExplorePagingNum      int
 		IssuePagingNum        int
 		RepoSearchPagingNum   int
+		MembersPagingNum      int
 		FeedMaxCommitNum      int
 		GraphMaxCommitNum     int
 		CodeCommentLines      int
@@ -191,6 +192,7 @@ var (
 		ExplorePagingNum:    20,
 		IssuePagingNum:      10,
 		RepoSearchPagingNum: 10,
+		MembersPagingNum:    20,
 		FeedMaxCommitNum:    5,
 		GraphMaxCommitNum:   100,
 		CodeCommentLines:    4,
diff --git a/routers/api/v1/org/member.go b/routers/api/v1/org/member.go
index be47b6963..45519e560 100644
--- a/routers/api/v1/org/member.go
+++ b/routers/api/v1/org/member.go
@@ -18,30 +18,13 @@ import (
 // listMembers list an organization's members
 func listMembers(ctx *context.APIContext, publicOnly bool) {
 	var members []*models.User
-	if publicOnly {
-		orgUsers, err := models.GetOrgUsersByOrgID(ctx.Org.Organization.ID)
-		if err != nil {
-			ctx.Error(500, "GetOrgUsersByOrgID", err)
-			return
-		}
-
-		memberIDs := make([]int64, 0, len(orgUsers))
-		for _, orgUser := range orgUsers {
-			if orgUser.IsPublic {
-				memberIDs = append(memberIDs, orgUser.UID)
-			}
-		}
-
-		if members, err = models.GetUsersByIDs(memberIDs); err != nil {
-			ctx.Error(500, "GetUsersByIDs", err)
-			return
-		}
-	} else {
-		if err := ctx.Org.Organization.GetMembers(); err != nil {
-			ctx.Error(500, "GetMembers", err)
-			return
-		}
-		members = ctx.Org.Organization.Members
+	members, _, err := models.FindOrgMembers(models.FindOrgMembersOpts{
+		OrgID:      ctx.Org.Organization.ID,
+		PublicOnly: publicOnly,
+	})
+	if err != nil {
+		ctx.Error(500, "GetUsersByIDs", err)
+		return
 	}
 
 	apiMembers := make([]*api.User, len(members))
diff --git a/routers/org/members.go b/routers/org/members.go
index f9cb275e8..b9805c2c0 100644
--- a/routers/org/members.go
+++ b/routers/org/members.go
@@ -25,14 +25,44 @@ func Members(ctx *context.Context) {
 	ctx.Data["Title"] = org.FullName
 	ctx.Data["PageIsOrgMembers"] = true
 
-	if err := org.GetMembers(); err != nil {
+	page := ctx.QueryInt("page")
+	if page <= 1 {
+		page = 1
+	}
+
+	var opts = models.FindOrgMembersOpts{
+		OrgID:      org.ID,
+		PublicOnly: true,
+	}
+
+	if ctx.User != nil {
+		isMember, err := ctx.Org.Organization.IsOrgMember(ctx.User.ID)
+		if err != nil {
+			ctx.Error(500, "IsOrgMember")
+			return
+		}
+		opts.PublicOnly = !isMember
+	}
+
+	total, err := models.CountOrgMembers(opts)
+	if err != nil {
+		ctx.Error(500, "CountOrgMembers")
+		return
+	}
+
+	pager := context.NewPagination(int(total), setting.UI.MembersPagingNum, page, 5)
+	opts.Start = (page - 1) * setting.UI.MembersPagingNum
+	opts.Limit = setting.UI.MembersPagingNum
+	members, membersIsPublic, err := models.FindOrgMembers(opts)
+	if err != nil {
 		ctx.ServerError("GetMembers", err)
 		return
 	}
-	ctx.Data["Members"] = org.Members
-	ctx.Data["MembersIsPublicMember"] = org.MembersIsPublic
-	ctx.Data["MembersIsUserOrgOwner"] = org.Members.IsUserOrgOwner(org.ID)
-	ctx.Data["MembersTwoFaStatus"] = org.Members.GetTwoFaStatus()
+	ctx.Data["Page"] = pager
+	ctx.Data["Members"] = members
+	ctx.Data["MembersIsPublicMember"] = membersIsPublic
+	ctx.Data["MembersIsUserOrgOwner"] = members.IsUserOrgOwner(org.ID)
+	ctx.Data["MembersTwoFaStatus"] = members.GetTwoFaStatus()
 
 	ctx.HTML(200, tplMembers)
 }
diff --git a/routers/user/home.go b/routers/user/home.go
index 8465216bc..2eff88910 100644
--- a/routers/user/home.go
+++ b/routers/user/home.go
@@ -537,14 +537,37 @@ func showOrgProfile(ctx *context.Context) {
 		return
 	}
 
-	if err := org.GetMembers(); err != nil {
-		ctx.ServerError("GetMembers", err)
+	var opts = models.FindOrgMembersOpts{
+		OrgID:      org.ID,
+		PublicOnly: true,
+		Limit:      25,
+	}
+
+	if ctx.User != nil {
+		isMember, err := org.IsOrgMember(ctx.User.ID)
+		if err != nil {
+			ctx.Error(500, "IsOrgMember")
+			return
+		}
+		opts.PublicOnly = !isMember
+	}
+
+	members, _, err := models.FindOrgMembers(opts)
+	if err != nil {
+		ctx.ServerError("FindOrgMembers", err)
+		return
+	}
+
+	membersCount, err := models.CountOrgMembers(opts)
+	if err != nil {
+		ctx.ServerError("CountOrgMembers", err)
 		return
 	}
 
 	ctx.Data["Repos"] = repos
 	ctx.Data["Total"] = count
-	ctx.Data["Members"] = org.Members
+	ctx.Data["MembersTotal"] = membersCount
+	ctx.Data["Members"] = members
 	ctx.Data["Teams"] = org.Teams
 
 	pager := context.NewPagination(int(count), setting.UI.User.RepoPagingNum, page, 5)
diff --git a/templates/org/member/members.tmpl b/templates/org/member/members.tmpl
index 9db506ee5..03aadf97b 100644
--- a/templates/org/member/members.tmpl
+++ b/templates/org/member/members.tmpl
@@ -57,6 +57,8 @@
 				</div>
 			{{end}}
 		</div>
+
+		{{template "base/paginate" .}}
 	</div>
 </div>
 {{template "base/footer" .}}