feat!: Update the Order schema to make currency a required field#283
feat!: Update the Order schema to make currency a required field#283deinck6 wants to merge 6 commits intoUniversal-Commerce-Protocol:mainfrom
Conversation
| "permalink_url", | ||
| "line_items", | ||
| "fulfillment", | ||
| "currency", |
There was a problem hiding this comment.
For future reference: you can also use the transition schema attribute to avoid an immediate breaking change (example:
ucp/source/schemas/shopping/checkout.json
Line 32 in e8d0480
There was a problem hiding this comment.
+1 please add a transition annotation.
There was a problem hiding this comment.
Is this needed if I am making the field required immediately for the new version? My understanding was this annotation signals a future change
sinhanurag
left a comment
sinhanurag
left a comment
There was a problem hiding this comment.
We should get this in before the next version cut.
igrigorik
left a comment
igrigorik
left a comment
There was a problem hiding this comment.
LGTM, modulo missing transition annotation: please add, and we can land this.
| "permalink_url", | ||
| "line_items", | ||
| "fulfillment", | ||
| "currency", |
There was a problem hiding this comment.
+1 please add a transition annotation.
| "type": "string", | ||
| "description": "ISO 4217 currency code. MUST match the currency from the originating checkout session." | ||
| "description": "ISO 4217 currency code. MUST match the currency from the originating checkout session.", | ||
| "ucp_request": { |
There was a problem hiding this comment.
shouldn't it be request: omit? currency is a response only attribute, the caller does not provide currency.
There was a problem hiding this comment.
A fly-by comment: I think we need to revisit Order capability schema more holistically on top of the currency field - given it currently doesn't have any associated operations (besides webhook support - of which currency will be required in the "request" body or webhook payload that business sends to platform), none of the required attributes are actually marked as response-only..
6 participants
Description
This PR is making the currency field in Order required. This is a followup change from PR 210. When adding the currency field to the Order schema the intention was to make it required during the next version update / breaking change.
Type of change
Please delete options that are not relevant.
functionality to not work as expected, including removal of schema files
or fields)
Is this a Breaking Change or Removal?
If you checked "Breaking change" above, or if you are removing any schema
files or fields:
!to my PR title (e.g.,feat!: remove field).