Browse Source

Added some bugfixes for escalation calculation.

Martin Edenhofer 12 years ago
parent
commit
4f3cfb8e74

+ 1 - 1
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,
       )

+ 19 - 5
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

+ 1 - 1
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

+ 4 - 4
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

+ 5 - 1
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
 

+ 4 - 1
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 = []

+ 4 - 4
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

+ 152 - 9
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

+ 14 - 0
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] )