umbraco / Umbraco.Commerce.Issues

17 stars 2 forks source link

New orderNumber is generated everytime payment is initiated #309

Open MichaelNielsenDK opened 3 years ago

MichaelNielsenDK commented 3 years ago

Whenever a client goes to the payment gateway, a new orderNumber is generated.

This can cause issues, where clients pay on the "wrong" orderNumber, and will give the same transationId but different orderNumbers in Vendr and in the Payment Gateway.

When going through checkout, on the final step before payment, CTRL click on the button that sends you to the payment, so it will open in a new tab.

Here I've CTRL clicked twice, and opened two new tabs https://prnt.sc/15h3z00

The order in Vendr will now have the orderNumber from window 2, but if I go through payment in window 1, the gateway will get the orderNumber from window 1.

The payment will go through and the callback will work, as there is no check on the orderNumber property in the callback. The transactionId will be added to the order.

My suggestion is, that when an orderNumber has been generated on an order, this never changes again.

mattbrailsford commented 3 years ago

@MichaelNielsenDK unfortunately this is not possible. There are some payment gateways that cannot process the same order number more than once and so a customer canceling out of a payment window and changing the order and coming back to pay for the now updated order requires that orders are given a new order number at this point as the value has now changed from the previous value to the payment gateway had.

RE the callback, there should definitely be a check in the in this for the order number as all callback URL's passed to the gateway consist of the orders ID and Order Number and these are checked to make sure they match the order before continuing. What payment method are you using? Is it custom or one of the core/community ones?

MichaelNielsenDK commented 3 years ago

@mattbrailsford

All gateways I've worked with, does not accept the same ordernumber on different payments, so while that is true, then I don't think that is an issue here.

The gateway does not save the ordernumber, until the payment has been completed, so initiating multiple payments with the same ordernumber is fine, as long as no payment has been completed on that ordernumber.

All the other commerce systems we work with does this, and there are no issues with it.

mattbrailsford commented 3 years ago

@MichaelNielsenDK the issue is, gateways are sent details of the payment coming when you render the payment form usually on the preview step and some gateways don't allow the price to be changed and updated for this once they have received it so if a customer went to the payment gateway then realized they missed something (so haven't completed the payment yet) goes back and adds a new item, this needs to trigger a new order number and is what we are doing.

Whilst the gateways you've dealt with might be ok with re-sending the same order number with a new price, SOME aren't and so as a platform that can work with many gateways we have to support that behavior and so resetting the order number if the order is changed.

MichaelNielsenDK commented 3 years ago

@mattbrailsford

Do you have examples of gateways where this is the case?

Because that would mean the other commerce platforms we work with, would not work with those gateways, and I would very much like to test that out.

But this issue will result in the orders cannot be captured in Vendr, or as in my case, from Microsoft NAV, as the orderNumber and transactionId does not match.

mattbrailsford commented 3 years ago

@MichaelNielsenDK I don't, these are just lessons passed down to us from Tea Solutions.

Regardless of the order number changing though, these SHOULD be validated when you are capturing the payment via the payment method callback URLs, so I'm confused why you are seeing the capturing orders with miss-matching order numbers?

The structure of payment provider URL's are:

/vendr/payment/{action}/{paymentProviderAlias}/{orderId}/{orderNumber}

Where these are validated against the order before continuing with the callback so it shouldn't be right that an order which was updated with an order number in window 2 should be able to finalize an order with the order number in window 1. Window 2 should update the order with the new order number and window 1 should fail because the URL references an order that now no longer matches.

MichaelNielsenDK commented 3 years ago

@mattbrailsford

Create Payment I don't think theres any validation on the ordernumber in the callback when a payment is created, but that's all handled by Vendr anyway, so you probably know that better than me.

I'm guessing the order is found via the id in the callback, not the ordernumber, and then transactionId is set on the order.

Perhaps an alternative solution could be to check if the ordernumber recieved in the callback, is the same as the one found on the order, and if the payment is authorized, then change it?

Capture Payment I'm guessing the reason they cannot be captured is exactly because the validation won't allow it.

mattbrailsford commented 3 years ago

Ok, so I think this could be our problem, we do validate the order reference (which is the order id + order number combo) matches the persisted order and if it doesn't match, we update the order status to be errored, but it does appear we still allow the callback method to continue to run for some reason. This should really be wrapped in an else.

image

MichaelNielsenDK commented 3 years ago

I just feel though, that erroring the order is not the best solution.

Take an end-users perspective, they've gone through the checkout and payment without any errors, landed on the receipt page and recieved a confirmation email, and then their order hasn't gone through.

Now the shop owner has to contact them, explain that something happened and send them a payment link or have them order again.

But that's just my opinion šŸ¤·ā€ā™‚ļøšŸ˜Š

I'll keep an eye out for the fix, whichever solution it will be.

Thanks šŸ‘

mattbrailsford commented 3 years ago

@MichaelNielsenDK sure, but I'm not sure what else can be done. Most payment gateways finalize the order async, so there is no way of notifying the User of anything until that happens and then it's occurring outside of their current request, so the only thing you can do is show the continue screen. Really, you should probably not show "order confirmed" on that screen and instead should probably say something like "Your order is processing, we'll let you know if everything goes to plan".

MichaelNielsenDK commented 3 years ago

@mattbrailsford

Ok, so we don't show "order confirmed" on a receipt page, we do have to inform the customer that the order has been recieved, that's just standard ecommerce practice.

We definitely cannot tell them that we're not sure about it, shop owners would be buried in requests whether customers orders has been received or not. šŸ˜…

I'll just have to inform the client, that this error is just something that will happen now and again, and there isn't anything we can do about it,

But from my point of view, it should not even be possible for this error to occur, and not changing the order number would be the best solution. I've found 10 orders out of the last 3000 orders, so while not often, it does occur from time to time.

I understand your reluctance, but you're not aware of any gateways where it's an issue, and other platforms, like Ucommerce, who supports a wide range of gateways also don't change the ordernumber.

I can't force you to do anything, I can only offer up my opinion. šŸ¤·ā€ā™‚ļøšŸ˜Š

mattbrailsford commented 3 years ago

@MichaelNielsenDK and for that I'm extremely grateful šŸ™

It's defo something I'll consider further and maybe talk to Tea Solutions on who they know had this problem.

RE the 10 orders you've found like this though, I do still wonder what happening to cause the order number to change as this should only happen if the order is modified after they are sent to the payment gateway šŸ¤”

MichaelNielsenDK commented 3 years ago

@mattbrailsford

Well thanks for that, I now understand what is going on. šŸ˜Š

If I look at the Vendr demo store checkout, I can see the following steps: Cart -> Information -> Shipping Method -> Payment Method -> Review -> Pay -> Confirmation

But we don't want users to go through all those steps. Mostly there's a only few steps, but in this specific case, it's a one-page-checkout.

But the common thing is, the last page the user sees before payment, is the page where they enter their information. After submitting this form, the user is sent to the payment gateway, which modifies the order, and that is why the ordernumber changes.

So I guess our options are:

mattbrailsford commented 3 years ago

@MichaelNielsenDK you can do the same as we do on the Vendr site and show a page that says "Redirecting to payment gateway" and on that page you render the payment form and have it auto submit using some javascript.

<h2>Redirecting to Payment Gateway</h2>
@using (Html.BeginPaymentForm(Model.Order))
{
    <p>If you are not redirected in the next 10 seconds, please click the button below.</p>
    <button type="submit">Continue</button>
}
<script>
    setTimeout(function() {
        var form = document.forms[0]; // Assumes only one form on the page
        if (form.hasAttribute("onsubmit")) {
            form.dispatchEvent(new Event('submit', { cancelable: true }));
        } else {
            form.submit();
        }
    }, 100)
</script>

If you want to get fancier, we've also done it before where that auto redirecting page also gets loaded into a hidden iframe if you don't want to show it.

MichaelNielsenDK commented 3 years ago

@mattbrailsford

We actually already do that, I just didn't mention it to further complicate things.

But that doesn't stop the ordernumber from changing, as the user still submits the information form to get to the auto-redirect, before being sent to the payment gateway.

mattbrailsford commented 3 years ago

Then Iā€™m still confused how are your customers editing the order after the order number is generated? BeginPaymentForm is what sets the order number so if this is only rendered on the redirect page, what is happening that is causing the order to be edited again?

MichaelNielsenDK commented 3 years ago

Because to go to the redirect page where BeginPaymentForm is on, you'll have to submit the address information form, which modifies the order, which then changes the ordernumber.

I've tested it out, and on the checkout page, I CTRL click twice on the button here https://prnt.sc/15lrg9f

Which opens two tabs, and I've disabled the auto-redirect, and then I can see, there's a different ordernumber in the two tabs https://prnt.sc/15ls9r5

mattbrailsford commented 3 years ago

I'm still not 100% here.

So I'll be clear. BeginPaymentForm should only be called once just before your redirect your customer to the payment page. So in my example snippet above, this is the page that shows the redirecting message, or if you follow Vendr's demo store example, it's rendered on the review step where there is nothing to edit.

If you are just updating order details, you should be using your own controller to update the order, and prior to actually going to the payment gateway, you shouldn't be using BeginPaymentForm.

If the above is followed, then there really shouldn't be a way for customers to alter the order in a way that creates a new orde rnumber prior to being sent to the payment gateway.

Now yes, you could be on the customer details page and CTRL click the "continue to payment" button and this would open multiple windows that would trigger the rendering of BeginPaymentForm multiple times which would cause multiple order numbers to be created, but why are your customers doing this?

If it is something like the later, and it's just an edge case because users are doing weird stuff, I can probably look to try and do something whereby calling BeginPaymentForm caches the output and clears it if the order is re-calculated such that, if the customer tries to go to the payment gateway for the same order for the same amount, then just use the same response. But I can't tell if it is an edge case, or there is some legitimate reason your customers need to do this?

MichaelNielsenDK commented 3 years ago

So I'll be clear. BeginPaymentForm should only be called once just before your redirect your customer to the payment page. So in my example snippet above, this is the page that shows the redirecting message, or if you follow Vendr's demo store example, it's rendered on the review step where there is nothing to edit.

And as mentioned, this is exactly the way it is

If you are just updating order details, you should be using your own controller to update the order, and prior to actually going to the payment gateway, you shouldn't be using BeginPaymentForm.

We're definitely not using BeginPaymentForm to update the order.

Now yes, you could be on the customer details page and CTRL click the "continue to payment" button and this would open multiple windows that would trigger the rendering of BeginPaymentForm multiple times which would cause multiple order numbers to be created, but why are your customers doing this?

Beats me šŸ˜… If I should speculate, then they might have multiple tabs open, to look at several products. Perhaps when getting to the payment the first time, they need to go get their credit card, get's sidetracked, and a little later open up the payment again in a new tab or window šŸ¤·ā€ā™‚ļø

If it is something like the later, and it's just an edge case because users are doing weird stuff, I can probably look to try and do something whereby calling BeginPaymentForm caches the output and clears it if the order is re-calculated such that, if the customer tries to go to the payment gateway for the same order for the same amount, then just use the same response. But I can't tell if it is an edge case, or there is some legitimate reason your customers need to do this?

They definitely don't need to do this, there's no reason they should do this, but some do, creating an annoying error.

mattbrailsford commented 3 years ago

Ok, great. At least I understand it is an edge case now šŸ‘

Let me give it some more thought šŸ¤”

MichelCollard commented 3 years ago

I think we have a similar issue with our webshop. About 6 months ago the first issue was: https://our.umbraco.com/packages/website-utilities/vendr/vendr-support/104622-value-can-not-be-null-param-name-ordernumber Even after some time(and updates of course) we will occasionally receive paid orders that won't show up in Vendr.