Merge branch '8226-add-service-desk-tests' into 'master'

Refactor Service Desk specs

Closes #8226

See merge request gitlab-org/gitlab-ee!9846

ee/spec/lib/gitlab/email/handler/ee/service_desk_handler_spec.rb

@@ -4,6 +4,7 @@ require 'spec_helper'
 
 describe Gitlab::Email::Handler::EE::ServiceDeskHandler do
   include_context :email_shared_context
+
   before do
     stub_incoming_email_setting(enabled: true, address: "incoming+%{key}@appmail.adventuretime.ooo")
     stub_config_setting(host: 'localhost')
@@ -16,59 +17,60 @@ describe Gitlab::Email::Handler::EE::ServiceDeskHandler do
     let(:project) { create(:project, :public, namespace: namespace, path: 'test', service_desk_enabled: true) }
 
     before do
-      allow(Notify).to receive(:service_desk_thank_you_email)
-        .with(kind_of(Integer)).and_return(double(deliver_later!: true))
-
       allow(::EE::Gitlab::ServiceDesk).to receive(:enabled?).and_return(true)
       allow(::EE::Gitlab::ServiceDesk).to receive(:enabled?).with(project: project).and_return(true)
     end
 
-    context 'when everything is fine' do
-      shared_examples 'a new issue request' do
-        it 'sends thank you the email and creates issue' do
-          setup_attachment
+    shared_examples 'a new issue request' do
+      before do
+        setup_attachment
+      end
 
-          expect(Notify).to receive(:service_desk_thank_you_email).with(kind_of(Integer))
+      it 'creates a new issue' do
+        expect { receiver.execute }.to change { Issue.count }.by(1)
 
-          expect { receiver.execute }.to change { Issue.count }.by(1)
+        new_issue = Issue.last
 
-          new_issue = Issue.last
+        expect(new_issue.author).to eql(User.support_bot)
+        expect(new_issue.confidential?).to be true
+        expect(new_issue.all_references.all).to be_empty
+        expect(new_issue.title).to eq("Service Desk (from jake@adventuretime.ooo): The message subject! @all")
+        expect(new_issue.description).to eq("Service desk stuff!\n\n```\na = b\n```\n\n![image](uploads/image.png)")
+      end
 
-          expect(new_issue.author).to eql(User.support_bot)
-          expect(new_issue.confidential?).to be true
-          expect(new_issue.all_references.all).to be_empty
-          expect(new_issue.title).to eq("Service Desk (from jake@adventuretime.ooo): The message subject! @all")
-          expect(new_issue.description).to eq("Service desk stuff!\n\n```\na = b\n```\n\n![image](uploads/image.png)")
-        end
+      it 'sends thank you email' do
+        expect { receiver.execute }.to have_enqueued_job.on_queue('mailers')
       end
+    end
 
+    context 'when everything is fine' do
       it_behaves_like 'a new issue request'
 
-      context "creates a new issue with legacy email address" do
+      context 'with legacy incoming email address' do
         let(:email_raw) { fixture_file('emails/service_desk_legacy.eml', dir: 'ee') }
 
         it_behaves_like 'a new issue request'
       end
     end
 
-    context 'when email key' do
+    describe '#can_handle?' do
       let(:mail) { Mail::Message.new(email_raw) }
 
-      it "matches the new format" do
+      it 'handles the new email key format' do
         handler = described_class.new(mail, "h5bp-html5-boilerplate-#{project.project_id}-issue-")
 
         expect(handler.instance_variable_get(:@project_id).to_i).to eq project.project_id
         expect(handler.can_handle?).to be_truthy
       end
 
-      it "matches the legacy format" do
+      it 'handles the legacy email key format' do
         handler = described_class.new(mail, "h5bp/html5-boilerplate")
 
         expect(handler.instance_variable_get(:@project_path)).to eq 'h5bp/html5-boilerplate'
         expect(handler.can_handle?).to be_truthy
       end
 
-      it "doesn't match either format" do
+      it "doesn't handle invalid email key" do
         handler = described_class.new(mail, "h5bp-html5-boilerplate-invalid")
 
         expect(handler.can_handle?).to be_falsey
@@ -81,11 +83,13 @@ describe Gitlab::Email::Handler::EE::ServiceDeskHandler do
           .and_return(nil)
       end
 
-      it "does not send thank you email but create an issue" do
-        expect(Notify).not_to receive(:service_desk_thank_you_email)
-
+      it "creates a new issue" do
         expect { receiver.execute }.to change { Issue.count }.by(1)
       end
+
+      it 'does not send thank you email' do
+        expect { receiver.execute }.not_to have_enqueued_job.on_queue('mailers')
+      end
     end
 
     context 'when there is a sender address and a from address' do
@@ -94,17 +98,11 @@ describe Gitlab::Email::Handler::EE::ServiceDeskHandler do
       it 'prefers the from address' do
         setup_attachment
 
-        expect(Notify).to receive(:service_desk_thank_you_email).with(kind_of(Integer))
-
         expect { receiver.execute }.to change { Issue.count }.by(1)
 
         new_issue = Issue.last
 
-        expect(new_issue.author).to eql(User.support_bot)
-        expect(new_issue.confidential?).to be true
-        expect(new_issue.all_references.all).to be_empty
-        expect(new_issue.title).to eq("Service Desk (from finn@adventuretime.ooo): The message subject! @all")
-        expect(new_issue.description).to eq("Service desk stuff!\n\n```\na = b\n```\n\n![image](uploads/image.png)")
+        expect(new_issue.service_desk_reply_to).to eq('finn@adventuretime.ooo')
       end
     end
 
@@ -113,31 +111,19 @@ describe Gitlab::Email::Handler::EE::ServiceDeskHandler do
         allow(::EE::Gitlab::ServiceDesk).to receive(:enabled?).and_return(false)
       end
 
-      it 'does not create an issue or send email' do
-        expect(Notify).not_to receive(:service_desk_thank_you_email)
-
+      it 'does not create an issue' do
         expect { receiver.execute rescue nil }.not_to change { Issue.count }
       end
+
+      it 'does not send thank you email' do
+        expect { receiver.execute rescue nil }.not_to have_enqueued_job.on_queue('mailers')
+      end
     end
 
     context 'when the email is forwarded through an alias' do
       let(:email_raw) { email_fixture('emails/service_desk_forwarded.eml', dir: 'ee') }
 
-      it 'sends thank you the email and creates issue' do
-        setup_attachment
-
-        expect(Notify).to receive(:service_desk_thank_you_email).with(kind_of(Integer))
-
-        expect { receiver.execute }.to change { Issue.count }.by(1)
-
-        new_issue = Issue.last
-
-        expect(new_issue.author).to eql(User.support_bot)
-        expect(new_issue.confidential?).to be true
-        expect(new_issue.all_references.all).to be_empty
-        expect(new_issue.title).to eq("Service Desk (from jake@adventuretime.ooo): The message subject! @all")
-        expect(new_issue.description).to eq("Service desk stuff!\n\n```\na = b\n```\n\n![image](uploads/image.png)")
-      end
+      it_behaves_like 'a new issue request'
     end
   end
 

ee/spec/mailers/emails/service_desk_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-describe Emails::ServiceDesk do
-  it 'adds email methods to Notify' do
-    subject.instance_methods.each do |email_method|
-      expect(Notify).to be_respond_to(email_method)
-    end
-  end
-end

ee/spec/mailers/notify_spec.rb

@@ -37,11 +37,19 @@ describe Notify do
 
   context 'for a project' do
     context 'for service desk issues' do
+      before do
+        issue.update!(service_desk_reply_to: 'service.desk@example.com')
+      end
+
       describe 'thank you email' do
         subject { described_class.service_desk_thank_you_email(issue.id) }
 
         it_behaves_like 'an unsubscribeable thread'
 
+        it 'has the correct recipient' do
+          is_expected.to deliver_to('service.desk@example.com')
+        end
+
         it 'has the correct subject and body' do
           aggregate_failures do
             is_expected.to have_referable_subject(issue, include_project: false, reply: true)
@@ -57,6 +65,10 @@ describe Notify do
 
         it_behaves_like 'an unsubscribeable thread'
 
+        it 'has the correct recipient' do
+          is_expected.to deliver_to('service.desk@example.com')
+        end
+
         it 'has the correct subject and body' do
           aggregate_failures do
             is_expected.to have_referable_subject(issue, include_project: false, reply: true)