added handling of parameters of stripev3.load#35
added handling of parameters of stripev3.load#35caltabid wants to merge 2 commits intocode-corps:developfrom
Conversation
|
@caltabid thanks for the PR! Are there any issues with the recent merges here? Would you mind rebasing and checking? Thank you! |
|
@caltabid I see you've back merged develop into your branch. Think I therefore need to set the merge target as |
|
@lindyhopchris for my project I need both SCA modification introduced in develop and the possibility to handle options parameters in stripe.load introduced in my branch. |
|
@caltabid totally understand. The I'll change the target branch for this PR to I'll take a look at #65 now. |
| .DS_Store | ||
| yarn-error.log | ||
|
|
||
| #Eclipse |
There was a problem hiding this comment.
This change isn't necessary. If it's the editor you use, then it should be in your global gitignore not here.
|
|
||
| setEventListeners() { | ||
| let stripeElement = get(this, 'stripeElement'); | ||
| stripeElement.on('ready', (event) => this.sendAction('ready', stripeElement, event)); |
There was a problem hiding this comment.
The changes to this file need to be removed because they are unrelated to what this PR is trying to do - i.e. add options to the init method on the service.
|
|
||
| unload() { | ||
| this.set('didConfigure', false); | ||
| this.set('didLoad', false); |
There was a problem hiding this comment.
In configure, the methods from the Stripe object are copied across to this. Totally agree with adding an unload method, in case you need to swap Stripe Connect accounts. However, I think unload should set all the methods that were copied across to null so that they are cleared out.
| this.set('didLoad', false); | ||
| }, | ||
|
|
||
| load(publishableKey = null, params = null) { |
There was a problem hiding this comment.
can we set the params on this so that they are available in tests to assert that the correct params were passed through? Unlike publishableKey, I don't think they need to be wrapped in an if before setting, because no params are taken from default configuration.
|
@caltabid generally approach looks good and it'll be great to have these changes - so thanks for the PR. Have added some comments. Now that it's against the |
4 participants
I added this feature to support direct payment on connected accounts.