Commit 413952ff944d10164d9d08a8a48e7725fe44b1b3
1 parent
566de5ab
Exists in
master
and in
4 other branches
Creating or updating a MR returns more informative status codes.
Using the API library to create or update a merge request at the moment a 404 error is returned. This is fine when the merge request in question does not exist, but does not provide good information that for example a required attribute is missing. A status code of 400 (Bad request) is returned when creating or updating a merge request when either `source_branch` or `target_branch` is missing. A status code of 409 is returned when `source_branch` and `target_branch` are the same. Tests are added for these cases.
Showing
2 changed files
with
44 additions
and
0 deletions
Show diff stats
lib/api/merge_requests.rb
... | ... | @@ -60,6 +60,13 @@ module Gitlab |
60 | 60 | merge_request.reload_code |
61 | 61 | present merge_request, with: Entities::MergeRequest |
62 | 62 | else |
63 | + if merge_request.errors[:target_branch].any? | |
64 | + error!(merge_request.errors[:target_branch], 400) | |
65 | + elsif merge_request.errors[:source_branch].any? | |
66 | + error!(merge_request.errors[:source_branch], 400) | |
67 | + elsif merge_request.errors[:base].any? | |
68 | + error!(merge_request.errors[:base], 422) | |
69 | + end | |
63 | 70 | not_found! |
64 | 71 | end |
65 | 72 | end |
... | ... | @@ -88,6 +95,13 @@ module Gitlab |
88 | 95 | merge_request.mark_as_unchecked |
89 | 96 | present merge_request, with: Entities::MergeRequest |
90 | 97 | else |
98 | + if merge_request.errors[:target_branch].any? | |
99 | + error!(merge_request.errors[:target_branch], 400) | |
100 | + elsif merge_request.errors[:source_branch].any? | |
101 | + error!(merge_request.errors[:source_branch], 400) | |
102 | + elsif merge_request.errors[:base].any? | |
103 | + error!(merge_request.errors[:base], 422) | |
104 | + end | |
91 | 105 | not_found! |
92 | 106 | end |
93 | 107 | end | ... | ... |
spec/requests/api/merge_requests_spec.rb
... | ... | @@ -41,6 +41,24 @@ describe Gitlab::API do |
41 | 41 | response.status.should == 201 |
42 | 42 | json_response['title'].should == 'Test merge_request' |
43 | 43 | end |
44 | + | |
45 | + it "should return 422 when source_branch equals target_branch" do | |
46 | + post api("/projects/#{project.id}/merge_requests", user), | |
47 | + title: "Test merge_request", source_branch: "master", target_branch: "master", author: user | |
48 | + response.status.should == 422 | |
49 | + end | |
50 | + | |
51 | + it "should return 400 when source_branch is missing" do | |
52 | + post api("/projects/#{project.id}/merge_requests", user), | |
53 | + title: "Test merge_request", target_branch: "master", author: user | |
54 | + response.status.should == 400 | |
55 | + end | |
56 | + | |
57 | + it "should return 400 when target_branch is missing" do | |
58 | + post api("/projects/#{project.id}/merge_requests", user), | |
59 | + title: "Test merge_request", source_branch: "stable", author: user | |
60 | + response.status.should == 400 | |
61 | + end | |
44 | 62 | end |
45 | 63 | |
46 | 64 | describe "PUT /projects/:id/merge_request/:merge_request_id" do |
... | ... | @@ -49,6 +67,18 @@ describe Gitlab::API do |
49 | 67 | response.status.should == 200 |
50 | 68 | json_response['title'].should == 'New title' |
51 | 69 | end |
70 | + | |
71 | + it "should return 422 when source_branch and target_branch are renamed the same" do | |
72 | + put api("/projects/#{project.id}/merge_request/#{merge_request.id}", user), | |
73 | + source_branch: "master", target_branch: "master" | |
74 | + response.status.should == 422 | |
75 | + end | |
76 | + | |
77 | + it "should return merge_request with renamed target_branch" do | |
78 | + put api("/projects/#{project.id}/merge_request/#{merge_request.id}", user), target_branch: "test" | |
79 | + response.status.should == 200 | |
80 | + json_response['target_branch'].should == 'test' | |
81 | + end | |
52 | 82 | end |
53 | 83 | |
54 | 84 | describe "POST /projects/:id/merge_request/:merge_request_id/comments" do | ... | ... |