diff --git a/pullRequestHandler.go b/pullRequestHandler.go index 4fee661..032a870 100644 --- a/pullRequestHandler.go +++ b/pullRequestHandler.go @@ -1,6 +1,7 @@ package main import ( + "bytes" "context" "fmt" "strings" @@ -30,6 +31,10 @@ func handlePullRequest(req types.PullRequestOuter) { client := auth.MakeClient(ctx, token) + if !passBranchValidation(ctx, req, client) { + return + } + hasUnsignedCommits, err := hasUnsigned(req, client) if err != nil { @@ -124,6 +129,91 @@ func hasUnsigned(req types.PullRequestOuter, client *github.Client) (bool, error return hasUnsigned, err } +func passBranchValidation(ctx context.Context, req types.PullRequestOuter, client *github.Client) bool { + isValidPullRequest := validateHeadBranch(ctx, req, client) && validateBaseBranch(ctx, req, client) + + if !isValidPullRequest { + closePullRequest(req) + } + + return isValidPullRequest +} + +func validateHeadBranch(ctx context.Context, req types.PullRequestOuter, client *github.Client) bool { + if isMasterHeadBranch(req) { + body := "Thank you for your contribution. It appears that you are submitting changes directly from your master branch." + + "Please raise a new pull request from a named branch i.e. `git checkout -b my_feature`. " + + createComment(ctx, body, req, client) + + applyLabel(ctx, "review/source-branch", req, client) + } + return !isMasterHeadBranch(req) +} + +func validateBaseBranch(ctx context.Context, req types.PullRequestOuter, client *github.Client) bool { + if !isAgainstDefaultBranch(req) { + body := "Thank you for your contribution. It appears that you are submitting changes not against the default repository branch." + + "Please raise a new pull request againgst the default branch. " + + createComment(ctx, body, req, client) + + applyLabel(ctx, "review/target-branch", req, client) + } + + return isAgainstDefaultBranch(req) +} + +func isValidHeadAndBaseBranch(req types.PullRequestOuter) bool { + return !isMasterHeadBranch(req) && isAgainstDefaultBranch(req) +} + +func isMasterHeadBranch(req types.PullRequestOuter) bool { + return req.GetHeadBranch() == "master" +} + +func isAgainstDefaultBranch(req types.PullRequestOuter) bool { + return req.GetDefaultBranch() == req.GetBaseBranch() +} + func isSigned(msg string) bool { return strings.Contains(msg, "Signed-off-by:") } + +func createComment(ctx context.Context, body string, req types.PullRequestOuter, client *github.Client) { + + comment := &github.PullRequestComment{ + Body: &body, + } + + client.PullRequests.CreateComment(ctx, req.Repository.Owner.Login, req.Repository.Name, req.PullRequest.Number, comment) +} + +func applyLabel(ctx context.Context, label string, req types.PullRequestOuter, client *github.Client) { + fmt.Sprintf("Applying label %s to pull request %d", label, req.PullRequest.Number) + _, res, assignLabelErr := client.Issues.AddLabelsToIssue(ctx, req.Repository.Owner.Login, req.Repository.Name, req.PullRequest.Number, []string{label}) + if assignLabelErr != nil { + log.Fatalf("%s limit: %d, remaining: %d", assignLabelErr, res.Limit, res.Remaining) + } +} + +func closePullRequest(req types.PullRequestOuter) (string, error) { + var buffer bytes.Buffer + + newState, validTransition := checkTransition(closeConstant, req.PullRequest.State) + + if !validTransition { + buffer.WriteString(fmt.Sprintf("Cannot close pull request with a %s state", req.PullRequest.State)) + return buffer.String(), nil + } + + input := &github.PullRequest{State: &newState} + + client, ctx := makeClient(req.Installation.ID) + + _, _, err := client.PullRequests.Edit(ctx, req.Repository.Owner.Login, req.Repository.Name, req.PullRequest.Number, input) + if err != nil { + return buffer.String(), err + } + return buffer.String(), nil +} diff --git a/pullRequestHandler_test.go b/pullRequestHandler_test.go index 60c0b4b..afb6027 100644 --- a/pullRequestHandler_test.go +++ b/pullRequestHandler_test.go @@ -3,6 +3,7 @@ package main import ( "testing" + "github.com/alexellis/derek/types" "github.com/google/go-github/github" ) @@ -92,3 +93,99 @@ func Test_hasNoDcoLabel(t *testing.T) { }) } } + +func Test_isValidHeadAndBaseBranch(t *testing.T) { + + pullBranchOptions := []struct { + title string + headBranchName string + baseBranchName string + defaultBranchName string + expectedResult bool + }{ + { + title: "Incorrectly named master head branch. Base branch equal to default.", + headBranchName: "master", + baseBranchName: "master", + defaultBranchName: "master", + expectedResult: false, + }, + { + title: "Correctly named non-master head branch. Base branch equal to default.", + headBranchName: "test_branch", + baseBranchName: "master", + defaultBranchName: "master", + expectedResult: true, + }, + { + title: "Incorrectly named master head branch. Base branch not equal to default.", + headBranchName: "master", + baseBranchName: "master", + defaultBranchName: "development", + expectedResult: false, + }, + { + title: "Correctly named non-master head branch. Base branch not equal to default.", + headBranchName: "test_branch", + baseBranchName: "master", + defaultBranchName: "development", + expectedResult: false, + }, + { + title: "Correctly named non-master head branch. Base branch not equal to default.", + headBranchName: "development", + baseBranchName: "master", + defaultBranchName: "development", + expectedResult: false, + }, + { + title: "Correctly named non-master head branch. Base branch equal to default.", + headBranchName: "development", + baseBranchName: "development", + defaultBranchName: "development", + expectedResult: true, + }, + { + title: "Incorrectly named master head branch. Base branch equal to default.", + headBranchName: "master", + baseBranchName: "development", + defaultBranchName: "development", + expectedResult: false, + }, + { + title: "Correctly named master head branch. Base branch equal to default.", + headBranchName: "test_branch", + baseBranchName: "development", + defaultBranchName: "development", + expectedResult: true, + }, + } + for _, test := range pullBranchOptions { + t.Run(test.title, func(t *testing.T) { + repo := types.Repository{ + Name: "test_repo", + DefaultBranch: test.defaultBranchName, + } + headBranch := types.Branch{ + Repository: repo, + Name: test.headBranchName, + } + baseBranch := types.Branch{ + Repository: repo, + Name: test.baseBranchName, + } + req := types.PullRequestOuter{ + Repository: repo, + BaseBranch: baseBranch, + HeadBranch: headBranch, + } + + isValidHeadAndBaseBranch := isValidHeadAndBaseBranch(req) + + if isValidHeadAndBaseBranch != test.expectedResult { + t.Errorf("Is valid head and base branch (head: %s, base: %s, default: %s) - wanted: %t, found %t", + test.headBranchName, test.baseBranchName, test.defaultBranchName, test.expectedResult, isValidHeadAndBaseBranch) + } + }) + } +} diff --git a/types/types.go b/types/types.go index 3bf70ed..db2659a 100644 --- a/types/types.go +++ b/types/types.go @@ -1,8 +1,14 @@ package types type Repository struct { - Owner Owner `json:"owner"` - Name string `json:"name"` + Owner Owner `json:"owner"` + Name string `json:"name"` + DefaultBranch string `json:"default_branch"` +} + +type Branch struct { + Repository Repository `json:"repository"` + Name string `json:"ref"` } type Owner struct { @@ -11,7 +17,8 @@ type Owner struct { } type PullRequest struct { - Number int `json:"number"` + Number int `json:"number"` + State string `json:"state"` } type InstallationRequest struct { @@ -25,10 +32,24 @@ type ID struct { type PullRequestOuter struct { Repository Repository `json:"repository"` PullRequest PullRequest `json:"pull_request"` + BaseBranch Branch `json:"base"` + HeadBranch Branch `json:"head"` Action string `json:"action"` InstallationRequest } +func (req *PullRequestOuter) GetDefaultBranch() string { + return req.Repository.DefaultBranch +} + +func (req *PullRequestOuter) GetBaseBranch() string { + return req.BaseBranch.Name +} + +func (req *PullRequestOuter) GetHeadBranch() string { + return req.HeadBranch.Name +} + type IssueCommentOuter struct { Repository Repository `json:"repository"` Comment Comment `json:"comment"`