Commit 2acf3a56 authored by Toon Claes's avatar Toon Claes

Make mail notifications of discussion notes In-Reply-To of each other

When a note is part of a discussion, the email sent out should be
`In-Reply-To` the previous note in that discussion.

Closes gitlab-org/gitlab-ce#36054
parent 43b98944
...@@ -5,7 +5,7 @@ module Emails ...@@ -5,7 +5,7 @@ module Emails
@commit = @note.noteable @commit = @note.noteable
@target_url = project_commit_url(*note_target_url_options) @target_url = project_commit_url(*note_target_url_options)
mail_answer_thread(@commit, note_thread_options(recipient_id)) mail_answer_note_thread(@commit, @note, note_thread_options(recipient_id))
end end
def note_issue_email(recipient_id, note_id) def note_issue_email(recipient_id, note_id)
...@@ -13,7 +13,7 @@ module Emails ...@@ -13,7 +13,7 @@ module Emails
@issue = @note.noteable @issue = @note.noteable
@target_url = project_issue_url(*note_target_url_options) @target_url = project_issue_url(*note_target_url_options)
mail_answer_thread(@issue, note_thread_options(recipient_id)) mail_answer_note_thread(@issue, @note, note_thread_options(recipient_id))
end end
def note_merge_request_email(recipient_id, note_id) def note_merge_request_email(recipient_id, note_id)
...@@ -21,7 +21,7 @@ module Emails ...@@ -21,7 +21,7 @@ module Emails
@merge_request = @note.noteable @merge_request = @note.noteable
@target_url = project_merge_request_url(*note_target_url_options) @target_url = project_merge_request_url(*note_target_url_options)
mail_answer_thread(@merge_request, note_thread_options(recipient_id)) mail_answer_note_thread(@merge_request, @note, note_thread_options(recipient_id))
end end
def note_snippet_email(recipient_id, note_id) def note_snippet_email(recipient_id, note_id)
...@@ -29,7 +29,7 @@ module Emails ...@@ -29,7 +29,7 @@ module Emails
@snippet = @note.noteable @snippet = @note.noteable
@target_url = project_snippet_url(*note_target_url_options) @target_url = project_snippet_url(*note_target_url_options)
mail_answer_thread(@snippet, note_thread_options(recipient_id)) mail_answer_note_thread(@snippet, @note, note_thread_options(recipient_id))
end end
def note_personal_snippet_email(recipient_id, note_id) def note_personal_snippet_email(recipient_id, note_id)
...@@ -37,7 +37,7 @@ module Emails ...@@ -37,7 +37,7 @@ module Emails
@snippet = @note.noteable @snippet = @note.noteable
@target_url = snippet_url(@note.noteable) @target_url = snippet_url(@note.noteable)
mail_answer_thread(@snippet, note_thread_options(recipient_id)) mail_answer_note_thread(@snippet, @note, note_thread_options(recipient_id))
end end
private private
......
...@@ -156,6 +156,18 @@ class Notify < BaseMailer ...@@ -156,6 +156,18 @@ class Notify < BaseMailer
mail_thread(model, headers) mail_thread(model, headers)
end end
def mail_answer_note_thread(model, note, headers = {})
headers['Message-ID'] = message_id(note)
headers['In-Reply-To'] = message_id(note.replies_to)
headers['References'] = message_id(model)
headers['X-GitLab-Discussion-ID'] = note.discussion.id if note.part_of_discussion?
headers[:subject]&.prepend('Re: ')
mail_thread(model, headers)
end
def reply_key def reply_key
@reply_key ||= SentNotification.reply_key @reply_key ||= SentNotification.reply_key
end end
......
...@@ -360,6 +360,15 @@ class Note < ActiveRecord::Base ...@@ -360,6 +360,15 @@ class Note < ActiveRecord::Base
end end
end end
def replies_to
if part_of_discussion?
previous_note = discussion.notes.take_while { |n| n.id < id }.last
return previous_note if previous_note
end
noteable
end
def expire_etag_cache def expire_etag_cache
return unless noteable&.discussions_rendered_on_frontend? return unless noteable&.discussions_rendered_on_frontend?
......
---
title: Make mail notifications of discussion notes In-Reply-To of each other
merge_request: 14289
author:
type: changed
...@@ -756,6 +756,31 @@ describe Note do ...@@ -756,6 +756,31 @@ describe Note do
end end
end end
describe '#replies_to' do
context 'when part of a discussion' do
let(:note) { create(:discussion_note_on_issue) }
it 'returns noteable when there are not earlier notes in the discussion' do
expect(note.replies_to).to eq(note.noteable)
end
it 'returns previous note in discussion' do
reply = create(:discussion_note_on_issue, in_reply_to: note)
expect(reply.replies_to).to eq(note)
end
end
context 'when not part of a discussion' do
subject { create(:note) }
let(:note) { create(:note, in_reply_to: subject) }
it 'returns the noteable' do
expect(note.replies_to).to eq(note.noteable)
end
end
end
describe 'expiring ETag cache' do describe 'expiring ETag cache' do
let(:note) { build(:note_on_issue) } let(:note) { build(:note_on_issue) }
......
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