diff options
44 files changed, 663 insertions, 130 deletions
diff --git a/app/controllers/admin/hook_logs_controller.rb b/app/controllers/admin/hook_logs_controller.rb index aa069b89563..3017f96c26f 100644 --- a/app/controllers/admin/hook_logs_controller.rb +++ b/app/controllers/admin/hook_logs_controller.rb @@ -10,9 +10,9 @@ class Admin::HookLogsController < Admin::ApplicationController end def retry - status, message = hook.execute(hook_log.request_data, hook_log.trigger) + result = hook.execute(hook_log.request_data, hook_log.trigger) - set_hook_execution_notice(status, message) + set_hook_execution_notice(result) redirect_to edit_admin_hook_path(@hook) end diff --git a/app/controllers/admin/hooks_controller.rb b/app/controllers/admin/hooks_controller.rb index 054c3500b35..77e3c95d197 100644 --- a/app/controllers/admin/hooks_controller.rb +++ b/app/controllers/admin/hooks_controller.rb @@ -38,9 +38,9 @@ class Admin::HooksController < Admin::ApplicationController end def test - status, message = hook.execute(sample_hook_data, 'system_hooks') + result = TestHooks::SystemService.new(hook, current_user, params[:trigger]).execute - set_hook_execution_notice(status, message) + set_hook_execution_notice(result) redirect_back_or_default end @@ -66,15 +66,4 @@ class Admin::HooksController < Admin::ApplicationController :url ) end - - def sample_hook_data - { - event_name: "project_create", - name: "Ruby", - path: "ruby", - project_id: 1, - owner_name: "Someone", - owner_email: "example@gitlabhq.com" - } - end end diff --git a/app/controllers/concerns/hooks_execution.rb b/app/controllers/concerns/hooks_execution.rb index 846cd60518f..a22e46b4860 100644 --- a/app/controllers/concerns/hooks_execution.rb +++ b/app/controllers/concerns/hooks_execution.rb @@ -3,11 +3,14 @@ module HooksExecution private - def set_hook_execution_notice(status, message) - if status && status >= 200 && status < 400 - flash[:notice] = "Hook executed successfully: HTTP #{status}" - elsif status - flash[:alert] = "Hook executed successfully but returned HTTP #{status} #{message}" + def set_hook_execution_notice(result) + http_status = result[:http_status] + message = result[:message] + + if http_status && http_status >= 200 && http_status < 400 + flash[:notice] = "Hook executed successfully: HTTP #{http_status}" + elsif http_status + flash[:alert] = "Hook executed successfully but returned HTTP #{http_status} #{message}" else flash[:alert] = "Hook execution failed: #{message}" end diff --git a/app/controllers/projects/hook_logs_controller.rb b/app/controllers/projects/hook_logs_controller.rb index b9c4b29580a..745e89fc843 100644 --- a/app/controllers/projects/hook_logs_controller.rb +++ b/app/controllers/projects/hook_logs_controller.rb @@ -14,9 +14,9 @@ class Projects::HookLogsController < Projects::ApplicationController end def retry - status, message = hook.execute(hook_log.request_data, hook_log.trigger) + result = hook.execute(hook_log.request_data, hook_log.trigger) - set_hook_execution_notice(status, message) + set_hook_execution_notice(result) redirect_to edit_project_hook_path(@project, @hook) end diff --git a/app/controllers/projects/hooks_controller.rb b/app/controllers/projects/hooks_controller.rb index 18895c3f0f3..85d35900c71 100644 --- a/app/controllers/projects/hooks_controller.rb +++ b/app/controllers/projects/hooks_controller.rb @@ -9,6 +9,10 @@ class Projects::HooksController < Projects::ApplicationController layout "project_settings" + def index + redirect_to project_settings_integrations_path(@project) + end + def create @hook = @project.hooks.new(hook_params) @hook.save @@ -33,13 +37,9 @@ class Projects::HooksController < Projects::ApplicationController end def test - if !@project.empty_repo? - status, message = TestHookService.new.execute(hook, current_user) + result = TestHooks::ProjectService.new(hook, current_user, params[:trigger]).execute - set_hook_execution_notice(status, message) - else - flash[:alert] = 'Hook execution failed. Ensure the project has commits.' - end + set_hook_execution_notice(result) redirect_back_or_default(default: { action: 'index' }) end diff --git a/app/helpers/hooks_helper.rb b/app/helpers/hooks_helper.rb new file mode 100644 index 00000000000..551b9cca6b1 --- /dev/null +++ b/app/helpers/hooks_helper.rb @@ -0,0 +1,17 @@ +module HooksHelper + def link_to_test_hook(hook, trigger) + path = case hook + when ProjectHook + project = hook.project + test_project_hook_path(project, hook, trigger: trigger) + when SystemHook + test_admin_hook_path(hook, trigger: trigger) + end + + trigger_human_name = trigger.to_s.tr('_', ' ').camelize + + link_to path, rel: 'nofollow' do + content_tag(:span, trigger_human_name) + end + end +end diff --git a/app/models/hooks/project_hook.rb b/app/models/hooks/project_hook.rb index ee6165fd32d..a8c424a6614 100644 --- a/app/models/hooks/project_hook.rb +++ b/app/models/hooks/project_hook.rb @@ -1,11 +1,20 @@ class ProjectHook < WebHook - belongs_to :project + 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 + + TRIGGERS.each do |trigger, event| + scope trigger, -> { where(event => true) } + end - scope :issue_hooks, -> { where(issues_events: true) } - scope :confidential_issue_hooks, -> { where(confidential_issues_events: true) } - scope :note_hooks, -> { where(note_events: true) } - scope :merge_request_hooks, -> { where(merge_requests_events: true) } - scope :job_hooks, -> { where(job_events: true) } - scope :pipeline_hooks, -> { where(pipeline_events: true) } - scope :wiki_page_hooks, -> { where(wiki_page_events: true) } + belongs_to :project + validates :project, presence: true end diff --git a/app/models/hooks/service_hook.rb b/app/models/hooks/service_hook.rb index 40e43c27f91..aef11514945 100644 --- a/app/models/hooks/service_hook.rb +++ b/app/models/hooks/service_hook.rb @@ -1,5 +1,6 @@ class ServiceHook < WebHook belongs_to :service + validates :service, presence: true def execute(data) WebHookService.new(self, data, 'service_hook').execute diff --git a/app/models/hooks/system_hook.rb b/app/models/hooks/system_hook.rb index 1584235ab00..180c479c41b 100644 --- a/app/models/hooks/system_hook.rb +++ b/app/models/hooks/system_hook.rb @@ -1,5 +1,13 @@ class SystemHook < WebHook - scope :repository_update_hooks, -> { where(repository_update_events: true) } + TRIGGERS = { + repository_update_hooks: :repository_update_events, + push_hooks: :push_events, + tag_push_hooks: :tag_push_events + }.freeze + + TRIGGERS.each do |trigger, event| + scope trigger, -> { where(event => true) } + end default_value_for :push_events, false default_value_for :repository_update_events, true diff --git a/app/models/hooks/web_hook.rb b/app/models/hooks/web_hook.rb index 7a9f8997959..5a70e114f56 100644 --- a/app/models/hooks/web_hook.rb +++ b/app/models/hooks/web_hook.rb @@ -1,22 +1,8 @@ class WebHook < ActiveRecord::Base include Sortable - default_value_for :push_events, true - default_value_for :issues_events, false - default_value_for :confidential_issues_events, false - default_value_for :note_events, false - default_value_for :merge_requests_events, false - default_value_for :tag_push_events, false - default_value_for :job_events, false - default_value_for :pipeline_events, false - default_value_for :repository_update_events, false - default_value_for :enable_ssl_verification, true - has_many :web_hook_logs, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent - scope :push_hooks, -> { where(push_events: true) } - scope :tag_push_hooks, -> { where(tag_push_events: true) } - validates :url, presence: true, url: true def execute(data, hook_name) diff --git a/app/services/system_hooks_service.rb b/app/services/system_hooks_service.rb index ed476fc9d0c..bd58a54592f 100644 --- a/app/services/system_hooks_service.rb +++ b/app/services/system_hooks_service.rb @@ -4,7 +4,7 @@ class SystemHooksService end def execute_hooks(data, hooks_scope = :all) - SystemHook.send(hooks_scope).each do |hook| + SystemHook.public_send(hooks_scope).find_each do |hook| hook.async_execute(data, 'system_hooks') end end diff --git a/app/services/test_hook_service.rb b/app/services/test_hook_service.rb deleted file mode 100644 index 280c81f7d2d..00000000000 --- a/app/services/test_hook_service.rb +++ /dev/null @@ -1,6 +0,0 @@ -class TestHookService - def execute(hook, current_user) - data = Gitlab::DataBuilder::Push.build_sample(hook.project, current_user) - hook.execute(data, 'push_hooks') - end -end diff --git a/app/services/test_hooks/base_service.rb b/app/services/test_hooks/base_service.rb new file mode 100644 index 00000000000..74ba814afff --- /dev/null +++ b/app/services/test_hooks/base_service.rb @@ -0,0 +1,41 @@ +module TestHooks + class BaseService + attr_accessor :hook, :current_user, :trigger + + def initialize(hook, current_user, trigger) + @hook = hook + @current_user = current_user + @trigger = trigger + end + + def execute + trigger_data_method = "#{trigger}_data" + + if !self.respond_to?(trigger_data_method, true) || + !hook.class::TRIGGERS.value?(trigger.to_sym) + + return error('Testing not available for this hook') + end + + error_message = catch(:validation_error) do + sample_data = self.__send__(trigger_data_method) + + return hook.execute(sample_data, trigger) + end + + error(error_message) + end + + private + + def error(message, http_status = nil) + result = { + message: message, + status: :error + } + + result[:http_status] = http_status if http_status + result + end + end +end diff --git a/app/services/test_hooks/project_service.rb b/app/services/test_hooks/project_service.rb new file mode 100644 index 00000000000..01d5d774cd5 --- /dev/null +++ b/app/services/test_hooks/project_service.rb @@ -0,0 +1,63 @@ +module TestHooks + class ProjectService < TestHooks::BaseService + private + + def project + @project ||= hook.project + end + + def push_events_data + throw(:validation_error, 'Ensure the project has at least one commit.') if project.empty_repo? + + Gitlab::DataBuilder::Push.build_sample(project, current_user) + end + + alias_method :tag_push_events_data, :push_events_data + + def note_events_data + note = project.notes.first + throw(:validation_error, 'Ensure the project has notes.') unless note.present? + + Gitlab::DataBuilder::Note.build(note, current_user) + end + + def issues_events_data + issue = project.issues.first + throw(:validation_error, 'Ensure the project has issues.') unless issue.present? + + issue.to_hook_data(current_user) + end + + alias_method :confidential_issues_events_data, :issues_events_data + + def merge_requests_events_data + merge_request = project.merge_requests.first + throw(:validation_error, 'Ensure the project has merge requests.') unless merge_request.present? + + merge_request.to_hook_data(current_user) + end + + def job_events_data + build = project.builds.first + throw(:validation_error, 'Ensure the project has CI jobs.') unless build.present? + + Gitlab::DataBuilder::Build.build(build) + end + + def pipeline_events_data + pipeline = project.pipelines.first + throw(:validation_error, 'Ensure the project has CI pipelines.') unless pipeline.present? + + Gitlab::DataBuilder::Pipeline.build(pipeline) + end + + def wiki_page_events_data + page = project.wiki.pages.first + if !project.wiki_enabled? || page.blank? + throw(:validation_error, 'Ensure the wiki is enabled and has pages.') + end + + Gitlab::DataBuilder::WikiPage.build(page, current_user, 'create') + end + end +end diff --git a/app/services/test_hooks/system_service.rb b/app/services/test_hooks/system_service.rb new file mode 100644 index 00000000000..76c3c19bd74 --- /dev/null +++ b/app/services/test_hooks/system_service.rb @@ -0,0 +1,48 @@ +module TestHooks + class SystemService < TestHooks::BaseService + private + + def project + @project ||= begin + project = Project.first + + throw(:validation_error, 'Ensure that at least one project exists.') unless project + + project + end + end + + def push_events_data + if project.empty_repo? + throw(:validation_error, "Ensure project \"#{project.human_name}\" has commits.") + end + + Gitlab::DataBuilder::Push.build_sample(project, current_user) + end + + def tag_push_events_data + if project.repository.tags.empty? + throw(:validation_error, "Ensure project \"#{project.human_name}\" has tags.") + end + + Gitlab::DataBuilder::Push.build_sample(project, current_user) + end + + def repository_update_events_data + commit = project.commit + ref = "#{Gitlab::Git::BRANCH_REF_PREFIX}#{project.default_branch}" + + unless commit + throw(:validation_error, "Ensure project \"#{project.human_name}\" has commits.") + end + + change = Gitlab::DataBuilder::Repository.single_change( + commit.parent_id || Gitlab::Git::BLANK_SHA, + commit.id, + ref + ) + + Gitlab::DataBuilder::Repository.update(project, current_user, [change], [ref]) + end + end +end diff --git a/app/services/web_hook_service.rb b/app/services/web_hook_service.rb index 4241b912d5b..a5110a23cad 100644 --- a/app/services/web_hook_service.rb +++ b/app/services/web_hook_service.rb @@ -39,7 +39,11 @@ class WebHookService execution_duration: Time.now - start_time ) - [response.code, response.to_s] + { + status: :success, + http_status: response.code, + message: response.to_s + } rescue SocketError, OpenSSL::SSL::SSLError, Errno::ECONNRESET, Errno::ECONNREFUSED, Net::OpenTimeout => e log_execution( trigger: hook_name, @@ -52,7 +56,10 @@ class WebHookService Rails.logger.error("WebHook Error => #{e}") - [nil, e.to_s] + { + status: :error, + message: e.to_s + } end def async_execute diff --git a/app/services/wiki_pages/base_service.rb b/app/services/wiki_pages/base_service.rb index 14317ea65c8..260c04a8b94 100644 --- a/app/services/wiki_pages/base_service.rb +++ b/app/services/wiki_pages/base_service.rb @@ -1,23 +1,9 @@ module WikiPages class BaseService < ::BaseService - def hook_data(page, action) - hook_data = { - object_kind: page.class.name.underscore, - user: current_user.hook_attrs, - project: @project.hook_attrs, - wiki: @project.wiki.hook_attrs, - object_attributes: page.hook_attrs - } - - page_url = Gitlab::UrlBuilder.build(page) - hook_data[:object_attributes].merge!(url: page_url, action: action) - hook_data - end - private def execute_hooks(page, action = 'create') - page_data = hook_data(page, action) + page_data = Gitlab::DataBuilder::WikiPage.build(page, current_user, action) @project.execute_hooks(page_data, :wiki_page_hooks) @project.execute_services(page_data, :wiki_page_hooks) end diff --git a/app/views/admin/hooks/edit.html.haml b/app/views/admin/hooks/edit.html.haml index 0e35a1905bf..665e8c7e74f 100644 --- a/app/views/admin/hooks/edit.html.haml +++ b/app/views/admin/hooks/edit.html.haml @@ -12,7 +12,7 @@ = render partial: 'form', locals: { form: f, hook: @hook } .form-actions = f.submit 'Save changes', class: 'btn btn-create' - = link_to 'Test hook', test_admin_hook_path(@hook), class: 'btn btn-default' + = 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 e92b8bc39f4..fed6002528d 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 - = link_to 'Test hook', test_admin_hook_path(hook), class: 'btn btn-sm' + = render 'shared/web_hooks/test_button', triggers: SystemHook::TRIGGERS, hook: hook, button_class: 'btn-small' = 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 - - %w(repository_update_events push_events tag_push_events issues_events note_events merge_requests_events job_events).each do |trigger| - - if hook.send(trigger) - %span.label.label-gray= trigger.titleize + - 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 4944e0c8041..c8c17d2d828 100644 --- a/app/views/projects/hooks/edit.html.haml +++ b/app/views/projects/hooks/edit.html.haml @@ -13,9 +13,10 @@ = render partial: 'shared/web_hooks/form', locals: { form: f, hook: @hook } = f.submit 'Save changes', class: 'btn btn-create' - = link_to 'Test hook', test_project_hook_path(@project, @hook), class: 'btn btn-default' + = 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 = render partial: 'projects/hook_logs/index', locals: { hook: @hook, hook_logs: @hook_logs, project: @project } + diff --git a/app/views/projects/settings/integrations/_project_hook.html.haml b/app/views/projects/settings/integrations/_project_hook.html.haml index 00700e286c3..d5792e95f5a 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 - - %w(push_events tag_push_events issues_events confidential_issues_events note_events merge_requests_events job_events pipeline_events wiki_page_events).each do |trigger| - - if hook.send(trigger) - %span.label.label-gray.deploy-project-label= trigger.titleize + - 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" - = link_to "Test", test_project_hook_path(@project, hook), class: "btn btn-sm" - = link_to project_hook_path(@project, hook), data: { confirm: 'Are you sure?'}, method: :delete, class: "btn btn-transparent" do + 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-small' + = 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/_test_button.html.haml b/app/views/shared/web_hooks/_test_button.html.haml new file mode 100644 index 00000000000..cf1d5e061c6 --- /dev/null +++ b/app/views/shared/web_hooks/_test_button.html.haml @@ -0,0 +1,12 @@ +- triggers = local_assigns.fetch(:triggers) +- button_class = local_assigns.fetch(:button_class, '') +- hook = local_assigns.fetch(:hook) + +.hook-test-button.dropdown.inline + %button.btn{ 'data-toggle' => 'dropdown', class: button_class } + Test + = icon('caret-down') + %ul.dropdown-menu.dropdown-menu-align-right{ role: 'menu' } + - triggers.each_value do |event| + %li + = link_to_test_hook(hook, event) diff --git a/changelogs/unreleased/5971-webhook-testing.yml b/changelogs/unreleased/5971-webhook-testing.yml new file mode 100644 index 00000000000..58233091977 --- /dev/null +++ b/changelogs/unreleased/5971-webhook-testing.yml @@ -0,0 +1,4 @@ +--- +title: Allow testing any events for project hooks and system hooks +merge_request: 11728 +author: Alexander Randa (@randaalex) diff --git a/doc/user/project/integrations/img/webhook_testing.png b/doc/user/project/integrations/img/webhook_testing.png Binary files differnew file mode 100644 index 00000000000..176dcec9d8a --- /dev/null +++ b/doc/user/project/integrations/img/webhook_testing.png diff --git a/doc/user/project/integrations/webhooks.md b/doc/user/project/integrations/webhooks.md index 023c6932e41..c03a2df9a72 100644 --- a/doc/user/project/integrations/webhooks.md +++ b/doc/user/project/integrations/webhooks.md @@ -1014,6 +1014,13 @@ X-Gitlab-Event: Build Hook } ``` +## Testing webhooks + +You can trigger the webhook manually. Sample data from the project will be used.Sample data will take from the project. +> For example: for triggering `Push Events` your project should have at least one commit. + + + ## Troubleshoot webhooks Gitlab stores each perform of the webhook. @@ -1056,7 +1063,7 @@ Pick an unused port (e.g. 8000) and start the script: `ruby print_http_body.rb 8000`. Then add your server as a webhook receiver in GitLab as `http://my.host:8000/`. -When you press 'Test Hook' in GitLab, you should see something like this in the +When you press 'Test' in GitLab, you should see something like this in the console: ``` diff --git a/lib/gitlab/data_builder/push.rb b/lib/gitlab/data_builder/push.rb index e81d19a7a2e..8c8729b6557 100644 --- a/lib/gitlab/data_builder/push.rb +++ b/lib/gitlab/data_builder/push.rb @@ -74,6 +74,8 @@ module Gitlab build(project, user, commits.last&.id, commits.first&.id, ref, commits) end + private + def checkout_sha(repository, newrev, ref) # Checkout sha is nil when we remove branch or tag return if Gitlab::Git.blank_ref?(newrev) diff --git a/lib/gitlab/data_builder/wiki_page.rb b/lib/gitlab/data_builder/wiki_page.rb new file mode 100644 index 00000000000..226974b698c --- /dev/null +++ b/lib/gitlab/data_builder/wiki_page.rb @@ -0,0 +1,22 @@ +module Gitlab + module DataBuilder + module WikiPage + extend self + + def build(wiki_page, user, action) + wiki = wiki_page.wiki + + { + object_kind: wiki_page.class.name.underscore, + user: user.hook_attrs, + project: wiki.project.hook_attrs, + wiki: wiki.hook_attrs, + object_attributes: wiki_page.hook_attrs.merge( + url: Gitlab::UrlBuilder.build(wiki_page), + action: action + ) + } + end + end + end +end diff --git a/spec/controllers/projects/hooks_controller_spec.rb b/spec/controllers/projects/hooks_controller_spec.rb new file mode 100644 index 00000000000..b93ab220f4d --- /dev/null +++ b/spec/controllers/projects/hooks_controller_spec.rb @@ -0,0 +1,21 @@ +require 'spec_helper' + +describe Projects::HooksController do + let(:project) { create(:empty_project) } + let(:user) { create(:user) } + + before do + project.team << [user, :master] + sign_in(user) + end + + describe '#index' do + it 'redirects to settings/integrations page' do + get(:index, namespace_id: project.namespace, project_id: project) + + expect(response).to redirect_to( + project_settings_integrations_path(project) + ) + end + end +end diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb index 678cebe365b..5bba1dec7db 100644 --- a/spec/factories/ci/builds.rb +++ b/spec/factories/ci/builds.rb @@ -110,7 +110,7 @@ FactoryGirl.define do end after(:build) do |build, evaluator| - build.project = build.pipeline.project + build.project ||= build.pipeline.project end factory :ci_not_started_build do diff --git a/spec/factories/project_hooks.rb b/spec/factories/project_hooks.rb index cd754ea235f..d754e980931 100644 --- a/spec/factories/project_hooks.rb +++ b/spec/factories/project_hooks.rb @@ -2,6 +2,7 @@ FactoryGirl.define do factory :project_hook do url { generate(:url) } enable_ssl_verification false + project factory: :empty_project trait :token do token { SecureRandom.hex(10) } diff --git a/spec/features/admin/admin_hooks_spec.rb b/spec/features/admin/admin_hooks_spec.rb index 1e675fc0ce7..9a438b65e68 100644 --- a/spec/features/admin/admin_hooks_spec.rb +++ b/spec/features/admin/admin_hooks_spec.rb @@ -74,11 +74,13 @@ describe 'Admin::Hooks', feature: true do end end - describe 'Test' do + describe 'Test', js: true do before do WebMock.stub_request(:post, @system_hook.url) visit admin_hooks_path - click_link 'Test hook' + + find('.hook-test-button.dropdown').click + click_link 'Push events' end it { expect(current_path).to eq(admin_hooks_path) } diff --git a/spec/features/projects/settings/integration_settings_spec.rb b/spec/features/projects/settings/integration_settings_spec.rb index 13313bfde24..6ae242af87f 100644 --- a/spec/features/projects/settings/integration_settings_spec.rb +++ b/spec/features/projects/settings/integration_settings_spec.rb @@ -36,14 +36,14 @@ feature 'Integration settings', feature: true do expect(page.status_code).to eq(200) expect(page).to have_content(hook.url) expect(page).to have_content('SSL Verification: enabled') - expect(page).to have_content('Push Events') - expect(page).to have_content('Tag Push Events') - expect(page).to have_content('Issues Events') - expect(page).to have_content('Confidential Issues Events') - expect(page).to have_content('Note Events') - expect(page).to have_content('Merge Requests Events') - expect(page).to have_content('Pipeline Events') - expect(page).to have_content('Wiki Page Events') + expect(page).to have_content('Push events') + expect(page).to have_content('Tag push events') + expect(page).to have_content('Issues events') + expect(page).to have_content('Confidential issues events') + expect(page).to have_content('Note events') + expect(page).to have_content('Merge requests events') + expect(page).to have_content('Pipeline events') + expect(page).to have_content('Wiki page events') end scenario 'create webhook' do @@ -58,8 +58,8 @@ feature 'Integration settings', feature: true do expect(page).to have_content(url) expect(page).to have_content('SSL Verification: enabled') - expect(page).to have_content('Push Events') - expect(page).to have_content('Tag Push Events') + expect(page).to have_content('Push events') + expect(page).to have_content('Tag push events') expect(page).to have_content('Job events') end @@ -76,11 +76,12 @@ feature 'Integration settings', feature: true do expect(page).to have_content(url) end - scenario 'test existing webhook' do + scenario 'test existing webhook', js: true do WebMock.stub_request(:post, hook.url) visit integrations_path - click_link 'Test' + find('.hook-test-button.dropdown').click + click_link 'Push events' expect(current_path).to eq(integrations_path) end diff --git a/spec/helpers/hooks_helper_spec.rb b/spec/helpers/hooks_helper_spec.rb new file mode 100644 index 00000000000..9f0004bf8cf --- /dev/null +++ b/spec/helpers/hooks_helper_spec.rb @@ -0,0 +1,20 @@ +require 'spec_helper' + +describe HooksHelper do + let(:project) { create(:empty_project) } + let(:project_hook) { create(:project_hook, project: project) } + let(:system_hook) { create(:system_hook) } + let(:trigger) { 'push_events' } + + describe '#link_to_test_hook' do + it 'returns project namespaced link' do + expect(helper.link_to_test_hook(project_hook, trigger)) + .to include("href=\"#{test_project_hook_path(project, project_hook, trigger: trigger)}\"") + end + + it 'returns admin namespaced link' do + expect(helper.link_to_test_hook(system_hook, trigger)) + .to include("href=\"#{test_admin_hook_path(system_hook, trigger: trigger)}\"") + end + end +end diff --git a/spec/lib/gitlab/data_builder/wiki_page_spec.rb b/spec/lib/gitlab/data_builder/wiki_page_spec.rb new file mode 100644 index 00000000000..a776d888c47 --- /dev/null +++ b/spec/lib/gitlab/data_builder/wiki_page_spec.rb @@ -0,0 +1,21 @@ +require 'spec_helper' + +describe Gitlab::DataBuilder::WikiPage do + let(:project) { create(:project, :repository) } + let(:wiki_page) { create(:wiki_page, wiki: project.wiki) } + let(:user) { create(:user) } + + describe '.build' do + let(:data) { described_class.build(wiki_page, user, 'create') } + + it { expect(data).to be_a(Hash) } + it { expect(data[:object_kind]).to eq('wiki_page') } + it { expect(data[:user]).to eq(user.hook_attrs) } + it { expect(data[:project]).to eq(project.hook_attrs) } + it { expect(data[:wiki]).to eq(project.wiki.hook_attrs) } + + it { expect(data[:object_attributes]).to include(wiki_page.hook_attrs) } + it { expect(data[:object_attributes]).to include(url: Gitlab::UrlBuilder.build(wiki_page)) } + it { expect(data[:object_attributes]).to include(action: 'create') } + end +end diff --git a/spec/models/hooks/project_hook_spec.rb b/spec/models/hooks/project_hook_spec.rb index 474ae62ccec..0af270014b5 100644 --- a/spec/models/hooks/project_hook_spec.rb +++ b/spec/models/hooks/project_hook_spec.rb @@ -1,10 +1,14 @@ require 'spec_helper' describe ProjectHook, models: true do - describe "Associations" do + describe 'associations' do it { is_expected.to belong_to :project } end + describe 'validations' do + it { is_expected.to validate_presence_of(:project) } + end + describe '.push_hooks' do it 'returns hooks for push events only' do hook = create(:project_hook, push_events: true) diff --git a/spec/models/hooks/service_hook_spec.rb b/spec/models/hooks/service_hook_spec.rb index 57454d2a773..8e871a41a8c 100644 --- a/spec/models/hooks/service_hook_spec.rb +++ b/spec/models/hooks/service_hook_spec.rb @@ -5,6 +5,10 @@ describe ServiceHook, models: true do it { is_expected.to belong_to :service } end + describe 'validations' do + it { is_expected.to validate_presence_of(:service) } + end + describe 'execute' do let(:hook) { build(:service_hook) } let(:data) { { key: 'value' } } diff --git a/spec/models/hooks/system_hook_spec.rb b/spec/models/hooks/system_hook_spec.rb index 0d2b622132e..559778257fa 100644 --- a/spec/models/hooks/system_hook_spec.rb +++ b/spec/models/hooks/system_hook_spec.rb @@ -7,8 +7,7 @@ describe SystemHook, models: true do it 'sets defined default parameters' do attrs = { push_events: false, - repository_update_events: true, - enable_ssl_verification: true + repository_update_events: true } expect(system_hook).to have_attributes(attrs) end diff --git a/spec/models/project_services/microsoft_teams_service_spec.rb b/spec/models/project_services/microsoft_teams_service_spec.rb index bd50a2d1470..fb95c4cda35 100644 --- a/spec/models/project_services/microsoft_teams_service_spec.rb +++ b/spec/models/project_services/microsoft_teams_service_spec.rb @@ -112,7 +112,7 @@ describe MicrosoftTeamsService, models: true do let(:wiki_page_sample_data) do service = WikiPages::CreateService.new(project, user, opts) wiki_page = service.execute - service.hook_data(wiki_page, 'create') + Gitlab::DataBuilder::WikiPage.build(wiki_page, user, 'create') end it "calls Microsoft Teams API" do diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 90769b580cd..fdcb011d685 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -309,10 +309,14 @@ describe Project, models: true do end describe 'delegation' do - it { is_expected.to delegate_method(:add_guest).to(:team) } - it { is_expected.to delegate_method(:add_reporter).to(:team) } - it { is_expected.to delegate_method(:add_developer).to(:team) } - it { is_expected.to delegate_method(:add_master).to(:team) } + [:add_guest, :add_reporter, :add_developer, :add_master, :add_user, :add_users].each do |method| + it { is_expected.to delegate_method(method).to(:team) } + end + + it { is_expected.to delegate_method(:empty_repo?).to(:repository) } + it { is_expected.to delegate_method(:members).to(:team).with_prefix(true) } + it { is_expected.to delegate_method(:count).to(:forks).with_prefix(true) } + it { is_expected.to delegate_method(:name).to(:owner).with_prefix(true).with_arguments(allow_nil: true) } end describe '#to_reference' do diff --git a/spec/services/test_hook_service_spec.rb b/spec/services/test_hook_service_spec.rb deleted file mode 100644 index f99fd8434c2..00000000000 --- a/spec/services/test_hook_service_spec.rb +++ /dev/null @@ -1,14 +0,0 @@ -require 'spec_helper' - -describe TestHookService, services: true do - let(:user) { create(:user) } - let(:project) { create(:project, :repository) } - let(:hook) { create(:project_hook, project: project) } - - describe '#execute' do - it "executes successfully" do - stub_request(:post, hook.url).to_return(status: 200) - expect(TestHookService.new.execute(hook, user)).to be_truthy - end - end -end diff --git a/spec/services/test_hooks/project_service_spec.rb b/spec/services/test_hooks/project_service_spec.rb new file mode 100644 index 00000000000..4218c15a3ce --- /dev/null +++ b/spec/services/test_hooks/project_service_spec.rb @@ -0,0 +1,188 @@ +require 'spec_helper' + +describe TestHooks::ProjectService do + let(:current_user) { create(:user) } + + describe '#execute' do + let(:project) { create(:project, :repository) } + let(:hook) { create(:project_hook, project: project) } + let(:service) { described_class.new(hook, current_user, trigger) } + let(:sample_data) { { data: 'sample' } } + let(:success_result) { { status: :success, http_status: 200, message: 'ok' } } + + context 'hook with not implemented test' do + let(:trigger) { 'not_implemented_events' } + + it 'returns error message' do + expect(hook).not_to receive(:execute) + expect(service.execute).to include({ status: :error, message: 'Testing not available for this hook' }) + end + end + + context 'push_events' do + let(:trigger) { 'push_events' } + + it 'returns error message if not enough data' do + allow(project).to receive(:empty_repo?).and_return(true) + + expect(hook).not_to receive(:execute) + expect(service.execute).to include({ status: :error, message: 'Ensure the project has at least one commit.' }) + end + + it 'executes hook' do + allow(project).to receive(:empty_repo?).and_return(false) + allow(Gitlab::DataBuilder::Push).to receive(:build_sample).and_return(sample_data) + + expect(hook).to receive(:execute).with(sample_data, trigger).and_return(success_result) + expect(service.execute).to include(success_result) + end + end + + context 'tag_push_events' do + let(:trigger) { 'tag_push_events' } + + it 'returns error message if not enough data' do + allow(project).to receive(:empty_repo?).and_return(true) + + expect(hook).not_to receive(:execute) + expect(service.execute).to include({ status: :error, message: 'Ensure the project has at least one commit.' }) + end + + it 'executes hook' do + allow(project).to receive(:empty_repo?).and_return(false) + allow(Gitlab::DataBuilder::Push).to receive(:build_sample).and_return(sample_data) + + expect(hook).to receive(:execute).with(sample_data, trigger).and_return(success_result) + expect(service.execute).to include(success_result) + end + end + + context 'note_events' do + let(:trigger) { 'note_events' } + + it 'returns error message if not enough data' do + expect(hook).not_to receive(:execute) + expect(service.execute).to include({ status: :error, message: 'Ensure the project has notes.' }) + end + + it 'executes hook' do + allow(project).to receive(:notes).and_return([Note.new]) + allow(Gitlab::DataBuilder::Note).to receive(:build).and_return(sample_data) + + expect(hook).to receive(:execute).with(sample_data, trigger).and_return(success_result) + expect(service.execute).to include(success_result) + end + end + + context 'issues_events' do + let(:trigger) { 'issues_events' } + let(:issue) { build(:issue) } + + it 'returns error message if not enough data' do + expect(hook).not_to receive(:execute) + expect(service.execute).to include({ status: :error, message: 'Ensure the project has issues.' }) + end + + it 'executes hook' do + allow(project).to receive(:issues).and_return([issue]) + allow(issue).to receive(:to_hook_data).and_return(sample_data) + + expect(hook).to receive(:execute).with(sample_data, trigger).and_return(success_result) + expect(service.execute).to include(success_result) + end + end + + context 'confidential_issues_events' do + let(:trigger) { 'confidential_issues_events' } + let(:issue) { build(:issue) } + + it 'returns error message if not enough data' do + expect(hook).not_to receive(:execute) + expect(service.execute).to include({ status: :error, message: 'Ensure the project has issues.' }) + end + + it 'executes hook' do + allow(project).to receive(:issues).and_return([issue]) + allow(issue).to receive(:to_hook_data).and_return(sample_data) + + expect(hook).to receive(:execute).with(sample_data, trigger).and_return(success_result) + expect(service.execute).to include(success_result) + end + end + + context 'merge_requests_events' do + let(:trigger) { 'merge_requests_events' } + + it 'returns error message if not enough data' do + expect(hook).not_to receive(:execute) + expect(service.execute).to include({ status: :error, message: 'Ensure the project has merge requests.' }) + end + + it 'executes hook' do + 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).and_return(success_result) + expect(service.execute).to include(success_result) + end + end + + context 'job_events' do + let(:trigger) { 'job_events' } + + it 'returns error message if not enough data' do + expect(hook).not_to receive(:execute) + expect(service.execute).to include({ status: :error, message: 'Ensure the project has CI jobs.' }) + end + + it 'executes hook' do + create(:ci_build, project: project) + allow(Gitlab::DataBuilder::Build).to receive(:build).and_return(sample_data) + + expect(hook).to receive(:execute).with(sample_data, trigger).and_return(success_result) + expect(service.execute).to include(success_result) + end + end + + context 'pipeline_events' do + let(:trigger) { 'pipeline_events' } + + it 'returns error message if not enough data' do + expect(hook).not_to receive(:execute) + expect(service.execute).to include({ status: :error, message: 'Ensure the project has CI pipelines.' }) + end + + it 'executes hook' do + create(:ci_empty_pipeline, project: project) + allow(Gitlab::DataBuilder::Pipeline).to receive(:build).and_return(sample_data) + + expect(hook).to receive(:execute).with(sample_data, trigger).and_return(success_result) + expect(service.execute).to include(success_result) + end + end + + context 'wiki_page_events' do + let(:trigger) { 'wiki_page_events' } + + it 'returns error message if wiki disabled' do + allow(project).to receive(:wiki_enabled?).and_return(false) + + expect(hook).not_to receive(:execute) + expect(service.execute).to include({ status: :error, message: 'Ensure the wiki is enabled and has pages.' }) + end + + it 'returns error message if not enough data' do + expect(hook).not_to receive(:execute) + expect(service.execute).to include({ status: :error, message: 'Ensure the wiki is enabled and has pages.' }) + end + + it 'executes hook' do + create(:wiki_page, wiki: project.wiki) + allow(Gitlab::DataBuilder::WikiPage).to receive(:build).and_return(sample_data) + + expect(hook).to receive(:execute).with(sample_data, trigger).and_return(success_result) + expect(service.execute).to include(success_result) + end + end + end +end diff --git a/spec/services/test_hooks/system_service_spec.rb b/spec/services/test_hooks/system_service_spec.rb new file mode 100644 index 00000000000..00d89924766 --- /dev/null +++ b/spec/services/test_hooks/system_service_spec.rb @@ -0,0 +1,82 @@ +require 'spec_helper' + +describe TestHooks::SystemService do + let(:current_user) { create(:user) } + + describe '#execute' do + let(:project) { create(:project, :repository) } + let(:hook) { create(:system_hook) } + let(:service) { described_class.new(hook, current_user, trigger) } + let(:sample_data) { { data: 'sample' }} + let(:success_result) { { status: :success, http_status: 200, message: 'ok' } } + + before do + allow(Project).to receive(:first).and_return(project) + end + + context 'hook with not implemented test' do + let(:trigger) { 'not_implemented_events' } + + it 'returns error message' do + expect(hook).not_to receive(:execute) + expect(service.execute).to include({ status: :error, message: 'Testing not available for this hook' }) + end + end + + context 'push_events' do + let(:trigger) { 'push_events' } + + it 'returns error message if not enough data' do + allow(project).to receive(:empty_repo?).and_return(true) + + expect(hook).not_to receive(:execute) + expect(service.execute).to include({ status: :error, message: "Ensure project \"#{project.human_name}\" has commits." }) + end + + it 'executes hook' do + allow(project).to receive(:empty_repo?).and_return(false) + allow(Gitlab::DataBuilder::Push).to receive(:build_sample).and_return(sample_data) + + expect(hook).to receive(:execute).with(sample_data, trigger).and_return(success_result) + expect(service.execute).to include(success_result) + end + end + + context 'tag_push_events' do + let(:trigger) { 'tag_push_events' } + + it 'returns error message if not enough data' do + allow(project.repository).to receive(:tags).and_return([]) + + expect(hook).not_to receive(:execute) + expect(service.execute).to include({ status: :error, message: "Ensure project \"#{project.human_name}\" has tags." }) + end + + it 'executes hook' do + allow(project.repository).to receive(:tags).and_return(['tag']) + allow(Gitlab::DataBuilder::Push).to receive(:build_sample).and_return(sample_data) + + expect(hook).to receive(:execute).with(sample_data, trigger).and_return(success_result) + expect(service.execute).to include(success_result) + end + end + + context 'repository_update_events' do + let(:trigger) { 'repository_update_events' } + + it 'returns error message if not enough data' do + allow(project).to receive(:commit).and_return(nil) + expect(hook).not_to receive(:execute) + expect(service.execute).to include({ status: :error, message: "Ensure project \"#{project.human_name}\" has commits." }) + end + + it 'executes hook' do + allow(project).to receive(:empty_repo?).and_return(false) + allow(Gitlab::DataBuilder::Repository).to receive(:update).and_return(sample_data) + + expect(hook).to receive(:execute).with(sample_data, trigger).and_return(success_result) + expect(service.execute).to include(success_result) + end + end + end +end diff --git a/spec/services/web_hook_service_spec.rb b/spec/services/web_hook_service_spec.rb index b5abc46e80c..7ff37c22963 100644 --- a/spec/services/web_hook_service_spec.rb +++ b/spec/services/web_hook_service_spec.rb @@ -58,7 +58,7 @@ describe WebHookService, services: true do exception = exception_class.new('Exception message') WebMock.stub_request(:post, project_hook.url).to_raise(exception) - expect(service_instance.execute).to eq([nil, exception.message]) + expect(service_instance.execute).to eq({ status: :error, message: exception.message }) expect { service_instance.execute }.not_to raise_error end end @@ -66,13 +66,13 @@ describe WebHookService, services: true do it 'handles 200 status code' do WebMock.stub_request(:post, project_hook.url).to_return(status: 200, body: 'Success') - expect(service_instance.execute).to eq([200, 'Success']) + expect(service_instance.execute).to include({ status: :success, http_status: 200, message: 'Success' }) end it 'handles 2xx status codes' do WebMock.stub_request(:post, project_hook.url).to_return(status: 201, body: 'Success') - expect(service_instance.execute).to eq([201, 'Success']) + expect(service_instance.execute).to include({ status: :success, http_status: 201, message: 'Success' }) end context 'execution logging' do diff --git a/spec/support/slack_mattermost_notifications_shared_examples.rb b/spec/support/slack_mattermost_notifications_shared_examples.rb index 044c09d5fde..6accf16bea4 100644 --- a/spec/support/slack_mattermost_notifications_shared_examples.rb +++ b/spec/support/slack_mattermost_notifications_shared_examples.rb @@ -78,7 +78,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do wiki_page_service = WikiPages::CreateService.new(project, user, opts) @wiki_page = wiki_page_service.execute - @wiki_page_sample_data = wiki_page_service.hook_data(@wiki_page, 'create') + @wiki_page_sample_data = Gitlab::DataBuilder::WikiPage.build(@wiki_page, user, 'create') end it "calls Slack/Mattermost API for push events" do |
