Added tests for email retry to deliver or add a note it delivery was not possible.

.gitlab-ci.yml

@@ -72,6 +72,17 @@ test:integration:email_helper:
     - ruby -I test/ test/integration/email_helper_test.rb
     - rake db:drop
 
+test:integration:email_deliver:
+  stage: test
+  tags:
+    - core
+  script:
+    - export RAILS_ENV=test
+    - rake db:create
+    - rake db:migrate
+    - ruby -I test/ test/integration/email_deliver_test.rb
+    - rake db:drop
+
 test:integration:twitter:
   stage: test
   tags:

app/models/channel.rb

@@ -244,6 +244,7 @@ send via account
       self.status_out = 'error'
       self.last_log_out = error
       save
+      raise error
     end
     result
   end

app/models/observer/ticket/article/communicate_email/background_job.rb

@@ -15,11 +15,17 @@ class Observer::Ticket::Article::CommunicateEmail::BackgroundJob
                 ticket.subject_build(record.subject)
               end
 
+    # set retry count
+    if !record.preferences['delivery_retry']
+      record.preferences['delivery_retry'] = 0
+    end
+    record.preferences['delivery_retry'] += 1
+
     # send email
     if !ticket.group.email_address_id
-      raise "Can't send email, no email address definde for group id '#{ticket.group.id}'"
+      log_error(record, "Unable to send email, no email address definde for group id '#{ticket.group.id}'")
     elsif !ticket.group.email_address.channel_id
-      raise "Can't send email, no channel definde for email_address id '#{ticket.group.email_address_id}'"
+      log_error(record, "Unable to send email, no channel definde for email_address id '#{ticket.group.email_address_id}'")
     end
 
     channel = ticket.group.email_address.channel
@@ -31,21 +37,36 @@ class Observer::Ticket::Article::CommunicateEmail::BackgroundJob
     end
 
     # get linked channel and send
-    message = channel.deliver(
-      {
-        message_id: record.message_id,
-        in_reply_to: record.in_reply_to,
-        references: ticket.get_references([record.message_id]),
-        from: record.from,
-        to: record.to,
-        cc: record.cc,
-        subject: subject,
-        content_type: record.content_type,
-        body: record.body,
-        attachments: record.attachments
-      },
-      notification
-    )
+    begin
+      message = channel.deliver(
+        {
+          message_id: record.message_id,
+          in_reply_to: record.in_reply_to,
+          references: ticket.get_references([record.message_id]),
+          from: record.from,
+          to: record.to,
+          cc: record.cc,
+          subject: subject,
+          content_type: record.content_type,
+          body: record.body,
+          attachments: record.attachments
+        },
+        notification
+      )
+    rescue => e
+      log_error(record, e.message)
+      return
+    end
+    if !message
+      log_error(record, 'Unable to send email')
+      return
+    end
+
+    # set delivery status
+    record.preferences['delivery_status_message'] = nil
+    record.preferences['delivery_status'] = 'success'
+    record.preferences['delivery_status_date'] = Time.zone.now
+    record.save
 
     # store mail plain
     Store.add(
@@ -83,4 +104,39 @@ class Observer::Ticket::Article::CommunicateEmail::BackgroundJob
       created_by_id: record.created_by_id,
     )
   end
+
+  def log_error(local_record, message)
+    local_record.preferences['delivery_status'] = 'fail'
+    local_record.preferences['delivery_status_message'] = message
+    local_record.preferences['delivery_status_date'] = Time.zone.now
+    local_record.save
+    Rails.logger.error message
+
+    if local_record.preferences['delivery_retry'] > 3
+      Ticket::Article.create(
+        ticket_id: local_record.ticket_id,
+        content_type: 'text/plain',
+        body: "Unable to send email: #{message}",
+        internal: true,
+        sender: Ticket::Article::Sender.find_by(name: 'System'),
+        type: Ticket::Article::Type.find_by(name: 'note'),
+        updated_by_id: 1,
+        created_by_id: 1,
+      )
+      return
+    end
+
+    raise message
+  end
+
+  def max_attempts
+    4
+  end
+
+  def reschedule_at(current_time, attempts)
+    if Rails.env.production?
+      return current_time + attempts * 20.seconds
+    end
+    current_time + 5.seconds
+  end
 end

test/browser/aaa_getting_started_test.rb

@@ -3,7 +3,6 @@ require 'browser_test_helper'
 
 class AaaGettingStartedTest < TestCase
   def test_a_getting_started
-    #return # TODO: temp disable
     if !ENV['MAILBOX_INIT']
       #raise "Need MAILBOX_INIT as ENV variable like export MAILBOX_INIT='unittest01@znuny.com:somepass'"
       puts "NOTICE: Need MAILBOX_INIT as ENV variable like export MAILBOX_INIT='unittest01@znuny.com:somepass'"

test/integration/email_deliver_test.rb

@@ -0,0 +1,270 @@
+# encoding: utf-8
+require 'test_helper'
+
+class EmailDeliverTest < ActiveSupport::TestCase
+  test 'basic check' do
+
+    if !ENV['MAIL_SERVER']
+      raise "Need MAIL_SERVER as ENV variable like export MAIL_SERVER='mx.example.com'"
+    end
+    if !ENV['MAIL_SERVER_ACCOUNT']
+      raise "Need MAIL_SERVER_ACCOUNT as ENV variable like export MAIL_SERVER_ACCOUNT='user:somepass'"
+    end
+    server_login = ENV['MAIL_SERVER_ACCOUNT'].split(':')[0]
+    server_password = ENV['MAIL_SERVER_ACCOUNT'].split(':')[1]
+
+    email_address = EmailAddress.create(
+      realname: 'me Helpdesk',
+      email: "me#{rand(999_999_999)}@example.com",
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+
+    group = Group.create_or_update(
+      name: 'DeliverTest',
+      email_address_id: email_address.id,
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+
+    channel = Channel.create(
+      area: 'Email::Account',
+      group_id: group.id,
+      options: {
+        inbound: {
+          adapter: 'imap',
+          options: {
+            host: 'mx1.example.com',
+            user: 'example',
+            password: 'some_pw',
+            ssl: true,
+          }
+        },
+        outbound: {
+          adapter: 'sendmail'
+        }
+      },
+      active: true,
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+
+    email_address.channel_id = channel.id
+    email_address.save
+
+    ticket1 = Ticket.create(
+      title: 'some delivery test',
+      group: group,
+      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')
+
+    article1 = Ticket::Article.create(
+      ticket_id: ticket1.id,
+      to: 'some_recipient@example_not_existing_what_ever.com',
+      subject: 'some subject',
+      message_id: 'some@id',
+      body: 'some message delivery test',
+      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(nil, article1.preferences['delivery_retry'])
+    assert_equal(nil, article1.preferences['delivery_status'])
+    assert_equal(nil, article1.preferences['delivery_status_date'])
+    assert_equal(nil, article1.preferences['delivery_status_message'])
+
+    result = Observer::Ticket::Article::CommunicateEmail::BackgroundJob.new(article1.id)
+    assert(result.perform)
+
+    article1_lookup = Ticket::Article.find(article1.id)
+    assert_equal(1, article1_lookup.preferences['delivery_retry'])
+    assert_equal('success', article1_lookup.preferences['delivery_status'])
+    assert(article1_lookup.preferences['delivery_status_date'])
+    assert_equal(nil, article1_lookup.preferences['delivery_status_message'])
+
+    # send with invalid smtp settings
+    channel.update_attributes(
+      options: {
+        inbound: {
+          adapter: 'imap',
+          options: {
+            host: 'mx1.example.com',
+            user: 'example',
+            password: 'some_pw',
+            ssl: true,
+          }
+        },
+        outbound: {
+          adapter: 'smtp',
+          options: {
+            host: 'mx1.example.com',
+            port: 25,
+            start_tls: true,
+            user: 'not_existing',
+            password: 'not_existing',
+          },
+        },
+      },
+    )
+    assert_raises(RuntimeError) {
+      result = Observer::Ticket::Article::CommunicateEmail::BackgroundJob.new(article1.id)
+      assert_not(result.perform)
+    }
+    article1_lookup = Ticket::Article.find(article1.id)
+    assert_equal(2, article1_lookup.preferences['delivery_retry'])
+    assert_equal('fail', article1_lookup.preferences['delivery_status'])
+    assert(article1_lookup.preferences['delivery_status_date'])
+    assert(article1_lookup.preferences['delivery_status_message'])
+
+    # send with correct smtp settings
+    channel.update_attributes(
+      options: {
+        inbound: {
+          adapter: 'imap',
+          options: {
+            host: 'mx1.example.com',
+            user: 'example',
+            password: 'some_pw',
+            ssl: true,
+          }
+        },
+        outbound: {
+          adapter: 'smtp',
+          options: {
+            host: ENV['MAIL_SERVER'],
+            port: 25,
+            start_tls: true,
+            user: server_login,
+            password: server_password,
+          },
+        },
+      },
+    )
+
+    result = Observer::Ticket::Article::CommunicateEmail::BackgroundJob.new(article1.id)
+    assert(result.perform)
+    article1_lookup = Ticket::Article.find(article1.id)
+    assert_equal(3, article1_lookup.preferences['delivery_retry'])
+    assert_equal('success', article1_lookup.preferences['delivery_status'])
+    assert(article1_lookup.preferences['delivery_status_date'])
+    assert_nil(article1_lookup.preferences['delivery_status_message'])
+
+    # check retry jobs
+    # remove background jobs
+    Delayed::Job.destroy_all
+
+    # send with invalid smtp settings
+    channel.update_attributes(
+      options: {
+        inbound: {
+          adapter: 'imap',
+          options: {
+            host: 'mx1.example.com',
+            user: 'example',
+            password: 'some_pw',
+            ssl: true,
+          }
+        },
+        outbound: {
+          adapter: 'smtp',
+          options: {
+            host: 'mx1.example.com',
+            port: 25,
+            start_tls: true,
+            user: 'not_existing',
+            password: 'not_existing',
+          },
+        },
+      },
+    )
+
+    # remove background jobs
+    Delayed::Job.destroy_all
+
+    article2 = Ticket::Article.create(
+      ticket_id: ticket1.id,
+      to: 'some_recipient@example_not_existing_what_ever.com',
+      subject: 'some subject2',
+      message_id: 'some@id',
+      body: 'some message delivery test2',
+      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_raises(RuntimeError) {
+      Scheduler.worker(true)
+    }
+
+    article2_lookup = Ticket::Article.find(article2.id)
+    assert_equal(2, ticket1.articles.count)
+    assert_equal(1, article2_lookup.preferences['delivery_retry'])
+    assert_equal('fail', article2_lookup.preferences['delivery_status'])
+    assert(article2_lookup.preferences['delivery_status_date'])
+    assert(article2_lookup.preferences['delivery_status_message'])
+
+    Scheduler.worker(true)
+
+    article2_lookup = Ticket::Article.find(article2.id)
+    assert_equal(2, ticket1.articles.count)
+    assert_equal(1, article2_lookup.preferences['delivery_retry'])
+    assert_equal('fail', article2_lookup.preferences['delivery_status'])
+    assert(article2_lookup.preferences['delivery_status_date'])
+    assert(article2_lookup.preferences['delivery_status_message'])
+
+    sleep 6
+    assert_raises(RuntimeError) {
+      Scheduler.worker(true)
+    }
+
+    article2_lookup = Ticket::Article.find(article2.id)
+    assert_equal(2, ticket1.articles.count)
+    assert_equal(2, article2_lookup.preferences['delivery_retry'])
+    assert_equal('fail', article2_lookup.preferences['delivery_status'])
+    assert(article2_lookup.preferences['delivery_status_date'])
+    assert(article2_lookup.preferences['delivery_status_message'])
+
+    Scheduler.worker(true)
+    article2_lookup = Ticket::Article.find(article2.id)
+    assert_equal(2, ticket1.articles.count)
+    assert_equal(2, article2_lookup.preferences['delivery_retry'])
+    assert_equal('fail', article2_lookup.preferences['delivery_status'])
+    assert(article2_lookup.preferences['delivery_status_date'])
+    assert(article2_lookup.preferences['delivery_status_message'])
+
+    sleep 11
+    assert_raises(RuntimeError) {
+      Scheduler.worker(true)
+    }
+
+    article2_lookup = Ticket::Article.find(article2.id)
+    assert_equal(2, ticket1.articles.count)
+    assert_equal(3, article2_lookup.preferences['delivery_retry'])
+    assert_equal('fail', article2_lookup.preferences['delivery_status'])
+    assert(article2_lookup.preferences['delivery_status_date'])
+    assert(article2_lookup.preferences['delivery_status_message'])
+
+    sleep 16
+    Scheduler.worker(true)
+
+    article2_lookup = Ticket::Article.find(article2.id)
+    assert_equal(3, ticket1.articles.count)
+    assert_equal('System', ticket1.articles.last.sender.name)
+    assert_equal(4, article2_lookup.preferences['delivery_retry'])
+    assert_equal('fail', article2_lookup.preferences['delivery_status'])
+    assert(article2_lookup.preferences['delivery_status_date'])
+    assert(article2_lookup.preferences['delivery_status_message'])
+
+  end
+
+end