From f1896313b3306757fa0c291349f2de108be69547 Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Fri, 8 Jun 2018 12:32:38 +0200 Subject: [PATCH] Move shipping_address_from_distributor to OrderUpdater This is still Spree-1.3 compatible not considering multiple shipments and shipping methods. --- app/models/order_updater.rb | 11 +++++++++ app/models/spree/order_decorator.rb | 13 ---------- spec/models/order_updater_spec.rb | 38 ++++++++++++++++++++++++++--- spec/models/spree/order_spec.rb | 35 -------------------------- 4 files changed, 46 insertions(+), 51 deletions(-) diff --git a/app/models/order_updater.rb b/app/models/order_updater.rb index 9b41145ea3..f27103385e 100644 --- a/app/models/order_updater.rb +++ b/app/models/order_updater.rb @@ -31,6 +31,10 @@ class OrderUpdater < SimpleDelegator order.payment_state end + def before_save_hook + shipping_address_from_distributor + end + private # Taken from order.outstanding_balance in Spree 2.4 @@ -38,4 +42,11 @@ class OrderUpdater < SimpleDelegator def canceled_and_paid_for? order.canceled? && order.payments.present? && !order.payments.completed.empty? end + + def shipping_address_from_distributor + shipping_method = order.shipments.first.shipping_methods.first + return if shipping_method.require_ship_address + + order.ship_address = order.distributor.address + end end diff --git a/app/models/spree/order_decorator.rb b/app/models/spree/order_decorator.rb index eed4cefd0b..e7e208acd1 100644 --- a/app/models/spree/order_decorator.rb +++ b/app/models/spree/order_decorator.rb @@ -20,7 +20,6 @@ Spree::Order.class_eval do validate :disallow_guest_order attr_accessible :order_cycle_id, :distributor_id, :customer_id - before_validation :shipping_address_from_distributor before_validation :associate_customer, unless: :customer_id? before_validation :ensure_customer, unless: :customer_is_valid? @@ -337,18 +336,6 @@ Spree::Order.class_eval do private - def shipping_address_from_distributor - if distributor - # This method is confusing to conform to the vagaries of the multi-step checkout - # We copy over the shipping address when we have no shipping method selected - # We can refactor this when we drop the multi-step checkout option - # - if shipping_method.andand.require_ship_address == false - self.ship_address = address_from_distributor - end - end - end - def address_from_distributor address = distributor.address.clone if bill_address diff --git a/spec/models/order_updater_spec.rb b/spec/models/order_updater_spec.rb index 217a5dd491..bb1e01ea5c 100644 --- a/spec/models/order_updater_spec.rb +++ b/spec/models/order_updater_spec.rb @@ -1,10 +1,10 @@ require 'spec_helper' describe OrderUpdater do - context "#updating_payment_state" do - let(:order) { build(:order) } - let(:order_updater) { described_class.new(Spree::OrderUpdater.new(order)) } + let(:order) { build(:order) } + let(:order_updater) { described_class.new(Spree::OrderUpdater.new(order)) } + context "#updating_payment_state" do it "is failed if no valid payments" do allow(order).to receive_message_chain(:payments, :valid, :empty?) { true } @@ -98,4 +98,36 @@ describe OrderUpdater do end end end + + context '#before_save_hook' do + let(:distributor) { build(:distributor_enterprise) } + let(:shipment) { build(:shipment) } + let(:order) { build(:order, distributor: distributor) } + + before do + shipment.shipping_methods << shipping_method + order.shipments << shipment + end + + context 'when the shipping method doesn\'t require a delivery address' do + let(:shipping_method) { build(:base_shipping_method, require_ship_address: false) } + + it "populates the shipping address" do + order_updater.before_save_hook + expect(order.ship_address.firstname).to eq(distributor.address.firstname) + end + end + + context 'when the shipping method requires a delivery address' do + let(:shipping_method) { build(:base_shipping_method, require_ship_address: true) } + let(:address) { build(:address, firstname: 'will') } + + before { order.ship_address = address } + + it "does not populate the shipping address" do + order_updater.before_save_hook + expect(order.ship_address.firstname).to eq("will") + end + end + end end diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb index 345cbb661f..4e23a102ef 100644 --- a/spec/models/spree/order_spec.rb +++ b/spec/models/spree/order_spec.rb @@ -509,41 +509,6 @@ describe Spree::Order do end end - describe "shipping address prepopulation" do - let(:distributor) { create(:distributor_enterprise) } - let(:order) { build(:order, distributor: distributor) } - - before do - order.ship_address = distributor.address.clone - order.save # just to trigger our autopopulate the first time ;) - end - - it "autopopulates the shipping address on save" do - order.should_receive(:shipping_address_from_distributor).and_return true - order.save - end - - it "populates the shipping address if the shipping method doesn't require a delivery address" do - order.shipping_method = create(:shipping_method, require_ship_address: false) - order.ship_address.update_attribute :firstname, "will" - order.save - order.ship_address.firstname.should == distributor.address.firstname - end - - it "does not populate the shipping address if the shipping method requires a delivery address" do - order.shipping_method = create(:shipping_method, require_ship_address: true) - order.ship_address.update_attribute :firstname, "will" - order.save - order.ship_address.firstname.should == "will" - end - - it "doesn't attempt to create a shipment if the order is not yet valid" do - order.shipping_method = create(:shipping_method, require_ship_address: false) - #Shipment.should_not_r - order.create_shipment! - end - end - describe "checking if an order is an account invoice" do let(:accounts_distributor) { create(:distributor_enterprise) } let(:order_account_invoice) { create(:order, distributor: accounts_distributor) }