diff --git a/app/models/observer/ticket/escalation_calculation.rb b/app/models/observer/ticket/escalation_calculation.rb deleted file mode 100644 index a089c8fa2..000000000 --- a/app/models/observer/ticket/escalation_calculation.rb +++ /dev/null @@ -1,41 +0,0 @@ -# Copyright (C) 2012-2014 Zammad Foundation, http://zammad-foundation.org/ - -class Observer::Ticket::EscalationCalculation < ActiveRecord::Observer - observe 'ticket', 'ticket::_article' - - def after_create(record) - - # return if we run import mode - return if Setting.get('import_mode') && !Setting.get('import_ignore_sla') - - # do not recalculation if first respons is already out - if record.class.name == 'Ticket::Article' - record.ticket.escalation_calculation - return true - end - - # update escalation - return if record.callback_loop - record.callback_loop = true - record.escalation_calculation - record.callback_loop = false - end - - def after_update(record) - - # return if we run import mode - return if Setting.get('import_mode') && !Setting.get('import_ignore_sla') - - # do not recalculation if first respons is already out - if record.class.name == 'Ticket::Article' - record.ticket.escalation_calculation - return true - end - - # update escalation - return if record.callback_loop - record.callback_loop = true - record.escalation_calculation - record.callback_loop = false - end -end diff --git a/app/models/ticket.rb b/app/models/ticket.rb index c456fa36f..2d5519422 100644 --- a/app/models/ticket.rb +++ b/app/models/ticket.rb @@ -16,8 +16,8 @@ class Ticket < ApplicationModel extend Ticket::Search store :preferences - before_create :check_generate, :check_defaults, :check_title - before_update :check_defaults, :check_title, :reset_pending_time + before_create :check_generate, :check_defaults, :check_title, :check_escalation_update + before_update :check_defaults, :check_title, :reset_pending_time, :check_escalation_update before_destroy :destroy_dependencies notify_clients_support @@ -876,6 +876,11 @@ result self.pending_time = nil end + def check_escalation_update + escalation_calculation_int + true + end + def destroy_dependencies # delete articles diff --git a/app/models/ticket/escalation.rb b/app/models/ticket/escalation.rb index e02746eb3..8853e4c45 100644 --- a/app/models/ticket/escalation.rb +++ b/app/models/ticket/escalation.rb @@ -16,8 +16,10 @@ returns def self.rebuild_all state_list_open = Ticket::State.by_category('open') - tickets = Ticket.where(state_id: state_list_open) - tickets.each(&:escalation_calculation) + ticket_ids = Ticket.where(state_id: state_list_open).pluck(:id) + ticket_ids.each { |ticket_id| + Ticket.find(ticket_id).escalation_calculation(true) + } end =begin @@ -29,17 +31,37 @@ rebuild escalation for ticket returns - result = true + result = true # true = ticket has been updated | false = no changes on ticket =end - def escalation_calculation + def escalation_calculation(force = false) + return if !escalation_calculation_int(force) + self.callback_loop = true + save! + self.callback_loop = false + true + end - # set escalation off if ticket is already closed + def escalation_calculation_int(force = false) + return if callback_loop == true + + # return if we run import mode + return if Setting.get('import_mode') && !Setting.get('import_ignore_sla') + + # set escalation off if current state is not escalation relativ (e. g. ticket is closed) + return if !state_id state = Ticket::State.lookup(id: state_id) escalation_disabled = false if state.ignore_escalation? escalation_disabled = true + + # early exit if nothing current state is not escalation relativ + if !force + return false if escalation_time.nil? + self.escalation_time = nil + return true + end end # get sla for ticket @@ -51,23 +73,71 @@ returns # if no escalation is enabled if !sla || !calendar + preferences[:escalation_calculation] = {} # nothing to change - return true if !escalation_time - + return false if !escalation_time self.escalation_time = nil - self.updated_by_id = 1 - self.callback_loop = true - save return true end - # reset escalation attributes - self.escalation_time = nil - self.first_response_escal_date = nil - self.update_time_escal_date = nil - self.close_time_escal_date = nil + # get last_update + if !last_contact_customer && !last_contact_agent + last_update = created_at + elsif !last_contact_customer && last_contact_agent + last_update = last_contact_agent + elsif last_contact_customer && !last_contact_agent + last_update = last_contact_customer + elsif last_contact_agent > last_contact_customer + last_update = last_contact_agent + elsif last_contact_agent < last_contact_customer + last_update = last_contact_customer + end + # check if calculation need be done + escalation_calculation = preferences[:escalation_calculation] || {} + sla_changed = true + if escalation_calculation['sla_id'] == sla.id && escalation_calculation['sla_updated_at'] == sla.updated_at + sla_changed = false + end + calendar_changed = true + if escalation_calculation['calendar_id'] == calendar.id && escalation_calculation['calendar_updated_at'] == calendar.updated_at + calendar_changed = false + end + if sla_changed == true || calendar_changed == true + force = true + end + first_response_changed = true + if escalation_calculation['first_response'] == first_response + first_response_changed = false + end + last_update_changed = true + if escalation_calculation['last_update'] == last_update + last_update_changed = false + end + close_time_changed = true + if escalation_calculation['close_time'] == close_time + close_time_changed = false + end + + if !force && preferences[:escalation_calculation] + if first_response_changed == false && + last_update_changed == false && + close_time_changed == false && + sla_changed == false && + calendar_changed == false && + escalation_calculation['escalation_disabled'] == escalation_disabled + return false + end + end + + # reset escalation attributes + self.escalation_time = nil + if force == true + self.first_response_escal_date = nil + self.update_time_escal_date = nil + self.close_time_escal_date = nil + end biz = Biz::Schedule.new do |config| # get business hours @@ -98,89 +168,99 @@ returns } end config.holidays = holidays - - # get timezone config.time_zone = calendar.timezone end # get history data - history_data = history_get + history_data = nil - # fist response # calculate first response escalation - if sla.first_response_time - self.first_response_escal_date = destination_time(created_at, sla.first_response_time, biz, history_data) + if force == true || first_response_changed == true + if !history_data + history_data = history_get + end + if sla.first_response_time + self.first_response_escal_date = destination_time(created_at, sla.first_response_time, biz, history_data) + end + + # get response time in min + if first_response + self.first_response_in_min = pending_minutes(created_at, first_response, biz, history_data, 'business_minutes') + end + + # set time to show if sla is raised or not + if sla.first_response_time && first_response_in_min + self.first_response_diff_in_min = sla.first_response_time - first_response_in_min + end end - # get response time in min - if first_response - self.first_response_in_min = pending_minutes(created_at, first_response, biz, history_data, 'business_minutes') - else - self.escalation_time = first_response_escal_date + # calculate update time escalation + if force == true || last_update_changed == true + if !history_data + history_data = history_get + end + if sla.update_time && last_update + self.update_time_escal_date = destination_time(last_update, sla.update_time, biz, history_data) + end + + # get update time in min + if last_update && last_update != created_at + self.update_time_in_min = pending_minutes(created_at, last_update, biz, history_data, 'business_minutes') + end + + # set sla time + if sla.update_time && update_time_in_min + self.update_time_diff_in_min = sla.update_time - update_time_in_min + end end - # set time to show if sla is raised or not - if sla.first_response_time && first_response_in_min - self.first_response_diff_in_min = sla.first_response_time - first_response_in_min - end - - # update time - # calculate escalation - if !last_contact_customer && !last_contact_agent - last_update = created_at - elsif !last_contact_customer && last_contact_agent - last_update = last_contact_agent - elsif last_contact_customer && !last_contact_agent - last_update = last_contact_customer - elsif last_contact_agent > last_contact_customer - last_update = last_contact_agent - elsif last_contact_agent < last_contact_customer - last_update = last_contact_customer - end - if sla.update_time && last_update - self.update_time_escal_date = destination_time(last_update, sla.update_time, biz, history_data) - end - if update_time_escal_date && ((!escalation_time && update_time_escal_date) || update_time_escal_date < escalation_time) - self.escalation_time = update_time_escal_date - end - - # get update time in min - if last_update && last_update != created_at - self.update_time_in_min = pending_minutes(created_at, last_update, biz, history_data, 'business_minutes') - end - - # set sla time - if sla.update_time && update_time_in_min - self.update_time_diff_in_min = sla.update_time - update_time_in_min - end - - # close time # calculate close time escalation - if sla.solution_time - self.close_time_escal_date = destination_time(created_at, sla.solution_time, biz, history_data) - end - - # get close time in min - if close_time - self.close_time_in_min = pending_minutes(created_at, close_time, biz, history_data, 'business_minutes') - elsif close_time_escal_date && ((!escalation_time && close_time_escal_date) || close_time_escal_date < escalation_time) - self.escalation_time = close_time_escal_date - end - - # set time to show if sla is raised or not - if sla.solution_time && close_time_in_min - self.close_time_diff_in_min = sla.solution_time - close_time_in_min + if force == true || close_time_changed == true + if !history_data + history_data = history_get + end + if sla.solution_time + self.close_time_escal_date = destination_time(created_at, sla.solution_time, biz, history_data) + end + + # get close time in min + if close_time + self.close_time_in_min = pending_minutes(created_at, close_time, biz, history_data, 'business_minutes') + end + + # set time to show if sla is raised or not + if sla.solution_time && close_time_in_min + self.close_time_diff_in_min = sla.solution_time - close_time_in_min + end end + # set closest escalation time if escalation_disabled self.escalation_time = nil + else + if !first_response && first_response_escal_date + self.escalation_time = first_response_escal_date + end + if update_time_escal_date && ((!escalation_time && update_time_escal_date) || update_time_escal_date < escalation_time) + self.escalation_time = update_time_escal_date + end + if !close_time && close_time_escal_date && ((!escalation_time && close_time_escal_date) || close_time_escal_date < escalation_time) + self.escalation_time = close_time_escal_date + end end - return if !changed? - - self.callback_loop = true - self.updated_by_id = 1 - save + # remember already counted time to do on next update only the diff + preferences[:escalation_calculation] = { + first_response: first_response, + last_update: last_update, + close_time: close_time, + sla_id: sla.id, + sla_updated_at: sla.updated_at, + calendar_id: calendar.id, + calendar_updated_at: calendar.updated_at, + escalation_disabled: escalation_disabled, + } + true end =begin diff --git a/config/application.rb b/config/application.rb index e783fae9a..05dc172b3 100644 --- a/config/application.rb +++ b/config/application.rb @@ -31,7 +31,6 @@ module Zammad 'observer::_ticket::_article::_communicate_facebook', 'observer::_ticket::_article::_communicate_twitter', 'observer::_ticket::_reset_new_state', - 'observer::_ticket::_escalation_calculation', 'observer::_ticket::_ref_object_touch', 'observer::_ticket::_online_notification_seen', 'observer::_ticket::_stats_reopen', diff --git a/test/unit/calendar_subscription_test.rb b/test/unit/calendar_subscription_test.rb index f8451c2e9..580dc6abf 100644 --- a/test/unit/calendar_subscription_test.rb +++ b/test/unit/calendar_subscription_test.rb @@ -77,7 +77,7 @@ class CalendarSubscriptionTest < ActiveSupport::TestCase Ticket.destroy_all - ticket1 = Ticket.create( + ticket1 = Ticket.create!( title: 'some title1 - new - group_calendar', group: group_calendar, customer_id: customer1.id, @@ -88,7 +88,7 @@ class CalendarSubscriptionTest < ActiveSupport::TestCase updated_by_id: 1, created_by_id: 1, ) - ticket2 = Ticket.create( + ticket2 = Ticket.create!( title: 'some title1 - new - group_default', group: group_default, customer_id: customer1.id, @@ -99,7 +99,7 @@ class CalendarSubscriptionTest < ActiveSupport::TestCase updated_by_id: 1, created_by_id: 1, ) - ticket3 = Ticket.create( + ticket3 = Ticket.create!( title: 'some title1 - pending - group_calendar', group: group_calendar, customer_id: customer1.id, @@ -111,7 +111,7 @@ class CalendarSubscriptionTest < ActiveSupport::TestCase updated_by_id: 1, created_by_id: 1, ) - ticket4 = Ticket.create( + ticket4 = Ticket.create!( title: 'some title1 - pending - group_default', group: group_default, customer_id: customer1.id, @@ -123,32 +123,33 @@ class CalendarSubscriptionTest < ActiveSupport::TestCase updated_by_id: 1, created_by_id: 1, ) - ticket5 = Ticket.create( + ticket5 = Ticket.create!( title: 'some title1 - escalation - group_calendar', group: group_calendar, customer_id: customer1.id, owner_id: agent1.id, state: Ticket::State.lookup(name: 'new'), - escalation_time: '2016-02-07 17:39:00', priority: Ticket::Priority.lookup(name: '2 normal'), created_at: '2016-02-05 16:41:00', updated_by_id: 1, created_by_id: 1, ) - ticket6 = Ticket.create( + ticket5.update_columns(escalation_time: '2016-02-07 17:39:00') + + ticket6 = Ticket.create!( title: 'some title1 - escalation - group_default', group: group_default, customer_id: customer1.id, owner_id: agent2.id, state: Ticket::State.lookup(name: 'new'), - escalation_time: '2016-02-07 16:37:00', priority: Ticket::Priority.lookup(name: '2 normal'), created_at: '2016-02-05 16:42:00', updated_by_id: 1, created_by_id: 1, ) + ticket6.update_columns(escalation_time: '2016-02-07 16:37:00') - ticket7 = Ticket.create( + ticket7 = Ticket.create!( title: 'some title2 - new - group_calendar', group: group_calendar, customer_id: customer1.id, @@ -159,7 +160,7 @@ class CalendarSubscriptionTest < ActiveSupport::TestCase updated_by_id: 1, created_by_id: 1, ) - ticket8 = Ticket.create( + ticket8 = Ticket.create!( title: 'some title2 - new - group_default', group: group_default, customer_id: customer1.id, @@ -170,7 +171,7 @@ class CalendarSubscriptionTest < ActiveSupport::TestCase updated_by_id: 1, created_by_id: 1, ) - ticket9 = Ticket.create( + ticket9 = Ticket.create!( title: 'some title2 - pending - group_calendar', group: group_calendar, customer_id: customer1.id, @@ -182,7 +183,7 @@ class CalendarSubscriptionTest < ActiveSupport::TestCase updated_by_id: 1, created_by_id: 1, ) - ticket10 = Ticket.create( + ticket10 = Ticket.create!( title: 'some title2 - pending - group_default', group: group_default, customer_id: customer1.id, @@ -194,30 +195,32 @@ class CalendarSubscriptionTest < ActiveSupport::TestCase updated_by_id: 1, created_by_id: 1, ) - ticket11 = Ticket.create( + ticket11 = Ticket.create!( title: 'some title2 - escalation - group_calendar', group: group_calendar, customer_id: customer1.id, owner_id: 1, state: Ticket::State.lookup(name: 'new'), - escalation_time: '2016-02-08 18:37:00', priority: Ticket::Priority.lookup(name: '2 normal'), created_at: '2016-02-05 17:41:00', updated_by_id: 1, created_by_id: 1, ) - ticket12 = Ticket.create( + ticket11.update_columns(escalation_time: '2016-02-08 18:37:00') + + ticket12 = Ticket.create!( title: 'some title2 - escalation - group_default', group: group_default, customer_id: customer1.id, owner_id: 1, state: Ticket::State.lookup(name: 'new'), - escalation_time: '2016-02-08 18:38:00', priority: Ticket::Priority.lookup(name: '2 normal'), created_at: '2016-02-05 17:42:00', updated_by_id: 1, created_by_id: 1, ) + ticket12.update_columns(escalation_time: '2016-02-08 18:38:00') + Cache.clear # set escalation_time manually, to clear cache to have correct content later # check agent 1 calendar_subscriptions = CalendarSubscriptions.new(agent1) diff --git a/test/unit/ticket_selector_test.rb b/test/unit/ticket_selector_test.rb index 0e89cff26..2d602b7f2 100644 --- a/test/unit/ticket_selector_test.rb +++ b/test/unit/ticket_selector_test.rb @@ -85,7 +85,7 @@ class TicketSelectorTest < ActiveSupport::TestCase Ticket.destroy_all - ticket1 = Ticket.create( + ticket1 = Ticket.create!( title: 'some title1', group: group, customer_id: customer1.id, @@ -102,7 +102,7 @@ class TicketSelectorTest < ActiveSupport::TestCase assert_equal(ticket1.organization.id, organization1.id) sleep 1 - ticket2 = Ticket.create( + ticket2 = Ticket.create!( title: 'some title2', group: group, customer_id: customer2.id, @@ -118,18 +118,18 @@ class TicketSelectorTest < ActiveSupport::TestCase assert_equal(ticket2.organization_id, nil) sleep 1 - ticket3 = Ticket.create( + ticket3 = Ticket.create!( title: 'some title3', group: group, customer_id: customer2.id, state: Ticket::State.lookup(name: 'open'), priority: Ticket::Priority.lookup(name: '2 normal'), - escalation_time: '2015-02-06 10:00:00', created_at: '2015-02-05 16:37:00', #updated_at: '2015-02-05 17:37:00', updated_by_id: 1, created_by_id: 1, ) + ticket3.update_columns(escalation_time: '2015-02-06 10:00:00') assert(ticket3, 'ticket created') assert_equal(ticket3.customer.id, customer2.id) assert_equal(ticket3.organization_id, nil) diff --git a/test/unit/ticket_sla_test.rb b/test/unit/ticket_sla_test.rb index 3c9f528ef..d81318700 100644 --- a/test/unit/ticket_sla_test.rb +++ b/test/unit/ticket_sla_test.rb @@ -11,7 +11,7 @@ class TicketSlaTest < ActiveSupport::TestCase delete = Ticket.destroy_all assert(delete, 'ticket destroy_all') - ticket = Ticket.create( + ticket = Ticket.create!( title: 'some title äöüß', group: Group.lookup(name: 'Users'), customer_id: 2, @@ -343,7 +343,7 @@ class TicketSlaTest < ActiveSupport::TestCase delete = ticket.destroy assert(delete, 'ticket destroy') - ticket = Ticket.create( + ticket = Ticket.create!( title: 'some title äöüß', group: Group.lookup(name: 'Users'), customer_id: 2, @@ -361,7 +361,7 @@ class TicketSlaTest < ActiveSupport::TestCase assert_equal(ticket.state.name, 'new', 'ticket.state verify') # create inbound article - article_inbound = Ticket::Article.create( + article_inbound = Ticket::Article.create!( ticket_id: ticket.id, from: 'some_sender@example.com', to: 'some_recipient@example.com', @@ -385,7 +385,7 @@ class TicketSlaTest < ActiveSupport::TestCase assert_equal(ticket.close_time, nil, 'ticket.close_time verify - inbound') # create outbound article - article_outbound = Ticket::Article.create( + article_outbound = Ticket::Article.create!( ticket_id: ticket.id, from: 'some_recipient@example.com', to: 'some_sender@example.com', @@ -414,7 +414,7 @@ class TicketSlaTest < ActiveSupport::TestCase delete = ticket.destroy assert(delete, 'ticket destroy') - ticket = Ticket.create( + ticket = Ticket.create!( title: 'some title äöüß', group: Group.lookup(name: 'Users'), customer_id: 2, @@ -432,7 +432,7 @@ class TicketSlaTest < ActiveSupport::TestCase assert_equal(ticket.state.name, 'new', 'ticket.state verify') # create inbound article - article_inbound = Ticket::Article.create( + article_inbound = Ticket::Article.create!( ticket_id: ticket.id, from: 'some_sender@example.com', subject: 'some subject', @@ -455,7 +455,7 @@ class TicketSlaTest < ActiveSupport::TestCase assert_equal(ticket.close_time, nil, 'ticket.close_time verify - inbound') # create note article - article_note = Ticket::Article.create( + article_note = Ticket::Article.create!( ticket_id: ticket.id, from: 'some_sender@example.com', subject: 'some subject', @@ -478,7 +478,7 @@ class TicketSlaTest < ActiveSupport::TestCase assert_equal(ticket.close_time, nil, 'ticket.close_time verify - inbound') # create outbound article - article_outbound = Ticket::Article.create( + article_outbound = Ticket::Article.create!( ticket_id: ticket.id, from: 'some_sender@example.com', subject: 'some subject', @@ -515,7 +515,7 @@ class TicketSlaTest < ActiveSupport::TestCase delete = Ticket.destroy_all assert(delete, 'ticket destroy_all') - ticket = Ticket.create( + ticket = Ticket.create!( title: 'some title äöüß', group: Group.lookup(name: 'Users'), customer_id: 2, @@ -610,7 +610,7 @@ class TicketSlaTest < ActiveSupport::TestCase delete = ticket.destroy assert(delete, 'ticket destroy') - ticket = Ticket.create( + ticket = Ticket.create!( title: 'some title äöüß', group: Group.lookup(name: 'Users'), customer_id: 2, @@ -700,7 +700,7 @@ class TicketSlaTest < ActiveSupport::TestCase delete = sla.destroy assert(delete, 'sla destroy') - ticket = Ticket.create( + ticket = Ticket.create!( title: 'some title äöüß', group: Group.lookup(name: 'Users'), customer_id: 2, @@ -732,7 +732,7 @@ class TicketSlaTest < ActiveSupport::TestCase assert_equal(ticket.update_time_escal_date.gmtime.to_s, '2013-10-21 09:00:00 UTC', 'ticket.update_time_escal_date verify 1') assert_equal(ticket.close_time_escal_date.gmtime.to_s, '2013-10-21 10:00:00 UTC', 'ticket.close_time_escal_date verify 1') - ticket = Ticket.create( + ticket = Ticket.create!( title: 'some title holiday test', group: Group.lookup(name: 'Users'), customer_id: 2, @@ -765,7 +765,7 @@ class TicketSlaTest < ActiveSupport::TestCase delete = Ticket.destroy_all assert(delete, 'ticket destroy_all') - ticket1 = Ticket.create( + ticket1 = Ticket.create!( title: 'some title äöüß3', group: Group.lookup(name: 'Users'), customer_id: 2, @@ -936,7 +936,7 @@ class TicketSlaTest < ActiveSupport::TestCase created_at: '2013-06-04 15:00:00 UTC', updated_at: '2013-06-04 15:00:00 UTC', ) - ticket2.escalation_calculation + ticket2.escalation_calculation(true) ticket2 = Ticket.find(ticket2.id) assert_equal(ticket2.escalation_time.gmtime.to_s, '2013-06-04 16:00:00 UTC', 'ticket2.escalation_time verify 1') assert_equal(ticket2.first_response_escal_date.gmtime.to_s, '2013-06-04 16:00:00 UTC', 'ticket2.first_response_escal_date verify 1') @@ -953,7 +953,7 @@ class TicketSlaTest < ActiveSupport::TestCase end test 'ticket escalation suspend' do - ticket = Ticket.create( + ticket = Ticket.create!( title: 'some title äöüß3', group: Group.lookup(name: 'Users'), customer_id: 2, @@ -1091,7 +1091,7 @@ class TicketSlaTest < ActiveSupport::TestCase assert(delete, 'ticket destroy') # test Ticket created in state pending and closed without reopen or state change - ticket = Ticket.create( + ticket = Ticket.create!( title: 'some title äöüß3', group: Group.lookup(name: 'Users'), customer_id: 2, @@ -1121,6 +1121,7 @@ class TicketSlaTest < ActiveSupport::TestCase ticket.update_attributes( close_time: '2013-06-04 12:00:00 UTC', ) + ticket.escalation_calculation(true) calendar = Calendar.create_or_update( name: 'EU 5', @@ -1188,7 +1189,7 @@ class TicketSlaTest < ActiveSupport::TestCase assert(delete, 'ticket destroy') # test Ticket created in state pending, changed state to openen, back to pending and closed - ticket = Ticket.create( + ticket = Ticket.create!( title: 'some title äöüß3', group: Group.lookup(name: 'Users'), customer_id: 2, @@ -1316,7 +1317,7 @@ class TicketSlaTest < ActiveSupport::TestCase ### Test Ticket created in state pending, changed state to openen, back to pending and back to open then ### close ticket - ticket = Ticket.create( + ticket = Ticket.create!( title: 'some title äöüß3', group: Group.lookup(name: 'Users'), customer_id: 2,