Merge branch 'zj-expose-system-hooks' into 'master'

Expose more info for SystemHooks

See merge request !6964

4 files changed, 44 insertions, 16 deletions

diff --git a/doc/api/system_hooks.md b/doc/api/system_hooks.md
index 073e99b7147..efd23d514bc 100644
--- a/doc/api/system_hooks.md
+++ b/doc/api/system_hooks.md
@@ -27,11 +27,14 @@ Example response:
 
 ```json
 [
-   {
-      "id" : 1,
-      "url" : "https://gitlab.example.com/hook",
-      "created_at" : "2015-11-04T20:07:35.874Z"
-   }
+  {
+    "id":1,
+    "url":"https://gitlab.example.com/hook",
+    "created_at":"2016-10-31T12:32:15.192Z",
+    "push_events":true,
+    "tag_push_events":false,
+    "enable_ssl_verification":true
+  }
 ]
 ```
 
@@ -48,6 +51,10 @@ POST /hooks
 | Attribute | Type | Required | Description |
 | --------- | ---- | -------- | ----------- |
 | `url` | string | yes | The hook URL |
+| `token` | string | no | Secret token to validate received payloads; this will not be returned in the response |
+| `push_events` | boolean |  no | When true, the hook will fire on push events |
+| `tag_push_events` | boolean | no | When true, the hook will fire on new tags being pushed |
+| `enable_ssl_verification` | boolean | no | Do SSL verification when triggering the hook |
 
 Example request:
 
@@ -59,11 +66,14 @@ Example response:
 
 ```json
 [
-   {
-      "id" : 2,
-      "url" : "https://gitlab.example.com/hook",
-      "created_at" : "2015-11-04T20:07:35.874Z"
-   }
+  {
+    "id":1,
+    "url":"https://gitlab.example.com/hook",
+    "created_at":"2016-10-31T12:32:15.192Z",
+    "push_events":true,
+    "tag_push_events":false,
+    "enable_ssl_verification":true
+  }
 ]
 ```
 
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index ab9d2d54f4b..d52496451a2 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -43,14 +43,13 @@ module API
     end
 
     class Hook < Grape::Entity
-      expose :id, :url, :created_at
+      expose :id, :url, :created_at, :push_events, :tag_push_events
+      expose :enable_ssl_verification
     end
 
     class ProjectHook < Hook
-      expose :project_id, :push_events
-      expose :issues_events, :merge_requests_events, :tag_push_events
+      expose :project_id, :issues_events, :merge_requests_events
       expose :note_events, :build_events, :pipeline_events, :wiki_page_events
-      expose :enable_ssl_verification
     end
 
     class BasicProjectDetails < Grape::Entity
diff --git a/lib/api/system_hooks.rb b/lib/api/system_hooks.rb
index 794e34874f4..32f731c5652 100644
--- a/lib/api/system_hooks.rb
+++ b/lib/api/system_hooks.rb
@@ -12,6 +12,7 @@ module API
       end
       get do
         hooks = SystemHook.all
+
         present hooks, with: Entities::Hook
       end
 
@@ -19,10 +20,14 @@ module API
         success Entities::Hook
       end
       params do
-        requires :url, type: String, desc: 'The URL for the system hook'
+        requires :url, type: String, desc: "The URL to send the request to"
+        optional :token, type: String, desc: 'The token used to validate payloads'
+        optional :push_events, type: Boolean, desc: "Trigger hook on push events"
+        optional :tag_push_events, type: Boolean, desc: "Trigger hook on tag push events"
+        optional :enable_ssl_verification, type: Boolean, desc: "Do SSL verification when triggering the hook"
       end
       post do
-        hook = SystemHook.new declared(params).to_h
+        hook = SystemHook.new declared(params, include_missing: false).to_h
 
         if hook.save
           present hook, with: Entities::Hook
diff --git a/spec/requests/api/system_hooks_spec.rb b/spec/requests/api/system_hooks_spec.rb
index f8a1aed5441..f685a3685e6 100644
--- a/spec/requests/api/system_hooks_spec.rb
+++ b/spec/requests/api/system_hooks_spec.rb
@@ -13,6 +13,7 @@ describe API::API, api: true  do
     context "when no user" do
       it "returns authentication error" do
         get api("/hooks")
+
         expect(response).to have_http_status(401)
       end
     end
@@ -20,6 +21,7 @@ describe API::API, api: true  do
     context "when not an admin" do
       it "returns forbidden error" do
         get api("/hooks", user)
+
         expect(response).to have_http_status(403)
       end
     end
@@ -27,9 +29,12 @@ describe API::API, api: true  do
     context "when authenticated as admin" do
       it "returns an array of hooks" do
         get api("/hooks", admin)
+
         expect(response).to have_http_status(200)
         expect(json_response).to be_an Array
         expect(json_response.first['url']).to eq(hook.url)
+        expect(json_response.first['push_events']).to be true
+        expect(json_response.first['tag_push_events']).to be false
       end
     end
   end
@@ -43,6 +48,7 @@ describe API::API, api: true  do
 
     it "responds with 400 if url not given" do
       post api("/hooks", admin)
+
       expect(response).to have_http_status(400)
     end
 
@@ -51,6 +57,14 @@ describe API::API, api: true  do
         post api("/hooks", admin)
       end.not_to change { SystemHook.count }
     end
+
+    it 'sets default values for events' do
+      post api('/hooks', admin), url: 'http://mep.mep', enable_ssl_verification: true
+
+      expect(response).to have_http_status(201)
+      expect(json_response['enable_ssl_verification']).to be true
+      expect(json_response['tag_push_events']).to be false
+    end
   end
 
   describe "GET /hooks/:id" do