From 8d0c0c634a1999d61093fc15fa88d1476e03db90 Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Tue, 12 Jun 2018 13:59:18 +0200 Subject: [PATCH] Fixed issue #2069 - Cannot close tickets when you reply to an email at the same time. --- .../observer/ticket/escalation_update.rb | 12 +- app/models/observer/ticket/reset_new_state.rb | 9 +- .../observer/ticket/user_ticket_counter.rb | 12 +- test/unit/ticket_state_change_test.rb | 123 ++++++++++++++++++ 4 files changed, 138 insertions(+), 18 deletions(-) create mode 100644 test/unit/ticket_state_change_test.rb diff --git a/app/models/observer/ticket/escalation_update.rb b/app/models/observer/ticket/escalation_update.rb index 0ee7c72e0..c65a07b1a 100644 --- a/app/models/observer/ticket/escalation_update.rb +++ b/app/models/observer/ticket/escalation_update.rb @@ -3,11 +3,7 @@ class Observer::Ticket::EscalationUpdate < ActiveRecord::Observer observe 'ticket' - def after_create(record) - _check(record) - end - - def after_update(record) + def after_commit(record) _check(record) end @@ -16,9 +12,11 @@ class Observer::Ticket::EscalationUpdate < ActiveRecord::Observer def _check(record) # return if we run import mode - return false if Setting.get('import_mode') + return true if Setting.get('import_mode') - return false if !record.saved_changes? + return true if record.destroyed? + + return true if !record.saved_changes? record.escalation_calculation end diff --git a/app/models/observer/ticket/reset_new_state.rb b/app/models/observer/ticket/reset_new_state.rb index 05e20ada3..8caa211cc 100644 --- a/app/models/observer/ticket/reset_new_state.rb +++ b/app/models/observer/ticket/reset_new_state.rb @@ -6,10 +6,10 @@ class Observer::Ticket::ResetNewState < ActiveRecord::Observer def after_create(record) # return if we run import mode - return if Setting.get('import_mode') + return true if Setting.get('import_mode') # only change state if not processed via postmaster - return if ApplicationHandleInfo.postmaster? + return true if ApplicationHandleInfo.postmaster? # if article in internal return true if record.internal @@ -21,12 +21,13 @@ class Observer::Ticket::ResetNewState < ActiveRecord::Observer return true if !Ticket::Article::Type.lookup(id: record.type_id).communication # if current ticket state is still new - ticket = Ticket.lookup(id: record.ticket_id) + ticket = Ticket.find_by(id: record.ticket_id) + return true if !ticket new_state = Ticket::State.find_by(default_create: true) return true if ticket.state_id != new_state.id state = Ticket::State.find_by(default_follow_up: true) - return if !state + return true if !state # set ticket to open ticket.state_id = state.id diff --git a/app/models/observer/ticket/user_ticket_counter.rb b/app/models/observer/ticket/user_ticket_counter.rb index ffa899ac3..b2de03b1e 100644 --- a/app/models/observer/ticket/user_ticket_counter.rb +++ b/app/models/observer/ticket/user_ticket_counter.rb @@ -3,20 +3,18 @@ class Observer::Ticket::UserTicketCounter < ActiveRecord::Observer observe 'ticket' - def after_create(record) - user_ticket_counter_update(record) - end - - def after_update(record) + def after_commit(record) user_ticket_counter_update(record) end def user_ticket_counter_update(record) # return if we run import mode - return if Setting.get('import_mode') + return true if Setting.get('import_mode') - return if !record.customer_id + return true if record.destroyed? + + return true if !record.customer_id # send background job Delayed::Job.enqueue( diff --git a/test/unit/ticket_state_change_test.rb b/test/unit/ticket_state_change_test.rb new file mode 100644 index 000000000..066e4bf87 --- /dev/null +++ b/test/unit/ticket_state_change_test.rb @@ -0,0 +1,123 @@ + +require 'test_helper' + +class TicketStateChangeTest < ActiveSupport::TestCase + + test 'check if after reply ticket is open' do + + ticket1 = Ticket.create!( + title: 'com test 1', + group: Group.lookup(name: 'Users'), + customer_id: 2, + state: Ticket::State.lookup(name: 'new'), + priority: Ticket::Priority.lookup(name: '2 normal'), + updated_by_id: 1, + created_by_id: 1, + ) + assert(ticket1, 'ticket created') + + article1 = Ticket::Article.create!( + ticket_id: ticket1.id, + from: 'some_customer_com-1@example.com', + to: 'some_zammad_com-1@example.com', + subject: 'com test 1', + message_id: 'some@id_com_1', + body: 'some message 123', + internal: false, + sender: Ticket::Article::Sender.find_by(name: 'Customer'), + type: Ticket::Article::Type.find_by(name: 'email'), + updated_by_id: 1, + created_by_id: 1, + ) + + Observer::Transaction.commit + Scheduler.worker(true) + + ticket1.reload + assert_equal('new', ticket1.state.name) + + ticket1.with_lock do + ticket1.update!(state_id: Ticket::State.find_by(name: 'new').id) + article2 = Ticket::Article.create!( + ticket_id: ticket1.id, + from: 'some_zammad_com-1@example.com', + to: 'some_customer_com-1@example.com', + subject: 'com test 1', + message_id: 'some@id_com_2', + body: 'some message 123', + internal: false, + sender: Ticket::Article::Sender.find_by(name: 'Agent'), + type: Ticket::Article::Type.find_by(name: 'email'), + updated_by_id: 1, + created_by_id: 1, + ) + end + + Observer::Transaction.commit + Scheduler.worker(true) + + ticket1.reload + assert_equal('open', ticket1.state.name) + + end + + test 'check if after reply ticket is closed' do + + ticket1 = Ticket.create!( + title: 'com test 1', + group: Group.lookup(name: 'Users'), + customer_id: 2, + state: Ticket::State.lookup(name: 'new'), + priority: Ticket::Priority.lookup(name: '2 normal'), + updated_by_id: 1, + created_by_id: 1, + ) + assert(ticket1, 'ticket created') + + article1 = Ticket::Article.create!( + ticket_id: ticket1.id, + from: 'some_customer_com-1@example.com', + to: 'some_zammad_com-1@example.com', + subject: 'com test 1', + message_id: 'some@id_com_1', + body: 'some message 123', + internal: false, + sender: Ticket::Article::Sender.find_by(name: 'Customer'), + type: Ticket::Article::Type.find_by(name: 'email'), + updated_by_id: 1, + created_by_id: 1, + ) + + Observer::Transaction.commit + Scheduler.worker(true) + + ticket1.reload + assert_equal('new', ticket1.state.name) + + ticket1.with_lock do + ticket1.update!(state_id: Ticket::State.find_by(name: 'closed').id) + + article2 = Ticket::Article.create!( + ticket_id: ticket1.id, + from: 'some_zammad_com-1@example.com', + to: 'some_customer_com-1@example.com', + subject: 'com test 1', + message_id: 'some@id_com_2', + body: 'some message 123', + internal: false, + sender: Ticket::Article::Sender.find_by(name: 'Agent'), + type: Ticket::Article::Type.find_by(name: 'email'), + updated_by_id: 1, + created_by_id: 1, + ) + end + + Observer::Transaction.commit + Scheduler.worker(true) + + ticket1.reload + assert_equal('closed', ticket1.state.name) + + end + +end