CLDC-1768 Bulk upload file setup email (#1358)

* add placeholder tests for bulk upload mailer

* bulk upload fix setup errors email

- this plumbs in the condition so if any setup sections are incomplete we
  send that partcular email and prevent the remaining flow

* tag bulk upload setup errors for downsteam use

* add category to bulk upload errors

* persist bulk upload error category

* populate bulk upload mailer with errors

app/mailers/bulk_upload_mailer.rb

@@ -66,17 +66,40 @@ class BulkUploadMailer < NotifyMailer
     )
   end
 
-  def send_bulk_upload_failed_file_setup_error_mail(user, bulk_upload)
+  def send_bulk_upload_failed_file_setup_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
+
+    validator_class = if bulk_upload.lettings?
+                        BulkUpload::Lettings::Validator
+                      else
+                        BulkUpload::Sales::Validator
+                      end
+
+    errors = bulk_upload
+      .bulk_upload_errors
+      .where(category: "setup")
+      .group(:col, :field)
+      .count
+      .keys
+      .sort_by { |_col, field| field }
+      .map do |col, field|
+        "- Column #{col} (#{validator_class.question_for_field(field.to_sym)})"
+      end
+
     send_email(
-      user.email,
+      bulk_upload.user.email,
       BULK_UPLOAD_FAILED_FILE_SETUP_ERROR_TEMPLATE_ID,
       {
-        filename: "[#{bulk_upload} filename]",
+        filename: bulk_upload.filename,
-        upload_timestamp: "[#{bulk_upload} upload_timestamp]",
+        upload_timestamp: bulk_upload.created_at.to_fs(:govuk_date_and_time),
-        lettings_or_sales: "[#{bulk_upload} lettings_or_sales]",
+        lettings_or_sales: bulk_upload.log_type,
-        year_combo: "[#{bulk_upload} year_combo]",
+        year_combo: bulk_upload.year_combo,
-        errors_list: "[#{bulk_upload} errors_list]",
+        errors_list: errors.join("\n"),
-        bulk_upload_link: "[#{bulk_upload} bulk_upload_link]",
+        bulk_upload_link:,
       },
     )
   end

app/services/bulk_upload/lettings/row_parser.rb

@@ -203,6 +203,10 @@ class BulkUpload::Lettings::RowParser
     block_log_creation
   end
 
+  def setup_section_incomplete?
+    log.form.setup_sections[0].subsections[0].is_incomplete?(log)
+  end
+
 private
 
   def validate_location_related
@@ -222,7 +226,7 @@ private
 
   def validate_location_exists
     if scheme && field_5.present? && location.nil?
-      errors.add(:field_5, "Location could be found with provided scheme code")
+      errors.add(:field_5, "Location could be found with provided scheme code", category: :setup)
     end
   end
 
@@ -240,7 +244,7 @@ private
 
   def validate_scheme_exists
     if field_4.present? && scheme.nil?
-      errors.add(:field_4, "The management group code is not correct")
+      errors.add(:field_4, "The management group code is not correct", category: :setup)
     end
   end
 
@@ -254,7 +258,7 @@ private
   def validate_managing_org_exists
     if managing_organisation.nil?
       errors.delete(:field_113)
-      errors.add(:field_113, "The managing organisation code is incorrect")
+      errors.add(:field_113, "The managing organisation code is incorrect", category: :setup)
     end
   end
 
@@ -269,7 +273,7 @@ private
   def validate_owning_org_exists
     if owning_organisation.nil?
       errors.delete(:field_111)
-      errors.add(:field_111, "The owning organisation code is incorrect")
+      errors.add(:field_111, "The owning organisation code is incorrect", category: :setup)
     end
   end
 
@@ -387,10 +391,18 @@ private
       next if log.optional_fields.include?(question.id)
       next if question.completed?(log)
 
-      fields.each { |field| errors.add(field, I18n.t("validations.not_answered", question: question.check_answer_label&.downcase)) }
+      if setup_question?(question)
+        fields.each { |field| errors.add(field, I18n.t("validations.not_answered", question: question.check_answer_label&.downcase), category: :setup) }
+      else
+        fields.each { |field| errors.add(field, I18n.t("validations.not_answered", question: question.check_answer_label&.downcase)) }
+      end
     end
   end
 
+  def setup_question?(question)
+    log.form.setup_sections[0].subsections[0].questions.include?(question)
+  end
+
   def field_mapping_for_errors
     {
       lettype: [:field_1],
@@ -398,6 +410,8 @@ private
       postcode_known: %i[field_107 field_108 field_109],
       postcode_full: %i[field_107 field_108 field_109],
       la: %i[field_107],
+      owning_organisation: [:field_111],
+      managing_organisation: [:field_113],
       owning_organisation_id: [:field_111],
       managing_organisation_id: [:field_113],
       renewal: [:field_134],

app/services/bulk_upload/lettings/validator.rb

@@ -168,6 +168,7 @@ class BulkUpload::Lettings::Validator
           row:,
           cell: "#{cols[field_number_for_attribute(error.attribute) - col_offset + 1]}#{row}",
           col: cols[field_number_for_attribute(error.attribute) - col_offset + 1],
+          category: error.options[:category],
         )
       end
     end
@@ -185,12 +186,12 @@ class BulkUpload::Lettings::Validator
     QUESTIONS[field]
   end
 
-private
-
   def any_setup_sections_incomplete?
-    row_parsers.any? { |row_parser| row_parser.log.form.setup_sections[0].subsections[0].is_incomplete?(row_parser.log) }
+    row_parsers.any?(&:setup_section_incomplete?)
   end
 
+private
+
   def over_column_error_threshold?
     fields = ("field_1".."field_134").to_a
     percentage_threshold = (row_parsers.size * COLUMN_PERCENTAGE_ERROR_THRESHOLD).ceil

app/services/bulk_upload/processor.rb

@@ -12,11 +12,15 @@ class BulkUpload::Processor
 
     validator.call
 
-    create_logs if validator.create_logs?
+    if validator.any_setup_sections_incomplete?
-    send_correct_and_upload_again_mail unless validator.create_logs?
+      send_setup_errors_mail
-
+    elsif validator.create_logs?
-    send_fix_errors_mail if created_logs_but_incompleted?
+      create_logs
-    send_success_mail if created_logs_and_all_completed?
+      send_fix_errors_mail if created_logs_but_incompleted?
+      send_success_mail if created_logs_and_all_completed?
+    else
+      send_correct_and_upload_again_mail
+    end
   rescue StandardError => e
     Sentry.capture_exception(e)
     send_failure_mail
@@ -26,16 +30,28 @@ class BulkUpload::Processor
 
 private
 
+  def send_setup_errors_mail
+    BulkUploadMailer
+      .send_bulk_upload_failed_file_setup_error_mail(bulk_upload:)
+      .deliver_later
+  end
+
   def send_correct_and_upload_again_mail
-    BulkUploadMailer.send_correct_and_upload_again_mail(bulk_upload:).deliver_later
+    BulkUploadMailer
+      .send_correct_and_upload_again_mail(bulk_upload:)
+      .deliver_later
   end
 
   def send_fix_errors_mail
-    BulkUploadMailer.send_bulk_upload_with_errors_mail(bulk_upload:).deliver_later
+    BulkUploadMailer
+      .send_bulk_upload_with_errors_mail(bulk_upload:)
+      .deliver_later
   end
 
   def send_success_mail
-    BulkUploadMailer.send_bulk_upload_complete_mail(user:, bulk_upload:).deliver_later
+    BulkUploadMailer
+      .send_bulk_upload_complete_mail(user:, bulk_upload:)
+      .deliver_later
   end
 
   def created_logs_but_incompleted?
@@ -47,7 +63,9 @@ private
   end
 
   def send_failure_mail
-    BulkUploadMailer.send_bulk_upload_failed_service_error_mail(bulk_upload:).deliver_later
+    BulkUploadMailer
+      .send_bulk_upload_failed_service_error_mail(bulk_upload:)
+      .deliver_later
   end
 
   def user

db/migrate/20230301120116_add_category_to_bulk_upload_errors.rb

@@ -0,0 +1,5 @@
+class AddCategoryToBulkUploadErrors < ActiveRecord::Migration[7.0]
+  def change
+    add_column :bulk_upload_errors, :category, :text, null: true
+  end
+end

db/schema.rb

@@ -26,6 +26,7 @@ ActiveRecord::Schema[7.0].define(version: 2023_03_01_144555) do
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
     t.text "col"
+    t.text "category"
     t.index ["bulk_upload_id"], name: "index_bulk_upload_errors_on_bulk_upload_id"
   end
 

spec/mailers/bulk_upload_mailer_spec.rb

@@ -12,6 +12,38 @@ RSpec.describe BulkUploadMailer do
     allow(notify_client).to receive(:send_email).and_return(true)
   end
 
+  describe "#send_bulk_upload_failed_file_setup_error_mail" do
+    before do
+      create(:bulk_upload_error, bulk_upload:, col: "A", field: "field_1", category: "setup")
+      create(:bulk_upload_error, bulk_upload:, col: "E", field: "field_4", category: "setup")
+      create(:bulk_upload_error, bulk_upload:, col: "F", field: "field_5")
+    end
+
+    let(:expected_errors) do
+      [
+        "- Column A (What is the letting type?)",
+        "- Column E (Management group code)",
+      ]
+    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_FAILED_FILE_SETUP_ERROR_TEMPLATE_ID,
+        personalisation: {
+          filename: bulk_upload.filename,
+          upload_timestamp: bulk_upload.created_at.to_fs(:govuk_date_and_time),
+          lettings_or_sales: bulk_upload.log_type,
+          year_combo: bulk_upload.year_combo,
+          errors_list: expected_errors.join("\n"),
+          bulk_upload_link: start_bulk_upload_lettings_logs_url,
+        },
+      )
+
+      mailer.send_bulk_upload_failed_file_setup_error_mail(bulk_upload:)
+    end
+  end
+
   describe "#send_bulk_upload_complete_mail" do
     it "sends correctly formed email" do
       expect(notify_client).to receive(:send_email).with(

spec/services/bulk_upload/lettings/row_parser_spec.rb

@@ -210,6 +210,16 @@ RSpec.describe BulkUpload::Lettings::RowParser do
       end
     end
 
+    context "when setup section not complete" do
+      let(:attributes) { { bulk_upload:, field_7: "123" } }
+
+      it "has errors on setup fields" do
+        errors = parser.errors.select { |e| e.options[:category] == :setup }.map(&:attribute)
+
+        expect(errors).to eql(%i[field_1 field_129 field_130 field_98 field_97 field_96 field_111 field_113])
+      end
+    end
+
     describe "#field_1" do
       context "when null" do
         let(:attributes) { { bulk_upload:, field_1: nil, field_4: "1" } }

spec/services/bulk_upload/lettings/validator_spec.rb

@@ -51,6 +51,11 @@ RSpec.describe BulkUpload::Lettings::Validator do
         expect(error.row).to eql("7")
         expect(error.cell).to eql("L7")
         expect(error.col).to eql("L")
+        expect(error.category).to be_nil
+
+        error = BulkUploadError.order(:row, :field).find_by(field: "field_111")
+
+        expect(error.category).to eql("setup")
       end
     end
 

spec/services/bulk_upload/processor_spec.rb

@@ -91,6 +91,42 @@ RSpec.describe BulkUpload::Processor do
       end
     end
 
+    context "when a log has an incomplete setup section" 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,
+          any_setup_sections_incomplete?: 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 "sends setup failure email" do
+        mail_double = instance_double("ActionMailer::MessageDelivery", deliver_later: nil)
+
+        allow(BulkUploadMailer).to receive(:send_bulk_upload_failed_file_setup_error_mail).and_return(mail_double)
+
+        processor.call
+
+        expect(BulkUploadMailer).to have_received(:send_bulk_upload_failed_file_setup_error_mail)
+        expect(mail_double).to have_received(:deliver_later)
+      end
+    end
+
     context "when processing a bulk upload with errors but below threshold (therefore creates logs)" do
       let(:mock_downloader) do
         instance_double(
@@ -106,6 +142,7 @@ RSpec.describe BulkUpload::Processor do
           BulkUpload::Lettings::Validator,
           invalid?: false,
           call: nil,
+          any_setup_sections_incomplete?: false,
           create_logs?: true,
         )
       end
@@ -156,6 +193,7 @@ RSpec.describe BulkUpload::Processor do
           BulkUpload::Lettings::Validator,
           invalid?: false,
           call: nil,
+          any_setup_sections_incomplete?: false,
           create_logs?: false,
         )
       end
@@ -216,6 +254,7 @@ RSpec.describe BulkUpload::Processor do
           BulkUpload::Lettings::Validator,
           call: nil,
           create_logs?: true,
+          any_setup_sections_incomplete?: false,
           invalid?: false,
         )
       end