Suggested fix to make $quote->getItemById more robust

[seansan]

Several new quote and order flows do not always trigger and set the quoteid ... the quote is then created with items but has no id yet ... and getItemById () does not work in these case because there is a different key. This happens for instance when orders are created via

        /** @var Mage_Sales_Model_Service_Quote $service */
        $service = Mage::getModel('sales/service_quote', $quote);
        $service->submitAll();

Please review and UPDATE my code suggestion directly. Needs a proper review as it is quite in the core of quotes of M1

[Flyingmana]

not sure if it is the best solution forthe issue, probably not, but I dont see any down sides or possible side effects

[seansan]

I agree with you. But it does fix a lot of problems or exotic situations when you cannot grasp why it does not work. Please correct my if else code if necessary. Thx sean

…

On Mon, 14 Oct 2019 at 23:41, Daniel Fahlke ***@***.***> wrote: ***@***.**** approved this pull request. not sure if it is the best solution forthe issue, probably not, but I dont see any down sides or possible side effects — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#834?email_source=notifications&email_token=AAE7I25ZMGHQU522BWLPK53QOTRP3A5CNFSM4JAM4G4KYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCH4VX3I#pullrequestreview-301554669>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAE7I2Y7MKP2VBMA2M4NHZDQOTRP3ANCNFSM4JAM4G4A> .

[joshua-bn]

Scary that this got committed missing those two giant issues that got fixed in the other two commits

[henrykbrzoska]

if someone have exception like
SQLSTATE[23000]: Integrity constraint violation: 1452 Cannot add or update a child row: a foreign key constraint fails (magento.sales_flat_quote_item_option, CONSTRAINT FK_5F20E478CA64B6891EA8A9D6C2735739 FOREIGN KEY (item_id) REFERENCES sales_flat_quote_item (item_id) ON DELETE CASCADE O)
it's because of this change.
in my example it occurs in IWD order manager when trying to add disabled product to cart(without supermode on quote)

[digitalpianism]

@henrykbrzoska same issue here with the Amasty_Promo module with an "Auto add product to cart" rule. the $itemId being null, the comparator isn't correct so I had to make a fix in the module.

[Flyingmana]

@henrykbrzoska @digitalpianism
Could one of you create an Issue for this and reference it here?

[digitalpianism]

@Flyingmana I'm not entirely sure this is an issue.

Basically the module's code look like this:

$cart->removeItem($item->getId());

On Magento, as the $item does not have an id, it does nothing.

On Openmage, this piece of code here:

foreach ($this->getItemsCollection() as $item) {
                if ($item->getId() == $itemId) {
                    $quoteItem = $item;
                    return $quoteItem;
                } 
            }

the condition is true as there's an item without id and the $itemId is null.

I've just added a condition in the module's code as I assumed it was the module badly written rather than this openmage change.

[digitalpianism]

@Flyingmana I'm not entirely sure why this change was made to start with even though I read @seansan comment.

If the condition I mentionned above, maybe we should add an "isset" or "!is_null" condition. I dont know which one of the two is supposed to be set.