Skip to content

Commit

Permalink
Improve details view for required approvals (#494)
Browse files Browse the repository at this point in the history 
This makes several improvements to the rule details UI for required
approvals and methods:

* Hide details about required approvals and methods when rules are
  skipped or when they require no approval

* Combine the required approval count with the header for approver
  details instead of using a separate section.

* Combine membership requirements and permission requirements in to a
  single section

* Update the phrasing of section headers for readability
  • Loading branch information
@bluekeyes
bluekeyes authored Nov 8, 2022
1 parent e3f46af commit 32b43eb
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 48 deletions.
31 changes: 23 additions & 8 deletions server/handler/frontend.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package handler

import (
"fmt"
"html/template"
"net/http"
"path"
Expand Down Expand Up @@ -95,40 +96,54 @@ func Static(prefix string, c *FilesConfig) http.Handler {
}

func getMethods(result *common.Result) map[string][]string {
const (
commentKey = "Comments containing"
commentPatternKey = "Comments matching patterns"
bodyPatternKey = "The pull request body matching patterns"
reviewKey = "GitHub reviews with status"
)

patternInfo := make(map[string][]string)
for _, comment := range result.Methods.Comments {
patternInfo["Comments"] = append(patternInfo["Comments"], comment)
patternInfo[commentKey] = append(patternInfo[commentKey], comment)
}
for _, commentPattern := range result.Methods.CommentPatterns {
patternInfo["Comment Patterns"] = append(patternInfo["Comment Patterns"], commentPattern.String())
patternInfo[commentPatternKey] = append(patternInfo[commentPatternKey], commentPattern.String())
}
for _, bodyPattern := range result.Methods.BodyPatterns {
patternInfo["Body Patterns"] = append(patternInfo["Body Patterns"], bodyPattern.String())
patternInfo[bodyPatternKey] = append(patternInfo[bodyPatternKey], bodyPattern.String())
}
if result.Methods.GithubReview != nil && *result.Methods.GithubReview {
reviewPatternKey := reviewKey + fmt.Sprintf(" %s matching patterns", result.Methods.GithubReviewState)
if len(result.Methods.GithubReviewCommentPatterns) > 0 {
for _, githubReviewCommentPattern := range result.Methods.GithubReviewCommentPatterns {
patternInfo["Github Review Comment Patterns + Github Review Approval"] = append(patternInfo["Github Review Comment Patterns + Github Review Approval"], githubReviewCommentPattern.String())
patternInfo[reviewPatternKey] = append(patternInfo[reviewPatternKey], githubReviewCommentPattern.String())
}
} else {
patternInfo["Github Review State"] = append(patternInfo["Github Review State"], string(result.Methods.GithubReviewState))
patternInfo[reviewKey] = append(patternInfo[reviewKey], string(result.Methods.GithubReviewState))
}
}
return patternInfo
}

func getActors(result *common.Result, githubURL string) map[string][]Membership {
const (
orgKey = "Members of the organizations"
teamKey = "Members of the teams"
userKey = "Users"
)

membershipInfo := make(map[string][]Membership)
for _, org := range result.Requires.Actors.Organizations {
membershipInfo["Organizations"] = append(membershipInfo["Organizations"], Membership{Name: org, Link: githubURL + "/orgs/" + org + "/people"})
membershipInfo[orgKey] = append(membershipInfo[orgKey], Membership{Name: org, Link: githubURL + "/orgs/" + org + "/people"})
}
for _, team := range result.Requires.Actors.Teams {
teamName := strings.Split(team, "/")
membershipInfo["Teams"] = append(membershipInfo["Teams"], Membership{Name: team, Link: githubURL + "/orgs/" + teamName[0] + "/teams/" + teamName[1] + "/members"})
membershipInfo[teamKey] = append(membershipInfo[teamKey], Membership{Name: team, Link: githubURL + "/orgs/" + teamName[0] + "/teams/" + teamName[1] + "/members"})

}
for _, user := range result.Requires.Actors.Users {
membershipInfo["Users"] = append(membershipInfo["Users"], Membership{Name: user, Link: githubURL + "/" + user})
membershipInfo[userKey] = append(membershipInfo[userKey], Membership{Name: user, Link: githubURL + "/" + user})
}
return membershipInfo
}
Expand Down
80 changes: 40 additions & 40 deletions server/templates/details.html.tmpl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,27 +60,25 @@
<details class="bg-light-gray5 p-2 mt-2 text-sm">
<summary class="text-dark-gray3 font-semibold">Details</summary>
<div class="border-t border-light-gray3 mt-2 overflow-x-auto whitespace-nowrap">
{{if .PredicateResults}}
{{if .PredicateResults}}
<div class="pt-2">
{{template "result-predicates-details" .}}
</div>
{{end}}
<div class="pt-2">
{{template "result-requires-count" .}}
</div>
{{if (hasActors .Requires)}}
<div class="pt-2">
{{template "result-actors-details" .}}
</div>
{{end}}
{{if (hasActorsPermissions .Requires)}}
<div class="pt-2">
{{template "result-actors-permissions-details" .}}
</div>
{{if ne $s "skipped"}}{{/* only show approval details for active rules */}}
{{if gt .Requires.Count 0 }}{{/* only show approval details if they're required */}}
<div class="pt-2">
{{template "result-approver-details" .}}
</div>
<div class="pt-2">
{{template "result-methods-details" .}}
</div>
{{else}}
<div class="pt-2">
<b class="font-bold text-sm">This rule is automatically approved and requires no reviews</b>
</div>
{{end}}
{{end}}
<div class="pt-2">
{{template "result-methods-details" .}}
</div>
</div>
</details>
{{end}}
Expand Down Expand Up @@ -148,40 +146,42 @@
</ul>
{{end}}

{{define "result-requires-count"}}
<b class="font-bold text-sm">This rule requires at least this many reviews</b>
<dl class="my-2">
<dt> {{.Requires.Count}}</dt>
</dl>
{{end}}

{{define "result-methods-details"}}
<b class="font-bold text-sm">This rule requires review using any of these methods</b>
<b class="font-bold text-sm">Approvals may use any of these methods:</b>
<dl class="my-2">
{{range $k, $v := getMethods .}}
<dt> {{$k}}</dt>
<dt>{{$k}}:</dt>
<dd>
<ul class="list-disc list-outside pl-6 py-2">{{range $v}}<li class="font-mono text-sm-mono">{{.}}</li>{{end}}</ul>
<ul class="list-disc list-outside pl-6 py-2">{{range $v}}<li class="font-mono text-sm-mono">{{.}}</li>{{end}}</ul>
</dd>
{{end}}
</dl>
{{end}}

{{define "result-actors-details"}}
<b class="font-bold text-sm">This rule requires review from</b>
<dl class="my-2">
{{range $k, $v := getActors .}}
<dt> {{$k}}</dt>
<dd>
{{define "result-approver-details"}}
{{$hasActors := hasActors .Requires}}
{{$hasPerms := hasActorsPermissions .Requires}}
{{if or $hasActors $hasPerms}}
<b class="font-bold text-sm">{{template "result-reviews-count" .Requires}} from:</b>
<dl class="my-2">
{{if $hasActors}}
{{range $k, $v := getActors .}}
<dt>{{$k}}:</dt>
<dd>
<ul class="list-disc list-outside pl-6 py-2">{{range $v}}<li class="font-mono text-sm-mono"><a href={{.Link}}>{{.Name}}</a></li>{{end}}</ul>
</dd>
</dd>
{{end}}
{{end}}
{{if $hasPerms}}
<dt>Users with the permissions:</dt>
<dd>
<ul class="list-disc list-outside pl-6 py-2">{{range getPermissions .}}<li class="font-mono text-sm-mono">{{.}}</li>{{end}}</ul>
</dd>
{{end}}
</dl>
{{else}}
<b class="font-bold text-sm">{{template "result-reviews-count" .Requires}}</b>
{{end}}
</dl>
{{end}}

{{define "result-actors-permissions-details"}}
<b class="font-bold text-sm">This rule requires review from users with these permissions</b>
<ul class="list-disc list-outside pl-6 py-2">
{{range getPermissions .}}<li class="font-mono text-sm-mono">{{.}}</li>{{end}}
</ul>
{{end}}
{{define "result-reviews-count"}}This rule requires at least {{.Count}} approval{{if gt .Count 1}}s{{end}}{{end}}

0 comments on commit 32b43eb

Please sign in to comment.