Skip to content
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

Document proposed shipping methods v3 API #503

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

bc-luke
Copy link
Member

@bc-luke bc-luke commented Sep 6, 2024

Draft shipping methods API to support new shipping promotions. This doesn't need to be public but perhaps we will choose to release it publicly. To be discussed.

image

What changed?

Document the parts of a shipping methods API that would be necessary to support the new shipping promotions UI including:

  • Filter by name, channel ID and zone
  • Optionally include zone information (zone name and ID) and channel IDs
  • Method name
  • Method ID

Release notes draft

Anything else?

ping {names}

type: string
example: "Courier delivery"
shipping_zone:
description: Zone information. This is available only when "include=shipping_zone" is present in the querystring.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like a sensible optional include 👌

@@ -355,6 +440,41 @@ components:
- commodity_description
- international_shipping
- hs_codes
shippingMethods:
title: shippingMethods
description: Data about the customs information object.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

title: shippingMethods
description: Data about the customs information object.
type: object
x-internal: false
Copy link
Contributor

@theromulans theromulans Sep 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand this represents that the shippingMethods V3 API is not intended to be a public API. Do we see a reason to hold this back?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's make it public, I have seen at least a few requests from agencies to have an API to query all methods without having to iterate over all the zones.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, just noticed this also: #490 (comment)

Copy link
Member Author

@bc-luke bc-luke Sep 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For a public release, we would probably want to add more properties so we have parity with V2 GET. What we have here are just the minimum to drive the promotions UI. Perhaps we can implement that first and then add the rest and follow that with a public release. Considering the driver for public release relates to querying perhaps we can also keep the hold operations for a future iteration.

type: string
example: "Thailand"
channel_ids:
description: Channel information. This is available only when "include=shipping_zone" is present in the querystring.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would we not want to always include the Channel IDs? Well, perhaps only when the store has multiple channels?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes probably a reasonable expecation. Updated!

Comment on lines 96 to 98
description: |-
* `shipping_zone` - shipping zone information
* `shipping_methods.channel_ids` - IDs of the associated channels
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
description: |-
* `shipping_zone` - shipping zone information
* `shipping_methods.channel_ids` - IDs of the associated channels
description: |-
* `shipping_zone` - Include shipping zone information with each shipping method.
* `shipping_methods.channel_ids` - Include the IDs of the associated channels for each shipping method.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔧Fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants