summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRobert Schilling <rschilling@student.tugraz.at>2016-10-13 19:32:10 +0200
committerRobert Schilling <rschilling@student.tugraz.at>2016-10-14 14:16:27 +0200
commit04593581037bca7a7c3b00c719404e610c158cc1 (patch)
treebbdf004bd6d7ae543ffe7fb22be5259762150227
parentfbeaa7518d8cf86ad62e94e9bc86ffe63715dffd (diff)
downloadgitlab-ce-fix-system-hook-api.tar.gz
API: Fix Sytem hooks delete behaviorfix-system-hook-api
-rw-r--r--doc/api/system_hooks.md7
-rw-r--r--lib/api/system_hooks.rb10
-rw-r--r--spec/requests/api/system_hooks_spec.rb7
3 files changed, 10 insertions, 14 deletions
diff --git a/doc/api/system_hooks.md b/doc/api/system_hooks.md
index 1802fae14fe..073e99b7147 100644
--- a/doc/api/system_hooks.md
+++ b/doc/api/system_hooks.md
@@ -98,11 +98,8 @@ Example response:
## 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, a JSON object is returned. An error is raised if the
-hook is not found.
+Deletes a system hook. It returns `200 OK` if the hooks is deleted and
+`404 Not Found` if the hook is not found.
---
diff --git a/lib/api/system_hooks.rb b/lib/api/system_hooks.rb
index 2e76b91051f..794e34874f4 100644
--- a/lib/api/system_hooks.rb
+++ b/lib/api/system_hooks.rb
@@ -56,12 +56,10 @@ module API
requires :id, type: Integer, desc: 'The ID of the system hook'
end
delete ":id" do
- begin
- hook = SystemHook.find(params[:id])
- present hook.destroy, with: Entities::Hook
- rescue
- # SystemHook raises an Error if no hook with id found
- end
+ hook = SystemHook.find_by(id: params[:id])
+ not_found!('System hook') unless hook
+
+ present hook.destroy, with: Entities::Hook
end
end
end
diff --git a/spec/requests/api/system_hooks_spec.rb b/spec/requests/api/system_hooks_spec.rb
index 1ce2658569e..f8a1aed5441 100644
--- a/spec/requests/api/system_hooks_spec.rb
+++ b/spec/requests/api/system_hooks_spec.rb
@@ -73,9 +73,10 @@ describe API::API, api: true do
end.to change { SystemHook.count }.by(-1)
end
- it "returns success if hook id not found" do
- delete api("/hooks/12345", admin)
- expect(response).to have_http_status(200)
+ it 'returns 404 if the system hook does not exist' do
+ delete api('/hooks/12345', admin)
+
+ expect(response).to have_http_status(404)
end
end
end