Commit 9544f9038981b881b539419be72276b2b2fd079f
1 parent
818caf0b
Exists in
master
and in
4 other branches
Adding a project hook returns status code 400 if url is not given
When adding a project hook a url must be specified or a 400 error code is returned * Specs added to check status code on handling project hooks * refactored code, extracted a method
Showing
2 changed files
with
41 additions
and
12 deletions
Show diff stats
lib/api/projects.rb
| @@ -4,6 +4,15 @@ module Gitlab | @@ -4,6 +4,15 @@ module Gitlab | ||
| 4 | before { authenticate! } | 4 | before { authenticate! } |
| 5 | 5 | ||
| 6 | resource :projects do | 6 | resource :projects do |
| 7 | + helpers do | ||
| 8 | + def handle_project_member_errors(errors) | ||
| 9 | + if errors[:project_access].any? | ||
| 10 | + error!(errors[:project_access], 422) | ||
| 11 | + end | ||
| 12 | + not_found! | ||
| 13 | + end | ||
| 14 | + end | ||
| 15 | + | ||
| 7 | # Get a projects list for authenticated user | 16 | # Get a projects list for authenticated user |
| 8 | # | 17 | # |
| 9 | # Example Request: | 18 | # Example Request: |
| @@ -36,6 +45,7 @@ module Gitlab | @@ -36,6 +45,7 @@ module Gitlab | ||
| 36 | # Example Request | 45 | # Example Request |
| 37 | # POST /projects | 46 | # POST /projects |
| 38 | post do | 47 | post do |
| 48 | + error!("Name is required", 400) if !params.has_key? :name | ||
| 39 | attrs = attributes_for_keys [:name, | 49 | attrs = attributes_for_keys [:name, |
| 40 | :description, | 50 | :description, |
| 41 | :default_branch, | 51 | :default_branch, |
| @@ -43,6 +53,7 @@ module Gitlab | @@ -43,6 +53,7 @@ module Gitlab | ||
| 43 | :wall_enabled, | 53 | :wall_enabled, |
| 44 | :merge_requests_enabled, | 54 | :merge_requests_enabled, |
| 45 | :wiki_enabled] | 55 | :wiki_enabled] |
| 56 | + | ||
| 46 | @project = ::Projects::CreateContext.new(current_user, attrs).execute | 57 | @project = ::Projects::CreateContext.new(current_user, attrs).execute |
| 47 | if @project.saved? | 58 | if @project.saved? |
| 48 | present @project, with: Entities::Project | 59 | present @project, with: Entities::Project |
| @@ -106,10 +117,7 @@ module Gitlab | @@ -106,10 +117,7 @@ module Gitlab | ||
| 106 | @member = team_member.user | 117 | @member = team_member.user |
| 107 | present @member, with: Entities::ProjectMember, project: user_project | 118 | present @member, with: Entities::ProjectMember, project: user_project |
| 108 | else | 119 | else |
| 109 | - if team_member.errors[:project_access].any? | ||
| 110 | - error!(team_member.errors[:project_access], 422) | ||
| 111 | - end | ||
| 112 | - not_found! | 120 | + handle_project_member_errors team_member.errors |
| 113 | end | 121 | end |
| 114 | end | 122 | end |
| 115 | 123 | ||
| @@ -132,10 +140,7 @@ module Gitlab | @@ -132,10 +140,7 @@ module Gitlab | ||
| 132 | @member = team_member.user | 140 | @member = team_member.user |
| 133 | present @member, with: Entities::ProjectMember, project: user_project | 141 | present @member, with: Entities::ProjectMember, project: user_project |
| 134 | else | 142 | else |
| 135 | - if team_member.errors[:project_access].any? | ||
| 136 | - error!(team_member.errors[:project_access], 422) | ||
| 137 | - end | ||
| 138 | - not_found! | 143 | + handle_project_member_errors team_member.errors |
| 139 | end | 144 | end |
| 140 | end | 145 | end |
| 141 | 146 | ||
| @@ -210,8 +215,9 @@ module Gitlab | @@ -210,8 +215,9 @@ module Gitlab | ||
| 210 | @hook = user_project.hooks.find(params[:hook_id]) | 215 | @hook = user_project.hooks.find(params[:hook_id]) |
| 211 | authorize! :admin_project, user_project | 216 | authorize! :admin_project, user_project |
| 212 | 217 | ||
| 213 | - attrs = attributes_for_keys [:url] | 218 | + error!("Url not given", 400) if !params.has_key? :url |
| 214 | 219 | ||
| 220 | + attrs = attributes_for_keys [:url] | ||
| 215 | if @hook.update_attributes attrs | 221 | if @hook.update_attributes attrs |
| 216 | present @hook, with: Entities::Hook | 222 | present @hook, with: Entities::Hook |
| 217 | else | 223 | else |
spec/requests/api/projects_spec.rb
| @@ -46,9 +46,9 @@ describe Gitlab::API do | @@ -46,9 +46,9 @@ describe Gitlab::API do | ||
| 46 | response.status.should == 201 | 46 | response.status.should == 201 |
| 47 | end | 47 | end |
| 48 | 48 | ||
| 49 | - it "should respond with 404 on failure" do | 49 | + it "should respond with 400 if name is not given" do |
| 50 | post api("/projects", user) | 50 | post api("/projects", user) |
| 51 | - response.status.should == 404 | 51 | + response.status.should == 400 |
| 52 | end | 52 | end |
| 53 | 53 | ||
| 54 | it "should assign attributes to project" do | 54 | it "should assign attributes to project" do |
| @@ -237,6 +237,13 @@ describe Gitlab::API do | @@ -237,6 +237,13 @@ describe Gitlab::API do | ||
| 237 | delete api("/projects/#{project.id}/members/#{user3.id}", user) | 237 | delete api("/projects/#{project.id}/members/#{user3.id}", user) |
| 238 | }.to change { UsersProject.count }.by(-1) | 238 | }.to change { UsersProject.count }.by(-1) |
| 239 | end | 239 | end |
| 240 | + | ||
| 241 | + it "should return 200 if team member is not part of a project" do | ||
| 242 | + delete api("/projects/#{project.id}/members/#{user3.id}", user) | ||
| 243 | + expect { | ||
| 244 | + delete api("/projects/#{project.id}/members/#{user3.id}", user) | ||
| 245 | + }.to_not change { UsersProject.count }.by(1) | ||
| 246 | + end | ||
| 240 | end | 247 | end |
| 241 | 248 | ||
| 242 | describe "DELETE /projects/:id/members/:user_id" do | 249 | describe "DELETE /projects/:id/members/:user_id" do |
| @@ -268,6 +275,11 @@ describe Gitlab::API do | @@ -268,6 +275,11 @@ describe Gitlab::API do | ||
| 268 | response.status.should == 200 | 275 | response.status.should == 200 |
| 269 | json_response['url'].should == hook.url | 276 | json_response['url'].should == hook.url |
| 270 | end | 277 | end |
| 278 | + | ||
| 279 | + it "should return a 404 error if hook id is not available" do | ||
| 280 | + get api("/projects/#{project.id}/hooks/1234", user) | ||
| 281 | + response.status.should == 404 | ||
| 282 | + end | ||
| 271 | end | 283 | end |
| 272 | 284 | ||
| 273 | describe "POST /projects/:id/hooks" do | 285 | describe "POST /projects/:id/hooks" do |
| @@ -276,6 +288,7 @@ describe Gitlab::API do | @@ -276,6 +288,7 @@ describe Gitlab::API do | ||
| 276 | post api("/projects/#{project.id}/hooks", user), | 288 | post api("/projects/#{project.id}/hooks", user), |
| 277 | "url" => "http://example.com" | 289 | "url" => "http://example.com" |
| 278 | }.to change {project.hooks.count}.by(1) | 290 | }.to change {project.hooks.count}.by(1) |
| 291 | + response.status.should == 200 | ||
| 279 | end | 292 | end |
| 280 | end | 293 | end |
| 281 | 294 | ||
| @@ -286,8 +299,17 @@ describe Gitlab::API do | @@ -286,8 +299,17 @@ describe Gitlab::API do | ||
| 286 | response.status.should == 200 | 299 | response.status.should == 200 |
| 287 | json_response['url'].should == 'http://example.org' | 300 | json_response['url'].should == 'http://example.org' |
| 288 | end | 301 | end |
| 289 | - end | ||
| 290 | 302 | ||
| 303 | + it "should return 404 error if hook id is not found" do | ||
| 304 | + put api("/projects/#{project.id}/hooks/1234", user), url: 'http://example.org' | ||
| 305 | + response.status.should == 404 | ||
| 306 | + end | ||
| 307 | + | ||
| 308 | + it "should return 400 error if url is not given" do | ||
| 309 | + put api("/projects/#{project.id}/hooks/#{hook.id}", user) | ||
| 310 | + response.status.should == 400 | ||
| 311 | + end | ||
| 312 | + end | ||
| 291 | 313 | ||
| 292 | describe "DELETE /projects/:id/hooks" do | 314 | describe "DELETE /projects/:id/hooks" do |
| 293 | it "should delete hook from project" do | 315 | it "should delete hook from project" do |
| @@ -295,6 +317,7 @@ describe Gitlab::API do | @@ -295,6 +317,7 @@ describe Gitlab::API do | ||
| 295 | delete api("/projects/#{project.id}/hooks", user), | 317 | delete api("/projects/#{project.id}/hooks", user), |
| 296 | hook_id: hook.id | 318 | hook_id: hook.id |
| 297 | }.to change {project.hooks.count}.by(-1) | 319 | }.to change {project.hooks.count}.by(-1) |
| 320 | + response.status.should == 200 | ||
| 298 | end | 321 | end |
| 299 | end | 322 | end |
| 300 | 323 |