Make diff_refs_match_commit validation reusable

Move it to DiffPositionableNote concern which will be re-used
in EE in DraftNote model.

4 files changed, 64 insertions, 7 deletions

diff --git a/app/models/concerns/diff_positionable_note.rb b/app/models/concerns/diff_positionable_note.rb
index 2d09eff0111..195d9e107c5 100644
--- a/app/models/concerns/diff_positionable_note.rb
+++ b/app/models/concerns/diff_positionable_note.rb
@@ -10,6 +10,8 @@ module DiffPositionableNote
     serialize :original_position, Gitlab::Diff::Position # rubocop:disable Cop/ActiveRecordSerialize
     serialize :position, Gitlab::Diff::Position # rubocop:disable Cop/ActiveRecordSerialize
     serialize :change_position, Gitlab::Diff::Position # rubocop:disable Cop/ActiveRecordSerialize
+
+    validate :diff_refs_match_commit, if: :for_commit?
   end
 
   %i(original_position position change_position).each do |meth|
@@ -71,4 +73,10 @@ module DiffPositionableNote
       self.position = result[:position]
     end
   end
+
+  def diff_refs_match_commit
+    return if self.original_position.diff_refs == commit&.diff_refs
+
+    errors.add(:commit_id, 'does not match the diff refs')
+  end
 end
diff --git a/app/models/diff_note.rb b/app/models/diff_note.rb
index f75c32633b1..861185dc222 100644
--- a/app/models/diff_note.rb
+++ b/app/models/diff_note.rb
@@ -20,7 +20,6 @@ class DiffNote < Note
   validates :noteable_type, inclusion: { in: -> (_note) { noteable_types } }
   validate :positions_complete
   validate :verify_supported
-  validate :diff_refs_match_commit, if: :for_commit?
 
   before_validation :set_line_code, if: :on_text?
   after_save :keep_around_commits
@@ -154,12 +153,6 @@ class DiffNote < Note
     errors.add(:position, "is invalid")
   end
 
-  def diff_refs_match_commit
-    return if self.original_position.diff_refs == self.commit.diff_refs
-
-    errors.add(:commit_id, 'does not match the diff refs')
-  end
-
   def keep_around_commits
     shas = [
       self.original_position.base_sha,
diff --git a/spec/models/diff_note_spec.rb b/spec/models/diff_note_spec.rb
index d9e1fe4b165..8d7dafc523d 100644
--- a/spec/models/diff_note_spec.rb
+++ b/spec/models/diff_note_spec.rb
@@ -34,6 +34,10 @@ describe DiffNote do
 
   subject { create(:diff_note_on_merge_request, project: project, position: position, noteable: merge_request) }
 
+  describe 'validations' do
+    it_behaves_like 'a valid diff positionable note', :diff_note_on_commit
+  end
+
   describe "#position=" do
     context "when provided a string" do
       it "sets the position" do
diff --git a/spec/support/shared_examples/models/diff_positionable_note_shared_examples.rb b/spec/support/shared_examples/models/diff_positionable_note_shared_examples.rb
new file mode 100644
index 00000000000..8b298c5c974
--- /dev/null
+++ b/spec/support/shared_examples/models/diff_positionable_note_shared_examples.rb
@@ -0,0 +1,52 @@
+# frozen_string_literal: true
+
+shared_examples_for 'a valid diff positionable note' do |factory_on_commit|
+  context 'for commit' do
+    let(:project) { create(:project, :repository) }
+    let(:commit) { project.commit(sample_commit.id) }
+    let(:commit_id) { commit.id }
+    let(:diff_refs) { commit.diff_refs }
+
+    let(:position) do
+      Gitlab::Diff::Position.new(
+        old_path: "files/ruby/popen.rb",
+        new_path: "files/ruby/popen.rb",
+        old_line: nil,
+        new_line: 14,
+        diff_refs: diff_refs
+      )
+    end
+
+    subject { build(factory_on_commit, commit_id: commit_id, position: position) }
+
+    context 'position diff refs matches commit diff refs' do
+      it 'is valid' do
+        expect(subject).to be_valid
+        expect(subject.errors).not_to have_key(:commit_id)
+      end
+    end
+
+    context 'position diff refs does not match commit diff refs' do
+      let(:diff_refs) do
+        Gitlab::Diff::DiffRefs.new(
+          base_sha: "not_existing_sha",
+          head_sha: "existing_sha"
+        )
+      end
+
+      it 'is invalid' do
+        expect(subject).to be_invalid
+        expect(subject.errors).to have_key(:commit_id)
+      end
+    end
+
+    context 'commit does not exist' do
+      let(:commit_id) { 'non-existing' }
+
+      it 'is invalid' do
+        expect(subject).to be_invalid
+        expect(subject.errors).to have_key(:commit_id)
+      end
+    end
+  end
+end