diff --git a/app/models/bulk_upload.rb b/app/models/bulk_upload.rb index 0952b60af..556ee4b4f 100644 --- a/app/models/bulk_upload.rb +++ b/app/models/bulk_upload.rb @@ -60,6 +60,12 @@ class BulkUpload < ApplicationRecord "BulkUpload::#{type_class}::#{year_class}".constantize end + def make_logs_visible + logs + .rewhere(visible: false) + .update_all(visible: true) + end + private def generate_identifier diff --git a/app/services/bulk_upload/processor.rb b/app/services/bulk_upload/processor.rb index 9a5efbeda..dff2e6d49 100644 --- a/app/services/bulk_upload/processor.rb +++ b/app/services/bulk_upload/processor.rb @@ -15,15 +15,16 @@ class BulkUpload::Processor if validator.any_setup_errors? send_setup_errors_mail elsif validator.create_logs? - if bulk_upload.bulk_upload_errors.count.zero? - create_visible_logs + create_invisible_logs - send_fix_errors_mail if created_logs_but_incompleted? - send_success_mail if created_logs_and_all_completed? - else - create_invisible_logs + if created_logs_but_incompleted? send_how_fix_upload_mail end + + if created_logs_and_all_completed? + bulk_upload.make_logs_visible + send_success_mail + end else send_correct_and_upload_again_mail # summary/full report end @@ -35,20 +36,13 @@ class BulkUpload::Processor end def approve - make_logs_visible + bulk_upload.make_logs_visible ensure downloader.delete_local_file! end private - def make_logs_visible - bulk_upload - .logs - .rewhere(visible: false) - .update_all(visible: true) - end - def send_how_fix_upload_mail BulkUploadMailer .send_how_fix_upload_mail(bulk_upload:) @@ -80,11 +74,11 @@ private end def created_logs_but_incompleted? - validator.create_logs? && bulk_upload.logs.where.not(status: %w[completed]).count.positive? + bulk_upload.logs.rewhere(visible: false).where.not(status: %w[completed]).count.positive? end def created_logs_and_all_completed? - validator.create_logs? && bulk_upload.logs.group(:status).count.keys == %w[completed] + bulk_upload.logs.rewhere(visible: false).group(:status).count.keys == %w[completed] end def send_failure_mail(errors: []) @@ -97,13 +91,6 @@ private bulk_upload.user end - def create_visible_logs - log_creator_class.new( - bulk_upload:, - path: downloader.path, - ).call - end - def create_invisible_logs log_creator_class.new( bulk_upload:, diff --git a/spec/services/bulk_upload/processor_spec.rb b/spec/services/bulk_upload/processor_spec.rb index f6c6e6f2f..1bca12d3b 100644 --- a/spec/services/bulk_upload/processor_spec.rb +++ b/spec/services/bulk_upload/processor_spec.rb @@ -3,7 +3,9 @@ require "rails_helper" RSpec.describe BulkUpload::Processor do subject(:processor) { described_class.new(bulk_upload:) } - let(:bulk_upload) { create(:bulk_upload, :lettings) } + let(:bulk_upload) { create(:bulk_upload, :lettings, user:) } + let(:user) { create(:user, organisation: owning_org) } + let(:owning_org) { create(:organisation, old_visible_id: 123) } describe "#call" do context "when the bulk upload itself is not considered valid" do @@ -128,60 +130,70 @@ RSpec.describe BulkUpload::Processor do end end - context "when processing a bulk upload with errors but below threshold (therefore creates logs)" do + context "when processing a bulk with perfect data" do let(:mock_downloader) do instance_double( BulkUpload::Downloader, call: nil, - path: file_fixture("2022_23_lettings_bulk_upload.csv"), + path:, delete_local_file!: nil, ) end - let(:mock_validator) do - instance_double( - BulkUpload::Lettings::Validator, - invalid?: false, - call: nil, - any_setup_errors?: false, - create_logs?: true, + let(:file) { Tempfile.new } + let(:path) { file.path } + + let(:log) do + build( + :lettings_log, + :completed, + renttype: 3, + age1: 20, + owning_organisation: owning_org, + managing_organisation: owning_org, + created_by: nil, + national: 18, + waityear: 9, + joint: 2, + tenancy: 9, + ppcodenk: 0, ) end before do + file.write(BulkUpload::LogToCsv.new(log:, col_offset: 0).to_2022_csv_row) + file.rewind + allow(BulkUpload::Downloader).to receive(:new).with(bulk_upload:).and_return(mock_downloader) - allow(BulkUpload::Lettings::Validator).to receive(:new).and_return(mock_validator) end - it "deletes the local file afterwards" do - processor.call - - expect(mock_downloader).to have_received(:delete_local_file!) + it "creates logs" do + expect { processor.call }.to change(LettingsLog, :count).by(1) end - it "sends fix errors email" do - mail_double = instance_double("ActionMailer::MessageDelivery", deliver_later: nil) - - allow(BulkUploadMailer).to receive(:send_bulk_upload_with_errors_mail).and_return(mail_double) + it "makes logs visible" do + allow(bulk_upload).to receive(:make_logs_visible).and_call_original processor.call - expect(BulkUploadMailer).to have_received(:send_bulk_upload_with_errors_mail) - expect(mail_double).to have_received(:deliver_later) + expect(bulk_upload).to have_received(:make_logs_visible) end - it "does not send success email" do - allow(BulkUploadMailer).to receive(:send_bulk_upload_complete_mail).and_call_original + it "sends success email" do + mail_double = instance_double("ActionMailer::MessageDelivery", deliver_later: nil) + + allow(BulkUploadMailer).to receive(:send_bulk_upload_complete_mail).and_return(mail_double) + + create(:lettings_log, :completed, bulk_upload:) processor.call - expect(BulkUploadMailer).not_to have_received(:send_bulk_upload_complete_mail) + expect(BulkUploadMailer).to have_received(:send_bulk_upload_complete_mail) + expect(mail_double).to have_received(:deliver_later) end end - context "when processing a bulk with perfect data" do - let(:path) { file_fixture("2022_23_lettings_bulk_upload.csv") } - + context "when a bulk upload has an in progress log" do let(:mock_downloader) do instance_double( BulkUpload::Downloader, @@ -191,54 +203,85 @@ RSpec.describe BulkUpload::Processor do ) end - let(:mock_validator) do - instance_double( - BulkUpload::Lettings::Validator, - call: nil, - create_logs?: true, - any_setup_errors?: false, - invalid?: false, + let(:file) { Tempfile.new } + let(:path) { file.path } + + let(:log) do + LettingsLog.new( + lettype: 2, + renttype: 3, + owning_organisation: owning_org, + managing_organisation: owning_org, + startdate: Time.zone.local(2022, 10, 1), + renewal: 2, ) end - let(:mock_creator) do + before do + file.write(BulkUpload::LogToCsv.new(log:, col_offset: 0).to_2022_csv_row) + file.rewind + + allow(BulkUpload::Downloader).to receive(:new).with(bulk_upload:).and_return(mock_downloader) + end + + it "creates invisible log" do + expect { processor.call }.to change(LettingsLog.rewhere(visible: false), :count).by(1) + end + + it "sends how_fix_upload_mail" do + mail_double = instance_double("ActionMailer::MessageDelivery", deliver_later: nil) + + allow(BulkUploadMailer).to receive(:send_how_fix_upload_mail).and_return(mail_double) + + processor.call + + expect(BulkUploadMailer).to have_received(:send_how_fix_upload_mail) + expect(mail_double).to have_received(:deliver_later) + end + end + + context "when upload has no setup errors something blocks log creation" do + let(:mock_downloader) do instance_double( - BulkUpload::Lettings::LogCreator, + BulkUpload::Downloader, call: nil, path:, + delete_local_file!: nil, ) end - before do - allow(BulkUpload::Downloader).to receive(:new).with(bulk_upload:).and_return(mock_downloader) - allow(BulkUpload::Lettings::Validator).to receive(:new).and_return(mock_validator) - allow(BulkUpload::Lettings::LogCreator).to receive(:new).with(bulk_upload:, path:).and_return(mock_creator) - end + let(:file) { Tempfile.new } + let(:path) { file.path } - it "creates logs" do - processor.call + let(:other_user) { create(:user) } - expect(mock_creator).to have_received(:call) + let(:log) do + LettingsLog.new( + lettype: 2, + renttype: 3, + owning_organisation: owning_org, + managing_organisation: owning_org, + startdate: Time.zone.local(2022, 10, 1), + renewal: 2, + created_by: other_user, # unaffiliated user + ) end - it "does not send fix errors email" do - allow(BulkUploadMailer).to receive(:send_bulk_upload_with_errors_mail).and_call_original - - processor.call + before do + file.write(BulkUpload::LogToCsv.new(log:, col_offset: 0).to_2022_csv_row) + file.rewind - expect(BulkUploadMailer).not_to have_received(:send_bulk_upload_with_errors_mail) + allow(BulkUpload::Downloader).to receive(:new).with(bulk_upload:).and_return(mock_downloader) end - it "sends success email" do + it "sends correct_and_upload_again_mail" do mail_double = instance_double("ActionMailer::MessageDelivery", deliver_later: nil) - allow(BulkUploadMailer).to receive(:send_bulk_upload_complete_mail).and_return(mail_double) - - create(:lettings_log, :completed, bulk_upload:) + allow(BulkUploadMailer).to receive(:send_correct_and_upload_again_mail).and_return(mail_double) processor.call - expect(BulkUploadMailer).to have_received(:send_bulk_upload_complete_mail) + expect(BulkUploadMailer).to have_received(:send_correct_and_upload_again_mail) expect(mail_double).to have_received(:deliver_later) end end diff --git a/spec/support/bulk_upload/log_to_csv.rb b/spec/support/bulk_upload/log_to_csv.rb index a2d8c2f5f..55a199e7f 100644 --- a/spec/support/bulk_upload/log_to_csv.rb +++ b/spec/support/bulk_upload/log_to_csv.rb @@ -170,7 +170,7 @@ class BulkUpload::LogToCsv nil, # 110 log.owning_organisation&.old_visible_id, - nil, + log.created_by&.email, log.managing_organisation&.old_visible_id, leftreg, nil,