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

Add API support for scheduled fulfillment orders #797

Merged
merged 1 commit into from Jan 15, 2021

Conversation

linzhao125
Copy link
Contributor

@linzhao125 linzhao125 commented Nov 2, 2020

Fixes: https://github.com/Shopify/shopify/issues/255067

Add open and reschedule APIs support for scheduled fulfillment orders.

  • POST /admin/api/2021-01/fulfillment_orders/{fulfillment_order_id}/open.json

  • POST /admin/api/2021-01/fulfillment_orders/{fulfillment_order_id}/reschedule.json

@linzhao125 linzhao125 requested a review from a team as a code owner November 2, 2020 16:01
@andyw8
Copy link
Contributor

andyw8 commented Nov 2, 2020

Can you add an update to the CHANGELOG please (in the Unreleased section).

@@ -51,6 +51,19 @@ def cancel
keyed_fulfillment_orders
end

def open
load_attributes_from_response(post(:open, {}, body.to_json))
Copy link
Contributor

Choose a reason for hiding this comment

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

There is no body here, so I think this needs to be: load_attributes_from_response(post(:open, {}, only_id))

@linzhao125 linzhao125 force-pushed the subscriptions_support branch 2 times, most recently from b0a70ba to a3553c3 Compare November 2, 2020 22:01
fulfillment_order.status = 'scheduled'

opened = ActiveSupport::JSON.decode(load_fixture('fulfillment_order'))
opened['status'] = 'open'
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we set the status here before making the POST?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, we're comparing it against the fixture, I see.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This fixture has the status open already actually, here I am trying to make sure it is open, it should be fine without it. Just clearer this way IMO.

@andyw8
Copy link
Contributor

andyw8 commented Nov 5, 2020

FYI, the version of shopify_api in core is a little behind.

I tested with the latest release recently and found a few failures, so we'd have to address those before upgrading:

https://github.com/shopify/shopify/commit/84c96b3404feb604403717dfbcf4418cf7aa0adb
https://buildkite.com/shopify/shopify-selective-tests/builds/270945

@andyw8
Copy link
Contributor

andyw8 commented Nov 16, 2020

PR for the above: https://github.com/Shopify/shopify/pull/269163

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.

None yet

4 participants