From 21300973408b1b02b67d7d05cffaccab8c6d489c Mon Sep 17 00:00:00 2001 From: Filippo Liverani Date: Tue, 18 Feb 2020 09:36:04 +0100 Subject: [PATCH 1/6] Add preference to drive new Address name adoption --- .../install/templates/config/initializers/spree.rb.tt | 3 +++ core/lib/spree/app_configuration.rb | 5 +++++ core/lib/spree/testing_support/dummy_app.rb | 1 + 3 files changed, 9 insertions(+) diff --git a/core/lib/generators/solidus/install/templates/config/initializers/spree.rb.tt b/core/lib/generators/solidus/install/templates/config/initializers/spree.rb.tt index 56fd870b9c5..912e2bbfd42 100644 --- a/core/lib/generators/solidus/install/templates/config/initializers/spree.rb.tt +++ b/core/lib/generators/solidus/install/templates/config/initializers/spree.rb.tt @@ -10,6 +10,9 @@ Spree.config do |config| # from address for transactional emails config.mails_from = "store@example.com" + # Use combined first and last name attribute in HTML views and API responses + config.use_combined_first_and_last_name_in_address = true + # Uncomment to stop tracking inventory levels in the application # config.track_inventory_levels = false diff --git a/core/lib/spree/app_configuration.rb b/core/lib/spree/app_configuration.rb index 700d5889d50..fb3bf0685f7 100644 --- a/core/lib/spree/app_configuration.rb +++ b/core/lib/spree/app_configuration.rb @@ -227,6 +227,11 @@ class AppConfiguration < Preferences::Configuration # @return [Boolean] Don't escape HTML of product descriptions. (default: +false+) preference :show_raw_product_description, :boolean, default: false + # @!attribute [rw] use_combined_first_and_last_name_in_address + # @return [Boolean] Use Spree::Address combined first and last name in HTML views and + # API responses. (default: +false+) + preference :use_combined_first_and_last_name_in_address, :boolean, default: false + # @!attribute [rw] tax_using_ship_address # @return [Boolean] Use the shipping address rather than the billing address to determine tax (default: +true+) preference :tax_using_ship_address, :boolean, default: true diff --git a/core/lib/spree/testing_support/dummy_app.rb b/core/lib/spree/testing_support/dummy_app.rb index 333ecb2d1a2..541884c973d 100644 --- a/core/lib/spree/testing_support/dummy_app.rb +++ b/core/lib/spree/testing_support/dummy_app.rb @@ -103,6 +103,7 @@ class Application < ::Rails::Application Spree.config do |config| config.mails_from = "store@example.com" config.raise_with_invalid_currency = false + config.use_combined_first_and_last_name_in_address = true end # Raise on deprecation warnings From adaaa4c5f68cd2ff45e686a92d58c8dfcaabd2bb Mon Sep 17 00:00:00 2001 From: Filippo Liverani Date: Tue, 18 Feb 2020 17:18:00 +0100 Subject: [PATCH 2/6] Add validation errors to correct Address name field --- core/app/models/spree/address.rb | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/core/app/models/spree/address.rb b/core/app/models/spree/address.rb index 83fb13736be..206b4e1f2fc 100644 --- a/core/app/models/spree/address.rb +++ b/core/app/models/spree/address.rb @@ -11,10 +11,11 @@ class Address < Spree::Base belongs_to :country, class_name: "Spree::Country", optional: true belongs_to :state, class_name: "Spree::State", optional: true - validates :firstname, :address1, :city, :country_id, presence: true + validates :address1, :city, :country_id, presence: true validates :zipcode, presence: true, if: :require_zipcode? validates :phone, presence: true, if: :require_phone? + validate :validate_name validate :state_validate validate :validate_state_matches_country @@ -186,6 +187,17 @@ def name=(value) private + def validate_name + return if firstname.present? + + name_attribute = if Spree::Config.use_combined_first_and_last_name_in_address + :name + else + :firstname + end + errors.add(name_attribute, :blank) + end + def state_validate # Skip state validation without country (also required) # or when disabled by preference From 3244eecac4fc500341be10093c1238e790a35fce Mon Sep 17 00:00:00 2001 From: Filippo Liverani Date: Tue, 18 Feb 2020 10:12:27 +0100 Subject: [PATCH 3/6] Enable Address name usage in api Favor name attribute usage over firstname and lastname. To avoid internally relying on deprecated code for backwards compatibility, deprecated fields are rendered in API responses only when `show_deprecated_attributes` preference is set to true. --- api/app/helpers/spree/api/api_helpers.rb | 17 ++++++++++++----- api/openapi/api.oas2.yml | 4 ++++ .../requests/spree/api/address_books_spec.rb | 10 ++++------ .../spree/api/checkouts_controller_spec.rb | 7 +++---- .../spree/api/orders_controller_spec.rb | 4 ++-- .../requests/spree/api/users_controller_spec.rb | 6 ++---- core/app/models/spree/address.rb | 1 + core/lib/spree/permitted_attributes.rb | 2 +- 8 files changed, 29 insertions(+), 22 deletions(-) diff --git a/api/app/helpers/spree/api/api_helpers.rb b/api/app/helpers/spree/api/api_helpers.rb index b3374b86a3c..76d4510867a 100644 --- a/api/app/helpers/spree/api/api_helpers.rb +++ b/api/app/helpers/spree/api/api_helpers.rb @@ -121,11 +121,18 @@ def required_fields_for(model) :id, :number, :state, :order_id, :memo, :created_at, :updated_at ] - @@address_attributes = [ - :id, :firstname, :lastname, :full_name, :address1, :address2, :city, - :zipcode, :phone, :company, :alternative_phone, :country_id, :country_iso, - :state_id, :state_name, :state_text - ] + @@address_base_attributes = [ + :id, :name, :address1, :address2, :city, :zipcode, :phone, :company, + :alternative_phone, :country_id, :country_iso, :state_id, :state_name, + :state_text + ] + + @@address_attributes = if Spree::Config.use_combined_first_and_last_name_in_address + @@address_base_attributes + else + @@address_base_attributes + + Spree::Address::LEGACY_NAME_ATTRS.map(&:to_sym) + end @@country_attributes = [:id, :iso_name, :iso, :iso3, :name, :numcode] diff --git a/api/openapi/api.oas2.yml b/api/openapi/api.oas2.yml index 99978e76a67..60ea5f155ac 100644 --- a/api/openapi/api.oas2.yml +++ b/api/openapi/api.oas2.yml @@ -5084,6 +5084,8 @@ definitions: type: integer lastname: type: string + name: + type: string phone: type: string state: @@ -5614,6 +5616,8 @@ definitions: properties: id: type: integer + name: + type: string firstname: type: string lastname: diff --git a/api/spec/requests/spree/api/address_books_spec.rb b/api/spec/requests/spree/api/address_books_spec.rb index bae16f3dac5..d2236bb1e17 100644 --- a/api/spec/requests/spree/api/address_books_spec.rb +++ b/api/spec/requests/spree/api/address_books_spec.rb @@ -7,8 +7,7 @@ module Spree let!(:state) { create(:state) } let!(:harry_address_attributes) do { - 'firstname' => 'Harry', - 'lastname' => 'Potter', + 'name' => 'Harry Potter', 'address1' => '4 Privet Drive', 'address2' => 'cupboard under the stairs', 'city' => 'Surrey', @@ -21,8 +20,7 @@ module Spree let!(:ron_address_attributes) do { - 'firstname' => 'Ron', - 'lastname' => 'Weasly', + 'name' => 'Ron Weasly', 'address1' => 'Ottery St. Catchpole', 'address2' => '4th floor', 'city' => 'Devon, West Country', @@ -55,7 +53,7 @@ module Spree it 'updates my address book' do user = create(:user, spree_api_key: 'galleon') address = user.save_in_address_book(harry_address_attributes, true) - harry_address_attributes['firstname'] = 'Ron' + harry_address_attributes['name'] = 'Ron Weasly' expect { put "/api/users/#{user.id}/address_book", @@ -145,7 +143,7 @@ module Spree it "updates another user's address" do other_user = create(:user) address = other_user.save_in_address_book(harry_address_attributes, true) - updated_harry_address = harry_address_attributes.merge('firstname' => 'Ron') + updated_harry_address = harry_address_attributes.merge('name' => 'Ron Weasly') expect { put "/api/users/#{other_user.id}/address_book", diff --git a/api/spec/requests/spree/api/checkouts_controller_spec.rb b/api/spec/requests/spree/api/checkouts_controller_spec.rb index 6c1acc2babb..d076c9034ee 100644 --- a/api/spec/requests/spree/api/checkouts_controller_spec.rb +++ b/api/spec/requests/spree/api/checkouts_controller_spec.rb @@ -75,8 +75,7 @@ module Spree let(:address) do { - firstname: 'John', - lastname: 'Doe', + name: 'John Doe', address1: '7735 Old Georgetown Road', city: 'Bethesda', phone: '3014445002', @@ -93,8 +92,8 @@ module Spree ship_address_attributes: address } } expect(json_response['state']).to eq('delivery') - expect(json_response['bill_address']['firstname']).to eq('John') - expect(json_response['ship_address']['firstname']).to eq('John') + expect(json_response['bill_address']['name']).to eq('John Doe') + expect(json_response['ship_address']['name']).to eq('John Doe') expect(response.status).to eq(200) end diff --git a/api/spec/requests/spree/api/orders_controller_spec.rb b/api/spec/requests/spree/api/orders_controller_spec.rb index faab02e29fb..a643ea2a824 100644 --- a/api/spec/requests/spree/api/orders_controller_spec.rb +++ b/api/spec/requests/spree/api/orders_controller_spec.rb @@ -498,12 +498,12 @@ module Spree let(:address_params) { { country_id: country.id } } let(:billing_address) { - { firstname: "Tiago", lastname: "Motta", address1: "Av Paulista", + { name: "Tiago Motta", address1: "Av Paulista", city: "Sao Paulo", zipcode: "01310-300", phone: "12345678", country_id: country.id } } let(:shipping_address) { - { firstname: "Tiago", lastname: "Motta", address1: "Av Paulista", + { name: "Tiago Motta", address1: "Av Paulista", city: "Sao Paulo", zipcode: "01310-300", phone: "12345678", country_id: country.id } } diff --git a/api/spec/requests/spree/api/users_controller_spec.rb b/api/spec/requests/spree/api/users_controller_spec.rb index 9e5da533631..e2ba03dd77c 100644 --- a/api/spec/requests/spree/api/users_controller_spec.rb +++ b/api/spec/requests/spree/api/users_controller_spec.rb @@ -47,8 +47,7 @@ module Spree put spree.api_user_path(user.id), params: { token: user.spree_api_key, user: { email: "mine@example.com", bill_address_attributes: { - first_name: 'First', - last_name: 'Last', + name: 'First Last', address1: '1 Test Rd', city: 'City', country_id: country.id, @@ -57,8 +56,7 @@ module Spree phone: '5555555555' }, ship_address_attributes: { - first_name: 'First', - last_name: 'Last', + name: 'First Last', address1: '1 Test Rd', city: 'City', country_id: country.id, diff --git a/core/app/models/spree/address.rb b/core/app/models/spree/address.rb index 206b4e1f2fc..899f36ee524 100644 --- a/core/app/models/spree/address.rb +++ b/core/app/models/spree/address.rb @@ -25,6 +25,7 @@ class Address < Spree::Base DB_ONLY_ATTRS = %w(id updated_at created_at) TAXATION_ATTRS = %w(state_id country_id zipcode) + LEGACY_NAME_ATTRS = %w(firstname lastname full_name) self.whitelisted_ransackable_attributes = %w[firstname lastname] diff --git a/core/lib/spree/permitted_attributes.rb b/core/lib/spree/permitted_attributes.rb index 27c43a30fa9..3a646d8bd3c 100644 --- a/core/lib/spree/permitted_attributes.rb +++ b/core/lib/spree/permitted_attributes.rb @@ -36,7 +36,7 @@ module PermittedAttributes mattr_reader(*ATTRIBUTES) @@address_attributes = [ - :id, :firstname, :lastname, :first_name, :last_name, + :id, :name, :firstname, :lastname, :first_name, :last_name, :address1, :address2, :city, :country_id, :state_id, :zipcode, :phone, :state_name, :country_iso, :alternative_phone, :company, country: [:iso, :name, :iso3, :iso_name], From 0f1f15b3ad863e81fb4c562c347b8a1ae925a44b Mon Sep 17 00:00:00 2001 From: Filippo Liverani Date: Tue, 18 Feb 2020 14:53:39 +0100 Subject: [PATCH 4/6] Enable Address name usage in frontend Favor name attribute usage over firstname and lastname attributes. To avoid internally relying on deprecated code for backwards compatibility, deprecated fields are rendered in HTML views instead of name field only when `show_deprecated_attributes` preference is set to true. --- core/config/locales/en.yml | 3 ++ .../app/views/spree/address/_form.html.erb | 23 ++++++++----- .../views/spree/address/_form_hidden.html.erb | 8 +++-- .../spree/checkout_controller_spec.rb | 4 +-- frontend/spec/features/checkout_spec.rb | 33 +++++++++---------- frontend/spec/features/coupon_code_spec.rb | 3 +- .../features/first_order_promotion_spec.rb | 3 +- .../features/free_shipping_promotions_spec.rb | 3 +- 8 files changed, 45 insertions(+), 35 deletions(-) diff --git a/core/config/locales/en.yml b/core/config/locales/en.yml index 26b39f6ee85..d2cf4043c88 100644 --- a/core/config/locales/en.yml +++ b/core/config/locales/en.yml @@ -28,6 +28,7 @@ en: company: Company firstname: First Name lastname: Last Name + name: Name phone: Phone zipcode: Zip Code spree/adjustment: @@ -131,6 +132,7 @@ en: city: Billing address city firstname: Billing address first name lastname: Billing address last name + name: Billing address name phone: Billing address phone state: Billing address state zipcode: Billing address zipcode @@ -139,6 +141,7 @@ en: city: Shipping address city firstname: Shipping address first name lastname: Shipping address last name + name: Shipping address name phone: Shipping address phone state: Shipping address state zipcode: Shipping address zipcode diff --git a/frontend/app/views/spree/address/_form.html.erb b/frontend/app/views/spree/address/_form.html.erb index 2b38b7720b2..c950e799044 100644 --- a/frontend/app/views/spree/address/_form.html.erb +++ b/frontend/app/views/spree/address/_form.html.erb @@ -1,14 +1,21 @@ <% address_id = address_type.chars.first %>
> -
> - <%= form.label :firstname, t('spree.first_name') %> - <%= form.text_field :firstname, class: 'required', autocomplete: address_type + ' given-name', required: true, autofocus: true %> -
+ <% if Spree::Config.use_combined_first_and_last_name_in_address %> +
"> + <%= form.label :name, t('spree.name') %> + <%= form.text_field :name, class: 'required', autocomplete: address_type + ' name', required: true, autofocus: true %> +
+ <% else %> +
"> + <%= form.label :firstname, t('spree.first_name') %> + <%= form.text_field :firstname, class: 'required', autocomplete: address_type + ' given-name', required: true, autofocus: true %> +
-
> - <%= form.label :lastname, t('spree.last_name') %> - <%= form.text_field :lastname, autocomplete: address_type + ' family-name' %> -
+
"> + <%= form.label :lastname, t('spree.last_name') %> + <%= form.text_field :lastname, autocomplete: address_type + ' family-name' %> +
+ <% end %> <% if Spree::Config[:company] %>
> diff --git a/frontend/app/views/spree/address/_form_hidden.html.erb b/frontend/app/views/spree/address/_form_hidden.html.erb index a605ae145d0..ca0d4ada325 100644 --- a/frontend/app/views/spree/address/_form_hidden.html.erb +++ b/frontend/app/views/spree/address/_form_hidden.html.erb @@ -1,5 +1,9 @@ -<%= form.hidden_field :firstname %> -<%= form.hidden_field :lastname %> +<% if Spree::Config.use_combined_first_and_last_name_in_address %> + <%= form.hidden_field :name %> +<% else %> + <%= form.hidden_field :firstname %> + <%= form.hidden_field :lastname %> +<% end %> <%= form.hidden_field :company %> <%= form.hidden_field :address1 %> <%= form.hidden_field :address2 %> diff --git a/frontend/spec/controllers/spree/checkout_controller_spec.rb b/frontend/spec/controllers/spree/checkout_controller_spec.rb index 84513efe35f..7b3bc3b9d87 100644 --- a/frontend/spec/controllers/spree/checkout_controller_spec.rb +++ b/frontend/spec/controllers/spree/checkout_controller_spec.rb @@ -8,8 +8,8 @@ let(:order) { FactoryBot.create(:order_with_totals) } let(:address_params) do - address = FactoryBot.build(:address) - address.attributes.except("created_at", "updated_at") + attributes_for(:address, name: 'John Doe') + .except("created_at", "updated_at") end before do diff --git a/frontend/spec/features/checkout_spec.rb b/frontend/spec/features/checkout_spec.rb index 4ade202226f..e541662dfb5 100644 --- a/frontend/spec/features/checkout_spec.rb +++ b/frontend/spec/features/checkout_spec.rb @@ -113,16 +113,16 @@ end context "when user has default addresses saved" do - let(:saved_bill_address) { create(:address, name: 'Bill') } - let(:saved_ship_address) { create(:address, name: 'Steve') } + let(:saved_bill_address) { create(:address, name: 'Bill Doe') } + let(:saved_ship_address) { create(:address, name: 'Steve Smith') } it "shows the saved addresses" do within("#billing") do - expect(find_field('First Name').value).to eq 'Bill' + expect(find_field('Name').value).to eq 'Bill Doe' end within("#shipping") do - expect(find_field('First Name').value).to eq 'Steve' + expect(find_field('Name').value).to eq 'Steve Smith' end end end @@ -133,11 +133,11 @@ it 'shows an empty address' do within("#billing") do - expect(find_field('First Name').value).to be_nil + expect(find_field('Name').value).to be_blank end within("#shipping") do - expect(find_field('First Name').value).to be_nil + expect(find_field('Name').value).to be_blank end end end @@ -150,11 +150,11 @@ click_button "Checkout" within("#billing") do - expect(find_field('First Name').value).to be_nil + expect(find_field('Name').value).to be_blank end within("#shipping") do - expect(find_field('First Name').value).to be_nil + expect(find_field('Name').value).to be_blank end end end @@ -183,27 +183,27 @@ it 'shows empty addresses' do within("#billing") do - expect(find_field('First Name').value).to be_nil + expect(find_field('Name').value).to be_blank end within("#shipping") do - expect(find_field('First Name').value).to be_nil + expect(find_field('Name').value).to be_blank end end end # Regression test for https://github.com/solidusio/solidus/issues/1811 context "when does have saved addresses" do - let(:saved_bill_address) { create(:address, name: 'Bill') } - let(:saved_ship_address) { create(:address, name: 'Steve') } + let(:saved_bill_address) { create(:address, name: 'Bill Doe') } + let(:saved_ship_address) { create(:address, name: 'Steve Smith') } it 'shows empty addresses' do within("#billing") do - expect(find_field('First Name').value).to eq 'Bill' + expect(find_field('Name').value).to eq 'Bill Doe' end within("#shipping") do - expect(find_field('First Name').value).to eq 'Steve' + expect(find_field('Name').value).to eq 'Steve Smith' end end end @@ -396,7 +396,7 @@ click_on "Checkout" # edit an address field - fill_in "order_bill_address_attributes_firstname", with: "Ryann" + fill_in "order_bill_address_attributes_name", with: "Ryann Bigg" click_on "Save and Continue" click_on "Save and Continue" click_on "Save and Continue" @@ -732,8 +732,7 @@ def fill_in_credit_card(number: "4111 1111 1111 1111") def fill_in_address address = "order_bill_address_attributes" - fill_in "#{address}_firstname", with: "Ryan" - fill_in "#{address}_lastname", with: "Bigg" + fill_in "#{address}_name", with: "Ryan Bigg" fill_in "#{address}_address1", with: "143 Swan Street" fill_in "#{address}_city", with: "Richmond" select "United States of America", from: "#{address}_country_id" diff --git a/frontend/spec/features/coupon_code_spec.rb b/frontend/spec/features/coupon_code_spec.rb index 2128bffbecd..2d82d2e81cb 100644 --- a/frontend/spec/features/coupon_code_spec.rb +++ b/frontend/spec/features/coupon_code_spec.rb @@ -42,8 +42,7 @@ def create_basic_coupon_promotion(code) click_button "add-to-cart-button" click_button "Checkout" fill_in "order_email", with: "spree@example.com" - fill_in "First Name", with: "John" - fill_in "Last Name", with: "Smith" + fill_in "Name", with: "John Smith" fill_in "Street Address", with: "1 John Street" fill_in "City", with: "City of John" fill_in "Zip", with: "01337" diff --git a/frontend/spec/features/first_order_promotion_spec.rb b/frontend/spec/features/first_order_promotion_spec.rb index fdf01cb00ad..d9aaeed1889 100644 --- a/frontend/spec/features/first_order_promotion_spec.rb +++ b/frontend/spec/features/first_order_promotion_spec.rb @@ -48,8 +48,7 @@ def fill_in_address address = "order_bill_address_attributes" - fill_in "#{address}_firstname", with: "Ryan" - fill_in "#{address}_lastname", with: "Bigg" + fill_in "#{address}_name", with: "Ryan Bigg" fill_in "#{address}_address1", with: "143 Swan Street" fill_in "#{address}_city", with: "Richmond" select "United States of America", from: "#{address}_country_id" diff --git a/frontend/spec/features/free_shipping_promotions_spec.rb b/frontend/spec/features/free_shipping_promotions_spec.rb index 68edd8a552b..e36cd9b329d 100644 --- a/frontend/spec/features/free_shipping_promotions_spec.rb +++ b/frontend/spec/features/free_shipping_promotions_spec.rb @@ -34,8 +34,7 @@ click_button "add-to-cart-button" click_button "Checkout" fill_in "order_email", with: "spree@example.com" - fill_in "First Name", with: "John" - fill_in "Last Name", with: "Smith" + fill_in "Name", with: "John Smith" fill_in "Street Address", with: "1 John Street" fill_in "City", with: "City of John" fill_in "Zip", with: "01337" From bd32a889f64494ce31aa3f88b3e2bd9a18d54b28 Mon Sep 17 00:00:00 2001 From: Filippo Liverani Date: Tue, 18 Feb 2020 17:00:54 +0100 Subject: [PATCH 5/6] Enable Address name usage in backend Favor name attribute usage in backend over firstname and lastname. To avoid internally relying on deprecated code for backwards compatibility, deprecated fields are rendered in HTML views and API responses instead of name field only when `show_deprecated_attributes` preference is set to true. --- .../orders/customer_details/autocomplete.hbs | 4 +- .../javascripts/spree/backend/user_picker.js | 3 +- .../spree/backend/views/order/address.js | 2 +- .../backend/views/order/customer_select.js | 3 +- .../spree/backend/sections/_orders.scss | 6 ++- .../spree/admin/search_controller.rb | 3 +- .../views/spree/admin/orders/index.html.erb | 23 ++++++---- .../spree/admin/search/users.json.jbuilder | 16 ++++++- .../spree/admin/shared/_address_form.html.erb | 23 ++++++---- .../spree/admin/search_controller_spec.rb | 22 +-------- .../spree/admin/users_controller_spec.rb | 21 ++++----- .../admin/orders/customer_details_spec.rb | 17 +++---- .../features/admin/orders/new_order_spec.rb | 3 +- .../features/admin/orders/payments_spec.rb | 46 ++++++++++--------- .../app/models/concerns/spree/user_methods.rb | 3 +- core/app/models/spree/order.rb | 3 +- core/config/locales/en.yml | 1 + 17 files changed, 105 insertions(+), 94 deletions(-) diff --git a/backend/app/assets/javascripts/spree/backend/templates/orders/customer_details/autocomplete.hbs b/backend/app/assets/javascripts/spree/backend/templates/orders/customer_details/autocomplete.hbs index f15fc508d82..db6b4eff4a1 100644 --- a/backend/app/assets/javascripts/spree/backend/templates/orders/customer_details/autocomplete.hbs +++ b/backend/app/assets/javascripts/spree/backend/templates/orders/customer_details/autocomplete.hbs @@ -1,9 +1,9 @@
{{customer.email}} - {{#if bill_address.firstname }} + {{#if bill_address.name }} {{t 'bill_address' }} - {{bill_address.firstname}} {{bill_address.lastname}}
+ {{bill_address.name}}
{{bill_address.address1}}, {{bill_address.address2}}
{{bill_address.city}}
{{#if bill_address.state_id }} diff --git a/backend/app/assets/javascripts/spree/backend/user_picker.js b/backend/app/assets/javascripts/spree/backend/user_picker.js index 47de8625e99..fc87106a4c3 100644 --- a/backend/app/assets/javascripts/spree/backend/user_picker.js +++ b/backend/app/assets/javascripts/spree/backend/user_picker.js @@ -28,8 +28,7 @@ $.fn.userAutocomplete = function () { q: { m: 'or', email_start: term, - addresses_firstname_start: term, - addresses_lastname_start: term + firstname_or_lastname_start: term } }; }, diff --git a/backend/app/assets/javascripts/spree/backend/views/order/address.js b/backend/app/assets/javascripts/spree/backend/views/order/address.js index ae35a7c7fc8..e7cd6376e70 100644 --- a/backend/app/assets/javascripts/spree/backend/views/order/address.js +++ b/backend/app/assets/javascripts/spree/backend/views/order/address.js @@ -23,7 +23,7 @@ Spree.Views.Order.Address = Backbone.View.extend({ eachField: function(callback){ var view = this; - var fields = ["firstname", "lastname", "company", "address1", "address2", + var fields = ["name", "company", "address1", "address2", "city", "zipcode", "phone", "country_id", "state_name"]; _.each(fields, function(field) { var el = view.$('[name$="[' + field + ']"]'); diff --git a/backend/app/assets/javascripts/spree/backend/views/order/customer_select.js b/backend/app/assets/javascripts/spree/backend/views/order/customer_select.js index d55d0e1d284..7ff8ce092ca 100644 --- a/backend/app/assets/javascripts/spree/backend/views/order/customer_select.js +++ b/backend/app/assets/javascripts/spree/backend/views/order/customer_select.js @@ -34,8 +34,7 @@ Spree.Views.Order.CustomerSelect = Backbone.View.extend({ q: { m: 'or', email_start: term, - addresses_firstname_start: term, - addresses_lastname_start: term + firstname_or_lastname_start: term } } }, diff --git a/backend/app/assets/stylesheets/spree/backend/sections/_orders.scss b/backend/app/assets/stylesheets/spree/backend/sections/_orders.scss index b5eb68a216f..b29088a1935 100644 --- a/backend/app/assets/stylesheets/spree/backend/sections/_orders.scss +++ b/backend/app/assets/stylesheets/spree/backend/sections/_orders.scss @@ -65,7 +65,11 @@ } label[for="order_bill_address_attributes_firstname"] { - margin-top: 45px; + margin-top: 53.5px; +} + +label[for="order_bill_address_attributes_name"] { + margin-top: 53.5px; } #risk_analysis legend { diff --git a/backend/app/controllers/spree/admin/search_controller.rb b/backend/app/controllers/spree/admin/search_controller.rb index 80f2f54c94a..c2baa511857 100644 --- a/backend/app/controllers/spree/admin/search_controller.rb +++ b/backend/app/controllers/spree/admin/search_controller.rb @@ -16,8 +16,7 @@ def users @users = Spree.user_class.ransack({ m: 'or', email_start: params[:q], - addresses_firstname_start: params[:q], - addresses_lastname_start: params[:q] + firstname_or_lastname_start: params[:q] }).result.limit(10) end end diff --git a/backend/app/views/spree/admin/orders/index.html.erb b/backend/app/views/spree/admin/orders/index.html.erb index ca4d830e63c..2b3063544a8 100644 --- a/backend/app/views/spree/admin/orders/index.html.erb +++ b/backend/app/views/spree/admin/orders/index.html.erb @@ -65,14 +65,21 @@
-
- <%= label_tag :q_bill_address_firstname_start, t('spree.first_name_begins_with') %> - <%= f.text_field :bill_address_firstname_start, size: 25 %> -
-
- <%= label_tag :q_bill_address_lastname_start, t('spree.last_name_begins_with') %> - <%= f.text_field :bill_address_lastname_start, size: 25%> -
+ <% if Spree::Config.use_combined_first_and_last_name_in_address %> +
+ <%= label_tag :q_bill_address_firstname_or_lastname_start, t('spree.name_contains') %> + <%= f.text_field :bill_address_firstname_or_lastname_start, size: 25 %> +
+ <% else %> +
+ <%= label_tag :q_bill_address_firstname_start, t('spree.first_name_begins_with') %> + <%= f.text_field :bill_address_firstname_start, size: 25 %> +
+
+ <%= label_tag :q_bill_address_lastname_start, t('spree.last_name_begins_with') %> + <%= f.text_field :bill_address_lastname_start, size: 25 %> +
+ <% end %>
diff --git a/backend/app/views/spree/admin/search/users.json.jbuilder b/backend/app/views/spree/admin/search/users.json.jbuilder index 24f87bb0445..83f70c666ab 100644 --- a/backend/app/views/spree/admin/search/users.json.jbuilder +++ b/backend/app/views/spree/admin/search/users.json.jbuilder @@ -4,7 +4,21 @@ json.array!(@users) do |user| json.id user.id json.email user.email - address_fields = [:firstname, :lastname, :address1, :address2, :city, :zipcode, :phone, :state_name, :state_id, :country_id, :company] + address_fields = [ + :name, + :address1, + :address2, + :city, + :zipcode, + :phone, + :state_name, + :state_id, + :country_id, + :company + ] + unless Spree::Config.use_combined_first_and_last_name_in_address + address_fields.push(:firstname, :lastname) + end json.ship_address do if user.ship_address json.(user.ship_address, *address_fields) diff --git a/backend/app/views/spree/admin/shared/_address_form.html.erb b/backend/app/views/spree/admin/shared/_address_form.html.erb index cb1e1dd9293..dda71610d3b 100644 --- a/backend/app/views/spree/admin/shared/_address_form.html.erb +++ b/backend/app/views/spree/admin/shared/_address_form.html.erb @@ -1,15 +1,22 @@ <% s_or_b = type.chars.first %>
-
"> - <%= f.label :firstname %> - <%= f.text_field :firstname, class: 'fullwidth' %> -
+ <% if Spree::Config.use_combined_first_and_last_name_in_address %> +
"> + <%= f.label :name %> + <%= f.text_field :name, class: 'fullwidth' %> +
+ <% else %> +
"> + <%= f.label :firstname %> + <%= f.text_field :firstname, class: 'fullwidth' %> +
-
"> - <%= f.label :lastname %> - <%= f.text_field :lastname, class: 'fullwidth' %> -
+
"> + <%= f.label :lastname %> + <%= f.text_field :lastname, class: 'fullwidth' %> +
+ <% end %> <% if Spree::Config[:company] %>
"> diff --git a/backend/spec/controllers/spree/admin/search_controller_spec.rb b/backend/spec/controllers/spree/admin/search_controller_spec.rb index 199795dffd2..adab87f5e28 100644 --- a/backend/spec/controllers/spree/admin/search_controller_spec.rb +++ b/backend/spec/controllers/spree/admin/search_controller_spec.rb @@ -39,27 +39,9 @@ end end - context 'when searching by ship addresss first name' do + context 'when searching by bill address name' do it_should_behave_like 'user found by search' do - let(:user_attribute) { user.ship_address.firstname } - end - end - - context 'when searching by ship address last name' do - it_should_behave_like 'user found by search' do - let(:user_attribute) { user.ship_address.lastname } - end - end - - context 'when searching by bill address first name' do - it_should_behave_like 'user found by search' do - let(:user_attribute) { user.bill_address.firstname } - end - end - - context 'when searching by bill address last name' do - it_should_behave_like 'user found by search' do - let(:user_attribute) { user.bill_address.lastname } + let(:user_attribute) { user.bill_address.name } end end end diff --git a/backend/spec/controllers/spree/admin/users_controller_spec.rb b/backend/spec/controllers/spree/admin/users_controller_spec.rb index 415d7a935b8..6606dc96aba 100644 --- a/backend/spec/controllers/spree/admin/users_controller_spec.rb +++ b/backend/spec/controllers/spree/admin/users_controller_spec.rb @@ -8,8 +8,7 @@ let(:state) { create(:state, state_code: 'NY') } let(:valid_address_attributes) do { - firstname: 'Foo', - lastname: 'Bar', + name: 'Foo Bar', city: "New York", country_id: state.country.id, state_id: state.id, @@ -89,24 +88,24 @@ def user end it "can create user with roles" do - post :create, params: { user: { first_name: "Bob", spree_role_ids: [dummy_role.id] } } + post :create, params: { user: { name: "Bob Bloggs", spree_role_ids: [dummy_role.id] } } expect(user.spree_roles).to eq([dummy_role]) end it "can create user without roles" do - post :create, params: { user: { first_name: "Bob" } } + post :create, params: { user: { name: "Bob Bloggs" } } expect(user.spree_roles).to eq([]) end end context "when the user cannot manage roles" do it "cannot assign users roles" do - post :create, params: { user: { first_name: "Bob", spree_role_ids: [dummy_role.id] } } + post :create, params: { user: { name: "Bob Bloggs", spree_role_ids: [dummy_role.id] } } expect(user.spree_roles).to eq([]) end it "can create user without roles" do - post :create, params: { user: { first_name: "Bob" } } + post :create, params: { user: { name: "Bob Bloggs" } } expect(user.spree_roles).to eq([]) end end @@ -145,21 +144,21 @@ def user it "can set roles" do expect { - put :update, params: { id: user.id, user: { first_name: "Bob", spree_role_ids: [dummy_role.id] } } + put :update, params: { id: user.id, user: { name: "Bob Bloggs", spree_role_ids: [dummy_role.id] } } }.to change { user.reload.spree_roles.to_a }.to([dummy_role]) end it "can clear roles" do user.spree_roles << dummy_role expect { - put :update, params: { id: user.id, user: { first_name: "Bob", spree_role_ids: [""] } } + put :update, params: { id: user.id, user: { name: "Bob Bloggs", spree_role_ids: [""] } } }.to change { user.reload.spree_roles.to_a }.to([]) end context 'when no role params are present' do it 'does not clear all present user roles' do user.spree_roles << dummy_role - put :update, params: { id: user.id, user: { first_name: "Bob" } } + put :update, params: { id: user.id, user: { name: "Bob Bloggs" } } expect(user.reload.spree_roles).to_not be_empty end end @@ -168,14 +167,14 @@ def user context "when the user cannot manage roles" do it "cannot set roles" do expect { - put :update, params: { id: user.id, user: { first_name: "Bob", spree_role_ids: [dummy_role.id] } } + put :update, params: { id: user.id, user: { name: "Bob Bloggs", spree_role_ids: [dummy_role.id] } } }.not_to change { user.reload.spree_roles.to_a } end it "cannot clear roles" do user.spree_roles << dummy_role expect { - put :update, params: { id: user.id, user: { first_name: "Bob" } } + put :update, params: { id: user.id, user: { name: "Bob Bloggs" } } }.not_to change { user.reload.spree_roles.to_a } end end diff --git a/backend/spec/features/admin/orders/customer_details_spec.rb b/backend/spec/features/admin/orders/customer_details_spec.rb index 6ed9af845f7..8c80d89734e 100644 --- a/backend/spec/features/admin/orders/customer_details_spec.rb +++ b/backend/spec/features/admin/orders/customer_details_spec.rb @@ -14,8 +14,8 @@ let!(:product) { create(:product_in_stock) } # We need a unique name that will appear for the customer dropdown - let!(:ship_address) { create(:address, country: country, state: state, first_name: "Rumpelstiltskin") } - let!(:bill_address) { create(:address, country: country, state: state, first_name: "Rumpelstiltskin") } + let!(:ship_address) { create(:address, country: country, state: state, name: "Jane Doe") } + let!(:bill_address) { create(:address, country: country, state: state, name: "Jane Doe") } let!(:user) { create(:user, email: 'foobar@example.com', ship_address: ship_address, bill_address: bill_address) } @@ -37,8 +37,7 @@ # Regression test for https://github.com/spree/spree/issues/3335 and https://github.com/spree/spree/issues/5317 it "associates a user when not using guest checkout" do # 5317 - Address prefills using user's default. - expect(page).to have_field('First Name', with: user.bill_address.firstname) - expect(page).to have_field('Last Name', with: user.bill_address.lastname) + expect(page).to have_field('Name', with: user.bill_address.name) expect(page).to have_field('Street Address', with: user.bill_address.address1) expect(page).to have_field("Street Address (cont'd)", with: user.bill_address.address2) expect(page).to have_field('City', with: user.bill_address.city) @@ -120,7 +119,7 @@ order.update!(ship_address_id: nil) click_link "Customer" click_button "Update" - expect(page).to have_content("Shipping address first name can't be blank") + expect(page).to have_content("Shipping address name can't be blank") end context "for an order in confirm state with a user" do @@ -171,9 +170,8 @@ specify do click_link "Customer" # Need to fill in valid information so it passes validations - fill_in "order_ship_address_attributes_firstname", with: "John 99" - fill_in "order_ship_address_attributes_lastname", with: "Doe" - fill_in "order_ship_address_attributes_lastname", with: "Company" + fill_in "order_ship_address_attributes_name", with: "John 99 Doe" + fill_in "order_ship_address_attributes_company", with: "Company" fill_in "order_ship_address_attributes_address1", with: "100 first lane" fill_in "order_ship_address_attributes_address2", with: "#101" fill_in "order_ship_address_attributes_city", with: "Bethesda" @@ -190,8 +188,7 @@ end def fill_in_address - fill_in "First Name", with: "John 99" - fill_in "Last Name", with: "Doe" + fill_in "Name", with: "John 99 Doe" fill_in "Company", with: "Company" fill_in "Street Address", with: "100 first lane" fill_in "Street Address (cont'd)", with: "#101" diff --git a/backend/spec/features/admin/orders/new_order_spec.rb b/backend/spec/features/admin/orders/new_order_spec.rb index f95af00380b..98d261ebb1b 100644 --- a/backend/spec/features/admin/orders/new_order_spec.rb +++ b/backend/spec/features/admin/orders/new_order_spec.rb @@ -367,8 +367,7 @@ end def fill_in_address - fill_in "First Name", with: "John 99" - fill_in "Last Name", with: "Doe" + fill_in "Name", with: "John 99 Doe" fill_in "Street Address", with: "100 first lane" fill_in "Street Address (cont'd)", with: "#101" fill_in "City", with: "Bethesda" diff --git a/backend/spec/features/admin/orders/payments_spec.rb b/backend/spec/features/admin/orders/payments_spec.rb index 5f897917f42..dd86bb9870c 100644 --- a/backend/spec/features/admin/orders/payments_spec.rb +++ b/backend/spec/features/admin/orders/payments_spec.rb @@ -7,18 +7,6 @@ let(:state) { 'checkout' } - def create_payment(opts = {}) - create( - :payment, - { - order: order, - amount: order.outstanding_balance, - payment_method: create(:credit_card_payment_method), - state: state - }.merge(opts) - ) - end - context "with a pre-existing payment" do let!(:payment) { create_payment } @@ -205,10 +193,7 @@ def create_payment(opts = {}) end it "is able to create a new credit card payment with valid information", js: true do - fill_in_with_force "Card Number", with: "4111 1111 1111 1111" - fill_in "Name", with: "Test User" - fill_in_with_force "Expiration", with: "09 / #{Time.current.year + 1}" - fill_in "Card Code", with: "007" + fill_in_credit_card_form # Regression test for https://github.com/spree/spree/issues/4277 expect(page).to have_css('.ccType[value="visa"]', visible: false) click_button "Continue" @@ -290,10 +275,7 @@ def create_payment(opts = {}) it "is able to create a new payment", js: true do choose payment_method.name - fill_in_with_force "Card Number", with: "4111 1111 1111 1111" - fill_in "Name", with: "Test User" - fill_in_with_force "Expiration", with: "09 / #{Time.current.year + 1}" - fill_in "Card Code", with: "007" + fill_in_credit_card_form click_button "Continue" expect(page).to have_content("Payment has been successfully created!") end @@ -319,12 +301,32 @@ def create_payment(opts = {}) it "displays an error" do choose payment_method.name + fill_in_credit_card_form + click_button "Continue" + expect(page).to have_content I18n.t('spree.insufficient_stock_for_order') + end + end + + private + + def create_payment(opts = {}) + create( + :payment, + { + order: order, + amount: order.outstanding_balance, + payment_method: create(:credit_card_payment_method), + state: state + }.merge(opts) + ) + end + + def fill_in_credit_card_form + within('.js-new-credit-card-form') do fill_in_with_force "Card Number", with: "4111 1111 1111 1111" fill_in "Name", with: "Test User" fill_in_with_force "Expiration", with: "09 / #{Time.current.year + 1}" fill_in "Card Code", with: "007" - click_button "Continue" - expect(page).to have_content I18n.t('spree.insufficient_stock_for_order') end end end diff --git a/core/app/models/concerns/spree/user_methods.rb b/core/app/models/concerns/spree/user_methods.rb index e50e5d90ca3..7931c20781e 100644 --- a/core/app/models/concerns/spree/user_methods.rb +++ b/core/app/models/concerns/spree/user_methods.rb @@ -34,8 +34,9 @@ module UserMethods include Spree::RansackableAttributes unless included_modules.include?(Spree::RansackableAttributes) + ransack_alias :firstname_or_lastname, :addresses_firstname_or_addresses_lastname self.whitelisted_ransackable_associations = %w[addresses spree_roles] - self.whitelisted_ransackable_attributes = %w[id email created_at] + self.whitelisted_ransackable_attributes = %w[firstname_or_lastname id email created_at] end def wallet diff --git a/core/app/models/spree/order.rb b/core/app/models/spree/order.rb index 020e98ba18d..acccd006c27 100644 --- a/core/app/models/spree/order.rb +++ b/core/app/models/spree/order.rb @@ -42,8 +42,9 @@ class CannotRebuildShipments < StandardError; end go_to_state :confirm end + ransack_alias :bill_address_firstname_or_lastname, :bill_address_firstname_or_bill_address_lastname self.whitelisted_ransackable_associations = %w[shipments user order_promotions promotions bill_address ship_address line_items] - self.whitelisted_ransackable_attributes = %w[completed_at created_at email number state payment_state shipment_state total store_id] + self.whitelisted_ransackable_attributes = %w[bill_address_firstname_or_lastname completed_at created_at email number state payment_state shipment_state total store_id] attr_reader :coupon_code attr_accessor :temporary_address diff --git a/core/config/locales/en.yml b/core/config/locales/en.yml index d2cf4043c88..9749fd6204d 100644 --- a/core/config/locales/en.yml +++ b/core/config/locales/en.yml @@ -1570,6 +1570,7 @@ en: my_account: My Account my_orders: My Orders name: Name + name_contains: Name Contains name_on_card: Name on card name_or_sku: Name or SKU (enter at least first 4 characters of product name) negative_movement_absent_item: Cannot create negative movement for absent stock From 8b43d286b3ee77888c504fc9e590bac27c969258 Mon Sep 17 00:00:00 2001 From: Filippo Liverani Date: Tue, 25 Feb 2020 12:38:09 +0100 Subject: [PATCH 6/6] Add Address name attribute to developer guide --- guides/source/developers/users/addresses.html.md | 1 + 1 file changed, 1 insertion(+) diff --git a/guides/source/developers/users/addresses.html.md b/guides/source/developers/users/addresses.html.md index 28a98b42f56..1c42837b3bd 100644 --- a/guides/source/developers/users/addresses.html.md +++ b/guides/source/developers/users/addresses.html.md @@ -17,6 +17,7 @@ Spree::User.find(1).addresses `Spree::Address` objects have the following attributes: +- `name`: The full name for the person at this address. - `firstname`: The first name for the person at this address. - `lastname`: The last name for the person at this address. - `address1` and `address2`: The street address (with an optional second line).