Improve auto assignment for inactive agents or agents in inactive groups to default "-" user for issue #1816

app/models/ticket.rb

@@ -23,7 +23,7 @@ class Ticket < ApplicationModel
   store          :preferences
   before_create  :check_generate, :check_defaults, :check_title, :set_default_state, :set_default_priority
   after_create   :check_escalation_update
-  before_update  :check_defaults, :check_title, :reset_pending_time
+  before_update  :check_defaults, :check_title, :reset_pending_time, :check_owner_active
   after_update   :check_escalation_update
 
   validates :group_id, presence: true
@@ -1207,4 +1207,11 @@ result
     self.priority_id = default_ticket_priority.id
     true
   end
+
+  def check_owner_active
+    return true if owner.login == '-' # return when ticket is unassigned
+    return true if owner.groups.any?(&:active?) && owner.active? # return if user in any groups assigned is active and user account is active
+    self.owner = User.find_by(login: '-') # else set the owner of the ticket to the default user as unassigned
+    true
+  end
 end

test/unit/history_test.rb

@@ -122,7 +122,7 @@ class HistoryTest < ActiveSupport::TestCase
             value_to: 'Unit Test 2 (äöüß) - update!',
           },
           {
-            result: true,
+            result: false,
             history_object: 'Ticket',
             history_type: 'updated',
             history_attribute: 'owner',

test/unit/ticket_trigger_test.rb

@@ -877,6 +877,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       updated_by_id: 1,
       created_by_id: 1,
     )
+    groups = Group.where(name: 'Users')
     roles = Role.where(name: 'Agent')
     agent1 = User.create_or_update(
       login: 'agent-has-changed@example.com',
@@ -886,6 +887,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       password: 'agentpw',
       active: true,
       roles: roles,
+      groups: groups,
       updated_at: '2015-02-05 16:37:00',
       updated_by_id: 1,
       created_by_id: 1,
@@ -1190,6 +1192,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       created_by_id: 1,
       updated_by_id: 1,
     )
+    groups = Group.where(name: 'Users')
     roles = Role.where(name: 'Agent')
     agent = User.create_or_update(
       login: 'agent@example.com',
@@ -1199,6 +1202,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       password: 'agentpw',
       active: true,
       roles: roles,
+      groups: groups,
       updated_by_id: 1,
       created_by_id: 1,
     )
@@ -1325,6 +1329,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       created_by_id: 1,
       updated_by_id: 1,
     )
+    groups = Group.where(name: 'Users')
     roles = Role.where(name: 'Agent')
     agent = User.create_or_update(
       login: 'agent@example.com',
@@ -1334,6 +1339,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       password: 'agentpw',
       active: true,
       roles: roles,
+      groups: groups,
       updated_by_id: 1,
       created_by_id: 1,
     )
@@ -1438,6 +1444,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       updated_by_id: 1,
     )
     roles = Role.where(name: 'Agent')
+    groups = Group.where(name: 'Users')
     agent = User.create_or_update(
       login: 'agent@example.com',
       firstname: 'Trigger',
@@ -1446,6 +1453,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       password: 'agentpw',
       active: true,
       roles: roles,
+      groups: groups,
       updated_by_id: 1,
       created_by_id: 1,
     )
@@ -1548,6 +1556,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       created_by_id: 1,
       updated_by_id: 1,
     )
+    groups = Group.where(name: 'Users')
     roles = Role.where(name: 'Agent')
     agent = User.create_or_update(
       login: 'agent@example.com',
@@ -1557,6 +1566,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       password: 'agentpw',
       active: true,
       roles: roles,
+      groups: groups,
       updated_by_id: 1,
       created_by_id: 1,
     )
@@ -1663,6 +1673,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       created_by_id: 1,
       updated_by_id: 1,
     )
+    groups = Group.where(name: 'Users')
     roles = Role.where(name: 'Agent')
     agent1 = User.create_or_update(
       login: 'agent@example.com',
@@ -1672,6 +1683,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       password: 'agentpw',
       active: true,
       roles: roles,
+      groups: groups,
       updated_by_id: 1,
       created_by_id: 1,
     )
@@ -1683,6 +1695,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       password: 'agentpw',
       active: true,
       roles: roles,
+      groups: groups,
       updated_by_id: 1,
       created_by_id: 1,
     )
@@ -1855,6 +1868,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       created_by_id: 1,
       updated_by_id: 1,
     )
+    groups = Group.where(name: 'Users')
     roles = Role.where(name: 'Agent')
     agent = User.create_or_update(
       login: 'agent@example.com',
@@ -1864,6 +1878,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       password: 'agentpw',
       active: true,
       roles: roles,
+      groups: groups,
       updated_by_id: 1,
       created_by_id: 1,
     )
@@ -1989,6 +2004,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       created_by_id: 1,
       updated_by_id: 1,
     )
+    groups = Group.where(name: 'Users')
     roles = Role.where(name: 'Agent')
     agent = User.create_or_update(
       login: 'agent@example.com',
@@ -1998,6 +2014,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       password: 'agentpw',
       active: true,
       roles: roles,
+      groups: groups,
       updated_by_id: 1,
       created_by_id: 1,
     )
@@ -2129,6 +2146,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       created_by_id: 1,
       updated_by_id: 1,
     )
+    groups = Group.where(name: 'Users')
     roles = Role.where(name: 'Agent')
     agent1 = User.create_or_update(
       login: 'agent1@example.com',
@@ -2138,6 +2156,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       password: 'agentpw',
       active: true,
       roles: roles,
+      groups: groups,
       updated_by_id: 1,
       created_by_id: 1,
     )
@@ -2149,6 +2168,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       password: 'agentpw',
       active: true,
       roles: roles,
+      groups: groups,
       updated_by_id: 1,
       created_by_id: 1,
     )
@@ -2261,6 +2281,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       created_by_id: 1,
       updated_by_id: 1,
     )
+    groups = Group.where(name: 'Users')
     roles = Role.where(name: 'Agent')
     agent = User.create_or_update(
       login: 'agent@example.com',
@@ -2270,6 +2291,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       password: 'agentpw',
       active: true,
       roles: roles,
+      groups: groups,
       updated_by_id: 1,
       created_by_id: 1,
     )
@@ -2544,6 +2566,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       updated_by_id: 1,
     )
 
+    groups = Group.where(name: 'Users')
     roles = Role.where(name: 'Agent')
     agent = User.create_or_update(
       login: 'agent@example.com',
@@ -2553,6 +2576,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       password: 'agentpw',
       active: true,
       roles: roles,
+      groups: groups,
       updated_by_id: 1,
       created_by_id: 1,
     )
@@ -2972,6 +2996,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       password: 'adminpw',
       active: true,
       roles: Role.where(name: %w[Admin Agent]),
+      groups: Group.where(name: 'Users'),
       updated_by_id: 1,
       created_by_id: 1,
     )
@@ -3907,6 +3932,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
 
   test 'change owner' do
     roles = Role.where(name: 'Agent')
+    groups = Group.where(name: 'Users')
     agent1 = User.create_or_update(
       login: 'agent-has-changed@example.com',
       firstname: 'Has Changed',
@@ -3915,6 +3941,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       password: 'agentpw',
       active: true,
       roles: roles,
+      groups: groups,
       updated_at: '2015-02-05 16:37:00',
       updated_by_id: 1,
       created_by_id: 1,
@@ -3928,6 +3955,7 @@ class TicketTriggerTest < ActiveSupport::TestCase
       password: 'agentpw',
       active: true,
       roles: roles,
+      groups: groups,
       updated_at: '2015-02-05 16:37:00',
       updated_by_id: 1,
       created_by_id: 1,

test/unit/ticket_update_test.rb

@@ -0,0 +1,191 @@
+require 'test_helper'
+
+class TicketUpdateTest < ActiveSupport::TestCase
+
+  setup do
+    UserInfo.current_user_id = 1
+    Group.create_or_update(
+      name: 'Disabled Group',
+      follow_up_possible: 'yes',
+      follow_up_assignment: true,
+      active: false,
+    )
+    groups = Group.where(name: 'Users')
+    roles = Role.where(name: 'Agent')
+    @agent1 = User.create_or_update(
+      login: 'ticket-customer-organization-update-agent1@example.com',
+      firstname: 'FollowUpCheck',
+      lastname: 'Agent1',
+      email: 'ticket-customer-organization-update-agent1@example.com',
+      password: 'agentpw',
+      active: true,
+      roles: roles,
+      groups: groups,
+    )
+    roles = Role.where(name: 'Customer')
+    @organization1 = Organization.create_if_not_exists(
+      name: 'Customer Organization Update',
+    )
+    @customer1 = User.create_or_update(
+      login: 'ticket-customer-organization-update-customer1@example.com',
+      firstname: 'FollowUpCheck',
+      lastname: 'Customer1',
+      email: 'ticket-customer-organization-update-customer1@example.com',
+      password: 'customerpw',
+      active: true,
+      organization_id: @organization1.id,
+      roles: roles,
+    )
+    UserInfo.current_user_id = nil
+  end
+
+  test 'create ticket, update owner to user with disabled group' do
+
+    ticket = Ticket.create!(
+      title: "some title1\n äöüß",
+      group: Group.lookup(name: 'Users'),
+      customer_id: @customer1.id,
+      owner_id: @agent1.id,
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+    assert(ticket, 'ticket created')
+    assert_equal(@customer1.id, ticket.customer.id)
+    assert_equal(@organization1.id, ticket.organization.id)
+
+    @agent1.groups = Group.where(name: 'Disabled Group')
+    @agent1.save!
+
+    ticket.owner = @agent1
+    ticket.save!
+
+    ticket.reload
+    assert_equal('-', ticket.owner.login) # reassigned to default agent
+  end
+
+  test 'create ticket, update owner to user which is inactive' do
+
+    ticket = Ticket.create!(
+      title: "some title1\n äöüß",
+      group: Group.lookup(name: 'Users'),
+      customer_id: @customer1.id,
+      owner_id: @agent1.id,
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+    assert(ticket, 'ticket created')
+    assert_equal(@customer1.id, ticket.customer.id)
+    assert_equal(@organization1.id, ticket.organization.id)
+
+    @agent1.active = false
+    @agent1.save!
+
+    ticket.owner = @agent1
+    ticket.save!
+
+    ticket.reload
+    assert_equal('-', ticket.owner.login) # reassigned to default agent
+  end
+
+  test 'create ticket, update owner to user which active and is in active group' do
+
+    ticket = Ticket.create!(
+      title: "some title1\n äöüß",
+      group: Group.lookup(name: 'Users'),
+      customer_id: @customer1.id,
+      owner_id: @agent1.id,
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+    assert(ticket, 'ticket created')
+    assert_equal(@customer1.id, ticket.customer.id)
+    assert_equal(@organization1.id, ticket.organization.id)
+
+    ticket.owner = @agent1
+    ticket.save!
+
+    ticket.reload
+    assert_equal(ticket.owner.login, 'ticket-customer-organization-update-agent1@example.com') # should not be reassigned
+  end
+
+  test 'check if ticket is unassigned on follow up via model if owner in a group is inactive' do
+    ticket = Ticket.create!(
+      title: 'follow up check for invalid owner',
+      group: Group.lookup(name: 'Users'),
+      customer: @customer1,
+      owner: @agent1,
+      state: Ticket::State.lookup(name: 'closed'),
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+    article = Ticket::Article.create!(
+      ticket_id: ticket.id,
+      from: 'some_sender@example.com',
+      to: 'some_recipient@example.com',
+      subject: 'follow up check',
+      body: 'some message article',
+      internal: false,
+      sender: Ticket::Article::Sender.lookup(name: 'Agent'),
+      type: Ticket::Article::Type.lookup(name: 'email'),
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+
+    @agent1.groups = Group.where(name: 'Disabled Group')
+    @agent1.save!
+
+    email_raw = "From: me@example.com
+To: customer@example.com
+Subject: #{ticket.subject_build('some new subject')}
+
+Some Text"
+
+    ticket_p, article_p, user_p = Channel::EmailParser.new.process({}, email_raw)
+    assert_equal(ticket.id, ticket_p.id)
+
+    assert_equal('open', ticket_p.state.name)
+    assert_equal('-', ticket_p.owner.login)
+
+  end
+
+  test 'check if ticket is unassigned on follow up via email if current owner is inactive' do
+
+    ticket = Ticket.create!(
+      title: 'follow up check for invalid owner',
+      group: Group.lookup(name: 'Users'),
+      customer: @customer1,
+      owner: @agent1,
+      state: Ticket::State.lookup(name: 'closed'),
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+    article = Ticket::Article.create!(
+      ticket_id: ticket.id,
+      from: 'some_sender@example.com',
+      to: 'some_recipient@example.com',
+      subject: 'follow up check',
+      body: 'some message article',
+      internal: false,
+      sender: Ticket::Article::Sender.lookup(name: 'Agent'),
+      type: Ticket::Article::Type.lookup(name: 'email'),
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+
+    @agent1.active = false
+    @agent1.save!
+
+    email_raw = "From: me@example.com
+To: customer@example.com
+Subject: #{ticket.subject_build('some new subject')}
+
+Some Text"
+
+    ticket_p, article_p, user_p = Channel::EmailParser.new.process({}, email_raw)
+    assert_equal(ticket.id, ticket_p.id)
+
+    assert_equal('open', ticket_p.state.name)
+    assert_equal('-', ticket_p.owner.login)
+  end
+
+end