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*

(cherry picked from commit 43c252dfeaf9ab03c4db3e7ac5169bc0d69901ac)

Conflicts:
	models/organization/org_test.go
	models/organization/org_user_test.go
	routers/web/org/home.go

  rather simple conflict resolution but not trivial
  tests/integration/user_count_test.go had to be adapted (simple)
  because it does not exist in Gitea and uses the modified model
This commit is contained in:
6543 2024-11-11 01:38:30 +01:00 committed by Earl Warren
parent 45435a8789
commit 7751bb64cb
No known key found for this signature in database
GPG key ID: 0579CB2928A78A00
8 changed files with 76 additions and 56 deletions

View file

@ -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))
}
@ -524,9 +531,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)

View file

@ -4,6 +4,7 @@
package organization_test
import (
"sort"
"testing"
"code.gitea.io/gitea/models/db"
@ -104,7 +105,7 @@ func TestUser_GetTeams(t *testing.T) {
func TestUser_GetMembers(t *testing.T) {
require.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})
require.NoError(t, err)
if assert.Len(t, members, 3) {
assert.Equal(t, int64(2), members[0].ID)
@ -211,37 +212,42 @@ func TestFindOrgs(t *testing.T) {
func TestGetOrgUsersByOrgID(t *testing.T) {
require.NoError(t, unittest.PrepareTestDatabase())
orgUsers, err := organization.GetOrgUsersByOrgID(db.DefaultContext, &organization.FindOrgMembersOpts{
ListOptions: db.ListOptions{},
OrgID: 3,
PublicOnly: false,
})
require.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])
opts := &organization.FindOrgMembersOpts{
Doer: &user_model.User{IsAdmin: true},
OrgID: 3,
}
assert.False(t, opts.PublicOnly())
orgUsers, err := organization.GetOrgUsersByOrgID(db.DefaultContext, opts)
require.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)
require.NoError(t, err)
assert.Len(t, orgUsers, 2)
orgUsers, err = organization.GetOrgUsersByOrgID(db.DefaultContext, &organization.FindOrgMembersOpts{
ListOptions: db.ListOptions{},
OrgID: unittest.NonexistentID,
PublicOnly: false,
})
require.NoError(t, err)
assert.Empty(t, orgUsers)

View file

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