Skip to content

Refactor FindOrgOptions to use enum instead of bool, fix membership visibility #34629

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 9, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion models/activities/statistic.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"code.gitea.io/gitea/models/webhook"
"code.gitea.io/gitea/modules/optional"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/structs"
)

// Statistic contains the database statistics
Expand Down Expand Up @@ -68,7 +69,7 @@ func GetStatistic(ctx context.Context) (stats Statistic) {
}
stats.Counter.UsersNotActive = user_model.CountUsers(ctx, &usersNotActiveOpts)

stats.Counter.Org, _ = db.Count[organization.Organization](ctx, organization.FindOrgOptions{IncludePrivate: true})
stats.Counter.Org, _ = db.Count[organization.Organization](ctx, organization.FindOrgOptions{IncludeVisibility: structs.VisibleTypePrivate})
stats.Counter.PublicKey, _ = e.Count(new(asymkey_model.PublicKey))
stats.Counter.Repo, _ = repo_model.CountRepositories(ctx, repo_model.CountRepositoryOptions{})
stats.Counter.Watch, _ = e.Count(new(repo_model.Watch))
Expand Down
21 changes: 15 additions & 6 deletions models/organization/org_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ type SearchOrganizationsOptions struct {
// FindOrgOptions finds orgs options
type FindOrgOptions struct {
db.ListOptions
UserID int64
IncludePrivate bool
UserID int64
IncludeVisibility structs.VisibleType
}

func queryUserOrgIDs(userID int64, includePrivate bool) *builder.Builder {
Expand All @@ -65,18 +65,27 @@ func queryUserOrgIDs(userID int64, includePrivate bool) *builder.Builder {
func (opts FindOrgOptions) ToConds() builder.Cond {
var cond builder.Cond = builder.Eq{"`user`.`type`": user_model.UserTypeOrganization}
if opts.UserID > 0 {
cond = cond.And(builder.In("`user`.`id`", queryUserOrgIDs(opts.UserID, opts.IncludePrivate)))
}
if !opts.IncludePrivate {
cond = cond.And(builder.Eq{"`user`.visibility": structs.VisibleTypePublic})
cond = cond.And(builder.In("`user`.`id`", queryUserOrgIDs(opts.UserID, opts.IncludeVisibility == structs.VisibleTypePrivate)))
}
// public=0, limited=1, private=2
cond = cond.And(builder.Lte{"`user`.visibility": opts.IncludeVisibility})
return cond
}

func (opts FindOrgOptions) ToOrders() string {
return "`user`.lower_name ASC"
}

func DoerViewOtherVisibility(doer, other *user_model.User) structs.VisibleType {
if doer == nil || other == nil {
return structs.VisibleTypePublic
}
if doer.IsAdmin || doer.ID == other.ID {
return structs.VisibleTypePrivate
}
return structs.VisibleTypeLimited
}

// GetOrgsCanCreateRepoByUserID returns a list of organizations where given user ID
// are allowed to create repos.
func GetOrgsCanCreateRepoByUserID(ctx context.Context, userID int64) ([]*Organization, error) {
Expand Down
41 changes: 26 additions & 15 deletions models/organization/org_list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,48 +10,53 @@ import (
"code.gitea.io/gitea/models/organization"
"code.gitea.io/gitea/models/unittest"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/structs"

"github.com/stretchr/testify/assert"
)

func TestCountOrganizations(t *testing.T) {
func TestOrgList(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
t.Run("CountOrganizations", testCountOrganizations)
t.Run("FindOrgs", testFindOrgs)
t.Run("GetUserOrgsList", testGetUserOrgsList)
t.Run("LoadOrgListTeams", testLoadOrgListTeams)
t.Run("DoerViewOtherVisibility", testDoerViewOtherVisibility)
}

func testCountOrganizations(t *testing.T) {
expected, err := db.GetEngine(db.DefaultContext).Where("type=?", user_model.UserTypeOrganization).Count(&organization.Organization{})
assert.NoError(t, err)
cnt, err := db.Count[organization.Organization](db.DefaultContext, organization.FindOrgOptions{IncludePrivate: true})
cnt, err := db.Count[organization.Organization](db.DefaultContext, organization.FindOrgOptions{IncludeVisibility: structs.VisibleTypePrivate})
assert.NoError(t, err)
assert.Equal(t, expected, cnt)
}

func TestFindOrgs(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())

func testFindOrgs(t *testing.T) {
orgs, err := db.Find[organization.Organization](db.DefaultContext, organization.FindOrgOptions{
UserID: 4,
IncludePrivate: true,
UserID: 4,
IncludeVisibility: structs.VisibleTypePrivate,
})
assert.NoError(t, err)
if assert.Len(t, orgs, 1) {
assert.EqualValues(t, 3, orgs[0].ID)
}

orgs, err = db.Find[organization.Organization](db.DefaultContext, organization.FindOrgOptions{
UserID: 4,
IncludePrivate: false,
UserID: 4,
})
assert.NoError(t, err)
assert.Empty(t, orgs)

total, err := db.Count[organization.Organization](db.DefaultContext, organization.FindOrgOptions{
UserID: 4,
IncludePrivate: true,
UserID: 4,
IncludeVisibility: structs.VisibleTypePrivate,
})
assert.NoError(t, err)
assert.EqualValues(t, 1, total)
}

func TestGetUserOrgsList(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
func testGetUserOrgsList(t *testing.T) {
orgs, err := organization.GetUserOrgsList(db.DefaultContext, &user_model.User{ID: 4})
assert.NoError(t, err)
if assert.Len(t, orgs, 1) {
Expand All @@ -61,8 +66,7 @@ func TestGetUserOrgsList(t *testing.T) {
}
}

func TestLoadOrgListTeams(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
func testLoadOrgListTeams(t *testing.T) {
orgs, err := organization.GetUserOrgsList(db.DefaultContext, &user_model.User{ID: 4})
assert.NoError(t, err)
assert.Len(t, orgs, 1)
Expand All @@ -71,3 +75,10 @@ func TestLoadOrgListTeams(t *testing.T) {
assert.Len(t, teamsMap, 1)
assert.Len(t, teamsMap[3], 5)
}

func testDoerViewOtherVisibility(t *testing.T) {
assert.Equal(t, structs.VisibleTypePublic, organization.DoerViewOtherVisibility(nil, nil))
assert.Equal(t, structs.VisibleTypeLimited, organization.DoerViewOtherVisibility(&user_model.User{ID: 1}, &user_model.User{ID: 2}))
assert.Equal(t, structs.VisibleTypePrivate, organization.DoerViewOtherVisibility(&user_model.User{ID: 1}, &user_model.User{ID: 1}))
assert.Equal(t, structs.VisibleTypePrivate, organization.DoerViewOtherVisibility(&user_model.User{ID: 1, IsAdmin: true}, &user_model.User{ID: 2}))
}
8 changes: 3 additions & 5 deletions routers/api/v1/org/org.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,10 @@ import (

func listUserOrgs(ctx *context.APIContext, u *user_model.User) {
listOptions := utils.GetListOptions(ctx)
showPrivate := ctx.IsSigned && (ctx.Doer.IsAdmin || ctx.Doer.ID == u.ID)

opts := organization.FindOrgOptions{
ListOptions: listOptions,
UserID: u.ID,
IncludePrivate: showPrivate,
ListOptions: listOptions,
UserID: u.ID,
IncludeVisibility: organization.DoerViewOtherVisibility(ctx.Doer, u),
}
orgs, maxResults, err := db.FindAndCount[organization.Organization](ctx, opts)
if err != nil {
Expand Down
7 changes: 4 additions & 3 deletions routers/web/admin/users.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/optional"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/templates"
"code.gitea.io/gitea/modules/web"
"code.gitea.io/gitea/routers/web/explore"
Expand Down Expand Up @@ -292,9 +293,9 @@ func ViewUser(ctx *context.Context) {
ctx.Data["EmailsTotal"] = len(emails)

orgs, err := db.Find[org_model.Organization](ctx, org_model.FindOrgOptions{
ListOptions: db.ListOptionsAll,
UserID: u.ID,
IncludePrivate: true,
ListOptions: db.ListOptionsAll,
UserID: u.ID,
IncludeVisibility: structs.VisibleTypePrivate,
})
if err != nil {
ctx.ServerError("FindOrgs", err)
Expand Down
7 changes: 3 additions & 4 deletions routers/web/shared/user/header.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,12 @@ func prepareContextForProfileBigAvatar(ctx *context.Context) {
ctx.Data["RenderedDescription"] = content
}

showPrivate := ctx.IsSigned && (ctx.Doer.IsAdmin || ctx.Doer.ID == ctx.ContextUser.ID)
orgs, err := db.Find[organization.Organization](ctx, organization.FindOrgOptions{
UserID: ctx.ContextUser.ID,
IncludePrivate: showPrivate,
UserID: ctx.ContextUser.ID,
IncludeVisibility: organization.DoerViewOtherVisibility(ctx.Doer, ctx.ContextUser),
ListOptions: db.ListOptions{
Page: 1,
// query one more results (without a separate counting) to see whether we need to add the "show more orgs" link
// query one more result (without a separate counting) to see whether we need to add the "show more orgs" link
PageSize: setting.UI.User.OrgPagingNum + 1,
},
})
Expand Down
10 changes: 5 additions & 5 deletions routers/web/user/profile.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,7 @@ func userProfile(ctx *context.Context) {

profileDbRepo, profileReadmeBlob := shared_user.FindOwnerProfileReadme(ctx, ctx.Doer)

showPrivate := ctx.IsSigned && (ctx.Doer.IsAdmin || ctx.Doer.ID == ctx.ContextUser.ID)
prepareUserProfileTabData(ctx, showPrivate, profileDbRepo, profileReadmeBlob)
prepareUserProfileTabData(ctx, profileDbRepo, profileReadmeBlob)

// prepare the user nav header data after "prepareUserProfileTabData" to avoid re-querying the NumFollowers & NumFollowing
// because ctx.Data["NumFollowers"] and "NumFollowing" logic duplicates in both of them
Expand All @@ -90,7 +89,7 @@ func userProfile(ctx *context.Context) {
ctx.HTML(http.StatusOK, tplProfile)
}

func prepareUserProfileTabData(ctx *context.Context, showPrivate bool, profileDbRepo *repo_model.Repository, profileReadme *git.Blob) {
func prepareUserProfileTabData(ctx *context.Context, profileDbRepo *repo_model.Repository, profileReadme *git.Blob) {
// if there is a profile readme, default to "overview" page, otherwise, default to "repositories" page
// if there is not a profile readme, the overview tab should be treated as the repositories tab
tab := ctx.FormString("tab")
Expand Down Expand Up @@ -175,6 +174,7 @@ func prepareUserProfileTabData(ctx *context.Context, showPrivate bool, profileDb
case "activity":
date := ctx.FormString("date")
pagingNum = setting.UI.FeedPagingNum
showPrivate := ctx.IsSigned && (ctx.Doer.IsAdmin || ctx.Doer.ID == ctx.ContextUser.ID)
items, count, err := feed_service.GetFeeds(ctx, activities_model.GetFeedsOptions{
RequestedUser: ctx.ContextUser,
Actor: ctx.Doer,
Expand Down Expand Up @@ -265,8 +265,8 @@ func prepareUserProfileTabData(ctx *context.Context, showPrivate bool, profileDb
}
case "organizations":
orgs, count, err := db.FindAndCount[organization.Organization](ctx, organization.FindOrgOptions{
UserID: ctx.ContextUser.ID,
IncludePrivate: showPrivate,
UserID: ctx.ContextUser.ID,
IncludeVisibility: organization.DoerViewOtherVisibility(ctx.Doer, ctx.ContextUser),
ListOptions: db.ListOptions{
Page: page,
PageSize: pagingNum,
Expand Down
5 changes: 3 additions & 2 deletions routers/web/user/setting/profile.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/optional"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/templates"
"code.gitea.io/gitea/modules/translation"
"code.gitea.io/gitea/modules/typesniffer"
Expand Down Expand Up @@ -206,8 +207,8 @@ func Organization(ctx *context.Context) {
PageSize: setting.UI.Admin.UserPagingNum,
Page: ctx.FormInt("page"),
},
UserID: ctx.Doer.ID,
IncludePrivate: ctx.IsSigned,
UserID: ctx.Doer.ID,
IncludeVisibility: structs.VisibleTypePrivate,
}

if opts.Page <= 0 {
Expand Down
9 changes: 5 additions & 4 deletions services/oauth2_provider/access_token.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ import (
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/setting"
api "code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/timeutil"
"code.gitea.io/gitea/modules/util"

"github.com/golang-jwt/jwt/v5"
)
Expand Down Expand Up @@ -231,12 +233,11 @@ func NewAccessTokenResponse(ctx context.Context, grant *auth.OAuth2Grant, server
}, nil
}

// returns a list of "org" and "org:team" strings,
// that the given user is a part of.
// GetOAuthGroupsForUser returns a list of "org" and "org:team" strings, that the given user is a part of.
func GetOAuthGroupsForUser(ctx context.Context, user *user_model.User, onlyPublicGroups bool) ([]string, error) {
orgs, err := db.Find[org_model.Organization](ctx, org_model.FindOrgOptions{
UserID: user.ID,
IncludePrivate: !onlyPublicGroups,
UserID: user.ID,
IncludeVisibility: util.Iif(onlyPublicGroups, api.VisibleTypePublic, api.VisibleTypePrivate),
})
if err != nil {
return nil, fmt.Errorf("GetUserOrgList: %w", err)
Expand Down
5 changes: 3 additions & 2 deletions services/user/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/storage"
"code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/util"
"code.gitea.io/gitea/services/agit"
asymkey_service "code.gitea.io/gitea/services/asymkey"
Expand Down Expand Up @@ -177,8 +178,8 @@ func DeleteUser(ctx context.Context, u *user_model.User, purge bool) error {
PageSize: repo_model.RepositoryListDefaultPageSize,
Page: 1,
},
UserID: u.ID,
IncludePrivate: true,
UserID: u.ID,
IncludeVisibility: structs.VisibleTypePrivate,
})
if err != nil {
return fmt.Errorf("unable to find org list for %s[%d]. Error: %w", u.Name, u.ID, err)
Expand Down
5 changes: 3 additions & 2 deletions tests/integration/auth_ldap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"code.gitea.io/gitea/models/unittest"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/optional"
"code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/test"
"code.gitea.io/gitea/modules/translation"
"code.gitea.io/gitea/modules/util"
Expand Down Expand Up @@ -437,8 +438,8 @@ func TestLDAPGroupTeamSyncAddMember(t *testing.T) {
Name: gitLDAPUser.UserName,
})
usersOrgs, err := db.Find[organization.Organization](db.DefaultContext, organization.FindOrgOptions{
UserID: user.ID,
IncludePrivate: true,
UserID: user.ID,
IncludeVisibility: structs.VisibleTypePrivate,
})
assert.NoError(t, err)
allOrgTeams, err := organization.GetUserOrgTeams(db.DefaultContext, org.ID, user.ID)
Expand Down
4 changes: 2 additions & 2 deletions tests/integration/org_count_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,8 @@ func doCheckOrgCounts(username string, orgCounts map[string]int, strict bool, ca
})

orgs, err := db.Find[organization.Organization](db.DefaultContext, organization.FindOrgOptions{
UserID: user.ID,
IncludePrivate: true,
UserID: user.ID,
IncludeVisibility: api.VisibleTypePrivate,
})
assert.NoError(t, err)

Expand Down