CLDC-1762 Bulk upload fix errors email (#1267)

* refactor bulk upload with extract method

* add bulk upload mailer test

* bulk upload mails users to fix logs on website

* bulk uplad email change resume link

* refactor local variable name

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

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
+
+  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

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

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)