aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
author6543 <6543@obermui.de>2024-11-11 01:38:30 +0100
committerGitHub <noreply@github.com>2024-11-11 01:38:30 +0100
commit43c252dfeaf9ab03c4db3e7ac5169bc0d69901ac (patch)
tree0fdac424af8a41ad804a2f6385666e48e7f12520
parentb1f42a0cdddc8db9eef87041d6bcb328b2ef35fc (diff)
downloadgitea-43c252dfeaf9ab03c4db3e7ac5169bc0d69901ac.tar.gz
gitea-43c252dfeaf9ab03c4db3e7ac5169bc0d69901ac.zip
Calculate `PublicOnly` for org membership only once (#32234)
Refactoring of #32211 this move the PublicOnly() filter calcuation next to the DB querys and let it be decided by the Doer --- *Sponsored by Kithara Software GmbH*
-rw-r--r--models/organization/org.go18
-rw-r--r--models/organization/org_test.go58
-rw-r--r--models/organization/org_user_test.go4
-rw-r--r--routers/api/v1/org/member.go22
-rw-r--r--routers/web/org/home.go8
-rw-r--r--routers/web/org/members.go8
-rw-r--r--services/context/org.go7
7 files changed, 72 insertions, 53 deletions
diff --git a/models/organization/org.go b/models/organization/org.go
index b33d15d29c..28a46ec8f5 100644
--- a/models/organization/org.go
+++ b/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
+ IsDoerMember bool
+ OrgID int64
+}
+
+func (opts FindOrgMembersOpts) PublicOnly() bool {
+ return opts.Doer == nil || !(opts.IsDoerMember || 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)
diff --git a/models/organization/org_test.go b/models/organization/org_test.go
index 23ef22e2fb..5442c37ccc 100644
--- a/models/organization/org_test.go
+++ b/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)
diff --git a/models/organization/org_user_test.go b/models/organization/org_user_test.go
index cf7acdf83b..55abb63203 100644
--- a/models/organization/org_user_test.go
+++ b/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))
}
diff --git a/routers/api/v1/org/member.go b/routers/api/v1/org/member.go
index 9db9ad964b..edcee1e207 100644
--- a/routers/api/v1/org/member.go
+++ b/routers/api/v1/org/member.go
@@ -18,11 +18,12 @@ import (
)
// listMembers list an organization's members
-func listMembers(ctx *context.APIContext, publicOnly bool) {
+func listMembers(ctx *context.APIContext, isMember bool) {
opts := &organization.FindOrgMembersOpts{
- OrgID: ctx.Org.Organization.ID,
- PublicOnly: publicOnly,
- ListOptions: utils.GetListOptions(ctx),
+ Doer: ctx.Doer,
+ IsDoerMember: isMember,
+ OrgID: ctx.Org.Organization.ID,
+ ListOptions: utils.GetListOptions(ctx),
}
count, err := organization.CountOrgMembers(ctx, opts)
@@ -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
diff --git a/routers/web/org/home.go b/routers/web/org/home.go
index 069bd549c1..544f5362b8 100644
--- a/routers/web/org/home.go
+++ b/routers/web/org/home.go
@@ -95,10 +95,12 @@ func home(ctx *context.Context, viewRepositories bool) {
}
opts := &organization.FindOrgMembersOpts{
- OrgID: org.ID,
- PublicOnly: ctx.Org.PublicMemberOnly,
- ListOptions: db.ListOptions{Page: 1, PageSize: 25},
+ Doer: ctx.Doer,
+ OrgID: org.ID,
+ IsDoerMember: ctx.Org.IsMember,
+ ListOptions: db.ListOptions{Page: 1, PageSize: 25},
}
+
members, _, err := organization.FindOrgMembers(ctx, opts)
if err != nil {
ctx.ServerError("FindOrgMembers", err)
diff --git a/routers/web/org/members.go b/routers/web/org/members.go
index 8ff75b0651..97dfff3afe 100644
--- a/routers/web/org/members.go
+++ b/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.IsDoerMember = isMember
}
- ctx.Data["PublicOnly"] = opts.PublicOnly
+ ctx.Data["PublicOnly"] = opts.PublicOnly()
total, err := organization.CountOrgMembers(ctx, opts)
if err != nil {
diff --git a/services/context/org.go b/services/context/org.go
index 7eba80ff96..e420629372 100644
--- a/services/context/org.go
+++ b/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,
+ IsDoerMember: ctx.Org.IsMember,
}
ctx.Data["NumMembers"], err = organization.CountOrgMembers(ctx, opts)
if err != nil {