API routes referencing a specific merge request should use the MR `iid`

- As opposed to the `id` that was previously being used.
- This brings the API routes closer to the web interface's routes.
- This is specific to API v4.

lib/api/helpers.rb

@@ -86,12 +86,12 @@ module API
       IssuesFinder.new(current_user, project_id: user_project.id).find_by!(iid: iid)
     end
 
-    def find_project_merge_request(id)
-      MergeRequestsFinder.new(current_user, project_id: user_project.id).find(id)
+    def find_project_merge_request(iid)
+      MergeRequestsFinder.new(current_user, project_id: user_project.id).find_by!(iid: iid)
     end
 
-    def find_merge_request_with_access(id, access_level = :read_merge_request)
-      merge_request = user_project.merge_requests.find(id)
+    def find_merge_request_with_access(iid, access_level = :read_merge_request)
+      merge_request = user_project.merge_requests.find_by!(iid: iid)
       authorize! access_level, merge_request
       merge_request
     end

lib/api/merge_requests.rb

@@ -101,23 +101,23 @@ module API
 
       desc 'Delete a merge request'
       params do
-        requires :merge_request_id, type: Integer, desc: 'The ID of a merge request'
+        requires :merge_request_iid, type: Integer, desc: 'The IID of a merge request'
       end
-      delete ":id/merge_requests/:merge_request_id" do
-        merge_request = find_project_merge_request(params[:merge_request_id])
+      delete ":id/merge_requests/:merge_request_iid" do
+        merge_request = find_project_merge_request(params[:merge_request_iid])
 
         authorize!(:destroy_merge_request, merge_request)
         merge_request.destroy
       end
 
       params do
-        requires :merge_request_id, type: Integer, desc: 'The ID of a merge request'
+        requires :merge_request_iid, type: Integer, desc: 'The IID of a merge request'
       end
       desc 'Get a single merge request' do
         success Entities::MergeRequest
       end
-      get ':id/merge_requests/:merge_request_id' do
-        merge_request = find_merge_request_with_access(params[:merge_request_id])
+      get ':id/merge_requests/:merge_request_iid' do
+        merge_request = find_merge_request_with_access(params[:merge_request_iid])
 
         present merge_request, with: Entities::MergeRequest, current_user: current_user, project: user_project
       end
@@ -125,8 +125,8 @@ module API
       desc 'Get the commits of a merge request' do
         success Entities::RepoCommit
       end
-      get ':id/merge_requests/:merge_request_id/commits' do
-        merge_request = find_merge_request_with_access(params[:merge_request_id])
+      get ':id/merge_requests/:merge_request_iid/commits' do
+        merge_request = find_merge_request_with_access(params[:merge_request_iid])
         commits = ::Kaminari.paginate_array(merge_request.commits)
 
         present paginate(commits), with: Entities::RepoCommit
@@ -135,8 +135,8 @@ module API
       desc 'Show the merge request changes' do
         success Entities::MergeRequestChanges
       end
-      get ':id/merge_requests/:merge_request_id/changes' do
-        merge_request = find_merge_request_with_access(params[:merge_request_id])
+      get ':id/merge_requests/:merge_request_iid/changes' do
+        merge_request = find_merge_request_with_access(params[:merge_request_iid])
 
         present merge_request, with: Entities::MergeRequestChanges, current_user: current_user
       end
@@ -154,8 +154,8 @@ module API
                         :milestone_id, :labels, :state_event,
                         :remove_source_branch
       end
-      put ':id/merge_requests/:merge_request_id' do
-        merge_request = find_merge_request_with_access(params.delete(:merge_request_id), :update_merge_request)
+      put ':id/merge_requests/:merge_request_iid' do
+        merge_request = find_merge_request_with_access(params.delete(:merge_request_iid), :update_merge_request)
 
         mr_params = declared_params(include_missing: false)
         mr_params[:force_remove_source_branch] = mr_params.delete(:remove_source_branch) if mr_params[:remove_source_branch].present?
@@ -180,8 +180,8 @@ module API
                                                 desc: 'When true, this merge request will be merged when the pipeline succeeds'
         optional :sha, type: String, desc: 'When present, must have the HEAD SHA of the source branch'
       end
-      put ':id/merge_requests/:merge_request_id/merge' do
-        merge_request = find_project_merge_request(params[:merge_request_id])
+      put ':id/merge_requests/:merge_request_iid/merge' do
+        merge_request = find_project_merge_request(params[:merge_request_iid])
 
         # Merge request can not be merged
         # because user dont have permissions to push into target branch
@@ -216,8 +216,8 @@ module API
       desc 'Cancel merge if "Merge When Pipeline Succeeds" is enabled' do
         success Entities::MergeRequest
       end
-      post ':id/merge_requests/:merge_request_id/cancel_merge_when_pipeline_succeeds' do
-        merge_request = find_project_merge_request(params[:merge_request_id])
+      post ':id/merge_requests/:merge_request_iid/cancel_merge_when_pipeline_succeeds' do
+        merge_request = find_project_merge_request(params[:merge_request_iid])
 
         unauthorized! unless merge_request.can_cancel_merge_when_pipeline_succeeds?(current_user)
 
@@ -232,8 +232,8 @@ module API
       params do
         use :pagination
       end
-      get ':id/merge_requests/:merge_request_id/comments' do
-        merge_request = find_merge_request_with_access(params[:merge_request_id])
+      get ':id/merge_requests/:merge_request_iid/comments' do
+        merge_request = find_merge_request_with_access(params[:merge_request_iid])
         present paginate(merge_request.notes.fresh), with: Entities::MRNote
       end
 
@@ -243,8 +243,8 @@ module API
       params do
         requires :note, type: String, desc: 'The text of the comment'
       end
-      post ':id/merge_requests/:merge_request_id/comments' do
-        merge_request = find_merge_request_with_access(params[:merge_request_id], :create_note)
+      post ':id/merge_requests/:merge_request_iid/comments' do
+        merge_request = find_merge_request_with_access(params[:merge_request_iid], :create_note)
 
         opts = {
           note: params[:note],
@@ -267,8 +267,8 @@ module API
       params do
         use :pagination
       end
-      get ':id/merge_requests/:merge_request_id/closes_issues' do
-        merge_request = find_merge_request_with_access(params[:merge_request_id])
+      get ':id/merge_requests/:merge_request_iid/closes_issues' do
+        merge_request = find_merge_request_with_access(params[:merge_request_iid])
         issues = ::Kaminari.paginate_array(merge_request.closes_issues(current_user))
         present paginate(issues), with: issue_entity(user_project), current_user: current_user
       end

lib/api/v3/helpers.rb

@@ -4,6 +4,16 @@ module API
       def find_project_issue(id)
         IssuesFinder.new(current_user, project_id: user_project.id).find(id)
       end
+
+      def find_project_merge_request(id)
+        MergeRequestsFinder.new(current_user, project_id: user_project.id).find(id)
+      end
+
+      def find_merge_request_with_access(id, access_level = :read_merge_request)
+        merge_request = user_project.merge_requests.find(id)
+        authorize! access_level, merge_request
+        merge_request
+      end
     end
   end
 end

spec/requests/api/merge_requests_spec.rb

@@ -153,9 +153,9 @@ describe API::MergeRequests, api: true  do
     end
   end
 
-  describe "GET /projects/:id/merge_requests/:merge_request_id" do
+  describe "GET /projects/:id/merge_requests/:merge_request_iid" do
     it 'exposes known attributes' do
-      get api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user)
+      get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user)
 
       expect(response).to have_http_status(200)
       expect(json_response['id']).to eq(merge_request.id)
@@ -184,7 +184,7 @@ describe API::MergeRequests, api: true  do
     end
 
     it "returns merge_request" do
-      get api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user)
+      get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user)
       expect(response).to have_http_status(200)
       expect(json_response['title']).to eq(merge_request.title)
       expect(json_response['iid']).to eq(merge_request.iid)
@@ -194,25 +194,31 @@ describe API::MergeRequests, api: true  do
       expect(json_response['force_close_merge_request']).to be_falsy
     end
 
-    it "returns a 404 error if merge_request_id not found" do
+    it "returns a 404 error if merge_request_iid not found" do
       get api("/projects/#{project.id}/merge_requests/999", user)
       expect(response).to have_http_status(404)
     end
 
+    it "returns a 404 error if merge_request `id` is used instead of iid" do
+      get api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user)
+
+      expect(response).to have_http_status(404)
+    end
+
     context 'Work in Progress' do
       let!(:merge_request_wip) { create(:merge_request, author: user, assignee: user, source_project: project, target_project: project, title: "WIP: Test", created_at: base_time + 1.second) }
 
       it "returns merge_request" do
-        get api("/projects/#{project.id}/merge_requests/#{merge_request_wip.id}", user)
+        get api("/projects/#{project.id}/merge_requests/#{merge_request_wip.iid}", user)
         expect(response).to have_http_status(200)
         expect(json_response['work_in_progress']).to eq(true)
       end
     end
   end
 
-  describe 'GET /projects/:id/merge_requests/:merge_request_id/commits' do
+  describe 'GET /projects/:id/merge_requests/:merge_request_iid/commits' do
     it 'returns a 200 when merge request is valid' do
-      get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/commits", user)
+      get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/commits", user)
       commit = merge_request.commits.first
 
       expect(response.status).to eq 200
@@ -223,24 +229,36 @@ describe API::MergeRequests, api: true  do
       expect(json_response.first['title']).to eq(commit.title)
     end
 
-    it 'returns a 404 when merge_request_id not found' do
+    it 'returns a 404 when merge_request_iid not found' do
       get api("/projects/#{project.id}/merge_requests/999/commits", user)
       expect(response).to have_http_status(404)
     end
+
+    it 'returns a 404 when merge_request id is used instead of iid' do
+      get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/commits", user)
+
+      expect(response).to have_http_status(404)
+    end
   end
 
-  describe 'GET /projects/:id/merge_requests/:merge_request_id/changes' do
+  describe 'GET /projects/:id/merge_requests/:merge_request_iid/changes' do
     it 'returns the change information of the merge_request' do
-      get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/changes", user)
+      get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/changes", user)
 
       expect(response.status).to eq 200
       expect(json_response['changes'].size).to eq(merge_request.diffs.size)
     end
 
-    it 'returns a 404 when merge_request_id not found' do
+    it 'returns a 404 when merge_request_iid not found' do
       get api("/projects/#{project.id}/merge_requests/999/changes", user)
       expect(response).to have_http_status(404)
     end
+
+    it 'returns a 404 when merge_request id is used instead of iid' do
+      get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/changes", user)
+
+      expect(response).to have_http_status(404)
+    end
   end
 
   describe "POST /projects/:id/merge_requests" do
@@ -400,7 +418,7 @@ describe API::MergeRequests, api: true  do
     end
   end
 
-  describe "DELETE /projects/:id/merge_requests/:merge_request_id" do
+  describe "DELETE /projects/:id/merge_requests/:merge_request_iid" do
     context "when the user is developer" do
       let(:developer) { create(:user) }
 
@@ -409,25 +427,37 @@ describe API::MergeRequests, api: true  do
       end
 
       it "denies the deletion of the merge request" do
-        delete api("/projects/#{project.id}/merge_requests/#{merge_request.id}", developer)
+        delete api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", developer)
         expect(response).to have_http_status(403)
       end
     end
 
     context "when the user is project owner" do
       it "destroys the merge request owners can destroy" do
-        delete api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user)
+        delete api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user)
 
         expect(response).to have_http_status(204)
       end
+
+      it "returns 404 for an invalid merge request IID" do
+        delete api("/projects/#{project.id}/merge_requests/12345", user)
+
+        expect(response).to have_http_status(404)
+      end
+
+      it "returns 404 if the merge request id is used instead of iid" do
+        delete api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user)
+
+        expect(response).to have_http_status(404)
+      end
     end
   end
 
-  describe "PUT /projects/:id/merge_requests/:merge_request_id/merge" do
+  describe "PUT /projects/:id/merge_requests/:merge_request_iid/merge" do
     let(:pipeline) { create(:ci_pipeline_without_jobs) }
 
     it "returns merge_request in case of success" do
-      put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/merge", user)
+      put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user)
 
       expect(response).to have_http_status(200)
     end
@@ -436,7 +466,7 @@ describe API::MergeRequests, api: true  do
       allow_any_instance_of(MergeRequest).
         to receive(:can_be_merged?).and_return(false)
 
-      put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/merge", user)
+      put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user)
 
       expect(response).to have_http_status(406)
       expect(json_response['message']).to eq('Branch cannot be merged')
@@ -444,14 +474,14 @@ describe API::MergeRequests, api: true  do
 
     it "returns 405 if merge_request is not open" do
       merge_request.close
-      put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/merge", user)
+      put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user)
       expect(response).to have_http_status(405)
       expect(json_response['message']).to eq('405 Method Not Allowed')
     end
 
     it "returns 405 if merge_request is a work in progress" do
       merge_request.update_attribute(:title, "WIP: #{merge_request.title}")
-      put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/merge", user)
+      put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user)
       expect(response).to have_http_status(405)
       expect(json_response['message']).to eq('405 Method Not Allowed')
     end
@@ -459,7 +489,7 @@ describe API::MergeRequests, api: true  do
     it 'returns 405 if the build failed for a merge request that requires success' do
       allow_any_instance_of(MergeRequest).to receive(:mergeable_ci_state?).and_return(false)
 
-      put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/merge", user)
+      put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user)
 
       expect(response).to have_http_status(405)
       expect(json_response['message']).to eq('405 Method Not Allowed')
@@ -468,20 +498,20 @@ describe API::MergeRequests, api: true  do
     it "returns 401 if user has no permissions to merge" do
       user2 = create(:user)
       project.team << [user2, :reporter]
-      put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/merge", user2)
+      put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user2)
       expect(response).to have_http_status(401)
       expect(json_response['message']).to eq('401 Unauthorized')
     end
 
     it "returns 409 if the SHA parameter doesn't match" do
-      put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/merge", user), sha: merge_request.diff_head_sha.reverse
+      put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user), sha: merge_request.diff_head_sha.reverse
 
       expect(response).to have_http_status(409)
       expect(json_response['message']).to start_with('SHA does not match HEAD of source branch')
     end
 
     it "succeeds if the SHA parameter matches" do
-      put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/merge", user), sha: merge_request.diff_head_sha
+      put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user), sha: merge_request.diff_head_sha
 
       expect(response).to have_http_status(200)
     end
@@ -490,18 +520,30 @@ describe API::MergeRequests, api: true  do
       allow_any_instance_of(MergeRequest).to receive(:head_pipeline).and_return(pipeline)
       allow(pipeline).to receive(:active?).and_return(true)
 
-      put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/merge", user), merge_when_pipeline_succeeds: true
+      put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user), merge_when_pipeline_succeeds: true
 
       expect(response).to have_http_status(200)
       expect(json_response['title']).to eq('Test')
       expect(json_response['merge_when_pipeline_succeeds']).to eq(true)
     end
+
+    it "returns 404 for an invalid merge request IID" do
+      put api("/projects/#{project.id}/merge_requests/12345/merge", user)
+
+      expect(response).to have_http_status(404)
+    end
+
+    it "returns 404 if the merge request id is used instead of iid" do
+      put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/merge", user)
+
+      expect(response).to have_http_status(404)
+    end
   end
 
-  describe "PUT /projects/:id/merge_requests/:merge_request_id" do
+  describe "PUT /projects/:id/merge_requests/:merge_request_iid" do
     context "to close a MR" do
       it "returns merge_request" do
-        put api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user), state_event: "close"
+        put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), state_event: "close"
 
         expect(response).to have_http_status(200)
         expect(json_response['state']).to eq('closed')
@@ -509,38 +551,38 @@ describe API::MergeRequests, api: true  do
     end
 
     it "updates title and returns merge_request" do
-      put api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user), title: "New title"
+      put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), title: "New title"
       expect(response).to have_http_status(200)
       expect(json_response['title']).to eq('New title')
     end
 
     it "updates description and returns merge_request" do
-      put api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user), description: "New description"
+      put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), description: "New description"
       expect(response).to have_http_status(200)
       expect(json_response['description']).to eq('New description')
     end
 
     it "updates milestone_id and returns merge_request" do
-      put api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user), milestone_id: milestone.id
+      put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), milestone_id: milestone.id
       expect(response).to have_http_status(200)
       expect(json_response['milestone']['id']).to eq(milestone.id)
     end
 
     it "returns merge_request with renamed target_branch" do
-      put api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user), target_branch: "wiki"
+      put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), target_branch: "wiki"
       expect(response).to have_http_status(200)
       expect(json_response['target_branch']).to eq('wiki')
     end
 
     it "returns merge_request that removes the source branch" do
-      put api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user), remove_source_branch: true
+      put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), remove_source_branch: true
 
       expect(response).to have_http_status(200)
       expect(json_response['force_remove_source_branch']).to be_truthy
     end
 
     it 'allows special label names' do
-      put api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user),
+      put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user),
         title: 'new issue',
         labels: 'label, label?, label&foo, ?, &'
 
@@ -553,7 +595,7 @@ describe API::MergeRequests, api: true  do
     end
 
     it 'does not update state when title is empty' do
-      put api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user), state_event: 'close', title: nil
+      put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), state_event: 'close', title: nil
 
       merge_request.reload
       expect(response).to have_http_status(400)
@@ -561,19 +603,31 @@ describe API::MergeRequests, api: true  do
     end
 
     it 'does not update state when target_branch is empty' do
-      put api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user), state_event: 'close', target_branch: nil
+      put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), state_event: 'close', target_branch: nil
 
       merge_request.reload
       expect(response).to have_http_status(400)
       expect(merge_request.state).to eq('opened')
     end
+
+    it "returns 404 for an invalid merge request IID" do
+      put api("/projects/#{project.id}/merge_requests/12345", user), state_event: "close"
+
+      expect(response).to have_http_status(404)
+    end
+
+    it "returns 404 if the merge request id is used instead of iid" do
+      put api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user), state_event: "close"
+
+      expect(response).to have_http_status(404)
+    end
   end
 
-  describe "POST /projects/:id/merge_requests/:merge_request_id/comments" do
+  describe "POST /projects/:id/merge_requests/:merge_request_iid/comments" do
     it "returns comment" do
       original_count = merge_request.notes.size
 
-      post api("/projects/#{project.id}/merge_requests/#{merge_request.id}/comments", user), note: "My comment"
+      post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/comments", user), note: "My comment"
 
       expect(response).to have_http_status(201)
       expect(json_response['note']).to eq('My comment')
@@ -583,23 +637,29 @@ describe API::MergeRequests, api: true  do
     end
 
     it "returns 400 if note is missing" do
-      post api("/projects/#{project.id}/merge_requests/#{merge_request.id}/comments", user)
+      post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/comments", user)
       expect(response).to have_http_status(400)
     end
 
-    it "returns 404 if note is attached to non existent merge request" do
+    it "returns 404 if merge request iid is invalid" do
       post api("/projects/#{project.id}/merge_requests/404/comments", user),
         note: 'My comment'
       expect(response).to have_http_status(404)
     end
+
+    it "returns 404 if merge request id is used instead of iid" do
+      post api("/projects/#{project.id}/merge_requests/#{merge_request.id}/comments", user),
+        note: 'My comment'
+      expect(response).to have_http_status(404)
+    end
   end
 
-  describe "GET :id/merge_requests/:merge_request_id/comments" do
+  describe "GET :id/merge_requests/:merge_request_iid/comments" do
     let!(:note)  { create(:note_on_merge_request, author: user, project: project, noteable: merge_request, note: "a comment on a MR") }
     let!(:note2) { create(:note_on_merge_request, author: user, project: project, noteable: merge_request, note: "another comment on a MR") }
 
     it "returns merge_request comments ordered by created_at" do
-      get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/comments", user)
+      get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/comments", user)
 
       expect(response).to have_http_status(200)
       expect(response).to include_pagination_headers
@@ -610,20 +670,25 @@ describe API::MergeRequests, api: true  do
       expect(json_response.last['note']).to eq("another comment on a MR")
     end
 
-    it "returns a 404 error if merge_request_id not found" do
+    it "returns a 404 error if merge_request_iid is invalid" do
       get api("/projects/#{project.id}/merge_requests/999/comments", user)
       expect(response).to have_http_status(404)
     end
+
+    it "returns a 404 error if merge_request id is used instead of iid" do
+      get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/comments", user)
+      expect(response).to have_http_status(404)
+    end
   end
 
-  describe 'GET :id/merge_requests/:merge_request_id/closes_issues' do
+  describe 'GET :id/merge_requests/:merge_request_iid/closes_issues' do
     it 'returns the issue that will be closed on merge' do
       issue = create(:issue, project: project)
       mr = merge_request.tap do |mr|
         mr.update_attribute(:description, "Closes #{issue.to_reference(mr.project)}")
       end
 
-      get api("/projects/#{project.id}/merge_requests/#{mr.id}/closes_issues", user)
+      get api("/projects/#{project.id}/merge_requests/#{mr.iid}/closes_issues", user)
 
       expect(response).to have_http_status(200)
       expect(response).to include_pagination_headers
@@ -633,7 +698,7 @@ describe API::MergeRequests, api: true  do
     end
 
     it 'returns an empty array when there are no issues to be closed' do
-      get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/closes_issues", user)
+      get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/closes_issues", user)
 
       expect(response).to have_http_status(200)
       expect(response).to include_pagination_headers
@@ -647,7 +712,7 @@ describe API::MergeRequests, api: true  do
       merge_request = create(:merge_request, :simple, author: user, assignee: user, source_project: jira_project)
       merge_request.update_attribute(:description, "Closes #{issue.to_reference(jira_project)}")
 
-      get api("/projects/#{jira_project.id}/merge_requests/#{merge_request.id}/closes_issues", user)
+      get api("/projects/#{jira_project.id}/merge_requests/#{merge_request.iid}/closes_issues", user)
 
       expect(response).to have_http_status(200)
       expect(response).to include_pagination_headers
@@ -663,22 +728,34 @@ describe API::MergeRequests, api: true  do
       guest = create(:user)
       project.team << [guest, :guest]
 
-      get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/closes_issues", guest)
+      get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/closes_issues", guest)
 
       expect(response).to have_http_status(403)
     end
+
+    it "returns 404 for an invalid merge request IID" do
+      get api("/projects/#{project.id}/merge_requests/12345/closes_issues", user)
+
+      expect(response).to have_http_status(404)
     end
 
-  describe 'POST :id/merge_requests/:merge_request_id/subscribe' do
+    it "returns 404 if the merge request id is used instead of iid" do
+      get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/closes_issues", user)
+
+      expect(response).to have_http_status(404)
+    end
+  end
+
+  describe 'POST :id/merge_requests/:merge_request_iid/subscribe' do
     it 'subscribes to a merge request' do
-      post api("/projects/#{project.id}/merge_requests/#{merge_request.id}/subscribe", admin)
+      post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/subscribe", admin)
 
       expect(response).to have_http_status(201)
       expect(json_response['subscribed']).to eq(true)
     end
 
     it 'returns 304 if already subscribed' do
-      post api("/projects/#{project.id}/merge_requests/#{merge_request.id}/subscribe", user)
+      post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/subscribe", user)
 
       expect(response).to have_http_status(304)
     end
@@ -689,26 +766,32 @@ describe API::MergeRequests, api: true  do
       expect(response).to have_http_status(404)
     end
 
+    it 'returns 404 if the merge request id is used instead of iid' do
+      post api("/projects/#{project.id}/merge_requests/#{merge_request.id}/subscribe", user)
+
+      expect(response).to have_http_status(404)
+    end
+
     it 'returns 403 if user has no access to read code' do
       guest = create(:user)
       project.team << [guest, :guest]
 
-      post api("/projects/#{project.id}/merge_requests/#{merge_request.id}/subscribe", guest)
+      post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/subscribe", guest)
 
       expect(response).to have_http_status(403)
     end
   end
 
-  describe 'POST :id/merge_requests/:merge_request_id/unsubscribe' do
+  describe 'POST :id/merge_requests/:merge_request_iid/unsubscribe' do
     it 'unsubscribes from a merge request' do
-      post api("/projects/#{project.id}/merge_requests/#{merge_request.id}/unsubscribe", user)
+      post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/unsubscribe", user)
 
       expect(response).to have_http_status(201)
       expect(json_response['subscribed']).to eq(false)
     end
 
     it 'returns 304 if not subscribed' do
-      post api("/projects/#{project.id}/merge_requests/#{merge_request.id}/unsubscribe", admin)
+      post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/unsubscribe", admin)
 
       expect(response).to have_http_status(304)
     end
@@ -719,11 +802,17 @@ describe API::MergeRequests, api: true  do
       expect(response).to have_http_status(404)
     end
 
+    it 'returns 404 if the merge request id is used instead of iid' do
+      post api("/projects/#{project.id}/merge_requests/#{merge_request.id}/unsubscribe", user)
+
+      expect(response).to have_http_status(404)
+    end
+
     it 'returns 403 if user has no access to read code' do
       guest = create(:user)
       project.team << [guest, :guest]
 
-      post api("/projects/#{project.id}/merge_requests/#{merge_request.id}/unsubscribe", guest)
+      post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/unsubscribe", guest)
 
       expect(response).to have_http_status(403)
     end