Commit 24070bac authored by hhoopes's avatar hhoopes Committed by Sean McGivern

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.
parent 38ed96e9
...@@ -4,6 +4,7 @@ module Emails ...@@ -4,6 +4,7 @@ module Emails
setup_note_mail(note_id, recipient_id) setup_note_mail(note_id, recipient_id)
@commit = @note.noteable @commit = @note.noteable
@discussion = @note.to_discussion if @note.diff_note?
@target_url = namespace_project_commit_url(*note_target_url_options) @target_url = namespace_project_commit_url(*note_target_url_options)
mail_answer_thread(@commit, mail_answer_thread(@commit,
...@@ -24,6 +25,7 @@ module Emails ...@@ -24,6 +25,7 @@ module Emails
setup_note_mail(note_id, recipient_id) setup_note_mail(note_id, recipient_id)
@merge_request = @note.noteable @merge_request = @note.noteable
@discussion = @note.to_discussion if @note.diff_note?
@target_url = namespace_project_merge_request_url(*note_target_url_options) @target_url = namespace_project_merge_request_url(*note_target_url_options)
mail_answer_thread(@merge_request, note_thread_options(recipient_id)) mail_answer_thread(@merge_request, note_thread_options(recipient_id))
end end
......
= 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'
= 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}
...@@ -2,8 +2,8 @@ New comment for Commit <%= @commit.short_id %> ...@@ -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}")) %> <%= url_for(namespace_project_commit_url(@note.project.namespace, @note.project, id: @commit.id, anchor: "note_#{@note.id}")) %>
<% if current_application_settings.email_author_in_body %>
Author: <%= @note.author_name %> <%= @note.author_name %> wrote:
<% end %>
<%= @note.note %> <%= @note.note %>
= content_for :head do %p.details
= stylesheet_link_tag 'mailers/highlighted_diff_email' New comment for Merge Request
= @merge_request.to_reference
- if @note.diff_note? && @note.diff_file on
%p.details = render partial: 'note_mr_or_commit_email',
New comment on diff for locals: { note: @note,
= link_to @note.diff_file.file_path, @target_url discussion: @discussion}
\:
.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'
...@@ -2,8 +2,8 @@ New comment for Merge Request <%= @merge_request.to_reference %> ...@@ -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}")) %> <%= url_for(namespace_project_merge_request_url(@merge_request.target_project.namespace, @merge_request.target_project, @merge_request, anchor: "note_#{@note.id}")) %>
<% if current_application_settings.email_author_in_body %>
<%= @note.author_name %> <%= @note.author_name %> wrote:
<% end %>
<%= @note.note %> <%= @note.note %>
...@@ -580,10 +580,8 @@ describe Notify do ...@@ -580,10 +580,8 @@ describe Notify do
let(:note_author) { create(:user, name: 'author_name') } let(:note_author) { create(:user, name: 'author_name') }
let(:note) { create(:note, project: project, author: note_author) } let(:note) { create(:note, project: project, author: note_author) }
before do |example| before :each do
unless example.metadata[:skip_before] allow(Note).to receive(:find).with(note.id).and_return(note)
allow(Note).to receive(:find).with(note.id).and_return(note)
end
end end
shared_examples 'a note email' do shared_examples 'a note email' do
...@@ -665,19 +663,6 @@ describe Notify do ...@@ -665,19 +663,6 @@ describe Notify do
end end
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 describe 'on an issue' do
let(:issue) { create(:issue, project: project) } let(:issue) { create(:issue, project: project) }
let(:note_on_issue_path) { namespace_project_issue_path(project.namespace, project, issue, anchor: "note_#{note.id}") } let(:note_on_issue_path) { namespace_project_issue_path(project.namespace, project, issue, anchor: "note_#{note.id}") }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment