Browse Source

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
pull/1630/head
Phil Lee 2 years ago committed by GitHub
parent
commit
cf9d9550e9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 18
      app/services/bulk_upload/lettings/validator.rb
  2. 4
      app/services/bulk_upload/lettings/year2022/csv_parser.rb
  3. 4
      app/services/bulk_upload/lettings/year2023/csv_parser.rb
  4. 9
      app/services/bulk_upload/sales/validator.rb
  5. 1
      app/services/bulk_upload/sales/year2022/csv_parser.rb
  6. 1
      app/services/bulk_upload/sales/year2023/csv_parser.rb
  7. 2
      spec/services/bulk_upload/lettings/year2022/csv_parser_spec.rb
  8. 2
      spec/services/bulk_upload/lettings/year2023/csv_parser_spec.rb
  9. 12
      spec/services/bulk_upload/sales/validator_spec.rb

18
app/services/bulk_upload/lettings/validator.rb

@ -1,9 +1,6 @@
require "csv" require "csv"
class BulkUpload::Lettings::Validator class BulkUpload::Lettings::Validator
COLUMN_PERCENTAGE_ERROR_THRESHOLD = 0.6
COLUMN_ABSOLUTE_ERROR_THRESHOLD = 16
include ActiveModel::Validations include ActiveModel::Validations
attr_reader :bulk_upload, :path attr_reader :bulk_upload, :path
@ -66,19 +63,6 @@ class BulkUpload::Lettings::Validator
.positive? .positive?
end 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? def any_logs_already_exist?
row_parsers.any?(&:log_already_exists?) row_parsers.any?(&:log_already_exists?)
end end
@ -147,7 +131,7 @@ private
def validate_field_numbers_count def validate_field_numbers_count
return if halt_validations? 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 end
def validate_max_columns_count_if_no_headers def validate_max_columns_count_if_no_headers

4
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] cols[field_numbers.find_index(field) + col_offset]
end end
def incorrect_field_count? def correct_field_count?
valid_field_numbers_count = field_numbers.count { |f| f != "field_blank" } valid_field_numbers_count = field_numbers.count { |f| f != "field_blank" }
valid_field_numbers_count != FIELDS valid_field_numbers_count == FIELDS
end end
def too_many_columns? def too_many_columns?

4
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] cols[field_numbers.find_index(field) + col_offset]
end end
def incorrect_field_count? def correct_field_count?
valid_field_numbers_count = field_numbers.count { |f| f != "field_blank" } valid_field_numbers_count = field_numbers.count { |f| f != "field_blank" }
valid_field_numbers_count != FIELDS valid_field_numbers_count == FIELDS
end end
def too_many_columns? def too_many_columns?

9
app/services/bulk_upload/sales/validator.rb

@ -4,7 +4,6 @@ class BulkUpload::Sales::Validator
attr_reader :bulk_upload, :path attr_reader :bulk_upload, :path
validate :validate_file_not_empty validate :validate_file_not_empty
validate :validate_min_columns
validate :validate_max_columns validate :validate_max_columns
def initialize(bulk_upload:, path:) def initialize(bulk_upload:, path:)
@ -116,14 +115,6 @@ private
end end
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 def validate_max_columns
return if halt_validations? return if halt_validations?

1
app/services/bulk_upload/sales/year2022/csv_parser.rb

@ -1,7 +1,6 @@
require "csv" require "csv"
class BulkUpload::Sales::Year2022::CsvParser class BulkUpload::Sales::Year2022::CsvParser
MIN_COLUMNS = 125
MAX_COLUMNS = 126 MAX_COLUMNS = 126
attr_reader :path attr_reader :path

1
app/services/bulk_upload/sales/year2023/csv_parser.rb

@ -1,7 +1,6 @@
require "csv" require "csv"
class BulkUpload::Sales::Year2023::CsvParser class BulkUpload::Sales::Year2023::CsvParser
MIN_COLUMNS = 135
MAX_COLUMNS = 142 MAX_COLUMNS = 142
attr_reader :path attr_reader :path

2
spec/services/bulk_upload/lettings/year2022/csv_parser_spec.rb

@ -78,7 +78,7 @@ RSpec.describe BulkUpload::Lettings::Year2022::CsvParser do
end end
it "counts the number of valid field numbers correctly" do 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
end end

2
spec/services/bulk_upload/lettings/year2023/csv_parser_spec.rb

@ -102,7 +102,7 @@ RSpec.describe BulkUpload::Lettings::Year2023::CsvParser do
end end
it "counts the number of valid field numbers correctly" do 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
end end

12
spec/services/bulk_upload/sales/validator_spec.rb

@ -16,18 +16,6 @@ RSpec.describe BulkUpload::Sales::Validator do
end end
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 context "when file has too many columns" do
before do before do
file.write((%w[a] * 127).join(",")) file.write((%w[a] * 127).join(","))

Loading…
Cancel
Save