Add checkout settings for payment method behavior

Picking this up

Here's an example screencast. The summary logic is wrong. But the flow is at least set as an example.

Patch. Not functional. Just config form. But so much word bikeshedding to be done, getting this up now.

It works-ish. needs more testings and bikeshedding.

I had to add logic in PaymentMethodAddForm to support the checkbox. So that the payment method is flagged properly as reusable before passing to the payment gateway. Which forces the form to consider checkout logic. Not sure how we can decouple this, easily, due to elements.

    $form['reusable'] = [
      '#type' => 'value',
      '#value' => $payment_method->isReusable(),
    ];
    if ($form['#allow_reusable']) {
      if ($form['#always_save']) {
        $form['reusable']['#value'] = TRUE;
      }
      else {
        $form['reusable'] = [
          '#type' => 'checkbox',
          '#title' => t('Save this payment method for later use'),
          '#default_value' => FALSE,
        ];
      }
    }

Need to add a pseudo path that could be applied to 8.5.0 that I will be using.

Need to add a pseudo path that could be applied to 8.5.0 that I will be using.

I don't know what this means. And no interdiff was posted, and we lost 1kb of changes.

Here with some bikeshedding: What if we have multiple payment gateways? Can this functionality be enabled/disabled per gateway? From the gif above, it appears not.

Cannot apply patch to the latest dev branch or latest release.

Looks like this needs a reroll. @mglaman, should we be rerolling #7 or #9?

It would be #7

Attempt at rerolling the patch in #7. Note that the patch is smaller in size because of changes that were made in Commerce between #7 and now (e.g., \Drupal\Tests\commerce_payment\FunctionalJavascript\PaymentCheckoutTest::testFreeOrder already exists).

The interdiff command failed with this message:

1 out of 8 hunks FAILED -- saving rejects to file /tmp/interdiff-1.n1mNua.rej
interdiff: Error applying patch1 to reconstructed file

The rejected hunk:

--- interdiff-1.KAtXCn
+++ interdiff-1.KAtXCn
@@ -221,7 +324,7 @@ class PaymentInformation extends CheckoutPaneBase {
 
     $selected_option = $pane_form['payment_method'][$default_option];
     $payment_gateway = $payment_gateways[$selected_option['#payment_gateway']];
-    if ($payment_gateway->getPlugin() instanceof SupportsStoredPaymentMethodsInterface) {
+    if (!$collect_billing_only && $payment_gateway->getPlugin() instanceof SupportsStoredPaymentMethodsInterface) {
       if (!empty($selected_option['#payment_method_type'])) {
         /** @var \Drupal\commerce_payment\PaymentMethodStorageInterface $payment_method_storage */
         $payment_method_storage = $this->entityTypeManager->getStorage('commerce_payment_method');

Not sure exactly why that's failing.

Also, it looks like \Drupal\commerce_payment\Plugin\Commerce\CheckoutPane\PaymentInformation::buildPaneForm has been changed to hide the payment method form by default for a free order, so we should change that to check the configuration (in case you want to collect payment information on free orders).

This moves the check for $collect_billing_only up in \Drupal\commerce_payment\Plugin\Commerce\CheckoutPane\PaymentInformation::buildPaneForm, since \Drupal\commerce_payment\Plugin\Commerce\CheckoutPane\PaymentInformation::collectBillingProfileOnly checks whether the order is 0 as well.

Any news on this? This is a very important function. Thanks for your work.

We need this for a client, so I'm going to take a look at it and see if it works as a proof of concept.

Attached reroll patches against both latest release (2.13) and HEAD (8.x-2.x).

This post may end up getting a bit confusing because I am going to talk about the PaymentMethodAddForm class, however in commerce alone there are 3 classes with this name.

\Drupal\commerce_payment\Form\PaymentMethodAddForm
\Drupal\commerce_payment\PluginForm\PaymentMethodAddForm
\Drupal\commerce_payment_example\PluginForm\Onsite\PaymentMethodAddForm

We can completely ignore the commerce_payment_example one as that is not relevant.

The patches so far have made changes to Form\PaymentMethodAddForm, not PluginForm\PaymentMethodAddForm. As far as I can tell, the only process on the site that uses Form\PaymentMethodAddForm is in routing for the /user/{user}/payment-methods/add path. If like I suspect, there is no other use for the Form\PaymentMethodAddForm, then at present it is not relevant, as the configuration options we have added so far is for a specific checkout flow, and the payment options added are completely independent of checkout flow.

Also there is an inherent assumption in adding a payment method to a user account that the user wants to store the payment method for future use, so we don't need to worry about that use case.

If we are talking about the use case that we want to add the option for a user to decide whether they want to store payment details during checkout (or globally disable storage) then I think we want to make changes to PluginForm\PaymentMethodAddForm, as this is actually used when rendering the Pane form in the checkout flow.

I am going to start working on these changes, please feel free to correct me if I am wrong or missing something..

This is patch for making the PaymentInformation pane form actually work with the reusability settings.

The last patch is not applicable to the version 2.14 of Commerce

Any update on this?

Re-rolled the patch for 2.16 (and it also applies to the 8.x-2.x branch). I didn't change a thing and it seems to work well. Still testing.

Uploading this for myself to cover some history. I forgot this issue was meant to include collecting payment methods on free orders as well -- like free trials.

For modern gateways with a temporary payment token like Stripe and Braintree this should not be a big issue. Chase's Hosted Payment Form (HPF) iframe however serves for both creating a payment method and a payment. The actual use depends on the iframe parameters. So if the customer opts in to store the payment method we want to display the payment method version of HPF in the order_information checkout step and create the payment upon review submission using the chase soap api and the created payment method. If the customer does not want to store the payment method we don't display the HPF in the 'order_information' checkout step. On the other hand we display the payment version of HPF in the 'payment' checkout step.

Fixing some tests.

Rerolling #30.

Re-allowing the non-reusable order payment method which also fixes some tests.

Fixing some tests and changing the new test to Functional.

More (failing) tests, code refactoring, coding standards and documentation.

Have not reviewed, yet. This will require patches in all payment gateways.

* If not opt-in, flag reusable as false.
* All payment gateways need to be updated to respect the set resuable flag.

So Stripe should still save a local payment method entity, but it references the payment instrument nonce generated, not an actual remote payment profile. This is how Square works. Each saved payment method entity is a non-reusable way to store the payment nonce for processing.

The patch in #39 is failing tests since the rendered billing profile is not showing up in "Payment information" on review page of checkout and the test assert this.

The catch with the tests is that onsite billing information is not displayed on checkout review while offsite billing information is. I don't think this is intentional. See PaymentInformation::buildPaneSummary().

For onsite gateways we have a payment method on checkout review and we render it. The billing profile uid however is 0 (see #3022850: [Addressbook, part 1] Rework the ownership model for customer profiles why) so the user has no access to it (checked by the ER field formatter). As a consequence the billing information won't be displayed.

In the offsite case there's no payment method yet, so we render the billing profile directly, without an access check.

That's why the test has been changed to assert the profile field value and not the page display.

Fixing tests.

@czigor, looks like this block would be more succinct and readable as following:

public function buildConfigurationSummary() {
  if (!empty($this->configuration['payment_methods_reusable'])) {
    if (!empty($this->configuration['payment_methods_always_save'])) {
      if (empty($this->configuration['payment_methods_free_orders'])) {
        $summary = $this->t('Customer payment methods will always be stored');
      }
      else {
        $summary = $this->t('Customer payment methods will always be stored, even if order balance is zero.');
      }
    }
    else {
      $summary = $this->t('Customers can opt-in to store payment methods.');
    }
  }
  else {
    $summary = $this->t('Customers cannot reuse payment methods');
  }

  return $summary;
}

public function buildConfigurationSummary() {
  if (empty($this->configuration['payment_methods_reusable'])) {
    return $this->t('Customers cannot reuse payment methods');
  }

  if (empty($this->configuration['payment_methods_always_save'])) {
    return $this->t('Customers can opt-in to store payment methods.');
  }

  if (empty($this->configuration['payment_methods_free_orders'])) {
    return $this->t('Customer payment methods will always be stored');
  }

  return $this->t('Customer payment methods will always be stored, even if order balance is zero.');
}

Other than that, I think the patch looks great!

Thanks @recidive, that makes sense.

Adding the changes of #48.

Assigning for review

Hi all - i've applied this patch and it seems to work, except that for Stripe payments, the previously used card still appears as an option when making a purchase, even if it is not a stored payment method. Thoughts?

This is for a follow-on...

Clearly this is a change which is needed.

For a site which sells individual products and also subscriptions, the individual products do not need an ongoing payment method, so storing the details should be never, or optional.

For subscriptions, a long-lasting payment method is necessary unless the user is prepared to manually renew each year. This patch does not check whether the products require a re-usable payment method. I don't think there's a general method for determining whether an order requires re-usable, but perhaps the best would be to check whether the order was for a recurring product type, and display a message to the user that their subscription will fail if they do not choose to store customer data.

There's also a class of post-paid subscriptions. For these the initial order total is zero, but I think the payment method details should be collected. However I'm not certain of that.
Edit: These should be covered by collecting payment info for free. But this setting does not come from the requirements of the order.

Just tested this set for customers to opt in, purchasing as a guest. Is it correct that the tickbox to store appears? As a guest, there's nowhere to store this, is there? So how can the tickbox make sense?

But what if the guest has the option to make an account after the purchase? By then it may be too late.

My test was run using Stripe, and the product was a recurring product, so it's possible that one of these are the cause of the issue.

Updated patch #50

Patch from this comment https://www.drupal.org/project/commerce/issues/2871483#comment-13992456 also require some work for each of the payment methods (Paypal, Braintree, Square and Authorize.net etc) because they are not respecting new settings (as it’s described in the issue description for Square).
It means that after installing this patch, new configurations won’t work because additional changes should be done for each of the payment methods.
In general patch works but for each of the payment methods need replace $payment_method->setReusable(FALSE); by code that respecting new settings.

Thanks for the review, Vitaliy! What this means to me is that we need to have some way to only show the checkbox when we know a payment gateway module has been updated to respect this setting. The challenge is to do this without breaking backwards compatibility. My proposal is:

1. Add a function to the PaymentGatewayBase plugin (and related interface) called supportsReusableOption() that returns FALSE by default.
2. In PaymentMethodAddForm where we add the "reusable" checkbox, we should only do so if $payment_method->getPaymentGateway()->supportsReusableOption() returns TRUE.
3. We instruct payment gateways to override this function to return TRUE once they've been updated to properly support the feature.

In Commerce 3.x, I'd expect us to move this function into the SupportsStoredPaymentMethodsInterface and require all payment gateway modules to implement it in order to upgrade to Commerce 3.x.

I haven't done a thorough review of the rest of the patch, but one thing sticks out to me worth reconsidering ... I see an "always_save" boolean being passed around, but it's my understanding we always save the payment method if we can. The intention seems to be more like "always_make_reusable" or something similar.

Add a function to the PaymentGatewayBase plugin (and related interface) called supportsReusableOption()

We used an annotation for a very similar transition with requires_billing_information. Maybe better BC with an annotation.

Stripe distinguishes between 2 different kinds of reusability: on_session and off_session. The difference has SCA implications.

I don't think we need to consider this here, it seems like commerce_stripe can add a Stripe-specific payment information pane extending the work here and adding an additional option. Just mentioning in case anyone else can see an implication from this use case.

Re-rolled so it applies with current dev-2.x

I think this had more to do with a commerce payment module update than anything else.

When I use this I get the following:

PHP Fatal error: Could not check compatibility between Drupal\commerce_payment\PaymentMethodStorage::loadMultipleByUser(Drupal\commerce_payment\AccountInterface $account) and Drupal\commerce_payment\PaymentMethodStorageInterface::loadMultipleByUser(Drupal\Core\Session\AccountInterface $account), because class Drupal\commerce_payment\AccountInterface is not available in drupal9/html/modules/contrib/commerce/modules/payment/src/PaymentMethodStorage.php on line 98

If I add use Drupal\commerce_payment\AccountInterface I get the following:

Fatal error: Class Drupal\commerce_payment\PaymentMethodStorage contains 1 abstract method and must therefore be declared abstract or implement the remaining methods (Drupal\commerce_payment\PaymentMethodStorageInterface::loadMultipleByUser) in drupal9/html/modules/contrib/commerce/modules/payment/src/PaymentMethodStorage.php on line 17

I selected "always"
Used square sandbox as my payment gateway
and no card was saved.

would be great to have a way to give this kind of option to OffsitePaymentGatewayInterface implementing SupportsStoredPaymentMethodsInterface too. atm, it is a struggle to give the user the choice and this patch wouldn't help either as implementing SupportsCreatingPaymentMethodsInterface is off the table as it requires to know the payment_type upfront AND creates the $payment_method - what shouldn't be done as that is within the authority of the OffsitePaymentGatewayInterface (see my issue #3274863: Payment options for OffsitePaymentGatewayBase implementing SupportsStoredPaymentMethodsInterface are inconsistent).

Expected behavior (when opting out or disabling stored payment method):
- payment method not reusable at checkout
- payment method does not get written to database

Observed behavior:
- old payment method not visible at checkout
- payment method still written to database

Just been testing this patch and seeing the same as AaronBauman in a fresh commerce install.

I tried changing the saved method to reusable and it shows in the payment details tab on the account and on checkout but you can't use it or delete it. So my assumption is that this is just a place holder for the order payment details?

Other than that patch #60 seems to be working with Braintree Commerce for me

More testing with patch #60, and I'm seeing that the payment method marked `reusable = 0` is in fact reusable.
After configuring the default (and only) payment flow for "no reusable payments", I can reuse a payment by:
1. put a product in the cart
2. go to checkout
3. enter credit card and click through to "review" step
4. do not submit payment, but go back to cart
5. empty cart
6. add a product to cart again
7. go to checkout - the payment method from 3 is there

Expected behavior would be that, after step 4 or 5, the payment method is no longer visible or usable.

Well "reusable" means reusable for other orders, if you saved a card for a particular order, then ofc you can use it for that order...
If we'd empty the credit card when emptying the cart, I'm sure most other people would complain.

From a site builder's perspective, having the ability for a site user to NOT have their credit card details stored in my database is essential.

I'm not interested in taking on the security implications of storing a user's credit card details.

I think this issue needs upgrading to at least major.

@glennnz: Raw credit card details are never stored, and are in fact not even received by drupal. The card fields are iframes hosted elsewhere (for on site gateways).

The resulting payment method id received back from the gateway is stored. Only the last 4 digits of the card can be retrieved using that.

@john-pitcairn Ah, excellent! Thanks.

Just jumping in here, patch 60 seems to be working in initial testing.

Merge request from #74 seems to be missing the "Save this payment method for later use" checkbox at checkout for some reason.

@tBKot can you provide a patch and interdiff.txt between your work from 74 and the patch from #60?

I am seeing the same thing as @tonytheferg

patch in #60 seems to work, but the MR does not show the "Save this payment method for later use" checkbox on the checkout form.

Here is an interdiff of #60 and MR 149

It appears that with the MR, in order for the checkbox to appear on the checkout form, the payment gateway needs to have the public method

 public function supportsReusableOption(): bool {
    return TRUE;
  }

When it does, the checkbox appears and the payment method is stored at checkout when checked.

This follows the suggestions in #57

I think this is a good addition, but gateway contrib modules will need to support this.

Here's a patch version of the PR as of comment 74, along with patches for commerce_stripe and commerce_authnet to make them compatible.

I suspect those probably belong in the other modules' issue queues, but until we are sure this is how things are being done I think it makes sense to keep it all in one place.

I found that the patch above caused a crash on free orders that didn't have a payment method attached when trying to produce the payment information summary pane. The attached version makes a small check to avoid that crash. (I initially reported this as a separate issue, which I will close, but there are more details there if you are interested.

Handling of free orders is actually not mentioned in the issue summary here but sneaked into the patch ;) Because currently we always make payment methods reusable, and here this option is squeezed under always reusable, I am creating #3468552: Require creating payment method on free orders and will extract the relevant part of #79 in there.

Currently not commenting on the reusability feature but will come back to comment on that soon.

Here are some objectives that should be included in this issue's scope:
- customer must be able to opt-out from reusable payment method (::setReusable(FALSE)). Non-reusable payment methods will only be used to create initial payment.
- gateway must be able to declare whether the payment method it creates is reusable. We must be able to filter out all unreusable payment methods. whether existing or new, depending on the context. For example, commerce_recurring must be able to enforce checkout with a reusable payment method for the sake of subsequent charges
- some gateways, depending on configuration, may be reusable but only in the Customer Initiated Transaction (CIT) scenario. This is already possible for Stripe. Such payment methods should not be reusable for Merchant Initiated Transactions (MIT). We must be able to filter them out #3469534: Define payment method's reusability context

I don't think we currently have one but this probably calls for a meta issue so we can divide the scope into some smaller chunks and work on them one by one,

Should we be making this feature for 3.x now?

It looks like 3.0 already handles free orders with a new setting for require_payment_method to 'Collect payment methods on orders with zero balance'

I don't think so that the automated collection of payment methods, (commerce_stripe credit card details) without user concern is a good behaver, (GDPR compliant).

So this would be great to implement.

Should we be making this feature for 3.x now?

Yes, any MR should target 3.0.x.

Add a function to the PaymentGatewayBase plugin (and related interface) called supportsReusableOption() that returns FALSE by default.

Looking at the patch, I don't really understand why OffsiteRedirect overrides supportsReusableOption() and returns TRUE.

Also, not sure "option" is fully necessary for the method name, as it seems to imply "Forms API" / Fullstack Drupal though I'm not really sure how this could resurface this "setting" on a Headless setup.

Several comments:

• The "payment_methods_reusable" checkout pane setting: Not sure this setting makes sense as is as existing payment gateways wouldn't respect it from the start since gateways are currently conditionally flagging a payment method as reusable.
• I think we might just need a setting to allow a customer to opt out from saving the payment method (So perhaps only 'payment_methods_always_reusable').

Also, I have a problem with a general method / setting at the gateway level that basically indicates whether the gateway supports reusable payment methods.
The main issue I see here is a payment gateway like Braintree or Stripe for example that supports both PayPal and credit cards for example. Some payment method types might be reusable, while others might not.
Also, I see that the Stripe module defines an isReusable() method at the payment method type level, the "Affirm" payment methods aren't reusable for instance.

The current approach is too simplistic, and doesn't take into account the current state of the different contrib payment gateways, I'm really concerned we could end up with a crappy UX where the reusable checkbox is exposed when it shouldn't and vice versa.

Regarding:

We used an annotation for a very similar transition with requires_billing_information. Maybe better BC with an annotation.

We could go with "supports_reusable_payment_methods", which probably needs to default to TRUE, to take into account the existing behavior (i.e. for BC reasons).
I think similarly to "requires_billing_information", we need an associated method (similar to collectsBillingInformation(), where the default implementation reads from the annotation/ gateway attribute, but more complex use cases like the Stripe one could have conditional logic based on the payment method type.

In such case, I think supportsReusableOption() needs to either accept a $payment_method or a $payment_method_type. Passing the payment method gives more flexibility as the payment method type can be obtained like the following:

$payment_method_type = $payment_method->getType();

From the PaymentInformation pane, we could call supportsReusableOption() (or its equivalent right after calling creating the payment method via the createForCustomer() method.

So the code would look like the following:

    $payment_method = $payment_method_storage->createForCustomer(
      $payment_option->getPaymentMethodTypeId(),
      $payment_option->getPaymentGatewayId(),
      $this->order->getCustomerId(),
      $this->order->getBillingProfile()
    );

   if ($payment_method->getPaymentGateway()->getPlugin()->supportsReusableOption($payment_method)) {
   }

Additionally, rather than passing flags via the $pane_form['add_payment_method'] array, we should probably extend the PaymentGatewayForm configuration.
From:

    $inline_form = $this->inlineFormManager->createInstance('payment_gateway_form', [
      'operation' => 'add-payment-method',
    ], $payment_method);

To:

    $inline_form = $this->inlineFormManager->createInstance('payment_gateway_form', [
      'operation' => 'add-payment-method',
      'supports_reusable_payment_methods' => ...,
     // We need to expose one or 2 different settings here, one to allow opting out saving the payment method, and another flag to indicate whether the reusable checkbox should be exposed. 
      'expose_reusable_checkbox' => ...,
    ], $payment_method);

One last thing, we either need as @zaporylie mentioned a way for commerce_recurring to enforce saving a payment method as always reusable... Or... Since this is the default behavior that should be explicitly turned off, we can assume that somebody using commerce_recurring is not going to change the default behavior?

Since the same site could in theory sell digital subscriptions and physical products, it does make sense to have this configurable at the checkout pane level so the behavior can be tweaked per checkout flow.

This patch conflicts with a commit to Commerce 3.x #2907504: Display payment method in the CheckoutPane Payment informations even if only single payment gateway available.

MR450 is blank

The above MR as a patch for builds.

Are the patches in #78 still needed?

if you are using those payment processors and want payment methods using them to be reusable, then yes.

I was able to fully remove the “By providing your card information…” message from Stripe Elements in Drupal Commerce by hooking into the Stripe Payment Intent creation event and unsetting the setup_future_usage parameter.

Stripe shows that message automatically whenever setup_future_usage is present (even if the value is null). Some Drupal Commerce payment gateways add it implicitly.

To force single-use payments only and prevent Stripe from displaying the saved-card consent message, I added an event subscriber:

How to implement (folder structure + file)

Create a custom module (example: konordo_stripe_event_subscriber) with the following structure:

modules/custom/konordo_stripe_event_subscriber/
  ├── konordo_stripe_event_subscriber.info.yml
  ├── konordo_stripe_event_subscriber.services.yml
  └── src/EventSubscriber/KonordoStripeEventSubscriber.php

konordo_stripe_event_subscriber.info.yml

name: Konordo Stripe Event Subscriber
type: module
description: Custom overrides for Commerce Stripe events.
core_version_requirement: ^10 || ^11
package: Commerce
dependencies:
  - drupal:commerce_stripe

konordo_stripe_event_subscriber.services.yml

services:
  konordo_stripe_event_subscriber:
    class: Drupal\konordo_stripe_event_subscriber\EventSubscriber\KonordoStripeEventSubscriber
    tags:
      - { name: event_subscriber }

src/EventSubscriber/KonordoStripeEventSubscriber.php

<?php

namespace Drupal\konordo_stripe_event_subscriber\EventSubscriber;

use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Drupal\commerce_stripe\Event\StripeEvents;
use Drupal\commerce_stripe\Event\StripePaymentIntentCreateEvent;

/**
 * Removes setup_future_usage to disable saved-card message.
 */
class KonordoStripeEventSubscriber implements EventSubscriberInterface {

  /**
   * {@inheritdoc}
   */
  public static function getSubscribedEvents() {
    $events[StripeEvents::PAYMENT_INTENT_CREATE][] = ['onPaymentIntentCreate'];
    return $events;
  }

  /**
   * Forces single-use payments by unsetting setup_future_usage.
   */
  public function onPaymentIntentCreate(StripePaymentIntentCreateEvent $event) {
    $payment_intent = $event->getPaymentIntent();

    // Remove future usage so Stripe does NOT show the saved-card disclaimer.
    $payment_intent->unsetSetupFutureUsage();
  }

}

Result

Stripe Elements no longer displays:

“By providing your card information, you allow to charge your card for future payments…”

The payment now behaves as a real one-time charge, with no implication of future billing.

This is very useful for Commerce sites that do not store cards or use subscriptions.

Great work! Glad to see this finally possible.

Some suggestions:
* This would need a note in the release notes documenting the change, including the different steps that must be taken to disable it, as imho it's a major change.
* Default lock_payment_method_reusability to FALSE; I would wager most sites do not want to force this for visitors.
* The option during checkout to save the payment method should default to FALSE, or at least the site builder should be able to change the default.

FWIW in my local testing this does seem to work correctly for new orders.

Here's a patch that works on top of MR #450 that allows controlling whether the payments are saved by default.

Attaching new patches for commerce_stripe and commerce_authnet that are compatible with the latest MR.

Found a case that wasn't working as expected: Not allowing payment methods to be saved.

Here is a patch that works on top of #97 to allow `reusable` to be set to FALSE when locked. Previously it was only considering if the payment method had it `reusable` field set (which they do by default) but not respecting the configured default.