Wishlist updates proposed in Magento 2 Commerce PR (internal) #403
Conversation
| wishlists: [Wishlist!]! @doc(description: "Customer multiple wishlists") # Multiple wishlists Commerce functionality | ||
| wishlist: Wishlist! @deprecated(reason: "Use `Customer.wishlists` or `Customer.wishlistByID") | ||
| wishlistByID(id: ID!): Wishlist | ||
| wishlists: [Wishlist!]! @doc(description: "Customer wishlists are limited to a max of 1 wishlist in Magento Open Source") |
There was a problem hiding this comment.
Can also be limited to one in Commerce, with the Enable Multiple Wish Lists feature set to false
https://docs.magento.com/user-guide/configuration/customers/wishlist.html#general-options
| wishlist: Wishlist! @doc(description: "Customer wishlist") # Commerce will extend filed with required `id` argument `wishlists(ids: ID!)` | ||
| wishlists: [Wishlist!]! @doc(description: "Customer multiple wishlists") # Multiple wishlists Commerce functionality | ||
| wishlist: Wishlist! @deprecated(reason: "Use `Customer.wishlists` or `Customer.wishlistByID") | ||
| wishlistByID(id: ID!): Wishlist |
There was a problem hiding this comment.
How about v2 instead of byId, this would be more aligned with our current strategy.
| wishlistByID(id: ID!): Wishlist | |
| wishlist_v2(id: ID!): Wishlist |
There was a problem hiding this comment.
Would like to respectfully question our current strategy a little bit. I'm not sure prepending things with _v2 by default is the best design we can do.
A field name should really describe to the client what the purpose of the field is.
wishlistByID(id: ID!)- Makes it clear you're getting a wishlist by ID, but a bit redundantwishlist(id: ID)- Makes it clear you're getting a wishlist by IDwishlist_v2(id: ID!)- Makes it apparent that you're looking up by ID, but also makes you wonder why the v1 version of the field had issues.
I can see _v2 in a scenario where we can't come up with a more descriptive name, but I think we have a variety of naming choices here. Can also be redundant and do customer_wishlist or wishlist_details. Plenty of options!
@nrkapoor mentioned recently that we have rules around how long a deprecation lasts before we're safe to remove a field. So the _v2 will likely live long after the original is gone.
There was a problem hiding this comment.
Spoke with @nishant, and he's in favor of _v2, and I'm in favor of consensus :)
I'll push up changes
|
I believe you mentioned the wrong user as I have no connection to this repository |
|
Whoops - sorry about that @nishant! |
Based on issue discovered in https://github.com/magento/partners-magento2ee/pull/248.
There is a separate PR with some of the work.
Proposal was approved by @nrkapoor and @fnhipster, but sending through the proper channels for review.