#1276, Refactor AsyncBatchDestroyJob to increase efficiency

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

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

spec/jobs/async_batch_destroy_job_spec.rb

@@ -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