CLDC-2357 Correct sales bulk upload UPRN errors (#1626)

* feat: process postcode changes to clear la etc when uprn errors clear values

* refactor: how_fix -> how_to_fix

* feat: explicitly blank uprn

* feat: add uprn to error mapping

* refactor: typos

* refactor: cleanup

* feat: make postcode secretly optional, same as lettings

* test: update

* test: add main tests

* feat: remove unneccessary assignement

* feat: add fix choice error message to sales

* feat: typo fix

* refactor: lint

app/models/log.rb

@@ -163,6 +163,7 @@ class Log < ApplicationRecord
       self.town_or_city = nil
       self.postcode_full = nil
       self.county = nil
+      process_postcode_changes!
     end
   end
 

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

@@ -383,7 +383,7 @@ class BulkUpload::Lettings::Year2023::RowParser
 
   validate :validate_nulls, on: :after_log
 
-  validate :validate_uprn_exists_if_any_key_adddress_fields_are_blank, on: :after_log
+  validate :validate_uprn_exists_if_any_key_address_fields_are_blank, on: :after_log
 
   validate :validate_incomplete_soft_validations, on: :after_log
 
@@ -503,7 +503,7 @@ private
     @created_by ||= User.find_by(email: field_3)
   end
 
-  def validate_uprn_exists_if_any_key_adddress_fields_are_blank
+  def validate_uprn_exists_if_any_key_address_fields_are_blank
     if field_18.blank? && (field_19.blank? || field_21.blank?)
       errors.add(:field_18, I18n.t("validations.not_answered", question: "UPRN"))
     end

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

@@ -408,10 +408,8 @@ class BulkUpload::Sales::Year2023::RowParser
   validate :validate_relevant_collection_window, on: :after_log
   validate :validate_incomplete_soft_validations, on: :after_log
 
-  validate :validate_uprn_exists_if_any_key_adddress_fields_are_blank, on: :after_log
-  validate :validate_address_line_1, on: :after_log
-  validate :validate_town_or_city, on: :after_log
-  validate :validate_postcode, on: :after_log
+  validate :validate_uprn_exists_if_any_key_address_fields_are_blank, on: :after_log
+  validate :validate_address_fields, on: :after_log
   validate :validate_if_log_already_exists, on: :after_log, if: -> { FeatureToggle.bulk_upload_duplicate_log_check_enabled? }
 
   def self.question_for_field(field)
@@ -507,32 +505,22 @@ private
     end
   end
 
-  def validate_uprn_exists_if_any_key_adddress_fields_are_blank
-    if field_19.blank? && (field_20.blank? || field_22.blank? || field_24.blank? || field_25.blank?)
+  def validate_uprn_exists_if_any_key_address_fields_are_blank
+    if field_19.blank? && (field_20.blank? || field_22.blank?)
       errors.add(:field_19, I18n.t("validations.not_answered", question: "UPRN"))
     end
   end
 
-  def validate_address_line_1
-    if field_19.blank? && field_20.blank?
+  def validate_address_fields
+    if field_19.blank? || log.errors.attribute_names.include?(:uprn)
+      if field_20.blank?
         errors.add(:field_20, I18n.t("validations.not_answered", question: "address line 1"))
       end
-  end
 
-  def validate_town_or_city
-    if field_19.blank? && field_22.blank?
+      if field_22.blank?
         errors.add(:field_22, I18n.t("validations.not_answered", question: "town or city"))
       end
     end
-
-  def validate_postcode
-    if field_19.blank? && field_24.blank?
-      errors.add(:field_24, I18n.t("validations.not_answered", question: "part 1 of the property's postcode"))
-    end
-
-    if field_19.blank? && field_25.blank?
-      errors.add(:field_25, I18n.t("validations.not_answered", question: "part 2 of the property's postcode"))
-    end
   end
 
   def shared_ownership?
@@ -784,7 +772,7 @@ private
     attributes["proptype"] = field_17
     attributes["builtype"] = field_18
     attributes["la_known"] = field_26.present? ? 1 : 0
-    attributes["la"] = field_24
+    attributes["la"] = field_26
     attributes["is_la_inferred"] = false
     attributes["pcodenk"] = 0 if postcode_full.present?
     attributes["postcode_full"] = postcode_full

config/locales/en.yml

@@ -73,6 +73,11 @@ en:
             choice:
               blank: Select how you would like to fix these errors
               inclusion: You must select one of the following options for how you would like to fix these errors
+        forms/bulk_upload_sales_resume/fix_choice:
+          attributes:
+            choice:
+              blank: Select how you would like to fix these errors
+              inclusion: You must select one of the following options for how you would like to fix these errors
         forms/bulk_upload_lettings_soft_validations_check/confirm_soft_errors:
           attributes:
             confirm_soft_errors:
@@ -81,11 +86,7 @@ en:
           attributes:
             confirm_soft_errors:
               blank: You must select if there are errors in these fields
-        forms/bulk_upload_sales_resume/fix_choice:
-            attributes:
-              choice:
-                blank: Select how you would like to fix these errors
-                inclusion: You must select one of the following options for how you would like to fix these errors
+
 
   activerecord:
     errors:

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

@@ -194,6 +194,9 @@ RSpec.describe BulkUpload::Sales::Year2023::RowParser do
       stub_request(:get, "https://api.os.uk/search/places/v1/uprn?key=OS_DATA_KEY&uprn=2")
         .to_return(status: 200, body:, headers: {})
 
+      stub_request(:get, "https://api.os.uk/search/places/v1/uprn?key=OS_DATA_KEY&uprn=3")
+        .to_return(status: 404, body:, headers: {})
+
       parser.valid?
     end
 
@@ -613,11 +616,10 @@ RSpec.describe BulkUpload::Sales::Year2023::RowParser do
     [
       { field: :field_20, name: "address line 1" },
       { field: :field_22, name: "town or city" },
-      { field: :field_24, name: "postcode part 1" },
-      { field: :field_25, name: "postcode part 2" },
     ].each do |data|
       describe "##{data[:field]} (#{data[:name]})" do
         context "when UPRN present" do
+          context "when UPRN valid" do
             let(:attributes) { setup_section_params.merge({ field_19: "100023336956", data[:field] => nil }) }
 
             it "can be blank" do
@@ -625,6 +627,15 @@ RSpec.describe BulkUpload::Sales::Year2023::RowParser do
             end
           end
 
+          context "when UPRN invalid" do
+            let(:attributes) { setup_section_params.merge({ field_19: "3", data[:field] => nil }) }
+
+            it "cannot be blank" do
+              expect(parser.errors[data[:field]]).not_to be_blank
+            end
+          end
+        end
+
         context "when UPRN not present" do
           let(:attributes) { setup_section_params.merge({ field_19: nil, data[:field] => nil }) }