From 9435da0a36ada863ef315a9e28f82f9b9b0c410b Mon Sep 17 00:00:00 2001
From: Rolf Schmidt <rolf.schmidt@zammad.com>
Date: Tue, 30 Nov 2021 14:04:17 +0100
Subject: [PATCH] Fixes #3871 - Wrong SLA is used (alphabetical order is
 ignored).

---
 app/models/sla.rb                             |  2 +-
 .../initializers/active_record_as_batches.rb  | 51 +++++++++++++++++++
 spec/lib/core_ext/relation/as_batches_spec.rb | 41 +++++++++++++++
 spec/models/sla_spec.rb                       | 14 +++++
 4 files changed, 107 insertions(+), 1 deletion(-)
 create mode 100644 config/initializers/active_record_as_batches.rb
 create mode 100644 spec/lib/core_ext/relation/as_batches_spec.rb

diff --git a/app/models/sla.rb b/app/models/sla.rb
index 6575230f4..50225f5ec 100644
--- a/app/models/sla.rb
+++ b/app/models/sla.rb
@@ -25,7 +25,7 @@ class Sla < ApplicationModel
 
   def self.for_ticket(ticket)
     fallback = nil
-    all.order(:name, :created_at).find_each do |record|
+    all.order(:name, :created_at).as_batches(size: 10) do |record|
       if record.condition.present?
         return record if record.condition_matches?(ticket)
       else
diff --git a/config/initializers/active_record_as_batches.rb b/config/initializers/active_record_as_batches.rb
new file mode 100644
index 000000000..c249dfeb8
--- /dev/null
+++ b/config/initializers/active_record_as_batches.rb
@@ -0,0 +1,51 @@
+# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/
+
+# https://github.com/telent/ar-as-batches
+# TODO: Should be reconsidered with rails 6.1 because then
+# find_each might be able to handle order as well
+# e.g. Ticket::Priority.order(updated: :desc).find_each... is not possbile atm with find_each
+module ActiveRecord
+  module AsBatches
+    class Batch
+      def initialize(arel, args)
+        @offset = arel.offset || 0
+        @limit  = arel.limit
+        @size   = args[:size] || 100
+        return if !@limit || (@limit > @size)
+
+        @size = @limit
+      end
+
+      def get_records(query)
+        query.offset(@offset).limit(@size).all
+      end
+
+      def as_batches(query, &blk)
+        records = get_records(query)
+        while records.any?
+          @offset += records.size
+          records.each(&blk)
+
+          if @limit
+            @limit -= records.size
+            if @limit < size
+              @size = @limit
+            end
+
+            return if @limit.zero?
+          end
+
+          records = get_records(query)
+        end
+      end
+    end
+
+    def as_batches(args = {}, &blk)
+      Batch.new(arel, args).as_batches(self, &blk)
+    end
+  end
+
+  class Relation
+    include AsBatches
+  end
+end
diff --git a/spec/lib/core_ext/relation/as_batches_spec.rb b/spec/lib/core_ext/relation/as_batches_spec.rb
new file mode 100644
index 000000000..5bd094c6b
--- /dev/null
+++ b/spec/lib/core_ext/relation/as_batches_spec.rb
@@ -0,0 +1,41 @@
+# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/
+
+require 'rails_helper'
+
+RSpec.describe 'AsBatches' do
+  def priorities_asc(size)
+    result = []
+    Ticket::Priority.order(name: :asc).as_batches(size: size) do |prio|
+      result << prio
+    end
+    result
+  end
+
+  def priorities_desc(size)
+    result = []
+    Ticket::Priority.order(name: :desc).as_batches(size: size) do |prio|
+      result << prio
+    end
+    result
+  end
+
+  context 'when batch is smaller then total result' do
+    it 'does return all priorities ascending' do
+      expect(priorities_asc(1)).to eq([ Ticket::Priority.find_by(name: '1 low'), Ticket::Priority.find_by(name: '2 normal'), Ticket::Priority.find_by(name: '3 high') ])
+    end
+
+    it 'does return all priorities decending' do
+      expect(priorities_desc(1)).to eq([ Ticket::Priority.find_by(name: '3 high'), Ticket::Priority.find_by(name: '2 normal'), Ticket::Priority.find_by(name: '1 low') ])
+    end
+  end
+
+  context 'when batch is equal to total result' do
+    it 'does return all priorities ascending' do
+      expect(priorities_asc(100)).to eq([ Ticket::Priority.find_by(name: '1 low'), Ticket::Priority.find_by(name: '2 normal'), Ticket::Priority.find_by(name: '3 high') ])
+    end
+
+    it 'does return all priorities decending' do
+      expect(priorities_desc(100)).to eq([ Ticket::Priority.find_by(name: '3 high'), Ticket::Priority.find_by(name: '2 normal'), Ticket::Priority.find_by(name: '1 low') ])
+    end
+  end
+end
diff --git a/spec/models/sla_spec.rb b/spec/models/sla_spec.rb
index 005d4006d..d84e81beb 100644
--- a/spec/models/sla_spec.rb
+++ b/spec/models/sla_spec.rb
@@ -78,6 +78,20 @@ RSpec.describe Sla, type: :model do
         sla_blank
         expect(described_class.for_ticket(ticket_matching)).to eq sla
       end
+
+      context 'when multiple SLAs are matching' do
+        let(:sla) { create(:sla, :condition_title, condition_title: 'matching', name: 'ZZZ 1') }
+        let(:sla2) { create(:sla, :condition_title, condition_title: 'matching', name: 'AAA 1') }
+
+        before do
+          sla
+          sla2
+        end
+
+        it 'returns the AAA 1 sla as matching' do
+          expect(described_class.for_ticket(ticket_matching)).to eq sla2
+        end
+      end
     end
   end
 end