Update diff discussion position per discussion instead of per notedm-update-discussion-diff-position

3 files changed, 23 insertions, 18 deletions

diff --git a/app/models/diff_discussion.rb b/app/models/diff_discussion.rb
index 800574d8be0..07c4846e2ac 100644
--- a/app/models/diff_discussion.rb
+++ b/app/models/diff_discussion.rb
@@ -10,6 +10,7 @@ class DiffDiscussion < Discussion
 
   delegate  :position,
             :original_position,
+            :change_position,
 
             to: :first_note
 
diff --git a/app/models/diff_note.rb b/app/models/diff_note.rb
index 2a4cff37566..6fbc6a9f58b 100644
--- a/app/models/diff_note.rb
+++ b/app/models/diff_note.rb
@@ -95,13 +95,21 @@ class DiffNote < Note
 
     return if active?
 
-    Notes::DiffPositionUpdateService.new(
-      self.project,
-      nil,
+    tracer = Gitlab::Diff::PositionTracer.new(
+      project: self.project,
       old_diff_refs: self.position.diff_refs,
-      new_diff_refs: noteable.diff_refs,
+      new_diff_refs: self.noteable.diff_refs,
       paths: self.position.paths
-    ).execute(self)
+    )
+
+    result = tracer.trace(self.position)
+    return unless result
+
+    if result[:outdated]
+      self.change_position = result[:position]
+    else
+      self.position = result[:position]
+    end
   end
 
   def verify_supported
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 356af776b8d..1c4bb119bbe 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -421,7 +421,7 @@ class MergeRequest < ActiveRecord::Base
     MergeRequests::MergeRequestDiffCacheService.new.execute(self)
     new_diff_refs = self.diff_refs
 
-    update_diff_notes_positions(
+    update_diff_discussion_positions(
       old_diff_refs: old_diff_refs,
       new_diff_refs: new_diff_refs,
       current_user: current_user
@@ -853,19 +853,18 @@ class MergeRequest < ActiveRecord::Base
     diff_refs && diff_refs.complete?
   end
 
-  def update_diff_notes_positions(old_diff_refs:, new_diff_refs:, current_user: nil)
+  def update_diff_discussion_positions(old_diff_refs:, new_diff_refs:, current_user: nil)
     return unless has_complete_diff_refs?
     return if new_diff_refs == old_diff_refs
 
-    active_diff_notes = self.notes.new_diff_notes.select do |note|
-      note.active?(old_diff_refs)
+    active_diff_discussions = self.notes.new_diff_notes.discussions.select do |discussion|
+      discussion.active?(old_diff_refs)
     end
+    return if active_diff_discussions.empty?
 
-    return if active_diff_notes.empty?
+    paths = active_diff_discussions.flat_map { |n| n.diff_file.paths }.uniq
 
-    paths = active_diff_notes.flat_map { |n| n.diff_file.paths }.uniq
-
-    service = Notes::DiffPositionUpdateService.new(
+    service = Discussions::UpdateDiffPositionService.new(
       self.project,
       current_user,
       old_diff_refs: old_diff_refs,
@@ -873,11 +872,8 @@ class MergeRequest < ActiveRecord::Base
       paths: paths
     )
 
-    transaction do
-      active_diff_notes.each do |note|
-        service.execute(note)
-        Gitlab::Timeless.timeless(note, &:save)
-      end
+    active_diff_discussions.each do |discussion|
+      service.execute(discussion)
     end
   end