feat: Add ec.totals.change to Embedded Checkout Protocol#272
feat: Add ec.totals.change to Embedded Checkout Protocol#272
ec.totals.change to Embedded Checkout Protocol#272Conversation
westeezy
left a comment
There was a problem hiding this comment.
I think this seems reasonable and appreciate the strict rule on ordering to keep it predictable.
jingyli
left a comment
There was a problem hiding this comment.
Change overall LGTM, just have a conceptual question regarding expectation when a domain-specific notification exists.
| When a change also triggers a domain-specific message (e.g., | ||
| `ec.line_items.change`, `ec.buyer.change`, or `ec.payment.change`), the business | ||
| **MUST** send the domain-specific message first, then follow it with | ||
| `ec.totals.change`. |
There was a problem hiding this comment.
Naive QQ: I understand this is nice from a consistency perspective, but is there any additional benefits from mandating business to MUST send both notifications if the domain-specific message also has its own mandate to send the entire latest checkout state (which naturally encapsulates ec.totals.change)? It feels like we are going to be exchanging the same checkout object multiple times throughout the embedded checkout's lifecycle (especially if we are going to mirror this pattern for other EP bindings like Embedded Cart, where it's possible many line_item changes will occur).
Should we loosen the guideline to MAY (or SHOULD) or do the opposite where if a domain-specific message exist, then that must be the one message sent back to host with the updated totals?
There was a problem hiding this comment.
We discussed this internally too, as were also a little worried by the amount of data flying over the wire for these changes. In the end, I think this contract is still right, though — it is weird for only some changes to totals to be communicated with this message, but not others, and I felt uncomfortable with the implementation instructions for both host and embedded checkout when I tried writing docs for a version that deduplicated ec.totals.change with other messages.
bdcef3d to
9f22986
Compare
|
Thanks for putting this together @lemonmade !! Beautifully done! :) |
Description
This PR adds an
ec.totals.changemessage to the Embedded Checkout Protocol. This message allows hosts to be informed of changes to the checkout state that do not have an accompanyingec.{domain}.changeevent today, like taxes and fees, which are entirely contained intotals. It also allows hosts to be informed of totals-only changes that may result from asynchronous updates after a domain event, such as potentially-slow shipping rate updates after aec.fulfillment.changemessage has indicated the initial application of the change.Type of change
Please delete options that are not relevant.
functionality to not work as expected, including removal of schema files
or fields)
Checklist