mirror of
https://github.com/go-gitea/gitea
synced 2024-11-21 21:11:33 +01:00
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*
This commit is contained in:
parent
b1f42a0cdd
commit
43c252dfea
7 changed files with 73 additions and 54 deletions
|
@ -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
|
||||
Doer *user_model.User
|
||||
IsDoerMember bool
|
||||
OrgID int64
|
||||
PublicOnly bool
|
||||
}
|
||||
|
||||
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)
|
||||
|
||||
|
|
|
@ -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{},
|
||||
opts := &organization.FindOrgMembersOpts{
|
||||
Doer: &user_model.User{IsAdmin: true},
|
||||
OrgID: 3,
|
||||
PublicOnly: false,
|
||||
})
|
||||
}
|
||||
assert.False(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,
|
||||
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,
|
||||
}, *orgUsers[0])
|
||||
assert.Equal(t, organization.OrgUser{
|
||||
ID: orgUsers[1].ID,
|
||||
}, {
|
||||
ID: 2,
|
||||
OrgID: 3,
|
||||
UID: 4,
|
||||
IsPublic: false,
|
||||
}, *orgUsers[1])
|
||||
assert.Equal(t, organization.OrgUser{
|
||||
ID: orgUsers[2].ID,
|
||||
}, {
|
||||
ID: 9,
|
||||
OrgID: 3,
|
||||
UID: 28,
|
||||
IsPublic: true,
|
||||
}, *orgUsers[2])
|
||||
}
|
||||
}}, orgUsers)
|
||||
|
||||
opts = &organization.FindOrgMembersOpts{OrgID: 3}
|
||||
assert.True(t, opts.PublicOnly())
|
||||
orgUsers, err = organization.GetOrgUsersByOrgID(db.DefaultContext, opts)
|
||||
assert.NoError(t, err)
|
||||
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)
|
||||
|
|
|
@ -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))
|
||||
}
|
||||
|
|
|
@ -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,
|
||||
IsDoerMember: 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
|
||||
|
|
|
@ -95,10 +95,12 @@ func home(ctx *context.Context, viewRepositories bool) {
|
|||
}
|
||||
|
||||
opts := &organization.FindOrgMembersOpts{
|
||||
Doer: ctx.Doer,
|
||||
OrgID: org.ID,
|
||||
PublicOnly: ctx.Org.PublicMemberOnly,
|
||||
IsDoerMember: ctx.Org.IsMember,
|
||||
ListOptions: db.ListOptions{Page: 1, PageSize: 25},
|
||||
}
|
||||
|
||||
members, _, err := organization.FindOrgMembers(ctx, opts)
|
||||
if err != nil {
|
||||
ctx.ServerError("FindOrgMembers", err)
|
||||
|
|
|
@ -34,8 +34,8 @@ func Members(ctx *context.Context) {
|
|||
}
|
||||
|
||||
opts := &organization.FindOrgMembersOpts{
|
||||
Doer: ctx.Doer,
|
||||
OrgID: org.ID,
|
||||
PublicOnly: true,
|
||||
}
|
||||
|
||||
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 {
|
||||
|
|
|
@ -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{
|
||||
Doer: ctx.Doer,
|
||||
OrgID: org.ID,
|
||||
PublicOnly: ctx.Org.PublicMemberOnly,
|
||||
IsDoerMember: ctx.Org.IsMember,
|
||||
}
|
||||
ctx.Data["NumMembers"], err = organization.CountOrgMembers(ctx, opts)
|
||||
if err != nil {
|
||||
|
|
Loading…
Reference in a new issue