CLDC-2226 Update UPRN validations in bulk upload (#1524)

* feat: add validation to check UPRN exists if address doesn't

* test: add tests for UPRN/address fields

* test: make UPRN<=12 chars test more specific

* feat: show qu header on BU error template if check_answer_label missing

* test: that validate_nulls gets header if check_answer_label is missing

* fix: drop "known" from "You must answer UPRN known"

* fix: update uprn validation logic

* test: make UPRN being missing explicit

* test: improve test descriptions

* test: that errors added to address fields too when uprn & address fields all null

* refactor: standardise check for presence of needs type

* test: attempt to stub setup question to have nil check_answer_label

* Revert "test: attempt to stub setup question to have nil check_answer_label"

This reverts commit f5b3f6179b.

* test: put test within context block for clarity

* test: fix typo in test description

* test: ensure one other non-blank field exists in row

* feat: output "this question" if qu has no header or check_answer_label

* chore: lint

* fix: add .presence to question.header and remove & before .downcase

* fix: add :after_log context to uprn validation

app/services/bulk_upload/lettings/year2023/row_parser.rb

@@ -292,6 +292,7 @@ class BulkUpload::Lettings::Year2023::RowParser
   validates :field_72, format: { with: /\A\d{1,3}\z|\AR\z/, message: "Age of person 7 must be a number or the letter R" }, allow_blank: true, on: :after_log
   validates :field_76, format: { with: /\A\d{1,3}\z|\AR\z/, message: "Age of person 8 must be a number or the letter R" }, allow_blank: true, on: :after_log
 
+  validates :field_4, presence: { message: I18n.t("validations.not_answered", question: "needs type") }, on: :after_log
   validates :field_6, presence: { message: I18n.t("validations.not_answered", question: "property renewal") }, on: :after_log
   validates :field_7, presence: { message: I18n.t("validations.not_answered", question: "tenancy start date (day)") }, on: :after_log
   validates :field_8, presence: { message: I18n.t("validations.not_answered", question: "tenancy start date (month)") }, on: :after_log
@@ -299,7 +300,6 @@ class BulkUpload::Lettings::Year2023::RowParser
 
   validates :field_9, format: { with: /\A\d{2}\z/, message: I18n.t("validations.setup.startdate.year_not_two_digits") }, on: :after_log
 
-  validate :validate_needs_type_present, on: :after_log
   validate :validate_data_types, on: :after_log
   validate :validate_nulls, on: :after_log
   validate :validate_relevant_collection_window, on: :after_log
@@ -334,6 +334,8 @@ class BulkUpload::Lettings::Year2023::RowParser
 
   validate :validate_valid_radio_option, on: :before_log
 
+  validate :validate_uprn_exists_if_any_key_adddress_fields_are_blank, on: :after_log
+
   def self.question_for_field(field)
     QUESTIONS[field]
   end
@@ -431,9 +433,9 @@ private
     @created_by ||= User.find_by(email: field_3)
   end
 
-  def validate_needs_type_present
+  def validate_uprn_exists_if_any_key_adddress_fields_are_blank
-    if field_4.blank?
+    if field_18.blank? && (field_19.blank? || field_21.blank?)
-      errors.add(:field_4, I18n.t("validations.not_answered", question: "needs type"))
+      errors.add(:field_18, I18n.t("validations.not_answered", question: "UPRN"))
     end
   end
 
@@ -540,13 +542,15 @@ private
       if setup_question?(question)
         fields.each do |field|
           if errors[field].present?
-            errors.add(field, I18n.t("validations.not_answered", question: question.check_answer_label&.downcase), category: :setup)
+            question_text = question.check_answer_label.presence || question.header.presence || "this question"
+            errors.add(field, I18n.t("validations.not_answered", question: question_text.downcase), category: :setup)
           end
         end
       else
         fields.each do |field|
           unless errors.any? { |e| fields.include?(e.attribute) }
-            errors.add(field, I18n.t("validations.not_answered", question: question.check_answer_label&.downcase))
+            question_text = question.check_answer_label.presence || question.header.presence || "this question"
+            errors.add(field, I18n.t("validations.not_answered", question: question_text.downcase))
           end
         end
       end

spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb

@@ -236,6 +236,18 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do
           expect(questions.map(&:id)).to eql([])
         end
       end
+
+      describe "#validate_nulls" do
+        context "when non-setup questions are null" do
+          let(:attributes) { { bulk_upload:, field_1: "a", field_18: "", field_19: "", field_21: "" } }
+
+          it "fetches the question's check_answer_label if it exists, otherwise it gets the question's header" do
+            parser.valid?
+            expect(parser.errors[:field_19]).to eql(["You must answer q12 - address"])
+            expect(parser.errors[:field_21]).to eql(["You must answer town or city"])
+          end
+        end
+      end
     end
 
     context "when setup section not complete" do
@@ -806,8 +818,50 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do
       context "when over 12 characters" do
         let(:attributes) { { bulk_upload:, field_18: "1234567890123" } }
 
-        it "has errors on the field" do
+        it "adds an appropriate error" do
-          expect(parser.errors[:field_18]).to be_present
+          expect(parser.errors[:field_18]).to eql(["UPRN must be 12 digits or less"])
+        end
+      end
+
+      context "when neither UPRN nor address fields are given" do
+        let(:attributes) do
+          {
+            bulk_upload:,
+            field_1: "1",
+          }
+        end
+
+        it "adds appropriate errors" do
+          expect(parser.errors[:field_18]).to eql(["You must answer UPRN"])
+          expect(parser.errors[:field_19]).to eql(["You must answer q12 - address"])
+          expect(parser.errors[:field_21]).to eql(["You must answer town or city"])
+        end
+      end
+
+      context "when UPRN is given but address fields are not" do
+        let(:attributes) do
+          {
+            bulk_upload:,
+            field_18: "123456789012",
+          }
+        end
+
+        it "doesn't add an error" do
+          expect(parser.errors[:field_18]).to be_empty
+        end
+      end
+
+      context "when address is given but UPRN is not" do
+        let(:attributes) do
+          {
+            bulk_upload:,
+            field_19: "1 Example Rd",
+            field_21: "Example Town/City",
+          }
+        end
+
+        it "doesn't add an error" do
+          expect(parser.errors[:field_18]).to be_empty
         end
       end
     end