Commit f55aaca5 authored by Toon Claes's avatar Toon Claes

Make discussion mail References all notes in the discussion

When a note is part of a discussion, the email sent out will be
`In-Reply-To` the previous note in that discussion. It also
`References` all the previous notes in that discussion, and the
original issue.

Closes gitlab-org/gitlab-ce#36054.
parent 2acf3a56
...@@ -119,8 +119,8 @@ class Notify < BaseMailer ...@@ -119,8 +119,8 @@ class Notify < BaseMailer
headers['Reply-To'] = address headers['Reply-To'] = address
fallback_reply_message_id = "<reply-#{reply_key}@#{Gitlab.config.gitlab.host}>".freeze fallback_reply_message_id = "<reply-#{reply_key}@#{Gitlab.config.gitlab.host}>".freeze
headers['References'] ||= '' headers['References'] ||= []
headers['References'] << ' ' << fallback_reply_message_id headers['References'] << fallback_reply_message_id
@reply_by_email = true @reply_by_email = true
end end
...@@ -158,8 +158,8 @@ class Notify < BaseMailer ...@@ -158,8 +158,8 @@ class Notify < BaseMailer
def mail_answer_note_thread(model, note, headers = {}) def mail_answer_note_thread(model, note, headers = {})
headers['Message-ID'] = message_id(note) headers['Message-ID'] = message_id(note)
headers['In-Reply-To'] = message_id(note.replies_to) headers['In-Reply-To'] = message_id(note.references.last)
headers['References'] = message_id(model) headers['References'] = note.references.map { |ref| message_id(ref) }
headers['X-GitLab-Discussion-ID'] = note.discussion.id if note.part_of_discussion? headers['X-GitLab-Discussion-ID'] = note.discussion.id if note.part_of_discussion?
......
...@@ -360,13 +360,14 @@ class Note < ActiveRecord::Base ...@@ -360,13 +360,14 @@ class Note < ActiveRecord::Base
end end
end end
def replies_to def references
refs = [noteable]
if part_of_discussion? if part_of_discussion?
previous_note = discussion.notes.take_while { |n| n.id < id }.last refs += discussion.notes.take_while { |n| n.id < id }
return previous_note if previous_note
end end
noteable refs
end end
def expire_etag_cache def expire_etag_cache
......
...@@ -342,6 +342,46 @@ describe Notify do ...@@ -342,6 +342,46 @@ describe Notify do
end end
end end
context 'for issue notes' do
let(:host) { Gitlab.config.gitlab.host }
context 'in discussion' do
set(:first_note) { create(:discussion_note_on_issue) }
set(:second_note) { create(:discussion_note_on_issue, in_reply_to: first_note) }
set(:third_note) { create(:discussion_note_on_issue, in_reply_to: second_note) }
subject { described_class.note_issue_email(recipient.id, third_note.id) }
it 'has In-Reply-To header pointing to previous note in discussion' do
expect(subject.header['In-Reply-To'].message_ids).to eq(["note_#{second_note.id}@#{host}"])
end
it 'has References header including the notes and issue of the discussion' do
expect(subject.header['References'].message_ids).to include("issue_#{first_note.noteable.id}@#{host}",
"note_#{first_note.id}@#{host}",
"note_#{second_note.id}@#{host}")
end
it 'has X-GitLab-Discussion-ID header' do
expect(subject.header['X-GitLab-Discussion-ID'].value).to eq(third_note.discussion.id)
end
end
context 'individual issue comments' do
set(:note) { create(:note_on_issue) }
subject { described_class.note_issue_email(recipient.id, note.id) }
it 'has In-Reply-To header pointing to the issue' do
expect(subject.header['In-Reply-To'].message_ids).to eq(["issue_#{note.noteable.id}@#{host}"])
end
it 'has References header including the notes and issue of the discussion' do
expect(subject.header['References'].message_ids).to include("issue_#{note.noteable.id}@#{host}")
end
end
end
context 'for snippet notes' do context 'for snippet notes' do
let(:project_snippet) { create(:project_snippet, project: project) } let(:project_snippet) { create(:project_snippet, project: project) }
let(:project_snippet_note) { create(:note_on_project_snippet, project: project, noteable: project_snippet) } let(:project_snippet_note) { create(:note_on_project_snippet, project: project, noteable: project_snippet) }
......
...@@ -756,18 +756,15 @@ describe Note do ...@@ -756,18 +756,15 @@ describe Note do
end end
end end
describe '#replies_to' do describe '#references' do
context 'when part of a discussion' do context 'when part of a discussion' do
let(:note) { create(:discussion_note_on_issue) } it 'references all earlier notes in the discussion' do
first_note = create(:discussion_note_on_issue)
second_note = create(:discussion_note_on_issue, in_reply_to: first_note)
third_note = create(:discussion_note_on_issue, in_reply_to: second_note)
create(:discussion_note_on_issue, in_reply_to: third_note)
it 'returns noteable when there are not earlier notes in the discussion' do expect(third_note.references).to eq([first_note.noteable, first_note, second_note])
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
end end
...@@ -776,7 +773,7 @@ describe Note do ...@@ -776,7 +773,7 @@ describe Note do
let(:note) { create(:note, in_reply_to: subject) } let(:note) { create(:note, in_reply_to: subject) }
it 'returns the noteable' do it 'returns the noteable' do
expect(note.replies_to).to eq(note.noteable) expect(note.references).to eq([note.noteable])
end end
end end
end end
......
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