Allow slack service to send messages on different channels

10 files changed, 199 insertions, 21 deletions

diff --git a/CHANGELOG b/CHANGELOG
index d60d25d9ac4..53553d25165 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -101,6 +101,7 @@ v 8.10.0 (unreleased)
   - Don't render discussion notes when requesting diff tab through AJAX
   - Add basic system information like memory and disk usage to the admin panel
   - Don't garbage collect commits that have related DB records like comments
+  - Allow to setup event by channel on slack service
   - More descriptive message for git hooks and file locks
   - Aliases of award emoji should be stored as original name. !5060 (dixpac)
   - Handle custom Git hook result in GitLab UI
diff --git a/app/controllers/admin/services_controller.rb b/app/controllers/admin/services_controller.rb
index 46133588332..40938986a92 100644
--- a/app/controllers/admin/services_controller.rb
+++ b/app/controllers/admin/services_controller.rb
@@ -39,11 +39,14 @@ class Admin::ServicesController < Admin::ApplicationController
   end
 
   def application_services_params
+    dynamic_params = []
+    dynamic_params.concat(@service.event_channel_names) if @service.is_a?(SlackService)
+
     application_services_params = params.permit(:id,
-      service: Projects::ServicesController::ALLOWED_PARAMS)
+      service: Projects::ServicesController::ALLOWED_PARAMS + dynamic_params)
     if application_services_params[:service].is_a?(Hash)
       Projects::ServicesController::FILTER_BLANK_PARAMS.each do |param|
-        application_services_params[:service].delete(param) if application_services_params[:service][param].blank? 
+        application_services_params[:service].delete(param) if application_services_params[:service][param].blank?
       end
     end
     application_services_params
diff --git a/app/controllers/projects/services_controller.rb b/app/controllers/projects/services_controller.rb
index 1b91882048e..80553e035f0 100644
--- a/app/controllers/projects/services_controller.rb
+++ b/app/controllers/projects/services_controller.rb
@@ -66,10 +66,15 @@ class Projects::ServicesController < Projects::ApplicationController
   end
 
   def service_params
-    service_params = params.require(:service).permit(ALLOWED_PARAMS)
+    dynamic_params = []
+    dynamic_params.concat(@service.event_channel_names) if @service.is_a?(SlackService)
+
+    service_params = params.require(:service).permit(ALLOWED_PARAMS + dynamic_params)
+
     FILTER_BLANK_PARAMS.each do |param|
       service_params.delete(param) if service_params[param].blank?
     end
+
     service_params
   end
 end
diff --git a/app/helpers/services_helper.rb b/app/helpers/services_helper.rb
new file mode 100644
index 00000000000..98753ab2c93
--- /dev/null
+++ b/app/helpers/services_helper.rb
@@ -0,0 +1,25 @@
+module ServicesHelper
+  def service_event_description(event)
+    case event
+    when "push"
+      "Webhook will triggered by a push to the repository"
+    when "tag_push"
+      "Webhook will be triggered when a new tag is pushed to the repository"
+    when "note"
+      "Webhook will be triggered when someone adds a comment"
+    when "issue"
+      "Webhook will be triggered when an issue is created/updated/merged"
+    when "merge_request"
+      "Webhook will be triggered when a merge request is created/updated/merged"
+    when "build"
+      "Webhook will be triggered when a build status changes"
+    when "wiki_page"
+      "Webhook will be triggered when a wiki page is created/updated"
+    end
+  end
+
+  def service_event_field_name(event)
+    event = event.pluralize if %w(merge_request issue).include?(event)
+    "#{event}_events"
+  end
+end
diff --git a/app/models/project_services/slack_service.rb b/app/models/project_services/slack_service.rb
index cf9e4d5a8b6..a1f146ac05a 100644
--- a/app/models/project_services/slack_service.rb
+++ b/app/models/project_services/slack_service.rb
@@ -4,6 +4,9 @@ class SlackService < Service
   validates :webhook, presence: true, url: true, if: :activated?
 
   def initialize_properties
+    # Custom serialized properties initialization
+    self.supported_events.each { |event| self.class.prop_accessor event_channel_name(event) }
+
     if properties.nil?
       self.properties = {}
       self.notify_only_broken_builds = true
@@ -29,13 +32,15 @@ class SlackService < Service
   end
 
   def fields
-    [
-      { type: 'text', name: 'webhook',
-        placeholder: 'https://hooks.slack.com/services/...' },
-      { type: 'text', name: 'username', placeholder: 'username' },
-      { type: 'text', name: 'channel', placeholder: '#channel' },
-      { type: 'checkbox', name: 'notify_only_broken_builds' },
-    ]
+    default_fields =
+      [
+        { type: 'text', name: 'webhook',   placeholder: 'https://hooks.slack.com/services/...' },
+        { type: 'text', name: 'username', placeholder: 'username' },
+        { type: 'text', name: 'channel', placeholder: "#General" },
+        { type: 'checkbox', name: 'notify_only_broken_builds' },
+      ]
+
+    default_fields + build_event_channels
   end
 
   def supported_events
@@ -74,7 +79,10 @@ class SlackService < Service
       end
 
     opt = {}
-    opt[:channel] = channel if channel
+
+    event_channel = get_channel_field(object_kind) || channel
+
+    opt[:channel] = event_channel if event_channel
     opt[:username] = username if username
 
     if message
@@ -83,8 +91,32 @@ class SlackService < Service
     end
   end
 
+  def event_channel_names
+    supported_events.map { |event| event_channel_name(event) }
+  end
+
   private
 
+  def get_channel_field(event)
+    field_name = event_channel_name(event)
+    self.send(field_name)
+  end
+
+  def build_event_channels
+    channels = []
+
+    supported_events.each do |event|
+      channel_name = event_channel_name(event)
+      channels << { type: 'text', name: channel_name, placeholder: "#General" }
+    end
+
+    channels
+  end
+
+  def event_channel_name(event)
+    "#{event}_channel"
+  end
+
   def project_name
     project.name_with_namespace.gsub(/\s/, '')
   end
diff --git a/app/views/admin/services/_form.html.haml b/app/views/admin/services/_form.html.haml
index cdbfc60f9a4..c6f8fc61a04 100644
--- a/app/views/admin/services/_form.html.haml
+++ b/app/views/admin/services/_form.html.haml
@@ -4,7 +4,10 @@
 %p #{@service.description} template
 
 = form_for :service, url: admin_application_settings_service_path, method: :put, html: { class: 'form-horizontal fieldset-form' } do |form|
-  = render 'shared/service_settings', form: form
+  - if @service.is_a?(SlackService)
+    = render 'projects/services/slack/service_settings', form: form
+  - else
+    = render 'shared/service_settings', form: form
 
   .form-actions
     = form.submit 'Save', class: 'btn btn-save'
diff --git a/app/views/projects/services/_form.html.haml b/app/views/projects/services/_form.html.haml
index 166dc4a01fc..abad7954db5 100644
--- a/app/views/projects/services/_form.html.haml
+++ b/app/views/projects/services/_form.html.haml
@@ -7,7 +7,11 @@
     %p= @service.description
   .col-lg-9
     = form_for(@service, as: :service, url: namespace_project_service_path(@project.namespace, @project, @service.to_param), method: :put, html: { class: 'form-horizontal' }) do |form|
-      = render 'shared/service_settings', form: form
+      - if @service.is_a?(SlackService)
+        = render 'projects/services/slack/service_settings', form: form
+      - else
+        = render 'shared/service_settings', form: form
+
       = form.submit 'Save changes', class: 'btn btn-save'
       &nbsp;
       - if @service.valid? && @service.activated?
diff --git a/app/views/projects/services/slack/_service_settings.html.haml b/app/views/projects/services/slack/_service_settings.html.haml
new file mode 100644
index 00000000000..12f4c2e45b9
--- /dev/null
+++ b/app/views/projects/services/slack/_service_settings.html.haml
@@ -0,0 +1,34 @@
+= form_errors(@service)
+
+- if @service.help.present?
+  .well
+    = preserve do
+      = markdown @service.help
+
+.form-group
+  = form.label :active, "Active", class: "control-label"
+  .col-sm-10
+    = form.check_box :active
+
+.form-group
+  = form.label :url, "Trigger", class: 'control-label'
+
+  .col-sm-10
+    - @service.supported_events.each do |event|
+      %div
+        = form.check_box service_event_field_name(event), class: 'pull-left'
+        .prepend-left-20
+          = form.label service_event_field_name(event), class: 'list-label' do
+            %strong
+              = event.humanize
+
+      %p
+        - field = @service.fields.select{ |field| field[:name] == "#{event}_channel"}.first
+        = form.text_field field[:name], class: "form-control", placeholder: field[:placeholder]
+
+      %p.light
+        = service_event_description(event)
+
+- @service.fields.each do |field|
+  - if %w(webhook username notify_only_broken_builds).include?(field[:name])
+    = render 'shared/field', form: form, field: field
diff --git a/features/steps/admin/settings.rb b/features/steps/admin/settings.rb
index 037f7494a77..03f87df7a60 100644
--- a/features/steps/admin/settings.rb
+++ b/features/steps/admin/settings.rb
@@ -27,19 +27,19 @@ class Spinach::Features::AdminSettings < Spinach::FeatureSteps
 
   step 'I check all events and submit form' do
     page.check('Active')
-    page.check('Push events')
-    page.check('Tag push events')
-    page.check('Comments')
-    page.check('Issues events')
-    page.check('Merge Request events')
-    page.check('Build events')
+    page.check('Push')
+    page.check('Tag push')
+    page.check('Note')
+    page.check('Issue')
+    page.check('Merge request')
+    page.check('Build')
     click_on 'Save'
   end
 
   step 'I fill out Slack settings' do
     fill_in 'Webhook', with: 'http://localhost'
     fill_in 'Username', with: 'test_user'
-    fill_in 'Channel', with: '#test_channel'
+    fill_in 'service_push_channel', with: '#test_channel'
     page.check('Notify only broken builds')
   end
 
@@ -56,6 +56,6 @@ class Spinach::Features::AdminSettings < Spinach::FeatureSteps
   step 'I should see Slack settings saved' do
     expect(find_field('Webhook').value).to eq 'http://localhost'
     expect(find_field('Username').value).to eq 'test_user'
-    expect(find_field('Channel').value).to eq '#test_channel'
+    expect(find('#service_push_channel').value).to eq '#test_channel'
   end
 end
diff --git a/spec/models/project_services/slack_service_spec.rb b/spec/models/project_services/slack_service_spec.rb
index 155f3e74e0d..1beb7c63b0b 100644
--- a/spec/models/project_services/slack_service_spec.rb
+++ b/spec/models/project_services/slack_service_spec.rb
@@ -124,6 +124,7 @@ describe SlackService, models: true do
        and_return(
          double(:slack_service).as_null_object
        )
+
       slack.execute(push_sample_data)
     end
 
@@ -136,6 +137,76 @@ describe SlackService, models: true do
         )
       slack.execute(push_sample_data)
     end
+
+    context "event channels" do
+      it "should user the right channel for push event" do
+        slack.update_attributes(push_channel: "random")
+
+        expect(Slack::Notifier).to receive(:new).
+         with(webhook_url, channel: "random").
+         and_return(
+           double(:slack_service).as_null_object
+         )
+
+        slack.execute(push_sample_data)
+      end
+
+      it "should use the right channel for merge request event" do
+        slack.update_attributes(merge_request_channel: "random")
+
+        expect(Slack::Notifier).to receive(:new).
+         with(webhook_url, channel: "random").
+         and_return(
+           double(:slack_service).as_null_object
+         )
+
+        slack.execute(@merge_sample_data)
+      end
+
+      it "should use the right channel for issue event" do
+        slack.update_attributes(issue_channel: "random")
+
+        expect(Slack::Notifier).to receive(:new).
+         with(webhook_url, channel: "random").
+         and_return(
+           double(:slack_service).as_null_object
+         )
+
+        slack.execute(@issues_sample_data)
+      end
+
+      it "should use the right channel for wiki event" do
+        slack.update_attributes(wiki_page_channel: "random")
+
+        expect(Slack::Notifier).to receive(:new).
+         with(webhook_url, channel: "random").
+         and_return(
+           double(:slack_service).as_null_object
+         )
+
+        slack.execute(@wiki_page_sample_data)
+      end
+
+      context "note event" do
+        let(:issue_note) do
+          create(:note_on_issue, project: project, note: "issue note")
+        end
+
+        it "should use the right channel" do
+          slack.update_attributes(note_channel: "random")
+
+          note_data = Gitlab::NoteDataBuilder.build(issue_note, user)
+
+          expect(Slack::Notifier).to receive(:new).
+           with(webhook_url, channel: "random").
+           and_return(
+             double(:slack_service).as_null_object
+           )
+
+          slack.execute(note_data)
+        end
+      end
+    end
   end
 
   describe "Note events" do