-
Notifications
You must be signed in to change notification settings - Fork 154
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
ECP-765: GraphQL Schema for Configurable Options Selection #394
Changes from all commits
f8601df
e5783d8
ee4e5a0
355296a
627ee5f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
type ConfigurableProduct { | ||
configurable_options_selection_metadata(selectedConfigurableOptionValues: [ID!]): ConfigurableOptionsSelectionMetadata @doc(description: "Metadata for the specified configurable options selection") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It was pretty hard to determine what the API’s should do. The naming didn’t make it clear what the It seems the return value of
Not sure if this is the best name or we can figure out something more descriptive? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Paul, thanks for reviewing. We can extend description of the field. Please check https://github.com/magento/architecture/pull/394/files?short_path=567352f#diff-567352fc8df3d0073eac2ea540e6f57f and let me know what would you like to include in the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, regarding There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yes, that is true that it returns some form of metadata, but following that logic you can also call it: In my opinion the word |
||
} | ||
|
||
type ConfigurableOptionsSelectionMetadata @doc(description: "Metadata corresponding to the configurable options selection.") | ||
{ | ||
options_available_for_selection: [ConfigurableOptionAvailableForSelection!] @doc(description: "Configurable options available for further selection based on current selection.") | ||
media_gallery: [MediaGalleryInterface!] @doc(description: "Product images and videos corresponding to the specified configurable options selection.") | ||
variant: SimpleProduct @doc(description: "Variant represented by the specified configurable options selection. It is expected to be null, until selections are made for each configurable option.") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @paliarush should this be an array of simple products? Also, is it possible to have a virtual/bundled/grouped product as a variant? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please check Can configurable variant be of any types other than simple? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @paliarush assuming we can still get to a simple product using default values for all unselected options. Use case: a color selection from the swatch should change the image on the UI. cc: @DrewML There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If they are default, client will explicitly pass them to GraphQL. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
@nrkapoor my understanding is that Magento 2 itself does not have a concept of a "default value" for configurable options. Basically, the client will be able to get a list of all options available based on current selections, and it'll be up to the client to choose the strategy for "default", based on the data they have. So this schema does unlock that feature, but it'll be up to the client to define some of their own logic around it. There are extensions in the Marketplace that add defaults - I think they'll be able to extend our schema to support that (guessing they'd add a field to |
||
} | ||
Comment on lines
+5
to
+10
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I like the possible extension point here to add delivery date information, etc. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's discuss in the comments above since it is all related: #394 (comment) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't follow how that comments answers the |
||
|
||
type ConfigurableOptionAvailableForSelection @doc(description: "Configurable option available for further selection based on current selection.") { | ||
available_value_ids: [ID!]! @doc(description: "Configurable option values available for further selection.") | ||
attribute_code: String! @doc(description: "Attribute code that uniquely identifies configurable option.") | ||
} | ||
|
||
# Configurable option value type has to be extended to include ID which can be used to uniquely identify the option value across the system. This is consistent with proposal of single mutation for add-to-cart | ||
type ConfigurableProductOptionsValues { | ||
id: ID! | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just to verify, this ID includes a SKU in it so there won't be collisions, right? Want to make sure 2 products can't create the same There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it should, based on discussions in other PR related to IDs. |
||
is_available_for_selection: Boolean! | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,211 @@ | ||
## Use cases | ||
|
||
### Render configurable option values available for selection on the product page | ||
|
||
User navigates to the configurable product page. Option values available for selection are rendered on the page. | ||
|
||
```graphql | ||
{ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've been looking at this query for a bit, and trying to think through how folks like @magento/pwa-studio-team are going to consume this and render it in a UI. It looks like the UI will do something like this:
That nested looping is necessary because What would the impact be if we moved Idea 1With this model, clients always get back a list of all option values, and can do their own filtering to decide whether they're shown or not. This supports the UX where a user can see a green shirt has a size "small," but it's not available. {
products(filter: {sku: {eq: "sku-here"}}) {
...on ConfigurableProduct {
configurable_options(selectedOptionValueIDs: [ID!]) {
attribute_code
label
values {
id # ID can be checked against `available_value_ids`
}
available_value_ids
}
}
}
} Idea 2With this model, client can either select a list of all option values, or stick to just the values currently available to the client. This supports the UX where options that aren't available just aren't rendered. {
products(filter: {sku: {eq: "sku-here"}}) {
...on ConfigurableProduct {
configurable_options(selectedOptionValueIDs: [ID!]) {
attribute_code
label
# Client can select this if they want a UI that shows greyed-out, unavailable options
values {
id
}
# Same return type as `values` field, but limited to just values allowed.
# Useful for UIs that don't render unavailable options at all
availableValues {
id
}
}
}
}
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As mentioned in other comments above, in this case we are loosing the "Metadata" aspect and extensibility. My understanding was that on the first request (with no selections made), the client will only be using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This is my understanding as well, so far on the same page 👍
Cool 👍 I like the idea.
If some fields aren't necessary on the second request (
It's Monday and I'm not running on all cylinders yet 😅 - can you give an example of the extensibility we lose this way? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added Regarding extensibility, I mean that the fields like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
My biggest concern is still around how ergonomic this is for UI developers to consume, which is the API's primary audience. In a typical modern front-end, things are broken up by components, and you just describe how you want the UI to look by composing those components. Let's step a bit back from this specific schema, and talk more in terms of how the UI code would be structured. In this case, the UI code would ideally just want to loop over the options groups (color/size/etc), and then render the options with that dataset. Note: Code below uses less abstract names than our schema, to try and make the example as clear as possible // Something like this using JSX, but pretty similar in a template-based world like Vue
const options = option_groups.map(option =>
<OptionGroup
attrCode={option.attribute_code}
label={option.label}
values={option.values}
/>
); Then, the With the currently proposed schema, if the UI wanted to do this, it would have to either have to normalize data into a lookup table every time a response comes in, or do something like this: const options = options_groups.map(option =>
<OptionGroup
attrCode={option.attribute_code}
label={option.label}
values={option.values}
// This nested loop could be several hundred iterations for each option group,
// and just isn't super fun to write
availableValues={options.find(o => o.attribute_code === option.attributeCode)}
/>
); There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Having said all of that, if you can't see an alternative schema design working, we can always start with this and change course or add later if we find usability problems. |
||
products(filter: {sku: {eq: "configurable-sku"}}) { | ||
items { | ||
description { | ||
html | ||
} | ||
name | ||
... on ConfigurableProduct { | ||
configurable_options { | ||
attribute_code | ||
label | ||
values { | ||
id | ||
is_available_for_selection | ||
value_index | ||
label | ||
swatch_data { | ||
value | ||
} | ||
use_default_value | ||
} | ||
} | ||
configurable_options_selection_metadata { | ||
options_available_for_selection { | ||
attribute_code | ||
available_value_ids | ||
} | ||
media_gallery { | ||
url | ||
label | ||
position | ||
disabled | ||
} | ||
} | ||
} | ||
} | ||
} | ||
} | ||
|
||
``` | ||
|
||
The user makes a selection for the first option and the list of option values available for selection is updated for the remaining options. | ||
The images and videos relevant for the selection are also updated. | ||
|
||
```graphql | ||
{ | ||
products(filter: {sku: {eq: "configurable-sku"}}) { | ||
items { | ||
... on ConfigurableProduct { | ||
configurable_options_selection_metadata( | ||
selectedConfigurableOptionValues: ["hash from selected option value"] | ||
) { | ||
options_available_for_selection { | ||
attribute_code | ||
available_value_ids | ||
} | ||
media_gallery { | ||
url | ||
label | ||
position | ||
disabled | ||
} | ||
} | ||
} | ||
} | ||
} | ||
} | ||
``` | ||
|
||
### User opens URL leading to configurable product page and configurable option selections are specified in the URL | ||
|
||
In this case URL will have to be resolved first: | ||
|
||
```graphql | ||
{ | ||
urlResolver(url: "http://magento.instance/configurable_product.html?configurable_options[0]=first-selection-hash&configurable_options[1]=second-selection-hash") { | ||
id | ||
type | ||
} | ||
} | ||
``` | ||
|
||
Then the product data along with available selections can be requested in a single query: | ||
|
||
```graphql | ||
{ | ||
products(filter: {sku: {eq: "resolved-sku"}}) { | ||
items { | ||
description { | ||
html | ||
} | ||
name | ||
... on ConfigurableProduct { | ||
configurable_options { | ||
attribute_code | ||
label | ||
values { | ||
id | ||
is_available_for_selection | ||
value_index | ||
label | ||
swatch_data { | ||
value | ||
} | ||
use_default_value | ||
} | ||
} | ||
configurable_options_selection_metadata( | ||
selectedConfigurableOptionValues: ["hash from selected option value", "hash from another option value"] | ||
) { | ||
options_available_for_selection { | ||
attribute_code | ||
available_value_ids | ||
} | ||
media_gallery { | ||
url | ||
label | ||
position | ||
disabled | ||
} | ||
variant { | ||
sku | ||
} | ||
} | ||
} | ||
} | ||
} | ||
} | ||
``` | ||
|
||
### Add to cart | ||
|
||
After the user makes final selection, the corresponding simple product data becomes available and the product can now be added to cart. | ||
|
||
```graphql | ||
{ | ||
products(filter: {sku: {eq: "configurable-sku"}}) { | ||
items { | ||
... on ConfigurableProduct { | ||
configurable_options_selection_metadata( | ||
selectedConfigurableOptionValues: ["hash from selected option value", "hash from another option value"] | ||
) { | ||
options_available_for_selection { | ||
attribute_code | ||
available_value_ids | ||
} | ||
media_gallery { | ||
url | ||
label | ||
position | ||
disabled | ||
} | ||
variant { | ||
sku | ||
} | ||
} | ||
} | ||
} | ||
} | ||
} | ||
``` | ||
|
||
Information about variant is taken from previous query result and used to add configurable product to cart. | ||
|
||
### Render configurable option values available for selection on the category page | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I think this might be an unsafe assumption. In a headless world we want to avoid limiting use-cases, especially ones that would allow a shopper to get a product into the cart faster. Is the idea to not support that at all, or that folks would have to go back to using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is not possible to provide selections in category/search queries, unless I am missing something. This is mainly the reason why I decided to have a dedicated query for option selections instead of making them a field of configurable product. The only way to use them would be with empty selections, which does not make a lot of sense because we already have access to full set of option values (not only those available for selection, but all of them). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Not quite talking about including it in the query used for rendering the results page. Here is the general use-case I'm talking about:
As a shopper this is an experience familiar to me on ecommerce websites, and as a front-end dev it's an experience I'd like the API to make possible. FWIW, I'm not saying your API doesn't support that, more just commenting on us explicitly documenting that we don't want to support this flow, which I think is fairly typical for ecomm.
Do we expect this to always be true? I'd assume that a search service would support faceted search (cc @kokoc), and I believe facets are driven by attributes? If a search filtered to only include the "Color" attribute with value "Green," I think the client should ideally be able to pre-select that and get back the other options after color selection (like available sizes). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. On the category use case agree, we can clarify that your use case is supported by the new query. What I meant is that product listing query itself should not support fetching available selections, because all possible selections can be displayed there. Regarding faceted search, this information may be available in the facet section. Again I don't see a use case why and how we can implement it on the level of the specific product returned in search results. To me this is the same use case as for category and I would not separate them. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
It's likely I'm explaining the facet filtering poorly, but I do think this is a separate use-case. Let me try and expand more. Imagine that this screenshot of Luma is from a headless store built entirely against GraphQL. I'm a shopper and I open up the filters on the left and choose "Red." This will limit my results to shorts that come in red, but the result set will still have other colors, too. At this point, the UI knows that I'm trying to drill down to get a red product in my cart. A sufficiently smart UI should now be able to pre-select "red" on each tile for me, s the sizes displayed on each tile are limited to those available in red (because red should be selected). Keeping the query attached to the query FacetedCategoryResults($categoryID: ID, $selectedOptionIDs: [ID!]!) {
# Note: Only selecting one category here, but looks weird cause we don't have
# a root query to return a single category
categories(filters: { ids: [$categoryID] }) {
items {
...on ConfigurableProduct {
# Field available on ConfigurableProduct type allows us to do this in 1 query
options_values(selectedOptionIDs: $selectedOptionIDs) {
# pick out fields needed for UI
}
}
}
}
} If we make available options for a selection only available as a root query, the client would have to do N+1 requests to enable this same flow. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you for explanation, now your use case is clear. In this case we will have to think how it will work in other scenarios involving @nrkapoor do we want to go this route to support additional use cases like Andrew described above? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, have to give credit for the use-case to @soumya-ashok. She had this idea for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @DrewML Thanks for remembering and surfacing the idea we'd discussed. Additionally, if the listing is clicked on to go the PDP, the applied color filter could be the automatic selection. The same should apply for size or any other filtered parameter. This should be the more common pattern in Commerce experiences, but surprisingly isn't! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed |
||
|
||
In case when the facet filter was used on the category page, for example to search "Red" shorts, it would be a good idea to display available sizes in "Red" for each product on the page. This can be achieved with the following query: | ||
|
||
```graphql | ||
{ | ||
products(filter: {category_id: {eq: "shorts category ID"}}) { | ||
items { | ||
name | ||
sku | ||
... on ConfigurableProduct { | ||
configurable_options_selection_metadata( | ||
selectedConfigurableOptionValues: ["hash from selected red color option"] | ||
) { | ||
options_available_for_selection { | ||
attribute_code | ||
available_value_ids | ||
} | ||
media_gallery { | ||
url | ||
label | ||
position | ||
disabled | ||
} | ||
} | ||
} | ||
} | ||
} | ||
} | ||
``` | ||
|
||
### Extension points | ||
|
||
`ConfigurableOptionsSelectionMetadata` type can be extended to support additional use cases, which are not currently supported by Magento like: | ||
- Price range for the variants based on configurable options selection | ||
- Low stock notification based on configurable options selection | ||
|
||
### Long term vision | ||
|
||
In the future all option types will be unified to support additional use cases like conflicting custom options, or price range based on custom + configurable options selection. The new query will be introduced on the top level, and current solution being specific to configurable options only will be deprecated. |
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.
Similar Comment
Maybe we can drop the
configurable
prefix here? If you're working with aConfigurableProduct
, you typically are fetching it from a field usingProductInterface
, so a client query already has to include...on ConfigurableProduct
.Based on that, I think we can drop the
configurable_
prefix, because it's assumed that a configurable product has configurable options.Thoughts?
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.
Se response in another related comment #394 (comment)