Deprecate Spree::Address firstname, lastname and full_name

api/openapi/api.oas2.yml

@@ -5078,12 +5078,15 @@ definitions:
         type: string
       firstname:
         type: string
+        description: '*Deprecated: will be removed in Solidus 3.0, please use `name` property*'
       full_name:
         type: string
+        description: '*Deprecated: will be removed in Solidus 3.0, please use `name` property*'
       id:
         type: integer
       lastname:
         type: string
+        description: '*Deprecated: will be removed in Solidus 3.0, please use `name` property*'
       name:
         type: string
       phone:
@@ -5620,8 +5623,10 @@ definitions:
         type: string
       firstname:
         type: string
+        description: '*Deprecated: will be removed in Solidus 3.0, please use `name` property*'
       lastname:
         type: string
+        description: '*Deprecated: will be removed in Solidus 3.0, please use `name` property*'
       address1:
         type: string
       address2:

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

@@ -55,7 +55,7 @@ module Admin
 
             let(:address_attributes) do
               {
-                'firstname' => address.firstname,
+                'name' => address.name,
                 'address1' => address.address1,
                 'city' => address.city,
                 'country_id' => address.country_id,
@@ -68,7 +68,7 @@ module Admin
             it 'associates the address' do
               expect(order.payments.count).to eq(1)
               credit_card = order.payments.last.source
-              expect(credit_card.address.attributes).to include(address_attributes)
+              expect(credit_card.address.as_json).to include(address_attributes)
             end
           end
         end

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

@@ -25,23 +25,27 @@
     end
 
     context 'when searching by user attributes' do
-      let(:params) { { q: user_attribute } }
+      let(:params) { { q: search_query } }
+
+      def starting_letters(string)
+        string.first(3)
+      end
 
       context 'when searching by email' do
         it_should_behave_like 'user found by search' do
-          let(:user_attribute) { user.email }
+          let(:search_query) { starting_letters(user.email) }
         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 }
+          let(:search_query) { starting_letters(user.ship_address.name) }
         end
       end
 
       context 'when searching by bill address name' do
         it_should_behave_like 'user found by search' do
-          let(:user_attribute) { user.bill_address.name }
+          let(:search_query) { starting_letters(user.bill_address.name) }
         end
       end
     end

core/app/models/spree/address.rb

@@ -33,6 +33,11 @@ class Address < Spree::Base
       where(value_attributes(attributes))
     end
 
+    Spree::Deprecation.deprecate_methods(
+      Spree::Address,
+      LEGACY_NAME_ATTRS.product([:name]).to_h
+    )
+
     # @return [Address] an address with default attributes
     def self.build_default(*args, &block)
       where(country: Spree::Country.default).build(*args, &block)
@@ -175,7 +180,10 @@ def country_iso
 
     # @return [String] the full name on this address
     def name
-      Spree::Address::Name.new(firstname, lastname).value
+      Spree::Address::Name.new(
+        read_attribute(:firstname),
+        read_attribute(:lastname)
+      ).value
     end
 
     def name=(value)
@@ -186,10 +194,20 @@ def name=(value)
       write_attribute(:lastname, name_from_value.last_name)
     end
 
+    def as_json(options = {})
+      if Spree::Config.use_combined_first_and_last_name_in_address
+        super(options.merge(except: LEGACY_NAME_ATTRS)).tap do |hash|
+          hash['name'] = name
+        end
+      else
+        super
+      end
+    end
+
     private
 
     def validate_name
-      return if firstname.present?
+      return if name.present?
 
       name_attribute = if Spree::Config.use_combined_first_and_last_name_in_address
         :name

core/lib/spree/testing_support/factories/address_factory.rb

@@ -11,8 +11,7 @@
       state_code { 'AL' }
     end
 
-    firstname { 'John' }
-    lastname { nil }
+    name { 'John Von Doe' }
     company { 'Company' }
     address1 { '10 Lovely Street' }
     address2 { 'Northwest' }

core/spec/models/spree/address_spec.rb

@@ -6,6 +6,11 @@
   subject { Spree::Address }
 
   context "aliased attributes" do
+    before do
+      allow(Spree::Deprecation).to receive(:warn).and_call_original
+      allow(Spree::Deprecation).to receive(:warn).with(/firstname|lastname/, any_args)
+    end
+
     let(:address) { Spree::Address.new firstname: 'Ryan', lastname: 'Bigg' }
 
     it " first_name" do
@@ -154,19 +159,19 @@
         end
 
         it 'accepts other attributes' do
-          address = Spree::Address.build_default(first_name: 'Ryan')
+          address = Spree::Address.build_default(name: 'Ryan')
 
           expect(address.country).to eq default_country
-          expect(address.first_name).to eq 'Ryan'
+          expect(address.name).to eq 'Ryan'
         end
 
         it 'accepts a block' do
           address = Spree::Address.build_default do |record|
-            record.first_name = 'Ryan'
+            record.name = 'Ryan'
           end
 
           expect(address.country).to eq default_country
-          expect(address.first_name).to eq 'Ryan'
+          expect(address.name).to eq 'Ryan'
         end
 
         it 'can override the country' do
@@ -208,7 +213,7 @@
       end
     end
 
-    let(:new_address_attributes) { attributes_for(:address) }
+    let(:new_address_attributes) { build(:address).attributes }
     subject { Spree::Address.immutable_merge(existing_address, new_address_attributes) }
 
     context "no existing address supplied" do
@@ -222,7 +227,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, firstname: 'Jordan') }
+        let!(:matching_address) { create(:address, name: 'Jordan') }
 
         it "returns the matching address" do
           expect(subject.attributes).to be_address_equivalent_attributes(new_address_attributes)
@@ -251,7 +256,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, firstname: 'Jordan') }
+        let!(:matching_address) { create(:address, name: 'Jordan') }
 
         it 'returns the matching address' do
           expect(subject.attributes).to be_address_equivalent_attributes(new_address_attributes)
@@ -314,10 +319,10 @@
 
   describe '.taxation_attributes' do
     context 'both taxation and non-taxation attributes are present ' do
-      let(:address) { Spree::Address.new firstname: 'Michael', lastname: 'Jackson', state_id: 1, country_id: 2, zipcode: '12345' }
+      let(:address) { Spree::Address.new name: 'Michael Jackson', state_id: 1, country_id: 2, zipcode: '12345' }
 
       it 'removes the non-taxation attributes' do
-        expect(address.taxation_attributes).not_to eq('firstname' => 'Michael', 'lastname' => 'Jackson')
+        expect(address.taxation_attributes).not_to eq('name' => 'Michael Jackson')
       end
 
       it 'returns only the taxation attributes' do
@@ -326,7 +331,7 @@
     end
 
     context 'taxation attributes are blank' do
-      let(:address) { Spree::Address.new firstname: 'Michael', lastname: 'Jackson' }
+      let(:address) { Spree::Address.new name: 'Michael 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)
@@ -374,6 +379,12 @@
 
       expect(address.name).to eq('')
     end
+
+    it 'is included in json representation' do
+      address = Spree::Address.new(name: 'Jane Von Doe')
+
+      expect(address.as_json).to include('name' => 'Jane Von Doe')
+    end
   end
 
   context '#state_text' do
@@ -400,4 +411,30 @@
 
     it { is_expected.to be_require_phone }
   end
+
+  context 'deprecations' do
+    let(:address) { described_class.new }
+
+    specify 'json representation does not contain deprecated fields' do
+      expect(address.as_json).not_to include('firstname', 'lastname')
+    end
+
+    specify 'firstname is deprecated' do
+      expect(Spree::Deprecation).to receive(:warn).with(/firstname/, any_args)
+
+      address.firstname
+    end
+
+    specify 'lastname is deprecated' do
+      expect(Spree::Deprecation).to receive(:warn).with(/lastname/, any_args)
+
+      address.lastname
+    end
+
+    specify 'full_name is deprecated' do
+      expect(Spree::Deprecation).to receive(:warn).with(/full_name/, any_args)
+
+      address.full_name
+    end
+  end
 end

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

@@ -77,16 +77,16 @@ module Spree
           end
 
           context "and changing another address field at the same time" do
-            let(:updated_address_attributes) { address.attributes.tap { |value| value[:first_name] = "Newbie" } }
+            let(:updated_address_attributes) { address.attributes.tap { |value| value[:city] = "Dallas" } }
 
             subject { user.save_in_address_book(updated_address_attributes, true) }
 
-            it "changes first name" do
-              expect(subject.first_name).to eq updated_address_attributes[:first_name]
+            it "changes city" do
+              expect(subject.city).to eq updated_address_attributes[:city]
             end
 
-            it "preserves last name" do
-              expect(subject.last_name).to eq address.last_name
+            it "preserves name" do
+              expect(subject.name).to eq address.name
             end
 
             it "is a new immutable address instance" do

guides/source/developers/users/addresses.html.md

@@ -18,8 +18,8 @@ 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.
+- `firstname`: *Deprecated: will be removed in Solidus 3.0, please use `name` attribute* - The first name for the person at this address.
+- `lastname`: *Deprecated: will be removed in Solidus 3.0, please use `name` attribute* - The last name for the person at this address.
 - `address1` and `address2`: The street address (with an optional second line).
 - `city`: The city where the address is.
 - `zipcode`: The postal code.