VM 3.6.8, back end editing the quantity of articles in orders -> wrong discount

[ibauer]

Hello,

I've found the following problem VM3.6.8 10197:

When editing the quantity of articles in an existing order in the backend the itemDiscount get's calculated wrong:

I searched in the sources and found in the orders model the function calculateRow(). Right at the start of this function the itemDiscount gets calculated by dividing the product_subtotal_discount by the quantity.
Well in that situation the product_subtotal_discount is still the one calculated before and so it's the one calc. with "old" quantity, but the quantity in the request data is the changed "new" one. So the discount gets wrong and all following calculcation too.

I hot fixed this by calculating the itemDiscount that way: product_priceWithoutTax - product_discountedPriceWithoutTax. But I'm not sure if this really works for all circumstances.

Thanks,
Ingmar

[Milbo]

Thank you for writing this bug post.

I think I found a quite okey solution, around line 679

Code Select Expand


if(empty($data['product_subtotal_discount'])){
$data['product_subtotal_discount'] = 0.0;
} else {
$itemDiscount = $data['product_subtotal_discount'];

if($itemDiscount<0.0){
$itemDiscount = $itemDiscount * (-1);
}

if($daTax and VirtueMartModelOrders::isNotEmptyDec($data,'product_basePriceWithTax') and VirtueMartModelOrders::isNotEmptyDec($data,'product_final_price')){
$itemDiscount = $data['product_basePriceWithTax'] - $data['product_final_price'];
} else if(!$daTax and VirtueMartModelOrders::isEmptyDec($data,'product_subtotal_discount') and VirtueMartModelOrders::isNotEmptyDec($data,'product_final_price')){
$itemDiscount = round($data['product_item_price'] - $data['product_final_price'] + $data['product_final_price'] * $taxValue/(100 + $taxValue), $rounding);
} else {
$itemDiscount = $itemDiscount/$quantity;
}
}


The problem is, that it overwrites now the discount. The correct solution would be a dropdown with a discount to select, and a textfield with an override value. The thing is, atm we have only a value.

But there is at trick, so you can still set a discount and let vm calculate the baseprice. Just delete the baseprice with tax.

[ibauer]

Tanks for your quick response!

I tried the code you suggested and it generally works. But I've a little special case where an article is discounted by 100% for some customer groups.
Due to that special thing I needed to adjust your code in the following way:

Code Select Expand


                      if(($daTax and
                            VirtueMartModelOrders::isNotEmptyDec($data,'product_basePriceWithTax') and
                            VirtueMartModelOrders::isNotEmptyDec($data,'product_final_price')) ||
                           
                            ($daTax && isset($data['product_final_price']) && is_numeric($data['product_final_price']))) {

$itemDiscount = $data['product_basePriceWithTax'] - $data['product_final_price'];
} else if(!$daTax and VirtueMartModelOrders::isEmptyDec($data,'product_subtotal_discount') and VirtueMartModelOrders::isNotEmptyDec($data,'product_final_price')){

$itemDiscount = round($data['product_item_price'] - $data['product_final_price'] + $data['product_final_price'] * $taxValue/(100 + $taxValue), $rounding);
} else {
$itemDiscount = $itemDiscount/$quantity;
}


Guess it's not the best way, but for me product_final_price is 0 (overwritten price in article for some customers), so isNotEmptyDec($data, 'product_final_price') will always return false.
So the else - branch is executed instead of the first one. Now I checked only if it's generally set and if it's a numeric value. With that change everyting works fine now.

You're right the discount gets overwritten now when saving, but in my case that's exactly the expected behaviour. The user simply changes the amount of articles and wants the discount per article not to be changed.
Or do I miss some aspect of this?

[Milbo]

No, any aspect checked.

On the long run it would be cooler to have a rule to select, then the rule is used, or to use the set value.

[ibauer]

Ok, so thanks for the help!
Do you think that change could be merged in the next update? For now I'll keep my change, but with next update it would be gone.

[Milbo]

It will be in the next release. The discount can still be set extra, and the set discount is taken. In this case the editor must just empty some values, for example the final price, so vm knows it should use the given discount and recalculate the final price.

[ibauer]

Thanks for including this in the current release!