diff --git a/app/jobs/async_batch_destroy_job.rb b/app/jobs/async_batch_destroy_job.rb index 949408dbf..ceaf21d6c 100644 --- a/app/jobs/async_batch_destroy_job.rb +++ b/app/jobs/async_batch_destroy_job.rb @@ -11,18 +11,28 @@ def perform(model_class, sql_query, who_is) @model_class = model_class.constantize @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 - end + + @model_class.where(id: record_ids(sql_query)).find_in_batches(batch_size: BATCH_SIZE) do |records| + records.each do |record| + delete_record(record) end - end until scoped_records.empty? + end + end + + def record_ids(sql_query) + @model_class.find_by_sql(sql_query).pluck(:id) + end + + def delete_record(record) + unless record.destroy + Rails.logger.warn "#{@model_class} ##{record.id} can't be deleted: #{record.errors.full_messages.to_sentence}" + end + rescue ActiveRecord::InvalidForeignKey => e + Rails.logger.error error_message_for(id: record.id, error_class: e.class) end - def error_message_for(record) - "#{model_class} ##{record.id} can't be deleted: #{record.errors.full_messages.to_sentence}" + def error_message_for(id:, error_class:) + "#{@model_class} ##{id} can't be deleted: #{error_class}" end def reschedule_at(_time, _attempts) diff --git a/spec/jobs/async_batch_destroy_job_spec.rb b/spec/jobs/async_batch_destroy_job_spec.rb index 2b20916c5..177a90d6b 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,55 @@ context 'when record cannot be destroyed' do let(:model_class) { 'Dialpeer' } - let!(:dialpeers) { FactoryBot.create_list(:dialpeer, 4) } + let!(:dialpeer_with_pricelist) { FactoryBot.create(:dialpeer) } + let!(:dialpeer) { 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_with_pricelist) end let(:sql_query) { Dialpeer.all.to_sql } + let(:error_message) { "Dialpeer ##{dialpeer_with_pricelist.id} can't be deleted: Can't be deleted because linked to not applied Rate Management Pricelist(s) ##{item.pricelist_id}" } + let(:rails_logger) { Rails.logger } - 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) + before do + allow(Rails).to receive(:logger).and_return(rails_logger) + allow(rails_logger).to receive(:info) + end - dialpeers.first(2).each do |dialpeer| - expect(Dialpeer).not_to be_exists(dialpeer.id) - end + it 'should delete dialpeer that do not have related pricelist' do + expect { subject }.not_to raise_error + expect { dialpeer_with_pricelist.reload }.not_to raise_error ActiveRecord::RecordNotFound + expect { dialpeer.reload }.to raise_error ActiveRecord::RecordNotFound + end - dialpeers.last(2).each do |dialpeer| - expect(Dialpeer).to be_exists(dialpeer.id) - end + it 'should log error through Rails logger' do + expect(rails_logger).to receive(:warn).once.and_wrap_original { |_m, arg| expect(arg).to eq error_message } + subject + end + end + + context 'when Routing::Numberlist has related customer_auth' do + let(:model_class) { 'Routing::Numberlist' } + let(:sql_query) { Routing::Numberlist.where(id: [record_that_cant_be_deleted.id, record_that_can_be_deleted.id]).to_sql } + let!(:record_that_cant_be_deleted) { FactoryBot.create(:numberlist) } + let!(:record_that_can_be_deleted) { FactoryBot.create(:numberlist) } + let!(:customers_auth) { FactoryBot.create(:customers_auth, src_numberlist: record_that_cant_be_deleted) } + + it 'should remove number_lists without related Customer Auth' do + subject + + expect { record_that_can_be_deleted.reload }.to raise_error ActiveRecord::RecordNotFound + end + end + + context 'when NumberList already deleted' do + let(:model_class) { 'Routing::Numberlist' } + let(:sql_query) { Routing::Numberlist.all.to_sql } + let!(:record) { FactoryBot.create(:numberlist) } + + before { record.destroy! } + + it 'should perform job without error' do + expect { subject }.not_to raise_error end end end diff --git a/spec/models/routing/numberlist_spec.rb b/spec/models/routing/numberlist_spec.rb index d0385ef52..84b0c2c8c 100644 --- a/spec/models/routing/numberlist_spec.rb +++ b/spec/models/routing/numberlist_spec.rb @@ -149,4 +149,17 @@ end end end + + describe '.destroy!' do + subject { record.destroy! } + + context 'when record has related customer_auth' do + let!(:record) { FactoryBot.create(:numberlist) } + let!(:customers_auth) { FactoryBot.create(:customers_auth, src_numberlist: record) } + + it 'should destroy record' do + expect { subject }.to change(described_class, :count).by(-1) + end + end + end end