diff options
-rw-r--r-- | app/mailers/emails/notes.rb | 4 | ||||
-rw-r--r-- | spec/mailers/notify_spec.rb | 14 | ||||
-rw-r--r-- | spec/support/helpers/email_helpers.rb | 15 |
3 files changed, 11 insertions, 22 deletions
diff --git a/app/mailers/emails/notes.rb b/app/mailers/emails/notes.rb index 506c8d251b7..04db1980b99 100644 --- a/app/mailers/emails/notes.rb +++ b/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) diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index fa1343fe759..56bbcc4c306 100644 --- a/spec/mailers/notify_spec.rb +++ b/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 diff --git a/spec/support/helpers/email_helpers.rb b/spec/support/helpers/email_helpers.rb index a7175491fa0..ed049daba80 100644 --- a/spec/support/helpers/email_helpers.rb +++ b/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})" |