1 files changed, 47 insertions, 37 deletions

diff --git a/app/models/sent_notification.rb b/app/models/sent_notification.rb
index f4bcb49b34d..0ae5864615a 100644
--- a/app/models/sent_notification.rb
+++ b/app/models/sent_notification.rb
@@ -5,10 +5,11 @@ class SentNotification < ActiveRecord::Base
   belongs_to :noteable, polymorphic: true
   belongs_to :recipient, class_name: "User"
 
-  validates :project, :recipient, :reply_key, presence: true
-  validates :reply_key, uniqueness: true
+  validates :project, :recipient, presence: true
+  validates :reply_key, presence: true, uniqueness: true
   validates :noteable_id, presence: true, unless: :for_commit?
   validates :commit_id, presence: true, if: :for_commit?
+  validates :in_reply_to_discussion_id, format: { with: /\A\h{40}\z/, allow_nil: true }
   validate :note_valid
 
   after_save :keep_around_commit
@@ -22,9 +23,7 @@ class SentNotification < ActiveRecord::Base
       find_by(reply_key: reply_key)
     end
 
-    def record(noteable, recipient_id, reply_key, attrs = {})
-      return unless reply_key
-
+    def record(noteable, recipient_id, reply_key = self.reply_key, attrs = {})
       noteable_id = nil
       commit_id = nil
       if noteable.is_a?(Commit)
@@ -34,23 +33,20 @@ class SentNotification < ActiveRecord::Base
       end
 
       attrs.reverse_merge!(
-        project:        noteable.project,
-        noteable_type:  noteable.class.name,
-        noteable_id:    noteable_id,
-        commit_id:      commit_id,
-        recipient_id:   recipient_id,
-        reply_key:      reply_key
+        project: noteable.project,
+        recipient_id: recipient_id,
+        reply_key: reply_key,
+
+        noteable_type: noteable.class.name,
+        noteable_id: noteable_id,
+        commit_id: commit_id
       )
 
       create(attrs)
     end
 
-    def record_note(note, recipient_id, reply_key, attrs = {})
-      if note.diff_note?
-        attrs[:note_type] = note.type
-
-        attrs.merge!(note.diff_attributes)
-      end
+    def record_note(note, recipient_id, reply_key = self.reply_key, attrs = {})
+      attrs[:in_reply_to_discussion_id] = note.discussion_id
 
       record(note.noteable, recipient_id, reply_key, attrs)
     end
@@ -89,31 +85,45 @@ class SentNotification < ActiveRecord::Base
     self.reply_key
   end
 
-  def note_attributes
-    {
-      project:        self.project,
-      author:         self.recipient,
-      type:           self.note_type,
-      noteable_type:  self.noteable_type,
-      noteable_id:    self.noteable_id,
-      commit_id:      self.commit_id,
-      line_code:      self.line_code,
-      position:       self.position.to_json
-    }
-  end
-
-  def create_note(note)
-    Notes::CreateService.new(
-      self.project,
-      self.recipient,
-      self.note_attributes.merge(note: note)
-    ).execute
+  def create_reply(message, dryrun: false)
+    klass = dryrun ? Notes::BuildService : Notes::CreateService
+    klass.new(self.project, self.recipient, reply_params.merge(note: message)).execute
   end
 
   private
 
+  def reply_params
+    attrs = {
+      noteable_type: self.noteable_type,
+      noteable_id: self.noteable_id,
+      commit_id: self.commit_id
+    }
+
+    if self.in_reply_to_discussion_id.present?
+      attrs[:in_reply_to_discussion_id] = self.in_reply_to_discussion_id
+    else
+      # Remove in GitLab 10.0, when we will not support replying to SentNotifications
+      # that don't have `in_reply_to_discussion_id` anymore.
+      attrs.merge!(
+        type: self.note_type,
+
+        # LegacyDiffNote
+        line_code: self.line_code,
+
+        # DiffNote
+        position: self.position.to_json
+      )
+    end
+
+    attrs
+  end
+
   def note_valid
-    Note.new(note_attributes.merge(note: "Test")).valid?
+    note = create_reply('Test', dryrun: true)
+
+    unless note.valid?
+      self.errors.add(:base, "Note parameters are invalid: #{note.errors.full_messages.to_sentence}")
+    end
   end
 
   def keep_around_commit