fix: Handle GitHub IsMember/IsCollaborator 404s properly

fixes jenkins-x#105 Signed-off-by: Andrew Bayer <[email protected]>
eddycharly · Apr 24, 2020 · 9bdccb1 · 9bdccb1
1 parent aad099b
commit 9bdccb1
Show file tree

Hide file tree

Showing 4 changed files with 94 additions and 2 deletions.
diff --git a/scm/driver/github/org.go b/scm/driver/github/org.go
@@ -51,7 +51,7 @@ type membership struct {
 func (s *organizationService) IsMember(ctx context.Context, org string, user string) (bool, *scm.Response, error) {
 	path := fmt.Sprintf("orgs/%s/members/%s", org, user)
 	res, err := s.client.do(ctx, "GET", path, nil, nil)
-	if err != nil {
+	if err != nil && res == nil {
 		return false, res, err
 	}
 	code := res.Status

diff --git a/scm/driver/github/org_test.go b/scm/driver/github/org_test.go
@@ -237,3 +237,49 @@ func TestIsAdminFalse(t *testing.T) {
 
 	assert.False(t, isAdmin)
 }
+
+func TestOrganizationService_IsMember_False(t *testing.T) {
+	defer gock.Off()
+
+	gock.New("https://api.github.com").
+		Get("/orgs/octocat/members/someuser").
+		Reply(404).
+		SetHeaders(mockHeaders)
+
+	client := NewDefault()
+	got, res, err := client.Organizations.IsMember(context.Background(), "octocat", "someuser")
+	if err != nil {
+		t.Error(err)
+		return
+	}
+
+	if got {
+		t.Errorf("Expected user to not be a member")
+	}
+
+	t.Run("Request", testRequest(res))
+	t.Run("Rate", testRate(res))
+}
+
+func TestOrganizationService_IsMember_True(t *testing.T) {
+	defer gock.Off()
+
+	gock.New("https://api.github.com").
+		Get("/orgs/octocat/members/someuser").
+		Reply(204).
+		SetHeaders(mockHeaders)
+
+	client := NewDefault()
+	got, res, err := client.Organizations.IsMember(context.Background(), "octocat", "someuser")
+	if err != nil {
+		t.Error(err)
+		return
+	}
+
+	if !got {
+		t.Errorf("Expected user to be a member")
+	}
+
+	t.Run("Request", testRequest(res))
+	t.Run("Rate", testRate(res))
+}
diff --git a/scm/driver/github/repo.go b/scm/driver/github/repo.go
@@ -81,7 +81,7 @@ func (s *repositoryService) IsCollaborator(ctx context.Context, repo, user strin
 		},
 	}
 	res, err := s.client.doRequest(ctx, req, nil, nil)
-	if err != nil {
+	if err != nil && res == nil {
 		return false, res, err
 	}
 	code := res.Status

diff --git a/scm/driver/github/repo_test.go b/scm/driver/github/repo_test.go
@@ -556,3 +556,49 @@ func TestHookEvents(t *testing.T) {
 		}
 	}
 }
+
+func TestRepositoryService_IsCollaborator_False(t *testing.T) {
+	defer gock.Off()
+
+	gock.New("https://api.github.com").
+		Get("/repos/octocat/hello-world/collaborators/someuser").
+		Reply(404).
+		SetHeaders(mockHeaders)
+
+	client := NewDefault()
+	got, res, err := client.Repositories.IsCollaborator(context.Background(), "octocat/hello-world", "someuser")
+	if err != nil {
+		t.Error(err)
+		return
+	}
+
+	if got {
+		t.Errorf("Expected user to not be a collaborator")
+	}
+
+	t.Run("Request", testRequest(res))
+	t.Run("Rate", testRate(res))
+}
+
+func TestRepositoryService_IsCollaborator_True(t *testing.T) {
+	defer gock.Off()
+
+	gock.New("https://api.github.com").
+		Get("/repos/octocat/hello-world/collaborators/someuser").
+		Reply(204).
+		SetHeaders(mockHeaders)
+
+	client := NewDefault()
+	got, res, err := client.Repositories.IsCollaborator(context.Background(), "octocat/hello-world", "someuser")
+	if err != nil {
+		t.Error(err)
+		return
+	}
+
+	if !got {
+		t.Errorf("Expected user to be a collaborator")
+	}
+
+	t.Run("Request", testRequest(res))
+	t.Run("Rate", testRate(res))
+}