Refactor sale import validations to be consistent with lettings

app/services/imports/sales_logs_import_service.rb

@@ -217,44 +217,38 @@ module Imports
           attributes.delete("ppcodenk") if error.attribute == :ppostcode_full
         end
         @logs_overridden << sales_log.old_id
-        save_sales_log(attributes, previous_status)
+        return save_sales_log(attributes, previous_status)
-      elsif sales_log.errors.of_kind?(:postcode_full, :postcodes_not_matching)
+      end
-        @logger.warn("Log #{sales_log.old_id}: Removing previous postcode known and previous postcode as the postcode is invalid")
+
-        @logs_overridden << sales_log.old_id
+      errors = {
-        attributes.delete("ppcodenk")
+        %i[postcode_full postcodes_not_matching] => %w[ppcodenk ppostcode_full],
-        attributes.delete("ppostcode_full")
+        %i[exdate over_a_year_from_saledate] => %w[exdate],
-        save_sales_log(attributes, previous_status)
+        %i[income1 over_hard_max_for_outside_london] => %w[income1],
-      elsif sales_log.errors.of_kind?(:exdate, :over_a_year_from_saledate)
+        %i[income1 over_hard_max_for_london] => %w[income1],
-        @logger.warn("Log #{sales_log.old_id}: Removing exchange date as the exchange date is invalid")
+        %i[income2 over_hard_max_for_london] => %w[income2],
-        @logs_overridden << sales_log.old_id
+        %i[equity over_max] => %w[equity],
-        attributes.delete("exdate")
+        %i[equity under_min] => %w[equity],
-        save_sales_log(attributes, previous_status)
+        %i[mortgage cannot_be_0] => %w[mortgage],
-      elsif sales_log.errors.of_kind?(:income1, :over_hard_max_for_outside_london) || sales_log.errors.of_kind?(:income1, :over_hard_max_for_london)
+      }
-        @logger.warn("Log #{sales_log.old_id}: Removing income1 as the income1 is invalid")
+
-        @logs_overridden << sales_log.old_id
+      errors.each do |(error, fields)|
-        attributes.delete("income1")
+        next unless sales_log.errors.of_kind?(*error)
-        save_sales_log(attributes, previous_status)
+
-      elsif sales_log.errors.of_kind?(:income2, :over_hard_max_for_london)
+        attribute, _type = error
-        @logger.warn("Log #{sales_log.old_id}: Removing income2 as the income2 is invalid")
+        fields.each do |field|
-        @logs_overridden << sales_log.old_id
+          @logger.warn("Log #{sales_log.old_id}: Removing #{field} with error: #{sales_log.errors[attribute].sort.join(', ')}")
-        attributes.delete("income2")
+          attributes.delete(field)
-        save_sales_log(attributes, previous_status)
+        end
-      elsif sales_log.errors.of_kind?(:equity, :over_max) || sales_log.errors.of_kind?(:equity, :under_min)
-        @logger.warn("Log #{sales_log.old_id}: Removing equity as the equity is invalid")
         @logs_overridden << sales_log.old_id
-        attributes.delete("equity")
+        return save_sales_log(attributes, previous_status)
-        save_sales_log(attributes, previous_status)
+      end
-      elsif sales_log.errors.of_kind?(:postcode_full, :wrong_format)
+
+      if sales_log.errors.of_kind?(:postcode_full, :wrong_format)
         @logger.warn("Log #{sales_log.old_id}: Removing postcode as the postcode is invalid")
         @logs_overridden << sales_log.old_id
         attributes.delete("postcode_full")
         attributes["pcodenk"] = attributes["la"].present? ? 1 : nil
         save_sales_log(attributes, previous_status)
-      elsif sales_log.errors.of_kind?(:mortgage, :cannot_be_0)
-        @logger.warn("Log #{sales_log.old_id}: Removing mortgage because it cannot be 0")
-        @logs_overridden << sales_log.old_id
-        attributes.delete("mortgage")
-        save_sales_log(attributes, previous_status)
       elsif sales_log.errors.of_kind?(:uprn, :uprn_error)
         @logger.warn("Log #{sales_log.old_id}: Setting uprn_known to no with error: #{sales_log.errors[:uprn].join(', ')}")
         @logs_overridden << sales_log.old_id

spec/services/imports/sales_logs_import_service_spec.rb

@@ -528,7 +528,7 @@ RSpec.describe Imports::SalesLogsImportService do
       end
 
       it "intercepts the relevant validation error" do
-        expect(logger).to receive(:warn).with(/Log shared_ownership_sales_log: Removing equity as the equity is invalid/)
+        expect(logger).to receive(:warn).with(/Removing equity with error: The maximum initial equity stake is 75%/)
         expect { sales_log_service.send(:create_log, sales_log_xml) }
           .not_to raise_error
       end
@@ -552,7 +552,7 @@ RSpec.describe Imports::SalesLogsImportService do
       end
 
       it "intercepts the relevant validation error" do
-        expect(logger).to receive(:warn).with(/Log shared_ownership_sales_log: Removing equity as the equity is invalid/)
+        expect(logger).to receive(:warn).with(/Removing equity with error: The minimum initial equity stake for this type of shared ownership sale is 25%/)
         expect { sales_log_service.send(:create_log, sales_log_xml) }
           .not_to raise_error
       end
@@ -631,7 +631,8 @@ RSpec.describe Imports::SalesLogsImportService do
       end
 
       it "intercepts the relevant validation error" do
-        expect(logger).to receive(:warn).with(/Removing previous postcode known and previous postcode as the postcode is invalid/)
+        expect(logger).to receive(:warn).with(/Removing ppcodenk with error: Buyer's last accommodation and discounted ownership postcodes must match, Last settled accommodation and discounted ownership property postcodes must match/)
+        expect(logger).to receive(:warn).with(/Removing ppostcode_full with error: Buyer's last accommodation and discounted ownership postcodes must match, Last settled accommodation and discounted ownership property postcodes must match/)
         expect { sales_log_service.send(:create_log, sales_log_xml) }
           .not_to raise_error
       end
@@ -701,7 +702,7 @@ RSpec.describe Imports::SalesLogsImportService do
       end
 
       it "intercepts the relevant validation error" do
-        expect(logger).to receive(:warn).with(/Removing exchange date as the exchange date is invalid/)
+        expect(logger).to receive(:warn).with(/Removing exdate with error: Contract exchange date must be less than 1 year before sale completion date/)
         expect { sales_log_service.send(:create_log, sales_log_xml) }
           .not_to raise_error
       end
@@ -726,7 +727,7 @@ RSpec.describe Imports::SalesLogsImportService do
       end
 
       it "intercepts the relevant validation error" do
-        expect(logger).to receive(:warn).with(/Removing mortgage because it cannot be 0/)
+        expect(logger).to receive(:warn).with(/Removing mortgage with error: Mortgage amount must be at least £1, Mortgage value cannot be £0 if a mortgage was used for the purchase of this property/)
         expect { sales_log_service.send(:create_log, sales_log_xml) }
           .not_to raise_error
       end
@@ -751,7 +752,7 @@ RSpec.describe Imports::SalesLogsImportService do
       end
 
       it "intercepts the relevant validation error" do
-        expect(logger).to receive(:warn).with(/Removing income1 as the income1 is invalid/)
+        expect(logger).to receive(:warn).with(/Removing income1 with error: Income must be £80,000 or lower for properties outside London local authority/)
         expect { sales_log_service.send(:create_log, sales_log_xml) }
           .not_to raise_error
       end
@@ -776,7 +777,7 @@ RSpec.describe Imports::SalesLogsImportService do
       end
 
       it "intercepts the relevant validation error" do
-        expect(logger).to receive(:warn).with(/Removing income1 as the income1 is invalid/)
+        expect(logger).to receive(:warn).with(/Removing income1 with error: Income must be £90,000 or lower for properties within a London local authority/)
         expect { sales_log_service.send(:create_log, sales_log_xml) }
           .not_to raise_error
       end
@@ -801,7 +802,7 @@ RSpec.describe Imports::SalesLogsImportService do
       end
 
       it "intercepts the relevant validation error" do
-        expect(logger).to receive(:warn).with(/Removing income2 as the income2 is invalid/)
+        expect(logger).to receive(:warn).with(/Removing income2 with error: Combined income must be £90,000 or lower for properties within a London local authority, Income must be £90,000 or lower for properties within a London local authority/)
         expect { sales_log_service.send(:create_log, sales_log_xml) }
           .not_to raise_error
       end