From be2b3911a20735a27ed8d4b866594deec7268374 Mon Sep 17 00:00:00 2001 From: Phil Lee Date: Mon, 24 Apr 2023 16:50:57 +0100 Subject: [PATCH] fix 2023 lettings setup errors --- .../lettings/year2023/row_parser.rb | 86 ++++++++++++++----- .../lettings/year2023/row_parser_spec.rb | 32 +++---- 2 files changed, 78 insertions(+), 40 deletions(-) diff --git a/app/services/bulk_upload/lettings/year2023/row_parser.rb b/app/services/bulk_upload/lettings/year2023/row_parser.rb index 46ff0ff12..df8e9e2cd 100644 --- a/app/services/bulk_upload/lettings/year2023/row_parser.rb +++ b/app/services/bulk_upload/lettings/year2023/row_parser.rb @@ -280,9 +280,61 @@ class BulkUpload::Lettings::Year2023::RowParser attribute :field_133, :integer attribute :field_134, :decimal - validates :field_5, presence: { message: I18n.t("validations.not_answered", question: "letting type") }, - inclusion: { in: (1..12).to_a, message: I18n.t("validations.invalid_option", question: "letting type") }, on: :after_log - validates :field_16, presence: { if: proc { [2, 4, 6, 8, 10, 12].include?(field_5) } }, on: :after_log + validate :validate_valid_radio_option, on: :before_log + + validates :field_5, + presence: { + message: I18n.t("validations.not_answered", question: "letting type"), + category: :setup, + }, + inclusion: { + in: (1..12).to_a, + message: I18n.t("validations.invalid_option", question: "letting type"), + unless: -> { field_5.blank? }, + category: :setup, + }, + on: :after_log + + validates :field_6, + presence: { + message: I18n.t("validations.not_answered", question: "property renewal"), + category: :setup, + }, + on: :after_log + + validates :field_7, + presence: { + message: I18n.t("validations.not_answered", question: "tenancy start date (day)"), + category: :setup, + }, + on: :after_log + + validates :field_8, + presence: { + message: I18n.t("validations.not_answered", question: "tenancy start date (month)"), + category: :setup, + }, + on: :after_log + + validates :field_9, + presence: { + message: I18n.t("validations.not_answered", question: "tenancy start date (year)"), + category: :setup, + }, + format: { + with: /\A\d{2}\z/, + message: I18n.t("validations.setup.startdate.year_not_two_digits"), + category: :setup, + unless: -> { field_9.blank? }, + }, + on: :after_log + + validates :field_16, + presence: { + if: proc { [2, 4, 6, 8, 10, 12].include?(field_5) }, + category: :setup, + }, + on: :after_log validates :field_46, format: { with: /\A\d{1,3}\z|\AR\z/, message: "Age of person 1 must be a number or the letter R" }, on: :after_log validates :field_52, format: { with: /\A\d{1,3}\z|\AR\z/, message: "Age of person 2 must be a number or the letter R" }, allow_blank: true, on: :after_log @@ -293,16 +345,8 @@ class BulkUpload::Lettings::Year2023::RowParser validates :field_72, format: { with: /\A\d{1,3}\z|\AR\z/, message: "Age of person 7 must be a number or the letter R" }, allow_blank: true, on: :after_log validates :field_76, format: { with: /\A\d{1,3}\z|\AR\z/, message: "Age of person 8 must be a number or the letter R" }, allow_blank: true, on: :after_log - validates :field_4, presence: { message: I18n.t("validations.not_answered", question: "needs type") }, on: :after_log - validates :field_6, presence: { message: I18n.t("validations.not_answered", question: "property renewal") }, on: :after_log - validates :field_7, presence: { message: I18n.t("validations.not_answered", question: "tenancy start date (day)") }, on: :after_log - validates :field_8, presence: { message: I18n.t("validations.not_answered", question: "tenancy start date (month)") }, on: :after_log - validates :field_9, presence: { message: I18n.t("validations.not_answered", question: "tenancy start date (year)") }, on: :after_log - - validates :field_9, format: { with: /\A\d{2}\z/, message: I18n.t("validations.setup.startdate.year_not_two_digits") }, on: :after_log - + validate :validate_needs_type_present validate :validate_data_types, on: :after_log - validate :validate_nulls, on: :after_log validate :validate_relevant_collection_window, on: :after_log validate :validate_la_with_local_housing_referral, on: :after_log validate :validate_cannot_be_la_referral_if_general_needs_and_la, on: :after_log @@ -336,7 +380,7 @@ class BulkUpload::Lettings::Year2023::RowParser validate :validate_declaration_acceptance, on: :after_log - validate :validate_valid_radio_option, on: :before_log + validate :validate_nulls, on: :after_log validate :validate_uprn_exists_if_any_key_adddress_fields_are_blank, on: :after_log @@ -599,7 +643,7 @@ private if setup_question?(question) fields.each do |field| - if errors[field].present? + if errors.select { |e| fields.include?(e.attribute) }.none? question_text = question.check_answer_label.presence || question.header.presence || "this question" errors.add(field, I18n.t("validations.not_answered", question: question_text.downcase), category: :setup) end @@ -620,19 +664,19 @@ private unless location.scheme == scheme block_log_creation! - errors.add(:field_17, "Scheme code must relate to a location that is owned by owning organisation or managing organisation") + errors.add(:field_17, "Scheme code must relate to a location that is owned by owning organisation or managing organisation", category: :setup) end end def validate_location_exists if scheme && field_17.present? && location.nil? - errors.add(:field_17, "Location could be found with provided scheme code") + errors.add(:field_17, "Location could be found with provided scheme code", category: :setup) end end def validate_location_data_given if supported_housing? && field_17.blank? - errors.add(:field_17, "The scheme code must be present", category: "setup") + errors.add(:field_17, I18n.t("validations.not_answered", question: "scheme code"), category: "setup") end end @@ -644,19 +688,19 @@ private unless owned_by_owning_org || owned_by_managing_org block_log_creation! - errors.add(:field_16, "This management group code does not belong to your organisation, or any of your stock owners / managing agents") + errors.add(:field_16, "This management group code does not belong to your organisation, or any of your stock owners / managing agents", category: :setup) end end def validate_scheme_exists if field_16.present? && scheme.nil? - errors.add(:field_16, "The management group code is not correct") + errors.add(:field_16, "The management group code is not correct", category: :setup) end end def validate_scheme_data_given if supported_housing? && field_16.blank? - errors.add(:field_16, "The management group code is not correct", category: "setup") + errors.add(:field_16, I18n.t("validations.not_answered", question: "management group code"), category: "setup") end end @@ -710,7 +754,7 @@ private def validate_owning_org_data_given if field_1.blank? block_log_creation! - errors.add(:field_1, "The owning organisation code is incorrect", category: :setup) + errors.add(:field_1, I18n.t("validations.not_answered", question: "owning organisation"), category: :setup) end end diff --git a/spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb b/spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb index f98eed6d0..4bf48b979 100644 --- a/spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb +++ b/spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb @@ -501,8 +501,8 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do context "when matching scheme cannot be found" do let(:attributes) { { bulk_upload:, field_5: "1", field_16: "123" } } - it "returns an error" do - expect(parser.errors[:field_16]).to be_present + it "returns a setup error" do + expect(parser.errors.where(:field_16, category: :setup)).to be_present end end @@ -510,8 +510,8 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do let(:other_scheme) { create(:scheme, :with_old_visible_id) } let(:attributes) { { bulk_upload:, field_5: "1", field_16: other_scheme.old_visible_id, field_1: owning_org.old_visible_id } } - it "returns an error" do - expect(parser.errors[:field_16]).to include("This management group code does not belong to your organisation, or any of your stock owners / managing agents") + it "returns a setup error" do + expect(parser.errors.where(:field_16, category: :setup).map(&:message)).to include("This management group code does not belong to your organisation, or any of your stock owners / managing agents") end end @@ -547,8 +547,8 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do } end - it "returns an error" do - expect(parser.errors[:field_17]).to be_present + it "returns a setup error" do + expect(parser.errors.where(:field_17, category: :setup)).to be_present end end @@ -581,8 +581,8 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do } end - it "returns an error" do - expect(parser.errors[:field_17]).to be_present + it "returns a setup error" do + expect(parser.errors.where(:field_17, category: :setup)).to be_present end end end @@ -784,9 +784,7 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do let(:attributes) { { bulk_upload:, field_1: "", field_4: 1 } } 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_1 }.message).to eql("The owning organisation code is incorrect") + expect(parser.errors.where(:field_1, category: :setup).map(&:message)).to eql(["You must answer owning organisation"]) end it "blocks log creation" do @@ -887,16 +885,12 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do end end - describe "#field_4" do + describe "#field_4" do # needs type context "when blank" do let(:attributes) { { bulk_upload:, field_4: nil, field_13: "123" } } it "is reported as a setup error" do - errors = parser.errors.select { |e| e.options[:category] == :setup } - error = errors.find { |e| e.attribute == :field_4 } - - expect(error).to be_present - expect(error.type).to eql("You must answer needs type") + expect(parser.errors.where(:field_4, category: :setup).map(&:message)).to eql(["You must answer needs type"]) end end end @@ -905,8 +899,8 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do context "when blank" do let(:attributes) { { bulk_upload:, field_1: owning_org.old_visible_id, field_6: "" } } - it "has errors on the field" do - expect(parser.errors[:field_6]).to include("You must answer property renewal") + it "has setup errors on the field" do + expect(parser.errors.where(:field_6, category: :setup).map(&:message)).to eql(["You must answer property renewal"]) end end end