Commit 091c2608 authored by Z.J. van de Weg's avatar Z.J. van de Weg

Improve readability and add test

parent fed319b4
...@@ -17,7 +17,7 @@ class ChatNotificationService < Service ...@@ -17,7 +17,7 @@ class ChatNotificationService < Service
if properties.nil? if properties.nil?
self.properties = {} self.properties = {}
self.notify_only_broken_pipelines = true self.notify_only_broken_pipelines = true
self.notify_only_default_branch = false self.notify_only_default_branch = true
end end
end end
...@@ -137,20 +137,17 @@ class ChatNotificationService < Service ...@@ -137,20 +137,17 @@ class ChatNotificationService < Service
end end
def should_pipeline_be_notified?(data) def should_pipeline_be_notified?(data)
notify_for_branch(data) && notify_for_pipeline(data) notify_for_ref?(data) && notify_for_pipeline?(data)
end end
def notify_for_branch(data) def notify_for_ref?(data)
ref_type = data[:object_attributes][:tag] ? 'tag' : 'branch' return true if data[:object_attributes][:tag]
return true unless notify_only_default_branch
if ref_type == 'branch' && notify_only_default_branch
data[:object_attributes][:ref] == project.default_branch data[:object_attributes][:ref] == project.default_branch
else
true
end
end end
def notify_for_pipeline(data) def notify_for_pipeline?(data)
case data[:object_attributes][:status] case data[:object_attributes][:status]
when 'success' when 'success'
!notify_only_broken_pipelines? !notify_only_broken_pipelines?
......
...@@ -34,6 +34,7 @@ feature 'Admin updates settings', feature: true do ...@@ -34,6 +34,7 @@ feature 'Admin updates settings', feature: true do
fill_in 'Username', with: 'test_user' fill_in 'Username', with: 'test_user'
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.check('Notify only default branch')
check_all_events check_all_events
click_on 'Save' click_on 'Save'
......
...@@ -324,5 +324,24 @@ RSpec.shared_examples 'slack or mattermost notifications' do ...@@ -324,5 +324,24 @@ RSpec.shared_examples 'slack or mattermost notifications' do
it_behaves_like 'call Slack/Mattermost API' it_behaves_like 'call Slack/Mattermost API'
end end
end end
context 'only notify for the default branch' do
context 'when enabled' do
let(:pipeline) do
create(:ci_pipeline, project: project, status: 'failed', ref: 'not-the-default-branch')
end
before do
chat_service.notify_only_default_branch = true
end
it 'does not call the Slack/Mattermost API for pipeline events' do
data = Gitlab::DataBuilder::Pipeline.build(pipeline)
result = chat_service.execute(data)
expect(result).to be_falsy
end
end
end
end end
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