Commit 8e9c073a authored by Douwe Maan's avatar Douwe Maan

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

System hooks for Merge Requests

See merge request gitlab-org/gitlab-ce!14387
parents d617c24f 6ef1b94c
......@@ -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
......@@ -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
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
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
......
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
......@@ -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)
......
......@@ -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
......
......@@ -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)
......
......@@ -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
......@@ -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
......
......@@ -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
......
......@@ -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'}
......@@ -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
......
......@@ -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')
......@@ -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
......
---
title: System hooks for Merge Requests
merge_request: 14387
author: Alexis Reigel
type: added
......@@ -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
}
]
......
......@@ -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).
......
......@@ -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:
......
......@@ -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
......
......@@ -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
......
......@@ -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
......
......@@ -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
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
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
......@@ -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
......
......@@ -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
......@@ -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
......
......@@ -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
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment