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

Collection improvements / fixes #715

Merged
merged 1 commit into from
Aug 27, 2019
Merged

Conversation

ob-stripe
Copy link
Contributor

r? @brandur-stripe @remi-stripe
cc @stripe/api-libraries

A bunch of improvements and fixes to the Collection model:

  • renamed requestParams to filters, which conveys the intent better and is consistent with stripe-ruby (this is a breaking change, though I don't think many users use this)
  • override offsetGet (magic method invoked when accessing elements via []) to output a helpful error message when using numeric indices (this is consistent with stripe-ruby)
  • add emptyCollection() static constructor, and nextPage() and previousPage() helper methods. Simplify autoPagingIterator() by using nextPage() (again, this is consistent with stripe-ruby)
  • fix the all/create/retrieve method. Previously they updated the filters on the instance, which was incorrect. Now all correctly sets the filters on the returned object (not the object on which the method is called), and create and retrieve don't set the filters.
    • There's a bit of duplicated code between the trait methods for all/create/retrieve and the methods in Collection, but unfortunately PHP does not have anything like Ruby's include vs. extend to add trait methods as static methods or instance methods.
  • added a bunch of tests for the new stuff

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.

WFM. Thanks for the great comments too!

@ob-stripe ob-stripe merged commit 5c2a2fa into integration-v7 Aug 27, 2019
@ob-stripe ob-stripe deleted the ob-collection-fixes branch August 27, 2019 19:36
@ob-stripe ob-stripe mentioned this pull request Sep 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants