diff options
26 files changed, 106 insertions, 84 deletions
diff --git a/app/mailers/emails/notes.rb b/app/mailers/emails/notes.rb index 46fa6fd9f6d..de58314fef2 100644 --- a/app/mailers/emails/notes.rb +++ b/app/mailers/emails/notes.rb @@ -4,7 +4,7 @@ module Emails setup_note_mail(note_id, recipient_id) @commit = @note.noteable - @discussion = @note.to_discussion if @note.diff_note? + @discussion = @note.discussion if @note.part_of_discussion? @target_url = namespace_project_commit_url(*note_target_url_options) mail_answer_thread(@commit, @@ -17,6 +17,7 @@ module Emails setup_note_mail(note_id, recipient_id) @issue = @note.noteable + @discussion = @note.discussion if @note.part_of_discussion? @target_url = namespace_project_issue_url(*note_target_url_options) mail_answer_thread(@issue, note_thread_options(recipient_id)) end @@ -25,7 +26,7 @@ module Emails setup_note_mail(note_id, recipient_id) @merge_request = @note.noteable - @discussion = @note.to_discussion if @note.diff_note? + @discussion = @note.discussion if @note.part_of_discussion? @target_url = namespace_project_merge_request_url(*note_target_url_options) mail_answer_thread(@merge_request, note_thread_options(recipient_id)) end @@ -34,6 +35,7 @@ module Emails setup_note_mail(note_id, recipient_id) @snippet = @note.noteable + @discussion = @note.discussion if @note.part_of_discussion? @target_url = namespace_project_snippet_url(*note_target_url_options) mail_answer_thread(@snippet, note_thread_options(recipient_id)) end @@ -42,6 +44,7 @@ module Emails setup_note_mail(note_id, recipient_id) @snippet = @note.noteable + @discussion = @note.discussion if @note.part_of_discussion? @target_url = snippet_url(@note.noteable) mail_answer_thread(@snippet, note_thread_options(recipient_id)) end diff --git a/app/models/diff_discussion.rb b/app/models/diff_discussion.rb index 9a934c7520c..9c80b190b74 100644 --- a/app/models/diff_discussion.rb +++ b/app/models/diff_discussion.rb @@ -4,12 +4,14 @@ class DiffDiscussion < Discussion delegate :line_code, :original_line_code, :diff_file, + :diff_line, :for_line?, :active?, to: :first_note - delegate :blob, + delegate :file_path, + :blob, :highlighted_diff_lines, :diff_lines, diff --git a/app/models/discussion.rb b/app/models/discussion.rb index 314aea2c63a..8ab9031e42c 100644 --- a/app/models/discussion.rb +++ b/app/models/discussion.rb @@ -87,6 +87,10 @@ class Discussion false end + def new_discussion? + notes.length == 1 + end + def potentially_resolvable? first_note.for_merge_request? end diff --git a/app/models/note.rb b/app/models/note.rb index 6385747b571..06ceb60b982 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -261,7 +261,7 @@ class Note < ActiveRecord::Base # Returns the entire discussion this note is part of def discussion if part_of_discussion? - self.noteable.notes.find_discussion(self.discussion_id) + self.noteable.notes.find_discussion(self.discussion_id) || to_discussion else to_discussion end diff --git a/app/views/layouts/mailer.text.erb b/app/views/layouts/mailer.text.erb new file mode 100644 index 00000000000..198f30a1dc4 --- /dev/null +++ b/app/views/layouts/mailer.text.erb @@ -0,0 +1,4 @@ +<%= yield -%> + +--- +You're receiving this email because of your account on <%= Gitlab.config.gitlab.host %>. diff --git a/app/views/layouts/mailer.text.haml b/app/views/layouts/mailer.text.haml deleted file mode 100644 index 6a9c6ced9cc..00000000000 --- a/app/views/layouts/mailer.text.haml +++ /dev/null @@ -1,5 +0,0 @@ -= yield - -You're receiving this email because of your account on #{Gitlab.config.gitlab.host}. -Manage all notifications: #{profile_notifications_url} -Help: #{help_url} diff --git a/app/views/layouts/notify.text.erb b/app/views/layouts/notify.text.erb new file mode 100644 index 00000000000..b4ce02eead8 --- /dev/null +++ b/app/views/layouts/notify.text.erb @@ -0,0 +1,12 @@ +<%= yield -%> + +--- +<% if @target_url -%> +<% if @reply_by_email -%> +<%= "Reply to this email directly or view it on GitLab: #{@target_url}" -%> +<% else -%> +<%= "View it on GitLab: #{@target_url}" -%> +<% end -%> +<% end -%> + +You're receiving this email because of your account on <%= Gitlab.config.gitlab.host %>. diff --git a/app/views/notify/_note_email.html.haml b/app/views/notify/_note_email.html.haml new file mode 100644 index 00000000000..8b139a150b7 --- /dev/null +++ b/app/views/notify/_note_email.html.haml @@ -0,0 +1,36 @@ +- if @discussion + %p.details + = succeed ':' do + = link_to @note.author_name, user_url(@note.author) + + - 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} + - else + - if @discussion.new_discussion? + started a new discussion + - else + commented on a #{link_to 'discussion', @target_url} + +- elsif current_application_settings.email_author_in_body + %p.details + #{link_to @note.author_name, user_url(@note.author)} commented: + +- if @discussion&.diff_discussion? + = content_for :head do + = stylesheet_link_tag 'mailers/highlighted_diff_email' + + %table + = render partial: "projects/diffs/line", + collection: @discussion.truncated_diff_lines, + as: :line, + locals: { diff_file: @discussion.diff_file, + plain: true, + email: true } + +%div + = 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 new file mode 100644 index 00000000000..f2db321859f --- /dev/null +++ b/app/views/notify/_note_email.text.erb @@ -0,0 +1,25 @@ +<% if @discussion -%> +<%= @note.author_name -%> +<% if @discussion.new_discussion? -%> +<%= " started a new discussion" -%> +<% else -%> +<%= " commented on a discussion" -%> +<% end -%> +<% if @discussion.diff_discussion? -%> +<%= " on #{@discussion.file_path}" -%> +<% end -%> +<%= ":" -%> + + +<% elsif current_application_settings.email_author_in_body -%> +<%= "#{@note.author_name} commented:" -%> + + +<% end -%> +<% if @discussion&.diff_discussion? -%> +<% @discussion.truncated_diff_lines(highlight: false).each do |line| -%> +<%= "> #{line.text}\n" -%> +<% end -%> + +<% end -%> +<%= @note.note -%> diff --git a/app/views/notify/_note_message.html.haml b/app/views/notify/_note_message.html.haml deleted file mode 100644 index e9c66170877..00000000000 --- a/app/views/notify/_note_message.html.haml +++ /dev/null @@ -1,5 +0,0 @@ -- if current_application_settings.email_author_in_body - %div - #{link_to @note.author_name, user_url(@note.author)} wrote: -%div - = markdown(@note.note, pipeline: :email, author: @note.author) diff --git a/app/views/notify/_note_message.text.erb b/app/views/notify/_note_message.text.erb deleted file mode 100644 index f82cbc9a3fc..00000000000 --- a/app/views/notify/_note_message.text.erb +++ /dev/null @@ -1,5 +0,0 @@ -<% 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 deleted file mode 100644 index edf8dfe7e9e..00000000000 --- a/app/views/notify/_note_mr_or_commit_email.html.haml +++ /dev/null @@ -1,18 +0,0 @@ -= content_for :head do - = stylesheet_link_tag 'mailers/highlighted_diff_email' - -New comment - -- if @discussion && @discussion.diff_file - on - = 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_mr_or_commit_email.text.erb b/app/views/notify/_note_mr_or_commit_email.text.erb deleted file mode 100644 index b4fcdf6b1e9..00000000000 --- a/app/views/notify/_note_mr_or_commit_email.text.erb +++ /dev/null @@ -1,8 +0,0 @@ -<% if @discussion && @discussion.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 deleted file mode 100644 index c28d1cc34d3..00000000000 --- a/app/views/notify/_simple_diff.text.erb +++ /dev/null @@ -1,3 +0,0 @@ -<% @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 0a650e3b2ca..5e69f01a486 100644 --- a/app/views/notify/note_commit_email.html.haml +++ b/app/views/notify/note_commit_email.html.haml @@ -1,2 +1 @@ -%p.details - = render 'note_mr_or_commit_email' += render 'note_email' diff --git a/app/views/notify/note_commit_email.text.erb b/app/views/notify/note_commit_email.text.erb index 6aa085a172e..5585e7c3ee8 100644 --- a/app/views/notify/note_commit_email.text.erb +++ b/app/views/notify/note_commit_email.text.erb @@ -1,2 +1 @@ -New comment for Commit <%= @commit.short_id -%> -<%= render partial: 'note_mr_or_commit_email', locals: { url: @target_url } %> +<%= render partial: 'note_email' %> diff --git a/app/views/notify/note_issue_email.html.haml b/app/views/notify/note_issue_email.html.haml index 2fa2f784661..5e69f01a486 100644 --- a/app/views/notify/note_issue_email.html.haml +++ b/app/views/notify/note_issue_email.html.haml @@ -1 +1 @@ -= render 'note_message' += render 'note_email' diff --git a/app/views/notify/note_issue_email.text.erb b/app/views/notify/note_issue_email.text.erb index e33cbcd70f2..5585e7c3ee8 100644 --- a/app/views/notify/note_issue_email.text.erb +++ b/app/views/notify/note_issue_email.text.erb @@ -1,9 +1 @@ -New comment for Issue <%= @issue.iid %> - -<%= url_for(namespace_project_issue_url(@issue.project.namespace, @issue.project, @issue, anchor: "note_#{@note.id}")) %> - - -Author: <%= @note.author_name %> - -<%= @note.note %> - +<%= render partial: 'note_email' %> diff --git a/app/views/notify/note_merge_request_email.html.haml b/app/views/notify/note_merge_request_email.html.haml index 0a650e3b2ca..5e69f01a486 100644 --- a/app/views/notify/note_merge_request_email.html.haml +++ b/app/views/notify/note_merge_request_email.html.haml @@ -1,2 +1 @@ -%p.details - = render 'note_mr_or_commit_email' += render 'note_email' diff --git a/app/views/notify/note_merge_request_email.text.erb b/app/views/notify/note_merge_request_email.text.erb index 2ce64c494cf..381f0366971 100644 --- a/app/views/notify/note_merge_request_email.text.erb +++ b/app/views/notify/note_merge_request_email.text.erb @@ -1,2 +1 @@ -New comment for Merge Request <%= @merge_request.to_reference -%> -<%= render partial: 'note_mr_or_commit_email', locals: { url: @target_url }%> +<%= render partial: 'note_email'%> diff --git a/app/views/notify/note_personal_snippet_email.html.haml b/app/views/notify/note_personal_snippet_email.html.haml index 2fa2f784661..5e69f01a486 100644 --- a/app/views/notify/note_personal_snippet_email.html.haml +++ b/app/views/notify/note_personal_snippet_email.html.haml @@ -1 +1 @@ -= render 'note_message' += render 'note_email' diff --git a/app/views/notify/note_personal_snippet_email.text.erb b/app/views/notify/note_personal_snippet_email.text.erb index b2a8809a23b..5585e7c3ee8 100644 --- a/app/views/notify/note_personal_snippet_email.text.erb +++ b/app/views/notify/note_personal_snippet_email.text.erb @@ -1,8 +1 @@ -New comment for Snippet <%= @snippet.id %> - -<%= url_for(snippet_url(@snippet, anchor: "note_#{@note.id}")) %> - - -Author: <%= @note.author_name %> - -<%= @note.note %> +<%= render partial: 'note_email' %> diff --git a/app/views/notify/note_snippet_email.html.haml b/app/views/notify/note_snippet_email.html.haml index 2fa2f784661..5e69f01a486 100644 --- a/app/views/notify/note_snippet_email.html.haml +++ b/app/views/notify/note_snippet_email.html.haml @@ -1 +1 @@ -= render 'note_message' += render 'note_email' diff --git a/app/views/notify/note_snippet_email.text.erb b/app/views/notify/note_snippet_email.text.erb index 4d5a406f4b0..5585e7c3ee8 100644 --- a/app/views/notify/note_snippet_email.text.erb +++ b/app/views/notify/note_snippet_email.text.erb @@ -1,8 +1 @@ -New comment for Snippet <%= @snippet.id %> - -<%= url_for(namespace_project_snippet_url(@snippet.project.namespace, @snippet.project, @snippet, anchor: "note_#{@note.id}")) %> - - -Author: <%= @note.author_name %> - -<%= @note.note %> +<%= render partial: 'note_email' %> diff --git a/lib/gitlab/diff/line.rb b/lib/gitlab/diff/line.rb index 114656958e3..0a15c6d9358 100644 --- a/lib/gitlab/diff/line.rb +++ b/lib/gitlab/diff/line.rb @@ -33,6 +33,10 @@ module Gitlab new_pos unless removed? || meta? end + def line + new_line || old_line + end + def unchanged? type.nil? end diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index 6a89b007f96..01246f87dff 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -536,6 +536,8 @@ describe Notify do allow(Note).to receive(:find).with(note.id).and_return(note) end + # TODO: Test discussions + shared_examples 'a note email' do it_behaves_like 'it should have Gmail Actions links' |