diff options
131 files changed, 1699 insertions, 610 deletions
diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 7d879d18e79..d01b3fb91da 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -18,17 +18,6 @@ Capybara/CurrentPathExpectation: Layout/ArgumentAlignment: Enabled: false -# Offense count: 13 -# Cop supports --auto-correct. -Layout/ClosingParenthesisIndentation: - Exclude: - - 'db/post_migrate/20180704145007_update_project_indexes.rb' - - 'ee/db/geo/migrate/20180405074130_add_partial_index_project_repository_verification.rb' - - 'spec/services/issues/resolve_discussions_spec.rb' - - 'spec/services/projects/update_service_spec.rb' - - 'spec/support/helpers/stub_object_storage.rb' - - 'spec/workers/remove_unreferenced_lfs_objects_worker_spec.rb' - # Offense count: 72 # Cop supports --auto-correct. Layout/EmptyLinesAroundArguments: @@ -57,17 +46,6 @@ Layout/FirstArrayElementIndentation: Layout/FirstHashElementIndentation: Enabled: false -# Offense count: 4 -# Cop supports --auto-correct. -# Configuration parameters: EnforcedStyle, IndentationWidth. -# SupportedStyles: consistent, align_parentheses -Layout/FirstParameterIndentation: - Exclude: - - 'lib/gitlab/cross_project_access.rb' - - 'lib/gitlab/data_builder/push.rb' - - 'spec/support/helpers/repo_helpers.rb' - - 'spec/support/helpers/stub_object_storage.rb' - # Offense count: 2164 # Cop supports --auto-correct. # Configuration parameters: AllowMultipleStyles, EnforcedHashRocketStyle, EnforcedColonStyle, EnforcedLastArgumentHashStyle. @@ -91,33 +69,11 @@ Layout/LineLength: Layout/MultilineOperationIndentation: Enabled: false -# Offense count: 9 -# Cop supports --auto-correct. -Layout/RescueEnsureAlignment: - Exclude: - - 'app/models/blob_viewer/dependency_manager.rb' - - 'app/models/project.rb' - - 'app/services/prometheus/proxy_service.rb' - - 'app/workers/delete_stored_files_worker.rb' - - 'config/initializers/1_settings.rb' - - 'config/initializers/trusted_proxies.rb' - - 'lib/gitlab/background_migration/archive_legacy_traces.rb' - - 'lib/gitlab/highlight.rb' - - 'lib/tasks/gitlab/lfs/migrate.rake' - # Offense count: 36 # Cop supports --auto-correct. Layout/SpaceAroundMethodCallOperator: Enabled: false -# Offense count: 2 -# Cop supports --auto-correct. -# Configuration parameters: AllowForAlignment. -Layout/SpaceBeforeFirstArg: - Exclude: - - 'spec/requests/api/runner_spec.rb' - - 'spec/support/shared_examples/requests/api/custom_attributes_shared_examples.rb' - # Offense count: 642 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle. @@ -153,15 +109,6 @@ Lint/NonDeterministicRequireOrder: - 'qa/spec/spec_helper.rb' - 'spec/spec_helper.rb' -# Offense count: 3 -# Configuration parameters: AllowedImplicitNamespaces. -# AllowedImplicitNamespaces: Gem -Lint/RaiseException: - Exclude: - - 'db/migrate/20190402150158_backport_enterprise_schema.rb' - - 'ee/spec/requests/api/helpers_spec.rb' - - 'spec/requests/api/helpers_spec.rb' - # Offense count: 27 # Cop supports --auto-correct. Lint/RedundantCopDisableDirective: @@ -818,14 +765,6 @@ Style/RescueModifier: Style/RescueStandardError: Enabled: false -# Offense count: 4 -# Cop supports --auto-correct. -Style/SelfAssignment: - Exclude: - - 'app/models/concerns/bulk_member_access_load.rb' - - 'app/serializers/base_serializer.rb' - - 'spec/support/import_export/configuration_helper.rb' - # Offense count: 50 # Cop supports --auto-correct. # Configuration parameters: AllowIfMethodIsEmpty. diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 0d9079f6165..acaadbc3902 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -12dcff902c9a2178fa6f4992d9d562ad9b422dd2 +12d115c50517935dc8e7e2e1248aa450bf00710e diff --git a/app/assets/javascripts/integrations/edit/components/jira_issues_fields.vue b/app/assets/javascripts/integrations/edit/components/jira_issues_fields.vue index 5a1f86718b0..1baa2b440b0 100644 --- a/app/assets/javascripts/integrations/edit/components/jira_issues_fields.vue +++ b/app/assets/javascripts/integrations/edit/components/jira_issues_fields.vue @@ -37,6 +37,11 @@ export default { required: false, default: null, }, + gitlabIssuesEnabled: { + type: Boolean, + required: false, + default: true, + }, upgradePlanPath: { type: String, required: false, @@ -133,7 +138,7 @@ export default { :disabled="!enableJiraIssues" /> </gl-form-group> - <p> + <p v-if="gitlabIssuesEnabled"> <gl-sprintf :message=" s__( diff --git a/app/assets/javascripts/integrations/edit/index.js b/app/assets/javascripts/integrations/edit/index.js index 915884dabef..2ee25bcb1e7 100644 --- a/app/assets/javascripts/integrations/edit/index.js +++ b/app/assets/javascripts/integrations/edit/index.js @@ -34,6 +34,7 @@ function parseDatasetToProps(data) { enableComments, showJiraIssuesIntegration, enableJiraIssues, + gitlabIssuesEnabled, } = parseBooleanInData(booleanAttributes); return { @@ -50,6 +51,7 @@ function parseDatasetToProps(data) { showJiraIssuesIntegration, initialEnableJiraIssues: enableJiraIssues, initialProjectKey: projectKey, + gitlabIssuesEnabled, upgradePlanPath, editProjectPath, }, diff --git a/app/assets/javascripts/issuables_list/components/issuable.vue b/app/assets/javascripts/issuables_list/components/issuable.vue index a749e0698ec..c92154017f8 100644 --- a/app/assets/javascripts/issuables_list/components/issuable.vue +++ b/app/assets/javascripts/issuables_list/components/issuable.vue @@ -158,7 +158,7 @@ export default { value: this.issuable.merge_requests_count, title: __('Related merge requests'), dataTestId: 'merge-requests', - class: 'js-merge-requests icon-merge-request-unmerged', + class: 'js-merge-requests', icon: 'merge-request', }, { diff --git a/app/assets/javascripts/issuables_list/service_desk_helper.js b/app/assets/javascripts/issuables_list/service_desk_helper.js index 71a7dcdd3ba..0a34b754377 100644 --- a/app/assets/javascripts/issuables_list/service_desk_helper.js +++ b/app/assets/javascripts/issuables_list/service_desk_helper.js @@ -1,54 +1,111 @@ import { __ } from '~/locale'; /** + * Generates empty state messages for Service Desk issues list. + * + * @param {emptyStateMeta} emptyStateMeta - Meta data used to generate empty state messages + * @returns {Object} Object containing empty state messages generated using the meta data. + */ +export function generateMessages(emptyStateMeta) { + const { + svgPath, + serviceDeskHelpPage, + serviceDeskAddress, + editProjectPage, + incomingEmailHelpPage, + } = emptyStateMeta; + + const serviceDeskSupportedTitle = __( + 'Use Service Desk to connect with your users (e.g. to offer customer support) through email right inside GitLab', + ); + + const serviceDeskSupportedMessage = __( + 'Those emails automatically become issues (with the comments becoming the email conversation) listed here.', + ); + + const commonDescription = ` + <span>${serviceDeskSupportedMessage}</span> + <a href="${serviceDeskHelpPage}">${__('Read more')}</a>`; + + return { + serviceDeskEnabledAndCanEditProjectSettings: { + title: serviceDeskSupportedTitle, + svgPath, + description: `<p>${__('Have your users email')} + <code>${serviceDeskAddress}</code> + </p> + ${commonDescription}`, + }, + serviceDeskEnabledAndCannotEditProjectSettings: { + title: serviceDeskSupportedTitle, + svgPath, + description: commonDescription, + }, + serviceDeskDisabledAndCanEditProjectSettings: { + title: serviceDeskSupportedTitle, + svgPath, + description: commonDescription, + primaryLink: editProjectPage, + primaryText: __('Turn on Service Desk'), + }, + serviceDeskDisabledAndCannotEditProjectSettings: { + title: serviceDeskSupportedTitle, + svgPath, + description: commonDescription, + }, + serviceDeskIsNotSupported: { + title: __('Service Desk is not supported'), + svgPath, + description: __( + 'In order to enable Service Desk for your instance, you must first set up incoming email.', + ), + primaryLink: incomingEmailHelpPage, + primaryText: __('More information'), + }, + serviceDeskIsNotEnabled: { + title: __('Service Desk is not enabled'), + svgPath, + description: __( + 'For help setting up the Service Desk for your instance, please contact an administrator.', + ), + }, + }; +} + +/** * Returns the attributes used for gl-empty-state in the Service Desk issues list. + * + * @param {Object} emptyStateMeta - Meta data used to generate empty state messages + * @returns {Object} */ export function emptyStateHelper(emptyStateMeta) { - const { isServiceDeskSupported, svgPath, serviceDeskHelpPage } = emptyStateMeta; + const messages = generateMessages(emptyStateMeta); + + const { isServiceDeskSupported, canEditProjectSettings, isServiceDeskEnabled } = emptyStateMeta; if (isServiceDeskSupported) { - const title = __( - 'Use Service Desk to connect with your users (e.g. to offer customer support) through email right inside GitLab', - ); - const commonMessage = __( - 'Those emails automatically become issues (with the comments becoming the email conversation) listed here.', - ); - const commonDescription = ` - <span>${commonMessage}</span> - <a href="${serviceDeskHelpPage}">${__('Read more')}</a>`; - - if (emptyStateMeta.canEditProjectSettings && emptyStateMeta.isServiceDeskEnabled) { - return { - title, - svgPath, - description: `<p>${__('Have your users email')} <code>${ - emptyStateMeta.serviceDeskAddress - }</code></p> ${commonDescription}`, - }; + if (isServiceDeskEnabled && canEditProjectSettings) { + return messages.serviceDeskEnabledAndCanEditProjectSettings; } - if (emptyStateMeta.canEditProjectSettings && !emptyStateMeta.isServiceDeskEnabled) { - return { - title, - svgPath, - description: commonDescription, - primaryLink: emptyStateMeta.editProjectPage, - primaryText: __('Turn on Service Desk'), - }; + if (isServiceDeskEnabled && !canEditProjectSettings) { + return messages.serviceDeskEnabledAndCannotEditProjectSettings; } - return { - title, - svgPath, - description: commonDescription, - }; + // !isServiceDeskEnabled && canEditProjectSettings + if (canEditProjectSettings) { + return messages.serviceDeskDisabledAndCanEditProjectSettings; + } + + // !isServiceDeskEnabled && !canEditProjectSettings + return messages.serviceDeskDisabledAndCannotEditProjectSettings; } - return { - title: __('Service Desk is enabled but not yet active'), - svgPath, - description: __('You must set up incoming email before it becomes active.'), - primaryLink: emptyStateMeta.incomingEmailHelpPage, - primaryText: __('More information'), - }; + // !serviceDeskSupported && canEditProjectSettings + if (canEditProjectSettings) { + return messages.serviceDeskIsNotSupported; + } + + // !serviceDeskSupported && !canEditProjectSettings + return messages.serviceDeskIsNotEnabled; } diff --git a/app/assets/javascripts/issue_show/components/description.vue b/app/assets/javascripts/issue_show/components/description.vue index 9cf46be5345..4c586f6c3a6 100644 --- a/app/assets/javascripts/issue_show/components/description.vue +++ b/app/assets/javascripts/issue_show/components/description.vue @@ -48,11 +48,16 @@ export default { return { preAnimation: false, pulseAnimation: false, + initialUpdate: true, }; }, watch: { - descriptionHtml() { - this.animateChange(); + descriptionHtml(newDescription, oldDescription) { + if (!this.initialUpdate && newDescription !== oldDescription) { + this.animateChange(); + } else { + this.initialUpdate = false; + } this.$nextTick(() => { this.renderGFM(); diff --git a/app/assets/javascripts/issue_show/components/pinned_links.vue b/app/assets/javascripts/issue_show/components/pinned_links.vue index 36375ca743b..d38189307bd 100644 --- a/app/assets/javascripts/issue_show/components/pinned_links.vue +++ b/app/assets/javascripts/issue_show/components/pinned_links.vue @@ -20,20 +20,25 @@ export default { }, computed: { pinnedLinks() { - return [ - { + const links = []; + if (this.publishedIncidentUrl) { + links.push({ id: 'publishedIncidentUrl', url: this.publishedIncidentUrl, text: STATUS_PAGE_PUBLISHED, icon: 'tanuki', - }, - { + }); + } + if (this.zoomMeetingUrl) { + links.push({ id: 'zoomMeetingUrl', url: this.zoomMeetingUrl, text: JOIN_ZOOM_MEETING, icon: 'brand-zoom', - }, - ]; + }); + } + + return links; }, }, methods: { @@ -45,7 +50,7 @@ export default { </script> <template> - <div class="gl-display-flex gl-justify-content-start"> + <div v-if="pinnedLinks && pinnedLinks.length" class="gl-display-flex gl-justify-content-start"> <template v-for="(link, i) in pinnedLinks"> <div v-if="link.url" :key="link.id" :class="{ 'gl-pr-3': needsPaddingClass(i) }"> <gl-button 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 e000279b794..6ba0ca24c5b 100644 --- a/app/assets/javascripts/packages/shared/components/package_list_row.vue +++ b/app/assets/javascripts/packages/shared/components/package_list_row.vue @@ -4,6 +4,7 @@ import PackageTags from './package_tags.vue'; import PublishMethod from './publish_method.vue'; import { getPackageTypeLabel } from '../utils'; import timeagoMixin from '~/vue_shared/mixins/timeago'; +import ListItem from '~/vue_shared/components/registry/list_item.vue'; export default { name: 'PackageListRow', @@ -14,6 +15,7 @@ export default { GlSprintf, PackageTags, PublishMethod, + ListItem, }, directives: { GlTooltip: GlTooltipDirective, @@ -59,14 +61,10 @@ export default { </script> <template> - <div class="gl-responsive-table-row" data-qa-selector="packages-row"> - <div class="table-section section-50 d-flex flex-md-column justify-content-between flex-wrap"> - <div class="d-flex align-items-center mr-2"> - <gl-link - :href="packageLink" - data-qa-selector="package_link" - class="text-dark font-weight-bold mb-md-1" - > + <list-item data-qa-selector="packages-row"> + <template #left-primary> + <div class="gl-display-flex gl-align-items-center gl-mr-3"> + <gl-link :href="packageLink" class="gl-text-body" data-qa-selector="package_link"> {{ packageEntity.name }} </gl-link> @@ -78,52 +76,51 @@ export default { :tag-display-limit="1" /> </div> - - <div class="d-flex text-secondary text-truncate mt-md-2"> + </template> + <template #left-secondary> + <div class="gl-display-flex"> <span>{{ packageEntity.version }}</span> - <div v-if="hasPipeline" class="d-none d-md-inline-block ml-1"> + <div v-if="hasPipeline" class="gl-display-none gl-display-sm-flex gl-ml-2"> <gl-sprintf :message="s__('PackageRegistry|published by %{author}')"> <template #author>{{ packageEntity.pipeline.user.name }}</template> </gl-sprintf> </div> - <div v-if="hasProjectLink" class="d-flex align-items-center"> - <gl-icon name="review-list" class="text-secondary ml-2 mr-1" /> + <div v-if="hasProjectLink" class="gl-display-flex gl-align-items-center"> + <gl-icon name="review-list" class="gl-ml-3 gl-mr-2" /> <gl-link + class="gl-text-body" data-testid="packages-row-project" :href="`/${packageEntity.project_path}`" - class="text-secondary" - >{{ packageEntity.projectPathName }}</gl-link > + {{ packageEntity.projectPathName }} + </gl-link> </div> <div v-if="showPackageType" class="d-flex align-items-center" data-testid="package-type"> - <gl-icon name="package" class="text-secondary ml-2 mr-1" /> + <gl-icon name="package" class="gl-ml-3 gl-mr-2" /> <span>{{ packageType }}</span> </div> </div> - </div> + </template> - <div - class="table-section d-flex flex-md-column justify-content-between align-items-md-end" - :class="disableDelete ? 'section-50' : 'section-40'" - > + <template #right-primary> <publish-method :package-entity="packageEntity" :is-group="isGroup" /> + </template> - <div class="text-secondary order-0 order-md-1 mt-md-2"> - <gl-sprintf :message="__('Created %{timestamp}')"> - <template #timestamp> - <span v-gl-tooltip :title="tooltipTitle(packageEntity.created_at)"> - {{ timeFormatted(packageEntity.created_at) }} - </span> - </template> - </gl-sprintf> - </div> - </div> + <template #right-secondary> + <gl-sprintf :message="__('Created %{timestamp}')"> + <template #timestamp> + <span v-gl-tooltip :title="tooltipTitle(packageEntity.created_at)"> + {{ timeFormatted(packageEntity.created_at) }} + </span> + </template> + </gl-sprintf> + </template> - <div v-if="!disableDelete" class="table-section section-10 d-flex justify-content-end"> + <template v-if="!disableDelete" #right-action> <gl-button data-testid="action-delete" icon="remove" @@ -134,6 +131,6 @@ export default { :disabled="!packageEntity._links.delete_api_path" @click="$emit('packageToDelete', packageEntity)" /> - </div> - </div> + </template> + </list-item> </template> diff --git a/app/assets/javascripts/packages/shared/components/publish_method.vue b/app/assets/javascripts/packages/shared/components/publish_method.vue index 1e18562a421..84e89d3b2d4 100644 --- a/app/assets/javascripts/packages/shared/components/publish_method.vue +++ b/app/assets/javascripts/packages/shared/components/publish_method.vue @@ -36,10 +36,10 @@ export default { </script> <template> - <div class="d-flex align-items-center text-secondary order-1 order-md-0 mb-md-1"> + <div class="d-flex align-items-center order-1 order-md-0 mb-md-1"> <template v-if="hasPipeline"> <gl-icon name="git-merge" class="mr-1" /> - <strong ref="pipeline-ref" class="mr-1 text-dark">{{ packageEntity.pipeline.ref }}</strong> + <span ref="pipeline-ref" class="mr-1">{{ packageEntity.pipeline.ref }}</span> <gl-icon name="commit" class="mr-1" /> <gl-link ref="pipeline-sha" :href="linkToCommit" class="mr-1">{{ packageShaShort }}</gl-link> @@ -47,15 +47,13 @@ export default { <clipboard-button :text="packageEntity.pipeline.sha" :title="__('Copy commit SHA')" - css-class="border-0 text-secondary py-0 px-1" + css-class="border-0 py-0 px-1" /> </template> <template v-else> <gl-icon name="upload" class="mr-1" /> - <strong ref="manual-ref" class="text-dark">{{ - s__('PackageRegistry|Manually Published') - }}</strong> + <span ref="manual-ref">{{ s__('PackageRegistry|Manually Published') }}</span> </template> </div> </template> diff --git a/app/assets/javascripts/registry/explorer/components/delete_button.vue b/app/assets/javascripts/registry/explorer/components/delete_button.vue index dab6a26ea16..ee856a3e546 100644 --- a/app/assets/javascripts/registry/explorer/components/delete_button.vue +++ b/app/assets/javascripts/registry/explorer/components/delete_button.vue @@ -47,7 +47,6 @@ export default { :disabled="disabled" :title="title" :aria-label="title" - category="secondary" variant="danger" icon="remove" @click="$emit('delete')" diff --git a/app/assets/javascripts/registry/explorer/components/details_page/tags_list.vue b/app/assets/javascripts/registry/explorer/components/details_page/tags_list.vue index 8494967ab57..328026d0953 100644 --- a/app/assets/javascripts/registry/explorer/components/details_page/tags_list.vue +++ b/app/assets/javascripts/registry/explorer/components/details_page/tags_list.vue @@ -67,7 +67,6 @@ export default { :key="tag.path" :tag="tag" :first="index === 0" - :last="index === tags.length - 1" :selected="selectedItems[tag.name]" :is-desktop="isDesktop" @select="updateSelectedItems(tag.name)" 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 8ec5cebbe8e..a4c497d0f59 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 @@ -5,8 +5,8 @@ import ClipboardButton from '~/vue_shared/components/clipboard_button.vue'; import { numberToHumanSize } from '~/lib/utils/number_utils'; import TimeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue'; import { formatDate } from '~/lib/utils/datetime_utility'; +import ListItem from '~/vue_shared/components/registry/list_item.vue'; import DeleteButton from '../delete_button.vue'; -import ListItem from '../list_item.vue'; import DetailsRow from '~/registry/shared/components/details_row.vue'; import { REMOVE_TAG_BUTTON_TITLE, diff --git a/app/assets/javascripts/registry/explorer/components/list_page/image_list.vue b/app/assets/javascripts/registry/explorer/components/list_page/image_list.vue index 65cf51fd1d1..d1b9894da0e 100644 --- a/app/assets/javascripts/registry/explorer/components/list_page/image_list.vue +++ b/app/assets/javascripts/registry/explorer/components/list_page/image_list.vue @@ -38,7 +38,6 @@ export default { :key="index" :item="listItem" :first="index === 0" - :last="index === images.length - 1" @delete="$emit('delete', $event)" /> 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 102311c6062..32bf27f1143 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 @@ -2,7 +2,7 @@ import { GlTooltipDirective, GlIcon, GlSprintf } from '@gitlab/ui'; import { n__ } from '~/locale'; import ClipboardButton from '~/vue_shared/components/clipboard_button.vue'; -import ListItem from '../list_item.vue'; +import ListItem from '~/vue_shared/components/registry/list_item.vue'; import DeleteButton from '../delete_button.vue'; import { diff --git a/app/assets/javascripts/registry/explorer/components/list_item.vue b/app/assets/javascripts/vue_shared/components/registry/list_item.vue index c57645cc3a1..f978de124d1 100644 --- a/app/assets/javascripts/registry/explorer/components/list_item.vue +++ b/app/assets/javascripts/vue_shared/components/registry/list_item.vue @@ -10,11 +10,6 @@ export default { default: false, required: false, }, - last: { - type: Boolean, - default: false, - required: false, - }, disabled: { type: Boolean, default: false, @@ -35,12 +30,10 @@ export default { computed: { optionalClasses() { return { - 'gl-border-t-1': !this.first, - 'gl-border-t-2': this.first, - 'gl-border-b-1': !this.last, - 'gl-border-b-2': this.last, + 'gl-border-t-transparent': !this.first && !this.selected, + 'gl-border-t-gray-100': this.first && !this.selected, 'disabled-content': this.disabled, - 'gl-border-gray-100': !this.selected, + 'gl-border-b-gray-100': !this.selected, 'gl-bg-blue-50 gl-border-blue-200': this.selected, }; }, @@ -58,21 +51,26 @@ export default { <template> <div - class="gl-display-flex gl-flex-direction-column gl-border-b-solid gl-border-t-solid" + class="gl-display-flex gl-flex-direction-column gl-border-b-solid gl-border-t-solid gl-border-t-1 gl-border-b-1" :class="optionalClasses" > - <div class="gl-display-flex gl-align-items-center gl-py-4 gl-px-2"> + <div class="gl-display-flex gl-align-items-center gl-py-5"> <div v-if="$slots['left-action']" class="gl-w-7 gl-display-none gl-display-sm-flex gl-justify-content-start gl-pl-2" > <slot name="left-action"></slot> </div> - <div class="gl-display-flex gl-flex-direction-column gl-flex-fill-1"> + <div + class="gl-display-flex gl-xs-flex-direction-column gl-justify-content-space-between gl-align-items-stretch gl-flex-fill-1" + > <div - class="gl-display-flex gl-align-items-center gl-justify-content-space-between gl-text-body gl-font-weight-bold" + class="gl-display-flex gl-flex-direction-column gl-justify-content-space-between gl-xs-mb-3" > - <div class="gl-display-flex gl-align-items-center"> + <div + v-if="$slots['left-primary']" + class="gl-display-flex gl-align-items-center gl-text-body gl-font-weight-bold gl-min-h-6" + > <slot name="left-primary"></slot> <gl-button v-if="detailsSlots.length > 0" @@ -83,24 +81,27 @@ export default { @click="toggleDetails" /> </div> - <div> - <slot name="right-primary"></slot> + <div v-if="$slots['left-secondary']" class="gl-text-gray-500 gl-mt-1 gl-min-h-6"> + <slot name="left-secondary"></slot> </div> </div> <div - class="gl-display-flex gl-align-items-center gl-justify-content-space-between gl-font-sm gl-text-gray-300" + class="gl-display-flex gl-flex-direction-column gl-sm-align-items-flex-end gl-justify-content-space-between gl-text-gray-500" > - <div> - <slot name="left-secondary"></slot> + <div + v-if="$slots['right-primary']" + class="gl-sm-text-body gl-sm-font-weight-bold gl-min-h-6" + > + <slot name="right-primary"></slot> </div> - <div> + <div v-if="$slots['right-secondary']" class="gl-mt-1 gl-min-h-6"> <slot name="right-secondary"></slot> </div> </div> </div> <div v-if="$slots['right-action']" - class="gl-w-9 gl-display-none gl-display-sm-flex gl-justify-content-end gl-pr-2" + class="gl-w-9 gl-display-none gl-display-sm-flex gl-justify-content-end gl-pr-1" > <slot name="right-action"></slot> </div> diff --git a/app/assets/stylesheets/utilities.scss b/app/assets/stylesheets/utilities.scss index 6758429b78f..20482f30994 100644 --- a/app/assets/stylesheets/utilities.scss +++ b/app/assets/stylesheets/utilities.scss @@ -120,6 +120,43 @@ } } + .gl-shadow-x0-y0-b3-s1-blue-500 { box-shadow: inset 0 0 3px $gl-border-size-1 $blue-500; } + +// remove when https://gitlab.com/gitlab-org/gitlab-ui/-/merge_requests/1692 is merged +.gl-border-t-transparent { + border-top-color: transparent; +} + +.gl-align-items-flex-end { + align-items: flex-end; +} + +.gl-sm-align-items-flex-end { + @media (min-width: $breakpoint-sm) { + align-items: flex-end; + } +} + +.gl-sm-text-body { + @media (min-width: $breakpoint-sm) { + color: $body-color; + } +} + +.gl-sm-font-weight-bold { + @media (min-width: $breakpoint-sm) { + font-weight: $gl-font-weight-bold; + } +} + +.gl-align-items-stretch { + align-items: stretch; +} + +.gl-min-h-6 { + min-height: $gl-spacing-scale-6; +} + diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index 58967ef6e2f..bf8953ed39d 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -26,6 +26,7 @@ class Projects::IssuesController < Projects::ApplicationController before_action :whitelist_query_limiting, only: [:create, :create_merge_request, :move, :bulk_update] before_action :check_issues_available! before_action :issue, unless: ->(c) { c.issue_except_actions.include?(c.action_name.to_sym) } + after_action :log_issue_show, unless: ->(c) { c.issue_except_actions.include?(c.action_name.to_sym) } before_action :set_issuables_index, if: ->(c) { c.set_issuables_index_only_actions.include?(c.action_name.to_sym) } @@ -249,6 +250,13 @@ class Projects::IssuesController < Projects::ApplicationController @issue end # rubocop: enable CodeReuse/ActiveRecord + + def log_issue_show + return unless current_user && @issue + + ::Gitlab::Search::RecentIssues.new(user: current_user).log_view(@issue) + end + alias_method :subscribable_resource, :issue alias_method :issuable, :issue alias_method :awardable, :issue diff --git a/app/helpers/search_helper.rb b/app/helpers/search_helper.rb index d3a34dc38c0..61e0fe19c77 100644 --- a/app/helpers/search_helper.rb +++ b/app/helpers/search_helper.rb @@ -7,6 +7,7 @@ module SearchHelper return unless current_user resources_results = [ + recent_issues_autocomplete(term), groups_autocomplete(term), projects_autocomplete(term) ].flatten @@ -178,6 +179,20 @@ module SearchHelper } end end + + def recent_issues_autocomplete(term, limit = 5) + return [] unless current_user + + ::Gitlab::Search::RecentIssues.new(user: current_user).search(term).limit(limit).map do |i| + { + category: "Recent issues", + id: i.id, + label: search_result_sanitize(i.title), + url: issue_path(i), + avatar_url: i.project.avatar_url || '' + } + end + end # rubocop: enable CodeReuse/ActiveRecord def search_result_sanitize(str) diff --git a/app/models/blob_viewer/dependency_manager.rb b/app/models/blob_viewer/dependency_manager.rb index a851f22bfcd..1be7120a955 100644 --- a/app/models/blob_viewer/dependency_manager.rb +++ b/app/models/blob_viewer/dependency_manager.rb @@ -33,8 +33,8 @@ module BlobViewer @json_data ||= begin prepare! Gitlab::Json.parse(blob.data) - rescue - {} + rescue + {} end end diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 9b304571999..a2b645b47ec 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -880,7 +880,9 @@ module Ci end def test_report_summary - Gitlab::Ci::Reports::TestReportSummary.new(latest_builds_report_results) + strong_memoize(:test_report_summary) do + Gitlab::Ci::Reports::TestReportSummary.new(latest_builds_report_results) + end end def test_reports diff --git a/app/models/concerns/bulk_member_access_load.rb b/app/models/concerns/bulk_member_access_load.rb index 041ed3755e0..f44ad474cd5 100644 --- a/app/models/concerns/bulk_member_access_load.rb +++ b/app/models/concerns/bulk_member_access_load.rb @@ -22,7 +22,7 @@ module BulkMemberAccessLoad end # Look up only the IDs we need - resource_ids = resource_ids - access.keys + resource_ids -= access.keys return access if resource_ids.empty? diff --git a/app/models/concerns/from_except.rb b/app/models/concerns/from_except.rb new file mode 100644 index 00000000000..b9ca9dda4b0 --- /dev/null +++ b/app/models/concerns/from_except.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +module FromExcept + extend ActiveSupport::Concern + + class_methods do + # Produces a query that uses a FROM to select data using an EXCEPT. + # + # Example: + # groups = Group.from_except([group1.self_and_hierarchy, group2.self_and_hierarchy]) + # + # This would produce the following SQL query: + # + # SELECT * + # FROM ( + # SELECT "namespaces". * + # ... + # + # EXCEPT + # + # SELECT "namespaces". * + # ... + # ) groups; + # + # members - An Array of ActiveRecord::Relation objects to use in the except. + # + # remove_duplicates - A boolean indicating if duplicate entries should be + # removed. Defaults to true. + # + # alias_as - The alias to use for the sub query. Defaults to the name of the + # table of the current model. + # rubocop: disable Gitlab/Except + extend FromSetOperator + define_set_operator Gitlab::SQL::Except + # rubocop: enable Gitlab/Except + end +end diff --git a/app/models/concerns/from_intersect.rb b/app/models/concerns/from_intersect.rb new file mode 100644 index 00000000000..428e63eb45e --- /dev/null +++ b/app/models/concerns/from_intersect.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +module FromIntersect + extend ActiveSupport::Concern + + class_methods do + # Produces a query that uses a FROM to select data using an INTERSECT. + # + # Example: + # groups = Group.from_intersect([group1.self_and_hierarchy, group2.self_and_hierarchy]) + # + # This would produce the following SQL query: + # + # SELECT * + # FROM ( + # SELECT "namespaces". * + # ... + # + # INTERSECT + # + # SELECT "namespaces". * + # ... + # ) groups; + # + # members - An Array of ActiveRecord::Relation objects to use in the intersect. + # + # remove_duplicates - A boolean indicating if duplicate entries should be + # removed. Defaults to true. + # + # alias_as - The alias to use for the sub query. Defaults to the name of the + # table of the current model. + # rubocop: disable Gitlab/Intersect + extend FromSetOperator + define_set_operator Gitlab::SQL::Intersect + # rubocop: enable Gitlab/Intersect + end +end diff --git a/app/models/concerns/from_set_operator.rb b/app/models/concerns/from_set_operator.rb new file mode 100644 index 00000000000..593fd251c5c --- /dev/null +++ b/app/models/concerns/from_set_operator.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module FromSetOperator + # Define a high level method to more easily work with the SQL set operations + # of UNION, INTERSECT, and EXCEPT as defined by Gitlab::SQL::Union, + # Gitlab::SQL::Intersect, and Gitlab::SQL::Except respectively. + def define_set_operator(operator) + method_name = 'from_' + operator.name.demodulize.downcase + method_name = method_name.to_sym + + raise "Trying to redefine method '#{method(method_name)}'" if methods.include?(method_name) + + define_method(method_name) do |members, remove_duplicates: true, alias_as: table_name| + operator_sql = operator.new(members, remove_duplicates: remove_duplicates).to_sql + + from(Arel.sql("(#{operator_sql}) #{alias_as}")) + end + end +end diff --git a/app/models/concerns/from_union.rb b/app/models/concerns/from_union.rb index e28dee34815..e25d603b802 100644 --- a/app/models/concerns/from_union.rb +++ b/app/models/concerns/from_union.rb @@ -35,13 +35,29 @@ module FromUnion # alias_as - The alias to use for the sub query. Defaults to the name of the # table of the current model. # rubocop: disable Gitlab/Union + extend FromSetOperator + define_set_operator Gitlab::SQL::Union + + alias_method :from_union_set_operator, :from_union def from_union(members, remove_duplicates: true, alias_as: table_name) + if Feature.enabled?(:sql_set_operators) + from_union_set_operator(members, remove_duplicates: remove_duplicates, alias_as: alias_as) + else + # The original from_union method. + standard_from_union(members, remove_duplicates: remove_duplicates, alias_as: alias_as) + end + end + + private + + def standard_from_union(members, remove_duplicates: true, alias_as: table_name) union = Gitlab::SQL::Union .new(members, remove_duplicates: remove_duplicates) .to_sql from(Arel.sql("(#{union}) #{alias_as}")) end + # rubocop: enable Gitlab/Union end end diff --git a/app/models/concerns/id_in_ordered.rb b/app/models/concerns/id_in_ordered.rb new file mode 100644 index 00000000000..b89409e6841 --- /dev/null +++ b/app/models/concerns/id_in_ordered.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +module IdInOrdered + extend ActiveSupport::Concern + + included do + scope :id_in_ordered, -> (ids) do + raise ArgumentError, "ids must be an array of integers" unless ids.is_a?(Enumerable) && ids.all? { |id| id.is_a?(Integer) } + + # No need to sort if no more than 1 and the sorting code doesn't work + # with an empty array + return id_in(ids) unless ids.count > 1 + + id_attribute = arel_table[:id] + id_in(ids) + .order( + Arel.sql("array_position(ARRAY[#{ids.join(',')}], #{id_attribute.relation.name}.#{id_attribute.name})")) + end + end +end diff --git a/app/models/issue.rb b/app/models/issue.rb index fc668ad75fd..f4e1e5a6612 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -18,6 +18,7 @@ class Issue < ApplicationRecord include MilestoneEventable include WhereComposite include StateEventable + include IdInOrdered DueDateStruct = Struct.new(:title, :name).freeze NoDueDate = DueDateStruct.new('No Due Date', '0').freeze diff --git a/app/models/project.rb b/app/models/project.rb index a3de3c06de6..89dda183788 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -2152,8 +2152,8 @@ class Project < ApplicationRecord data = repository.route_map_for(sha) Gitlab::RouteMap.new(data) if data - rescue Gitlab::RouteMap::FormatError - nil + rescue Gitlab::RouteMap::FormatError + nil end end diff --git a/app/serializers/base_serializer.rb b/app/serializers/base_serializer.rb index 4744a7c1cc8..90bd4730f1e 100644 --- a/app/serializers/base_serializer.rb +++ b/app/serializers/base_serializer.rb @@ -9,7 +9,7 @@ class BaseSerializer end def represent(resource, opts = {}, entity_class = nil) - entity_class = entity_class || self.class.entity_class + entity_class ||= self.class.entity_class entity_class .represent(resource, opts.merge(request: @request)) diff --git a/app/serializers/pipeline_entity.rb b/app/serializers/pipeline_entity.rb index de1e07139ad..64958b06f1d 100644 --- a/app/serializers/pipeline_entity.rb +++ b/app/serializers/pipeline_entity.rb @@ -85,10 +85,6 @@ class PipelineEntity < Grape::Entity pipeline.failed_builds end - expose :tests_total_count do |pipeline| - pipeline.test_report_summary.total[:count] - end - private alias_method :pipeline, :object diff --git a/app/services/event_create_service.rb b/app/services/event_create_service.rb index c7be0a1f686..85658598afc 100644 --- a/app/services/event_create_service.rb +++ b/app/services/event_create_service.rb @@ -23,11 +23,15 @@ class EventCreateService end def open_mr(merge_request, current_user) - create_record_event(merge_request, current_user, :created) + create_record_event(merge_request, current_user, :created).tap do + track_event(event_action: :created, event_target: MergeRequest, author_id: current_user.id) + end end def close_mr(merge_request, current_user) - create_record_event(merge_request, current_user, :closed) + create_record_event(merge_request, current_user, :closed).tap do + track_event(event_action: :closed, event_target: MergeRequest, author_id: current_user.id) + end end def reopen_mr(merge_request, current_user) @@ -35,7 +39,9 @@ class EventCreateService end def merge_mr(merge_request, current_user) - create_record_event(merge_request, current_user, :merged) + create_record_event(merge_request, current_user, :merged).tap do + track_event(event_action: :merged, event_target: MergeRequest, author_id: current_user.id) + end end def open_milestone(milestone, current_user) @@ -55,7 +61,11 @@ class EventCreateService end def leave_note(note, current_user) - create_record_event(note, current_user, :commented) + create_record_event(note, current_user, :commented).tap do + if note.is_a?(DiffNote) && note.for_merge_request? + track_event(event_action: :commented, event_target: MergeRequest, author_id: current_user.id) + end + end end def join_project(project, current_user) @@ -109,7 +119,7 @@ class EventCreateService def wiki_event(wiki_page_meta, author, action, fingerprint) raise IllegalActionError, action unless Event::WIKI_ACTIONS.include?(action) - Gitlab::UsageDataCounters::TrackUniqueEvents.track_event(event_action: action, event_target: wiki_page_meta.class, author_id: author.id) + track_event(event_action: action, event_target: wiki_page_meta.class, author_id: author.id) duplicate = Event.for_wiki_meta(wiki_page_meta).for_fingerprint(fingerprint).first return duplicate if duplicate.present? @@ -154,7 +164,7 @@ class EventCreateService result = Event.insert_all(attribute_sets, returning: %w[id]) tuples.each do |record, status, _| - Gitlab::UsageDataCounters::TrackUniqueEvents.track_event(event_action: status, event_target: record.class, author_id: current_user.id) + track_event(event_action: status, event_target: record.class, author_id: current_user.id) end result @@ -172,7 +182,7 @@ class EventCreateService new_event end - Gitlab::UsageDataCounters::TrackUniqueEvents.track_event(event_action: :pushed, event_target: Project, author_id: current_user.id) + track_event(event_action: :pushed, event_target: Project, author_id: current_user.id) Users::LastPushEventService.new(current_user) .cache_last_push_event(event) @@ -206,6 +216,10 @@ class EventCreateService { resource_parent_attr => resource_parent.id } end + + def track_event(**params) + Gitlab::UsageDataCounters::TrackUniqueEvents.track_event(**params) + end end EventCreateService.prepend_if_ee('EE::EventCreateService') diff --git a/app/services/prometheus/proxy_service.rb b/app/services/prometheus/proxy_service.rb index 33635796771..c1bafd03b48 100644 --- a/app/services/prometheus/proxy_service.rb +++ b/app/services/prometheus/proxy_service.rb @@ -44,8 +44,8 @@ module Prometheus def self.from_cache(proxyable_class_name, proxyable_id, method, path, params) proxyable_class = begin proxyable_class_name.constantize - rescue NameError - nil + rescue NameError + nil end return unless proxyable_class diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml index a7817ad5552..8d8fdcf70ea 100644 --- a/app/views/projects/issues/show.html.haml +++ b/app/views/projects/issues/show.html.haml @@ -64,7 +64,8 @@ -# haml-lint:disable InlineJavaScript %script#js-issuable-app-initial-data{ type: "application/json" }= issuable_initial_data(@issue).to_json #js-issuable-app - %h2.title= markdown_field(@issue, :title) + .title-container + %h2.title= markdown_field(@issue, :title) - if @issue.description.present? .description{ class: can?(current_user, :update_issue, @issue) ? 'js-task-list-container' : '' } .md= markdown_field(@issue, :description) diff --git a/app/workers/delete_stored_files_worker.rb b/app/workers/delete_stored_files_worker.rb index 9cf5631b7d8..689ac3dd0ce 100644 --- a/app/workers/delete_stored_files_worker.rb +++ b/app/workers/delete_stored_files_worker.rb @@ -9,8 +9,8 @@ class DeleteStoredFilesWorker # rubocop:disable Scalability/IdempotentWorker def perform(class_name, keys) klass = begin class_name.constantize - rescue NameError - nil + rescue NameError + nil end unless klass diff --git a/app/workers/partition_creation_worker.rb b/app/workers/partition_creation_worker.rb index b833e818b32..119ecd28003 100644 --- a/app/workers/partition_creation_worker.rb +++ b/app/workers/partition_creation_worker.rb @@ -9,5 +9,7 @@ class PartitionCreationWorker def perform Gitlab::Database::Partitioning::PartitionCreator.new.create_partitions + ensure + Gitlab::Database::Partitioning::PartitionMonitoring.new.report_metrics end end diff --git a/changelogs/unreleased/205578-add-creator-id-to-packages.yml b/changelogs/unreleased/205578-add-creator-id-to-packages.yml new file mode 100644 index 00000000000..81f2b42dae7 --- /dev/null +++ b/changelogs/unreleased/205578-add-creator-id-to-packages.yml @@ -0,0 +1,5 @@ +--- +title: Adds creator_id field to packages_packages table +merge_request: 40562 +author: +type: other diff --git a/changelogs/unreleased/225555-update-usage-ping-data-to-track-usage-of-diffnotes-2.yml b/changelogs/unreleased/225555-update-usage-ping-data-to-track-usage-of-diffnotes-2.yml new file mode 100644 index 00000000000..57abc24d2a0 --- /dev/null +++ b/changelogs/unreleased/225555-update-usage-ping-data-to-track-usage-of-diffnotes-2.yml @@ -0,0 +1,5 @@ +--- +title: Add merge request usage to usage data +merge_request: 40391 +author: +type: other diff --git a/changelogs/unreleased/235060-the-service-desk-info-messages-need-to-be-updated.yml b/changelogs/unreleased/235060-the-service-desk-info-messages-need-to-be-updated.yml new file mode 100644 index 00000000000..6423571ab28 --- /dev/null +++ b/changelogs/unreleased/235060-the-service-desk-info-messages-need-to-be-updated.yml @@ -0,0 +1,5 @@ +--- +title: Display informative messages when service desk is unsupported +merge_request: 40454 +author: +type: other diff --git a/changelogs/unreleased/closing-paren-cop.yml b/changelogs/unreleased/closing-paren-cop.yml new file mode 100644 index 00000000000..764b17a81b8 --- /dev/null +++ b/changelogs/unreleased/closing-paren-cop.yml @@ -0,0 +1,5 @@ +--- +title: Fix Layout/ClosingParenthesisIndentation cop +merge_request: 41084 +author: Rajendra Kadam +type: fixed diff --git a/changelogs/unreleased/first-param-cop.yml b/changelogs/unreleased/first-param-cop.yml new file mode 100644 index 00000000000..b47eb43656c --- /dev/null +++ b/changelogs/unreleased/first-param-cop.yml @@ -0,0 +1,5 @@ +--- +title: Fix Layout/FirstParameterIndentation cop +merge_request: 41089 +author: +type: fixed diff --git a/changelogs/unreleased/raise-exceptio-cop.yml b/changelogs/unreleased/raise-exceptio-cop.yml new file mode 100644 index 00000000000..abac61bf942 --- /dev/null +++ b/changelogs/unreleased/raise-exceptio-cop.yml @@ -0,0 +1,5 @@ +--- +title: Fix Lint/RaiseException cop +merge_request: 41099 +author: Rajendra Kadam +type: fixed diff --git a/changelogs/unreleased/rescue-alignment.yml b/changelogs/unreleased/rescue-alignment.yml new file mode 100644 index 00000000000..5d77a147259 --- /dev/null +++ b/changelogs/unreleased/rescue-alignment.yml @@ -0,0 +1,5 @@ +--- +title: Fix Layout/RescueEnsureAlignment cop +merge_request: 41093 +author: Rajendra Kadam +type: fixed diff --git a/changelogs/unreleased/self-assign-cop.yml b/changelogs/unreleased/self-assign-cop.yml new file mode 100644 index 00000000000..f02ebf5332b --- /dev/null +++ b/changelogs/unreleased/self-assign-cop.yml @@ -0,0 +1,5 @@ +--- +title: Fix Style/SelfAssignment cop +merge_request: 41079 +author: Rajendra Kadam +type: fixed diff --git a/changelogs/unreleased/space-before-first-arg.yml b/changelogs/unreleased/space-before-first-arg.yml new file mode 100644 index 00000000000..46b71f88e6e --- /dev/null +++ b/changelogs/unreleased/space-before-first-arg.yml @@ -0,0 +1,5 @@ +--- +title: Fix Layout/SpaceBeforeFirstArg cop +merge_request: 41097 +author: Rajendra Kadam +type: fixed diff --git a/config/feature_flags/development/recent_items_search.yml b/config/feature_flags/development/recent_items_search.yml new file mode 100644 index 00000000000..d0a70ca3d9d --- /dev/null +++ b/config/feature_flags/development/recent_items_search.yml @@ -0,0 +1,7 @@ +--- +name: recent_items_search +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/40669 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/244277 +group: group::global search +type: development +default_enabled: false
\ No newline at end of file diff --git a/config/feature_flags/development/sql-set-operators.yml b/config/feature_flags/development/sql-set-operators.yml new file mode 100644 index 00000000000..cefe2a83782 --- /dev/null +++ b/config/feature_flags/development/sql-set-operators.yml @@ -0,0 +1,7 @@ +--- +name: sql-set-operators +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/39786 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/39786 +group: group::access +type: development +default_enabled: false diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index da275b34f5d..d3868cfb8cd 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -176,8 +176,8 @@ Settings.gitlab['user'] ||= 'git' Settings.gitlab['ssh_user'] ||= Settings.gitlab.user Settings.gitlab['user_home'] ||= begin Etc.getpwnam(Settings.gitlab['user']).dir -rescue ArgumentError # no user configured - '/home/' + Settings.gitlab['user'] + rescue ArgumentError # no user configured + '/home/' + Settings.gitlab['user'] end Settings.gitlab['time_zone'] ||= nil Settings.gitlab['signup_enabled'] ||= true if Settings.gitlab['signup_enabled'].nil? diff --git a/config/initializers/trusted_proxies.rb b/config/initializers/trusted_proxies.rb index 13896408806..93c4d2b10cc 100644 --- a/config/initializers/trusted_proxies.rb +++ b/config/initializers/trusted_proxies.rb @@ -15,7 +15,7 @@ end gitlab_trusted_proxies = Array(Gitlab.config.gitlab.trusted_proxies).map do |proxy| IPAddr.new(proxy) -rescue IPAddr::InvalidAddressError + rescue IPAddr::InvalidAddressError end.compact Rails.application.config.action_dispatch.trusted_proxies = ( diff --git a/config/routes/group.rb b/config/routes/group.rb index e07ed0fab05..e5bbfdf7548 100644 --- a/config/routes/group.rb +++ b/config/routes/group.rb @@ -23,9 +23,7 @@ constraints(::Constraints::GroupUrlConstrainer.new) do get 'archived', action: :show, as: :group_archived # rubocop:disable Cop/PutGroupRoutesUnderScope end - # These routes are legit and the cop rule will be improved in - # https://gitlab.com/gitlab-org/gitlab/-/issues/230703 - get '/', action: :show, as: :group_canonical # rubocop:disable Cop/PutGroupRoutesUnderScope + get '/', action: :show, as: :group_canonical end scope(path: 'groups/*group_id/-', @@ -112,11 +110,9 @@ constraints(::Constraints::GroupUrlConstrainer.new) do as: :group, constraints: { id: Gitlab::PathRegex.full_namespace_route_regex, format: /(html|json|atom)/ }, controller: :groups) do - # These routes are legit and the cop rule will be improved in - # https://gitlab.com/gitlab-org/gitlab/-/issues/230703 - get '/', action: :show # rubocop:disable Cop/PutGroupRoutesUnderScope - patch '/', action: :update # rubocop:disable Cop/PutGroupRoutesUnderScope - put '/', action: :update # rubocop:disable Cop/PutGroupRoutesUnderScope - delete '/', action: :destroy # rubocop:disable Cop/PutGroupRoutesUnderScope + get '/', action: :show + patch '/', action: :update + put '/', action: :update + delete '/', action: :destroy end end diff --git a/db/migrate/20190402150158_backport_enterprise_schema.rb b/db/migrate/20190402150158_backport_enterprise_schema.rb index 912da09af9d..dcf84d762a3 100644 --- a/db/migrate/20190402150158_backport_enterprise_schema.rb +++ b/db/migrate/20190402150158_backport_enterprise_schema.rb @@ -914,7 +914,7 @@ class BackportEnterpriseSchema < ActiveRecord::Migration[5.0] MSG end - raise Exception.new(message) + raise StandardError.new(message) end def create_missing_tables diff --git a/db/migrate/20200827005322_add_creator_id_to_packages.rb b/db/migrate/20200827005322_add_creator_id_to_packages.rb new file mode 100644 index 00000000000..ecd73ff8785 --- /dev/null +++ b/db/migrate/20200827005322_add_creator_id_to_packages.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddCreatorIdToPackages < ActiveRecord::Migration[6.0] + DOWNTIME = false + + def change + add_column(:packages_packages, :creator_id, :integer) + end +end diff --git a/db/migrate/20200830201204_add_index_to_package_creator.rb b/db/migrate/20200830201204_add_index_to_package_creator.rb new file mode 100644 index 00000000000..fbc8bbade94 --- /dev/null +++ b/db/migrate/20200830201204_add_index_to_package_creator.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +class AddIndexToPackageCreator < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + INDEX_NAME = 'index_packages_packages_on_creator_id' + + def up + add_concurrent_index :packages_packages, :creator_id, name: INDEX_NAME + add_concurrent_foreign_key(:packages_packages, :users, column: :creator_id, on_delete: :nullify) + end + + def down + remove_foreign_key_if_exists(:packages_packages, :users, column: :creator_id) + remove_concurrent_index_by_name(:packages_packages, INDEX_NAME) + end +end diff --git a/db/schema_migrations/20200827005322 b/db/schema_migrations/20200827005322 new file mode 100644 index 00000000000..23ae58e766c --- /dev/null +++ b/db/schema_migrations/20200827005322 @@ -0,0 +1 @@ +f4f1efcc93476a1d70add93e166f4c702ad7dfc97ad29c3455722fd98824498f
\ No newline at end of file diff --git a/db/schema_migrations/20200830201204 b/db/schema_migrations/20200830201204 new file mode 100644 index 00000000000..370640eb119 --- /dev/null +++ b/db/schema_migrations/20200830201204 @@ -0,0 +1 @@ +1e8dd4542b13009b748d352933a4a59fcabb31e916226fcbf87043396f94e09f
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 19eec2aea26..61a9dadba08 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -14025,7 +14025,8 @@ CREATE TABLE public.packages_packages ( updated_at timestamp with time zone NOT NULL, name character varying NOT NULL, version character varying, - package_type smallint NOT NULL + package_type smallint NOT NULL, + creator_id integer ); CREATE SEQUENCE public.packages_packages_id_seq @@ -20457,6 +20458,8 @@ CREATE INDEX index_packages_package_files_on_file_store ON public.packages_packa CREATE INDEX index_packages_package_files_on_package_id_and_file_name ON public.packages_package_files USING btree (package_id, file_name); +CREATE INDEX index_packages_packages_on_creator_id ON public.packages_packages USING btree (creator_id); + CREATE INDEX index_packages_packages_on_name_trigram ON public.packages_packages USING gin (name public.gin_trgm_ops); CREATE INDEX index_packages_packages_on_project_id_and_created_at ON public.packages_packages USING btree (project_id, created_at); @@ -22039,6 +22042,9 @@ ALTER TABLE ONLY public.ci_builds ALTER TABLE ONLY public.design_management_versions ADD CONSTRAINT fk_c1440b4896 FOREIGN KEY (author_id) REFERENCES public.users(id) ON DELETE SET NULL; +ALTER TABLE ONLY public.packages_packages + ADD CONSTRAINT fk_c188f0dba4 FOREIGN KEY (creator_id) REFERENCES public.users(id) ON DELETE SET NULL; + ALTER TABLE ONLY public.geo_event_log ADD CONSTRAINT fk_c1f241c70d FOREIGN KEY (upload_deleted_event_id) REFERENCES public.geo_upload_deleted_events(id) ON DELETE CASCADE; diff --git a/doc/administration/monitoring/prometheus/gitlab_metrics.md b/doc/administration/monitoring/prometheus/gitlab_metrics.md index 5339017ecbf..d9379061544 100644 --- a/doc/administration/monitoring/prometheus/gitlab_metrics.md +++ b/doc/administration/monitoring/prometheus/gitlab_metrics.md @@ -194,6 +194,15 @@ The following metrics are available: |:--------------------------------- |:--------- |:------------------------------------------------------------- |:-------------------------------------- | | `db_load_balancing_hosts` | Gauge | [12.3](https://gitlab.com/gitlab-org/gitlab/-/issues/13630) | Current number of load balancing hosts | +## Database partitioning metrics **(PREMIUM ONLY)** + +The following metrics are available: + +| Metric | Type | Since | Description | +|:--------------------------------- |:--------- |:------------------------------------------------------------- |:----------------------------------------------------------------- | +| `db_partitions_present` | Gauge | [13.4](https://gitlab.com/gitlab-org/gitlab/-/issues/227353) | Number of database partitions present | +| `db_partitions_missing` | Gauge | [13.4](https://gitlab.com/gitlab-org/gitlab/-/issues/227353) | Number of database partitions currently expected, but not present | + ## Connection pool metrics These metrics record the status of the database diff --git a/doc/administration/reference_architectures/10k_users.md b/doc/administration/reference_architectures/10k_users.md index 9a8aa176e77..5fdf31339ff 100644 --- a/doc/administration/reference_architectures/10k_users.md +++ b/doc/administration/reference_architectures/10k_users.md @@ -1438,7 +1438,7 @@ On each node: gitlab_workhorse['enable'] = false grafana['enable'] = false - # If you run a seperate monitoring node you can disable these services + # If you run a separate monitoring node you can disable these services alertmanager['enable'] = false prometheus['enable'] = false diff --git a/doc/administration/reference_architectures/25k_users.md b/doc/administration/reference_architectures/25k_users.md index adf4f7ea0e3..7ef8c688eed 100644 --- a/doc/administration/reference_architectures/25k_users.md +++ b/doc/administration/reference_architectures/25k_users.md @@ -1438,7 +1438,7 @@ On each node: gitlab_workhorse['enable'] = false grafana['enable'] = false - # If you run a seperate monitoring node you can disable these services + # If you run a separate monitoring node you can disable these services alertmanager['enable'] = false prometheus['enable'] = false diff --git a/doc/administration/reference_architectures/2k_users.md b/doc/administration/reference_architectures/2k_users.md index bbea7107b11..55c2f05a3af 100644 --- a/doc/administration/reference_architectures/2k_users.md +++ b/doc/administration/reference_architectures/2k_users.md @@ -419,7 +419,7 @@ To configure the Gitaly server: gitlab_workhorse['enable'] = false grafana['enable'] = false - # If you run a seperate monitoring node you can disable these services + # If you run a separate monitoring node you can disable these services alertmanager['enable'] = false prometheus['enable'] = false diff --git a/doc/administration/reference_architectures/3k_users.md b/doc/administration/reference_architectures/3k_users.md index e1e6250632e..e3827ade735 100644 --- a/doc/administration/reference_architectures/3k_users.md +++ b/doc/administration/reference_architectures/3k_users.md @@ -1145,7 +1145,7 @@ On each node: grafana['enable'] = false gitlab_exporter['enable'] = false - # If you run a seperate monitoring node you can disable these services + # If you run a separate monitoring node you can disable these services alertmanager['enable'] = false prometheus['enable'] = false diff --git a/doc/administration/reference_architectures/50k_users.md b/doc/administration/reference_architectures/50k_users.md index 6fe1174dac0..60cd82b9896 100644 --- a/doc/administration/reference_architectures/50k_users.md +++ b/doc/administration/reference_architectures/50k_users.md @@ -1438,7 +1438,7 @@ On each node: gitlab_workhorse['enable'] = false grafana['enable'] = false - # If you run a seperate monitoring node you can disable these services + # If you run a separate monitoring node you can disable these services alertmanager['enable'] = false prometheus['enable'] = false diff --git a/doc/administration/reference_architectures/5k_users.md b/doc/administration/reference_architectures/5k_users.md index 5a52b6d9d0f..67803a8064c 100644 --- a/doc/administration/reference_architectures/5k_users.md +++ b/doc/administration/reference_architectures/5k_users.md @@ -1144,7 +1144,7 @@ On each node: grafana['enable'] = false gitlab_exporter['enable'] = false - # If you run a seperate monitoring node you can disable these services + # If you run a separate monitoring node you can disable these services alertmanager['enable'] = false prometheus['enable'] = false diff --git a/doc/api/graphql/reference/gitlab_schema.graphql b/doc/api/graphql/reference/gitlab_schema.graphql index d5b80575acb..f66a897c1c2 100644 --- a/doc/api/graphql/reference/gitlab_schema.graphql +++ b/doc/api/graphql/reference/gitlab_schema.graphql @@ -17411,6 +17411,11 @@ type Vulnerability { description: String """ + Timestamp of when the vulnerability was first detected + """ + detectedAt: Time! + + """ GraphQL ID of the vulnerability """ id: ID! diff --git a/doc/api/graphql/reference/gitlab_schema.json b/doc/api/graphql/reference/gitlab_schema.json index 683a0a032e3..ecf3cb3e38c 100644 --- a/doc/api/graphql/reference/gitlab_schema.json +++ b/doc/api/graphql/reference/gitlab_schema.json @@ -51107,6 +51107,24 @@ "deprecationReason": null }, { + "name": "detectedAt", + "description": "Timestamp of when the vulnerability was first detected", + "args": [ + + ], + "type": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "SCALAR", + "name": "Time", + "ofType": null + } + }, + "isDeprecated": false, + "deprecationReason": null + }, + { "name": "id", "description": "GraphQL ID of the vulnerability", "args": [ diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 9b4fd3daf67..7ead6d397a2 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -2580,6 +2580,7 @@ Represents a vulnerability. | Name | Type | Description | | --- | ---- | ---------- | | `description` | String | Description of the vulnerability | +| `detectedAt` | Time! | Timestamp of when the vulnerability was first detected | | `id` | ID! | GraphQL ID of the vulnerability | | `identifiers` | VulnerabilityIdentifier! => Array | Identifiers of the vulnerability. | | `location` | VulnerabilityLocation | Location metadata for the vulnerability. Its fields depend on the type of security scan that found the vulnerability | diff --git a/lib/gitlab/cross_project_access.rb b/lib/gitlab/cross_project_access.rb index 4ddc7e02d1b..93baf1e596c 100644 --- a/lib/gitlab/cross_project_access.rb +++ b/lib/gitlab/cross_project_access.rb @@ -18,7 +18,7 @@ module Gitlab end def add_check( - klass, + klass, actions: {}, positive_condition: nil, negative_condition: nil, diff --git a/lib/gitlab/data_builder/push.rb b/lib/gitlab/data_builder/push.rb index af363705bed..f941c57a6dd 100644 --- a/lib/gitlab/data_builder/push.rb +++ b/lib/gitlab/data_builder/push.rb @@ -86,7 +86,7 @@ module Gitlab # # rubocop:disable Metrics/ParameterLists def build( - project:, user:, ref:, oldrev: nil, newrev: nil, + project:, user:, ref:, oldrev: nil, newrev: nil, commits: [], commits_count: nil, message: nil, push_options: {}, with_changed_files: true) diff --git a/lib/gitlab/database/partitioning/partition_monitoring.rb b/lib/gitlab/database/partitioning/partition_monitoring.rb new file mode 100644 index 00000000000..9ec9ae684a5 --- /dev/null +++ b/lib/gitlab/database/partitioning/partition_monitoring.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +module Gitlab + module Database + module Partitioning + class PartitionMonitoring + attr_reader :models + + def initialize(models = PartitionCreator.models) + @models = models + end + + def report_metrics + models.each do |model| + strategy = model.partitioning_strategy + + gauge_present.set({ table: model.table_name }, strategy.current_partitions.size) + gauge_missing.set({ table: model.table_name }, strategy.missing_partitions.size) + end + end + + private + + def gauge_present + @gauge_present ||= Gitlab::Metrics.gauge(:db_partitions_present, 'Number of database partitions present') + end + + def gauge_missing + @gauge_missing ||= Gitlab::Metrics.gauge(:db_partitions_missing, 'Number of database partitions currently expected, but not present') + end + end + end + end +end diff --git a/lib/gitlab/highlight.rb b/lib/gitlab/highlight.rb index 63cf80a10ed..40dee0142b9 100644 --- a/lib/gitlab/highlight.rb +++ b/lib/gitlab/highlight.rb @@ -31,8 +31,8 @@ module Gitlab def lexer @lexer ||= custom_language || begin Rouge::Lexer.guess(filename: @blob_name, source: @blob_content).new - rescue Rouge::Guesser::Ambiguous => e - e.alternatives.min_by(&:tag) + rescue Rouge::Guesser::Ambiguous => e + e.alternatives.min_by(&:tag) end end diff --git a/lib/gitlab/search/recent_issues.rb b/lib/gitlab/search/recent_issues.rb new file mode 100644 index 00000000000..755ca35bd66 --- /dev/null +++ b/lib/gitlab/search/recent_issues.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +module Gitlab + module Search + class RecentIssues + ITEMS_LIMIT = 100 + EXPIRES_AFTER = 7.days + + def initialize(user:, items_limit: ITEMS_LIMIT, expires_after: EXPIRES_AFTER) + @user = user + @items_limit = items_limit + @expires_after = expires_after + end + + def log_view(issue) + return unless recent_items_enabled? + + with_redis do |redis| + redis.zadd(key, Time.now.to_f, issue.id) + redis.expire(key, @expires_after) + + # There is a race condition here where we could end up removing an + # item from 2 places concurrently but this is fine since worst case + # scenario we remove an extra item from the end of the list. + if redis.zcard(key) > @items_limit + redis.zremrangebyrank(key, 0, 0) # Remove least recent + end + end + end + + def search(term) + return Issue.none unless recent_items_enabled? + + ids = with_redis do |redis| + redis.zrevrange(key, 0, @items_limit - 1) + end.map(&:to_i) + + IssuesFinder.new(@user, search: term, in: 'title').execute.reorder(nil).id_in_ordered(ids) # rubocop: disable CodeReuse/ActiveRecord + end + + private + + def with_redis(&blk) + Gitlab::Redis::SharedState.with(&blk) # rubocop: disable CodeReuse/ActiveRecord + end + + def key + "recent_items:#{type.name.downcase}:#{@user.id}" + end + + def type + Issue + end + + def recent_items_enabled? + Feature.enabled?(:recent_items_search, @user) + end + end + end +end diff --git a/lib/gitlab/sql/except.rb b/lib/gitlab/sql/except.rb new file mode 100644 index 00000000000..82cbfa8d4ab --- /dev/null +++ b/lib/gitlab/sql/except.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +module Gitlab + module SQL + # Class for building SQL EXCEPT statements. + # + # ORDER BYs are dropped from the relations as the final sort order is not + # guaranteed any way. + # + # Example usage: + # + # except = Gitlab::SQL::Except.new([user.projects, user.personal_projects]) + # sql = except.to_sql + # + # Project.where("id IN (#{sql})") + class Except < SetOperator + def self.operator_keyword + 'EXCEPT' + end + end + end +end diff --git a/lib/gitlab/sql/intersect.rb b/lib/gitlab/sql/intersect.rb new file mode 100644 index 00000000000..c661db3d4c5 --- /dev/null +++ b/lib/gitlab/sql/intersect.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module Gitlab + module SQL + # Class for building SQL INTERSECT statements. + # + # ORDER BYs are dropped from the relations as the final sort order is not + # guaranteed any way. + # + # Example usage: + # + # hierarchies = [group1.self_and_hierarchy, group2.self_and_hierarchy] + # intersect = Gitlab::SQL::Intersect.new(hierarchies) + # sql = intersect.to_sql + # + # Project.where("id IN (#{sql})") + class Intersect < SetOperator + def self.operator_keyword + 'INTERSECT' + end + end + end +end diff --git a/lib/gitlab/sql/set_operator.rb b/lib/gitlab/sql/set_operator.rb new file mode 100644 index 00000000000..d58a1415493 --- /dev/null +++ b/lib/gitlab/sql/set_operator.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +module Gitlab + module SQL + # Class for building SQL set operator statements (UNION, INTERSECT, and + # EXCEPT). + # + # ORDER BYs are dropped from the relations as the final sort order is not + # guaranteed any way. + # + # Example usage: + # + # union = Gitlab::SQL::Union.new([user.personal_projects, user.projects]) + # sql = union.to_sql + # + # Project.where("id IN (#{sql})") + class SetOperator + def initialize(relations, remove_duplicates: true) + @relations = relations + @remove_duplicates = remove_duplicates + end + + def self.operator_keyword + raise NotImplementedError + end + + def to_sql + # Some relations may include placeholders for prepared statements, these + # aren't incremented properly when joining relations together this way. + # By using "unprepared_statements" we remove the usage of placeholders + # (thus fixing this problem), at a slight performance cost. + fragments = ActiveRecord::Base.connection.unprepared_statement do + relations.map { |rel| rel.reorder(nil).to_sql }.reject(&:blank?) + end + + if fragments.any? + "(" + fragments.join(")\n#{operator_keyword_fragment}\n(") + ")" + else + 'NULL' + end + end + + # UNION [ALL] | INTERSECT [ALL] | EXCEPT [ALL] + def operator_keyword_fragment + remove_duplicates ? self.class.operator_keyword : "#{self.class.operator_keyword} ALL" + end + + private + + attr_reader :relations, :remove_duplicates + end + end +end diff --git a/lib/gitlab/sql/union.rb b/lib/gitlab/sql/union.rb index b15f2ca385a..7fb3487a5e5 100644 --- a/lib/gitlab/sql/union.rb +++ b/lib/gitlab/sql/union.rb @@ -13,30 +13,9 @@ module Gitlab # sql = union.to_sql # # Project.where("id IN (#{sql})") - class Union - def initialize(relations, remove_duplicates: true) - @relations = relations - @remove_duplicates = remove_duplicates - end - - def to_sql - # Some relations may include placeholders for prepared statements, these - # aren't incremented properly when joining relations together this way. - # By using "unprepared_statements" we remove the usage of placeholders - # (thus fixing this problem), at a slight performance cost. - fragments = ActiveRecord::Base.connection.unprepared_statement do - @relations.map { |rel| rel.reorder(nil).to_sql }.reject(&:blank?) - end - - if fragments.any? - "(" + fragments.join(")\n#{union_keyword}\n(") + ")" - else - 'NULL' - end - end - - def union_keyword - @remove_duplicates ? 'UNION' : 'UNION ALL' + class Union < SetOperator + def self.operator_keyword + 'UNION' end end end diff --git a/lib/gitlab/usage_data.rb b/lib/gitlab/usage_data.rb index 504ffe0a2ad..1dd6eab1743 100644 --- a/lib/gitlab/usage_data.rb +++ b/lib/gitlab/usage_data.rb @@ -426,16 +426,17 @@ module Gitlab {} # augmented in EE end - # rubocop: disable CodeReuse/ActiveRecord def merge_requests_users(time_period) - distinct_count( - Event.where(target_type: Event::TARGET_TYPES[:merge_request].to_s).where(time_period), - :author_id, - start: user_minimum_id, - finish: user_maximum_id - ) + counter = Gitlab::UsageDataCounters::TrackUniqueEvents + + redis_usage_data do + counter.count_unique_events( + event_action: Gitlab::UsageDataCounters::TrackUniqueEvents::MERGE_REQUEST_ACTION, + date_from: time_period[:created_at].first, + date_to: time_period[:created_at].last + ) + end end - # rubocop: enable CodeReuse/ActiveRecord def installation_type if Rails.env.production? diff --git a/lib/gitlab/usage_data_counters/track_unique_events.rb b/lib/gitlab/usage_data_counters/track_unique_events.rb index f2a217e980e..71a4574eea9 100644 --- a/lib/gitlab/usage_data_counters/track_unique_events.rb +++ b/lib/gitlab/usage_data_counters/track_unique_events.rb @@ -6,6 +6,7 @@ module Gitlab WIKI_ACTION = :wiki_action DESIGN_ACTION = :design_action PUSH_ACTION = :project_action + MERGE_REQUEST_ACTION = :merge_request_action ACTION_TRANSFORMATIONS = HashWithIndifferentAccess.new({ wiki: { @@ -20,6 +21,12 @@ module Gitlab }, project: { pushed: PUSH_ACTION + }, + merge_request: { + closed: MERGE_REQUEST_ACTION, + merged: MERGE_REQUEST_ACTION, + created: MERGE_REQUEST_ACTION, + commented: MERGE_REQUEST_ACTION } }).freeze diff --git a/lib/tasks/gitlab/lfs/migrate.rake b/lib/tasks/gitlab/lfs/migrate.rake index 470a12c39cd..3d4c847a0f0 100644 --- a/lib/tasks/gitlab/lfs/migrate.rake +++ b/lib/tasks/gitlab/lfs/migrate.rake @@ -9,12 +9,12 @@ namespace :gitlab do LfsObject.with_files_stored_locally .find_each(batch_size: 10) do |lfs_object| - lfs_object.file.migrate!(LfsObjectUploader::Store::REMOTE) + lfs_object.file.migrate!(LfsObjectUploader::Store::REMOTE) - logger.info("Transferred LFS object #{lfs_object.oid} of size #{lfs_object.size.to_i.bytes} to object storage") - rescue => e - logger.error("Failed to transfer LFS object #{lfs_object.oid} with error: #{e.message}") - end + logger.info("Transferred LFS object #{lfs_object.oid} of size #{lfs_object.size.to_i.bytes} to object storage") + rescue => e + logger.error("Failed to transfer LFS object #{lfs_object.oid} with error: #{e.message}") + end end task migrate_to_local: :environment do diff --git a/locale/gitlab.pot b/locale/gitlab.pot index e780e4ffc8a..06a802bcac4 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -10972,6 +10972,9 @@ msgstr "" msgid "Footer message" msgstr "" +msgid "For help setting up the Service Desk for your instance, please contact an administrator." +msgstr "" + msgid "For internal projects, any logged in user can view pipelines and access job details (output logs and artifacts)" msgstr "" @@ -13046,6 +13049,9 @@ msgstr "" msgid "In %{time_to_now}" msgstr "" +msgid "In order to enable Service Desk for your instance, you must first set up incoming email." +msgstr "" + msgid "In order to gather accurate feature usage data, it can take 1 to 2 weeks to see your index." msgstr "" @@ -22392,6 +22398,12 @@ msgstr "" msgid "Service Desk is enabled but not yet active" msgstr "" +msgid "Service Desk is not enabled" +msgstr "" + +msgid "Service Desk is not supported" +msgstr "" + msgid "Service Templates" msgstr "" diff --git a/package.json b/package.json index 5a3ab3a9ef6..a794d41db92 100644 --- a/package.json +++ b/package.json @@ -43,7 +43,7 @@ "@babel/preset-env": "^7.10.1", "@gitlab/at.js": "1.5.5", "@gitlab/svgs": "1.161.0", - "@gitlab/ui": "20.13.0", + "@gitlab/ui": "20.16.0", "@gitlab/visual-review-tools": "1.6.1", "@rails/actioncable": "^6.0.3-1", "@sentry/browser": "^5.10.2", diff --git a/rubocop/cop/gitlab/except.rb b/rubocop/cop/gitlab/except.rb new file mode 100644 index 00000000000..24da6962457 --- /dev/null +++ b/rubocop/cop/gitlab/except.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Gitlab + # Cop that disallows the use of `Gitlab::SQL::Except`, in favour of using + # the `FromExcept` module. + class Except < RuboCop::Cop::Cop + MSG = 'Use the `FromExcept` concern, instead of using `Gitlab::SQL::Except` directly' + + def_node_matcher :raw_except?, <<~PATTERN + (send (const (const (const nil? :Gitlab) :SQL) :Except) :new ...) + PATTERN + + def on_send(node) + return unless raw_except?(node) + + add_offense(node, location: :expression) + end + end + end + end +end diff --git a/rubocop/cop/gitlab/intersect.rb b/rubocop/cop/gitlab/intersect.rb new file mode 100644 index 00000000000..4b61073b804 --- /dev/null +++ b/rubocop/cop/gitlab/intersect.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Gitlab + # Cop that disallows the use of `Gitlab::SQL::Intersect`, in favour of using + # the `FromIntersect` module. + class Intersect < RuboCop::Cop::Cop + MSG = 'Use the `FromIntersect` concern, instead of using `Gitlab::SQL::Intersect` directly' + + def_node_matcher :raw_intersect?, <<~PATTERN + (send (const (const (const nil? :Gitlab) :SQL) :Intersect) :new ...) + PATTERN + + def on_send(node) + return unless raw_intersect?(node) + + add_offense(node, location: :expression) + end + end + end + end +end diff --git a/rubocop/routes_under_scope.rb b/rubocop/routes_under_scope.rb index 3f049bb09fa..7ffb0fd36f2 100644 --- a/rubocop/routes_under_scope.rb +++ b/rubocop/routes_under_scope.rb @@ -12,6 +12,7 @@ module RuboCop def on_send(node) return unless route_method?(node) return unless outside_scope?(node) + return if root_route?(node) add_offense(node) end @@ -25,5 +26,13 @@ module RuboCop def route_method?(node) ROUTE_METHODS.include?(node.method_name) end + + def root_route?(node) + first_argument = node.arguments.first + + if first_argument.respond_to?(:value) + first_argument.value == '/' + end + end end end diff --git a/scripts/lint-doc.sh b/scripts/lint-doc.sh index 72e6334d0fc..e4e0e21ca15 100755 --- a/scripts/lint-doc.sh +++ b/scripts/lint-doc.sh @@ -55,7 +55,18 @@ then ((ERRORCODE++)) fi -MD_DOC_PATH=${MD_DOC_PATH:-doc} +# Run Vale and Markdownlint only on changed files. Only works on merged results +# pipelines, so first checks if a merged results CI variable is present. If not present, +# runs test on all files. +if [ -z "${CI_MERGE_REQUEST_TARGET_BRANCH_SHA}" ] +then + MD_DOC_PATH=${MD_DOC_PATH:-doc} + echo "Merge request pipeline (detached) detected. Testing all files." +else + MERGE_BASE=$(git merge-base ${CI_MERGE_REQUEST_TARGET_BRANCH_SHA} ${CI_MERGE_REQUEST_SOURCE_BRANCH_SHA}) + MD_DOC_PATH=$(git diff --name-only "${MERGE_BASE}..${CI_MERGE_REQUEST_SOURCE_BRANCH_SHA}" '*.md') + echo -e "Merged results pipeline detected. Testing only the following files:\n${MD_DOC_PATH}" + fi function run_locally_or_in_docker() { local cmd=$1 diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index f851418ae09..0be90a09565 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -1011,6 +1011,24 @@ RSpec.describe Projects::IssuesController do end end end + + it 'logs the view with Gitlab::Search::RecentIssues' do + sign_in(user) + recent_issues_double = instance_double(::Gitlab::Search::RecentIssues, log_view: nil) + expect(::Gitlab::Search::RecentIssues).to receive(:new).with(user: user).and_return(recent_issues_double) + + go(id: issue.to_param) + + expect(recent_issues_double).to have_received(:log_view) + end + + context 'when not logged in' do + it 'does not log the view with Gitlab::Search::RecentIssues' do + expect(::Gitlab::Search::RecentIssues).not_to receive(:new) + + go(id: issue.to_param) + end + end end describe 'GET #realtime_changes' do diff --git a/spec/controllers/projects/pipelines_controller_spec.rb b/spec/controllers/projects/pipelines_controller_spec.rb index f295834d1b6..1196a702115 100644 --- a/spec/controllers/projects/pipelines_controller_spec.rb +++ b/spec/controllers/projects/pipelines_controller_spec.rb @@ -43,7 +43,7 @@ RSpec.describe Projects::PipelinesController do end end - it 'executes N+1 queries' do + it 'does not execute N+1 queries' do get_pipelines_index_json control_count = ActiveRecord::QueryRecorder.new do @@ -53,7 +53,7 @@ RSpec.describe Projects::PipelinesController do create_all_pipeline_types # There appears to be one extra query for Pipelines#has_warnings? for some reason - expect { get_pipelines_index_json }.not_to exceed_query_limit(control_count + 7) + expect { get_pipelines_index_json }.not_to exceed_query_limit(control_count + 1) expect(response).to have_gitlab_http_status(:ok) expect(json_response['pipelines'].count).to eq 12 end diff --git a/spec/features/issuables/issuable_list_spec.rb b/spec/features/issuables/issuable_list_spec.rb index 7790d8f1c4c..b1ffaaa7c7e 100644 --- a/spec/features/issuables/issuable_list_spec.rb +++ b/spec/features/issuables/issuable_list_spec.rb @@ -51,8 +51,8 @@ RSpec.describe 'issuable list', :js do it "counts merge requests closing issues icons for each issue" do visit_issuable_list(:issue) - expect(page).to have_selector('.icon-merge-request-unmerged', count: 1) - expect(first('.icon-merge-request-unmerged').find(:xpath, '..')).to have_content(1) + expect(page).to have_selector('[data-testid="merge-requests"]', count: 1) + expect(first('[data-testid="merge-requests"]').find(:xpath, '..')).to have_content(1) end def visit_issuable_list(issuable_type) diff --git a/spec/features/issues/service_desk_spec.rb b/spec/features/issues/service_desk_spec.rb index 2912ac33625..666fbe84fae 100644 --- a/spec/features/issues/service_desk_spec.rb +++ b/spec/features/issues/service_desk_spec.rb @@ -7,6 +7,7 @@ RSpec.describe 'Service Desk Issue Tracker', :js do let(:user) { create(:user) } before do + # The following two conditions equate to Gitlab::ServiceDesk.supported == true allow(Gitlab::IncomingEmail).to receive(:enabled?).and_return(true) allow(Gitlab::IncomingEmail).to receive(:supports_wildcard?).and_return(true) @@ -27,53 +28,7 @@ RSpec.describe 'Service Desk Issue Tracker', :js do end describe 'issues list' do - context 'when service desk is misconfigured' do - before do - allow(Gitlab::ServiceDesk).to receive(:supported?).and_return(false) - visit service_desk_project_issues_path(project) - end - - it 'shows a message to say the configuration is incomplete' do - expect(page).to have_css('.empty-state') - expect(page).to have_text('Service Desk is enabled but not yet active') - expect(page).to have_text('You must set up incoming email before it becomes active') - expect(page).to have_link('More information', href: help_page_path('administration/incoming_email', anchor: 'set-it-up')) - end - end - - context 'when service desk has not been activated' do - let(:project_without_service_desk) { create(:project, :private, service_desk_enabled: false) } - - describe 'service desk info content' do - context 'when user has permissions to edit project settings' do - before do - project_without_service_desk.add_maintainer(user) - visit service_desk_project_issues_path(project_without_service_desk) - end - - it 'displays the large info box, documentation, and a button to configure' do - aggregate_failures do - expect(page).to have_css('.empty-state') - expect(page).to have_link('Read more', href: help_page_path('user/project/service_desk')) - expect(page).to have_link('Turn on Service Desk') - end - end - end - - context 'when user does not have permission to edit project settings' do - before do - project_without_service_desk.add_guest(user) - visit service_desk_project_issues_path(project_without_service_desk) - end - - it 'does not show a button configure service desk' do - expect(page).not_to have_link('Turn on Service Desk') - end - end - end - end - - context 'when service desk has been activated' do + context 'when service desk is supported' do context 'when there are no issues' do describe 'service desk info content' do it 'displays the large info box, documentation, and the address' do @@ -81,6 +36,7 @@ RSpec.describe 'Service Desk Issue Tracker', :js do aggregate_failures do expect(page).to have_css('.empty-state') + expect(page).to have_text('Use Service Desk to connect with your users') expect(page).to have_link('Read more', href: help_page_path('user/project/service_desk')) expect(page).not_to have_link('Turn on Service Desk') expect(page).to have_content(project.service_desk_address) @@ -99,6 +55,7 @@ RSpec.describe 'Service Desk Issue Tracker', :js do it 'displays the large info box and the documentation link' do aggregate_failures do expect(page).to have_css('.empty-state') + expect(page).to have_text('Use Service Desk to connect with your users') expect(page).to have_link('Read more', href: help_page_path('user/project/service_desk')) expect(page).not_to have_link('Turn on Service Desk') expect(page).not_to have_content(project.service_desk_address) @@ -155,5 +112,46 @@ RSpec.describe 'Service Desk Issue Tracker', :js do end end end + + context 'when service desk is not supported' do + let(:project_without_service_desk) { create(:project, :private, service_desk_enabled: false) } + + before do + allow(Gitlab::ServiceDesk).to receive(:supported?).and_return(false) + visit service_desk_project_issues_path(project) + end + + describe 'service desk info content' do + context 'when user has permissions to edit project settings' do + before do + project_without_service_desk.add_maintainer(user) + visit service_desk_project_issues_path(project_without_service_desk) + end + + it 'informs user to setup incoming email to turn on support for Service Desk' do + aggregate_failures do + expect(page).to have_css('.empty-state') + expect(page).to have_text('Service Desk is not supported') + expect(page).to have_text('In order to enable Service Desk for your instance, you must first set up incoming email.') + expect(page).to have_link('More information', href: help_page_path('administration/incoming_email', anchor: 'set-it-up')) + end + end + end + + context 'when user does not have permission to edit project settings' do + before do + project_without_service_desk.add_developer(user) + visit service_desk_project_issues_path(project_without_service_desk) + end + + it 'informs user to contact an administrator to enable service desk' do + expect(page).to have_css('.empty-state') + # NOTE: here, "enabled" is not used in the sense of "ServiceDesk::Enabled?" + expect(page).to have_text('Service Desk is not enabled') + expect(page).to have_text('For help setting up the Service Desk for your instance, please contact an administrator.') + end + end + end + end end end diff --git a/spec/frontend/integrations/edit/components/jira_issues_fields_spec.js b/spec/frontend/integrations/edit/components/jira_issues_fields_spec.js index 70ab8fa7bcf..a727bb9c734 100644 --- a/spec/frontend/integrations/edit/components/jira_issues_fields_spec.js +++ b/spec/frontend/integrations/edit/components/jira_issues_fields_spec.js @@ -92,5 +92,21 @@ describe('JiraIssuesFields', () => { expect(wrapper.find(`a[href="${defaultProps.editProjectPath}"]`).exists()).toBe(true); }); + + describe('GitLab issues warning', () => { + const expectedText = 'Consider disabling GitLab issues'; + + it('contains warning when GitLab issues is enabled', () => { + createComponent(); + + expect(wrapper.text()).toContain(expectedText); + }); + + it('does not contain warning when GitLab issues is disabled', () => { + createComponent({ gitlabIssuesEnabled: false }); + + expect(wrapper.text()).not.toContain(expectedText); + }); + }); }); }); diff --git a/spec/frontend/issuables_list/service_desk_helper_spec.js b/spec/frontend/issuables_list/service_desk_helper_spec.js new file mode 100644 index 00000000000..901b58e01a3 --- /dev/null +++ b/spec/frontend/issuables_list/service_desk_helper_spec.js @@ -0,0 +1,28 @@ +import { emptyStateHelper, generateMessages } from '~/issuables_list/service_desk_helper'; + +describe('service desk helper', () => { + const emptyStateMessages = generateMessages({}); + + // Note: isServiceDeskEnabled must not be true when isServiceDeskSupported is false (it's an invalid case). + describe.each` + isServiceDeskSupported | isServiceDeskEnabled | canEditProjectSettings | expectedMessage + ${true} | ${true} | ${true} | ${'serviceDeskEnabledAndCanEditProjectSettings'} + ${true} | ${true} | ${false} | ${'serviceDeskEnabledAndCannotEditProjectSettings'} + ${true} | ${false} | ${true} | ${'serviceDeskDisabledAndCanEditProjectSettings'} + ${true} | ${false} | ${false} | ${'serviceDeskDisabledAndCannotEditProjectSettings'} + ${false} | ${false} | ${true} | ${'serviceDeskIsNotSupported'} + ${false} | ${false} | ${false} | ${'serviceDeskIsNotEnabled'} + `( + 'isServiceDeskSupported = $isServiceDeskSupported, isServiceDeskEnabled = $isServiceDeskEnabled, canEditProjectSettings = $canEditProjectSettings', + ({ isServiceDeskSupported, isServiceDeskEnabled, canEditProjectSettings, expectedMessage }) => { + it(`displays ${expectedMessage} message`, () => { + const emptyStateMeta = { + isServiceDeskEnabled, + isServiceDeskSupported, + canEditProjectSettings, + }; + expect(emptyStateHelper(emptyStateMeta)).toEqual(emptyStateMessages[expectedMessage]); + }); + }, + ); +}); diff --git a/spec/frontend/issue_show/components/description_spec.js b/spec/frontend/issue_show/components/description_spec.js index f3c078184a9..bc7511225a0 100644 --- a/spec/frontend/issue_show/components/description_spec.js +++ b/spec/frontend/issue_show/components/description_spec.js @@ -36,12 +36,27 @@ describe('Description component', () => { $('.issuable-meta .flash-container').remove(); }); - it('animates description changes', () => { + it('doesnt animate first description changes', () => { vm.descriptionHtml = 'changed'; + return vm.$nextTick().then(() => { + expect( + vm.$el.querySelector('.md').classList.contains('issue-realtime-pre-pulse'), + ).toBeFalsy(); + jest.runAllTimers(); + return vm.$nextTick(); + }); + }); + + it('animates description changes on live update', () => { + vm.descriptionHtml = 'changed'; return vm .$nextTick() .then(() => { + vm.descriptionHtml = 'changed second time'; + return vm.$nextTick(); + }) + .then(() => { expect( vm.$el.querySelector('.md').classList.contains('issue-realtime-pre-pulse'), ).toBeTruthy(); 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 eab8d7b67cc..e999ae69f8d 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 @@ -2,99 +2,139 @@ exports[`packages_list_row renders 1`] = ` <div - class="gl-responsive-table-row" + class="gl-display-flex gl-flex-direction-column gl-border-b-solid gl-border-t-solid gl-border-t-1 gl-border-b-1 gl-border-t-transparent gl-border-b-gray-100" data-qa-selector="packages-row" > <div - class="table-section section-50 d-flex flex-md-column justify-content-between flex-wrap" + class="gl-display-flex gl-align-items-center gl-py-5" > - <div - class="d-flex align-items-center mr-2" - > - <gl-link-stub - class="text-dark font-weight-bold mb-md-1" - data-qa-selector="package_link" - href="foo" - > - - Test package - - </gl-link-stub> - - <!----> - </div> + <!----> <div - class="d-flex text-secondary text-truncate mt-md-2" + class="gl-display-flex gl-xs-flex-direction-column gl-justify-content-space-between gl-align-items-stretch gl-flex-fill-1" > - <span> - 1.0.0 - </span> - - <!----> - <div - class="d-flex align-items-center" + class="gl-display-flex gl-flex-direction-column gl-justify-content-space-between gl-xs-mb-3" > - <gl-icon-stub - class="text-secondary ml-2 mr-1" - name="review-list" - size="16" - /> + <div + class="gl-display-flex gl-align-items-center gl-text-body gl-font-weight-bold gl-min-h-6" + > + <div + class="gl-display-flex gl-align-items-center gl-mr-3" + > + <gl-link-stub + class="gl-text-body" + data-qa-selector="package_link" + href="foo" + > + + Test package + + </gl-link-stub> + + <!----> + </div> + + <!----> + </div> - <gl-link-stub - class="text-secondary" - data-testid="packages-row-project" - href="/foo/bar/baz" + <div + class="gl-text-gray-500 gl-mt-1 gl-min-h-6" > + <div + class="gl-display-flex" + > + <span> + 1.0.0 + </span> + + <!----> + + <div + class="gl-display-flex gl-align-items-center" + > + <gl-icon-stub + class="gl-ml-3 gl-mr-2" + name="review-list" + size="16" + /> + + <gl-link-stub + class="gl-text-body" + data-testid="packages-row-project" + href="/foo/bar/baz" + > + - </gl-link-stub> + + </gl-link-stub> + </div> + + <div + class="d-flex align-items-center" + data-testid="package-type" + > + <gl-icon-stub + class="gl-ml-3 gl-mr-2" + name="package" + size="16" + /> + + <span> + Maven + </span> + </div> + </div> + </div> </div> <div - class="d-flex align-items-center" - data-testid="package-type" + class="gl-display-flex gl-flex-direction-column gl-sm-align-items-flex-end gl-justify-content-space-between gl-text-gray-500" > - <gl-icon-stub - class="text-secondary ml-2 mr-1" - name="package" - size="16" - /> + <div + class="gl-sm-text-body gl-sm-font-weight-bold gl-min-h-6" + > + <publish-method-stub + packageentity="[object Object]" + /> + </div> - <span> - Maven - </span> + <div + class="gl-mt-1 gl-min-h-6" + > + <gl-sprintf-stub + message="Created %{timestamp}" + /> + </div> </div> </div> - </div> - - <div - class="table-section d-flex flex-md-column justify-content-between align-items-md-end section-40" - > - <publish-method-stub - packageentity="[object Object]" - /> <div - class="text-secondary order-0 order-md-1 mt-md-2" + class="gl-w-9 gl-display-none gl-display-sm-flex gl-justify-content-end gl-pr-1" > - <gl-sprintf-stub - message="Created %{timestamp}" + <gl-button-stub + aria-label="Remove package" + category="primary" + data-testid="action-delete" + icon="remove" + size="medium" + title="Remove package" + variant="danger" /> </div> </div> <div - class="table-section section-10 d-flex justify-content-end" + class="gl-display-flex" > - <gl-button-stub - aria-label="Remove package" - category="primary" - data-testid="action-delete" - icon="remove" - size="medium" - title="Remove package" - variant="danger" + <div + class="gl-w-7" + /> + + <!----> + + <div + class="gl-w-9" /> </div> </div> diff --git a/spec/frontend/packages/shared/components/__snapshots__/publish_method_spec.js.snap b/spec/frontend/packages/shared/components/__snapshots__/publish_method_spec.js.snap index 5ecca63d41d..490aa34766c 100644 --- a/spec/frontend/packages/shared/components/__snapshots__/publish_method_spec.js.snap +++ b/spec/frontend/packages/shared/components/__snapshots__/publish_method_spec.js.snap @@ -2,7 +2,7 @@ exports[`publish_method renders 1`] = ` <div - class="d-flex align-items-center text-secondary order-1 order-md-0 mb-md-1" + class="d-flex align-items-center order-1 order-md-0 mb-md-1" > <gl-icon-stub class="mr-1" @@ -10,11 +10,11 @@ exports[`publish_method renders 1`] = ` size="16" /> - <strong - class="mr-1 text-dark" + <span + class="mr-1" > branch-name - </strong> + </span> <gl-icon-stub class="mr-1" @@ -30,7 +30,7 @@ exports[`publish_method renders 1`] = ` </gl-link-stub> <clipboard-button-stub - cssclass="border-0 text-secondary py-0 px-1" + cssclass="border-0 py-0 px-1" text="sha-baz" title="Copy commit SHA" tooltipplacement="top" 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 c0ae972d519..f4eabf7bb67 100644 --- a/spec/frontend/packages/shared/components/package_list_row_spec.js +++ b/spec/frontend/packages/shared/components/package_list_row_spec.js @@ -1,6 +1,7 @@ -import { mount, shallowMount } from '@vue/test-utils'; +import { shallowMount } from '@vue/test-utils'; import PackagesListRow from '~/packages/shared/components/package_list_row.vue'; import PackageTags from '~/packages/shared/components/package_tags.vue'; +import ListItem from '~/vue_shared/components/registry/list_item.vue'; import { packageList } from '../../mock_data'; describe('packages_list_row', () => { @@ -17,14 +18,12 @@ describe('packages_list_row', () => { const mountComponent = ({ isGroup = false, packageEntity = packageWithoutTags, - shallow = true, showPackageType = true, disableDelete = false, } = {}) => { - const mountFunc = shallow ? shallowMount : mount; - - wrapper = mountFunc(PackagesListRow, { + wrapper = shallowMount(PackagesListRow, { store, + stubs: { ListItem }, propsData: { packageLink: 'foo', packageEntity, @@ -92,15 +91,14 @@ describe('packages_list_row', () => { }); describe('delete event', () => { - beforeEach(() => mountComponent({ packageEntity: packageWithoutTags, shallow: false })); + beforeEach(() => mountComponent({ packageEntity: packageWithoutTags })); - it('emits the packageToDelete event when the delete button is clicked', () => { - findDeleteButton().trigger('click'); + it('emits the packageToDelete event when the delete button is clicked', async () => { + findDeleteButton().vm.$emit('click'); - return wrapper.vm.$nextTick().then(() => { - expect(wrapper.emitted('packageToDelete')).toBeTruthy(); - expect(wrapper.emitted('packageToDelete')[0]).toEqual([packageWithoutTags]); - }); + await wrapper.vm.$nextTick(); + expect(wrapper.emitted('packageToDelete')).toBeTruthy(); + expect(wrapper.emitted('packageToDelete')[0]).toEqual([packageWithoutTags]); }); }); }); diff --git a/spec/frontend/registry/explorer/components/delete_button_spec.js b/spec/frontend/registry/explorer/components/delete_button_spec.js index bb0fe81117a..a79ca77a464 100644 --- a/spec/frontend/registry/explorer/components/delete_button_spec.js +++ b/spec/frontend/registry/explorer/components/delete_button_spec.js @@ -54,7 +54,6 @@ describe('delete_button', () => { mountComponent({ disabled: true }); expect(findButton().attributes()).toMatchObject({ 'aria-label': 'Foo title', - category: 'secondary', icon: 'remove', title: 'Foo title', variant: 'danger', diff --git a/spec/frontend/registry/explorer/components/details_page/tags_list_spec.js b/spec/frontend/registry/explorer/components/details_page/tags_list_spec.js index 1f560753476..401202026bb 100644 --- a/spec/frontend/registry/explorer/components/details_page/tags_list_spec.js +++ b/spec/frontend/registry/explorer/components/details_page/tags_list_spec.js @@ -115,7 +115,6 @@ describe('Tags List', () => { // The list has only two tags and for some reasons .at(-1) does not work expect(rows.at(1).attributes()).toMatchObject({ - last: 'true', isdesktop: 'true', }); }); 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 aaeaaf00748..c5b4b3fa5d8 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 @@ -3,7 +3,7 @@ import { GlIcon, GlSprintf } from '@gitlab/ui'; import { createMockDirective, getBinding } from 'helpers/vue_mock_directive'; import ClipboardButton from '~/vue_shared/components/clipboard_button.vue'; import Component from '~/registry/explorer/components/list_page/image_list_row.vue'; -import ListItem from '~/registry/explorer/components/list_item.vue'; +import ListItem from '~/vue_shared/components/registry/list_item.vue'; import DeleteButton from '~/registry/explorer/components/delete_button.vue'; import { ROW_SCHEDULED_FOR_DELETION, diff --git a/spec/frontend/registry/explorer/stubs.js b/spec/frontend/registry/explorer/stubs.js index 8f95fce2867..b6c0ee67757 100644 --- a/spec/frontend/registry/explorer/stubs.js +++ b/spec/frontend/registry/explorer/stubs.js @@ -1,5 +1,5 @@ import RealDeleteModal from '~/registry/explorer/components/details_page/delete_modal.vue'; -import RealListItem from '~/registry/explorer/components/list_item.vue'; +import RealListItem from '~/vue_shared/components/registry/list_item.vue'; export const GlModal = { template: '<div><slot name="modal-title"></slot><slot></slot><slot name="modal-ok"></slot></div>', diff --git a/spec/frontend/registry/explorer/components/list_item_spec.js b/spec/frontend/vue_shared/components/registry/list_item_spec.js index f244627a8c3..e2cfdedb4bf 100644 --- a/spec/frontend/registry/explorer/components/list_item_spec.js +++ b/spec/frontend/vue_shared/components/registry/list_item_spec.js @@ -1,6 +1,6 @@ import { GlButton } from '@gitlab/ui'; import { shallowMount } from '@vue/test-utils'; -import component from '~/registry/explorer/components/list_item.vue'; +import component from '~/vue_shared/components/registry/list_item.vue'; describe('list item', () => { let wrapper; @@ -34,7 +34,7 @@ describe('list item', () => { wrapper = null; }); - it.each` + describe.each` slotName | finderFunction ${'left-primary'} | ${findLeftPrimarySlot} ${'left-secondary'} | ${findLeftSecondarySlot} @@ -42,10 +42,18 @@ describe('list item', () => { ${'right-secondary'} | ${findRightSecondarySlot} ${'left-action'} | ${findLeftActionSlot} ${'right-action'} | ${findRightActionSlot} - `('has a $slotName slot', ({ finderFunction }) => { - mountComponent(); + `('$slotName slot', ({ finderFunction, slotName }) => { + it('exist when the slot is filled', () => { + mountComponent(); + + expect(finderFunction().exists()).toBe(true); + }); - expect(finderFunction().exists()).toBe(true); + it('does not exist when the slot is empty', () => { + mountComponent({}, { [slotName]: '' }); + + expect(finderFunction().exists()).toBe(false); + }); }); describe.each` @@ -106,51 +114,22 @@ describe('list item', () => { }); }); - describe('first prop', () => { - it('when is true displays a double top border', () => { - mountComponent({ first: true }); - - expect(wrapper.classes('gl-border-t-2')).toBe(true); - }); - - it('when is false display a single top border', () => { - mountComponent({ first: false }); - - expect(wrapper.classes('gl-border-t-1')).toBe(true); - }); - }); - - describe('last prop', () => { - it('when is true displays a double bottom border', () => { - mountComponent({ last: true }); - - expect(wrapper.classes('gl-border-b-2')).toBe(true); - }); - - it('when is false display a single bottom border', () => { - mountComponent({ last: false }); - - expect(wrapper.classes('gl-border-b-1')).toBe(true); - }); - }); - - describe('selected prop', () => { - it('when true applies the selected border and background', () => { - mountComponent({ selected: true }); - - expect(wrapper.classes()).toEqual( - expect.arrayContaining(['gl-bg-blue-50', 'gl-border-blue-200']), - ); - expect(wrapper.classes()).toEqual(expect.not.arrayContaining(['gl-border-gray-100'])); - }); - - it('when false applies the default border', () => { - mountComponent({ selected: false }); - - expect(wrapper.classes()).toEqual( - expect.not.arrayContaining(['gl-bg-blue-50', 'gl-border-blue-200']), - ); - expect(wrapper.classes()).toEqual(expect.arrayContaining(['gl-border-gray-100'])); - }); + describe('borders and selection', () => { + it.each` + first | selected | shouldHave | shouldNotHave + ${true} | ${true} | ${['gl-bg-blue-50', 'gl-border-blue-200']} | ${['gl-border-t-transparent', 'gl-border-t-gray-100']} + ${false} | ${true} | ${['gl-bg-blue-50', 'gl-border-blue-200']} | ${['gl-border-t-transparent', 'gl-border-t-gray-100']} + ${true} | ${false} | ${['gl-border-b-gray-100']} | ${['gl-bg-blue-50', 'gl-border-blue-200']} + ${false} | ${false} | ${['gl-border-b-gray-100']} | ${['gl-bg-blue-50', 'gl-border-blue-200']} + `( + 'when first is $first and selected is $selected', + ({ first, selected, shouldHave, shouldNotHave }) => { + mountComponent({ first, selected }); + + expect(wrapper.classes()).toEqual(expect.arrayContaining(shouldHave)); + + expect(wrapper.classes()).toEqual(expect.not.arrayContaining(shouldNotHave)); + }, + ); }); }); diff --git a/spec/helpers/search_helper_spec.rb b/spec/helpers/search_helper_spec.rb index ad8d42a79f4..a39ef8634df 100644 --- a/spec/helpers/search_helper_spec.rb +++ b/spec/helpers/search_helper_spec.rb @@ -73,6 +73,39 @@ RSpec.describe SearchHelper do expect(result.keys).to match_array(%i[category id label url avatar_url]) end + it 'includes the first 5 of the users recent issues' do + recent_issues = instance_double(::Gitlab::Search::RecentIssues) + expect(::Gitlab::Search::RecentIssues).to receive(:new).with(user: user).and_return(recent_issues) + project1 = create(:project, :with_avatar, namespace: user.namespace) + project2 = create(:project, namespace: user.namespace) + issue1 = create(:issue, title: 'issue 1', project: project1) + issue2 = create(:issue, title: 'issue 2', project: project2) + + other_issues = create_list(:issue, 5) + + expect(recent_issues).to receive(:search).with('the search term').and_return(Issue.id_in_ordered([issue1.id, issue2.id, *other_issues.map(&:id)])) + + results = search_autocomplete_opts("the search term") + + expect(results.count).to eq(5) + + expect(results[0]).to include({ + category: 'Recent issues', + id: issue1.id, + label: 'issue 1', + url: Gitlab::Routing.url_helpers.project_issue_path(issue1.project, issue1), + avatar_url: project1.avatar_url + }) + + expect(results[1]).to include({ + category: 'Recent issues', + id: issue2.id, + label: 'issue 2', + url: Gitlab::Routing.url_helpers.project_issue_path(issue2.project, issue2), + avatar_url: '' # This project didn't have an avatar so set this to '' + }) + end + it "does not include the public group" do group = create(:group) expect(search_autocomplete_opts(group.name).size).to eq(0) diff --git a/spec/lib/gitlab/database/partitioning/partition_monitoring_spec.rb b/spec/lib/gitlab/database/partitioning/partition_monitoring_spec.rb new file mode 100644 index 00000000000..67596211f71 --- /dev/null +++ b/spec/lib/gitlab/database/partitioning/partition_monitoring_spec.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::Partitioning::PartitionMonitoring do + describe '#report_metrics' do + subject { described_class.new(models).report_metrics } + + let(:models) { [model] } + let(:model) { double(partitioning_strategy: partitioning_strategy, table_name: table) } + let(:partitioning_strategy) { double(missing_partitions: missing_partitions, current_partitions: current_partitions) } + let(:table) { "some_table" } + + let(:missing_partitions) do + [double] + end + + let(:current_partitions) do + [double, double] + end + + it 'reports number of present partitions' do + subject + + expect(Gitlab::Metrics.registry.get(:db_partitions_present).get({ table: table })).to eq(current_partitions.size) + end + + it 'reports number of missing partitions' do + subject + + expect(Gitlab::Metrics.registry.get(:db_partitions_missing).get({ table: table })).to eq(missing_partitions.size) + end + end +end diff --git a/spec/lib/gitlab/search/recent_issues_spec.rb b/spec/lib/gitlab/search/recent_issues_spec.rb new file mode 100644 index 00000000000..211c7658e71 --- /dev/null +++ b/spec/lib/gitlab/search/recent_issues_spec.rb @@ -0,0 +1,123 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ::Gitlab::Search::RecentIssues, :clean_gitlab_redis_shared_state do + let(:user) { create(:user) } + let(:issue) { create(:issue, title: 'hello world 1', project: project) } + let(:recent_issues) { described_class.new(user: user, items_limit: 5) } + let(:project) { create(:project, :public) } + + before do + stub_feature_flags(recent_items_search: true) + end + + describe '#log_viewing' do + it 'adds the item to the recent items' do + recent_issues.log_view(issue) + + results = recent_issues.search('hello') + + expect(results).to eq([issue]) + end + + it 'removes an item when it exceeds the size items_limit' do + (1..6).each do |i| + recent_issues.log_view(create(:issue, title: "issue #{i}", project: project)) + end + + results = recent_issues.search('issue') + + expect(results.map(&:title)).to contain_exactly('issue 6', 'issue 5', 'issue 4', 'issue 3', 'issue 2') + end + + it 'expires the items after expires_after' do + recent_issues = described_class.new(user: user, expires_after: 0) + + recent_issues.log_view(issue) + + results = recent_issues.search('hello') + + expect(results).to be_empty + end + + it 'does not include results logged for another user' do + another_user = create(:user) + another_issue = create(:issue, title: 'hello world 2', project: project) + described_class.new(user: another_user).log_view(another_issue) + recent_issues.log_view(issue) + + results = recent_issues.search('hello') + + expect(results).to eq([issue]) + end + + context 'when recent_items_search feature flag is disabled' do + before do + stub_feature_flags(recent_items_search: false) + end + + it 'does not store anything' do + recent_issues.log_view(issue) + + # Re-enable before searching to prove that the `log_view` call did + # not persist it + stub_feature_flags(recent_items_search: true) + + results = recent_issues.search('hello') + + expect(results).to be_empty + end + end + end + + describe '#search' do + let(:issue1) { create(:issue, title: "matching issue 1", project: project) } + let(:issue2) { create(:issue, title: "matching issue 2", project: project) } + let(:issue3) { create(:issue, title: "matching issue 3", project: project) } + let(:non_matching_issue) { create(:issue, title: "different issue", project: project) } + let!(:non_viewed_issued) { create(:issue, title: "matching but not viewed issue", project: project) } + + before do + recent_issues.log_view(issue1) + recent_issues.log_view(issue2) + recent_issues.log_view(issue3) + recent_issues.log_view(non_matching_issue) + end + + it 'matches partial text in the issue title' do + expect(recent_issues.search('matching')).to contain_exactly(issue1, issue2, issue3) + end + + it 'returns results sorted by recently viewed' do + recent_issues.log_view(issue2) + + expect(recent_issues.search('matching')).to eq([issue2, issue3, issue1]) + end + + it 'does not leak issues you no longer have access to' do + private_project = create(:project, :public, namespace: create(:group)) + private_issue = create(:issue, project: private_project, title: 'matching issue title') + + recent_issues.log_view(private_issue) + + private_project.update!(visibility_level: Project::PRIVATE) + + expect(recent_issues.search('matching')).not_to include(private_issue) + end + + context 'when recent_items_search feature flag is disabled' do + it 'does not return anything' do + recent_issues.log_view(issue) + + # Disable after persisting to prove that the `search` is not searching + # anything + stub_feature_flags(recent_items_search: false) + + results = recent_issues.search('hello') + + expect(results).to be_empty + end + end + end +end diff --git a/spec/lib/gitlab/sql/except_spec.rb b/spec/lib/gitlab/sql/except_spec.rb new file mode 100644 index 00000000000..a3d6990ee2e --- /dev/null +++ b/spec/lib/gitlab/sql/except_spec.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::SQL::Except do + it_behaves_like 'SQL set operator', 'EXCEPT' +end diff --git a/spec/lib/gitlab/sql/intersect_spec.rb b/spec/lib/gitlab/sql/intersect_spec.rb new file mode 100644 index 00000000000..cf076796712 --- /dev/null +++ b/spec/lib/gitlab/sql/intersect_spec.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::SQL::Intersect do + it_behaves_like 'SQL set operator', 'INTERSECT' +end diff --git a/spec/lib/gitlab/sql/union_spec.rb b/spec/lib/gitlab/sql/union_spec.rb index c8be83c093d..a41551e25bf 100644 --- a/spec/lib/gitlab/sql/union_spec.rb +++ b/spec/lib/gitlab/sql/union_spec.rb @@ -3,40 +3,5 @@ require 'spec_helper' RSpec.describe Gitlab::SQL::Union do - let(:relation_1) { User.where(email: 'alice@example.com').select(:id) } - let(:relation_2) { User.where(email: 'bob@example.com').select(:id) } - - def to_sql(relation) - relation.reorder(nil).to_sql - end - - describe '#to_sql' do - it 'returns a String joining relations together using a UNION' do - union = described_class.new([relation_1, relation_2]) - - expect(union.to_sql).to eq("(#{to_sql(relation_1)})\nUNION\n(#{to_sql(relation_2)})") - end - - it 'skips Model.none segements' do - empty_relation = User.none - union = described_class.new([empty_relation, relation_1, relation_2]) - - expect {User.where("users.id IN (#{union.to_sql})").to_a}.not_to raise_error - expect(union.to_sql).to eq("(#{to_sql(relation_1)})\nUNION\n(#{to_sql(relation_2)})") - end - - it 'uses UNION ALL when removing duplicates is disabled' do - union = described_class - .new([relation_1, relation_2], remove_duplicates: false) - - expect(union.to_sql).to include('UNION ALL') - end - - it 'returns `NULL` if all relations are empty' do - empty_relation = User.none - union = described_class.new([empty_relation, empty_relation]) - - expect(union.to_sql).to eq('NULL') - end - end + it_behaves_like 'SQL set operator', 'UNION' end diff --git a/spec/lib/gitlab/usage_data_spec.rb b/spec/lib/gitlab/usage_data_spec.rb index 69af6c523f5..2a51a9e3410 100644 --- a/spec/lib/gitlab/usage_data_spec.rb +++ b/spec/lib/gitlab/usage_data_spec.rb @@ -959,24 +959,25 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do end end - describe '.merge_requests_users' do - let(:time_period) { { created_at: 2.days.ago..Time.current } } - let(:merge_request) { create(:merge_request) } - let(:other_user) { create(:user) } - let(:another_user) { create(:user) } + describe '.merge_requests_users', :clean_gitlab_redis_shared_state do + let(:time_period) { { created_at: 2.days.ago..time } } + let(:time) { Time.current } before do - create(:event, target: merge_request, author: merge_request.author, created_at: 1.day.ago) - create(:event, target: merge_request, author: merge_request.author, created_at: 1.hour.ago) - create(:event, target: merge_request, author: merge_request.author, created_at: 3.days.ago) - create(:event, target: merge_request, author: other_user, created_at: 1.day.ago) - create(:event, target: merge_request, author: other_user, created_at: 1.hour.ago) - create(:event, target: merge_request, author: other_user, created_at: 3.days.ago) - create(:event, target: merge_request, author: another_user, created_at: 4.days.ago) + counter = Gitlab::UsageDataCounters::TrackUniqueEvents + merge_request = Event::TARGET_TYPES[:merge_request] + project = Event::TARGET_TYPES[:project] + + counter.track_event(event_action: :commented, event_target: merge_request, author_id: 1, time: time) + counter.track_event(event_action: :opened, event_target: merge_request, author_id: 1, time: time) + counter.track_event(event_action: :merged, event_target: merge_request, author_id: 2, time: time) + counter.track_event(event_action: :closed, event_target: merge_request, author_id: 3, time: time) + counter.track_event(event_action: :opened, event_target: merge_request, author_id: 4, time: time - 3.days) + counter.track_event(event_action: :created, event_target: project, author_id: 5, time: time) end it 'returns the distinct count of users using merge requests (via events table) within the specified time period' do - expect(described_class.merge_requests_users(time_period)).to eq(2) + expect(described_class.merge_requests_users(time_period)).to eq(3) end end diff --git a/spec/models/concerns/from_except_spec.rb b/spec/models/concerns/from_except_spec.rb new file mode 100644 index 00000000000..3b081b4f572 --- /dev/null +++ b/spec/models/concerns/from_except_spec.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe FromExcept do + it_behaves_like 'from set operator', Gitlab::SQL::Except +end diff --git a/spec/models/concerns/from_intersect_spec.rb b/spec/models/concerns/from_intersect_spec.rb new file mode 100644 index 00000000000..78884f8ae56 --- /dev/null +++ b/spec/models/concerns/from_intersect_spec.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe FromIntersect do + it_behaves_like 'from set operator', Gitlab::SQL::Intersect +end diff --git a/spec/models/concerns/from_set_operator_spec.rb b/spec/models/concerns/from_set_operator_spec.rb new file mode 100644 index 00000000000..8ebbb5550c9 --- /dev/null +++ b/spec/models/concerns/from_set_operator_spec.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe FromSetOperator do + describe 'when set operator method already exists' do + let(:redefine_method) do + Class.new do + def self.from_union + # This method intentionally left blank. + end + + extend FromSetOperator + define_set_operator Gitlab::SQL::Union + end + end + + it { expect { redefine_method }.to raise_exception(RuntimeError) } + end +end diff --git a/spec/models/concerns/from_union_spec.rb b/spec/models/concerns/from_union_spec.rb index 9819a6ec3de..bd2893090a8 100644 --- a/spec/models/concerns/from_union_spec.rb +++ b/spec/models/concerns/from_union_spec.rb @@ -3,38 +3,13 @@ require 'spec_helper' RSpec.describe FromUnion do - describe '.from_union' do - let(:model) do - Class.new(ActiveRecord::Base) do - self.table_name = 'users' - - include FromUnion + [true, false].each do |sql_set_operator| + context "when sql-set-operators feature flag is #{sql_set_operator}" do + before do + stub_feature_flags(sql_set_operators: sql_set_operator) end - end - - it 'selects from the results of the UNION' do - query = model.from_union([model.where(id: 1), model.where(id: 2)]) - - expect(query.to_sql).to match(/FROM \(\(SELECT.+\)\nUNION\n\(SELECT.+\)\) users/m) - end - - it 'supports the use of a custom alias for the sub query' do - query = model.from_union( - [model.where(id: 1), model.where(id: 2)], - alias_as: 'kittens' - ) - - expect(query.to_sql).to match(/FROM \(\(SELECT.+\)\nUNION\n\(SELECT.+\)\) kittens/m) - end - - it 'supports keeping duplicate rows' do - query = model.from_union( - [model.where(id: 1), model.where(id: 2)], - remove_duplicates: false - ) - expect(query.to_sql) - .to match(/FROM \(\(SELECT.+\)\nUNION ALL\n\(SELECT.+\)\) users/m) + it_behaves_like 'from set operator', Gitlab::SQL::Union end end end diff --git a/spec/models/concerns/id_in_ordered_spec.rb b/spec/models/concerns/id_in_ordered_spec.rb new file mode 100644 index 00000000000..6c507c242ea --- /dev/null +++ b/spec/models/concerns/id_in_ordered_spec.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe IdInOrdered do + describe 'Issue' do + describe '.id_in_ordered' do + it 'returns issues matching the ids in the same order as the ids' do + issue1 = create(:issue) + issue2 = create(:issue) + issue3 = create(:issue) + issue4 = create(:issue) + issue5 = create(:issue) + + expect(Issue.id_in_ordered([issue3.id, issue1.id, issue4.id, issue5.id, issue2.id])).to eq([ + issue3, issue1, issue4, issue5, issue2 +]) + end + + context 'when the ids are not an array of integers' do + it 'raises ArgumentError' do + expect { Issue.id_in_ordered([1, 'no SQL injection']) }.to raise_error(ArgumentError, "ids must be an array of integers") + end + end + + context 'when an empty array is given' do + it 'does not raise error' do + expect(Issue.id_in_ordered([]).to_a).to be_empty + end + end + end + end +end diff --git a/spec/requests/api/graphql/project/issues_spec.rb b/spec/requests/api/graphql/project/issues_spec.rb index 5c4882bb421..ceb32476f7d 100644 --- a/spec/requests/api/graphql/project/issues_spec.rb +++ b/spec/requests/api/graphql/project/issues_spec.rb @@ -261,7 +261,7 @@ RSpec.describe 'getting an issue list for a project' do <<~QUERY edges { node { - id + iid alertManagementAlert { title } @@ -270,9 +270,9 @@ RSpec.describe 'getting an issue list for a project' do QUERY end - # Alerts need to reporter and above + # Alerts need to have developer permission and above before do - project.add_reporter(current_user) + project.add_developer(current_user) end it 'avoids N+1 queries' do @@ -286,7 +286,10 @@ RSpec.describe 'getting an issue list for a project' do it 'returns the alert data' do post_graphql(query, current_user: current_user) - issues_data + alert_titles = issues_data.map { |issue| issue.dig('node', 'alertManagementAlert', 'title') } + expected_titles = issues.map { |issue| issue.alert_management_alert&.title } + + expect(alert_titles).to contain_exactly(*expected_titles) end end end diff --git a/spec/requests/api/helpers_spec.rb b/spec/requests/api/helpers_spec.rb index fefa7105327..090ff3c1b5a 100644 --- a/spec/requests/api/helpers_spec.rb +++ b/spec/requests/api/helpers_spec.rb @@ -39,7 +39,7 @@ RSpec.describe API::Helpers do end def error!(message, status, header) - raise Exception.new("#{status} - #{message}") + raise StandardError.new("#{status} - #{message}") end def set_param(key, value) diff --git a/spec/rubocop/cop/gitlab/except_spec.rb b/spec/rubocop/cop/gitlab/except_spec.rb new file mode 100644 index 00000000000..50277d15a57 --- /dev/null +++ b/spec/rubocop/cop/gitlab/except_spec.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require 'rubocop' +require 'rubocop/rspec/support' +require_relative '../../../../rubocop/cop/gitlab/except' + +RSpec.describe RuboCop::Cop::Gitlab::Except, type: :rubocop do + include CopHelper + + subject(:cop) { described_class.new } + + it 'flags the use of Gitlab::SQL::Except.new' do + expect_offense(<<~SOURCE) + Gitlab::SQL::Except.new([foo]) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use the `FromExcept` concern, instead of using `Gitlab::SQL::Except` directly + SOURCE + end +end diff --git a/spec/rubocop/cop/gitlab/intersect_spec.rb b/spec/rubocop/cop/gitlab/intersect_spec.rb new file mode 100644 index 00000000000..351033d0ed2 --- /dev/null +++ b/spec/rubocop/cop/gitlab/intersect_spec.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require 'rubocop' +require 'rubocop/rspec/support' +require_relative '../../../../rubocop/cop/gitlab/intersect' + +RSpec.describe RuboCop::Cop::Gitlab::Intersect, type: :rubocop do + include CopHelper + + subject(:cop) { described_class.new } + + it 'flags the use of Gitlab::SQL::Intersect.new' do + expect_offense(<<~SOURCE) + Gitlab::SQL::Intersect.new([foo]) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use the `FromIntersect` concern, instead of using `Gitlab::SQL::Intersect` directly + SOURCE + end +end diff --git a/spec/rubocop/cop/put_group_routes_under_scope_spec.rb b/spec/rubocop/cop/put_group_routes_under_scope_spec.rb index c55d9bf22d6..888d1b6a2ba 100644 --- a/spec/rubocop/cop/put_group_routes_under_scope_spec.rb +++ b/spec/rubocop/cop/put_group_routes_under_scope_spec.rb @@ -46,4 +46,18 @@ RSpec.describe RuboCop::Cop::PutGroupRoutesUnderScope, type: :rubocop do end PATTERN end + + it 'does not register an offense for the root route' do + expect_no_offenses(<<~PATTERN) + get '/' + PATTERN + end + + it 'does not register an offense for the root route within scope' do + expect_no_offenses(<<~PATTERN) + scope(path: 'groups/*group_id/-', module: :groups) do + get '/' + end + PATTERN + end end diff --git a/spec/rubocop/cop/put_project_routes_under_scope_spec.rb b/spec/rubocop/cop/put_project_routes_under_scope_spec.rb index 05e1cd7b693..eebb7f3eb61 100644 --- a/spec/rubocop/cop/put_project_routes_under_scope_spec.rb +++ b/spec/rubocop/cop/put_project_routes_under_scope_spec.rb @@ -46,4 +46,18 @@ RSpec.describe RuboCop::Cop::PutProjectRoutesUnderScope, type: :rubocop do end PATTERN end + + it 'does not register an offense for the root route' do + expect_no_offenses(<<~PATTERN) + get '/' + PATTERN + end + + it 'does not register an offense for the root route within scope' do + expect_no_offenses(<<~PATTERN) + scope '-' do + get '/' + end + PATTERN + end end diff --git a/spec/serializers/pipeline_entity_spec.rb b/spec/serializers/pipeline_entity_spec.rb index e00f05a8fe8..d7cd13edec8 100644 --- a/spec/serializers/pipeline_entity_spec.rb +++ b/spec/serializers/pipeline_entity_spec.rb @@ -260,13 +260,5 @@ RSpec.describe PipelineEntity do end end end - - context 'when pipeline has build report results' do - let(:pipeline) { create(:ci_pipeline, :with_report_results, project: project, user: user) } - - it 'exposes tests total count' do - expect(subject[:tests_total_count]).to eq(2) - end - end end end diff --git a/spec/serializers/pipeline_serializer_spec.rb b/spec/serializers/pipeline_serializer_spec.rb index dfe51e9006f..b42a4f6ad3f 100644 --- a/spec/serializers/pipeline_serializer_spec.rb +++ b/spec/serializers/pipeline_serializer_spec.rb @@ -155,7 +155,7 @@ RSpec.describe PipelineSerializer do it 'verifies number of queries', :request_store do recorded = ActiveRecord::QueryRecorder.new { subject } - expected_queries = Gitlab.ee? ? 46 : 43 + expected_queries = Gitlab.ee? ? 43 : 40 expect(recorded.count).to be_within(2).of(expected_queries) expect(recorded.cached_count).to eq(0) diff --git a/spec/services/event_create_service_spec.rb b/spec/services/event_create_service_spec.rb index 039aa12265f..3c67c15f10a 100644 --- a/spec/services/event_create_service_spec.rb +++ b/spec/services/event_create_service_spec.rb @@ -8,6 +8,16 @@ RSpec.describe EventCreateService do let_it_be(:user, reload: true) { create :user } let_it_be(:project) { create(:project) } + shared_examples 'it records the event in the event counter' do + specify do + tracking_params = { event_action: event_action, date_from: Date.yesterday, date_to: Date.today } + + expect { subject } + .to change { Gitlab::UsageDataCounters::TrackUniqueEvents.count_unique_events(tracking_params) } + .by(1) + end + end + describe 'Issues' do describe '#open_issue' do let(:issue) { create(:issue) } @@ -40,34 +50,52 @@ RSpec.describe EventCreateService do end end - describe 'Merge Requests' do + describe 'Merge Requests', :clean_gitlab_redis_shared_state do describe '#open_mr' do + subject(:open_mr) { service.open_mr(merge_request, merge_request.author) } + let(:merge_request) { create(:merge_request) } - it { expect(service.open_mr(merge_request, merge_request.author)).to be_truthy } + it { expect(open_mr).to be_truthy } it "creates new event" do - expect { service.open_mr(merge_request, merge_request.author) }.to change { Event.count } + expect { open_mr }.to change { Event.count } + end + + it_behaves_like "it records the event in the event counter" do + let(:event_action) { Gitlab::UsageDataCounters::TrackUniqueEvents::MERGE_REQUEST_ACTION } end end describe '#close_mr' do + subject(:close_mr) { service.close_mr(merge_request, merge_request.author) } + let(:merge_request) { create(:merge_request) } - it { expect(service.close_mr(merge_request, merge_request.author)).to be_truthy } + it { expect(close_mr).to be_truthy } it "creates new event" do - expect { service.close_mr(merge_request, merge_request.author) }.to change { Event.count } + expect { close_mr }.to change { Event.count } + end + + it_behaves_like "it records the event in the event counter" do + let(:event_action) { Gitlab::UsageDataCounters::TrackUniqueEvents::MERGE_REQUEST_ACTION } end end describe '#merge_mr' do + subject(:merge_mr) { service.merge_mr(merge_request, merge_request.author) } + let(:merge_request) { create(:merge_request) } - it { expect(service.merge_mr(merge_request, merge_request.author)).to be_truthy } + it { expect(merge_mr).to be_truthy } it "creates new event" do - expect { service.merge_mr(merge_request, merge_request.author) }.to change { Event.count } + expect { merge_mr }.to change { Event.count } + end + + it_behaves_like "it records the event in the event counter" do + let(:event_action) { Gitlab::UsageDataCounters::TrackUniqueEvents::MERGE_REQUEST_ACTION } end end @@ -180,6 +208,8 @@ RSpec.describe EventCreateService do where(:action) { Event::WIKI_ACTIONS.map { |action| [action] } } with_them do + subject { create_event } + it 'creates the event' do expect(create_event).to have_attributes( wiki_page?: true, @@ -201,13 +231,8 @@ RSpec.describe EventCreateService do expect(duplicate).to eq(event) end - it 'records the event in the event counter' do - counter_class = Gitlab::UsageDataCounters::TrackUniqueEvents - tracking_params = { event_action: counter_class::WIKI_ACTION, date_from: Date.yesterday, date_to: Date.today } - - expect { create_event } - .to change { counter_class.count_unique_events(tracking_params) } - .by(1) + it_behaves_like "it records the event in the event counter" do + let(:event_action) { Gitlab::UsageDataCounters::TrackUniqueEvents::WIKI_ACTION } end end @@ -242,13 +267,8 @@ RSpec.describe EventCreateService do it_behaves_like 'service for creating a push event', PushEventPayloadService - it 'records the event in the event counter' do - counter_class = Gitlab::UsageDataCounters::TrackUniqueEvents - tracking_params = { event_action: counter_class::PUSH_ACTION, date_from: Date.yesterday, date_to: Date.today } - - expect { subject } - .to change { counter_class.count_unique_events(tracking_params) } - .from(0).to(1) + it_behaves_like "it records the event in the event counter" do + let(:event_action) { Gitlab::UsageDataCounters::TrackUniqueEvents::PUSH_ACTION } end end @@ -265,13 +285,8 @@ RSpec.describe EventCreateService do it_behaves_like 'service for creating a push event', BulkPushEventPayloadService - it 'records the event in the event counter' do - counter_class = Gitlab::UsageDataCounters::TrackUniqueEvents - tracking_params = { event_action: counter_class::PUSH_ACTION, date_from: Date.yesterday, date_to: Date.today } - - expect { subject } - .to change { counter_class.count_unique_events(tracking_params) } - .from(0).to(1) + it_behaves_like "it records the event in the event counter" do + let(:event_action) { Gitlab::UsageDataCounters::TrackUniqueEvents::PUSH_ACTION } end end @@ -299,7 +314,7 @@ RSpec.describe EventCreateService do let_it_be(:updated) { create_list(:design, 5) } let_it_be(:created) { create_list(:design, 3) } - let(:result) { service.save_designs(author, create: created, update: updated) } + subject(:result) { service.save_designs(author, create: created, update: updated) } specify { expect { result }.to change { Event.count }.by(8) } @@ -319,13 +334,8 @@ RSpec.describe EventCreateService do expect(events.map(&:design)).to match_array(updated) end - it 'records the event in the event counter' do - counter_class = Gitlab::UsageDataCounters::TrackUniqueEvents - tracking_params = { event_action: counter_class::DESIGN_ACTION, date_from: Date.yesterday, date_to: Date.today } - - expect { result } - .to change { counter_class.count_unique_events(tracking_params) } - .from(0).to(1) + it_behaves_like "it records the event in the event counter" do + let(:event_action) { Gitlab::UsageDataCounters::TrackUniqueEvents::DESIGN_ACTION } end end @@ -333,7 +343,7 @@ RSpec.describe EventCreateService do let_it_be(:designs) { create_list(:design, 5) } let_it_be(:author) { create(:user) } - let(:result) { service.destroy_designs(designs, author) } + subject(:result) { service.destroy_designs(designs, author) } specify { expect { result }.to change { Event.count }.by(5) } @@ -346,13 +356,37 @@ RSpec.describe EventCreateService do expect(events.map(&:design)).to match_array(designs) end - it 'records the event in the event counter' do + it_behaves_like "it records the event in the event counter" do + let(:event_action) { Gitlab::UsageDataCounters::TrackUniqueEvents::DESIGN_ACTION } + end + end + end + + describe '#leave_note' do + subject(:leave_note) { service.leave_note(note, author) } + + let(:note) { create(:note) } + let(:author) { create(:user) } + let(:event_action) { Gitlab::UsageDataCounters::TrackUniqueEvents::MERGE_REQUEST_ACTION } + + it { expect(leave_note).to be_truthy } + + it "creates new event" do + expect { leave_note }.to change { Event.count }.by(1) + end + + context 'when it is a diff note' do + it_behaves_like "it records the event in the event counter" do + let(:note) { create(:diff_note_on_merge_request) } + end + end + + context 'when it is not a diff note' do + it 'does not change the unique action counter' do counter_class = Gitlab::UsageDataCounters::TrackUniqueEvents - tracking_params = { event_action: counter_class::DESIGN_ACTION, date_from: Date.yesterday, date_to: Date.today } + tracking_params = { event_action: event_action, date_from: Date.yesterday, date_to: Date.today } - expect { result } - .to change { counter_class.count_unique_events(tracking_params) } - .from(0).to(1) + expect { subject }.not_to change { counter_class.count_unique_events(tracking_params) } end end end diff --git a/spec/services/issues/resolve_discussions_spec.rb b/spec/services/issues/resolve_discussions_spec.rb index a541d92feb2..9fbc9cbcca6 100644 --- a/spec/services/issues/resolve_discussions_spec.rb +++ b/spec/services/issues/resolve_discussions_spec.rb @@ -79,7 +79,7 @@ RSpec.describe Issues::ResolveDiscussions do noteable: merge_request, project: merge_request.target_project, line_number: 15 - )]) + )]) service = DummyService.new( project, user, diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index dbe7f88a451..7832d727220 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/spec/services/projects/update_service_spec.rb @@ -272,7 +272,7 @@ RSpec.describe Projects::UpdateService do result = update_project(project, user, project_feature_attributes: { issues_access_level: ProjectFeature::PRIVATE } - ) + ) expect(result).to eq({ status: :success }) expect(project.project_feature.issues_access_level).to be(ProjectFeature::PRIVATE) @@ -479,14 +479,14 @@ RSpec.describe Projects::UpdateService do attributes_for(:prometheus_service, project: project, properties: { api_url: "http://new.prometheus.com", manual_configuration: "0" } - ) + ) end let!(:prometheus_service) do create(:prometheus_service, project: project, properties: { api_url: "http://old.prometheus.com", manual_configuration: "0" } - ) + ) end it 'updates existing record' do @@ -503,7 +503,7 @@ RSpec.describe Projects::UpdateService do attributes_for(:prometheus_service, project: project, properties: { api_url: "http://example.prometheus.com", manual_configuration: "0" } - ) + ) end it 'creates new record' do @@ -519,7 +519,7 @@ RSpec.describe Projects::UpdateService do attributes_for(:prometheus_service, project: project, properties: { api_url: nil, manual_configuration: "1" } - ) + ) end it 'does not create new record' do diff --git a/spec/support/helpers/repo_helpers.rb b/spec/support/helpers/repo_helpers.rb index 7741c805b37..bbba58d60d6 100644 --- a/spec/support/helpers/repo_helpers.rb +++ b/spec/support/helpers/repo_helpers.rb @@ -125,7 +125,7 @@ eos end def create_file_in_repo( - project, start_branch, branch_name, filename, content, + project, start_branch, branch_name, filename, content, commit_message: 'Add new content') Files::CreateService.new( project, diff --git a/spec/support/helpers/stub_object_storage.rb b/spec/support/helpers/stub_object_storage.rb index 8a52a614821..0486eccf48c 100644 --- a/spec/support/helpers/stub_object_storage.rb +++ b/spec/support/helpers/stub_object_storage.rb @@ -14,14 +14,14 @@ module StubObjectStorage end def stub_object_storage_uploader( - config:, + config:, uploader:, remote_directory:, enabled: true, proxy_download: false, background_upload: false, direct_upload: false - ) + ) allow(config).to receive(:enabled) { enabled } allow(config).to receive(:proxy_download) { proxy_download } allow(config).to receive(:background_upload) { background_upload } diff --git a/spec/support/import_export/configuration_helper.rb b/spec/support/import_export/configuration_helper.rb index 6f67b0f3dd7..b731ee626c0 100644 --- a/spec/support/import_export/configuration_helper.rb +++ b/spec/support/import_export/configuration_helper.rb @@ -44,8 +44,8 @@ module ConfigurationHelper import_export_config = config_hash(config) excluded_attributes = import_export_config[:excluded_attributes][relation_name.to_sym] included_attributes = import_export_config[:included_attributes][relation_name.to_sym] - attributes = attributes - Gitlab::Json.parse(excluded_attributes.to_json) if excluded_attributes - attributes = attributes & Gitlab::Json.parse(included_attributes.to_json) if included_attributes + attributes -= Gitlab::Json.parse(excluded_attributes.to_json) if excluded_attributes + attributes &= Gitlab::Json.parse(included_attributes.to_json) if included_attributes attributes end diff --git a/spec/support/shared_examples/lib/gitlab/sql/set_operator_shared_examples.rb b/spec/support/shared_examples/lib/gitlab/sql/set_operator_shared_examples.rb new file mode 100644 index 00000000000..73beef06855 --- /dev/null +++ b/spec/support/shared_examples/lib/gitlab/sql/set_operator_shared_examples.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'SQL set operator' do |operator_keyword| + operator_keyword = operator_keyword.upcase + + let(:relation_1) { User.where(email: 'alice@example.com').select(:id) } + let(:relation_2) { User.where(email: 'bob@example.com').select(:id) } + + def to_sql(relation) + relation.reorder(nil).to_sql + end + + describe '.operator_keyword' do + it { expect(described_class.operator_keyword).to eq operator_keyword } + end + + describe '#to_sql' do + it "returns a String joining relations together using a #{operator_keyword}" do + set_operator = described_class.new([relation_1, relation_2]) + + expect(set_operator.to_sql).to eq("(#{to_sql(relation_1)})\n#{operator_keyword}\n(#{to_sql(relation_2)})") + end + + it 'skips Model.none segements' do + empty_relation = User.none + set_operator = described_class.new([empty_relation, relation_1, relation_2]) + + expect {User.where("users.id IN (#{set_operator.to_sql})").to_a}.not_to raise_error + expect(set_operator.to_sql).to eq("(#{to_sql(relation_1)})\n#{operator_keyword}\n(#{to_sql(relation_2)})") + end + + it "uses #{operator_keyword} ALL when removing duplicates is disabled" do + set_operator = described_class + .new([relation_1, relation_2], remove_duplicates: false) + + expect(set_operator.to_sql).to include("#{operator_keyword} ALL") + end + + it 'returns `NULL` if all relations are empty' do + empty_relation = User.none + set_operator = described_class.new([empty_relation, empty_relation]) + + expect(set_operator.to_sql).to eq('NULL') + end + end +end diff --git a/spec/support/shared_examples/models/concerns/from_set_operator_shared_examples.rb b/spec/support/shared_examples/models/concerns/from_set_operator_shared_examples.rb new file mode 100644 index 00000000000..6b208c0024d --- /dev/null +++ b/spec/support/shared_examples/models/concerns/from_set_operator_shared_examples.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'from set operator' do |sql_klass| + from_set_operator_concern = described_class + operator_keyword = sql_klass.operator_keyword + operator_method = "from_#{sql_klass.operator_keyword.downcase}" + + describe "##{operator_method}" do + let(:model) do + Class.new(ActiveRecord::Base) do + self.table_name = 'users' + + include from_set_operator_concern + end + end + + it "selects from the results of the #{operator_keyword}" do + query = model.public_send(operator_method, [model.where(id: 1), model.where(id: 2)]) + + expect(query.to_sql).to match(/FROM \(\(SELECT.+\)\n#{operator_keyword}\n\(SELECT.+\)\) users/m) + end + + it 'supports the use of a custom alias for the sub query' do + query = model.public_send(operator_method, + [model.where(id: 1), model.where(id: 2)], + alias_as: 'kittens' + ) + + expect(query.to_sql).to match(/FROM \(\(SELECT.+\)\n#{operator_keyword}\n\(SELECT.+\)\) kittens/m) + end + + it 'supports keeping duplicate rows' do + query = model.public_send(operator_method, + [model.where(id: 1), model.where(id: 2)], + remove_duplicates: false + ) + + expect(query.to_sql) + .to match(/FROM \(\(SELECT.+\)\n#{operator_keyword} ALL\n\(SELECT.+\)\) users/m) + end + end +end diff --git a/spec/support/shared_examples/requests/api/custom_attributes_shared_examples.rb b/spec/support/shared_examples/requests/api/custom_attributes_shared_examples.rb index ef5070c2a40..0dfc49f2d41 100644 --- a/spec/support/shared_examples/requests/api/custom_attributes_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/custom_attributes_shared_examples.rb @@ -132,7 +132,7 @@ RSpec.shared_examples 'custom attributes endpoints' do |attributable_name| end context 'with an authorized user' do - it'returns a single custom attribute' do + it 'returns a single custom attribute' do get api("/#{attributable_name}/#{attributable.id}/custom_attributes/foo", admin) expect(response).to have_gitlab_http_status(:ok) diff --git a/spec/workers/partition_creation_worker_spec.rb b/spec/workers/partition_creation_worker_spec.rb index 50ed9c901c1..37225cc1f79 100644 --- a/spec/workers/partition_creation_worker_spec.rb +++ b/spec/workers/partition_creation_worker_spec.rb @@ -4,16 +4,26 @@ require "spec_helper" RSpec.describe PartitionCreationWorker do describe '#perform' do - let(:creator) { double(create_partitions: nil) } + subject { described_class.new.perform } + + let(:creator) { instance_double('PartitionCreator', create_partitions: nil) } + let(:monitoring) { instance_double('PartitionMonitoring', report_metrics: nil) } before do allow(Gitlab::Database::Partitioning::PartitionCreator).to receive(:new).and_return(creator) + allow(Gitlab::Database::Partitioning::PartitionMonitoring).to receive(:new).and_return(monitoring) end it 'delegates to PartitionCreator' do expect(creator).to receive(:create_partitions) - described_class.new.perform + subject + end + + it 'reports partition metrics' do + expect(monitoring).to receive(:report_metrics) + + subject end end end diff --git a/spec/workers/remove_unreferenced_lfs_objects_worker_spec.rb b/spec/workers/remove_unreferenced_lfs_objects_worker_spec.rb index f14c2b67f2c..21b9a7b844b 100644 --- a/spec/workers/remove_unreferenced_lfs_objects_worker_spec.rb +++ b/spec/workers/remove_unreferenced_lfs_objects_worker_spec.rb @@ -16,21 +16,21 @@ RSpec.describe RemoveUnreferencedLfsObjectsWorker do create(:lfs_objects_project, project: project1, lfs_object: referenced_lfs_object1 - ) + ) end let!(:lfs_objects_project2_1) do create(:lfs_objects_project, project: project2, lfs_object: referenced_lfs_object1 - ) + ) end let!(:lfs_objects_project1_2) do create(:lfs_objects_project, project: project1, lfs_object: referenced_lfs_object2 - ) + ) end it 'removes unreferenced lfs objects' do diff --git a/yarn.lock b/yarn.lock index c189cf64aa3..62102a3cbec 100644 --- a/yarn.lock +++ b/yarn.lock @@ -848,10 +848,10 @@ resolved "https://registry.yarnpkg.com/@gitlab/svgs/-/svgs-1.161.0.tgz#661e8d19862dfba0e4c558e2eb6d64b402c1453e" integrity sha512-qsbboEICn08ZoEoAX/TuYygsFaXlzsCY+CfmdOzqvJbOdfHhVXmrJBxd2hP2qqjTZm2PkbRRmn+03+ce1jvatQ== -"@gitlab/ui@20.13.0": - version "20.13.0" - resolved "https://registry.yarnpkg.com/@gitlab/ui/-/ui-20.13.0.tgz#03e50f42ff25777f0538cea4348b40700c73a247" - integrity sha512-00NxjSmFS78rUNtcqhKfmf9Ip8YjBMoO3JHY8a6sacYVJ+a+UKHvsi1ksCn7F5elSIZIuVmngSvIebw0LUqvpw== +"@gitlab/ui@20.16.0": + version "20.16.0" + resolved "https://registry.yarnpkg.com/@gitlab/ui/-/ui-20.16.0.tgz#45bad5500e6f47bc4bebb95a17b446f254f9ee21" + integrity sha512-k1+X8RgBXknCu40dEWTjE0Sd9jzGmpoPWPlgmoo28tE1vewhRmCOa8aL0tAcm6VNg5av5ZEqoQEJvgFKxLQB1Q== dependencies: "@babel/standalone" "^7.0.0" "@gitlab/vue-toasted" "^1.3.0" |