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 1 commit
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
Prev Previous commit
Next Next commit
Remove extra property on ProtectedBranch for PR approvals
  • Loading branch information
James Lakin committed Nov 1, 2019
commit 43b3e7df98827c6378b0572f4ddfd7ab1f89a9bd
23 changes: 11 additions & 12 deletions models/branches.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,18 +31,17 @@ type ProtectedBranch struct {
BranchName string `xorm:"UNIQUE(s)"`
CanPush bool `xorm:"NOT NULL DEFAULT false"`
EnableWhitelist bool
WhitelistUserIDs []int64 `xorm:"JSON TEXT"`
WhitelistTeamIDs []int64 `xorm:"JSON TEXT"`
EnableMergeWhitelist bool `xorm:"NOT NULL DEFAULT false"`
WhitelistDeployKeys bool `xorm:"NOT NULL DEFAULT false"`
MergeWhitelistUserIDs []int64 `xorm:"JSON TEXT"`
MergeWhitelistTeamIDs []int64 `xorm:"JSON TEXT"`
EnableStatusCheck bool `xorm:"NOT NULL DEFAULT false"`
StatusCheckContexts []string `xorm:"JSON TEXT"`
ApprovalsWhitelistUserIDs []int64 `xorm:"JSON TEXT"`
ApprovalsWhitelistTeamIDs []int64 `xorm:"JSON TEXT"`
RequiredApprovals int64 `xorm:"NOT NULL DEFAULT 0"`
GrantedApprovalsCount int64
WhitelistUserIDs []int64 `xorm:"JSON TEXT"`
WhitelistTeamIDs []int64 `xorm:"JSON TEXT"`
EnableMergeWhitelist bool `xorm:"NOT NULL DEFAULT false"`
WhitelistDeployKeys bool `xorm:"NOT NULL DEFAULT false"`
MergeWhitelistUserIDs []int64 `xorm:"JSON TEXT"`
MergeWhitelistTeamIDs []int64 `xorm:"JSON TEXT"`
EnableStatusCheck bool `xorm:"NOT NULL DEFAULT false"`
StatusCheckContexts []string `xorm:"JSON TEXT"`
ApprovalsWhitelistUserIDs []int64 `xorm:"JSON TEXT"`
ApprovalsWhitelistTeamIDs []int64 `xorm:"JSON TEXT"`
RequiredApprovals int64 `xorm:"NOT NULL DEFAULT 0"`
CreatedUnix timeutil.TimeStamp `xorm:"created"`
UpdatedUnix timeutil.TimeStamp `xorm:"updated"`
}
Expand Down
15 changes: 11 additions & 4 deletions 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 @@ -234,10 +236,13 @@ func issues(ctx *context.Context, milestoneID int64, isPullOption util.OptionalB
}
pull := issues[i].PullRequest

if err := pull.LoadProtectedBranch(); err == nil {
if pull.ProtectedBranch != nil && pull.ProtectedBranch.RequiredApprovals != 0 {
pull.ProtectedBranch.GrantedApprovalsCount = pull.ProtectedBranch.GetGrantedApprovalsCount(pull)
}
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

@davidsvantesson davidsvantesson Dec 4, 2019

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()
Expand All @@ -246,6 +251,8 @@ func issues(ctx *context.Context, milestoneID int64, isPullOption util.OptionalB

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: 2 additions & 4 deletions templates/repo/issue/list.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,10 +223,8 @@
{{end}}

{{if .IsPull}}
Copy link
Member

@lafriks lafriks Oct 31, 2019

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

@jamesorlakin jamesorlakin Oct 31, 2019

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

@jamesorlakin jamesorlakin Nov 1, 2019

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

@6543 6543 Nov 2, 2019

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

@davidsvantesson davidsvantesson Dec 4, 2019

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 .PullRequest.ProtectedBranch}}
{{if .PullRequest.ProtectedBranch.RequiredApprovals}}
<span class="comment ui right"><i class="octicon octicon-eye"></i> {{.PullRequest.ProtectedBranch.GrantedApprovalsCount}} / {{.PullRequest.ProtectedBranch.RequiredApprovals}}</span>
{{end}}
{{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}}

Expand Down