Merge branch 'preserve-note_type-and-position' into 'master'

Preserve note_type and position for notes from emails

Closes #23208

See merge request !7010

5 files changed, 17 insertions, 16 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index de24e08c52f..73dc323e02c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -45,6 +45,7 @@ Please view this file on the master branch, on stable branches it's out of date.
   - Speed-up group milestones show page
   - Fix inconsistent options dropdown caret on mobile viewports (ClemMakesApps)
   - Extract project#update_merge_requests and SystemHooks to its own worker from GitPushService
+  - Fix discussion thread from emails for merge requests. !7010
   - Don't include archived projects when creating group milestones. !4940 (Jeroen Jacobs)
   - Add tag shortcut from the Commit page. !6543
   - Keep refs for each deployment
diff --git a/lib/gitlab/email/handler/create_note_handler.rb b/lib/gitlab/email/handler/create_note_handler.rb
index 06dae31cc27..447c7a6a6b9 100644
--- a/lib/gitlab/email/handler/create_note_handler.rb
+++ b/lib/gitlab/email/handler/create_note_handler.rb
@@ -46,7 +46,9 @@ module Gitlab
             noteable_type:  sent_notification.noteable_type,
             noteable_id:    sent_notification.noteable_id,
             commit_id:      sent_notification.commit_id,
-            line_code:      sent_notification.line_code
+            line_code:      sent_notification.line_code,
+            position:       sent_notification.position,
+            type:           sent_notification.note_type
           ).execute
         end
       end
diff --git a/spec/fixtures/emails/commands_in_reply.eml b/spec/fixtures/emails/commands_in_reply.eml
index 06bf60ab734..712f6f797b4 100644
--- a/spec/fixtures/emails/commands_in_reply.eml
+++ b/spec/fixtures/emails/commands_in_reply.eml
@@ -23,8 +23,6 @@ Cool!
 
 /close
 /todo
-/due tomorrow
-
 
 On Sun, Jun 9, 2013 at 1:39 PM, eviltrout via Discourse Meta
 <reply+59d8df8370b7e95c5a49fbf86aeb2c93@appmail.adventuretime.ooo> wrote:
diff --git a/spec/fixtures/emails/commands_only_reply.eml b/spec/fixtures/emails/commands_only_reply.eml
index aed64224b06..2d2e2f94290 100644
--- a/spec/fixtures/emails/commands_only_reply.eml
+++ b/spec/fixtures/emails/commands_only_reply.eml
@@ -21,8 +21,6 @@ X-Scanned-By: MIMEDefang 2.69 on IPv6:2001:470:1d:165::1
 
 /close
 /todo
-/due tomorrow
-
 
 On Sun, Jun 9, 2013 at 1:39 PM, eviltrout via Discourse Meta
 <reply+59d8df8370b7e95c5a49fbf86aeb2c93@appmail.adventuretime.ooo> wrote:
diff --git a/spec/lib/gitlab/email/handler/create_note_handler_spec.rb b/spec/lib/gitlab/email/handler/create_note_handler_spec.rb
index 4909fed6b77..48660d1dd1b 100644
--- a/spec/lib/gitlab/email/handler/create_note_handler_spec.rb
+++ b/spec/lib/gitlab/email/handler/create_note_handler_spec.rb
@@ -12,10 +12,13 @@ describe Gitlab::Email::Handler::CreateNoteHandler, lib: true do
 
   let(:email_raw) { fixture_file('emails/valid_reply.eml') }
   let(:project)   { create(:project, :public) }
-  let(:noteable)  { create(:issue, project: project) }
   let(:user)      { create(:user) }
+  let(:note)      { create(:diff_note_on_merge_request, project: project) }
+  let(:noteable)  { note.noteable }
 
-  let!(:sent_notification) { SentNotification.record(noteable, user.id, mail_key) }
+  let!(:sent_notification) do
+    SentNotification.record_note(note, user.id, mail_key)
+  end
 
   context "when the recipient address doesn't include a mail key" do
     let(:email_raw) { fixture_file('emails/valid_reply.eml').gsub(mail_key, "") }
@@ -82,7 +85,6 @@ describe Gitlab::Email::Handler::CreateNoteHandler, lib: true do
           expect { receiver.execute }.to change { noteable.notes.count }.by(1)
 
           expect(noteable.reload).to be_closed
-          expect(noteable.due_date).to eq(Date.tomorrow)
           expect(TodoService.new.todo_exist?(noteable, user)).to be_truthy
         end
       end
@@ -100,7 +102,6 @@ describe Gitlab::Email::Handler::CreateNoteHandler, lib: true do
         expect { receiver.execute }.to change { noteable.notes.count }.by(1)
 
         expect(noteable.reload).to be_open
-        expect(noteable.due_date).to be_nil
         expect(TodoService.new.todo_exist?(noteable, user)).to be_falsy
       end
     end
@@ -117,7 +118,6 @@ describe Gitlab::Email::Handler::CreateNoteHandler, lib: true do
         expect { receiver.execute }.to change { noteable.notes.count }.by(2)
 
         expect(noteable.reload).to be_closed
-        expect(noteable.due_date).to eq(Date.tomorrow)
         expect(TodoService.new.todo_exist?(noteable, user)).to be_truthy
       end
     end
@@ -138,10 +138,11 @@ describe Gitlab::Email::Handler::CreateNoteHandler, lib: true do
 
     it "creates a comment" do
       expect { receiver.execute }.to change { noteable.notes.count }.by(1)
-      note = noteable.notes.last
+      new_note = noteable.notes.last
 
-      expect(note.author).to eq(sent_notification.recipient)
-      expect(note.note).to include("I could not disagree more.")
+      expect(new_note.author).to eq(sent_notification.recipient)
+      expect(new_note.position).to eq(note.position)
+      expect(new_note.note).to include("I could not disagree more.")
     end
 
     it "adds all attachments" do
@@ -160,10 +161,11 @@ describe Gitlab::Email::Handler::CreateNoteHandler, lib: true do
       shared_examples 'an email that contains a mail key' do |header|
         it "fetches the mail key from the #{header} header and creates a comment" do
           expect { receiver.execute }.to change { noteable.notes.count }.by(1)
-          note = noteable.notes.last
+          new_note = noteable.notes.last
 
-          expect(note.author).to eq(sent_notification.recipient)
-          expect(note.note).to include('I could not disagree more.')
+          expect(new_note.author).to eq(sent_notification.recipient)
+          expect(new_note.position).to eq(note.position)
+          expect(new_note.note).to include('I could not disagree more.')
         end
       end