diff options
| -rw-r--r-- | CHANGELOG | 1 | ||||
| -rw-r--r-- | app/controllers/admin/services_controller.rb | 15 | ||||
| -rw-r--r-- | app/controllers/concerns/service_params.rb | 35 | ||||
| -rw-r--r-- | app/controllers/projects/services_controller.rb | 27 | ||||
| -rw-r--r-- | app/helpers/services_helper.rb | 25 | ||||
| -rw-r--r-- | app/models/project_services/slack_service.rb | 51 | ||||
| -rw-r--r-- | app/models/service.rb | 12 | ||||
| -rw-r--r-- | app/views/projects/services/_form.html.haml | 1 | ||||
| -rw-r--r-- | app/views/shared/_service_settings.html.haml | 81 | ||||
| -rw-r--r-- | doc/integration/slack.md | 5 | ||||
| -rw-r--r-- | doc/project_services/img/slack_configuration.png | bin | 0 -> 72072 bytes | |||
| -rw-r--r-- | doc/project_services/project_services.md | 2 | ||||
| -rw-r--r-- | doc/project_services/slack.md | 26 | ||||
| -rw-r--r-- | features/steps/admin/settings.rb | 16 | ||||
| -rw-r--r-- | spec/features/projects/slack_service/slack_service_spec.rb | 26 | ||||
| -rw-r--r-- | spec/models/project_services/slack_service_spec.rb | 71 |
16 files changed, 276 insertions, 118 deletions
diff --git a/CHANGELOG b/CHANGELOG index d43901e2bb3..40bb4dd780a 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -103,6 +103,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..7c37f3155da 100644 --- a/app/controllers/admin/services_controller.rb +++ b/app/controllers/admin/services_controller.rb @@ -1,4 +1,6 @@ class Admin::ServicesController < Admin::ApplicationController + include ServiceParams + before_action :service, only: [:edit, :update] def index @@ -13,7 +15,7 @@ class Admin::ServicesController < Admin::ApplicationController end def update - if service.update_attributes(application_services_params[:service]) + if service.update_attributes(service_params[:service]) redirect_to admin_application_settings_services_path, notice: 'Application settings saved successfully' else @@ -37,15 +39,4 @@ class Admin::ServicesController < Admin::ApplicationController def service @service ||= Service.where(id: params[:id], template: true).first 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 diff --git a/app/controllers/concerns/service_params.rb b/app/controllers/concerns/service_params.rb new file mode 100644 index 00000000000..471d15af913 --- /dev/null +++ b/app/controllers/concerns/service_params.rb @@ -0,0 +1,35 @@ +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 diff --git a/app/controllers/projects/services_controller.rb b/app/controllers/projects/services_controller.rb index 1b91882048e..6a227d85f6f 100644 --- a/app/controllers/projects/services_controller.rb +++ b/app/controllers/projects/services_controller.rb @@ -1,20 +1,5 @@ class Projects::ServicesController < Projects::ApplicationController - 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] + include ServiceParams # Authorize before_action :authorize_admin_project! @@ -33,7 +18,7 @@ class Projects::ServicesController < Projects::ApplicationController end def update - if @service.update_attributes(service_params) + if @service.update_attributes(service_params[:service]) redirect_to( edit_namespace_project_service_path(@project.namespace, @project, @service.to_param, notice: @@ -64,12 +49,4 @@ class Projects::ServicesController < Projects::ApplicationController def service @service ||= @project.services.find { |service| service.to_param == params[:id] } 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 diff --git a/app/helpers/services_helper.rb b/app/helpers/services_helper.rb new file mode 100644 index 00000000000..2dd0bf5d71e --- /dev/null +++ b/app/helpers/services_helper.rb @@ -0,0 +1,25 @@ +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 diff --git a/app/models/project_services/slack_service.rb b/app/models/project_services/slack_service.rb index cf9e4d5a8b6..abbc780dc1a 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,35 @@ class SlackService < Service 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 + 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 project.name_with_namespace.gsub(/\s/, '') end diff --git a/app/models/service.rb b/app/models/service.rb index 4821096379c..a8e1cc2f422 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -82,6 +82,18 @@ class Service < ActiveRecord::Base Gitlab::PushDataBuilder.build_sample(project, user) end + def event_channel_names + [] + end + + def event_field(event) + nil + end + + def global_fields + fields + end + def supported_events %w(push tag_push issue merge_request wiki_page) end diff --git a/app/views/projects/services/_form.html.haml b/app/views/projects/services/_form.html.haml index 166dc4a01fc..752fbc21a11 100644 --- a/app/views/projects/services/_form.html.haml +++ b/app/views/projects/services/_form.html.haml @@ -8,6 +8,7 @@ .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 + = form.submit 'Save changes', class: 'btn btn-save' - if @service.valid? && @service.activated? diff --git a/app/views/shared/_service_settings.html.haml b/app/views/shared/_service_settings.html.haml index 4eaf7c2a025..5254d265918 100644 --- a/app/views/shared/_service_settings.html.haml +++ b/app/views/shared/_service_settings.html.haml @@ -10,69 +10,28 @@ .col-sm-10 = form.check_box :active -- if @service.supported_events.length > 1 - .form-group - = form.label :url, "Trigger", class: 'control-label' - .col-sm-10 - - if @service.supported_events.include?("push") - %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 - = form.check_box :wiki_page_events, class: 'pull-left' - .prepend-left-20 - = form.label :wiki_page_events, class: 'list-label' do - %strong Wiki Page events - %p.light - This url will be triggered when a wiki page is created/updated +.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 + + - 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] - if type == 'fieldset' diff --git a/doc/integration/slack.md b/doc/integration/slack.md index f6ba80f46d5..11f956fed3e 100644 --- a/doc/integration/slack.md +++ b/doc/integration/slack.md @@ -26,14 +26,13 @@ After Slack is ready we need to setup GitLab. Here are the steps to achieve this 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 - Webhook: Paste the Webhook URL from the step above - 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 - + 1. Save your settings Have fun :) diff --git a/doc/project_services/img/slack_configuration.png b/doc/project_services/img/slack_configuration.png Binary files differnew file mode 100644 index 00000000000..d3ebe5969af --- /dev/null +++ b/doc/project_services/img/slack_configuration.png diff --git a/doc/project_services/project_services.md b/doc/project_services/project_services.md index e15d5db3253..4442b7c1742 100644 --- a/doc/project_services/project_services.md +++ b/doc/project_services/project_services.md @@ -45,7 +45,7 @@ further configuration instructions and details. Contributions are welcome. | 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 | | [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 diff --git a/doc/project_services/slack.md b/doc/project_services/slack.md new file mode 100644 index 00000000000..4ed33838f7c --- /dev/null +++ b/doc/project_services/slack.md @@ -0,0 +1,26 @@ +# 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. + + + + + +| 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.| + + 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/features/projects/slack_service/slack_service_spec.rb b/spec/features/projects/slack_service/slack_service_spec.rb new file mode 100644 index 00000000000..16541f51d98 --- /dev/null +++ b/spec/features/projects/slack_service/slack_service_spec.rb @@ -0,0 +1,26 @@ +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 diff --git a/spec/models/project_services/slack_service_spec.rb b/spec/models/project_services/slack_service_spec.rb index 155f3e74e0d..df511b1bc4c 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 "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 describe "Note events" do |
