From df5a42d1618a747aadd156d2693347990c3d9f98 Mon Sep 17 00:00:00 2001 From: Gusted Date: Mon, 31 Jan 2022 21:04:27 +0100 Subject: [PATCH 1/4] COrrect use `UserID` in `SearchTeams` - Use `UserID` in the `SearchTeams` function, currently it was useless to pass such information. Now it does a INNER statement to `team_user` which obtains UserID -> TeamID data. - Make OrgID optional. - Resolves #18484 --- models/org_team.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/models/org_team.go b/models/org_team.go index bce4afb0611b1..4897ec70d4522 100644 --- a/models/org_team.go +++ b/models/org_team.go @@ -84,10 +84,17 @@ func SearchTeam(opts *SearchTeamOptions) ([]*Team, int64, error) { cond = cond.And(keywordCond) } - cond = cond.And(builder.Eq{"org_id": opts.OrgID}) + if opts.OrgID > 0 { + cond = cond.And(builder.Eq{"org_id": opts.OrgID}) + } sess := db.GetEngine(db.DefaultContext) + if opts.UserID > 0 { + sess = sess.Join("INNER", "team_user", "team_user.team_id = team.id"). + And("team_user.uid=?", opts.UserID) + } + count, err := sess. Where(cond). Count(new(Team)) From c55975d9ab8cf11a3e530d2a6378dd21bb4290a7 Mon Sep 17 00:00:00 2001 From: Gusted Date: Mon, 31 Jan 2022 22:40:26 +0100 Subject: [PATCH 2/4] Seperate searching specific user --- models/org_team.go | 72 +++++++++++++++++++++++-------- models/org_team_test.go | 4 +- modules/repository/create_test.go | 2 +- routers/api/v1/org/team.go | 11 +++-- 4 files changed, 62 insertions(+), 27 deletions(-) diff --git a/models/org_team.go b/models/org_team.go index 4897ec70d4522..c0ee38195c4f7 100644 --- a/models/org_team.go +++ b/models/org_team.go @@ -49,22 +49,67 @@ func init() { db.RegisterModel(new(TeamUnit)) } -// SearchTeamOptions holds the search options -type SearchTeamOptions struct { +// SearchOrgTeamOptions holds the search options +type SearchOrgTeamOptions struct { db.ListOptions - UserID int64 Keyword string OrgID int64 IncludeDesc bool } +// GetUserTeamOptions holds the search options. +type GetUserTeamOptions struct { + db.ListOptions + UserID int64 +} + // 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) { +// GetUserTeams search for org teams. Caller is responsible to check permissions. +func GetUserTeams(opts *GetUserTeamOptions) ([]*Team, int64, error) { + if opts.Page <= 0 { + opts.Page = 1 + } + if opts.PageSize == 0 { + // Default limit + opts.PageSize = 10 + } + + sess := db.GetEngine(db.DefaultContext) + + sess = sess.Join("INNER", "team_user", "team_user.team_id = team.id"). + And("team_user.uid=?", opts.UserID) + + count, err := sess. + Count(new(Team)) + if err != nil { + return nil, 0, err + } + + if opts.PageSize == -1 { + opts.PageSize = int(count) + } else { + sess = sess.Limit(opts.PageSize, (opts.Page-1)*opts.PageSize) + } + + sess = sess.Join("INNER", "team_user", "team_user.team_id = team.id"). + And("team_user.uid=?", opts.UserID) + + teams := make([]*Team, 0, opts.PageSize) + if err = sess. + OrderBy("lower_name"). + Find(&teams); err != nil { + return nil, 0, err + } + + return teams, count, nil +} + +// SearchOrgTeams search for org teams. Caller is responsible to check permissions. +func SearchOrgTeams(opts *SearchOrgTeamOptions) ([]*Team, int64, error) { if opts.Page <= 0 { opts.Page = 1 } @@ -84,25 +129,16 @@ func SearchTeam(opts *SearchTeamOptions) ([]*Team, int64, error) { cond = cond.And(keywordCond) } - if opts.OrgID > 0 { - cond = cond.And(builder.Eq{"org_id": opts.OrgID}) - } + cond = cond.And(builder.Eq{"org_id": opts.OrgID}) sess := db.GetEngine(db.DefaultContext) - if opts.UserID > 0 { - sess = sess.Join("INNER", "team_user", "team_user.team_id = team.id"). - And("team_user.uid=?", opts.UserID) - } - count, err := sess. - Where(cond). Count(new(Team)) if err != nil { return nil, 0, err } - sess = sess.Where(cond) if opts.PageSize == -1 { opts.PageSize = int(count) } else { @@ -203,7 +239,7 @@ func (t *Team) getRepositories(e db.Engine) error { } // GetRepositories returns paginated repositories in team of organization. -func (t *Team) GetRepositories(opts *SearchTeamOptions) error { +func (t *Team) GetRepositories(opts *SearchOrgTeamOptions) error { if opts.Page == 0 { return t.getRepositories(db.GetEngine(db.DefaultContext)) } @@ -723,7 +759,7 @@ func UpdateTeam(t *Team, authChanged, includeAllChanged bool) (err error) { // DeleteTeam deletes given team. // It's caller's responsibility to assign organization ID. func DeleteTeam(t *Team) error { - if err := t.GetRepositories(&SearchTeamOptions{}); err != nil { + if err := t.GetRepositories(&SearchOrgTeamOptions{}); err != nil { return err } @@ -865,7 +901,7 @@ func AddTeamMember(team *Team, userID int64) error { } // Get team and its repositories. - if err := team.GetRepositories(&SearchTeamOptions{}); err != nil { + if err := team.GetRepositories(&SearchOrgTeamOptions{}); err != nil { return err } diff --git a/models/org_team_test.go b/models/org_team_test.go index aa62cc58e2d00..cf3a7979911d7 100644 --- a/models/org_team_test.go +++ b/models/org_team_test.go @@ -46,7 +46,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.GetRepositories(&SearchOrgTeamOptions{})) assert.Len(t, team.Repos, team.NumRepos) for _, repo := range team.Repos { unittest.AssertExistsAndLoadBean(t, &TeamRepo{TeamID: teamID, RepoID: repo.ID}) @@ -292,7 +292,7 @@ func TestGetTeamMembers(t *testing.T) { func TestGetUserTeams(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) test := func(userID int64) { - teams, _, err := SearchTeam(&SearchTeamOptions{UserID: userID}) + teams, _, err := GetUserTeams(&GetUserTeamOptions{UserID: userID}) assert.NoError(t, err) for _, team := range teams { unittest.AssertExistsAndLoadBean(t, &TeamUser{TeamID: team.ID, UID: userID}) diff --git a/modules/repository/create_test.go b/modules/repository/create_test.go index ed890ace43319..4e232a8609e8d 100644 --- a/modules/repository/create_test.go +++ b/modules/repository/create_test.go @@ -23,7 +23,7 @@ func TestIncludesAllRepositoriesTeams(t *testing.T) { testTeamRepositories := func(teamID int64, repoIds []int64) { team := unittest.AssertExistsAndLoadBean(t, &models.Team{ID: teamID}).(*models.Team) - assert.NoError(t, team.GetRepositories(&models.SearchTeamOptions{}), "%s: GetRepositories", team.Name) + assert.NoError(t, team.GetRepositories(&models.SearchOrgTeamOptions{}), "%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 { diff --git a/routers/api/v1/org/team.go b/routers/api/v1/org/team.go index cc7a63af337ec..62e6c0a6b4f3c 100644 --- a/routers/api/v1/org/team.go +++ b/routers/api/v1/org/team.go @@ -47,7 +47,7 @@ func ListTeams(ctx *context.APIContext) { // "200": // "$ref": "#/responses/TeamList" - teams, count, err := models.SearchTeam(&models.SearchTeamOptions{ + teams, count, err := models.SearchOrgTeams(&models.SearchOrgTeamOptions{ ListOptions: utils.GetListOptions(ctx), OrgID: ctx.Org.Organization.ID, }) @@ -90,7 +90,7 @@ func ListUserTeams(ctx *context.APIContext) { // "200": // "$ref": "#/responses/TeamList" - teams, count, err := models.SearchTeam(&models.SearchTeamOptions{ + teams, count, err := models.GetUserTeams(&models.GetUserTeamOptions{ ListOptions: utils.GetListOptions(ctx), UserID: ctx.User.ID, }) @@ -533,7 +533,7 @@ func GetTeamRepos(ctx *context.APIContext) { // "$ref": "#/responses/RepositoryList" team := ctx.Org.Team - if err := team.GetRepositories(&models.SearchTeamOptions{ + if err := team.GetRepositories(&models.SearchOrgTeamOptions{ ListOptions: utils.GetListOptions(ctx), }); err != nil { ctx.Error(http.StatusInternalServerError, "GetTeamRepos", err) @@ -707,15 +707,14 @@ func SearchTeam(ctx *context.APIContext) { listOptions := utils.GetListOptions(ctx) - opts := &models.SearchTeamOptions{ - UserID: ctx.User.ID, + opts := &models.SearchOrgTeamOptions{ Keyword: ctx.FormTrim("q"), OrgID: ctx.Org.Organization.ID, IncludeDesc: ctx.FormString("include_desc") == "" || ctx.FormBool("include_desc"), ListOptions: listOptions, } - teams, maxResults, err := models.SearchTeam(opts) + teams, maxResults, err := models.SearchOrgTeams(opts) if err != nil { log.Error("SearchTeam failed: %v", err) ctx.JSON(http.StatusInternalServerError, map[string]interface{}{ From 65cc8b1236c58a3d63990638e7eb0c6c04a3bb9d Mon Sep 17 00:00:00 2001 From: Gusted Date: Mon, 31 Jan 2022 22:43:19 +0100 Subject: [PATCH 3/4] Add condition back --- models/org_team.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/models/org_team.go b/models/org_team.go index c0ee38195c4f7..17f95bb5b08be 100644 --- a/models/org_team.go +++ b/models/org_team.go @@ -134,11 +134,13 @@ func SearchOrgTeams(opts *SearchOrgTeamOptions) ([]*Team, int64, error) { sess := db.GetEngine(db.DefaultContext) count, err := sess. + Where(cond). Count(new(Team)) if err != nil { return nil, 0, err } + sess = sess.Where(cond) if opts.PageSize == -1 { opts.PageSize = int(count) } else { From 98b7eaf6c3644f364e03ba074554be0b4a361fa7 Mon Sep 17 00:00:00 2001 From: Gusted Date: Mon, 31 Jan 2022 23:28:27 +0100 Subject: [PATCH 4/4] Use correct struct type --- routers/web/org/teams.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/web/org/teams.go b/routers/web/org/teams.go index 732f24b22c27d..9b0212e56955a 100644 --- a/routers/web/org/teams.go +++ b/routers/web/org/teams.go @@ -319,7 +319,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(&models.SearchTeamOptions{}); err != nil { + if err := ctx.Org.Team.GetRepositories(&models.SearchOrgTeamOptions{}); err != nil { ctx.ServerError("GetRepositories", err) return }