Commit 40d3d574 authored by GitLab Bot's avatar GitLab Bot

Add latest changes from gitlab-org/gitlab@master

parent 5939b09f
......@@ -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,
......
......@@ -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)
......
# 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
......@@ -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)
......
......@@ -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)
......
......@@ -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
......
---
title: Support chat notifications to be fired for protected branches
merge_request: 32176
author:
type: added
......@@ -6,9 +6,9 @@ class AddTargetProjectIdToMergeTrains < ActiveRecord::Migration[5.1]
DOWNTIME = false
def change
# rubocop: disable Rails/NotNullColumn
# rubocop:disable Rails/NotNullColumn, Migration/AddReference
add_reference :merge_trains, :target_project, null: false, index: true, foreign_key: { on_delete: :cascade, to_table: :projects }, type: :integer
add_column :merge_trains, :target_branch, :text, null: false
# rubocop: enable Rails/NotNullColumn
# rubocop:enable Rails/NotNullColumn, Migration/AddReference
end
end
......@@ -4,7 +4,9 @@ class AddEnvironmentIdToClustersKubernetesNamespaces < ActiveRecord::Migration[5
DOWNTIME = false
def change
# rubocop:disable Migration/AddReference
add_reference :clusters_kubernetes_namespaces, :environment,
index: true, type: :bigint, foreign_key: { on_delete: :nullify }
# rubocop:enable Migration/AddReference
end
end
......@@ -4,7 +4,9 @@ class AddIssueIdToVersions < ActiveRecord::Migration[5.2]
DOWNTIME = false
def up
# rubocop:disable Migration/AddReference
add_reference :design_management_versions, :issue, index: true, foreign_key: { on_delete: :cascade }
# rubocop:enable Migration/AddReference
end
def down
......
......@@ -4,11 +4,13 @@ class AddColumnForSelfMonitoringProjectId < ActiveRecord::Migration[5.2]
DOWNTIME = false
def change
# rubocop:disable Migration/AddReference
add_reference(
:application_settings,
:instance_administration_project,
index: { name: 'index_applicationsettings_on_instance_administration_project_id' },
foreign_key: { to_table: :projects, on_delete: :nullify }
)
# rubocop:enable Migration/AddReference
end
end
......@@ -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 |
......
......@@ -88,6 +88,7 @@ future GitLab releases.**
| `CI_PROJECT_PATH_SLUG` | 9.3 | all | `$CI_PROJECT_PATH` lowercased and with everything except `0-9` and `a-z` replaced with `-`. Use in URLs and domain names. |
| `CI_PROJECT_URL` | 8.10 | 0.5 | The HTTP(S) address to access project |
| `CI_PROJECT_VISIBILITY` | 10.3 | all | The project visibility (internal, private, public) |
| `CI_PROJECT_REPOSITORY_LANGUAGES` | 12.3 | all | Comma-separated, lowercased list of the languages used in the repository (e.g. `ruby,javascript,html,css`) |
| `CI_COMMIT_REF_PROTECTED` | 11.11 | all | If the job is running on a protected branch |
| `CI_REGISTRY` | 8.10 | 0.5 | If the Container Registry is enabled it returns the address of GitLab's Container Registry |
| `CI_REGISTRY_IMAGE` | 8.10 | 0.5 | If the Container Registry is enabled for the project it returns the address of the registry tied to the specific project |
......
......@@ -49,10 +49,14 @@ After making your changes, click the **Commit** button in the bottom left to
review the list of changed files. Click on each file to review the changes and
click the tick icon to stage the file.
Once you have staged some changes, you can add a commit message and commit the
staged changes. Unstaged changes will not be committed.
![Review changes](img/review_changes_v12_3.png)
![Commit changes](img/commit_changes.png)
Once you have staged some changes, you can add a commit message, commit the
staged changes and directly create a merge request. In case you don't have write
access to the selected branch, you will see a warning, but still be able to create
a new branch and start a merge request.
![Commit changes](img/commit_changes_v12_3.png)
## Reviewing changes
......
......@@ -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 ""
......
......@@ -6,7 +6,7 @@ module QA
attr_accessor :name, :expires_at
attribute :username do
Page::Project::Settings::Repository.perform do |page|
Page::Project::Settings::Repository.perform do |page| # rubocop:disable QA/AmbiguousPageObjectName
page.expand_deploy_tokens do |token|
token.token_username
end
......@@ -14,7 +14,7 @@ module QA
end
attribute :password do
Page::Project::Settings::Repository.perform do |page|
Page::Project::Settings::Repository.perform do |page| # rubocop:disable QA/AmbiguousPageObjectName
page.expand_deploy_tokens do |token|
token.token_password
end
......
......@@ -27,7 +27,7 @@ module QA
Page::Project::Show.perform(&:create_first_new_file!)
Page::File::Form.perform do |page|
Page::File::Form.perform do |page| # rubocop:disable QA/AmbiguousPageObjectName
page.add_name(@name)
page.add_content(@content)
page.add_commit_message(@commit_message)
......
......@@ -41,7 +41,7 @@ module QA
fork_new.choose_namespace(user.name)
end
Page::Layout::Banner.perform do |page|
Page::Layout::Banner.perform do |page| # rubocop:disable QA/AmbiguousPageObjectName
page.has_notice?('The project was successfully forked.')
end
......
......@@ -25,7 +25,7 @@ module QA
Page::Project::Show.perform(&:go_to_new_issue)
Page::Project::Issue::New.perform do |page|
Page::Project::Issue::New.perform do |page| # rubocop:disable QA/AmbiguousPageObjectName
page.add_title(@title)
page.add_description(@description)
page.create_new_issue
......
......@@ -24,7 +24,7 @@ module QA
Page::Project::Operations::Kubernetes::Add.perform(
&:add_existing_cluster)
Page::Project::Operations::Kubernetes::AddExisting.perform do |page|
Page::Project::Operations::Kubernetes::AddExisting.perform do |page| # rubocop:disable QA/AmbiguousPageObjectName
page.set_cluster_name(@cluster.cluster_name)
page.set_api_url(@cluster.api_url)
page.set_ca_certificate(@cluster.ca_certificate)
......@@ -34,7 +34,7 @@ module QA
end
if @install_helm_tiller
Page::Project::Operations::Kubernetes::Show.perform do |page|
Page::Project::Operations::Kubernetes::Show.perform do |page| # rubocop:disable QA/AmbiguousPageObjectName
# We must wait a few seconds for permissions to be set up correctly for new cluster
sleep 10
......
......@@ -28,7 +28,7 @@ module QA
Page::Project::Menu.perform(&:go_to_labels)
Page::Label::Index.perform(&:click_new_label_button)
Page::Label::New.perform do |page|
Page::Label::New.perform do |page| # rubocop:disable QA/AmbiguousPageObjectName
page.fill_title(@title)
page.fill_description(@description)
page.fill_color(@color)
......
......@@ -16,7 +16,7 @@ module QA
Page::Main::Menu.perform(&:click_settings_link)
Page::Profile::Menu.perform(&:click_access_tokens)
Page::Profile::PersonalAccessTokens.perform do |page|
Page::Profile::PersonalAccessTokens.perform do |page| # rubocop:disable QA/AmbiguousPageObjectName
page.fill_token_name(name || 'api-test-token')
page.check_api
page.click_create_token_button
......
......@@ -30,13 +30,13 @@ module QA
end
attribute :repository_ssh_location do
Page::Project::Show.perform do |page|
Page::Project::Show.perform do |page| # rubocop:disable QA/AmbiguousPageObjectName
page.repository_clone_ssh_location
end
end
attribute :repository_http_location do
Page::Project::Show.perform do |page|
Page::Project::Show.perform do |page| # rubocop:disable QA/AmbiguousPageObjectName
page.repository_clone_http_location
end
end
......@@ -59,7 +59,7 @@ module QA
Page::Group::Show.perform(&:go_to_new_project)
end
Page::Project::New.perform do |page|
Page::Project::New.perform do |page| # rubocop:disable QA/AmbiguousPageObjectName
page.choose_test_namespace
page.choose_name(@name)
page.add_description(@description)
......
......@@ -17,15 +17,15 @@ module QA
Page::Group::Show.perform(&:go_to_new_project)
Page::Project::New.perform do |page|
Page::Project::New.perform do |page| # rubocop:disable QA/AmbiguousPageObjectName
page.click_import_project
end
Page::Project::New.perform do |page|
Page::Project::New.perform do |page| # rubocop:disable QA/AmbiguousPageObjectName
page.click_github_link
end
Page::Project::Import::Github.perform do |page|
Page::Project::Import::Github.perform do |page| # rubocop:disable QA/AmbiguousPageObjectName
page.add_personal_access_token(@personal_access_token)
page.list_repos
page.import!(@github_repository_path, @name)
......
......@@ -18,7 +18,7 @@ module QA
def fabricate!
project.visit!
Page::Project::Menu.perform do |page|
Page::Project::Menu.perform do |page| # rubocop:disable QA/AmbiguousPageObjectName
page.click_issues
page.click_milestones
end
......
......@@ -18,11 +18,11 @@ module QA
def fabricate!
Page::Main::Menu.perform(&:go_to_groups)
Page::Dashboard::Groups.perform do |page|
if page.has_group?(path)
page.click_group(path)
Page::Dashboard::Groups.perform do |groups_page|
if groups_page.has_group?(path)
groups_page.click_group(path)
else
page.click_new_group
groups_page.click_new_group
Page::Group::New.perform do |group|
group.set_path(path)
......
......@@ -16,7 +16,7 @@ module QA
def fabricate!
Page::Dashboard::Snippet::Index.perform(&:go_to_new_snippet_page)
Page::Dashboard::Snippet::New.perform do |page|
Page::Dashboard::Snippet::New.perform do |page| # rubocop:disable QA/AmbiguousPageObjectName
page.fill_title(@title)
page.fill_description(@description)
page.set_visibility(@visibility)
......
......@@ -17,7 +17,7 @@ module QA
Page::Main::Menu.perform(&:click_settings_link)
Page::Profile::Menu.perform(&:click_ssh_keys)
Page::Profile::SSHKeys.perform do |page|
Page::Profile::SSHKeys.perform do |page| # rubocop:disable QA/AmbiguousPageObjectName
page.add_key(public_key, title)
end
end
......
......@@ -8,7 +8,7 @@ module QA
Page::Main::Login.perform(&:sign_in_using_credentials)
Page::Main::Menu.perform(&:go_to_groups)
Page::Dashboard::Groups.perform do |page|
Page::Dashboard::Groups.perform do |page| # rubocop:disable QA/AmbiguousPageObjectName
page.click_new_group
expect(page).to have_content(
......
......@@ -11,7 +11,7 @@ module QA
Runtime::Browser.visit(:mattermost, Page::Mattermost::Login)
Page::Mattermost::Login.perform(&:sign_in_using_oauth)
Page::Mattermost::Main.perform do |page|
Page::Mattermost::Main.perform do |page| # rubocop:disable QA/AmbiguousPageObjectName
expect(page).to have_content(/(Welcome to: Mattermost|Logout GitLab Mattermost)/)
end
end
......
......@@ -15,7 +15,7 @@ module QA
project.visit!
Page::Project::Menu.perform(&:go_to_members_settings)
Page::Project::Settings::Members.perform do |page|
Page::Project::Settings::Members.perform do |page| # rubocop:disable QA/AmbiguousPageObjectName
page.add_member(user.username)
end
......
......@@ -30,7 +30,7 @@ module QA
project.visit!
Page::Project::Show.perform(&:go_to_members_settings)
Page::Project::Settings::Members.perform do |page|
Page::Project::Settings::Members.perform do |page| # rubocop:disable QA/AmbiguousPageObjectName
page.add_member(user.username)
end
......
......@@ -57,7 +57,7 @@ module QA
@project.visit!
Page::Project::Show.perform(&:create_new_file!)
Page::File::Form.perform do |page|
Page::File::Form.perform do |page| # rubocop:disable QA/AmbiguousPageObjectName
page.select_template template[:file_name], template[:name]
end
......
......@@ -35,7 +35,7 @@ module QA
def view_commit
@project.visit!
Page::Project::Show.perform do |page|
Page::Project::Show.perform do |page| # rubocop:disable QA/AmbiguousPageObjectName
page.click_commit(@commit_message)
end
end
......
......@@ -57,7 +57,7 @@ module QA
@project.visit!
Page::Project::Show.perform(&:open_web_ide!)
Page::Project::WebIDE::Edit.perform do |page|
Page::Project::WebIDE::Edit.perform do |page| # rubocop:disable QA/AmbiguousPageObjectName
page.create_new_file_from_template template[:file_name], template[:name]
expect(page.has_file?(template[:file_name])).to be_truthy
......
......@@ -16,7 +16,7 @@ module QA
validate_content('My First Wiki Content')
Page::Project::Wiki::Edit.perform(&:click_edit)
Page::Project::Wiki::New.perform do |page|
Page::Project::Wiki::New.perform do |page| # rubocop:disable QA/AmbiguousPageObjectName
page.set_content("My Second Wiki Content")
page.save_changes
end
......
......@@ -23,7 +23,7 @@ module QA
issue.title = 'Performance bar test'
end
Page::Layout::PerformanceBar.perform do |page|
Page::Layout::PerformanceBar.perform do |page| # rubocop:disable QA/AmbiguousPageObjectName
expect(page).to have_performance_bar
expect(page).to have_detailed_metrics
expect(page).to have_request_for('realtime_changes') # Always requested on issue pages
......
......@@ -32,7 +32,7 @@ module QA
token_name = 'api-test-token'
Page::Profile::PersonalAccessTokens.perform do |page|
Page::Profile::PersonalAccessTokens.perform do |page| # rubocop:disable QA/AmbiguousPageObjectName
while page.has_token_row_for_name?(token_name)
page.revoke_first_token_with_name(token_name)
print "\e[32m.\e[0m"
......
......@@ -4,22 +4,62 @@ require_relative '../../migration_helpers'
module RuboCop
module Cop
module Migration
# Cop that checks if a foreign key constraint is added and require a index for it
# add_reference can only be used with newly created tables.
# Additionally, the cop here checks that we create an index for the foreign key, too.
class AddReference < RuboCop::Cop::Cop
include MigrationHelpers
MSG = '`add_reference` requires `index: true` or `index: { options... }`'
MSG = '`add_reference` requires downtime for existing tables, use `add_concurrent_foreign_key` instead. When used for new tables, `index: true` or `index: { options... } is required.`'
def on_send(node)
def on_def(node)
return unless in_migration?(node)
name = node.children[1]
new_tables = []
return unless name == :add_reference
node.each_descendant(:send) do |send_node|
first_arg = first_argument(send_node)
# The first argument of "create_table" / "add_reference" is the table
# name.
new_tables << first_arg if create_table?(send_node)
next if method_name(send_node) != :add_reference
# Using "add_reference" is fine for newly created tables as there's no
# data in these tables yet.
if existing_table?(new_tables, first_arg)
add_offense(send_node, location: :selector)
end
# We require an index on the foreign key column.
if index_missing?(node)
add_offense(send_node, location: :selector)
end
end
end
private
def existing_table?(new_tables, table)
!new_tables.include?(table)
end
def create_table?(node)
method_name(node) == :create_table
end
def method_name(node)
node.children[1]
end
def first_argument(node)
node.children[2]
end
def index_missing?(node)
opts = node.children.last
add_offense(node, location: :selector) unless opts && opts.type == :hash
return true if opts && opts.type == :hash
index_present = false
......@@ -27,11 +67,9 @@ module RuboCop
index_present ||= index_enabled?(pair)
end
add_offense(node, location: :selector) unless index_present
!index_present
end
private
def index_enabled?(pair)
return unless hash_key_type(pair) == :sym
return unless hash_key_name(pair) == :index
......
# frozen_string_literal: true
require_relative '../../qa_helpers'
module RuboCop
module Cop
module QA
# This cop checks for the usage of the ambiguous name "page"
#
# @example
#
# # bad
# Page::Object.perform do |page| do ...
# Page::Another.perform { |page| ... }
#
# # good
# Page::Object.perform do |object| do ...
# Page::Another.perform { |another| ... }
class AmbiguousPageObjectName < RuboCop::Cop::Cop
include QAHelpers
MESSAGE = "Don't use 'page' as a name for a Page Object. Use `%s` instead.".freeze
def_node_matcher :ambiguous_page?, <<~PATTERN
(block
(send
(const ...) :perform)
(args
(arg :page)) ...)
PATTERN
def on_block(node)
return unless in_qa_file?(node)
return unless ambiguous_page?(node)
add_offense(node.arguments.each_node(:arg).first,
message: MESSAGE % page_object_name(node))
end
private
def page_object_name(node)
node.send_node.children[-2].const_name.downcase.split('::').last
end
end
end
end
end
......@@ -36,6 +36,7 @@ require_relative 'cop/rspec/env_assignment'
require_relative 'cop/rspec/factories_in_migration_specs'
require_relative 'cop/rspec/top_level_describe_path'
require_relative 'cop/qa/element_with_pattern'
require_relative 'cop/qa/ambiguous_page_object_name'
require_relative 'cop/sidekiq_options_queue'
require_relative 'cop/scalability/file_uploads'
require_relative 'cop/destroy_all'
......
......@@ -35,30 +35,16 @@ fi
# Do not use 'README.md', instead use 'index.md'
# Number of 'README.md's as of 2018-03-26
NUMBER_READMES_CE=46
NUMBER_READMES_EE=46
NUMBER_READMES=46
FIND_READMES=$(find doc/ -name "README.md" | wc -l)
echo '=> Checking for new README.md files...'
if [ "${CI_PROJECT_NAME}" == 'gitlab-ce' ]
if [ ${FIND_READMES} -ne $NUMBER_READMES ]
then
if [ ${FIND_READMES} -ne ${NUMBER_READMES_CE} ]
then
echo
echo ' ✖ ERROR: New README.md file(s) detected, prefer index.md over README.md.' >&2
echo ' https://docs.gitlab.com/ee/development/documentation/styleguide.html#working-with-directories-and-files'
echo
exit 1
fi
elif [ "${CI_PROJECT_NAME}" == 'gitlab-ee' ]
then
if [ ${FIND_READMES} -ne $NUMBER_READMES_EE ]
then
echo
echo ' ✖ ERROR: New README.md file(s) detected, prefer index.md over README.md.' >&2
echo ' https://docs.gitlab.com/ee/development/documentation/styleguide.html#working-with-directories-and-files'
echo
exit 1
fi
echo
echo ' ✖ ERROR: New README.md file(s) detected, prefer index.md over README.md.' >&2
echo ' https://docs.gitlab.com/ee/development/documentation/styleguide.html#working-with-directories-and-files'
echo
exit 1
fi
echo "✔ Linting passed"
......
......@@ -69,9 +69,9 @@ end
#
def slug
case ENV["CI_PROJECT_NAME"]
when 'gitlab-ce'
when 'gitlab-foss'
'ce'
when 'gitlab-ee'
when 'gitlab'
'ee'
when 'gitlab-runner'
'runner'
......
......@@ -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'
......
......@@ -2215,6 +2215,7 @@ describe Ci::Build do
{ key: 'CI_PROJECT_NAMESPACE', value: project.namespace.full_path, public: true, masked: false },
{ key: 'CI_PROJECT_URL', value: project.web_url, public: true, masked: false },
{ key: 'CI_PROJECT_VISIBILITY', value: 'private', public: true, masked: false },
{ key: 'CI_PROJECT_REPOSITORY_LANGUAGES', value: project.repository_languages.map(&:name).join(',').downcase, public: true, masked: false },
{ key: 'CI_PAGES_DOMAIN', value: Gitlab.config.pages.host, public: true, masked: false },
{ key: 'CI_PAGES_URL', value: project.pages_url, public: true, masked: false },
{ key: 'CI_API_V4_URL', value: 'http://localhost/api/v4', public: true, masked: false },
......
......@@ -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
......@@ -25,38 +25,86 @@ describe RuboCop::Cop::Migration::AddReference do
allow(cop).to receive(:in_migration?).and_return(true)
end
it 'registers an offense when using add_reference without index' do
expect_offense(<<~RUBY)
call do
add_reference(:projects, :users)
^^^^^^^^^^^^^ `add_reference` requires `index: true` or `index: { options... }`
let(:offense) { '`add_reference` requires downtime for existing tables, use `add_concurrent_foreign_key` instead. When used for new tables, `index: true` or `index: { options... } is required.`' }
context 'when the table existed before' do
it 'registers an offense when using add_reference' do
expect_offense(<<~RUBY)
def up
add_reference(:projects, :users)
^^^^^^^^^^^^^ #{offense}
end
RUBY
end
it 'registers an offense when using add_reference with index enabled' do
expect_offense(<<~RUBY)
def up
add_reference(:projects, :users, index: true)
^^^^^^^^^^^^^ #{offense}
end
RUBY
RUBY
end
it 'registers an offense if only a different table was created' do
expect_offense(<<~RUBY)
def up
create_table(:foo) do |t|
t.string :name
end
add_reference(:projects, :users, index: true)
^^^^^^^^^^^^^ #{offense}
end
RUBY
end
end
it 'registers an offense when using add_reference index disabled' do
expect_offense(<<~RUBY)
context 'when creating the table at the same time' do
let(:create_table_statement) do
<<~RUBY
create_table(:projects) do |t|
t.string :name
end
RUBY
end
it 'registers an offense when using add_reference without index' do
expect_offense(<<~RUBY)
def up
#{create_table_statement}
add_reference(:projects, :users)
^^^^^^^^^^^^^ #{offense}
end
RUBY
end
it 'registers an offense when using add_reference index disabled' do
expect_offense(<<~RUBY)
def up
#{create_table_statement}
add_reference(:projects, :users, index: false)
^^^^^^^^^^^^^ `add_reference` requires `index: true` or `index: { options... }`
^^^^^^^^^^^^^ #{offense}
end
RUBY
end
RUBY
end
it 'does not register an offense when using add_reference with index enabled' do
expect_no_offenses(<<~RUBY)
it 'does not register an offense when using add_reference with index enabled' do
expect_no_offenses(<<~RUBY)
def up
#{create_table_statement}
add_reference(:projects, :users, index: true)
end
RUBY
end
RUBY
end
it 'does not register an offense when the index is unique' do
expect_no_offenses(<<~RUBY)
it 'does not register an offense when the index is unique' do
expect_no_offenses(<<~RUBY)
def up
#{create_table_statement}
add_reference(:projects, :users, index: { unique: true } )
end
RUBY
RUBY
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../../rubocop/cop/qa/ambiguous_page_object_name'
describe RuboCop::Cop::QA::AmbiguousPageObjectName do
include CopHelper
let(:source_file) { 'qa/page.rb' }
subject(:cop) { described_class.new }
context 'in a QA file' do
before do
allow(cop).to receive(:in_qa_file?).and_return(true)
end
it "registers an offense for pages named `page`" do
expect_offense(<<-RUBY)
Page::Layout::Bar.perform do |page|
^^^^ Don't use 'page' as a name for a Page Object. Use `bar` instead.
expect(page).to have_performance_bar
expect(page).to have_detailed_metrics
end
RUBY
end
it "doesnt offend if the page object is named otherwise" do
expect_no_offenses(<<-RUBY)
Page::Object.perform do |obj|
obj.whatever
end
RUBY
end
end
context 'outside of a QA file' do
before do
allow(cop).to receive(:in_qa_file?).and_return(false)
end
it "does not register an offense" do
expect_no_offenses(<<-RUBY)
Page::Object.perform do |page|
page.do_something
end
RUBY
end
end
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