Commit 02a68dca authored by Andy Soiron's avatar Andy Soiron Committed by Bob Van Landuyt

Add option for chat service label filter to match all labels

parent 4badc4a8
...@@ -12,6 +12,7 @@ module ServiceParams ...@@ -12,6 +12,7 @@ module ServiceParams
:bamboo_url, :bamboo_url,
:branches_to_be_notified, :branches_to_be_notified,
:labels_to_be_notified, :labels_to_be_notified,
:labels_to_be_notified_behavior,
:build_key, :build_key,
:build_type, :build_type,
:ca_pem, :ca_pem,
......
...@@ -15,9 +15,14 @@ class ChatNotificationService < Service ...@@ -15,9 +15,14 @@ class ChatNotificationService < Service
EVENT_CHANNEL = proc { |event| "#{event}_channel" } 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' 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 # Custom serialized properties initialization
prop_accessor(*SUPPORTED_EVENTS.map { |event| EVENT_CHANNEL[event] }) prop_accessor(*SUPPORTED_EVENTS.map { |event| EVENT_CHANNEL[event] })
...@@ -25,12 +30,14 @@ class ChatNotificationService < Service ...@@ -25,12 +30,14 @@ class ChatNotificationService < Service
boolean_accessor :notify_only_broken_pipelines, :notify_only_default_branch boolean_accessor :notify_only_broken_pipelines, :notify_only_default_branch
validates :webhook, presence: true, public_url: true, if: :activated? 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 def initialize_properties
if properties.nil? if properties.nil?
self.properties = {} self.properties = {}
self.notify_only_broken_pipelines = true self.notify_only_broken_pipelines = true
self.branches_to_be_notified = "default" self.branches_to_be_notified = "default"
self.labels_to_be_notified_behavior = MATCH_ANY_LABEL
elsif !self.notify_only_default_branch.nil? elsif !self.notify_only_default_branch.nil?
# In older versions, there was only a boolean property named # In older versions, there was only a boolean property named
# `notify_only_default_branch`. Now we have a string property named # `notify_only_default_branch`. Now we have a string property named
...@@ -65,7 +72,20 @@ class ChatNotificationService < Service ...@@ -65,7 +72,20 @@ class ChatNotificationService < Service
{ type: 'text', name: 'username', placeholder: 'GitLab-integration' }.freeze, { 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: '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: '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 ].freeze
end end
...@@ -136,11 +156,17 @@ class ChatNotificationService < Service ...@@ -136,11 +156,17 @@ class ChatNotificationService < Service
def notify_label?(data) def notify_label?(data)
return true unless SUPPORTED_EVENTS_FOR_LABEL_FILTER.include?(data[:object_kind]) && labels_to_be_notified.present? return true unless SUPPORTED_EVENTS_FOR_LABEL_FILTER.include?(data[:object_kind]) && labels_to_be_notified.present?
issue_labels = data.dig(:issue, :labels) || [] labels = data.dig(:issue, :labels) || data.dig(:merge_request, :labels)
merge_request_labels = data.dig(:merge_request, :labels) || []
label_titles = (issue_labels + merge_request_labels).pluck(:title) 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 end
def user_id_from_hook_data(data) def user_id_from_hook_data(data)
......
---
title: Add options for Slack and Mattermost label filter behavior
merge_request: 56657
author:
type: added
...@@ -48,10 +48,16 @@ notification. You do not need to add the hash sign (`#`). ...@@ -48,10 +48,16 @@ notification. You do not need to add the hash sign (`#`).
Then fill in the integration configuration: Then fill in the integration configuration:
| Field | Description | - **Webhook**: The incoming webhook URL on Mattermost, similar to
| ----- | ----------- | `http://mattermost.example/hooks/5xo…`.
| **Webhook** | The incoming webhook URL on Mattermost, similar to: `http://mattermost.example/hooks/5xo…`. | - **Username**: (Optional) The username shown in messages sent to Mattermost.
| **Username** | (Optional) The username to show on messages sent to Mattermost. Fill this in to change the username of the bot. | To change the bot's username, provide a value.
| **Notify only broken pipelines** | If you enable the **Pipeline** event and you want to be notified about failed pipelines only. | - **Notify only broken pipelines**: If you enable the **Pipeline** event, and you want
| **Branches to be notified** | Select which branches to send notifications for. | notifications about failed pipelines only.
| **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. | - **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.
...@@ -266,6 +266,7 @@ RSpec.describe 'Admin updates settings' do ...@@ -266,6 +266,7 @@ RSpec.describe 'Admin updates settings' do
fill_in 'service[push_channel]', with: '#test_channel' fill_in 'service[push_channel]', with: '#test_channel'
page.check('Notify only broken pipelines') page.check('Notify only broken pipelines')
page.select 'All branches', from: 'Branches to be notified' 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 check_all_events
click_button 'Save changes' click_button 'Save changes'
......
...@@ -11,6 +11,10 @@ RSpec.describe ChatNotificationService do ...@@ -11,6 +11,10 @@ RSpec.describe ChatNotificationService do
it { is_expected.to validate_presence_of :webhook } it { is_expected.to validate_presence_of :webhook }
end 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 describe '#can_test?' do
context 'with empty repository' do context 'with empty repository' do
it 'returns true' do it 'returns true' do
...@@ -32,8 +36,9 @@ RSpec.describe ChatNotificationService do ...@@ -32,8 +36,9 @@ RSpec.describe ChatNotificationService do
describe '#execute' do describe '#execute' do
subject(:chat_service) { described_class.new } subject(:chat_service) { described_class.new }
let_it_be(:project) { create(:project, :repository) }
let(:user) { create(:user) } let(:user) { create(:user) }
let(:project) { create(:project, :repository) }
let(:webhook_url) { 'https://example.gitlab.com/' } let(:webhook_url) { 'https://example.gitlab.com/' }
let(:data) { Gitlab::DataBuilder::Push.build_sample(subject.project, user) } let(:data) { Gitlab::DataBuilder::Push.build_sample(subject.project, user) }
...@@ -76,9 +81,12 @@ RSpec.describe ChatNotificationService do ...@@ -76,9 +81,12 @@ RSpec.describe ChatNotificationService do
end end
context 'when the data object has a label' do context 'when the data object has a label' do
let(:label) { create(:label, project: project, name: 'Bug')} let_it_be(:label) { create(:label, name: 'Bug') }
let(:issue) { create(:labeled_issue, project: project, labels: [label]) } let_it_be(:label_2) { create(:label, name: 'Community contribution') }
let(:note) { create(:note, noteable: issue, project: project)} 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) } let(:data) { Gitlab::DataBuilder::Note.build(note, user) }
it 'notifies the chat service' do it 'notifies the chat service' do
...@@ -87,23 +95,123 @@ RSpec.describe ChatNotificationService do ...@@ -87,23 +95,123 @@ RSpec.describe ChatNotificationService do
chat_service.execute(data) chat_service.execute(data)
end end
context 'and the chat_service has a label filter that does not matches the label' do shared_examples 'notifies the chat service' do
subject(:chat_service) { described_class.new(labels_to_be_notified: '~some random label') } 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 shared_examples 'does not notify the chat service' do
expect(chat_service).not_to receive(:notify) specify do
expect(chat_service).not_to receive(:notify).with(any_args)
chat_service.execute(data) chat_service.execute(data)
end end
end end
context 'and the chat_service has a label filter that matches the label' do context 'when labels_to_be_notified_behavior is not defined' do
subject(:chat_service) { described_class.new(labels_to_be_notified: '~Backend, ~Bug') } subject(:chat_service) { described_class.new(labels_to_be_notified: label_filter) }
it 'notifies the chat service' do context 'no matching labels' do
expect(chat_service).to receive(:notify).with(any_args) 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 end
end end
......
...@@ -30,6 +30,8 @@ Service.available_services_names.each do |service| ...@@ -30,6 +30,8 @@ Service.available_services_names.each do |service|
hash.merge!(k => '1,2,3') hash.merge!(k => '1,2,3')
elsif service == 'emails_on_push' && k == :recipients elsif service == 'emails_on_push' && k == :recipients
hash.merge!(k => 'foo@bar.com') hash.merge!(k => 'foo@bar.com')
elsif service == 'slack' || service == 'mattermost' && k == :labels_to_be_notified_behavior
hash.merge!(k => "match_any")
else else
hash.merge!(k => "someword") hash.merge!(k => "someword")
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment