From cf9d9550e9d6deeac8f8fde0b11ddfafe76e6682 Mon Sep 17 00:00:00 2001 From: Phil Lee Date: Fri, 12 May 2023 16:35:15 +0100 Subject: [PATCH] CLDC-2344 Remove bulk upload minimum column check (#1621) # Context - https://digital.dclg.gov.uk/jira/browse/CLDC-2344 - So the reported bug and these change do not match 100% - As the reported bug should have already been resolved in an earlier change as the error message that was reported has already been removed from the system - These changes remove the check from sales plus a few minor tidy ups # Changes - Remove dead code around error threshold which is no longer used or needed - Inverted negative predicate to positive one, `incorrect_field_count` => `correct_field_count` - Remove `validate_min_columns` from sales bulk upload --- app/services/bulk_upload/lettings/validator.rb | 18 +----------------- .../lettings/year2022/csv_parser.rb | 4 ++-- .../lettings/year2023/csv_parser.rb | 4 ++-- app/services/bulk_upload/sales/validator.rb | 9 --------- .../bulk_upload/sales/year2022/csv_parser.rb | 1 - .../bulk_upload/sales/year2023/csv_parser.rb | 1 - .../lettings/year2022/csv_parser_spec.rb | 2 +- .../lettings/year2023/csv_parser_spec.rb | 2 +- .../bulk_upload/sales/validator_spec.rb | 12 ------------ 9 files changed, 7 insertions(+), 46 deletions(-) diff --git a/app/services/bulk_upload/lettings/validator.rb b/app/services/bulk_upload/lettings/validator.rb index d56919bb4..c24e67235 100644 --- a/app/services/bulk_upload/lettings/validator.rb +++ b/app/services/bulk_upload/lettings/validator.rb @@ -1,9 +1,6 @@ require "csv" class BulkUpload::Lettings::Validator - COLUMN_PERCENTAGE_ERROR_THRESHOLD = 0.6 - COLUMN_ABSOLUTE_ERROR_THRESHOLD = 16 - include ActiveModel::Validations attr_reader :bulk_upload, :path @@ -66,19 +63,6 @@ class BulkUpload::Lettings::Validator .positive? end - def over_column_error_threshold? - fields = ("field_1".."field_134").to_a - percentage_threshold = (row_parsers.size * COLUMN_PERCENTAGE_ERROR_THRESHOLD).ceil - - fields.any? do |field| - count = row_parsers.count { |row_parser| row_parser.errors[field].present? } - - next if count < COLUMN_ABSOLUTE_ERROR_THRESHOLD - - count > percentage_threshold - end - end - def any_logs_already_exist? row_parsers.any?(&:log_already_exists?) end @@ -147,7 +131,7 @@ private def validate_field_numbers_count return if halt_validations? - errors.add(:base, :wrong_field_numbers_count) if csv_parser.incorrect_field_count? + errors.add(:base, :wrong_field_numbers_count) unless csv_parser.correct_field_count? end def validate_max_columns_count_if_no_headers diff --git a/app/services/bulk_upload/lettings/year2022/csv_parser.rb b/app/services/bulk_upload/lettings/year2022/csv_parser.rb index 3e6aaa640..08cf4709d 100644 --- a/app/services/bulk_upload/lettings/year2022/csv_parser.rb +++ b/app/services/bulk_upload/lettings/year2022/csv_parser.rb @@ -47,10 +47,10 @@ class BulkUpload::Lettings::Year2022::CsvParser cols[field_numbers.find_index(field) + col_offset] end - def incorrect_field_count? + def correct_field_count? valid_field_numbers_count = field_numbers.count { |f| f != "field_blank" } - valid_field_numbers_count != FIELDS + valid_field_numbers_count == FIELDS end def too_many_columns? diff --git a/app/services/bulk_upload/lettings/year2023/csv_parser.rb b/app/services/bulk_upload/lettings/year2023/csv_parser.rb index cc351d603..b3b959be6 100644 --- a/app/services/bulk_upload/lettings/year2023/csv_parser.rb +++ b/app/services/bulk_upload/lettings/year2023/csv_parser.rb @@ -47,10 +47,10 @@ class BulkUpload::Lettings::Year2023::CsvParser cols[field_numbers.find_index(field) + col_offset] end - def incorrect_field_count? + def correct_field_count? valid_field_numbers_count = field_numbers.count { |f| f != "field_blank" } - valid_field_numbers_count != FIELDS + valid_field_numbers_count == FIELDS end def too_many_columns? diff --git a/app/services/bulk_upload/sales/validator.rb b/app/services/bulk_upload/sales/validator.rb index ede32266b..54831d36e 100644 --- a/app/services/bulk_upload/sales/validator.rb +++ b/app/services/bulk_upload/sales/validator.rb @@ -4,7 +4,6 @@ class BulkUpload::Sales::Validator attr_reader :bulk_upload, :path validate :validate_file_not_empty - validate :validate_min_columns validate :validate_max_columns def initialize(bulk_upload:, path:) @@ -116,14 +115,6 @@ private end end - def validate_min_columns - return if halt_validations? - - column_count = rows.map(&:size).min - - errors.add(:base, :under_min_column_count) if column_count < csv_parser.class::MIN_COLUMNS - end - def validate_max_columns return if halt_validations? diff --git a/app/services/bulk_upload/sales/year2022/csv_parser.rb b/app/services/bulk_upload/sales/year2022/csv_parser.rb index d9887e5c7..42fb6bcc4 100644 --- a/app/services/bulk_upload/sales/year2022/csv_parser.rb +++ b/app/services/bulk_upload/sales/year2022/csv_parser.rb @@ -1,7 +1,6 @@ require "csv" class BulkUpload::Sales::Year2022::CsvParser - MIN_COLUMNS = 125 MAX_COLUMNS = 126 attr_reader :path diff --git a/app/services/bulk_upload/sales/year2023/csv_parser.rb b/app/services/bulk_upload/sales/year2023/csv_parser.rb index 2080dc4a1..1409d5510 100644 --- a/app/services/bulk_upload/sales/year2023/csv_parser.rb +++ b/app/services/bulk_upload/sales/year2023/csv_parser.rb @@ -1,7 +1,6 @@ require "csv" class BulkUpload::Sales::Year2023::CsvParser - MIN_COLUMNS = 135 MAX_COLUMNS = 142 attr_reader :path diff --git a/spec/services/bulk_upload/lettings/year2022/csv_parser_spec.rb b/spec/services/bulk_upload/lettings/year2022/csv_parser_spec.rb index cae0285bf..f2b9dda6a 100644 --- a/spec/services/bulk_upload/lettings/year2022/csv_parser_spec.rb +++ b/spec/services/bulk_upload/lettings/year2022/csv_parser_spec.rb @@ -78,7 +78,7 @@ RSpec.describe BulkUpload::Lettings::Year2022::CsvParser do end it "counts the number of valid field numbers correctly" do - expect(service.incorrect_field_count?).to be false + expect(service).to be_correct_field_count end end diff --git a/spec/services/bulk_upload/lettings/year2023/csv_parser_spec.rb b/spec/services/bulk_upload/lettings/year2023/csv_parser_spec.rb index c94bdae35..936cf1103 100644 --- a/spec/services/bulk_upload/lettings/year2023/csv_parser_spec.rb +++ b/spec/services/bulk_upload/lettings/year2023/csv_parser_spec.rb @@ -102,7 +102,7 @@ RSpec.describe BulkUpload::Lettings::Year2023::CsvParser do end it "counts the number of valid field numbers correctly" do - expect(service.incorrect_field_count?).to be false + expect(service).to be_correct_field_count end end diff --git a/spec/services/bulk_upload/sales/validator_spec.rb b/spec/services/bulk_upload/sales/validator_spec.rb index f56c8a95b..a496ad2a3 100644 --- a/spec/services/bulk_upload/sales/validator_spec.rb +++ b/spec/services/bulk_upload/sales/validator_spec.rb @@ -16,18 +16,6 @@ RSpec.describe BulkUpload::Sales::Validator do end end - context "when file has too few columns" do - before do - file.write("a," * 112) - file.write("\n") - file.rewind - end - - it "is not valid" do - expect(validator).not_to be_valid - end - end - context "when file has too many columns" do before do file.write((%w[a] * 127).join(","))