dotmailer / dotmailer-magento2-extension

The official Dotdigital for Magento2 extension
https://dotdigital.com/integrations/magento
MIT License
48 stars 64 forks source link

Fix merging quote on configurable products #601

Closed paco-shum closed 1 year ago

paco-shum commented 1 year ago

Issue: When an abandoned cart contains configurable items, the original method won't be able to add the parent due to missing selected options on $currentQuote->addProduct($item->getProduct()); See screenshot as example Screenshot 2023-02-09 at 18 59 06

Solution: Using getAllVisibleItems() instead, and loop through visible items to check for simple products that are contained within a configurable product. This method is borrowed from the merge() function within Quote but slightly simplified to match the original implementation. (Currently this duplicate when adding the same configurable product but this can fixed by back step a bit on simplification and use if ($quoteItem->compare($item) or similar instead)

actually now thinking,...why not just use the merge function on quote...ah I forgot why whoops

edit: We have confirmed it will indeed duplicate the item (not adding QTY) using this fix so need slight modification

sta1r commented 1 year ago

@paco-shum Thank you for this. We'll check it out and come back to you.

pvpcookie commented 1 year ago

Hi Paco,

Would you be able to list out the steps you took to get to this point please, we have been struggling to recreate the issue you reported.

When you refer to selected option are you refering to the Customizable options on the product ?

paco-shum commented 1 year ago

Hi @pvpcookie Thanks for getting back and sorry for the confusion, here are the steps I took to trigger that function

  1. on browser A, add a configurable product to cart
  2. then on browser B, add another configurable product to cart
  3. on browser B, access {siteURL}/connector/email/getbasket/quote_id/{the quote created on step 1}/

The assumption

I have just checked it again, the product is indeed a configurable product with child simple products (under the product -> 'Configurations' tab, not 'Customizable Options ')

Hope this helps. Kind regards,

pvpcookie commented 1 year ago

Hi @paco-shum

Thank you this helps immensely, i will look further into this and get back to you.

pvpcookie commented 1 year ago

Hi @paco-shum ,

Thanks for the patients and contribution it really appreciated, could you please direct you PR to the develop branch and we can a take it from there :)

paco-shum commented 1 year ago

Hmm. 1 sec

paco-shum commented 1 year ago

@pvpcookie fixed. hope this is ok.

pvpcookie commented 1 year ago

Hi @paco-shum

Perfect, thank you.

paco-shum commented 1 year ago

Hi @pvpcookie may I ask is there a timeline for this fix gets released to master/ new version? We have a merchant that would like to have this sorted. Thanks.

sta1r commented 1 year ago

@paco-shum Hi, we're aiming to have this reviewed and merged into our next release 4.20.3, scheduled for 15 March.

I've seen this PR includes 319 commits, even though the changeset is correct. I presume this is a result of changing the target branch. Does git pull <our-remote> develop fix this? If not we have two options:

Let us know what you'd like to do.

paco-shum commented 1 year ago

I think my fork/ local git is messing up, made a new PR to fix that. https://github.com/dotmailer/dotmailer-magento2-extension/pull/603

Thanks