-
Notifications
You must be signed in to change notification settings - Fork 685
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
[feature]: Add to Cart support for Virtual products #2769
Conversation
#dmcdindia2020 |
|
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 @shikhamis11,
thank you for Pull Request from my point of view you should test cases for:
- packages/peregrine/lib/talons/ProductFullDetail/tests/useProductFullDetail.spec.js
- packages/venia-ui/lib/components/ProductFullDetail/tests/productFullDetail.spec.js
Without test, you reduce coverage from 79.9% to 79.52.
@tjwiebell would do you think ?
Kind regards,
Lars
Thanks for the contribution @shikhamis11. Nice work. Please do update the tests to cover the new feature. |
@revanth0212 do you mind checking what the team's plans around adopting the single mutation for adding products to the cart are that lands in 2.4.1 - see magento/magento2#27914 as to avoid building something that may get superseded very soon. |
@fooman Thank you for bringing this up! As the issue, #2768 was opened recently we haven't yet had a chance to analyze it internally or to provide feedback on the issue. As you have identified, it may be possible for us to migrate to the new mutation, I do think we should try to implement the new mutation, but I also see value in this pull request as it will support Magento < 2.4.0. I have opened https://jira.corp.magento.com/browse/PWA-1016 internally for tracking the use/implementation of the generic mutation. |
@sirugh isn't the support matrix PWA-Studio 8.0.0 supports 2.4.0 - 2.4.1, while this PR would only make it into 9.0.0 which would then be 2.4.1-2.4.2? |
Yes, but as far as I know they aren't removing the old mutations yet. |
@shikhamis11 Can you please take care of failed test and merge conflict? |
@shikhamis11 - I've resolved those conflicts, but I'm unfortunately being asked to stall this PR until critical path scenarios for virtual product are completed. @dpatil-magento pointed out during QA that add to cart works as expected, but it does prevent customers from checking out, and there probably isn't support for accessing products. We aren't certain what required flows are needed, so we would hate to ask you take on the completion of that work, but we're happy to continue collaborating on this feature if you're interested. We appreciate the contribution and will use it as the starting point for the Virtual Product epic we have coming up that will fill in the rest of the gaps around checkout and product delivery. I realize this was a contribution day submission, and will see what I can do about crediting you for the PR, even though it will be closed. Thank you again for the submission. We're making an effort to better supply features and bugs the team is prepared to accept and provide support around implementation. This can be found on the Community Backlog project under the Prioritized column 🙂 |
Hi there @tjwiebell , Regards |
Description
Add to Cart support for Virtual products
Related Issue
Closes #2768
Acceptance
Verification Stakeholders
Specification
Verification Steps
go to virtual product product page and add to cart
Screenshots / Screen Captures (if appropriate)
Checklist