Skip to content

Show approvals on the PR listing page #8762

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

Show approvals on the PR listing page #8762

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a25f336
Show approvals on the PR list page
Oct 31, 2019
43b3e7d
Remove extra property on ProtectedBranch for PR approvals
Nov 1, 2019
ade2ffe
Merge branch 'master' into PRListApprovals
jamesorlakin Nov 1, 2019
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
16 changes: 15 additions & 1 deletion routers/repo/issue.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,8 @@ func issues(ctx *context.Context, milestoneID int64, isPullOption util.OptionalB
}

var commitStatus = make(map[int64]*models.CommitStatus, len(issues))
var requiredApprovals = make(map[int64]int64, len(issues))
var approvals = make(map[int64]int64, len(issues))

// Get posters.
for i := range issues {
Expand All @@ -232,13 +234,25 @@ func issues(ctx *context.Context, milestoneID int64, isPullOption util.OptionalB
ctx.ServerError("LoadPullRequest", err)
return
}
pull := issues[i].PullRequest

commitStatus[issues[i].PullRequest.ID], _ = issues[i].PullRequest.GetLastCommitStatus()
if err := pull.LoadProtectedBranch(); err != nil {
return
}

if pull.ProtectedBranch != nil && pull.ProtectedBranch.RequiredApprovals != 0 {
requiredApprovals[pull.ID] = pull.ProtectedBranch.RequiredApprovals
approvals[pull.ID] = pull.ProtectedBranch.GetGrantedApprovalsCount(pull)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Set value in slice also when branch is not protected? (with 0 value)


commitStatus[pull.ID], _ = pull.GetLastCommitStatus()
}
}

ctx.Data["Issues"] = issues
ctx.Data["CommitStatus"] = commitStatus
ctx.Data["RequiredApprovals"] = requiredApprovals
ctx.Data["Approvals"] = approvals

// Get assignees.
ctx.Data["Assignees"], err = repo.GetAssignees()
Expand Down
6 changes: 6 additions & 0 deletions templates/repo/issue/list.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,12 @@
<a class="ui label has-emoji" href="{{$.Link}}?q={{$.Keyword}}&type={{$.ViewType}}&state={{$.State}}&labels={{.ID}}&milestone={{$.MilestoneID}}&assignee={{$.AssigneeID}}" style="color: {{.ForegroundColor}}; background-color: {{.Color}}" title="{{.Description}}">{{.Name}}</a>
{{end}}

{{if .IsPull}}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can be reduced to one if statement:

Suggested change
{{if .IsPull}}
{{if and .IsPull .PullRequest.ProtectedBranch .PullRequest.ProtectedBranch.RequiredApprovals}}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aah that's much simpler, will do

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately this didn't work for branches without protection enabled

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{{if and .IsPull (if ne (index $.RequiredApprovals .PullRequest.ID) 0)}} ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose that requires that $.RequiredApprovals contain all indexes? Should be possible by setting them 0 in issue.go.

{{if ne (index $.RequiredApprovals .PullRequest.ID) 0}}
<span class="comment ui right"><i class="octicon octicon-eye"></i> {{(index $.Approvals .PullRequest.ID)}} / {{(index $.RequiredApprovals .PullRequest.ID)}}</span>
{{end}}
{{end}}

{{if .NumComments}}
<span class="comment ui right"><i class="octicon octicon-comment"></i> {{.NumComments}}</span>
{{end}}
Expand Down