diff --git a/CHANGELOG-EE b/CHANGELOG-EE index b83a2b07f1..55271e3240 100644 --- a/CHANGELOG-EE +++ b/CHANGELOG-EE @@ -4,6 +4,8 @@ v 7.9.0 (unreleased) - Check if LDAP admin group exists before querying for user membership - Use one custom header logo for all GitLab themes in appearance settings - Escape wildcards when searching LDAP by group name. +v 7.9.0 + - Group level Web Hooks v 7.8.0 - Improved Jira issue closing integration diff --git a/app/controllers/groups/application_controller.rb b/app/controllers/groups/application_controller.rb index 7f27f2bb73..04c47275b9 100644 --- a/app/controllers/groups/application_controller.rb +++ b/app/controllers/groups/application_controller.rb @@ -7,4 +7,8 @@ class Groups::ApplicationController < ApplicationController return render_404 end end + + def group + @group ||= Group.find_by(path: params[:group_id]) + end end diff --git a/app/controllers/groups/group_members_controller.rb b/app/controllers/groups/group_members_controller.rb index 613a8ee0a4..4c84e1a419 100644 --- a/app/controllers/groups/group_members_controller.rb +++ b/app/controllers/groups/group_members_controller.rb @@ -71,10 +71,6 @@ class Groups::GroupMembersController < Groups::ApplicationController protected - def group - @group ||= Group.find_by(path: params[:group_id]) - end - def member_params params.require(:group_member).permit(:access_level, :user_id) end diff --git a/app/controllers/groups/hooks_controller.rb b/app/controllers/groups/hooks_controller.rb new file mode 100644 index 0000000000..601549579f --- /dev/null +++ b/app/controllers/groups/hooks_controller.rb @@ -0,0 +1,59 @@ +class Groups::HooksController < Groups::ApplicationController + # Authorize + before_filter :group + before_filter :authorize_admin_group! + + respond_to :html + + layout "group" + + def index + @hooks = @group.hooks + @hook = GroupHook.new + end + + def create + @hook = @group.hooks.new(hook_params) + @hook.save + + if @hook.valid? + redirect_to group_hooks_path(@group) + else + @hooks = @group.hooks.select(&:persisted?) + render :index + end + end + + def test + if @group.first_non_empty_project + status = TestHookService.new.execute(hook, current_user) + + if status + flash[:notice] = 'Hook successfully executed.' + else + flash[:alert] = 'Hook execution failed. '\ + 'Ensure hook URL is correct and service is up.' + end + else + flash[:alert] = 'Hook execution failed. Ensure the group has a project with commits.' + end + + redirect_to :back + end + + def destroy + hook.destroy + + redirect_to group_hooks_path(@group) + end + + private + + def hook + @hook ||= @group.hooks.find(params[:id]) + end + + def hook_params + params.require(:hook).permit(:url, :push_events, :issues_events, :merge_requests_events, :tag_push_events) + end +end diff --git a/app/controllers/groups/ldap_group_links_controller.rb b/app/controllers/groups/ldap_group_links_controller.rb index 74c12c3828..6686c39fb3 100644 --- a/app/controllers/groups/ldap_group_links_controller.rb +++ b/app/controllers/groups/ldap_group_links_controller.rb @@ -1,4 +1,4 @@ -class Groups::LdapGroupLinksController < ApplicationController +class Groups::LdapGroupLinksController < Groups::ApplicationController before_action :group before_action :require_ldap_enabled before_action :authorize_admin_group! @@ -28,14 +28,6 @@ class Groups::LdapGroupLinksController < ApplicationController private - def group - @group ||= Group.find_by(path: params[:group_id]) - end - - def authorize_admin_group! - render_404 unless can?(current_user, :manage_group, group) - end - def ldap_group_link_params params.require(:ldap_group_link).permit(:cn, :group_access, :provider) end diff --git a/app/controllers/groups/ldaps_controller.rb b/app/controllers/groups/ldaps_controller.rb index 5e5f8fc035..2774e3fc4f 100644 --- a/app/controllers/groups/ldaps_controller.rb +++ b/app/controllers/groups/ldaps_controller.rb @@ -1,4 +1,4 @@ -class Groups::LdapsController < ApplicationController +class Groups::LdapsController < Groups::ApplicationController before_filter :group before_filter :authorize_admin_group! @@ -7,16 +7,4 @@ class Groups::LdapsController < ApplicationController redirect_to members_group_path(@group), notice: 'Access reset complete' end - - private - - def group - @group ||= Group.find_by(path: params[:group_id]) - end - - def authorize_admin_group! - unless can?(current_user, :manage_group, group) - return render_404 - end - end end diff --git a/app/helpers/groups_helper.rb b/app/helpers/groups_helper.rb index 5855bcabe5..0fdb1e82a1 100644 --- a/app/helpers/groups_helper.rb +++ b/app/helpers/groups_helper.rb @@ -36,7 +36,7 @@ module GroupsHelper def group_settings_page? if current_controller?('groups') current_action?('edit') || current_action?('projects') - elsif current_controller?('ldap_group_links') || current_controller?('audit_events') + elsif ['ldap_group_links', 'audit_events', 'hooks'].any?{ |c| current_controller? c } true else false diff --git a/app/models/group.rb b/app/models/group.rb index 209345b677..b4670b4567 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -22,6 +22,7 @@ class Group < Namespace has_many :project_group_links, dependent: :destroy has_many :shared_projects, through: :project_group_links, source: :project has_many :ldap_group_links, foreign_key: 'group_id', dependent: :destroy + has_many :hooks, dependent: :destroy, class_name: 'GroupHook' validate :avatar_type, if: ->(user) { user.avatar_changed? } validates :avatar, file_size: { maximum: 200.kilobytes.to_i } @@ -118,4 +119,8 @@ class Group < Namespace def system_hook_service SystemHooksService.new end + + def first_non_empty_project + projects.detect{ |project| !project.empty_repo? } + end end diff --git a/app/models/hooks/group_hook.rb b/app/models/hooks/group_hook.rb new file mode 100644 index 0000000000..dae2e56eff --- /dev/null +++ b/app/models/hooks/group_hook.rb @@ -0,0 +1,20 @@ +# == Schema Information +# +# Table name: web_hooks +# +# id :integer not null, primary key +# url :string(255) +# project_id :integer +# created_at :datetime +# updated_at :datetime +# 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 +# tag_push_events :boolean default(FALSE) +# + +class GroupHook < ProjectHook + belongs_to :group +end diff --git a/app/models/project.rb b/app/models/project.rb index 233ec048dc..a887b6cd3c 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -491,6 +491,11 @@ class Project < ActiveRecord::Base hooks.send(hooks_scope).each do |hook| hook.async_execute(data) end + if group + group.hooks.send(hooks_scope).each do |hook| + hook.async_execute(data) + end + end end def execute_services(data, hooks_scope = :push_hooks) diff --git a/app/services/test_hook_service.rb b/app/services/test_hook_service.rb index 21ec2c01cb..ab661984fc 100644 --- a/app/services/test_hook_service.rb +++ b/app/services/test_hook_service.rb @@ -1,6 +1,16 @@ class TestHookService def execute(hook, current_user) - data = Gitlab::PushDataBuilder.build_sample(hook.project, current_user) + data = Gitlab::PushDataBuilder.build_sample(project(hook), current_user) hook.execute(data) end + + private + + def project(hook) + if hook.is_a? GroupHook + hook.group.first_non_empty_project + else + hook.project + end + end end diff --git a/app/views/groups/_settings_nav.html.haml b/app/views/groups/_settings_nav.html.haml index 5ccb906470..6c4a7572ce 100644 --- a/app/views/groups/_settings_nav.html.haml +++ b/app/views/groups/_settings_nav.html.haml @@ -15,6 +15,11 @@ %i.fa.fa-exchange %span LDAP Groups + = nav_link(controller: :hooks) do + = link_to group_hooks_path(@group) do + %i.fa.fa-link + %span + Web Hooks = nav_link(controller: :audit_events) do = link_to group_audit_events_path(@group) do %i.fa.fa-file-text-o diff --git a/app/views/groups/hooks/index.html.haml b/app/views/groups/hooks/index.html.haml new file mode 100644 index 0000000000..7039c070d9 --- /dev/null +++ b/app/views/groups/hooks/index.html.haml @@ -0,0 +1,68 @@ +%h3.page-title + Web hooks + +%p.light + #{link_to "Web hooks ", help_page_path("web_hooks", "web_hooks"), class: "vlink"} can be + used for binding events when something is happening within any project inside this group. + +%hr.clearfix + += form_for [@group, @hook], as: :hook, url: group_hooks_path(@group), html: { class: 'form-horizontal' } do |f| + -if @hook.errors.any? + .alert.alert-danger + - @hook.errors.full_messages.each do |msg| + %p= msg + .form-group + = f.label :url, "URL", class: 'control-label' + .col-sm-10 + = f.text_field :url, class: "form-control", placeholder: 'http://example.com/trigger-ci.json' + .form-group + = f.label :url, "Trigger", class: 'control-label' + .col-sm-10 + %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 by a push to the repository + %div + = f.check_box :tag_push_events, class: 'pull-left' + .prepend-left-20 + = f.label :tag_push_events, class: 'list-label' do + %strong Tag push events + %p.light + This url will be triggered when a new tag is pushed to the 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 when an issue is created + %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 when a merge request is created + .form-actions + = f.submit "Add Web Hook", class: "btn btn-create" + +-if @hooks.any? + .panel.panel-default + .panel-heading + Web hooks (#{@hooks.count}) + %ul.well-list + - @hooks.each do |hook| + %li + .pull-right + = link_to 'Test Hook', test_group_hook_path(@group, hook), class: "btn btn-small btn-grouped" + = link_to 'Remove', group_hook_path(@group, hook), data: { confirm: 'Are you sure?'}, method: :delete, class: "btn btn-remove btn-small btn-grouped" + .clearfix + %span.monospace= hook.url + %p + - %w(push_events tag_push_events issues_events merge_requests_events).each do |trigger| + - if hook.send(trigger) + %span.label.label-gray= trigger.titleize diff --git a/config/routes.rb b/config/routes.rb index c76fa537dc..511b981f7e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -271,6 +271,12 @@ Gitlab::Application.routes.draw do end get "/audit_events" => "audit_events#group_log" + + resources :hooks, only: [:index, :create, :destroy], constraints: { id: /\d+/ }, module: :groups do + member do + get :test + end + end end get 'unsubscribes/:email', to: 'unsubscribes#show', as: :unsubscribe diff --git a/db/migrate/20150312000132_add_group_id_to_web_hooks.rb b/db/migrate/20150312000132_add_group_id_to_web_hooks.rb new file mode 100644 index 0000000000..074a71c4f2 --- /dev/null +++ b/db/migrate/20150312000132_add_group_id_to_web_hooks.rb @@ -0,0 +1,5 @@ +class AddGroupIdToWebHooks < ActiveRecord::Migration + def change + add_column :web_hooks, :group_id, :integer, after: :project_id + end +end \ No newline at end of file diff --git a/db/schema.rb b/db/schema.rb index 5c6d2c3eb2..72f0139f34 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20150306023112) do +ActiveRecord::Schema.define(version: 20150312000132) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -556,6 +556,7 @@ ActiveRecord::Schema.define(version: 20150306023112) do t.boolean "issues_events", default: false, null: false t.boolean "merge_requests_events", default: false, null: false t.boolean "tag_push_events", default: false + t.integer "group_id" end add_index "web_hooks", ["created_at", "id"], name: "index_web_hooks_on_created_at_and_id", using: :btree diff --git a/features/group_hooks.feature b/features/group_hooks.feature new file mode 100644 index 0000000000..19658047ec --- /dev/null +++ b/features/group_hooks.feature @@ -0,0 +1,37 @@ +Feature: Group Hooks + Background: + Given I sign in as a user + And I own group "Sourcing" + + Scenario: I should see hook list + Given I own project "Shop" in group "Sourcing" + And group has hook + When I visit group hooks page + Then I should see group hook + + Scenario: I add new hook + GivenI own project "Shop" in group "Sourcing" + And I visit group hooks page + When I submit new hook + Then I should see newly created hook + + Scenario: I test hook + Given I own project "Shop" in group "Sourcing" + And group has hook + And I visit group hooks page + When I click test hook button + Then hook should be triggered + + Scenario: I test a hook on empty project + Given I own empty project "Empty Shop" in group "Sourcing" + And group has hook + And I visit group hooks page + When I click test hook button + Then I should see hook error message + + Scenario: I test a hook on down URL + Given I own project "Shop" in group "Sourcing" + And group has hook + And I visit group hooks page + When I click test hook button with invalid URL + Then I should see hook service down error message diff --git a/features/steps/group_hooks.rb b/features/steps/group_hooks.rb new file mode 100644 index 0000000000..ce0d6ce504 --- /dev/null +++ b/features/steps/group_hooks.rb @@ -0,0 +1,72 @@ +require 'webmock' + +class Spinach::Features::GroupHooks < Spinach::FeatureSteps + include SharedAuthentication + include SharedProject + include SharedPaths + include RSpec::Matchers + include RSpec::Mocks::ExampleMethods + include WebMock::API + + step 'I own group "Sourcing"' do + @group = create :group, name: "Sourcing" + @group.add_owner(current_user) + end + + step 'I own project "Shop" in group "Sourcing"' do + @project = create(:project, + name: 'Shop', group: @group) + end + + step 'I own empty project "Empty Shop" in group "Sourcing"' do + @project = create(:empty_project, + name: 'Shop', group: @group) + end + + step 'group has hook' do + @hook = create(:group_hook, group: @group) + end + + step 'I should see group hook' do + page.should have_content @hook.url + end + + step 'I submit new hook' do + @url = Faker::Internet.uri("http") + fill_in "hook_url", with: @url + expect { click_button "Add Web Hook" }.to change(GroupHook, :count).by(1) + end + + step 'I should see newly created hook' do + current_path.should == group_hooks_path(@group) + page.should have_content(@url) + end + + step 'I click test hook button' do + stub_request(:post, @hook.url).to_return(status: 200) + click_link 'Test Hook' + end + + step 'I click test hook button with invalid URL' do + stub_request(:post, @hook.url).to_raise(SocketError) + click_link 'Test Hook' + end + + step 'hook should be triggered' do + current_path.should == group_hooks_path(@group) + page.should have_selector '.flash-notice', + text: 'Hook successfully executed.' + end + + step 'I should see hook error message' do + page.should have_selector '.flash-alert', + text: 'Hook execution failed. Ensure the group has a project with commits.' + end + + step 'I should see hook service down error message' do + page.should have_selector '.flash-alert', + text: 'Hook execution failed. '\ + 'Ensure hook URL is correct and '\ + 'service is up.' + end +end diff --git a/features/steps/shared/paths.rb b/features/steps/shared/paths.rb index d7642694be..cc9de0c92f 100644 --- a/features/steps/shared/paths.rb +++ b/features/steps/shared/paths.rb @@ -251,6 +251,10 @@ module SharedPaths visit namespace_project_hooks_path(@project.namespace, @project) end + step 'I visit group hooks page' do + visit group_hooks_path(@group) + end + step 'I visit project git hooks page' do visit namespace_project_git_hooks_path(@project.namespace, @project) end diff --git a/spec/factories.rb b/spec/factories.rb index 2b3cfe3580..784b112bf4 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -152,6 +152,10 @@ FactoryGirl.define do url end + factory :group_hook do + url + end + factory :project_snippet do project author diff --git a/spec/models/hooks/group_hook_spec.rb b/spec/models/hooks/group_hook_spec.rb new file mode 100644 index 0000000000..c668a14f82 --- /dev/null +++ b/spec/models/hooks/group_hook_spec.rb @@ -0,0 +1,24 @@ +# == Schema Information +# +# Table name: web_hooks +# +# id :integer not null, primary key +# url :string(255) +# project_id :integer +# created_at :datetime +# updated_at :datetime +# 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 +# tag_push_events :boolean default(FALSE) +# + +require 'spec_helper' + +describe GroupHook do + describe "Associations" do + it { is_expected.to belong_to :group } + end +end diff --git a/spec/services/test_hook_service_spec.rb b/spec/services/test_hook_service_spec.rb index d2b505f55a..f21ff1f011 100644 --- a/spec/services/test_hook_service_spec.rb +++ b/spec/services/test_hook_service_spec.rb @@ -1,14 +1,23 @@ require 'spec_helper' describe TestHookService do - let (:user) { create :user } - let (:project) { create :project } - let (:hook) { create :project_hook, project: project } + let (:user) { create :user } + let (:group) { create :group } + let (:project) { create :project, group: group } + let (:project_hook){ create :project_hook, project: project } + let (:group_hook) { create :group_hook, group: group } + describe :execute do - it "should execute successfully" do - stub_request(:post, hook.url).to_return(status: 200) - expect(TestHookService.new.execute(hook, user)).to be_truthy + it "should successfully execute the project hook" do + stub_request(:post, project_hook.url).to_return(status: 200) + expect(TestHookService.new.execute(project_hook, user)).to be_truthy + end + + it "should successfully execute the group hook" do + project.reload + stub_request(:post, group_hook.url).to_return(status: 200) + expect(TestHookService.new.execute(group_hook, user)).to be_truthy end end end