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

[11.x] WIP: SEPA Debit support #893

Closed
wants to merge 1 commit into from
Closed

Conversation

lsmith77
Copy link

@lsmith77 lsmith77 commented Mar 21, 2020

see #435

Note when setting up the intent it is necessary to specifiy "seba_debit", f.e. if you want to allow both card and sepa_debit:

        $options = [
            'payment_method_types' => ['card', 'sepa_debit']
        ];

        $setupIntent = SetupIntent::create($options, Cashier::stripeOptions());

I managed to do a charge in testmode inside my Spark app (needed to do some modifications of course inside the Spark app too). Also noticed that there seems to be a 10k EUR limit on my account and I wanted to charge 10.8k EUR .. doh .. but with a lower amount it worked.

src/Billable.php Outdated
$this->card_last_four = $paymentMethod->card->last4;
break;
case 'sepa_debit':
$this->card_brand = 'SEPA Debit';
Copy link
Author

@lsmith77 lsmith77 Mar 21, 2020

Choose a reason for hiding this comment

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

following the example of Bank Account here .. obviously we could also store the bank_code here but I guess in most cases people just want to know if its a SEPA payment method:
https://stripe.com/docs/api/payment_methods/object#payment_method_object-card-generated_from-payment_method_details-sepa_debit-bank_code

@driesvints driesvints changed the title WIP: SEPA Debit support [11.x] WIP: SEPA Debit support Mar 23, 2020
src/Billable.php Outdated

// "type" is temporarily required by Stripe...
$paymentMethods = StripePaymentMethod::all(
['customer' => $this->stripe_id, 'type' => 'card'] + $parameters,
['type' => 'card'] + $parameters,
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't the below work?

Suggested change
['type' => 'card'] + $parameters,
['type' => ['card', 'sepa_debit']] + $parameters,

Copy link
Author

Choose a reason for hiding this comment

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

let me check once more .. IIRC I ran into issues.

Copy link
Author

Choose a reason for hiding this comment

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

unfortunately this gives an Invalid string: {:"0"=>"card", :"1"=>"sepa_debit"}

@driesvints
Copy link
Member

Thanks for the PR. This will still need to be fleshed out a lot more with tests, an update to the payment.blade.php page and some updates to the PaymentMethod class. I might have some time this week to look at this.

@lsmith77
Copy link
Author

I can look into doing those additional fixes sometime this week.

BTW I assume this should go into master .. or could it also go into 10.x ?
Spark is currently locked on cashier 10.x and a specific stripe API version which conflicts with master.

@driesvints
Copy link
Member

Definitely master.

@lsmith77
Copy link
Author

I just added some changes to payment.blade.php .. not tested at all.

I looked at PaymentMethod and did't see what should be added there.
Would you prefer if the SEPA Debit string we defined over there?

@lsmith77
Copy link
Author

not sure how soon I get some more time for this .. tests are obviously also still missing.

@driesvints
Copy link
Member

@lsmith77 think yu still need to push your changes?

@lsmith77
Copy link
Author

oops .. tried to push to origin .. pushed

@lsmith77
Copy link
Author

lsmith77 commented Apr 1, 2020

just rebased on master.

@driesvints
Copy link
Member

@lsmith77 we made the decision to not include SEPA for now. There's still quite a bit of work with this PR and we'll need to test various scenarios regarding SCA, failed payments, delayed notifications. We want to get v11 finally out of the door now and will look again at this PR for v12 maybe. Thanks for your work. Don't consider this lost, I'll re-look at this once I get to it.

@driesvints driesvints closed this Apr 3, 2020
@lsmith77
Copy link
Author

lsmith77 commented Apr 3, 2020

ok. no worries. if I bring this to prod on my side I can do this in a fork. just wondering how do you plan to retain the work for v12? I guess a closed PR will remain available and its linked in the issue that remains open.

@driesvints
Copy link
Member

@lsmith77 yeah the code is still here and the issue is linked 👍

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.

2 participants