Merge branch...

Merge branch 'ee-55183-frozenerror-can-t-modify-frozen-string-in-app-mailers-notify-rb' into 'master'

[EE] Fix a potential frozen string error in app/mailers/notify.rb and add tests for Service desk emails

Closes gitlab-ce#55183

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

app/mailers/notify.rb

@@ -130,7 +130,7 @@ class Notify < BaseMailer
         address.display_name = reply_display_name(model)
       end
 
-      fallback_reply_message_id = "<reply-#{reply_key}@#{Gitlab.config.gitlab.host}>".freeze
+      fallback_reply_message_id = "<reply-#{reply_key}@#{Gitlab.config.gitlab.host}>"
       headers['References'] ||= []
       headers['References'].unshift(fallback_reply_message_id)
 
@@ -180,7 +180,7 @@ class Notify < BaseMailer
 
     headers['X-GitLab-Discussion-ID'] = note.discussion.id if note.part_of_discussion?
 
-    headers[:subject]&.prepend('Re: ')
+    headers[:subject] = "Re: #{headers[:subject]}" if headers[:subject]
 
     mail_thread(model, headers)
   end

changelogs/unreleased/55183-frozenerror-can-t-modify-frozen-string-in-app-mailers-notify-rb.yml

+---
+title: Fix a potential frozen string error in app/mailers/notify.rb
+merge_request: 23728
+author:
+type: fixed

ee/app/mailers/emails/service_desk.rb

@@ -11,13 +11,13 @@ module Emails
     def service_desk_thank_you_email(issue_id)
       setup_service_desk_mail(issue_id)
 
-      mail_new_thread(@issue, service_desk_options(@support_bot.id))
+      mail_new_thread(@issue, service_desk_options(@support_bot.id).merge(subject: "Re: #{@issue.title} (##{@issue.iid})"))
     end
 
     def service_desk_new_note_email(issue_id, note_id)
       @note = Note.find(note_id)
       setup_service_desk_mail(issue_id)
-      mail_answer_thread(@issue, service_desk_options(@note.author_id))
+      mail_answer_thread(@issue, service_desk_options(@note.author_id).merge(subject: "#{@issue.title} (##{@issue.iid})"))
     end
 
     private
@@ -33,8 +33,7 @@ module Emails
     def service_desk_options(author_id)
       {
         from: sender(author_id),
-        to: @issue.service_desk_reply_to,
-        subject: "Re: #{@issue.title} (##{@issue.iid})"
+        to: @issue.service_desk_reply_to
       }
     end
   end

ee/spec/mailers/notify_spec.rb

@@ -4,6 +4,7 @@ require 'email_spec'
 describe Notify do
   include EmailSpec::Helpers
   include EmailSpec::Matchers
+  include EmailHelpers
   include RepoHelpers
 
   include_context 'gitlab email notification'
@@ -20,6 +21,13 @@ describe Notify do
                            description: 'Awesome description')
   end
 
+  set(:issue) do
+    create(:issue, author: current_user,
+                   assignees: [assignee],
+                   project: project,
+                   description: 'My awesome description!')
+  end
+
   set(:project2) { create(:project, :repository) }
   set(:merge_request_without_assignee) do
     create(:merge_request, source_project: project2,
@@ -28,6 +36,36 @@ describe Notify do
   end
 
   context 'for a project' do
+    context 'for service desk issues' do
+      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 subject and body' do
+          aggregate_failures do
+            is_expected.to have_referable_subject(issue, include_project: false, reply: true)
+            is_expected.to have_body_text("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
+        end
+      end
+
+      describe 'new note email' do
+        set(:first_note) { create(:discussion_note_on_issue, note: 'Hello world') }
+
+        subject { described_class.service_desk_new_note_email(issue.id, first_note.id) }
+
+        it_behaves_like 'an unsubscribeable thread'
+
+        it 'has the correct subject and body' do
+          aggregate_failures do
+            is_expected.to have_referable_subject(issue, include_project: false, reply: true)
+            is_expected.to have_body_text(first_note.note)
+          end
+        end
+      end
+    end
+
     context 'for merge requests' do
       describe "that are new with approver" do
         before do

spec/mailers/notify_spec.rb

@@ -4,6 +4,7 @@ require 'email_spec'
 describe Notify do
   include EmailSpec::Helpers
   include EmailSpec::Matchers
+  include EmailHelpers
   include RepoHelpers
 
   include_context 'gitlab email notification'
@@ -27,15 +28,6 @@ describe Notify do
                    description: 'My awesome description!')
   end
 
-  def have_referable_subject(referable, reply: false)
-    prefix = (referable.project ? "#{referable.project.name} | " : '').freeze
-    prefix = "Re: #{prefix}" if reply
-
-    suffix = "#{referable.title} (#{referable.to_reference})"
-
-    have_subject [prefix, suffix].compact.join
-  end
-
   context 'for a project' do
     shared_examples 'an assignee email' do
       it 'is sent to the assignee as the author' do

spec/support/helpers/email_helpers.rb

@@ -34,4 +34,13 @@ module EmailHelpers
   def find_email_for(user)
     ActionMailer::Base.deliveries.find { |d| d.to.include?(user.notification_email) }
   end
+
+  def have_referable_subject(referable, include_project: true, reply: false)
+    prefix = (include_project && referable.project ? "#{referable.project.name} | " : '').freeze
+    prefix = "Re: #{prefix}" if reply
+
+    suffix = "#{referable.title} (#{referable.to_reference})"
+
+    have_subject [prefix, suffix].compact.join
+  end
 end