Require creating payment method on free orders

Just wanted to provide some input here. Some payment gateways like Braintree forbid "Negative option billing".
See https://developer.paypal.com/braintree/articles/guides/recurring-billing... for more information.

It's generally not recommended to start charging a customer after a free trial, especially if we're failing to notify prior to the charge, or if we don't offer a way to opt out...
Not really sure that affects us, as this is probably on the merchant shoulder to solve, but figured I'd share this anyway.

Created MR with a functional draft.

I improved the interface comment for the SupportsFreeOrdersInterface.

Let me elaborate on the motivations here. (1) Both Onsite and Offsite payment gateways should be allowed to proceed through checkout and collect payment methods. This is why SupportsFreeOrdersInterface inherits from SupportsStoredPaymentMethodsInterface. (2) The SupportsFreeOrdersInterface interface doesn't define any methods at the moment as none is strictly necessary to proceed with free order. However, each payment gateway that supports free orders will likely have to adapt its code to explicitly support free orders. (a) ::createPaymentMethod() the API call for creating Payment Method may need modification depending if the total amount is a positive number or zero (b) ::createPayment() may need to omit to create payment if the amount is zero, (c) ::onReturn() and ::onNotify() as well as PaymentOffsiteForm::buildConfigurationForm may need certain adjustments to support free orders. (3) We may want to use SupportsFreeOrdersInterface for defining a method that would show helpful text in the checkout. Ie. "Your credit card will be authorized during the checkout process and stored for future payment" or "You may notice 1EUR authorization on your credit card that will be reverted immediately. This is required to validate your payment details.". I don't think it's strictly necessary and can be easily done by the merchant by customizing their checkout to include such information. (4) FreeOrderPaymentOptionsSubscriber is taking care of filtering incompatible payment options from the payment options list available to customers. The filtering is happening only if PaymentInformation checkout pane was configured to collect payments on free orders.

We need tests coverage, and we should probably have either the Onsite payment gateway and/or the example_stored_offsite_redirect implement this interface so we can test that the changes are working as expected.

We need to test that the payment information pane is visible, that a payment method is collected etc... I think PaymentCheckoutTest would be the right fit for that.

And we also need a change record, to inform gateway developers of the change.

Let's rename the subscriber service to commerce_payment.free_order_payments_options_subscriber

Renamed the service and added support to example_stored_offsite_redirect and example_onsite. Given different gateways may need to approach implementation differently example_onsite does create payment even if the payment amount is 0,- and example_stored_offsite_redirect creates a payment method but never creates a payment if the payment amount is 0,-.

I will add automated tests later today and work on the CR

I just realized that the subscriber doesn't even check the order total.

I'm now shared and wondering whether we shouldn't adopt an approach similar to the "requires_billing_information" boolean.
Whenever a payment gateway explicitly say that it doesn't require billing information, the payment gateway base outputs an additional setting:

$form['collect_billing_information'] = [
      '#type' => 'checkbox',
      '#title' => $this->t('Collect billing information'),
      '#description' => $this->t('Before disabling, make sure you are not legally required to collect billing information.'),
      '#default_value' => $this->configuration['collect_billing_information'],
      // Merchants can disable collecting billing information only if the
      // payment gateway indicated that it doesn't require it.
      '#access' => !$this->pluginDefinition['requires_billing_information'],
    ];

Perhaps we should do the same here?

The annotation key / attribute value could be "support_free_orders"?

Sharing here a potential alternative approach... Which could consist in dispatching an event from the isVisible() method, so modules can have a say on whether the payment information pane should be visible.
This way, thinking of the recurring use case, commerce_recurring could force the payment information pane visible, if the order contains a subscription item?

Oh wait, this proposal doesn't work at all... For whatever reason, thought the pane was made not visible and had an isVisible() implementation.

Re #9

I am not a big supporter of this approach as I said, the checkbox would conflict with the order total condition that can be set on the Payment Gateway. I believe the compatibility should be set via interface and whether the gateway shows or not can be controlled via condition.

Changing a bit naming of this issue as while we discussed it, it became obvious that what we're trying to do here is to force payment method collection rather than allow it. Allowing wasn't a correct term either for the current implementation as the compatible payment method was required to complete the checkout. The changes to MR will follow:
- rename the checkbox accordingly
- dispatch the event which can modify the value set by the checkbox

Let's make this happen in 3.0.x instead.

zaporylie → closed merge request !306

I closed the previous MR and opened a new one, moving the target branch to 3.0.x

Some changes since #13
- Introduced PaymentCheckoutPaneBase which is shared between PaymentInformation and PaymentProcess for the sake of sharing collectBillingProfileOnly()
- switched from FILTER_PAYMENT_OPTIONS to FILTER_PAYMENT_GATEWAYS and updated the subscriber to affect list of available gateways depending on order balance
- added tests

Added draft CR https://www.drupal.org/node/3470809 → for review and publishing

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