From b764e7b3e53f7389bfcba3212190c09322e624e7 Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Tue, 8 May 2018 14:18:15 +0200 Subject: [PATCH] Fixes issue #1995 - Missing DB index on article columns queried by ticket waiting time report backend. --- lib/stats/ticket_waiting_time.rb | 40 +++--- test/unit/stats_ticket_waiting_time_test.rb | 132 ++++++++++++++++++-- 2 files changed, 144 insertions(+), 28 deletions(-) diff --git a/lib/stats/ticket_waiting_time.rb b/lib/stats/ticket_waiting_time.rb index 026bbb151..d36e3c9a8 100644 --- a/lib/stats/ticket_waiting_time.rb +++ b/lib/stats/ticket_waiting_time.rb @@ -7,12 +7,14 @@ class Stats::TicketWaitingTime # get users groups group_ids = user.group_ids_access('full') - own_waiting = Ticket.where( - 'owner_id = ? AND group_id IN (?) AND updated_at > ?', user.id, group_ids, Time.zone.today - ) - all_waiting = Ticket.where( - 'group_id IN (?) AND updated_at > ?', group_ids, Time.zone.today - ) + own_waiting = [] + all_waiting = [] + Ticket.where('group_id IN (?) AND updated_at > ?', group_ids.sort, Time.zone.today).pluck(:id, :owner_id).each do |ticket| + all_waiting.push ticket[0] + if ticket[1] == user.id + own_waiting.push ticket[0] + end + end handling_time = calculate_average(own_waiting, Time.zone.today) if handling_time.positive? @@ -51,20 +53,24 @@ class Stats::TicketWaitingTime result end - def self.calculate_average(tickets, start_time) + def self.calculate_average(ticket_ids, start_time) average_time = 0 count_articles = 0 + last_ticket_id = nil + count_time = nil - tickets.each do |ticket| - count_time = 0 - ticket.articles.joins(:type).where('ticket_articles.created_at > ? AND ticket_articles.internal = ? AND ticket_article_types.communication = ?', start_time, false, true).each do |article| - if article.sender.name == 'Customer' - count_time = article.created_at.to_i - elsif count_time.positive? - average_time += article.created_at.to_i - count_time - count_articles += 1 - count_time = 0 - end + Ticket::Article.joins(:type).joins(:sender).where('ticket_articles.ticket_id IN (?) AND ticket_articles.created_at > ? AND ticket_articles.internal = ? AND ticket_article_types.communication = ?', ticket_ids, start_time, false, true).order(:ticket_id, :created_at).pluck(:created_at, :sender_id, :ticket_id, :id).each do |article| + if last_ticket_id != article[2] + last_ticket_id = article[2] + count_time = 0 + end + sender = Ticket::Article::Sender.lookup(id: article[1]) + if sender.name == 'Customer' + count_time = article[0].to_i + elsif count_time.positive? + average_time += article[0].to_i - count_time + count_articles += 1 + count_time = 0 end end diff --git a/test/unit/stats_ticket_waiting_time_test.rb b/test/unit/stats_ticket_waiting_time_test.rb index a1842090c..b2a8fed44 100644 --- a/test/unit/stats_ticket_waiting_time_test.rb +++ b/test/unit/stats_ticket_waiting_time_test.rb @@ -5,9 +5,49 @@ require 'stats/ticket_waiting_time' class StatsTicketWaitingTimeTest < ActiveSupport::TestCase test 'single ticket' do - ticket1 = Ticket.create( + + group1 = Group.create!( + name: 'Group 1', + active: true, + email_address: EmailAddress.first, + created_by_id: 1, + updated_by_id: 1, + ) + roles = Role.where(name: 'Agent') + user1 = User.create!( + login: 'assets_stats1@example.org', + firstname: 'assets_stats1', + lastname: 'assets_stats1', + email: 'assets_stats1@example.org', + password: 'some_pass', + active: true, + groups: [group1], + roles: roles, + created_by_id: 1, + updated_by_id: 1, + ) + user2 = User.create!( + login: 'assets_stats2@example.org', + firstname: 'assets_stats2', + lastname: 'assets_stats2', + email: 'assets_sla2@example.org', + password: 'some_pass', + active: true, + groups: [group1], + roles: roles, + created_by_id: 1, + updated_by_id: 1, + ) + + result = Stats::TicketWaitingTime.generate(user1) + assert_equal(0, result[:handling_time]) + assert_equal('supergood', result[:state]) + assert_equal(0, result[:average_per_agent]) + assert_equal(0, result[:percent]) + + ticket1 = Ticket.create!( title: 'com test 1', - group: Group.lookup(name: 'Users'), + group: group1, customer_id: 2, state: Ticket::State.lookup(name: 'new'), priority: Ticket::Priority.lookup(name: '2 normal'), @@ -16,7 +56,7 @@ class StatsTicketWaitingTimeTest < ActiveSupport::TestCase ) # communication 1: waiting time 2 hours (BUT too old yesterday) - Ticket::Article.create( + Ticket::Article.create!( ticket_id: ticket1.id, from: 'a@example.com', to: 'a@example.com', @@ -31,7 +71,7 @@ class StatsTicketWaitingTimeTest < ActiveSupport::TestCase created_at: '2017-04-12 08:00', updated_at: '2017-04-12 08:00', ) - Ticket::Article.create( + Ticket::Article.create!( ticket_id: ticket1.id, from: 'a@example.com', to: 'a@example.com', @@ -48,7 +88,7 @@ class StatsTicketWaitingTimeTest < ActiveSupport::TestCase ) # communication 2: waiting time 2 hours - Ticket::Article.create( + Ticket::Article.create!( ticket_id: ticket1.id, from: 'a@example.com', to: 'a@example.com', @@ -63,7 +103,7 @@ class StatsTicketWaitingTimeTest < ActiveSupport::TestCase created_at: '2017-04-13 08:00', updated_at: '2017-04-13 08:00', ) - Ticket::Article.create( + Ticket::Article.create!( ticket_id: ticket1.id, from: 'a@example.com', to: 'a@example.com', @@ -80,7 +120,7 @@ class StatsTicketWaitingTimeTest < ActiveSupport::TestCase ) # communication 3: waiting time 4 hours - Ticket::Article.create( + Ticket::Article.create!( ticket_id: ticket1.id, from: 'a@example.com', to: 'a@example.com', @@ -95,7 +135,7 @@ class StatsTicketWaitingTimeTest < ActiveSupport::TestCase created_at: '2017-04-13 11:00', updated_at: '2017-04-13 11:00', ) - Ticket::Article.create( + Ticket::Article.create!( ticket_id: ticket1.id, from: 'a@example.com', to: 'a@example.com', @@ -112,7 +152,7 @@ class StatsTicketWaitingTimeTest < ActiveSupport::TestCase ) # communication 4: INVALID waiting time 1 hour (because internal) - Ticket::Article.create( + Ticket::Article.create!( ticket_id: ticket1.id, from: 'a@example.com', to: 'a@example.com', @@ -127,7 +167,7 @@ class StatsTicketWaitingTimeTest < ActiveSupport::TestCase created_at: '2017-04-13 15:00', updated_at: '2017-04-13 15:00', ) - Ticket::Article.create( + Ticket::Article.create!( ticket_id: ticket1.id, from: 'a@example.com', to: 'a@example.com', @@ -143,13 +183,83 @@ class StatsTicketWaitingTimeTest < ActiveSupport::TestCase updated_at: '2017-04-13 15:10', ) - average_time = Stats::TicketWaitingTime.calculate_average([ticket1, ticket1], '2017-04-13 00:00:00') + ticket2 = Ticket.create!( + title: 'com test 2', + group: group1, + customer_id: 2, + state: Ticket::State.lookup(name: 'new'), + priority: Ticket::Priority.lookup(name: '2 normal'), + updated_by_id: 1, + created_by_id: 1, + ) + + average_time = Stats::TicketWaitingTime.calculate_average([ticket1.id, ticket2.id], '2017-04-13 00:00:00') expected_average_time = 60 * 60 * 2 # for communication 2 expected_average_time += 60 * 60 * 4 # for communication 3 expected_average_time = expected_average_time / 2 # for average + travel_to Time.zone.local(2017, 0o4, 13, 23, 0o0, 44) + assert_equal(expected_average_time, average_time) + + result = Stats::TicketWaitingTime.generate(user1) + assert_equal(0, result[:handling_time]) + assert_equal('supergood', result[:state]) + assert_equal(180, result[:average_per_agent]) + assert_equal(0.0, result[:percent]) + + ticket3 = Ticket.create!( + title: 'com test 3', + group: group1, + customer_id: 2, + owner: user1, + state: Ticket::State.lookup(name: 'new'), + priority: Ticket::Priority.lookup(name: '2 normal'), + updated_by_id: 1, + created_by_id: 1, + ) + + # communication 1: waiting time 2 hours (BUT too old yesterday) + Ticket::Article.create!( + ticket_id: ticket3.id, + from: 'a@example.com', + to: 'a@example.com', + subject: 'com test 3', + message_id: 'some@id_com_1', + body: 'some message 123', + internal: false, + sender: Ticket::Article::Sender.find_by(name: 'Customer'), + type: Ticket::Article::Type.find_by(name: 'email'), + updated_by_id: 1, + created_by_id: 1, + created_at: '2017-04-13 08:00', + updated_at: '2017-04-13 08:00', + ) + Ticket::Article.create!( + ticket_id: ticket3.id, + from: 'a@example.com', + to: 'a@example.com', + subject: 'com test 3', + message_id: 'some@id_com_1', + body: 'some message 123', + internal: false, + sender: Ticket::Article::Sender.find_by(name: 'Agent'), + type: Ticket::Article::Type.find_by(name: 'email'), + updated_by_id: 1, + created_by_id: 1, + created_at: '2017-04-13 09:00', + updated_at: '2017-04-13 09:00', + ) + + result = Stats::TicketWaitingTime.generate(user1) + assert_equal(60, result[:handling_time]) + assert_equal('supergood', result[:state]) + assert_equal(140, result[:average_per_agent]) + assert_equal(1.0, result[:percent]) + + travel_back + end end