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

adding usage charge resource #221

Merged
merged 2 commits into from Feb 10, 2016
Merged

adding usage charge resource #221

merged 2 commits into from Feb 10, 2016

Conversation

ch33sybr3ad
Copy link
Contributor

@ch33sybr3ad ch33sybr3ad commented Nov 5, 2015

Description

  • adds usage charge resource to shopify api

@Shopify/finance @dominiquesr @Not-Jake

@@ -19,5 +19,9 @@ def cancel
def activate
load_attributes_from_response(post(:activate))
end

def customize(capped_amount)

Choose a reason for hiding this comment

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

Should this be a hash so its customize(capped_amount: 10) ?

@ch33sybr3ad
Copy link
Contributor Author

@Shopify/finance 👀 please

Need to merge, so I can write some api unit tests for usage charges within shopify to help @joshubrown with documentation

class << self
def index(recurring_application_charge_id)
UsageCharge.find(:all, :params => { :recurring_application_charge_id => recurring_application_charge_id })
rescue ActiveResource::ForbiddenAccess

Choose a reason for hiding this comment

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

why are you catching this - if you let it bubble up doesn't the client get a 403 and wouldn't that be preferred?

Choose a reason for hiding this comment

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

Sorry - this is the client API already - this is the 403 coming back from the server

@ch33sybr3ad
Copy link
Contributor Author


usage_charges = ShopifyAPI::UsageCharge.find(:all, params: {recurring_application_charge_id: 654381177})

assert_equal 2, usage_charges.length

Choose a reason for hiding this comment

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

add a test for RecurringAppCharges with no usage charges

@ch33sybr3ad ch33sybr3ad force-pushed the usage_charge_resource branch 3 times, most recently from d867423 to 933754e Compare February 8, 2016 15:48
@sluggoman
Copy link

👍

1 similar comment
@dominiquesr
Copy link
Contributor

👍

ch33sybr3ad added a commit that referenced this pull request Feb 10, 2016
@ch33sybr3ad ch33sybr3ad merged commit 5f93f6a into master Feb 10, 2016
@ch33sybr3ad ch33sybr3ad deleted the usage_charge_resource branch February 10, 2016 19:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants