diff options
-rw-r--r-- | app/models/concerns/discussion_on_diff.rb | 5 | ||||
-rw-r--r-- | app/views/notify/_note_email.html.haml | 23 | ||||
-rw-r--r-- | app/views/notify/_note_email.text.erb | 13 | ||||
-rw-r--r-- | app/workers/new_note_worker.rb | 9 | ||||
-rw-r--r-- | spec/models/concerns/discussion_on_diff_spec.rb | 28 |
5 files changed, 61 insertions, 17 deletions
diff --git a/app/models/concerns/discussion_on_diff.rb b/app/models/concerns/discussion_on_diff.rb index c180d7b7c9a..266c37fa3a1 100644 --- a/app/models/concerns/discussion_on_diff.rb +++ b/app/models/concerns/discussion_on_diff.rb @@ -38,12 +38,13 @@ module DiscussionOnDiff end # Returns an array of at most 16 highlighted lines above a diff note - def truncated_diff_lines(highlight: true) + def truncated_diff_lines(highlight: true, diff_limit: nil) return [] if diff_line.nil? && first_note.is_a?(LegacyDiffNote) + diff_limit = [diff_limit, NUMBER_OF_TRUNCATED_DIFF_LINES].compact.min lines = highlight ? highlighted_diff_lines : diff_lines - initial_line_index = [diff_line.index - NUMBER_OF_TRUNCATED_DIFF_LINES + 1, 0].max + initial_line_index = [diff_line.index - diff_limit + 1, 0].max prev_lines = [] diff --git a/app/views/notify/_note_email.html.haml b/app/views/notify/_note_email.html.haml index 94bd6f96dbc..1fbae2f64ed 100644 --- a/app/views/notify/_note_email.html.haml +++ b/app/views/notify/_note_email.html.haml @@ -1,13 +1,18 @@ -- discussion = @note.discussion if @note.part_of_discussion? +- note = local_assigns.fetch(:note, @note) +- diff_limit = local_assigns.fetch(:diff_limit, nil) +- target_url = local_assigns.fetch(:target_url, @target_url) +- note_style = local_assigns.fetch(:note_style, "") + +- discussion = note.discussion if note.part_of_discussion? - diff_discussion = discussion&.diff_discussion? - on_image = discussion.on_image? if diff_discussion - if discussion - phrase_end_char = on_image ? "." : ":" - %p.details + %p{ style: "color: #777777;" } = succeed phrase_end_char do - = link_to @note.author_name, user_url(@note.author) + = link_to note.author_name, user_url(note.author) - if diff_discussion - if discussion.new_discussion? @@ -15,16 +20,16 @@ - else commented on a discussion - on #{link_to discussion.file_path, @target_url} + 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} + 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 note.author_name, user_url(note.author)} commented: - if diff_discussion && !on_image = content_for :head do @@ -32,11 +37,11 @@ %table = render partial: "projects/diffs/line", - collection: discussion.truncated_diff_lines, + collection: discussion.truncated_diff_lines(diff_limit: diff_limit), as: :line, locals: { diff_file: discussion.diff_file, plain: true, email: true } -%div - = markdown(@note.note, pipeline: :email, author: @note.author) +%div{ style: note_style } + = markdown(note.note, pipeline: :email, author: note.author) diff --git a/app/views/notify/_note_email.text.erb b/app/views/notify/_note_email.text.erb index c319cb55e87..4bf252b6ce1 100644 --- a/app/views/notify/_note_email.text.erb +++ b/app/views/notify/_note_email.text.erb @@ -1,6 +1,9 @@ -<% discussion = @note.discussion if @note.part_of_discussion? -%> +<% note = local_assigns.fetch(:note, @note) -%> +<% diff_limit = local_assigns.fetch(:diff_limit, nil) -%> + +<% discussion = note.discussion if note.part_of_discussion? -%> <% if discussion && !discussion.individual_note? -%> -<%= @note.author_name -%> +<%= note.author_name -%> <% if discussion.new_discussion? -%> <%= " started a new discussion" -%> <% else -%> @@ -13,14 +16,14 @@ <% elsif Gitlab::CurrentSettings.email_author_in_body -%> -<%= "#{@note.author_name} commented:" -%> +<%= "#{note.author_name} commented:" -%> <% end -%> <% if discussion&.diff_discussion? -%> -<% discussion.truncated_diff_lines(highlight: false).each do |line| -%> +<% discussion.truncated_diff_lines(highlight: false, diff_limit: diff_limit).each do |line| -%> <%= "> #{line.text}\n" -%> <% end -%> <% end -%> -<%= @note.note -%> +<%= note.note -%> diff --git a/app/workers/new_note_worker.rb b/app/workers/new_note_worker.rb index 42f5b945a75..98f9f45e608 100644 --- a/app/workers/new_note_worker.rb +++ b/app/workers/new_note_worker.rb @@ -8,11 +8,18 @@ class NewNoteWorker # rubocop: disable CodeReuse/ActiveRecord def perform(note_id, _params = {}) if note = Note.find_by(id: note_id) - NotificationService.new.new_note(note) + NotificationService.new.new_note(note) unless skip_notification?(note) Notes::PostProcessService.new(note).execute else Rails.logger.error("NewNoteWorker: couldn't find note with ID=#{note_id}, skipping job") end end + + private + + # EE-only method + def skip_notification?(note) + false + end # rubocop: enable CodeReuse/ActiveRecord end diff --git a/spec/models/concerns/discussion_on_diff_spec.rb b/spec/models/concerns/discussion_on_diff_spec.rb index 8cd129dc851..73eb7a1160d 100644 --- a/spec/models/concerns/discussion_on_diff_spec.rb +++ b/spec/models/concerns/discussion_on_diff_spec.rb @@ -12,6 +12,34 @@ describe DiscussionOnDiff do expect(truncated_lines.count).to be <= DiffDiscussion::NUMBER_OF_TRUNCATED_DIFF_LINES end + + context 'with truncated diff lines diff limit set' do + let(:truncated_lines) do + subject.truncated_diff_lines( + diff_limit: diff_limit + ) + end + + context 'when diff limit is higher than default' do + let(:diff_limit) { DiffDiscussion::NUMBER_OF_TRUNCATED_DIFF_LINES + 1 } + + it 'returns fewer lines than the default' do + expect(subject.diff_lines.count).to be > diff_limit + + expect(truncated_lines.count).to be <= DiffDiscussion::NUMBER_OF_TRUNCATED_DIFF_LINES + end + end + + context 'when diff_limit is lower than default' do + let(:diff_limit) { 3 } + + it 'returns fewer lines than the default' do + expect(subject.diff_lines.count).to be > DiffDiscussion::NUMBER_OF_TRUNCATED_DIFF_LINES + + expect(truncated_lines.count).to be <= diff_limit + end + end + end end context "when some diff lines are meta" do |