Refactoring: SLA and Escalation logic. Fixes #3410, fixes #3140, fixes …

…#2579.

.rubocop/todo.rspec.yml

@@ -255,8 +255,11 @@ RSpec/ExampleLength:
     - 'spec/models/role_group_spec.rb'
     - 'spec/models/role_spec.rb'
     - 'spec/models/scheduler_spec.rb'
+    - 'spec/models/sla/has_escalation_calculation_impact_examples.rb'
     - 'spec/models/taskbar_spec.rb'
     - 'spec/models/ticket/article_spec.rb'
+    - 'spec/models/ticket/article/has_ticket_contact_attributes_impact_examples.rb'
+    - 'spec/models/ticket/escalation_examples.rb'
     - 'spec/models/ticket/overviews_spec.rb'
     - 'spec/models/ticket_spec.rb'
     - 'spec/models/translation_spec.rb'
@@ -528,9 +531,12 @@ RSpec/MultipleExpectations:
     - 'spec/models/object_manager/attribute_spec.rb'
     - 'spec/models/overview_spec.rb'
     - 'spec/models/scheduler_spec.rb'
+    - 'spec/models/sla/has_escalation_calculation_impact_examples.rb'
     - 'spec/models/smime_certificate_spec.rb'
     - 'spec/models/taskbar_spec.rb'
     - 'spec/models/ticket/article_spec.rb'
+    - 'spec/models/ticket/article/has_ticket_contact_attributes_impact_examples.rb'
+    - 'spec/models/ticket/escalation_examples.rb'
     - 'spec/models/ticket/number/increment_spec.rb'
     - 'spec/models/ticket/number_spec.rb'
     - 'spec/models/ticket/overviews_spec.rb'

.rubocop/todo.yml

@@ -156,7 +156,6 @@ Metrics/AbcSize:
     - 'app/models/link.rb'
     - 'app/models/object_manager/attribute.rb'
     - 'app/models/observer/chat/leave/background_job.rb'
-    - 'app/models/observer/sla/ticket_rebuild_escalation.rb'
     - 'app/models/observer/ticket/article/communicate_email.rb'
     - 'app/models/observer/ticket/article/communicate_facebook.rb'
     - 'app/models/observer/ticket/article/communicate_sms.rb'
@@ -165,7 +164,6 @@ Metrics/AbcSize:
     - 'app/models/observer/ticket/article/fillup_from_email.rb'
     - 'app/models/observer/ticket/article/fillup_from_general.rb'
     - 'app/models/observer/ticket/article/fillup_from_origin_by_id.rb'
-    - 'app/models/observer/ticket/article_changes.rb'
     - 'app/models/observer/ticket/last_owner_update.rb'
     - 'app/models/observer/ticket/ref_object_touch.rb'
     - 'app/models/observer/ticket/reset_new_state.rb'
@@ -199,6 +197,7 @@ Metrics/AbcSize:
     - 'app/models/ticket.rb'
     - 'app/models/ticket/article.rb'
     - 'app/models/ticket/article/assets.rb'
+    - 'app/models/ticket/article/has_ticket_contact_attributes_impact.rb'
     - 'app/models/ticket/assets.rb'
     - 'app/models/ticket/escalation.rb'
     - 'app/models/ticket/number/date.rb'
@@ -546,15 +545,13 @@ Metrics/CyclomaticComplexity:
     - 'app/models/karma/activity_log.rb'
     - 'app/models/knowledge_base.rb'
     - 'app/models/object_manager/attribute.rb'
-    - 'app/models/observer/sla/ticket_rebuild_escalation.rb'
     - 'app/models/observer/ticket/article/communicate_email.rb'
     - 'app/models/observer/ticket/article/communicate_facebook.rb'
     - 'app/models/observer/ticket/article/communicate_sms.rb'
     - 'app/models/observer/ticket/article/communicate_twitter.rb'
     - 'app/models/observer/ticket/article/fillup_from_email.rb'
     - 'app/models/observer/ticket/article/fillup_from_general.rb'
     - 'app/models/observer/ticket/article/fillup_from_origin_by_id.rb'
-    - 'app/models/observer/ticket/article_changes.rb'
     - 'app/models/observer/ticket/last_owner_update.rb'
     - 'app/models/observer/ticket/ref_object_touch.rb'
     - 'app/models/observer/ticket/reset_new_state.rb'
@@ -576,6 +573,7 @@ Metrics/CyclomaticComplexity:
     - 'app/models/taskbar.rb'
     - 'app/models/ticket/article.rb'
     - 'app/models/ticket/article/assets.rb'
+    - 'app/models/ticket/article/has_ticket_contact_attributes_impact.rb'
     - 'app/models/ticket/assets.rb'
     - 'app/models/ticket/escalation.rb'
     - 'app/models/ticket/number/date.rb'
@@ -777,15 +775,13 @@ Metrics/PerceivedComplexity:
     - 'app/models/karma/activity_log.rb'
     - 'app/models/knowledge_base.rb'
     - 'app/models/object_manager/attribute.rb'
-    - 'app/models/observer/sla/ticket_rebuild_escalation.rb'
     - 'app/models/observer/ticket/article/communicate_email.rb'
     - 'app/models/observer/ticket/article/communicate_facebook.rb'
     - 'app/models/observer/ticket/article/communicate_sms.rb'
     - 'app/models/observer/ticket/article/communicate_twitter.rb'
     - 'app/models/observer/ticket/article/fillup_from_email.rb'
     - 'app/models/observer/ticket/article/fillup_from_general.rb'
     - 'app/models/observer/ticket/article/fillup_from_origin_by_id.rb'
-    - 'app/models/observer/ticket/article_changes.rb'
     - 'app/models/observer/ticket/last_owner_update.rb'
     - 'app/models/observer/ticket/ref_object_touch.rb'
     - 'app/models/observer/ticket/reset_new_state.rb'
@@ -806,6 +802,7 @@ Metrics/PerceivedComplexity:
     - 'app/models/ticket.rb'
     - 'app/models/ticket/article.rb'
     - 'app/models/ticket/article/assets.rb'
+    - 'app/models/ticket/article/has_ticket_contact_attributes_impact.rb'
     - 'app/models/ticket/escalation.rb'
     - 'app/models/ticket/number/date.rb'
     - 'app/models/ticket/overviews.rb'

app/jobs/ticket_escalation_rebuild_job.rb

@@ -0,0 +1,16 @@
+class TicketEscalationRebuildJob < ApplicationJob
+  include HasActiveJobLock
+
+  def perform
+    scope.in_batches.each_record do |ticket|
+      ticket.escalation_calculation(true)
+    end
+  end
+
+  private
+
+  def scope
+    Ticket.where(state_id: Ticket::State.by_category(:open))
+  end
+
+end

app/models/calendar.rb

@@ -3,6 +3,7 @@
 class Calendar < ApplicationModel
   include ChecksClientNotification
   include CanUniqName
+  include HasEscalationCalculationImpact
 
   store :business_hours
   store :public_holidays
@@ -324,7 +325,7 @@ def public_holidays_to_array
     holidays
   end
 
-  def biz
+  def biz(breaks: {})
     Biz::Schedule.new do |config|
 
       # get business hours
@@ -335,7 +336,10 @@ def biz
 
       # get holidays
       config.holidays = public_holidays_to_array
+
       config.time_zone = timezone
+
+      config.breaks = breaks
     end
   end
 

app/models/concerns/has_escalation_calculation_impact.rb

@@ -0,0 +1,31 @@
+# Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/
+module HasEscalationCalculationImpact
+  extend ActiveSupport::Concern
+
+  included do
+    after_commit :enqueue_ticket_escalation_rebuild_job
+  end
+
+  private
+
+  def enqueue_ticket_escalation_rebuild_job
+
+    # return if we run import mode
+    return if Setting.get('import_mode') && !Setting.get('import_ignore_sla')
+
+    # check if condition has changed
+    fields_to_check = if instance_of?(Sla)
+                        %w[condition calendar_id first_response_time update_time solution_time]
+                      else
+                        %w[timezone business_hours default ical_url public_holidays]
+                      end
+
+    return if fields_to_check.none? do |item|
+      next if !saved_change_to_attribute(item)
+
+      saved_change_to_attribute(item)[0] != saved_change_to_attribute(item)[1]
+    end
+
+    TicketEscalationRebuildJob.perform_later
+  end
+end

app/models/sla.rb

@@ -3,11 +3,29 @@
 class Sla < ApplicationModel
   include ChecksClientNotification
   include ChecksConditionValidation
+  include HasEscalationCalculationImpact
 
   include Sla::Assets
 
   store      :condition
   store      :data
   validates  :name, presence: true
   belongs_to :calendar, optional: true
+
+  def condition_matches?(ticket)
+    query_condition, bind_condition, tables = Ticket.selector2sql(condition)
+    Ticket.where(query_condition, *bind_condition).joins(tables).exists?(ticket.id)
+  end
+
+  def self.for_ticket(ticket)
+    fallback = nil
+    all.order(:name, :created_at).find_each do |record|
+      if record.condition.present?
+        return record if record.condition_matches?(ticket)
+      else
+        fallback = record
+      end
+    end
+    fallback
+  end
 end

app/models/ticket.rb

@@ -16,12 +16,12 @@ class Ticket < ApplicationModel
   include HasObjectManagerAttributesValidation
   include HasTaskbars
 
-  include Ticket::Escalation
-  include Ticket::Subject
-  include Ticket::Assets
-  include Ticket::SearchIndex
-  include Ticket::Search
-  include Ticket::MergeHistory
+  include ::Ticket::Escalation
+  include ::Ticket::Subject
+  include ::Ticket::Assets
+  include ::Ticket::SearchIndex
+  include ::Ticket::Search
+  include ::Ticket::MergeHistory
 
   store          :preferences
   before_create  :check_generate, :check_defaults, :check_title, :set_default_state, :set_default_priority
@@ -1334,6 +1334,24 @@ def articles
     Ticket::Article.where(ticket_id: id).order(:created_at, :id)
   end
 
+  # Get whichever #last_contact_* was later
+  # This is not identical to #last_contact_at
+  # It returns time to last original (versus follow up) contact
+  # @return [Time, nil]
+  def last_original_update_at
+    [last_contact_agent_at, last_contact_customer_at].compact.max
+  end
+
+  # true if conversation did happen and agent responded
+  # false if customer is waiting for response or agent reached out and customer did not respond yet
+  # @return [Bool]
+  def agent_responded?
+    return false if last_contact_customer_at.blank?
+    return false if last_contact_agent_at.blank?
+
+    last_contact_customer_at < last_contact_agent_at
+  end
+
   private
 
   def check_generate

app/models/ticket/article.rb

@@ -10,6 +10,7 @@ class Ticket::Article < ApplicationModel
   include HasObjectManagerAttributesValidation
 
   include Ticket::Article::Assets
+  include Ticket::Article::HasTicketContactAttributesImpact
 
   belongs_to :ticket, optional: true
   has_one    :ticket_time_accounting, class_name: 'Ticket::TimeAccounting', foreign_key: :ticket_article_id, dependent: :destroy, inverse_of: :ticket_article