From fe6d392236fb6f1edd5dc1c33d52806cb4fa8a39 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 12 May 2014 16:34:41 +0300 Subject: Draft API method for merge MR Signed-off-by: Dmitriy Zaporozhets --- lib/api/merge_requests.rb | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index 4b88b0f84c1..fe615dfac05 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -111,6 +111,45 @@ module API end end + # Merge MR + # + # Parameters: + # id (required) - The ID of a project + # merge_request_id (required) - ID of MR + # merge_commit_message (optional) - Custom merge commit message + # Example: + # PUT /projects/:id/merge_request/:merge_request_id/merge + # + put ":id/merge_request/:merge_request_id/merge" do + merge_request = user_project.merge_requests.find(params[:merge_request_id]) + + action = if user_project.protected_branch?(merge_request.target_branch) + :push_code_to_protected_branches + else + :push_code + end + + if can?(current_user, action, project) + # Check if MR can be merged by GitLab + if merge_request.unchecked? + merge_request.check_if_can_be_merged + end + + if merge_request.open? && merge_request.can_be_merged? + merge_request.automerge!(current_user, params[:merge_commit_message] || merge_request.merge_commit_message) + + # return success + else + + # Checkif can be merged + end + + else + # not allowed + end + end + + # Get a merge request's comments # # Parameters: -- cgit v1.2.3 From 0c73e6664389ef5af352560c2fced1fbfc650a27 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 12 May 2014 17:51:43 +0300 Subject: Specify error codes for merge api Signed-off-by: Dmitriy Zaporozhets --- lib/api/merge_requests.rb | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index fe615dfac05..f9a4037dd26 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -130,22 +130,25 @@ module API end if can?(current_user, action, project) - # Check if MR can be merged by GitLab if merge_request.unchecked? merge_request.check_if_can_be_merged end - if merge_request.open? && merge_request.can_be_merged? - merge_request.automerge!(current_user, params[:merge_commit_message] || merge_request.merge_commit_message) - - # return success + if merge_request.open? + if merge_request.can_be_merged? + merge_request.automerge!(current_user, params[:merge_commit_message] || merge_request.merge_commit_message) + else + render_api_error!('Branch cannot be merged', 405) + end else - - # Checkif can be merged + # Merge request can not be merged + # because it is already closed/merged + not_allowed! end - else - # not allowed + # Merge request can not be merged + # because user dont have permissions to push into target branch + unauthorized! end end -- cgit v1.2.3 From 5880d7df6253fc97024005e7c32dbc41def99aaf Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 12 May 2014 18:01:22 +0300 Subject: Docs for merge api Signed-off-by: Dmitriy Zaporozhets --- doc/api/merge_requests.md | 48 +++++++++++++++++++++++++++++++++++++++++++++++ lib/api/merge_requests.rb | 1 + 2 files changed, 49 insertions(+) diff --git a/doc/api/merge_requests.md b/doc/api/merge_requests.md index d5b106729c9..d68f34971f1 100644 --- a/doc/api/merge_requests.md +++ b/doc/api/merge_requests.md @@ -189,6 +189,54 @@ Parameters: ``` +## Accept MR + +Merge changes submitted with MR usign this API. +If merge success you get 200 OK. +If it has some conflicts and can not be merged - you get 405 and error message 'Branch cannot be merged' +If merge request is already merged or closed - you get 405 and error message 'Method Not Allowed' +If you dont have permissions to accept this merge request - you get 401 + +``` +PUT /projects/:id/merge_request/:merge_request_id/merge +``` + +Parameters: + ++ `id` (required) - The ID of a project ++ `merge_request_id` (required) - ID of MR ++ `merge_commit_message` (optional) - Custom merge commit message + +```json +{ + "id": 1, + "target_branch": "master", + "source_branch": "test1", + "project_id": 3, + "title": "test1", + "state": "merged", + "upvotes": 0, + "downvotes": 0, + "author": { + "id": 1, + "username": "admin", + "email": "admin@local.host", + "name": "Administrator", + "state": "active", + "created_at": "2012-04-29T08:46:00Z" + }, + "assignee": { + "id": 1, + "username": "admin", + "email": "admin@local.host", + "name": "Administrator", + "state": "active", + "created_at": "2012-04-29T08:46:00Z" + } +} +``` + + ## Post comment to MR Adds a comment to a merge request. diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index f9a4037dd26..3dffe7bd4df 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -137,6 +137,7 @@ module API if merge_request.open? if merge_request.can_be_merged? merge_request.automerge!(current_user, params[:merge_commit_message] || merge_request.merge_commit_message) + present merge_request, with: Entities::MergeRequest else render_api_error!('Branch cannot be merged', 405) end -- cgit v1.2.3 From ab032256da9f24351871143058343f7463f9d7fc Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 13 May 2014 11:01:40 +0300 Subject: Add some tests for merge API Signed-off-by: Dmitriy Zaporozhets --- CHANGELOG | 1 + lib/api/merge_requests.rb | 4 ++-- spec/requests/api/merge_requests_spec.rb | 15 +++++++++++---- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 72bd6984869..91a7a22fe1b 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -14,6 +14,7 @@ v 6.9.0 - Fix wiki backup skip bug - Two Step MR creation process - Remove unwanted files from satellite working directory with git clean -fdx + - Accept merge request via API (sponsored by O'Reilly Media) v 6.8.0 - Ability to at mention users that are participating in issue and merge req. discussion diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index 3dffe7bd4df..7fb135b37b8 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -34,7 +34,7 @@ module API when "closed" then user_project.merge_requests.closed when "merged" then user_project.merge_requests.merged else user_project.merge_requests - end + end present paginate(mrs), with: Entities::MergeRequest end @@ -129,7 +129,7 @@ module API :push_code end - if can?(current_user, action, project) + if can?(current_user, action, user_project) if merge_request.unchecked? merge_request.check_if_can_be_merged end diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index db7c30e1ab8..b47cbbee773 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -183,11 +183,18 @@ describe API::API, api: true do end end - describe "PUT /projects/:id/merge_request/:merge_request_id to merge MR" do - it "should return merge_request" do - put api("/projects/#{project.id}/merge_request/#{merge_request.id}", user), state_event: "merge" + describe "PUT /projects/:id/merge_request/:merge_request_id/merge" do + it "should return merge_request in case of success" do + MergeRequest.any_instance.stub(can_be_merged?: true, automerge!: true) + put api("/projects/#{project.id}/merge_request/#{merge_request.id}/merge", user) response.status.should == 200 - json_response['state'].should == 'merged' + end + + it "should return 405 if branch can't be merged" do + MergeRequest.any_instance.stub(can_be_merged?: false) + put api("/projects/#{project.id}/merge_request/#{merge_request.id}/merge", user) + response.status.should == 405 + json_response['message'].should == 'Branch cannot be merged' end end -- cgit v1.2.3 From 2d2b2da45a586bdf29e115dcb4b4f66f9a1feed0 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 13 May 2014 13:20:10 +0300 Subject: More tests for merge api Signed-off-by: Dmitriy Zaporozhets --- spec/requests/api/merge_requests_spec.rb | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index b47cbbee773..2fb3684fdf0 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -196,6 +196,21 @@ describe API::API, api: true do response.status.should == 405 json_response['message'].should == 'Branch cannot be merged' end + + it "should return 405 if merge_request is not open" do + merge_request.close + put api("/projects/#{project.id}/merge_request/#{merge_request.id}/merge", user) + response.status.should == 405 + json_response['message'].should == 'Method Not Allowed' + end + + it "should return 401 if user has no permissions to merge" do + user2 = create(:user) + project.team << [user2, :reporter] + put api("/projects/#{project.id}/merge_request/#{merge_request.id}/merge", user2) + response.status.should == 401 + json_response['message'].should == '401 Unauthorized' + end end describe "PUT /projects/:id/merge_request/:merge_request_id" do -- cgit v1.2.3