Bug in discount price display for multi-priced products

[lowmips]

Description:
Products which have multiple prices and are discounted have a random retail price chosen as the comparison price, resulting in very strange discount numbers.  This also affects the price shown in the main product configuration page.

Versions affected:
I have checked both VM114 and VM115, both are affected.

Offending code:
ps_product->get_retail_price()

Code: [Select]

$q = "SELECT product_price,product_currency,price_quantity_start,price_quantity_end
FROM #__{vm}_product_price
WHERE product_id='$product_id' AND
shopper_group_id='$default_shopper_group_id'";
$db->query($q);
if ($db->next_record()) {
$price_info["product_price"]= $db->f("product_price");
$price_info["product_currency"]=$db->f("product_currency");
$price_info["price_quantity_start"]=$db->f("price_quantity_start"); // added alatak
$price_info["price_quantity_end"]=$db->f("price_quantity_end");// added alatak
}
else {
$price_info["product_price"]= "";
$price_info["product_currency"] = $_SESSION['vendor_currency'];
$price_info["price_quantity_start"]=$db->f("price_quantity_start"); // added alatak
$price_info["price_quantity_end"]=$db->f("price_quantity_end");// added alatak
}
return $price_info;

The code grabs the first result.  That resulting price ultimately depends on the order by which the administrator entered prices.

Fix:
Sort the results from highest to lowest, so the first price (and therefore the resulting retail price) is the highest possible retail price:

Code: [Select]

$q = "
SELECT
product_price,product_currency,price_quantity_start,price_quantity_end
FROM
#__{vm}_product_price
WHERE
product_id='$product_id'
AND
shopper_group_id='$default_shopper_group_id'
ORDER BY
product_price DESC";

I have used this fix for a client with hundres of products, most of which have volume pricing.  No issues with the fix reported so far.

[zanardi]

When you say "multiple prices", which one of the following do you refer to?
1. different prices for different vendors;
2. different prices for different shopper groups;
3. different prices for different quantities

[lowmips]

When you say "multiple prices", which one of the following do you refer to?
1. different prices for different vendors;
2. different prices for different shopper groups;
3. different prices for different quantities

Prices for quantities.  The code looks up the correct shopper group, but does not sort the prices if there are multiple prices.

[zanardi]

Sorry for late reply.
Now i understand you are just talking about administrator product list and product page. Yes, the price shown is the first record in the database. But i think that grabbing the highest does not necessarily make more sense, either. I think it is better to order them by quantity and i will do that in 1.1.6
Just a note: what do you mean by "price chosen as the *comparison* price"? You're still talking about admin pages or what?
Thank you for your explanations.

[lowmips]

I'm not near the code right now, so I can't give you the variable name, but the "comparison" price is the retail price before the discount.  This "comparison" price is shown on the flypage struck through in red, with the new discounted price shown below.

Thanks,
Reggie

[zanardi]

That is confusing me. Nothing in the front end calls the get_retail_price() function, so i cannot understand your issue in the front end and how you did manage to fix it by modifying get_retail_price().
Maybe i'm, just dumb, forgive me but i'll need more info if we want to fix that in the core code. Tell me the steps to reproduce the issue in the front end, please.

[lowmips]

I'll have to get back to you on this.  I haven't had time to retrace it.  That function is not directly called, but is a secondary or tertiary call.

[zanardi]

Ok, no hurry, it's just that if there really is an issue, i'd like to fix in the upcoming 1.1.6 release.

[lowmips]

Zanardi,
I finally had a chance to sit down and trace this. 

And I feel kind of silly

I used the function in a front end mod I did for a client, so it only affected the front end for that mod.  It does, however, affect the original VirtueMart code on the product,product_form.php and product.product_display.php files on the back end. 

I think it would be a good idea to go ahead and fix the logic anyway, in case future versions do use the call on the front end. Perhaps a very low priority issue?

Thanks for your time.
Reggie

[zanardi]

I'll set this in the tracker for 1.1.7.
1.1.6 is very near release and i would not like to make other changes now.
Thank you!