-
Notifications
You must be signed in to change notification settings - Fork 9.4k
Fix #24091 - Selected configurable product attribute options are not displaying in wishlist page. #28157
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Hi @Bartlomiejsz. Thank you for your contribution
For more details, please, review the Magento Contributor Guide documentation. |
96efff4
to
93372ea
Compare
…re not displaying in wishlist page.
93372ea
to
a128f39
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @Bartlomiejsz thanks for the pull request, it looks great! Looking at the problem, I believe we can solve this just on the JS part, follow below my thoughts:
Since the _updateWishlistData
is triggered by an event, we have the event.target
that is the selector of the option that the user has changed within the product block (it can be the product page or an item within a product list), from that perspective we can extract the selected attribute values from the correct product and also on _updateAddToWishlistButton
we can update the post data on the right add-to-wishlist
element.
Using this approach, it won't be a need to have an object dataToAdd
with productId, since the event gives us the context that we need to collect the data from the right product and update it's form.
We would still have to have some mechanism to figure out if we are on list or product page, but I think that can be done on JS as well, what do you think?
Please let me know if there are any questions.
…t_options_on_wishlist
…r configurable product on wishlist
Hi @gabrieldagama, that is great idea to limit execution of wishlist button update to only current product instead of lopping for every item possible. I managed to reduce js needed for it and also accomplish that, thank you. However I'm not sure if checking if current page is product list via js is good idea. Could you please check current solution after my changes and let me know what do you think? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @Bartlomiejsz, thanks for the updates! Agree with your arguments there, please follow some very small suggestions to keep consistency, also we must cover all PRs will tests, the add-to-wishlist
has a jasmine test that may need to be updated based on the changes and it would be great if we can cover this with MFTF tests, let me know if you need any help with the tests!
Thanks!
app/code/Magento/Wishlist/view/frontend/layout/catalog_category_view.xml
Outdated
Show resolved
Hide resolved
app/code/Magento/Wishlist/view/frontend/layout/catalogsearch_result_index.xml
Outdated
Show resolved
Hide resolved
@magento run all tests |
…esult_index.xml Co-authored-by: Gabriel da Gama <[email protected]>
…y_view.xml Co-authored-by: Gabriel da Gama <[email protected]>
Hi @Bartlomiejsz. I'll take care of the test coverage if you don't mind. |
@engcom-Echo i would not be able to do it this week so that's great, thank you! |
@magento run all tests |
@magento run all tests |
@magento run Functional Tests B2B |
1 similar comment
@magento run Functional Tests B2B |
@magento give me test instance with edition ee |
Hi @gabrieldagama. Thank you for your request. I'm working on Magento instance for you. |
Hi @gabrieldagama, here is your Magento Instance: https://6ae7b32b33bba4c9ac25485f2e8b61fd.instances.magento-community.engineering |
…t_options_on_wishlist
@magento give me new instance with edition b2b |
Hi @gabrieldagama. Thank you for your request. I'm working on Magento instance for you. |
Hi @gabrieldagama, here is your Magento Instance: https://6ae7b32b33bba4c9ac25485f2e8b61fd.instances.magento-community.engineering |
@magento run all tests |
@magento run all tests |
…ist' of github.com:Bartlomiejsz/magento2 into feature/fix_24091_configurable_product_options_on_wishlist
@magento run all tests |
1 similar comment
@magento run all tests |
✔️ QA Passed Preconditions: Magento 2.4-develop with sample data Case 1Manual testing scenario:
Before: ✖️ The selected color and size is not visible in the wishlist page. After: ✔️ The selected color and size is visible in the wishlist page. Case 2Manual testing scenario:
After: ✔️ the system didn't assign different product options to the wishlist product. |
@magento run Functional Tests B2B,Functional Tests EE |
…ions are not displaying in wishlist page. #28157
Hi @Bartlomiejsz, thank you for your contribution! |
Description (*)
This fixes #24091 and #22503
The reason of the issue is that in add-to-wishlist.js there are event observers that are being triggered on for every configurable product on page after modifying one of them. So if the last product on page doesn't contain any option checked, no matter which will be added to wishlist, it won't contain options.
Related Pull Requests
Fixed Issues (if relevant)
Fixes #22503
Fixes #24091
Manual testing scenarios (*)
Check fixed issues for steps to test
Questions or comments
Contribution checklist (*)