Fixed a three problems that could happen when you have bundle products #3692
Conversation
…g for bundle products
…em_id This is a problem because then they are equal but should not so now we check if one is undefined and then return false in that case.
…into fix/bundle-products-cases
| @@ -1,7 +1,7 @@ | |||
| <template> | |||
| <form class="custom-options"> | |||
| <div v-for="option in product.bundle_options" :key="('bundleOption_' + option.option_id)"> | |||
| <product-bundle-option :option="option" @optionChanged="optionChanged" :error-messages="errorMessages" /> | |||
| <product-bundle-option :option="option" @option-changed="optionChanged" :error-messages="errorMessages" /> | |||
| </div> | |||
There was a problem hiding this comment.
Shouldn’t it be “bundleOptionChanged” as for the function name?
There was a problem hiding this comment.
I think that this is fine as it is. As it only Happens in files that have bundle in the name so it should be clear what is happening here.
When you think it is not so clear we can change it.
| getServerItemId(product1) === getServerItemId(product2) | ||
| const isServerIdsEquals = (product1: CartItem, product2: CartItem): boolean => { | ||
| if (getServerItemId(product1) === undefined || getServerItemId(product2) === undefined) { | ||
| return false |
There was a problem hiding this comment.
What if both products have empty server_item_id (say both products added in the offline mode and not yet synced); it’s probably from the caller context that it should be false
In this case correct?
There was a problem hiding this comment.
I should always be false when one of the products has no server_item_id or item_id other wise when both don't have it they are the same product but aren't in most cases.
There was a problem hiding this comment.
But for that case we have the check of the checksum.
andrzejewsky
added
the
QA - Ready for tests
|
@ResuBaka could you please add a short info how to test / reproduce bug you mentioned? - this is for understanding for QA team. |
|
As it is already merged I still write in here how you could have run into some of the problems. The Event name change is that in our theme it has not worked I didn't under stand why. So I changed it to more like how actual dom event are named as there are no uppercase in them allowed as far as I am aware of it. Then the change in the for checking if an bundle option has multible options is when each or one bundle option has only one option in it. The change for if an product has an server_item_id or item_id is there when it happens that both are not set for what ever reason in both product both are undefined that it would be true which is wrong. This is hard to force I thing as I don't even know what that is happen for us but as it is an edge case it is still good to have it fixed. |
Related issues
closes #
Short description and why it's useful
All three cases above are fix with this PR.
Screenshots of visual changes before/after (if there are any)
Which environment this relates to
Check your case. In case of any doubts please read about Release Cycle
developbranch and want to merge it back todevelopreleasebranch and want to merge it back toreleasehotfixormasterbranch and want to merge it back tohotfixUpgrade Notes and Changelog
IMPORTANT NOTICE - Remember to update
CHANGELOG.mdwith description of your changeContribution and currently important rules acceptance