diff options
| -rw-r--r-- | CHANGELOG | 1 | ||||
| -rw-r--r-- | app/controllers/admin/services_controller.rb | 7 | ||||
| -rw-r--r-- | app/controllers/projects/services_controller.rb | 7 | ||||
| -rw-r--r-- | app/helpers/services_helper.rb | 25 | ||||
| -rw-r--r-- | app/models/project_services/slack_service.rb | 48 | ||||
| -rw-r--r-- | app/views/admin/services/_form.html.haml | 5 | ||||
| -rw-r--r-- | app/views/projects/services/_form.html.haml | 6 | ||||
| -rw-r--r-- | app/views/projects/services/slack/_service_settings.html.haml | 34 | ||||
| -rw-r--r-- | features/steps/admin/settings.rb | 16 | ||||
| -rw-r--r-- | spec/models/project_services/slack_service_spec.rb | 71 |
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' - 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 |