Merge pull request #398 from bonobos/order_address_state_change

Prevent attempted save in address book on a nil address

core/app/models/concerns/spree/user_address_book.rb

@@ -54,19 +54,29 @@ def default_address_attributes=(attributes)
  alias_method :ship_address, :default_address
  alias_method :ship_address_attributes=, :default_address_attributes=
 
+ # saves address in address book
+ # sets address to the default if automatic_default_address is set to true
+ # if address is nil, does nothing and returns nil
  def ship_address=(address)
  be_default = Spree::Config.automatic_default_address
- save_in_address_book(address.attributes, be_default)
+ save_in_address_book(address.attributes, be_default) if address
  end
 
+ # saves order.ship_address and order.bill_address in address book
+ # sets ship_address to the default if automatic_default_address is set to true
+ # sets bill_address to the default if automatic_default_address is set to true and there is no ship_address
+ # if one address is nil, does not save that address
+ # if both do not save, returns nil
  def persist_order_address(order)
- if Spree::Config.automatic_default_address
- save_in_address_book(order.ship_address.attributes, true)
- save_in_address_book(order.bill_address.attributes, order.ship_address.nil?)
- else
- save_in_address_book(order.ship_address.attributes)
- save_in_address_book(order.bill_address.attributes)
- end
+ save_in_address_book(
+ order.ship_address.attributes,
+ Spree::Config.automatic_default_address
+ ) if order.ship_address
+
+ save_in_address_book(
+ order.bill_address.attributes,
+ order.ship_address.nil? && Spree::Config.automatic_default_address
+ ) if order.bill_address
  end
 
  # Add an address to the user's list of saved addresses for future autofill

core/spec/models/spree/concerns/user_address_book_spec.rb

@@ -261,7 +261,7 @@ module Spree
  context "when automatic_default_address preference is false" do
  before do
  Spree::Config.automatic_default_address = false
- expect(user).to receive(:save_in_address_book).with(kind_of(Hash)).twice
+ expect(user).to receive(:save_in_address_book).with(kind_of(Hash),false).twice
  #and not the optional 2nd argument
  end
 
@@ -270,6 +270,50 @@ module Spree
  user.persist_order_address(order)
  end
  end
+
+ context "when address is nil" do
+ context "when automatic_default_address preference is at a default of true" do
+ before do
+ Spree::Config.automatic_default_address = true
+ expect(user).to receive(:save_in_address_book).with(kind_of(Hash), true).once
+ end
+
+ it "does not call save_in_address_book on ship address" do
+ order = build(:order)
+ order.ship_address = nil
+
+ user.persist_order_address(order)
+ end
+
+ it "does not call save_in_address_book on bill address" do
+ order = build(:order)
+ order.bill_address = nil
+
+ user.persist_order_address(order)
+ end
+ end
+ end
+
+ context "when automatic_default_address preference is false" do
+ before do
+ Spree::Config.automatic_default_address = false
+ expect(user).to receive(:save_in_address_book).with(kind_of(Hash), false).once
+ end
+
+ it "does not call save_in_address_book on ship address" do
+ order = build(:order)
+ order.ship_address = nil
+
+ user.persist_order_address(order)
+ end
+
+ it "does not call save_in_address_book on bill address" do
+ order = build(:order)
+ order.bill_address = nil
+
+ user.persist_order_address(order)
+ end
+ end
  end
  end