diff options
author | Sean McGivern <sean@mcgivern.me.uk> | 2018-03-02 16:04:53 +0000 |
---|---|---|
committer | Sean McGivern <sean@mcgivern.me.uk> | 2018-03-02 16:04:53 +0000 |
commit | 9b704ef327cc0224bf09c1e8d8d27df88ab13734 (patch) | |
tree | 0fac87f39fb4bc718d60124af8fcf7bb1edc331f | |
parent | a519a346da300972a0061fb341b44afd6d2b857a (diff) | |
parent | 158514bba3445b1a8303c6a4d5d7ba2403ef8a9f (diff) | |
download | gitlab-ce-9b704ef327cc0224bf09c1e8d8d27df88ab13734.tar.gz |
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
3 files changed, 32 insertions, 3 deletions
diff --git a/app/models/project_services/chat_notification_service.rb b/app/models/project_services/chat_notification_service.rb index 818cfb01b14..e2ffdf72201 100644 --- a/app/models/project_services/chat_notification_service.rb +++ b/app/models/project_services/chat_notification_service.rb @@ -99,7 +99,7 @@ class ChatNotificationService < Service def get_message(object_kind, data) case object_kind when "push", "tag_push" - ChatMessage::PushMessage.new(data) + ChatMessage::PushMessage.new(data) if notify_for_ref?(data) when "issue" ChatMessage::IssueMessage.new(data) unless update?(data) when "merge_request" @@ -145,10 +145,16 @@ class ChatNotificationService < Service end 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? - 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 def notify_for_pipeline?(data) diff --git a/changelogs/unreleased/33570-slack-notify-default-branch.yml b/changelogs/unreleased/33570-slack-notify-default-branch.yml new file mode 100644 index 00000000000..5c90ce47729 --- /dev/null +++ b/changelogs/unreleased/33570-slack-notify-default-branch.yml @@ -0,0 +1,5 @@ +--- +title: Fix Slack/Mattermost notifications not respecting `notify_only_default_branch` setting for pushes +merge_request: 17345 +author: +type: fixed diff --git a/spec/support/slack_mattermost_notifications_shared_examples.rb b/spec/support/slack_mattermost_notifications_shared_examples.rb index e827a8da0b7..5e1ce19eafb 100644 --- a/spec/support/slack_mattermost_notifications_shared_examples.rb +++ b/spec/support/slack_mattermost_notifications_shared_examples.rb @@ -337,6 +337,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do before do chat_service.notify_only_default_branch = true + WebMock.stub_request(:post, webhook_url) end it 'does not call the Slack/Mattermost API for pipeline events' do @@ -345,6 +346,23 @@ RSpec.shared_examples 'slack or mattermost notifications' do expect(result).to be_falsy 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 context 'when disabled' do |