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 --- app/controllers/concerns/service_params.rb | 2 +- app/mailers/emails/notes.rb | 6 +++- .../concerns/notification_branch_selection.rb | 39 ++++++++++++++++++++++ app/models/project.rb | 1 + .../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 ++++++++++++------ app/views/layouts/nav/sidebar/_profile.html.haml | 2 +- 10 files changed, 85 insertions(+), 28 deletions(-) create mode 100644 app/models/concerns/notification_branch_selection.rb (limited to 'app') diff --git a/app/controllers/concerns/service_params.rb b/app/controllers/concerns/service_params.rb index b07ddd6f684..fd9d5fad38e 100644 --- a/app/controllers/concerns/service_params.rb +++ b/app/controllers/concerns/service_params.rb @@ -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, diff --git a/app/mailers/emails/notes.rb b/app/mailers/emails/notes.rb index c9a31b22207..51b6368a307 100644 --- a/app/mailers/emails/notes.rb +++ b/app/mailers/emails/notes.rb @@ -45,7 +45,11 @@ module Emails private def note_target_url_options - [@project || @group, @note.noteable, anchor: "note_#{@note.id}"] + [@project || @group, @note.noteable, note_target_url_query_params] + end + + def note_target_url_query_params + { anchor: "note_#{@note.id}" } end def note_thread_options(recipient_id, reason) diff --git a/app/models/concerns/notification_branch_selection.rb b/app/models/concerns/notification_branch_selection.rb new file mode 100644 index 00000000000..d8e18de7551 --- /dev/null +++ b/app/models/concerns/notification_branch_selection.rb @@ -0,0 +1,39 @@ +# 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 diff --git a/app/models/project.rb b/app/models/project.rb index 04d68d31812..96c715095f6 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1811,6 +1811,7 @@ class Project < ApplicationRecord .append(key: 'CI_PROJECT_NAMESPACE', value: namespace.full_path) .append(key: 'CI_PROJECT_URL', value: web_url) .append(key: 'CI_PROJECT_VISIBILITY', value: visibility) + .append(key: 'CI_PROJECT_REPOSITORY_LANGUAGES', value: repository_languages.map(&:name).join(',').downcase) .concat(pages_variables) .concat(container_registry_variables) .concat(auto_devops_variables) 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) diff --git a/app/views/layouts/nav/sidebar/_profile.html.haml b/app/views/layouts/nav/sidebar/_profile.html.haml index 7dd33f3c641..22ac9ee234d 100644 --- a/app/views/layouts/nav/sidebar/_profile.html.haml +++ b/app/views/layouts/nav/sidebar/_profile.html.haml @@ -111,7 +111,7 @@ = nav_link(controller: :gpg_keys) do = link_to profile_gpg_keys_path do .nav-icon-container - = sprite_icon('key-modern') + = sprite_icon('key') %span.nav-item-name = _('GPG Keys') %ul.sidebar-sub-level-items.is-fly-out-only -- cgit v1.2.3