From 169b4ce0ca2ef04d37deb6bdb054cb8124a9db61 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 2 Dec 2013 21:33:43 +0200 Subject: [PATCH 01/12] Add new fields to web_hooks table. Next fields were added: * push_events * issues_events * merge_requests_events Main goal is to create web hooks that can be triggered only for certain event types. For example you can have separate web hook for push events and another one for issues. Signed-off-by: Dmitriy Zaporozhets --- .../20131202192556_add_event_fields_for_web_hook.rb | 7 +++++++ db/schema.rb | 11 +++++++---- 2 files changed, 14 insertions(+), 4 deletions(-) create mode 100644 db/migrate/20131202192556_add_event_fields_for_web_hook.rb diff --git a/db/migrate/20131202192556_add_event_fields_for_web_hook.rb b/db/migrate/20131202192556_add_event_fields_for_web_hook.rb new file mode 100644 index 0000000000..d29e996852 --- /dev/null +++ b/db/migrate/20131202192556_add_event_fields_for_web_hook.rb @@ -0,0 +1,7 @@ +class AddEventFieldsForWebHook < ActiveRecord::Migration + def change + add_column :web_hooks, :push_events, :boolean, default: true, null: false + add_column :web_hooks, :issues_events, :boolean, default: false, null: false + add_column :web_hooks, :merge_requests_events, :boolean, default: false, null: false + end +end diff --git a/db/schema.rb b/db/schema.rb index 24d2fef4a1..6219ce7769 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended to check this file into your version control system. -ActiveRecord::Schema.define(:version => 20131112220935) do +ActiveRecord::Schema.define(:version => 20131202192556) do create_table "broadcast_messages", :force => true do |t| t.text "message", :null => false @@ -334,10 +334,13 @@ ActiveRecord::Schema.define(:version => 20131112220935) do create_table "web_hooks", :force => true do |t| t.string "url" t.integer "project_id" - t.datetime "created_at", :null => false - t.datetime "updated_at", :null => false - t.string "type", :default => "ProjectHook" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false + t.string "type", :default => "ProjectHook" t.integer "service_id" + t.boolean "push_events", :default => true, :null => false + t.boolean "issues_events", :default => false, :null => false + t.boolean "merge_requests_events", :default => false, :null => false end add_index "web_hooks", ["project_id"], :name => "index_web_hooks_on_project_id" From 0687ecb66d52868db27ef89a18e4109abc70b935 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 2 Dec 2013 21:38:32 +0200 Subject: [PATCH 02/12] UI with event types for Project WebHooks Signed-off-by: Dmitriy Zaporozhets --- .../stylesheets/gitlab_bootstrap/forms.scss | 7 +++ app/models/project_hook.rb | 2 + app/views/projects/hooks/index.html.haml | 43 +++++++++++++++---- 3 files changed, 44 insertions(+), 8 deletions(-) diff --git a/app/assets/stylesheets/gitlab_bootstrap/forms.scss b/app/assets/stylesheets/gitlab_bootstrap/forms.scss index d4da3fe713..1a310a75c3 100644 --- a/app/assets/stylesheets/gitlab_bootstrap/forms.scss +++ b/app/assets/stylesheets/gitlab_bootstrap/forms.scss @@ -13,6 +13,13 @@ form { margin-top: 1px !important; } } + + &.list-label { + float: none; + padding: 0 !important; + margin: 0; + text-align: left; + } } } diff --git a/app/models/project_hook.rb b/app/models/project_hook.rb index 2576fc979d..4edb38d3bf 100644 --- a/app/models/project_hook.rb +++ b/app/models/project_hook.rb @@ -13,4 +13,6 @@ class ProjectHook < WebHook belongs_to :project + + attr_accessible :push_events, :issues_events, :merge_requests_events end diff --git a/app/views/projects/hooks/index.html.haml b/app/views/projects/hooks/index.html.haml index f748eb2929..e1166742b2 100644 --- a/app/views/projects/hooks/index.html.haml +++ b/app/views/projects/hooks/index.html.haml @@ -1,9 +1,9 @@ %h3.page-title - Post-receive hooks + Web hooks %p.light - #{link_to "Post-receive hooks ", help_web_hooks_path, class: "vlink"} can be - used for binding events when someone pushes to the repository. + #{link_to "Web hooks ", help_web_hooks_path, class: "vlink"} can be + used for binding events when something happends to the the project. %hr.clearfix @@ -13,23 +13,50 @@ - @hook.errors.full_messages.each do |msg| %p= msg .control-group - = f.label :url, "URL:" + = f.label :url, "URL" .controls = f.text_field :url, class: "text_field input-xxlarge input-xpadding", placeholder: 'http://example.com/trigger-ci.json'   = f.submit "Add Web Hook", class: "btn btn-create" + .control-group + = f.label :url, "Trigger" + .controls + %div + = f.check_box :push_events, class: 'pull-left' + .prepend-left-20 + = f.label :push_events, class: 'list-label' do + %strong Push events + %p.light + This url will be triggered in case of push to repository + %div + = f.check_box :issues_events, class: 'pull-left' + .prepend-left-20 + = f.label :issues_events, class: 'list-label' do + %strong Issues events + %p.light + This url will be triggered for created issues + %div + = f.check_box :merge_requests_events, class: 'pull-left' + .prepend-left-20 + = f.label :merge_requests_events, class: 'list-label' do + %strong Merge Request events + %p.light + This url will be triggered for created merge requests %hr -if @hooks.any? .ui-box .title - Hooks (#{@hooks.count}) + Web Hooks (#{@hooks.count}) %ul.well-list - @hooks.each do |hook| %li - %span.badge.badge-info POST - → - %span.monospace= hook.url .pull-right = link_to 'Test Hook', test_project_hook_path(@project, hook), class: "btn btn-small grouped" = link_to 'Remove', project_hook_path(@project, hook), confirm: 'Are you sure?', method: :delete, class: "btn btn-remove btn-small grouped" + .clearfix + %span.monospace= hook.url + %p + - %w(push_events issues_events merge_requests_events).each do |trigger| + - if hook.send(trigger) + %span.label.label-gray= trigger.titleize From 25951b914619a9e056122f0c39ce64e4afe453d8 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 3 Dec 2013 11:31:56 +0200 Subject: [PATCH 03/12] Prepare ProjectHooks to work with issues and merge_requests * Add event scopes to ProjectHook * Added Issuable#to_hook_data * Project#execute_hooks now accept hook filter as argument Signed-off-by: Dmitriy Zaporozhets --- app/models/concerns/issuable.rb | 7 +++++++ app/models/project.rb | 6 ++++-- app/models/project_hook.rb | 4 ++++ 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 7f820f950b..58bf621f91 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -111,4 +111,11 @@ module Issuable end users.concat(mentions.reduce([], :|)).uniq end + + def to_hook_data + { + object_kind: self.class.name.underscore, + object_attributes: self.attributes + } + end end diff --git a/app/models/project.rb b/app/models/project.rb index 0edc0746fb..ed30b5ea89 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -298,8 +298,10 @@ class Project < ActiveRecord::Base ProjectTransferService.new.transfer(self, new_namespace) end - def execute_hooks(data) - hooks.each { |hook| hook.async_execute(data) } + def execute_hooks(data, hooks_scope = :push_hooks) + hooks.send(hooks_scope).each do |hook| + hook.async_execute(data) + end end def execute_services(data) diff --git a/app/models/project_hook.rb b/app/models/project_hook.rb index 4edb38d3bf..c5ef13a2b8 100644 --- a/app/models/project_hook.rb +++ b/app/models/project_hook.rb @@ -15,4 +15,8 @@ class ProjectHook < WebHook belongs_to :project attr_accessible :push_events, :issues_events, :merge_requests_events + + scope :push_hooks, -> { where(push_events: true) } + scope :issue_hooks, -> { where(issues_events: true) } + scope :merge_request_hooks, -> { where(merge_requests_events: true) } end From cfcf24dce53d79c950de12eca0353b890ad95508 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 3 Dec 2013 11:34:06 +0200 Subject: [PATCH 04/12] Execute project hooks when issue or merge request created Signed-off-by: Dmitriy Zaporozhets --- app/observers/issue_observer.rb | 8 ++++-- app/observers/merge_request_observer.rb | 21 +++++++++++++--- app/services/git_push_service.rb | 2 +- spec/services/git_push_service_spec.rb | 33 ++++++------------------- 4 files changed, 31 insertions(+), 33 deletions(-) diff --git a/app/observers/issue_observer.rb b/app/observers/issue_observer.rb index 886d8b776f..b150e39e23 100644 --- a/app/observers/issue_observer.rb +++ b/app/observers/issue_observer.rb @@ -1,14 +1,14 @@ class IssueObserver < BaseObserver def after_create(issue) notification.new_issue(issue, current_user) - issue.create_cross_references!(issue.project, current_user) + execute_hooks(issue) end def after_close(issue, transition) notification.close_issue(issue, current_user) - create_note(issue) + execute_hooks(issue) end def after_reopen(issue, transition) @@ -29,4 +29,8 @@ class IssueObserver < BaseObserver def create_note(issue) Note.create_status_change_note(issue, issue.project, current_user, issue.state, current_commit) end + + def execute_hooks(issue) + issue.project.execute_hooks(issue.to_hook_data, :issue_hooks) + end end diff --git a/app/observers/merge_request_observer.rb b/app/observers/merge_request_observer.rb index d70da514cd..9e41f8c38a 100644 --- a/app/observers/merge_request_observer.rb +++ b/app/observers/merge_request_observer.rb @@ -7,15 +7,15 @@ class MergeRequestObserver < ActivityObserver end notification.new_merge_request(merge_request, current_user) - merge_request.create_cross_references!(merge_request.project, current_user) + execute_hooks(merge_request) end def after_close(merge_request, transition) create_event(merge_request, Event::CLOSED) - Note.create_status_change_note(merge_request, merge_request.target_project, current_user, merge_request.state, nil) - notification.close_mr(merge_request, current_user) + create_note(merge_request) + execute_hooks(merge_request) end def after_merge(merge_request, transition) @@ -31,11 +31,13 @@ class MergeRequestObserver < ActivityObserver action: Event::MERGED, author_id: merge_request.author_id_of_changes ) + + execute_hooks(merge_request) end def after_reopen(merge_request, transition) create_event(merge_request, Event::REOPENED) - Note.create_status_change_note(merge_request, merge_request.target_project, current_user, merge_request.state, nil) + create_note(merge_request) end def after_update(merge_request) @@ -53,4 +55,15 @@ class MergeRequestObserver < ActivityObserver author_id: current_user.id ) end + + private + + # Create merge request note with service comment like 'Status changed to closed' + def create_note(merge_request) + Note.create_status_change_note(merge_request, merge_request.target_project, current_user, merge_request.state, nil) + end + + def execute_hooks(merge_request) + merge_request.project.execute_hooks(merge_request.to_hook_data, :merge_request_hooks) + end end diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb index e732b5fd1e..e54f88e42d 100644 --- a/app/services/git_push_service.rb +++ b/app/services/git_push_service.rb @@ -32,7 +32,7 @@ class GitPushService end if push_to_branch?(ref) - project.execute_hooks(@push_data.dup) + project.execute_hooks(@push_data.dup, :push_hooks) project.execute_services(@push_data.dup) end diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb index 2870f59195..b46022fb2d 100644 --- a/spec/services/git_push_service_spec.rb +++ b/spec/services/git_push_service_spec.rb @@ -74,38 +74,19 @@ describe GitPushService do end describe "Web Hooks" do - context "with web hooks" do - before do - @project_hook = create(:project_hook) - @project_hook_2 = create(:project_hook) - project.hooks << [@project_hook, @project_hook_2] - - stub_request(:post, @project_hook.url) - stub_request(:post, @project_hook_2.url) - end - - it "executes multiple web hook" do - @project_hook.should_receive(:async_execute).once - @project_hook_2.should_receive(:async_execute).once - - service.execute(project, user, @oldrev, @newrev, @ref) - end - end - context "execute web hooks" do - before do - @project_hook = create(:project_hook) - project.hooks << [@project_hook] - stub_request(:post, @project_hook.url) - end - it "when pushing a branch for the first time" do - @project_hook.should_receive(:async_execute) + project.should_receive(:execute_hooks) service.execute(project, user, @blankrev, 'newrev', 'refs/heads/master') end + it "when pushing new commits to existing branch" do + project.should_receive(:execute_hooks) + service.execute(project, user, 'oldrev', 'newrev', 'refs/heads/master') + end + it "when pushing tags" do - @project_hook.should_not_receive(:async_execute) + project.should_not_receive(:execute_hooks) service.execute(project, user, 'newrev', 'newrev', 'refs/tags/v1.0.0') end end From 5f9cdbab1d8373f3455c1cd8136db6479a54adf7 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 3 Dec 2013 12:08:00 +0200 Subject: [PATCH 05/12] Modify test to pass new project hooks Signed-off-by: Dmitriy Zaporozhets --- app/observers/merge_request_observer.rb | 4 +++- spec/observers/merge_request_observer_spec.rb | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/app/observers/merge_request_observer.rb b/app/observers/merge_request_observer.rb index 9e41f8c38a..9649219811 100644 --- a/app/observers/merge_request_observer.rb +++ b/app/observers/merge_request_observer.rb @@ -64,6 +64,8 @@ class MergeRequestObserver < ActivityObserver end def execute_hooks(merge_request) - merge_request.project.execute_hooks(merge_request.to_hook_data, :merge_request_hooks) + if merge_request.project + merge_request.project.execute_hooks(merge_request.to_hook_data, :merge_request_hooks) + end end end diff --git a/spec/observers/merge_request_observer_spec.rb b/spec/observers/merge_request_observer_spec.rb index 3f5250a004..3e5cdfaf5d 100644 --- a/spec/observers/merge_request_observer_spec.rb +++ b/spec/observers/merge_request_observer_spec.rb @@ -4,7 +4,7 @@ describe MergeRequestObserver do let(:some_user) { create :user } let(:assignee) { create :user } let(:author) { create :user } - let(:mr_mock) { double(:merge_request, id: 42, assignee: assignee, author: author) } + let(:mr_mock) { double(:merge_request, id: 42, assignee: assignee, author: author).as_null_object } let(:assigned_mr) { create(:merge_request, assignee: assignee, author: author, target_project: create(:project)) } let(:unassigned_mr) { create(:merge_request, author: author, target_project: create(:project)) } let(:closed_assigned_mr) { create(:closed_merge_request, assignee: assignee, author: author, target_project: create(:project)) } From d4c50dbd33e03cb6b5ec06f9411bb77afb70e5e4 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 3 Dec 2013 19:11:10 +0200 Subject: [PATCH 06/12] Annotate web_hooks table Signed-off-by: Dmitriy Zaporozhets --- app/models/project_hook.rb | 17 ++++++++++------- app/models/service_hook.rb | 17 ++++++++++------- app/models/system_hook.rb | 17 ++++++++++------- app/models/web_hook.rb | 17 ++++++++++------- spec/models/service_hook_spec.rb | 17 ++++++++++------- spec/models/system_hook_spec.rb | 17 ++++++++++------- spec/models/web_hook_spec.rb | 17 ++++++++++------- 7 files changed, 70 insertions(+), 49 deletions(-) diff --git a/app/models/project_hook.rb b/app/models/project_hook.rb index c5ef13a2b8..e1c9ed01bc 100644 --- a/app/models/project_hook.rb +++ b/app/models/project_hook.rb @@ -2,13 +2,16 @@ # # Table name: web_hooks # -# id :integer not null, primary key -# url :string(255) -# project_id :integer -# created_at :datetime not null -# updated_at :datetime not null -# type :string(255) default("ProjectHook") -# service_id :integer +# id :integer not null, primary key +# url :string(255) +# project_id :integer +# created_at :datetime not null +# updated_at :datetime not null +# type :string(255) default("ProjectHook") +# service_id :integer +# push_events :boolean default(TRUE), not null +# issues_events :boolean default(FALSE), not null +# merge_requests_events :boolean default(FALSE), not null # class ProjectHook < WebHook diff --git a/app/models/service_hook.rb b/app/models/service_hook.rb index 4cd2b272ee..6f22a863d9 100644 --- a/app/models/service_hook.rb +++ b/app/models/service_hook.rb @@ -2,13 +2,16 @@ # # Table name: web_hooks # -# id :integer not null, primary key -# url :string(255) -# project_id :integer -# created_at :datetime not null -# updated_at :datetime not null -# type :string(255) default("ProjectHook") -# service_id :integer +# id :integer not null, primary key +# url :string(255) +# project_id :integer +# created_at :datetime not null +# updated_at :datetime not null +# type :string(255) default("ProjectHook") +# service_id :integer +# push_events :boolean default(TRUE), not null +# issues_events :boolean default(FALSE), not null +# merge_requests_events :boolean default(FALSE), not null # class ServiceHook < WebHook diff --git a/app/models/system_hook.rb b/app/models/system_hook.rb index 5cdf046644..bffcbbf00f 100644 --- a/app/models/system_hook.rb +++ b/app/models/system_hook.rb @@ -2,13 +2,16 @@ # # Table name: web_hooks # -# id :integer not null, primary key -# url :string(255) -# project_id :integer -# created_at :datetime not null -# updated_at :datetime not null -# type :string(255) default("ProjectHook") -# service_id :integer +# id :integer not null, primary key +# url :string(255) +# project_id :integer +# created_at :datetime not null +# updated_at :datetime not null +# type :string(255) default("ProjectHook") +# service_id :integer +# push_events :boolean default(TRUE), not null +# issues_events :boolean default(FALSE), not null +# merge_requests_events :boolean default(FALSE), not null # class SystemHook < WebHook diff --git a/app/models/web_hook.rb b/app/models/web_hook.rb index 3f22b1082f..c0aa373491 100644 --- a/app/models/web_hook.rb +++ b/app/models/web_hook.rb @@ -2,13 +2,16 @@ # # Table name: web_hooks # -# id :integer not null, primary key -# url :string(255) -# project_id :integer -# created_at :datetime not null -# updated_at :datetime not null -# type :string(255) default("ProjectHook") -# service_id :integer +# id :integer not null, primary key +# url :string(255) +# project_id :integer +# created_at :datetime not null +# updated_at :datetime not null +# type :string(255) default("ProjectHook") +# service_id :integer +# push_events :boolean default(TRUE), not null +# issues_events :boolean default(FALSE), not null +# merge_requests_events :boolean default(FALSE), not null # class WebHook < ActiveRecord::Base diff --git a/spec/models/service_hook_spec.rb b/spec/models/service_hook_spec.rb index 0b0262c97f..40a5fbc71d 100644 --- a/spec/models/service_hook_spec.rb +++ b/spec/models/service_hook_spec.rb @@ -2,13 +2,16 @@ # # Table name: web_hooks # -# id :integer not null, primary key -# url :string(255) -# project_id :integer -# created_at :datetime not null -# updated_at :datetime not null -# type :string(255) default("ProjectHook") -# service_id :integer +# id :integer not null, primary key +# url :string(255) +# project_id :integer +# created_at :datetime not null +# updated_at :datetime not null +# type :string(255) default("ProjectHook") +# service_id :integer +# push_events :boolean default(TRUE), not null +# issues_events :boolean default(FALSE), not null +# merge_requests_events :boolean default(FALSE), not null # require "spec_helper" diff --git a/spec/models/system_hook_spec.rb b/spec/models/system_hook_spec.rb index a9ed6a5fa7..6a0d99dcc5 100644 --- a/spec/models/system_hook_spec.rb +++ b/spec/models/system_hook_spec.rb @@ -2,13 +2,16 @@ # # Table name: web_hooks # -# id :integer not null, primary key -# url :string(255) -# project_id :integer -# created_at :datetime not null -# updated_at :datetime not null -# type :string(255) default("ProjectHook") -# service_id :integer +# id :integer not null, primary key +# url :string(255) +# project_id :integer +# created_at :datetime not null +# updated_at :datetime not null +# type :string(255) default("ProjectHook") +# service_id :integer +# push_events :boolean default(TRUE), not null +# issues_events :boolean default(FALSE), not null +# merge_requests_events :boolean default(FALSE), not null # require "spec_helper" diff --git a/spec/models/web_hook_spec.rb b/spec/models/web_hook_spec.rb index 2d9301732d..d603408101 100644 --- a/spec/models/web_hook_spec.rb +++ b/spec/models/web_hook_spec.rb @@ -2,13 +2,16 @@ # # Table name: web_hooks # -# id :integer not null, primary key -# url :string(255) -# project_id :integer -# created_at :datetime not null -# updated_at :datetime not null -# type :string(255) default("ProjectHook") -# service_id :integer +# id :integer not null, primary key +# url :string(255) +# project_id :integer +# created_at :datetime not null +# updated_at :datetime not null +# type :string(255) default("ProjectHook") +# service_id :integer +# push_events :boolean default(TRUE), not null +# issues_events :boolean default(FALSE), not null +# merge_requests_events :boolean default(FALSE), not null # require 'spec_helper' From 887c0c67acdc4f9b847ea27d293328b09e703d8d Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 3 Dec 2013 19:25:16 +0200 Subject: [PATCH 07/12] Update WebHook help page Signed-off-by: Dmitriy Zaporozhets --- app/views/help/web_hooks.html.haml | 64 +++++++++++++++++++++++++++--- 1 file changed, 58 insertions(+), 6 deletions(-) diff --git a/app/views/help/web_hooks.html.haml b/app/views/help/web_hooks.html.haml index 25865251de..4435648be4 100644 --- a/app/views/help/web_hooks.html.haml +++ b/app/views/help/web_hooks.html.haml @@ -1,12 +1,64 @@ = render layout: 'help/layout' do - %h3.page-title Web hooks + %h3.page-title Project web hooks + %p.light + Project web hooks allow you to trigger url if new code is pushed or new issue is created + %hr - %p.slead - Every GitLab project can trigger a web server whenever the repo is pushed to. + %p + You can configure web hook to listen for specific events like pushes, issues, merge requests. + %br + GitLab will send POST request with data to web hook url. %br Web Hooks can be used to update an external issue tracker, trigger CI builds, update a backup mirror, or even deploy to your production server. - %br - GitLab will send POST request with commits information on every push. - %h5 Hooks request example: + + + %h4 Push events = render "projects/hooks/data_ex" + + %h4 Issues events + %pre + :preserve + { + "object_kind":"issue", + "object_attributes":{ + "id":301, + "title":"New API: create/update/delete file", + "assignee_id":51, + "author_id":51, + "project_id":14, + "created_at":"2013-12-03T17:15:43Z", + "updated_at":"2013-12-03T17:15:43Z", + "position":0, + "branch_name":null, + "description":"Create new API for manipulations with repository", + "milestone_id":null, + "state":"opened", + "iid":23 + } + } + %h4 Merge request events + %pre + :preserve + { + "object_kind":"merge_request", + "object_attributes":{ + "id":99, + "target_branch":"master", + "source_branch":"ms-viewport", + "source_project_id":14, + "author_id":51, + "assignee_id":6, + "title":"MS-Viewport", + "created_at":"2013-12-03T17:23:34Z", + "updated_at":"2013-12-03T17:23:34Z", + "st_commits":null, + "st_diffs":null, + "milestone_id":null, + "state":"opened", + "merge_status":"unchecked", + "target_project_id":14, + "iid":1, + "description":"" + } + } From cf5d3bb0add2344ea6fd31520535fd2f382ff637 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 3 Dec 2013 21:59:06 +0200 Subject: [PATCH 08/12] Improve help page for Project web hooks Signed-off-by: Dmitriy Zaporozhets --- app/helpers/application_helper.rb | 8 +++ app/views/help/web_hooks.html.haml | 69 +++++++++++++++++++--- app/views/projects/hooks/_data_ex.html.erb | 44 -------------- 3 files changed, 68 insertions(+), 53 deletions(-) delete mode 100644 app/views/projects/hooks/_data_ex.html.erb diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 04fda026bf..4b9514750a 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -207,4 +207,12 @@ module ApplicationHelper def broadcast_message BroadcastMessage.current end + + def highlight_js(&block) + string = capture(&block) + + content_tag :div, class: user_color_scheme_class do + Pygments::Lexer[:js].highlight(string).html_safe + end + end end diff --git a/app/views/help/web_hooks.html.haml b/app/views/help/web_hooks.html.haml index 4435648be4..66ab7b75bd 100644 --- a/app/views/help/web_hooks.html.haml +++ b/app/views/help/web_hooks.html.haml @@ -4,21 +4,68 @@ Project web hooks allow you to trigger url if new code is pushed or new issue is created %hr - %p + %p.slead You can configure web hook to listen for specific events like pushes, issues, merge requests. %br GitLab will send POST request with data to web hook url. %br Web Hooks can be used to update an external issue tracker, trigger CI builds, update a backup mirror, or even deploy to your production server. - + %hr %h4 Push events - = render "projects/hooks/data_ex" + %p.light + Triggered when you push to the repository except pushing tags. + %br + Request body: + = highlight_js do + :erb + { + "before": "95790bf891e76fee5e1747ab589903a6a1f80f22", + "after": "da1560886d4f094c3e6c9ef40349f7d38b5d27d7", + "ref": "refs/heads/master", + "user_id": 4, + "user_name": "John Smith", + "project_id": 15, + "repository": { + "name": "Diaspora", + "url": "git@localhost:diaspora.git", + "description": "", + "homepage": "http://localhost/diaspora", + }, + "commits": [ + { + "id": "b6568db1bc1dcd7f8b4d5a946b0b91f9dacd7327", + "message": "Update Catalan translation to e38cb41.", + "timestamp": "2011-12-12T14:27:31+02:00", + "url": "http://localhost/diaspora/commits/b6568db1bc1dcd7f8b4d5a946b0b91f9dacd7327", + "author": { + "name": "Jordi Mallach", + "email": "jordi@softcatala.org", + } + }, + // ... + { + "id": "da1560886d4f094c3e6c9ef40349f7d38b5d27d7", + "message": "fixed readme", + "timestamp": "2012-01-03T23:36:29+02:00", + "url": "http://localhost/diaspora/commits/da1560886d4f094c3e6c9ef40349f7d38b5d27d7", + "author": { + "name": "GitLab dev user", + "email": "gitlabdev@dv6700.(none)", + }, + }, + ], + "total_commits_count": 4, + }; - %h4 Issues events - %pre - :preserve + %h4.prepend-top-20 Issues events + %p.light + Triggered when new issue created or existing issue was closed. + %br + Request body: + = highlight_js do + :erb { "object_kind":"issue", "object_attributes":{ @@ -37,9 +84,13 @@ "iid":23 } } - %h4 Merge request events - %pre - :preserve + %h4.prepend-top-20 Merge request events + %p.light + Triggered when new merge request created or existing merge request was merged/closed. + %br + Request body: + = highlight_js do + :erb { "object_kind":"merge_request", "object_attributes":{ diff --git a/app/views/projects/hooks/_data_ex.html.erb b/app/views/projects/hooks/_data_ex.html.erb deleted file mode 100644 index 5092aaf675..0000000000 --- a/app/views/projects/hooks/_data_ex.html.erb +++ /dev/null @@ -1,44 +0,0 @@ -<% data_ex_str = < -
- <%= raw Pygments::Lexer[:js].highlight(data_ex_str) %> -
From fff69bdb97d9038976b8a5716238c796487dba50 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 4 Dec 2013 13:02:16 +0200 Subject: [PATCH 09/12] Mention advanced project hooks in CHANGELOG Signed-off-by: Dmitriy Zaporozhets --- CHANGELOG | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG b/CHANGELOG index 884bb25328..d0a796fb4c 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -7,6 +7,7 @@ v 6.4.0 - Side-by-side diff view (Steven Thonus) - Internal projects (Jason Hollingsworth) - Allow removal of avatar (Drew Blessing) + - Project web hooks now support issues and merge request events v 6.3.0 - API for adding gitlab-ci service From 882029d9c62c54293147da0d5e2d3b6eaaa89535 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 4 Dec 2013 13:19:53 +0200 Subject: [PATCH 10/12] Expose ProjectHook attributes via API Signed-off-by: Dmitriy Zaporozhets --- lib/api/entities.rb | 4 ++++ lib/api/project_hooks.rb | 8 ++++---- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 90cb69760a..7daf8ace24 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -24,6 +24,10 @@ module API expose :id, :url, :created_at end + class ProjectHook < Hook + expose :project_id, :push_events, :issues_events, :merge_requests_events + end + class ForkedFromProject < Grape::Entity expose :id expose :name, :name_with_namespace diff --git a/lib/api/project_hooks.rb b/lib/api/project_hooks.rb index 738974955f..e5da15d3ea 100644 --- a/lib/api/project_hooks.rb +++ b/lib/api/project_hooks.rb @@ -22,7 +22,7 @@ module API # GET /projects/:id/hooks get ":id/hooks" do @hooks = paginate user_project.hooks - present @hooks, with: Entities::Hook + present @hooks, with: Entities::ProjectHook end # Get a project hook @@ -34,7 +34,7 @@ module API # GET /projects/:id/hooks/:hook_id get ":id/hooks/:hook_id" do @hook = user_project.hooks.find(params[:hook_id]) - present @hook, with: Entities::Hook + present @hook, with: Entities::ProjectHook end @@ -50,7 +50,7 @@ module API @hook = user_project.hooks.new({"url" => params[:url]}) if @hook.save - present @hook, with: Entities::Hook + present @hook, with: Entities::ProjectHook else if @hook.errors[:url].present? error!("Invalid url given", 422) @@ -73,7 +73,7 @@ module API attrs = attributes_for_keys [:url] if @hook.update_attributes attrs - present @hook, with: Entities::Hook + present @hook, with: Entities::ProjectHook else if @hook.errors[:url].present? error!("Invalid url given", 422) From 822dac87eccad5044e0e82fd671e8b71f21942a5 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 4 Dec 2013 13:20:16 +0200 Subject: [PATCH 11/12] Extract tests for project hooks API Signed-off-by: Dmitriy Zaporozhets --- spec/requests/api/project_hooks_spec.rb | 132 ++++++++++++++++++++++++ spec/requests/api/projects_spec.rb | 116 --------------------- 2 files changed, 132 insertions(+), 116 deletions(-) create mode 100644 spec/requests/api/project_hooks_spec.rb diff --git a/spec/requests/api/project_hooks_spec.rb b/spec/requests/api/project_hooks_spec.rb new file mode 100644 index 0000000000..c2a60abb48 --- /dev/null +++ b/spec/requests/api/project_hooks_spec.rb @@ -0,0 +1,132 @@ +require 'spec_helper' + +describe API::API, 'ProjectHooks' do + include ApiHelpers + before(:each) { enable_observers } + after(:each) { disable_observers } + + let(:user) { create(:user) } + let(:user3) { create(:user) } + let!(:project) { create(:project_with_code, creator_id: user.id, namespace: user.namespace) } + let!(:hook) { create(:project_hook, project: project, url: "http://example.com") } + + before do + project.team << [user, :master] + project.team << [user3, :developer] + end + + describe "GET /projects/:id/hooks" do + context "authorized user" do + it "should return project hooks" do + get api("/projects/#{project.id}/hooks", user) + response.status.should == 200 + + json_response.should be_an Array + json_response.count.should == 1 + json_response.first['url'].should == "http://example.com" + end + end + + context "unauthorized user" do + it "should not access project hooks" do + get api("/projects/#{project.id}/hooks", user3) + response.status.should == 403 + end + end + end + + describe "GET /projects/:id/hooks/:hook_id" do + context "authorized user" do + it "should return a project hook" do + get api("/projects/#{project.id}/hooks/#{hook.id}", user) + response.status.should == 200 + json_response['url'].should == hook.url + end + + it "should return a 404 error if hook id is not available" do + get api("/projects/#{project.id}/hooks/1234", user) + response.status.should == 404 + end + end + + context "unauthorized user" do + it "should not access an existing hook" do + get api("/projects/#{project.id}/hooks/#{hook.id}", user3) + response.status.should == 403 + end + end + + it "should return a 404 error if hook id is not available" do + get api("/projects/#{project.id}/hooks/1234", user) + response.status.should == 404 + end + end + + describe "POST /projects/:id/hooks" do + it "should add hook to project" do + expect { + post api("/projects/#{project.id}/hooks", user), + url: "http://example.com" + }.to change {project.hooks.count}.by(1) + response.status.should == 201 + end + + it "should return a 400 error if url not given" do + post api("/projects/#{project.id}/hooks", user) + response.status.should == 400 + end + + it "should return a 422 error if url not valid" do + post api("/projects/#{project.id}/hooks", user), "url" => "ftp://example.com" + response.status.should == 422 + end + end + + describe "PUT /projects/:id/hooks/:hook_id" do + it "should update an existing project hook" do + put api("/projects/#{project.id}/hooks/#{hook.id}", user), + url: 'http://example.org' + response.status.should == 200 + json_response['url'].should == 'http://example.org' + end + + it "should return 404 error if hook id not found" do + put api("/projects/#{project.id}/hooks/1234", user), url: 'http://example.org' + response.status.should == 404 + end + + it "should return 400 error if url is not given" do + put api("/projects/#{project.id}/hooks/#{hook.id}", user) + response.status.should == 400 + end + + it "should return a 422 error if url is not valid" do + put api("/projects/#{project.id}/hooks/#{hook.id}", user), url: 'ftp://example.com' + response.status.should == 422 + end + end + + describe "DELETE /projects/:id/hooks/:hook_id" do + it "should delete hook from project" do + expect { + delete api("/projects/#{project.id}/hooks/#{hook.id}", user) + }.to change {project.hooks.count}.by(-1) + response.status.should == 200 + end + + it "should return success when deleting hook" do + delete api("/projects/#{project.id}/hooks/#{hook.id}", user) + response.status.should == 200 + end + + it "should return success when deleting non existent hook" do + delete api("/projects/#{project.id}/hooks/42", user) + response.status.should == 200 + end + + it "should return a 405 error if hook id not given" do + delete api("/projects/#{project.id}/hooks", user) + response.status.should == 405 + end + end +end diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 7322d793c9..8e0b906767 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -10,7 +10,6 @@ describe API::API do let(:user3) { create(:user) } let(:admin) { create(:admin) } let!(:project) { create(:project_with_code, creator_id: user.id, namespace: user.namespace) } - let!(:hook) { create(:project_hook, project: project, url: "http://example.com") } let!(:snippet) { create(:project_snippet, author: user, project: project, title: 'example') } let!(:users_project) { create(:users_project, user: user, project: project, project_access: UsersProject::MASTER) } let!(:users_project2) { create(:users_project, user: user3, project: project, project_access: UsersProject::DEVELOPER) } @@ -439,121 +438,6 @@ describe API::API do end end - describe "GET /projects/:id/hooks" do - context "authorized user" do - it "should return project hooks" do - get api("/projects/#{project.id}/hooks", user) - response.status.should == 200 - - json_response.should be_an Array - json_response.count.should == 1 - json_response.first['url'].should == "http://example.com" - end - end - - context "unauthorized user" do - it "should not access project hooks" do - get api("/projects/#{project.id}/hooks", user3) - response.status.should == 403 - end - end - end - - describe "GET /projects/:id/hooks/:hook_id" do - context "authorized user" do - it "should return a project hook" do - get api("/projects/#{project.id}/hooks/#{hook.id}", user) - response.status.should == 200 - json_response['url'].should == hook.url - end - - it "should return a 404 error if hook id is not available" do - get api("/projects/#{project.id}/hooks/1234", user) - response.status.should == 404 - end - end - - context "unauthorized user" do - it "should not access an existing hook" do - get api("/projects/#{project.id}/hooks/#{hook.id}", user3) - response.status.should == 403 - end - end - - it "should return a 404 error if hook id is not available" do - get api("/projects/#{project.id}/hooks/1234", user) - response.status.should == 404 - end - end - - describe "POST /projects/:id/hooks" do - it "should add hook to project" do - expect { - post api("/projects/#{project.id}/hooks", user), - url: "http://example.com" - }.to change {project.hooks.count}.by(1) - response.status.should == 201 - end - - it "should return a 400 error if url not given" do - post api("/projects/#{project.id}/hooks", user) - response.status.should == 400 - end - - it "should return a 422 error if url not valid" do - post api("/projects/#{project.id}/hooks", user), "url" => "ftp://example.com" - response.status.should == 422 - end - end - - describe "PUT /projects/:id/hooks/:hook_id" do - it "should update an existing project hook" do - put api("/projects/#{project.id}/hooks/#{hook.id}", user), - url: 'http://example.org' - response.status.should == 200 - json_response['url'].should == 'http://example.org' - end - - it "should return 404 error if hook id not found" do - put api("/projects/#{project.id}/hooks/1234", user), url: 'http://example.org' - response.status.should == 404 - end - - it "should return 400 error if url is not given" do - put api("/projects/#{project.id}/hooks/#{hook.id}", user) - response.status.should == 400 - end - - it "should return a 422 error if url is not valid" do - put api("/projects/#{project.id}/hooks/#{hook.id}", user), url: 'ftp://example.com' - response.status.should == 422 - end - end - - describe "DELETE /projects/:id/hooks/:hook_id" do - it "should delete hook from project" do - expect { - delete api("/projects/#{project.id}/hooks/#{hook.id}", user) - }.to change {project.hooks.count}.by(-1) - response.status.should == 200 - end - - it "should return success when deleting hook" do - delete api("/projects/#{project.id}/hooks/#{hook.id}", user) - response.status.should == 200 - end - - it "should return success when deleting non existent hook" do - delete api("/projects/#{project.id}/hooks/42", user) - response.status.should == 200 - end - - it "should return a 405 error if hook id not given" do - delete api("/projects/#{project.id}/hooks", user) - response.status.should == 405 - end - end - describe "GET /projects/:id/snippets" do it "should return an array of project snippets" do get api("/projects/#{project.id}/snippets", user) From 21f4e5d3ac80c319592ecedb8bbf0c162841beb7 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 4 Dec 2013 13:35:38 +0200 Subject: [PATCH 12/12] ProjectHook API supports new event fields Signed-off-by: Dmitriy Zaporozhets --- doc/api/projects.md | 10 ++++++++++ lib/api/project_hooks.rb | 5 +++-- spec/requests/api/project_hooks_spec.rb | 4 ++-- 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/doc/api/projects.md b/doc/api/projects.md index 447c923b9b..53acc4a025 100644 --- a/doc/api/projects.md +++ b/doc/api/projects.md @@ -402,6 +402,10 @@ Parameters: { "id": 1, "url": "http://example.com/hook", + "project_id": 3, + "push_events": "true", + "issues_events": "true", + "merge_requests_events": "true", "created_at": "2012-10-12T17:04:47Z" } ``` @@ -419,6 +423,9 @@ Parameters: + `id` (required) - The ID or NAME of a project + `url` (required) - The hook URL ++ `push_events` - Trigger hook on push events ++ `issues_events` - Trigger hook on issues events ++ `merge_requests_events` - Trigger hook on merge_requests events ### Edit project hook @@ -434,6 +441,9 @@ Parameters: + `id` (required) - The ID or NAME of a project + `hook_id` (required) - The ID of a project hook + `url` (required) - The hook URL ++ `push_events` - Trigger hook on push events ++ `issues_events` - Trigger hook on issues events ++ `merge_requests_events` - Trigger hook on merge_requests events ### Delete project hook diff --git a/lib/api/project_hooks.rb b/lib/api/project_hooks.rb index e5da15d3ea..c271dd8b61 100644 --- a/lib/api/project_hooks.rb +++ b/lib/api/project_hooks.rb @@ -47,8 +47,9 @@ module API # POST /projects/:id/hooks post ":id/hooks" do required_attributes! [:url] + attrs = attributes_for_keys [:url, :push_events, :issues_events, :merge_requests_events] + @hook = user_project.hooks.new(attrs) - @hook = user_project.hooks.new({"url" => params[:url]}) if @hook.save present @hook, with: Entities::ProjectHook else @@ -70,8 +71,8 @@ module API put ":id/hooks/:hook_id" do @hook = user_project.hooks.find(params[:hook_id]) required_attributes! [:url] + attrs = attributes_for_keys [:url, :push_events, :issues_events, :merge_requests_events] - attrs = attributes_for_keys [:url] if @hook.update_attributes attrs present @hook, with: Entities::ProjectHook else diff --git a/spec/requests/api/project_hooks_spec.rb b/spec/requests/api/project_hooks_spec.rb index c2a60abb48..beccd61866 100644 --- a/spec/requests/api/project_hooks_spec.rb +++ b/spec/requests/api/project_hooks_spec.rb @@ -66,7 +66,7 @@ describe API::API, 'ProjectHooks' do it "should add hook to project" do expect { post api("/projects/#{project.id}/hooks", user), - url: "http://example.com" + url: "http://example.com", issues_events: true }.to change {project.hooks.count}.by(1) response.status.should == 201 end @@ -85,7 +85,7 @@ describe API::API, 'ProjectHooks' do describe "PUT /projects/:id/hooks/:hook_id" do it "should update an existing project hook" do put api("/projects/#{project.id}/hooks/#{hook.id}", user), - url: 'http://example.org' + url: 'http://example.org', push_events: false response.status.should == 200 json_response['url'].should == 'http://example.org' end