handle duplicates for bulk upload

app/mailers/bulk_upload_mailer.rb

@@ -43,8 +43,29 @@ class BulkUploadMailer < NotifyMailer
     end
   end
 
-  def send_correct_and_upload_again_mail(bulk_upload:)
+  def send_correct_and_upload_again_mail(bulk_upload:, errors:)
-    error_description = "We noticed that you have a lot of similar errors in column #{columns_with_errors(bulk_upload:)}. Please correct your data export and upload again."
+    any_setup_sections_incomplete_error_description = "logs where the setup sections were incomplete"
+    over_column_error_threshold_error_description = "logs with a lot of similar errors in column #{columns_with_errors(bulk_upload:)}"
+    any_logs_already_exist_error_description = "logs you are trying to upload have been created previously"
+
+    errors_list = []
+
+    errors_list << if errors.size.zero?
+                     "Please correct your data export and upload again."
+                   else
+                     "We noticed the following issues with your upload:"
+                   end
+
+    errors
+      .select { |_k, v| v }
+      .each_key do |k|
+        local_var = "#{k}_error_description"
+        errors_list << "- #{binding.local_variable_get(local_var)}"
+      end
+
+    errors_list << ""
+
+    error_description = errors_list.join("\n")
 
     summary_report_link = if BulkUploadErrorSummaryTableComponent.new(bulk_upload:).errors?
                             summary_bulk_upload_lettings_result_url(bulk_upload)

app/services/bulk_upload/lettings/row_parser.rb

@@ -166,6 +166,7 @@ class BulkUpload::Lettings::RowParser
   validate :validate_cannot_be_la_referral_if_general_needs_and_la
   validate :validate_leaving_reason_for_renewal
   validate :validate_lettings_type_matches_bulk_upload
+  validate :validate_if_log_already_exists
   validate :validate_only_one_housing_needs_type
   validate :validate_no_disabled_needs_conjunction
   validate :validate_dont_know_disabled_needs_conjunction
@@ -226,8 +227,24 @@ class BulkUpload::Lettings::RowParser
     log.form.setup_sections[0].subsections[0].is_incomplete?(log)
   end
 
+  def log_already_exists?
+    LettingsLog.exists?(duplicity_check_fields.index_with { |field| log.public_send(field) })
+  end
+
 private
 
+  def duplicity_check_fields
+    %w[
+      startdate
+      age1
+      sex1
+      ecstat1
+      owning_organisation
+      tcharge
+      propcode
+    ]
+  end
+
   def validate_location_related
     return if scheme.blank? || location.blank?
 
@@ -432,6 +449,25 @@ private
     log.form.setup_sections[0].subsections[0].questions.include?(question)
   end
 
+  def validate_if_log_already_exists
+    if log_already_exists?
+      error_message = "This is a duplicate log"
+
+      errors.add(:field_12, error_message) # age1
+      errors.add(:field_20, error_message) # sex1
+      errors.add(:field_35, error_message) # ecstat1
+
+      errors.add(:field_84, error_message) # tcharge
+
+      errors.add(:field_96, error_message) # startdate
+      errors.add(:field_97, error_message) # startdate
+      errors.add(:field_98, error_message) # startdate
+
+      errors.add(:field_100, error_message) # propcode
+      errors.add(:field_111, error_message) # owning_organisation
+    end
+  end
+
   def field_mapping_for_errors
     {
       lettype: [:field_1],
@@ -630,18 +666,10 @@ private
     Organisation.find_by_id_on_mulitple_fields(field_111)
   end
 
-  def owning_organisation_id
-    owning_organisation&.id
-  end
-
   def managing_organisation
     Organisation.find_by_id_on_mulitple_fields(field_113)
   end
 
-  def managing_organisation_id
-    managing_organisation&.id
-  end
-
   def attributes_for_log
     attributes = {}
 
@@ -650,8 +678,8 @@ private
     attributes["la"] = field_107
     attributes["postcode_known"] = postcode_known
     attributes["postcode_full"] = postcode_full
-    attributes["owning_organisation_id"] = owning_organisation_id
+    attributes["owning_organisation"] = owning_organisation
-    attributes["managing_organisation_id"] = managing_organisation_id
+    attributes["managing_organisation"] = managing_organisation
     attributes["renewal"] = renewal
     attributes["scheme"] = scheme
     attributes["location"] = location

app/services/bulk_upload/lettings/validator.rb

@@ -178,8 +178,10 @@ class BulkUpload::Lettings::Validator
     return false if any_setup_sections_incomplete?
     return false if over_column_error_threshold?
     return false if row_parsers.any?(&:block_log_creation?)
+    return false if any_logs_already_exist?
+    return false if any_logs_invalid?
 
-    row_parsers.all? { |row_parser| row_parser.log.valid? }
+    true
   end
 
   def self.question_for_field(field)
@@ -190,8 +192,6 @@ class BulkUpload::Lettings::Validator
     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
@@ -205,6 +205,16 @@ private
     end
   end
 
+  def any_logs_already_exist?
+    row_parsers.any?(&:log_already_exists?)
+  end
+
+private
+
+  def any_logs_invalid?
+    row_parsers.any? { |row_parser| row_parser.log.invalid? }
+  end
+
   def csv_parser
     @csv_parser ||= BulkUpload::Lettings::CsvParser.new(path:)
   end

app/services/bulk_upload/processor.rb

@@ -37,9 +37,14 @@ private
   end
 
   def send_correct_and_upload_again_mail
-    BulkUploadMailer
+    BulkUploadMailer.send_correct_and_upload_again_mail(
-      .send_correct_and_upload_again_mail(bulk_upload:)
+      bulk_upload:,
-      .deliver_later
+      errors: {
+        any_setup_sections_incomplete: validator.any_setup_sections_incomplete?,
+        over_column_error_threshold: validator.over_column_error_threshold?,
+        any_logs_already_exist: validator.any_logs_already_exist?,
+      },
+    ).deliver_later
   end
 
   def send_fix_errors_mail

spec/mailers/bulk_upload_mailer_spec.rb

@@ -109,13 +109,8 @@ RSpec.describe BulkUploadMailer do
   end
 
   describe "#send_correct_and_upload_again_mail" do
-    context "when 2 columns with errors" do
+    context "when there are no errors" do
-      before do
+      it "sends correctly formed email" do
-        create(:bulk_upload_error, bulk_upload:, col: "A")
-        create(:bulk_upload_error, bulk_upload:, col: "B")
-      end
-
-      it "sends correctly formed email with A, B" do
         expect(notify_client).to receive(:send_email).with(
           email_address: user.email,
           template_id: described_class::BULK_UPLOAD_FAILED_CSV_ERRORS_TEMPLATE_ID,
@@ -124,16 +119,16 @@ RSpec.describe BulkUploadMailer do
             upload_timestamp: bulk_upload.created_at.to_fs(:govuk_date_and_time),
             year_combo: bulk_upload.year_combo,
             lettings_or_sales: bulk_upload.log_type,
-            error_description: "We noticed that you have a lot of similar errors in column A, B. Please correct your data export and upload again.",
             summary_report_link: "http://localhost:3000/lettings-logs/bulk-upload-results/#{bulk_upload.id}",
+            error_description: "Please correct your data export and upload again.\n",
           },
         )
 
-        mailer.send_correct_and_upload_again_mail(bulk_upload:)
+        mailer.send_correct_and_upload_again_mail(bulk_upload:, errors: {})
       end
     end
 
-    context "when 4 columns with errors" do
+    context "when are multiple errors" do
       before do
         stub_const("BulkUploadErrorSummaryTableComponent::DISPLAY_THRESHOLD", 0)
 
@@ -144,6 +139,8 @@ RSpec.describe BulkUploadMailer do
       end
 
       it "sends correctly formed email with A, B, C and more" do
+        error_description = "We noticed the following issues with your upload:\n- logs where the setup sections were incomplete\n- logs with a lot of similar errors in column A, B, C and more\n- logs you are trying to upload have been created previously\n"
+
         expect(notify_client).to receive(:send_email).with(
           email_address: user.email,
           template_id: described_class::BULK_UPLOAD_FAILED_CSV_ERRORS_TEMPLATE_ID,
@@ -152,12 +149,18 @@ RSpec.describe BulkUploadMailer do
             upload_timestamp: bulk_upload.created_at.to_fs(:govuk_date_and_time),
             year_combo: bulk_upload.year_combo,
             lettings_or_sales: bulk_upload.log_type,
-            error_description: "We noticed that you have a lot of similar errors in column A, B, C and more. Please correct your data export and upload again.",
+            error_description:,
             summary_report_link: "http://localhost:3000/lettings-logs/bulk-upload-results/#{bulk_upload.id}/summary",
           },
         )
 
-        mailer.send_correct_and_upload_again_mail(bulk_upload:)
+        errors = {
+          any_setup_sections_incomplete: true,
+          over_column_error_threshold: true,
+          any_logs_already_exist: true,
+        }
+
+        mailer.send_correct_and_upload_again_mail(bulk_upload:, errors:)
       end
     end
   end

spec/services/bulk_upload/lettings/row_parser_spec.rb

@@ -209,6 +209,35 @@ RSpec.describe BulkUpload::Lettings::RowParser do
           expect(questions.map(&:id).size).to eq(0)
           expect(questions.map(&:id)).to eql([])
         end
+
+        context "when the log already exists in the db" do
+          before do
+            parser.log.save!
+          end
+
+          it "is not a valid row" do
+            expect(parser).not_to be_valid
+          end
+
+          it "adds an error to all (and only) the fields used to determine duplicity" do
+            parser.valid?
+
+            error_message = "This is a duplicate log"
+
+            expected_errors = {
+              field_12: [error_message], # age1
+              field_20: [error_message], # sex1
+              field_35: [error_message], # ecstat1
+              field_84: [error_message], # tcharge
+              field_96: [error_message], # startdate
+              field_97: [error_message], # startdate
+              field_98: [error_message], # startdate
+              field_100: [error_message], # propcode
+              field_111: [error_message], # owning_organisation
+            }
+            expect(parser.errors.as_json).to eq(expected_errors)
+          end
+        end
       end
     end
 

spec/services/bulk_upload/processor_spec.rb

@@ -193,8 +193,10 @@ RSpec.describe BulkUpload::Processor do
           BulkUpload::Lettings::Validator,
           invalid?: false,
           call: nil,
-          any_setup_sections_incomplete?: false,
           create_logs?: false,
+          any_setup_sections_incomplete?: false,
+          over_column_error_threshold?: false,
+          any_logs_already_exist?: false,
         )
       end