Merge branch '14080-slack-multiple-channels' into 'master'

Allow multiple Slack channels for notifications

Closes #14080

See merge request gitlab-org/gitlab!24132

Gemfile

@@ -237,7 +237,7 @@ gem 'atlassian-jwt', '~> 0.2.0'
 gem 'flowdock', '~> 0.7'
 
 # Slack integration
-gem 'slack-notifier', '~> 1.5.1'
+gem 'slack-messenger', '~> 2.3.3'
 
 # Hangouts Chat integration
 gem 'hangouts-chat', '~> 0.0.5'

Gemfile.lock

@@ -1021,7 +1021,7 @@ GEM
       simplecov-html (~> 0.10.0)
     simplecov-html (0.10.2)
     sixarm_ruby_unaccent (1.2.0)
-    slack-notifier (1.5.1)
+    slack-messenger (2.3.3)
     snowplow-tracker (0.6.1)
       contracts (~> 0.7, <= 0.11)
     spring (2.0.2)
@@ -1376,7 +1376,7 @@ DEPENDENCIES
   sidekiq-cron (~> 1.0)
   simple_po_parser (~> 1.1.2)
   simplecov (~> 0.16.1)
-  slack-notifier (~> 1.5.1)
+  slack-messenger (~> 2.3.3)
   snowplow-tracker (~> 0.6.1)
   spring (~> 2.0.0)
   spring-commands-rspec (~> 1.0.4)

app/models/project_services/chat_message/base_message.rb

 # frozen_string_literal: true
 
-require 'slack-notifier'
-
 module ChatMessage
   class BaseMessage
     RELATIVE_LINK_REGEX = /!\[[^\]]*\]\((\/uploads\/[^\)]*)\)/.freeze
@@ -59,7 +57,7 @@ module ChatMessage
     end
 
     def format(string)
-      Slack::Notifier::LinkFormatter.format(format_relative_links(string))
+      Slack::Messenger::Util::LinkFormatter.format(format_relative_links(string))
     end
 
     def format_relative_links(string)

app/models/project_services/chat_message/pipeline_message.rb

 # frozen_string_literal: true
-require 'slack-notifier'
 
 module ChatMessage
   class PipelineMessage < BaseMessage
@@ -98,7 +97,7 @@ module ChatMessage
     def failed_stages_field
       {
         title: s_("ChatMessage|Failed stage").pluralize(failed_stages.length),
-        value: Slack::Notifier::LinkFormatter.format(failed_stages_links),
+        value: Slack::Messenger::Util::LinkFormatter.format(failed_stages_links),
         short: true
       }
     end
@@ -106,7 +105,7 @@ module ChatMessage
     def failed_jobs_field
       {
         title: s_("ChatMessage|Failed job").pluralize(failed_jobs.length),
-        value: Slack::Notifier::LinkFormatter.format(failed_jobs_links),
+        value: Slack::Messenger::Util::LinkFormatter.format(failed_jobs_links),
         short: true
       }
     end
@@ -123,12 +122,12 @@ module ChatMessage
       fields = [
         {
           title: ref_type == "tag" ? s_("ChatMessage|Tag") : s_("ChatMessage|Branch"),
-          value: Slack::Notifier::LinkFormatter.format(ref_link),
+          value: Slack::Messenger::Util::LinkFormatter.format(ref_link),
           short: true
         },
         {
           title: s_("ChatMessage|Commit"),
-          value: Slack::Notifier::LinkFormatter.format(commit_link),
+          value: Slack::Messenger::Util::LinkFormatter.format(commit_link),
           short: true
         }
       ]

app/models/project_services/chat_message/push_message.rb

@@ -48,7 +48,7 @@ module ChatMessage
     end
 
     def format(string)
-      Slack::Notifier::LinkFormatter.format(string)
+      Slack::Messenger::Util::LinkFormatter.format(string)
     end
 
     def commit_messages

app/models/project_services/chat_notification_service.rb

@@ -84,10 +84,10 @@ class ChatNotificationService < Service
 
     event_type = data[:event_type] || object_kind
 
-    channel_name = get_channel_field(event_type).presence || channel
+    channel_names = get_channel_field(event_type).presence || channel
 
     opts = {}
-    opts[:channel] = channel_name if channel_name
+    opts[:channel] = channel_names.split(',').map(&:strip) if channel_names
     opts[:username] = username if username
 
     return false unless notify(message, opts)

app/models/project_services/slack_service.rb

@@ -13,18 +13,8 @@ class SlackService < ChatNotificationService
     'slack'
   end
 
-  def help
-    'This service sends notifications about projects events to Slack channels.<br />
-    To set up this service:
-    <ol>
-      <li><a href="https://slack.com/apps/A0F7XDUAZ-incoming-webhooks">Add an incoming webhook</a> in your Slack team. The default channel can be overridden for each event.</li>
-      <li>Paste the <strong>Webhook URL</strong> into the field below.</li>
-      <li>Select events below to enable notifications. The <strong>Channel name</strong> and <strong>Username</strong> fields are optional.</li>
-    </ol>'
-  end
-
   def default_channel_placeholder
-    "Channel name (e.g. general)"
+    _('Slack channels (e.g. general, development)')
   end
 
   def webhook_placeholder
@@ -35,8 +25,8 @@ class SlackService < ChatNotificationService
     private
 
     def notify(message, opts)
-      # See https://github.com/stevenosloan/slack-notifier#custom-http-client
-      notifier = Slack::Notifier.new(webhook, opts.merge(http_client: HTTPClient))
+      # See https://gitlab.com/gitlab-org/slack-notifier/#custom-http-client
+      notifier = Slack::Messenger.new(webhook, opts.merge(http_client: HTTPClient))
 
       notifier.ping(
         message.pretext,

app/models/project_services/slack_slash_commands_service.rb

@@ -29,6 +29,6 @@ class SlackSlashCommandsService < SlashCommandsService
   private
 
   def format(text)
-    Slack::Notifier::LinkFormatter.format(text) if text
+    Slack::Messenger::Util::LinkFormatter.format(text) if text
   end
 end

app/views/projects/services/slack/_help.haml

+- webhooks_link_url = 'https://slack.com/apps/A0F7XDUAZ-incoming-webhooks'
+- webhooks_link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: webhooks_link_url }
+
+.info-well
+  .well-segment
+    %p= s_('SlackIntegration|This service send notifications about projects\' events to Slack channels. To set up this service:')
+    %ol
+      %li
+        = s_('SlackIntegration|%{webhooks_link_start}Add an incoming webhook%{webhooks_link_end} in your Slack team. The default channel can be overridden for each event.').html_safe % { webhooks_link_start: webhooks_link_start, webhooks_link_end: '</a>'.html_safe }
+      %li
+        = s_('SlackIntegration|Paste the <strong>Webhook URL</strong> into the field below.').html_safe
+      %li
+        = s_('SlackIntegration|Select events below to enable notifications. The <strong>Slack channel names</strong> and <strong>Slack username</strong> fields are optional.').html_safe
+    %p.mt-3.mb-0
+      = s_('SlackIntegration|<strong>Note:</strong> Usernames and private channels are not supported.').html_safe
+      = link_to _('Learn more'), help_page_path('user/project/integrations/slack')

changelogs/unreleased/14080-slack-multiple-channels.yml

+---
+title: Allow multiple Slack channels for notifications
+merge_request: 24132
+author:
+type: added

doc/user/project/integrations/slack.md

@@ -16,7 +16,7 @@ The Slack Notifications Service allows your GitLab project to send events (e.g.
 1. Select the **Slack notifications** project service to configure it.
 1. Check the **Active** checkbox to turn on the service.
 1. Check the checkboxes corresponding to the GitLab events you want to send to Slack as a notification.
-1. For each event, optionally enter the Slack channel where you want to send the event. (Do _not_ include the `#` symbol.) If left empty, the event will be sent to the default channel that you configured in the Slack Configuration step.
+1. For each event, optionally enter the Slack channel names where you want to send the event, separated by a comma. If left empty, the event will be sent to the default channel that you configured in the Slack Configuration step. **Note:** Usernames and private channels are not supported. To send direct messages, use the Member ID found under user's Slack profile.
 1. Paste the **Webhook URL** that you copied from the Slack Configuration step.
 1. Optionally customize the Slack bot username that will be sending the notifications.
 1. Configure the remaining options and click `Save changes`.

lib/gitlab/slash_commands/presenters/base.rb

@@ -63,7 +63,7 @@ module Gitlab
 
         # Convert Markdown to slacks format
         def format(string)
-          Slack::Notifier::LinkFormatter.format(string)
+          Slack::Messenger::Util::LinkFormatter.format(string)
         end
 
         def resource_url

locale/gitlab.pot

@@ -17847,9 +17847,27 @@ msgstr ""
 msgid "Slack application"
 msgstr ""
 
+msgid "Slack channels (e.g. general, development)"
+msgstr ""
+
 msgid "Slack integration allows you to interact with GitLab via slash commands in a chat window."
 msgstr ""
 
+msgid "SlackIntegration|%{webhooks_link_start}Add an incoming webhook%{webhooks_link_end} in your Slack team. The default channel can be overridden for each event."
+msgstr ""
+
+msgid "SlackIntegration|<strong>Note:</strong> Usernames and private channels are not supported."
+msgstr ""
+
+msgid "SlackIntegration|Paste the <strong>Webhook URL</strong> into the field below."
+msgstr ""
+
+msgid "SlackIntegration|Select events below to enable notifications. The <strong>Slack channel names</strong> and <strong>Slack username</strong> fields are optional."
+msgstr ""
+
+msgid "SlackIntegration|This service send notifications about projects' events to Slack channels. To set up this service:"
+msgstr ""
+
 msgid "SlackService|2. Paste the <strong>Token</strong> into the field below"
 msgstr ""
 

spec/models/project_services/chat_notification_service_spec.rb

@@ -74,5 +74,28 @@ describe ChatNotificationService do
         chat_service.execute(data)
       end
     end
+
+    shared_examples 'with channel specified' do |channel, expected_channels|
+      before do
+        allow(chat_service).to receive(:push_channel).and_return(channel)
+      end
+
+      it 'notifies all channels' do
+        expect(chat_service).to receive(:notify).with(any_args, hash_including(channel: expected_channels)).and_return(true)
+        expect(chat_service.execute(data)).to be(true)
+      end
+    end
+
+    context 'with single channel specified' do
+      it_behaves_like 'with channel specified', 'slack-integration', ['slack-integration']
+    end
+
+    context 'with multiple channel names specified' do
+      it_behaves_like 'with channel specified', 'slack-integration,#slack-test', ['slack-integration', '#slack-test']
+    end
+
+    context 'with multiple channel names with spaces specified' do
+      it_behaves_like 'with channel specified', 'slack-integration, #slack-test, @UDLP91W0A', ['slack-integration', '#slack-test', '@UDLP91W0A']
+    end
   end
 end

spec/support/shared_examples/models/slack_mattermost_notifications_shared_examples.rb

@@ -151,22 +151,14 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
     it 'uses the username as an option for slack when configured' do
       allow(chat_service).to receive(:username).and_return(username)
 
-      expect(Slack::Notifier).to receive(:new)
-       .with(webhook_url, username: username, http_client: SlackService::Notifier::HTTPClient)
-       .and_return(
-         double(:slack_service).as_null_object
-       )
+      expect(Slack::Messenger).to execute_with_options(username: username)
 
       chat_service.execute(data)
     end
 
     it 'uses the channel as an option when it is configured' do
       allow(chat_service).to receive(:channel).and_return(channel)
-      expect(Slack::Notifier).to receive(:new)
-        .with(webhook_url, channel: channel, http_client: SlackService::Notifier::HTTPClient)
-        .and_return(
-          double(:slack_service).as_null_object
-        )
+      expect(Slack::Messenger).to execute_with_options(channel: [channel])
       chat_service.execute(data)
     end
 
@@ -174,11 +166,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
       it "uses the right channel for push event" do
         chat_service.update(push_channel: "random")
 
-        expect(Slack::Notifier).to receive(:new)
-         .with(webhook_url, channel: "random", http_client: SlackService::Notifier::HTTPClient)
-         .and_return(
-           double(:slack_service).as_null_object
-         )
+        expect(Slack::Messenger).to execute_with_options(channel: ['random'])
 
         chat_service.execute(data)
       end
@@ -186,11 +174,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
       it "uses the right channel for merge request event" do
         chat_service.update(merge_request_channel: "random")
 
-        expect(Slack::Notifier).to receive(:new)
-         .with(webhook_url, channel: "random", http_client: SlackService::Notifier::HTTPClient)
-         .and_return(
-           double(:slack_service).as_null_object
-         )
+        expect(Slack::Messenger).to execute_with_options(channel: ['random'])
 
         chat_service.execute(@merge_sample_data)
       end
@@ -198,11 +182,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
       it "uses the right channel for issue event" do
         chat_service.update(issue_channel: "random")
 
-        expect(Slack::Notifier).to receive(:new)
-         .with(webhook_url, channel: "random", http_client: SlackService::Notifier::HTTPClient)
-         .and_return(
-           double(:slack_service).as_null_object
-         )
+        expect(Slack::Messenger).to execute_with_options(channel: ['random'])
 
         chat_service.execute(@issues_sample_data)
       end
@@ -213,7 +193,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
         it "uses confidential issue channel" do
           chat_service.update(confidential_issue_channel: 'confidential')
 
-          expect(Slack::Notifier).to execute_with_options(channel: 'confidential')
+          expect(Slack::Messenger).to execute_with_options(channel: ['confidential'])
 
           chat_service.execute(@issues_sample_data)
         end
@@ -221,7 +201,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
         it 'falls back to issue channel' do
           chat_service.update(issue_channel: 'fallback_channel')
 
-          expect(Slack::Notifier).to execute_with_options(channel: 'fallback_channel')
+          expect(Slack::Messenger).to execute_with_options(channel: ['fallback_channel'])
 
           chat_service.execute(@issues_sample_data)
         end
@@ -230,11 +210,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
       it "uses the right channel for wiki event" do
         chat_service.update(wiki_page_channel: "random")
 
-        expect(Slack::Notifier).to receive(:new)
-         .with(webhook_url, channel: "random", http_client: SlackService::Notifier::HTTPClient)
-         .and_return(
-           double(:slack_service).as_null_object
-         )
+        expect(Slack::Messenger).to execute_with_options(channel: ['random'])
 
         chat_service.execute(@wiki_page_sample_data)
       end
@@ -249,11 +225,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
 
           note_data = Gitlab::DataBuilder::Note.build(issue_note, user)
 
-          expect(Slack::Notifier).to receive(:new)
-           .with(webhook_url, channel: "random", http_client: SlackService::Notifier::HTTPClient)
-           .and_return(
-             double(:slack_service).as_null_object
-           )
+          expect(Slack::Messenger).to execute_with_options(channel: ['random'])
 
           chat_service.execute(note_data)
         end
@@ -268,7 +240,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
 
             note_data = Gitlab::DataBuilder::Note.build(issue_note, user)
 
-            expect(Slack::Notifier).to execute_with_options(channel: 'confidential')
+            expect(Slack::Messenger).to execute_with_options(channel: ['confidential'])
 
             chat_service.execute(note_data)
           end
@@ -278,7 +250,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
 
             note_data = Gitlab::DataBuilder::Note.build(issue_note, user)
 
-            expect(Slack::Notifier).to execute_with_options(channel: 'fallback_channel')
+            expect(Slack::Messenger).to execute_with_options(channel: ['fallback_channel'])
 
             chat_service.execute(note_data)
           end