From deeff56967516764b287e15b2063899b13395b41 Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Fri, 15 May 2015 23:33:31 -0700
Subject: [PATCH] Add support for Webhook note events

Closes https://github.com/gitlabhq/gitlabhq/issues/6745
---
 CHANGELOG                                     |   1 +
 app/controllers/projects/hooks_controller.rb  |   2 +-
 app/models/hooks/project_hook.rb              |   2 +
 app/models/hooks/service_hook.rb              |   1 +
 app/models/hooks/system_hook.rb               |   1 +
 app/models/hooks/web_hook.rb                  |   2 +
 app/services/notes/create_service.rb          |   2 +-
 app/views/projects/hooks/index.html.haml      |   9 +-
 ...0516060434_add_note_events_to_web_hooks.rb |   9 +
 db/schema.rb                                  |   3 +-
 doc/web_hooks/web_hooks.md                    | 279 ++++++++++++++++++
 lib/api/project_hooks.rb                      |   6 +-
 spec/models/hooks/project_hook_spec.rb        |   1 +
 spec/models/hooks/service_hook_spec.rb        |   1 +
 spec/models/hooks/system_hook_spec.rb         |   1 +
 spec/models/hooks/web_hook_spec.rb            |   1 +
 spec/services/notes/create_service_spec.rb    |   2 +
 17 files changed, 317 insertions(+), 6 deletions(-)
 create mode 100644 db/migrate/20150516060434_add_note_events_to_web_hooks.rb

diff --git a/CHANGELOG b/CHANGELOG
index d8474968179..a1e972db8cb 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
@@ -34,6 +34,13 @@
             %strong Tag push events
           %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
@@ -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
 
-- 
GitLab