From 779b0f04490943648d7ba3871832111addb3a9dc Mon Sep 17 00:00:00 2001 From: Ashir Amer Date: Fri, 29 Jul 2022 00:00:39 +0000 Subject: [PATCH 1/3] Add a simulate page This allows a user to: - Input a branch that has a policy file. The simulate page will evaluate that policy file on the given PR - In put a user to simulate a fake approval for. The simulate page will evaluate the PR as if the user had submitted an approval - Or both! Abstracted away the majority of functionality into a BaseDetails handler. This allows consumers to submit a policy and a user to override the defaults done by the Details handler. The Details page still exists since it currently sends the information back to GitHub. --- server/assets/js/footer-link.js | 17 ++ server/assets/js/prefill.js | 23 +++ server/handler/base.go | 21 ++- server/handler/base_details.go | 229 ++++++++++++++++++++++++ server/handler/details.go | 157 +--------------- server/handler/fetcher.go | 13 +- server/handler/simulate.go | 85 +++++++++ server/server.go | 24 ++- server/templates/base-details.html.tmpl | 156 ++++++++++++++++ server/templates/details.html.tmpl | 150 ++-------------- server/templates/simulate.html.tmpl | 62 +++++++ 11 files changed, 634 insertions(+), 303 deletions(-) create mode 100644 server/assets/js/footer-link.js create mode 100644 server/assets/js/prefill.js create mode 100644 server/handler/base_details.go create mode 100644 server/handler/simulate.go create mode 100644 server/templates/base-details.html.tmpl create mode 100644 server/templates/simulate.html.tmpl diff --git a/server/assets/js/footer-link.js b/server/assets/js/footer-link.js new file mode 100644 index 00000000..1abef0fd --- /dev/null +++ b/server/assets/js/footer-link.js @@ -0,0 +1,17 @@ +(function() { + function addSimulateRedirect(){ + const buttonId = "details-to-simulate" + function redirectToSimulate(event) { + const currentPath = location.href + let simulatePath = currentPath.replace("details", "simulate") + document.location = simulatePath + } + document.getElementById(buttonId).addEventListener("click", redirectToSimulate) + } + + document.addEventListener('DOMContentLoaded', function() { + addSimulateRedirect() + }, false); + + +})(); \ No newline at end of file diff --git a/server/assets/js/prefill.js b/server/assets/js/prefill.js new file mode 100644 index 00000000..5d02cea7 --- /dev/null +++ b/server/assets/js/prefill.js @@ -0,0 +1,23 @@ +(function() { + function prefillSimulation(){ + function prefillParam(param){ + const queryString = window.location.search; + const urlParams = new URLSearchParams(queryString); + if (urlParams.has(param)) { + let paramValue = urlParams.get(param) + document.getElementById(param).value = paramValue + } + } + + const BRANCH = "branch" + const USERNAME = "username" + + prefillParam(BRANCH) + prefillParam(USERNAME) + } + document.addEventListener('DOMContentLoaded', function() { + prefillSimulation() + }, false); + + +})(); \ No newline at end of file diff --git a/server/handler/base.go b/server/handler/base.go index 6eb544fc..bb03d6de 100644 --- a/server/handler/base.go +++ b/server/handler/base.go @@ -114,7 +114,7 @@ func (b *Base) PostStatus(ctx context.Context, prctx pull.Context, client *githu } publicURL := strings.TrimSuffix(b.BaseConfig.PublicURL, "/") - detailsURL := fmt.Sprintf("%s/details/%s/%s/%d", publicURL, owner, repo, prctx.Number()) + detailsURL := fmt.Sprintf("%s/details/%s/%s/%DetailsData", publicURL, owner, repo, prctx.Number()) contextWithBranch := fmt.Sprintf("%s: %s", b.PullOpts.StatusCheckContext, base) status := &github.RepoStatus{ @@ -233,10 +233,16 @@ func (b *Base) ValidateFetchedConfig(ctx context.Context, prctx pull.Context, cl return evaluator, nil } -func (b *Base) EvaluateFetchedConfig(ctx context.Context, prctx pull.Context, client *github.Client, evaluator common.Evaluator, fc FetchedConfig) (common.Result, error) { +func (b *Base) EvaluateConfig(ctx context.Context, prctx pull.Context, client *github.Client, evaluator common.Evaluator, fc FetchedConfig) common.Result { + result := evaluator.Evaluate(ctx, prctx) + + return result +} + +func (b *Base) PostEvaluatedResults(ctx context.Context, prctx pull.Context, client *github.Client, evaluator common.Evaluator, fc FetchedConfig, result common.Result) (common.Result, error) { logger := zerolog.Ctx(ctx) + statusDescription := result.StatusDescription - result := evaluator.Evaluate(ctx, prctx) if result.Error != nil { msg := fmt.Sprintf("Error evaluating policy in %s: %s", fc.Source, fc.Path) logger.Warn().Err(result.Error).Msg(msg) @@ -245,8 +251,6 @@ func (b *Base) EvaluateFetchedConfig(ctx context.Context, prctx pull.Context, cl return result, result.Error } - statusDescription := result.StatusDescription - var statusState string switch result.Status { case common.StatusApproved: @@ -268,6 +272,13 @@ func (b *Base) EvaluateFetchedConfig(ctx context.Context, prctx pull.Context, cl return result, nil } +func (b *Base) EvaluateFetchedConfig(ctx context.Context, prctx pull.Context, client *github.Client, evaluator common.Evaluator, fc FetchedConfig) (common.Result, error) { + res := b.EvaluateConfig(ctx, prctx, client, evaluator, fc) + var result, err = b.PostEvaluatedResults(ctx, prctx, client, evaluator, fc, res) + + return result, err +} + func (b *Base) RequestReviewsForResult(ctx context.Context, prctx pull.Context, client *github.Client, trigger common.Trigger, result common.Result) error { logger := zerolog.Ctx(ctx) diff --git a/server/handler/base_details.go b/server/handler/base_details.go new file mode 100644 index 00000000..3b736085 --- /dev/null +++ b/server/handler/base_details.go @@ -0,0 +1,229 @@ +// Copyright 2022 Palantir Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package handler + +import ( + "context" + "fmt" + "goji.io/pat" + "net/http" + "net/url" + "path" + "strconv" + "strings" + + "github.com/alexedwards/scs" + "github.com/bluekeyes/templatetree" + "github.com/google/go-github/v45/github" + "github.com/palantir/go-githubapp/githubapp" + "github.com/palantir/policy-bot/policy/common" + "github.com/palantir/policy-bot/pull" + "github.com/pkg/errors" +) + +type DetailsBase struct { + Base + Sessions *scs.Manager + Templates templatetree.HTMLTree +} + +type DetailsData struct { + Error error + IsTemporaryError bool + Result *common.Result + PullRequest *github.PullRequest + User string + PolicyURL string +} + +func (h *DetailsBase) getUrlParams(w http.ResponseWriter, r *http.Request) (string, string, int, error) { + owner := pat.Param(r, "owner") + repo := pat.Param(r, "repo") + //number := + number, err := strconv.Atoi(pat.Param(r, "number")) + if err != nil { + http.Error(w, fmt.Sprintf("Invalid pull request number: %v", err), http.StatusBadRequest) + return "", "", -1, err + } + + return owner, repo, number, nil +} + +func (h *DetailsBase) generatePrContext(ctx context.Context, owner string, repo string, number int) (pull.Context, error) { + installation, err := h.Installations.GetByOwner(ctx, owner) + if err != nil { + if _, notFound := err.(githubapp.InstallationNotFound); notFound { + //h.render404(w, owner, repo, number) + return nil, nil + } + return nil, err + } + + client, err := h.ClientCreator.NewInstallationClient(installation.ID) + if err != nil { + return nil, errors.Wrap(err, "failed to create github client") + } + + v4client, err := h.ClientCreator.NewInstallationV4Client(installation.ID) + if err != nil { + return nil, errors.Wrap(err, "failed to create github client") + } + + pr, _, err := client.PullRequests.Get(ctx, owner, repo, number) + if err != nil { + if isNotFound(err) { + //h.render404(w, owner, repo, number) + return nil, nil + } + return nil, errors.Wrap(err, "failed to get pull request") + } + mbrCtx := NewCrossOrgMembershipContext(ctx, client, owner, h.Installations, h.ClientCreator) + + prCtx, err := pull.NewGitHubContext(ctx, mbrCtx, client, v4client, pull.Locator{ + Owner: owner, + Repo: repo, + Number: number, + Value: pr, + }) + + return prCtx, err +} + +func (h *DetailsBase) getPolicyConfig(ctx context.Context, prCtx pull.Context, branch string) (FetchedConfig, error) { + owner := prCtx.RepositoryOwner() + + installation, err := h.Installations.GetByOwner(ctx, owner) + if err != nil { + if _, notFound := err.(githubapp.InstallationNotFound); notFound { + //h.render404(w, owner, repo, number) + err = errors.Wrap(err, "failed to get github installation") + } + } + client, err := h.ClientCreator.NewInstallationClient(installation.ID) + if err != nil { + err = errors.Wrap(err, "failed to get github installaion") + //return nil, errors.Wrap(err, "failed to create github client") + } + + config := h.ConfigFetcher.ConfigForPRBranch(ctx, prCtx, client, branch) + + return config, err +} + +func (h *DetailsBase) generateEvaluationDetails(w http.ResponseWriter, r *http.Request, policyConfig FetchedConfig, prCtx pull.Context) (*DetailsData, *github.Client, common.Evaluator, error) { + ctx := r.Context() + //logger := zerolog.Ctx(ctx) + + owner := prCtx.RepositoryOwner() + repo := prCtx.RepositoryName() + number := prCtx.Number() + + installation, err := h.Installations.GetByOwner(ctx, owner) + if err != nil { + if _, notFound := err.(githubapp.InstallationNotFound); notFound { + h.render404(w, owner, repo, number) + return nil, nil, nil, errors.Wrap(err, "Unable to find installation") + } + return nil, nil, nil, err + } + + client, err := h.ClientCreator.NewInstallationClient(installation.ID) + if err != nil { + return nil, nil, nil, errors.Wrap(err, "failed to create github client") + } + + sess := h.Sessions.Load(r) + user, err := sess.GetString(SessionKeyUsername) + if err != nil { + return nil, nil, nil, errors.Wrap(err, "failed to read sessions") + } + + level, _, err := client.Repositories.GetPermissionLevel(ctx, owner, repo, user) + if err != nil { + if isNotFound(err) { + h.render404(w, owner, repo, number) + return nil, nil, nil, nil + } + return nil, nil, nil, errors.Wrap(err, "failed to get user permission level") + } + + // if the user does not have permission, pretend the repo/PR doesn't exist + if level.GetPermission() == "none" { + h.render404(w, owner, repo, number) + return nil, nil, nil, nil + } + + pr, _, err := client.PullRequests.Get(ctx, owner, repo, number) + if err != nil { + if isNotFound(err) { + h.render404(w, owner, repo, number) + return nil, nil, nil, nil + } + return nil, nil, nil, errors.Wrap(err, "failed to get pull request") + } + + var data DetailsData + + data.PullRequest = pr + data.User = user + + data.PolicyURL = getPolicyURL(pr, policyConfig) + + evaluator, err := h.Base.ValidateFetchedConfig(ctx, prCtx, client, policyConfig, common.TriggerAll) + if err != nil { + data.Error = err + return &data, nil, nil, nil + } + if evaluator == nil { + data.Error = errors.Errorf("Invalid policy at %s: %s", policyConfig.Source, policyConfig.Path) + return &data, nil, nil, nil + } + + return &data, client, evaluator, nil +} + +func (h *DetailsBase) render404(w http.ResponseWriter, owner, repo string, number int) { + msg := fmt.Sprintf( + "Not Found: %s/%s#%DetailsData\n\nThe repository or pull request does not exist, you do not have permission, or policy-bot is not installed.", + owner, repo, number, + ) + http.Error(w, msg, http.StatusNotFound) +} + +func (h *DetailsBase) render(w http.ResponseWriter, templateName string, data interface{}) error { + w.Header().Set("Content-Type", "text/html") + w.WriteHeader(http.StatusOK) + + //fmt.Println(data) + return h.Templates.ExecuteTemplate(w, templateName, data) +} + +func getPolicyURL(pr *github.PullRequest, config FetchedConfig) string { + base := pr.GetBase().GetRepo().GetHTMLURL() + if u, _ := url.Parse(base); u != nil { + srcParts := strings.Split(config.Source, "@") + if len(srcParts) != 2 { + return base + } + u.Path = path.Join(srcParts[0], "blob", srcParts[1], config.Path) + return u.String() + } + return base +} + +func isNotFound(err error) bool { + rerr, ok := err.(*github.ErrorResponse) + return ok && rerr.Response.StatusCode == http.StatusNotFound +} diff --git a/server/handler/details.go b/server/handler/details.go index 46d9f877..7b63e038 100644 --- a/server/handler/details.go +++ b/server/handler/details.go @@ -15,169 +15,30 @@ package handler import ( - "fmt" "net/http" - "net/url" - "path" - "strconv" - "strings" - - "github.com/alexedwards/scs" - "github.com/bluekeyes/templatetree" - "github.com/google/go-github/v45/github" - "github.com/palantir/go-githubapp/githubapp" - "github.com/palantir/policy-bot/policy/common" - "github.com/palantir/policy-bot/pull" - "github.com/pkg/errors" - "goji.io/pat" ) type Details struct { - Base - Sessions *scs.Manager - Templates templatetree.HTMLTree + DetailsBase } func (h *Details) ServeHTTP(w http.ResponseWriter, r *http.Request) error { ctx := r.Context() - owner := pat.Param(r, "owner") - repo := pat.Param(r, "repo") - - number, err := strconv.Atoi(pat.Param(r, "number")) - if err != nil { - http.Error(w, fmt.Sprintf("Invalid pull request number: %v", err), http.StatusBadRequest) - return nil - } - - installation, err := h.Installations.GetByOwner(ctx, owner) - if err != nil { - if _, notFound := err.(githubapp.InstallationNotFound); notFound { - h.render404(w, owner, repo, number) - return nil - } - return err - } - - client, err := h.ClientCreator.NewInstallationClient(installation.ID) - if err != nil { - return errors.Wrap(err, "failed to create github client") - } - - v4client, err := h.ClientCreator.NewInstallationV4Client(installation.ID) - if err != nil { - return errors.Wrap(err, "failed to create github client") - } - - sess := h.Sessions.Load(r) - user, err := sess.GetString(SessionKeyUsername) - if err != nil { - return errors.Wrap(err, "failed to read sessions") - } - - level, _, err := client.Repositories.GetPermissionLevel(ctx, owner, repo, user) - if err != nil { - if isNotFound(err) { - h.render404(w, owner, repo, number) - return nil - } - return errors.Wrap(err, "failed to get user permission level") - } - - // if the user does not have permission, pretend the repo/PR doesn't exist - if level.GetPermission() == "none" { - h.render404(w, owner, repo, number) - return nil - } - - pr, _, err := client.PullRequests.Get(ctx, owner, repo, number) - if err != nil { - if isNotFound(err) { - h.render404(w, owner, repo, number) - return nil - } - return errors.Wrap(err, "failed to get pull request") - } - - ctx, _ = h.PreparePRContext(ctx, installation.ID, pr) - - mbrCtx := NewCrossOrgMembershipContext(ctx, client, owner, h.Installations, h.ClientCreator) - prctx, err := pull.NewGitHubContext(ctx, mbrCtx, client, v4client, pull.Locator{ - Owner: owner, - Repo: repo, - Number: number, - Value: pr, - }) + owner, repo, number, err := h.getUrlParams(w, r) if err != nil { return err } - var data struct { - Error error - IsTemporaryError bool - Result *common.Result - PullRequest *github.PullRequest - User string - PolicyURL string - } - - data.PullRequest = pr - data.User = user + prCtx, err := h.generatePrContext(ctx, owner, repo, number) + branch, _ := prCtx.Branches() - config := h.ConfigFetcher.ConfigForPR(ctx, prctx, client) - data.PolicyURL = getPolicyURL(pr, config) + policyConfig, _ := h.getPolicyConfig(ctx, prCtx, branch) + details, client, evaluator, _ := h.generateEvaluationDetails(w, r, policyConfig, prCtx) - evaluator, err := h.Base.ValidateFetchedConfig(ctx, prctx, client, config, common.TriggerAll) - if err != nil { - data.Error = err - return h.render(w, data) - } - if evaluator == nil { - data.Error = errors.Errorf("Invalid policy at %s: %s", config.Source, config.Path) - return h.render(w, data) - } - - result, err := h.Base.EvaluateFetchedConfig(ctx, prctx, client, evaluator, config) - data.Result = &result - - if err != nil { - if _, ok := errors.Cause(err).(*pull.TemporaryError); ok { - data.IsTemporaryError = true - } - data.Error = err - } - - return h.render(w, data) -} - -func (h *Details) render(w http.ResponseWriter, data interface{}) error { + result, err := h.Base.EvaluateFetchedConfig(ctx, prCtx, client, evaluator, policyConfig) + details.Result = &result w.Header().Set("Content-Type", "text/html") w.WriteHeader(http.StatusOK) - return h.Templates.ExecuteTemplate(w, "details.html.tmpl", data) -} - -func (h *Details) render404(w http.ResponseWriter, owner, repo string, number int) { - msg := fmt.Sprintf( - "Not Found: %s/%s#%d\n\nThe repository or pull request does not exist, you do not have permission, or policy-bot is not installed.", - owner, repo, number, - ) - http.Error(w, msg, http.StatusNotFound) -} - -func getPolicyURL(pr *github.PullRequest, config FetchedConfig) string { - base := pr.GetBase().GetRepo().GetHTMLURL() - if u, _ := url.Parse(base); u != nil { - srcParts := strings.Split(config.Source, "@") - if len(srcParts) != 2 { - return base - } - u.Path = path.Join(srcParts[0], "blob", srcParts[1], config.Path) - return u.String() - } - return base -} - -func isNotFound(err error) bool { - rerr, ok := err.(*github.ErrorResponse) - return ok && rerr.Response.StatusCode == http.StatusNotFound + return h.render(w, "details.html.tmpl", details) } diff --git a/server/handler/fetcher.go b/server/handler/fetcher.go index 5af388bf..e1208d69 100644 --- a/server/handler/fetcher.go +++ b/server/handler/fetcher.go @@ -16,7 +16,6 @@ package handler import ( "context" - "github.com/google/go-github/v45/github" "github.com/palantir/go-githubapp/appconfig" "github.com/palantir/policy-bot/policy" @@ -37,10 +36,8 @@ type ConfigFetcher struct { Loader *appconfig.Loader } -func (cf *ConfigFetcher) ConfigForPR(ctx context.Context, prctx pull.Context, client *github.Client) FetchedConfig { - base, _ := prctx.Branches() - - c, err := cf.Loader.LoadConfig(ctx, client, prctx.RepositoryOwner(), prctx.RepositoryName(), base) +func (cf *ConfigFetcher) ConfigForPRBranch(ctx context.Context, prctx pull.Context, client *github.Client, branch string) FetchedConfig { + c, err := cf.Loader.LoadConfig(ctx, client, prctx.RepositoryOwner(), prctx.RepositoryName(), branch) fc := FetchedConfig{ Source: c.Source, Path: c.Path, @@ -62,3 +59,9 @@ func (cf *ConfigFetcher) ConfigForPR(ctx context.Context, prctx pull.Context, cl } return fc } + +func (cf *ConfigFetcher) ConfigForPR(ctx context.Context, prctx pull.Context, client *github.Client) FetchedConfig { + base, _ := prctx.Branches() + + return cf.ConfigForPRBranch(ctx, prctx, client, base) +} diff --git a/server/handler/simulate.go b/server/handler/simulate.go new file mode 100644 index 00000000..75dd1c5f --- /dev/null +++ b/server/handler/simulate.go @@ -0,0 +1,85 @@ +// Copyright 2022 Palantir Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package handler + +import ( + "github.com/palantir/policy-bot/pull" + "net/http" + "time" +) + +type Simulate struct { + DetailsBase +} + +type fakePrContext struct { + pull.Context + + additionalApprovals []string +} + +func (f *fakePrContext) Reviews() ([]*pull.Review, error) { + reviews, err := f.Context.Reviews() + + for _, username := range f.additionalApprovals { + reviews = append(reviews, &pull.Review{CreatedAt: time.Now(), UpdatedAt: time.Now(), Author: username, State: pull.ReviewApproved}) + } + + return reviews, err +} + +func new(ctx pull.Context) *fakePrContext { + return &fakePrContext{Context: ctx} +} + +func (f *fakePrContext) addApproval(username string) { + f.additionalApprovals = append(f.additionalApprovals, username) +} + +func (h *Simulate) ServeHTTP(w http.ResponseWriter, r *http.Request) error { + ctx := r.Context() + + owner, repo, number, err := h.getUrlParams(w, r) + if err != nil { + return err + } + + queryParams := r.URL.Query() + prCtx, err := h.generatePrContext(ctx, owner, repo, number) + var branch, username string + + if queryParams.Has("branch") { + branch = queryParams.Get("branch") + } else { + branch, _ = prCtx.Branches() + } + + if queryParams.Has("username") && queryParams.Get("username") != "" { + username = queryParams.Get("username") + fakePrContext := new(prCtx) + fakePrContext.addApproval(username) + prCtx = fakePrContext + } + + policyConfig, _ := h.getPolicyConfig(ctx, prCtx, branch) + details, client, evaluator, _ := h.generateEvaluationDetails(w, r, policyConfig, prCtx) + + result := h.Base.EvaluateConfig(ctx, prCtx, client, evaluator, policyConfig) + details.Result = &result + + w.Header().Set("Content-Type", "text/html") + w.WriteHeader(http.StatusOK) + return h.render(w, "simulate.html.tmpl", details) +} diff --git a/server/server.go b/server/server.go index c604b2c0..c304f256 100644 --- a/server/server.go +++ b/server/server.go @@ -217,13 +217,27 @@ func New(c *Config) (*Server, error) { details := goji.SubMux() details.Use(handler.RequireLogin(sessions, basePath)) - details.Handle(pat.Get("/:owner/:repo/:number"), hatpear.Try(&handler.Details{ - Base: basePolicyHandler, - Sessions: sessions, - Templates: templates, - })) + details.Handle(pat.Get("/:owner/:repo/:number"), hatpear.Try( + &handler.Details{ + DetailsBase: handler.DetailsBase{ + Base: basePolicyHandler, + Sessions: sessions, + Templates: templates, + }})) mux.Handle(pat.New("/details/*"), details) + simulate := goji.SubMux() + simulate.Use(handler.RequireLogin(sessions, basePath)) + simulate.Handle(pat.Get("/:owner/:repo/:number"), hatpear.Try( + &handler.Simulate{ + DetailsBase: handler.DetailsBase{ + Base: basePolicyHandler, + Sessions: sessions, + Templates: templates, + }})) + + mux.Handle(pat.New("/simulate/*"), simulate) + return &Server{ config: c, base: base, diff --git a/server/templates/base-details.html.tmpl b/server/templates/base-details.html.tmpl new file mode 100644 index 00000000..f10929c8 --- /dev/null +++ b/server/templates/base-details.html.tmpl @@ -0,0 +1,156 @@ +{{/* templatetree:extends page.html.tmpl */}} +{{define "title"}}{{.PullRequest.GetBase.GetRepo.GetFullName}}#{{.PullRequest.GetNumber}} - Details | PolicyBot{{end}} + +{{define "scripts"}} + +{{end}} + +{{define "body-class"}}bg-light-gray5 text-dark-gray1 flex flex-col h-screen{{end}} + +{{define "body"}} + {{block "top" .}}{{end}} + {{block "details" .}}{{end}} + {{ block "footer" .}}{{end}} +{{end}} + +{{define "details"}} + {{if .Error}} +
+

Error

+

{{.Error}}

+ {{if .IsTemporaryError}} +

This error may be temporary. Wait 30 seconds and refresh this page to retry.

+ {{end}} +
+ {{else}} + {{ $s := (or (and .Result.Error "error") (.Result.Status | print)) }} +
+
+

Status: {{$s | titlecase}}

+

{{or .Result.Error .Result.StatusDescription}}

+
+
+
+ + +
+
+
+
+ +
+ {{end}} +{{end}} + + +{{define "result"}} + {{ $s := (or (and .Error "error") (.Status | print)) }} +
  • +
    + {{template "result-details" .}} + {{if (or (.PredicateResults) (hasRequires .Requires) (hasRequiresPermissions .Requires))}} +
    + Details +
    + {{if .PredicateResults}} +
    + {{template "result-predicates-details" .}} +
    + {{end}} + {{if (hasRequires .Requires)}} +
    + {{template "result-requires-details" .}} +
    + {{end}} + {{if (hasRequiresPermissions .Requires)}} +
    + {{template "result-requires-permissions-details" .}} +
    + {{end}} +
    +
    + {{end}} +
    + {{if .Children}} + + {{end}} +
  • +{{end}} + +{{define "result-details"}} + {{ $s := (or (and .Error "error") (.Status | print)) }} +

    + {{.Name}} + {{$s | titlecase}} +

    + {{if (and .Description (ne $s "skipped"))}} +

    {{.Description}}

    + {{end}} +

    {{or .Error .StatusDescription}}

    +{{end}} + +{{define "result-predicates-details"}} + {{ $s := (or (and .Error "error") (.Status | print)) }} + {{if eq $s "skipped"}} + This rule is skipped because + {{else}} + This rule is selected because + {{end}} + {{template "result-predicates-info" .}} +{{end}} + +{{define "result-predicates-info"}} + {{ $s := (or (and .Error "error") (.Status | print)) }} + +{{end}} + +{{define "result-requires-details"}} + This rule requires review from +
    + {{range $k, $v := getRequires .}} +
    {{$k}}
    +
    + +
    + {{end}} +
    +{{end}} + +{{define "result-requires-permissions-details"}} + This rule requires review from users with the following permissions + +{{end}} diff --git a/server/templates/details.html.tmpl b/server/templates/details.html.tmpl index 2791e5b4..6cfb8730 100644 --- a/server/templates/details.html.tmpl +++ b/server/templates/details.html.tmpl @@ -1,12 +1,14 @@ -{{/* templatetree:extends page.html.tmpl */}} +{{/* templatetree:extends base-details.html.tmpl */}} {{define "title"}}{{.PullRequest.GetBase.GetRepo.GetFullName}}#{{.PullRequest.GetNumber}} - Details | PolicyBot{{end}} {{define "scripts"}} - + + {{end}} {{define "body-class"}}bg-light-gray5 text-dark-gray1 flex flex-col h-screen{{end}} -{{define "body"}} + +{{define "top"}}
    @@ -21,142 +23,10 @@ {{.User}}
    - {{if .Error}} -
    -

    Error

    -

    {{.Error}}

    - {{if .IsTemporaryError}} -

    This error may be temporary. Wait 30 seconds and refresh this page to retry.

    - {{end}} -
    - {{else}} - {{ $s := (or (and .Result.Error "error") (.Result.Status | print)) }} -
    -
    -

    Status: {{$s | titlecase}}

    -

    {{or .Result.Error .Result.StatusDescription}}

    -
    -
    -
    - - -
    -
    -
    -
    -
      - {{range .Result.Children | sortByStatus}}{{template "result" .}}{{end}} -
    -
    - {{end}} -{{end}} - -{{define "result"}} -{{ $s := (or (and .Error "error") (.Status | print)) }} -
  • -
    - {{template "result-details" .}} - {{if (or (.PredicateResults) (hasRequires .Requires) (hasRequiresPermissions .Requires))}} -
    - Details -
    - {{if .PredicateResults}} -
    - {{template "result-predicates-details" .}} -
    - {{end}} - {{if (hasRequires .Requires)}} -
    - {{template "result-requires-details" .}} -
    - {{end}} - {{if (hasRequiresPermissions .Requires)}} -
    - {{template "result-requires-permissions-details" .}} -
    - {{end}} -
    -
    - {{end}} -
    - {{if .Children}} -
      - {{range .Children | sortByStatus}}{{template "result" .}}{{end}} -
    - {{end}} -
  • -{{end}} - -{{define "result-details"}} - {{ $s := (or (and .Error "error") (.Status | print)) }} -

    - {{.Name}} - {{$s | titlecase}} -

    - {{if (and .Description (ne $s "skipped"))}} -

    {{.Description}}

    - {{end}} -

    {{or .Error .StatusDescription}}

    -{{end}} - -{{define "result-predicates-details"}} - {{ $s := (or (and .Error "error") (.Status | print)) }} - {{if eq $s "skipped"}} - This rule is skipped because - {{else}} - This rule is selected because - {{end}} - {{template "result-predicates-info" .}} -{{end}} - -{{define "result-predicates-info"}} - {{ $s := (or (and .Error "error") (.Status | print)) }} - {{end}} -{{define "result-requires-details"}} - This rule requires review from -
    - {{range $k, $v := getRequires .}} -
    {{$k}}
    -
    - -
    - {{end}} -
    -{{end}} - -{{define "result-requires-permissions-details"}} - This rule requires review from users with the following permissions - -{{end}} +{{define "footer"}} + +{{end}} \ No newline at end of file diff --git a/server/templates/simulate.html.tmpl b/server/templates/simulate.html.tmpl new file mode 100644 index 00000000..e0e3704a --- /dev/null +++ b/server/templates/simulate.html.tmpl @@ -0,0 +1,62 @@ +{{/* templatetree:extends base-details.html.tmpl */}} +{{define "title"}}{{.PullRequest.GetBase.GetRepo.GetFullName}}#{{.PullRequest.GetNumber}} - Details | PolicyBot{{end}} + +{{define "scripts"}} + + +{{end}} + +{{define "body-class"}}bg-light-gray5 text-dark-gray1 flex flex-col h-screen{{end}} + +{{define "top"}} +
    + + {{.PullRequest.GetBase.GetRepo.GetFullName}}: {{.PullRequest.GetBase.GetRef}} + +

    + + #{{.PullRequest.GetNumber}}: + {{.PullRequest.GetTitle}} +

    + + {{.User}} + +
    +
    +
    +
    +
    +
    + Simulation Details +
    +
    +
    +
    + +
    +
    + +

    The branch of this repository to find .policy.yml

    +
    +
    +
    +
    + +
    +
    + +

    A user to simulate an approval from

    +
    +
    +
    +
    +
    + +
    +
    +
    +
    +
    +{{end}} + From 37ebb8c6eebc36c7f72338c96e95eac818788cbd Mon Sep 17 00:00:00 2001 From: Ashir Amer Date: Fri, 5 Aug 2022 23:58:01 +0000 Subject: [PATCH 2/3] Convert FetchedConfig to a pointer type This makes it possible to handle errors similar to how it was done before --- server/handler/base.go | 10 +++++----- server/handler/base_details.go | 17 ++++++++--------- server/handler/details.go | 14 ++++++++++---- server/handler/fetcher.go | 10 +++++----- server/handler/simulate.go | 7 ++++++- 5 files changed, 34 insertions(+), 24 deletions(-) diff --git a/server/handler/base.go b/server/handler/base.go index bb03d6de..86ba4013 100644 --- a/server/handler/base.go +++ b/server/handler/base.go @@ -114,7 +114,7 @@ func (b *Base) PostStatus(ctx context.Context, prctx pull.Context, client *githu } publicURL := strings.TrimSuffix(b.BaseConfig.PublicURL, "/") - detailsURL := fmt.Sprintf("%s/details/%s/%s/%DetailsData", publicURL, owner, repo, prctx.Number()) + detailsURL := fmt.Sprintf("%s/details/%s/%s/%d", publicURL, owner, repo, prctx.Number()) contextWithBranch := fmt.Sprintf("%s: %s", b.PullOpts.StatusCheckContext, base) status := &github.RepoStatus{ @@ -189,7 +189,7 @@ func (b *Base) Evaluate(ctx context.Context, installationID int64, trigger commo return b.RequestReviewsForResult(ctx, prctx, client, trigger, result) } -func (b *Base) ValidateFetchedConfig(ctx context.Context, prctx pull.Context, client *github.Client, fc FetchedConfig, trigger common.Trigger) (common.Evaluator, error) { +func (b *Base) ValidateFetchedConfig(ctx context.Context, prctx pull.Context, client *github.Client, fc *FetchedConfig, trigger common.Trigger) (common.Evaluator, error) { logger := zerolog.Ctx(ctx) switch { @@ -233,13 +233,13 @@ func (b *Base) ValidateFetchedConfig(ctx context.Context, prctx pull.Context, cl return evaluator, nil } -func (b *Base) EvaluateConfig(ctx context.Context, prctx pull.Context, client *github.Client, evaluator common.Evaluator, fc FetchedConfig) common.Result { +func (b *Base) EvaluateConfig(ctx context.Context, prctx pull.Context, client *github.Client, evaluator common.Evaluator, fc *FetchedConfig) common.Result { result := evaluator.Evaluate(ctx, prctx) return result } -func (b *Base) PostEvaluatedResults(ctx context.Context, prctx pull.Context, client *github.Client, evaluator common.Evaluator, fc FetchedConfig, result common.Result) (common.Result, error) { +func (b *Base) PostEvaluatedResults(ctx context.Context, prctx pull.Context, client *github.Client, evaluator common.Evaluator, fc *FetchedConfig, result common.Result) (common.Result, error) { logger := zerolog.Ctx(ctx) statusDescription := result.StatusDescription @@ -272,7 +272,7 @@ func (b *Base) PostEvaluatedResults(ctx context.Context, prctx pull.Context, cli return result, nil } -func (b *Base) EvaluateFetchedConfig(ctx context.Context, prctx pull.Context, client *github.Client, evaluator common.Evaluator, fc FetchedConfig) (common.Result, error) { +func (b *Base) EvaluateFetchedConfig(ctx context.Context, prctx pull.Context, client *github.Client, evaluator common.Evaluator, fc *FetchedConfig) (common.Result, error) { res := b.EvaluateConfig(ctx, prctx, client, evaluator, fc) var result, err = b.PostEvaluatedResults(ctx, prctx, client, evaluator, fc, res) diff --git a/server/handler/base_details.go b/server/handler/base_details.go index 3b736085..7b90fd94 100644 --- a/server/handler/base_details.go +++ b/server/handler/base_details.go @@ -48,7 +48,7 @@ type DetailsData struct { PolicyURL string } -func (h *DetailsBase) getUrlParams(w http.ResponseWriter, r *http.Request) (string, string, int, error) { +func (h *DetailsBase) getURLParams(w http.ResponseWriter, r *http.Request) (string, string, int, error) { owner := pat.Param(r, "owner") repo := pat.Param(r, "repo") //number := @@ -101,20 +101,19 @@ func (h *DetailsBase) generatePrContext(ctx context.Context, owner string, repo return prCtx, err } -func (h *DetailsBase) getPolicyConfig(ctx context.Context, prCtx pull.Context, branch string) (FetchedConfig, error) { +func (h *DetailsBase) getPolicyConfig(ctx context.Context, prCtx pull.Context, branch string) (*FetchedConfig, error) { owner := prCtx.RepositoryOwner() installation, err := h.Installations.GetByOwner(ctx, owner) if err != nil { if _, notFound := err.(githubapp.InstallationNotFound); notFound { - //h.render404(w, owner, repo, number) - err = errors.Wrap(err, "failed to get github installation") + return nil, errors.Wrap(err, "failed to get github installation") } } client, err := h.ClientCreator.NewInstallationClient(installation.ID) if err != nil { - err = errors.Wrap(err, "failed to get github installaion") - //return nil, errors.Wrap(err, "failed to create github client") + err = errors.Wrap(err, "failed to get github installation") + return nil, errors.Wrap(err, "failed to create github client") } config := h.ConfigFetcher.ConfigForPRBranch(ctx, prCtx, client, branch) @@ -122,7 +121,7 @@ func (h *DetailsBase) getPolicyConfig(ctx context.Context, prCtx pull.Context, b return config, err } -func (h *DetailsBase) generateEvaluationDetails(w http.ResponseWriter, r *http.Request, policyConfig FetchedConfig, prCtx pull.Context) (*DetailsData, *github.Client, common.Evaluator, error) { +func (h *DetailsBase) generateEvaluationDetails(w http.ResponseWriter, r *http.Request, policyConfig *FetchedConfig, prCtx pull.Context) (*DetailsData, *github.Client, common.Evaluator, error) { ctx := r.Context() //logger := zerolog.Ctx(ctx) @@ -196,7 +195,7 @@ func (h *DetailsBase) generateEvaluationDetails(w http.ResponseWriter, r *http.R func (h *DetailsBase) render404(w http.ResponseWriter, owner, repo string, number int) { msg := fmt.Sprintf( - "Not Found: %s/%s#%DetailsData\n\nThe repository or pull request does not exist, you do not have permission, or policy-bot is not installed.", + "Not Found: %s/%s#%d\n\nThe repository or pull request does not exist, you do not have permission, or policy-bot is not installed.", owner, repo, number, ) http.Error(w, msg, http.StatusNotFound) @@ -210,7 +209,7 @@ func (h *DetailsBase) render(w http.ResponseWriter, templateName string, data in return h.Templates.ExecuteTemplate(w, templateName, data) } -func getPolicyURL(pr *github.PullRequest, config FetchedConfig) string { +func getPolicyURL(pr *github.PullRequest, config *FetchedConfig) string { base := pr.GetBase().GetRepo().GetHTMLURL() if u, _ := url.Parse(base); u != nil { srcParts := strings.Split(config.Source, "@") diff --git a/server/handler/details.go b/server/handler/details.go index 7b63e038..4bdceaf2 100644 --- a/server/handler/details.go +++ b/server/handler/details.go @@ -15,6 +15,7 @@ package handler import ( + "github.com/pkg/errors" "net/http" ) @@ -25,18 +26,23 @@ type Details struct { func (h *Details) ServeHTTP(w http.ResponseWriter, r *http.Request) error { ctx := r.Context() - owner, repo, number, err := h.getUrlParams(w, r) + owner, repo, number, err := h.getURLParams(w, r) if err != nil { return err } - prCtx, err := h.generatePrContext(ctx, owner, repo, number) + prCtx, _ := h.generatePrContext(ctx, owner, repo, number) branch, _ := prCtx.Branches() - policyConfig, _ := h.getPolicyConfig(ctx, prCtx, branch) + policyConfig, err := h.getPolicyConfig(ctx, prCtx, branch) + if err != nil { + h.render404(w, owner, repo, number) + return errors.Wrap(err, "failed to get policy config") + } + details, client, evaluator, _ := h.generateEvaluationDetails(w, r, policyConfig, prCtx) - result, err := h.Base.EvaluateFetchedConfig(ctx, prCtx, client, evaluator, policyConfig) + result, _ := h.Base.EvaluateFetchedConfig(ctx, prCtx, client, evaluator, policyConfig) details.Result = &result w.Header().Set("Content-Type", "text/html") w.WriteHeader(http.StatusOK) diff --git a/server/handler/fetcher.go b/server/handler/fetcher.go index e1208d69..b9224dff 100644 --- a/server/handler/fetcher.go +++ b/server/handler/fetcher.go @@ -36,7 +36,7 @@ type ConfigFetcher struct { Loader *appconfig.Loader } -func (cf *ConfigFetcher) ConfigForPRBranch(ctx context.Context, prctx pull.Context, client *github.Client, branch string) FetchedConfig { +func (cf *ConfigFetcher) ConfigForPRBranch(ctx context.Context, prctx pull.Context, client *github.Client, branch string) *FetchedConfig { c, err := cf.Loader.LoadConfig(ctx, client, prctx.RepositoryOwner(), prctx.RepositoryName(), branch) fc := FetchedConfig{ Source: c.Source, @@ -46,9 +46,9 @@ func (cf *ConfigFetcher) ConfigForPRBranch(ctx context.Context, prctx pull.Conte switch { case err != nil: fc.LoadError = err - return fc + return &fc case c.IsUndefined(): - return fc + return &fc } var pc policy.Config @@ -57,10 +57,10 @@ func (cf *ConfigFetcher) ConfigForPRBranch(ctx context.Context, prctx pull.Conte } else { fc.Config = &pc } - return fc + return &fc } -func (cf *ConfigFetcher) ConfigForPR(ctx context.Context, prctx pull.Context, client *github.Client) FetchedConfig { +func (cf *ConfigFetcher) ConfigForPR(ctx context.Context, prctx pull.Context, client *github.Client) *FetchedConfig { base, _ := prctx.Branches() return cf.ConfigForPRBranch(ctx, prctx, client, base) diff --git a/server/handler/simulate.go b/server/handler/simulate.go index 75dd1c5f..d56c23ad 100644 --- a/server/handler/simulate.go +++ b/server/handler/simulate.go @@ -16,6 +16,7 @@ package handler import ( "github.com/palantir/policy-bot/pull" + "github.com/pkg/errors" "net/http" "time" ) @@ -51,7 +52,7 @@ func (f *fakePrContext) addApproval(username string) { func (h *Simulate) ServeHTTP(w http.ResponseWriter, r *http.Request) error { ctx := r.Context() - owner, repo, number, err := h.getUrlParams(w, r) + owner, repo, number, err := h.getURLParams(w, r) if err != nil { return err } @@ -74,6 +75,10 @@ func (h *Simulate) ServeHTTP(w http.ResponseWriter, r *http.Request) error { } policyConfig, _ := h.getPolicyConfig(ctx, prCtx, branch) + if err != nil { + h.render404(w, owner, repo, number) + return errors.Wrap(err, "failed to get policy config") + } details, client, evaluator, _ := h.generateEvaluationDetails(w, r, policyConfig, prCtx) result := h.Base.EvaluateConfig(ctx, prCtx, client, evaluator, policyConfig) From 3d3dc32ae2404ed0f6ed6fc9d37e08307a2455e0 Mon Sep 17 00:00:00 2001 From: Ashir Amer Date: Thu, 29 Sep 2022 23:57:29 +0000 Subject: [PATCH 3/3] Round 1 of PR Feedback --- server/handler/base.go | 5 ++--- server/handler/base_details.go | 8 ++------ server/handler/details.go | 7 +++++-- server/handler/simulate.go | 19 +++++++++++-------- server/templates/base-details.html.tmpl | 1 + server/templates/details.html.tmpl | 8 -------- server/templates/simulate.html.tmpl | 10 ++-------- 7 files changed, 23 insertions(+), 35 deletions(-) diff --git a/server/handler/base.go b/server/handler/base.go index 86ba4013..89b29587 100644 --- a/server/handler/base.go +++ b/server/handler/base.go @@ -239,7 +239,7 @@ func (b *Base) EvaluateConfig(ctx context.Context, prctx pull.Context, client *g return result } -func (b *Base) PostEvaluatedResults(ctx context.Context, prctx pull.Context, client *github.Client, evaluator common.Evaluator, fc *FetchedConfig, result common.Result) (common.Result, error) { +func (b *Base) PostEvaluatedResults(ctx context.Context, prctx pull.Context, client *github.Client, fc *FetchedConfig, result common.Result) (common.Result, error) { logger := zerolog.Ctx(ctx) statusDescription := result.StatusDescription @@ -274,9 +274,8 @@ func (b *Base) PostEvaluatedResults(ctx context.Context, prctx pull.Context, cli func (b *Base) EvaluateFetchedConfig(ctx context.Context, prctx pull.Context, client *github.Client, evaluator common.Evaluator, fc *FetchedConfig) (common.Result, error) { res := b.EvaluateConfig(ctx, prctx, client, evaluator, fc) - var result, err = b.PostEvaluatedResults(ctx, prctx, client, evaluator, fc, res) - return result, err + return b.PostEvaluatedResults(ctx, prctx, client, fc, res) } func (b *Base) RequestReviewsForResult(ctx context.Context, prctx pull.Context, client *github.Client, trigger common.Trigger, result common.Result) error { diff --git a/server/handler/base_details.go b/server/handler/base_details.go index 7b90fd94..a405ce29 100644 --- a/server/handler/base_details.go +++ b/server/handler/base_details.go @@ -48,10 +48,10 @@ type DetailsData struct { PolicyURL string } -func (h *DetailsBase) getURLParams(w http.ResponseWriter, r *http.Request) (string, string, int, error) { +func (h *DetailsBase) getPrDetails(w http.ResponseWriter, r *http.Request) (string, string, int, error) { owner := pat.Param(r, "owner") repo := pat.Param(r, "repo") - //number := + number, err := strconv.Atoi(pat.Param(r, "number")) if err != nil { http.Error(w, fmt.Sprintf("Invalid pull request number: %v", err), http.StatusBadRequest) @@ -65,7 +65,6 @@ func (h *DetailsBase) generatePrContext(ctx context.Context, owner string, repo installation, err := h.Installations.GetByOwner(ctx, owner) if err != nil { if _, notFound := err.(githubapp.InstallationNotFound); notFound { - //h.render404(w, owner, repo, number) return nil, nil } return nil, err @@ -123,7 +122,6 @@ func (h *DetailsBase) getPolicyConfig(ctx context.Context, prCtx pull.Context, b func (h *DetailsBase) generateEvaluationDetails(w http.ResponseWriter, r *http.Request, policyConfig *FetchedConfig, prCtx pull.Context) (*DetailsData, *github.Client, common.Evaluator, error) { ctx := r.Context() - //logger := zerolog.Ctx(ctx) owner := prCtx.RepositoryOwner() repo := prCtx.RepositoryName() @@ -132,7 +130,6 @@ func (h *DetailsBase) generateEvaluationDetails(w http.ResponseWriter, r *http.R installation, err := h.Installations.GetByOwner(ctx, owner) if err != nil { if _, notFound := err.(githubapp.InstallationNotFound); notFound { - h.render404(w, owner, repo, number) return nil, nil, nil, errors.Wrap(err, "Unable to find installation") } return nil, nil, nil, err @@ -205,7 +202,6 @@ func (h *DetailsBase) render(w http.ResponseWriter, templateName string, data in w.Header().Set("Content-Type", "text/html") w.WriteHeader(http.StatusOK) - //fmt.Println(data) return h.Templates.ExecuteTemplate(w, templateName, data) } diff --git a/server/handler/details.go b/server/handler/details.go index 4bdceaf2..fb3a9c6e 100644 --- a/server/handler/details.go +++ b/server/handler/details.go @@ -26,7 +26,7 @@ type Details struct { func (h *Details) ServeHTTP(w http.ResponseWriter, r *http.Request) error { ctx := r.Context() - owner, repo, number, err := h.getURLParams(w, r) + owner, repo, number, err := h.getPrDetails(w, r) if err != nil { return err } @@ -40,7 +40,10 @@ func (h *Details) ServeHTTP(w http.ResponseWriter, r *http.Request) error { return errors.Wrap(err, "failed to get policy config") } - details, client, evaluator, _ := h.generateEvaluationDetails(w, r, policyConfig, prCtx) + details, client, evaluator, err := h.generateEvaluationDetails(w, r, policyConfig, prCtx) + if err != nil { + h.render404(w, owner, repo, number) + } result, _ := h.Base.EvaluateFetchedConfig(ctx, prCtx, client, evaluator, policyConfig) details.Result = &result diff --git a/server/handler/simulate.go b/server/handler/simulate.go index d56c23ad..3b93b75a 100644 --- a/server/handler/simulate.go +++ b/server/handler/simulate.go @@ -25,13 +25,13 @@ type Simulate struct { DetailsBase } -type fakePrContext struct { +type SimulatePullContext struct { pull.Context additionalApprovals []string } -func (f *fakePrContext) Reviews() ([]*pull.Review, error) { +func (f *SimulatePullContext) Reviews() ([]*pull.Review, error) { reviews, err := f.Context.Reviews() for _, username := range f.additionalApprovals { @@ -41,18 +41,18 @@ func (f *fakePrContext) Reviews() ([]*pull.Review, error) { return reviews, err } -func new(ctx pull.Context) *fakePrContext { - return &fakePrContext{Context: ctx} +func newSimulatePullContext(ctx pull.Context) *SimulatePullContext { + return &SimulatePullContext{Context: ctx} } -func (f *fakePrContext) addApproval(username string) { +func (f *SimulatePullContext) addApproval(username string) { f.additionalApprovals = append(f.additionalApprovals, username) } func (h *Simulate) ServeHTTP(w http.ResponseWriter, r *http.Request) error { ctx := r.Context() - owner, repo, number, err := h.getURLParams(w, r) + owner, repo, number, err := h.getPrDetails(w, r) if err != nil { return err } @@ -69,7 +69,7 @@ func (h *Simulate) ServeHTTP(w http.ResponseWriter, r *http.Request) error { if queryParams.Has("username") && queryParams.Get("username") != "" { username = queryParams.Get("username") - fakePrContext := new(prCtx) + fakePrContext := newSimulatePullContext(prCtx) fakePrContext.addApproval(username) prCtx = fakePrContext } @@ -79,7 +79,10 @@ func (h *Simulate) ServeHTTP(w http.ResponseWriter, r *http.Request) error { h.render404(w, owner, repo, number) return errors.Wrap(err, "failed to get policy config") } - details, client, evaluator, _ := h.generateEvaluationDetails(w, r, policyConfig, prCtx) + details, client, evaluator, err := h.generateEvaluationDetails(w, r, policyConfig, prCtx) + if err != nil { + h.render404(w, owner, repo, number) + } result := h.Base.EvaluateConfig(ctx, prCtx, client, evaluator, policyConfig) details.Result = &result diff --git a/server/templates/base-details.html.tmpl b/server/templates/base-details.html.tmpl index f10929c8..f0edc28d 100644 --- a/server/templates/base-details.html.tmpl +++ b/server/templates/base-details.html.tmpl @@ -3,6 +3,7 @@ {{define "scripts"}} + {{end}} {{define "body-class"}}bg-light-gray5 text-dark-gray1 flex flex-col h-screen{{end}} diff --git a/server/templates/details.html.tmpl b/server/templates/details.html.tmpl index 6cfb8730..cfdfef0e 100644 --- a/server/templates/details.html.tmpl +++ b/server/templates/details.html.tmpl @@ -1,12 +1,4 @@ {{/* templatetree:extends base-details.html.tmpl */}} -{{define "title"}}{{.PullRequest.GetBase.GetRepo.GetFullName}}#{{.PullRequest.GetNumber}} - Details | PolicyBot{{end}} - -{{define "scripts"}} - - -{{end}} - -{{define "body-class"}}bg-light-gray5 text-dark-gray1 flex flex-col h-screen{{end}} {{define "top"}}
    diff --git a/server/templates/simulate.html.tmpl b/server/templates/simulate.html.tmpl index e0e3704a..0d891ca1 100644 --- a/server/templates/simulate.html.tmpl +++ b/server/templates/simulate.html.tmpl @@ -1,10 +1,4 @@ {{/* templatetree:extends base-details.html.tmpl */}} -{{define "title"}}{{.PullRequest.GetBase.GetRepo.GetFullName}}#{{.PullRequest.GetNumber}} - Details | PolicyBot{{end}} - -{{define "scripts"}} - - -{{end}} {{define "body-class"}}bg-light-gray5 text-dark-gray1 flex flex-col h-screen{{end}} @@ -36,7 +30,7 @@
    - +

    The branch of this repository to find .policy.yml

    @@ -45,7 +39,7 @@
    - +

    A user to simulate an approval from