From 38acce2f3fb1489da7ba3a82dcdbb86172342db3 Mon Sep 17 00:00:00 2001 From: Gusted Date: Mon, 16 May 2022 09:48:16 +0000 Subject: [PATCH] Fix issue overview for teams (#19652) (#19653) - Backport #19652 - Don't use hacky solution to limit to the correct RepoID's, instead use current code to handle these limits. The existing code is more correct than the hacky solution. - Resolves #19636 --- go.mod | 2 +- go.sum | 3 ++- models/issue.go | 7 ++----- models/repo_list.go | 18 ++++++++++++++++-- routers/web/user/home.go | 19 +++---------------- 5 files changed, 24 insertions(+), 25 deletions(-) diff --git a/go.mod b/go.mod index e34a3e740..ad6770c62 100644 --- a/go.mod +++ b/go.mod @@ -102,7 +102,7 @@ require ( gopkg.in/yaml.v2 v2.4.0 mvdan.cc/xurls/v2 v2.2.0 strk.kbt.io/projects/go/libravatar v0.0.0-20191008002943-06d1c002b251 - xorm.io/builder v0.3.9 + xorm.io/builder v0.3.10 xorm.io/xorm v1.2.5 ) diff --git a/go.sum b/go.sum index f6aeda963..aa4bafad2 100644 --- a/go.sum +++ b/go.sum @@ -2336,7 +2336,8 @@ sigs.k8s.io/yaml v1.2.0/go.mod h1:yfXDCHCao9+ENCvLSE62v9VSji2MKu5jeNfTrofGhJc= sourcegraph.com/sourcegraph/appdash v0.0.0-20190731080439-ebfcffb1b5c0/go.mod h1:hI742Nqp5OhwiqlzhgfbWU4mW4yO10fP+LoT9WOswdU= strk.kbt.io/projects/go/libravatar v0.0.0-20191008002943-06d1c002b251 h1:mUcz5b3FJbP5Cvdq7Khzn6J9OCUQJaBwgBkCR+MOwSs= strk.kbt.io/projects/go/libravatar v0.0.0-20191008002943-06d1c002b251/go.mod h1:FJGmPh3vz9jSos1L/F91iAgnC/aejc0wIIrF2ZwJxdY= -xorm.io/builder v0.3.9 h1:Sd65/LdWyO7LR8+Cbd+e7mm3sK/7U9k0jS3999IDHMc= xorm.io/builder v0.3.9/go.mod h1:aUW0S9eb9VCaPohFCH3j7czOx1PMW3i1HrSzbLYGBSE= +xorm.io/builder v0.3.10 h1:Rvkncad3Lo9YIVqCbgIf6QnpR/HcW3IEr0AANNpuyMQ= +xorm.io/builder v0.3.10/go.mod h1:aUW0S9eb9VCaPohFCH3j7czOx1PMW3i1HrSzbLYGBSE= xorm.io/xorm v1.2.5 h1:tqN7OhN8P9xi52qBb76I8m5maAJMz/SSbgK2RGPCPbo= xorm.io/xorm v1.2.5/go.mod h1:fTG8tSjk6O1BYxwuohZUK+S1glnRycsCF05L1qQyEU0= diff --git a/models/issue.go b/models/issue.go index fe2b04ce2..f2552c0a1 100644 --- a/models/issue.go +++ b/models/issue.go @@ -1346,9 +1346,7 @@ func (opts *IssuesOptions) setupSessionNoLimit(sess *xorm.Session) { } if opts.User != nil { - sess.And( - issuePullAccessibleRepoCond("issue.repo_id", opts.User.ID, opts.Org, opts.Team, opts.IsPull.IsTrue()), - ) + sess.And(issuePullAccessibleRepoCond("issue.repo_id", opts.User.ID, opts.Org, opts.Team, opts.IsPull.IsTrue())) } } @@ -1413,7 +1411,6 @@ func CountIssuesByRepo(opts *IssuesOptions) (map[int64]int64, error) { e := db.GetEngine(db.DefaultContext) sess := e.Join("INNER", "repository", "`issue`.repo_id = `repository`.id") - opts.setupSessionNoLimit(sess) countsSlice := make([]*struct { @@ -1440,7 +1437,6 @@ func GetRepoIDsForIssuesOptions(opts *IssuesOptions, user *user_model.User) ([]i e := db.GetEngine(db.DefaultContext) sess := e.Join("INNER", "repository", "`issue`.repo_id = `repository`.id") - opts.setupSessionNoLimit(sess) accessCond := accessibleRepositoryCondition(user) @@ -1485,6 +1481,7 @@ func CountIssues(opts *IssuesOptions) (int64, error) { sess := e.Select("COUNT(issue.id) AS count").Table("issue") sess.Join("INNER", "repository", "`issue`.repo_id = `repository`.id") opts.setupSessionNoLimit(sess) + if err := sess.Find(&countsSlice); err != nil { return 0, fmt.Errorf("unable to CountIssues: %w", err) } diff --git a/models/repo_list.go b/models/repo_list.go index 9cb7a163f..a8bff1552 100644 --- a/models/repo_list.go +++ b/models/repo_list.go @@ -233,14 +233,28 @@ func teamUnitsRepoCond(id string, userID, orgID, teamID int64, units ...unit.Typ builder.Select("repo_id").From("team_repo").Where( builder.Eq{ "team_id": teamID, - }.And( + }.And(builder.Or( + // Check if the user is member of the team. builder.In( "team_id", builder.Select("team_id").From("team_user").Where( builder.Eq{ "uid": userID, }, ), - )).And( + ), + // Check if the user is in the owner team of the organisation. + builder.Exists(builder.Select("team_id").From("team_user"). + Where(builder.Eq{ + "org_id": orgID, + "team_id": builder.Select("id").From("team").Where( + builder.Eq{ + "org_id": orgID, + "lower_name": strings.ToLower(ownerTeamName), + }), + "uid": userID, + }), + ), + )).And( builder.In( "team_id", builder.Select("team_id").From("team_unit").Where( builder.Eq{ diff --git a/routers/web/user/home.go b/routers/web/user/home.go index b2d9a9ca5..7c8f01885 100644 --- a/routers/web/user/home.go +++ b/routers/web/user/home.go @@ -442,12 +442,13 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) { AllLimited: false, } - if ctxUser.IsOrganization() && ctx.Org.Team != nil { - repoOpts.TeamID = ctx.Org.Team.ID + if team != nil { + repoOpts.TeamID = team.ID } switch filterMode { case models.FilterModeAll: + case models.FilterModeYourRepositories: case models.FilterModeAssign: opts.AssigneeID = ctx.User.ID case models.FilterModeCreate: @@ -456,13 +457,6 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) { opts.MentionedID = ctx.User.ID case models.FilterModeReviewRequested: opts.ReviewRequestedID = ctx.User.ID - case models.FilterModeYourRepositories: - if ctxUser.IsOrganization() && ctx.Org.Team != nil { - // Fixes a issue whereby the user's ID would be used - // to check if it's in the team(which possible isn't the case). - opts.User = nil - } - opts.RepoCond = models.SearchRepositoryCondition(repoOpts) } // keyword holds the search term entered into the search field. @@ -594,13 +588,6 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) { Org: org, Team: team, } - if filterMode == models.FilterModeYourRepositories { - statsOpts.RepoCond = models.SearchRepositoryCondition(repoOpts) - } - // Detect when we only should search by team. - if opts.User == nil { - statsOpts.UserID = 0 - } issueStats, err = models.GetUserIssueStats(statsOpts) if err != nil { ctx.ServerError("GetUserIssueStats Shown", err)