Fix bug

Author: lunny

models/db/list_options.go

@@ -5,8 +5,6 @@
 package db
 
 import (
-	"context"
-
 	"code.gitea.io/gitea/modules/setting"
 
 	"xorm.io/xorm"
@@ -25,15 +23,6 @@ func GetPaginatedSession(p Paginator) *xorm.Session {
 	return x.Limit(take, skip)
 }
 
-// WithPaginator returns a Context from a context.Context and Paginator
-func WithPaginator(ctx context.Context, p Paginator) context.Context {
-	skip, take := p.GetSkipTake()
-	return &Context{
-		Context: ctx,
-		e:       GetEngine(ctx).Limit(take, skip),
-	}
-}
-
 // SetSessionPagination sets pagination for a database session
 func SetSessionPagination(sess Engine, p Paginator) *xorm.Session {
 	skip, take := p.GetSkipTake()

models/org_team.go

@@ -397,17 +397,17 @@ func UpdateTeam(t *organization.Team, authChanged, includeAllChanged bool) (err
 // DeleteTeam deletes given team.
 // It's caller's responsibility to assign organization ID.
 func DeleteTeam(t *organization.Team) error {
-	if err := t.GetRepositories(&organization.SearchTeamOptions{}); err != nil {
-		return err
-	}
-
 	ctx, committer, err := db.TxContext()
 	if err != nil {
 		return err
 	}
 	defer committer.Close()
 	sess := db.GetEngine(ctx)
 
+	if err := t.GetRepositoriesCtx(ctx); err != nil {
+		return err
+	}
+
 	if err := t.GetMembersCtx(ctx); err != nil {
 		return err
 	}
@@ -492,17 +492,17 @@ func AddTeamMember(team *organization.Team, userID int64) error {
 		return err
 	}
 
-	// Get team and its repositories.
-	if err := team.GetRepositories(&organization.SearchTeamOptions{}); err != nil {
-		return err
-	}
-
 	ctx, committer, err := db.TxContext()
 	if err != nil {
 		return err
 	}
 	defer committer.Close()
 
+	// Get team and its repositories.
+	if err := team.GetRepositoriesCtx(ctx); err != nil {
+		return err
+	}
+
 	sess := db.GetEngine(ctx)
 
 	if err := db.Insert(ctx, &organization.TeamUser{

models/organization/team.go

@@ -96,11 +96,6 @@ type SearchTeamOptions struct {
 	IncludeDesc bool
 }
 
-// SearchMembersOptions holds the search options
-type SearchMembersOptions struct {
-	db.ListOptions
-}
-
 // SearchTeam search for teams. Caller is responsible to check permissions.
 func SearchTeam(opts *SearchTeamOptions) ([]*Team, int64, error) {
 	if opts.Page <= 0 {
@@ -224,40 +219,24 @@ func (t *Team) IsMember(userID int64) bool {
 }
 
 // GetRepositoriesCtx returns paginated repositories in team of organization.
-func (t *Team) GetRepositoriesCtx(ctx context.Context) error {
+func (t *Team) GetRepositoriesCtx(ctx context.Context) (err error) {
 	if t.Repos != nil {
 		return nil
 	}
-	return db.GetEngine(ctx).Join("INNER", "team_repo", "repository.id = team_repo.repo_id").
-		Where("team_repo.team_id=?", t.ID).
-		OrderBy("repository.name").
-		Find(&t.Repos)
-}
-
-// GetRepositories returns paginated repositories in team of organization.
-func (t *Team) GetRepositories(opts *SearchTeamOptions) error {
-	if opts.Page == 0 {
-		return t.GetRepositoriesCtx(db.DefaultContext)
-	}
-
-	return t.GetRepositoriesCtx(db.WithPaginator(db.DefaultContext, opts))
+	t.Repos, err = GetTeamRepositories(ctx, &SearchTeamRepoOptions{
+		TeamID: t.ID,
+	})
+	return
 }
 
 // GetMembersCtx returns paginated members in team of organization.
 func (t *Team) GetMembersCtx(ctx context.Context) (err error) {
-	t.Members, err = GetTeamMembers(ctx, t.ID)
+	t.Members, err = GetTeamMembers(ctx, &SearchMembersOptions{
+		TeamID: t.ID,
+	})
 	return err
 }
 
-// GetMembers returns paginated members in team of organization.
-func (t *Team) GetMembers(opts *SearchMembersOptions) (err error) {
-	if opts.Page == 0 {
-		return t.GetMembersCtx(db.DefaultContext)
-	}
-
-	return t.GetMembersCtx(db.WithPaginator(db.DefaultContext, opts))
-}
-
 // UnitEnabled returns if the team has the given unit type enabled
 func (t *Team) UnitEnabled(tp unit.Type) bool {
 	return t.unitEnabled(db.DefaultContext, tp)

models/organization/team_repo.go

@@ -9,6 +9,7 @@ import (
 
 	"code.gitea.io/gitea/models/db"
 	"code.gitea.io/gitea/models/perm"
+	repo_model "code.gitea.io/gitea/models/repo"
 )
 
 // TeamRepo represents an team-repository relation.
@@ -29,6 +30,26 @@ func HasTeamRepo(ctx context.Context, orgID, teamID, repoID int64) bool {
 	return has
 }
 
+type SearchTeamRepoOptions struct {
+	db.ListOptions
+	TeamID int64
+}
+
+// GetRepositories returns paginated repositories in team of organization.
+func GetTeamRepositories(ctx context.Context, opts *SearchTeamRepoOptions) ([]*repo_model.Repository, error) {
+	sess := db.GetEngine(ctx)
+	if opts.TeamID > 0 {
+		sess = sess.Join("INNER", "team_repo", "repository.id = team_repo.repo_id").
+			Where("team_repo.team_id=?", opts.TeamID)
+	}
+	if opts.PageSize > 0 {
+		sess.Limit(opts.PageSize, opts.Page*opts.PageSize)
+	}
+	var repos []*repo_model.Repository
+	return repos, sess.OrderBy("repository.name").
+		Find(&repos)
+}
+
 // AddTeamRepo addes a repo for an organization's team
 func AddTeamRepo(ctx context.Context, orgID, teamID, repoID int64) error {
 	_, err := db.GetEngine(ctx).Insert(&TeamRepo{

models/organization/team_test.go

@@ -42,7 +42,7 @@ func TestTeam_GetRepositories(t *testing.T) {
 
 	test := func(teamID int64) {
 		team := unittest.AssertExistsAndLoadBean(t, &Team{ID: teamID}).(*Team)
-		assert.NoError(t, team.GetRepositories(&SearchTeamOptions{}))
+		assert.NoError(t, team.GetRepositoriesCtx(db.DefaultContext))
 		assert.Len(t, team.Repos, team.NumRepos)
 		for _, repo := range team.Repos {
 			unittest.AssertExistsAndLoadBean(t, &TeamRepo{TeamID: teamID, RepoID: repo.ID})
@@ -57,7 +57,7 @@ func TestTeam_GetMembers(t *testing.T) {
 
 	test := func(teamID int64) {
 		team := unittest.AssertExistsAndLoadBean(t, &Team{ID: teamID}).(*Team)
-		assert.NoError(t, team.GetMembers(&SearchMembersOptions{}))
+		assert.NoError(t, team.GetMembersCtx(db.DefaultContext))
 		assert.Len(t, team.Members, team.NumMembers)
 		for _, member := range team.Members {
 			unittest.AssertExistsAndLoadBean(t, &TeamUser{UID: member.ID, TeamID: teamID})
@@ -126,7 +126,9 @@ func TestGetTeamMembers(t *testing.T) {
 
 	test := func(teamID int64) {
 		team := unittest.AssertExistsAndLoadBean(t, &Team{ID: teamID}).(*Team)
-		members, err := GetTeamMembers(db.DefaultContext, teamID)
+		members, err := GetTeamMembers(db.DefaultContext, &SearchMembersOptions{
+			TeamID: teamID,
+		})
 		assert.NoError(t, err)
 		assert.Len(t, members, team.NumMembers)
 		for _, member := range members {

models/organization/team_user.go

@@ -6,11 +6,10 @@ package organization
 
 import (
 	"context"
-	"fmt"
-	"sort"
 
 	"code.gitea.io/gitea/models/db"
 	user_model "code.gitea.io/gitea/models/user"
+	"xorm.io/builder"
 )
 
 // TeamUser represents an team-user relation.
@@ -39,23 +38,33 @@ func GetTeamUsersByTeamID(ctx context.Context, teamID int64) ([]*TeamUser, error
 		Find(&teamUsers)
 }
 
+// SearchMembersOptions holds the search options
+type SearchMembersOptions struct {
+	db.ListOptions
+	TeamID int64
+}
+
+func (opts SearchMembersOptions) ToConds() builder.Cond {
+	cond := builder.NewCond()
+	if opts.TeamID > 0 {
+		cond = cond.And(builder.Eq{"team_user.team_id": opts.TeamID})
+	}
+	return cond
+}
+
 // GetTeamMembers returns all members in given team of organization.
-func GetTeamMembers(ctx context.Context, teamID int64) (_ []*user_model.User, err error) {
-	teamUsers, err := GetTeamUsersByTeamID(ctx, teamID)
-	if err != nil {
-		return nil, fmt.Errorf("GetTeamUsersByTeamID: %v", err)
+func GetTeamMembers(ctx context.Context, opts *SearchMembersOptions) ([]*user_model.User, error) {
+	var members []*user_model.User
+	sess := db.GetEngine(ctx).
+		Join("INNER", "team_user", "team_user.uid=user.id").
+		Asc("full_name + name").
+		Where(opts)
+	if opts.PageSize > 0 && opts.Page > -1 {
+		sess = sess.Limit(opts.PageSize, opts.Page*opts.PageSize)
 	}
-	members := make([]*user_model.User, len(teamUsers))
-	for i, teamUser := range teamUsers {
-		member, err := user_model.GetUserByIDCtx(ctx, teamUser.UID)
-		if err != nil {
-			return nil, fmt.Errorf("GetUserByIDCtx[id: %d]: %v", teamUser.UID, err)
-		}
-		members[i] = member
+	if err := sess.Find(&members); err != nil {
+		return nil, err
 	}
-	sort.Slice(members, func(i, j int) bool {
-		return members[i].DisplayName() < members[j].DisplayName()
-	})
 	return members, nil
 }
 

modules/repository/create_test.go

@@ -24,7 +24,7 @@ func TestIncludesAllRepositoriesTeams(t *testing.T) {
 
 	testTeamRepositories := func(teamID int64, repoIds []int64) {
 		team := unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: teamID}).(*organization.Team)
-		assert.NoError(t, team.GetRepositories(&organization.SearchTeamOptions{}), "%s: GetRepositories", team.Name)
+		assert.NoError(t, team.GetRepositoriesCtx(db.DefaultContext), "%s: GetRepositories", team.Name)
 		assert.Len(t, team.Repos, team.NumRepos, "%s: len repo", team.Name)
 		assert.Len(t, team.Repos, len(repoIds), "%s: repo count", team.Name)
 		for i, rid := range repoIds {

routers/api/v1/org/team.go

@@ -378,14 +378,17 @@ func GetTeamMembers(ctx *context.APIContext) {
 		return
 	}
 
-	if err := ctx.Org.Team.GetMembers(&organization.SearchMembersOptions{
+	teamMembers, err := organization.GetTeamMembers(ctx, &organization.SearchMembersOptions{
 		ListOptions: utils.GetListOptions(ctx),
-	}); err != nil {
+		TeamID:      ctx.Org.Team.ID,
+	})
+	if err != nil {
 		ctx.Error(http.StatusInternalServerError, "GetTeamMembers", err)
 		return
 	}
+
 	members := make([]*api.User, len(ctx.Org.Team.Members))
-	for i, member := range ctx.Org.Team.Members {
+	for i, member := range teamMembers {
 		members[i] = convert.ToUser(member, ctx.Doer)
 	}
 
@@ -534,13 +537,16 @@ func GetTeamRepos(ctx *context.APIContext) {
 	//     "$ref": "#/responses/RepositoryList"
 
 	team := ctx.Org.Team
-	if err := team.GetRepositories(&organization.SearchTeamOptions{
+	teamRepos, err := organization.GetTeamRepositories(ctx, &organization.SearchTeamRepoOptions{
 		ListOptions: utils.GetListOptions(ctx),
-	}); err != nil {
+		TeamID:      team.ID,
+	})
+	if err != nil {
 		ctx.Error(http.StatusInternalServerError, "GetTeamRepos", err)
+		return
 	}
 	repos := make([]*api.Repository, len(team.Repos))
-	for i, repo := range team.Repos {
+	for i, repo := range teamRepos {
 		access, err := models.AccessLevel(ctx.Doer, repo)
 		if err != nil {
 			ctx.Error(http.StatusInternalServerError, "GetTeamRepos", err)

routers/web/org/teams.go

@@ -44,7 +44,7 @@ func Teams(ctx *context.Context) {
 	ctx.Data["PageIsOrgTeams"] = true
 
 	for _, t := range ctx.Org.Teams {
-		if err := t.GetMembers(&organization.SearchMembersOptions{}); err != nil {
+		if err := t.GetMembersCtx(ctx); err != nil {
 			ctx.ServerError("GetMembers", err)
 			return
 		}
@@ -308,7 +308,7 @@ func TeamMembers(ctx *context.Context) {
 	ctx.Data["Title"] = ctx.Org.Team.Name
 	ctx.Data["PageIsOrgTeams"] = true
 	ctx.Data["PageIsOrgTeamMembers"] = true
-	if err := ctx.Org.Team.GetMembers(&organization.SearchMembersOptions{}); err != nil {
+	if err := ctx.Org.Team.GetMembersCtx(ctx); err != nil {
 		ctx.ServerError("GetMembers", err)
 		return
 	}
@@ -321,7 +321,7 @@ func TeamRepositories(ctx *context.Context) {
 	ctx.Data["Title"] = ctx.Org.Team.Name
 	ctx.Data["PageIsOrgTeams"] = true
 	ctx.Data["PageIsOrgTeamRepos"] = true
-	if err := ctx.Org.Team.GetRepositories(&organization.SearchTeamOptions{}); err != nil {
+	if err := ctx.Org.Team.GetRepositoriesCtx(ctx); err != nil {
 		ctx.ServerError("GetRepositories", err)
 		return
 	}

services/issue/assignee.go

@@ -243,11 +243,14 @@ func TeamReviewRequest(issue *models.Issue, doer *user_model.User, reviewer *org
 		return
 	}
 
-	if err = reviewer.GetMembers(&organization.SearchMembersOptions{}); err != nil {
+	members, err := organization.GetTeamMembers(db.DefaultContext, &organization.SearchMembersOptions{
+		TeamID: reviewer.ID,
+	})
+	if err != nil {
 		return
 	}
 
-	for _, member := range reviewer.Members {
+	for _, member := range members {
 		if member.ID == comment.Issue.PosterID {
 			continue
 		}