Removed 2 unneeded function calls. Local var is already there.

[woutersamaey]

Removed 2 unneeded function calls. Local var is already there.

[github-actions]

Unit Test Results

1 files  ±0  1 suites  ±0   0s ⏱️ ±0s
0 tests ±0  0 ✔️ ±0  0 💤 ±0  0 ❌ ±0 
2 runs  ±0  2 ✔️ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 2fe9658. ± Comparison against base commit 6270dc4.

[hannes011]

Hi @woutersamaey @colinmollenhour @tmotyl @sreichel

I think we've to rollback this change. It appears to change and break some basic behavior of Magento on which several modules and functionalities depend on. This gets clear when you compare the two data sources $product and $this->getProduct() when dealing with configurable products!

It's quite a habit of some developers to rely on the SKU containing always the simple SKU (also if you have the quote/order item of a configurable product). Here some example stackexchange question from the community: https://magento.stackexchange.com/questions/19372/get-id-of-simple-product-of-any-configurable-product-on-cart-phtml-magento/126318

One of my customers was complaining about wrong weights and SKUs (and for some reason also wrong MPNs) on their invoices. After rolling back your changes everything was back to normal again.

Here some example data:

[kiatng]

Here's the code snippet that supports @hannes011:

    /**
     * Retrieve product model object associated with item
     *
     * @return Mage_Catalog_Model_Product
     */
    public function getProduct()
    {
        $product = $this->_getData('product');
        if ($product === null && $this->getProductId()) {
            $product = Mage::getModel('catalog/product')
                ->setStoreId($this->getQuote()->getStoreId())
                ->load($this->getProductId());
            $this->setProduct($product);
        }

        /**
         * Reset product final price because it related to custom options
         */
        $product->setFinalPrice(null);
        if (is_array($this->_optionsByCode)) {
            $product->setCustomOptions($this->_optionsByCode);
        }
        return $product;
    }

That is called twice. May be a patch like this:

public function setProduct($product)
    {
        if ($this->getQuote()) {
            $product->setStoreId($this->getQuote()->getStoreId());
            $product->setCustomerGroupId($this->getQuote()->getCustomerGroupId());
        }
        $product = $this->setData('product', $product)->getProduct();
        $this
            ->setProductId($product->getId())
            ->setProductType($product->getTypeId())
            ->setSku($product->getSku())
            ->setName($product->getName())
            ->setWeight($product->getWeight())
            ->setTaxClassId($product->getTaxClassId())
            ->setBaseCost($product->getCost())
            ->setIsRecurring($product->getIsRecurring());
...

[colinmollenhour]

@hannes011 does the proposed solution from @kiatng fix the issue in your case?

[hannes011]

@colinmollenhour
it works, but it's basically doing the exact same thing as before this pull request was applied. The setData is just pulled in one upwards.
I'd be fine with a rollback or that other change.
Unfortunately, version 19.4.10 was released by now so it might come to some more requests requiring this change https://github.com/OpenMage/magento-lts/releases/tag/v19.4.10 (hope not, though)

Cheers, Hannes

[sreichel]

I'd be fine with a rollback or that other change.

Can you please provide a PR (just rollback for now)?

(I only testet with simple products ... :/ I'd prefer rollback asap too, impovements can be (re-)done later.)

[AlterWeb]

Our customers also have issues with this change. I fixed this also by reverting back those lines and came here to mention the problem. A quick release of a fixed version would be preferable I guess, because this is corrupting order data while in this state.

[seansan]

@AlterWeb how did you revert this exact change? (Sorry not too familiar with git)

[AlterWeb]

@seansan The most easy way is to copy app/code/core/Mage/Sales/Model/Quote/Item.php to app/code/local/Mage/Sales/Model/Quote/Item.php. This way the core file stays intact but Magento is using the local version. In the local file you then change $product->getSku() back to $this->getProduct()->getSku() and $product->getWeight() back to $this->getProduct()->getWeight().

Don't forget to remove this local copy after you update OpenMage to a newer version where this issue is resolved (it looks like it will be fixed in 19.4.11 when it will be released).

[addison74]

After reading how many issues created this commit a famous phrase is coming in my mind: if it ain't broke, don't fix it!