Commit bca260ad authored by Robert Speicher's avatar Robert Speicher

Merge branch 'ee-slack-protected-branch-notification' into 'master'

[EE port]  Support notifications to be fired for protected branches also

See merge request gitlab-org/gitlab!16405
parents 1d197c06 3e12f61a
......@@ -10,6 +10,7 @@ module ServiceParams
:api_url,
:api_version,
:bamboo_url,
:branches_to_be_notified,
:build_key,
:build_type,
:ca_pem,
......@@ -41,7 +42,6 @@ module ServiceParams
:new_issue_url,
:notify,
:notify_only_broken_pipelines,
:notify_only_default_branch,
:password,
:priority,
:project_key,
......
# frozen_string_literal: true
# Concern handling functionality around deciding whether to send notification
# for activities on a specified branch or not. Will be included in
# ChatNotificationService and PipelinesEmailService classes.
module NotificationBranchSelection
extend ActiveSupport::Concern
BRANCH_CHOICES = [
[_('All branches'), 'all'],
[_('Default branch'), 'default'],
[_('Protected branches'), 'protected'],
[_('Default branch and protected branches'), 'default_and_protected']
].freeze
def notify_for_branch?(data)
ref = if data[:ref]
Gitlab::Git.ref_name(data[:ref])
else
data.dig(:object_attributes, :ref)
end
is_default_branch = ref == project.default_branch
is_protected_branch = project.protected_branches.exists?(name: ref)
case branches_to_be_notified
when "all"
true
when "default"
is_default_branch
when "protected"
is_protected_branch
when "default_and_protected"
is_default_branch || is_protected_branch
else
false
end
end
end
......@@ -4,6 +4,7 @@
# This class is not meant to be used directly, but only to inherit from.
class ChatNotificationService < Service
include ChatMessage
include NotificationBranchSelection
SUPPORTED_EVENTS = %w[
push issue confidential_issue merge_request note confidential_note
......@@ -14,7 +15,7 @@ class ChatNotificationService < Service
default_value_for :category, 'chat'
prop_accessor :webhook, :username, :channel
prop_accessor :webhook, :username, :channel, :branches_to_be_notified
# Custom serialized properties initialization
prop_accessor(*SUPPORTED_EVENTS.map { |event| EVENT_CHANNEL[event] })
......@@ -27,7 +28,16 @@ class ChatNotificationService < Service
if properties.nil?
self.properties = {}
self.notify_only_broken_pipelines = true
self.notify_only_default_branch = true
self.branches_to_be_notified = "default"
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
# `branches_to_be_notified`. Instead of doing a background migration, we
# opted to set a value for the new property based on the old one, if
# users hasn't specified one already. When users edit the service and
# selects a value for this new property, it will override everything.
self.branches_to_be_notified ||= notify_only_default_branch? ? "default" : "all"
end
end
......@@ -52,7 +62,7 @@ class ChatNotificationService < Service
{ type: 'text', name: 'webhook', placeholder: "e.g. #{webhook_placeholder}", required: true },
{ type: 'text', name: 'username', placeholder: 'e.g. GitLab' },
{ type: 'checkbox', name: 'notify_only_broken_pipelines' },
{ type: 'checkbox', name: 'notify_only_default_branch' }
{ type: 'select', name: 'branches_to_be_notified', choices: BRANCH_CHOICES }
]
end
......@@ -168,15 +178,8 @@ class ChatNotificationService < Service
def notify_for_ref?(data)
return true if data[:object_kind] == 'tag_push'
return true if data.dig(:object_attributes, :tag)
return true unless notify_only_default_branch?
ref = if data[:ref]
Gitlab::Git.ref_name(data[:ref])
else
data.dig(:object_attributes, :ref)
end
ref == project.default_branch
notify_for_branch?(data)
end
def notify_for_pipeline?(data)
......
......@@ -42,7 +42,7 @@ class DiscordService < ChatNotificationService
[
{ type: "text", name: "webhook", placeholder: "e.g. https://discordapp.com/api/webhooks/…" },
{ type: "checkbox", name: "notify_only_broken_pipelines" },
{ type: "checkbox", name: "notify_only_default_branch" }
{ type: 'select', name: 'branches_to_be_notified', choices: BRANCH_CHOICES }
]
end
......
......@@ -44,7 +44,7 @@ class HangoutsChatService < ChatNotificationService
[
{ type: 'text', name: 'webhook', placeholder: "e.g. #{webhook_placeholder}" },
{ type: 'checkbox', name: 'notify_only_broken_pipelines' },
{ type: 'checkbox', name: 'notify_only_default_branch' }
{ type: 'select', name: 'branches_to_be_notified', choices: BRANCH_CHOICES }
]
end
......
......@@ -42,7 +42,7 @@ class MicrosoftTeamsService < ChatNotificationService
[
{ type: 'text', name: 'webhook', placeholder: "e.g. #{webhook_placeholder}" },
{ type: 'checkbox', name: 'notify_only_broken_pipelines' },
{ type: 'checkbox', name: 'notify_only_default_branch' }
{ type: 'select', name: 'branches_to_be_notified', choices: BRANCH_CHOICES }
]
end
......
# frozen_string_literal: true
class PipelinesEmailService < Service
prop_accessor :recipients
include NotificationBranchSelection
prop_accessor :recipients, :branches_to_be_notified
boolean_accessor :notify_only_broken_pipelines, :notify_only_default_branch
validates :recipients, presence: true, if: :valid_recipients?
def initialize_properties
self.properties ||= { notify_only_broken_pipelines: true, notify_only_default_branch: false }
if properties.nil?
self.properties = {}
self.notify_only_broken_pipelines = true
self.branches_to_be_notified = "default"
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
# `branches_to_be_notified`. Instead of doing a background migration, we
# opted to set a value for the new property based on the old one, if
# users hasn't specified one already. When users edit the service and
# selects a value for this new property, it will override everything.
self.branches_to_be_notified ||= notify_only_default_branch? ? "default" : "all"
end
end
def title
......@@ -55,8 +70,9 @@ class PipelinesEmailService < Service
required: true },
{ type: 'checkbox',
name: 'notify_only_broken_pipelines' },
{ type: 'checkbox',
name: 'notify_only_default_branch' }
{ type: 'select',
name: 'branches_to_be_notified',
choices: BRANCH_CHOICES }
]
end
......@@ -69,13 +85,7 @@ class PipelinesEmailService < Service
end
def should_pipeline_be_notified?(data)
notify_for_pipeline_branch?(data) && notify_for_pipeline?(data)
end
def notify_for_pipeline_branch?(data)
return true unless notify_only_default_branch?
data[:object_attributes][:ref] == data[:project][:default_branch]
notify_for_branch?(data) && notify_for_pipeline?(data)
end
def notify_for_pipeline?(data)
......
---
title: Support chat notifications to be fired for protected branches
merge_request: 32176
author:
type: added
......@@ -436,7 +436,8 @@ Parameters:
| --------- | ---- | -------- | ----------- |
| `webhook` | string | true | The Hangouts Chat webhook. For example, `https://chat.googleapis.com/v1/spaces...`. |
| `notify_only_broken_pipelines` | boolean | false | Send notifications for broken pipelines |
| `notify_only_default_branch` | boolean | false | Send notifications only for the default branch |
| `notify_only_default_branch` | boolean | false | DEPRECATED: This parameter has been replaced with `branches_to_be_notified` |
| `branches_to_be_notified` | string | all | Branches to send notifications for. Valid options are "all", "default", "protected", and "default_and_protected" |
| `push_events` | boolean | false | Enable notifications for push events |
| `issues_events` | boolean | false | Enable notifications for issue events |
| `confidential_issues_events` | boolean | false | Enable notifications for confidential issue events |
......@@ -745,7 +746,8 @@ Parameters:
| `recipients` | string | yes | Comma-separated list of recipient email addresses |
| `add_pusher` | boolean | no | Add pusher to recipients list |
| `notify_only_broken_pipelines` | boolean | no | Notify only broken pipelines |
| `notify_only_default_branch` | boolean | no | Send notifications only for the default branch ([introduced in GitLab 12.0](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/28271)) |
| `notify_only_default_branch` | boolean | no | DEPRECATED: This parameter has been replaced with `branches_to_be_notified` ([introduced in GitLab 12.0](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/28271)) |
| `branches_to_be_notified` | string | all | Branches to send notifications for. Valid options are "all", "default", "protected", and "default_and_protected" |
| `pipeline_events` | boolean | false | Enable notifications for pipeline events |
### Delete Pipeline-Emails service
......@@ -933,7 +935,8 @@ Parameters:
| `username` | string | false | username |
| `channel` | string | false | Default channel to use if others are not configured |
| `notify_only_broken_pipelines` | boolean | false | Send notifications for broken pipelines |
| `notify_only_default_branch` | boolean | false | Send notifications only for the default branch |
| `notify_only_default_branch` | boolean | false | DEPRECATED: This parameter has been replaced with `branches_to_be_notified` |
| `branches_to_be_notified` | string | all | Branches to send notifications for. Valid options are "all", "default", "protected", and "default_and_protected" |
| `commit_events` | boolean | false | Enable notifications for commit events |
| `confidential_issue_channel` | string | false | The name of the channel to receive confidential issues events notifications |
| `confidential_issues_events` | boolean | false | Enable notifications for confidential issue events |
......@@ -991,7 +994,8 @@ Parameters:
| --------- | ---- | -------- | ----------- |
| `webhook` | string | true | The Microsoft Teams webhook. For example, `https://outlook.office.com/webhook/...` |
| `notify_only_broken_pipelines` | boolean | false | Send notifications for broken pipelines |
| `notify_only_default_branch` | boolean | false | Send notifications only for the default branch |
| `notify_only_default_branch` | boolean | false | DEPRECATED: This parameter has been replaced with `branches_to_be_notified` |
| `branches_to_be_notified` | string | all | Branches to send notifications for. Valid options are "all", "default", "protected", and "default_and_protected" |
| `push_events` | boolean | false | Enable notifications for push events |
| `issues_events` | boolean | false | Enable notifications for issue events |
| `confidential_issues_events` | boolean | false | Enable notifications for confidential issue events |
......@@ -1040,7 +1044,8 @@ Parameters:
| `username` | string | false | username |
| `channel` | string | false | Default channel to use if others are not configured |
| `notify_only_broken_pipelines` | boolean | false | Send notifications for broken pipelines |
| `notify_only_default_branch` | boolean | false | Send notifications only for the default branch |
| `notify_only_default_branch` | boolean | false | DEPRECATED: This parameter has been replaced with `branches_to_be_notified` |
| `branches_to_be_notified` | string | all | Branches to send notifications for. Valid options are "all", "default", "protected", and "default_and_protected" |
| `push_events` | boolean | false | Enable notifications for push events |
| `issues_events` | boolean | false | Enable notifications for issue events |
| `confidential_issues_events` | boolean | false | Enable notifications for confidential issue events |
......
......@@ -27,6 +27,12 @@ module API
name: :channel,
type: String,
desc: 'The default chat channel'
},
{
required: false,
name: :branches_to_be_notified,
type: String,
desc: 'Branches for which notifications are to be sent'
}
].freeze
end
......@@ -38,12 +44,6 @@ module API
name: :notify_only_broken_pipelines,
type: Boolean,
desc: 'Send notifications for broken pipelines'
},
{
required: false,
name: :notify_only_default_branch,
type: Boolean,
desc: 'Send notifications only for the default branch'
}
].freeze
end
......@@ -540,9 +540,9 @@ module API
},
{
required: false,
name: :notify_only_default_branch,
type: Boolean,
desc: 'Send notifications only for the default branch'
name: :branches_to_be_notified,
type: String,
desc: 'Branches for which notifications are to be sent'
}
],
'pivotaltracker' => [
......
......@@ -1228,6 +1228,9 @@ msgstr ""
msgid "All Members"
msgstr ""
msgid "All branches"
msgstr ""
msgid "All changes are committed"
msgstr ""
......@@ -4650,6 +4653,12 @@ msgstr ""
msgid "Default artifacts expiration"
msgstr ""
msgid "Default branch"
msgstr ""
msgid "Default branch and protected branches"
msgstr ""
msgid "Default classification label"
msgstr ""
......@@ -12343,6 +12352,9 @@ msgstr ""
msgid "Protected Tag"
msgstr ""
msgid "Protected branches"
msgstr ""
msgid "ProtectedEnvironment|%{environment_name} will be writable for developers. Are you sure?"
msgstr ""
......
......@@ -216,7 +216,7 @@ describe 'Admin updates settings' do
fill_in 'Username', with: 'test_user'
fill_in 'service_push_channel', with: '#test_channel'
page.check('Notify only broken pipelines')
page.check('Notify only default branch')
page.select 'All branches', from: 'Branches to be notified'
check_all_events
click_on 'Save'
......
......@@ -3,5 +3,5 @@
require 'spec_helper'
describe MattermostService do
it_behaves_like "slack or mattermost notifications"
it_behaves_like "slack or mattermost notifications", "Mattermost"
end
......@@ -226,9 +226,10 @@ describe MicrosoftTeamsService do
)
end
shared_examples 'call Microsoft Teams API' do
shared_examples 'call Microsoft Teams API' do |branches_to_be_notified: nil|
before do
WebMock.stub_request(:post, webhook_url)
chat_service.branches_to_be_notified = branches_to_be_notified if branches_to_be_notified
end
it 'calls Microsoft Teams API for pipeline events' do
......@@ -245,6 +246,18 @@ describe MicrosoftTeamsService do
end
end
shared_examples 'does not call Microsoft Teams API' do |branches_to_be_notified: nil|
before do
chat_service.branches_to_be_notified = branches_to_be_notified if branches_to_be_notified
end
it 'does not call Microsoft Teams API for pipeline events' do
data = Gitlab::DataBuilder::Pipeline.build(pipeline)
result = chat_service.execute(data)
expect(result).to be_falsy
end
end
context 'with failed pipeline' do
let(:status) { 'failed' }
......@@ -272,35 +285,73 @@ describe MicrosoftTeamsService do
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
context 'with default branch' do
let(:pipeline) do
create(:ci_pipeline, project: project, status: 'failed', sha: project.commit.sha, ref: project.default_branch)
end
before do
chat_service.notify_only_default_branch = true
end
context 'only notify for the default branch' do
it_behaves_like 'call Microsoft Teams API', branches_to_be_notified: "default"
end
it 'does not call the Microsoft Teams API for pipeline events' do
data = Gitlab::DataBuilder::Pipeline.build(pipeline)
result = chat_service.execute(data)
context 'notify for only protected branches' do
it_behaves_like 'does not call Microsoft Teams API', branches_to_be_notified: "protected"
end
expect(result).to be_falsy
end
context 'notify for only default and protected branches' do
it_behaves_like 'call Microsoft Teams API', branches_to_be_notified: "default_and_protected"
end
context 'when disabled' do
let(:pipeline) do
create(:ci_pipeline, :failed, project: project,
sha: project.commit.sha, ref: 'not-the-default-branch')
end
context 'notify for all branches' do
it_behaves_like 'call Microsoft Teams API', branches_to_be_notified: "all"
end
end
before do
chat_service.notify_only_default_branch = false
end
context 'with protected branch' do
before do
create(:protected_branch, project: project, name: 'a-protected-branch')
end
it_behaves_like 'call Microsoft Teams API'
let(:pipeline) do
create(:ci_pipeline, project: project, status: 'failed', sha: project.commit.sha, ref: 'a-protected-branch')
end
context 'only notify for the default branch' do
it_behaves_like 'does not call Microsoft Teams API', branches_to_be_notified: "default"
end
context 'notify for only protected branches' do
it_behaves_like 'call Microsoft Teams API', branches_to_be_notified: "protected"
end
context 'notify for only default and protected branches' do
it_behaves_like 'call Microsoft Teams API', branches_to_be_notified: "default_and_protected"
end
context 'notify for all branches' do
it_behaves_like 'call Microsoft Teams API', branches_to_be_notified: "all"
end
end
context 'with neither protected nor default branch' do
let(:pipeline) do
create(:ci_pipeline, project: project, status: 'failed', sha: project.commit.sha, ref: 'a-random-branch')
end
context 'only notify for the default branch' do
it_behaves_like 'does not call Microsoft Teams API', branches_to_be_notified: "default"
end
context 'notify for only protected branches' do
it_behaves_like 'does not call Microsoft Teams API', branches_to_be_notified: "protected"
end
context 'notify for only default and protected branches' do
it_behaves_like 'does not call Microsoft Teams API', branches_to_be_notified: "default_and_protected"
end
context 'notify for all branches' do
it_behaves_like 'call Microsoft Teams API', branches_to_be_notified: "all"
end
end
end
......
......@@ -53,9 +53,10 @@ describe PipelinesEmailService, :mailer do
end
end
shared_examples 'sending email' do
shared_examples 'sending email' do |branches_to_be_notified: nil|
before do
subject.recipients = recipients
subject.branches_to_be_notified = branches_to_be_notified if branches_to_be_notified
perform_enqueued_jobs do
run
......@@ -69,9 +70,10 @@ describe PipelinesEmailService, :mailer do
end
end
shared_examples 'not sending email' do
shared_examples 'not sending email' do |branches_to_be_notified: nil|
before do
subject.recipients = recipients
subject.branches_to_be_notified = branches_to_be_notified if branches_to_be_notified
perform_enqueued_jobs do
run
......@@ -101,27 +103,84 @@ describe PipelinesEmailService, :mailer do
it_behaves_like 'sending email'
end
context 'when pipeline is failed and on a non-default branch' do
before do
data[:object_attributes][:ref] = 'not-the-default-branch'
pipeline.update(ref: 'not-the-default-branch')
context 'when the pipeline failed' do
context 'on default branch' do
before do
data[:object_attributes][:ref] = project.default_branch
pipeline.update(ref: project.default_branch)
end
context 'notifications are enabled only for default branch' do
it_behaves_like 'sending email', branches_to_be_notified: "default"
end
context 'notifications are enabled only for protected branch' do
it_behaves_like 'sending email', branches_to_be_notified: "protected"
end
context 'notifications are enabled only for default and protected branches ' do
it_behaves_like 'sending email', branches_to_be_notified: "default_and_protected"
end
context 'notifications are enabled only for all branches' do
it_behaves_like 'sending email', branches_to_be_notified: "all"
end
end
context 'with notify_only_default branch on' do
context 'on a protected branch' do
before do
subject.notify_only_default_branch = true
create(:protected_branch, project: project, name: 'a-protected-branch')
data[:object_attributes][:ref] = 'a-protected-branch'
pipeline.update(ref: 'a-protected-branch')
end
it_behaves_like 'sending email'
context 'notifications are enabled only for default branch' do
it_behaves_like 'sending email', branches_to_be_notified: "default"
end
context 'notifications are enabled only for protected branch' do
it_behaves_like 'sending email', branches_to_be_notified: "protected"
end
context 'notifications are enabled only for default and protected branches ' do
it_behaves_like 'sending email', branches_to_be_notified: "default_and_protected"
end
context 'notifications are enabled only for all branches' do
it_behaves_like 'sending email', branches_to_be_notified: "all"
end
end
context 'with notify_only_default_branch off' do
it_behaves_like 'sending email'
context 'on a neither protected nor default branch' do
before do
data[:object_attributes][:ref] = 'a-random-branch'
pipeline.update(ref: 'a-random-branch')
end
context 'notifications are enabled only for default branch' do
it_behaves_like 'sending email', branches_to_be_notified: "default"
end
context 'notifications are enabled only for protected branch' do
it_behaves_like 'sending email', branches_to_be_notified: "protected"
end
context 'notifications are enabled only for default and protected branches ' do
it_behaves_like 'sending email', branches_to_be_notified: "default_and_protected"
end
context 'notifications are enabled only for all branches' do
it_behaves_like 'sending email', branches_to_be_notified: "all"
end
end
end
end
describe '#execute' do
before do
subject.project = project
end
def run
subject.execute(data)
end
......@@ -159,37 +218,75 @@ describe PipelinesEmailService, :mailer do
end
end
context 'with notify_only_default_branch off' do
context 'with default branch' do
it_behaves_like 'sending email'
context 'when the pipeline failed' do
context 'on default branch' do
before do
data[:object_attributes][:ref] = project.default_branch
pipeline.update(ref: project.default_branch)
end
context 'notifications are enabled only for default branch' do
it_behaves_like 'sending email', branches_to_be_notified: "default"
end
context 'notifications are enabled only for protected branch' do
it_behaves_like 'not sending email', branches_to_be_notified: "protected"
end
context 'notifications are enabled only for default and protected branches ' do
it_behaves_like 'sending email', branches_to_be_notified: "default_and_protected"
end
context 'notifications are enabled only for all branches' do
it_behaves_like 'sending email', branches_to_be_notified: "all"
end
end
context 'with non default branch' do
context 'on a protected branch' do
before do
data[:object_attributes][:ref] = 'not-the-default-branch'
pipeline.update(ref: 'not-the-default-branch')
create(:protected_branch, project: project, name: 'a-protected-branch')
data[:object_attributes][:ref] = 'a-protected-branch'
pipeline.update(ref: 'a-protected-branch')
end
it_behaves_like 'sending email'
end
end
context 'notifications are enabled only for default branch' do
it_behaves_like 'not sending email', branches_to_be_notified: "default"
end
context 'with notify_only_default_branch on' do
before do
subject.notify_only_default_branch = true
end
context 'notifications are enabled only for protected branch' do
it_behaves_like 'sending email', branches_to_be_notified: "protected"
end
context 'with default branch' do
it_behaves_like 'sending email'
context 'notifications are enabled only for default and protected branches ' do
it_behaves_like 'sending email', branches_to_be_notified: "default_and_protected"
end
context 'notifications are enabled only for all branches' do
it_behaves_like 'sending email', branches_to_be_notified: "all"
end
end
context 'with non default branch' do
context 'on a neither protected nor default branch' do
before do
data[:object_attributes][:ref] = 'not-the-default-branch'
pipeline.update(ref: 'not-the-default-branch')
data[:object_attributes][:ref] = 'a-random-branch'
pipeline.update(ref: 'a-random-branch')
end
it_behaves_like 'not sending email'
context 'notifications are enabled only for default branch' do
it_behaves_like 'not sending email', branches_to_be_notified: "default"
end
context 'notifications are enabled only for protected branch' do
it_behaves_like 'not sending email', branches_to_be_notified: "protected"
end
context 'notifications are enabled only for default and protected branches ' do
it_behaves_like 'not sending email', branches_to_be_notified: "default_and_protected"
end
context 'notifications are enabled only for all branches' do
it_behaves_like 'sending email', branches_to_be_notified: "all"
end
end
end
end
......
......@@ -3,5 +3,5 @@
require 'spec_helper'
describe SlackService do
it_behaves_like "slack or mattermost notifications"
it_behaves_like "slack or mattermost notifications", "Slack"
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