Merge pull request #9710 from mkllnk/9002-customer-association-updates

Associate and update customer records when ordering

app/models/spree/order.rb

@@ -704,43 +704,12 @@ module Spree
     end
 
     def require_customer?
-      return false if new_record? || state == 'cart'
-
-      true
-    end
-
-    def customer_is_valid?
-      return true unless require_customer?
-
-      customer.present? && customer.enterprise_id == distributor_id && customer.email == email_for_customer
-    end
-
-    def email_for_customer
-      (user&.email || email)&.downcase
-    end
-
-    def associate_customer
-      return customer if customer.present?
-
-      Customer.of(distributor).find_by(email: email_for_customer)
-    end
-
-    def create_customer
-      return if customer_is_valid?
-
-      Customer.create(
-        enterprise: distributor,
-        email: email_for_customer,
-        user: user,
-        first_name: bill_address&.first_name.to_s,
-        last_name: bill_address&.last_name.to_s,
-        bill_address: bill_address&.clone,
-        ship_address: ship_address&.clone
-      )
+      persisted? && state != "cart"
     end
 
     def ensure_customer
-      self.customer = associate_customer || create_customer
+      self.customer ||= CustomerSyncer.find_and_update_customer(self)
+      self.customer ||= CustomerSyncer.create_customer(self) if require_customer?
     end
 
     def update_adjustment!(adjustment)

app/services/customer_syncer.rb

@@ -0,0 +1,48 @@
+# frozen_string_literal: true
+
+# Create, find and update customer records.
+#
+# P.S.: I almost couldn't resist to call this CustomerService.
+class CustomerSyncer
+  def self.find_and_update_customer(order)
+    find_customer(order).tap { |customer| synchronise_email(order, customer) }
+  end
+
+  def self.find_customer(order)
+    order.user&.customers&.of(order.distributor)&.first ||
+      Customer.of(order.distributor).find_by(email: customer_email(order))
+  end
+
+  def self.synchronise_email(order, customer)
+    email = order.user&.email
+
+    return unless email && customer && email != customer.email
+
+    duplicate = Customer.find_by(email: email, enterprise: order.distributor)
+
+    if duplicate.present?
+      Spree::Order.where(customer_id: duplicate.id).update_all(customer_id: customer.id)
+      Subscription.where(customer_id: duplicate.id).update_all(customer_id: customer.id)
+
+      duplicate.destroy
+    end
+
+    customer.update(email: email)
+  end
+
+  def self.create_customer(order)
+    Customer.create(
+      enterprise: order.distributor,
+      email: customer_email(order),
+      user: order.user,
+      first_name: order.bill_address&.first_name.to_s,
+      last_name: order.bill_address&.last_name.to_s,
+      bill_address: order.bill_address&.clone,
+      ship_address: order.ship_address&.clone
+    )
+  end
+
+  def self.customer_email(order)
+    (order.user&.email || order.email)&.downcase
+  end
+end

app/services/order_cart_reset.rb

@@ -30,7 +30,6 @@ class OrderCartReset
     return unless current_user
 
     order.associate_user!(current_user) if order.user.blank? || order.email.blank?
-    order.__send__(:associate_customer) if order.customer.nil? # Only associates existing customers
   end
 
   def reset_order_cycle(current_customer)

spec/models/spree/order/callbacks_spec.rb

@@ -21,10 +21,10 @@ describe Spree::Order do
 
   context "#save" do
     context "when associated with a registered user" do
-      let(:user) { double(:user, email: "test@example.com") }
+      let(:user) { Spree::User.new(email: "test@example.com") }
 
       before do
-        allow(order).to receive_messages user: user
+        order.user = user
       end
 
       it "should assign the email address of the user" do

spec/models/spree/order_spec.rb

@@ -893,45 +893,27 @@ describe Spree::Order do
     end
   end
 
-  describe "#require_customer?" do
-    context "when the record is new" do
-      let(:order) { build(:order, state: state) }
-
-      context "and the state is not cart" do
-        let(:state) { "complete" }
-
-        it "returns false" do
-          expect(order.send(:require_customer?)).to eq(false)
-        end
-      end
-
-      context "and the state is cart" do
-        let(:state) { "cart" }
-
-        it "returns false" do
-          expect(order.send(:require_customer?)).to eq(false)
-        end
-      end
+  describe "#customer" do
+    it "is not required for new records" do
+      is_expected.to_not validate_presence_of(:customer)
     end
 
-    context "when the record is not new" do
-      let(:order) { create(:order, state: state) }
+    it "is not required for new complete orders" do
+      order = Spree::Order.new(state: "complete")
 
-      context "and the state is not cart" do
-        let(:state) { "complete" }
+      expect(order).to_not validate_presence_of(:customer)
+    end
 
-        it "returns true" do
-          expect(order.send(:require_customer?)).to eq(true)
-        end
-      end
+    it "is not required for existing orders in cart state" do
+      order = create(:order)
 
-      context "and the state is cart" do
-        let(:state) { "cart" }
+      expect(order).to_not validate_presence_of(:customer)
+    end
 
-        it "returns false" do
-          expect(order.send(:require_customer?)).to eq(false)
-        end
-      end
+    it "is created for existing orders in complete state" do
+      order = create(:order, state: "complete")
+
+      expect { order.valid? }.to change { order.customer }.from(nil)
     end
   end
 
@@ -991,6 +973,50 @@ describe Spree::Order do
 
         expect(order.customer).to be_present
       end
+
+      it "recognises users with changed email address" do
+        order.update!(state: "complete")
+
+        # Change email instantly without confirmation via Devise:
+        order.user.update_columns(email: "new@email.org")
+
+        other_order = create(:order, user: order.user, distributor: distributor)
+
+        expect {
+          other_order.update!(state: "complete")
+        }.to_not change { Customer.count }
+
+        expect(other_order.customer.email).to eq "new@email.org"
+        expect(order.customer).to eq other_order.customer
+        expect(order.reload.customer.email).to eq "new@email.org"
+      end
+
+      it "resolves conflicts with duplicate customer entries" do
+        order.update!(state: "complete")
+
+        # The user may check out as guest first:
+        guest_order = create(:order, user: nil, email: "new@email.org", distributor: distributor)
+        guest_order.update!(state: "complete")
+
+        # Afterwards the user changes their email in their profile.
+        # Change email instantly without confirmation via Devise:
+        order.user.update_columns(email: "new@email.org")
+
+        other_order = nil
+
+        # The two customer entries are merged and one is deleted:
+        expect {
+          other_order = create(:order, user: order.user, distributor: distributor)
+        }.to change { Customer.count }.by(-1)
+
+        expect(other_order.customer.email).to eq "new@email.org"
+        expect(order.customer).to eq other_order.customer
+        expect(order.reload.customer.email).to eq "new@email.org"
+
+        expect(order.customer.orders).to match_array [
+          order, guest_order, other_order
+        ]
+      end
     end
   end