Browse Source

Fixed issue #1375 - assignment_timeout in group settings has no effect.

Martin Edenhofer 7 years ago
parent
commit
fffca5b549

+ 34 - 0
app/models/observer/ticket/last_owner_update.rb

@@ -0,0 +1,34 @@
+# Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/
+
+class Observer::Ticket::LastOwnerUpdate < ActiveRecord::Observer
+  observe 'ticket'
+
+  def before_create(record)
+    _check('create', record)
+  end
+
+  def before_update(record)
+    _check('update', record)
+  end
+
+  private
+
+  def _check(type, record)
+
+    # return if we run import mode
+    return true if Setting.get('import_mode')
+
+    # check if owner has changed
+    if type == 'update'
+      return true if record.changes['owner_id'].blank?
+    end
+
+    # check if owner is nobody
+    if record.owner_id.blank? || record.owner_id == 1
+      record.last_owner_update_at = nil
+      return true
+    end
+
+    record.last_owner_update_at = Time.zone.now
+  end
+end

+ 44 - 2
app/models/ticket.rb

@@ -48,6 +48,7 @@ class Ticket < ApplicationModel
                                      :last_contact_at,
                                      :last_contact_agent_at,
                                      :last_contact_customer_at,
+                                     :last_owner_update_at,
                                      :preferences
 
   history_attributes_ignored :create_article_type_id,
@@ -113,7 +114,7 @@ returns
     pending_action = Ticket::StateType.find_by(name: 'pending action')
     ticket_states_pending_action = Ticket::State.where(state_type_id: pending_action)
                                                 .where.not(next_state_id: nil)
-    if !ticket_states_pending_action.empty?
+    if ticket_states_pending_action.present?
       next_state_map = {}
       ticket_states_pending_action.each { |state|
         next_state_map[state.id] = state.next_state_id
@@ -137,7 +138,7 @@ returns
     pending_reminder = Ticket::StateType.find_by(name: 'pending reminder')
     ticket_states_pending_reminder = Ticket::State.where(state_type_id: pending_reminder)
 
-    if !ticket_states_pending_reminder.empty?
+    if ticket_states_pending_reminder.present?
       reminder_state_map = {}
       ticket_states_pending_reminder.each { |state|
         reminder_state_map[state.id] = state.next_state_id
@@ -228,6 +229,47 @@ returns
 
 =begin
 
+processes tickets which auto unassign time has reached
+
+  processed_tickets = Ticket.process_auto_unassign
+
+returns
+
+  processed_tickets = [<Ticket>, ...]
+
+=end
+
+  def self.process_auto_unassign
+
+    # process pending action tickets
+    state_ids = Ticket::State.by_category(:work_on).pluck(:id)
+    return [] if state_ids.blank?
+    result = []
+    groups = Group.where(active: true).where('assignment_timeout IS NOT NULL AND groups.assignment_timeout != 0')
+    return [] if groups.blank?
+    groups.each { |group|
+      next if group.assignment_timeout.blank?
+      ticket_ids = Ticket.where('state_id IN (?) AND owner_id != 1 AND group_id = ?', state_ids, group.id).limit(600).pluck(:id)
+      ticket_ids.each { |ticket_id|
+        ticket = Ticket.find_by(id: ticket_id)
+        next if !ticket
+        minutes_since_last_assignment = Time.zone.now - ticket.last_owner_update_at
+        next if (minutes_since_last_assignment / 60) <= group.assignment_timeout
+        Transaction.execute do
+          ticket.owner_id      = 1
+          ticket.updated_at    = Time.zone.now
+          ticket.updated_by_id = 1
+          ticket.save!
+        end
+        result.push ticket
+      }
+    }
+
+    result
+  end
+
+=begin
+
 merge tickets
 
   ticket = Ticket.find(123)

+ 1 - 0
config/application.rb

@@ -23,6 +23,7 @@ module Zammad
     config.active_record.observers =
       'observer::_session',
       'observer::_ticket::_close_time',
+      'observer::_ticket::_last_owner_update',
       'observer::_ticket::_user_ticket_counter',
       'observer::_ticket::_article_changes',
       'observer::_ticket::_article::_fillup_from_general',

+ 2 - 0
db/migrate/20120101000010_create_ticket.rb

@@ -69,6 +69,7 @@ class CreateTicket < ActiveRecord::Migration
       t.column :last_contact_at,                  :timestamp, limit: 3,   null: true
       t.column :last_contact_agent_at,            :timestamp, limit: 3,   null: true
       t.column :last_contact_customer_at,         :timestamp, limit: 3,   null: true
+      t.column :last_owner_update_at,             :timestamp, limit: 3,   null: true
       t.column :create_article_type_id,           :integer,               null: true
       t.column :create_article_sender_id,         :integer,               null: true
       t.column :article_count,                    :integer,               null: true
@@ -103,6 +104,7 @@ class CreateTicket < ActiveRecord::Migration
     add_index :tickets, [:last_contact_at]
     add_index :tickets, [:last_contact_agent_at]
     add_index :tickets, [:last_contact_customer_at]
+    add_index :tickets, [:last_owner_update_at]
     add_index :tickets, [:create_article_type_id]
     add_index :tickets, [:create_article_sender_id]
     add_index :tickets, [:created_by_id]

+ 55 - 0
db/migrate/20170830000001_last_owner_update.rb

@@ -0,0 +1,55 @@
+class LastOwnerUpdate < ActiveRecord::Migration
+  def up
+
+    # return if it's a new setup
+    return if !Setting.find_by(name: 'system_init_done')
+
+    # reset assignment_timeout to prevent unwanted things happen
+    Group.all.each { |group|
+      group.assignment_timeout = nil
+      group.save!
+    }
+
+    add_column :tickets, :last_owner_update_at, :timestamp, limit: 3, null: true
+    add_index :tickets, [:last_owner_update_at]
+    Ticket.reset_column_information
+
+    Scheduler.create_if_not_exists(
+      name: 'Process auto unassign tickets',
+      method: 'Ticket.process_auto_unassign',
+      period: 10.minutes,
+      prio: 1,
+      active: true,
+    )
+
+    state_ids = Ticket::State.by_category(:work_on).pluck(:id)
+    if state_ids.present?
+      ticket_ids = Ticket.where('tickets.state_id IN (?) AND tickets.owner_id != 1', state_ids).order(created_at: :desc).limit(1000).pluck(:id)
+      ticket_ids.each { |ticket_id|
+        ticket = Ticket.find_by(id: ticket_id)
+        next if !ticket
+        ticket.last_owner_update_at = last_owner_update_at(ticket)
+        ticket.save!
+      }
+    end
+  end
+
+  def last_owner_update_at(ticket)
+    type = History::Type.lookup(name: 'updated')
+    if type
+      object = History::Object.lookup(name: 'Ticket')
+      if object
+        attribute = History::Attribute.lookup(name: 'owner')
+        if attribute
+          history = History.where(o_id: ticket.id, history_type_id: type.id, history_object_id: object.id, history_attribute_id: attribute.id).where.not(id_to: 1).order(created_at: :desc).limit(1)
+          if history.present?
+            return history.first.created_at
+          end
+        end
+      end
+    end
+    return nil if ticket.owner_id == 1
+    ticket.created_at
+  end
+
+end

+ 258 - 0
test/unit/ticket_last_owner_update_test.rb

@@ -0,0 +1,258 @@
+# encoding: utf-8
+require 'test_helper'
+
+class TicketLastOwnerUpdateTest < ActiveSupport::TestCase
+
+  setup do
+    group = Group.create_or_update(
+      name: 'LastOwnerUpdate',
+      assignment_timeout: 60,
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+    roles = Role.where(name: 'Agent')
+    @agent1 = User.create_or_update(
+      login: 'ticket-assignment_timeout-agent1@example.com',
+      firstname: 'Overview',
+      lastname: 'Agent1',
+      email: 'ticket-assignment_timeout-agent1@example.com',
+      password: 'agentpw',
+      active: true,
+      roles: roles,
+      groups: Group.all,
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+  end
+
+  test 'last_owner_update_at check' do
+
+    ticket = Ticket.create!(
+      title: 'assignment_timeout test 1',
+      group: Group.lookup(name: 'LastOwnerUpdate'),
+      customer_id: 2,
+      state: Ticket::State.lookup(name: 'new'),
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+    assert_nil(ticket.last_owner_update_at)
+
+    travel 1.hour
+    ticket.owner = @agent1
+    ticket.save!
+    assert_equal(ticket.last_owner_update_at.to_s, ticket.updated_at.to_s)
+
+    ticket = Ticket.create!(
+      title: 'assignment_timeout test 1',
+      group: Group.lookup(name: 'LastOwnerUpdate'),
+      customer_id: 2,
+      state: Ticket::State.lookup(name: 'closed'),
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+    assert_nil(ticket.last_owner_update_at)
+
+    travel 1.hour
+    ticket.owner = @agent1
+    ticket.save!
+    assert_equal(ticket.last_owner_update_at.to_s, ticket.updated_at.to_s)
+
+    ticket = Ticket.create!(
+      title: 'assignment_timeout test 1',
+      group: Group.lookup(name: 'LastOwnerUpdate'),
+      owner: @agent1,
+      customer_id: 2,
+      state: Ticket::State.lookup(name: 'new'),
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+    assert_equal(ticket.last_owner_update_at.to_s, ticket.updated_at.to_s)
+
+    ticket.owner_id = 1
+    ticket.save!
+    assert_nil(ticket.last_owner_update_at)
+
+    ticket = Ticket.create!(
+      title: 'assignment_timeout test 1',
+      group: Group.lookup(name: 'LastOwnerUpdate'),
+      owner: @agent1,
+      customer_id: 2,
+      state: Ticket::State.lookup(name: 'closed'),
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+    assert_equal(ticket.last_owner_update_at.to_s, ticket.updated_at.to_s)
+
+    ticket.owner_id = 1
+    ticket.save!
+    assert_nil(ticket.last_owner_update_at)
+
+    ticket = Ticket.create!(
+      title: 'assignment_timeout test 2',
+      group: Group.lookup(name: 'Users'),
+      customer_id: 2,
+      state: Ticket::State.lookup(name: 'new'),
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+    assert_nil(ticket.last_owner_update_at)
+
+    travel 1.hour
+    ticket.owner = @agent1
+    ticket.save!
+    assert_equal(ticket.last_owner_update_at.to_s, ticket.updated_at.to_s)
+
+    ticket = Ticket.create!(
+      title: 'assignment_timeout test 2',
+      group: Group.lookup(name: 'Users'),
+      customer_id: 2,
+      state: Ticket::State.lookup(name: 'closed'),
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+    assert_nil(ticket.last_owner_update_at)
+
+    travel 1.hour
+    ticket.owner = @agent1
+    ticket.save!
+    assert_equal(ticket.last_owner_update_at.to_s, ticket.updated_at.to_s)
+
+    ticket = Ticket.create!(
+      title: 'assignment_timeout test 2',
+      group: Group.lookup(name: 'Users'),
+      owner: @agent1,
+      customer_id: 2,
+      state: Ticket::State.lookup(name: 'new'),
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+    assert_equal(ticket.last_owner_update_at.to_s, ticket.updated_at.to_s)
+
+    ticket.owner_id = 1
+    ticket.save!
+    assert_nil(ticket.last_owner_update_at)
+
+    ticket = Ticket.create!(
+      title: 'assignment_timeout test 2',
+      group: Group.lookup(name: 'Users'),
+      owner: @agent1,
+      customer_id: 2,
+      state: Ticket::State.lookup(name: 'closed'),
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+    assert_equal(ticket.last_owner_update_at.to_s, ticket.updated_at.to_s)
+
+    ticket.owner_id = 1
+    ticket.save!
+    assert_nil(ticket.last_owner_update_at)
+
+  end
+
+  test 'last_owner_update_at assignment_timeout check' do
+
+    ticket1 = Ticket.create!(
+      title: 'assignment_timeout test 1',
+      group: Group.lookup(name: 'LastOwnerUpdate'),
+      customer_id: 2,
+      state: Ticket::State.lookup(name: 'new'),
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+    assert_nil(ticket1.last_owner_update_at)
+
+    ticket2 = Ticket.create!(
+      title: 'assignment_timeout test 2',
+      group: Group.lookup(name: 'LastOwnerUpdate'),
+      owner: @agent1,
+      customer_id: 2,
+      state: Ticket::State.lookup(name: 'new'),
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+    assert_equal(ticket2.last_owner_update_at.to_s, ticket2.updated_at.to_s)
+
+    ticket3 = Ticket.create!(
+      title: 'assignment_timeout test 3',
+      group: Group.lookup(name: 'LastOwnerUpdate'),
+      owner: @agent1,
+      customer_id: 2,
+      state: Ticket::State.lookup(name: 'closed'),
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+    assert_equal(ticket3.last_owner_update_at.to_s, ticket3.updated_at.to_s)
+
+    ticket4 = Ticket.create!(
+      title: 'assignment_timeout test 4',
+      group: Group.lookup(name: 'Users'),
+      customer_id: 2,
+      state: Ticket::State.lookup(name: 'new'),
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+    assert_nil(ticket4.last_owner_update_at)
+
+    ticket5 = Ticket.create!(
+      title: 'assignment_timeout test 5',
+      group: Group.lookup(name: 'Users'),
+      owner: @agent1,
+      customer_id: 2,
+      state: Ticket::State.lookup(name: 'new'),
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+    assert_equal(ticket5.last_owner_update_at.to_s, ticket5.updated_at.to_s)
+
+    travel 55.minutes
+    Ticket.process_auto_unassign
+
+    ticket1after = Ticket.find(ticket1.id)
+    assert_nil(ticket1.last_owner_update_at)
+    assert_equal(ticket1.updated_at.to_s, ticket1after.updated_at.to_s)
+
+    ticket2after = Ticket.find(ticket2.id)
+    assert_equal(ticket2.last_owner_update_at.to_s, ticket2after.last_owner_update_at.to_s)
+    assert_equal(ticket2.updated_at.to_s, ticket2after.updated_at.to_s)
+
+    ticket3after = Ticket.find(ticket3.id)
+    assert_equal(ticket3.last_owner_update_at.to_s, ticket3after.last_owner_update_at.to_s)
+    assert_equal(ticket3.updated_at.to_s, ticket3after.updated_at.to_s)
+
+    ticket4after = Ticket.find(ticket4.id)
+    assert_nil(ticket4.last_owner_update_at)
+    assert_equal(ticket4.updated_at.to_s, ticket4after.updated_at.to_s)
+
+    ticket5after = Ticket.find(ticket5.id)
+    assert_equal(ticket5after.owner_id, @agent1.id)
+    assert_equal(ticket5.updated_at.to_s, ticket5after.updated_at.to_s)
+
+    travel 15.minutes
+    Ticket.process_auto_unassign
+    ticket2_updated_at = Time.current
+
+    ticket1after = Ticket.find(ticket1.id)
+    assert_nil(ticket1.last_owner_update_at)
+    assert_equal(ticket1.updated_at.to_s, ticket1after.updated_at.to_s)
+
+    ticket2after = Ticket.find(ticket2.id)
+    assert_nil(ticket2after.last_owner_update_at)
+    assert_equal(ticket2after.owner_id, 1)
+    assert_equal(ticket2_updated_at.to_s, ticket2after.updated_at.to_s)
+
+    ticket3after = Ticket.find(ticket3.id)
+    assert_equal(ticket3after.owner_id, @agent1.id)
+    assert_equal(ticket3.last_owner_update_at.to_s, ticket3after.last_owner_update_at.to_s)
+    assert_equal(ticket3.updated_at.to_s, ticket3after.updated_at.to_s)
+
+    ticket4after = Ticket.find(ticket4.id)
+    assert_nil(ticket4.last_owner_update_at)
+    assert_equal(ticket4.updated_at.to_s, ticket4after.updated_at.to_s)
+
+    ticket5after = Ticket.find(ticket5.id)
+    assert_equal(ticket5after.owner_id, @agent1.id)
+    assert_equal(ticket5.updated_at.to_s, ticket5after.updated_at.to_s)
+
+  end
+
+end