Refactoring: Migrate ticket_article_communicate_test to RSpec

spec/jobs/ticket_article_communicate_email_job_spec.rb

@@ -0,0 +1,28 @@
+require 'rails_helper'
+
+RSpec.describe TicketArticleCommunicateEmailJob, type: :job do
+  describe '#perform' do
+    context 'for an email article' do
+      let(:article) { create(:ticket_article, type_name: 'email') }
+      let(:recipient_list) { [article.to, article.cc].reject(&:blank?).join(',') }
+
+      before { allow(Rails.logger).to receive(:info) }
+
+      # What we _really_ want is to expect an email to be sent.
+      # So why are we testing log messages instead?
+      #
+      # Because so far, our attempts to test email dispatch have either
+      # a) been closely tied to implementation, with lots of ugly mock objects; or
+      # b) had to test faraway classes like Channel::Driver::Imap.
+      #
+      # In other words, this test is NOT set in stone, and very open to improvement.
+      it 'records outgoing email dispatch to Rails log' do
+        described_class.perform_now(article.id)
+
+        expect(Rails.logger)
+          .to have_received(:info)
+          .with("Send email to: '#{recipient_list}' (from #{article.from})")
+      end
+    end
+  end
+end

spec/lib/application_handle_info_spec.rb

@@ -8,7 +8,21 @@ RSpec.describe ApplicationHandleInfo do
     end
 
     context 'for a given starting ApplicationHandleInfo' do
-      before { described_class.current = 'foo' }
+      # This `around` block is identical to ApplicationHandleInfo.use.
+      #
+      # Q: So why don't we just use it here to DRY things up?
+      # A: Because that's the method we're trying to test, dummy!
+      #
+      # Q: Why can't we do `before { ApplicationHandleInfo.current = 'foo' }` instead?
+      # A: Because that would change `ApplicationHandleInfo.current` for all subsequent specs.
+      #    (RSpec uses database transactions to keep test environments clean,
+      #    but `ApplicationHandleInfo.current` lives outside of the database.)
+      around do |example|
+        original = described_class.current
+        described_class.current = 'foo'
+        example.run
+        described_class.current = original
+      end
 
       it 'runs the block using the given ApplicationHandleInfo' do
         described_class.use('bar') do
@@ -26,7 +40,7 @@ RSpec.describe ApplicationHandleInfo do
         it 'does not rescue the error, and still resets ApplicationHandleInfo' do
           expect { described_class.use('bar') { raise } }
             .to raise_error(StandardError)
-            .and not_change { described_class.current }
+            .and not_change(described_class, :current)
         end
       end
     end

spec/models/channel/email_parser_spec.rb

@@ -978,5 +978,23 @@ RSpec.describe Channel::EmailParser, type: :model do
         end
       end
     end
+
+    describe 'suppressing normal Ticket::Article callbacks' do
+      context 'from sender: "Agent"' do
+        let(:agent) { create(:agent_user) }
+
+        it 'does not dispatch an email on article creation' do
+          expect(TicketArticleCommunicateEmailJob).not_to receive(:perform_later)
+
+          described_class.new.process({}, <<~RAW.chomp)
+            From: #{agent.email}
+            To: customer@example.com
+            Subject: some subject
+
+            Some Text
+          RAW
+        end
+      end
+    end
   end
 end

spec/models/ticket/article_spec.rb

@@ -403,6 +403,64 @@ RSpec.describe Ticket::Article, type: :model do
         end
       end
     end
+
+    describe 'Sending of outgoing emails', performs_jobs: true do
+      subject(:article) { create(:ticket_article, type_name: type, sender_name: sender) }
+
+      shared_examples 'sends email' do
+        it 'dispatches an email on creation (via TicketArticleCommunicateEmailJob)' do
+          expect { article }
+            .to have_enqueued_job(TicketArticleCommunicateEmailJob)
+        end
+      end
+
+      shared_examples 'does not send email' do
+        it 'does not dispatch an email' do
+          expect { article }
+            .not_to have_enqueued_job(TicketArticleCommunicateEmailJob)
+        end
+      end
+
+      context 'with "application_server" application handle', application_handle: 'application_server' do
+        context 'for type: "email"' do
+          let(:type) { 'email' }
+
+          context 'from sender: "Agent"' do
+            let(:sender) { 'Agent' }
+
+            include_examples 'sends email'
+          end
+
+          context 'from sender: "Customer"' do
+            let(:sender) { 'Customer' }
+
+            include_examples 'does not send email'
+          end
+        end
+
+        context 'for any other type' do
+          let(:type) { 'sms' }
+
+          context 'from any sender' do
+            let(:sender) { 'Agent' }
+
+            include_examples 'does not send email'
+          end
+        end
+      end
+
+      context 'with "*.postmaster" application handle', application_handle: 'scheduler.postmaster' do
+        context 'for any type' do
+          let(:type) { 'email' }
+
+          context 'from any sender' do
+            let(:sender) { 'Agent' }
+
+            include_examples 'does not send email'
+          end
+        end
+      end
+    end
   end
 
   describe 'clone attachments' do

test/test_helper.rb

@@ -74,23 +74,4 @@ class ActiveSupport::TestCase
     count
   end
 
-  def email_count(recipient)
-
-    # read config file and count & recipients
-    file = Rails.root.join('log', "#{Rails.env}.log")
-    lines = []
-    IO.foreach(file) do |line|
-      lines.push line
-    end
-    count = 0
-    lines.reverse_each do |line|
-      break if line.match?(/\+\+\+\+NEW\+\+\+\+TEST\+\+\+\+/)
-      next if line !~ /Send email to:/
-      next if line !~ /to:\s'#{recipient}'/
-
-      count += 1
-    end
-    count
-  end
-
 end

test/unit/ticket_article_communicate_test.rb

@@ -1,158 +0,0 @@
-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
-X-Loop: yes
-
-Some Text"
-
-    _ticket_p, article_p, _user_p, _mail = Channel::EmailParser.new.process({ trusted: true }, email_raw_string)
-    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