Allow payment gateways to explicitly support payment method creation outside of a payment transaction

I just realized this title doesn't quite make sense, because we do allow creation of payment methods from user pages when there is an on-site payment gateway.

Here's an initial patch spinning out the work from the parent issue. All this does is add the new SupportsCreatingPaymentMethodsInterface interface and adjust internal code to differentiate between this and SupportsStoredPaymentMethodsInterface on PaymentInformation and PaymentProcess panes.

We shouldn't have any test fails. The next steps require porting the example_stored_offsite_redirect payment gateway plugin and verifying an off-site payment gateway can have a saved payment method charged.

1. +++ b/modules/payment/src/Plugin/Commerce/CheckoutPane/PaymentInformation.php
   @@ -350,7 +383,8 @@ class PaymentInformation extends CheckoutPaneBase {
          // The billing profile is provided either because the order is free,
   -      // or the selected gateway is off-site. If it's the former, stop here.
   +      // or the selected gateway does not support stored payment methods.
   +      // If it's the former, stop here.
          if ($this->order->isPaid() || $this->order->getTotalPrice()->isZero()) {
   

   This comment feels like we have a logic issue.

   I don't know if it should be fixed here or in #2871483: Add checkout settings for payment method behavior, where we will handle payment information settings in the context of free orders.

2. +++ b/modules/payment/src/Plugin/Commerce/CheckoutPane/PaymentProcess.php
   @@ -173,9 +174,9 @@ class PaymentProcess extends CheckoutPaneBase {
   -    if ($payment_gateway_plugin instanceof OnsitePaymentGatewayInterface) {
   +    if ($payment_gateway_plugin instanceof SupportsStoredPaymentMethodsInterface && $payment_method = $this->order->payment_method->entity) {
   

   This section be evaluated as well, but in another issue: what happens if someone doesn't implement one of our *PaymentGatewayInterface interfaces we just continue, assuming the value is null due to a free order.

   We should open a follow up to explicitly throw an exception if the order isn't free and we're unable to process payment.

Okay, no regressions. Time to add the tests.

Here is a patch which adds the example_stored_offsite_redirect gateway from the parent issue. I've added the testCheckoutWithStoredOffsiteRedirectPost test which goes through checkout with the gateway. It generates a payment method and then ensures we can go back through checkout again with the generated payment method.

It also exposes a need for #3137636: commerce_payment.order_updater should sync order payment method from payment transaction.

1. +++ b/modules/payment/src/Plugin/Commerce/CheckoutPane/PaymentInformation.php
   @@ -207,16 +210,46 @@ class PaymentInformation extends CheckoutPaneBase {
   +    $payment_gateway_plugin = $payment_gateway->getPlugin();
   +    // If this payment gateway plugin supports creating payment methods, we
   +    // build the "add-payment-method" plugin form.
   +    if ($payment_gateway_plugin instanceof SupportsCreatingPaymentMethodsInterface) {
   
   +++ b/modules/payment/src/Plugin/Commerce/PaymentGateway/SupportsCreatingPaymentMethodsInterface.php
   @@ -0,0 +1,31 @@
   + * @todo better description
   + *
   + * Payment gateways that implement this interface identify that they allow
   + * creating payment methods outside of the process of creating a payment.
   

   Need to work on the words for what "supports creating payment methods" means.

2. +++ b/modules/payment/src/Plugin/Commerce/CheckoutPane/PaymentInformation.php
   @@ -207,16 +210,46 @@ class PaymentInformation extends CheckoutPaneBase {
   +    // If the gateway does not support creating payment methods at checkout, but
   +    // it does still support payment with a stored payment method, do not
   +    // collect billing information for existing payment methods.
   +    if ($payment_gateway instanceof SupportsStoredPaymentMethodsInterface && $payment_option->getPaymentMethodId()) {
   

   This feels awkward, but I can't think of a better way.

3. +++ b/modules/payment/src/Plugin/Commerce/CheckoutPane/PaymentProcess.php
   @@ -173,9 +173,9 @@ class PaymentProcess extends CheckoutPaneBase {
   -    if ($payment_gateway_plugin instanceof OnsitePaymentGatewayInterface) {
   +    if ($payment_gateway_plugin instanceof SupportsStoredPaymentMethodsInterface && $payment_method = $this->order->payment_method->entity) {
   ...
   -        $payment->payment_method = $this->order->payment_method->entity;
   +        $payment->payment_method = $payment_method;
   

   Copied over from the last patch, I don't like doing variable definitions within logic checks.

   This also leaves us in possible a weird state.

   What if we have a SupportsCreatingPaymentMethods payment plugin that has no payment method (for whatever reason)?

   The next condition checking OffsitePaymentGatewayInterface would fail, so would the check for ManualPaymentGatewayInterface, and then it'd just proceed without payment.

   This is part of the problem identified in #5. Part of it should be addressed outside of this issue, but it's more apparent here.

4. +++ b/modules/payment/tests/src/FunctionalJavascript/PaymentCheckoutTest.php
   @@ -156,6 +157,18 @@ class PaymentCheckoutTest extends CommerceWebDriverTestBase {
   +    /** @var \Drupal\commerce_payment\Entity\PaymentGatewayInterface $payment_gateway */
   +    $payment_gateway = PaymentGateway::create([
   +      'id' => 'stored_offsite',
   +      'label' => 'Stored off-site',
   +      'plugin' => 'example_stored_offsite_redirect',
   +      'configuration' => [
   +        'redirect_method' => 'post',
   +        'payment_method_types' => ['credit_card'],
   +      ],
   +    ]);
   +    $payment_gateway->save();
   

   The test might be cleaner if we had a test module which installed the gateway configurations.

This adds the test fix.

Addressed #7.1 and #7.2. Added an explicitly early check if the default payment option is an existing payment method before checking if we should render the payment method form or billing information form.

I opened #3137644: Harden logic in payment process step for #7.3. It's an edgecase

Also, for the payment method check, we can just change to:

    $payment_method = $this->order->payment_method->entity;
    if ($payment_gateway_plugin instanceof SupportsStoredPaymentMethodsInterface) {
      try {
        $payment->payment_method = $payment_method;
        $payment_gateway_plugin->createPayment($payment, $this->configuration['capture']);

Payment gateways should be using \Drupal\commerce_payment\Plugin\Commerce\PaymentGateway\PaymentGatewayBase::assertPaymentMethod to check the payment method received. If the value is null an exception will be thrown and the customer kicked back. The exception text will be logged and the developer knows how to fix their integration.

1. +++ b/modules/payment/src/Plugin/Commerce/CheckoutPane/PaymentInformation.php
   @@ -207,10 +210,23 @@ class PaymentInformation extends CheckoutPaneBase {
   +    $payment_gateway_plugin = $payment_gateway->getPlugin();
   ...
   +    // If this is an existing payment method, return the pane form.
   +    // Editing payment methods at checkout is not supported.
   +    if ($default_option->getPaymentMethodId()) {
   +      return $pane_form;
   +    }
   

   🤔 we could put this logic check earlier, even.

   There's no need to perform these lines of code if we have an existing payment method:

       $default_payment_gateway_id = $default_option->getPaymentGatewayId();
       $payment_gateway = $payment_gateways[$default_payment_gateway_id];
       $payment_gateway_plugin = $payment_gateway->getPlugin();
   

2. +++ b/modules/payment/src/Plugin/Commerce/CheckoutPane/PaymentInformation.php
   @@ -231,11 +247,6 @@ class PaymentInformation extends CheckoutPaneBase {
      protected function buildPaymentMethodForm(array $pane_form, FormStateInterface $form_state, PaymentOption $payment_option) {
   -    if ($payment_option->getPaymentMethodId() && !$payment_option->getPaymentMethodTypeId()) {
   -      // Editing payment methods at checkout is not supported.
   -      return $pane_form;
   -    }
   -
   

   No longer needed in this method, since we're checking ahead of time.

🤦‍♂️ we need the check for SupportsStoredPaymentMethodsInterface and a valid payment method. Otherwise, off-site payment gateways with support for stored payment gateways fail if not reusing a payment method.

We can't explicitly check SupportsCreatingPaymentMethodsInterface, either, as that would skip off-site payment gateways when reusing a payment method.

I guess the fix is to add back the checkout for the payment method and add a safeguard here:

    else {
      $this->checkoutFlow->redirectToStep($next_step_id);
    }

Throw an exception and redirect to the error step, fixing #3137644: Harden logic in payment process step in this issue.

Fixes identified in #11.

Committing after getting review of the change record.

Committed!