diff --git a/app/mailers/bulk_upload_mailer.rb b/app/mailers/bulk_upload_mailer.rb index ac8f626f3..59d03ce9b 100644 --- a/app/mailers/bulk_upload_mailer.rb +++ b/app/mailers/bulk_upload_mailer.rb @@ -83,16 +83,24 @@ class BulkUploadMailer < NotifyMailer ) end - def send_bulk_upload_with_errors_mail(user, bulk_upload) + def send_bulk_upload_with_errors_mail(bulk_upload:) + count = bulk_upload.logs.where.not(status: %w[completed]).count + + n_logs = pluralize(count, "log") + + title = "We found #{n_logs} with errors" + + error_description = "We created logs from your #{bulk_upload.year_combo} #{bulk_upload.log_type} data. There was a problem with #{count} of the logs. Click the below link to fix these logs." + send_email( - user.email, + bulk_upload.user.email, BULK_UPLOAD_WITH_ERRORS_TEMPLATE_ID, { - title: "[#{bulk_upload} title]", - filename: "[#{bulk_upload} filename]", - upload_timestamp: "[#{bulk_upload} upload_timestamp]", - error_description: "[#{bulk_upload} error_description]", - summary_report_link: "[#{bulk_upload} summary_report_link]", + title:, + filename: bulk_upload.filename, + upload_timestamp: bulk_upload.created_at.to_fs(:govuk_date_and_time), + error_description:, + summary_report_link: resume_bulk_upload_lettings_result_url(bulk_upload), }, ) end diff --git a/app/services/bulk_upload/processor.rb b/app/services/bulk_upload/processor.rb index 0cf059da2..d69f76243 100644 --- a/app/services/bulk_upload/processor.rb +++ b/app/services/bulk_upload/processor.rb @@ -11,8 +11,11 @@ class BulkUpload::Processor return send_failure_mail if validator.invalid? validator.call + create_logs if validator.create_logs? - send_success_mail + + send_fix_errors_mail if created_logs_but_incompleted? + send_success_mail if created_logs_and_all_completed? rescue StandardError => e Sentry.capture_exception(e) send_failure_mail @@ -22,10 +25,20 @@ class BulkUpload::Processor private + def send_fix_errors_mail + BulkUploadMailer.send_bulk_upload_with_errors_mail(bulk_upload:).deliver_later + end + def send_success_mail - if validator.create_logs? && bulk_upload.logs.group(:status).count.keys == %w[completed] - BulkUploadMailer.send_bulk_upload_complete_mail(user:, bulk_upload:).deliver_later - end + BulkUploadMailer.send_bulk_upload_complete_mail(user:, bulk_upload:).deliver_later + end + + def created_logs_but_incompleted? + validator.create_logs? && bulk_upload.logs.where.not(status: %w[completed]).count.positive? + end + + def created_logs_and_all_completed? + validator.create_logs? && bulk_upload.logs.group(:status).count.keys == %w[completed] end def send_failure_mail diff --git a/spec/mailers/bulk_upload_mailer_spec.rb b/spec/mailers/bulk_upload_mailer_spec.rb index 3d7c0454f..a3706beb3 100644 --- a/spec/mailers/bulk_upload_mailer_spec.rb +++ b/spec/mailers/bulk_upload_mailer_spec.rb @@ -47,4 +47,32 @@ RSpec.describe BulkUploadMailer do mailer.send_bulk_upload_failed_service_error_mail(bulk_upload:) end end + + context "when bulk upload has log which is not completed" do + before do + create(:lettings_log, :in_progress, bulk_upload:) + end + + describe "#send_bulk_upload_with_errors_mail" do + let(:error_description) do + "We created logs from your 2022/23 lettings data. There was a problem with 1 of the logs. Click the below link to fix these logs." + end + + it "sends correctly formed email" do + expect(notify_client).to receive(:send_email).with( + email_address: bulk_upload.user.email, + template_id: described_class::BULK_UPLOAD_WITH_ERRORS_TEMPLATE_ID, + personalisation: { + title: "We found 1 log with errors", + filename: bulk_upload.filename, + upload_timestamp: bulk_upload.created_at.to_fs(:govuk_date_and_time), + error_description:, + summary_report_link: "http://localhost:3000/lettings-logs/bulk-upload-results/#{bulk_upload.id}/resume", + }, + ) + + mailer.send_bulk_upload_with_errors_mail(bulk_upload:) + end + end + end end diff --git a/spec/services/bulk_upload/processor_spec.rb b/spec/services/bulk_upload/processor_spec.rb index 7c2ebaf06..b751e7dd7 100644 --- a/spec/services/bulk_upload/processor_spec.rb +++ b/spec/services/bulk_upload/processor_spec.rb @@ -91,7 +91,7 @@ RSpec.describe BulkUpload::Processor do end end - context "when processing a bulk upload with errors" do + context "when processing a bulk upload with errors but below threshold (therefore creates logs)" do let(:mock_downloader) do instance_double( BulkUpload::Downloader, @@ -101,12 +101,68 @@ RSpec.describe BulkUpload::Processor do ) end + let(:mock_validator) do + instance_double( + BulkUpload::Lettings::Validator, + invalid?: false, + call: nil, + create_logs?: true, + ) + 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 "deletes the local file afterwards" do + processor.call + + expect(mock_downloader).to have_received(:delete_local_file!) end - it "persist the validation errors" do - expect { processor.call }.to change(BulkUploadError, :count) + it "sends fix errors email" do + mail_double = instance_double("ActionMailer::MessageDelivery", deliver_later: nil) + + allow(BulkUploadMailer).to receive(:send_bulk_upload_with_errors_mail).and_return(mail_double) + + processor.call + + expect(BulkUploadMailer).to have_received(:send_bulk_upload_with_errors_mail) + expect(mail_double).to have_received(:deliver_later) + end + + it "does not send success email" do + allow(BulkUploadMailer).to receive(:send_bulk_upload_complete_mail).and_call_original + + processor.call + + expect(BulkUploadMailer).not_to have_received(:send_bulk_upload_complete_mail) + end + end + + context "when processing a bulk upload with errors but above threshold (therefore does not create logs)" 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, + call: nil, + create_logs?: 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) end it "deletes the local file afterwards" do @@ -115,6 +171,14 @@ RSpec.describe BulkUpload::Processor do expect(mock_downloader).to have_received(:delete_local_file!) end + it "does not send fix errors email" do + allow(BulkUploadMailer).to receive(:send_bulk_upload_with_errors_mail).and_call_original + + processor.call + + expect(BulkUploadMailer).not_to have_received(:send_bulk_upload_with_errors_mail) + end + it "does not send success email" do allow(BulkUploadMailer).to receive(:send_bulk_upload_complete_mail).and_call_original @@ -165,6 +229,14 @@ RSpec.describe BulkUpload::Processor do expect(mock_creator).to have_received(:call) end + it "does not send fix errors email" do + allow(BulkUploadMailer).to receive(:send_bulk_upload_with_errors_mail).and_call_original + + processor.call + + expect(BulkUploadMailer).not_to have_received(:send_bulk_upload_with_errors_mail) + end + it "sends success email" do mail_double = instance_double("ActionMailer::MessageDelivery", deliver_later: nil)