From 7d052439bb3918f2538886caf032bb6b8444944d Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Sat, 20 Aug 2016 23:50:33 +0200 Subject: [PATCH] Improved tests for ticket creation by agent oder system email address via postmaster. --- app/models/channel/filter/identify_sender.rb | 49 +++-- .../filter/sender_is_system_address.rb | 13 ++ ..._sender_is_system_address_or_agent_test.rb | 174 ++++++++++++++++++ ...l_process_sender_is_system_address_test.rb | 92 --------- test/unit/ticket_article_communicate_test.rb | 159 ++++++++++++++++ 5 files changed, 378 insertions(+), 109 deletions(-) create mode 100644 test/unit/email_process_sender_is_system_address_or_agent_test.rb delete mode 100644 test/unit/email_process_sender_is_system_address_test.rb create mode 100644 test/unit/ticket_article_communicate_test.rb diff --git a/app/models/channel/filter/identify_sender.rb b/app/models/channel/filter/identify_sender.rb index a89f77f91..e2c3a7aed 100644 --- a/app/models/channel/filter/identify_sender.rb +++ b/app/models/channel/filter/identify_sender.rb @@ -4,15 +4,15 @@ module Channel::Filter::IdentifySender def self.run(_channel, mail) - user_id = mail[ 'x-zammad-customer-id'.to_sym ] - user = nil - if user_id - user = User.lookup(id: user_id) - if !user - Rails.logger.debug "Invalid x-zammad-customer-id header '#{user_id}', no such user." + customer_user_id = mail[ 'x-zammad-customer-id'.to_sym ] + customer_user = nil + if customer_user_id + customer_user = User.lookup(id: customer_user_id) + if !customer_user + Rails.logger.debug "Invalid x-zammad-customer-id header '#{customer_user_id}', no such user." else - Rails.logger.debug "Took customer form x-zammad-customer-id header '#{user_id}'." - if user + Rails.logger.debug "Took customer form x-zammad-customer-id header '#{customer_user_id}'." + if customer_user create_recipients(mail) return end @@ -21,12 +21,12 @@ module Channel::Filter::IdentifySender # check if sender exists in database if mail[ 'x-zammad-customer-login'.to_sym ] - user = User.find_by(login: mail[ 'x-zammad-customer-login'.to_sym ]) + customer_user = User.find_by(login: mail[ 'x-zammad-customer-login'.to_sym ]) end - if !user - user = User.find_by(email: mail[ 'x-zammad-customer-email'.to_sym ] || mail[:from_email]) + if !customer_user + customer_user = User.find_by(email: mail[ 'x-zammad-customer-email'.to_sym ]) end - if !user + if !customer_user # get correct customer if mail[ 'x-zammad-ticket-create-article-sender'.to_sym ] == 'Agent' @@ -37,7 +37,11 @@ module Channel::Filter::IdentifySender if mail[to] && mail[to].addrs items = mail[to].addrs items.each { |item| - user = user_create( + + # skip if recipient is system email + next if EmailAddress.find_by(email: item.address) + + customer_user = user_create( login: item.address, firstname: item.display_name, email: item.address, @@ -49,8 +53,8 @@ module Channel::Filter::IdentifySender Rails.logger.error 'ERROR: SenderIsSystemAddress: ' + e.inspect end end - if !user - user = user_create( + if !customer_user + customer_user = user_create( login: mail[ 'x-zammad-customer-login'.to_sym ] || mail[ 'x-zammad-customer-email'.to_sym ] || mail[:from_email], firstname: mail[ 'x-zammad-customer-firstname'.to_sym ] || mail[:from_display_name], lastname: mail[ 'x-zammad-customer-lastname'.to_sym ], @@ -61,7 +65,7 @@ module Channel::Filter::IdentifySender create_recipients(mail) - mail[ 'x-zammad-customer-id'.to_sym ] = user.id + mail[ 'x-zammad-customer-id'.to_sym ] = customer_user.id end # create to and cc user @@ -108,7 +112,18 @@ module Channel::Filter::IdentifySender # return existing user = User.find_by(login: data[:email].downcase) - return user if user + + # check if firstname or lastname need to be updated + if user + if user.firstname.empty? && user.lastname.empty? + if !data[:firstname].empty? + user.update_attributes( + firstname: data[:firstname] + ) + end + end + return user + end # create new user role_ids = Role.signup_role_ids diff --git a/app/models/channel/filter/sender_is_system_address.rb b/app/models/channel/filter/sender_is_system_address.rb index e34602732..ce30a4687 100644 --- a/app/models/channel/filter/sender_is_system_address.rb +++ b/app/models/channel/filter/sender_is_system_address.rb @@ -27,6 +27,19 @@ module Channel::Filter::SenderIsSystemAddress Rails.logger.error 'ERROR: SenderIsSystemAddress: ' + e.inspect end + # check if sender is agent + return if mail[:from_email].empty? + begin + user = User.find_by(email: mail[:from_email]) + return if !user + return if !user.permissions?('ticket.agent') + mail[ 'x-zammad-ticket-create-article-sender'.to_sym ] = 'Agent' + mail[ 'x-zammad-article-sender'.to_sym ] = 'Agent' + return true + rescue => e + Rails.logger.error 'ERROR: SenderIsSystemAddress: ' + e.inspect + end + true end end diff --git a/test/unit/email_process_sender_is_system_address_or_agent_test.rb b/test/unit/email_process_sender_is_system_address_or_agent_test.rb new file mode 100644 index 000000000..2fe284b84 --- /dev/null +++ b/test/unit/email_process_sender_is_system_address_or_agent_test.rb @@ -0,0 +1,174 @@ +# encoding: utf-8 +require 'test_helper' + +class EmailProcessSenderIsSystemAddressOrAgent < ActiveSupport::TestCase + + setup do + EmailAddress.create_or_update( + channel_id: 1, + realname: 'My System', + email: 'myzammad@system.test', + active: true, + updated_by_id: 1, + created_by_id: 1, + ) + end + + test 'process email with sender as system address check' do + subject = "some new subject #{rand(9_999_999)}" + email_raw_string = "From: me+is+customer@example.com +To: customer@example.com +Subject: #{subject} + +Some Text" + + ticket_p, article_p, user_p, mail = Channel::EmailParser.new.process({}, email_raw_string) + ticket = Ticket.find(ticket_p.id) + article = Ticket::Article.find(article_p.id) + assert_equal(subject, ticket.title) + assert_equal('new', ticket.state.name) + assert_equal('Customer', ticket.create_article_sender.name) + assert_equal('Customer', article.sender.name) + assert_equal('me+is+customer@example.com', ticket.customer.email) + + # check article sender + customer of ticket + subject = "some new subject #{rand(9_999_999)}" + email_raw_string = "From: myzammad@system.test +To: me+is+customer@example.com, customer@example.com +Subject: #{subject} +Message-ID: <123456789-1@linuxhotel.de> + + +Some Text" + + ticket_p, article_p, user_p, mail = Channel::EmailParser.new.process({}, email_raw_string) + ticket = Ticket.find(ticket_p.id) + article = Ticket::Article.find(article_p.id) + + assert_equal(subject, ticket.title) + assert_equal('open', ticket.state.name) + assert_equal('Agent', ticket.create_article_sender.name) + assert_equal('Agent', article.sender.name) + assert_equal('me+is+customer@example.com', ticket.customer.email) + + # 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', []) + + # follow up possible because same subject + email_raw_string = "From: me+is+customer@example.com +To: myzammad@system.test +Subject: #{subject} +Message-ID: <123456789-2@linuxhotel.de> +References: <123456789-1@linuxhotel.de> + +Some Text" + + ticket_p2, article_p, user_p, mail = Channel::EmailParser.new.process({}, email_raw_string) + ticket2 = Ticket.find(ticket_p2.id) + article = Ticket::Article.find(article_p.id) + assert_equal(subject, ticket2.title) + assert_equal(ticket.id, ticket2.id) + + # follow up not possible because subject has changed + subject = "new subject without ticket ref #{rand(9_999_999)}" + email_raw_string = "From: me+is+customer@example.com +To: myzammad@system.test +Subject: #{subject} +Message-ID: <123456789-3@linuxhotel.de> +References: <123456789-1@linuxhotel.de> + +Some Text" + + ticket_p2, article_p, user_p, mail = Channel::EmailParser.new.process({}, email_raw_string) + ticket2 = Ticket.find(ticket_p2.id) + article = Ticket::Article.find(article_p.id) + assert_not_equal(ticket.id, ticket2.id) + assert_equal(subject, ticket2.title) + assert_equal('new', ticket2.state.name) + + Setting.set('postmaster_follow_up_search_in', setting_orig) + + end + + test 'process email with sender as agent address check' do + + # create customer + roles = Role.where(name: 'Customer') + customer1 = User.create_or_update( + login: 'ticket-system-sender-customer1@example.com', + firstname: 'system-sender', + lastname: 'Customer1', + email: 'ticket-system-sender-customer1@example.com', + password: 'customerpw', + active: true, + roles: roles, + updated_by_id: 1, + created_by_id: 1, + ) + + # create agent + groups = Group.all + roles = Role.where(name: 'Agent') + agent1 = User.create_or_update( + login: 'ticket-system-sender-agent1@example.com', + firstname: 'system-sender', + lastname: 'Agent1', + email: 'ticket-system-sender-agent1@example.com', + password: 'agentpw', + active: true, + roles: roles, + groups: groups, + updated_by_id: 1, + created_by_id: 1, + ) + + # process customer email + email_raw_string = "From: ticket-system-sender-customer1@example.com +To: myzammad@system.test +Subject: some subject #1 + +Some Text" + + ticket_p, article_p, user_p, mail = Channel::EmailParser.new.process({}, email_raw_string) + ticket = Ticket.find(ticket_p.id) + article = Ticket::Article.find(article_p.id) + assert_equal('some subject #1', ticket.title) + assert_equal('new', ticket.state.name) + assert_equal('Customer', ticket.create_article_sender.name) + assert_equal('Customer', article.sender.name) + assert_equal('ticket-system-sender-customer1@example.com', ticket.customer.email) + + # process agent email + email_raw_string = "From: ticket-system-sender-agent1@example.com +To: ticket-system-sender-customer1@example.com, myzammad@system.test +Subject: some subject #2 + +Some Text" + + ticket_p, article_p, user_p, mail = Channel::EmailParser.new.process({}, email_raw_string) + ticket = Ticket.find(ticket_p.id) + article = Ticket::Article.find(article_p.id) + assert_equal('some subject #2', ticket.title) + assert_equal('open', ticket.state.name) + assert_equal('Agent', ticket.create_article_sender.name) + assert_equal('Agent', article.sender.name) + assert_equal('ticket-system-sender-customer1@example.com', ticket.customer.email) + + email_raw_string = "From: ticket-system-sender-agent1@example.com +To: myzammad@system.test, ticket-system-sender-customer1@example.com +Subject: some subject #3 + +Some Text" + + ticket_p, article_p, user_p, mail = Channel::EmailParser.new.process({}, email_raw_string) + ticket = Ticket.find(ticket_p.id) + article = Ticket::Article.find(article_p.id) + assert_equal('some subject #3', ticket.title) + assert_equal('open', ticket.state.name) + assert_equal('Agent', ticket.create_article_sender.name) + assert_equal('Agent', article.sender.name) + assert_equal('ticket-system-sender-customer1@example.com', ticket.customer.email) + + end +end diff --git a/test/unit/email_process_sender_is_system_address_test.rb b/test/unit/email_process_sender_is_system_address_test.rb deleted file mode 100644 index fd8b39c88..000000000 --- a/test/unit/email_process_sender_is_system_address_test.rb +++ /dev/null @@ -1,92 +0,0 @@ -# encoding: utf-8 -require 'test_helper' - -class EmailProcessSenderIsSystemAddress < ActiveSupport::TestCase - - test 'process with ticket creates and system address check' do - - EmailAddress.create_or_update( - channel_id: 1, - realname: 'My System', - email: 'my@system.test', - active: true, - updated_by_id: 1, - created_by_id: 1, - ) - subject = "some new subject #{rand(9_999_999)}" - email_raw_string = "From: me@example.com -To: customer@example.com -Subject: #{subject} - -Some Text" - - ticket_p, article_p, user_p, mail = Channel::EmailParser.new.process({}, email_raw_string) - ticket = Ticket.find(ticket_p.id) - article = Ticket::Article.find(article_p.id) - assert_equal(subject, ticket.title) - assert_equal('new', ticket.state.name) - assert_equal('Customer', ticket.create_article_sender.name) - assert_equal('Customer', article.sender.name) - assert_equal('me@example.com', ticket.customer.email) - - # check article sender + customer of ticket - subject = "some new subject #{rand(9_999_999)}" - email_raw_string = "From: my@system.test -To: me@example.com, customer@example.com -Subject: #{subject} -Message-ID: <123456789-1@linuxhotel.de> - - -Some Text" - - ticket_p, article_p, user_p, mail = Channel::EmailParser.new.process({}, email_raw_string) - ticket = Ticket.find(ticket_p.id) - article = Ticket::Article.find(article_p.id) - - assert_equal(subject, ticket.title) - assert_equal('open', ticket.state.name) - assert_equal('Agent', ticket.create_article_sender.name) - assert_equal('Agent', article.sender.name) - assert_equal('me@example.com', ticket.customer.email) - - # 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', []) - - # follow up possible because same subject - email_raw_string = "From: me@example.com -To: my@system.test -Subject: #{subject} -Message-ID: <123456789-2@linuxhotel.de> -References: <123456789-1@linuxhotel.de> - -Some Text" - - ticket_p2, article_p, user_p, mail = Channel::EmailParser.new.process({}, email_raw_string) - ticket2 = Ticket.find(ticket_p2.id) - article = Ticket::Article.find(article_p.id) - assert_equal(subject, ticket2.title) - assert_equal(ticket.id, ticket2.id) - - # follow up not possible because subject has changed - subject = "new subject without ticket ref #{rand(9_999_999)}" - email_raw_string = "From: me@example.com -To: my@system.test -Subject: #{subject} -Message-ID: <123456789-3@linuxhotel.de> -References: <123456789-1@linuxhotel.de> - -Some Text" - - ticket_p2, article_p, user_p, mail = Channel::EmailParser.new.process({}, email_raw_string) - ticket2 = Ticket.find(ticket_p2.id) - article = Ticket::Article.find(article_p.id) - assert_not_equal(ticket.id, ticket2.id) - assert_equal(subject, ticket2.title) - assert_equal('new', ticket2.state.name) - - Setting.set('postmaster_follow_up_search_in', setting_orig) - - end - -end diff --git a/test/unit/ticket_article_communicate_test.rb b/test/unit/ticket_article_communicate_test.rb new file mode 100644 index 000000000..f89002746 --- /dev/null +++ b/test/unit/ticket_article_communicate_test.rb @@ -0,0 +1,159 @@ +# encoding: utf-8 +require 'test_helper' + +class TicketArticleCommunicateTest < ActiveSupport::TestCase + + test 'via config' do + + # via application server + ApplicationHandleInfo.current = 'application_server' + 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') + + article_email1_1 = 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, + ) + assert_equal('some_customer_com-1@example.com', article_email1_1.from) + assert_equal('some_zammad_com-1@example.com', article_email1_1.to) + assert_equal(0, email_count('some_customer_com-1@example.com')) + assert_equal(0, email_count('some_zammad_com-1@example.com')) + Scheduler.worker(true) + assert_equal('some_customer_com-1@example.com', article_email1_1.from) + assert_equal('some_zammad_com-1@example.com', article_email1_1.to) + assert_equal(0, email_count('some_customer_com-1@example.com')) + assert_equal(0, email_count('some_zammad_com-1@example.com')) + + article_email1_2 = 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, + ) + assert_equal('Zammad ', article_email1_2.from) + assert_equal('some_customer_com-1@example.com', article_email1_2.to) + assert_equal(0, email_count('some_customer_com-1@example.com')) + assert_equal(0, email_count('some_zammad_com-1@example.com')) + Scheduler.worker(true) + assert_equal('Zammad ', article_email1_2.from) + assert_equal('some_customer_com-1@example.com', article_email1_2.to) + assert_equal(1, email_count('some_customer_com-1@example.com')) + assert_equal(0, email_count('some_zammad_com-1@example.com')) + + # via scheduler (e. g. postmaster) + ApplicationHandleInfo.current = 'scheduler.postmaster' + ticket2 = Ticket.create( + title: 'com test 2', + 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(ticket2, 'ticket created') + + article_email2_1 = Ticket::Article.create( + ticket_id: ticket2.id, + from: 'some_customer_com-2@example.com', + to: 'some_zammad_com-2@example.com', + subject: 'com test 2', + 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, + ) + assert_equal('some_customer_com-2@example.com', article_email2_1.from) + assert_equal('some_zammad_com-2@example.com', article_email2_1.to) + assert_equal(0, email_count('some_customer_com-2@example.com')) + assert_equal(0, email_count('some_zammad_com-2@example.com')) + Scheduler.worker(true) + assert_equal('some_customer_com-2@example.com', article_email2_1.from) + assert_equal('some_zammad_com-2@example.com', article_email2_1.to) + assert_equal(0, email_count('some_customer_com-2@example.com')) + assert_equal(0, email_count('some_zammad_com-2@example.com')) + + ApplicationHandleInfo.current = 'scheduler.postmaster' + article_email2_2 = Ticket::Article.create( + ticket_id: ticket2.id, + from: 'some_zammad_com-2@example.com', + to: 'some_customer_com-2@example.com', + subject: 'com test 2', + 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, + ) + assert_equal(0, email_count('some_customer_com-2@example.com')) + assert_equal(0, email_count('some_zammad_com-2@example.com')) + assert_equal('some_zammad_com-2@example.com', article_email2_2.from) + assert_equal('some_customer_com-2@example.com', article_email2_2.to) + Scheduler.worker(true) + assert_equal(0, email_count('some_customer_com-2@example.com')) + assert_equal(0, email_count('some_zammad_com-2@example.com')) + assert_equal('some_zammad_com-2@example.com', article_email2_2.from) + assert_equal('some_customer_com-2@example.com', article_email2_2.to) + end + + test 'postmaster process - do not change from - verify article sender' do + email_raw_string = "From: my_zammad@example.com +To: customer@example.com +Subject: some subject +X-Zammad-Article-Sender: Agent + +Some Text" + + ticket_p, article_p, user_p, mail = Channel::EmailParser.new.process({ trusted: true }, email_raw_string) + ticket = Ticket.find(ticket_p.id) + article_email = Ticket::Article.find(article_p.id) + + assert_equal(0, email_count('my_zammad@example.com')) + assert_equal(0, email_count('customer@example.com')) + assert_equal('my_zammad@example.com', article_email.from) + assert_equal('customer@example.com', article_email.to) + assert_equal('Agent', article_email.sender.name) + assert_equal('email', article_email.type.name) + assert_equal(1, article_email.ticket.articles.count) + + Scheduler.worker(true) + article_email = Ticket::Article.find(article_p.id) + assert_equal(0, email_count('my_zammad@example.com')) + assert_equal(0, email_count('customer@example.com')) + assert_equal('my_zammad@example.com', article_email.from) + assert_equal('customer@example.com', article_email.to) + assert_equal('Agent', article_email.sender.name) + assert_equal('email', article_email.type.name) + assert_equal(1, article_email.ticket.articles.count) + end + +end