Merge branch '63863-fix-groups-in-email-subject' into 'master'

Remove group in notification email subject

Closes #63863

See merge request gitlab-org/gitlab-ce!30214

app/mailers/emails/notes.rb

@@ -51,7 +51,7 @@ module Emails
     def note_thread_options(recipient_id)
       {
         from: sender(@note.author_id),
-        to: recipient(recipient_id, @group),
+        to: recipient(recipient_id, @project&.group || @group),
         subject: subject("#{@note.noteable.title} (#{@note.noteable.reference_link_text})")
       }
     end
@@ -60,7 +60,7 @@ module Emails
       # `note_id` is a `Note` when originating in `NotifyPreview`
       @note = note_id.is_a?(Note) ? note_id : Note.find(note_id)
       @project = @note.project
-      @group = @project.try(:group) || @note.noteable.try(:group)
+      @group = @note.noteable.try(:group)
       if (@project || @group) && @note.persisted?
         @sent_notification = SentNotification.record_note(@note, recipient_id, reply_key)

spec/mailers/notify_spec.rb

@@ -606,7 +606,7 @@ describe Notify do
       it_behaves_like 'a user cannot unsubscribe through footer link'
       it 'has the correct subject and body' do
-        is_expected.to have_referable_subject(project_snippet, include_group: true, reply: true)
+        is_expected.to have_referable_subject(project_snippet, reply: true)
         is_expected.to have_body_text project_snippet_note.note
       end
     end
@@ -813,7 +813,7 @@ describe Notify do
         it 'has the correct subject and body' do
           aggregate_failures do
-            is_expected.to have_subject("Re: #{project.name} | #{project.group.name} | #{commit.title} (#{commit.short_id})")
+            is_expected.to have_subject("Re: #{project.name} | #{commit.title} (#{commit.short_id})")
             is_expected.to have_body_text(commit.short_id)
           end
         end
@@ -839,7 +839,7 @@ describe Notify do
         it 'has the correct subject and body' do
           aggregate_failures do
-            is_expected.to have_referable_subject(merge_request, include_group: true, reply: true)
+            is_expected.to have_referable_subject(merge_request, reply: true)
             is_expected.to have_body_text note_on_merge_request_path
           end
         end
@@ -865,7 +865,7 @@ describe Notify do
         it 'has the correct subject and body' do
           aggregate_failures do
-            is_expected.to have_referable_subject(issue, include_group: true, reply: true)
+            is_expected.to have_referable_subject(issue, reply: true)
             is_expected.to have_body_text(note_on_issue_path)
           end
         end
@@ -931,7 +931,7 @@ describe Notify do
         it_behaves_like 'appearance header and footer not enabled'
         it 'has the correct subject' do
-          is_expected.to have_subject "Re: #{project.name} | #{project.group.name} | #{commit.title} (#{commit.short_id})"
+          is_expected.to have_subject "Re: #{project.name} | #{commit.title} (#{commit.short_id})"
         end
         it 'contains a link to the commit' do
@@ -959,7 +959,7 @@ describe Notify do
         it_behaves_like 'appearance header and footer not enabled'
         it 'has the correct subject' do
-          is_expected.to have_referable_subject(merge_request, include_group: true, reply: true)
+          is_expected.to have_referable_subject(merge_request, reply: true)
         end
         it 'contains a link to the merge request note' do
@@ -987,7 +987,7 @@ describe Notify do
         it_behaves_like 'appearance header and footer not enabled'
         it 'has the correct subject' do
-          is_expected.to have_referable_subject(issue, include_group: true, reply: true)
+          is_expected.to have_referable_subject(issue, reply: true)
         end
         it 'contains a link to the issue note' do

spec/support/helpers/email_helpers.rb

@@ -37,19 +37,8 @@ module EmailHelpers
     ActionMailer::Base.deliveries.find { |d| d.to.include?(user.notification_email) }
   end
-  def have_referable_subject(referable, include_project: true, include_group: false, reply: false)
-    context = []
-
-    context << referable.project.name if include_project && referable.project
-    context << referable.project.group.name if include_group && referable.project.group
-
-    prefix =
-      if context.any?
-        context.join(' | ') + ' | '
-      else
-        ''
-      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})"