CDS: Update export functionality to allow for multiple collection years at once (#1491)

* Remove CSV export

* Add collection to lettings export

* Use staging bucket

app/jobs/data_export_csv_job.rb

@@ -1,10 +0,0 @@
-class DataExportCsvJob < ApplicationJob
-  queue_as :default
-
-  def perform
-    storage_service = Storage::S3Service.new(Configuration::PaasConfigurationService.new, ENV["EXPORT_PAAS_INSTANCE"])
-    export_service = Exports::LettingsLogExportService.new(storage_service)
-
-    export_service.export_csv_lettings_logs
-  end
-end

app/services/exports/lettings_log_export_service.rb

@@ -8,49 +8,49 @@ module Exports
       @logger = logger
     end
 
-    def export_csv_lettings_logs
-      time_str = Time.zone.now.strftime("%F").underscore
-      lettings_logs = retrieve_lettings_logs(Time.zone.now, true)
-      csv_io = build_export_csv(lettings_logs)
-      @storage_service.write_file("export_#{time_str}.csv", csv_io)
-    end
-
     def export_xml_lettings_logs(full_update: false)
       start_time = Time.zone.now
-      lettings_logs = retrieve_lettings_logs(start_time, full_update)
-      export = build_export_run(start_time, full_update)
-      daily_run = get_daily_run_number
-      archive_datetimes = write_export_archive(export, lettings_logs)
-      export.empty_export = archive_datetimes.empty?
-      write_master_manifest(daily_run, archive_datetimes)
+      logs_by_collection = retrieve_lettings_logs(start_time, full_update).group_by(&:collection_start_year)
+      daily_run_number = get_daily_run_number
+      archives_for_manifest = []
+      base_number = LogsExport.where(empty_export: false).maximum(:base_number) || 1
+      available_collection_years.each do |collection|
+        lettings_logs = logs_by_collection.fetch(collection, LettingsLog.none)
+        export = build_export_run(collection, start_time, base_number, full_update)
+        archives = write_export_archive(export, lettings_logs)
+
+        archives_for_manifest << archives if archives.any?
+        export.empty_export = archives.empty?
         export.save!
       end
 
+      write_master_manifest(daily_run_number, archives_for_manifest.flatten)
+    end
+
   private
 
     def get_daily_run_number
       today = Time.zone.today
-      LogsExport.where(created_at: today.beginning_of_day..today.end_of_day).count + 1
+      LogsExport.where(created_at: today.beginning_of_day..today.end_of_day).select(:started_at).distinct.count + 1
     end
 
-    def build_export_run(current_time, full_update)
-      previous_exports_with_data = LogsExport.where(empty_export: false)
-
-      if previous_exports_with_data.empty?
-        return LogsExport.new(started_at: current_time)
-      end
+    def build_export_run(collection, current_time, base_number, full_update)
+      previous_exports_with_data = LogsExport.where(collection:, empty_export: false)
 
-      base_number = previous_exports_with_data.maximum(:base_number)
-      increment_number = previous_exports_with_data.where(base_number:).maximum(:increment_number)
+      increment_number = previous_exports_with_data.where(base_number:).maximum(:increment_number) || 1
 
       if full_update
-        base_number += 1
+        base_number += 1 if LogsExport.any? # Only increment when it's not the first run
         increment_number = 1
       else
         increment_number += 1
       end
 
-      LogsExport.new(started_at: current_time, base_number:, increment_number:)
+      if previous_exports_with_data.empty?
+        return LogsExport.new(collection:, base_number:, started_at: current_time)
+      end
+
+      LogsExport.new(collection:, started_at: current_time, base_number:, increment_number:)
     end
 
     def write_master_manifest(daily_run, archive_datetimes)
@@ -237,23 +237,6 @@ module Exports
         !EXPORT_FIELDS.include?(field_name)
     end
 
-    def build_export_csv(lettings_logs)
-      csv_io = CSV.generate do |csv|
-        attribute_keys = nil
-        lettings_logs.each do |lettings_log|
-          attribute_hash = apply_cds_transformation(lettings_log, EXPORT_MODE[:csv])
-          if attribute_keys.nil?
-            attribute_keys = attribute_hash.keys
-            filter_keys!(attribute_keys)
-            csv << attribute_keys
-          end
-          csv << attribute_keys.map { |attribute_key| attribute_hash[attribute_key] }
-        end
-      end
-
-      StringIO.new(csv_io)
-    end
-
     def build_export_xml(lettings_logs)
       doc = Nokogiri::XML("<forms/>")
 
@@ -273,5 +256,9 @@ module Exports
 
       xml_doc_to_temp_file(doc)
     end
+
+    def available_collection_years
+      FormHandler.instance.lettings_forms.values.map { |f| f.start_date.year }.uniq
+    end
   end
 end

db/migrate/20230330084455_add_collection_to_logs_export.rb

@@ -0,0 +1,5 @@
+class AddCollectionToLogsExport < ActiveRecord::Migration[7.0]
+  def change
+    add_column :logs_exports, :collection, :string
+  end
+end

db/schema.rb

@@ -351,6 +351,7 @@ ActiveRecord::Schema[7.0].define(version: 2023_04_12_143245) do
     t.integer "base_number", default: 1, null: false
     t.integer "increment_number", default: 1, null: false
     t.boolean "empty_export", default: false, null: false
+    t.string "collection"
   end
 
   create_table "organisation_relationships", force: :cascade do |t|
@@ -518,7 +519,6 @@ ActiveRecord::Schema[7.0].define(version: 2023_04_12_143245) do
     t.integer "prevten"
     t.integer "mortgageused"
     t.integer "wchair"
-    t.integer "income2_value_check"
     t.integer "armedforcesspouse"
     t.datetime "hodate", precision: nil
     t.integer "hoday"
@@ -543,13 +543,14 @@ ActiveRecord::Schema[7.0].define(version: 2023_04_12_143245) do
     t.integer "retirement_value_check"
     t.integer "hodate_check"
     t.integer "extrabor_value_check"
+    t.integer "grant_value_check"
+    t.integer "staircase_bought_value_check"
     t.integer "deposit_and_mortgage_value_check"
     t.integer "shared_ownership_deposit_value_check"
-    t.integer "grant_value_check"
-    t.integer "value_value_check"
     t.integer "old_persons_shared_ownership_value_check"
-    t.integer "staircase_bought_value_check"
+    t.integer "income2_value_check"
     t.integer "monthly_charges_value_check"
+    t.integer "value_value_check"
     t.integer "details_known_5"
     t.integer "details_known_6"
     t.integer "saledate_check"
@@ -559,9 +560,10 @@ ActiveRecord::Schema[7.0].define(version: 2023_04_12_143245) do
     t.integer "ethnicbuy2"
     t.integer "proplen_asked"
     t.string "old_id"
+    t.integer "pregblank"
     t.integer "buy2living"
     t.integer "prevtenbuy2"
-    t.integer "pregblank"
+    t.integer "nationalbuy2"
     t.string "uprn"
     t.integer "uprn_known"
     t.integer "uprn_confirmed"
@@ -569,7 +571,6 @@ ActiveRecord::Schema[7.0].define(version: 2023_04_12_143245) do
     t.string "address_line2"
     t.string "town_or_city"
     t.string "county"
-    t.integer "nationalbuy2"
     t.integer "discounted_sale_value_check"
     t.integer "student_not_child_value_check"
     t.index ["bulk_upload_id"], name: "index_sales_logs_on_bulk_upload_id"

spec/jobs/data_export_csv_job_spec.rb

@@ -1,19 +0,0 @@
-require "rails_helper"
-
-describe DataExportCsvJob do
-  let(:storage_service) { instance_double(Storage::S3Service) }
-  let(:paas_config_service) { instance_double(Configuration::PaasConfigurationService) }
-  let(:export_service) { instance_double(Exports::LettingsLogExportService) }
-
-  before do
-    allow(Storage::S3Service).to receive(:new).and_return(storage_service)
-    allow(Configuration::PaasConfigurationService).to receive(:new).and_return(paas_config_service)
-    allow(Exports::LettingsLogExportService).to receive(:new).and_return(export_service)
-  end
-
-  it "calls the export service" do
-    expect(export_service).to receive(:export_csv_lettings_logs)
-
-    described_class.perform_now
-  end
-end

spec/lib/tasks/data_export_spec.rb

@@ -26,12 +26,4 @@ describe "rake core:data_export", type: task do
       expect { task.invoke }.to enqueue_job(DataExportXmlJob)
     end
   end
-
-  context "when exporting lettings logs with CSV format" do
-    let(:task) { Rake::Task["core:data_export_csv"] }
-
-    it "starts the CSV export process" do
-      expect { task.invoke }.to enqueue_job(DataExportCsvJob)
-    end
-  end
 end

spec/services/exports/lettings_log_export_service_spec.rb

@@ -217,7 +217,7 @@ RSpec.describe Exports::LettingsLogExportService do
 
       it "creates a logs export record in a database with correct time" do
         expect { export_service.export_xml_lettings_logs }
-          .to change(LogsExport, :count).by(1)
+          .to change(LogsExport, :count).by(3)
         expect(LogsExport.last.started_at).to eq(start_time)
       end
 
@@ -294,32 +294,6 @@ RSpec.describe Exports::LettingsLogExportService do
         expect(LogsExport.last.increment_number).to eq(1)
       end
     end
-
-    context "and the export has an error" do
-      before { allow(storage_service).to receive(:write_file).and_raise(StandardError.new("This is an exception")) }
-
-      it "does not save a record in the database" do
-        expect { export_service.export_xml_lettings_logs }
-          .to raise_error(StandardError)
-                .and(change(LogsExport, :count).by(0))
-      end
-    end
-  end
-
-  context "when exporting a general needs lettings logs in CSV" do
-    let(:csv_export_file) { File.open("spec/fixtures/exports/general_needs_log.csv", "r:UTF-8") }
-    let(:expected_csv_filename) { "export_2022_05_01.csv" }
-
-    let(:lettings_log) { FactoryBot.create(:lettings_log, :completed, propcode: "123", ppostcode_full: "SE2 6RT", postcode_full: "NW1 5TY", tenancycode: "BZ737", startdate: Time.zone.local(2022, 2, 2, 10, 36, 49), voiddate: Time.zone.local(2019, 11, 3), mrcdate: Time.zone.local(2020, 5, 5, 10, 36, 49), tenancylength: 5, underoccupation_benefitcap: 4) }
-
-    it "generates an CSV export file with the expected content" do
-      expected_content = replace_entity_ids(lettings_log, csv_export_file.read)
-      expect(storage_service).to receive(:write_file).with(expected_csv_filename, any_args) do |_, content|
-        expect(content).not_to be_nil
-        expect(content.read).to eq(expected_content)
-      end
-      export_service.export_csv_lettings_logs
-    end
   end
 
   context "when exporting a supporting housing lettings logs in XML" do