From e3b1fbfefea38bbb36fbd7ad2bc4454f68699fe0 Mon Sep 17 00:00:00 2001 From: Lorenzo Fermi Date: Sun, 29 Mar 2020 14:26:57 +0200 Subject: [PATCH] Remove calls to deprecated methods (in both code and specs) --- .../concerns/spree/user_address_book.rb | 18 +++++++++++---- core/app/models/spree/order.rb | 4 ++-- .../spree/concerns/user_address_book_spec.rb | 23 +++++++------------ core/spec/models/spree/order/checkout_spec.rb | 4 ++-- 4 files changed, 26 insertions(+), 23 deletions(-) diff --git a/core/app/models/concerns/spree/user_address_book.rb b/core/app/models/concerns/spree/user_address_book.rb index 5e6ee465c60..e250c9d555a 100644 --- a/core/app/models/concerns/spree/user_address_book.rb +++ b/core/app/models/concerns/spree/user_address_book.rb @@ -13,10 +13,19 @@ def find_first_by_address_values(address_attrs) def mark_default(user_address, address_type: :shipping) column_for_default = address_type == :shipping ? :default : :default_billing ActiveRecord::Base.transaction do - (self - [user_address]).each do |ua| # update_all would be nice, but it bypasses ActiveRecord callbacks - ua.persisted? ? ua.update!({ column_for_default => false }) : ua.write_attribute(column_for_default, false) + (self - [user_address]).each do |address| # update_all would be nice, but it bypasses ActiveRecord callbacks + if address.persisted? + address.update!(column_for_default => false) + else + address.write_attribute(column_for_default, false) + end + end + + if user_address.persisted? + user_address.update!(column_for_default => true, archived: false) + else + user_address.write_attribute(column_for_default, true) end - user_address.persisted? ? user_address.update!({ column_for_default => true, :archived => false }) : user_address.write_attribute(column_for_default, true) end end end @@ -112,7 +121,8 @@ def persist_order_address(order) # @param default set whether or not this address will show up from # #default_address or not def save_in_address_book(address_attributes, default = false, address_type = :shipping) - return nil unless address_attributes.present? + return nil if address_attributes.blank? + address_attributes = address_attributes.to_h.with_indifferent_access new_address = Spree::Address.factory(address_attributes) diff --git a/core/app/models/spree/order.rb b/core/app/models/spree/order.rb index 4a0f28ed48c..57454b32502 100644 --- a/core/app/models/spree/order.rb +++ b/core/app/models/spree/order.rb @@ -744,8 +744,8 @@ def ship_address_attributes=(attributes) # @note This doesn't persist the change bill_address or ship_address def assign_default_user_addresses if user - bill_address = (user.bill_address || user.default_address) - ship_address = (user.ship_address || user.default_address) + bill_address = user.bill_address + ship_address = user.ship_address # this is one of 2 places still using User#bill_address self.bill_address ||= bill_address if bill_address.try!(:valid?) # Skip setting ship address if order doesn't have a delivery checkout step diff --git a/core/spec/models/spree/concerns/user_address_book_spec.rb b/core/spec/models/spree/concerns/user_address_book_spec.rb index f3072da8931..ebc3cc357c1 100644 --- a/core/spec/models/spree/concerns/user_address_book_spec.rb +++ b/core/spec/models/spree/concerns/user_address_book_spec.rb @@ -11,7 +11,7 @@ module Spree let!(:user) { create(:user) } describe "#save_in_address_book" do - context "saving a shipping address" do + context "saving a default shipping address" do let(:user_address) { user.user_addresses.find_first_by_address_values(address.attributes) } subject do @@ -45,7 +45,7 @@ module Spree end end - it "adds the address to the user's the associated addresses" do + it "adds the address to the user's associated addresses" do is_expected.to change { user.reload.addresses.count }.by(1) end end @@ -79,7 +79,6 @@ module Spree user.save_in_address_book(address.attributes, true) expect(user.addresses.count).to eq 3 expect(user.ship_address.address1).to eq address.address1 - expect(user.default_address.address1).to eq address.address1 end end end @@ -243,7 +242,6 @@ module Spree it "sets the passed address as default shipping address" do subject expect(user.ship_address).to eq address - expect(user.default_address).to eq address end end @@ -406,7 +404,7 @@ module Spree end end - context "when one order address is nil" do + context "when either ship_address or bill_address is nil" do context "when automatic_default_address preference is at a default of true" do before do stub_spree_preferences(automatic_default_address: true) @@ -430,12 +428,13 @@ module Spree end context "when automatic_default_address preference is false" do + let(:order) { build(:order) } + before do stub_spree_preferences(automatic_default_address: false) end it "does not call save_in_address_book on ship address" do - order = build(:order) order.ship_address = nil expect(user).to receive(:save_in_address_book).with(kind_of(Hash), false, :billing).once @@ -443,7 +442,6 @@ module Spree end it "does not call save_in_address_book on bill address" do - order = build(:order) order.bill_address = nil expect(user).to receive(:save_in_address_book).with(kind_of(Hash), false).once @@ -460,11 +458,6 @@ module Spree it "stores the ship_address" do expect(subject.ship_address).to eq ship_address end - - it "is also available as default_address" do - expect(subject.ship_address).to eq ship_address - expect(subject.default_address).to eq ship_address - end end describe "#ship_address=" do @@ -472,7 +465,7 @@ module Spree let!(:address) { create(:address) } # https://github.com/solidusio/solidus/issues/1241 - it "resets the association and persists" do + it "resets the association and persists the ship_address" do # Load (which will cache) the has_one association expect(user.ship_address).to be_nil @@ -489,7 +482,7 @@ module Spree let!(:address) { create(:address) } # https://github.com/solidusio/solidus/issues/1241 - it "resets the association and persists" do + it "resets the association and persists the bill_address" do # Load (which will cache) the has_one association expect(user.bill_address).to be_nil @@ -565,7 +558,7 @@ module Spree allow(Spree::Address).to receive(:immutable_merge).and_return address end - it do + it "updates ship_address with its present attributes merged with the passed ones" do expect(Spree::Address).to receive(:immutable_merge).with(user.ship_address, attributes) expect(user).to receive(:ship_address=).with address diff --git a/core/spec/models/spree/order/checkout_spec.rb b/core/spec/models/spree/order/checkout_spec.rb index ceb3b20a663..41d85b96d67 100644 --- a/core/spec/models/spree/order/checkout_spec.rb +++ b/core/spec/models/spree/order/checkout_spec.rb @@ -140,7 +140,7 @@ def assert_state_changed(order, from, to) let(:address_kind) { :ship } before do order.user = FactoryBot.create(:user) - order.user.default_address = default_address + order.user.ship_address = default_address order.next! order.reload end @@ -150,7 +150,7 @@ def assert_state_changed(order, from, to) let(:address_kind) { :bill } before do order.user = FactoryBot.create(:user) - order.user.default_address = default_address + order.user.bill_address = default_address order.next! order.reload end