Merge branch '14139-sha-parameter-in-accept-merge-request-api' into 'master'

Resolve "SHA parameter in accept merge request API"

Add a `sha` parameter to the MR merge API, which must match the source SHA for the branch to be merged.

Also add the same parameter to the UI:

![MR_SHA](/uploads/616da728695dc19fa7ef7ef6a016ff81/MR_SHA.gif)

@DouweM and I discussed adding some smart feature to that, like updating the source SHA on navigating to the diff tab, but for now it will just require a refresh :smiley:

Closes #14139.

See merge request !4414

2 files changed, 99 insertions, 0 deletions

diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb
index 4f621a43d7e..8499bf07e9f 100644
--- a/spec/controllers/projects/merge_requests_controller_spec.rb
+++ b/spec/controllers/projects/merge_requests_controller_spec.rb
@@ -185,6 +185,92 @@ describe Projects::MergeRequestsController do
     end
   end
 
+  describe 'POST #merge' do
+    let(:base_params) do
+      {
+        namespace_id: project.namespace.path,
+        project_id: project.path,
+        id: merge_request.iid,
+        format: 'raw'
+      }
+    end
+
+    context 'when the user does not have access' do
+      before do
+        project.team.truncate
+        project.team << [user, :reporter]
+        post :merge, base_params
+      end
+
+      it 'returns not found' do
+        expect(response).to be_not_found
+      end
+    end
+
+    context 'when the merge request is not mergeable' do
+      before do
+        merge_request.update_attributes(title: "WIP: #{merge_request.title}")
+
+        post :merge, base_params
+      end
+
+      it 'returns :failed' do
+        expect(assigns(:status)).to eq(:failed)
+      end
+    end
+
+    context 'when the sha parameter does not match the source SHA' do
+      before { post :merge, base_params.merge(sha: 'foo') }
+
+      it 'returns :sha_mismatch' do
+        expect(assigns(:status)).to eq(:sha_mismatch)
+      end
+    end
+
+    context 'when the sha parameter matches the source SHA' do
+      def merge_with_sha
+        post :merge, base_params.merge(sha: merge_request.source_sha)
+      end
+
+      it 'returns :success' do
+        merge_with_sha
+
+        expect(assigns(:status)).to eq(:success)
+      end
+
+      it 'starts the merge immediately' do
+        expect(MergeWorker).to receive(:perform_async).with(merge_request.id, anything, anything)
+
+        merge_with_sha
+      end
+
+      context 'when merge_when_build_succeeds is passed' do
+        def merge_when_build_succeeds
+          post :merge, base_params.merge(sha: merge_request.source_sha, merge_when_build_succeeds: '1')
+        end
+
+        before do
+          create(:ci_empty_commit, project: project, sha: merge_request.source_sha, ref: merge_request.source_branch)
+        end
+
+        it 'returns :merge_when_build_succeeds' do
+          merge_when_build_succeeds
+
+          expect(assigns(:status)).to eq(:merge_when_build_succeeds)
+        end
+
+        it 'sets the MR to merge when the build succeeds' do
+          service = double(:merge_when_build_succeeds_service)
+
+          expect(MergeRequests::MergeWhenBuildSucceedsService).to receive(:new).with(project, anything, anything).and_return(service)
+          expect(service).to receive(:execute).with(merge_request)
+
+          merge_when_build_succeeds
+        end
+      end
+    end
+  end
+
   describe "DELETE #destroy" do
     it "denies access to users unless they're admin or project owner" do
       delete :destroy, namespace_id: project.namespace.path, project_id: project.path, id: merge_request.iid
diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb
index 4b0111df149..5aa98ec4014 100644
--- a/spec/requests/api/merge_requests_spec.rb
+++ b/spec/requests/api/merge_requests_spec.rb
@@ -428,6 +428,19 @@ describe API::API, api: true  do
       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.source_sha.succ
+
+      expect(response.status).to eq(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.source_sha
+
+      expect(response.status).to eq(200)
+    end
+
     it "enables merge when build succeeds if the ci is active" do
       allow_any_instance_of(MergeRequest).to receive(:ci_commit).and_return(ci_commit)
       allow(ci_commit).to receive(:active?).and_return(true)