From c54cf1134832a81f41e65063e0559e806cf52e7e Mon Sep 17 00:00:00 2001 From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com> Date: Thu, 6 Mar 2025 08:38:55 +0000 Subject: [PATCH] CLDC-3860: Bulk upload add validation, prevent reasonpref dontknow being selected (#2955) * Add validation on reasonpref dontknow * Add test * Update method * Update setting errors * Update test * Add tests for rake task * separate out tests * Test updated at, excluding pre 2024 * Update 2025 row parser * Clear values if rp_dontknow conflict exists * Reorder, no change just precautionary * Put back code removed --- .../lettings/year2024/row_parser.rb | 31 ++++++-- .../lettings/year2025/row_parser.rb | 31 ++++++-- .../lettings/2024/bulk_upload.en.yml | 4 ++ .../lettings/2025/bulk_upload.en.yml | 4 ++ ...calculate_invalid_reasonpref_dontknow.rake | 12 ++++ ...culate_invalid_reasonpref_dontknow_spec.rb | 70 +++++++++++++++++++ .../lettings/year2024/row_parser_spec.rb | 18 ++++- .../lettings/year2025/row_parser_spec.rb | 13 ++++ 8 files changed, 171 insertions(+), 12 deletions(-) create mode 100644 lib/tasks/recalculate_invalid_reasonpref_dontknow.rake create mode 100644 spec/lib/tasks/recalculate_invalid_reasonpref_dontknow_spec.rb diff --git a/app/services/bulk_upload/lettings/year2024/row_parser.rb b/app/services/bulk_upload/lettings/year2024/row_parser.rb index acf5d6467..2b3aae5d5 100644 --- a/app/services/bulk_upload/lettings/year2024/row_parser.rb +++ b/app/services/bulk_upload/lettings/year2024/row_parser.rb @@ -418,6 +418,7 @@ class BulkUpload::Lettings::Year2024::RowParser 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_reasonable_preference_dont_know, 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? } @@ -751,6 +752,15 @@ private end end + def validate_reasonable_preference_dont_know + if rp_dontknow_conflict? + errors.add(:field_111, I18n.t("#{ERROR_BASE_KEY}.reasonpref.conflict.dont_know")) + %i[field_107 field_108 field_109 field_110].each do |field| + errors.add(field, I18n.t("#{ERROR_BASE_KEY}.reasonpref.conflict.other")) if send(field) == 1 + end + end + end + def validate_reasonable_preference_homeless reason_fields = %i[field_107 field_108 field_109 field_110 field_111] if field_106 == 1 && reason_fields.all? { |field| attributes[field.to_s].blank? } @@ -1270,11 +1280,11 @@ private attributes["ppostcode_full"] = ppostcode_full attributes["reasonpref"] = field_106 - attributes["rp_homeless"] = field_107 - attributes["rp_insan_unsat"] = field_108 - attributes["rp_medwel"] = field_109 - attributes["rp_hardship"] = field_110 - attributes["rp_dontknow"] = field_111 + attributes["rp_homeless"] = field_107 unless rp_dontknow_conflict? + attributes["rp_insan_unsat"] = field_108 unless rp_dontknow_conflict? + attributes["rp_medwel"] = field_109 unless rp_dontknow_conflict? + attributes["rp_hardship"] = field_110 unless rp_dontknow_conflict? + attributes["rp_dontknow"] = field_111 unless rp_dontknow_conflict? attributes["cbl"] = cbl attributes["chr"] = chr @@ -1661,4 +1671,15 @@ private def bulk_upload_organisation Organisation.find(bulk_upload.organisation_id) end + + def rp_dontknow_conflict? + other_reason_fields = %i[field_107 field_108 field_109 field_110] + if field_106 == 1 + selected_reasons = other_reason_fields.select { |field| send(field) == 1 } + dont_know_selected = field_111 == 1 + + return true if selected_reasons.any? && dont_know_selected + end + false + end end diff --git a/app/services/bulk_upload/lettings/year2025/row_parser.rb b/app/services/bulk_upload/lettings/year2025/row_parser.rb index 26b2e39f9..522960aa1 100644 --- a/app/services/bulk_upload/lettings/year2025/row_parser.rb +++ b/app/services/bulk_upload/lettings/year2025/row_parser.rb @@ -417,6 +417,7 @@ class BulkUpload::Lettings::Year2025::RowParser 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_reasonable_preference_dont_know, 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? } @@ -750,6 +751,15 @@ private end end + def validate_reasonable_preference_dont_know + if rp_dontknow_conflict? + errors.add(:field_111, I18n.t("#{ERROR_BASE_KEY}.reasonpref.conflict.dont_know")) + %i[field_107 field_108 field_109 field_110].each do |field| + errors.add(field, I18n.t("#{ERROR_BASE_KEY}.reasonpref.conflict.other")) if send(field) == 1 + end + end + end + def validate_reasonable_preference_homeless reason_fields = %i[field_107 field_108 field_109 field_110 field_111] if field_106 == 1 && reason_fields.all? { |field| attributes[field.to_s].blank? } @@ -1268,11 +1278,11 @@ private attributes["ppostcode_full"] = ppostcode_full attributes["reasonpref"] = field_106 - attributes["rp_homeless"] = field_107 - attributes["rp_insan_unsat"] = field_108 - attributes["rp_medwel"] = field_109 - attributes["rp_hardship"] = field_110 - attributes["rp_dontknow"] = field_111 + attributes["rp_homeless"] = field_107 unless rp_dontknow_conflict? + attributes["rp_insan_unsat"] = field_108 unless rp_dontknow_conflict? + attributes["rp_medwel"] = field_109 unless rp_dontknow_conflict? + attributes["rp_hardship"] = field_110 unless rp_dontknow_conflict? + attributes["rp_dontknow"] = field_111 unless rp_dontknow_conflict? attributes["cbl"] = cbl attributes["chr"] = chr @@ -1666,6 +1676,17 @@ private end end + def rp_dontknow_conflict? + other_reason_fields = %i[field_107 field_108 field_109 field_110] + if field_106 == 1 + selected_reasons = other_reason_fields.select { |field| send(field) == 1 } + dont_know_selected = field_111 == 1 + + return true if selected_reasons.any? && dont_know_selected + end + false + end + def referral_type mapping = { 1 => [20, 2, 8], diff --git a/config/locales/validations/lettings/2024/bulk_upload.en.yml b/config/locales/validations/lettings/2024/bulk_upload.en.yml index 18d051b4d..7431b074a 100644 --- a/config/locales/validations/lettings/2024/bulk_upload.en.yml +++ b/config/locales/validations/lettings/2024/bulk_upload.en.yml @@ -58,3 +58,7 @@ en: invalid: "Select a valid nationality." charges: missing_charges: "Please enter the %{sentence_fragment}. If there is no %{sentence_fragment}, please enter '0'." + reasonpref: + conflict: + dont_know: "You cannot select 'Don't know' if any of the other reasonable preference reasons are also selected." + other: "You cannot select this reasonable preference reason as you've also selected 'Don't know' as a reason." diff --git a/config/locales/validations/lettings/2025/bulk_upload.en.yml b/config/locales/validations/lettings/2025/bulk_upload.en.yml index 7acb0726c..28866de21 100644 --- a/config/locales/validations/lettings/2025/bulk_upload.en.yml +++ b/config/locales/validations/lettings/2025/bulk_upload.en.yml @@ -58,3 +58,7 @@ en: invalid: "Select a valid nationality." charges: missing_charges: "Please enter the %{sentence_fragment}. If there is no %{sentence_fragment}, please enter '0'." + reasonpref: + conflict: + dont_know: "You cannot select 'Don't know' if any of the other reasonable preference reasons are also selected." + other: "You cannot select this reasonable preference reason as you've also selected 'Don't know' as a reason." diff --git a/lib/tasks/recalculate_invalid_reasonpref_dontknow.rake b/lib/tasks/recalculate_invalid_reasonpref_dontknow.rake new file mode 100644 index 000000000..14d387662 --- /dev/null +++ b/lib/tasks/recalculate_invalid_reasonpref_dontknow.rake @@ -0,0 +1,12 @@ +desc "Bulk update logs with invalid rp_dontknow values" +task recalculate_invalid_rpdontknow: :environment do + validation_trigger_condition = "rp_dontknow = 1 AND (rp_homeless = 1 OR rp_insan_unsat = 1 OR rp_medwel = 1 OR rp_hardship = 1)" + + LettingsLog.filter_by_year(2024).where(validation_trigger_condition).find_each do |log| + log.rp_dontknow = 0 + + unless log.save + Rails.logger.info "Could not save changes to lettings log #{log.id}" + end + end +end diff --git a/spec/lib/tasks/recalculate_invalid_reasonpref_dontknow_spec.rb b/spec/lib/tasks/recalculate_invalid_reasonpref_dontknow_spec.rb new file mode 100644 index 000000000..3487f3989 --- /dev/null +++ b/spec/lib/tasks/recalculate_invalid_reasonpref_dontknow_spec.rb @@ -0,0 +1,70 @@ +require "rails_helper" +require "rake" + +RSpec.describe "recalculate_invalid_reasonpref_dontknow" do + subject(:task) { Rake::Task["recalculate_invalid_rpdontknow"] } + + before do + Rake.application.rake_require("tasks/recalculate_invalid_reasonpref_dontknow") + Rake::Task.define_task(:environment) + task.reenable + end + + let(:invalid_logs) { create_list(:lettings_log, 5, :completed, reasonpref: 1, rp_dontknow: 1, rp_homeless: 1, rp_insan_unsat: rand(2), rp_medwel: rand(2), rp_hardship: rand(2), updated_at: Time.zone.local(2024, 4, 2, 12, 0, 0)) } + let(:pre_2024_invalid_logs) do + create_list(:lettings_log, 5, :completed, reasonpref: 1, rp_dontknow: 1, rp_homeless: 1, rp_insan_unsat: rand(2), rp_medwel: rand(2), rp_hardship: rand(2)).each do |log| + log.startdate = Time.zone.local(rand(2021..2023), 4, 1) + log.save!(validate: false) + end + end + let(:valid_logs) { create_list(:lettings_log, 3, :completed, reasonpref: 1, rp_dontknow: 0, rp_homeless: 1, rp_insan_unsat: 1, rp_medwel: rand(2), rp_hardship: rand(2), updated_at: Time.zone.local(2024, 4, 2, 12, 0, 0)) } + + it "updates the logs from 2024/25 with invalid rp_dontknow values" do + invalid_logs.each do |log| + expect(log.reasonpref).to eq(1) + expect(log.rp_dontknow).to eq(1) + expect(log.rp_homeless).to eq(1) + end + task.invoke + invalid_logs.each do |log| + log.reload + expect(log.reasonpref).to eq(1) + expect(log.rp_dontknow).to eq(0) + expect(log.rp_homeless).to eq(1) + expect(log.updated_at).not_to eq(Time.zone.local(2024, 4, 2, 12, 0, 0)) + end + end + + it "does not update the logs pre 2024 with invalid rp_dontknow values" do + pre_2024_invalid_logs.each do |log| + expect(log.reasonpref).to eq(1) + expect(log.rp_dontknow).to eq(1) + expect(log.rp_homeless).to eq(1) + end + task.invoke + pre_2024_invalid_logs.each do |log| + log.reload + expect(log.reasonpref).to eq(1) + expect(log.rp_dontknow).to eq(1) + expect(log.rp_homeless).to eq(1) + end + end + + it "does not update the logs with valid rp_dontknow values" do + valid_logs.each do |log| + expect(log.reasonpref).to eq(1) + expect(log.rp_dontknow).to eq(0) + expect(log.rp_homeless).to eq(1) + expect(log.rp_insan_unsat).to eq(1) + end + task.invoke + valid_logs.each do |log| + log.reload + expect(log.reasonpref).to eq(1) + expect(log.rp_dontknow).to eq(0) + expect(log.rp_homeless).to eq(1) + expect(log.rp_insan_unsat).to eq(1) + expect(log.updated_at).to eq(Time.zone.local(2024, 4, 2, 12, 0, 0)) + 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 a3392e50d..e4b34ac36 100644 --- a/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb +++ b/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb @@ -1247,7 +1247,7 @@ RSpec.describe BulkUpload::Lettings::Year2024::RowParser do end end - context "when some reasonable preference options are seleceted" do + context "when some reasonable preference options are selected" do let(:attributes) { setup_section_params.merge({ bulk_upload:, field_106: "1", field_107: "1", field_108: nil, field_109: "1", field_110: nil, field_111: nil }) } it "sets the rest of the options to 0" do @@ -1260,7 +1260,7 @@ RSpec.describe BulkUpload::Lettings::Year2024::RowParser do end end - context "when some reasonable preference options are seleceted but reasonpref is No" do + context "when some reasonable preference options are selected but reasonpref is No" do let(:attributes) { setup_section_params.merge({ bulk_upload:, field_106: "2", field_107: "1", field_108: nil, field_109: "1", field_110: nil, field_111: nil }) } it "sets the options to nil" do @@ -1285,6 +1285,20 @@ RSpec.describe BulkUpload::Lettings::Year2024::RowParser do expect(parser.errors[:field_111]).to be_present end end + + context "when reasonpref is Yes, some reasonable preferences are selected but also so is 'Don't know'" do + let(:attributes) { setup_section_params.merge({ bulk_upload:, field_106: "1", field_107: "1", field_108: "1", field_109: nil, field_110: nil, field_111: "1" }) } + + it "is not permitted" do + parser.valid? + expect(parser.errors[:field_107]).to be_present + expect(parser.errors[:field_108]).to be_present + expect(parser.errors[:field_111]).to be_present + expect(parser.errors[:field_107]).to include(I18n.t("validations.lettings.2024.bulk_upload.reasonpref.conflict.other")) + expect(parser.errors[:field_108]).to include(I18n.t("validations.lettings.2024.bulk_upload.reasonpref.conflict.other")) + expect(parser.errors[:field_111]).to include(I18n.t("validations.lettings.2024.bulk_upload.reasonpref.conflict.dont_know")) + end + end end describe "#field_116" do # referral diff --git a/spec/services/bulk_upload/lettings/year2025/row_parser_spec.rb b/spec/services/bulk_upload/lettings/year2025/row_parser_spec.rb index a822d6f2a..3f0ef35b9 100644 --- a/spec/services/bulk_upload/lettings/year2025/row_parser_spec.rb +++ b/spec/services/bulk_upload/lettings/year2025/row_parser_spec.rb @@ -1096,6 +1096,19 @@ RSpec.describe BulkUpload::Lettings::Year2025::RowParser do expect(parser.errors[:field_111]).to be_present end end + + context "when some reasonable preference options are selected" do + let(:attributes) { setup_section_params.merge({ bulk_upload:, field_106: "1", field_107: "1", field_108: nil, field_109: "1", field_110: nil, field_111: nil }) } + + it "sets the rest of the options to 0" do + parser.valid? + expect(parser.log.rp_homeless).to eq(1) + expect(parser.log.rp_insan_unsat).to eq(0) + expect(parser.log.rp_medwel).to eq(1) + expect(parser.log.rp_hardship).to eq(0) + expect(parser.log.rp_dontknow).to eq(0) + end + end end describe "#field_116" do # referral