diff --git a/doc/api/system_hooks.md b/doc/api/system_hooks.md index f6e11ed238e8f6dd6f9c9286e4677ff04a4b0e93..dca22c43f833fc0a903ce3a71873c1ba755adfff 100644 --- a/doc/api/system_hooks.md +++ b/doc/api/system_hooks.md @@ -8,7 +8,10 @@ Get list of system hooks GET /hooks ``` -Will return hooks with status `200 OK` on success, or `404 Not found` on fail. +Parameters: + ++ **none** + ## Add new system hook hook @@ -20,7 +23,6 @@ Parameters: + `url` (required) - The hook URL -Will return status `201 Created` on success, or `404 Not found` on fail. ## Test system hook @@ -32,10 +34,12 @@ Parameters: + `id` (required) - The ID of hook -Will return hook with status `200 OK` on success, or `404 Not found` on fail. ## Delete system hook +Deletes a system hook. This is an idempotent API function and returns `200 Ok` even if the hook +is not available. If the hook is deleted it is also returned as JSON. + ``` DELETE /hooks/:id ``` @@ -43,5 +47,3 @@ DELETE /hooks/:id Parameters: + `id` (required) - The ID of hook - -Will return status `200 OK` on success, or `404 Not found` on fail. \ No newline at end of file diff --git a/lib/api/system_hooks.rb b/lib/api/system_hooks.rb index 665a1cdd0d257e3a5379dca01dabb4d31df21f92..da0b005d69dc089a00ac0397c401f68e5d19dfb9 100644 --- a/lib/api/system_hooks.rb +++ b/lib/api/system_hooks.rb @@ -1,7 +1,10 @@ module Gitlab # Hooks API class SystemHooks < Grape::API - before { authenticated_as_admin! } + before { + authenticate! + authenticated_as_admin! + } resource :hooks do # Get the list of system hooks @@ -21,6 +24,7 @@ module Gitlab # POST /hooks post do attrs = attributes_for_keys [:url] + required_attributes! [:url] @hook = SystemHook.new attrs if @hook.save present @hook, with: Entities::Hook @@ -47,13 +51,19 @@ module Gitlab data end - # Delete a hook - # + # Delete a hook. This is an idempotent function. + # + # Parameters: + # id (required) - ID of the hook # Example Request: # DELETE /hooks/:id delete ":id" do - @hook = SystemHook.find(params[:id]) - @hook.destroy + begin + @hook = SystemHook.find(params[:id]) + @hook.destroy + rescue + # SystemHook raises an Error if no hook with id found + end end end end diff --git a/spec/requests/api/system_hooks_spec.rb b/spec/requests/api/system_hooks_spec.rb index 9842ae91ec375d40d10fbdd3bb414f5462dbeb84..fe1b324c9210b173aba25f65655a33e3f7992a21 100644 --- a/spec/requests/api/system_hooks_spec.rb +++ b/spec/requests/api/system_hooks_spec.rb @@ -10,6 +10,13 @@ describe Gitlab::API do before { stub_request(:post, hook.url) } describe "GET /hooks" do + context "when no user" do + it "should return authentication error" do + get api("/hooks") + response.status.should == 401 + end + end + context "when not an admin" do it "should return forbidden error" do get api("/hooks", user) @@ -34,9 +41,9 @@ describe Gitlab::API do }.to change { SystemHook.count }.by(1) end - it "should respond with 404 on failure" do + it "should respond with 400 if url not given" do post api("/hooks", admin) - response.status.should == 404 + response.status.should == 400 end it "should not create new hook without url" do @@ -65,5 +72,10 @@ describe Gitlab::API do delete api("/hooks/#{hook.id}", admin) }.to change { SystemHook.count }.by(-1) end + + it "should return success if hook id not found" do + delete api("/hooks/12345", admin) + response.status.should == 200 + end end end \ No newline at end of file