[ongoing discussion] Draft for cart operations and other GraphQL improvements #15

paliarush · 2018-07-31T02:50:25Z

For the context see recording of previous GraphQL community meeting.

GraphQL project wiki: https://github.com/magento/graphql-ce/wiki

paales · 2018-07-31T11:39:19Z

design-documents/graph-ql/coverage/add-items-to-cart/AddConfigurableProductToCart.graphqls

+    quantity: Float!
+    configurable_options:[ConfigurableOptionInput!]!
+    customizable_options:[CustomizableOptionInput!]
+}


Can't configurable_options be replaced by a single simple_sku: Sku! field?

If we go with simple_sku, then the client on its end will have to map selected options combination to the SKU:

Get a map during product request (option1, option2, optionN => simple SKU, ...)

Calculate current simple SKU based on selected options

Send simple SKU along with add product to cart request

Looks like more work for client app developers. I am probably missing something, please clarify why simple_sku is preferable.

Thanks for the review!

I think there already is some form of simple_sku mapping in the frontend, especially if a frontend developer wishes to fetch images from the simple product etc. Or are there other strategies in that regard?

@paales Can you help by providing the motivation for using simple_sku in lieu of sending along options?

AFAICT, it seems like, in any situation you'd be adding an item to a cart, you'd always have a list of the options (since the shopper needs to select them). So it feels like needing to track a simple_sku on the client-side might be work that is unnecessary. Granted, it's very little extra work, but I'm curious what the motivation would be for that > options list.

In the past I've always found it much easier to supply a sku rather than a list of options that map to a simple product sku. I agree with @paales that usually the sku is already known on the frontend.

DrewML · 2018-07-31T18:37:52Z

design-documents/graph-ql/coverage/add-items-to-cart.md

@@ -0,0 +1,49 @@
+**Overview**
+
+As a Magento developer, I need to manipulate the shopping cart via GraphQL so that I can build basic ecommerce experiences for shoppers on the front-end using only GraphQL.


for shoppers on the front-end using only GraphQL.

This is a big nitpick, so...sorry!

I think it's super important that we avoid language that couples the concept of GraphQL in Magento to the front-end specifically.

I know it's a small thing, but we want to make sure everyone has the mindset that this is an API for any external consumer.

Please suggest how to stress, that GraphQL in Magento is intended for store front scenarios only, not for admin ones.

How about:

As a Magento developer, I need to manipulate the shopping cart via GraphQL so that I can programmatically create orders on behalf of a shopper.

Thoughts?

DrewML · 2018-07-31T18:43:19Z

design-documents/graph-ql/coverage/add-items-to-cart.md

+- Can create "order in one call" mutation in the future if needed
+- Hashed IDs for cart items
+- Single input object
+- Async nature of the flow must be supported on the client side (via AJAX calls)


Might want to expand on this point more, since all requests from the browser are async (barring sync XMLHTTPRequest, which is deprecated).

Maybe add some additional language around this clarifying that it creates a "job" of sorts, which helps imply that the results of the mutation will be some object representing a task that will be finished some time in the future.

Asynchronous server side implementation is not approved yet. The schema may change, if it will be approved.

DrewML · 2018-07-31T18:44:13Z

design-documents/graph-ql/coverage/add-items-to-cart.md

+
+**Open questions:**
+
+- Do we want to implement server-side asynchronous mutations by default?


By default for cart mutations, or for all mutations?

@akaplya suggested for all mutations. Still under discussion.

For a server less architecture as we have in CIF this might not work OOTB.

The decision is to have Synchronous implementation by default. Asynchronous implementation may be added later on the framework level as it was for REST.

DrewML · 2018-07-31T18:49:02Z

design-documents/graph-ql/coverage/add-items-to-cart.md

+- Can create "order in one call" mutation in the future if needed
+- Hashed IDs for cart items
+- Single input object
+- Async nature of the flow must be supported on the client side (via AJAX calls)


I think it's important to include why there is a desire for these mutations to be async. When we chatted briefly yesterday, I got the impression that it's because this could be a long-running operation, and we don't want to keep the request open. Is that accurate?

It's just worth making the justification clear, because a concept of a "job" that needs to be polled (or a subscription) pushes some complexity to the consumer of the API.

Not necessarily long-running, may be just a lot of concurrent requests during peak hours. Decision on the async server side is not made yet.

masodu · 2018-08-01T17:56:56Z

design-documents/graph-ql/coverage/add-items-to-cart/AddSimpleProductToCart.graphqls

+    qty: Float!
+    product: CartItemProduct!
+    image: CartItemImage!
+    prices: CartItemPrices!


How/where are the calculated cartItem prices like: prices with(out) taxes? Or the applied discounts?

masodu · 2018-08-01T17:59:58Z

design-documents/graph-ql/coverage/add-items-to-cart/AddSimpleProductToCart.graphqls

+    name: String!
+    type: String!
+    # Does it make sense to provide several basic fields (with potentially better performance) in addition to the ful product object?
+    details: ProductInterface!


This should depend on the client so I am in favour of adding it to the cartItem. I guess that most clients will just query for some fields and not for the entire ProductInterface (not sure if it matters for performance).

masodu · 2018-08-01T18:03:22Z

design-documents/graph-ql/coverage/add-items-to-cart/AddSimpleProductToCart.graphqls

+
+type CartItemImage {
+    alt_text: String!
+    thumbnail: ProductMediaGalleryEntriesContent!


Probably out of scope the current PR scope but would be really nice to have the absolute url here.

I think URL is already part of ProductInterface, what if we remove cart item image and just leave reference to ProductInterface?

masodu · 2018-08-01T18:06:31Z

design-documents/graph-ql/coverage/add-items-to-cart/AddSimpleProductToCart.graphqls

+type Cart {
+    id: String
+    items: [CartItemInterface]
+}


Missing the cart totals, currency, updated dates and all the other information. I know that this is not in the scope but my suggestion is to add all these data to cart definition ASAP to make things clear. Btw how can a client be sure that the cart he is currently updating is the last one? The use case here is when the same cart is updated by the end user and also by a back office.

The cart is updated by ID and there is only one active cart per customer at a time. If there are conflicting writes to the same cart (from store front and from the admin), then the last one wins. This should not happen in normal use cases.

masodu · 2018-08-01T18:51:40Z

design-documents/graph-ql/coverage/add-items-to-cart.md

+
+**Open questions:**
+
+- Do we want to implement server-side asynchronous mutations by default?


For a server less architecture as we have in CIF this might not work OOTB.

… using ProductInterface

misha-kotov · 2018-08-03T18:48:46Z

design-documents/graph-ql/coverage/CouponOperations.graphqls

+    code: String!
+}
+
+input CancelCouponForCartInput {


suggest "remove coupon from cart", as in "RemoveCouponFromCartInput", "RemoveCouponFromCartOutput"

masodu · 2018-08-08T01:49:55Z

design-documents/graph-ql/coverage/CartPrices.graphqls

+}
+
+type CartAddress {
+    pices: CartAddressTotals


Is this the CartAddressPrices? Also, note the small typo, the field name should be prices

Why plural?

I think plural makes sense because there are multiple price types stored in this field.
Do you strongly believe it should be singular in this case?

Not really, but there's this small inconsistency where you have a plural to designate lists but here is a single object. Why don't name it totals?

masodu · 2018-08-08T01:52:09Z

design-documents/graph-ql/coverage/CartPrices.graphqls

@@ -0,0 +1,57 @@
+interface CartItemInterface {
+    prices: CartItemTotals


Where is the CartItemTotals defined?

Good catch, I called the type CartItemTotals first and then renamed it to CartItemPrices, forgot to change here.

masodu · 2018-08-08T01:53:31Z

design-documents/graph-ql/coverage/CartPrices.graphqls

+}
+
+type Cart {
+    prices: CartTotals


Where is the CartTotals defined?

Vinai · 2018-08-15T22:30:22Z

design-documents/graph-ql/coverage/Cart.graphqls

+    suffix: String
+    gender: GenderEnum
+    date_of_birth: String
+    vat_number: String # Do we need it at all on storefront? Do we need more details


The presence of a vat_number can influence the rendering in many ways, so I'd say yes.

- Improved extensibility for cart item mutations - Added cart item removal and update schemas

…array of options

misha-kotov · 2018-09-19T16:59:18Z

design-documents/graph-ql/coverage/CartAddressOperations.graphqls

+    postcode: String
+    country_code: String!
+    telephone: String!
+    fax: String


do we really need fax anymore? I would get rid of it if we can do it without breaking things

misha-kotov · 2018-09-19T18:08:16Z

design-documents/graph-ql/coverage/CartAddressOperations.graphqls

+}
+
+input AddShippingAddressToCartInput {
+    address_id: Int # Can be provided in one-page checkout and is required for multi-shipping checkout


Have we accounted for multishipping scenarios here? I don't yet see how we tie cart items with shipping addresses, in case there are multiple

misha-kotov · 2018-09-19T18:45:14Z

design-documents/graph-ql/coverage/CartAddressOperations.graphqls

+    addShippingAddressToCart(input: AddShippingAddressToCartInput): AddShippingAddressToCartOutput
+}
+
+input AddBillingAddressToCartInput {


Suggestion: rename to setBillingAddressOnCart (to show that there can only be one)

misha-kotov · 2018-09-19T18:56:10Z

design-documents/graph-ql/coverage/CartAddressOperations.graphqls

+input AddShippingAddressToCartInput {
+    address_id: Int # Can be provided in one-page checkout and is required for multi-shipping checkout
+    address: CartAddressInput
+    shipping_method_code: String # Should be on address level for multi-shipping checkout


Needs to be a separate mutation

irenelagno · 2018-09-19T19:09:05Z

design-documents/graph-ql/coverage/CartAddressOperations.graphqls

+}
+
+input CartAddressInput {
+    firstname: String!


How are we going to add customer address attributes?

irenelagno · 2018-09-19T19:16:25Z

design-documents/graph-ql/coverage/CartAddressOperations.graphqls

+}
+
+type CartAddress {
+    firstname: String!


how are we going to set information about customer address attributes

irenelagno · 2018-09-19T19:17:05Z

design-documents/graph-ql/coverage/CartAddressOperations.graphqls

+    BILLING
+}
+
+type CheckoutShippingMethod {


I think we also need to add information about the error message, it can be displayed if in the system configuration we choose carriers/dhl/showmethod = 1

case code/label become optional if error message is displayed (setting "show method if not applicable")

irenelagno · 2018-09-19T19:17:59Z

design-documents/graph-ql/coverage/CartAddressOperations.graphqls

+
+type CheckoutShippingMethod {
+    code: String!
+    label: String!


Also, online shipping methods can have different rates, how are we going to display this information?

misha-kotov · 2018-09-19T19:38:48Z

design-documents/graph-ql/coverage/Cart.graphqls

+type CheckoutPaymentMethod {
+    code: String!
+    label: String!
+    # Do we need more info from quote_payment DB table?


need to add "amount" or "balance" for things like store credit, gift card, reward pts

misha-kotov · 2018-09-19T19:44:44Z

design-documents/graph-ql/coverage/Cart.graphqls

+type CheckoutPaymentMethod {
+    code: String!
+    label: String!
+    # Do we need more info from quote_payment DB table?


Add sort order

misha-kotov · 2018-09-19T19:58:56Z

design-documents/graph-ql/coverage/Cart.graphqls

+type CheckoutPaymentMethod {
+    code: String!
+    label: String!
+    # Do we need more info from quote_payment DB table?


handle payment method-specific field such as "instructions", "payable to", "send check to", etc

misha-kotov · 2018-09-19T20:02:04Z

design-documents/graph-ql/coverage/Cart.graphqls

+type CheckoutPaymentMethod {
+    code: String!
+    label: String!
+    # Do we need more info from quote_payment DB table?


consider "terms and conditions" and "billing agreements"

misha-kotov · 2018-09-19T20:02:49Z

design-documents/graph-ql/coverage/Cart.graphqls

+type CheckoutPaymentMethod {
+    code: String!
+    label: String!
+    # Do we need more info from quote_payment DB table?


consider gift messaging, gift wrapping, gift receipt

irenelagno · 2018-09-19T20:05:17Z

design-documents/graph-ql/coverage/Cart.graphqls

@@ -0,0 +1,58 @@
+type Query {


please add information

about shipping address if we add an item from gift registry;

how can we set gift messages, gift wrapping, printed cart information;

how can we set checkout agreements
<extension_attributes for="Magento\Quote\Api\Data\PaymentInterface">  <attribute code="agreement_ids" type="string[]" /> </extension_attributes>

Also, if graphQL be compatible with B2B edition

real34 · 2018-09-20T14:53:48Z

design-documents/graph-ql/coverage/Cart.graphqls

+    is_virtual: Boolean!
+}
+
+type CheckoutCustomer {


It would be great to use polymorphism here.

interface CheckoutCustomer { # the mandatory fields } type GuestCheckoutCustomer implements CheckoutCustomer { # … middle_name: String } type LoggedInCheckoutCustomer implements CheckoutCustomer { # … middle_name: String! }

React components could then require different fragments depending on the type

Good idea in general. I don't understand though, why middle_name should be required for the registered customer. The only difference I see right now is that we can eliminate is_guest field in this way.

Please clarify which exact fields should be different between guest and customer implementations.

I agree that the middle_name is not a good example. I meant user information mandatory upon registration, such as email for instance.

The idea of having such a structure, is to open rooms for extension in the shop. For instance in a B2B context, a LoggedInCheckoutCustomer may have a sales_representative: SalesPerson! attached which would not be available for guests.
Developers would then only extend the LoggedInCheckoutCustomer type, making implementation more explicit.

real34 · 2018-09-20T14:54:32Z

design-documents/graph-ql/coverage/Cart.graphqls

+    middle_name: String
+    suffix: String
+    gender: GenderEnum
+    date_of_birth: String


Are there any plans for a scalar Date type in the core?

We do not have plans for that, but I know that this topic was discussed in scope of Webonyx project: webonyx/graphql-php#228

Ok, thank you.

real34 · 2018-09-20T14:54:45Z

design-documents/graph-ql/coverage/Cart.graphqls

+    code: String!
+    label: String!
+    balance: Money
+    sort_order: Int


Is there a use case for exposing this value (vs having sort params in queries returning this type)?

We thought of a use case when the client app needs to honor sort order of available payment methods during rendering.

I think this pattern is better to be delegated to the server logic. By default available_payment_methods would return sorted values.

If a use case where methods should be sorted differently, it could lead to a different signature with an optional param:

available_payment_methods(sortDesc: Boolean)

naydav · 2018-09-25T12:49:48Z

design-documents/graph-ql/coverage/add-items-to-cart/AddBundleProductToCart.graphqls

+}
+
+input UpdateBundleProductCartItemInput {
+    details: UpdateCartItemDetailsInput!


Maybe it will be better to use data instead of details

data: CartItemInput!

naydav · 2018-09-26T13:17:28Z

design-documents/graph-ql/coverage/add-items-to-cart/AddSimpleProductToCart.graphqls

+
+type SelectedCustomizableOptionValue {
+    id: Int
+    label: String!


What is label for SelectedCustomizableOptionValue ?
Is the value entered / selected by the user?

naydav · 2018-09-26T13:18:50Z

design-documents/graph-ql/coverage/add-items-to-cart/AddSimpleProductToCart.graphqls

+type SelectedCustomizableOption {
+    id: Int!
+    label: String!
+    type: String!


Do we need to add Required flag value?

naydav · 2018-09-26T13:44:33Z

design-documents/graph-ql/coverage/add-items-to-cart/AddSimpleProductToCart.graphqls

+type SelectedCustomizableOptionValue {
+    id: Int
+    label: String!
+    price: CartItemSelectedOptionValuePrice!


Do we need price info?

Maybe id and value will be enough for the response about selected options

{ "id": 14, "value": "test", }

naydav · 2018-09-26T15:27:56Z

design-documents/graph-ql/coverage/add-items-to-cart/AddSimpleProductToCart.graphqls

+}
+
+input UpdateSimpleProductCartItemInput {
+    details: UpdateCartItemDetailsInput!


Why do we need one more level like UpdateCartItemDetailsInput

Now request is

cartItems: [ { data: { sku: "product_dynamic_24494", qty: 1 }, customizable_options: [ {id: 5, value: "test"}, ] } ]

Can we use next?

cartItems: [ { sku: "product_dynamic_24494", qty: 1, customizable_options: [ {id: 5, value: "test"}, ] } ] ``

…t-schema

…art-schema

buskamuza · 2018-10-19T16:06:48Z

Merged the initial document.
Please process any remaining questions in smaller documents that modify the original one.

Initial draft for add items to cart GraphQL mutations

d8e9846

paales reviewed Jul 31, 2018

View reviewed changes

DrewML reviewed Jul 31, 2018

View reviewed changes

Update AddGiftCardProductToCart.graphqls

30ef22f

masodu reviewed Aug 1, 2018

View reviewed changes

paliarush mentioned this pull request Aug 2, 2018

[Cart Operations] Fetch guest shopper cart magento/graphql-ce#8

Closed

paliarush added 2 commits August 2, 2018 09:52

Eliminated CartItemProduct and CartItemImage. All data can be fetched…

dc06523

… using ProductInterface

Added coupon codes support

d638fa9

misha-kotov reviewed Aug 3, 2018

View reviewed changes

paliarush added 4 commits August 3, 2018 14:24

Renamed coupon removal mutation

902e999

Reworded "overview" in the readme

43829b3

Proposals for input and output wrappers

7d71efc

Schema for cart prices

83aa880

masodu reviewed Aug 8, 2018

View reviewed changes

paliarush added 6 commits August 8, 2018 09:57

Fixed typos in schema for cart prices

c6f7d84

Added basic fields for cart type

47b9607

Adjusted cart schema

bdd70b4

Adjusted cart schema

c5aaccb

Proposal to change resolver interface return type

c87159c

Proposal to change resolver interface return type

f2ca044

paliarush changed the title ~~Initial draft for add items to cart GraphQL mutations~~ Initial draft for cart mutations and other GraphQL improvements Aug 15, 2018

Adjusted cart schema

10ce77e

Vinai reviewed Aug 15, 2018

View reviewed changes

paliarush added 4 commits August 16, 2018 09:23

Adjusted cart schema

5cc770f

Adjusted cart schema

b0bc80e

- Improved extensibility for cart item mutations - Added cart item removal and update schemas

Proposal for input-output extensibility

fa08ab1

Proposal for input-output extensibility

5a831aa

Adding configurable products to cart using variant SKU instead of an …

a940475

…array of options

paliarush mentioned this pull request Sep 12, 2018

Design schema for checkout operations magento/graphql-ce#181

Closed

Added schema for cart address operations

299809a

misha-kotov reviewed Sep 19, 2018

View reviewed changes

irenelagno reviewed Sep 19, 2018

View reviewed changes

misha-kotov reviewed Sep 19, 2018

View reviewed changes

irenelagno reviewed Sep 19, 2018

View reviewed changes

Iterated on schema for cart address operations

9635092

real34 reviewed Sep 20, 2018

View reviewed changes

naydav reviewed Sep 25, 2018

View reviewed changes

naydav reviewed Sep 26, 2018

View reviewed changes

paliarush added 2 commits October 19, 2018 10:59

Merge remote-tracking branch 'remotes/origin/master' into graphql-car…

f0e011a

…t-schema

Merge remote-tracking branch 'remotes/mainline/master' into graphql-c…

ca3a063

…art-schema

buskamuza merged commit 646fc6f into magento:master Oct 19, 2018

		@@ -0,0 +1,49 @@
		Overview

		As a Magento developer, I need to manipulate the shopping cart via GraphQL so that I can build basic ecommerce experiences for shoppers on the front-end using only GraphQL.


		Open questions:

		- Do we want to implement server-side asynchronous mutations by default?

		@@ -0,0 +1,57 @@
		interface CartItemInterface {
		prices: CartItemTotals

[ongoing discussion] Draft for cart operations and other GraphQL improvements #15

[ongoing discussion] Draft for cart operations and other GraphQL improvements #15

Uh oh!

Conversation

paliarush commented Jul 31, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DrewML Aug 3, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DrewML Jul 31, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

misha-kotov Aug 3, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

paliarush commented Jul 31, 2018 •

edited

Loading

DrewML Aug 3, 2018 •

edited

Loading

DrewML Jul 31, 2018 •

edited

Loading

misha-kotov Aug 3, 2018 •

edited

Loading