From c80565a54d7baf7c0d251f85b63319020cf06c56 Mon Sep 17 00:00:00 2001
From: natdeanlewissoftwire
 <94526761+natdeanlewissoftwire@users.noreply.github.com>
Date: Tue, 12 Sep 2023 13:45:20 +0100
Subject: [PATCH] Make saledate nil safe(r) (#1904)

* feat: allow missing day/month/year values if CompletionDate exists as we don't always receive these from old core

* feat: make safe string as decimal nil safer too

* feat: add test
---
 app/services/imports/logs_import_service.rb   |  2 +-
 .../imports/sales_logs_import_service.rb      |  4 +--
 .../lettings_logs_import_service_spec.rb      | 21 ++++++++++++++++
 .../imports/sales_logs_import_service_spec.rb | 25 +++++++++++++++++++
 4 files changed, 49 insertions(+), 3 deletions(-)

diff --git a/app/services/imports/logs_import_service.rb b/app/services/imports/logs_import_service.rb
index a82208bbe..f7456a3fb 100644
--- a/app/services/imports/logs_import_service.rb
+++ b/app/services/imports/logs_import_service.rb
@@ -72,7 +72,7 @@ module Imports
       if str.nil?
         nil
       else
-        BigDecimal(str, exception: false).round(2)
+        BigDecimal(str, exception: false)&.round(2)
       end
     end
 
diff --git a/app/services/imports/sales_logs_import_service.rb b/app/services/imports/sales_logs_import_service.rb
index 1a9fe54a0..7d10f52ca 100644
--- a/app/services/imports/sales_logs_import_service.rb
+++ b/app/services/imports/sales_logs_import_service.rb
@@ -15,7 +15,7 @@ module Imports
     def create_log(xml_doc)
       # only import sales logs from 22/23 collection period onwards
       return unless meta_field_value(xml_doc, "form-name").include?("Sales")
-      return unless compose_date(xml_doc, "DAY", "MONTH", "YEAR") >= Time.zone.local(2022, 4, 1)
+      return unless (compose_date(xml_doc, "DAY", "MONTH", "YEAR") || Time.zone.parse(field_value(xml_doc, "xmlns", "CompletionDate"))) >= Time.zone.local(2022, 4, 1)
 
       attributes = {}
 
@@ -24,7 +24,7 @@ module Imports
       # Required fields for status complete or logic to work
       # Note: order matters when we derive from previous values (attributes parameter)
 
-      attributes["saledate"] = compose_date(xml_doc, "DAY", "MONTH", "YEAR")
+      attributes["saledate"] = compose_date(xml_doc, "DAY", "MONTH", "YEAR") || Time.zone.parse(field_value(xml_doc, "xmlns", "CompletionDate"))
       attributes["owning_organisation_id"] = find_organisation_id(xml_doc, "OWNINGORGID")
       attributes["type"] = unsafe_string_as_integer(xml_doc, "DerSaleType")
       attributes["old_id"] = meta_field_value(xml_doc, "document-id")
diff --git a/spec/services/imports/lettings_logs_import_service_spec.rb b/spec/services/imports/lettings_logs_import_service_spec.rb
index 800955cfc..dad2e68ed 100644
--- a/spec/services/imports/lettings_logs_import_service_spec.rb
+++ b/spec/services/imports/lettings_logs_import_service_spec.rb
@@ -566,6 +566,27 @@ RSpec.describe Imports::LettingsLogsImportService do
         end
       end
 
+      context "and the number of times the property was relet is a non nil string that is nil as a BigDecimal" do
+        before do
+          lettings_log_xml.at_xpath("//xmlns:Q20").content = "a"
+        end
+
+        it "doesn't throw an error" do
+          expect { lettings_log_service.send(:create_log, lettings_log_xml) }
+            .not_to raise_error
+        end
+
+        it "clears out the number offered answer" do
+          allow(logger).to receive(:warn)
+
+          lettings_log_service.send(:create_log, lettings_log_xml)
+          lettings_log = LettingsLog.find_by(old_id: lettings_log_id)
+
+          expect(lettings_log).not_to be_nil
+          expect(lettings_log.offered).to be_nil
+        end
+      end
+
       context "and the gender identity is refused" do
         before do
           lettings_log_xml.at_xpath("//xmlns:P1Sex").content = "Person prefers not to say"
diff --git a/spec/services/imports/sales_logs_import_service_spec.rb b/spec/services/imports/sales_logs_import_service_spec.rb
index e4d5b9473..6d067dbd4 100644
--- a/spec/services/imports/sales_logs_import_service_spec.rb
+++ b/spec/services/imports/sales_logs_import_service_spec.rb
@@ -228,6 +228,31 @@ RSpec.describe Imports::SalesLogsImportService do
       end
     end
 
+    context "and the log startdate is only present in the CompletionDate field" do
+      let(:sales_log_id) { "shared_ownership_sales_log" }
+
+      before do
+        sales_log_xml.at_xpath("//xmlns:DAY").content = nil
+        sales_log_xml.at_xpath("//xmlns:MONTH").content = nil
+        sales_log_xml.at_xpath("//xmlns:YEAR").content = nil
+        sales_log_xml.at_xpath("//xmlns:CompletionDate").content = "2022-10-9"
+        sales_log_xml.at_xpath("//xmlns:HODAY").content = 9
+        sales_log_xml.at_xpath("//xmlns:HOMONTH").content = 10
+        sales_log_xml.at_xpath("//xmlns:HOYEAR").content = 2022
+        sales_log_xml.at_xpath("//xmlns:EXDAY").content = 9
+        sales_log_xml.at_xpath("//xmlns:EXMONTH").content = 10
+        sales_log_xml.at_xpath("//xmlns:EXYEAR").content = 2022
+      end
+
+      it "creates the log with the correct saledate" do
+        expect(logger).not_to receive(:error)
+        expect(logger).not_to receive(:warn)
+        expect { sales_log_service.send(:create_log, sales_log_xml) }
+          .to change(SalesLog, :count).by(1)
+        expect(SalesLog.last.saledate).to eq(Time.zone.local(2022, 10, 9))
+      end
+    end
+
     context "when the log is valid" do
       let(:sales_log_id) { "shared_ownership_sales_log" }