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

feat: create token from bank account #591

Merged
merged 7 commits into from
Feb 9, 2022

Conversation

arekkubaczkowski
Copy link
Collaborator

closes #575

@joeyboth
Copy link

joeyboth commented Oct 7, 2021

Please add this :(

@DaveyCornelissen
Copy link

Hey guys,

Can you please add this feature?
I would like to implement it!

Thanks ;)

@marquina-abreu
Copy link

I need you to approve this

@esteban199
Copy link

Please approve

@CesarDav
Copy link

It would be great if this could be approved

accountHolderType = mapToBankAccountType(accountHolderType)
)

runBlocking {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this done on a background thread?

Copy link
Collaborator

Choose a reason for hiding this comment

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

As I just take it over from Arek I'm not sure I get the comment @michelleb-stripe. runBlocking is needed here because without it we need an error Suspend function 'createBankAccountToken' should be called only from a coroutine or another suspend function

Copy link
Contributor

Choose a reason for hiding this comment

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

@souhe When the ReactMethod is called is it being called from a UI thread. The reason the routine is marked as suspend is to help indicate that it should not be called from a UI thread.

@germangutierrezv
Copy link

Please approve

@kjmendoza
Copy link

Please approve the PR is necessary to move forward with a project :(

@fdelacruzsoto
Copy link

@arekkubaczkowski are you planning to update this PR based on the comments you got?

@BogdanVV
Copy link

Any updates?

@CoryWritesCode
Copy link
Collaborator

Any movement on this? 🙏

val type = getValOr(params, "type", null)?.let {
if (it != "Card") {
val type = getValOr(params, "type", null)
type?.let {
Copy link
Contributor

Choose a reason for hiding this comment

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

You can use:
when(type){ "BankAccount" -> {...} null -> {...} }

Copy link
Collaborator

Choose a reason for hiding this comment

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

Good idea. I just refactored this function into a big when expression

bankAccountParams.routingNumber = routingNumber

if let holderType = Mappers.mapToBankAccountHolderType(accountHolderType) {
bankAccountParams.accountHolderType = holderType

Choose a reason for hiding this comment

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

@charliecruzan-stripe .accountHolderType defaults to .individual on iOS. What does it default to on Android? Do we want consistent defaults across for both platforms?

Copy link
Collaborator

Choose a reason for hiding this comment

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

fixed in bf363b8

@michelleb-stripe
Copy link
Contributor

I am just headed out, but I can take a look at this in the morning.

@michelleb-stripe
Copy link
Contributor

Thanks for making those updates. It now look good to me, just need a final approval from @ramont-stripe

Copy link

@ramont-stripe ramont-stripe left a comment

Choose a reason for hiding this comment

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

Thank you for updating. Changes look good 👍

@charliecruzan-stripe charliecruzan-stripe changed the title create token from bank account feat: create token from bank account Feb 9, 2022
@charliecruzan-stripe charliecruzan-stripe merged commit eeeb998 into master Feb 9, 2022
@charliecruzan-stripe charliecruzan-stripe deleted the feat/create-token-bank-account branch February 9, 2022 15:20
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.

Create token BankAccount with params