Updates from PR review

lib/shopify_api/resources/fulfillment_order.rb

@@ -14,12 +14,20 @@ def fulfillments(options = {})
     end
 
     def move(new_location_id:)
-      body = { fulfillment_order: { new_location_id: new_location_id } }
-      load_values(post(:move, body, only_id))
+      body = { fulfillment_order: { new_location_id: new_location_id } }.to_json
+      keyed_fos = load_keyed_fulfillment_orders_from_response(post(:move, {}, body))
+      if keyed_fos&.fetch('original_fulfillment_order', nil)&.attributes
+        load(keyed_fos['original_fulfillment_order'].attributes, false, true)
+      end
+      keyed_fos
     end
 
     def cancel
-      load_values(post(:cancel, {}, only_id))
+      keyed_fos = load_keyed_fulfillment_orders_from_response(post(:cancel, {}, only_id))
+      if keyed_fos&.fetch('fulfillment_order', nil)&.attributes
+        load(keyed_fos['fulfillment_order'].attributes, false, true)
+      end
+      keyed_fos
     end
 
     def close
@@ -28,11 +36,11 @@ def close
 
     private
 
-    def load_values(response)
+    def load_keyed_fulfillment_orders_from_response(response)
       return load_attributes_from_response(response) if response.code != '200'
 
-      keyed_fulfillments = ActiveSupport::JSON.decode(response.body)
-      keyed_fulfillments.map do |key, fo_attributes|
+      keyed_fulfillment_orders = ActiveSupport::JSON.decode(response.body)
+      keyed_fulfillment_orders.map do |key, fo_attributes|
         if fo_attributes.nil?
           [key, nil]
         else

test/fulfillment_order_test.rb

@@ -55,27 +55,29 @@ def setup
     end
 
     context "#move" do
-      should "be able to move fulfillment order to a new_location_id" do
+      should "move a fulfillment order to a new_location_id" do
         fulfillment_order = ShopifyAPI::FulfillmentOrder.find(519788021)
         new_location_id = 5
 
+        original = fulfillment_order.clone
+        original.status = 'closed'
         moved = ActiveSupport::JSON.decode(load_fixture('fulfillment_order'))
         moved['assigned_location_id'] = new_location_id
-        fulfillment_order.status = 'closed'
 
+        request_body = { fulfillment_order: { new_location_id: 5 } }
         body = {
-            original_fulfillment_order: fulfillment_order,
-            moved_fulfillment_order: moved,
-            remaining_fulfillment_order: nil,
+          original_fulfillment_order: original,
+          moved_fulfillment_order: moved,
+          remaining_fulfillment_order: nil,
         }
-        api_version = ShopifyAPI::ApiVersion.find_version('2019-01')
-        endpoint = "fulfillment_orders/519788021/move"
-        extension = ".json"
-        url = "https://this-is-my-test-shop.myshopify.com#{api_version.construct_api_path("#{endpoint}#{extension}")}"
-        url = url + "?fulfillment_order%5Bnew_location_id%5D=5"
-        fake endpoint, :method => :post, :url => url, :body => ActiveSupport::JSON.encode(body)
+        fake "fulfillment_orders/519788021/move", :method => :post,
+          :request_body => ActiveSupport::JSON.encode(request_body),
+          :body => ActiveSupport::JSON.encode(body)
 
         response_fos = fulfillment_order.move(new_location_id: new_location_id)
+
+        assert_equal 'closed', fulfillment_order.status
+
         assert_equal 3, response_fos.count
         original_fulfillment_order = response_fos['original_fulfillment_order']
         refute_nil original_fulfillment_order
@@ -94,7 +96,7 @@ def setup
     end
 
     context "#cancel" do
-      should "be able to cancel fulfillment order" do
+      should "cancel a fulfillment order" do
         fulfillment_order = ShopifyAPI::FulfillmentOrder.find(519788021)
         assert_equal 'open', fulfillment_order.status
 
@@ -107,6 +109,8 @@ def setup
         fake "fulfillment_orders/519788021/cancel", :method => :post, :body => ActiveSupport::JSON.encode(body)
 
         response_fos = fulfillment_order.cancel
+
+        assert_equal 'cancelled', fulfillment_order.status
         assert_equal 2, response_fos.count
         fulfillment_order = response_fos['fulfillment_order']
         assert_equal 'cancelled', fulfillment_order.status

test/test_helper.rb

@@ -92,6 +92,7 @@ def assert_request_body(expected)
       end
 
       def fake(endpoint, options={})
+        request_body = options.has_key?(:request_body) ? options.delete(:request_body) : nil
         body   = options.has_key?(:body) ? options.delete(:body) : load_fixture(endpoint)
         format = options.delete(:format) || :json
         method = options.delete(:method) || :get
@@ -104,7 +105,9 @@ def fake(endpoint, options={})
           "https://this-is-my-test-shop.myshopify.com#{api_version.construct_api_path("#{endpoint}#{extension}")}"
         end
 
-        WebMock.stub_request(method, url).to_return(
+        stubbing = WebMock.stub_request(method, url)
+        stubbing = stubbing.with(body: request_body) if request_body
+        stubbing.to_return(
           body: body, status: status, headers: { content_type: "text/#{format}", content_length: 1 }.merge(options)
         )
       end