From 9bdccb1ad65887ac0aed01f41ecd7cab320a8de2 Mon Sep 17 00:00:00 2001 From: Andrew Bayer Date: Fri, 24 Apr 2020 10:08:09 -0400 Subject: [PATCH] fix: Handle GitHub IsMember/IsCollaborator 404s properly fixes #105 Signed-off-by: Andrew Bayer --- scm/driver/github/org.go | 2 +- scm/driver/github/org_test.go | 46 ++++++++++++++++++++++++++++++++++ scm/driver/github/repo.go | 2 +- scm/driver/github/repo_test.go | 46 ++++++++++++++++++++++++++++++++++ 4 files changed, 94 insertions(+), 2 deletions(-) diff --git a/scm/driver/github/org.go b/scm/driver/github/org.go index 1aeac961f..fa79c3508 100644 --- 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 index 6044eed72..e976b8bdb 100644 --- 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 index 093ace11d..28cfea82d 100644 --- 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 index f7d058a2b..0e9501cda 100644 --- 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)) +}