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

New custom_method DSL for defining custom API request methods as static methods #754

Merged
merged 2 commits into from
Apr 2, 2019

Conversation

ob-stripe
Copy link
Contributor

r? @brandur-stripe @remi-stripe

This PR adds a new custom_method DSL that can be used in API resources to define a class method for custom API request methods. custom_method takes 3 parameters:

  • the name of the method
  • the HTTP verb
  • the path to append to the URL. This is optional, if not provided, the name of the method is used as the path

For instance, on Stripe::Charge:

custom_method :capture, :post

is equivalent to:

def self.capture(id, params = {}, opts = {})
  url = "#{resource_url}/#{CGI.escape(id)}/capture"
  resp, opts = request(http_method, url, params, opts)
  Util.convert_to_stripe_object(resp.data, opts)
end

This should make it easier to transition to static methods and deprecate instance methods (cf. #752).

I've added custom_method everywhere where it should apply (though I may have missed some), and added tests for some (not all) of the new methods.

I've also added a Stripe::Discount class for discount objects.

Copy link
Contributor

@brandur-stripe brandur-stripe left a comment

Choose a reason for hiding this comment

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

Nice OB!

Left a couple comments, but looks great.

ptal @ob-stripe

lib/stripe/api_resource.rb Outdated Show resolved Hide resolved
lib/stripe/api_resource.rb Outdated Show resolved Hide resolved
lib/stripe/api_resource.rb Outdated Show resolved Hide resolved
@ob-stripe ob-stripe force-pushed the ob-dsl-custom-method branch from 2de8786 to 9883454 Compare March 29, 2019 17:20
@ob-stripe
Copy link
Contributor Author

Made the requested changes, ptal @brandur-stripe.

Copy link
Contributor

@brandur-stripe brandur-stripe left a comment

Choose a reason for hiding this comment

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

Awesome OB! Looks great to me.

lib/stripe/api_resource.rb Show resolved Hide resolved
@ob-stripe
Copy link
Contributor Author

@remi-stripe Want to take over the branch to check that I haven't missed any custom methods and add the tests that are missing?

@ob-stripe ob-stripe assigned remi-stripe and unassigned ob-stripe Mar 29, 2019
.rubocop.yml Show resolved Hide resolved
lib/stripe/api_resource.rb Show resolved Hide resolved
lib/stripe/account.rb Show resolved Hide resolved
lib/stripe/api_resource.rb Show resolved Hide resolved
test/stripe/charge_test.rb Show resolved Hide resolved
lib/stripe/customer.rb Show resolved Hide resolved
lib/stripe/subscription_item.rb Outdated Show resolved Hide resolved
lib/stripe/subscription_schedule.rb Outdated Show resolved Hide resolved
lib/stripe/transfer.rb Show resolved Hide resolved
test/stripe/subscription_schedule_test.rb Outdated Show resolved Hide resolved
@stripe-ci stripe-ci assigned ob-stripe and unassigned remi-stripe Mar 30, 2019
@remi-stripe
Copy link
Contributor

@ob-stripe Also pushed changes to the tests.

@remi-stripe
Copy link
Contributor

@ob-stripe I just realized we forgot to do all the namespaced resources like terminal and such too

Copy link
Contributor

@remi-stripe remi-stripe left a comment

Choose a reason for hiding this comment

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

re-assigning to ob to finish reviewing and release

.rubocop.yml Show resolved Hide resolved
lib/stripe/account.rb Show resolved Hide resolved
lib/stripe/api_resource.rb Show resolved Hide resolved
lib/stripe/subscription_item.rb Outdated Show resolved Hide resolved
lib/stripe/subscription_schedule.rb Outdated Show resolved Hide resolved
.rubocop.yml Show resolved Hide resolved
test/stripe/issuing/authorization_test.rb Outdated Show resolved Hide resolved
test/stripe/issuing/authorization_test.rb Show resolved Hide resolved
@remi-stripe remi-stripe force-pushed the ob-dsl-custom-method branch from 6135b24 to f063abb Compare April 2, 2019 03:05
@remi-stripe
Copy link
Contributor

@ob-stripe turns out I only messed up 2 of them!!!! Re-assigning to you again :p

Copy link
Contributor Author

@ob-stripe ob-stripe left a comment

Choose a reason for hiding this comment

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

LGTM!

@remi-stripe remi-stripe changed the title [WIP] New custom_method DSL for defining custom API request methods as static methods New custom_method DSL for defining custom API request methods as static methods Apr 2, 2019
@ob-stripe ob-stripe merged commit 160028a into master Apr 2, 2019
@ob-stripe ob-stripe deleted the ob-dsl-custom-method branch April 2, 2019 17:06
@ob-stripe
Copy link
Contributor Author

Released as 4.12.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants