Don't use original_discussion_id

10 files changed, 10 insertions, 72 deletions

diff --git a/app/models/concerns/noteable.rb b/app/models/concerns/noteable.rb
index 7900af6aaac..d378152eb56 100644
--- a/app/models/concerns/noteable.rb
+++ b/app/models/concerns/noteable.rb
@@ -3,7 +3,7 @@ module Noteable
     notes
   end
 
-  delegate :find_discussion, :find_original_discussion, to: :discussion_notes
+  delegate :find_discussion, to: :discussion_notes
 
   def discussions
     @discussions ||= discussion_notes
diff --git a/app/models/diff_discussion.rb b/app/models/diff_discussion.rb
index e349f743fdd..8acb70eb7cb 100644
--- a/app/models/diff_discussion.rb
+++ b/app/models/diff_discussion.rb
@@ -6,14 +6,6 @@ class DiffDiscussion < Discussion
 
             to: :first_note
 
-  def self.build_discussion_id(note)
-    [*super(note), *note.position.key]
-  end
-
-  def self.build_original_discussion_id(note)
-    [*Discussion.build_discussion_id(note), *note.original_position.key]
-  end
-
   def legacy_diff_discussion?
     false
   end
diff --git a/app/models/diff_note.rb b/app/models/diff_note.rb
index 38477528279..6029fc42e9c 100644
--- a/app/models/diff_note.rb
+++ b/app/models/diff_note.rb
@@ -16,9 +16,6 @@ class DiffNote < Note
 
   before_validation :set_original_position, :update_position, on: :create
   before_validation :set_line_code
-  # We need to do this again, because it's already in `Note`, but is affected by
-  # `update_position` and needs to run after that.
-  before_validation :set_discussion_id, if: :position_changed?
   after_save :keep_around_commits
 
   def discussion_class(*)
diff --git a/app/models/discussion.rb b/app/models/discussion.rb
index 27ed0480e6d..782db4044ed 100644
--- a/app/models/discussion.rb
+++ b/app/models/discussion.rb
@@ -34,24 +34,13 @@ class Discussion
     nil
   end
 
-  def self.build_discussion_id(note)
+  def self.build_discussion_id_base(note)
     noteable_id = note.noteable_id || note.commit_id
     [:discussion, note.noteable_type.try(:underscore), noteable_id]
   end
 
-  def self.original_discussion_id(note)
-    original_discussion_id = build_original_discussion_id(note)
-    if original_discussion_id
-      Digest::SHA1.hexdigest(original_discussion_id.join("-"))
-    else
-      note.discussion_id
-    end
-  end
-
-  # Optionally build a separate original discussion ID that will never change,
-  # if the main discussion ID _can_ change, like in the case of DiffDiscussion.
-  def self.build_original_discussion_id(note)
-    nil
+  def self.build_discussion_id(note)
+    [*build_discussion_id_base(note), SecureRandom.hex]
   end
 
   def initialize(notes, noteable = nil)
@@ -80,10 +69,6 @@ class Discussion
 
   alias_method :to_param, :id
 
-  def original_id
-    first_note.original_discussion_id
-  end
-
   def diff_discussion?
     false
   end
@@ -109,7 +94,7 @@ class Discussion
   end
 
   def reply_attributes
-    first_note.slice(:type, :noteable_type, :noteable_id, :commit_id)
+    first_note.slice(:type, :noteable_type, :noteable_id, :commit_id, :discussion_id)
   end
 
   private
diff --git a/app/models/individual_note_discussion.rb b/app/models/individual_note_discussion.rb
index 585b8527883..ebcf60beaf3 100644
--- a/app/models/individual_note_discussion.rb
+++ b/app/models/individual_note_discussion.rb
@@ -1,8 +1,4 @@
 class IndividualNoteDiscussion < Discussion
-  def self.build_discussion_id(note)
-    [*super(note), SecureRandom.hex]
-  end
-
   # Keep this method in sync with the `potentially_resolvable` scope on `ResolvableNote`
   def potentially_resolvable?
     false
diff --git a/app/models/legacy_diff_discussion.rb b/app/models/legacy_diff_discussion.rb
index 53fe9fbab06..36612a28ba1 100644
--- a/app/models/legacy_diff_discussion.rb
+++ b/app/models/legacy_diff_discussion.rb
@@ -1,10 +1,6 @@
 class LegacyDiffDiscussion < Discussion
   include DiscussionOnDiff
 
-  def self.build_discussion_id(note)
-    [*super(note), note.line_code]
-  end
-
   def legacy_diff_discussion?
     true
   end
diff --git a/app/models/note.rb b/app/models/note.rb
index eef868a05d6..3db1656ba57 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -52,7 +52,7 @@ class Note < ActiveRecord::Base
   validates :noteable_id, presence: true, unless: [:for_commit?, :importing?]
   validates :commit_id, presence: true, if: :for_commit?
   validates :author, presence: true
-  validates :discussion_id, :original_discussion_id, presence: true, format: { with: /\A\h{40}\z/ }
+  validates :discussion_id, presence: true, format: { with: /\A\h{40}\z/ }
 
   validate unless: [:for_commit?, :importing?, :for_personal_snippet?] do |note|
     unless note.noteable.try(:project) == note.project
@@ -84,9 +84,9 @@ class Note < ActiveRecord::Base
              project: [:project_members, { group: [:group_members] }])
   end
 
-  after_initialize :ensure_discussion_id, :ensure_original_discussion_id
+  after_initialize :ensure_discussion_id
   before_validation :nullify_blank_type, :nullify_blank_line_code
-  before_validation :set_discussion_id, :set_original_discussion_id, on: :create
+  before_validation :set_discussion_id, on: :create
   after_save :keep_around_commit, unless: :for_personal_snippet?
   after_save :expire_etag_cache
 
@@ -99,13 +99,6 @@ class Note < ActiveRecord::Base
       Discussion.build_collection(fresh, noteable)
     end
 
-    def find_original_discussion(discussion_id)
-      note = find_by(original_discussion_id: discussion_id)
-      return unless note
-
-      note.to_discussion
-    end
-
     def find_discussion(discussion_id)
       notes = where(discussion_id: discussion_id).fresh.to_a
       return if notes.empty?
@@ -309,20 +302,6 @@ class Note < ActiveRecord::Base
     self.discussion_id ||= discussion_class.discussion_id(self)
   end
 
-  def ensure_original_discussion_id
-    return unless self.persisted?
-    # Needed in case the SELECT statement doesn't ask for `original_discussion_id`
-    return unless self.has_attribute?(:original_discussion_id)
-    return if self.original_discussion_id
-
-    set_original_discussion_id
-    update_column(:original_discussion_id, self.original_discussion_id)
-  end
-
-  def set_original_discussion_id
-    self.original_discussion_id = discussion_class.original_discussion_id(self)
-  end
-
   def expire_etag_cache
     return unless for_issue?
 
diff --git a/app/models/out_of_context_discussion.rb b/app/models/out_of_context_discussion.rb
index 0019064e25c..ecbfd97699f 100644
--- a/app/models/out_of_context_discussion.rb
+++ b/app/models/out_of_context_discussion.rb
@@ -2,7 +2,7 @@ class OutOfContextDiscussion < Discussion
   # To make sure all out-of-context notes are displayed in one discussion,
   # we override the discussion ID to be a newly generated but consistent ID.
   def self.override_discussion_id(note)
-    discussion_id(note)
+    Digest::SHA1.hexdigest(build_discussion_id_base(note).join("-"))
   end
 
   # Keep this method in sync with the `potentially_resolvable` scope on `ResolvableNote`
diff --git a/app/models/sent_notification.rb b/app/models/sent_notification.rb
index 81fc2ddac77..7d65b2b7993 100644
--- a/app/models/sent_notification.rb
+++ b/app/models/sent_notification.rb
@@ -46,7 +46,7 @@ class SentNotification < ActiveRecord::Base
     end
 
     def record_note(note, recipient_id, reply_key = self.reply_key, attrs = {})
-      attrs[:in_reply_to_discussion_id] = note.original_discussion_id
+      attrs[:in_reply_to_discussion_id] = note.discussion_id
 
       record(note.noteable, recipient_id, reply_key, attrs)
     end
diff --git a/app/models/simple_discussion.rb b/app/models/simple_discussion.rb
index a5ef5c565e7..41c679daf2d 100644
--- a/app/models/simple_discussion.rb
+++ b/app/models/simple_discussion.rb
@@ -1,9 +1,2 @@
 class SimpleDiscussion < Discussion
-  def self.build_discussion_id(note)
-    [*super(note), SecureRandom.hex]
-  end
-
-  def reply_attributes
-    super.merge(discussion_id: self.id)
-  end
 end