Add latest changes from gitlab-org/gitlab@master

1 files changed, 153 insertions, 102 deletions

diff --git a/spec/mailers/emails/service_desk_spec.rb b/spec/mailers/emails/service_desk_spec.rb
index 521cbe469d9..c50d5ce2571 100644
--- a/spec/mailers/emails/service_desk_spec.rb
+++ b/spec/mailers/emails/service_desk_spec.rb
@@ -26,7 +26,25 @@ RSpec.describe Emails::ServiceDesk, feature_category: :service_desk do
     issue.issue_email_participants.create!(email: email)
   end
 
-  shared_examples 'handle template content' do |template_key, attachments_count|
+  shared_examples 'a service desk notification email' do |attachments_count|
+    it 'builds the email correctly' do
+      aggregate_failures do
+        is_expected.to have_referable_subject(issue, include_project: false, reply: reply_in_subject)
+
+        expect(subject.attachments.count).to eq(attachments_count.to_i)
+
+        expect(subject.content_type).to include('multipart/alternative')
+
+        expect(subject.parts[0].body.to_s).to include(expected_text)
+        expect(subject.parts[0].content_type).to include('text/plain')
+
+        expect(subject.parts[1].body.to_s).to include(expected_html)
+        expect(subject.parts[1].content_type).to include('text/html')
+      end
+    end
+  end
+
+  shared_examples 'a service desk notification email with template content' do |template_key, attachments_count|
     before do
       expect(Gitlab::Template::ServiceDeskTemplate).to receive(:find)
         .with(template_key, issue.project)
@@ -36,9 +54,18 @@ RSpec.describe Emails::ServiceDesk, feature_category: :service_desk do
     it 'builds the email correctly' do
       aggregate_failures do
         is_expected.to have_referable_subject(issue, include_project: false, reply: reply_in_subject)
-        is_expected.to have_body_text(expected_body)
+        is_expected.to have_body_text(expected_template_html)
+
         expect(subject.attachments.count).to eq(attachments_count.to_i)
-        expect(subject.content_type).to include(attachments_count.to_i > 0 ? 'multipart/mixed' : 'text/html')
+
+        if attachments_count.to_i > 0
+          # Envelope for emails with attachments is always multipart/mixed
+          expect(subject.content_type).to include('multipart/mixed')
+          # Template content only renders a html body, so ensure its content type is set accordingly
+          expect(subject.parts.first.content_type).to include('text/html')
+        else
+          expect(subject.content_type).to include('text/html')
+        end
       end
     end
   end
@@ -63,7 +90,8 @@ RSpec.describe Emails::ServiceDesk, feature_category: :service_desk do
       let(:project) { create(:project, :custom_repo, files: { ".gitlab/service_desk_templates/another_file.md" => template_content }) }
 
       it 'uses the default template' do
-        is_expected.to have_body_text(default_text)
+        expect(subject.text_part.to_s).to include(expected_text)
+        expect(subject.html_part.to_s).to include(expected_html)
       end
     end
 
@@ -71,7 +99,8 @@ RSpec.describe Emails::ServiceDesk, feature_category: :service_desk do
       let(:project) { create(:project, :custom_repo, files: { "other_directory/another_file.md" => template_content }) }
 
       it 'uses the default template' do
-        is_expected.to have_body_text(default_text)
+        expect(subject.text_part.to_s).to include(expected_text)
+        expect(subject.html_part.to_s).to include(expected_html)
       end
     end
 
@@ -79,7 +108,8 @@ RSpec.describe Emails::ServiceDesk, feature_category: :service_desk do
       let(:project) { create(:project) }
 
       it 'uses the default template' do
-        is_expected.to have_body_text(default_text)
+        expect(subject.text_part.to_s).to include(expected_text)
+        expect(subject.html_part.to_s).to include(expected_html)
       end
     end
   end
@@ -117,20 +147,23 @@ RSpec.describe Emails::ServiceDesk, feature_category: :service_desk do
 
   describe '.service_desk_thank_you_email' do
     let_it_be(:reply_in_subject) { true }
-    let_it_be(:default_text) do
+    let_it_be(:expected_text) do
       "Thank you for your support request! We are tracking your request as ticket #{issue.to_reference}, and will respond as soon as we can."
     end
 
+    let_it_be(:expected_html) { expected_text }
+
     subject { ServiceEmailClass.service_desk_thank_you_email(issue.id) }
 
+    it_behaves_like 'a service desk notification email'
     it_behaves_like 'read template from repository', 'thank_you'
 
     context 'handling template markdown' do
       context 'with a simple text' do
         let(:template_content) { 'thank you, **your new issue** has been created.' }
-        let(:expected_body) { 'thank you, <strong>your new issue</strong> has been created.' }
+        let(:expected_template_html) { 'thank you, <strong>your new issue</strong> has been created.' }
 
-        it_behaves_like 'handle template content', 'thank_you'
+        it_behaves_like 'a service desk notification email with template content', 'thank_you'
       end
 
       context 'with an issue id, issue path and unsubscribe url placeholders' do
@@ -139,12 +172,12 @@ RSpec.describe Emails::ServiceDesk, feature_category: :service_desk do
             '[Unsubscribe](%{UNSUBSCRIBE_URL})'
         end
 
-        let(:expected_body) do
+        let(:expected_template_html) do
           "<p dir=\"auto\">thank you, <strong>your new issue:</strong> ##{issue.iid}, path: #{project.full_path}##{issue.iid}" \
             "<a href=\"#{expected_unsubscribe_url}\">Unsubscribe</a></p>"
         end
 
-        it_behaves_like 'handle template content', 'thank_you'
+        it_behaves_like 'a service desk notification email with template content', 'thank_you'
       end
 
       context 'with header and footer placeholders' do
@@ -160,42 +193,44 @@ RSpec.describe Emails::ServiceDesk, feature_category: :service_desk do
 
       context 'with an issue id placeholder with whitespace' do
         let(:template_content) { 'thank you, **your new issue:** %{  ISSUE_ID}' }
-        let(:expected_body) { "thank you, <strong>your new issue:</strong> ##{issue.iid}" }
+        let(:expected_template_html) { "thank you, <strong>your new issue:</strong> ##{issue.iid}" }
 
-        it_behaves_like 'handle template content', 'thank_you'
+        it_behaves_like 'a service desk notification email with template content', 'thank_you'
       end
 
       context 'with unexpected placeholder' do
         let(:template_content) { 'thank you, **your new issue:** %{this is issue}' }
-        let(:expected_body) { "thank you, <strong>your new issue:</strong> %{this is issue}" }
+        let(:expected_template_html) { "thank you, <strong>your new issue:</strong> %{this is issue}" }
 
-        it_behaves_like 'handle template content', 'thank_you'
+        it_behaves_like 'a service desk notification email with template content', 'thank_you'
       end
 
       context 'when issue description placeholder is used' do
         let(:template_content) { 'thank you, your new issue has been created. %{ISSUE_DESCRIPTION}' }
-        let(:expected_body) { "<p dir=\"auto\">thank you, your new issue has been created. </p>#{issue.description_html}" }
+        let(:expected_template_html) { "<p dir=\"auto\">thank you, your new issue has been created. </p>#{issue.description_html}" }
 
-        it_behaves_like 'handle template content', 'thank_you'
+        it_behaves_like 'a service desk notification email with template content', 'thank_you'
       end
     end
   end
 
   describe '.service_desk_new_note_email' do
     let_it_be(:reply_in_subject) { false }
-    let_it_be(:note) { create(:note_on_issue, noteable: issue, project: project) }
-    let_it_be(:default_text) { note.note }
+    let_it_be(:expected_text) { 'My **note**' }
+    let_it_be(:expected_html) { 'My <strong>note</strong>' }
+    let_it_be(:note) { create(:note_on_issue, noteable: issue, project: project, note: expected_text) }
 
     subject { ServiceEmailClass.service_desk_new_note_email(issue.id, note.id, email) }
 
+    it_behaves_like 'a service desk notification email'
     it_behaves_like 'read template from repository', 'new_note'
 
-    context 'handling template markdown' do
+    context 'with template' do
       context 'with a simple text' do
         let(:template_content) { 'thank you, **new note on issue** has been created.' }
-        let(:expected_body) { 'thank you, <strong>new note on issue</strong> has been created.' }
+        let(:expected_template_html) { 'thank you, <strong>new note on issue</strong> has been created.' }
 
-        it_behaves_like 'handle template content', 'new_note'
+        it_behaves_like 'a service desk notification email with template content', 'new_note'
       end
 
       context 'with an issue id, issue path, note and unsubscribe url placeholders' do
@@ -204,12 +239,12 @@ RSpec.describe Emails::ServiceDesk, feature_category: :service_desk do
             '[Unsubscribe](%{UNSUBSCRIBE_URL})'
         end
 
-        let(:expected_body) do
-          "<p dir=\"auto\">thank you, <strong>new note on issue:</strong> ##{issue.iid}, path: #{project.full_path}##{issue.iid}: #{note.note}" \
+        let(:expected_template_html) do
+          "<p dir=\"auto\">thank you, <strong>new note on issue:</strong> ##{issue.iid}, path: #{project.full_path}##{issue.iid}: #{expected_html}" \
             "<a href=\"#{expected_unsubscribe_url}\">Unsubscribe</a></p>"
         end
 
-        it_behaves_like 'handle template content', 'new_note'
+        it_behaves_like 'a service desk notification email with template content', 'new_note'
       end
 
       context 'with header and footer placeholders' do
@@ -225,124 +260,140 @@ RSpec.describe Emails::ServiceDesk, feature_category: :service_desk do
 
       context 'with an issue id placeholder with whitespace' do
         let(:template_content) { 'thank you, **new note on issue:** %{  ISSUE_ID}: %{ NOTE_TEXT  }' }
-        let(:expected_body) { "thank you, <strong>new note on issue:</strong> ##{issue.iid}: #{note.note}" }
+        let(:expected_template_html) { "thank you, <strong>new note on issue:</strong> ##{issue.iid}: #{expected_html}" }
 
-        it_behaves_like 'handle template content', 'new_note'
+        it_behaves_like 'a service desk notification email with template content', 'new_note'
       end
 
       context 'with unexpected placeholder' do
         let(:template_content) { 'thank you, **new note on issue:** %{this is issue}' }
-        let(:expected_body) { "thank you, <strong>new note on issue:</strong> %{this is issue}" }
+        let(:expected_template_html) { "thank you, <strong>new note on issue:</strong> %{this is issue}" }
 
-        it_behaves_like 'handle template content', 'new_note'
+        it_behaves_like 'a service desk notification email with template content', 'new_note'
       end
 
-      context 'with upload link in the note' do
-        let_it_be(:secret) { 'e90decf88d8f96fe9e1389afc2e4a91f' }
-        let_it_be(:filename) { 'test.jpg' }
-        let_it_be(:path) { "#{secret}/#{filename}" }
-        let_it_be(:upload_path) { "/uploads/#{path}" }
-        let_it_be(:template_content) { 'some text %{ NOTE_TEXT  }' }
-        let_it_be(:note) { create(:note_on_issue, noteable: issue, project: project, note: "a new comment with [#{filename}](#{upload_path})") }
-        let!(:upload) { create(:upload, :issuable_upload, :with_file, model: note.project, path: path, secret: secret) }
+      context 'with all-user reference in a an external author comment' do
+        let_it_be(:note) { create(:note_on_issue, noteable: issue, project: project, note: "Hey @all, just a ping", author: User.support_bot) }
 
-        context 'when total uploads size is more than 10mb' do
-          before do
-            allow_next_instance_of(FileUploader) do |instance|
-              allow(instance).to receive(:size).and_return(10.1.megabytes)
-            end
-          end
+        let(:template_content) { 'some text %{ NOTE_TEXT  }' }
+        let(:expected_template_html) { 'Hey , just a ping' }
 
-          let_it_be(:expected_body) { %Q(some text a new comment with <a href="#{project.web_url}#{upload_path}" data-canonical-src="#{upload_path}" data-link="true" class="gfm">#{filename}</a>) }
+        it_behaves_like 'a service desk notification email with template content', 'new_note'
+      end
+    end
 
-          it_behaves_like 'handle template content', 'new_note'
+    # handle email without and with template in this context to reduce code duplication
+    context 'with upload link in the note' do
+      let_it_be(:secret) { 'e90decf88d8f96fe9e1389afc2e4a91f' }
+      let_it_be(:filename) { 'test.jpg' }
+      let_it_be(:path) { "#{secret}/#{filename}" }
+      let_it_be(:upload_path) { "/uploads/#{path}" }
+      let_it_be(:template_content) { 'some text %{ NOTE_TEXT  }' }
+      let_it_be(:expected_text) { "a new comment with [#{filename}](#{upload_path})" }
+      let_it_be(:expected_html) { "a new comment with <strong>#{filename}</strong>" }
+      let_it_be(:note) { create(:note_on_issue, noteable: issue, project: project, note: expected_text) }
+      let!(:upload) { create(:upload, :issuable_upload, :with_file, model: note.project, path: path, secret: secret) }
+
+      context 'when total uploads size is more than 10mb' do
+        before do
+          allow_next_instance_of(FileUploader) do |instance|
+            allow(instance).to receive(:size).and_return(10.1.megabytes)
+          end
         end
 
-        context 'when total uploads size is less or equal 10mb' do
-          context 'when it has only one upload' do
-            before do
-              allow_next_instance_of(FileUploader) do |instance|
-                allow(instance).to receive(:size).and_return(10.megabytes)
-              end
-            end
+        let_it_be(:expected_html) { %Q(a new comment with <a href="#{project.web_url}#{upload_path}" data-canonical-src="#{upload_path}" data-link="true" class="gfm">#{filename}</a>) }
+        let_it_be(:expected_template_html) { %Q(some text #{expected_html}) }
 
-            context 'when upload name is not changed in markdown' do
-              let_it_be(:expected_body) { %Q(some text a new comment with <strong>#{filename}</strong>) }
+        it_behaves_like 'a service desk notification email'
+        it_behaves_like 'a service desk notification email with template content', 'new_note'
+      end
 
-              it_behaves_like 'handle template content', 'new_note', 1
+      context 'when total uploads size is less or equal 10mb' do
+        context 'when it has only one upload' do
+          before do
+            allow_next_instance_of(FileUploader) do |instance|
+              allow(instance).to receive(:size).and_return(10.megabytes)
+              allow(instance).to receive(:read).and_return('')
             end
+          end
 
-            context 'when upload name is changed in markdown' do
-              let_it_be(:upload_name_in_markdown) { 'Custom name' }
-              let_it_be(:note) { create(:note_on_issue, noteable: issue, project: project, note: "a new comment with [#{upload_name_in_markdown}](#{upload_path})") }
-              let_it_be(:expected_body) { %Q(some text a new comment with <strong>#{upload_name_in_markdown} (#{filename})</strong>) }
+          context 'when upload name is not changed in markdown' do
+            let_it_be(:expected_template_html) { %Q(some text a new comment with <strong>#{filename}</strong>) }
 
-              it_behaves_like 'handle template content', 'new_note', 1
-            end
+            it_behaves_like 'a service desk notification email', 1
+            it_behaves_like 'a service desk notification email with template content', 'new_note', 1
           end
 
-          context 'when it has more than one upload' do
+          context 'when upload name is changed in markdown' do
+            let_it_be(:upload_name_in_markdown) { 'Custom name' }
+            let_it_be(:note) { create(:note_on_issue, noteable: issue, project: project, note: "a new comment with [#{upload_name_in_markdown}](#{upload_path})") }
+            let_it_be(:expected_text) { %Q(a new comment with [#{upload_name_in_markdown}](#{upload_path})) }
+            let_it_be(:expected_html) { %Q(a new comment with <strong>#{upload_name_in_markdown} (#{filename})</strong>) }
+            let_it_be(:expected_template_html) { %Q(some text #{expected_html}) }
+
+            it_behaves_like 'a service desk notification email', 1
+            it_behaves_like 'a service desk notification email with template content', 'new_note', 1
+          end
+        end
+
+        context 'when it has more than one upload' do
+          let_it_be(:secret_1) { '17817c73e368777e6f743392e334fb8a' }
+          let_it_be(:filename_1) { 'test1.jpg' }
+          let_it_be(:path_1) { "#{secret_1}/#{filename_1}" }
+          let_it_be(:upload_path_1) { "/uploads/#{path_1}" }
+          let_it_be(:note) { create(:note_on_issue, noteable: issue, project: project, note: "a new comment with [#{filename}](#{upload_path}) [#{filename_1}](#{upload_path_1})") }
+
+          context 'when all uploads processed correct' do
             before do
               allow_next_instance_of(FileUploader) do |instance|
                 allow(instance).to receive(:size).and_return(5.megabytes)
+                allow(instance).to receive(:read).and_return('')
               end
             end
 
-            let_it_be(:secret_1) { '17817c73e368777e6f743392e334fb8a' }
-            let_it_be(:filename_1) { 'test1.jpg' }
-            let_it_be(:path_1) { "#{secret_1}/#{filename_1}" }
-            let_it_be(:upload_path_1) { "/uploads/#{path_1}" }
-            let_it_be(:note) { create(:note_on_issue, noteable: issue, project: project, note: "a new comment with [#{filename}](#{upload_path}) [#{filename_1}](#{upload_path_1})") }
+            let_it_be(:upload_1) { create(:upload, :issuable_upload, :with_file, model: note.project, path: path_1, secret: secret_1) }
 
-            context 'when all uploads processed correct' do
-              let_it_be(:upload_1) { create(:upload, :issuable_upload, :with_file, model: note.project, path: path_1, secret: secret_1) }
-              let_it_be(:expected_body) { %Q(some text a new comment with <strong>#{filename}</strong> <strong>#{filename_1}</strong>) }
+            let_it_be(:expected_html) { %Q(a new comment with <strong>#{filename}</strong> <strong>#{filename_1}</strong>) }
+            let_it_be(:expected_template_html) { %Q(some text #{expected_html}) }
 
-              it_behaves_like 'handle template content', 'new_note', 2
-            end
+            it_behaves_like 'a service desk notification email', 2
+            it_behaves_like 'a service desk notification email with template content', 'new_note', 2
+          end
 
-            context 'when not all uploads processed correct' do
-              let_it_be(:expected_body) { %Q(some text a new comment with <strong>#{filename}</strong> <a href="#{project.web_url}#{upload_path_1}" data-canonical-src="#{upload_path_1}" data-link="true" class="gfm">#{filename_1}</a>) }
+          context 'when not all uploads processed correct' do
+            let_it_be(:expected_html) { %Q(a new comment with <strong>#{filename}</strong> <a href="#{project.web_url}#{upload_path_1}" data-canonical-src="#{upload_path_1}" data-link="true" class="gfm">#{filename_1}</a>) }
+            let_it_be(:expected_template_html) { %Q(some text #{expected_html}) }
 
-              it_behaves_like 'handle template content', 'new_note', 1
-            end
+            it_behaves_like 'a service desk notification email', 1
+            it_behaves_like 'a service desk notification email with template content', 'new_note', 1
           end
         end
+      end
 
-        context 'when UploaderFinder is raising error' do
-          before do
-            allow_next_instance_of(UploaderFinder) do |instance|
-              allow(instance).to receive(:execute).and_raise(StandardError)
-            end
-            expect(Gitlab::ErrorTracking).to receive(:track_exception).with(StandardError, project_id: note.project_id)
+      context 'when UploaderFinder is raising error' do
+        before do
+          allow_next_instance_of(UploaderFinder) do |instance|
+            allow(instance).to receive(:execute).and_raise(StandardError)
           end
-
-          let_it_be(:expected_body) { %Q(some text a new comment with <a href="#{project.web_url}#{upload_path}" data-canonical-src="#{upload_path}" data-link="true" class="gfm">#{filename}</a>) }
-
-          it_behaves_like 'handle template content', 'new_note'
+          expect(Gitlab::ErrorTracking).to receive(:track_exception).with(StandardError, project_id: note.project_id)
         end
 
-        context 'when FileUploader is raising error' do
-          before do
-            allow_next_instance_of(FileUploader) do |instance|
-              allow(instance).to receive(:read).and_raise(StandardError)
-            end
-            expect(Gitlab::ErrorTracking).to receive(:track_exception).with(StandardError, project_id: note.project_id)
-          end
+        let_it_be(:expected_template_html) { %Q(some text a new comment with <a href="#{project.web_url}#{upload_path}" data-canonical-src="#{upload_path}" data-link="true" class="gfm">#{filename}</a>) }
 
-          let_it_be(:expected_body) { %Q(some text a new comment with <a href="#{project.web_url}#{upload_path}" data-canonical-src="#{upload_path}" data-link="true" class="gfm">#{filename}</a>) }
-
-          it_behaves_like 'handle template content', 'new_note'
-        end
+        it_behaves_like 'a service desk notification email with template content', 'new_note'
       end
 
-      context 'with all-user reference in a an external author comment' do
-        let_it_be(:note) { create(:note_on_issue, noteable: issue, project: project, note: "Hey @all, just a ping", author: User.support_bot) }
+      context 'when FileUploader is raising error' do
+        before do
+          allow_next_instance_of(FileUploader) do |instance|
+            allow(instance).to receive(:read).and_raise(StandardError)
+          end
+          expect(Gitlab::ErrorTracking).to receive(:track_exception).with(StandardError, project_id: note.project_id)
+        end
 
-        let(:template_content) { 'some text %{ NOTE_TEXT  }' }
-        let(:expected_body) { 'Hey , just a ping' }
+        let_it_be(:expected_template_html) { %Q(some text a new comment with <a href="#{project.web_url}#{upload_path}" data-canonical-src="#{upload_path}" data-link="true" class="gfm">#{filename}</a>) }
 
-        it_behaves_like 'handle template content', 'new_note'
+        it_behaves_like 'a service desk notification email with template content', 'new_note'
       end
     end
   end