Wrong data type when testing webhooks

17 files changed, 374 insertions, 41 deletions

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