Merge pull request openfoodfoundation#7433 from coopdevs/raise-on-fai…

…led-customer-creation

Notify Bugsnag if customer fails to be created

app/models/spree/order.rb

@@ -732,11 +732,17 @@ def associate_customer
     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
+
+      Bugsnag.notify(customer.errors.full_messages.join(", ")) unless customer.persisted?
     end
 
     def update_adjustment!(adjustment)

spec/controllers/api/v0/orders_controller_spec.rb

@@ -66,30 +66,13 @@ module Api
       context 'as an admin user' do
         before do
           allow(controller).to receive(:spree_current_user) { admin_user }
-          get :index
         end
 
         it "retrieves a list of orders with appropriate attributes,
             including line items with appropriate attributes" do
+          get :index
           returns_orders(json_response)
         end
-
-        it "formats completed_at to 'yyyy-mm-dd hh:mm'" do
-          completed_dates = json_response['orders'].map{ |order| order['completed_at'] }
-          correct_formats = completed_dates.all?{ |a| a == order1.completed_at.strftime('%B %d, %Y') }
-
-          expect(correct_formats).to be_truthy
-        end
-
-        it "returns distributor object with id key" do
-          distributors = json_response['orders'].map{ |order| order['distributor'] }
-          expect(distributors.all?{ |d| d.key?('id') }).to be_truthy
-        end
-
-        it "returns the order number" do
-          order_numbers = json_response['orders'].map{ |order| order['number'] }
-          expect(order_numbers.all?{ |number| number.match("^R\\d{5,10}$") }).to be_truthy
-        end
       end
 
       context 'as an enterprise user' do

spec/models/spree/order/checkout_spec.rb

@@ -69,6 +69,7 @@
       before do
         order.state = 'address'
         order.shipments << create(:shipment)
+        order.distributor = build(:distributor_enterprise)
         order.email = "[email protected]"
         order.save!
       end

spec/models/spree/order_spec.rb

@@ -922,7 +922,7 @@
       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 @@
       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
@@ -1308,13 +1324,9 @@ def advance_to_delivery_state(order)
       end
     end
 
-    context 'when the is not complete' do
+    context 'when the order is not complete' do
       let(:order) do
-        build(
-          :order,
-          completed_at: nil,
-          line_items: [build(:line_item)]
-        )
+        build(:order, completed_at: nil, line_items: [build(:line_item)])
       end
 
       it 'transitions to :cart state' do
@@ -1327,7 +1339,7 @@ def advance_to_delivery_state(order)
   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

spec/serializers/api/admin/order_serializer_spec.rb

@@ -4,6 +4,7 @@
 
 describe Api::Admin::OrderSerializer do
   let(:serializer) { described_class.new order }
+  let(:order) { build(:order) }
 
   describe "#display_outstanding_balance" do
     let(:order) { create(:order) }
@@ -67,4 +68,26 @@
       end
     end
   end
+
+  describe "#completed_at" do
+    let(:order) { build(:order, state: 'complete', completed_at: DateTime.parse("2021-04-02")) }
+
+    it "formats the date" do
+      expect(serializer.completed_at).to eq("April 02, 2021")
+    end
+  end
+
+  describe "#distributor" do
+    before { order.distributor = build(:distributor_enterprise) }
+
+    it "returns distributor object with id key" do
+      expect(serializer.distributor.id).to eq(order.distributor.id)
+    end
+  end
+
+  describe "#number" do
+    it "returns the order number" do
+      expect(serializer.number).to eq(order.number)
+    end
+  end
 end

spec/serializers/api/current_order_serializer_spec.rb

@@ -3,11 +3,17 @@
 require 'spec_helper'
 
 describe Api::CurrentOrderSerializer do
-  let(:distributor) { create(:distributor_enterprise) }
-  let(:order_cycle) { create(:simple_order_cycle) }
-  let(:line_item) { create(:line_item, variant: create(:variant)) }
-  let(:order) { create(:order, line_items: [line_item]) }
-  let(:serializer) { Api::CurrentOrderSerializer.new(order, current_distributor: distributor, current_order_cycle: order_cycle).to_json }
+  let(:distributor) { build(:distributor_enterprise) }
+  let(:order_cycle) { build(:simple_order_cycle) }
+  let(:line_item) { build(:line_item, variant: create(:variant)) }
+  let(:order) { build(:order, line_items: [line_item]) }
+  let(:serializer) do
+    Api::CurrentOrderSerializer.new(
+      order,
+      current_distributor: distributor,
+      current_order_cycle: order_cycle
+    ).to_json
+  end
 
   it "serializers the current order" do
     expect(serializer).to match order.id.to_s
@@ -28,7 +34,11 @@
   end
 
   context 'when there is a shipment' do
-    before { create(:shipment, order: order) }
+    let(:shipping_method) { build(:shipping_method) }
+
+    before do
+      allow(order).to receive(:shipping_method).and_return(shipping_method)
+    end
 
     it 'includes the shipping method of the order' do
       expect(serializer).to match("\"shipping_method_id\":#{order.shipping_method.id}")