From e52e639faff5c383a85a9a96bd83198e55ad5442 Mon Sep 17 00:00:00 2001 From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com> Date: Wed, 16 Apr 2025 18:47:23 +0100 Subject: [PATCH 1/3] Add validation message for missing charge fields in bulk upload --- app/services/bulk_upload/lettings/year2024/row_parser.rb | 6 ++---- app/services/bulk_upload/lettings/year2025/row_parser.rb | 6 ++---- config/locales/validations/lettings/2024/bulk_upload.en.yml | 1 + config/locales/validations/lettings/2025/bulk_upload.en.yml | 1 + 4 files changed, 6 insertions(+), 8 deletions(-) diff --git a/app/services/bulk_upload/lettings/year2024/row_parser.rb b/app/services/bulk_upload/lettings/year2024/row_parser.rb index 3d36e743e..5016ebc8b 100644 --- a/app/services/bulk_upload/lettings/year2024/row_parser.rb +++ b/app/services/bulk_upload/lettings/year2024/row_parser.rb @@ -937,10 +937,8 @@ private errors.add(field, I18n.t("#{ERROR_BASE_KEY}.charges.missing_charges", sentence_fragment: charge)) end - other_charge_fields.each do |field, _charge| - blank_charge_fields.each do |_blank_field, blank_charge| - errors.add(field, I18n.t("#{ERROR_BASE_KEY}.charges.missing_charges", sentence_fragment: blank_charge)) - end + other_charge_fields.each_key do |field| + errors.add(field, I18n.t("#{ERROR_BASE_KEY}.charges.related_to_missing_charge")) 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 485446f0b..89a8cca38 100644 --- a/app/services/bulk_upload/lettings/year2025/row_parser.rb +++ b/app/services/bulk_upload/lettings/year2025/row_parser.rb @@ -936,10 +936,8 @@ private errors.add(field, I18n.t("#{ERROR_BASE_KEY}.charges.missing_charges", sentence_fragment: charge)) end - other_charge_fields.each do |field, _charge| - blank_charge_fields.each do |_blank_field, blank_charge| - errors.add(field, I18n.t("#{ERROR_BASE_KEY}.charges.missing_charges", sentence_fragment: blank_charge)) - end + other_charge_fields.each_key do |field| + errors.add(field, I18n.t("#{ERROR_BASE_KEY}.charges.related_to_missing_charge")) end end diff --git a/config/locales/validations/lettings/2024/bulk_upload.en.yml b/config/locales/validations/lettings/2024/bulk_upload.en.yml index 7431b074a..0413972f7 100644 --- a/config/locales/validations/lettings/2024/bulk_upload.en.yml +++ b/config/locales/validations/lettings/2024/bulk_upload.en.yml @@ -58,6 +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'." + related_to_missing_charge: "You must enter all these fields: basic rent, service charge, personal service charge and support charge. Please enter ‘0' for any types of charges that you don’t issue." reasonpref: conflict: dont_know: "You cannot select 'Don't know' if any of the other reasonable preference reasons are also selected." diff --git a/config/locales/validations/lettings/2025/bulk_upload.en.yml b/config/locales/validations/lettings/2025/bulk_upload.en.yml index 28866de21..500dbdaa9 100644 --- a/config/locales/validations/lettings/2025/bulk_upload.en.yml +++ b/config/locales/validations/lettings/2025/bulk_upload.en.yml @@ -58,6 +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'." + related_to_missing_charge: "You must enter all these fields: basic rent, service charge, personal service charge and support charge. Please enter ‘0' for any types of charges that you don’t issue." reasonpref: conflict: dont_know: "You cannot select 'Don't know' if any of the other reasonable preference reasons are also selected." From f190716e83388aaba3d2ef4d82ec3ea021dd852b Mon Sep 17 00:00:00 2001 From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com> Date: Thu, 17 Apr 2025 12:00:28 +0100 Subject: [PATCH 2/3] Update error messages for missing charges in row parser tests --- .../bulk_upload/lettings/year2024/row_parser_spec.rb | 8 ++++---- .../bulk_upload/lettings/year2025/row_parser_spec.rb | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) 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 1e3657dbe..091ac68d9 100644 --- a/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb +++ b/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb @@ -2766,7 +2766,7 @@ RSpec.describe BulkUpload::Lettings::Year2024::RowParser do expect(parser.errors[:field_125]).to eql([I18n.t("validations.lettings.2024.bulk_upload.charges.missing_charges", sentence_fragment: "basic rent")]) expect(parser.errors[:field_126]).to eql([I18n.t("validations.lettings.2024.bulk_upload.charges.missing_charges", sentence_fragment: "service charge")]) expect(parser.errors[:field_127]).to eql([I18n.t("validations.lettings.2024.bulk_upload.charges.missing_charges", sentence_fragment: "personal service charge")]) - expect(parser.errors[:field_128]).to eql([I18n.t("validations.lettings.2024.bulk_upload.charges.missing_charges", sentence_fragment: "basic rent"), I18n.t("validations.lettings.2024.bulk_upload.charges.missing_charges", sentence_fragment: "service charge"), I18n.t("validations.lettings.2024.bulk_upload.charges.missing_charges", sentence_fragment: "personal service charge")]) + expect(parser.errors[:field_128]).to eql([I18n.t("validations.lettings.2024.bulk_upload.charges.related_to_missing_charge")]) end end end @@ -2787,9 +2787,9 @@ RSpec.describe BulkUpload::Lettings::Year2024::RowParser do it "adds an error to all charges" do parser.valid? - expect(parser.errors[:field_125]).to eql([I18n.t("validations.lettings.2024.bulk_upload.charges.missing_charges", sentence_fragment: "support charge")]) - expect(parser.errors[:field_126]).to eql([I18n.t("validations.lettings.2024.bulk_upload.charges.missing_charges", sentence_fragment: "support charge")]) - expect(parser.errors[:field_127]).to eql([I18n.t("validations.lettings.2024.bulk_upload.charges.missing_charges", sentence_fragment: "support charge")]) + expect(parser.errors[:field_125]).to eql([I18n.t("validations.lettings.2024.bulk_upload.charges.related_to_missing_charge")]) + expect(parser.errors[:field_126]).to eql([I18n.t("validations.lettings.2024.bulk_upload.charges.related_to_missing_charge")]) + expect(parser.errors[:field_127]).to eql([I18n.t("validations.lettings.2024.bulk_upload.charges.related_to_missing_charge")]) expect(parser.errors[:field_128]).to eql([I18n.t("validations.lettings.2024.bulk_upload.charges.missing_charges", sentence_fragment: "support charge")]) end end 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 902985943..84f6bb2a7 100644 --- a/spec/services/bulk_upload/lettings/year2025/row_parser_spec.rb +++ b/spec/services/bulk_upload/lettings/year2025/row_parser_spec.rb @@ -2496,7 +2496,7 @@ RSpec.describe BulkUpload::Lettings::Year2025::RowParser do expect(parser.errors[:field_124]).to eql([I18n.t("validations.lettings.2025.bulk_upload.charges.missing_charges", sentence_fragment: "basic rent")]) expect(parser.errors[:field_125]).to eql([I18n.t("validations.lettings.2025.bulk_upload.charges.missing_charges", sentence_fragment: "service charge")]) expect(parser.errors[:field_126]).to eql([I18n.t("validations.lettings.2025.bulk_upload.charges.missing_charges", sentence_fragment: "personal service charge")]) - expect(parser.errors[:field_127]).to eql([I18n.t("validations.lettings.2025.bulk_upload.charges.missing_charges", sentence_fragment: "basic rent"), I18n.t("validations.lettings.2025.bulk_upload.charges.missing_charges", sentence_fragment: "service charge"), I18n.t("validations.lettings.2025.bulk_upload.charges.missing_charges", sentence_fragment: "personal service charge")]) + expect(parser.errors[:field_127]).to eql([I18n.t("validations.lettings.2025.bulk_upload.charges.related_to_missing_charge")]) end end @@ -2515,9 +2515,9 @@ RSpec.describe BulkUpload::Lettings::Year2025::RowParser do it "adds an error to all charges" do parser.valid? - expect(parser.errors[:field_124]).to eql([I18n.t("validations.lettings.2025.bulk_upload.charges.missing_charges", sentence_fragment: "support charge")]) - expect(parser.errors[:field_125]).to eql([I18n.t("validations.lettings.2025.bulk_upload.charges.missing_charges", sentence_fragment: "support charge")]) - expect(parser.errors[:field_126]).to eql([I18n.t("validations.lettings.2025.bulk_upload.charges.missing_charges", sentence_fragment: "support charge")]) + expect(parser.errors[:field_124]).to eql([I18n.t("validations.lettings.2025.bulk_upload.charges.related_to_missing_charge")]) + expect(parser.errors[:field_125]).to eql([I18n.t("validations.lettings.2025.bulk_upload.charges.related_to_missing_charge")]) + expect(parser.errors[:field_126]).to eql([I18n.t("validations.lettings.2025.bulk_upload.charges.related_to_missing_charge")]) expect(parser.errors[:field_127]).to eql([I18n.t("validations.lettings.2025.bulk_upload.charges.missing_charges", sentence_fragment: "support charge")]) end end From e41d69feeb3150c484ecfe48bbbc659ecc5a173d Mon Sep 17 00:00:00 2001 From: Samuel Date: Wed, 30 Apr 2025 17:14:16 +0100 Subject: [PATCH 3/3] only annotate errors to the other fields if there was an error --- app/services/bulk_upload/lettings/year2024/row_parser.rb | 7 +++++-- app/services/bulk_upload/lettings/year2025/row_parser.rb | 7 +++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/app/services/bulk_upload/lettings/year2024/row_parser.rb b/app/services/bulk_upload/lettings/year2024/row_parser.rb index 101352f0a..064aa3f26 100644 --- a/app/services/bulk_upload/lettings/year2024/row_parser.rb +++ b/app/services/bulk_upload/lettings/year2024/row_parser.rb @@ -945,8 +945,11 @@ private errors.add(field, I18n.t("#{ERROR_BASE_KEY}.charges.missing_charges", sentence_fragment: charge)) end - other_charge_fields.each_key do |field| - errors.add(field, I18n.t("#{ERROR_BASE_KEY}.charges.related_to_missing_charge")) + # if there were any errors, add an error to all the remaining present charges to make this entire section invalid + if blank_charge_fields.any? + other_charge_fields.each_key do |field| + errors.add(field, I18n.t("#{ERROR_BASE_KEY}.charges.related_to_missing_charge")) + end 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 61351764b..0745056d2 100644 --- a/app/services/bulk_upload/lettings/year2025/row_parser.rb +++ b/app/services/bulk_upload/lettings/year2025/row_parser.rb @@ -944,8 +944,11 @@ private errors.add(field, I18n.t("#{ERROR_BASE_KEY}.charges.missing_charges", sentence_fragment: charge)) end - other_charge_fields.each_key do |field| - errors.add(field, I18n.t("#{ERROR_BASE_KEY}.charges.related_to_missing_charge")) + # if there were any errors, add an error to all the remaining present charges to make this entire section invalid + if blank_charge_fields.any? + other_charge_fields.each_key do |field| + errors.add(field, I18n.t("#{ERROR_BASE_KEY}.charges.related_to_missing_charge")) + end end end