From 2f5275185ee520f3f4a673484f755a9913ae51a6 Mon Sep 17 00:00:00 2001
From: Rachael Booth <rachael.booth@softwire.com>
Date: Wed, 9 Oct 2024 17:17:51 +0100
Subject: [PATCH] CLDC-3413: Make BU validate_nulls account for errors on
 before_log when choosing whether to add errors

---
 .../bulk_upload/lettings/year2024/row_parser.rb        |  8 ++++----
 app/services/bulk_upload/sales/year2024/row_parser.rb  |  8 ++++----
 .../bulk_upload/lettings/year2024/row_parser_spec.rb   | 10 ++++++++++
 .../bulk_upload/sales/year2024/row_parser_spec.rb      | 10 ++++++++++
 4 files changed, 28 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 3a18ac633..5002deac1 100644
--- a/app/services/bulk_upload/lettings/year2024/row_parser.rb
+++ b/app/services/bulk_upload/lettings/year2024/row_parser.rb
@@ -456,12 +456,12 @@ class BulkUpload::Lettings::Year2024::RowParser
     return @valid = true if blank_row?
 
     super(:before_log)
-    before_errors = errors.dup
+    @before_errors = errors.dup
 
     log.valid?
 
     super(:after_log)
-    errors.merge!(before_errors)
+    errors.merge!(@before_errors)
 
     log.errors.each do |error|
       fields = field_mapping_for_errors[error.attribute] || []
@@ -815,13 +815,13 @@ private
 
       if setup_question?(question)
         fields.each do |field|
-          if errors.select { |e| fields.include?(e.attribute) }.none? && field.present?
+          if field.present? && errors.none? { |e| fields.include?(e.attribute) } && @before_errors.none? { |e| fields.include?(e.attribute) }
             errors.add(field, question.unanswered_error_message, category: :setup)
           end
         end
       else
         fields.each do |field|
-          unless errors.any? { |e| fields.include?(e.attribute) }
+          if errors.none? { |e| fields.include?(e.attribute) } && @before_errors.none? { |e| fields.include?(e.attribute) }
             errors.add(field, question.unanswered_error_message)
           end
         end
diff --git a/app/services/bulk_upload/sales/year2024/row_parser.rb b/app/services/bulk_upload/sales/year2024/row_parser.rb
index df1a06528..d024e9c86 100644
--- a/app/services/bulk_upload/sales/year2024/row_parser.rb
+++ b/app/services/bulk_upload/sales/year2024/row_parser.rb
@@ -504,12 +504,12 @@ class BulkUpload::Sales::Year2024::RowParser
     return true if blank_row?
 
     super(:before_log)
-    before_errors = errors.dup
+    @before_errors = errors.dup
 
     log.valid?
 
     super(:after_log)
-    errors.merge!(before_errors)
+    errors.merge!(@before_errors)
 
     log.errors.each do |error|
       fields = field_mapping_for_errors[error.attribute] || []
@@ -1377,13 +1377,13 @@ private
 
       if setup_question?(question)
         fields.each do |field|
-          unless errors.any? { |e| fields.include?(e.attribute) }
+          if errors.none? { |e| fields.include?(e.attribute) } && @before_errors.none? { |e| fields.include?(e.attribute) }
             errors.add(field, question.unanswered_error_message, category: :setup)
           end
         end
       else
         fields.each do |field|
-          unless errors.any? { |e| fields.include?(e.attribute) }
+          if errors.none? { |e| fields.include?(e.attribute) } && @before_errors.none? { |e| fields.include?(e.attribute) }
             errors.add(field, question.unanswered_error_message)
           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 3910e1281..8e2fe6097 100644
--- a/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb
+++ b/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb
@@ -782,6 +782,16 @@ RSpec.describe BulkUpload::Lettings::Year2024::RowParser do
             expect(parser.errors[:field_112]).to eql(["You must answer was the letting made under the Choice-Based Lettings (CBL)?"])
           end
         end
+
+        context "when an invalid value error has been added" do
+          let(:attributes) { setup_section_params.merge({ field_116: "100" }) }
+
+          it "does not add an additional error" do
+            parser.valid?
+            expect(parser.errors[:field_116].length).to eq(1)
+            expect(parser.errors[:field_116]).to include(match "Enter a valid value for")
+          end
+        end
       end
     end
 
diff --git a/spec/services/bulk_upload/sales/year2024/row_parser_spec.rb b/spec/services/bulk_upload/sales/year2024/row_parser_spec.rb
index b74545a9a..f68ec9e9a 100644
--- a/spec/services/bulk_upload/sales/year2024/row_parser_spec.rb
+++ b/spec/services/bulk_upload/sales/year2024/row_parser_spec.rb
@@ -313,6 +313,16 @@ RSpec.describe BulkUpload::Sales::Year2024::RowParser do
             expect(parser.errors[:field_23]).to eql(["You must answer address line 1"])
           end
         end
+
+        context "when an invalid value error has been added" do
+          let(:attributes) { setup_section_params.merge({ field_35: "100" }) }
+
+          it "does not add an additional error" do
+            parser.valid?
+            expect(parser.errors[:field_35].length).to eq(1)
+            expect(parser.errors[:field_35]).to include(match "Enter a valid value for")
+          end
+        end
       end
     end