Merge branch 'feature/merge-request-system-hook' into 'master'

System hooks for Merge Requests

See merge request gitlab-org/gitlab-ce!14387

29 files changed, 419 insertions, 61 deletions

diff --git a/app/controllers/admin/hooks_controller.rb b/app/controllers/admin/hooks_controller.rb
index 77e3c95d197..2b47819303e 100644
--- a/app/controllers/admin/hooks_controller.rb
+++ b/app/controllers/admin/hooks_controller.rb
@@ -59,11 +59,9 @@ class Admin::HooksController < Admin::ApplicationController
   def hook_params
     params.require(:hook).permit(
       :enable_ssl_verification,
-      :push_events,
-      :tag_push_events,
-      :repository_update_events,
       :token,
-      :url
+      :url,
+      *SystemHook.triggers.values
     )
   end
 end
diff --git a/app/controllers/projects/hooks_controller.rb b/app/controllers/projects/hooks_controller.rb
index 6f51e7b9b40..dd7aa1a67b9 100644
--- a/app/controllers/projects/hooks_controller.rb
+++ b/app/controllers/projects/hooks_controller.rb
@@ -64,18 +64,10 @@ class Projects::HooksController < Projects::ApplicationController
 
   def hook_params
     params.require(:hook).permit(
-      :job_events,
-      :pipeline_events,
       :enable_ssl_verification,
-      :issues_events,
-      :confidential_issues_events,
-      :merge_requests_events,
-      :note_events,
-      :push_events,
-      :tag_push_events,
       :token,
       :url,
-      :wiki_page_events
+      *ProjectHook.triggers.values
     )
   end
 end
diff --git a/app/models/concerns/triggerable_hooks.rb b/app/models/concerns/triggerable_hooks.rb
new file mode 100644
index 00000000000..ec0ed3b795a
--- /dev/null
+++ b/app/models/concerns/triggerable_hooks.rb
@@ -0,0 +1,40 @@
+module TriggerableHooks
+  AVAILABLE_TRIGGERS = {
+    repository_update_hooks:  :repository_update_events,
+    push_hooks:               :push_events,
+    tag_push_hooks:           :tag_push_events,
+    issue_hooks:              :issues_events,
+    confidential_issue_hooks: :confidential_issues_events,
+    note_hooks:               :note_events,
+    merge_request_hooks:      :merge_requests_events,
+    job_hooks:                :job_events,
+    pipeline_hooks:           :pipeline_events,
+    wiki_page_hooks:          :wiki_page_events
+  }.freeze
+
+  extend ActiveSupport::Concern
+
+  class_methods do
+    attr_reader :triggerable_hooks
+
+    attr_reader :triggers
+
+    def hooks_for(trigger)
+      callable_scopes = triggers.keys + [:all]
+      return none unless callable_scopes.include?(trigger)
+
+      public_send(trigger) # rubocop:disable GitlabSecurity/PublicSend
+    end
+
+    private
+
+    def triggerable_hooks(hooks)
+      triggers = AVAILABLE_TRIGGERS.slice(*hooks)
+      @triggers = triggers
+
+      triggers.each do |trigger, event|
+        scope trigger, -> { where(event => true) }
+      end
+    end
+  end
+end
diff --git a/app/models/hooks/project_hook.rb b/app/models/hooks/project_hook.rb
index a8c424a6614..b6dd39b860b 100644
--- a/app/models/hooks/project_hook.rb
+++ b/app/models/hooks/project_hook.rb
@@ -1,19 +1,17 @@
 class ProjectHook < WebHook
-  TRIGGERS = {
-    push_hooks:               :push_events,
-    tag_push_hooks:           :tag_push_events,
-    issue_hooks:              :issues_events,
-    confidential_issue_hooks: :confidential_issues_events,
-    note_hooks:               :note_events,
-    merge_request_hooks:      :merge_requests_events,
-    job_hooks:                :job_events,
-    pipeline_hooks:           :pipeline_events,
-    wiki_page_hooks:          :wiki_page_events
-  }.freeze
+  include TriggerableHooks
 
-  TRIGGERS.each do |trigger, event|
-    scope trigger, -> { where(event => true) }
-  end
+  triggerable_hooks [
+    :push_hooks,
+    :tag_push_hooks,
+    :issue_hooks,
+    :confidential_issue_hooks,
+    :note_hooks,
+    :merge_request_hooks,
+    :job_hooks,
+    :pipeline_hooks,
+    :wiki_page_hooks
+  ]
 
   belongs_to :project
   validates :project, presence: true
diff --git a/app/models/hooks/system_hook.rb b/app/models/hooks/system_hook.rb
index 180c479c41b..0528266e5b3 100644
--- a/app/models/hooks/system_hook.rb
+++ b/app/models/hooks/system_hook.rb
@@ -1,14 +1,14 @@
 class SystemHook < WebHook
-  TRIGGERS = {
-    repository_update_hooks: :repository_update_events,
-    push_hooks:              :push_events,
-    tag_push_hooks:          :tag_push_events
-  }.freeze
+  include TriggerableHooks
 
-  TRIGGERS.each do |trigger, event|
-    scope trigger, -> { where(event => true) }
-  end
+  triggerable_hooks [
+    :repository_update_hooks,
+    :push_hooks,
+    :tag_push_hooks,
+    :merge_request_hooks
+  ]
 
   default_value_for :push_events, false
   default_value_for :repository_update_events, true
+  default_value_for :merge_requests_events, false
 end
diff --git a/app/models/project.rb b/app/models/project.rb
index d011b614c69..4017864f718 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -967,10 +967,12 @@ class Project < ActiveRecord::Base
 
   def execute_hooks(data, hooks_scope = :push_hooks)
     run_after_commit_or_now do
-      hooks.public_send(hooks_scope).each do |hook| # rubocop:disable GitlabSecurity/PublicSend
+      hooks.hooks_for(hooks_scope).each do |hook|
         hook.async_execute(data, hooks_scope.to_s)
       end
     end
+
+    SystemHooksService.new.execute_hooks(data, hooks_scope)
   end
 
   def execute_services(data, hooks_scope = :push_hooks)
diff --git a/app/services/system_hooks_service.rb b/app/services/system_hooks_service.rb
index af6d77ef5e8..a6b7a6e1416 100644
--- a/app/services/system_hooks_service.rb
+++ b/app/services/system_hooks_service.rb
@@ -8,7 +8,7 @@ class SystemHooksService
   end
 
   def execute_hooks(data, hooks_scope = :all)
-    SystemHook.public_send(hooks_scope).find_each do |hook| # rubocop:disable GitlabSecurity/PublicSend
+    SystemHook.hooks_for(hooks_scope).find_each do |hook|
       hook.async_execute(data, 'system_hooks')
     end
   end
diff --git a/app/services/test_hooks/base_service.rb b/app/services/test_hooks/base_service.rb
index 20d90504bd2..e9aefb1fb75 100644
--- a/app/services/test_hooks/base_service.rb
+++ b/app/services/test_hooks/base_service.rb
@@ -9,7 +9,7 @@ module TestHooks
     end
 
     def execute
-      trigger_key = hook.class::TRIGGERS.key(trigger.to_sym)
+      trigger_key = hook.class.triggers.key(trigger.to_sym)
       trigger_data_method = "#{trigger}_data"
 
       if trigger_key.nil? || !self.respond_to?(trigger_data_method, true)
diff --git a/app/services/test_hooks/system_service.rb b/app/services/test_hooks/system_service.rb
index 67552edefc9..9016c77b7f0 100644
--- a/app/services/test_hooks/system_service.rb
+++ b/app/services/test_hooks/system_service.rb
@@ -13,5 +13,12 @@ module TestHooks
     def repository_update_events_data
       Gitlab::DataBuilder::Repository.sample_data
     end
+
+    def merge_requests_events_data
+      merge_request = MergeRequest.of_projects(current_user.projects.select(:id)).first
+      throw(:validation_error, 'Ensure one of your projects has merge requests.') unless merge_request.present?
+
+      merge_request.to_hook_data(current_user)
+    end
   end
 end
diff --git a/app/views/admin/hooks/_form.html.haml b/app/views/admin/hooks/_form.html.haml
index 645005c6deb..d8f96ed5b0d 100644
--- a/app/views/admin/hooks/_form.html.haml
+++ b/app/views/admin/hooks/_form.html.haml
@@ -38,6 +38,13 @@
           %strong Tag push events
         %p.light
           This URL will be triggered when a new tag is pushed to the repository
+    %div
+      = form.check_box :merge_requests_events, class: 'pull-left'
+      .prepend-left-20
+        = form.label :merge_requests_events, class: 'list-label' do
+          %strong Merge request events
+        %p.light
+          This URL will be triggered when a merge request is created/updated/merged
 .form-group
   = form.label :enable_ssl_verification, 'SSL verification', class: 'control-label checkbox'
   .col-sm-10
diff --git a/app/views/admin/hooks/edit.html.haml b/app/views/admin/hooks/edit.html.haml
index efb15ccc8df..629b1a9940f 100644
--- a/app/views/admin/hooks/edit.html.haml
+++ b/app/views/admin/hooks/edit.html.haml
@@ -13,7 +13,7 @@
   = render partial: 'form', locals: { form: f, hook: @hook }
   .form-actions
     = f.submit 'Save changes', class: 'btn btn-create'
-    = render 'shared/web_hooks/test_button', triggers: SystemHook::TRIGGERS, hook: @hook
+    = render 'shared/web_hooks/test_button', triggers: SystemHook.triggers, hook: @hook
     = link_to 'Remove', admin_hook_path(@hook), method: :delete, class: 'btn btn-remove pull-right', data: { confirm: 'Are you sure?' }
 
 %hr
diff --git a/app/views/admin/hooks/index.html.haml b/app/views/admin/hooks/index.html.haml
index b6e1df5f3ac..bc02d9969d6 100644
--- a/app/views/admin/hooks/index.html.haml
+++ b/app/views/admin/hooks/index.html.haml
@@ -22,12 +22,12 @@
       - @hooks.each do |hook|
         %li
           .controls
-            = render 'shared/web_hooks/test_button', triggers: SystemHook::TRIGGERS, hook: hook, button_class: 'btn-sm'
+            = render 'shared/web_hooks/test_button', triggers: SystemHook.triggers, hook: hook, button_class: 'btn-sm'
             = link_to 'Edit', edit_admin_hook_path(hook), class: 'btn btn-sm'
             = link_to 'Remove', admin_hook_path(hook), data: { confirm: 'Are you sure?' }, method: :delete, class: 'btn btn-remove btn-sm'
           .monospace= hook.url
           %div
-            - SystemHook::TRIGGERS.each_value do |event|
+            - SystemHook.triggers.each_value do |event|
               - if hook.public_send(event)
                 %span.label.label-gray= event.to_s.titleize
             %span.label.label-gray SSL Verification: #{hook.enable_ssl_verification ? 'enabled' : 'disabled'}
diff --git a/app/views/projects/hooks/edit.html.haml b/app/views/projects/hooks/edit.html.haml
index b1219f019d7..dcc1f0e3fbe 100644
--- a/app/views/projects/hooks/edit.html.haml
+++ b/app/views/projects/hooks/edit.html.haml
@@ -12,7 +12,7 @@
       = render partial: 'shared/web_hooks/form', locals: { form: f, hook: @hook }
 
       = f.submit 'Save changes', class: 'btn btn-create'
-      = render 'shared/web_hooks/test_button', triggers: ProjectHook::TRIGGERS, hook: @hook
+      = render 'shared/web_hooks/test_button', triggers: ProjectHook.triggers, hook: @hook
       = link_to 'Remove', project_hook_path(@project, @hook), method: :delete, class: 'btn btn-remove pull-right', data: { confirm: 'Are you sure?' }
 
 %hr
diff --git a/app/views/projects/settings/integrations/_project_hook.html.haml b/app/views/projects/settings/integrations/_project_hook.html.haml
index 82516cb4bcf..cd003107d66 100644
--- a/app/views/projects/settings/integrations/_project_hook.html.haml
+++ b/app/views/projects/settings/integrations/_project_hook.html.haml
@@ -3,14 +3,14 @@
     .col-md-8.col-lg-7
       %strong.light-header= hook.url
       %div
-        - ProjectHook::TRIGGERS.each_value do |event|
+        - ProjectHook.triggers.each_value do |event|
           - if hook.public_send(event)
             %span.label.label-gray.deploy-project-label= event.to_s.titleize
     .col-md-4.col-lg-5.text-right-lg.prepend-top-5
       %span.append-right-10.inline
         SSL Verification: #{hook.enable_ssl_verification ? 'enabled' : 'disabled'}
       = link_to 'Edit', edit_project_hook_path(@project, hook), class: 'btn btn-sm'
-      = render 'shared/web_hooks/test_button', triggers: ProjectHook::TRIGGERS, hook: hook, button_class: 'btn-sm'
+      = render 'shared/web_hooks/test_button', triggers: ProjectHook.triggers, hook: hook, button_class: 'btn-sm'
       = link_to project_hook_path(@project, hook), data: { confirm: 'Are you sure?'}, method: :delete, class: 'btn btn-transparent' do
         %span.sr-only Remove
         = icon('trash')
diff --git a/app/views/shared/web_hooks/_form.html.haml b/app/views/shared/web_hooks/_form.html.haml
index 1f0e7629fb4..ad4d39b4aa1 100644
--- a/app/views/shared/web_hooks/_form.html.haml
+++ b/app/views/shared/web_hooks/_form.html.haml
@@ -50,7 +50,7 @@
       = form.check_box :merge_requests_events, class: 'pull-left'
       .prepend-left-20
         = form.label :merge_requests_events, class: 'list-label' do
-          %strong Merge Request events
+          %strong Merge request events
         %p.light
           This URL will be triggered when a merge request is created/updated/merged
     %li
diff --git a/changelogs/unreleased/feature-merge-request-system-hook.yml b/changelogs/unreleased/feature-merge-request-system-hook.yml
new file mode 100644
index 00000000000..cfc4c4235d6
--- /dev/null
+++ b/changelogs/unreleased/feature-merge-request-system-hook.yml
@@ -0,0 +1,5 @@
+---
+title: System hooks for Merge Requests
+merge_request: 14387
+author: Alexis Reigel
+type: added
diff --git a/doc/api/system_hooks.md b/doc/api/system_hooks.md
index 9750475f0a6..dd424470b67 100644
--- a/doc/api/system_hooks.md
+++ b/doc/api/system_hooks.md
@@ -33,6 +33,7 @@ Example response:
     "created_at":"2016-10-31T12:32:15.192Z",
     "push_events":true,
     "tag_push_events":false,
+    "merge_requests_events": true,
     "enable_ssl_verification":true
   }
 ]
@@ -54,6 +55,7 @@ POST /hooks
 | `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 |
+| `merge_requests_events` | boolean | no | Trigger hook on merge requests events |
 | `enable_ssl_verification` | boolean | no | Do SSL verification when triggering the hook |
 
 Example request:
@@ -72,6 +74,7 @@ Example response:
     "created_at":"2016-10-31T12:32:15.192Z",
     "push_events":true,
     "tag_push_events":false,
+    "merge_requests_events": true,
     "enable_ssl_verification":true
   }
 ]
diff --git a/doc/system_hooks/system_hooks.md b/doc/system_hooks/system_hooks.md
index 8c8501bcc23..9ba05c7815b 100644
--- a/doc/system_hooks/system_hooks.md
+++ b/doc/system_hooks/system_hooks.md
@@ -465,6 +465,135 @@ X-Gitlab-Event: System Hook
   "total_commits_count": 0
 }
 ```
+
+### Merge request events
+
+Triggered when a new merge request is created, an existing merge request was
+updated/merged/closed or a commit is added in the source branch.
+
+**Request header**:
+
+```
+X-Gitlab-Event: System Hook
+```
+
+```json
+{
+  "object_kind": "merge_request",
+  "user": {
+    "name": "Administrator",
+    "username": "root",
+    "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon"
+  },
+  "project": {
+    "name": "Example",
+    "description": "",
+    "web_url": "http://example.com/jsmith/example",
+    "avatar_url": null,
+    "git_ssh_url": "git@example.com:jsmith/example.git",
+    "git_http_url": "http://example.com/jsmith/example.git",
+    "namespace": "Jsmith",
+    "visibility_level": 0,
+    "path_with_namespace": "jsmith/example",
+    "default_branch": "master",
+    "ci_config_path": "",
+    "homepage": "http://example.com/jsmith/example",
+    "url": "git@example.com:jsmith/example.git",
+    "ssh_url": "git@example.com:jsmith/example.git",
+    "http_url": "http://example.com/jsmith/example.git"
+  },
+  "object_attributes": {
+    "id": 90,
+    "target_branch": "master",
+    "source_branch": "ms-viewport",
+    "source_project_id": 14,
+    "author_id": 51,
+    "assignee_id": 6,
+    "title": "MS-Viewport",
+    "created_at": "2017-09-20T08:31:45.944Z",
+    "updated_at": "2017-09-28T12:23:42.365Z",
+    "milestone_id": null,
+    "state": "opened",
+    "merge_status": "unchecked",
+    "target_project_id": 14,
+    "iid": 1,
+    "description": "",
+    "updated_by_id": 1,
+    "merge_error": null,
+    "merge_params": {
+      "force_remove_source_branch": "0"
+    },
+    "merge_when_pipeline_succeeds": false,
+    "merge_user_id": null,
+    "merge_commit_sha": null,
+    "deleted_at": null,
+    "in_progress_merge_commit_sha": null,
+    "lock_version": 5,
+    "time_estimate": 0,
+    "last_edited_at": "2017-09-27T12:43:37.558Z",
+    "last_edited_by_id": 1,
+    "head_pipeline_id": 61,
+    "ref_fetched": true,
+    "merge_jid": null,
+    "source": {
+      "name": "Awesome Project",
+      "description": "",
+      "web_url": "http://example.com/awesome_space/awesome_project",
+      "avatar_url": null,
+      "git_ssh_url": "git@example.com:awesome_space/awesome_project.git",
+      "git_http_url": "http://example.com/awesome_space/awesome_project.git",
+      "namespace": "root",
+      "visibility_level": 0,
+      "path_with_namespace": "awesome_space/awesome_project",
+      "default_branch": "master",
+      "ci_config_path": "",
+      "homepage": "http://example.com/awesome_space/awesome_project",
+      "url": "http://example.com/awesome_space/awesome_project.git",
+      "ssh_url": "git@example.com:awesome_space/awesome_project.git",
+      "http_url": "http://example.com/awesome_space/awesome_project.git"
+    },
+    "target": {
+      "name": "Awesome Project",
+      "description": "Aut reprehenderit ut est.",
+      "web_url": "http://example.com/awesome_space/awesome_project",
+      "avatar_url": null,
+      "git_ssh_url": "git@example.com:awesome_space/awesome_project.git",
+      "git_http_url": "http://example.com/awesome_space/awesome_project.git",
+      "namespace": "Awesome Space",
+      "visibility_level": 0,
+      "path_with_namespace": "awesome_space/awesome_project",
+      "default_branch": "master",
+      "ci_config_path": "",
+      "homepage": "http://example.com/awesome_space/awesome_project",
+      "url": "http://example.com/awesome_space/awesome_project.git",
+      "ssh_url": "git@example.com:awesome_space/awesome_project.git",
+      "http_url": "http://example.com/awesome_space/awesome_project.git"
+    },
+    "last_commit": {
+      "id": "ba3e0d8ff79c80d5b0bbb4f3e2e343e0aaa662b7",
+      "message": "fixed readme",
+      "timestamp": "2017-09-26T16:12:57Z",
+      "url": "http://example.com/awesome_space/awesome_project/commits/da1560886d4f094c3e6c9ef40349f7d38b5d27d7",
+      "author": {
+        "name": "GitLab dev user",
+        "email": "gitlabdev@dv6700.(none)"
+      }
+    },
+    "work_in_progress": false,
+    "total_time_spent": 0,
+    "human_total_time_spent": null,
+    "human_time_estimate": null
+  },
+  "labels": null,
+  "repository": {
+    "name": "git-gpg-test",
+    "url": "git@example.com:awesome_space/awesome_project.git",
+    "description": "",
+    "homepage": "http://example.com/awesome_space/awesome_project"
+  }
+}
+```
+
 ## Repository Update events
 
 Triggered only once when you push to the repository (including tags).
diff --git a/doc/workflow/notifications.md b/doc/workflow/notifications.md
index 3e2e7d0f7b6..0203757e0e1 100644
--- a/doc/workflow/notifications.md
+++ b/doc/workflow/notifications.md
@@ -65,7 +65,7 @@ Below is the table of events users can be notified of:
 | Group access level changed   | User                                                              | Sent when user group access level is changed |
 | Project moved                | Project members [1]                                               | [1] not disabled |
 
-### Issue / Merge Request events
+### Issue / Merge request events
 
 In all of the below cases, the notification will be sent to:
 - Participants:
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 971cb83a2d9..3f4b62dc1b2 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -65,12 +65,12 @@ module API
     end
 
     class Hook < Grape::Entity
-      expose :id, :url, :created_at, :push_events, :tag_push_events, :repository_update_events
+      expose :id, :url, :created_at, :push_events, :tag_push_events, :merge_requests_events, :repository_update_events
       expose :enable_ssl_verification
     end
 
     class ProjectHook < Hook
-      expose :project_id, :issues_events, :merge_requests_events
+      expose :project_id, :issues_events
       expose :note_events, :pipeline_events, :wiki_page_events
       expose :job_events
     end
diff --git a/lib/api/system_hooks.rb b/lib/api/system_hooks.rb
index 6b6a03e3300..c7a460df46a 100644
--- a/lib/api/system_hooks.rb
+++ b/lib/api/system_hooks.rb
@@ -26,6 +26,7 @@ module API
         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 :merge_requests_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
diff --git a/spec/controllers/admin/hooks_controller_spec.rb b/spec/controllers/admin/hooks_controller_spec.rb
index e6ba596117a..d2c1e634930 100644
--- a/spec/controllers/admin/hooks_controller_spec.rb
+++ b/spec/controllers/admin/hooks_controller_spec.rb
@@ -11,11 +11,13 @@ describe Admin::HooksController do
     it 'sets all parameters' do
       hook_params = {
         enable_ssl_verification: true,
+        token: "TEST TOKEN",
+        url: "http://example.com",
+
         push_events: true,
         tag_push_events: true,
         repository_update_events: true,
-        token: "TEST TOKEN",
-        url: "http://example.com"
+        merge_requests_events: true
       }
 
       post :create, hook: hook_params
diff --git a/spec/controllers/projects/hooks_controller_spec.rb b/spec/controllers/projects/hooks_controller_spec.rb
index aba70c6d4c1..2d473d5bf52 100644
--- a/spec/controllers/projects/hooks_controller_spec.rb
+++ b/spec/controllers/projects/hooks_controller_spec.rb
@@ -18,4 +18,30 @@ describe Projects::HooksController do
       )
     end
   end
+
+  describe '#create' do
+    it 'sets all parameters' do
+      hook_params = {
+        enable_ssl_verification: true,
+        token: "TEST TOKEN",
+        url: "http://example.com",
+
+        push_events: true,
+        tag_push_events: true,
+        merge_requests_events: true,
+        issues_events: true,
+        confidential_issues_events: true,
+        note_events: true,
+        job_events: true,
+        pipeline_events: true,
+        wiki_page_events: true
+      }
+
+      post :create, namespace_id: project.namespace, project_id: project, hook: hook_params
+
+      expect(response).to have_http_status(302)
+      expect(ProjectHook.all.size).to eq(1)
+      expect(ProjectHook.first).to have_attributes(hook_params)
+    end
+  end
 end
diff --git a/spec/features/admin/admin_hooks_spec.rb b/spec/features/admin/admin_hooks_spec.rb
index eec44549a03..f266f2ecc54 100644
--- a/spec/features/admin/admin_hooks_spec.rb
+++ b/spec/features/admin/admin_hooks_spec.rb
@@ -1,11 +1,10 @@
 require 'spec_helper'
 
-describe 'Admin::Hooks', :js do
-  before do
-    @project = create(:project)
-    sign_in(create(:admin))
+describe 'Admin::Hooks' do
+  let(:user) { create(:admin) }
 
-    @system_hook = create(:system_hook)
+  before do
+    sign_in(user)
   end
 
   describe 'GET /admin/hooks' do
@@ -13,15 +12,17 @@ describe 'Admin::Hooks', :js do
       visit admin_root_path
 
       page.within '.nav-sidebar' do
-        click_on 'Hooks'
+        click_on 'System Hooks', match: :first
       end
 
       expect(current_path).to eq(admin_hooks_path)
     end
 
     it 'has hooks list' do
+      system_hook = create(:system_hook)
+
       visit admin_hooks_path
-      expect(page).to have_content(@system_hook.url)
+      expect(page).to have_content(system_hook.url)
     end
   end
 
@@ -43,6 +44,10 @@ describe 'Admin::Hooks', :js do
   describe 'Update existing hook' do
     let(:new_url) { generate(:url) }
 
+    before do
+      create(:system_hook)
+    end
+
     it 'updates existing hook' do
       visit admin_hooks_path
 
@@ -57,7 +62,11 @@ describe 'Admin::Hooks', :js do
     end
   end
 
-  describe 'Remove existing hook' do
+  describe 'Remove existing hook', :js do
+    before do
+      create(:system_hook)
+    end
+
     context 'removes existing hook' do
       it 'from hooks list page' do
         visit admin_hooks_path
@@ -76,7 +85,8 @@ describe 'Admin::Hooks', :js do
 
   describe 'Test', :js do
     before do
-      WebMock.stub_request(:post, @system_hook.url)
+      system_hook = create(:system_hook)
+      WebMock.stub_request(:post, system_hook.url)
       visit admin_hooks_path
 
       find('.hook-test-button.dropdown').click
@@ -85,4 +95,41 @@ describe 'Admin::Hooks', :js do
 
     it { expect(current_path).to eq(admin_hooks_path) }
   end
+
+  context 'Merge request hook' do
+    describe 'New Hook' do
+      let(:url) { generate(:url) }
+
+      it 'adds new hook' do
+        visit admin_hooks_path
+
+        fill_in 'hook_url', with: url
+        uncheck 'Repository update events'
+        check 'Merge request events'
+
+        expect { click_button 'Add system hook' }.to change(SystemHook, :count).by(1)
+        expect(current_path).to eq(admin_hooks_path)
+        expect(page).to have_content(url)
+      end
+    end
+
+    describe 'Test', :js do
+      before do
+        system_hook = create(:system_hook)
+        WebMock.stub_request(:post, system_hook.url)
+      end
+
+      it 'succeeds if the user has a repository with a merge request' do
+        project = create(:project, :repository)
+        create(:project_member, user: user, project: project)
+        create(:merge_request, source_project: project)
+
+        visit admin_hooks_path
+        find('.hook-test-button.dropdown').click
+        click_link 'Merge requests events'
+
+        expect(page).to have_content 'Hook executed successfully'
+      end
+    end
+  end
 end
diff --git a/spec/models/concerns/triggerable_hooks_spec.rb b/spec/models/concerns/triggerable_hooks_spec.rb
new file mode 100644
index 00000000000..621d2d38eae
--- /dev/null
+++ b/spec/models/concerns/triggerable_hooks_spec.rb
@@ -0,0 +1,43 @@
+require 'rails_helper'
+
+RSpec.describe TriggerableHooks do
+  before do
+    class TestableHook < WebHook
+      include TriggerableHooks
+      triggerable_hooks [:push_hooks]
+    end
+  end
+
+  describe 'scopes' do
+    it 'defines a scope for each of the requested triggers' do
+      expect(TestableHook).to respond_to :push_hooks
+      expect(TestableHook).not_to respond_to :tag_push_hooks
+    end
+  end
+
+  describe '.hooks_for' do
+    context 'the model has the required trigger scope' do
+      it 'returns the record' do
+        hook = TestableHook.create!(url: 'http://example.com', push_events: true)
+
+        expect(TestableHook.hooks_for(:push_hooks)).to eq [hook]
+      end
+    end
+
+    context 'the model does not have the required trigger scope' do
+      it 'returns an empty relation' do
+        TestableHook.create!(url: 'http://example.com')
+
+        expect(TestableHook.hooks_for(:tag_push_hooks)).to eq []
+      end
+    end
+
+    context 'the stock scope ".all" is accepted' do
+      it 'returns the record' do
+        hook = TestableHook.create!(url: 'http://example.com')
+
+        expect(TestableHook.hooks_for(:all)).to eq [hook]
+      end
+    end
+  end
+end
diff --git a/spec/models/hooks/system_hook_spec.rb b/spec/models/hooks/system_hook_spec.rb
index 0e965f541d8..8bc45715dcd 100644
--- a/spec/models/hooks/system_hook_spec.rb
+++ b/spec/models/hooks/system_hook_spec.rb
@@ -7,7 +7,8 @@ describe SystemHook do
     it 'sets defined default parameters' do
       attrs = {
         push_events: false,
-        repository_update_events: true
+        repository_update_events: true,
+        merge_requests_events: false
       }
       expect(system_hook).to have_attributes(attrs)
     end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 78223c44999..987be8e8b46 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -3206,4 +3206,23 @@ describe Project do
       expect { project.write_repository_config }.not_to raise_error
     end
   end
+
+  describe '#execute_hooks' do
+    it 'executes the projects hooks with the specified scope' do
+      hook1 = create(:project_hook, merge_requests_events: true, tag_push_events: false)
+      hook2 = create(:project_hook, merge_requests_events: false, tag_push_events: true)
+      project = create(:project, hooks: [hook1, hook2])
+
+      expect_any_instance_of(ProjectHook).to receive(:async_execute).once
+
+      project.execute_hooks({}, :tag_push_hooks)
+    end
+
+    it 'executes the system hooks with the specified scope' do
+      expect_any_instance_of(SystemHooksService).to receive(:execute_hooks).with({ data: 'data' }, :merge_request_hooks)
+
+      project = build(:project)
+      project.execute_hooks({ data: 'data' }, :merge_request_hooks)
+    end
+  end
 end
diff --git a/spec/requests/api/system_hooks_spec.rb b/spec/requests/api/system_hooks_spec.rb
index c7a009e999e..6c57d443cbf 100644
--- a/spec/requests/api/system_hooks_spec.rb
+++ b/spec/requests/api/system_hooks_spec.rb
@@ -36,6 +36,7 @@ describe API::SystemHooks do
         expect(json_response.first['url']).to eq(hook.url)
         expect(json_response.first['push_events']).to be false
         expect(json_response.first['tag_push_events']).to be false
+        expect(json_response.first['merge_requests_events']).to be false
         expect(json_response.first['repository_update_events']).to be true
       end
     end
@@ -67,11 +68,28 @@ describe API::SystemHooks do
     end
 
     it 'sets default values for events' do
-      post api('/hooks', admin), url: 'http://mep.mep', enable_ssl_verification: true
+      post api('/hooks', admin), url: 'http://mep.mep'
 
       expect(response).to have_gitlab_http_status(201)
       expect(json_response['enable_ssl_verification']).to be true
+      expect(json_response['push_events']).to be false
       expect(json_response['tag_push_events']).to be false
+      expect(json_response['merge_requests_events']).to be false
+    end
+
+    it 'sets explicit values for events' do
+      post api('/hooks', admin),
+        url: 'http://mep.mep',
+        enable_ssl_verification: false,
+        push_events: true,
+        tag_push_events: true,
+        merge_requests_events: true
+
+      expect(response).to have_http_status(201)
+      expect(json_response['enable_ssl_verification']).to be false
+      expect(json_response['push_events']).to be true
+      expect(json_response['tag_push_events']).to be true
+      expect(json_response['merge_requests_events']).to be true
     end
   end
 
diff --git a/spec/services/test_hooks/system_service_spec.rb b/spec/services/test_hooks/system_service_spec.rb
index ff8b9595538..74d7715e50f 100644
--- a/spec/services/test_hooks/system_service_spec.rb
+++ b/spec/services/test_hooks/system_service_spec.rb
@@ -60,5 +60,25 @@ describe TestHooks::SystemService do
         expect(service.execute).to include(success_result)
       end
     end
+
+    context 'merge_requests_events' do
+      let(:trigger) { 'merge_requests_events' }
+
+      it 'returns error message if the user does not have any repository with a merge request' do
+        expect(hook).not_to receive(:execute)
+        expect(service.execute).to include({ status: :error, message: 'Ensure one of your projects has merge requests.' })
+      end
+
+      it 'executes hook' do
+        trigger_key = :merge_request_hooks
+        sample_data = { data: 'sample' }
+        create(:project_member, user: current_user, project: project)
+        create(:merge_request, source_project: project)
+        allow_any_instance_of(MergeRequest).to receive(:to_hook_data).and_return(sample_data)
+
+        expect(hook).to receive(:execute).with(sample_data, trigger_key).and_return(success_result)
+        expect(service.execute).to include(success_result)
+      end
+    end
   end
 end