From 3c8959e4b2e2c384cb13c9e67056bbea1495fd3b Mon Sep 17 00:00:00 2001 From: Ted-U <92022120+Ted-U@users.noreply.github.com> Date: Tue, 17 May 2022 14:40:42 +0100 Subject: [PATCH] Export improvements (#581) * improved export - set start point for reference * remove completed status from exports * Added exception handling to export * Improved tests, replaced rescue block with Sentry Co-authored-by: Kat --- .../exports/case_log_export_service.rb | 22 ++++++---- db/migrate/20220516111514_add_started_at.rb | 5 +++ db/schema.rb | 3 +- .../exports/case_log_export_service_spec.rb | 41 +++++++++++++++++++ 4 files changed, 63 insertions(+), 8 deletions(-) create mode 100644 db/migrate/20220516111514_add_started_at.rb diff --git a/app/services/exports/case_log_export_service.rb b/app/services/exports/case_log_export_service.rb index 9b786d28e..e8aca786e 100644 --- a/app/services/exports/case_log_export_service.rb +++ b/app/services/exports/case_log_export_service.rb @@ -6,10 +6,12 @@ module Exports end def export_case_logs - case_logs = retrieve_case_logs - export = save_export_run + current_time = Time.zone.now + case_logs = retrieve_case_logs(current_time) + export = save_export_run(current_time) write_master_manifest(export) write_export_data(case_logs) + export.save! end def is_omitted_field?(field_name) @@ -22,14 +24,14 @@ module Exports private - def save_export_run + def save_export_run(current_time) today = Time.zone.today last_daily_run_number = LogsExport.where(created_at: today.beginning_of_day..today.end_of_day).maximum(:daily_run_number) last_daily_run_number = 0 if last_daily_run_number.nil? export = LogsExport.new export.daily_run_number = last_daily_run_number + 1 - export.save! + export.started_at = current_time export end @@ -49,9 +51,15 @@ module Exports @storage_service.write_file(file_path, string_io) end - def retrieve_case_logs - params = { from: Time.current.beginning_of_day, to: Time.current, status: CaseLog.statuses[:completed] } - CaseLog.where("updated_at >= :from and updated_at <= :to and status = :status", params) + def retrieve_case_logs(current_time) + recent_export = LogsExport.order("started_at").last + if recent_export + params = { from: recent_export.started_at, to: current_time } + CaseLog.where("updated_at >= :from and updated_at <= :to", params) + else + params = { to: current_time } + CaseLog.where("updated_at <= :to", params) + end end def build_manifest_csv_io diff --git a/db/migrate/20220516111514_add_started_at.rb b/db/migrate/20220516111514_add_started_at.rb new file mode 100644 index 000000000..1eedcf22a --- /dev/null +++ b/db/migrate/20220516111514_add_started_at.rb @@ -0,0 +1,5 @@ +class AddStartedAt < ActiveRecord::Migration[7.0] + def change + add_column :logs_exports, :started_at, :datetime + end +end diff --git a/db/schema.rb b/db/schema.rb index 240e514df..be67fb5f8 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.0].define(version: 2022_05_13_123456) do +ActiveRecord::Schema[7.0].define(version: 2022_05_16_111514) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -261,6 +261,7 @@ ActiveRecord::Schema[7.0].define(version: 2022_05_13_123456) do create_table "logs_exports", force: :cascade do |t| t.integer "daily_run_number" t.datetime "created_at", default: -> { "CURRENT_TIMESTAMP" } + t.datetime "started_at" end create_table "organisation_las", force: :cascade do |t| diff --git a/spec/services/exports/case_log_export_service_spec.rb b/spec/services/exports/case_log_export_service_spec.rb index 4fb431df4..3f4007c70 100644 --- a/spec/services/exports/case_log_export_service_spec.rb +++ b/spec/services/exports/case_log_export_service_spec.rb @@ -46,10 +46,17 @@ RSpec.describe Exports::CaseLogExportService do end context "and case logs are available for export" do + let(:time_now) { Time.zone.now } + before do + Timecop.freeze(time_now) case_log end + after do + LogsExport.destroy_all + end + it "generates an XML export file with the expected filename" do expect(storage_service).to receive(:write_file).with(expected_data_filename, any_args) export_service.export_case_logs @@ -63,6 +70,30 @@ RSpec.describe Exports::CaseLogExportService do export_service.export_case_logs expect(actual_content).to eq(expected_content) end + + it "creates a logs export record in a database with correct time" do + export_service.export_case_logs + records_from_db = ActiveRecord::Base.connection.execute("select started_at, id from logs_exports ").to_a + expect(records_from_db[0]["started_at"]).to eq(time_now) + expect(records_from_db.count).to eq(1) + end + + it "gets the logs for correct timeframe" do + start_time = Time.zone.local(2022, 4, 13, 2, 2, 2) + export = LogsExport.new(started_at: start_time, daily_run_number: 1) + export.save! + params = { from: start_time, to: time_now } + allow(CaseLog).to receive(:where).with("updated_at >= :from and updated_at <= :to", params).once.and_return([]) + export_service.export_case_logs + end + + context "when this is the first export" do + it "gets the logs for the timeframe up until the current time" do + params = { to: time_now } + allow(CaseLog).to receive(:where).with("updated_at <= :to", params).once.and_return([]) + export_service.export_case_logs + end + end end context "and a previous export has run the same day" do @@ -75,5 +106,15 @@ RSpec.describe Exports::CaseLogExportService do export_service.export_case_logs end end + + context "when export has an error" do + it "does not save a record in the database" do + allow(storage_service).to receive(:write_file).and_raise(StandardError.new("This is an exception")) + export = LogsExport.new + allow(LogsExport).to receive(:new).and_return(export) + expect(export).not_to receive(:save!) + expect { export_service.export_case_logs }.to raise_error(StandardError) + end + end end end