diff options
author | hhoopes <heidih@gmail.com> | 2016-08-22 23:36:30 -0600 |
---|---|---|
committer | Sean McGivern <sean@gitlab.com> | 2016-11-25 15:23:49 +0000 |
commit | 24070bac45134c915c13d3e94723a44f59ab4e3a (patch) | |
tree | 59ba105e715d8a2f65547ca1f03e9ba231b48ff4 | |
parent | 38ed96e9b1a47dca5aa2590fa9b0ade908337435 (diff) | |
download | gitlab-ce-24070bac45134c915c13d3e94723a44f59ab4e3a.tar.gz |
Add new template to handle both commit & mr notes
Currently comments on commits and merge requests do not require merge request- or commit-specific information, but can use the same template. Rather than change the method which calls the template, I opted to keep the templates separate and create a new template to highlight their identicality, while preserving the option to distinguish them from each other in the future.
Also removed some of the inconsistencies between text and html email versions.
Still needed is a text-only version of git diffs and testing.
-rw-r--r-- | app/mailers/emails/notes.rb | 2 | ||||
-rw-r--r-- | app/views/notify/_note_mr_or_commit_email.html.haml | 16 | ||||
-rw-r--r-- | app/views/notify/note_commit_email.html.haml | 9 | ||||
-rw-r--r-- | app/views/notify/note_commit_email.text.erb | 6 | ||||
-rw-r--r-- | app/views/notify/note_merge_request_email.html.haml | 22 | ||||
-rw-r--r-- | app/views/notify/note_merge_request_email.text.erb | 6 | ||||
-rw-r--r-- | spec/mailers/notify_spec.rb | 19 |
7 files changed, 40 insertions, 40 deletions
diff --git a/app/mailers/emails/notes.rb b/app/mailers/emails/notes.rb index 96116e916dd..0d20c9092c4 100644 --- a/app/mailers/emails/notes.rb +++ b/app/mailers/emails/notes.rb @@ -4,6 +4,7 @@ module Emails setup_note_mail(note_id, recipient_id) @commit = @note.noteable + @discussion = @note.to_discussion if @note.diff_note? @target_url = namespace_project_commit_url(*note_target_url_options) mail_answer_thread(@commit, @@ -24,6 +25,7 @@ module Emails setup_note_mail(note_id, recipient_id) @merge_request = @note.noteable + @discussion = @note.to_discussion if @note.diff_note? @target_url = namespace_project_merge_request_url(*note_target_url_options) mail_answer_thread(@merge_request, note_thread_options(recipient_id)) end diff --git a/app/views/notify/_note_mr_or_commit_email.html.haml b/app/views/notify/_note_mr_or_commit_email.html.haml new file mode 100644 index 00000000000..3e2046aa9cf --- /dev/null +++ b/app/views/notify/_note_mr_or_commit_email.html.haml @@ -0,0 +1,16 @@ += content_for :head do + = stylesheet_link_tag 'mailers/highlighted_diff_email' + +- if note.diff_note? && note.diff_file + = link_to note.diff_file.file_path, @target_url, class: 'details' + \: + + %table + = render partial: "projects/diffs/line", + collection: discussion.truncated_diff_lines, + as: :line, + locals: { diff_file: note.diff_file, + plain: true, + email: true } + += render 'note_message' diff --git a/app/views/notify/note_commit_email.html.haml b/app/views/notify/note_commit_email.html.haml index 1d961e4424c..f4293a3aa50 100644 --- a/app/views/notify/note_commit_email.html.haml +++ b/app/views/notify/note_commit_email.html.haml @@ -1,2 +1,7 @@ -= render 'note_message' - +%p.details + New comment for Commit + = @commit.short_id + on + = render partial: 'note_mr_or_commit_email', + locals: { note: @note, + discussion: @discussion} diff --git a/app/views/notify/note_commit_email.text.erb b/app/views/notify/note_commit_email.text.erb index aaeaf5fdf73..a1ef9858021 100644 --- a/app/views/notify/note_commit_email.text.erb +++ b/app/views/notify/note_commit_email.text.erb @@ -2,8 +2,8 @@ New comment for Commit <%= @commit.short_id %> <%= url_for(namespace_project_commit_url(@note.project.namespace, @note.project, id: @commit.id, anchor: "note_#{@note.id}")) %> - -Author: <%= @note.author_name %> +<% if current_application_settings.email_author_in_body %> + <%= @note.author_name %> wrote: +<% end %> <%= @note.note %> - diff --git a/app/views/notify/note_merge_request_email.html.haml b/app/views/notify/note_merge_request_email.html.haml index de3f32e6415..75250b0383d 100644 --- a/app/views/notify/note_merge_request_email.html.haml +++ b/app/views/notify/note_merge_request_email.html.haml @@ -1,15 +1,7 @@ -= content_for :head do - = stylesheet_link_tag 'mailers/highlighted_diff_email' - -- if @note.diff_note? && @note.diff_file - %p.details - New comment on diff for - = link_to @note.diff_file.file_path, @target_url - \: - - .diff-content.code.js-syntax-highlight - %table - - Discussion.new([@note]).truncated_diff_lines.each do |line| - = render "projects/diffs/line", line: line, diff_file: @note.diff_file, plain: true - -= render 'note_message' +%p.details + New comment for Merge Request + = @merge_request.to_reference + on + = render partial: 'note_mr_or_commit_email', + locals: { note: @note, + discussion: @discussion} diff --git a/app/views/notify/note_merge_request_email.text.erb b/app/views/notify/note_merge_request_email.text.erb index 8cdab63829e..42d29b34ebc 100644 --- a/app/views/notify/note_merge_request_email.text.erb +++ b/app/views/notify/note_merge_request_email.text.erb @@ -2,8 +2,8 @@ New comment for Merge Request <%= @merge_request.to_reference %> <%= url_for(namespace_project_merge_request_url(@merge_request.target_project.namespace, @merge_request.target_project, @merge_request, anchor: "note_#{@note.id}")) %> - -<%= @note.author_name %> +<% if current_application_settings.email_author_in_body %> + <%= @note.author_name %> wrote: +<% end %> <%= @note.note %> - diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index b40a6b1cbac..932a5dc4862 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -580,10 +580,8 @@ describe Notify do let(:note_author) { create(:user, name: 'author_name') } let(:note) { create(:note, project: project, author: note_author) } - before do |example| - unless example.metadata[:skip_before] - allow(Note).to receive(:find).with(note.id).and_return(note) - end + before :each do + allow(Note).to receive(:find).with(note.id).and_return(note) end shared_examples 'a note email' do @@ -665,19 +663,6 @@ describe Notify do end end - describe "on a merge request with diffs", :skip_before do - let(:merge_request) { create(:merge_request_with_diffs) } - let(:note_with_diff) {create(:diff_note_on_merge_request)} - - before(:each) { allow(note_with_diff).to receive(:noteable).and_return(merge_request) } - subject { Notify.note_merge_request_email(recipient.id, note_with_diff.id) } - - it 'includes diffs with character-level highlighting' do - expected_line_text = Discussion.new([note_with_diff]).truncated_diff_lines.first.text - is_expected.to have_body_text expected_line_text - end - end - describe 'on an issue' do let(:issue) { create(:issue, project: project) } let(:note_on_issue_path) { namespace_project_issue_path(project.namespace, project, issue, anchor: "note_#{note.id}") } |