Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add PR branch validation #47

Closed
wants to merge 1 commit into from
Closed

Add PR branch validation #47

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 56 additions & 0 deletions handler/pullRequestHandler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,33 @@ func HandlePullRequest(req types.PullRequestOuter, contributingURL string, confi
}
}

if validateBranch, validateBranchMessage, sourceBranchReviewLabel, targetBranchReviewLabel := validatePullRequestBranch(req); !validateBranch {

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

comment, resp, err := client.Issues.CreateComment(ctx, req.Repository.Owner.Login, req.Repository.Name, req.PullRequest.Number, comment)
if err != nil {
log.Fatalf("Failed on branch validation: %s", err)
log.Fatal(err)
}
fmt.Println(comment, resp.Rate)

if len(sourceBranchReviewLabel) > 0 {
_, res, assignLabelErr := client.Issues.AddLabelsToIssue(ctx, req.Repository.Owner.Login, req.Repository.Name, req.PullRequest.Number, []string{sourceBranchReviewLabel})
if assignLabelErr != nil {
log.Fatalf("%s limit: %d, remaining: %d", assignLabelErr, res.Limit, res.Remaining)
}
}
if len(targetBranchReviewLabel) > 0 {
_, res, assignLabelErr := client.Issues.AddLabelsToIssue(ctx, req.Repository.Owner.Login, req.Repository.Name, req.PullRequest.Number, []string{targetBranchReviewLabel})
if assignLabelErr != nil {
log.Fatalf("%s limit: %d, remaining: %d", assignLabelErr, res.Limit, res.Remaining)
}
}
}

if err != nil {
log.Fatal(err)
} else if hasUnsignedCommits {
Expand Down Expand Up @@ -200,3 +227,32 @@ func isSigned(msg string) bool {
func hasDescription(pr types.PullRequest) bool {
return len(strings.TrimSpace(pr.Body)) > 0
}

func validatePullRequestBranch(req types.PullRequestOuter) (bool, string, string, string) {

validHeadBranch, messageFromHeadBranchValidation, sourceBranchReviewLabel := pullRequestIsNotOpenFromMasterBranch(req)
validBaseBranch, messageFromBaseBranchValidation, targetBranchReviewLabel := pullRequestIsOpenAgainstDefaultBranch(req)

return validHeadBranch && validBaseBranch, messageFromHeadBranchValidation + messageFromBaseBranchValidation, sourceBranchReviewLabel, targetBranchReviewLabel

}

func pullRequestIsOpenAgainstDefaultBranch(req types.PullRequestOuter) (bool, string, string) {
if req.GetDefaultBranch() != req.GetBaseBranch() {
message := fmt.Sprintf("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: %s\n", req.GetDefaultBranch())
label := "review/target-branch"
return false, message, label
}
return true, "", ""
}

func pullRequestIsNotOpenFromMasterBranch(req types.PullRequestOuter) (bool, string, string) {
if req.GetHeadBranch() == "master" {
message := "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`.\n"
label := "review/source-branch"
return false, message, label
}
return true, "", ""
}
129 changes: 129 additions & 0 deletions handler/pullRequestHandler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,3 +124,132 @@ func Test_hasDescription(t *testing.T) {
}
}
}

func Test_validatePullRequestBranch(t *testing.T) {

pullBranchOptions := []struct {
title string
headBranchName string
baseBranchName string
defaultBranchName string
expectedResult bool
expectedMessage string
expectedLabels string
}{
{
title: "Incorrectly named master head branch. Base branch equal to default.",
headBranchName: "master",
baseBranchName: "master",
defaultBranchName: "master",
expectedResult: false,
expectedMessage: "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`.\n",
expectedLabels: "review/source-branch",
},
{
title: "Correctly named non-master head branch. Base branch equal to default.",
headBranchName: "test_branch",
baseBranchName: "master",
defaultBranchName: "master",
expectedResult: true,
expectedMessage: "",
expectedLabels: "",
},
{
title: "Incorrectly named master head branch. Base branch not equal to default.",
headBranchName: "master",
baseBranchName: "master",
defaultBranchName: "development",
expectedResult: false,
expectedMessage: "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`.\n" +
"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: development\n",
expectedLabels: "review/source-branchreview/target-branch",
},
{
title: "Correctly named non-master head branch. Base branch not equal to default.",
headBranchName: "test_branch",
baseBranchName: "master",
defaultBranchName: "development",
expectedResult: false,
expectedMessage: "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: development\n",
expectedLabels: "review/target-branch",
},
{
title: "Correctly named non-master head branch. Base branch not equal to default.",
headBranchName: "development",
baseBranchName: "master",
defaultBranchName: "development",
expectedResult: false,
expectedMessage: "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: development\n",
expectedLabels: "review/target-branch",
},
{
title: "Correctly named non-master head branch. Base branch equal to default.",
headBranchName: "development",
baseBranchName: "development",
defaultBranchName: "development",
expectedResult: true,
expectedMessage: "",
expectedLabels: "",
},
{
title: "Incorrectly named master head branch. Base branch equal to default.",
headBranchName: "master",
baseBranchName: "development",
defaultBranchName: "development",
expectedResult: false,
expectedMessage: "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`.\n",
expectedLabels: "review/source-branch",
},
{
title: "Correctly named master head branch. Base branch equal to default.",
headBranchName: "test_branch",
baseBranchName: "development",
defaultBranchName: "development",
expectedResult: true,
expectedMessage: "",
expectedLabels: "",
},
}
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,
}

validHeadAndBaseBranch, message, headBranchLabel, baseBranchLabel := validatePullRequestBranch(req)

if validHeadAndBaseBranch != test.expectedResult {
t.Errorf("Valid head and base branch check (head: %s, base: %s, default: %s) - want: %t, got %t",
test.headBranchName, test.baseBranchName, test.defaultBranchName, test.expectedResult, validHeadAndBaseBranch)
}
if message != test.expectedMessage {
t.Errorf("Valid head and base branch message check (head: %s, base: %s, default: %s) - want: %s, got %s",
test.headBranchName, test.baseBranchName, test.defaultBranchName, test.expectedMessage, message)
}
if headBranchLabel+baseBranchLabel != test.expectedLabels {
t.Errorf("Valid head and base branch message check (head: %s, base: %s, default: %s) - want: %s, got %s",
test.headBranchName, test.baseBranchName, test.defaultBranchName, test.expectedLabels, headBranchLabel+baseBranchLabel)
}
})
}
}
36 changes: 34 additions & 2 deletions types/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,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 @@ -17,6 +23,7 @@ type PullRequest struct {
Number int `json:"number"`
AuthorAssociation string `json:"author_association"`
Body string `json:"body"`
State string `json:"state"`
}

type InstallationRequest struct {
Expand All @@ -30,10 +37,35 @@ 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
}

/*
* The default branch of a repository.
* Usually set to `master`
*/
func (req *PullRequestOuter) GetDefaultBranch() string {
return req.Repository.DefaultBranch
}

/*
* The branch a pull request is open against.
* It should be the default branch.
*/
func (req *PullRequestOuter) GetBaseBranch() string {
return req.BaseBranch.Name
}

/*
* The branch a pull request is open from
*/
func (req *PullRequestOuter) GetHeadBranch() string {
return req.HeadBranch.Name
}

type IssueCommentOuter struct {
Repository Repository `json:"repository"`
Comment Comment `json:"comment"`
Expand Down