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

Send object=bank_account when using AccountExternalAccountParams #486

Merged
merged 3 commits into from
Oct 25, 2017

Conversation

brandur
Copy link
Contributor

@brandur brandur commented Oct 25, 2017

When using AccountExternalAccountParams for creating a bank account
from an account, sending an object field is required. Here we give the
params struct a custom AppendTo so that we can comply with this logic.

Partial fix for #485.

r? @ob-stripe

When using `AccountExternalAccountParams` for creating a bank account
from an account, sending an `object` field is required. Here we give the
params struct a custom `AppendTo` so that we can comply with this logic.

Partial fix for #485.
@ob-stripe
Copy link
Contributor

lgtm in the specific context of #485, but ideally we should also be able to handle the following cases:

  • sending a token in external_account instead of bank account details
  • sending card details instead of bank account details

At the moment I think the library is not capable of either of these. This can be handled in a future PR though.

@remi-stripe
Copy link
Contributor

sending card details instead of bank account details

We explicitly refuse to support this one since it's not PCI compliant and would be blocked for almost all users by default.

@remi-stripe
Copy link
Contributor

Worked with @brandur-stripe on this and pushed a fix for the token part so that you can send it as needed.

@ob-stripe
Copy link
Contributor

Nice! Mind adding a test for the token case?

@remi-stripe
Copy link
Contributor

I looked unfortunately I don't understand the mock logic for this test case. I get a panic that does not make sense to me since the code does work as expected. Will defer to @brandur-stripe

@brandur-stripe
Copy link
Contributor

Good call on the additional functionality @ob-stripe, and thanks for the patch @remi-stripe!

Added an additional test in f7de6b1 and am bringing this one in.

@brandur-stripe brandur-stripe merged commit 76e48b3 into master Oct 25, 2017
@brandur-stripe brandur-stripe deleted the brandur-send-object branch October 25, 2017 19:01
nadaismail-stripe pushed a commit that referenced this pull request Oct 18, 2024
Bumps [sorbet-runtime](https://github.com/sorbet/sorbet) from 0.5.10073 to 0.5.10090.
- [Release notes](https://github.com/sorbet/sorbet/releases)
- [Commits](https://github.com/sorbet/sorbet/commits)

---
updated-dependencies:
- dependency-name: sorbet-runtime
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
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.

4 participants