From d4df4d7982177d5086e560d9903e48cabc140ad7 Mon Sep 17 00:00:00 2001 From: Arthur Campbell Date: Mon, 19 Jun 2023 17:28:02 +0100 Subject: [PATCH] do not add soft validations to fields related to unrouted questions --- .../lettings/year2022/row_parser.rb | 4 +++- .../lettings/year2023/row_parser.rb | 4 +++- .../bulk_upload/sales/year2022/row_parser.rb | 4 +++- .../bulk_upload/sales/year2023/row_parser.rb | 4 +++- .../lettings/year2022/row_parser_spec.rb | 18 +++++++++++++++--- .../lettings/year2023/row_parser_spec.rb | 16 ++++++++++++++-- .../sales/year2022/row_parser_spec.rb | 15 +++++++++++++-- .../sales/year2023/row_parser_spec.rb | 14 ++++++++++++-- 8 files changed, 66 insertions(+), 13 deletions(-) diff --git a/app/services/bulk_upload/lettings/year2022/row_parser.rb b/app/services/bulk_upload/lettings/year2022/row_parser.rb index c6251c81f..45acffd79 100644 --- a/app/services/bulk_upload/lettings/year2022/row_parser.rb +++ b/app/services/bulk_upload/lettings/year2022/row_parser.rb @@ -793,8 +793,10 @@ private next if question.completed?(log) question.page.interruption_screen_question_ids.each do |interruption_screen_question_id| + next if log.form.questions.none? { |q| q.id == interruption_screen_question_id && q.page.routed_to?(log, nil) } + field_mapping_for_errors[interruption_screen_question_id.to_sym]&.each do |field| - unless errors.any? { |e| e.options[:category] == :soft_validation && field_mapping_for_errors[interruption_screen_question_id.to_sym].include?(e.attribute) } + if errors.none? { |e| e.options[:category] == :soft_validation && field_mapping_for_errors[interruption_screen_question_id.to_sym].include?(e.attribute) } error_message = [display_title_text(question.page.title_text, log), display_informative_text(question.page.informative_text, log)].reject(&:empty?).join(". ") errors.add(field, message: error_message, category: :soft_validation) end diff --git a/app/services/bulk_upload/lettings/year2023/row_parser.rb b/app/services/bulk_upload/lettings/year2023/row_parser.rb index 9c3c76fab..5dc2778bf 100644 --- a/app/services/bulk_upload/lettings/year2023/row_parser.rb +++ b/app/services/bulk_upload/lettings/year2023/row_parser.rb @@ -538,8 +538,10 @@ private next if question.completed?(log) question.page.interruption_screen_question_ids.each do |interruption_screen_question_id| + next if log.form.questions.none? { |q| q.id == interruption_screen_question_id && q.page.routed_to?(log, nil) } + field_mapping_for_errors[interruption_screen_question_id.to_sym]&.each do |field| - unless errors.any? { |e| field_mapping_for_errors[interruption_screen_question_id.to_sym].include?(e.attribute) } + if errors.none? { |e| field_mapping_for_errors[interruption_screen_question_id.to_sym].include?(e.attribute) } error_message = [display_title_text(question.page.title_text, log), display_informative_text(question.page.informative_text, log)].reject(&:empty?).join(". ") errors.add(field, message: error_message, category: :soft_validation) end diff --git a/app/services/bulk_upload/sales/year2022/row_parser.rb b/app/services/bulk_upload/sales/year2022/row_parser.rb index 1ac6258fd..ea7da4563 100644 --- a/app/services/bulk_upload/sales/year2022/row_parser.rb +++ b/app/services/bulk_upload/sales/year2022/row_parser.rb @@ -1110,8 +1110,10 @@ private next if question.completed?(log) question.page.interruption_screen_question_ids.each do |interruption_screen_question_id| + next if log.form.questions.none? { |q| q.id == interruption_screen_question_id && q.page.routed_to?(log, nil) } + field_mapping_for_errors[interruption_screen_question_id.to_sym]&.each do |field| - unless errors.any? { |e| e.options[:category] == :soft_validation && field_mapping_for_errors[interruption_screen_question_id.to_sym].include?(e.attribute) } + if errors.none? { |e| e.options[:category] == :soft_validation && field_mapping_for_errors[interruption_screen_question_id.to_sym].include?(e.attribute) } error_message = [display_title_text(question.page.title_text, log), display_informative_text(question.page.informative_text, log)].reject(&:empty?).join(". ") errors.add(field, message: error_message, category: :soft_validation) end diff --git a/app/services/bulk_upload/sales/year2023/row_parser.rb b/app/services/bulk_upload/sales/year2023/row_parser.rb index 17ec939d4..d7087d121 100644 --- a/app/services/bulk_upload/sales/year2023/row_parser.rb +++ b/app/services/bulk_upload/sales/year2023/row_parser.rb @@ -1285,8 +1285,10 @@ private next if question.completed?(log) question.page.interruption_screen_question_ids.each do |interruption_screen_question_id| + next if log.form.questions.none? { |q| q.id == interruption_screen_question_id && q.page.routed_to?(log, nil) } + field_mapping_for_errors[interruption_screen_question_id.to_sym]&.each do |field| - unless errors.any? { |e| e.options[:category] == :soft_validation && field_mapping_for_errors[interruption_screen_question_id.to_sym].include?(e.attribute) } + if errors.none? { |e| e.options[:category] == :soft_validation && field_mapping_for_errors[interruption_screen_question_id.to_sym].include?(e.attribute) } error_message = [display_title_text(question.page.title_text, log), display_informative_text(question.page.informative_text, log)].reject(&:empty?).join(". ") errors.add(field, message: error_message, category: :soft_validation) end 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..44fd38a79 100644 --- a/spec/services/bulk_upload/lettings/year2022/row_parser_spec.rb +++ b/spec/services/bulk_upload/lettings/year2022/row_parser_spec.rb @@ -990,11 +990,23 @@ RSpec.describe BulkUpload::Lettings::Year2022::RowParser do end end + context "when a soft validation is triggered that relates both to fields that are and are not routed to" do + let(:attributes) { setup_section_params.merge({ field_47: "1", field_20: "M", field_21: "M" }) } + + it "adds errors to fields that are routed to" do + expect(parser.errors.where(:field_20, category: :soft_validation)).to be_present + expect(parser.errors.where(:field_21, category: :soft_validation)).to be_present + end + + it "does not add errors to fields that are not routed to" do + expect(parser.errors.where(:field_22, category: :soft_validation)).not_to be_present + expect(parser.errors.where(:field_23, category: :soft_validation)).not_to be_present + end + end + context "when soft validation is triggered and the mappings for errors are not defined" do let(:attributes) { setup_section_params.merge({ field_12: 22, field_35: 5 }) } - # rubocop:disable RSpec/AnyInstance - let(:mock_interruption_ids) { allow_any_instance_of(Form::Page).to receive(:interruption_screen_question_ids).and_return(%w[fake_question_id]) } - # rubocop:enable RSpec/AnyInstance + let(:mock_interruption_ids) { allow_any_instance_of(Form::Page).to receive(:interruption_screen_question_ids).and_return(%w[fake_question_id]) } # rubocop:disable RSpec/AnyInstance it "does not crash" do expect(parser.errors).to be_present 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..1bce7d138 100644 --- a/spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb +++ b/spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb @@ -49,8 +49,6 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do FormHandler.instance.use_real_forms! example.run - - FormHandler.instance.use_fake_forms! end describe "#blank_row?" do @@ -1019,6 +1017,20 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do end end + context "when a soft validation is triggered that relates both to fields that are and are not routed to" do + let(:attributes) { setup_section_params.merge({ field_82: "1", field_47: "M", field_53: "M", field_57: "M" }) } + + it "adds errors to fields that are routed to" do + expect(parser.errors.where(:field_53, category: :soft_validation)).to be_present + expect(parser.errors.where(:field_57, category: :soft_validation)).to be_present + end + + it "does not add errors to fields that are not routed to" do + expect(parser.errors.where(:field_61, category: :soft_validation)).not_to be_present + expect(parser.errors.where(:field_65, category: :soft_validation)).not_to be_present + end + end + context "when soft validation is triggered and not required" do let(:attributes) { setup_section_params.merge({ field_128: 120, field_126: 1, field_32: 1, field_4: 1, field_5: "3", field_25: "E09000008" }) } 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..484449860 100644 --- a/spec/services/bulk_upload/sales/year2022/row_parser_spec.rb +++ b/spec/services/bulk_upload/sales/year2022/row_parser_spec.rb @@ -112,8 +112,6 @@ RSpec.describe BulkUpload::Sales::Year2022::RowParser do Singleton.__init__(FormHandler) example.run end - Timecop.return - Singleton.__init__(FormHandler) end describe "#blank_row?" do @@ -723,6 +721,19 @@ RSpec.describe BulkUpload::Sales::Year2022::RowParser do expect(parser.errors.where(:field_24, category: :soft_validation).first.message).to eql("You told us this person is aged 22 years and retired.") end end + + context "when a soft validation is triggered that relates both to fields that are and are not routed to" do + let(:attributes) { valid_attributes.merge({ field_123: "2" }) } + + it "adds errors to fields that are routed to" do + expect(parser.errors.where(:field_123, category: :soft_validation)).to be_present + end + + it "does not add errors to fields that are not routed to" do + expect(parser.errors.where(:field_73, category: :soft_validation)).not_to be_present + expect(parser.errors.where(:field_70, category: :soft_validation)).not_to be_present + end + end end context "when the log already exists in the db" do 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..1ed532e44 100644 --- a/spec/services/bulk_upload/sales/year2023/row_parser_spec.rb +++ b/spec/services/bulk_upload/sales/year2023/row_parser_spec.rb @@ -115,8 +115,6 @@ RSpec.describe BulkUpload::Sales::Year2023::RowParser do Singleton.__init__(FormHandler) example.run end - Timecop.return - Singleton.__init__(FormHandler) end describe "#blank_row?" do @@ -871,6 +869,18 @@ RSpec.describe BulkUpload::Sales::Year2023::RowParser do expect(parser.errors.where(:field_30, category: :soft_validation).first.message).to eql("You told us this person is aged 22 years and retired.") end end + + context "when a soft validation is triggered that relates both to fields that are and are not routed to" do + let(:attributes) { valid_attributes.merge({ field_103: "300000" }) } + + it "adds errors to fields that are routed to" do + expect(parser.errors.where(:field_103, category: :soft_validation)).to be_present + end + + it "does not add errors to fields that are not routed to" do + expect(parser.errors.where(:field_112, category: :soft_validation)).not_to be_present + end + end end end