When prices are generated, they get cached in uc_discount_price_handler_alter based on the node id and the context... This doesn't work when you have products that vary in price based on attributes, because the same node may have multiple prices. So if you add multiple instances of the same node with prices varying based on attribute to your cart, they all end up with the same price.

This little patch, which may totally suck for all I know, just swaps out the cache key for the product model instead. Since the model of the node passed in with the context is always the primary SKU, I swap out the model of the product with the model specified in the cart item, which is whatever variant they selected.

This still will have problems if someone uses attributes that alter price without specifying atomic SKU's for each variation. So this whole approach may not work anyway.

Also, I noticed that if I turn caching off in the uc_discount_price_handler_alter function, none of the CA functions return the correct prices once you kick into checkout from the cart view. It seems that as soon as the "location" is "cart-checkout-item" the CA actions don't return the altered price anymore. I traced through the CA chain in uc_discount and just couldn't seem to figure out why that was happening... Anyway, since the last stage of checkout seems to be dependent on the caching functionality of uc_discount_price_handler_alter, making the caching issue even more important ;)

Comments

Island Usurper’s picture

StatusFileSize
new1.25 KB

Really, the only thing that identifies the items in the cart are their data arrays. If I change the static $prices array to be keyed off those serialized arrays, though, I think we really need to get rid of the CA predicate locking that is still going on. I'm going to post a patch at #369742: CA predicates maybe don't need to be prevented from being evaluated more than once. to do that, but I'm going to test both with and without it just in case.

Afterthought: Wait. Maybe locking wouldn't apply since the $node->sell_price would be different because it's getting the total $item->price which includes the attribute price adjustments.

Island Usurper’s picture

StatusFileSize
new1.16 KB

Wrong file for this thread, dummy. :P

Island Usurper’s picture

Status: Needs review » Fixed

Looks like it works without removing the predicate locking.

Committed.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.