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
This commit is contained in:
		
							parent
							
								
									f422a115f4
								
							
						
					
					
						commit
						80db44267c
					
				
					 8 changed files with 154 additions and 4 deletions
				
			
		|  | @ -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 | ||||
| } | ||||
|  |  | |||
|  | @ -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 { | ||||
| 
 | ||||
|  |  | |||
|  | @ -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. | ||||
|  |  | |||
|  | @ -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 | ||||
|  |  | |||
|  | @ -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 | ||||
|  |  | |||
|  | @ -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}} | ||||
|  |  | |||
|  | @ -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}} | ||||
|  |  | |||
|  | @ -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}} | ||||
|  |  | |||
		Loading…
	
		Reference in a new issue