diff options
| author | GitLab Bot <gitlab-bot@gitlab.com> | 2019-09-13 18:06:03 +0000 |
|---|---|---|
| committer | GitLab Bot <gitlab-bot@gitlab.com> | 2019-09-13 18:06:03 +0000 |
| commit | 40d3d574132d2849646c20eb9d8742b159d9bfc8 (patch) | |
| tree | 431dee6675639da4421dbb1d6f50b7734a3190c3 | |
| parent | 5939b09fd3db37ec98dfce0345162617d9d1d313 (diff) | |
| download | gitlab-ce-40d3d574132d2849646c20eb9d8742b159d9bfc8.tar.gz | |
Add latest changes from gitlab-org/gitlab@master
65 files changed, 968 insertions, 335 deletions
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 diff --git a/changelogs/unreleased/slack-protected-branch-notification.yml b/changelogs/unreleased/slack-protected-branch-notification.yml new file mode 100644 index 00000000000..a8cb8a80bbe --- /dev/null +++ b/changelogs/unreleased/slack-protected-branch-notification.yml @@ -0,0 +1,5 @@ +--- +title: Support chat notifications to be fired for protected branches +merge_request: 32176 +author: +type: added diff --git a/db/migrate/20190611161641_add_target_project_id_to_merge_trains.rb b/db/migrate/20190611161641_add_target_project_id_to_merge_trains.rb index c200208e4c3..553e9371ab3 100644 --- a/db/migrate/20190611161641_add_target_project_id_to_merge_trains.rb +++ b/db/migrate/20190611161641_add_target_project_id_to_merge_trains.rb @@ -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 diff --git a/db/migrate/20190712040400_add_environment_id_to_clusters_kubernetes_namespaces.rb b/db/migrate/20190712040400_add_environment_id_to_clusters_kubernetes_namespaces.rb index 5ab5a9ba2f8..7f465c0a962 100644 --- a/db/migrate/20190712040400_add_environment_id_to_clusters_kubernetes_namespaces.rb +++ b/db/migrate/20190712040400_add_environment_id_to_clusters_kubernetes_namespaces.rb @@ -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 diff --git a/db/migrate/20190715042813_add_issue_id_to_versions.rb b/db/migrate/20190715042813_add_issue_id_to_versions.rb index 1cefdbc9df2..623e72c1096 100644 --- a/db/migrate/20190715042813_add_issue_id_to_versions.rb +++ b/db/migrate/20190715042813_add_issue_id_to_versions.rb @@ -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 diff --git a/db/migrate/20190724112147_add_column_for_self_monitoring_project_id.rb b/db/migrate/20190724112147_add_column_for_self_monitoring_project_id.rb index ce249a527e6..cf44804cdb4 100644 --- a/db/migrate/20190724112147_add_column_for_self_monitoring_project_id.rb +++ b/db/migrate/20190724112147_add_column_for_self_monitoring_project_id.rb @@ -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 diff --git a/doc/api/services.md b/doc/api/services.md index 7d025cd3bdf..8c6c5738bb8 100644 --- a/doc/api/services.md +++ b/doc/api/services.md @@ -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 | diff --git a/doc/ci/variables/predefined_variables.md b/doc/ci/variables/predefined_variables.md index a3c253cc517..9aff58f1a7c 100644 --- a/doc/ci/variables/predefined_variables.md +++ b/doc/ci/variables/predefined_variables.md @@ -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 | diff --git a/doc/user/project/integrations/img/hangouts_chat_configuration.png b/doc/user/project/integrations/img/hangouts_chat_configuration.png Binary files differindex 54aaef6632d..c40c9b92edb 100644 --- a/doc/user/project/integrations/img/hangouts_chat_configuration.png +++ b/doc/user/project/integrations/img/hangouts_chat_configuration.png diff --git a/doc/user/project/integrations/img/mattermost_configuration.png b/doc/user/project/integrations/img/mattermost_configuration.png Binary files differindex 6abf5c5c8d6..d196b1fc8e4 100644 --- a/doc/user/project/integrations/img/mattermost_configuration.png +++ b/doc/user/project/integrations/img/mattermost_configuration.png diff --git a/doc/user/project/integrations/img/microsoft_teams_configuration.png b/doc/user/project/integrations/img/microsoft_teams_configuration.png Binary files differindex 627715d5c18..8794991f2ec 100644 --- a/doc/user/project/integrations/img/microsoft_teams_configuration.png +++ b/doc/user/project/integrations/img/microsoft_teams_configuration.png diff --git a/doc/user/project/integrations/img/slack_configuration.png b/doc/user/project/integrations/img/slack_configuration.png Binary files differindex 10d2cda6dc7..6922c70f253 100644 --- a/doc/user/project/integrations/img/slack_configuration.png +++ b/doc/user/project/integrations/img/slack_configuration.png diff --git a/doc/user/project/web_ide/img/commit_changes.png b/doc/user/project/web_ide/img/commit_changes.png Binary files differdeleted file mode 100644 index a5364c12760..00000000000 --- a/doc/user/project/web_ide/img/commit_changes.png +++ /dev/null diff --git a/doc/user/project/web_ide/img/commit_changes_v12_3.png b/doc/user/project/web_ide/img/commit_changes_v12_3.png Binary files differnew file mode 100644 index 00000000000..0ee7da26d1a --- /dev/null +++ b/doc/user/project/web_ide/img/commit_changes_v12_3.png diff --git a/doc/user/project/web_ide/img/review_changes_v12_3.png b/doc/user/project/web_ide/img/review_changes_v12_3.png Binary files differnew file mode 100644 index 00000000000..cbc96e3dcd9 --- /dev/null +++ b/doc/user/project/web_ide/img/review_changes_v12_3.png diff --git a/doc/user/project/web_ide/index.md b/doc/user/project/web_ide/index.md index ecd8f74194e..bebb17e6381 100644 --- a/doc/user/project/web_ide/index.md +++ b/doc/user/project/web_ide/index.md @@ -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. + - +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. + + ## Reviewing changes diff --git a/lib/api/helpers/services_helpers.rb b/lib/api/helpers/services_helpers.rb index 607a498ffb2..5331de3c035 100644 --- a/lib/api/helpers/services_helpers.rb +++ b/lib/api/helpers/services_helpers.rb @@ -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' => [ diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 97213a026f2..2d549861893 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -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 "" diff --git a/qa/qa/resource/deploy_token.rb b/qa/qa/resource/deploy_token.rb index fca5ed83c87..f0f0da27412 100644 --- a/qa/qa/resource/deploy_token.rb +++ b/qa/qa/resource/deploy_token.rb @@ -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 diff --git a/qa/qa/resource/file.rb b/qa/qa/resource/file.rb index ca74654bf90..a870e7d5d26 100644 --- a/qa/qa/resource/file.rb +++ b/qa/qa/resource/file.rb @@ -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) diff --git a/qa/qa/resource/fork.rb b/qa/qa/resource/fork.rb index 03bc1f0820b..54b0c2d0059 100644 --- a/qa/qa/resource/fork.rb +++ b/qa/qa/resource/fork.rb @@ -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 diff --git a/qa/qa/resource/issue.rb b/qa/qa/resource/issue.rb index 8539eaeb337..a894e5c2033 100644 --- a/qa/qa/resource/issue.rb +++ b/qa/qa/resource/issue.rb @@ -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 diff --git a/qa/qa/resource/kubernetes_cluster.rb b/qa/qa/resource/kubernetes_cluster.rb index 27ab7b60211..6778094cee4 100644 --- a/qa/qa/resource/kubernetes_cluster.rb +++ b/qa/qa/resource/kubernetes_cluster.rb @@ -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 diff --git a/qa/qa/resource/label.rb b/qa/qa/resource/label.rb index b5e88d8aefc..a9177ef3df6 100644 --- a/qa/qa/resource/label.rb +++ b/qa/qa/resource/label.rb @@ -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) diff --git a/qa/qa/resource/personal_access_token.rb b/qa/qa/resource/personal_access_token.rb index f5c632cd8d2..f5bebd25202 100644 --- a/qa/qa/resource/personal_access_token.rb +++ b/qa/qa/resource/personal_access_token.rb @@ -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 diff --git a/qa/qa/resource/project.rb b/qa/qa/resource/project.rb index 157064dfe37..2e49f69bd55 100644 --- a/qa/qa/resource/project.rb +++ b/qa/qa/resource/project.rb @@ -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) diff --git a/qa/qa/resource/project_imported_from_github.rb b/qa/qa/resource/project_imported_from_github.rb index a160cdb3273..e4a9a8bcd3d 100644 --- a/qa/qa/resource/project_imported_from_github.rb +++ b/qa/qa/resource/project_imported_from_github.rb @@ -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) diff --git a/qa/qa/resource/project_milestone.rb b/qa/qa/resource/project_milestone.rb index 70640eac095..39077b64a22 100644 --- a/qa/qa/resource/project_milestone.rb +++ b/qa/qa/resource/project_milestone.rb @@ -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 diff --git a/qa/qa/resource/sandbox.rb b/qa/qa/resource/sandbox.rb index 6eff0e87ddc..47bd86440a0 100644 --- a/qa/qa/resource/sandbox.rb +++ b/qa/qa/resource/sandbox.rb @@ -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) diff --git a/qa/qa/resource/snippet.rb b/qa/qa/resource/snippet.rb index f58d7b5113f..23c17fdb32a 100644 --- a/qa/qa/resource/snippet.rb +++ b/qa/qa/resource/snippet.rb @@ -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) diff --git a/qa/qa/resource/ssh_key.rb b/qa/qa/resource/ssh_key.rb index 4e399ae730e..9b6494c11bc 100644 --- a/qa/qa/resource/ssh_key.rb +++ b/qa/qa/resource/ssh_key.rb @@ -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 diff --git a/qa/qa/specs/features/browser_ui/1_manage/group/create_group_with_mattermost_team_spec.rb b/qa/qa/specs/features/browser_ui/1_manage/group/create_group_with_mattermost_team_spec.rb index 94d20106de4..de33349a8b2 100644 --- a/qa/qa/specs/features/browser_ui/1_manage/group/create_group_with_mattermost_team_spec.rb +++ b/qa/qa/specs/features/browser_ui/1_manage/group/create_group_with_mattermost_team_spec.rb @@ -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( diff --git a/qa/qa/specs/features/browser_ui/1_manage/login/log_into_mattermost_via_gitlab_spec.rb b/qa/qa/specs/features/browser_ui/1_manage/login/log_into_mattermost_via_gitlab_spec.rb index 0a999cf00fa..babe6f7741f 100644 --- a/qa/qa/specs/features/browser_ui/1_manage/login/log_into_mattermost_via_gitlab_spec.rb +++ b/qa/qa/specs/features/browser_ui/1_manage/login/log_into_mattermost_via_gitlab_spec.rb @@ -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 diff --git a/qa/qa/specs/features/browser_ui/1_manage/project/add_project_member_spec.rb b/qa/qa/specs/features/browser_ui/1_manage/project/add_project_member_spec.rb index 1c1f552e224..07298b8a9be 100644 --- a/qa/qa/specs/features/browser_ui/1_manage/project/add_project_member_spec.rb +++ b/qa/qa/specs/features/browser_ui/1_manage/project/add_project_member_spec.rb @@ -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 diff --git a/qa/qa/specs/features/browser_ui/2_plan/issue/check_mentions_for_xss_spec.rb b/qa/qa/specs/features/browser_ui/2_plan/issue/check_mentions_for_xss_spec.rb index 70c03e10449..55e15b19200 100644 --- a/qa/qa/specs/features/browser_ui/2_plan/issue/check_mentions_for_xss_spec.rb +++ b/qa/qa/specs/features/browser_ui/2_plan/issue/check_mentions_for_xss_spec.rb @@ -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 diff --git a/qa/qa/specs/features/browser_ui/3_create/repository/add_file_template_spec.rb b/qa/qa/specs/features/browser_ui/3_create/repository/add_file_template_spec.rb index 567c6a83ddf..c4a6ce13f4c 100644 --- a/qa/qa/specs/features/browser_ui/3_create/repository/add_file_template_spec.rb +++ b/qa/qa/specs/features/browser_ui/3_create/repository/add_file_template_spec.rb @@ -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 diff --git a/qa/qa/specs/features/browser_ui/3_create/repository/user_views_commit_diff_patch_spec.rb b/qa/qa/specs/features/browser_ui/3_create/repository/user_views_commit_diff_patch_spec.rb index 21785ca3ed3..b2eef38f896 100644 --- a/qa/qa/specs/features/browser_ui/3_create/repository/user_views_commit_diff_patch_spec.rb +++ b/qa/qa/specs/features/browser_ui/3_create/repository/user_views_commit_diff_patch_spec.rb @@ -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 diff --git a/qa/qa/specs/features/browser_ui/3_create/web_ide/add_file_template_spec.rb b/qa/qa/specs/features/browser_ui/3_create/web_ide/add_file_template_spec.rb index c09c65a57a5..0a89f0c9d41 100644 --- a/qa/qa/specs/features/browser_ui/3_create/web_ide/add_file_template_spec.rb +++ b/qa/qa/specs/features/browser_ui/3_create/web_ide/add_file_template_spec.rb @@ -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 diff --git a/qa/qa/specs/features/browser_ui/3_create/wiki/create_edit_clone_push_wiki_spec.rb b/qa/qa/specs/features/browser_ui/3_create/wiki/create_edit_clone_push_wiki_spec.rb index e689ba4c69c..2c3f2c86c23 100644 --- a/qa/qa/specs/features/browser_ui/3_create/wiki/create_edit_clone_push_wiki_spec.rb +++ b/qa/qa/specs/features/browser_ui/3_create/wiki/create_edit_clone_push_wiki_spec.rb @@ -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 diff --git a/qa/qa/specs/features/browser_ui/non_devops/performance_bar_spec.rb b/qa/qa/specs/features/browser_ui/non_devops/performance_bar_spec.rb index a04efb94def..4fca2db3d0f 100644 --- a/qa/qa/specs/features/browser_ui/non_devops/performance_bar_spec.rb +++ b/qa/qa/specs/features/browser_ui/non_devops/performance_bar_spec.rb @@ -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 diff --git a/qa/qa/tools/revoke_all_personal_access_tokens.rb b/qa/qa/tools/revoke_all_personal_access_tokens.rb index 8a74f52324d..f14975c0e5e 100644 --- a/qa/qa/tools/revoke_all_personal_access_tokens.rb +++ b/qa/qa/tools/revoke_all_personal_access_tokens.rb @@ -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" diff --git a/rubocop/cop/migration/add_reference.rb b/rubocop/cop/migration/add_reference.rb index 1d471b9797e..33840fc7caf 100644 --- a/rubocop/cop/migration/add_reference.rb +++ b/rubocop/cop/migration/add_reference.rb @@ -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 diff --git a/rubocop/cop/qa/ambiguous_page_object_name.rb b/rubocop/cop/qa/ambiguous_page_object_name.rb new file mode 100644 index 00000000000..5cd8ea25c87 --- /dev/null +++ b/rubocop/cop/qa/ambiguous_page_object_name.rb @@ -0,0 +1,48 @@ +# 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 diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb index 29864777f59..8e7df62ea75 100644 --- a/rubocop/rubocop.rb +++ b/rubocop/rubocop.rb @@ -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' diff --git a/scripts/lint-doc.sh b/scripts/lint-doc.sh index 8c9b8b9fb02..e2c22785963 100755 --- a/scripts/lint-doc.sh +++ b/scripts/lint-doc.sh @@ -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" diff --git a/scripts/trigger-build-docs b/scripts/trigger-build-docs index 83841512f1c..046ee5bceb8 100755 --- a/scripts/trigger-build-docs +++ b/scripts/trigger-build-docs @@ -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' diff --git a/spec/features/admin/admin_settings_spec.rb b/spec/features/admin/admin_settings_spec.rb index eb59de2e132..be34315118d 100644 --- a/spec/features/admin/admin_settings_spec.rb +++ b/spec/features/admin/admin_settings_spec.rb @@ -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' diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index bc853d45085..5c0f8bd392a 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -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 }, diff --git a/spec/models/project_services/mattermost_service_spec.rb b/spec/models/project_services/mattermost_service_spec.rb index 6261c70f266..5b974985706 100644 --- a/spec/models/project_services/mattermost_service_spec.rb +++ b/spec/models/project_services/mattermost_service_spec.rb @@ -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 diff --git a/spec/models/project_services/microsoft_teams_service_spec.rb b/spec/models/project_services/microsoft_teams_service_spec.rb index 73c20359091..275244fa5fd 100644 --- a/spec/models/project_services/microsoft_teams_service_spec.rb +++ b/spec/models/project_services/microsoft_teams_service_spec.rb @@ -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 diff --git a/spec/models/project_services/pipelines_email_service_spec.rb b/spec/models/project_services/pipelines_email_service_spec.rb index b85565e0c25..67358d6c3d6 100644 --- a/spec/models/project_services/pipelines_email_service_spec.rb +++ b/spec/models/project_services/pipelines_email_service_spec.rb @@ -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 diff --git a/spec/models/project_services/slack_service_spec.rb b/spec/models/project_services/slack_service_spec.rb index 01f580c5d01..f751dd6ffb9 100644 --- a/spec/models/project_services/slack_service_spec.rb +++ b/spec/models/project_services/slack_service_spec.rb @@ -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 diff --git a/spec/rubocop/cop/migration/add_reference_spec.rb b/spec/rubocop/cop/migration/add_reference_spec.rb index c348fc0efac..0b56fe8ed83 100644 --- a/spec/rubocop/cop/migration/add_reference_spec.rb +++ b/spec/rubocop/cop/migration/add_reference_spec.rb @@ -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 diff --git a/spec/rubocop/cop/qa/ambiguous_page_object_name_spec.rb b/spec/rubocop/cop/qa/ambiguous_page_object_name_spec.rb new file mode 100644 index 00000000000..8ee720af9a5 --- /dev/null +++ b/spec/rubocop/cop/qa/ambiguous_page_object_name_spec.rb @@ -0,0 +1,54 @@ +# 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 diff --git a/spec/support/shared_examples/models/chat_service_shared_examples.rb b/spec/support/shared_examples/models/chat_service_shared_examples.rb index b6a3d50d14a..98bf647a9bc 100644 --- a/spec/support/shared_examples/models/chat_service_shared_examples.rb +++ b/spec/support/shared_examples/models/chat_service_shared_examples.rb @@ -49,7 +49,11 @@ shared_examples_for "chat service" do |service_name| WebMock.stub_request(:post, webhook_url) end - shared_examples "#{service_name} service" do + shared_examples "triggered #{service_name} service" do |branches_to_be_notified: nil| + before do + subject.branches_to_be_notified = branches_to_be_notified if branches_to_be_notified + end + it "calls #{service_name} API" do subject.execute(sample_data) @@ -57,12 +61,24 @@ shared_examples_for "chat service" do |service_name| end end + shared_examples "untriggered #{service_name} service" do |branches_to_be_notified: nil| + before do + subject.branches_to_be_notified = branches_to_be_notified if branches_to_be_notified + end + + it "does not call #{service_name} API" do + result = subject.execute(sample_data) + + expect(result).to be_falsy + end + end + context "with push events" do let(:sample_data) do Gitlab::DataBuilder::Push.build_sample(project, user) end - it_behaves_like "#{service_name} service" + it_behaves_like "triggered #{service_name} service" it "specifies the webhook when it is configured" do expect(client).to receive(:new).with(client_arguments).and_return(double(:chat_service).as_null_object) @@ -70,29 +86,73 @@ shared_examples_for "chat service" do |service_name| subject.execute(sample_data) end - context "with not default branch" do + context "with default branch" do let(:sample_data) do - Gitlab::DataBuilder::Push.build(project: project, user: user, ref: "not-the-default-branch") + Gitlab::DataBuilder::Push.build(project: project, user: user, ref: project.default_branch) end - context "when notify_only_default_branch enabled" do - before do - subject.notify_only_default_branch = true - end + context "when only default branch are to be notified" do + it_behaves_like "triggered #{service_name} service", branches_to_be_notified: "default" + end - it "does not call the Discord Webhooks API" do - result = subject.execute(sample_data) + context "when only protected branches are to be notified" do + it_behaves_like "untriggered #{service_name} service", branches_to_be_notified: "protected" + end - expect(result).to be_falsy - end + context "when default and protected branches are to be notified" do + it_behaves_like "triggered #{service_name} service", branches_to_be_notified: "default_and_protected" end - context "when notify_only_default_branch disabled" do - before do - subject.notify_only_default_branch = false - end + context "when all branches are to be notified" do + it_behaves_like "triggered #{service_name} service", branches_to_be_notified: "all" + end + end + + context "with protected branch" do + before do + create(:protected_branch, project: project, name: "a-protected-branch") + end + + let(:sample_data) do + Gitlab::DataBuilder::Push.build(project: project, user: user, ref: "a-protected-branch") + end + + context "when only default branch are to be notified" do + it_behaves_like "untriggered #{service_name} service", branches_to_be_notified: "default" + end + + context "when only protected branches are to be notified" do + it_behaves_like "triggered #{service_name} service", branches_to_be_notified: "protected" + end - it_behaves_like "#{service_name} service" + context "when default and protected branches are to be notified" do + it_behaves_like "triggered #{service_name} service", branches_to_be_notified: "default_and_protected" + end + + context "when all branches are to be notified" do + it_behaves_like "triggered #{service_name} service", branches_to_be_notified: "all" + end + end + + context "with neither default nor protected branch" do + let(:sample_data) do + Gitlab::DataBuilder::Push.build(project: project, user: user, ref: "a-random-branch") + end + + context "when only default branch are to be notified" do + it_behaves_like "untriggered #{service_name} service", branches_to_be_notified: "default" + end + + context "when only protected branches are to be notified" do + it_behaves_like "untriggered #{service_name} service", branches_to_be_notified: "protected" + end + + context "when default and protected branches are to be notified" do + it_behaves_like "untriggered #{service_name} service", branches_to_be_notified: "default_and_protected" + end + + context "when all branches are to be notified" do + it_behaves_like "triggered #{service_name} service", branches_to_be_notified: "all" end end end @@ -105,7 +165,7 @@ shared_examples_for "chat service" do |service_name| service.hook_data(issue, "open") end - it_behaves_like "#{service_name} service" + it_behaves_like "triggered #{service_name} service" end context "with merge events" do @@ -128,7 +188,7 @@ shared_examples_for "chat service" do |service_name| project.add_developer(user) end - it_behaves_like "#{service_name} service" + it_behaves_like "triggered #{service_name} service" end context "with wiki page events" do @@ -143,7 +203,7 @@ shared_examples_for "chat service" do |service_name| let(:wiki_page) { create(:wiki_page, wiki: project.wiki, attrs: opts) } let(:sample_data) { Gitlab::DataBuilder::WikiPage.build(wiki_page, user, "create") } - it_behaves_like "#{service_name} service" + it_behaves_like "triggered #{service_name} service" end context "with note events" do @@ -158,7 +218,7 @@ shared_examples_for "chat service" do |service_name| note: "a comment on a commit") end - it_behaves_like "#{service_name} service" + it_behaves_like "triggered #{service_name} service" end context "with merge request comment" do @@ -166,7 +226,7 @@ shared_examples_for "chat service" do |service_name| create(:note_on_merge_request, project: project, note: "merge request note") end - it_behaves_like "#{service_name} service" + it_behaves_like "triggered #{service_name} service" end context "with issue comment" do @@ -174,7 +234,7 @@ shared_examples_for "chat service" do |service_name| create(:note_on_issue, project: project, note: "issue note") end - it_behaves_like "#{service_name} service" + it_behaves_like "triggered #{service_name} service" end context "with snippet comment" do @@ -182,7 +242,7 @@ shared_examples_for "chat service" do |service_name| create(:note_on_project_snippet, project: project, note: "snippet note") end - it_behaves_like "#{service_name} service" + it_behaves_like "triggered #{service_name} service" end end @@ -197,14 +257,14 @@ shared_examples_for "chat service" do |service_name| context "with failed pipeline" do let(:status) { "failed" } - it_behaves_like "#{service_name} service" + it_behaves_like "triggered #{service_name} service" end context "with succeeded pipeline" do let(:status) { "success" } context "with default notify_only_broken_pipelines" do - it "does not call Discord Webhooks API" do + it "does not call #{service_name} API" do result = subject.execute(sample_data) expect(result).to be_falsy @@ -216,34 +276,77 @@ shared_examples_for "chat service" do |service_name| subject.notify_only_broken_pipelines = false end - it_behaves_like "#{service_name} service" + it_behaves_like "triggered #{service_name} service" end end - context "with not default branch" do - let(:pipeline) do - create(:ci_pipeline, :failed, project: project, - sha: project.commit.sha, ref: "not-the-default-branch") + context "with default branch" do + let(:sample_data) do + Gitlab::DataBuilder::Push.build(project: project, user: user, ref: project.default_branch) end - context "when notify_only_default_branch enabled" do - before do - subject.notify_only_default_branch = true - end + context "when only default branch are to be notified" do + it_behaves_like "triggered #{service_name} service", branches_to_be_notified: "default" + end - it "does not call the Discord Webhooks API" do - result = subject.execute(sample_data) + context "when only protected branches are to be notified" do + it_behaves_like "untriggered #{service_name} service", branches_to_be_notified: "protected" + end - expect(result).to be_falsy - end + context "when default and protected branches are to be notified" do + it_behaves_like "triggered #{service_name} service", branches_to_be_notified: "default_and_protected" end - context "when notify_only_default_branch disabled" do - before do - subject.notify_only_default_branch = false - end + context "when all branches are to be notified" do + it_behaves_like "triggered #{service_name} service", branches_to_be_notified: "all" + end + end + + context "with protected branch" do + before do + create(:protected_branch, project: project, name: "a-protected-branch") + end + + let(:sample_data) do + Gitlab::DataBuilder::Push.build(project: project, user: user, ref: "a-protected-branch") + end + + context "when only default branch are to be notified" do + it_behaves_like "untriggered #{service_name} service", branches_to_be_notified: "default" + end + + context "when only protected branches are to be notified" do + it_behaves_like "triggered #{service_name} service", branches_to_be_notified: "protected" + end + + context "when default and protected branches are to be notified" do + it_behaves_like "triggered #{service_name} service", branches_to_be_notified: "default_and_protected" + end + + context "when all branches are to be notified" do + it_behaves_like "triggered #{service_name} service", branches_to_be_notified: "all" + end + end + + context "with neither default nor protected branch" do + let(:sample_data) do + Gitlab::DataBuilder::Push.build(project: project, user: user, ref: "a-random-branch") + end + + context "when only default branch are to be notified" do + it_behaves_like "untriggered #{service_name} service", branches_to_be_notified: "default" + end + + context "when only protected branches are to be notified" do + it_behaves_like "untriggered #{service_name} service", branches_to_be_notified: "protected" + end + + context "when default and protected branches are to be notified" do + it_behaves_like "untriggered #{service_name} service", branches_to_be_notified: "default_and_protected" + end - it_behaves_like "#{service_name} service" + context "when all branches are to be notified" do + it_behaves_like "triggered #{service_name} service", branches_to_be_notified: "all" end end end diff --git a/spec/support/shared_examples/slack_mattermost_notifications_shared_examples.rb b/spec/support/shared_examples/slack_mattermost_notifications_shared_examples.rb index 8ce94064dc3..455f7b117b2 100644 --- a/spec/support/shared_examples/slack_mattermost_notifications_shared_examples.rb +++ b/spec/support/shared_examples/slack_mattermost_notifications_shared_examples.rb @@ -2,7 +2,7 @@ Dir[Rails.root.join("app/models/project_services/chat_message/*.rb")].each { |f| require f } -RSpec.shared_examples 'slack or mattermost notifications' do +RSpec.shared_examples 'slack or mattermost notifications' do |service_name| let(:chat_service) { described_class.new } let(:webhook_url) { 'https://example.gitlab.com/' } @@ -35,6 +35,28 @@ RSpec.shared_examples 'slack or mattermost notifications' do end end + shared_examples "triggered #{service_name} service" do |event_type: nil, branches_to_be_notified: nil| + before do + chat_service.branches_to_be_notified = branches_to_be_notified if branches_to_be_notified + end + + it "notifies about #{event_type} events" do + chat_service.execute(data) + expect(WebMock).to have_requested(:post, webhook_url) + end + end + + shared_examples "untriggered #{service_name} service" do |event_type: nil, branches_to_be_notified: nil| + before do + chat_service.branches_to_be_notified = branches_to_be_notified if branches_to_be_notified + end + + it "notifies about #{event_type} events" do + chat_service.execute(data) + expect(WebMock).not_to have_requested(:post, webhook_url) + end + end + describe "#execute" do let(:user) { create(:user) } let(:project) { create(:project, :repository, :wiki_repo) } @@ -42,7 +64,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do let(:channel) { 'slack_channel' } let(:issue_service_options) { { title: 'Awesome issue', description: 'please fix' } } - let(:push_sample_data) do + let(:data) do Gitlab::DataBuilder::Push.build_sample(project, user) end @@ -84,31 +106,31 @@ RSpec.shared_examples 'slack or mattermost notifications' do @wiki_page_sample_data = Gitlab::DataBuilder::WikiPage.build(@wiki_page, user, 'create') end - it "calls Slack/Mattermost API for push events" do - chat_service.execute(push_sample_data) + it "calls #{service_name} API for push events" do + chat_service.execute(data) expect(WebMock).to have_requested(:post, webhook_url).once end - it "calls Slack/Mattermost API for issue events" do + it "calls #{service_name} API for issue events" do chat_service.execute(@issues_sample_data) expect(WebMock).to have_requested(:post, webhook_url).once end - it "calls Slack/Mattermost API for merge requests events" do + it "calls #{service_name} API for merge requests events" do chat_service.execute(@merge_sample_data) expect(WebMock).to have_requested(:post, webhook_url).once end - it "calls Slack/Mattermost API for wiki page events" do + it "calls #{service_name} API for wiki page events" do chat_service.execute(@wiki_page_sample_data) expect(WebMock).to have_requested(:post, webhook_url).once end - it "calls Slack/Mattermost API for deployment events" do + it "calls #{service_name} API for deployment events" do deployment_event_data = { object_kind: 'deployment' } chat_service.execute(deployment_event_data) @@ -125,7 +147,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do double(:slack_service).as_null_object ) - chat_service.execute(push_sample_data) + chat_service.execute(data) end it 'uses the channel as an option when it is configured' do @@ -135,7 +157,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do .and_return( double(:slack_service).as_null_object ) - chat_service.execute(push_sample_data) + chat_service.execute(data) end context "event channels" do @@ -148,7 +170,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do double(:slack_service).as_null_object ) - chat_service.execute(push_sample_data) + chat_service.execute(data) end it "uses the right channel for merge request event" do @@ -269,64 +291,132 @@ RSpec.shared_examples 'slack or mattermost notifications' do WebMock.stub_request(:post, webhook_url) end - context 'only notify for the default branch' do - context 'when enabled' do - before do - chat_service.notify_only_default_branch = true + context 'on default branch' do + let(:data) do + Gitlab::DataBuilder::Push.build( + project: project, + user: user, + ref: project.default_branch + ) + end + + context 'pushing tags' do + let(:data) do + Gitlab::DataBuilder::Push.build( + project: project, + user: user, + ref: "#{Gitlab::Git::TAG_REF_PREFIX}test" + ) 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: project, user: user, ref: ref) + it_behaves_like "triggered #{service_name} service", event_type: "push" + end - chat_service.execute(push_sample_data) + context 'notification enabled only for default branch' do + it_behaves_like "triggered #{service_name} service", event_type: "push", branches_to_be_notified: "default" + end - expect(WebMock).not_to have_requested(:post, webhook_url) - end + context 'notification enabled only for protected branches' do + it_behaves_like "untriggered #{service_name} service", event_type: "push", branches_to_be_notified: "protected" + end + + context 'notification enabled only for default and protected branches' do + it_behaves_like "triggered #{service_name} service", event_type: "push", branches_to_be_notified: "default_and_protected" + end + + context 'notification enabled for all branches' do + it_behaves_like "triggered #{service_name} service", event_type: "push", branches_to_be_notified: "all" + end + end - it 'notifies about push events for the default branch' do - push_sample_data = Gitlab::DataBuilder::Push.build_sample(project, user) + context 'on a protected branch' do + before do + create(:protected_branch, project: project, name: 'a-protected-branch') + end - chat_service.execute(push_sample_data) + let(:data) do + Gitlab::DataBuilder::Push.build( + project: project, + user: user, + ref: 'a-protected-branch' + ) + end - expect(WebMock).to have_requested(:post, webhook_url).once + context 'pushing tags' do + let(:data) do + Gitlab::DataBuilder::Push.build( + project: project, + user: user, + ref: "#{Gitlab::Git::TAG_REF_PREFIX}test" + ) end - it 'still notifies about pushed tags' do - ref = "#{Gitlab::Git::TAG_REF_PREFIX}test" - push_sample_data = Gitlab::DataBuilder::Push.build(project: project, user: user, ref: ref) + it_behaves_like "triggered #{service_name} service", event_type: "push" + end - chat_service.execute(push_sample_data) + context 'notification enabled only for default branch' do + it_behaves_like "untriggered #{service_name} service", event_type: "push", branches_to_be_notified: "default" + end - expect(WebMock).to have_requested(:post, webhook_url).once - end + context 'notification enabled only for protected branches' do + it_behaves_like "triggered #{service_name} service", event_type: "push", branches_to_be_notified: "protected" end - context 'when disabled' do - before do - chat_service.notify_only_default_branch = false - end + context 'notification enabled only for default and protected branches' do + it_behaves_like "triggered #{service_name} service", event_type: "push", branches_to_be_notified: "default_and_protected" + end - it 'notifies about all push events' do - ref = "#{Gitlab::Git::BRANCH_REF_PREFIX}test" - push_sample_data = Gitlab::DataBuilder::Push.build(project: project, user: user, ref: ref) + context 'notification enabled for all branches' do + it_behaves_like "triggered #{service_name} service", event_type: "push", branches_to_be_notified: "all" + end + end - chat_service.execute(push_sample_data) + context 'on a neither protected nor default branch' do + let(:data) do + Gitlab::DataBuilder::Push.build( + project: project, + user: user, + ref: 'a-random-branch' + ) + end - expect(WebMock).to have_requested(:post, webhook_url).once + context 'pushing tags' do + let(:data) do + Gitlab::DataBuilder::Push.build( + project: project, + user: user, + ref: "#{Gitlab::Git::TAG_REF_PREFIX}test" + ) end + + it_behaves_like "triggered #{service_name} service", event_type: "push" + end + + context 'notification enabled only for default branch' do + it_behaves_like "untriggered #{service_name} service", event_type: "push", branches_to_be_notified: "default" + end + + context 'notification enabled only for protected branches' do + it_behaves_like "untriggered #{service_name} service", event_type: "push", branches_to_be_notified: "protected" + end + + context 'notification enabled only for default and protected branches' do + it_behaves_like "untriggered #{service_name} service", event_type: "push", branches_to_be_notified: "default_and_protected" + end + + context 'notification enabled for all branches' do + it_behaves_like "triggered #{service_name} service", event_type: "push", branches_to_be_notified: "all" end end end - describe "Note events" do + describe 'Note events' do let(:user) { create(:user) } let(:project) { create(:project, :repository, creator: user) } before do allow(chat_service).to receive_messages( project: project, - project_id: project.id, service_hook: true, webhook: webhook_url ) @@ -342,61 +432,56 @@ RSpec.shared_examples 'slack or mattermost notifications' do note: 'a comment on a commit') end - it "calls Slack/Mattermost API for commit comment events" do - data = Gitlab::DataBuilder::Note.build(commit_note, user) - chat_service.execute(data) - - expect(WebMock).to have_requested(:post, webhook_url).once + let(:data) do + Gitlab::DataBuilder::Note.build(commit_note, user) end + + it_behaves_like "triggered #{service_name} service", event_type: "commit comment" end context 'when merge request comment event executed' do let(:merge_request_note) do create(:note_on_merge_request, project: project, - note: "merge request note") + note: 'a comment on a merge request') end - it "calls Slack API for merge request comment events" do - data = Gitlab::DataBuilder::Note.build(merge_request_note, user) - chat_service.execute(data) - - expect(WebMock).to have_requested(:post, webhook_url).once + let(:data) do + Gitlab::DataBuilder::Note.build(merge_request_note, user) end + + it_behaves_like "triggered #{service_name} service", event_type: "merge request comment" end context 'when issue comment event executed' do let(:issue_note) do - create(:note_on_issue, project: project, note: "issue note") + create(:note_on_issue, project: project, + note: 'a comment on an issue') end - let(:data) { Gitlab::DataBuilder::Note.build(issue_note, user) } - - it "calls Slack API for issue comment events" do - chat_service.execute(data) - - expect(WebMock).to have_requested(:post, webhook_url).once + let(:data) do + Gitlab::DataBuilder::Note.build(issue_note, user) end + + it_behaves_like "triggered #{service_name} service", event_type: "issue comment" end context 'when snippet comment event executed' do let(:snippet_note) do create(:note_on_project_snippet, project: project, - note: "snippet note") + note: 'a comment on a snippet') end - it "calls Slack API for snippet comment events" do - data = Gitlab::DataBuilder::Note.build(snippet_note, user) - chat_service.execute(data) - - expect(WebMock).to have_requested(:post, webhook_url).once + let(:data) do + Gitlab::DataBuilder::Note.build(snippet_note, user) end + + it_behaves_like "triggered #{service_name} service", event_type: "snippet comment" end end describe 'Pipeline events' do let(:user) { create(:user) } - let(:project) { create(:project, :repository) } - + let(:project) { create(:project, :repository, creator: user) } let(:pipeline) do create(:ci_pipeline, project: project, status: status, @@ -409,77 +494,108 @@ RSpec.shared_examples 'slack or mattermost notifications' do service_hook: true, webhook: webhook_url ) + + WebMock.stub_request(:post, webhook_url) end - shared_examples 'call Slack/Mattermost API' do - before do - WebMock.stub_request(:post, webhook_url) + context 'with succeeded pipeline' do + let(:status) { 'success' } + let(:data) { Gitlab::DataBuilder::Pipeline.build(pipeline) } + + context 'with default to notify_only_broken_pipelines' do + it_behaves_like "untriggered #{service_name} service", event_type: "pipeline" end - it 'calls Slack/Mattermost API for pipeline events' do - data = Gitlab::DataBuilder::Pipeline.build(pipeline) - chat_service.execute(data) + context 'with setting notify_only_broken_pipelines to false' do + before do + chat_service.notify_only_broken_pipelines = false + end - expect(WebMock).to have_requested(:post, webhook_url).once + it_behaves_like "triggered #{service_name} service", event_type: "pipeline" end end context 'with failed pipeline' do - let(:status) { 'failed' } + context 'on default branch' do + let(:pipeline) do + create(:ci_pipeline, + project: project, status: :failed, + sha: project.commit.sha, ref: project.default_branch) + end - it_behaves_like 'call Slack/Mattermost API' - end + let(:data) { Gitlab::DataBuilder::Pipeline.build(pipeline) } - context 'with succeeded pipeline' do - let(:status) { 'success' } + context 'notification enabled only for default branch' do + it_behaves_like "triggered #{service_name} service", event_type: "pipeline", branches_to_be_notified: "default" + end - context 'with default to notify_only_broken_pipelines' do - it 'does not call Slack/Mattermost API for pipeline events' do - data = Gitlab::DataBuilder::Pipeline.build(pipeline) - result = chat_service.execute(data) + context 'notification enabled only for protected branches' do + it_behaves_like "untriggered #{service_name} service", event_type: "pipeline", branches_to_be_notified: "protected" + end - expect(result).to be_falsy + context 'notification enabled only for default and protected branches' do + it_behaves_like "triggered #{service_name} service", event_type: "pipeline", branches_to_be_notified: "default_and_protected" + end + + context 'notification enabled for all branches' do + it_behaves_like "triggered #{service_name} service", event_type: "pipeline", branches_to_be_notified: "all" end end - context 'with setting notify_only_broken_pipelines to false' do + context 'on a protected branch' do before do - chat_service.notify_only_broken_pipelines = false + create(:protected_branch, project: project, name: 'a-protected-branch') end - it_behaves_like 'call Slack/Mattermost API' - end - end - - context 'only notify for the default branch' do - context 'when enabled' do let(:pipeline) do - create(:ci_pipeline, :failed, project: project, sha: project.commit.sha, ref: 'not-the-default-branch') + create(:ci_pipeline, + project: project, status: :failed, + sha: project.commit.sha, ref: 'a-protected-branch') end - before do - chat_service.notify_only_default_branch = true - WebMock.stub_request(:post, webhook_url) + let(:data) { Gitlab::DataBuilder::Pipeline.build(pipeline) } + + context 'notification enabled only for default branch' do + it_behaves_like "untriggered #{service_name} service", event_type: "pipeline", branches_to_be_notified: "default" end - it 'does not call the Slack/Mattermost API for pipeline events' do - data = Gitlab::DataBuilder::Pipeline.build(pipeline) - result = chat_service.execute(data) + context 'notification enabled only for protected branches' do + it_behaves_like "triggered #{service_name} service", event_type: "pipeline", branches_to_be_notified: "protected" + end - expect(result).to be_falsy + context 'notification enabled only for default and protected branches' do + it_behaves_like "triggered #{service_name} service", event_type: "pipeline", branches_to_be_notified: "default_and_protected" + end + + context 'notification enabled for all branches' do + it_behaves_like "triggered #{service_name} service", event_type: "pipeline", branches_to_be_notified: "all" end end - context 'when disabled' do + context 'on a neither protected nor default branch' do let(:pipeline) do - create(:ci_pipeline, :failed, project: project, sha: project.commit.sha, ref: 'not-the-default-branch') + create(:ci_pipeline, + project: project, status: :failed, + sha: project.commit.sha, ref: 'a-random-branch') end - before do - chat_service.notify_only_default_branch = false + let(:data) { Gitlab::DataBuilder::Pipeline.build(pipeline) } + + context 'notification enabled only for default branch' do + it_behaves_like "untriggered #{service_name} service", event_type: "pipeline", branches_to_be_notified: "default" + end + + context 'notification enabled only for protected branches' do + it_behaves_like "untriggered #{service_name} service", event_type: "pipeline", branches_to_be_notified: "protected" end - it_behaves_like 'call Slack/Mattermost API' + context 'notification enabled only for default and protected branches' do + it_behaves_like "untriggered #{service_name} service", event_type: "pipeline", branches_to_be_notified: "default_and_protected" + end + + context 'notification enabled for all branches' do + it_behaves_like "triggered #{service_name} service", event_type: "pipeline", branches_to_be_notified: "all" + end end end end |
