From 020ea32e767b9ad033f9fedcaa902865a01fa944 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Tue, 2 Aug 2016 18:06:31 +0800
Subject: [PATCH 01/42] Implement pipeline hooks, extracted from !5525

Closes #20115
---
 CHANGELOG                                     |  1 +
 app/controllers/concerns/service_params.rb    | 15 ++---
 app/controllers/projects/hooks_controller.rb  |  1 +
 app/models/ci/pipeline.rb                     | 10 ++-
 app/models/hooks/project_hook.rb              |  1 +
 app/models/hooks/web_hook.rb                  |  1 +
 app/models/service.rb                         |  5 ++
 app/services/ci/create_pipeline_service.rb    |  1 +
 .../projects/hooks/_project_hook.html.haml    |  2 +-
 app/views/shared/web_hooks/_form.html.haml    |  7 ++
 ...081025_add_pipeline_events_to_web_hooks.rb | 16 +++++
 ...8103734_add_pipeline_events_to_services.rb | 16 +++++
 lib/api/entities.rb                           |  6 +-
 lib/api/project_hooks.rb                      |  2 +
 .../data_builder/pipeline_data_builder.rb     | 66 +++++++++++++++++++
 .../pipeline_data_builder_spec.rb             | 32 +++++++++
 spec/models/build_spec.rb                     |  6 +-
 spec/models/ci/pipeline_spec.rb               | 33 +++++++++-
 spec/requests/api/project_hooks_spec.rb       |  7 +-
 19 files changed, 210 insertions(+), 18 deletions(-)
 create mode 100644 db/migrate/20160728081025_add_pipeline_events_to_web_hooks.rb
 create mode 100644 db/migrate/20160728103734_add_pipeline_events_to_services.rb
 create mode 100644 lib/gitlab/data_builder/pipeline_data_builder.rb
 create mode 100644 spec/lib/gitlab/data_builder/pipeline_data_builder_spec.rb

diff --git a/CHANGELOG b/CHANGELOG
index c099c63ce86..3199e66a0d9 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -28,6 +28,7 @@ v 8.11.0 (unreleased)
   - The overhead of instrumented method calls has been reduced
   - Remove `search_id` of labels dropdown filter to fix 'Missleading URI for labels in Merge Requests and Issues view'. !5368 (Scott Le)
   - Load project invited groups and members eagerly in `ProjectTeam#fetch_members`
+  - Add pipeline events hook
   - Bump gitlab_git to speedup DiffCollection iterations
   - Make branches sortable without push permission !5462 (winniehell)
   - Check for Ci::Build artifacts at database level on pipeline partial
diff --git a/app/controllers/concerns/service_params.rb b/app/controllers/concerns/service_params.rb
index 471d15af913..58877c5ad5d 100644
--- a/app/controllers/concerns/service_params.rb
+++ b/app/controllers/concerns/service_params.rb
@@ -7,11 +7,12 @@ module ServiceParams
                     :build_key, :server, :teamcity_url, :drone_url, :build_type,
                     :description, :issues_url, :new_issue_url, :restrict_to_branch, :channel,
                     :colorize_messages, :channels,
-                    :push_events, :issues_events, :merge_requests_events, :tag_push_events,
-                    :note_events, :build_events, :wiki_page_events,
-                    :notify_only_broken_builds, :add_pusher,
-                    :send_from_committer_email, :disable_diffs, :external_wiki_url,
-                    :notify, :color,
+                    # See app/helpers/services_helper.rb
+                    # for why we need issues_events and merge_requests_events.
+                    :issues_events, :merge_requests_events,
+                    :notify_only_broken_builds, :notify_only_broken_pipelines,
+                    :add_pusher, :send_from_committer_email, :disable_diffs,
+                    :external_wiki_url, :notify, :color,
                     :server_host, :server_port, :default_irc_uri, :enable_ssl_verification,
                     :jira_issue_transition_id]
 
@@ -19,9 +20,7 @@ module ServiceParams
   FILTER_BLANK_PARAMS = [:password]
 
   def service_params
-    dynamic_params = []
-    dynamic_params.concat(@service.event_channel_names)
-
+    dynamic_params = @service.event_channel_names + @service.event_names
     service_params = params.permit(:id, service: ALLOWED_PARAMS + dynamic_params)
 
     if service_params[:service].is_a?(Hash)
diff --git a/app/controllers/projects/hooks_controller.rb b/app/controllers/projects/hooks_controller.rb
index a60027ff477..b5624046387 100644
--- a/app/controllers/projects/hooks_controller.rb
+++ b/app/controllers/projects/hooks_controller.rb
@@ -56,6 +56,7 @@ class Projects::HooksController < Projects::ApplicationController
   def hook_params
     params.require(:hook).permit(
       :build_events,
+      :pipeline_events,
       :enable_ssl_verification,
       :issues_events,
       :merge_requests_events,
diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index bce6a992af6..4e6ccf48c68 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -237,7 +237,15 @@ module Ci
       self.started_at = statuses.started_at
       self.finished_at = statuses.finished_at
       self.duration = statuses.latest.duration
-      save
+      saved = save
+      execute_hooks if saved && !skip_ci?
+      saved
+    end
+
+    def execute_hooks
+      pipeline_data = Gitlab::DataBuilder::PipelineDataBuilder.build(self)
+      project.execute_hooks(pipeline_data, :pipeline_hooks)
+      project.execute_services(pipeline_data.dup, :pipeline_hooks)
     end
 
     def keep_around_commits
diff --git a/app/models/hooks/project_hook.rb b/app/models/hooks/project_hook.rb
index ba42a8eeb70..836a75b0608 100644
--- a/app/models/hooks/project_hook.rb
+++ b/app/models/hooks/project_hook.rb
@@ -5,5 +5,6 @@ class ProjectHook < WebHook
   scope :note_hooks, -> { where(note_events: true) }
   scope :merge_request_hooks, -> { where(merge_requests_events: true) }
   scope :build_hooks, -> { where(build_events: true) }
+  scope :pipeline_hooks, -> { where(pipeline_events: true) }
   scope :wiki_page_hooks, ->  { where(wiki_page_events: true) }
 end
diff --git a/app/models/hooks/web_hook.rb b/app/models/hooks/web_hook.rb
index 8b87b6c3d64..f365dee3141 100644
--- a/app/models/hooks/web_hook.rb
+++ b/app/models/hooks/web_hook.rb
@@ -8,6 +8,7 @@ class WebHook < ActiveRecord::Base
   default_value_for :merge_requests_events, false
   default_value_for :tag_push_events, false
   default_value_for :build_events, false
+  default_value_for :pipeline_events, false
   default_value_for :enable_ssl_verification, true
 
   scope :push_hooks, -> { where(push_events: true) }
diff --git a/app/models/service.rb b/app/models/service.rb
index 40cd9b861f0..e4cd44f542a 100644
--- a/app/models/service.rb
+++ b/app/models/service.rb
@@ -36,6 +36,7 @@ class Service < ActiveRecord::Base
   scope :merge_request_hooks, -> { where(merge_requests_events: true, active: true) }
   scope :note_hooks, -> { where(note_events: true, active: true) }
   scope :build_hooks, -> { where(build_events: true, active: true) }
+  scope :pipeline_hooks, -> { where(pipeline_events: true, active: true) }
   scope :wiki_page_hooks, -> { where(wiki_page_events: true, active: true) }
   scope :external_issue_trackers, -> { issue_trackers.active.without_defaults }
 
@@ -86,6 +87,10 @@ class Service < ActiveRecord::Base
     []
   end
 
+  def event_names
+    supported_events.map { |event| "#{event}_events" }
+  end
+
   def event_field(event)
     nil
   end
diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb
index be91bf0db85..7a8b0683acb 100644
--- a/app/services/ci/create_pipeline_service.rb
+++ b/app/services/ci/create_pipeline_service.rb
@@ -27,6 +27,7 @@ module Ci
       end
 
       pipeline.save!
+      pipeline.touch
 
       unless pipeline.create_builds(current_user)
         pipeline.errors.add(:base, 'No builds for this pipeline.')
diff --git a/app/views/projects/hooks/_project_hook.html.haml b/app/views/projects/hooks/_project_hook.html.haml
index 8151187d499..3fcf1692e09 100644
--- a/app/views/projects/hooks/_project_hook.html.haml
+++ b/app/views/projects/hooks/_project_hook.html.haml
@@ -3,7 +3,7 @@
     .col-md-8.col-lg-7
       %strong.light-header= hook.url
       %div
-        - %w(push_events tag_push_events issues_events note_events merge_requests_events build_events wiki_page_events).each do |trigger|
+        - %w(push_events tag_push_events issues_events note_events merge_requests_events build_events pipeline_events wiki_page_events).each do |trigger|
           - if hook.send(trigger)
             %span.label.label-gray.deploy-project-label= trigger.titleize
     .col-md-4.col-lg-5.text-right-lg.prepend-top-5
diff --git a/app/views/shared/web_hooks/_form.html.haml b/app/views/shared/web_hooks/_form.html.haml
index 2585ed9360b..106161d6515 100644
--- a/app/views/shared/web_hooks/_form.html.haml
+++ b/app/views/shared/web_hooks/_form.html.haml
@@ -65,6 +65,13 @@
                 %strong Build events
               %p.light
                 This url will be triggered when the build status changes
+          %li
+            = f.check_box :pipeline_events, class: 'pull-left'
+            .prepend-left-20
+              = f.label :pipeline_events, class: 'list-label' do
+                %strong Pipeline events
+              %p.light
+                This url will be triggered when the pipeline status changes
           %li
             = f.check_box :wiki_page_events, class: 'pull-left'
             .prepend-left-20
diff --git a/db/migrate/20160728081025_add_pipeline_events_to_web_hooks.rb b/db/migrate/20160728081025_add_pipeline_events_to_web_hooks.rb
new file mode 100644
index 00000000000..b800e6d7283
--- /dev/null
+++ b/db/migrate/20160728081025_add_pipeline_events_to_web_hooks.rb
@@ -0,0 +1,16 @@
+class AddPipelineEventsToWebHooks < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_column_with_default(:web_hooks, :pipeline_events, :boolean,
+                            default: false, allow_null: false)
+  end
+
+  def down
+    remove_column(:web_hooks, :pipeline_events)
+  end
+end
diff --git a/db/migrate/20160728103734_add_pipeline_events_to_services.rb b/db/migrate/20160728103734_add_pipeline_events_to_services.rb
new file mode 100644
index 00000000000..bcd24fe1566
--- /dev/null
+++ b/db/migrate/20160728103734_add_pipeline_events_to_services.rb
@@ -0,0 +1,16 @@
+class AddPipelineEventsToServices < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_column_with_default(:services, :pipeline_events, :boolean,
+                            default: false, allow_null: false)
+  end
+
+  def down
+    remove_column(:services, :pipeline_events)
+  end
+end
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 3e21b7a0b8a..b6f6b11d97b 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -48,7 +48,8 @@ module API
 
     class ProjectHook < Hook
       expose :project_id, :push_events
-      expose :issues_events, :merge_requests_events, :tag_push_events, :note_events, :build_events
+      expose :issues_events, :merge_requests_events, :tag_push_events
+      expose :note_events, :build_events, :pipeline_events
       expose :enable_ssl_verification
     end
 
@@ -342,7 +343,8 @@ module API
 
     class ProjectService < Grape::Entity
       expose :id, :title, :created_at, :updated_at, :active
-      expose :push_events, :issues_events, :merge_requests_events, :tag_push_events, :note_events, :build_events
+      expose :push_events, :issues_events, :merge_requests_events
+      expose :tag_push_events, :note_events, :build_events, :pipeline_events
       # Expose serialized properties
       expose :properties do |service, options|
         field_names = service.fields.
diff --git a/lib/api/project_hooks.rb b/lib/api/project_hooks.rb
index 6bb70bc8bc3..3f63cd678e8 100644
--- a/lib/api/project_hooks.rb
+++ b/lib/api/project_hooks.rb
@@ -45,6 +45,7 @@ module API
           :tag_push_events,
           :note_events,
           :build_events,
+          :pipeline_events,
           :enable_ssl_verification
         ]
         @hook = user_project.hooks.new(attrs)
@@ -78,6 +79,7 @@ module API
           :tag_push_events,
           :note_events,
           :build_events,
+          :pipeline_events,
           :enable_ssl_verification
         ]
 
diff --git a/lib/gitlab/data_builder/pipeline_data_builder.rb b/lib/gitlab/data_builder/pipeline_data_builder.rb
new file mode 100644
index 00000000000..13417ba09eb
--- /dev/null
+++ b/lib/gitlab/data_builder/pipeline_data_builder.rb
@@ -0,0 +1,66 @@
+module Gitlab
+  module DataBuilder
+    module PipelineDataBuilder
+      module_function
+
+      def build(pipeline)
+        {
+          object_kind: 'pipeline',
+          object_attributes: hook_attrs(pipeline),
+          user: pipeline.user.try(:hook_attrs),
+          project: pipeline.project.hook_attrs(backward: false),
+          commit: pipeline.commit.try(:hook_attrs),
+          builds: pipeline.builds.map(&method(:build_hook_attrs))
+        }
+      end
+
+      def hook_attrs(pipeline)
+        first_pending_build = pipeline.builds.first_pending
+        config_processor = pipeline.config_processor
+
+        {
+          id: pipeline.id,
+          ref: pipeline.ref,
+          tag: pipeline.tag,
+          sha: pipeline.sha,
+          before_sha: pipeline.before_sha,
+          status: pipeline.status,
+          stage: first_pending_build.try(:stage),
+          stages: config_processor.try(:stages),
+          created_at: pipeline.created_at,
+          finished_at: pipeline.finished_at,
+          duration: pipeline.duration
+        }
+      end
+
+      def build_hook_attrs(build)
+        {
+          id: build.id,
+          stage: build.stage,
+          name: build.name,
+          status: build.status,
+          created_at: build.created_at,
+          started_at: build.started_at,
+          finished_at: build.finished_at,
+          when: build.when,
+          manual: build.manual?,
+          user: build.user.try(:hook_attrs),
+          runner: build.runner && runner_hook_attrs(build.runner),
+          artifacts_file: {
+            filename: build.artifacts_file.filename,
+            size: build.artifacts_size
+          }
+        }
+      end
+
+      def runner_hook_attrs(runner)
+        {
+          id: runner.id,
+          description: runner.description,
+          active: runner.active?,
+          is_shared: runner.is_shared?
+        }
+      end
+    end
+  end
+end
diff --git a/spec/lib/gitlab/data_builder/pipeline_data_builder_spec.rb b/spec/lib/gitlab/data_builder/pipeline_data_builder_spec.rb
new file mode 100644
index 00000000000..24d39b318c0
--- /dev/null
+++ b/spec/lib/gitlab/data_builder/pipeline_data_builder_spec.rb
@@ -0,0 +1,32 @@
+require 'spec_helper'
+
+describe Gitlab::DataBuilder::PipelineDataBuilder do
+  let(:user) { create(:user) }
+  let(:project) { create(:project) }
+  let(:pipeline) do
+    create(:ci_pipeline,
+           project: project, status: 'success',
+           sha: project.commit.sha, ref: project.default_branch)
+  end
+  let!(:build) { create(:ci_build, pipeline: pipeline) }
+
+  describe '.build' do
+    let(:data) { Gitlab::DataBuilder::PipelineDataBuilder.build(pipeline) }
+    let(:attributes) { data[:object_attributes] }
+    let(:build_data) { data[:builds].first }
+    let(:project_data) { data[:project] }
+
+    it { expect(attributes).to be_a(Hash) }
+    it { expect(attributes[:ref]).to eq(pipeline.ref) }
+    it { expect(attributes[:sha]).to eq(pipeline.sha) }
+    it { expect(attributes[:tag]).to eq(pipeline.tag) }
+    it { expect(attributes[:id]).to eq(pipeline.id) }
+    it { expect(attributes[:status]).to eq(pipeline.status) }
+
+    it { expect(build_data).to be_a(Hash) }
+    it { expect(build_data[:id]).to eq(build.id) }
+    it { expect(build_data[:status]).to eq(build.status) }
+
+    it { expect(project_data).to eq(project.hook_attrs(backward: false)) }
+  end
+end
diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb
index dc88697199b..47c489e6af1 100644
--- a/spec/models/build_spec.rb
+++ b/spec/models/build_spec.rb
@@ -275,7 +275,8 @@ describe Ci::Build, models: true do
 
     context 'when yaml_variables are undefined' do
       before do
-        build.yaml_variables = nil
+        build.update(yaml_variables: nil)
+        build.reload # reload pipeline so that it resets config_processor
       end
 
       context 'use from gitlab-ci.yml' do
@@ -854,7 +855,8 @@ describe Ci::Build, models: true do
 
     context 'if is undefined' do
       before do
-        build.when = nil
+        build.update(when: nil)
+        build.reload # reload pipeline so that it resets config_processor
       end
 
       context 'use from gitlab-ci.yml' do
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index 0d4c86955ce..aa05fc78f94 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -513,7 +513,7 @@ describe Ci::Pipeline, models: true do
         create :ci_build, :success, pipeline: pipeline, name: 'rspec'
         create :ci_build, :allowed_to_fail, :failed, pipeline: pipeline, name: 'rubocop'
       end
-      
+
       it 'returns true' do
         is_expected.to be_truthy
       end
@@ -524,7 +524,7 @@ describe Ci::Pipeline, models: true do
         create :ci_build, :success, pipeline: pipeline, name: 'rspec'
         create :ci_build, :allowed_to_fail, :success, pipeline: pipeline, name: 'rubocop'
       end
-      
+
       it 'returns false' do
         is_expected.to be_falsey
       end
@@ -542,4 +542,33 @@ describe Ci::Pipeline, models: true do
       end
     end
   end
+
+  describe '#execute_hooks' do
+    let!(:hook) do
+      create(:project_hook, project: project, pipeline_events: enabled)
+    end
+    let(:enabled) { raise NotImplementedError }
+
+    before do
+      WebMock.stub_request(:post, hook.url)
+      pipeline.touch
+      ProjectWebHookWorker.drain
+    end
+
+    context 'with pipeline hooks enabled' do
+      let(:enabled) { true }
+
+      it 'executes pipeline_hook after touched' do
+        expect(WebMock).to have_requested(:post, hook.url).once
+      end
+    end
+
+    context 'with pipeline hooks disabled' do
+      let(:enabled) { false }
+
+      it 'did not execute pipeline_hook after touched' do
+        expect(WebMock).not_to have_requested(:post, hook.url)
+      end
+    end
+  end
 end
diff --git a/spec/requests/api/project_hooks_spec.rb b/spec/requests/api/project_hooks_spec.rb
index fd1fffa6223..504deed81f9 100644
--- a/spec/requests/api/project_hooks_spec.rb
+++ b/spec/requests/api/project_hooks_spec.rb
@@ -8,8 +8,9 @@ describe API::API, 'ProjectHooks', api: true do
   let!(:hook) do
     create(:project_hook,
            project: project, url: "http://example.com",
-           push_events: true, merge_requests_events: true, tag_push_events: true,
-           issues_events: true, note_events: true, build_events: true,
+           push_events: true, merge_requests_events: true,
+           tag_push_events: true, issues_events: true, note_events: true,
+           build_events: true, pipeline_events: true,
            enable_ssl_verification: true)
   end
 
@@ -33,6 +34,7 @@ describe API::API, 'ProjectHooks', api: true do
         expect(json_response.first['tag_push_events']).to eq(true)
         expect(json_response.first['note_events']).to eq(true)
         expect(json_response.first['build_events']).to eq(true)
+        expect(json_response.first['pipeline_events']).to eq(true)
         expect(json_response.first['enable_ssl_verification']).to eq(true)
       end
     end
@@ -91,6 +93,7 @@ describe API::API, 'ProjectHooks', api: true do
       expect(json_response['tag_push_events']).to eq(false)
       expect(json_response['note_events']).to eq(false)
       expect(json_response['build_events']).to eq(false)
+      expect(json_response['pipeline_events']).to eq(false)
       expect(json_response['enable_ssl_verification']).to eq(true)
     end
 

From 62f115dd25c4d3639dceac1b3b81c9fe42eeedd3 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Wed, 3 Aug 2016 14:35:11 +0800
Subject: [PATCH 02/42] Introduce execute_hooks_unless_ci_skipped

---
 app/models/ci/pipeline.rb | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 4e6ccf48c68..f8506e33295 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -18,6 +18,7 @@ module Ci
 
     # Invalidate object and save if when touched
     after_touch :update_state
+    after_touch :execute_hooks_unless_ci_skipped
     after_save :keep_around_commits
 
     # ref can't be HEAD or SHA, can only be branch/tag name
@@ -237,9 +238,11 @@ module Ci
       self.started_at = statuses.started_at
       self.finished_at = statuses.finished_at
       self.duration = statuses.latest.duration
-      saved = save
-      execute_hooks if saved && !skip_ci?
-      saved
+      save
+    end
+
+    def execute_hooks_unless_ci_skipped
+      execute_hooks unless skip_ci?
     end
 
     def execute_hooks

From 853b0dffe787aa185d299ac1868619a231b6965b Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Wed, 3 Aug 2016 17:24:59 +0800
Subject: [PATCH 03/42] Use when instead of if, feedback:

https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5620/diffs#note_13540211
---
 spec/models/build_spec.rb | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb
index 47c489e6af1..1837986e0f4 100644
--- a/spec/models/build_spec.rb
+++ b/spec/models/build_spec.rb
@@ -853,7 +853,7 @@ describe Ci::Build, models: true do
   describe '#when' do
     subject { build.when }
 
-    context 'if is undefined' do
+    context 'when `when` is undefined' do
       before do
         build.update(when: nil)
         build.reload # reload pipeline so that it resets config_processor
@@ -864,13 +864,13 @@ describe Ci::Build, models: true do
           stub_ci_pipeline_yaml_file(config)
         end
 
-        context 'if config is not found' do
+        context 'when config is not found' do
           let(:config) { nil }
 
           it { is_expected.to eq('on_success') }
         end
 
-        context 'if config does not have a questioned job' do
+        context 'when config does not have a questioned job' do
           let(:config) do
             YAML.dump({
                         test_other: {
@@ -882,7 +882,7 @@ describe Ci::Build, models: true do
           it { is_expected.to eq('on_success') }
         end
 
-        context 'if config has when' do
+        context 'when config has when' do
           let(:config) do
             YAML.dump({
                         test: {

From db123d29e73419e9a3e3fc3afe03fd626ae9d776 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Wed, 3 Aug 2016 17:33:34 +0800
Subject: [PATCH 04/42] More descriptive comments, feedback:

https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5620#note_13539571
---
 app/controllers/concerns/service_params.rb | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/app/controllers/concerns/service_params.rb b/app/controllers/concerns/service_params.rb
index 58877c5ad5d..a69877edfd4 100644
--- a/app/controllers/concerns/service_params.rb
+++ b/app/controllers/concerns/service_params.rb
@@ -7,8 +7,12 @@ module ServiceParams
                     :build_key, :server, :teamcity_url, :drone_url, :build_type,
                     :description, :issues_url, :new_issue_url, :restrict_to_branch, :channel,
                     :colorize_messages, :channels,
-                    # See app/helpers/services_helper.rb
-                    # for why we need issues_events and merge_requests_events.
+                    # We're using `issues_events` and `merge_requests_events`
+                    # in the view so we still need to explicitly state them
+                    # here. `Service#event_names` would only give
+                    # `issue_events` and `merge_request_events` (singular!)
+                    # See app/helpers/services_helper.rb for how we
+                    # make those event names plural as special case.
                     :issues_events, :merge_requests_events,
                     :notify_only_broken_builds, :notify_only_broken_pipelines,
                     :add_pusher, :send_from_committer_email, :disable_diffs,

From 3691a391524911a21d5af1c75cb4cd16a8a6e475 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Wed, 3 Aug 2016 23:06:28 +0800
Subject: [PATCH 05/42] We don't have to touch it because builds would touch
 pipeline anyway

---
 app/services/ci/create_pipeline_service.rb   |  1 -
 app/services/create_commit_builds_service.rb | 10 ----------
 2 files changed, 11 deletions(-)

diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb
index 7a8b0683acb..be91bf0db85 100644
--- a/app/services/ci/create_pipeline_service.rb
+++ b/app/services/ci/create_pipeline_service.rb
@@ -27,7 +27,6 @@ module Ci
       end
 
       pipeline.save!
-      pipeline.touch
 
       unless pipeline.create_builds(current_user)
         pipeline.errors.add(:base, 'No builds for this pipeline.')
diff --git a/app/services/create_commit_builds_service.rb b/app/services/create_commit_builds_service.rb
index 0b66b854dea..5e77768abe7 100644
--- a/app/services/create_commit_builds_service.rb
+++ b/app/services/create_commit_builds_service.rb
@@ -53,17 +53,7 @@ class CreateCommitBuildsService
       return false
     end
 
-    save_pipeline!
-  end
-
-  private
-
-  ##
-  # Create a new pipeline and touch object to calculate status
-  #
-  def save_pipeline!
     @pipeline.save!
-    @pipeline.touch
     @pipeline
   end
 end

From 9e06bde269b80b3af01b7cc00bdada1ce2e5e563 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Wed, 3 Aug 2016 23:39:14 +0800
Subject: [PATCH 06/42] Make sure we only fire hooks upon status changed

---
 app/models/ci/pipeline.rb       |  6 +++---
 spec/models/ci/pipeline_spec.rb | 22 +++++++++++++++++++++-
 2 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index f8506e33295..822ba7b6c00 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -18,7 +18,7 @@ module Ci
 
     # Invalidate object and save if when touched
     after_touch :update_state
-    after_touch :execute_hooks_unless_ci_skipped
+    after_save :execute_hooks_if_status_changed
     after_save :keep_around_commits
 
     # ref can't be HEAD or SHA, can only be branch/tag name
@@ -241,8 +241,8 @@ module Ci
       save
     end
 
-    def execute_hooks_unless_ci_skipped
-      execute_hooks unless skip_ci?
+    def execute_hooks_if_status_changed
+      execute_hooks if status_changed? && !skip_ci?
     end
 
     def execute_hooks
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index aa05fc78f94..a8c49daf9bb 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -551,7 +551,7 @@ describe Ci::Pipeline, models: true do
 
     before do
       WebMock.stub_request(:post, hook.url)
-      pipeline.touch
+      pipeline.save
       ProjectWebHookWorker.drain
     end
 
@@ -561,6 +561,26 @@ describe Ci::Pipeline, models: true do
       it 'executes pipeline_hook after touched' do
         expect(WebMock).to have_requested(:post, hook.url).once
       end
+
+      context 'with multiple builds' do
+        def create_build(name)
+          create(:ci_build, :pending, pipeline: pipeline, name: name)
+        end
+
+        let(:build_a) { create_build('a') }
+        let(:build_b) { create_build('b') }
+
+        before do
+          build_a.run
+          build_b.run
+          build_a.success
+          build_b.success
+        end
+
+        it 'fires 3 hooks' do
+          expect(WebMock).to have_requested(:post, hook.url).times(3)
+        end
+      end
     end
 
     context 'with pipeline hooks disabled' do

From 77540619d5ce0e63edec89ea3d406fa457696a0e Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Wed, 3 Aug 2016 23:54:44 +0800
Subject: [PATCH 07/42] Test against the status in the payload

---
 spec/models/ci/pipeline_spec.rb | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index a8c49daf9bb..b0496a017f4 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -567,6 +567,12 @@ describe Ci::Pipeline, models: true do
           create(:ci_build, :pending, pipeline: pipeline, name: name)
         end
 
+        def requested status
+          have_requested(:post, hook.url).with do |req|
+            JSON.parse(req.body)['object_attributes']['status'] == status
+          end.once
+        end
+
         let(:build_a) { create_build('a') }
         let(:build_b) { create_build('b') }
 
@@ -578,7 +584,9 @@ describe Ci::Pipeline, models: true do
         end
 
         it 'fires 3 hooks' do
-          expect(WebMock).to have_requested(:post, hook.url).times(3)
+          %w(pending running success).each do |status|
+            expect(WebMock).to requested(status)
+          end
         end
       end
     end

From 3f7680a61933cd2f710236474b01e9cd9b849beb Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Thu, 4 Aug 2016 00:04:01 +0800
Subject: [PATCH 08/42] I was too used to this style...

---
 spec/models/ci/pipeline_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index b0496a017f4..b20c617f089 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -567,7 +567,7 @@ describe Ci::Pipeline, models: true do
           create(:ci_build, :pending, pipeline: pipeline, name: name)
         end
 
-        def requested status
+        def requested(status)
           have_requested(:post, hook.url).with do |req|
             JSON.parse(req.body)['object_attributes']['status'] == status
           end.once

From 94b3d33de1417b31ef3994e43f901941dc302ca0 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Thu, 4 Aug 2016 00:46:58 +0800
Subject: [PATCH 09/42] If we use Rails magic it's breaking this test:

spec/lib/gitlab/data_builder/pipeline_data_builder_spec.rb

Because it would trigger the event just after saved and it would load
no builds and cache it. We should really avoid adding more magic.
---
 app/models/ci/pipeline.rb       | 10 ++++------
 spec/models/ci/pipeline_spec.rb |  2 +-
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 822ba7b6c00..ca41a998a2b 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -18,7 +18,6 @@ module Ci
 
     # Invalidate object and save if when touched
     after_touch :update_state
-    after_save :execute_hooks_if_status_changed
     after_save :keep_around_commits
 
     # ref can't be HEAD or SHA, can only be branch/tag name
@@ -230,6 +229,7 @@ module Ci
 
     def update_state
       statuses.reload
+      last_status = status
       self.status = if yaml_errors.blank?
                       statuses.latest.status || 'skipped'
                     else
@@ -238,11 +238,9 @@ module Ci
       self.started_at = statuses.started_at
       self.finished_at = statuses.finished_at
       self.duration = statuses.latest.duration
-      save
-    end
-
-    def execute_hooks_if_status_changed
-      execute_hooks if status_changed? && !skip_ci?
+      saved = save
+      execute_hooks if last_status != status && saved && !skip_ci?
+      saved
     end
 
     def execute_hooks
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index b20c617f089..326d3c9b44d 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -551,7 +551,7 @@ describe Ci::Pipeline, models: true do
 
     before do
       WebMock.stub_request(:post, hook.url)
-      pipeline.save
+      pipeline.touch
       ProjectWebHookWorker.drain
     end
 

From 431792f78404c4b83aa8c962ff306f4aacd35f8b Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Thu, 4 Aug 2016 01:05:17 +0800
Subject: [PATCH 10/42] Revert "We don't have to touch it because builds would
 touch pipeline anyway"

This reverts commit 3691a391524911a21d5af1c75cb4cd16a8a6e475.
---
 app/services/ci/create_pipeline_service.rb   |  1 +
 app/services/create_commit_builds_service.rb | 10 ++++++++++
 2 files changed, 11 insertions(+)

diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb
index be91bf0db85..7a8b0683acb 100644
--- a/app/services/ci/create_pipeline_service.rb
+++ b/app/services/ci/create_pipeline_service.rb
@@ -27,6 +27,7 @@ module Ci
       end
 
       pipeline.save!
+      pipeline.touch
 
       unless pipeline.create_builds(current_user)
         pipeline.errors.add(:base, 'No builds for this pipeline.')
diff --git a/app/services/create_commit_builds_service.rb b/app/services/create_commit_builds_service.rb
index 5e77768abe7..0b66b854dea 100644
--- a/app/services/create_commit_builds_service.rb
+++ b/app/services/create_commit_builds_service.rb
@@ -53,7 +53,17 @@ class CreateCommitBuildsService
       return false
     end
 
+    save_pipeline!
+  end
+
+  private
+
+  ##
+  # Create a new pipeline and touch object to calculate status
+  #
+  def save_pipeline!
     @pipeline.save!
+    @pipeline.touch
     @pipeline
   end
 end

From 80671bf75cdac3f50615253b058fa04da6235a4f Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Thu, 4 Aug 2016 01:18:33 +0800
Subject: [PATCH 11/42] Separate the concern for executing hooks and updating
 states

---
 app/models/ci/pipeline.rb | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index ca41a998a2b..81991e8aa60 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -228,8 +228,18 @@ module Ci
     end
 
     def update_state
-      statuses.reload
       last_status = status
+
+      if update_state_from_commit_statuses
+        execute_hooks if last_status != status && !skip_ci?
+        true
+      else
+        false
+      end
+    end
+
+    def update_state_from_commit_statuses
+      statuses.reload
       self.status = if yaml_errors.blank?
                       statuses.latest.status || 'skipped'
                     else
@@ -238,9 +248,7 @@ module Ci
       self.started_at = statuses.started_at
       self.finished_at = statuses.finished_at
       self.duration = statuses.latest.duration
-      saved = save
-      execute_hooks if last_status != status && saved && !skip_ci?
-      saved
+      save
     end
 
     def execute_hooks

From 984367f957c8f8d02fa82b08817e2f2f318c6bff Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Thu, 4 Aug 2016 23:44:27 +0800
Subject: [PATCH 12/42] Move those builders to their own namespace, feedback:

https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5620#note_13540099
---
 app/models/ci/build.rb                        |  4 ++--
 .../project_services/builds_email_service.rb  |  2 +-
 app/models/service.rb                         |  2 +-
 app/services/delete_branch_service.rb         |  2 +-
 app/services/delete_tag_service.rb            |  2 +-
 app/services/git_push_service.rb              |  4 ++--
 app/services/git_tag_push_service.rb          |  4 ++--
 app/services/notes/post_process_service.rb    |  2 +-
 app/services/test_hook_service.rb             |  3 ++-
 .../{ => data_builder}/build_data_builder.rb  |  6 +++--
 .../{ => data_builder}/note_data_builder.rb   |  6 +++--
 .../{ => data_builder}/push_data_builder.rb   |  6 +++--
 .../build_data_builder_spec.rb                |  4 ++--
 .../note_data_builder_spec.rb                 |  4 ++--
 .../push_data_builder_spec.rb                 |  2 +-
 .../project_services/assembla_service_spec.rb |  3 ++-
 .../builds_email_service_spec.rb              |  8 ++++---
 .../project_services/drone_ci_service_spec.rb |  4 +++-
 .../project_services/flowdock_service_spec.rb |  3 ++-
 .../gemnasium_service_spec.rb                 |  3 ++-
 .../project_services/hipchat_service_spec.rb  | 24 +++++++++++++------
 .../project_services/irker_service_spec.rb    |  4 +++-
 .../project_services/jira_service_spec.rb     |  3 ++-
 .../project_services/pushover_service_spec.rb |  4 +++-
 .../project_services/slack_service_spec.rb    |  4 +++-
 spec/models/user_spec.rb                      |  4 +++-
 spec/workers/build_email_worker_spec.rb       |  2 +-
 spec/workers/emails_on_push_worker_spec.rb    |  4 +++-
 28 files changed, 79 insertions(+), 44 deletions(-)
 rename lib/gitlab/{ => data_builder}/build_data_builder.rb (96%)
 rename lib/gitlab/{ => data_builder}/note_data_builder.rb (96%)
 rename lib/gitlab/{ => data_builder}/push_data_builder.rb (97%)
 rename spec/lib/gitlab/{ => data_builder}/build_data_builder_spec.rb (85%)
 rename spec/lib/gitlab/{ => data_builder}/note_data_builder_spec.rb (96%)
 rename spec/lib/gitlab/{ => data_builder}/push_data_builder_spec.rb (97%)

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 08f396210c9..b919846af22 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -349,7 +349,7 @@ module Ci
 
     def execute_hooks
       return unless project
-      build_data = Gitlab::BuildDataBuilder.build(self)
+      build_data = Gitlab::DataBuilder::BuildDataBuilder.build(self)
       project.execute_hooks(build_data.dup, :build_hooks)
       project.execute_services(build_data.dup, :build_hooks)
       project.running_or_pending_build_count(force: true)
@@ -461,7 +461,7 @@ module Ci
 
     def build_attributes_from_config
       return {} unless pipeline.config_processor
-      
+
       pipeline.config_processor.build_attributes(name)
     end
   end
diff --git a/app/models/project_services/builds_email_service.rb b/app/models/project_services/builds_email_service.rb
index 5e166471077..bf8c68244a1 100644
--- a/app/models/project_services/builds_email_service.rb
+++ b/app/models/project_services/builds_email_service.rb
@@ -52,7 +52,7 @@ class BuildsEmailService < Service
 
   def test_data(project = nil, user = nil)
     build = project.builds.last
-    Gitlab::BuildDataBuilder.build(build)
+    Gitlab::DataBuilder::BuildDataBuilder.build(build)
   end
 
   def fields
diff --git a/app/models/service.rb b/app/models/service.rb
index e4cd44f542a..76f588f234d 100644
--- a/app/models/service.rb
+++ b/app/models/service.rb
@@ -80,7 +80,7 @@ class Service < ActiveRecord::Base
   end
 
   def test_data(project, user)
-    Gitlab::PushDataBuilder.build_sample(project, user)
+    Gitlab::DataBuilder::PushDataBuilder.build_sample(project, user)
   end
 
   def event_channel_names
diff --git a/app/services/delete_branch_service.rb b/app/services/delete_branch_service.rb
index 87f066edb6f..33c0fdc3c9d 100644
--- a/app/services/delete_branch_service.rb
+++ b/app/services/delete_branch_service.rb
@@ -39,7 +39,7 @@ class DeleteBranchService < BaseService
   end
 
   def build_push_data(branch)
-    Gitlab::PushDataBuilder
+    Gitlab::DataBuilder::PushDataBuilder
       .build(project, current_user, branch.target.sha, Gitlab::Git::BLANK_SHA, "#{Gitlab::Git::BRANCH_REF_PREFIX}#{branch.name}", [])
   end
 end
diff --git a/app/services/delete_tag_service.rb b/app/services/delete_tag_service.rb
index 32e0eed6b63..41f8006b46c 100644
--- a/app/services/delete_tag_service.rb
+++ b/app/services/delete_tag_service.rb
@@ -33,7 +33,7 @@ class DeleteTagService < BaseService
   end
 
   def build_push_data(tag)
-    Gitlab::PushDataBuilder
+    Gitlab::DataBuilder::PushDataBuilder
       .build(project, current_user, tag.target.sha, Gitlab::Git::BLANK_SHA, "#{Gitlab::Git::TAG_REF_PREFIX}#{tag.name}", [])
   end
 end
diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb
index 3f6a177bf3a..473eb5d902f 100644
--- a/app/services/git_push_service.rb
+++ b/app/services/git_push_service.rb
@@ -138,12 +138,12 @@ class GitPushService < BaseService
   end
 
   def build_push_data
-    @push_data ||= Gitlab::PushDataBuilder.
+    @push_data ||= Gitlab::DataBuilder::PushDataBuilder.
       build(@project, current_user, params[:oldrev], params[:newrev], params[:ref], push_commits)
   end
 
   def build_push_data_system_hook
-    @push_data_system ||= Gitlab::PushDataBuilder.
+    @push_data_system ||= Gitlab::DataBuilder::PushDataBuilder.
       build(@project, current_user, params[:oldrev], params[:newrev], params[:ref], [])
   end
 
diff --git a/app/services/git_tag_push_service.rb b/app/services/git_tag_push_service.rb
index 969530c4fdc..73bbbc36270 100644
--- a/app/services/git_tag_push_service.rb
+++ b/app/services/git_tag_push_service.rb
@@ -34,12 +34,12 @@ class GitTagPushService < BaseService
       end
     end
 
-    Gitlab::PushDataBuilder.
+    Gitlab::DataBuilder::PushDataBuilder.
       build(project, current_user, params[:oldrev], params[:newrev], params[:ref], commits, message)
   end
 
   def build_system_push_data
-    Gitlab::PushDataBuilder.
+    Gitlab::DataBuilder::PushDataBuilder.
       build(project, current_user, params[:oldrev], params[:newrev], params[:ref], [], '')
   end
 end
diff --git a/app/services/notes/post_process_service.rb b/app/services/notes/post_process_service.rb
index 534c48aefff..a9ee7949936 100644
--- a/app/services/notes/post_process_service.rb
+++ b/app/services/notes/post_process_service.rb
@@ -16,7 +16,7 @@ module Notes
     end
 
     def hook_data
-      Gitlab::NoteDataBuilder.build(@note, @note.author)
+      Gitlab::DataBuilder::NoteDataBuilder.build(@note, @note.author)
     end
 
     def execute_note_hooks
diff --git a/app/services/test_hook_service.rb b/app/services/test_hook_service.rb
index e85e58751e7..60b85882092 100644
--- a/app/services/test_hook_service.rb
+++ b/app/services/test_hook_service.rb
@@ -1,6 +1,7 @@
 class TestHookService
   def execute(hook, current_user)
-    data = Gitlab::PushDataBuilder.build_sample(hook.project, current_user)
+    data = Gitlab::DataBuilder::PushDataBuilder.
+      build_sample(hook.project, current_user)
     hook.execute(data, 'push_hooks')
   end
 end
diff --git a/lib/gitlab/build_data_builder.rb b/lib/gitlab/data_builder/build_data_builder.rb
similarity index 96%
rename from lib/gitlab/build_data_builder.rb
rename to lib/gitlab/data_builder/build_data_builder.rb
index 9f45aefda0f..5175645e238 100644
--- a/lib/gitlab/build_data_builder.rb
+++ b/lib/gitlab/data_builder/build_data_builder.rb
@@ -1,6 +1,8 @@
 module Gitlab
-  class BuildDataBuilder
-    class << self
+  module DataBuilder
+    module BuildDataBuilder
+      module_function
+
       def build(build)
         project = build.project
         commit = build.pipeline
diff --git a/lib/gitlab/note_data_builder.rb b/lib/gitlab/data_builder/note_data_builder.rb
similarity index 96%
rename from lib/gitlab/note_data_builder.rb
rename to lib/gitlab/data_builder/note_data_builder.rb
index 8bdc89a7751..12ae1b99f9c 100644
--- a/lib/gitlab/note_data_builder.rb
+++ b/lib/gitlab/data_builder/note_data_builder.rb
@@ -1,6 +1,8 @@
 module Gitlab
-  class NoteDataBuilder
-    class << self
+  module DataBuilder
+    module NoteDataBuilder
+      module_function
+
       # Produce a hash of post-receive data
       #
       # For all notes:
diff --git a/lib/gitlab/push_data_builder.rb b/lib/gitlab/data_builder/push_data_builder.rb
similarity index 97%
rename from lib/gitlab/push_data_builder.rb
rename to lib/gitlab/data_builder/push_data_builder.rb
index c8f12577112..f0cad51dd36 100644
--- a/lib/gitlab/push_data_builder.rb
+++ b/lib/gitlab/data_builder/push_data_builder.rb
@@ -1,6 +1,8 @@
 module Gitlab
-  class PushDataBuilder
-    class << self
+  module DataBuilder
+    module PushDataBuilder
+      module_function
+
       # Produce a hash of post-receive data
       #
       # data = {
diff --git a/spec/lib/gitlab/build_data_builder_spec.rb b/spec/lib/gitlab/data_builder/build_data_builder_spec.rb
similarity index 85%
rename from spec/lib/gitlab/build_data_builder_spec.rb
rename to spec/lib/gitlab/data_builder/build_data_builder_spec.rb
index 23ae5cfacc4..41b9207df2d 100644
--- a/spec/lib/gitlab/build_data_builder_spec.rb
+++ b/spec/lib/gitlab/data_builder/build_data_builder_spec.rb
@@ -1,11 +1,11 @@
 require 'spec_helper'
 
-describe 'Gitlab::BuildDataBuilder' do
+describe Gitlab::DataBuilder::BuildDataBuilder do
   let(:build) { create(:ci_build) }
 
   describe '.build' do
     let(:data) do
-      Gitlab::BuildDataBuilder.build(build)
+      Gitlab::DataBuilder::BuildDataBuilder.build(build)
     end
 
     it { expect(data).to be_a(Hash) }
diff --git a/spec/lib/gitlab/note_data_builder_spec.rb b/spec/lib/gitlab/data_builder/note_data_builder_spec.rb
similarity index 96%
rename from spec/lib/gitlab/note_data_builder_spec.rb
rename to spec/lib/gitlab/data_builder/note_data_builder_spec.rb
index 3d6bcdfd873..bc5d6cdc358 100644
--- a/spec/lib/gitlab/note_data_builder_spec.rb
+++ b/spec/lib/gitlab/data_builder/note_data_builder_spec.rb
@@ -1,9 +1,9 @@
 require 'spec_helper'
 
-describe 'Gitlab::NoteDataBuilder', lib: true do
+describe Gitlab::DataBuilder::NoteDataBuilder, lib: true do
   let(:project) { create(:project) }
   let(:user) { create(:user) }
-  let(:data) { Gitlab::NoteDataBuilder.build(note, user) }
+  let(:data) { Gitlab::DataBuilder::NoteDataBuilder.build(note, user) }
   let(:fixed_time) { Time.at(1425600000) } # Avoid time precision errors
 
   before(:each) do
diff --git a/spec/lib/gitlab/push_data_builder_spec.rb b/spec/lib/gitlab/data_builder/push_data_builder_spec.rb
similarity index 97%
rename from spec/lib/gitlab/push_data_builder_spec.rb
rename to spec/lib/gitlab/data_builder/push_data_builder_spec.rb
index 6bd7393aaa7..beb3e0eda7e 100644
--- a/spec/lib/gitlab/push_data_builder_spec.rb
+++ b/spec/lib/gitlab/data_builder/push_data_builder_spec.rb
@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-describe Gitlab::PushDataBuilder, lib: true do
+describe Gitlab::DataBuilder::PushDataBuilder, lib: true do
   let(:project) { create(:project) }
   let(:user) { create(:user) }
 
diff --git a/spec/models/project_services/assembla_service_spec.rb b/spec/models/project_services/assembla_service_spec.rb
index 17e9361dd5c..63d1c4ce845 100644
--- a/spec/models/project_services/assembla_service_spec.rb
+++ b/spec/models/project_services/assembla_service_spec.rb
@@ -39,7 +39,8 @@ describe AssemblaService, models: true do
         token: 'verySecret',
         subdomain: 'project_name'
       )
-      @sample_data = Gitlab::PushDataBuilder.build_sample(project, user)
+      @sample_data = Gitlab::DataBuilder::PushDataBuilder.
+        build_sample(project, user)
       @api_url = 'https://atlas.assembla.com/spaces/project_name/github_tool?secret_key=verySecret'
       WebMock.stub_request(:post, @api_url)
     end
diff --git a/spec/models/project_services/builds_email_service_spec.rb b/spec/models/project_services/builds_email_service_spec.rb
index ca2cd8aa551..2c8c842babe 100644
--- a/spec/models/project_services/builds_email_service_spec.rb
+++ b/spec/models/project_services/builds_email_service_spec.rb
@@ -1,7 +1,9 @@
 require 'spec_helper'
 
 describe BuildsEmailService do
-  let(:data) { Gitlab::BuildDataBuilder.build(create(:ci_build)) }
+  let(:data) do
+    Gitlab::DataBuilder::BuildDataBuilder.build(create(:ci_build))
+  end
 
   describe 'Validations' do
     context 'when service is active' do
@@ -39,7 +41,7 @@ describe BuildsEmailService do
 
   describe '#test' do
     it 'sends email' do
-      data = Gitlab::BuildDataBuilder.build(create(:ci_build))
+      data = Gitlab::DataBuilder::BuildDataBuilder.build(create(:ci_build))
       subject.recipients = 'test@gitlab.com'
 
       expect(BuildEmailWorker).to receive(:perform_async)
@@ -49,7 +51,7 @@ describe BuildsEmailService do
 
     context 'notify only failed builds is true' do
       it 'sends email' do
-        data = Gitlab::BuildDataBuilder.build(create(:ci_build))
+        data = Gitlab::DataBuilder::BuildDataBuilder.build(create(:ci_build))
         data[:build_status] = "success"
         subject.recipients = 'test@gitlab.com'
 
diff --git a/spec/models/project_services/drone_ci_service_spec.rb b/spec/models/project_services/drone_ci_service_spec.rb
index 3a8e67438fc..eb6955d5048 100644
--- a/spec/models/project_services/drone_ci_service_spec.rb
+++ b/spec/models/project_services/drone_ci_service_spec.rb
@@ -84,7 +84,9 @@ describe DroneCiService, models: true do
     include_context :drone_ci_service
 
     let(:user)    { create(:user, username: 'username') }
-    let(:push_sample_data) { Gitlab::PushDataBuilder.build_sample(project, user) }
+    let(:push_sample_data) do
+      Gitlab::DataBuilder::PushDataBuilder.build_sample(project, user)
+    end
 
     it do
       service_hook = double
diff --git a/spec/models/project_services/flowdock_service_spec.rb b/spec/models/project_services/flowdock_service_spec.rb
index b7e627e6518..a021f82b374 100644
--- a/spec/models/project_services/flowdock_service_spec.rb
+++ b/spec/models/project_services/flowdock_service_spec.rb
@@ -52,7 +52,8 @@ describe FlowdockService, models: true do
         service_hook: true,
         token: 'verySecret'
       )
-      @sample_data = Gitlab::PushDataBuilder.build_sample(project, user)
+      @sample_data = Gitlab::DataBuilder::PushDataBuilder.
+        build_sample(project, user)
       @api_url = 'https://api.flowdock.com/v1/messages'
       WebMock.stub_request(:post, @api_url)
     end
diff --git a/spec/models/project_services/gemnasium_service_spec.rb b/spec/models/project_services/gemnasium_service_spec.rb
index a08f1ac229f..39ce8ee3387 100644
--- a/spec/models/project_services/gemnasium_service_spec.rb
+++ b/spec/models/project_services/gemnasium_service_spec.rb
@@ -55,7 +55,8 @@ describe GemnasiumService, models: true do
         token: 'verySecret',
         api_key: 'GemnasiumUserApiKey'
       )
-      @sample_data = Gitlab::PushDataBuilder.build_sample(project, user)
+      @sample_data = Gitlab::DataBuilder::PushDataBuilder.
+        build_sample(project, user)
     end
     it "should call Gemnasium service" do
       expect(Gemnasium::GitlabService).to receive(:execute).with(an_instance_of(Hash)).once
diff --git a/spec/models/project_services/hipchat_service_spec.rb b/spec/models/project_services/hipchat_service_spec.rb
index 62ae5f6cf74..79d7fb776f9 100644
--- a/spec/models/project_services/hipchat_service_spec.rb
+++ b/spec/models/project_services/hipchat_service_spec.rb
@@ -48,7 +48,9 @@ describe HipchatService, models: true do
     let(:project_name) { project.name_with_namespace.gsub(/\s/, '') }
     let(:token) { 'verySecret' }
     let(:server_url) { 'https://hipchat.example.com'}
-    let(:push_sample_data) { Gitlab::PushDataBuilder.build_sample(project, user) }
+    let(:push_sample_data) do
+      Gitlab::DataBuilder::PushDataBuilder.build_sample(project, user)
+    end
 
     before(:each) do
       allow(hipchat).to receive_messages(
@@ -108,7 +110,15 @@ describe HipchatService, models: true do
     end
 
     context 'tag_push events' do
-      let(:push_sample_data) { Gitlab::PushDataBuilder.build(project, user, Gitlab::Git::BLANK_SHA, '1' * 40, 'refs/tags/test', []) }
+      let(:push_sample_data) do
+        Gitlab::DataBuilder::PushDataBuilder.build(
+          project,
+          user,
+          Gitlab::Git::BLANK_SHA,
+          '1' * 40,
+          'refs/tags/test',
+          [])
+      end
 
       it "should call Hipchat API for tag push events" do
         hipchat.execute(push_sample_data)
@@ -185,7 +195,7 @@ describe HipchatService, models: true do
         end
 
         it "should call Hipchat API for commit comment events" do
-          data = Gitlab::NoteDataBuilder.build(commit_note, user)
+          data = Gitlab::DataBuilder::NoteDataBuilder.build(commit_note, user)
           hipchat.execute(data)
 
           expect(WebMock).to have_requested(:post, api_url).once
@@ -217,7 +227,7 @@ describe HipchatService, models: true do
         end
 
         it "should call Hipchat API for merge request comment events" do
-          data = Gitlab::NoteDataBuilder.build(merge_request_note, user)
+          data = Gitlab::DataBuilder::NoteDataBuilder.build(merge_request_note, user)
           hipchat.execute(data)
 
           expect(WebMock).to have_requested(:post, api_url).once
@@ -244,7 +254,7 @@ describe HipchatService, models: true do
         end
 
         it "should call Hipchat API for issue comment events" do
-          data = Gitlab::NoteDataBuilder.build(issue_note, user)
+          data = Gitlab::DataBuilder::NoteDataBuilder.build(issue_note, user)
           hipchat.execute(data)
 
           message = hipchat.send(:create_message, data)
@@ -270,7 +280,7 @@ describe HipchatService, models: true do
         end
 
         it "should call Hipchat API for snippet comment events" do
-          data = Gitlab::NoteDataBuilder.build(snippet_note, user)
+          data = Gitlab::DataBuilder::NoteDataBuilder.build(snippet_note, user)
           hipchat.execute(data)
 
           expect(WebMock).to have_requested(:post, api_url).once
@@ -292,7 +302,7 @@ describe HipchatService, models: true do
 
     context 'build events' do
       let(:build) { create(:ci_build) }
-      let(:data) { Gitlab::BuildDataBuilder.build(build) }
+      let(:data) { Gitlab::DataBuilder::BuildDataBuilder.build(build) }
 
       context 'for failed' do
         before { build.drop }
diff --git a/spec/models/project_services/irker_service_spec.rb b/spec/models/project_services/irker_service_spec.rb
index 4ee022a5171..d49948e693d 100644
--- a/spec/models/project_services/irker_service_spec.rb
+++ b/spec/models/project_services/irker_service_spec.rb
@@ -46,7 +46,9 @@ describe IrkerService, models: true do
     let(:irker) { IrkerService.new }
     let(:user) { create(:user) }
     let(:project) { create(:project) }
-    let(:sample_data) { Gitlab::PushDataBuilder.build_sample(project, user) }
+    let(:sample_data) do
+      Gitlab::DataBuilder::PushDataBuilder.build_sample(project, user)
+    end
 
     let(:recipients) { '#commits irc://test.net/#test ftp://bad' }
     let(:colorize_messages) { '1' }
diff --git a/spec/models/project_services/jira_service_spec.rb b/spec/models/project_services/jira_service_spec.rb
index 5a97cf370da..13aec5172e9 100644
--- a/spec/models/project_services/jira_service_spec.rb
+++ b/spec/models/project_services/jira_service_spec.rb
@@ -66,7 +66,8 @@ describe JiraService, models: true do
         password: 'gitlab_jira_password'
       )
       @jira_service.save # will build API URL, as api_url was not specified above
-      @sample_data = Gitlab::PushDataBuilder.build_sample(project, user)
+      @sample_data = Gitlab::DataBuilder::PushDataBuilder.
+        build_sample(project, user)
       # https://github.com/bblimke/webmock#request-with-basic-authentication
       @api_url = 'http://gitlab_jira_username:gitlab_jira_password@jira.example.com/rest/api/2/issue/JIRA-123/transitions'
       @comment_url = 'http://gitlab_jira_username:gitlab_jira_password@jira.example.com/rest/api/2/issue/JIRA-123/comment'
diff --git a/spec/models/project_services/pushover_service_spec.rb b/spec/models/project_services/pushover_service_spec.rb
index 555d9757b47..8c0141b4788 100644
--- a/spec/models/project_services/pushover_service_spec.rb
+++ b/spec/models/project_services/pushover_service_spec.rb
@@ -48,7 +48,9 @@ describe PushoverService, models: true do
     let(:pushover) { PushoverService.new }
     let(:user) { create(:user) }
     let(:project) { create(:project) }
-    let(:sample_data) { Gitlab::PushDataBuilder.build_sample(project, user) }
+    let(:sample_data) do
+      Gitlab::DataBuilder::PushDataBuilder.build_sample(project, user)
+    end
 
     let(:api_key) { 'verySecret' }
     let(:user_key) { 'verySecret' }
diff --git a/spec/models/project_services/slack_service_spec.rb b/spec/models/project_services/slack_service_spec.rb
index df511b1bc4c..373ab8bd79e 100644
--- a/spec/models/project_services/slack_service_spec.rb
+++ b/spec/models/project_services/slack_service_spec.rb
@@ -45,7 +45,9 @@ describe SlackService, models: true do
     let(:slack)   { SlackService.new }
     let(:user)    { create(:user) }
     let(:project) { create(:project) }
-    let(:push_sample_data) { Gitlab::PushDataBuilder.build_sample(project, user) }
+    let(:push_sample_data) do
+      Gitlab::DataBuilder::PushDataBuilder.build_sample(project, user)
+    end
     let(:webhook_url) { 'https://hooks.slack.com/services/SVRWFV0VVAR97N/B02R25XN3/ZBqu7xMupaEEICInN685' }
     let(:username) { 'slack_username' }
     let(:channel) { 'slack_channel' }
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 2a5a7fb2fc6..9394e9dc2e2 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -895,7 +895,9 @@ describe User, models: true do
     subject { create(:user) }
     let!(:project1) { create(:project) }
     let!(:project2) { create(:project, forked_from_project: project1) }
-    let!(:push_data) { Gitlab::PushDataBuilder.build_sample(project2, subject) }
+    let!(:push_data) do
+      Gitlab::DataBuilder::PushDataBuilder.build_sample(project2, subject)
+    end
     let!(:push_event) { create(:event, action: Event::PUSHED, project: project2, target: project1, author: subject, data: push_data) }
 
     before do
diff --git a/spec/workers/build_email_worker_spec.rb b/spec/workers/build_email_worker_spec.rb
index 98deae0a588..9317ad60220 100644
--- a/spec/workers/build_email_worker_spec.rb
+++ b/spec/workers/build_email_worker_spec.rb
@@ -5,7 +5,7 @@ describe BuildEmailWorker do
 
   let(:build) { create(:ci_build) }
   let(:user) { create(:user) }
-  let(:data) { Gitlab::BuildDataBuilder.build(build) }
+  let(:data) { Gitlab::DataBuilder::BuildDataBuilder.build(build) }
 
   subject { BuildEmailWorker.new }
 
diff --git a/spec/workers/emails_on_push_worker_spec.rb b/spec/workers/emails_on_push_worker_spec.rb
index 796751efe8d..1ecc594c311 100644
--- a/spec/workers/emails_on_push_worker_spec.rb
+++ b/spec/workers/emails_on_push_worker_spec.rb
@@ -5,7 +5,9 @@ describe EmailsOnPushWorker do
 
   let(:project) { create(:project) }
   let(:user) { create(:user) }
-  let(:data) { Gitlab::PushDataBuilder.build_sample(project, user) }
+  let(:data) do
+    Gitlab::DataBuilder::PushDataBuilder.build_sample(project, user)
+  end
   let(:recipients) { user.email }
   let(:perform) { subject.perform(project.id, recipients, data.stringify_keys) }
 

From c245cb39854658290b9155f4c4041351259d509d Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Fri, 5 Aug 2016 00:26:04 +0800
Subject: [PATCH 13/42] Missed renaming them

---
 spec/models/project_services/slack_service_spec.rb | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/spec/models/project_services/slack_service_spec.rb b/spec/models/project_services/slack_service_spec.rb
index 373ab8bd79e..8b15024de7b 100644
--- a/spec/models/project_services/slack_service_spec.rb
+++ b/spec/models/project_services/slack_service_spec.rb
@@ -197,7 +197,8 @@ describe SlackService, models: true do
         it "uses the right channel" do
           slack.update_attributes(note_channel: "random")
 
-          note_data = Gitlab::NoteDataBuilder.build(issue_note, user)
+          note_data = Gitlab::DataBuilder::NoteDataBuilder.
+            build(issue_note, user)
 
           expect(Slack::Notifier).to receive(:new).
            with(webhook_url, channel: "random").
@@ -237,7 +238,7 @@ describe SlackService, models: true do
       end
 
       it "should call Slack API for commit comment events" do
-        data = Gitlab::NoteDataBuilder.build(commit_note, user)
+        data = Gitlab::DataBuilder::NoteDataBuilder.build(commit_note, user)
         slack.execute(data)
 
         expect(WebMock).to have_requested(:post, webhook_url).once
@@ -251,7 +252,8 @@ describe SlackService, models: true do
       end
 
       it "should call Slack API for merge request comment events" do
-        data = Gitlab::NoteDataBuilder.build(merge_request_note, user)
+        data = Gitlab::DataBuilder::NoteDataBuilder.
+          build(merge_request_note, user)
         slack.execute(data)
 
         expect(WebMock).to have_requested(:post, webhook_url).once
@@ -264,7 +266,7 @@ describe SlackService, models: true do
       end
 
       it "should call Slack API for issue comment events" do
-        data = Gitlab::NoteDataBuilder.build(issue_note, user)
+        data = Gitlab::DataBuilder::NoteDataBuilder.build(issue_note, user)
         slack.execute(data)
 
         expect(WebMock).to have_requested(:post, webhook_url).once
@@ -278,7 +280,7 @@ describe SlackService, models: true do
       end
 
       it "should call Slack API for snippet comment events" do
-        data = Gitlab::NoteDataBuilder.build(snippet_note, user)
+        data = Gitlab::DataBuilder::NoteDataBuilder.build(snippet_note, user)
         slack.execute(data)
 
         expect(WebMock).to have_requested(:post, webhook_url).once

From aa75728853ec03c845adc6ba1ae483023ae8bcd4 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Fri, 5 Aug 2016 00:27:12 +0800
Subject: [PATCH 14/42] Removed the abstract let, feedback:

https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5620#note_13580861
---
 spec/models/ci/pipeline_spec.rb | 1 -
 1 file changed, 1 deletion(-)

diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index 326d3c9b44d..f45684414e6 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -547,7 +547,6 @@ describe Ci::Pipeline, models: true do
     let!(:hook) do
       create(:project_hook, project: project, pipeline_events: enabled)
     end
-    let(:enabled) { raise NotImplementedError }
 
     before do
       WebMock.stub_request(:post, hook.url)

From 3b2c5a85414090d93de33e26912b3ac2d771dfe9 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Fri, 5 Aug 2016 00:31:55 +0800
Subject: [PATCH 15/42] Touch it after builds were created, aligning with:

CreateCommitBuildsService
---
 app/services/ci/create_pipeline_service.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb
index 7a8b0683acb..b3772968ef3 100644
--- a/app/services/ci/create_pipeline_service.rb
+++ b/app/services/ci/create_pipeline_service.rb
@@ -27,13 +27,13 @@ module Ci
       end
 
       pipeline.save!
-      pipeline.touch
 
       unless pipeline.create_builds(current_user)
         pipeline.errors.add(:base, 'No builds for this pipeline.')
       end
 
       pipeline.save
+      pipeline.touch
       pipeline
     end
 

From 584258dbb82f76c627d8552fc96689c7879b36f6 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Fri, 5 Aug 2016 00:43:16 +0800
Subject: [PATCH 16/42] Share nothing so it's safest, feedback:

https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5620#note_13581090
---
 app/models/ci/pipeline.rb | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 81991e8aa60..59ab8b5ce35 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -252,9 +252,12 @@ module Ci
     end
 
     def execute_hooks
-      pipeline_data = Gitlab::DataBuilder::PipelineDataBuilder.build(self)
       project.execute_hooks(pipeline_data, :pipeline_hooks)
-      project.execute_services(pipeline_data.dup, :pipeline_hooks)
+      project.execute_services(pipeline_data, :pipeline_hooks)
+    end
+
+    def pipeline_data
+      Gitlab::DataBuilder::PipelineDataBuilder.build(self)
     end
 
     def keep_around_commits

From a92dd5449524780c2a56e9627059b6c0e2362805 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Fri, 5 Aug 2016 00:44:17 +0800
Subject: [PATCH 17/42] Define utility functions later, feedback:

https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5620#note_13581143
---
 spec/models/ci/pipeline_spec.rb | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index f45684414e6..542264eb1d9 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -562,16 +562,6 @@ describe Ci::Pipeline, models: true do
       end
 
       context 'with multiple builds' do
-        def create_build(name)
-          create(:ci_build, :pending, pipeline: pipeline, name: name)
-        end
-
-        def requested(status)
-          have_requested(:post, hook.url).with do |req|
-            JSON.parse(req.body)['object_attributes']['status'] == status
-          end.once
-        end
-
         let(:build_a) { create_build('a') }
         let(:build_b) { create_build('b') }
 
@@ -587,6 +577,16 @@ describe Ci::Pipeline, models: true do
             expect(WebMock).to requested(status)
           end
         end
+
+        def create_build(name)
+          create(:ci_build, :pending, pipeline: pipeline, name: name)
+        end
+
+        def requested(status)
+          have_requested(:post, hook.url).with do |req|
+            JSON.parse(req.body)['object_attributes']['status'] == status
+          end.once
+        end
       end
     end
 

From 901536b36f3f5d95bd9ba33a2b99ef1c171c1133 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Fri, 5 Aug 2016 00:53:07 +0800
Subject: [PATCH 18/42] No need to check that as in CreateCommitBuildsService:

Feedback:
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5620#note_13581358
---
 app/models/ci/pipeline.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 59ab8b5ce35..d6b75411022 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -231,7 +231,7 @@ module Ci
       last_status = status
 
       if update_state_from_commit_statuses
-        execute_hooks if last_status != status && !skip_ci?
+        execute_hooks if last_status != status
         true
       else
         false

From f88d4523f3e9523494a96c149c28796df8023e8d Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Fri, 5 Aug 2016 13:53:43 +0800
Subject: [PATCH 19/42] We still need to skip loading config_processor if
 skip_ci?

---
 lib/gitlab/data_builder/pipeline_data_builder.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/gitlab/data_builder/pipeline_data_builder.rb b/lib/gitlab/data_builder/pipeline_data_builder.rb
index 13417ba09eb..a4c770b630f 100644
--- a/lib/gitlab/data_builder/pipeline_data_builder.rb
+++ b/lib/gitlab/data_builder/pipeline_data_builder.rb
@@ -16,7 +16,7 @@ module Gitlab
 
       def hook_attrs(pipeline)
         first_pending_build = pipeline.builds.first_pending
-        config_processor = pipeline.config_processor
+        config_processor = pipeline.config_processor unless pipeline.skip_ci?
 
         {
           id: pipeline.id,

From 5607fdf9fe51da75099effbaf8277c98290d6d96 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Fri, 5 Aug 2016 14:37:45 +0800
Subject: [PATCH 20/42] Let's make sure cache were cleared:

I can't reproduce this failure locally.
Here's the failure:
https://gitlab.com/gitlab-org/gitlab-ce/builds/2864250
---
 spec/helpers/notes_helper_spec.rb | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/spec/helpers/notes_helper_spec.rb b/spec/helpers/notes_helper_spec.rb
index af371248ae9..f782c153017 100644
--- a/spec/helpers/notes_helper_spec.rb
+++ b/spec/helpers/notes_helper_spec.rb
@@ -38,6 +38,10 @@ describe NotesHelper do
   end
 
   describe '#preload_max_access_for_authors' do
+    before do
+      RequestStore.clear! # make sure cache were cleared
+    end
+
     it 'loads multiple users' do
       expected_access = {
         owner.id => Gitlab::Access::OWNER,

From ffa75a497a23bf6f87de626fee08ff4538a12587 Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Thu, 11 Aug 2016 17:23:07 +0200
Subject: [PATCH 21/42] Remove stage parameter from send payload

---
 app/models/ci/pipeline.rb                        | 2 ++
 db/schema.rb                                     | 2 ++
 lib/gitlab/data_builder/pipeline_data_builder.rb | 6 +-----
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 9545a6e3ab9..e2663f50dd1 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -19,6 +19,8 @@ module Ci
 
     after_save :keep_around_commits
 
+    delegate :stages, to: :statuses
+
     # ref can't be HEAD or SHA, can only be branch/tag name
     scope :latest_successful_for, ->(ref = default_branch) do
       where(ref: ref).success.order(id: :desc).limit(1)
diff --git a/db/schema.rb b/db/schema.rb
index 6c85e1e9dba..5b901b17265 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -895,6 +895,7 @@ ActiveRecord::Schema.define(version: 20160810142633) do
     t.string   "category",              default: "common", null: false
     t.boolean  "default",               default: false
     t.boolean  "wiki_page_events",      default: true
+    t.boolean  "pipeline_events",       default: false,    null: false
   end
 
   add_index "services", ["project_id"], name: "index_services_on_project_id", using: :btree
@@ -1098,6 +1099,7 @@ ActiveRecord::Schema.define(version: 20160810142633) do
     t.boolean  "build_events",                         default: false,         null: false
     t.boolean  "wiki_page_events",                     default: false,         null: false
     t.string   "token"
+    t.boolean  "pipeline_events",                      default: false,         null: false
   end
 
   add_index "web_hooks", ["project_id"], name: "index_web_hooks_on_project_id", using: :btree
diff --git a/lib/gitlab/data_builder/pipeline_data_builder.rb b/lib/gitlab/data_builder/pipeline_data_builder.rb
index a4c770b630f..3dc4d50fcde 100644
--- a/lib/gitlab/data_builder/pipeline_data_builder.rb
+++ b/lib/gitlab/data_builder/pipeline_data_builder.rb
@@ -15,9 +15,6 @@ module Gitlab
       end
 
       def hook_attrs(pipeline)
-        first_pending_build = pipeline.builds.first_pending
-        config_processor = pipeline.config_processor unless pipeline.skip_ci?
-
         {
           id: pipeline.id,
           ref: pipeline.ref,
@@ -25,8 +22,7 @@ module Gitlab
           sha: pipeline.sha,
           before_sha: pipeline.before_sha,
           status: pipeline.status,
-          stage: first_pending_build.try(:stage),
-          stages: config_processor.try(:stages),
+          stages: pipeline.stages,
           created_at: pipeline.created_at,
           finished_at: pipeline.finished_at,
           duration: pipeline.duration

From 99928aca755f4ccf98a58445a0176b80cd16159c Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Thu, 11 Aug 2016 17:23:35 +0200
Subject: [PATCH 22/42] Enhance a pipeline event tests to analyse number of
 returned builds

---
 .../pipeline_data_builder_spec.rb             |  8 +++-
 spec/models/ci/pipeline_spec.rb               | 39 +++++++++----------
 2 files changed, 25 insertions(+), 22 deletions(-)

diff --git a/spec/lib/gitlab/data_builder/pipeline_data_builder_spec.rb b/spec/lib/gitlab/data_builder/pipeline_data_builder_spec.rb
index 24d39b318c0..8a2f00c4347 100644
--- a/spec/lib/gitlab/data_builder/pipeline_data_builder_spec.rb
+++ b/spec/lib/gitlab/data_builder/pipeline_data_builder_spec.rb
@@ -3,11 +3,15 @@ require 'spec_helper'
 describe Gitlab::DataBuilder::PipelineDataBuilder do
   let(:user) { create(:user) }
   let(:project) { create(:project) }
+
   let(:pipeline) do
     create(:ci_pipeline,
-           project: project, status: 'success',
-           sha: project.commit.sha, ref: project.default_branch)
+           project: project,
+           status: 'success',
+           sha: project.commit.sha,
+           ref: project.default_branch)
   end
+
   let!(:build) { create(:ci_build, pipeline: pipeline) }
 
   describe '.build' do
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index ceae2e60f2f..7da044d4f16 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -2,7 +2,7 @@ require 'spec_helper'
 
 describe Ci::Pipeline, models: true do
   let(:project) { FactoryGirl.create :empty_project }
-  let(:pipeline) { FactoryGirl.create :ci_pipeline, project: project }
+  let(:pipeline) { FactoryGirl.create :ci_empty_pipeline, project: project }
 
   it { is_expected.to belong_to(:project) }
   it { is_expected.to belong_to(:user) }
@@ -18,6 +18,8 @@ describe Ci::Pipeline, models: true do
   it { is_expected.to respond_to :git_author_email }
   it { is_expected.to respond_to :short_sha }
 
+  it { is_expected.to delegate_method(:stages).to(:statuses) }
+
   describe '#valid_commit_sha' do
     context 'commit.sha can not start with 00000000' do
       before do
@@ -261,43 +263,40 @@ describe Ci::Pipeline, models: true do
     end
 
     before do
-      WebMock.stub_request(:post, hook.url)
-      pipeline.touch
       ProjectWebHookWorker.drain
     end
 
     context 'with pipeline hooks enabled' do
       let(:enabled) { true }
 
-      it 'executes pipeline_hook after touched' do
-        expect(WebMock).to have_requested(:post, hook.url).once
-      end
-
       context 'with multiple builds' do
-        let(:build_a) { create_build('a') }
-        let(:build_b) { create_build('b') }
+        let!(:build_a) { create_build('a') }
+        let!(:build_b) { create_build('b') }
 
-        before do
+        it 'fires exactly 3 hooks' do
+          stub_request('pending')
+          build_a.queue
+          build_b.queue
+
+          stub_request('running')
           build_a.run
           build_b.run
+
+          stub_request('success')
           build_a.success
           build_b.success
         end
 
-        it 'fires 3 hooks' do
-          %w(pending running success).each do |status|
-            expect(WebMock).to requested(status)
-          end
-        end
-
         def create_build(name)
           create(:ci_build, :pending, pipeline: pipeline, name: name)
         end
 
-        def requested(status)
-          have_requested(:post, hook.url).with do |req|
-            JSON.parse(req.body)['object_attributes']['status'] == status
-          end.once
+        def stub_request(status)
+          WebMock.stub_request(:post, hook.url).with do |req|
+            json_body = JSON.parse(req.body)
+            json_body['object_attributes']['status'] == status &&
+              json_body['builds'].length == 2
+          end
         end
       end
     end

From 478990bb3ee0aa6939b656763a97d637189f062d Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Thu, 11 Aug 2016 18:37:36 +0200
Subject: [PATCH 23/42] Fix pipeline status change from pending to running

---
 app/models/ci/pipeline.rb       |  3 ++-
 app/models/commit_status.rb     |  6 +++++
 spec/models/ci/pipeline_spec.rb | 45 +++++++++++++++++++++++++++++++++
 3 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index e2663f50dd1..bf8750ca0f6 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -187,6 +187,7 @@ module Ci
 
     def process!
       Ci::ProcessPipelineService.new(project, user).execute(self)
+
       reload_status!
     end
 
@@ -197,7 +198,7 @@ module Ci
     end
 
     def reload_status!
-      statuses.reload
+      reload
       self.status =
         if yaml_errors.blank?
           statuses.latest.status || 'skipped'
diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb
index 20713314a25..3ab44461179 100644
--- a/app/models/commit_status.rb
+++ b/app/models/commit_status.rb
@@ -76,6 +76,12 @@ class CommitStatus < ActiveRecord::Base
 
       commit_status.pipeline.process! if commit_status.pipeline
     end
+
+    around_transition any => [:pending, :running] do |commit_status, block|
+      block.call
+
+      commit_status.pipeline.reload_status! if commit_status.pipeline
+    end
   end
 
   delegate :sha, :short_sha, to: :pipeline
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index 7da044d4f16..317f4147545 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -257,6 +257,51 @@ describe Ci::Pipeline, models: true do
     end
   end
 
+  describe '#status' do
+    let!(:build) { create(:ci_build, :created, pipeline: pipeline, name: 'test') }
+
+    subject { pipeline.reload.status }
+
+    context 'on queuing' do
+      before { build.queue }
+
+      it { is_expected.to eq('pending') }
+    end
+
+    context 'on run' do
+      before do
+        build.queue
+        build.run
+      end
+
+      it { is_expected.to eq('running') }
+    end
+
+    context 'on drop' do
+      before do
+        build.drop
+      end
+
+      it { is_expected.to eq('failed') }
+    end
+
+    context 'on success' do
+      before do
+        build.success
+      end
+
+      it { is_expected.to eq('success') }
+    end
+
+    context 'on cancel' do
+      before do
+        build.cancel
+      end
+
+      it { is_expected.to eq('canceled') }
+    end
+  end
+
   describe '#execute_hooks' do
     let!(:hook) do
       create(:project_hook, project: project, pipeline_events: enabled)

From e2c01f397f2f00138d2b4e618306ee2aa141c97c Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Thu, 11 Aug 2016 18:37:50 +0200
Subject: [PATCH 24/42] Fix tests for pipeline events

---
 app/models/ci/pipeline.rb       |  4 +-
 spec/models/ci/pipeline_spec.rb | 68 ++++++++++++++++++++++++---------
 2 files changed, 52 insertions(+), 20 deletions(-)

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index bf8750ca0f6..f8c0e27a5c3 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -208,8 +208,10 @@ module Ci
       self.started_at = statuses.started_at
       self.finished_at = statuses.finished_at
       self.duration = statuses.latest.duration
+
+      should_execute_hooks = status_changed?
       save
-      execute_hooks if status_changed?
+      execute_hooks if should_execute_hooks
     end
 
     private
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index 317f4147545..685c4178e89 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -2,7 +2,7 @@ require 'spec_helper'
 
 describe Ci::Pipeline, models: true do
   let(:project) { FactoryGirl.create :empty_project }
-  let(:pipeline) { FactoryGirl.create :ci_empty_pipeline, project: project }
+  let(:pipeline) { FactoryGirl.create :ci_empty_pipeline, status: 'created', project: project }
 
   it { is_expected.to belong_to(:project) }
   it { is_expected.to belong_to(:user) }
@@ -303,6 +303,9 @@ describe Ci::Pipeline, models: true do
   end
 
   describe '#execute_hooks' do
+    let!(:build_a) { create_build('a') }
+    let!(:build_b) { create_build('b') }
+
     let!(:hook) do
       create(:project_hook, project: project, pipeline_events: enabled)
     end
@@ -314,30 +317,48 @@ describe Ci::Pipeline, models: true do
     context 'with pipeline hooks enabled' do
       let(:enabled) { true }
 
+      before do
+        WebMock.stub_request(:post, hook.url)
+      end
+
       context 'with multiple builds' do
-        let!(:build_a) { create_build('a') }
-        let!(:build_b) { create_build('b') }
+        context 'when build is queued' do
+          before do
+            build_a.queue
+            build_b.queue
+          end
 
-        it 'fires exactly 3 hooks' do
-          stub_request('pending')
-          build_a.queue
-          build_b.queue
-
-          stub_request('running')
-          build_a.run
-          build_b.run
-
-          stub_request('success')
-          build_a.success
-          build_b.success
+          it 'receive a pending event once' do
+            expect(WebMock).to requested('pending').once
+          end
         end
 
-        def create_build(name)
-          create(:ci_build, :pending, pipeline: pipeline, name: name)
+        context 'when build is run' do
+          before do
+            build_a.queue
+            build_a.run
+            build_b.queue
+            build_b.run
+          end
+
+          it 'receive a running event once' do
+            expect(WebMock).to requested('running').once
+          end
         end
 
-        def stub_request(status)
-          WebMock.stub_request(:post, hook.url).with do |req|
+        context 'when all builds succeed' do
+          before do
+            build_a.success
+            build_b.success
+          end
+
+          it 'receive a success event once' do
+            expect(WebMock).to requested('success').once
+          end
+        end
+
+        def requested(status)
+          have_requested(:post, hook.url).with do |req|
             json_body = JSON.parse(req.body)
             json_body['object_attributes']['status'] == status &&
               json_body['builds'].length == 2
@@ -349,9 +370,18 @@ describe Ci::Pipeline, models: true do
     context 'with pipeline hooks disabled' do
       let(:enabled) { false }
 
+      before do
+        build_a.queue
+        build_b.queue
+      end
+
       it 'did not execute pipeline_hook after touched' do
         expect(WebMock).not_to have_requested(:post, hook.url)
       end
     end
+
+    def create_build(name)
+      create(:ci_build, :created, pipeline: pipeline, name: name)
+    end
   end
 end

From 5f5503fb94abc3703c0ff08ad5b7f6be0bf4b7ac Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Fri, 12 Aug 2016 15:11:16 +0800
Subject: [PATCH 25/42] Make the comment more clear, feedback:

https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5620#note_13810778
---
 spec/helpers/notes_helper_spec.rb | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/spec/helpers/notes_helper_spec.rb b/spec/helpers/notes_helper_spec.rb
index fb0ace73707..853b8b2f7f7 100644
--- a/spec/helpers/notes_helper_spec.rb
+++ b/spec/helpers/notes_helper_spec.rb
@@ -39,7 +39,9 @@ describe NotesHelper do
 
   describe '#preload_max_access_for_authors' do
     before do
-      RequestStore.clear! # make sure cache were cleared
+      # #preload_max_access_for_authors would read cache from RequestStore,
+      # so we should make sure it's clean.
+      RequestStore.clear!
     end
 
     it 'loads multiple users' do

From ce4a669d03d5d1b1982361333cff647c0cd114e9 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Fri, 12 Aug 2016 15:16:14 +0800
Subject: [PATCH 26/42] Prefer described_class, feedback:

https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5620#note_13810811
And other similar places.
---
 spec/lib/gitlab/data_builder/build_data_builder_spec.rb    | 2 +-
 spec/lib/gitlab/data_builder/note_data_builder_spec.rb     | 2 +-
 spec/lib/gitlab/data_builder/pipeline_data_builder_spec.rb | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/spec/lib/gitlab/data_builder/build_data_builder_spec.rb b/spec/lib/gitlab/data_builder/build_data_builder_spec.rb
index 41b9207df2d..359e6e09d10 100644
--- a/spec/lib/gitlab/data_builder/build_data_builder_spec.rb
+++ b/spec/lib/gitlab/data_builder/build_data_builder_spec.rb
@@ -5,7 +5,7 @@ describe Gitlab::DataBuilder::BuildDataBuilder do
 
   describe '.build' do
     let(:data) do
-      Gitlab::DataBuilder::BuildDataBuilder.build(build)
+      described_class.build(build)
     end
 
     it { expect(data).to be_a(Hash) }
diff --git a/spec/lib/gitlab/data_builder/note_data_builder_spec.rb b/spec/lib/gitlab/data_builder/note_data_builder_spec.rb
index bc5d6cdc358..56b2312007d 100644
--- a/spec/lib/gitlab/data_builder/note_data_builder_spec.rb
+++ b/spec/lib/gitlab/data_builder/note_data_builder_spec.rb
@@ -3,7 +3,7 @@ require 'spec_helper'
 describe Gitlab::DataBuilder::NoteDataBuilder, lib: true do
   let(:project) { create(:project) }
   let(:user) { create(:user) }
-  let(:data) { Gitlab::DataBuilder::NoteDataBuilder.build(note, user) }
+  let(:data) { described_class.build(note, user) }
   let(:fixed_time) { Time.at(1425600000) } # Avoid time precision errors
 
   before(:each) do
diff --git a/spec/lib/gitlab/data_builder/pipeline_data_builder_spec.rb b/spec/lib/gitlab/data_builder/pipeline_data_builder_spec.rb
index 8a2f00c4347..a35c53c4054 100644
--- a/spec/lib/gitlab/data_builder/pipeline_data_builder_spec.rb
+++ b/spec/lib/gitlab/data_builder/pipeline_data_builder_spec.rb
@@ -15,7 +15,7 @@ describe Gitlab::DataBuilder::PipelineDataBuilder do
   let!(:build) { create(:ci_build, pipeline: pipeline) }
 
   describe '.build' do
-    let(:data) { Gitlab::DataBuilder::PipelineDataBuilder.build(pipeline) }
+    let(:data) { described_class.build(pipeline) }
     let(:attributes) { data[:object_attributes] }
     let(:build_data) { data[:builds].first }
     let(:project_data) { data[:project] }

From aaf30b4e8180588fbd26c11967ec036081f0a87e Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Fri, 12 Aug 2016 15:19:28 +0800
Subject: [PATCH 27/42] if -> when; when -> `when`; %w() -> %w[]; and fix some
 typos:

Feedback:
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5620#note_13810945
---
 spec/models/build_spec.rb | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb
index 395316a8766..3d66ccf3f28 100644
--- a/spec/models/build_spec.rb
+++ b/spec/models/build_spec.rb
@@ -42,7 +42,7 @@ describe Ci::Build, models: true do
   describe '#ignored?' do
     subject { build.ignored? }
 
-    context 'if build is not allowed to fail' do
+    context 'when build is not allowed to fail' do
       before do
         build.allow_failure = false
       end
@@ -64,7 +64,7 @@ describe Ci::Build, models: true do
       end
     end
 
-    context 'if build is allowed to fail' do
+    context 'when build is allowed to fail' do
       before do
         build.allow_failure = true
       end
@@ -92,7 +92,7 @@ describe Ci::Build, models: true do
 
     it { is_expected.to be_empty }
 
-    context 'if build.trace contains text' do
+    context 'when build.trace contains text' do
       let(:text) { 'example output' }
       before do
         build.trace = text
@@ -102,7 +102,7 @@ describe Ci::Build, models: true do
       it { expect(subject.length).to be >= text.length }
     end
 
-    context 'if build.trace hides token' do
+    context 'when build.trace hides token' do
       let(:token) { 'my_secret_token' }
 
       before do
@@ -284,13 +284,13 @@ describe Ci::Build, models: true do
           stub_ci_pipeline_yaml_file(config)
         end
 
-        context 'if config is not found' do
+        context 'when config is not found' do
           let(:config) { nil }
 
           it { is_expected.to eq(predefined_variables) }
         end
 
-        context 'if config does not have a questioned job' do
+        context 'when config does not have a questioned job' do
           let(:config) do
             YAML.dump({
               test_other: {
@@ -302,7 +302,7 @@ describe Ci::Build, models: true do
           it { is_expected.to eq(predefined_variables) }
         end
 
-        context 'if config has variables' do
+        context 'when config has variables' do
           let(:config) do
             YAML.dump({
               test: {
@@ -394,7 +394,7 @@ describe Ci::Build, models: true do
       it { is_expected.to be_falsey }
     end
 
-    context 'if there are runner' do
+    context 'when there are runners' do
       let(:runner) { create(:ci_runner) }
 
       before do
@@ -424,8 +424,8 @@ describe Ci::Build, models: true do
   describe '#stuck?' do
     subject { build.stuck? }
 
-    %w(pending).each do |state|
-      context "if commit_status.status is #{state}" do
+    %w[pending].each do |state|
+      context "when commit_status.status is #{state}" do
         before do
           build.status = state
         end
@@ -445,8 +445,8 @@ describe Ci::Build, models: true do
       end
     end
 
-    %w(success failed canceled running).each do |state|
-      context "if commit_status.status is #{state}" do
+    %w[success failed canceled running].each do |state|
+      context "when commit_status.status is #{state}" do
         before do
           build.status = state
         end
@@ -768,7 +768,7 @@ describe Ci::Build, models: true do
   describe '#when' do
     subject { build.when }
 
-    context 'if is undefined' do
+    context 'when `when` is undefined' do
       before do
         build.when = nil
       end
@@ -778,13 +778,13 @@ describe Ci::Build, models: true do
           stub_ci_pipeline_yaml_file(config)
         end
 
-        context 'if config is not found' do
+        context 'when config is not found' do
           let(:config) { nil }
 
           it { is_expected.to eq('on_success') }
         end
 
-        context 'if config does not have a questioned job' do
+        context 'when config does not have a questioned job' do
           let(:config) do
             YAML.dump({
                         test_other: {
@@ -796,7 +796,7 @@ describe Ci::Build, models: true do
           it { is_expected.to eq('on_success') }
         end
 
-        context 'if config has when' do
+        context 'when config has `when`' do
           let(:config) do
             YAML.dump({
                         test: {
@@ -882,7 +882,7 @@ describe Ci::Build, models: true do
 
     subject { build.play }
 
-    it 'enques a build' do
+    it 'enqueues a build' do
       is_expected.to be_pending
       is_expected.to eq(build)
     end

From abd3ac4a038a83cce5db2aa93f685c921710abba Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Fri, 12 Aug 2016 15:29:49 +0800
Subject: [PATCH 28/42] Make it more grammatically correct, feedback:

https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5620#note_13811203
---
 spec/models/ci/pipeline_spec.rb | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index 685c4178e89..2266b954aeb 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -329,7 +329,7 @@ describe Ci::Pipeline, models: true do
           end
 
           it 'receive a pending event once' do
-            expect(WebMock).to requested('pending').once
+            expect(WebMock).to have_requested_pipeline_hook('pending').once
           end
         end
 
@@ -342,7 +342,7 @@ describe Ci::Pipeline, models: true do
           end
 
           it 'receive a running event once' do
-            expect(WebMock).to requested('running').once
+            expect(WebMock).to have_requested_pipeline_hook('running').once
           end
         end
 
@@ -353,11 +353,11 @@ describe Ci::Pipeline, models: true do
           end
 
           it 'receive a success event once' do
-            expect(WebMock).to requested('success').once
+            expect(WebMock).to have_requested_pipeline_hook('success').once
           end
         end
 
-        def requested(status)
+        def have_requested_pipeline_hook(status)
           have_requested(:post, hook.url).with do |req|
             json_body = JSON.parse(req.body)
             json_body['object_attributes']['status'] == status &&

From 0a20897bbee538761352595e8f632c747b6d1b35 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Fri, 12 Aug 2016 15:33:28 +0800
Subject: [PATCH 29/42] Prefer extend self over module_function, feedback:

https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5620#note_13672004
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5620#note_13810498
---
 lib/gitlab/data_builder/build_data_builder.rb    | 2 +-
 lib/gitlab/data_builder/note_data_builder.rb     | 2 +-
 lib/gitlab/data_builder/pipeline_data_builder.rb | 2 +-
 lib/gitlab/data_builder/push_data_builder.rb     | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/gitlab/data_builder/build_data_builder.rb b/lib/gitlab/data_builder/build_data_builder.rb
index 5175645e238..1b42769bf5b 100644
--- a/lib/gitlab/data_builder/build_data_builder.rb
+++ b/lib/gitlab/data_builder/build_data_builder.rb
@@ -1,7 +1,7 @@
 module Gitlab
   module DataBuilder
     module BuildDataBuilder
-      module_function
+      extend self
 
       def build(build)
         project = build.project
diff --git a/lib/gitlab/data_builder/note_data_builder.rb b/lib/gitlab/data_builder/note_data_builder.rb
index 12ae1b99f9c..78dc583abc7 100644
--- a/lib/gitlab/data_builder/note_data_builder.rb
+++ b/lib/gitlab/data_builder/note_data_builder.rb
@@ -1,7 +1,7 @@
 module Gitlab
   module DataBuilder
     module NoteDataBuilder
-      module_function
+      extend self
 
       # Produce a hash of post-receive data
       #
diff --git a/lib/gitlab/data_builder/pipeline_data_builder.rb b/lib/gitlab/data_builder/pipeline_data_builder.rb
index 3dc4d50fcde..1cba74c7065 100644
--- a/lib/gitlab/data_builder/pipeline_data_builder.rb
+++ b/lib/gitlab/data_builder/pipeline_data_builder.rb
@@ -1,7 +1,7 @@
 module Gitlab
   module DataBuilder
     module PipelineDataBuilder
-      module_function
+      extend self
 
       def build(pipeline)
         {
diff --git a/lib/gitlab/data_builder/push_data_builder.rb b/lib/gitlab/data_builder/push_data_builder.rb
index f0cad51dd36..f0debe7b19f 100644
--- a/lib/gitlab/data_builder/push_data_builder.rb
+++ b/lib/gitlab/data_builder/push_data_builder.rb
@@ -1,7 +1,7 @@
 module Gitlab
   module DataBuilder
     module PushDataBuilder
-      module_function
+      extend self
 
       # Produce a hash of post-receive data
       #

From d5264e8804bca70e613c418a9d346f5787c6fc7a Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Fri, 12 Aug 2016 16:09:29 +0800
Subject: [PATCH 30/42] Simplify the name for data builder, feedback:

https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5620#note_13671791
---
 app/models/ci/build.rb                        |  2 +-
 app/models/ci/pipeline.rb                     |  2 +-
 .../project_services/builds_email_service.rb  |  3 +--
 app/models/service.rb                         |  2 +-
 app/services/delete_branch_service.rb         |  9 +++++++--
 app/services/delete_tag_service.rb            |  9 +++++++--
 app/services/git_push_service.rb              | 18 +++++++++++++----
 app/services/git_tag_push_service.rb          | 20 +++++++++++++++----
 app/services/notes/post_process_service.rb    |  2 +-
 app/services/test_hook_service.rb             |  3 +--
 .../{build_data_builder.rb => build.rb}       |  2 +-
 .../{note_data_builder.rb => note.rb}         |  2 +-
 .../{pipeline_data_builder.rb => pipeline.rb} |  2 +-
 .../{push_data_builder.rb => push.rb}         |  2 +-
 ...ild_data_builder_spec.rb => build_spec.rb} |  2 +-
 ...note_data_builder_spec.rb => note_spec.rb} |  2 +-
 ..._data_builder_spec.rb => pipeline_spec.rb} |  2 +-
 ...push_data_builder_spec.rb => push_spec.rb} |  2 +-
 .../project_services/assembla_service_spec.rb |  3 +--
 .../builds_email_service_spec.rb              |  6 +++---
 .../project_services/campfire_service_spec.rb |  2 +-
 .../project_services/drone_ci_service_spec.rb |  2 +-
 .../project_services/flowdock_service_spec.rb |  3 +--
 .../gemnasium_service_spec.rb                 |  3 +--
 .../project_services/hipchat_service_spec.rb  | 14 ++++++-------
 .../project_services/irker_service_spec.rb    |  2 +-
 .../project_services/jira_service_spec.rb     |  3 +--
 .../project_services/pushover_service_spec.rb |  2 +-
 .../project_services/slack_service_spec.rb    | 14 ++++++-------
 spec/models/user_spec.rb                      |  2 +-
 spec/workers/build_email_worker_spec.rb       |  2 +-
 spec/workers/emails_on_push_worker_spec.rb    |  4 +---
 32 files changed, 85 insertions(+), 63 deletions(-)
 rename lib/gitlab/data_builder/{build_data_builder.rb => build.rb} (98%)
 rename lib/gitlab/data_builder/{note_data_builder.rb => note.rb} (98%)
 rename lib/gitlab/data_builder/{pipeline_data_builder.rb => pipeline.rb} (98%)
 rename lib/gitlab/data_builder/{push_data_builder.rb => push.rb} (99%)
 rename spec/lib/gitlab/data_builder/{build_data_builder_spec.rb => build_spec.rb} (92%)
 rename spec/lib/gitlab/data_builder/{note_data_builder_spec.rb => note_spec.rb} (98%)
 rename spec/lib/gitlab/data_builder/{pipeline_data_builder_spec.rb => pipeline_spec.rb} (95%)
 rename spec/lib/gitlab/data_builder/{push_data_builder_spec.rb => push_spec.rb} (97%)

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 05b11f3b115..e2b0b996b6f 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -343,7 +343,7 @@ module Ci
 
     def execute_hooks
       return unless project
-      build_data = Gitlab::DataBuilder::BuildDataBuilder.build(self)
+      build_data = Gitlab::DataBuilder::Build.build(self)
       project.execute_hooks(build_data.dup, :build_hooks)
       project.execute_services(build_data.dup, :build_hooks)
       project.running_or_pending_build_count(force: true)
diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index f8c0e27a5c3..2bfe8aa5ddd 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -222,7 +222,7 @@ module Ci
     end
 
     def pipeline_data
-      Gitlab::DataBuilder::PipelineDataBuilder.build(self)
+      Gitlab::DataBuilder::Pipeline.build(self)
     end
 
     def keep_around_commits
diff --git a/app/models/project_services/builds_email_service.rb b/app/models/project_services/builds_email_service.rb
index bf8c68244a1..fa66e5864b8 100644
--- a/app/models/project_services/builds_email_service.rb
+++ b/app/models/project_services/builds_email_service.rb
@@ -51,8 +51,7 @@ class BuildsEmailService < Service
   end
 
   def test_data(project = nil, user = nil)
-    build = project.builds.last
-    Gitlab::DataBuilder::BuildDataBuilder.build(build)
+    Gitlab::DataBuilder::Build.build(project.builds.last)
   end
 
   def fields
diff --git a/app/models/service.rb b/app/models/service.rb
index 76f588f234d..09b4717a523 100644
--- a/app/models/service.rb
+++ b/app/models/service.rb
@@ -80,7 +80,7 @@ class Service < ActiveRecord::Base
   end
 
   def test_data(project, user)
-    Gitlab::DataBuilder::PushDataBuilder.build_sample(project, user)
+    Gitlab::DataBuilder::Push.build_sample(project, user)
   end
 
   def event_channel_names
diff --git a/app/services/delete_branch_service.rb b/app/services/delete_branch_service.rb
index 33c0fdc3c9d..918eddaa53a 100644
--- a/app/services/delete_branch_service.rb
+++ b/app/services/delete_branch_service.rb
@@ -39,7 +39,12 @@ class DeleteBranchService < BaseService
   end
 
   def build_push_data(branch)
-    Gitlab::DataBuilder::PushDataBuilder
-      .build(project, current_user, branch.target.sha, Gitlab::Git::BLANK_SHA, "#{Gitlab::Git::BRANCH_REF_PREFIX}#{branch.name}", [])
+    Gitlab::DataBuilder::Push.build(
+      project,
+      current_user,
+      branch.target.sha,
+      Gitlab::Git::BLANK_SHA,
+      "#{Gitlab::Git::BRANCH_REF_PREFIX}#{branch.name}",
+      [])
   end
 end
diff --git a/app/services/delete_tag_service.rb b/app/services/delete_tag_service.rb
index 41f8006b46c..d0cb151a010 100644
--- a/app/services/delete_tag_service.rb
+++ b/app/services/delete_tag_service.rb
@@ -33,7 +33,12 @@ class DeleteTagService < BaseService
   end
 
   def build_push_data(tag)
-    Gitlab::DataBuilder::PushDataBuilder
-      .build(project, current_user, tag.target.sha, Gitlab::Git::BLANK_SHA, "#{Gitlab::Git::TAG_REF_PREFIX}#{tag.name}", [])
+    Gitlab::DataBuilder::Push.build(
+      project,
+      current_user,
+      tag.target.sha,
+      Gitlab::Git::BLANK_SHA,
+      "#{Gitlab::Git::TAG_REF_PREFIX}#{tag.name}",
+      [])
   end
 end
diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb
index b7c5cfb58b4..e3f25ff1597 100644
--- a/app/services/git_push_service.rb
+++ b/app/services/git_push_service.rb
@@ -138,13 +138,23 @@ class GitPushService < BaseService
   end
 
   def build_push_data
-    @push_data ||= Gitlab::DataBuilder::PushDataBuilder.
-      build(@project, current_user, params[:oldrev], params[:newrev], params[:ref], push_commits)
+    @push_data ||= Gitlab::DataBuilder::Push.build(
+      @project,
+      current_user,
+      params[:oldrev],
+      params[:newrev],
+      params[:ref],
+      push_commits)
   end
 
   def build_push_data_system_hook
-    @push_data_system ||= Gitlab::DataBuilder::PushDataBuilder.
-      build(@project, current_user, params[:oldrev], params[:newrev], params[:ref], [])
+    @push_data_system ||= Gitlab::DataBuilder::Push.build(
+      @project,
+      current_user,
+      params[:oldrev],
+      params[:newrev],
+      params[:ref],
+      [])
   end
 
   def push_to_existing_branch?
diff --git a/app/services/git_tag_push_service.rb b/app/services/git_tag_push_service.rb
index a578aaaa3b1..e6002b03b93 100644
--- a/app/services/git_tag_push_service.rb
+++ b/app/services/git_tag_push_service.rb
@@ -34,12 +34,24 @@ class GitTagPushService < BaseService
       end
     end
 
-    Gitlab::DataBuilder::PushDataBuilder.
-      build(project, current_user, params[:oldrev], params[:newrev], params[:ref], commits, message)
+    Gitlab::DataBuilder::Push.build(
+      project,
+      current_user,
+      params[:oldrev],
+      params[:newrev],
+      params[:ref],
+      commits,
+      message)
   end
 
   def build_system_push_data
-    Gitlab::DataBuilder::PushDataBuilder.
-      build(project, current_user, params[:oldrev], params[:newrev], params[:ref], [], '')
+    Gitlab::DataBuilder::Push.build(
+      project,
+      current_user,
+      params[:oldrev],
+      params[:newrev],
+      params[:ref],
+      [],
+      '')
   end
 end
diff --git a/app/services/notes/post_process_service.rb b/app/services/notes/post_process_service.rb
index a9ee7949936..e4cd3fc7833 100644
--- a/app/services/notes/post_process_service.rb
+++ b/app/services/notes/post_process_service.rb
@@ -16,7 +16,7 @@ module Notes
     end
 
     def hook_data
-      Gitlab::DataBuilder::NoteDataBuilder.build(@note, @note.author)
+      Gitlab::DataBuilder::Note.build(@note, @note.author)
     end
 
     def execute_note_hooks
diff --git a/app/services/test_hook_service.rb b/app/services/test_hook_service.rb
index 60b85882092..280c81f7d2d 100644
--- a/app/services/test_hook_service.rb
+++ b/app/services/test_hook_service.rb
@@ -1,7 +1,6 @@
 class TestHookService
   def execute(hook, current_user)
-    data = Gitlab::DataBuilder::PushDataBuilder.
-      build_sample(hook.project, current_user)
+    data = Gitlab::DataBuilder::Push.build_sample(hook.project, current_user)
     hook.execute(data, 'push_hooks')
   end
 end
diff --git a/lib/gitlab/data_builder/build_data_builder.rb b/lib/gitlab/data_builder/build.rb
similarity index 98%
rename from lib/gitlab/data_builder/build_data_builder.rb
rename to lib/gitlab/data_builder/build.rb
index 1b42769bf5b..6548e6475c6 100644
--- a/lib/gitlab/data_builder/build_data_builder.rb
+++ b/lib/gitlab/data_builder/build.rb
@@ -1,6 +1,6 @@
 module Gitlab
   module DataBuilder
-    module BuildDataBuilder
+    module Build
       extend self
 
       def build(build)
diff --git a/lib/gitlab/data_builder/note_data_builder.rb b/lib/gitlab/data_builder/note.rb
similarity index 98%
rename from lib/gitlab/data_builder/note_data_builder.rb
rename to lib/gitlab/data_builder/note.rb
index 78dc583abc7..50fea1232af 100644
--- a/lib/gitlab/data_builder/note_data_builder.rb
+++ b/lib/gitlab/data_builder/note.rb
@@ -1,6 +1,6 @@
 module Gitlab
   module DataBuilder
-    module NoteDataBuilder
+    module Note
       extend self
 
       # Produce a hash of post-receive data
diff --git a/lib/gitlab/data_builder/pipeline_data_builder.rb b/lib/gitlab/data_builder/pipeline.rb
similarity index 98%
rename from lib/gitlab/data_builder/pipeline_data_builder.rb
rename to lib/gitlab/data_builder/pipeline.rb
index 1cba74c7065..06a783ebc1c 100644
--- a/lib/gitlab/data_builder/pipeline_data_builder.rb
+++ b/lib/gitlab/data_builder/pipeline.rb
@@ -1,6 +1,6 @@
 module Gitlab
   module DataBuilder
-    module PipelineDataBuilder
+    module Pipeline
       extend self
 
       def build(pipeline)
diff --git a/lib/gitlab/data_builder/push_data_builder.rb b/lib/gitlab/data_builder/push.rb
similarity index 99%
rename from lib/gitlab/data_builder/push_data_builder.rb
rename to lib/gitlab/data_builder/push.rb
index f0debe7b19f..4f81863da35 100644
--- a/lib/gitlab/data_builder/push_data_builder.rb
+++ b/lib/gitlab/data_builder/push.rb
@@ -1,6 +1,6 @@
 module Gitlab
   module DataBuilder
-    module PushDataBuilder
+    module Push
       extend self
 
       # Produce a hash of post-receive data
diff --git a/spec/lib/gitlab/data_builder/build_data_builder_spec.rb b/spec/lib/gitlab/data_builder/build_spec.rb
similarity index 92%
rename from spec/lib/gitlab/data_builder/build_data_builder_spec.rb
rename to spec/lib/gitlab/data_builder/build_spec.rb
index 359e6e09d10..6c71e98066b 100644
--- a/spec/lib/gitlab/data_builder/build_data_builder_spec.rb
+++ b/spec/lib/gitlab/data_builder/build_spec.rb
@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-describe Gitlab::DataBuilder::BuildDataBuilder do
+describe Gitlab::DataBuilder::Build do
   let(:build) { create(:ci_build) }
 
   describe '.build' do
diff --git a/spec/lib/gitlab/data_builder/note_data_builder_spec.rb b/spec/lib/gitlab/data_builder/note_spec.rb
similarity index 98%
rename from spec/lib/gitlab/data_builder/note_data_builder_spec.rb
rename to spec/lib/gitlab/data_builder/note_spec.rb
index 56b2312007d..9a4dec91e56 100644
--- a/spec/lib/gitlab/data_builder/note_data_builder_spec.rb
+++ b/spec/lib/gitlab/data_builder/note_spec.rb
@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-describe Gitlab::DataBuilder::NoteDataBuilder, lib: true do
+describe Gitlab::DataBuilder::Note, lib: true do
   let(:project) { create(:project) }
   let(:user) { create(:user) }
   let(:data) { described_class.build(note, user) }
diff --git a/spec/lib/gitlab/data_builder/pipeline_data_builder_spec.rb b/spec/lib/gitlab/data_builder/pipeline_spec.rb
similarity index 95%
rename from spec/lib/gitlab/data_builder/pipeline_data_builder_spec.rb
rename to spec/lib/gitlab/data_builder/pipeline_spec.rb
index a35c53c4054..a68f5943a6a 100644
--- a/spec/lib/gitlab/data_builder/pipeline_data_builder_spec.rb
+++ b/spec/lib/gitlab/data_builder/pipeline_spec.rb
@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-describe Gitlab::DataBuilder::PipelineDataBuilder do
+describe Gitlab::DataBuilder::Pipeline do
   let(:user) { create(:user) }
   let(:project) { create(:project) }
 
diff --git a/spec/lib/gitlab/data_builder/push_data_builder_spec.rb b/spec/lib/gitlab/data_builder/push_spec.rb
similarity index 97%
rename from spec/lib/gitlab/data_builder/push_data_builder_spec.rb
rename to spec/lib/gitlab/data_builder/push_spec.rb
index beb3e0eda7e..b73434e8dd7 100644
--- a/spec/lib/gitlab/data_builder/push_data_builder_spec.rb
+++ b/spec/lib/gitlab/data_builder/push_spec.rb
@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-describe Gitlab::DataBuilder::PushDataBuilder, lib: true do
+describe Gitlab::DataBuilder::Push, lib: true do
   let(:project) { create(:project) }
   let(:user) { create(:user) }
 
diff --git a/spec/models/project_services/assembla_service_spec.rb b/spec/models/project_services/assembla_service_spec.rb
index d20decc13a7..d672d80156c 100644
--- a/spec/models/project_services/assembla_service_spec.rb
+++ b/spec/models/project_services/assembla_service_spec.rb
@@ -39,8 +39,7 @@ describe AssemblaService, models: true do
         token: 'verySecret',
         subdomain: 'project_name'
       )
-      @sample_data = Gitlab::DataBuilder::PushDataBuilder.
-        build_sample(project, user)
+      @sample_data = Gitlab::DataBuilder::Push.build_sample(project, user)
       @api_url = 'https://atlas.assembla.com/spaces/project_name/github_tool?secret_key=verySecret'
       WebMock.stub_request(:post, @api_url)
     end
diff --git a/spec/models/project_services/builds_email_service_spec.rb b/spec/models/project_services/builds_email_service_spec.rb
index 2c8c842babe..0194f9e2563 100644
--- a/spec/models/project_services/builds_email_service_spec.rb
+++ b/spec/models/project_services/builds_email_service_spec.rb
@@ -2,7 +2,7 @@ require 'spec_helper'
 
 describe BuildsEmailService do
   let(:data) do
-    Gitlab::DataBuilder::BuildDataBuilder.build(create(:ci_build))
+    Gitlab::DataBuilder::Build.build(create(:ci_build))
   end
 
   describe 'Validations' do
@@ -41,7 +41,7 @@ describe BuildsEmailService do
 
   describe '#test' do
     it 'sends email' do
-      data = Gitlab::DataBuilder::BuildDataBuilder.build(create(:ci_build))
+      data = Gitlab::DataBuilder::Build.build(create(:ci_build))
       subject.recipients = 'test@gitlab.com'
 
       expect(BuildEmailWorker).to receive(:perform_async)
@@ -51,7 +51,7 @@ describe BuildsEmailService do
 
     context 'notify only failed builds is true' do
       it 'sends email' do
-        data = Gitlab::DataBuilder::BuildDataBuilder.build(create(:ci_build))
+        data = Gitlab::DataBuilder::Build.build(create(:ci_build))
         data[:build_status] = "success"
         subject.recipients = 'test@gitlab.com'
 
diff --git a/spec/models/project_services/campfire_service_spec.rb b/spec/models/project_services/campfire_service_spec.rb
index 1adf93258f3..c76ae21421b 100644
--- a/spec/models/project_services/campfire_service_spec.rb
+++ b/spec/models/project_services/campfire_service_spec.rb
@@ -54,7 +54,7 @@ describe CampfireService, models: true do
         subdomain: 'project-name',
         room: 'test-room'
       )
-      @sample_data = Gitlab::PushDataBuilder.build_sample(project, user)
+      @sample_data = Gitlab::DataBuilder::Push.build_sample(project, user)
       @rooms_url = 'https://verySecret:X@project-name.campfirenow.com/rooms.json'
       @headers = { 'Content-Type' => 'application/json; charset=utf-8' }
     end
diff --git a/spec/models/project_services/drone_ci_service_spec.rb b/spec/models/project_services/drone_ci_service_spec.rb
index eb6955d5048..8ef892259f2 100644
--- a/spec/models/project_services/drone_ci_service_spec.rb
+++ b/spec/models/project_services/drone_ci_service_spec.rb
@@ -85,7 +85,7 @@ describe DroneCiService, models: true do
 
     let(:user)    { create(:user, username: 'username') }
     let(:push_sample_data) do
-      Gitlab::DataBuilder::PushDataBuilder.build_sample(project, user)
+      Gitlab::DataBuilder::Push.build_sample(project, user)
     end
 
     it do
diff --git a/spec/models/project_services/flowdock_service_spec.rb b/spec/models/project_services/flowdock_service_spec.rb
index 1f00562fa05..d2557019756 100644
--- a/spec/models/project_services/flowdock_service_spec.rb
+++ b/spec/models/project_services/flowdock_service_spec.rb
@@ -52,8 +52,7 @@ describe FlowdockService, models: true do
         service_hook: true,
         token: 'verySecret'
       )
-      @sample_data = Gitlab::DataBuilder::PushDataBuilder.
-        build_sample(project, user)
+      @sample_data = Gitlab::DataBuilder::Push.build_sample(project, user)
       @api_url = 'https://api.flowdock.com/v1/messages'
       WebMock.stub_request(:post, @api_url)
     end
diff --git a/spec/models/project_services/gemnasium_service_spec.rb b/spec/models/project_services/gemnasium_service_spec.rb
index 5940e44e57d..3d0b6c9816b 100644
--- a/spec/models/project_services/gemnasium_service_spec.rb
+++ b/spec/models/project_services/gemnasium_service_spec.rb
@@ -55,8 +55,7 @@ describe GemnasiumService, models: true do
         token: 'verySecret',
         api_key: 'GemnasiumUserApiKey'
       )
-      @sample_data = Gitlab::DataBuilder::PushDataBuilder.
-        build_sample(project, user)
+      @sample_data = Gitlab::DataBuilder::Push.build_sample(project, user)
     end
     it "calls Gemnasium service" do
       expect(Gemnasium::GitlabService).to receive(:execute).with(an_instance_of(Hash)).once
diff --git a/spec/models/project_services/hipchat_service_spec.rb b/spec/models/project_services/hipchat_service_spec.rb
index bb6c75d6050..34eafbe555d 100644
--- a/spec/models/project_services/hipchat_service_spec.rb
+++ b/spec/models/project_services/hipchat_service_spec.rb
@@ -49,7 +49,7 @@ describe HipchatService, models: true do
     let(:token) { 'verySecret' }
     let(:server_url) { 'https://hipchat.example.com'}
     let(:push_sample_data) do
-      Gitlab::DataBuilder::PushDataBuilder.build_sample(project, user)
+      Gitlab::DataBuilder::Push.build_sample(project, user)
     end
 
     before(:each) do
@@ -111,7 +111,7 @@ describe HipchatService, models: true do
 
     context 'tag_push events' do
       let(:push_sample_data) do
-        Gitlab::DataBuilder::PushDataBuilder.build(
+        Gitlab::DataBuilder::Push.build(
           project,
           user,
           Gitlab::Git::BLANK_SHA,
@@ -195,7 +195,7 @@ describe HipchatService, models: true do
         end
 
         it "calls Hipchat API for commit comment events" do
-          data = Gitlab::DataBuilder::NoteDataBuilder.build(commit_note, user)
+          data = Gitlab::DataBuilder::Note.build(commit_note, user)
           hipchat.execute(data)
 
           expect(WebMock).to have_requested(:post, api_url).once
@@ -227,7 +227,7 @@ describe HipchatService, models: true do
         end
 
         it "calls Hipchat API for merge request comment events" do
-          data = Gitlab::DataBuilder::NoteDataBuilder.build(merge_request_note, user)
+          data = Gitlab::DataBuilder::Note.build(merge_request_note, user)
           hipchat.execute(data)
 
           expect(WebMock).to have_requested(:post, api_url).once
@@ -254,7 +254,7 @@ describe HipchatService, models: true do
         end
 
         it "calls Hipchat API for issue comment events" do
-          data = Gitlab::DataBuilder::NoteDataBuilder.build(issue_note, user)
+          data = Gitlab::DataBuilder::Note.build(issue_note, user)
           hipchat.execute(data)
 
           message = hipchat.send(:create_message, data)
@@ -280,7 +280,7 @@ describe HipchatService, models: true do
         end
 
         it "calls Hipchat API for snippet comment events" do
-          data = Gitlab::DataBuilder::NoteDataBuilder.build(snippet_note, user)
+          data = Gitlab::DataBuilder::Note.build(snippet_note, user)
           hipchat.execute(data)
 
           expect(WebMock).to have_requested(:post, api_url).once
@@ -303,7 +303,7 @@ describe HipchatService, models: true do
     context 'build events' do
       let(:pipeline) { create(:ci_empty_pipeline) }
       let(:build) { create(:ci_build, pipeline: pipeline) }
-      let(:data) { Gitlab::DataBuilder::BuildDataBuilder.build(build) }
+      let(:data) { Gitlab::DataBuilder::Build.build(build) }
 
       context 'for failed' do
         before { build.drop }
diff --git a/spec/models/project_services/irker_service_spec.rb b/spec/models/project_services/irker_service_spec.rb
index 646f0d54424..3bed0e6093f 100644
--- a/spec/models/project_services/irker_service_spec.rb
+++ b/spec/models/project_services/irker_service_spec.rb
@@ -47,7 +47,7 @@ describe IrkerService, models: true do
     let(:user) { create(:user) }
     let(:project) { create(:project) }
     let(:sample_data) do
-      Gitlab::DataBuilder::PushDataBuilder.build_sample(project, user)
+      Gitlab::DataBuilder::Push.build_sample(project, user)
     end
 
     let(:recipients) { '#commits irc://test.net/#test ftp://bad' }
diff --git a/spec/models/project_services/jira_service_spec.rb b/spec/models/project_services/jira_service_spec.rb
index 88800bb61d4..9037ca5cc20 100644
--- a/spec/models/project_services/jira_service_spec.rb
+++ b/spec/models/project_services/jira_service_spec.rb
@@ -66,8 +66,7 @@ describe JiraService, models: true do
         password: 'gitlab_jira_password'
       )
       @jira_service.save # will build API URL, as api_url was not specified above
-      @sample_data = Gitlab::DataBuilder::PushDataBuilder.
-        build_sample(project, user)
+      @sample_data = Gitlab::DataBuilder::Push.build_sample(project, user)
       # https://github.com/bblimke/webmock#request-with-basic-authentication
       @api_url = 'http://gitlab_jira_username:gitlab_jira_password@jira.example.com/rest/api/2/issue/JIRA-123/transitions'
       @comment_url = 'http://gitlab_jira_username:gitlab_jira_password@jira.example.com/rest/api/2/issue/JIRA-123/comment'
diff --git a/spec/models/project_services/pushover_service_spec.rb b/spec/models/project_services/pushover_service_spec.rb
index f1a245ead2c..5959c81577d 100644
--- a/spec/models/project_services/pushover_service_spec.rb
+++ b/spec/models/project_services/pushover_service_spec.rb
@@ -49,7 +49,7 @@ describe PushoverService, models: true do
     let(:user) { create(:user) }
     let(:project) { create(:project) }
     let(:sample_data) do
-      Gitlab::DataBuilder::PushDataBuilder.build_sample(project, user)
+      Gitlab::DataBuilder::Push.build_sample(project, user)
     end
 
     let(:api_key) { 'verySecret' }
diff --git a/spec/models/project_services/slack_service_spec.rb b/spec/models/project_services/slack_service_spec.rb
index 75f07397fb1..28af68d13b4 100644
--- a/spec/models/project_services/slack_service_spec.rb
+++ b/spec/models/project_services/slack_service_spec.rb
@@ -46,7 +46,7 @@ describe SlackService, models: true do
     let(:user)    { create(:user) }
     let(:project) { create(:project) }
     let(:push_sample_data) do
-      Gitlab::DataBuilder::PushDataBuilder.build_sample(project, user)
+      Gitlab::DataBuilder::Push.build_sample(project, user)
     end
     let(:webhook_url) { 'https://hooks.slack.com/services/SVRWFV0VVAR97N/B02R25XN3/ZBqu7xMupaEEICInN685' }
     let(:username) { 'slack_username' }
@@ -197,8 +197,7 @@ describe SlackService, models: true do
         it "uses the right channel" do
           slack.update_attributes(note_channel: "random")
 
-          note_data = Gitlab::DataBuilder::NoteDataBuilder.
-            build(issue_note, user)
+          note_data = Gitlab::DataBuilder::Note.build(issue_note, user)
 
           expect(Slack::Notifier).to receive(:new).
            with(webhook_url, channel: "random").
@@ -238,7 +237,7 @@ describe SlackService, models: true do
       end
 
       it "calls Slack API for commit comment events" do
-        data = Gitlab::DataBuilder::NoteDataBuilder.build(commit_note, user)
+        data = Gitlab::DataBuilder::Note.build(commit_note, user)
         slack.execute(data)
 
         expect(WebMock).to have_requested(:post, webhook_url).once
@@ -252,8 +251,7 @@ describe SlackService, models: true do
       end
 
       it "calls Slack API for merge request comment events" do
-        data = Gitlab::DataBuilder::NoteDataBuilder.
-          build(merge_request_note, user)
+        data = Gitlab::DataBuilder::Note.build(merge_request_note, user)
         slack.execute(data)
 
         expect(WebMock).to have_requested(:post, webhook_url).once
@@ -266,7 +264,7 @@ describe SlackService, models: true do
       end
 
       it "calls Slack API for issue comment events" do
-        data = Gitlab::DataBuilder::NoteDataBuilder.build(issue_note, user)
+        data = Gitlab::DataBuilder::Note.build(issue_note, user)
         slack.execute(data)
 
         expect(WebMock).to have_requested(:post, webhook_url).once
@@ -280,7 +278,7 @@ describe SlackService, models: true do
       end
 
       it "calls Slack API for snippet comment events" do
-        data = Gitlab::DataBuilder::NoteDataBuilder.build(snippet_note, user)
+        data = Gitlab::DataBuilder::Note.build(snippet_note, user)
         slack.execute(data)
 
         expect(WebMock).to have_requested(:post, webhook_url).once
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index bd86415050c..54505f6b822 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -896,7 +896,7 @@ describe User, models: true do
     let!(:project1) { create(:project) }
     let!(:project2) { create(:project, forked_from_project: project1) }
     let!(:push_data) do
-      Gitlab::DataBuilder::PushDataBuilder.build_sample(project2, subject)
+      Gitlab::DataBuilder::Push.build_sample(project2, subject)
     end
     let!(:push_event) { create(:event, action: Event::PUSHED, project: project2, target: project1, author: subject, data: push_data) }
 
diff --git a/spec/workers/build_email_worker_spec.rb b/spec/workers/build_email_worker_spec.rb
index 9317ad60220..788b92c1b84 100644
--- a/spec/workers/build_email_worker_spec.rb
+++ b/spec/workers/build_email_worker_spec.rb
@@ -5,7 +5,7 @@ describe BuildEmailWorker do
 
   let(:build) { create(:ci_build) }
   let(:user) { create(:user) }
-  let(:data) { Gitlab::DataBuilder::BuildDataBuilder.build(build) }
+  let(:data) { Gitlab::DataBuilder::Build.build(build) }
 
   subject { BuildEmailWorker.new }
 
diff --git a/spec/workers/emails_on_push_worker_spec.rb b/spec/workers/emails_on_push_worker_spec.rb
index 1ecc594c311..eecc32875a5 100644
--- a/spec/workers/emails_on_push_worker_spec.rb
+++ b/spec/workers/emails_on_push_worker_spec.rb
@@ -5,9 +5,7 @@ describe EmailsOnPushWorker do
 
   let(:project) { create(:project) }
   let(:user) { create(:user) }
-  let(:data) do
-    Gitlab::DataBuilder::PushDataBuilder.build_sample(project, user)
-  end
+  let(:data) { Gitlab::DataBuilder::Push.build_sample(project, user) }
   let(:recipients) { user.email }
   let(:perform) { subject.perform(project.id, recipients, data.stringify_keys) }
 

From 9fdcbcb0de414967618cfd7f26141e85805fcb54 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Fri, 12 Aug 2016 16:48:47 +0800
Subject: [PATCH 31/42] Have trait all_events_enabled so that's easier to
 reuse, feedback:

https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5620#note_13823349
---
 spec/factories/project_hooks.rb         | 12 ++++++++++++
 spec/requests/api/project_hooks_spec.rb |  7 +++----
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/spec/factories/project_hooks.rb b/spec/factories/project_hooks.rb
index 3195fb3ddcc..c709432c865 100644
--- a/spec/factories/project_hooks.rb
+++ b/spec/factories/project_hooks.rb
@@ -5,5 +5,17 @@ FactoryGirl.define do
     trait :token do
       token { SecureRandom.hex(10) }
     end
+
+    trait :all_events_enabled do
+      %w[push_events
+         merge_requests_events
+         tag_push_events
+         issues_events
+         note_events
+         build_events
+         pipeline_events].each do |event|
+        send(event, true)
+      end
+    end
   end
 end
diff --git a/spec/requests/api/project_hooks_spec.rb b/spec/requests/api/project_hooks_spec.rb
index 92c38e69283..914e88c9487 100644
--- a/spec/requests/api/project_hooks_spec.rb
+++ b/spec/requests/api/project_hooks_spec.rb
@@ -7,10 +7,9 @@ describe API::API, 'ProjectHooks', api: true do
   let!(:project) { create(:project, creator_id: user.id, namespace: user.namespace) }
   let!(:hook) do
     create(:project_hook,
-           project: project, url: "http://example.com",
-           push_events: true, merge_requests_events: true,
-           tag_push_events: true, issues_events: true, note_events: true,
-           build_events: true, pipeline_events: true,
+           :all_events_enabled,
+           project: project,
+           url: 'http://example.com',
            enable_ssl_verification: true)
   end
 

From 2c06cf98a6dc982caf81c2e4faba195ece9a3b77 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Fri, 12 Aug 2016 17:14:11 +0800
Subject: [PATCH 32/42] Fix tests. We cannot reload unless it's already saved:

Not sure if this is the right fix...
Or maybe we should actually merge:
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5782
---
 app/services/ci/create_pipeline_service.rb | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb
index 7398fd8e10a..dabf94fe4c4 100644
--- a/app/services/ci/create_pipeline_service.rb
+++ b/app/services/ci/create_pipeline_service.rb
@@ -93,7 +93,10 @@ module Ci
 
     def error(message, save: false)
       pipeline.errors.add(:base, message)
-      pipeline.reload_status! if save
+      if save
+        pipeline.save
+        pipeline.reload_status!
+      end
       pipeline
     end
   end

From 706d872eb2ebb462b5c226890120f51cf15ba7c5 Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Fri, 12 Aug 2016 12:06:37 +0200
Subject: [PATCH 33/42] Make `execute_methods` public

---
 app/models/ci/pipeline.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 40615097804..ad836bbebb8 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -255,13 +255,13 @@ module Ci
       self.duration = statuses.latest.duration
     end
 
-    private
-
     def execute_hooks
       project.execute_hooks(pipeline_data, :pipeline_hooks)
       project.execute_services(pipeline_data, :pipeline_hooks)
     end
 
+    private
+
     def pipeline_data
       Gitlab::DataBuilder::Pipeline.build(self)
     end

From 160aaca0fbfaa9848dec769019df9c0c78059b56 Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Fri, 12 Aug 2016 12:11:28 +0200
Subject: [PATCH 34/42] Make pipeline to be in created state for hooks tests

---
 spec/models/ci/pipeline_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index 9c5d56fe5bb..a3f9934971a 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -2,7 +2,7 @@ require 'spec_helper'
 
 describe Ci::Pipeline, models: true do
   let(:project) { FactoryGirl.create :empty_project }
-  let(:pipeline) { FactoryGirl.create :ci_empty_pipeline, project: project }
+  let(:pipeline) { FactoryGirl.create :ci_empty_pipeline, status: 'created', project: project }
 
   it { is_expected.to belong_to(:project) }
   it { is_expected.to belong_to(:user) }

From a391652fe60930e2139ecfacb175da6aa0f3b1e9 Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Fri, 12 Aug 2016 12:23:47 +0200
Subject: [PATCH 35/42] Fix test failures

---
 app/models/ci/pipeline.rb                | 2 +-
 spec/features/projects/pipelines_spec.rb | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index ad836bbebb8..98185ecd447 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -23,7 +23,7 @@ module Ci
 
     state_machine :status, initial: :created do
       event :queue do
-        transition :created => :pending
+        transition created: :pending
         transition any - [:created, :pending] => :running
       end
 
diff --git a/spec/features/projects/pipelines_spec.rb b/spec/features/projects/pipelines_spec.rb
index b57652b3ea2..29d150bc597 100644
--- a/spec/features/projects/pipelines_spec.rb
+++ b/spec/features/projects/pipelines_spec.rb
@@ -64,7 +64,7 @@ describe "Pipelines" do
         before { click_link('Retry') }
 
         it { expect(page).not_to have_link('Retry') }
-        it { expect(page).to have_selector('.ci-pending') }
+        it { expect(page).to have_selector('.ci-running') }
       end
     end
 

From a52fe1648e8a91f6e2f4b3a5c966a06b7f9e01e3 Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Fri, 12 Aug 2016 17:34:43 +0200
Subject: [PATCH 36/42] Rename queue to enqueue in tests

---
 Gemfile                         |  1 +
 Gemfile.lock                    |  3 +++
 bin/spring                      |  2 +-
 spec/models/ci/pipeline_spec.rb | 12 ++++++------
 4 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/Gemfile b/Gemfile
index 8b44b54e22c..9bcfc0302b6 100644
--- a/Gemfile
+++ b/Gemfile
@@ -296,6 +296,7 @@ group :development, :test do
   gem 'spring-commands-rspec',    '~> 1.0.4'
   gem 'spring-commands-spinach',  '~> 1.1.0'
   gem 'spring-commands-teaspoon', '~> 0.0.2'
+  gem "spring-commands-sidekiq"
 
   gem 'rubocop', '~> 0.41.2', require: false
   gem 'rubocop-rspec', '~> 1.5.0', require: false
diff --git a/Gemfile.lock b/Gemfile.lock
index 3ba6048143c..4bd5b78a047 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -696,6 +696,8 @@ GEM
     spring (1.7.2)
     spring-commands-rspec (1.0.4)
       spring (>= 0.9.1)
+    spring-commands-sidekiq (1.0.0)
+      spring (>= 0.9.1)
     spring-commands-spinach (1.1.0)
       spring (>= 0.9.1)
     spring-commands-teaspoon (0.0.2)
@@ -955,6 +957,7 @@ DEPENDENCIES
   spinach-rerun-reporter (~> 0.0.2)
   spring (~> 1.7.0)
   spring-commands-rspec (~> 1.0.4)
+  spring-commands-sidekiq
   spring-commands-spinach (~> 1.1.0)
   spring-commands-teaspoon (~> 0.0.2)
   sprockets (~> 3.6.0)
diff --git a/bin/spring b/bin/spring
index e0d140fe0c7..7fe232c3aae 100755
--- a/bin/spring
+++ b/bin/spring
@@ -3,7 +3,7 @@
 # This file loads spring without using Bundler, in order to be fast.
 # It gets overwritten when you run the `spring binstub` command.
 
-unless (defined?(Spring) || ENV['ENABLE_SPRING'] != '1') && File.basename($0) != 'spring'
+unless defined?(Spring)
   require 'rubygems'
   require 'bundler'
 
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index 19f1aacaabc..8137e9f8f71 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -335,8 +335,8 @@ describe Ci::Pipeline, models: true do
       context 'with multiple builds' do
         context 'when build is queued' do
           before do
-            build_a.queue
-            build_b.queue
+            build_a.enqueue
+            build_b.enqueue
           end
 
           it 'receive a pending event once' do
@@ -346,9 +346,9 @@ describe Ci::Pipeline, models: true do
 
         context 'when build is run' do
           before do
-            build_a.queue
+            build_a.enqueue
             build_a.run
-            build_b.queue
+            build_b.enqueue
             build_b.run
           end
 
@@ -382,8 +382,8 @@ describe Ci::Pipeline, models: true do
       let(:enabled) { false }
 
       before do
-        build_a.queue
-        build_b.queue
+        build_a.enqueue
+        build_b.enqueue
       end
 
       it 'did not execute pipeline_hook after touched' do

From 8af6bea81ca696427faddec5c8e0d5a25c7e2d22 Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Fri, 12 Aug 2016 17:52:02 +0200
Subject: [PATCH 37/42] Added documentation for pipeline hooks

---
 doc/web_hooks/web_hooks.md | 168 +++++++++++++++++++++++++++++++++++++
 1 file changed, 168 insertions(+)

diff --git a/doc/web_hooks/web_hooks.md b/doc/web_hooks/web_hooks.md
index d4b28d875cd..33c1a79d59c 100644
--- a/doc/web_hooks/web_hooks.md
+++ b/doc/web_hooks/web_hooks.md
@@ -754,6 +754,174 @@ X-Gitlab-Event: Wiki Page Hook
 }
 ```
 
+## Pipeline events
+
+Triggered on status change of Pipeline.
+
+**Request Header**:
+
+```
+X-Gitlab-Event: Pipeline Hook
+```
+
+**Request Body**:
+
+```json
+{
+   "object_kind": "pipeline",
+   "object_attributes":{
+      "id": 31,
+      "ref": "master",
+      "tag": false,
+      "sha": "bcbb5ec396a2c0f828686f14fac9b80b780504f2",
+      "before_sha": "bcbb5ec396a2c0f828686f14fac9b80b780504f2",
+      "status": "success",
+      "stages":[
+         "build",
+         "test",
+         "deploy"
+      ],
+      "created_at": "2016-08-12 15:23:28 UTC",
+      "finished_at": "2016-08-12 15:26:29 UTC",
+      "duration": 63
+   },
+   "user":{
+      "name": "Administrator",
+      "username": "root",
+      "avatar_url": "http://www.gravatar.com/avatar/e32bd13e2add097461cb96824b7a829c?s=80\u0026d=identicon"
+   },
+   "project":{
+      "name": "Gitlab Test",
+      "description": "Atque in sunt eos similique dolores voluptatem.",
+      "web_url": "http://192.168.64.1:3005/gitlab-org/gitlab-test",
+      "avatar_url": null,
+      "git_ssh_url": "git@192.168.64.1:gitlab-org/gitlab-test.git",
+      "git_http_url": "http://192.168.64.1:3005/gitlab-org/gitlab-test.git",
+      "namespace": "Gitlab Org",
+      "visibility_level": 20,
+      "path_with_namespace": "gitlab-org/gitlab-test",
+      "default_branch": "master"
+   },
+   "commit":{
+      "id": "bcbb5ec396a2c0f828686f14fac9b80b780504f2",
+      "message": "test\n",
+      "timestamp": "2016-08-12T17:23:21+02:00",
+      "url": "http://example.com/gitlab-org/gitlab-test/commit/bcbb5ec396a2c0f828686f14fac9b80b780504f2",
+      "author":{
+         "name": "User",
+         "email": "user@gitlab.com"
+      }
+   },
+   "builds":[
+      {
+         "id": 380,
+         "stage": "deploy",
+         "name": "production",
+         "status": "skipped",
+         "created_at": "2016-08-12 15:23:28 UTC",
+         "started_at": null,
+         "finished_at": null,
+         "when": "manual",
+         "manual": true,
+         "user":{
+            "name": "Administrator",
+            "username": "root",
+            "avatar_url": "http://www.gravatar.com/avatar/e32bd13e2add097461cb96824b7a829c?s=80\u0026d=identicon"
+         },
+         "runner": null,
+         "artifacts_file":{
+            "filename": null,
+            "size": null
+         }
+      },
+      {
+         "id": 377,
+         "stage": "test",
+         "name": "test-image",
+         "status": "success",
+         "created_at": "2016-08-12 15:23:28 UTC",
+         "started_at": "2016-08-12 15:26:12 UTC",
+         "finished_at": null,
+         "when": "on_success",
+         "manual": false,
+         "user":{
+            "name": "Administrator",
+            "username": "root",
+            "avatar_url": "http://www.gravatar.com/avatar/e32bd13e2add097461cb96824b7a829c?s=80\u0026d=identicon"
+         },
+         "runner": null,
+         "artifacts_file":{
+            "filename": null,
+            "size": null
+         }
+      },
+      {
+         "id": 378,
+         "stage": "test",
+         "name": "test-build",
+         "status": "success",
+         "created_at": "2016-08-12 15:23:28 UTC",
+         "started_at": "2016-08-12 15:26:12 UTC",
+         "finished_at": "2016-08-12 15:26:29 UTC",
+         "when": "on_success",
+         "manual": false,
+         "user":{
+            "name": "Administrator",
+            "username": "root",
+            "avatar_url": "http://www.gravatar.com/avatar/e32bd13e2add097461cb96824b7a829c?s=80\u0026d=identicon"
+         },
+         "runner": null,
+         "artifacts_file":{
+            "filename": null,
+            "size": null
+         }
+      },
+      {
+         "id": 376,
+         "stage": "build",
+         "name": "build-image",
+         "status": "success",
+         "created_at": "2016-08-12 15:23:28 UTC",
+         "started_at": "2016-08-12 15:24:56 UTC",
+         "finished_at": "2016-08-12 15:25:26 UTC",
+         "when": "on_success",
+         "manual": false,
+         "user":{
+            "name": "Administrator",
+            "username": "root",
+            "avatar_url": "http://www.gravatar.com/avatar/e32bd13e2add097461cb96824b7a829c?s=80\u0026d=identicon"
+         },
+         "runner": null,
+         "artifacts_file":{
+            "filename": null,
+            "size": null
+         }
+      },
+      {
+         "id": 379,
+         "stage": "deploy",
+         "name": "staging",
+         "status": "created",
+         "created_at": "2016-08-12 15:23:28 UTC",
+         "started_at": null,
+         "finished_at": null,
+         "when": "on_success",
+         "manual": false,
+         "user":{
+            "name": "Administrator",
+            "username": "root",
+            "avatar_url": "http://www.gravatar.com/avatar/e32bd13e2add097461cb96824b7a829c?s=80\u0026d=identicon"
+         },
+         "runner": null,
+         "artifacts_file":{
+            "filename": null,
+            "size": null
+         }
+      }
+   ]
+}
+```
+
 #### Example webhook receiver
 
 If you want to see GitLab's webhooks in action for testing purposes you can use

From f7abe46109c5e90ff2ac0a8f812ea43016b5c2bc Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Fri, 12 Aug 2016 21:58:28 +0200
Subject: [PATCH 38/42] Remove changes not related to this MR

---
 Gemfile      | 1 -
 Gemfile.lock | 3 ---
 bin/spring   | 2 +-
 3 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/Gemfile b/Gemfile
index 9bcfc0302b6..8b44b54e22c 100644
--- a/Gemfile
+++ b/Gemfile
@@ -296,7 +296,6 @@ group :development, :test do
   gem 'spring-commands-rspec',    '~> 1.0.4'
   gem 'spring-commands-spinach',  '~> 1.1.0'
   gem 'spring-commands-teaspoon', '~> 0.0.2'
-  gem "spring-commands-sidekiq"
 
   gem 'rubocop', '~> 0.41.2', require: false
   gem 'rubocop-rspec', '~> 1.5.0', require: false
diff --git a/Gemfile.lock b/Gemfile.lock
index 4bd5b78a047..3ba6048143c 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -696,8 +696,6 @@ GEM
     spring (1.7.2)
     spring-commands-rspec (1.0.4)
       spring (>= 0.9.1)
-    spring-commands-sidekiq (1.0.0)
-      spring (>= 0.9.1)
     spring-commands-spinach (1.1.0)
       spring (>= 0.9.1)
     spring-commands-teaspoon (0.0.2)
@@ -957,7 +955,6 @@ DEPENDENCIES
   spinach-rerun-reporter (~> 0.0.2)
   spring (~> 1.7.0)
   spring-commands-rspec (~> 1.0.4)
-  spring-commands-sidekiq
   spring-commands-spinach (~> 1.1.0)
   spring-commands-teaspoon (~> 0.0.2)
   sprockets (~> 3.6.0)
diff --git a/bin/spring b/bin/spring
index 7fe232c3aae..e0d140fe0c7 100755
--- a/bin/spring
+++ b/bin/spring
@@ -3,7 +3,7 @@
 # This file loads spring without using Bundler, in order to be fast.
 # It gets overwritten when you run the `spring binstub` command.
 
-unless defined?(Spring)
+unless (defined?(Spring) || ENV['ENABLE_SPRING'] != '1') && File.basename($0) != 'spring'
   require 'rubygems'
   require 'bundler'
 

From 5822a333d4c1bb43304c781f3a8b8d3eb99861a8 Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Fri, 12 Aug 2016 22:09:26 +0200
Subject: [PATCH 39/42] Capitalise URL on web_hooks/form

---
 app/views/shared/web_hooks/_form.html.haml | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/app/views/shared/web_hooks/_form.html.haml b/app/views/shared/web_hooks/_form.html.haml
index a672c28de39..d2ec6c3ddef 100644
--- a/app/views/shared/web_hooks/_form.html.haml
+++ b/app/views/shared/web_hooks/_form.html.haml
@@ -29,56 +29,56 @@
               = f.label :push_events, class: 'list-label' do
                 %strong Push events
               %p.light
-                This url will be triggered by a push to the repository
+                This URL will be triggered by a push to the repository
           %li
             = f.check_box :tag_push_events, class: 'pull-left'
             .prepend-left-20
               = f.label :tag_push_events, class: 'list-label' do
                 %strong Tag push events
               %p.light
-                This url will be triggered when a new tag is pushed to the repository
+                This URL will be triggered when a new tag is pushed to the repository
           %li
             = f.check_box :note_events, class: 'pull-left'
             .prepend-left-20
               = f.label :note_events, class: 'list-label' do
                 %strong Comments
               %p.light
-                This url will be triggered when someone adds a comment
+                This URL will be triggered when someone adds a comment
           %li
             = f.check_box :issues_events, class: 'pull-left'
             .prepend-left-20
               = f.label :issues_events, class: 'list-label' do
                 %strong Issues events
               %p.light
-                This url will be triggered when an issue is created/updated/merged
+                This URL will be triggered when an issue is created/updated/merged
           %li
             = f.check_box :merge_requests_events, class: 'pull-left'
             .prepend-left-20
               = f.label :merge_requests_events, class: 'list-label' do
                 %strong Merge Request events
               %p.light
-                This url will be triggered when a merge request is created/updated/merged
+                This URL will be triggered when a merge request is created/updated/merged
           %li
             = f.check_box :build_events, class: 'pull-left'
             .prepend-left-20
               = f.label :build_events, class: 'list-label' do
                 %strong Build events
               %p.light
-                This url will be triggered when the build status changes
+                This URL will be triggered when the build status changes
           %li
             = f.check_box :pipeline_events, class: 'pull-left'
             .prepend-left-20
               = f.label :pipeline_events, class: 'list-label' do
                 %strong Pipeline events
               %p.light
-                This url will be triggered when the pipeline status changes
+                This URL will be triggered when the pipeline status changes
           %li
             = f.check_box :wiki_page_events, class: 'pull-left'
             .prepend-left-20
               = f.label :wiki_page_events, class: 'list-label' do
                 %strong Wiki Page events
               %p.light
-                This url will be triggered when a wiki page is created/updated
+                This URL will be triggered when a wiki page is created/updated
       .form-group
         = f.label :enable_ssl_verification, "SSL verification", class: 'label-light checkbox'
         .checkbox

From cd4d8b91eb77b12a168bbca3a7ee9ec8bf1fba5e Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Fri, 12 Aug 2016 22:09:58 +0200
Subject: [PATCH 40/42] Make explicit call for all event types for ProjectHook
 factory

---
 spec/factories/project_hooks.rb | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/spec/factories/project_hooks.rb b/spec/factories/project_hooks.rb
index c709432c865..4fd51a23490 100644
--- a/spec/factories/project_hooks.rb
+++ b/spec/factories/project_hooks.rb
@@ -7,15 +7,13 @@ FactoryGirl.define do
     end
 
     trait :all_events_enabled do
-      %w[push_events
-         merge_requests_events
-         tag_push_events
-         issues_events
-         note_events
-         build_events
-         pipeline_events].each do |event|
-        send(event, true)
-      end
+      push_events true
+      merge_requests_events true
+      tag_push_events true
+      issues_events true
+      note_events true
+      build_events true
+      pipeline_events true
     end
   end
 end

From 7cdb5173f105247463a96f6526868627f0b26a85 Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Mon, 15 Aug 2016 23:33:36 +0200
Subject: [PATCH 41/42] Make rubocop happy

---
 .../project_services/irker_service_spec.rb     | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/spec/models/project_services/irker_service_spec.rb b/spec/models/project_services/irker_service_spec.rb
index ec092297f4f..ffb17fd3259 100644
--- a/spec/models/project_services/irker_service_spec.rb
+++ b/spec/models/project_services/irker_service_spec.rb
@@ -57,15 +57,15 @@ describe IrkerService, models: true do
       @irker_server = TCPServer.new 'localhost', 0
 
       allow(irker).to receive_messages(
-                        active: true,
-                        project: project,
-                        project_id: project.id,
-                        service_hook: true,
-                        server_host: @irker_server.addr[2],
-                        server_port: @irker_server.addr[1],
-                        default_irc_uri: 'irc://chat.freenode.net/',
-                        recipients: recipients,
-                        colorize_messages: colorize_messages)
+        active: true,
+        project: project,
+        project_id: project.id,
+        service_hook: true,
+        server_host: @irker_server.addr[2],
+        server_port: @irker_server.addr[1],
+        default_irc_uri: 'irc://chat.freenode.net/',
+        recipients: recipients,
+        colorize_messages: colorize_messages)
 
       irker.valid?
     end

From 5b52da9c32842849f0b6430a6a820fc7456b4841 Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Tue, 16 Aug 2016 10:00:13 +0200
Subject: [PATCH 42/42] Revert unrelevant changes

---
 app/models/ci/pipeline.rb         |  5 +++--
 spec/helpers/notes_helper_spec.rb |  3 +--
 spec/models/build_spec.rb         | 34 ++++++++++++++-----------------
 3 files changed, 19 insertions(+), 23 deletions(-)

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 08b104ccfc8..130afeb724e 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -250,8 +250,9 @@ module Ci
     end
 
     def execute_hooks
-      project.execute_hooks(pipeline_data, :pipeline_hooks)
-      project.execute_services(pipeline_data, :pipeline_hooks)
+      data = pipeline_data
+      project.execute_hooks(data, :pipeline_hooks)
+      project.execute_services(data, :pipeline_hooks)
     end
 
     private
diff --git a/spec/helpers/notes_helper_spec.rb b/spec/helpers/notes_helper_spec.rb
index 853b8b2f7f7..9c577501f00 100644
--- a/spec/helpers/notes_helper_spec.rb
+++ b/spec/helpers/notes_helper_spec.rb
@@ -39,8 +39,7 @@ describe NotesHelper do
 
   describe '#preload_max_access_for_authors' do
     before do
-      # #preload_max_access_for_authors would read cache from RequestStore,
-      # so we should make sure it's clean.
+      # This method reads cache from RequestStore, so make sure it's clean.
       RequestStore.clear!
     end
 
diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb
index 79f872b2624..ee2c3d04984 100644
--- a/spec/models/build_spec.rb
+++ b/spec/models/build_spec.rb
@@ -275,8 +275,7 @@ describe Ci::Build, models: true do
 
     context 'when yaml_variables are undefined' do
       before do
-        build.update(yaml_variables: nil)
-        build.reload # reload pipeline so that it resets config_processor
+        build.yaml_variables = nil
       end
 
       context 'use from gitlab-ci.yml' do
@@ -424,24 +423,22 @@ describe Ci::Build, models: true do
   describe '#stuck?' do
     subject { build.stuck? }
 
-    %w[pending].each do |state|
-      context "when commit_status.status is #{state}" do
+    context "when commit_status.status is pending" do
+      before do
+        build.status = 'pending'
+      end
+
+      it { is_expected.to be_truthy }
+
+      context "and there are specific runner" do
+        let(:runner) { create(:ci_runner, contacted_at: 1.second.ago) }
+
         before do
-          build.status = state
+          build.project.runners << runner
+          runner.save
         end
 
-        it { is_expected.to be_truthy }
-
-        context "and there are specific runner" do
-          let(:runner) { create(:ci_runner, contacted_at: 1.second.ago) }
-
-          before do
-            build.project.runners << runner
-            runner.save
-          end
-
-          it { is_expected.to be_falsey }
-        end
+        it { is_expected.to be_falsey }
       end
     end
 
@@ -904,8 +901,7 @@ describe Ci::Build, models: true do
 
     context 'when `when` is undefined' do
       before do
-        build.update(when: nil)
-        build.reload # reload pipeline so that it resets config_processor
+        build.when = nil
       end
 
       context 'use from gitlab-ci.yml' do