diff --git a/app/services/imports/lettings_logs_field_import_service.rb b/app/services/imports/lettings_logs_field_import_service.rb index c7044a062..4530b5c38 100644 --- a/app/services/imports/lettings_logs_field_import_service.rb +++ b/app/services/imports/lettings_logs_field_import_service.rb @@ -259,23 +259,27 @@ module Imports record = LettingsLog.find_by(old_id:) return @logger.warn("lettings log with old id #{old_id} not found") unless record - - return @logger.info("lettings log #{record.id} has values for sex2 and relat2, skipping update") if record.sex2 && record.relat2 return @logger.info("lettings log #{record.id} has no hhmemb value, skipping update") if record.hhmemb.blank? - return @logger.info("lettings log #{record.id} has value 'no' for details_known2, skipping update") if record.details_known_2 == 1 - if record.sex2.blank? - record.sex2 = sex(xml_doc, 2) - @logger.info("lettings log #{record.id}'s sex2 value has been set to #{record.sex2}") - end + (2..record.hhmemb).each do |i| + next @logger.info("lettings log #{record.id} has values for sex#{i} and relat#{i}, skipping update") if record["sex#{i}"] && record["relat#{i}"] + next @logger.info("lettings log #{record.id} has value 'no' for details_known_#{i}, skipping update") if record["details_known_#{i}"] == 1 + + if record["sex#{i}"].blank? + record["sex#{i}"] = sex(xml_doc, i) + @logger.info("lettings log #{record.id}'s sex#{i} value has been set to #{record["sex#{i}"]}") + end - if record.relat2.blank? - record.relat2 = relat(xml_doc, 2) - @logger.info("lettings log #{record.id}'s relat2 value has been set to #{record.relat2}") + if record["relat#{i}"].blank? + record["relat#{i}"] = relat(xml_doc, i) + @logger.info("lettings log #{record.id}'s relat#{i} value has been set to #{record["relat#{i}"]}") + end end - record.values_updated_at = Time.zone.now - record.save! + if record.changed? + record.values_updated_at = Time.zone.now + record.save! + end end end end diff --git a/spec/services/imports/lettings_logs_field_import_service_spec.rb b/spec/services/imports/lettings_logs_field_import_service_spec.rb index 6985200b9..782ce86c8 100644 --- a/spec/services/imports/lettings_logs_field_import_service_spec.rb +++ b/spec/services/imports/lettings_logs_field_import_service_spec.rb @@ -809,7 +809,7 @@ RSpec.describe Imports::LettingsLogsFieldImportService do it "does not update the lettings_log sex and relat value if details for person are not known" do lettings_log.update!(details_known_2: 1) - expect(logger).to receive(:info).with(/lettings log \d+ has value 'no' for details_known2, skipping update/) + expect(logger).to receive(:info).with(/lettings log \d+ has value 'no' for details_known_2, skipping update/) import_service.send(:update_sex_and_relat, lettings_log_xml) lettings_log.reload expect(lettings_log.sex2).to eq(nil) @@ -877,5 +877,64 @@ RSpec.describe Imports::LettingsLogsFieldImportService do expect(lettings_log.values_updated_at).to be_nil end end + + context "when there are multiple people details given" do + before do + (2..8).each do |i| + lettings_log_xml.at_xpath("//xmlns:P#{i}Sex").content = "Person prefers not to say" + lettings_log_xml.at_xpath("//xmlns:P#{i}Rel").content = "Person prefers not to say" + end + end + + it "correctly skips and sets sex and relat values" do + lettings_log.update!(sex2: "F", relat2: "X", details_known_2: 0, + sex3: nil, relat3: nil, details_known_3: 0, + sex4: "F", relat4: nil, details_known_4: 0, + sex5: nil, relat5: "X", details_known_5: 0, + sex6: nil, relat6: nil, details_known_6: 1, + hhmemb: 6, values_updated_at: nil) + + expect(logger).to receive(:info).with(/lettings log \d+ has values for sex2 and relat2, skipping update/) + expect(logger).to receive(:info).with(/lettings log \d+'s sex3 value has been set to R/) + expect(logger).to receive(:info).with(/lettings log \d+'s relat3 value has been set to R/) + expect(logger).to receive(:info).with(/lettings log \d+'s relat4 value has been set to R/) + expect(logger).to receive(:info).with(/lettings log \d+'s sex5 value has been set to R/) + expect(logger).to receive(:info).with(/lettings log \d+ has value 'no' for details_known_6, skipping update/) + import_service.send(:update_sex_and_relat, lettings_log_xml) + + lettings_log.reload + expect(lettings_log.sex2).to eq("F") + expect(lettings_log.relat2).to eq("X") + expect(lettings_log.sex3).to eq("R") + expect(lettings_log.relat3).to eq("R") + expect(lettings_log.sex4).to eq("F") + expect(lettings_log.relat4).to eq("R") + expect(lettings_log.sex5).to eq("R") + expect(lettings_log.relat5).to eq("X") + expect(lettings_log.sex6).to eq(nil) + expect(lettings_log.relat6).to eq(nil) + expect(lettings_log.sex7).to eq(nil) + expect(lettings_log.relat7).to eq(nil) + expect(lettings_log.sex8).to eq(nil) + expect(lettings_log.relat8).to eq(nil) + expect(lettings_log.values_updated_at).not_to be_nil + end + + it "correctly skips the entire update if none of the fields need to be updated" do + lettings_log.update!(sex2: "F", relat2: "X", details_known_2: 0, + sex3: nil, relat3: nil, details_known_3: 1, + sex4: "F", relat4: "X", details_known_4: 0, + sex5: nil, relat5: nil, details_known_5: 1, + hhmemb: 5, values_updated_at: nil) + + expect(logger).to receive(:info).with(/lettings log \d+ has values for sex2 and relat2, skipping update/) + expect(logger).to receive(:info).with(/lettings log \d+ has value 'no' for details_known_3, skipping update/) + expect(logger).to receive(:info).with(/lettings log \d+ has values for sex4 and relat4, skipping update/) + expect(logger).to receive(:info).with(/lettings log \d+ has value 'no' for details_known_5, skipping update/) + + import_service.send(:update_sex_and_relat, lettings_log_xml) + expect(lettings_log.values_updated_at).to be_nil + end + end end end