From 8f45f1eb9760722575854e78b377474ff8f91fa2 Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Tue, 10 Jan 2017 14:27:00 -0800 Subject: [PATCH 1/4] Deprecate Spree::Address#empty? --- core/app/models/spree/address.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/core/app/models/spree/address.rb b/core/app/models/spree/address.rb index e7f990023b6..65e7ff26b05 100644 --- a/core/app/models/spree/address.rb +++ b/core/app/models/spree/address.rb @@ -106,16 +106,17 @@ def ==(other_address) end def same_as?(other_address) - Spree::Deprecation.warn("Address.same_as? is deprecated. It's equivalent to Address.==", caller) + Spree::Deprecation.warn("Address#same_as? is deprecated. It's equivalent to Address.==", caller) self == other_address end def same_as(other_address) - Spree::Deprecation.warn("Address.same_as is deprecated. It's equivalent to Address.==", caller) + Spree::Deprecation.warn("Address#same_as is deprecated. It's equivalent to Address.==", caller) self == other_address end def empty? + Spree::Deprecation.warn("Address#empty? is deprecated.", caller) attributes.except('id', 'created_at', 'updated_at', 'country_id').all? { |_, v| v.nil? } end From 9faf2f323d46c9d38d2a766717f7145cc94b48ab Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Tue, 10 Jan 2017 14:44:59 -0800 Subject: [PATCH 2/4] Fix Address#blank? Previously this would return Address#empty?, but an Address should always be considered present. --- core/app/models/spree/address.rb | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/core/app/models/spree/address.rb b/core/app/models/spree/address.rb index 65e7ff26b05..6e4890039a6 100644 --- a/core/app/models/spree/address.rb +++ b/core/app/models/spree/address.rb @@ -115,11 +115,19 @@ def same_as(other_address) self == other_address end + # @deprecated Do not use this def empty? Spree::Deprecation.warn("Address#empty? is deprecated.", caller) attributes.except('id', 'created_at', 'updated_at', 'country_id').all? { |_, v| v.nil? } end + # This exists because the default Object#blank?, checks empty? if it is + # defined, and we have defined empty. + # This should be removed once empty? is removed + def blank? + false + end + # @return [Hash] an ActiveMerchant compatible address hash def active_merchant_hash { From dde7863a6c369bc50496158df99d0f46e1978fbc Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Tue, 17 Jan 2017 10:15:40 -0800 Subject: [PATCH 3/4] Assert that use_billing is checked by default --- backend/spec/features/admin/orders/new_order_spec.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/backend/spec/features/admin/orders/new_order_spec.rb b/backend/spec/features/admin/orders/new_order_spec.rb index f2c6a3806b8..d6a904592e0 100644 --- a/backend/spec/features/admin/orders/new_order_spec.rb +++ b/backend/spec/features/admin/orders/new_order_spec.rb @@ -35,7 +35,7 @@ targetted_select2_search user.email, from: "#s2id_customer_search" end - check "order_use_billing" + expect(page).to have_checked_field('order_use_billing') fill_in_address click_on "Update" @@ -73,7 +73,7 @@ targetted_select2_search user.email, from: "#s2id_customer_search" end - check "order_use_billing" + expect(page).to have_checked_field('order_use_billing') fill_in_address click_on "Update" @@ -108,7 +108,7 @@ targetted_select2_search user.email, from: "#s2id_customer_search" end - check "order_use_billing" + expect(page).to have_checked_field('order_use_billing') fill_in_address click_on "Update" @@ -157,7 +157,7 @@ targetted_select2_search user.email, from: "#s2id_customer_search" end - check "order_use_billing" + expect(page).to have_checked_field('order_use_billing') fill_in_address click_on "Update" From 57b005ac85e670c875ddbc5760de0420cdc35374 Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Tue, 17 Jan 2017 15:09:27 -0800 Subject: [PATCH 4/4] Don't require address#empty? for use_billing Previously, this would only check the box if the two addresses were identical AND they were both "empty". Instead, this now checks "Use billing" any time the two addresses are equal and the ship_address is a new record. I think this was probably the original intention of checking empty? (only new records can be empty because of Address's validations). --- .../views/spree/admin/orders/customer_details/_form.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/app/views/spree/admin/orders/customer_details/_form.html.erb b/backend/app/views/spree/admin/orders/customer_details/_form.html.erb index de9b9735c8f..842f0a4a912 100644 --- a/backend/app/views/spree/admin/orders/customer_details/_form.html.erb +++ b/backend/app/views/spree/admin/orders/customer_details/_form.html.erb @@ -44,7 +44,7 @@ <% if Spree::Config[:order_bill_address_used] %>
- <%= check_box_tag 'order[use_billing]', '1', ((@order.bill_address.empty? && @order.ship_address.empty?) && @order.bill_address == @order.ship_address) %> + <%= check_box_tag 'order[use_billing]', '1', (@order.ship_address.new_record? && @order.bill_address == @order.ship_address) %> <%= label_tag 'order[use_billing]', Spree.t(:use_billing_address) %>