Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
23 changes: 23 additions & 0 deletions github/repos.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,16 @@ package github
import (
"context"
"encoding/json"
"errors"
"fmt"
"net/http"
"strings"
)

const githubBranchNotProtected string = "Branch not protected"

var ErrBranchNotProtected = errors.New("branch is not protected")

// RepositoriesService handles communication with the repository related
// methods of the GitHub API.
//
Expand Down Expand Up @@ -1009,6 +1014,9 @@ func (s *RepositoriesService) GetBranchProtection(ctx context.Context, owner, re
p := new(Protection)
resp, err := s.client.Do(ctx, req, p)
if err != nil {
if isBranchNotProtected(err) {
err = ErrBranchNotProtected
}
return nil, resp, err
}

Expand All @@ -1028,6 +1036,9 @@ func (s *RepositoriesService) GetRequiredStatusChecks(ctx context.Context, owner
p := new(RequiredStatusChecks)
resp, err := s.client.Do(ctx, req, p)
if err != nil {
if isBranchNotProtected(err) {
err = ErrBranchNotProtected
}
return nil, resp, err
}

Expand All @@ -1046,6 +1057,9 @@ func (s *RepositoriesService) ListRequiredStatusChecksContexts(ctx context.Conte

resp, err = s.client.Do(ctx, req, &contexts)
if err != nil {
if isBranchNotProtected(err) {
err = ErrBranchNotProtected
}
return nil, resp, err
}

Expand Down Expand Up @@ -1539,3 +1553,12 @@ func (s *RepositoriesService) Dispatch(ctx context.Context, owner, repo string,

return r, resp, nil
}

// isBranchNotProtected determines whether a branch is not protected
// based on the error message returned by GitHub API.
func isBranchNotProtected(err error) bool {
if errorResponse, ok := err.(*ErrorResponse); ok {
return errorResponse.Message == githubBranchNotProtected
}
return false
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if errorResponse, ok := err.(*ErrorResponse); ok {
return errorResponse.Message == githubBranchNotProtected
}
return false
errorResponse, ok := err.(*ErrorResponse)
return ok && errorResponse.Message == githubBranchNotProtected

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gmlewis Thank you for suggesting this change. I certainly missed this simplification.

}
78 changes: 78 additions & 0 deletions github/repos_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1150,6 +1150,32 @@ func TestRepositoriesService_GetBranchProtection_noDismissalRestrictions(t *test
}
}

func TestRepositoriesService_GetBranchProtection_branchNotProtected(t *testing.T) {
client, mux, _, teardown := setup()
defer teardown()

mux.HandleFunc("/repos/o/r/branches/b/protection", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, "GET")

w.WriteHeader(http.StatusBadRequest)
fmt.Fprintf(w, `{
"message": "Branch not protected",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"message": "Branch not protected",
"message": githubBranchNotProtected,

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gmlewis I'm not sure if we'll be able to use a variable inside a raw string. Am I missing something..?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whups, you are correct. Please use %v and the variable name in that case.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, %q makes more sense here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @gmlewis. I've updated the PR with the suggested changes.

"documentation_url": "https://docs.github.com/rest/reference/repos#get-branch-protection"
}`)
})

ctx := context.Background()
protection, _, err := client.Repositories.GetBranchProtection(ctx, "o", "r", "b")

if protection != nil {
t.Errorf("Repositories.GetBranchProtection returned non-nil protection data")
}

if err != ErrBranchNotProtected {
t.Errorf("Repositories.GetBranchProtection returned an invalid error: %v", err)
}
}

func TestRepositoriesService_UpdateBranchProtection(t *testing.T) {
client, mux, _, teardown := setup()
defer teardown()
Expand Down Expand Up @@ -1387,6 +1413,32 @@ func TestRepositoriesService_GetRequiredStatusChecks(t *testing.T) {
})
}

func TestRepositoriesService_GetRequiredStatusChecks_branchNotProtected(t *testing.T) {
client, mux, _, teardown := setup()
defer teardown()

mux.HandleFunc("/repos/o/r/branches/b/protection/required_status_checks", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, "GET")

w.WriteHeader(http.StatusBadRequest)
fmt.Fprintf(w, `{
"message": "Branch not protected",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"message": "Branch not protected",
"message": githubBranchNotProtected,

"documentation_url": "https://docs.github.com/rest/reference/repos#get-branch-protection"
}`)
})

ctx := context.Background()
checks, _, err := client.Repositories.GetRequiredStatusChecks(ctx, "o", "r", "b")

if checks != nil {
t.Errorf("Repositories.GetRequiredStatusChecks returned non-nil status-checks data")
}

if err != ErrBranchNotProtected {
t.Errorf("Repositories.GetRequiredStatusChecks returned an invalid error: %v", err)
}
}

func TestRepositoriesService_UpdateRequiredStatusChecks(t *testing.T) {
client, mux, _, teardown := setup()
defer teardown()
Expand Down Expand Up @@ -1502,6 +1554,32 @@ func TestRepositoriesService_ListRequiredStatusChecksContexts(t *testing.T) {
})
}

func TestRepositoriesService_ListRequiredStatusChecksContexts_branchNotProtected(t *testing.T) {
client, mux, _, teardown := setup()
defer teardown()

mux.HandleFunc("/repos/o/r/branches/b/protection/required_status_checks/contexts", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, "GET")

w.WriteHeader(http.StatusBadRequest)
fmt.Fprintf(w, `{
"message": "Branch not protected",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"message": "Branch not protected",
"message": githubBranchNotProtected,

"documentation_url": "https://docs.github.com/rest/reference/repos#get-branch-protection"
}`)
})

ctx := context.Background()
contexts, _, err := client.Repositories.ListRequiredStatusChecksContexts(ctx, "o", "r", "b")

if contexts != nil {
t.Errorf("Repositories.ListRequiredStatusChecksContexts returned non-nil contexts data")
}

if err != ErrBranchNotProtected {
t.Errorf("Repositories.ListRequiredStatusChecksContexts returned an invalid error: %v", err)
}
}

func TestRepositoriesService_GetPullRequestReviewEnforcement(t *testing.T) {
client, mux, _, teardown := setup()
defer teardown()
Expand Down