From a761c59a6bfc4d66649910d01e4c8412bb0b40ec Mon Sep 17 00:00:00 2001 From: hhoopes Date: Wed, 31 Aug 2016 10:18:26 -0600 Subject: Add keyword arguments to truncated_diff method * Added keyword arguments to truncated_diff_lines method to allow for using highlighting or not (html templates vs. text) * Tweaked templates for consistency and format appropriateness --- app/models/discussion.rb | 7 ++++--- app/views/discussions/_diff_with_notes.html.haml | 2 +- app/views/notify/_note_message.text.erb | 5 +++++ app/views/notify/_note_mr_or_commit_email.html.haml | 11 +++++++---- app/views/notify/_note_mr_or_commit_email.text.erb | 8 ++++++++ app/views/notify/_simple_diff.text.erb | 5 ++--- app/views/notify/note_commit_email.html.haml | 3 --- app/views/notify/note_commit_email.text.erb | 13 +++---------- app/views/notify/note_merge_request_email.html.haml | 3 --- app/views/notify/note_merge_request_email.text.erb | 13 +++---------- lib/gitlab/diff/file.rb | 11 ++--------- spec/mailers/notify_spec.rb | 8 +------- spec/models/discussion_spec.rb | 4 ++-- 13 files changed, 38 insertions(+), 55 deletions(-) create mode 100644 app/views/notify/_note_message.text.erb create mode 100644 app/views/notify/_note_mr_or_commit_email.text.erb diff --git a/app/models/discussion.rb b/app/models/discussion.rb index 486bfd2c766..9bd37fe6d89 100644 --- a/app/models/discussion.rb +++ b/app/models/discussion.rb @@ -27,7 +27,7 @@ class Discussion delegate :blob, :highlighted_diff_lines, - :text_parsed_diff_lines, + :diff_lines, to: :diff_file, allow_nil: true @@ -164,10 +164,11 @@ class Discussion end # Returns an array of at most 16 highlighted lines above a diff note - def truncated_diff_lines + def truncated_diff_lines(highlight: true) + initial_lines = highlight ? highlighted_diff_lines : diff_lines prev_lines = [] - diff_file.diff_lines.each do |line| + initial_lines.each do |line| if line.meta? prev_lines.clear else diff --git a/app/views/discussions/_diff_with_notes.html.haml b/app/views/discussions/_diff_with_notes.html.haml index 06493ba0105..5c667e4842b 100644 --- a/app/views/discussions/_diff_with_notes.html.haml +++ b/app/views/discussions/_diff_with_notes.html.haml @@ -9,7 +9,7 @@ %table - discussions = { discussion.original_line_code => discussion } = render partial: "projects/diffs/line", - collection: discussion.highlighted_diff_lines(discussion.truncated_diff_lines), + collection: discussion.highlighted_diff_lines, as: :line, locals: { diff_file: diff_file, discussions: discussions, diff --git a/app/views/notify/_note_message.text.erb b/app/views/notify/_note_message.text.erb new file mode 100644 index 00000000000..f82cbc9a3fc --- /dev/null +++ b/app/views/notify/_note_message.text.erb @@ -0,0 +1,5 @@ +<% if current_application_settings.email_author_in_body %> + <%= @note.author_name %> wrote: +<% end -%> + +<%= @note.note %> diff --git a/app/views/notify/_note_mr_or_commit_email.html.haml b/app/views/notify/_note_mr_or_commit_email.html.haml index 7033842b557..15e92c42b14 100644 --- a/app/views/notify/_note_mr_or_commit_email.html.haml +++ b/app/views/notify/_note_mr_or_commit_email.html.haml @@ -1,17 +1,20 @@ = content_for :head do = stylesheet_link_tag 'mailers/highlighted_diff_email' +New comment + - if @note.diff_note? && @note.diff_file on = link_to @note.diff_file.file_path, @target_url, class: 'details' - \: +\: +- if @discussion %table = render partial: "projects/diffs/line", - collection: @discussion.highlighted_diff_lines(@discussion.truncated_diff_lines), + collection: @discussion.truncated_diff_lines, as: :line, locals: { diff_file: @note.diff_file, - plain: true, - email: true } + plain: true, + email: true } = render 'note_message' diff --git a/app/views/notify/_note_mr_or_commit_email.text.erb b/app/views/notify/_note_mr_or_commit_email.text.erb new file mode 100644 index 00000000000..3dd1b4d4c0e --- /dev/null +++ b/app/views/notify/_note_mr_or_commit_email.text.erb @@ -0,0 +1,8 @@ +<% if @note.diff_note? && @note.diff_file -%> + on <%= @note.diff_file.file_path -%> +<% end -%>: + +<%= url %> + +<%= render 'simple_diff' if @discussion -%> +<%= render 'note_message' %> diff --git a/app/views/notify/_simple_diff.text.erb b/app/views/notify/_simple_diff.text.erb index a5a796bc168..58b0018c0ca 100644 --- a/app/views/notify/_simple_diff.text.erb +++ b/app/views/notify/_simple_diff.text.erb @@ -1,4 +1,3 @@ -<% lines = @discussion.truncated_diff_lines %> -<% @discussion.text_parsed_diff_lines(lines).each do |line| %> - <%= line %> +<% @discussion.truncated_diff_lines(highlight: false).each do |line| %> + <%= "> " + line.text %> <% end %> diff --git a/app/views/notify/note_commit_email.html.haml b/app/views/notify/note_commit_email.html.haml index 17dcf36689f..0a650e3b2ca 100644 --- a/app/views/notify/note_commit_email.html.haml +++ b/app/views/notify/note_commit_email.html.haml @@ -1,5 +1,2 @@ %p.details - New comment for Commit - = @commit.short_id - = render 'note_mr_or_commit_email' diff --git a/app/views/notify/note_commit_email.text.erb b/app/views/notify/note_commit_email.text.erb index 715e58af61c..dc764b61659 100644 --- a/app/views/notify/note_commit_email.text.erb +++ b/app/views/notify/note_commit_email.text.erb @@ -1,11 +1,4 @@ -New comment for Commit <%= @commit.short_id %> +<% url = 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}")) %> - -<%= render 'simple_diff' if @discussion %> - -<% if current_application_settings.email_author_in_body %> - <%= @note.author_name %> wrote: -<% end %> - -<%= @note.note %> +New comment for Commit <%= @commit.short_id -%> +<%= render partial: 'note_mr_or_commit_email', locals: { url: url } %> diff --git a/app/views/notify/note_merge_request_email.html.haml b/app/views/notify/note_merge_request_email.html.haml index b7758f191dc..0a650e3b2ca 100644 --- a/app/views/notify/note_merge_request_email.html.haml +++ b/app/views/notify/note_merge_request_email.html.haml @@ -1,5 +1,2 @@ %p.details - New comment for Merge Request - = @merge_request.to_reference - = render 'note_mr_or_commit_email' diff --git a/app/views/notify/note_merge_request_email.text.erb b/app/views/notify/note_merge_request_email.text.erb index d24e15af91f..e33d15daded 100644 --- a/app/views/notify/note_merge_request_email.text.erb +++ b/app/views/notify/note_merge_request_email.text.erb @@ -1,11 +1,4 @@ -New comment for Merge Request <%= @merge_request.to_reference %> +<% url = 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}")) %> - -<%= render 'simple_diff' if @discussion %> - -<% if current_application_settings.email_author_in_body %> - <%= @note.author_name %> wrote: -<% end %> - -<%= @note.note %> +New comment for Merge Request <%= @merge_request.to_reference -%> +<%= render partial: 'note_mr_or_commit_email', locals: { url: url }%> diff --git a/lib/gitlab/diff/file.rb b/lib/gitlab/diff/file.rb index 9b60102947a..84784aaf2fd 100644 --- a/lib/gitlab/diff/file.rb +++ b/lib/gitlab/diff/file.rb @@ -76,15 +76,8 @@ module Gitlab @diff_lines ||= Gitlab::Diff::Parser.new.parse(raw_diff.each_line).to_a end - def highlighted_diff_lines(lines = self) - @highlighted_diff_lines ||= Gitlab::Diff::Highlight.new(lines, repository: self.repository).highlight - end - - def text_parsed_diff_lines(lines) - @text_parsed_diff_lines ||= - lines.map do | line | - "> " + line.text - end + def highlighted_diff_lines + @highlighted_diff_lines ||= Gitlab::Diff::Highlight.new(self, repository: self.repository).highlight end # Array[] with right/left keys that contains Gitlab::Diff::Line objects which text is hightlighted diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index a80eb114c17..824b516f856 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -697,7 +697,7 @@ describe Notify do let(:note) { create(model, project: project, author: note_author) } it "includes diffs with character-level highlighting" do - is_expected.to have_body_text /\vars = {<\/span>/ + is_expected.to have_body_text /}<\/span><\/span>/ end it 'contains a link to the diff file' do @@ -743,9 +743,6 @@ describe Notify do subject { Notify.note_commit_email(recipient.id, note.id) } it_behaves_like 'a note email on a diff', :diff_note_on_commit - # it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do - # let(:model) { commit } - # end it_behaves_like 'it should show Gmail Actions View Commit link' it_behaves_like 'a user cannot unsubscribe through footer link' end @@ -757,9 +754,6 @@ describe Notify do subject { Notify.note_merge_request_email(recipient.id, note.id) } it_behaves_like 'a note email on a diff', :diff_note_on_merge_request - # it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do - # let(:model) { merge_request } - # end it_behaves_like 'it should show Gmail Actions View Merge request link' it_behaves_like 'an unsubscribeable thread' end diff --git a/spec/models/discussion_spec.rb b/spec/models/discussion_spec.rb index d4b1f480c56..187d0bc0150 100644 --- a/spec/models/discussion_spec.rb +++ b/spec/models/discussion_spec.rb @@ -595,7 +595,7 @@ describe Discussion, model: true do let(:truncated_lines) { subject.truncated_diff_lines } context "when diff is greater than allowed number of truncated diff lines " do - let(:initial_line_count) { subject.diff_file.diff_lines.count } + let(:initial_line_count) { subject.diff_lines.count } let(:truncated_line_count) { truncated_lines.count } it "returns fewer lines" do @@ -606,7 +606,7 @@ describe Discussion, model: true do end context "when some diff lines are meta" do - let(:initial_meta_lines?) { subject.diff_file.diff_lines.any?(&:meta?) } + let(:initial_meta_lines?) { subject.diff_lines.any?(&:meta?) } let(:truncated_meta_lines?) { truncated_lines.any?(&:meta?) } it "returns no meta lines" do -- cgit v1.2.1