From 20321fa66bf53c1c9b27b05b232541f28ae47291 Mon Sep 17 00:00:00 2001 From: Anton-Ivanov Date: Thu, 18 Jul 2024 10:08:02 +0300 Subject: [PATCH] within batch deletion job skip records that is used by another table --- app/jobs/async_batch_destroy_job.rb | 30 +++++++++++++++++------ lib/resource_dsl/acts_as_async_destroy.rb | 2 +- spec/jobs/async_batch_destroy_job_spec.rb | 22 ++++++----------- 3 files changed, 32 insertions(+), 22 deletions(-) diff --git a/app/jobs/async_batch_destroy_job.rb b/app/jobs/async_batch_destroy_job.rb index 949408dbf..66cea7f2f 100644 --- a/app/jobs/async_batch_destroy_job.rb +++ b/app/jobs/async_batch_destroy_job.rb @@ -5,26 +5,42 @@ class AsyncBatchDestroyJob < ApplicationJob BATCH_SIZE = 1000 queue_as 'batch_actions' - attr_reader :model_class, :who_is + attr_reader :model_class, :who_is, :sql_query def perform(model_class, sql_query, who_is) @model_class = model_class.constantize + @sql_query = sql_query @who_is = who_is set_audit_log_data - begin - scoped_records = @model_class.find_by_sql(sql_query + " LIMIT #{BATCH_SIZE}") - @model_class.transaction do - scoped_records.each do |record| - raise ActiveRecord::RecordNotDestroyed.new(error_message_for(record), record) unless record.destroy + @model_class.transaction do + relation.find_in_batches(batch_size: BATCH_SIZE) do |records| + records.each do |record| + unless record.destroy + logger.warn { error_message_for(record) } + next + end end end - end until scoped_records.empty? + end + end + + def relation + if @sql_query.present? && extract_where_clause(@sql_query).present? + @model_class.where(extract_where_clause(sql_query)) + else + @model_class.all + end end def error_message_for(record) "#{model_class} ##{record.id} can't be deleted: #{record.errors.full_messages.to_sentence}" end + def extract_where_clause(sql_query) + where_clause_match = sql_query.match(/WHERE\s+(.+)\z/i) + where_clause_match ? where_clause_match[1].strip : nil + end + def reschedule_at(_time, _attempts) 10.seconds.from_now end diff --git a/lib/resource_dsl/acts_as_async_destroy.rb b/lib/resource_dsl/acts_as_async_destroy.rb index dbcac7b57..a05cc71ed 100644 --- a/lib/resource_dsl/acts_as_async_destroy.rb +++ b/lib/resource_dsl/acts_as_async_destroy.rb @@ -10,7 +10,7 @@ def acts_as_async_destroy(model_class) scoped_collection_action :async_destroy, title: 'Delete batch', if: proc { authorized?(:batch_destroy, resource_klass) } do - AsyncBatchDestroyJob.perform_later(model_class, scoped_collection_records.except(:eager_load).to_sql, @paper_trail_info) + AsyncBatchDestroyJob.perform_later(model_class, scoped_collection_records.except(:eager_load, :preload).to_sql, @paper_trail_info) flash[:notice] = I18n.t('flash.actions.batch_actions.batch_destroy.job_scheduled') head 200 end diff --git a/spec/jobs/async_batch_destroy_job_spec.rb b/spec/jobs/async_batch_destroy_job_spec.rb index 2b20916c5..d0b45bcc6 100644 --- a/spec/jobs/async_batch_destroy_job_spec.rb +++ b/spec/jobs/async_batch_destroy_job_spec.rb @@ -64,9 +64,9 @@ let(:sql_query) { Contractor.all.to_sql } it 'error performed job' do expect do - expect { subject }.to raise_error(ActiveRecord::RecordNotDestroyed) + expect { subject }.not_to raise_error end.to change(LogicLog, :count).by 1 - expect(LogicLog.last.msg).to start_with 'Error' + expect(LogicLog.last.msg).to start_with 'Success' end end end @@ -74,23 +74,17 @@ context 'when record cannot be destroyed' do let(:model_class) { 'Dialpeer' } - let!(:dialpeers) { FactoryBot.create_list(:dialpeer, 4) } + let!(:dialpeer_free) { FactoryBot.create(:dialpeer) } + let!(:dialpeer_used) { FactoryBot.create(:dialpeer) } let!(:item) do - FactoryBot.create(:rate_management_pricelist_item, :with_pricelist, :filed_from_project, dialpeer: dialpeers.last) + FactoryBot.create(:rate_management_pricelist_item, :with_pricelist, :filed_from_project, dialpeer: dialpeer_used) end let(:sql_query) { Dialpeer.all.to_sql } it 'should raise validation error' do - error_message = "Dialpeer ##{dialpeers.last.id} can't be deleted: Can't be deleted because linked to not applied Rate Management Pricelist(s) ##{item.pricelist_id}" - expect { subject }.to raise_error(ActiveRecord::RecordNotDestroyed, error_message) - - dialpeers.first(2).each do |dialpeer| - expect(Dialpeer).not_to be_exists(dialpeer.id) - end - - dialpeers.last(2).each do |dialpeer| - expect(Dialpeer).to be_exists(dialpeer.id) - end + expect { subject }.not_to raise_error + expect { dialpeer_free.reload }.to raise_error(ActiveRecord::RecordNotFound) + expect { dialpeer_used.reload }.not_to raise_error end end end