summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorFelipe Artur <felipefac@gmail.com>2016-12-06 15:59:03 -0200
committerFelipe Artur <felipefac@gmail.com>2016-12-15 15:32:49 -0200
commita5ccaded656fb215f1f8d503b88c8f28bf90ce68 (patch)
treef68c114323f6d345a753f0062c1633449e76db21
parent141faaacf9119ce5d765efe73c6509030ba078cd (diff)
downloadgitlab-ce-a5ccaded656fb215f1f8d503b88c8f28bf90ce68.tar.gz
Change SlackService to SlackNotificationsServiceissue_22269_fix_eeissue_22269
-rw-r--r--app/models/project.rb4
-rw-r--r--app/models/project_services/chat_notification_service.rb147
-rw-r--r--app/models/project_services/chat_service.rb143
-rw-r--r--app/models/project_services/mattermost_notification_service.rb (renamed from app/models/project_services/mattermost_service.rb)6
-rw-r--r--app/models/project_services/mattermost_slash_commands_service.rb12
-rw-r--r--app/models/project_services/slack_notification_service.rb (renamed from app/models/project_services/slack_service.rb)4
-rw-r--r--app/models/service.rb4
-rw-r--r--db/migrate/20141006143943_move_slack_service_to_webhook.rb6
-rw-r--r--db/migrate/20161213172958_change_slack_service_to_slack_notification_service.rb14
-rw-r--r--db/schema.rb2
-rw-r--r--lib/api/services.rb10
-rw-r--r--spec/features/projects/import_export/test_project_export.tar.gzbin681774 -> 679415 bytes
-rw-r--r--spec/features/projects/services/slack_service_spec.rb4
-rw-r--r--spec/lib/gitlab/import_export/all_models.yml4
-rw-r--r--spec/models/project_services/chat_notification_service_spec.rb12
-rw-r--r--spec/models/project_services/chat_service_spec.rb11
-rw-r--r--spec/models/project_services/mattermost_notification_service_spec.rb (renamed from spec/models/project_services/slack_service_spec.rb)2
-rw-r--r--spec/models/project_services/slack_notification_service_spec.rb (renamed from spec/models/project_services/mattermost_service_spec.rb)2
-rw-r--r--spec/models/project_spec.rb4
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
index 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
Binary files differ
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) }