New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Discount Price Rule cant be specified as gross or net #3544

Open
NiklasBr opened this Issue Oct 26, 2018 · 11 comments

Comments

Projects
None yet
2 participants
@NiklasBr
Contributor

NiklasBr commented Oct 26, 2018

Bug Report

Expected behaviour

The Pricing Rule Cart discount applies is applied inconsistently with regards to gross/net prices and other Price Modifications.

Actual behaviour

A Cart Discount currently does not allow us to specify if the "Absolute discount" is pre-tax or post-tax. It also applies to non-cart items such as Shipping and other price modifications.

Steps to reproduce

  1. Add an item with a cost, let's say 200 to the cart, with a 25% tax class, result: Cart now says GrandTotal is 250, as it should be.
  2. Let Pimcore add Shipping for 100, with a 25% tax class like Code example, result: Cart now says GrandTotal is 375, as it should be.
  3. Add an active Pricing Rule with the following action: Absolute Discount: 100, result: Cart now says GrandTotal is 275, but it should have been calculated like this: ((200-100)*1,25) + (100*1,25) = 250.

Code example

$shipping = new Shipping();
$shipping->setTaxClass($taxClass); // 25%
$shipping->setCharge(100); // actually a Decimal object, but simplified in this example
$cart->getPriceCalculator()->addModificator($shipping);
@fashxp

This comment has been minimized.

Member

fashxp commented Oct 29, 2018

hmm, I cannot really follow you.
most important: all amounts of pricing rules and price modifications that are shipped with the framework are gross amounts - which is maybe your issue here. see also docs here.
so when you call setCharge as you did in your example above, you will set the gross amount (=100) and the net amount will be calculated accordingly (=80 when you have 25% taxes).

when I try to reconstruct your sample I get following results:

  1. adding product with net price 200 and 25% tax to the cart results in grand total net amount 200 and gross amount 250.
  2. adding shipping costs (gross amount 100) as in your example results in grand total net amount 280 and grand total gross amount 350. (I don't get 375 as you above)
  3. add active price rule with absolute discount 100 (which is gross amount) results in grand total net amount 200 and grand total gross amount 250.
  4. remove rule from 3.) and add another price rule with relative discount 10% results in grand total net amout 260 and grand total gross amount 325 (which is off 10% of gross subtotal 250, additional shipping costs are not included in calculation of percentage).

so, what is what you expect should be different?

@NiklasBr

This comment has been minimized.

Contributor

NiklasBr commented Oct 30, 2018

I expect all amounts to actually be gross, as the docs claims, but they act as if they are not.

If I specify an amount for a Product at 100, shipping with 50, then a discount of 10 I expect all these amounts to modify the gross amount in the grand total calculation. But they do not as can be seen in this example:

  1. Create a product with a 100 amount, set a tax class object at 25% to it, add 10 of it to your cart.
  2. Create a Shipping object with a 200 amount, set a tax class of 25% to it, add it to your cart.
  3. Create a Pricing Rule with an absolute product discount of 100 (no tax class possible). This should result in effectively free products.

This should result in only the shipping plus the tax on shipping (200+50) being in the grand total, but the grand total is calculated as 500, which makes no sense.

Calculation should be ((10*100)-(10*100))*1.25 + (200*1.25) = 250 but instead it applies the pricing rule to the net amount like this: (10*100*1.25)-(10*100) + (200*1.25) = 500

@fashxp

This comment has been minimized.

Member

fashxp commented Oct 30, 2018

maybe I'm wrong, but gross price is (net price + tax).

so, if your product has net price 100 + 25% taxes and you add 10 of it to a cart
=> grand total net will be (10*100) = 1000
=> grand total gross will be (10*100)*1.25 = 1250

if you add shipping with gross value 200, its net value will be calculated automatically as 160 resulting in
=> grand total net will be (10*100) + 160 = 1160
=> grand total gross will be (10*100)*1.25 + 160 * 1.25 = 1450

if you set a product discount of 100, this is always a gross discount which is applied to the gross amount of product price. net parts are calculated automatically.
=> grand total net will be (10*100) - (10*80) + 160 = 360
=> grand total gross will be (10*100)*1.25 - (10 * 100) + 160 * 1.25 = 450

if you want to define free products with product pricing rules, you need to define gross price of product as discount (which is always considered as a gross amount).

here is the place, where product price rules are applied. as you see, calculation always takes place on gross amounts.

these calculations I also just tested and they happen as stated above. maybe you changed something in your setup?

@NiklasBr

This comment has been minimized.

Contributor

NiklasBr commented Oct 30, 2018

An experiment to show the discrepancy, let's add 100 discount for every item in the cart:

foreach ($cart->getItems() as $item) {
    $discountAmounts[] = -100;
}
$discountAmount= Decimal::create(array_sum($discountAmounts));
foreach ($priceCalculator->getModificators() as &$modificator) {
    if ($modificator instanceof Discount && $modificator->getRuleId() == $environment->getRule()->getId()) {
        $modificator->setAmount($discountAmount);
        $priceCalculator->reset();
        return $this;
    }
}
  1. Add 1 item to the cart, let's say with the price 100, and a 25% tax class.
  2. Add 1 Shipping to the cart, let's say with the price 50, and a 25% tax class.
$shipping = new Shipping();
$shipping->setTaxClass($taxClass); // is a \Pimcore\Model\DataObject\OnlineShopTaxClass
$shipping->setCharge(Decimal::create(50));
$priceCalculator = $cart->getPriceCalculator();
$priceCalculator->addModificator();
$priceCalculator->reset();

As your own post say it should be like this:
Net: ((1 * 100) - (1 * 100)) + (1 * 50) = 50
Gross: (1 * 100 * 1.25) + (1 * 50 * 1.25) - (1 * 100) = 87.5

But as I showed above, that is not happening.

@fashxp

This comment has been minimized.

Member

fashxp commented Oct 30, 2018

I don't get what you are trying to do with your first code block. This is certainly not a usage as it is supposed to use.

But anyway, again: modificators always work with gross values. So when you are calling $modificator->setAmount($discountAmount); and also $shipping->setCharge(Decimal::create(50)); you are setting the gross amount.

So calculation will always be like:
Gross: (1 * 100 * 1.25) + (1 * 50) - (1 * 100) = 75

Net values are calculated based on that and will be:
Net: (1 * 100 * 1.25) + (1 * 40) - (1 * 80) = 60

Maybe I still cannot follow you. Could you create a unit test case and outline your expected behavior so we execute the same piece of code?

@NiklasBr

This comment has been minimized.

Contributor

NiklasBr commented Oct 31, 2018

I don't get what you are trying to do with your first code block. This is certainly not a usage as it is supposed to use.

I'm trying to get the Discount Price Rule to work, because it does not work otherwise (it is a frustrating lack of clarity in the documentation that led me to create that code block :().

@fashxp

This comment has been minimized.

Member

fashxp commented Oct 31, 2018

there is a product discount action that can easily configured in admin ui
image

@NiklasBr

This comment has been minimized.

Contributor

NiklasBr commented Oct 31, 2018

But anyway, again: modificators always work with gross values.

Yes, and I am trying to add the gross value, but the price calculation of the grand total is incorrect.

@NiklasBr

This comment has been minimized.

Contributor

NiklasBr commented Oct 31, 2018

there is a product discount action that can easily configured in admin ui

Yes, but it does nothing unless I add the above code to the Cart Controller, no discount at all, with that code at least it gets added.

@fashxp

This comment has been minimized.

Member

fashxp commented Oct 31, 2018

there is a product discount action that can easily configured in admin ui

Yes, but it does nothing unless I add the above code to the Cart Controller.

cannot reproduce that. it works as expected in our environment ...

As I asked before - would be great if you can create a unit test that reproduces your wrong behavior, then we can execute the same code and discuss about it. My feeling is, that we are talking at cross purposes.

@NiklasBr

This comment has been minimized.

Contributor

NiklasBr commented Oct 31, 2018

In cart Controller, this is the code that gets the grand total

public function getTotals(ICart $cart) :array
    {
        $grandTotal = $cart->getPriceCalculator()->getGrandTotal();
        $totals = [];

        foreach ($cart->getPriceCalculator()->getPriceModifications() as $name => $modificator) {
            switch ($name) {
                case 'shipping':
                    $weight = $this->sum($this->getShippingWeightsSummary($cart))->asNumeric();
                    $totals[] = [
                        'type' => 'net',
                        'label' => sprintf("Shipping (%s)", WeightFormatter::format($weight)),
                        'value' => $this->formatDecimalFromPrice($modificator->getNetAmount(), $grandTotal),
                        'class' => "shipping",
                        'sort' => -10,
                    ];
                    break;

                default:
                    if (!$modificator->getGrossAmount()->isZero()) {
                        $totals[] = [
                            'type' => 'net',
                            'label' => $modificator->getDescription(),
                            'value' => $this->formatDecimalFromPrice($modificator->getNetAmount(), $grandTotal),
                            'class' => $name,
                            'sort' => 0,
                        ];
                    }
                    break;
            }
        }

        $totals[] = [
            'type' => 'net',
            'label' => 'Sum excl VAT',
            'value' => $this->formatPrice($grandTotal, true),
            'class' => "net",
            'sort' => 10,
        ];

        foreach ($grandTotal->getTaxEntries() as $key => $entry) {
            $totals[] = [
                'type' => 'tax',
                'label' => $key,
                'value' => $this->formatDecimalFromPrice($entry->getAmount(), $grandTotal),
                'class' => "tax",
                'sort' => 20,
            ];
        }

        $totals[] = [
            'type' => 'gross',
            'label' => 'Sum inc VAT',
            'value' => $this->formatPrice($grandTotal),
            'class' => "sum",
            'sort' => 100,
        ];

        usort($totals, function($a, $b) {
            return $a['sort'] - $b['sort'];
        });

        return $totals;
    }

    public function formatPrice(IPrice $price, bool $net = false): string
    {
        $currency = $price->getCurrency();
        $amount = $net ? $price->getNetAmount() : $price->getGrossAmount();
        return $currency->toCurrency($amount->asNumeric(), self::PRICE_FORMAT);
    }

    public function formatDecimalFromPrice(Decimal $decimal, IPrice $price): string
    {
        $currency = $price->getCurrency();
        return $currency->toCurrency($decimal->asNumeric(), self::PRICE_FORMAT);
    }
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment