Extract shared methods into logs import service and lint

app/services/imports/lettings_logs_import_service.rb

@@ -1,5 +1,5 @@
 module Imports
-  class LettingsLogsImportService < ImportService
+  class LettingsLogsImportService < LogsImportService
     def initialize(storage_service, logger = Rails.logger)
       @logs_with_discrepancies = Set.new
       @logs_overridden = Set.new
@@ -318,43 +318,6 @@ module Imports
       end
     end
 
-    # Safe: A string that represents only an integer (or empty/nil)
-    def safe_string_as_integer(xml_doc, attribute)
-      str = field_value(xml_doc, "xmlns", attribute)
-      Integer(str, exception: false)
-    end
-
-    # Safe: A string that represents only a decimal (or empty/nil)
-    def safe_string_as_decimal(xml_doc, attribute)
-      str = string_or_nil(xml_doc, attribute)
-      if str.nil?
-        nil
-      else
-        BigDecimal(str, exception: false)
-      end
-    end
-
-    # Unsafe: A string that has more than just the integer value
-    def unsafe_string_as_integer(xml_doc, attribute)
-      str = string_or_nil(xml_doc, attribute)
-      if str.nil?
-        nil
-      else
-        str.to_i
-      end
-    end
-
-    def compose_date(xml_doc, day_str, month_str, year_str)
-      day = Integer(field_value(xml_doc, "xmlns", day_str), exception: false)
-      month = Integer(field_value(xml_doc, "xmlns", month_str), exception: false)
-      year = Integer(field_value(xml_doc, "xmlns", year_str), exception: false)
-      if day.nil? || month.nil? || year.nil?
-        nil
-      else
-        Time.zone.local(year, month, day)
-      end
-    end
-
     def get_form_name_component(xml_doc, index)
       form_name = meta_field_value(xml_doc, "form-name")
       form_type_components = form_name.split("-")
@@ -399,42 +362,6 @@ module Imports
       end
     end
 
-    def find_organisation_id(xml_doc, id_field)
-      old_visible_id = string_or_nil(xml_doc, id_field)
-      organisation = Organisation.find_by(old_visible_id:)
-      raise "Organisation not found with legacy ID #{old_visible_id}" if organisation.nil?
-
-      organisation.id
-    end
-
-    def sex(xml_doc, index)
-      sex = string_or_nil(xml_doc, "P#{index}Sex")
-      case sex
-      when "Male"
-        "M"
-      when "Female"
-        "F"
-      when "Other", "Non-binary"
-        "X"
-      when "Refused"
-        "R"
-      end
-    end
-
-    def relat(xml_doc, index)
-      relat = string_or_nil(xml_doc, "P#{index}Rel")
-      case relat
-      when "Child"
-        "C"
-      when "Partner"
-        "P"
-      when "Other", "Non-binary"
-        "X"
-      when "Refused"
-        "R"
-      end
-    end
-
     def age_known(xml_doc, index, hhmemb)
       return nil if hhmemb.present? && index > hhmemb
 
@@ -473,16 +400,6 @@ module Imports
       end
     end
 
-    def compose_postcode(xml_doc, outcode, incode)
-      outcode_value = string_or_nil(xml_doc, outcode)
-      incode_value = string_or_nil(xml_doc, incode)
-      if outcode_value.nil? || incode_value.nil? || !"#{outcode_value} #{incode_value}".match(POSTCODE_REGEXP)
-        nil
-      else
-        "#{outcode_value} #{incode_value}"
-      end
-    end
-
     def london_affordable_rent(xml_doc)
       lar = unsafe_string_as_integer(xml_doc, "LAR")
       if lar == 1
@@ -502,34 +419,6 @@ module Imports
       end
     end
 
-    def string_or_nil(xml_doc, attribute)
-      str = field_value(xml_doc, "xmlns", attribute)
-      str.presence
-    end
-
-    def ethnic_group(ethnic)
-      case ethnic
-      when 1, 2, 3, 18
-        # White
-        0
-      when 4, 5, 6, 7
-        # Mixed
-        1
-      when 8, 9, 10, 11, 15
-        # Asian
-        2
-      when 12, 13, 14
-        # Black
-        3
-      when 16, 19
-        # Others
-        4
-      when 17
-        # Refused
-        17
-      end
-    end
-
     # Letters should be lowercase to match case
     def housing_needs(xml_doc, letter)
       housing_need = string_or_nil(xml_doc, "Q10-#{letter}")

app/services/imports/logs_import_service.rb

@@ -0,0 +1,127 @@
+module Imports
+  class LogsImportService < ImportService
+  private
+
+    # Safe: A string that represents only an integer (or empty/nil)
+    def safe_string_as_integer(xml_doc, attribute)
+      str = field_value(xml_doc, "xmlns", attribute)
+      Integer(str, exception: false)
+    end
+
+    # Unsafe: A string that has more than just the integer value
+    def unsafe_string_as_integer(xml_doc, attribute)
+      str = string_or_nil(xml_doc, attribute)
+      if str.nil?
+        nil
+      else
+        str.to_i
+      end
+    end
+
+    def compose_date(xml_doc, day_str, month_str, year_str)
+      day = Integer(field_value(xml_doc, "xmlns", day_str), exception: false)
+      month = Integer(field_value(xml_doc, "xmlns", month_str), exception: false)
+      year = Integer(field_value(xml_doc, "xmlns", year_str), exception: false)
+      if day.nil? || month.nil? || year.nil?
+        nil
+      else
+        Time.zone.local(year, month, day)
+      end
+    end
+
+    def find_organisation_id(xml_doc, id_field)
+      old_visible_id = string_or_nil(xml_doc, id_field)
+      organisation = Organisation.find_by(old_visible_id:)
+      raise "Organisation not found with legacy ID #{old_visible_id}" if organisation.nil?
+
+      organisation.id
+    end
+
+    def string_or_nil(xml_doc, attribute)
+      str = field_value(xml_doc, "xmlns", attribute)
+      str.presence
+    end
+
+    def ethnic_group(ethnic)
+      case ethnic
+      when 1, 2, 3, 18
+        # White
+        0
+      when 4, 5, 6, 7
+        # Mixed
+        1
+      when 8, 9, 10, 11, 15
+        # Asian
+        2
+      when 12, 13, 14
+        # Black
+        3
+      when 16, 19
+        # Others
+        4
+      when 17
+        # Refused
+        17
+      end
+    end
+
+    # Safe: A string that represents only a decimal (or empty/nil)
+    def safe_string_as_decimal(xml_doc, attribute)
+      str = string_or_nil(xml_doc, attribute)
+      if str.nil?
+        nil
+      else
+        BigDecimal(str, exception: false)
+      end
+    end
+
+    def compose_postcode(xml_doc, outcode, incode)
+      outcode_value = string_or_nil(xml_doc, outcode)
+      incode_value = string_or_nil(xml_doc, incode)
+      if outcode_value.nil? || incode_value.nil? || !"#{outcode_value} #{incode_value}".match(POSTCODE_REGEXP)
+        nil
+      else
+        "#{outcode_value} #{incode_value}"
+      end
+    end
+
+    def previous_postcode_known(xml_doc, previous_postcode, prevloc)
+      previous_postcode_known = string_or_nil(xml_doc, "Q7UnknownPostcode")
+      if previous_postcode_known == "If postcode not known tick" || (previous_postcode.nil? && prevloc.present?)
+        1
+      elsif previous_postcode.nil?
+        nil
+      else
+        0
+      end
+    end
+
+    def sex(xml_doc, index)
+      sex = string_or_nil(xml_doc, "P#{index}Sex")
+      case sex
+      when "Male"
+        "M"
+      when "Female"
+        "F"
+      when "Other", "Non-binary"
+        "X"
+      when "Refused"
+        "R"
+      end
+    end
+
+    def relat(xml_doc, index)
+      relat = string_or_nil(xml_doc, "P#{index}Rel")
+      case relat
+      when "Child"
+        "C"
+      when "Partner"
+        "P"
+      when "Other", "Non-binary"
+        "X"
+      when "Refused", "Buyer prefers not to say"
+        "R"
+      end
+    end
+  end
+end

app/services/imports/sales_logs_import_service.rb

@@ -1,5 +1,5 @@
 module Imports
-  class SalesLogsImportService < ImportService
+  class SalesLogsImportService < LogsImportService
     def initialize(storage_service, logger = Rails.logger)
       @logs_with_discrepancies = Set.new
       @logs_overridden = Set.new
@@ -62,7 +62,7 @@ module Imports
       attributes["hholdcount"] = safe_string_as_integer(xml_doc, "LiveInOther") || 0
       attributes["la"] = string_or_nil(xml_doc, "Q14ONSLACode")
       attributes["income1"] = safe_string_as_integer(xml_doc, "Q2Person1Income")
-      attributes["income1nk"] = income_known(unsafe_string_as_integer(xml_doc, "P1IncKnown")) 
+      attributes["income1nk"] = income_known(unsafe_string_as_integer(xml_doc, "P1IncKnown"))
       attributes["inc1mort"] = unsafe_string_as_integer(xml_doc, "Q2Person1Mortgage")
       attributes["income2"] = safe_string_as_integer(xml_doc, "Q2Person2Income")
       attributes["income2nk"] = income_known(unsafe_string_as_integer(xml_doc, "P2IncKnown"))
@@ -96,7 +96,6 @@ module Imports
       attributes["pregother"] = 1 if string_or_nil(xml_doc, "PREGOTHER") == "Yes"
       attributes["ppostcode_full"] = compose_postcode(xml_doc, "PPOSTC1", "PPOSTC2")
       attributes["prevloc"] = string_or_nil(xml_doc, "Q7ONSLACODE")
-      # attributes["is_previous_la_inferred"] = nil
       attributes["ppcodenk"] = previous_postcode_known(xml_doc, attributes["ppostcode_full"], attributes["prevloc"]) # Q7UNKNOWNPOSTCODE check mapping
       attributes["ppostc1"] = string_or_nil(xml_doc, "PPOSTC1")
       attributes["ppostc2"] = string_or_nil(xml_doc, "PPOSTC2")
@@ -136,9 +135,8 @@ module Imports
       attributes["soctenant"] = soctenant(attributes)
 
       # Required for our form invalidated questions (not present in import)
-      attributes["previous_la_known"] = 1 if attributes["prevloc"].present? && !attributes["ppostcode_full"].present?
+      attributes["previous_la_known"] = 1 if attributes["prevloc"].present? && attributes["ppostcode_full"].blank?
-      # attributes["is_la_inferred"] = attributes["postcode_full"].present?
+      attributes["la_known"] = 1 if attributes["la"].present? && attributes["postcode_full"].blank?
-      attributes["la_known"] = 1 if attributes["la"].present? && !attributes["postcode_full"].present?
 
       # Sets the log creator
       owner_id = meta_field_value(xml_doc, "owner-user-id").strip
@@ -215,127 +213,6 @@ module Imports
       end
     end
 
-    # Safe: A string that represents only an integer (or empty/nil)
-    def safe_string_as_integer(xml_doc, attribute)
-      str = field_value(xml_doc, "xmlns", attribute)
-      Integer(str, exception: false)
-    end
-
-    # Unsafe: A string that has more than just the integer value
-    def unsafe_string_as_integer(xml_doc, attribute)
-      str = string_or_nil(xml_doc, attribute)
-      if str.nil?
-        nil
-      else
-        str.to_i
-      end
-    end
-
-    def compose_date(xml_doc, day_str, month_str, year_str)
-      day = Integer(field_value(xml_doc, "xmlns", day_str), exception: false)
-      month = Integer(field_value(xml_doc, "xmlns", month_str), exception: false)
-      year = Integer(field_value(xml_doc, "xmlns", year_str), exception: false)
-      if day.nil? || month.nil? || year.nil?
-        nil
-      else
-        Time.zone.local(year, month, day)
-      end
-    end
-
-    def find_organisation_id(xml_doc, id_field)
-      old_visible_id = string_or_nil(xml_doc, id_field)
-      organisation = Organisation.find_by(old_visible_id:)
-      raise "Organisation not found with legacy ID #{old_visible_id}" if organisation.nil?
-
-      organisation.id
-    end
-
-    def string_or_nil(xml_doc, attribute)
-      str = field_value(xml_doc, "xmlns", attribute)
-      str.presence
-    end
-
-    def ethnic_group(ethnic)
-      case ethnic
-      when 1, 2, 3, 18
-        # White
-        0
-      when 4, 5, 6, 7
-        # Mixed
-        1
-      when 8, 9, 10, 11, 15
-        # Asian
-        2
-      when 12, 13, 14
-        # Black
-        3
-      when 16, 19
-        # Others
-        4
-      when 17
-        # Refused
-        17
-      end
-    end
-
-    def safe_string_as_decimal(xml_doc, attribute)
-      str = string_or_nil(xml_doc, attribute)
-      if str.nil?
-        nil
-      else
-        BigDecimal(str, exception: false)
-      end
-    end
-
-    def compose_postcode(xml_doc, outcode, incode)
-      outcode_value = string_or_nil(xml_doc, outcode)
-      incode_value = string_or_nil(xml_doc, incode)
-      if outcode_value.nil? || incode_value.nil? || !"#{outcode_value} #{incode_value}".match(POSTCODE_REGEXP)
-        nil
-      else
-        "#{outcode_value} #{incode_value}"
-      end
-    end
-
-    def previous_postcode_known(xml_doc, previous_postcode, prevloc)
-      previous_postcode_known = string_or_nil(xml_doc, "Q7UnknownPostcode")
-      if previous_postcode_known == "If postcode not known tick" || (previous_postcode.nil? && prevloc.present?)
-        1
-      elsif previous_postcode.nil?
-        nil
-      else
-        0
-      end
-    end
-
-    def sex(xml_doc, index)
-      sex = string_or_nil(xml_doc, "P#{index}Sex")
-      case sex
-      when "Male"
-        "M"
-      when "Female"
-        "F"
-      when "Other", "Non-binary"
-        "X"
-      when "Refused"
-        "R"
-      end
-    end
-
-    def relat(xml_doc, index)
-      relat = string_or_nil(xml_doc, "P#{index}Rel")
-      case relat
-      when "Child"
-        "C"
-      when "Partner"
-        "P"
-      when "Other", "Non-binary"
-        "X"
-      when "Buyer prefers not to say"
-        "R"
-      end
-    end
-
     def age_known(_xml_doc, index, hhmemb, age)
       return nil if hhmemb.present? && index > hhmemb
 
@@ -409,9 +286,9 @@ module Imports
 
     def income_known(value)
       case value
-      when 1 #known
+      when 1 # known
         0
-      when 2 #unknown
+      when 2 # unknown
         1
       end
     end