Magento Block Class Rewrite In Config.xml - Second Guessing My Methodology

Question

so I have written a module that works quite well. However, I am trying to make sure I write all of my modules as conflict-free as possible. The short: my module displays reviews for the Associated products of Grouped and Configurable products on the parent product page and listings.

In catalog/product/list.phtml there are 3 lines of code that if the parent product has no reviews, even though the children products have reviews, it will not satisfy this if condition in order to display the summary html.

<?php if($_product->getRatingSummary()): ?>
<?php echo $this->getReviewsSummaryHtml($_product) ?>
<?php endif; ?>

So, my goal was to add data to each element in this collection before it reached the template file.

$_productCollection=$this->getLoadedProductCollection();

So, I did a rewrite of Mage_Catalog_Block_Product_List and rewrote the method below.

protected function _getProductCollection()

Actually, I copied the exact method over and added my simple code to the bottom of it and it does indeed do exactly what I want it to do.

protected function _getProductCollection()
{
  if (is_null($this->_productCollection)) {
      $layer = $this->getLayer();
      /* @var $layer Mage_Catalog_Model_Layer */
      if ($this->getShowRootCategory()) {
          $this->setCategoryId(Mage::app()->getStore()->getRootCategoryId());
      }

      // if this is a product view page
      if (Mage::registry('product')) {
          // get collection of categories this product is associated with
          $categories = Mage::registry('product')->getCategoryCollection()
              ->setPage(1, 1)
              ->load();
          // if the product is associated with any category
          if ($categories->count()) {
              // show products from this category
              $this->setCategoryId(current($categories->getIterator()));
          }
      }

      $origCategory = null;
      if ($this->getCategoryId()) {
          $category = Mage::getModel('catalog/category')->load($this->getCategoryId());
          if ($category->getId()) {
              $origCategory = $layer->getCurrentCategory();
              $layer->setCurrentCategory($category);
              $this->addModelTags($category);
          }
      }
      $this->_productCollection = $layer->getProductCollection();

      $this->prepareSortableFieldsByCategory($layer->getCurrentCategory());

      if ($origCategory) {
          $layer->setCurrentCategory($origCategory);
      }
  }

  foreach($this->_productCollection as $product){
    if(($product->getTypeId() == 'configurable' || $product->getTypeId() == 'grouped') 
    && !$product->getRatingSummary()) {
        $product->setRatingSummary(1);
    }
  }

  return $this->_productCollection;
}

What I cant help but second guess is that do I really need to rewrite this function just to add data to the collection that it returns or is there a way to simply add my data to the collection before it reaches the template file without doing an actual rewrite like I have done above. The category product list is one of the main data collections on a Magento site and I am not crazy about messing with it just to add a tiny snippet of data. Also, I worry about conflicts with future extensions.

Please let me know how to alter the collection object without a rewrite, if possible.

Thank you.

Answer 1

If you are going to do a rewrite, I'd recommend a methodology like this.

protected function _getProductCollection()
{
    $collection = parent::_getProductCollection();
    //do your stuff to the collection here
    return $collection;
}

Your block class extends the existing block class. That means you can call the parent method and have the same code run. When you copy and paste code as you did in your example, you risk your extension not working in past/future versions of Magento where this code has changed.

My general rule of thumb on using rewrites is: If you stay in control of the code and can debug potential conflicts, a rewrite is fine. If you don't stay in control of your code, you shouldn't rewrite a method.

If you want to avoid a rewrite, your other option is Magento's event system. Every collection in Magento fires an event before and after it's loaded. The trick is finding the name of the event you're after, which requires either digging through the source of Mage::dispatch, or using a tool like Commerce Bug (warning: self link) to track down the events your'e after.

I prefer the second method, so using Commerce Bug's observer's tab (which shows any existing Magento observers setup on a page), we can see Magento itself has a listener setup for the product collection. Specifically, on for the catalog_product_collection_load_after event.

If you look at the source of Magento's cataloginventory/observer, you can even see how Magento modifies the collection after its been loaded

#File: app/code/core/Mage/CatalogInventory/Model/Observer.php
public function addStockStatusToCollection($observer)
{
    $productCollection = $observer->getEvent()->getCollection();
    if ($productCollection->hasFlag('require_stock_items')) {
        Mage::getModel('cataloginventory/stock')->addItemsToProducts($productCollection);
    } else {
        Mage::getModel('cataloginventory/stock_status')->addStockStatusToProducts($productCollection);
    }
    return $this;
}

If your case, instead of checking the require_stock_items flag on the products collection, check for something about the state of the system that says it's loading the page where you want your information added.

That should give you enough to get started with your own listener. Good luck!