Bug: Bank Transfer payment redirect loop

[lenvanessen]

Hi,

We've found a bug in the plug-in. You can't order when using Mollie's Bank Transfer payment. What happens if the payment status remains open (because it's a bank transfer this is normal, mollies webhook will update the site when the payment was received), you keep going back tot the site, which redirects you back to Mollie.

Normally when the payment is open, this would be fine, but an exception must be made for the bank transfer, since the user doesn't directly pay like with CC or iDeal.

[lenvanessen]

@lukeholder any fix for this?

[alexthn]

Any update on this? We have customers getting quite confused because of this redirect-loop.

Basically it would be if method == "banktransfer" and status == "open" then payment = pending.
When pending redirect to special thank you page or thank you page with some additional logic to show "thank you for your order, once your payment has been received we will process it".

[rbrv]

Following closely as well. We've nearly completed a migration of an existing shop to Craft Commerce 3.
Payment by bank Transfer is a must for this shop, as 20% of all orders on the current site are using this payment option.

[alexthn]

Any update on this issue?
Or maybe anyone that has a workaround?

[lenvanessen]

@lukeholder @brandonkelly can this get addressed? Multiple clients have issues with this, and it's a really low-hanging fruit to solve.

[nfourtythree]

Hi All,

I think the issue here with the implementation is how and when Mollie calls the webhook (please correct me if I have any information wrong).

Stepping through the Mollie payment process for a bank transfer there seems to be three payment states open, paid and expired.

At the moment it seems like we handle paid and expired correctly. paid the order completes customer is returned to the site. expired customer would in theory "timeout" and remind on the mollie side of things, a webhook is then sent and a failed transaction is registered.

That leaves open. According to the Mollie documentation the open status does not call the webhook.

The payment has been created, but nothing else has happened yet. This is not a status Mollie will call your webhook for.

Commerce, therefore, has no knowledge of what has happened to this order/transaction. Unfortunately, I am only able to check this with the test suite. If anyone can enlighten me on what happens in a live situation that would be great (e.g. does the user get returned, where do they end up, what endpoints get callled etc).

It seems like this is a situation where an order needs to be able to be completed without any form of payment. To do that we do require some kind of return from Mollie so we have some information to work with.

Thanks.

[alexthn]

Hi Nathaniel,

I checked and in real life the customer is given a screen where they are given the information to make the transfer, also they can enter their email address to receive the instructions by email.
After they click the "Continue to website" button they are returned to the craft on the redirectUrl given by the plugin.
I also took a look at the webhook but it indeed seems that Mollie only calls this on expiry and when the payment has been received. No call for open status on the webhook.

As far as i can see this is also where the problem is. Because they go to the redirectUrl (index.php?p=actions/commerce/payments/complete-payment&commerceTransactionHash=???) craft checks the status and sees its still open but doesn't know the payment is pending.

Maybe the easiest fix would be to have craft set a different redirectUrl if bank-transfer is selected so that when the customer returns, craft knows the customer is coming back from Mollie and should show a "thank you, order will be processed after the payment is received" page instead of redirecting back to Mollie.

Hope this helps, if you require more info please let us know.
Thanks for picking up on this.

Alex

[nfourtythree]

Hi @alexthn

Thank you for the information you sent across it was really helpful in being able to determine what was happening here. Especially in the live environment.

We have just pushed up a commit that looks to make an update to the functionality around bank transfers.

The significant change here is that "open" bank transfers being routed back to the complete payment process will now have a transaction status of processing. This means these orders will now be completed during that process (with them obviously being marked as unpaid).

This change shouldn't see any effects on any other payment types.

It would be great if people are willing to do some testing with the updated code, especially when attached to a live instance of Mollie and let us know that everything is working as expected.

To get this code, you can change your craftcms/commerce-mollie requirement in your project's composer.json to:

"require": {
  "craftcms/commerce-mollie": "dev-develop#ec91465e23f163bd123bf9cecd5c4e789c8950a5 as 2.1.2.1",
  "...": "..."
}

Then run composer update.

Please let me know how everything goes and we can look at getting an official release for these changes.

[alexthn]

Hi @nfourtythree

Thank you for the work.

In my initial testing it seems to be working fine,
One thing I wonder about is what happens/should happen if a banktransfer expires or gets cancelled. I know Mollie calls the webhook to inform about the expire/cancellation but this doesn't change the order. It basically stays as a unpaid-completed-order, is this the desired behavior?

In our case we have multiple order statuses and the orders move from one to the other depending on the state (mostly automatically). It would become a little unclear for our people if these stay as "new" (our initial/default status) for ever. Normally our goal is to have the new list empty, so we know every order is being worked on.

Also for further testing we first have to make some adjustments to our emails and thank-you pages to show the correct things (like, once we receive your payment your order will be processed). After we do these changes we can really do real live-testing and get the results but its looking promising!

Please let me know your thoughts about the expire point.

[nfourtythree]

Hi @alexthn

Thank you for taking the time to do some testing.

It terms of the order staying completed but unpaid, this is expected behaviour when allowing orders with "processing" transactions to complete. Especially as you could in theory make a payment on this order at any time. This is probably a little more confusing because of the semantics in Commerce. If you were to swap the idea of "completed" to "received" this would make a little more sense. This is of course possible with the use of custom order statuses.

We are also looking into the idea of order states for future versions of Commerce which could allow for a more obvious overview of where an order is in its lifecycle.

If you are already changing order statuses automatically then there is probably an addition to be made with these types of orders and transactions.

Using the Transactions::EVENT_AFTER_SAVE_TRANSACTION event you can check the transaction's status as well as Mollie's transaction information (found in the transaction's response attribute) to see if it is a bank transfer and its status (you would be looking at the method and status keys in the response). The order can then be moved into a different order status. This would also give you the ability to trigger emails specific to those situations.

As a side note, there is also the Order::EVENT_AFTER_ORDER_PAID event that could be used but it seems like it is the bank transfer side of things you are more concerned with.

As you mentioned, it probably requires a little reworking of your order completion process (emails, thank you pages and order statuses) due to now being able to have complete orders that have yet to be paid for.

Hope this makes sense, let me know how you get on.

Thanks!

[rbrv]

@nfourtythree I'm having some serious reservations about the proposed appoach.

The current situation is simple and elegant and easy to explain:

• A cart is converted to an order ONLY AFTER the actual payment has been done
• This is always the case, for ALL payment methods, even bank transfers

Changing this behaviour as proposed above will break backwards compatibility and more complexity.

Everything is working fine at the moment, with mollie correctly triggering the webhook when the bank transfer has been completed, and at that moment converting the cart to an actual order.

Because a cart with a pending bank transfer is not an actual order yet, it will disappear automatically when expired. So no need for extra statuses and more complexity.

What would be nice, is the possibity to define the target of the "back to website" button on mollie payment page.

[nfourtythree]

Hi @rbrv

Thank you for your feedback. We really appreciate it.

I definitely understand your reservation behind the implementation of this behaviour, which is why we wanted people to discuss it before folding the functionality into a release.

One thing is that we are certainly open to the idea of having this functionality enabled based on a config setting in the plugin. We would default this to be disabled, therefore, keeping the same behaviour as currently exists.

Obviously having it disabled would leave your project in the same situation as you have now which is open bank transfer payments leave the order as a cart. And as it currently stands a weird redirect loop back to the payment page of your project.

This brings us on to the "back to website" button. This is currently using the complete payment action URL, are your thoughts that you would like to customise that so you could send them back to a different page, leaving their order as a cart but maybe displaying a message or something?

Thanks

[jerome2710]

After checking the changes of the major version update from 2.1.2.1 to 3.0.0, I stumbled across this issue and corresponding commit ec91465. I agree with @rbrv having serious reservations with this approach, although I do understand @nfourtythree as well that it is quite difficult to properly handle the way bank transfers work.

Nevertheless, I would strongly suggest marking 3.0.0 as a breaking change, as I do believe this might cause business logic issues for users implementing bank transfers before and not properly checking the changes before updating.

[rbrv]

@nfourtythree I see this functionality is implemented in 3.0.0.

You mentioned earlier, there would be config setting for keeping the old behaviour. Which config setting is this?

[rbrv]

@nfourtythree Any update about the config setting?