Merge branch 'add-webhook-note-events' into 'master'

Add support for Webhook note events

This MR adds support for note events (comments) for WebHooks.

Perhaps the only potential source of confusion is that the settings indicates "Note Events" instead of "Comments". What do you think?

Screenshot after:

![image](https://gitlab.com/stanhu/gitlab-ce/uploads/299b141d82f7b14943a74258d5c57ca6/image.png)

Closes https://github.com/gitlabhq/gitlabhq/issues/6745

See merge request !673

17 files changed, 317 insertions, 6 deletions

diff --git a/CHANGELOG b/CHANGELOG
index d1ecfb40350..ac4670ba943 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,6 +1,7 @@
 Please view this file on the master branch, on stable branches it's out of date.
 
 v 7.12.0 (unreleased)
+  - Add web hook support for note events (Stan Hu)
   - Allow to configure location of the `.gitlab_shell_secret` file. (Jakub Jirutka)
   - Disabled expansion of top/bottom blobs for new file diffs
   - Update Asciidoctor gem to version 1.5.2. (Jakub Jirutka)
diff --git a/app/controllers/projects/hooks_controller.rb b/app/controllers/projects/hooks_controller.rb
index 57fc48ac7da..76062446c92 100644
--- a/app/controllers/projects/hooks_controller.rb
+++ b/app/controllers/projects/hooks_controller.rb
@@ -53,6 +53,6 @@ class Projects::HooksController < Projects::ApplicationController
   end
 
   def hook_params
-    params.require(:hook).permit(:url, :push_events, :issues_events, :merge_requests_events, :tag_push_events)
+    params.require(:hook).permit(:url, :push_events, :issues_events, :merge_requests_events, :tag_push_events, :note_events)
   end
 end
diff --git a/app/models/hooks/project_hook.rb b/app/models/hooks/project_hook.rb
index 21867a9316c..ca7066b959a 100644
--- a/app/models/hooks/project_hook.rb
+++ b/app/models/hooks/project_hook.rb
@@ -13,6 +13,7 @@
 #  issues_events         :boolean          default(FALSE), not null
 #  merge_requests_events :boolean          default(FALSE), not null
 #  tag_push_events       :boolean          default(FALSE)
+#  note_events           :boolean          default(FALSE), not null
 #
 
 class ProjectHook < WebHook
@@ -21,5 +22,6 @@ class ProjectHook < WebHook
   scope :push_hooks, -> { where(push_events: true) }
   scope :tag_push_hooks, -> { where(tag_push_events: true) }
   scope :issue_hooks, -> { where(issues_events: true) }
+  scope :note_hooks, -> { where(note_events: true) }
   scope :merge_request_hooks, -> { where(merge_requests_events: true) }
 end
diff --git a/app/models/hooks/service_hook.rb b/app/models/hooks/service_hook.rb
index 5b38ade2e6b..b55e217975f 100644
--- a/app/models/hooks/service_hook.rb
+++ b/app/models/hooks/service_hook.rb
@@ -13,6 +13,7 @@
 #  issues_events         :boolean          default(FALSE), not null
 #  merge_requests_events :boolean          default(FALSE), not null
 #  tag_push_events       :boolean          default(FALSE)
+#  note_events           :boolean          default(FALSE), not null
 #
 
 class ServiceHook < WebHook
diff --git a/app/models/hooks/system_hook.rb b/app/models/hooks/system_hook.rb
index ee32b49bc66..6fb2d421026 100644
--- a/app/models/hooks/system_hook.rb
+++ b/app/models/hooks/system_hook.rb
@@ -13,6 +13,7 @@
 #  issues_events         :boolean          default(FALSE), not null
 #  merge_requests_events :boolean          default(FALSE), not null
 #  tag_push_events       :boolean          default(FALSE)
+#  note_events           :boolean          default(FALSE), not null
 #
 
 class SystemHook < WebHook
diff --git a/app/models/hooks/web_hook.rb b/app/models/hooks/web_hook.rb
index e9fd441352d..46fb85336e5 100644
--- a/app/models/hooks/web_hook.rb
+++ b/app/models/hooks/web_hook.rb
@@ -13,6 +13,7 @@
 #  issues_events         :boolean          default(FALSE), not null
 #  merge_requests_events :boolean          default(FALSE), not null
 #  tag_push_events       :boolean          default(FALSE)
+#  note_events           :boolean          default(FALSE), not null
 #
 
 class WebHook < ActiveRecord::Base
@@ -21,6 +22,7 @@ class WebHook < ActiveRecord::Base
 
   default_value_for :push_events, true
   default_value_for :issues_events, false
+  default_value_for :note_events, false
   default_value_for :merge_requests_events, false
   default_value_for :tag_push_events, false
 
diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb
index d19a6c2eca3..0ff37c41743 100644
--- a/app/services/notes/create_service.rb
+++ b/app/services/notes/create_service.rb
@@ -31,7 +31,7 @@ module Notes
 
     def execute_hooks(note)
       note_data = hook_data(note)
-      # TODO: Support Webhooks
+      note.project.execute_hooks(note_data, :note_hooks)
       note.project.execute_services(note_data, :note_hooks)
     end
   end
diff --git a/app/views/projects/hooks/index.html.haml b/app/views/projects/hooks/index.html.haml
index 808c03148f4..eadbf61fdd4 100644
--- a/app/views/projects/hooks/index.html.haml
+++ b/app/views/projects/hooks/index.html.haml
@@ -35,6 +35,13 @@
           %p.light
             This url will be triggered when a new tag is pushed to the repository
       %div
+        = f.check_box :note_events, class: 'pull-left'
+        .prepend-left-20
+          = f.label :note_events, class: 'list-label' do
+            %strong Comments
+          %p.light
+            This url will be triggered when someone adds a comment
+      %div
         = f.check_box :issues_events, class: 'pull-left'
         .prepend-left-20
           = f.label :issues_events, class: 'list-label' do
@@ -64,6 +71,6 @@
           .clearfix
             %span.monospace= hook.url
           %p
-            - %w(push_events tag_push_events issues_events merge_requests_events).each do |trigger|
+            - %w(push_events tag_push_events issues_events note_events merge_requests_events).each do |trigger|
               - if hook.send(trigger)
                 %span.label.label-gray= trigger.titleize
diff --git a/db/migrate/20150516060434_add_note_events_to_web_hooks.rb b/db/migrate/20150516060434_add_note_events_to_web_hooks.rb
new file mode 100644
index 00000000000..0097587b4f6
--- /dev/null
+++ b/db/migrate/20150516060434_add_note_events_to_web_hooks.rb
@@ -0,0 +1,9 @@
+class AddNoteEventsToWebHooks < ActiveRecord::Migration
+  def up
+    add_column :web_hooks, :note_events, :boolean, default: false, null: false
+  end
+
+  def down
+    remove_column :web_hooks, :note_events, :boolean
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index f7581eaf7fb..1ab91256406 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20150509180749) do
+ActiveRecord::Schema.define(version: 20150516060434) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -533,6 +533,7 @@ ActiveRecord::Schema.define(version: 20150509180749) do
     t.boolean  "issues_events",         default: false,         null: false
     t.boolean  "merge_requests_events", default: false,         null: false
     t.boolean  "tag_push_events",       default: false
+    t.boolean  "note_events",           default: false,         null: false
   end
 
   add_index "web_hooks", ["created_at", "id"], name: "index_web_hooks_on_created_at_and_id", using: :btree
diff --git a/doc/web_hooks/web_hooks.md b/doc/web_hooks/web_hooks.md
index d140f3a457a..73717ffc7d6 100644
--- a/doc/web_hooks/web_hooks.md
+++ b/doc/web_hooks/web_hooks.md
@@ -140,6 +140,285 @@ X-Gitlab-Event: Issue Hook
   }
 }
 ```
+## Comment events
+
+Triggered when a new comment is made on commits, merge requests, issues, and code snippets.
+The note data will be stored in `object_attributes` (e.g. `note`, `noteable_type`). The
+payload will also include information about the target of the comment. For example,
+a comment on a issue will include the specific issue information under the `issue` key.
+Valid target types:
+
+1. `commit`
+2. `merge_request`
+3. `issue`
+4. `snippet`
+
+### Comment on commit
+
+**Request header**:
+
+```
+X-Gitlab-Event: Note Hook
+```
+
+**Request body:**
+
+```json
+{
+  "object_kind": "note",
+  "user": {
+    "name": "Adminstrator",
+    "username": "root",
+    "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=40\u0026d=identicon"
+  },
+  "project_id": 5,
+  "repository": {
+    "name": "Gitlab Test",
+    "url": "http://localhost/gitlab-org/gitlab-test.git",
+    "description": "Aut reprehenderit ut est.",
+    "homepage": "http://example.com/gitlab-org/gitlab-test"
+  },
+  "object_attributes": {
+    "id": 1243,
+    "note": "This is a commit comment. How does this work?",
+    "noteable_type": "Commit",
+    "author_id": 1,
+    "created_at": "2015-05-17 18:08:09 UTC",
+    "updated_at": "2015-05-17 18:08:09 UTC",
+    "project_id": 5,
+    "attachment":null,
+    "line_code": "bec9703f7a456cd2b4ab5fb3220ae016e3e394e3_0_1",
+    "commit_id": "cfe32cf61b73a0d5e9f13e774abde7ff789b1660",
+    "noteable_id": null,
+    "system": false,
+    "st_diff": {
+      "diff": "--- /dev/null\n+++ b/six\n@@ -0,0 +1 @@\n+Subproject commit 409f37c4f05865e4fb208c771485f211a22c4c2d\n",
+      "new_path": "six",
+      "old_path": "six",
+      "a_mode": "0",
+      "b_mode": "160000",
+      "new_file": true,
+      "renamed_file": false,
+      "deleted_file": false
+    },
+    "url": "http://example.com/gitlab-org/gitlab-test/commit/cfe32cf61b73a0d5e9f13e774abde7ff789b1660#note_1243"
+  },
+  "commit": {
+    "id": "cfe32cf61b73a0d5e9f13e774abde7ff789b1660",
+    "message": "Add submodule\n\nSigned-off-by: Dmitriy Zaporozhets \u003cdmitriy.zaporozhets@gmail.com\u003e\n",
+    "timestamp": "2014-02-27T10:06:20+02:00",
+    "url": "http://example.com/gitlab-org/gitlab-test/commit/cfe32cf61b73a0d5e9f13e774abde7ff789b1660",
+    "author": {
+      "name": "Dmitriy Zaporozhets",
+      "email": "dmitriy.zaporozhets@gmail.com"
+    }
+  }
+}
+```
+
+### Comment on merge request
+
+**Request header**:
+
+```
+X-Gitlab-Event: Note Hook
+```
+
+**Request body:**
+
+```json
+{
+  "object_kind": "note",
+  "user": {
+    "name": "Administrator",
+    "username": "root",
+    "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=40\u0026d=identicon"
+  },
+  "project_id": 5,
+  "repository": {
+    "name": "Gitlab Test",
+    "url": "http://example.com/gitlab-org/gitlab-test.git",
+    "description": "Aut reprehenderit ut est.",
+    "homepage": "http://example.com/gitlab-org/gitlab-test"
+  },
+  "object_attributes": {
+    "id": 1244,
+    "note": "This MR needs work.",
+    "noteable_type": "MergeRequest",
+    "author_id": 1,
+    "created_at": "2015-05-17 18:21:36 UTC",
+    "updated_at": "2015-05-17 18:21:36 UTC",
+    "project_id": 5,
+    "attachment": null,
+    "line_code": null,
+    "commit_id": "",
+    "noteable_id": 7,
+    "system": false,
+    "st_diff": null,
+    "url": "http://example.com/gitlab-org/gitlab-test/merge_requests/1#note_1244"
+  },
+  "merge_request": {
+    "id": 7,
+    "target_branch": "markdown",
+    "source_branch": "master",
+    "source_project_id": 5,
+    "author_id": 8,
+    "assignee_id": 28,
+    "title": "Tempora et eos debitis quae laborum et.",
+    "created_at": "2015-03-01 20:12:53 UTC",
+    "updated_at": "2015-03-21 18:27:27 UTC",
+    "milestone_id": 11,
+    "state": "opened",
+    "merge_status": "cannot_be_merged",
+    "target_project_id": 5,
+    "iid": 1,
+    "description": "Et voluptas corrupti assumenda temporibus. Architecto cum animi eveniet amet asperiores. Vitae numquam voluptate est natus sit et ad id.",
+    "position": 0,
+    "locked_at": null,
+    "source": {
+      "name": "Gitlab Test",
+      "ssh_url": "git@example.com:gitlab-org/gitlab-test.git",
+      "http_url": "http://example.com/gitlab-org/gitlab-test.git",
+      "namespace": "Gitlab Org",
+      "visibility_level": 10
+    },
+    "target": {
+      "name": "Gitlab Test",
+      "ssh_url": "git@example.com:gitlab-org/gitlab-test.git",
+      "http_url": "http://example.com/gitlab-org/gitlab-test.git",
+      "namespace": "Gitlab Org",
+      "visibility_level": 10
+    },
+    "last_commit": {
+      "id": "562e173be03b8ff2efb05345d12df18815438a4b",
+      "message": "Merge branch 'another-branch' into 'master'\n\nCheck in this test\n",
+      "timestamp": "2015-04-08T21: 00:25-07:00",
+      "url": "http://example.com/gitlab-org/gitlab-test/commit/562e173be03b8ff2efb05345d12df18815438a4b",
+      "author": {
+        "name": "John Smith",
+        "email": "john@example.com"
+      }
+    }
+  }
+}
+```
+
+### Comment on issue
+
+**Request header**:
+
+```
+X-Gitlab-Event: Note Hook
+```
+
+**Request body:**
+
+```json
+{
+  "object_kind": "note",
+  "user": {
+    "name": "Adminstrator",
+    "username": "root",
+    "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=40\u0026d=identicon"
+  },
+  "project_id": 5,
+  "repository": {
+    "name": "Gitlab Test",
+    "url": "http://example.com/gitlab-org/gitlab-test.git",
+    "description": "Aut reprehenderit ut est.",
+    "homepage": "http://example.com/gitlab-org/gitlab-test"
+  },
+  "object_attributes": {
+    "id": 1241,
+    "note": "Hello world",
+    "noteable_type": "Issue",
+    "author_id": 1,
+    "created_at": "2015-05-17 17:06:40 UTC",
+    "updated_at": "2015-05-17 17:06:40 UTC",
+    "project_id": 5,
+    "attachment": null,
+    "line_code": null,
+    "commit_id": "",
+    "noteable_id": 92,
+    "system": false,
+    "st_diff": null,
+    "url": "http://example.com/gitlab-org/gitlab-test/issues/17#note_1241"
+  },
+  "issue": {
+    "id": 92,
+    "title": "test",
+    "assignee_id": null,
+    "author_id": 1,
+    "project_id": 5,
+    "created_at": "2015-04-12 14:53:17 UTC",
+    "updated_at": "2015-04-26 08:28:42 UTC",
+    "position": 0,
+    "branch_name": null,
+    "description": "test",
+    "milestone_id": null,
+    "state": "closed",
+    "iid": 17
+  }
+}
+```
+
+### Comment on code snippet
+
+
+**Request header**:
+
+```
+X-Gitlab-Event: Note Hook
+```
+
+**Request body:**
+
+```
+{
+  "object_kind": "note",
+  "user": {
+    "name": "Administrator",
+    "username": "root",
+    "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=40\u0026d=identicon"
+  },
+  "project_id": 5,
+  "repository": {
+    "name": "Gitlab Test",
+    "url": "http://example.com/gitlab-org/gitlab-test.git",
+    "description": "Aut reprehenderit ut est.",
+    "homepage": "http://example.com/gitlab-org/gitlab-test"
+  },
+  "object_attributes": {
+    "id": 1245,
+    "note": "Is this snippet doing what it's supposed to be doing?",
+    "noteable_type": "Snippet",
+    "author_id": 1,
+    "created_at": "2015-05-17 18:35:50 UTC",
+    "updated_at": "2015-05-17 18:35:50 UTC",
+    "project_id": 5,
+    "attachment": null,
+    "line_code": null,
+    "commit_id": "",
+    "noteable_id": 53,
+    "system": false,
+    "st_diff": null,
+    "url": "http://example.com/gitlab-org/gitlab-test/snippets/53#note_1245"
+  },
+  "snippet": {
+    "id": 53,
+    "title": "test",
+    "content": "puts 'Hello world'",
+    "author_id": 1,
+    "project_id": 5,
+    "created_at": "2015-04-09 02:40:38 UTC",
+    "updated_at": "2015-04-09 02:40:38 UTC",
+    "file_name": "test.rb",
+    "expires_at": null,
+    "type": "ProjectSnippet",
+    "visibility_level": 0
+  }
+}
+```
 
 ## Merge request events
 
diff --git a/lib/api/project_hooks.rb b/lib/api/project_hooks.rb
index be9850367b9..ad4d2e65dfd 100644
--- a/lib/api/project_hooks.rb
+++ b/lib/api/project_hooks.rb
@@ -43,7 +43,8 @@ module API
           :push_events,
           :issues_events,
           :merge_requests_events,
-          :tag_push_events
+          :tag_push_events,
+          :note_events
         ]
         @hook = user_project.hooks.new(attrs)
 
@@ -73,7 +74,8 @@ module API
           :push_events,
           :issues_events,
           :merge_requests_events,
-          :tag_push_events
+          :tag_push_events,
+          :note_events
         ]
 
         if @hook.update_attributes attrs
diff --git a/spec/models/hooks/project_hook_spec.rb b/spec/models/hooks/project_hook_spec.rb
index 4e0d50d7f3f..dae7e399cfb 100644
--- a/spec/models/hooks/project_hook_spec.rb
+++ b/spec/models/hooks/project_hook_spec.rb
@@ -13,6 +13,7 @@
 #  issues_events         :boolean          default(FALSE), not null
 #  merge_requests_events :boolean          default(FALSE), not null
 #  tag_push_events       :boolean          default(FALSE)
+#  note_events           :boolean          default(FALSE), not null
 #
 
 require 'spec_helper'
diff --git a/spec/models/hooks/service_hook_spec.rb b/spec/models/hooks/service_hook_spec.rb
index d9714596f5d..fb5111dd9f5 100644
--- a/spec/models/hooks/service_hook_spec.rb
+++ b/spec/models/hooks/service_hook_spec.rb
@@ -13,6 +13,7 @@
 #  issues_events         :boolean          default(FALSE), not null
 #  merge_requests_events :boolean          default(FALSE), not null
 #  tag_push_events       :boolean          default(FALSE)
+#  note_events           :boolean          default(FALSE), not null
 #
 
 require "spec_helper"
diff --git a/spec/models/hooks/system_hook_spec.rb b/spec/models/hooks/system_hook_spec.rb
index e4b6b886565..edb21fc2e47 100644
--- a/spec/models/hooks/system_hook_spec.rb
+++ b/spec/models/hooks/system_hook_spec.rb
@@ -13,6 +13,7 @@
 #  issues_events         :boolean          default(FALSE), not null
 #  merge_requests_events :boolean          default(FALSE), not null
 #  tag_push_events       :boolean          default(FALSE)
+#  note_events           :boolean          default(FALSE), not null
 #
 
 require "spec_helper"
diff --git a/spec/models/hooks/web_hook_spec.rb b/spec/models/hooks/web_hook_spec.rb
index 9f5ef3eff70..4c3f0cbcbbf 100644
--- a/spec/models/hooks/web_hook_spec.rb
+++ b/spec/models/hooks/web_hook_spec.rb
@@ -13,6 +13,7 @@
 #  issues_events         :boolean          default(FALSE), not null
 #  merge_requests_events :boolean          default(FALSE), not null
 #  tag_push_events       :boolean          default(FALSE)
+#  note_events           :boolean          default(FALSE), not null
 #
 
 require 'spec_helper'
diff --git a/spec/services/notes/create_service_spec.rb b/spec/services/notes/create_service_spec.rb
index 1a02299bf19..0dc3b412783 100644
--- a/spec/services/notes/create_service_spec.rb
+++ b/spec/services/notes/create_service_spec.rb
@@ -15,6 +15,8 @@ describe Notes::CreateService do
           noteable_id: issue.id
         }
 
+        expect(project).to receive(:execute_hooks)
+        expect(project).to receive(:execute_services)
         @note = Notes::CreateService.new(project, user, opts).execute
       end