diff options
116 files changed, 2505 insertions, 424 deletions
diff --git a/app/assets/javascripts/content_editor/extensions/blockquote.js b/app/assets/javascripts/content_editor/extensions/blockquote.js index f9fb31cae1d..4512ead44bc 100644 --- a/app/assets/javascripts/content_editor/extensions/blockquote.js +++ b/app/assets/javascripts/content_editor/extensions/blockquote.js @@ -18,9 +18,7 @@ export default Blockquote.extend({ (p) => p.nodeName.toLowerCase() === 'blockquote', ); - return { - multiline: source && !source.startsWith('>') && !parentsIncludeBlockquote, - }; + return source && !source.startsWith('>') && !parentsIncludeBlockquote; }, }, }; diff --git a/app/assets/javascripts/content_editor/extensions/bullet_list.js b/app/assets/javascripts/content_editor/extensions/bullet_list.js index 941a04daf98..8d0faf7a9fe 100644 --- a/app/assets/javascripts/content_editor/extensions/bullet_list.js +++ b/app/assets/javascripts/content_editor/extensions/bullet_list.js @@ -11,7 +11,7 @@ export default BulletList.extend({ parseHTML(element) { const bullet = getMarkdownSource(element)?.charAt(0); - return { bullet: '*+-'.includes(bullet) ? bullet : '*' }; + return '*+-'.includes(bullet) ? bullet : '*'; }, }, }; diff --git a/app/assets/javascripts/content_editor/extensions/code_block_highlight.js b/app/assets/javascripts/content_editor/extensions/code_block_highlight.js index c6d32fb8547..25f5837d2a6 100644 --- a/app/assets/javascripts/content_editor/extensions/code_block_highlight.js +++ b/app/assets/javascripts/content_editor/extensions/code_block_highlight.js @@ -8,11 +8,7 @@ export default CodeBlockLowlight.extend({ return { language: { default: null, - parseHTML: (element) => { - return { - language: extractLanguage(element), - }; - }, + parseHTML: (element) => extractLanguage(element), }, class: { default: 'code highlight js-syntax-highlight', diff --git a/app/assets/javascripts/content_editor/extensions/description_item.js b/app/assets/javascripts/content_editor/extensions/description_item.js index 018a6d8769b..957fdede27b 100644 --- a/app/assets/javascripts/content_editor/extensions/description_item.js +++ b/app/assets/javascripts/content_editor/extensions/description_item.js @@ -9,9 +9,7 @@ export default Node.create({ return { isTerm: { default: true, - parseHTML: (element) => ({ - isTerm: element.tagName.toLowerCase() === 'dt', - }), + parseHTML: (element) => element.tagName.toLowerCase() === 'dt', }, }; }, diff --git a/app/assets/javascripts/content_editor/extensions/emoji.js b/app/assets/javascripts/content_editor/extensions/emoji.js index d88b9f92215..de608c3aaa2 100644 --- a/app/assets/javascripts/content_editor/extensions/emoji.js +++ b/app/assets/javascripts/content_editor/extensions/emoji.js @@ -17,30 +17,18 @@ export default Node.create({ return { moji: { default: null, - parseHTML: (element) => { - return { - moji: element.textContent, - }; - }, + parseHTML: (element) => element.textContent, }, name: { default: null, - parseHTML: (element) => { - return { - name: element.dataset.name, - }; - }, + parseHTML: (element) => element.dataset.name, }, title: { default: null, }, unicodeVersion: { default: '6.0', - parseHTML: (element) => { - return { - unicodeVersion: element.dataset.unicodeVersion, - }; - }, + parseHTML: (element) => element.dataset.unicodeVersion, }, }; }, diff --git a/app/assets/javascripts/content_editor/extensions/html_marks.js b/app/assets/javascripts/content_editor/extensions/html_marks.js index e312775e75c..54adb9efa0c 100644 --- a/app/assets/javascripts/content_editor/extensions/html_marks.js +++ b/app/assets/javascripts/content_editor/extensions/html_marks.js @@ -44,7 +44,7 @@ export default marks.map((name) => ...acc, [attr]: { default: null, - parseHTML: (element) => ({ [attr]: element.getAttribute(attr) }), + parseHTML: (element) => element.getAttribute(attr), }, }), {}, diff --git a/app/assets/javascripts/content_editor/extensions/image.js b/app/assets/javascripts/content_editor/extensions/image.js index afd23b55a63..837fab0585f 100644 --- a/app/assets/javascripts/content_editor/extensions/image.js +++ b/app/assets/javascripts/content_editor/extensions/image.js @@ -28,27 +28,19 @@ export default Image.extend({ parseHTML: (element) => { const img = resolveImageEl(element); - return { - src: img.dataset.src || img.getAttribute('src'), - }; + return img.dataset.src || img.getAttribute('src'); }, }, canonicalSrc: { default: null, - parseHTML: (element) => { - return { - canonicalSrc: element.dataset.canonicalSrc, - }; - }, + parseHTML: (element) => element.dataset.canonicalSrc, }, alt: { default: null, parseHTML: (element) => { const img = resolveImageEl(element); - return { - alt: img.getAttribute('alt'), - }; + return img.getAttribute('alt'); }, }, title: { @@ -56,9 +48,7 @@ export default Image.extend({ parseHTML: (element) => { const img = resolveImageEl(element); - return { - title: img.getAttribute('title'), - }; + return img.getAttribute('title'); }, }, }; diff --git a/app/assets/javascripts/content_editor/extensions/inline_diff.js b/app/assets/javascripts/content_editor/extensions/inline_diff.js index 9471d324764..3bd328958df 100644 --- a/app/assets/javascripts/content_editor/extensions/inline_diff.js +++ b/app/assets/javascripts/content_editor/extensions/inline_diff.js @@ -14,11 +14,7 @@ export default Mark.create({ return { type: { default: 'addition', - parseHTML: (element) => { - return { - type: element.classList.contains('deletion') ? 'deletion' : 'addition', - }; - }, + parseHTML: (element) => (element.classList.contains('deletion') ? 'deletion' : 'addition'), }, }; }, diff --git a/app/assets/javascripts/content_editor/extensions/link.js b/app/assets/javascripts/content_editor/extensions/link.js index e51e6832972..fc0f38e6935 100644 --- a/app/assets/javascripts/content_editor/extensions/link.js +++ b/app/assets/javascripts/content_editor/extensions/link.js @@ -36,27 +36,15 @@ export default Link.extend({ ...this.parent?.(), href: { default: null, - parseHTML: (element) => { - return { - href: element.getAttribute('href'), - }; - }, + parseHTML: (element) => element.getAttribute('href'), }, title: { title: null, - parseHTML: (element) => { - return { - title: element.getAttribute('title'), - }; - }, + parseHTML: (element) => element.getAttribute('title'), }, canonicalSrc: { default: null, - parseHTML: (element) => { - return { - canonicalSrc: element.dataset.canonicalSrc, - }; - }, + parseHTML: (element) => element.dataset.canonicalSrc, }, }; }, diff --git a/app/assets/javascripts/content_editor/extensions/ordered_list.js b/app/assets/javascripts/content_editor/extensions/ordered_list.js index ccbb29167d3..57d5bd6ebf8 100644 --- a/app/assets/javascripts/content_editor/extensions/ordered_list.js +++ b/app/assets/javascripts/content_editor/extensions/ordered_list.js @@ -8,9 +8,7 @@ export default OrderedList.extend({ parens: { default: false, - parseHTML: (element) => ({ - parens: /^[0-9]+\)/.test(getMarkdownSource(element)), - }), + parseHTML: (element) => /^[0-9]+\)/.test(getMarkdownSource(element)), }, }; }, diff --git a/app/assets/javascripts/content_editor/extensions/playable.js b/app/assets/javascripts/content_editor/extensions/playable.js index e9d9ec8905a..0062bc563db 100644 --- a/app/assets/javascripts/content_editor/extensions/playable.js +++ b/app/assets/javascripts/content_editor/extensions/playable.js @@ -16,9 +16,7 @@ export default Node.create({ parseHTML: (element) => { const playable = queryPlayableElement(element, this.options.mediaType); - return { - src: playable.src, - }; + return playable.src; }, }, canonicalSrc: { @@ -26,9 +24,7 @@ export default Node.create({ parseHTML: (element) => { const playable = queryPlayableElement(element, this.options.mediaType); - return { - canonicalSrc: playable.dataset.canonicalSrc, - }; + return playable.dataset.canonicalSrc; }, }, alt: { @@ -36,9 +32,7 @@ export default Node.create({ parseHTML: (element) => { const playable = queryPlayableElement(element, this.options.mediaType); - return { - alt: playable.dataset.title, - }; + return playable.dataset.title; }, }, }; diff --git a/app/assets/javascripts/content_editor/extensions/reference.js b/app/assets/javascripts/content_editor/extensions/reference.js index fa5429ae0f4..5e459e65de2 100644 --- a/app/assets/javascripts/content_editor/extensions/reference.js +++ b/app/assets/javascripts/content_editor/extensions/reference.js @@ -19,43 +19,23 @@ export default Node.create({ return { className: { default: null, - parseHTML: (element) => { - return { - className: getAnchor(element).className, - }; - }, + parseHTML: (element) => getAnchor(element).className, }, referenceType: { default: null, - parseHTML: (element) => { - return { - referenceType: getAnchor(element).dataset.referenceType, - }; - }, + parseHTML: (element) => getAnchor(element).dataset.referenceType, }, originalText: { default: null, - parseHTML: (element) => { - return { - originalText: getAnchor(element).dataset.original, - }; - }, + parseHTML: (element) => getAnchor(element).dataset.original, }, href: { default: null, - parseHTML: (element) => { - return { - href: getAnchor(element).getAttribute('href'), - }; - }, + parseHTML: (element) => getAnchor(element).getAttribute('href'), }, text: { default: null, - parseHTML: (element) => { - return { - text: getAnchor(element).textContent, - }; - }, + parseHTML: (element) => getAnchor(element).textContent, }, }; }, diff --git a/app/assets/javascripts/content_editor/extensions/task_item.js b/app/assets/javascripts/content_editor/extensions/task_item.js index d586478358d..9b050edcb28 100644 --- a/app/assets/javascripts/content_editor/extensions/task_item.js +++ b/app/assets/javascripts/content_editor/extensions/task_item.js @@ -13,7 +13,8 @@ export default TaskItem.extend({ default: false, parseHTML: (element) => { const checkbox = element.querySelector('input[type=checkbox].task-list-item-checkbox'); - return { checked: checkbox?.checked }; + + return checkbox?.checked; }, renderHTML: (attributes) => ({ 'data-checked': attributes.checked, diff --git a/app/assets/javascripts/content_editor/extensions/task_list.js b/app/assets/javascripts/content_editor/extensions/task_list.js index 821f578d9fc..72c6e020102 100644 --- a/app/assets/javascripts/content_editor/extensions/task_list.js +++ b/app/assets/javascripts/content_editor/extensions/task_list.js @@ -8,25 +8,17 @@ export default TaskList.extend({ return { numeric: { default: false, - parseHTML: (element) => ({ - numeric: element.tagName.toLowerCase() === 'ol', - }), + parseHTML: (element) => element.tagName.toLowerCase() === 'ol', }, - start: { default: 1, - parseHTML: (element) => ({ - start: element.hasAttribute('start') - ? parseInt(element.getAttribute('start') || '', 10) - : 1, - }), + parseHTML: (element) => + element.hasAttribute('start') ? parseInt(element.getAttribute('start') || '', 10) : 1, }, parens: { default: false, - parseHTML: (element) => ({ - parens: /^[0-9]+\)/.test(getMarkdownSource(element)), - }), + parseHTML: (element) => /^[0-9]+\)/.test(getMarkdownSource(element)), }, }; }, diff --git a/app/assets/javascripts/packages_and_registries/package_registry/components/list/package_search.vue b/app/assets/javascripts/packages_and_registries/package_registry/components/list/package_search.vue new file mode 100644 index 00000000000..280d292ce0b --- /dev/null +++ b/app/assets/javascripts/packages_and_registries/package_registry/components/list/package_search.vue @@ -0,0 +1,57 @@ +<script> +import { mapState, mapActions } from 'vuex'; +import { s__ } from '~/locale'; +import { sortableFields } from '~/packages/list/utils'; +import { OPERATOR_IS_ONLY } from '~/vue_shared/components/filtered_search_bar/constants'; +import RegistrySearch from '~/vue_shared/components/registry/registry_search.vue'; +import UrlSync from '~/vue_shared/components/url_sync.vue'; +import PackageTypeToken from './tokens/package_type_token.vue'; + +export default { + tokens: [ + { + type: 'type', + icon: 'package', + title: s__('PackageRegistry|Type'), + unique: true, + token: PackageTypeToken, + operators: OPERATOR_IS_ONLY, + }, + ], + components: { RegistrySearch, UrlSync }, + computed: { + ...mapState({ + isGroupPage: (state) => state.config.isGroupPage, + sorting: (state) => state.sorting, + filter: (state) => state.filter, + }), + sortableFields() { + return sortableFields(this.isGroupPage); + }, + }, + methods: { + ...mapActions(['setSorting', 'setFilter']), + updateSorting(newValue) { + this.setSorting(newValue); + this.$emit('update'); + }, + }, +}; +</script> + +<template> + <url-sync> + <template #default="{ updateQuery }"> + <registry-search + :filter="filter" + :sorting="sorting" + :tokens="$options.tokens" + :sortable-fields="sortableFields" + @sorting:changed="updateSorting" + @filter:changed="setFilter" + @filter:submit="$emit('update')" + @query:changed="updateQuery" + /> + </template> + </url-sync> +</template> diff --git a/app/assets/javascripts/packages_and_registries/package_registry/components/list/package_title.vue b/app/assets/javascripts/packages_and_registries/package_registry/components/list/package_title.vue new file mode 100644 index 00000000000..6e00a48586e --- /dev/null +++ b/app/assets/javascripts/packages_and_registries/package_registry/components/list/package_title.vue @@ -0,0 +1,47 @@ +<script> +import { n__ } from '~/locale'; +import { LIST_INTRO_TEXT, LIST_TITLE_TEXT } from '~/packages/list/constants'; +import MetadataItem from '~/vue_shared/components/registry/metadata_item.vue'; +import TitleArea from '~/vue_shared/components/registry/title_area.vue'; + +export default { + name: 'PackageTitle', + components: { + TitleArea, + MetadataItem, + }, + props: { + count: { + type: Number, + required: false, + default: null, + }, + helpUrl: { + type: String, + required: true, + }, + }, + computed: { + showPackageCount() { + return Number.isInteger(this.count); + }, + packageAmountText() { + return n__(`%d Package`, `%d Packages`, this.count); + }, + infoMessages() { + return [{ text: LIST_INTRO_TEXT, link: this.helpUrl }]; + }, + }, + i18n: { + LIST_TITLE_TEXT, + }, +}; +</script> + +<template> + <title-area :title="$options.i18n.LIST_TITLE_TEXT" :info-messages="infoMessages"> + <template #metadata-amount> + <metadata-item v-if="showPackageCount" icon="package" :text="packageAmountText" /> + </template> + </title-area> +</template> diff --git a/app/assets/javascripts/packages_and_registries/package_registry/components/list/packages_list.vue b/app/assets/javascripts/packages_and_registries/package_registry/components/list/packages_list.vue new file mode 100644 index 00000000000..25bac687dbf --- /dev/null +++ b/app/assets/javascripts/packages_and_registries/package_registry/components/list/packages_list.vue @@ -0,0 +1,129 @@ +<script> +import { GlPagination, GlModal, GlSprintf } from '@gitlab/ui'; +import { mapState, mapGetters } from 'vuex'; +import { s__ } from '~/locale'; +import PackagesListRow from '~/packages/shared/components/package_list_row.vue'; +import PackagesListLoader from '~/packages/shared/components/packages_list_loader.vue'; +import { TrackingActions } from '~/packages/shared/constants'; +import { packageTypeToTrackCategory } from '~/packages/shared/utils'; +import Tracking from '~/tracking'; + +export default { + components: { + GlPagination, + GlModal, + GlSprintf, + PackagesListLoader, + PackagesListRow, + }, + mixins: [Tracking.mixin()], + data() { + return { + itemToBeDeleted: null, + }; + }, + computed: { + ...mapState({ + perPage: (state) => state.pagination.perPage, + totalItems: (state) => state.pagination.total, + page: (state) => state.pagination.page, + isGroupPage: (state) => state.config.isGroupPage, + isLoading: 'isLoading', + }), + ...mapGetters({ list: 'getList' }), + currentPage: { + get() { + return this.page; + }, + set(value) { + this.$emit('page:changed', value); + }, + }, + isListEmpty() { + return !this.list || this.list.length === 0; + }, + modalAction() { + return s__('PackageRegistry|Delete package'); + }, + deletePackageName() { + return this.itemToBeDeleted?.name ?? ''; + }, + tracking() { + const category = this.itemToBeDeleted + ? packageTypeToTrackCategory(this.itemToBeDeleted.package_type) + : undefined; + return { + category, + }; + }, + }, + methods: { + setItemToBeDeleted(item) { + this.itemToBeDeleted = { ...item }; + this.track(TrackingActions.REQUEST_DELETE_PACKAGE); + this.$refs.packageListDeleteModal.show(); + }, + deleteItemConfirmation() { + this.$emit('package:delete', this.itemToBeDeleted); + this.track(TrackingActions.DELETE_PACKAGE); + this.itemToBeDeleted = null; + }, + deleteItemCanceled() { + this.track(TrackingActions.CANCEL_DELETE_PACKAGE); + this.itemToBeDeleted = null; + }, + }, + i18n: { + deleteModalContent: s__( + 'PackageRegistry|You are about to delete %{name}, this operation is irreversible, are you sure?', + ), + }, +}; +</script> + +<template> + <div class="gl-display-flex gl-flex-direction-column"> + <slot v-if="isListEmpty && !isLoading" name="empty-state"></slot> + + <div v-else-if="isLoading"> + <packages-list-loader /> + </div> + + <template v-else> + <div data-qa-selector="packages-table"> + <packages-list-row + v-for="packageEntity in list" + :key="packageEntity.id" + :package-entity="packageEntity" + :package-link="packageEntity._links.web_path" + :is-group="isGroupPage" + @packageToDelete="setItemToBeDeleted" + /> + </div> + + <gl-pagination + v-model="currentPage" + :per-page="perPage" + :total-items="totalItems" + align="center" + class="gl-w-full gl-mt-3" + /> + + <gl-modal + ref="packageListDeleteModal" + modal-id="confirm-delete-pacakge" + ok-variant="danger" + @ok="deleteItemConfirmation" + @cancel="deleteItemCanceled" + > + <template #modal-title>{{ modalAction }}</template> + <template #modal-ok>{{ modalAction }}</template> + <gl-sprintf :message="$options.i18n.deleteModalContent"> + <template #name> + <strong>{{ deletePackageName }}</strong> + </template> + </gl-sprintf> + </gl-modal> + </template> + </div> +</template> diff --git a/app/assets/javascripts/packages_and_registries/package_registry/components/list/packages_list_app.vue b/app/assets/javascripts/packages_and_registries/package_registry/components/list/packages_list_app.vue new file mode 100644 index 00000000000..75fbdb80192 --- /dev/null +++ b/app/assets/javascripts/packages_and_registries/package_registry/components/list/packages_list_app.vue @@ -0,0 +1,132 @@ +<script> +import { GlEmptyState, GlLink, GlSprintf } from '@gitlab/ui'; +import { mapActions, mapState } from 'vuex'; +import createFlash from '~/flash'; +import { historyReplaceState } from '~/lib/utils/common_utils'; +import { s__ } from '~/locale'; +import { DELETE_PACKAGE_SUCCESS_MESSAGE } from '~/packages/list/constants'; +import { SHOW_DELETE_SUCCESS_ALERT } from '~/packages/shared/constants'; +import { FILTERED_SEARCH_TERM } from '~/packages_and_registries/shared/constants'; +import { getQueryParams, extractFilterAndSorting } from '~/packages_and_registries/shared/utils'; +import PackageList from './packages_list.vue'; + +export default { + components: { + GlEmptyState, + GlLink, + GlSprintf, + PackageList, + PackageTitle: () => + import(/* webpackChunkName: 'package_registry_components' */ './package_title.vue'), + PackageSearch: () => + import(/* webpackChunkName: 'package_registry_components' */ './package_search.vue'), + InfrastructureTitle: () => + import( + /* webpackChunkName: 'infrastructure_registry_components' */ '~/packages_and_registries/infrastructure_registry/components/infrastructure_title.vue' + ), + InfrastructureSearch: () => + import( + /* webpackChunkName: 'infrastructure_registry_components' */ '~/packages_and_registries/infrastructure_registry/components/infrastructure_search.vue' + ), + }, + inject: { + titleComponent: { + from: 'titleComponent', + default: 'PackageTitle', + }, + searchComponent: { + from: 'searchComponent', + default: 'PackageSearch', + }, + emptyPageTitle: { + from: 'emptyPageTitle', + default: s__('PackageRegistry|There are no packages yet'), + }, + noResultsText: { + from: 'noResultsText', + default: s__( + 'PackageRegistry|Learn how to %{noPackagesLinkStart}publish and share your packages%{noPackagesLinkEnd} with GitLab.', + ), + }, + }, + computed: { + ...mapState({ + emptyListIllustration: (state) => state.config.emptyListIllustration, + emptyListHelpUrl: (state) => state.config.emptyListHelpUrl, + filter: (state) => state.filter, + selectedType: (state) => state.selectedType, + packageHelpUrl: (state) => state.config.packageHelpUrl, + packagesCount: (state) => state.pagination?.total, + }), + emptySearch() { + return ( + this.filter.filter((f) => f.type !== FILTERED_SEARCH_TERM || f.value?.data).length === 0 + ); + }, + + emptyStateTitle() { + return this.emptySearch + ? this.emptyPageTitle + : s__('PackageRegistry|Sorry, your filter produced no results'); + }, + }, + mounted() { + const queryParams = getQueryParams(window.document.location.search); + const { sorting, filters } = extractFilterAndSorting(queryParams); + this.setSorting(sorting); + this.setFilter(filters); + this.requestPackagesList(); + this.checkDeleteAlert(); + }, + methods: { + ...mapActions([ + 'requestPackagesList', + 'requestDeletePackage', + 'setSelectedType', + 'setSorting', + 'setFilter', + ]), + onPageChanged(page) { + return this.requestPackagesList({ page }); + }, + onPackageDeleteRequest(item) { + return this.requestDeletePackage(item); + }, + checkDeleteAlert() { + const urlParams = new URLSearchParams(window.location.search); + const showAlert = urlParams.get(SHOW_DELETE_SUCCESS_ALERT); + if (showAlert) { + // to be refactored to use gl-alert + createFlash({ message: DELETE_PACKAGE_SUCCESS_MESSAGE, type: 'notice' }); + const cleanUrl = window.location.href.split('?')[0]; + historyReplaceState(cleanUrl); + } + }, + }, + i18n: { + widenFilters: s__('PackageRegistry|To widen your search, change or remove the filters above.'), + }, +}; +</script> + +<template> + <div> + <component :is="titleComponent" :help-url="packageHelpUrl" :count="packagesCount" /> + <component :is="searchComponent" @update="requestPackagesList" /> + + <package-list @page:changed="onPageChanged" @package:delete="onPackageDeleteRequest"> + <template #empty-state> + <gl-empty-state :title="emptyStateTitle" :svg-path="emptyListIllustration"> + <template #description> + <gl-sprintf v-if="!emptySearch" :message="$options.i18n.widenFilters" /> + <gl-sprintf v-else :message="noResultsText"> + <template #noPackagesLink="{ content }"> + <gl-link :href="emptyListHelpUrl" target="_blank">{{ content }}</gl-link> + </template> + </gl-sprintf> + </template> + </gl-empty-state> + </template> + </package-list> + </div> +</template> diff --git a/app/assets/javascripts/packages_and_registries/package_registry/components/list/tokens/package_type_token.vue b/app/assets/javascripts/packages_and_registries/package_registry/components/list/tokens/package_type_token.vue new file mode 100644 index 00000000000..529a7893dfc --- /dev/null +++ b/app/assets/javascripts/packages_and_registries/package_registry/components/list/tokens/package_type_token.vue @@ -0,0 +1,26 @@ +<script> +import { GlFilteredSearchToken, GlFilteredSearchSuggestion } from '@gitlab/ui'; +import { PACKAGE_TYPES } from '~/packages/list/constants'; + +export default { + components: { + GlFilteredSearchToken, + GlFilteredSearchSuggestion, + }, + PACKAGE_TYPES, +}; +</script> + +<template> + <gl-filtered-search-token v-bind="{ ...$attrs }" v-on="$listeners"> + <template #suggestions> + <gl-filtered-search-suggestion + v-for="(type, index) in $options.PACKAGE_TYPES" + :key="index" + :value="type.type" + > + {{ type.title }} + </gl-filtered-search-suggestion> + </template> + </gl-filtered-search-token> +</template> diff --git a/app/assets/javascripts/packages_and_registries/package_registry/pages/list.js b/app/assets/javascripts/packages_and_registries/package_registry/pages/list.js new file mode 100644 index 00000000000..1e01b75aabc --- /dev/null +++ b/app/assets/javascripts/packages_and_registries/package_registry/pages/list.js @@ -0,0 +1,16 @@ +import Vue from 'vue'; +import Translate from '~/vue_shared/translate'; +import PackagesListApp from '../components/list/packages_list_app.vue'; + +Vue.use(Translate); + +export default () => { + const el = document.getElementById('js-vue-packages-list'); + + return new Vue({ + el, + render(createElement) { + return createElement(PackagesListApp); + }, + }); +}; diff --git a/app/assets/javascripts/vue_shared/components/user_popover/user_popover.vue b/app/assets/javascripts/vue_shared/components/user_popover/user_popover.vue index 97ec1140853..74616763f8f 100644 --- a/app/assets/javascripts/vue_shared/components/user_popover/user_popover.vue +++ b/app/assets/javascripts/vue_shared/components/user_popover/user_popover.vue @@ -1,5 +1,12 @@ <script> -import { GlPopover, GlLink, GlSkeletonLoader, GlIcon, GlSafeHtmlDirective } from '@gitlab/ui'; +import { + GlPopover, + GlLink, + GlSkeletonLoader, + GlIcon, + GlSafeHtmlDirective, + GlSprintf, +} from '@gitlab/ui'; import UserNameWithStatus from '~/sidebar/components/assignees/user_name_with_status.vue'; import { glEmojiTag } from '../../../emoji'; import UserAvatarImage from '../user_avatar/user_avatar_image.vue'; @@ -16,6 +23,7 @@ export default { GlSkeletonLoader, UserAvatarImage, UserNameWithStatus, + GlSprintf, }, directives: { SafeHtml: GlSafeHtmlDirective, @@ -103,7 +111,9 @@ export default { <div v-if="user.bot" class="gl-text-blue-500"> <gl-icon name="question" /> <gl-link data-testid="user-popover-bot-docs-link" :href="user.websiteUrl"> - {{ sprintf(__('Learn more about %{username}'), { username: user.name }) }} + <gl-sprintf :message="__('Learn more about %{username}')"> + <template #username>{{ user.name }}</template> + </gl-sprintf> </gl-link> </div> </template> diff --git a/app/assets/javascripts/vue_shared/security_reports/store/modules/sast/state.js b/app/assets/javascripts/vue_shared/security_reports/store/modules/sast/state.js index e860e3af924..c1b3f546431 100644 --- a/app/assets/javascripts/vue_shared/security_reports/store/modules/sast/state.js +++ b/app/assets/javascripts/vue_shared/security_reports/store/modules/sast/state.js @@ -1,7 +1,5 @@ export default () => ({ paths: { - head: null, - base: null, diffEndpoint: null, }, diff --git a/app/assets/javascripts/vue_shared/security_reports/store/modules/secret_detection/state.js b/app/assets/javascripts/vue_shared/security_reports/store/modules/secret_detection/state.js index e860e3af924..c1b3f546431 100644 --- a/app/assets/javascripts/vue_shared/security_reports/store/modules/secret_detection/state.js +++ b/app/assets/javascripts/vue_shared/security_reports/store/modules/secret_detection/state.js @@ -1,7 +1,5 @@ export default () => ({ paths: { - head: null, - base: null, diffEndpoint: null, }, diff --git a/app/assets/stylesheets/page_bundles/escalation_policies.scss b/app/assets/stylesheets/page_bundles/escalation_policies.scss index f188dde1183..6f3873fea0c 100644 --- a/app/assets/stylesheets/page_bundles/escalation_policies.scss +++ b/app/assets/stylesheets/page_bundles/escalation_policies.scss @@ -16,9 +16,6 @@ $stroke-size: 1px; .right-arrow { @include gl-relative; - @include gl-mx-5; - @include gl-display-inline-block; - @include gl-vertical-align-middle; height: $stroke-size; background-color: var(--gray-900, $gray-900); min-width: $gl-spacing-scale-7; @@ -27,7 +24,6 @@ $stroke-size: 1px; @include gl-absolute; top: -2*$stroke-size; left: calc(100% - #{5*$stroke-size}); - @include gl-display-inline-block; @include gl-p-1; @include gl-border-solid; border-width: 0 $stroke-size $stroke-size 0; @@ -35,3 +31,24 @@ $stroke-size: 1px; transform: rotate(-45deg); } } + +.escalation-rule-row { + @media (max-width: $breakpoint-lg) { + @include gl-flex-wrap; + } +} + +.rule-condition { + @media (min-width: $breakpoint-lg) { + flex-basis: 25%; + flex-shrink: 0; + } + + @media (max-width: $breakpoint-lg) { + @include gl-w-full; + } +} + +.rule-action { + min-width: 0; +} diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index d5b771886f6..f885ff9b45b 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -231,7 +231,7 @@ class Projects::IssuesController < Projects::ApplicationController IssuableExportCsvWorker.perform_async(:issue, current_user.id, project.id, finder_options.to_h) # rubocop:disable CodeReuse/Worker index_path = project_issues_path(project) - message = _('Your CSV export has started. It will be emailed to %{email} when complete.') % { email: current_user.notification_email } + message = _('Your CSV export has started. It will be emailed to %{email} when complete.') % { email: current_user.notification_email_or_default } redirect_to(index_path, notice: message) end diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 1129f3ee7e1..cb68aaf4583 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -378,7 +378,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo IssuableExportCsvWorker.perform_async(:merge_request, current_user.id, project.id, finder_options.to_h) # rubocop:disable CodeReuse/Worker index_path = project_merge_requests_path(project) - message = _('Your CSV export has started. It will be emailed to %{email} when complete.') % { email: current_user.notification_email } + message = _('Your CSV export has started. It will be emailed to %{email} when complete.') % { email: current_user.notification_email_or_default } redirect_to(index_path, notice: message) end diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index 8897d73613c..40e86b4623c 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -221,7 +221,7 @@ module IssuesHelper can_bulk_update: can?(current_user, :admin_issue, project).to_s, can_edit: can?(current_user, :admin_project, project).to_s, can_import_issues: can?(current_user, :import_issues, @project).to_s, - email: current_user&.notification_email, + email: current_user&.notification_email_or_default, emails_help_page_path: help_page_path('development/emails', anchor: 'email-namespace'), export_csv_path: export_csv_project_issues_path(project), has_any_issues: project_issues(project).exists?.to_s, diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index f30223f6f1e..d7f1cd505e9 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -435,7 +435,7 @@ module ProjectsHelper def git_user_email if current_user - current_user.commit_email + current_user.commit_email_or_default else "your@email.com" end diff --git a/app/mailers/emails/admin_notification.rb b/app/mailers/emails/admin_notification.rb index f4540ef81a5..e11f06d8fc9 100644 --- a/app/mailers/emails/admin_notification.rb +++ b/app/mailers/emails/admin_notification.rb @@ -4,7 +4,7 @@ module Emails module AdminNotification def send_admin_notification(user_id, subject, body) user = User.find(user_id) - email = user.notification_email + email = user.notification_email_or_default @unsubscribe_url = unsubscribe_url(email: Base64.urlsafe_encode64(email)) @body = body mail to: email, subject: subject @@ -12,7 +12,7 @@ module Emails def send_unsubscribed_notification(user_id) user = User.find(user_id) - email = user.notification_email + email = user.notification_email_or_default mail to: email, subject: "Unsubscribed from GitLab administrator notifications" end end diff --git a/app/mailers/emails/profile.rb b/app/mailers/emails/profile.rb index a8affb34f62..592c394bb48 100644 --- a/app/mailers/emails/profile.rb +++ b/app/mailers/emails/profile.rb @@ -6,7 +6,7 @@ module Emails @current_user = @user = User.find(user_id) @target_url = user_url(@user) @token = token - mail(to: @user.notification_email, subject: subject("Account was created for you")) + mail(to: @user.notification_email_or_default, subject: subject("Account was created for you")) end def instance_access_request_email(user, recipient) @@ -14,7 +14,7 @@ module Emails @recipient = recipient profile_email_with_layout( - to: recipient.notification_email, + to: recipient.notification_email_or_default, subject: subject(_("GitLab Account Request"))) end @@ -42,7 +42,7 @@ module Emails @current_user = @user = @key.user @target_url = user_url(@user) - mail(to: @user.notification_email, subject: subject("SSH key was added to your account")) + mail(to: @user.notification_email_or_default, subject: subject("SSH key was added to your account")) end # rubocop: enable CodeReuse/ActiveRecord @@ -54,7 +54,7 @@ module Emails @current_user = @user = @gpg_key.user @target_url = user_url(@user) - mail(to: @user.notification_email, subject: subject("GPG key was added to your account")) + mail(to: @user.notification_email_or_default, subject: subject("GPG key was added to your account")) end # rubocop: enable CodeReuse/ActiveRecord @@ -67,7 +67,7 @@ module Emails @days_to_expire = PersonalAccessToken::DAYS_TO_EXPIRE Gitlab::I18n.with_locale(@user.preferred_language) do - mail(to: @user.notification_email, subject: subject(_("Your personal access tokens will expire in %{days_to_expire} days or less") % { days_to_expire: @days_to_expire })) + mail(to: @user.notification_email_or_default, subject: subject(_("Your personal access tokens will expire in %{days_to_expire} days or less") % { days_to_expire: @days_to_expire })) end end @@ -78,7 +78,7 @@ module Emails @target_url = profile_personal_access_tokens_url Gitlab::I18n.with_locale(@user.preferred_language) do - mail(to: @user.notification_email, subject: subject(_("Your personal access token has expired"))) + mail(to: @user.notification_email_or_default, subject: subject(_("Your personal access token has expired"))) end end @@ -90,7 +90,7 @@ module Emails @target_url = profile_keys_url Gitlab::I18n.with_locale(@user.preferred_language) do - mail(to: @user.notification_email, subject: subject(_("Your SSH key has expired"))) + mail(to: @user.notification_email_or_default, subject: subject(_("Your SSH key has expired"))) end end @@ -102,7 +102,7 @@ module Emails @target_url = profile_keys_url Gitlab::I18n.with_locale(@user.preferred_language) do - mail(to: @user.notification_email, subject: subject(_("Your SSH key is expiring soon."))) + mail(to: @user.notification_email_or_default, subject: subject(_("Your SSH key is expiring soon."))) end end @@ -114,7 +114,7 @@ module Emails Gitlab::I18n.with_locale(@user.preferred_language) do profile_email_with_layout( - to: @user.notification_email, + to: @user.notification_email_or_default, subject: subject(_("%{host} sign-in from new location") % { host: Gitlab.config.gitlab.host })) end end @@ -125,7 +125,7 @@ module Emails @user = user Gitlab::I18n.with_locale(@user.preferred_language) do - mail(to: @user.notification_email, subject: subject(_("Two-factor authentication disabled"))) + mail(to: @user.notification_email_or_default, subject: subject(_("Two-factor authentication disabled"))) end end diff --git a/app/models/ci/job_artifact.rb b/app/models/ci/job_artifact.rb index ad6a7c1c558..ad3e867f9d5 100644 --- a/app/models/ci/job_artifact.rb +++ b/app/models/ci/job_artifact.rb @@ -185,7 +185,6 @@ module Ci scope :order_expired_desc, -> { order(expire_at: :desc) } scope :with_destroy_preloads, -> { includes(project: [:route, :statistics]) } - scope :scoped_project, -> { where('ci_job_artifacts.project_id = projects.id') } scope :for_project, ->(project) { where(project_id: project) } scope :created_in_time_range, ->(from: nil, to: nil) { where(created_at: from..to) } diff --git a/app/models/loose_foreign_keys.rb b/app/models/loose_foreign_keys.rb new file mode 100644 index 00000000000..0f45c0b5568 --- /dev/null +++ b/app/models/loose_foreign_keys.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +module LooseForeignKeys + def self.table_name_prefix + 'loose_foreign_keys_' + end +end diff --git a/app/models/loose_foreign_keys/deleted_record.rb b/app/models/loose_foreign_keys/deleted_record.rb new file mode 100644 index 00000000000..a39d88b2e49 --- /dev/null +++ b/app/models/loose_foreign_keys/deleted_record.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +class LooseForeignKeys::DeletedRecord < ApplicationRecord + extend SuppressCompositePrimaryKeyWarning + include PartitionedTable + + partitioned_by :created_at, strategy: :monthly, retain_for: 3.months, retain_non_empty_partitions: true + + scope :ordered_by_primary_keys, -> { order(:created_at, :deleted_table_name, :deleted_table_primary_key_value) } + + def self.load_batch(batch_size) + ordered_by_primary_keys + .limit(batch_size) + .to_a + end + + # Because the table has composite primary keys, the delete_all or delete methods are not going to work. + # This method implements deletion that benefits from the primary key index, example: + # + # > DELETE + # > FROM "loose_foreign_keys_deleted_records" + # > WHERE (created_at, + # > deleted_table_name, + # > deleted_table_primary_key_value) IN + # > (SELECT created_at::TIMESTAMP WITH TIME ZONE, + # > deleted_table_name, + # > deleted_table_primary_key_value + # > FROM (VALUES (LIST_OF_VALUES)) AS primary_key_values (created_at, deleted_table_name, deleted_table_primary_key_value)) + def self.delete_records(records) + values = records.pluck(:created_at, :deleted_table_name, :deleted_table_primary_key_value) + + primary_keys = connection.primary_keys(table_name).join(', ') + + primary_keys_with_type_cast = [ + Arel.sql('created_at::timestamp with time zone'), + Arel.sql('deleted_table_name'), + Arel.sql('deleted_table_primary_key_value') + ] + + value_list = Arel::Nodes::ValuesList.new(values) + + # (SELECT primary keys FROM VALUES) + inner_query = Arel::SelectManager.new + inner_query.from("#{Arel::Nodes::Grouping.new([value_list]).as('primary_key_values').to_sql} (#{primary_keys})") + inner_query.projections = primary_keys_with_type_cast + + where(Arel::Nodes::Grouping.new([Arel.sql(primary_keys)]).in(inner_query)).delete_all + end +end diff --git a/app/models/namespace.rb b/app/models/namespace.rb index 53375b72e69..0c160cedb4d 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -530,17 +530,27 @@ class Namespace < ApplicationRecord def nesting_level_allowed if ancestors.count > Group::NUMBER_OF_ANCESTORS_ALLOWED - errors.add(:parent_id, 'has too deep level of nesting') + errors.add(:parent_id, _('has too deep level of nesting')) end end def validate_parent_type - return unless has_parent? + unless has_parent? + if project? + errors.add(:parent_id, _('must be set for a project namespace')) + end + + return + end + + if parent.project? + errors.add(:parent_id, _('project namespace cannot be the parent of another namespace')) + end if user? - errors.add(:parent_id, 'a user namespace cannot have a parent') + errors.add(:parent_id, _('cannot not be used for user namespace')) elsif group? - errors.add(:parent_id, 'a group cannot have a user namespace as its parent') if parent.user? + errors.add(:parent_id, _('user namespace cannot be the parent of another namespace')) if parent.user? end end diff --git a/app/models/note.rb b/app/models/note.rb index 5e6b58a54c2..a8f5c305d9b 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -115,7 +115,6 @@ class Note < ApplicationRecord scope :updated_after, ->(time) { where('updated_at > ?', time) } scope :with_updated_at, ->(time) { where(updated_at: time) } scope :with_suggestions, -> { joins(:suggestions) } - scope :inc_author_project, -> { includes(:project, :author) } scope :inc_author, -> { includes(:author) } scope :with_api_entity_associations, -> { preload(:note_diff_file, :author) } scope :inc_relations_for_view, -> do diff --git a/app/models/user.rb b/app/models/user.rb index b0c79ffd0f3..b5f0251f639 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -229,10 +229,9 @@ class User < ApplicationRecord validates :first_name, length: { maximum: 127 } validates :last_name, length: { maximum: 127 } validates :email, confirmation: true - validates :notification_email, presence: true - validates :notification_email, devise_email: true, if: ->(user) { user.notification_email != user.email } + validates :notification_email, devise_email: true, allow_blank: true, if: ->(user) { user.notification_email != user.email } validates :public_email, uniqueness: true, devise_email: true, allow_blank: true - validates :commit_email, devise_email: true, allow_nil: true, if: ->(user) { user.commit_email != user.email } + validates :commit_email, devise_email: true, allow_blank: true, if: ->(user) { user.commit_email != user.email && user.commit_email != Gitlab::PrivateCommitEmail::TOKEN } validates :projects_limit, presence: true, numericality: { greater_than_or_equal_to: 0, less_than_or_equal_to: Gitlab::Database::MAX_INT_VALUE } @@ -384,7 +383,7 @@ class User < ApplicationRecord after_transition any => :deactivated do |user| next unless Gitlab::CurrentSettings.user_deactivation_emails_enabled - NotificationService.new.user_deactivated(user.name, user.notification_email) + NotificationService.new.user_deactivated(user.name, user.notification_email_or_default) end # rubocop: enable CodeReuse/ServiceClass @@ -932,33 +931,18 @@ class User < ApplicationRecord end end - # Define commit_email-related attribute methods explicitly instead of relying - # on ActiveRecord to provide them. Some of the specs use the current state of - # the model code but an older database schema, so we need to guard against the - # possibility of the commit_email column not existing. - - def commit_email - return self.email unless has_attribute?(:commit_email) - - if super == Gitlab::PrivateCommitEmail::TOKEN + def commit_email_or_default + if self.commit_email == Gitlab::PrivateCommitEmail::TOKEN return private_commit_email end # The commit email is the same as the primary email if undefined - super.presence || self.email - end - - def commit_email=(email) - super if has_attribute?(:commit_email) - end - - def commit_email_changed? - has_attribute?(:commit_email) && super + self.commit_email.presence || self.email end - def notification_email + def notification_email_or_default # The notification email is the same as the primary email if undefined - super.presence || self.email + self.notification_email.presence || self.email end def private_commit_email @@ -1640,7 +1624,7 @@ class User < ApplicationRecord def notification_email_for(notification_group) # Return group-specific email address if present, otherwise return global notification email address - notification_group&.notification_email_for(self) || notification_email + notification_group&.notification_email_for(self) || notification_email_or_default end def notification_settings_for(source, inherit: false) @@ -2019,9 +2003,9 @@ class User < ApplicationRecord private def notification_email_verified - return if read_attribute(:notification_email).blank? || temp_oauth_email? + return if notification_email.blank? || temp_oauth_email? - errors.add(:notification_email, _("must be an email you have verified")) unless verified_emails.include?(notification_email) + errors.add(:notification_email, _("must be an email you have verified")) unless verified_emails.include?(notification_email_or_default) end def public_email_verified @@ -2031,9 +2015,9 @@ class User < ApplicationRecord end def commit_email_verified - return if read_attribute(:commit_email).blank? + return if commit_email.blank? - errors.add(:commit_email, _("must be an email you have verified")) unless verified_emails.include?(commit_email) + errors.add(:commit_email, _("must be an email you have verified")) unless verified_emails.include?(commit_email_or_default) end def callout_dismissed?(callout, ignore_dismissal_earlier_than) diff --git a/app/views/profiles/_email_settings.html.haml b/app/views/profiles/_email_settings.html.haml index 95306633556..bc678c2c429 100644 --- a/app/views/profiles/_email_settings.html.haml +++ b/app/views/profiles/_email_settings.html.haml @@ -11,6 +11,6 @@ - commit_email_link_url = help_page_path('user/profile/index', anchor: 'change-the-email-displayed-on-your-commits', target: '_blank') - commit_email_link_start = '<a href="%{url}">'.html_safe % { url: commit_email_link_url } - commit_email_docs_link = s_('Profiles|This email will be used for web based operations, such as edits and merges. %{commit_email_link_start}Learn more%{commit_email_link_end}').html_safe % { commit_email_link_start: commit_email_link_start, commit_email_link_end: '</a>'.html_safe } -= form.select :commit_email, options_for_select(commit_email_select_options(@user), selected: @user.read_attribute(:commit_email)), += form.select :commit_email, options_for_select(commit_email_select_options(@user), selected: @user.commit_email), { help: commit_email_docs_link }, control_class: 'select2 input-lg', disabled: email_change_disabled diff --git a/app/views/profiles/emails/index.html.haml b/app/views/profiles/emails/index.html.haml index c14efa99555..35bdfbb1c29 100644 --- a/app/views/profiles/emails/index.html.haml +++ b/app/views/profiles/emails/index.html.haml @@ -38,21 +38,21 @@ = render partial: 'shared/email_with_badge', locals: { email: @primary_email, verified: current_user.confirmed? } %span.float-right %span.badge.badge-muted.badge-pill.gl-badge.badge-success= s_('Profiles|Primary email') - - if @primary_email === current_user.commit_email + - if @primary_email === current_user.commit_email_or_default %span.badge.badge-muted.badge-pill.gl-badge.badge-info= s_('Profiles|Commit email') - if @primary_email === current_user.public_email %span.badge.badge-muted.badge-pill.gl-badge.badge-info= s_('Profiles|Public email') - - if @primary_email === current_user.notification_email + - if @primary_email === current_user.notification_email_or_default %span.badge.badge-muted.badge-pill.gl-badge.badge-info= s_('Profiles|Default notification email') - @emails.each do |email| %li{ data: { qa_selector: 'email_row_content' } } = render partial: 'shared/email_with_badge', locals: { email: email.email, verified: email.confirmed? } %span.float-right - - if email.email === current_user.commit_email + - if email.email === current_user.commit_email_or_default %span.badge.badge-muted.badge-pill.gl-badge.badge-info= s_('Profiles|Commit email') - if email.email === current_user.public_email %span.badge.badge-muted.badge-pill.gl-badge.badge-info= s_('Profiles|Public email') - - if email.email === current_user.notification_email + - if email.email === current_user.notification_email_or_default %span.badge.badge-muted.badge-pill.gl-badge.badge-info= s_('Profiles|Notification email') - unless email.confirmed? - confirm_title = "#{email.confirmation_sent_at ? _('Resend confirmation email') : _('Send confirmation email')}" diff --git a/app/views/profiles/notifications/_email_settings.html.haml b/app/views/profiles/notifications/_email_settings.html.haml index e1e4b2bf2d6..f2121199412 100644 --- a/app/views/profiles/notifications/_email_settings.html.haml +++ b/app/views/profiles/notifications/_email_settings.html.haml @@ -1,7 +1,7 @@ - form = local_assigns.fetch(:form) .form-group = form.label :notification_email, class: "label-bold" - = form.select :notification_email, @user.public_verified_emails, { include_blank: _('Use primary email (%{email})') % { email: @user.email }, selected: @user.read_attribute(:notification_email) }, class: "select2", disabled: local_assigns.fetch(:email_change_disabled, nil) + = form.select :notification_email, @user.public_verified_emails, { include_blank: _('Use primary email (%{email})') % { email: @user.email }, selected: @user.notification_email }, class: "select2", disabled: local_assigns.fetch(:email_change_disabled, nil) .help-block = local_assigns.fetch(:help_text, nil) .form-group diff --git a/app/views/projects/issues/_nav_btns.html.haml b/app/views/projects/issues/_nav_btns.html.haml index 1289f7aa0c4..0d69f6f69aa 100644 --- a/app/views/projects/issues/_nav_btns.html.haml +++ b/app/views/projects/issues/_nav_btns.html.haml @@ -3,7 +3,7 @@ - show_export_button = local_assigns.fetch(:show_export_button, true) - issuable_type = 'issues' - can_edit = can?(current_user, :admin_project, @project) -- notification_email = @current_user.present? ? @current_user.notification_email : nil +- notification_email = @current_user.present? ? @current_user.notification_email_or_default : nil .nav-controls.issues-nav-controls - if show_feed_buttons diff --git a/app/views/projects/merge_requests/_nav_btns.html.haml b/app/views/projects/merge_requests/_nav_btns.html.haml index 84edf62b300..b34cf23634c 100644 --- a/app/views/projects/merge_requests/_nav_btns.html.haml +++ b/app/views/projects/merge_requests/_nav_btns.html.haml @@ -1,5 +1,5 @@ - issuable_type = 'merge-requests' -- notification_email = @current_user.present? ? @current_user.notification_email : nil +- notification_email = @current_user.present? ? @current_user.notification_email_or_default : nil = render 'shared/issuable/feed_buttons', show_calendar_button: false .js-csv-import-export-buttons{ data: { show_export_button: "true", issuable_type: issuable_type, issuable_count: issuables_count_for_state(issuable_type.to_sym, params[:state]), email: notification_email, export_csv_path: export_csv_project_merge_requests_path(@project, request.query_parameters), container_class: 'gl-mr-3' } } diff --git a/app/workers/concerns/application_worker.rb b/app/workers/concerns/application_worker.rb index 6cc6c30c5e9..3399a4f9b57 100644 --- a/app/workers/concerns/application_worker.rb +++ b/app/workers/concerns/application_worker.rb @@ -58,10 +58,7 @@ module ApplicationWorker Gitlab::SidekiqConfig::WorkerRouter.queue_name_from_worker_name(self) end - override :validate_worker_attributes! def validate_worker_attributes! - super - # Since the delayed data_consistency will use sidekiq built in retry mechanism, it is required that this mechanism # is not disabled. if retry_disabled? && get_data_consistency == :delayed @@ -81,6 +78,13 @@ module ApplicationWorker end end + override :data_consistency + def data_consistency(data_consistency, feature_flag: nil) + super + + validate_worker_attributes! + end + def perform_async(*args) # Worker execution for workers with data_consistency set to :delayed or :sticky # will be delayed to give replication enough time to complete diff --git a/app/workers/concerns/worker_attributes.rb b/app/workers/concerns/worker_attributes.rb index 806fce38636..eebea30655c 100644 --- a/app/workers/concerns/worker_attributes.rb +++ b/app/workers/concerns/worker_attributes.rb @@ -92,17 +92,6 @@ module WorkerAttributes set_class_attribute(:data_consistency_feature_flag, feature_flag) if feature_flag set_class_attribute(:data_consistency, data_consistency) - - validate_worker_attributes! - end - - def validate_worker_attributes! - # Since the deduplication should always take into account the latest binary replication pointer into account, - # not the first one, the deduplication will not work with sticky or delayed. - # Follow up issue to improve this: https://gitlab.com/gitlab-org/gitlab/-/issues/325291 - if idempotent? && utilizes_load_balancing_capabilities? - raise ArgumentError, "Class can't be marked as idempotent if data_consistency is not set to :always" - end end # If data_consistency is not set to :always, worker will try to utilize load balancing capabilities and use the replica @@ -147,8 +136,6 @@ module WorkerAttributes def idempotent! set_class_attribute(:idempotent, true) - - validate_worker_attributes! end def idempotent? diff --git a/config/feature_flags/development/preserve_latest_wal_locations_for_idempotent_jobs.yml b/config/feature_flags/development/preserve_latest_wal_locations_for_idempotent_jobs.yml new file mode 100644 index 00000000000..52ef276e950 --- /dev/null +++ b/config/feature_flags/development/preserve_latest_wal_locations_for_idempotent_jobs.yml @@ -0,0 +1,8 @@ +--- +name: preserve_latest_wal_locations_for_idempotent_jobs +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/66280 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/338350 +milestone: '14.3' +type: development +group: group::memory +default_enabled: false diff --git a/config/initializers/postgres_partitioning.rb b/config/initializers/postgres_partitioning.rb index 49936e7cc79..67f155a6861 100644 --- a/config/initializers/postgres_partitioning.rb +++ b/config/initializers/postgres_partitioning.rb @@ -5,6 +5,7 @@ Gitlab::Database::Partitioning::PartitionManager.register(AuditEvent) Gitlab::Database::Partitioning::PartitionManager.register(WebHookLog) +Gitlab::Database::Partitioning::PartitionManager.register(LooseForeignKeys::DeletedRecord) if Gitlab.ee? Gitlab::Database::Partitioning::PartitionManager.register(IncidentManagement::PendingEscalations::Alert) diff --git a/db/migrate/20210826122748_create_loose_foreign_keys_deleted_records.rb b/db/migrate/20210826122748_create_loose_foreign_keys_deleted_records.rb new file mode 100644 index 00000000000..5abea4393b4 --- /dev/null +++ b/db/migrate/20210826122748_create_loose_foreign_keys_deleted_records.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +class CreateLooseForeignKeysDeletedRecords < ActiveRecord::Migration[6.1] + include Gitlab::Database::PartitioningMigrationHelpers::TableManagementHelpers + + def up + constraint_name = check_constraint_name('loose_foreign_keys_deleted_records', 'deleted_table_name', 'max_length') + execute(<<~SQL) + CREATE TABLE loose_foreign_keys_deleted_records ( + created_at timestamp with time zone NOT NULL DEFAULT NOW(), + deleted_table_name text NOT NULL, + deleted_table_primary_key_value bigint NOT NULL, + PRIMARY KEY (created_at, deleted_table_name, deleted_table_primary_key_value), + CONSTRAINT #{constraint_name} CHECK ((char_length(deleted_table_name) <= 63)) + ) PARTITION BY RANGE (created_at); + SQL + + min_date = Date.today - 1.month + max_date = Date.today + 3.months + create_daterange_partitions('loose_foreign_keys_deleted_records', 'created_at', min_date, max_date) + end + + def down + drop_table :loose_foreign_keys_deleted_records + end +end diff --git a/db/migrate/20210826145509_add_function_for_inserting_deleted_records.rb b/db/migrate/20210826145509_add_function_for_inserting_deleted_records.rb new file mode 100644 index 00000000000..ef688cdfd8c --- /dev/null +++ b/db/migrate/20210826145509_add_function_for_inserting_deleted_records.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +class AddFunctionForInsertingDeletedRecords < ActiveRecord::Migration[6.1] + include Gitlab::Database::MigrationHelpers + include Gitlab::Database::MigrationHelpers::LooseForeignKeyHelpers + + def up + execute(<<~SQL) + CREATE FUNCTION #{DELETED_RECORDS_INSERT_FUNCTION_NAME}() + RETURNS TRIGGER AS + $$ + BEGIN + INSERT INTO loose_foreign_keys_deleted_records + (deleted_table_name, deleted_table_primary_key_value) + SELECT TG_TABLE_NAME, old_table.id FROM old_table + ON CONFLICT DO NOTHING; + + RETURN NULL; + END + $$ LANGUAGE PLPGSQL + SQL + end + + def down + drop_function(DELETED_RECORDS_INSERT_FUNCTION_NAME) + end +end diff --git a/db/post_migrate/20210909104800_reschedule_extract_project_topics_into_separate_table_2.rb b/db/post_migrate/20210909104800_reschedule_extract_project_topics_into_separate_table_2.rb new file mode 100644 index 00000000000..ad31a40f324 --- /dev/null +++ b/db/post_migrate/20210909104800_reschedule_extract_project_topics_into_separate_table_2.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +class RescheduleExtractProjectTopicsIntoSeparateTable2 < Gitlab::Database::Migration[1.0] + MIGRATION = 'ExtractProjectTopicsIntoSeparateTable' + DELAY_INTERVAL = 4.minutes + + disable_ddl_transaction! + + def up + requeue_background_migration_jobs_by_range_at_intervals(MIGRATION, DELAY_INTERVAL) + end + + def down + # no-op + end +end diff --git a/db/post_migrate/20210915202900_prepare_index_resource_group_status_commit_id_for_ci_builds.rb b/db/post_migrate/20210915202900_prepare_index_resource_group_status_commit_id_for_ci_builds.rb new file mode 100644 index 00000000000..42d21806405 --- /dev/null +++ b/db/post_migrate/20210915202900_prepare_index_resource_group_status_commit_id_for_ci_builds.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class PrepareIndexResourceGroupStatusCommitIdForCiBuilds < Gitlab::Database::Migration[1.0] + INDEX_NAME = 'index_ci_builds_on_resource_group_and_status_and_commit_id' + + def up + prepare_async_index :ci_builds, [:resource_group_id, :status, :commit_id], + where: 'resource_group_id IS NOT NULL', + name: INDEX_NAME + end + + def down + unprepare_async_index_by_name :ci_builds, INDEX_NAME + end +end diff --git a/db/schema_migrations/20210826122748 b/db/schema_migrations/20210826122748 new file mode 100644 index 00000000000..e6d87674da3 --- /dev/null +++ b/db/schema_migrations/20210826122748 @@ -0,0 +1 @@ +a1290cc671c487a7c24bfdb02c564d656a6606258e680e65ed108e3a28de10ca
\ No newline at end of file diff --git a/db/schema_migrations/20210826145509 b/db/schema_migrations/20210826145509 new file mode 100644 index 00000000000..9f93b675b12 --- /dev/null +++ b/db/schema_migrations/20210826145509 @@ -0,0 +1 @@ +661b2f03f2387f0d49cbb11c333ad29c6af5caed1f43e860fa0f263f8e7371c2
\ No newline at end of file diff --git a/db/schema_migrations/20210909104800 b/db/schema_migrations/20210909104800 new file mode 100644 index 00000000000..70ad28d72b8 --- /dev/null +++ b/db/schema_migrations/20210909104800 @@ -0,0 +1 @@ +834825de758314db01a3daa2d2b943c11751553705c4dd078f2087b9ec6ff7f7
\ No newline at end of file diff --git a/db/schema_migrations/20210915202900 b/db/schema_migrations/20210915202900 new file mode 100644 index 00000000000..bbe7f918f7a --- /dev/null +++ b/db/schema_migrations/20210915202900 @@ -0,0 +1 @@ +cc8f1bea8c722dfa06fc23a3dbaaacd7f1b7c5f7b529ebfab34b7c7c38e5db79
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index bfc2abf08f5..534a2bcef8d 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -22,6 +22,19 @@ RETURN NULL; END $$; +CREATE FUNCTION insert_into_loose_foreign_keys_deleted_records() RETURNS trigger + LANGUAGE plpgsql + AS $$ +BEGIN + INSERT INTO loose_foreign_keys_deleted_records + (deleted_table_name, deleted_table_primary_key_value) + SELECT TG_TABLE_NAME, old_table.id FROM old_table + ON CONFLICT DO NOTHING; + + RETURN NULL; +END +$$; + CREATE FUNCTION integrations_set_type_new() RETURNS trigger LANGUAGE plpgsql AS $$ @@ -153,6 +166,14 @@ CREATE TABLE incident_management_pending_issue_escalations ( ) PARTITION BY RANGE (process_at); +CREATE TABLE loose_foreign_keys_deleted_records ( + created_at timestamp with time zone DEFAULT now() NOT NULL, + deleted_table_name text NOT NULL, + deleted_table_primary_key_value bigint NOT NULL, + CONSTRAINT check_7229f9527e CHECK ((char_length(deleted_table_name) <= 63)) +) +PARTITION BY RANGE (created_at); + CREATE TABLE web_hook_logs ( id bigint NOT NULL, web_hook_id integer NOT NULL, @@ -23077,6 +23098,9 @@ ALTER TABLE ONLY list_user_preferences ALTER TABLE ONLY lists ADD CONSTRAINT lists_pkey PRIMARY KEY (id); +ALTER TABLE ONLY loose_foreign_keys_deleted_records + ADD CONSTRAINT loose_foreign_keys_deleted_records_pkey PRIMARY KEY (created_at, deleted_table_name, deleted_table_primary_key_value); + ALTER TABLE ONLY members ADD CONSTRAINT members_pkey PRIMARY KEY (id); diff --git a/doc/.vale/gitlab/ElementDescriptors.yml b/doc/.vale/gitlab/ElementDescriptors.yml new file mode 100644 index 00000000000..254da16d00c --- /dev/null +++ b/doc/.vale/gitlab/ElementDescriptors.yml @@ -0,0 +1,14 @@ +--- +# Suggestion: gitlab.ElementDescriptors +# +# Suggests the correct way to describe elements in a form. +# +# For a list of all options, see https://errata-ai.github.io/vale/styles/ +extends: substitution +message: 'When describing elements, %s "%s".' +link: https://docs.gitlab.com/ee/development/documentation/styleguide/index.html#language +level: suggestion +ignorecase: true +swap: + button: 'if possible, rewrite to not use' + area: 'use "section" instead of' diff --git a/doc/administration/object_storage.md b/doc/administration/object_storage.md index a24de2a0817..6c77576cb27 100644 --- a/doc/administration/object_storage.md +++ b/doc/administration/object_storage.md @@ -74,6 +74,7 @@ because it does not require a shared folder. Consolidated object storage configuration can't be used for backups or Mattermost. See the [full table for a complete list](#storage-specific-configuration). +However, backups can be configured with [server side encryption](../raketasks/backup_restore.md#s3-encrypted-buckets) separately. Enabling consolidated object storage enables object storage for all object types. If you want to use local storage for specific object types, you can diff --git a/doc/administration/packages/container_registry.md b/doc/administration/packages/container_registry.md index 1134ef8ac77..1067474c8b4 100644 --- a/doc/administration/packages/container_registry.md +++ b/doc/administration/packages/container_registry.md @@ -1054,6 +1054,80 @@ PATH=/usr/local/sbin:/usr/local/bin:/sbin:/bin:/usr/sbin:/usr/bin You may want to add the `-m` flag to [remove untagged manifests and unreferenced layers](#removing-untagged-manifests-and-unreferenced-layers). +## Configuring GitLab and Registry to run on separate nodes (Omnibus GitLab) + +By default, package assumes that both services are running on the same node. +In order to get GitLab and Registry to run on a separate nodes, separate configuration +is necessary for Registry and GitLab. + +### Configuring Registry + +Below you will find configuration options you should set in `/etc/gitlab/gitlab.rb`, +for Registry to run separately from GitLab: + +- `registry['registry_http_addr']`, default [set programmatically](https://gitlab.com/gitlab-org/omnibus-gitlab/blob/10-3-stable/files/gitlab-cookbooks/gitlab/libraries/registry.rb#L50). Needs to be reachable by web server (or LB). +- `registry['token_realm']`, default [set programmatically](https://gitlab.com/gitlab-org/omnibus-gitlab/blob/10-3-stable/files/gitlab-cookbooks/gitlab/libraries/registry.rb#L53). Specifies the endpoint to use to perform authentication, usually the GitLab URL. + This endpoint needs to be reachable by user. +- `registry['http_secret']`, [random string](https://gitlab.com/gitlab-org/omnibus-gitlab/blob/10-3-stable/files/gitlab-cookbooks/gitlab/libraries/registry.rb#L32). A random piece of data used to sign state that may be stored with the client to protect against tampering. +- `registry['internal_key']`, default [automatically generated](https://gitlab.com/gitlab-org/omnibus-gitlab/blob/10-3-stable/files/gitlab-cookbooks/gitlab/recipes/gitlab-rails.rb#L113-119). Contents of the key that GitLab uses to sign the tokens. They key gets created on the Registry server, but it won't be used there. +- `gitlab_rails['registry_key_path']`, default [set programmatically](https://gitlab.com/gitlab-org/omnibus-gitlab/blob/10-3-stable/files/gitlab-cookbooks/gitlab/recipes/gitlab-rails.rb#L35). This is the path where `internal_key` contents will be written to disk. +- `registry['internal_certificate']`, default [automatically generated](https://gitlab.com/gitlab-org/omnibus-gitlab/blob/10-3-stable/files/gitlab-cookbooks/registry/recipes/enable.rb#L60-66). Contents of the certificate that GitLab uses to sign the tokens. +- `registry['rootcertbundle']`, default [set programmatically](https://gitlab.com/gitlab-org/omnibus-gitlab/blob/10-3-stable/files/gitlab-cookbooks/registry/recipes/enable.rb#L60). Path to certificate. This is the path where `internal_certificate` + contents will be written to disk. +- `registry['health_storagedriver_enabled']`, default [set programmatically](https://gitlab.com/gitlab-org/omnibus-gitlab/blob/10-7-stable/files/gitlab-cookbooks/gitlab/libraries/registry.rb#L88). Configure whether health checks on the configured storage driver are enabled. +- `gitlab_rails['registry_issuer']`, [default value](https://gitlab.com/gitlab-org/omnibus-gitlab/blob/10-3-stable/files/gitlab-cookbooks/gitlab/attributes/default.rb#L153). This setting needs to be set the same between Registry and GitLab. + +### Configuring GitLab + +Below you will find configuration options you should set in `/etc/gitlab/gitlab.rb`, +for GitLab to run separately from Registry: + +- `gitlab_rails['registry_enabled']`, must be set to `true`. This setting will + signal to GitLab that it should allow Registry API requests. +- `gitlab_rails['registry_api_url']`, default [set programmatically](https://gitlab.com/gitlab-org/omnibus-gitlab/blob/10-3-stable/files/gitlab-cookbooks/gitlab/libraries/registry.rb#L52). This is the Registry URL used internally that users do not need to interact with, `registry['registry_http_addr']` with scheme. +- `gitlab_rails['registry_host']`, eg. `registry.gitlab.example`. Registry endpoint without the scheme, the address that gets shown to the end user. +- `gitlab_rails['registry_port']`. Registry endpoint port, visible to the end user. +- `gitlab_rails['registry_issuer']` must match the issuer in the Registry configuration. +- `gitlab_rails['registry_key_path']`, path to the key that matches the certificate on the + Registry side. +- `gitlab_rails['internal_key']`, contents of the key that GitLab uses to sign the tokens. + +## Architecture of GitLab Container Registry + +The GitLab registry is what users use to store their own Docker images. +Because of that the Registry is client facing, meaning that we expose it directly +on the web server (or load balancers, LB for short). + + + +The flow described by the diagram above: + +1. A user runs `docker login registry.gitlab.example` on their client. This reaches the web server (or LB) on port 443. +1. Web server connects to the Registry backend pool (by default, using port 5000). Since the user + didn’t provide a valid token, the Registry returns a 401 HTTP code and the URL (`token_realm` from + Registry configuration) where to get one. This points to the GitLab API. +1. The Docker client then connects to the GitLab API and obtains a token. +1. The API signs the token with the registry key and hands it to the Docker client +1. The Docker client now logs in again with the token received from the API. It can now push and pull Docker images. + +Reference: <https://docs.docker.com/registry/spec/auth/token/> + +### Communication between GitLab and Registry + +Registry doesn’t have a way to authenticate users internally so it relies on +GitLab to validate credentials. The connection between Registry and GitLab is +TLS encrypted. The key is used by GitLab to sign the tokens while the certificate +is used by Registry to validate the signature. By default, a self-signed certificate key pair is generated +for all installations. This can be overridden as needed. + +GitLab interacts with the Registry using the Registry private key. When a Registry +request goes out, a new short-living (10 minutes) namespace limited token is generated +and signed with the private key. +The Registry then verifies that the signature matches the registry certificate +specified in its configuration and allows the operation. +GitLab background jobs processing (through Sidekiq) also interacts with Registry. +These jobs talk directly to Registry in order to handle image deletion. + ## Troubleshooting Before diving in to the following sections, here's some basic troubleshooting: @@ -1122,7 +1196,7 @@ CI/CD > Container Registry > Authorization token duration (minutes)**. ### Docker login attempt fails with: 'token signed by untrusted key' -[Registry relies on GitLab to validate credentials](https://docs.gitlab.com/omnibus/architecture/registry/). +[Registry relies on GitLab to validate credentials](#architecture-of-gitlab-container-registry) If the registry fails to authenticate valid login attempts, you get the following error message: ```shell diff --git a/doc/administration/packages/img/gitlab-registry-architecture.png b/doc/administration/packages/img/gitlab-registry-architecture.png Binary files differnew file mode 100644 index 00000000000..742678d5e11 --- /dev/null +++ b/doc/administration/packages/img/gitlab-registry-architecture.png diff --git a/doc/development/service_ping/metrics_dictionary.md b/doc/development/service_ping/metrics_dictionary.md index 7c2bf41794d..8dc2d2255d1 100644 --- a/doc/development/service_ping/metrics_dictionary.md +++ b/doc/development/service_ping/metrics_dictionary.md @@ -22,6 +22,9 @@ All metrics are stored in YAML files: - [`config/metrics`](https://gitlab.com/gitlab-org/gitlab/-/tree/master/config/metrics) +WARNING: +Only metrics with a metric definition YAML are added to the Service Ping JSON payload. + Each metric is defined in a separate YAML file consisting of a number of fields: | Field | Required | Additional information | diff --git a/doc/raketasks/backup_restore.md b/doc/raketasks/backup_restore.md index cddf62644d4..b6f772dee17 100644 --- a/doc/raketasks/backup_restore.md +++ b/doc/raketasks/backup_restore.md @@ -407,6 +407,67 @@ For Omnibus GitLab packages: 1. [Reconfigure GitLab](../administration/restart_gitlab.md#omnibus-gitlab-reconfigure) for the changes to take effect +##### S3 Encrypted Buckets + +AWS supports these [modes for server side encryption](https://docs.aws.amazon.com/AmazonS3/latest/userguide/serv-side-encryption.html): + +- Amazon S3-Managed Keys (SSE-S3) +- Customer Master Keys (CMKs) Stored in AWS Key Management Service (SSE-KMS) +- Customer-Provided Keys (SSE-C) + +Use your mode of choice with GitLab. Each mode has similar, but slightly +different, configuration methods. + +###### SSE-S3 + +To enable SSE-S3, in the backup storage options set the `server_side_encryption` +field to `AES256`. For example, in Omnibus GitLab: + +```ruby +gitlab_rails['backup_upload_storage_options'] = { + 'server_side_encryption' => 'AES256' +} +``` + +###### SSE-KMS + +To enable SSE-KMS, you'll need the [KMS key via its Amazon Resource Name (ARN) +in the `arn:aws:kms:region:acct-id:key/key-id` format](https://docs.aws.amazon.com/AmazonS3/latest/userguide/UsingKMSEncryption.html). Under the `backup_upload_storage_options` config setting, set: + +- `server_side_encryption` to `aws:kms`. +- `server_side_encryption_kms_key_id` to the ARN of the key. + +For example, in Omnibus GitLab: + +```ruby +gitlab_rails['backup_upload_storage_options'] = { + 'server_side_encryption' => 'aws:kms', + 'server_side_encryption_kms_key_id' => 'arn:aws:<YOUR KMS KEY ID>:' +} +``` + +###### SSE-C + +SSE-C requires you to set these encryption options: + +- `backup_encryption`: AES256. +- `backup_encryption_key`: Unencoded, 32-byte (256 bits) key. The upload fails if this isn't exactly 32 bytes. + +For example, in Omnibus GitLab: + +```ruby +gitlab_rails['backup_encryption'] = 'AES256' +gitlab_rails['backup_encryption_key'] = '<YOUR 32-BYTE KEY HERE>' +``` + +If the key contains binary characters and cannot be encoded in UTF-8, +instead, specify the key with the `GITLAB_BACKUP_ENCRYPTION_KEY` environment variable. +For example: + +```ruby +gitlab_rails['env'] = { 'GITLAB_BACKUP_ENCRYPTION_KEY' => "\xDE\xAD\xBE\xEF" * 8 } +``` + ##### Digital Ocean Spaces This example can be used for a bucket in Amsterdam (AMS3): @@ -458,15 +519,25 @@ For installations from source: # use_iam_profile: 'true' # The remote 'directory' to store your backups. For S3, this would be the bucket name. remote_directory: 'my.s3.bucket' - # Turns on AWS Server-Side Encryption with Amazon S3-Managed Keys for backups, this is optional - # encryption: 'AES256' - # Turns on AWS Server-Side Encryption with Amazon Customer-Provided Encryption Keys for backups, this is optional - # This should be set to the encryption key for Amazon S3 to use to encrypt or decrypt your data. - # 'encryption' must also be set in order for this to have any effect. - # To avoid storing the key on disk, the key can also be specified via the `GITLAB_BACKUP_ENCRYPTION_KEY` environment variable. - # encryption_key: '<key>' # Specifies Amazon S3 storage class to use for backups, this is optional # storage_class: 'STANDARD' + # + # Turns on AWS Server-Side Encryption with Amazon Customer-Provided Encryption Keys for backups, this is optional + # 'encryption' must be set in order for this to have any effect. + # 'encryption_key' should be set to the 256-bit encryption key for Amazon S3 to use to encrypt or decrypt. + # To avoid storing the key on disk, the key can also be specified via the `GITLAB_BACKUP_ENCRYPTION_KEY` your data. + # encryption: 'AES256' + # encryption_key: '<key>' + # + # + # Turns on AWS Server-Side Encryption with Amazon S3-Managed keys (optional) + # https://docs.aws.amazon.com/AmazonS3/latest/userguide/serv-side-encryption.html + # For SSE-S3, set 'server_side_encryption' to 'AES256'. + # For SS3-KMS, set 'server_side_encryption' to 'aws:kms'. Set + # 'server_side_encryption_kms_key_id' to the ARN of customer master key. + # storage_options: + # server_side_encryption: 'aws:kms' + # server_side_encryption_kms_key_id: 'arn:aws:kms:YOUR-KEY-ID-HERE' ``` 1. [Restart GitLab](../administration/restart_gitlab.md#installations-from-source) diff --git a/doc/user/analytics/index.md b/doc/user/analytics/index.md index 7cb5db4379a..5b7e6e39187 100644 --- a/doc/user/analytics/index.md +++ b/doc/user/analytics/index.md @@ -75,12 +75,12 @@ in one place. [Learn more about instance-level analytics](../admin_area/analytics/index.md). -## Group-level analytics **(PREMIUM)** +## Group-level analytics > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/195979) in GitLab 12.8. > - Moved to GitLab Premium in 13.9. -The following analytics features are available at the group level: +GitLab provides several analytics features at the group level. Some of these features require you to use a higher tier than GitLab Free. - [Application Security](../application_security/security_dashboard/#group-security-dashboard) - [Contribution](../group/contribution_analytics/index.md) @@ -93,7 +93,7 @@ The following analytics features are available at the group level: ## Project-level analytics -The following analytics features are available at the project level: +You can use GitLab to review analytics at the project level. Some of these features require you to use a higher tier than GitLab Free. - [Application Security](../application_security/security_dashboard/#project-security-dashboard) - [CI/CD](ci_cd_analytics.md) @@ -105,8 +105,10 @@ The following analytics features are available at the project level: - [Repository](repository_analytics.md) - [Value Stream](value_stream_analytics.md) -## User-configurable analytics **(ULTIMATE)** +## User-configurable analytics The following analytics features are available for users to create personalized views: - [Application Security](../application_security/security_dashboard/#security-center) + +Be sure to review the documentation page for this feature for GitLab tier requirements. diff --git a/lib/api/entities/global_notification_setting.rb b/lib/api/entities/global_notification_setting.rb index f3ca64347f0..f35efad5d01 100644 --- a/lib/api/entities/global_notification_setting.rb +++ b/lib/api/entities/global_notification_setting.rb @@ -4,7 +4,7 @@ module API module Entities class GlobalNotificationSetting < Entities::NotificationSetting expose :notification_email do |notification_setting, options| - notification_setting.user.notification_email + notification_setting.user.notification_email_or_default end end end diff --git a/lib/api/entities/user_public.rb b/lib/api/entities/user_public.rb index 78f088d3c1a..5d0e464abe1 100644 --- a/lib/api/entities/user_public.rb +++ b/lib/api/entities/user_public.rb @@ -14,7 +14,7 @@ module API expose :two_factor_enabled?, as: :two_factor_enabled expose :external expose :private_profile - expose :commit_email + expose :commit_email_or_default, as: :commit_email end end end diff --git a/lib/gitlab/background_migration/extract_project_topics_into_separate_table.rb b/lib/gitlab/background_migration/extract_project_topics_into_separate_table.rb index c5beb5a6865..31b5b5cdb73 100644 --- a/lib/gitlab/background_migration/extract_project_topics_into_separate_table.rb +++ b/lib/gitlab/background_migration/extract_project_topics_into_separate_table.rb @@ -33,11 +33,15 @@ module Gitlab def perform(start_id, stop_id) Tagging.includes(:tag).where(taggable_type: 'Project', id: start_id..stop_id).each do |tagging| - if Project.exists?(id: tagging.taggable_id) - topic = Topic.find_or_create_by(name: tagging.tag.name) - project_topic = ProjectTopic.find_or_create_by(project_id: tagging.taggable_id, topic: topic) - - tagging.delete if project_topic.persisted? + if Project.exists?(id: tagging.taggable_id) && tagging.tag + begin + topic = Topic.find_or_create_by(name: tagging.tag.name) + project_topic = ProjectTopic.find_or_create_by(project_id: tagging.taggable_id, topic: topic) + + tagging.delete if project_topic.persisted? + rescue StandardError => e + Gitlab::ErrorTracking.log_exception(e, tagging_id: tagging.id) + end else tagging.delete end diff --git a/lib/gitlab/background_migration/mailers/unconfirm_mailer.rb b/lib/gitlab/background_migration/mailers/unconfirm_mailer.rb index c096dae0631..3605b157f4f 100644 --- a/lib/gitlab/background_migration/mailers/unconfirm_mailer.rb +++ b/lib/gitlab/background_migration/mailers/unconfirm_mailer.rb @@ -14,7 +14,7 @@ module Gitlab mail( template_path: 'unconfirm_mailer', template_name: 'unconfirm_notification_email', - to: @user.notification_email, + to: @user.notification_email_or_default, subject: subject('GitLab email verification request') ) end diff --git a/lib/gitlab/database/connection.rb b/lib/gitlab/database/connection.rb index 6eac115be13..cda6220ee6c 100644 --- a/lib/gitlab/database/connection.rb +++ b/lib/gitlab/database/connection.rb @@ -187,6 +187,19 @@ module Gitlab row['system_identifier'] end + def pg_wal_lsn_diff(location1, location2) + lsn1 = connection.quote(location1) + lsn2 = connection.quote(location2) + + query = <<-SQL.squish + SELECT pg_wal_lsn_diff(#{lsn1}, #{lsn2}) + AS result + SQL + + row = connection.select_all(query).first + row['result'] if row + end + # @param [ActiveRecord::Connection] ar_connection # @return [String] def get_write_location(ar_connection) diff --git a/lib/gitlab/database/load_balancing/sidekiq_server_middleware.rb b/lib/gitlab/database/load_balancing/sidekiq_server_middleware.rb index f7fda14b215..15f8f0fb240 100644 --- a/lib/gitlab/database/load_balancing/sidekiq_server_middleware.rb +++ b/lib/gitlab/database/load_balancing/sidekiq_server_middleware.rb @@ -57,7 +57,7 @@ module Gitlab end def get_wal_locations(job) - job['wal_locations'] || legacy_wal_location(job) + job['dedup_wal_locations'] || job['wal_locations'] || legacy_wal_location(job) end # Already scheduled jobs could still contain legacy database write location. diff --git a/lib/gitlab/database/migration_helpers/loose_foreign_key_helpers.rb b/lib/gitlab/database/migration_helpers/loose_foreign_key_helpers.rb new file mode 100644 index 00000000000..30601bffd7a --- /dev/null +++ b/lib/gitlab/database/migration_helpers/loose_foreign_key_helpers.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +module Gitlab + module Database + module MigrationHelpers + module LooseForeignKeyHelpers + include Gitlab::Database::SchemaHelpers + + DELETED_RECORDS_INSERT_FUNCTION_NAME = 'insert_into_loose_foreign_keys_deleted_records' + + def track_record_deletions(table) + execute(<<~SQL) + CREATE TRIGGER #{record_deletion_trigger_name(table)} + AFTER DELETE ON #{table} REFERENCING OLD TABLE AS old_table + FOR EACH STATEMENT + EXECUTE FUNCTION #{DELETED_RECORDS_INSERT_FUNCTION_NAME}(); + SQL + end + + def untrack_record_deletions(table) + drop_trigger(table, record_deletion_trigger_name(table)) + end + + private + + def record_deletion_trigger_name(table) + "#{table}_loose_fk_trigger" + end + end + end + end +end diff --git a/lib/gitlab/git/user.rb b/lib/gitlab/git/user.rb index 05ae3391040..0798cc51055 100644 --- a/lib/gitlab/git/user.rb +++ b/lib/gitlab/git/user.rb @@ -6,7 +6,7 @@ module Gitlab attr_reader :username, :name, :email, :gl_id, :timezone def self.from_gitlab(gitlab_user) - new(gitlab_user.username, gitlab_user.name, gitlab_user.commit_email, Gitlab::GlId.gl_id(gitlab_user), gitlab_user.timezone) + new(gitlab_user.username, gitlab_user.name, gitlab_user.commit_email_or_default, Gitlab::GlId.gl_id(gitlab_user), gitlab_user.timezone) end def self.from_gitaly(gitaly_user) diff --git a/lib/gitlab/sidekiq_logging/structured_logger.rb b/lib/gitlab/sidekiq_logging/structured_logger.rb index 842e53b2ffb..1aebce987fe 100644 --- a/lib/gitlab/sidekiq_logging/structured_logger.rb +++ b/lib/gitlab/sidekiq_logging/structured_logger.rb @@ -69,6 +69,7 @@ module Gitlab message = base_message(payload) payload['load_balancing_strategy'] = job['load_balancing_strategy'] if job['load_balancing_strategy'] + payload['dedup_wal_locations'] = job['dedup_wal_locations'] if job['dedup_wal_locations'].present? if job_exception payload['message'] = "#{message}: fail: #{payload['duration_s']} sec" diff --git a/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job.rb b/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job.rb index c1dc616cbb2..aeb58d7c153 100644 --- a/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job.rb +++ b/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job.rb @@ -17,10 +17,26 @@ module Gitlab # # When new jobs can be scheduled again, the strategy calls `#delete`. class DuplicateJob + include Gitlab::Utils::StrongMemoize + DUPLICATE_KEY_TTL = 6.hours + WAL_LOCATION_TTL = 60.seconds + MAX_REDIS_RETRIES = 5 DEFAULT_STRATEGY = :until_executing STRATEGY_NONE = :none + LUA_SET_WAL_SCRIPT = <<~EOS + local key, wal, offset, ttl = KEYS[1], ARGV[1], tonumber(ARGV[2]), ARGV[3] + local existing_offset = redis.call("LINDEX", key, -1) + if existing_offset == false then + redis.call("RPUSH", key, wal, offset) + redis.call("EXPIRE", key, ttl) + elseif offset > tonumber(existing_offset) then + redis.call("LSET", key, 0, wal) + redis.call("LSET", key, -1, offset) + end + EOS + attr_reader :existing_jid def initialize(job, queue_name) @@ -44,22 +60,59 @@ module Gitlab # This method will return the jid that was set in redis def check!(expiry = DUPLICATE_KEY_TTL) read_jid = nil + read_wal_locations = {} Sidekiq.redis do |redis| redis.multi do |multi| redis.set(idempotency_key, jid, ex: expiry, nx: true) + read_wal_locations = check_existing_wal_locations!(redis, expiry) read_jid = redis.get(idempotency_key) end end job['idempotency_key'] = idempotency_key + # We need to fetch values since the read_wal_locations and read_jid were obtained inside transaction, under redis.multi command. + self.existing_wal_locations = read_wal_locations.transform_values(&:value) self.existing_jid = read_jid.value end + def update_latest_wal_location! + return unless job_wal_locations.present? + + Sidekiq.redis do |redis| + redis.multi do + job_wal_locations.each do |connection_name, location| + redis.eval(LUA_SET_WAL_SCRIPT, keys: [wal_location_key(connection_name)], argv: [location, pg_wal_lsn_diff(connection_name).to_i, WAL_LOCATION_TTL]) + end + end + end + end + + def latest_wal_locations + return {} unless job_wal_locations.present? + + strong_memoize(:latest_wal_locations) do + read_wal_locations = {} + + Sidekiq.redis do |redis| + redis.multi do + job_wal_locations.keys.each do |connection_name| + read_wal_locations[connection_name] = redis.lindex(wal_location_key(connection_name), 0) + end + end + end + + read_wal_locations.transform_values(&:value).compact + end + end + def delete! Sidekiq.redis do |redis| - redis.del(idempotency_key) + redis.multi do |multi| + redis.del(idempotency_key) + delete_wal_locations!(redis) + end end end @@ -93,6 +146,7 @@ module Gitlab private + attr_accessor :existing_wal_locations attr_reader :queue_name, :job attr_writer :existing_jid @@ -100,6 +154,10 @@ module Gitlab @worker_klass ||= worker_class_name.to_s.safe_constantize end + def pg_wal_lsn_diff(connection_name) + Gitlab::Database::DATABASES[connection_name].pg_wal_lsn_diff(job_wal_locations[connection_name], existing_wal_locations[connection_name]) + end + def strategy return DEFAULT_STRATEGY unless worker_klass return DEFAULT_STRATEGY unless worker_klass.respond_to?(:idempotent?) @@ -120,6 +178,20 @@ module Gitlab job['jid'] end + def job_wal_locations + return {} unless preserve_wal_location? + + job['wal_locations'] || {} + end + + def existing_wal_location_key(connection_name) + "#{idempotency_key}:#{connection_name}:existing_wal_location" + end + + def wal_location_key(connection_name) + "#{idempotency_key}:#{connection_name}:wal_location" + end + def idempotency_key @idempotency_key ||= job['idempotency_key'] || "#{namespace}:#{idempotency_hash}" end @@ -135,6 +207,29 @@ module Gitlab def idempotency_string "#{worker_class_name}:#{Sidekiq.dump_json(arguments)}" end + + def delete_wal_locations!(redis) + job_wal_locations.keys.each do |connection_name| + redis.del(wal_location_key(connection_name)) + redis.del(existing_wal_location_key(connection_name)) + end + end + + def check_existing_wal_locations!(redis, expiry) + read_wal_locations = {} + + job_wal_locations.each do |connection_name, location| + key = existing_wal_location_key(connection_name) + redis.set(key, location, ex: expiry, nx: true) + read_wal_locations[connection_name] = redis.get(key) + end + + read_wal_locations + end + + def preserve_wal_location? + Feature.enabled?(:preserve_latest_wal_locations_for_idempotent_jobs, default_enabled: :yaml) + end end end end diff --git a/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/deduplicates_when_scheduling.rb b/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/deduplicates_when_scheduling.rb index 469033a5e52..fc58d4f5323 100644 --- a/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/deduplicates_when_scheduling.rb +++ b/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/deduplicates_when_scheduling.rb @@ -14,6 +14,8 @@ module Gitlab job['duplicate-of'] = duplicate_job.existing_jid if duplicate_job.idempotent? + duplicate_job.update_latest_wal_location! + Gitlab::SidekiqLogging::DeduplicationLogger.instance.log( job, "dropped #{strategy_name}", duplicate_job.options) return false @@ -23,8 +25,16 @@ module Gitlab yield end + def perform(job) + update_job_wal_location!(job) + end + private + def update_job_wal_location!(job) + job['dedup_wal_locations'] = duplicate_job.latest_wal_locations if duplicate_job.latest_wal_locations.present? + end + def deduplicatable_job? !duplicate_job.scheduled? || duplicate_job.options[:including_scheduled] end diff --git a/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/until_executed.rb b/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/until_executed.rb index 738efa36fc8..5164b994267 100644 --- a/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/until_executed.rb +++ b/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/until_executed.rb @@ -8,9 +8,14 @@ module Gitlab # removes the lock after the job has executed preventing a new job to be queued # while a job is still executing. class UntilExecuted < Base + extend ::Gitlab::Utils::Override + include DeduplicatesWhenScheduling - def perform(_job) + override :perform + def perform(job) + super + yield duplicate_job.delete! diff --git a/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/until_executing.rb b/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/until_executing.rb index 68d66383b2b..1f7e3a4ea30 100644 --- a/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/until_executing.rb +++ b/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/until_executing.rb @@ -8,9 +8,13 @@ module Gitlab # removes the lock before the job starts allowing a new job to be queued # while a job is still executing. class UntilExecuting < Base + extend ::Gitlab::Utils::Override + include DeduplicatesWhenScheduling - def perform(_job) + override :perform + def perform(job) + super duplicate_job.delete! yield diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 146d6d03480..c03801ed9e2 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -13438,6 +13438,12 @@ msgstr "" msgid "Escalation policies must have at least one rule" msgstr "" +msgid "EscalationPolicies|%{clockIcon} IF alert is not %{alertStatus} in %{minutes}" +msgstr "" + +msgid "EscalationPolicies|%{notificationIcon} THEN %{doAction} %{forScheduleOrUser}" +msgstr "" + msgid "EscalationPolicies|+ Add an additional rule" msgstr "" @@ -13486,9 +13492,6 @@ msgstr "" msgid "EscalationPolicies|Failed to load oncall-schedules" msgstr "" -msgid "EscalationPolicies|IF alert is not %{alertStatus} in %{minutes} %{then} THEN %{doAction} %{scheduleOrUser}" -msgstr "" - msgid "EscalationPolicies|IF alert is not %{alertStatus} in %{minutes} minutes" msgstr "" @@ -35921,9 +35924,6 @@ msgstr "" msgid "Unable to fetch branches list, please close the form and try again" msgstr "" -msgid "Unable to fetch unscanned projects" -msgstr "" - msgid "Unable to fetch vulnerable projects" msgstr "" @@ -36113,18 +36113,6 @@ msgstr "" msgid "Unresolved" msgstr "" -msgid "UnscannedProjects|15 or more days" -msgstr "" - -msgid "UnscannedProjects|30 or more days" -msgstr "" - -msgid "UnscannedProjects|5 or more days" -msgstr "" - -msgid "UnscannedProjects|60 or more days" -msgstr "" - msgid "Unschedule job" msgstr "" @@ -39445,6 +39433,9 @@ msgstr "" msgid "cannot merge" msgstr "" +msgid "cannot not be used for user namespace" +msgstr "" + msgid "ciReport|%{degradedNum} degraded" msgstr "" @@ -39955,6 +39946,9 @@ msgstr "" msgid "has been completed." msgstr "" +msgid "has too deep level of nesting" +msgstr "" + msgid "help" msgstr "" @@ -40503,6 +40497,9 @@ msgstr "" msgid "must be less than the limit of %{tag_limit} tags" msgstr "" +msgid "must be set for a project namespace" +msgstr "" + msgid "must be unique by status and elapsed time within a policy" msgstr "" @@ -40664,6 +40661,9 @@ msgstr "" msgid "project name" msgstr "" +msgid "project namespace cannot be the parent of another namespace" +msgstr "" + msgid "projects" msgstr "" @@ -40900,6 +40900,9 @@ msgstr "" msgid "user avatar" msgstr "" +msgid "user namespace cannot be the parent of another namespace" +msgstr "" + msgid "user preferences" msgstr "" diff --git a/package.json b/package.json index 3509ae3362f..8df8e9eef25 100644 --- a/package.json +++ b/package.json @@ -63,12 +63,12 @@ "@rails/ujs": "6.1.3-2", "@sentry/browser": "5.30.0", "@sourcegraph/code-host-integration": "0.0.60", - "@tiptap/core": "^2.0.0-beta.103", + "@tiptap/core": "^2.0.0-beta.105", "@tiptap/extension-blockquote": "^2.0.0-beta.15", "@tiptap/extension-bold": "^2.0.0-beta.15", "@tiptap/extension-bullet-list": "^2.0.0-beta.15", "@tiptap/extension-code": "^2.0.0-beta.16", - "@tiptap/extension-code-block-lowlight": "2.0.0-beta.36", + "@tiptap/extension-code-block-lowlight": "2.0.0-beta.37", "@tiptap/extension-document": "^2.0.0-beta.13", "@tiptap/extension-dropcursor": "^2.0.0-beta.19", "@tiptap/extension-gapcursor": "^2.0.0-beta.19", @@ -78,18 +78,18 @@ "@tiptap/extension-horizontal-rule": "^2.0.0-beta.19", "@tiptap/extension-image": "^2.0.0-beta.15", "@tiptap/extension-italic": "^2.0.0-beta.15", - "@tiptap/extension-link": "^2.0.0-beta.19", + "@tiptap/extension-link": "^2.0.0-beta.20", "@tiptap/extension-list-item": "^2.0.0-beta.14", - "@tiptap/extension-ordered-list": "^2.0.0-beta.15", + "@tiptap/extension-ordered-list": "^2.0.0-beta.16", "@tiptap/extension-paragraph": "^2.0.0-beta.17", "@tiptap/extension-strike": "^2.0.0-beta.17", "@tiptap/extension-subscript": "^2.0.0-beta.4", "@tiptap/extension-superscript": "^2.0.0-beta.4", "@tiptap/extension-table": "^2.0.0-beta.30", - "@tiptap/extension-table-cell": "^2.0.0-beta.14", - "@tiptap/extension-table-header": "^2.0.0-beta.16", + "@tiptap/extension-table-cell": "^2.0.0-beta.15", + "@tiptap/extension-table-header": "^2.0.0-beta.17", "@tiptap/extension-table-row": "^2.0.0-beta.14", - "@tiptap/extension-task-item": "^2.0.0-beta.17", + "@tiptap/extension-task-item": "^2.0.0-beta.18", "@tiptap/extension-task-list": "^2.0.0-beta.17", "@tiptap/extension-text": "^2.0.0-beta.13", "@tiptap/vue-2": "^2.0.0-beta.50", @@ -123,7 +123,7 @@ "dateformat": "^4.5.1", "deckar01-task_list": "^2.3.1", "diff": "^3.4.0", - "dompurify": "^2.3.1", + "dompurify": "^2.3.2", "dropzone": "^4.2.0", "editorconfig": "^0.15.3", "emoji-regex": "^7.0.3", diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb index 2d5125c9d5e..015c36c9335 100644 --- a/spec/controllers/admin/users_controller_spec.rb +++ b/spec/controllers/admin/users_controller_spec.rb @@ -146,7 +146,7 @@ RSpec.describe Admin::UsersController do it 'sends the user a rejection email' do expect_next_instance_of(NotificationService) do |notification| - allow(notification).to receive(:user_admin_rejection).with(user.name, user.notification_email) + allow(notification).to receive(:user_admin_rejection).with(user.name, user.notification_email_or_default) end subject diff --git a/spec/factories/namespaces/project_namespaces.rb b/spec/factories/namespaces/project_namespaces.rb index 97c03ca8b8a..10b86f48090 100644 --- a/spec/factories/namespaces/project_namespaces.rb +++ b/spec/factories/namespaces/project_namespaces.rb @@ -7,5 +7,6 @@ FactoryBot.define do path { project.path } type { Namespaces::ProjectNamespace.sti_name } owner { nil } + parent factory: :group end end diff --git a/spec/features/groups/members/request_access_spec.rb b/spec/features/groups/members/request_access_spec.rb index 827962fee61..f806c7d3704 100644 --- a/spec/features/groups/members/request_access_spec.rb +++ b/spec/features/groups/members/request_access_spec.rb @@ -24,7 +24,7 @@ RSpec.describe 'Groups > Members > Request access' do it 'user can request access to a group' do perform_enqueued_jobs { click_link 'Request Access' } - expect(ActionMailer::Base.deliveries.last.to).to eq [owner.notification_email] + expect(ActionMailer::Base.deliveries.last.to).to eq [owner.notification_email_or_default] expect(ActionMailer::Base.deliveries.last.subject).to match "Request to join the #{group.name} group" expect(group.requesters.exists?(user_id: user)).to be_truthy diff --git a/spec/features/issues/csv_spec.rb b/spec/features/issues/csv_spec.rb index 51e0d54ca5e..b4c737495b4 100644 --- a/spec/features/issues/csv_spec.rb +++ b/spec/features/issues/csv_spec.rb @@ -44,7 +44,7 @@ RSpec.describe 'Issues csv', :js do request_csv expect(page).to have_content 'CSV export has started' - expect(page).to have_content "emailed to #{user.notification_email}" + expect(page).to have_content "emailed to #{user.notification_email_or_default}" end it 'includes a csv attachment', :sidekiq_might_not_need_inline do diff --git a/spec/features/projects/members/user_requests_access_spec.rb b/spec/features/projects/members/user_requests_access_spec.rb index 94543290050..113ba692497 100644 --- a/spec/features/projects/members/user_requests_access_spec.rb +++ b/spec/features/projects/members/user_requests_access_spec.rb @@ -23,7 +23,7 @@ RSpec.describe 'Projects > Members > User requests access', :js do it 'user can request access to a project' do perform_enqueued_jobs { click_link 'Request Access' } - expect(ActionMailer::Base.deliveries.last.to).to eq [maintainer.notification_email] + expect(ActionMailer::Base.deliveries.last.to).to eq [maintainer.notification_email_or_default] expect(ActionMailer::Base.deliveries.last.subject).to eq "Request to join the #{project.full_name} project" expect(project.requesters.exists?(user_id: user)).to be_truthy diff --git a/spec/frontend/content_editor/services/markdown_sourcemap_spec.js b/spec/frontend/content_editor/services/markdown_sourcemap_spec.js index a6ebe204078..6f908f468f6 100644 --- a/spec/frontend/content_editor/services/markdown_sourcemap_spec.js +++ b/spec/frontend/content_editor/services/markdown_sourcemap_spec.js @@ -28,8 +28,7 @@ const SourcemapExtension = Extension.create({ source: { parseHTML: (element) => { const source = getMarkdownSource(element); - if (source) return { source }; - return {}; + return source; }, }, }, diff --git a/spec/frontend/content_editor/test_utils.js b/spec/frontend/content_editor/test_utils.js index b5a2abc2389..cf5aa3f2938 100644 --- a/spec/frontend/content_editor/test_utils.js +++ b/spec/frontend/content_editor/test_utils.js @@ -98,9 +98,7 @@ export const createTestContentEditorExtension = ({ commands = [] } = {}) => { return { labelName: { default: null, - parseHTML: (element) => { - return { labelName: element.dataset.labelName }; - }, + parseHTML: (element) => element.dataset.labelName, }, }; }, diff --git a/spec/frontend/packages_and_registries/package_registry/components/list/__snapshots__/packages_list_app_spec.js.snap b/spec/frontend/packages_and_registries/package_registry/components/list/__snapshots__/packages_list_app_spec.js.snap new file mode 100644 index 00000000000..dbebdeeb452 --- /dev/null +++ b/spec/frontend/packages_and_registries/package_registry/components/list/__snapshots__/packages_list_app_spec.js.snap @@ -0,0 +1,68 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`packages_list_app renders 1`] = ` +<div> + <div + help-url="foo" + /> + + <div /> + + <div> + <section + class="row empty-state text-center" + > + <div + class="col-12" + > + <div + class="svg-250 svg-content" + > + <img + alt="" + class="gl-max-w-full" + role="img" + src="helpSvg" + /> + </div> + </div> + + <div + class="col-12" + > + <div + class="text-content gl-mx-auto gl-my-0 gl-p-5" + > + <h1 + class="h4" + > + There are no packages yet + </h1> + + <p> + Learn how to + <b-link-stub + class="gl-link" + event="click" + href="helpUrl" + routertag="a" + target="_blank" + > + publish and share your packages + </b-link-stub> + with GitLab. + </p> + + <div + class="gl-display-flex gl-flex-wrap gl-justify-content-center" + > + <!----> + + <!----> + </div> + </div> + </div> + </section> + </div> +</div> +`; diff --git a/spec/frontend/packages_and_registries/package_registry/components/list/packages_list_app_spec.js b/spec/frontend/packages_and_registries/package_registry/components/list/packages_list_app_spec.js new file mode 100644 index 00000000000..6c871a34d50 --- /dev/null +++ b/spec/frontend/packages_and_registries/package_registry/components/list/packages_list_app_spec.js @@ -0,0 +1,273 @@ +import { GlEmptyState, GlSprintf, GlLink } from '@gitlab/ui'; +import { shallowMount, createLocalVue } from '@vue/test-utils'; +import Vuex from 'vuex'; +import setWindowLocation from 'helpers/set_window_location_helper'; +import createFlash from '~/flash'; +import * as commonUtils from '~/lib/utils/common_utils'; +import { DELETE_PACKAGE_SUCCESS_MESSAGE } from '~/packages/list/constants'; +import { SHOW_DELETE_SUCCESS_ALERT } from '~/packages/shared/constants'; +import PackageListApp from '~/packages_and_registries/package_registry/components/list/packages_list_app.vue'; +import { FILTERED_SEARCH_TERM } from '~/packages_and_registries/shared/constants'; +import * as packageUtils from '~/packages_and_registries/shared/utils'; + +jest.mock('~/lib/utils/common_utils'); +jest.mock('~/flash'); + +const localVue = createLocalVue(); +localVue.use(Vuex); + +describe('packages_list_app', () => { + let wrapper; + let store; + + const PackageList = { + name: 'package-list', + template: '<div><slot name="empty-state"></slot></div>', + }; + const GlLoadingIcon = { name: 'gl-loading-icon', template: '<div>loading</div>' }; + + // we need to manually stub dynamic imported components because shallowMount is not able to stub them automatically. See: https://github.com/vuejs/vue-test-utils/issues/1279 + const PackageSearch = { name: 'PackageSearch', template: '<div></div>' }; + const PackageTitle = { name: 'PackageTitle', template: '<div></div>' }; + const InfrastructureTitle = { name: 'InfrastructureTitle', template: '<div></div>' }; + const InfrastructureSearch = { name: 'InfrastructureSearch', template: '<div></div>' }; + + const emptyListHelpUrl = 'helpUrl'; + const findEmptyState = () => wrapper.find(GlEmptyState); + const findListComponent = () => wrapper.find(PackageList); + const findPackageSearch = () => wrapper.find(PackageSearch); + const findPackageTitle = () => wrapper.find(PackageTitle); + const findInfrastructureTitle = () => wrapper.find(InfrastructureTitle); + const findInfrastructureSearch = () => wrapper.find(InfrastructureSearch); + + const createStore = (filter = []) => { + store = new Vuex.Store({ + state: { + isLoading: false, + config: { + resourceId: 'project_id', + emptyListIllustration: 'helpSvg', + emptyListHelpUrl, + packageHelpUrl: 'foo', + }, + filter, + }, + }); + store.dispatch = jest.fn(); + }; + + const mountComponent = (provide) => { + wrapper = shallowMount(PackageListApp, { + localVue, + store, + stubs: { + GlEmptyState, + GlLoadingIcon, + PackageList, + GlSprintf, + GlLink, + PackageSearch, + PackageTitle, + InfrastructureTitle, + InfrastructureSearch, + }, + provide, + }); + }; + + beforeEach(() => { + createStore(); + jest.spyOn(packageUtils, 'getQueryParams').mockReturnValue({}); + }); + + afterEach(() => { + wrapper.destroy(); + }); + + it('renders', () => { + mountComponent(); + expect(wrapper.element).toMatchSnapshot(); + }); + + it('call requestPackagesList on page:changed', () => { + mountComponent(); + store.dispatch.mockClear(); + + const list = findListComponent(); + list.vm.$emit('page:changed', 1); + expect(store.dispatch).toHaveBeenCalledWith('requestPackagesList', { page: 1 }); + }); + + it('call requestDeletePackage on package:delete', () => { + mountComponent(); + + const list = findListComponent(); + list.vm.$emit('package:delete', 'foo'); + expect(store.dispatch).toHaveBeenCalledWith('requestDeletePackage', 'foo'); + }); + + it('does call requestPackagesList only one time on render', () => { + mountComponent(); + + expect(store.dispatch).toHaveBeenCalledTimes(3); + expect(store.dispatch).toHaveBeenNthCalledWith(1, 'setSorting', expect.any(Object)); + expect(store.dispatch).toHaveBeenNthCalledWith(2, 'setFilter', expect.any(Array)); + expect(store.dispatch).toHaveBeenNthCalledWith(3, 'requestPackagesList'); + }); + + describe('url query string handling', () => { + const defaultQueryParamsMock = { + search: [1, 2], + type: 'npm', + sort: 'asc', + orderBy: 'created', + }; + + it('calls setSorting with the query string based sorting', () => { + jest.spyOn(packageUtils, 'getQueryParams').mockReturnValue(defaultQueryParamsMock); + + mountComponent(); + + expect(store.dispatch).toHaveBeenNthCalledWith(1, 'setSorting', { + orderBy: defaultQueryParamsMock.orderBy, + sort: defaultQueryParamsMock.sort, + }); + }); + + it('calls setFilter with the query string based filters', () => { + jest.spyOn(packageUtils, 'getQueryParams').mockReturnValue(defaultQueryParamsMock); + + mountComponent(); + + expect(store.dispatch).toHaveBeenNthCalledWith(2, 'setFilter', [ + { type: 'type', value: { data: defaultQueryParamsMock.type } }, + { type: FILTERED_SEARCH_TERM, value: { data: defaultQueryParamsMock.search[0] } }, + { type: FILTERED_SEARCH_TERM, value: { data: defaultQueryParamsMock.search[1] } }, + ]); + }); + + it('calls setSorting and setFilters with the results of extractFilterAndSorting', () => { + jest + .spyOn(packageUtils, 'extractFilterAndSorting') + .mockReturnValue({ filters: ['foo'], sorting: { sort: 'desc' } }); + + mountComponent(); + + expect(store.dispatch).toHaveBeenNthCalledWith(1, 'setSorting', { sort: 'desc' }); + expect(store.dispatch).toHaveBeenNthCalledWith(2, 'setFilter', ['foo']); + }); + }); + + describe('empty state', () => { + it('generate the correct empty list link', () => { + mountComponent(); + + const link = findListComponent().find(GlLink); + + expect(link.attributes('href')).toBe(emptyListHelpUrl); + expect(link.text()).toBe('publish and share your packages'); + }); + + it('includes the right content on the default tab', () => { + mountComponent(); + + const heading = findEmptyState().find('h1'); + + expect(heading.text()).toBe('There are no packages yet'); + }); + }); + + describe('filter without results', () => { + beforeEach(() => { + createStore([{ type: 'something' }]); + mountComponent(); + }); + + it('should show specific empty message', () => { + expect(findEmptyState().text()).toContain('Sorry, your filter produced no results'); + expect(findEmptyState().text()).toContain( + 'To widen your search, change or remove the filters above', + ); + }); + }); + + describe('Package Search', () => { + it('exists', () => { + mountComponent(); + + expect(findPackageSearch().exists()).toBe(true); + }); + + it('on update fetches data from the store', () => { + mountComponent(); + store.dispatch.mockClear(); + + findPackageSearch().vm.$emit('update'); + + expect(store.dispatch).toHaveBeenCalledWith('requestPackagesList'); + }); + }); + + describe('Infrastructure config', () => { + it('defaults to package registry components', () => { + mountComponent(); + + expect(findPackageSearch().exists()).toBe(true); + expect(findPackageTitle().exists()).toBe(true); + + expect(findInfrastructureTitle().exists()).toBe(false); + expect(findInfrastructureSearch().exists()).toBe(false); + }); + + it('mount different component based on the provided values', () => { + mountComponent({ + titleComponent: 'InfrastructureTitle', + searchComponent: 'InfrastructureSearch', + }); + + expect(findPackageSearch().exists()).toBe(false); + expect(findPackageTitle().exists()).toBe(false); + + expect(findInfrastructureTitle().exists()).toBe(true); + expect(findInfrastructureSearch().exists()).toBe(true); + }); + }); + + describe('delete alert handling', () => { + const originalLocation = window.location.href; + const search = `?${SHOW_DELETE_SUCCESS_ALERT}=true`; + + beforeEach(() => { + createStore(); + jest.spyOn(commonUtils, 'historyReplaceState').mockImplementation(() => {}); + setWindowLocation(search); + }); + + afterEach(() => { + setWindowLocation(originalLocation); + }); + + it(`creates a flash if the query string contains ${SHOW_DELETE_SUCCESS_ALERT}`, () => { + mountComponent(); + + expect(createFlash).toHaveBeenCalledWith({ + message: DELETE_PACKAGE_SUCCESS_MESSAGE, + type: 'notice', + }); + }); + + it('calls historyReplaceState with a clean url', () => { + mountComponent(); + + expect(commonUtils.historyReplaceState).toHaveBeenCalledWith(originalLocation); + }); + + it(`does nothing if the query string does not contain ${SHOW_DELETE_SUCCESS_ALERT}`, () => { + setWindowLocation('?'); + mountComponent(); + + expect(createFlash).not.toHaveBeenCalled(); + expect(commonUtils.historyReplaceState).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/spec/frontend/packages_and_registries/package_registry/components/list/packages_list_spec.js b/spec/frontend/packages_and_registries/package_registry/components/list/packages_list_spec.js new file mode 100644 index 00000000000..b624e66482d --- /dev/null +++ b/spec/frontend/packages_and_registries/package_registry/components/list/packages_list_spec.js @@ -0,0 +1,217 @@ +import { GlTable, GlPagination, GlModal } from '@gitlab/ui'; +import { mount, createLocalVue } from '@vue/test-utils'; +import { last } from 'lodash'; +import Vuex from 'vuex'; +import stubChildren from 'helpers/stub_children'; +import { packageList } from 'jest/packages/mock_data'; +import PackagesListRow from '~/packages/shared/components/package_list_row.vue'; +import PackagesListLoader from '~/packages/shared/components/packages_list_loader.vue'; +import { TrackingActions } from '~/packages/shared/constants'; +import * as SharedUtils from '~/packages/shared/utils'; +import PackagesList from '~/packages_and_registries/package_registry/components/list/packages_list.vue'; +import Tracking from '~/tracking'; + +const localVue = createLocalVue(); +localVue.use(Vuex); + +describe('packages_list', () => { + let wrapper; + let store; + + const EmptySlotStub = { name: 'empty-slot-stub', template: '<div>bar</div>' }; + + const findPackagesListLoader = () => wrapper.find(PackagesListLoader); + const findPackageListPagination = () => wrapper.find(GlPagination); + const findPackageListDeleteModal = () => wrapper.find(GlModal); + const findEmptySlot = () => wrapper.find(EmptySlotStub); + const findPackagesListRow = () => wrapper.find(PackagesListRow); + + const createStore = (isGroupPage, packages, isLoading) => { + const state = { + isLoading, + packages, + pagination: { + perPage: 1, + total: 1, + page: 1, + }, + config: { + isGroupPage, + }, + sorting: { + orderBy: 'version', + sort: 'desc', + }, + }; + store = new Vuex.Store({ + state, + getters: { + getList: () => packages, + }, + }); + store.dispatch = jest.fn(); + }; + + const mountComponent = ({ + isGroupPage = false, + packages = packageList, + isLoading = false, + ...options + } = {}) => { + createStore(isGroupPage, packages, isLoading); + + wrapper = mount(PackagesList, { + localVue, + store, + stubs: { + ...stubChildren(PackagesList), + GlTable, + GlModal, + }, + ...options, + }); + }; + + afterEach(() => { + wrapper.destroy(); + wrapper = null; + }); + + describe('when is loading', () => { + beforeEach(() => { + mountComponent({ + packages: [], + isLoading: true, + }); + }); + + it('shows skeleton loader when loading', () => { + expect(findPackagesListLoader().exists()).toBe(true); + }); + }); + + describe('when is not loading', () => { + beforeEach(() => { + mountComponent(); + }); + + it('does not show skeleton loader when not loading', () => { + expect(findPackagesListLoader().exists()).toBe(false); + }); + }); + + describe('layout', () => { + beforeEach(() => { + mountComponent(); + }); + + it('contains a pagination component', () => { + const sorting = findPackageListPagination(); + expect(sorting.exists()).toBe(true); + }); + + it('contains a modal component', () => { + const sorting = findPackageListDeleteModal(); + expect(sorting.exists()).toBe(true); + }); + }); + + describe('when the user can destroy the package', () => { + beforeEach(() => { + mountComponent(); + }); + + it('setItemToBeDeleted sets itemToBeDeleted and open the modal', () => { + const mockModalShow = jest.spyOn(wrapper.vm.$refs.packageListDeleteModal, 'show'); + const item = last(wrapper.vm.list); + + findPackagesListRow().vm.$emit('packageToDelete', item); + + return wrapper.vm.$nextTick().then(() => { + expect(wrapper.vm.itemToBeDeleted).toEqual(item); + expect(mockModalShow).toHaveBeenCalled(); + }); + }); + + it('deleteItemConfirmation resets itemToBeDeleted', () => { + wrapper.setData({ itemToBeDeleted: 1 }); + wrapper.vm.deleteItemConfirmation(); + expect(wrapper.vm.itemToBeDeleted).toEqual(null); + }); + + it('deleteItemConfirmation emit package:delete', () => { + const itemToBeDeleted = { id: 2 }; + wrapper.setData({ itemToBeDeleted }); + wrapper.vm.deleteItemConfirmation(); + return wrapper.vm.$nextTick(() => { + expect(wrapper.emitted('package:delete')[0]).toEqual([itemToBeDeleted]); + }); + }); + + it('deleteItemCanceled resets itemToBeDeleted', () => { + wrapper.setData({ itemToBeDeleted: 1 }); + wrapper.vm.deleteItemCanceled(); + expect(wrapper.vm.itemToBeDeleted).toEqual(null); + }); + }); + + describe('when the list is empty', () => { + beforeEach(() => { + mountComponent({ + packages: [], + slots: { + 'empty-state': EmptySlotStub, + }, + }); + }); + + it('show the empty slot', () => { + const emptySlot = findEmptySlot(); + expect(emptySlot.exists()).toBe(true); + }); + }); + + describe('pagination component', () => { + let pagination; + let modelEvent; + + beforeEach(() => { + mountComponent(); + pagination = findPackageListPagination(); + // retrieve the event used by v-model, a more sturdy approach than hardcoding it + modelEvent = pagination.vm.$options.model.event; + }); + + it('emits page:changed events when the page changes', () => { + pagination.vm.$emit(modelEvent, 2); + expect(wrapper.emitted('page:changed')).toEqual([[2]]); + }); + }); + + describe('tracking', () => { + let eventSpy; + let utilSpy; + const category = 'foo'; + + beforeEach(() => { + mountComponent(); + eventSpy = jest.spyOn(Tracking, 'event'); + utilSpy = jest.spyOn(SharedUtils, 'packageTypeToTrackCategory').mockReturnValue(category); + wrapper.setData({ itemToBeDeleted: { package_type: 'conan' } }); + }); + + it('tracking category calls packageTypeToTrackCategory', () => { + expect(wrapper.vm.tracking.category).toBe(category); + expect(utilSpy).toHaveBeenCalledWith('conan'); + }); + + it('deleteItemConfirmation calls event', () => { + wrapper.vm.deleteItemConfirmation(); + expect(eventSpy).toHaveBeenCalledWith( + category, + TrackingActions.DELETE_PACKAGE, + expect.any(Object), + ); + }); + }); +}); diff --git a/spec/frontend/packages_and_registries/package_registry/components/list/packages_search_spec.js b/spec/frontend/packages_and_registries/package_registry/components/list/packages_search_spec.js new file mode 100644 index 00000000000..42bc9fa3a9e --- /dev/null +++ b/spec/frontend/packages_and_registries/package_registry/components/list/packages_search_spec.js @@ -0,0 +1,128 @@ +import { shallowMount, createLocalVue } from '@vue/test-utils'; +import Vuex from 'vuex'; +import { sortableFields } from '~/packages/list/utils'; +import component from '~/packages_and_registries/package_registry/components/list/package_search.vue'; +import PackageTypeToken from '~/packages_and_registries/package_registry/components/list/tokens/package_type_token.vue'; +import RegistrySearch from '~/vue_shared/components/registry/registry_search.vue'; +import UrlSync from '~/vue_shared/components/url_sync.vue'; + +const localVue = createLocalVue(); +localVue.use(Vuex); + +describe('Package Search', () => { + let wrapper; + let store; + + const findRegistrySearch = () => wrapper.findComponent(RegistrySearch); + const findUrlSync = () => wrapper.findComponent(UrlSync); + + const createStore = (isGroupPage) => { + const state = { + config: { + isGroupPage, + }, + sorting: { + orderBy: 'version', + sort: 'desc', + }, + filter: [], + }; + store = new Vuex.Store({ + state, + }); + store.dispatch = jest.fn(); + }; + + const mountComponent = (isGroupPage = false) => { + createStore(isGroupPage); + + wrapper = shallowMount(component, { + localVue, + store, + stubs: { + UrlSync, + }, + }); + }; + + afterEach(() => { + wrapper.destroy(); + wrapper = null; + }); + + it('has a registry search component', () => { + mountComponent(); + + expect(findRegistrySearch().exists()).toBe(true); + expect(findRegistrySearch().props()).toMatchObject({ + filter: store.state.filter, + sorting: store.state.sorting, + tokens: expect.arrayContaining([ + expect.objectContaining({ token: PackageTypeToken, type: 'type', icon: 'package' }), + ]), + sortableFields: sortableFields(), + }); + }); + + it.each` + isGroupPage | page + ${false} | ${'project'} + ${true} | ${'group'} + `('in a $page page binds the right props', ({ isGroupPage }) => { + mountComponent(isGroupPage); + + expect(findRegistrySearch().props()).toMatchObject({ + filter: store.state.filter, + sorting: store.state.sorting, + tokens: expect.arrayContaining([ + expect.objectContaining({ token: PackageTypeToken, type: 'type', icon: 'package' }), + ]), + sortableFields: sortableFields(isGroupPage), + }); + }); + + it('on sorting:changed emits update event and calls vuex setSorting', () => { + const payload = { sort: 'foo' }; + + mountComponent(); + + findRegistrySearch().vm.$emit('sorting:changed', payload); + + expect(store.dispatch).toHaveBeenCalledWith('setSorting', payload); + expect(wrapper.emitted('update')).toEqual([[]]); + }); + + it('on filter:changed calls vuex setFilter', () => { + const payload = ['foo']; + + mountComponent(); + + findRegistrySearch().vm.$emit('filter:changed', payload); + + expect(store.dispatch).toHaveBeenCalledWith('setFilter', payload); + }); + + it('on filter:submit emits update event', () => { + mountComponent(); + + findRegistrySearch().vm.$emit('filter:submit'); + + expect(wrapper.emitted('update')).toEqual([[]]); + }); + + it('has a UrlSync component', () => { + mountComponent(); + + expect(findUrlSync().exists()).toBe(true); + }); + + it('on query:changed calls updateQuery from UrlSync', () => { + jest.spyOn(UrlSync.methods, 'updateQuery').mockImplementation(() => {}); + + mountComponent(); + + findRegistrySearch().vm.$emit('query:changed'); + + expect(UrlSync.methods.updateQuery).toHaveBeenCalled(); + }); +}); diff --git a/spec/frontend/packages_and_registries/package_registry/components/list/packages_title_spec.js b/spec/frontend/packages_and_registries/package_registry/components/list/packages_title_spec.js new file mode 100644 index 00000000000..3fa96ce1d29 --- /dev/null +++ b/spec/frontend/packages_and_registries/package_registry/components/list/packages_title_spec.js @@ -0,0 +1,71 @@ +import { shallowMount } from '@vue/test-utils'; +import { LIST_INTRO_TEXT, LIST_TITLE_TEXT } from '~/packages/list/constants'; +import PackageTitle from '~/packages_and_registries/package_registry/components/list/package_title.vue'; +import MetadataItem from '~/vue_shared/components/registry/metadata_item.vue'; +import TitleArea from '~/vue_shared/components/registry/title_area.vue'; + +describe('PackageTitle', () => { + let wrapper; + let store; + + const findTitleArea = () => wrapper.find(TitleArea); + const findMetadataItem = () => wrapper.find(MetadataItem); + + const mountComponent = (propsData = { helpUrl: 'foo' }) => { + wrapper = shallowMount(PackageTitle, { + store, + propsData, + stubs: { + TitleArea, + }, + }); + }; + + afterEach(() => { + wrapper.destroy(); + wrapper = null; + }); + + describe('title area', () => { + it('exists', () => { + mountComponent(); + + expect(findTitleArea().exists()).toBe(true); + }); + + it('has the correct props', () => { + mountComponent(); + + expect(findTitleArea().props()).toMatchObject({ + title: LIST_TITLE_TEXT, + infoMessages: [{ text: LIST_INTRO_TEXT, link: 'foo' }], + }); + }); + }); + + describe.each` + count | exist | text + ${null} | ${false} | ${''} + ${undefined} | ${false} | ${''} + ${0} | ${true} | ${'0 Packages'} + ${1} | ${true} | ${'1 Package'} + ${2} | ${true} | ${'2 Packages'} + `('when count is $count metadata item', ({ count, exist, text }) => { + beforeEach(() => { + mountComponent({ count, helpUrl: 'foo' }); + }); + + it(`is ${exist} that it exists`, () => { + expect(findMetadataItem().exists()).toBe(exist); + }); + + if (exist) { + it('has the correct props', () => { + expect(findMetadataItem().props()).toMatchObject({ + icon: 'package', + text, + }); + }); + } + }); +}); diff --git a/spec/frontend/packages_and_registries/package_registry/components/list/tokens/package_type_token_spec.js b/spec/frontend/packages_and_registries/package_registry/components/list/tokens/package_type_token_spec.js new file mode 100644 index 00000000000..b0cbe34f0b9 --- /dev/null +++ b/spec/frontend/packages_and_registries/package_registry/components/list/tokens/package_type_token_spec.js @@ -0,0 +1,48 @@ +import { GlFilteredSearchToken, GlFilteredSearchSuggestion } from '@gitlab/ui'; +import { shallowMount } from '@vue/test-utils'; +import component from '~/packages/list/components/tokens/package_type_token.vue'; +import { PACKAGE_TYPES } from '~/packages/list/constants'; + +describe('packages_filter', () => { + let wrapper; + + const findFilteredSearchToken = () => wrapper.find(GlFilteredSearchToken); + const findFilteredSearchSuggestions = () => wrapper.findAll(GlFilteredSearchSuggestion); + + const mountComponent = ({ attrs, listeners } = {}) => { + wrapper = shallowMount(component, { + attrs, + listeners, + }); + }; + + afterEach(() => { + wrapper.destroy(); + wrapper = null; + }); + + it('it binds all of his attrs to filtered search token', () => { + mountComponent({ attrs: { foo: 'bar' } }); + + expect(findFilteredSearchToken().attributes('foo')).toBe('bar'); + }); + + it('it binds all of his events to filtered search token', () => { + const clickListener = jest.fn(); + mountComponent({ listeners: { click: clickListener } }); + + findFilteredSearchToken().vm.$emit('click'); + + expect(clickListener).toHaveBeenCalled(); + }); + + it.each(PACKAGE_TYPES.map((p, index) => [p, index]))( + 'displays a suggestion for %p', + (packageType, index) => { + mountComponent(); + const item = findFilteredSearchSuggestions().at(index); + expect(item.text()).toBe(packageType.title); + expect(item.props('value')).toBe(packageType.type); + }, + ); +}); diff --git a/spec/frontend/vue_shared/components/user_popover/user_popover_spec.js b/spec/frontend/vue_shared/components/user_popover/user_popover_spec.js index 3080045e0b1..926223e0670 100644 --- a/spec/frontend/vue_shared/components/user_popover/user_popover_spec.js +++ b/spec/frontend/vue_shared/components/user_popover/user_popover_spec.js @@ -248,6 +248,13 @@ describe('User Popover Component', () => { const securityBotDocsLink = findSecurityBotDocsLink(); expect(securityBotDocsLink.exists()).toBe(true); expect(securityBotDocsLink.attributes('href')).toBe(SECURITY_BOT_USER.websiteUrl); + expect(securityBotDocsLink.text()).toBe('Learn more about GitLab Security Bot'); + }); + + it("doesn't escape user's name", () => { + createWrapper({ user: { ...SECURITY_BOT_USER, name: '%<>\';"' } }); + const securityBotDocsLink = findSecurityBotDocsLink(); + expect(securityBotDocsLink.text()).toBe('Learn more about %<>\';"'); }); }); }); diff --git a/spec/helpers/issues_helper_spec.rb b/spec/helpers/issues_helper_spec.rb index 261037ccceb..f5f26d306fb 100644 --- a/spec/helpers/issues_helper_spec.rb +++ b/spec/helpers/issues_helper_spec.rb @@ -310,7 +310,7 @@ RSpec.describe IssuesHelper do can_bulk_update: 'true', can_edit: 'true', can_import_issues: 'true', - email: current_user&.notification_email, + email: current_user&.notification_email_or_default, emails_help_page_path: help_page_path('development/emails', anchor: 'email-namespace'), empty_state_svg_path: '#', export_csv_path: export_csv_project_issues_path(project), diff --git a/spec/lib/gitlab/background_migration/extract_project_topics_into_separate_table_spec.rb b/spec/lib/gitlab/background_migration/extract_project_topics_into_separate_table_spec.rb index 05f40ee2501..a111007a984 100644 --- a/spec/lib/gitlab/background_migration/extract_project_topics_into_separate_table_spec.rb +++ b/spec/lib/gitlab/background_migration/extract_project_topics_into_separate_table_spec.rb @@ -22,8 +22,9 @@ RSpec.describe Gitlab::BackgroundMigration::ExtractProjectTopicsIntoSeparateTabl other_tagging = taggings.create!(taggable_type: 'Other', taggable_id: project.id, context: 'topics', tag_id: tag_1.id) tagging_3 = taggings.create!(taggable_type: 'Project', taggable_id: project.id, context: 'topics', tag_id: tag_3.id) tagging_4 = taggings.create!(taggable_type: 'Project', taggable_id: -1, context: 'topics', tag_id: tag_1.id) + tagging_5 = taggings.create!(taggable_type: 'Project', taggable_id: project.id, context: 'topics', tag_id: -1) - subject.perform(tagging_1.id, tagging_4.id) + subject.perform(tagging_1.id, tagging_5.id) # Tagging records expect { tagging_1.reload }.to raise_error(ActiveRecord::RecordNotFound) @@ -31,6 +32,7 @@ RSpec.describe Gitlab::BackgroundMigration::ExtractProjectTopicsIntoSeparateTabl expect { other_tagging.reload }.not_to raise_error(ActiveRecord::RecordNotFound) expect { tagging_3.reload }.to raise_error(ActiveRecord::RecordNotFound) expect { tagging_4.reload }.to raise_error(ActiveRecord::RecordNotFound) + expect { tagging_5.reload }.to raise_error(ActiveRecord::RecordNotFound) # Topic records topic_1 = topics.find_by(name: 'Topic1') diff --git a/spec/lib/gitlab/database/load_balancing/sidekiq_server_middleware_spec.rb b/spec/lib/gitlab/database/load_balancing/sidekiq_server_middleware_spec.rb index 1fe348bbf2e..9f23eb0094f 100644 --- a/spec/lib/gitlab/database/load_balancing/sidekiq_server_middleware_spec.rb +++ b/spec/lib/gitlab/database/load_balancing/sidekiq_server_middleware_spec.rb @@ -98,6 +98,16 @@ RSpec.describe Gitlab::Database::LoadBalancing::SidekiqServerMiddleware do it_behaves_like 'replica is up to date', 'replica' end + context 'when deduplication wal location is set' do + let(:job) { { 'job_id' => 'a180b47c-3fd6-41b8-81e9-34da61c3400e', 'dedup_wal_locations' => wal_locations } } + + before do + allow(load_balancer).to receive(:select_up_to_date_host).with(wal_locations[:main]).and_return(true) + end + + it_behaves_like 'replica is up to date', 'replica' + end + context 'when legacy wal location is set' do let(:job) { { 'job_id' => 'a180b47c-3fd6-41b8-81e9-34da61c3400e', 'database_write_location' => '0/D525E3A8' } } diff --git a/spec/lib/gitlab/database/migration_helpers/loose_foreign_key_helpers_spec.rb b/spec/lib/gitlab/database/migration_helpers/loose_foreign_key_helpers_spec.rb new file mode 100644 index 00000000000..708d1be6e00 --- /dev/null +++ b/spec/lib/gitlab/database/migration_helpers/loose_foreign_key_helpers_spec.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::MigrationHelpers::LooseForeignKeyHelpers do + let_it_be(:migration) do + ActiveRecord::Migration.new.extend(described_class) + end + + let(:model) do + Class.new(ApplicationRecord) do + self.table_name = 'loose_fk_test_table' + end + end + + before(:all) do + migration.create_table :loose_fk_test_table do |t| + t.timestamps + end + end + + before do + 3.times { model.create! } + end + + context 'when the record deletion tracker trigger is not installed' do + it 'does store record deletions' do + model.delete_all + + expect(LooseForeignKeys::DeletedRecord.count).to eq(0) + end + end + + context 'when the record deletion tracker trigger is installed' do + before do + migration.track_record_deletions(:loose_fk_test_table) + end + + it 'stores the record deletion' do + records = model.all + record_to_be_deleted = records.last + + record_to_be_deleted.delete + + expect(LooseForeignKeys::DeletedRecord.count).to eq(1) + deleted_record = LooseForeignKeys::DeletedRecord.all.first + + expect(deleted_record.deleted_table_primary_key_value).to eq(record_to_be_deleted.id) + expect(deleted_record.deleted_table_name).to eq('loose_fk_test_table') + end + + it 'stores multiple record deletions' do + model.delete_all + + expect(LooseForeignKeys::DeletedRecord.count).to eq(3) + end + end +end diff --git a/spec/lib/gitlab/instrumentation/redis_spec.rb b/spec/lib/gitlab/instrumentation/redis_spec.rb index 6cddf958f2a..ebc2e92a0dd 100644 --- a/spec/lib/gitlab/instrumentation/redis_spec.rb +++ b/spec/lib/gitlab/instrumentation/redis_spec.rb @@ -28,6 +28,13 @@ RSpec.describe Gitlab::Instrumentation::Redis do describe '.payload', :request_store do before do + # If this is the first spec in a spec run that uses Redis, there + # will be an extra SELECT command to choose the right database. We + # don't want to make the spec less precise, so we force that to + # happen (if needed) first, then clear the counts. + Gitlab::Redis::Cache.with { |redis| redis.info } + RequestStore.clear! + Gitlab::Redis::Cache.with { |redis| redis.set('cache-test', 321) } Gitlab::Redis::SharedState.with { |redis| redis.set('shared-state-test', 123) } end diff --git a/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job_spec.rb b/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job_spec.rb index d67cb95f483..cc69a11f7f8 100644 --- a/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job_spec.rb +++ b/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job_spec.rb @@ -9,7 +9,14 @@ RSpec.describe Gitlab::SidekiqMiddleware::DuplicateJobs::DuplicateJob, :clean_gi described_class.new(job, queue) end - let(:job) { { 'class' => 'AuthorizedProjectsWorker', 'args' => [1], 'jid' => '123' } } + let(:wal_locations) do + { + main: '0/D525E3A8', + ci: 'AB/12345' + } + end + + let(:job) { { 'class' => 'AuthorizedProjectsWorker', 'args' => [1], 'jid' => '123', 'wal_locations' => wal_locations } } let(:queue) { 'authorized_projects' } let(:idempotency_key) do @@ -74,13 +81,39 @@ RSpec.describe Gitlab::SidekiqMiddleware::DuplicateJobs::DuplicateJob, :clean_gi context 'when there was no job in the queue yet' do it { expect(duplicate_job.check!).to eq('123') } - it "adds a key with ttl set to #{described_class::DUPLICATE_KEY_TTL}" do + it "adds a idempotency key with ttl set to #{described_class::DUPLICATE_KEY_TTL}" do expect { duplicate_job.check! } .to change { read_idempotency_key_with_ttl(idempotency_key) } .from([nil, -2]) .to(['123', be_within(1).of(described_class::DUPLICATE_KEY_TTL)]) end + context 'when wal locations is not empty' do + it "adds a existing wal locations key with ttl set to #{described_class::DUPLICATE_KEY_TTL}" do + expect { duplicate_job.check! } + .to change { read_idempotency_key_with_ttl(existing_wal_location_key(idempotency_key, :main)) } + .from([nil, -2]) + .to([wal_locations[:main], be_within(1).of(described_class::DUPLICATE_KEY_TTL)]) + .and change { read_idempotency_key_with_ttl(existing_wal_location_key(idempotency_key, :ci)) } + .from([nil, -2]) + .to([wal_locations[:ci], be_within(1).of(described_class::DUPLICATE_KEY_TTL)]) + end + end + + context 'when preserve_latest_wal_locations_for_idempotent_jobs feature flag is disabled' do + before do + stub_feature_flags(preserve_latest_wal_locations_for_idempotent_jobs: false) + end + + it "does not change the existing wal locations key's TTL" do + expect { duplicate_job.check! } + .to not_change { read_idempotency_key_with_ttl(existing_wal_location_key(idempotency_key, :main)) } + .from([nil, -2]) + .and not_change { read_idempotency_key_with_ttl(existing_wal_location_key(idempotency_key, :ci)) } + .from([nil, -2]) + end + end + it "adds the idempotency key to the jobs payload" do expect { duplicate_job.check! }.to change { job['idempotency_key'] }.from(nil).to(idempotency_key) end @@ -89,6 +122,9 @@ RSpec.describe Gitlab::SidekiqMiddleware::DuplicateJobs::DuplicateJob, :clean_gi context 'when there was already a job with same arguments in the same queue' do before do set_idempotency_key(idempotency_key, 'existing-key') + wal_locations.each do |config_name, location| + set_idempotency_key(existing_wal_location_key(idempotency_key, config_name), location) + end end it { expect(duplicate_job.check!).to eq('existing-key') } @@ -99,6 +135,14 @@ RSpec.describe Gitlab::SidekiqMiddleware::DuplicateJobs::DuplicateJob, :clean_gi .from(['existing-key', -1]) end + it "does not change the existing wal locations key's TTL" do + expect { duplicate_job.check! } + .to not_change { read_idempotency_key_with_ttl(existing_wal_location_key(idempotency_key, :main)) } + .from([wal_locations[:main], -1]) + .and not_change { read_idempotency_key_with_ttl(existing_wal_location_key(idempotency_key, :ci)) } + .from([wal_locations[:ci], -1]) + end + it 'sets the existing jid' do duplicate_job.check! @@ -107,6 +151,117 @@ RSpec.describe Gitlab::SidekiqMiddleware::DuplicateJobs::DuplicateJob, :clean_gi end end + describe '#update_latest_wal_location!' do + let(:offset) { '1024' } + + before do + allow(duplicate_job).to receive(:pg_wal_lsn_diff).with(:main).and_return(offset) + allow(duplicate_job).to receive(:pg_wal_lsn_diff).with(:ci).and_return(offset) + end + + shared_examples 'updates wal location' do + it 'updates a wal location to redis with an offset' do + expect { duplicate_job.update_latest_wal_location! } + .to change { read_range_from_redis(wal_location_key(idempotency_key, :main)) } + .from(existing_wal_with_offset[:main]) + .to(new_wal_with_offset[:main]) + .and change { read_range_from_redis(wal_location_key(idempotency_key, :ci)) } + .from(existing_wal_with_offset[:ci]) + .to(new_wal_with_offset[:ci]) + end + end + + context 'when preserve_latest_wal_locations_for_idempotent_jobs feature flag is disabled' do + before do + stub_feature_flags(preserve_latest_wal_locations_for_idempotent_jobs: false) + end + + it "doesn't call Sidekiq.redis" do + expect(Sidekiq).not_to receive(:redis) + + duplicate_job.update_latest_wal_location! + end + + it "doesn't update a wal location to redis with an offset" do + expect { duplicate_job.update_latest_wal_location! } + .to not_change { read_range_from_redis(wal_location_key(idempotency_key, :main)) } + .from([]) + .and not_change { read_range_from_redis(wal_location_key(idempotency_key, :ci)) } + .from([]) + end + end + + context "when the key doesn't exists in redis" do + include_examples 'updates wal location' do + let(:existing_wal_with_offset) { { main: [], ci: [] } } + let(:new_wal_with_offset) { wal_locations.transform_values { |v| [v, offset] } } + end + end + + context "when the key exists in redis" do + let(:existing_offset) { '1023'} + let(:existing_wal_locations) do + { + main: '0/D525E3NM', + ci: 'AB/111112' + } + end + + before do + rpush_to_redis_key(wal_location_key(idempotency_key, :main), existing_wal_locations[:main], existing_offset) + rpush_to_redis_key(wal_location_key(idempotency_key, :ci), existing_wal_locations[:ci], existing_offset) + end + + context "when the new offset is bigger then the existing one" do + include_examples 'updates wal location' do + let(:existing_wal_with_offset) { existing_wal_locations.transform_values { |v| [v, existing_offset] } } + let(:new_wal_with_offset) { wal_locations.transform_values { |v| [v, offset] } } + end + end + + context "when the old offset is not bigger then the existing one" do + let(:existing_offset) { offset } + + it "does not update a wal location to redis with an offset" do + expect { duplicate_job.update_latest_wal_location! } + .to not_change { read_range_from_redis(wal_location_key(idempotency_key, :main)) } + .from([existing_wal_locations[:main], existing_offset]) + .and not_change { read_range_from_redis(wal_location_key(idempotency_key, :ci)) } + .from([existing_wal_locations[:ci], existing_offset]) + end + end + end + end + + describe '#latest_wal_locations' do + context 'when job was deduplicated and wal locations were already persisted' do + before do + rpush_to_redis_key(wal_location_key(idempotency_key, :main), wal_locations[:main], 1024) + rpush_to_redis_key(wal_location_key(idempotency_key, :ci), wal_locations[:ci], 1024) + end + + it { expect(duplicate_job.latest_wal_locations).to eq(wal_locations) } + end + + context 'when job is not deduplication and wal locations were not persisted' do + it { expect(duplicate_job.latest_wal_locations).to be_empty } + end + + context 'when preserve_latest_wal_locations_for_idempotent_jobs feature flag is disabled' do + before do + stub_feature_flags(preserve_latest_wal_locations_for_idempotent_jobs: false) + end + + it "doesn't call Sidekiq.redis" do + expect(Sidekiq).not_to receive(:redis) + + duplicate_job.latest_wal_locations + end + + it { expect(duplicate_job.latest_wal_locations).to eq({}) } + end + end + describe '#delete!' do context "when we didn't track the definition" do it { expect { duplicate_job.delete! }.not_to raise_error } @@ -115,14 +270,79 @@ RSpec.describe Gitlab::SidekiqMiddleware::DuplicateJobs::DuplicateJob, :clean_gi context 'when the key exists in redis' do before do set_idempotency_key(idempotency_key, 'existing-jid') + wal_locations.each do |config_name, location| + set_idempotency_key(existing_wal_location_key(idempotency_key, config_name), location) + set_idempotency_key(wal_location_key(idempotency_key, config_name), location) + end end shared_examples 'deleting the duplicate job' do - it 'removes the key from redis' do - expect { duplicate_job.delete! } - .to change { read_idempotency_key_with_ttl(idempotency_key) } - .from(['existing-jid', -1]) - .to([nil, -2]) + shared_examples 'deleting keys from redis' do |key_name| + it "removes the #{key_name} from redis" do + expect { duplicate_job.delete! } + .to change { read_idempotency_key_with_ttl(key) } + .from([from_value, -1]) + .to([nil, -2]) + end + end + + shared_examples 'does not delete key from redis' do |key_name| + it "does not remove the #{key_name} from redis" do + expect { duplicate_job.delete! } + .to not_change { read_idempotency_key_with_ttl(key) } + .from([from_value, -1]) + end + end + + it_behaves_like 'deleting keys from redis', 'idempotent key' do + let(:key) { idempotency_key } + let(:from_value) { 'existing-jid' } + end + + it_behaves_like 'deleting keys from redis', 'existing wal location keys for main database' do + let(:key) { existing_wal_location_key(idempotency_key, :main) } + let(:from_value) { wal_locations[:main] } + end + + it_behaves_like 'deleting keys from redis', 'existing wal location keys for ci database' do + let(:key) { existing_wal_location_key(idempotency_key, :ci) } + let(:from_value) { wal_locations[:ci] } + end + + it_behaves_like 'deleting keys from redis', 'latest wal location keys for main database' do + let(:key) { wal_location_key(idempotency_key, :main) } + let(:from_value) { wal_locations[:main] } + end + + it_behaves_like 'deleting keys from redis', 'latest wal location keys for ci database' do + let(:key) { wal_location_key(idempotency_key, :ci) } + let(:from_value) { wal_locations[:ci] } + end + + context 'when preserve_latest_wal_locations_for_idempotent_jobs feature flag is disabled' do + before do + stub_feature_flags(preserve_latest_wal_locations_for_idempotent_jobs: false) + end + + it_behaves_like 'does not delete key from redis', 'latest wal location keys for main database' do + let(:key) { existing_wal_location_key(idempotency_key, :main) } + let(:from_value) { wal_locations[:main] } + end + + it_behaves_like 'does not delete key from redis', 'latest wal location keys for ci database' do + let(:key) { existing_wal_location_key(idempotency_key, :ci) } + let(:from_value) { wal_locations[:ci] } + end + + it_behaves_like 'does not delete key from redis', 'latest wal location keys for main database' do + let(:key) { wal_location_key(idempotency_key, :main) } + let(:from_value) { wal_locations[:main] } + end + + it_behaves_like 'does not delete key from redis', 'latest wal location keys for ci database' do + let(:key) { wal_location_key(idempotency_key, :ci) } + let(:from_value) { wal_locations[:ci] } + end end end @@ -254,10 +474,22 @@ RSpec.describe Gitlab::SidekiqMiddleware::DuplicateJobs::DuplicateJob, :clean_gi end end + def existing_wal_location_key(idempotency_key, config_name) + "#{idempotency_key}:#{config_name}:existing_wal_location" + end + + def wal_location_key(idempotency_key, config_name) + "#{idempotency_key}:#{config_name}:wal_location" + end + def set_idempotency_key(key, value = '1') Sidekiq.redis { |r| r.set(key, value) } end + def rpush_to_redis_key(key, wal, offset) + Sidekiq.redis { |r| r.rpush(key, [wal, offset]) } + end + def read_idempotency_key_with_ttl(key) Sidekiq.redis do |redis| redis.pipelined do |p| @@ -266,4 +498,10 @@ RSpec.describe Gitlab::SidekiqMiddleware::DuplicateJobs::DuplicateJob, :clean_gi end end end + + def read_range_from_redis(key) + Sidekiq.redis do |redis| + redis.lrange(key, 0, -1) + end + end end diff --git a/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/until_executed_spec.rb b/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/until_executed_spec.rb index b3d463b6f6b..9772255fc50 100644 --- a/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/until_executed_spec.rb +++ b/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/until_executed_spec.rb @@ -7,6 +7,10 @@ RSpec.describe Gitlab::SidekiqMiddleware::DuplicateJobs::Strategies::UntilExecut describe '#perform' do let(:proc) { -> {} } + before do + allow(fake_duplicate_job).to receive(:latest_wal_locations).and_return( {} ) + end + it 'deletes the lock after executing' do expect(proc).to receive(:call).ordered expect(fake_duplicate_job).to receive(:delete!).ordered diff --git a/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/until_executing_spec.rb b/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/until_executing_spec.rb index d45b6c5fcd1..c4045b8c63b 100644 --- a/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/until_executing_spec.rb +++ b/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/until_executing_spec.rb @@ -7,6 +7,10 @@ RSpec.describe Gitlab::SidekiqMiddleware::DuplicateJobs::Strategies::UntilExecut describe '#perform' do let(:proc) { -> {} } + before do + allow(fake_duplicate_job).to receive(:latest_wal_locations).and_return( {} ) + end + it 'deletes the lock before executing' do expect(fake_duplicate_job).to receive(:delete!).ordered expect(proc).to receive(:call).ordered diff --git a/spec/mailers/emails/in_product_marketing_spec.rb b/spec/mailers/emails/in_product_marketing_spec.rb index 74354630ade..99beef92dea 100644 --- a/spec/mailers/emails/in_product_marketing_spec.rb +++ b/spec/mailers/emails/in_product_marketing_spec.rb @@ -23,7 +23,7 @@ RSpec.describe Emails::InProductMarketing do it 'sends to the right user with a link to unsubscribe' do aggregate_failures do - expect(subject).to deliver_to(user.notification_email) + expect(subject).to deliver_to(user.notification_email_or_default) expect(subject).to have_body_text(profile_notifications_url) end end diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index dd1b08b506f..f39037cf744 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -71,7 +71,7 @@ RSpec.describe Notify do it 'is sent to the assignee as the author' do aggregate_failures do expect_sender(current_user) - expect(subject).to deliver_to(recipient.notification_email) + expect(subject).to deliver_to(recipient.notification_email_or_default) end end end @@ -710,7 +710,7 @@ RSpec.describe Notify do it 'contains all the useful information' do to_emails = subject.header[:to].addrs.map(&:address) - expect(to_emails).to eq([recipient.notification_email]) + expect(to_emails).to eq([recipient.notification_email_or_default]) is_expected.to have_subject "Request to join the #{project.full_name} project" is_expected.to have_body_text project.full_name @@ -1047,7 +1047,7 @@ RSpec.describe Notify do it 'is sent to the given recipient as the author' do aggregate_failures do expect_sender(note_author) - expect(subject).to deliver_to(recipient.notification_email) + expect(subject).to deliver_to(recipient.notification_email_or_default) end end @@ -1204,7 +1204,7 @@ RSpec.describe Notify do it 'is sent to the given recipient as the author' do aggregate_failures do expect_sender(note_author) - expect(subject).to deliver_to(recipient.notification_email) + expect(subject).to deliver_to(recipient.notification_email_or_default) end end @@ -1341,7 +1341,7 @@ RSpec.describe Notify do it 'contains all the useful information' do to_emails = subject.header[:to].addrs.map(&:address) - expect(to_emails).to eq([recipient.notification_email]) + expect(to_emails).to eq([recipient.notification_email_or_default]) is_expected.to have_subject "Request to join the #{group.name} group" is_expected.to have_body_text group.name diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index d177f5380fb..d536a0783bc 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -82,7 +82,7 @@ RSpec.describe Group do group = build(:group, parent: build(:namespace)) expect(group).not_to be_valid - expect(group.errors[:parent_id].first).to eq('a group cannot have a user namespace as its parent') + expect(group.errors[:parent_id].first).to eq('user namespace cannot be the parent of another namespace') end it 'allows a group to have another group as its parent' do diff --git a/spec/models/integrations/pipelines_email_spec.rb b/spec/models/integrations/pipelines_email_spec.rb index 761049f25fe..afd9d71ebc4 100644 --- a/spec/models/integrations/pipelines_email_spec.rb +++ b/spec/models/integrations/pipelines_email_spec.rb @@ -48,7 +48,7 @@ RSpec.describe Integrations::PipelinesEmail, :mailer do end it 'sends email' do - emails = receivers.map { |r| double(notification_email: r) } + emails = receivers.map { |r| double(notification_email_or_default: r) } should_only_email(*emails, kind: :bcc) end diff --git a/spec/models/loose_foreign_keys/deleted_record_spec.rb b/spec/models/loose_foreign_keys/deleted_record_spec.rb new file mode 100644 index 00000000000..db2f8b4d2d3 --- /dev/null +++ b/spec/models/loose_foreign_keys/deleted_record_spec.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe LooseForeignKeys::DeletedRecord do + let_it_be(:deleted_record_1) { described_class.create!(created_at: 1.day.ago, deleted_table_name: 'projects', deleted_table_primary_key_value: 5) } + let_it_be(:deleted_record_2) { described_class.create!(created_at: 3.days.ago, deleted_table_name: 'projects', deleted_table_primary_key_value: 1) } + let_it_be(:deleted_record_3) { described_class.create!(created_at: 5.days.ago, deleted_table_name: 'projects', deleted_table_primary_key_value: 3) } + let_it_be(:deleted_record_4) { described_class.create!(created_at: 10.days.ago, deleted_table_name: 'projects', deleted_table_primary_key_value: 1) } # duplicate + + # skip created_at because it gets truncated after insert + def map_attributes(records) + records.pluck(:deleted_table_name, :deleted_table_primary_key_value) + end + + describe 'partitioning strategy' do + it 'has retain_non_empty_partitions option' do + expect(described_class.partitioning_strategy.retain_non_empty_partitions).to eq(true) + end + end + + describe '.load_batch' do + it 'loads records and orders them by creation date' do + records = described_class.load_batch(4) + + expect(map_attributes(records)).to eq([['projects', 1], ['projects', 3], ['projects', 1], ['projects', 5]]) + end + + it 'supports configurable batch size' do + records = described_class.load_batch(2) + + expect(map_attributes(records)).to eq([['projects', 1], ['projects', 3]]) + end + end + + describe '.delete_records' do + it 'deletes exactly one record' do + described_class.delete_records([deleted_record_2]) + + expect(described_class.count).to eq(3) + expect(described_class.find_by(created_at: deleted_record_2.created_at)).to eq(nil) + end + + it 'deletes two records' do + described_class.delete_records([deleted_record_2, deleted_record_4]) + + expect(described_class.count).to eq(2) + end + + it 'deletes all records' do + described_class.delete_records([deleted_record_1, deleted_record_2, deleted_record_3, deleted_record_4]) + + expect(described_class.count).to eq(0) + end + end +end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 9744d58abb9..51a26d82daa 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -36,27 +36,34 @@ RSpec.describe Namespace do it { is_expected.to validate_numericality_of(:max_artifacts_size).only_integer.is_greater_than(0) } context 'validating the parent of a namespace' do - context 'when the namespace has no parent' do - it 'allows a namespace to have no parent associated with it' do - namespace = build(:namespace) - - expect(namespace).to be_valid - end + using RSpec::Parameterized::TableSyntax + + where(:parent_type, :child_type, :error) do + nil | 'User' | nil + nil | 'Group' | nil + nil | 'Project' | 'must be set for a project namespace' + 'Project' | 'User' | 'project namespace cannot be the parent of another namespace' + 'Project' | 'Group' | 'project namespace cannot be the parent of another namespace' + 'Project' | 'Project' | 'project namespace cannot be the parent of another namespace' + 'Group' | 'User' | 'cannot not be used for user namespace' + 'Group' | 'Group' | nil + 'Group' | 'Project' | nil + 'User' | 'User' | 'cannot not be used for user namespace' + 'User' | 'Group' | 'user namespace cannot be the parent of another namespace' + 'User' | 'Project' | nil end - context 'when the namespace has a parent' do - it 'does not allow a namespace to have a group as its parent' do - namespace = build(:namespace, parent: build(:group)) - - expect(namespace).not_to be_valid - expect(namespace.errors[:parent_id].first).to eq('a user namespace cannot have a parent') - end - - it 'does not allow a namespace to have another namespace as its parent' do - namespace = build(:namespace, parent: build(:namespace)) - - expect(namespace).not_to be_valid - expect(namespace.errors[:parent_id].first).to eq('a user namespace cannot have a parent') + with_them do + it 'validates namespace parent' do + parent = build(:namespace, type: parent_type) if parent_type + namespace = build(:namespace, type: child_type, parent: parent) + + if error + expect(namespace).not_to be_valid + expect(namespace.errors[:parent_id].first).to eq(error) + else + expect(namespace).to be_valid + end end end @@ -159,7 +166,8 @@ RSpec.describe Namespace do describe 'handling STI', :aggregate_failures do let(:namespace_type) { nil } - let(:namespace) { Namespace.find(create(:namespace, type: namespace_type).id) } + let(:parent) { nil } + let(:namespace) { Namespace.find(create(:namespace, type: namespace_type, parent: parent).id) } context 'creating a Group' do let(:namespace_type) { 'Group' } @@ -173,6 +181,7 @@ RSpec.describe Namespace do context 'creating a ProjectNamespace' do let(:namespace_type) { 'Project' } + let(:parent) { create(:group) } it 'is valid' do expect(Namespace.find(namespace.id)).to be_a(Namespaces::ProjectNamespace) diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 9c2f269de46..dc55214c1dd 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -1826,7 +1826,7 @@ RSpec.describe Repository do expect(merge_commit.message).to eq('Custom message') expect(merge_commit.author_name).to eq(user.name) - expect(merge_commit.author_email).to eq(user.commit_email) + expect(merge_commit.author_email).to eq(user.commit_email_or_default) expect(repository.blob_at(merge_commit.id, 'files/ruby/feature.rb')).to be_present end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index c9644b0e67a..334f9b4ae30 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -435,12 +435,12 @@ RSpec.describe User do subject { create(:user).tap { |user| user.emails << build(:email, email: email_value, confirmed_at: Time.current) } } end - describe '#commit_email' do + describe '#commit_email_or_default' do subject(:user) { create(:user) } it 'defaults to the primary email' do expect(user.email).to be_present - expect(user.commit_email).to eq(user.email) + expect(user.commit_email_or_default).to eq(user.email) end it 'defaults to the primary email when the column in the database is null' do @@ -448,14 +448,18 @@ RSpec.describe User do found_user = described_class.find_by(id: user.id) - expect(found_user.commit_email).to eq(user.email) + expect(found_user.commit_email_or_default).to eq(user.email) end it 'returns the private commit email when commit_email has _private' do user.update_column(:commit_email, Gitlab::PrivateCommitEmail::TOKEN) - expect(user.commit_email).to eq(user.private_commit_email) + expect(user.commit_email_or_default).to eq(user.private_commit_email) end + end + + describe '#commit_email=' do + subject(:user) { create(:user) } it 'can be set to a confirmed email' do confirmed = create(:email, :confirmed, user: user) @@ -699,6 +703,15 @@ RSpec.describe User do expect(user).to be_valid end end + + context 'when commit_email is changed to _private' do + it 'passes user validations' do + user = create(:user) + user.commit_email = '_private' + + expect(user).to be_valid + end + end end end @@ -1246,53 +1259,57 @@ RSpec.describe User do end end - describe '#update_notification_email' do - # Regression: https://gitlab.com/gitlab-org/gitlab-foss/issues/22846 - context 'when changing :email' do - let(:user) { create(:user) } - let(:new_email) { 'new-email@example.com' } + describe 'when changing email' do + let(:user) { create(:user) } + let(:new_email) { 'new-email@example.com' } + context 'if notification_email was nil' do it 'sets :unconfirmed_email' do expect do user.tap { |u| u.update!(email: new_email) }.reload end.to change(user, :unconfirmed_email).to(new_email) end - it 'does not change :notification_email' do + + it 'does not change notification_email or notification_email_or_default before email is confirmed' do expect do user.tap { |u| u.update!(email: new_email) }.reload - end.not_to change(user, :notification_email) + end.not_to change(user, :notification_email_or_default) + + expect(user.notification_email).to be_nil end - it 'updates :notification_email to the new email once confirmed' do + it 'updates notification_email_or_default to the new email once confirmed' do user.update!(email: new_email) expect do user.tap(&:confirm).reload - end.to change(user, :notification_email).to eq(new_email) + end.to change(user, :notification_email_or_default).to eq(new_email) + + expect(user.notification_email).to be_nil end + end - context 'and :notification_email is set to a secondary email' do - let!(:email_attrs) { attributes_for(:email, :confirmed, user: user) } - let(:secondary) { create(:email, :confirmed, email: 'secondary@example.com', user: user) } + context 'when notification_email is set to a secondary email' do + let!(:email_attrs) { attributes_for(:email, :confirmed, user: user) } + let(:secondary) { create(:email, :confirmed, email: 'secondary@example.com', user: user) } - before do - user.emails.create!(email_attrs) - user.tap { |u| u.update!(notification_email: email_attrs[:email]) }.reload - end + before do + user.emails.create!(email_attrs) + user.tap { |u| u.update!(notification_email: email_attrs[:email]) }.reload + end - it 'does not change :notification_email to :email' do - expect do - user.tap { |u| u.update!(email: new_email) }.reload - end.not_to change(user, :notification_email) - end + it 'does not change notification_email to email before email is confirmed' do + expect do + user.tap { |u| u.update!(email: new_email) }.reload + end.not_to change(user, :notification_email) + end - it 'does not change :notification_email to :email once confirmed' do - user.update!(email: new_email) + it 'does not change notification_email to email once confirmed' do + user.update!(email: new_email) - expect do - user.tap(&:confirm).reload - end.not_to change(user, :notification_email) - end + expect do + user.tap(&:confirm).reload + end.not_to change(user, :notification_email) end end end @@ -1878,6 +1895,7 @@ RSpec.describe User do end it 'does not send deactivated user an email' do expect(NotificationService).not_to receive(:new) + user.deactivate end end @@ -1885,7 +1903,7 @@ RSpec.describe User do context "when user deactivation emails are enabled" do it 'sends deactivated user an email' do expect_next_instance_of(NotificationService) do |notification| - allow(notification).to receive(:user_deactivated).with(user.name, user.notification_email) + allow(notification).to receive(:user_deactivated).with(user.name, user.notification_email_or_default) end user.deactivate @@ -3084,15 +3102,15 @@ RSpec.describe User do end end - describe '#notification_email' do + describe '#notification_email_or_default' do let(:email) { 'gonzo@muppets.com' } context 'when the column in the database is null' do subject { create(:user, email: email, notification_email: nil) } it 'defaults to the primary email' do - expect(subject.read_attribute(:notification_email)).to be nil - expect(subject.notification_email).to eq(email) + expect(subject.notification_email).to be nil + expect(subject.notification_email_or_default).to eq(email) end end end @@ -5335,7 +5353,7 @@ RSpec.describe User do let(:group) { nil } it 'returns global notification email' do - is_expected.to eq(user.notification_email) + is_expected.to eq(user.notification_email_or_default) end end @@ -5343,7 +5361,7 @@ RSpec.describe User do it 'returns global notification email' do create(:notification_setting, user: user, source: group, notification_email: '') - is_expected.to eq(user.notification_email) + is_expected.to eq(user.notification_email_or_default) end end @@ -6132,7 +6150,7 @@ RSpec.describe User do it 'does nothing' do expect(subject).not_to receive(:save) subject.unset_secondary_emails_matching_deleted_email!(deleted_email) - expect(subject.read_attribute(:commit_email)).to eq commit_email + expect(subject.commit_email).to eq commit_email end end @@ -6142,7 +6160,7 @@ RSpec.describe User do it 'un-sets the secondary email' do expect(subject).to receive(:save) subject.unset_secondary_emails_matching_deleted_email!(deleted_email) - expect(subject.read_attribute(:commit_email)).to be nil + expect(subject.commit_email).to be nil end end end diff --git a/spec/requests/api/notification_settings_spec.rb b/spec/requests/api/notification_settings_spec.rb index 7b4a58e63da..b5551c21738 100644 --- a/spec/requests/api/notification_settings_spec.rb +++ b/spec/requests/api/notification_settings_spec.rb @@ -13,7 +13,7 @@ RSpec.describe API::NotificationSettings do expect(response).to have_gitlab_http_status(:ok) expect(json_response).to be_a Hash - expect(json_response['notification_email']).to eq(user.notification_email) + expect(json_response['notification_email']).to eq(user.notification_email_or_default) expect(json_response['level']).to eq(user.global_notification_setting.level) end end diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 8b864346c5d..ac86c922813 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -1202,7 +1202,7 @@ RSpec.describe API::Users do it 'updates user with a new email' do old_email = user.email - old_notification_email = user.notification_email + old_notification_email = user.notification_email_or_default put api("/users/#{user.id}", admin), params: { email: 'new@email.com' } user.reload @@ -1210,7 +1210,7 @@ RSpec.describe API::Users do expect(response).to have_gitlab_http_status(:ok) expect(user).to be_confirmed expect(user.email).to eq(old_email) - expect(user.notification_email).to eq(old_notification_email) + expect(user.notification_email_or_default).to eq(old_notification_email) expect(user.unconfirmed_email).to eq('new@email.com') end diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 3c4d7d50002..a03f1f17b39 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -2623,7 +2623,7 @@ RSpec.describe NotificationService, :mailer do let_it_be(:user) { create(:user) } it 'sends the user an email' do - notification.user_deactivated(user.name, user.notification_email) + notification.user_deactivated(user.name, user.notification_email_or_default) should_only_email(user) end diff --git a/spec/services/suggestions/apply_service_spec.rb b/spec/services/suggestions/apply_service_spec.rb index d3dcbf0b668..dc330a5546f 100644 --- a/spec/services/suggestions/apply_service_spec.rb +++ b/spec/services/suggestions/apply_service_spec.rb @@ -70,7 +70,7 @@ RSpec.describe Suggestions::ApplyService do author = suggestions.first.note.author expect(user.commit_email).not_to eq(user.email) - expect(commit.author_email).to eq(author.commit_email) + expect(commit.author_email).to eq(author.commit_email_or_default) expect(commit.committer_email).to eq(user.commit_email) expect(commit.author_name).to eq(author.name) expect(commit.committer_name).to eq(user.name) @@ -333,9 +333,9 @@ RSpec.describe Suggestions::ApplyService do end it 'created commit by same author and committer' do - expect(user.commit_email).to eq(author.commit_email) + expect(user.commit_email).to eq(author.commit_email_or_default) expect(author).to eq(user) - expect(commit.author_email).to eq(author.commit_email) + expect(commit.author_email).to eq(author.commit_email_or_default) expect(commit.committer_email).to eq(user.commit_email) expect(commit.author_name).to eq(author.name) expect(commit.committer_name).to eq(user.name) @@ -350,7 +350,7 @@ RSpec.describe Suggestions::ApplyService do it 'created commit has authors info and commiters info' do expect(user.commit_email).not_to eq(user.email) expect(author).not_to eq(user) - expect(commit.author_email).to eq(author.commit_email) + expect(commit.author_email).to eq(author.commit_email_or_default) expect(commit.committer_email).to eq(user.commit_email) expect(commit.author_name).to eq(author.name) expect(commit.committer_name).to eq(user.name) @@ -359,7 +359,7 @@ RSpec.describe Suggestions::ApplyService do end context 'multiple suggestions' do - let(:author_emails) { suggestions.map {|s| s.note.author.commit_email } } + let(:author_emails) { suggestions.map {|s| s.note.author.commit_email_or_default } } let(:first_author) { suggestion.note.author } let(:commit) { project.repository.commit } @@ -369,8 +369,8 @@ RSpec.describe Suggestions::ApplyService do end it 'uses first authors information' do - expect(author_emails).to include(first_author.commit_email).exactly(3) - expect(commit.author_email).to eq(first_author.commit_email) + expect(author_emails).to include(first_author.commit_email_or_default).exactly(3) + expect(commit.author_email).to eq(first_author.commit_email_or_default) end end diff --git a/spec/services/users/reject_service_spec.rb b/spec/services/users/reject_service_spec.rb index 0e34f0e67ba..5a243e876ac 100644 --- a/spec/services/users/reject_service_spec.rb +++ b/spec/services/users/reject_service_spec.rb @@ -44,7 +44,7 @@ RSpec.describe Users::RejectService do it 'emails the user on rejection' do expect_next_instance_of(NotificationService) do |notification| - allow(notification).to receive(:user_admin_rejection).with(user.name, user.notification_email) + allow(notification).to receive(:user_admin_rejection).with(user.name, user.notification_email_or_default) end subject diff --git a/spec/support/helpers/email_helpers.rb b/spec/support/helpers/email_helpers.rb index 6df33e68629..d0f6fd466d0 100644 --- a/spec/support/helpers/email_helpers.rb +++ b/spec/support/helpers/email_helpers.rb @@ -2,7 +2,7 @@ module EmailHelpers def sent_to_user(user, recipients: email_recipients) - recipients.count { |to| to == user.notification_email } + recipients.count { |to| to == user.notification_email_or_default } end def reset_delivered_emails! @@ -45,7 +45,7 @@ module EmailHelpers end def find_email_for(user) - ActionMailer::Base.deliveries.find { |d| d.to.include?(user.notification_email) } + ActionMailer::Base.deliveries.find { |d| d.to.include?(user.notification_email_or_default) } end def have_referable_subject(referable, include_project: true, reply: false) diff --git a/spec/support/shared_examples/lib/gitlab/sidekiq_middleware/strategy_shared_examples.rb b/spec/support/shared_examples/lib/gitlab/sidekiq_middleware/strategy_shared_examples.rb index 89b793d5e16..708bc71ae96 100644 --- a/spec/support/shared_examples/lib/gitlab/sidekiq_middleware/strategy_shared_examples.rb +++ b/spec/support/shared_examples/lib/gitlab/sidekiq_middleware/strategy_shared_examples.rb @@ -39,6 +39,7 @@ RSpec.shared_examples 'deduplicating jobs when scheduling' do |strategy_name| allow(fake_duplicate_job).to receive(:scheduled?).and_return(false) allow(fake_duplicate_job).to receive(:check!).and_return('the jid') allow(fake_duplicate_job).to receive(:idempotent?).and_return(true) + allow(fake_duplicate_job).to receive(:update_latest_wal_location!) allow(fake_duplicate_job).to receive(:options).and_return({}) job_hash = {} @@ -63,6 +64,7 @@ RSpec.shared_examples 'deduplicating jobs when scheduling' do |strategy_name| .with(Gitlab::SidekiqMiddleware::DuplicateJobs::DuplicateJob::DUPLICATE_KEY_TTL) .and_return('the jid')) allow(fake_duplicate_job).to receive(:idempotent?).and_return(true) + allow(fake_duplicate_job).to receive(:update_latest_wal_location!) job_hash = {} expect(fake_duplicate_job).to receive(:duplicate?).and_return(true) @@ -83,6 +85,7 @@ RSpec.shared_examples 'deduplicating jobs when scheduling' do |strategy_name| allow(fake_duplicate_job).to( receive(:check!).with(time_diff.to_i).and_return('the jid')) allow(fake_duplicate_job).to receive(:idempotent?).and_return(true) + allow(fake_duplicate_job).to receive(:update_latest_wal_location!) job_hash = {} expect(fake_duplicate_job).to receive(:duplicate?).and_return(true) @@ -105,6 +108,13 @@ RSpec.shared_examples 'deduplicating jobs when scheduling' do |strategy_name| allow(fake_duplicate_job).to receive(:options).and_return({}) allow(fake_duplicate_job).to receive(:existing_jid).and_return('the jid') allow(fake_duplicate_job).to receive(:idempotent?).and_return(true) + allow(fake_duplicate_job).to receive(:update_latest_wal_location!) + end + + it 'updates latest wal location' do + expect(fake_duplicate_job).to receive(:update_latest_wal_location!) + + strategy.schedule({ 'jid' => 'new jid' }) {} end it 'drops the job' do @@ -136,4 +146,46 @@ RSpec.shared_examples 'deduplicating jobs when scheduling' do |strategy_name| end end end + + describe '#perform' do + let(:proc) { -> {} } + let(:job) { { 'jid' => 'new jid', 'wal_locations' => { 'main' => '0/1234', 'ci' => '0/1234' } } } + let(:wal_locations) do + { + main: '0/D525E3A8', + ci: 'AB/12345' + } + end + + before do + allow(fake_duplicate_job).to receive(:delete!) + allow(fake_duplicate_job).to receive(:latest_wal_locations).and_return( wal_locations ) + end + + it 'updates job hash with dedup_wal_locations' do + strategy.perform(job) do + proc.call + end + + expect(job['dedup_wal_locations']).to eq(wal_locations) + end + + shared_examples 'does not update job hash' do + it 'does not update job hash with dedup_wal_locations' do + strategy.perform(job) do + proc.call + end + + expect(job).not_to include('dedup_wal_locations') + end + end + + context 'when latest_wal_location is empty' do + before do + allow(fake_duplicate_job).to receive(:latest_wal_locations).and_return( {} ) + end + + include_examples 'does not update job hash' + end + end end diff --git a/spec/support/shared_examples/mailers/notify_shared_examples.rb b/spec/support/shared_examples/mailers/notify_shared_examples.rb index b10ebb4d2a3..e1f7a9030e2 100644 --- a/spec/support/shared_examples/mailers/notify_shared_examples.rb +++ b/spec/support/shared_examples/mailers/notify_shared_examples.rb @@ -2,7 +2,7 @@ RSpec.shared_examples 'a multiple recipients email' do it 'is sent to the given recipient' do - is_expected.to deliver_to recipient.notification_email + is_expected.to deliver_to recipient.notification_email_or_default end end @@ -21,7 +21,7 @@ end RSpec.shared_examples 'an email sent to a user' do it 'is sent to user\'s global notification email address' do - expect(subject).to deliver_to(recipient.notification_email) + expect(subject).to deliver_to(recipient.notification_email_or_default) end context 'with group notification email' do @@ -227,7 +227,7 @@ RSpec.shared_examples 'a note email' do aggregate_failures do expect(sender.display_name).to eq("#{note_author.name} (@#{note_author.username})") expect(sender.address).to eq(gitlab_sender) - expect(subject).to deliver_to(recipient.notification_email) + expect(subject).to deliver_to(recipient.notification_email_or_default) end end diff --git a/spec/workers/concerns/worker_attributes_spec.rb b/spec/workers/concerns/worker_attributes_spec.rb index d4b17c65f46..ad9d5eeccbe 100644 --- a/spec/workers/concerns/worker_attributes_spec.rb +++ b/spec/workers/concerns/worker_attributes_spec.rb @@ -35,45 +35,17 @@ RSpec.describe WorkerAttributes do end end - context 'when job is idempotent' do - context 'when data_consistency is not :always' do - it 'raise exception' do - worker.idempotent! - - expect { worker.data_consistency(:sticky) } - .to raise_error("Class can't be marked as idempotent if data_consistency is not set to :always") - end - end - - context 'when feature_flag is provided' do - before do - stub_feature_flags(test_feature_flag: false) - skip_feature_flags_yaml_validation - skip_default_enabled_yaml_check - end - - it 'returns correct feature flag value' do - worker.data_consistency(:sticky, feature_flag: :test_feature_flag) - - expect(worker.get_data_consistency_feature_flag_enabled?).not_to be_truthy - end + context 'when feature_flag is provided' do + before do + stub_feature_flags(test_feature_flag: false) + skip_feature_flags_yaml_validation + skip_default_enabled_yaml_check end - end - end - - describe '.idempotent!' do - it 'sets `idempotent` attribute of the worker class to true' do - worker.idempotent! - expect(worker.send(:class_attributes)[:idempotent]).to eq(true) - end - - context 'when data consistency is not :always' do - it 'raise exception' do - worker.data_consistency(:sticky) + it 'returns correct feature flag value' do + worker.data_consistency(:sticky, feature_flag: :test_feature_flag) - expect { worker.idempotent! } - .to raise_error("Class can't be marked as idempotent if data_consistency is not set to :always") + expect(worker.get_data_consistency_feature_flag_enabled?).not_to be_truthy end end end diff --git a/yarn.lock b/yarn.lock index 62b4fc6b499..5c3a022ca35 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1467,10 +1467,10 @@ dom-accessibility-api "^0.5.1" pretty-format "^26.4.2" -"@tiptap/core@^2.0.0-beta.103": - version "2.0.0-beta.103" - resolved "https://registry.yarnpkg.com/@tiptap/core/-/core-2.0.0-beta.103.tgz#f88f0217a26a3dc5413f44ffff246c4692709afb" - integrity sha512-tCU94zEJgv/mADMZ0dM1/+23uJoc2YyrRauEekt6Yd44TdPTJSDk7v82GJdmMLWj5mDjLbXVL7kvrJpi0/4zjg== +"@tiptap/core@^2.0.0-beta.105": + version "2.0.0-beta.105" + resolved "https://registry.yarnpkg.com/@tiptap/core/-/core-2.0.0-beta.105.tgz#3d758cbbf3e8c9b806d1017cd2e7444f192d8109" + integrity sha512-ut0ts9hrJXUJlSXZIN8iEt2TPbqYLFBanucUAr0ENLjIlpyyNVXz9IhS3bBEmo7vWExirZmY8O9oidQztesf2A== dependencies: "@types/prosemirror-commands" "^1.0.4" "@types/prosemirror-inputrules" "^1.0.4" @@ -1517,22 +1517,22 @@ dependencies: prosemirror-inputrules "^1.1.3" -"@tiptap/extension-code-block-lowlight@2.0.0-beta.36": - version "2.0.0-beta.36" - resolved "https://registry.yarnpkg.com/@tiptap/extension-code-block-lowlight/-/extension-code-block-lowlight-2.0.0-beta.36.tgz#f4d5fed298d376fb2dc33955a67e55ebcb04bc8d" - integrity sha512-Z/Oza3eaBJKYcE76d0zU5ScnQU31KAmzwb16G+lUKU5oYxGvWWLgseYiHdU1oQinf9FC1c7asMwI6x3ydkKIlA== +"@tiptap/extension-code-block-lowlight@2.0.0-beta.37": + version "2.0.0-beta.37" + resolved "https://registry.yarnpkg.com/@tiptap/extension-code-block-lowlight/-/extension-code-block-lowlight-2.0.0-beta.37.tgz#672b2f21d49077285a507c2e204b07085df93906" + integrity sha512-EN8MoCZKB23nHNqgvB/wOS84ySUY9ahB6oao7wDpKAYqBkAF/hEXmDsylDySvyCKXf824lS/vztfaF0hHQbQ/g== dependencies: - "@tiptap/extension-code-block" "^2.0.0-beta.17" + "@tiptap/extension-code-block" "^2.0.0-beta.18" "@types/lowlight" "^0.0.3" lowlight "^1.20.0" prosemirror-model "^1.14.3" prosemirror-state "^1.3.4" prosemirror-view "^1.20.0" -"@tiptap/extension-code-block@^2.0.0-beta.17": - version "2.0.0-beta.17" - resolved "https://registry.yarnpkg.com/@tiptap/extension-code-block/-/extension-code-block-2.0.0-beta.17.tgz#b12ab35561da08b359f4d8dced2b8c30eb62fcdb" - integrity sha512-u3RY991mXtjuw+trVaDwbAhuPPlU8l6kS4rXIxWJ5W/sNElbmfHLVu7RP++YwM8KOQrCrQl8TJbZTEIekMw61w== +"@tiptap/extension-code-block@^2.0.0-beta.18": + version "2.0.0-beta.18" + resolved "https://registry.yarnpkg.com/@tiptap/extension-code-block/-/extension-code-block-2.0.0-beta.18.tgz#3b43730cfca1ba26171530951b3cc324a500cb11" + integrity sha512-E2gz7ovl9nXLZzheqLyN3hi7A10fCaodDn4DvIl4wiEbKZpF7WFBNeb+FQetWNay9UWNeDO94SCX9+rT9H+yHA== dependencies: prosemirror-inputrules "^1.1.3" @@ -1608,10 +1608,10 @@ resolved "https://registry.yarnpkg.com/@tiptap/extension-italic/-/extension-italic-2.0.0-beta.15.tgz#9a81f686cf221110478935596f0b47a76d4c2f45" integrity sha512-ZCz1vCysLdvOUrwODuyBP0BDaemCLh6ib7qTYoSDKdive9kfn0Vc5Fg3o8xgHrtrUfwKIJz/sWOknjDEGIc9cw== -"@tiptap/extension-link@^2.0.0-beta.19": - version "2.0.0-beta.19" - resolved "https://registry.yarnpkg.com/@tiptap/extension-link/-/extension-link-2.0.0-beta.19.tgz#cba1bd266058fa25543d347a20fba1f9acef8538" - integrity sha512-XTj4w3TbkN8sGqoTr7XCtlyOojAkCWpufjLBpKmQnm2Cd8q6Gy7At0z93H0kx3vDKM8u1ELzXRsO/RiYrFK0lQ== +"@tiptap/extension-link@^2.0.0-beta.20": + version "2.0.0-beta.20" + resolved "https://registry.yarnpkg.com/@tiptap/extension-link/-/extension-link-2.0.0-beta.20.tgz#dbb2aa4f01212bbd0c3fb14b563e28f31140eae2" + integrity sha512-VljjF5Pmd8xeUxN+Wbypyh8zGEMBWJTwzHUVMbkAEU8IZTdk2gDDcAOiYPyU9+j0vAVbSXi1EAsBRIIlj0OaKQ== dependencies: prosemirror-state "^1.3.4" @@ -1620,10 +1620,10 @@ resolved "https://registry.yarnpkg.com/@tiptap/extension-list-item/-/extension-list-item-2.0.0-beta.14.tgz#65a9ff9daa11bc9ca8bc2989a891abe68081cfbd" integrity sha512-t6xwEqP+d5443Ul2Jvqz9kXb3ro7bA7yY9HA0vskm3120WxxHW9jxgxZN+82Ot5Tm7nXOAlsN6vuqnt4idnxZQ== -"@tiptap/extension-ordered-list@^2.0.0-beta.15": - version "2.0.0-beta.15" - resolved "https://registry.yarnpkg.com/@tiptap/extension-ordered-list/-/extension-ordered-list-2.0.0-beta.15.tgz#5645efe300489d5ea2ed7f98eaa84fbdb6951af8" - integrity sha512-j9Xh8CYtV+C/wrTXEWN+U7NJIQ/cQrjta80Mm2hFiE2KDtFNkpsPqG6UBoky04EPFphR5xDUsO1nCT7T7Tei5A== +"@tiptap/extension-ordered-list@^2.0.0-beta.16": + version "2.0.0-beta.16" + resolved "https://registry.yarnpkg.com/@tiptap/extension-ordered-list/-/extension-ordered-list-2.0.0-beta.16.tgz#3ef25a8dd8ddbd2b1aa5ce89d5a2e5a3ecafcf4e" + integrity sha512-3n0h5FBfQqBrN/zqF/Ngoyd1bZxeIRLwWI7ak4KulpvOg5V/yw3sw5CSxr2f13ZI9AgGaTq8yOsTYs9dkCCnsQ== dependencies: prosemirror-inputrules "^1.1.3" @@ -1647,15 +1647,15 @@ resolved "https://registry.yarnpkg.com/@tiptap/extension-superscript/-/extension-superscript-2.0.0-beta.4.tgz#16906d71dd8f9892101cf792f42005f8cd404516" integrity sha512-rTQCnSnloSf6UN1y3zhu6j41MxrcCVWm5JIPX8VEt60WsOXJLAc/YJHLYi2FWhh/Psq8k78sPrmZbjYUrj3Dkw== -"@tiptap/extension-table-cell@^2.0.0-beta.14": - version "2.0.0-beta.14" - resolved "https://registry.yarnpkg.com/@tiptap/extension-table-cell/-/extension-table-cell-2.0.0-beta.14.tgz#a164a82ba6e15ba95673a9a9d9cbd68fdaf0362b" - integrity sha512-VoZ8SF8urU1/I3XnjDez2pbDO0h4cow+o8Xi6mhkMTXbxr212T674W7lu5DVMJ4Y1NF1eLe9eJ9kGJE43cA0+Q== +"@tiptap/extension-table-cell@^2.0.0-beta.15": + version "2.0.0-beta.15" + resolved "https://registry.yarnpkg.com/@tiptap/extension-table-cell/-/extension-table-cell-2.0.0-beta.15.tgz#2059946b1657f0f22415bda84cee77fb1eb232b1" + integrity sha512-MyZHJlFOF2z4Y1DQ/uuFbDPYQxSrl/PW8TEpAh2UQI/PGVsKkjzHItTvLtWSgo7t+tuPHbEOvIBDfpQKSyfbWg== -"@tiptap/extension-table-header@^2.0.0-beta.16": - version "2.0.0-beta.16" - resolved "https://registry.yarnpkg.com/@tiptap/extension-table-header/-/extension-table-header-2.0.0-beta.16.tgz#7c200d8c64111b6d84f23888752b421b89c2428e" - integrity sha512-L/r+ez4G7ZlninS4MwK4tYoVuA6bMMCz7OLvHGBmp24vZm7OA3qyUP4+GGtOXtdhv3aydeBis03gSrrB50aRWw== +"@tiptap/extension-table-header@^2.0.0-beta.17": + version "2.0.0-beta.17" + resolved "https://registry.yarnpkg.com/@tiptap/extension-table-header/-/extension-table-header-2.0.0-beta.17.tgz#1bd008825146e6f5fc607a1d8682b5d47ba08f25" + integrity sha512-HctO608QQEPe29QBtxXcDfBKfv+m9u2jBJ5AYpo7HjYGfUYRxv5uQpGXX3RW/c50+AhyTF0skF6Td1jiZcEPPw== "@tiptap/extension-table-row@^2.0.0-beta.14": version "2.0.0-beta.14" @@ -1670,10 +1670,10 @@ prosemirror-tables "^1.1.1" prosemirror-view "^1.20.0" -"@tiptap/extension-task-item@^2.0.0-beta.17": - version "2.0.0-beta.17" - resolved "https://registry.yarnpkg.com/@tiptap/extension-task-item/-/extension-task-item-2.0.0-beta.17.tgz#e4e010c321b8f9aa5f49847c48e4e3a0695a47a9" - integrity sha512-uV5f9rWo9NAZ+oyt/mkhFEZfcngr92pVL2sn291RiT0qw9NRvdIQ6PLH24il9zu+jjeWmh1cJnBuQos+MQqjyg== +"@tiptap/extension-task-item@^2.0.0-beta.18": + version "2.0.0-beta.18" + resolved "https://registry.yarnpkg.com/@tiptap/extension-task-item/-/extension-task-item-2.0.0-beta.18.tgz#f109c15c997038d8099b64dba4cfc4e219b426e4" + integrity sha512-SmXWdfpDFIBGxWH4Xhb9d6jRSK7jJqsw0GqC5KAtIQ9gNvAQS1j5FqAtWfxlb8FkunIfV6MNnxDP2ZgbKvaEug== dependencies: prosemirror-inputrules "^1.1.3" @@ -4675,10 +4675,10 @@ dompurify@2.3.0: resolved "https://registry.yarnpkg.com/dompurify/-/dompurify-2.3.0.tgz#07bb39515e491588e5756b1d3e8375b5964814e2" integrity sha512-VV5C6Kr53YVHGOBKO/F86OYX6/iLTw2yVSI721gKetxpHCK/V5TaLEf9ODjRgl1KLSWRMY6cUhAbv/c+IUnwQw== -dompurify@^2.3.1: - version "2.3.1" - resolved "https://registry.yarnpkg.com/dompurify/-/dompurify-2.3.1.tgz#a47059ca21fd1212d3c8f71fdea6943b8bfbdf6a" - integrity sha512-xGWt+NHAQS+4tpgbOAI08yxW0Pr256Gu/FNE2frZVTbgrBUn8M7tz7/ktS/LZ2MHeGqz6topj0/xY+y8R5FBFw== +dompurify@^2.3.1, dompurify@^2.3.2: + version "2.3.2" + resolved "https://registry.yarnpkg.com/dompurify/-/dompurify-2.3.2.tgz#c773efa410abb5c087c7caf44934fefa448f6e60" + integrity sha512-jXJnvWloI+scD+N5uBikpUMsYXZb0LCAXxLFAOLS5duCzKfXLqBCpuINvFOiI4eJgTLggrngljT18HNoakHUsA== domutils@^1.5.1: version "1.7.0" |
