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.

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

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

Merged
merged 14 commits into from
Nov 14, 2017
Merged

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
9901531
#720 Initial commit for listing Plans and Plan Accounts
lackerman Sep 28, 2017
9157290
Added the fullstops and omitempty to maintain consistency
lackerman Sep 28, 2017
2fb4833
Added remaining functionality along with basic tests
lackerman Oct 1, 2017
6ac3c7e
Run go generate to gen accessor methods for Marketplace* struct fields
lackerman Oct 2, 2017
6c4b54e
Added Stubbed tests and switched MarketplaceService to use struct ins…
lackerman Oct 22, 2017
bb51e74
Removed unused field
lackerman Oct 24, 2017
f9251be
Merge branch 'master' into apps_marketplace
lackerman Oct 31, 2017
354464d
Made the chanegs suggested by @gmlewis
lackerman Oct 31, 2017
46617e6
Incorrectly named variable.
lackerman Oct 31, 2017
c4466d1
Ordered the list of services in github.go
lackerman Oct 31, 2017
f2b216a
Renaming and ordering
lackerman Oct 31, 2017
ecac90f
Re-added the line link to documentation
lackerman Nov 12, 2017
24610dd
Merge branch 'master' of https://github.com/google/go-github into app…
lackerman Nov 12, 2017
b8f7249
Re-generated the accessors
lackerman Nov 12, 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
46 changes: 33 additions & 13 deletions github/apps_marketplace.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,18 @@ import (
// methods of the GitHub API.
//
// GitHub API docs: https://developer.github.com/v3/apps/marketplace/
type MarketplaceService service
type MarketplaceService struct {
client *Client
// Stubbed controls whether endpoints that return stubbed data are used
// instead of production endpoints. Stubbed data is fake data that's useful
// for testing your GitHub Apps. Stubbed data is hard-coded and will not
// change based on actual subscriptions.
//
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: lines 23-24 are unnecessary due to line 16 above. Please remove them.

Copy link
Member

Choose a reason for hiding this comment

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

@gmlewis See https://github.com/google/go-github/pull/732/files#r148168072 for why I think it's still helpful to include them. It just so happens the URL matches the one for the service, I wish we could link to a more specific section, but I don't see one...

// GitHub API docs: https://developer.github.com/v3/apps/marketplace/
Stubbed bool
Copy link
Member

@dmitshur dmitshur Nov 1, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

This Stubbed field is quite unusual and may cause people to seek more information about it. I think it's worth it to include a link to GitHub API documentation for it as well here, as I suggested in #732 (comment):

 // Stubbed controls whether endpoints that return stubbed data are used
 // instead of production endpoints. Stubbed data is fake data that's useful
 // for testing your GitHub Apps. Stubbed data is hard-coded and will not
 // change based on actual subscriptions.
+//
+// GitHub API docs: https://developer.github.com/v3/apps/marketplace/
 Stubbed bool

This is also a way of us hinting that it's not our own invention specific to go-github Go library, but a GitHub API feature.

Copy link
Member

@dmitshur dmitshur Nov 12, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

@gmlewis Does that sound good to you?

If so, I can apply it and merge. Edit: @lackerman has already pushed ecac90f.


ListOptions
Copy link
Member

@dmitshur dmitshur Oct 24, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

ListOptions probably doesn't belong here in a service struct, does it? I'm guessing we forgot to remove it from back when we considered a MarketplaceOptions wrapper, yeah?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yurp, pushed a fix.

}

// MarketplacePlan represents a GitHub Apps Marketplace Listing Plan.
type MarketplacePlan struct {
Expand Down Expand Up @@ -53,8 +64,9 @@ type MarketplacePlanAccount struct {
// ListPlans lists all plans for your Marketplace listing.
//
// 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)
func (s *MarketplaceService) ListPlans(ctx context.Context, opt *ListOptions) ([]*MarketplacePlan, *Response, error) {
uri := s.marketplaceURI("plans")
u, err := addOptions(uri, opt)
if err != nil {
return nil, nil, err
}
Expand All @@ -79,8 +91,9 @@ func (s *MarketplaceService) ListPlans(ctx context.Context, stubbed bool, opt *L
// ListPlanAccountsForPlan lists all GitHub accounts (user or organization) on a specific plan.
//
// GitHub API docs: https://developer.github.com/v3/apps/marketplace/#list-all-github-accounts-user-or-organization-on-a-specific-plan
func (s *MarketplaceService) ListPlanAccountsForPlan(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)
func (s *MarketplaceService) ListPlanAccountsForPlan(ctx context.Context, planID int, opt *ListOptions) ([]*MarketplacePlanAccount, *Response, error) {
uri := s.marketplaceURI(fmt.Sprintf("plans/%v/accounts", planID))
u, err := addOptions(uri, opt)
if err != nil {
return nil, nil, err
}
Expand All @@ -105,8 +118,9 @@ func (s *MarketplaceService) ListPlanAccountsForPlan(ctx context.Context, planID
// ListPlanAccountsForAccount lists all GitHub accounts (user or organization) associated with an account.
//
// GitHub API docs: https://developer.github.com/v3/apps/marketplace/#check-if-a-github-account-is-associated-with-any-marketplace-listing
func (s *MarketplaceService) ListPlanAccountsForAccount(ctx context.Context, accountID int, stubbed bool, opt *ListOptions) ([]*MarketplacePlanAccount, *Response, error) {
u, err := addOptions(fmt.Sprintf("%v/accounts/%v", marketplaceURI(stubbed), accountID), opt)
func (s *MarketplaceService) ListPlanAccountsForAccount(ctx context.Context, accountID int, opt *ListOptions) ([]*MarketplacePlanAccount, *Response, error) {
uri := s.marketplaceURI(fmt.Sprintf("accounts/%v", accountID))
u, err := addOptions(uri, opt)
if err != nil {
return nil, nil, err
}
Expand All @@ -131,8 +145,13 @@ func (s *MarketplaceService) ListPlanAccountsForAccount(ctx context.Context, acc
// ListMarketplacePurchasesForUser lists all GitHub marketplace purchases made by a user.
//
// GitHub API docs: https://developer.github.com/v3/apps/marketplace/#get-a-users-marketplace-purchases
func (s *MarketplaceService) ListMarketplacePurchasesForUser(ctx context.Context, stubbed bool, opt *ListOptions) ([]*MarketplacePurchase, *Response, error) {
u, err := addOptions("apps/user/marketplace_purchases", opt)
func (s *MarketplaceService) ListMarketplacePurchasesForUser(ctx context.Context, opt *ListOptions) ([]*MarketplacePurchase, *Response, error) {
uri := "user/marketplace_purchases"
if s.Stubbed {
uri = "user/marketplace_purchases/stubbed"
}

u, err := addOptions(uri, opt)
if err != nil {
return nil, nil, err
}
Expand All @@ -154,9 +173,10 @@ func (s *MarketplaceService) ListMarketplacePurchasesForUser(ctx context.Context
return i, resp, nil
}

func marketplaceURI(stubbed bool) string {
if stubbed {
return "apps/marketplace_listing/stubbed"
func (s *MarketplaceService) marketplaceURI(endpoint string) string {
url := "marketplace_listing"
if s.Stubbed {
url = "marketplace_listing/stubbed"
}
return "apps/marketplace_listing"
return url + "/" + endpoint
}
100 changes: 87 additions & 13 deletions github/apps_marketplace_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ func TestMarketplaceService_ListPlans(t *testing.T) {
setup()
defer teardown()

mux.HandleFunc("/apps/marketplace_listing/plans", func(w http.ResponseWriter, r *http.Request) {
mux.HandleFunc("/marketplace_listing/plans", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, "GET")
testHeader(t, r, "Accept", mediaTypeMarketplacePreview)
testFormValues(t, r, values{
Expand All @@ -28,7 +28,8 @@ func TestMarketplaceService_ListPlans(t *testing.T) {
})

opt := &ListOptions{Page: 1, PerPage: 2}
plans, _, err := client.Marketplace.ListPlans(context.Background(), false, opt)
client.Marketplace.Stubbed = false
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmmm... I wish we didn't use a package global client for unit testing, as this will prevent unit tests from all being run in parallel in the future.

Ideally, we would have something like:

client, teardown := setup()
defer teardown()

for each test. No action is needed in this PR, but I'll file a new issue and point here for motivation.

plans, _, err := client.Marketplace.ListPlans(context.Background(), opt)
if err != nil {
t.Errorf("Marketplace.ListPlans returned error: %v", err)
}
Expand All @@ -39,40 +40,42 @@ func TestMarketplaceService_ListPlans(t *testing.T) {
}
}

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

mux.HandleFunc("/apps/marketplace_listing/stubbed/plans", func(w http.ResponseWriter, r *http.Request) {
mux.HandleFunc("/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)
client.Marketplace.Stubbed = true
plans, _, err := client.Marketplace.ListPlans(context.Background(), opt)
if err != nil {
t.Errorf("Marketplace.ListPlans returned error: %v", err)
t.Errorf("Marketplace.ListPlans (Stubbed) returned error: %v", err)
}

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

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

mux.HandleFunc("/apps/marketplace_listing/plans/1/accounts", func(w http.ResponseWriter, r *http.Request) {
mux.HandleFunc("/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.ListPlanAccountsForPlan(context.Background(), 1, false, opt)
client.Marketplace.Stubbed = false
accounts, _, err := client.Marketplace.ListPlanAccountsForPlan(context.Background(), 1, opt)
if err != nil {
t.Errorf("Marketplace.ListPlanAccountsForPlan returned error: %v", err)
}
Expand All @@ -83,18 +86,42 @@ func TestMarketplaceService_ListPlanAccountsForPlan(t *testing.T) {
}
}

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

mux.HandleFunc("/marketplace_listing/stubbed/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}
client.Marketplace.Stubbed = true
accounts, _, err := client.Marketplace.ListPlanAccountsForPlan(context.Background(), 1, opt)
if err != nil {
t.Errorf("Marketplace.ListPlanAccountsForPlan (Stubbed) returned error: %v", err)
}

want := []*MarketplacePlanAccount{{ID: Int(1)}}
if !reflect.DeepEqual(accounts, want) {
t.Errorf("Marketplace.ListPlanAccountsForPlan (Stubbed) returned %+v, want %+v", accounts, want)
}
}

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

mux.HandleFunc("/apps/marketplace_listing/accounts/1", func(w http.ResponseWriter, r *http.Request) {
mux.HandleFunc("/marketplace_listing/accounts/1", 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.ListPlanAccountsForAccount(context.Background(), 1, false, opt)
client.Marketplace.Stubbed = false
accounts, _, err := client.Marketplace.ListPlanAccountsForAccount(context.Background(), 1, opt)
if err != nil {
t.Errorf("Marketplace.ListPlanAccountsForAccount returned error: %v", err)
}
Expand All @@ -105,18 +132,65 @@ func TestMarketplaceService_ListPlanAccountsForAccount(t *testing.T) {
}
}

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

mux.HandleFunc("/marketplace_listing/stubbed/accounts/1", 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}
client.Marketplace.Stubbed = true
accounts, _, err := client.Marketplace.ListPlanAccountsForAccount(context.Background(), 1, opt)
if err != nil {
t.Errorf("Marketplace.ListPlanAccountsForAccount (Stubbed) returned error: %v", err)
}

want := []*MarketplacePlanAccount{{ID: Int(1)}}
if !reflect.DeepEqual(accounts, want) {
t.Errorf("Marketplace.ListPlanAccountsForAccount (Stubbed) returned %+v, want %+v", accounts, want)
}
}

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

mux.HandleFunc("/apps/user/marketplace_purchases", func(w http.ResponseWriter, r *http.Request) {
mux.HandleFunc("/user/marketplace_purchases", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, "GET")
testHeader(t, r, "Accept", mediaTypeMarketplacePreview)
fmt.Fprint(w, `[{"billing_cycle":"monthly"}]`)
})

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

want := []*MarketplacePurchase{{BillingCycle: String("monthly")}}
if !reflect.DeepEqual(purchases, want) {
t.Errorf("Marketplace.ListMarketplacePurchasesForUser returned %+v, want %+v", purchases, want)
}
}

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

mux.HandleFunc("/user/marketplace_purchases/stubbed", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, "GET")
testHeader(t, r, "Accept", mediaTypeMarketplacePreview)
fmt.Fprint(w, `[{"billing_cycle":"monthly"}]`)
})

opt := &ListOptions{Page: 1, PerPage: 2}
purchases, _, err := client.Marketplace.ListMarketplacePurchasesForUser(context.Background(), false, opt)
client.Marketplace.Stubbed = true
purchases, _, err := client.Marketplace.ListMarketplacePurchasesForUser(context.Background(), opt)
if err != nil {
t.Errorf("Marketplace.ListMarketplacePurchasesForUser returned error: %v", err)
}
Expand Down
2 changes: 1 addition & 1 deletion github/github.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +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.Marketplace = &MarketplaceService{client: c}
c.Migrations = (*MigrationService)(&c.common)
c.Organizations = (*OrganizationsService)(&c.common)
c.Projects = (*ProjectsService)(&c.common)
Expand Down