summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorHeinrich Lee Yu <heinrich@gitlab.com>2019-07-01 16:23:11 +0800
committerHeinrich Lee Yu <heinrich@gitlab.com>2019-07-01 16:23:11 +0800
commiteec2c480130b4faea3446e366f77221fa3f982f0 (patch)
treef3e206c80f710d7ffd4d7d1874de7a8b1b851509
parent7b1d9a159a302cfb5c6e82c8412eb7ece2e59931 (diff)
downloadgitlab-ce-eec2c480130b4faea3446e366f77221fa3f982f0.tar.gz
Remove group in notification email subject
This reverts to previous behavior where we have the group only when we don't have a project
-rw-r--r--app/mailers/emails/notes.rb4
-rw-r--r--spec/mailers/notify_spec.rb14
-rw-r--r--spec/support/helpers/email_helpers.rb15
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})"