From c15582526dd15f5ea8fac0906624cc5cd2db03ab Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Wed, 26 Oct 2022 12:11:43 +0000 Subject: Add latest changes from gitlab-org/gitlab@master --- .../javascripts/dirty_submit/dirty_submit_form.js | 2 - .../utils/confirm_via_gl_modal/confirm_action.js | 57 ++++++++++++ .../confirm_via_gl_modal/confirm_via_gl_modal.js | 78 +++------------- app/assets/stylesheets/framework/layout.scss | 2 +- app/assets/stylesheets/startup/startup-dark.scss | 15 +++ .../stylesheets/startup/startup-general.scss | 15 +++ app/controllers/profiles/passwords_controller.rb | 2 + app/controllers/registrations_controller.rb | 9 -- app/helpers/application_helper.rb | 14 +-- app/policies/user_policy.rb | 1 + app/services/groups/create_service.rb | 11 +-- app/views/layouts/header/_default.html.haml | 41 ++------ app/views/shared/issuable/_form.html.haml | 5 +- app/workers/loose_foreign_keys/cleanup_worker.rb | 6 +- app/workers/onboarding/issue_created_worker.rb | 3 - app/workers/onboarding/pipeline_created_worker.rb | 3 - app/workers/onboarding/progress_worker.rb | 3 - app/workers/onboarding/user_added_worker.rb | 3 - .../experiment/logged_out_marketing_header.yml | 8 -- ...914_create_routing_table_for_builds_metadata.rb | 27 +----- ..._create_routing_table_for_builds_metadata_v2.rb | 41 ++++++++ db/schema_migrations/20221021145820 | 1 + doc/api/users.md | 27 ++++++ doc/development/api_graphql_styleguide.md | 9 +- lib/api/entities/user_associations_count.rb | 23 +++++ lib/api/helpers/users_helpers.rb | 7 ++ lib/api/users.rb | 47 ++++++++-- .../convert_table_to_first_list_partition.rb | 31 ++++++- .../table_management_helpers.rb | 5 +- lib/gitlab/sidekiq_migrate_jobs.rb | 6 +- locale/gitlab.pot | 13 ++- spec/controllers/registrations_controller_spec.rb | 22 ----- spec/features/user_sees_marketing_header_spec.rb | 52 ++--------- .../environments/environment_actions_spec.js | 6 +- .../confirm_via_gl_modal/confirm_action_spec.js | 103 +++++++++++++++++++++ .../confirm_via_gl_modal_spec.js | 80 ++++++++++++++++ spec/frontend/pipelines/pipelines_actions_spec.js | 6 +- .../deployment/deployment_actions_spec.js | 6 +- .../components/work_item_description_spec.js | 6 +- spec/helpers/application_helper_spec.rb | 42 +++------ .../convert_table_to_first_list_partition_spec.rb | 14 ++- .../table_management_helpers_spec.rb | 9 +- ...te_routing_table_for_builds_metadata_v2_spec.rb | 36 +++++++ spec/policies/user_policy_spec.rb | 44 +++++++++ spec/requests/api/users_spec.rb | 68 ++++++++++++++ spec/services/groups/create_service_spec.rb | 29 ------ 46 files changed, 685 insertions(+), 353 deletions(-) create mode 100644 app/assets/javascripts/lib/utils/confirm_via_gl_modal/confirm_action.js delete mode 100644 config/feature_flags/experiment/logged_out_marketing_header.yml create mode 100644 db/post_migrate/20221021145820_create_routing_table_for_builds_metadata_v2.rb create mode 100644 db/schema_migrations/20221021145820 create mode 100644 lib/api/entities/user_associations_count.rb create mode 100644 spec/frontend/lib/utils/confirm_via_gl_modal/confirm_action_spec.js create mode 100644 spec/frontend/lib/utils/confirm_via_gl_modal/confirm_via_gl_modal_spec.js create mode 100644 spec/migrations/20221021145820_create_routing_table_for_builds_metadata_v2_spec.rb diff --git a/app/assets/javascripts/dirty_submit/dirty_submit_form.js b/app/assets/javascripts/dirty_submit/dirty_submit_form.js index 83dd4b0a124..941c42f75eb 100644 --- a/app/assets/javascripts/dirty_submit/dirty_submit_form.js +++ b/app/assets/javascripts/dirty_submit/dirty_submit_form.js @@ -1,4 +1,3 @@ -import $ from 'jquery'; import { memoize, throttle } from 'lodash'; import createEventHub from '~/helpers/event_hub_factory'; @@ -34,7 +33,6 @@ class DirtySubmitForm { this.form.addEventListener('input', throttledUpdateDirtyInput); this.form.addEventListener('change', throttledUpdateDirtyInput); - $(this.form).on('change.select2', throttledUpdateDirtyInput); this.form.addEventListener('submit', (event) => this.formSubmit(event)); } diff --git a/app/assets/javascripts/lib/utils/confirm_via_gl_modal/confirm_action.js b/app/assets/javascripts/lib/utils/confirm_via_gl_modal/confirm_action.js new file mode 100644 index 00000000000..3bfbfea7f22 --- /dev/null +++ b/app/assets/javascripts/lib/utils/confirm_via_gl_modal/confirm_action.js @@ -0,0 +1,57 @@ +import Vue from 'vue'; + +export function confirmAction( + message, + { + primaryBtnVariant, + primaryBtnText, + secondaryBtnVariant, + secondaryBtnText, + cancelBtnVariant, + cancelBtnText, + modalHtmlMessage, + title, + hideCancel, + } = {}, +) { + return new Promise((resolve) => { + let confirmed = false; + let component; + + const ConfirmAction = Vue.extend({ + components: { + ConfirmModal: () => import('./confirm_modal.vue'), + }, + render(h) { + return h( + 'confirm-modal', + { + props: { + secondaryText: secondaryBtnText, + secondaryVariant: secondaryBtnVariant, + primaryVariant: primaryBtnVariant, + primaryText: primaryBtnText, + cancelVariant: cancelBtnVariant, + cancelText: cancelBtnText, + title, + modalHtmlMessage, + hideCancel, + }, + on: { + confirmed() { + confirmed = true; + }, + closed() { + component.$destroy(); + resolve(confirmed); + }, + }, + }, + [message], + ); + }, + }); + + component = new ConfirmAction().$mount(); + }); +} diff --git a/app/assets/javascripts/lib/utils/confirm_via_gl_modal/confirm_via_gl_modal.js b/app/assets/javascripts/lib/utils/confirm_via_gl_modal/confirm_via_gl_modal.js index 2dc479db80a..0e959e899e9 100644 --- a/app/assets/javascripts/lib/utils/confirm_via_gl_modal/confirm_via_gl_modal.js +++ b/app/assets/javascripts/lib/utils/confirm_via_gl_modal/confirm_via_gl_modal.js @@ -1,75 +1,21 @@ -import Vue from 'vue'; +import { confirmAction } from './confirm_action'; -export function confirmAction( - message, - { - primaryBtnVariant, - primaryBtnText, - secondaryBtnVariant, - secondaryBtnText, - cancelBtnVariant, - cancelBtnText, - modalHtmlMessage, - title, - hideCancel, - } = {}, -) { - return new Promise((resolve) => { - let confirmed = false; - - const component = new Vue({ - components: { - ConfirmModal: () => import('./confirm_modal.vue'), - }, - render(h) { - return h( - 'confirm-modal', - { - props: { - secondaryText: secondaryBtnText, - secondaryVariant: secondaryBtnVariant, - primaryVariant: primaryBtnVariant, - primaryText: primaryBtnText, - cancelVariant: cancelBtnVariant, - cancelText: cancelBtnText, - title, - modalHtmlMessage, - hideCancel, - }, - on: { - confirmed() { - confirmed = true; - }, - closed() { - component.$destroy(); - resolve(confirmed); - }, - }, - }, - [message], - ); - }, - }).$mount(); - }); -} - -export function confirmViaGlModal(message, element) { - const primaryBtnConfig = {}; - - const { confirmBtnVariant } = element.dataset; - - if (confirmBtnVariant) { - primaryBtnConfig.primaryBtnVariant = confirmBtnVariant; - } +function confirmViaGlModal(message, element) { + const { confirmBtnVariant, title, isHtmlMessage } = element.dataset; const screenReaderText = element.querySelector('.gl-sr-only')?.textContent || element.querySelector('.sr-only')?.textContent || element.getAttribute('aria-label'); - if (screenReaderText) { - primaryBtnConfig.primaryBtnText = screenReaderText; - } + const config = { + ...(screenReaderText && { primaryBtnText: screenReaderText }), + ...(confirmBtnVariant && { primaryBtnVariant: confirmBtnVariant }), + ...(title && { title }), + ...(isHtmlMessage && { modalHtmlMessage: message }), + }; - return confirmAction(message, primaryBtnConfig); + return confirmAction(message, config); } + +export { confirmAction, confirmViaGlModal }; diff --git a/app/assets/stylesheets/framework/layout.scss b/app/assets/stylesheets/framework/layout.scss index 02b76b89482..7a92adf7b7b 100644 --- a/app/assets/stylesheets/framework/layout.scss +++ b/app/assets/stylesheets/framework/layout.scss @@ -207,7 +207,7 @@ body { } @include media-breakpoint-up(sm) { - .logged-out-marketing-header-candidate { + .logged-out-marketing-header { --header-height: 72px; } } diff --git a/app/assets/stylesheets/startup/startup-dark.scss b/app/assets/stylesheets/startup/startup-dark.scss index e6f8c6a5676..687b466ab76 100644 --- a/app/assets/stylesheets/startup/startup-dark.scss +++ b/app/assets/stylesheets/startup/startup-dark.scss @@ -603,6 +603,11 @@ svg { html { overflow-y: scroll; } +@media (min-width: 576px) { + .logged-out-marketing-header { + --header-height: 72px; + } +} .btn { border-radius: 4px; font-size: 0.875rem; @@ -2054,9 +2059,19 @@ body.gl-dark { .gl-display-none { display: none; } +@media (min-width: 576px) { + .gl-sm-display-none { + display: none; + } +} .gl-display-flex { display: flex; } +@media (min-width: 992px) { + .gl-lg-display-flex { + display: flex; + } +} @media (min-width: 576px) { .gl-sm-display-block { display: block; diff --git a/app/assets/stylesheets/startup/startup-general.scss b/app/assets/stylesheets/startup/startup-general.scss index b8079c274f1..802262ba346 100644 --- a/app/assets/stylesheets/startup/startup-general.scss +++ b/app/assets/stylesheets/startup/startup-general.scss @@ -584,6 +584,11 @@ svg { html { overflow-y: scroll; } +@media (min-width: 576px) { + .logged-out-marketing-header { + --header-height: 72px; + } +} .btn { border-radius: 4px; font-size: 0.875rem; @@ -1700,9 +1705,19 @@ svg.s16 { .gl-display-none { display: none; } +@media (min-width: 576px) { + .gl-sm-display-none { + display: none; + } +} .gl-display-flex { display: flex; } +@media (min-width: 992px) { + .gl-lg-display-flex { + display: flex; + } +} @media (min-width: 576px) { .gl-sm-display-block { display: block; diff --git a/app/controllers/profiles/passwords_controller.rb b/app/controllers/profiles/passwords_controller.rb index 5eb0f80ddc9..09f91d036d0 100644 --- a/app/controllers/profiles/passwords_controller.rb +++ b/app/controllers/profiles/passwords_controller.rb @@ -94,3 +94,5 @@ class Profiles::PasswordsController < Profiles::ApplicationController } end end + +Profiles::PasswordsController.prepend_mod diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb index 31fe30f3f06..aa35dcbf486 100644 --- a/app/controllers/registrations_controller.rb +++ b/app/controllers/registrations_controller.rb @@ -40,7 +40,6 @@ class RegistrationsController < Devise::RegistrationsController persist_accepted_terms_if_required(new_user) set_role_required(new_user) - track_experiment_event(new_user) send_custom_confirmation_instructions(new_user, token) if pending_approval? @@ -239,14 +238,6 @@ class RegistrationsController < Devise::RegistrationsController current_user end - def track_experiment_event(new_user) - # Track signed up event to relate it with click "Sign up" button events from - # the experimental logged out header with marketing links. This allows us to - # have a funnel of visitors clicking on the header and those visitors - # signing up and becoming users - experiment(:logged_out_marketing_header, actor: new_user).track(:signed_up) if new_user.persisted? - end - def identity_verification_redirect_path # overridden by EE module end diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 32af1599bd1..bc640297834 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -315,7 +315,7 @@ module ApplicationHelper class_names << 'epic-boards-page gl-overflow-auto' if current_controller?(:epic_boards) class_names << 'with-performance-bar' if performance_bar_enabled? class_names << system_message_class - class_names << marketing_header_experiment_class + class_names << 'logged-out-marketing-header' unless current_user class_names end @@ -456,18 +456,6 @@ module ApplicationHelper def appearance ::Appearance.current end - - def marketing_header_experiment_class - return if current_user - - experiment(:logged_out_marketing_header, actor: nil) do |e| - html_class = 'logged-out-marketing-header-candidate' - e.candidate { html_class } - e.variant(:trial_focused) { html_class } - e.control {} - e.run - end - end end ApplicationHelper.prepend_mod diff --git a/app/policies/user_policy.rb b/app/policies/user_policy.rb index f62ccef826c..4f3dafbf5c8 100644 --- a/app/policies/user_policy.rb +++ b/app/policies/user_policy.rb @@ -36,6 +36,7 @@ class UserPolicy < BasePolicy rule { (private_profile | blocked_user | unconfirmed_user) & ~(user_is_self | admin) }.prevent :read_user_profile rule { user_is_self | admin }.enable :disable_two_factor rule { (user_is_self | admin) & ~blocked }.enable :create_user_personal_access_token + rule { (user_is_self | admin) & ~blocked }.enable :get_user_associations_count end UserPolicy.prepend_mod_with('UserPolicy') diff --git a/app/services/groups/create_service.rb b/app/services/groups/create_service.rb index d508865ef32..68bb6427350 100644 --- a/app/services/groups/create_service.rb +++ b/app/services/groups/create_service.rb @@ -57,7 +57,7 @@ module Groups end def after_create_hook - track_experiment_event + # overridden in EE end def remove_unallowed_params @@ -109,15 +109,6 @@ module Groups @group.shared_runners_enabled = @group.parent.shared_runners_enabled @group.allow_descendants_override_disabled_shared_runners = @group.parent.allow_descendants_override_disabled_shared_runners end - - def track_experiment_event - return unless group.persisted? - - # Track namespace created events to relate them with signed up events for - # the same experiment. This will let us associate created namespaces to - # users that signed up from the experimental logged out header. - experiment(:logged_out_marketing_header, actor: current_user).track(:namespace_created, namespace: group) - end end end diff --git a/app/views/layouts/header/_default.html.haml b/app/views/layouts/header/_default.html.haml index bbcfb9c1565..10e38ab63f7 100644 --- a/app/views/layouts/header/_default.html.haml +++ b/app/views/layouts/header/_default.html.haml @@ -19,14 +19,7 @@ .gl-display-none.gl-sm-display-block = render "layouts/nav/top_nav" - else - - experiment(:logged_out_marketing_header, actor: nil) do |e| - - e.candidate do - = render 'layouts/header/marketing_links' - - e.try(:trial_focused) do - = render 'layouts/header/marketing_links' - - e.control do - .gl-display-none.gl-sm-display-block - = render "layouts/nav/top_nav" + = render 'layouts/header/marketing_links' - if top_nav_show_search .navbar-collapse.gl-transition-medium.collapse.gl-mr-auto.global-search-container.hide-when-top-nav-responsive-open @@ -110,14 +103,8 @@ .dropdown-menu.dropdown-menu-right = render 'layouts/header/help_dropdown' - unless current_user - - experiment(:logged_out_marketing_header, actor: nil) do |e| - - e.candidate do - %li.nav-item.gl-display-none.gl-sm-display-block - = render "layouts/nav/top_nav" - - e.try(:trial_focused) do - %li.nav-item.gl-display-none.gl-sm-display-block - = render "layouts/nav/top_nav" - - e.control {} + %li.nav-item.gl-display-none.gl-sm-display-block + = render "layouts/nav/top_nav" - if header_link?(:user_dropdown) %li.nav-item.header-user.js-nav-user-dropdown.dropdown{ data: { track_label: "profile_dropdown", track_action: "click_dropdown", track_value: "", qa_selector: 'user_menu', testid: 'user-menu' }, class: ('mr-0' if has_impersonation_link) } = link_to current_user, class: user_dropdown_class, data: { toggle: "dropdown" } do @@ -131,23 +118,11 @@ = link_to admin_impersonation_path, class: 'nav-link impersonation-btn', method: :delete, title: _('Stop impersonation'), aria: { label: _('Stop impersonation') }, data: { toggle: 'tooltip', placement: 'bottom', container: 'body', qa_selector: 'stop_impersonation_link' } do = sprite_icon('incognito', size: 18) - if header_link?(:sign_in) - - experiment(:logged_out_marketing_header, actor: nil) do |e| - - e.candidate do - %li.nav-item.gl-display-none.gl-sm-display-block - = link_to _('Sign up now'), new_user_registration_path, class: 'gl-button btn btn-default btn-sign-in', data: { track_action: 'click_button', track_experiment: e.name, track_label: 'sign_up_now' } - %li.nav-item.gl-display-none.gl-sm-display-block - = link_to _('Login'), new_session_path(:user, redirect_to_referer: 'yes') - = render 'layouts/header/sign_in_register_button', class: 'gl-sm-display-none' - - e.try(:trial_focused) do - %li.nav-item.gl-display-none.gl-sm-display-block - = link_to _('Get a free trial'), 'https://about.gitlab.com/free-trial/', class: 'gl-button btn btn-default btn-sign-in', data: { track_action: 'click_button', track_experiment: e.name, track_label: 'get_a_free_trial' } - %li.nav-item.gl-display-none.gl-sm-display-block - = link_to _('Sign up'), new_user_registration_path, data: { track_action: 'click_button', track_experiment: e.name, track_label: 'sign_up' } - %li.nav-item.gl-display-none.gl-sm-display-block - = link_to _('Login'), new_session_path(:user, redirect_to_referer: 'yes') - = render 'layouts/header/sign_in_register_button', class: 'gl-sm-display-none' - - e.control do - = render 'layouts/header/sign_in_register_button' + %li.nav-item.gl-display-none.gl-sm-display-block + = link_to _('Sign up now'), new_user_registration_path, class: 'gl-button btn btn-default btn-sign-in' + %li.nav-item.gl-display-none.gl-sm-display-block + = link_to _('Login'), new_session_path(:user, redirect_to_referer: 'yes') + = render 'layouts/header/sign_in_register_button', class: 'gl-sm-display-none' %button.navbar-toggler.d-block.d-sm-none{ type: 'button', class: 'gl-border-none!', data: { testid: 'top-nav-responsive-toggle', qa_selector: 'mobile_navbar_button' } } %span.sr-only= _('Toggle navigation') diff --git a/app/views/shared/issuable/_form.html.haml b/app/views/shared/issuable/_form.html.haml index 5b7f9c4226c..a325ad5f447 100644 --- a/app/views/shared/issuable/_form.html.haml +++ b/app/views/shared/issuable/_form.html.haml @@ -71,7 +71,10 @@ - else = link_to _('Cancel'), polymorphic_path([@project, issuable]), class: 'gl-button btn btn-default js-reset-autosave' - if can?(current_user, :"destroy_#{issuable.to_ability_name}", @project) - = link_to 'Delete', polymorphic_path([@project, issuable], params: { destroy_confirm: true }), data: { confirm: _('%{issuableType} will be removed! Are you sure?') % { issuableType: issuable.human_class_name } }, method: :delete, class: 'btn gl-button btn-danger btn-danger-secondary gl-float-right js-reset-autosave' + - confirm_title = _('Delete %{issuableType}?') % { issuableType: issuable.human_class_name } + - confirm_body = _('You’re about to permanently delete the %{issuableType} ‘%{strongOpen}%{issuableTitle}%{strongClose}’. To avoid data loss, consider %{strongOpen}closing this %{issuableType}%{strongClose} instead. Once deleted, it cannot be undone or recovered.') % { issuableType: issuable.human_class_name, issuableTitle: issuable.title, strongOpen: '', strongClose: '' } + - confirm_primary_btn_text = _('Delete %{issuableType}') % { issuableType: issuable.human_class_name } + = link_to _('Delete'), polymorphic_path([@project, issuable], params: { destroy_confirm: true }), data: { title: confirm_title, confirm: confirm_body, is_html_message: true, confirm_btn_variant: 'danger'}, method: :delete, class: 'btn gl-button btn-danger btn-danger-secondary gl-float-right js-reset-autosave', "aria-label": confirm_primary_btn_text - if issuable.respond_to?(:issue_type) = form.hidden_field :issue_type diff --git a/app/workers/loose_foreign_keys/cleanup_worker.rb b/app/workers/loose_foreign_keys/cleanup_worker.rb index 0a3a834578a..9a0909598bb 100644 --- a/app/workers/loose_foreign_keys/cleanup_worker.rb +++ b/app/workers/loose_foreign_keys/cleanup_worker.rb @@ -12,7 +12,11 @@ module LooseForeignKeys idempotent! def perform - in_lock(self.class.name.underscore, ttl: ModificationTracker::MAX_RUNTIME, retries: 0) do + # Add small buffer on MAX_RUNTIME to account for single long running + # query or extra worker time after the cleanup. + lock_ttl = ModificationTracker::MAX_RUNTIME + 20.seconds + + in_lock(self.class.name.underscore, ttl: lock_ttl, retries: 0) do stats = {} connection_name, base_model = current_connection_name_and_base_model diff --git a/app/workers/onboarding/issue_created_worker.rb b/app/workers/onboarding/issue_created_worker.rb index ff39fefad81..73e96850786 100644 --- a/app/workers/onboarding/issue_created_worker.rb +++ b/app/workers/onboarding/issue_created_worker.rb @@ -22,6 +22,3 @@ module Onboarding end end end - -# remove in %15.6 as per https://gitlab.com/gitlab-org/gitlab/-/issues/372432 -Namespaces::OnboardingIssueCreatedWorker = Onboarding::IssueCreatedWorker diff --git a/app/workers/onboarding/pipeline_created_worker.rb b/app/workers/onboarding/pipeline_created_worker.rb index 6bd5863b0e0..c6e84882d6f 100644 --- a/app/workers/onboarding/pipeline_created_worker.rb +++ b/app/workers/onboarding/pipeline_created_worker.rb @@ -22,6 +22,3 @@ module Onboarding end end end - -# remove in %15.6 as per https://gitlab.com/gitlab-org/gitlab/-/issues/372432 -Namespaces::OnboardingPipelineCreatedWorker = Onboarding::PipelineCreatedWorker diff --git a/app/workers/onboarding/progress_worker.rb b/app/workers/onboarding/progress_worker.rb index 525934c4a7c..34503bfa451 100644 --- a/app/workers/onboarding/progress_worker.rb +++ b/app/workers/onboarding/progress_worker.rb @@ -23,6 +23,3 @@ module Onboarding end end end - -# remove in %15.6 as per https://gitlab.com/gitlab-org/gitlab/-/issues/372432 -Namespaces::OnboardingProgressWorker = Onboarding::ProgressWorker diff --git a/app/workers/onboarding/user_added_worker.rb b/app/workers/onboarding/user_added_worker.rb index 38e9cd063ea..b096bf752dc 100644 --- a/app/workers/onboarding/user_added_worker.rb +++ b/app/workers/onboarding/user_added_worker.rb @@ -19,6 +19,3 @@ module Onboarding end end end - -# remove in %15.6 as per https://gitlab.com/gitlab-org/gitlab/-/issues/372432 -Namespaces::OnboardingUserAddedWorker = Onboarding::UserAddedWorker diff --git a/config/feature_flags/experiment/logged_out_marketing_header.yml b/config/feature_flags/experiment/logged_out_marketing_header.yml deleted file mode 100644 index 8bc09d59b16..00000000000 --- a/config/feature_flags/experiment/logged_out_marketing_header.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: logged_out_marketing_header -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/76076 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/348525 -milestone: '14.7' -type: experiment -group: group::activation -default_enabled: false diff --git a/db/post_migrate/20221004074914_create_routing_table_for_builds_metadata.rb b/db/post_migrate/20221004074914_create_routing_table_for_builds_metadata.rb index 71a8625dce2..a792fc91d3d 100644 --- a/db/post_migrate/20221004074914_create_routing_table_for_builds_metadata.rb +++ b/db/post_migrate/20221004074914_create_routing_table_for_builds_metadata.rb @@ -1,30 +1,7 @@ # frozen_string_literal: true class CreateRoutingTableForBuildsMetadata < Gitlab::Database::Migration[2.0] - include Gitlab::Database::PartitioningMigrationHelpers::TableManagementHelpers + def up; end - disable_ddl_transaction! - - TABLE_NAME = :ci_builds_metadata - PARENT_TABLE_NAME = :p_ci_builds_metadata - FIRST_PARTITION = 100 - PARTITION_COLUMN = :partition_id - - def up - convert_table_to_first_list_partition( - table_name: TABLE_NAME, - partitioning_column: PARTITION_COLUMN, - parent_table_name: PARENT_TABLE_NAME, - initial_partitioning_value: FIRST_PARTITION - ) - end - - def down - revert_converting_table_to_first_list_partition( - table_name: TABLE_NAME, - partitioning_column: PARTITION_COLUMN, - parent_table_name: PARENT_TABLE_NAME, - initial_partitioning_value: FIRST_PARTITION - ) - end + def down; end end diff --git a/db/post_migrate/20221021145820_create_routing_table_for_builds_metadata_v2.rb b/db/post_migrate/20221021145820_create_routing_table_for_builds_metadata_v2.rb new file mode 100644 index 00000000000..e5f1ba5cb87 --- /dev/null +++ b/db/post_migrate/20221021145820_create_routing_table_for_builds_metadata_v2.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +class CreateRoutingTableForBuildsMetadataV2 < Gitlab::Database::Migration[2.0] + include Gitlab::Database::PartitioningMigrationHelpers::TableManagementHelpers + + disable_ddl_transaction! + + TABLE_NAME = :ci_builds_metadata + PARENT_TABLE_NAME = :p_ci_builds_metadata + FIRST_PARTITION = 100 + PARTITION_COLUMN = :partition_id + + def up + return if connection.table_exists?(PARENT_TABLE_NAME) && partition_attached? + + convert_table_to_first_list_partition( + table_name: TABLE_NAME, + partitioning_column: PARTITION_COLUMN, + parent_table_name: PARENT_TABLE_NAME, + initial_partitioning_value: FIRST_PARTITION, + lock_tables: [:ci_builds, :ci_builds_metadata] + ) + end + + def down + revert_converting_table_to_first_list_partition( + table_name: TABLE_NAME, + partitioning_column: PARTITION_COLUMN, + parent_table_name: PARENT_TABLE_NAME, + initial_partitioning_value: FIRST_PARTITION + ) + end + + private + + def partition_attached? + connection.select_value(<<~SQL) + SELECT true FROM postgres_partitions WHERE name = '#{TABLE_NAME}'; + SQL + end +end diff --git a/db/schema_migrations/20221021145820 b/db/schema_migrations/20221021145820 new file mode 100644 index 00000000000..e3d50c654ba --- /dev/null +++ b/db/schema_migrations/20221021145820 @@ -0,0 +1 @@ +e9fd4d60833624e20fcf9b01b883dca15e6c135aa99f1afd1c7a365eebac17fb \ No newline at end of file diff --git a/doc/api/users.md b/doc/api/users.md index f4aadfd33db..a605f776c9c 100644 --- a/doc/api/users.md +++ b/doc/api/users.md @@ -968,6 +968,33 @@ Example response: Please refer to the [List of user projects](projects.md#list-user-projects). +## List associations count for user + +Get a list of a specified user's count of projects, groups, issues and merge requests. + +Administrators can query any user, but non-administrators can only query themselves. + +```plaintext +GET /users/:id/associations_count +``` + +Parameters: + +| Attribute | Type | Required | Description | +|-----------|---------|----------|------------------| +| `id` | integer | yes | ID of a user | + +Example response: + +```json +{ + "groups_count": 2, + "projects_count": 3, + "issues_count": 8, + "merge_requests_count": 5 +} +``` + ## List SSH keys Get a list of currently authenticated user's SSH keys. diff --git a/doc/development/api_graphql_styleguide.md b/doc/development/api_graphql_styleguide.md index d3053a1ca5f..a6df26ff0d2 100644 --- a/doc/development/api_graphql_styleguide.md +++ b/doc/development/api_graphql_styleguide.md @@ -1733,15 +1733,18 @@ there are no problems we need to inform the user of. #### Failure (relevant to the user) -An error that affects the **user** occurred. We refer to these as _mutation errors_. In -this case there is typically no `thing` to return: +An error that affects the **user** occurred. We refer to these as _mutation errors_. + +In a _create_ mutation there is typically no `thing` to return. + +In an _update_ mutation we return the current true state of `thing`. Developers may need to call `#reset` on the `thing` instance to ensure this happens. ```javascript { data: { doTheThing: { errors: ["you cannot touch the thing"], - thing: null + thing: { .. } } } } diff --git a/lib/api/entities/user_associations_count.rb b/lib/api/entities/user_associations_count.rb new file mode 100644 index 00000000000..af744d2d49a --- /dev/null +++ b/lib/api/entities/user_associations_count.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module API + module Entities + class UserAssociationsCount < Grape::Entity + expose :groups_count do |user| + user.groups.size + end + + expose :projects_count do |user| + user.projects.size + end + + expose :issues_count do |user| + user.issues.size + end + + expose :merge_requests_count do |user| + user.merge_requests.size + end + end + end +end diff --git a/lib/api/helpers/users_helpers.rb b/lib/api/helpers/users_helpers.rb index 1a019283bc6..e80b89488a2 100644 --- a/lib/api/helpers/users_helpers.rb +++ b/lib/api/helpers/users_helpers.rb @@ -18,6 +18,13 @@ module API error_messages[:bio] = error_messages.delete(:"user_detail.bio") if error_messages.has_key?(:"user_detail.bio") end end + + # rubocop: disable CodeReuse/ActiveRecord + def find_user_by_id(params) + id = params[:user_id] || params[:id] + User.find_by(id: id) || not_found!('User') + end + # rubocop: enable CodeReuse/ActiveRecord end end end diff --git a/lib/api/users.rb b/lib/api/users.rb index 7f44e46f1ca..0ce7e56f919 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -8,9 +8,18 @@ module API allow_access_with_scope :read_user, if: -> (request) { request.get? || request.head? } - feature_category :users, ['/users/:id/custom_attributes', '/users/:id/custom_attributes/:key'] - - urgency :medium, ['/users/:id/custom_attributes', '/users/:id/custom_attributes/:key'] + feature_category :users, + %w[ + /users/:id/custom_attributes + /users/:id/custom_attributes/:key + /users/:id/associations_count + ] + + urgency :medium, + %w[ + /users/:id/custom_attributes + /users/:id/custom_attributes/:key + ] resource :users, requirements: { uid: /[0-9]*/, id: /[0-9]*/ } do include CustomAttributesEndpoints @@ -22,13 +31,6 @@ module API helpers Helpers::UsersHelpers helpers do - # rubocop: disable CodeReuse/ActiveRecord - def find_user_by_id(params) - id = params[:user_id] || params[:id] - User.find_by(id: id) || not_found!('User') - end - # rubocop: enable CodeReuse/ActiveRecord - # rubocop: disable CodeReuse/ActiveRecord def reorder_users(users) if params[:order_by] && params[:sort] @@ -75,6 +77,31 @@ module API end end + resources ':id/associations_count' do + helpers do + def present_entity(result) + present result, + with: ::API::Entities::UserAssociationsCount + end + end + + desc "Returns a list of a specified user's count of projects, groups, issues and merge requests." + params do + requires :id, + type: Integer, + desc: 'ID of the user to query.' + end + get do + authenticate! + + user = find_user_by_id(params) + forbidden! unless can?(current_user, :get_user_associations_count, user) + not_found!('User') unless user + + present_entity(user) + end + end + desc 'Get the list of users' do success Entities::UserBasic end diff --git a/lib/gitlab/database/partitioning/convert_table_to_first_list_partition.rb b/lib/gitlab/database/partitioning/convert_table_to_first_list_partition.rb index 23a8dc0b44f..58447481e60 100644 --- a/lib/gitlab/database/partitioning/convert_table_to_first_list_partition.rb +++ b/lib/gitlab/database/partitioning/convert_table_to_first_list_partition.rb @@ -10,13 +10,17 @@ module Gitlab attr_reader :partitioning_column, :table_name, :parent_table_name, :zero_partition_value - def initialize(migration_context:, table_name:, parent_table_name:, partitioning_column:, zero_partition_value:) + def initialize( + migration_context:, table_name:, parent_table_name:, partitioning_column:, + zero_partition_value:, lock_tables: []) + @migration_context = migration_context @connection = migration_context.connection @table_name = table_name @parent_table_name = parent_table_name @partitioning_column = partitioning_column @zero_partition_value = zero_partition_value + @lock_tables = Array.wrap(lock_tables) end def prepare_for_partitioning @@ -35,7 +39,12 @@ module Gitlab create_parent_table attach_foreign_keys_to_parent - migration_context.with_lock_retries(raise_on_exhaustion: true) do + lock_args = { + raise_on_exhaustion: true, + timing_configuration: lock_timing_configuration + } + + migration_context.with_lock_retries(**lock_args) do migration_context.execute(sql_to_convert_table) end end @@ -74,6 +83,7 @@ module Gitlab # but they acquire the same locks so it's much faster to incude them # here. [ + lock_tables_statement, attach_table_to_parent_statement, alter_sequence_statements(old_table: table_name, new_table: parent_table_name), remove_constraint_statement @@ -162,6 +172,16 @@ module Gitlab end end + def lock_tables_statement + return if @lock_tables.empty? + + table_names = @lock_tables.map { |name| quote_table_name(name) }.join(', ') + + <<~SQL + LOCK #{table_names} IN ACCESS EXCLUSIVE MODE + SQL + end + def attach_table_to_parent_statement <<~SQL ALTER TABLE #{quote_table_name(parent_table_name)} @@ -235,6 +255,13 @@ module Gitlab ALTER TABLE #{connection.quote_table_name(table_name)} OWNER TO CURRENT_USER SQL end + + def lock_timing_configuration + iterations = Gitlab::Database::WithLockRetries::DEFAULT_TIMING_CONFIGURATION + aggressive_iterations = Array.new(5) { [10.seconds, 1.minute] } + + iterations + aggressive_iterations + end end end end diff --git a/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers.rb b/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers.rb index 695a5d7ec77..f9790bf53b9 100644 --- a/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers.rb +++ b/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers.rb @@ -275,7 +275,7 @@ module Gitlab ).revert_preparation_for_partitioning end - def convert_table_to_first_list_partition(table_name:, partitioning_column:, parent_table_name:, initial_partitioning_value:) + def convert_table_to_first_list_partition(table_name:, partitioning_column:, parent_table_name:, initial_partitioning_value:, lock_tables: []) validate_not_in_transaction!(:convert_table_to_first_list_partition) Gitlab::Database::Partitioning::ConvertTableToFirstListPartition @@ -283,7 +283,8 @@ module Gitlab table_name: table_name, parent_table_name: parent_table_name, partitioning_column: partitioning_column, - zero_partition_value: initial_partitioning_value + zero_partition_value: initial_partitioning_value, + lock_tables: lock_tables ).partition end diff --git a/lib/gitlab/sidekiq_migrate_jobs.rb b/lib/gitlab/sidekiq_migrate_jobs.rb index 76fa3515253..8c1153d6112 100644 --- a/lib/gitlab/sidekiq_migrate_jobs.rb +++ b/lib/gitlab/sidekiq_migrate_jobs.rb @@ -59,7 +59,11 @@ module Gitlab routing_rules_queues = mappings.values.uniq logger&.info("List of queues based on routing rules: #{routing_rules_queues}") Sidekiq.redis do |conn| - conn.scan_each(match: "queue:*", type: 'list') do |key| + # Redis 6 supports conn.scan_each(match: "queue:*", type: 'list') + conn.scan_each(match: "queue:*") do |key| + # Redis 5 compatibility + next unless conn.type(key) == 'list' + queue_from = key.split(':', 2).last next if routing_rules_queues.include?(queue_from) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 78d7c34d036..a41ecd395b5 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -10692,10 +10692,10 @@ msgstr "" msgid "Contribution Analytics" msgstr "" -msgid "ContributionAnalytics|%{created_count} created, %{closed_count} closed." +msgid "ContributionAnalytics|%{created} created, %{closed} closed." msgstr "" -msgid "ContributionAnalytics|%{created_count} created, %{merged_count} merged, %{closed_count} closed." +msgid "ContributionAnalytics|%{created} created, %{merged} merged, %{closed} closed." msgstr "" msgid "ContributionAnalytics|%{pushes}, more than %{commits} by %{contributors}." @@ -12742,6 +12742,9 @@ msgstr "" msgid "Delete %{issuableType}" msgstr "" +msgid "Delete %{issuableType}?" +msgstr "" + msgid "Delete %{name}" msgstr "" @@ -17999,9 +18002,6 @@ msgstr "" msgid "Get a free instance review" msgstr "" -msgid "Get a free trial" -msgstr "" - msgid "Get a support subscription" msgstr "" @@ -47143,6 +47143,9 @@ msgstr "" msgid "Your username is %{username}." msgstr "" +msgid "You’re about to permanently delete the %{issuableType} ‘%{strongOpen}%{issuableTitle}%{strongClose}’. To avoid data loss, consider %{strongOpen}closing this %{issuableType}%{strongClose} instead. Once deleted, it cannot be undone or recovered." +msgstr "" + msgid "ZenTaoIntegration|Failed to load ZenTao issue. View the issue in ZenTao, or reload the page." msgstr "" diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb index 782350edfdb..ceb00660658 100644 --- a/spec/controllers/registrations_controller_spec.rb +++ b/spec/controllers/registrations_controller_spec.rb @@ -478,28 +478,6 @@ RSpec.describe RegistrationsController do subject end - describe 'logged_out_marketing_header experiment', :experiment do - before do - stub_experiments(logged_out_marketing_header: :candidate) - end - - it 'tracks signed_up event' do - expect(experiment(:logged_out_marketing_header)).to track(:signed_up).on_next_instance - - subject - end - - context 'when registration fails' do - let_it_be(:user_params) { { user: base_user_params.merge({ username: '' }) } } - - it 'does not track signed_up event' do - expect(experiment(:logged_out_marketing_header)).not_to track(:signed_up) - - subject - end - end - end - context 'when the password is weak' do render_views let_it_be(:new_user_params) { { new_user: base_user_params.merge({ password: "password" }) } } diff --git a/spec/features/user_sees_marketing_header_spec.rb b/spec/features/user_sees_marketing_header_spec.rb index 31f088ce010..eae964cec02 100644 --- a/spec/features/user_sees_marketing_header_spec.rb +++ b/spec/features/user_sees_marketing_header_spec.rb @@ -6,50 +6,14 @@ RSpec.describe 'User sees experimental lmarketing header' do let_it_be(:project) { create(:project, :public) } context 'when not logged in' do - context 'when experiment candidate' do - it 'shows marketing header links', :aggregate_failures do - stub_experiments(logged_out_marketing_header: :candidate) - - visit project_path(project) - - expect(page).to have_text "About GitLab" - expect(page).to have_text "Pricing" - expect(page).to have_text "Talk to an expert" - expect(page).to have_text "Sign up now" - expect(page).to have_text "Login" - end - end - - context 'when experiment candidate (trial focused variant)' do - it 'shows marketing header links', :aggregate_failures do - stub_experiments(logged_out_marketing_header: :trial_focused) - - visit project_path(project) - - expect(page).to have_text "About GitLab" - expect(page).to have_text "Pricing" - expect(page).to have_text "Talk to an expert" - expect(page).to have_text "Get a free trial" - expect(page).to have_text "Sign up" - expect(page).to have_text "Login" - end - end - - context 'when experiment control' do - it 'does not show marketing header links', :aggregate_failures do - stub_experiments(logged_out_marketing_header: :control) - - visit project_path(project) + it 'shows marketing header links', :aggregate_failures do + visit project_path(project) - expect(page).not_to have_text "About GitLab" - expect(page).not_to have_text "Pricing" - expect(page).not_to have_text "Talk to an expert" - expect(page).not_to have_text "Sign up now" - expect(page).not_to have_text "Login" - expect(page).not_to have_text "Get a free trial" - expect(page).not_to have_text "Sign up" - expect(page).to have_text "Sign in / Register" - end + expect(page).to have_text "About GitLab" + expect(page).to have_text "Pricing" + expect(page).to have_text "Talk to an expert" + expect(page).to have_text "Sign up now" + expect(page).to have_text "Login" end end @@ -57,8 +21,6 @@ RSpec.describe 'User sees experimental lmarketing header' do it 'does not show marketing header links', :aggregate_failures do sign_in(create(:user)) - stub_experiments(logged_out_marketing_header: :candidate) - visit project_path(project) expect(page).not_to have_text "About GitLab" diff --git a/spec/frontend/environments/environment_actions_spec.js b/spec/frontend/environments/environment_actions_spec.js index 68895b194a1..48483152f7a 100644 --- a/spec/frontend/environments/environment_actions_spec.js +++ b/spec/frontend/environments/environment_actions_spec.js @@ -10,11 +10,7 @@ import actionMutation from '~/environments/graphql/mutations/action.mutation.gra import { confirmAction } from '~/lib/utils/confirm_via_gl_modal/confirm_via_gl_modal'; import createMockApollo from 'helpers/mock_apollo_helper'; -jest.mock('~/lib/utils/confirm_via_gl_modal/confirm_via_gl_modal', () => { - return { - confirmAction: jest.fn(), - }; -}); +jest.mock('~/lib/utils/confirm_via_gl_modal/confirm_via_gl_modal'); const scheduledJobAction = { name: 'scheduled action', diff --git a/spec/frontend/lib/utils/confirm_via_gl_modal/confirm_action_spec.js b/spec/frontend/lib/utils/confirm_via_gl_modal/confirm_action_spec.js new file mode 100644 index 00000000000..142c76f7bc0 --- /dev/null +++ b/spec/frontend/lib/utils/confirm_via_gl_modal/confirm_action_spec.js @@ -0,0 +1,103 @@ +import Vue, { nextTick } from 'vue'; +import { createWrapper } from '@vue/test-utils'; +import { setHTMLFixture, resetHTMLFixture } from 'helpers/fixtures'; +import { confirmAction } from '~/lib/utils/confirm_via_gl_modal/confirm_action'; +import ConfirmModal from '~/lib/utils/confirm_via_gl_modal/confirm_modal.vue'; + +const originalMount = Vue.prototype.$mount; + +describe('confirmAction', () => { + let modalWrapper; + let confirActionPromise; + let modal; + + const findConfirmModal = () => modalWrapper.findComponent(ConfirmModal); + const renderRootComponent = async (message, opts) => { + confirActionPromise = confirmAction(message, opts); + // We have to wait for two ticks here. + // The first one is to wait for rendering of the root component + // The second one to wait for rendering of the dynamically + // loaded confirm-modal component + await nextTick(); + await nextTick(); + modal = findConfirmModal(); + }; + const mockMount = (vm, el) => { + originalMount.call(vm, el); + modalWrapper = createWrapper(vm); + return vm; + }; + + beforeEach(() => { + setHTMLFixture('
'); + const el = document.getElementById('component'); + // We mock the implementation only once to make sure that we mock + // it only for the root component in confirm_action. + // Mounting other components (like confirm-modal) should not be affected with + // this mock + jest.spyOn(Vue.prototype, '$mount').mockImplementationOnce(function mock() { + return mockMount(this, el); + }); + }); + + afterEach(() => { + resetHTMLFixture(); + Vue.prototype.$mount.mockRestore(); + modalWrapper?.destroy(); + modalWrapper = null; + modal?.destroy(); + modal = null; + }); + + it('creats a ConfirmModal with message as slot', async () => { + const message = 'Bonjour le monde!'; + await renderRootComponent(message); + + expect(modal.vm.$slots.default[0].text).toBe(message); + }); + + it('creats a ConfirmModal with props', async () => { + const options = { + primaryBtnText: 'primaryBtnText', + primaryBtnVariant: 'info', + secondaryBtnText: 'secondaryBtnText', + secondaryBtnVariant: 'success', + cancelBtnText: 'cancelBtnText', + cancelBtnVariant: 'danger', + modalHtmlMessage: 'Hello', + title: 'title', + hideCancel: true, + }; + await renderRootComponent('', options); + expect(modal.props()).toEqual( + expect.objectContaining({ + primaryText: options.primaryBtnText, + primaryVariant: options.primaryBtnVariant, + secondaryText: options.secondaryBtnText, + secondaryVariant: options.secondaryBtnVariant, + cancelText: options.cancelBtnText, + cancelVariant: options.cancelBtnVariant, + modalHtmlMessage: options.modalHtmlMessage, + title: options.title, + hideCancel: options.hideCancel, + }), + ); + }); + + it('resolves promise when modal emit `closed`', async () => { + await renderRootComponent(''); + + modal.vm.$emit('closed'); + + await expect(confirActionPromise).resolves.toBe(false); + }); + + it('confirms when modal emit `confirmed` before `closed`', async () => { + await renderRootComponent(''); + + modal.vm.$emit('confirmed'); + modal.vm.$emit('closed'); + + await expect(confirActionPromise).resolves.toBe(true); + }); +}); diff --git a/spec/frontend/lib/utils/confirm_via_gl_modal/confirm_via_gl_modal_spec.js b/spec/frontend/lib/utils/confirm_via_gl_modal/confirm_via_gl_modal_spec.js new file mode 100644 index 00000000000..6966c79b232 --- /dev/null +++ b/spec/frontend/lib/utils/confirm_via_gl_modal/confirm_via_gl_modal_spec.js @@ -0,0 +1,80 @@ +import { setHTMLFixture, resetHTMLFixture } from 'helpers/fixtures'; +import { confirmViaGlModal } from '~/lib/utils/confirm_via_gl_modal/confirm_via_gl_modal'; +import { confirmAction } from '~/lib/utils/confirm_via_gl_modal/confirm_action'; + +jest.mock('~/lib/utils/confirm_via_gl_modal/confirm_action'); + +describe('confirmViaGlModal', () => { + let el; + + afterEach(() => { + el = undefined; + resetHTMLFixture(); + jest.resetAllMocks(); + }); + + const createElement = (html) => { + setHTMLFixture(html); + return document.body.firstChild; + }; + + it('returns confirmAction result', async () => { + confirmAction.mockReturnValue(Promise.resolve(true)); + el = createElement(`
`); + + await expect(confirmViaGlModal('', el)).resolves.toBe(true); + }); + + it('calls confirmAction with message', () => { + el = createElement(`
`); + + confirmViaGlModal('message', el); + + expect(confirmAction).toHaveBeenCalledWith('message', {}); + }); + + it.each(['gl-sr-only', 'sr-only'])( + `uses slot.%s contentText as primaryBtnText`, + (srOnlyClass) => { + el = createElement( + `Delete merge request`, + ); + + confirmViaGlModal('', el); + + expect(confirmAction).toHaveBeenCalledWith('', { + primaryBtnText: 'Delete merge request', + }); + }, + ); + + it('uses `aria-label` value as `primaryBtnText`', () => { + el = createElement(``); + + confirmViaGlModal('', el); + + expect(confirmAction).toHaveBeenCalledWith('', { + primaryBtnText: 'Delete merge request', + }); + }); + + it.each([ + ['title', 'title', 'Delete?'], + ['confirm-btn-variant', `primaryBtnVariant`, 'danger'], + ])('uses data-%s value as confirmAction config', (dataKey, configKey, value) => { + el = createElement(``); + + confirmViaGlModal('message', el); + + expect(confirmAction).toHaveBeenCalledWith('message', { [configKey]: value }); + }); + + it('uses message as modalHtmlMessage value when data-is-html-message is true', () => { + el = createElement(``); + const message = 'Hola mundo!'; + + confirmViaGlModal(message, el); + + expect(confirmAction).toHaveBeenCalledWith(message, { modalHtmlMessage: message }); + }); +}); diff --git a/spec/frontend/pipelines/pipelines_actions_spec.js b/spec/frontend/pipelines/pipelines_actions_spec.js index 26e61efc4f6..a70ef10aa7b 100644 --- a/spec/frontend/pipelines/pipelines_actions_spec.js +++ b/spec/frontend/pipelines/pipelines_actions_spec.js @@ -13,11 +13,7 @@ import GlCountdown from '~/vue_shared/components/gl_countdown.vue'; import { TRACKING_CATEGORIES } from '~/pipelines/constants'; jest.mock('~/flash'); -jest.mock('~/lib/utils/confirm_via_gl_modal/confirm_via_gl_modal', () => { - return { - confirmAction: jest.fn(), - }; -}); +jest.mock('~/lib/utils/confirm_via_gl_modal/confirm_via_gl_modal'); describe('Pipelines Actions dropdown', () => { let wrapper; diff --git a/spec/frontend/vue_merge_request_widget/deployment/deployment_actions_spec.js b/spec/frontend/vue_merge_request_widget/deployment/deployment_actions_spec.js index 58dadb2c679..41df485b0de 100644 --- a/spec/frontend/vue_merge_request_widget/deployment/deployment_actions_spec.js +++ b/spec/frontend/vue_merge_request_widget/deployment/deployment_actions_spec.js @@ -23,11 +23,7 @@ import { jest.mock('~/flash'); jest.mock('~/lib/utils/url_utility'); -jest.mock('~/lib/utils/confirm_via_gl_modal/confirm_via_gl_modal', () => { - return { - confirmAction: jest.fn(), - }; -}); +jest.mock('~/lib/utils/confirm_via_gl_modal/confirm_via_gl_modal'); describe('DeploymentAction component', () => { let wrapper; diff --git a/spec/frontend/work_items/components/work_item_description_spec.js b/spec/frontend/work_items/components/work_item_description_spec.js index 0691fe25e0d..4a9978bbadf 100644 --- a/spec/frontend/work_items/components/work_item_description_spec.js +++ b/spec/frontend/work_items/components/work_item_description_spec.js @@ -18,11 +18,7 @@ import { workItemQueryResponse, } from '../mock_data'; -jest.mock('~/lib/utils/confirm_via_gl_modal/confirm_via_gl_modal', () => { - return { - confirmAction: jest.fn(), - }; -}); +jest.mock('~/lib/utils/confirm_via_gl_modal/confirm_via_gl_modal'); jest.mock('~/lib/utils/autosave'); const workItemId = workItemQueryResponse.data.workItem.id; diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index a4b2c963c74..b29c0ce6f42 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -506,42 +506,24 @@ RSpec.describe ApplicationHelper do end describe '#page_class' do - context 'when logged_out_marketing_header experiment is enabled' do - let_it_be(:expected_class) { 'logged-out-marketing-header-candidate' } + let_it_be(:expected_class) { 'logged-out-marketing-header' } - let(:current_user) { nil } - let(:variant) { :candidate } + let(:current_user) { nil } - subject do - helper.page_class.flatten - end - - before do - stub_experiments(logged_out_marketing_header: variant) - allow(helper).to receive(:current_user) { current_user } - end - - context 'when candidate' do - it { is_expected.to include(expected_class) } - end - - context 'when candidate (:trial_focused variant)' do - let(:variant) { :trial_focused } - - it { is_expected.to include(expected_class) } - end + subject do + helper.page_class.flatten + end - context 'when control' do - let(:variant) { :control } + before do + allow(helper).to receive(:current_user) { current_user } + end - it { is_expected.not_to include(expected_class) } - end + it { is_expected.to include(expected_class) } - context 'when a user is logged in' do - let(:current_user) { create(:user) } + context 'when a user is logged in' do + let(:current_user) { create(:user) } - it { is_expected.not_to include(expected_class) } - end + it { is_expected.not_to include(expected_class) } end end diff --git a/spec/lib/gitlab/database/partitioning/convert_table_to_first_list_partition_spec.rb b/spec/lib/gitlab/database/partitioning/convert_table_to_first_list_partition_spec.rb index 0e804b4feac..cd3a94f5737 100644 --- a/spec/lib/gitlab/database/partitioning/convert_table_to_first_list_partition_spec.rb +++ b/spec/lib/gitlab/database/partitioning/convert_table_to_first_list_partition_spec.rb @@ -16,6 +16,7 @@ RSpec.describe Gitlab::Database::Partitioning::ConvertTableToFirstListPartition let(:referenced_table_name) { '_test_referenced_table' } let(:other_referenced_table_name) { '_test_other_referenced_table' } let(:parent_table_name) { "#{table_name}_parent" } + let(:lock_tables) { [] } let(:model) { define_batchable_model(table_name, connection: connection) } @@ -27,7 +28,8 @@ RSpec.describe Gitlab::Database::Partitioning::ConvertTableToFirstListPartition table_name: table_name, partitioning_column: partitioning_column, parent_table_name: parent_table_name, - zero_partition_value: partitioning_default + zero_partition_value: partitioning_default, + lock_tables: lock_tables ) end @@ -168,6 +170,16 @@ RSpec.describe Gitlab::Database::Partitioning::ConvertTableToFirstListPartition end end + context 'with locking tables' do + let(:lock_tables) { [table_name] } + + it 'locks the table' do + recorder = ActiveRecord::QueryRecorder.new { partition } + + expect(recorder.log).to include(/LOCK "_test_table_to_partition" IN ACCESS EXCLUSIVE MODE/) + end + end + context 'when an error occurs during the conversion' do def fail_first_time # We can't directly use a boolean here, as we need something that will be passed by-reference to the proc diff --git a/spec/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers_spec.rb b/spec/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers_spec.rb index 8bb9ad2737a..e76b1da3834 100644 --- a/spec/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers_spec.rb +++ b/spec/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers_spec.rb @@ -43,6 +43,7 @@ RSpec.describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHe context 'list partitioning conversion helpers' do shared_examples_for 'delegates to ConvertTableToFirstListPartition' do + let(:extra_options) { {} } it 'throws an error if in a transaction' do allow(migration).to receive(:transaction_open?).and_return(true) expect { migrate }.to raise_error(/cannot be run inside a transaction/) @@ -54,7 +55,8 @@ RSpec.describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHe table_name: source_table, parent_table_name: partitioned_table, partitioning_column: partition_column, - zero_partition_value: min_date) do |converter| + zero_partition_value: min_date, + **extra_options) do |converter| expect(converter).to receive(expected_method) end @@ -64,12 +66,15 @@ RSpec.describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHe describe '#convert_table_to_first_list_partition' do it_behaves_like 'delegates to ConvertTableToFirstListPartition' do + let(:lock_tables) { [source_table] } + let(:extra_options) { { lock_tables: lock_tables } } let(:expected_method) { :partition } let(:migrate) do migration.convert_table_to_first_list_partition(table_name: source_table, partitioning_column: partition_column, parent_table_name: partitioned_table, - initial_partitioning_value: min_date) + initial_partitioning_value: min_date, + lock_tables: lock_tables) end end end diff --git a/spec/migrations/20221021145820_create_routing_table_for_builds_metadata_v2_spec.rb b/spec/migrations/20221021145820_create_routing_table_for_builds_metadata_v2_spec.rb new file mode 100644 index 00000000000..48a00df430d --- /dev/null +++ b/spec/migrations/20221021145820_create_routing_table_for_builds_metadata_v2_spec.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_migration! + +RSpec.describe CreateRoutingTableForBuildsMetadataV2, :migration do + let_it_be(:migration) { described_class.new } + + describe '#up' do + context 'when the table is already partitioned' do + before do + # `convert_table_to_first_list_partition` checks if it's being executed + # inside a transaction, but we're using transactional fixtures here so we + # need to tell it that it's not inside a transaction. + # We toggle the behavior depending on how many transactions we have open + # instead of just returning `false` because the migration could have the + # DDL transaction enabled. + # + open_transactions = ActiveRecord::Base.connection.open_transactions + allow(migration).to receive(:transaction_open?) do + ActiveRecord::Base.connection.open_transactions > open_transactions + end + + migration.convert_table_to_first_list_partition( + table_name: :ci_builds_metadata, + partitioning_column: :partition_id, + parent_table_name: :p_ci_builds_metadata, + initial_partitioning_value: 100) + end + + it 'skips the migration' do + expect { migrate! }.not_to raise_error + end + end + end +end diff --git a/spec/policies/user_policy_spec.rb b/spec/policies/user_policy_spec.rb index b800e7dbc43..d02a94b810e 100644 --- a/spec/policies/user_policy_spec.rb +++ b/spec/policies/user_policy_spec.rb @@ -84,6 +84,50 @@ RSpec.describe UserPolicy do end end + describe "reading a user's associations count" do + context 'when current_user is not an admin' do + context 'fetching their own data' do + subject { described_class.new(current_user, current_user) } + + context 'when current_user is not blocked' do + it { is_expected.to be_allowed(:get_user_associations_count ) } + end + + context 'when current_user is blocked' do + let(:current_user) { create(:user, :blocked) } + + it { is_expected.not_to be_allowed(:get_user_associations_count) } + end + end + + context "fetching a different user's data" do + it { is_expected.not_to be_allowed(:get_user_associations_count) } + end + end + + context 'when current_user is an admin' do + let(:current_user) { admin } + + context 'fetching their own data', :enable_admin_mode do + subject { described_class.new(current_user, current_user) } + + context 'when current_user is not blocked' do + it { is_expected.to be_allowed(:get_user_associations_count ) } + end + + context 'when current_user is blocked' do + let(:current_user) { create(:admin, :blocked) } + + it { is_expected.not_to be_allowed(:get_user_associations_count) } + end + end + + context "fetching a different user's data", :enable_admin_mode do + it { is_expected.to be_allowed(:get_user_associations_count) } + end + end + end + shared_examples 'changing a user' do |ability| context "when a regular user tries to destroy another regular user" do it { is_expected.not_to be_allowed(ability) } diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 1b0a27e78e3..fb8706ab4c3 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -4395,6 +4395,74 @@ RSpec.describe API::Users do end end + describe 'GET /users/:id/associations_count' do + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, :public, group: group) } + let(:associations) do + { + groups_count: 1, + projects_count: 1, + issues_count: 2, + merge_requests_count: 1 + }.as_json + end + + before :all do + group.add_member(user, Gitlab::Access::OWNER) + project.add_member(user, Gitlab::Access::OWNER) + create(:merge_request, source_project: project, source_branch: "my-personal-branch-1", author: user) + create_list(:issue, 2, project: project, author: user) + end + + context 'as an unauthorized user' do + it 'returns 401 unauthorized' do + get api("/users/#{user.id}/associations_count", nil) + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + + context 'as a non-admin user' do + context 'with a different user id' do + it 'returns 403 Forbidden' do + get api("/users/#{omniauth_user.id}/associations_count", user) + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'with the current user id' do + it 'returns valid JSON response' do + get api("/users/#{user.id}/associations_count", user) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_a Hash + expect(json_response).to match(associations) + end + end + end + + context 'as an admin user' do + context 'with invalid user id' do + it 'returns 404 User Not Found' do + get api("/users/#{non_existing_record_id}/associations_count", admin) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'with valid user id' do + it 'returns valid JSON response' do + get api("/users/#{user.id}/associations_count", admin) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_a Hash + expect(json_response).to match(associations) + end + end + end + end + it_behaves_like 'custom attributes endpoints', 'users' do let(:attributable) { user } let(:other_attributable) { admin } diff --git a/spec/services/groups/create_service_spec.rb b/spec/services/groups/create_service_spec.rb index 0a8164c9ca3..0425ba3e631 100644 --- a/spec/services/groups/create_service_spec.rb +++ b/spec/services/groups/create_service_spec.rb @@ -271,33 +271,4 @@ RSpec.describe Groups::CreateService, '#execute' do end end end - - describe 'logged_out_marketing_header experiment', :experiment do - let(:service) { described_class.new(user, group_params) } - - subject { service.execute } - - before do - stub_experiments(logged_out_marketing_header: :candidate) - end - - it 'tracks signed_up event' do - expect(experiment(:logged_out_marketing_header)).to track( - :namespace_created, - namespace: an_instance_of(Group) - ).on_next_instance.with_context(actor: user) - - subject - end - - context 'when group has not been persisted' do - let(:service) { described_class.new(user, group_params.merge(name: '')) } - - it 'does not track signed_up event' do - expect(experiment(:logged_out_marketing_header)).not_to track(:namespace_created) - - subject - end - end - end end -- cgit v1.2.3