From c65d82be01924900024ebc6a9c8ac04633ea19f7 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Fri, 16 Apr 2021 13:51:28 +0200
Subject: [PATCH] Notify Bugsnag if customer fails to be created

This will hopefully let us see in Bugsnag when #6038 happens and so
we'll have all the diagnostics data. The fact that the `order.distributor`
is optional but mandatory to create the customer already gives us some
clue.
---
 app/models/spree/order.rb       | 18 +++++++++++-----
 spec/models/spree/order_spec.rb | 38 +++++++++++++++++++++++----------
 2 files changed, 40 insertions(+), 16 deletions(-)

diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb
index 2a919c5463..9f753c6cf8 100644
--- a/app/models/spree/order.rb
+++ b/app/models/spree/order.rb
@@ -756,11 +756,19 @@ module Spree
     def ensure_customer
       return if associate_customer
 
-      customer_name = bill_address.andand.full_name
-      self.customer = Customer.create(enterprise: distributor, email: email_for_customer,
-                                      user: user, name: customer_name,
-                                      bill_address: bill_address.andand.clone,
-                                      ship_address: ship_address.andand.clone)
+      self.customer = Customer.new(
+        enterprise: distributor,
+        email: email_for_customer,
+        user: user,
+        name: bill_address.andand.full_name,
+        bill_address: bill_address.andand.clone,
+        ship_address: ship_address.andand.clone
+      )
+      customer.save
+
+      if customer.invalid?
+        Bugsnag.notify(customer.errors.full_messages.join(", "))
+      end
     end
 
     def update_adjustment!(adjustment)
diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb
index f9e258dc9d..34354f43de 100644
--- a/spec/models/spree/order_spec.rb
+++ b/spec/models/spree/order_spec.rb
@@ -922,7 +922,7 @@ describe Spree::Order do
       context "and the state is not cart" do
         let(:state) { "complete" }
 
-        it "returns true" do
+        it "returns false" do
           expect(order.send(:require_customer?)).to eq(false)
         end
       end
@@ -1048,18 +1048,34 @@ describe Spree::Order do
       end
 
       context "and order#email_for_customer does not match any existing customers" do
-        before {
+        before do
           order.bill_address = create(:address)
           order.ship_address = create(:address)
-        }
-        it "creates a new customer with defaut name and addresses" do
-          expect(order.customer).to be_nil
-          expect{ order.send(:ensure_customer) }.to change{ Customer.count }.by 1
-          expect(order.customer).to be_a Customer
+        end
 
-          expect(order.customer.name).to eq order.bill_address.full_name
-          expect(order.customer.bill_address.same_as?(order.bill_address)).to be true
-          expect(order.customer.ship_address.same_as?(order.ship_address)).to be true
+        context "and the customer is not valid" do
+          before do
+            order.distributor = nil
+            order.user = nil
+            order.email = nil
+          end
+
+          it "sends an error to Bugsnag" do
+            expect(Bugsnag)
+              .to receive(:notify).with("Email can't be blank, Enterprise can't be blank")
+            order.send(:ensure_customer)
+          end
+        end
+
+        context "and the customer is valid" do
+          it "creates a new customer with defaut name and addresses" do
+            expect(order.customer).to be_nil
+            expect { order.send(:ensure_customer) }.to change{ Customer.count }.by 1
+
+            expect(order.customer.name).to eq order.bill_address.full_name
+            expect(order.customer.bill_address.same_as?(order.bill_address)).to be true
+            expect(order.customer.ship_address.same_as?(order.ship_address)).to be true
+          end
         end
       end
     end
@@ -1327,7 +1343,7 @@ describe Spree::Order do
   describe '#set_payment_amount!' do
     let(:order) do
       shipment = build(:shipment_with, :shipping_method, shipping_method: build(:shipping_method))
-      build(:order, shipments: [shipment] )
+      build(:order, shipments: [shipment])
     end
 
     context 'after transitioning to payment' do