diff --git a/app/services/imports/case_logs_import_service.rb b/app/services/imports/case_logs_import_service.rb
index adec47526..7a9799078 100644
--- a/app/services/imports/case_logs_import_service.rb
+++ b/app/services/imports/case_logs_import_service.rb
@@ -96,8 +96,8 @@ module Imports
attributes["prevten"] = unsafe_string_as_integer(xml_doc, "Q11")
attributes["prevloc"] = string_or_nil(xml_doc, "Q12aONS")
- attributes["previous_postcode_known"] = previous_postcode_known(xml_doc)
attributes["ppostcode_full"] = compose_postcode(xml_doc, "PPOSTC1", "PPOSTC2")
+ attributes["previous_postcode_known"] = previous_postcode_known(xml_doc, attributes["ppostcode_full"])
attributes["layear"] = unsafe_string_as_integer(xml_doc, "Q12c")
attributes["waityear"] = unsafe_string_as_integer(xml_doc, "Q12d")
attributes["homeless"] = unsafe_string_as_integer(xml_doc, "Q13")
@@ -142,15 +142,17 @@ module Imports
attributes["postcode_known"] = attributes["postcode_full"].nil? ? 0 : 1
# Not specific to our form but required for consistency (present in import)
- attributes["old_form_id"] = Integer(field_value(xml_doc, "xmlns", "FORM"))
+ attributes["old_form_id"] = safe_string_as_integer(xml_doc, "FORM")
attributes["created_at"] = Time.zone.parse(field_value(xml_doc, "meta", "created-date"))
attributes["updated_at"] = Time.zone.parse(field_value(xml_doc, "meta", "modified-date"))
+ attributes["old_id"] = field_value(xml_doc, "meta", "document-id")
# Required for our form invalidated questions (not present in import)
attributes["previous_la_known"] = 1 # Defaulting to Yes (Required)
attributes["la_known"] = 1 # Defaulting to Yes (Required)
attributes["is_la_inferred"] = false # Always keep the given LA
attributes["first_time_property_let_as_social_housing"] = first_time_let(attributes["rsnvac"])
+ attributes["declaration"] = declaration(xml_doc)
unless attributes["brent"].nil? &&
attributes["scharge"].nil? &&
@@ -163,8 +165,19 @@ module Imports
end
case_log = CaseLog.new(attributes)
- case_log.save!
+ save_case_log(case_log, attributes)
compute_differences(case_log, attributes)
+ check_status_completed(case_log)
+ end
+
+ def save_case_log(case_log, attributes)
+ case_log.save!
+ rescue ActiveRecord::RecordNotUnique
+ unless attributes["old_id"].nil?
+ record = CaseLog.find_by(old_id: attributes["old_id"])
+ @logger.info "Updating case log #{case_log.id} with legacy ID #{attributes['old_id']}"
+ record.update!(attributes)
+ end
end
def compute_differences(case_log, attributes)
@@ -172,10 +185,16 @@ module Imports
attributes.each do |key, value|
case_log_value = case_log.send(key.to_sym)
if value != case_log_value
- differences.push("#{key} #{value} #{case_log_value}")
+ differences.push("#{key} #{value.inspect} #{case_log_value.inspect}")
end
end
- raise "Differences found when saving log #{case_log.id}: #{differences}" unless differences.empty?
+ @logger.warn "Differences found when saving log #{case_log.id}: #{differences}" unless differences.empty?
+ end
+
+ def check_status_completed(case_log)
+ unless case_log.status == "completed"
+ @logger.warn "Case log #{case_log.id} is not completed"
+ end
end
# Safe: A string that represents only an integer (or empty/nil)
@@ -186,8 +205,8 @@ module Imports
# Safe: A string that represents only a decimal (or empty/nil)
def safe_string_as_decimal(xml_doc, attribute)
- str = field_value(xml_doc, "xmlns", attribute)
- if str.blank?
+ str = string_or_nil(xml_doc, attribute)
+ if str.nil?
nil
else
BigDecimal(str, exception: false)
@@ -196,8 +215,8 @@ module Imports
# Unsafe: A string that has more than just the integer value
def unsafe_string_as_integer(xml_doc, attribute)
- str = field_value(xml_doc, "xmlns", attribute)
- if str.blank?
+ str = string_or_nil(xml_doc, attribute)
+ if str.nil?
nil
else
str.to_i
@@ -281,7 +300,7 @@ module Imports
end
def sex(xml_doc, index)
- sex = field_value(xml_doc, "xmlns", "P#{index}Sex")
+ sex = string_or_nil(xml_doc, "P#{index}Sex")
case sex
when "Male"
"M"
@@ -295,7 +314,7 @@ module Imports
end
def relat(xml_doc, index)
- relat = field_value(xml_doc, "xmlns", "P#{index}Rel")
+ relat = string_or_nil(xml_doc, "P#{index}Rel")
case relat
when "Child"
"C"
@@ -311,7 +330,7 @@ module Imports
def age_known(xml_doc, index, hhmemb)
return nil if index > hhmemb
- age_refused = field_value(xml_doc, "xmlns", "P#{index}AR")
+ age_refused = string_or_nil(xml_doc, "P#{index}AR")
if age_refused == "AGE_REFUSED"
1 # No
else
@@ -332,19 +351,21 @@ module Imports
end
end
- def previous_postcode_known(xml_doc)
- previous_postcode_known = field_value(xml_doc, "xmlns", "Q12bnot")
+ def previous_postcode_known(xml_doc, previous_postcode)
+ previous_postcode_known = string_or_nil(xml_doc, "Q12bnot")
if previous_postcode_known == "Temporary or Unknown"
0
+ elsif previous_postcode.nil?
+ nil
else
1
end
end
def compose_postcode(xml_doc, outcode, incode)
- outcode_value = field_value(xml_doc, "xmlns", outcode)
- incode_value = field_value(xml_doc, "xmlns", incode)
- if outcode_value.blank? || incode_value.blank?
+ outcode_value = string_or_nil(xml_doc, outcode)
+ incode_value = string_or_nil(xml_doc, incode)
+ if outcode_value.nil? || incode_value.nil?
nil
else
"#{outcode_value}#{incode_value}"
@@ -400,7 +421,7 @@ module Imports
# Letters should be lowercase to match case
def housing_needs(xml_doc, letter)
- housing_need = field_value(xml_doc, "xmlns", "Q10-#{letter}")
+ housing_need = string_or_nil(xml_doc, "Q10-#{letter}")
if housing_need == "Yes"
1
else
@@ -409,7 +430,7 @@ module Imports
end
def net_income_known(xml_doc, earnings)
- incref = field_value(xml_doc, "xmlns", "Q8Refused")
+ incref = string_or_nil(xml_doc, "Q8Refused")
if incref == "Refused"
# Tenant prefers not to say
2
@@ -438,5 +459,12 @@ module Imports
0
end
end
+
+ def declaration(xml_doc)
+ declaration = string_or_nil(xml_doc, "Qdp")
+ if declaration == "Yes"
+ 1
+ end
+ end
end
end
diff --git a/app/services/imports/import_service.rb b/app/services/imports/import_service.rb
index bd48dc8a3..3f319b358 100644
--- a/app/services/imports/import_service.rb
+++ b/app/services/imports/import_service.rb
@@ -14,7 +14,7 @@ module Imports
xml_document = Nokogiri::XML(file_io)
send(create_method, xml_document)
rescue StandardError => e
- @logger.error "#{e.class} in #{filename}: #{e.message}"
+ @logger.error "#{e.class} in #{filename}: #{e.message}. Caller: #{e.backtrace.first}"
end
end
diff --git a/db/migrate/20220425143130_add_old_id_and_index_to_case_logs.rb b/db/migrate/20220425143130_add_old_id_and_index_to_case_logs.rb
new file mode 100644
index 000000000..882a77f43
--- /dev/null
+++ b/db/migrate/20220425143130_add_old_id_and_index_to_case_logs.rb
@@ -0,0 +1,8 @@
+class AddOldIdAndIndexToCaseLogs < ActiveRecord::Migration[7.0]
+ def change
+ change_table :case_logs, bulk: true do |t|
+ t.column :old_id, :string
+ end
+ add_index :case_logs, :old_id, unique: true
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index dcd9e007a..40cd9cd83 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.
-ActiveRecord::Schema[7.0].define(version: 2022_04_21_140410) do
+ActiveRecord::Schema[7.0].define(version: 2022_04_25_143130) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
@@ -219,7 +219,9 @@ ActiveRecord::Schema[7.0].define(version: 2022_04_21_140410) do
t.integer "old_form_id"
t.integer "lar"
t.integer "irproduct"
+ t.string "old_id"
t.index ["managing_organisation_id"], name: "index_case_logs_on_managing_organisation_id"
+ t.index ["old_id"], name: "index_case_logs_on_old_id", unique: true
t.index ["owning_organisation_id"], name: "index_case_logs_on_owning_organisation_id"
end
diff --git a/spec/fixtures/exports/case_logs.xml b/spec/fixtures/exports/case_logs.xml
index a5dd626af..e837ae266 100644
--- a/spec/fixtures/exports/case_logs.xml
+++ b/spec/fixtures/exports/case_logs.xml
@@ -161,6 +161,7 @@
+
1
diff --git a/spec/services/imports/case_logs_import_service_spec.rb b/spec/services/imports/case_logs_import_service_spec.rb
index 111106f31..801d1a065 100644
--- a/spec/services/imports/case_logs_import_service_spec.rb
+++ b/spec/services/imports/case_logs_import_service_spec.rb
@@ -5,8 +5,6 @@ RSpec.describe Imports::CaseLogsImportService do
let(:fixture_directory) { "spec/fixtures/softwire_imports/case_logs" }
let(:case_log_id) { "0ead17cb-1668-442d-898c-0d52879ff592" }
let(:case_log_id2) { "166fc004-392e-47a8-acb8-1c018734882b" }
- let(:case_log_file) { File.open("#{fixture_directory}/#{case_log_id}.xml") }
- let(:case_log_file2) { File.open("#{fixture_directory}/#{case_log_id2}.xml") }
let(:storage_service) { instance_double(StorageService) }
let(:real_2021_2022_form) { Form.new("config/forms/2021_2022.json", "2021_2022") }
let(:logger) { instance_double(ActiveSupport::Logger) }
@@ -14,24 +12,38 @@ RSpec.describe Imports::CaseLogsImportService do
context "when importing users" do
subject(:case_log_service) { described_class.new(storage_service, logger) }
+ def open_file(directory, filename)
+ File.open("#{directory}/#{filename}.xml")
+ end
+
before do
# Stub the S3 file listing and download
allow(storage_service).to receive(:list_files)
.and_return(%W[#{remote_folder}/#{case_log_id}.xml #{remote_folder}/#{case_log_id2}.xml])
allow(storage_service).to receive(:get_file_io)
.with("#{remote_folder}/#{case_log_id}.xml")
- .and_return(case_log_file)
+ .and_return(open_file(fixture_directory, case_log_id), open_file(fixture_directory, case_log_id))
allow(storage_service).to receive(:get_file_io)
.with("#{remote_folder}/#{case_log_id2}.xml")
- .and_return(case_log_file2)
+ .and_return(open_file(fixture_directory, case_log_id2), open_file(fixture_directory, case_log_id2))
# Stub the form handler to use the real form
allow(FormHandler.instance).to receive(:get_form).with(anything).and_return(real_2021_2022_form)
end
it "successfully create all case logs" do
expect(logger).not_to receive(:error)
+ expect(logger).not_to receive(:warn)
+ expect(logger).not_to receive(:info)
expect { case_log_service.create_logs(remote_folder) }
.to change(CaseLog, :count).by(2)
end
+
+ it "only updates existing case logs" do
+ expect(logger).not_to receive(:error)
+ expect(logger).not_to receive(:warn)
+ expect(logger).to receive(:info).with(/Updating case log/).twice
+ expect { 2.times { case_log_service.create_logs(remote_folder) } }
+ .to change(CaseLog, :count).by(2)
+ end
end
end