GordonLesti / Lesti_Fpc

Simple Magento Fullpagecache
https://gordonlesti.com/lesti-fpc-documentationversion-1-4-5/
Other
358 stars 158 forks source link

getStockStatusChangedAuto() vs getStockStatusChangedAutomatically() #112

Closed peterjaap closed 9 years ago

peterjaap commented 9 years ago

In Lesti_Fpc_Model_Observer_Save, there is this piece of code;

public function cataloginventoryStockItemSaveAfter($observer)
{
    $item = $observer->getEvent()->getItem();
    if ($item->getStockStatusChangedAuto()) {
        $this->_getFpc()->clean(sha1('product_' . $item->getProductId()));
    }
}

The function getStockStatusChangedAuto() should have an equivalent setStockStatusChangedAuto() function. But when I look through the Magento core, I get this;

➜  ~/magento git:(master) ✗ ack setStockStatusChangedAuto app/code/core
app/code/core/Mage/ImportExport/Model/Import/Entity/Product.php
1876:                    $stockItem->setStockStatusChangedAutomatically((int) !$stockItem->verifyStock());

app/code/core/Mage/CatalogInventory/Model/Stock.php
208:                    ->setStockStatusChangedAutomaticallyFlag(true);

app/code/core/Mage/CatalogInventory/Model/Stock/Item.php
61: * @method Mage_CatalogInventory_Model_Stock_Item setStockStatusChangedAutomatically(int $value)
727:                    ->setStockStatusChangedAutomaticallyFlag(true);
738:            $this->setStockStatusChangedAutomatically(0);
740:                $this->setStockStatusChangedAutomatically((int)$this->getStockStatusChangedAutomaticallyFlag());
➜  ~/magento git:(master) ✗

Which leads me to believe that getStockStatusChangedAuto() should actually be getStockStatusChangedAutomatically(). I've looked at the magic setters and getters to make sure there is no max length on the magic functions, but it seems there isn't (reference), leading me to assume that this flag is never set, thereby the product will not be invalidated by this trigger.

peterjaap commented 9 years ago

This issue in Nexcessnet_Turpentine seems to confirm my suspicions; https://github.com/nexcess/magento-turpentine/issues/159

peterjaap commented 9 years ago

Ok but now I'm not sure whether it should be getStockStatusChangedAutomatically() or getStockStatusChangedAutomaticallyFlag()

peterjaap commented 9 years ago

Seeing as the function is called upon the Item, and not the Stock model itself, I'm assuming getStockStatusChangedAutomatically(). I've updated the first post accordingly.

peterjaap commented 9 years ago

Possibly related to https://github.com/GordonLesti/Lesti_Fpc/issues/55

GordonLesti commented 9 years ago

Hello @peterjaap thank you for the issue. There seems to be an mapping from stock_status_changed_automatically to stock_status_changed_auto. https://github.com/OpenMage/magento-mirror/blob/053e0b286cbd6d52ac69ca9fd53a3b72c78aca1d/app/code/core/Mage/CatalogInventory/Model/Stock/Item.php#L156 but I'm not sure if that means, that getStockStatusChangedAuto really works.

peterjaap commented 9 years ago

From what I gather, that means those fields are synced to each other; https://github.com/OpenMage/magento-mirror/blob/053e0b286cbd6d52ac69ca9fd53a3b72c78aca1d/lib/Varien/Object.php#L115

So this isn't an issue. Closing.

peterjaap commented 9 years ago

Addendum; I did some further digging and found out that the stock_status_changed_auto(matically) is only set to true when the quantity of the product has changed only by setting the quantity to zero (and thereby setting the stock status to out of stock)! If you also set the is_in_stock flag manually, then this field isn't changed (hence the 'automatically').

We have a use case where the product should be invalidated every time the stock changes, since some attributes on the frontend depend on a specific value range for qty. So I've decided to completely remove the if() loop around the clean() method call;

public function cataloginventoryStockItemSaveAfter($observer)
{
    $item = $observer->getEvent()->getItem();
    $this->_getFpc()->clean(sha1('product_' . $item->getProductId()));
}
GordonLesti commented 9 years ago

Maybe it's better to override the observer? (I didn't mean model rewrite)

qx54 commented 9 years ago

Is this the only way to automatically flush/clear the product page cache of a product that has been ordered?

peterjaap commented 9 years ago

As usual in Magento, no this is not the only way. You could of course create a small extension with an observer on sales_order_place_after that loops through the order and invalidates each product using the hash.

qx54 commented 9 years ago

I'm not a coder, so I guess I have to go with this.

Mic2005 commented 8 years ago

I have stil this issue on Byte Hypernode. No custom stockstatus update (when quant < 0) after placing the order. So the only way to make it work (for non developers) is to remove catalog_product_view from Lesti::FPC Cachable actions?