BigW Consortium Gitlab

Commit 9b704ef3 by Sean McGivern

Merge branch '33570-slack-notify-default-branch' into 'master'

Slack notifications - Respect "Notify only default branch" for push events Closes #33570 See merge request gitlab-org/gitlab-ce!17345
parents a519a346 158514bb
...@@ -99,7 +99,7 @@ class ChatNotificationService < Service ...@@ -99,7 +99,7 @@ class ChatNotificationService < Service
def get_message(object_kind, data) def get_message(object_kind, data)
case object_kind case object_kind
when "push", "tag_push" when "push", "tag_push"
ChatMessage::PushMessage.new(data) ChatMessage::PushMessage.new(data) if notify_for_ref?(data)
when "issue" when "issue"
ChatMessage::IssueMessage.new(data) unless update?(data) ChatMessage::IssueMessage.new(data) unless update?(data)
when "merge_request" when "merge_request"
...@@ -145,10 +145,16 @@ class ChatNotificationService < Service ...@@ -145,10 +145,16 @@ class ChatNotificationService < Service
end end
def notify_for_ref?(data) def notify_for_ref?(data)
return true if data[:object_attributes][:tag] return true if data.dig(:object_attributes, :tag)
return true unless notify_only_default_branch? return true unless notify_only_default_branch?
data[:object_attributes][:ref] == project.default_branch ref = if data[:ref]
Gitlab::Git.ref_name(data[:ref])
else
data.dig(:object_attributes, :ref)
end
ref == project.default_branch
end end
def notify_for_pipeline?(data) def notify_for_pipeline?(data)
......
---
title: Fix Slack/Mattermost notifications not respecting `notify_only_default_branch` setting for pushes
merge_request: 17345
author:
type: fixed
...@@ -337,6 +337,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do ...@@ -337,6 +337,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do
before do before do
chat_service.notify_only_default_branch = true chat_service.notify_only_default_branch = true
WebMock.stub_request(:post, webhook_url)
end end
it 'does not call the Slack/Mattermost API for pipeline events' do it 'does not call the Slack/Mattermost API for pipeline events' do
...@@ -345,6 +346,23 @@ RSpec.shared_examples 'slack or mattermost notifications' do ...@@ -345,6 +346,23 @@ RSpec.shared_examples 'slack or mattermost notifications' do
expect(result).to be_falsy expect(result).to be_falsy
end end
it 'does not notify push events if they are not for the default branch' do
ref = "#{Gitlab::Git::BRANCH_REF_PREFIX}test"
push_sample_data = Gitlab::DataBuilder::Push.build(project, user, nil, nil, ref, [])
chat_service.execute(push_sample_data)
expect(WebMock).not_to have_requested(:post, webhook_url)
end
it 'notifies about push events for the default branch' do
push_sample_data = Gitlab::DataBuilder::Push.build_sample(project, user)
chat_service.execute(push_sample_data)
expect(WebMock).to have_requested(:post, webhook_url).once
end
end end
context 'when disabled' do context 'when disabled' do
......
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