diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index f161da24e9..2c2df175cd 100644 --- a/app/models/spree/order.rb +++ b/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) diff --git a/app/services/customer_syncer.rb b/app/services/customer_syncer.rb new file mode 100644 index 0000000000..f97b845c64 --- /dev/null +++ b/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 diff --git a/app/services/order_cart_reset.rb b/app/services/order_cart_reset.rb index 9401d60c4c..e84fd67e88 100644 --- a/app/services/order_cart_reset.rb +++ b/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) diff --git a/spec/models/spree/order/callbacks_spec.rb b/spec/models/spree/order/callbacks_spec.rb index a3cb0744ec..ed5b4c2c35 100644 --- a/spec/models/spree/order/callbacks_spec.rb +++ b/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 diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb index c490457aa3..0f51eb7b7f 100644 --- a/spec/models/spree/order_spec.rb +++ b/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