From 4f3cfb8e745948397a366e8affdbab419a01f341 Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Fri, 29 Mar 2013 00:13:15 +0100 Subject: [PATCH] Added some bugfixes for escalation calculation. --- app/controllers/ticket_articles_controller.rb | 2 +- .../observer/ticket/escalation_calculation.rb | 24 ++- app/models/observer/ticket/first_response.rb | 2 +- app/models/observer/ticket/last_contact.rb | 8 +- app/models/ticket.rb | 6 +- app/models/ticket/article.rb | 5 +- lib/business_time/core_ext/time_fix.rb | 8 +- test/unit/ticket_test.rb | 161 +++++++++++++++++- test/unit/working_time_test.rb | 14 ++ 9 files changed, 204 insertions(+), 26 deletions(-) diff --git a/app/controllers/ticket_articles_controller.rb b/app/controllers/ticket_articles_controller.rb index f17596350..07a6ccf0f 100644 --- a/app/controllers/ticket_articles_controller.rb +++ b/app/controllers/ticket_articles_controller.rb @@ -25,7 +25,7 @@ class TicketArticlesController < ApplicationController # find attachments in upload cache if form_id - @article['attachments'] = Store.list( + @article.attachments = Store.list( :object => 'UploadCache', :o_id => form_id, ) diff --git a/app/models/observer/ticket/escalation_calculation.rb b/app/models/observer/ticket/escalation_calculation.rb index c7545b552..e2fd24791 100644 --- a/app/models/observer/ticket/escalation_calculation.rb +++ b/app/models/observer/ticket/escalation_calculation.rb @@ -2,6 +2,21 @@ 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) @@ -9,18 +24,17 @@ class Observer::Ticket::EscalationCalculation < ActiveRecord::Observer # return if we run import mode return if Setting.get('import_mode') && !Setting.get('import_ignore_sla') - # prevent loops - return if record[:escalation_calc] - record[:escalation_calc] = true - # do not recalculation if first respons is already out if record.class.name == 'Ticket::Article' - return true if record.ticket.first_response 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 \ No newline at end of file diff --git a/app/models/observer/ticket/first_response.rb b/app/models/observer/ticket/first_response.rb index e4949a31b..a5a6f25fa 100644 --- a/app/models/observer/ticket/first_response.rb +++ b/app/models/observer/ticket/first_response.rb @@ -20,7 +20,7 @@ class Observer::Ticket::FirstResponse < ActiveRecord::Observer return true if record.ticket.first_response # set first_response - record.ticket.first_response = Time.now + record.ticket.first_response = record.created_at # save ticket record.ticket.save diff --git a/app/models/observer/ticket/last_contact.rb b/app/models/observer/ticket/last_contact.rb index c6a7cb489..244d79da2 100644 --- a/app/models/observer/ticket/last_contact.rb +++ b/app/models/observer/ticket/last_contact.rb @@ -18,10 +18,10 @@ class Observer::Ticket::LastContact < ActiveRecord::Observer if record.ticket.last_contact_customer == nil || record.ticket.last_contact_agent == nil || record.ticket.last_contact_agent.to_i > record.ticket.last_contact_customer.to_i - record.ticket.last_contact_customer = Time.now + record.ticket.last_contact_customer = record.created_at # set last_contact - record.ticket.last_contact = Time.now + record.ticket.last_contact = record.created_at # save ticket record.ticket.save @@ -32,10 +32,10 @@ class Observer::Ticket::LastContact < ActiveRecord::Observer if sender.name == 'Agent' # set last_contact_agent - record.ticket.last_contact_agent = Time.now + record.ticket.last_contact_agent = record.created_at # set last_contact - record.ticket.last_contact = Time.now + record.ticket.last_contact = record.created_at # save ticket record.ticket.save diff --git a/app/models/ticket.rb b/app/models/ticket.rb index 7f5537ce0..5376e2479 100644 --- a/app/models/ticket.rb +++ b/app/models/ticket.rb @@ -15,6 +15,8 @@ class Ticket < ApplicationModel belongs_to :create_article_type, :class_name => 'Ticket::Article::Type' belongs_to :create_article_sender, :class_name => 'Ticket::Article::Sender' + attr_accessor :callback_loop + def self.number_check (string) self.number_adapter.number_check_item(string) end @@ -529,6 +531,7 @@ class Ticket < ApplicationModel self.escalation_time = nil # self.first_response_escal_date = nil # self.close_time_escal_date = nil + self.callback_loop = true self.save return true end @@ -541,6 +544,7 @@ class Ticket < ApplicationModel self.escalation_time = nil # self.first_response_escal_date = nil # self.close_time_escal_date = nil + self.callback_loop = true self.save return true end @@ -561,7 +565,6 @@ class Ticket < ApplicationModel end if self.first_response# && !self.first_response_in_min self.first_response_in_min = TimeCalculation.business_time_diff( self.created_at, self.first_response ) - end # set sla time if sla_selected.first_response_time && self.first_response_in_min @@ -604,6 +607,7 @@ class Ticket < ApplicationModel if sla_selected.close_time && self.close_time_in_min self.close_time_diff_in_min = sla_selected.close_time - self.close_time_in_min end + self.callback_loop = true self.save end diff --git a/app/models/ticket/article.rb b/app/models/ticket/article.rb index dfaefa717..cf6578e94 100644 --- a/app/models/ticket/article.rb +++ b/app/models/ticket/article.rb @@ -5,11 +5,14 @@ class Ticket::Article < ApplicationModel belongs_to :ticket_article_sender, :class_name => 'Ticket::Article::Sender' belongs_to :created_by, :class_name => 'User' + attr_accessor :attachments + private + def attachment_check # do nothing if no attachment exists - return 1 if self['attachments'] == nil + return 1 if self.attachments == nil # store attachments article_store = [] diff --git a/lib/business_time/core_ext/time_fix.rb b/lib/business_time/core_ext/time_fix.rb index 956c73d08..ce000b22b 100644 --- a/lib/business_time/core_ext/time_fix.rb +++ b/lib/business_time/core_ext/time_fix.rb @@ -83,10 +83,10 @@ class Time time_b = Time::roll_forward(time_b) # If same date, then calculate difference straight forward - if time_a.to_date == time_b.to_date - result = time_b - time_a - return result *= direction - end +# if time_a.to_date == time_b.to_date +# result = time_b - time_a +# return result *= direction +# end # Both times are in different dates result = Time.parse(time_a.strftime('%Y-%m-%d ') + BusinessTime::Config.end_of_workday) - time_a # First day diff --git a/test/unit/ticket_test.rb b/test/unit/ticket_test.rb index b4b581abd..0c2d5713f 100644 --- a/test/unit/ticket_test.rb +++ b/test/unit/ticket_test.rb @@ -18,6 +18,85 @@ class TicketTest < ActiveSupport::TestCase assert_equal( ticket.group.name, 'Users', 'ticket.group verify' ) assert_equal( ticket.ticket_state.name, 'new', 'ticket.state verify' ) + # create inbound article + article_inbound = Ticket::Article.create( + :ticket_id => ticket.id, + :from => 'some_sender@example.com', + :to => 'some_recipient@example.com', + :subject => 'some subject', + :message_id => 'some@id', + :body => 'some message', + :internal => false, + :ticket_article_sender => Ticket::Article::Sender.where(:name => 'Customer').first, + :ticket_article_type => Ticket::Article::Type.where(:name => 'email').first, + :updated_by_id => 1, + :created_by_id => 1, + ) + ticket = Ticket.find(ticket.id) + assert_equal( ticket.article_count, 1, 'ticket.article_count verify - inbound' ) + assert_equal( ticket.last_contact, article_inbound.created_at, 'ticket.last_contact verify - inbound' ) + assert_equal( ticket.last_contact_customer, article_inbound.created_at, 'ticket.last_contact_customer verify - inbound' ) + assert_equal( ticket.last_contact_agent, nil, 'ticket.last_contact_agent verify - inbound' ) + assert_equal( ticket.first_response, nil, 'ticket.first_response verify - inbound' ) + assert_equal( ticket.close_time, nil, 'ticket.close_time verify - inbound' ) + + # create note article + article_note = Ticket::Article.create( + :ticket_id => ticket.id, + :from => 'some persion', + :subject => 'some note', + :body => 'some message', + :internal => true, + :ticket_article_sender => Ticket::Article::Sender.where(:name => 'Agent').first, + :ticket_article_type => Ticket::Article::Type.where(:name => 'note').first, + :updated_by_id => 1, + :created_by_id => 1, + ) + + ticket = Ticket.find(ticket.id) + assert_equal( ticket.article_count, 2, 'ticket.article_count verify - note' ) + assert_equal( ticket.last_contact, article_inbound.created_at, 'ticket.last_contact verify - note' ) + assert_equal( ticket.last_contact_customer, article_inbound.created_at, 'ticket.last_contact_customer verify - note' ) + assert_equal( ticket.last_contact_agent, nil, 'ticket.last_contact_agent verify - note' ) + assert_equal( ticket.first_response, nil, 'ticket.first_response verify - note' ) + assert_equal( ticket.close_time, nil, 'ticket.close_time verify - note' ) + + # create outbound article + sleep 10 + article_outbound = Ticket::Article.create( + :ticket_id => ticket.id, + :from => 'some_recipient@example.com', + :to => 'some_sender@example.com', + :subject => 'some subject', + :message_id => 'some@id2', + :body => 'some message 2', + :internal => false, + :ticket_article_sender => Ticket::Article::Sender.where(:name => 'Agent').first, + :ticket_article_type => Ticket::Article::Type.where(:name => 'email').first, + :updated_by_id => 1, + :created_by_id => 1, + ) + + ticket = Ticket.find(ticket.id) + assert_equal( ticket.article_count, 3, 'ticket.article_count verify - outbound' ) + assert_equal( ticket.last_contact, article_outbound.created_at, 'ticket.last_contact verify - outbound' ) + assert_equal( ticket.last_contact_customer, article_inbound.created_at, 'ticket.last_contact_customer verify - outbound' ) + assert_equal( ticket.last_contact_agent, article_outbound.created_at, 'ticket.last_contact_agent verify - outbound' ) + assert_equal( ticket.first_response, article_outbound.created_at, 'ticket.first_response verify - outbound' ) + assert_equal( ticket.close_time, nil, 'ticket.close_time verify - outbound' ) + + ticket.ticket_state_id = Ticket::State.where(:name => 'closed').first.id + ticket.save + + ticket = Ticket.find(ticket.id) + assert_equal( ticket.article_count, 3, 'ticket.article_count verify - state update' ) + assert_equal( ticket.last_contact, article_outbound.created_at, 'ticket.last_contact verify - state update' ) + assert_equal( ticket.last_contact_customer, article_inbound.created_at, 'ticket.last_contact_customer verify - state update' ) + assert_equal( ticket.last_contact_agent, article_outbound.created_at, 'ticket.last_contact_agent verify - state update' ) + assert_equal( ticket.first_response, article_outbound.created_at, 'ticket.first_response verify - state update' ) + assert( ticket.close_time, 'ticket.close_time verify - state update' ) + + delete = ticket.destroy assert( delete, "ticket destroy" ) end @@ -101,7 +180,6 @@ class TicketTest < ActiveSupport::TestCase ticket.update_attributes( :first_response => '2013-03-21 10:00:00 UTC', ) - ticket.escalation_calculation puts ticket.inspect assert_equal( ticket.escalation_time.gmtime.to_s, '2013-03-21 11:30:00 UTC', 'ticket.escalation_time verify 3' ) @@ -122,7 +200,6 @@ class TicketTest < ActiveSupport::TestCase ticket.update_attributes( :first_response => '2013-03-21 14:00:00 UTC', ) - ticket.escalation_calculation puts ticket.inspect assert_equal( ticket.escalation_time.gmtime.to_s, '2013-03-21 11:30:00 UTC', 'ticket.escalation_time verify 4' ) @@ -143,7 +220,6 @@ class TicketTest < ActiveSupport::TestCase ticket.update_attributes( :last_contact_agent => '2013-03-21 11:00:00 UTC', ) - ticket.escalation_calculation assert_equal( ticket.escalation_time.gmtime.to_s, '2013-03-21 12:30:00 UTC', 'ticket.escalation_time verify 5' ) assert_equal( ticket.first_response_escal_date.gmtime.to_s, '2013-03-21 10:30:00 UTC', 'ticket.first_response_escal_date verify 5' ) @@ -163,7 +239,6 @@ class TicketTest < ActiveSupport::TestCase ticket.update_attributes( :last_contact_agent => '2013-03-21 12:00:00 UTC', ) - ticket.escalation_calculation assert_equal( ticket.escalation_time.gmtime.to_s, '2013-03-21 12:30:00 UTC', 'ticket.escalation_time verify 6' ) assert_equal( ticket.first_response_escal_date.gmtime.to_s, '2013-03-21 10:30:00 UTC', 'ticket.first_response_escal_date verify 6' ) @@ -183,7 +258,6 @@ class TicketTest < ActiveSupport::TestCase ticket.update_attributes( :close_time => '2013-03-21 11:30:00 UTC', ) - ticket.escalation_calculation assert_equal( ticket.escalation_time.gmtime.to_s, '2013-03-21 14:00:00 UTC', 'ticket.escalation_time verify 7' ) assert_equal( ticket.first_response_escal_date.gmtime.to_s, '2013-03-21 10:30:00 UTC', 'ticket.first_response_escal_date verify 7' ) @@ -203,7 +277,6 @@ class TicketTest < ActiveSupport::TestCase ticket.update_attributes( :close_time => '2013-03-21 13:00:00 UTC', ) - ticket.escalation_calculation assert_equal( ticket.escalation_time.gmtime.to_s, '2013-03-21 14:00:00 UTC', 'ticket.escalation_time verify 8' ) assert_equal( ticket.first_response_escal_date.gmtime.to_s, '2013-03-21 10:30:00 UTC', 'ticket.first_response_escal_date verify 8' ) @@ -223,7 +296,6 @@ class TicketTest < ActiveSupport::TestCase ticket.update_attributes( :ticket_state => Ticket::State.lookup( :name => 'closed' ) ) - ticket.escalation_calculation assert_equal( ticket.escalation_time, nil, 'ticket.escalation_time verify 9' ) assert_equal( ticket.first_response_escal_date.gmtime.to_s, '2013-03-21 10:30:00 UTC', 'ticket.first_response_escal_date verify 9' ) @@ -239,12 +311,83 @@ class TicketTest < ActiveSupport::TestCase assert_equal( ticket.close_time_in_min, 210, 'ticket.close_time_in_min verify 9' ) assert_equal( ticket.close_time_diff_in_min, -30, 'ticket.close_time_diff_in_min verify 9' ) + delete = ticket.destroy + assert( delete, "ticket destroy" ) + + ticket = Ticket.create( + :title => 'some title äöüß', + :group => Group.lookup( :name => 'Users'), + :customer_id => 2, + :ticket_state => Ticket::State.lookup( :name => 'new' ), + :ticket_priority => Ticket::Priority.lookup( :name => '2 normal' ), + :updated_by_id => 1, + :created_by_id => 1, + ) + assert( ticket, "ticket created" ) + + assert_equal( ticket.title, 'some title äöüß', 'ticket.title verify' ) + assert_equal( ticket.group.name, 'Users', 'ticket.group verify' ) + assert_equal( ticket.ticket_state.name, 'new', 'ticket.state verify' ) + + # create inbound article + article_inbound = Ticket::Article.create( + :ticket_id => ticket.id, + :from => 'some_sender@example.com', + :to => 'some_recipient@example.com', + :subject => 'some subject', + :message_id => 'some@id', + :body => 'some message', + :internal => false, + :ticket_article_sender => Ticket::Article::Sender.where(:name => 'Customer').first, + :ticket_article_type => Ticket::Article::Type.where(:name => 'email').first, + :updated_by_id => 1, + :created_by_id => 1, + :created_at => '2013-03-28 23:49:00 UTC', + :updated_at => '2013-03-28 23:49:00 UTC', + ) + ticket = Ticket.find(ticket.id) + assert_equal( ticket.article_count, 1, 'ticket.article_count verify - inbound' ) + assert_equal( ticket.last_contact, article_inbound.created_at, 'ticket.last_contact verify - inbound' ) + assert_equal( ticket.last_contact_customer, article_inbound.created_at, 'ticket.last_contact_customer verify - inbound' ) + assert_equal( ticket.last_contact_agent, nil, 'ticket.last_contact_agent verify - inbound' ) + assert_equal( ticket.first_response, nil, 'ticket.first_response verify - inbound' ) + assert_equal( ticket.close_time, nil, 'ticket.close_time verify - inbound' ) + + # create outbound article + article_outbound = Ticket::Article.create( + :ticket_id => ticket.id, + :from => 'some_recipient@example.com', + :to => 'some_sender@example.com', + :subject => 'some subject', + :message_id => 'some@id2', + :body => 'some message 2', + :internal => false, + :ticket_article_sender => Ticket::Article::Sender.where(:name => 'Agent').first, + :ticket_article_type => Ticket::Article::Type.where(:name => 'email').first, + :updated_by_id => 1, + :created_by_id => 1, + :created_at => '2013-03-29 08:00:03 UTC', + :updated_at => '2013-03-29 08:00:03 UTC', + ) + + ticket = Ticket.find(ticket.id) + assert_equal( ticket.article_count, 2, 'ticket.article_count verify - outbound' ) + assert_equal( ticket.last_contact, article_outbound.created_at, 'ticket.last_contact verify - outbound' ) + assert_equal( ticket.last_contact_customer, article_inbound.created_at, 'ticket.last_contact_customer verify - outbound' ) + assert_equal( ticket.last_contact_agent, article_outbound.created_at, 'ticket.last_contact_agent verify - outbound' ) + assert_equal( ticket.first_response, article_outbound.created_at, 'ticket.first_response verify - outbound' ) + assert_equal( ticket.first_response_in_min, 0, 'ticket.first_response_in_min verify - outbound' ) + assert_equal( ticket.first_response_diff_in_min, 60, 'ticket.first_response_diff_in_min verify - outbound' ) + assert_equal( ticket.close_time, nil, 'ticket.close_time verify - outbound' ) - delete = sla.destroy - assert( delete, "sla destroy 2" ) delete = ticket.destroy assert( delete, "ticket destroy" ) + + + delete = sla.destroy + assert( delete, "sla destroy" ) + delete = sla.destroy assert( delete, "sla destroy" ) end diff --git a/test/unit/working_time_test.rb b/test/unit/working_time_test.rb index abee11931..73ba48971 100644 --- a/test/unit/working_time_test.rb +++ b/test/unit/working_time_test.rb @@ -73,6 +73,20 @@ class WorkingTimeTest < ActiveSupport::TestCase ], }, }, + { + :start => '2013-02-28 17:00:00', + :end => '2013-02-28 23:59:59', + :diff => 60, + :config => { + 'Mon' => true, + 'Tue' => true, + 'Wed' => true, + 'Thu' => true, + 'Fri' => true, + 'beginning_of_workday' => '8:00 am', + 'end_of_workday' => '6:00 pm', + }, + }, ] tests.each { |test| TimeCalculation.config( test[:config] )