From df4c1a75dec4da15c38066ca73312ed91baeb782 Mon Sep 17 00:00:00 2001 From: Frank van Rest Date: Fri, 19 Apr 2019 20:10:52 +0200 Subject: Uniform html and text emails Uniform new_issue_email html and text emails Uniform note_email html and text emails Uniform new_merge_request_email html and text emails --- app/views/notify/_note_email.html.haml | 30 +++++++++------------- app/views/notify/_note_email.text.erb | 26 ++++++++----------- app/views/notify/new_issue_email.html.haml | 5 ++-- app/views/notify/new_issue_email.text.erb | 12 +++++---- app/views/notify/new_merge_request_email.html.haml | 18 ++++++------- app/views/notify/new_merge_request_email.text.erb | 6 ++--- .../unreleased/58065-uniform-html-txt-email.yml | 5 ++++ spec/mailers/notify_spec.rb | 28 +++++++------------- .../shared_examples/notify_shared_examples.rb | 14 ++-------- 9 files changed, 59 insertions(+), 85 deletions(-) create mode 100644 changelogs/unreleased/58065-uniform-html-txt-email.yml 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 discussion" 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 -- cgit v1.2.1