Updating addresses on standing order updates addresses on any initialised orders

app/forms/standing_order_form.rb

@@ -53,6 +53,8 @@ class StandingOrderForm
     future_and_undated_orders.each do |order|
       order.assign_attributes(customer_id: customer_id, email: customer.andand.email, distributor_id: shop_id)
 
+      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?
       update_payment_for(order) if payment_method_id_changed?
 
@@ -83,6 +85,22 @@ class StandingOrderForm
     @future_and_undated_orders = orders.joins(:order_cycle).merge(OrderCycle.not_closed).readonly(false)
   end
 
+  def update_bill_address_for(order)
+    unless addresses_match?(order.bill_address, bill_address)
+      return add_order_update_issue(order, I18n.t('bill_address'))
+    end
+    order.bill_address.update_attributes(bill_address.attributes.slice(*relevant_address_attrs))
+  end
+
+  def update_ship_address_for(order)
+    force_update = force_ship_address_update_for?(order)
+    return unless force_update || order.shipping_method.require_ship_address?
+    unless force_update || addresses_match?(order.ship_address, ship_address)
+      return add_order_update_issue(order, I18n.t('ship_address'))
+    end
+    order.ship_address.update_attributes(ship_address.attributes.slice(*relevant_address_attrs))
+  end
+
   def update_payment_for(order)
     payment = order.payments.with_state('checkout').where(payment_method_id: payment_method_id_was).last
     if payment
@@ -169,6 +187,24 @@ class StandingOrderForm
     order_update_issues[order.id] << issue
   end
 
+  def relevant_address_attrs
+    ["firstname","lastname","address1","zipcode","city","state_id","country_id","phone"]
+  end
+
+  def addresses_match?(order_address, standing_order_address)
+    relevant_address_attrs.all? do |attr|
+      order_address[attr] == standing_order_address.send("#{attr}_was") ||
+      order_address[attr] == standing_order_address[attr]
+    end
+  end
+
+  def force_ship_address_update_for?(order)
+    return false unless shipping_method.require_ship_address?
+    distributor_address = order.send(:address_from_distributor)
+    relevant_address_attrs.all? do |attr|
+      order.ship_address[attr] == distributor_address[attr]
+    end
+  end
 
   def standing_order_valid?
     unless standing_order.valid?

app/models/spree/order_decorator.rb

@@ -336,17 +336,21 @@ Spree::Order.class_eval do
       # We can refactor this when we drop the multi-step checkout option
       #
       if shipping_method.andand.require_ship_address == false
-        self.ship_address = distributor.address.clone
-
-        if bill_address
-          ship_address.firstname = bill_address.firstname
-          ship_address.lastname = bill_address.lastname
-          ship_address.phone = bill_address.phone
-        end
+        self.ship_address = address_from_distributor
       end
     end
   end
 
+  def address_from_distributor
+    address = distributor.address.clone
+    if bill_address
+      address.firstname = bill_address.firstname
+      address.lastname = bill_address.lastname
+      address.phone = bill_address.phone
+    end
+    address
+  end
+
   def provided_by_order_cycle?(line_item)
     order_cycle_variants = order_cycle.andand.variants || []
     order_cycle_variants.include? line_item.variant

spec/forms/standing_order_form_spec.rb

@@ -273,6 +273,178 @@ describe StandingOrderForm do
     end
   end
 
+  describe "changing the billing address" do
+    let(:standing_order) { create(:standing_order, with_items: true, with_proxy_orders: true) }
+    let(:shipping_method) { standing_order.shipping_method }
+    let!(:order) { standing_order.proxy_orders.first.initialise_order! }
+    let!(:bill_address_attrs) { standing_order.bill_address.attributes }
+    let!(:ship_address_attrs) { standing_order.ship_address.attributes }
+    let(:params) { { bill_address_attributes: { id: bill_address_attrs["id"], firstname: "Bill", address1: "123 abc st", phone: "1123581321" } } }
+    let(:form) { StandingOrderForm.new(standing_order, params) }
+
+    context "when a ship address is not required" do
+      before { shipping_method.update_attributes(require_ship_address: false) }
+
+      context "when the bill_address on the order matches that on the standing order" do
+        it "updates all bill_address attrs and ship_address names + phone" do
+          expect(form.save).to be true
+          expect(form.order_update_issues.keys).to_not include order.id
+          order.reload; standing_order.reload;
+          expect(standing_order.bill_address.firstname).to eq "Bill"
+          expect(standing_order.bill_address.lastname).to eq bill_address_attrs["lastname"]
+          expect(standing_order.bill_address.address1).to eq "123 abc st"
+          expect(standing_order.bill_address.phone).to eq "1123581321"
+          expect(order.bill_address.firstname).to eq "Bill"
+          expect(order.bill_address.lastname).to eq bill_address_attrs["lastname"]
+          expect(order.bill_address.address1).to eq "123 abc st"
+          expect(order.bill_address.phone).to eq "1123581321"
+          expect(order.ship_address.firstname).to eq "Bill"
+          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 "1123581321"
+        end
+      end
+
+      context "when the bill_address on the order doesn't match that on the standing order" do
+        before { order.bill_address.update_attributes(firstname: "Jane") }
+        it "does not update bill_address or ship_address on the order" do
+          expect(form.save).to be true
+          expect(form.order_update_issues.keys).to include order.id
+          order.reload; standing_order.reload;
+          expect(standing_order.bill_address.firstname).to eq "Bill"
+          expect(standing_order.bill_address.lastname).to eq bill_address_attrs["lastname"]
+          expect(standing_order.bill_address.address1).to eq "123 abc st"
+          expect(standing_order.bill_address.phone).to eq "1123581321"
+          expect(order.bill_address.firstname).to eq "Jane"
+          expect(order.bill_address.lastname).to eq bill_address_attrs["lastname"]
+          expect(order.bill_address.address1).to eq bill_address_attrs["address1"]
+          expect(order.bill_address.phone).to eq bill_address_attrs["phone"]
+          expect(order.ship_address.firstname).to eq "Jane"
+          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
+    end
+
+    context "when a ship address is required" do
+      before { shipping_method.update_attributes(require_ship_address: true) }
+
+      context "when the bill_address on the order matches that on the standing order" do
+        it "only updates bill_address attrs" do
+          expect(form.save).to be true
+          expect(form.order_update_issues.keys).to_not include order.id
+          order.reload; standing_order.reload;
+          expect(order.bill_address.firstname).to eq "Bill"
+          expect(order.bill_address.lastname).to eq bill_address_attrs["lastname"]
+          expect(order.bill_address.address1).to eq "123 abc st"
+          expect(order.bill_address.phone).to eq "1123581321"
+          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 bill_address on the order doesn't match that on the standing order" do
+        before { order.bill_address.update_attributes(firstname: "Jane") }
+        it "does not update bill_address or ship_address on the order" do
+          expect(form.save).to be true
+          expect(form.order_update_issues.keys).to include order.id
+          order.reload; standing_order.reload;
+          expect(standing_order.bill_address.firstname).to eq "Bill"
+          expect(standing_order.bill_address.lastname).to eq bill_address_attrs["lastname"]
+          expect(standing_order.bill_address.address1).to eq "123 abc st"
+          expect(standing_order.bill_address.phone).to eq "1123581321"
+          expect(order.bill_address.firstname).to eq "Jane"
+          expect(order.bill_address.lastname).to eq bill_address_attrs["lastname"]
+          expect(order.bill_address.address1).to eq bill_address_attrs["address1"]
+          expect(order.bill_address.phone).to eq bill_address_attrs["phone"]
+          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
+    end
+  end
+
+  describe "changing the ship address" do
+    let(:standing_order) { create(:standing_order, with_items: true, with_proxy_orders: true) }
+    let(:shipping_method) { standing_order.shipping_method }
+    let!(:order) { standing_order.proxy_orders.first.initialise_order! }
+    let!(:bill_address_attrs) { standing_order.bill_address.attributes }
+    let!(:ship_address_attrs) { standing_order.ship_address.attributes }
+    let(:params) { { ship_address_attributes: { id: ship_address_attrs["id"], firstname: "Ship", address1: "123 abc st", phone: "1123581321" } } }
+    let(:form) { StandingOrderForm.new(standing_order, params) }
+
+    context "when a ship address is not required" do
+      before { shipping_method.update_attributes(require_ship_address: false) }
+
+      it "does not change the ship address" do
+        expect(form.save).to be true
+        expect(form.order_update_issues.keys).to_not include order.id
+        order.reload; standing_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
+
+      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) }
+        before { params.merge!({ shipping_method_id: new_shipping_method.id }) }
+
+        it "updates ship_address attrs" do
+          expect(form.save).to be true
+          expect(form.order_update_issues.keys).to_not include order.id
+          order.reload; standing_order.reload;
+          expect(order.ship_address.firstname).to eq "Ship"
+          expect(order.ship_address.lastname).to eq ship_address_attrs["lastname"]
+          expect(order.ship_address.address1).to eq "123 abc st"
+          expect(order.ship_address.phone).to eq "1123581321"
+        end
+      end
+    end
+
+    context "when a ship address is required" do
+      before { shipping_method.update_attributes(require_ship_address: true) }
+
+      context "when the ship address on the order matches that on the standing order" do
+        it "updates ship_address attrs" do
+          expect(form.save).to be true
+          expect(form.order_update_issues.keys).to_not include order.id
+          order.reload; standing_order.reload;
+          expect(standing_order.ship_address.firstname).to eq "Ship"
+          expect(standing_order.ship_address.lastname).to eq ship_address_attrs["lastname"]
+          expect(standing_order.ship_address.address1).to eq "123 abc st"
+          expect(standing_order.ship_address.phone).to eq "1123581321"
+          expect(order.ship_address.firstname).to eq "Ship"
+          expect(order.ship_address.lastname).to eq ship_address_attrs["lastname"]
+          expect(order.ship_address.address1).to eq "123 abc st"
+          expect(order.ship_address.phone).to eq "1123581321"
+        end
+      end
+
+      context "when the ship address on the order doesn't match that on the standing order" do
+        before { order.ship_address.update_attributes(firstname: "Jane") }
+        it "does not update ship_address on the order" do
+          expect(form.save).to be true
+          expect(form.order_update_issues.keys).to include order.id
+          order.reload; standing_order.reload;
+          expect(standing_order.ship_address.firstname).to eq "Ship"
+          expect(standing_order.ship_address.lastname).to eq ship_address_attrs["lastname"]
+          expect(standing_order.ship_address.address1).to eq "123 abc st"
+          expect(standing_order.ship_address.phone).to eq "1123581321"
+          expect(order.ship_address.firstname).to eq "Jane"
+          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
+    end
+  end
+
   describe "changing the quantity of a line item" do
     let(:standing_order) { create(:standing_order, with_items: true, with_proxy_orders: true) }
     let(:order) { standing_order.proxy_orders.first.initialise_order! }