From 5940a9f6de81df3135796bf0680563d82fe6aae0 Mon Sep 17 00:00:00 2001 From: Muhammad Nuzaihan Date: Sat, 10 Feb 2018 18:40:57 +0800 Subject: [PATCH] Improve auto assignment for inactive agents or agents in inactive groups to default "-" user --- app/models/ticket.rb | 9 +- test/unit/history_test.rb | 2 +- test/unit/ticket_trigger_test.rb | 28 +++++ test/unit/ticket_update_test.rb | 191 +++++++++++++++++++++++++++++++ 4 files changed, 228 insertions(+), 2 deletions(-) create mode 100644 test/unit/ticket_update_test.rb diff --git a/app/models/ticket.rb b/app/models/ticket.rb index 3fdfe0c92..83b6723d2 100644 --- a/app/models/ticket.rb +++ b/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 diff --git a/test/unit/history_test.rb b/test/unit/history_test.rb index 51c06e078..9c58aa4e9 100644 --- a/test/unit/history_test.rb +++ b/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', diff --git a/test/unit/ticket_trigger_test.rb b/test/unit/ticket_trigger_test.rb index e04c0b7f9..88c679f9a 100644 --- a/test/unit/ticket_trigger_test.rb +++ b/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, diff --git a/test/unit/ticket_update_test.rb b/test/unit/ticket_update_test.rb new file mode 100644 index 000000000..21744b74a --- /dev/null +++ b/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