diff options
30 files changed, 1010 insertions, 578 deletions
diff --git a/.rubocop_todo/rspec/context_wording.yml b/.rubocop_todo/rspec/context_wording.yml index c1751ebed4c..c34185e4e0b 100644 --- a/.rubocop_todo/rspec/context_wording.yml +++ b/.rubocop_todo/rspec/context_wording.yml @@ -689,7 +689,6 @@ RSpec/ContextWording: - 'ee/spec/services/ee/keys/destroy_service_spec.rb' - 'ee/spec/services/ee/members/create_service_spec.rb' - 'ee/spec/services/ee/members/destroy_service_spec.rb' - - 'ee/spec/services/ee/members/import_project_team_service_spec.rb' - 'ee/spec/services/ee/members/invite_service_spec.rb' - 'ee/spec/services/ee/merge_requests/base_service_spec.rb' - 'ee/spec/services/ee/merge_requests/create_service_spec.rb' diff --git a/.rubocop_todo/style/symbol_proc.yml b/.rubocop_todo/style/symbol_proc.yml index 67229a8b704..a8f966d2266 100644 --- a/.rubocop_todo/style/symbol_proc.yml +++ b/.rubocop_todo/style/symbol_proc.yml @@ -13,7 +13,6 @@ Style/SymbolProc: - 'app/models/members/project_member.rb' - 'app/models/namespace.rb' - 'app/models/preloaders/merge_request_diff_preloader.rb' - - 'app/models/project_team.rb' - 'app/models/release.rb' - 'app/models/remote_mirror.rb' - 'app/models/snippet_input_action_collection.rb' diff --git a/app/assets/javascripts/invite_members/components/import_project_members_modal.vue b/app/assets/javascripts/invite_members/components/import_project_members_modal.vue index 66d4a9ccc07..5599ad276f0 100644 --- a/app/assets/javascripts/invite_members/components/import_project_members_modal.vue +++ b/app/assets/javascripts/invite_members/components/import_project_members_modal.vue @@ -1,5 +1,5 @@ <script> -import { GlFormGroup, GlModal, GlSprintf } from '@gitlab/ui'; +import { GlFormGroup, GlModal, GlSprintf, GlAlert, GlCollapse, GlIcon, GlButton } from '@gitlab/ui'; import { uniqueId, isEmpty } from 'lodash'; import { importProjectMembers } from '~/api/projects_api'; import { BV_SHOW_MODAL, BV_HIDE_MODAL } from '~/lib/utils/constants'; @@ -16,8 +16,10 @@ import { PROJECT_SELECT_LABEL_ID, IMPORT_PROJECT_MEMBERS_MODAL_TRACKING_CATEGORY, IMPORT_PROJECT_MEMBERS_MODAL_TRACKING_LABEL, + MEMBER_MODAL_LABELS, } from '../constants'; +import { responseFromSuccess } from '../utils/response_message_parser'; import UserLimitNotification from './user_limit_notification.vue'; import ProjectSelect from './project_select.vue'; @@ -27,6 +29,10 @@ export default { GlFormGroup, GlModal, GlSprintf, + GlAlert, + GlCollapse, + GlIcon, + GlButton, UserLimitNotification, ProjectSelect, }, @@ -60,6 +66,9 @@ export default { return { projectToBeImported: {}, invalidFeedbackMessage: '', + totalMembersCount: 0, + invalidMembers: {}, + isErrorsSectionExpanded: false, isLoading: false, }; }, @@ -94,6 +103,40 @@ export default { actionCancel() { return { text: this.$options.i18n.modalCancelButton }; }, + hasInvalidMembers() { + return !isEmpty(this.invalidMembers); + }, + memberErrorTitle() { + return sprintf( + s__( + 'InviteMembersModal|The following %{errorMembersLength} out of %{totalMembersCount} members could not be added', + ), + { errorMembersLength: this.errorList.length, totalMembersCount: this.totalMembersCount }, + ); + }, + errorList() { + return Object.entries(this.invalidMembers).map(([member, error]) => { + return { member, displayedMemberName: `@${member}`, message: error }; + }); + }, + errorsLimited() { + return this.errorList.slice(0, this.$options.errorsLimit); + }, + errorsExpanded() { + return this.errorList.slice(this.$options.errorsLimit); + }, + shouldErrorsSectionExpand() { + return Boolean(this.errorsExpanded.length); + }, + errorCollapseText() { + if (this.isErrorsSectionExpanded) { + return this.$options.labels.expandedErrors; + } + + return sprintf(this.$options.labels.collapsedErrors, { + count: this.errorsExpanded.length, + }); + }, }, mounted() { if (this.reloadPageOnSubmit) { @@ -113,21 +156,37 @@ export default { this.$root.$emit(BV_HIDE_MODAL, this.$options.modalId); }, resetFields() { + this.clearValidation(); this.invalidFeedbackMessage = ''; this.projectToBeImported = {}; }, - submitImport(e) { + async submitImport(event) { // We never want to hide when submitting - e.preventDefault(); + event.preventDefault(); this.isLoading = true; - return importProjectMembers(this.projectId, this.projectToBeImported.id) - .then(this.onInviteSuccess) - .catch(this.showErrorAlert) - .finally(() => { - this.isLoading = false; - this.projectToBeImported = {}; - }); + + try { + const response = await importProjectMembers(this.projectId, this.projectToBeImported.id); + + const { error, message } = responseFromSuccess(response); + + if (error) { + this.totalMembersCount = response.data.total_members_count; + this.showMemberErrors(message); + } else { + this.onInviteSuccess(); + } + } catch { + this.showErrorAlert(); + } finally { + this.isLoading = false; + this.projectToBeImported = {}; + } + }, + showMemberErrors(message) { + this.invalidMembers = message; + this.$refs.alerts.focus(); }, onInviteSuccess() { this.track('invite_successful'); @@ -151,6 +210,13 @@ export default { onClose() { this.track('click_x'); }, + clearValidation() { + this.invalidFeedbackMessage = ''; + this.invalidMembers = {}; + }, + toggleErrorExpansion() { + this.isErrorsSectionExpanded = !this.isErrorsSectionExpanded; + }, }, toastOptions() { return { @@ -173,8 +239,10 @@ export default { defaultError: s__('ImportAProjectModal|Unable to import project members'), successMessage: s__('ImportAProjectModal|Successfully imported'), }, + errorsLimit: 2, projectSelectLabelId: PROJECT_SELECT_LABEL_ID, modalId: uniqueId('import-a-project-modal-'), + labels: MEMBER_MODAL_LABELS, }; </script> @@ -186,18 +254,62 @@ export default { :title="$options.i18n.modalTitle" :action-primary="actionPrimary" :action-cancel="actionCancel" + data-testid="import-project-members-modal" no-focus-on-show @primary="submitImport" @hidden="resetFields" @cancel="onCancel" @close="onClose" > - <user-limit-notification - v-if="showUserLimitNotification" - class="gl-mb-5" - :limit-variant="limitVariant" - :users-limit-dataset="usersLimitDataset" - /> + <div ref="alerts" tabindex="-1"> + <gl-alert + v-if="hasInvalidMembers" + class="gl-mb-4" + variant="danger" + :dismissible="false" + :title="memberErrorTitle" + data-testid="alert-member-error" + > + {{ $options.labels.memberErrorListText }} + <ul class="gl-pl-5 gl-mb-0"> + <li v-for="error in errorsLimited" :key="error.member" data-testid="errors-limited-item"> + <strong>{{ error.displayedMemberName }}:</strong> {{ error.message }} + </li> + </ul> + <template v-if="shouldErrorsSectionExpand"> + <gl-collapse v-model="isErrorsSectionExpanded"> + <ul class="gl-pl-5 gl-mb-0"> + <li + v-for="error in errorsExpanded" + :key="error.member" + data-testid="errors-expanded-item" + > + <strong>{{ error.displayedMemberName }}:</strong> {{ error.message }} + </li> + </ul> + </gl-collapse> + <gl-button + class="gl-text-decoration-none! gl-shadow-none! gl-mt-3" + data-testid="accordion-button" + variant="link" + @click="toggleErrorExpansion" + > + {{ errorCollapseText }} + <gl-icon + name="chevron-down" + class="gl-transition-medium" + :class="{ 'gl-rotate-180': isErrorsSectionExpanded }" + /> + </gl-button> + </template> + </gl-alert> + <user-limit-notification + v-else-if="showUserLimitNotification" + class="gl-mb-5" + :limit-variant="limitVariant" + :users-limit-dataset="usersLimitDataset" + /> + </div> <p ref="modalIntro"> <gl-sprintf :message="modalIntro"> <template #strong="{ content }"> diff --git a/app/assets/javascripts/vue_shared/components/actions_button.vue b/app/assets/javascripts/vue_shared/components/actions_button.vue deleted file mode 100644 index 1d6dbef799a..00000000000 --- a/app/assets/javascripts/vue_shared/components/actions_button.vue +++ /dev/null @@ -1,74 +0,0 @@ -<script> -import { - GlDisclosureDropdown, - GlDisclosureDropdownGroup, - GlDisclosureDropdownItem, -} from '@gitlab/ui'; - -export default { - components: { - GlDisclosureDropdown, - GlDisclosureDropdownGroup, - GlDisclosureDropdownItem, - }, - props: { - toggleText: { - type: String, - required: true, - }, - actions: { - type: Array, - required: true, - }, - category: { - type: String, - required: false, - default: 'secondary', - }, - variant: { - type: String, - required: false, - default: 'default', - }, - }, - methods: { - handleItemClick(action) { - return action.handle?.(); - }, - }, -}; -</script> - -<template> - <gl-disclosure-dropdown - :variant="variant" - :category="category" - :toggle-text="toggleText" - data-qa-selector="action_dropdown" - fluid-width - block - @shown="$emit('shown')" - @hidden="$emit('hidden')" - > - <gl-disclosure-dropdown-group class="edit-dropdown-group-width"> - <gl-disclosure-dropdown-item - v-for="action in actions" - :key="action.key" - v-bind="action.attrs" - :item="action" - :data-qa-selector="`${action.key}_menu_item`" - @action="handleItemClick(action)" - > - <template #list-item> - <div class="gl-display-flex gl-flex-direction-column"> - <span class="gl-font-weight-bold gl-mb-2">{{ action.text }}</span> - <span class="gl-text-gray-700"> - {{ action.secondaryText }} - </span> - </div> - </template> - </gl-disclosure-dropdown-item> - </gl-disclosure-dropdown-group> - <slot></slot> - </gl-disclosure-dropdown> -</template> diff --git a/app/assets/javascripts/vue_shared/components/web_ide_link.vue b/app/assets/javascripts/vue_shared/components/web_ide_link.vue index 9a06c0ecf30..756d9b42e99 100644 --- a/app/assets/javascripts/vue_shared/components/web_ide_link.vue +++ b/app/assets/javascripts/vue_shared/components/web_ide_link.vue @@ -1,8 +1,14 @@ <script> -import { GlModal, GlSprintf, GlLink } from '@gitlab/ui'; +import { + GlModal, + GlSprintf, + GlLink, + GlDisclosureDropdown, + GlDisclosureDropdownGroup, + GlDisclosureDropdownItem, +} from '@gitlab/ui'; import { s__, __ } from '~/locale'; import { visitUrl } from '~/lib/utils/url_utility'; -import ActionsButton from '~/vue_shared/components/actions_button.vue'; import ConfirmForkModal from '~/vue_shared/components/web_ide/confirm_fork_modal.vue'; import { KEY_EDIT, KEY_WEB_IDE, KEY_GITPOD, KEY_PIPELINE_EDITOR } from './constants'; @@ -25,10 +31,12 @@ export const i18n = { export default { name: 'CEWebIdeLink', components: { - ActionsButton, GlModal, GlSprintf, GlLink, + GlDisclosureDropdown, + GlDisclosureDropdownGroup, + GlDisclosureDropdownItem, ConfirmForkModal, }, i18n, @@ -174,7 +182,6 @@ export default { text: __('Edit single file'), secondaryText: __('Edit this file only.'), attrs: { - 'data-qa-selector': 'edit_button', 'data-track-action': 'click_consolidated_edit', 'data-track-label': 'edit', }, @@ -217,7 +224,6 @@ export default { text: this.webIdeActionText, secondaryText: this.$options.i18n.webIdeText, attrs: { - 'data-qa-selector': 'web_ide_button', 'data-track-action': 'click_consolidated_edit_ide', 'data-track-label': 'web_ide', }, @@ -246,10 +252,11 @@ export default { key: KEY_PIPELINE_EDITOR, text: __('Edit in pipeline editor'), secondaryText, + href: this.pipelineEditorUrl, attrs: { - 'data-qa-selector': 'pipeline_editor_button', + 'data-track-action': 'click_consolidated_pipeline_editor', + 'data-track-label': 'pipeline_editor', }, - href: this.pipelineEditorUrl, }; }, gitpodAction() { @@ -270,9 +277,6 @@ export default { key: KEY_GITPOD, text: this.gitpodActionText, secondaryText, - attrs: { - 'data-qa-selector': 'gitpod_button', - }, ...handleOptions, }; }, @@ -306,25 +310,50 @@ export default { showModal(dataKey) { this[dataKey] = true; }, + executeAction(action) { + action.handle?.(); + }, }, - webIdeButtonId: 'web-ide-link', }; </script> <template> <div class="gl-sm-ml-3"> - <actions-button + <gl-disclosure-dropdown v-if="hasActions" - :id="$options.webIdeButtonId" - :actions="actions" - :toggle-text="$options.i18n.toggleText" :variant="isBlob ? 'confirm' : 'default'" :category="isBlob ? 'primary' : 'secondary'" - @hidden="$emit('hidden')" + :toggle-text="$options.i18n.toggleText" + data-qa-selector="action_dropdown" + fluid-width + block @shown="$emit('shown')" + @hidden="$emit('hidden')" > - <slot></slot> - </actions-button> + <slot name="before-actions"></slot> + <gl-disclosure-dropdown-group class="edit-dropdown-group-width"> + <gl-disclosure-dropdown-item + v-for="action in actions" + :key="action.key" + v-bind="action.attrs" + :item="action" + :data-qa-selector="`${action.key}_menu_item`" + @action="executeAction(action)" + > + <template #list-item> + <div class="gl-display-flex gl-flex-direction-column"> + <span data-testid="action-primary-text" class="gl-font-weight-bold gl-mb-2">{{ + action.text + }}</span> + <span data-testid="action-secondary-text" class="gl-text-gray-700"> + {{ action.secondaryText }} + </span> + </div> + </template> + </gl-disclosure-dropdown-item> + </gl-disclosure-dropdown-group> + <slot name="after-actions"></slot> + </gl-disclosure-dropdown> <gl-modal v-if="computedShowGitpodButton && !gitpodEnabled" v-model="showEnableGitpodModal" diff --git a/app/models/project_team.rb b/app/models/project_team.rb index fbdc88e7b76..3b9b82ee094 100644 --- a/app/models/project_team.rb +++ b/app/models/project_team.rb @@ -141,12 +141,10 @@ class ProjectTeam end ProjectMember.transaction do - source_members.each do |member| - member.save - end + source_members.each(&:save) end - true + source_members rescue StandardError false end diff --git a/app/services/members/import_project_team_service.rb b/app/services/members/import_project_team_service.rb index 6efd65e2575..ef43d8206a9 100644 --- a/app/services/members/import_project_team_service.rb +++ b/app/services/members/import_project_team_service.rb @@ -2,36 +2,83 @@ module Members class ImportProjectTeamService < BaseService - attr_reader :params, :current_user + ImportProjectTeamForbiddenError = Class.new(StandardError) - def target_project_id - @target_project_id ||= params[:id].presence + def initialize(*args) + super + + @errors = {} end - def source_project_id - @source_project_id ||= params[:project_id].presence + def execute + check_target_and_source_projects_exist! + check_user_permissions! + + import_project_team + process_import_result + + result + rescue ArgumentError, ImportProjectTeamForbiddenError => e + ServiceResponse.error(message: e.message, reason: :unprocessable_entity) end - def target_project - @target_project ||= Project.find_by_id(target_project_id) + private + + attr_reader :members, :params, :current_user, :errors, :result + + def import_project_team + @members = target_project.team.import(source_project, current_user) + + if members.is_a?(Array) + members.each { |member| check_member_validity(member) } + else + @result = ServiceResponse.error(message: 'Import failed', reason: :unprocessable_entity) + end end - def source_project - @source_project ||= Project.find_by_id(source_project_id) + def check_target_and_source_projects_exist! + if target_project.blank? + raise ArgumentError, 'Target project does not exist' + elsif source_project.blank? + raise ArgumentError, 'Source project does not exist' + end end - def execute - import_project_team + def check_user_permissions! + return if can?(current_user, :read_project_member, source_project) && + can?(current_user, :import_project_members_from_another_project, target_project) + + raise ImportProjectTeamForbiddenError, 'Forbidden' end - private + def check_member_validity(member) + return if member.valid? - def import_project_team - return false unless target_project.present? && source_project.present? && current_user.present? - return false unless can?(current_user, :read_project_member, source_project) - return false unless can?(current_user, :import_project_members_from_another_project, target_project) + errors[member.user.username] = member.errors.full_messages.to_sentence + end + + def process_import_result + @result ||= if errors.any? + ServiceResponse.error(message: errors, payload: { total_members_count: members.size }) + else + ServiceResponse.success(message: 'Successfully imported') + end + end + + def target_project_id + params[:id] + end - target_project.team.import(source_project, current_user) + def source_project_id + params[:project_id] + end + + def target_project + @target_project ||= Project.find_by_id(target_project_id) + end + + def source_project + @source_project ||= Project.find_by_id(source_project_id) end end end diff --git a/app/views/projects/blob/_breadcrumb.html.haml b/app/views/projects/blob/_breadcrumb.html.haml index 417c11ba37a..3178426ee8b 100644 --- a/app/views/projects/blob/_breadcrumb.html.haml +++ b/app/views/projects/blob/_breadcrumb.html.haml @@ -1,7 +1,7 @@ - blame = local_assigns.fetch(:blame, false) .nav-block .tree-ref-container - .tree-ref-holder + .tree-ref-holder.gl-max-w-26 #js-tree-ref-switcher{ data: { project_id: @project.id, project_root_path: project_path(@project), ref: current_ref, ref_type: @ref_type.to_s } } %ul.breadcrumb.repo-breadcrumb diff --git a/app/views/projects/blob/show.html.haml b/app/views/projects/blob/show.html.haml index c8cf12c36f9..1ffcb11247b 100644 --- a/app/views/projects/blob/show.html.haml +++ b/app/views/projects/blob/show.html.haml @@ -10,7 +10,7 @@ = render 'projects/last_push' -#tree-holder.tree-holder +#tree-holder.tree-holder.gl-pt-4 = render 'blob', blob: @blob = render partial: 'pipeline_tour_success' if show_suggest_pipeline_creation_celebration? diff --git a/app/views/projects/find_file/show.html.haml b/app/views/projects/find_file/show.html.haml index 7e93e44c463..541b8c1147d 100644 --- a/app/views/projects/find_file/show.html.haml +++ b/app/views/projects/find_file/show.html.haml @@ -1,9 +1,9 @@ - page_title _("Find File"), @ref - add_page_specific_style 'page_bundles/tree' -.file-finder-holder.tree-holder.clearfix.js-file-finder{ 'data-file-find-url': "#{escape_javascript(project_files_path(@project, @ref, format: :json))}", 'data-find-tree-url': escape_javascript(project_tree_path(@project, @ref)), 'data-blob-url-template': escape_javascript(project_blob_path(@project, @ref)) } +.file-finder-holder.tree-holder.clearfix.js-file-finder.gl-pt-4{ 'data-file-find-url': "#{escape_javascript(project_files_path(@project, @ref, format: :json))}", 'data-find-tree-url': escape_javascript(project_tree_path(@project, @ref)), 'data-blob-url-template': escape_javascript(project_blob_path(@project, @ref)) } .nav-block.gl-xs-mr-0 - .tree-ref-holder.gl-xs-mb-3.gl-xs-w-full + .tree-ref-holder.gl-xs-mb-3.gl-xs-w-full.gl-max-w-26 #js-blob-ref-switcher{ data: { project_id: @project.id, ref: @ref, namespace: "/-/find_file" } } %ul.breadcrumb.repo-breadcrumb.gl-flex-nowrap %li.breadcrumb-item.gl-white-space-nowrap diff --git a/doc/api/projects.md b/doc/api/projects.md index 6afed915135..813d0fe35cb 100644 --- a/doc/api/projects.md +++ b/doc/api/projects.md @@ -2647,6 +2647,33 @@ Returns: - `404 Project Not Found` if the target or source project does not exist or cannot be accessed by the requester. - `422 Unprocessable Entity` if the import of project members does not complete successfully. +Example responses: + +When all emails were successfully sent (`200` HTTP status code): + +```json +{ "status": "success" } +``` + +When there was any error importing 1 or more members (`200` HTTP status code): + +```json +{ + "status": "error", + "message": { + "john_smith": "Some individual error message", + "jane_smith": "Some individual error message" + }, + "total_members_count": 3 +} +``` + +When there is a system error (`404` and `422` HTTP status codes): + +```json +{ "message": "Import failed" } +``` + ## Hooks Also called Project Hooks and Webhooks. These are different for [System Hooks](system_hooks.md) diff --git a/doc/development/database/batched_background_migrations.md b/doc/development/database/batched_background_migrations.md index 1bdd24afcad..10490df7b5e 100644 --- a/doc/development/database/batched_background_migrations.md +++ b/doc/development/database/batched_background_migrations.md @@ -13,6 +13,13 @@ in our guidelines. For example, you can use batched background migrations to migrate data that's stored in a single JSON column to a separate table instead. +NOTE: +Batched background migrations replaced the legacy background migrations framework. +Check that documentation in reference to any changes involving that framework. + +NOTE: +The batched background migrations framework has ChatOps support. Using ChatOps, GitLab engineers can interact with the batched background migrations present in the system. + ## When to use batched background migrations Use a batched background migration when you migrate _data_ in tables containing @@ -201,6 +208,13 @@ models defined in `app/models` except the `ApplicationRecord` classes). Because these migrations can take a long time to run, it's possible for new versions to deploy while the migrations are still running. +### Depending on migrated data + +Unlike a regular or a post migration, waiting for the next release is not enough to guarantee that the data was fully migrated. +That means that you shouldn't depend on the data until the BBM is finished. If having 100% of the data migrated is a requirement, +then, the `ensure_batched_background_migration_is_finished` helper can be used to guarantee that the migration was finished and the +data fully migrated. ([See an example](https://gitlab.com/gitlab-org/gitlab/-/blob/41fbe34a4725a4e357a83fda66afb382828767b2/db/post_migrate/20210707210916_finalize_ci_stages_bigint_conversion.rb#L13-18)). + ## How to ### Generate a batched background migration @@ -538,6 +552,107 @@ Bumping the [import/export version](../../user/project/settings/import_export.md be required, if importing a project from a prior version of GitLab requires the data to be in the new format. +### Add indexes to support batched background migrations + +Sometimes it is necessary to add a new or temporary index to support a batched background migration. +To do this, create the index in a post-deployment migration that precedes the post-deployment +migration that queues the background migration. + +See the documentation for [adding database indexes](adding_database_indexes.md#analyzing-a-new-index-before-a-batched-background-migration) +for additional information about some cases that require special attention to allow the index to be used directly after +creation. + +### Execute a particular batch on the database testing pipeline + +NOTE: +Only [database maintainers](https://gitlab.com/groups/gitlab-org/maintainers/database/-/group_members?with_inherited_permissions=exclude) can view the database testing pipeline artifacts. Ask one for help if you need to use this method. + +Let's assume that a batched background migration failed on a particular batch on GitLab.com and you want to figure out which query failed and why. At the moment, we don't have a good way to retrieve query information (especially the query parameters) and rerunning the entire migration with more logging would be a long process. + +Fortunately you can leverage our [database migration pipeline](database_migration_pipeline.md) to rerun a particular batch with additional logging and/or fix to see if it solves the problem. + +For an example see [Draft: `Test PG::CardinalityViolation` fix](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/110910) but make sure to read the entire section. + +To do that, you need to: + +1. [Find the batch `start_id` and `end_id`](#find-the-batch-start_id-and-end_id) +1. [Create a regular migration](#create-a-regular-migration) +1. [Apply a workaround for our migration helpers](#apply-a-workaround-for-our-migration-helpers-optional) (optional) +1. [Start the database migration pipeline](#start-the-database-migration-pipeline) + +#### Find the batch `start_id` and `end_id` + +You should be able to find those in [Kibana](#viewing-failure-error-logs). + +#### Create a regular migration + +Schedule the batch in the `up` block of a regular migration: + +```ruby +def up + instance = Gitlab::BackgroundMigration::YourBackgroundMigrationClass.new( + start_id: <batch start_id>, + end_id: <batch end_id>, + batch_table: <table name>, + batch_column: <batching column>, + sub_batch_size: <sub batch size>, + pause_ms: <miliseconds between batches>, + job_arguments: <job arguments if any>, + connection: connection + ) + + instance.perform +end + +def down + # no-op +end +``` + +#### Apply a workaround for our migration helpers (optional) + +If your batched background migration touches tables from a schema other than the one you specified by using `restrict_gitlab_migration` helper (example: the scheduling migration has `restrict_gitlab_migration gitlab_schema: :gitlab_main` but the background job uses tables from the `:gitlab_ci` schema) then the migration will fail. To prevent that from happening you must to monkey patch database helpers so they don't fail the testing pipeline job: + +1. Add the schema names to [`RestrictGitlabSchema`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/database/migration_helpers/restrict_gitlab_schema.rb#L57) + +```diff +diff --git a/lib/gitlab/database/migration_helpers/restrict_gitlab_schema.rb b/lib/gitlab/database/migration_helpers/restrict_gitlab_schema.rb +index b8d1d21a0d2d2a23d9e8c8a0a17db98ed1ed40b7..912e20659a6919f771045178c66828563cb5a4a1 100644 +--- a/lib/gitlab/database/migration_helpers/restrict_gitlab_schema.rb ++++ b/lib/gitlab/database/migration_helpers/restrict_gitlab_schema.rb +@@ -55,7 +55,7 @@ def unmatched_schemas + end + + def allowed_schemas_for_connection +- Gitlab::Database.gitlab_schemas_for_connection(connection) ++ Gitlab::Database.gitlab_schemas_for_connection(connection) << :gitlab_ci + end + end + end +``` + +1. Add the schema names to [`RestrictAllowedSchemas`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/database/query_analyzers/restrict_allowed_schemas.rb#L82) + +```diff +diff --git a/lib/gitlab/database/query_analyzers/restrict_allowed_schemas.rb b/lib/gitlab/database/query_analyzers/restrict_allowed_schemas.rb +index 4ae3622479f0800c0553959e132143ec9051898e..d556ec7f55adae9d46a56665ce02de782cb09f2d 100644 +--- a/lib/gitlab/database/query_analyzers/restrict_allowed_schemas.rb ++++ b/lib/gitlab/database/query_analyzers/restrict_allowed_schemas.rb +@@ -79,7 +79,7 @@ def restrict_to_dml_only(parsed) + tables = self.dml_tables(parsed) + schemas = self.dml_schemas(tables) + +- if (schemas - self.allowed_gitlab_schemas).any? ++ if (schemas - (self.allowed_gitlab_schemas << :gitlab_ci)).any? + raise DMLAccessDeniedError, \ + "Select/DML queries (SELECT/UPDATE/DELETE) do access '#{tables}' (#{schemas.to_a}) " \ + "which is outside of list of allowed schemas: '#{self.allowed_gitlab_schemas}'. " \ +``` + +#### Start the database migration pipeline + +Create a Draft merge request with your changes and trigger the manual `db:gitlabcom-database-testing` job. + ## Managing NOTE: @@ -683,22 +798,109 @@ migration is scheduled in the GitLab FOSS context. You can use the [generator](#generate-a-batched-background-migration) to generate an EE-only migration scaffold by passing `--ee-only` flag when generating a new batched background migration. -## Throttling batched migrations +## Debug + +### Viewing failure error logs -Because batched migrations are update-heavy and there were few incidents in the past because of the heavy load from migrations while the database was underperforming, a throttling mechanism exists to mitigate them. +You can view failures in two ways: -These database indicators are checked to throttle a migration. On getting a -stop signal, the migration is paused for a set time (10 minutes): +- Via GitLab logs: + 1. After running a batched background migration, if any jobs fail, + view the logs in [Kibana](https://log.gprd.gitlab.net/goto/4cb43f40-f861-11ec-b86b-d963a1a6788e). + View the production Sidekiq log and filter for: -- WAL queue pending archival crossing a threshold. -- Active autovacuum on the tables on which the migration works on. -- Patroni apdex SLI dropping below the SLO. + - `json.new_state: failed` + - `json.job_class_name: <Batched Background Migration job class name>` + - `json.job_arguments: <Batched Background Migration job class arguments>` -It's an ongoing effort to add more indicators to further enhance the -database health check framework. For more details, see -[epic 7594](https://gitlab.com/groups/gitlab-org/-/epics/7594). + 1. Review the `json.exception_class` and `json.exception_message` values to help + understand why the jobs failed. + + 1. Remember the retry mechanism. Having a failure does not mean the job failed. + Always check the last status of the job. + +- Via database: + + 1. Get the batched background migration `CLASS_NAME`. + 1. Execute the following query in the PostgreSQL console: + + ```sql + SELECT migration.id, migration.job_class_name, transition_logs.exception_class, transition_logs.exception_message + FROM batched_background_migrations as migration + INNER JOIN batched_background_migration_jobs as jobs + ON jobs.batched_background_migration_id = migration.id + INNER JOIN batched_background_migration_job_transition_logs as transition_logs + ON transition_logs.batched_background_migration_job_id = jobs.id + WHERE transition_logs.next_status = '2' AND migration.job_class_name = "CLASS_NAME"; + ``` + +## Testing + +Writing tests is required for: + +- The batched background migrations' queueing migration. +- The batched background migration itself. +- A cleanup migration. + +The `:migration` and `schema: :latest` RSpec tags are automatically set for +background migration specs. Refer to the +[Testing Rails migrations](../testing_guide/testing_migrations_guide.md#testing-a-non-activerecordmigration-class) +style guide. + +Remember that `before` and `after` RSpec hooks +migrate your database down and up. These hooks can result in other batched background +migrations being called. Using `spy` test doubles with +`have_received` is encouraged, instead of using regular test doubles, because +your expectations defined in a `it` block can conflict with what is +called in RSpec hooks. Refer to [issue #35351](https://gitlab.com/gitlab-org/gitlab/-/issues/18839) +for more details. + +## Best practices + +1. Know how much data you're dealing with. +1. Make sure the batched background migration jobs are idempotent. +1. Confirm the tests you write are not false positives. +1. If the data being migrated is critical and cannot be lost, the + clean-up migration must also check the final state of the data before completing. +1. Discuss the numbers with a database specialist. The migration may add + more pressure on DB than you expect. Measure on staging, + or ask someone to measure on production. +1. Know how much time is required to run the batched background migration. +1. Be careful when silently rescuing exceptions inside job classes. This may lead to + jobs being marked as successful, even in a failure scenario. + + ```ruby + # good + def perform + each_sub_batch do |sub_batch| + sub_batch.update_all(name: 'My Name') + end + end + + # acceptable + def perform + each_sub_batch do |sub_batch| + sub_batch.update_all(name: 'My Name') + rescue Exception => error + logger.error(message: error.message, class: error.class) + + raise + end + end + + # bad + def perform + each_sub_batch do |sub_batch| + sub_batch.update_all(name: 'My Name') + rescue Exception => error + logger.error(message: error.message, class: self.class.name) + end + end + ``` + +## Examples -## Example +### Routes use-case The `routes` table has a `source_type` field that's used for a polymorphic relationship. As part of a database redesign, we're removing the polymorphic relationship. One step of @@ -867,216 +1069,3 @@ background migration. After the batched migration is completed, you can safely depend on the data in `routes.namespace_id` being populated. - -## Testing - -Writing tests is required for: - -- The batched background migrations' queueing migration. -- The batched background migration itself. -- A cleanup migration. - -The `:migration` and `schema: :latest` RSpec tags are automatically set for -background migration specs. Refer to the -[Testing Rails migrations](../testing_guide/testing_migrations_guide.md#testing-a-non-activerecordmigration-class) -style guide. - -Remember that `before` and `after` RSpec hooks -migrate your database down and up. These hooks can result in other batched background -migrations being called. Using `spy` test doubles with -`have_received` is encouraged, instead of using regular test doubles, because -your expectations defined in a `it` block can conflict with what is -called in RSpec hooks. Refer to [issue #35351](https://gitlab.com/gitlab-org/gitlab/-/issues/18839) -for more details. - -## Best practices - -1. Know how much data you're dealing with. -1. Make sure the batched background migration jobs are idempotent. -1. Confirm the tests you write are not false positives. -1. If the data being migrated is critical and cannot be lost, the - clean-up migration must also check the final state of the data before completing. -1. Discuss the numbers with a database specialist. The migration may add - more pressure on DB than you expect. Measure on staging, - or ask someone to measure on production. -1. Know how much time is required to run the batched background migration. -1. Be careful when silently rescuing exceptions inside job classes. This may lead to - jobs being marked as successful, even in a failure scenario. - - ```ruby - # good - def perform - each_sub_batch do |sub_batch| - sub_batch.update_all(name: 'My Name') - end - end - - # acceptable - def perform - each_sub_batch do |sub_batch| - sub_batch.update_all(name: 'My Name') - rescue Exception => error - logger.error(message: error.message, class: error.class) - - raise - end - end - - # bad - def perform - each_sub_batch do |sub_batch| - sub_batch.update_all(name: 'My Name') - rescue Exception => error - logger.error(message: error.message, class: self.class.name) - end - end - ``` - -## Additional tips and strategies - -### ChatOps integration - -The batched background migrations framework has ChatOps support. Using ChatOps, GitLab engineers can interact with the batched background migrations present in the system. - -### Viewing failure error logs - -You can view failures in two ways: - -- Via GitLab logs: - 1. After running a batched background migration, if any jobs fail, - view the logs in [Kibana](https://log.gprd.gitlab.net/goto/4cb43f40-f861-11ec-b86b-d963a1a6788e). - View the production Sidekiq log and filter for: - - - `json.new_state: failed` - - `json.job_class_name: <Batched Background Migration job class name>` - - `json.job_arguments: <Batched Background Migration job class arguments>` - - 1. Review the `json.exception_class` and `json.exception_message` values to help - understand why the jobs failed. - - 1. Remember the retry mechanism. Having a failure does not mean the job failed. - Always check the last status of the job. - -- Via database: - - 1. Get the batched background migration `CLASS_NAME`. - 1. Execute the following query in the PostgreSQL console: - - ```sql - SELECT migration.id, migration.job_class_name, transition_logs.exception_class, transition_logs.exception_message - FROM batched_background_migrations as migration - INNER JOIN batched_background_migration_jobs as jobs - ON jobs.batched_background_migration_id = migration.id - INNER JOIN batched_background_migration_job_transition_logs as transition_logs - ON transition_logs.batched_background_migration_job_id = jobs.id - WHERE transition_logs.next_status = '2' AND migration.job_class_name = "CLASS_NAME"; - ``` - -### Executing a particular batch on the database testing pipeline - -NOTE: -Only [database maintainers](https://gitlab.com/groups/gitlab-org/maintainers/database/-/group_members?with_inherited_permissions=exclude) can view the database testing pipeline artifacts. Ask one for help if you need to use this method. - -Let's assume that a batched background migration failed on a particular batch on GitLab.com and you want to figure out which query failed and why. At the moment, we don't have a good way to retrieve query information (especially the query parameters) and rerunning the entire migration with more logging would be a long process. - -Fortunately you can leverage our [database migration pipeline](database_migration_pipeline.md) to rerun a particular batch with additional logging and/or fix to see if it solves the problem. - -<!-- vale gitlab.Substitutions = NO --> - -For an example see [Draft: Test PG::CardinalityViolation fix](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/110910) but make sure to read the entire section. - -To do that, you need to: - -1. Find the batch `start_id` and `end_id` -1. Create a regular migration -1. Apply a workaround for our migration helpers (optional) -1. Start the database migration pipeline - -#### 1. Find the batch `start_id` and `end_id` - -You should be able to find those in [Kibana](#viewing-failure-error-logs). - -#### 2. Create a regular migration - -Schedule the batch in the `up` block of a regular migration: - -```ruby -def up - instance = Gitlab::BackgroundMigration::YourBackgroundMigrationClass.new( - start_id: <batch start_id>, - end_id: <batch end_id>, - batch_table: <table name>, - batch_column: <batching column>, - sub_batch_size: <sub batch size>, - pause_ms: <miliseconds between batches>, - job_arguments: <job arguments if any>, - connection: connection - ) - - instance.perform -end - - -def down - # no-op -end -``` - -#### 3. Apply a workaround for our migration helpers (optional) - -If your batched background migration touches tables from a schema other than the one you specified by using `restrict_gitlab_migration` helper (example: the scheduling migration has `restrict_gitlab_migration gitlab_schema: :gitlab_main` but the background job uses tables from the `:gitlab_ci` schema) then the migration will fail. To prevent that from happening you must to monkey patch database helpers so they don't fail the testing pipeline job: - -1. Add the schema names to [`RestrictGitlabSchema`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/database/migration_helpers/restrict_gitlab_schema.rb#L57) - -```diff -diff --git a/lib/gitlab/database/migration_helpers/restrict_gitlab_schema.rb b/lib/gitlab/database/migration_helpers/restrict_gitlab_schema.rb -index b8d1d21a0d2d2a23d9e8c8a0a17db98ed1ed40b7..912e20659a6919f771045178c66828563cb5a4a1 100644 ---- a/lib/gitlab/database/migration_helpers/restrict_gitlab_schema.rb -+++ b/lib/gitlab/database/migration_helpers/restrict_gitlab_schema.rb -@@ -55,7 +55,7 @@ def unmatched_schemas - end - - def allowed_schemas_for_connection -- Gitlab::Database.gitlab_schemas_for_connection(connection) -+ Gitlab::Database.gitlab_schemas_for_connection(connection) << :gitlab_ci - end - end - end -``` - -1. Add the schema names to [`RestrictAllowedSchemas`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/database/query_analyzers/restrict_allowed_schemas.rb#L82) - -```diff -diff --git a/lib/gitlab/database/query_analyzers/restrict_allowed_schemas.rb b/lib/gitlab/database/query_analyzers/restrict_allowed_schemas.rb -index 4ae3622479f0800c0553959e132143ec9051898e..d556ec7f55adae9d46a56665ce02de782cb09f2d 100644 ---- a/lib/gitlab/database/query_analyzers/restrict_allowed_schemas.rb -+++ b/lib/gitlab/database/query_analyzers/restrict_allowed_schemas.rb -@@ -79,7 +79,7 @@ def restrict_to_dml_only(parsed) - tables = self.dml_tables(parsed) - schemas = self.dml_schemas(tables) - -- if (schemas - self.allowed_gitlab_schemas).any? -+ if (schemas - (self.allowed_gitlab_schemas << :gitlab_ci)).any? - raise DMLAccessDeniedError, \ - "Select/DML queries (SELECT/UPDATE/DELETE) do access '#{tables}' (#{schemas.to_a}) " \ - "which is outside of list of allowed schemas: '#{self.allowed_gitlab_schemas}'. " \ -``` - -#### 4. Start the database migration pipeline - -Create a Draft merge request with your changes and trigger the manual `db:gitlabcom-database-testing` job. - -### Adding indexes to support batched background migrations - -Sometimes it is necessary to add a new or temporary index to support a batched background migration. -To do this, create the index in a post-deployment migration that precedes the post-deployment -migration that queues the background migration. - -See the documentation for [adding database indexes](adding_database_indexes.md#analyzing-a-new-index-before-a-batched-background-migration) -for additional information about some cases that require special attention to allow the index to be used directly after -creation. - -## Legacy background migrations - -Batched background migrations replaced the legacy background migrations framework. -Check that documentation in reference to any changes involving that framework. diff --git a/doc/development/features_inside_dot_gitlab.md b/doc/development/features_inside_dot_gitlab.md index f55d7e52fbc..b7eda423584 100644 --- a/doc/development/features_inside_dot_gitlab.md +++ b/doc/development/features_inside_dot_gitlab.md @@ -17,3 +17,5 @@ When implementing new features, please refer to these existing features to avoid - [Customize Auto DevOps Helm Values](../topics/autodevops/customize.md#customize-helm-chart-values): `.gitlab/auto-deploy-values.yaml`. - [Insights](../user/project/insights/index.md#configure-project-insights): `.gitlab/insights.yml`. - [Service Desk Templates](../user/project/service_desk.md#customize-emails-sent-to-the-requester): `.gitlab/service_desk_templates/`. +- [Secret Detection Custom Rulesets](../user/application_security/secret_detection/index.md#disable-predefined-analyzer-rules): `.gitlab/secret-detection-ruleset.toml` +- [Static Analysis Custom Rulesets](../user/application_security/sast/customize_rulesets.md#create-the-configuration-file): `.gitlab/sast-ruleset.toml` diff --git a/doc/topics/offline/quick_start_guide.md b/doc/topics/offline/quick_start_guide.md index dd739fdaf77..82a88b53dcf 100644 --- a/doc/topics/offline/quick_start_guide.md +++ b/doc/topics/offline/quick_start_guide.md @@ -215,15 +215,22 @@ On offline instances, the [GitLab Geo check Rake task](../../administration/geo/ always fails because it uses `pool.ntp.org`. This error can be ignored but you can [read more about how to work around it](../../administration/geo/replication/troubleshooting.md#message-machine-clock-is-synchronized--exception). -## Enabling the package metadata database +## Enabling the Package Metadata Database -Enabling the package metadata database is required to enable [license scanning of CycloneDX files](../../user/compliance/license_scanning_of_cyclonedx_files). -This process requires usage of the GitLab License Database, which is licensed under the [EE License](https://storage.googleapis.com/prod-export-license-bucket-1a6c642fc4de57d4/v1/LICENSE). -Note the following in relation to use of the License Database: +Enabling the Package Metadata Database is required to enable [license scanning of CycloneDX files](../../user/compliance/license_scanning_of_cyclonedx_files). +This process requires the use of License and/or Advisory Data under what is collectively called the Package Metadata Database, which is licensed under the [EE License](https://storage.googleapis.com/prod-export-license-bucket-1a6c642fc4de57d4/LICENSE). +Note the following in relation to use of the Package Metadata Database: -- We may change or discontinue all or any part of the License Database, at any time and without notice, at our sole discretion. -- The License Database may contain links to third-party websites or resources. We provide these links only as a convenience and are not responsible for any third-party data, content, products, or services from those websites or resources or links displayed on such websites. -- The License Database is based in part on information made available by third parties, and GitLab is not responsible for the accuracy or completeness of content made available. +- We may change or discontinue all or any part of the Package Metadata Database, at any time and without notice, at our sole discretion. +- The Package Metadata Database may contain links to third-party websites or resources. We provide these links only as a convenience and are not responsible for any third-party data, content, products, or services from those websites or resources or links displayed on such websites. +- The Package Metadata Database is based in part on information made available by third parties, and GitLab is not responsible for the accuracy or completeness of content made available. + +Enabling the Package Metadata Database is also required to enable Continuous Vulnerability Scans for Dependency Scanning (see [epic 9534](https://gitlab.com/groups/gitlab-org/-/epics/9534) tracking this work for more info). + +Package metadata is stored in the following Google Cloud Provider (GCP) buckets: + +- License Scanning - prod-export-license-bucket-1a6c642fc4de57d4 +- Dependency Scanning - prod-export-advisory-bucket-1a6c642fc4de57d4 ### Using the gsutil tool to download the package metadata exports @@ -235,34 +242,58 @@ Note the following in relation to use of the License Database: echo $GITLAB_RAILS_ROOT_DIR ``` +1. Set the type of data you wish to sync. + + ```shell + # For License Scanning + export PKG_METADATA_BUCKET=prod-export-license-bucket-1a6c642fc4de57d4 + export DATA_DIR="licenses" + + # For Dependency Scanning + export PKG_METADATA_BUCKET=prod-export-advisory-bucket-1a6c642fc4de57d4 + export DATA_DIR="advisories" + ``` + 1. Download the package metadata exports. ```shell # To download the package metadata exports, an outbound connection to Google Cloud Storage bucket must be allowed. - mkdir $GITLAB_RAILS_ROOT_DIR/vendor/package_metadata_db/ - gsutil -m rsync -r -d gs://prod-export-license-bucket-1a6c642fc4de57d4 $GITLAB_RAILS_ROOT_DIR/vendor/package_metadata_db/ + mkdir -p "$GITLAB_RAILS_ROOT_DIR/vendor/package_metadata/$DATA_DIR" + gsutil -m rsync -r -d gs://$PKG_METADATA_BUCKET "$GITLAB_RAILS_ROOT_DIR/vendor/package_metadata/$DATA_DIR" # Alternatively, if the GitLab instance is not allowed to connect to the Google Cloud Storage bucket, the package metadata # exports can be downloaded using a machine with the allowed access, and then copied to the root of the GitLab Rails directory. - rsync rsync://example_username@gitlab.example.com/package_metadata_db $GITLAB_RAILS_ROOT_DIR/vendor/package_metadata_db/ + rsync rsync://example_username@gitlab.example.com/package_metadata/$DATA_DIR "$GITLAB_RAILS_ROOT_DIR/vendor/package_metadata/$DATA_DIR" ``` ### Using the Google Cloud Storage REST API to download the package metadata exports -The package metadata exports can also be downloaded using the Google Cloud Storage API. The contents are available at [https://storage.googleapis.com/storage/v1/b/prod-export-license-bucket-1a6c642fc4de57d4/o](https://storage.googleapis.com/storage/v1/b/prod-export-license-bucket-1a6c642fc4de57d4/o). The following is an example of how this can be downloaded using [cURL](https://curl.se/) and [jq](https://stedolan.github.io/jq/). +The package metadata exports can also be downloaded using the Google Cloud Storage API. The contents are available at [https://storage.googleapis.com/storage/v1/b/prod-export-license-bucket-1a6c642fc4de57d4/o](https://storage.googleapis.com/storage/v1/b/prod-export-license-bucket-1a6c642fc4de57d4/o) and [https://storage.googleapis.com/storage/v1/b/prod-export-advisory-bucket-1a6c642fc4de57d4/o](https://storage.googleapis.com/storage/v1/b/prod-export-advisory-bucket-1a6c642fc4de57d4/o). The following is an example of how this can be downloaded using [cURL](https://curl.se/) and [jq](https://stedolan.github.io/jq/). ```shell #!/bin/bash set -euo pipefail +DATA_TYPE=$1 + GITLAB_RAILS_ROOT_DIR="$(gitlab-rails runner 'puts Rails.root.to_s')" -PKG_METADATA_DIR="$GITLAB_RAILS_ROOT_DIR/vendor/package_metadata_db" -PKG_METADATA_MANIFEST_OUTPUT_FILE="/tmp/license_db_export_manifest.json" -PKG_METADATA_DOWNLOADS_OUTPUT_FILE="/tmp/license_db_object_links.tsv" + +if [ "$DATA_TYPE" == "license" ]; then + PKG_METADATA_DIR="$GITLAB_RAILS_ROOT_DIR/vendor/package_metadata/licenses" +elif [ "$DATA_TYPE" == "advisory" ]; then + PKG_METADATA_DIR="$GITLAB_RAILS_ROOT_DIR/vendor/package_metadata/advisories" +else + echo "Usage: import_script.sh [licenses|advisories]" + exit 1 +fi + +PKG_METADATA_BUCKET="prod-export-$DATA_TYPE-bucket-1a6c642fc4de57d4" +PKG_METADATA_MANIFEST_OUTPUT_FILE="/tmp/package_metadata_${DATA_TYPE}_export_manifest.json" +PKG_METADATA_DOWNLOADS_OUTPUT_FILE="/tmp/package_metadata_${DATA_TYPE}_object_links.tsv" # Download the contents of the bucket -curl --silent --show-error --request GET "https://storage.googleapis.com/storage/v1/b/prod-export-license-bucket-1a6c642fc4de57d4/o?maxResults=7500" > "$PKG_METADATA_MANIFEST_OUTPUT_FILE" +curl --silent --show-error --request GET "https://storage.googleapis.com/storage/v1/b/$PKG_METADATA_BUCKET/o?maxResults=7500" > "$PKG_METADATA_MANIFEST_OUTPUT_FILE" # Parse the links and names for the bucket objects and output them into a tsv file jq -r '.items[] | [.name, .mediaLink] | @tsv' "$PKG_METADATA_MANIFEST_OUTPUT_FILE" > "$PKG_METADATA_DOWNLOADS_OUTPUT_FILE" @@ -294,9 +325,29 @@ echo "All objects saved to $PKG_METADATA_DIR" ### Automatic synchronization -Your GitLab instance is synchronized [every hour](https://gitlab.com/gitlab-org/gitlab/-/blob/d4331343d26d6e2a81fadd8f7ecd72f7cb74d04d/config/initializers/1_settings.rb#L831-832) with the contents of the `package_metadata_db` directory. +Your GitLab instance is synchronized [regularly](https://gitlab.com/gitlab-org/gitlab/-/blob/63a187d47f6da353ba4514650bbbbeb99c356325/config/initializers/1_settings.rb#L840-842) with the contents of the `package_metadata` directory. To automatically update your local copy with the upstream changes, a cron job can be added to periodically download new exports. For example, the following crontabs can be added to setup a cron job that runs every 30 minutes. +For License Scanning: + +```plaintext +*/30 * * * * gsutil -m rsync -r -d gs://prod-export-license-bucket-1a6c642fc4de57d4 $GITLAB_RAILS_ROOT_DIR/vendor/package_metadata/licenses +``` + +For Dependency Scanning: + ```plaintext -*/30 * * * * gsutil -m rsync -r -d gs://prod-export-license-bucket-1a6c642fc4de57d4 $GITLAB_RAILS_ROOT_DIR/vendor/package_metadata_db/ +*/30 * * * * gsutil -m rsync -r -d gs://prod-export-advisory-bucket-1a6c642fc4de57d4 $GITLAB_RAILS_ROOT_DIR/vendor/package_metadata/advisories ``` + +### Change note + +The directory for package metadata changed with the release of 16.2 from `vendor/package_metadata_db` to `vendor/package_metadata/licenses`. If this directory already exists on the instance and Dependency Scanning needs to be added then you need to take the following steps. + +1. Rename the licenses directory: `mv vendor/package_metadata_db vendor/package_metadata/licenses`. +1. Update any automation scripts or commands saved to change `vendor/package_metadata_db` to `vendor/package_metadata/licenses`. +1. Update any cron entries to change `vendor/package_metadata_db` to `vendor/package_metadata/licenses`. + + ```shell + sed -i '.bckup' -e 's#vendor/package_metadata_db#vendor/package_metadata/licenses#g' [FILE ...] + ``` diff --git a/doc/user/analytics/img/dora_performers_score_panel_v16_3.png b/doc/user/analytics/img/dora_performers_score_panel_v16_3.png Binary files differnew file mode 100644 index 00000000000..4193788aba7 --- /dev/null +++ b/doc/user/analytics/img/dora_performers_score_panel_v16_3.png diff --git a/doc/user/analytics/value_streams_dashboard.md b/doc/user/analytics/value_streams_dashboard.md index a853263d20f..39975476778 100644 --- a/doc/user/analytics/value_streams_dashboard.md +++ b/doc/user/analytics/value_streams_dashboard.md @@ -44,6 +44,18 @@ that are the largest value contributors, overperforming, or underperforming. You can also drill down the metrics for further analysis. When you hover over a metric, a tooltip displays an explanation of the metric and a link to the related documentation page. +## DORA Performers score panel + +The DORA Performers score panel is a bar chart that visualizes the status of the organization's DevOps performance levels across different projects. + +The chart is a breakdown of your project's DORA scores, categorized as high, medium, or low. +Each bar on the chart displays the sum of total projects per score category, calculated monthly. +Hovering over each bar reveals a dialog that explains the score's definition. + +For example, if a project has a high score for Deployment Frequency (Velocity), it means that the project has one or more deploys to production per day. + + + ## View the value streams dashboard Prerequisite: diff --git a/doc/user/project/repository/branches/default.md b/doc/user/project/repository/branches/default.md index ae978e2123d..6dc12c3f4f7 100644 --- a/doc/user/project/repository/branches/default.md +++ b/doc/user/project/repository/branches/default.md @@ -94,7 +94,7 @@ Users with the Owner role of groups and subgroups can configure the default bran Projects created in this group after you change the setting use the custom branch name, unless a subgroup configuration overrides it. -## Protect initial default branches **(FREE SELF)** +## Protect initial default branches **(FREE)** > Full protection after initial push [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/118729) in GitLab 16.0. diff --git a/lib/api/projects.rb b/lib/api/projects.rb index 468f284f136..dfd6cb2441a 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -792,10 +792,12 @@ module API desc 'Import members from another project' do detail 'This feature was introduced in GitLab 14.2' - success code: 201 + success code: 200 failure [ { code: 403, message: 'Unauthenticated' }, - { code: 404, message: 'Not found' } + { code: 403, message: 'Forbidden - Project' }, + { code: 404, message: 'Project Not Found' }, + { code: 422, message: 'Import failed' } ] tags %w[projects] end @@ -812,10 +814,12 @@ module API result = ::Members::ImportProjectTeamService.new(current_user, params).execute - if result - { status: result, message: 'Successfully imported' } + if result.success? + { status: result.status } + elsif result.reason == :unprocessable_entity + render_api_error!(result.message, result.reason) else - render_api_error!('Import failed', :unprocessable_entity) + { status: result.status, message: result.message, total_members_count: result.payload[:total_members_count] } end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index a73a86dfaef..e7920c8e762 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -14016,6 +14016,11 @@ msgstr "" msgid "DNS" msgstr "" +msgid "DORA4Metrics|%d project" +msgid_plural "DORA4Metrics|%d projects" +msgstr[0] "" +msgstr[1] "" + msgid "DORA4Metrics|Accept testing terms" msgstr "" @@ -14037,6 +14042,9 @@ msgstr "" msgid "DORA4Metrics|Change Failure Rate" msgstr "" +msgid "DORA4Metrics|Change Failure Rate (Quality)" +msgstr "" + msgid "DORA4Metrics|Change failure rate" msgstr "" @@ -14064,12 +14072,18 @@ msgstr "" msgid "DORA4Metrics|Deployment Frequency" msgstr "" +msgid "DORA4Metrics|Deployment Frequency (Velocity)" +msgstr "" + msgid "DORA4Metrics|Deployment frequency" msgstr "" msgid "DORA4Metrics|Deploys" msgstr "" +msgid "DORA4Metrics|Failed to load DORA performance scores for Namespace: %{fullPath}" +msgstr "" + msgid "DORA4Metrics|Failed to load YAML config from Project: %{fullPath}" msgstr "" @@ -14091,12 +14105,32 @@ msgstr "" msgid "DORA4Metrics|Go to docs" msgstr "" +msgid "DORA4Metrics|Has no calculated data." +msgid_plural "DORA4Metrics|Have no calculated data." +msgstr[0] "" +msgstr[1] "" + +msgid "DORA4Metrics|Have 30 or more deploys to production per day." +msgstr "" + +msgid "DORA4Metrics|Have between 1 to 29 deploys to production per day." +msgstr "" + +msgid "DORA4Metrics|Have less than 1 deploy to production per day." +msgstr "" + +msgid "DORA4Metrics|High" +msgstr "" + msgid "DORA4Metrics|High Vulnerabilities over time" msgstr "" msgid "DORA4Metrics|Lead Time for Changes" msgstr "" +msgid "DORA4Metrics|Lead Time for Changes (Velocity)" +msgstr "" + msgid "DORA4Metrics|Lead time" msgstr "" @@ -14106,6 +14140,18 @@ msgstr "" msgid "DORA4Metrics|Lead time for changes (median days)" msgstr "" +msgid "DORA4Metrics|Low" +msgstr "" + +msgid "DORA4Metrics|Made 15%% or less changes to production resulted in degraded service." +msgstr "" + +msgid "DORA4Metrics|Made between 16%% to 44%% of changes to production resulted in degraded service." +msgstr "" + +msgid "DORA4Metrics|Made more than 45%% of changes to production resulted in degraded service." +msgstr "" + msgid "DORA4Metrics|Median (last %{days}d)" msgstr "" @@ -14115,6 +14161,9 @@ msgstr "" msgid "DORA4Metrics|Median time an incident was open in a production environment over the given time period." msgstr "" +msgid "DORA4Metrics|Medium" +msgstr "" + msgid "DORA4Metrics|Merge request throughput" msgstr "" @@ -14130,12 +14179,18 @@ msgstr "" msgid "DORA4Metrics|New issues" msgstr "" +msgid "DORA4Metrics|No data available for Namespace: %{fullPath}" +msgstr "" + msgid "DORA4Metrics|No incidents during this period" msgstr "" msgid "DORA4Metrics|No merge requests were deployed during this period" msgstr "" +msgid "DORA4Metrics|Not included" +msgstr "" + msgid "DORA4Metrics|Number of incidents divided by the number of deployments to a production environment in the given time period." msgstr "" @@ -14175,15 +14230,45 @@ msgstr "" msgid "DORA4Metrics|This is a lower-bound approximation. Your group has too many issues and MRs to calculate in real time." msgstr "" +msgid "DORA4Metrics|This visualization is not supported for project namespaces." +msgstr "" + msgid "DORA4Metrics|Time to Restore Service" msgstr "" +msgid "DORA4Metrics|Time to Restore Service (Quality)" +msgstr "" + msgid "DORA4Metrics|Time to restore service" msgstr "" msgid "DORA4Metrics|Time to restore service (median days)" msgstr "" +msgid "DORA4Metrics|Took 1 day or less to restore service when a service incident or a defect that impacts users occurs." +msgstr "" + +msgid "DORA4Metrics|Took 7 days or less to go from code committed to code successfully running in production." +msgstr "" + +msgid "DORA4Metrics|Took between 2 to 6 days to restore service when a service incident or a defect that impacts users occurs." +msgstr "" + +msgid "DORA4Metrics|Took between 8 to 29 days to go from code committed to code successfully running in production." +msgstr "" + +msgid "DORA4Metrics|Took more than 30 days to go from code committed to code successfully running in production." +msgstr "" + +msgid "DORA4Metrics|Took more than 7 days to restore service when a service incident or a defect that impacts users occurs." +msgstr "" + +msgid "DORA4Metrics|Total projects (%{count}) by DORA performers score for %{groupName} group" +msgstr "" + +msgid "DORA4Metrics|Total projects by DORA performers score" +msgstr "" + msgid "DORA4Metrics|Value Streams Dashboard" msgstr "" @@ -25115,6 +25200,9 @@ msgstr "" msgid "InviteMembersModal|Something went wrong" msgstr "" +msgid "InviteMembersModal|The following %{errorMembersLength} out of %{totalMembersCount} members could not be added" +msgstr "" + msgid "InviteMembersModal|The following member couldn't be invited" msgid_plural "InviteMembersModal|The following %d members couldn't be invited" msgstr[0] "" @@ -30634,6 +30722,9 @@ msgstr "" msgid "No credit card required." msgstr "" +msgid "No data" +msgstr "" + msgid "No data available" msgstr "" diff --git a/qa/qa/page/file/show.rb b/qa/qa/page/file/show.rb index 173baa21160..284fab58d5d 100644 --- a/qa/qa/page/file/show.rb +++ b/qa/qa/page/file/show.rb @@ -12,7 +12,7 @@ module QA element :lock_button end - view 'app/assets/javascripts/vue_shared/components/actions_button.vue' do + view 'app/assets/javascripts/vue_shared/components/web_ide_link.vue' do element :action_dropdown element :edit_menu_item, ':data-qa-selector="`${action.key}_menu_item`"' # rubocop:disable QA/ElementWithPattern element :webide_menu_item, ':data-qa-selector="`${action.key}_menu_item`"' # rubocop:disable QA/ElementWithPattern diff --git a/spec/features/projects/members/import_project_members_spec.rb b/spec/features/projects/members/import_project_members_spec.rb new file mode 100644 index 00000000000..20cf42cd135 --- /dev/null +++ b/spec/features/projects/members/import_project_members_spec.rb @@ -0,0 +1,79 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Projects > Members > Import project members', :js, feature_category: :groups_and_projects do + include Features::MembersHelpers + include ListboxHelpers + + let_it_be(:user) { create(:user) } + let_it_be(:user_mike) { create(:user, name: 'Mike') } + let_it_be(:group) { create(:group) } + let_it_be(:project) do + create(:project, group: group).tap do |p| + p.add_maintainer(user) + p.add_developer(create(:user)) + end + end + + let_it_be(:project2) do + create(:project).tap do |p| + p.add_maintainer(user) + p.add_reporter(user_mike) + end + end + + before do + sign_in(user) + + visit(project_project_members_path(project)) + end + + it 'imports a team from another project' do + select_project(project2) + submit_import + + expect(find_member_row(user_mike)).to have_content('Reporter') + end + + it 'fails to import the other team when source project does not exist' do + select_project(project2) + submit_import { project2.destroy! } + + within import_project_members_modal_selector do + expect(page).to have_content('Unable to import project members') + end + end + + it 'fails to import some members' do + group.add_owner(user_mike) + + select_project(project2) + submit_import + + within import_project_members_modal_selector do + expect(page).to have_content "The following 1 out of 2 members could not be added" + expect(page).to have_content "@#{user_mike.username}: Access level should be greater than or equal to " \ + "Owner inherited membership from group #{group.name}" + end + end + + def select_project(source_project) + click_on 'Import from a project' + click_on 'Select a project' + wait_for_requests + + select_listbox_item(source_project.name_with_namespace) + end + + def submit_import + yield if block_given? # rubocop:disable RSpec/AvoidConditionalStatements + + click_button 'Import project members' + wait_for_requests + end + + def import_project_members_modal_selector + '[data-testid="import-project-members-modal"]' + end +end diff --git a/spec/features/projects/settings/user_manages_project_members_spec.rb b/spec/features/projects/members/user_manages_project_members_spec.rb index df571e13979..b1c3132767c 100644 --- a/spec/features/projects/settings/user_manages_project_members_spec.rb +++ b/spec/features/projects/members/user_manages_project_members_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Projects > Settings > User manages project members', feature_category: :groups_and_projects do +RSpec.describe 'Projects > Settings > User manages project members', :js, feature_category: :groups_and_projects do include Features::MembersHelpers include Spec::Support::Helpers::ModalHelpers include ListboxHelpers @@ -20,7 +20,7 @@ RSpec.describe 'Projects > Settings > User manages project members', feature_cat sign_in(user) end - it 'cancels a team member', :js do + it 'cancels a team member' do visit(project_project_members_path(project)) show_actions_for_username(user_dmitriy) @@ -37,24 +37,7 @@ RSpec.describe 'Projects > Settings > User manages project members', feature_cat expect(members_table).not_to have_content(user_dmitriy.username) end - it 'imports a team from another project', :js do - project2.add_maintainer(user) - project2.add_reporter(user_mike) - - visit(project_project_members_path(project)) - - click_on 'Import from a project' - click_on 'Select a project' - wait_for_requests - - select_listbox_item(project2.name_with_namespace) - click_button 'Import project members' - wait_for_requests - - expect(find_member_row(user_mike)).to have_content('Reporter') - end - - it 'shows all members of project shared group', :js do + it 'shows all members of project shared group' do group.add_owner(user) group.add_developer(user_dmitriy) diff --git a/spec/frontend/invite_members/components/import_project_members_modal_spec.js b/spec/frontend/invite_members/components/import_project_members_modal_spec.js index 224ebe18e2a..4fac21f7a57 100644 --- a/spec/frontend/invite_members/components/import_project_members_modal_spec.js +++ b/spec/frontend/invite_members/components/import_project_members_modal_spec.js @@ -1,4 +1,4 @@ -import { GlFormGroup, GlSprintf, GlModal } from '@gitlab/ui'; +import { GlFormGroup, GlSprintf, GlModal, GlCollapse, GlIcon } from '@gitlab/ui'; import MockAdapter from 'axios-mock-adapter'; import { nextTick } from 'vue'; import { createWrapper } from '@vue/test-utils'; @@ -12,6 +12,7 @@ import eventHub from '~/invite_members/event_hub'; import ImportProjectMembersModal from '~/invite_members/components/import_project_members_modal.vue'; import ProjectSelect from '~/invite_members/components/project_select.vue'; import axios from '~/lib/utils/axios_utils'; +import { HTTP_STATUS_CREATED } from '~/lib/utils/http_status'; import { displaySuccessfulInvitationAlert, @@ -19,9 +20,14 @@ import { } from '~/invite_members/utils/trigger_successful_invite_alert'; import { + EXPANDED_ERRORS, IMPORT_PROJECT_MEMBERS_MODAL_TRACKING_CATEGORY, IMPORT_PROJECT_MEMBERS_MODAL_TRACKING_LABEL, } from '~/invite_members/constants'; +import { + IMPORT_PROJECT_MEMBERS_PATH, + importProjectMembersApiResponse, +} from '../mock_data/api_responses'; jest.mock('~/invite_members/utils/trigger_successful_invite_alert'); @@ -92,6 +98,14 @@ describe('ImportProjectMembersModal', () => { const formGroupInvalidFeedback = () => findFormGroup().props('invalidFeedback'); const formGroupErrorState = () => findFormGroup().props('state'); const findProjectSelect = () => wrapper.findComponent(ProjectSelect); + const findMemberErrorAlert = () => wrapper.findByTestId('alert-member-error'); + const findMoreInviteErrorsButton = () => wrapper.findByTestId('accordion-button'); + const findAccordion = () => wrapper.findComponent(GlCollapse); + const findErrorsIcon = () => wrapper.findComponent(GlIcon); + const findMemberErrorMessage = (element) => + `@${Object.keys(importProjectMembersApiResponse.EXPANDED_IMPORT_ERRORS.message)[element]}: ${ + Object.values(importProjectMembersApiResponse.EXPANDED_IMPORT_ERRORS.message)[element] + }`; describe('rendering the modal', () => { beforeEach(() => { @@ -241,7 +255,7 @@ describe('ImportProjectMembersModal', () => { }); }); - describe('when the import fails', () => { + describe('when the import fails due to generic api error', () => { beforeEach(async () => { createComponent(); @@ -276,5 +290,82 @@ describe('ImportProjectMembersModal', () => { expect(formGroupErrorState()).not.toBe(false); }); }); + + describe('when the import fails with member import errors', () => { + const mockInvitationsApi = (code, data) => { + mock.onPost(IMPORT_PROJECT_MEMBERS_PATH).reply(code, data); + }; + + beforeEach(() => { + createComponent(); + findProjectSelect().vm.$emit('input', projectToBeImported); + }); + + it('displays the error alert', async () => { + mockInvitationsApi( + HTTP_STATUS_CREATED, + importProjectMembersApiResponse.NO_COLLAPSE_IMPORT_ERRORS, + ); + + clickImportButton(); + await waitForPromises(); + + expect(findMemberErrorAlert().props('title')).toContain( + 'The following 2 out of 2 members could not be added', + ); + expect(findMemberErrorAlert().text()).toContain(findMemberErrorMessage(0)); + expect(findMemberErrorAlert().text()).toContain(findMemberErrorMessage(1)); + }); + + it('displays collapse when there are more than 2 errors', async () => { + mockInvitationsApi( + HTTP_STATUS_CREATED, + importProjectMembersApiResponse.EXPANDED_IMPORT_ERRORS, + ); + + clickImportButton(); + await waitForPromises(); + + expect(findAccordion().exists()).toBe(true); + expect(findMoreInviteErrorsButton().text()).toContain('Show more (2)'); + }); + + it('toggles the collapse on click', async () => { + mockInvitationsApi( + HTTP_STATUS_CREATED, + importProjectMembersApiResponse.EXPANDED_IMPORT_ERRORS, + ); + + clickImportButton(); + await waitForPromises(); + + expect(findMoreInviteErrorsButton().text()).toContain('Show more (2)'); + expect(findErrorsIcon().attributes('class')).not.toContain('gl-rotate-180'); + expect(findAccordion().attributes('visible')).toBeUndefined(); + + await findMoreInviteErrorsButton().vm.$emit('click'); + + expect(findMoreInviteErrorsButton().text()).toContain(EXPANDED_ERRORS); + expect(findErrorsIcon().attributes('class')).toContain('gl-rotate-180'); + expect(findAccordion().attributes('visible')).toBeDefined(); + + await findMoreInviteErrorsButton().vm.$emit('click'); + + expect(findMoreInviteErrorsButton().text()).toContain('Show more (2)'); + }); + + it("doesn't display collapse when there are 2 or less errors", async () => { + mockInvitationsApi( + HTTP_STATUS_CREATED, + importProjectMembersApiResponse.NO_COLLAPSE_IMPORT_ERRORS, + ); + + clickImportButton(); + await waitForPromises(); + + expect(findAccordion().exists()).toBe(false); + expect(findMoreInviteErrorsButton().exists()).toBe(false); + }); + }); }); }); diff --git a/spec/frontend/invite_members/mock_data/api_responses.js b/spec/frontend/invite_members/mock_data/api_responses.js index 6fe06decb6b..e3e2426fcfc 100644 --- a/spec/frontend/invite_members/mock_data/api_responses.js +++ b/spec/frontend/invite_members/mock_data/api_responses.js @@ -57,3 +57,27 @@ export const invitationsApiResponse = { EMAIL_TAKEN, EXPANDED_RESTRICTED, }; + +export const IMPORT_PROJECT_MEMBERS_PATH = '/api/v4/projects/1/import_project_members/2'; +const EXPANDED_IMPORT_ERRORS = { + message: { + bob_smith: 'Something is wrong for this member.', + john_smith: 'Something is wrong for this member.', + doug_logan: 'Something is wrong for this member.', + root: 'Something is wrong for this member.', + }, + total_members_count: '4', + status: 'error', +}; +const NO_COLLAPSE_IMPORT_ERRORS = { + message: { + bob_smith: 'Something is wrong for this member.', + john_smith: 'Something is wrong for this member.', + }, + total_members_count: '2', + status: 'error', +}; +export const importProjectMembersApiResponse = { + EXPANDED_IMPORT_ERRORS, + NO_COLLAPSE_IMPORT_ERRORS, +}; diff --git a/spec/frontend/vue_shared/components/actions_button_spec.js b/spec/frontend/vue_shared/components/actions_button_spec.js deleted file mode 100644 index 9f9a27c6997..00000000000 --- a/spec/frontend/vue_shared/components/actions_button_spec.js +++ /dev/null @@ -1,119 +0,0 @@ -import { - GlDisclosureDropdown, - GlDisclosureDropdownGroup, - GlDisclosureDropdownItem, -} from '@gitlab/ui'; -import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; -import ActionsButton from '~/vue_shared/components/actions_button.vue'; - -const TEST_ACTION = { - key: 'action1', - text: 'Sample', - secondaryText: 'Lorem ipsum.', - href: '/sample', - attrs: { - 'data-test': '123', - category: 'secondary', - href: '/sample', - variant: 'default', - }, - handle: jest.fn(), -}; -const TEST_ACTION_2 = { - key: 'action2', - text: 'Sample 2', - secondaryText: 'Dolar sit amit.', - href: '#', - attrs: { 'data-test': '456' }, - handle: jest.fn(), -}; - -describe('vue_shared/components/actions_button', () => { - let wrapper; - - function createComponent({ props = {}, slots = {} } = {}) { - wrapper = shallowMountExtended(ActionsButton, { - propsData: { actions: [TEST_ACTION, TEST_ACTION_2], toggleText: 'Edit', ...props }, - stubs: { - GlDisclosureDropdownItem, - }, - slots, - }); - } - const findDropdown = () => wrapper.findComponent(GlDisclosureDropdown); - - it('dropdown toggle displays provided toggleLabel', () => { - createComponent(); - - expect(findDropdown().props().toggleText).toBe('Edit'); - }); - - it('dropdown has a fluid width', () => { - createComponent(); - - expect(findDropdown().props().fluidWidth).toBe(true); - }); - - it('provides a default slot', () => { - const slotContent = 'default text'; - - createComponent({ - slots: { - default: slotContent, - }, - }); - - expect(findDropdown().text()).toContain(slotContent); - }); - - it('allows customizing variant and category', () => { - const variant = 'confirm'; - const category = 'secondary'; - - createComponent({ props: { variant, category } }); - - expect(findDropdown().props()).toMatchObject({ category, variant }); - }); - - it('displays a single dropdown group', () => { - createComponent(); - - expect(wrapper.findAllComponents(GlDisclosureDropdownGroup)).toHaveLength(1); - }); - - it('create dropdown items for every action', () => { - createComponent(); - - [TEST_ACTION, TEST_ACTION_2].forEach((action, index) => { - const dropdownItem = wrapper.findAllComponents(GlDisclosureDropdownItem).at(index); - - expect(dropdownItem.props().item).toBe(action); - expect(dropdownItem.attributes()).toMatchObject(action.attrs); - expect(dropdownItem.text()).toContain(action.text); - expect(dropdownItem.text()).toContain(action.secondaryText); - }); - }); - - describe('when clicking a dropdown item', () => { - it("invokes the action's handle method", () => { - createComponent(); - - [TEST_ACTION, TEST_ACTION_2].forEach((action, index) => { - const dropdownItem = wrapper.findAllComponents(GlDisclosureDropdownItem).at(index); - - dropdownItem.vm.$emit('action'); - - expect(action.handle).toHaveBeenCalled(); - }); - }); - }); - - it.each(['shown', 'hidden'])( - 'bubbles up %s event from the disclosure dropdown component', - (event) => { - createComponent(); - findDropdown().vm.$emit(event); - expect(wrapper.emitted(event)).toHaveLength(1); - }, - ); -}); diff --git a/spec/frontend/vue_shared/components/web_ide_link_spec.js b/spec/frontend/vue_shared/components/web_ide_link_spec.js index b6c22ceaa23..063f1d793ef 100644 --- a/spec/frontend/vue_shared/components/web_ide_link_spec.js +++ b/spec/frontend/vue_shared/components/web_ide_link_spec.js @@ -1,14 +1,17 @@ -import { GlModal } from '@gitlab/ui'; +import { GlModal, GlDisclosureDropdown, GlDisclosureDropdownItem } from '@gitlab/ui'; import Vue, { nextTick } from 'vue'; import VueApollo from 'vue-apollo'; import getWritableForksResponse from 'test_fixtures/graphql/vue_shared/components/web_ide/get_writable_forks.query.graphql_none.json'; -import ActionsButton from '~/vue_shared/components/actions_button.vue'; import WebIdeLink, { i18n } from '~/vue_shared/components/web_ide_link.vue'; import ConfirmForkModal from '~/vue_shared/components/web_ide/confirm_fork_modal.vue'; import { stubComponent } from 'helpers/stub_component'; -import { shallowMountExtended, mountExtended } from 'helpers/vue_test_utils_helper'; +import { + shallowMountExtended, + mountExtended, + extendedWrapper, +} from 'helpers/vue_test_utils_helper'; import createMockApollo from 'helpers/mock_apollo_helper'; import { visitUrl } from '~/lib/utils/url_utility'; @@ -26,11 +29,11 @@ const forkPath = '/some/fork/path'; const ACTION_EDIT = { href: TEST_EDIT_URL, - key: 'edit', + handle: undefined, text: 'Edit single file', secondaryText: 'Edit this file only.', attrs: { - 'data-qa-selector': 'edit_button', + 'data-qa-selector': 'edit_menu_item', 'data-track-action': 'click_consolidated_edit', 'data-track-label': 'edit', }, @@ -41,14 +44,14 @@ const ACTION_EDIT_CONFIRM_FORK = { handle: expect.any(Function), }; const ACTION_WEB_IDE = { - key: 'webide', secondaryText: i18n.webIdeText, text: 'Web IDE', attrs: { - 'data-qa-selector': 'web_ide_button', + 'data-qa-selector': 'webide_menu_item', 'data-track-action': 'click_consolidated_edit_ide', 'data-track-label': 'web_ide', }, + href: undefined, handle: expect.any(Function), }; const ACTION_WEB_IDE_CONFIRM_FORK = { @@ -58,11 +61,11 @@ const ACTION_WEB_IDE_CONFIRM_FORK = { const ACTION_WEB_IDE_EDIT_FORK = { ...ACTION_WEB_IDE, text: 'Edit fork in Web IDE' }; const ACTION_GITPOD = { href: TEST_GITPOD_URL, - key: 'gitpod', + handle: undefined, secondaryText: 'Launch a ready-to-code development environment for your project.', text: 'Gitpod', attrs: { - 'data-qa-selector': 'gitpod_button', + 'data-qa-selector': 'gitpod_menu_item', }, }; const ACTION_GITPOD_ENABLE = { @@ -72,11 +75,12 @@ const ACTION_GITPOD_ENABLE = { }; const ACTION_PIPELINE_EDITOR = { href: TEST_PIPELINE_EDITOR_URL, - key: 'pipeline_editor', secondaryText: 'Edit, lint, and visualize your pipeline.', text: 'Edit in pipeline editor', attrs: { - 'data-qa-selector': 'pipeline_editor_button', + 'data-qa-selector': 'pipeline_editor_menu_item', + 'data-track-action': 'click_consolidated_pipeline_editor', + 'data-track-label': 'pipeline_editor', }, }; @@ -108,14 +112,34 @@ describe('vue_shared/components/web_ide_link', () => { <slot name="modal-footer"></slot> </div>`, }), + GlDisclosureDropdownItem, }, apolloProvider: fakeApollo, }); } - const findActionsButton = () => wrapper.findComponent(ActionsButton); + const findDisclosureDropdown = () => wrapper.findComponent(GlDisclosureDropdown); + const findDisclosureDropdownItems = () => wrapper.findAllComponents(GlDisclosureDropdownItem); const findModal = () => wrapper.findComponent(GlModal); const findForkConfirmModal = () => wrapper.findComponent(ConfirmForkModal); + const getDropdownItemsAsData = () => + findDisclosureDropdownItems().wrappers.map((item) => { + const extendedWrapperItem = extendedWrapper(item); + const attributes = extendedWrapperItem.attributes(); + const props = extendedWrapperItem.props(); + + return { + text: extendedWrapperItem.findByTestId('action-primary-text').text(), + secondaryText: extendedWrapperItem.findByTestId('action-secondary-text').text(), + href: props.item.href, + handle: props.item.handle, + attrs: { + 'data-qa-selector': attributes['data-qa-selector'], + 'data-track-action': attributes['data-track-action'], + 'data-track-label': attributes['data-track-label'], + }, + }; + }); it.each([ { @@ -210,7 +234,7 @@ describe('vue_shared/components/web_ide_link', () => { ])('renders actions with appropriately for given props', ({ props, expectedActions }) => { createComponent(props); - expect(findActionsButton().props('actions')).toEqual(expectedActions); + expect(getDropdownItemsAsData()).toEqual(expectedActions); }); it('bubbles up shown and hidden events triggered by actions button component', () => { @@ -219,17 +243,17 @@ describe('vue_shared/components/web_ide_link', () => { expect(wrapper.emitted('shown')).toBe(undefined); expect(wrapper.emitted('hidden')).toBe(undefined); - findActionsButton().vm.$emit('shown'); - findActionsButton().vm.$emit('hidden'); + findDisclosureDropdown().vm.$emit('shown'); + findDisclosureDropdown().vm.$emit('hidden'); expect(wrapper.emitted('shown')).toHaveLength(1); expect(wrapper.emitted('hidden')).toHaveLength(1); }); - it('exposes a default slot', () => { - const slotContent = 'default slot content'; + it.each(['before-actions', 'after-actions'])('exposes a %s slot', (slot) => { + const slotContent = 'slot content'; - createComponent({}, { slots: { default: slotContent } }); + createComponent({}, { slots: { [slot]: slotContent } }); expect(wrapper.text()).toContain(slotContent); }); @@ -248,13 +272,15 @@ describe('vue_shared/components/web_ide_link', () => { }); it('displays Pipeline Editor as the first action', () => { - expect(findActionsButton().props()).toMatchObject({ - actions: [ACTION_PIPELINE_EDITOR, ACTION_WEB_IDE, ACTION_GITPOD], - }); + expect(getDropdownItemsAsData()).toEqual([ + ACTION_PIPELINE_EDITOR, + ACTION_WEB_IDE, + ACTION_GITPOD, + ]); }); it('when web ide button is clicked it opens in a new tab', async () => { - findActionsButton().props('actions')[1].handle(); + findDisclosureDropdownItems().at(1).props().item.handle(); await nextTick(); expect(visitUrl).toHaveBeenCalledWith(TEST_WEB_IDE_URL, true); }); @@ -289,7 +315,7 @@ describe('vue_shared/components/web_ide_link', () => { ({ props, expectedEventPayload }) => { createComponent({ ...props, needsToFork: true, disableForkModal: true }); - findActionsButton().props('actions')[0].handle(); + findDisclosureDropdownItems().at(0).props().item.handle(); expect(wrapper.emitted('edit')).toEqual([[expectedEventPayload]]); }, @@ -309,7 +335,7 @@ describe('vue_shared/components/web_ide_link', () => { it.each(testActions)('opens the modal when the button is clicked', async ({ props }) => { createComponent({ ...props, needsToFork: true }, { mountFn: mountExtended }); - wrapper.findComponent(ActionsButton).props().actions[0].handle(); + findDisclosureDropdownItems().at(0).props().item.handle(); await nextTick(); await wrapper.findByRole('button', { name: /Web IDE|Edit/im }).trigger('click'); diff --git a/spec/models/project_team_spec.rb b/spec/models/project_team_spec.rb index f3139e72113..7df49fbe548 100644 --- a/spec/models/project_team_spec.rb +++ b/spec/models/project_team_spec.rb @@ -176,10 +176,11 @@ RSpec.describe ProjectTeam, feature_category: :groups_and_projects do let_it_be(:source_project_owner) { source_project.first_owner } let_it_be(:source_project_developer) { create(:user) { |user| source_project.add_developer(user) } } let_it_be(:current_user) { create(:user) { |user| target_project.add_maintainer(user) } } + let(:imported_members) { [source_project_owner.members.last, source_project_developer.members.last] } subject(:import) { target_project.team.import(source_project, current_user) } - it { is_expected.to be_truthy } + it { is_expected.to match(imported_members) } it 'target project includes source member with the same access' do import @@ -219,6 +220,12 @@ RSpec.describe ProjectTeam, feature_category: :groups_and_projects do let(:target_access_level) { Gitlab::Access::OWNER } end end + + context 'when source_project does not exist' do + let_it_be(:source_project) { nil } + + it { is_expected.to eq(false) } + end end describe '#find_member' do diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index f5d1bbbc7e8..1ff2adfcbcf 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -3654,7 +3654,7 @@ RSpec.describe API::Projects, :aggregate_failures, feature_category: :groups_and end.to change { project.members.count }.by(2) expect(response).to have_gitlab_http_status(:created) - expect(json_response['message']).to eq('Successfully imported') + expect(json_response['status']).to eq('success') end it 'returns 404 if the source project does not exist' do @@ -3712,6 +3712,22 @@ RSpec.describe API::Projects, :aggregate_failures, feature_category: :groups_and expect(response).to have_gitlab_http_status(:unprocessable_entity) expect(json_response['message']).to eq('Import failed') end + + context 'when importing of members did not work for some or all members' do + it 'fails to import some members' do + project_bot = create(:user, :project_bot) + project2.add_developer(project_bot) + + expect do + post api(path, user) + end.to change { project.members.count }.by(2) + + expect(response).to have_gitlab_http_status(:created) + error_message = { project_bot.username => 'User project bots cannot be added to other groups / projects' } + expect(json_response['message']).to eq(error_message) + expect(json_response['total_members_count']).to eq(3) + end + end end describe 'PUT /projects/:id' do diff --git a/spec/services/members/import_project_team_service_spec.rb b/spec/services/members/import_project_team_service_spec.rb index 7dcdb70f2cd..e0657cfa8cc 100644 --- a/spec/services/members/import_project_team_service_spec.rb +++ b/spec/services/members/import_project_team_service_spec.rb @@ -8,7 +8,10 @@ RSpec.describe Members::ImportProjectTeamService, feature_category: :groups_and_ let_it_be(:target_project) { create(:project) } let_it_be(:user) { create(:user) } - subject { described_class.new(user, { id: target_project_id, project_id: source_project_id }) } + let(:source_project_id) { source_project.id } + let(:target_project_id) { target_project.id } + + subject(:import) { described_class.new(user, { id: target_project_id, project_id: source_project_id }) } before_all do source_project.add_guest(user) @@ -16,74 +19,110 @@ RSpec.describe Members::ImportProjectTeamService, feature_category: :groups_and_ end context 'when project team members are imported successfully' do - let(:source_project_id) { source_project.id } - let(:target_project_id) { target_project.id } + it 'returns a successful response' do + result = import.execute - it 'returns true' do - expect(subject.execute).to be(true) + expect(result).to be_a(ServiceResponse) + expect(result.success?).to be(true) + expect(result.message).to eq('Successfully imported') end end context 'when the project team import fails' do context 'when the target project cannot be found' do - let(:source_project_id) { source_project.id } let(:target_project_id) { non_existing_record_id } - it 'returns false' do - expect(subject.execute).to be(false) + it 'returns unsuccessful response' do + result = import.execute + + expect(result).to be_a(ServiceResponse) + expect(result.error?).to be(true) + expect(result.message).to eq('Target project does not exist') + expect(result.reason).to eq(:unprocessable_entity) end end context 'when the source project cannot be found' do let(:source_project_id) { non_existing_record_id } - let(:target_project_id) { target_project.id } - it 'returns false' do - expect(subject.execute).to be(false) + it 'returns unsuccessful response' do + result = import.execute + + expect(result).to be_a(ServiceResponse) + expect(result.error?).to be(true) + expect(result.message).to eq('Source project does not exist') + expect(result.reason).to eq(:unprocessable_entity) end end context 'when the user doing the import does not exist' do let(:user) { nil } - let(:source_project_id) { source_project.id } - let(:target_project_id) { target_project.id } - it 'returns false' do - expect(subject.execute).to be(false) + it 'returns unsuccessful response' do + result = import.execute + + expect(result).to be_a(ServiceResponse) + expect(result.error?).to be(true) + expect(result.message).to eq('Forbidden') + expect(result.reason).to eq(:unprocessable_entity) end end context 'when the user does not have permission to read the source project members' do - let(:user) { create(:user) } let(:source_project_id) { create(:project, :private).id } - let(:target_project_id) { target_project.id } - it 'returns false' do - expect(subject.execute).to be(false) + it 'returns unsuccessful response' do + result = import.execute + + expect(result).to be_a(ServiceResponse) + expect(result.error?).to be(true) + expect(result.message).to eq('Forbidden') + expect(result.reason).to eq(:unprocessable_entity) end end context 'when the user does not have permission to admin the target project' do - let(:source_project_id) { source_project.id } let(:target_project_id) { create(:project).id } - it 'returns false' do - expect(subject.execute).to be(false) + it 'returns unsuccessful response' do + result = import.execute + + expect(result).to be_a(ServiceResponse) + expect(result.error?).to be(true) + expect(result.message).to eq('Forbidden') + expect(result.reason).to eq(:unprocessable_entity) end end context 'when the source and target project are valid but the ProjectTeam#import command fails' do - let(:source_project_id) { source_project.id } - let(:target_project_id) { target_project.id } - before do allow_next_instance_of(ProjectTeam) do |project_team| allow(project_team).to receive(:import).and_return(false) end end - it 'returns false' do - expect(subject.execute).to be(false) + it 'returns unsuccessful response' do + result = import.execute + + expect(result).to be_a(ServiceResponse) + expect(result.error?).to be(true) + expect(result.message).to eq('Import failed') + expect(result.reason).to eq(:unprocessable_entity) + end + end + + context 'when one of the imported project members is invalid' do + it 'returns unsuccessful response' do + project_bot = create(:user, :project_bot) + source_project.add_developer(project_bot) + + result = import.execute + + expect(result).to be_a(ServiceResponse) + expect(result.error?).to be(true) + message = { project_bot.username => 'User project bots cannot be added to other groups / projects' } + expect(result.message).to eq(message) + expect(result.payload[:total_members_count]).to eq(2) end end end diff --git a/spec/support/rspec_order_todo.yml b/spec/support/rspec_order_todo.yml index 3cce22c00e6..b9ddd4a7385 100644 --- a/spec/support/rspec_order_todo.yml +++ b/spec/support/rspec_order_todo.yml @@ -4149,7 +4149,7 @@ - './spec/features/projects/settings/user_changes_default_branch_spec.rb' - './spec/features/projects/settings/user_interacts_with_deploy_keys_spec.rb' - './spec/features/projects/settings/user_manages_merge_requests_settings_spec.rb' -- './spec/features/projects/settings/user_manages_project_members_spec.rb' +- './spec/features/projects/members/user_manages_project_members_spec.rb' - './spec/features/projects/settings/user_renames_a_project_spec.rb' - './spec/features/projects/settings/user_searches_in_settings_spec.rb' - './spec/features/projects/settings/user_sees_revoke_deploy_token_modal_spec.rb' |
