baarkerlounger
/
submit-social-housing-lettings-and-sales-data
mirror of https://github.com/communitiesuk/submit-social-housing-lettings-and-sales-data.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity
Browse Source

Fix rubocop errors

CLDC-1222-improve-case-log-import-performance
Mo Seedat 2 years ago
parent
a232f5ec9b
commit
746a6bb73e
5 changed files with 39 additions and 48 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 2
      app/services/imports/import_service.rb
  2. 4
      app/services/imports/lettings_logs_import_processor.rb
  3. 4
      spec/jobs/lettings_log_import_job_spec.rb
  4. 45
      spec/services/imports/lettings_logs_import_processor_spec.rb
  5. 32
      spec/services/imports/lettings_logs_import_service_spec.rb

2
app/services/imports/import_service.rb
Unescape View File

@ -2,8 +2,6 @@ module Imports
class ImportService
include Imports::ImportUtils
private
def initialize(storage_service, logger = Rails.logger)
@storage_service = storage_service
@logger = logger

4
app/services/imports/lettings_logs_import_processor.rb
Unescape View File

@ -320,14 +320,14 @@ module Imports
end
def compute_differences(lettings_log, attributes)
differences = attributes.map do |key, value|
differences = attributes.map { |key, value|
lettings_log_value = lettings_log.send(key.to_sym)
next if FIELDS_NOT_PRESENT_IN_SOFTWIRE_DATA.include?(key)
next if value == lettings_log_value
"#{key} #{value.inspect} #{lettings_log_value.inspect}"
end.compact
}.compact
if differences.any?
@logger.warn "Differences found when saving log #{lettings_log.old_id}: #{differences}"

4
spec/jobs/lettings_log_import_job_spec.rb
Unescape View File

@ -7,10 +7,6 @@ RSpec.describe LettingsLogImportJob do
describe "#perform" do
context "with valid params" do
before do
end
it "executes LettingsLogsImportProcessor" do
expect(Imports::LettingsLogsImportProcessor).to receive(:new)

45
spec/services/imports/lettings_logs_import_processor_spec.rb
Unescape View File

@ -11,10 +11,9 @@ RSpec.describe Imports::LettingsLogsImportProcessor do
let(:organisation) { FactoryBot.create(:organisation, old_visible_id: "1", provider_type: "PRP") }
let(:scheme1) { FactoryBot.create(:scheme, old_visible_id: 123, owning_organisation: organisation) }
let(:scheme2) { FactoryBot.create(:scheme, old_visible_id: 456, owning_organisation: organisation) }
def open_file(directory, filename)
File.open("#{directory}/#{filename}.xml")
end
let(:lettings_log_id) { "0ead17cb-1668-442d-898c-0d52879ff592" }
let(:lettings_log_file) { open_file(fixture_directory, lettings_log_id) }
let(:lettings_log_xml) { Nokogiri::XML(lettings_log_file) }
before do
WebMock.stub_request(:get, /api.postcodes.io\/postcodes\/LS166FT/)
@ -37,14 +36,10 @@ RSpec.describe Imports::LettingsLogsImportProcessor do
allow(FormHandler.instance).to receive(:get_form).with("current_lettings").and_return(real_2022_2023_form)
end
let(:lettings_log_id) { "0ead17cb-1668-442d-898c-0d52879ff592" }
let(:lettings_log_file) { open_file(fixture_directory, lettings_log_id) }
let(:lettings_log_xml) { Nokogiri::XML(lettings_log_file) }
describe '#initialize' do
describe "#initialize" do
context "with valid params" do
it "sets document-id as old_id" do
import = Imports::LettingsLogsImportProcessor.new(lettings_log_xml.to_s, logger)
import = described_class.new(lettings_log_xml.to_s, logger)
expect(import.old_id).to eq "0ead17cb-1668-442d-898c-0d52879ff592"
end
@ -57,7 +52,7 @@ RSpec.describe Imports::LettingsLogsImportProcessor do
expect(logger).to receive(:warn).with(/is not completed/).once
expect(logger).to receive(:warn).with(/lettings log with old id:#{lettings_log_id} is incomplete but status should be complete/).once
import = Imports::LettingsLogsImportProcessor.new(lettings_log_xml.to_s, logger)
import = described_class.new(lettings_log_xml.to_s, logger)
lettings_log = LettingsLog.where(old_id: lettings_log_id).first
expect(lettings_log&.voiddate).to be_nil
@ -69,7 +64,7 @@ RSpec.describe Imports::LettingsLogsImportProcessor do
before { lettings_log_xml.at_xpath("//xmlns:OWNINGORGID").content = 99_999 }
it "raises an exception" do
expect { Imports::LettingsLogsImportProcessor.new(lettings_log_xml.to_s, logger) }
expect { described_class.new(lettings_log_xml.to_s, logger) }
.to raise_error(RuntimeError, "Organisation not found with legacy ID 99999")
end
end
@ -83,7 +78,7 @@ RSpec.describe Imports::LettingsLogsImportProcessor do
it "sets the economic status to child under 16" do
# The update is done when calculating derived variables
expect(logger).to receive(:warn).with(/Differences found when saving log/)
Imports::LettingsLogsImportProcessor.new(lettings_log_xml.to_s, logger)
described_class.new(lettings_log_xml.to_s, logger)
lettings_log = LettingsLog.where(old_id: lettings_log_id).first
expect(lettings_log&.ecstat2).to be(9)
@ -94,7 +89,7 @@ RSpec.describe Imports::LettingsLogsImportProcessor do
before { lettings_log_xml.at_xpath("//xmlns:P2Rel").content = "Refused" }
it "sets the relationship to lead tenant to child" do
Imports::LettingsLogsImportProcessor.new(lettings_log_xml.to_s, logger)
described_class.new(lettings_log_xml.to_s, logger)
lettings_log = LettingsLog.where(old_id: lettings_log_id).first
expect(lettings_log&.relat2).to eq("C")
@ -110,14 +105,14 @@ RSpec.describe Imports::LettingsLogsImportProcessor do
it "intercepts the relevant validation error" do
expect(logger).to receive(:warn).with(/Removing internal transfer referral since previous tenancy is a non social housing/)
expect { Imports::LettingsLogsImportProcessor.new(lettings_log_xml.to_s, logger) }
expect { described_class.new(lettings_log_xml.to_s, logger) }
.not_to raise_error
end
it "clears out the referral answer" do
allow(logger).to receive(:warn)
Imports::LettingsLogsImportProcessor.new(lettings_log_xml.to_s, logger)
described_class.new(lettings_log_xml.to_s, logger)
lettings_log = LettingsLog.find_by(old_id: lettings_log_id)
expect(lettings_log).not_to be_nil
@ -132,14 +127,14 @@ RSpec.describe Imports::LettingsLogsImportProcessor do
it "intercepts the relevant validation error" do
expect(logger).to receive(:warn).with(/Removing internal transfer referral since previous tenancy is fixed terms or lifetime/)
expect { Imports::LettingsLogsImportProcessor.new(lettings_log_xml.to_s, logger) }
expect { described_class.new(lettings_log_xml.to_s, logger) }
.not_to raise_error
end
it "clears out the referral answer" do
allow(logger).to receive(:warn)
Imports::LettingsLogsImportProcessor.new(lettings_log_xml.to_s, logger)
described_class.new(lettings_log_xml.to_s, logger)
lettings_log = LettingsLog.find_by(old_id: lettings_log_id)
expect(lettings_log).not_to be_nil
@ -155,7 +150,7 @@ RSpec.describe Imports::LettingsLogsImportProcessor do
end
it "completes the log" do
Imports::LettingsLogsImportProcessor.new(lettings_log_xml.to_s, logger)
described_class.new(lettings_log_xml.to_s, logger)
lettings_log = LettingsLog.find_by(old_id: lettings_log_id)
expect(lettings_log.status).to eq("completed")
end
@ -179,7 +174,7 @@ RSpec.describe Imports::LettingsLogsImportProcessor do
end
it "completes the log" do
Imports::LettingsLogsImportProcessor.new(lettings_log_xml.to_s, logger)
described_class.new(lettings_log_xml.to_s, logger)
lettings_log = LettingsLog.find_by(old_id: lettings_log_id)
expect(lettings_log.status).to eq("completed")
end
@ -192,7 +187,7 @@ RSpec.describe Imports::LettingsLogsImportProcessor do
end
it "completes the log" do
Imports::LettingsLogsImportProcessor.new(lettings_log_xml.to_s, logger)
described_class.new(lettings_log_xml.to_s, logger)
lettings_log = LettingsLog.find_by(old_id: lettings_log_id)
expect(lettings_log.status).to eq("completed")
end
@ -203,7 +198,7 @@ RSpec.describe Imports::LettingsLogsImportProcessor do
it "sets the scheme and location values" do
expect(logger).not_to receive(:warn)
Imports::LettingsLogsImportProcessor.new(lettings_log_xml.to_s, logger)
described_class.new(lettings_log_xml.to_s, logger)
lettings_log = LettingsLog.find_by(old_id: lettings_log_id)
expect(lettings_log.scheme_id).not_to be_nil
@ -219,7 +214,7 @@ RSpec.describe Imports::LettingsLogsImportProcessor do
it "sets the scheme and location values" do
expect(logger).not_to receive(:warn)
Imports::LettingsLogsImportProcessor.new(lettings_log_xml.to_s, logger)
described_class.new(lettings_log_xml.to_s, logger)
lettings_log = LettingsLog.find_by(old_id: lettings_log_id)
expect(lettings_log.scheme_id).not_to be_nil
@ -228,4 +223,8 @@ RSpec.describe Imports::LettingsLogsImportProcessor do
end
end
end
def open_file(directory, filename)
File.open("#{directory}/#{filename}.xml")
end
end

32
spec/services/imports/lettings_logs_import_service_spec.rb
Unescape View File

@ -14,9 +14,15 @@ RSpec.describe Imports::LettingsLogsImportService do
let(:scheme1) { FactoryBot.create(:scheme, old_visible_id: 123, owning_organisation: organisation) }
let(:scheme2) { FactoryBot.create(:scheme, old_visible_id: 456, owning_organisation: organisation) }
def open_file(directory, filename)
File.open("#{directory}/#{filename}.xml")
end
let(:remote_folder) { "lettings_logs" }
let(:lettings_log_id) { "0ead17cb-1668-442d-898c-0d52879ff592" }
let(:lettings_log_id2) { "166fc004-392e-47a8-acb8-1c018734882b" }
let(:lettings_log_id3) { "00d2343e-d5fa-4c89-8400-ec3854b0f2b4" }
let(:lettings_log_id4) { "0b4a68df-30cc-474a-93c0-a56ce8fdad3b" }
let(:lettings_log_id5) { "893ufj2s-lq77-42m4-rty6-ej09gh585uy1" }
let(:lettings_log_id6) { "5ybz29dj-l33t-k1l0-hj86-n4k4ma77xkcd" }
let(:lettings_log_file) { open_file(fixture_directory, lettings_log_id5) }
let(:lettings_log_xml) { Nokogiri::XML(lettings_log_file) }
before do
WebMock.stub_request(:get, /api.postcodes.io\/postcodes\/LS166FT/)
@ -37,19 +43,7 @@ RSpec.describe Imports::LettingsLogsImportService do
# Stub the form handler to use the real form
allow(FormHandler.instance).to receive(:get_form).with("previous_lettings").and_return(real_2021_2022_form)
allow(FormHandler.instance).to receive(:get_form).with("current_lettings").and_return(real_2022_2023_form)
end
let(:remote_folder) { "lettings_logs" }
let(:lettings_log_id) { "0ead17cb-1668-442d-898c-0d52879ff592" }
let(:lettings_log_id2) { "166fc004-392e-47a8-acb8-1c018734882b" }
let(:lettings_log_id3) { "00d2343e-d5fa-4c89-8400-ec3854b0f2b4" }
let(:lettings_log_id4) { "0b4a68df-30cc-474a-93c0-a56ce8fdad3b" }
let(:lettings_log_id5) { "893ufj2s-lq77-42m4-rty6-ej09gh585uy1" }
let(:lettings_log_id6) { "5ybz29dj-l33t-k1l0-hj86-n4k4ma77xkcd" }
let(:lettings_log_file) { open_file(fixture_directory, lettings_log_id5) }
let(:lettings_log_xml) { Nokogiri::XML(lettings_log_file) }
before do
# Stub the S3 file listing and download
allow(storage_service).to receive(:list_files)
.and_return(%W[#{remote_folder}/#{lettings_log_id}.xml #{remote_folder}/#{lettings_log_id2}.xml #{remote_folder}/#{lettings_log_id3}.xml #{remote_folder}/#{lettings_log_id4}.xml])
@ -86,12 +80,12 @@ RSpec.describe Imports::LettingsLogsImportService do
expect(logger).not_to receive(:error)
expect(logger).not_to receive(:warn)
expect do
expect {
perform_enqueued_jobs do
lettings_log_service.create_logs(remote_folder)
lettings_log_service.create_logs(remote_folder)
end
end.to change(LettingsLog, :count).by(4) # Rather than 8
}.to change(LettingsLog, :count).by(4) # Rather than 8
end
end
@ -135,4 +129,8 @@ RSpec.describe Imports::LettingsLogsImportService do
end
end
end
def open_file(directory, filename)
File.open("#{directory}/#{filename}.xml")
end
end

Write Preview
Loading…
Cancel
Save