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

Init options with forced parameter values #1699

Merged
merged 1 commit into from
Jul 18, 2019
Merged

Conversation

ob-stripe
Copy link
Contributor

r? @remi-stripe @brandur-stripe

Some options classes have forced parameter values, e.g. CardListOptions always includes object=card. However if the user passes null then the parameter is not sent.

This is kind of an ugly ad-hoc fix. Ideally we would always create an instance of the appropriate options class, but doing this would require us to make all the helper methods in Service generic to accept the options class as a type param, and I'd rather not do that just yet.

Fixes #1676.

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.

While I'm okay with the change, I think this is a breaking change. While the current behaviour is a bug, changing it would change the number of results returned in List endpoints.

Also we really should consider adding something that can fetch a list of IPaymentSource like we have (or used to?) in java. Not being able to list all payment sources (I can't say payment methods though I want to) is really confusing.

@brandur-stripe
Copy link
Contributor

+1 Remi's comments, but LGTM otherwise. (Thanks for the quick fix!)

@ob-stripe ob-stripe changed the base branch from master to integration-v28 July 10, 2019 01:18
@ob-stripe ob-stripe merged commit e8b894d into integration-v28 Jul 18, 2019
@ob-stripe ob-stripe deleted the ob-init-options branch July 18, 2019 17:33
@ob-stripe ob-stripe mentioned this pull request Jul 18, 2019
19 tasks
ob-stripe added a commit that referenced this pull request Oct 9, 2019
* Better string enums (#1675)

* Init options with default parameter values (#1699)

* Remove multiple deprecated features (#1713)

Many features were marked as obsolete and are now being removed:
* AccountBalance on Customer, use Balance instead
* Billing on Subscription/Invoice/SubscriptionSchedule, use CollectionMethod instead
* TaxInfo and TaxInfoVerification on Customer, use TaxId instead
* AllowedSourceTypes on PaymentIntent, use PaymentMethodTypes instead
* Start on Subscription, use StartDate instead
* ApplicationFee on Charge, use ApplicationFeeAmount instead
* Date when listing Invoices, use Created instead
* OperatorAccount on all Terminal APIs

* Remove all Expand* fields (#1715)

* Use properl SetupIntent class

* Removed Id and FileId suffixes to stay as close to the API as possible. (#1738)

* Add a test to ensure that JSON names match property names (#1744)

* A few more renames (#1739)

* Remove dead code for service expansions (#1751)

* Bump Stylecop.Analyzers to latest version (#1752)

* Bump SourceLink and create symbol package (#1755)

* Fix all classes not inheriting from the generic version of StripeEntity

* Add a wholesome test to ensure proper JSON converters are applied (#1761)

* Remove more deprecated stuff (#1802)

* Remove more Id/FileId suffixes (#1803)

* One more rename (#1805)

* Fix ExternalAccountUpdateOptions (#1806)

* Rename AccountOpener to Representative and move to latest API version

* Enable test for JSON<->property name consistency (#1804)
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.

None yet

4 participants