Cart checkout schemas

[aldeed]

Impact: minor
Type: feature

Changes

Add Cart checkout related GraphQL schemas.

Breaking changes

NONE

[aldeed]

@spencern @ticean I have this all cleaned up and it's making sense to me. We didn't flesh out the shipping quotes stuff, but I'm ok with merging this and discussing that next.

[spencern]

@ticean did you have comments on this schema PR before we merge?

[ticean]

This looks good. Just one comment on a specific example that could maybe be made more generic and accurate for implementation.

[ticean]

This is an example but in a hard type definition. Does it make sense to define a Union PaymentMethodData along with a StripePaymentMethodData? Would that the best way to define a valid schema for this now that can evolve as new payment methods are added? What do you think, @aldeed?

[aldeed]

Maybe. I was planning to punt this part until we do a payment plugin and see what makes sense. In the future microservice world, I figured we'd have a default type here and the payment service would overwrite that type with one that is a union of all registered payment plugins.

The main point of confusion for me is: what should the schema have as the type prior to stitching in the payment service schema? The best solution I can think of would be making the type JSONObject in the base schema because to the cart, it is just a black box. Does that sound right, @ticean?

[ticean]

Yes, the stitching by the payment system sounds good to me. I hope we can avoid all new instances of JSONObject.

UNION is a good fit here because we can't define inheritable fields on them. So we don't need to define some base kind of payment provider data. We just need to define one that will work as an example for now.

To keep the example completely abstract we could include an ExamplePaymentMethodData and put it in the union. Include any attributes you want. savedCardId would be fine.

By doing that here we define how the schema works and how it can be extended. We can drop in a real implementation in the future.

[aldeed]

@ticean I added the "Example" prefix but I can't do it as a union unless we make two of them. I could union it with JSONObject, but that seems to defeat the purpose. I'm still leaning toward making it JSONObject in here because nobody will see it. As long as you have the payment service spun up and stitched in, which it will be by default, you'll see our default Stripe schema pasted over it.

[ticean]

Oh ok, thanks. I didn't know that the spec requires at least two types for a union.

So, do we want to normalize the interface so that it's the same for 1 or more payment methods? Or do you think it's OK for that to be whatever? Using the data from a union is different than using it from a type because you need to include the on to get the fields.

We could use a union of the default Example Payment type (is that our default?) and an example payment plugin.

[aldeed]

@ticean Good point about needing the on. Given that, I don't think this can be a union because our client code would need to use on if there are 2+ payment plugins or not use on if there's just one.

So now I think we should:

• Remove "Example" prefix. We'll have only a single type with a single example property
• Plugins will extend that type to add additional properties they need (rather than altering the data type of data prop)

So the base is something like this:

type PaymentData {
  transactionId: String
}

input PaymentDataInput {
  transactionId: String
}

And the Stripe payment plugin schema that gets stitched in would have something like:

extend type PaymentData {
  savedCardId: String
  # etc.
}

extend input PaymentDataInput {
  savedCardId: String
  name: String
  # etc.
}

[ticean]

Hi @aldeed. If we merge additional properties into a single type then it won't be possible to require any of them because a required field for payment type A would case input for payment type B to fail as it wouldn't have A's required field.

I still think a union is the way to go. The example payment (the default) and an example plugin would give us two payment methods to use. This means that the payments would be in the schema but doesn't require that they be enabled. The payments service can overwrite the union with any payment types and remove the example plugin.

When using a merged type without required properties we would rely on app code to validate the fields in the payload. When using a union we would rely on app code to check the enabled status of the plugin, which I think would be easier but most importantly would provide more useful information in the schema to the consumer and edge validation against that schema.

---

We could also consider lifting the polymorphism from the PaymentData up to the PaymentMethod. Meaning a union on the PaymentMethod.

The reason is that even with strongly typed data there's no currently no guarantee by the type system that PaymentMethod.type matches PaymentMethod.data.

I've been working on experimental Avro types which have similar requirements. There, I have a type with an enum property that has a single value, and a typed data property. It's sure to enforce that the type as a whole is correct.

[aldeed]

@ticean I pushed the union changes we discussed. Unfortunately you can't do a union of input, so the server would not start with that. But I left it in there commented out, with a link to the GraphQL GitHub issue, and I'm using JSONObject for the input for now.

[aldeed]

@spencern @ticean For reference, here is the GraphQL issue about adding support for input unions: graphql/graphql-js#207