From 5d9dde3ec04ecd17baa8e6a352599725bd21c3ae Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Tue, 12 Nov 2024 03:57:06 -0500 Subject: [PATCH] Add age/expiry duration constants to `BulkImport` class (#32839) --- app/lib/vacuum/imports_vacuum.rb | 10 ++++++-- app/models/bulk_import.rb | 6 +++++ spec/models/bulk_import_spec.rb | 42 ++++++++++++++++++++++++++++++++ 3 files changed, 56 insertions(+), 2 deletions(-) create mode 100644 spec/models/bulk_import_spec.rb diff --git a/app/lib/vacuum/imports_vacuum.rb b/app/lib/vacuum/imports_vacuum.rb index b67865194f..7f101c4506 100644 --- a/app/lib/vacuum/imports_vacuum.rb +++ b/app/lib/vacuum/imports_vacuum.rb @@ -9,10 +9,16 @@ class Vacuum::ImportsVacuum private def clean_unconfirmed_imports! - BulkImport.state_unconfirmed.where(created_at: ..10.minutes.ago).in_batches.delete_all + BulkImport + .confirmation_missed + .in_batches + .delete_all end def clean_old_imports! - BulkImport.where(created_at: ..1.week.ago).in_batches.delete_all + BulkImport + .archival_completed + .in_batches + .delete_all end end diff --git a/app/models/bulk_import.rb b/app/models/bulk_import.rb index 9b3f4c8a3a..e3e46d7b1c 100644 --- a/app/models/bulk_import.rb +++ b/app/models/bulk_import.rb @@ -21,6 +21,9 @@ class BulkImport < ApplicationRecord self.inheritance_column = false + ARCHIVE_PERIOD = 1.week + CONFIRM_PERIOD = 10.minutes + belongs_to :account has_many :rows, class_name: 'BulkImportRow', inverse_of: :bulk_import, dependent: :delete_all @@ -42,6 +45,9 @@ class BulkImport < ApplicationRecord validates :type, presence: true + scope :archival_completed, -> { where(created_at: ..ARCHIVE_PERIOD.ago) } + scope :confirmation_missed, -> { state_unconfirmed.where(created_at: ..CONFIRM_PERIOD.ago) } + def self.progress!(bulk_import_id, imported: false) # Use `increment_counter` so that the incrementation is done atomically in the database BulkImport.increment_counter(:processed_items, bulk_import_id) diff --git a/spec/models/bulk_import_spec.rb b/spec/models/bulk_import_spec.rb new file mode 100644 index 0000000000..a3bd01d2a8 --- /dev/null +++ b/spec/models/bulk_import_spec.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe BulkImport do + describe 'Associations' do + it { is_expected.to belong_to(:account).required } + it { is_expected.to have_many(:rows).class_name('BulkImportRow').inverse_of(:bulk_import).dependent(:delete_all) } + end + + describe 'Validations' do + subject { Fabricate.build :bulk_import } + + it { is_expected.to validate_presence_of(:type) } + end + + describe 'Scopes' do + describe '.archival_completed' do + let!(:old_import) { Fabricate :bulk_import, created_at: 1.month.ago } + let!(:new_import) { Fabricate :bulk_import, created_at: 1.day.ago } + + it 'returns imports which have passed the archive window period' do + expect(described_class.archival_completed) + .to include(old_import) + .and not_include(new_import) + end + end + + describe '.confirmation_missed' do + let!(:old_unconfirmed_import) { Fabricate :bulk_import, created_at: 1.week.ago, state: :unconfirmed } + let!(:old_scheduled_import) { Fabricate :bulk_import, created_at: 1.week.ago, state: :scheduled } + let!(:new_unconfirmed_import) { Fabricate :bulk_import, created_at: 1.minute.ago, state: :unconfirmed } + + it 'returns imports which have passed the confirmation window without confirming' do + expect(described_class.confirmation_missed) + .to include(old_unconfirmed_import) + .and not_include(old_scheduled_import) + .and not_include(new_unconfirmed_import) + end + end + end +end