Skip to content

Commit

Permalink
Add PR branch validation
Browse files Browse the repository at this point in the history
This checks if the branch a pull request is raised from is non-master.
Validates, as well, if a pull request is raised against the default
branch.

Signed-off-by: Ivana Yovcheva (VMware) <[email protected]>
  • Loading branch information
ivanayov committed Apr 4, 2018
1 parent 1922233 commit 965d216
Show file tree
Hide file tree
Showing 3 changed files with 176 additions and 3 deletions.
67 changes: 67 additions & 0 deletions pullRequestHandler.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package main

import (
"bytes"
"context"
"fmt"
"strings"
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -124,6 +129,68 @@ 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 {
if !isValidHeadAndBaseBranch(req) {
body := `Thank you for your contribution. You have opened a pull request from master branch or against the non-default branch.
Please rename your branch to a non-master and reopen the pull request against the default branch. Thank you!
Closing. `

comment := &github.IssueComment{
Body: &body,
}

client.Issues.CreateComment(ctx, req.Repository.Owner.Login, req.Repository.Name, req.PullRequest.Number, comment)

closePullRequest(req)
}
return isValidHeadAndBaseBranch(req)
}

func isValidHeadAndBaseBranch(req types.PullRequestOuter) bool {
return !isMasterHeadBranch(req) && isAgainstDefaultBranch(req)
}

func getDefaultBranch(req types.PullRequestOuter) string {
return req.Repository.DefaultBranch
}

func getBaseBranch(req types.PullRequestOuter) string {
return req.BaseBranch.Name
}

func getHeadBranch(req types.PullRequestOuter) string {
return req.HeadBranch.Name
}

func isMasterHeadBranch(req types.PullRequestOuter) bool {
return req.HeadBranch.Name == "master"
}

func isAgainstDefaultBranch(req types.PullRequestOuter) bool {
return getDefaultBranch(req) == getBaseBranch(req)
}

func isSigned(msg string) bool {
return strings.Contains(msg, "Signed-off-by:")
}

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
}
97 changes: 97 additions & 0 deletions pullRequestHandler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package main
import (
"testing"

"github.com/alexellis/derek/types"
"github.com/google/go-github/github"
)

Expand Down Expand Up @@ -92,3 +93,99 @@ func Test_hasNoDcoLabel(t *testing.T) {
})
}
}

func Test_isValidHeadAndBaseBranch(t *testing.T) {

var actionOptions = []struct {
title string
headBranchName string
baseBranchName string
defaultBranchName string
expectedBool bool
}{
{
title: "Incorrectly named master head branch. Base branch equal to default.",
headBranchName: "master",
baseBranchName: "master",
defaultBranchName: "master",
expectedBool: false,
},
{
title: "Correctly named non-master head branch. Base branch equal to default.",
headBranchName: "test_branch",
baseBranchName: "master",
defaultBranchName: "master",
expectedBool: true,
},
{
title: "Incorrectly named master head branch. Base branch not equal to default.",
headBranchName: "master",
baseBranchName: "master",
defaultBranchName: "development",
expectedBool: false,
},
{
title: "Correctly named non-master head branch. Base branch not equal to default.",
headBranchName: "test_branch",
baseBranchName: "master",
defaultBranchName: "development",
expectedBool: false,
},
{
title: "Correctly named non-master head branch. Base branch not equal to default.",
headBranchName: "development",
baseBranchName: "master",
defaultBranchName: "development",
expectedBool: false,
},
{
title: "Correctly named non-master head branch. Base branch equal to default.",
headBranchName: "development",
baseBranchName: "development",
defaultBranchName: "development",
expectedBool: true,
},
{
title: "Incorrectly named master head branch. Base branch equal to default.",
headBranchName: "master",
baseBranchName: "development",
defaultBranchName: "development",
expectedBool: false,
},
{
title: "Correctly named master head branch. Base branch equal to default.",
headBranchName: "test_branch",
baseBranchName: "development",
defaultBranchName: "development",
expectedBool: true,
},
}
for _, test := range actionOptions {
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.expectedBool {
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.expectedBool, isValidHeadAndBaseBranch)
}
})
}
}
15 changes: 12 additions & 3 deletions types/types.go
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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 {
Expand All @@ -25,6 +32,8 @@ 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
}
Expand Down

0 comments on commit 965d216

Please sign in to comment.