From c85691891fc5170dfe25812796e5226b1189d43c Mon Sep 17 00:00:00 2001 From: Mayur Shah Date: Fri, 21 Feb 2025 12:35:17 +0530 Subject: [PATCH 1/4] Update core to have firstname and lastname on address - Unignored `firstname` and `lastname` field in the address model. - Updated validations, attributes, and views to handle 'firstname' and 'lastname' separately. - Modified tests, factories, and locales to align with the new structure. - Introduced a 'set_full_name' method callback to concatenate 'firstname' and 'lastname' for backward compatibility. --- core/app/models/spree/address.rb | 18 +++- core/config/locales/en.yml | 8 ++ core/lib/spree/permitted_attributes.rb | 2 +- .../factories/address_factory.rb | 2 + .../lib/spree/core/importer/order_spec.rb | 2 + core/spec/models/spree/address_spec.rb | 93 +++++++++++++++++-- .../spree/concerns/user_address_book_spec.rb | 14 ++- core/spec/models/spree/credit_card_spec.rb | 2 + core/spec/models/spree/order_spec.rb | 11 ++- sample/db/samples/addresses.rb | 13 ++- 10 files changed, 140 insertions(+), 25 deletions(-) diff --git a/core/app/models/spree/address.rb b/core/app/models/spree/address.rb index 7e03938d937..dd0cfd5c296 100644 --- a/core/app/models/spree/address.rb +++ b/core/app/models/spree/address.rb @@ -14,19 +14,22 @@ class Address < Spree::Base belongs_to :country, class_name: "Spree::Country", optional: true belongs_to :state, class_name: "Spree::State", optional: true - validates :address1, :city, :country_id, :name, 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 do self.class.state_validator_class.new(self).perform end - self.ignored_columns = %w(firstname lastname) + before_save :set_full_name + DB_ONLY_ATTRS = %w(id updated_at created_at).freeze TAXATION_ATTRS = %w(state_id country_id zipcode).freeze - self.allowed_ransackable_attributes = %w[name] + self.allowed_ransackable_attributes = %w[name firstname lastname] unless ActiveRecord::Relation.method_defined? :with_values # Rails 7.1+ scope :with_values, ->(attributes) do @@ -82,6 +85,10 @@ def state_text state.try(:abbr) || state.try(:name) || state_name end + def set_full_name + self.name = "#{firstname} #{lastname}".strip if name.blank? + end + def to_s "#{name}: #{address1}" end @@ -138,5 +145,10 @@ def country_iso=(iso) def country_iso country && country.iso end + + def validate_name + errors.add(:firstname, :blank) if firstname.blank? && (name.blank? || lastname.present?) + errors.add(:name, :blank) if name.blank? && firstname.blank? + end end end diff --git a/core/config/locales/en.yml b/core/config/locales/en.yml index b95282b9d3f..1ee1f0d2317 100644 --- a/core/config/locales/en.yml +++ b/core/config/locales/en.yml @@ -24,6 +24,8 @@ en: address2: Street Address (cont'd) city: City company: Company + firstname: First Name + lastname: Last Name name: Name phone: Phone zipcode: Zip Code @@ -127,6 +129,8 @@ en: spree/order/bill_address: address1: Billing address street 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 @@ -134,6 +138,8 @@ en: spree/order/ship_address: address1: Shipping address street 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 @@ -1568,6 +1574,7 @@ en: first_item: First Item first_name: First Name first_name_begins_with: First Name Begins With + firstname: First Name flat_percent: Flat Percent flat_rate_per_order: Flat Rate flexible_rate: Flexible Rate @@ -1706,6 +1713,7 @@ en: path: Path last_name: Last Name last_name_begins_with: Last Name Begins With + lastname: Last Name learn_more: Learn More lifetime_stats: Lifetime Stats line_item_adjustments: Line item adjustments diff --git a/core/lib/spree/permitted_attributes.rb b/core/lib/spree/permitted_attributes.rb index ff783a8bf17..ce817dea7f0 100644 --- a/core/lib/spree/permitted_attributes.rb +++ b/core/lib/spree/permitted_attributes.rb @@ -39,7 +39,7 @@ module PermittedAttributes mattr_reader(*ATTRIBUTES) @@address_attributes = [ - :id, :name, :address1, :address2, :city, :country_id, :state_id, + :id, :name, :firstname, :lastname, :address1, :address2, :city, :country_id, :state_id, :zipcode, :phone, :state_name, :country_iso, :alternative_phone, :company, country: [:iso, :name, :iso3, :iso_name], state: [:name, :abbr] diff --git a/core/lib/spree/testing_support/factories/address_factory.rb b/core/lib/spree/testing_support/factories/address_factory.rb index 3a14d02d7e4..cf823aae588 100644 --- a/core/lib/spree/testing_support/factories/address_factory.rb +++ b/core/lib/spree/testing_support/factories/address_factory.rb @@ -9,6 +9,8 @@ end name { 'John Von Doe' } + firstname { 'John' } + lastname { 'Von Doe' } company { 'Company' } address1 { '10 Lovely Street' } address2 { 'Northwest' } diff --git a/core/spec/lib/spree/core/importer/order_spec.rb b/core/spec/lib/spree/core/importer/order_spec.rb index 56c244fb20d..05be3f6c913 100644 --- a/core/spec/lib/spree/core/importer/order_spec.rb +++ b/core/spec/lib/spree/core/importer/order_spec.rb @@ -37,6 +37,8 @@ module Core { address1: '123 Testable Way', name: 'Fox Mulder', + firstname: 'Fox', + lastname: 'Mulder', city: 'Washington', country_id: country.id, state_id: state.id, diff --git a/core/spec/models/spree/address_spec.rb b/core/spec/models/spree/address_spec.rb index 0f2eea77233..ceaadd6cd79 100644 --- a/core/spec/models/spree/address_spec.rb +++ b/core/spec/models/spree/address_spec.rb @@ -64,6 +64,49 @@ expect(address.errors[:zipcode]).to be_blank end end + + context 'name validation' do + it 'adds an error when both firstname and name are blank' do + address.firstname = '' + address.name = '' + address.validate_name + expect(address.errors[:name]).to include("can't be blank") + end + + it 'adds an error when firstname is blank and name is blank but lastname is present' do + address.firstname = '' + address.name = '' + address.lastname = 'Doe' + address.validate_name + expect(address.errors[:firstname]).to include("can't be blank") + end + + it 'does not add an error when firstname is present and name is blank' do + address.firstname = 'John' + address.name = '' + address.validate_name + expect(address.errors[:firstname]).to be_empty + expect(address).to be_valid + end + + it 'does not add an error when name is present and firstname is blank' do + address.firstname = '' + address.lastname = '' + address.name = 'John Doe' + address.validate_name + expect(address.errors[:name]).to be_empty + expect(address).to be_valid + end + + it 'does not add an error when both firstname and name are present' do + address.firstname = 'John' + address.name = 'Doe' + address.validate_name + expect(address.errors[:firstname]).to be_empty + expect(address.errors[:name]).to be_empty + expect(address).to be_valid + end + end end context ".build_default" do @@ -80,19 +123,22 @@ end it 'accepts other attributes' do - address = Spree::Address.build_default(name: 'Ryan') + address = Spree::Address.build_default(firstname: 'Ryan', name: 'Ryan') expect(address.country).to eq default_country expect(address.name).to eq 'Ryan' + expect(address.firstname).to eq 'Ryan' end it 'accepts a block' do address = Spree::Address.build_default do |record| record.name = 'Ryan' + record.firstname = 'Ryan' end expect(address.country).to eq default_country expect(address.name).to eq 'Ryan' + expect(address.firstname).to eq 'Ryan' end it 'can override the country' do @@ -126,7 +172,7 @@ context ".immutable_merge" do RSpec::Matchers.define :be_address_equivalent_attributes do |expected| - fields_of_interest = [:name, :company, :address1, :address2, :city, :zipcode, :phone, :alternative_phone] + fields_of_interest = [:name, :firstname, :lastname, :company, :address1, :address2, :city, :zipcode, :phone, :alternative_phone] match do |actual| expected_attrs = expected.symbolize_keys.slice(*fields_of_interest) actual_attrs = actual.symbolize_keys.slice(*fields_of_interest) @@ -148,7 +194,7 @@ context 'and there is a matching address in the database' do let(:new_address_attributes) { Spree::Address.value_attributes(matching_address.attributes) } - let!(:matching_address) { create(:address, name: 'Jordan') } + let!(:matching_address) { create(:address, name: 'Jordan', firstname: 'Jordan') } it "returns the matching address" do expect(subject.attributes).to be_address_equivalent_attributes(new_address_attributes) @@ -177,7 +223,7 @@ context 'and changed address matches an existing address' do let(:new_address_attributes) { Spree::Address.value_attributes(matching_address.attributes) } - let!(:matching_address) { create(:address, name: 'Jordan') } + let!(:matching_address) { create(:address, name: 'Jordan', firstname: 'Jordan') } it 'returns the matching address' do expect(subject.attributes).to be_address_equivalent_attributes(new_address_attributes) @@ -225,10 +271,10 @@ describe '.taxation_attributes' do context 'both taxation and non-taxation attributes are present ' do - let(:address) { Spree::Address.new name: 'Michael Jackson', state_id: 1, country_id: 2, zipcode: '12345' } + let(:address) { Spree::Address.new name: 'Michael Jackson', firstname: 'Michael', lastname: 'Jackson', state_id: 1, country_id: 2, zipcode: '12345' } it 'removes the non-taxation attributes' do - expect(address.taxation_attributes).not_to eq('name' => 'Michael Jackson') + expect(address.taxation_attributes).not_to eq('name' => 'Michael Jackson', 'firstname' => 'Michael', 'lastname' => 'Jackson') end it 'returns only the taxation attributes' do @@ -237,7 +283,7 @@ end context 'taxation attributes are blank' do - let(:address) { Spree::Address.new name: 'Michael Jackson' } + let(:address) { Spree::Address.new name: 'Michael Jackson', firstname: 'Michael', lastname: 'Jackson' } it 'returns a subset of the attributes with the correct keys and nil values' do expect(address.taxation_attributes).to eq('state_id' => nil, 'country_id' => nil, 'zipcode' => nil) @@ -263,10 +309,39 @@ context '#name' do it 'is included in json representation' do - address = Spree::Address.new(name: 'Jane Von Doe') + address = Spree::Address.new(firstname: 'Jane', lastname: 'Von Doe', name: 'Jane Von Doe') + expect(address.as_json).to include('firstname' => 'Jane') + expect(address.as_json).to include('lastname' => 'Von Doe') expect(address.as_json).to include('name' => 'Jane Von Doe') - expect(address.as_json.keys).not_to include('firstname', 'lastname') + expect(address.as_json.keys).to include('firstname', 'lastname', 'name') + end + + let(:address) { build(:address, firstname: 'Jane', lastname: 'Von Doe', name: '') } + + it 'returns the concatenated full name' do + address.save + expect(address.name).to eq('Jane Von Doe') + end + end + + describe '#set_full_name' do + let(:address) { build(:address, firstname: 'John', lastname: 'Doe') } + + context 'when name is blank' do + it 'sets the name as concatenation of firstname and lastname' do + address.name = '' + address.save + expect(address.name).to eq('John Doe') + end + end + + context 'when name is set by the user' do + it 'does not modify the name if it was set manually' do + address.name = 'Custom Name' + address.save + expect(address.name).to eq('Custom Name') + end end end 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 48e155f8361..234ff2e62e8 100644 --- a/core/spec/models/spree/concerns/user_address_book_spec.rb +++ b/core/spec/models/spree/concerns/user_address_book_spec.rb @@ -134,6 +134,10 @@ module Spree expect(subject.city).to eq updated_address_attributes[:city] end + it "preserves firstname" do + expect(subject.firstname).to eq address.firstname + end + it "preserves name" do expect(subject.name).to eq address.name end @@ -158,9 +162,12 @@ module Spree context "updating an address and making default at once" do let(:address1) { create(:address) } - let(:address2) { create(:address, name: "Different") } + let(:address2) { create(:address, firstname: "Different", name: "Different") } let(:updated_attrs) do - address2.attributes.tap { |value| value[:name] = "Johnny" } + address2.attributes.tap do |value| + value[:firstname] = "Johnny" + value[:name] = "Johnny" + end end before do @@ -170,6 +177,7 @@ module Spree it "returns the edit as the first address" do user.save_in_address_book(updated_attrs, true) + expect(user.user_addresses.first.address.firstname).to eq "Johnny" expect(user.user_addresses.first.address.name).to eq "Johnny" end end @@ -229,7 +237,7 @@ module Spree context "#remove_from_address_book" do let(:address1) { create(:address) } - let(:address2) { create(:address, name: "Different") } + let(:address2) { create(:address, firstname: "Different", name: "Different") } let(:remove_id) { address1.id } subject { user.remove_from_address_book(remove_id) } diff --git a/core/spec/models/spree/credit_card_spec.rb b/core/spec/models/spree/credit_card_spec.rb index 6227e6d7791..b4e292c65cb 100644 --- a/core/spec/models/spree/credit_card_spec.rb +++ b/core/spec/models/spree/credit_card_spec.rb @@ -93,6 +93,8 @@ def self.payment_states let(:valid_address_attributes) do { name: "Hugo Furst", + firstname: "Hugo", + lastname: "Furst", address1: "123 Main", city: "Somewhere", country_id: country.id, diff --git a/core/spec/models/spree/order_spec.rb b/core/spec/models/spree/order_spec.rb index 275436fc887..8a0d7f5bd72 100644 --- a/core/spec/models/spree/order_spec.rb +++ b/core/spec/models/spree/order_spec.rb @@ -1971,8 +1971,8 @@ def validate(line_item) end describe "#name" do - let(:bill_address) { create(:address, name: "John Doe") } - let(:ship_address) { create(:address, name: "Jane Doe") } + let(:bill_address) { create(:address, firstname: "John", lastname: "Doe", name: "") } + let(:ship_address) { create(:address, firstname: "Jane", lastname: "Doe", name: "") } let(:order) { create(:order, bill_address:, ship_address:) } @@ -2074,12 +2074,15 @@ def validate(line_item) describe "#bill_address_attributes=" do let(:order) { create(:order) } - let(:address_attributes) { { name: "Mickey Mouse" } } + let(:address_attributes) { { firstname: "Mickey", lastname: "Mouse", name: "Mickey Mouse" } } subject { order.bill_address_attributes = address_attributes } it "creates a new bill address" do - expect { subject }.to change { order.bill_address.name }.to("Mickey Mouse") + subject + expect(order.bill_address.firstname).to eq("Mickey") + expect(order.bill_address.lastname).to eq("Mouse") + expect(order.bill_address.name).to eq("Mickey Mouse") end end diff --git a/sample/db/samples/addresses.rb b/sample/db/samples/addresses.rb index 972b2f1e0ef..b8e74304368 100644 --- a/sample/db/samples/addresses.rb +++ b/sample/db/samples/addresses.rb @@ -3,10 +3,12 @@ united_states = Spree::Country.find_by!(iso: "US") new_york = Spree::State.find_by!(name: "New York") -names = ["Sterling Torp", "Jennette Vandervort", "Salome Stroman", "Lyla Lang", - "Lola Zulauf", "Cheree Bruen", "Hettie Torp", "Barbie Gutmann", - "Amelia Renner", "Marceline Bergstrom", "Keeley Sauer", "Mi Gaylord", - "Karon Mills", "Jessika Daugherty", "Emmy Stark"] +first_names = ["Sterling", "Jennette", "Salome", "Lyla", "Lola", "Cheree", + "Hettie", "Barbie", "Amelia", "Marceline", "Keeley", "Mi", + "Karon", "Jessika", "Emmy"] +last_names = ["Torp", "Vandervort", "Stroman", "Lang", "Zulauf", "Bruen", + "Torp", "Gutmann", "Renner", "Bergstrom", "Sauer", "Gaylord", + "Mills", "Daugherty", "Stark"] street_addresses = ["7377 Jacobi Passage", "4725 Serena Ridges", "79832 Hamill Creek", "0746 Genoveva Villages", "86717 D'Amore Hollow", "8529 Delena Well", @@ -26,7 +28,8 @@ 2.times do Spree::Address.create!( - name: names.sample, + firstname: first_names.sample, + lastname: last_names.sample, address1: street_addresses.sample, address2: secondary_addresses.sample, city: cities.sample, From 2ea3d5b13c4d0269027c4b15e71ae140cef5ec5b Mon Sep 17 00:00:00 2001 From: Mayur Shah Date: Fri, 21 Feb 2025 12:26:53 +0530 Subject: [PATCH 2/4] Update API to have firstname and lastname fields Update the address handling logic in the APIs and tests to for 'firstname' and 'lastname' fields, instead of using a single 'name' field. --- api/lib/spree/api_configuration.rb | 2 +- api/spec/requests/spree/api/address_books_spec.rb | 12 ++++++++++-- api/spec/requests/spree/api/checkouts_spec.rb | 4 ++++ api/spec/requests/spree/api/orders_spec.rb | 4 ++-- api/spec/requests/spree/api/users_spec.rb | 8 ++++++++ 5 files changed, 25 insertions(+), 5 deletions(-) diff --git a/api/lib/spree/api_configuration.rb b/api/lib/spree/api_configuration.rb index 9ce561e23c1..cb609313263 100644 --- a/api/lib/spree/api_configuration.rb +++ b/api/lib/spree/api_configuration.rb @@ -61,7 +61,7 @@ class ApiConfiguration < Preferences::Configuration ] preference :address_attributes, :array, default: [ - :id, :name, :address1, :address2, :city, :zipcode, :phone, :company, + :id, :name, :firstname, :lastname, :address1, :address2, :city, :zipcode, :phone, :company, :alternative_phone, :country_id, :country_iso, :state_id, :state_name, :state_text ] diff --git a/api/spec/requests/spree/api/address_books_spec.rb b/api/spec/requests/spree/api/address_books_spec.rb index daf0159b87d..77e7b0f954b 100644 --- a/api/spec/requests/spree/api/address_books_spec.rb +++ b/api/spec/requests/spree/api/address_books_spec.rb @@ -8,6 +8,8 @@ module Spree::Api let!(:harry_address_attributes) do { 'name' => 'Harry Potter', + 'firstname' => 'Harry', + 'lastname' => 'Potter', 'address1' => '4 Privet Drive', 'address2' => 'cupboard under the stairs', 'city' => 'Surrey', @@ -21,6 +23,8 @@ module Spree::Api let!(:ron_address_attributes) do { 'name' => 'Ron Weasly', + 'firstname' => 'Ron', + 'lastname' => 'Weasly', 'address1' => 'Ottery St. Catchpole', 'address2' => '4th floor', 'city' => 'Devon, West Country', @@ -54,6 +58,8 @@ module Spree::Api user = create(:user, spree_api_key: 'galleon') address = user.save_in_address_book(harry_address_attributes, true) harry_address_attributes['name'] = 'Ron Weasly' + harry_address_attributes['firstname'] = 'Ron' + harry_address_attributes['lastname'] = 'Weasly' expect { put "/api/users/#{user.id}/address_book", @@ -73,7 +79,7 @@ module Spree::Api context "when updating a default address" do let(:user) { create(:user, spree_api_key: 'galleon') } - let(:changes) { { name: "Hermione Granger", id: user.ship_address.id} } + let(:changes) { { name: "Hermione Granger", firstname: "Hermione", lastname: "Granger", id: user.ship_address.id} } before do # Create "Harry Potter" default shipping address user.save_in_address_book(harry_address_attributes, true) @@ -168,7 +174,9 @@ module Spree::Api 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('name' => 'Ron Weasly') + harry_address_attributes.merge('firstname' => 'Ron') + harry_address_attributes.merge('name' => 'Ron Weasly') + updated_harry_address = harry_address_attributes.merge('lastname' => 'Weasly') expect { put "/api/users/#{other_user.id}/address_book", diff --git a/api/spec/requests/spree/api/checkouts_spec.rb b/api/spec/requests/spree/api/checkouts_spec.rb index c9eafbcf811..752a1056957 100644 --- a/api/spec/requests/spree/api/checkouts_spec.rb +++ b/api/spec/requests/spree/api/checkouts_spec.rb @@ -72,6 +72,8 @@ module Spree::Api let(:address) do { name: 'John Doe', + firstname: 'John', + lastname: 'Doe', address1: '7735 Old Georgetown Road', city: 'Bethesda', phone: '3014445002', @@ -96,6 +98,8 @@ module Spree::Api expect(json_response['state']).to eq('delivery') expect(json_response['bill_address']['name']).to eq('John Doe') expect(json_response['ship_address']['name']).to eq('John Doe') + expect(json_response['bill_address']['firstname']).to eq('John') + expect(json_response['ship_address']['firstname']).to eq('John') expect(response.status).to eq(200) end diff --git a/api/spec/requests/spree/api/orders_spec.rb b/api/spec/requests/spree/api/orders_spec.rb index 1fa53c3dc9b..1f486206d7e 100644 --- a/api/spec/requests/spree/api/orders_spec.rb +++ b/api/spec/requests/spree/api/orders_spec.rb @@ -531,12 +531,12 @@ module Spree::Api let(:address_params) { { country_id: country.id } } let(:billing_address) { - { name: "Tiago Motta", address1: "Av Paulista", + { name: "Tiago Motta", firstname: "Tiago", lastname: "Motta", address1: "Av Paulista", city: "Sao Paulo", zipcode: "01310-300", phone: "12345678", country_id: country.id, state_id: state.id } } let(:shipping_address) { - { name: "Tiago Motta", address1: "Av Paulista", + { name: "Tiago Motta", firstname: "Tiago", lastname: "Motta", address1: "Av Paulista", city: "Sao Paulo", zipcode: "01310-300", phone: "12345678", country_id: country.id, state_id: state.id } } diff --git a/api/spec/requests/spree/api/users_spec.rb b/api/spec/requests/spree/api/users_spec.rb index 7b0d2d9d7b1..964328841f6 100644 --- a/api/spec/requests/spree/api/users_spec.rb +++ b/api/spec/requests/spree/api/users_spec.rb @@ -51,6 +51,8 @@ module Spree::Api email: "mine@example.com", bill_address_attributes: { name: 'First Last', + firstname: 'First', + lastname: 'Last', address1: '1 Test Rd', city: 'City', country_id: country.id, @@ -60,6 +62,8 @@ module Spree::Api }, ship_address_attributes: { name: 'First Last', + firstname: 'First', + lastname: 'Last', address1: '1 Test Rd', city: 'City', country_id: country.id, @@ -84,6 +88,8 @@ module Spree::Api email: "mine@example.com", bill_address_attributes: { name: 'First Last', + firstname: 'First', + lastname: 'Last', address1: '1 Test Rd', city: 'City', country_id: country.id, @@ -93,6 +99,8 @@ module Spree::Api }, ship_address_attributes: { name: 'First Last', + firstname: 'First', + lastname: 'Last', address1: '1 Test Rd', city: 'City', country_id: country.id, From b2bd8487a4956f7ffcd29951101c8d2e0e7b8da2 Mon Sep 17 00:00:00 2001 From: Mayur Shah Date: Fri, 21 Feb 2025 12:32:58 +0530 Subject: [PATCH 3/4] Update backend to have firstname and lastname fields - Update the backend logic to separate 'name', 'firstname' and 'lastname' for better data handling. - Adjusted user and address forms, search functionality, and related views to use the new fields. --- .../orders/customer_details/autocomplete.hbs | 1 + .../javascripts/spree/backend/user_picker.js | 4 ++- .../spree/backend/views/order/address.js | 2 +- .../backend/views/order/customer_select.js | 4 ++- .../spree/backend/sections/_orders.scss | 2 +- .../spree/admin/search_controller.rb | 4 ++- .../spree/admin/search/users.json.jbuilder | 2 ++ .../spree/admin/shared/_address.html.erb | 2 +- .../spree/admin/shared/_address_form.html.erb | 10 +++++++ backend/lib/spree/backend_configuration.rb | 2 +- .../spree/admin/payments_controller_spec.rb | 1 + .../spree/admin/search_controller_spec.rb | 26 ++++++++++++++++++- .../spree/admin/users_controller_spec.rb | 2 ++ .../admin/orders/customer_details_spec.rb | 11 ++++++-- .../features/admin/orders/new_order_spec.rb | 2 ++ 15 files changed, 65 insertions(+), 10 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 db6b4eff4a1..a46906a287e 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 @@ -4,6 +4,7 @@ {{#if bill_address.name }} {{t 'bill_address' }} {{bill_address.name}}
+ {{bill_address.firstname}} {{bill_address.lastname}}
{{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 6cdc7dc9e1c..d539e6b6633 100644 --- a/backend/app/assets/javascripts/spree/backend/user_picker.js +++ b/backend/app/assets/javascripts/spree/backend/user_picker.js @@ -28,7 +28,9 @@ $.fn.userAutocomplete = function () { q: { m: 'or', email_start: term, - name_start: term + name_start: term, + firstname_start: term, + 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 e7cd6376e70..58744b4790f 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 = ["name", "company", "address1", "address2", + var fields = ["name", "firstname", "lastname", "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 dc31f58a58f..265614d3a2c 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,7 +34,9 @@ Spree.Views.Order.CustomerSelect = Backbone.View.extend({ q: { m: 'or', email_start: term, - name_start: term + name_start: term, + firstname_start: term, + 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 82b2a5d0a90..0c9fd8082a0 100644 --- a/backend/app/assets/stylesheets/spree/backend/sections/_orders.scss +++ b/backend/app/assets/stylesheets/spree/backend/sections/_orders.scss @@ -65,7 +65,7 @@ } label[for="order_bill_address_attributes_name"] { - margin-top: 53.5px; + margin-top: 45px; } #risk_analysis legend { diff --git a/backend/app/controllers/spree/admin/search_controller.rb b/backend/app/controllers/spree/admin/search_controller.rb index 60fe94049e7..3c9b85aa8d7 100644 --- a/backend/app/controllers/spree/admin/search_controller.rb +++ b/backend/app/controllers/spree/admin/search_controller.rb @@ -16,7 +16,9 @@ def users @users = Spree.user_class.ransack({ m: 'or', email_start: params[:q], - name_start: params[:q] + name_start: params[:q], + addresses_firstname_start: params[:q], + addresses_lastname_start: params[:q] }).result.limit(10) end end diff --git a/backend/app/views/spree/admin/search/users.json.jbuilder b/backend/app/views/spree/admin/search/users.json.jbuilder index 9b430af9f1c..21df1556b7c 100644 --- a/backend/app/views/spree/admin/search/users.json.jbuilder +++ b/backend/app/views/spree/admin/search/users.json.jbuilder @@ -6,6 +6,8 @@ json.array!(@users) do |user| address_fields = [ :name, + :firstname, + :lastname, :address1, :address2, :city, diff --git a/backend/app/views/spree/admin/shared/_address.html.erb b/backend/app/views/spree/admin/shared/_address.html.erb index d803a9d61d0..2d5ee61b866 100644 --- a/backend/app/views/spree/admin/shared/_address.html.erb +++ b/backend/app/views/spree/admin/shared/_address.html.erb @@ -1,5 +1,5 @@
- <%= address.name %> <%= address.company unless address.company.blank? %>
+ <%= address.name %> <%= "#{address.firstname} #{address.lastname} " %> <%= address.company unless address.company.blank? %>
<%= address.phone %><%= address.alternative_phone unless address.alternative_phone.blank? %>
<%= address.address1 %>
<% if address.address2.present? %><%= address.address2 %>
<% end %> 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 e023cc8c7fe..cedd852a8ec 100644 --- a/backend/app/views/spree/admin/shared/_address_form.html.erb +++ b/backend/app/views/spree/admin/shared/_address_form.html.erb @@ -6,6 +6,16 @@ <%= f.text_field :name, class: 'fullwidth' %>
+
"> + <%= f.label :firstname %> + <%= f.text_field :firstname, class: 'fullwidth' %> +
+ +
"> + <%= f.label :lastname %> + <%= f.text_field :lastname, class: 'fullwidth' %> +
+ <% if Spree::Config[:company] %>
"> <%= f.label :company %> diff --git a/backend/lib/spree/backend_configuration.rb b/backend/lib/spree/backend_configuration.rb index 5d0c130e30e..6c7bf99a944 100644 --- a/backend/lib/spree/backend_configuration.rb +++ b/backend/lib/spree/backend_configuration.rb @@ -72,7 +72,7 @@ class BackendConfiguration < Preferences::Configuration { partial: "spree/admin/shared/search_fields/text_field", locals: { - ransack: :bill_address_name_cont, + ransack: :bill_address_firstname_or_bill_address_lastname_cont, label: -> { I18n.t(:name_contains, scope: :spree) } } }, diff --git a/backend/spec/controllers/spree/admin/payments_controller_spec.rb b/backend/spec/controllers/spree/admin/payments_controller_spec.rb index da351bcbeb3..c452c458c0d 100644 --- a/backend/spec/controllers/spree/admin/payments_controller_spec.rb +++ b/backend/spec/controllers/spree/admin/payments_controller_spec.rb @@ -56,6 +56,7 @@ module Admin let(:address_attributes) do { 'name' => address.name, + 'firstname' => address.firstname, 'address1' => address.address1, 'city' => address.city, 'country_id' => address.country_id, diff --git a/backend/spec/controllers/spree/admin/search_controller_spec.rb b/backend/spec/controllers/spree/admin/search_controller_spec.rb index 3f2b0754353..67df511b3b8 100644 --- a/backend/spec/controllers/spree/admin/search_controller_spec.rb +++ b/backend/spec/controllers/spree/admin/search_controller_spec.rb @@ -37,7 +37,7 @@ def starting_letters(string) end end - context 'when searching by ship addresss name' do + context 'when searching by ship address name' do it_should_behave_like 'user found by search' do let(:search_query) { starting_letters(user.ship_address.name) } end @@ -48,6 +48,30 @@ def starting_letters(string) let(:search_query) { starting_letters(user.bill_address.name) } end end + + context 'when searching by ship address first name' do + it_should_behave_like 'user found by search' do + let(:search_query) { starting_letters(user.ship_address.firstname) } + end + end + + context 'when searching by bill address first name' do + it_should_behave_like 'user found by search' do + let(:search_query) { starting_letters(user.bill_address.firstname) } + end + end + + context 'when searching by ship address last name' do + it_should_behave_like 'user found by search' do + let(:search_query) { starting_letters(user.ship_address.lastname) } + end + end + + context 'when searching by bill address last name' do + it_should_behave_like 'user found by search' do + let(:search_query) { starting_letters(user.bill_address.lastname) } + end + end end context 'when searching by user ids' do diff --git a/backend/spec/controllers/spree/admin/users_controller_spec.rb b/backend/spec/controllers/spree/admin/users_controller_spec.rb index c8ad0d41f61..e12555d733a 100644 --- a/backend/spec/controllers/spree/admin/users_controller_spec.rb +++ b/backend/spec/controllers/spree/admin/users_controller_spec.rb @@ -9,6 +9,8 @@ let(:valid_address_attributes) do { name: 'Foo Bar', + firstname: 'Foo', + lastname: 'Bar', city: "New York", country_id: state.country.id, state_id: state.id, diff --git a/backend/spec/features/admin/orders/customer_details_spec.rb b/backend/spec/features/admin/orders/customer_details_spec.rb index e3b91f0b2a8..ffde429fb59 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:, state:, name: "Jane Doe") } - let!(:bill_address) { create(:address, country:, state:, name: "Jane Doe") } + let!(:ship_address) { create(:address, country:, state:, firstname: "Jane", lastname: "Doe", name: "Jane Doe") } + let!(:bill_address) { create(:address, country:, state:, firstname: "Jane", lastname: "Doe", name: "Jane Doe") } let!(:user) { create(:user, email: 'foobar@example.com', ship_address:, bill_address:) } @@ -38,6 +38,8 @@ it "associates a user when not using guest checkout" do # 5317 - Address prefills using user's default. expect(page).to have_field('Name', with: user.bill_address.name) + 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('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,6 +122,7 @@ click_link "Customer" click_button "Update" expect(page).to have_content("Shipping address name can't be blank") + expect(page).to have_content("Shipping address first name can't be blank") end context "for an order in confirm state with a user" do @@ -171,6 +174,8 @@ click_link "Customer" # Need to fill in valid information so it passes validations fill_in "order_ship_address_attributes_name", with: "John 99 Doe" + 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_company", with: "Company" fill_in "order_ship_address_attributes_address1", with: "100 first lane" fill_in "order_ship_address_attributes_address2", with: "#101" @@ -189,6 +194,8 @@ def fill_in_address fill_in "Name", with: "John 99 Doe" + fill_in "First Name", with: "John 99" + fill_in "Last Name", with: "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 b844856f0bf..885fe620f55 100644 --- a/backend/spec/features/admin/orders/new_order_spec.rb +++ b/backend/spec/features/admin/orders/new_order_spec.rb @@ -368,6 +368,8 @@ def fill_in_address fill_in "Name", with: "John 99 Doe" + fill_in "First Name", with: "John 99" + fill_in "Last Name", with: "Doe" fill_in "Street Address", with: "100 first lane" fill_in "Street Address (cont'd)", with: "#101" fill_in "City", with: "Bethesda" From 53e17332e3ee1000b7076ee6fba979b5c96c8f7d Mon Sep 17 00:00:00 2001 From: Mayur Shah Date: Fri, 21 Feb 2025 12:22:13 +0530 Subject: [PATCH 4/4] Update Admin to have firstname and lastname fields Update the address handling logic in the admin components and tests for 'firstname' and 'lastname' fields, instead of using a single 'name' field. --- .../solidus_admin/ui/forms/address/component.html.erb | 5 +++++ admin/spec/features/orders/show_spec.rb | 4 ++++ admin/spec/features/users_spec.rb | 8 +++++++- admin/spec/requests/solidus_admin/users_spec.rb | 2 ++ 4 files changed, 18 insertions(+), 1 deletion(-) diff --git a/admin/app/components/solidus_admin/ui/forms/address/component.html.erb b/admin/app/components/solidus_admin/ui/forms/address/component.html.erb index 0dd61697425..fb556ac924a 100644 --- a/admin/app/components/solidus_admin/ui/forms/address/component.html.erb +++ b/admin/app/components/solidus_admin/ui/forms/address/component.html.erb @@ -4,6 +4,11 @@ >
<%= render component("ui/forms/field").text_field(@name, :name, object: @address) %> + <%= render component("ui/forms/field").text_field(@name, :firstname, object: @address) %> + <%= render component("ui/forms/field").text_field(@name, :lastname, object: @address) %> + <% if Spree::Config[:company] %> + <%= render component("ui/forms/field").text_field(@name, :company, object: @address) %> + <% end %> <%= render component("ui/forms/field").text_field(@name, :address1, object: @address) %> <%= render component("ui/forms/field").text_field(@name, :address2, object: @address) %>
diff --git a/admin/spec/features/orders/show_spec.rb b/admin/spec/features/orders/show_spec.rb index 986acbac250..3fe350ed7bf 100644 --- a/admin/spec/features/orders/show_spec.rb +++ b/admin/spec/features/orders/show_spec.rb @@ -52,6 +52,8 @@ within("dialog") do fill_in "Name", with: "John Doe" + fill_in "First Name", with: "John" + fill_in "Last Name", with: "Doe" fill_in "Street Address", with: "1 John Doe Street" fill_in "Street Address (cont'd)", with: "Apartment 2" fill_in "City", with: "John Doe City" @@ -78,6 +80,8 @@ within("dialog") do fill_in "Name", with: "Jane Doe" + fill_in "First Name", with: "Jane" + fill_in "Last Name", with: "Doe" fill_in "Street Address", with: "1 Jane Doe Street" fill_in "Street Address (cont'd)", with: "Apartment 3" fill_in "City", with: "Jane Doe City" diff --git a/admin/spec/features/users_spec.rb b/admin/spec/features/users_spec.rb index b6ac4e097ab..c0588955465 100644 --- a/admin/spec/features/users_spec.rb +++ b/admin/spec/features/users_spec.rb @@ -129,14 +129,16 @@ # Invalid submission within("form.ship_address") do fill_in "Name", with: "" + fill_in "First Name", with: "" fill_in "Street Address", with: "" click_on "Update" end - expect(page).to have_content("can't be blank").twice + expect(page).to have_content("can't be blank").thrice # Valid submission within("form.bill_address") do fill_in "Name", with: "Galadriel" + fill_in "First Name", with: "Galadriel" click_on "Update" end expect(page).to have_content("Billing Address has been successfully updated.") @@ -144,6 +146,7 @@ # Valid submission within("form.ship_address") do fill_in "Name", with: "Elrond" + fill_in "First Name", with: "Elrond" click_on "Update" end expect(page).to have_content("Shipping Address has been successfully updated.") @@ -151,6 +154,7 @@ # Cancel submission within("form.bill_address") do fill_in "Name", with: "Smeagol" + fill_in "First Name", with: "Smeagol" click_on "Cancel" end expect(page).to have_content("Users / customer@example.com / Addresses") @@ -159,6 +163,8 @@ # The address forms weirdly only have values rather than actual text on the page. expect(page).to have_field("user[bill_address_attributes][name]", with: "Galadriel") expect(page).to have_field("user[ship_address_attributes][name]", with: "Elrond") + expect(page).to have_field("user[bill_address_attributes][firstname]", with: "Galadriel") + expect(page).to have_field("user[ship_address_attributes][firstname]", with: "Elrond") end end diff --git a/admin/spec/requests/solidus_admin/users_spec.rb b/admin/spec/requests/solidus_admin/users_spec.rb index c2ca7c88e2b..c7dd4feb3ad 100644 --- a/admin/spec/requests/solidus_admin/users_spec.rb +++ b/admin/spec/requests/solidus_admin/users_spec.rb @@ -12,6 +12,8 @@ user: { bill_address_attributes: { name: address.name, + firstname: address.firstname, + lastname: address.lastname, address1: address.address1, address2: address.address2, city: address.city,