Merge branch...

Merge branch '27946-merge-request-discussions-api-doesn-t-reject-an-error-input-in-some-case' into 'master'

Resolve "Merge request discussions API doesn't reject an error input in some case"

Closes #27946

See merge request gitlab-org/gitlab!21936

changelogs/unreleased/27946-merge-request-discussions-api-doesn-t-reject-an-error-input-in-some.yml

+---
+title: Resolve "Merge request discussions API doesn't reject an error input in some
+  case"
+merge_request: 21936
+author:
+type: fixed

spec/models/diff_note_spec.rb

@@ -5,11 +5,11 @@ require 'spec_helper'
 describe DiffNote do
   include RepoHelpers
 
-  let!(:merge_request) { create(:merge_request) }
-  let(:project) { merge_request.project }
-  let(:commit) { project.commit(sample_commit.id) }
+  let_it_be(:merge_request) { create(:merge_request) }
+  let_it_be(:project) { merge_request.project }
+  let_it_be(:commit) { project.commit(sample_commit.id) }
 
-  let(:path) { "files/ruby/popen.rb" }
+  let_it_be(:path) { "files/ruby/popen.rb" }
 
   let(:diff_refs) { merge_request.diff_refs }
   let!(:position) do

spec/requests/api/discussions_spec.rb

@@ -49,6 +49,18 @@ describe API::Discussions do
     it_behaves_like 'discussions API', 'projects', 'merge_requests', 'iid', can_reply_to_individual_notes: true
     it_behaves_like 'diff discussions API', 'projects', 'merge_requests', 'iid'
     it_behaves_like 'resolvable discussions API', 'projects', 'merge_requests', 'iid'
+
+    context "when position is for a previous commit on the merge request" do
+      it "returns a 400 bad request error because the line_code is old" do
+        # SHA taken from an earlier commit listed in spec/factories/merge_requests.rb
+        position = diff_note.position.to_h.merge(new_line: 'c1acaa58bbcbc3eafe538cb8274ba387047b69f8')
+
+        post api("/projects/#{project.id}/merge_requests/#{noteable['iid']}/discussions", user),
+          params: { body: 'hi!', position: position }
+
+        expect(response).to have_gitlab_http_status(400)
+      end
+    end
   end
 
   context 'when noteable is a Commit' do

spec/support/shared_examples/requests/api/diff_discussions.rb

@@ -38,13 +38,24 @@ shared_examples 'diff discussions API' do |parent_type, noteable_type, id_name|
       expect(json_response['notes'].first['position']).to eq(position.stringify_keys)
     end
 
-    it "returns a 400 bad request error when position is invalid" do
-      position = diff_note.position.to_h.merge(new_line: '100000')
+    context "when position is invalid" do
+      it "returns a 400 bad request error when position is not plausible" do
+        position = diff_note.position.to_h.merge(new_line: '100000')
 
-      post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/discussions", user),
-        params: { body: 'hi!', position: position }
+        post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/discussions", user),
+          params: { body: 'hi!', position: position }
+
+        expect(response).to have_gitlab_http_status(400)
+      end
+
+      it "returns a 400 bad request error when the position is not valid for this discussion" do
+        position = diff_note.position.to_h.merge(new_line: '588440f66559714280628a4f9799f0c4eb880a4a')
+
+        post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/discussions", user),
+          params: { body: 'hi!', position: position }
 
-      expect(response).to have_gitlab_http_status(400)
+        expect(response).to have_gitlab_http_status(400)
+      end
     end
   end