Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
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
50 changes: 50 additions & 0 deletions github/orgs_outside_collaborators.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
// Copyright 2017 The go-github AUTHORS. All rights reserved.
//
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

package github

import "fmt"

// ListOutsideCollaboratorsOptions specifies optional parameters to the
// OrganizationsService.ListOutsideCollaborators method.
type ListOutsideCollaboratorsOptions struct {
// Filter outside collaborators returned in the list. Possible values are:
// 2fa_disabled, all. Default is "all".
Filter string `url:"filter,omitempty"`

ListOptions
}

// ListOutsideCollaborators lists outside collaborators of organization's repositories.
// This will only work if the authenticated
// user is an owner of the organization.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nitpick, the sentence "This will only work if the authenticated user is an owner of the organization." can probably fit on a single line, instead of being split into two lines of lengths 40 and 37 characaters.

Also, question, what's the source of this statement? I don't see it mentioned in https://developer.github.com/v3/orgs/outside_collaborators/#list-outside-collaborators.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

While trying to use the API by hand I was informed (with an error) that only the owner can do this.

//
// Warning: The API may change without advance notice during the preview period.
// Preview features are not supported for production use.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We don't usually include this warning in all other similar situations. Should we start doing it here?

It'll create more work and might get out of date. We have the // TODO: remove custom Accept header when this API fully launches. comments in code, and GitHub API itself is a good source for knowing which endpoints are stable or not.

I'm not against it, just a thought, because this is the first comment of its kind.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's not the first one, I copied it from another file using the same Accept header. (and thought that's the convention). I'll find the file when I'll get back home.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oh okay, keeping it is perfectly fine then. Thanks!

//
// GitHub API docs: https://developer.github.com/v3/orgs/outside_collaborators/#list-outside-collaborators
func (s *OrganizationsService) ListOutsideCollaborators(org string, opt *ListOutsideCollaboratorsOptions) ([]*User, *Response, error) {
u := fmt.Sprintf("orgs/%v/outside_collaborators", org)
u, err := addOptions(u, opt)
if err != nil {
return nil, nil, err
}

req, err := s.client.NewRequest("GET", u, nil)
if err != nil {
return nil, nil, err
}

// TODO: remove custom Accept header when this API fully launches.
req.Header.Set("Accept", mediaTypeOrgMembershipPreview)

var members []*User
resp, err := s.client.Do(req, &members)
if err != nil {
return nil, resp, err
}

return members, resp, nil
}
47 changes: 47 additions & 0 deletions github/orgs_outside_collaborators_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// Copyright 2017 The go-github AUTHORS. All rights reserved.
//
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

package github

import (
"fmt"
"net/http"
"reflect"
"testing"
)

func TestOrganizationsService_ListOutsideCollaborators(t *testing.T) {
setup()
defer teardown()

mux.HandleFunc("/orgs/o/outside_collaborators", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, "GET")
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

please add test for preview header... see other examples in repo

testFormValues(t, r, values{
"filter": "2fa_disabled",
"page": "2",
})
testHeader(t, r, "Accept", mediaTypeOrgMembershipPreview)
fmt.Fprint(w, `[{"id":1}]`)
})

opt := &ListOutsideCollaboratorsOptions{
Filter: "2fa_disabled",
ListOptions: ListOptions{Page: 2},
}
members, _, err := client.Organizations.ListOutsideCollaborators("o", opt)
if err != nil {
t.Errorf("Organizations.ListOutsideCollaborators returned error: %v", err)
}

want := []*User{{ID: Int(1)}}
if !reflect.DeepEqual(members, want) {
t.Errorf("Organizations.ListOutsideCollaborators returned %+v, want %+v", members, want)
}
}

func TestOrganizationsService_ListOutsideCollaborators_invalidOrg(t *testing.T) {
_, _, err := client.Organizations.ListOutsideCollaborators("%", nil)
testURLParseError(t, err)
}