From d738ba980c5ce598811b700e215ab957132f3a67 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Wed, 19 Jan 2022 18:14:01 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- Gemfile | 4 +- Gemfile.lock | 8 +- .../content_editor/extensions/image.js | 11 + .../services/serialization_helpers.js | 9 +- .../environments/components/deployment.vue | 14 +- .../components/deployment_status_badge.vue | 60 +++ .../javascripts/integrations/constants.js | 2 + .../edit/components/integration_form.vue | 49 ++- .../javascripts/integrations/edit/index.js | 7 +- .../pages/admin/integrations/edit/index.js | 2 +- .../settings/integrations/edit/index.js | 2 +- .../pages/projects/services/edit/index.js | 2 +- app/assets/stylesheets/framework/emojis.scss | 4 + .../secondary_navigation_elements.scss | 10 +- .../stylesheets/startup/startup-dark.scss | 3 + .../stylesheets/themes/theme_helper.scss | 22 +- .../concerns/integrations/actions.rb | 3 + .../projects/services_controller.rb | 3 + ...ation_for_namespace_creation_experiment.rb | 27 ++ app/helpers/application_settings_helper.rb | 6 + app/helpers/integrations_helper.rb | 8 +- app/models/application_setting.rb | 12 +- .../application_setting_implementation.rb | 6 + app/models/ci/job_artifact.rb | 4 +- app/models/container_repository.rb | 6 + app/models/experiment.rb | 24 +- app/models/user.rb | 1 + .../upsert_credit_card_validation_service.rb | 5 +- app/views/projects/services/_form.html.haml | 9 +- ...l.haml => _integration_settings.html.haml} | 2 +- app/views/shared/integrations/_form.html.haml | 4 +- app/views/shared/integrations/edit.html.haml | 5 +- ...ansaction.yml => vue_integration_form.yml} | 12 +- ...vulnerability_finding_replace_metadata.yml | 3 +- ...re_verification_for_namespace_creation.yml | 8 + .../ops/ci_unsafe_regexp_logger.yml | 8 + ...registry_migration_application_settings.rb | 15 + ...ation_columns_to_container_repositories.rb | 19 + ...d_requires_verification_to_user_details.rb | 9 + ...ontainer_repositories_migration_columns.rb | 15 + ...o_container_registry_import_target_plan.rb | 13 + ...ast_site_profiles_builds_ci_build_id_fk.rb | 18 + ...cts_ci_pipeline_artifacts_project_id_fk.rb | 17 + db/schema_migrations/20220106230629 | 1 + db/schema_migrations/20220106230712 | 1 + db/schema_migrations/20220110233155 | 1 + db/schema_migrations/20220112115413 | 1 + db/schema_migrations/20220117225936 | 1 + db/schema_migrations/20220118141950 | 1 + db/schema_migrations/20220119141736 | 1 + db/structure.sql | 28 +- .../redis/replication_and_failover.md | 6 + doc/development/redis/new_redis_instance.md | 125 ++++++ doc/topics/git/troubleshooting_git.md | 7 + lib/api/users.rb | 2 +- lib/gitlab/ci/build/policy/refs.rb | 5 +- .../database/gitlab_loose_foreign_keys.yml | 4 + lib/gitlab/sidekiq_logging/json_formatter.rb | 1 + .../sidekiq_logging/structured_logger.rb | 2 + lib/gitlab/sidekiq_middleware/monitor.rb | 2 + lib/gitlab/untrusted_regexp/ruby_syntax.rb | 16 +- locale/gitlab.pot | 27 ++ ..._for_namespace_creation_experiment_spec.rb | 59 +++ ...activates_mattermost_slash_command_spec.rb | 15 + .../integrations/user_activates_jira_spec.rb | 2 +- .../content_editor/extensions/image_spec.js | 41 ++ .../services/markdown_serializer_spec.js | 4 + spec/frontend/environments/deployment_spec.js | 29 ++ .../deployment_status_badge_spec.js | 42 ++ .../edit/components/integration_form_spec.js | 391 ++++++++++-------- spec/helpers/integrations_helper_spec.rb | 14 +- .../database/no_cross_db_foreign_keys_spec.rb | 1 - spec/models/application_setting_spec.rb | 12 + spec/models/ci/job_artifact_spec.rb | 16 +- spec/models/ci/pipeline_artifact_spec.rb | 7 + spec/models/container_repository_spec.rb | 8 + spec/models/experiment_spec.rb | 48 +++ spec/models/user_spec.rb | 3 + ...ert_credit_card_validation_service_spec.rb | 8 +- .../projects/services/_form.haml_spec.rb | 30 +- .../cleanup_package_file_worker_spec.rb | 5 +- 81 files changed, 1148 insertions(+), 290 deletions(-) create mode 100644 app/assets/javascripts/environments/components/deployment_status_badge.vue create mode 100644 app/experiments/require_verification_for_namespace_creation_experiment.rb rename app/views/shared/{_service_settings.html.haml => _integration_settings.html.haml} (92%) rename config/feature_flags/development/{ci_store_trace_outside_transaction.yml => vue_integration_form.yml} (52%) create mode 100644 config/feature_flags/experiment/require_verification_for_namespace_creation.yml create mode 100644 config/feature_flags/ops/ci_unsafe_regexp_logger.yml create mode 100644 db/migrate/20220106230629_add_registry_migration_application_settings.rb create mode 100644 db/migrate/20220106230712_add_migration_columns_to_container_repositories.rb create mode 100644 db/migrate/20220112115413_add_requires_verification_to_user_details.rb create mode 100644 db/migrate/20220117225936_add_text_limits_to_container_repositories_migration_columns.rb create mode 100644 db/migrate/20220118141950_add_text_limit_to_container_registry_import_target_plan.rb create mode 100644 db/post_migrate/20220110233155_remove_dast_site_profiles_builds_ci_build_id_fk.rb create mode 100644 db/post_migrate/20220119141736_remove_projects_ci_pipeline_artifacts_project_id_fk.rb create mode 100644 db/schema_migrations/20220106230629 create mode 100644 db/schema_migrations/20220106230712 create mode 100644 db/schema_migrations/20220110233155 create mode 100644 db/schema_migrations/20220112115413 create mode 100644 db/schema_migrations/20220117225936 create mode 100644 db/schema_migrations/20220118141950 create mode 100644 db/schema_migrations/20220119141736 create mode 100644 spec/experiments/require_verification_for_namespace_creation_experiment_spec.rb create mode 100644 spec/frontend/content_editor/extensions/image_spec.js create mode 100644 spec/frontend/environments/deployment_spec.js create mode 100644 spec/frontend/environments/deployment_status_badge_spec.js diff --git a/Gemfile b/Gemfile index 473194f4e62..334e7df7681 100644 --- a/Gemfile +++ b/Gemfile @@ -196,7 +196,7 @@ gem 'acts-as-taggable-on', '~> 9.0' # Background jobs gem 'sidekiq', '~> 6.3' -gem 'sidekiq-cron', '~> 1.0' +gem 'sidekiq-cron', '~> 1.2' gem 'redis-namespace', '~> 1.8.1' gem 'gitlab-sidekiq-fetcher', '0.8.0', require: 'sidekiq-reliable-fetch' @@ -392,8 +392,6 @@ group :development, :test do gem 'parallel', '~> 1.19', require: false - gem 'rblineprof', '~> 0.3.6', platform: :mri, require: false - gem 'test_file_finder', '~> 0.1.3' end diff --git a/Gemfile.lock b/Gemfile.lock index e6b3673c6e9..797a72ce943 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -239,7 +239,6 @@ GEM danger gitlab (~> 4.2, >= 4.2.0) database_cleaner (1.7.0) - debugger-ruby_core_source (1.3.8) deckar01-task_list (2.3.1) html-pipeline declarative (0.0.20) @@ -1009,8 +1008,6 @@ GEM rb-fsevent (0.10.4) rb-inotify (0.10.1) ffi (~> 1.0) - rblineprof (0.3.6) - debugger-ruby_core_source (~> 1.3) rbtrace (0.4.14) ffi (>= 1.0.6) msgpack (>= 0.4.3) @@ -1183,7 +1180,7 @@ GEM connection_pool (>= 2.2.2) rack (~> 2.0) redis (>= 4.2.0) - sidekiq-cron (1.0.4) + sidekiq-cron (1.2.0) fugit (~> 1.1) sidekiq (>= 4.2.1) signet (0.14.0) @@ -1594,7 +1591,6 @@ DEPENDENCIES rails-controller-testing rails-i18n (~> 6.0) rainbow (~> 3.0) - rblineprof (~> 0.3.6) rbtrace (~> 0.4) rdoc (~> 6.3.2) re2 (~> 1.2.0) @@ -1630,7 +1626,7 @@ DEPENDENCIES settingslogic (~> 2.0.9) shoulda-matchers (~> 4.0.1) sidekiq (~> 6.3) - sidekiq-cron (~> 1.0) + sidekiq-cron (~> 1.2) simple_po_parser (~> 1.1.2) simplecov (~> 0.18.5) simplecov-cobertura (~> 1.3.1) diff --git a/app/assets/javascripts/content_editor/extensions/image.js b/app/assets/javascripts/content_editor/extensions/image.js index d7fb617f7ee..519f7f168ce 100644 --- a/app/assets/javascripts/content_editor/extensions/image.js +++ b/app/assets/javascripts/content_editor/extensions/image.js @@ -66,6 +66,17 @@ export default Image.extend({ }, ]; }, + renderHTML({ HTMLAttributes }) { + return [ + 'img', + { + src: HTMLAttributes.src, + alt: HTMLAttributes.alt, + title: HTMLAttributes.title, + 'data-canonical-src': HTMLAttributes.canonicalSrc, + }, + ]; + }, addNodeView() { return VueNodeViewRenderer(ImageWrapper); }, diff --git a/app/assets/javascripts/content_editor/services/serialization_helpers.js b/app/assets/javascripts/content_editor/services/serialization_helpers.js index ed5910fca18..4d5a54c0347 100644 --- a/app/assets/javascripts/content_editor/services/serialization_helpers.js +++ b/app/assets/javascripts/content_editor/services/serialization_helpers.js @@ -1,4 +1,4 @@ -import { uniq } from 'lodash'; +import { uniq, isString } from 'lodash'; const defaultAttrs = { td: { colspan: 1, rowspan: 1, colwidth: null }, @@ -325,9 +325,12 @@ export function renderHardBreak(state, node, parent, index) { export function renderImage(state, node) { const { alt, canonicalSrc, src, title } = node.attrs; - const quotedTitle = title ? ` ${state.quote(title)}` : ''; - state.write(`![${state.esc(alt || '')}](${state.esc(canonicalSrc || src)}${quotedTitle})`); + if (isString(src) || isString(canonicalSrc)) { + const quotedTitle = title ? ` ${state.quote(title)}` : ''; + + state.write(`![${state.esc(alt || '')}](${state.esc(canonicalSrc || src)}${quotedTitle})`); + } } export function renderPlayable(state, node) { diff --git a/app/assets/javascripts/environments/components/deployment.vue b/app/assets/javascripts/environments/components/deployment.vue index 292f9a366c3..ef43ca6bc33 100644 --- a/app/assets/javascripts/environments/components/deployment.vue +++ b/app/assets/javascripts/environments/components/deployment.vue @@ -1,13 +1,25 @@ diff --git a/app/assets/javascripts/environments/components/deployment_status_badge.vue b/app/assets/javascripts/environments/components/deployment_status_badge.vue new file mode 100644 index 00000000000..5a026911766 --- /dev/null +++ b/app/assets/javascripts/environments/components/deployment_status_badge.vue @@ -0,0 +1,60 @@ + + diff --git a/app/assets/javascripts/integrations/constants.js b/app/assets/javascripts/integrations/constants.js index 7d32fafdf92..b90658fb13c 100644 --- a/app/assets/javascripts/integrations/constants.js +++ b/app/assets/javascripts/integrations/constants.js @@ -26,3 +26,5 @@ export const I18N_SUCCESSFUL_CONNECTION_MESSAGE = s__('Integrations|Connection s export const settingsTabTitle = __('Settings'); export const overridesTabTitle = s__('Integrations|Projects using custom settings'); + +export const INTEGRATION_FORM_SELECTOR = '.js-integration-settings-form'; diff --git a/app/assets/javascripts/integrations/edit/components/integration_form.vue b/app/assets/javascripts/integrations/edit/components/integration_form.vue index ca38c83547b..c3cc35adfa5 100644 --- a/app/assets/javascripts/integrations/edit/components/integration_form.vue +++ b/app/assets/javascripts/integrations/edit/components/integration_form.vue @@ -1,5 +1,5 @@ diff --git a/app/assets/javascripts/integrations/edit/index.js b/app/assets/javascripts/integrations/edit/index.js index 9c9e3edbeb8..fbda8c1e3d0 100644 --- a/app/assets/javascripts/integrations/edit/index.js +++ b/app/assets/javascripts/integrations/edit/index.js @@ -28,9 +28,11 @@ function parseDatasetToProps(data) { cancelPath, testPath, resetPath, + formPath, vulnerabilitiesIssuetype, jiraIssueTransitionAutomatic, jiraIssueTransitionId, + redirectTo, ...booleanAttributes } = data; const { @@ -57,6 +59,7 @@ function parseDatasetToProps(data) { canTest, testPath, resetPath, + formPath, triggerFieldsProps: { initialTriggerCommit: commitEvents, initialTriggerMergeRequest: mergeRequestEvents, @@ -82,10 +85,11 @@ function parseDatasetToProps(data) { inheritFromId: parseInt(inheritFromId, 10), integrationLevel, id: parseInt(id, 10), + redirectTo, }; } -export default function initIntegrationSettingsForm(formSelector) { +export default function initIntegrationSettingsForm() { const customSettingsEl = document.querySelector('.js-vue-integration-settings'); const defaultSettingsEl = document.querySelector('.js-vue-default-integration-settings'); @@ -115,7 +119,6 @@ export default function initIntegrationSettingsForm(formSelector) { return createElement(IntegrationForm, { props: { helpHtml, - formSelector, }, }); }, diff --git a/app/assets/javascripts/pages/admin/integrations/edit/index.js b/app/assets/javascripts/pages/admin/integrations/edit/index.js index 8485b460261..c354ed1c142 100644 --- a/app/assets/javascripts/pages/admin/integrations/edit/index.js +++ b/app/assets/javascripts/pages/admin/integrations/edit/index.js @@ -1,7 +1,7 @@ import initIntegrationSettingsForm from '~/integrations/edit'; import PrometheusMetrics from '~/prometheus_metrics/prometheus_metrics'; -initIntegrationSettingsForm('.js-integration-settings-form'); +initIntegrationSettingsForm(); const prometheusSettingsSelector = '.js-prometheus-metrics-monitoring'; const prometheusSettingsWrapper = document.querySelector(prometheusSettingsSelector); diff --git a/app/assets/javascripts/pages/groups/settings/integrations/edit/index.js b/app/assets/javascripts/pages/groups/settings/integrations/edit/index.js index 8485b460261..c354ed1c142 100644 --- a/app/assets/javascripts/pages/groups/settings/integrations/edit/index.js +++ b/app/assets/javascripts/pages/groups/settings/integrations/edit/index.js @@ -1,7 +1,7 @@ import initIntegrationSettingsForm from '~/integrations/edit'; import PrometheusMetrics from '~/prometheus_metrics/prometheus_metrics'; -initIntegrationSettingsForm('.js-integration-settings-form'); +initIntegrationSettingsForm(); const prometheusSettingsSelector = '.js-prometheus-metrics-monitoring'; const prometheusSettingsWrapper = document.querySelector(prometheusSettingsSelector); diff --git a/app/assets/javascripts/pages/projects/services/edit/index.js b/app/assets/javascripts/pages/projects/services/edit/index.js index a2b18d86240..2048d3dfc37 100644 --- a/app/assets/javascripts/pages/projects/services/edit/index.js +++ b/app/assets/javascripts/pages/projects/services/edit/index.js @@ -2,7 +2,7 @@ import initIntegrationSettingsForm from '~/integrations/edit'; import PrometheusAlerts from '~/prometheus_alerts'; import CustomMetrics from '~/prometheus_metrics/custom_metrics'; -initIntegrationSettingsForm('.js-integration-settings-form'); +initIntegrationSettingsForm(); const prometheusSettingsSelector = '.js-prometheus-metrics-monitoring'; const prometheusSettingsWrapper = document.querySelector(prometheusSettingsSelector); diff --git a/app/assets/stylesheets/framework/emojis.scss b/app/assets/stylesheets/framework/emojis.scss index 1ddde3d2ed6..a31910e3090 100644 --- a/app/assets/stylesheets/framework/emojis.scss +++ b/app/assets/stylesheets/framework/emojis.scss @@ -43,6 +43,10 @@ gl-emoji { border-bottom-color: transparent; } +.emoji-picker-category-active { + border-bottom-color: var(--gl-theme-accent, $theme-indigo-500); +} + .emoji-picker .gl-new-dropdown-inner > :last-child { padding-bottom: 0; } diff --git a/app/assets/stylesheets/framework/secondary_navigation_elements.scss b/app/assets/stylesheets/framework/secondary_navigation_elements.scss index 685f1f413e6..563075b911c 100644 --- a/app/assets/stylesheets/framework/secondary_navigation_elements.scss +++ b/app/assets/stylesheets/framework/secondary_navigation_elements.scss @@ -40,9 +40,11 @@ a.active { color: $black; font-weight: $gl-font-weight-bold; + border-bottom: 2px solid var(--gl-theme-accent, $theme-indigo-500); .badge.badge-pill { color: $black; + font-weight: $gl-font-weight-bold; } } @@ -126,14 +128,6 @@ input { display: inline-block; position: relative; - - &:not[type='checkbox'] { - /* Medium devices (desktops, 992px and up) */ - @include media-breakpoint-up(md) { width: 200px; } - - /* Large devices (large desktops, 1200px and up) */ - @include media-breakpoint-up(lg) { width: 250px; } - } } @include media-breakpoint-up(md) { diff --git a/app/assets/stylesheets/startup/startup-dark.scss b/app/assets/stylesheets/startup/startup-dark.scss index 96dee4a3da1..c72de0e6f29 100644 --- a/app/assets/stylesheets/startup/startup-dark.scss +++ b/app/assets/stylesheets/startup/startup-dark.scss @@ -1795,6 +1795,9 @@ body.gl-dark { .nav-sidebar li.active:not(.fly-out-top-item) > a:not(.has-sub-items) { background-color: var(--indigo-900-alpha-008); } +body.gl-dark { + --gl-theme-accent: #868686; +} body.gl-dark .navbar-gitlab { background-color: #fafafa; } diff --git a/app/assets/stylesheets/themes/theme_helper.scss b/app/assets/stylesheets/themes/theme_helper.scss index dead5287170..ec0928fc3d4 100644 --- a/app/assets/stylesheets/themes/theme_helper.scss +++ b/app/assets/stylesheets/themes/theme_helper.scss @@ -4,12 +4,16 @@ */ @mixin gitlab-theme( $search-and-nav-links, - $active-tab-border, + $accent, $border-and-box-shadow, $sidebar-text, $nav-svg-color, $color-alternate ) { + // Set custom properties + + --gl-theme-accent: #{$accent}; + // Header .navbar-gitlab { @@ -219,22 +223,6 @@ } } - .nav-links li { - &.active a, - &.md-header-tab.active button, - a.active { - border-bottom: 2px solid $active-tab-border; - - .badge.badge-pill { - font-weight: $gl-font-weight-bold; - } - } - } - - .emoji-picker-category-active { - border-bottom-color: $active-tab-border; - } - .branch-header-title { color: $border-and-box-shadow; } diff --git a/app/controllers/concerns/integrations/actions.rb b/app/controllers/concerns/integrations/actions.rb index 1f788860c8f..f6e98c25b72 100644 --- a/app/controllers/concerns/integrations/actions.rb +++ b/app/controllers/concerns/integrations/actions.rb @@ -8,6 +8,9 @@ module Integrations::Actions include IntegrationsHelper before_action :integration, only: [:edit, :update, :overrides, :test] + before_action do + push_frontend_feature_flag(:vue_integration_form, current_user, default_enabled: :yaml) + end urgency :low, [:test] end diff --git a/app/controllers/projects/services_controller.rb b/app/controllers/projects/services_controller.rb index 1321111faaf..9896f75c099 100644 --- a/app/controllers/projects/services_controller.rb +++ b/app/controllers/projects/services_controller.rb @@ -12,6 +12,9 @@ class Projects::ServicesController < Projects::ApplicationController before_action :web_hook_logs, only: [:edit, :update] before_action :set_deprecation_notice_for_prometheus_integration, only: [:edit, :update] before_action :redirect_deprecated_prometheus_integration, only: [:update] + before_action do + push_frontend_feature_flag(:vue_integration_form, current_user, default_enabled: :yaml) + end respond_to :html diff --git a/app/experiments/require_verification_for_namespace_creation_experiment.rb b/app/experiments/require_verification_for_namespace_creation_experiment.rb new file mode 100644 index 00000000000..1cadac7e7d4 --- /dev/null +++ b/app/experiments/require_verification_for_namespace_creation_experiment.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +class RequireVerificationForNamespaceCreationExperiment < ApplicationExperiment # rubocop:disable Gitlab/NamespacedClass + def control_behavior + false + end + + def candidate_behavior + true + end + + def candidate? + run + end + + def record_conversion(namespace) + return unless should_track? + + Experiment.by_name(name).record_conversion_event_for_subject(subject, namespace_id: namespace.id) + end + + private + + def subject + context.value[:user] + end +end diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb index 90861e440fb..7541247b19f 100644 --- a/app/helpers/application_settings_helper.rb +++ b/app/helpers/application_settings_helper.rb @@ -404,6 +404,12 @@ module ApplicationSettingsHelper :rate_limiting_response_text, :container_registry_expiration_policies_worker_capacity, :container_registry_cleanup_tags_service_max_list_size, + :container_registry_import_max_tags_count, + :container_registry_import_max_retries, + :container_registry_import_start_max_retries, + :container_registry_import_max_step_duration, + :container_registry_import_target_plan, + :container_registry_import_created_before, :keep_latest_artifact, :whats_new_variant, :user_deactivation_emails_enabled, diff --git a/app/helpers/integrations_helper.rb b/app/helpers/integrations_helper.rb index 2b62d746e71..230f80e20a5 100644 --- a/app/helpers/integrations_helper.rb +++ b/app/helpers/integrations_helper.rb @@ -90,7 +90,9 @@ module IntegrationsHelper cancel_path: scoped_integrations_path(project: project, group: group), can_test: integration.testable?.to_s, test_path: scoped_test_integration_path(integration, project: project, group: group), - reset_path: scoped_reset_integration_path(integration, group: group) + reset_path: scoped_reset_integration_path(integration, group: group), + form_path: scoped_integration_path(integration, project: project, group: group), + redirect_to: request.referer } if integration.is_a?(Integrations::Jira) @@ -226,6 +228,10 @@ module IntegrationsHelper name: integration.to_param } end + + def vue_integration_form_enabled? + Feature.enabled?(:vue_integration_form, current_user, default_enabled: :yaml) + end end IntegrationsHelper.prepend_mod_with('IntegrationsHelper') diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index 9d822243563..b69c0199c70 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -357,13 +357,19 @@ class ApplicationSetting < ApplicationRecord validates :hashed_storage_enabled, inclusion: { in: [true], message: _("Hashed storage can't be disabled anymore for new projects") } validates :container_registry_delete_tags_service_timeout, + :container_registry_cleanup_tags_service_max_list_size, + :container_registry_expiration_policies_worker_capacity, numericality: { only_integer: true, greater_than_or_equal_to: 0 } - validates :container_registry_cleanup_tags_service_max_list_size, + validates :container_registry_import_max_tags_count, + :container_registry_import_max_retries, + :container_registry_import_start_max_retries, + :container_registry_import_max_step_duration, + allow_nil: false, numericality: { only_integer: true, greater_than_or_equal_to: 0 } - validates :container_registry_expiration_policies_worker_capacity, - numericality: { only_integer: true, greater_than_or_equal_to: 0 } + validates :container_registry_import_target_plan, presence: true + validates :container_registry_import_created_before, presence: true validates :dependency_proxy_ttl_group_policy_worker_capacity, allow_nil: false, diff --git a/app/models/application_setting_implementation.rb b/app/models/application_setting_implementation.rb index d548b88204f..25198178f69 100644 --- a/app/models/application_setting_implementation.rb +++ b/app/models/application_setting_implementation.rb @@ -217,6 +217,12 @@ module ApplicationSettingImplementation wiki_page_max_content_bytes: 50.megabytes, container_registry_delete_tags_service_timeout: 250, container_registry_expiration_policies_worker_capacity: 0, + container_registry_import_max_tags_count: 100, + container_registry_import_max_retries: 3, + container_registry_import_start_max_retries: 50, + container_registry_import_max_step_duration: 5.minutes, + container_registry_import_target_plan: 'free', + container_registry_import_created_before: '2022-01-23 00:00:00', kroki_enabled: false, kroki_url: nil, kroki_formats: { blockdiag: false, bpmn: false, excalidraw: false }, diff --git a/app/models/ci/job_artifact.rb b/app/models/ci/job_artifact.rb index 7d2168def10..3426c4d5248 100644 --- a/app/models/ci/job_artifact.rb +++ b/app/models/ci/job_artifact.rb @@ -348,9 +348,7 @@ module Ci def store_after_commit? strong_memoize(:store_after_commit) do - trace? && - JobArtifactUploader.direct_upload_enabled? && - Feature.enabled?(:ci_store_trace_outside_transaction, project, default_enabled: :yaml) + trace? && JobArtifactUploader.direct_upload_enabled? end end diff --git a/app/models/container_repository.rb b/app/models/container_repository.rb index c914819f79d..b03d946fc47 100644 --- a/app/models/container_repository.rb +++ b/app/models/container_repository.rb @@ -13,9 +13,15 @@ class ContainerRepository < ApplicationRecord validates :name, length: { minimum: 0, allow_nil: false } validates :name, uniqueness: { scope: :project_id } + validates :migration_state, presence: true + + validates :migration_retries_count, presence: true, + numericality: { greater_than_or_equal_to: 0 }, + allow_nil: false enum status: { delete_scheduled: 0, delete_failed: 1 } enum expiration_policy_cleanup_status: { cleanup_unscheduled: 0, cleanup_scheduled: 1, cleanup_unfinished: 2, cleanup_ongoing: 3 } + enum migration_skipped_reason: { not_in_plan: 0, too_many_retries: 1, too_many_tags: 2, root_namespace_in_deny_list: 3 } delegate :client, to: :registry diff --git a/app/models/experiment.rb b/app/models/experiment.rb index cd0814c476a..2300ec2996d 100644 --- a/app/models/experiment.rb +++ b/app/models/experiment.rb @@ -7,7 +7,7 @@ class Experiment < ApplicationRecord validates :name, presence: true, uniqueness: true, length: { maximum: 255 } def self.add_user(name, group_type, user, context = {}) - find_or_create_by!(name: name).record_user_and_group(user, group_type, context) + by_name(name).record_user_and_group(user, group_type, context) end def self.add_group(name, variant:, group:) @@ -15,11 +15,15 @@ class Experiment < ApplicationRecord end def self.add_subject(name, variant:, subject:) - find_or_create_by!(name: name).record_subject_and_variant!(subject, variant) + by_name(name).record_subject_and_variant!(subject, variant) end def self.record_conversion_event(name, user, context = {}) - find_or_create_by!(name: name).record_conversion_event_for_user(user, context) + by_name(name).record_conversion_event_for_user(user, context) + end + + def self.by_name(name) + find_or_create_by!(name: name) end # Create or update the recorded experiment_user row for the user in this experiment. @@ -41,6 +45,16 @@ class Experiment < ApplicationRecord experiment_user.update!(converted_at: Time.current, context: merged_context(experiment_user, context)) end + def record_conversion_event_for_subject(subject, context = {}) + raise 'Incompatible subject provided!' unless ExperimentSubject.valid_subject?(subject) + + attr_name = subject.class.table_name.singularize.to_sym + experiment_subject = experiment_subjects.find_by(attr_name => subject) + return unless experiment_subject + + experiment_subject.update!(converted_at: Time.current, context: merged_context(experiment_subject, context)) + end + def record_subject_and_variant!(subject, variant) raise 'Incompatible subject provided!' unless ExperimentSubject.valid_subject?(subject) @@ -57,7 +71,7 @@ class Experiment < ApplicationRecord private - def merged_context(experiment_user, new_context) - experiment_user.context.deep_merge(new_context.deep_stringify_keys) + def merged_context(experiment_subject, new_context) + experiment_subject.context.deep_merge(new_context.deep_stringify_keys) end end diff --git a/app/models/user.rb b/app/models/user.rb index 406eb2d6204..a587723053f 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -331,6 +331,7 @@ class User < ApplicationRecord delegate :pronouns, :pronouns=, to: :user_detail, allow_nil: true delegate :pronunciation, :pronunciation=, to: :user_detail, allow_nil: true delegate :registration_objective, :registration_objective=, to: :user_detail, allow_nil: true + delegate :requires_credit_card_verification, :requires_credit_card_verification=, to: :user_detail, allow_nil: true accepts_nested_attributes_for :user_preference, update_only: true accepts_nested_attributes_for :user_detail, update_only: true diff --git a/app/services/users/upsert_credit_card_validation_service.rb b/app/services/users/upsert_credit_card_validation_service.rb index 61cf598f178..7190c82bea3 100644 --- a/app/services/users/upsert_credit_card_validation_service.rb +++ b/app/services/users/upsert_credit_card_validation_service.rb @@ -2,8 +2,9 @@ module Users class UpsertCreditCardValidationService < BaseService - def initialize(params) + def initialize(params, user) @params = params.to_h.with_indifferent_access + @current_user = user end def execute @@ -18,6 +19,8 @@ module Users ::Users::CreditCardValidation.upsert(@params) + ::Users::UpdateService.new(current_user, user: current_user, requires_credit_card_verification: false).execute! + ServiceResponse.success(message: 'CreditCardValidation was set') rescue ActiveRecord::InvalidForeignKey, ActiveRecord::NotNullViolation => e ServiceResponse.error(message: "Could not set CreditCardValidation: #{e.message}") diff --git a/app/views/projects/services/_form.html.haml b/app/views/projects/services/_form.html.haml index 84abbe74581..419dd827e49 100644 --- a/app/views/projects/services/_form.html.haml +++ b/app/views/projects/services/_form.html.haml @@ -6,9 +6,12 @@ - if integration.operating? = sprite_icon('check', css_class: 'gl-text-green-500') -= form_for(integration, as: :service, url: scoped_integration_path(integration, project: @project, group: @group), method: :put, html: { class: 'gl-show-field-errors integration-settings-form js-integration-settings-form', data: { 'test-url' => test_project_integration_path(@project, integration) } }) do |form| - = render 'shared/service_settings', form: form, integration: integration - %input{ id: 'services_redirect_to', type: 'hidden', name: 'redirect_to', value: request.referer } +- if vue_integration_form_enabled? + = render 'shared/integration_settings', integration: integration +- else + = form_for(integration, as: :service, url: scoped_integration_path(integration, project: @project, group: @group), method: :put, html: { class: 'gl-show-field-errors integration-settings-form js-integration-settings-form', data: { 'test-url' => test_project_integration_path(@project, integration), testid: 'integration-form' } }) do |form| + = render 'shared/integration_settings', form: form, integration: integration + %input{ id: 'services_redirect_to', type: 'hidden', name: 'redirect_to', value: request.referer } - if lookup_context.template_exists?('show', "projects/services/#{integration.to_param}", true) %hr diff --git a/app/views/shared/_service_settings.html.haml b/app/views/shared/_integration_settings.html.haml similarity index 92% rename from app/views/shared/_service_settings.html.haml rename to app/views/shared/_integration_settings.html.haml index adacaeadfab..93606ca0aba 100644 --- a/app/views/shared/_service_settings.html.haml +++ b/app/views/shared/_integration_settings.html.haml @@ -1,6 +1,6 @@ = form_errors(integration) -.service-settings +%div{ data: { testid: "integration-settings-form" } } - if @default_integration .js-vue-default-integration-settings{ data: integration_form_data(@default_integration, group: @group, project: @project) } .js-vue-integration-settings{ data: integration_form_data(integration, group: @group, project: @project) } diff --git a/app/views/shared/integrations/_form.html.haml b/app/views/shared/integrations/_form.html.haml index 89c127408e1..e2457bc0632 100644 --- a/app/views/shared/integrations/_form.html.haml +++ b/app/views/shared/integrations/_form.html.haml @@ -1,4 +1,4 @@ - integration = local_assigns.fetch(:integration) -= form_for integration, as: :service, url: scoped_integration_path(integration, group: @group), method: :put, html: { class: 'gl-show-field-errors integration-settings-form js-integration-settings-form', data: { 'test-url' => scoped_test_integration_path(integration, group: @group) } } do |form| - = render 'shared/service_settings', form: form, integration: integration += form_for integration, as: :service, url: scoped_integration_path(integration, group: @group), method: :put, html: { class: 'gl-show-field-errors integration-settings-form js-integration-settings-form', data: { 'test-url' => scoped_test_integration_path(integration, group: @group), testid: 'integration-form' } } do |form| + = render 'shared/integration_settings', form: form, integration: integration diff --git a/app/views/shared/integrations/edit.html.haml b/app/views/shared/integrations/edit.html.haml index acb0c7ee52e..4ceaedc2a69 100644 --- a/app/views/shared/integrations/edit.html.haml +++ b/app/views/shared/integrations/edit.html.haml @@ -7,4 +7,7 @@ = @integration.title = render 'shared/integrations/tabs', integration: @integration, active_tab: 'edit' do - = render 'shared/integrations/form', integration: @integration + - if vue_integration_form_enabled? + = render 'shared/integration_settings', integration: @integration + - else + = render 'shared/integrations/form', integration: @integration diff --git a/config/feature_flags/development/ci_store_trace_outside_transaction.yml b/config/feature_flags/development/vue_integration_form.yml similarity index 52% rename from config/feature_flags/development/ci_store_trace_outside_transaction.yml rename to config/feature_flags/development/vue_integration_form.yml index 1be425c6bbf..a11c42b8d4a 100644 --- a/config/feature_flags/development/ci_store_trace_outside_transaction.yml +++ b/config/feature_flags/development/vue_integration_form.yml @@ -1,8 +1,8 @@ --- -name: ci_store_trace_outside_transaction -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/66203 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/336280 -milestone: '14.5' +name: vue_integration_form +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/77934 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/350444 +milestone: '14.7' type: development -group: group::pipeline execution -default_enabled: true +group: group::integrations +default_enabled: false diff --git a/config/feature_flags/development/vulnerability_finding_replace_metadata.yml b/config/feature_flags/development/vulnerability_finding_replace_metadata.yml index f7b3cb67c38..2774547668f 100644 --- a/config/feature_flags/development/vulnerability_finding_replace_metadata.yml +++ b/config/feature_flags/development/vulnerability_finding_replace_metadata.yml @@ -2,6 +2,7 @@ name: vulnerability_finding_replace_metadata introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/66868 rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/337253 +milestone: '14.2' group: group::threat insights type: development -default_enabled: false \ No newline at end of file +default_enabled: false diff --git a/config/feature_flags/experiment/require_verification_for_namespace_creation.yml b/config/feature_flags/experiment/require_verification_for_namespace_creation.yml new file mode 100644 index 00000000000..5772d3217b8 --- /dev/null +++ b/config/feature_flags/experiment/require_verification_for_namespace_creation.yml @@ -0,0 +1,8 @@ +--- +name: require_verification_for_namespace_creation +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/77315 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/350251 +milestone: '14.8' +type: experiment +group: group::activation +default_enabled: false diff --git a/config/feature_flags/ops/ci_unsafe_regexp_logger.yml b/config/feature_flags/ops/ci_unsafe_regexp_logger.yml new file mode 100644 index 00000000000..00dbab724f8 --- /dev/null +++ b/config/feature_flags/ops/ci_unsafe_regexp_logger.yml @@ -0,0 +1,8 @@ +--- +name: ci_unsafe_regexp_logger +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/78458 +rollout_issue_url: +milestone: '14.8' +type: ops +group: group::pipeline authoring +default_enabled: true diff --git a/db/migrate/20220106230629_add_registry_migration_application_settings.rb b/db/migrate/20220106230629_add_registry_migration_application_settings.rb new file mode 100644 index 00000000000..191443de6eb --- /dev/null +++ b/db/migrate/20220106230629_add_registry_migration_application_settings.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class AddRegistryMigrationApplicationSettings < Gitlab::Database::Migration[1.0] + # rubocop:disable Migration/AddLimitToTextColumns + # limit is added in 20220118141950_add_text_limit_to_container_registry_import_target_plan.rb + def change + add_column :application_settings, :container_registry_import_max_tags_count, :integer, default: 100, null: false + add_column :application_settings, :container_registry_import_max_retries, :integer, default: 3, null: false + add_column :application_settings, :container_registry_import_start_max_retries, :integer, default: 50, null: false + add_column :application_settings, :container_registry_import_max_step_duration, :integer, default: 5.minutes, null: false + add_column :application_settings, :container_registry_import_target_plan, :text, default: 'free', null: false + add_column :application_settings, :container_registry_import_created_before, :datetime_with_timezone, default: '2022-01-23 00:00:00', null: false + end + # rubocop:enable Migration/AddLimitToTextColumns +end diff --git a/db/migrate/20220106230712_add_migration_columns_to_container_repositories.rb b/db/migrate/20220106230712_add_migration_columns_to_container_repositories.rb new file mode 100644 index 00000000000..76dccbe785f --- /dev/null +++ b/db/migrate/20220106230712_add_migration_columns_to_container_repositories.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class AddMigrationColumnsToContainerRepositories < Gitlab::Database::Migration[1.0] + # rubocop:disable Migration/AddLimitToTextColumns + # limit is added in 20220117225936_add_text_limits_to_container_repositories_migration_columns.rb + def change + add_column :container_repositories, :migration_pre_import_started_at, :datetime_with_timezone + add_column :container_repositories, :migration_pre_import_done_at, :datetime_with_timezone + add_column :container_repositories, :migration_import_started_at, :datetime_with_timezone + add_column :container_repositories, :migration_import_done_at, :datetime_with_timezone + add_column :container_repositories, :migration_aborted_at, :datetime_with_timezone + add_column :container_repositories, :migration_skipped_at, :datetime_with_timezone + add_column :container_repositories, :migration_retries_count, :integer, default: 0, null: false + add_column :container_repositories, :migration_skipped_reason, :smallint + add_column :container_repositories, :migration_state, :text, default: 'default', null: false + add_column :container_repositories, :migration_aborted_in_state, :text + end + # rubocop:enable Migration/AddLimitToTextColumns +end diff --git a/db/migrate/20220112115413_add_requires_verification_to_user_details.rb b/db/migrate/20220112115413_add_requires_verification_to_user_details.rb new file mode 100644 index 00000000000..01fe4f1d5cf --- /dev/null +++ b/db/migrate/20220112115413_add_requires_verification_to_user_details.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddRequiresVerificationToUserDetails < Gitlab::Database::Migration[1.0] + enable_lock_retries! + + def change + add_column :user_details, :requires_credit_card_verification, :boolean, null: false, default: false + end +end diff --git a/db/migrate/20220117225936_add_text_limits_to_container_repositories_migration_columns.rb b/db/migrate/20220117225936_add_text_limits_to_container_repositories_migration_columns.rb new file mode 100644 index 00000000000..91c0612716b --- /dev/null +++ b/db/migrate/20220117225936_add_text_limits_to_container_repositories_migration_columns.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class AddTextLimitsToContainerRepositoriesMigrationColumns < Gitlab::Database::Migration[1.0] + disable_ddl_transaction! + + def up + add_text_limit :container_repositories, :migration_state, 255 + add_text_limit :container_repositories, :migration_aborted_in_state, 255 + end + + def down + remove_text_limit :container_repositories, :migration_state + remove_text_limit :container_repositories, :migration_aborted_in_state + end +end diff --git a/db/migrate/20220118141950_add_text_limit_to_container_registry_import_target_plan.rb b/db/migrate/20220118141950_add_text_limit_to_container_registry_import_target_plan.rb new file mode 100644 index 00000000000..c7247d03423 --- /dev/null +++ b/db/migrate/20220118141950_add_text_limit_to_container_registry_import_target_plan.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class AddTextLimitToContainerRegistryImportTargetPlan < Gitlab::Database::Migration[1.0] + disable_ddl_transaction! + + def up + add_text_limit :application_settings, :container_registry_import_target_plan, 255 + end + + def down + remove_text_limit :application_settings, :container_registry_import_target_plan + end +end diff --git a/db/post_migrate/20220110233155_remove_dast_site_profiles_builds_ci_build_id_fk.rb b/db/post_migrate/20220110233155_remove_dast_site_profiles_builds_ci_build_id_fk.rb new file mode 100644 index 00000000000..00d8a39216b --- /dev/null +++ b/db/post_migrate/20220110233155_remove_dast_site_profiles_builds_ci_build_id_fk.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class RemoveDastSiteProfilesBuildsCiBuildIdFk < Gitlab::Database::Migration[1.0] + disable_ddl_transaction! + + CONSTRAINT_NAME = 'fk_a325505e99' + + def up + with_lock_retries do + execute('LOCK ci_builds, dast_site_profiles_builds IN ACCESS EXCLUSIVE MODE') + remove_foreign_key_if_exists(:dast_site_profiles_builds, :ci_builds, name: CONSTRAINT_NAME) + end + end + + def down + add_concurrent_foreign_key(:dast_site_profiles_builds, :ci_builds, column: :ci_build_id, on_delete: :cascade, name: CONSTRAINT_NAME) + end +end diff --git a/db/post_migrate/20220119141736_remove_projects_ci_pipeline_artifacts_project_id_fk.rb b/db/post_migrate/20220119141736_remove_projects_ci_pipeline_artifacts_project_id_fk.rb new file mode 100644 index 00000000000..59a003c8f8d --- /dev/null +++ b/db/post_migrate/20220119141736_remove_projects_ci_pipeline_artifacts_project_id_fk.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class RemoveProjectsCiPipelineArtifactsProjectIdFk < Gitlab::Database::Migration[1.0] + disable_ddl_transaction! + + def up + with_lock_retries do + execute('LOCK projects, ci_pipeline_artifacts IN ACCESS EXCLUSIVE MODE') + + remove_foreign_key_if_exists(:ci_pipeline_artifacts, :projects, name: "fk_rails_4a70390ca6") + end + end + + def down + add_concurrent_foreign_key(:ci_pipeline_artifacts, :projects, name: "fk_rails_4a70390ca6", column: :project_id, target_column: :id, on_delete: :cascade) + end +end diff --git a/db/schema_migrations/20220106230629 b/db/schema_migrations/20220106230629 new file mode 100644 index 00000000000..e8751a6616c --- /dev/null +++ b/db/schema_migrations/20220106230629 @@ -0,0 +1 @@ +675d8f7bf77ddb860e707c25811d4eaaac1173c9fe62ce96c2708f0bbd0f4d48 \ No newline at end of file diff --git a/db/schema_migrations/20220106230712 b/db/schema_migrations/20220106230712 new file mode 100644 index 00000000000..589b65d423c --- /dev/null +++ b/db/schema_migrations/20220106230712 @@ -0,0 +1 @@ +672b51ca014d208f971efe698edb8a8b32f541bf9d21a7f555c53f749ee936a4 \ No newline at end of file diff --git a/db/schema_migrations/20220110233155 b/db/schema_migrations/20220110233155 new file mode 100644 index 00000000000..9301c7a2a7a --- /dev/null +++ b/db/schema_migrations/20220110233155 @@ -0,0 +1 @@ +e7d9d79ffb8989ab39fe719217f22736244df70c2b1461ef5a1a3f1e74e43870 \ No newline at end of file diff --git a/db/schema_migrations/20220112115413 b/db/schema_migrations/20220112115413 new file mode 100644 index 00000000000..9c8c653f69b --- /dev/null +++ b/db/schema_migrations/20220112115413 @@ -0,0 +1 @@ +1199adba4c13e9234eabadefeb55ed3cfb19e9d5a87c07b90d438e4f48a973f7 \ No newline at end of file diff --git a/db/schema_migrations/20220117225936 b/db/schema_migrations/20220117225936 new file mode 100644 index 00000000000..7ced75915e4 --- /dev/null +++ b/db/schema_migrations/20220117225936 @@ -0,0 +1 @@ +484eaf2ce1df1e2915b7ea8a5c9f4e044957c25d1ccf5841f24c75791d1a1a13 \ No newline at end of file diff --git a/db/schema_migrations/20220118141950 b/db/schema_migrations/20220118141950 new file mode 100644 index 00000000000..7c6549a1e60 --- /dev/null +++ b/db/schema_migrations/20220118141950 @@ -0,0 +1 @@ +a4131f86bc415f0c1897e3b975494806ffc5a834dca2b39c998c6a406e695e15 \ No newline at end of file diff --git a/db/schema_migrations/20220119141736 b/db/schema_migrations/20220119141736 new file mode 100644 index 00000000000..431ed37ea5c --- /dev/null +++ b/db/schema_migrations/20220119141736 @@ -0,0 +1 @@ +faa30b386af9adf3e9f54a0e8e2880310490e4dc1378eae68b346872d0bb8bfd \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index b035e62d6ee..1963e3a3403 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -10483,6 +10483,12 @@ CREATE TABLE application_settings ( future_subscriptions jsonb DEFAULT '[]'::jsonb NOT NULL, user_email_lookup_limit integer DEFAULT 60 NOT NULL, packages_cleanup_package_file_worker_capacity smallint DEFAULT 2 NOT NULL, + container_registry_import_max_tags_count integer DEFAULT 100 NOT NULL, + container_registry_import_max_retries integer DEFAULT 3 NOT NULL, + container_registry_import_start_max_retries integer DEFAULT 50 NOT NULL, + container_registry_import_max_step_duration integer DEFAULT 300 NOT NULL, + container_registry_import_target_plan text DEFAULT 'free'::text NOT NULL, + container_registry_import_created_before timestamp with time zone DEFAULT '2022-01-23 00:00:00+00'::timestamp with time zone NOT NULL, runner_token_expiration_interval integer, group_runner_token_expiration_interval integer, project_runner_token_expiration_interval integer, @@ -10496,6 +10502,7 @@ CREATE TABLE application_settings ( CONSTRAINT check_17d9558205 CHECK ((char_length((kroki_url)::text) <= 1024)), CONSTRAINT check_2dba05b802 CHECK ((char_length(gitpod_url) <= 255)), CONSTRAINT check_32710817e9 CHECK ((char_length(static_objects_external_storage_auth_token_encrypted) <= 255)), + CONSTRAINT check_3559645ae5 CHECK ((char_length(container_registry_import_target_plan) <= 255)), CONSTRAINT check_3def0f1829 CHECK ((char_length(sentry_clientside_dsn) <= 255)), CONSTRAINT check_4f8b811780 CHECK ((char_length(sentry_dsn) <= 255)), CONSTRAINT check_51700b31b5 CHECK ((char_length(default_branch_name) <= 255)), @@ -12910,7 +12917,19 @@ CREATE TABLE container_repositories ( status smallint, expiration_policy_started_at timestamp with time zone, expiration_policy_cleanup_status smallint DEFAULT 0 NOT NULL, - expiration_policy_completed_at timestamp with time zone + expiration_policy_completed_at timestamp with time zone, + migration_pre_import_started_at timestamp with time zone, + migration_pre_import_done_at timestamp with time zone, + migration_import_started_at timestamp with time zone, + migration_import_done_at timestamp with time zone, + migration_aborted_at timestamp with time zone, + migration_skipped_at timestamp with time zone, + migration_retries_count integer DEFAULT 0 NOT NULL, + migration_skipped_reason smallint, + migration_state text DEFAULT 'default'::text NOT NULL, + migration_aborted_in_state text, + CONSTRAINT check_13c58fe73a CHECK ((char_length(migration_state) <= 255)), + CONSTRAINT check_97f0249439 CHECK ((char_length(migration_aborted_in_state) <= 255)) ); CREATE SEQUENCE container_repositories_id_seq @@ -20308,6 +20327,7 @@ CREATE TABLE user_details ( pronunciation text, registration_objective smallint, phone text, + requires_credit_card_verification boolean DEFAULT false NOT NULL, CONSTRAINT check_245664af82 CHECK ((char_length(webauthn_xid) <= 100)), CONSTRAINT check_a73b398c60 CHECK ((char_length(phone) <= 50)), CONSTRAINT check_b132136b01 CHECK ((char_length(other_role) <= 100)), @@ -29634,9 +29654,6 @@ ALTER TABLE ONLY ci_builds ALTER TABLE ONLY ci_pipelines ADD CONSTRAINT fk_a23be95014 FOREIGN KEY (merge_request_id) REFERENCES merge_requests(id) ON DELETE CASCADE; -ALTER TABLE ONLY dast_site_profiles_builds - ADD CONSTRAINT fk_a325505e99 FOREIGN KEY (ci_build_id) REFERENCES ci_builds(id) ON DELETE CASCADE; - ALTER TABLE ONLY bulk_import_entities ADD CONSTRAINT fk_a44ff95be5 FOREIGN KEY (parent_id) REFERENCES bulk_import_entities(id) ON DELETE CASCADE; @@ -30465,9 +30482,6 @@ ALTER TABLE ONLY user_custom_attributes ALTER TABLE ONLY upcoming_reconciliations ADD CONSTRAINT fk_rails_497b4938ac FOREIGN KEY (namespace_id) REFERENCES namespaces(id) ON DELETE CASCADE; -ALTER TABLE ONLY ci_pipeline_artifacts - ADD CONSTRAINT fk_rails_4a70390ca6 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; - ALTER TABLE ONLY ci_job_token_project_scope_links ADD CONSTRAINT fk_rails_4b2ee3290b FOREIGN KEY (source_project_id) REFERENCES projects(id) ON DELETE CASCADE; diff --git a/doc/administration/redis/replication_and_failover.md b/doc/administration/redis/replication_and_failover.md index db652a80780..086057d80b4 100644 --- a/doc/administration/redis/replication_and_failover.md +++ b/doc/administration/redis/replication_and_failover.md @@ -648,6 +648,7 @@ persistence classes. | `actioncable` | Pub/Sub queue backend for ActionCable. | | `trace_chunks` | Store [CI trace chunks](../job_logs.md#enable-or-disable-incremental-logging) data. | | `rate_limiting` | Store [rate limiting](../../user/admin_area/settings/user_and_ip_rate_limits.md) state. | +| `sessions` | Store [sessions](../../../ee/development/session.md#gitlabsession). | To make this work with Sentinel: @@ -661,6 +662,7 @@ To make this work with Sentinel: gitlab_rails['redis_actioncable_instance'] = REDIS_ACTIONCABLE_URL gitlab_rails['redis_trace_chunks_instance'] = REDIS_TRACE_CHUNKS_URL gitlab_rails['redis_rate_limiting_instance'] = REDIS_RATE_LIMITING_URL + gitlab_rails['redis_sessions_instance'] = REDIS_SESSIONS_URL # Configure the Sentinels gitlab_rails['redis_cache_sentinels'] = [ @@ -687,6 +689,10 @@ To make this work with Sentinel: { host: RATE_LIMITING_SENTINEL_HOST, port: 26379 }, { host: RATE_LIMITING_SENTINEL_HOST2, port: 26379 } ] + gitlab_rails['redis_sessions_sentinels'] = [ + { host: SESSIONS_SENTINEL_HOST, port: 26379 }, + { host: SESSIONS_SENTINEL_HOST2, port: 26379 } + ] ``` - Redis URLs should be in the format: `redis://:PASSWORD@SENTINEL_PRIMARY_NAME`, where: diff --git a/doc/development/redis/new_redis_instance.md b/doc/development/redis/new_redis_instance.md index 37ee51ebb82..dcd79be0e5c 100644 --- a/doc/development/redis/new_redis_instance.md +++ b/doc/development/redis/new_redis_instance.md @@ -110,6 +110,131 @@ documentation for feature flags. When we have been using the new instance 100% of the time in production for a while and there are no issues, we can proceed. +### Proposed solution: Migrate data by using MultiStore with the fallback strategy + +We need a way to migrate users to a new Redis store without causing any inconveniences from UX perspective. +We also want the ability to fall back to the "old" Redis instance if something goes wrong with the new instance. + +Migration Requirements: + +- No downtime. +- No loss of stored data until the TTL for storing data expires. +- Partial rollout using Feature Flags or ENV vars or combinations of both. +- Monitoring of the switch. +- Prometheus metrics in place. +- Easy rollback without downtime in case the new instance or logic does not behave as expected. + +It is somewhat similar to the zero-downtime DB table rename. +We need to write data into both Redis instances (old + new). +We read from the new instance, but we need to fall back to the old instance when pre-fetching from the new dedicated Redis instance that failed. +We need to log any issues or exceptions with a new instance, but still fall back to the old instance. + +The proposed migration strategy is to implement and use the [MultiStore](https://gitlab.com/gitlab-org/gitlab/-/blob/fcc42e80ed261a862ee6ca46b182eee293ae60b6/lib/gitlab/redis/multi_store.rb). +We used this approach with [adding new dedicated Redis instance for session keys](https://gitlab.com/groups/gitlab-com/gl-infra/-/epics/579). +Also MultiStore comes with corresponding [specs](https://gitlab.com/gitlab-org/gitlab/-/blob/master/spec/lib/gitlab/redis/multi_store_spec.rb). + +The MultiStore looks like a `redis-rb ::Redis` instance. + +In the new Redis instance class you added in [Step 1](#step-1-support-configuring-the-new-instance), +override the [Redis](https://gitlab.com/gitlab-org/gitlab/-/blob/fcc42e80ed261a862ee6ca46b182eee293ae60b6/lib/gitlab/redis/sessions.rb#L20-28) method from the `::Gitlab::Redis::Wrapper` + +```ruby +module Gitlab + module Redis + class Foo < ::Gitlab::Redis::Wrapper + ... + def self.redis + # Don't use multistore if redis.foo configuration is not provided + return super if config_fallback? + + primary_store = ::Redis.new(params) + secondary_store = ::Redis.new(config_fallback.params) + + MultiStore.new(primary_store, secondary_store, store_name) + end + end + end +end +``` + +MultiStore is initialized by providing the new Redis instance as a primary store, and [old (fallback-instance)](#fallback-instance) as a secondary store. +The third argument is `store_name` which is used for logs, metrics and feature flag names, in case we use MultiStore implementation for different Redis stores at the same time. + +By default, the MultiStore reads and writes only from the default Redis store. +The default Redis store is `secondary_store` (the old fallback-instance). +This allows us to introduce MultiStore without changing the default behavior. + +MultiStore uses two feature flags to control the actual migration: + +- `use_primary_and_secondary_stores_for_[store_name]` +- `use_primary_store_as_default_for_[store_name]` + +For example, if our new Redis instance is called `Gitlab::Redis::Foo`, we can [create](../../../ee/development/feature_flags/#create-a-new-feature-flag) two feature flags by executing: + +```shell +bin/feature-flag use_primary_and_secondary_stores_for_foo +bin/feature-flag use_primary_store_as_default_for_foo +``` + +By enabling `use_primary_and_secondary_stores_for_foo` feature flag, our `Gitlab::Redis::Foo` will use `MultiStore` to write to both new Redis instance +and the [old (fallback-instance)](#fallback-instance). +If we fail to fetch data from the new instance, we will fallback and read from the old Redis instance. + +We can monitor logs for `Gitlab::Redis::MultiStore::ReadFromPrimaryError`, and also the Prometheus counter `gitlab_redis_multi_store_read_fallback_total`. +Once we stop seeing them, this means that we are no longer relying on the data stored on the old Redis store. +At this point, we are probably safe to move the traffic to the new Redis store. + +By enabling `use_primary_store_as_default_for_foo` feature flag, the `MultiStore` will use `primary_store` (new instance) as default Redis store. + +Once this feature flag is enabled, we can disable `use_primary_and_secondary_stores_for_foo` feature flag. +This will allow the MultiStore to read and write only from the primary Redis store (new store), moving all the traffic to the new Redis store. + +Once we have moved all our traffic to the primary store, our data migration is complete. +We can safely remove the MultiStore implementation and continue to use newly introduced Redis store instance. + +#### Implementation details + +MultiStore implements read and write Redis commands separately. + +##### Read commands + +- `get` +- `mget` +- `smembers` +- `scard` + +##### Write commands + +- `set` +- `setnx` +- `setex` +- `sadd` +- `srem` +- `del` +- `pipelined` +- `flushdb` + +When a command outside of the supported list is used, `method_missing` will pass it to the old Redis instance and keep track of it. +This ensures that anything unexpected behaves like it would before. + +NOTE: +By tracking `gitlab_redis_multi_store_method_missing_total` counter and `Gitlab::Redis::MultiStore::MethodMissingError`, +a developer will need to add an implementation for missing Redis commands before proceeding with the migration. + +##### Errors + +| error | message | +|-------------------------------------------------|-----------------------------------------------------------------------| +| `Gitlab::Redis::MultiStore::ReadFromPrimaryError` | Value not found on the Redis primary store. Read from the Redis secondary store successful. | +| `Gitlab::Redis::MultiStore::MethodMissingError` | Method missing. Falling back to execute method on the Redis secondary store. | + +##### Metrics + +| metrics name | type | labels | description | +|-------------------------------------------------|--------------------|------------------------|----------------------------------------------------| +| gitlab_redis_multi_store_read_fallback_total | Prometheus Counter | command, instance_name | Client side Redis MultiStore reading fallback total| +| gitlab_redis_multi_store_method_missing_total | Prometheus Counter | command, instance_name | Client side Redis MultiStore method missing total | + ## Step 4: clean up after the migration diff --git a/doc/topics/git/troubleshooting_git.md b/doc/topics/git/troubleshooting_git.md index 328aba4960f..f881826e74a 100644 --- a/doc/topics/git/troubleshooting_git.md +++ b/doc/topics/git/troubleshooting_git.md @@ -110,6 +110,13 @@ ssh_exchange_identification: Connection closed by remote host fatal: The remote end hung up unexpectedly ``` +or + +```plaintext +kex_exchange_identification: Connection closed by remote host +Connection closed by x.x.x.x port 22 +``` + This error usually indicates that SSH daemon's `MaxStartups` value is throttling SSH connections. This setting specifies the maximum number of concurrent, unauthenticated connections to the SSH daemon. This affects users with proper authentication diff --git a/lib/api/users.rb b/lib/api/users.rb index efecc7593d0..eeb5244466a 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -1076,7 +1076,7 @@ module API attrs = declared_params(include_missing: false) - service = ::Users::UpsertCreditCardValidationService.new(attrs).execute + service = ::Users::UpsertCreditCardValidationService.new(attrs, user).execute if service.success? present user.credit_card_validation, with: Entities::UserCreditCardValidations diff --git a/lib/gitlab/ci/build/policy/refs.rb b/lib/gitlab/ci/build/policy/refs.rb index afe0ccb361e..7ade9ca5085 100644 --- a/lib/gitlab/ci/build/policy/refs.rb +++ b/lib/gitlab/ci/build/policy/refs.rb @@ -35,7 +35,10 @@ module Gitlab # patterns can be matched only when branch or tag is used # the pattern matching does not work for merge requests pipelines if pipeline.branch? || pipeline.tag? - if regexp = Gitlab::UntrustedRegexp::RubySyntax.fabricate(pattern, fallback: true) + regexp = Gitlab::UntrustedRegexp::RubySyntax + .fabricate(pattern, fallback: true, project: pipeline.project) + + if regexp regexp.match?(pipeline.ref) else pattern == pipeline.ref diff --git a/lib/gitlab/database/gitlab_loose_foreign_keys.yml b/lib/gitlab/database/gitlab_loose_foreign_keys.yml index 93e43073b0a..d694165574d 100644 --- a/lib/gitlab/database/gitlab_loose_foreign_keys.yml +++ b/lib/gitlab/database/gitlab_loose_foreign_keys.yml @@ -154,3 +154,7 @@ ci_secure_files: - table: projects column: project_id on_delete: async_delete +ci_pipeline_artifacts: + - table: projects + column: project_id + on_delete: async_delete diff --git a/lib/gitlab/sidekiq_logging/json_formatter.rb b/lib/gitlab/sidekiq_logging/json_formatter.rb index a6281bbdf26..dd50fef8c3d 100644 --- a/lib/gitlab/sidekiq_logging/json_formatter.rb +++ b/lib/gitlab/sidekiq_logging/json_formatter.rb @@ -2,6 +2,7 @@ # This is needed for sidekiq-cluster require 'json' +require 'sidekiq/job_retry' module Gitlab module SidekiqLogging diff --git a/lib/gitlab/sidekiq_logging/structured_logger.rb b/lib/gitlab/sidekiq_logging/structured_logger.rb index 3438bc0f3ef..a9bfcce2e0a 100644 --- a/lib/gitlab/sidekiq_logging/structured_logger.rb +++ b/lib/gitlab/sidekiq_logging/structured_logger.rb @@ -2,6 +2,8 @@ require 'active_record' require 'active_record/log_subscriber' +require 'sidekiq/job_logger' +require 'sidekiq/job_retry' module Gitlab module SidekiqLogging diff --git a/lib/gitlab/sidekiq_middleware/monitor.rb b/lib/gitlab/sidekiq_middleware/monitor.rb index ed825dbfd60..d38fed3b768 100644 --- a/lib/gitlab/sidekiq_middleware/monitor.rb +++ b/lib/gitlab/sidekiq_middleware/monitor.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require 'sidekiq/job_retry' + module Gitlab module SidekiqMiddleware class Monitor diff --git a/lib/gitlab/untrusted_regexp/ruby_syntax.rb b/lib/gitlab/untrusted_regexp/ruby_syntax.rb index 6adf119aa75..010214cf295 100644 --- a/lib/gitlab/untrusted_regexp/ruby_syntax.rb +++ b/lib/gitlab/untrusted_regexp/ruby_syntax.rb @@ -20,13 +20,13 @@ module Gitlab !!self.fabricate(pattern, fallback: fallback) end - def self.fabricate(pattern, fallback: false) - self.fabricate!(pattern, fallback: fallback) + def self.fabricate(pattern, fallback: false, project: nil) + self.fabricate!(pattern, fallback: fallback, project: project) rescue RegexpError nil end - def self.fabricate!(pattern, fallback: false) + def self.fabricate!(pattern, fallback: false, project: nil) raise RegexpError, 'Pattern is not string!' unless pattern.is_a?(String) matches = pattern.match(PATTERN) @@ -38,6 +38,16 @@ module Gitlab raise unless fallback && Feature.enabled?(:allow_unsafe_ruby_regexp, default_enabled: false) + if Feature.enabled?(:ci_unsafe_regexp_logger, type: :ops, default_enabled: :yaml) + Gitlab::AppJsonLogger.info( + class: self.class.name, + regexp: pattern.to_s, + fabricated: 'unsafe ruby regexp', + project_id: project&.id, + project_path: project&.full_path + ) + end + create_ruby_regexp(matches[:regexp], matches[:flags]) end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 0c10c5022f7..0f413a116ff 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -11988,9 +11988,30 @@ msgstr[1] "" msgid "Deployment|API" msgstr "" +msgid "Deployment|Cancelled" +msgstr "" + +msgid "Deployment|Created" +msgstr "" + +msgid "Deployment|Failed" +msgstr "" + +msgid "Deployment|Running" +msgstr "" + +msgid "Deployment|Skipped" +msgstr "" + +msgid "Deployment|Success" +msgstr "" + msgid "Deployment|This deployment was created using the API" msgstr "" +msgid "Deployment|Waiting" +msgstr "" + msgid "Deployment|blocked" msgstr "" @@ -17847,9 +17868,15 @@ msgstr "" msgid "Identities" msgstr "" +msgid "IdentityVerification|Before you create your first project, we need you to verify your identity with a valid payment method. You will not be charged during this step. If we ever need to charge you, we will let you know." +msgstr "" + msgid "IdentityVerification|Before you create your group, we need you to verify your identity with a valid payment method." msgstr "" +msgid "IdentityVerification|Create a project" +msgstr "" + msgid "IdentityVerification|Verify your identity" msgstr "" diff --git a/spec/experiments/require_verification_for_namespace_creation_experiment_spec.rb b/spec/experiments/require_verification_for_namespace_creation_experiment_spec.rb new file mode 100644 index 00000000000..87417fe1637 --- /dev/null +++ b/spec/experiments/require_verification_for_namespace_creation_experiment_spec.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe RequireVerificationForNamespaceCreationExperiment, :experiment do + subject(:experiment) { described_class.new(user: user) } + + let_it_be(:user) { create(:user) } + + describe '#candidate?' do + context 'when experiment subject is candidate' do + before do + stub_experiments(require_verification_for_namespace_creation: :candidate) + end + + it 'returns true' do + expect(experiment.candidate?).to eq(true) + end + end + + context 'when experiment subject is control' do + before do + stub_experiments(require_verification_for_namespace_creation: :control) + end + + it 'returns false' do + expect(experiment.candidate?).to eq(false) + end + end + end + + describe '#record_conversion' do + let_it_be(:namespace) { create(:namespace) } + + context 'when should_track? is false' do + before do + allow(experiment).to receive(:should_track?).and_return(false) + end + + it 'does not record a conversion event' do + expect(experiment.publish_to_database).to be_nil + expect(experiment.record_conversion(namespace)).to be_nil + end + end + + context 'when should_track? is true' do + before do + allow(experiment).to receive(:should_track?).and_return(true) + end + + it 'records a conversion event' do + experiment_subject = experiment.publish_to_database + + expect { experiment.record_conversion(namespace) }.to change { experiment_subject.reload.converted_at }.from(nil) + .and change { experiment_subject.context }.to include('namespace_id' => namespace.id) + end + end + end +end diff --git a/spec/features/admin/integrations/user_activates_mattermost_slash_command_spec.rb b/spec/features/admin/integrations/user_activates_mattermost_slash_command_spec.rb index 22a27b33671..793a5bced00 100644 --- a/spec/features/admin/integrations/user_activates_mattermost_slash_command_spec.rb +++ b/spec/features/admin/integrations/user_activates_mattermost_slash_command_spec.rb @@ -19,4 +19,19 @@ RSpec.describe 'User activates the instance-level Mattermost Slash Command integ expect(page).to have_link('Settings', href: edit_path) expect(page).to have_link('Projects using custom settings', href: overrides_path) end + + it 'does not render integration form element' do + expect(page).not_to have_selector('[data-testid="integration-form"]') + end + + context 'when `vue_integration_form` feature flag is disabled' do + before do + stub_feature_flags(vue_integration_form: false) + visit_instance_integration('Mattermost slash commands') + end + + it 'renders integration form element' do + expect(page).to have_selector('[data-testid="integration-form"]') + end + end end diff --git a/spec/features/projects/integrations/user_activates_jira_spec.rb b/spec/features/projects/integrations/user_activates_jira_spec.rb index 23fa837dfb9..50010950f0e 100644 --- a/spec/features/projects/integrations/user_activates_jira_spec.rb +++ b/spec/features/projects/integrations/user_activates_jira_spec.rb @@ -41,7 +41,7 @@ RSpec.describe 'User activates Jira', :js do fill_in 'service_password', with: 'password' click_test_integration - page.within('.service-settings') do + page.within('[data-testid="integration-settings-form"]') do expect(page).to have_content('This field is required.') end end diff --git a/spec/frontend/content_editor/extensions/image_spec.js b/spec/frontend/content_editor/extensions/image_spec.js new file mode 100644 index 00000000000..256f7bad309 --- /dev/null +++ b/spec/frontend/content_editor/extensions/image_spec.js @@ -0,0 +1,41 @@ +import Image from '~/content_editor/extensions/image'; +import { createTestEditor, createDocBuilder } from '../test_utils'; + +describe('content_editor/extensions/image', () => { + let tiptapEditor; + let doc; + let p; + let image; + + beforeEach(() => { + tiptapEditor = createTestEditor({ extensions: [Image] }); + + ({ + builders: { doc, p, image }, + } = createDocBuilder({ + tiptapEditor, + names: { + image: { nodeType: Image.name }, + }, + })); + }); + + it('adds data-canonical-src attribute when rendering to HTML', () => { + const initialDoc = doc( + p( + image({ + canonicalSrc: 'uploads/image.jpg', + src: '/-/wikis/uploads/image.jpg', + alt: 'image', + title: 'this is an image', + }), + ), + ); + + tiptapEditor.commands.setContent(initialDoc.toJSON()); + + expect(tiptapEditor.getHTML()).toEqual( + '

image

', + ); + }); +}); diff --git a/spec/frontend/content_editor/services/markdown_serializer_spec.js b/spec/frontend/content_editor/services/markdown_serializer_spec.js index 68ccb01ddd1..01d4c994e88 100644 --- a/spec/frontend/content_editor/services/markdown_serializer_spec.js +++ b/spec/frontend/content_editor/services/markdown_serializer_spec.js @@ -352,6 +352,10 @@ this is not really json but just trying out whether this case works or not ); }); + it('does not serialize an image when src and canonicalSrc are empty', () => { + expect(serialize(paragraph(image({})))).toBe(''); + }); + it('correctly serializes an image with a title', () => { expect(serialize(paragraph(image({ src: 'img.jpg', title: 'baz', alt: 'foo bar' })))).toBe( '![foo bar](img.jpg "baz")', diff --git a/spec/frontend/environments/deployment_spec.js b/spec/frontend/environments/deployment_spec.js new file mode 100644 index 00000000000..37209bdc86c --- /dev/null +++ b/spec/frontend/environments/deployment_spec.js @@ -0,0 +1,29 @@ +import { mountExtended } from 'helpers/vue_test_utils_helper'; +import Deployment from '~/environments/components/deployment.vue'; +import DeploymentStatusBadge from '~/environments/components/deployment_status_badge.vue'; +import { resolvedEnvironment } from './graphql/mock_data'; + +describe('~/environments/components/deployment.vue', () => { + let wrapper; + + const createWrapper = ({ propsData = {} } = {}) => + mountExtended(Deployment, { + propsData: { + deployment: resolvedEnvironment.lastDeployment, + ...propsData, + }, + }); + + afterEach(() => { + wrapper?.destroy(); + }); + + describe('status', () => { + it('should pass the deployable status to the badge', () => { + wrapper = createWrapper(); + expect(wrapper.findComponent(DeploymentStatusBadge).props('status')).toBe( + resolvedEnvironment.lastDeployment.status, + ); + }); + }); +}); diff --git a/spec/frontend/environments/deployment_status_badge_spec.js b/spec/frontend/environments/deployment_status_badge_spec.js new file mode 100644 index 00000000000..02aae57396a --- /dev/null +++ b/spec/frontend/environments/deployment_status_badge_spec.js @@ -0,0 +1,42 @@ +import { GlBadge } from '@gitlab/ui'; +import { mountExtended } from 'helpers/vue_test_utils_helper'; +import { s__ } from '~/locale'; +import DeploymentStatusBadge from '~/environments/components/deployment_status_badge.vue'; + +describe('~/environments/components/deployment_status_badge.vue', () => { + let wrapper; + + const createWrapper = ({ propsData = {} } = {}) => + mountExtended(DeploymentStatusBadge, { + propsData, + }); + + describe.each` + status | text | variant | icon + ${'created'} | ${s__('Deployment|Created')} | ${'neutral'} | ${'status_created'} + ${'running'} | ${s__('Deployment|Running')} | ${'info'} | ${'status_running'} + ${'success'} | ${s__('Deployment|Success')} | ${'success'} | ${'status_success'} + ${'failed'} | ${s__('Deployment|Failed')} | ${'danger'} | ${'status_failed'} + ${'canceled'} | ${s__('Deployment|Cancelled')} | ${'neutral'} | ${'status_canceled'} + ${'skipped'} | ${s__('Deployment|Skipped')} | ${'neutral'} | ${'status_skipped'} + ${'blocked'} | ${s__('Deployment|Waiting')} | ${'neutral'} | ${'status_manual'} + `('$status', ({ status, text, variant, icon }) => { + let badge; + + beforeEach(() => { + wrapper = createWrapper({ propsData: { status } }); + badge = wrapper.findComponent(GlBadge); + }); + + it(`sets the text to ${text}`, () => { + expect(wrapper.text()).toBe(text); + }); + + it(`sets the variant to ${variant}`, () => { + expect(badge.props('variant')).toBe(variant); + }); + it(`sets the icon to ${icon}`, () => { + expect(badge.props('icon')).toBe(icon); + }); + }); +}); diff --git a/spec/frontend/integrations/edit/components/integration_form_spec.js b/spec/frontend/integrations/edit/components/integration_form_spec.js index 6aa955033f7..8cf8a403e5d 100644 --- a/spec/frontend/integrations/edit/components/integration_form_spec.js +++ b/spec/frontend/integrations/edit/components/integration_form_spec.js @@ -1,8 +1,9 @@ +import { GlForm } from '@gitlab/ui'; import axios from 'axios'; import MockAdapter from 'axios-mock-adapter'; import * as Sentry from '@sentry/browser'; import { setHTMLFixture } from 'helpers/fixtures'; -import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; +import { mountExtended, shallowMountExtended } from 'helpers/vue_test_utils_helper'; import waitForPromises from 'helpers/wait_for_promises'; import ActiveCheckbox from '~/integrations/edit/components/active_checkbox.vue'; import ConfirmationModal from '~/integrations/edit/components/confirmation_modal.vue'; @@ -36,12 +37,18 @@ describe('IntegrationForm', () => { let dispatch; let mockAxios; let mockForm; + let vueIntegrationFormFeatureFlag; + + const createForm = () => { + mockForm = document.createElement('form'); + jest.spyOn(document, 'querySelector').mockReturnValue(mockForm); + }; const createComponent = ({ customStateProps = {}, - featureFlags = {}, initialState = {}, props = {}, + mountFn = shallowMountExtended, } = {}) => { const store = createStore({ customState: { ...mockIntegrationProps, ...customStateProps }, @@ -49,11 +56,12 @@ describe('IntegrationForm', () => { }); dispatch = jest.spyOn(store, 'dispatch').mockImplementation(); - wrapper = shallowMountExtended(IntegrationForm, { - propsData: { ...props, formSelector: '.test' }, - provide: { - glFeatures: featureFlags, - }, + if (!vueIntegrationFormFeatureFlag) { + createForm(); + } + + wrapper = mountFn(IntegrationForm, { + propsData: { ...props }, store, stubs: { OverrideDropdown, @@ -67,16 +75,14 @@ describe('IntegrationForm', () => { show: mockToastShow, }, }, + provide: { + glFeatures: { + vueIntegrationForm: vueIntegrationFormFeatureFlag, + }, + }, }); }; - const createForm = ({ isValid = true } = {}) => { - mockForm = document.createElement('form'); - jest.spyOn(document, 'querySelector').mockReturnValue(mockForm); - jest.spyOn(mockForm, 'checkValidity').mockReturnValue(isValid); - jest.spyOn(mockForm, 'submit'); - }; - const findOverrideDropdown = () => wrapper.findComponent(OverrideDropdown); const findActiveCheckbox = () => wrapper.findComponent(ActiveCheckbox); const findConfirmationModal = () => wrapper.findComponent(ConfirmationModal); @@ -88,6 +94,14 @@ describe('IntegrationForm', () => { const findJiraTriggerFields = () => wrapper.findComponent(JiraTriggerFields); const findJiraIssuesFields = () => wrapper.findComponent(JiraIssuesFields); const findTriggerFields = () => wrapper.findComponent(TriggerFields); + const findGlForm = () => wrapper.findComponent(GlForm); + const findRedirectToField = () => wrapper.findByTestId('redirect-to-field'); + const findFormElement = () => (vueIntegrationFormFeatureFlag ? findGlForm().element : mockForm); + + const mockFormFunctions = ({ checkValidityReturn }) => { + jest.spyOn(findFormElement(), 'checkValidity').mockReturnValue(checkValidityReturn); + jest.spyOn(findFormElement(), 'submit'); + }; beforeEach(() => { mockAxios = new MockAdapter(axios); @@ -223,6 +237,7 @@ describe('IntegrationForm', () => { createComponent({ customStateProps: { type: 'jira', testPath: '/test' }, + mountFn: mountExtended, }); }); @@ -341,6 +356,19 @@ describe('IntegrationForm', () => { }); }); }); + + describe('when `vueIntegrationForm` feature flag is $vueIntegrationFormEnabled', () => { + it('renders hidden fields', () => { + vueIntegrationFormFeatureFlag = true; + createComponent({ + customStateProps: { + redirectTo: '/services', + }, + }); + + expect(findRedirectToField().attributes('value')).toBe('/services'); + }); + }); }); describe('ActiveCheckbox', () => { @@ -361,193 +389,216 @@ describe('IntegrationForm', () => { }); describe.each` - formActive | novalidate - ${true} | ${null} - ${false} | ${'true'} + formActive | vueIntegrationFormEnabled | novalidate + ${true} | ${true} | ${null} + ${false} | ${true} | ${'novalidate'} + ${true} | ${false} | ${null} + ${false} | ${false} | ${'true'} `( - 'when `toggle-integration-active` is emitted with $formActive', - ({ formActive, novalidate }) => { + 'when `vueIntegrationForm` feature flag is $vueIntegrationFormEnabled and `toggle-integration-active` is emitted with $formActive', + ({ formActive, vueIntegrationFormEnabled, novalidate }) => { beforeEach(async () => { - createForm(); + vueIntegrationFormFeatureFlag = vueIntegrationFormEnabled; + createComponent({ customStateProps: { showActive: true, initialActivated: false, }, + mountFn: mountExtended, }); + mockFormFunctions({ checkValidityReturn: false }); await findActiveCheckbox().vm.$emit('toggle-integration-active', formActive); }); it(`sets noValidate to ${novalidate}`, () => { - expect(mockForm.getAttribute('novalidate')).toBe(novalidate); + expect(findFormElement().getAttribute('novalidate')).toBe(novalidate); }); }, ); }); - describe('when `save` button is clicked', () => { - describe('buttons', () => { - beforeEach(async () => { - createForm(); - createComponent({ - customStateProps: { - showActive: true, - canTest: true, - initialActivated: true, - }, - }); - - await findProjectSaveButton().vm.$emit('click', new Event('click')); - }); - - it('sets save button `loading` prop to `true`', () => { - expect(findProjectSaveButton().props('loading')).toBe(true); - }); - - it('sets test button `disabled` prop to `true`', () => { - expect(findTestButton().props('disabled')).toBe(true); - }); - }); - - describe.each` - checkValidityReturn | integrationActive - ${true} | ${false} - ${true} | ${true} - ${false} | ${false} - `( - 'when form is valid (checkValidity returns $checkValidityReturn and integrationActive is $integrationActive)', - ({ integrationActive, checkValidityReturn }) => { - beforeEach(async () => { - createForm({ isValid: checkValidityReturn }); - createComponent({ - customStateProps: { - showActive: true, - canTest: true, - initialActivated: integrationActive, - }, - }); - - await findProjectSaveButton().vm.$emit('click', new Event('click')); - }); - - it('submit form', () => { - expect(mockForm.submit).toHaveBeenCalledTimes(1); - }); - }, - ); - - describe('when form is invalid (checkValidity returns false and integrationActive is true)', () => { - beforeEach(async () => { - createForm({ isValid: false }); - createComponent({ - customStateProps: { - showActive: true, - canTest: true, - initialActivated: true, - }, - }); - - await findProjectSaveButton().vm.$emit('click', new Event('click')); - }); - - it('does not submit form', () => { - expect(mockForm.submit).not.toHaveBeenCalled(); - }); - - it('sets save button `loading` prop to `false`', () => { - expect(findProjectSaveButton().props('loading')).toBe(false); - }); - - it('sets test button `disabled` prop to `false`', () => { - expect(findTestButton().props('disabled')).toBe(false); - }); - - it('emits `VALIDATE_INTEGRATION_FORM_EVENT`', () => { - expect(eventHub.$emit).toHaveBeenCalledWith(VALIDATE_INTEGRATION_FORM_EVENT); - }); - }); - }); - - describe('when `test` button is clicked', () => { - describe('when form is invalid', () => { - it('emits `VALIDATE_INTEGRATION_FORM_EVENT` event to the event hub', () => { - createForm({ isValid: false }); - createComponent({ - customStateProps: { - showActive: true, - canTest: true, - }, - }); - - findTestButton().vm.$emit('click', new Event('click')); - - expect(eventHub.$emit).toHaveBeenCalledWith(VALIDATE_INTEGRATION_FORM_EVENT); - }); - }); - - describe('when form is valid', () => { - const mockTestPath = '/test'; - + describe.each` + vueIntegrationFormEnabled + ${true} + ${false} + `( + 'when `vueIntegrationForm` feature flag is $vueIntegrationFormEnabled', + ({ vueIntegrationFormEnabled }) => { beforeEach(() => { - createForm({ isValid: true }); - createComponent({ - customStateProps: { - showActive: true, - canTest: true, - testPath: mockTestPath, - }, - }); + vueIntegrationFormFeatureFlag = vueIntegrationFormEnabled; }); - describe('buttons', () => { - beforeEach(async () => { - await findTestButton().vm.$emit('click', new Event('click')); - }); + describe('when `save` button is clicked', () => { + describe('buttons', () => { + beforeEach(async () => { + createComponent({ + customStateProps: { + showActive: true, + canTest: true, + initialActivated: true, + }, + mountFn: mountExtended, + }); - it('sets test button `loading` prop to `true`', () => { - expect(findTestButton().props('loading')).toBe(true); - }); - - it('sets save button `disabled` prop to `true`', () => { - expect(findProjectSaveButton().props('disabled')).toBe(true); - }); - }); - - describe.each` - scenario | replyStatus | errorMessage | expectToast | expectSentry - ${'when "test settings" request fails'} | ${httpStatus.INTERNAL_SERVER_ERROR} | ${undefined} | ${I18N_DEFAULT_ERROR_MESSAGE} | ${true} - ${'when "test settings" returns an error'} | ${httpStatus.OK} | ${'an error'} | ${'an error'} | ${false} - ${'when "test settings" succeeds'} | ${httpStatus.OK} | ${undefined} | ${I18N_SUCCESSFUL_CONNECTION_MESSAGE} | ${false} - `('$scenario', ({ replyStatus, errorMessage, expectToast, expectSentry }) => { - beforeEach(async () => { - mockAxios.onPut(mockTestPath).replyOnce(replyStatus, { - error: Boolean(errorMessage), - message: errorMessage, + await findProjectSaveButton().vm.$emit('click', new Event('click')); }); - await findTestButton().vm.$emit('click', new Event('click')); - await waitForPromises(); + it('sets save button `loading` prop to `true`', () => { + expect(findProjectSaveButton().props('loading')).toBe(true); + }); + + it('sets test button `disabled` prop to `true`', () => { + expect(findTestButton().props('disabled')).toBe(true); + }); }); - it(`calls toast with '${expectToast}'`, () => { - expect(mockToastShow).toHaveBeenCalledWith(expectToast); - }); + describe.each` + checkValidityReturn | integrationActive + ${true} | ${false} + ${true} | ${true} + ${false} | ${false} + `( + 'when form is valid (checkValidity returns $checkValidityReturn and integrationActive is $integrationActive)', + ({ integrationActive, checkValidityReturn }) => { + beforeEach(async () => { + createComponent({ + customStateProps: { + showActive: true, + canTest: true, + initialActivated: integrationActive, + }, + mountFn: mountExtended, + }); - it('sets `loading` prop of test button to `false`', () => { - expect(findTestButton().props('loading')).toBe(false); - }); + mockFormFunctions({ checkValidityReturn }); - it('sets save button `disabled` prop to `false`', () => { - expect(findProjectSaveButton().props('disabled')).toBe(false); - }); + await findProjectSaveButton().vm.$emit('click', new Event('click')); + }); - it(`${expectSentry ? 'does' : 'does not'} capture exception in Sentry`, () => { - expect(Sentry.captureException).toHaveBeenCalledTimes(expectSentry ? 1 : 0); + it('submits form', () => { + expect(findFormElement().submit).toHaveBeenCalledTimes(1); + }); + }, + ); + + describe('when form is invalid (checkValidity returns false and integrationActive is true)', () => { + beforeEach(async () => { + createComponent({ + customStateProps: { + showActive: true, + canTest: true, + initialActivated: true, + }, + mountFn: mountExtended, + }); + mockFormFunctions({ checkValidityReturn: false }); + + await findProjectSaveButton().vm.$emit('click', new Event('click')); + }); + + it('does not submit form', () => { + expect(findFormElement().submit).not.toHaveBeenCalled(); + }); + + it('sets save button `loading` prop to `false`', () => { + expect(findProjectSaveButton().props('loading')).toBe(false); + }); + + it('sets test button `disabled` prop to `false`', () => { + expect(findTestButton().props('disabled')).toBe(false); + }); + + it('emits `VALIDATE_INTEGRATION_FORM_EVENT`', () => { + expect(eventHub.$emit).toHaveBeenCalledWith(VALIDATE_INTEGRATION_FORM_EVENT); + }); }); }); - }); - }); + + describe('when `test` button is clicked', () => { + describe('when form is invalid', () => { + it('emits `VALIDATE_INTEGRATION_FORM_EVENT` event to the event hub', () => { + createComponent({ + customStateProps: { + showActive: true, + canTest: true, + }, + mountFn: mountExtended, + }); + mockFormFunctions({ checkValidityReturn: false }); + + findTestButton().vm.$emit('click', new Event('click')); + + expect(eventHub.$emit).toHaveBeenCalledWith(VALIDATE_INTEGRATION_FORM_EVENT); + }); + }); + + describe('when form is valid', () => { + const mockTestPath = '/test'; + + beforeEach(() => { + createComponent({ + customStateProps: { + showActive: true, + canTest: true, + testPath: mockTestPath, + }, + mountFn: mountExtended, + }); + mockFormFunctions({ checkValidityReturn: true }); + }); + + describe('buttons', () => { + beforeEach(async () => { + await findTestButton().vm.$emit('click', new Event('click')); + }); + + it('sets test button `loading` prop to `true`', () => { + expect(findTestButton().props('loading')).toBe(true); + }); + + it('sets save button `disabled` prop to `true`', () => { + expect(findProjectSaveButton().props('disabled')).toBe(true); + }); + }); + + describe.each` + scenario | replyStatus | errorMessage | expectToast | expectSentry + ${'when "test settings" request fails'} | ${httpStatus.INTERNAL_SERVER_ERROR} | ${undefined} | ${I18N_DEFAULT_ERROR_MESSAGE} | ${true} + ${'when "test settings" returns an error'} | ${httpStatus.OK} | ${'an error'} | ${'an error'} | ${false} + ${'when "test settings" succeeds'} | ${httpStatus.OK} | ${undefined} | ${I18N_SUCCESSFUL_CONNECTION_MESSAGE} | ${false} + `('$scenario', ({ replyStatus, errorMessage, expectToast, expectSentry }) => { + beforeEach(async () => { + mockAxios.onPut(mockTestPath).replyOnce(replyStatus, { + error: Boolean(errorMessage), + message: errorMessage, + }); + + await findTestButton().vm.$emit('click', new Event('click')); + await waitForPromises(); + }); + + it(`calls toast with '${expectToast}'`, () => { + expect(mockToastShow).toHaveBeenCalledWith(expectToast); + }); + + it('sets `loading` prop of test button to `false`', () => { + expect(findTestButton().props('loading')).toBe(false); + }); + + it('sets save button `disabled` prop to `false`', () => { + expect(findProjectSaveButton().props('disabled')).toBe(false); + }); + + it(`${expectSentry ? 'does' : 'does not'} capture exception in Sentry`, () => { + expect(Sentry.captureException).toHaveBeenCalledTimes(expectSentry ? 1 : 0); + }); + }); + }); + }); + }, + ); describe('when `reset-confirmation-modal` emits `reset` event', () => { const mockResetPath = '/reset'; diff --git a/spec/helpers/integrations_helper_spec.rb b/spec/helpers/integrations_helper_spec.rb index df5f9be3800..38ce17e34ba 100644 --- a/spec/helpers/integrations_helper_spec.rb +++ b/spec/helpers/integrations_helper_spec.rb @@ -20,6 +20,12 @@ RSpec.describe IntegrationsHelper do end describe '#integration_form_data' do + before do + allow(helper).to receive_messages( + request: double(referer: '/services') + ) + end + let(:fields) do [ :id, @@ -39,7 +45,9 @@ RSpec.describe IntegrationsHelper do :cancel_path, :can_test, :test_path, - :reset_path + :reset_path, + :form_path, + :redirect_to ] end @@ -61,6 +69,10 @@ RSpec.describe IntegrationsHelper do specify do expect(subject[:reset_path]).to eq(helper.scoped_reset_integration_path(integration)) end + + specify do + expect(subject[:redirect_to]).to eq('/services') + end end context 'Jira service' do diff --git a/spec/lib/gitlab/database/no_cross_db_foreign_keys_spec.rb b/spec/lib/gitlab/database/no_cross_db_foreign_keys_spec.rb index 35674dea0d5..e5a8143fcc3 100644 --- a/spec/lib/gitlab/database/no_cross_db_foreign_keys_spec.rb +++ b/spec/lib/gitlab/database/no_cross_db_foreign_keys_spec.rb @@ -23,7 +23,6 @@ RSpec.describe 'cross-database foreign keys' do ci_job_token_project_scope_links.target_project_id ci_pending_builds.namespace_id ci_pending_builds.project_id - ci_pipeline_artifacts.project_id ci_pipeline_schedules.owner_id ci_pipeline_schedules.project_id ci_pipelines.merge_request_id diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index 8cc763b0201..0ece212d692 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -77,6 +77,18 @@ RSpec.describe ApplicationSetting do it { is_expected.to validate_numericality_of(:container_registry_cleanup_tags_service_max_list_size).only_integer.is_greater_than_or_equal_to(0) } it { is_expected.to validate_numericality_of(:container_registry_expiration_policies_worker_capacity).only_integer.is_greater_than_or_equal_to(0) } + it { is_expected.to validate_numericality_of(:container_registry_import_max_tags_count).only_integer.is_greater_than_or_equal_to(0) } + it { is_expected.to validate_numericality_of(:container_registry_import_max_retries).only_integer.is_greater_than_or_equal_to(0) } + it { is_expected.to validate_numericality_of(:container_registry_import_start_max_retries).only_integer.is_greater_than_or_equal_to(0) } + it { is_expected.to validate_numericality_of(:container_registry_import_max_step_duration).only_integer.is_greater_than_or_equal_to(0) } + it { is_expected.not_to allow_value(nil).for(:container_registry_import_max_tags_count) } + it { is_expected.not_to allow_value(nil).for(:container_registry_import_max_retries) } + it { is_expected.not_to allow_value(nil).for(:container_registry_import_start_max_retries) } + it { is_expected.not_to allow_value(nil).for(:container_registry_import_max_step_duration) } + + it { is_expected.to validate_presence_of(:container_registry_import_target_plan) } + it { is_expected.to validate_presence_of(:container_registry_import_created_before) } + it { is_expected.to validate_numericality_of(:dependency_proxy_ttl_group_policy_worker_capacity).only_integer.is_greater_than_or_equal_to(0) } it { is_expected.not_to allow_value(nil).for(:dependency_proxy_ttl_group_policy_worker_capacity) } diff --git a/spec/models/ci/job_artifact_spec.rb b/spec/models/ci/job_artifact_spec.rb index bc9c073d78b..2e8c41b410a 100644 --- a/spec/models/ci/job_artifact_spec.rb +++ b/spec/models/ci/job_artifact_spec.rb @@ -545,20 +545,8 @@ RSpec.describe Ci::JobArtifact do context 'when the artifact is a trace' do let(:file_type) { :trace } - context 'when ci_store_trace_outside_transaction is enabled' do - it 'returns true' do - expect(artifact.store_after_commit?).to be_truthy - end - end - - context 'when ci_store_trace_outside_transaction is disabled' do - before do - stub_feature_flags(ci_store_trace_outside_transaction: false) - end - - it 'returns false' do - expect(artifact.store_after_commit?).to be_falsey - end + it 'returns true' do + expect(artifact.store_after_commit?).to be_truthy end end diff --git a/spec/models/ci/pipeline_artifact_spec.rb b/spec/models/ci/pipeline_artifact_spec.rb index f65483d2290..801505f0231 100644 --- a/spec/models/ci/pipeline_artifact_spec.rb +++ b/spec/models/ci/pipeline_artifact_spec.rb @@ -215,4 +215,11 @@ RSpec.describe Ci::PipelineArtifact, type: :model do end end end + + context 'loose foreign key on ci_pipeline_artifacts.project_id' do + it_behaves_like 'cleanup by a loose foreign key' do + let!(:parent) { create(:project) } + let!(:model) { create(:ci_pipeline_artifact, project: parent) } + end + end end diff --git a/spec/models/container_repository_spec.rb b/spec/models/container_repository_spec.rb index 51fdbfebd3a..8f7c13d7ae6 100644 --- a/spec/models/container_repository_spec.rb +++ b/spec/models/container_repository_spec.rb @@ -25,12 +25,20 @@ RSpec.describe ContainerRepository do headers: { 'Content-Type' => 'application/json' }) end + it_behaves_like 'having unique enum values' + describe 'associations' do it 'belongs to the project' do expect(repository).to belong_to(:project) end end + describe 'validations' do + it { is_expected.to validate_presence_of(:migration_retries_count) } + it { is_expected.to validate_numericality_of(:migration_retries_count).is_greater_than_or_equal_to(0) } + it { is_expected.to validate_presence_of(:migration_state) } + end + describe '#tag' do it 'has a test tag' do expect(repository.tag('test')).not_to be_nil diff --git a/spec/models/experiment_spec.rb b/spec/models/experiment_spec.rb index ea5d2b27028..de6ce3ba053 100644 --- a/spec/models/experiment_spec.rb +++ b/spec/models/experiment_spec.rb @@ -235,6 +235,54 @@ RSpec.describe Experiment do end end + describe '#record_conversion_event_for_subject' do + let_it_be(:user) { create(:user) } + let_it_be(:experiment) { create(:experiment) } + let_it_be(:context) { { a: 42 } } + + subject(:record_conversion) { experiment.record_conversion_event_for_subject(user, context) } + + context 'when no existing experiment_subject record exists for the given user' do + it 'does not update or create an experiment_subject record' do + expect { record_conversion }.not_to change { ExperimentSubject.all.to_a } + end + end + + context 'when an existing experiment_subject exists for the given user' do + context 'but it has already been converted' do + let(:experiment_subject) { create(:experiment_subject, experiment: experiment, user: user, converted_at: 2.days.ago) } + + it 'does not update the converted_at value' do + expect { record_conversion }.not_to change { experiment_subject.converted_at } + end + end + + context 'and it has not yet been converted' do + let(:experiment_subject) { create(:experiment_subject, experiment: experiment, user: user) } + + it 'updates the converted_at value' do + expect { record_conversion }.to change { experiment_subject.reload.converted_at } + end + end + + context 'with no existing context' do + let(:experiment_subject) { create(:experiment_subject, experiment: experiment, user: user) } + + it 'updates the context' do + expect { record_conversion }.to change { experiment_subject.reload.context }.to('a' => 42) + end + end + + context 'with an existing context' do + let(:experiment_subject) { create(:experiment_subject, experiment: experiment, user: user, converted_at: 2.days.ago, context: { b: 1 } ) } + + it 'merges the context' do + expect { record_conversion }.to change { experiment_subject.reload.context }.to('a' => 42, 'b' => 1) + end + end + end + end + describe '#record_subject_and_variant!' do let_it_be(:subject_to_record) { create(:group) } let_it_be(:variant) { :control } diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 6593461f807..ac2474ac393 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -83,6 +83,9 @@ RSpec.describe User do it { is_expected.to delegate_method(:registration_objective).to(:user_detail).allow_nil } it { is_expected.to delegate_method(:registration_objective=).to(:user_detail).with_arguments(:args).allow_nil } + + it { is_expected.to delegate_method(:requires_credit_card_verification).to(:user_detail).allow_nil } + it { is_expected.to delegate_method(:requires_credit_card_verification=).to(:user_detail).with_arguments(:args).allow_nil } end describe 'associations' do diff --git a/spec/services/users/upsert_credit_card_validation_service_spec.rb b/spec/services/users/upsert_credit_card_validation_service_spec.rb index 952d482f1bd..ac7e619612f 100644 --- a/spec/services/users/upsert_credit_card_validation_service_spec.rb +++ b/spec/services/users/upsert_credit_card_validation_service_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe Users::UpsertCreditCardValidationService do - let_it_be(:user) { create(:user) } + let_it_be(:user) { create(:user, requires_credit_card_verification: true) } let(:user_id) { user.id } let(:credit_card_validated_time) { Time.utc(2020, 1, 1) } @@ -21,7 +21,7 @@ RSpec.describe Users::UpsertCreditCardValidationService do end describe '#execute' do - subject(:service) { described_class.new(params) } + subject(:service) { described_class.new(params, user) } context 'successfully set credit card validation record for the user' do context 'when user does not have credit card validation record' do @@ -42,6 +42,10 @@ RSpec.describe Users::UpsertCreditCardValidationService do expiration_date: Date.new(expiration_year, 1, 31) ) end + + it 'sets the requires_credit_card_verification attribute on the user to false' do + expect { service.execute }.to change { user.reload.requires_credit_card_verification }.to(false) + end end context 'when user has credit card validation record' do diff --git a/spec/views/projects/services/_form.haml_spec.rb b/spec/views/projects/services/_form.haml_spec.rb index 177f703ba6c..f212fd78b1a 100644 --- a/spec/views/projects/services/_form.haml_spec.rb +++ b/spec/views/projects/services/_form.haml_spec.rb @@ -20,13 +20,33 @@ RSpec.describe 'projects/services/_form' do ) end - context 'commit_events and merge_request_events' do - it 'display merge_request_events and commit_events descriptions' do - allow(Integrations::Redmine).to receive(:supported_events).and_return(%w(commit merge_request)) - + context 'integrations form' do + it 'does not render form element' do render - expect(rendered).to have_css("input[name='redirect_to'][value='/services']", count: 1, visible: false) + expect(rendered).not_to have_selector('[data-testid="integration-form"]') + end + + context 'when vue_integration_form feature flag is disabled' do + before do + stub_feature_flags(vue_integration_form: false) + end + + it 'renders form element' do + render + + expect(rendered).to have_selector('[data-testid="integration-form"]') + end + + context 'commit_events and merge_request_events' do + it 'display merge_request_events and commit_events descriptions' do + allow(Integrations::Redmine).to receive(:supported_events).and_return(%w(commit merge_request)) + + render + + expect(rendered).to have_css("input[name='redirect_to'][value='/services']", count: 1, visible: false) + end + end end end end diff --git a/spec/workers/packages/cleanup_package_file_worker_spec.rb b/spec/workers/packages/cleanup_package_file_worker_spec.rb index 4dd06f13aff..b423c4d3f06 100644 --- a/spec/workers/packages/cleanup_package_file_worker_spec.rb +++ b/spec/workers/packages/cleanup_package_file_worker_spec.rb @@ -20,10 +20,7 @@ RSpec.describe Packages::CleanupPackageFileWorker do let_it_be(:package_file3) { create(:package_file, :pending_destruction, package: package, updated_at: 1.year.ago, created_at: 1.year.ago) } it 'deletes the oldest package file pending destruction based on id', :aggregate_failures do - # NOTE: The worker doesn't explicitly look for the lower id value, but this is how PostgreSQL works when - # using LIMIT without ORDER BY. - expect(worker).to receive(:log_extra_metadata_on_done).with(:package_file_id, package_file2.id) - expect(worker).to receive(:log_extra_metadata_on_done).with(:package_id, package.id) + expect(worker).to receive(:log_extra_metadata_on_done).twice expect { subject }.to change { Packages::PackageFile.count }.by(-1) end