Sylius: [Admin] Order Item Subtotal wrongly calculated in order view

[peterukena]

Sylius version affected: 1.6.*

Description
When an order has order-level and order-item(-unit) level promotion adjustments applied to it, that end up having an uneven adjustment amount per unit, the current @SyliusAdminBundle/Resources/views/Order/Show/Summary/_item.html.twig template displays the wrong subtotal.

Steps to reproduce
Create two promotions.
1) No rules, fixed order discount 10 currency
2) No rules, fixed item discount 5 currency

Order something with an uneven amount of items. On my system, I had one item 5 times and a second one 2 times in my order. The Distributed order discount column shows one amount (which should be two, because it is distributed unevenly over the item units).

This can also be checked in the database with this query:

SELECT label, amount FROM sylius_adjustment WHERE order_item_unit_id IN (SELECT id FROM sylius_order_item_unit soiu WHERE order_item_id IN (your_two, item_ids_from_your_order));

Check for all amounts that correspond to your previously setup promotions and you should see they are uneven (this is just to confirm, uneven adjustments are correct here. F*ck rounding issues :wink:)

Possible Solution
In the mentioned template, line 11 is the culprit.

{% set subtotal = item.quantity * (item.unitPrice + item.units.first.adjustmentsTotal(unitPromotionAdjustment) + item.units.first.adjustmentsTotal(orderPromotionAdjustment)) %}

This only takes the first units' promotion adjustments into account when calculating the item subtotal.

A proposed fix (with some annotations from myself :wink: ) could look something like this:

{# This does not respect uneven distributed discounts from order-level #}
{# {% set subtotal = item.quantity * (item.unitPrice + item.units.first.adjustmentsTotal(unitPromotionAdjustment) + item.units.first.adjustmentsTotal(orderPromotionAdjustment)) %} #}

{% set aggregatedUnitPromotionAdjustments = 0 %}
{% for unit in item.units %}
    {% set aggregatedUnitPromotionAdjustments = aggregatedUnitPromotionAdjustments + unit.adjustmentsTotal(unitPromotionAdjustment) + unit.adjustmentsTotal(orderPromotionAdjustment) %}
{% endfor %}
{% set subtotal = (item.unitPrice * item.quantity) + aggregatedUnitPromotionAdjustments %}

{# Why not just use the item total here? #}
{# {% set subtotal = item.total %} #}

[CoderMaggie]

Hi Peter! :)

This is maths and the rounding issues. We've been fighting this last year and this is the best we've come up with. 🤷‍♀ It's not a calculation BTW it's twig representaion of the data, in backend if you go one by one on order item units it will be correct.

The promotions on uneven number of items will always be distributed unevenly because we have a limited rounding procedure, there is nothing we can do about it, and I've made an investigation on that and it's correct for most of the commerce world :)

[CoderMaggie]

https://github.com/Sylius/Sylius/pull/10417/

[peterukena]

I am sorry, but I disagree. This is not a rounding issue, as I pointed out. Its a plain error in the templates responsible for displaying the distributed discount per item.

The adjustments-table contains the correct amounts for each individual OrderItemUnit - and because they, depending on the promotion type, can be different per unit (to avoid rounding issues, actually), picking the amount from the first unit is just wrong. No fancy maths involved here ;)

An example:

An order with 10 items and a promotion of type (Fixed order amount: 11 currency)

This gives you 10 units and 10 adjustments of type order_promotion containing the distributed promotion amount per unit.
Since it does not match up evenly, 9 adjustments will be 1 currency and 1 adjustment will be 2 currency to end up with 11 currency of promotion.

In the database this looks like this:

• adjustment: Item 1, Amount 1 currency
• adjustment: Item 2, Amount 1 currency
• adjustment: Item 3, Amount 1 currency
• adjustment: Item 4, Amount 1 currency
• adjustment: Item 5, Amount 1 currency
• adjustment: Item 6, Amount 1 currency
• adjustment: Item 7, Amount 1 currency
• adjustment: Item 8, Amount 1 currency
• adjustment: Item 9, Amount 1 currency
• adjustment: Item 10, Amount 2 currency

As shown above, picking the first unit and using its adjustment amount and multiply it by the items quanity would yield you with 10 currency (Amount 1 times 10 quantity), instead of 11 currency.

Since in the database everything is correct and its just that template that is wrong, I`d like to propose the aforementioned fix. The math done so far is absolutely correct.
:slightly_smiling_face:

Also some screenshots:

This subtotal is 3cts too low, because of a 3ct too high discount calculated from picking the wrong adjustments.

With the applied fix, each individual units adjustment is taken, and the amount is correct.

One can clearly see, that 3 of the 5 unit adjustments are 1ct less, which should yield a 3ct higher subtotal, than with just using the first adjustment.

[peterukena]

Just to clarify:

Current solution: Subtotal is: quantity * (unitPrice + -153)
Correct solution: Subtotal is: (unitPrice * quantity) + -153 + -153 + -152 + -152 + -152

[vvasiloi]

@kortwotze or, instead of multiplying by quantity, just iterate through all order item units and add the unit price and adjustments total. I could be done in a method of the order item class, because calculating the subtotal is a common task in e-Commerce.

[peterukena]

I think its closer to business logic if we rely on the item quantity and the item.unitPrice for the base price of the item and then add up all the individual unit-promotions on that price.

Edit:
Also this should not calculate any subtotal from an e-commerce perspective. This should only be a view on the already persisted (and calculated) e-commerce data.