work invisible logs into bulk upload flow

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

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
-
-        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:,

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,
-        )
-      end
+      let(:file) { Tempfile.new }
+      let(:path) { file.path }
 
-      let(:mock_creator) do
-        instance_double(
-          BulkUpload::Lettings::LogCreator,
-          call: nil,
-          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
 
       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)
-        allow(BulkUpload::Lettings::LogCreator).to receive(:new).with(bulk_upload:, path:).and_return(mock_creator)
       end
 
-      it "creates logs" do
+      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(mock_creator).to have_received(:call)
+        expect(BulkUploadMailer).to have_received(:send_how_fix_upload_mail)
+        expect(mail_double).to have_received(:deliver_later)
+      end
     end
 
-      it "does not send fix errors email" do
-        allow(BulkUploadMailer).to receive(:send_bulk_upload_with_errors_mail).and_call_original
+    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
 
-        processor.call
+      let(:file) { Tempfile.new }
+      let(:path) { file.path }
 
-        expect(BulkUploadMailer).not_to have_received(:send_bulk_upload_with_errors_mail)
+      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
 
-      it "sends success email" do
-        mail_double = instance_double("ActionMailer::MessageDelivery", deliver_later: nil)
+      before do
+        file.write(BulkUpload::LogToCsv.new(log:, col_offset: 0).to_2022_csv_row)
+        file.rewind
 
-        allow(BulkUploadMailer).to receive(:send_bulk_upload_complete_mail).and_return(mail_double)
+        allow(BulkUpload::Downloader).to receive(:new).with(bulk_upload:).and_return(mock_downloader)
+      end
 
-        create(:lettings_log, :completed, bulk_upload:)
+      it "sends correct_and_upload_again_mail" do
+        mail_double = instance_double("ActionMailer::MessageDelivery", deliver_later: nil)
+
+        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

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,