From 7ac89d05dbf6c932e9cb6e87cc64c46e015a22d5 Mon Sep 17 00:00:00 2001
From: James Lopez <james@jameslopez.es>
Date: Wed, 3 May 2017 14:53:54 +0200
Subject: [PATCH 01/18] add service spec

---
 spec/models/service_spec.rb | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb
index 134882648b9..e9acbb81ad9 100644
--- a/spec/models/service_spec.rb
+++ b/spec/models/service_spec.rb
@@ -254,4 +254,30 @@ describe Service, models: true do
       end
     end
   end
+
+  describe "#update_and_propagate" do
+    let!(:service) do
+      RedmineService.new(
+        project: project,
+        active: false,
+        properties: {
+          project_url: 'http://redmine/projects/project_name_in_redmine',
+          issues_url: "http://redmine/#{project.id}/project_name_in_redmine/:id",
+          new_issue_url: 'http://redmine/projects/project_name_in_redmine/issues/new'
+        }
+      )
+    end
+
+    it 'updates the service params successfully and calls the propagation worker' do
+      expect(PropagateProjectServiceWorker).to receve(:perform_async)
+
+      expect(service.update_and_propagate(active: true)).to be true
+    end
+
+    it 'updates the service params successfully' do
+      expect(PropagateProjectServiceWorker).not_to receve(:perform_asyncs)
+
+      expect(service.update_and_propagate(properties: {})).to be true
+    end
+  end
 end

From e81ea165ba738fedab07d5e20423856e004e2594 Mon Sep 17 00:00:00 2001
From: James Lopez <james@jameslopez.es>
Date: Wed, 3 May 2017 15:43:26 +0200
Subject: [PATCH 02/18] added worker spec

---
 .../propagate_project_service_worker_spec.rb  | 36 +++++++++++++++++++
 1 file changed, 36 insertions(+)
 create mode 100644 spec/workers/propagate_project_service_worker_spec.rb

diff --git a/spec/workers/propagate_project_service_worker_spec.rb b/spec/workers/propagate_project_service_worker_spec.rb
new file mode 100644
index 00000000000..ce01a663a8f
--- /dev/null
+++ b/spec/workers/propagate_project_service_worker_spec.rb
@@ -0,0 +1,36 @@
+require 'spec_helper'
+
+describe PruneOldEventsWorker do
+  describe '#perform' do
+    let!(:service_template) do
+      PushoverService.create(
+        template: true,
+        properties: {
+          device: 'MyDevice',
+          sound: 'mic',
+          priority: 4,
+          api_key: '123456789'
+        })
+    end
+
+    let!(:project) { create(:empty_project) }
+
+    it 'creates services for projects' do
+      expect { subject.perform }.to change { Service.count }.by(1)
+    end
+
+    it 'does not create the service if it exists already' do
+      Service.build_from_template(project.id, service_template).save!
+
+      expect { subject.perform }.not_to change { Service.count }
+    end
+
+    it 'creates the service containing the template attributes' do
+      subject.perform
+
+      service = Service.find_by(service_template.merge(project_id: project.id, template: false))
+
+      expect(service).not_to be_nil
+    end
+  end
+end

From 264bf229277caf1c1eaca4e83921ca1b305d5401 Mon Sep 17 00:00:00 2001
From: James Lopez <james@jameslopez.es>
Date: Wed, 3 May 2017 17:20:12 +0200
Subject: [PATCH 03/18] add propagate service worker and updated spec and
 controller

---
 app/controllers/admin/services_controller.rb  |  2 +-
 app/models/service.rb                         | 10 ++++
 .../propagate_project_service_worker.rb       | 49 +++++++++++++++++++
 .../propagate_project_service_worker_spec.rb  | 19 ++++---
 4 files changed, 73 insertions(+), 7 deletions(-)
 create mode 100644 app/workers/propagate_project_service_worker.rb

diff --git a/app/controllers/admin/services_controller.rb b/app/controllers/admin/services_controller.rb
index 37a1a23178e..2b6f335cb2b 100644
--- a/app/controllers/admin/services_controller.rb
+++ b/app/controllers/admin/services_controller.rb
@@ -15,7 +15,7 @@ class Admin::ServicesController < Admin::ApplicationController
   end
 
   def update
-    if service.update_attributes(service_params[:service])
+    if service.update_and_propagate(service_params[:service])
       redirect_to admin_application_settings_services_path,
         notice: 'Application settings saved successfully'
     else
diff --git a/app/models/service.rb b/app/models/service.rb
index c71a7d169ec..dea22fd96a7 100644
--- a/app/models/service.rb
+++ b/app/models/service.rb
@@ -254,6 +254,16 @@ class Service < ActiveRecord::Base
     service
   end
 
+  def update_and_propagate(service_params)
+    return false unless update_attributes(service_params)
+
+    if service_params[:active] == 1
+      PropagateProjectServiceWorker.perform_async(service_params[:id])
+    end
+
+    true
+  end
+
   private
 
   def cache_project_has_external_issue_tracker
diff --git a/app/workers/propagate_project_service_worker.rb b/app/workers/propagate_project_service_worker.rb
new file mode 100644
index 00000000000..53551770968
--- /dev/null
+++ b/app/workers/propagate_project_service_worker.rb
@@ -0,0 +1,49 @@
+# Worker for updating any project specific caches.
+class PropagateProjectServiceWorker
+  include Sidekiq::Worker
+  include DedicatedSidekiqQueue
+
+  LEASE_TIMEOUT = 30.minutes.to_i
+
+  def perform(template_id)
+    template = Service.find_by(id: template_id)
+
+    return unless template&.active
+    return unless try_obtain_lease_for(template.id)
+
+    Rails.logger.info("Propagating services for template #{template.id}")
+
+    project_ids_for_template(template) do |project_id|
+      Service.build_from_template(project_id, template).save!
+    end
+  end
+
+  private
+
+  def project_ids_for_template(template)
+    limit = 100
+    offset = 0
+
+    loop do
+      batch = project_ids_batch(limit, offset, template.type)
+
+      batch.each { |project_id| yield(project_id) }
+
+      break if batch.count < limit
+
+      offset += limit
+    end
+  end
+
+  def project_ids_batch(limit, offset, template_type)
+    Project.joins('LEFT JOIN services ON services.project_id = projects.id').
+      where('services.type != ? OR services.id IS NULL', template_type).
+      limit(limit).offset(offset).pluck(:id)
+  end
+
+  def try_obtain_lease_for(template_id)
+    Gitlab::ExclusiveLease.
+      new("propagate_project_service_worker:#{template_id}", timeout: LEASE_TIMEOUT).
+      try_obtain
+  end
+end
diff --git a/spec/workers/propagate_project_service_worker_spec.rb b/spec/workers/propagate_project_service_worker_spec.rb
index ce01a663a8f..d525a8b4a23 100644
--- a/spec/workers/propagate_project_service_worker_spec.rb
+++ b/spec/workers/propagate_project_service_worker_spec.rb
@@ -1,36 +1,43 @@
 require 'spec_helper'
 
-describe PruneOldEventsWorker do
+describe PropagateProjectServiceWorker do
   describe '#perform' do
     let!(:service_template) do
       PushoverService.create(
         template: true,
+        active: true,
         properties: {
           device: 'MyDevice',
           sound: 'mic',
           priority: 4,
+          user_key: 'asdf',
           api_key: '123456789'
         })
     end
 
     let!(:project) { create(:empty_project) }
 
+    before do
+      allow_any_instance_of(Gitlab::ExclusiveLease).to receive(:try_obtain).
+        and_return(true)
+    end
+
     it 'creates services for projects' do
-      expect { subject.perform }.to change { Service.count }.by(1)
+      expect { subject.perform(service_template.id) }.to change { Service.count }.by(1)
     end
 
     it 'does not create the service if it exists already' do
       Service.build_from_template(project.id, service_template).save!
 
-      expect { subject.perform }.not_to change { Service.count }
+      expect { subject.perform(service_template.id) }.not_to change { Service.count }
     end
 
     it 'creates the service containing the template attributes' do
-      subject.perform
+      subject.perform(service_template.id)
 
-      service = Service.find_by(service_template.merge(project_id: project.id, template: false))
+      service = Service.find_by(type: service_template.type, template: false)
 
-      expect(service).not_to be_nil
+      expect(service.properties).to eq(service_template.properties)
     end
   end
 end

From 2f7f1ce4e66db847414e2fc3de09556e75c51eb4 Mon Sep 17 00:00:00 2001
From: James Lopez <james@jameslopez.es>
Date: Thu, 4 May 2017 11:32:39 +0200
Subject: [PATCH 04/18] fix sidekiq spec, add changelog

---
 changelogs/unreleased/fix-admin-integrations.yml | 4 ++++
 config/sidekiq_queues.yml                        | 1 +
 2 files changed, 5 insertions(+)
 create mode 100644 changelogs/unreleased/fix-admin-integrations.yml

diff --git a/changelogs/unreleased/fix-admin-integrations.yml b/changelogs/unreleased/fix-admin-integrations.yml
new file mode 100644
index 00000000000..7689623501f
--- /dev/null
+++ b/changelogs/unreleased/fix-admin-integrations.yml
@@ -0,0 +1,4 @@
+---
+title: Fix new admin integrations not taking effect on existing projects
+merge_request:
+author:
diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml
index c3bd73533d0..91ea1c0f779 100644
--- a/config/sidekiq_queues.yml
+++ b/config/sidekiq_queues.yml
@@ -53,3 +53,4 @@
   - [pages, 1]
   - [system_hook_push, 1]
   - [update_user_activity, 1]
+  - [propagate_project_service, 1]

From f81cf84035213002ce7931af6c3ffa917fe7fcbd Mon Sep 17 00:00:00 2001
From: James Lopez <james@jameslopez.es>
Date: Thu, 4 May 2017 12:13:33 +0200
Subject: [PATCH 05/18] refactor worker into service

---
 app/services/projects/propagate_service.rb    | 47 ++++++++++++++++
 .../propagate_project_service_worker.rb       | 34 ++----------
 .../projects/propagate_service_spec.rb        | 40 ++++++++++++++
 .../propagate_project_service_worker_spec.rb  | 54 +++++++------------
 4 files changed, 111 insertions(+), 64 deletions(-)
 create mode 100644 app/services/projects/propagate_service.rb
 create mode 100644 spec/services/projects/propagate_service_spec.rb

diff --git a/app/services/projects/propagate_service.rb b/app/services/projects/propagate_service.rb
new file mode 100644
index 00000000000..3c05dcce07c
--- /dev/null
+++ b/app/services/projects/propagate_service.rb
@@ -0,0 +1,47 @@
+module Projects
+  class PropagateService
+    BATCH_SIZE = 100
+
+    def self.propagate!(*args)
+      new(*args).propagate!
+    end
+
+    def initialize(template)
+      @template = template
+    end
+
+    def propagate!
+      return unless @template&.active
+
+      Rails.logger.info("Propagating services for template #{@template.id}")
+
+      propagate_projects_with_template
+    end
+
+    private
+
+    def propagate_projects_with_template
+      offset = 0
+
+      loop do
+        batch = project_ids_batch(offset)
+
+        batch.each { |project_id| create_from_template(project_id) }
+
+        break if batch.count < BATCH_SIZE
+
+        offset += BATCH_SIZE
+      end
+    end
+
+    def create_from_template(project_id)
+      Service.build_from_template(project_id, @template).save!
+    end
+
+    def project_ids_batch(offset)
+      Project.joins('LEFT JOIN services ON services.project_id = projects.id').
+        where('services.type != ? OR services.id IS NULL', @template.type).
+        limit(BATCH_SIZE).offset(offset).pluck(:id)
+    end
+  end
+end
diff --git a/app/workers/propagate_project_service_worker.rb b/app/workers/propagate_project_service_worker.rb
index 53551770968..ab2b7738f9a 100644
--- a/app/workers/propagate_project_service_worker.rb
+++ b/app/workers/propagate_project_service_worker.rb
@@ -3,44 +3,18 @@ class PropagateProjectServiceWorker
   include Sidekiq::Worker
   include DedicatedSidekiqQueue
 
+  sidekiq_options retry: 3
+
   LEASE_TIMEOUT = 30.minutes.to_i
 
   def perform(template_id)
-    template = Service.find_by(id: template_id)
+    return unless try_obtain_lease_for(template_id)
 
-    return unless template&.active
-    return unless try_obtain_lease_for(template.id)
-
-    Rails.logger.info("Propagating services for template #{template.id}")
-
-    project_ids_for_template(template) do |project_id|
-      Service.build_from_template(project_id, template).save!
-    end
+    Projects::PropagateService.propagate!(Service.find_by(id: template_id))
   end
 
   private
 
-  def project_ids_for_template(template)
-    limit = 100
-    offset = 0
-
-    loop do
-      batch = project_ids_batch(limit, offset, template.type)
-
-      batch.each { |project_id| yield(project_id) }
-
-      break if batch.count < limit
-
-      offset += limit
-    end
-  end
-
-  def project_ids_batch(limit, offset, template_type)
-    Project.joins('LEFT JOIN services ON services.project_id = projects.id').
-      where('services.type != ? OR services.id IS NULL', template_type).
-      limit(limit).offset(offset).pluck(:id)
-  end
-
   def try_obtain_lease_for(template_id)
     Gitlab::ExclusiveLease.
       new("propagate_project_service_worker:#{template_id}", timeout: LEASE_TIMEOUT).
diff --git a/spec/services/projects/propagate_service_spec.rb b/spec/services/projects/propagate_service_spec.rb
new file mode 100644
index 00000000000..ee40a7ecbab
--- /dev/null
+++ b/spec/services/projects/propagate_service_spec.rb
@@ -0,0 +1,40 @@
+require 'spec_helper'
+
+describe Projects::PropagateService, services: true do
+  describe '.propagate!' do
+    let!(:service_template) do
+      PushoverService.create(
+        template: true,
+        active: true,
+        properties: {
+          device: 'MyDevice',
+          sound: 'mic',
+          priority: 4,
+          user_key: 'asdf',
+          api_key: '123456789'
+        })
+    end
+
+    let!(:project) { create(:empty_project) }
+
+    it 'creates services for projects' do
+      expect { described_class.propagate!(service_template) }.
+        to change { Service.count }.by(1)
+    end
+
+    it 'does not create the service if it exists already' do
+      Service.build_from_template(project.id, service_template).save!
+
+      expect { described_class.propagate!(service_template) }.
+        not_to change { Service.count }
+    end
+
+    it 'creates the service containing the template attributes' do
+      described_class.propagate!(service_template)
+
+      service = Service.find_by(type: service_template.type, template: false)
+
+      expect(service.properties).to eq(service_template.properties)
+    end
+  end
+end
diff --git a/spec/workers/propagate_project_service_worker_spec.rb b/spec/workers/propagate_project_service_worker_spec.rb
index d525a8b4a23..c16e95bd49b 100644
--- a/spec/workers/propagate_project_service_worker_spec.rb
+++ b/spec/workers/propagate_project_service_worker_spec.rb
@@ -1,43 +1,29 @@
 require 'spec_helper'
 
 describe PropagateProjectServiceWorker do
+  let!(:service_template) do
+    PushoverService.create(
+      template: true,
+      active: true,
+      properties: {
+        device: 'MyDevice',
+        sound: 'mic',
+        priority: 4,
+        user_key: 'asdf',
+        api_key: '123456789'
+      })
+  end
+
+  before do
+    allow_any_instance_of(Gitlab::ExclusiveLease).to receive(:try_obtain).
+      and_return(true)
+  end
+
   describe '#perform' do
-    let!(:service_template) do
-      PushoverService.create(
-        template: true,
-        active: true,
-        properties: {
-          device: 'MyDevice',
-          sound: 'mic',
-          priority: 4,
-          user_key: 'asdf',
-          api_key: '123456789'
-        })
-    end
+    it 'calls the propagate service with the template' do
+      expect(Projects::PropagateService).to receive(:propagate!).with(service_template)
 
-    let!(:project) { create(:empty_project) }
-
-    before do
-      allow_any_instance_of(Gitlab::ExclusiveLease).to receive(:try_obtain).
-        and_return(true)
-    end
-
-    it 'creates services for projects' do
-      expect { subject.perform(service_template.id) }.to change { Service.count }.by(1)
-    end
-
-    it 'does not create the service if it exists already' do
-      Service.build_from_template(project.id, service_template).save!
-
-      expect { subject.perform(service_template.id) }.not_to change { Service.count }
-    end
-
-    it 'creates the service containing the template attributes' do
       subject.perform(service_template.id)
-
-      service = Service.find_by(type: service_template.type, template: false)
-
-      expect(service.properties).to eq(service_template.properties)
     end
   end
 end

From 3d807dc81b27c6366390b2355e40a5c65bbf02c2 Mon Sep 17 00:00:00 2001
From: James Lopez <james@jameslopez.es>
Date: Thu, 4 May 2017 12:21:35 +0200
Subject: [PATCH 06/18] update lease timeout

---
 app/workers/propagate_project_service_worker.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/workers/propagate_project_service_worker.rb b/app/workers/propagate_project_service_worker.rb
index ab2b7738f9a..6cc3fe84635 100644
--- a/app/workers/propagate_project_service_worker.rb
+++ b/app/workers/propagate_project_service_worker.rb
@@ -5,7 +5,7 @@ class PropagateProjectServiceWorker
 
   sidekiq_options retry: 3
 
-  LEASE_TIMEOUT = 30.minutes.to_i
+  LEASE_TIMEOUT = 4.hours.to_i
 
   def perform(template_id)
     return unless try_obtain_lease_for(template_id)

From 3bff8da8c1e3223e81bccd5343902b840f005fcf Mon Sep 17 00:00:00 2001
From: James Lopez <james@jameslopez.es>
Date: Thu, 4 May 2017 12:37:54 +0200
Subject: [PATCH 07/18] fix service spec

---
 app/models/service.rb       |  2 +-
 spec/models/service_spec.rb | 11 ++++++-----
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/app/models/service.rb b/app/models/service.rb
index dea22fd96a7..18c046aff54 100644
--- a/app/models/service.rb
+++ b/app/models/service.rb
@@ -257,7 +257,7 @@ class Service < ActiveRecord::Base
   def update_and_propagate(service_params)
     return false unless update_attributes(service_params)
 
-    if service_params[:active] == 1
+    if service_params[:active]
       PropagateProjectServiceWorker.perform_async(service_params[:id])
     end
 
diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb
index e9acbb81ad9..7a5fb509bf5 100644
--- a/spec/models/service_spec.rb
+++ b/spec/models/service_spec.rb
@@ -256,26 +256,27 @@ describe Service, models: true do
   end
 
   describe "#update_and_propagate" do
+    let(:project) { create(:empty_project) }
     let!(:service) do
       RedmineService.new(
         project: project,
         active: false,
         properties: {
-          project_url: 'http://redmine/projects/project_name_in_redmine',
-          issues_url: "http://redmine/#{project.id}/project_name_in_redmine/:id",
-          new_issue_url: 'http://redmine/projects/project_name_in_redmine/issues/new'
+          project_url: 'http://abc',
+          issues_url: 'http://abc',
+          new_issue_url: 'http://abc'
         }
       )
     end
 
     it 'updates the service params successfully and calls the propagation worker' do
-      expect(PropagateProjectServiceWorker).to receve(:perform_async)
+      expect(PropagateProjectServiceWorker).to receive(:perform_async)
 
       expect(service.update_and_propagate(active: true)).to be true
     end
 
     it 'updates the service params successfully' do
-      expect(PropagateProjectServiceWorker).not_to receve(:perform_asyncs)
+      expect(PropagateProjectServiceWorker).not_to receive(:perform_async)
 
       expect(service.update_and_propagate(properties: {})).to be true
     end

From b871564383cbade7fff312b8f045cee6c871f1e0 Mon Sep 17 00:00:00 2001
From: James Lopez <james@jameslopez.es>
Date: Thu, 4 May 2017 12:46:03 +0200
Subject: [PATCH 08/18] fix service spec

---
 app/models/service.rb       | 2 +-
 spec/models/service_spec.rb | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/app/models/service.rb b/app/models/service.rb
index 18c046aff54..f8534387703 100644
--- a/app/models/service.rb
+++ b/app/models/service.rb
@@ -258,7 +258,7 @@ class Service < ActiveRecord::Base
     return false unless update_attributes(service_params)
 
     if service_params[:active]
-      PropagateProjectServiceWorker.perform_async(service_params[:id])
+      PropagateProjectServiceWorker.perform_async(id)
     end
 
     true
diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb
index 7a5fb509bf5..3a7d8b72993 100644
--- a/spec/models/service_spec.rb
+++ b/spec/models/service_spec.rb
@@ -258,7 +258,7 @@ describe Service, models: true do
   describe "#update_and_propagate" do
     let(:project) { create(:empty_project) }
     let!(:service) do
-      RedmineService.new(
+      RedmineService.create(
         project: project,
         active: false,
         properties: {
@@ -270,7 +270,7 @@ describe Service, models: true do
     end
 
     it 'updates the service params successfully and calls the propagation worker' do
-      expect(PropagateProjectServiceWorker).to receive(:perform_async)
+      expect(PropagateProjectServiceWorker).to receive(:perform_async).with(service.id)
 
       expect(service.update_and_propagate(active: true)).to be true
     end

From 78d059141b5f3345d797992e03680654ce762c76 Mon Sep 17 00:00:00 2001
From: James Lopez <james@jameslopez.es>
Date: Thu, 4 May 2017 16:41:07 +0200
Subject: [PATCH 09/18] add more examples for testing SQL

---
 .../projects/propagate_service_spec.rb        | 30 +++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/spec/services/projects/propagate_service_spec.rb b/spec/services/projects/propagate_service_spec.rb
index ee40a7ecbab..409c14655cd 100644
--- a/spec/services/projects/propagate_service_spec.rb
+++ b/spec/services/projects/propagate_service_spec.rb
@@ -22,8 +22,38 @@ describe Projects::PropagateService, services: true do
         to change { Service.count }.by(1)
     end
 
+    it 'creates services for a project that has another service' do
+      other_service = BambooService.create(
+        template: true,
+        active: true,
+        properties: {
+          bamboo_url: 'http://gitlab.com',
+          username: 'mic',
+          password: "password",
+          build_key: 'build'
+        }
+      )
+
+      Service.build_from_template(project.id, other_service).save!
+
+      expect { described_class.propagate!(service_template) }.
+        to change { Service.count }.by(1)
+    end
+
     it 'does not create the service if it exists already' do
+      other_service = BambooService.create(
+        template: true,
+        active: true,
+        properties: {
+          bamboo_url: 'http://gitlab.com',
+          username: 'mic',
+          password: "password",
+          build_key: 'build'
+        }
+      )
+
       Service.build_from_template(project.id, service_template).save!
+      Service.build_from_template(project.id, other_service).save!
 
       expect { described_class.propagate!(service_template) }.
         not_to change { Service.count }

From cf002738e766f977bdb0e857759f548a5c65c9bd Mon Sep 17 00:00:00 2001
From: James Lopez <james@jameslopez.es>
Date: Thu, 4 May 2017 18:11:28 +0200
Subject: [PATCH 10/18] refactor a few things based on feedback

---
 app/controllers/admin/services_controller.rb  |  4 ++-
 app/models/service.rb                         | 10 ------
 app/services/projects/propagate_service.rb    |  8 ++---
 .../propagate_project_service_worker.rb       |  2 +-
 .../admin/services_controller_spec.rb         | 32 +++++++++++++++++++
 spec/models/service_spec.rb                   | 27 ----------------
 .../projects/propagate_service_spec.rb        |  8 ++---
 .../propagate_project_service_worker_spec.rb  |  2 +-
 8 files changed, 45 insertions(+), 48 deletions(-)

diff --git a/app/controllers/admin/services_controller.rb b/app/controllers/admin/services_controller.rb
index 2b6f335cb2b..e335fbfffed 100644
--- a/app/controllers/admin/services_controller.rb
+++ b/app/controllers/admin/services_controller.rb
@@ -15,7 +15,9 @@ class Admin::ServicesController < Admin::ApplicationController
   end
 
   def update
-    if service.update_and_propagate(service_params[:service])
+    if service.update_attributes(service_params[:service])
+      PropagateProjectServiceWorker.perform_async(service.id) if  service.active?
+
       redirect_to admin_application_settings_services_path,
         notice: 'Application settings saved successfully'
     else
diff --git a/app/models/service.rb b/app/models/service.rb
index f8534387703..c71a7d169ec 100644
--- a/app/models/service.rb
+++ b/app/models/service.rb
@@ -254,16 +254,6 @@ class Service < ActiveRecord::Base
     service
   end
 
-  def update_and_propagate(service_params)
-    return false unless update_attributes(service_params)
-
-    if service_params[:active]
-      PropagateProjectServiceWorker.perform_async(id)
-    end
-
-    true
-  end
-
   private
 
   def cache_project_has_external_issue_tracker
diff --git a/app/services/projects/propagate_service.rb b/app/services/projects/propagate_service.rb
index 3c05dcce07c..6e24a67d8b0 100644
--- a/app/services/projects/propagate_service.rb
+++ b/app/services/projects/propagate_service.rb
@@ -2,15 +2,15 @@ module Projects
   class PropagateService
     BATCH_SIZE = 100
 
-    def self.propagate!(*args)
-      new(*args).propagate!
+    def self.propagate(*args)
+      new(*args).propagate
     end
 
     def initialize(template)
       @template = template
     end
 
-    def propagate!
+    def propagate
       return unless @template&.active
 
       Rails.logger.info("Propagating services for template #{@template.id}")
@@ -28,7 +28,7 @@ module Projects
 
         batch.each { |project_id| create_from_template(project_id) }
 
-        break if batch.count < BATCH_SIZE
+        break if batch.size < BATCH_SIZE
 
         offset += BATCH_SIZE
       end
diff --git a/app/workers/propagate_project_service_worker.rb b/app/workers/propagate_project_service_worker.rb
index 6cc3fe84635..5eabe4eaecd 100644
--- a/app/workers/propagate_project_service_worker.rb
+++ b/app/workers/propagate_project_service_worker.rb
@@ -10,7 +10,7 @@ class PropagateProjectServiceWorker
   def perform(template_id)
     return unless try_obtain_lease_for(template_id)
 
-    Projects::PropagateService.propagate!(Service.find_by(id: template_id))
+    Projects::PropagateService.propagate(Service.find_by(id: template_id))
   end
 
   private
diff --git a/spec/controllers/admin/services_controller_spec.rb b/spec/controllers/admin/services_controller_spec.rb
index e5cdd52307e..808c98edb7f 100644
--- a/spec/controllers/admin/services_controller_spec.rb
+++ b/spec/controllers/admin/services_controller_spec.rb
@@ -23,4 +23,36 @@ describe Admin::ServicesController do
       end
     end
   end
+
+  describe "#update" do
+    let(:project) { create(:empty_project) }
+    let!(:service) do
+      RedmineService.create(
+        project: project,
+        active: false,
+        template: true,
+        properties: {
+          project_url: 'http://abc',
+          issues_url: 'http://abc',
+          new_issue_url: 'http://abc'
+        }
+      )
+    end
+
+    it 'updates the service params successfully and calls the propagation worker' do
+      expect(PropagateProjectServiceWorker).to receive(:perform_async).with(service.id)
+
+      put :update, id: service.id, service: { active: true }
+
+      expect(response).to have_http_status(302)
+    end
+
+    it 'updates the service params successfully' do
+      expect(PropagateProjectServiceWorker).not_to receive(:perform_async)
+
+      put :update, id: service.id, service: { properties: {} }
+
+      expect(response).to have_http_status(302)
+    end
+  end
 end
diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb
index 3a7d8b72993..134882648b9 100644
--- a/spec/models/service_spec.rb
+++ b/spec/models/service_spec.rb
@@ -254,31 +254,4 @@ describe Service, models: true do
       end
     end
   end
-
-  describe "#update_and_propagate" do
-    let(:project) { create(:empty_project) }
-    let!(:service) do
-      RedmineService.create(
-        project: project,
-        active: false,
-        properties: {
-          project_url: 'http://abc',
-          issues_url: 'http://abc',
-          new_issue_url: 'http://abc'
-        }
-      )
-    end
-
-    it 'updates the service params successfully and calls the propagation worker' do
-      expect(PropagateProjectServiceWorker).to receive(:perform_async).with(service.id)
-
-      expect(service.update_and_propagate(active: true)).to be true
-    end
-
-    it 'updates the service params successfully' do
-      expect(PropagateProjectServiceWorker).not_to receive(:perform_async)
-
-      expect(service.update_and_propagate(properties: {})).to be true
-    end
-  end
 end
diff --git a/spec/services/projects/propagate_service_spec.rb b/spec/services/projects/propagate_service_spec.rb
index 409c14655cd..d4ec7c0b357 100644
--- a/spec/services/projects/propagate_service_spec.rb
+++ b/spec/services/projects/propagate_service_spec.rb
@@ -18,7 +18,7 @@ describe Projects::PropagateService, services: true do
     let!(:project) { create(:empty_project) }
 
     it 'creates services for projects' do
-      expect { described_class.propagate!(service_template) }.
+      expect { described_class.propagate(service_template) }.
         to change { Service.count }.by(1)
     end
 
@@ -36,7 +36,7 @@ describe Projects::PropagateService, services: true do
 
       Service.build_from_template(project.id, other_service).save!
 
-      expect { described_class.propagate!(service_template) }.
+      expect { described_class.propagate(service_template) }.
         to change { Service.count }.by(1)
     end
 
@@ -55,12 +55,12 @@ describe Projects::PropagateService, services: true do
       Service.build_from_template(project.id, service_template).save!
       Service.build_from_template(project.id, other_service).save!
 
-      expect { described_class.propagate!(service_template) }.
+      expect { described_class.propagate(service_template) }.
         not_to change { Service.count }
     end
 
     it 'creates the service containing the template attributes' do
-      described_class.propagate!(service_template)
+      described_class.propagate(service_template)
 
       service = Service.find_by(type: service_template.type, template: false)
 
diff --git a/spec/workers/propagate_project_service_worker_spec.rb b/spec/workers/propagate_project_service_worker_spec.rb
index c16e95bd49b..4c7edbfcd3e 100644
--- a/spec/workers/propagate_project_service_worker_spec.rb
+++ b/spec/workers/propagate_project_service_worker_spec.rb
@@ -21,7 +21,7 @@ describe PropagateProjectServiceWorker do
 
   describe '#perform' do
     it 'calls the propagate service with the template' do
-      expect(Projects::PropagateService).to receive(:propagate!).with(service_template)
+      expect(Projects::PropagateService).to receive(:propagate).with(service_template)
 
       subject.perform(service_template.id)
     end

From 1fe8b7f646603239f530b1a18427f4f5bc0e2060 Mon Sep 17 00:00:00 2001
From: James Lopez <james@jameslopez.es>
Date: Fri, 5 May 2017 09:40:44 +0200
Subject: [PATCH 11/18] refactor propagate service to use batch inserts and
 subquery instead of left join

---
 app/services/projects/propagate_service.rb | 32 ++++++++++++++++++----
 1 file changed, 26 insertions(+), 6 deletions(-)

diff --git a/app/services/projects/propagate_service.rb b/app/services/projects/propagate_service.rb
index 6e24a67d8b0..b067fc2cd64 100644
--- a/app/services/projects/propagate_service.rb
+++ b/app/services/projects/propagate_service.rb
@@ -26,7 +26,7 @@ module Projects
       loop do
         batch = project_ids_batch(offset)
 
-        batch.each { |project_id| create_from_template(project_id) }
+        bulk_create_from_template(batch)
 
         break if batch.size < BATCH_SIZE
 
@@ -34,14 +34,34 @@ module Projects
       end
     end
 
-    def create_from_template(project_id)
-      Service.build_from_template(project_id, @template).save!
+    def bulk_create_from_template(batch)
+      service_hash_list = batch.map do |project_id|
+        service_hash.merge('project_id' => project_id)
+      end
+
+      Project.transaction do
+        Service.create!(service_hash_list)
+      end
     end
 
     def project_ids_batch(offset)
-      Project.joins('LEFT JOIN services ON services.project_id = projects.id').
-        where('services.type != ? OR services.id IS NULL', @template.type).
-        limit(BATCH_SIZE).offset(offset).pluck(:id)
+      Project.connection.execute(
+        <<-SQL
+          SELECT id
+          FROM projects
+          WHERE NOT EXISTS (
+            SELECT true
+            FROM services
+            WHERE services.project_id = projects.id
+            AND services.type = '#{@template.type}'
+          )
+          LIMIT #{BATCH_SIZE} OFFSET #{offset}
+      SQL
+      ).to_a.flatten
+    end
+
+    def service_hash
+      @service_hash ||= @template.as_json(methods: :type).except('id', 'template')
     end
   end
 end

From adcff298f8f3041faa29b75ee3711fb4ce1cbb69 Mon Sep 17 00:00:00 2001
From: James Lopez <james@jameslopez.es>
Date: Fri, 5 May 2017 10:43:56 +0200
Subject: [PATCH 12/18] fixed all issues - not doing bulk create.

---
 app/services/projects/propagate_service.rb       | 10 +++-------
 spec/services/projects/propagate_service_spec.rb | 12 ++++++++++++
 2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/app/services/projects/propagate_service.rb b/app/services/projects/propagate_service.rb
index b067fc2cd64..716a6209537 100644
--- a/app/services/projects/propagate_service.rb
+++ b/app/services/projects/propagate_service.rb
@@ -21,16 +21,12 @@ module Projects
     private
 
     def propagate_projects_with_template
-      offset = 0
-
       loop do
-        batch = project_ids_batch(offset)
+        batch = project_ids_batch
 
         bulk_create_from_template(batch)
 
         break if batch.size < BATCH_SIZE
-
-        offset += BATCH_SIZE
       end
     end
 
@@ -44,7 +40,7 @@ module Projects
       end
     end
 
-    def project_ids_batch(offset)
+    def project_ids_batch
       Project.connection.execute(
         <<-SQL
           SELECT id
@@ -55,7 +51,7 @@ module Projects
             WHERE services.project_id = projects.id
             AND services.type = '#{@template.type}'
           )
-          LIMIT #{BATCH_SIZE} OFFSET #{offset}
+          LIMIT #{BATCH_SIZE}
       SQL
       ).to_a.flatten
     end
diff --git a/spec/services/projects/propagate_service_spec.rb b/spec/services/projects/propagate_service_spec.rb
index d4ec7c0b357..ac25c8b3d56 100644
--- a/spec/services/projects/propagate_service_spec.rb
+++ b/spec/services/projects/propagate_service_spec.rb
@@ -66,5 +66,17 @@ describe Projects::PropagateService, services: true do
 
       expect(service.properties).to eq(service_template.properties)
     end
+
+    describe 'bulk update' do
+      it 'creates services for all projects' do
+        project_total = 5
+        stub_const 'Projects::PropagateService::BATCH_SIZE', 3
+
+        project_total.times { create(:empty_project) }
+
+        expect { described_class.propagate(service_template) }.
+          to change { Service.count }.by(project_total + 1)
+      end
+    end
   end
 end

From 9ec39568c5284f5a3a17a342d12f87befb6cfb4c Mon Sep 17 00:00:00 2001
From: James Lopez <james@jameslopez.es>
Date: Fri, 5 May 2017 10:51:25 +0200
Subject: [PATCH 13/18] use select_values

---
 app/services/projects/propagate_service.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/services/projects/propagate_service.rb b/app/services/projects/propagate_service.rb
index 716a6209537..f952b5108bb 100644
--- a/app/services/projects/propagate_service.rb
+++ b/app/services/projects/propagate_service.rb
@@ -41,7 +41,7 @@ module Projects
     end
 
     def project_ids_batch
-      Project.connection.execute(
+      Project.connection.select_values(
         <<-SQL
           SELECT id
           FROM projects
@@ -53,7 +53,7 @@ module Projects
           )
           LIMIT #{BATCH_SIZE}
       SQL
-      ).to_a.flatten
+      )
     end
 
     def service_hash

From 606584c115d5f7a22f3b5c7e0ac6803b96fe999e Mon Sep 17 00:00:00 2001
From: James Lopez <james@jameslopez.es>
Date: Fri, 5 May 2017 14:43:22 +0200
Subject: [PATCH 14/18] bulk insert FTW - This would introduce more complexity,
 but should be faster

---
 app/services/projects/propagate_service.rb | 27 ++++++++++++++++------
 lib/gitlab/sql/bulk_insert.rb              | 21 +++++++++++++++++
 2 files changed, 41 insertions(+), 7 deletions(-)
 create mode 100644 lib/gitlab/sql/bulk_insert.rb

diff --git a/app/services/projects/propagate_service.rb b/app/services/projects/propagate_service.rb
index f952b5108bb..c420f24fe02 100644
--- a/app/services/projects/propagate_service.rb
+++ b/app/services/projects/propagate_service.rb
@@ -24,20 +24,23 @@ module Projects
       loop do
         batch = project_ids_batch
 
-        bulk_create_from_template(batch)
+        bulk_create_from_template(batch) unless batch.empty?
 
         break if batch.size < BATCH_SIZE
       end
     end
 
     def bulk_create_from_template(batch)
-      service_hash_list = batch.map do |project_id|
-        service_hash.merge('project_id' => project_id)
+      service_list = batch.map do |project_id|
+        service_hash.merge('project_id' => project_id).values
       end
 
-      Project.transaction do
-        Service.create!(service_hash_list)
-      end
+      # Project.transaction do
+      #   Service.create!(service_hash_list)
+      # end
+      Gitlab::SQL::BulkInsert.new(service_hash.keys + ['project_id'],
+                                  service_list,
+                                  'services').execute
     end
 
     def project_ids_batch
@@ -57,7 +60,17 @@ module Projects
     end
 
     def service_hash
-      @service_hash ||= @template.as_json(methods: :type).except('id', 'template')
+      @service_hash ||=
+        begin
+          template_hash = @template.as_json(methods: :type).except('id', 'template', 'project_id')
+
+          template_hash.each_with_object({}) do |(key, value), service_hash|
+            value = value.is_a?(Hash) ? value.to_json : value
+            key = Gitlab::Database.postgresql? ? "\"#{key}\"" : "`#{key}`"
+
+            service_hash[key] = ActiveRecord::Base.sanitize(value)
+          end
+        end
     end
   end
 end
diff --git a/lib/gitlab/sql/bulk_insert.rb b/lib/gitlab/sql/bulk_insert.rb
new file mode 100644
index 00000000000..097f9ff237b
--- /dev/null
+++ b/lib/gitlab/sql/bulk_insert.rb
@@ -0,0 +1,21 @@
+module Gitlab
+  module SQL
+    # Class for building SQL bulk inserts
+    class BulkInsert
+      def initialize(columns, values_array, table)
+        @columns = columns
+        @values_array = values_array
+        @table = table
+      end
+
+      def execute
+        ActiveRecord::Base.connection.execute(
+          <<-SQL.strip_heredoc
+          INSERT INTO #{@table} (#{@columns.join(', ')})
+          VALUES #{@values_array.map { |tuple| "(#{tuple.join(', ')})" }.join(', ')}
+        SQL
+        )
+      end
+    end
+  end
+end

From ce418036c763219df8239632f71ef0e9782be7ea Mon Sep 17 00:00:00 2001
From: James Lopez <james@jameslopez.es>
Date: Fri, 5 May 2017 16:16:02 +0200
Subject: [PATCH 15/18] add callbacks in bulk

---
 app/services/projects/propagate_service.rb    | 30 +++++++++++++++----
 .../projects/propagate_service_spec.rb        | 18 +++++++++++
 2 files changed, 42 insertions(+), 6 deletions(-)

diff --git a/app/services/projects/propagate_service.rb b/app/services/projects/propagate_service.rb
index c420f24fe02..f4fae478609 100644
--- a/app/services/projects/propagate_service.rb
+++ b/app/services/projects/propagate_service.rb
@@ -35,12 +35,12 @@ module Projects
         service_hash.merge('project_id' => project_id).values
       end
 
-      # Project.transaction do
-      #   Service.create!(service_hash_list)
-      # end
-      Gitlab::SQL::BulkInsert.new(service_hash.keys + ['project_id'],
-                                  service_list,
-                                  'services').execute
+      Project.transaction do
+        Gitlab::SQL::BulkInsert.new(service_hash.keys + ['project_id'],
+                                    service_list,
+                                    'services').execute
+        run_callbacks(batch)
+      end
     end
 
     def project_ids_batch
@@ -72,5 +72,23 @@ module Projects
           end
         end
     end
+
+    def run_callbacks(batch)
+      if active_external_issue_tracker?
+        Project.where(id: batch).update_all(has_external_issue_tracker: true)
+      end
+
+      if active_external_wiki?
+        Project.where(id: batch).update_all(has_external_wiki: true)
+      end
+    end
+
+    def active_external_issue_tracker?
+      @template['category'] == 'issue_tracker' && @template['active'] && !@template['default']
+    end
+
+    def active_external_wiki?
+      @template['type'] == 'ExternalWikiService' && @template['active']
+    end
   end
 end
diff --git a/spec/services/projects/propagate_service_spec.rb b/spec/services/projects/propagate_service_spec.rb
index ac25c8b3d56..b8aa4de5bd1 100644
--- a/spec/services/projects/propagate_service_spec.rb
+++ b/spec/services/projects/propagate_service_spec.rb
@@ -78,5 +78,23 @@ describe Projects::PropagateService, services: true do
           to change { Service.count }.by(project_total + 1)
       end
     end
+
+    describe 'external tracker' do
+      it 'updates the project external tracker' do
+        service_template.update(category: 'issue_tracker', default: false)
+
+        expect { described_class.propagate(service_template) }.
+          to change { project.reload.has_external_issue_tracker }.to(true)
+      end
+    end
+
+    describe 'external wiki' do
+      it 'updates the project external tracker' do
+        service_template.update(type: 'ExternalWikiService')
+
+        expect { described_class.propagate(service_template) }.
+          to change { project.reload.has_external_wiki }.to(true)
+      end
+    end
   end
 end

From 6ecf16b8f70335e1e6868b7c918cd031f5eb2f8d Mon Sep 17 00:00:00 2001
From: James Lopez <james@jameslopez.es>
Date: Fri, 5 May 2017 18:01:33 +0200
Subject: [PATCH 16/18] refactor code based on feedback

---
 app/controllers/admin/services_controller.rb  |  2 +-
 ...rvice.rb => propagate_service_template.rb} | 21 +++++++++++++------
 ...b => propagate_service_template_worker.rb} |  6 +++---
 config/sidekiq_queues.yml                     |  2 +-
 lib/gitlab/sql/bulk_insert.rb                 | 21 -------------------
 .../admin/services_controller_spec.rb         |  8 +++----
 ....rb => propagate_service_template_spec.rb} | 17 +++++++--------
 ...propagate_service_template_worker_spec.rb} |  4 ++--
 8 files changed, 34 insertions(+), 47 deletions(-)
 rename app/services/projects/{propagate_service.rb => propagate_service_template.rb} (77%)
 rename app/workers/{propagate_project_service_worker.rb => propagate_service_template_worker.rb} (63%)
 delete mode 100644 lib/gitlab/sql/bulk_insert.rb
 rename spec/services/projects/{propagate_service_spec.rb => propagate_service_template_spec.rb} (83%)
 rename spec/workers/{propagate_project_service_worker_spec.rb => propagate_service_template_worker_spec.rb} (79%)

diff --git a/app/controllers/admin/services_controller.rb b/app/controllers/admin/services_controller.rb
index e335fbfffed..a40ce3c2418 100644
--- a/app/controllers/admin/services_controller.rb
+++ b/app/controllers/admin/services_controller.rb
@@ -16,7 +16,7 @@ class Admin::ServicesController < Admin::ApplicationController
 
   def update
     if service.update_attributes(service_params[:service])
-      PropagateProjectServiceWorker.perform_async(service.id) if  service.active?
+      PropagateServiceTemplateWorker.perform_async(service.id) if  service.active?
 
       redirect_to admin_application_settings_services_path,
         notice: 'Application settings saved successfully'
diff --git a/app/services/projects/propagate_service.rb b/app/services/projects/propagate_service_template.rb
similarity index 77%
rename from app/services/projects/propagate_service.rb
rename to app/services/projects/propagate_service_template.rb
index f4fae478609..32ad68673ac 100644
--- a/app/services/projects/propagate_service.rb
+++ b/app/services/projects/propagate_service_template.rb
@@ -1,5 +1,5 @@
 module Projects
-  class PropagateService
+  class PropagateServiceTemplate
     BATCH_SIZE = 100
 
     def self.propagate(*args)
@@ -36,9 +36,7 @@ module Projects
       end
 
       Project.transaction do
-        Gitlab::SQL::BulkInsert.new(service_hash.keys + ['project_id'],
-                                    service_list,
-                                    'services').execute
+        bulk_insert_services(service_hash.keys + ['project_id'], service_list)
         run_callbacks(batch)
       end
     end
@@ -54,11 +52,22 @@ module Projects
             WHERE services.project_id = projects.id
             AND services.type = '#{@template.type}'
           )
+          AND projects.pending_delete = false
+          AND projects.archived = false
           LIMIT #{BATCH_SIZE}
       SQL
       )
     end
 
+    def bulk_insert_services(columns, values_array)
+      ActiveRecord::Base.connection.execute(
+        <<-SQL.strip_heredoc
+          INSERT INTO services (#{columns.join(', ')})
+          VALUES #{values_array.map { |tuple| "(#{tuple.join(', ')})" }.join(', ')}
+      SQL
+      )
+    end
+
     def service_hash
       @service_hash ||=
         begin
@@ -84,11 +93,11 @@ module Projects
     end
 
     def active_external_issue_tracker?
-      @template['category'] == 'issue_tracker' && @template['active'] && !@template['default']
+      @template.category == :issue_tracker && !@template.default
     end
 
     def active_external_wiki?
-      @template['type'] == 'ExternalWikiService' && @template['active']
+      @template.type == 'ExternalWikiService'
     end
   end
 end
diff --git a/app/workers/propagate_project_service_worker.rb b/app/workers/propagate_service_template_worker.rb
similarity index 63%
rename from app/workers/propagate_project_service_worker.rb
rename to app/workers/propagate_service_template_worker.rb
index 5eabe4eaecd..f1fc7ccb955 100644
--- a/app/workers/propagate_project_service_worker.rb
+++ b/app/workers/propagate_service_template_worker.rb
@@ -1,5 +1,5 @@
 # Worker for updating any project specific caches.
-class PropagateProjectServiceWorker
+class PropagateServiceTemplateWorker
   include Sidekiq::Worker
   include DedicatedSidekiqQueue
 
@@ -10,14 +10,14 @@ class PropagateProjectServiceWorker
   def perform(template_id)
     return unless try_obtain_lease_for(template_id)
 
-    Projects::PropagateService.propagate(Service.find_by(id: template_id))
+    Projects::PropagateServiceTemplate.propagate(Service.find_by(id: template_id))
   end
 
   private
 
   def try_obtain_lease_for(template_id)
     Gitlab::ExclusiveLease.
-      new("propagate_project_service_worker:#{template_id}", timeout: LEASE_TIMEOUT).
+      new("propagate_service_template_worker:#{template_id}", timeout: LEASE_TIMEOUT).
       try_obtain
   end
 end
diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml
index 91ea1c0f779..433381e79d3 100644
--- a/config/sidekiq_queues.yml
+++ b/config/sidekiq_queues.yml
@@ -53,4 +53,4 @@
   - [pages, 1]
   - [system_hook_push, 1]
   - [update_user_activity, 1]
-  - [propagate_project_service, 1]
+  - [propagate_service_template, 1]
diff --git a/lib/gitlab/sql/bulk_insert.rb b/lib/gitlab/sql/bulk_insert.rb
deleted file mode 100644
index 097f9ff237b..00000000000
--- a/lib/gitlab/sql/bulk_insert.rb
+++ /dev/null
@@ -1,21 +0,0 @@
-module Gitlab
-  module SQL
-    # Class for building SQL bulk inserts
-    class BulkInsert
-      def initialize(columns, values_array, table)
-        @columns = columns
-        @values_array = values_array
-        @table = table
-      end
-
-      def execute
-        ActiveRecord::Base.connection.execute(
-          <<-SQL.strip_heredoc
-          INSERT INTO #{@table} (#{@columns.join(', ')})
-          VALUES #{@values_array.map { |tuple| "(#{tuple.join(', ')})" }.join(', ')}
-        SQL
-        )
-      end
-    end
-  end
-end
diff --git a/spec/controllers/admin/services_controller_spec.rb b/spec/controllers/admin/services_controller_spec.rb
index 808c98edb7f..c94616d8508 100644
--- a/spec/controllers/admin/services_controller_spec.rb
+++ b/spec/controllers/admin/services_controller_spec.rb
@@ -39,16 +39,16 @@ describe Admin::ServicesController do
       )
     end
 
-    it 'updates the service params successfully and calls the propagation worker' do
-      expect(PropagateProjectServiceWorker).to receive(:perform_async).with(service.id)
+    it 'calls the propagation worker when service is active' do
+      expect(PropagateServiceTemplateWorker).to receive(:perform_async).with(service.id)
 
       put :update, id: service.id, service: { active: true }
 
       expect(response).to have_http_status(302)
     end
 
-    it 'updates the service params successfully' do
-      expect(PropagateProjectServiceWorker).not_to receive(:perform_async)
+    it 'does not call the propagation worker when service is not active' do
+      expect(PropagateServiceTemplateWorker).not_to receive(:perform_async)
 
       put :update, id: service.id, service: { properties: {} }
 
diff --git a/spec/services/projects/propagate_service_spec.rb b/spec/services/projects/propagate_service_template_spec.rb
similarity index 83%
rename from spec/services/projects/propagate_service_spec.rb
rename to spec/services/projects/propagate_service_template_spec.rb
index b8aa4de5bd1..331fb3c5ac5 100644
--- a/spec/services/projects/propagate_service_spec.rb
+++ b/spec/services/projects/propagate_service_template_spec.rb
@@ -1,7 +1,7 @@
 require 'spec_helper'
 
-describe Projects::PropagateService, services: true do
-  describe '.propagate!' do
+describe Projects::PropagateServiceTemplate, services: true do
+  describe '.propagate' do
     let!(:service_template) do
       PushoverService.create(
         template: true,
@@ -23,9 +23,10 @@ describe Projects::PropagateService, services: true do
     end
 
     it 'creates services for a project that has another service' do
-      other_service = BambooService.create(
+      BambooService.create(
         template: true,
         active: true,
+        project: project,
         properties: {
           bamboo_url: 'http://gitlab.com',
           username: 'mic',
@@ -34,8 +35,6 @@ describe Projects::PropagateService, services: true do
         }
       )
 
-      Service.build_from_template(project.id, other_service).save!
-
       expect { described_class.propagate(service_template) }.
         to change { Service.count }.by(1)
     end
@@ -62,7 +61,7 @@ describe Projects::PropagateService, services: true do
     it 'creates the service containing the template attributes' do
       described_class.propagate(service_template)
 
-      service = Service.find_by(type: service_template.type, template: false)
+      service = Service.find_by!(type: service_template.type, template: false)
 
       expect(service.properties).to eq(service_template.properties)
     end
@@ -70,7 +69,7 @@ describe Projects::PropagateService, services: true do
     describe 'bulk update' do
       it 'creates services for all projects' do
         project_total = 5
-        stub_const 'Projects::PropagateService::BATCH_SIZE', 3
+        stub_const 'Projects::PropagateServiceTemplate::BATCH_SIZE', 3
 
         project_total.times { create(:empty_project) }
 
@@ -81,7 +80,7 @@ describe Projects::PropagateService, services: true do
 
     describe 'external tracker' do
       it 'updates the project external tracker' do
-        service_template.update(category: 'issue_tracker', default: false)
+        service_template.update!(category: 'issue_tracker', default: false)
 
         expect { described_class.propagate(service_template) }.
           to change { project.reload.has_external_issue_tracker }.to(true)
@@ -90,7 +89,7 @@ describe Projects::PropagateService, services: true do
 
     describe 'external wiki' do
       it 'updates the project external tracker' do
-        service_template.update(type: 'ExternalWikiService')
+        service_template.update!(type: 'ExternalWikiService')
 
         expect { described_class.propagate(service_template) }.
           to change { project.reload.has_external_wiki }.to(true)
diff --git a/spec/workers/propagate_project_service_worker_spec.rb b/spec/workers/propagate_service_template_worker_spec.rb
similarity index 79%
rename from spec/workers/propagate_project_service_worker_spec.rb
rename to spec/workers/propagate_service_template_worker_spec.rb
index 4c7edbfcd3e..7040d5ef81c 100644
--- a/spec/workers/propagate_project_service_worker_spec.rb
+++ b/spec/workers/propagate_service_template_worker_spec.rb
@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-describe PropagateProjectServiceWorker do
+describe PropagateServiceTemplateWorker do
   let!(:service_template) do
     PushoverService.create(
       template: true,
@@ -21,7 +21,7 @@ describe PropagateProjectServiceWorker do
 
   describe '#perform' do
     it 'calls the propagate service with the template' do
-      expect(Projects::PropagateService).to receive(:propagate).with(service_template)
+      expect(Projects::PropagateServiceTemplate).to receive(:propagate).with(service_template)
 
       subject.perform(service_template.id)
     end

From f15466bd5bd2ce5390e392785d7c750c176acbec Mon Sep 17 00:00:00 2001
From: James Lopez <james@jameslopez.es>
Date: Fri, 5 May 2017 18:18:39 +0200
Subject: [PATCH 17/18] refactor code based on feedback

---
 app/controllers/admin/services_controller.rb        | 2 +-
 app/services/projects/propagate_service_template.rb | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/controllers/admin/services_controller.rb b/app/controllers/admin/services_controller.rb
index a40ce3c2418..4c3d336b3af 100644
--- a/app/controllers/admin/services_controller.rb
+++ b/app/controllers/admin/services_controller.rb
@@ -16,7 +16,7 @@ class Admin::ServicesController < Admin::ApplicationController
 
   def update
     if service.update_attributes(service_params[:service])
-      PropagateServiceTemplateWorker.perform_async(service.id) if  service.active?
+      PropagateServiceTemplateWorker.perform_async(service.id) if service.active?
 
       redirect_to admin_application_settings_services_path,
         notice: 'Application settings saved successfully'
diff --git a/app/services/projects/propagate_service_template.rb b/app/services/projects/propagate_service_template.rb
index 32ad68673ac..2999e1af385 100644
--- a/app/services/projects/propagate_service_template.rb
+++ b/app/services/projects/propagate_service_template.rb
@@ -11,7 +11,7 @@ module Projects
     end
 
     def propagate
-      return unless @template&.active
+      return unless @template&.active?
 
       Rails.logger.info("Propagating services for template #{@template.id}")
 

From 856a511b4804a0b78294a29bbba86ac111d960f8 Mon Sep 17 00:00:00 2001
From: James Lopez <james@jameslopez.es>
Date: Fri, 5 May 2017 18:57:52 +0200
Subject: [PATCH 18/18] refactor code based on feedback

---
 .../projects/propagate_service_template.rb     | 12 ++++++------
 .../propagate_service_template_worker.rb       |  2 --
 .../propagate_service_template_spec.rb         | 18 +++++++++++-------
 3 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/app/services/projects/propagate_service_template.rb b/app/services/projects/propagate_service_template.rb
index 2999e1af385..a8ef2108492 100644
--- a/app/services/projects/propagate_service_template.rb
+++ b/app/services/projects/propagate_service_template.rb
@@ -11,7 +11,7 @@ module Projects
     end
 
     def propagate
-      return unless @template&.active?
+      return unless @template.active?
 
       Rails.logger.info("Propagating services for template #{@template.id}")
 
@@ -32,11 +32,11 @@ module Projects
 
     def bulk_create_from_template(batch)
       service_list = batch.map do |project_id|
-        service_hash.merge('project_id' => project_id).values
+        service_hash.values << project_id
       end
 
       Project.transaction do
-        bulk_insert_services(service_hash.keys + ['project_id'], service_list)
+        bulk_insert_services(service_hash.keys << 'project_id', service_list)
         run_callbacks(batch)
       end
     end
@@ -75,9 +75,9 @@ module Projects
 
           template_hash.each_with_object({}) do |(key, value), service_hash|
             value = value.is_a?(Hash) ? value.to_json : value
-            key = Gitlab::Database.postgresql? ? "\"#{key}\"" : "`#{key}`"
 
-            service_hash[key] = ActiveRecord::Base.sanitize(value)
+            service_hash[ActiveRecord::Base.connection.quote_column_name(key)] =
+              ActiveRecord::Base.sanitize(value)
           end
         end
     end
@@ -93,7 +93,7 @@ module Projects
     end
 
     def active_external_issue_tracker?
-      @template.category == :issue_tracker && !@template.default
+      @template.issue_tracker? && !@template.default
     end
 
     def active_external_wiki?
diff --git a/app/workers/propagate_service_template_worker.rb b/app/workers/propagate_service_template_worker.rb
index f1fc7ccb955..5ce0e0405d0 100644
--- a/app/workers/propagate_service_template_worker.rb
+++ b/app/workers/propagate_service_template_worker.rb
@@ -3,8 +3,6 @@ class PropagateServiceTemplateWorker
   include Sidekiq::Worker
   include DedicatedSidekiqQueue
 
-  sidekiq_options retry: 3
-
   LEASE_TIMEOUT = 4.hours.to_i
 
   def perform(template_id)
diff --git a/spec/services/projects/propagate_service_template_spec.rb b/spec/services/projects/propagate_service_template_spec.rb
index 331fb3c5ac5..90eff3bbc1e 100644
--- a/spec/services/projects/propagate_service_template_spec.rb
+++ b/spec/services/projects/propagate_service_template_spec.rb
@@ -18,8 +18,11 @@ describe Projects::PropagateServiceTemplate, services: true do
     let!(:project) { create(:empty_project) }
 
     it 'creates services for projects' do
-      expect { described_class.propagate(service_template) }.
-        to change { Service.count }.by(1)
+      expect(project.pushover_service).to be_nil
+
+      described_class.propagate(service_template)
+
+      expect(project.reload.pushover_service).to be_present
     end
 
     it 'creates services for a project that has another service' do
@@ -35,8 +38,11 @@ describe Projects::PropagateServiceTemplate, services: true do
         }
       )
 
-      expect { described_class.propagate(service_template) }.
-        to change { Service.count }.by(1)
+      expect(project.pushover_service).to be_nil
+
+      described_class.propagate(service_template)
+
+      expect(project.reload.pushover_service).to be_present
     end
 
     it 'does not create the service if it exists already' do
@@ -61,9 +67,7 @@ describe Projects::PropagateServiceTemplate, services: true do
     it 'creates the service containing the template attributes' do
       described_class.propagate(service_template)
 
-      service = Service.find_by!(type: service_template.type, template: false)
-
-      expect(service.properties).to eq(service_template.properties)
+      expect(project.pushover_service.properties).to eq(service_template.properties)
     end
 
     describe 'bulk update' do