diff --git a/app/services/imports/case_logs_import_service.rb b/app/services/imports/case_logs_import_service.rb index adec47526..8eb272d62 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"] = 1 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) @@ -332,10 +351,12 @@ module Imports end end - def previous_postcode_known(xml_doc) + def previous_postcode_known(xml_doc, previous_postcode) previous_postcode_known = field_value(xml_doc, "xmlns", "Q12bnot") if previous_postcode_known == "Temporary or Unknown" 0 + elsif previous_postcode.nil? + nil else 1 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..467c06f38 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,37 @@ 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 { 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