Add default tax class rate to the price index

[nicobatty]

Hi!

This PR is related to this issue: #571

This is an attempt at fixing the price index so that it includes the default store tax.

In order to recover this tax, I added the tax_class_id to the price index request to limit the performance impact.

I'm not yet sure about the $this->taxCalculation->getCalculatedRate call in terms of performance, though it seems like the rate recovery is cached:
https://github.com/magento/magento2/blob/50f974b0c8f3ada8d7a2c9b0f9924976dacd2088/app/code/Magento/Tax/Model/Calculation.php#L362-L378

[nicobatty]

@romainruaud I submitted this PR a while ago and I am wondering what you think about it.

Is there anything I have to fix?

[romainruaud]

Hello @nicobatty

to me it's quite unclear what you are trying to achieve here.

I see that you calculate on-the-fly the product prices according to their tax class id, but I do not understand the reason behind this.

Maybe you can give me more details about it. Also, it may be due to a custom tax configuration by your side, so would you mind posting a screenshot of your tax configuration in the Magento Back-Office ? I suppose there is something related with the way to display product price (incl or excl tax), and things like this.

Best regards

[nicobatty]

Hi @romainruaud,

Sure no problem I can explain this to you.

Basically, in Magento you have two configurations:

• There is a configuration to specify if the catalog price exclude or include the tax (the price set in the admin) : Stores > Configuration > Sales > Calculation Settings > Catalog Prices.
• There is a configuration to specify which country, state, post code to use as a default tax calculation for the store, so for example, in France we woud set it to France, *, * : Stores > Configuration > Sales > Default Tax Destination Calculation.

Magento by default is using the "Including Tax" for the "Catalog Prices" config, in this case the catalog_product_index_price table contains the prices including the tax but when is it set to "Excluding Tax", it will contains the price excluding the tax.

the issue is that the Price indexer for ElasticSuite is taking the raw price in the catalog_product_index_price which is not the actual displayed price on the frontend if you take the Default Tax Destination Calculation setting into consideration.

It might be easier to explain with an example:

1. I set the "Catalog Prices" config to "Excluding Tax".
2. I set the "Default destination" config to France, *, *.
3. I set the "Display Product Prices In Catalog" config to "Including Tax".
4. I create a new tax class Apparel with 20% tax for France, *, *
5. I create a product with a tax class of Apparel and a price of 20.
6. We can see the product on our store appears with a price of 24€.
7. If we check on the catalog_product_index_price table, the price is set at 20 (untaxed).

This is what Magento does basically.

The issue is that the current implementation of the price indexer for ElasticSuite is taking the raw catalog_product_index_price information, so in this example, it will set the product price as 20 in the index, which is wrong since the price displayed on the store is actually 24.

You can also check the needPriceConversion function I added as a condition. This function is called by Magento when it tries to get the product tax price which check two configurations, if the price include the tax (the Catalog Prices config) and the display price (if the displayed price include of not the tax, or both).

[cmuench]

@nicobatty @romainruaud We have the same problem here in Germany. Looks like a good idea to index the prices with tax if possible for France and Germany.

[kevinvuillemin]

Hello,

@nicobatty Did you find au solution please ? We have this problem too with multi taxes.

#1489

[nicobatty]

Hi @kevinvuillemin,

I no longer work on Magento projects but this PR should do the trick.

At least it used to work back when I did this change and it was used on a multistore / multitax website that was doing B2C and B2B.

All it really does is add the tax_class_id to the request for the regular price, recover the tax rate and calculate the taxed price.

I think you are probably better off patching these changes since it will probably never going to be merged.

The use case is specific to those using untaxed price by default and @romainruaud don't like the fact that the price is being recalculate here. The problem is, it is not calculate anywhere otherwise we wouldn't be using the price index as a reference.

I explained the issue in detail on this PR so if you want to patch it in an other way you are free to do it.