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 option to expand reviewers in details UI #656
Conversation
If the `options.expand_required_reviewers` server option is set, when a user expands a rule in the details UI, policy-bot will expand the teams, organizations, and permission levels that can approve that rule in to a list of user names. This can make it easier to figure out who needs to approve your PR. This feature is disabled by default because it can expose otherwise private information about membership. Only enable it in trusted environments. This uses the HTMX library to handle loading user lists in the frontend on demand, instead of precomputing all of the information everytime someone views the details of a PR.
|
||
owner := pat.Param(r, "owner") | ||
repo := pat.Param(r, "repo") | ||
var data struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This diff is weird, but everything after this line in this function is the same as before, except for some variable names. I extracted all the stuff that came before this in to helper functions for reuse.
@@ -73,6 +80,8 @@ func (p *PullEvaluationOptions) SetValuesFromEnv(prefix string) { | |||
setStringFromEnv("SHARED_REPOSITORY", prefix, &p.SharedRepository) | |||
setStringFromEnv("SHARED_POLICY_PATH", prefix, &p.SharedPolicyPath) | |||
setStringFromEnv("STATUS_CHECK_CONTEXT", prefix, &p.StatusCheckContext) | |||
setBoolFromEnv("EXPAND_REQUIRED_REVIEWERS", prefix, &p.ExpandRequiredReviewers) | |||
setBoolFromEnv("POST_INSECURE_STATUS_CHECKS", prefix, &p.PostInsecureStatusChecks) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume that this is an existing setting that's missing from the env vars?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I figured I would add it for consistency now that there's a setBoolFromEnv
function
If the
options.expand_required_reviewers
server option is set, when a user expands a rule in the details UI, policy-bot will expand the teams, organizations, and permission levels that can approve that rule in to a list of user names. This can make it easier to figure out who needs to approve your PR.This feature is disabled by default because it can expose otherwise private information about membership. Only enable it in trusted environments.
This uses the HTMX library to handle loading user lists in the frontend on demand, instead of precomputing all of the information every time someone views the details of a PR. For now, I've committed a minified copy of the library, but we'll probably want to install it with
yarn
in the future.Here's a quick screen capture that shows both the feature and the error behavior:
Since this is purely information, if there are any problems loading information, we warn the user that results may be incomplete but otherwise don't fail.
I'd also like to revisit the layout of users, since a single list will be pretty long if there are a lot of users. I think columns would work, but I was wasting too much time trying to make a column layout that didn't fall apart with really long usernames. I'll revisit this either before this PR merges or in a follow-up.
Closes #49.