Merge branch 'merge-ref-diffs-position-updates' into 'master'

Update diff discussion positions on load

See merge request gitlab-org/gitlab!34148

app/controllers/projects/merge_requests/diffs_controller.rb

@@ -8,6 +8,7 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic
   before_action :commit
   before_action :define_diff_vars
   before_action :define_diff_comment_vars, except: [:diffs_batch, :diffs_metadata]
+  before_action :update_diff_discussion_positions!
 
   around_action :allow_gitaly_ref_name_caching
 
@@ -171,4 +172,12 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic
 
     @notes.concat(draft_notes)
   end
+
+  def update_diff_discussion_positions!
+    return unless Feature.enabled?(:merge_ref_head_comments, @merge_request.target_project, default_enabled: true)
+    return unless Feature.enabled?(:merge_red_head_comments_position_on_demand, @merge_request.target_project, default_enabled: true)
+    return if @merge_request.has_any_diff_note_positions?
+
+    Discussions::CaptureDiffNotePositionsService.new(@merge_request).execute
+  end
 end

app/models/concerns/noteable.rb

@@ -67,6 +67,10 @@ module Noteable
     false
   end
 
+  def has_any_diff_note_positions?
+    notes.any? && DiffNotePosition.where(note: notes).exists?
+  end
+
   def discussion_notes
     notes
   end

changelogs/unreleased/merge-ref-diffs-position-updates.yml

+---
+title: Update diff discussion positions on demand
+merge_request: 34148
+author:
+type: added

spec/controllers/projects/merge_requests/diffs_controller_spec.rb

@@ -91,6 +91,17 @@ RSpec.describe Projects::MergeRequests::DiffsController do
     end
   end
 
+  shared_examples "diff note on-demand position creation" do
+    it "updates diff discussion positions" do
+      service = double("service")
+
+      expect(Discussions::CaptureDiffNotePositionsService).to receive(:new).with(merge_request).and_return(service)
+      expect(service).to receive(:execute)
+
+      go
+    end
+  end
+
   let(:project) { create(:project, :repository) }
   let(:user) { create(:user) }
   let(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: project) }
@@ -146,6 +157,7 @@ RSpec.describe Projects::MergeRequests::DiffsController do
 
     it_behaves_like 'persisted preferred diff view cookie'
     it_behaves_like 'cached diff collection'
+    it_behaves_like 'diff note on-demand position creation'
   end
 
   describe 'GET diffs_metadata' do

spec/models/concerns/noteable_spec.rb

@@ -262,4 +262,44 @@ describe Noteable do
       end
     end
   end
+
+  describe "#has_any_diff_note_positions?" do
+    let(:source_branch) { "compare-with-merge-head-source" }
+    let(:target_branch) { "compare-with-merge-head-target" }
+    let(:merge_request) { create(:merge_request, source_branch: source_branch, target_branch: target_branch) }
+
+    let!(:note) do
+      path = "files/markdown/ruby-style-guide.md"
+
+      position = Gitlab::Diff::Position.new(
+        old_path: path,
+        new_path: path,
+        new_line: 508,
+        diff_refs: merge_request.diff_refs
+      )
+
+      create(:diff_note_on_merge_request, project: merge_request.project, position: position, noteable: merge_request)
+    end
+
+    before do
+      MergeRequests::MergeToRefService.new(merge_request.project, merge_request.author).execute(merge_request)
+      Discussions::CaptureDiffNotePositionsService.new(merge_request).execute
+    end
+
+    it "returns true when it has diff note positions" do
+      expect(merge_request.has_any_diff_note_positions?).to be(true)
+    end
+
+    it "returns false when it has notes but no diff note positions" do
+      DiffNotePosition.where(note: note).find_each(&:delete)
+
+      expect(merge_request.has_any_diff_note_positions?).to be(false)
+    end
+
+    it "returns false when it has no notes" do
+      merge_request.notes.find_each(&:destroy)
+
+      expect(merge_request.has_any_diff_note_positions?).to be(false)
+    end
+  end
 end