From 736c8d7650e1b317dea73c431ff38db96ff46df0 Mon Sep 17 00:00:00 2001 From: Rachael Booth Date: Thu, 6 Jun 2024 14:44:25 +0100 Subject: [PATCH] CLDC-3483: Enforce that all lettings allocation method fields are present for bulk uploads --- .../lettings/year2023/row_parser.rb | 27 ++++++++------- .../lettings/year2024/row_parser.rb | 34 +++++++++++-------- .../lettings/year2023/row_parser_spec.rb | 20 ++++++----- .../lettings/year2024/row_parser_spec.rb | 21 +++++++----- 4 files changed, 59 insertions(+), 43 deletions(-) diff --git a/app/services/bulk_upload/lettings/year2023/row_parser.rb b/app/services/bulk_upload/lettings/year2023/row_parser.rb index 89469b040..8825c294c 100644 --- a/app/services/bulk_upload/lettings/year2023/row_parser.rb +++ b/app/services/bulk_upload/lettings/year2023/row_parser.rb @@ -338,25 +338,37 @@ class BulkUpload::Lettings::Year2023::RowParser on: :after_log validates :field_116, + presence: { + message: I18n.t("validations.not_answered", question: QUESTIONS[:field_116]), + category: :not_answered, + }, inclusion: { in: [1, 2], - message: I18n.t("validations.invalid_option", question: "was the letting made under the Choice-Based Lettings (CBL)"), + message: I18n.t("validations.invalid_option", question: QUESTIONS[:field_116]), if: -> { field_116.present? }, }, on: :after_log validates :field_117, + presence: { + message: I18n.t("validations.not_answered", question: QUESTIONS[:field_117]), + category: :not_answered, + }, inclusion: { in: [1, 2], - message: I18n.t("validations.invalid_option", question: "was the letting made under the Common Allocation Policy (CAP)"), + message: I18n.t("validations.invalid_option", question: QUESTIONS[:field_117]), if: -> { field_117.present? }, }, on: :after_log validates :field_118, + presence: { + message: I18n.t("validations.not_answered", question: QUESTIONS[:field_118]), + category: :not_answered, + }, inclusion: { in: [1, 2], - message: I18n.t("validations.invalid_option", question: "was the letting made under the Common Housing Register (CHR)"), + message: I18n.t("validations.invalid_option", question: QUESTIONS[:field_118]), if: -> { field_118.present? }, }, on: :after_log @@ -384,7 +396,6 @@ class BulkUpload::Lettings::Year2023::RowParser validate :validate_no_housing_needs_questions_answered, on: :after_log validate :validate_reasonable_preference_homeless, on: :after_log validate :validate_condition_effects, on: :after_log - validate :validate_lettings_allocation, on: :after_log validate :validate_if_log_already_exists, on: :after_log, if: -> { FeatureToggle.bulk_upload_duplicate_log_check_enabled? } validate :validate_owning_org_data_given, on: :after_log @@ -679,14 +690,6 @@ private end end - def validate_lettings_allocation - if cbl.blank? && cap.blank? && chr.blank? - errors.add(:field_116, I18n.t("validations.not_answered", question: "was the letting made under the Choice-Based Lettings (CBL)?"), category: :not_answered) - errors.add(:field_117, I18n.t("validations.not_answered", question: "was the letting made under the Common Allocation Policy (CAP)?"), category: :not_answered) - errors.add(:field_118, I18n.t("validations.not_answered", question: "was the letting made under the Common Housing Register (CHR)?"), category: :not_answered) - end - end - def household_no_illness? field_89 != 1 end diff --git a/app/services/bulk_upload/lettings/year2024/row_parser.rb b/app/services/bulk_upload/lettings/year2024/row_parser.rb index 47ad36186..23cc4fd2a 100644 --- a/app/services/bulk_upload/lettings/year2024/row_parser.rb +++ b/app/services/bulk_upload/lettings/year2024/row_parser.rb @@ -338,33 +338,49 @@ class BulkUpload::Lettings::Year2024::RowParser on: :after_log validates :field_112, + presence: { + message: I18n.t("validations.not_answered", question: QUESTIONS[:field_112]), + category: :not_answered, + }, inclusion: { in: [1, 2], - message: I18n.t("validations.invalid_option", question: "was the letting made under the Choice-Based Lettings (CBL)"), + message: I18n.t("validations.invalid_option", question: QUESTIONS[:field_112]), if: -> { field_112.present? }, }, on: :after_log validates :field_113, + presence: { + message: I18n.t("validations.not_answered", question: QUESTIONS[:field_113]), + category: :not_answered, + }, inclusion: { in: [1, 2], - message: I18n.t("validations.invalid_option", question: "was the letting made under the Common Allocation Policy (CAP)"), + message: I18n.t("validations.invalid_option", question: QUESTIONS[:field_113]), if: -> { field_113.present? }, }, on: :after_log validates :field_114, + presence: { + message: I18n.t("validations.not_answered", question: QUESTIONS[:field_114]), + category: :not_answered, + }, inclusion: { in: [1, 2], - message: I18n.t("validations.invalid_option", question: "was the letting made under the Common Housing Register (CHR)"), + message: I18n.t("validations.invalid_option", question: QUESTIONS[:field_114]), if: -> { field_114.present? }, }, on: :after_log validates :field_115, + presence: { + message: I18n.t("validations.not_answered", question: QUESTIONS[:field_115]), + category: :not_answered, + }, inclusion: { in: [1, 2], - message: I18n.t("validations.invalid_option", question: "was the letting made under the Accessible Register"), + message: I18n.t("validations.invalid_option", question: QUESTIONS[:field_115]), if: -> { field_115.present? }, }, on: :after_log @@ -391,7 +407,6 @@ class BulkUpload::Lettings::Year2024::RowParser validate :validate_no_housing_needs_questions_answered, on: :after_log validate :validate_reasonable_preference_homeless, on: :after_log validate :validate_condition_effects, on: :after_log - validate :validate_lettings_allocation, on: :after_log validate :validate_if_log_already_exists, on: :after_log, if: -> { FeatureToggle.bulk_upload_duplicate_log_check_enabled? } validate :validate_owning_org_data_given, on: :after_log @@ -712,15 +727,6 @@ private end end - def validate_lettings_allocation - if cbl.blank? && cap.blank? && chr.blank? && accessible_register.blank? - errors.add(:field_112, I18n.t("validations.not_answered", question: "was the letting made under the Choice-Based Lettings (CBL)?")) - errors.add(:field_113, I18n.t("validations.not_answered", question: "was the letting made under the Common Allocation Policy (CAP)?")) - errors.add(:field_114, I18n.t("validations.not_answered", question: "was the letting made under the Common Housing Register (CHR)?")) - errors.add(:field_115, I18n.t("validations.not_answered", question: "was the letting made under the Accessible Register?")) - end - end - def household_no_illness? field_85 != 1 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 01de24879..37291cf08 100644 --- a/spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb +++ b/spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb @@ -1306,14 +1306,18 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do end end - describe "#field_116, 117, 118" do - context "when none of field_116, 117, 118 are given" do - let(:attributes) { { bulk_upload:, field_116: "", field_117: "", field_118: "", field_89: "1" } } - - it "sets correct errors" do - expect(parser.errors[:field_116]).to include("You must answer was the letting made under the Choice-Based Lettings (CBL)?") - expect(parser.errors[:field_117]).to include("You must answer was the letting made under the Common Allocation Policy (CAP)?") - expect(parser.errors[:field_118]).to include("You must answer was the letting made under the Common Housing Register (CHR)?") + describe "#field_116, 117, 118 (lettings allocation methods)" do + %i[field_116 field_117 field_118].each do |field| + context "when only #{field} is not given" do + let(:attributes) do + override = {} + override[field] = "" + { bulk_upload:, field_116: "2", field_117: "1", field_118: "2" }.merge(override) + end + + it "adds an error to #{field}" do + expect(parser.errors[field]).to be_present + end end end end diff --git a/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb b/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb index 2c4d475ee..435bc1c30 100644 --- a/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb +++ b/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb @@ -1188,15 +1188,18 @@ RSpec.describe BulkUpload::Lettings::Year2024::RowParser do end end - describe "#field_112, 117, 119" do - context "when none of field_112, 117, 119 are given" do - let(:attributes) { { bulk_upload:, field_112: "", field_113: "", field_114: "", field_85: "1" } } - - it "sets correct errors" do - expect(parser.errors[:field_112]).to include("You must answer was the letting made under the Choice-Based Lettings (CBL)?") - expect(parser.errors[:field_113]).to include("You must answer was the letting made under the Common Allocation Policy (CAP)?") - expect(parser.errors[:field_114]).to include("You must answer was the letting made under the Common Housing Register (CHR)?") - expect(parser.errors[:field_115]).to include("You must answer was the letting made under the Accessible Register?") + describe "#field_112 - 115 (lettings allocation methods)" do + %i[field_112 field_113 field_114 field_115].each do |field| + context "when only #{field} is not given" do + let(:attributes) do + override = {} + override[field] = "" + { bulk_upload:, field_112: "2", field_113: "1", field_114: "2", field_115: "1" }.merge(override) + end + + it "adds an error to #{field}" do + expect(parser.errors[field]).to be_present + end end end end