Problem/Motivation

There are multiple use cases for payments where 3ds is not suitable and the Stripe review pane is not relevent:
Recurring: #3171408: Allow off_session payment intents for recurring payments
Staff: #3222298: Allow administrators to add payment methods & take payments

Proposed resolution

In the gateway's createPayment() method, create a payment intent if one is not already present on the order.

We should assume it is an off_session payment intent, as if a customer is present the Stripe review pane should be used in order to allow for 3ds.

User interface changes

None.

API changes

None.

Data model changes

None.

Issue fork commerce_stripe-3259349

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

jonathanshaw created an issue. See original summary.

jonathanshaw opened merge request !11

jonathanshaw’s picture

jonathanshaw
Primary language English
Location Stroud, UK
commented
Status: Active » Postponed
Related issues: +#3259211: Provide more parameters to createPaymentIntent()

This is blocked on #3259211: Provide more parameters to createPaymentIntent()

jonathanshaw’s picture

jonathanshaw
Primary language English
Location Stroud, UK
commented
Title: Allow for non-checkout payments & intents » Allow for payments without stripe review pane
jonathanshaw’s picture

jonathanshaw
Primary language English
Location Stroud, UK
commented

The remaining test fails should be because createPaymentIntent() is not using the intent attributes that are being passed to it. This is fixed by the blocking issue #3259211: Provide more parameters to createPaymentIntent().

johnpitcairn’s picture

johnpitcairn commented

Running this as part of a combo patch in #3171408: Allow off_session payment intents for recurring payments produces the same test fails as #3259211: Provide more parameters to createPaymentIntent(), confirming the test failures here will be fixed by that blocking issue.

johnpitcairn’s picture

johnpitcairn commented
Related issues: +#3171408: Allow off_session payment intents for recurring payments
johnpitcairn’s picture

johnpitcairn commented
Status: Postponed » Needs work

#3259211: Provide more parameters to createPaymentIntent() is committed so we can move forward with this.

johnpitcairn’s picture

johnpitcairn commented
Status: Needs work » Needs review
johnpitcairn’s picture

johnpitcairn commented
Status: Needs review » Needs work

This will need re-testing against current dev and 9.x. Not sure how to do that.

rivimey’s picture

rivimey
she/her
commented
Status: Needs work » Needs review

John, the automated testing setup of this project needs to be updated, by a maintainer such as @jsacksick, to update the test targets to D9 / D10 and recent DB versions.

However, while I agree the testing should be done, I feel that putting the status back to needs work is not helpful as we don't actually know of any work that needs doing, and maintainers won't necessarily check on issues in needs work state.

jonathanshaw’s picture

jonathanshaw
Primary language English
Location Stroud, UK
commented
Status: Needs review » Needs work
jonathanshaw’s picture

jonathanshaw
Primary language English
Location Stroud, UK
commented

I've become doubtful about something in the tests; I'm debugging a bit with the last commit.

jonathanshaw’s picture

jonathanshaw
Primary language English
Location Stroud, UK
commented
Status: Needs work » Needs review

rtbc I think

jsacksick’s picture

jsacksick commented

@rivimey: Tests are running on Drupal 9.5, not sure what the problem is?

rivimey’s picture

rivimey
she/her
commented

@rivimey: Tests are running on Drupal 9.5, not sure what the problem is?

At the time of the comment all the tests had been against D8.9 and using php7.1, both of which were eol at the time.

Is there a MR version of the 'add test/retest' option, ideally that enables different combinations to be tested. I ask because it's unsafe to only test on php7 -- php8.1 is needed too as there are a lot of bugs being found with php8's stricter checks.

jonathanshaw’s picture

jonathanshaw
Primary language English
Location Stroud, UK
commented

Is there a MR version of the 'add test/retest' option

Yes. I see it now on this page under the test results, although previously I didn't.

rivimey’s picture

rivimey
she/her
commented

Don't know if it helps as a non-Commerce dev, but I've looked over the code and cannot see any issues. Looks good to me!

RTBC +1

johnpitcairn’s picture

johnpitcairn commented
Status: Needs review » Reviewed & tested by the community

Working as expected here and looks good, let's see what @jsacksick thinks.

jonathanshaw’s picture

jonathanshaw
Primary language English
Location Stroud, UK
commented

Tests are still green, still RTBC. The D10 test failure is unrelated to any change in this MR.

jsacksick’s picture

jsacksick commented

hm... But does this still aply after #3185801: Stripe Charge to Stripe PaymentIntent got in?

jonathanshaw’s picture

jonathanshaw
Primary language English
Location Stroud, UK
commented

I thought it wouldn't do, but I reran the tests and they're still green:
https://www.drupal.org/pift-ci-job/2576783

jonathanshaw’s picture

jonathanshaw
Primary language English
Location Stroud, UK
commented
Issue tags: +Needs reroll

Ah, my bad: the MR needs a rebase, it doesn't automatically run against latest master.

jonathanshaw’s picture

jonathanshaw
Primary language English
Location Stroud, UK
commented

No significant conflict with #3185801: Stripe Charge to Stripe PaymentIntent, Gitlab handled automatic rebase fine.

jsacksick’s picture

jsacksick commented
Status: Reviewed & tested by the community » Fixed

Committed, thank you!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.