Extract method _calculateProductQuantity in observer

[shineability]

We have the following custom business logic to implement in our aggregation module.

Customers (mostly departments, affiliates, ...) can place orders and together they can lower the price of an item by using a "global" tier pricing setup.

So the final price of an item should be based on the total quantity available in the cart - as implemented right now by this package - plus the quantities already ordered by other customers.

We are using the latest tagged version v2.1.0, where the methods in the observer are defined as private. In the master branch, however, they already seem to be defined as protected.

So we could already rewrite _calcConfigProductTierPricing and implement our own logic. Even better, imo, would be to extract the quantity calculation to its own method.

This way we could implement our logic in a very DRY way...

class Rewrite_ConfigurableTierPrices_Model_Observer extends Spranks_ConfigurableTierPrices_Model_Observer 
{
    protected _calculateProductQuantity($product) 
    {
        $quantity = parent::_calculateProductQuantity($product);

        return $quantity + $totalQuantityOfAllOrdersCustomValue;
    }
}

Wdyt?

Would you accept a pull request that implements this?

[sprankhub]

Thanks for your useful feedback. Indeed, the tagged version is quite old. I would accept a pull request with such a change. Afterwards, I would tag a new version. Mind that we switched the namespace from Spranks to CustomGento though.

[shineability]

Great, one pull request coming up! :)
We're aware of the changed namespace, btw...

[shineability]

@sprankhub

In the observer on line 58...

if ($item->getParentItem()) {
    continue;
}

Is that used to filter out non-configurable products?

If so, isn't that already covered by...

if (Mage::registry('current_product') || !$product->isConfigurable()) {
    return $this;
}

If not, please explain :)

[sprankhub]

It's been a while since I wrote this code :-)

Mind that we iterate over all visible quote items and then filter by $item->getParentItem(). This has nothing to do with the current product. Hence, I guess both filters make sense.

[shineability]

Correct, I realized that right after posting my comment :)

I'm still a bit confused, though.
I have one simple product and two configurable products (t-shirts - medium and large).
I would have expected that for the two t-shirts, $item->getParentItem() would return the parent configurable product, but then the code wouldn't work of course...

So I think my question is, what scenario is the if statement covering? Bundled products?

(Hope I'm making sense)

[sprankhub]

Mind that $item->getParentItem() returns a quote item and not a product. If you add a configurable product to the cart, there are two quote items - a parent and a child item. Simply add a configurable product to the cart and check the quote item table. This should make things a bit clearer.

[shineability]

Aha, makes sense, tnx!

[shineability]

Actually, checking for $item->getParentItem() is not necessary since you're using getAllVisibleItems()...

It's already being filtered out in Mage_Sales_Model_Quote...

public function getAllVisibleItems()
{
    $items = array();
    foreach ($this->getItemsCollection() as $item) {
        if (!$item->isDeleted() && !$item->getParentItemId()) {
            $items[] =  $item;
        }
    }
    return $items;
}

You agree?

[sprankhub]

I think you should be right :-)

[sprankhub]

This will be fixed with #34.