search

pronamic / wp-pronamic-pay

The Pronamic Pay plugin allows you to easily accept payments with payment methods like credit card, iDEAL, Bancontact and Sofort through a variety of payment providers on your WordPress website.
https://pronamicpay.com
35 stars 14 forks source link

Gateways / payment methods input fields API #154

Closed remcotolsma closed 2 years ago

remcotolsma commented 4 years ago

Some gateways and payment methods require extra input from the customer. For example: iDEAL payment method requires sometimes an iDEAL issuer / bank and AfterPay often requires a birth date and gender. In this core library we have support for input fields. There is however room for improvement so we can write code more according to the DRY principle. Some gateways / payment methods require the same input. In our internal Basecamp project we also have a to-do open to this:

Abstractere manier vinden voor het opvragen van extra benodigde informatie zoals geboortedatum, geslacht, IBAN, etc.

I think we need a global place where can define 'checkout fields'. In this core library we can define the most common used 'checkout fields' like:

Also see:

https://github.com/woocommerce/woocommerce/blob/4.7.0/includes/class-wc-checkout.php#L197-L281

https://github.com/woocommerce/woocommerce/blob/4.7.0/includes/abstracts/abstract-wc-payment-gateway.php#L392-L405

For each gateway / payment method we must be able to indicate what input is required. As soon as an extension does not provide the required data, we can show an intermediate screen in which the missing data is requested. Similar to how, for example, OmniKassa 2.0 does this:

Schermafbeelding 2018-11-06 om 14 34 49

We might need to extend the way we define which payment methods are supported by the gateways in get_supported_payment_methods():

<?php

class PaymentMethod {
    private $id;

    private $name;

    private $icon;

    private $supported;

    private $active;

    private $fields;

    // ...
}

class Gateway {
    // ...

    public function __construct() {
        // ...

        $this->register_payment_method( new PaymentMethod( 'ideal', 'iDEAL', $supported = true, $active = true, $fields = array( Fields::get( 'ideal-issueur' ) ) ) );
        $this->register_payment_method( new PaymentMethod( 'afterpay', 'AfterPay', $supported = true, $active = true, $fields = array( Fields::get( 'birth-date' ), Fields::get( 'gender' ) ) ) );
        $this->register_payment_method( new PaymentMethodBancontact() );
    }
}

// ...

That way we can also only show the input fields in the test meta box that are required for the chosen payment method.

Schermafbeelding 2020-11-12 om 12 48 21

This requires some thought.

rvdsteege commented 4 years ago

Sounds like a good enhancement. First thoughts:

  1. I don't think the $supported parameter makes sense if payment methods are registered this way?
  2. I'm not sure if we should include an $active parameter here. We might want do something different for the 'active' part, as we probably also want to be able to determine that based on other parameters (country, currency, etc.). Also, it would be convenient if payment methods can just be registered as in the Bancontact example without parameters: 
    $this->register_payment_method( new PaymentMethodBancontact() );
  3. A field should be able to somehow have access to the gateway, as for example the issuer options for an ideal-issuer field differ by gateway.
remcotolsma commented 4 years ago
  1. I don't think the $supported parameter makes sense if payment methods are registered this way?

It might be useful to register unsupported payment methods. In the UI we can inform users about unsupported payment methods.

  1. I'm not sure if we should include an $active parameter here.

Good point, maybe this is a better approach?


// ...

$gateway->can_process_payment( $payment );

// ...

class Gateway {
    // ...

    public function can_process_payment( Payment $payment ) {
        $method = $payment->get_method();

        // check if method is supported?

        // check if method is active at payment provider?

        // check if amount is sufficient?

        // check country / currency / etc.
    }
}
  1. A field should be able to somehow have access to the gateway, as for example the issuer options for an ideal-issuer field differ by gateway.

Yes, i agree. A gateway integration could extend the default iDEAL issuer field:


class IDealIssuerField extends CoreIDealIssuerField {
    public function __construct( $gateway ) {
        $this->gateway = $gateway;
    }

    public function get_options() {
        // Request iDEAL issuer via `$gateway`?
    }
}

Thats requires gateway integrations to register their own iDEAL issuer field.

We could also transform the core iDEAL issuers options to the gateway / provider corresponding iDEAL issuer value. Downside of that is that we no longer have a dynamic iDEAL issuers list that is updated automatically.

Or we could implement a callback in the PaymentMethodIDeal or IDealIsseurField class to retrieve the options / issuers:


$this->register_payment_method( new PaymentMethodIDeal( $callback = function() use( $gateway ) {
  // Request iDEAL issuer via `$gateway`?
} ) );
rvdsteege commented 4 years ago

It might be useful to register unsupported payment methods. In the UI we can inform users about unsupported payment methods.

👍

Good point, maybe this is a better approach?

Yes, $gateway->can_process_payment( $payment ); is a better fit.

A gateway integration could extend the default iDEAL issuer field [...]

That seems the best approach to me. Loosing the dynamic lists is not an option.

remcotolsma commented 3 years ago 
$this->register_payment_method( new PaymentMethod( 'ideal', 'iDEAL', $supported = true, $active = true, $fields = array( Fields::get( 'ideal-issueur' ) ) ) );

For some gateways the iDEAL bank / issuer is required to start a payment, other gateways will ask customers to choose the bank / issuer in their environment. Therefore it is probably not useful to reuse the iDEAL issuer field over different gateways.

$ideal_issuer_field = new IDealIsseurField();
$ideal_issuer_field->set_required( true );

$payment_method_ideal = new PaymentMethod( 'ideal', 'iDEAL' );
$payment_method_ideal->set_supported( true );
$payment_method_ideal->add_field( $ideal_issuer_field );

https://www.digiwallet.nl/nl/documentation/paymethods/directdebit#startapi

$iban_field = new IBANField();
$iban_field->set_required( true );

$name_field = new NameField();
$name_field->set_required( true );

$payment_method_direct_debit = new PaymentMethod( 'direct_debit', 'Direct Debit' );
$payment_method_direct_debit->set_supported( true );
$payment_method_direct_debit->add_field( $iban_field );
$payment_method_direct_debit->add_field( $name_field );

https://github.com/wp-pay-extensions/woocommerce/blob/bc3dd708cde1c54f0aec218da91b097f54f7c89a/src/Gateway.php#L181-L196

In WooCommerce we can change this to something like:

$payment_method = $gateway->get_payment_method( $this->payment_method );

$fields = $payment_method->get_fields();

In the test meta box we can iterate over all the available payment methods.

foreach ( $gateway->get_payment_methods() as $payment_method ) {
    foreach ( $payment_method->get_fields() as $field ) {
        // render field
    }
}

With some JavaScript / AlpineJS magic we can show only the fields for the selected payment method.

For forms plugins like Gravity Forms it's harder to make sure we retrieve all the required input.

rvdsteege commented 2 years ago

Related issue/PR, which would benefit from improved payment method registration:

remcotolsma commented 2 years ago

Currently we have the following fields:

Gender:

Date of birth:

Consumer bank details name:

Consumer bank details IBAN:

Issuer iDEAL

Issuer credit card

remcotolsma commented 2 years ago

I removed the $gateway->get_issuer_field() method in https://github.com/pronamic/wp-pay-core/commit/f8a263781d3a4f72bd9d793ed615f0df232352a9. This method is however still in use in Gravity Forms and Formidable Forms:

To-do:

remcotolsma commented 2 years ago

We should not forget that PaymentMethods::DIRECT_DEBIT_BANCONTACT, PaymentMethods::DIRECT_DEBIT_IDEAL and PaymentMethods::DIRECT_DEBIT_SOFORT may also require fields. For PaymentMethods::DIRECT_DEBIT_IDEAL we want to display the iDEAL issuers (banks).

remcotolsma commented 2 years ago
remcotolsma commented 2 years ago

In SelectField we need better support for <optgroup>: https://developer.mozilla.org/en-US/docs/Web/API/HTMLSelectElement/add

Not all supported extensions have support for <optgroup>.

We need a way to flat <optgroup> to just a flat list of options.

remcotolsma commented 2 years ago

In https://github.com/pronamic/wp-pay-core/commit/f21d102f70f2dba6c7ed0b77631041017350944b we removed the phone number field from the test meta box. We have to figure out how we handle checkout fields like a phone number. Payment methods like AfterPay.nl often also require a complete shipping and billing address. Perhaps we should extend the test meta box to a full-fledged checkout form such as WooCommerce. For example, for the AfterPay.nl payment method, we may have to indicate that the fields 'shipping address' and 'billing address' are required, but leave the display and handling of these fields to the extensions. Also see:

remcotolsma commented 2 years ago

I think discussed before my holiday with @rvdsteege, but could not find any information about this. We could add a $gateway->get_payment_methods( $query_args ) parameter.

https://github.com/pronamic/wp-pay-core/blob/97dabf83b1d0eca8150b2be7586731d54eecb1e5/src/Core/Gateway.php#L127-L134

That allows us to 'query' for example only the payment methods with the status active:

$payment_methods = $gateway->get_payment_methods( [
    'status' => 'active',
] );
$payment_methods = $gateway->get_payment_methods( [
    'status'        => 'active',
    'cache_results' => false,
] );

Something similar might also come in handy within the fields:

$fields = $payment_method->get_fields( [
    'class' => IdealIssuerField::class,
] );

https://github.com/pronamic/wp-pay-core/commit/93b251b657dd38d5a8e965efa401914644d9c81c

remcotolsma commented 2 years ago

I think discussed before my holiday with @rvdsteege, but could not find any information about this.

Found it in https://github.com/pronamic/wp-pronamic-pay-private/issues/8.

remcotolsma commented 2 years ago

For next week we have still a few things to-do:

remcotolsma commented 2 years ago

Match fields with https://wordpress.github.io/gutenberg/?path=/story/components-selectcontrol--default naming?

We stick to term field instead of control, we define the required fields, the term control is more UI related.

remcotolsma commented 2 years ago
remcotolsma commented 2 years ago

Add support for fields processing / handling, how is input via $_POST handled?

For the iDEAL issuer this is currently handled in: Plugin:: complement_payment( Payment $payment ). https://github.com/pronamic/wp-pay-core/blob/046ed470ff0966419fda8ae663a9630b95b0c70a/src/Plugin.php#L932-L951

We should no longer use the filter_has_var() and filter_input() functions, but using the super global $_POST is also not handy. Some extensions may store the post data in another variable, see for example: https://github.com/pronamic/wp-pronamic-pay-restrict-content-pro/blob/8ec4a2aae1c446bb2c9fc271f351d219b58c9121/src/Util.php#L60-L67.

In Restrict Content Pro:

$payment->post_data = $gateway->subscription_data['post_data'];

in core:

$payment->post_data = $_POST;

also in core:

Plugin::process_payment_post_data( $payment ) {
    $gateway = $payment->get_gateway();

    if ( null === $gateway ) {
        return;
    }

    $payment_method = $gateway->get_payment_method( $payment->get_payment_method() );

    if ( null === $payment_method ) {
        return;
    }

    foreach ( $payment_method->get_fields() as $field ) {
        if ( array_key_exists( $field->get_id(), $payment->post_data ) ) {
            // ?
        }
    }   
}
remcotolsma commented 2 years ago

I think we are done, the items from https://github.com/pronamic/wp-pronamic-pay/issues/154#issuecomment-1222428469 are included on following issue: