diff options
author | Felipe Artur <felipefac@gmail.com> | 2016-12-06 15:59:03 -0200 |
---|---|---|
committer | Felipe Artur <felipefac@gmail.com> | 2016-12-15 15:32:49 -0200 |
commit | a5ccaded656fb215f1f8d503b88c8f28bf90ce68 (patch) | |
tree | f68c114323f6d345a753f0062c1633449e76db21 | |
parent | 141faaacf9119ce5d765efe73c6509030ba078cd (diff) | |
download | gitlab-ce-a5ccaded656fb215f1f8d503b88c8f28bf90ce68.tar.gz |
Change SlackService to SlackNotificationsServiceissue_22269_fix_eeissue_22269
19 files changed, 223 insertions, 168 deletions
diff --git a/app/models/project.rb b/app/models/project.rb index 19c2d24212d..5d092ca42c2 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -95,8 +95,8 @@ class Project < ActiveRecord::Base has_one :asana_service, dependent: :destroy has_one :gemnasium_service, dependent: :destroy has_one :mattermost_slash_commands_service, dependent: :destroy - has_one :mattermost_service, dependent: :destroy - has_one :slack_service, dependent: :destroy + has_one :mattermost_notification_service, dependent: :destroy + has_one :slack_notification_service, dependent: :destroy has_one :buildkite_service, dependent: :destroy has_one :bamboo_service, dependent: :destroy has_one :teamcity_service, dependent: :destroy diff --git a/app/models/project_services/chat_notification_service.rb b/app/models/project_services/chat_notification_service.rb new file mode 100644 index 00000000000..b0556987721 --- /dev/null +++ b/app/models/project_services/chat_notification_service.rb @@ -0,0 +1,147 @@ +# Base class for Chat notifications services +# This class is not meant to be used directly, but only to inherit from. +class ChatNotificationService < Service + include ChatMessage + + default_value_for :category, 'chat' + + prop_accessor :webhook, :username, :channel + boolean_accessor :notify_only_broken_builds, :notify_only_broken_pipelines + + 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 + self.notify_only_broken_pipelines = true + end + end + + def can_test? + valid? + end + + def supported_events + %w[push issue confidential_issue merge_request note tag_push + build pipeline wiki_page] + end + + def execute(data) + return unless supported_events.include?(data[:object_kind]) + return unless webhook.present? + + object_kind = data[:object_kind] + + data = data.merge( + project_url: project_url, + project_name: project_name + ) + + # WebHook events often have an 'update' event that follows a 'open' or + # 'close' action. Ignore update events for now to prevent duplicate + # messages from arriving. + + message = get_message(object_kind, data) + + return false unless message + + opt = {} + + opt[:channel] = get_channel_field(object_kind).presence || channel || default_channel + opt[:username] = username if username + notifier = Slack::Notifier.new(webhook, opt) + notifier.ping(message.pretext, attachments: message.attachments, fallback: message.fallback) + + true + 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 + + def default_channel + raise NotImplementedError + end + + private + + def get_message(object_kind, data) + case object_kind + when "push", "tag_push" + PushMessage.new(data) + when "issue" + IssueMessage.new(data) unless is_update?(data) + when "merge_request" + MergeMessage.new(data) unless is_update?(data) + when "note" + NoteMessage.new(data) + when "build" + BuildMessage.new(data) if should_build_be_notified?(data) + when "pipeline" + PipelineMessage.new(data) if should_pipeline_be_notified?(data) + when "wiki_page" + WikiPageMessage.new(data) + end + end + + 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: default_channel } + end + end + + def event_channel_name(event) + "#{event}_channel" + end + + def project_name + project.name_with_namespace.gsub(/\s/, '') + end + + def project_url + project.web_url + end + + def is_update?(data) + data[:object_attributes][:action] == 'update' + end + + def should_build_be_notified?(data) + case data[:commit][:status] + when 'success' + !notify_only_broken_builds? + when 'failed' + true + else + false + end + end + + def should_pipeline_be_notified?(data) + case data[:object_attributes][:status] + when 'success' + !notify_only_broken_pipelines? + when 'failed' + true + else + false + end + end +end diff --git a/app/models/project_services/chat_service.rb b/app/models/project_services/chat_service.rb index 8ac049ba939..574788462de 100644 --- a/app/models/project_services/chat_service.rb +++ b/app/models/project_services/chat_service.rb @@ -1,148 +1,21 @@ # Base class for Chat services -# This class is not meant to be used directly, but only to inherrit from. +# This class is not meant to be used directly, but only to inherit from. class ChatService < Service - include ChatMessage - default_value_for :category, 'chat' - prop_accessor :webhook, :username, :channel - boolean_accessor :notify_only_broken_builds, :notify_only_broken_pipelines - - 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)) } + has_many :chat_names, foreign_key: :service_id - if properties.nil? - self.properties = {} - self.notify_only_broken_builds = true - self.notify_only_broken_pipelines = true - end - end - - def can_test? - valid? + def valid_token?(token) + self.respond_to?(:token) && + self.token.present? && + ActiveSupport::SecurityUtils.variable_size_secure_compare(token, self.token) end def supported_events - %w[push issue confidential_issue merge_request note tag_push - build pipeline wiki_page] + [] end - def execute(data) - return unless supported_events.include?(data[:object_kind]) - return unless webhook.present? - - object_kind = data[:object_kind] - - data = data.merge( - project_url: project_url, - project_name: project_name - ) - - # WebHook events often have an 'update' event that follows a 'open' or - # 'close' action. Ignore update events for now to prevent duplicate - # messages from arriving. - - message = get_message(object_kind, data) - - return false unless message - - opt = {} - - opt[:channel] = get_channel_field(object_kind).presence || channel || default_channel - opt[:username] = username if username - - notifier = Slack::Notifier.new(webhook, opt) - notifier.ping(message.pretext, attachments: message.attachments, fallback: message.fallback) - - true - 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 - - def default_channel + def trigger(params) raise NotImplementedError end - - private - - def get_message(object_kind, data) - case object_kind - when "push", "tag_push" - PushMessage.new(data) - when "issue" - IssueMessage.new(data) unless is_update?(data) - when "merge_request" - MergeMessage.new(data) unless is_update?(data) - when "note" - NoteMessage.new(data) - when "build" - BuildMessage.new(data) if should_build_be_notified?(data) - when "pipeline" - PipelineMessage.new(data) if should_pipeline_be_notified?(data) - when "wiki_page" - WikiPageMessage.new(data) - end - end - - 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: default_channel } - end - end - - def event_channel_name(event) - "#{event}_channel" - end - - def project_name - project.name_with_namespace.gsub(/\s/, '') - end - - def project_url - project.web_url - end - - def is_update?(data) - data[:object_attributes][:action] == 'update' - end - - def should_build_be_notified?(data) - case data[:commit][:status] - when 'success' - !notify_only_broken_builds? - when 'failed' - true - else - false - end - end - - def should_pipeline_be_notified?(data) - case data[:object_attributes][:status] - when 'success' - !notify_only_broken_pipelines? - when 'failed' - true - else - false - end - end end diff --git a/app/models/project_services/mattermost_service.rb b/app/models/project_services/mattermost_notification_service.rb index 9d61c251a32..de18c4b1f00 100644 --- a/app/models/project_services/mattermost_service.rb +++ b/app/models/project_services/mattermost_notification_service.rb @@ -1,4 +1,4 @@ -class MattermostService < ChatService +class MattermostNotificationService < ChatNotificationService def title 'Mattermost notifications' end @@ -8,7 +8,7 @@ class MattermostService < ChatService end def to_param - 'mattermost' + 'mattermost_notification' end def help @@ -28,7 +28,7 @@ class MattermostService < ChatService def default_fields [ - { type: 'text', name: 'webhook', placeholder: 'http://mattermost_host/hooks/...' }, + { type: 'text', name: 'webhook', placeholder: 'http://mattermost_host/hooks/...' }, { type: 'text', name: 'username', placeholder: 'username' }, { type: 'checkbox', name: 'notify_only_broken_builds' }, { type: 'checkbox', name: 'notify_only_broken_pipelines' }, diff --git a/app/models/project_services/mattermost_slash_commands_service.rb b/app/models/project_services/mattermost_slash_commands_service.rb index 3993dfbda17..33431f41dc2 100644 --- a/app/models/project_services/mattermost_slash_commands_service.rb +++ b/app/models/project_services/mattermost_slash_commands_service.rb @@ -1,18 +1,8 @@ -class MattermostSlashCommandsService < Service +class MattermostSlashCommandsService < ChatService include TriggersHelper prop_accessor :token - def valid_token?(token) - self.respond_to?(:token) && - self.token.present? && - ActiveSupport::SecurityUtils.variable_size_secure_compare(token, self.token) - end - - def supported_events - [] - end - def can_test? false end diff --git a/app/models/project_services/slack_service.rb b/app/models/project_services/slack_notification_service.rb index 0df1743c4ba..3cbf89efba4 100644 --- a/app/models/project_services/slack_service.rb +++ b/app/models/project_services/slack_notification_service.rb @@ -1,4 +1,4 @@ -class SlackService < ChatService +class SlackNotificationService < ChatNotificationService def title 'Slack notifications' end @@ -8,7 +8,7 @@ class SlackService < ChatService end def to_param - 'slack' + 'slack_notification' end def help diff --git a/app/models/service.rb b/app/models/service.rb index 8e58f2a1925..0bbab078cf6 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -220,8 +220,8 @@ class Service < ActiveRecord::Base pivotaltracker pushover redmine - mattermost - slack + mattermost_notification + slack_notification teamcity ] end diff --git a/db/migrate/20141006143943_move_slack_service_to_webhook.rb b/db/migrate/20141006143943_move_slack_service_to_webhook.rb index 8cb120f7007..42e88d6d6e3 100644 --- a/db/migrate/20141006143943_move_slack_service_to_webhook.rb +++ b/db/migrate/20141006143943_move_slack_service_to_webhook.rb @@ -1,7 +1,11 @@ # rubocop:disable all class MoveSlackServiceToWebhook < ActiveRecord::Migration + + DOWNTIME = true + DOWNTIME_REASON = 'Move old fields "token" and "subdomain" to one single field "webhook"' + def change - SlackService.all.each do |slack_service| + SlackNotificationService.all.each do |slack_service| if ["token", "subdomain"].all? { |property| slack_service.properties.key? property } token = slack_service.properties['token'] subdomain = slack_service.properties['subdomain'] diff --git a/db/migrate/20161213172958_change_slack_service_to_slack_notification_service.rb b/db/migrate/20161213172958_change_slack_service_to_slack_notification_service.rb new file mode 100644 index 00000000000..a7278d7b5a6 --- /dev/null +++ b/db/migrate/20161213172958_change_slack_service_to_slack_notification_service.rb @@ -0,0 +1,14 @@ +class ChangeSlackServiceToSlackNotificationService < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = true + DOWNTIME_REASON = 'Rename SlackService to SlackNotificationService' + + def up + execute("UPDATE services SET type = 'SlackNotificationService' WHERE type = 'SlackService'") + end + + def down + execute("UPDATE services SET type = 'SlackService' WHERE type = 'SlackNotificationService'") + end +end diff --git a/db/schema.rb b/db/schema.rb index 4711b7873af..5bd4ef1f21b 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20161212142807) do +ActiveRecord::Schema.define(version: 20161213172958) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" diff --git a/lib/api/services.rb b/lib/api/services.rb index b1e072b4f47..59232c84c24 100644 --- a/lib/api/services.rb +++ b/lib/api/services.rb @@ -473,7 +473,7 @@ module API desc: 'The description of the tracker' } ], - 'slack' => [ + 'slack-notification' => [ { required: true, name: :webhook, @@ -493,6 +493,14 @@ module API desc: 'The channel name' } ], + 'mattermost-notification' => [ + { + required: true, + name: :webhook, + type: String, + desc: 'The Mattermost webhook. e.g. http://mattermost_host/hooks/...' + } + ], 'teamcity' => [ { required: true, diff --git a/spec/features/projects/import_export/test_project_export.tar.gz b/spec/features/projects/import_export/test_project_export.tar.gz Binary files differindex bfe59bdb90e..d3165d07d7b 100644 --- a/spec/features/projects/import_export/test_project_export.tar.gz +++ b/spec/features/projects/import_export/test_project_export.tar.gz diff --git a/spec/features/projects/services/slack_service_spec.rb b/spec/features/projects/services/slack_service_spec.rb index 16541f51d98..320ed13a01d 100644 --- a/spec/features/projects/services/slack_service_spec.rb +++ b/spec/features/projects/services/slack_service_spec.rb @@ -2,8 +2,8 @@ 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) } + let(:service) { SlackNotificationService.new } + let(:project) { create(:project, slack_notification_service: service) } background do service.fields diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 068137f6255..9b49d6837c3 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -136,8 +136,8 @@ project: - assembla_service - asana_service - gemnasium_service -- slack_service -- mattermost_service +- slack_notification_service +- mattermost_notification_service - buildkite_service - bamboo_service - teamcity_service diff --git a/spec/models/project_services/chat_notification_service_spec.rb b/spec/models/project_services/chat_notification_service_spec.rb new file mode 100644 index 00000000000..b4fb1cd9ed9 --- /dev/null +++ b/spec/models/project_services/chat_notification_service_spec.rb @@ -0,0 +1,12 @@ +require 'spec_helper' + +describe ChatNotificationService, models: true do + describe "Associations" do + + before do + allow(subject).to receive(:activated?).and_return(true) + end + + it { is_expected.to validate_presence_of :webhook } + end +end diff --git a/spec/models/project_services/chat_service_spec.rb b/spec/models/project_services/chat_service_spec.rb index e6314a43501..c6a45a3e1be 100644 --- a/spec/models/project_services/chat_service_spec.rb +++ b/spec/models/project_services/chat_service_spec.rb @@ -2,7 +2,14 @@ require 'spec_helper' describe ChatService, models: true do describe "Associations" do - before { allow(subject).to receive(:activated?).and_return(true) } - it { is_expected.to validate_presence_of :webhook } + it { is_expected.to have_many :chat_names } + end + + describe '#valid_token?' do + subject { described_class.new } + + it 'is false as it has no token' do + expect(subject.valid_token?('wer')).to be_falsey + end end end diff --git a/spec/models/project_services/slack_service_spec.rb b/spec/models/project_services/mattermost_notification_service_spec.rb index 4928391fd7e..c01e64b4c8e 100644 --- a/spec/models/project_services/slack_service_spec.rb +++ b/spec/models/project_services/mattermost_notification_service_spec.rb @@ -1,5 +1,5 @@ require 'spec_helper' -describe SlackService, models: true do +describe MattermostNotificationService, models: true do it_behaves_like "slack or mattermost" end diff --git a/spec/models/project_services/mattermost_service_spec.rb b/spec/models/project_services/slack_notification_service_spec.rb index 1e5b4c715c3..59ddddf7454 100644 --- a/spec/models/project_services/mattermost_service_spec.rb +++ b/spec/models/project_services/slack_notification_service_spec.rb @@ -1,5 +1,5 @@ require 'spec_helper' -describe MattermostService, models: true do +describe SlackNotificationService, models: true do it_behaves_like "slack or mattermost" end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 1d8e42202ea..bab3c3dbb02 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -22,8 +22,8 @@ describe Project, models: true do it { is_expected.to have_many(:protected_branches).dependent(:destroy) } it { is_expected.to have_many(:chat_services) } it { is_expected.to have_one(:forked_project_link).dependent(:destroy) } - it { is_expected.to have_one(:slack_service).dependent(:destroy) } - it { is_expected.to have_one(:mattermost_service).dependent(:destroy) } + it { is_expected.to have_one(:slack_notification_service).dependent(:destroy) } + it { is_expected.to have_one(:mattermost_notification_service).dependent(:destroy) } it { is_expected.to have_one(:pushover_service).dependent(:destroy) } it { is_expected.to have_one(:asana_service).dependent(:destroy) } it { is_expected.to have_many(:boards).dependent(:destroy) } |