From 7cc4339f71be5a71e1d8a95c4524c4671e9d8a24 Mon Sep 17 00:00:00 2001
From: Sebastian Ziebell <sebastian.ziebell@asquera.de>
Date: Tue, 12 Feb 2013 17:44:42 +0100
Subject: [PATCH] API: changed status codes for project hooks functions
Different status codes in the API lib are returned on hook creation, update or deletion.
If a required parameter is not given (e.g. `url` in `/projects/:id/hooks/:hook_id`) status
code 400 (Bad request) is returned. On hook deletion a 200 status code is returned, regardless if
the hook is present or not. This makes the DELETE function an idempotent operation. Appropriate tests
are added to check these status codes.
---
lib/api/projects.rb | 16 ++++++++++++----
spec/requests/api/projects_spec.rb | 24 ++++++++++++++++++++++--
2 files changed, 34 insertions(+), 6 deletions(-)
diff --git a/lib/api/projects.rb b/lib/api/projects.rb
index f1e0f32e606..293353ab286 100644
--- a/lib/api/projects.rb
+++ b/lib/api/projects.rb
@@ -195,11 +195,14 @@ module Gitlab
# POST /projects/:id/hooks
post ":id/hooks" do
authorize! :admin_project, user_project
+
+ error!("Url not given", 400) unless params.has_key? :url
+
@hook = user_project.hooks.new({"url" => params[:url]})
if @hook.save
present @hook, with: Entities::Hook
else
- error!({'message' => '404 Not found'}, 404)
+ not_found!
end
end
@@ -215,7 +218,7 @@ module Gitlab
@hook = user_project.hooks.find(params[:hook_id])
authorize! :admin_project, user_project
- error!("Url not given", 400) if !params.has_key? :url
+ error!("Url not given", 400) unless params.has_key? :url
attrs = attributes_for_keys [:url]
if @hook.update_attributes attrs
@@ -234,8 +237,13 @@ module Gitlab
# DELETE /projects/:id/hooks
delete ":id/hooks" do
authorize! :admin_project, user_project
- @hook = user_project.hooks.find(params[:hook_id])
- @hook.destroy
+ error!("Hook id not given", 400) unless params.has_key? :hook_id
+
+ begin
+ @hook = ProjectHook.find(params[:hook_id])
+ @hook.destroy
+ rescue
+ end
end
# Get a project repository branches
diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb
index 6f53b38cb61..18fd1caea36 100644
--- a/spec/requests/api/projects_spec.rb
+++ b/spec/requests/api/projects_spec.rb
@@ -6,8 +6,8 @@ describe Gitlab::API do
let(:user) { create(:user) }
let(:user2) { create(:user) }
let(:user3) { create(:user) }
- let!(:hook) { create(:project_hook, project: project, url: "http://example.com") }
let!(:project) { create(:project, namespace: user.namespace ) }
+ let!(:hook) { create(:project_hook, project: project, url: "http://example.com") }
let!(:snippet) { create(:snippet, author: user, project: project, title: 'example') }
let!(:users_project) { create(:users_project, user: user, project: project, project_access: UsersProject::MASTER) }
let!(:users_project2) { create(:users_project, user: user3, project: project, project_access: UsersProject::DEVELOPER) }
@@ -290,6 +290,11 @@ describe Gitlab::API do
}.to change {project.hooks.count}.by(1)
response.status.should == 201
end
+
+ it "should return a 400 error if url not given" do
+ post api("/projects/#{project.id}/hooks", user)
+ response.status.should == 400
+ end
end
describe "PUT /projects/:id/hooks/:hook_id" do
@@ -300,7 +305,7 @@ describe Gitlab::API do
json_response['url'].should == 'http://example.org'
end
- it "should return 404 error if hook id is not found" do
+ it "should return 404 error if hook id not found" do
put api("/projects/#{project.id}/hooks/1234", user), url: 'http://example.org'
response.status.should == 404
end
@@ -319,6 +324,21 @@ describe Gitlab::API do
}.to change {project.hooks.count}.by(-1)
response.status.should == 200
end
+
+ it "should return success when deleting hook" do
+ delete api("/projects/#{project.id}/hooks", user), hook_id: hook.id
+ response.status.should == 200
+ end
+
+ it "should return success when deleting non existent hook" do
+ delete api("/projects/#{project.id}/hooks", user), hook_id: 42
+ response.status.should == 200
+ end
+
+ it "should return a 400 error if hook id not given" do
+ delete api("/projects/#{project.id}/hooks", user)
+ response.status.should == 400
+ end
end
describe "GET /projects/:id/repository/tags" do
--
GitLab