pronamic / wp-pay-core

Core components for the WordPress payment processing library. This library is used in the WordPress plugin Pronamic Pay: https://www.pronamicpay.com/, but also allows other plugin developers to set up a payment plugin.
https://www.wp-pay.org/
GNU General Public License v3.0
27 stars 3 forks source link

"start" funcation getting called twice #97

Closed knit-pay closed 1 year ago

knit-pay commented 1 year ago

Hello Team

$gateway->start( $payment ); function getting called twice.

  1. While initiating payment. https://github.com/pronamic/wp-pay-core/blob/0c4296428a2f31f542113cd81424c5ddd9f4a5d4/src/Plugin.php#L1196

  2. While performing redirection in many extensions https://github.com/pronamic/wp-pay-core/blob/0c4296428a2f31f542113cd81424c5ddd9f4a5d4/src/Plugin.php#L607

In some gateways, API request needs to be called in start function. Due to this twice call of start function. API calls and many other operations are getting executed twice, which is causing conflict in many cases.

In my opinion, we should remove this second execution. If you think second execution is required for some gateways, kindly change the below code. https://github.com/pronamic/wp-pay-core/blob/0c4296428a2f31f542113cd81424c5ddd9f4a5d4/src/Admin/AdminModule.php#L834

to

wp_redirect( $payment->get_pay_redirect_url() );
exit();

So that while testing a gateway from the configuration page, we can get a similar flow.

rvdsteege commented 1 year ago

The 2nd $gateway->start( $payment ) you're referring to, is intentional for gateways using the form method. For these gateways, the start() method doesn't do much except for setting the action URL of the form.

In $gateway->get_output_fields( $payment ) you then build the parameters to be submitted through a form to the action URL. If you need to call an API endpoint to be able to construct the form fields, you should do that here — in $gateway->get_output_fields( $payment ).

For examples of gateways using the form method type, see:

  1. https://github.com/pronamic/wp-pronamic-pay-ingenico/blob/969e26961cc51a7691cb91115937a5fb4f36443f/src/OrderStandard/Gateway.php#L131-L146
  2. https://github.com/pronamic/wp-pronamic-pay-ideal-basic/blob/66d5747915db073cdeaa0fb123a9feb7489299ac/src/Gateway.php#L73-L109
  3. https://github.com/pronamic/wp-pronamic-pay-ems-e-commerce/blob/60cd0b35b0480c589e4c2f3538f064cff86b01dc/src/Gateway.php#L63-L73

If the response of your gateway API endpoint call contains an URL to which you need to redirect the user, you need to change the method to HTTP redirect in the gateway constructor:

$this->set_method( self::METHOD_HTTP_REDIRECT );
knit-pay commented 1 year ago

@rvdsteege Sometimes API calls throw errors. If I call it in get_output_fields, we will not be able to show these error messages on the frontend checkout page. If we throw errors from the start function, it gets displayed on the checkout page. Secondly, action_url and other details are already saved even after the first call of the start function. Why do we have to call the start function again? Is there any specific reason for the same? https://github.com/pronamic/wp-pay-core/blob/0c4296428a2f31f542113cd81424c5ddd9f4a5d4/src/Plugin.php#L1213

knit-pay commented 1 year ago

@rvdsteege Do you need more information from my end on this?

rvdsteege commented 1 year ago

@remcotolsma thoughts?

knit-pay commented 1 year ago

@rvdsteege and @remcotolsma Thanks for understanding the issue and fixing it.