diff --git a/app/services/bulk_upload/sales/validator.rb b/app/services/bulk_upload/sales/validator.rb index e6ea92bf1..ac786916f 100644 --- a/app/services/bulk_upload/sales/validator.rb +++ b/app/services/bulk_upload/sales/validator.rb @@ -12,9 +12,11 @@ class BulkUpload::Sales::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| @@ -66,6 +68,19 @@ class BulkUpload::Sales::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/sales/year2022/row_parser.rb b/app/services/bulk_upload/sales/year2022/row_parser.rb index 7eafd7e06..2900c5ad1 100644 --- a/app/services/bulk_upload/sales/year2022/row_parser.rb +++ b/app/services/bulk_upload/sales/year2022/row_parser.rb @@ -411,6 +411,27 @@ class BulkUpload::Sales::Year2022::RowParser field_1 end + def spreadsheet_duplicate_hash + attributes.slice( + "field_1", # purcahser code + "field_2", # saledate + "field_3", # saledate + "field_4", # saledate + "field_7", # age1 + "field_13", # sex1 + "field_24", # ecstat1 + "field_54", # postcode + "field_55", # postcode + "field_92", # 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_data_protection_answered diff --git a/app/services/bulk_upload/sales/year2023/row_parser.rb b/app/services/bulk_upload/sales/year2023/row_parser.rb index f2c3e6d8b..e2c6a2c89 100644 --- a/app/services/bulk_upload/sales/year2023/row_parser.rb +++ b/app/services/bulk_upload/sales/year2023/row_parser.rb @@ -533,6 +533,27 @@ class BulkUpload::Sales::Year2023::RowParser field_6 end + def spreadsheet_duplicate_hash + attributes.slice( + "field_1", # owning org + "field_3", # saledate + "field_4", # saledate + "field_5", # saledate + "field_6", # purchaser_code + "field_24", # postcode + "field_25", # postcode + "field_30", # age1 + "field_31", # sex1 + "field_35", # ecstat1 + ) + 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_data_protection_answered diff --git a/spec/factories/sales_log.rb b/spec/factories/sales_log.rb index 90d5e0ddb..905298e5e 100644 --- a/spec/factories/sales_log.rb +++ b/spec/factories/sales_log.rb @@ -36,6 +36,7 @@ FactoryBot.define do jointpur { 2 } end trait :completed do + purchid { rand(999_999_999).to_s } ownershipsch { 2 } type { 8 } saledate { Time.utc(2023, 2, 2, 10, 36, 49) } diff --git a/spec/services/bulk_upload/sales/validator_spec.rb b/spec/services/bulk_upload/sales/validator_spec.rb index 1df7d067f..0a10d1b52 100644 --- a/spec/services/bulk_upload/sales/validator_spec.rb +++ b/spec/services/bulk_upload/sales/validator_spec.rb @@ -83,6 +83,22 @@ RSpec.describe BulkUpload::Sales::Validator do expect { validator.call }.to change(BulkUploadError, :count) end end + + context "when duplicate rows present" do + let(:file) { Tempfile.new } + let(:path) { file.path } + let(:log) { build(:sales_log, :completed) } + + before do + file.write(BulkUpload::SalesLogToCsv.new(log:, col_offset: 0).to_2022_csv_row) + file.write(BulkUpload::SalesLogToCsv.new(log:, col_offset: 0).to_2022_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(20) + end + end end describe "#create_logs?" do diff --git a/spec/services/bulk_upload/sales/year2022/row_parser_spec.rb b/spec/services/bulk_upload/sales/year2022/row_parser_spec.rb index b3c008682..041ee1971 100644 --- a/spec/services/bulk_upload/sales/year2022/row_parser_spec.rb +++ b/spec/services/bulk_upload/sales/year2022/row_parser_spec.rb @@ -804,4 +804,16 @@ RSpec.describe BulkUpload::Sales::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/sales/year2023/row_parser_spec.rb b/spec/services/bulk_upload/sales/year2023/row_parser_spec.rb index c63e83a12..c2e13841a 100644 --- a/spec/services/bulk_upload/sales/year2023/row_parser_spec.rb +++ b/spec/services/bulk_upload/sales/year2023/row_parser_spec.rb @@ -1045,4 +1045,16 @@ RSpec.describe BulkUpload::Sales::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 diff --git a/spec/services/csv/sales_log_csv_service_spec.rb b/spec/services/csv/sales_log_csv_service_spec.rb index 96099aa70..358e1d40a 100644 --- a/spec/services/csv/sales_log_csv_service_spec.rb +++ b/spec/services/csv/sales_log_csv_service_spec.rb @@ -3,7 +3,7 @@ require "rails_helper" RSpec.describe Csv::SalesLogCsvService do let(:form_handler_mock) { instance_double(FormHandler) } let(:organisation) { create(:organisation) } - let!(:log) { create(:sales_log, :completed, owning_organisation: organisation) } + let!(:log) { create(:sales_log, :completed, owning_organisation: organisation, purchid: nil) } let(:service) { described_class.new(export_type: "labels") } let(:csv) { CSV.parse(service.prepare_csv(SalesLog.all)) }