Fixes #3871 - Wrong SLA is used (alphabetical order is ignored).
This commit is contained in:
parent
32b10f5023
commit
f3d3eefc46
4 changed files with 107 additions and 1 deletions
|
@ -25,7 +25,7 @@ class Sla < ApplicationModel
|
||||||
|
|
||||||
def self.for_ticket(ticket)
|
def self.for_ticket(ticket)
|
||||||
fallback = nil
|
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?
|
if record.condition.present?
|
||||||
return record if record.condition_matches?(ticket)
|
return record if record.condition_matches?(ticket)
|
||||||
else
|
else
|
||||||
|
|
51
config/initializers/active_record_as_batches.rb
Normal file
51
config/initializers/active_record_as_batches.rb
Normal file
|
@ -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
|
41
spec/lib/core_ext/relation/as_batches_spec.rb
Normal file
41
spec/lib/core_ext/relation/as_batches_spec.rb
Normal file
|
@ -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
|
|
@ -78,6 +78,20 @@ RSpec.describe Sla, type: :model do
|
||||||
sla_blank
|
sla_blank
|
||||||
expect(described_class.for_ticket(ticket_matching)).to eq sla
|
expect(described_class.for_ticket(ticket_matching)).to eq sla
|
||||||
end
|
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
|
end
|
||||||
end
|
end
|
||||||
|
|
Loading…
Reference in a new issue