diff --git a/app/jobs/standing_order_placement_job.rb b/app/jobs/standing_order_placement_job.rb index 199e4029a7..b606de46d5 100644 --- a/app/jobs/standing_order_placement_job.rb +++ b/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) diff --git a/config/locales/en.yml b/config/locales/en.yml index bdc4d33039..5da10ae9fb 100644 --- a/config/locales/en.yml +++ b/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) diff --git a/lib/open_food_network/standing_order_summarizer.rb b/lib/open_food_network/standing_order_summarizer.rb index bfa063e922..1be1cccd39 100644 --- a/lib/open_food_network/standing_order_summarizer.rb +++ b/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 diff --git a/spec/jobs/standing_order_placement_job_spec.rb b/spec/jobs/standing_order_placement_job_spec.rb index 133f18a70e..be0bac3a18 100644 --- a/spec/jobs/standing_order_placement_job_spec.rb +++ b/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 diff --git a/spec/lib/open_food_network/standing_order_summarizer_spec.rb b/spec/lib/open_food_network/standing_order_summarizer_spec.rb index c01c5e0906..07c1094cf0 100644 --- a/spec/lib/open_food_network/standing_order_summarizer_spec.rb +++ b/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 diff --git a/spec/mailers/standing_order_mailer_spec.rb b/spec/mailers/standing_order_mailer_spec.rb index 7be631b6a4..7f7b92908a 100644 --- a/spec/mailers/standing_order_mailer_spec.rb +++ b/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")