Merge branch '5971-webhook-testing' into 'master'

Wrong data type when testing webhooks

Closes #5971

See merge request !11728

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

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

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

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

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

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

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
 
-  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) }
+  TRIGGERS.each do |trigger, event|
+    scope trigger, -> { where(event => true) }
+  end
+
+  belongs_to :project
+  validates :project, presence: true
 end

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

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

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)

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

app/services/test_hook_service.rb

@@ -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

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

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

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

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

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

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

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'}

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 }
+

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')

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)

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)

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.
+
+![Webhook testing](img/webhook_testing.png)
+
 ## 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:
 
 ```

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)

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

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

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

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) }

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) }

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

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

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

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)

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' } }

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

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

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

spec/services/test_hook_service_spec.rb

@@ -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

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

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

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

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