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

Make iDEAL name parameter optional, and accept empty string as nil. #940

Merged
merged 1 commit into from
May 21, 2018

Conversation

danj-stripe
Copy link
Contributor

Summary

I considered removing the name: parameter completely, but this maintains backwards
compatibility, by just marking it as nullable/optional.

Also handling the empty string the same as nil for these optional fields, so that there
isn't a surprise waiting. IMO, an optional field that contains the empty string shouldn't
cause the request to fail.

Motivation

When iDEAL support was originally implemented in the iOS SDK, this was a required
parameter. It isn't any more.

IOS-739

Testing

Added a couple of functional tests that exercise this case.

When iDEAL support was originally implemented in the iOS SDK, this was a required
parameter. It isn't any more.

I considered removing the `name:` parameter completely, but this maintains backwards
compatibility.

Also handling the empty string the same as `nil` for these optional fields, so that there
isn't a surprise waiting. IMO, an optional field that contains the empty string shouldn't
cause the request to fail.
@joeydong-stripe
Copy link
Contributor

Maybe we need to add a changelog entry saying something like: "Creating iDEAL sources no longer requires name". I dont think the detail about mapping empty strings to nil is useful to mention.

@danj-stripe danj-stripe merged commit 08f6a79 into master May 21, 2018
@danj-stripe danj-stripe deleted the danj/bugfix/ideal-optional-name branch May 21, 2018 17:47
mludowise-stripe pushed a commit that referenced this pull request Apr 2, 2022
* Add Link Notice component

* Cleanup
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.

3 participants