Commit a3d3d8e9 authored by Rémy Coutable's avatar Rémy Coutable

Merge branch 'issue_8110' into 'master'

Allow slack service to send messages on different channels

closes #8110 

## Allow slack service to send messages on different channels  

![new_slack_service](/uploads/87de0bd6b02a4f7853358676b5e74dff/new_slack_service.png)

## Does this MR meet the acceptance criteria?

- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- [x] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)
- [x] API support added
- Tests
  - [x] Added for this feature/bug
  - [x] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)

See merge request !5124
parent a8d9b22e
...@@ -103,6 +103,7 @@ v 8.10.0 (unreleased) ...@@ -103,6 +103,7 @@ v 8.10.0 (unreleased)
- Don't render discussion notes when requesting diff tab through AJAX - Don't render discussion notes when requesting diff tab through AJAX
- Add basic system information like memory and disk usage to the admin panel - 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 - 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 - More descriptive message for git hooks and file locks
- Aliases of award emoji should be stored as original name. !5060 (dixpac) - Aliases of award emoji should be stored as original name. !5060 (dixpac)
- Handle custom Git hook result in GitLab UI - Handle custom Git hook result in GitLab UI
......
class Admin::ServicesController < Admin::ApplicationController class Admin::ServicesController < Admin::ApplicationController
include ServiceParams
before_action :service, only: [:edit, :update] before_action :service, only: [:edit, :update]
def index def index
...@@ -13,7 +15,7 @@ class Admin::ServicesController < Admin::ApplicationController ...@@ -13,7 +15,7 @@ class Admin::ServicesController < Admin::ApplicationController
end end
def update def update
if service.update_attributes(application_services_params[:service]) if service.update_attributes(service_params[:service])
redirect_to admin_application_settings_services_path, redirect_to admin_application_settings_services_path,
notice: 'Application settings saved successfully' notice: 'Application settings saved successfully'
else else
...@@ -37,15 +39,4 @@ class Admin::ServicesController < Admin::ApplicationController ...@@ -37,15 +39,4 @@ class Admin::ServicesController < Admin::ApplicationController
def service def service
@service ||= Service.where(id: params[:id], template: true).first @service ||= Service.where(id: params[:id], template: true).first
end end
def application_services_params
application_services_params = params.permit(:id,
service: Projects::ServicesController::ALLOWED_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?
end
end
application_services_params
end
end end
module ServiceParams
extend ActiveSupport::Concern
ALLOWED_PARAMS = [:title, :token, :type, :active, :api_key, :api_url, :api_version, :subdomain,
:room, :recipients, :project_url, :webhook,
:user_key, :device, :priority, :sound, :bamboo_url, :username, :password,
: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,
:server_host, :server_port, :default_irc_uri, :enable_ssl_verification,
:jira_issue_transition_id]
# Parameters to ignore if no value is specified
FILTER_BLANK_PARAMS = [:password]
def service_params
dynamic_params = []
dynamic_params.concat(@service.event_channel_names)
service_params = params.permit(:id, service: ALLOWED_PARAMS + dynamic_params)
if service_params[:service].is_a?(Hash)
FILTER_BLANK_PARAMS.each do |param|
service_params[:service].delete(param) if service_params[:service][param].blank?
end
end
service_params
end
end
class Projects::ServicesController < Projects::ApplicationController class Projects::ServicesController < Projects::ApplicationController
ALLOWED_PARAMS = [:title, :token, :type, :active, :api_key, :api_url, :api_version, :subdomain, include ServiceParams
:room, :recipients, :project_url, :webhook,
:user_key, :device, :priority, :sound, :bamboo_url, :username, :password,
: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,
:server_host, :server_port, :default_irc_uri, :enable_ssl_verification,
:jira_issue_transition_id]
# Parameters to ignore if no value is specified
FILTER_BLANK_PARAMS = [:password]
# Authorize # Authorize
before_action :authorize_admin_project! before_action :authorize_admin_project!
...@@ -33,7 +18,7 @@ class Projects::ServicesController < Projects::ApplicationController ...@@ -33,7 +18,7 @@ class Projects::ServicesController < Projects::ApplicationController
end end
def update def update
if @service.update_attributes(service_params) if @service.update_attributes(service_params[:service])
redirect_to( redirect_to(
edit_namespace_project_service_path(@project.namespace, @project, edit_namespace_project_service_path(@project.namespace, @project,
@service.to_param, notice: @service.to_param, notice:
...@@ -64,12 +49,4 @@ class Projects::ServicesController < Projects::ApplicationController ...@@ -64,12 +49,4 @@ class Projects::ServicesController < Projects::ApplicationController
def service def service
@service ||= @project.services.find { |service| service.to_param == params[:id] } @service ||= @project.services.find { |service| service.to_param == params[:id] }
end end
def service_params
service_params = params.require(:service).permit(ALLOWED_PARAMS)
FILTER_BLANK_PARAMS.each do |param|
service_params.delete(param) if service_params[param].blank?
end
service_params
end
end end
module ServicesHelper
def service_event_description(event)
case event
when "push"
"Event will be triggered by a push to the repository"
when "tag_push"
"Event will be triggered when a new tag is pushed to the repository"
when "note"
"Event will be triggered when someone adds a comment"
when "issue"
"Event will be triggered when an issue is created/updated/merged"
when "merge_request"
"Event will be triggered when a merge request is created/updated/merged"
when "build"
"Event will be triggered when a build status changes"
when "wiki_page"
"Event 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
...@@ -4,6 +4,9 @@ class SlackService < Service ...@@ -4,6 +4,9 @@ class SlackService < Service
validates :webhook, presence: true, url: true, if: :activated? validates :webhook, presence: true, url: true, if: :activated?
def initialize_properties def initialize_properties
# Custom serialized properties initialization
self.supported_events.each { |event| self.class.prop_accessor(event_channel_name(event)) }
if properties.nil? if properties.nil?
self.properties = {} self.properties = {}
self.notify_only_broken_builds = true self.notify_only_broken_builds = true
...@@ -29,13 +32,15 @@ class SlackService < Service ...@@ -29,13 +32,15 @@ class SlackService < Service
end end
def fields def fields
default_fields =
[ [
{ type: 'text', name: 'webhook', { type: 'text', name: 'webhook', placeholder: 'https://hooks.slack.com/services/...' },
placeholder: 'https://hooks.slack.com/services/...' },
{ type: 'text', name: 'username', placeholder: 'username' }, { type: 'text', name: 'username', placeholder: 'username' },
{ type: 'text', name: 'channel', placeholder: '#channel' }, { type: 'text', name: 'channel', placeholder: "#general" },
{ type: 'checkbox', name: 'notify_only_broken_builds' }, { type: 'checkbox', name: 'notify_only_broken_builds' },
] ]
default_fields + build_event_channels
end end
def supported_events def supported_events
...@@ -74,7 +79,10 @@ class SlackService < Service ...@@ -74,7 +79,10 @@ class SlackService < Service
end end
opt = {} 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 opt[:username] = username if username
if message if message
...@@ -83,8 +91,35 @@ class SlackService < Service ...@@ -83,8 +91,35 @@ class SlackService < Service
end end
end end
def event_channel_names
supported_events.map { |event| event_channel_name(event) }
end
def event_field(event)
fields.find { |field| field[:name] == event_channel_name(event) }
end
def global_fields
fields.reject { |field| field[:name].end_with?('channel') }
end
private private
def get_channel_field(event)
field_name = event_channel_name(event)
self.public_send(field_name)
end
def build_event_channels
supported_events.reduce([]) do |channels, event|
channels << { type: 'text', name: event_channel_name(event), placeholder: "#general" }
end
end
def event_channel_name(event)
"#{event}_channel"
end
def project_name def project_name
project.name_with_namespace.gsub(/\s/, '') project.name_with_namespace.gsub(/\s/, '')
end end
......
...@@ -82,6 +82,18 @@ class Service < ActiveRecord::Base ...@@ -82,6 +82,18 @@ class Service < ActiveRecord::Base
Gitlab::PushDataBuilder.build_sample(project, user) Gitlab::PushDataBuilder.build_sample(project, user)
end end
def event_channel_names
[]
end
def event_field(event)
nil
end
def global_fields
fields
end
def supported_events def supported_events
%w(push tag_push issue merge_request wiki_page) %w(push tag_push issue merge_request wiki_page)
end end
......
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
.col-lg-9 .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| = 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 = render 'shared/service_settings', form: form
= form.submit 'Save changes', class: 'btn btn-save' = form.submit 'Save changes', class: 'btn btn-save'
&nbsp; &nbsp;
- if @service.valid? && @service.activated? - if @service.valid? && @service.activated?
......
...@@ -10,69 +10,28 @@ ...@@ -10,69 +10,28 @@
.col-sm-10 .col-sm-10
= form.check_box :active = form.check_box :active
- if @service.supported_events.length > 1 .form-group
.form-group
= form.label :url, "Trigger", class: 'control-label' = form.label :url, "Trigger", class: 'control-label'
.col-sm-10 .col-sm-10
- if @service.supported_events.include?("push") - @service.supported_events.each do |event|
%div
= form.check_box :push_events, class: 'pull-left'
.prepend-left-20
= form.label :push_events, class: 'list-label' do
%strong Push events
%p.light
This url will be triggered by a push to the repository
- if @service.supported_events.include?("tag_push")
%div
= form.check_box :tag_push_events, class: 'pull-left'
.prepend-left-20
= form.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
- if @service.supported_events.include?("note")
%div
= form.check_box :note_events, class: 'pull-left'
.prepend-left-20
= form.label :note_events, class: 'list-label' do
%strong Comments
%p.light
This url will be triggered when someone adds a comment
- if @service.supported_events.include?("issue")
%div
= form.check_box :issues_events, class: 'pull-left'
.prepend-left-20
= form.label :issues_events, class: 'list-label' do
%strong Issues events
%p.light
This url will be triggered when an issue is created/updated/merged
- if @service.supported_events.include?("merge_request")
%div
= form.check_box :merge_requests_events, class: 'pull-left'
.prepend-left-20
= form.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
- if @service.supported_events.include?("build")
%div
= form.check_box :build_events, class: 'pull-left'
.prepend-left-20
= form.label :build_events, class: 'list-label' do
%strong Build events
%p.light
This url will be triggered when a build status changes
- if @service.supported_events.include?("wiki_page")
%div %div
= form.check_box :wiki_page_events, class: 'pull-left' = form.check_box service_event_field_name(event), class: 'pull-left'
.prepend-left-20 .prepend-left-20
= form.label :wiki_page_events, class: 'list-label' do = form.label service_event_field_name(event), class: 'list-label' do
%strong Wiki Page events %strong
%p.light = event.humanize
This url will be triggered when a wiki page is created/updated
- field = @service.event_field(event)
- if field
%p
= form.text_field field[:name], class: "form-control", placeholder: field[:placeholder]
%p.light
= service_event_description(event)
- @service.fields.each do |field| - @service.global_fields.each do |field|
- type = field[:type] - type = field[:type]
- if type == 'fieldset' - if type == 'fieldset'
......
...@@ -26,12 +26,11 @@ After Slack is ready we need to setup GitLab. Here are the steps to achieve this ...@@ -26,12 +26,11 @@ After Slack is ready we need to setup GitLab. Here are the steps to achieve this
1. Navigate to Settings -> Services -> Slack 1. Navigate to Settings -> Services -> Slack
1. Pick the triggers you want to activate 1. Pick the triggers you want to activate and respective channel (`#general` by default).
1. Fill in your Slack details 1. Fill in your Slack details
- Webhook: Paste the Webhook URL from the step above - Webhook: Paste the Webhook URL from the step above
- Username: Fill this in if you want to change the username of the bot - Username: Fill this in if you want to change the username of the bot
- Channel: Fill this in if you want to change the channel where the messages will be posted
- Mark it as active - Mark it as active
1. Save your settings 1. Save your settings
......
...@@ -45,7 +45,7 @@ further configuration instructions and details. Contributions are welcome. ...@@ -45,7 +45,7 @@ further configuration instructions and details. Contributions are welcome.
| PivotalTracker | Project Management Software (Source Commits Endpoint) | | PivotalTracker | Project Management Software (Source Commits Endpoint) |
| Pushover | Pushover makes it easy to get real-time notifications on your Android device, iPhone, iPad, and Desktop | | Pushover | Pushover makes it easy to get real-time notifications on your Android device, iPhone, iPad, and Desktop |
| [Redmine](redmine.md) | Redmine issue tracker | | [Redmine](redmine.md) | Redmine issue tracker |
| Slack | A team communication tool for the 21st century | | [Slack](slack.md) | A team communication tool for the 21st century |
## Services Templates ## Services Templates
......
# Slack Service
Go to your project's **Settings > Services > Slack** and you will see a checkbox with the following events that can be triggered:
* Push
* Issue
* Merge request
* Note
* Tag push
* Build
* Wiki page
Below each of these event checkboxes you will have an input to insert which Slack channel do you want to send that event message,
`#general` channel is default.
![Slack configuration](img/slack_configuration.png)
| Field | Description |
| ----- | ----------- |
| `Webhook` | The incoming webhook url which you have to setup on slack. (https://my.slack.com/services/new/incoming-webhook/) |
| `Username` | Optional username which can be on messages sent to slack. |
| `notify only broken builds` | Notify only about broken builds, when build events are marked to be sent.|
...@@ -27,19 +27,19 @@ class Spinach::Features::AdminSettings < Spinach::FeatureSteps ...@@ -27,19 +27,19 @@ class Spinach::Features::AdminSettings < Spinach::FeatureSteps
step 'I check all events and submit form' do step 'I check all events and submit form' do
page.check('Active') page.check('Active')
page.check('Push events') page.check('Push')
page.check('Tag push events') page.check('Tag push')
page.check('Comments') page.check('Note')
page.check('Issues events') page.check('Issue')
page.check('Merge Request events') page.check('Merge request')
page.check('Build events') page.check('Build')
click_on 'Save' click_on 'Save'
end end
step 'I fill out Slack settings' do step 'I fill out Slack settings' do
fill_in 'Webhook', with: 'http://localhost' fill_in 'Webhook', with: 'http://localhost'
fill_in 'Username', with: 'test_user' 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') page.check('Notify only broken builds')
end end
...@@ -56,6 +56,6 @@ class Spinach::Features::AdminSettings < Spinach::FeatureSteps ...@@ -56,6 +56,6 @@ class Spinach::Features::AdminSettings < Spinach::FeatureSteps
step 'I should see Slack settings saved' do step 'I should see Slack settings saved' do
expect(find_field('Webhook').value).to eq 'http://localhost' expect(find_field('Webhook').value).to eq 'http://localhost'
expect(find_field('Username').value).to eq 'test_user' 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
end end
require 'spec_helper'
feature 'Projects > Slack service > Setup events', feature: true do
let(:user) { create(:user) }
let(:service) { SlackService.new }
let(:project) { create(:project, slack_service: service) }
background do
service.fields
service.update_attributes(push_channel: 1, issue_channel: 2, merge_request_channel: 3, note_channel: 4, tag_push_channel: 5, build_channel: 6, wiki_page_channel: 7)
project.team << [user, :master]
login_as(user)
end
scenario 'user can filter events by channel' do
visit edit_namespace_project_service_path(project.namespace, project, service)
expect(page.find_field("service_push_channel").value).to have_content '1'
expect(page.find_field("service_issue_channel").value).to have_content '2'
expect(page.find_field("service_merge_request_channel").value).to have_content '3'
expect(page.find_field("service_note_channel").value).to have_content '4'
expect(page.find_field("service_tag_push_channel").value).to have_content '5'
expect(page.find_field("service_build_channel").value).to have_content '6'
expect(page.find_field("service_wiki_page_channel").value).to have_content '7'
end
end
...@@ -124,6 +124,7 @@ describe SlackService, models: true do ...@@ -124,6 +124,7 @@ describe SlackService, models: true do
and_return( and_return(
double(:slack_service).as_null_object double(:slack_service).as_null_object
) )
slack.execute(push_sample_data) slack.execute(push_sample_data)
end end
...@@ -136,6 +137,76 @@ describe SlackService, models: true do ...@@ -136,6 +137,76 @@ describe SlackService, models: true do
) )
slack.execute(push_sample_data) slack.execute(push_sample_data)
end end
context "event channels" do
it "uses 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 "uses 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 "uses 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 "uses 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 "uses 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 end
describe "Note events" do describe "Note events" do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment