Restore firstname lastname on address

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

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,13 +129,17 @@ 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
         zipcode: Billing address zipcode
       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

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]

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' }

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,

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
 

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) }

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,

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
 

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,