diff --git a/app/services/bulk_upload/lettings/validator.rb b/app/services/bulk_upload/lettings/validator.rb index 1a0e6ecf5..ec18e946b 100644 --- a/app/services/bulk_upload/lettings/validator.rb +++ b/app/services/bulk_upload/lettings/validator.rb @@ -43,8 +43,10 @@ class BulkUpload::Lettings::Validator def create_logs? return false if any_setup_errors? 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) @@ -59,8 +61,6 @@ class BulkUpload::Lettings::Validator .positive? end -private - def over_column_error_threshold? fields = ("field_1".."field_134").to_a percentage_threshold = (row_parsers.size * COLUMN_PERCENTAGE_ERROR_THRESHOLD).ceil @@ -74,6 +74,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 ||= case bulk_upload.year when 2022 diff --git a/app/services/bulk_upload/lettings/year2022/row_parser.rb b/app/services/bulk_upload/lettings/year2022/row_parser.rb index 650c3e896..5c811da7f 100644 --- a/app/services/bulk_upload/lettings/year2022/row_parser.rb +++ b/app/services/bulk_upload/lettings/year2022/row_parser.rb @@ -307,6 +307,7 @@ class BulkUpload::Lettings::Year2022::RowParser validate :validate_no_disabled_needs_conjunction, on: :after_log validate :validate_dont_know_disabled_needs_conjunction, on: :after_log validate :validate_no_and_dont_know_disabled_needs_conjunction, on: :after_log + validate :validate_if_log_already_exists, on: :after_log validate :validate_owning_org_data_given, on: :after_log validate :validate_owning_org_exists, on: :after_log @@ -392,6 +393,10 @@ class BulkUpload::Lettings::Year2022::RowParser field_100 end + def log_already_exists? + LettingsLog.exists?(duplicity_check_fields.index_with { |field| log.public_send(field) }) + end + private def validate_declaration_acceptance @@ -436,6 +441,18 @@ private @created_by ||= User.find_by(email: field_112) end + def duplicity_check_fields + %w[ + startdate + age1 + sex1 + ecstat1 + owning_organisation + tcharge + propcode + ] + end + def validate_location_related return if scheme.blank? || location.blank? @@ -694,6 +711,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], @@ -893,18 +929,10 @@ private Organisation.find_by_id_on_multiple_fields(field_111) end - def owning_organisation_id - owning_organisation&.id - end - def managing_organisation Organisation.find_by_id_on_multiple_fields(field_113) end - def managing_organisation_id - managing_organisation&.id - end - def attributes_for_log attributes = {} @@ -913,8 +941,8 @@ private attributes["la"] = field_107 attributes["postcode_known"] = postcode_known attributes["postcode_full"] = postcode_full - attributes["owning_organisation_id"] = owning_organisation_id - attributes["managing_organisation_id"] = managing_organisation_id + attributes["owning_organisation"] = owning_organisation + attributes["managing_organisation"] = managing_organisation attributes["renewal"] = renewal attributes["scheme"] = scheme attributes["location"] = location diff --git a/app/services/bulk_upload/processor.rb b/app/services/bulk_upload/processor.rb index 49d44261f..adc0de0e3 100644 --- a/app/services/bulk_upload/processor.rb +++ b/app/services/bulk_upload/processor.rb @@ -54,9 +54,14 @@ private 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:, + 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_success_mail diff --git a/spec/mailers/bulk_upload_mailer_spec.rb b/spec/mailers/bulk_upload_mailer_spec.rb index 20f78de4d..e28a5dd70 100644 --- a/spec/mailers/bulk_upload_mailer_spec.rb +++ b/spec/mailers/bulk_upload_mailer_spec.rb @@ -90,10 +90,11 @@ RSpec.describe BulkUploadMailer do year_combo: bulk_upload.year_combo, lettings_or_sales: bulk_upload.log_type, 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 end diff --git a/spec/services/bulk_upload/lettings/year2022/row_parser_spec.rb b/spec/services/bulk_upload/lettings/year2022/row_parser_spec.rb index f092b141e..3fe08135f 100644 --- a/spec/services/bulk_upload/lettings/year2022/row_parser_spec.rb +++ b/spec/services/bulk_upload/lettings/year2022/row_parser_spec.rb @@ -216,6 +216,35 @@ RSpec.describe BulkUpload::Lettings::Year2022::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