Show review labels in the pull request lists

models/branches.go

@@ -148,31 +148,6 @@ func (protectBranch *ProtectedBranch) isUserOfficialReviewer(e Engine, user *Use
 	return inTeam, nil
 }
 
-// HasEnoughApprovals returns true if pr has enough granted approvals.
-func (protectBranch *ProtectedBranch) HasEnoughApprovals(pr *PullRequest) bool {
-	if protectBranch.RequiredApprovals == 0 {
-		return true
-	}
-	return protectBranch.GetGrantedApprovalsCount(pr) >= protectBranch.RequiredApprovals
-}
-
-// GetGrantedApprovalsCount returns the number of granted approvals for pr. A granted approval must be authored by a user in an approval whitelist.
-func (protectBranch *ProtectedBranch) GetGrantedApprovalsCount(pr *PullRequest) int64 {
-	sess := x.Where("issue_id = ?", pr.IssueID).
-		And("type = ?", ReviewTypeApprove).
-		And("official = ?", true)
-	if protectBranch.DismissStaleApprovals {
-		sess = sess.And("stale = ?", false)
-	}
-	approvals, err := sess.Count(new(Review))
-	if err != nil {
-		log.Error("GetGrantedApprovalsCount: %v", err)
-		return 0
-	}
-
-	return approvals
-}
-
 // MergeBlockedByRejectedReview returns true if merge is blocked by rejected reviews
 func (protectBranch *ProtectedBranch) MergeBlockedByRejectedReview(pr *PullRequest) bool {
 	if !protectBranch.BlockOnRejectedReviews {

models/pull.go

@@ -89,6 +89,10 @@ func (pr *PullRequest) loadAttributes(e Engine) (err error) {
 		}
 	}
 
+	if err = pr.loadProtectedBranch(e); err != nil {
+		return fmt.Errorf("pr.loadAttributes: loadProtectedBranch: %v", err)
+	}
+
 	return nil
 }
 
@@ -171,6 +175,79 @@ func (pr *PullRequest) loadProtectedBranch(e Engine) (err error) {
 	return
 }
 
+func (pr *PullRequest) requiredApprovals() int64 {
+	if pr.ProtectedBranch != nil {
+		return pr.ProtectedBranch.RequiredApprovals
+	}
+	return 0
+}
+
+// RequiresApproval returns true if this pull request requires approval, otherwise returns false
+func (pr *PullRequest) RequiresApproval() bool {
+	return pr.requiredApprovals() > 0
+}
+
+// ReviewStatusCount holds the Status and Count of a Review
+type ReviewStatusCount struct {
+	Status string
+	Count  int64
+}
+
+// GetReviewStatus returns a ReviewStatusCount of this pull request
+func (pr *PullRequest) GetReviewStatus() ReviewStatusCount {
+	if pr.RequiresApproval() {
+		rejections := pr.GetRejectedReviewsCount()
+		if rejections > 0 {
+			return ReviewStatusCount{Status: "rejected", Count: rejections}
+		}
+		approvals := pr.GetGrantedApprovalsCount()
+		if approvals >= pr.ProtectedBranch.RequiredApprovals {
+			return ReviewStatusCount{Status: "approved", Count: approvals}
+		}
+		return ReviewStatusCount{Status: "required", Count: pr.ProtectedBranch.RequiredApprovals - approvals}
+	}
+	return ReviewStatusCount{Status: "approved", Count: 0} // by default
+}
+
+// GetRejectedReviewsCount returns the number of rejected reviews for pr.
+func (pr *PullRequest) GetRejectedReviewsCount() int64 {
+	rejects, err := x.Where("issue_id = ?", pr.IssueID).
+		And("type = ?", ReviewTypeReject).
+		And("official = ?", true).
+		Count(new(Review))
+	if err != nil {
+		log.Error("GetRejectedReviewsCount: %v", err)
+		return 0
+	}
+
+	return rejects
+}
+
+// HasEnoughApprovals returns true if pr has enough granted approvals.
+func (pr *PullRequest) HasEnoughApprovals() bool {
+	if pr.ProtectedBranch.RequiredApprovals == 0 {
+		return true
+	}
+	return pr.GetGrantedApprovalsCount() >= pr.ProtectedBranch.RequiredApprovals
+}
+
+// GetGrantedApprovalsCount returns the number of granted approvals for pr. A granted approval must be authored by a user in an approval whitelist.
+func (pr *PullRequest) GetGrantedApprovalsCount() int64 {
+	sess := x.Where("issue_id = ?", pr.IssueID).
+		And("type = ?", ReviewTypeApprove).
+		And("official = ?", true)
+	if pr.ProtectedBranch.DismissStaleApprovals {
+		sess = sess.And("stale = ?", false)
+	}
+	approvals, err := sess.Count(new(Review))
+	if err != nil {
+		log.Error("GetGrantedApprovalsCount: %v", err)
+		return 0
+	}
+
+	return approvals
+}
+
 // GetDefaultMergeMessage returns default message used when merging pull request
 func (pr *PullRequest) GetDefaultMergeMessage() string {
 	if pr.HeadRepo == nil {

models/pull_list.go

@@ -113,7 +113,30 @@ func (prs PullRequestList) loadAttributes(e Engine) error {
 		return nil
 	}
 
-	// Load issues.
+	if err := prs.loadIssues(e); err != nil {
+		return fmt.Errorf("prs.loadAttributes: loadIssues: %v", err)
+	}
+
+	if err := prs.loadProtectedBranches(e); err != nil {
+		return fmt.Errorf("prs.loadAttributes: loadProtectedBranches: %v", err)
+	}
+
+	return nil
+}
+
+func (prs PullRequestList) getIssueIDs() []int64 {
+	issueIDs := make([]int64, 0, len(prs))
+	for i := range prs {
+		issueIDs = append(issueIDs, prs[i].IssueID)
+	}
+	return issueIDs
+}
+
+func (prs PullRequestList) loadIssues(e Engine) (err error) {
+	if len(prs) == 0 {
+		return nil
+	}
+
 	issueIDs := prs.getIssueIDs()
 	issues := make([]*Issue, 0, len(issueIDs))
 	if err := e.
@@ -133,12 +156,54 @@ func (prs PullRequestList) loadAttributes(e Engine) error {
 	return nil
 }
 
-func (prs PullRequestList) getIssueIDs() []int64 {
-	issueIDs := make([]int64, 0, len(prs))
-	for i := range prs {
-		issueIDs = append(issueIDs, prs[i].IssueID)
+type protectedBranchKey struct {
+	RepoID     int64
+	BranchName string
+}
+
+func (prs PullRequestList) getProtectedBranchKeys() []protectedBranchKey {
+	prKeys := make(map[protectedBranchKey]struct{}, len(prs))
+	for _, pr := range prs {
+		if pr.BaseRepoID <= 0 {
+			continue
+		}
+		key := protectedBranchKey{pr.BaseRepoID, pr.BaseBranch}
+		if _, ok := prKeys[key]; !ok {
+			prKeys[key] = struct{}{}
+		}
 	}
-	return issueIDs
+	return valuesProtectedBranchKeys(prKeys)
+}
+
+func valuesProtectedBranchKeys(m map[protectedBranchKey]struct{}) []protectedBranchKey {
+	values := make([]protectedBranchKey, 0, len(m))
+	for k := range m {
+		values = append(values, k)
+	}
+	return values
+}
+
+func (prs PullRequestList) loadProtectedBranches(e Engine) (err error) {
+	if len(prs) == 0 {
+		return nil
+	}
+
+	prKeys := prs.getProtectedBranchKeys()
+	prMaps := make(map[protectedBranchKey]*ProtectedBranch, len(prKeys))
+	sess := e.Where("repo_id = ? and branch_name = ?", prKeys[0].RepoID, prKeys[0].BranchName)
+	for _, prk := range prKeys[1:] {
+		sess.Or("repo_id = ? and branch_name = ?", prk.RepoID, prk.BranchName)
+	}
+	err = sess.Find(&prMaps)
+	if err != nil {
+		return err
+	}
+
+	for _, pr := range prs {
+		pr.ProtectedBranch = prMaps[protectedBranchKey{pr.BaseRepoID, pr.BaseBranch}]
+	}
+
+	return nil
 }
 
 // LoadAttributes load all the prs attributes

models/pull_sign.go

@@ -57,7 +57,7 @@ func (pr *PullRequest) SignMerge(u *User, tmpBasePath, baseCommit, headCommit st
 			if protectedBranch == nil {
 				return false, "", &ErrWontSign{approved}
 			}
-			if protectedBranch.GetGrantedApprovalsCount(pr) < 1 {
+			if pr.GetGrantedApprovalsCount() < 1 {
 				return false, "", &ErrWontSign{approved}
 			}
 		case baseSigned:

options/locale/locale_en-US.ini

@@ -1100,6 +1100,9 @@ pulls.update_branch = Update branch
 pulls.update_branch_success = Branch update was successful
 pulls.update_not_allowed = You are not allowed to update branch
 pulls.outdated_with_base_branch = This branch is out-of-date with the base branch
+pulls.review_required = Review required <span class="ui white small label">%d</span>
+pulls.review_approved = Approved <span class="ui white small label">%d</span>
+pulls.review_rejected = Changes requested <span class="ui white small label">%d</span>
 
 milestones.new = New Milestone
 milestones.open_tab = %d Open

routers/repo/issue.go

@@ -229,8 +229,8 @@ func issues(ctx *context.Context, milestoneID int64, isPullOption util.OptionalB
 		}
 
 		if issues[i].IsPull {
-			if err := issues[i].LoadPullRequest(); err != nil {
-				ctx.ServerError("LoadPullRequest", err)
+			if err := issues[i].PullRequest.LoadAttributes(); err != nil {
+				ctx.ServerError("LoadAttributes", err)
 				return
 			}
 
@@ -971,8 +971,8 @@ func ViewIssue(ctx *context.Context) {
 			return
 		}
 		if pull.ProtectedBranch != nil {
-			cnt := pull.ProtectedBranch.GetGrantedApprovalsCount(pull)
-			ctx.Data["IsBlockedByApprovals"] = !pull.ProtectedBranch.HasEnoughApprovals(pull)
+			cnt := pull.GetGrantedApprovalsCount()
+			ctx.Data["IsBlockedByApprovals"] = !pull.HasEnoughApprovals()
 			ctx.Data["IsBlockedByRejection"] = pull.ProtectedBranch.MergeBlockedByRejectedReview(pull)
 			ctx.Data["GrantedApprovals"] = cnt
 			ctx.Data["RequireSigned"] = pull.ProtectedBranch.RequireSignedCommits

routers/user/home.go

@@ -535,6 +535,11 @@ func Issues(ctx *context.Context) {
 
 		if isPullList {
 			commitStatus[issue.PullRequest.ID], _ = issue.PullRequest.GetLastCommitStatus()
+
+			if err := issue.PullRequest.LoadAttributes(); err != nil {
+				ctx.ServerError("LoadAttributes", err)
+				return
+			}
 		}
 	}
 

services/pull/merge.go

@@ -559,7 +559,7 @@ func CheckPRReadyToMerge(pr *models.PullRequest) (err error) {
 		}
 	}
 
-	if enoughApprovals := pr.ProtectedBranch.HasEnoughApprovals(pr); !enoughApprovals {
+	if enoughApprovals := pr.HasEnoughApprovals(); !enoughApprovals {
 		return models.ErrNotAllowedToMerge{
 			Reason: "Does not have enough approvals",
 		}

templates/repo/issue/list.tmpl

@@ -239,7 +239,18 @@
 						{{else}}
 							{{$.i18n.Tr .GetLastEventLabelFake $timeStr (.Poster.GetDisplayName | Escape) | Safe}}
 						{{end}}
-
+						{{if .IsPull }}{{if .PullRequest.RequiresApproval }}
+							{{ $reviewStatus := .PullRequest.GetReviewStatus }}
+							<a class="milestone" href="{{$.Link}}/{{.Index}}">
+								{{if eq $reviewStatus.Status "rejected"}}
+									{{$.i18n.Tr "repo.pulls.review_rejected" $reviewStatus.Count | Safe}}
+								{{else if eq $reviewStatus.Status "required"}}
+									{{$.i18n.Tr "repo.pulls.review_required" $reviewStatus.Count | Safe}}
+								{{else}}
+									{{$.i18n.Tr "repo.pulls.review_approved" $reviewStatus.Count | Safe}}
+								{{end}}
+							</a>
+						{{end}}{{end}}
 						{{if .Milestone}}
 							<a class="milestone" href="{{$.Link}}?q={{$.Keyword}}&type={{$.ViewType}}&state={{$.State}}&labels={{$.SelectLabels}}&milestone={{.Milestone.ID}}&assignee={{$.AssigneeID}}">
 								{{svg "octicon-milestone" 16}} {{.Milestone.Name}}

templates/repo/issue/milestone_issues.tmpl

@@ -206,6 +206,18 @@
 						{{else}}
 							{{$.i18n.Tr .GetLastEventLabelFake $timeStr (.Poster.GetDisplayName|Escape) | Safe}}
 						{{end}}
+						{{if .IsPull }}{{if .PullRequest.RequiresApproval }}
+							{{ $reviewStatus := .PullRequest.GetReviewStatus }}
+							<a class="milestone" href="{{$.RepoLink}}/pulls/{{.Index}}">
+								{{if eq $reviewStatus.Status "rejected"}}
+									{{$.i18n.Tr "repo.pulls.review_rejected" $reviewStatus.Count | Safe}}
+								{{else if eq $reviewStatus.Status "required"}}
+									{{$.i18n.Tr "repo.pulls.review_required" $reviewStatus.Count | Safe}}
+								{{else}}
+									{{$.i18n.Tr "repo.pulls.review_approved" $reviewStatus.Count | Safe}}
+								{{end}}
+							</a>
+						{{end}}{{end}}
 						{{if .Ref}}
 							<a class="ref" href="{{$.RepoLink}}/src/branch/{{.Ref}}">
 								{{svg "octicon-git-branch" 16}} {{.Ref}}

templates/user/dashboard/issues.tmpl

@@ -124,6 +124,18 @@
 								{{else}}
 									{{$.i18n.Tr .GetLastEventLabelFake $timeStr (.Poster.GetDisplayName|Escape) | Safe}}
 								{{end}}
+								{{if .IsPull }}{{if .PullRequest.RequiresApproval }}
+									{{ $reviewStatus := .PullRequest.GetReviewStatus }}
+									<a class="milestone" href="{{AppSubUrl}}/{{.Repo.Owner.Name}}/{{.Repo.Name}}/pulls/{{.Index}}">
+										{{if eq $reviewStatus.Status "rejected"}}
+											{{$.i18n.Tr "repo.pulls.review_rejected" $reviewStatus.Count | Safe}}
+										{{else if eq $reviewStatus.Status "required"}}
+											{{$.i18n.Tr "repo.pulls.review_required" $reviewStatus.Count | Safe}}
+										{{else}}
+											{{$.i18n.Tr "repo.pulls.review_approved" $reviewStatus.Count | Safe}}
+										{{end}}
+									</a>
+								{{end}}{{end}}
 								{{if .Milestone}}
 									<a class="milestone" href="{{AppSubUrl}}/{{.Repo.Owner.Name}}/{{.Repo.Name}}{{$.Link}}?q={{$.Keyword}}&type={{$.ViewType}}&state={{$.State}}&labels={{$.SelectLabels}}&milestone={{.Milestone.ID}}&assignee={{$.AssigneeID}}">
 										{{svg "octicon-milestone" 16}} {{.Milestone.Name}}