Variable Return Values from getSalePrice

[JeRoNZ]

https://github.com/concrete5-community-store/community_store/blob/951a083a41de6d422573f75a296658db299b1088/src/CommunityStore/Product/Product.php#L1309

It seems that this function can return either boolean false, a string e.g. "0.00" or a float.

This means that getActivePrice has a hard time figuring out what to do, since it's testing for sale price being not an empty string. "0.00" is not an empty string, thus zero is displayed.

The underlying database field is a decimal 10.2 which can be null, so maybe this function should always return integer 0 if the field is null. 0.00 or anything else that effectively means the product is not on sale.

[losttheplot]

How about:

    public function getActivePrice($qty = 1)
    {
        if(Wholesale::isUserWholesale()){
            return $this->getWholesalePrice();
        } else {
            $salePrice = $this->getSalePrice();
            if (!empty($salePrice) && !$this->hasQuantityPrice()) {
                return $salePrice;
            }
            return $this->getPrice($qty);
        }
    }

And while we're there:

        if (!empty($saleStart) && $saleStart > $now) {
            return false;
        }

        if (!empty($saleEnd) && $now > $saleEnd) {
            return false;
        }

[Mesuva]

Yes this bit was already a bit of a mess. I think it needs to either return a float, or a false, but never a string. i also think returning a 0 should be for when the sale price is set to 0, i.e. it's free.

Did you pick up that something is not currently working with this, or is it because it making it difficult to programatically use?

[JeRoNZ]

What drew my attention was the zero price being shown in the dashboard product listing. I had written a loader and imported 300 products and was checking out the data. I thought I'd screwed it up and not loaded the price correctly, but that wasn't the case. Even with sale price and wholesale price explicitly set to zero by editing the product through the dashboard, the price remained zero in the list.

[Mesuva]

When you imported the products, did you import 0 as the sale price, or a null/blank value?

[JeRoNZ]

Sale price was explicitly set to integer 0

$product->setPrice($row['daily_rate']);
$product->setWholesalePrice($row['daily_rate']);
$product->setSalePrice(0);

[Mesuva]

So the (admittedly undocumented) intention is that setting 0 is actually setting a sale price, of $0. If you don't want a sale price, set it to an empty string.

[JeRoNZ]

OK, but the database field is a decimal that allows null, so it cannot in fact be an empty string.:

update CommunityStoreProducts set pSalePrice=''; ERROR 1366 (HY000): Incorrect decimal value: '' for column 'pSalePrice' at row 1

[Mesuva]

If you look at your setSalePrice function in your Product.php file, what's it got in the body of that function?

[JeRoNZ]

Yeah, I guess that it's doing what was intended but not documented. For the record, it sets the value to null if it's an empty string.

$this->pSalePrice = ($price !== '' ? (float)$price : null);

[Mesuva]

This looks a little older, I think there was a subtle tweak to that function not too long ago.