diff --git a/app/services/bulk_upload/lettings/row_parser.rb b/app/services/bulk_upload/lettings/row_parser.rb index b772d0153..ea1ca963a 100644 --- a/app/services/bulk_upload/lettings/row_parser.rb +++ b/app/services/bulk_upload/lettings/row_parser.rb @@ -174,15 +174,19 @@ class BulkUpload::Lettings::RowParser validate :validate_owning_org_permitted validate :validate_owning_org_owns_stock validate :validate_owning_org_exists + validate :validate_owning_org_data_given validate :validate_managing_org_related validate :validate_managing_org_exists + validate :validate_managing_org_data_given validate :validate_scheme_related validate :validate_scheme_exists + validate :validate_scheme_data_given validate :validate_location_related validate :validate_location_exists + validate :validate_location_data_given def valid? errors.clear @@ -222,10 +226,6 @@ class BulkUpload::Lettings::RowParser block_log_creation end - def setup_section_incomplete? - log.form.setup_sections[0].subsections[0].is_incomplete?(log) - end - private def validate_location_related @@ -245,7 +245,13 @@ private def validate_location_exists if scheme && field_5.present? && location.nil? - errors.add(:field_5, "Location could be found with provided scheme code", category: :setup) + errors.add(:field_5, "Location could be found with provided scheme code") + end + end + + def validate_location_data_given + if bulk_upload.supported_housing? && field_5.blank? + errors.add(:field_5, "The scheme code must be present", category: "setup") end end @@ -263,7 +269,13 @@ private def validate_scheme_exists if field_4.present? && scheme.nil? - errors.add(:field_4, "The management group code is not correct", category: :setup) + errors.add(:field_4, "The management group code is not correct") + end + end + + def validate_scheme_data_given + if bulk_upload.supported_housing? && field_4.blank? + errors.add(:field_4, "The management group code is not correct", category: "setup") end end @@ -277,6 +289,12 @@ private def validate_managing_org_exists if managing_organisation.nil? errors.delete(:field_113) + errors.add(:field_113, "The managing organisation code is incorrect") + end + end + + def validate_managing_org_data_given + if field_113.blank? errors.add(:field_113, "The managing organisation code is incorrect", category: :setup) end end @@ -292,6 +310,12 @@ private def validate_owning_org_exists if owning_organisation.nil? errors.delete(:field_111) + errors.add(:field_111, "The owning organisation code is incorrect") + end + end + + def validate_owning_org_data_given + if field_111.blank? errors.add(:field_111, "The owning organisation code is incorrect", category: :setup) end end diff --git a/app/services/bulk_upload/lettings/validator.rb b/app/services/bulk_upload/lettings/validator.rb index 779f0315b..f5450039f 100644 --- a/app/services/bulk_upload/lettings/validator.rb +++ b/app/services/bulk_upload/lettings/validator.rb @@ -175,7 +175,7 @@ class BulkUpload::Lettings::Validator end def create_logs? - return false if any_setup_sections_incomplete? + return false if any_setup_errors? return false if over_column_error_threshold? return false if row_parsers.any?(&:block_log_creation?) @@ -186,8 +186,12 @@ class BulkUpload::Lettings::Validator QUESTIONS[field] end - def any_setup_sections_incomplete? - row_parsers.any?(&:setup_section_incomplete?) + def any_setup_errors? + bulk_upload + .bulk_upload_errors + .where(category: "setup") + .count + .positive? end private diff --git a/app/services/bulk_upload/processor.rb b/app/services/bulk_upload/processor.rb index dcf68e594..ab4fe50a7 100644 --- a/app/services/bulk_upload/processor.rb +++ b/app/services/bulk_upload/processor.rb @@ -12,7 +12,7 @@ class BulkUpload::Processor validator.call - if validator.any_setup_sections_incomplete? + if validator.any_setup_errors? send_setup_errors_mail elsif validator.create_logs? create_logs diff --git a/spec/services/bulk_upload/lettings/row_parser_spec.rb b/spec/services/bulk_upload/lettings/row_parser_spec.rb index 73b35d70c..68990c59b 100644 --- a/spec/services/bulk_upload/lettings/row_parser_spec.rb +++ b/spec/services/bulk_upload/lettings/row_parser_spec.rb @@ -298,10 +298,13 @@ RSpec.describe BulkUpload::Lettings::RowParser do describe "#field_4" do context "when nullable not permitted" do + let(:bulk_upload) { create(:bulk_upload, :lettings, user:, needstype: 2) } let(:attributes) { { bulk_upload:, field_1: "2", field_4: nil } } it "cannot be nulled" do - expect(parser.errors[:field_4]).to be_present + setup_errors = parser.errors.select { |e| e.options[:category] == "setup" } + + expect(setup_errors.find { |e| e.attribute == :field_4 }).to be_present end end @@ -350,6 +353,17 @@ RSpec.describe BulkUpload::Lettings::RowParser do end describe "#field_5" do + context "when not nullable" do + let(:bulk_upload) { create(:bulk_upload, :lettings, user:, needstype: 2) } + let(:attributes) { { bulk_upload:, field_1: "2", field_5: nil } } + + it "cannot be nulled" do + setup_errors = parser.errors.select { |e| e.options[:category] == "setup" } + + expect(setup_errors.find { |e| e.attribute == :field_5 }).to be_present + end + end + context "when location does not exist" do let(:scheme) { create(:scheme, :with_old_visible_id, owning_organisation: owning_org) } let(:attributes) do @@ -544,12 +558,12 @@ RSpec.describe BulkUpload::Lettings::RowParser do context "when all of these fields are blank" do let(:attributes) { { bulk_upload:, field_1: "1", field_96: nil, field_97: nil, field_98: nil } } - it "returns an error" do - parser.valid? + it "returns them as setup errors" do + setup_errors = parser.errors.select { |e| e.options[:category] == :setup } - expect(parser.errors[:field_96]).to be_present - expect(parser.errors[:field_97]).to be_present - expect(parser.errors[:field_98]).to be_present + expect(setup_errors.find { |e| e.attribute == :field_96 }).to be_present + expect(setup_errors.find { |e| e.attribute == :field_97 }).to be_present + expect(setup_errors.find { |e| e.attribute == :field_98 }).to be_present end end @@ -557,8 +571,6 @@ RSpec.describe BulkUpload::Lettings::RowParser do let(:attributes) { { bulk_upload:, field_1: "1", field_96: "1", field_97: "1", field_98: nil } } it "returns an error only on blank field" do - parser.valid? - expect(parser.errors[:field_96]).to be_blank expect(parser.errors[:field_97]).to be_blank expect(parser.errors[:field_98]).to be_present @@ -569,8 +581,6 @@ RSpec.describe BulkUpload::Lettings::RowParser do let(:attributes) { { bulk_upload:, field_98: "2022" } } it "returns an error" do - parser.valid? - expect(parser.errors[:field_98]).to include("Tenancy start year must be 2 digits") end end @@ -589,8 +599,6 @@ RSpec.describe BulkUpload::Lettings::RowParser do let(:bulk_upload) { create(:bulk_upload, :lettings, user:, year: 2022) } it "does not return errors" do - parser.valid? - expect(parser.errors[:field_96]).not_to be_present expect(parser.errors[:field_97]).not_to be_present expect(parser.errors[:field_98]).not_to be_present @@ -609,8 +617,6 @@ RSpec.describe BulkUpload::Lettings::RowParser do let(:bulk_upload) { create(:bulk_upload, :lettings, user:, year: 2022) } it "returns errors" do - parser.valid? - expect(parser.errors[:field_96]).to be_present expect(parser.errors[:field_97]).to be_present expect(parser.errors[:field_98]).to be_present @@ -619,6 +625,16 @@ RSpec.describe BulkUpload::Lettings::RowParser do end describe "#field_111" do # owning org + context "when no data given" do + let(:attributes) { { bulk_upload:, field_1: "1", field_111: "" } } + + it "is not permitted as setup error" do + setup_errors = parser.errors.select { |e| e.options[:category] == :setup } + + expect(setup_errors.find { |e| e.attribute == :field_111 }.message).to eql("The owning organisation code is incorrect") + end + end + context "when cannot find owning org" do let(:attributes) { { bulk_upload:, field_111: "donotexist" } } diff --git a/spec/services/bulk_upload/lettings/validator_spec.rb b/spec/services/bulk_upload/lettings/validator_spec.rb index b9d351281..ba48bd9e7 100644 --- a/spec/services/bulk_upload/lettings/validator_spec.rb +++ b/spec/services/bulk_upload/lettings/validator_spec.rb @@ -53,7 +53,7 @@ RSpec.describe BulkUpload::Lettings::Validator do expect(error.col).to eql("L") expect(error.category).to be_nil - error = BulkUploadError.order(:row, :field).find_by(field: "field_111") + error = BulkUploadError.find_by(row: "7", category: "setup", field: "field_111") expect(error.category).to eql("setup") end diff --git a/spec/services/bulk_upload/processor_spec.rb b/spec/services/bulk_upload/processor_spec.rb index 48b0c0258..8aaef8c36 100644 --- a/spec/services/bulk_upload/processor_spec.rb +++ b/spec/services/bulk_upload/processor_spec.rb @@ -106,7 +106,7 @@ RSpec.describe BulkUpload::Processor do BulkUpload::Lettings::Validator, invalid?: false, call: nil, - any_setup_sections_incomplete?: true, + any_setup_errors?: true, ) end @@ -142,7 +142,7 @@ RSpec.describe BulkUpload::Processor do BulkUpload::Lettings::Validator, invalid?: false, call: nil, - any_setup_sections_incomplete?: false, + any_setup_errors?: false, create_logs?: true, ) end @@ -193,7 +193,7 @@ RSpec.describe BulkUpload::Processor do BulkUpload::Lettings::Validator, invalid?: false, call: nil, - any_setup_sections_incomplete?: false, + any_setup_errors?: false, create_logs?: false, ) end @@ -254,7 +254,7 @@ RSpec.describe BulkUpload::Processor do BulkUpload::Lettings::Validator, call: nil, create_logs?: true, - any_setup_sections_incomplete?: false, + any_setup_errors?: false, invalid?: false, ) end