From 1fab6d8acb481978746d98dccbc3664b879390ba Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Mon, 30 Jan 2017 15:47:12 +0100 Subject: [PATCH] Fixed issue #686 - Follow up detection not working if ticket_hook_position "none" is used. --- app/models/channel/email_parser.rb | 42 ++++++++++---------- app/models/channel/filter/follow_up_check.rb | 4 +- test/unit/email_process_follow_up_test.rb | 37 +++++++++++++++++ 3 files changed, 60 insertions(+), 23 deletions(-) diff --git a/app/models/channel/email_parser.rb b/app/models/channel/email_parser.rb index 568abd637..288335a8c 100644 --- a/app/models/channel/email_parser.rb +++ b/app/models/channel/email_parser.rb @@ -79,14 +79,14 @@ class Channel::EmailParser data[field.name.to_s.downcase.to_sym] = Encode.conv('utf8', field.to_s) # if we need to access the lines by objects later again - data[ "raw-#{field.name.downcase}".to_sym ] = field + data["raw-#{field.name.downcase}".to_sym] = field } # get sender from = nil ['from', 'reply-to', 'return-path'].each { |item| - next if !mail[ item.to_sym ] - from = mail[ item.to_sym ].value + next if !mail[item.to_sym] + from = mail[item.to_sym].value break if from } @@ -376,7 +376,7 @@ returns do not raise an exception - e. g. if used by scheduler parser = Channel::EmailParser.new - ticket, article, user = parser.process(channel, email_raw_string, fakse) + ticket, article, user = parser.process(channel, email_raw_string, false) returns @@ -430,7 +430,7 @@ returns } # check ignore header - if mail[ 'x-zammad-ignore'.to_sym ] == 'true' || mail[ 'x-zammad-ignore'.to_sym ] == true + if mail['x-zammad-ignore'.to_sym] == 'true' || mail['x-zammad-ignore'.to_sym] == true Rails.logger.info "ignored email with msgid '#{mail[:message_id]}' from '#{mail[:from]}' because of x-zammad-ignore header" return true end @@ -446,7 +446,7 @@ returns Transaction.execute(interface_handle: "#{original_interface_handle}.postmaster") do # get sender user - session_user_id = mail[ 'x-zammad-session-user-id'.to_sym ] + session_user_id = mail['x-zammad-session-user-id'.to_sym] if !session_user_id raise 'No x-zammad-session-user-id, no sender set!' end @@ -459,11 +459,11 @@ returns UserInfo.current_user_id = session_user.id # get ticket# based on email headers - if mail[ 'x-zammad-ticket-id'.to_sym ] - ticket = Ticket.find_by(id: mail[ 'x-zammad-ticket-id'.to_sym ]) + if mail['x-zammad-ticket-id'.to_sym] + ticket = Ticket.find_by(id: mail['x-zammad-ticket-id'.to_sym]) end - if mail[ 'x-zammad-ticket-number'.to_sym ] - ticket = Ticket.find_by(number: mail[ 'x-zammad-ticket-number'.to_sym ]) + if mail['x-zammad-ticket-number'.to_sym] + ticket = Ticket.find_by(number: mail['x-zammad-ticket-number'.to_sym]) end # set ticket state to open if not new @@ -482,8 +482,8 @@ returns end # set ticket to open again - if !mail[ 'x-zammad-ticket-followup-state'.to_sym ] - if state_type.name != 'new' && !mail[ 'x-zammad-out-of-office'.to_sym ] + if !mail['x-zammad-ticket-followup-state'.to_sym] && !mail['x-zammad-ticket-followup-state_id'.to_sym] + if state_type.name != 'new' && !mail['x-zammad-out-of-office'.to_sym] ticket.state = Ticket::State.find_by(name: 'open') ticket.save! end @@ -626,25 +626,25 @@ returns # only set value on _id if value/reference lookup exists if mail[ header.to_sym ] - Rails.logger.info "header #{header} found #{mail[ header.to_sym ]}" + Rails.logger.info "header #{header} found #{mail[header.to_sym]}" item_object.class.reflect_on_all_associations.map { |assoc| next if assoc.name.to_s != key_short - Rails.logger.info "ASSOC found #{assoc.class_name} lookup #{mail[ header.to_sym ]}" + Rails.logger.info "ASSOC found #{assoc.class_name} lookup #{mail[header.to_sym]}" item = assoc.class_name.constantize assoc_object = nil assoc_has_object = false if item.respond_to?(:name) assoc_has_object = true - if item.lookup(name: mail[ header.to_sym ]) - assoc_object = item.lookup(name: mail[ header.to_sym ]) + if item.lookup(name: mail[header.to_sym]) + assoc_object = item.lookup(name: mail[header.to_sym]) end elsif item.respond_to?(:login) assoc_has_object = true - if item.lookup(login: mail[ header.to_sym ]) - assoc_object = item.lookup(login: mail[ header.to_sym ]) + if item.lookup(login: mail[header.to_sym]) + assoc_object = item.lookup(login: mail[header.to_sym]) end end @@ -666,9 +666,9 @@ returns if suffix header = "x-zammad-#{header_name}-#{suffix}-#{key}" end - if mail[ header.to_sym ] - Rails.logger.info "header #{header} found #{mail[ header.to_sym ]}" - item_object[key] = mail[ header.to_sym ] + if mail[header.to_sym] + Rails.logger.info "header #{header} found #{mail[header.to_sym]}" + item_object[key] = mail[header.to_sym] end } end diff --git a/app/models/channel/filter/follow_up_check.rb b/app/models/channel/filter/follow_up_check.rb index 8fcdddcef..50346428e 100644 --- a/app/models/channel/filter/follow_up_check.rb +++ b/app/models/channel/filter/follow_up_check.rb @@ -39,7 +39,7 @@ module Channel::Filter::FollowUpCheck end # get ticket# from references - if setting.include?('references') || mail[ 'x-zammad-is-auto-response'.to_sym ] == true + if setting.include?('references') || (mail[ 'x-zammad-is-auto-response'.to_sym ] == true || Setting.get('ticket_hook_position') == 'none') # get all references 'References' + 'In-Reply-To' references = '' @@ -66,7 +66,7 @@ module Channel::Filter::FollowUpCheck end # get ticket# from references current email has same subject as inital article - if !mail[:subject].empty? + if mail[:subject].present? # get all references 'References' + 'In-Reply-To' references = '' diff --git a/test/unit/email_process_follow_up_test.rb b/test/unit/email_process_follow_up_test.rb index a7b3b9d74..f7c12a2bb 100644 --- a/test/unit/email_process_follow_up_test.rb +++ b/test/unit/email_process_follow_up_test.rb @@ -325,4 +325,41 @@ Some Text" Setting.set('postmaster_follow_up_search_in', setting_orig) end + test 'process with follow up check - with none ticket# in subject' do + + setting_orig = Setting.get('postmaster_follow_up_search_in') + Setting.set('postmaster_follow_up_search_in', []) + ticket_hook_position_orig = Setting.get('ticket_hook_position') + Setting.set('ticket_hook_position', 'none') + + subject = 'some title' + email_raw_string = "From: me@example.com +To: bob@example.com +Subject: #{subject} +Message-ID: <123456789-follow-up-test-ticket_hook_position-none@linuxhotel.de> + +Some Text" + + ticket_p1, article_1, user_1, mail = Channel::EmailParser.new.process({}, email_raw_string) + ticket1 = Ticket.find(ticket_p1.id) + assert_equal(subject, ticket1.title) + + # follow up possible because same subject + subject = 'new subject lalala' + email_raw_string = "From: bob@example.com +To: me@example.com +Subject: AW: #{subject} +Message-ID: <123456789-follow-up-test-ticket_hook_position-none-2@linuxhotel.de> +References: <123456789-follow-up-test-ticket_hook_position-none@linuxhotel.de> + +Some Text" + + ticket_p2, article_p2, user_p2, mail = Channel::EmailParser.new.process({}, email_raw_string) + ticket2 = Ticket.find(ticket_p2.id) + assert_equal(ticket1.id, ticket2.id) + + Setting.set('ticket_hook_position', ticket_hook_position_orig) + Setting.set('postmaster_follow_up_search_in', setting_orig) + end + end