Calculate PublicOnly for org membership only once

models/organization/org.go

@@ -141,8 +141,9 @@ func (org *Organization) LoadTeams(ctx context.Context) ([]*Team, error) {
 }
 
 // GetMembers returns all members of organization.
-func (org *Organization) GetMembers(ctx context.Context) (user_model.UserList, map[int64]bool, error) {
+func (org *Organization) GetMembers(ctx context.Context, doer *user_model.User) (user_model.UserList, map[int64]bool, error) {
 return FindOrgMembers(ctx, &FindOrgMembersOpts{
+Doer: doer,
 OrgID: org.ID,
 })
 }
@@ -195,16 +196,22 @@ func (org *Organization) CanCreateRepo() bool {
 // FindOrgMembersOpts represensts find org members conditions
 type FindOrgMembersOpts struct {
 db.ListOptions
-OrgID int64
-PublicOnly bool
+Doer *user_model.User
+IsMember bool
+OrgID int64
+}
+
+func (opts FindOrgMembersOpts) PublicOnly() bool {
+return opts.Doer == nil || !(opts.IsMember || opts.Doer.IsAdmin)
 }
 
 // CountOrgMembers counts the organization's members
 func CountOrgMembers(ctx context.Context, opts *FindOrgMembersOpts) (int64, error) {
 sess := db.GetEngine(ctx).Where("org_id=?", opts.OrgID)
-if opts.PublicOnly {
+if opts.PublicOnly() {
 sess.And("is_public = ?", true)
 }
+
 return sess.Count(new(OrgUser))
 }
 
@@ -525,9 +532,10 @@ func GetOrgsCanCreateRepoByUserID(ctx context.Context, userID int64) ([]*Organiz
 // GetOrgUsersByOrgID returns all organization-user relations by organization ID.
 func GetOrgUsersByOrgID(ctx context.Context, opts *FindOrgMembersOpts) ([]*OrgUser, error) {
 sess := db.GetEngine(ctx).Where("org_id=?", opts.OrgID)
-if opts.PublicOnly {
+if opts.PublicOnly() {
 sess.And("is_public = ?", true)
 }
+
 if opts.ListOptions.PageSize > 0 {
 sess = db.SetSessionPagination(sess, opts)
 

models/organization/org_test.go

@@ -4,6 +4,7 @@
 package organization_test
 
 import (
+"sort"
 "testing"
 
 "code.gitea.io/gitea/models/db"
@@ -103,7 +104,7 @@ func TestUser_GetTeams(t *testing.T) {
 func TestUser_GetMembers(t *testing.T) {
 assert.NoError(t, unittest.PrepareTestDatabase())
 org := unittest.AssertExistsAndLoadBean(t, &organization.Organization{ID: 3})
-members, _, err := org.GetMembers(db.DefaultContext)
+members, _, err := org.GetMembers(db.DefaultContext, &user_model.User{IsAdmin: true})
 assert.NoError(t, err)
 if assert.Len(t, members, 3) {
 assert.Equal(t, int64(2), members[0].ID)
@@ -210,37 +211,42 @@ func TestFindOrgs(t *testing.T) {
 func TestGetOrgUsersByOrgID(t *testing.T) {
 assert.NoError(t, unittest.PrepareTestDatabase())
 
-orgUsers, err := organization.GetOrgUsersByOrgID(db.DefaultContext, &organization.FindOrgMembersOpts{
-ListOptions: db.ListOptions{},
-OrgID: 3,
-PublicOnly: false,
+opts := &organization.FindOrgMembersOpts{
+Doer: &user_model.User{IsAdmin: true},
+OrgID: 3,
+}
+assert.False(t, opts.PublicOnly())
+orgUsers, err := organization.GetOrgUsersByOrgID(db.DefaultContext, opts)
+assert.NoError(t, err)
+sort.Slice(orgUsers, func(i, j int) bool {
+return orgUsers[i].ID < orgUsers[j].ID
 })
+assert.EqualValues(t, []*organization.OrgUser{{
+ID: 1,
+OrgID: 3,
+UID: 2,
+IsPublic: true,
+}, {
+ID: 2,
+OrgID: 3,
+UID: 4,
+IsPublic: false,
+}, {
+ID: 9,
+OrgID: 3,
+UID: 28,
+IsPublic: true,
+}}, orgUsers)
+
+opts = &organization.FindOrgMembersOpts{OrgID: 3}
+assert.True(t, opts.PublicOnly())
+orgUsers, err = organization.GetOrgUsersByOrgID(db.DefaultContext, opts)
 assert.NoError(t, err)
-if assert.Len(t, orgUsers, 3) {
-assert.Equal(t, organization.OrgUser{
-ID: orgUsers[0].ID,
-OrgID: 3,
-UID: 2,
-IsPublic: true,
-}, *orgUsers[0])
-assert.Equal(t, organization.OrgUser{
-ID: orgUsers[1].ID,
-OrgID: 3,
-UID: 4,
-IsPublic: false,
-}, *orgUsers[1])
-assert.Equal(t, organization.OrgUser{
-ID: orgUsers[2].ID,
-OrgID: 3,
-UID: 28,
-IsPublic: true,
-}, *orgUsers[2])
-}
+assert.Len(t, orgUsers, 2)
 
 orgUsers, err = organization.GetOrgUsersByOrgID(db.DefaultContext, &organization.FindOrgMembersOpts{
 ListOptions: db.ListOptions{},
 OrgID: unittest.NonexistentID,
-PublicOnly: false,
 })
 assert.NoError(t, err)
 assert.Len(t, orgUsers, 0)

models/organization/org_user_test.go

@@ -94,7 +94,7 @@ func TestUserListIsPublicMember(t *testing.T) {
 func testUserListIsPublicMember(t *testing.T, orgID int64, expected map[int64]bool) {
 org, err := organization.GetOrgByID(db.DefaultContext, orgID)
 assert.NoError(t, err)
-_, membersIsPublic, err := org.GetMembers(db.DefaultContext)
+_, membersIsPublic, err := org.GetMembers(db.DefaultContext, &user_model.User{IsAdmin: true})
 assert.NoError(t, err)
 assert.Equal(t, expected, membersIsPublic)
 }
@@ -121,7 +121,7 @@ func TestUserListIsUserOrgOwner(t *testing.T) {
 func testUserListIsUserOrgOwner(t *testing.T, orgID int64, expected map[int64]bool) {
 org, err := organization.GetOrgByID(db.DefaultContext, orgID)
 assert.NoError(t, err)
-members, _, err := org.GetMembers(db.DefaultContext)
+members, _, err := org.GetMembers(db.DefaultContext, &user_model.User{IsAdmin: true})
 assert.NoError(t, err)
 assert.Equal(t, expected, organization.IsUserOrgOwner(db.DefaultContext, members, orgID))
 }

routers/api/v1/org/member.go

@@ -18,10 +18,11 @@ import (
 )
 
 // listMembers list an organization's members
-func listMembers(ctx *context.APIContext, publicOnly bool) {
+func listMembers(ctx *context.APIContext, isMember bool) {
 opts := &organization.FindOrgMembersOpts{
+Doer: ctx.Doer,
+IsMember: isMember,
 OrgID: ctx.Org.Organization.ID,
-PublicOnly: publicOnly,
 ListOptions: utils.GetListOptions(ctx),
 }
 
@@ -73,16 +74,19 @@ func ListMembers(ctx *context.APIContext) {
 // "404":
 // "$ref": "#/responses/notFound"
 
-publicOnly := true
+var (
+isMember bool
+err error
+)
+
 if ctx.Doer != nil {
-isMember, err := ctx.Org.Organization.IsOrgMember(ctx, ctx.Doer.ID)
+isMember, err = ctx.Org.Organization.IsOrgMember(ctx, ctx.Doer.ID)
 if err != nil {
 ctx.Error(http.StatusInternalServerError, "IsOrgMember", err)
 return
 }
-publicOnly = !isMember && !ctx.Doer.IsAdmin
 }
-listMembers(ctx, publicOnly)
+listMembers(ctx, isMember)
 }
 
 // ListPublicMembers list an organization's public members
@@ -112,7 +116,7 @@ func ListPublicMembers(ctx *context.APIContext) {
 // "404":
 // "$ref": "#/responses/notFound"
 
-listMembers(ctx, true)
+listMembers(ctx, false)
 }
 
 // IsMember check if a user is a member of an organization

routers/web/org/home.go

@@ -95,10 +95,12 @@ func home(ctx *context.Context, viewRepositories bool) {
 }
 
 opts := &organization.FindOrgMembersOpts{
+Doer: ctx.Doer,
 OrgID: org.ID,
-PublicOnly: ctx.Org.PublicMemberOnly,
+IsMember:  ctx.Org.IsMember,
 ListOptions: db.ListOptions{Page: 1, PageSize: 25},
 }
+
 members, _, err := organization.FindOrgMembers(ctx, opts)
 if err != nil {
 ctx.ServerError("FindOrgMembers", err)

routers/web/org/members.go

@@ -34,8 +34,8 @@ func Members(ctx *context.Context) {
 }
 
 opts := &organization.FindOrgMembersOpts{
-OrgID:  org.ID,
-PublicOnly: true,
+Doer: ctx.Doer,
+OrgID: org.ID,
 }
 
 if ctx.Doer != nil {
@@ -44,9 +44,9 @@ func Members(ctx *context.Context) {
 ctx.Error(http.StatusInternalServerError, "IsOrgMember")
 return
 }
-opts.PublicOnly = !isMember && !ctx.Doer.IsAdmin
+opts.IsMember = isMember
 }
-ctx.Data["PublicOnly"] = opts.PublicOnly
+ctx.Data["PublicOnly"] = opts.PublicOnly()
 
 total, err := organization.CountOrgMembers(ctx, opts)
 if err != nil {

services/context/org.go

@@ -26,7 +26,6 @@ type Organization struct {
 Organization *organization.Organization
 OrgLink string
 CanCreateOrgRepo bool
-PublicMemberOnly bool // Only display public members
 
 Team *organization.Team
 Teams []*organization.Team
@@ -176,10 +175,10 @@ func HandleOrgAssignment(ctx *Context, args ...bool) {
 ctx.Data["OrgLink"] = ctx.Org.OrgLink
 
 // Member
-ctx.Org.PublicMemberOnly = ctx.Doer == nil || !ctx.Org.IsMember && !ctx.Doer.IsAdmin
 opts := &organization.FindOrgMembersOpts{
-OrgID: org.ID,
-PublicOnly: ctx.Org.PublicMemberOnly,
+Doer: ctx.Doer,
+OrgID: org.ID,
+IsMember: ctx.Org.IsMember,
 }
 ctx.Data["NumMembers"], err = organization.CountOrgMembers(ctx, opts)
 if err != nil {