From 40d3d574132d2849646c20eb9d8742b159d9bfc8 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Fri, 13 Sep 2019 18:06:03 +0000 Subject: Add latest changes from gitlab-org/gitlab@master --- .../project_services/chat_notification_service.rb | 25 +++++++++-------- app/models/project_services/discord_service.rb | 2 +- .../project_services/hangouts_chat_service.rb | 2 +- .../project_services/microsoft_teams_service.rb | 2 +- .../project_services/pipelines_email_service.rb | 32 ++++++++++++++-------- 5 files changed, 38 insertions(+), 25 deletions(-) (limited to 'app/models/project_services') diff --git a/app/models/project_services/chat_notification_service.rb b/app/models/project_services/chat_notification_service.rb index cb75c89136e..ecea1a5b630 100644 --- a/app/models/project_services/chat_notification_service.rb +++ b/app/models/project_services/chat_notification_service.rb @@ -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) diff --git a/app/models/project_services/discord_service.rb b/app/models/project_services/discord_service.rb index 4385834ed0a..ff1a9552a6c 100644 --- a/app/models/project_services/discord_service.rb +++ b/app/models/project_services/discord_service.rb @@ -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 diff --git a/app/models/project_services/hangouts_chat_service.rb b/app/models/project_services/hangouts_chat_service.rb index 699cf1659d1..d105bd012d6 100644 --- a/app/models/project_services/hangouts_chat_service.rb +++ b/app/models/project_services/hangouts_chat_service.rb @@ -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 diff --git a/app/models/project_services/microsoft_teams_service.rb b/app/models/project_services/microsoft_teams_service.rb index 2334b3f7f66..5cabce1376b 100644 --- a/app/models/project_services/microsoft_teams_service.rb +++ b/app/models/project_services/microsoft_teams_service.rb @@ -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 diff --git a/app/models/project_services/pipelines_email_service.rb b/app/models/project_services/pipelines_email_service.rb index ae5d5038099..8452239129d 100644 --- a/app/models/project_services/pipelines_email_service.rb +++ b/app/models/project_services/pipelines_email_service.rb @@ -1,12 +1,27 @@ # 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) -- cgit v1.2.3