Skip to content
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

Add wishlists in products query as a graph #482

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions design-documents/graph-ql/best-practice/nullability.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ type Query {
product(id: ID): Product
}

type Product {
type ProductInterface {
id: ID!
name: String!
price: Money!
Expand Down Expand Up @@ -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
}
```
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
12 changes: 11 additions & 1 deletion design-documents/graph-ql/coverage/customer/Wishlist.graphqls
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -190,3 +190,13 @@ 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: [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
}
32 changes: 32 additions & 0 deletions design-documents/graph-ql/coverage/customer/wishlist.md
Original file line number Diff line number Diff line change
Expand Up @@ -111,3 +111,35 @@ type WishlistItem {
}
```
This value was previously used for display only, other operations like update or delete are not implemented yet.

## Exposure through product interface

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: [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 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.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@supernova-at how do you feel about this? we would lean to not expose the whole wishlist type and create a loop

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I'd lean that way too. I suppose I could envision a scenario where the UI would want to show the names of the wishlists that the product belonged to, but I think it's perfectly reasonable to have the front end make an additional query for the full wishlist data.

As long as the minimal type would give us enough information to look up the full wishlist, I think that's fine 👍 .

For example, type gives wishlistID and then there's a way to get wishlists by wishlistID.

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: [Wishlist]! @doc(description: "A product can be assigned to multiple wishlist or now 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.