From a3e459c099af37f7b9a739711731b831204178b3 Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire <94526761+natdeanlewissoftwire@users.noreply.github.com> Date: Wed, 21 Jun 2023 10:27:52 +0100 Subject: [PATCH] CLDC-2436 Multiple checkbox field bu bug (#1687) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: add checkbox clearing behaviour * feat: fix ££ bug * feat: unfix ££ bug, leave to another ticket * feat: fix homeless assignment bug * refactor: simplify wip * feat: validate reasonable preference/homeless in bu 23 * feat: validate reasonable preference/homeless in bu 22 * refactor: simplify * feat: actually clear values * feat: add specific errors to condition_effects * feat: add specific errors to condition_effects * feat: add tests * refactor: lint * feat: update tests * feat: add additional null checks * feat: add additional null checks to sales * feat: fix string/sym assignment * feat: add tests --- app/controllers/form_controller.rb | 4 +- app/models/form/question.rb | 4 ++ app/models/log.rb | 7 +- .../lettings/year2022/row_parser.rb | 34 +++++++++ .../lettings/year2023/row_parser.rb | 45 +++++++++--- .../bulk_upload/sales/year2022/row_parser.rb | 12 +++- .../bulk_upload/sales/year2023/row_parser.rb | 12 +++- .../lettings/year2022/row_parser_spec.rb | 72 +++++++++++++++++-- .../lettings/year2023/row_parser_spec.rb | 60 +++++++++++++++- .../sales/year2022/row_parser_spec.rb | 13 ++++ .../sales/year2023/row_parser_spec.rb | 13 ++++ 11 files changed, 251 insertions(+), 25 deletions(-) diff --git a/app/controllers/form_controller.rb b/app/controllers/form_controller.rb index 07ec12c5d..0219b3c57 100644 --- a/app/controllers/form_controller.rb +++ b/app/controllers/form_controller.rb @@ -95,7 +95,7 @@ private next unless question_params if %w[checkbox validation_override].include?(question.type) - question.answer_options.keys.reject { |x| x.match(/divider/) }.each do |option| + question.answer_keys_without_dividers.each do |option| result[option] = question_params.include?(option) ? 1 : 0 end else @@ -194,7 +194,7 @@ private def question_missing_response?(responses_for_page, question) if %w[checkbox validation_override].include?(question.type) - answered = question.answer_options.keys.reject { |x| x.match(/divider/) }.map do |option| + answered = question.answer_keys_without_dividers.map do |option| session["fields"][option] = @log[option] = params[@log.model_name.param_key][question.id].include?(option) ? 1 : 0 params[@log.model_name.param_key][question.id].exclude?(option) end diff --git a/app/models/form/question.rb b/app/models/form/question.rb index 2d6b3aa58..2f09a061d 100644 --- a/app/models/form/question.rb +++ b/app/models/form/question.rb @@ -249,6 +249,10 @@ class Form::Question end end + def answer_keys_without_dividers + answer_options.keys.reject { |x| x.match(/divider/) } + end + private def selected_answer_option_is_derived?(log) diff --git a/app/models/log.rb b/app/models/log.rb index 3291b0ed8..d394c0360 100644 --- a/app/models/log.rb +++ b/app/models/log.rb @@ -106,7 +106,12 @@ class Log < ApplicationRecord errors.each do |error| next if setup_ids.include?(error.attribute.to_s) - public_send("#{error.attribute}=", nil) + question = form.questions.find { |q| q.id == error.attribute.to_s } + if question&.type == "checkbox" + question.answer_keys_without_dividers.each { |attribute| public_send("#{attribute}=", nil) } + else + public_send("#{error.attribute}=", nil) + end end blank_compound_invalid_non_setup_fields! diff --git a/app/services/bulk_upload/lettings/year2022/row_parser.rb b/app/services/bulk_upload/lettings/year2022/row_parser.rb index 83ed20457..b43650bfc 100644 --- a/app/services/bulk_upload/lettings/year2022/row_parser.rb +++ b/app/services/bulk_upload/lettings/year2022/row_parser.rb @@ -341,6 +341,8 @@ class BulkUpload::Lettings::Year2022::RowParser validate :validate_dont_know_disabled_needs_conjunction, on: :after_log validate :validate_no_and_dont_know_disabled_needs_conjunction, on: :after_log 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_if_log_already_exists, on: :after_log, if: -> { FeatureToggle.bulk_upload_duplicate_log_check_enabled? } validate :validate_owning_org_data_given, on: :after_log @@ -688,6 +690,38 @@ private end end + def validate_reasonable_preference_homeless + if field_69 == 1 && homeless == 1 && field_70 == 1 + errors.add(:field_70, I18n.t("validations.household.reasonpref.not_homeless")) + else + reason_fields = %i[field_70 field_71 field_72 field_73 field_74] + if field_69 == 1 && reason_fields.all? { |field| attributes[field.to_s].blank? } + reason_fields.each do |field| + errors.add(field, I18n.t("validations.not_answered", question: "reason for reasonable preference")) + end + end + end + end + + def validate_condition_effects + illness_option_fields = %i[field_119 field_120 field_121 field_122 field_123 field_124 field_125 field_126 field_127 field_128] + if household_no_illness? + illness_option_fields.each do |field| + if attributes[field.to_s] == 1 + errors.add(field, I18n.t("validations.household.condition_effects.no_choices")) + end + end + elsif illness_option_fields.all? { |field| attributes[field.to_s].blank? } + illness_option_fields.each do |field| + errors.add(field, I18n.t("validations.not_answered", question: "how is person affected by condition or illness")) + end + end + end + + def household_no_illness? + field_118 != 1 + end + def validate_lettings_type_matches_bulk_upload if [1, 3, 5, 7, 9, 11].include?(field_1) && !bulk_upload.general_needs? errors.add(:field_1, I18n.t("validations.setup.lettype.supported_housing_mismatch")) diff --git a/app/services/bulk_upload/lettings/year2023/row_parser.rb b/app/services/bulk_upload/lettings/year2023/row_parser.rb index 9c3c76fab..0fe2d7238 100644 --- a/app/services/bulk_upload/lettings/year2023/row_parser.rb +++ b/app/services/bulk_upload/lettings/year2023/row_parser.rb @@ -357,6 +357,8 @@ class BulkUpload::Lettings::Year2023::RowParser validate :validate_dont_know_disabled_needs_conjunction, on: :after_log validate :validate_no_and_dont_know_disabled_needs_conjunction, on: :after_log 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_if_log_already_exists, on: :after_log, if: -> { FeatureToggle.bulk_upload_duplicate_log_check_enabled? } validate :validate_owning_org_data_given, on: :after_log @@ -617,6 +619,38 @@ private end end + def validate_reasonable_preference_homeless + if field_110 == 1 && field_105 == 1 && field_111 == 1 + errors.add(:field_111, I18n.t("validations.household.reasonpref.not_homeless")) + else + reason_fields = %i[field_111 field_112 field_113 field_114 field_115] + if field_110 == 1 && reason_fields.all? { |field| attributes[field.to_s].blank? } + reason_fields.each do |field| + errors.add(field, I18n.t("validations.not_answered", question: "reason for reasonable preference")) + end + end + end + end + + def validate_condition_effects + illness_option_fields = %i[field_98 field_92 field_95 field_90 field_91 field_93 field_94 field_97 field_96 field_99] + if household_no_illness? + illness_option_fields.each do |field| + if attributes[field.to_s] == 1 + errors.add(field, I18n.t("validations.household.condition_effects.no_choices")) + end + end + elsif illness_option_fields.all? { |field| attributes[field.to_s].blank? } + illness_option_fields.each do |field| + errors.add(field, I18n.t("validations.not_answered", question: "how is person affected by condition or illness")) + end + end + end + + def household_no_illness? + field_89 != 1 + end + def validate_lettings_type_matches_bulk_upload if [1, 3, 5, 7, 9, 11].include?(field_5) && !general_needs? errors.add(:field_5, I18n.t("validations.setup.lettype.supported_housing_mismatch")) @@ -1104,7 +1138,7 @@ private attributes["reason"] = field_102 attributes["reasonother"] = field_103 attributes["prevten"] = field_104 - attributes["homeless"] = homeless + attributes["homeless"] = field_105 attributes["prevloc"] = prevloc attributes["previous_la_known"] = previous_la_known @@ -1374,15 +1408,6 @@ private return 1 if field_86 == 1 end - def homeless - case field_105 - when 1 - 1 - when 12 - 11 - end - end - def prevloc field_109 end diff --git a/app/services/bulk_upload/sales/year2022/row_parser.rb b/app/services/bulk_upload/sales/year2022/row_parser.rb index 40860c0d0..1acb9ddf2 100644 --- a/app/services/bulk_upload/sales/year2022/row_parser.rb +++ b/app/services/bulk_upload/sales/year2022/row_parser.rb @@ -370,6 +370,7 @@ class BulkUpload::Sales::Year2022::RowParser validate :validate_if_log_already_exists, on: :after_log, if: -> { FeatureToggle.bulk_upload_duplicate_log_check_enabled? } validate :validate_data_protection_answered, on: :after_log + validate :validate_buyers_organisations, on: :after_log def self.question_for_field(field) QUESTIONS[field] @@ -467,6 +468,15 @@ private end end + def validate_buyers_organisations + organisations_fields = %i[field_44 field_45 field_46 field_47] + if organisations_fields.all? { |field| attributes[field.to_s].blank? } + organisations_fields.each do |field| + errors.add(field, "At least one option must be selected of these four") + end + end + end + def buyer_not_interviewed? field_6 == 1 end @@ -546,7 +556,6 @@ private pregla: %i[field_45], pregghb: %i[field_46], pregother: %i[field_47], - pregblank: %i[field_44 field_45 field_46 field_47], disabled: %i[field_48], wheel: %i[field_49], beds: %i[field_50], @@ -679,7 +688,6 @@ private attributes["pregla"] = field_45 attributes["pregghb"] = field_46 attributes["pregother"] = field_47 - attributes["pregblank"] = 1 if [field_44, field_45, field_46, field_47].all?(&:blank?) attributes["disabled"] = buyer_not_interviewed? && field_48.blank? ? 3 : field_48 attributes["wheel"] = buyer_not_interviewed? && field_49.blank? ? 3 : field_49 diff --git a/app/services/bulk_upload/sales/year2023/row_parser.rb b/app/services/bulk_upload/sales/year2023/row_parser.rb index 17ec939d4..3a9f7ed41 100644 --- a/app/services/bulk_upload/sales/year2023/row_parser.rb +++ b/app/services/bulk_upload/sales/year2023/row_parser.rb @@ -467,6 +467,7 @@ class BulkUpload::Sales::Year2023::RowParser validate :validate_if_log_already_exists, on: :after_log, if: -> { FeatureToggle.bulk_upload_duplicate_log_check_enabled? } validate :validate_data_protection_answered, on: :after_log + validate :validate_buyers_organisations, on: :after_log def self.question_for_field(field) QUESTIONS[field] @@ -562,6 +563,15 @@ private end end + def validate_buyers_organisations + organisations_fields = %i[field_67 field_68 field_69 field_70] + if organisations_fields.all? { |field| attributes[field.to_s].blank? } + organisations_fields.each do |field| + errors.add(field, "At least one option must be selected of these four") + end + end + end + def prevtenbuy2 case field_72 when "R" @@ -684,7 +694,6 @@ private pregla: %i[field_69], pregghb: %i[field_70], pregother: %i[field_68], - pregblank: %i[field_67 field_69 field_70 field_68], disabled: %i[field_76], wheel: %i[field_77], @@ -847,7 +856,6 @@ private attributes["pregla"] = field_69 attributes["pregghb"] = field_70 attributes["pregother"] = field_68 - attributes["pregblank"] = 1 if [field_67, field_69, field_70, field_78].all?(&:blank?) attributes["disabled"] = field_76 attributes["wheel"] = field_77 diff --git a/spec/services/bulk_upload/lettings/year2022/row_parser_spec.rb b/spec/services/bulk_upload/lettings/year2022/row_parser_spec.rb index 365771bb6..041a9265a 100644 --- a/spec/services/bulk_upload/lettings/year2022/row_parser_spec.rb +++ b/spec/services/bulk_upload/lettings/year2022/row_parser_spec.rb @@ -649,6 +649,28 @@ RSpec.describe BulkUpload::Lettings::Year2022::RowParser do end end + describe "#field_68 - 74" do + context "when not homeless but reasonable preference for homelessness" do + let(:attributes) { { bulk_upload:, field_68: "1", field_69: "1", field_70: "1" } } + + it "is not permitted" do + expect(parser.errors[:field_70]).to be_present + end + end + + context "when there is a reasonable preference but none is given" do + let(:attributes) { { bulk_upload:, field_69: "1", field_70: nil, field_71: nil, field_72: nil, field_73: nil, field_74: nil } } + + it "is not permitted" do + expect(parser.errors[:field_70]).to be_present + expect(parser.errors[:field_71]).to be_present + expect(parser.errors[:field_72]).to be_present + expect(parser.errors[:field_73]).to be_present + expect(parser.errors[:field_74]).to be_present + end + end + end + describe "#field_78" do # referral context "when 3 ie PRP nominated by LA and owning org is LA" do let(:attributes) { { bulk_upload:, field_78: "3", field_111: owning_org.old_visible_id } } @@ -779,6 +801,20 @@ RSpec.describe BulkUpload::Lettings::Year2022::RowParser do end end + describe "#field_103" do + context "when null" do + let(:attributes) { setup_section_params.merge({ field_103: nil }) } + + it "returns an error" do + expect(parser.errors[:field_103]).to be_present + end + + it "populates with correct error message" do + expect(parser.errors[:field_103]).to eql(["You must answer type of building"]) + end + end + end + describe "#field_111" do # owning org context "when no data given" do let(:attributes) { { bulk_upload:, field_1: "1", field_111: "" } } @@ -937,16 +973,38 @@ RSpec.describe BulkUpload::Lettings::Year2022::RowParser do end end - describe "#field_103" do - context "when null" do - let(:attributes) { setup_section_params.merge({ field_103: nil }) } + describe "#field_118, field_119 - 128" do + context "when no illness but illnesses answered" do + let(:attributes) { { bulk_upload:, field_118: "2", field_119: "1", field_120: "1", field_121: "1" } } - it "returns an error" do - expect(parser.errors[:field_103]).to be_present + it "errors added to correct fields" do + expect(parser.errors[:field_119]).to be_present + expect(parser.errors[:field_120]).to be_present + expect(parser.errors[:field_121]).to be_present + expect(parser.errors[:field_122]).not_to be_present + expect(parser.errors[:field_123]).not_to be_present + expect(parser.errors[:field_124]).not_to be_present + expect(parser.errors[:field_125]).not_to be_present + expect(parser.errors[:field_126]).not_to be_present + expect(parser.errors[:field_127]).not_to be_present + expect(parser.errors[:field_128]).not_to be_present end + end - it "populates with correct error message" do - expect(parser.errors[:field_103]).to eql(["You must answer type of building"]) + context "when illness but no illnesses answered" do + let(:attributes) { { bulk_upload:, field_118: "1", field_119: nil, field_120: nil, field_121: nil, field_122: nil, field_123: nil, field_124: nil, field_125: nil, field_126: nil, field_127: nil, field_128: nil } } + + it "errors added to correct fields" do + expect(parser.errors[:field_119]).to be_present + expect(parser.errors[:field_120]).to be_present + expect(parser.errors[:field_121]).to be_present + expect(parser.errors[:field_122]).to be_present + expect(parser.errors[:field_123]).to be_present + expect(parser.errors[:field_124]).to be_present + expect(parser.errors[:field_125]).to be_present + expect(parser.errors[:field_126]).to be_present + expect(parser.errors[:field_127]).to be_present + expect(parser.errors[:field_128]).to be_present end 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 f7e5527a5..d8df99a2e 100644 --- a/spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb +++ b/spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb @@ -205,7 +205,7 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do field_101: "2", field_102: "31", field_104: "3", - field_105: "12", + field_105: "11", field_106: "1", field_107: "EC1N", @@ -689,6 +689,64 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do end end + describe "#field_89, field_98 - 99" do + context "when no illness but illnesses answered" do + let(:attributes) { { bulk_upload:, field_89: "2", field_90: "1", field_91: "1", field_92: "1" } } + + it "errors added to correct fields" do + expect(parser.errors[:field_90]).to be_present + expect(parser.errors[:field_91]).to be_present + expect(parser.errors[:field_92]).to be_present + expect(parser.errors[:field_93]).not_to be_present + expect(parser.errors[:field_94]).not_to be_present + expect(parser.errors[:field_95]).not_to be_present + 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 + expect(parser.errors[:field_99]).not_to be_present + end + end + + context "when illness but no illnesses answered" do + let(:attributes) { { bulk_upload:, field_89: "1", field_90: nil, field_91: nil, field_92: nil, field_93: nil, field_94: nil, field_95: nil, field_96: nil, field_97: nil, field_98: nil, field_99: nil } } + + it "errors added to correct fields" do + expect(parser.errors[:field_90]).to be_present + expect(parser.errors[:field_91]).to be_present + expect(parser.errors[:field_92]).to be_present + expect(parser.errors[:field_93]).to be_present + expect(parser.errors[:field_94]).to be_present + expect(parser.errors[:field_95]).to be_present + 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(parser.errors[:field_99]).to be_present + end + end + end + + describe "#field_105, field_110 - 15" do + context "when not homeless but reasonable preference for homelessness" do + let(:attributes) { { bulk_upload:, field_105: "1", field_110: "1", field_111: "1" } } + + it "is not permitted" do + expect(parser.errors[:field_111]).to be_present + end + end + + context "when there is a reasonable preference but none is given" do + let(:attributes) { { bulk_upload:, field_110: "1", field_111: nil, field_112: nil, field_113: nil, field_114: nil, field_115: nil } } + + it "is not permitted" do + expect(parser.errors[:field_111]).to be_present + expect(parser.errors[:field_112]).to be_present + expect(parser.errors[:field_113]).to be_present + expect(parser.errors[:field_114]).to be_present + expect(parser.errors[:field_115]).to be_present + end + end + end + describe "#field_119" do # referral context "when 3 ie PRP nominated by LA and owning org is LA" do let(:attributes) { { bulk_upload:, field_119: "3", field_1: owning_org.old_visible_id } } diff --git a/spec/services/bulk_upload/sales/year2022/row_parser_spec.rb b/spec/services/bulk_upload/sales/year2022/row_parser_spec.rb index 9da1fbe79..f14ec9d6c 100644 --- a/spec/services/bulk_upload/sales/year2022/row_parser_spec.rb +++ b/spec/services/bulk_upload/sales/year2022/row_parser_spec.rb @@ -349,6 +349,19 @@ RSpec.describe BulkUpload::Sales::Year2022::RowParser do end end + describe "#field_44 - 7" do # buyers organisations + context "when all nil" do + let(:attributes) { setup_section_params.merge(field_44: nil, field_45: nil, field_46: nil, field_47: nil) } + + it "returns correct errors" do + expect(parser.errors[:field_44]).to be_present + expect(parser.errors[:field_45]).to be_present + expect(parser.errors[:field_46]).to be_present + expect(parser.errors[:field_47]).to be_present + end + end + end + describe "#field_57" do # type of shared ownership scheme context "when an invalid option" do let(:attributes) { setup_section_params.merge(field_57: "100", field_113: "1") } diff --git a/spec/services/bulk_upload/sales/year2023/row_parser_spec.rb b/spec/services/bulk_upload/sales/year2023/row_parser_spec.rb index 545efd0f5..a04b9822d 100644 --- a/spec/services/bulk_upload/sales/year2023/row_parser_spec.rb +++ b/spec/services/bulk_upload/sales/year2023/row_parser_spec.rb @@ -857,6 +857,19 @@ RSpec.describe BulkUpload::Sales::Year2023::RowParser do end end + describe "#field_67 - 70" do # buyers organisations + context "when all nil" do + let(:attributes) { setup_section_params.merge(field_67: nil, field_68: nil, field_69: nil, field_70: nil) } + + it "returns correct errors" do + expect(parser.errors[:field_67]).to be_present + expect(parser.errors[:field_68]).to be_present + expect(parser.errors[:field_69]).to be_present + expect(parser.errors[:field_70]).to be_present + end + end + end + describe "soft validations" do context "when soft validation is triggered" do let(:attributes) { valid_attributes.merge({ field_30: 22, field_35: 5 }) }