From 5077dc035bfd723d9875c99279ca84f58245ce7d Mon Sep 17 00:00:00 2001 From: Phil Lee Date: Fri, 3 Feb 2023 09:58:32 +0000 Subject: [PATCH] CLDC-1767 Bulk upload fail email (#1264) * bulk upload file validations send failure email * handle errors during bulk upload parsing --- app/mailers/bulk_upload_mailer.rb | 20 +++-- app/services/bulk_upload/processor.rb | 10 +++ spec/mailers/bulk_upload_mailer_spec.rb | 20 ++++- spec/services/bulk_upload/processor_spec.rb | 86 +++++++++++++++++++++ 4 files changed, 128 insertions(+), 8 deletions(-) diff --git a/app/mailers/bulk_upload_mailer.rb b/app/mailers/bulk_upload_mailer.rb index b406a5c4b..ac8f626f3 100644 --- a/app/mailers/bulk_upload_mailer.rb +++ b/app/mailers/bulk_upload_mailer.rb @@ -63,16 +63,22 @@ class BulkUploadMailer < NotifyMailer ) end - def send_bulk_upload_failed_service_error_mail(user, bulk_upload) + def send_bulk_upload_failed_service_error_mail(bulk_upload:) + bulk_upload_link = if bulk_upload.lettings? + start_bulk_upload_lettings_logs_url + else + start_bulk_upload_sales_logs_url + end + send_email( - user.email, + bulk_upload.user.email, BULK_UPLOAD_FAILED_SERVICE_ERROR_TEMPLATE_ID, { - filename: "[#{bulk_upload} filename]", - upload_timestamp: "[#{bulk_upload} upload_timestamp]", - lettings_or_sales: "[#{bulk_upload} lettings_or_sales]", - year_combo: "[#{bulk_upload} year_combo]", - bulk_upload_link: "[#{bulk_upload} bulk_upload_link]", + filename: bulk_upload.filename, + upload_timestamp: bulk_upload.created_at, + lettings_or_sales: bulk_upload.log_type, + year_combo: bulk_upload.year_combo, + bulk_upload_link:, }, ) end diff --git a/app/services/bulk_upload/processor.rb b/app/services/bulk_upload/processor.rb index 6b444a205..0cf059da2 100644 --- a/app/services/bulk_upload/processor.rb +++ b/app/services/bulk_upload/processor.rb @@ -7,9 +7,15 @@ class BulkUpload::Processor def call download + + return send_failure_mail if validator.invalid? + validator.call create_logs if validator.create_logs? send_success_mail + rescue StandardError => e + Sentry.capture_exception(e) + send_failure_mail ensure downloader.delete_local_file! end @@ -22,6 +28,10 @@ private end end + def send_failure_mail + BulkUploadMailer.send_bulk_upload_failed_service_error_mail(bulk_upload:).deliver_later + end + def user bulk_upload.user end diff --git a/spec/mailers/bulk_upload_mailer_spec.rb b/spec/mailers/bulk_upload_mailer_spec.rb index 526f3584a..3d7c0454f 100644 --- a/spec/mailers/bulk_upload_mailer_spec.rb +++ b/spec/mailers/bulk_upload_mailer_spec.rb @@ -5,7 +5,7 @@ RSpec.describe BulkUploadMailer do let(:notify_client) { instance_double(Notifications::Client) } let(:user) { create(:user, email: "user@example.com") } - let(:bulk_upload) { build(:bulk_upload, :lettings) } + let(:bulk_upload) { build(:bulk_upload, :lettings, user:) } before do allow(Notifications::Client).to receive(:new).and_return(notify_client) @@ -29,4 +29,22 @@ RSpec.describe BulkUploadMailer do mailer.send_bulk_upload_complete_mail(user:, bulk_upload:) end end + + describe "#send_bulk_upload_failed_service_error_mail" do + it "sends correctly formed email" do + expect(notify_client).to receive(:send_email).with( + email_address: user.email, + template_id: described_class::BULK_UPLOAD_FAILED_SERVICE_ERROR_TEMPLATE_ID, + personalisation: { + filename: bulk_upload.filename, + upload_timestamp: bulk_upload.created_at, + lettings_or_sales: bulk_upload.log_type, + year_combo: bulk_upload.year_combo, + bulk_upload_link: start_bulk_upload_lettings_logs_url, + }, + ) + + mailer.send_bulk_upload_failed_service_error_mail(bulk_upload:) + end + end end diff --git a/spec/services/bulk_upload/processor_spec.rb b/spec/services/bulk_upload/processor_spec.rb index 27fd9edd3..7c2ebaf06 100644 --- a/spec/services/bulk_upload/processor_spec.rb +++ b/spec/services/bulk_upload/processor_spec.rb @@ -6,6 +6,91 @@ RSpec.describe BulkUpload::Processor do let(:bulk_upload) { create(:bulk_upload, :lettings) } describe "#call" do + context "when the bulk upload itself is not considered valid" do + let(:mock_downloader) do + instance_double( + BulkUpload::Downloader, + call: nil, + path: file_fixture("2022_23_lettings_bulk_upload.csv"), + delete_local_file!: nil, + ) + end + + let(:mock_validator) do + instance_double( + BulkUpload::Lettings::Validator, + invalid?: true, + call: nil, + ) + end + + before do + allow(BulkUpload::Downloader).to receive(:new).with(bulk_upload:).and_return(mock_downloader) + allow(BulkUpload::Lettings::Validator).to receive(:new).and_return(mock_validator) + end + + it "sends failure email" do + mail_double = instance_double("ActionMailer::MessageDelivery", deliver_later: nil) + + allow(BulkUploadMailer).to receive(:send_bulk_upload_failed_service_error_mail).and_return(mail_double) + + processor.call + + expect(BulkUploadMailer).to have_received(:send_bulk_upload_failed_service_error_mail) + expect(mail_double).to have_received(:deliver_later) + end + + it "does not attempt to validate the contents of the file" do + processor.call + + expect(mock_validator).not_to have_received(:call) + end + end + + context "when the bulk upload processing throws an error" do + let(:mock_downloader) do + instance_double( + BulkUpload::Downloader, + call: nil, + path: file_fixture("2022_23_lettings_bulk_upload.csv"), + delete_local_file!: nil, + ) + end + + let(:mock_validator) do + instance_double( + BulkUpload::Lettings::Validator, + invalid?: false, + ) + end + + before do + allow(BulkUpload::Downloader).to receive(:new).with(bulk_upload:).and_return(mock_downloader) + allow(BulkUpload::Lettings::Validator).to receive(:new).and_return(mock_validator) + + allow(mock_validator).to receive(:call).and_raise(StandardError) + end + + it "sends failure email" do + mail_double = instance_double("ActionMailer::MessageDelivery", deliver_later: nil) + + allow(BulkUploadMailer).to receive(:send_bulk_upload_failed_service_error_mail).and_return(mail_double) + + processor.call + + expect(BulkUploadMailer).to have_received(:send_bulk_upload_failed_service_error_mail) + expect(mail_double).to have_received(:deliver_later) + end + + it "we log the failure" do + allow(Sentry).to receive(:capture_exception) + + processor.call + + expect(Sentry).to have_received(:capture_exception) + end + end + context "when processing a bulk upload with errors" do let(:mock_downloader) do instance_double( @@ -56,6 +141,7 @@ RSpec.describe BulkUpload::Processor do BulkUpload::Lettings::Validator, call: nil, create_logs?: true, + invalid?: false, ) end