From acb1ceb1f426e87e7f821c01ab5b60dad7abc03d Mon Sep 17 00:00:00 2001 From: Jimmy Praet Date: Sun, 17 Jan 2021 17:34:19 +0100 Subject: [PATCH] Add review requested filter on pull request overview (#13701) * Add review requested filter on pull request overview #13682 fix formatting * add review_requested filter to /repos/issues/search API endpoint * only Approve and Reject status should supersede Request status * add support for team reviews * refactor: remove duplication of issue filtering conditions --- models/issue.go | 159 +++++++++++++-------- options/locale/locale_en-US.ini | 1 + routers/api/v1/repo/issue.go | 9 +- routers/repo/issue.go | 53 +++---- routers/user/home.go | 6 +- templates/repo/issue/list.tmpl | 3 + templates/repo/issue/milestone_issues.tmpl | 1 + templates/swagger/v1_json.tmpl | 6 + templates/user/dashboard/issues.tmpl | 6 + 9 files changed, 156 insertions(+), 88 deletions(-) diff --git a/models/issue.go b/models/issue.go index 7731a59ed0..3cd85dc6af 100644 --- a/models/issue.go +++ b/models/issue.go @@ -1090,6 +1090,7 @@ type IssuesOptions struct { AssigneeID int64 PosterID int64 MentionedID int64 + ReviewRequestedID int64 MilestoneIDs []int64 ProjectID int64 ProjectBoardID int64 @@ -1151,8 +1152,7 @@ func (opts *IssuesOptions) setupSession(sess *xorm.Session) { } if len(opts.RepoIDs) > 0 { - // In case repository IDs are provided but actually no repository has issue. - sess.In("issue.repo_id", opts.RepoIDs) + applyReposCondition(sess, opts.RepoIDs) } switch opts.IsClosed { @@ -1163,18 +1163,19 @@ func (opts *IssuesOptions) setupSession(sess *xorm.Session) { } if opts.AssigneeID > 0 { - sess.Join("INNER", "issue_assignees", "issue.id = issue_assignees.issue_id"). - And("issue_assignees.assignee_id = ?", opts.AssigneeID) + applyAssigneeCondition(sess, opts.AssigneeID) } if opts.PosterID > 0 { - sess.And("issue.poster_id=?", opts.PosterID) + applyPosterCondition(sess, opts.PosterID) } if opts.MentionedID > 0 { - sess.Join("INNER", "issue_user", "issue.id = issue_user.issue_id"). - And("issue_user.is_mentioned = ?", true). - And("issue_user.uid = ?", opts.MentionedID) + applyMentionedCondition(sess, opts.MentionedID) + } + + if opts.ReviewRequestedID > 0 { + applyReviewRequestedCondition(sess, opts.ReviewRequestedID) } if len(opts.MilestoneIDs) > 0 { @@ -1232,6 +1233,33 @@ func (opts *IssuesOptions) setupSession(sess *xorm.Session) { } } +func applyReposCondition(sess *xorm.Session, repoIDs []int64) *xorm.Session { + return sess.In("issue.repo_id", repoIDs) +} + +func applyAssigneeCondition(sess *xorm.Session, assigneeID int64) *xorm.Session { + return sess.Join("INNER", "issue_assignees", "issue.id = issue_assignees.issue_id"). + And("issue_assignees.assignee_id = ?", assigneeID) +} + +func applyPosterCondition(sess *xorm.Session, posterID int64) *xorm.Session { + return sess.And("issue.poster_id=?", posterID) +} + +func applyMentionedCondition(sess *xorm.Session, mentionedID int64) *xorm.Session { + return sess.Join("INNER", "issue_user", "issue.id = issue_user.issue_id"). + And("issue_user.is_mentioned = ?", true). + And("issue_user.uid = ?", mentionedID) +} + +func applyReviewRequestedCondition(sess *xorm.Session, reviewRequestedID int64) *xorm.Session { + return sess.Join("INNER", []string{"review", "r"}, "issue.id = r.issue_id"). + And("r.type = ?", ReviewTypeRequest). + And("r.reviewer_id = ? and r.id in (select max(id) from review where issue_id = r.issue_id and reviewer_id = r.reviewer_id and type in (?, ?, ?))"+ + " or r.reviewer_team_id in (select team_id from team_user where uid = ?)", + reviewRequestedID, ReviewTypeApprove, ReviewTypeReject, ReviewTypeRequest, reviewRequestedID) +} + // CountIssuesByRepo map from repoID to number of issues matching the options func CountIssuesByRepo(opts *IssuesOptions) (map[int64]int64, error) { sess := x.NewSession() @@ -1364,6 +1392,7 @@ type IssueStats struct { AssignCount int64 CreateCount int64 MentionCount int64 + ReviewRequestedCount int64 } // Filter modes. @@ -1372,6 +1401,7 @@ const ( FilterModeAssign FilterModeCreate FilterModeMention + FilterModeReviewRequested ) func parseCountResult(results []map[string][]byte) int64 { @@ -1387,14 +1417,15 @@ func parseCountResult(results []map[string][]byte) int64 { // IssueStatsOptions contains parameters accepted by GetIssueStats. type IssueStatsOptions struct { - RepoID int64 - Labels string - MilestoneID int64 - AssigneeID int64 - MentionedID int64 - PosterID int64 - IsPull util.OptionalBool - IssueIDs []int64 + RepoID int64 + Labels string + MilestoneID int64 + AssigneeID int64 + MentionedID int64 + PosterID int64 + ReviewRequestedID int64 + IsPull util.OptionalBool + IssueIDs []int64 } // GetIssueStats returns issue statistic information by given conditions. @@ -1423,6 +1454,7 @@ func GetIssueStats(opts *IssueStatsOptions) (*IssueStats, error) { accum.AssignCount += stats.AssignCount accum.CreateCount += stats.CreateCount accum.OpenCount += stats.MentionCount + accum.ReviewRequestedCount += stats.ReviewRequestedCount i = chunk } return accum, nil @@ -1460,18 +1492,19 @@ func getIssueStatsChunk(opts *IssueStatsOptions, issueIDs []int64) (*IssueStats, } if opts.AssigneeID > 0 { - sess.Join("INNER", "issue_assignees", "issue.id = issue_assignees.issue_id"). - And("issue_assignees.assignee_id = ?", opts.AssigneeID) + applyAssigneeCondition(sess, opts.AssigneeID) } if opts.PosterID > 0 { - sess.And("issue.poster_id = ?", opts.PosterID) + applyPosterCondition(sess, opts.PosterID) } if opts.MentionedID > 0 { - sess.Join("INNER", "issue_user", "issue.id = issue_user.issue_id"). - And("issue_user.uid = ?", opts.MentionedID). - And("issue_user.is_mentioned = ?", true) + applyMentionedCondition(sess, opts.MentionedID) + } + + if opts.ReviewRequestedID > 0 { + applyReviewRequestedCondition(sess, opts.ReviewRequestedID) } switch opts.IsPull { @@ -1539,57 +1572,66 @@ func GetUserIssueStats(opts UserIssueStatsOptions) (*IssueStats, error) { switch opts.FilterMode { case FilterModeAll: - stats.OpenCount, err = sess(cond).And("issue.is_closed = ?", false). - And(builder.In("issue.repo_id", opts.UserRepoIDs)). + stats.OpenCount, err = applyReposCondition(sess(cond), opts.UserRepoIDs). + And("issue.is_closed = ?", false). Count(new(Issue)) if err != nil { return nil, err } - stats.ClosedCount, err = sess(cond).And("issue.is_closed = ?", true). - And(builder.In("issue.repo_id", opts.UserRepoIDs)). + stats.ClosedCount, err = applyReposCondition(sess(cond), opts.UserRepoIDs). + And("issue.is_closed = ?", true). Count(new(Issue)) if err != nil { return nil, err } case FilterModeAssign: - stats.OpenCount, err = sess(cond).And("issue.is_closed = ?", false). - Join("INNER", "issue_assignees", "issue.id = issue_assignees.issue_id"). - And("issue_assignees.assignee_id = ?", opts.UserID). + stats.OpenCount, err = applyAssigneeCondition(sess(cond), opts.UserID). + And("issue.is_closed = ?", false). Count(new(Issue)) if err != nil { return nil, err } - stats.ClosedCount, err = sess(cond).And("issue.is_closed = ?", true). - Join("INNER", "issue_assignees", "issue.id = issue_assignees.issue_id"). - And("issue_assignees.assignee_id = ?", opts.UserID). + stats.ClosedCount, err = applyAssigneeCondition(sess(cond), opts.UserID). + And("issue.is_closed = ?", true). Count(new(Issue)) if err != nil { return nil, err } case FilterModeCreate: - stats.OpenCount, err = sess(cond).And("issue.is_closed = ?", false). - And("issue.poster_id = ?", opts.UserID). + stats.OpenCount, err = applyPosterCondition(sess(cond), opts.UserID). + And("issue.is_closed = ?", false). Count(new(Issue)) if err != nil { return nil, err } - stats.ClosedCount, err = sess(cond).And("issue.is_closed = ?", true). - And("issue.poster_id = ?", opts.UserID). + stats.ClosedCount, err = applyPosterCondition(sess(cond), opts.UserID). + And("issue.is_closed = ?", true). Count(new(Issue)) if err != nil { return nil, err } case FilterModeMention: - stats.OpenCount, err = sess(cond).And("issue.is_closed = ?", false). - Join("INNER", "issue_user", "issue.id = issue_user.issue_id and issue_user.is_mentioned = ?", true). - And("issue_user.uid = ?", opts.UserID). + stats.OpenCount, err = applyMentionedCondition(sess(cond), opts.UserID). + And("issue.is_closed = ?", false). Count(new(Issue)) if err != nil { return nil, err } - stats.ClosedCount, err = sess(cond).And("issue.is_closed = ?", true). - Join("INNER", "issue_user", "issue.id = issue_user.issue_id and issue_user.is_mentioned = ?", true). - And("issue_user.uid = ?", opts.UserID). + stats.ClosedCount, err = applyMentionedCondition(sess(cond), opts.UserID). + And("issue.is_closed = ?", true). + Count(new(Issue)) + if err != nil { + return nil, err + } + case FilterModeReviewRequested: + stats.OpenCount, err = applyReviewRequestedCondition(sess(cond), opts.UserID). + And("issue.is_closed = ?", false). + Count(new(Issue)) + if err != nil { + return nil, err + } + stats.ClosedCount, err = applyReviewRequestedCondition(sess(cond), opts.UserID). + And("issue.is_closed = ?", true). Count(new(Issue)) if err != nil { return nil, err @@ -1597,32 +1639,27 @@ func GetUserIssueStats(opts UserIssueStatsOptions) (*IssueStats, error) { } cond = cond.And(builder.Eq{"issue.is_closed": opts.IsClosed}) - stats.AssignCount, err = sess(cond). - Join("INNER", "issue_assignees", "issue.id = issue_assignees.issue_id"). - And("issue_assignees.assignee_id = ?", opts.UserID). - Count(new(Issue)) + stats.AssignCount, err = applyAssigneeCondition(sess(cond), opts.UserID).Count(new(Issue)) if err != nil { return nil, err } - stats.CreateCount, err = sess(cond). - And("poster_id = ?", opts.UserID). - Count(new(Issue)) + stats.CreateCount, err = applyPosterCondition(sess(cond), opts.UserID).Count(new(Issue)) if err != nil { return nil, err } - stats.MentionCount, err = sess(cond). - Join("INNER", "issue_user", "issue.id = issue_user.issue_id and issue_user.is_mentioned = ?", true). - And("issue_user.uid = ?", opts.UserID). - Count(new(Issue)) + stats.MentionCount, err = applyMentionedCondition(sess(cond), opts.UserID).Count(new(Issue)) if err != nil { return nil, err } - stats.YourRepositoriesCount, err = sess(cond). - And(builder.In("issue.repo_id", opts.UserRepoIDs)). - Count(new(Issue)) + stats.YourRepositoriesCount, err = applyReposCondition(sess(cond), opts.UserRepoIDs).Count(new(Issue)) + if err != nil { + return nil, err + } + + stats.ReviewRequestedCount, err = applyReviewRequestedCondition(sess(cond), opts.UserID).Count(new(Issue)) if err != nil { return nil, err } @@ -1646,13 +1683,11 @@ func GetRepoIssueStats(repoID, uid int64, filterMode int, isPull bool) (numOpen switch filterMode { case FilterModeAssign: - openCountSession.Join("INNER", "issue_assignees", "issue.id = issue_assignees.issue_id"). - And("issue_assignees.assignee_id = ?", uid) - closedCountSession.Join("INNER", "issue_assignees", "issue.id = issue_assignees.issue_id"). - And("issue_assignees.assignee_id = ?", uid) + applyAssigneeCondition(openCountSession, uid) + applyAssigneeCondition(closedCountSession, uid) case FilterModeCreate: - openCountSession.And("poster_id = ?", uid) - closedCountSession.And("poster_id = ?", uid) + applyPosterCondition(openCountSession, uid) + applyPosterCondition(closedCountSession, uid) } openResult, _ := openCountSession.Count(new(Issue)) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 4546a06e81..73451eeebb 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1030,6 +1030,7 @@ issues.filter_type.all_issues = All issues issues.filter_type.assigned_to_you = Assigned to you issues.filter_type.created_by_you = Created by you issues.filter_type.mentioning_you = Mentioning you +issues.filter_type.review_requested = Review requested issues.filter_sort = Sort issues.filter_sort.latest = Newest issues.filter_sort.oldest = Oldest diff --git a/routers/api/v1/repo/issue.go b/routers/api/v1/repo/issue.go index 25153ad507..bab8f373ce 100644 --- a/routers/api/v1/repo/issue.go +++ b/routers/api/v1/repo/issue.go @@ -79,6 +79,10 @@ func SearchIssues(ctx *context.APIContext) { // in: query // description: filter (issues / pulls) mentioning you, default is false // type: boolean + // - name: review_requested + // in: query + // description: filter pulls requesting your review, default is false + // type: boolean // - name: page // in: query // description: page number of results to return (1-based) @@ -204,7 +208,7 @@ func SearchIssues(ctx *context.APIContext) { UpdatedAfterUnix: since, } - // Filter for: Created by User, Assigned to User, Mentioning User + // Filter for: Created by User, Assigned to User, Mentioning User, Review of User Requested if ctx.QueryBool("created") { issuesOpt.PosterID = ctx.User.ID } @@ -214,6 +218,9 @@ func SearchIssues(ctx *context.APIContext) { if ctx.QueryBool("mentioned") { issuesOpt.MentionedID = ctx.User.ID } + if ctx.QueryBool("review_requested") { + issuesOpt.ReviewRequestedID = ctx.User.ID + } if issues, err = models.Issues(issuesOpt); err != nil { ctx.Error(http.StatusInternalServerError, "Issues", err) diff --git a/routers/repo/issue.go b/routers/repo/issue.go index 478baf8d86..7b4044ac7b 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -113,16 +113,17 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption uti var err error viewType := ctx.Query("type") sortType := ctx.Query("sort") - types := []string{"all", "your_repositories", "assigned", "created_by", "mentioned"} + types := []string{"all", "your_repositories", "assigned", "created_by", "mentioned", "review_requested"} if !util.IsStringInSlice(viewType, types, true) { viewType = "all" } var ( - assigneeID = ctx.QueryInt64("assignee") - posterID int64 - mentionedID int64 - forceEmpty bool + assigneeID = ctx.QueryInt64("assignee") + posterID int64 + mentionedID int64 + reviewRequestedID int64 + forceEmpty bool ) if ctx.IsSigned { @@ -133,6 +134,8 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption uti mentionedID = ctx.User.ID case "assigned": assigneeID = ctx.User.ID + case "review_requested": + reviewRequestedID = ctx.User.ID } } @@ -169,14 +172,15 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption uti issueStats = &models.IssueStats{} } else { issueStats, err = models.GetIssueStats(&models.IssueStatsOptions{ - RepoID: repo.ID, - Labels: selectLabels, - MilestoneID: milestoneID, - AssigneeID: assigneeID, - MentionedID: mentionedID, - PosterID: posterID, - IsPull: isPullOption, - IssueIDs: issueIDs, + RepoID: repo.ID, + Labels: selectLabels, + MilestoneID: milestoneID, + AssigneeID: assigneeID, + MentionedID: mentionedID, + PosterID: posterID, + ReviewRequestedID: reviewRequestedID, + IsPull: isPullOption, + IssueIDs: issueIDs, }) if err != nil { ctx.ServerError("GetIssueStats", err) @@ -217,17 +221,18 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption uti Page: pager.Paginater.Current(), PageSize: setting.UI.IssuePagingNum, }, - RepoIDs: []int64{repo.ID}, - AssigneeID: assigneeID, - PosterID: posterID, - MentionedID: mentionedID, - MilestoneIDs: mileIDs, - ProjectID: projectID, - IsClosed: util.OptionalBoolOf(isShowClosed), - IsPull: isPullOption, - LabelIDs: labelIDs, - SortType: sortType, - IssueIDs: issueIDs, + RepoIDs: []int64{repo.ID}, + AssigneeID: assigneeID, + PosterID: posterID, + MentionedID: mentionedID, + ReviewRequestedID: reviewRequestedID, + MilestoneIDs: mileIDs, + ProjectID: projectID, + IsClosed: util.OptionalBoolOf(isShowClosed), + IsPull: isPullOption, + LabelIDs: labelIDs, + SortType: sortType, + IssueIDs: issueIDs, }) if err != nil { ctx.ServerError("Issues", err) diff --git a/routers/user/home.go b/routers/user/home.go index 3c27bbe2a8..a8a8a5f3d7 100644 --- a/routers/user/home.go +++ b/routers/user/home.go @@ -392,6 +392,8 @@ func buildIssueOverview(ctx *context.Context, unitType models.UnitType) { filterMode = models.FilterModeCreate case "mentioned": filterMode = models.FilterModeMention + case "review_requested": + filterMode = models.FilterModeReviewRequested case "your_repositories": // filterMode already set to All default: viewType = "your_repositories" @@ -431,7 +433,9 @@ func buildIssueOverview(ctx *context.Context, unitType models.UnitType) { case models.FilterModeCreate: opts.PosterID = ctx.User.ID case models.FilterModeMention: - opts.MentionedID = ctx.User.ID + opts.MentionedID = ctxUser.ID + case models.FilterModeReviewRequested: + opts.ReviewRequestedID = ctxUser.ID } if ctxUser.IsOrganization() { diff --git a/templates/repo/issue/list.tmpl b/templates/repo/issue/list.tmpl index 2b64d26700..7b856e60cb 100644 --- a/templates/repo/issue/list.tmpl +++ b/templates/repo/issue/list.tmpl @@ -89,6 +89,9 @@ {{.i18n.Tr "repo.issues.filter_type.assigned_to_you"}} {{.i18n.Tr "repo.issues.filter_type.created_by_you"}} {{.i18n.Tr "repo.issues.filter_type.mentioning_you"}} + {{if .PageIsPullList}} + {{.i18n.Tr "repo.issues.filter_type.review_requested"}} + {{end}} {{end}} diff --git a/templates/repo/issue/milestone_issues.tmpl b/templates/repo/issue/milestone_issues.tmpl index 638134c442..c2c81682ff 100644 --- a/templates/repo/issue/milestone_issues.tmpl +++ b/templates/repo/issue/milestone_issues.tmpl @@ -88,6 +88,7 @@ {{.i18n.Tr "repo.issues.filter_type.assigned_to_you"}} {{.i18n.Tr "repo.issues.filter_type.created_by_you"}} {{.i18n.Tr "repo.issues.filter_type.mentioning_you"}} + {{.i18n.Tr "repo.issues.filter_type.review_requested"}} {{end}} diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 81ccf4f725..6650c09bb4 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -1911,6 +1911,12 @@ "name": "mentioned", "in": "query" }, + { + "type": "boolean", + "description": "filter pulls requesting your review, default is false", + "name": "review_requested", + "in": "query" + }, { "type": "integer", "description": "page number of results to return (1-based)", diff --git a/templates/user/dashboard/issues.tmpl b/templates/user/dashboard/issues.tmpl index 62428dce42..dfd0e5d8ec 100644 --- a/templates/user/dashboard/issues.tmpl +++ b/templates/user/dashboard/issues.tmpl @@ -21,6 +21,12 @@ {{.i18n.Tr "repo.issues.filter_type.mentioning_you"}} {{CountFmt .IssueStats.MentionCount}} + {{if .PageIsPulls}} + + {{.i18n.Tr "repo.issues.filter_type.review_requested"}} + {{CountFmt .IssueStats.ReviewRequestedCount}} + + {{end}}
All