silvershop/silvershop-core

Order processor should not re-calculate order totals after payment

christopherbolt opened this issue · 4 comments

Currently the order totals are processed before redirecting to the gateway and then again afterwards.
They should not be processed again afterwards because this causes a significant issue with some offsite payment gateways such as Payment Express PxPay which notify the site of the successful payment in the background, so not part of the current user session. Consider the scenario where the site uses the SilverShop/discounts SpecificPricingExtension which allows specific prices top be set for different user groups: currently when the gateway notifies the site of a successful payment the order totals get re-calculated again but because this notification is not part of the user session there is no logged in user so the user specific discounts don't get factored in and so the order item prices and totals are wrong and the order is left with an amount outstanding.
There shouldn't be any need to recalculate the order totals again at this point as they were already calculated correctly before redirecting to the gateway.

wilr commented

Makes sense. PR welcome if you can track down where the total is recalculated post gateway redirect. Order::Total() should be used rather than calculating.

I've finally had a chance to look at this.
The solution is to simple; just remove all calls to $this->order->calculate() from SilverShop\Checkout\OrderProcessor.php
This is found in two functions placeorder and completePayment
Lines 220 and 288

Removing calls to $this->order->calculate() from the order processor should have no impact on functionality because the order totals have already been calculated at this point.
I will make a PR as soon as I have time

Have created a pull request: #720

wilr commented

Merged.