From baa9a295fb9ae13acc8261b1065e657a3d71fc10 Mon Sep 17 00:00:00 2001 From: Phil Lee Date: Wed, 29 Mar 2023 17:06:57 +0100 Subject: [PATCH] work invisible logs into bulk upload flow --- app/models/bulk_upload.rb | 6 + app/services/bulk_upload/processor.rb | 33 ++--- spec/services/bulk_upload/processor_spec.rb | 140 +++++++++++++------- spec/support/bulk_upload/log_to_csv.rb | 2 +- 4 files changed, 108 insertions(+), 73 deletions(-) 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 b6625b2cf..9a067e539 100644 --- a/spec/services/bulk_upload/processor_spec.rb +++ b/spec/services/bulk_upload/processor_spec.rb @@ -130,61 +130,72 @@ 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, + voiddate: nil, + mrcdate: nil, + startdate: Date.new(2022, 10, 1), + tenancylength: nil, ) 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) 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(:file) { Tempfile.new } - let(:path) { file.path } - + context "when a bulk upload has an in progress log" do let(:mock_downloader) do instance_double( BulkUpload::Downloader, @@ -194,24 +205,17 @@ RSpec.describe BulkUpload::Processor do ) end + let(:file) { Tempfile.new } + let(:path) { file.path } + let(:log) do - build( - :lettings_log, - :completed, + LettingsLog.new( + lettype: 2, 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, - voiddate: nil, - mrcdate: nil, - startdate: Date.new(2022, 10, 1), - tenancylength: nil, + startdate: Time.zone.local(2022, 10, 1), + renewal: 2, ) end @@ -222,26 +226,64 @@ RSpec.describe BulkUpload::Processor do allow(BulkUpload::Downloader).to receive(:new).with(bulk_upload:).and_return(mock_downloader) end - it "creates logs" do - expect { processor.call }.to change(LettingsLog, :count).by(1) + it "creates invisible log" do + expect { processor.call }.to change(LettingsLog.rewhere(visible: false), :count).by(1) end - it "does not send fix errors email" do - allow(BulkUploadMailer).to receive(:send_bulk_upload_with_errors_mail).and_call_original + 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).not_to have_received(:send_bulk_upload_with_errors_mail) + expect(BulkUploadMailer).to have_received(:send_how_fix_upload_mail) + expect(mail_double).to have_received(:deliver_later) end + end - it "sends success email" do + context "when upload has no setup errors something blocks log creation" do + let(:mock_downloader) do + instance_double( + BulkUpload::Downloader, + call: nil, + path:, + delete_local_file!: nil, + ) + end + + let(:file) { Tempfile.new } + let(:path) { file.path } + + let(:other_user) { create(:user) } + + 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 + + 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 "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) + 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,