Move generate_logs_report to import service

app/services/imports/import_report_service.rb

@@ -1,27 +1,57 @@
 module Imports
   class ImportReportService
-    def initialize(storage_service, old_organisation_ids, logger = Rails.logger)
+    def initialize(storage_service, institutions_csv, logger = Rails.logger)
       @storage_service = storage_service
       @logger = logger
-      @old_organisation_ids = old_organisation_ids
+      @institutions_csv = institutions_csv
     end
 
     BYTE_ORDER_MARK = "\uFEFF".freeze # Required to ensure Excel always reads CSV as UTF-8
 
     def create_report(report_suffix)
       generate_missing_data_coordinators_report(report_suffix)
+      generate_logs_report(report_suffix)
     end
 
     def generate_missing_data_coordinators_report(report_suffix)
-      csv_string = "Organisation ID,Old Organisation ID,Organisation Name\n"
+      report_csv = "Organisation ID,Old Organisation ID,Organisation Name\n"
-      @old_organisation_ids.each do |old_organisation_id|
+      organisations = @institutions_csv.map { |row| Organisation.find_by(name: row[0]) }.compact
-        organisation = Organisation.find_by(old_visible_id: old_organisation_id)
+      organisations.each do |organisation|
         if organisation.users.none?(&:data_coordinator?)
-          csv_string += "#{organisation.id},#{old_organisation_id},#{organisation.name}\n"
+          report_csv += "#{organisation.id},#{organisation.old_visible_id},#{organisation.name}\n"
         end
       end
 
-      @storage_service.write_file("OrganisationsWithoutDataCoordinators_#{report_suffix}.csv", BYTE_ORDER_MARK + csv_string)
+      report_name = "OrganisationsWithoutDataCoordinators_#{report_suffix}.csv"
+      @storage_service.write_file(report_name, BYTE_ORDER_MARK + report_csv)
+
+      @logger.info("Missing data coordinators report available in s3 import bucket at #{report_name}")
+    end
+
+    def generate_logs_report(report_suffix)
+      Rails.logger.info("Generating migrated logs report")
+
+      rep = CSV.generate do |report|
+        headers = ["Institution name", "Id", "Old Completed lettings logs", "Old In progress lettings logs", "Old Completed sales logs", "Old In progress sales logs", "New Completed lettings logs", "New In Progress lettings logs", "New Completed sales logs", "New In Progress sales logs"]
+        report << headers
+
+        @institutions_csv.each do |row|
+          name = row[0]
+          organisation = Organisation.find_by(name:)
+          next unless organisation
+
+          completed_sales_logs = organisation.owned_sales_logs.where(status: "completed").count
+          in_progress_sales_logs = organisation.owned_sales_logs.where(status: "in_progress").count
+          completed_lettings_logs = organisation.owned_lettings_logs.where(status: "completed").count
+          in_progress_lettings_logs = organisation.owned_lettings_logs.where(status: "in_progress").count
+          report << row.push(completed_lettings_logs, in_progress_lettings_logs, completed_sales_logs, in_progress_sales_logs)
+        end
+      end
+
+      report_name = "MigratedLogsReport_#{report_suffix}.csv"
+      @storage_service.write_file(report_name, BYTE_ORDER_MARK + rep)
+
+      @logger.info("Logs report available in s3 import bucket at #{report_name}")
     end
   end
 end

lib/tasks/full_import.rake

@@ -36,8 +36,6 @@ namespace :import do
     end
 
     Rails.logger.info("Finished initial imports")
-    reports_service = Imports::ImportReportService.new(s3_service, csv.map { |row| row[1] })
-    Rails.logger.info("Running report generation")
   end
 
   desc "Run logs import steps"
@@ -101,34 +99,9 @@ namespace :import do
     raise "Usage: rake import:generate_report['institutions_csv_name']" if institutions_csv_name.blank?
 
     s3_service = Storage::S3Service.new(Configuration::PaasConfigurationService.new, ENV["IMPORT_PAAS_INSTANCE"])
-    csv = CSV.parse(s3_service.get_file_io(institutions_csv_name), headers: true)
+    institutions_csv = CSV.parse(s3_service.get_file_io(institutions_csv_name), headers: true)
-
-    Rails.logger.info("Generating migrated logs report")
-
-    rep = CSV.generate do |report|
-      headers = ["Institution name", "Id", "Old Completed lettings logs", "Old In progress lettings logs", "Old Completed sales logs", "Old In progress sales logs", "New Completed lettings logs", "New In Progress lettings logs", "New Completed sales logs", "New In Progress sales logs"]
-      report << headers
-
-      csv.each do |row|
-        name = row[0]
-        organisation = Organisation.find_by(name:)
-        next unless organisation
-
-        completed_sales_logs = organisation.owned_sales_logs.where(status: "completed").count
-        in_progress_sales_logs = organisation.owned_sales_logs.where(status: "in_progress").count
-        completed_lettings_logs = organisation.owned_lettings_logs.where(status: "completed").count
-        in_progress_lettings_logs = organisation.owned_lettings_logs.where(status: "in_progress").count
-        report << row.push(completed_lettings_logs, in_progress_lettings_logs, completed_sales_logs, in_progress_sales_logs)
-      end
-    end
-
-    report_name = "MigratedLogsReport_#{institutions_csv_name}"
-    s3_service.write_file(report_name, rep)
-
-    old_organisation_ids = csv.map { |row| Organisation.find_by(name: row[0]).old_visible_id }.compact
-    Imports::ImportReportService.new(s3_service, old_organisation_ids).generate_missing_data_coordinators_report(institutions_csv_name)
 
-    Rails.logger.info("Logs report available in s3 import bucket at #{report_name}")
+    Imports::ImportReportService.new(s3_service, institutions_csv).create_report(institutions_csv_name)
   end
 
   desc "Run import from logs step to end"

spec/lib/tasks/full_import_spec.rb

@@ -23,9 +23,6 @@ describe "full import", type: :task do
       Rake.application.rake_require("tasks/full_import")
       Rake::Task.define_task(:environment)
       task.reenable
-
-      create(:organisation, old_visible_id: 1, name: "org1")
-      create(:organisation, old_visible_id: 2, name: "org2")
     end
 
     context "when generating report" do
@@ -35,10 +32,10 @@ describe "full import", type: :task do
         allow(Imports::ImportReportService).to receive(:new).and_return(import_report_service)
       end
 
-      it "creates an organisation from the given XML file" do
+      it "creates a report using given organisation csv" do
         expect(Storage::S3Service).to receive(:new).with(paas_config_service, instance_name)
-        expect(Imports::ImportReportService).to receive(:new).with(storage_service, %w[1 2])
+        expect(Imports::ImportReportService).to receive(:new).with(storage_service, CSV.parse(orgs_list, headers: true))
-        expect(import_report_service).to receive(:generate_missing_data_coordinators_report).with("some_name")
+        expect(import_report_service).to receive(:create_report).with("some_name")
 
         task.invoke("some_name")
       end

spec/services/imports/import_report_service_spec.rb

@@ -1,39 +1,55 @@
 require "rails_helper"
 
 RSpec.describe Imports::ImportReportService do
-  subject(:report_service) { described_class.new(storage_service, old_organisation_ids) }
+  subject(:report_service) { described_class.new(storage_service, institutions_csv) }
 
   let(:storage_service) { instance_double(Storage::S3Service) }
 
-  context "when all organisations have data coordinators" do
+  describe "#generate_missing_data_coordinators_report" do
-    let(:organisation) { create(:organisation, old_visible_id: "1") }
+    context "when all organisations have data coordinators" do
-    let(:old_organisation_ids) { [organisation.old_visible_id] }
+      let!(:organisation) { create(:organisation, old_visible_id: "1", name: "org1") }
+      let(:institutions_csv) { CSV.parse("Institution name,Id,Old Completed lettings logs,Old In progress lettings logs,Old Completed sales logs,Old In progress sales logs\norg1,1,2,1,4,3", headers: true) }
 
-    before do
+      before do
-      create(:user, :data_coordinator, organisation:)
+        create(:user, :data_coordinator, organisation:)
+      end
+
+      it "writes an empty organisations without a data coordinators report" do
+        expect(storage_service).to receive(:write_file).with("OrganisationsWithoutDataCoordinators_report_suffix.csv", "\uFEFFOrganisation ID,Old Organisation ID,Organisation Name\n")
+
+        report_service.generate_missing_data_coordinators_report("report_suffix")
+      end
     end
 
-    it "writes an empty organisations without a data coordinators report" do
+    context "when some organisations have no data coordinators" do
-      expect(storage_service).to receive(:write_file).with("OrganisationsWithoutDataCoordinators_report_suffix.csv", "\uFEFFOrganisation ID,Old Organisation ID,Organisation Name\n")
+      let!(:organisation) { create(:organisation, old_visible_id: "1", name: "org1") }
+      let!(:organisation2) { create(:organisation, old_visible_id: "2", name: "org2") }
+      let!(:organisation3) { create(:organisation, old_visible_id: "3", name: "org3") }
+      let(:institutions_csv) { CSV.parse("Institution name,Id,Old Completed lettings logs,Old In progress lettings logs,Old Completed sales logs,Old In progress sales logs\norg1,1,2,1,4,3\norg2,2,5,6,5,7\norg3,3,5,6,5,7", headers: true) }
 
-      report_service.create_report("report_suffix")
+      before do
+        create(:user, :data_coordinator, organisation:)
+      end
+
+      it "writes an empty organisations without a data coordinators report" do
+        expect(storage_service).to receive(:write_file).with("OrganisationsWithoutDataCoordinators_report_suffix.csv", "\uFEFFOrganisation ID,Old Organisation ID,Organisation Name\n#{organisation2.id},2,#{organisation2.name}\n#{organisation3.id},3,#{organisation3.name}\n")
+
+        report_service.generate_missing_data_coordinators_report("report_suffix")
+      end
     end
   end
 
-  context "when some organisations have no data coordinators" do
+  describe "#generate_logs_report" do
-    let(:organisation) { create(:organisation, old_visible_id: "") }
+    let(:institutions_csv) { CSV.parse("Institution name,Id,Old Completed lettings logs,Old In progress lettings logs,Old Completed sales logs,Old In progress sales logs\norg1,1,2,1,4,3\norg2,2,5,6,5,7", headers: true) }
-    let(:organisation2) { create(:organisation, old_visible_id: "2") }
-    let(:organisation3) { create(:organisation, old_visible_id: "3") }
-    let(:old_organisation_ids) { [organisation.old_visible_id, organisation2.old_visible_id, organisation3.old_visible_id] }
 
     before do
-      create(:user, :data_coordinator, organisation:)
+      create(:organisation, old_visible_id: "1", name: "org1")
+      create(:organisation, old_visible_id: "2", name: "org2")
     end
 
-    it "writes an empty organisations without a data coordinators report" do
+    it "generates a report with imported logs" do
-      expect(storage_service).to receive(:write_file).with("OrganisationsWithoutDataCoordinators_report_suffix.csv", "\uFEFFOrganisation ID,Old Organisation ID,Organisation Name\n#{organisation2.id},2,#{organisation2.name}\n#{organisation3.id},3,#{organisation3.name}\n")
+      expect(storage_service).to receive(:write_file).with("MigratedLogsReport_report_suffix.csv", "\uFEFFInstitution name,Id,Old Completed lettings logs,Old In progress lettings logs,Old Completed sales logs,Old In progress sales logs,New Completed lettings logs,New In Progress lettings logs,New Completed sales logs,New In Progress sales logs\norg1,1,2,1,4,3,0,0,0,0\norg2,2,5,6,5,7,0,0,0,0\n")
-
+      report_service.generate_logs_report("report_suffix")
-      report_service.create_report("report_suffix")
     end
   end
 end