From 27f3465d8a52afce630417b6e4b58992b6e403fe Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Wed, 24 Mar 2021 00:09:26 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- .../blob/file_template_selector.js | 2 +- app/assets/javascripts/blob_edit/edit_blob.js | 2 +- app/assets/javascripts/main.js | 41 +--- .../components/mr_widget_pipeline.vue | 26 ++- app/finders/issues_finder.rb | 9 + app/services/ci/pipeline_trigger_service.rb | 6 +- .../in_product_marketing_emails_service.rb | 3 +- ...270129-conan-project-download-urls-bug.yml | 6 + .../283893-mr-pipeline-coverage-update.yml | 5 + ...emove-ci_trigger_payload_into_pipeline.yml | 5 + ...confidentiality-filter-on-issue-finder.yml | 5 + ...ptimize_issue_filter_assigned_to_self.yml} | 12 +- config/metrics/schema.json | 2 +- config/webpack.config.js | 1 + config/webpack.vendor.config.js | 5 +- danger/changelog/Dangerfile | 9 +- doc/ci/triggers/README.md | 27 +-- .../usage_ping/metrics_dictionary.md | 12 +- .../incident_management/oncall_schedules.md | 2 +- .../settings/sign_in_restrictions.md | 27 ++- doc/user/application_security/dast/index.md | 28 +-- doc/user/packages/conan_repository/index.md | 13 -- jest.config.base.js | 1 + lib/api/helpers/packages/conan/api_helpers.rb | 3 +- .../isolated_mentioned_project_parser.rb | 25 +++ .../isolated_mentioned_user_parser.rb | 25 +++ .../gitlab/isolated_reference_extractor.rb | 2 +- .../lib/gitlab/isolated_visibility_level.rb | 60 ++++++ .../models/commit_user_mention.rb | 1 + .../models/concerns/isolated_feature_gate.rb | 20 ++ .../models/concerns/isolated_mentionable.rb | 4 +- .../concerns/namespace/recursive_traversal.rb | 2 +- .../models/design_management/design.rb | 3 + .../models/design_user_mention.rb | 1 + .../user_mentions/models/epic.rb | 6 +- .../user_mentions/models/epic_user_mention.rb | 1 + .../user_mentions/models/group.rb | 2 + .../user_mentions/models/merge_request.rb | 7 +- .../models/merge_request_user_mention.rb | 1 + .../user_mentions/models/namespace.rb | 8 +- .../user_mentions/models/note.rb | 4 +- .../user_mentions/models/project.rb | 48 +++++ .../user_mentions/models/user.rb | 37 ++++ ...heduler.rb => batched_migration_runner.rb} | 20 +- lib/gitlab/database/migration_helpers.rb | 131 +++++++----- locale/gitlab.pot | 28 ++- .../components/mr_widget_pipeline_spec.js | 83 +++---- .../batched_migration_runner_spec.rb | 185 ++++++++++++++++ .../background_migration/scheduler_spec.rb | 182 ---------------- .../gitlab/database/migration_helpers_spec.rb | 202 +++++++++++++----- .../ci/pipeline_trigger_service_spec.rb | 11 - ...n_product_marketing_emails_service_spec.rb | 7 +- .../api/conan_packages_shared_context.rb | 7 - .../assignees_filter_shared_examples.rb | 20 ++ .../api/conan_packages_shared_examples.rb | 31 +-- spec/tooling/danger/changelog_spec.rb | 157 ++++++++++---- spec/tooling/danger/project_helper_spec.rb | 2 +- tooling/danger/changelog.rb | 36 ++-- tooling/danger/project_helper.rb | 2 +- vendor/assets/javascripts/jasmine-jquery.js | 5 +- 60 files changed, 1051 insertions(+), 567 deletions(-) create mode 100644 changelogs/unreleased/270129-conan-project-download-urls-bug.yml create mode 100644 changelogs/unreleased/283893-mr-pipeline-coverage-update.yml create mode 100644 changelogs/unreleased/321027-remove-ci_trigger_payload_into_pipeline.yml create mode 100644 changelogs/unreleased/325470-optimize-confidentiality-filter-on-issue-finder.yml rename config/feature_flags/development/{ci_trigger_payload_into_pipeline.yml => optimize_issue_filter_assigned_to_self.yml} (52%) create mode 100644 lib/gitlab/background_migration/user_mentions/lib/banzai/reference_parser/isolated_mentioned_project_parser.rb create mode 100644 lib/gitlab/background_migration/user_mentions/lib/banzai/reference_parser/isolated_mentioned_user_parser.rb create mode 100644 lib/gitlab/background_migration/user_mentions/lib/gitlab/isolated_visibility_level.rb create mode 100644 lib/gitlab/background_migration/user_mentions/models/concerns/isolated_feature_gate.rb create mode 100644 lib/gitlab/background_migration/user_mentions/models/project.rb create mode 100644 lib/gitlab/background_migration/user_mentions/models/user.rb rename lib/gitlab/database/background_migration/{scheduler.rb => batched_migration_runner.rb} (78%) create mode 100644 spec/lib/gitlab/database/background_migration/batched_migration_runner_spec.rb delete mode 100644 spec/lib/gitlab/database/background_migration/scheduler_spec.rb diff --git a/app/assets/javascripts/blob/file_template_selector.js b/app/assets/javascripts/blob/file_template_selector.js index 21c23495315..12af9df34dc 100644 --- a/app/assets/javascripts/blob/file_template_selector.js +++ b/app/assets/javascripts/blob/file_template_selector.js @@ -47,7 +47,7 @@ export default class FileTemplateSelector { } isHidden() { - return this.$wrapper.hasClass('hidden'); + return !this.$wrapper || this.$wrapper.hasClass('hidden'); } getToggleText() { diff --git a/app/assets/javascripts/blob_edit/edit_blob.js b/app/assets/javascripts/blob_edit/edit_blob.js index 6c6c7e6a6d2..ab2fc80e653 100644 --- a/app/assets/javascripts/blob_edit/edit_blob.js +++ b/app/assets/javascripts/blob_edit/edit_blob.js @@ -82,7 +82,7 @@ export default class EditBlob { this.$editModePanes.hide(); - currentPane.fadeIn(200); + currentPane.show(); if (paneId === '#preview') { this.$toggleButton.hide(); diff --git a/app/assets/javascripts/main.js b/app/assets/javascripts/main.js index 417baddc031..30c7264c701 100644 --- a/app/assets/javascripts/main.js +++ b/app/assets/javascripts/main.js @@ -16,21 +16,16 @@ import { initRails } from '~/lib/utils/rails_ujs'; import * as popovers from '~/popovers'; import * as tooltips from '~/tooltips'; import initAlertHandler from './alert_handler'; -import { deprecatedCreateFlash as Flash, removeFlashClickListener } from './flash'; +import { removeFlashClickListener } from './flash'; import initTodoToggle from './header'; import initLayoutNav from './layout_nav'; -import { - handleLocationHash, - addSelectOnFocusBehaviour, - getCspNonceValue, -} from './lib/utils/common_utils'; +import { handleLocationHash, addSelectOnFocusBehaviour } from './lib/utils/common_utils'; import { localTimeAgo } from './lib/utils/datetime_utility'; import { getLocationHash, visitUrl } from './lib/utils/url_utility'; // everything else import initFeatureHighlight from './feature_highlight'; import LazyLoader from './lazy_loader'; -import { __ } from './locale'; import initLogoAnimation from './logo'; import initFrequentItemDropdowns from './frequent_items'; import initBreadcrumbs from './breadcrumb'; @@ -49,29 +44,8 @@ applyGitLabUIConfig(); window.jQuery = jQuery; window.$ = jQuery; -// Add nonce to jQuery script handler -jQuery.ajaxSetup({ - converters: { - // eslint-disable-next-line @gitlab/require-i18n-strings, func-names - 'text script': function (text) { - jQuery.globalEval(text, { nonce: getCspNonceValue() }); - return text; - }, - }, -}); - -function disableJQueryAnimations() { - $.fx.off = true; -} - -// Disable jQuery animations -if (gon?.disable_animations) { - disableJQueryAnimations(); -} - // inject test utilities if necessary if (process.env.NODE_ENV !== 'production' && gon?.test_env) { - disableJQueryAnimations(); import(/* webpackMode: "eager" */ './test_utils/'); } @@ -239,17 +213,6 @@ document.addEventListener('DOMContentLoaded', () => { } }); - // eslint-disable-next-line no-jquery/no-ajax-events - $(document).ajaxError((e, xhrObj) => { - const ref = xhrObj.status; - - if (ref === 401) { - Flash(__('You need to be logged in.')); - } else if (ref === 404 || ref === 500) { - Flash(__('Something went wrong on our end.')); - } - }); - $('.navbar-toggler').on('click', () => { $('.header-content').toggleClass('menu-expanded'); }); diff --git a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.vue b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.vue index 3419abd4738..370e2f62275 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.vue @@ -127,10 +127,20 @@ export default { pipelineCoverageJobNumberText() { return n__('from %d job', 'from %d jobs', this.buildsWithCoverage.length); }, + pipelineCoverageTooltipDeltaDescription() { + const delta = parseFloat(this.pipelineCoverageDelta) || 0; + if (delta > 0) { + return s__('Pipeline|This change will increase the overall test coverage if merged.'); + } + if (delta < 0) { + return s__('Pipeline|This change will decrease the overall test coverage if merged.'); + } + return s__('Pipeline|This change will not change the overall test coverage if merged.'); + }, pipelineCoverageTooltipDescription() { return n__( - 'Coverage value for this pipeline was calculated by the coverage value of %d job.', - 'Coverage value for this pipeline was calculated by averaging the resulting coverage values of %d jobs.', + 'Test coverage value for this pipeline was calculated by the coverage value of %d job.', + 'Test coverage value for this pipeline was calculated by averaging the resulting coverage values of %d jobs.', this.buildsWithCoverage.length, ); }, @@ -218,13 +228,15 @@ export default {
- {{ s__('Pipeline|Coverage') }} {{ pipeline.coverage }}% + {{ s__('Pipeline|Test coverage') }} {{ pipeline.coverage }}% ({{ pipelineCoverageDelta }}%) + ({{ pipelineCoverageDelta }}%) + {{ pipelineCoverageJobNumberText }} @@ -242,6 +254,12 @@ export default { {{ build.name }} ({{ build.coverage }}%)
+ + {{ pipelineCoverageTooltipDeltaDescription }} + diff --git a/app/finders/issues_finder.rb b/app/finders/issues_finder.rb index 5c9010ee3e0..fbf71a24f7e 100644 --- a/app/finders/issues_finder.rb +++ b/app/finders/issues_finder.rb @@ -47,6 +47,15 @@ class IssuesFinder < IssuableFinder # rubocop: disable CodeReuse/ActiveRecord def with_confidentiality_access_check return Issue.all if params.user_can_see_all_confidential_issues? + + if Feature.enabled?(:optimize_issue_filter_assigned_to_self, default_enabled: :yaml) + # If already filtering by assignee we can skip confidentiality since a user + # can always see confidential issues assigned to them. This is just an + # optimization since a very common usecase of this Finder is to load the + # count of issues assigned to the user for the header bar. + return Issue.all if current_user && params.assignees.include?(current_user) + end + return Issue.where('issues.confidential IS NOT TRUE') if params.user_cannot_see_confidential_issues? Issue.where(' diff --git a/app/services/ci/pipeline_trigger_service.rb b/app/services/ci/pipeline_trigger_service.rb index dbbaefb2b2f..602ae53beaf 100644 --- a/app/services/ci/pipeline_trigger_service.rb +++ b/app/services/ci/pipeline_trigger_service.rb @@ -73,11 +73,7 @@ module Ci end def variables - if ::Feature.enabled?(:ci_trigger_payload_into_pipeline, project, default_enabled: :yaml) - param_variables + [payload_variable] - else - param_variables - end + param_variables + [payload_variable] end def param_variables diff --git a/app/services/namespaces/in_product_marketing_emails_service.rb b/app/services/namespaces/in_product_marketing_emails_service.rb index cbbcbe2233f..3a03baa7364 100644 --- a/app/services/namespaces/in_product_marketing_emails_service.rb +++ b/app/services/namespaces/in_product_marketing_emails_service.rb @@ -110,7 +110,8 @@ module Namespaces end def range - (interval + 1).days.ago.beginning_of_day..(interval + 1).days.ago.end_of_day + date = (interval + 1).days.ago + date.beginning_of_day..date.end_of_day end def incomplete_action diff --git a/changelogs/unreleased/270129-conan-project-download-urls-bug.yml b/changelogs/unreleased/270129-conan-project-download-urls-bug.yml new file mode 100644 index 00000000000..876e975fe4e --- /dev/null +++ b/changelogs/unreleased/270129-conan-project-download-urls-bug.yml @@ -0,0 +1,6 @@ +--- +title: Fix Conan project-level API to return correct download-urls and fix Conan project-level + functionality +merge_request: 56899 +author: +type: fixed diff --git a/changelogs/unreleased/283893-mr-pipeline-coverage-update.yml b/changelogs/unreleased/283893-mr-pipeline-coverage-update.yml new file mode 100644 index 00000000000..79164376258 --- /dev/null +++ b/changelogs/unreleased/283893-mr-pipeline-coverage-update.yml @@ -0,0 +1,5 @@ +--- +title: Clarify what coverage means on the merge request pipeline section +merge_request: 57275 +author: +type: added diff --git a/changelogs/unreleased/321027-remove-ci_trigger_payload_into_pipeline.yml b/changelogs/unreleased/321027-remove-ci_trigger_payload_into_pipeline.yml new file mode 100644 index 00000000000..e33c9306f28 --- /dev/null +++ b/changelogs/unreleased/321027-remove-ci_trigger_payload_into_pipeline.yml @@ -0,0 +1,5 @@ +--- +title: Remove the FF ci_trigger_payload_into_pipeline +merge_request: 57087 +author: +type: other diff --git a/changelogs/unreleased/325470-optimize-confidentiality-filter-on-issue-finder.yml b/changelogs/unreleased/325470-optimize-confidentiality-filter-on-issue-finder.yml new file mode 100644 index 00000000000..be9675b4f3e --- /dev/null +++ b/changelogs/unreleased/325470-optimize-confidentiality-filter-on-issue-finder.yml @@ -0,0 +1,5 @@ +--- +title: Optimize database performance of loading assigned issue count on header bar +merge_request: 57073 +author: +type: performance diff --git a/config/feature_flags/development/ci_trigger_payload_into_pipeline.yml b/config/feature_flags/development/optimize_issue_filter_assigned_to_self.yml similarity index 52% rename from config/feature_flags/development/ci_trigger_payload_into_pipeline.yml rename to config/feature_flags/development/optimize_issue_filter_assigned_to_self.yml index 2130c6151e8..07a92fbff8e 100644 --- a/config/feature_flags/development/ci_trigger_payload_into_pipeline.yml +++ b/config/feature_flags/development/optimize_issue_filter_assigned_to_self.yml @@ -1,8 +1,8 @@ --- -name: ci_trigger_payload_into_pipeline -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/53837 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/321027 -milestone: '13.9' +name: optimize_issue_filter_assigned_to_self +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/57073 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/325470 +milestone: '13.11' type: development -group: group::pipeline authoring -default_enabled: true +group: group::global search +default_enabled: false diff --git a/config/metrics/schema.json b/config/metrics/schema.json index cc1eafcf0ba..4e5d4162a05 100644 --- a/config/metrics/schema.json +++ b/config/metrics/schema.json @@ -26,7 +26,7 @@ }, "status": { "type": ["string"], - "enum": ["data_available", "planned", "in_progress", "implemented", "not_used", "deprecated"] + "enum": ["data_available", "implemented", "not_used", "deprecated"] }, "milestone": { "type": ["string", "null"], diff --git a/config/webpack.config.js b/config/webpack.config.js index 39add7def22..2b770d67cc5 100644 --- a/config/webpack.config.js +++ b/config/webpack.config.js @@ -121,6 +121,7 @@ const alias = { images: path.join(ROOT_PATH, 'app/assets/images'), vendor: path.join(ROOT_PATH, 'vendor/assets/javascripts'), vue$: 'vue/dist/vue.esm.js', + jquery$: 'jquery/dist/jquery.slim.js', spec: path.join(ROOT_PATH, 'spec/javascripts'), jest: path.join(ROOT_PATH, 'spec/frontend'), shared_queries: path.join(ROOT_PATH, 'app/graphql/queries'), diff --git a/config/webpack.vendor.config.js b/config/webpack.vendor.config.js index 7e5365987ee..6d337c1d82b 100644 --- a/config/webpack.vendor.config.js +++ b/config/webpack.vendor.config.js @@ -13,6 +13,9 @@ module.exports = { mode: 'development', resolve: { extensions: ['.js'], + alias: { + jquery$: 'jquery/dist/jquery.slim.js', + }, }, // ensure output is not generated when errors are encountered @@ -22,7 +25,7 @@ module.exports = { entry: { vendor: [ - 'jquery', + 'jquery/dist/jquery.slim.js', 'pdfjs-dist/build/pdf', 'pdfjs-dist/build/pdf.worker.min', 'sql.js', diff --git a/danger/changelog/Dangerfile b/danger/changelog/Dangerfile index 444303d4b9e..26a844a3757 100644 --- a/danger/changelog/Dangerfile +++ b/danger/changelog/Dangerfile @@ -21,6 +21,7 @@ def check_changelog_yaml(path) fail "`type` should be set, in #{helper.html_link(path)}! #{SEE_DOC}" if yaml["type"].nil? return if helper.security_mr? + return if helper.mr_iid.empty? cherry_pick_against_stable_branch = helper.cherry_pick_mr? && helper.stable_branch? @@ -28,12 +29,12 @@ def check_changelog_yaml(path) mr_line = raw_file.lines.find_index("merge_request:\n") if mr_line - markdown(format(SUGGEST_MR_COMMENT, mr_iid: gitlab.mr_json["iid"]), file: path, line: mr_line.succ) + markdown(format(SUGGEST_MR_COMMENT, mr_iid: helper.mr_iid), file: path, line: mr_line.succ) else - message "Consider setting `merge_request` to #{gitlab.mr_json["iid"]} in #{helper.html_link(path)}. #{SEE_DOC}" + message "Consider setting `merge_request` to #{helper.mr_iid} in #{helper.html_link(path)}. #{SEE_DOC}" end - elsif yaml["merge_request"] != gitlab.mr_json["iid"] && !cherry_pick_against_stable_branch - fail "Merge request ID was not set to #{gitlab.mr_json["iid"]}! #{SEE_DOC}" + elsif yaml["merge_request"] != helper.mr_iid && !cherry_pick_against_stable_branch + fail "Merge request ID was not set to #{helper.mr_iid}! #{SEE_DOC}" end rescue Psych::Exception # YAML could not be parsed, fail the build. diff --git a/doc/ci/triggers/README.md b/doc/ci/triggers/README.md index fa97cbdfcec..9504f3cc237 100644 --- a/doc/ci/triggers/README.md +++ b/doc/ci/triggers/README.md @@ -188,38 +188,13 @@ source repository. Be sure to URL-encode `ref` if it contains slashes. ### Using webhook payload in the triggered pipeline > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/31197) in GitLab 13.9. -> - It's [deployed behind a feature flag](../../user/feature_flags.md), enabled by default. -> - It's enabled on GitLab.com. -> - It's recommended for production use. -> - For GitLab self-managed instances, GitLab administrators can opt to [disable it](#enable-or-disable-the-trigger_payload-variable). **(FREE SELF)** - -WARNING: -This feature might not be available to you. Check the **version history** note above for details. +> - [Feature flag removed](https://gitlab.com/gitlab-org/gitlab/-/issues/321027) in GitLab 13.11. If you trigger a pipeline by using a webhook, you can access the webhook payload with the `TRIGGER_PAYLOAD` [predefined CI/CD variable](../variables/predefined_variables.md). The payload is exposed as a [file-type variable](../variables/README.md#custom-cicd-variables-of-type-file), so you can access the data with `cat $TRIGGER_PAYLOAD` or a similar command. -#### Enable or disable the `TRIGGER_PAYLOAD` variable - -The `TRIGGER_PAYLOAD` CI/CD variable is under development but ready for production use. -It is deployed behind a feature flag that is **enabled by default**. -[GitLab administrators with access to the GitLab Rails console](../../administration/feature_flags.md) -can opt to disable it. - -To disable it: - -```ruby -Feature.disable(:ci_trigger_payload_into_pipeline) -``` - -To enable it: - -```ruby -Feature.enable(:ci_trigger_payload_into_pipeline) -``` - ## Making use of trigger variables You can pass any number of arbitrary variables in the trigger API call and they diff --git a/doc/development/usage_ping/metrics_dictionary.md b/doc/development/usage_ping/metrics_dictionary.md index 85d4b957640..103f6d5d9eb 100644 --- a/doc/development/usage_ping/metrics_dictionary.md +++ b/doc/development/usage_ping/metrics_dictionary.md @@ -33,7 +33,7 @@ Each metric is defined in a separate YAML file consisting of a number of fields: | `product_group` | yes | The [group](https://gitlab.com/gitlab-com/www-gitlab-com/blob/master/data/stages.yml) that owns the metric. | | `product_category` | no | The [product category](https://gitlab.com/gitlab-com/www-gitlab-com/blob/master/data/categories.yml) for the metric. | | `value_type` | yes | `string`; one of [`string`, `number`, `boolean`, `object`](https://json-schema.org/understanding-json-schema/reference/type.html). | -| `status` | yes | `string`; status of the metric, may be set to `data_available`, `planned`, `in_progress`, `implemented`, `not_used`, `deprecated` | +| `status` | yes | `string`; [status](#metric-statuses) of the metric, may be set to `data_available`, `implemented`, `not_used`, `deprecated`. | | `time_frame` | yes | `string`; may be set to a value like `7d`, `28d`, `all`, `none`. | | `data_source` | yes | `string`; may be set to a value like `database`, `redis`, `redis_hll`, `prometheus`, `ruby`. | | `distribution` | yes | `array`; may be set to one of `ce, ee` or `ee`. The [distribution](https://about.gitlab.com/handbook/marketing/strategic-marketing/tiers/#definitions) where the tracked feature is available. | @@ -43,6 +43,16 @@ Each metric is defined in a separate YAML file consisting of a number of fields: | `introduced_by_url` | no | The URL to the Merge Request that introduced the metric. | | `skip_validation` | no | This should **not** be set. [Used for imported metrics until we review, update and make them valid](https://gitlab.com/groups/gitlab-org/-/epics/5425). | +### Metric statuses + +Metric definitions can have one of the following statuses: + +- `data_available`: Metric data is available and used in a Sisense dashboard. +- `implemented`: Metric is implemented but data is not yet available. This is a temporary + status for newly added metrics awaiting inclusion in a new release. +- `not_used`: Metric is not used in any dashboard. +- `deprecated`: Metric is deprecated and possibly planned to be removed. + ### Example YAML metric definition The linked [`uuid`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/config/metrics/license/uuid.yml) diff --git a/doc/operations/incident_management/oncall_schedules.md b/doc/operations/incident_management/oncall_schedules.md index 5c7d4366c63..78959dd5c2d 100644 --- a/doc/operations/incident_management/oncall_schedules.md +++ b/doc/operations/incident_management/oncall_schedules.md @@ -6,7 +6,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w # On-call Schedule Management **(PREMIUM)** -> [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/4544) in [GitLab Premium](https://about.gitlab.com/pricing/) 13.10. +> [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/4544) in [GitLab Premium](https://about.gitlab.com/pricing/) 13.11. Use on-call schedule management to create schedules for responders to rotate on-call responsibilities. Maintain the availability of your software services by putting your teams on-call. diff --git a/doc/user/admin_area/settings/sign_in_restrictions.md b/doc/user/admin_area/settings/sign_in_restrictions.md index 50fd6a35354..7b2928a3873 100644 --- a/doc/user/admin_area/settings/sign_in_restrictions.md +++ b/doc/user/admin_area/settings/sign_in_restrictions.md @@ -25,6 +25,10 @@ You can restrict the password authentication for web interface and Git over HTTP ## Admin Mode +> - [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/2158) in GitLab 13.10. +> - It's [deployed behind the feature flag](../../../user/feature_flags.md) `:user_mode_in_session`, disabled by default. +> - To use it in GitLab self-managed instances, ask a GitLab administrator to enable it. + When this feature is enabled, instance administrators are limited as regular users. During that period, they do not have access to all projects, groups, or the **Admin Area** menu. @@ -47,7 +51,7 @@ OmniAuth providers and LDAP auth. The Admin Mode status is stored in the active session and remains active until it is explicitly disabled (it will be disabled automatically after a timeout otherwise). -### Limitations +### Limitations of Admin Mode The following access methods are **not** protected by Admin Mode: @@ -61,7 +65,7 @@ authentication steps. We may address these limitations in the future. For more information see the following epic: [Admin mode for GitLab Administrators](https://gitlab.com/groups/gitlab-org/-/epics/2158). -### Troubleshooting +### Troubleshooting Admin Mode If necessary, you can disable **Admin Mode** as an administrator by using one of these two methods: @@ -76,6 +80,25 @@ If necessary, you can disable **Admin Mode** as an administrator by using one of ```ruby ::Gitlab::CurrentSettings.update_attributes!(admin_mode: false) ``` + +## Enable or disable Admin Mode + +Admin Mode is under development and not ready for production use. It is +deployed behind a feature flag that is **disabled by default**. +[GitLab administrators with access to the GitLab Rails console](../../../administration/feature_flags.md) +can enable it. + +To enable it: + +```ruby +Feature.enable(:user_mode_in_session) +``` + +To disable it: + +```ruby +Feature.disable(:user_mode_in_session) +``` ## Two-factor authentication diff --git a/doc/user/application_security/dast/index.md b/doc/user/application_security/dast/index.md index ea365f1daed..77d906332dd 100644 --- a/doc/user/application_security/dast/index.md +++ b/doc/user/application_security/dast/index.md @@ -536,7 +536,8 @@ variables: ### URL scan -> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/214120) in [GitLab Ultimate](https://about.gitlab.com/pricing/) 13.4. +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/214120) in [GitLab Ultimate](https://about.gitlab.com/pricing/) 13.4. +> - [Improved](https://gitlab.com/gitlab-org/gitlab/-/issues/273141) in GitLab 13.11. A URL scan allows you to specify which parts of a website are scanned by DAST. @@ -560,26 +561,19 @@ category/shoes/page1.html ``` To scan the URLs in that file, set the CI/CD variable `DAST_PATHS_FILE` to the path of that file. +The file can be checked into the project repository or generated as an artifact by a job that +runs before DAST. + +By default, DAST scans do not clone the project repository. Instruct the DAST job to clone +the project by setting `GIT_STRATEGY` to fetch. Give a file path relative to `CI_PROJECT_DIR` to `DAST_PATHS_FILE`. ```yaml include: - template: DAST.gitlab-ci.yml variables: - DAST_PATHS_FILE: url_file.txt -``` - -By default, DAST scans do not clone the project repository. If the file is checked in to the project, instruct the DAST job to clone the project by setting GIT_STRATEGY to fetch. The file is expected to be in the `/zap/wrk` directory. - -```yaml -dast: - script: - - mkdir -p /zap/wrk - - cp url_file.txt /zap/wrk/url_file.txt - - /analyze -t $DAST_WEBSITE - variables: - GIT_STRATEGY: fetch - DAST_PATHS_FILE: url_file.txt + GIT_STRATEGY: fetch + DAST_PATHS_FILE: url_file.txt # url_file.txt lives in the root directory of the project ``` ##### Use `DAST_PATHS` CI/CD variable @@ -653,7 +647,7 @@ DAST can be [configured](#customizing-the-dast-settings) using CI/CD variables. | `DAST_MASK_HTTP_HEADERS` | string | Comma-separated list of request and response headers to be masked (GitLab 13.1). Must contain **all** headers to be masked. Refer to [list of headers that are masked by default](#hide-sensitive-information). | | `DAST_EXCLUDE_URLS` | URLs | The URLs to skip during the authenticated scan; comma-separated. Regular expression syntax can be used to match multiple URLs. For example, `.*` matches an arbitrary character sequence. Not supported for API scans. | | `DAST_FULL_SCAN_ENABLED` | boolean | Set to `true` to run a [ZAP Full Scan](https://github.com/zaproxy/zaproxy/wiki/ZAP-Full-Scan) instead of a [ZAP Baseline Scan](https://github.com/zaproxy/zaproxy/wiki/ZAP-Baseline-Scan). Default: `false` | -| `DAST_FULL_SCAN_DOMAIN_VALIDATION_REQUIRED` | boolean | Set to `true` to require [domain validation](#domain-validation) when running DAST full scans. Not supported for API scans. Default: `false` | +| `DAST_FULL_SCAN_DOMAIN_VALIDATION_REQUIRED` | boolean | [Deprecated](https://gitlab.com/gitlab-org/gitlab/-/issues/293595) in GitLab 13.8, to be removed in 14.0. Set to `true` to require [domain validation](#domain-validation) when running DAST full scans. Not supported for API scans. Default: `false` | | `DAST_AUTO_UPDATE_ADDONS` | boolean | ZAP add-ons are pinned to specific versions in the DAST Docker image. Set to `true` to download the latest versions when the scan starts. Default: `false` | | `DAST_API_HOST_OVERRIDE` | string | Used to override domains defined in API specification files. Only supported when importing the API specification from a URL. Example: `example.com:8080` | | `DAST_EXCLUDE_RULES` | string | Set to a comma-separated list of Vulnerability Rule IDs to exclude them from running during the scan. Rule IDs are numbers and can be found from the DAST log or on the [ZAP project](https://github.com/zaproxy/zaproxy/blob/develop/docs/scanners.md). For example, `HTTP Parameter Override` has a rule ID of `10026`. **Note:** In earlier versions of GitLab the excluded rules were executed but alerts they generated were suppressed. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/118641) in GitLab 12.10. | @@ -666,7 +660,7 @@ DAST can be [configured](#customizing-the-dast-settings) using CI/CD variables. | `DAST_INCLUDE_ALPHA_VULNERABILITIES` | boolean | Set to `true` to include alpha passive and active scan rules. Default: `false`. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/12652) in GitLab 13.1. | | `DAST_USE_AJAX_SPIDER` | boolean | Set to `true` to use the AJAX spider in addition to the traditional spider, useful for crawling sites that require JavaScript. Default: `false`. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/12652) in GitLab 13.1. | | `DAST_PATHS` | string | Set to a comma-separated list of URLs for DAST to scan. For example, `/page1.html,/category1/page3.html,/page2.html`. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/214120) in GitLab 13.4. | -| `DAST_PATHS_FILE` | string | The file path containing the paths within `DAST_WEBSITE` to scan. The file must be plain text with one path per line and be in `/zap/wrk`. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/258825) in GitLab 13.6. | +| `DAST_PATHS_FILE` | string | The file path containing the paths within `DAST_WEBSITE` to scan. The file must be plain text with one path per line. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/258825) in GitLab 13.6. | | `DAST_SUBMIT_FIELD` | string | The `id` or `name` of the element that when clicked submits the login form or the password form of a multi-page login process. [Introduced](https://gitlab.com/gitlab-org/gitlab-ee/issues/9894) in GitLab 12.4. | | `DAST_FIRST_SUBMIT_FIELD` | string | The `id` or `name` of the element that when clicked submits the username form of a multi-page login process. [Introduced](https://gitlab.com/gitlab-org/gitlab-ee/issues/9894) in GitLab 12.4. | | `DAST_ZAP_CLI_OPTIONS` | string | ZAP server command-line options. For example, `-Xmx3072m` would set the Java maximum memory allocation pool size. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/12652) in GitLab 13.1. | diff --git a/doc/user/packages/conan_repository/index.md b/doc/user/packages/conan_repository/index.md index 3b8be68cff6..af1f2f57e0e 100644 --- a/doc/user/packages/conan_repository/index.md +++ b/doc/user/packages/conan_repository/index.md @@ -402,16 +402,3 @@ The GitLab Conan repository supports the following Conan CLI commands: packages you have permission to view. - `conan info`: View the information on a given package from the Package Registry. - `conan remove`: Delete the package from the Package Registry. - -## Troubleshooting Conan packages - -### `ERROR: was not found in remote ` - -When you attempt to install a Conan package, you might receive a `404` error -like `ERROR: was not found in remote `. - -This issue occurs when you request a download from the project-level Conan API. -The resulting URL is missing is project's `/` and Conan commands, like -`conan install`, fail. - -For more information, see [issue 270129](https://gitlab.com/gitlab-org/gitlab/-/issues/270129). diff --git a/jest.config.base.js b/jest.config.base.js index 5b7ab4d9276..52e29339e55 100644 --- a/jest.config.base.js +++ b/jest.config.base.js @@ -50,6 +50,7 @@ module.exports = (path, options = {}) => { 'emojis(/.*).json': '/fixtures/emojis$1.json', '^spec/test_constants$': '/spec/frontend/__helpers__/test_constants', '^jest/(.*)$': '/spec/frontend/$1', + '^jquery$': '/node_modules/jquery/dist/jquery.slim.js', ...extModuleNameMapper, }; diff --git a/lib/api/helpers/packages/conan/api_helpers.rb b/lib/api/helpers/packages/conan/api_helpers.rb index d5f5448fd42..24ebeb007d3 100644 --- a/lib/api/helpers/packages/conan/api_helpers.rb +++ b/lib/api/helpers/packages/conan/api_helpers.rb @@ -14,7 +14,8 @@ module API package, current_user, project, - conan_package_reference: params[:conan_package_reference] + conan_package_reference: params[:conan_package_reference], + id: params[:id] ) render_api_error!("No recipe manifest found", 404) if yield(presenter).empty? diff --git a/lib/gitlab/background_migration/user_mentions/lib/banzai/reference_parser/isolated_mentioned_project_parser.rb b/lib/gitlab/background_migration/user_mentions/lib/banzai/reference_parser/isolated_mentioned_project_parser.rb new file mode 100644 index 00000000000..5930d65bc2c --- /dev/null +++ b/lib/gitlab/background_migration/user_mentions/lib/banzai/reference_parser/isolated_mentioned_project_parser.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +module Gitlab + module BackgroundMigration + module UserMentions + module Lib + module Banzai + module ReferenceParser + # isolated Banzai::ReferenceParser::MentionedGroupParser + class IsolatedMentionedProjectParser < ::Banzai::ReferenceParser::MentionedProjectParser + extend ::Gitlab::Utils::Override + + self.reference_type = :user + + override :references_relation + def references_relation + ::Gitlab::BackgroundMigration::UserMentions::Models::Project + end + end + end + end + end + end + end +end diff --git a/lib/gitlab/background_migration/user_mentions/lib/banzai/reference_parser/isolated_mentioned_user_parser.rb b/lib/gitlab/background_migration/user_mentions/lib/banzai/reference_parser/isolated_mentioned_user_parser.rb new file mode 100644 index 00000000000..f5f98517433 --- /dev/null +++ b/lib/gitlab/background_migration/user_mentions/lib/banzai/reference_parser/isolated_mentioned_user_parser.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +module Gitlab + module BackgroundMigration + module UserMentions + module Lib + module Banzai + module ReferenceParser + # isolated Banzai::ReferenceParser::MentionedGroupParser + class IsolatedMentionedUserParser < ::Banzai::ReferenceParser::MentionedUserParser + extend ::Gitlab::Utils::Override + + self.reference_type = :user + + override :references_relation + def references_relation + ::Gitlab::BackgroundMigration::UserMentions::Models::User + end + end + end + end + end + end + end +end diff --git a/lib/gitlab/background_migration/user_mentions/lib/gitlab/isolated_reference_extractor.rb b/lib/gitlab/background_migration/user_mentions/lib/gitlab/isolated_reference_extractor.rb index 1d3a3af81a1..8610129533d 100644 --- a/lib/gitlab/background_migration/user_mentions/lib/gitlab/isolated_reference_extractor.rb +++ b/lib/gitlab/background_migration/user_mentions/lib/gitlab/isolated_reference_extractor.rb @@ -7,7 +7,7 @@ module Gitlab module Gitlab # Extract possible GFM references from an arbitrary String for further processing. class IsolatedReferenceExtractor < ::Gitlab::ReferenceExtractor - REFERABLES = %i(isolated_mentioned_group).freeze + REFERABLES = %i(isolated_mentioned_group isolated_mentioned_user isolated_mentioned_project).freeze REFERABLES.each do |type| define_method("#{type}s") do diff --git a/lib/gitlab/background_migration/user_mentions/lib/gitlab/isolated_visibility_level.rb b/lib/gitlab/background_migration/user_mentions/lib/gitlab/isolated_visibility_level.rb new file mode 100644 index 00000000000..0334ea1dd08 --- /dev/null +++ b/lib/gitlab/background_migration/user_mentions/lib/gitlab/isolated_visibility_level.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +module Gitlab + module BackgroundMigration + module UserMentions + module Lib + module Gitlab + # Gitlab::IsolatedVisibilityLevel module + # + # Define allowed public modes that can be used for + # GitLab projects to determine project public mode + # + module IsolatedVisibilityLevel + extend ::ActiveSupport::Concern + + included do + scope :public_to_user, -> (user = nil) do + where(visibility_level: IsolatedVisibilityLevel.levels_for_user(user)) + end + end + + PRIVATE = 0 unless const_defined?(:PRIVATE) + INTERNAL = 10 unless const_defined?(:INTERNAL) + PUBLIC = 20 unless const_defined?(:PUBLIC) + + class << self + def levels_for_user(user = nil) + return [PUBLIC] unless user + + if user.can_read_all_resources? + [PRIVATE, INTERNAL, PUBLIC] + elsif user.external? + [PUBLIC] + else + [INTERNAL, PUBLIC] + end + end + end + + def private? + visibility_level_value == PRIVATE + end + + def internal? + visibility_level_value == INTERNAL + end + + def public? + visibility_level_value == PUBLIC + end + + def visibility_level_value + self[visibility_level_field] + end + end + end + end + end + end +end diff --git a/lib/gitlab/background_migration/user_mentions/models/commit_user_mention.rb b/lib/gitlab/background_migration/user_mentions/models/commit_user_mention.rb index bdb4d6c7d48..f4cc96c8bc0 100644 --- a/lib/gitlab/background_migration/user_mentions/models/commit_user_mention.rb +++ b/lib/gitlab/background_migration/user_mentions/models/commit_user_mention.rb @@ -7,6 +7,7 @@ module Gitlab module Models class CommitUserMention < ActiveRecord::Base self.table_name = 'commit_user_mentions' + self.inheritance_column = :_type_disabled def self.resource_foreign_key :commit_id diff --git a/lib/gitlab/background_migration/user_mentions/models/concerns/isolated_feature_gate.rb b/lib/gitlab/background_migration/user_mentions/models/concerns/isolated_feature_gate.rb new file mode 100644 index 00000000000..ba6b783f9f1 --- /dev/null +++ b/lib/gitlab/background_migration/user_mentions/models/concerns/isolated_feature_gate.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +module Gitlab + module BackgroundMigration + module UserMentions + module Models + module Concerns + # isolated FeatureGate module + module IsolatedFeatureGate + def flipper_id + return if new_record? + + "#{self.class.name}:#{id}" + end + end + end + end + end + end +end diff --git a/lib/gitlab/background_migration/user_mentions/models/concerns/isolated_mentionable.rb b/lib/gitlab/background_migration/user_mentions/models/concerns/isolated_mentionable.rb index be9c0ad2b3a..f684f789ea9 100644 --- a/lib/gitlab/background_migration/user_mentions/models/concerns/isolated_mentionable.rb +++ b/lib/gitlab/background_migration/user_mentions/models/concerns/isolated_mentionable.rb @@ -70,8 +70,8 @@ module Gitlab def build_mention_values(resource_foreign_key) refs = all_references(author) - mentioned_users_ids = array_to_sql(refs.mentioned_users.pluck(:id)) - mentioned_projects_ids = array_to_sql(refs.mentioned_projects.pluck(:id)) + mentioned_users_ids = array_to_sql(refs.isolated_mentioned_users.pluck(:id)) + mentioned_projects_ids = array_to_sql(refs.isolated_mentioned_projects.pluck(:id)) mentioned_groups_ids = array_to_sql(refs.isolated_mentioned_groups.pluck(:id)) return if mentioned_users_ids.blank? && mentioned_projects_ids.blank? && mentioned_groups_ids.blank? diff --git a/lib/gitlab/background_migration/user_mentions/models/concerns/namespace/recursive_traversal.rb b/lib/gitlab/background_migration/user_mentions/models/concerns/namespace/recursive_traversal.rb index 5cadfa45b5b..75759ed0111 100644 --- a/lib/gitlab/background_migration/user_mentions/models/concerns/namespace/recursive_traversal.rb +++ b/lib/gitlab/background_migration/user_mentions/models/concerns/namespace/recursive_traversal.rb @@ -6,7 +6,7 @@ module Gitlab module Models module Concerns module Namespace - # extracted methods for recursive traversing of namespace hierarchy + # isolate recursive traversal code for namespace hierarchy module RecursiveTraversal extend ActiveSupport::Concern diff --git a/lib/gitlab/background_migration/user_mentions/models/design_management/design.rb b/lib/gitlab/background_migration/user_mentions/models/design_management/design.rb index bdb90b5d2b9..d010d68600d 100644 --- a/lib/gitlab/background_migration/user_mentions/models/design_management/design.rb +++ b/lib/gitlab/background_migration/user_mentions/models/design_management/design.rb @@ -10,6 +10,9 @@ module Gitlab include EachBatch include Concerns::MentionableMigrationMethods + self.table_name = 'design_management_designs' + self.inheritance_column = :_type_disabled + def self.user_mention_model Gitlab::BackgroundMigration::UserMentions::Models::DesignUserMention end diff --git a/lib/gitlab/background_migration/user_mentions/models/design_user_mention.rb b/lib/gitlab/background_migration/user_mentions/models/design_user_mention.rb index 68205ecd3c2..eb00f6cfa3f 100644 --- a/lib/gitlab/background_migration/user_mentions/models/design_user_mention.rb +++ b/lib/gitlab/background_migration/user_mentions/models/design_user_mention.rb @@ -7,6 +7,7 @@ module Gitlab module Models class DesignUserMention < ActiveRecord::Base self.table_name = 'design_user_mentions' + self.inheritance_column = :_type_disabled def self.resource_foreign_key :design_id diff --git a/lib/gitlab/background_migration/user_mentions/models/epic.rb b/lib/gitlab/background_migration/user_mentions/models/epic.rb index 61d9244a4c9..cfd9a4faa9b 100644 --- a/lib/gitlab/background_migration/user_mentions/models/epic.rb +++ b/lib/gitlab/background_migration/user_mentions/models/epic.rb @@ -17,10 +17,10 @@ module Gitlab cache_markdown_field :description, issuable_state_filter_enabled: true self.table_name = 'epics' + self.inheritance_column = :_type_disabled - belongs_to :author, class_name: "User" - belongs_to :project - belongs_to :group + belongs_to :author, class_name: "::Gitlab::BackgroundMigration::UserMentions::Models::User" + belongs_to :group, class_name: "::Gitlab::BackgroundMigration::UserMentions::Models::Group" def self.user_mention_model Gitlab::BackgroundMigration::UserMentions::Models::EpicUserMention diff --git a/lib/gitlab/background_migration/user_mentions/models/epic_user_mention.rb b/lib/gitlab/background_migration/user_mentions/models/epic_user_mention.rb index 4e3ce9bf3a7..579e4d99612 100644 --- a/lib/gitlab/background_migration/user_mentions/models/epic_user_mention.rb +++ b/lib/gitlab/background_migration/user_mentions/models/epic_user_mention.rb @@ -7,6 +7,7 @@ module Gitlab module Models class EpicUserMention < ActiveRecord::Base self.table_name = 'epic_user_mentions' + self.inheritance_column = :_type_disabled def self.resource_foreign_key :epic_id diff --git a/lib/gitlab/background_migration/user_mentions/models/group.rb b/lib/gitlab/background_migration/user_mentions/models/group.rb index bc04172b9a2..a8b4b59b06c 100644 --- a/lib/gitlab/background_migration/user_mentions/models/group.rb +++ b/lib/gitlab/background_migration/user_mentions/models/group.rb @@ -7,6 +7,8 @@ module Gitlab # isolated Group model class Group < ::Gitlab::BackgroundMigration::UserMentions::Models::Namespace self.store_full_sti_class = false + self.inheritance_column = :_type_disabled + has_one :saml_provider def self.declarative_policy_class diff --git a/lib/gitlab/background_migration/user_mentions/models/merge_request.rb b/lib/gitlab/background_migration/user_mentions/models/merge_request.rb index 6b52afea17c..13addcc3c55 100644 --- a/lib/gitlab/background_migration/user_mentions/models/merge_request.rb +++ b/lib/gitlab/background_migration/user_mentions/models/merge_request.rb @@ -17,10 +17,11 @@ module Gitlab cache_markdown_field :description, issuable_state_filter_enabled: true self.table_name = 'merge_requests' + self.inheritance_column = :_type_disabled - belongs_to :author, class_name: "User" - belongs_to :target_project, class_name: "Project" - belongs_to :source_project, class_name: "Project" + belongs_to :author, class_name: "::Gitlab::BackgroundMigration::UserMentions::Models::User" + belongs_to :target_project, class_name: "::Gitlab::BackgroundMigration::UserMentions::Models::Project" + belongs_to :source_project, class_name: "::Gitlab::BackgroundMigration::UserMentions::Models::Project" alias_attribute :project, :target_project diff --git a/lib/gitlab/background_migration/user_mentions/models/merge_request_user_mention.rb b/lib/gitlab/background_migration/user_mentions/models/merge_request_user_mention.rb index e9b85e9cb8c..4a85892d7b8 100644 --- a/lib/gitlab/background_migration/user_mentions/models/merge_request_user_mention.rb +++ b/lib/gitlab/background_migration/user_mentions/models/merge_request_user_mention.rb @@ -7,6 +7,7 @@ module Gitlab module Models class MergeRequestUserMention < ActiveRecord::Base self.table_name = 'merge_request_user_mentions' + self.inheritance_column = :_type_disabled def self.resource_foreign_key :merge_request_id diff --git a/lib/gitlab/background_migration/user_mentions/models/namespace.rb b/lib/gitlab/background_migration/user_mentions/models/namespace.rb index 8fa0db5fd4b..6587417d048 100644 --- a/lib/gitlab/background_migration/user_mentions/models/namespace.rb +++ b/lib/gitlab/background_migration/user_mentions/models/namespace.rb @@ -5,9 +5,11 @@ module Gitlab module UserMentions module Models # isolated Namespace model - class Namespace < ApplicationRecord - include FeatureGate - include ::Gitlab::VisibilityLevel + class Namespace < ActiveRecord::Base + self.inheritance_column = :_type_disabled + + include Concerns::IsolatedFeatureGate + include Gitlab::BackgroundMigration::UserMentions::Lib::Gitlab::IsolatedVisibilityLevel include ::Gitlab::Utils::StrongMemoize include Gitlab::BackgroundMigration::UserMentions::Models::Concerns::Namespace::RecursiveTraversal diff --git a/lib/gitlab/background_migration/user_mentions/models/note.rb b/lib/gitlab/background_migration/user_mentions/models/note.rb index a3224c8c456..7da933c7b11 100644 --- a/lib/gitlab/background_migration/user_mentions/models/note.rb +++ b/lib/gitlab/background_migration/user_mentions/models/note.rb @@ -16,9 +16,9 @@ module Gitlab attr_mentionable :note, pipeline: :note cache_markdown_field :note, pipeline: :note, issuable_state_filter_enabled: true - belongs_to :author, class_name: "User" + belongs_to :author, class_name: "::Gitlab::BackgroundMigration::UserMentions::Models::User" belongs_to :noteable, polymorphic: true - belongs_to :project + belongs_to :project, class_name: "::Gitlab::BackgroundMigration::UserMentions::Models::Project" def for_personal_snippet? noteable && noteable.class.name == 'PersonalSnippet' diff --git a/lib/gitlab/background_migration/user_mentions/models/project.rb b/lib/gitlab/background_migration/user_mentions/models/project.rb new file mode 100644 index 00000000000..4e02bf97d12 --- /dev/null +++ b/lib/gitlab/background_migration/user_mentions/models/project.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +module Gitlab + module BackgroundMigration + module UserMentions + module Models + # isolated Namespace model + class Project < ActiveRecord::Base + include Concerns::IsolatedFeatureGate + include Gitlab::BackgroundMigration::UserMentions::Lib::Gitlab::IsolatedVisibilityLevel + + self.table_name = 'projects' + self.inheritance_column = :_type_disabled + + belongs_to :group, -> { where(type: 'Group') }, foreign_key: 'namespace_id', class_name: "::Gitlab::BackgroundMigration::UserMentions::Models::Group" + belongs_to :namespace, class_name: "::Gitlab::BackgroundMigration::UserMentions::Models::Namespace" + alias_method :parent, :namespace + + # Returns a collection of projects that is either public or visible to the + # logged in user. + def self.public_or_visible_to_user(user = nil, min_access_level = nil) + min_access_level = nil if user&.can_read_all_resources? + + return public_to_user unless user + + if user.is_a?(::Gitlab::BackgroundMigration::UserMentions::Models::User) + where('EXISTS (?) OR projects.visibility_level IN (?)', + user.authorizations_for_projects(min_access_level: min_access_level), + levels_for_user(user)) + end + end + + def grafana_integration + nil + end + + def default_issues_tracker? + true # we do not care of the issue tracker type(internal or external) when parsing mentions + end + + def visibility_level_field + :visibility_level + end + end + end + end + end +end diff --git a/lib/gitlab/background_migration/user_mentions/models/user.rb b/lib/gitlab/background_migration/user_mentions/models/user.rb new file mode 100644 index 00000000000..a30220b6934 --- /dev/null +++ b/lib/gitlab/background_migration/user_mentions/models/user.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +module Gitlab + module BackgroundMigration + module UserMentions + module Models + # isolated Namespace model + class User < ActiveRecord::Base + include Concerns::IsolatedFeatureGate + + self.table_name = 'users' + self.inheritance_column = :_type_disabled + + has_many :project_authorizations, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent + + def authorizations_for_projects(min_access_level: nil, related_project_column: 'projects.id') + authorizations = project_authorizations + .select(1) + .where("project_authorizations.project_id = #{related_project_column}") + + return authorizations unless min_access_level.present? + + authorizations.where('project_authorizations.access_level >= ?', min_access_level) + end + + def can_read_all_resources? + can?(:read_all_resources) + end + + def can?(action, subject = :global) + Ability.allowed?(self, action, subject) + end + end + end + end + end +end diff --git a/lib/gitlab/database/background_migration/scheduler.rb b/lib/gitlab/database/background_migration/batched_migration_runner.rb similarity index 78% rename from lib/gitlab/database/background_migration/scheduler.rb rename to lib/gitlab/database/background_migration/batched_migration_runner.rb index 5f8a5ec06a5..d1f208617b7 100644 --- a/lib/gitlab/database/background_migration/scheduler.rb +++ b/lib/gitlab/database/background_migration/batched_migration_runner.rb @@ -3,12 +3,12 @@ module Gitlab module Database module BackgroundMigration - class Scheduler - def perform(migration_wrapper: BatchedMigrationWrapper.new) - active_migration = BatchedMigration.active.queue_order.first - - return unless active_migration&.interval_elapsed? + class BatchedMigrationRunner + def initialize(migration_wrapper = BatchedMigrationWrapper.new) + @migration_wrapper = migration_wrapper + end + def run_migration_job(active_migration) if next_batched_job = create_next_batched_job!(active_migration) migration_wrapper.perform(next_batched_job) else @@ -16,8 +16,18 @@ module Gitlab end end + def run_entire_migration(migration) + while migration.active? + run_migration_job(migration) + + migration.reload_last_job + end + end + private + attr_reader :migration_wrapper + def create_next_batched_job!(active_migration) next_batch_range = find_next_batch_range(active_migration) diff --git a/lib/gitlab/database/migration_helpers.rb b/lib/gitlab/database/migration_helpers.rb index 31e733050e1..bc50f0c3c04 100644 --- a/lib/gitlab/database/migration_helpers.rb +++ b/lib/gitlab/database/migration_helpers.rb @@ -4,6 +4,7 @@ module Gitlab module Database module MigrationHelpers include Migrations::BackgroundMigrationHelpers + include DynamicModelHelpers # https://www.postgresql.org/docs/current/sql-syntax-lexical.html#SQL-SYNTAX-IDENTIFIERS MAX_IDENTIFIER_NAME_LENGTH = 63 @@ -927,52 +928,19 @@ module Gitlab # This is crucial for Primary Key conversions, because setting a column # as the PK converts even check constraints to NOT NULL constraints # and forces an inline re-verification of the whole table. - # - It backfills the new column with the values of the existing primary key - # by scheduling background jobs. - # - It tracks the scheduled background jobs through the use of - # Gitlab::Database::BackgroundMigrationJob - # which allows a more thorough check that all jobs succeeded in the - # cleanup migration and is way faster for very large tables. - # - It sets up a trigger to keep the two columns in sync - # - It does not schedule a cleanup job: we have to do that with followup - # post deployment migrations in the next release. + # - It sets up a trigger to keep the two columns in sync. # - # This needs to be done manually by using the - # `cleanup_initialize_conversion_of_integer_to_bigint` - # (not yet implemented - check #288005) + # Note: this helper is intended to be used in a regular (pre-deployment) migration. + # + # This helper is part 1 of a multi-step migration process: + # 1. initialize_conversion_of_integer_to_bigint to create the new column and database triggers + # 2. backfill_conversion_of_integer_to_bigint to copy historic data using background migrations + # 3. remaining steps TBD, see #288005 # # table - The name of the database table containing the column # column - The name of the column that we want to convert to bigint. # primary_key - The name of the primary key column (most often :id) - # batch_size - The number of rows to schedule in a single background migration - # sub_batch_size - The smaller batches that will be used by each scheduled job - # to update the table. Useful to keep each update at ~100ms while executing - # more updates per interval (2.minutes) - # Note that each execution of a sub-batch adds a constant 100ms sleep - # time in between the updates, which must be taken into account - # while calculating the batch, sub_batch and interval values. - # interval - The time interval between every background migration - # - # example: - # Assume that we have figured out that updating 200 records of the events - # table takes ~100ms on average. - # We can set the sub_batch_size to 200, leave the interval to the default - # and set the batch_size to 50_000 which will require - # ~50s = (50000 / 200) * (0.1 + 0.1) to complete and leaves breathing space - # between the scheduled jobs - def initialize_conversion_of_integer_to_bigint( - table, - column, - primary_key: :id, - batch_size: 20_000, - sub_batch_size: 1000, - interval: 2.minutes - ) - - if transaction_open? - raise 'initialize_conversion_of_integer_to_bigint can not be run inside a transaction' - end - + def initialize_conversion_of_integer_to_bigint(table, column, primary_key: :id) unless table_exists?(table) raise "Table #{table} does not exist" end @@ -1003,28 +971,85 @@ module Gitlab install_rename_triggers(table, column, tmp_column) end + end - source_model = Class.new(ActiveRecord::Base) do - include EachBatch + # Backfills the new column used in the conversion of an integer column to bigint using background migrations. + # + # - This helper should be called from a post-deployment migration. + # - In order for this helper to work properly, the new column must be first initialized with + # the `initialize_conversion_of_integer_to_bigint` helper. + # - It tracks the scheduled background jobs through Gitlab::Database::BackgroundMigration::BatchedMigration, + # which allows a more thorough check that all jobs succeeded in the + # cleanup migration and is way faster for very large tables. + # + # Note: this helper is intended to be used in a post-deployment migration, to ensure any new code is + # deployed (including background job changes) before we begin processing the background migration. + # + # This helper is part 2 of a multi-step migration process: + # 1. initialize_conversion_of_integer_to_bigint to create the new column and database triggers + # 2. backfill_conversion_of_integer_to_bigint to copy historic data using background migrations + # 3. remaining steps TBD, see #288005 + # + # table - The name of the database table containing the column + # column - The name of the column that we want to convert to bigint. + # primary_key - The name of the primary key column (most often :id) + # batch_size - The number of rows to schedule in a single background migration + # sub_batch_size - The smaller batches that will be used by each scheduled job + # to update the table. Useful to keep each update at ~100ms while executing + # more updates per interval (2.minutes) + # Note that each execution of a sub-batch adds a constant 100ms sleep + # time in between the updates, which must be taken into account + # while calculating the batch, sub_batch and interval values. + # interval - The time interval between every background migration + # + # example: + # Assume that we have figured out that updating 200 records of the events + # table takes ~100ms on average. + # We can set the sub_batch_size to 200, leave the interval to the default + # and set the batch_size to 50_000 which will require + # ~50s = (50000 / 200) * (0.1 + 0.1) to complete and leaves breathing space + # between the scheduled jobs + def backfill_conversion_of_integer_to_bigint( + table, + column, + primary_key: :id, + batch_size: 20_000, + sub_batch_size: 1000, + interval: 2.minutes + ) - self.table_name = table - self.inheritance_column = :_type_disabled + unless table_exists?(table) + raise "Table #{table} does not exist" end - queue_background_migration_jobs_by_range_at_intervals( - source_model, + unless column_exists?(table, primary_key) + raise "Column #{primary_key} does not exist on #{table}" + end + + unless column_exists?(table, column) + raise "Column #{column} does not exist on #{table}" + end + + tmp_column = "#{column}_convert_to_bigint" + + unless column_exists?(table, tmp_column) + raise 'The temporary column does not exist, initialize it with `initialize_conversion_of_integer_to_bigint`' + end + + batched_migration = queue_batched_background_migration( 'CopyColumnUsingBackgroundMigrationJob', - interval, + table, + primary_key, + column, + tmp_column, + job_interval: interval, batch_size: batch_size, - other_job_arguments: [table, primary_key, sub_batch_size, column, tmp_column], - track_jobs: true, - primary_column_name: primary_key - ) + sub_batch_size: sub_batch_size) if perform_background_migration_inline? # To ensure the schema is up to date immediately we perform the # migration inline in dev / test environments. - Gitlab::BackgroundMigration.steal('CopyColumnUsingBackgroundMigrationJob') + Gitlab::Database::BackgroundMigration::BatchedMigrationRunner.new.run_entire_migration(batched_migration) end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 7d51559caaf..ef5e374c265 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -8722,11 +8722,6 @@ msgstr "" msgid "Coverage Fuzzing" msgstr "" -msgid "Coverage value for this pipeline was calculated by the coverage value of %d job." -msgid_plural "Coverage value for this pipeline was calculated by averaging the resulting coverage values of %d jobs." -msgstr[0] "" -msgstr[1] "" - msgid "Create" msgstr "" @@ -22668,9 +22663,6 @@ msgstr "" msgid "Pipeline|Could not retrieve the pipeline status. For troubleshooting steps, read the %{linkStart}documentation%{linkEnd}." msgstr "" -msgid "Pipeline|Coverage" -msgstr "" - msgid "Pipeline|Created" msgstr "" @@ -22767,6 +22759,18 @@ msgstr "" msgid "Pipeline|Tag name" msgstr "" +msgid "Pipeline|Test coverage" +msgstr "" + +msgid "Pipeline|This change will decrease the overall test coverage if merged." +msgstr "" + +msgid "Pipeline|This change will increase the overall test coverage if merged." +msgstr "" + +msgid "Pipeline|This change will not change the overall test coverage if merged." +msgstr "" + msgid "Pipeline|Trigger author" msgstr "" @@ -29834,6 +29838,11 @@ msgstr "" msgid "Test coverage parsing" msgstr "" +msgid "Test coverage value for this pipeline was calculated by the coverage value of %d job." +msgid_plural "Test coverage value for this pipeline was calculated by averaging the resulting coverage values of %d jobs." +msgstr[0] "" +msgstr[1] "" + msgid "Test coverage: %d hit" msgid_plural "Test coverage: %d hits" msgstr[0] "" @@ -34910,9 +34919,6 @@ msgstr "" msgid "You need permission." msgstr "" -msgid "You need to be logged in." -msgstr "" - msgid "You need to register a two-factor authentication app before you can set up a device." msgstr "" diff --git a/spec/frontend/vue_mr_widget/components/mr_widget_pipeline_spec.js b/spec/frontend/vue_mr_widget/components/mr_widget_pipeline_spec.js index 28492018600..918b0973ff5 100644 --- a/spec/frontend/vue_mr_widget/components/mr_widget_pipeline_spec.js +++ b/spec/frontend/vue_mr_widget/components/mr_widget_pipeline_spec.js @@ -1,6 +1,7 @@ import { GlLoadingIcon } from '@gitlab/ui'; import { shallowMount, mount } from '@vue/test-utils'; import { trimText } from 'helpers/text_helper'; +import { extendedWrapper } from 'helpers/vue_test_utils_helper'; import PipelineMiniGraph from '~/pipelines/components/pipelines_list/pipeline_mini_graph.vue'; import PipelineStage from '~/pipelines/components/pipelines_list/pipeline_stage.vue'; import PipelineComponent from '~/vue_merge_request_widget/components/mr_widget_pipeline.vue'; @@ -22,27 +23,30 @@ describe('MRWidgetPipeline', () => { 'Could not retrieve the pipeline status. For troubleshooting steps, read the documentation.'; const monitoringMessage = 'Checking pipeline status.'; - const findCIErrorMessage = () => wrapper.find('[data-testid="ci-error-message"]'); - const findPipelineID = () => wrapper.find('[data-testid="pipeline-id"]'); - const findPipelineInfoContainer = () => wrapper.find('[data-testid="pipeline-info-container"]'); - const findCommitLink = () => wrapper.find('[data-testid="commit-link"]'); - const findPipelineMiniGraph = () => wrapper.find(PipelineMiniGraph); - const findAllPipelineStages = () => wrapper.findAll(PipelineStage); - const findPipelineCoverage = () => wrapper.find('[data-testid="pipeline-coverage"]'); - const findPipelineCoverageDelta = () => wrapper.find('[data-testid="pipeline-coverage-delta"]'); + const findCIErrorMessage = () => wrapper.findByTestId('ci-error-message'); + const findPipelineID = () => wrapper.findByTestId('pipeline-id'); + const findPipelineInfoContainer = () => wrapper.findByTestId('pipeline-info-container'); + const findCommitLink = () => wrapper.findByTestId('commit-link'); + const findPipelineMiniGraph = () => wrapper.findComponent(PipelineMiniGraph); + const findAllPipelineStages = () => wrapper.findAllComponents(PipelineStage); + const findPipelineCoverage = () => wrapper.findByTestId('pipeline-coverage'); + const findPipelineCoverageDelta = () => wrapper.findByTestId('pipeline-coverage-delta'); const findPipelineCoverageTooltipText = () => - wrapper.find('[data-testid="pipeline-coverage-tooltip"]').text(); - const findMonitoringPipelineMessage = () => - wrapper.find('[data-testid="monitoring-pipeline-message"]'); - const findLoadingIcon = () => wrapper.find(GlLoadingIcon); + wrapper.findByTestId('pipeline-coverage-tooltip').text(); + const findPipelineCoverageDeltaTooltipText = () => + wrapper.findByTestId('pipeline-coverage-delta-tooltip').text(); + const findMonitoringPipelineMessage = () => wrapper.findByTestId('monitoring-pipeline-message'); + const findLoadingIcon = () => wrapper.findComponent(GlLoadingIcon); const createWrapper = (props = {}, mountFn = shallowMount) => { - wrapper = mountFn(PipelineComponent, { - propsData: { - ...defaultProps, - ...props, - }, - }); + wrapper = extendedWrapper( + mountFn(PipelineComponent, { + propsData: { + ...defaultProps, + ...props, + }, + }), + ); }; afterEach(() => { @@ -94,7 +98,9 @@ describe('MRWidgetPipeline', () => { describe('should render pipeline coverage information', () => { it('should render coverage percentage', () => { - expect(findPipelineCoverage().text()).toMatch(`Coverage ${mockData.pipeline.coverage}%`); + expect(findPipelineCoverage().text()).toMatch( + `Test coverage ${mockData.pipeline.coverage}%`, + ); }); it('should render coverage delta', () => { @@ -102,24 +108,9 @@ describe('MRWidgetPipeline', () => { expect(findPipelineCoverageDelta().text()).toBe(`(${mockData.pipelineCoverageDelta}%)`); }); - it('coverage delta should have no special style if there is no coverage change', () => { - createWrapper({ pipelineCoverageDelta: '0' }); - expect(findPipelineCoverageDelta().classes()).toEqual([]); - }); - - it('coverage delta should have text-success style if coverage increased', () => { - createWrapper({ pipelineCoverageDelta: '10' }); - expect(findPipelineCoverageDelta().classes()).toEqual(['text-success']); - }); - - it('coverage delta should have text-danger style if coverage increased', () => { - createWrapper({ pipelineCoverageDelta: '-10' }); - expect(findPipelineCoverageDelta().classes()).toEqual(['text-danger']); - }); - it('should render tooltip for jobs contributing to code coverage', () => { const tooltipText = findPipelineCoverageTooltipText(); - const expectedDescription = `Coverage value for this pipeline was calculated by averaging the resulting coverage values of ${mockData.buildsWithCoverage.length} jobs.`; + const expectedDescription = `Test coverage value for this pipeline was calculated by averaging the resulting coverage values of ${mockData.buildsWithCoverage.length} jobs.`; expect(tooltipText).toContain(expectedDescription); }); @@ -132,6 +123,26 @@ describe('MRWidgetPipeline', () => { expect(tooltipText).toContain(`${build.name} (${build.coverage}%)`); }, ); + + describe.each` + style | coverageState | coverageChangeText | styleClass | pipelineCoverageDelta + ${'no special'} | ${'the same'} | ${'not change'} | ${''} | ${'0'} + ${'success'} | ${'increased'} | ${'increase'} | ${'text-success'} | ${'10'} + ${'danger'} | ${'decreased'} | ${'decrease'} | ${'text-danger'} | ${'-10'} + `( + 'if test coverage is $coverageState', + ({ style, styleClass, coverageChangeText, pipelineCoverageDelta }) => { + it(`coverage delta should have ${style}`, () => { + createWrapper({ pipelineCoverageDelta }); + expect(findPipelineCoverageDelta().classes()).toEqual(styleClass ? [styleClass] : []); + }); + + it(`coverage delta tooltip should say that the coverage will ${coverageChangeText}`, () => { + createWrapper({ pipelineCoverageDelta }); + expect(findPipelineCoverageDeltaTooltipText()).toContain(coverageChangeText); + }); + }, + ); }); }); @@ -163,7 +174,7 @@ describe('MRWidgetPipeline', () => { }); it('should render coverage information', () => { - expect(findPipelineCoverage().text()).toMatch(`Coverage ${mockData.pipeline.coverage}%`); + expect(findPipelineCoverage().text()).toMatch(`Test coverage ${mockData.pipeline.coverage}%`); }); }); diff --git a/spec/lib/gitlab/database/background_migration/batched_migration_runner_spec.rb b/spec/lib/gitlab/database/background_migration/batched_migration_runner_spec.rb new file mode 100644 index 00000000000..b5210d31a49 --- /dev/null +++ b/spec/lib/gitlab/database/background_migration/batched_migration_runner_spec.rb @@ -0,0 +1,185 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::BackgroundMigration::BatchedMigrationRunner do + let(:migration_wrapper) { double('test wrapper') } + let(:runner) { described_class.new(migration_wrapper) } + + describe '#run_migration_job' do + shared_examples_for 'it has completed the migration' do + it 'does not create and run a migration job' do + expect(migration_wrapper).not_to receive(:perform) + + expect do + runner.run_migration_job(migration) + end.not_to change { Gitlab::Database::BackgroundMigration::BatchedJob.count } + end + + it 'marks the migration as finished' do + relation = Gitlab::Database::BackgroundMigration::BatchedMigration.finished.where(id: migration.id) + + expect { runner.run_migration_job(migration) }.to change { relation.count }.by(1) + end + end + + context 'when the migration has no previous jobs' do + let(:migration) { create(:batched_background_migration, :active, batch_size: 2) } + + let(:job_relation) do + Gitlab::Database::BackgroundMigration::BatchedJob.where(batched_background_migration_id: migration.id) + end + + context 'when the migration has batches to process' do + let!(:event1) { create(:event) } + let!(:event2) { create(:event) } + let!(:event3) { create(:event) } + + it 'runs the job for the first batch' do + migration.update!(min_value: event1.id, max_value: event2.id) + + expect(migration_wrapper).to receive(:perform) do |job_record| + expect(job_record).to eq(job_relation.first) + end + + expect { runner.run_migration_job(migration) }.to change { job_relation.count }.by(1) + + expect(job_relation.first).to have_attributes( + min_value: event1.id, + max_value: event2.id, + batch_size: migration.batch_size, + sub_batch_size: migration.sub_batch_size) + end + end + + context 'when the batch maximum exceeds the migration maximum' do + let!(:events) { create_list(:event, 3) } + let(:event1) { events[0] } + let(:event2) { events[1] } + + it 'clamps the batch maximum to the migration maximum' do + migration.update!(min_value: event1.id, max_value: event2.id, batch_size: 5) + + expect(migration_wrapper).to receive(:perform) + + expect { runner.run_migration_job(migration) }.to change { job_relation.count }.by(1) + + expect(job_relation.first).to have_attributes( + min_value: event1.id, + max_value: event2.id, + batch_size: migration.batch_size, + sub_batch_size: migration.sub_batch_size) + end + end + + context 'when the migration has no batches to process' do + it_behaves_like 'it has completed the migration' + end + end + + context 'when the migration has previous jobs' do + let!(:event1) { create(:event) } + let!(:event2) { create(:event) } + let!(:event3) { create(:event) } + + let!(:migration) do + create(:batched_background_migration, :active, batch_size: 2, min_value: event1.id, max_value: event3.id) + end + + let!(:previous_job) do + create(:batched_background_migration_job, + batched_migration: migration, + min_value: event1.id, + max_value: event2.id, + batch_size: 2, + sub_batch_size: 1) + end + + let(:job_relation) do + Gitlab::Database::BackgroundMigration::BatchedJob.where(batched_background_migration_id: migration.id) + end + + context 'when the migration has batches to process' do + it 'runs the migration job for the next batch' do + expect(migration_wrapper).to receive(:perform) do |job_record| + expect(job_record).to eq(job_relation.last) + end + + expect { runner.run_migration_job(migration) }.to change { job_relation.count }.by(1) + + expect(job_relation.last).to have_attributes( + min_value: event3.id, + max_value: event3.id, + batch_size: migration.batch_size, + sub_batch_size: migration.sub_batch_size) + end + + context 'when the batch minimum exceeds the migration maximum' do + before do + migration.update!(batch_size: 5, max_value: event2.id) + end + + it_behaves_like 'it has completed the migration' + end + end + + context 'when the migration has no batches remaining' do + before do + create(:batched_background_migration_job, + batched_migration: migration, + min_value: event3.id, + max_value: event3.id, + batch_size: 2, + sub_batch_size: 1) + end + + it_behaves_like 'it has completed the migration' + end + end + end + + describe '#run_entire_migration' do + context 'when the given migration is not active' do + it 'does not create and run migration jobs' do + migration = build(:batched_background_migration, :finished) + + expect(migration_wrapper).not_to receive(:perform) + + expect do + runner.run_entire_migration(migration) + end.not_to change { Gitlab::Database::BackgroundMigration::BatchedJob.count } + end + end + + context 'when the given migration is active' do + let!(:event1) { create(:event) } + let!(:event2) { create(:event) } + let!(:event3) { create(:event) } + + let!(:migration) do + create(:batched_background_migration, :active, batch_size: 2, min_value: event1.id, max_value: event3.id) + end + + let(:job_relation) do + Gitlab::Database::BackgroundMigration::BatchedJob.where(batched_background_migration_id: migration.id) + end + + it 'runs all jobs inline until finishing the migration' do + expect(migration_wrapper).to receive(:perform) do |job_record| + expect(job_record).to eq(job_relation.first) + end + + expect(migration_wrapper).to receive(:perform) do |job_record| + expect(job_record).to eq(job_relation.last) + end + + expect { runner.run_entire_migration(migration) }.to change { job_relation.count }.by(2) + + expect(job_relation.first).to have_attributes(min_value: event1.id, max_value: event2.id) + expect(job_relation.last).to have_attributes(min_value: event3.id, max_value: event3.id) + + expect(migration.reload).to be_finished + end + end + end +end diff --git a/spec/lib/gitlab/database/background_migration/scheduler_spec.rb b/spec/lib/gitlab/database/background_migration/scheduler_spec.rb deleted file mode 100644 index ba745acdf8a..00000000000 --- a/spec/lib/gitlab/database/background_migration/scheduler_spec.rb +++ /dev/null @@ -1,182 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::Database::BackgroundMigration::Scheduler, '#perform' do - let(:scheduler) { described_class.new } - - shared_examples_for 'it has no jobs to run' do - it 'does not create and run a migration job' do - test_wrapper = double('test wrapper') - - expect(test_wrapper).not_to receive(:perform) - - expect do - scheduler.perform(migration_wrapper: test_wrapper) - end.not_to change { Gitlab::Database::BackgroundMigration::BatchedJob.count } - end - end - - context 'when there are no active migrations' do - let!(:migration) { create(:batched_background_migration, :finished) } - - it_behaves_like 'it has no jobs to run' - end - - shared_examples_for 'it has completed the migration' do - it 'marks the migration as finished' do - relation = Gitlab::Database::BackgroundMigration::BatchedMigration.finished.where(id: first_migration.id) - - expect { scheduler.perform }.to change { relation.count }.by(1) - end - end - - context 'when there are active migrations' do - let!(:first_migration) { create(:batched_background_migration, :active, batch_size: 2) } - let!(:last_migration) { create(:batched_background_migration, :active) } - - let(:job_relation) do - Gitlab::Database::BackgroundMigration::BatchedJob.where(batched_background_migration_id: first_migration.id) - end - - context 'when the migration interval has not elapsed' do - before do - expect_next_found_instance_of(Gitlab::Database::BackgroundMigration::BatchedMigration) do |migration| - expect(migration).to receive(:interval_elapsed?).and_return(false) - end - end - - it_behaves_like 'it has no jobs to run' - end - - context 'when the interval has elapsed' do - before do - expect_next_found_instance_of(Gitlab::Database::BackgroundMigration::BatchedMigration) do |migration| - expect(migration).to receive(:interval_elapsed?).and_return(true) - end - end - - context 'when the first migration has no previous jobs' do - context 'when the migration has batches to process' do - let!(:event1) { create(:event) } - let!(:event2) { create(:event) } - let!(:event3) { create(:event) } - - it 'runs the job for the first batch' do - first_migration.update!(min_value: event1.id, max_value: event3.id) - - expect_next_instance_of(Gitlab::Database::BackgroundMigration::BatchedMigrationWrapper) do |wrapper| - expect(wrapper).to receive(:perform).and_wrap_original do |_, job_record| - expect(job_record).to eq(job_relation.first) - end - end - - expect { scheduler.perform }.to change { job_relation.count }.by(1) - - expect(job_relation.first).to have_attributes( - min_value: event1.id, - max_value: event2.id, - batch_size: first_migration.batch_size, - sub_batch_size: first_migration.sub_batch_size) - end - end - - context 'when the migration has no batches to process' do - it_behaves_like 'it has no jobs to run' - it_behaves_like 'it has completed the migration' - end - end - - context 'when the first migration has previous jobs' do - let!(:event1) { create(:event) } - let!(:event2) { create(:event) } - let!(:event3) { create(:event) } - - let!(:previous_job) do - create(:batched_background_migration_job, - batched_migration: first_migration, - min_value: event1.id, - max_value: event2.id, - batch_size: 2, - sub_batch_size: 1) - end - - context 'when the migration is ready to process another job' do - it 'runs the migration job for the next batch' do - first_migration.update!(min_value: event1.id, max_value: event3.id) - - expect_next_instance_of(Gitlab::Database::BackgroundMigration::BatchedMigrationWrapper) do |wrapper| - expect(wrapper).to receive(:perform).and_wrap_original do |_, job_record| - expect(job_record).to eq(job_relation.last) - end - end - - expect { scheduler.perform }.to change { job_relation.count }.by(1) - - expect(job_relation.last).to have_attributes( - min_value: event3.id, - max_value: event3.id, - batch_size: first_migration.batch_size, - sub_batch_size: first_migration.sub_batch_size) - end - end - - context 'when the migration has no batches remaining' do - let!(:final_job) do - create(:batched_background_migration_job, - batched_migration: first_migration, - min_value: event3.id, - max_value: event3.id, - batch_size: 2, - sub_batch_size: 1) - end - - it_behaves_like 'it has no jobs to run' - it_behaves_like 'it has completed the migration' - end - end - - context 'when the bounds of the next batch exceed the migration maximum value' do - let!(:events) { create_list(:event, 3) } - let(:event1) { events[0] } - let(:event2) { events[1] } - - context 'when the batch maximum exceeds the migration maximum' do - it 'clamps the batch maximum to the migration maximum' do - first_migration.update!(batch_size: 5, min_value: event1.id, max_value: event2.id) - - expect_next_instance_of(Gitlab::Database::BackgroundMigration::BatchedMigrationWrapper) do |wrapper| - expect(wrapper).to receive(:perform) - end - - expect { scheduler.perform }.to change { job_relation.count }.by(1) - - expect(job_relation.first).to have_attributes( - min_value: event1.id, - max_value: event2.id, - batch_size: first_migration.batch_size, - sub_batch_size: first_migration.sub_batch_size) - end - end - - context 'when the batch minimum exceeds the migration maximum' do - let!(:previous_job) do - create(:batched_background_migration_job, - batched_migration: first_migration, - min_value: event1.id, - max_value: event2.id, - batch_size: 5, - sub_batch_size: 1) - end - - before do - first_migration.update!(batch_size: 5, min_value: 1, max_value: event2.id) - end - - it_behaves_like 'it has no jobs to run' - it_behaves_like 'it has completed the migration' - end - end - end - end -end diff --git a/spec/lib/gitlab/database/migration_helpers_spec.rb b/spec/lib/gitlab/database/migration_helpers_spec.rb index 9178707a3d0..43dc94e8c1b 100644 --- a/spec/lib/gitlab/database/migration_helpers_spec.rb +++ b/spec/lib/gitlab/database/migration_helpers_spec.rb @@ -1702,65 +1702,171 @@ RSpec.describe Gitlab::Database::MigrationHelpers do end describe '#initialize_conversion_of_integer_to_bigint' do - let(:user) { create(:user) } - let(:project) { create(:project, :repository) } - let(:issue) { create(:issue, project: project) } - let!(:event) do - create(:event, :created, project: project, target: issue, author: user) - end + let(:table) { :test_table } + let(:column) { :id } + let(:tmp_column) { "#{column}_convert_to_bigint" } - context 'in a transaction' do - it 'raises RuntimeError' do - allow(model).to receive(:transaction_open?).and_return(true) - - expect { model.initialize_conversion_of_integer_to_bigint(:events, :id) } - .to raise_error(RuntimeError) + before do + model.create_table table, id: false do |t| + t.integer :id, primary_key: true + t.integer :non_nullable_column, null: false + t.integer :nullable_column + t.timestamps end end - context 'outside a transaction' do + context 'when the target table does not exist' do + it 'raises an error' do + expect { model.initialize_conversion_of_integer_to_bigint(:this_table_is_not_real, column) } + .to raise_error('Table this_table_is_not_real does not exist') + end + end + + context 'when the primary key does not exist' do + it 'raises an error' do + expect { model.initialize_conversion_of_integer_to_bigint(table, column, primary_key: :foobar) } + .to raise_error("Column foobar does not exist on #{table}") + end + end + + context 'when the column to convert does not exist' do + let(:column) { :foobar } + + it 'raises an error' do + expect { model.initialize_conversion_of_integer_to_bigint(table, column) } + .to raise_error("Column #{column} does not exist on #{table}") + end + end + + context 'when the column to convert is the primary key' do + it 'creates a not-null bigint column and installs triggers' do + expect(model).to receive(:add_column).with(table, tmp_column, :bigint, default: 0, null: false) + + expect(model).to receive(:install_rename_triggers).with(table, column, tmp_column) + + model.initialize_conversion_of_integer_to_bigint(table, column) + end + end + + context 'when the column to convert is not the primary key, but non-nullable' do + let(:column) { :non_nullable_column } + + it 'creates a not-null bigint column and installs triggers' do + expect(model).to receive(:add_column).with(table, tmp_column, :bigint, default: 0, null: false) + + expect(model).to receive(:install_rename_triggers).with(table, column, tmp_column) + + model.initialize_conversion_of_integer_to_bigint(table, column) + end + end + + context 'when the column to convert is not the primary key, but nullable' do + let(:column) { :nullable_column } + + it 'creates a nullable bigint column and installs triggers' do + expect(model).to receive(:add_column).with(table, tmp_column, :bigint, default: nil) + + expect(model).to receive(:install_rename_triggers).with(table, column, tmp_column) + + model.initialize_conversion_of_integer_to_bigint(table, column) + end + end + end + + describe '#backfill_conversion_of_integer_to_bigint' do + let(:table) { :_test_backfill_table } + let(:column) { :id } + let(:tmp_column) { "#{column}_convert_to_bigint" } + + before do + model.create_table table, id: false do |t| + t.integer :id, primary_key: true + t.text :message, null: false + t.timestamps + end + + allow(model).to receive(:perform_background_migration_inline?).and_return(false) + end + + context 'when the target table does not exist' do + it 'raises an error' do + expect { model.backfill_conversion_of_integer_to_bigint(:this_table_is_not_real, column) } + .to raise_error('Table this_table_is_not_real does not exist') + end + end + + context 'when the primary key does not exist' do + it 'raises an error' do + expect { model.backfill_conversion_of_integer_to_bigint(table, column, primary_key: :foobar) } + .to raise_error("Column foobar does not exist on #{table}") + end + end + + context 'when the column to convert does not exist' do + let(:column) { :foobar } + + it 'raises an error' do + expect { model.backfill_conversion_of_integer_to_bigint(table, column) } + .to raise_error("Column #{column} does not exist on #{table}") + end + end + + context 'when the temporary column does not exist' do + it 'raises an error' do + expect { model.backfill_conversion_of_integer_to_bigint(table, column) } + .to raise_error('The temporary column does not exist, initialize it with `initialize_conversion_of_integer_to_bigint`') + end + end + + context 'when the conversion is properly initialized' do + let(:model_class) do + Class.new(ActiveRecord::Base) do + self.table_name = :_test_backfill_table + end + end + + let(:migration_relation) { Gitlab::Database::BackgroundMigration::BatchedMigration.active } + before do - allow(model).to receive(:transaction_open?).and_return(false) + model.initialize_conversion_of_integer_to_bigint(table, column) + + model_class.create!(message: 'hello') + model_class.create!(message: 'so long') end - it 'creates a bigint column and starts backfilling it' do - expect(model) - .to receive(:add_column) - .with( - :events, - 'id_convert_to_bigint', - :bigint, - default: 0, - null: false - ) + it 'creates the batched migration tracking record' do + last_record = model_class.create!(message: 'goodbye') - expect(model) - .to receive(:install_rename_triggers) - .with(:events, :id, 'id_convert_to_bigint') + expect do + model.backfill_conversion_of_integer_to_bigint(table, column, batch_size: 2, sub_batch_size: 1) + end.to change { migration_relation.count }.by(1) - expect(model).to receive(:queue_background_migration_jobs_by_range_at_intervals).and_call_original - - expect(BackgroundMigrationWorker) - .to receive(:perform_in) - .ordered - .with( - 2.minutes, - 'CopyColumnUsingBackgroundMigrationJob', - [event.id, event.id, :events, :id, 100, :id, 'id_convert_to_bigint'] - ) - - expect(Gitlab::BackgroundMigration) - .to receive(:steal) - .ordered - .with('CopyColumnUsingBackgroundMigrationJob') - - model.initialize_conversion_of_integer_to_bigint( - :events, - :id, - batch_size: 300, - sub_batch_size: 100 + expect(migration_relation.last).to have_attributes( + job_class_name: 'CopyColumnUsingBackgroundMigrationJob', + table_name: table.to_s, + column_name: column.to_s, + min_value: 1, + max_value: last_record.id, + interval: 120, + batch_size: 2, + sub_batch_size: 1, + job_arguments: [column.to_s, "#{column}_convert_to_bigint"] ) end + + context 'when the migration should be performed inline' do + it 'calls the runner to run the entire migration' do + expect(model).to receive(:perform_background_migration_inline?).and_return(true) + + expect_next_instance_of(Gitlab::Database::BackgroundMigration::BatchedMigrationRunner) do |scheduler| + expect(scheduler).to receive(:run_entire_migration) do |batched_migration| + expect(batched_migration).to eq(migration_relation.last) + end + end + + model.backfill_conversion_of_integer_to_bigint(table, column, batch_size: 2, sub_batch_size: 1) + end + end end end diff --git a/spec/services/ci/pipeline_trigger_service_spec.rb b/spec/services/ci/pipeline_trigger_service_spec.rb index 89d3da89011..36055779a2e 100644 --- a/spec/services/ci/pipeline_trigger_service_spec.rb +++ b/spec/services/ci/pipeline_trigger_service_spec.rb @@ -55,17 +55,6 @@ RSpec.describe Ci::PipelineTriggerService do expect(var.variable_type).to eq('file') end - context 'when FF ci_trigger_payload_into_pipeline is disabled' do - before do - stub_feature_flags(ci_trigger_payload_into_pipeline: false) - end - - it 'does not store the payload as a variable' do - expect { result }.not_to change { Ci::PipelineVariable.count } - expect(result[:pipeline].variables).to be_empty - end - end - context 'when commit message has [ci skip]' do before do allow_next(Ci::Pipeline).to receive(:git_commit_message) { '[ci skip]' } diff --git a/spec/services/namespaces/in_product_marketing_emails_service_spec.rb b/spec/services/namespaces/in_product_marketing_emails_service_spec.rb index 71a34983dab..97696c7df43 100644 --- a/spec/services/namespaces/in_product_marketing_emails_service_spec.rb +++ b/spec/services/namespaces/in_product_marketing_emails_service_spec.rb @@ -3,14 +3,12 @@ require 'spec_helper' RSpec.describe Namespaces::InProductMarketingEmailsService, '#execute' do - subject(:execute_service) do - travel_to(frozen_time) { described_class.new(track, interval).execute } - end + subject(:execute_service) { described_class.new(track, interval).execute } let(:track) { :create } let(:interval) { 1 } - let(:frozen_time) { Time.current } + let(:frozen_time) { Time.zone.parse('23 Mar 2021 10:14:40 UTC') } let(:previous_action_completed_at) { frozen_time - 2.days } let(:current_action_completed_at) { nil } let(:experiment_enabled) { true } @@ -21,6 +19,7 @@ RSpec.describe Namespaces::InProductMarketingEmailsService, '#execute' do let_it_be(:user) { create(:user, email_opted_in: true) } before do + travel_to(frozen_time) create(:onboarding_progress, namespace: group, **actions_completed) group.add_developer(user) stub_experiment_for_subject(in_product_marketing_emails: experiment_enabled) diff --git a/spec/support/shared_contexts/requests/api/conan_packages_shared_context.rb b/spec/support/shared_contexts/requests/api/conan_packages_shared_context.rb index f3bbb325475..ac53be1a1cb 100644 --- a/spec/support/shared_contexts/requests/api/conan_packages_shared_context.rb +++ b/spec/support/shared_contexts/requests/api/conan_packages_shared_context.rb @@ -41,13 +41,6 @@ RSpec.shared_context 'conan recipe endpoints' do let(:jwt) { build_jwt(personal_access_token) } let(:headers) { build_token_auth_header(jwt.encoded) } let(:conan_package_reference) { '123456789' } - let(:presenter) { double('::Packages::Conan::PackagePresenter') } - - before do - allow(::Packages::Conan::PackagePresenter).to receive(:new) - .with(package, user, package.project, any_args) - .and_return(presenter) - end end RSpec.shared_context 'conan file download endpoints' do diff --git a/spec/support/shared_examples/finders/assignees_filter_shared_examples.rb b/spec/support/shared_examples/finders/assignees_filter_shared_examples.rb index 96b05db4cd9..7588e1d971d 100644 --- a/spec/support/shared_examples/finders/assignees_filter_shared_examples.rb +++ b/spec/support/shared_examples/finders/assignees_filter_shared_examples.rb @@ -1,6 +1,16 @@ # frozen_string_literal: true RSpec.shared_examples 'assignee ID filter' do + context 'when optimize_issue_filter_assigned_to_self is disabled' do + before do + stub_feature_flags(optimize_issue_filter_assigned_to_self: false) + end + + it 'returns issuables assigned to that user' do + expect(issuables).to contain_exactly(*expected_issuables) + end + end + it 'returns issuables assigned to that user' do expect(issuables).to contain_exactly(*expected_issuables) end @@ -13,6 +23,16 @@ RSpec.shared_examples 'assignee NOT ID filter' do end RSpec.shared_examples 'assignee username filter' do + context 'when optimize_issue_filter_assigned_to_self is disabled' do + before do + stub_feature_flags(optimize_issue_filter_assigned_to_self: false) + end + + it 'returns issuables assigned to those users' do + expect(issuables).to contain_exactly(*expected_issuables) + end + end + it 'returns issuables assigned to those users' do expect(issuables).to contain_exactly(*expected_issuables) end diff --git a/spec/support/shared_examples/requests/api/conan_packages_shared_examples.rb b/spec/support/shared_examples/requests/api/conan_packages_shared_examples.rb index 54ea876bed2..87aaac673c1 100644 --- a/spec/support/shared_examples/requests/api/conan_packages_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/conan_packages_shared_examples.rb @@ -205,6 +205,14 @@ RSpec.shared_examples 'empty recipe for not found package' do 'aa/bb/%{project}/ccc' % { project: ::Packages::Conan::Metadatum.package_username_from(full_path: project.full_path) } end + let(:presenter) { double('::Packages::Conan::PackagePresenter') } + + before do + allow(::Packages::Conan::PackagePresenter).to receive(:new) + .with(package, user, package.project, any_args) + .and_return(presenter) + end + it 'returns not found' do allow(::Packages::Conan::PackagePresenter).to receive(:new) .with( @@ -248,8 +256,6 @@ RSpec.shared_examples 'recipe download_urls' do 'conanmanifest.txt' => "#{url_prefix}/packages/conan/v1/files/#{package.conan_recipe_path}/0/export/conanmanifest.txt" } - allow(presenter).to receive(:recipe_urls) { expected_response } - subject expect(json_response).to eq(expected_response) @@ -268,8 +274,6 @@ RSpec.shared_examples 'package download_urls' do 'conan_package.tgz' => "#{url_prefix}/packages/conan/v1/files/#{package.conan_recipe_path}/0/package/123456789/0/conan_package.tgz" } - allow(presenter).to receive(:package_urls) { expected_response } - subject expect(json_response).to eq(expected_response) @@ -309,12 +313,13 @@ RSpec.shared_examples 'recipe snapshot endpoint' do context 'with existing package' do it 'returns a hash of files with their md5 hashes' do - expected_response = { - 'conanfile.py' => 'md5hash1', - 'conanmanifest.txt' => 'md5hash2' - } + conan_file_file = package.package_files.find_by(file_name: 'conanfile.py') + conan_manifest_file = package.package_files.find_by(file_name: 'conanmanifest.txt') - allow(presenter).to receive(:recipe_snapshot) { expected_response } + expected_response = { + 'conanfile.py' => conan_file_file.file_md5, + 'conanmanifest.txt' => conan_manifest_file.file_md5 + } subject @@ -333,13 +338,11 @@ RSpec.shared_examples 'package snapshot endpoint' do context 'with existing package' do it 'returns a hash of md5 values for the files' do expected_response = { - 'conaninfo.txt' => "md5hash1", - 'conanmanifest.txt' => "md5hash2", - 'conan_package.tgz' => "md5hash3" + 'conaninfo.txt' => "12345abcde", + 'conanmanifest.txt' => "12345abcde", + 'conan_package.tgz' => "12345abcde" } - allow(presenter).to receive(:package_snapshot) { expected_response } - subject expect(json_response).to eq(expected_response) diff --git a/spec/tooling/danger/changelog_spec.rb b/spec/tooling/danger/changelog_spec.rb index b74039b3cd1..7ea2288fd45 100644 --- a/spec/tooling/danger/changelog_spec.rb +++ b/spec/tooling/danger/changelog_spec.rb @@ -161,23 +161,42 @@ RSpec.describe Tooling::Danger::Changelog do describe '#modified_text' do subject { changelog.modified_text } - context "when title is not changed from sanitization", :aggregate_failures do - let(:mr_title) { 'Fake Title' } + context 'when in CI context' do + shared_examples 'changelog modified text' do |key| + specify do + expect(subject).to include('CHANGELOG.md was edited') + expect(subject).to include('bin/changelog -m 1234 "Fake Title"') + expect(subject).to include('bin/changelog --ee -m 1234 "Fake Title"') + end + end - specify do - expect(subject).to include('CHANGELOG.md was edited') - expect(subject).to include('bin/changelog -m 1234 "Fake Title"') - expect(subject).to include('bin/changelog --ee -m 1234 "Fake Title"') + before do + allow(fake_helper).to receive(:ci?).and_return(true) + end + + context "when title is not changed from sanitization", :aggregate_failures do + let(:mr_title) { 'Fake Title' } + + it_behaves_like 'changelog modified text' + end + + context "when title needs sanitization", :aggregate_failures do + let(:mr_title) { 'DRAFT: Fake Title' } + + it_behaves_like 'changelog modified text' end end - context "when title needs sanitization", :aggregate_failures do - let(:mr_title) { 'DRAFT: Fake Title' } + context 'when in local context' do + let(:mr_title) { 'Fake Title' } + + before do + allow(fake_helper).to receive(:ci?).and_return(false) + end specify do expect(subject).to include('CHANGELOG.md was edited') - expect(subject).to include('bin/changelog -m 1234 "Fake Title"') - expect(subject).to include('bin/changelog --ee -m 1234 "Fake Title"') + expect(subject).not_to include('bin/changelog') end end end @@ -187,56 +206,116 @@ RSpec.describe Tooling::Danger::Changelog do subject { changelog.required_texts } - shared_examples 'changelog required text' do |key| - specify do - expect(subject).to have_key(key) - expect(subject[key]).to include('CHANGELOG missing') - expect(subject[key]).to include('bin/changelog -m 1234 "Fake Title"') - expect(subject[key]).not_to include('--ee') + context 'when in CI context' do + before do + allow(fake_helper).to receive(:ci?).and_return(true) + end + + shared_examples 'changelog required text' do |key| + specify do + expect(subject).to have_key(key) + expect(subject[key]).to include('CHANGELOG missing') + expect(subject[key]).to include('bin/changelog -m 1234 "Fake Title"') + expect(subject[key]).not_to include('--ee') + end + end + + context 'with a new migration file' do + let(:changes) { changes_class.new([change_class.new('foo', :added, :migration)]) } + + context "when title is not changed from sanitization", :aggregate_failures do + it_behaves_like 'changelog required text', :db_changes + end + + context "when title needs sanitization", :aggregate_failures do + let(:mr_title) { 'DRAFT: Fake Title' } + + it_behaves_like 'changelog required text', :db_changes + end + end + + context 'with a removed feature flag file' do + let(:changes) { changes_class.new([change_class.new('foo', :deleted, :feature_flag)]) } + + it_behaves_like 'changelog required text', :feature_flag_removed end end - context 'with a new migration file' do - let(:changes) { changes_class.new([change_class.new('foo', :added, :migration)]) } - - context "when title is not changed from sanitization", :aggregate_failures do - it_behaves_like 'changelog required text', :db_changes + context 'when in local context' do + before do + allow(fake_helper).to receive(:ci?).and_return(false) end - context "when title needs sanitization", :aggregate_failures do - let(:mr_title) { 'DRAFT: Fake Title' } - - it_behaves_like 'changelog required text', :db_changes + shared_examples 'changelog required text' do |key| + specify do + expect(subject).to have_key(key) + expect(subject[key]).to include('CHANGELOG missing') + expect(subject[key]).not_to include('bin/changelog') + expect(subject[key]).not_to include('--ee') + end end - end - context 'with a removed feature flag file' do - let(:changes) { changes_class.new([change_class.new('foo', :deleted, :feature_flag)]) } + context 'with a new migration file' do + let(:changes) { changes_class.new([change_class.new('foo', :added, :migration)]) } - it_behaves_like 'changelog required text', :feature_flag_removed + context "when title is not changed from sanitization", :aggregate_failures do + it_behaves_like 'changelog required text', :db_changes + end + + context "when title needs sanitization", :aggregate_failures do + let(:mr_title) { 'DRAFT: Fake Title' } + + it_behaves_like 'changelog required text', :db_changes + end + end + + context 'with a removed feature flag file' do + let(:changes) { changes_class.new([change_class.new('foo', :deleted, :feature_flag)]) } + + it_behaves_like 'changelog required text', :feature_flag_removed + end end end describe '#optional_text' do subject { changelog.optional_text } - context "when title is not changed from sanitization", :aggregate_failures do - let(:mr_title) { 'Fake Title' } + context 'when in CI context' do + shared_examples 'changelog optional text' do |key| + specify do + expect(subject).to include('CHANGELOG missing') + expect(subject).to include('bin/changelog -m 1234 "Fake Title"') + expect(subject).to include('bin/changelog --ee -m 1234 "Fake Title"') + end + end - specify do - expect(subject).to include('CHANGELOG missing') - expect(subject).to include('bin/changelog -m 1234 "Fake Title"') - expect(subject).to include('bin/changelog --ee -m 1234 "Fake Title"') + before do + allow(fake_helper).to receive(:ci?).and_return(true) + end + + context "when title is not changed from sanitization", :aggregate_failures do + let(:mr_title) { 'Fake Title' } + + it_behaves_like 'changelog optional text' + end + + context "when title needs sanitization", :aggregate_failures do + let(:mr_title) { 'DRAFT: Fake Title' } + + it_behaves_like 'changelog optional text' end end - context "when title needs sanitization", :aggregate_failures do - let(:mr_title) { 'DRAFT: Fake Title' } + context 'when in local context' do + let(:mr_title) { 'Fake Title' } + + before do + allow(fake_helper).to receive(:ci?).and_return(false) + end specify do expect(subject).to include('CHANGELOG missing') - expect(subject).to include('bin/changelog -m 1234 "Fake Title"') - expect(subject).to include('bin/changelog --ee -m 1234 "Fake Title"') + expect(subject).not_to include('bin/changelog') end end end diff --git a/spec/tooling/danger/project_helper_spec.rb b/spec/tooling/danger/project_helper_spec.rb index c61ca159224..b1bd9c2466a 100644 --- a/spec/tooling/danger/project_helper_spec.rb +++ b/spec/tooling/danger/project_helper_spec.rb @@ -203,7 +203,7 @@ RSpec.describe Tooling::Danger::ProjectHelper do describe '.local_warning_message' do it 'returns an informational message with rules that can run' do - expect(described_class.local_warning_message).to eq('==> Only the following Danger rules can be run locally: changes_size, commit_messages, database, documentation, duplicate_yarn_dependencies, eslint, karma, pajamas, pipeline, prettier, product_intelligence, utility_css') + expect(described_class.local_warning_message).to eq('==> Only the following Danger rules can be run locally: changelog, changes_size, commit_messages, database, documentation, duplicate_yarn_dependencies, eslint, karma, pajamas, pipeline, prettier, product_intelligence, utility_css') end end diff --git a/tooling/danger/changelog.rb b/tooling/danger/changelog.rb index 672d23d58e4..065c737050e 100644 --- a/tooling/danger/changelog.rb +++ b/tooling/danger/changelog.rb @@ -18,29 +18,35 @@ module Tooling CHANGELOG_MODIFIED_URL_TEXT = "**CHANGELOG.md was edited.** Please remove the additions and create a CHANGELOG entry.\n\n" CHANGELOG_MISSING_URL_TEXT = "**[CHANGELOG missing](https://docs.gitlab.com/ee/development/changelog.html)**:\n\n" - OPTIONAL_CHANGELOG_MESSAGE = <<~MSG - If you want to create a changelog entry for GitLab FOSS, run the following: + OPTIONAL_CHANGELOG_MESSAGE = { + local: "If this merge request [doesn't need a CHANGELOG entry](https://docs.gitlab.com/ee/development/changelog.html#what-warrants-a-changelog-entry), feel free to ignore this message.", + ci: <<~MSG + If you want to create a changelog entry for GitLab FOSS, run the following: - #{CREATE_CHANGELOG_COMMAND} + #{CREATE_CHANGELOG_COMMAND} - If you want to create a changelog entry for GitLab EE, run the following instead: + If you want to create a changelog entry for GitLab EE, run the following instead: - #{CREATE_EE_CHANGELOG_COMMAND} + #{CREATE_EE_CHANGELOG_COMMAND} - If this merge request [doesn't need a CHANGELOG entry](https://docs.gitlab.com/ee/development/changelog.html#what-warrants-a-changelog-entry), feel free to ignore this message. - MSG + If this merge request [doesn't need a CHANGELOG entry](https://docs.gitlab.com/ee/development/changelog.html#what-warrants-a-changelog-entry), feel free to ignore this message. + MSG + }.freeze REQUIRED_CHANGELOG_REASONS = { db_changes: 'introduces a database migration', feature_flag_removed: 'removes a feature flag' }.freeze - REQUIRED_CHANGELOG_MESSAGE = <<~MSG - To create a changelog entry, run the following: + REQUIRED_CHANGELOG_MESSAGE = { + local: "This merge request requires a changelog entry because it [%s](https://docs.gitlab.com/ee/development/changelog.html#what-warrants-a-changelog-entry).", + ci: <<~MSG + To create a changelog entry, run the following: - #{CREATE_CHANGELOG_COMMAND} + #{CREATE_CHANGELOG_COMMAND} - This merge request requires a changelog entry because it [%s](https://docs.gitlab.com/ee/development/changelog.html#what-warrants-a-changelog-entry). - MSG + This merge request requires a changelog entry because it [%s](https://docs.gitlab.com/ee/development/changelog.html#what-warrants-a-changelog-entry). + MSG + }.freeze def required_reasons [].tap do |reasons| @@ -67,20 +73,20 @@ module Tooling def modified_text CHANGELOG_MODIFIED_URL_TEXT + - format(OPTIONAL_CHANGELOG_MESSAGE, mr_iid: helper.mr_iid, mr_title: sanitized_mr_title) + (helper.ci? ? format(OPTIONAL_CHANGELOG_MESSAGE[:ci], mr_iid: helper.mr_iid, mr_title: sanitized_mr_title) : OPTIONAL_CHANGELOG_MESSAGE[:local]) end def required_texts required_reasons.each_with_object({}) do |required_reason, memo| memo[required_reason] = CHANGELOG_MISSING_URL_TEXT + - format(REQUIRED_CHANGELOG_MESSAGE, reason: REQUIRED_CHANGELOG_REASONS.fetch(required_reason), mr_iid: helper.mr_iid, mr_title: sanitized_mr_title) + (helper.ci? ? format(REQUIRED_CHANGELOG_MESSAGE[:ci], reason: REQUIRED_CHANGELOG_REASONS.fetch(required_reason), mr_iid: helper.mr_iid, mr_title: sanitized_mr_title) : REQUIRED_CHANGELOG_MESSAGE[:local]) end end def optional_text CHANGELOG_MISSING_URL_TEXT + - format(OPTIONAL_CHANGELOG_MESSAGE, mr_iid: helper.mr_iid, mr_title: sanitized_mr_title) + (helper.ci? ? format(OPTIONAL_CHANGELOG_MESSAGE[:ci], mr_iid: helper.mr_iid, mr_title: sanitized_mr_title) : OPTIONAL_CHANGELOG_MESSAGE[:local]) end private diff --git a/tooling/danger/project_helper.rb b/tooling/danger/project_helper.rb index 9291b725af2..2c4d40f50ee 100644 --- a/tooling/danger/project_helper.rb +++ b/tooling/danger/project_helper.rb @@ -4,6 +4,7 @@ module Tooling module Danger module ProjectHelper LOCAL_RULES ||= %w[ + changelog changes_size commit_messages database @@ -20,7 +21,6 @@ module Tooling CI_ONLY_RULES ||= %w[ ce_ee_vue_templates - changelog ci_templates metadata feature_flag diff --git a/vendor/assets/javascripts/jasmine-jquery.js b/vendor/assets/javascripts/jasmine-jquery.js index 9b7b245009d..a1f1e892c57 100644 --- a/vendor/assets/javascripts/jasmine-jquery.js +++ b/vendor/assets/javascripts/jasmine-jquery.js @@ -33,7 +33,10 @@ WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. (function (root, factory) { if (typeof module !== 'undefined' && module.exports) { - factory(root, root.jasmine, require('jquery')); + // The line below is patched from jquery => jquery/dist/jquery + // in order to load a jQuery with ajax, so that this testing library + // doesn't break + factory(root, root.jasmine, require('jquery/dist/jquery')); } else { factory(root, root.jasmine, root.jQuery); }