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

Fulfillment orders fulfillment cancellation request #637

Conversation

karmakaze
Copy link
Contributor

@karmakaze karmakaze commented Nov 13, 2019

This PR adds the following POST requests:

POST fulfillment_orders/:fulfillment_order_id/fulfillment_request
POST fulfillment_orders/:fulfillment_order_id/fulfillment_request/accept
POST fulfillment_orders/:fulfillment_order_id/fulfillment_request/reject

POST fulfillment_orders/:fulfillment_order_id/cancellation_request
POST fulfillment_orders/:fulfillment_order_id/cancellation_request/accept
POST fulfillment_orders/:fulfillment_order_id/cancellation_request/reject

GET requests are in PR #633
Actions move, cancel, close for fulfillment orders are in PR #635
Subsequent PRs will add other fulfillment order related POST requests to relevant resources.

Since all that's really needed is the fulfillment_order_id, if it's known you can do (rather than querying it first):

  FulfillmentOrder.new(id: <the-fo-id>).fulfillment_request(<params>)

Note to reviewer:

  • when the endpoint returns a single resource, it is loaded into the current resource and true is returned if the request is ok
  • when the endpoint returns multiple keyed resources (e.g. original_fulfillment_order, submitted_fulfillment_order, unsubmitted_fulfillment_order) the return value is a hash of the multiple resources keyed by the names. Is it conventional to reload the current resource with one of these as well?

@karmakaze karmakaze requested a review from a team as a code owner November 13, 2019 16:36
@karmakaze karmakaze changed the base branch from master to fulfillment-orders-move-cancel-close November 13, 2019 21:53
@karmakaze karmakaze force-pushed the fulfillment-orders-fulfillment-cancellation-request branch from 29f3a21 to 7f5e63c Compare November 13, 2019 23:00
@karmakaze karmakaze force-pushed the fulfillment-orders-move-cancel-close branch from 1d26023 to 6dae6dd Compare November 13, 2019 23:21
@karmakaze karmakaze force-pushed the fulfillment-orders-move-cancel-close branch from 6dae6dd to e66255e Compare November 14, 2019 22:09
@karmakaze karmakaze force-pushed the fulfillment-orders-fulfillment-cancellation-request branch from 7f5e63c to 90ab4b6 Compare November 14, 2019 22:34
lib/shopify_api/resources/fulfillment_order.rb Outdated Show resolved Hide resolved
lib/shopify_api/resources/fulfillment_order.rb Outdated Show resolved Hide resolved
lib/shopify_api/resources/fulfillment_order.rb Outdated Show resolved Hide resolved
test/fulfillment_order_test.rb Outdated Show resolved Hide resolved
test/fulfillment_order_test.rb Outdated Show resolved Hide resolved
test/fulfillment_order_test.rb Show resolved Hide resolved
test/fulfillment_order_test.rb Outdated Show resolved Hide resolved
test/test_helper.rb Outdated Show resolved Hide resolved
@karmakaze karmakaze force-pushed the fulfillment-orders-move-cancel-close branch 2 times, most recently from 634d2b3 to a47cdba Compare November 26, 2019 18:21
@karmakaze karmakaze force-pushed the fulfillment-orders-fulfillment-cancellation-request branch from 366a1e8 to 4ce57df Compare November 26, 2019 18:37
@karmakaze karmakaze force-pushed the fulfillment-orders-fulfillment-cancellation-request branch from 4ce57df to 8c5ddaf Compare December 2, 2019 18:49
@karmakaze karmakaze force-pushed the fulfillment-orders-move-cancel-close branch from a47cdba to 1a48c0e Compare December 2, 2019 20:07
@karmakaze karmakaze force-pushed the fulfillment-orders-fulfillment-cancellation-request branch from 8c5ddaf to 41b60d0 Compare December 2, 2019 20:07
Copy link

@froot froot left a comment

Choose a reason for hiding this comment

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

A small change and a couple of nits to keep the tests consistent to the actual API behaviour. Otherwise, looks good!

lib/shopify_api/resources/fulfillment_order.rb Outdated Show resolved Hide resolved
test/fulfillment_order_test.rb Outdated Show resolved Hide resolved
test/fulfillment_order_test.rb Outdated Show resolved Hide resolved
test/fulfillment_order_test.rb Outdated Show resolved Hide resolved
test/fulfillment_order_test.rb Outdated Show resolved Hide resolved
test/fulfillment_order_test.rb Outdated Show resolved Hide resolved
@karmakaze karmakaze force-pushed the fulfillment-orders-move-cancel-close branch from 1a48c0e to 2bbefa9 Compare December 10, 2019 20:13
@karmakaze karmakaze force-pushed the fulfillment-orders-fulfillment-cancellation-request branch from 41b60d0 to f1da843 Compare December 10, 2019 20:13
@karmakaze karmakaze force-pushed the fulfillment-orders-move-cancel-close branch from 2bbefa9 to 741c27e Compare December 16, 2019 18:21
@karmakaze karmakaze force-pushed the fulfillment-orders-fulfillment-cancellation-request branch from f1da843 to 46935ea Compare December 16, 2019 18:21
lib/shopify_api/resources/fulfillment_order.rb Outdated Show resolved Hide resolved
lib/shopify_api/resources/fulfillment_order.rb Outdated Show resolved Hide resolved
test/fulfillment_order_test.rb Outdated Show resolved Hide resolved
test/fulfillment_order_test.rb Outdated Show resolved Hide resolved
test/fulfillment_order_test.rb Outdated Show resolved Hide resolved
test/fulfillment_order_test.rb Outdated Show resolved Hide resolved
@karmakaze karmakaze force-pushed the fulfillment-orders-fulfillment-cancellation-request branch from 46935ea to 225ad9f Compare December 16, 2019 21:36
Copy link

@froot froot left a comment

Choose a reason for hiding this comment

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

Tested with postman, and sending a JSON body with { "fulfillment_order_line_items": null } will result in the expected behaviour of not setting the fulfillment_order_line_items key at all. Likewise with message.

Great work @karmakaze

@karmakaze karmakaze force-pushed the fulfillment-orders-move-cancel-close branch from 741c27e to 621057d Compare January 8, 2020 18:24
@karmakaze karmakaze force-pushed the fulfillment-orders-fulfillment-cancellation-request branch from 225ad9f to fc57449 Compare January 8, 2020 18:25
@karmakaze karmakaze force-pushed the fulfillment-orders-move-cancel-close branch from 621057d to 563d01d Compare January 9, 2020 21:06
@karmakaze karmakaze force-pushed the fulfillment-orders-fulfillment-cancellation-request branch from fc57449 to c5e8b3b Compare January 9, 2020 21:06
Copy link
Contributor

@linzhao125 linzhao125 left a comment

Choose a reason for hiding this comment

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

Looks great!

@karmakaze karmakaze force-pushed the fulfillment-orders-move-cancel-close branch from 563d01d to eb38d9a Compare January 10, 2020 15:46
…ulfillment_request, cancellation_request, accept_cancellation, reject_cancellation to FulfillmentOrder resource
@karmakaze karmakaze force-pushed the fulfillment-orders-fulfillment-cancellation-request branch from c5e8b3b to 7247a17 Compare January 10, 2020 15:46
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

5 participants