Generalise record_failure method to record_and_log_error

app/jobs/standing_order_placement_job.rb

@@ -4,7 +4,7 @@ class StandingOrderPlacementJob
   attr_accessor :summarizer
 
   delegate :record_order, :record_success, :record_issue, to: :summarizer
-  delegate :record_failure, :send_placement_summary_emails, to: :summarizer
+  delegate :record_and_log_error, :send_placement_summary_emails, to: :summarizer
 
   def initialize
     @summarizer = OpenFoodNetwork::StandingOrderSummarizer.new
@@ -42,7 +42,7 @@ class StandingOrderPlacementJob
     move_to_completion(order)
     send_placement_email(order, changes)
   rescue StateMachine::InvalidTransition
-    record_failure(order)
+    record_and_log_error(:processing, order)
   end
 
   def cap_quantity_and_store_changes(order)

config/locales/en.yml

@@ -129,8 +129,8 @@ en:
       complete:
         title: Already Processed (%{count} orders)
         explainer: These orders were already marked as complete, and were therefore left untouched
-      failure:
-        title: Failed To Process (%{count} orders)
+      processing:
+        title: Error Encountered (%{count} orders)
         explainer: Automatic processing of these orders failed due to an error. The error has been listed where possible.
       other:
         title: Other failure (%{count} orders)

lib/open_food_network/standing_order_summarizer.rb

@@ -20,11 +20,13 @@ module OpenFoodNetwork
       summary_for(order).record_issue(type, order, message)
     end
 
-    def record_failure(order)
-      line1 = "StandingOrderPlacementError: Cannot process order #{order.number} due to errors"
+    def record_and_log_error(type, order)
+      return record_issue(type, order) unless order.errors.any?
+      error = "StandingOrder#{type.to_s.camelize}Error"
+      line1 = "#{error}: Cannot process order #{order.number} due to errors"
       line2 = "Errors: #{order.errors.full_messages.join(', ')}"
       Rails.logger.info("#{line1}\n#{line2}")
-      record_issue(:failure, order, line2)
+      record_issue(type, order, line2)
     end
 
     def send_placement_summary_emails

spec/jobs/standing_order_placement_job_spec.rb

@@ -171,9 +171,9 @@ describe StandingOrderPlacementJob do
         context "when progression of the order fails" do
           before { allow(order).to receive(:next) { false } }
 
-          it "records a failure and does not attempt to send an email" do
+          it "records an error and does not attempt to send an email" do
             expect(job).to_not receive(:send_placement_email)
-            expect(job).to receive(:record_failure).once
+            expect(job).to receive(:record_and_log_error).once
             job.send(:process, order)
           end
         end

spec/lib/open_food_network/standing_order_summarizer_spec.rb

@@ -58,21 +58,33 @@ module OpenFoodNetwork
         end
       end
 
-      describe "#record_failure" do
+      describe "#record_and_log_error" do
         before do
           allow(order).to receive(:number) { "123" }
-          allow(order).to receive(:errors) { double(:errors, full_messages: ["Some error"]) }
-          allow(summarizer).to receive(:record_issue)
         end
 
-        it "sends error info to the rails logger" do
-          expect(Rails.logger).to receive(:info)
-          summarizer.record_failure(order)
+        context "when errors exist on the order" do
+          before do
+            allow(order).to receive(:errors) { double(:errors, any?: true, full_messages: ["Some error"]) }
+          end
+
+          it "sends error info to the rails logger and calls #record_issue on itself with an error message" do
+            expect(Rails.logger).to receive(:info)
+            expect(summarizer).to receive(:record_issue).with(:processing, order, "Errors: Some error")
+            summarizer.record_and_log_error(:processing, order)
+          end
         end
 
-        it "calls #record_issue on itself" do
-          summarizer.record_failure(order)
-          expect(summarizer).to have_received(:record_issue)
+        context "when no errors exist on the order" do
+          before do
+            allow(order).to receive(:errors) { double(:errors, any?: false) }
+          end
+
+          it "falls back to calling record_issue" do
+            expect(Rails.logger).to_not receive(:info)
+            expect(summarizer).to receive(:record_issue).with(:processing, order)
+            summarizer.record_and_log_error(:processing, order)
+          end
         end
       end
     end

spec/mailers/standing_order_mailer_spec.rb

@@ -136,7 +136,7 @@ describe StandingOrderMailer do
         allow(summary).to receive(:order_count) { 37 }
         allow(summary).to receive(:success_count) { 35 }
         allow(summary).to receive(:issue_count) { 2 }
-        allow(summary).to receive(:issues) { { failure: { 1 => "Some Error Message", 2 => nil } } }
+        allow(summary).to receive(:issues) { { processing: { 1 => "Some Error Message", 2 => nil } } }
         allow(summary).to receive(:orders_affected_by) { [order1, order2] }
       end
 
@@ -147,8 +147,8 @@ describe StandingOrderMailer do
           expect(body).to include I18n.t("#{scope}.summary_overview.total", count: 37)
           expect(body).to include I18n.t("#{scope}.summary_overview.success_some", count: 35)
           expect(body).to include I18n.t("#{scope}.summary_overview.issues")
-          expect(body).to include I18n.t("#{scope}.summary_detail.failure.title", count: 2)
-          expect(body).to include I18n.t("#{scope}.summary_detail.failure.explainer")
+          expect(body).to include I18n.t("#{scope}.summary_detail.processing.title", count: 2)
+          expect(body).to include I18n.t("#{scope}.summary_detail.processing.explainer")
 
           # Lists orders for which an error was encountered
           expect(body).to include order1.number
@@ -171,8 +171,8 @@ describe StandingOrderMailer do
         it "sends the email, which notifies the enterprise that some issues were encountered" do
           expect(summary).to receive(:orders_affected_by).with(:other) { [order3, order4] }
           StandingOrderMailer.placement_summary_email(summary).deliver
-          expect(body).to include I18n.t("#{scope}.summary_detail.failure.title", count: 2)
-          expect(body).to include I18n.t("#{scope}.summary_detail.failure.explainer")
+          expect(body).to include I18n.t("#{scope}.summary_detail.processing.title", count: 2)
+          expect(body).to include I18n.t("#{scope}.summary_detail.processing.explainer")
           expect(body).to include I18n.t("#{scope}.summary_detail.other.title", count: 2)
           expect(body).to include I18n.t("#{scope}.summary_detail.other.explainer")