Merge branch 'add_musst_have_all_labels_to_be_notified_option' into 'master'

Add option for chat service label filter to match all labels

See merge request gitlab-org/gitlab!56657

app/controllers/concerns/service_params.rb

@@ -12,6 +12,7 @@ module ServiceParams
     :bamboo_url,
     :branches_to_be_notified,
     :labels_to_be_notified,
+    :labels_to_be_notified_behavior,
     :build_key,
     :build_type,
     :ca_pem,

app/models/project_services/chat_notification_service.rb

@@ -15,9 +15,14 @@ class ChatNotificationService < Service
 
   EVENT_CHANNEL = proc { |event| "#{event}_channel" }
 
+  LABEL_NOTIFICATION_BEHAVIOURS = [
+    MATCH_ANY_LABEL = 'match_any',
+    MATCH_ALL_LABELS = 'match_all'
+  ].freeze
+
   default_value_for :category, 'chat'
 
-  prop_accessor :webhook, :username, :channel, :branches_to_be_notified, :labels_to_be_notified
+  prop_accessor :webhook, :username, :channel, :branches_to_be_notified, :labels_to_be_notified, :labels_to_be_notified_behavior
 
   # Custom serialized properties initialization
   prop_accessor(*SUPPORTED_EVENTS.map { |event| EVENT_CHANNEL[event] })
@@ -25,12 +30,14 @@ class ChatNotificationService < Service
   boolean_accessor :notify_only_broken_pipelines, :notify_only_default_branch
 
   validates :webhook, presence: true, public_url: true, if: :activated?
+  validates :labels_to_be_notified_behavior, inclusion: { in: LABEL_NOTIFICATION_BEHAVIOURS }, allow_nil: true
 
   def initialize_properties
     if properties.nil?
       self.properties = {}
       self.notify_only_broken_pipelines = true
       self.branches_to_be_notified = "default"
+      self.labels_to_be_notified_behavior = MATCH_ANY_LABEL
     elsif !self.notify_only_default_branch.nil?
       # In older versions, there was only a boolean property named
       # `notify_only_default_branch`. Now we have a string property named
@@ -65,7 +72,20 @@ class ChatNotificationService < Service
       { type: 'text', name: 'username', placeholder: 'GitLab-integration' }.freeze,
       { type: 'checkbox', name: 'notify_only_broken_pipelines', help: 'Do not send notifications for successful pipelines.' }.freeze,
       { type: 'select', name: 'branches_to_be_notified', choices: branch_choices }.freeze,
-      { type: 'text', name: 'labels_to_be_notified', placeholder: '~backend,~frontend', help: 'Send notifications for issue, merge request, and comment events with the listed labels only. Leave blank to receive notifications for all events.' }.freeze
+      {
+        type: 'text',
+        name: 'labels_to_be_notified',
+        placeholder: '~backend,~frontend',
+        help: 'Send notifications for issue, merge request, and comment events with the listed labels only. Leave blank to receive notifications for all events.'
+      }.freeze,
+      {
+        type: 'select',
+        name: 'labels_to_be_notified_behavior',
+        choices: [
+          ['Match any of the labels', MATCH_ANY_LABEL],
+          ['Match all of the labels', MATCH_ALL_LABELS]
+        ]
+      }.freeze
     ].freeze
   end
 
@@ -136,11 +156,17 @@ class ChatNotificationService < Service
   def notify_label?(data)
     return true unless SUPPORTED_EVENTS_FOR_LABEL_FILTER.include?(data[:object_kind]) && labels_to_be_notified.present?
 
-    issue_labels = data.dig(:issue, :labels) || []
-    merge_request_labels = data.dig(:merge_request, :labels) || []
-    label_titles = (issue_labels + merge_request_labels).pluck(:title)
+    labels = data.dig(:issue, :labels) || data.dig(:merge_request, :labels)
+
+    return false if labels.nil?
 
-    (labels_to_be_notified_list & label_titles).any?
+    matching_labels = labels_to_be_notified_list & labels.pluck(:title)
+
+    if labels_to_be_notified_behavior == MATCH_ALL_LABELS
+      labels_to_be_notified_list.difference(matching_labels).empty?
+    else
+      matching_labels.any?
+    end
   end
 
   def user_id_from_hook_data(data)

changelogs/unreleased/add_musst_have_all_labels_to_be_notified_option.yml

+---
+title: Add options for Slack and Mattermost label filter behavior
+merge_request: 56657
+author:
+type: added

doc/user/project/integrations/mattermost.md

@@ -48,10 +48,16 @@ notification. You do not need to add the hash sign (`#`).
 
 Then fill in the integration configuration:
 
-| Field | Description |
-| ----- | ----------- |
-| **Webhook**  | The incoming webhook URL on Mattermost, similar to: `http://mattermost.example/hooks/5xo…`. |
-| **Username** | (Optional) The username to show on messages sent to Mattermost. Fill this in to change the username of the bot. |
-| **Notify only broken pipelines** | If you enable the **Pipeline** event and you want to be notified about failed pipelines only. |
-| **Branches to be notified** | Select which branches to send notifications for. |
-| **Labels to be notified** | (Optional) Labels that the issue or merge request must have to trigger a notification. Leave blank to get notifications for all issues and merge requests. |
+- **Webhook**: The incoming webhook URL on Mattermost, similar to
+  `http://mattermost.example/hooks/5xo…`.
+- **Username**: (Optional) The username shown in messages sent to Mattermost.
+  To change the bot's username, provide a value.
+- **Notify only broken pipelines**: If you enable the **Pipeline** event, and you want
+  notifications about failed pipelines only.
+- **Branches to be notified**: The branches to send notifications for.
+- **Labels to be notified**: (Optional) Labels required for the issue or merge request
+  to trigger a notification. Leave blank to notify for all issues and merge requests.
+- **Labels to be notified behavior**: When you use the **Labels to be notified** filter,
+  messages are sent when an issue or merge request contains _any_ of the labels specified
+  in the filter. You can also choose to trigger messages only when the issue or merge request
+  contains _all_ the labels defined in the filter.

spec/features/admin/admin_settings_spec.rb

@@ -266,6 +266,7 @@ RSpec.describe 'Admin updates settings' do
         fill_in 'service[push_channel]', with: '#test_channel'
         page.check('Notify only broken pipelines')
         page.select 'All branches', from: 'Branches to be notified'
+        page.select 'Match any of the labels', from: 'Labels to be notified behavior'
 
         check_all_events
         click_button 'Save changes'

spec/models/project_services/chat_notification_service_spec.rb

@@ -11,6 +11,10 @@ RSpec.describe ChatNotificationService do
     it { is_expected.to validate_presence_of :webhook }
   end
 
+  describe 'validations' do
+    it { is_expected.to validate_inclusion_of(:labels_to_be_notified_behavior).in_array(%w[match_any match_all]) }
+  end
+
   describe '#can_test?' do
     context 'with empty repository' do
       it 'returns true' do
@@ -32,8 +36,9 @@ RSpec.describe ChatNotificationService do
   describe '#execute' do
     subject(:chat_service) { described_class.new }
 
+    let_it_be(:project) { create(:project, :repository) }
+
     let(:user) { create(:user) }
-    let(:project) { create(:project, :repository) }
     let(:webhook_url) { 'https://example.gitlab.com/' }
     let(:data) { Gitlab::DataBuilder::Push.build_sample(subject.project, user) }
 
@@ -76,9 +81,12 @@ RSpec.describe ChatNotificationService do
     end
 
     context 'when the data object has a label' do
-      let(:label) { create(:label, project: project, name: 'Bug')}
-      let(:issue) { create(:labeled_issue, project: project, labels: [label]) }
-      let(:note) { create(:note, noteable: issue, project: project)}
+      let_it_be(:label) { create(:label, name: 'Bug') }
+      let_it_be(:label_2) { create(:label, name: 'Community contribution') }
+      let_it_be(:label_3) { create(:label, name: 'Backend') }
+      let_it_be(:issue) { create(:labeled_issue, project: project, labels: [label, label_2, label_3]) }
+      let_it_be(:note) { create(:note, noteable: issue, project: project) }
+
       let(:data) { Gitlab::DataBuilder::Note.build(note, user) }
 
       it 'notifies the chat service' do
@@ -87,23 +95,123 @@ RSpec.describe ChatNotificationService do
         chat_service.execute(data)
       end
 
-      context 'and the chat_service has a label filter that does not matches the label' do
-        subject(:chat_service) { described_class.new(labels_to_be_notified: '~some random label') }
+      shared_examples 'notifies the chat service' do
+        specify do
+          expect(chat_service).to receive(:notify).with(any_args)
+
+          chat_service.execute(data)
+        end
+      end
 
-        it 'does not notify the chat service' do
-          expect(chat_service).not_to receive(:notify)
+      shared_examples 'does not notify the chat service' do
+        specify do
+          expect(chat_service).not_to receive(:notify).with(any_args)
 
           chat_service.execute(data)
         end
       end
 
-      context 'and the chat_service has a label filter that matches the label' do
-        subject(:chat_service) { described_class.new(labels_to_be_notified: '~Backend, ~Bug') }
+      context 'when labels_to_be_notified_behavior is not defined' do
+        subject(:chat_service) { described_class.new(labels_to_be_notified: label_filter) }
 
-        it 'notifies the chat service' do
-          expect(chat_service).to receive(:notify).with(any_args)
+        context 'no matching labels' do
+          let(:label_filter) { '~some random label' }
 
-          chat_service.execute(data)
+          it_behaves_like 'does not notify the chat service'
+        end
+
+        context 'only one label matches' do
+          let(:label_filter) { '~some random label, ~Bug' }
+
+          it_behaves_like 'notifies the chat service'
+        end
+      end
+
+      context 'when labels_to_be_notified_behavior is match_any' do
+        subject(:chat_service) do
+          described_class.new(
+            labels_to_be_notified: label_filter,
+            labels_to_be_notified_behavior: 'match_any'
+          )
+        end
+
+        context 'no label filter' do
+          let(:label_filter) { nil }
+
+          it_behaves_like 'notifies the chat service'
+        end
+
+        context 'no matching labels' do
+          let(:label_filter) { '~some random label' }
+
+          it_behaves_like 'does not notify the chat service'
+        end
+
+        context 'only one label matches' do
+          let(:label_filter) { '~some random label, ~Bug' }
+
+          it_behaves_like 'notifies the chat service'
+        end
+      end
+
+      context 'when labels_to_be_notified_behavior is match_all' do
+        subject(:chat_service) do
+          described_class.new(
+            labels_to_be_notified: label_filter,
+            labels_to_be_notified_behavior: 'match_all'
+          )
+        end
+
+        context 'no label filter' do
+          let(:label_filter) { nil }
+
+          it_behaves_like 'notifies the chat service'
+        end
+
+        context 'no matching labels' do
+          let(:label_filter) { '~some random label' }
+
+          it_behaves_like 'does not notify the chat service'
+        end
+
+        context 'only one label matches' do
+          let(:label_filter) { '~some random label, ~Bug' }
+
+          it_behaves_like 'does not notify the chat service'
+        end
+
+        context 'labels matches exactly' do
+          let(:label_filter) { '~Bug, ~Backend, ~Community contribution' }
+
+          it_behaves_like 'notifies the chat service'
+        end
+
+        context 'labels matches but object has more' do
+          let(:label_filter) { '~Bug, ~Backend' }
+
+          it_behaves_like 'notifies the chat service'
+        end
+
+        context 'labels are distributed on multiple objects' do
+          let(:label_filter) { '~Bug, ~Backend' }
+          let(:data) do
+            Gitlab::DataBuilder::Note.build(note, user).merge({
+              issue: {
+                labels: [
+                  { title: 'Bug' }
+                ]
+              },
+              merge_request: {
+                labels: [
+                  {
+                    title: 'Backend'
+                  }
+                ]
+              }
+            })
+          end
+
+          it_behaves_like 'does not notify the chat service'
         end
       end
     end

spec/support/shared_contexts/services_shared_context.rb

@@ -30,6 +30,8 @@ Service.available_services_names.each do |service|
           hash.merge!(k => '1,2,3')
         elsif service == 'emails_on_push' && k == :recipients
           hash.merge!(k => 'foo@bar.com')
+        elsif service == 'slack' || service == 'mattermost' && k == :labels_to_be_notified_behavior
+          hash.merge!(k => "match_any")
         else
           hash.merge!(k => "someword")
         end