From 4e3919f80ebdd668523fe604578927661e462fe9 Mon Sep 17 00:00:00 2001 From: Phil Lee Date: Tue, 31 Jan 2023 13:16:57 +0000 Subject: [PATCH] CLDC-1781 Bulk upload thresholds (#1252) * create logs iff the log itself is valid * do not create logs if a setup section not complete * bulk upload with 60% errors will not create logs * extract magic number to constant * add bulk upload absolute threshold error rate * refactor with extract method --- .../bulk_upload/lettings/validator.rb | 25 ++- .../bulk_upload/lettings/validator_spec.rb | 146 +++++++++++++++++- spec/support/bulk_upload/log_to_csv.rb | 7 +- 3 files changed, 173 insertions(+), 5 deletions(-) diff --git a/app/services/bulk_upload/lettings/validator.rb b/app/services/bulk_upload/lettings/validator.rb index b96739836..90da6efb3 100644 --- a/app/services/bulk_upload/lettings/validator.rb +++ b/app/services/bulk_upload/lettings/validator.rb @@ -1,6 +1,9 @@ require "csv" class BulkUpload::Lettings::Validator + COLUMN_PERCENTAGE_ERROR_THRESHOLD = 0.6 + COLUMN_ABSOLUTE_ERROR_THRESHOLD = 16 + include ActiveModel::Validations QUESTIONS = { @@ -171,7 +174,10 @@ class BulkUpload::Lettings::Validator end def create_logs? - row_parsers.all?(&:valid?) + return false if any_setup_sections_incomplete? + return false if over_column_error_threshold? + + row_parsers.all? { |row_parser| row_parser.log.valid? } end def self.question_for_field(field) @@ -180,6 +186,23 @@ class BulkUpload::Lettings::Validator private + def any_setup_sections_incomplete? + row_parsers.any? { |row_parser| row_parser.log.form.setup_sections[0].subsections[0].is_incomplete?(row_parser.log) } + 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 csv_parser @csv_parser ||= BulkUpload::Lettings::CsvParser.new(path:) end diff --git a/spec/services/bulk_upload/lettings/validator_spec.rb b/spec/services/bulk_upload/lettings/validator_spec.rb index 661119448..ec9b3b7f7 100644 --- a/spec/services/bulk_upload/lettings/validator_spec.rb +++ b/spec/services/bulk_upload/lettings/validator_spec.rb @@ -78,7 +78,7 @@ RSpec.describe BulkUpload::Lettings::Validator do before do file.write(BulkUpload::LogToCsv.new(log:, line_ending: "\r\n", col_offset: 0).to_csv_row) - file.rewind + file.close end it "creates validation errors" do @@ -112,4 +112,148 @@ RSpec.describe BulkUpload::Lettings::Validator do end end end + + describe "#create_logs?" do + context "when a log is not valid?" do + let(:log_1) { build(:lettings_log, :completed, created_by: user) } + let(:log_2) { build(:lettings_log, :completed, created_by: user) } + + before do + file.write(BulkUpload::LogToCsv.new(log: log_1, line_ending: "\r\n", col_offset: 0).to_csv_row) + file.write(BulkUpload::LogToCsv.new(log: log_2, line_ending: "\r\n", col_offset: 0, overrides: { illness: 100 }).to_csv_row) + file.close + end + + it "returns false" do + validator.call + expect(validator).not_to be_create_logs + end + end + + context "when all logs valid?" do + let(:log_1) { build(:lettings_log, :completed, renttype: 1, created_by: user) } + let(:log_2) { build(:lettings_log, :completed, renttype: 1, created_by: user) } + + before do + file.write(BulkUpload::LogToCsv.new(log: log_1, line_ending: "\r\n", col_offset: 0).to_csv_row) + file.write(BulkUpload::LogToCsv.new(log: log_2, line_ending: "\r\n", col_offset: 0).to_csv_row) + file.close + end + + it "returns true" do + validator.call + expect(validator).to be_create_logs + end + end + + context "when a log has incomplete setup secion" do + let(:log) { build(:lettings_log, :in_progress, created_by: user, startdate: Time.zone.local(2022, 5, 1)) } + + before do + file.write(BulkUpload::LogToCsv.new(log:, line_ending: "\r\n", col_offset: 0).to_csv_row) + file.close + end + + it "returns false" do + validator.call + expect(validator).not_to be_create_logs + end + end + + context "when a column has error rate below absolute threshold" do + context "when a column is over 60% error threshold" do + let(:log_1) { build(:lettings_log, :completed, renttype: 1, created_by: user) } + let(:log_2) { build(:lettings_log, renttype: 2, created_by: user, builtype: nil, startdate: Time.zone.local(2022, 5, 1)) } + let(:log_3) { build(:lettings_log, renttype: 2, created_by: user, builtype: nil, startdate: Time.zone.local(2022, 5, 1)) } + let(:log_4) { build(:lettings_log, renttype: 2, created_by: user, builtype: nil, startdate: Time.zone.local(2022, 5, 1)) } + let(:log_5) { build(:lettings_log, renttype: 2, created_by: user, builtype: nil, startdate: Time.zone.local(2022, 5, 1)) } + + before do + file.write(BulkUpload::LogToCsv.new(log: log_1, line_ending: "\r\n", col_offset: 0).to_csv_row) + file.write(BulkUpload::LogToCsv.new(log: log_2, line_ending: "\r\n", col_offset: 0).to_csv_row) + file.write(BulkUpload::LogToCsv.new(log: log_3, line_ending: "\r\n", col_offset: 0).to_csv_row) + file.write(BulkUpload::LogToCsv.new(log: log_4, line_ending: "\r\n", col_offset: 0).to_csv_row) + file.write(BulkUpload::LogToCsv.new(log: log_5, line_ending: "\r\n", col_offset: 0).to_csv_row) + file.close + end + + it "returns true" do + validator.call + expect(validator).to be_create_logs + end + end + + context "when a column is under 60% error threshold" do + let(:log_1) { build(:lettings_log, :completed, renttype: 1, created_by: user) } + let(:log_2) { build(:lettings_log, :completed, renttype: 1, created_by: user) } + let(:log_3) { build(:lettings_log, renttype: 2, created_by: user, builtype: nil, startdate: Time.zone.local(2022, 5, 1)) } + let(:log_4) { build(:lettings_log, renttype: 2, created_by: user, builtype: nil, startdate: Time.zone.local(2022, 5, 1)) } + let(:log_5) { build(:lettings_log, renttype: 2, created_by: user, builtype: nil, startdate: Time.zone.local(2022, 5, 1)) } + + before do + file.write(BulkUpload::LogToCsv.new(log: log_1, line_ending: "\r\n", col_offset: 0).to_csv_row) + file.write(BulkUpload::LogToCsv.new(log: log_2, line_ending: "\r\n", col_offset: 0).to_csv_row) + file.write(BulkUpload::LogToCsv.new(log: log_3, line_ending: "\r\n", col_offset: 0).to_csv_row) + file.write(BulkUpload::LogToCsv.new(log: log_4, line_ending: "\r\n", col_offset: 0).to_csv_row) + file.write(BulkUpload::LogToCsv.new(log: log_5, line_ending: "\r\n", col_offset: 0).to_csv_row) + file.close + end + + it "returns true" do + validator.call + expect(validator).to be_create_logs + end + end + end + + context "when a column has error rate above absolute threshold" do + before do + stub_const("BulkUpload::Lettings::Validator::COLUMN_ABSOLUTE_ERROR_THRESHOLD", 1) + end + + context "when a column is over 60% error threshold" do + let(:log_1) { build(:lettings_log, :completed, renttype: 1, created_by: user) } + let(:log_2) { build(:lettings_log, renttype: 2, created_by: user, builtype: nil, startdate: Time.zone.local(2022, 5, 1)) } + let(:log_3) { build(:lettings_log, renttype: 2, created_by: user, builtype: nil, startdate: Time.zone.local(2022, 5, 1)) } + let(:log_4) { build(:lettings_log, renttype: 2, created_by: user, builtype: nil, startdate: Time.zone.local(2022, 5, 1)) } + let(:log_5) { build(:lettings_log, renttype: 2, created_by: user, builtype: nil, startdate: Time.zone.local(2022, 5, 1)) } + + before do + file.write(BulkUpload::LogToCsv.new(log: log_1, line_ending: "\r\n", col_offset: 0).to_csv_row) + file.write(BulkUpload::LogToCsv.new(log: log_2, line_ending: "\r\n", col_offset: 0).to_csv_row) + file.write(BulkUpload::LogToCsv.new(log: log_3, line_ending: "\r\n", col_offset: 0).to_csv_row) + file.write(BulkUpload::LogToCsv.new(log: log_4, line_ending: "\r\n", col_offset: 0).to_csv_row) + file.write(BulkUpload::LogToCsv.new(log: log_5, line_ending: "\r\n", col_offset: 0).to_csv_row) + file.close + end + + it "returns false" do + validator.call + expect(validator).not_to be_create_logs + end + end + + context "when a column is under 60% error threshold" do + let(:log_1) { build(:lettings_log, :completed, renttype: 1, created_by: user) } + let(:log_2) { build(:lettings_log, :completed, renttype: 1, created_by: user) } + let(:log_3) { build(:lettings_log, renttype: 2, created_by: user, builtype: nil, startdate: Time.zone.local(2022, 5, 1)) } + let(:log_4) { build(:lettings_log, renttype: 2, created_by: user, builtype: nil, startdate: Time.zone.local(2022, 5, 1)) } + let(:log_5) { build(:lettings_log, renttype: 2, created_by: user, builtype: nil, startdate: Time.zone.local(2022, 5, 1)) } + + before do + file.write(BulkUpload::LogToCsv.new(log: log_1, line_ending: "\r\n", col_offset: 0).to_csv_row) + file.write(BulkUpload::LogToCsv.new(log: log_2, line_ending: "\r\n", col_offset: 0).to_csv_row) + file.write(BulkUpload::LogToCsv.new(log: log_3, line_ending: "\r\n", col_offset: 0).to_csv_row) + file.write(BulkUpload::LogToCsv.new(log: log_4, line_ending: "\r\n", col_offset: 0).to_csv_row) + file.write(BulkUpload::LogToCsv.new(log: log_5, line_ending: "\r\n", col_offset: 0).to_csv_row) + file.close + end + + it "returns true" do + validator.call + expect(validator).to be_create_logs + end + end + end + end end diff --git a/spec/support/bulk_upload/log_to_csv.rb b/spec/support/bulk_upload/log_to_csv.rb index 7734d9fe2..628ee14b8 100644 --- a/spec/support/bulk_upload/log_to_csv.rb +++ b/spec/support/bulk_upload/log_to_csv.rb @@ -1,10 +1,11 @@ class BulkUpload::LogToCsv - attr_reader :log, :line_ending, :col_offset + attr_reader :log, :line_ending, :col_offset, :overrides - def initialize(log:, line_ending: "\n", col_offset: 1) + def initialize(log:, line_ending: "\n", col_offset: 1, overrides: {}) @log = log @line_ending = line_ending @col_offset = col_offset + @overrides = overrides end def to_csv_row @@ -135,7 +136,7 @@ class BulkUpload::LogToCsv nil, log.incfreq, log.sheltered, - log.illness, + overrides[:illness] || log.illness, log.illness_type_1, log.illness_type_2, # 120