Improved tests for ticket creation by agent oder system email address via postmaster.

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 ]
+    customer_user_id = mail[ 'x-zammad-customer-id'.to_sym ]
-    user = nil
+    customer_user = nil
-    if user_id
+    if customer_user_id
-      user = User.lookup(id: user_id)
+      customer_user = User.lookup(id: customer_user_id)
-      if !user
+      if !customer_user
-        Rails.logger.debug "Invalid x-zammad-customer-id header '#{user_id}', no such 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}'."
+        Rails.logger.debug "Took customer form x-zammad-customer-id header '#{customer_user_id}'."
-        if user
+        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
+    if !customer_user
-      user = User.find_by(email: mail[ 'x-zammad-customer-email'.to_sym ] || mail[:from_email])
+      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
+      if !customer_user
-        user = user_create(
+        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

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

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

test/unit/email_process_sender_is_system_address_test.rb

@@ -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

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 <zammad@localhost>', 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 <zammad@localhost>', 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