Add Approval Counts to pulls list (#10238)

* Add Approval Counts to pulls list

Add simple approvals counts to pulls lists

* Remove non-official counts

* Add PR features to milestone_issues.tmpl

8 files changed, 154 insertions, 4 deletions

diff --git a/models/issue_list.go b/models/issue_list.go
index 4554f906c4..e6ca3fc890 100644
--- a/models/issue_list.go
+++ b/models/issue_list.go
@@ -515,3 +515,37 @@ func (issues IssueList) LoadComments() error {
 func (issues IssueList) LoadDiscussComments() error {
 	return issues.loadComments(x, builder.Eq{"comment.type": CommentTypeComment})
 }
+
+// GetApprovalCounts returns a map of issue ID to slice of approval counts
+// FIXME: only returns official counts due to double counting of non-official approvals
+func (issues IssueList) GetApprovalCounts() (map[int64][]*ReviewCount, error) {
+	return issues.getApprovalCounts(x)
+}
+
+func (issues IssueList) getApprovalCounts(e Engine) (map[int64][]*ReviewCount, error) {
+	rCounts := make([]*ReviewCount, 0, 6*len(issues))
+	ids := make([]int64, len(issues))
+	for i, issue := range issues {
+		ids[i] = issue.ID
+	}
+	sess := e.In("issue_id", ids)
+	err := sess.Select("issue_id, type, count(id) as `count`").Where("official = ?", true).GroupBy("issue_id, type").OrderBy("issue_id").Table("review").Find(&rCounts)
+	if err != nil {
+		return nil, err
+	}
+
+	approvalCountMap := make(map[int64][]*ReviewCount, len(issues))
+	if len(rCounts) > 0 {
+		start := 0
+		lastID := rCounts[0].IssueID
+		for i, current := range rCounts[1:] {
+			if lastID != current.IssueID {
+				approvalCountMap[lastID] = rCounts[start:i]
+				start = i
+				lastID = current.IssueID
+			}
+		}
+		approvalCountMap[lastID] = rCounts[start:]
+	}
+	return approvalCountMap, nil
+}
diff --git a/models/pull.go b/models/pull.go
index 3e177ea5e9..48d2340544 100644
--- a/models/pull.go
+++ b/models/pull.go
@@ -352,6 +352,25 @@ func (pr *PullRequest) GetCommitMessages() string {
 	return stringBuilder.String()
 }
 
+// ReviewCount represents a count of Reviews
+type ReviewCount struct {
+	IssueID int64
+	Type    ReviewType
+	Count   int64
+}
+
+// GetApprovalCounts returns the approval counts by type
+// FIXME: Only returns official counts due to double counting of non-official counts
+func (pr *PullRequest) GetApprovalCounts() ([]*ReviewCount, error) {
+	return pr.getApprovalCounts(x)
+}
+
+func (pr *PullRequest) getApprovalCounts(e Engine) ([]*ReviewCount, error) {
+	rCounts := make([]*ReviewCount, 0, 6)
+	sess := e.Where("issue_id = ?", pr.IssueID)
+	return rCounts, sess.Select("issue_id, type, count(id) as `count`").Where("official = ?", true).GroupBy("issue_id, type").Table("review").Find(&rCounts)
+}
+
 // GetApprovers returns the approvers of the pull request
 func (pr *PullRequest) GetApprovers() string {
 
diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini
index 375a2ec88d..ea4eccac8d 100644
--- a/options/locale/locale_en-US.ini
+++ b/options/locale/locale_en-US.ini
@@ -1082,6 +1082,11 @@ pulls.cannot_auto_merge_desc = This pull request cannot be merged automatically
 pulls.cannot_auto_merge_helper = Merge manually to resolve the conflicts.
 pulls.num_conflicting_files_1 = "%d conflicting file"
 pulls.num_conflicting_files_n = "%d conflicting files"
+pulls.approve_count_1 = "%d approval"
+pulls.approve_count_n = "%d approvals"
+pulls.reject_count_1 = "%d change request"
+pulls.reject_count_n = "%d change requests"
+
 pulls.no_merge_desc = This pull request cannot be merged because all repository merge options are disabled.
 pulls.no_merge_helper = Enable merge options in the repository settings or merge the pull request manually.
 pulls.no_merge_wip = This pull request can not be merged because it is marked as being a work in progress.
diff --git a/routers/repo/issue.go b/routers/repo/issue.go
index a4cd184c15..a8406d3154 100644
--- a/routers/repo/issue.go
+++ b/routers/repo/issue.go
@@ -216,6 +216,12 @@ func issues(ctx *context.Context, milestoneID int64, isPullOption util.OptionalB
 		}
 	}
 
+	approvalCounts, err := models.IssueList(issues).GetApprovalCounts()
+	if err != nil {
+		ctx.ServerError("ApprovalCounts", err)
+		return
+	}
+
 	var commitStatus = make(map[int64]*models.CommitStatus, len(issues))
 
 	// Get posters.
@@ -263,6 +269,22 @@ func issues(ctx *context.Context, milestoneID int64, isPullOption util.OptionalB
 		assigneeID = 0 // Reset ID to prevent unexpected selection of assignee.
 	}
 
+	ctx.Data["ApprovalCounts"] = func(issueID int64, typ string) int64 {
+		counts, ok := approvalCounts[issueID]
+		if !ok || len(counts) == 0 {
+			return 0
+		}
+		reviewTyp := models.ReviewTypeApprove
+		if typ == "reject" {
+			reviewTyp = models.ReviewTypeReject
+		}
+		for _, count := range counts {
+			if count.Type == reviewTyp {
+				return count.Count
+			}
+		}
+		return 0
+	}
 	ctx.Data["IssueStats"] = issueStats
 	ctx.Data["SelLabelIDs"] = labelIDs
 	ctx.Data["SelectLabels"] = selectLabels
diff --git a/routers/user/home.go b/routers/user/home.go
index 762fcca156..e310b97ad6 100644
--- a/routers/user/home.go
+++ b/routers/user/home.go
@@ -528,6 +528,12 @@ func Issues(ctx *context.Context) {
 		issues = []*models.Issue{}
 	}
 
+	approvalCounts, err := models.IssueList(issues).GetApprovalCounts()
+	if err != nil {
+		ctx.ServerError("ApprovalCounts", err)
+		return
+	}
+
 	showReposMap := make(map[int64]*models.Repository, len(counts))
 	for repoID := range counts {
 		if repoID > 0 {
@@ -639,6 +645,22 @@ func Issues(ctx *context.Context) {
 	}
 
 	ctx.Data["Issues"] = issues
+	ctx.Data["ApprovalCounts"] = func(issueID int64, typ string) int64 {
+		counts, ok := approvalCounts[issueID]
+		if !ok || len(counts) == 0 {
+			return 0
+		}
+		reviewTyp := models.ReviewTypeApprove
+		if typ == "reject" {
+			reviewTyp = models.ReviewTypeReject
+		}
+		for _, count := range counts {
+			if count.Type == reviewTyp {
+				return count.Count
+			}
+		}
+		return 0
+	}
 	ctx.Data["CommitStatus"] = commitStatus
 	ctx.Data["Repos"] = showRepos
 	ctx.Data["Counts"] = counts
diff --git a/templates/repo/issue/list.tmpl b/templates/repo/issue/list.tmpl
index 60644409c3..05018bacdd 100644
--- a/templates/repo/issue/list.tmpl
+++ b/templates/repo/issue/list.tmpl
@@ -202,6 +202,7 @@
 		</div>
 
 		<div class="issue list">
+			{{ $approvalCounts := .ApprovalCounts}}
 			{{range .Issues}}
 				<li class="item">
 					{{if $.CanWriteIssuesOrPulls}}
@@ -268,6 +269,16 @@
 							</a>
 						{{end}}
 						{{if .IsPull}}
+							{{$approveOfficial := call $approvalCounts .ID "approve"}}
+							{{$rejectOfficial := call $approvalCounts .ID "reject"}}
+							{{if or (gt $approveOfficial 0) (gt $rejectOfficial 0)}}
+								<span class="approvals">{{svg "octicon-check" 16}}
+									{{$.i18n.Tr (TrN $.i18n.Lang $approveOfficial "repo.pulls.approve_count_1" "repo.pulls.approve_count_n") $approveOfficial}}
+								{{if or (gt $rejectOfficial 0)}}
+									<span class="rejects">{{svg "octicon-x" 16}}
+										{{$.i18n.Tr (TrN $.i18n.Lang $rejectOfficial "repo.pulls.reject_count_1" "repo.pulls.reject_count_n") $rejectOfficial}}
+								{{end}}
+							{{end}}
 							{{if and (not .PullRequest.HasMerged) (gt (len .PullRequest.ConflictedFiles) 0)}}
 								<span class="conflicting">{{svg "octicon-mirror" 16}} {{$.i18n.Tr (TrN $.i18n.Lang (len .PullRequest.ConflictedFiles) "repo.pulls.num_conflicting_files_1" "repo.pulls.num_conflicting_files_n") (len .PullRequest.ConflictedFiles)}}</span>
 							{{end}}
diff --git a/templates/repo/issue/milestone_issues.tmpl b/templates/repo/issue/milestone_issues.tmpl
index 49712b1d09..6d08df87df 100644
--- a/templates/repo/issue/milestone_issues.tmpl
+++ b/templates/repo/issue/milestone_issues.tmpl
@@ -177,6 +177,7 @@
 		</div>
 
 		<div class="issue list">
+			{{ $approvalCounts := .ApprovalCounts}}
 			{{range .Issues}}
 				{{ $timeStr:= TimeSinceUnix .CreatedUnix $.Lang }}
 				<li class="item">
@@ -185,9 +186,15 @@
 						<input type="checkbox" data-issue-id={{.ID}}></input>
 					</div>
 					{{end}}
-					<div class="ui {{if .IsRead}}gray{{else}}green{{end}} label">#{{.Index}}</div>
+					<div class="ui {{if .IsClosed}}{{if .IsPull}}{{if .PullRequest.HasMerged}}purple{{else}}red{{end}}{{else}}red{{end}}{{else}}{{if .IsRead}}white{{else}}green{{end}}{{end}} label">#{{.Index}}</div>
 					<a class="title has-emoji" href="{{$.RepoLink}}/issues/{{.Index}}">{{.Title}}</a>
 
+					{{if .IsPull }}
+						{{if (index $.CommitStatus .PullRequest.ID)}}
+							{{template "repo/commit_status" (index $.CommitStatus .PullRequest.ID)}}
+						{{end}}
+					{{end}}
+
 					{{range .Labels}}
 						<a class="ui label has-emoji" href="{{$.Link}}?q={{$.Keyword}}&type={{$.ViewType}}&state={{$.State}}&labels={{.ID}}&assignee={{$.AssigneeID}}" style="color: {{.ForegroundColor}}; background-color: {{.Color}}" title="{{.Description}}">{{.Name}}</a>
 					{{end}}
@@ -201,11 +208,15 @@
 					{{end}}
 
 					<p class="desc">
-						{{if gt .Poster.ID 0}}
-							{{$.i18n.Tr .GetLastEventLabel $timeStr .Poster.HomeLink (.Poster.GetDisplayName|Escape) | Safe}}
+						{{ $timeStr := TimeSinceUnix .GetLastEventTimestamp $.Lang }}
+						{{if .OriginalAuthor }}
+							{{$.i18n.Tr .GetLastEventLabelFake $timeStr .OriginalAuthor | Safe}}
+						{{else if gt .Poster.ID 0}}
+							{{$.i18n.Tr .GetLastEventLabel $timeStr .Poster.HomeLink (.Poster.GetDisplayName | Escape) | Safe}}
 						{{else}}
-							{{$.i18n.Tr .GetLastEventLabelFake $timeStr (.Poster.GetDisplayName|Escape) | Safe}}
+							{{$.i18n.Tr .GetLastEventLabelFake $timeStr (.Poster.GetDisplayName | Escape) | Safe}}
 						{{end}}
+
 						{{if .Ref}}
 							<a class="ref" href="{{$.RepoLink}}/src/branch/{{.Ref}}">
 								{{svg "octicon-git-branch" 16}} {{.Ref}}
@@ -227,6 +238,21 @@
 								<img class="ui avatar image" src="{{.RelAvatarLink}}">
 							</a>
 						{{end}}
+						{{if .IsPull}}
+							{{$approveOfficial := call $approvalCounts .ID "approve"}}
+							{{$rejectOfficial := call $approvalCounts .ID "reject"}}
+							{{if or (gt $approveOfficial 0) (gt $rejectOfficial 0)}}
+								<span class="approvals">{{svg "octicon-check" 16}}
+									{{$.i18n.Tr (TrN $.i18n.Lang $approveOfficial "repo.pulls.approve_count_1" "repo.pulls.approve_count_n") $approveOfficial}}
+								{{if or (gt $rejectOfficial 0)}}
+									<span class="rejects">{{svg "octicon-x" 16}}
+										{{$.i18n.Tr (TrN $.i18n.Lang $rejectOfficial "repo.pulls.reject_count_1" "repo.pulls.reject_count_n") $rejectOfficial}}
+								{{end}}
+							{{end}}
+							{{if and (not .PullRequest.HasMerged) ((len .PullRequest.ConflictedFiles) gt 0)}}
+								<span class="conflicting">{{svg "octicon-mirror" 16}} {{$.i18n.Tr (TrN $.i18n.Lang (len .PullRequest.ConflictedFiles) "repo.pulls.num_conflicting_files_1" "repo.pulls.num_conflicting_files_n") (len .PullRequest.ConflictedFiles)}}</span>
+							{{end}}
+						{{end}}
 					</p>
 				</li>
 			{{end}}
diff --git a/templates/user/dashboard/issues.tmpl b/templates/user/dashboard/issues.tmpl
index de578b2820..447801d849 100644
--- a/templates/user/dashboard/issues.tmpl
+++ b/templates/user/dashboard/issues.tmpl
@@ -101,6 +101,7 @@
 				</div>
 
 				<div class="issue list">
+					{{ $approvalCounts := .ApprovalCounts}}
 					{{range .Issues}}
 
 						{{ $timeStr:= TimeSinceUnix .CreatedUnix $.Lang }}
@@ -170,6 +171,16 @@
 									</span>
 								{{end}}
 								{{if .IsPull}}
+									{{$approveOfficial := call $approvalCounts .ID "approve"}}
+									{{$rejectOfficial := call $approvalCounts .ID "reject"}}
+									{{if or (gt $approveOfficial 0) (gt $rejectOfficial 0) }}
+										<span class="approvals">{{svg "octicon-check" 16}}
+											{{$.i18n.Tr (TrN $.i18n.Lang $approveOfficial "repo.pulls.approve_count_1" "repo.pulls.approve_count_n") $approveOfficial}}
+										{{if or (gt $rejectOfficial 0)}}
+											<span class="rejects">{{svg "octicon-x" 16}}
+												{{$.i18n.Tr (TrN $.i18n.Lang $rejectOfficial "repo.pulls.reject_count_1" "repo.pulls.reject_count_n") $rejectOfficial}}
+										{{end}}
+									{{end}}
 									{{if and (not .PullRequest.HasMerged) (gt (len .PullRequest.ConflictedFiles) 0)}}
 										<span class="conflicting">{{svg "octicon-mirror" 16}} {{$.i18n.Tr (TrN $.i18n.Lang (len .PullRequest.ConflictedFiles) "repo.pulls.num_conflicting_files_1" "repo.pulls.num_conflicting_files_n") (len .PullRequest.ConflictedFiles)}}</span>
 									{{end}}