From ae0e90ba208a78a70350c090012a0bace98b5553 Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Sun, 30 Aug 2015 23:49:42 +0200 Subject: [PATCH] Added feature to search for followups in body, attachments and references. --- app/models/channel/email_parser.rb | 2 +- app/models/channel/filter/bounce_check.rb | 5 +- app/models/channel/filter/follow_up_check.rb | 65 +++++++++- db/migrate/20150830000002_update_settings2.rb | 50 ++++++++ db/seeds.rb | 7 +- test/unit/email_process_test.rb | 118 +++++++++++++++++- 6 files changed, 236 insertions(+), 11 deletions(-) create mode 100644 db/migrate/20150830000002_update_settings2.rb diff --git a/app/models/channel/email_parser.rb b/app/models/channel/email_parser.rb index fb4199167..d5d5d0228 100644 --- a/app/models/channel/email_parser.rb +++ b/app/models/channel/email_parser.rb @@ -357,7 +357,7 @@ retrns rescue => e Rails.logger.error "can't run postmaster pre filter #{backend}" Rails.logger.error e.inspect - return false + fail e end } diff --git a/app/models/channel/filter/bounce_check.rb b/app/models/channel/filter/bounce_check.rb index 38ad39be6..64b77dfd5 100644 --- a/app/models/channel/filter/bounce_check.rb +++ b/app/models/channel/filter/bounce_check.rb @@ -16,10 +16,11 @@ module Channel::Filter::BounceCheck result = Channel::EmailParser.new.parse(attachment[:data]) next if !result[:message_id] message_id_md5 = Digest::MD5.hexdigest(result[:message_id]) - article = Ticket::Article.where(message_id_md5: message_id_md5).order('id DESC').limit(1).first + article = Ticket::Article.where(message_id_md5: message_id_md5).order('created_at DESC, id DESC').limit(1).first if article + Rails.logger.debug "Follow up for '##{article.ticket.number}' in bounce email." mail[ 'x-zammad-ticket-id'.to_sym ] = article.ticket_id - break + return true end } diff --git a/app/models/channel/filter/follow_up_check.rb b/app/models/channel/filter/follow_up_check.rb index 1d22c7828..bdf573578 100644 --- a/app/models/channel/filter/follow_up_check.rb +++ b/app/models/channel/filter/follow_up_check.rb @@ -7,8 +7,67 @@ module Channel::Filter::FollowUpCheck return if mail[ 'x-zammad-ticket-id'.to_sym ] # get ticket# from subject - ticket = Ticket::Number.check( mail[:subject] ) - return if !ticket - mail[ 'x-zammad-ticket-id'.to_sym ] = ticket.id + ticket = Ticket::Number.check(mail[:subject]) + if ticket + Rails.logger.debug "Follow up for '##{ticket.number}' in subject." + mail[ 'x-zammad-ticket-id'.to_sym ] = ticket.id + return true + end + + setting = Setting.get('postmaster_follow_up_search_in') + + # get ticket# from body + if setting.include?('body') + ticket = Ticket::Number.check(mail[:body]) + if ticket + Rails.logger.debug "Follow up for '##{ticket.number}' in body." + mail[ 'x-zammad-ticket-id'.to_sym ] = ticket.id + return true + end + end + + # get ticket# from attachment + if setting.include?('attachment') && mail[:attachments] + mail[:attachments].each {|attachment| + next if !attachment[:data] + ticket = Ticket::Number.check(attachment[:data]) + if ticket + Rails.logger.debug "Follow up for '##{ticket.number}' in attachment." + mail[ 'x-zammad-ticket-id'.to_sym ] = ticket.id + return true + end + } + end + + # get ticket# from references + if setting.include?('references') + + # 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+/) + sender_type_agent = Ticket::Article::Sender.lookup(name:'Agent') + sender_type_system = Ticket::Article::Sender.lookup(name:'System') + message_ids.each {|message_id| + message_id_md5 = Digest::MD5.hexdigest(message_id) + article = Ticket::Article.where(message_id_md5: message_id_md5, sender_id: [sender_type_agent.id, sender_type_system.id]).order('created_at DESC, id DESC').limit(1).first + if article + Rails.logger.debug "Follow up for '##{article.ticket.number}' in references." + mail[ 'x-zammad-ticket-id'.to_sym ] = article.ticket_id + return true + end + } + end + end + end end diff --git a/db/migrate/20150830000002_update_settings2.rb b/db/migrate/20150830000002_update_settings2.rb new file mode 100644 index 000000000..a6d203115 --- /dev/null +++ b/db/migrate/20150830000002_update_settings2.rb @@ -0,0 +1,50 @@ +class UpdateSettings2 < ActiveRecord::Migration + def up + Setting.create_or_update( + title: 'Additional follow up detection', + name: 'postmaster_follow_up_search_in', + area: 'Email::Base', + description: 'In default the follow up check is done via the subject of an email. With this setting you can add more fields where the follow up ckeck is executed. "References" - Executes follow up check on In-Reply-To or References headers for mails. "Body" - Executes follow up check in mail body. "Attachment" - Executes follow up check in mail attachments.', + options: { + form: [ + { + display: '', + null: true, + name: 'postmaster_follow_up_search_in', + tag: 'checkbox', + options: { + 'references' => 'References', + 'body' => 'Body', + 'attachment' => 'Attachment', + }, + }, + ], + }, + state: [], + frontend: false + ) + Setting.create_or_update( + title: 'Ticket Hook Position', + name: 'ticket_hook_position', + area: 'Ticket::Base', + description: 'The format of the subject. "Left" means "[Ticket#12345] Some Subject", "Right" means "Some Subject [Ticket#12345]", "None" means "Some Subject" and no ticket number. In the last case you should enable "postmaster_follow_up_search_in" to recognize followups based on email headers and/or body.', + options: { + form: [ + { + display: '', + null: true, + name: 'ticket_hook_position', + tag: 'select', + options: { + 'left' => 'Left', + 'right' => 'Right', + 'none' => 'None', + }, + }, + ], + }, + state: 'right', + frontend: false + ) + end +end diff --git a/db/seeds.rb b/db/seeds.rb index 223ce7baa..c092a16dd 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -753,7 +753,7 @@ Setting.create_if_not_exists( title: 'Ticket Hook Position', name: 'ticket_hook_position', area: 'Ticket::Base', - description: 'The format of the subject. "Left" means "[Ticket#12345] Some Subject", "Right" means "Some Subject [Ticket#12345]", "None" means "Some Subject" and no ticket number. In the last case you should enable PostmasterFollowupSearchInRaw or PostmasterFollowUpSearchInReferences to recognize followups based on email headers and/or body.', + description: 'The format of the subject. "Left" means "[Ticket#12345] Some Subject", "Right" means "Some Subject [Ticket#12345]", "None" means "Some Subject" and no ticket number. In the last case you should enable "postmaster_follow_up_search_in" to recognize followups based on email headers and/or body.', options: { form: [ { @@ -1103,7 +1103,7 @@ Setting.create_if_not_exists( title: 'Additional follow up detection', name: 'postmaster_follow_up_search_in', area: 'Email::Base', - description: '"References" - Executes follow up checks on In-Reply-To or References headers for mails that don\'t have a ticket number in the subject. "Body" - Executes follow up mail body checks in mails that don\'t have a ticket number in the subject. "Attachment" - Executes follow up mail attachments checks in mails that don\'t have a ticket number in the subject. "Raw" - Executes follow up plain/raw mail checks in mails that don\'t have a ticket number in the subject.', + description: 'In default the follow up check is done via the subject of an email. With this setting you can add more fields where the follow up ckeck is executed. "References" - Executes follow up check on In-Reply-To or References headers for mails. "Body" - Executes follow up check in mail body. "Attachment" - Executes follow up check in mail attachments.', options: { form: [ { @@ -1115,12 +1115,11 @@ Setting.create_if_not_exists( 'references' => 'References', 'body' => 'Body', 'attachment' => 'Attachment', - 'raw' => 'Raw', }, }, ], }, - state: ['subject'], + state: [], frontend: false ) diff --git a/test/unit/email_process_test.rb b/test/unit/email_process_test.rb index 982b86306..192f4e3a9 100644 --- a/test/unit/email_process_test.rb +++ b/test/unit/email_process_test.rb @@ -2050,7 +2050,123 @@ Some Text', email_raw_string = IO.read('test/fixtures/mail33-undelivered-mail-returned-to-sender.box') ticket_p, article_p, user_p = Channel::EmailParser.new.process( {}, email_raw_string) - assert_equal(ticket_p.id, ticket.id) + assert_equal(ticket.id, ticket_p.id) + end + + test 'process with follow up check' do + + ticket = Ticket.create( + title: 'follow up check', + 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, + ) + article = Ticket::Article.create( + ticket_id: ticket.id, + from: 'some_sender@example.com', + to: 'some_recipient@example.com', + subject: 'follow up check', + message_id: '<20150830145601.30.608882@edenhofer.zammad.com>', + body: 'some message article', + internal: false, + sender: Ticket::Article::Sender.where(name: 'Agent').first, + type: Ticket::Article::Type.where(name: 'email').first, + updated_by_id: 1, + created_by_id: 1, + ) + + email_raw_string_subject = "From: me@example.com +To: customer@example.com +Subject: #{ticket.subject_build('some new subject')} + +Some Text" + + email_raw_string_body = "From: me@example.com +To: customer@example.com +Subject: no reference + +Some Text #{ticket.subject_build('some new subject')} " + + email_raw_string_attachment = "From: me@example.com +Content-Type: multipart/mixed; boundary=\"Apple-Mail=_ED77AC8D-FB6F-40E5-8FBE-D41FF5E1BAF2\" +Subject: no reference +Message-Id: +Date: Sun, 30 Aug 2015 23:20:54 +0200 +To: Martin Edenhofer +Mime-Version: 1.0 (Mac OS X Mail 8.2 \(2104\)) +X-Mailer: Apple Mail (2.2104) + + +--Apple-Mail=_ED77AC8D-FB6F-40E5-8FBE-D41FF5E1BAF2 +Content-Transfer-Encoding: 7bit +Content-Type: text/plain; + charset=us-ascii + +no reference +--Apple-Mail=_ED77AC8D-FB6F-40E5-8FBE-D41FF5E1BAF2 +Content-Disposition: attachment; + filename=test1.txt +Content-Type: text/plain; + name=\"test.txt\" +Content-Transfer-Encoding: 7bit + +Some Text #{ticket.subject_build('some new subject')} + +--Apple-Mail=_ED77AC8D-FB6F-40E5-8FBE-D41FF5E1BAF2--" + + email_raw_string_references1 = "From: me@example.com +To: customer@example.com +Subject: no reference +In-Reply-To: <20150830145601.30.608882@edenhofer.zammad.com> +References: + +no reference " + + email_raw_string_references2 = "From: me@example.com +To: customer@example.com +Subject: no reference +References: <20150830145601.30.608882@edenhofer.zammad.com> + +no reference " + + ticket_p, article_p, user_p = Channel::EmailParser.new.process( {}, email_raw_string_subject) + assert_equal(ticket.id, ticket_p.id) + + ticket_p, article_p, user_p = Channel::EmailParser.new.process( {}, email_raw_string_body) + assert_not_equal(ticket.id, ticket_p.id) + + ticket_p, article_p, user_p = Channel::EmailParser.new.process( {}, email_raw_string_attachment) + assert_not_equal(ticket.id, ticket_p.id) + + ticket_p, article_p, user_p = Channel::EmailParser.new.process( {}, email_raw_string_references1) + assert_not_equal(ticket.id, ticket_p.id) + + ticket_p, article_p, user_p = Channel::EmailParser.new.process( {}, email_raw_string_references2) + assert_not_equal(ticket.id, ticket_p.id) + + setting_orig = Setting.get('postmaster_follow_up_search_in') + Setting.set('postmaster_follow_up_search_in', ['body', 'attachment', 'references']) + + ticket_p, article_p, user_p = Channel::EmailParser.new.process( {}, email_raw_string_subject) + assert_equal(ticket.id, ticket_p.id) + + ticket_p, article_p, user_p = Channel::EmailParser.new.process( {}, email_raw_string_body) + assert_equal(ticket.id, ticket_p.id) + + ticket_p, article_p, user_p = Channel::EmailParser.new.process( {}, email_raw_string_attachment) + assert_equal(ticket.id, ticket_p.id) + + ticket_p, article_p, user_p = Channel::EmailParser.new.process( {}, email_raw_string_references1) + assert_equal(ticket.id, ticket_p.id) + + ticket_p, article_p, user_p = Channel::EmailParser.new.process( {}, email_raw_string_references2) + assert_equal(ticket.id, ticket_p.id) + + Setting.set('postmaster_follow_up_search_in', setting_orig) + end test 'process with postmaster filter' do