Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Approval Counts to pulls list #10238

Merged
merged 8 commits into from
Mar 6, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Add Approval Counts to pulls list
Add simple approvals counts to pulls lists
  • Loading branch information
zeripath committed Feb 17, 2020
commit cf2e82ee51eb08daceeb854cc8f4ec0019fc1536
33 changes: 33 additions & 0 deletions models/issue_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -515,3 +515,36 @@ 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
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, official, count(id) as `count`").GroupBy("issue_id, type, official").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
}
19 changes: 19 additions & 0 deletions models/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,25 @@ func (pr *PullRequest) GetCommitMessages() string {
return stringBuilder.String()
}

// ReviewCount represents a count of Reviews
type ReviewCount struct {
IssueID int64
Type ReviewType
Official bool
Count int64
}

// GetApprovalCounts returns the approval counts by type
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, official, count(id) as `count`").GroupBy("issue_id, type, official").Table("review").Find(&rCounts)
}

// GetApprovers returns the approvers of the pull request
func (pr *PullRequest) GetApprovers() string {

Expand Down
9 changes: 9 additions & 0 deletions options/locale/locale_en-US.ini
Original file line number Diff line number Diff line change
Expand Up @@ -1071,6 +1071,15 @@ 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.approve_non_official_count_1 = "+%d non-official"
pulls.approve_non_official_count_n = "+%d non-official"
pulls.reject_count_1 = "%d change request"
pulls.reject_count_n = "%d change requests"
pulls.reject_non_official_count_1 = "+%d non-official"
pulls.reject_non_official_count_n = "+%d non-official"

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.
Expand Down
22 changes: 22 additions & 0 deletions routers/repo/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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, official bool) 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 && count.Official == official {
return count.Count
}
}
return 0
}
ctx.Data["IssueStats"] = issueStats
ctx.Data["SelLabelIDs"] = labelIDs
ctx.Data["SelectLabels"] = selectLabels
Expand Down
22 changes: 22 additions & 0 deletions routers/user/home.go
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,12 @@ func Issues(ctx *context.Context) {
return
}

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 {
Expand Down Expand Up @@ -577,6 +583,22 @@ func Issues(ctx *context.Context) {
}

ctx.Data["Issues"] = issues
ctx.Data["ApprovalCounts"] = func(issueID int64, typ string, official bool) 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 && count.Official == official {
return count.Count
}
}
return 0
}
ctx.Data["CommitStatus"] = commitStatus
ctx.Data["Repos"] = showRepos
ctx.Data["Counts"] = counts
Expand Down
15 changes: 15 additions & 0 deletions templates/repo/issue/list.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@
</div>

<div class="issue list">
{{ $approvalCounts := .ApprovalCounts}}
{{range .Issues}}
<li class="item">
{{if $.CanWriteIssuesOrPulls}}
Expand Down Expand Up @@ -268,6 +269,20 @@
</a>
{{end}}
{{if .IsPull}}
{{$approveOfficial := call $approvalCounts .ID "approve" true}}
{{$approveNonOfficial := call $approvalCounts .ID "approve" false}}
{{$rejectOfficial := call $approvalCounts .ID "reject" true}}
{{$rejectNonOfficial := call $approvalCounts .ID "reject" false}}
{{if or (gt $approveOfficial 0) (gt $approveNonOfficial 0) (gt $rejectOfficial 0) (gt $rejectNonOfficial 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 gt $approveNonOfficial 0}}({{$.i18n.Tr (TrN $.i18n.Lang $approveNonOfficial "repo.pulls.approve_non_official_count_1" "repo.pulls.approve_non_official_count_n") $approveNonOfficial}}){{end}}</span>
{{if or (gt $rejectOfficial 0) (gt $rejectNonOfficial 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}}
{{if gt $rejectNonOfficial 0}}({{$.i18n.Tr (TrN $.i18n.Lang $rejectNonOfficial "repo.pulls.reject_non_official_count_1" "repo.pulls.reject_non_official_count_n") $rejectNonOfficial}}){{end}}</span>
{{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}}
Expand Down
15 changes: 15 additions & 0 deletions templates/user/dashboard/issues.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@
</div>

<div class="issue list">
{{ $approvalCounts := .ApprovalCounts}}
{{range .Issues}}

{{ $timeStr:= TimeSinceUnix .CreatedUnix $.Lang }}
Expand Down Expand Up @@ -152,6 +153,20 @@
</span>
{{end}}
{{if .IsPull}}
{{$approveOfficial := call $approvalCounts .ID "approve" true}}
{{$approveNonOfficial := call $approvalCounts .ID "approve" false}}
{{$rejectOfficial := call $approvalCounts .ID "reject" true}}
{{$rejectNonOfficial := call $approvalCounts .ID "reject" false}}
{{if or (gt $approveOfficial 0) (gt $approveNonOfficial 0) (gt $rejectOfficial 0) (gt $rejectNonOfficial 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 gt $approveNonOfficial 0}}({{$.i18n.Tr (TrN $.i18n.Lang $approveNonOfficial "repo.pulls.approve_non_official_count_1" "repo.pulls.approve_non_official_count_n") $approveNonOfficial}}){{end}}</span>
{{if or (gt $rejectOfficial 0) (gt $rejectNonOfficial 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}}
{{if gt $rejectNonOfficial 0}}({{$.i18n.Tr (TrN $.i18n.Lang $rejectNonOfficial "repo.pulls.reject_non_official_count_1" "repo.pulls.reject_non_official_count_n") $rejectNonOfficial}}){{end}}</span>
{{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}}
Expand Down