diff --git a/github/github.go b/github/github.go index 5878cc1..055c303 100644 --- a/github/github.go +++ b/github/github.go @@ -217,6 +217,25 @@ func CheckResponse(r *http.Response) error { return errorResponse } +// parseBoolResponse determines the boolean result from a GitHub API response. +// Several GitHub API methods return boolean responses indicated by the HTTP +// status code in the response (true indicated by a 204, false indicated by a +// 404). This helper function will determine that result and hide the 404 +// error if present. Any other error will be returned through as-is. +func parseBoolResponse(err error) (bool, error) { + if err == nil { + return true, nil + } + + if err, ok := err.(*ErrorResponse); ok && err.Response.StatusCode == http.StatusNotFound { + // Simply false. In this one case, we do not pass the error through. + return false, nil + } + + // some other real error occurred + return false, err +} + // API response wrapper to a rate limit request. type rateResponse struct { Rate *Rate `json:rate` diff --git a/github/github_test.go b/github/github_test.go index d3e66d1..4b967bc 100644 --- a/github/github_test.go +++ b/github/github_test.go @@ -218,6 +218,44 @@ func TestCheckResponse_noBody(t *testing.T) { } } +func TestParseBooleanResponse_true(t *testing.T) { + result, err := parseBoolResponse(nil) + + if err != nil { + t.Errorf("parseBoolResponse returned error: %+v", err) + } + + if want := true; result != want { + t.Errorf("parseBoolResponse returned %+v, want: %+v", result, want) + } +} + +func TestParseBooleanResponse_false(t *testing.T) { + v := &ErrorResponse{Response: &http.Response{StatusCode: http.StatusNotFound} } + result, err := parseBoolResponse(v) + + if err != nil { + t.Errorf("parseBoolResponse returned error: %+v", err) + } + + if want := false; result != want { + t.Errorf("parseBoolResponse returned %+v, want: %+v", result, want) + } +} + +func TestParseBooleanResponse_error(t *testing.T) { + v := &ErrorResponse{Response: &http.Response{StatusCode: http.StatusBadRequest} } + result, err := parseBoolResponse(v) + + if err == nil { + t.Errorf("Expected error to be returned.") + } + + if want := false; result != want { + t.Errorf("parseBoolResponse returned %+v, want: %+v", result, want) + } +} + func TestErrorResponse_Error(t *testing.T) { res := &http.Response{Request: &http.Request{}} err := ErrorResponse{Message: "m", Response: res} diff --git a/github/orgs.go b/github/orgs.go index 9654cad..641c223 100644 --- a/github/orgs.go +++ b/github/orgs.go @@ -8,7 +8,6 @@ package github import ( "fmt" - "net/http" "net/url" "strconv" "time" @@ -145,18 +144,7 @@ func (s *OrganizationsService) CheckMembership(org, user string) (bool, error) { } _, err = s.client.Do(req, nil) - if err != nil { - if err, ok := err.(*ErrorResponse); ok && err.Response.StatusCode == http.StatusNotFound { - // The user is not a member of the org. In this one case, we do not pass - // the error through. - return false, nil - } else { - // some other real error occurred - return false, err - } - } - - return err == nil, err + return parseBoolResponse(err) } // CheckPublicMembership checks if a user is a public member of an organization. @@ -170,18 +158,7 @@ func (s *OrganizationsService) CheckPublicMembership(org, user string) (bool, er } _, err = s.client.Do(req, nil) - if err != nil { - if err, ok := err.(*ErrorResponse); ok && err.Response.StatusCode == http.StatusNotFound { - // The user is not a member of the org. In this one case, we do not pass - // the error through. - return false, nil - } else { - // some other real error occurred - return false, err - } - } - - return true, nil + return parseBoolResponse(err) } // RemoveMember removes a user from all teams of an organization.