-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
[GraphQl] Adding Wishlist GraphQl coverage #28205
[GraphQl] Adding Wishlist GraphQl coverage #28205
Conversation
Hi @eduard13. Thank you for your contribution
For more details, please, review the Magento Contributor Guide documentation. |
1bb1dde
to
b32f610
Compare
@magento run Static Tests |
4e7ad8c
to
6d904f1
Compare
@magento run Static Tests |
@magento run all tests |
0677e22
to
5de8d9b
Compare
@magento run all tests |
/** | ||
* Adding products to wishlist resolver | ||
*/ | ||
class AddProductsToWishlistResolver implements ResolverInterface |
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.
As a rule, we don't prefix resolvers with the "Resolver" keyword. We have some cases but they come from the past. It's not about something critical, but with the prefix we have quite a long class/file name.
I would not put the prefix
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.
Was checking the resolvers from the same folder. However, the suffixes were removed.
@magento run Static Tests, Magento Health Index |
75e028e
to
7488a36
Compare
@magento run Static Tests, Magento Health Index |
Hi @rogyar, thank you for the review. |
@magento run WebAPI Tests |
Hi @lenaorobei those failing tests are caused by MultipleWishlist related PR. Please check this comment for additional information |
In this case those PRs should be delivered as related and builds triggered for corresponding ce/ee branches. |
@magento run all tests |
@lenaorobei looks like everything is green ✅ here. Are we able to proceed further with this one or are there any additional requirements from my side? |
@magento run all tests |
@dthampy this PR is ready for QA. |
I sent @eduard13 a schema file that contains updated descriptions. |
Adjusting schema's descriptions
Thank you @keharper, I've just updated it. |
@magento run all tests |
Hi @eduard13, thank you for your contribution! |
Description (*)
This PR implements the Wishlist GraphQl
Solution Architecture
Related Pull Requests
https://github.com/magento/partners-magento2ee/pull/247
Fixed Issues (if relevant)
Manual testing scenarios (*)
Please check the following document, in order to add different types of products to wishlist.
Add products to wishlist
Update wishlist item
Remove wishlist item
Questions or comments
Contribution checklist (*)