Uncomment and update OrderSyncer failing specs

This still achieves a logic issue before the Spree upgrade, where
switching from pick-up to delivery affects whether simultaneous changes
to shipping address are ignored or not.

This behaviour can be fixed in a separate PR.

app/services/order_syncer.rb

@@ -31,8 +31,19 @@ class OrderSyncer
 
   def update_associations_for(order)
     update_bill_address_for(order) if (bill_address.changes.keys & relevant_address_attrs).any?
-    update_ship_address_for(order) if (ship_address.changes.keys & relevant_address_attrs).any?
     update_shipment_for(order) if shipping_method_id_changed?
+
+    # The conditions here are to achieve the same behaviour in earlier versions of Spree, where
+    # switching from pick-up to delivery affects whether simultaneous changes to shipping address
+    # are ignored or not.
+    pickup_to_delivery = force_ship_address_required?(order)
+    if (ship_address.changes.keys & relevant_address_attrs).any?
+      update_ship_address_for(order) unless pickup_to_delivery && order.shipment.blank?
+    end
+    unless pickup_to_delivery && order.shipment.present?
+      order.updater.__send__(:shipping_address_from_distributor)
+    end
+
     update_payment_for(order) if payment_method_id_changed?
   end
 

spec/services/order_syncer_spec.rb

@@ -156,7 +156,7 @@ xdescribe OrderSyncer do
       end
 
       context "when the bill_address on the order matches that on the subscription" do
-        xit "updates all bill_address attrs and ship_address names + phone" do
+        it "updates all bill_address attrs and ship_address names + phone" do
           subscription.assign_attributes(params)
           expect(syncer.sync!).to be true
           expect(syncer.order_update_issues.keys).to_not include order.id
@@ -173,8 +173,12 @@ xdescribe OrderSyncer do
       end
 
       context "when the bill_address on the order doesn't match that on the subscription" do
-        before { order.bill_address.update_attributes(firstname: "Jane") }
-        xit "does not update bill_address or ship_address on the order" do
+        before do
+          order.bill_address.update_attributes!(firstname: "Jane")
+          order.update!
+        end
+
+        it "does not update bill_address or ship_address on the order" do
           subscription.assign_attributes(params)
           expect(syncer.sync!).to be true
           expect(syncer.order_update_issues.keys).to include order.id
@@ -214,7 +218,10 @@ xdescribe OrderSyncer do
       end
 
       context "when the bill_address on the order doesn't match that on the subscription" do
-        before { order.bill_address.update_attributes(firstname: "Jane") }
+        before do
+          order.bill_address.update_attributes!(firstname: "Jane")
+          order.update!
+        end
 
         it "does not update bill_address or ship_address on the order" do
           subscription.assign_attributes(params)
@@ -265,7 +272,7 @@ xdescribe OrderSyncer do
       end
 
       context "but the shipping method is being changed to one that requires a ship_address" do
-        let(:new_shipping_method) { create(:shipping_method, require_ship_address: true) }
+        let(:new_shipping_method) { create(:shipping_method, distributors: [distributor], require_ship_address: true) }
 
         before { params.merge!(shipping_method_id: new_shipping_method.id) }
 
@@ -284,22 +291,27 @@ xdescribe OrderSyncer do
                                   with_proxy_orders: true)
           end
 
-          before do
-            subscription.assign_attributes(params)
-          end
+          context "when there is no pending shipment using the former shipping method" do
+            before do
+              order.shipment.destroy
+              subscription.assign_attributes(params)
+            end
 
-          it "updates ship_address attrs" do
-            expect(syncer.sync!).to be true
-            expect(syncer.order_update_issues.keys).to include order.id
-            order.reload
-            expect(order.ship_address.firstname).to eq ship_address_attrs["firstname"]
-            expect(order.ship_address.lastname).to eq ship_address_attrs["lastname"]
-            expect(order.ship_address.address1).to eq ship_address_attrs["address1"]
-            expect(order.ship_address.phone).to eq ship_address_attrs["phone"]
+            it "updates ship_address attrs" do
+              expect(syncer.sync!).to be true
+              expect(syncer.order_update_issues.keys).to include order.id
+              order.reload
+              expect(order.ship_address.firstname).to eq ship_address_attrs["firstname"]
+              expect(order.ship_address.lastname).to eq ship_address_attrs["lastname"]
+              expect(order.ship_address.address1).to eq ship_address_attrs["address1"]
+              expect(order.ship_address.phone).to eq ship_address_attrs["phone"]
+            end
           end
 
           context "when the order has a pending shipment using the former shipping method" do
-            let!(:shipment) { create(:shipment, order: order, shipping_method: shipping_method) }
+            before do
+              subscription.assign_attributes(params)
+            end
 
             it "updates ship_address attrs" do
               expect(syncer.sync!).to be true
@@ -334,7 +346,11 @@ xdescribe OrderSyncer do
       end
 
       context "when the ship address on the order doesn't match that on the subscription" do
-        before { order.ship_address.update_attributes(firstname: "Jane") }
+        before do
+          order.ship_address.update_attributes(firstname: "Jane")
+          order.update!
+        end
+
         it "does not update ship_address on the order" do
           subscription.assign_attributes(params)
           expect(syncer.sync!).to be true