Price conversion in views for several displays linked to the same commerce product.

This issues is caused by the way commerce core handles the sell price calculation.
The function commerce_product_pricing_commerce_price_field_formatter_prepare_view currently only allows one calculation of a product price per site request.
This is done using a static cache that tracks which product sell price is calculated already.
Unfortunately it doesn't cache the generated price, to reuse it in case the product sell price is requested a second time during the same request.

The attached patch changes the behaviour to cache the calculated product sell price for later reuse.
I added the enhanced static cache pattern to keep the performance while adding the possibility to flush the product sell price cache.
Downside of the patch is an increased memory consumption per request due the higher amount of cached data.

Came across this myself just now and was going to fix it in the same way. Applied your patch and it fixes the problem.

You know what, in that case I'm sassy and set the status to RTBC ;)

I believe the general rule is not to set your own patch to RTBC but I probably should have done it anyway so there is no reason why this shouldn't be RTBC :)

Thanks for the patch!

I suggest we simply save the original value and recalculate everything in that case. The context can have changed, so it doesn't really make sense to save the calculated value in a static cache.

Untested patch attached.

Thanks for tracking this down, guys. I'm comfortable with Damien's approach, and it tested out just fine. I thought at first the static cache was introduced here to prevent pricing Rules from being executed multiple times against the same static cached field data, but removing the static cache prior to applying Damien's patch didn't have any adverse effects on my calculated prices.

Commit: http://drupalcode.org/project/commerce.git/commitdiff/cf9653e

+        if (isset($items[$product_id][0]['original'])) {
+          $items[$product_id] = array();
+          $items[$product_id][0] = $items[$product_id][0]['original'];
+        }

That doesn't look right to me, $items[$product_id][0]['original'] will get unset on the previous line.

lol How about that. I looked over this code several times trying to decide if it was necessary to initialize these arrays that way but went with it anyways because DamZ usually knows best. But you're right - here we need to extract the original value out of the array before we reset it. Apparently my test case passes whether that actually works or not. : P

It took me a while a while to reproduce the original problem, and when I did I finally did turn up the scenario I think I mentioned above that the code was designed to prevent - the duplicate application of calculation rules to price fields. It's not enough to reset the $items array passed in by reference; we also need to reset it on the $product itself in that loop since that's the object used to perform price calculation. The attached patch fixes the logic mjpa turned up and also copies the original value into every language of $product->commerce_price (avoiding the metadata wrapper) when we restore the original value.

Lemme know if resolves everything. : )

Whoops, hadn't saved before rolling the patch. This is the same thing with debug messages removed. :-/

@all: Thanks for working on this.
Damien's approach is indeed smarter, I didn't think about a possible context change...
And the latest patch looks good to me.

(avoiding the metadata wrapper)

Just in this special case, it's not a paradigm change, right?

Yeah, in this case. If we used the wrapper to swap the value into $product, I'd be afraid of missing a language value ... but I'm not sure price fields are translatable anyways. : P

+          // Reset the price field value on the product object used to perform
+          // the calculation.
+          foreach ($product->commerce_price as $langcode => $value) {
+            $product->commerce_price[$langcode] = $items[$product_id];
+          }

Are we sure that's necessary? Seems to me that we have this problem to begin with because $items is passed by reference and contaminates the product entity... isn't it?

Nope, those items aren't referenced to the actual field items on the $product itself, so changes to the $items array weren't affecting the field data on $product that was actually being used to generate the calculated price. I'm trying to track this down in the Field module itself, but I swear every place that invokes this hook is passing $null as $items. :-/

Ahh, I believe it's happening in _field_invoke_multiple(), something like this line:

$grouped_items[$field_id][$langcode][$id] = isset($entity->{$field_name}[$langcode]) ? $entity->{$field_name}[$langcode] : array();

Committing #9, as it fixes the original problem as far as I can tell. I'm sure we need to reset $product->commerce_price, though I suppose if we're doing that properly we may be able to get away without manipulation in $items. However, I think it best to keep using that array as you don't know what might happen to the $product->commerce_price data if the product were to be wrapped and manipulated / set somehow, causing the 'original' array to vanish since Entity API doesn't know about it.

What really matters is that the $product that gets passed to commerce_product_calculate_sell_price() contains the original price, and that's what this patch accomplishes.

Interestingly, my debug messages turned up some very inconsistent behavior. With a clean install of Commerce Kickstart, I added a View of all products on the site to the sidebar. This meant that I had products one, two, and three each rendering twice on the front page. Dumping the contents of each product's $items array in our code above, I found that only product one was getting the 'original' array in its price field data. I thought at first this was due to the rendering order - 3, 2, 1, 1, 2, 3 was the order products were being loaded and prepared for viewing. However, the same thing happened when I reversed the block (render order 3, 2, 1, 3, 2, 1) - only product one kept the calculated price between renders. It turned out this was because I had products 2 and 3 in my cart. That cart load was causing the field data to be reset when the unit price of the line item was being calculated. So... if you're testing and dumping debug messages, use a clean cart. : )

Please reopen if necessary.

Commit: http://drupalcode.org/project/commerce.git/commitdiff/ff06521