diff options
70 files changed, 1143 insertions, 263 deletions
diff --git a/.gitlab/CODEOWNERS b/.gitlab/CODEOWNERS index c63ad882487..153c15f99cd 100644 --- a/.gitlab/CODEOWNERS +++ b/.gitlab/CODEOWNERS @@ -270,6 +270,39 @@ Dangerfile @gl-quality/eng-prod /ee/spec/lib/gitlab/code_owners/ @reprazent @kerrizor @garyh /doc/user/project/code_owners.md @reprazent @kerrizor @garyh +[Merge Requests] +/app/controllers/projects/merge_requests/ @garyh @patrickbajao @marc_shaw @kerrizor +/app/models/merge_request.rb @dskim_gitlab @garyh @patrickbajao @marc_shaw @kerrizor +/app/services/merge_requests/ @dskim_gitlab @garyh @patrickbajao @marc_shaw @kerrizor +/app/workers/merge_requests/ @dskim_gitlab @garyh @patrickbajao @marc_shaw @kerrizor +/app/workers/merge_request_mergeability_check_worker.rb @dskim_gitlab @garyh @patrickbajao @marc_shaw @kerrizor +/lib/gitlab/diff/ @dskim_gitlab @garyh @patrickbajao @marc_shaw @kerrizor +/lib/gitlab/discussions_diff/ @dskim_gitlab @garyh @patrickbajao @marc_shaw @kerrizor +/lib/gitlab/quick_actions/ @dskim_gitlab @garyh @patrickbajao @marc_shaw @kerrizor + +/ee/app/models/merge_request.rb @dskim_gitlab @garyh @patrickbajao @marc_shaw @kerrizor +/ee/app/services/merge_requests/ @dskim_gitlab @garyh @patrickbajao @marc_shaw @kerrizor +/ee/app/workers/merge_requests/ @dskim_gitlab @garyh @patrickbajao @marc_shaw @kerrizor +/ee/app/workers/merge_request_reset_approvals_worker.rb @dskim_gitlab @garyh @patrickbajao @marc_shaw @kerrizor + +/app/assets/javascripts/diffs @viktomas @jboyson @iamphill @thomasrandolph +/app/assets/javascripts/batch_comments/ @viktomas @jboyson @iamphill @thomasrandolph +/app/assets/javascripts/notes @viktomas @jboyson @iamphill @thomasrandolph +/app/assets/javascripts/merge_request @viktomas @jboyson @iamphill @thomasrandolph +/app/assets/javascripts/merge_conflicts @viktomas @jboyson @iamphill @thomasrandolph +/app/assets/javascripts/mr_notes @viktomas @jboyson @iamphill @thomasrandolph +/app/assets/javascripts/mr_popover @viktomas @jboyson @iamphill @thomasrandolph +/app/assets/javascripts/vue_merge_request_widget @viktomas @jboyson @iamphill @thomasrandolph +/app/assets/javascripts/merge_request.js @viktomas @jboyson @iamphill @thomasrandolph +/app/assets/javascripts/merge_request_tabs.js @viktomas @jboyson @iamphill @thomasrandolph +/app/assets/stylesheets/framework/diffs.scss @viktomas @jboyson @iamphill @thomasrandolph +/app/assets/stylesheets/components/batch_comments/ @viktomas @jboyson @iamphill @thomasrandolph +/ee/app/assets/javascripts/diffs/ @viktomas @jboyson @iamphill @thomasrandolph +/ee/app/assets/javascripts/vue_merge_request_widget @viktomas @jboyson @iamphill @thomasrandolph +/spec/frontend/diffs/ @viktomas @jboyson @iamphill @thomasrandolph +/spec/frontend/batch_comments/ @viktomas @jboyson @iamphill @thomasrandolph + + [Product Intelligence] /ee/lib/gitlab/usage_data_counters/ @gitlab-org/growth/product-intelligence/engineers /ee/lib/ee/gitlab/usage_data.rb @gitlab-org/growth/product-intelligence/engineers diff --git a/app/assets/javascripts/packages/shared/components/package_list_row.vue b/app/assets/javascripts/packages/shared/components/package_list_row.vue index 4de4c191e51..eee0e470c7b 100644 --- a/app/assets/javascripts/packages/shared/components/package_list_row.vue +++ b/app/assets/javascripts/packages/shared/components/package_list_row.vue @@ -1,7 +1,9 @@ <script> import { GlButton, GlLink, GlSprintf, GlTooltipDirective, GlTruncate } from '@gitlab/ui'; +import { s__ } from '~/locale'; import ListItem from '~/vue_shared/components/registry/list_item.vue'; import timeagoMixin from '~/vue_shared/mixins/timeago'; +import { PACKAGE_ERROR_STATUS, PACKAGE_DEFAULT_STATUS } from '../constants'; import { getPackageTypeLabel } from '../utils'; import PackagePath from './package_path.vue'; import PackageTags from './package_tags.vue'; @@ -70,22 +72,45 @@ export default { hasProjectLink() { return Boolean(this.packageEntity.project_path); }, + showWarningIcon() { + return this.packageEntity.status === PACKAGE_ERROR_STATUS; + }, + disabledRow() { + return this.packageEntity.status && this.packageEntity.status !== PACKAGE_DEFAULT_STATUS; + }, + disabledDeleteButton() { + return this.disabledRow || !this.packageEntity._links.delete_api_path; + }, + }, + i18n: { + erroredPackageText: s__('PackageRegistry|Invalid Package: failed metadata extraction'), }, }; </script> <template> - <list-item data-qa-selector="package_row"> + <list-item data-qa-selector="package_row" :disabled="disabledRow"> <template #left-primary> <div class="gl-display-flex gl-align-items-center gl-mr-3 gl-min-w-0"> <gl-link :href="packageLink" class="gl-text-body gl-min-w-0" data-qa-selector="package_link" + :disabled="disabledRow" > <gl-truncate :text="packageEntity.name" /> </gl-link> + <gl-button + v-if="showWarningIcon" + v-gl-tooltip="{ title: $options.i18n.erroredPackageText }" + class="gl-hover-bg-transparent!" + icon="warning" + category="tertiary" + data-testid="warning-icon" + :aria-label="__('Warning')" + /> + <package-tags v-if="packageEntity.tags && packageEntity.tags.length" class="gl-ml-3" @@ -109,7 +134,11 @@ export default { {{ packageType }} </component> - <package-path v-if="hasProjectLink" :path="packageEntity.project_path" /> + <package-path + v-if="hasProjectLink" + :path="packageEntity.project_path" + :disabled="disabledRow" + /> </div> </template> @@ -137,7 +166,7 @@ export default { variant="danger" :title="s__('PackageRegistry|Remove package')" :aria-label="s__('PackageRegistry|Remove package')" - :disabled="!packageEntity._links.delete_api_path" + :disabled="disabledDeleteButton" @click="$emit('packageToDelete', packageEntity)" /> </template> diff --git a/app/assets/javascripts/packages/shared/components/package_path.vue b/app/assets/javascripts/packages/shared/components/package_path.vue index 9afe06ab497..6fb001e5e92 100644 --- a/app/assets/javascripts/packages/shared/components/package_path.vue +++ b/app/assets/javascripts/packages/shared/components/package_path.vue @@ -16,6 +16,11 @@ export default { type: String, required: true, }, + disabled: { + type: Boolean, + required: false, + default: false, + }, }, computed: { pathPieces() { @@ -45,7 +50,12 @@ export default { <div data-qa-selector="package-path" class="gl-display-flex gl-align-items-center"> <gl-icon data-testid="base-icon" name="project" class="gl-mx-3 gl-min-w-0" /> - <gl-link data-testid="root-link" class="gl-text-gray-500 gl-min-w-0" :href="`/${rootLink}`"> + <gl-link + data-testid="root-link" + class="gl-text-gray-500 gl-min-w-0" + :href="`/${rootLink}`" + :disabled="disabled" + > {{ root }} </gl-link> @@ -63,7 +73,12 @@ export default { <gl-icon data-testid="ellipsis-chevron" name="chevron-right" class="gl-mx-2 gl-min-w-0" /> </template> - <gl-link data-testid="leaf-link" class="gl-text-gray-500 gl-min-w-0" :href="`/${path}`"> + <gl-link + data-testid="leaf-link" + class="gl-text-gray-500 gl-min-w-0" + :href="`/${path}`" + :disabled="disabled" + > {{ leaf }} </gl-link> </template> diff --git a/app/assets/javascripts/packages/shared/constants.js b/app/assets/javascripts/packages/shared/constants.js index f7de31c2c86..b3df542e0ae 100644 --- a/app/assets/javascripts/packages/shared/constants.js +++ b/app/assets/javascripts/packages/shared/constants.js @@ -26,3 +26,8 @@ export const TrackingCategories = { export const SHOW_DELETE_SUCCESS_ALERT = 'showSuccessDeleteAlert'; export const DELETE_PACKAGE_ERROR_MESSAGE = __('Something went wrong while deleting the package.'); + +export const PACKAGE_ERROR_STATUS = 'error'; +export const PACKAGE_DEFAULT_STATUS = 'default'; +export const PACKAGE_HIDDEN_STATUS = 'hidden'; +export const PACKAGE_PROCESSING_STATUS = 'processing'; diff --git a/app/assets/javascripts/pages/projects/pipelines/new/index.js b/app/assets/javascripts/pages/projects/pipelines/new/index.js index 32299287a9c..e1f71965853 100644 --- a/app/assets/javascripts/pages/projects/pipelines/new/index.js +++ b/app/assets/javascripts/pages/projects/pipelines/new/index.js @@ -1,17 +1,3 @@ -import $ from 'jquery'; -import setupNativeFormVariableList from '~/ci_variable_list/native_form_variable_list'; -import NewBranchForm from '~/new_branch_form'; -import initNewPipeline from '~/pipeline_new/index'; +import initNewPipelineForm from '~/pipeline_new/index'; -const el = document.getElementById('js-new-pipeline'); - -if (el) { - initNewPipeline(); -} else { - new NewBranchForm($('.js-new-pipeline-form')); // eslint-disable-line no-new - - setupNativeFormVariableList({ - container: $('.js-ci-variable-list-section'), - formField: 'variables_attributes', - }); -} +initNewPipelineForm(); diff --git a/app/assets/javascripts/registry/explorer/components/details_page/tags_list_row.vue b/app/assets/javascripts/registry/explorer/components/details_page/tags_list_row.vue index 74027a376a7..45eb2ce51e4 100644 --- a/app/assets/javascripts/registry/explorer/components/details_page/tags_list_row.vue +++ b/app/assets/javascripts/registry/explorer/components/details_page/tags_list_row.vue @@ -50,6 +50,11 @@ export default { default: false, required: false, }, + disabled: { + type: Boolean, + default: false, + required: false, + }, }, i18n: { REMOVE_TAG_BUTTON_TITLE, @@ -92,19 +97,25 @@ export default { tagLocation() { return this.tag.path?.replace(`:${this.tag.name}`, ''); }, - invalidTag() { + isInvalidTag() { return !this.tag.digest; }, + isCheckboxDisabled() { + return this.isInvalidTag || this.disabled; + }, + isDeleteDisabled() { + return this.isInvalidTag || this.disabled || !this.tag.canDelete; + }, }, }; </script> <template> - <list-item v-bind="$attrs" :selected="selected"> + <list-item v-bind="$attrs" :selected="selected" :disabled="disabled"> <template #left-action> <gl-form-checkbox v-if="tag.canDelete" - :disabled="invalidTag" + :disabled="isCheckboxDisabled" class="gl-m-0" :checked="selected" @change="$emit('select')" @@ -126,10 +137,11 @@ export default { :title="tag.location" :text="tag.location" category="tertiary" + :disabled="disabled" /> <gl-icon - v-if="invalidTag" + v-if="isInvalidTag" v-gl-tooltip="{ title: $options.i18n.MISSING_MANIFEST_WARNING_TOOLTIP }" name="warning" class="gl-text-orange-500 gl-mb-2 gl-ml-2" @@ -162,7 +174,7 @@ export default { </template> <template #right-action> <delete-button - :disabled="!tag.canDelete || invalidTag" + :disabled="isDeleteDisabled" :title="$options.i18n.REMOVE_TAG_BUTTON_TITLE" :tooltip-title="$options.i18n.REMOVE_TAG_BUTTON_DISABLE_TOOLTIP" :tooltip-disabled="tag.canDelete" @@ -172,7 +184,7 @@ export default { /> </template> - <template v-if="!invalidTag" #details-published> + <template v-if="!isInvalidTag" #details-published> <details-row icon="clock" data-testid="published-date-detail"> <gl-sprintf :message="$options.i18n.PUBLISHED_DETAILS_ROW_TEXT"> <template #repositoryPath> @@ -187,7 +199,7 @@ export default { </gl-sprintf> </details-row> </template> - <template v-if="!invalidTag" #details-manifest-digest> + <template v-if="!isInvalidTag" #details-manifest-digest> <details-row icon="log" data-testid="manifest-detail"> <gl-sprintf :message="$options.i18n.MANIFEST_DETAILS_ROW_TEST"> <template #digest> @@ -200,10 +212,11 @@ export default { :text="tag.digest" category="tertiary" size="small" + :disabled="disabled" /> </details-row> </template> - <template v-if="!invalidTag" #details-configuration-digest> + <template v-if="!isInvalidTag" #details-configuration-digest> <details-row icon="cloud-gear" data-testid="configuration-detail"> <gl-sprintf :message="$options.i18n.CONFIGURATION_DETAILS_ROW_TEST"> <template #digest> @@ -216,6 +229,7 @@ export default { :text="formattedRevision" category="tertiary" size="small" + :disabled="disabled" /> </details-row> </template> diff --git a/app/assets/javascripts/registry/explorer/components/list_page/image_list_row.vue b/app/assets/javascripts/registry/explorer/components/list_page/image_list_row.vue index 0373a84b271..930ad01c758 100644 --- a/app/assets/javascripts/registry/explorer/components/list_page/image_list_row.vue +++ b/app/assets/javascripts/registry/explorer/components/list_page/image_list_row.vue @@ -78,6 +78,9 @@ export default { imageName() { return this.item.name ? this.item.path : `${this.item.path}/ ${ROOT_IMAGE_TEXT}`; }, + routerLinkEvent() { + return this.deleting ? '' : 'click'; + }, }, }; </script> @@ -97,6 +100,7 @@ export default { class="gl-text-body gl-font-weight-bold" data-testid="details-link" data-qa-selector="registry_image_content" + :event="routerLinkEvent" :to="{ name: 'details', params: { id } }" > {{ imageName }} diff --git a/app/assets/javascripts/vue_shared/components/registry/list_item.vue b/app/assets/javascripts/vue_shared/components/registry/list_item.vue index 4ade75e705e..b9e916bc199 100644 --- a/app/assets/javascripts/vue_shared/components/registry/list_item.vue +++ b/app/assets/javascripts/vue_shared/components/registry/list_item.vue @@ -32,7 +32,7 @@ export default { return { 'gl-border-t-transparent': !this.first && !this.selected, 'gl-border-t-gray-100': this.first && !this.selected, - 'disabled-content': this.disabled, + 'gl-opacity-5': this.disabled, 'gl-border-b-gray-100': !this.selected, 'gl-bg-blue-50 gl-border-blue-200': this.selected, }; diff --git a/app/assets/javascripts/vue_shared/components/user_popover/user_popover.vue b/app/assets/javascripts/vue_shared/components/user_popover/user_popover.vue index 18c9ed7a6f0..deac24d2270 100644 --- a/app/assets/javascripts/vue_shared/components/user_popover/user_popover.vue +++ b/app/assets/javascripts/vue_shared/components/user_popover/user_popover.vue @@ -79,7 +79,7 @@ export default { <div class="gl-text-gray-500"> <div v-if="user.bio" class="gl-display-flex gl-mb-2"> <gl-icon name="profile" class="gl-text-gray-400 gl-flex-shrink-0" /> - <span ref="bio" class="gl-ml-2" v-html="user.bioHtml"></span> + <span ref="bio" class="gl-ml-2 gl-overflow-hidden" v-html="user.bioHtml"></span> </div> <div v-if="user.workInformation" class="gl-display-flex gl-mb-2"> <gl-icon name="work" class="gl-text-gray-400 gl-flex-shrink-0" /> diff --git a/app/assets/javascripts/vue_shared/directives/validation.js b/app/assets/javascripts/vue_shared/directives/validation.js index df853a7c5b5..692f2769b88 100644 --- a/app/assets/javascripts/vue_shared/directives/validation.js +++ b/app/assets/javascripts/vue_shared/directives/validation.js @@ -136,3 +136,59 @@ export default function initValidation(customFeedbackMap = {}) { }, }; } + +/** + * This is a helper that initialize the form fields structure to be used in initForm + * @param {*} fieldValues + * @returns formObject + */ +const initFormField = ({ value, required = true, skipValidation = false }) => ({ + value, + required, + state: skipValidation ? true : null, + feedback: null, +}); + +/** + * This is a helper that initialize the form structure that is compliant to be used with the validation directive + * + * @example + * const form initForm = initForm({ + * fields: { + * name: { + * value: 'lorem' + * }, + * description: { + * value: 'ipsum', + * required: false, + * skipValidation: true + * } + * } + * }) + * + * @example + * const form initForm = initForm({ + * state: true, // to override + * foo: { // something custom + * bar: 'lorem' + * }, + * fields: {...} + * }) + * + * @param {*} formObject + * @returns form + */ +export const initForm = ({ fields = {}, ...rest } = {}) => { + const initFields = Object.fromEntries( + Object.entries(fields).map(([fieldName, fieldValues]) => { + return [fieldName, initFormField(fieldValues)]; + }), + ); + + return { + state: false, + showValidation: false, + ...rest, + fields: initFields, + }; +}; diff --git a/app/finders/deployments_finder.rb b/app/finders/deployments_finder.rb index a54408e7216..acce038dba6 100644 --- a/app/finders/deployments_finder.rb +++ b/app/finders/deployments_finder.rb @@ -131,20 +131,22 @@ class DeploymentsFinder end def optimize_sort_params!(sort_params) + sort_direction = sort_params.each_value.first + # Implicitly enforce the ordering when filtered by `updated_at` column for performance optimization. # See https://gitlab.com/gitlab-org/gitlab/-/issues/325627#note_552417509. # We remove this in https://gitlab.com/gitlab-org/gitlab/-/issues/328500. if filter_by_updated_at? && implicitly_enforce_ordering_for_updated_at_filter? - sort_params.replace('updated_at' => sort_params.each_value.first) + sort_params.replace('updated_at' => sort_direction) end if sort_params['created_at'] || sort_params['iid'] # Sorting by `id` produces the same result as sorting by `created_at` or `iid` - sort_params.replace(id: sort_params.each_value.first) + sort_params.replace(id: sort_direction) elsif sort_params['updated_at'] # This adds the order as a tie-breaker when multiple rows have the same updated_at value. # See https://gitlab.com/gitlab-org/gitlab/-/merge_requests/20848. - sort_params.merge!(id: :desc) + sort_params.merge!(id: sort_direction) end end diff --git a/app/graphql/types/project_type.rb b/app/graphql/types/project_type.rb index 5a24a6fed81..a2852588e89 100644 --- a/app/graphql/types/project_type.rb +++ b/app/graphql/types/project_type.rb @@ -30,8 +30,12 @@ module Types markdown_field :description_html, null: true field :tag_list, GraphQL::STRING_TYPE, null: true, + deprecated: { reason: 'Use `topics`', milestone: '13.12' }, description: 'List of project topics (not Git tags).' + field :topics, [GraphQL::STRING_TYPE], null: true, + description: 'List of project topics.' + field :ssh_url_to_repo, GraphQL::STRING_TYPE, null: true, description: 'URL to connect to the project via SSH.' field :http_url_to_repo, GraphQL::STRING_TYPE, null: true, diff --git a/app/helpers/nav/top_nav_helper.rb b/app/helpers/nav/top_nav_helper.rb index 386610e74b0..159b7ca87f9 100644 --- a/app/helpers/nav/top_nav_helper.rb +++ b/app/helpers/nav/top_nav_helper.rb @@ -3,12 +3,13 @@ module Nav module TopNavHelper PROJECTS_VIEW = :projects + GROUPS_VIEW = :groups - def top_nav_view_model(project:) + def top_nav_view_model(project:, group:) builder = ::Gitlab::Nav::TopNavViewModelBuilder.new if current_user - build_view_model(builder: builder, project: project) + build_view_model(builder: builder, project: project, group: group) else build_anonymous_view_model(builder: builder) end @@ -20,32 +21,65 @@ module Nav def build_anonymous_view_model(builder:) # These come from `app/views/layouts/nav/_explore.html.ham` - # TODO: We will move the rest of them shortly - # https://gitlab.com/gitlab-org/gitlab/-/merge_requests/56587 if explore_nav_link?(:projects) builder.add_primary_menu_item( - **projects_menu_item_attrs.merge({ - active: active_nav_link?(path: ['dashboard#show', 'root#show', 'projects#trending', 'projects#starred', 'projects#index']), - href: explore_root_path - }) + **projects_menu_item_attrs.merge( + { + active: active_nav_link?(path: %w[dashboard#show root#show projects#trending projects#starred projects#index]), + href: explore_root_path + }) + ) + end + + if explore_nav_link?(:groups) + builder.add_primary_menu_item( + **groups_menu_item_attrs.merge( + { + active: active_nav_link?(controller: [:groups, 'groups/milestones', 'groups/group_members']), + href: explore_groups_path + }) + ) + end + + if explore_nav_link?(:snippets) + builder.add_primary_menu_item( + **snippets_menu_item_attrs.merge( + { + active: active_nav_link?(controller: :snippets), + href: explore_snippets_path + }) ) end end - def build_view_model(builder:, project:) + def build_view_model(builder:, project:, group:) # These come from `app/views/layouts/nav/_dashboard.html.haml` if dashboard_nav_link?(:projects) current_item = project ? current_project(project: project) : {} builder.add_primary_menu_item( **projects_menu_item_attrs.merge({ - active: active_nav_link?(path: ['root#index', 'projects#trending', 'projects#starred', 'dashboard/projects#index']), + active: active_nav_link?(path: %w[root#index projects#trending projects#starred dashboard/projects#index]), css_class: 'qa-projects-dropdown', data: { track_label: "projects_dropdown", track_event: "click_dropdown", track_experiment: "new_repo" }, view: PROJECTS_VIEW }) ) - builder.add_view(PROJECTS_VIEW, container_view_props(current_item: current_item, submenu: projects_submenu)) + builder.add_view(PROJECTS_VIEW, container_view_props(namespace: 'projects', current_item: current_item, submenu: projects_submenu)) + end + + if dashboard_nav_link?(:groups) + current_item = group ? current_group(group: group) : {} + + builder.add_primary_menu_item( + **groups_menu_item_attrs.merge({ + active: active_nav_link?(path: %w[dashboard/groups explore/groups]), + css_class: 'qa-groups-dropdown', + data: { track_label: "groups_dropdown", track_event: "click_dropdown" }, + view: GROUPS_VIEW + }) + ) + builder.add_view(GROUPS_VIEW, container_view_props(namespace: 'groups', current_item: current_item, submenu: groups_submenu)) end if dashboard_nav_link?(:milestones) @@ -59,6 +93,27 @@ module Nav ) end + if dashboard_nav_link?(:snippets) + builder.add_primary_menu_item( + **snippets_menu_item_attrs.merge({ + active: active_nav_link?(controller: 'dashboard/snippets'), + data: { qa_selector: 'snippets_link' }, + href: dashboard_snippets_path + }) + ) + end + + if dashboard_nav_link?(:activity) + builder.add_primary_menu_item( + id: 'activity', + title: 'Activity', + active: active_nav_link?(path: 'dashboard#activity'), + icon: 'history', + data: { qa_selector: 'activity_link' }, + href: activity_dashboard_path + ) + end + # Using admin? is generally discouraged because it does not check for # "admin_mode". In this case we are migrating code and check both, so # we should be good. @@ -95,6 +150,15 @@ module Nav end end # rubocop: enable Cop/UserAdmin + + if Gitlab::Sherlock.enabled? + builder.add_secondary_menu_item( + id: 'sherlock', + title: _('Sherlock Transactions'), + icon: 'admin', + href: sherlock_transactions_path + ) + end end def projects_menu_item_attrs @@ -105,9 +169,25 @@ module Nav } end - def container_view_props(current_item:, submenu:) + def groups_menu_item_attrs { - namespace: 'projects', + id: 'groups', + title: 'Groups', + icon: 'group' + } + end + + def snippets_menu_item_attrs + { + id: 'snippets', + title: _('Snippets'), + icon: 'snippet' + } + end + + def container_view_props(namespace:, current_item:, submenu:) + { + namespace: namespace, currentUserName: current_user&.username, currentItem: current_item, linksPrimary: submenu[:primary], @@ -127,6 +207,18 @@ module Nav } end + def current_group(group:) + return {} unless group.persisted? + + { + id: group.id, + name: group.name, + namespace: group.full_name, + webUrl: group_path(group), + avatarUrl: group.avatar_url + } + end + def projects_submenu # These project links come from `app/views/layouts/nav/projects_dropdown/_show.html.haml` builder = ::Gitlab::Nav::TopNavMenuBuilder.new @@ -136,6 +228,15 @@ module Nav builder.add_secondary_menu_item(id: 'create', title: _('Create new project'), href: new_project_path) builder.build end + + def groups_submenu + # These group links come from `app/views/layouts/nav/groups_dropdown/_show.html.haml` + builder = ::Gitlab::Nav::TopNavMenuBuilder.new + builder.add_primary_menu_item(id: 'your', title: _('Your groups'), href: dashboard_groups_path) + builder.add_primary_menu_item(id: 'explore', title: _('Explore groups'), href: explore_groups_path) + builder.add_secondary_menu_item(id: 'create', title: _('Create group'), href: new_group_path(anchor: 'create-group-pane')) + builder.build + end end end diff --git a/app/models/bulk_imports/export.rb b/app/models/bulk_imports/export.rb index 47287db57a8..59ca4dbfec6 100644 --- a/app/models/bulk_imports/export.rb +++ b/app/models/bulk_imports/export.rb @@ -15,7 +15,7 @@ module BulkImports validates :group, presence: true, unless: :project validates :relation, :status, presence: true - validate :exportable_relation? + validate :portable_relation? state_machine :status, initial: :started do state :started, value: 0 @@ -36,34 +36,25 @@ module BulkImports end end - def self.config(exportable) - case exportable - when ::Project - Exports::ProjectConfig.new(exportable) - when ::Group - Exports::GroupConfig.new(exportable) - end - end - - def exportable_relation? - return unless exportable + def portable_relation? + return unless portable - errors.add(:relation, 'Unsupported exportable relation') unless config.exportable_relations.include?(relation) + errors.add(:relation, 'Unsupported portable relation') unless config.portable_relations.include?(relation) end - def exportable - strong_memoize(:exportable) do + def portable + strong_memoize(:portable) do project || group end end def relation_definition - config.exportable_tree[:include].find { |include| include[relation.to_sym] } + config.portable_tree[:include].find { |include| include[relation.to_sym] } end def config strong_memoize(:config) do - self.class.config(exportable) + FileTransfer.config_for(portable) end end end diff --git a/app/models/bulk_imports/file_transfer.rb b/app/models/bulk_imports/file_transfer.rb new file mode 100644 index 00000000000..5be954b98da --- /dev/null +++ b/app/models/bulk_imports/file_transfer.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +module BulkImports + module FileTransfer + extend self + + UnsupportedObjectType = Class.new(StandardError) + + def config_for(portable) + case portable + when ::Project + FileTransfer::ProjectConfig.new(portable) + when ::Group + FileTransfer::GroupConfig.new(portable) + else + raise(UnsupportedObjectType, "Unsupported object type: #{portable.class}") + end + end + end +end diff --git a/app/models/bulk_imports/exports/base_config.rb b/app/models/bulk_imports/file_transfer/base_config.rb index 240f75e7873..bb04e84ad72 100644 --- a/app/models/bulk_imports/exports/base_config.rb +++ b/app/models/bulk_imports/file_transfer/base_config.rb @@ -1,16 +1,16 @@ # frozen_string_literal: true module BulkImports - module Exports + module FileTransfer class BaseConfig include Gitlab::Utils::StrongMemoize - def initialize(exportable) - @exportable = exportable + def initialize(portable) + @portable = portable end - def exportable_tree - attributes_finder.find_root(exportable_class_sym) + def portable_tree + attributes_finder.find_root(portable_class_sym) end def export_path @@ -21,13 +21,13 @@ module BulkImports end end - def exportable_relations - import_export_config.dig(:tree, exportable_class_sym).keys.map(&:to_s) + def portable_relations + import_export_config.dig(:tree, portable_class_sym).keys.map(&:to_s) end private - attr_reader :exportable + attr_reader :portable def attributes_finder strong_memoize(:attributes_finder) do @@ -39,12 +39,12 @@ module BulkImports ::Gitlab::ImportExport::Config.new(config: import_export_yaml).to_h end - def exportable_class - @exportable_class ||= exportable.class + def portable_class + @portable_class ||= portable.class end - def exportable_class_sym - @exportable_class_sym ||= exportable_class.to_s.downcase.to_sym + def portable_class_sym + @portable_class_sym ||= portable_class.to_s.demodulize.underscore.to_sym end def import_export_yaml diff --git a/app/models/bulk_imports/exports/group_config.rb b/app/models/bulk_imports/file_transfer/group_config.rb index 546246af1d0..1f845b387b8 100644 --- a/app/models/bulk_imports/exports/group_config.rb +++ b/app/models/bulk_imports/file_transfer/group_config.rb @@ -1,10 +1,10 @@ # frozen_string_literal: true module BulkImports - module Exports + module FileTransfer class GroupConfig < BaseConfig def base_export_path - exportable.full_path + portable.full_path end def import_export_yaml diff --git a/app/models/bulk_imports/exports/project_config.rb b/app/models/bulk_imports/file_transfer/project_config.rb index e50d7013711..e42b5bfce3d 100644 --- a/app/models/bulk_imports/exports/project_config.rb +++ b/app/models/bulk_imports/file_transfer/project_config.rb @@ -1,10 +1,10 @@ # frozen_string_literal: true module BulkImports - module Exports + module FileTransfer class ProjectConfig < BaseConfig def base_export_path - exportable.disk_path + portable.disk_path end def import_export_yaml diff --git a/app/presenters/packages/detail/package_presenter.rb b/app/presenters/packages/detail/package_presenter.rb index 6640b0c5e94..4fa207b1205 100644 --- a/app/presenters/packages/detail/package_presenter.rb +++ b/app/presenters/packages/detail/package_presenter.rb @@ -17,6 +17,7 @@ module Packages name: name, package_files: @package.package_files.map { |pf| build_package_file_view(pf) }, package_type: @package.package_type, + status: @package.status, project_id: @package.project_id, tags: @package.tags.as_json, updated_at: @package.updated_at, diff --git a/app/services/bulk_imports/export_service.rb b/app/services/bulk_imports/export_service.rb index ca0c05bd017..33b3a8e187f 100644 --- a/app/services/bulk_imports/export_service.rb +++ b/app/services/bulk_imports/export_service.rb @@ -2,14 +2,14 @@ module BulkImports class ExportService - def initialize(exportable:, user:) - @exportable = exportable + def initialize(portable:, user:) + @portable = portable @current_user = user end def execute - Export.config(exportable).exportable_relations.each do |relation| - RelationExportWorker.perform_async(current_user.id, exportable.id, exportable.class.name, relation) + FileTransfer.config_for(portable).portable_relations.each do |relation| + RelationExportWorker.perform_async(current_user.id, portable.id, portable.class.name, relation) end ServiceResponse.success @@ -22,6 +22,6 @@ module BulkImports private - attr_reader :exportable, :current_user + attr_reader :portable, :current_user end end diff --git a/app/services/bulk_imports/relation_export_service.rb b/app/services/bulk_imports/relation_export_service.rb index 661c84582d0..53952a33b5f 100644 --- a/app/services/bulk_imports/relation_export_service.rb +++ b/app/services/bulk_imports/relation_export_service.rb @@ -4,9 +4,9 @@ module BulkImports class RelationExportService include Gitlab::ImportExport::CommandLineUtil - def initialize(user, exportable, relation, jid) + def initialize(user, portable, relation, jid) @user = user - @exportable = exportable + @portable = portable @relation = relation @jid = jid end @@ -22,28 +22,28 @@ module BulkImports private - attr_reader :user, :exportable, :relation, :jid + attr_reader :user, :portable, :relation, :jid def find_or_create_export! validate_user_permissions! - export = exportable.bulk_import_exports.safe_find_or_create_by!(relation: relation) + export = portable.bulk_import_exports.safe_find_or_create_by!(relation: relation) export.update!(status_event: 'start', jid: jid) yield export export.update!(status_event: 'finish', error: nil) rescue StandardError => e - Gitlab::ErrorTracking.track_exception(e, exportable_id: exportable.id, exportable_type: exportable.class.name) + Gitlab::ErrorTracking.track_exception(e, portable_id: portable.id, portable_type: portable.class.name) export&.update(status_event: 'fail_op', error: e.class) end def validate_user_permissions! - ability = "admin_#{exportable.to_ability_name}" + ability = "admin_#{portable.to_ability_name}" - user.can?(ability, exportable) || - raise(::Gitlab::ImportExport::Error.permission_error(user, exportable)) + user.can?(ability, portable) || + raise(::Gitlab::ImportExport::Error.permission_error(user, portable)) end def remove_existing_export_file!(export) @@ -72,23 +72,23 @@ module BulkImports upload.save! end - def export_config - @export_config ||= Export.config(exportable) + def config + @config ||= FileTransfer.config_for(portable) end def export_path - @export_path ||= export_config.export_path + @export_path ||= config.export_path end - def exportable_tree - @exportable_tree ||= export_config.exportable_tree + def portable_tree + @portable_tree ||= config.portable_tree end # rubocop: disable CodeReuse/Serializer def serializer @serializer ||= ::Gitlab::ImportExport::JSON::StreamingSerializer.new( - exportable, - exportable_tree, + portable, + portable_tree, json_writer, exportable_path: '' ) diff --git a/app/services/packages/rubygems/process_gem_service.rb b/app/services/packages/rubygems/process_gem_service.rb index f51d02d5eac..109c87a0444 100644 --- a/app/services/packages/rubygems/process_gem_service.rb +++ b/app/services/packages/rubygems/process_gem_service.rb @@ -16,6 +16,7 @@ module Packages end def execute + raise ExtractionError, 'Gem was not processed - package_file is not set' unless package_file return success if process_gem error('Gem was not processed') @@ -26,8 +27,6 @@ module Packages attr_reader :package_file def process_gem - return false unless package_file - try_obtain_lease do package.transaction do rename_package_and_set_version diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index 7d435ac03f8..4854e026ed3 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -1354,7 +1354,7 @@ :urgency: :low :resource_boundary: :unknown :weight: 1 - :idempotent: true + :idempotent: :tags: - :exclude_from_kubernetes - :name: pipeline_background:archive_trace diff --git a/app/workers/bulk_imports/relation_export_worker.rb b/app/workers/bulk_imports/relation_export_worker.rb index b7df9a3d0b0..9d9449e3a1b 100644 --- a/app/workers/bulk_imports/relation_export_worker.rb +++ b/app/workers/bulk_imports/relation_export_worker.rb @@ -11,17 +11,17 @@ module BulkImports tags :exclude_from_kubernetes sidekiq_options status_expiration: StuckExportJobsWorker::EXPORT_JOBS_EXPIRATION - def perform(user_id, exportable_id, exportable_class, relation) + def perform(user_id, portable_id, portable_class, relation) user = User.find(user_id) - exportable = exportable(exportable_id, exportable_class) + portable = portable(portable_id, portable_class) - RelationExportService.new(user, exportable, relation, jid).execute + RelationExportService.new(user, portable, relation, jid).execute end private - def exportable(exportable_id, exportable_class) - exportable_class.classify.constantize.find(exportable_id) + def portable(portable_id, portable_class) + portable_class.classify.constantize.find(portable_id) end end end diff --git a/app/workers/packages/nuget/extraction_worker.rb b/app/workers/packages/nuget/extraction_worker.rb index 78d08a12f06..97f900c4ff2 100644 --- a/app/workers/packages/nuget/extraction_worker.rb +++ b/app/workers/packages/nuget/extraction_worker.rb @@ -20,7 +20,7 @@ module Packages rescue ::Packages::Nuget::MetadataExtractionService::ExtractionError, ::Packages::Nuget::UpdatePackageFromMetadataService::InvalidMetadataError => e Gitlab::ErrorTracking.log_exception(e, project_id: package_file.project_id) - package_file.package.destroy! + package_file.package.update_column(:status, :error) end end end diff --git a/app/workers/packages/rubygems/extraction_worker.rb b/app/workers/packages/rubygems/extraction_worker.rb index e5cf049b003..a5630e89140 100644 --- a/app/workers/packages/rubygems/extraction_worker.rb +++ b/app/workers/packages/rubygems/extraction_worker.rb @@ -12,8 +12,6 @@ module Packages tags :exclude_from_kubernetes deduplicate :until_executing - idempotent! - def perform(package_file_id) package_file = ::Packages::PackageFile.find_by_id(package_file_id) @@ -23,7 +21,7 @@ module Packages rescue ::Packages::Rubygems::ProcessGemService::ExtractionError => e Gitlab::ErrorTracking.log_exception(e, project_id: package_file.project_id) - package_file.package.destroy! + package_file.package.update_column(:status, :error) end end end diff --git a/changelogs/unreleased/211373-improve-issues-list-api-memory.yml b/changelogs/unreleased/211373-improve-issues-list-api-memory.yml new file mode 100644 index 00000000000..504a994b77b --- /dev/null +++ b/changelogs/unreleased/211373-improve-issues-list-api-memory.yml @@ -0,0 +1,5 @@ +--- +title: Improve memory consumption of issuable APIs +merge_request: 61561 +author: +type: performance diff --git a/changelogs/unreleased/324206-error-packages.yml b/changelogs/unreleased/324206-error-packages.yml new file mode 100644 index 00000000000..7591816a61c --- /dev/null +++ b/changelogs/unreleased/324206-error-packages.yml @@ -0,0 +1,6 @@ +--- +title: Update RubyGems and NuGet packages to error status upon metadata extraction + failure +merge_request: 60172 +author: +type: changed diff --git a/changelogs/unreleased/fix-user-popover-bio-overflow.yml b/changelogs/unreleased/fix-user-popover-bio-overflow.yml new file mode 100644 index 00000000000..a977d571651 --- /dev/null +++ b/changelogs/unreleased/fix-user-popover-bio-overflow.yml @@ -0,0 +1,5 @@ +--- +title: Fix user popover bio overflow +merge_request: 61555 +author: +type: fixed diff --git a/changelogs/unreleased/project-topics-graphql-migration.yml b/changelogs/unreleased/project-topics-graphql-migration.yml new file mode 100644 index 00000000000..c1b108563ce --- /dev/null +++ b/changelogs/unreleased/project-topics-graphql-migration.yml @@ -0,0 +1,5 @@ +--- +title: Add GraphQL field 'Project.topics' and deprecate 'Project.tag_list' +merge_request: 61250 +author: Jonas Wälter @wwwjon +type: deprecated diff --git a/changelogs/unreleased/tie-breaker-should-respect-the-original-sort.yml b/changelogs/unreleased/tie-breaker-should-respect-the-original-sort.yml new file mode 100644 index 00000000000..7395ac3c764 --- /dev/null +++ b/changelogs/unreleased/tie-breaker-should-respect-the-original-sort.yml @@ -0,0 +1,5 @@ +--- +title: Tie-breaker in Deployment Finder should respect the original sort direction +merge_request: 61444 +author: +type: performance diff --git a/config/feature_flags/development/track_file_size_over_highlight_limit.yml b/config/feature_flags/development/track_file_size_over_highlight_limit.yml new file mode 100644 index 00000000000..431c646f54d --- /dev/null +++ b/config/feature_flags/development/track_file_size_over_highlight_limit.yml @@ -0,0 +1,8 @@ +--- +name: track_file_size_over_highlight_limit +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/61273 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/330374 +milestone: '13.12' +type: development +group: group::code review +default_enabled: false diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 517a51461f2..7d9a56ad56f 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -10804,8 +10804,9 @@ Represents vulnerability finding of a security report on the pipeline. | <a id="projectstarcount"></a>`starCount` | [`Int!`](#int) | Number of times the project has been starred. | | <a id="projectstatistics"></a>`statistics` | [`ProjectStatistics`](#projectstatistics) | Statistics of the project. | | <a id="projectsuggestioncommitmessage"></a>`suggestionCommitMessage` | [`String`](#string) | The commit message used to apply merge request suggestions. | -| <a id="projecttaglist"></a>`tagList` | [`String`](#string) | List of project topics (not Git tags). | +| <a id="projecttaglist"></a>`tagList` **{warning-solid}** | [`String`](#string) | **Deprecated** in 13.12. Use `topics`. | | <a id="projectterraformstates"></a>`terraformStates` | [`TerraformStateConnection`](#terraformstateconnection) | Terraform states associated with the project. (see [Connections](#connections)) | +| <a id="projecttopics"></a>`topics` | [`[String!]`](#string) | List of project topics. | | <a id="projectuserpermissions"></a>`userPermissions` | [`ProjectPermissions!`](#projectpermissions) | Permissions for the current user on the resource. | | <a id="projectvisibility"></a>`visibility` | [`String`](#string) | Visibility of the project. | | <a id="projectvulnerabilityscanners"></a>`vulnerabilityScanners` | [`VulnerabilityScannerConnection`](#vulnerabilityscannerconnection) | Vulnerability scanners reported on the project vulnerabilities. (see [Connections](#connections)) | diff --git a/doc/ci/unit_test_reports.md b/doc/ci/unit_test_reports.md index c71d455670c..e5d16a9d92d 100644 --- a/doc/ci/unit_test_reports.md +++ b/doc/ci/unit_test_reports.md @@ -336,14 +336,21 @@ If parsing JUnit report XML results in an error, an indicator is shown next to t > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/202114) in GitLab 13.0 behind the `:junit_pipeline_screenshots_view` feature flag, disabled by default. > - The feature flag was removed and was [made generally available](https://gitlab.com/gitlab-org/gitlab/-/issues/216979) in GitLab 13.12. -If JUnit report format XML files contain an `attachment` tag, GitLab parses the attachment. +Upload your screenshots as [artifacts](yaml/README.md#artifactsreportsjunit) to GitLab. If JUnit +report format XML files contain an `attachment` tag, GitLab parses the attachment. Note that: -```xml -<testcase time="1.00" name="Test"> - <system-out>[[ATTACHMENT|/absolute/path/to/some/file]]</system-out> -</testcase> -``` +- The `attachment` tag **must** contain the absolute path to the screenshots you uploaded. For + example: + + ```xml + <testcase time="1.00" name="Test"> + <system-out>[[ATTACHMENT|/absolute/path/to/some/file]]</system-out> + </testcase> + ``` -Upload your screenshots as [artifacts](yaml/README.md#artifactsreportsjunit) to GitLab. The `attachment` tag **must** contain the absolute path to the screenshots you uploaded. +- You should set the job that uploads the screenshot to + [`artifacts:when: on_failure`](yaml/README.md#artifactswhen) so that it uploads a screenshot when + a test fails. -A link to the test case attachment will appear in the test case details in [the pipeline test report](#viewing-unit-test-reports-on-gitlab). +A link to the test case attachment appears in the test case details in +[the pipeline test report](#viewing-unit-test-reports-on-gitlab). diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index 18183766519..7a414f4a257 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -3858,7 +3858,9 @@ failure. `artifacts:when` can be set to one of the following values: 1. `on_success` (default): Upload artifacts only when the job succeeds. -1. `on_failure`: Upload artifacts only when the job fails. +1. `on_failure`: Upload artifacts only when the job fails. Useful, for example, when + [uploading artifacts](../unit_test_reports.md#viewing-junit-screenshots-on-gitlab) required to + troubleshoot failing tests. 1. `always`: Always upload artifacts. For example, to upload artifacts only when a job fails: diff --git a/doc/integration/external-issue-tracker.md b/doc/integration/external-issue-tracker.md index 4e00057c4b7..e82c21947e2 100644 --- a/doc/integration/external-issue-tracker.md +++ b/doc/integration/external-issue-tracker.md @@ -31,7 +31,7 @@ Visit the links below for details: - [Bugzilla](../user/project/integrations/bugzilla.md) - [Custom Issue Tracker](../user/project/integrations/custom_issue_tracker.md) - [Engineering Workflow Management](../user/project/integrations/ewm.md) -- [Jira](../user/project/integrations/jira.md) +- [Jira](../integration/jira/index.md) - [Redmine](../user/project/integrations/redmine.md) - [YouTrack](../user/project/integrations/youtrack.md) diff --git a/doc/integration/jira/development_panel.md b/doc/integration/jira/development_panel.md index 378955f5721..3aba2e3b3a0 100644 --- a/doc/integration/jira/development_panel.md +++ b/doc/integration/jira/development_panel.md @@ -31,7 +31,7 @@ This integration connects all GitLab projects to projects in the Jira instance i including the projects in its subgroups. - A personal namespace: Connects the projects in that personal namespace to Jira. -This differs from the [Jira integration](../../user/project/integrations/jira.md), +This differs from the [Jira integration](index.md), where the mapping is between one GitLab project and the entire Jira instance. You can install both integrations to take advantage of both sets of features. A [feature comparison](index.md#direct-feature-comparison) is available. diff --git a/doc/integration/jira/dvcs.md b/doc/integration/jira/dvcs.md index 5d636406446..64f01d91d2e 100644 --- a/doc/integration/jira/dvcs.md +++ b/doc/integration/jira/dvcs.md @@ -135,7 +135,7 @@ Problems with SSL and TLS can cause this error message: Error obtaining access token. Cannot access https://gitlab.example.com from Jira. ``` -- The [GitLab Jira integration](../../user/project/integrations/jira.md) requires +- The [GitLab Jira integration](index.md) requires GitLab to connect to Jira. Any TLS issues that arise from a private certificate authority or self-signed certificate are resolved [on the GitLab server](https://docs.gitlab.com/omnibus/settings/ssl.html#other-certificate-authorities), diff --git a/doc/integration/jira/jira_cloud_configuration.md b/doc/integration/jira/jira_cloud_configuration.md index fd58b3f33f7..7b3f2bcb249 100644 --- a/doc/integration/jira/jira_cloud_configuration.md +++ b/doc/integration/jira/jira_cloud_configuration.md @@ -6,7 +6,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w # Create an API token in Jira on Atlassian cloud **(FREE)** -You need an API token to [integrate with Jira](../../user/project/integrations/jira.md) +You need an API token to [integrate with Jira](index.md) on Atlassian cloud. To create the API token: 1. Sign in to [`id.atlassian.com`](https://id.atlassian.com/manage-profile/security/api-tokens) diff --git a/doc/integration/jira/jira_server_configuration.md b/doc/integration/jira/jira_server_configuration.md index 3573a1f8b1e..21348416b73 100644 --- a/doc/integration/jira/jira_server_configuration.md +++ b/doc/integration/jira/jira_server_configuration.md @@ -6,7 +6,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w # Jira Server credentials **(FREE)** -To [integrate Jira with GitLab](../../user/project/integrations/jira.md), you must +To [integrate Jira with GitLab](index.md), you must create a Jira user account for your Jira projects to access projects in GitLab. This Jira user account must have write access to your Jira projects. To create the credentials, you must: diff --git a/doc/university/index.md b/doc/university/index.md index 0d194c7708d..e7461e1b68b 100644 --- a/doc/university/index.md +++ b/doc/university/index.md @@ -195,7 +195,7 @@ The GitLab University curriculum is composed of GitLab videos, screencasts, pres <!-- vale gitlab.Spelling = NO --> 1. [How to Integrate Jira and Jenkins with GitLab - Video](https://gitlabmeetings.webex.com/gitlabmeetings/ldr.php?RCID=44b548147a67ab4d8a62274047146415) -1. [How to Integrate Jira with GitLab](../user/project/integrations/jira.md) +1. [How to Integrate Jira with GitLab](../integration/jira/index.md) 1. [How to Integrate Jenkins with GitLab](../integration/jenkins.md) 1. [How to Integrate Bamboo with GitLab](../user/project/integrations/bamboo.md) 1. [How to Integrate Slack with GitLab](../user/project/integrations/slack.md) diff --git a/doc/user/application_security/index.md b/doc/user/application_security/index.md index 6caab1ee383..4588a731f85 100644 --- a/doc/user/application_security/index.md +++ b/doc/user/application_security/index.md @@ -104,6 +104,47 @@ rules: - if: $CI_PIPELINE_SOURCE == "merge_request_event" ``` +## Security Scanning with Auto DevOps + +When [Auto DevOps](../../topics/autodevops/) is enabled, all GitLab Security scanning tools are configured using default settings. + +- [Auto SAST](../../topics/autodevops/stages.md#auto-sast) +- [Auto Secret Detection](../../topics/autodevops/stages.md#auto-secret-detection) +- [Auto DAST](../../topics/autodevops/stages.md#auto-dast) +- [Auto Dependency Scanning](../../topics/autodevops/stages.md#auto-dependency-scanning) +- [Auto License Compliance](../../topics/autodevops/stages.md#auto-license-compliance) +- [Auto Container Scanning](../../topics/autodevops/stages.md#auto-container-scanning) + +While you cannot directly customize Auto DevOps, you can [include the Auto DevOps template in your project's `.gitlab-ci.yml` file](../../topics/autodevops/customize.md#customizing-gitlab-ciyml). + +## Default behavior of GitLab security scanning tools + +### Secure jobs in your pipeline + +If you add the security scanning jobs as described in [Security scanning with Auto DevOps](#security-scanning-with-auto-devops) or [Security scanning without Auto DevOps](#security-scanning-without-auto-devops) to your `.gitlab-ci.yml` each added [security scanning tool](#security-scanning-tools) behave as described below. + +For each compatible analyzer, a job is created in the `test`, `dast` or `fuzz` stage of your pipeline and runs on the next new branch pipeline. Features such as the [Security Dashboard](security_dashboard/index.md), [Vulnerability Report](vulnerability_report/index.md), and [Dependency List](dependency_list/index.md) that rely on this scan data only show results from pipelines on the default branch. Please note that one tool may use many analyzers. + +Our language and package manager specific jobs attempt to assess which analyzer(s) they should run for your project so that you can do less configuration. + +If you want to override this to increase the pipeline speed you may choose which analyzers to exclude if you know they are not applicable (languages or package managers not contained in your project) by following variable customization directions for that specific tool. + +### Secure job status + +Jobs pass if they are able to complete a scan. A _pass_ result does NOT indicate if they did, or did not, identify findings. The only exception is coverage fuzzing, which fails if it identifies findings. + +Jobs fail if they are unable to complete a scan. You can view the pipeline logs for more information. + +All jobs are permitted to fail by default. This means that if they fail it do not fail the pipeline. + +If you want to prevent vulnerabilities from being merged, you should do this by adding [Security Approvals in Merge Requests](#security-approvals-in-merge-requests) which prevents unknown, high or critical findings from being merged without an approval from a specific group of people that you choose. + +We do not recommend changing the job [`allow_failure` setting](../../ci/yaml/README.md#allow_failure) as that fails the entire pipeline. + +### JSON Artifact + +The artifact generated by the secure analyzer contains all findings it discovers on the target branch, regardless of whether they were previously found, dismissed, or completely new (it puts in everything that it finds). + ## View security scan information in merge requests **(FREE)** > - [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/4393) in GitLab Free 13.5. @@ -111,12 +152,44 @@ rules: > - Report download dropdown [added](https://gitlab.com/gitlab-org/gitlab/-/issues/273418) in 13.7. > - [Feature flag removed](https://gitlab.com/gitlab-org/gitlab/-/issues/249550) in GitLab 13.9. +### All tiers + Merge requests which have run security scans let you know that the generated reports are available to download. To download a report, click on the **Download results** dropdown, and select the desired report.  +### Ultimate + +A merge request contains a security widget which displays a summary of the NEW results. New results are determined by comparing the current findings against existing findings in the target (default) branch (if there are prior findings). + +We recommended you run a scan of the `default` branch before enabling feature branch scans for your developers. Otherwise, there is no base for comparison and all feature branches display the full scan results in the merge request security widget. + +The merge request security widget displays only a subset of the vulnerabilities in the generated JSON artifact because it contains both NEW and EXISTING findings. + +From the merge request security widget, select **Expand** to unfold the widget, displaying any new and no longer detected (removed) findings by scan type. Select **View Full Report** to go directly to the **Security** tab in the latest branch pipeline. + +## View security scan information in the pipeline Security tab + +A pipeline's security tab lists all findings in the current branch. It includes new findings introduced by this branch and existing vulnerabilities that were already present when the branch was created. These results likely do not match the findings displayed in the Merge Request security widget as those do not include the existing vulnerabilities (with the exception of showing any existing vulnerabilities that are no longer detected in the feature branch). + +For more details, see [security tab](security_dashboard/index.md#pipeline-security). + +## View security scan information in the Security Dashboard + +The Security Dashboard show vulnerabilities present in a project's default branch. Data is updated every 24 hours. Vulnerability count updates resulting from any feature branches introducing new vulnerabilities that are merged to default are included after the daily data refresh. + +For more details, see [Security Dashboard](security_dashboard/index.md). + +## View security scan information in the Vulnerability Report + +The vulnerability report shows the results of the last completed pipeline on the default branch. It is updated on every pipeline completion. All detected vulnerabilities are shown as well as any previous ones that are no longer detected in the latest scan. Vulnerabilities that are no longer detected may have been remediated or otherwise removed and can be marked as `Resolved` after proper verification. Vulnerabilities that are no longer detected are denoted with an icon for filtering and review. + +By default, the vulnerability report does not show vulnerabilities of `dismissed` or `resolved` status so you can focus on open vulnerabilities. You can change the Status filter to see these. + +[Read more about the Vulnerability report](vulnerability_report/index.md). + ## Security approvals in merge requests > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/9928) in [GitLab Ultimate](https://about.gitlab.com/pricing/) 12.2. @@ -258,6 +331,10 @@ You can do it quickly by following the hyperlink given to run a new pipeline.  +## DAST On-Demand Scans + +If you don’t want scans running in your normal DevOps process you can use on-demand scans instead. For more details, see [on-demand scans](dast/index.md#on-demand-scans). This feature is only available for DAST. If you run an on-demand scan against the default branch, it is reported as a "successful pipeline" and these results are included in the security dashboard and vulnerability report. + ## Security report validation > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/321918) in GitLab 13.11. @@ -298,6 +375,26 @@ For example, the configuration below enables validation for only the `sast` job: stage: security-scan ``` +## Interacting with findings and vulnerabilities + +There are a variety of locations and ways to interact with the results of the security scanning tools: + +- [Scan information in merge requests](#view-security-scan-information-in-merge-requests) +- [Project Security Dashboard](security_dashboard/#project-security-dashboard) +- [Security pipeline tab](security_dashboard/#pipeline-security) +- [Group Security Dashboard](security_dashboard/#group-security-dashboard) +- [Security Center](security_dashboard/#security-center) +- [Vulnerability Report](vulnerability_report/index.md) +- [Vulnerability Pages](vulnerabilities/index.md) +- [Dependency List](dependency_list/index.md) + +For more details about which findings or vulnerabilities you can view in each of those locations, select the respective link. Each page details the ways in which you can interact with the findings and vulnerabilities. As an example, in most cases findings start out as _detected_ status. You have the option to: + +- Change the status. +- Create an issue. +- Link it to an existing issue. +- In some cases, [apply an automatic remediation for a vulnerability](vulnerabilities/index.md#remediate-a-vulnerability-automatically). + ## Troubleshooting ### Getting error message `sast job: stage parameter should be [some stage name here]` diff --git a/lib/api/entities/issuable_entity.rb b/lib/api/entities/issuable_entity.rb index e2c674c0b8b..fd5d6c8137f 100644 --- a/lib/api/entities/issuable_entity.rb +++ b/lib/api/entities/issuable_entity.rb @@ -24,7 +24,7 @@ module API # entity according to the current top-level entity options, such # as the current_user. def lazy_issuable_metadata - BatchLoader.for(object).batch(key: [current_user, :issuable_metadata]) do |models, loader, args| + BatchLoader.for(object).batch(key: [current_user, :issuable_metadata], replace_methods: false) do |models, loader, args| current_user = args[:key].first issuable_metadata = Gitlab::IssuableMetadata.new(current_user, models) diff --git a/lib/api/entities/package.rb b/lib/api/entities/package.rb index e7153f9bebb..2f60a0bf6bd 100644 --- a/lib/api/entities/package.rb +++ b/lib/api/entities/package.rb @@ -22,6 +22,7 @@ module API expose :version expose :package_type + expose :status expose :_links do expose :web_path do |package| diff --git a/lib/api/group_export.rb b/lib/api/group_export.rb index 9c7f04352f4..6134515032f 100644 --- a/lib/api/group_export.rb +++ b/lib/api/group_export.rb @@ -48,7 +48,7 @@ module API detail 'This feature was introduced in GitLab 13.12' end post ':id/export_relations' do - response = ::BulkImports::ExportService.new(exportable: user_group, user: current_user).execute + response = ::BulkImports::ExportService.new(portable: user_group, user: current_user).execute if response.success? accepted! diff --git a/lib/gitlab/highlight.rb b/lib/gitlab/highlight.rb index 45f17d73f30..e4857280969 100644 --- a/lib/gitlab/highlight.rb +++ b/lib/gitlab/highlight.rb @@ -11,7 +11,11 @@ module Gitlab end def self.too_large?(size) - size.to_i > Gitlab.config.extra['maximum_text_highlight_size_kilobytes'] + return false unless size.to_i > Gitlab.config.extra['maximum_text_highlight_size_kilobytes'] + + over_highlight_size_limit.increment(source: "text highlighter") if Feature.enabled?(:track_file_size_over_highlight_limit) + + true end attr_reader :blob_name @@ -96,5 +100,12 @@ module Gitlab 'Counts the times highlights have timed out' ) end + + def self.over_highlight_size_limit + @over_highlight_size_limit ||= Gitlab::Metrics.counter( + :over_highlight_size_limit, + 'Count the times files have been over the highlight size limit' + ) + end end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 75cb8d700c0..cb6d7afdd1e 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -23359,6 +23359,9 @@ msgstr "" msgid "PackageRegistry|Install package version" msgstr "" +msgid "PackageRegistry|Invalid Package: failed metadata extraction" +msgstr "" + msgid "PackageRegistry|Learn how to %{noPackagesLinkStart}publish and share your packages%{noPackagesLinkEnd} with GitLab." msgstr "" @@ -31067,6 +31070,9 @@ msgstr "" msgid "Subscription deletion failed." msgstr "" +msgid "Subscription service outage" +msgstr "" + msgid "Subscription successfully applied to \"%{group_name}\"" msgstr "" @@ -31971,6 +31977,9 @@ msgstr "" msgid "The Compliance Dashboard gives you the ability to see a group's merge request activity by providing a high-level view for all projects in the group." msgstr "" +msgid "The GitLab subscription service (customers.gitlab.com) is currently experiencing an outage. You can monitor the status and get updates at %{linkStart}status.gitlab.com%{linkEnd}." +msgstr "" + msgid "The GitLab user to which the Jira user %{jiraDisplayName} will be mapped" msgstr "" diff --git a/spec/finders/deployments_finder_spec.rb b/spec/finders/deployments_finder_spec.rb index 0c8a06d7e28..b294f1117f5 100644 --- a/spec/finders/deployments_finder_spec.rb +++ b/spec/finders/deployments_finder_spec.rb @@ -201,17 +201,31 @@ RSpec.describe DeploymentsFinder do it 'adds `id` sorting as the second order column' do order_value = subject.order_values[1] - expect(order_value.to_sql).to eq(Deployment.arel_table[:id].desc.to_sql) + expect(order_value.to_sql).to eq(Deployment.arel_table[:id].asc.to_sql) end - it 'uses the `id DESC` as tie-breaker when ordering' do + it 'uses the `id ASC` as tie-breaker when ordering' do updated_at = Time.now deployment_1 = create(:deployment, :success, project: project, updated_at: updated_at) deployment_2 = create(:deployment, :success, project: project, updated_at: updated_at) deployment_3 = create(:deployment, :success, project: project, updated_at: updated_at) - expect(subject).to eq([deployment_3, deployment_2, deployment_1]) + expect(subject).to eq([deployment_1, deployment_2, deployment_3]) + end + + context 'when sort direction is desc' do + let(:params) { { **base_params, updated_after: 1.day.ago, order_by: 'updated_at', sort: 'desc' } } + + it 'uses the `id DESC` as tie-breaker when ordering' do + updated_at = Time.now + + deployment_1 = create(:deployment, :success, project: project, updated_at: updated_at) + deployment_2 = create(:deployment, :success, project: project, updated_at: updated_at) + deployment_3 = create(:deployment, :success, project: project, updated_at: updated_at) + + expect(subject).to eq([deployment_3, deployment_2, deployment_1]) + end end end @@ -228,7 +242,7 @@ RSpec.describe DeploymentsFinder do it 'sorts by `updated_at`' do expect(subject.order_values.first.to_sql).to eq(Deployment.arel_table[:updated_at].asc.to_sql) - expect(subject.order_values.second.to_sql).to eq(Deployment.arel_table[:id].desc.to_sql) + expect(subject.order_values.second.to_sql).to eq(Deployment.arel_table[:id].asc.to_sql) end context 'when deployments_finder_implicitly_enforce_ordering_for_updated_at_filter feature flag is disabled' do diff --git a/spec/fixtures/api/schemas/public_api/v4/packages/package.json b/spec/fixtures/api/schemas/public_api/v4/packages/package.json index 08909efd10c..607e0df1886 100644 --- a/spec/fixtures/api/schemas/public_api/v4/packages/package.json +++ b/spec/fixtures/api/schemas/public_api/v4/packages/package.json @@ -4,6 +4,7 @@ "name", "version", "package_type", + "status", "_links", "versions" ], @@ -20,6 +21,9 @@ "package_type": { "type": "string" }, + "status": { + "type": "string" + }, "_links": { "type": "object", "required": [ diff --git a/spec/frontend/packages/shared/components/__snapshots__/package_list_row_spec.js.snap b/spec/frontend/packages/shared/components/__snapshots__/package_list_row_spec.js.snap index 03b98478f3e..f4e617ecafe 100644 --- a/spec/frontend/packages/shared/components/__snapshots__/package_list_row_spec.js.snap +++ b/spec/frontend/packages/shared/components/__snapshots__/package_list_row_spec.js.snap @@ -34,6 +34,8 @@ exports[`packages_list_row renders 1`] = ` </gl-link-stub> <!----> + + <!----> </div> <!----> diff --git a/spec/frontend/packages/shared/components/package_list_row_spec.js b/spec/frontend/packages/shared/components/package_list_row_spec.js index fd54cd0f25d..bd15d48c4eb 100644 --- a/spec/frontend/packages/shared/components/package_list_row_spec.js +++ b/spec/frontend/packages/shared/components/package_list_row_spec.js @@ -1,8 +1,11 @@ +import { GlLink } from '@gitlab/ui'; import { shallowMount } from '@vue/test-utils'; +import { createMockDirective, getBinding } from 'helpers/vue_mock_directive'; import PackagesListRow from '~/packages/shared/components/package_list_row.vue'; import PackagePath from '~/packages/shared/components/package_path.vue'; import PackageTags from '~/packages/shared/components/package_tags.vue'; +import { PACKAGE_ERROR_STATUS } from '~/packages/shared/constants'; import ListItem from '~/vue_shared/components/registry/list_item.vue'; import { packageList } from '../../mock_data'; @@ -20,7 +23,10 @@ describe('packages_list_row', () => { const findPackagePath = () => wrapper.find(PackagePath); const findDeleteButton = () => wrapper.find('[data-testid="action-delete"]'); const findPackageIconAndName = () => wrapper.find(PackageIconAndName); - const findInfrastructureIconAndName = () => wrapper.find(InfrastructureIconAndName); + const findInfrastructureIconAndName = () => wrapper.findComponent(InfrastructureIconAndName); + const findListItem = () => wrapper.findComponent(ListItem); + const findPackageLink = () => wrapper.findComponent(GlLink); + const findWarningIcon = () => wrapper.find('[data-testid="warning-icon"]'); const mountComponent = ({ isGroup = false, @@ -44,6 +50,9 @@ describe('packages_list_row', () => { showPackageType, disableDelete, }, + directives: { + GlTooltip: createMockDirective(), + }, }); }; @@ -146,4 +155,31 @@ describe('packages_list_row', () => { expect(findInfrastructureIconAndName().exists()).toBe(true); }); }); + + describe(`when the package is in ${PACKAGE_ERROR_STATUS} status`, () => { + beforeEach(() => { + mountComponent({ packageEntity: { ...packageWithoutTags, status: PACKAGE_ERROR_STATUS } }); + }); + + it('list item has a disabled prop', () => { + expect(findListItem().props('disabled')).toBe(true); + }); + + it('details link is disabled', () => { + expect(findPackageLink().attributes('disabled')).toBe('true'); + }); + + it('has a warning icon', () => { + const icon = findWarningIcon(); + const tooltip = getBinding(icon.element, 'gl-tooltip'); + expect(icon.props('icon')).toBe('warning'); + expect(tooltip.value).toMatchObject({ + title: 'Invalid Package: failed metadata extraction', + }); + }); + + it('delete button is disabled', () => { + expect(findDeleteButton().props('disabled')).toBe(true); + }); + }); }); diff --git a/spec/frontend/packages/shared/components/package_path_spec.js b/spec/frontend/packages/shared/components/package_path_spec.js index 3c9cd3387ba..edbdd55c1d7 100644 --- a/spec/frontend/packages/shared/components/package_path_spec.js +++ b/spec/frontend/packages/shared/components/package_path_spec.js @@ -39,48 +39,66 @@ describe('PackagePath', () => { const pathPieces = path.split('/').slice(1); const hasTooltip = shouldExist.includes(ELLIPSIS_ICON); - beforeEach(() => { - mountComponent({ path }); - }); + describe('not disabled component', () => { + beforeEach(() => { + mountComponent({ path }); + }); - it('should have a base icon', () => { - expect(findItem(BASE_ICON).exists()).toBe(true); - }); + it('should have a base icon', () => { + expect(findItem(BASE_ICON).exists()).toBe(true); + }); - it('should have a root link', () => { - const root = findItem(ROOT_LINK); - expect(root.exists()).toBe(true); - expect(root.attributes('href')).toBe(rootUrl); - }); + it('should have a root link', () => { + const root = findItem(ROOT_LINK); + expect(root.exists()).toBe(true); + expect(root.attributes('href')).toBe(rootUrl); + }); - if (hasTooltip) { - it('should have a tooltip', () => { - const tooltip = findTooltip(findItem(ELLIPSIS_ICON)); - expect(tooltip).toBeDefined(); - expect(tooltip.value).toMatchObject({ - title: path, + if (hasTooltip) { + it('should have a tooltip', () => { + const tooltip = findTooltip(findItem(ELLIPSIS_ICON)); + expect(tooltip).toBeDefined(); + expect(tooltip.value).toMatchObject({ + title: path, + }); }); - }); - } + } - if (shouldExist.length) { - it.each(shouldExist)(`should have %s`, (element) => { - expect(findItem(element).exists()).toBe(true); - }); - } + if (shouldExist.length) { + it.each(shouldExist)(`should have %s`, (element) => { + expect(findItem(element).exists()).toBe(true); + }); + } - if (shouldNotExist.length) { - it.each(shouldNotExist)(`should not have %s`, (element) => { - expect(findItem(element).exists()).toBe(false); + if (shouldNotExist.length) { + it.each(shouldNotExist)(`should not have %s`, (element) => { + expect(findItem(element).exists()).toBe(false); + }); + } + + if (shouldExist.includes(LEAF_LINK)) { + it('the last link should be the last piece of the path', () => { + const leaf = findItem(LEAF_LINK); + expect(leaf.attributes('href')).toBe(`/${path}`); + expect(leaf.text()).toBe(pathPieces[pathPieces.length - 1]); + }); + } + }); + + describe('disabled component', () => { + beforeEach(() => { + mountComponent({ path, disabled: true }); }); - } - if (shouldExist.includes(LEAF_LINK)) { - it('the last link should be the last piece of the path', () => { - const leaf = findItem(LEAF_LINK); - expect(leaf.attributes('href')).toBe(`/${path}`); - expect(leaf.text()).toBe(pathPieces[pathPieces.length - 1]); + it('root link is disabled', () => { + expect(findItem(ROOT_LINK).attributes('disabled')).toBe('true'); }); - } + + if (shouldExist.includes(LEAF_LINK)) { + it('the last link is disabled', () => { + expect(findItem(LEAF_LINK).attributes('disabled')).toBe('true'); + }); + } + }); }); }); diff --git a/spec/frontend/registry/explorer/components/details_page/tags_list_row_spec.js b/spec/frontend/registry/explorer/components/details_page/tags_list_row_spec.js index 8b70f84c1bd..dc9063bde2c 100644 --- a/spec/frontend/registry/explorer/components/details_page/tags_list_row_spec.js +++ b/spec/frontend/registry/explorer/components/details_page/tags_list_row_spec.js @@ -1,5 +1,6 @@ import { GlFormCheckbox, GlSprintf, GlIcon } from '@gitlab/ui'; import { shallowMount } from '@vue/test-utils'; +import { nextTick } from 'vue'; import { createMockDirective, getBinding } from 'helpers/vue_mock_directive'; import DeleteButton from '~/registry/explorer/components/delete_button.vue'; @@ -72,8 +73,15 @@ describe('tags list row', () => { expect(findCheckbox().exists()).toBe(false); }); - it('is disabled when the digest is missing', () => { - mountComponent({ tag: { ...tag, digest: null } }); + it.each` + digest | disabled + ${'foo'} | ${true} + ${null} | ${false} + ${null} | ${true} + ${'foo'} | ${true} + `('is disabled when the digest $digest and disabled is $disabled', ({ digest, disabled }) => { + mountComponent({ tag: { ...tag, digest }, disabled }); + expect(findCheckbox().attributes('disabled')).toBe('true'); }); @@ -141,6 +149,12 @@ describe('tags list row', () => { title: tag.location, }); }); + + it('is disabled when the component is disabled', () => { + mountComponent({ ...defaultProps, disabled: true }); + + expect(findClipboardButton().attributes('disabled')).toBe('true'); + }); }); describe('warning icon', () => { @@ -266,15 +280,19 @@ describe('tags list row', () => { }); it.each` - canDelete | digest - ${true} | ${null} - ${false} | ${'foo'} - ${false} | ${null} - `('is disabled when canDelete is $canDelete and digest is $digest', ({ canDelete, digest }) => { - mountComponent({ ...defaultProps, tag: { ...tag, canDelete, digest } }); - - expect(findDeleteButton().attributes('disabled')).toBe('true'); - }); + canDelete | digest | disabled + ${true} | ${null} | ${true} + ${false} | ${'foo'} | ${true} + ${false} | ${null} | ${true} + ${true} | ${'foo'} | ${true} + `( + 'is disabled when canDelete is $canDelete and digest is $digest and disabled is $disabled', + ({ canDelete, digest, disabled }) => { + mountComponent({ ...defaultProps, tag: { ...tag, canDelete, digest }, disabled }); + + expect(findDeleteButton().attributes('disabled')).toBe('true'); + }, + ); it('delete event emits delete', () => { mountComponent(); @@ -287,13 +305,10 @@ describe('tags list row', () => { describe('details rows', () => { describe('when the tag has a digest', () => { - beforeEach(() => { + it('has 3 details rows', async () => { mountComponent(); + await nextTick(); - return wrapper.vm.$nextTick(); - }); - - it('has 3 details rows', () => { expect(findDetailsRows().length).toBe(3); }); @@ -303,17 +318,37 @@ describe('tags list row', () => { ${'manifest detail'} | ${findManifestDetail} | ${'Manifest digest: sha256:2cf3d2fdac1b04a14301d47d51cb88dcd26714c74f91440eeee99ce399089062'} | ${'log'} | ${true} ${'configuration detail'} | ${findConfigurationDetail} | ${'Configuration digest: sha256:c2613843ab33aabf847965442b13a8b55a56ae28837ce182627c0716eb08c02b'} | ${'cloud-gear'} | ${true} `('$name details row', ({ finderFunction, text, icon, clipboard }) => { - it(`has ${text} as text`, () => { + it(`has ${text} as text`, async () => { + mountComponent(); + await nextTick(); + expect(finderFunction().text()).toMatchInterpolatedText(text); }); - it(`has the ${icon} icon`, () => { + it(`has the ${icon} icon`, async () => { + mountComponent(); + await nextTick(); + expect(finderFunction().props('icon')).toBe(icon); }); - it(`is ${clipboard} that clipboard button exist`, () => { - expect(finderFunction().find(ClipboardButton).exists()).toBe(clipboard); - }); + if (clipboard) { + it(`clipboard button exist`, async () => { + mountComponent(); + await nextTick(); + + expect(finderFunction().find(ClipboardButton).exists()).toBe(clipboard); + }); + + it('is disabled when the component is disabled', async () => { + mountComponent({ ...defaultProps, disabled: true }); + await nextTick(); + + expect(finderFunction().findComponent(ClipboardButton).attributes('disabled')).toBe( + 'true', + ); + }); + } }); }); @@ -321,7 +356,7 @@ describe('tags list row', () => { it('hides the details rows', async () => { mountComponent({ tag: { ...tag, digest: null } }); - await wrapper.vm.$nextTick(); + await nextTick(); expect(findDetailsRows().length).toBe(0); }); }); diff --git a/spec/frontend/registry/explorer/components/list_page/image_list_row_spec.js b/spec/frontend/registry/explorer/components/list_page/image_list_row_spec.js index 6c897b983f7..323d7b177e7 100644 --- a/spec/frontend/registry/explorer/components/list_page/image_list_row_spec.js +++ b/spec/frontend/registry/explorer/components/list_page/image_list_row_spec.js @@ -25,10 +25,11 @@ describe('Image List Row', () => { const findDetailsLink = () => wrapper.find('[data-testid="details-link"]'); const findTagsCount = () => wrapper.find('[data-testid="tags-count"]'); - const findDeleteBtn = () => wrapper.find(DeleteButton); - const findClipboardButton = () => wrapper.find(ClipboardButton); + const findDeleteBtn = () => wrapper.findComponent(DeleteButton); + const findClipboardButton = () => wrapper.findComponent(ClipboardButton); const findWarningIcon = () => wrapper.find('[data-testid="warning-icon"]'); - const findSkeletonLoader = () => wrapper.find(GlSkeletonLoader); + const findSkeletonLoader = () => wrapper.findComponent(GlSkeletonLoader); + const findListItemComponent = () => wrapper.findComponent(ListItem); const mountComponent = (props) => { wrapper = shallowMount(Component, { @@ -52,20 +53,28 @@ describe('Image List Row', () => { wrapper = null; }); - describe('main tooltip', () => { - it(`the title is ${ROW_SCHEDULED_FOR_DELETION}`, () => { - mountComponent(); + describe('list item component', () => { + describe('tooltip', () => { + it(`the title is ${ROW_SCHEDULED_FOR_DELETION}`, () => { + mountComponent(); + + const tooltip = getBinding(wrapper.element, 'gl-tooltip'); + expect(tooltip).toBeDefined(); + expect(tooltip.value.title).toBe(ROW_SCHEDULED_FOR_DELETION); + }); - const tooltip = getBinding(wrapper.element, 'gl-tooltip'); - expect(tooltip).toBeDefined(); - expect(tooltip.value.title).toBe(ROW_SCHEDULED_FOR_DELETION); + it('is disabled when item is being deleted', () => { + mountComponent({ item: { ...item, status: IMAGE_DELETE_SCHEDULED_STATUS } }); + + const tooltip = getBinding(wrapper.element, 'gl-tooltip'); + expect(tooltip.value.disabled).toBe(false); + }); }); - it('is disabled when item is being deleted', () => { + it('is disabled when the item is in deleting status', () => { mountComponent({ item: { ...item, status: IMAGE_DELETE_SCHEDULED_STATUS } }); - const tooltip = getBinding(wrapper.element, 'gl-tooltip'); - expect(tooltip.value.disabled).toBe(false); + expect(findListItemComponent().props('disabled')).toBe(true); }); }); @@ -118,6 +127,20 @@ describe('Image List Row', () => { }, ); }); + + describe('when the item is deleting', () => { + beforeEach(() => { + mountComponent({ item: { ...item, status: IMAGE_DELETE_SCHEDULED_STATUS } }); + }); + + it('the router link is disabled', () => { + // we check the event prop as is the only workaround to disable a router link + expect(findDetailsLink().props('event')).toBe(''); + }); + it('the clipboard button is disabled', () => { + expect(findClipboardButton().attributes('disabled')).toBe('true'); + }); + }); }); describe('delete button', () => { diff --git a/spec/frontend/vue_shared/components/registry/list_item_spec.js b/spec/frontend/vue_shared/components/registry/list_item_spec.js index 33c9c808dc3..ca4bf0b0652 100644 --- a/spec/frontend/vue_shared/components/registry/list_item_spec.js +++ b/spec/frontend/vue_shared/components/registry/list_item_spec.js @@ -101,16 +101,16 @@ describe('list item', () => { }); describe('disabled prop', () => { - it('when true applies disabled-content class', () => { + it('when true applies gl-opacity-5 class', () => { mountComponent({ disabled: true }); - expect(wrapper.classes('disabled-content')).toBe(true); + expect(wrapper.classes('gl-opacity-5')).toBe(true); }); - it('when false does not apply disabled-content class', () => { + it('when false does not apply gl-opacity-5 class', () => { mountComponent({ disabled: false }); - expect(wrapper.classes('disabled-content')).toBe(false); + expect(wrapper.classes('gl-opacity-5')).toBe(false); }); }); diff --git a/spec/frontend/vue_shared/directives/validation_spec.js b/spec/frontend/vue_shared/directives/validation_spec.js index 0c84be9354f..80c283c1a91 100644 --- a/spec/frontend/vue_shared/directives/validation_spec.js +++ b/spec/frontend/vue_shared/directives/validation_spec.js @@ -1,10 +1,10 @@ import { shallowMount } from '@vue/test-utils'; -import validation from '~/vue_shared/directives/validation'; +import validation, { initForm } from '~/vue_shared/directives/validation'; describe('validation directive', () => { let wrapper; - const createComponent = ({ inputAttributes, showValidation, template } = {}) => { + const createComponentFactory = ({ inputAttributes, template, data }) => { const defaultInputAttributes = { type: 'text', required: true, @@ -23,16 +23,7 @@ describe('validation directive', () => { data() { return { attributes: inputAttributes || defaultInputAttributes, - showValidation, - form: { - state: null, - fields: { - exampleField: { - state: null, - feedback: '', - }, - }, - }, + ...data, }; }, template: template || defaultTemplate, @@ -41,6 +32,44 @@ describe('validation directive', () => { wrapper = shallowMount(component, { attachTo: document.body }); }; + const createComponent = ({ inputAttributes, showValidation, template } = {}) => + createComponentFactory({ + inputAttributes, + data: { + showValidation, + form: { + state: null, + fields: { + exampleField: { + state: null, + feedback: '', + }, + }, + }, + }, + template, + }); + + const createComponentWithInitForm = ({ inputAttributes } = {}) => + createComponentFactory({ + inputAttributes, + data: { + form: initForm({ + fields: { + exampleField: { + state: null, + value: 'lorem', + }, + }, + }), + }, + template: ` + <form> + <input v-validation:[form.showValidation] name="exampleField" v-bind="attributes" /> + </form> + `, + }); + afterEach(() => { wrapper.destroy(); wrapper = null; @@ -179,4 +208,86 @@ describe('validation directive', () => { }); }); }); + + describe('component using initForm', () => { + it('sets the form fields correctly', () => { + createComponentWithInitForm(); + + expect(getFormData().state).toBe(false); + expect(getFormData().showValidation).toBe(false); + + expect(getFormData().fields.exampleField).toMatchObject({ + value: 'lorem', + state: null, + required: true, + feedback: expect.any(String), + }); + }); + }); +}); + +describe('initForm', () => { + const MOCK_FORM = { + fields: { + name: { + value: 'lorem', + }, + description: { + value: 'ipsum', + required: false, + skipValidation: true, + }, + }, + }; + + it('returns form object', () => { + expect(initForm(MOCK_FORM)).toMatchObject({ + state: false, + showValidation: false, + fields: { + name: { value: 'lorem', required: true, state: null, feedback: null }, + description: { value: 'ipsum', required: false, state: true, feedback: null }, + }, + }); + }); + + it('returns form object with additional parameters', () => { + const customFormObject = { + foo: { + bar: 'lorem', + }, + }; + + const form = { + ...MOCK_FORM, + ...customFormObject, + }; + + expect(initForm(form)).toMatchObject({ + state: false, + showValidation: false, + fields: { + name: { value: 'lorem', required: true, state: null, feedback: null }, + description: { value: 'ipsum', required: false, state: true, feedback: null }, + }, + ...customFormObject, + }); + }); + + it('can override existing state and showValidation values', () => { + const form = { + ...MOCK_FORM, + state: true, + showValidation: true, + }; + + expect(initForm(form)).toMatchObject({ + state: true, + showValidation: true, + fields: { + name: { value: 'lorem', required: true, state: null, feedback: null }, + description: { value: 'ipsum', required: false, state: true, feedback: null }, + }, + }); + }); }); diff --git a/spec/graphql/types/project_type_spec.rb b/spec/graphql/types/project_type_spec.rb index 3ef2fbba002..0f7cadbd4a7 100644 --- a/spec/graphql/types/project_type_spec.rb +++ b/spec/graphql/types/project_type_spec.rb @@ -14,7 +14,7 @@ RSpec.describe GitlabSchema.types['Project'] do it 'has the expected fields' do expected_fields = %w[ user_permissions id full_path path name_with_namespace - name description description_html tag_list ssh_url_to_repo + name description description_html tag_list topics ssh_url_to_repo http_url_to_repo web_url star_count forks_count created_at last_activity_at archived visibility container_registry_enabled shared_runners_enabled diff --git a/spec/helpers/nav/top_nav_helper_spec.rb b/spec/helpers/nav/top_nav_helper_spec.rb index 42626ff7c23..5c9e1e82b01 100644 --- a/spec/helpers/nav/top_nav_helper_spec.rb +++ b/spec/helpers/nav/top_nav_helper_spec.rb @@ -11,12 +11,17 @@ RSpec.describe Nav::TopNavHelper do let(:current_user) { nil } let(:current_project) { nil } + let(:current_group) { nil } let(:with_current_settings_admin_mode) { false } let(:with_header_link_admin_mode) { false } + let(:with_sherlock_enabled) { false } let(:with_projects) { false } + let(:with_groups) { false } let(:with_milestones) { false } + let(:with_snippets) { false } + let(:with_activity) { false } - let(:subject) { helper.top_nav_view_model(project: current_project) } + let(:subject) { helper.top_nav_view_model(project: current_project, group: current_group) } let(:active_title) { 'Menu' } @@ -24,13 +29,17 @@ RSpec.describe Nav::TopNavHelper do allow(helper).to receive(:current_user) { current_user } allow(Gitlab::CurrentSettings).to receive(:admin_mode) { with_current_settings_admin_mode } allow(helper).to receive(:header_link?).with(:admin_mode) { with_header_link_admin_mode } + allow(Gitlab::Sherlock).to receive(:enabled?) { with_sherlock_enabled } # Defaulting all `dashboard_nav_link?` calls to false ensures the EE-specific behavior # is not enabled in this CE spec allow(helper).to receive(:dashboard_nav_link?).with(anything) { false } allow(helper).to receive(:dashboard_nav_link?).with(:projects) { with_projects } + allow(helper).to receive(:dashboard_nav_link?).with(:groups) { with_groups } allow(helper).to receive(:dashboard_nav_link?).with(:milestones) { with_milestones } + allow(helper).to receive(:dashboard_nav_link?).with(:snippets) { with_snippets } + allow(helper).to receive(:dashboard_nav_link?).with(:activity) { with_activity } end it 'has :activeTitle' do @@ -39,13 +48,30 @@ RSpec.describe Nav::TopNavHelper do context 'when current_user is nil (anonymous)' do it 'has expected :primary' do - expected_primary = ::Gitlab::Nav::TopNavMenuItem.build( + expected_projects_item = ::Gitlab::Nav::TopNavMenuItem.build( href: '/explore', icon: 'project', id: 'project', title: 'Projects' ) - expect(subject[:primary]).to eq([expected_primary]) + expected_groups_item = ::Gitlab::Nav::TopNavMenuItem.build( + href: '/explore/groups', + icon: 'group', + id: 'groups', + title: 'Groups' + ) + expected_snippets_item = ::Gitlab::Nav::TopNavMenuItem.build( + href: '/explore/snippets', + icon: 'snippet', + id: 'snippets', + title: 'Snippets' + ) + expect(subject[:primary]) + .to eq([ + expected_projects_item, + expected_groups_item, + expected_snippets_item + ]) end end @@ -124,7 +150,7 @@ RSpec.describe Nav::TopNavHelper do let_it_be(:project) { build_stubbed(:project) } let(:current_project) { project } - let(:avatar_url) { 'avatar_url' } + let(:avatar_url) { 'project_avatar_url' } before do allow(project).to receive(:persisted?) { true } @@ -146,6 +172,87 @@ RSpec.describe Nav::TopNavHelper do end end + context 'with groups' do + let(:with_groups) { true } + let(:groups_view) { subject[:views][:groups] } + + it 'has expected :primary' do + expected_primary = ::Gitlab::Nav::TopNavMenuItem.build( + css_class: 'qa-groups-dropdown', + data: { + track_event: 'click_dropdown', + track_label: 'groups_dropdown' + }, + icon: 'group', + id: 'groups', + title: 'Groups', + view: 'groups' + ) + expect(subject[:primary]).to eq([expected_primary]) + end + + context 'groups' do + it 'has expected :currentUserName' do + expect(groups_view[:currentUserName]).to eq(current_user.username) + end + + it 'has expected :namespace' do + expect(groups_view[:namespace]).to eq('groups') + end + + it 'has expected :linksPrimary' do + expected_links_primary = [ + ::Gitlab::Nav::TopNavMenuItem.build( + href: '/dashboard/groups', + id: 'your', + title: 'Your groups' + ), + ::Gitlab::Nav::TopNavMenuItem.build( + href: '/explore/groups', + id: 'explore', + title: 'Explore groups' + ) + ] + expect(groups_view[:linksPrimary]).to eq(expected_links_primary) + end + + it 'has expected :linksSecondary' do + expected_links_secondary = [ + ::Gitlab::Nav::TopNavMenuItem.build( + href: '/groups/new#create-group-pane', + id: 'create', + title: 'Create group' + ) + ] + expect(groups_view[:linksSecondary]).to eq(expected_links_secondary) + end + + context 'with persisted group' do + let_it_be(:group) { build_stubbed(:group) } + + let(:current_group) { group } + let(:avatar_url) { 'group_avatar_url' } + + before do + allow(group).to receive(:persisted?) { true } + allow(group).to receive(:avatar_url) { avatar_url } + end + + it 'has expected :container' do + expected_container = { + avatarUrl: avatar_url, + id: group.id, + name: group.name, + namespace: group.full_name, + webUrl: group_path(group) + } + + expect(groups_view[:currentItem]).to eq(expected_container) + end + end + end + end + context 'with milestones' do let(:with_milestones) { true } @@ -162,6 +269,61 @@ RSpec.describe Nav::TopNavHelper do expect(subject[:primary]).to eq([expected_primary]) end end + + context 'with snippets' do + let(:with_snippets) { true } + + it 'has expected :primary' do + expected_primary = ::Gitlab::Nav::TopNavMenuItem.build( + data: { + qa_selector: 'snippets_link' + }, + href: '/dashboard/snippets', + icon: 'snippet', + id: 'snippets', + title: 'Snippets' + ) + expect(subject[:primary]).to eq([expected_primary]) + end + end + + context 'with activity' do + let(:with_activity) { true } + + it 'has expected :primary' do + expected_primary = ::Gitlab::Nav::TopNavMenuItem.build( + data: { + qa_selector: 'activity_link' + }, + href: '/dashboard/activity', + icon: 'history', + id: 'activity', + title: 'Activity' + ) + expect(subject[:primary]).to eq([expected_primary]) + end + end + + context 'when sherlock is enabled' do + let(:with_sherlock_enabled) { true } + + before do + # Note: We have to mock the sherlock route because the route is conditional on + # sherlock being enabled, but it parsed at Rails load time and can't be overridden + # in a spec. + allow(helper).to receive(:sherlock_transactions_path) { '/fake_sherlock_path' } + end + + it 'has sherlock as last :secondary item' do + expected_sherlock_item = ::Gitlab::Nav::TopNavMenuItem.build( + id: 'sherlock', + title: 'Sherlock Transactions', + icon: 'admin', + href: '/fake_sherlock_path' + ) + expect(subject[:secondary].last).to eq(expected_sherlock_item) + end + end end context 'when current_user is admin' do diff --git a/spec/lib/gitlab/highlight_spec.rb b/spec/lib/gitlab/highlight_spec.rb index 5ec66e7f6a8..a5e4d37d306 100644 --- a/spec/lib/gitlab/highlight_spec.rb +++ b/spec/lib/gitlab/highlight_spec.rb @@ -46,12 +46,20 @@ RSpec.describe Gitlab::Highlight do expect(result).to eq(%[<span id="LC1" class="line" lang="plaintext">plain text contents</span>]) end - it 'returns plain version for long content' do - stub_config(extra: { 'maximum_text_highlight_size_kilobytes' => 0.0001 } ) # 1.024 bytes + context 'when content is too long to be highlighted' do + let(:result) { described_class.highlight(file_name, content) } # content is 44 bytes - result = described_class.highlight(file_name, content) # content is 44 bytes + before do + stub_config(extra: { 'maximum_text_highlight_size_kilobytes' => 0.0001 } ) # 1.024 bytes + end + + it 'increments the metric for oversized files' do + expect { result }.to change { over_highlight_size_limit('text highlighter') }.by(1) + end - expect(result).to eq(%[<span id="LC1" class="line" lang="">(make-pathname :defaults name</span>\n<span id="LC2" class="line" lang="">:type "assem")</span>]) + it 'returns plain version for long content' do + expect(result).to eq(%[<span id="LC1" class="line" lang="">(make-pathname :defaults name</span>\n<span id="LC2" class="line" lang="">:type "assem")</span>]) + end end it 'highlights multi-line comments' do @@ -168,4 +176,11 @@ RSpec.describe Gitlab::Highlight do .counter(:highlight_timeout, 'Counts the times highlights have timed out') .get(source: source) end + + def over_highlight_size_limit(source) + Gitlab::Metrics + .counter(:over_highlight_size_limit, + 'Count the times text has been over the highlight size limit') + .get(source: source) + end end diff --git a/spec/models/bulk_imports/export_spec.rb b/spec/models/bulk_imports/export_spec.rb index 26d25e6901e..d85b77d599b 100644 --- a/spec/models/bulk_imports/export_spec.rb +++ b/spec/models/bulk_imports/export_spec.rb @@ -47,12 +47,12 @@ RSpec.describe BulkImports::Export, type: :model do end end - describe '#exportable' do + describe '#portable' do context 'when associated with project' do it 'returns project' do export = create(:bulk_import_export, project: create(:project), group: nil) - expect(export.exportable).to be_instance_of(Project) + expect(export.portable).to be_instance_of(Project) end end @@ -60,7 +60,7 @@ RSpec.describe BulkImports::Export, type: :model do it 'returns group' do export = create(:bulk_import_export) - expect(export.exportable).to be_instance_of(Group) + expect(export.portable).to be_instance_of(Group) end end end @@ -70,7 +70,7 @@ RSpec.describe BulkImports::Export, type: :model do it 'returns project config' do export = create(:bulk_import_export, project: create(:project), group: nil) - expect(export.config).to be_instance_of(BulkImports::Exports::ProjectConfig) + expect(export.config).to be_instance_of(BulkImports::FileTransfer::ProjectConfig) end end @@ -78,7 +78,7 @@ RSpec.describe BulkImports::Export, type: :model do it 'returns group config' do export = create(:bulk_import_export) - expect(export.config).to be_instance_of(BulkImports::Exports::GroupConfig) + expect(export.config).to be_instance_of(BulkImports::FileTransfer::GroupConfig) end end end diff --git a/spec/models/bulk_imports/exports/group_config_spec.rb b/spec/models/bulk_imports/file_transfer/group_config_spec.rb index 856977c9310..21da71de3c7 100644 --- a/spec/models/bulk_imports/exports/group_config_spec.rb +++ b/spec/models/bulk_imports/file_transfer/group_config_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe BulkImports::Exports::GroupConfig do +RSpec.describe BulkImports::FileTransfer::GroupConfig do let_it_be(:exportable) { create(:group) } let_it_be(:hex) { '123' } @@ -18,7 +18,7 @@ RSpec.describe BulkImports::Exports::GroupConfig do expect(finder).to receive(:find_root).with(:group).and_call_original end - expect(subject.exportable_tree).not_to be_empty + expect(subject.portable_tree).not_to be_empty end end @@ -32,7 +32,7 @@ RSpec.describe BulkImports::Exports::GroupConfig do describe '#exportable_relations' do it 'returns a list of top level exportable relations' do - expect(subject.exportable_relations).to include('milestones', 'badges', 'boards', 'labels') + expect(subject.portable_relations).to include('milestones', 'badges', 'boards', 'labels') end end end diff --git a/spec/models/bulk_imports/exports/project_config_spec.rb b/spec/models/bulk_imports/file_transfer/project_config_spec.rb index c0b685a091d..021f96ac2a3 100644 --- a/spec/models/bulk_imports/exports/project_config_spec.rb +++ b/spec/models/bulk_imports/file_transfer/project_config_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe BulkImports::Exports::ProjectConfig do +RSpec.describe BulkImports::FileTransfer::ProjectConfig do let_it_be(:exportable) { create(:project) } let_it_be(:hex) { '123' } @@ -18,7 +18,7 @@ RSpec.describe BulkImports::Exports::ProjectConfig do expect(finder).to receive(:find_root).with(:project).and_call_original end - expect(subject.exportable_tree).not_to be_empty + expect(subject.portable_tree).not_to be_empty end end @@ -32,7 +32,7 @@ RSpec.describe BulkImports::Exports::ProjectConfig do describe '#exportable_relations' do it 'returns a list of top level exportable relations' do - expect(subject.exportable_relations).to include('issues', 'labels', 'milestones', 'merge_requests') + expect(subject.portable_relations).to include('issues', 'labels', 'milestones', 'merge_requests') end end end diff --git a/spec/models/bulk_imports/file_transfer_spec.rb b/spec/models/bulk_imports/file_transfer_spec.rb new file mode 100644 index 00000000000..5a2b303626c --- /dev/null +++ b/spec/models/bulk_imports/file_transfer_spec.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe BulkImports::FileTransfer do + describe '.config_for' do + context 'when portable is group' do + it 'returns group config' do + expect(described_class.config_for(build(:group))).to be_instance_of(BulkImports::FileTransfer::GroupConfig) + end + end + + context 'when portable is project' do + it 'returns project config' do + expect(described_class.config_for(build(:project))).to be_instance_of(BulkImports::FileTransfer::ProjectConfig) + end + end + + context 'when portable is unsupported' do + it 'raises an error' do + expect { described_class.config_for(nil) }.to raise_error(BulkImports::FileTransfer::UnsupportedObjectType) + end + end + end +end diff --git a/spec/presenters/packages/detail/package_presenter_spec.rb b/spec/presenters/packages/detail/package_presenter_spec.rb index 6f4ae5a5a96..d8f1c98e762 100644 --- a/spec/presenters/packages/detail/package_presenter_spec.rb +++ b/spec/presenters/packages/detail/package_presenter_spec.rb @@ -50,6 +50,7 @@ RSpec.describe ::Packages::Detail::PackagePresenter do name: package.name, package_files: expected_package_files, package_type: package.package_type, + status: package.status, project_id: package.project_id, tags: package.tags.as_json, updated_at: package.updated_at, diff --git a/spec/requests/api/graphql/project_query_spec.rb b/spec/requests/api/graphql/project_query_spec.rb index 2cdd7273b18..b367bbaaf43 100644 --- a/spec/requests/api/graphql/project_query_spec.rb +++ b/spec/requests/api/graphql/project_query_spec.rb @@ -57,6 +57,22 @@ RSpec.describe 'getting project information' do end end + context 'topics' do + it 'includes empty topics array if no topics set' do + post_graphql(query, current_user: current_user) + + expect(graphql_data_at(:project, :topics)).to match([]) + end + + it 'includes topics array' do + project.update!(tag_list: 'topic1, topic2, topic3') + + post_graphql(query, current_user: current_user) + + expect(graphql_data_at(:project, :topics)).to match(%w[topic1 topic2 topic3]) + end + end + it 'includes inherited members in project_members' do group_member = create(:group_member, group: group) project_member = create(:project_member, project: project) diff --git a/spec/services/bulk_imports/export_service_spec.rb b/spec/services/bulk_imports/export_service_spec.rb index 4a31094a22a..2414f7c5ca7 100644 --- a/spec/services/bulk_imports/export_service_spec.rb +++ b/spec/services/bulk_imports/export_service_spec.rb @@ -10,12 +10,12 @@ RSpec.describe BulkImports::ExportService do group.add_owner(user) end - subject { described_class.new(exportable: group, user: user) } + subject { described_class.new(portable: group, user: user) } describe '#execute' do it 'schedules RelationExportWorker for each top level relation' do expect(subject).to receive(:execute).and_return(ServiceResponse.success).and_call_original - top_level_relations = BulkImports::Export.config(group).exportable_relations + top_level_relations = BulkImports::FileTransfer.config_for(group).portable_relations top_level_relations.each do |relation| expect(BulkImports::RelationExportWorker) @@ -28,7 +28,7 @@ RSpec.describe BulkImports::ExportService do context 'when exception occurs' do it 'does not schedule RelationExportWorker' do - service = described_class.new(exportable: nil, user: user) + service = described_class.new(portable: nil, user: user) expect(service) .to receive(:execute) diff --git a/spec/services/bulk_imports/relation_export_service_spec.rb b/spec/services/bulk_imports/relation_export_service_spec.rb index 1116fb1f988..bf286998df2 100644 --- a/spec/services/bulk_imports/relation_export_service_spec.rb +++ b/spec/services/bulk_imports/relation_export_service_spec.rb @@ -77,7 +77,7 @@ RSpec.describe BulkImports::RelationExportService do it 'tracks exception' do expect(Gitlab::ErrorTracking) .to receive(:track_exception) - .with(exception_class, exportable_id: group.id, exportable_type: group.class.name) + .with(exception_class, portable_id: group.id, portable_type: group.class.name) .and_call_original subject.execute diff --git a/spec/services/packages/rubygems/process_gem_service_spec.rb b/spec/services/packages/rubygems/process_gem_service_spec.rb index 83e868d9579..64deb39c6d8 100644 --- a/spec/services/packages/rubygems/process_gem_service_spec.rb +++ b/spec/services/packages/rubygems/process_gem_service_spec.rb @@ -16,12 +16,11 @@ RSpec.describe Packages::Rubygems::ProcessGemService do describe '#execute' do subject { service.execute } - context 'no gem file', :aggregate_failures do + context 'no gem file' do let(:package_file) { nil } it 'returns an error' do - expect(subject.error?).to be(true) - expect(subject.message).to eq('Gem was not processed') + expect { subject }.to raise_error(::Packages::Rubygems::ProcessGemService::ExtractionError, 'Gem was not processed - package_file is not set') end end diff --git a/spec/workers/packages/nuget/extraction_worker_spec.rb b/spec/workers/packages/nuget/extraction_worker_spec.rb index 4703afc9413..3cc2c79176b 100644 --- a/spec/workers/packages/nuget/extraction_worker_spec.rb +++ b/spec/workers/packages/nuget/extraction_worker_spec.rb @@ -14,14 +14,15 @@ RSpec.describe Packages::Nuget::ExtractionWorker, type: :worker do subject { described_class.new.perform(package_file_id) } shared_examples 'handling the metadata error' do |exception_class: ::Packages::Nuget::UpdatePackageFromMetadataService::InvalidMetadataError| - it 'removes the package and the package file' do + it 'updates package status to error', :aggregate_failures do expect(Gitlab::ErrorTracking).to receive(:log_exception).with( instance_of(exception_class), project_id: package.project_id ) - expect { subject } - .to change { Packages::Package.count }.by(-1) - .and change { Packages::PackageFile.count }.by(-1) + + subject + + expect(package.reload).to be_error end end diff --git a/spec/workers/packages/rubygems/extraction_worker_spec.rb b/spec/workers/packages/rubygems/extraction_worker_spec.rb index 15c0a3be90c..6f65dceacf7 100644 --- a/spec/workers/packages/rubygems/extraction_worker_spec.rb +++ b/spec/workers/packages/rubygems/extraction_worker_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' RSpec.describe Packages::Rubygems::ExtractionWorker, type: :worker do describe '#perform' do - let_it_be(:package) { create(:rubygems_package) } + let_it_be(:package) { create(:rubygems_package, :processing) } let(:package_file) { package.package_files.first } let(:package_file_id) { package_file.id } @@ -14,15 +14,13 @@ RSpec.describe Packages::Rubygems::ExtractionWorker, type: :worker do subject { described_class.new.perform(*job_args) } - include_examples 'an idempotent worker' do - it 'processes the gem', :aggregate_failures do - expect { subject } - .to change { Packages::Package.count }.by(0) - .and change { Packages::PackageFile.count }.by(2) + it 'processes the gem', :aggregate_failures do + expect { subject } + .to change { Packages::Package.count }.by(0) + .and change { Packages::PackageFile.count }.by(1) - expect(Packages::Package.last.id).to be(package.id) - expect(package.name).not_to be(package_name) - end + expect(Packages::Package.last.id).to be(package.id) + expect(package.name).not_to be(package_name) end it 'handles a processing failure', :aggregate_failures do @@ -34,9 +32,9 @@ RSpec.describe Packages::Rubygems::ExtractionWorker, type: :worker do project_id: package.project_id ) - expect { subject } - .to change { Packages::Package.count }.by(-1) - .and change { Packages::PackageFile.count }.by(-2) + subject + + expect(package.reload).to be_error end context 'returns when there is no package file' do |