From 1c440a5f9a5cd9c5013311c74622ec29350f12f3 Mon Sep 17 00:00:00 2001 From: Phil Lee Date: Wed, 24 May 2023 12:17:08 +0100 Subject: [PATCH] handle spreadsheet dupes in lettings --- .../bulk_upload/lettings/validator.rb | 19 +++++++++++++-- .../lettings/year2022/row_parser.rb | 23 +++++++++++++++++++ .../lettings/year2023/row_parser.rb | 23 +++++++++++++++++++ app/services/bulk_upload/sales/log_creator.rb | 1 - .../bulk_upload/lettings/validator_spec.rb | 17 ++++++++++++++ .../lettings/year2022/row_parser_spec.rb | 12 ++++++++++ .../lettings/year2023/row_parser_spec.rb | 12 ++++++++++ 7 files changed, 104 insertions(+), 3 deletions(-) diff --git a/app/services/bulk_upload/lettings/validator.rb b/app/services/bulk_upload/lettings/validator.rb index cfae7b7d9..e8167a639 100644 --- a/app/services/bulk_upload/lettings/validator.rb +++ b/app/services/bulk_upload/lettings/validator.rb @@ -13,9 +13,11 @@ class BulkUpload::Lettings::Validator end def call - row_parsers.each_with_index do |row_parser, index| - row_parser.valid? + row_parsers.each(&:valid?) + + validate_duplicate_rows + row_parsers.each_with_index do |row_parser, index| row = index + row_offset + 1 row_parser.errors.each do |error| @@ -72,6 +74,19 @@ class BulkUpload::Lettings::Validator private + # n^2 algo + def validate_duplicate_rows + row_parsers.each do |rp| + dupe = row_parsers.reject { |r| r.object_id.equal?(rp.object_id) }.any? do |rp_counter| + rp.spreadsheet_duplicate_hash == rp_counter.spreadsheet_duplicate_hash + end + + if dupe + rp.add_duplicate_found_in_spreadsheet_errors + end + end + end + def any_logs_invalid? row_parsers.any? { |row_parser| row_parser.log.invalid? } end diff --git a/app/services/bulk_upload/lettings/year2022/row_parser.rb b/app/services/bulk_upload/lettings/year2022/row_parser.rb index 00bdb2d7e..922dc8be1 100644 --- a/app/services/bulk_upload/lettings/year2022/row_parser.rb +++ b/app/services/bulk_upload/lettings/year2022/row_parser.rb @@ -438,6 +438,29 @@ class BulkUpload::Lettings::Year2022::RowParser .exists?(duplicate_check_fields.index_with { |field| log.public_send(field) }) end + def spreadsheet_duplicate_hash + attributes.slice( + "field_5", # location + "field_12", # age1 + "field_20", # sex1 + "field_35", # ecstat1 + "field_84", # tcharge + "field_96", # startdate + "field_97", # startdate + "field_98", # startdate + "field_100", # propcode + "field_108", # postcode + "field_109", # postcode + "field_111", # owning org + ) + end + + def add_duplicate_found_in_spreadsheet_errors + spreadsheet_duplicate_hash.each_key do |field| + errors.add(field, "Duplicate row found in spreadsheet", category: :setup) + end + end + private def validate_declaration_acceptance diff --git a/app/services/bulk_upload/lettings/year2023/row_parser.rb b/app/services/bulk_upload/lettings/year2023/row_parser.rb index 8c9de5f62..ff095fbda 100644 --- a/app/services/bulk_upload/lettings/year2023/row_parser.rb +++ b/app/services/bulk_upload/lettings/year2023/row_parser.rb @@ -455,6 +455,29 @@ class BulkUpload::Lettings::Year2023::RowParser .exists?(duplicate_check_fields.index_with { |field| log.public_send(field) }) end + def spreadsheet_duplicate_hash + attributes.slice( + "field_1", # owning org + "field_7", # startdate + "field_8", # startdate + "field_9", # startdate + "field_14", # propcode + "field_17", # location + "field_23", # postcode + "field_24", # postcode + "field_46", # age1 + "field_47", # sex1 + "field_50", # ecstat1 + "field_132", # tcharge + ) + end + + def add_duplicate_found_in_spreadsheet_errors + spreadsheet_duplicate_hash.each_key do |field| + errors.add(field, "Duplicate row found in spreadsheet", category: :setup) + end + end + private def validate_declaration_acceptance diff --git a/app/services/bulk_upload/sales/log_creator.rb b/app/services/bulk_upload/sales/log_creator.rb index 69ef637fa..db389fc5c 100644 --- a/app/services/bulk_upload/sales/log_creator.rb +++ b/app/services/bulk_upload/sales/log_creator.rb @@ -16,7 +16,6 @@ class BulkUpload::Sales::LogCreator row_parser.log.bulk_upload = bulk_upload row_parser.log.skip_update_status = true row_parser.log.status = "pending" - row_parser.log.status_cache = row_parser.log.calculate_status begin diff --git a/spec/services/bulk_upload/lettings/validator_spec.rb b/spec/services/bulk_upload/lettings/validator_spec.rb index f4fd98a12..cbada52af 100644 --- a/spec/services/bulk_upload/lettings/validator_spec.rb +++ b/spec/services/bulk_upload/lettings/validator_spec.rb @@ -257,6 +257,23 @@ RSpec.describe BulkUpload::Lettings::Validator do end end + context "when duplicate rows present" do + let(:file) { Tempfile.new } + let(:path) { file.path } + let(:log) { build(:lettings_log, :completed) } + + before do + file.write(BulkUpload::LettingsLogToCsv.new(log:, line_ending: "\r\n").default_2023_field_numbers_row) + file.write(BulkUpload::LettingsLogToCsv.new(log:, line_ending: "\r\n").to_2023_csv_row) + file.write(BulkUpload::LettingsLogToCsv.new(log:, line_ending: "\r\n").to_2023_csv_row) + file.close + end + + it "creates errors" do + expect { validator.call }.to change(BulkUploadError.where(category: :setup, error: "Duplicate row found in spreadsheet"), :count).by(24) + end + end + context "with unix line endings" do let(:fixture_path) { file_fixture("2022_23_lettings_bulk_upload.csv") } let(:file) { Tempfile.new } 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 e107d1662..bac02dde6 100644 --- a/spec/services/bulk_upload/lettings/year2022/row_parser_spec.rb +++ b/spec/services/bulk_upload/lettings/year2022/row_parser_spec.rb @@ -1760,4 +1760,16 @@ RSpec.describe BulkUpload::Lettings::Year2022::RowParser do end end end + + describe "#spreadsheet_duplicate_hash" do + it "returns a hash" do + expect(parser.spreadsheet_duplicate_hash).to be_a(Hash) + end + end + + describe "#add_duplicate_found_in_spreadsheet_errors" do + it "adds errors" do + expect { parser.add_duplicate_found_in_spreadsheet_errors }.to change(parser.errors, :size) + end + end end diff --git a/spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb b/spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb index d0e18c915..0c885f11d 100644 --- a/spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb +++ b/spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb @@ -1830,4 +1830,16 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do end end end + + describe "#spreadsheet_duplicate_hash" do + it "returns a hash" do + expect(parser.spreadsheet_duplicate_hash).to be_a(Hash) + end + end + + describe "#add_duplicate_found_in_spreadsheet_errors" do + it "adds errors" do + expect { parser.add_duplicate_found_in_spreadsheet_errors }.to change(parser.errors, :size) + end + end end