Skip to content

Add Preview Support for Team Review Requests #682

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dmitshur merged 4 commits into from
Aug 9, 2017
Merged

Add Preview Support for Team Review Requests #682

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b17b12c
Add Preview Support for Team Review Requests
RaviTezu Aug 6, 2017
b505039
Bump the libraryVersion as this change involves a breaking API and al…
RaviTezu Aug 7, 2017
de6c06b
Making the suggested API changes
RaviTezu Aug 9, 2017
cb67572
Reordering the structs; as RevieweresRequest struct is used for input…
RaviTezu Aug 9, 2017
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
5 changes: 4 additions & 1 deletion github/github.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import (
)

const (
libraryVersion = "8"
libraryVersion = "9"
defaultBaseURL = "https://api.github.com/"
uploadBaseURL = "https://uploads.github.com/"
userAgent = "go-github/" + libraryVersion
Expand Down Expand Up @@ -99,6 +99,9 @@ const (

// https://developer.github.com/changes/2017-07-17-update-topics-on-repositories/
mediaTypeTopicsPreview = "application/vnd.github.mercy-preview+json"

// https://developer.github.com/changes/2017-07-26-team-review-request-thor-preview/
mediaTypeTeamReviewPreview = "application/vnd.github.thor-preview+json"
)

// A Client manages communication with the GitHub API.
Expand Down
51 changes: 30 additions & 21 deletions github/pulls_reviewers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,22 +10,31 @@ import (
"fmt"
)

// RequestReviewers creates a review request for the provided GitHub users for the specified pull request.
// ReviewersRequest specifies users and teams for a pull request review request.
type ReviewersRequest struct {
Reviewers []string `json:"reviewers,omitempty"`
TeamReviewers []string `json:"team_reviewers,omitempty"`
}

@dmitshur dmitshur Aug 9, 2017 •
edited
Loading

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.

Minor, swap the order of these two structs, so ReviewersRequest, the input, is on top, and Reviewers, the output, is below it.

@RaviTezu RaviTezu Aug 9, 2017

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.

@shurcooL Done.


// Reviewers represents reviewers of a pull request.
type Reviewers struct {
Users []*User `json:"users,omitempty"`
Teams []*Team `json:"teams,omitempty"`
}

// RequestReviewers creates a review request for the provided reviewers for the specified pull request.
//
// GitHub API docs: https://developer.github.com/v3/pulls/review_requests/#create-a-review-request
func (s *PullRequestsService) RequestReviewers(ctx context.Context, owner, repo string, number int, logins []string) (*PullRequest, *Response, error) {
func (s *PullRequestsService) RequestReviewers(ctx context.Context, owner, repo string, number int, reviewers ReviewersRequest) (*PullRequest, *Response, error) {
u := fmt.Sprintf("repos/%s/%s/pulls/%d/requested_reviewers", owner, repo, number)

reviewers := struct {
Reviewers []string `json:"reviewers,omitempty"`
}{
Reviewers: logins,
}
req, err := s.client.NewRequest("POST", u, &reviewers)
if err != nil {
return nil, nil, err
}

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

r := new(PullRequest)
resp, err := s.client.Do(ctx, req, r)
if err != nil {
Expand All @@ -35,10 +44,10 @@ func (s *PullRequestsService) RequestReviewers(ctx context.Context, owner, repo
return r, resp, nil
}

// ListReviewers lists users whose reviews have been requested on the specified pull request.
// ListReviewers lists reviewers whose reviews have been requested on the specified pull request.
//
// GitHub API docs: https://developer.github.com/v3/pulls/review_requests/#list-review-requests
func (s *PullRequestsService) ListReviewers(ctx context.Context, owner, repo string, number int, opt *ListOptions) ([]*User, *Response, error) {
func (s *PullRequestsService) ListReviewers(ctx context.Context, owner, repo string, number int, opt *ListOptions) (*Reviewers, *Response, error) {
u := fmt.Sprintf("repos/%v/%v/pulls/%d/requested_reviewers", owner, repo, number)
u, err := addOptions(u, opt)
if err != nil {
Expand All @@ -50,30 +59,30 @@ func (s *PullRequestsService) ListReviewers(ctx context.Context, owner, repo str
return nil, nil, err
}

var users []*User
resp, err := s.client.Do(ctx, req, &users)
// TODO: remove custom Accept header when this API fully launches.
req.Header.Set("Accept", mediaTypeTeamReviewPreview)

reviewers := new(Reviewers)
resp, err := s.client.Do(ctx, req, reviewers)
if err != nil {
return nil, resp, err
}

return users, resp, nil
return reviewers, resp, nil
}

// RemoveReviewers removes the review request for the provided GitHub users for the specified pull request.
// RemoveReviewers removes the review request for the provided reviewers for the specified pull request.
//
// GitHub API docs: https://developer.github.com/v3/pulls/review_requests/#delete-a-review-request
func (s *PullRequestsService) RemoveReviewers(ctx context.Context, owner, repo string, number int, logins []string) (*Response, error) {
func (s *PullRequestsService) RemoveReviewers(ctx context.Context, owner, repo string, number int, reviewers ReviewersRequest) (*Response, error) {
u := fmt.Sprintf("repos/%s/%s/pulls/%d/requested_reviewers", owner, repo, number)

reviewers := struct {
Reviewers []string `json:"reviewers,omitempty"`
}{
Reviewers: logins,
}
req, err := s.client.NewRequest("DELETE", u, &reviewers)
if err != nil {
return nil, err
}

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

return s.client.Do(ctx, req, reviewers)
}
31 changes: 21 additions & 10 deletions github/pulls_reviewers_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,13 @@ func TestRequestReviewers(t *testing.T) {

mux.HandleFunc("/repos/o/r/pulls/1/requested_reviewers", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, "POST")
testBody(t, r, `{"reviewers":["octocat","googlebot"]}`+"\n")
testBody(t, r, `{"reviewers":["octocat","googlebot"],"team_reviewers":["justice-league","injustice-league"]}`+"\n")
testHeader(t, r, "Accept", mediaTypeTeamReviewPreview)
fmt.Fprint(w, `{"number":1}`)
})

// This returns a PR, unmarshalling of which is tested elsewhere
pull, _, err := client.PullRequests.RequestReviewers(context.Background(), "o", "r", 1, []string{"octocat", "googlebot"})
pull, _, err := client.PullRequests.RequestReviewers(context.Background(), "o", "r", 1, ReviewersRequest{Reviewers: []string{"octocat", "googlebot"}, TeamReviewers: []string{"justice-league", "injustice-league"}})
if err != nil {
t.Errorf("PullRequests.RequestReviewers returned error: %v", err)
}
Expand All @@ -40,10 +41,11 @@ func TestRemoveReviewers(t *testing.T) {

mux.HandleFunc("/repos/o/r/pulls/1/requested_reviewers", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, "DELETE")
testBody(t, r, `{"reviewers":["octocat","googlebot"]}`+"\n")
testHeader(t, r, "Accept", mediaTypeTeamReviewPreview)
testBody(t, r, `{"reviewers":["octocat","googlebot"],"team_reviewers":["justice-league"]}`+"\n")
})

_, err := client.PullRequests.RemoveReviewers(context.Background(), "o", "r", 1, []string{"octocat", "googlebot"})
_, err := client.PullRequests.RemoveReviewers(context.Background(), "o", "r", 1, ReviewersRequest{Reviewers: []string{"octocat", "googlebot"}, TeamReviewers: []string{"justice-league"}})
if err != nil {
t.Errorf("PullRequests.RemoveReviewers returned error: %v", err)
}
Expand All @@ -55,18 +57,27 @@ func TestListReviewers(t *testing.T) {

mux.HandleFunc("/repos/o/r/pulls/1/requested_reviewers", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, "GET")
fmt.Fprint(w, `[{"login":"octocat","id":1}]`)
testHeader(t, r, "Accept", mediaTypeTeamReviewPreview)
fmt.Fprint(w, `{"users":[{"login":"octocat","id":1}],"teams":[{"id":1,"name":"Justice League"}]}`)
})

reviewers, _, err := client.PullRequests.ListReviewers(context.Background(), "o", "r", 1, nil)
if err != nil {
t.Errorf("PullRequests.ListReviewers returned error: %v", err)
}

want := []*User{
{
Login: String("octocat"),
ID: Int(1),
want := &Reviewers{
Users: []*User{
{
Login: String("octocat"),
ID: Int(1),
},
},
Teams: []*Team{
{
ID: Int(1),
Name: String("Justice League"),
},
},
}
if !reflect.DeepEqual(reviewers, want) {
Expand All @@ -83,7 +94,7 @@ func TestListReviewers_withOptions(t *testing.T) {
testFormValues(t, r, values{
"page": "2",
})
fmt.Fprint(w, `[]`)
fmt.Fprint(w, `{}`)
})

_, _, err := client.PullRequests.ListReviewers(context.Background(), "o", "r", 1, &ListOptions{Page: 2})
Expand Down