Skip to content

Commit

Permalink
Update AsyncBatchDestroyJob to handle destroy failures
Browse files Browse the repository at this point in the history
The AsyncBatchDestroyJob has been refactored to handle destroy failures, instead of raising errors. A warning is logged if a destroy operation fails, improving the robustness of the job. Corresponding changes have also been made to the related test cases.
  • Loading branch information
Ivanov-Anton committed Jul 18, 2024
1 parent d80346a commit c91703c
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 22 deletions.
30 changes: 23 additions & 7 deletions app/jobs/async_batch_destroy_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/resource_dsl/acts_as_async_destroy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
22 changes: 8 additions & 14 deletions spec/jobs/async_batch_destroy_job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,33 +64,27 @@
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

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
Expand Down

0 comments on commit c91703c

Please sign in to comment.