summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSean McGivern <sean@gitlab.com>2019-06-21 14:39:49 +0000
committerSean McGivern <sean@gitlab.com>2019-06-21 14:39:49 +0000
commitf318a14366a46e72a402402030ab809a2d4ee1ab (patch)
treecf00b643ef56f72e0585abc51d1c1f3317a4ab51
parent671d7cdc445b3a4fdbd2295996d149e833d6b6c2 (diff)
parentdf4c1a75dec4da15c38066ca73312ed91baeb782 (diff)
downloadgitlab-ce-f318a14366a46e72a402402030ab809a2d4ee1ab.tar.gz
Merge branch '58065-uniform-html-txt-email' into 'master'
Make HTML and text emails for new issues uniform and add the mail to mailer previews Closes #58065 See merge request gitlab-org/gitlab-ce!29886
-rw-r--r--app/views/notify/_note_email.html.haml30
-rw-r--r--app/views/notify/_note_email.text.erb26
-rw-r--r--app/views/notify/new_issue_email.html.haml5
-rw-r--r--app/views/notify/new_issue_email.text.erb12
-rw-r--r--app/views/notify/new_merge_request_email.html.haml18
-rw-r--r--app/views/notify/new_merge_request_email.text.erb6
-rw-r--r--changelogs/unreleased/58065-uniform-html-txt-email.yml5
-rw-r--r--spec/mailers/notify_spec.rb28
-rw-r--r--spec/support/shared_examples/notify_shared_examples.rb14
9 files changed, 59 insertions, 85 deletions
diff --git a/app/views/notify/_note_email.html.haml b/app/views/notify/_note_email.html.haml
index 83c7f548975..dc5529b489b 100644
--- a/app/views/notify/_note_email.html.haml
+++ b/app/views/notify/_note_email.html.haml
@@ -5,27 +5,21 @@
- discussion = note.discussion if note.part_of_discussion?
-- if discussion
- %p{ style: "color: #777777;" }
- = succeed ':' do
- = link_to note.author_name, user_url(note.author)
+%p{ style: "color: #777777;" }
+ = succeed ':' do
+ = link_to note.author_name, user_url(note.author)
+ - if discussion.nil?
+ commented
+ - else
+ - if discussion.new_discussion?
+ started a new
+ - else
+ commented on a
- if discussion&.diff_discussion?
- - if discussion.new_discussion?
- started a new discussion
- - else
- commented on a discussion
-
- on #{link_to discussion.file_path, target_url}
+ discussion on #{link_to(discussion.file_path, target_url)}
- else
- - if discussion.new_discussion?
- started a new discussion
- - else
- commented on a #{link_to 'discussion', target_url}
-
-- elsif Gitlab::CurrentSettings.email_author_in_body
- %p.details
- #{link_to note.author_name, user_url(note.author)} commented:
+ = link_to 'discussion', target_url
- if discussion&.diff_discussion? && discussion.on_text?
= content_for :head do
diff --git a/app/views/notify/_note_email.text.erb b/app/views/notify/_note_email.text.erb
index fae8fa3ccf3..a25daad8458 100644
--- a/app/views/notify/_note_email.text.erb
+++ b/app/views/notify/_note_email.text.erb
@@ -1,29 +1,25 @@
<% note = local_assigns.fetch(:note, @note) -%>
<% diff_limit = local_assigns.fetch(:diff_limit, nil) -%>
<% target_url = local_assigns.fetch(:target_url, @target_url) -%>
+<% discussion = note.discussion if note.part_of_discussion? -%>
-<% discussion = note.discussion if note.part_of_discussion? -%>
-<% if discussion && !discussion.individual_note? -%>
-<%= sanitize_name(note.author_name) -%>
+<%= sanitize_name(note.author_name) -%>
+<% if discussion.nil? -%>
+ <%= 'commented' -%>:
+<% else -%>
<% if discussion.new_discussion? -%>
-<%= " started a new discussion" -%>
+ <%= 'started a new discussion' -%>
<% else -%>
-<%= " commented on a discussion" -%>
+ <%= 'commented on a discussion' -%>
<% end -%>
<% if discussion.diff_discussion? -%>
-<%= " on #{discussion.file_path}" -%>
+ <%= "on #{discussion.file_path}" -%>
<% end -%>
-<%= ":" -%>
-<% if discussion.diff_discussion? || !discussion.new_discussion? -%>
-<%= " #{target_url}" -%>
-<% end -%>
-
-
-<% elsif Gitlab::CurrentSettings.email_author_in_body -%>
-<%= "#{sanitize_name(note.author_name)} commented:" -%>
+<%= ':' -%>
+<%= " #{target_url}" -%>
+<% end -%>
-<% end -%>
<% if discussion&.diff_discussion? && discussion.on_text? -%>
<% discussion.truncated_diff_lines(highlight: false, diff_limit: diff_limit).each do |line| -%>
<%= "> #{line.text}\n" -%>
diff --git a/app/views/notify/new_issue_email.html.haml b/app/views/notify/new_issue_email.html.haml
index 8aa7939dd0b..78afb42c9cf 100644
--- a/app/views/notify/new_issue_email.html.haml
+++ b/app/views/notify/new_issue_email.html.haml
@@ -1,6 +1,5 @@
-- if Gitlab::CurrentSettings.email_author_in_body
- %p.details
- #{link_to @issue.author_name, user_url(@issue.author)} created an issue:
+%p.details
+ #{link_to @issue.author_name, user_url(@issue.author)} created an issue:
- if @issue.assignees.any?
%p
diff --git a/app/views/notify/new_issue_email.text.erb b/app/views/notify/new_issue_email.text.erb
index ff258711b48..b93d95ef02f 100644
--- a/app/views/notify/new_issue_email.text.erb
+++ b/app/views/notify/new_issue_email.text.erb
@@ -1,7 +1,9 @@
-New Issue was created.
+<%= sanitize_name(@issue.author_name) %> <%= 'created an issue:' %>
-Issue <%= @issue.iid %>: <%= url_for(project_issue_url(@issue.project, @issue)) %>
-Author: <%= sanitize_name(@issue.author_name) %>
-<%= assignees_label(@issue) %>
+<% if @issue.assignees.any? -%>
+ <%= assignees_label(@issue) %>
+<% end %>
-<%= @issue.description %>
+<% if @issue.description -%>
+ <%= @issue.description %>
+<% end %>
diff --git a/app/views/notify/new_merge_request_email.html.haml b/app/views/notify/new_merge_request_email.html.haml
index 9ab648e2a64..2ddea0b9f16 100644
--- a/app/views/notify/new_merge_request_email.html.haml
+++ b/app/views/notify/new_merge_request_email.html.haml
@@ -1,15 +1,15 @@
-- if Gitlab::CurrentSettings.email_author_in_body
- %p.details
- #{link_to @merge_request.author_name, user_url(@merge_request.author)} created a merge request:
-
%p.details
- = merge_path_description(@merge_request, '→')
+ #{link_to @merge_request.author_name, user_url(@merge_request.author)} created a merge request:
-- if @merge_request.assignees.any?
- %p
+%p
+ .branch
+ = merge_path_description(@merge_request, 'to')
+ .author
+ Author #{@merge_request.author_name}
+ .assignee
= assignees_label(@merge_request)
-
-= render_if_exists 'notify/merge_request_approvers', presenter: @mr_presenter
+ .approvers
+ = render_if_exists 'notify/merge_request_approvers', presenter: @mr_presenter
- if @merge_request.description
%div
diff --git a/app/views/notify/new_merge_request_email.text.erb b/app/views/notify/new_merge_request_email.text.erb
index e6c42f1cf5f..c3f2902c78a 100644
--- a/app/views/notify/new_merge_request_email.text.erb
+++ b/app/views/notify/new_merge_request_email.text.erb
@@ -1,9 +1,7 @@
-New Merge Request <%= @merge_request.to_reference %>
-
-<%= url_for(project_merge_request_url(@merge_request.target_project, @merge_request)) %>
+<%= @merge_request.author_name %> <%= 'created a merge request:' %> <%= url_for(project_merge_request_url(@merge_request.target_project, @merge_request)) %>
<%= merge_path_description(@merge_request, 'to') %>
-Author: <%= @merge_request.author_name %>
+<%= 'Author' %>: <%= @merge_request.author_name %>
<%= assignees_label(@merge_request) %>
<%= render_if_exists 'notify/merge_request_approvers', presenter: @mr_presenter %>
diff --git a/changelogs/unreleased/58065-uniform-html-txt-email.yml b/changelogs/unreleased/58065-uniform-html-txt-email.yml
new file mode 100644
index 00000000000..be34e93ff83
--- /dev/null
+++ b/changelogs/unreleased/58065-uniform-html-txt-email.yml
@@ -0,0 +1,5 @@
+---
+title: Always shows author of created issue/started discussion/comment in HTML body and text of email
+merge_request: 29886
+author: Frank van Rest
+type: fixed
diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb
index cbbb22ad78c..11af6837dab 100644
--- a/spec/mailers/notify_spec.rb
+++ b/spec/mailers/notify_spec.rb
@@ -99,15 +99,9 @@ describe Notify do
end
end
- context 'when enabled email_author_in_body' do
- before do
- stub_application_setting(email_author_in_body: true)
- end
-
- it 'contains a link to note author' do
- is_expected.to have_body_text(issue.author_name)
- is_expected.to have_body_text 'created an issue:'
- end
+ it 'contains a link to issue author' do
+ is_expected.to have_body_text(issue.author_name)
+ is_expected.to have_body_text 'created an issue:'
end
end
@@ -314,15 +308,9 @@ describe Notify do
end
end
- context 'when enabled email_author_in_body' do
- before do
- stub_application_setting(email_author_in_body: true)
- end
-
- it 'contains a link to note author' do
- is_expected.to have_body_text merge_request.author_name
- is_expected.to have_body_text 'created a merge request:'
- end
+ it 'contains a link to merge request author' do
+ is_expected.to have_body_text merge_request.author_name
+ is_expected.to have_body_text 'created a merge request:'
end
end
@@ -907,7 +895,9 @@ describe Notify do
end
it 'contains an introduction' do
- is_expected.to have_body_text 'started a new discussion'
+ issuable_url = "project_#{note.noteable_type.underscore}_url"
+
+ is_expected.to have_body_text "started a new <a href=\"#{public_send(issuable_url, project, note.noteable, anchor: "note_#{note.id}")}\">discussion</a>"
end
context 'when a comment on an existing discussion' do
diff --git a/spec/support/shared_examples/notify_shared_examples.rb b/spec/support/shared_examples/notify_shared_examples.rb
index 897c9106d77..6894a63ce42 100644
--- a/spec/support/shared_examples/notify_shared_examples.rb
+++ b/spec/support/shared_examples/notify_shared_examples.rb
@@ -281,18 +281,8 @@ shared_examples 'a note email' do
is_expected.to have_body_text note.note
end
- it 'does not contain note author' do
- is_expected.not_to have_body_text note.author_name
- end
-
- context 'when enabled email_author_in_body' do
- before do
- stub_application_setting(email_author_in_body: true)
- end
-
- it 'contains a link to note author' do
- is_expected.to have_body_text note.author_name
- end
+ it 'contains a link to note author' do
+ is_expected.to have_body_text note.author_name
end
end