Add name to Spree::Address

api/spec/requests/spree/api/orders_controller_spec.rb

@@ -537,13 +537,13 @@ module Spree
       end
 
       it "receives error message if trying to add billing address with errors" do
-        billing_address[:firstname] = ""
+        billing_address[:city] = ""
 
         put spree.api_order_path(order), params: { order: { bill_address_attributes: billing_address } }
 
         expect(json_response['error']).not_to be_nil
         expect(json_response['errors']).not_to be_nil
-        expect(json_response['errors']['bill_address.firstname'].first).to eq "can't be blank"
+        expect(json_response['errors']['bill_address.city'].first).to eq "can't be blank"
       end
 
       it "can add shipping address" do
@@ -557,13 +557,13 @@ module Spree
       it "receives error message if trying to add shipping address with errors" do
         order.update!(ship_address_id: nil)
 
-        shipping_address[:firstname] = ""
+        shipping_address[:city] = ""
 
         put spree.api_order_path(order), params: { order: { ship_address_attributes: shipping_address } }
 
         expect(json_response['error']).not_to be_nil
         expect(json_response['errors']).not_to be_nil
-        expect(json_response['errors']['ship_address.firstname'].first).to eq "can't be blank"
+        expect(json_response['errors']['ship_address.city'].first).to eq "can't be blank"
       end
 
       it "cannot set the user_id for the order" do

backend/app/views/spree/admin/shared/_address.html.erb

@@ -1,8 +1,8 @@
 <div data-hook="address">
-  <%="#{address.firstname} #{address.lastname} "%><%= address.company unless address.company.blank? %><br />
-  <%="#{address.phone}" %><%= address.alternative_phone unless address.alternative_phone.blank? %><br />
-  <%= address.address1 %><br />
-  <% if address.address2.present? %><%= "#{address.address2}" %><br /><% end %>
-  <%= "#{address.city}, #{address.state_text}, #{address.zipcode}" %><br />
-  <%= "#{address.country.name}" %>
+  <%= address.name %> <%= address.company unless address.company.blank? %><br>
+  <%= address.phone %><%= address.alternative_phone unless address.alternative_phone.blank? %><br>
+  <%= address.address1 %><br>
+  <% if address.address2.present? %><%= address.address2 %><br><% end %>
+  <%= "#{address.city}, #{address.state_text}, #{address.zipcode}" %><br>
+  <%= address.country.name %>
 </div>

backend/spec/controllers/spree/admin/search_controller_spec.rb

@@ -33,6 +33,12 @@
         end
       end
 
+      context 'when searching by ship addresss name' do
+        it_should_behave_like 'user found by search' do
+          let(:user_attribute) { user.ship_address.name }
+        end
+      end
+
       context 'when searching by ship addresss first name' do
         it_should_behave_like 'user found by search' do
           let(:user_attribute) { user.ship_address.firstname }

core/app/models/spree/address.rb

@@ -20,6 +20,7 @@ class Address < Spree::Base
 
     alias_attribute :first_name, :firstname
     alias_attribute :last_name, :lastname
+    alias_attribute :full_name, :name
 
     DB_ONLY_ATTRS = %w(id updated_at created_at)
     TAXATION_ATTRS = %w(state_id country_id zipcode)
@@ -62,10 +63,13 @@ def self.immutable_merge(existing_address, new_attributes)
     def self.value_attributes(base_attributes, merge_attributes = {})
       base = base_attributes.stringify_keys.merge(merge_attributes.stringify_keys)
 
-      # TODO: Deprecate these aliased attributes
-      base['firstname'] = base['first_name'] if base.key?('first_name')
-      base['lastname'] = base['last_name'] if base.key?('last_name')
-
+      name_from_attributes = Spree::Address::Name.from_attributes(base)
+      if base['firstname'].presence || base['first_name'].presence
+        base['firstname'] = name_from_attributes.first_name
+      end
+      if base['lastname'].presence || base['last_name'].presence
+        base['lastname'] = name_from_attributes.last_name
+      end
       excluded_attributes = DB_ONLY_ATTRS + %w(first_name last_name)
 
       base.except(*excluded_attributes)
@@ -80,18 +84,13 @@ def taxation_attributes
       self.class.value_attributes(attributes.slice(*TAXATION_ATTRS))
     end
 
-    # @return [String] the full name on this address
-    def full_name
-      "#{firstname} #{lastname}".strip
-    end
-
     # @return [String] a string representation of this state
     def state_text
       state.try(:abbr) || state.try(:name) || state_name
     end
 
     def to_s
-      "#{full_name}: #{address1}"
+      "#{name}: #{address1}"
     end
 
     # @note This compares the addresses based on only the fields that make up
@@ -130,7 +129,7 @@ def blank?
     # @return [Hash] an ActiveMerchant compatible address hash
     def active_merchant_hash
       {
-        name: full_name,
+        name: name,
         address1: address1,
         address2: address2,
         city: city,
@@ -172,6 +171,19 @@ def country_iso
       country && country.iso
     end
 
+    # @return [String] the full name on this address
+    def name
+      Spree::Address::Name.new(firstname, lastname).value
+    end
+
+    def name=(value)
+      return if value.nil?
+
+      name_from_value = Spree::Address::Name.new(value)
+      write_attribute(:firstname, name_from_value.first_name)
+      write_attribute(:lastname, name_from_value.last_name)
+    end
+
     private
 
     def state_validate

core/app/models/spree/address/name.rb

@@ -0,0 +1,49 @@
+# frozen_string_literal: true
+
+module Spree
+  class Address
+    # Provides a value object to help transitioning from legacy
+    # firstname and lastname fields to a unified name field.
+    class Name
+      attr_reader :first_name, :last_name, :value
+
+      # Creates an instance of Spree::Address::Name parsing input attributes.
+      # @param attributes [Hash] an hash possibly containing name-related
+      #   attributes (name, firstname, lastname, first_name, last_name)
+      # @return [Spree::Address::Name] the object created
+      def self.from_attributes(attributes)
+        params = attributes.with_indifferent_access
+
+        if params[:name].present?
+          Spree::Address::Name.new(params[:name])
+        elsif params[:firstname].present?
+          Spree::Address::Name.new(params[:firstname], params[:lastname])
+        elsif params[:first_name].present?
+          Spree::Address::Name.new(params[:first_name], params[:last_name])
+        else
+          Spree::Address::Name.new
+        end
+      end
+
+      def initialize(*components)
+        @value = components.join(' ').strip
+        initialize_name_components(components)
+      end
+
+      def to_s
+        @value
+      end
+
+      private
+
+      def initialize_name_components(components)
+        if components.size == 2
+          @first_name = components[0].to_s
+          @last_name = components[1].to_s
+        else
+          @first_name, @last_name = @value.split(/[[:space:]]/, 2)
+        end
+      end
+    end
+  end
+end

core/app/models/spree/credit_card.rb

@@ -163,7 +163,7 @@ def has_payment_profile?
     #   the object to ActiveMerchant.
     # @return [String] the first name on this credit card
     def first_name
-      name.to_s.split(/[[:space:]]/, 2)[0]
+      Spree::Address::Name.new(name).first_name
     end
 
     # @note ActiveMerchant needs first_name/last_name because we pass it a
@@ -172,7 +172,7 @@ def first_name
     #   the object to ActiveMerchant.
     # @return [String] the last name on this credit card
     def last_name
-      name.to_s.split(/[[:space:]]/, 2)[1]
+      Spree::Address::Name.new(name).last_name
     end
 
     # @return [ActiveMerchant::Billing::CreditCard] an ActiveMerchant credit

core/app/models/spree/order.rb

@@ -136,9 +136,10 @@ def self.find_by_param!(value)
       find_by! number: value
     end
 
-    delegate :firstname, :lastname, to: :bill_address, prefix: true, allow_nil: true
+    delegate :name, :firstname, :lastname, to: :bill_address, prefix: true, allow_nil: true
     alias_method :billing_firstname, :bill_address_firstname
     alias_method :billing_lastname, :bill_address_lastname
+    alias_method :billing_name, :bill_address_name
 
     class_attribute :update_hooks
     self.update_hooks = Set.new
@@ -398,7 +399,7 @@ def refund_total
 
     def name
       if (address = bill_address || ship_address)
-        "#{address.firstname} #{address.lastname}"
+        address.name
       end
     end
 

core/spec/lib/spree/core/importer/order_spec.rb

@@ -36,8 +36,7 @@ module Core
       let(:ship_address) {
         {
          address1: '123 Testable Way',
-         firstname: 'Fox',
-         lastname: 'Mulder',
+         name: 'Fox Mulder',
          city: 'Washington',
          country_id: country.id,
          state_id: state.id,

core/spec/models/spree/address/name_spec.rb

@@ -0,0 +1,70 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe Spree::Address::Name do
+  it 'concatenates components to form a full name' do
+    name = described_class.new('Jane', 'Von', 'Doe')
+
+    expect(name.to_s).to eq('Jane Von Doe')
+  end
+
+  it 'keeps first name and last name' do
+    name = described_class.new('Jane', 'Doe')
+
+    expect(name.first_name).to eq('Jane')
+    expect(name.last_name).to eq('Doe')
+  end
+
+  it 'splits full name to emulate first name and last name' do
+    name = described_class.new('Jane Von Doe')
+
+    expect(name.first_name).to eq('Jane')
+    expect(name.last_name).to eq('Von Doe')
+  end
+
+  context 'from attributes' do
+    it 'returns name with nil first name if no relevant attribute found' do
+      name = described_class.from_attributes({})
+
+      expect(name.first_name).to be_nil
+      expect(name.last_name).to be_nil
+    end
+
+    it 'prioritizes name over firstname' do
+      attributes = {
+        name: 'Jane Doe',
+        firstname: 'Joe',
+        lastname: 'Bloggs'
+      }
+      name = described_class.from_attributes(attributes)
+
+      expect(name.first_name).to eq('Jane')
+      expect(name.last_name).to eq('Doe')
+    end
+
+    it 'prioritizes firstname over first_name' do
+      attributes = {
+        firstname: 'Jane',
+        lastname: 'Doe',
+        first_name: 'Joe',
+        last_name: 'Bloggs'
+      }
+      name = described_class.from_attributes(attributes)
+
+      expect(name.first_name).to eq('Jane')
+      expect(name.last_name).to eq('Doe')
+    end
+
+    it 'eventually uses first_name' do
+      attributes = {
+        first_name: 'Jane',
+        last_name: 'Doe'
+      }
+      name = described_class.from_attributes(attributes)
+
+      expect(name.first_name).to eq('Jane')
+      expect(name.last_name).to eq('Doe')
+    end
+  end
+end

core/spec/models/spree/address_spec.rb

@@ -350,25 +350,29 @@
     end
   end
 
-  context '#full_name' do
-    context 'both first and last names are present' do
-      let(:address) { Spree::Address.new firstname: 'Michael', lastname: 'Jackson' }
-      specify { expect(address.full_name).to eq('Michael Jackson') }
+  context '#name' do
+    it 'concatenates firstname and lastname' do
+      address = Spree::Address.new(firstname: 'Michael J.', lastname: 'Jackson')
+
+      expect(address.name).to eq('Michael J. Jackson')
     end
 
-    context 'first name is blank' do
-      let(:address) { Spree::Address.new firstname: nil, lastname: 'Jackson' }
-      specify { expect(address.full_name).to eq('Jackson') }
+    it 'returns lastname when firstname is blank' do
+      address = Spree::Address.new(firstname: nil, lastname: 'Jackson')
+
+      expect(address.name).to eq('Jackson')
     end
 
-    context 'last name is blank' do
-      let(:address) { Spree::Address.new firstname: 'Michael', lastname: nil }
-      specify { expect(address.full_name).to eq('Michael') }
+    it 'returns firstanme when lastname is blank' do
+      address = Spree::Address.new(firstname: 'Michael J.', lastname: nil)
+
+      expect(address.name).to eq('Michael J.')
     end
 
-    context 'both first and last names are blank' do
-      let(:address) { Spree::Address.new firstname: nil, lastname: nil }
-      specify { expect(address.full_name).to eq('') }
+    it 'returns empty string when firstname and lastname are blank' do
+      address = Spree::Address.new(firstname: nil, lastname: nil)
+
+      expect(address.name).to eq('')
     end
   end
 

core/spec/models/spree/concerns/user_address_book_spec.rb

@@ -102,9 +102,9 @@ module Spree
 
       context "updating an address and making default at once" do
         let(:address1) { create(:address) }
-        let(:address2) { create(:address, firstname: "Different") }
+        let(:address2) { create(:address, name: "Different") }
         let(:updated_attrs) do
-          address2.attributes.tap { |value| value[:firstname] = "Johnny" }
+          address2.attributes.tap { |value| value[:name] = "Johnny" }
         end
 
         before do
@@ -114,7 +114,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
 
@@ -173,7 +173,7 @@ module Spree
         end
 
         context "via an edit to another address" do
-          let(:address2) { create(:address, firstname: "Different") }
+          let(:address2) { create(:address, name: "Different") }
           let(:edited_attributes) do
             # conceptually edit address2 to match the values of address
             edited_attributes = address.attributes
@@ -219,7 +219,7 @@ module Spree
 
     context "#remove_from_address_book" do
       let(:address1) { create(:address) }
-      let(:address2) { create(:address, firstname: "Different") }
+      let(:address2) { create(:address, name: "Different") }
       let(:remove_id) { address1.id }
       subject { user.remove_from_address_book(remove_id) }
 

core/spec/models/spree/credit_card_spec.rb

@@ -90,8 +90,7 @@ def self.payment_states
     let!(:persisted_card) { Spree::CreditCard.find(credit_card.id) }
     let(:valid_address_attributes) do
       {
-        firstname: "Hugo",
-        lastname: "Furst",
+        name: "Hugo Furst",
         address1: "123 Main",
         city: "Somewhere",
         country_id: 1,