Incorrect calculation of tax

I wonder, are you using a Redis (or non-SQL) cache for the cache_field bin?

Thank you for your reply. No, we don't use any of the above.

I did some further debugging. It happens that the issue is significantly more complicated than I originally thought, but I still believe that it's Commerce Stock Reserve that needs to be patched. The bug results from incorrect interactions among four different modules: Commerce Stock Reserve, Commerce Product Attributes, Commerce Product Pricing, Commerce Tax. The last two are submodules of Commerce. The following happens:

1. A view is generated to display the line items in the cart of the user.
2. Commerce Product Attributes is a module that allows displaying product attributes within the aforementioned view. To accomplish that, Commerce Product Attributes adds the products corresponding to the line items in the cart as fields to the view.
3. Commerce Product Pricing is a module that formats prices of products in views that contain them. It defines hooks, such as _field_formatter_prepare_view() that would allow it to act upon a product field being added to the display. Then it calls the modules necessary (e.g. Commerce Tax) to do the formatting for the price of this product.
4. In this case, Commerce Product Pricing reacts to the product field being added by Commerce Product Attributes. It treats it just like any other product field: it attempts to format its price with Commerce Tax.
5. Commerce Product Pricing, of course, should only modify these prices for the purposes of displaying only. These changes should never end up in the database. This is why it creates an additional field for every product, original, where it stores the old price of the product to be restored when the view is fully formed.
6. What Commerce Product Pricing doesn't know is that there is another module that also modifies these products: Commerce Stock Reserve. Only as opposed to Commerce Product Pricing, Commerce Stock Reserve makes its modifications permanent by saving them in the database.
7. In the end, Commerce Stock Reserve saves both the changes it has made (the stock levels) and the temporary changes that Commerce Product Pricing has made, which were meant to be removed a little later and never show up in the database.

One might argue that the reason behind this bug is the bad architecture of Commerce Product Pricing, since this module assumes that nothing would be working with products at the same time when it does. However, rebuilding the architecture of a submodule of Commerce would be too complicated.

It was suggested that the best course of action is to make Commerce Stock Reserve independent of anything that can be messing up with products while it is doing its job, regardless of it being Commerce Product Pricing or anything else that a particular Drupal configuration might contain. This would be possible, if Commerce Stock Reserve retrieved its products directly from the database, instead of using their versions in the RAM, as it does now.

The patch is attached.

Once again, thank you very much for your input! Indeed, the patch I submitted wouldn't be functional. I guess, I failed to test it properly.

I have tested the latest patch on my configuration and can confirm that it does solve this issue!

I am reopening this issue due to a knock-on effect caused by the latest patch. Namely, the automated testing of the module returns failures when ran.

Below is the code causing the issue from testReserve() of commerce_stock_reserve_basic.test:

    $product = $this->createDummyProduct();
...
    // Create an order containing the product.
    $quantity = rand(1, 10);
    $order = $this->createDummyOrder(1, array(
      $product->product_id => $quantity,
    ));
...
    $actual_stock = commerce_stock_reserve_get_product_stock($product, TRUE);

Note that $product is not passed or returned by createDummyOrder(). Nonetheless, the function assumes that after createDummyOrder() is called, $product will pick up the change automatically. In other words, the function expects createDummyOrder() to find $product in the memory and modify it, even though createDummyOrder() does not even know that $product as well as the code using it exist. I do not believe that this approach is correct.

The test used to work before, since, indeed, the Entity Caching mechanism of Drupal forces createDummyOrder() to use our $product, which I consider a lucky coincidence. In general, several rules are invoked within createDummyOrder() which trigger several completely unrelated modules. It is somewhat naive to expect all of them to use caching.

In our case, the test fails with the patch, since in our patch we call entity_load(), which clears the entity cache for the entity being loaded. Hence, the alterations Commerce Stock Reserve does to the stock value are no longer stored in $product, the test cannot see them and returns a failure.

In the patch attached I use commerce_product_load() to locate the right place where the changes are stored. Once picked up by the system, the test is back to being successful.

Produced the patch for a wrong directory. Fixed.