Skip to content
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.

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

#720 Initial commit for listing Plans and Plan Accounts #732

Merged
merged 14 commits into from
Nov 14, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
109 changes: 109 additions & 0 deletions github/apps_marketplace.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
// Copyright 2013 The go-github AUTHORS. All rights reserved.
Copy link
Collaborator

Choose a reason for hiding this comment

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

s/2013/2017/

//
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

package github

import (
"context"
"fmt"
)

// MarketplaceService handles communication with the marketplace related
// methods of the GitHub API.
//
// GitHub API docs: https://developer.github.com/v3/apps/marketplace/
type MarketplaceService service

// MarketplacePlan represents a GitHub Apps Marketplace Listing Plan.
type MarketplacePlan struct {
URL *string `json:"url"`
AccountsURL *string `json:"accounts_url"`
ID *int `json:"id"`
Name *string `json:"name"`
Description *string `json:"description"`
MonthlyPriceInCents *int `json:"monthly_price_in_cents"`
YearlyPriceInCents *int `json:"yearly_price_in_cents"`
PriceModel *string `json:"price_model"`
UnitName *string `json:"unit_name"`
Bullets *[]string `json:"bullets"`
Copy link
Member

Choose a reason for hiding this comment

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

For consistency with other structs that we map to GitHub responses, all of these should have ,omitempty JSON tag option. E.g., see:

type Issue struct {
ID *int `json:"id,omitempty"`
Number *int `json:"number,omitempty"`
State *string `json:"state,omitempty"`
Locked *bool `json:"locked,omitempty"`
Title *string `json:"title,omitempty"`
Body *string `json:"body,omitempty"`
User *User `json:"user,omitempty"`
Labels []Label `json:"labels,omitempty"`
Assignee *User `json:"assignee,omitempty"`
Comments *int `json:"comments,omitempty"`
ClosedAt *time.Time `json:"closed_at,omitempty"`
CreatedAt *time.Time `json:"created_at,omitempty"`
UpdatedAt *time.Time `json:"updated_at,omitempty"`
ClosedBy *User `json:"closed_by,omitempty"`
URL *string `json:"url,omitempty"`
HTMLURL *string `json:"html_url,omitempty"`
Milestone *Milestone `json:"milestone,omitempty"`
PullRequestLinks *PullRequestLinks `json:"pull_request,omitempty"`
Repository *Repository `json:"repository,omitempty"`
Reactions *Reactions `json:"reactions,omitempty"`
Assignees []*User `json:"assignees,omitempty"`
// TextMatches is only populated from search results that request text matches
// See: search.go and https://developer.github.com/v3/search/#text-match-metadata
TextMatches []TextMatch `json:"text_matches,omitempty"`
}

It doesn't make a difference for unmarshaling JSON, but it does for marshaling. I don't think these will be marshaled (since we're receiving data from GitHub API, not sending these upstream). So, the only reason to do it is to maintain consistency.

See issue #537 for some more background information on this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was wondering why they were there. I didn't realise that they had no impact on unmarshalling (but it makes perfect sense). Happy to maintain the consistency.

}

// MarketplacePurchase represents a GitHub Apps Marketplace Purchase.
type MarketplacePurchase struct {
BillingCycle *string `json:"billing_cycle"`
NextBillingDate *string `json:"next_billing_date"`
UnitCount *int `json:"unit_count"`
Plan *MarketplacePlan `json:"plan"`
}

// MarketplacePlanAccount represents a GitHub Account (user or organization) on a specific plan
Copy link
Member

Choose a reason for hiding this comment

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

In general, I think we should strive to have all sentences in Go documentation to have no spelling or grammar mistakes, and to include periods at ends of sentences. So, I'll point out that you're missing a period at the end of this sentence.

type MarketplacePlanAccount struct {
URL *string `json:"url"`
AccountType *string `json:"type"`
ID *int `json:"id"`
Login *string `json:"login"`
Email *string `json:"email"`
OrganizationBillingEmail *string `json:"organization_billing_email"`
MarketplacePurchase *MarketplacePurchase `json:"marketplace_purchase"`
}

// ListPlans lists all plans for your Marketplace listing
Copy link
Member

Choose a reason for hiding this comment

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

Also missing a period at end of this sentence.

//
// GitHub API docs: https://developer.github.com/v3/apps/marketplace/#list-all-plans-for-your-marketplace-listing
func (s *MarketplaceService) ListPlans(ctx context.Context, stubbed bool, opt *ListOptions) ([]*MarketplacePlan, *Response, error) {
u, err := addOptions(fmt.Sprintf("%v/plans", marketplaceURI(stubbed)), 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", mediaTypeMarketplacePreview)

var i []*MarketplacePlan
Copy link
Collaborator

Choose a reason for hiding this comment

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

s/i/plans/
i usually connotes an index and since this variable is significant and not just any ordinary temp value, I think giving it a meaningful name here improves readability.

resp, err := s.client.Do(ctx, req, &i)
if err != nil {
return nil, resp, err
}

return i, resp, nil
}

// ListPlanAccounts lists all GitHub accounts (user or organization) on a specific plan
Copy link
Member

Choose a reason for hiding this comment

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

And this one.

//
// GitHub API docs: https://developer.github.com/v3/apps/marketplace/#list-all-github-accounts-user-or-organization-on-a-specific-plan
func (s *MarketplaceService) ListPlanAccounts(ctx context.Context, planID int, stubbed bool, opt *ListOptions) ([]*MarketplacePlanAccount, *Response, error) {
u, err := addOptions(fmt.Sprintf("%v/plans/%v/accounts", marketplaceURI(stubbed), planID), 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", mediaTypeMarketplacePreview)

var i []*MarketplacePlanAccount
Copy link
Collaborator

Choose a reason for hiding this comment

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

s/i/accounts/

resp, err := s.client.Do(ctx, req, &i)
if err != nil {
return nil, resp, err
}

return i, resp, nil
}

func marketplaceURI(stubbed bool) string {
if stubbed {
return "apps/marketplace_listing/stubbed"
}
return "apps/marketplace_listing"
}
84 changes: 84 additions & 0 deletions github/apps_marketplace_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
// Copyright 2013 The go-github AUTHORS. All rights reserved.
Copy link
Collaborator

Choose a reason for hiding this comment

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

s/2013/2017/

//
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

package github

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

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

mux.HandleFunc("/apps/marketplace_listing/plans", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, "GET")
testHeader(t, r, "Accept", mediaTypeMarketplacePreview)
testFormValues(t, r, values{
"page": "1",
"per_page": "2",
})
fmt.Fprint(w, `[{"id":1}]`)
})

opt := &ListOptions{Page: 1, PerPage: 2}
plans, _, err := client.Marketplace.ListPlans(context.Background(), false, opt)
if err != nil {
t.Errorf("Marketplace.ListPlans returned error: %v", err)
}

want := []*MarketplacePlan{{ID: Int(1)}}
if !reflect.DeepEqual(plans, want) {
t.Errorf("Marketplace.ListPlans returned %+v, want %+v", plans, want)
}
}

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

mux.HandleFunc("/apps/marketplace_listing/stubbed/plans", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, "GET")
testHeader(t, r, "Accept", mediaTypeMarketplacePreview)
fmt.Fprint(w, `[{"id":1}]`)
})

opt := &ListOptions{Page: 1, PerPage: 2}
plans, _, err := client.Marketplace.ListPlans(context.Background(), true, opt)
if err != nil {
t.Errorf("Marketplace.ListPlans returned error: %v", err)
}

want := []*MarketplacePlan{{ID: Int(1)}}
if !reflect.DeepEqual(plans, want) {
t.Errorf("Marketplace.ListPlans returned %+v, want %+v", plans, want)
}
}

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

mux.HandleFunc("/apps/marketplace_listing/plans/1/accounts", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, "GET")
testHeader(t, r, "Accept", mediaTypeMarketplacePreview)
fmt.Fprint(w, `[{"id":1}]`)
})

opt := &ListOptions{Page: 1, PerPage: 2}
accounts, _, err := client.Marketplace.ListPlanAccounts(context.Background(), 1, false, opt)
if err != nil {
t.Errorf("Marketplace.ListPlanAccounts returned error: %v", err)
}

want := []*MarketplacePlanAccount{{ID: Int(1)}}
if !reflect.DeepEqual(accounts, want) {
t.Errorf("Marketplace.ListPlanAccounts returned %+v, want %+v", accounts, want)
}
}
5 changes: 5 additions & 0 deletions github/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,9 @@ const (

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

// https://developer.github.com/v3/apps/marketplace/
mediaTypeMarketplacePreview = "application/vnd.github.valkyrie-preview+json"
)

// A Client manages communication with the GitHub API.
Expand Down Expand Up @@ -143,6 +146,7 @@ type Client struct {
Licenses *LicensesService
Migrations *MigrationService
Reactions *ReactionsService
Marketplace *MarketplaceService
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's also move this one two lines up to keep this list sorted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Have no problem moving it up, but note that there are items higher in the list which are not sorted.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Would you mind sorting them while you are here, please?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

}

type service struct {
Expand Down Expand Up @@ -224,6 +228,7 @@ func NewClient(httpClient *http.Client) *Client {
c.Gitignores = (*GitignoresService)(&c.common)
c.Issues = (*IssuesService)(&c.common)
c.Licenses = (*LicensesService)(&c.common)
c.Marketplace = (*MarketplaceService)(&c.common)
c.Migrations = (*MigrationService)(&c.common)
c.Organizations = (*OrganizationsService)(&c.common)
c.Projects = (*ProjectsService)(&c.common)
Expand Down