From 971e8f5bf1e746e5d5115cc67ab1015dc6d901bc Mon Sep 17 00:00:00 2001 From: Cristian Partica Date: Thu, 25 Mar 2021 10:57:23 -0500 Subject: [PATCH 1/5] - add wishlists in products query as a graph --- .../graph-ql/best-practice/nullability.md | 12 ++++---- .../coverage/customer/Wishlist.graphqls | 7 ++++- .../graph-ql/coverage/customer/wishlist.md | 29 +++++++++++++++++++ 3 files changed, 41 insertions(+), 7 deletions(-) diff --git a/design-documents/graph-ql/best-practice/nullability.md b/design-documents/graph-ql/best-practice/nullability.md index 9624db4a2..c3f31c2eb 100644 --- a/design-documents/graph-ql/best-practice/nullability.md +++ b/design-documents/graph-ql/best-practice/nullability.md @@ -51,7 +51,7 @@ type Query { product(id: ID): Product } -type Product { +type ProductInterface { id: ID! name: String! price: Money! @@ -128,7 +128,7 @@ It very rarely makes sense to have a resource that _can_ have an ID but might no IDs are extremely important for caching in most GraphQL clients, so it's worthwhile to be safe here. ```graphql -type Product { +type ProductInterface { id: ID! # Rarely makes sense for this to be nullable } ``` @@ -152,7 +152,7 @@ If you're not dealing with an `id` field or a top-level `Query` field, the most #### Example: Parent still usable with field error ```graphql -type Product { +type ProductInterface { # Recommended products are not critical data on a product page, and a UI can represent # a product safely without related products, so we keep the field nullable recommended_products: ProductRecommendations @@ -162,7 +162,7 @@ type Product { #### Example: Parent not usable with field error ```graphql -type Product { +type ProductInterface { # A user would not be able to add a product to the cart from the Product # details page if this field fails, because it may have required options. # We make the field's type non-nullable @@ -196,7 +196,7 @@ When deciding whether _List items_ should be nullable, the most important questi #### Example: Parent not usable if an item in List has an error ```graphql -type Product { +type ProductInterface { # Note: The "!" inside of the List ([]) means the list items are non-nullable # Making the list items non-nullable guarantees to the client that, if they receive # a list of product options, it will be complete/without errors @@ -207,7 +207,7 @@ type Product { #### Example: Parent still usable if an item in List has an error ```graphql -type Product { +type ProductInterface { # The absence of a "!" inside the list means that we could fail # to fetch a nested field in a related product, and it won't # impact our ability to render the rest of the product page diff --git a/design-documents/graph-ql/coverage/customer/Wishlist.graphqls b/design-documents/graph-ql/coverage/customer/Wishlist.graphqls index 1aeeb76ed..8e1f24df3 100644 --- a/design-documents/graph-ql/coverage/customer/Wishlist.graphqls +++ b/design-documents/graph-ql/coverage/customer/Wishlist.graphqls @@ -159,7 +159,7 @@ type CreateWishlistOutput { type DeleteWishlistOutput { status: Boolean! - wishlists: [Wishlist!]! + wishlists: [Wishlist!]! @doc(description: "A customer will always have at least one wishlist") } input WishlistItemCopyInput { @@ -190,3 +190,8 @@ type StoreConfig { maximum_number_of_wishlists: Int @doc(description: "If multiple wish lists are enabled, the maximum number of wish lists the customer can have") enable_multiple_wishlists: Boolean @doc(description: "Indicates whether customers can have multiple wish lists.") } + +#Allow Products to show assigned wishlists +type ProductInterface { + wishlists: [Wishlist]! @doc(description: "A product can be assigned to multiple wishlist of none") +} diff --git a/design-documents/graph-ql/coverage/customer/wishlist.md b/design-documents/graph-ql/coverage/customer/wishlist.md index d9be79843..746619ec5 100644 --- a/design-documents/graph-ql/coverage/customer/wishlist.md +++ b/design-documents/graph-ql/coverage/customer/wishlist.md @@ -111,3 +111,32 @@ type WishlistItem { } ``` This value was previously used for display only, other operations like update or delete are not implemented yet. + +## Exposure through product interface + +Products need to know which wishlists are they assigned to. Just like categories + +``` graphql +type ProductInterface { + wishlists: [Wishlist]! @doc(description: "A product can be assigned to multiple wishlist of none") +} +``` + +By default a product is not assigned to any wishlist + +### Considerations of performance versus graphql specs +Do we reference the Wishlist and create a minimal type based on what the UI would need or do we just output the whole Wishlist as a true graph would do? + +Alternatively we can do +``` graphql +type ProductInterface { + wishlists: [AssignedWishlist]! @doc(description: "A product can be assigned to multiple wishlist of none") +} +type AssignedWishlist { + wishlist_uid: ID! + name: String + number_of_items: Int +} +``` + +We can improve performance by "cutting the graph" and returning something fit for UI needs rather than loop through all the wishlist From 9ba2297666c57c4fdb0f8c56510072c7e71f7763 Mon Sep 17 00:00:00 2001 From: Cristian Partica Date: Thu, 25 Mar 2021 11:22:14 -0500 Subject: [PATCH 2/5] rewrite some text to be more clear --- design-documents/graph-ql/coverage/customer/wishlist.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/design-documents/graph-ql/coverage/customer/wishlist.md b/design-documents/graph-ql/coverage/customer/wishlist.md index 746619ec5..f9bbe7551 100644 --- a/design-documents/graph-ql/coverage/customer/wishlist.md +++ b/design-documents/graph-ql/coverage/customer/wishlist.md @@ -125,9 +125,12 @@ type ProductInterface { By default a product is not assigned to any wishlist ### Considerations of performance versus graphql specs -Do we reference the Wishlist and create a minimal type based on what the UI would need or do we just output the whole Wishlist as a true graph would do? +The UI really needs this to render a PDP page or Category with products page and list a dropdown of wishlists of which the product is assigned to. +It won't need all the data in wishlist for this purpose. However it would need all the wish lists available and only check the ones that the product belongs to. -Alternatively we can do +The question is: Do we reference the Wishlist and create a minimal type based on what the UI would need or do we just output the whole Wishlist as a true graph would do? + +Example: ``` graphql type ProductInterface { wishlists: [AssignedWishlist]! @doc(description: "A product can be assigned to multiple wishlist of none") From 2818c7cd764f4121fc3ae393077f9ccbe904b294 Mon Sep 17 00:00:00 2001 From: Cristian Partica Date: Thu, 25 Mar 2021 11:23:27 -0500 Subject: [PATCH 3/5] Update design-documents/graph-ql/coverage/customer/Wishlist.graphqls Co-authored-by: Andy Terranova <13182778+supernova-at@users.noreply.github.com> --- design-documents/graph-ql/coverage/customer/Wishlist.graphqls | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/design-documents/graph-ql/coverage/customer/Wishlist.graphqls b/design-documents/graph-ql/coverage/customer/Wishlist.graphqls index 8e1f24df3..57539ad90 100644 --- a/design-documents/graph-ql/coverage/customer/Wishlist.graphqls +++ b/design-documents/graph-ql/coverage/customer/Wishlist.graphqls @@ -193,5 +193,5 @@ type StoreConfig { #Allow Products to show assigned wishlists type ProductInterface { - wishlists: [Wishlist]! @doc(description: "A product can be assigned to multiple wishlist of none") + wishlists: [Wishlist]! @doc(description: "A product can be assigned to multiple wishlist or none") } From 3a5e12d0f61df9c28565cb9a1d6d47f7ae360557 Mon Sep 17 00:00:00 2001 From: Cristian Partica Date: Thu, 25 Mar 2021 16:57:46 -0500 Subject: [PATCH 4/5] - rewrite some text to be more clear --- .../coverage/customer/Wishlist.graphqls | 7 +++++- .../graph-ql/coverage/customer/wishlist.md | 22 +++++++++---------- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/design-documents/graph-ql/coverage/customer/Wishlist.graphqls b/design-documents/graph-ql/coverage/customer/Wishlist.graphqls index 57539ad90..c42023145 100644 --- a/design-documents/graph-ql/coverage/customer/Wishlist.graphqls +++ b/design-documents/graph-ql/coverage/customer/Wishlist.graphqls @@ -193,5 +193,10 @@ type StoreConfig { #Allow Products to show assigned wishlists type ProductInterface { - wishlists: [Wishlist]! @doc(description: "A product can be assigned to multiple wishlist or none") + wishlists: [WishlistReference]! @doc(description: "A product can be assigned to multiple wishlists, and by default is not assigned to any wishlist") +} + +type WishlistReference { + wishlist_uid: ID! + name: String } diff --git a/design-documents/graph-ql/coverage/customer/wishlist.md b/design-documents/graph-ql/coverage/customer/wishlist.md index f9bbe7551..ce3d7617e 100644 --- a/design-documents/graph-ql/coverage/customer/wishlist.md +++ b/design-documents/graph-ql/coverage/customer/wishlist.md @@ -114,32 +114,32 @@ This value was previously used for display only, other operations like update or ## Exposure through product interface -Products need to know which wishlists are they assigned to. Just like categories +When rendering a product you need to know which wishlists a particular product is assigned to. This is in a way just like categories. +Example: ``` graphql type ProductInterface { - wishlists: [Wishlist]! @doc(description: "A product can be assigned to multiple wishlist of none") + wishlists: [WishlistReference]! @doc(description: "A product can be assigned to multiple wishlist of none") +} +type WishlistReference { + wishlist_uid: ID! + name: String } ``` By default a product is not assigned to any wishlist -### Considerations of performance versus graphql specs +### Considerations of performance optimizations and usages The UI really needs this to render a PDP page or Category with products page and list a dropdown of wishlists of which the product is assigned to. -It won't need all the data in wishlist for this purpose. However it would need all the wish lists available and only check the ones that the product belongs to. +It won't need all the data in wishlist for this purpose. However, it would need all the wish lists available and only check the ones that the product belongs to. The question is: Do we reference the Wishlist and create a minimal type based on what the UI would need or do we just output the whole Wishlist as a true graph would do? Example: ``` graphql type ProductInterface { - wishlists: [AssignedWishlist]! @doc(description: "A product can be assigned to multiple wishlist of none") -} -type AssignedWishlist { - wishlist_uid: ID! - name: String - number_of_items: Int + wishlists: [Wishlist]! @doc(description: "A product can be assigned to multiple wishlist of none") } ``` -We can improve performance by "cutting the graph" and returning something fit for UI needs rather than loop through all the wishlist +By only going with a small subset of the Wishlist type (`WishlistRerefence`) can improve performance by "cutting the graph" and returning something fit for UI needs rather than loop through all the wishlist. From 67a7f67eb7369174cac42a86430d690042250d71 Mon Sep 17 00:00:00 2001 From: Cristian Partica Date: Thu, 25 Mar 2021 17:18:41 -0500 Subject: [PATCH 5/5] - rewrite some text to be more clear --- design-documents/graph-ql/coverage/customer/wishlist.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/design-documents/graph-ql/coverage/customer/wishlist.md b/design-documents/graph-ql/coverage/customer/wishlist.md index ce3d7617e..1d985ae4c 100644 --- a/design-documents/graph-ql/coverage/customer/wishlist.md +++ b/design-documents/graph-ql/coverage/customer/wishlist.md @@ -138,7 +138,7 @@ The question is: Do we reference the Wishlist and create a minimal type based on Example: ``` graphql type ProductInterface { - wishlists: [Wishlist]! @doc(description: "A product can be assigned to multiple wishlist of none") + wishlists: [Wishlist]! @doc(description: "A product can be assigned to multiple wishlist or now wishlist") } ```