From 0b244d00cb91cab00d9324bc9a1c158c054a2162 Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Fri, 19 Aug 2016 11:14:15 +0200 Subject: [PATCH] Do follow up detection also with references header and comparing subject per default. --- app/models/application_model/assets.rb | 3 +- app/models/channel/filter/follow_up_check.rb | 39 +++++++++++ test/unit/email_process_auto_response_test.rb | 10 +-- test/unit/email_process_bounce_test.rb | 2 +- test/unit/email_process_follow_up_test.rb | 65 +++++++++++++++++++ test/unit/email_process_out_of_office_test.rb | 18 ++--- 6 files changed, 121 insertions(+), 16 deletions(-) diff --git a/app/models/application_model/assets.rb b/app/models/application_model/assets.rb index 1ce5cfd3d..5e1914dd6 100644 --- a/app/models/application_model/assets.rb +++ b/app/models/application_model/assets.rb @@ -31,9 +31,10 @@ returns end return data if !self['created_by_id'] && !self['updated_by_id'] + app_model_user = User.to_app_model %w(created_by_id updated_by_id).each { |local_user_id| next if !self[ local_user_id ] - next if data[ User.to_app_model ] && data[ User.to_app_model ][ self[ local_user_id ] ] + next if data[ app_model_user ] && data[ app_model_user ][ self[ local_user_id ] ] user = User.lookup(id: self[ local_user_id ]) next if !user data = user.assets(data) diff --git a/app/models/channel/filter/follow_up_check.rb b/app/models/channel/filter/follow_up_check.rb index 5c9478d8c..080194452 100644 --- a/app/models/channel/filter/follow_up_check.rb +++ b/app/models/channel/filter/follow_up_check.rb @@ -65,5 +65,44 @@ module Channel::Filter::FollowUpCheck end end + # get ticket# from references current email has same subject as inital article + if !mail[:subject].empty? + + # get all references 'References' + 'In-Reply-To' + references = '' + if mail[:references] + references += mail[:references] + end + if mail['in-reply-to'.to_sym] + if references != '' + references += ' ' + end + references += mail['in-reply-to'.to_sym] + end + if references != '' + message_ids = references.split(/\s+/) + message_ids.each { |message_id| + message_id_md5 = Digest::MD5.hexdigest(message_id) + article = Ticket::Article.where(message_id_md5: message_id_md5).order('created_at DESC, id DESC').limit(1).first + next if !article + ticket = article.ticket + next if !ticket + article_first = ticket.articles.first + next if !article_first + + # remove leading "..:\s" and "..[\d+]:\s" e. g. "Re: " or "Re[5]: " + subject_to_check = mail[:subject] + subject_to_check.gsub!(/^(..(\[\d+\])?:\s)+/, '') + + # if subject is different, it's no followup + next if subject_to_check != article_first.subject + + Rails.logger.debug "Follow up for '##{article.ticket.number}' in references with same subject as inital article." + mail[ 'x-zammad-ticket-id'.to_sym ] = article_first.ticket_id + return true + } + end + end + end end diff --git a/test/unit/email_process_auto_response_test.rb b/test/unit/email_process_auto_response_test.rb index 4e6066458..1dcd35869 100644 --- a/test/unit/email_process_auto_response_test.rb +++ b/test/unit/email_process_auto_response_test.rb @@ -11,7 +11,7 @@ Subject: some new subject Some Text" - ticket_p, article_p, user_p, mail = Channel::EmailParser.new.process( {}, email_raw_string) + ticket_p, article_p, user_p, mail = Channel::EmailParser.new.process({}, email_raw_string) assert_equal(true, mail['x-zammad-send-auto-response'.to_sym]) email_raw_string = "From: me@example.com @@ -21,7 +21,7 @@ X-Loop: yes Some Text" - ticket_p, article_p, user_p, mail = Channel::EmailParser.new.process( {}, email_raw_string) + ticket_p, article_p, user_p, mail = Channel::EmailParser.new.process({}, email_raw_string) assert_equal(false, mail['x-zammad-send-auto-response'.to_sym]) email_raw_string = "From: me@example.com @@ -31,7 +31,7 @@ Precedence: Bulk Some Text" - ticket_p, article_p, user_p, mail = Channel::EmailParser.new.process( {}, email_raw_string) + ticket_p, article_p, user_p, mail = Channel::EmailParser.new.process({}, email_raw_string) assert_equal(false, mail['x-zammad-send-auto-response'.to_sym]) email_raw_string = "From: me@example.com @@ -41,7 +41,7 @@ Auto-Submitted: auto-generated Some Text" - ticket_p, article_p, user_p, mail = Channel::EmailParser.new.process( {}, email_raw_string) + ticket_p, article_p, user_p, mail = Channel::EmailParser.new.process({}, email_raw_string) assert_equal(false, mail['x-zammad-send-auto-response'.to_sym]) email_raw_string = "From: me@example.com @@ -52,7 +52,7 @@ X-Auto-Response-Suppress: All Some Text" - ticket_p, article_p, user_p, mail = Channel::EmailParser.new.process( {}, email_raw_string) + ticket_p, article_p, user_p, mail = Channel::EmailParser.new.process({}, email_raw_string) assert_equal(false, mail['x-zammad-send-auto-response'.to_sym]) end diff --git a/test/unit/email_process_bounce_test.rb b/test/unit/email_process_bounce_test.rb index a7cac1293..29fb303f3 100644 --- a/test/unit/email_process_bounce_test.rb +++ b/test/unit/email_process_bounce_test.rb @@ -29,7 +29,7 @@ class EmailProcessBounceTest < ActiveSupport::TestCase ) sleep 1 email_raw_string = IO.binread('test/fixtures/mail33-undelivered-mail-returned-to-sender.box') - ticket_p, article_p, user_p = Channel::EmailParser.new.process( {}, email_raw_string) + ticket_p, article_p, user_p = Channel::EmailParser.new.process({}, email_raw_string) assert_equal(ticket.id, ticket_p.id) assert_equal('new', ticket_p.state.name) end diff --git a/test/unit/email_process_follow_up_test.rb b/test/unit/email_process_follow_up_test.rb index bf13777dd..c1ecd19d1 100644 --- a/test/unit/email_process_follow_up_test.rb +++ b/test/unit/email_process_follow_up_test.rb @@ -257,4 +257,69 @@ Some Text" assert_equal('open', ticket.state.name) end + test 'process with follow up check - ticket initiated by customer without T# in subject and other people in Cc reply to all' do + + # check if follow up based on inital system sender address + setting_orig = Setting.get('postmaster_follow_up_search_in') + Setting.set('postmaster_follow_up_search_in', []) + + subject = "ticket initiated by customer without T# in subject and other people in Cc reply to all #{rand(9999)}" + + email_raw_string = "From: me@example.com +To: my@system.test, bob@example.com +Subject: #{subject} +Message-ID: <123456789-$follow-up-test§-1@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 + email_raw_string = "From: bob@example.com +To: my@system.test, me@example.com +Subject: AW: #{subject} +Message-ID: <123456789-$follow-up-test§-2@linuxhotel.de> +References: <123456789-$follow-up-test§-1@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) + assert_equal(subject, ticket2.title) + + # follow up possible because same subject + email_raw_string = "From: bob@example.com +To: my@system.test, me@example.com +Subject: AW: RE: #{subject} +Message-ID: <123456789-$follow-up-test§-2@linuxhotel.de> +References: <123456789-$follow-up-test§-1@linuxhotel.de> + +Some Text" + + ticket_p3, article_p3, user_p3, mail = Channel::EmailParser.new.process({}, email_raw_string) + ticket3 = Ticket.find(ticket_p3.id) + assert_equal(ticket1.id, ticket3.id) + assert_equal(subject, ticket3.title) + + # follow up not possible because subject has changed + subject = "new subject without ticket ref #{rand(9_999_999)}" + email_raw_string = "From: bob@example.com +To: my@system.test +Subject: #{subject} +Message-ID: <123456789-$follow-up-test§-3@linuxhotel.de> +References: <123456789-$follow-up-test§-1@linuxhotel.de> + +Some Text" + + ticket_p4, article_p4, user_p4, mail = Channel::EmailParser.new.process({}, email_raw_string) + ticket4 = Ticket.find(ticket_p4.id) + assert_not_equal(ticket1.id, ticket4.id) + assert_equal(subject, ticket4.title) + + Setting.set('postmaster_follow_up_search_in', setting_orig) + end + end diff --git a/test/unit/email_process_out_of_office_test.rb b/test/unit/email_process_out_of_office_test.rb index 60d634643..9aaebab69 100644 --- a/test/unit/email_process_out_of_office_test.rb +++ b/test/unit/email_process_out_of_office_test.rb @@ -45,7 +45,7 @@ x-tm-as-user-blocked-sender: No Some Text" - ticket_p, article_p, user_p, mail = Channel::EmailParser.new.process( {}, email_raw_string) + ticket_p, article_p, user_p, mail = Channel::EmailParser.new.process({}, email_raw_string) assert_equal(true, mail['x-zammad-out-of-office'.to_sym]) ticket = Ticket.find(ticket.id) assert_equal(ticket.id, ticket_p.id) @@ -59,7 +59,7 @@ X-MS-Exchange-Inbox-Rules-Loop: aaa.bbb@example.com Some Text 2" - ticket_p, article_p, user_p, mail = Channel::EmailParser.new.process( {}, email_raw_string) + ticket_p, article_p, user_p, mail = Channel::EmailParser.new.process({}, email_raw_string) assert_equal(false, mail['x-zammad-out-of-office'.to_sym]) ticket = Ticket.find(ticket.id) assert_equal(ticket.id, ticket_p.id) @@ -81,7 +81,7 @@ x-exclaimer-md-config: 8c10826d-4052-4c5c-a8e8-e09011276827 Some Text" - ticket_p, article_p, user_p, mail = Channel::EmailParser.new.process( {}, email_raw_string) + ticket_p, article_p, user_p, mail = Channel::EmailParser.new.process({}, email_raw_string) assert_equal(true, mail['x-zammad-out-of-office'.to_sym]) ticket = Ticket.find(ticket.id) assert_equal(ticket.id, ticket_p.id) @@ -125,7 +125,7 @@ X-Mailer: Zimbra 7.1.3_GA_3346 Some Text" - ticket_p, article_p, user_p, mail = Channel::EmailParser.new.process( {}, email_raw_string) + ticket_p, article_p, user_p, mail = Channel::EmailParser.new.process({}, email_raw_string) assert_equal(true, mail['x-zammad-out-of-office'.to_sym]) ticket = Ticket.find(ticket.id) assert_equal(ticket.id, ticket_p.id) @@ -139,7 +139,7 @@ X-Mailer: Zimbra 7.1.3_GA_3346 Some Text 2" - ticket_p, article_p, user_p, mail = Channel::EmailParser.new.process( {}, email_raw_string) + ticket_p, article_p, user_p, mail = Channel::EmailParser.new.process({}, email_raw_string) assert_equal(false, mail['x-zammad-out-of-office'.to_sym]) ticket = Ticket.find(ticket.id) assert_equal(ticket.id, ticket_p.id) @@ -159,7 +159,7 @@ X-Mailer: Zimbra 7.1.3_GA_3346 Some Text" - ticket_p, article_p, user_p, mail = Channel::EmailParser.new.process( {}, email_raw_string) + ticket_p, article_p, user_p, mail = Channel::EmailParser.new.process({}, email_raw_string) assert_equal(true, mail['x-zammad-out-of-office'.to_sym]) ticket = Ticket.find(ticket.id) assert_equal(ticket.id, ticket_p.id) @@ -201,7 +201,7 @@ Auto-submitted: auto-replied; owner-email=\"me@example.com\" Some Text" - ticket_p, article_p, user_p, mail = Channel::EmailParser.new.process( {}, email_raw_string) + ticket_p, article_p, user_p, mail = Channel::EmailParser.new.process({}, email_raw_string) assert_equal(true, mail['x-zammad-out-of-office'.to_sym]) ticket = Ticket.find(ticket.id) assert_equal(ticket.id, ticket_p.id) @@ -214,7 +214,7 @@ Subject: #{ticket.subject_build('some new subject - none')} Some Text 2" - ticket_p, article_p, user_p, mail = Channel::EmailParser.new.process( {}, email_raw_string) + ticket_p, article_p, user_p, mail = Channel::EmailParser.new.process({}, email_raw_string) assert_equal(false, mail['x-zammad-out-of-office'.to_sym]) ticket = Ticket.find(ticket.id) assert_equal(ticket.id, ticket_p.id) @@ -232,7 +232,7 @@ Auto-submitted: auto-replied; owner-email=\"me@example.com\" Some Text" - ticket_p, article_p, user_p, mail = Channel::EmailParser.new.process( {}, email_raw_string) + ticket_p, article_p, user_p, mail = Channel::EmailParser.new.process({}, email_raw_string) assert_equal(true, mail['x-zammad-out-of-office'.to_sym]) ticket = Ticket.find(ticket.id) assert_equal(ticket.id, ticket_p.id)