From 6564ca358757eb372248509f981f1daf15572079 Mon Sep 17 00:00:00 2001 From: James Rose Date: Thu, 13 Apr 2023 08:31:24 +0100 Subject: [PATCH] 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 | 10 --- .../exports/lettings_log_export_service.rb | 71 ++++++++----------- ...330084455_add_collection_to_logs_export.rb | 5 ++ db/schema.rb | 13 ++-- spec/jobs/data_export_csv_job_spec.rb | 19 ----- spec/lib/tasks/data_export_spec.rb | 8 --- .../lettings_log_export_service_spec.rb | 28 +------- 7 files changed, 42 insertions(+), 112 deletions(-) delete mode 100644 app/jobs/data_export_csv_job.rb create mode 100644 db/migrate/20230330084455_add_collection_to_logs_export.rb delete mode 100644 spec/jobs/data_export_csv_job_spec.rb diff --git a/app/jobs/data_export_csv_job.rb b/app/jobs/data_export_csv_job.rb deleted file mode 100644 index 8db19a202..000000000 --- a/app/jobs/data_export_csv_job.rb +++ /dev/null @@ -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 diff --git a/app/services/exports/lettings_log_export_service.rb b/app/services/exports/lettings_log_export_service.rb index 158d52147..1592f545d 100644 --- a/app/services/exports/lettings_log_export_service.rb +++ b/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) - export.save! + 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) + def build_export_run(collection, current_time, base_number, full_update) + previous_exports_with_data = LogsExport.where(collection:, empty_export: false) - if previous_exports_with_data.empty? - return LogsExport.new(started_at: current_time) - end - - 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("") @@ -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 diff --git a/db/migrate/20230330084455_add_collection_to_logs_export.rb b/db/migrate/20230330084455_add_collection_to_logs_export.rb new file mode 100644 index 000000000..4fa4b1000 --- /dev/null +++ b/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 diff --git a/db/schema.rb b/db/schema.rb index 34d3ed363..77ad1f3ea 100644 --- a/db/schema.rb +++ b/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" diff --git a/spec/jobs/data_export_csv_job_spec.rb b/spec/jobs/data_export_csv_job_spec.rb deleted file mode 100644 index cf06752c7..000000000 --- a/spec/jobs/data_export_csv_job_spec.rb +++ /dev/null @@ -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 diff --git a/spec/lib/tasks/data_export_spec.rb b/spec/lib/tasks/data_export_spec.rb index e1242e6b6..ba2e9928b 100644 --- a/spec/lib/tasks/data_export_spec.rb +++ b/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 diff --git a/spec/services/exports/lettings_log_export_service_spec.rb b/spec/services/exports/lettings_log_export_service_spec.rb index 6580a6acd..b2aaace1a 100644 --- a/spec/services/exports/lettings_log_export_service_spec.rb +++ b/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