diff options
83 files changed, 751 insertions, 801 deletions
diff --git a/.gitlab/ci/frontend.gitlab-ci.yml b/.gitlab/ci/frontend.gitlab-ci.yml index bfc38e73bb5..bbf8a804666 100644 --- a/.gitlab/ci/frontend.gitlab-ci.yml +++ b/.gitlab/ci/frontend.gitlab-ci.yml @@ -298,9 +298,13 @@ coverage-frontend: - *yarn-install - run_timed_command "retry yarn run webpack-prod" -qa-frontend-node:10: +qa-frontend-node:12: extends: .qa-frontend-node - image: ${GITLAB_DEPENDENCY_PROXY}node:dubnium + image: ${GITLAB_DEPENDENCY_PROXY}node:12 + +qa-frontend-node:14: + extends: .qa-frontend-node + image: ${GITLAB_DEPENDENCY_PROXY}node:14 qa-frontend-node:latest: extends: diff --git a/.rubocop_manual_todo.yml b/.rubocop_manual_todo.yml index 96571d48b1c..78769217d7c 100644 --- a/.rubocop_manual_todo.yml +++ b/.rubocop_manual_todo.yml @@ -2548,8 +2548,6 @@ Gitlab/FeatureAvailableUsage: - 'ee/app/views/projects/settings/operations/_status_page.html.haml' - 'ee/app/views/projects/settings/repository/_protected_branches.html.haml' - 'ee/app/views/projects/sidebar/_repository_locked_files.html.haml' - - 'ee/app/views/shared/issuable/_board_create_list_dropdown.html.haml' - - 'ee/app/views/shared/issuable/_board_create_list_dropdown.html.haml' - 'ee/app/views/shared/issuable/_group_bulk_update_sidebar.html.haml' - 'ee/app/views/shared/issuable/form/_default_templates.html.haml' - 'ee/app/views/shared/labels/_create_label_help_text.html.haml' diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index c73df338ee2..66f8e86e48d 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -c0bef09e1ad59c7d119ecc5d58a8a4e6e98a8a65 +48d7984d9912c935a2c2abba3b55593cf0be2d8e diff --git a/app/assets/javascripts/boards/components/new_list_dropdown.js b/app/assets/javascripts/boards/components/new_list_dropdown.js deleted file mode 100644 index 6eb1dbfb46a..00000000000 --- a/app/assets/javascripts/boards/components/new_list_dropdown.js +++ /dev/null @@ -1,119 +0,0 @@ -/* eslint-disable func-names, no-new */ - -import $ from 'jquery'; -import store from '~/boards/stores'; -import initDeprecatedJQueryDropdown from '~/deprecated_jquery_dropdown'; -import createFlash from '~/flash'; -import { getIdFromGraphQLId } from '~/graphql_shared/utils'; -import axios from '~/lib/utils/axios_utils'; -import { __ } from '~/locale'; -import CreateLabelDropdown from '../../create_label'; -import { fullLabelId } from '../boards_util'; -import boardsStore from '../stores/boards_store'; - -function shouldCreateListGraphQL(label) { - return store.getters.shouldUseGraphQL && !store.getters.getListByLabelId(fullLabelId(label)); -} - -// eslint-disable-next-line @gitlab/no-global-event-off -$(document) - .off('created.label') - .on('created.label', (e, label, addNewList) => { - if (!addNewList) { - return; - } - - if (shouldCreateListGraphQL(label)) { - store.dispatch('createList', { labelId: fullLabelId(label) }); - } else { - boardsStore.new({ - title: label.title, - position: boardsStore.state.lists.length - 2, - list_type: 'label', - label: { - id: label.id, - title: label.title, - color: label.color, - }, - }); - } - }); - -export default function initNewListDropdown() { - $('.js-new-board-list').each(function () { - const $dropdownToggle = $(this); - const $dropdown = $dropdownToggle.closest('.dropdown'); - new CreateLabelDropdown( - $dropdown.find('.dropdown-new-label'), - $dropdownToggle.data('namespacePath'), - $dropdownToggle.data('projectPath'), - ); - - initDeprecatedJQueryDropdown($dropdownToggle, { - data(term, callback) { - const reqFailed = () => { - $dropdownToggle.data('bs.dropdown').hide(); - createFlash({ - message: __('Error fetching labels.'), - }); - }; - - if (store.getters.shouldUseGraphQL) { - store - .dispatch('fetchLabels') - .then((data) => callback(data)) - .catch(reqFailed); - } else { - axios - .get($dropdownToggle.attr('data-list-labels-path')) - .then(({ data }) => callback(data)) - .catch(reqFailed); - } - }, - renderRow(label) { - const active = store.getters.shouldUseGraphQL - ? store.getters.getListByLabelId(label.id) - : boardsStore.findListByLabelId(label.id); - const $li = $('<li />'); - const $a = $('<a />', { - class: active ? `is-active js-board-list-${getIdFromGraphQLId(active.id)}` : '', - text: label.title, - href: '#', - }); - const $labelColor = $('<span />', { - class: 'dropdown-label-box', - style: `background-color: ${label.color}`, - }); - - return $li.append($a.prepend($labelColor)); - }, - search: { - fields: ['title'], - }, - filterable: true, - selectable: true, - multiSelect: true, - containerSelector: '.js-tab-container-labels .dropdown-page-one .dropdown-content', - clicked(options) { - const { e } = options; - const label = options.selectedObj; - e.preventDefault(); - - if (shouldCreateListGraphQL(label)) { - store.dispatch('createList', { labelId: label.id }); - } else if (!boardsStore.findListByLabelId(label.id)) { - boardsStore.new({ - title: label.title, - position: boardsStore.state.lists.length - 2, - list_type: 'label', - label: { - id: label.id, - title: label.title, - color: label.color, - }, - }); - } - }, - }); - }); -} diff --git a/app/assets/javascripts/boards/index.js b/app/assets/javascripts/boards/index.js index de7c8a3bd6b..a5dcabdd729 100644 --- a/app/assets/javascripts/boards/index.js +++ b/app/assets/javascripts/boards/index.js @@ -7,12 +7,7 @@ import { mapActions, mapGetters } from 'vuex'; import 'ee_else_ce/boards/models/issue'; import 'ee_else_ce/boards/models/list'; import BoardSidebar from 'ee_else_ce/boards/components/board_sidebar'; -import initNewListDropdown from 'ee_else_ce/boards/components/new_list_dropdown'; -import { - setWeightFetchingState, - setEpicFetchingState, - getMilestoneTitle, -} from 'ee_else_ce/boards/ee_functions'; +import { setWeightFetchingState, setEpicFetchingState } from 'ee_else_ce/boards/ee_functions'; import toggleEpicsSwimlanes from 'ee_else_ce/boards/toggle_epics_swimlanes'; import toggleLabels from 'ee_else_ce/boards/toggle_labels'; import BoardAddNewColumnTrigger from '~/boards/components/board_add_new_column_trigger.vue'; @@ -313,20 +308,6 @@ export default () => { }, }); - // eslint-disable-next-line no-new, @gitlab/no-runtime-template-compiler - new Vue({ - el: document.getElementById('js-add-list'), - data() { - return { - filters: boardsStore.state.filters, - ...getMilestoneTitle($boardApp), - }; - }, - mounted() { - initNewListDropdown(); - }, - }); - const createColumnTriggerEl = document.querySelector('.js-create-column-trigger'); if (createColumnTriggerEl) { // eslint-disable-next-line no-new diff --git a/app/assets/javascripts/pipeline_editor/components/commit/commit_section.vue b/app/assets/javascripts/pipeline_editor/components/commit/commit_section.vue index 8f4894a0bde..edc8c126cc8 100644 --- a/app/assets/javascripts/pipeline_editor/components/commit/commit_section.vue +++ b/app/assets/javascripts/pipeline_editor/components/commit/commit_section.vue @@ -10,7 +10,6 @@ import { import commitCIFile from '../../graphql/mutations/commit_ci_file.mutation.graphql'; import updateCurrentBranchMutation from '../../graphql/mutations/update_current_branch.mutation.graphql'; import updateLastCommitBranchMutation from '../../graphql/mutations/update_last_commit_branch.mutation.graphql'; -import getCommitSha from '../../graphql/queries/client/commit_sha.graphql'; import getCurrentBranch from '../../graphql/queries/client/current_branch.graphql'; import getIsNewCiConfigFile from '../../graphql/queries/client/is_new_ci_config_file.graphql'; import getPipelineEtag from '../../graphql/queries/client/pipeline_etag.graphql'; @@ -37,6 +36,11 @@ export default { type: String, required: true, }, + commitSha: { + type: String, + required: false, + default: '', + }, }, data() { return { @@ -49,9 +53,6 @@ export default { isNewCiConfigFile: { query: getIsNewCiConfigFile, }, - commitSha: { - query: getCommitSha, - }, currentBranch: { query: getCurrentBranch, }, @@ -96,13 +97,7 @@ export default { lastCommitId: this.commitSha, }, update(store, { data }) { - const commitSha = data?.commitCreate?.commit?.sha; const pipelineEtag = data?.commitCreate?.commit?.commitPipelinePath; - - if (commitSha) { - store.writeQuery({ query: getCommitSha, data: { commitSha } }); - } - if (pipelineEtag) { store.writeQuery({ query: getPipelineEtag, data: { pipelineEtag } }); } diff --git a/app/assets/javascripts/pipeline_editor/components/editor/text_editor.vue b/app/assets/javascripts/pipeline_editor/components/editor/text_editor.vue index 77ede396496..f2a0f474bc4 100644 --- a/app/assets/javascripts/pipeline_editor/components/editor/text_editor.vue +++ b/app/assets/javascripts/pipeline_editor/components/editor/text_editor.vue @@ -3,7 +3,6 @@ import { EDITOR_READY_EVENT } from '~/editor/constants'; import { CiSchemaExtension } from '~/editor/extensions/source_editor_ci_schema_ext'; import SourceEditor from '~/vue_shared/components/source_editor.vue'; import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; -import getCommitSha from '../../graphql/queries/client/commit_sha.graphql'; export default { components: { @@ -12,14 +11,11 @@ export default { mixins: [glFeatureFlagMixin()], inject: ['ciConfigPath', 'projectPath', 'projectNamespace', 'defaultBranch'], inheritAttrs: false, - data() { - return { - commitSha: '', - }; - }, - apollo: { + props: { commitSha: { - query: getCommitSha, + type: String, + required: false, + default: '', }, }, methods: { diff --git a/app/assets/javascripts/pipeline_editor/components/file_nav/branch_switcher.vue b/app/assets/javascripts/pipeline_editor/components/file_nav/branch_switcher.vue index 9a6eed50fbe..68065cc3c73 100644 --- a/app/assets/javascripts/pipeline_editor/components/file_nav/branch_switcher.vue +++ b/app/assets/javascripts/pipeline_editor/components/file_nav/branch_switcher.vue @@ -158,11 +158,9 @@ export default { const updatedPath = setUrlParams({ branch_name: newBranch }); historyPushState(updatedPath); - this.$emit('updateCommitSha', { newBranch }); - // refetching the content will cause a lot of components to re-render, // including the text editor which uses the commit sha to register the CI schema - // so we need to make sure the commit sha is updated first + // so we need to make sure the currentBranch (and consequently, the commitSha) are updated first await this.$nextTick(); this.$emit('refetchContent'); }, diff --git a/app/assets/javascripts/pipeline_editor/components/header/pipeline_editor_header.vue b/app/assets/javascripts/pipeline_editor/components/header/pipeline_editor_header.vue index 24bca04e115..fcc31f087ff 100644 --- a/app/assets/javascripts/pipeline_editor/components/header/pipeline_editor_header.vue +++ b/app/assets/javascripts/pipeline_editor/components/header/pipeline_editor_header.vue @@ -33,6 +33,11 @@ export default { type: Object, required: true, }, + commitSha: { + type: String, + required: false, + default: '', + }, isNewCiConfigFile: { type: Boolean, required: true, @@ -54,7 +59,11 @@ export default { </script> <template> <div class="gl-mb-5"> - <pipeline-status v-if="showPipelineStatus" :class="$options.pipelineStatusClasses" /> + <pipeline-status + v-if="showPipelineStatus" + :commit-sha="commitSha" + :class="$options.pipelineStatusClasses" + /> <validation-segment :class="validationStyling" :ci-config="ciConfigData" /> </div> </template> diff --git a/app/assets/javascripts/pipeline_editor/components/header/pipeline_status.vue b/app/assets/javascripts/pipeline_editor/components/header/pipeline_status.vue index 46f6f4a28c1..ec240854be5 100644 --- a/app/assets/javascripts/pipeline_editor/components/header/pipeline_status.vue +++ b/app/assets/javascripts/pipeline_editor/components/header/pipeline_status.vue @@ -3,7 +3,6 @@ import { GlButton, GlIcon, GlLink, GlLoadingIcon, GlSprintf } from '@gitlab/ui'; import { getIdFromGraphQLId } from '~/graphql_shared/utils'; import { truncateSha } from '~/lib/utils/text_utility'; import { s__ } from '~/locale'; -import getCommitSha from '~/pipeline_editor/graphql/queries/client/commit_sha.graphql'; import getPipelineQuery from '~/pipeline_editor/graphql/queries/client/pipeline.graphql'; import getPipelineEtag from '~/pipeline_editor/graphql/queries/client/pipeline_etag.graphql'; import { @@ -33,10 +32,14 @@ export default { GlSprintf, }, inject: ['projectFullPath'], - apollo: { + props: { commitSha: { - query: getCommitSha, + type: String, + required: false, + default: '', }, + }, + apollo: { pipelineEtag: { query: getPipelineEtag, }, @@ -51,7 +54,7 @@ export default { sha: this.commitSha, }; }, - update: (data) => { + update(data) { const { id, commitPath = '', detailedStatus = {} } = data.project?.pipeline || {}; return { @@ -60,6 +63,11 @@ export default { detailedStatus, }; }, + result(res) { + if (res.data?.project?.pipeline) { + this.hasError = false; + } + }, error() { this.hasError = true; }, @@ -68,7 +76,6 @@ export default { }, data() { return { - commitSha: '', hasError: false, }; }, @@ -84,7 +91,11 @@ export default { // (e.g. pipeline is null during fetch when the pipeline hasn't been // triggered yet), we can just show the loading state until the pipeline // details are ready to be fetched - return this.$apollo.queries.pipeline.loading || (!this.hasPipelineData && !this.hasError); + return ( + this.$apollo.queries.pipeline.loading || + this.commitSha.length === 0 || + (!this.hasPipelineData && !this.hasError) + ); }, shortSha() { return truncateSha(this.commitSha); diff --git a/app/assets/javascripts/pipeline_editor/components/pipeline_editor_tabs.vue b/app/assets/javascripts/pipeline_editor/components/pipeline_editor_tabs.vue index e463fcf379d..f7c9f10ea46 100644 --- a/app/assets/javascripts/pipeline_editor/components/pipeline_editor_tabs.vue +++ b/app/assets/javascripts/pipeline_editor/components/pipeline_editor_tabs.vue @@ -69,6 +69,11 @@ export default { type: String, required: true, }, + commitSha: { + type: String, + required: false, + default: '', + }, }, apollo: { appStatus: { @@ -110,7 +115,7 @@ export default { @click="setCurrentTab($options.tabConstants.CREATE_TAB)" > <ci-editor-header /> - <text-editor :value="ciFileContent" v-on="$listeners" /> + <text-editor :commit-sha="commitSha" :value="ciFileContent" v-on="$listeners" /> </editor-tab> <editor-tab class="gl-mb-3" diff --git a/app/assets/javascripts/pipeline_editor/graphql/mutations/update_commit_sha.mutation.graphql b/app/assets/javascripts/pipeline_editor/graphql/mutations/update_commit_sha.mutation.graphql deleted file mode 100644 index dce17cad808..00000000000 --- a/app/assets/javascripts/pipeline_editor/graphql/mutations/update_commit_sha.mutation.graphql +++ /dev/null @@ -1,3 +0,0 @@ -mutation updateCommitSha($commitSha: String) { - updateCommitSha(commitSha: $commitSha) @client -} diff --git a/app/assets/javascripts/pipeline_editor/graphql/queries/client/commit_sha.graphql b/app/assets/javascripts/pipeline_editor/graphql/queries/client/commit_sha.graphql deleted file mode 100644 index 6c7635887ec..00000000000 --- a/app/assets/javascripts/pipeline_editor/graphql/queries/client/commit_sha.graphql +++ /dev/null @@ -1,3 +0,0 @@ -query getCommitSha { - commitSha @client -} diff --git a/app/assets/javascripts/pipeline_editor/graphql/resolvers.js b/app/assets/javascripts/pipeline_editor/graphql/resolvers.js index 2bec2006e95..a34652b1495 100644 --- a/app/assets/javascripts/pipeline_editor/graphql/resolvers.js +++ b/app/assets/javascripts/pipeline_editor/graphql/resolvers.js @@ -1,6 +1,5 @@ import produce from 'immer'; import axios from '~/lib/utils/axios_utils'; -import getCommitShaQuery from './queries/client/commit_sha.graphql'; import getCurrentBranchQuery from './queries/client/current_branch.graphql'; import getLastCommitBranchQuery from './queries/client/last_commit_branch.query.graphql'; @@ -32,14 +31,6 @@ export const resolvers = { __typename: 'CiLintContent', })); }, - updateCommitSha: (_, { commitSha }, { cache }) => { - cache.writeQuery({ - query: getCommitShaQuery, - data: produce(cache.readQuery({ query: getCommitShaQuery }), (draftData) => { - draftData.commitSha = commitSha; - }), - }); - }, updateCurrentBranch: (_, { currentBranch }, { cache }) => { cache.writeQuery({ query: getCurrentBranchQuery, diff --git a/app/assets/javascripts/pipeline_editor/index.js b/app/assets/javascripts/pipeline_editor/index.js index e0f8d889cad..976ebe80aee 100644 --- a/app/assets/javascripts/pipeline_editor/index.js +++ b/app/assets/javascripts/pipeline_editor/index.js @@ -4,7 +4,6 @@ import VueApollo from 'vue-apollo'; import createDefaultClient from '~/lib/graphql'; import { resetServiceWorkersPublicPath } from '../lib/utils/webpack'; import { CODE_SNIPPET_SOURCE_SETTINGS } from './components/code_snippet_alert/constants'; -import getCommitSha from './graphql/queries/client/commit_sha.graphql'; import getCurrentBranch from './graphql/queries/client/current_branch.graphql'; import getLastCommitBranchQuery from './graphql/queries/client/last_commit_branch.query.graphql'; import getPipelineEtag from './graphql/queries/client/pipeline_etag.graphql'; @@ -26,7 +25,6 @@ export const initPipelineEditor = (selector = '#js-pipeline-editor') => { const { // Add to apollo cache as it can be updated by future queries - commitSha, initialBranchName, pipelineEtag, // Add to provide/inject API for static values @@ -70,13 +68,6 @@ export const initPipelineEditor = (selector = '#js-pipeline-editor') => { }); cache.writeQuery({ - query: getCommitSha, - data: { - commitSha, - }, - }); - - cache.writeQuery({ query: getPipelineEtag, data: { pipelineEtag, diff --git a/app/assets/javascripts/pipeline_editor/pipeline_editor_app.vue b/app/assets/javascripts/pipeline_editor/pipeline_editor_app.vue index 0e8a6805a59..cff6f549b6e 100644 --- a/app/assets/javascripts/pipeline_editor/pipeline_editor_app.vue +++ b/app/assets/javascripts/pipeline_editor/pipeline_editor_app.vue @@ -16,11 +16,9 @@ import { LOAD_FAILURE_UNKNOWN, STARTER_TEMPLATE_NAME, } from './constants'; -import updateCommitShaMutation from './graphql/mutations/update_commit_sha.mutation.graphql'; import getBlobContent from './graphql/queries/blob_content.graphql'; import getCiConfigData from './graphql/queries/ci_config.graphql'; import getAppStatus from './graphql/queries/client/app_status.graphql'; -import getCommitSha from './graphql/queries/client/commit_sha.graphql'; import getCurrentBranch from './graphql/queries/client/current_branch.graphql'; import getIsNewCiConfigFile from './graphql/queries/client/is_new_ci_config_file.graphql'; import getTemplate from './graphql/queries/get_starter_template.query.graphql'; @@ -156,7 +154,32 @@ export default { query: getAppStatus, }, commitSha: { - query: getCommitSha, + query: getLatestCommitShaQuery, + variables() { + return { + projectPath: this.projectFullPath, + ref: this.currentBranch, + }; + }, + update(data) { + const pipelineNodes = data.project?.pipelines?.nodes ?? []; + + // it's possible to query for the commit sha too early after an update + // (e.g. after committing a new branch, we might query for the commit sha + // but the pipeline nodes are still empty). + // in this case, we start polling until we get a commit sha. + if (pipelineNodes.length === 0) { + if (![EDITOR_APP_STATUS_LOADING, EDITOR_APP_STATUS_EMPTY].includes(this.appStatus)) { + this.$apollo.queries.commitSha.startPolling(1000); + return this.commitSha; + } + + return ''; + } + + this.$apollo.queries.commitSha.stopPolling(); + return pipelineNodes[0].sha; + }, }, currentBranch: { query: getCurrentBranch, @@ -257,38 +280,6 @@ export default { updateCiConfig(ciFileContent) { this.currentCiFileContent = ciFileContent; }, - async updateCommitSha({ newBranch }) { - let fetchResults; - - try { - fetchResults = await this.$apollo.query({ - query: getLatestCommitShaQuery, - variables: { - projectPath: this.projectFullPath, - ref: newBranch, - }, - }); - } catch { - this.showFetchError(); - return; - } - - if (fetchResults.errors?.length > 0) { - this.showFetchError(); - return; - } - - const pipelineNodes = fetchResults?.data?.project?.pipelines?.nodes ?? []; - if (pipelineNodes.length === 0) { - return; - } - - const commitSha = pipelineNodes[0].sha; - this.$apollo.mutate({ - mutation: updateCommitShaMutation, - variables: { commitSha }, - }); - }, updateOnCommit({ type }) { this.reportSuccess(type); @@ -336,12 +327,12 @@ export default { :ci-config-data="ciConfigData" :ci-file-content="currentCiFileContent" :is-new-ci-config-file="isNewCiConfigFile" + :commit-sha="commitSha" @commit="updateOnCommit" @resetContent="resetContent" @showError="showErrorAlert" @refetchContent="refetchContent" @updateCiConfig="updateCiConfig" - @updateCommitSha="updateCommitSha" /> <confirm-unsaved-changes-dialog :has-unsaved-changes="hasUnsavedChanges" /> </div> diff --git a/app/assets/javascripts/pipeline_editor/pipeline_editor_home.vue b/app/assets/javascripts/pipeline_editor/pipeline_editor_home.vue index dfe9c82b912..4324c64ab3b 100644 --- a/app/assets/javascripts/pipeline_editor/pipeline_editor_home.vue +++ b/app/assets/javascripts/pipeline_editor/pipeline_editor_home.vue @@ -25,6 +25,11 @@ export default { type: String, required: true, }, + commitSha: { + type: String, + required: false, + default: '', + }, isNewCiConfigFile: { type: Boolean, required: true, @@ -56,15 +61,22 @@ export default { <pipeline-editor-file-nav v-on="$listeners" /> <pipeline-editor-header :ci-config-data="ciConfigData" + :commit-sha="commitSha" :is-new-ci-config-file="isNewCiConfigFile" /> <pipeline-editor-tabs :ci-config-data="ciConfigData" :ci-file-content="ciFileContent" + :commit-sha="commitSha" v-on="$listeners" @set-current-tab="setCurrentTab" /> - <commit-section v-if="showCommitForm" :ci-file-content="ciFileContent" v-on="$listeners" /> + <commit-section + v-if="showCommitForm" + :ci-file-content="ciFileContent" + :commit-sha="commitSha" + v-on="$listeners" + /> <pipeline-editor-drawer v-if="showPipelineDrawer" /> </div> </template> diff --git a/app/assets/stylesheets/components/content_editor.scss b/app/assets/stylesheets/components/content_editor.scss index 64abf5574fa..9506eb99b9b 100644 --- a/app/assets/stylesheets/components/content_editor.scss +++ b/app/assets/stylesheets/components/content_editor.scss @@ -2,7 +2,7 @@ td, th, li { - :only-child { + :first-child { margin-bottom: 0 !important; } } diff --git a/app/assets/stylesheets/framework/filters.scss b/app/assets/stylesheets/framework/filters.scss index f76101d92b1..5dd71cec8d1 100644 --- a/app/assets/stylesheets/framework/filters.scss +++ b/app/assets/stylesheets/framework/filters.scss @@ -386,15 +386,6 @@ } } } - - .boards-add-list > .btn { - text-align: left; - - > svg { - position: absolute; - right: 6px; - } - } } .droplab-dropdown .dropdown-menu .filter-dropdown-item { diff --git a/app/assets/stylesheets/page_bundles/boards.scss b/app/assets/stylesheets/page_bundles/boards.scss index 10183f774b1..eb2d2252526 100644 --- a/app/assets/stylesheets/page_bundles/boards.scss +++ b/app/assets/stylesheets/page_bundles/boards.scss @@ -15,14 +15,6 @@ } } -.dropdown-menu-issues-board-new { - width: 320px; - - .dropdown-content { - max-height: 140px; - } -} - .issue-board-dropdown-content { margin: 0; padding: $gl-padding-4 $gl-padding $gl-padding; diff --git a/app/models/application_record.rb b/app/models/application_record.rb index d9375b55e89..29ef76547e0 100644 --- a/app/models/application_record.rb +++ b/app/models/application_record.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true class ApplicationRecord < ActiveRecord::Base + self.gitlab_schema = :gitlab_main self.abstract_class = true alias_method :reset, :reload diff --git a/app/models/ci/application_record.rb b/app/models/ci/application_record.rb index 9d4a8f0648e..913e7a62c66 100644 --- a/app/models/ci/application_record.rb +++ b/app/models/ci/application_record.rb @@ -2,6 +2,7 @@ module Ci class ApplicationRecord < ::ApplicationRecord + self.gitlab_schema = :gitlab_ci self.abstract_class = true def self.table_name_prefix diff --git a/app/models/ci/pending_build.rb b/app/models/ci/pending_build.rb index 7cf3a387516..237651e41c1 100644 --- a/app/models/ci/pending_build.rb +++ b/app/models/ci/pending_build.rb @@ -12,51 +12,55 @@ module Ci scope :queued_before, ->(time) { where(arel_table[:created_at].lt(time)) } scope :with_instance_runners, -> { where(instance_runners_enabled: true) } - def self.upsert_from_build!(build) - entry = self.new(args_from_build(build)) + class << self + def upsert_from_build!(build) + entry = self.new(args_from_build(build)) - entry.validate! + entry.validate! - self.upsert(entry.attributes.compact, returning: %w[build_id], unique_by: :build_id) - end + self.upsert(entry.attributes.compact, returning: %w[build_id], unique_by: :build_id) + end + + private + + def args_from_build(build) + args = { + build: build, + project: build.project, + protected: build.protected?, + namespace: build.project.namespace + } + + if Feature.enabled?(:ci_pending_builds_maintain_tags_data, type: :development, default_enabled: :yaml) + args.store(:tag_ids, build.tags_ids) + end + + if Feature.enabled?(:ci_pending_builds_maintain_shared_runners_data, type: :development, default_enabled: :yaml) + args.store(:instance_runners_enabled, shareable?(build)) + end - def self.args_from_build(build) - args = { - build: build, - project: build.project, - protected: build.protected?, - namespace: build.project.namespace - } - - if Feature.enabled?(:ci_pending_builds_maintain_shared_runners_data, type: :development, default_enabled: :yaml) - args.merge(instance_runners_enabled: shareable?(build)) - else args end - end - private_class_method :args_from_build - def self.shareable?(build) - shared_runner_enabled?(build) && - builds_access_level?(build) && - project_not_removed?(build) - end - private_class_method :shareable? + def shareable?(build) + shared_runner_enabled?(build) && + builds_access_level?(build) && + project_not_removed?(build) + end - def self.shared_runner_enabled?(build) - build.project.shared_runners.exists? - end - private_class_method :shared_runner_enabled? + def shared_runner_enabled?(build) + build.project.shared_runners.exists? + end - def self.project_not_removed?(build) - !build.project.pending_delete? - end - private_class_method :project_not_removed? + def project_not_removed?(build) + !build.project.pending_delete? + end - def self.builds_access_level?(build) - build.project.project_feature.builds_access_level.nil? || build.project.project_feature.builds_access_level > 0 + def builds_access_level?(build) + build.project.project_feature.builds_access_level.nil? || + build.project.project_feature.builds_access_level > 0 + end end - private_class_method :builds_access_level? end end diff --git a/app/models/concerns/taggable_queries.rb b/app/models/concerns/taggable_queries.rb index cba2e93a86d..06799f0a9f4 100644 --- a/app/models/concerns/taggable_queries.rb +++ b/app/models/concerns/taggable_queries.rb @@ -3,6 +3,10 @@ module TaggableQueries extend ActiveSupport::Concern + MAX_TAGS_IDS = 50 + + TooManyTagsError = Class.new(StandardError) + class_methods do # context is a name `acts_as_taggable context` def arel_tag_names_array(context = :tags) @@ -34,4 +38,10 @@ module TaggableQueries where("EXISTS (?)", matcher) end end + + def tags_ids + tags.limit(MAX_TAGS_IDS).order('id ASC').pluck(:id).tap do |ids| + raise TooManyTagsError if ids.size >= MAX_TAGS_IDS + end + end end diff --git a/app/models/user.rb b/app/models/user.rb index 19844bf2f9c..fbae7f95f01 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -254,8 +254,7 @@ class User < ApplicationRecord message: _("%{placeholder} is not a valid color scheme") % { placeholder: '%{value}' } } before_validation :sanitize_attrs - before_validation :set_public_email, if: :public_email_changed? - before_validation :set_commit_email, if: :commit_email_changed? + before_validation :reset_secondary_emails, if: :email_changed? before_save :default_private_profile_to_false before_save :set_public_email, if: :public_email_changed? # in case validation is skipped before_save :set_commit_email, if: :commit_email_changed? # in case validation is skipped @@ -928,24 +927,6 @@ class User < ApplicationRecord end end - def notification_email_verified - return if read_attribute(:notification_email).blank? || temp_oauth_email? - - errors.add(:notification_email, _("must be an email you have verified")) unless verified_emails.include?(notification_email) - end - - def public_email_verified - return if public_email.blank? - - errors.add(:public_email, _("must be an email you have verified")) unless verified_emails.include?(public_email) - end - - def commit_email_verified - return if read_attribute(:commit_email).blank? - - errors.add(:commit_email, _("must be an email you have verified")) unless verified_emails.include?(commit_email) - 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 @@ -1298,24 +1279,6 @@ class User < ApplicationRecord self.name = self.name.gsub(%r{</?[^>]*>}, '') end - def set_notification_email - if notification_email.blank? || all_emails.exclude?(notification_email) - self.notification_email = email - end - end - - def set_public_email - if public_email.blank? || all_emails.exclude?(public_email) - self.public_email = '' - end - end - - def set_commit_email - if commit_email.blank? || verified_emails.exclude?(commit_email) - self.commit_email = nil - end - end - def unset_secondary_emails_matching_deleted_email!(deleted_email) secondary_email_attribute_changed = false SECONDARY_EMAIL_ATTRIBUTES.each do |attribute| @@ -2025,6 +1988,48 @@ class User < ApplicationRecord private + def notification_email_verified + return if read_attribute(:notification_email).blank? || temp_oauth_email? + + errors.add(:notification_email, _("must be an email you have verified")) unless verified_emails.include?(notification_email) + end + + def public_email_verified + return if public_email.blank? + + errors.add(:public_email, _("must be an email you have verified")) unless verified_emails.include?(public_email) + end + + def commit_email_verified + return if read_attribute(:commit_email).blank? + + errors.add(:commit_email, _("must be an email you have verified")) unless verified_emails.include?(commit_email) + end + + def set_notification_email + if notification_email.blank? || verified_emails.exclude?(notification_email) + self.notification_email = nil + end + end + + def set_public_email + if public_email.blank? || verified_emails.exclude?(public_email) + self.public_email = '' + end + end + + def set_commit_email + if commit_email.blank? || verified_emails.exclude?(commit_email) + self.commit_email = nil + end + end + + def reset_secondary_emails + set_public_email + set_commit_email + set_notification_email + end + def callouts_by_feature_name @callouts_by_feature_name ||= callouts.index_by(&:feature_name) end diff --git a/app/services/ci/queue/pending_builds_strategy.rb b/app/services/ci/queue/pending_builds_strategy.rb index efe3a981d3a..e56699f172d 100644 --- a/app/services/ci/queue/pending_builds_strategy.rb +++ b/app/services/ci/queue/pending_builds_strategy.rb @@ -17,11 +17,19 @@ module Ci end def builds_matching_tag_ids(relation, ids) - relation.merge(CommitStatus.matches_tag_ids(ids, table: 'ci_pending_builds', column: 'build_id')) + if ::Feature.enabled?(:ci_queueing_denormalize_tags_information, runner, default_enabled: :yaml) + relation.where('tag_ids <@ ARRAY[?]::int[]', runner.tags_ids) + else + relation.merge(CommitStatus.matches_tag_ids(ids, table: 'ci_pending_builds', column: 'build_id')) + end end def builds_with_any_tags(relation) - relation.merge(CommitStatus.with_any_tags(table: 'ci_pending_builds', column: 'build_id')) + if ::Feature.enabled?(:ci_queueing_denormalize_tags_information, runner, default_enabled: :yaml) + relation.where('cardinality(tag_ids) > 0') + else + relation.merge(CommitStatus.with_any_tags(table: 'ci_pending_builds', column: 'build_id')) + end end def order(relation) diff --git a/app/views/shared/issuable/_board_create_list_dropdown.html.haml b/app/views/shared/issuable/_board_create_list_dropdown.html.haml deleted file mode 100644 index 74b064648c0..00000000000 --- a/app/views/shared/issuable/_board_create_list_dropdown.html.haml +++ /dev/null @@ -1,8 +0,0 @@ -.dropdown.gl-display-flex.gl-align-items-center.gl-ml-3#js-add-list - %button.gl-button.btn.btn-confirm.js-new-board-list{ type: "button", data: board_list_data } - Add list - .dropdown-menu.dropdown-extended-height.dropdown-menu-paging.dropdown-menu-right.dropdown-menu-issues-board-new.dropdown-menu-selectable.js-tab-container-labels - = render partial: "shared/issuable/label_page_default", locals: { show_footer: true, show_create: true, show_boards_content: true, title: "Add list" } - - if can?(current_user, :admin_label, board.resource_parent) - = render partial: "shared/issuable/label_page_create", locals: { show_add_list: true, add_list: true, add_list_class: 'd-none' } - = dropdown_loading diff --git a/app/views/shared/issuable/_search_bar.html.haml b/app/views/shared/issuable/_search_bar.html.haml index 737a0ff8c5b..e6c4b3f4814 100644 --- a/app/views/shared/issuable/_search_bar.html.haml +++ b/app/views/shared/issuable/_search_bar.html.haml @@ -207,10 +207,7 @@ #js-board-epics-swimlanes-toggle .js-board-config{ data: { can_admin_list: user_can_admin_list.to_s, has_scope: board.scoped?.to_s } } - if user_can_admin_list - - if Feature.enabled?(:board_new_list, board.resource_parent, default_enabled: :yaml) || board.to_type == "EpicBoard" - .js-create-column-trigger{ data: board_list_data } - - else - = render 'shared/issuable/board_create_list_dropdown', board: board + .js-create-column-trigger{ data: board_list_data } #js-toggle-focus-btn - elsif type != :productivity_analytics && show_sorting_dropdown = render 'shared/issuable/sort_dropdown' diff --git a/config/feature_flags/development/between_uses_list_commits.yml b/config/feature_flags/development/ci_build_tags_limit.yml index 84b34e1f8b2..20156aa4841 100644 --- a/config/feature_flags/development/between_uses_list_commits.yml +++ b/config/feature_flags/development/ci_build_tags_limit.yml @@ -1,8 +1,8 @@ --- -name: between_uses_list_commits -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/67591 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/337960 +name: ci_build_tags_limit +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/68380 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/338929 milestone: '14.2' type: development -group: group::source code +group: group::pipeline execution default_enabled: false diff --git a/config/feature_flags/development/ci_new_artifact_file_reader.yml b/config/feature_flags/development/ci_new_artifact_file_reader.yml index ccd36558b1d..d475f3f370d 100644 --- a/config/feature_flags/development/ci_new_artifact_file_reader.yml +++ b/config/feature_flags/development/ci_new_artifact_file_reader.yml @@ -5,4 +5,4 @@ rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/273755 milestone: '13.6' type: development group: group::pipeline authoring -default_enabled: false +default_enabled: true diff --git a/config/feature_flags/development/board_new_list.yml b/config/feature_flags/development/ci_pending_builds_maintain_tags_data.yml index 7d755dd6689..bd18789bbcf 100644 --- a/config/feature_flags/development/board_new_list.yml +++ b/config/feature_flags/development/ci_pending_builds_maintain_tags_data.yml @@ -1,8 +1,8 @@ --- -name: board_new_list -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/52061 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/299366 -milestone: '13.8' +name: ci_pending_builds_maintain_tags_data +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/65648 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/338363 +milestone: '14.2' type: development -group: group::project management -default_enabled: true +group: group::pipeline execution +default_enabled: false diff --git a/config/feature_flags/development/track_unique_visits.yml b/config/feature_flags/development/ci_queueing_denormalize_tags_information.yml index dd053211070..23f7be4cccc 100644 --- a/config/feature_flags/development/track_unique_visits.yml +++ b/config/feature_flags/development/ci_queueing_denormalize_tags_information.yml @@ -1,8 +1,8 @@ --- -name: track_unique_visits -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/33146 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/255994 -milestone: '13.2' +name: ci_queueing_denormalize_tags_information +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/65648 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/338366 +milestone: '14.1' type: development -group: group::optimize -default_enabled: true +group: group::pipeline execution +default_enabled: false diff --git a/config/initializers/00_active_record_gitlab_schema.rb b/config/initializers/00_active_record_gitlab_schema.rb new file mode 100644 index 00000000000..f1ddd4d4eb1 --- /dev/null +++ b/config/initializers/00_active_record_gitlab_schema.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +# This parameter describes a virtual context to indicate +# table affinity to other tables. +# +# Table affinity limits cross-joins, cross-modifications, +# foreign keys and validates relationship between tables +# +# By default it is undefined +ActiveRecord::Base.class_attribute :gitlab_schema, default: nil diff --git a/config/initializers/0_acts_as_taggable.rb b/config/initializers/0_acts_as_taggable.rb index 9f66d970ffd..04619590e3c 100644 --- a/config/initializers/0_acts_as_taggable.rb +++ b/config/initializers/0_acts_as_taggable.rb @@ -13,3 +13,7 @@ raise "Counter cache is not disabled" if ActsAsTaggableOn::Tagging.include IgnorableColumns ActsAsTaggableOn::Tagging.ignore_column :id_convert_to_bigint, remove_with: '14.2', remove_after: '2021-08-22' ActsAsTaggableOn::Tagging.ignore_column :taggable_id_convert_to_bigint, remove_with: '14.2', remove_after: '2021-08-22' + +# The tags and taggings are supposed to be part of `gitlab_ci` +ActsAsTaggableOn::Tag.gitlab_schema = :gitlab_ci +ActsAsTaggableOn::Tagging.gitlab_schema = :gitlab_ci diff --git a/config/metrics/counts_28d/20210216180814_events.yml b/config/metrics/counts_28d/20210216180814_events.yml index ddc06e6c9ea..384505384f8 100644 --- a/config/metrics/counts_28d/20210216180814_events.yml +++ b/config/metrics/counts_28d/20210216180814_events.yml @@ -1,20 +1,21 @@ --- data_category: optional key_path: usage_activity_by_stage_monthly.manage.events -description: +description: Number of distinct users who have generated a manage event by month product_section: dev -product_stage: +product_stage: manage product_group: group::manage product_category: value_type: number status: data_available time_frame: 28d -data_source: +data_source: database distribution: - ce - ee tier: - free -skip_validation: true +- premium +- ultimate performance_indicator_type: - umau diff --git a/db/migrate/20210707113056_add_tags_array_to_ci_pending_builds.rb b/db/migrate/20210707113056_add_tags_array_to_ci_pending_builds.rb new file mode 100644 index 00000000000..229dc01fb87 --- /dev/null +++ b/db/migrate/20210707113056_add_tags_array_to_ci_pending_builds.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddTagsArrayToCiPendingBuilds < ActiveRecord::Migration[6.1] + include Gitlab::Database::MigrationHelpers + + def up + with_lock_retries do + add_column :ci_pending_builds, :tag_ids, :integer, array: true, default: [] + end + end + + def down + with_lock_retries do + remove_column :ci_pending_builds, :tag_ids + end + end +end diff --git a/db/migrate/20210818185548_add_tag_ids_index_to_ci_pending_build.rb b/db/migrate/20210818185548_add_tag_ids_index_to_ci_pending_build.rb new file mode 100644 index 00000000000..b8e00ed9db0 --- /dev/null +++ b/db/migrate/20210818185548_add_tag_ids_index_to_ci_pending_build.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddTagIdsIndexToCiPendingBuild < ActiveRecord::Migration[6.1] + include Gitlab::Database::MigrationHelpers + + disable_ddl_transaction! + + INDEX_NAME = 'index_ci_pending_builds_on_tag_ids' + + def up + add_concurrent_index(:ci_pending_builds, :tag_ids, name: INDEX_NAME, where: 'cardinality(tag_ids) > 0') + end + + def down + remove_concurrent_index_by_name(:ci_pending_builds, name: INDEX_NAME) + end +end diff --git a/db/post_migrate/20210817024335_prepare_indexes_for_events_bigint_conversion.rb b/db/post_migrate/20210817024335_prepare_indexes_for_events_bigint_conversion.rb new file mode 100644 index 00000000000..1d102d6216c --- /dev/null +++ b/db/post_migrate/20210817024335_prepare_indexes_for_events_bigint_conversion.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +class PrepareIndexesForEventsBigintConversion < ActiveRecord::Migration[6.1] + include Gitlab::Database::MigrationHelpers + + TABLE_NAME = 'events' + + def up + prepare_async_index TABLE_NAME, :id_convert_to_bigint, unique: true, + name: :index_events_on_id_convert_to_bigint + + prepare_async_index TABLE_NAME, [:project_id, :id_convert_to_bigint], + name: :index_events_on_project_id_and_id_convert_to_bigint + + prepare_async_index TABLE_NAME, [:project_id, :id_convert_to_bigint], order: { id_convert_to_bigint: :desc }, + where: 'action = 7', name: :index_events_on_project_id_and_id_bigint_desc_on_merged_action + end + + def down + unprepare_async_index_by_name TABLE_NAME, :index_events_on_id_convert_to_bigint + unprepare_async_index_by_name TABLE_NAME, :index_events_on_project_id_and_id_convert_to_bigint + unprepare_async_index_by_name TABLE_NAME, :index_events_on_project_id_and_id_bigint_desc_on_merged_action + end +end diff --git a/db/schema_migrations/20210707113056 b/db/schema_migrations/20210707113056 new file mode 100644 index 00000000000..3526caf8109 --- /dev/null +++ b/db/schema_migrations/20210707113056 @@ -0,0 +1 @@ +837b9a56114c63064379cf276a3c7e2bbe845af9022a542c4fcec94a25062017
\ No newline at end of file diff --git a/db/schema_migrations/20210817024335 b/db/schema_migrations/20210817024335 new file mode 100644 index 00000000000..019ec0a26b7 --- /dev/null +++ b/db/schema_migrations/20210817024335 @@ -0,0 +1 @@ +360bb1c16c93d7a6564ed70fa2dea4212e1fd00d101cfdc9017b54f67eae797d
\ No newline at end of file diff --git a/db/schema_migrations/20210818185548 b/db/schema_migrations/20210818185548 new file mode 100644 index 00000000000..42826826512 --- /dev/null +++ b/db/schema_migrations/20210818185548 @@ -0,0 +1 @@ +88ca485c8513df96b1f1aec1585c385223dc53889e547db42b509b0cd1bea9b7
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index d7d5e85ba62..2f34ab47f28 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -10903,7 +10903,8 @@ CREATE TABLE ci_pending_builds ( protected boolean DEFAULT false NOT NULL, instance_runners_enabled boolean DEFAULT false NOT NULL, namespace_id bigint, - minutes_exceeded boolean DEFAULT false NOT NULL + minutes_exceeded boolean DEFAULT false NOT NULL, + tag_ids integer[] DEFAULT '{}'::integer[] ); CREATE SEQUENCE ci_pending_builds_id_seq @@ -23472,6 +23473,8 @@ CREATE INDEX index_ci_pending_builds_on_namespace_id ON ci_pending_builds USING CREATE INDEX index_ci_pending_builds_on_project_id ON ci_pending_builds USING btree (project_id); +CREATE INDEX index_ci_pending_builds_on_tag_ids ON ci_pending_builds USING btree (tag_ids) WHERE (cardinality(tag_ids) > 0); + CREATE INDEX index_ci_pipeline_artifacts_failed_verification ON ci_pipeline_artifacts USING btree (verification_retry_at NULLS FIRST) WHERE (verification_state = 3); CREATE INDEX index_ci_pipeline_artifacts_needs_verification ON ci_pipeline_artifacts USING btree (verification_state) WHERE ((verification_state = 0) OR (verification_state = 3)); diff --git a/doc/ci/yaml/index.md b/doc/ci/yaml/index.md index 63f626e524e..87181ef8f3e 100644 --- a/doc/ci/yaml/index.md +++ b/doc/ci/yaml/index.md @@ -1885,6 +1885,9 @@ variables: - echo "Hello runner selector feature" ``` +NOTE: +In [GitLab 14.3](https://gitlab.com/gitlab-org/gitlab/-/issues/338479) and later, the number of tags must be less than `50`. + ### `allow_failure` Use `allow_failure` when you want to let a job fail without impacting the rest of the CI diff --git a/doc/development/fe_guide/index.md b/doc/development/fe_guide/index.md index 549fa3261b1..a6b49394733 100644 --- a/doc/development/fe_guide/index.md +++ b/doc/development/fe_guide/index.md @@ -22,7 +22,7 @@ Be wary of [the limitations that come with using Hamlit](https://github.com/k0ku We also use [SCSS](https://sass-lang.com) and plain JavaScript with modern ECMAScript standards supported through [Babel](https://babeljs.io/) and ES module support through [webpack](https://webpack.js.org/). -Working with our frontend assets requires Node (v10.13.0 or greater) and Yarn +Working with our frontend assets requires Node (v12.22.1 or greater) and Yarn (v1.10.0 or greater). You can find information on how to install these on our [installation guide](../../install/installation.md#4-node). diff --git a/doc/user/project/issue_board.md b/doc/user/project/issue_board.md index 8c212c99913..4fcf669f518 100644 --- a/doc/user/project/issue_board.md +++ b/doc/user/project/issue_board.md @@ -339,14 +339,14 @@ As in other list types, click the trash icon to remove a list. ### Iteration lists **(PREMIUM)** > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/250479) in GitLab 13.11. -> - [Deployed behind the `board_new_list` and `iteration_board_lists` feature flags](../feature_flags.md), enabled by default. -> - Enabled on GitLab.com. -> - Recommended for production use. -> - For GitLab self-managed instances, GitLab administrators can opt to disable the feature flags: [`board_new_list`](#enable-or-disable-new-add-list-form) and [`iteration_board_lists`](#enable-or-disable-iteration-lists-in-boards). **(PREMIUM SELF)** +> - Enabled on GitLab.com and is ready for production use. +> - Enabled with `iteration_board_lists` flag for self-managed GitLab and is ready for production use. +> GitLab administrators can opt to [disable the feature flag](#enable-or-disable-iteration-lists-in-boards). -There can be -[risks when disabling released features](../../administration/feature_flags.md#risks-when-disabling-released-features). -Refer to this feature's version history for more details. +FLAG: +On self-managed GitLab, by default this feature is available. To hide the feature, ask an +administrator to [disable the `iteration_board_lists` flag](../../administration/feature_flags.md). +On GitLab.com, this feature is available. You're also able to create lists of an iteration. These are lists that filter issues by the assigned @@ -675,10 +675,6 @@ A few things to remember: ### Enable or disable GraphQL-based issue boards **(FREE SELF)** -NOTE: -When enabling GraphQL-based issue boards, you must also enable the -[new add list form](#enable-or-disable-new-add-list-form). - It is deployed behind a feature flag that is **enabled by default** as of GitLab 14.1. [GitLab administrators with access to the GitLab Rails console](../../administration/feature_flags.md) can disable it. @@ -695,30 +691,8 @@ To disable it: Feature.disable(:graphql_board_lists) ``` -### Enable or disable new add list form **(FREE SELF)** - -The new form for adding lists is under development but ready for production use. It is -deployed behind a feature flag that is **enabled by default**. -[GitLab administrators with access to the GitLab Rails console](../../administration/feature_flags.md) -can disable it. - -To enable it: - -```ruby -Feature.enable(:board_new_list) -``` - -To disable it: - -```ruby -Feature.disable(:board_new_list) -``` - ### Enable or disable iteration lists in boards **(PREMIUM SELF)** -NOTE: -When disabling iteration lists in boards, you also need to disable the [new add list form](#enable-or-disable-new-add-list-form). - The iteration list is under development but ready for production use. It is deployed behind a feature flag that is **enabled by default**. [GitLab administrators with access to the GitLab Rails console](../../administration/feature_flags.md) diff --git a/lib/gitlab/ci/artifact_file_reader.rb b/lib/gitlab/ci/artifact_file_reader.rb index d576953c1a0..3cfed8e5e2c 100644 --- a/lib/gitlab/ci/artifact_file_reader.rb +++ b/lib/gitlab/ci/artifact_file_reader.rb @@ -45,7 +45,7 @@ module Gitlab end def read_zip_file!(file_path) - if ::Feature.enabled?(:ci_new_artifact_file_reader, job.project, default_enabled: false) + if ::Feature.enabled?(:ci_new_artifact_file_reader, job.project, default_enabled: :yaml) read_with_new_artifact_file_reader(file_path) else read_with_legacy_artifact_file_reader(file_path) diff --git a/lib/gitlab/ci/config/entry/default.rb b/lib/gitlab/ci/config/entry/default.rb index eaaf9f69102..145772c7a92 100644 --- a/lib/gitlab/ci/config/entry/default.rb +++ b/lib/gitlab/ci/config/entry/default.rb @@ -53,7 +53,7 @@ module Gitlab description: 'Set retry default value.', inherit: false - entry :tags, ::Gitlab::Config::Entry::ArrayOfStrings, + entry :tags, Entry::Tags, description: 'Set the default tags.', inherit: false diff --git a/lib/gitlab/ci/config/entry/job.rb b/lib/gitlab/ci/config/entry/job.rb index bd4d5f33689..f867189d521 100644 --- a/lib/gitlab/ci/config/entry/job.rb +++ b/lib/gitlab/ci/config/entry/job.rb @@ -85,7 +85,7 @@ module Gitlab description: 'Retry configuration for this job.', inherit: true - entry :tags, ::Gitlab::Config::Entry::ArrayOfStrings, + entry :tags, Entry::Tags, description: 'Set the tags.', inherit: true diff --git a/lib/gitlab/ci/config/entry/tags.rb b/lib/gitlab/ci/config/entry/tags.rb new file mode 100644 index 00000000000..ca3b48372e2 --- /dev/null +++ b/lib/gitlab/ci/config/entry/tags.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +module Gitlab + module Ci + class Config + module Entry + ## + # Entry that represents an array of tags. + # + class Tags < ::Gitlab::Config::Entry::Node + include ::Gitlab::Config::Entry::Validatable + + TAGS_LIMIT = 50 + + validations do + validates :config, array_of_strings: true + + validate do + next unless ::Feature.enabled?(:ci_build_tags_limit, default_enabled: :yaml) + + if config.is_a?(Array) && config.size >= TAGS_LIMIT + errors.add(:config, _("must be less than the limit of %{tag_limit} tags") % { tag_limit: TAGS_LIMIT }) + end + end + end + end + end + end + end +end diff --git a/lib/gitlab/git/commit.rb b/lib/gitlab/git/commit.rb index 7fd4acb4179..49fe365e09e 100644 --- a/lib/gitlab/git/commit.rb +++ b/lib/gitlab/git/commit.rb @@ -116,13 +116,9 @@ module Gitlab return [] if Gitlab::Git.blank_ref?(base) || Gitlab::Git.blank_ref?(head) wrapped_gitaly_errors do - if Feature.enabled?(:between_uses_list_commits, default_enabled: :yaml) - revisions = [head, "^#{base}"] # base..head + revisions = [head, "^#{base}"] # base..head - repo.gitaly_commit_client.list_commits(revisions, reverse: true) - else - repo.gitaly_commit_client.between(base, head) - end + repo.gitaly_commit_client.list_commits(revisions, reverse: true) end end diff --git a/lib/gitlab/usage_data_counters/known_events/analytics.yml b/lib/gitlab/usage_data_counters/known_events/analytics.yml index e4f20b61901..261bdeb9bfa 100644 --- a/lib/gitlab/usage_data_counters/known_events/analytics.yml +++ b/lib/gitlab/usage_data_counters/known_events/analytics.yml @@ -2,84 +2,67 @@ category: analytics redis_slot: analytics aggregation: weekly - feature_flag: track_unique_visits - name: i_analytics_dev_ops_adoption category: analytics redis_slot: analytics aggregation: weekly - feature_flag: track_unique_visits - name: i_analytics_dev_ops_score category: analytics redis_slot: analytics aggregation: weekly - feature_flag: track_unique_visits - name: p_analytics_merge_request category: analytics redis_slot: analytics aggregation: weekly - feature_flag: track_unique_visits - name: i_analytics_instance_statistics category: analytics redis_slot: analytics aggregation: weekly - feature_flag: track_unique_visits - name: g_analytics_contribution category: analytics redis_slot: analytics aggregation: weekly - feature_flag: track_unique_visits - name: g_analytics_insights category: analytics redis_slot: analytics aggregation: weekly - feature_flag: track_unique_visits - name: g_analytics_issues category: analytics redis_slot: analytics aggregation: weekly - feature_flag: track_unique_visits - name: g_analytics_productivity category: analytics redis_slot: analytics aggregation: weekly - feature_flag: track_unique_visits - name: g_analytics_valuestream category: analytics redis_slot: analytics aggregation: weekly - feature_flag: track_unique_visits - name: p_analytics_pipelines category: analytics redis_slot: analytics aggregation: weekly - feature_flag: track_unique_visits - name: p_analytics_code_reviews category: analytics redis_slot: analytics aggregation: weekly - feature_flag: track_unique_visits - name: p_analytics_valuestream category: analytics redis_slot: analytics aggregation: weekly - feature_flag: track_unique_visits - name: p_analytics_insights category: analytics redis_slot: analytics aggregation: weekly - feature_flag: track_unique_visits - name: p_analytics_issues category: analytics redis_slot: analytics aggregation: weekly - feature_flag: track_unique_visits - name: p_analytics_repo category: analytics redis_slot: analytics aggregation: weekly - feature_flag: track_unique_visits - name: i_analytics_cohorts category: analytics redis_slot: analytics aggregation: weekly - feature_flag: track_unique_visits diff --git a/lib/gitlab/usage_data_counters/known_events/common.yml b/lib/gitlab/usage_data_counters/known_events/common.yml index 585f0a399d3..87785602a0c 100644 --- a/lib/gitlab/usage_data_counters/known_events/common.yml +++ b/lib/gitlab/usage_data_counters/known_events/common.yml @@ -4,27 +4,22 @@ redis_slot: compliance category: compliance aggregation: weekly - feature_flag: track_unique_visits - name: g_compliance_audit_events category: compliance redis_slot: compliance aggregation: weekly - feature_flag: track_unique_visits - name: i_compliance_audit_events category: compliance redis_slot: compliance aggregation: weekly - feature_flag: track_unique_visits - name: i_compliance_credential_inventory category: compliance redis_slot: compliance aggregation: weekly - feature_flag: track_unique_visits - name: a_compliance_audit_events_api category: compliance redis_slot: compliance aggregation: weekly - feature_flag: track_unique_visits - name: g_edit_by_web_ide category: ide_edit redis_slot: edit diff --git a/locale/gitlab.pot b/locale/gitlab.pot index e337162e5f2..8e6cfa285f8 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -4650,9 +4650,6 @@ msgstr "" msgid "Assignee lists not available with your current license" msgstr "" -msgid "Assignee lists show all issues assigned to the selected user." -msgstr "" - msgid "Assignee(s)" msgstr "" @@ -19448,9 +19445,6 @@ msgstr "" msgid "Label actions dropdown" msgstr "" -msgid "Label lists show all issues with the selected label." -msgstr "" - msgid "Label priority" msgstr "" @@ -21526,9 +21520,6 @@ msgstr "" msgid "Milestone lists not available with your current license" msgstr "" -msgid "Milestone lists show all issues from the selected milestone." -msgstr "" - msgid "MilestoneCombobox|An error occurred while searching for milestones" msgstr "" @@ -39962,6 +39953,9 @@ msgstr "" msgid "must be inside the fork network" msgstr "" +msgid "must be less than the limit of %{tag_limit} tags" +msgstr "" + msgid "must be unique by status and elapsed time within a policy" msgstr "" diff --git a/package.json b/package.json index 6db536c2372..badee93d163 100644 --- a/package.json +++ b/package.json @@ -277,7 +277,7 @@ "chokidar": "^3.4.0" }, "engines": { - "node": ">=10.13.0", + "node": ">=12.22.1", "yarn": "^1.10.0" } } diff --git a/spec/factories/users.rb b/spec/factories/users.rb index 476c57f2d80..04bacbe14e7 100644 --- a/spec/factories/users.rb +++ b/spec/factories/users.rb @@ -11,10 +11,6 @@ FactoryBot.define do confirmation_token { nil } can_create_group { true } - after(:stub) do |user| - user.notification_email = user.email - end - trait :admin do admin { true } end diff --git a/spec/features/atom/dashboard_issues_spec.rb b/spec/features/atom/dashboard_issues_spec.rb index 511cdcc2940..855c91f70d7 100644 --- a/spec/features/atom/dashboard_issues_spec.rb +++ b/spec/features/atom/dashboard_issues_spec.rb @@ -4,8 +4,20 @@ require 'spec_helper' RSpec.describe "Dashboard Issues Feed" do describe "GET /issues" do - let!(:user) { create(:user, email: 'private1@example.com', public_email: 'public1@example.com') } - let!(:assignee) { create(:user, email: 'private2@example.com', public_email: 'public2@example.com') } + let!(:user) do + user = create(:user, email: 'private1@example.com') + public_email = create(:email, :confirmed, user: user, email: 'public1@example.com') + user.update!(public_email: public_email.email) + user + end + + let!(:assignee) do + user = create(:user, email: 'private2@example.com') + public_email = create(:email, :confirmed, user: user, email: 'public2@example.com') + user.update!(public_email: public_email.email) + user + end + let!(:project1) { create(:project) } let!(:project2) { create(:project) } diff --git a/spec/features/atom/issues_spec.rb b/spec/features/atom/issues_spec.rb index e6a862ddccc..913f5a7bcf3 100644 --- a/spec/features/atom/issues_spec.rb +++ b/spec/features/atom/issues_spec.rb @@ -4,11 +4,23 @@ require 'spec_helper' RSpec.describe 'Issues Feed' do describe 'GET /issues' do - let_it_be_with_reload(:user) { create(:user, email: 'private1@example.com', public_email: 'public1@example.com') } - let_it_be(:assignee) { create(:user, email: 'private2@example.com', public_email: 'public2@example.com') } - let_it_be(:group) { create(:group) } - let_it_be(:project) { create(:project) } - let_it_be(:issue) { create(:issue, author: user, assignees: [assignee], project: project, due_date: Date.today) } + let_it_be_with_reload(:user) do + user = create(:user, email: 'private1@example.com') + public_email = create(:email, :confirmed, user: user, email: 'public1@example.com') + user.update!(public_email: public_email.email) + user + end + + let_it_be(:assignee) do + user = create(:user, email: 'private2@example.com') + public_email = create(:email, :confirmed, user: user, email: 'public2@example.com') + user.update!(public_email: public_email.email) + user + end + + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project) } + let_it_be(:issue) { create(:issue, author: user, assignees: [assignee], project: project, due_date: Date.today) } let_it_be(:issuable) { issue } # "alias" for shared examples before_all do diff --git a/spec/features/boards/user_adds_lists_to_board_spec.rb b/spec/features/boards/user_adds_lists_to_board_spec.rb index 5128fc4004e..0db3fe12a3e 100644 --- a/spec/features/boards/user_adds_lists_to_board_spec.rb +++ b/spec/features/boards/user_adds_lists_to_board_spec.rb @@ -17,6 +17,8 @@ RSpec.describe 'User adds lists', :js do let_it_be(:project_label) { create(:label, project: project) } let_it_be(:group_backlog_list) { create(:backlog_list, board: group_board) } let_it_be(:project_backlog_list) { create(:backlog_list, board: project_board) } + let_it_be(:backlog) { create(:group_label, group: group, name: 'Backlog') } + let_it_be(:closed) { create(:group_label, group: group, name: 'Closed') } let_it_be(:issue) { create(:labeled_issue, project: project, labels: [group_label, project_label]) } @@ -25,15 +27,11 @@ RSpec.describe 'User adds lists', :js do group.add_owner(user) end - where(:board_type, :graphql_board_lists_enabled, :board_new_list_enabled) do - :project | true | true - :project | false | true - :project | true | false - :project | false | false - :group | true | true - :group | false | true - :group | true | false - :group | false | false + where(:board_type, :graphql_board_lists_enabled) do + :project | true + :project | false + :group | true + :group | false end with_them do @@ -43,8 +41,7 @@ RSpec.describe 'User adds lists', :js do set_cookie('sidebar_collapsed', 'true') stub_feature_flags( - graphql_board_lists: graphql_board_lists_enabled, - board_new_list: board_new_list_enabled + graphql_board_lists: graphql_board_lists_enabled ) if board_type == :project @@ -57,39 +54,44 @@ RSpec.describe 'User adds lists', :js do end it 'creates new column for label containing labeled issue' do - click_button button_text(board_new_list_enabled) + click_button 'Create list' wait_for_all_requests - select_label(board_new_list_enabled, group_label) + select_label(group_label) wait_for_all_requests expect(page).to have_selector('.board', text: group_label.title) expect(find('.board:nth-child(2) .board-card')).to have_content(issue.title) end - end - def select_label(board_new_list_enabled, label) - if board_new_list_enabled - click_button 'Select a label' + it 'creates new list for Backlog and closed labels' do + click_button 'Create list' + wait_for_requests - find('label', text: label.title).click + select_label(backlog) - click_button 'Add to board' + click_button 'Create list' + wait_for_requests - wait_for_all_requests - else - page.within('.dropdown-menu-issues-board-new') do - click_link label.title - end + select_label(closed) + + wait_for_requests + + expect(page).to have_selector('.board', text: closed.title) + expect(find('.board:nth-child(2) .board-header')).to have_content(backlog.title) + expect(find('.board:nth-child(3) .board-header')).to have_content(closed.title) + expect(find('.board:nth-child(4) .board-header')).to have_content('Closed') end end - def button_text(board_new_list_enabled) - if board_new_list_enabled - 'Create list' - else - 'Add list' - end + def select_label(label) + click_button 'Select a label' + + find('label', text: label.title).click + + click_button 'Add to board' + + wait_for_all_requests end end diff --git a/spec/features/ics/dashboard_issues_spec.rb b/spec/features/ics/dashboard_issues_spec.rb index 4a93a4b490a..1d0ea495757 100644 --- a/spec/features/ics/dashboard_issues_spec.rb +++ b/spec/features/ics/dashboard_issues_spec.rb @@ -4,8 +4,20 @@ require 'spec_helper' RSpec.describe 'Dashboard Issues Calendar Feed' do describe 'GET /issues' do - let!(:user) { create(:user, email: 'private1@example.com', public_email: 'public1@example.com') } - let!(:assignee) { create(:user, email: 'private2@example.com', public_email: 'public2@example.com') } + let!(:user) do + user = create(:user, email: 'private1@example.com') + public_email = create(:email, :confirmed, user: user, email: 'public1@example.com') + user.update!(public_email: public_email.email) + user + end + + let!(:assignee) do + user = create(:user, email: 'private2@example.com') + public_email = create(:email, :confirmed, user: user, email: 'public2@example.com') + user.update!(public_email: public_email.email) + user + end + let!(:project) { create(:project) } let(:milestone) { create(:milestone, project_id: project.id, title: 'v1.0') } diff --git a/spec/features/ics/group_issues_spec.rb b/spec/features/ics/group_issues_spec.rb index 05caca4b5a8..f29c39ad4ef 100644 --- a/spec/features/ics/group_issues_spec.rb +++ b/spec/features/ics/group_issues_spec.rb @@ -4,8 +4,20 @@ require 'spec_helper' RSpec.describe 'Group Issues Calendar Feed' do describe 'GET /issues' do - let!(:user) { create(:user, email: 'private1@example.com', public_email: 'public1@example.com') } - let!(:assignee) { create(:user, email: 'private2@example.com', public_email: 'public2@example.com') } + let!(:user) do + user = create(:user, email: 'private1@example.com') + public_email = create(:email, :confirmed, user: user, email: 'public1@example.com') + user.update!(public_email: public_email.email) + user + end + + let!(:assignee) do + user = create(:user, email: 'private2@example.com') + public_email = create(:email, :confirmed, user: user, email: 'public2@example.com') + user.update!(public_email: public_email.email) + user + end + let!(:group) { create(:group) } let!(:project) { create(:project, group: group) } diff --git a/spec/features/ics/project_issues_spec.rb b/spec/features/ics/project_issues_spec.rb index 58a1a32eac2..771748060bb 100644 --- a/spec/features/ics/project_issues_spec.rb +++ b/spec/features/ics/project_issues_spec.rb @@ -4,8 +4,20 @@ require 'spec_helper' RSpec.describe 'Project Issues Calendar Feed' do describe 'GET /issues' do - let!(:user) { create(:user, email: 'private1@example.com', public_email: 'public1@example.com') } - let!(:assignee) { create(:user, email: 'private2@example.com', public_email: 'public2@example.com') } + let!(:user) do + user = create(:user, email: 'private1@example.com') + public_email = create(:email, :confirmed, user: user, email: 'public1@example.com') + user.update!(public_email: public_email.email) + user + end + + let!(:assignee) do + user = create(:user, email: 'private2@example.com') + public_email = create(:email, :confirmed, user: user, email: 'public2@example.com') + user.update!(public_email: public_email.email) + user + end + let!(:project) { create(:project) } let!(:issue) { create(:issue, author: user, assignees: [assignee], project: project) } diff --git a/spec/features/issues/user_views_issues_spec.rb b/spec/features/issues/user_views_issues_spec.rb index 165f4b10cff..56afa7eb6ba 100644 --- a/spec/features/issues/user_views_issues_spec.rb +++ b/spec/features/issues/user_views_issues_spec.rb @@ -34,7 +34,7 @@ RSpec.describe "User views issues" do .and have_content(open_issue2.title) .and have_no_content(closed_issue.title) .and have_content(moved_open_issue.title) - .and have_no_selector(".js-new-board-list") + .and have_no_content('Create list') end it "opens issues by label" do @@ -65,7 +65,7 @@ RSpec.describe "User views issues" do .and have_no_content(open_issue1.title) .and have_no_content(open_issue2.title) .and have_no_content(moved_open_issue.title) - .and have_no_selector(".js-new-board-list") + .and have_no_content('Create list') end include_examples "opens issue from list" do @@ -87,7 +87,7 @@ RSpec.describe "User views issues" do .and have_content(open_issue2.title) .and have_content(moved_open_issue.title) .and have_no_content('CLOSED (MOVED)') - .and have_no_selector(".js-new-board-list") + .and have_no_content('Create list') end include_examples "opens issue from list" do diff --git a/spec/features/labels_hierarchy_spec.rb b/spec/features/labels_hierarchy_spec.rb index fca5e946d0c..378af53dd79 100644 --- a/spec/features/labels_hierarchy_spec.rb +++ b/spec/features/labels_hierarchy_spec.rb @@ -17,7 +17,6 @@ RSpec.describe 'Labels Hierarchy', :js do let!(:project_label_1) { create(:label, project: project_1, title: 'Label_4') } before do - stub_feature_flags(board_new_list: false) grandparent.add_owner(user) sign_in(user) @@ -307,88 +306,4 @@ RSpec.describe 'Labels Hierarchy', :js do it_behaves_like 'filtering by ancestor labels for groups', true end end - - context 'creating boards lists' do - before do - stub_feature_flags(board_new_list: false) - end - - context 'on project boards' do - let(:board) { create(:board, project: project_1) } - - before do - project_1.add_developer(user) - visit project_board_path(project_1, board) - find('.js-new-board-list').click - wait_for_requests - end - - it 'creates lists from all ancestor labels' do - [grandparent_group_label, parent_group_label, project_label_1].each do |label| - find('a', text: label.title).click - end - - wait_for_requests - - expect(page).to have_selector('.board-title-text', text: grandparent_group_label.title) - expect(page).to have_selector('.board-title-text', text: parent_group_label.title) - expect(page).to have_selector('.board-title-text', text: project_label_1.title) - end - end - - context 'on group boards' do - let(:board) { create(:board, group: parent) } - - before do - parent.add_developer(user) - visit group_board_path(parent, board) - find('.js-new-board-list').click - wait_for_requests - end - - context 'when graphql_board_lists FF enabled' do - it 'creates lists from all ancestor group labels' do - [grandparent_group_label, parent_group_label].each do |label| - find('a', text: label.title).click - end - - wait_for_requests - - expect(page).to have_selector('.board-title-text', text: grandparent_group_label.title) - expect(page).to have_selector('.board-title-text', text: parent_group_label.title) - end - - it 'does not create lists from descendant groups' do - expect(page).not_to have_selector('a', text: child_group_label.title) - end - end - end - - context 'when graphql_board_lists FF disabled' do - let(:board) { create(:board, group: parent) } - - before do - stub_feature_flags(graphql_board_lists: false) - parent.add_developer(user) - visit group_board_path(parent, board) - find('.js-new-board-list').click - wait_for_requests - end - - it 'creates lists from all ancestor group labels' do - [grandparent_group_label, parent_group_label].each do |label| - find('a', text: label.title).click - end - - wait_for_requests - - expect(page).to have_selector('.board-title-text', text: grandparent_group_label.title) - expect(page).to have_selector('.board-title-text', text: parent_group_label.title) - end - - it 'does not create lists from descendant groups' do - expect(page).not_to have_selector('a', text: child_group_label.title) - end - end - end end diff --git a/spec/frontend/pipeline_editor/components/commit/commit_section_spec.js b/spec/frontend/pipeline_editor/components/commit/commit_section_spec.js index 39081e07e52..20c4a0fbf6a 100644 --- a/spec/frontend/pipeline_editor/components/commit/commit_section_spec.js +++ b/spec/frontend/pipeline_editor/components/commit/commit_section_spec.js @@ -1,5 +1,6 @@ import { GlFormTextarea, GlFormInput, GlLoadingIcon } from '@gitlab/ui'; import { mount } from '@vue/test-utils'; +import waitForPromises from 'helpers/wait_for_promises'; import { objectToQuery, redirectTo } from '~/lib/utils/url_utility'; import CommitForm from '~/pipeline_editor/components/commit/commit_form.vue'; import CommitSection from '~/pipeline_editor/components/commit/commit_section.vue'; @@ -48,7 +49,10 @@ describe('Pipeline Editor | Commit section', () => { let wrapper; let mockMutate; - const defaultProps = { ciFileContent: mockCiYml }; + const defaultProps = { + ciFileContent: mockCiYml, + commitSha: mockCommitSha, + }; const createComponent = ({ props = {}, options = {}, provide = {} } = {}) => { mockMutate = jest.fn().mockResolvedValue({ @@ -67,7 +71,6 @@ describe('Pipeline Editor | Commit section', () => { provide: { ...mockProvide, ...provide }, data() { return { - commitSha: mockCommitSha, currentBranch: mockDefaultBranch, isNewCiConfigFile: Boolean(options?.isNewCiConfigfile), }; @@ -97,8 +100,7 @@ describe('Pipeline Editor | Commit section', () => { await findCommitForm().find('[data-testid="new-mr-checkbox"]').setChecked(openMergeRequest); } await findCommitForm().find('[type="submit"]').trigger('click'); - // Simulate the write to local cache that occurs after a commit - await wrapper.setData({ commitSha: mockCommitNextSha }); + await waitForPromises(); }; const cancelCommitForm = async () => { @@ -188,7 +190,6 @@ describe('Pipeline Editor | Commit section', () => { update: expect.any(Function), variables: { ...mockVariables, - lastCommitId: mockCommitNextSha, branch: mockDefaultBranch, }, }); diff --git a/spec/frontend/pipeline_editor/components/editor/text_editor_spec.js b/spec/frontend/pipeline_editor/components/editor/text_editor_spec.js index c6c7f593cc5..85222f2ecbb 100644 --- a/spec/frontend/pipeline_editor/components/editor/text_editor_spec.js +++ b/spec/frontend/pipeline_editor/components/editor/text_editor_spec.js @@ -42,15 +42,12 @@ describe('Pipeline Editor | Text editor component', () => { defaultBranch: mockDefaultBranch, glFeatures, }, + propsData: { + commitSha: mockCommitSha, + }, attrs: { value: mockCiYml, }, - // Simulate graphQL client query result - data() { - return { - commitSha: mockCommitSha, - }; - }, listeners: { [EDITOR_READY_EVENT]: editorReadyListener, }, diff --git a/spec/frontend/pipeline_editor/components/file-nav/branch_switcher_spec.js b/spec/frontend/pipeline_editor/components/file-nav/branch_switcher_spec.js index 85b51d08f88..b5881790b0b 100644 --- a/spec/frontend/pipeline_editor/components/file-nav/branch_switcher_spec.js +++ b/spec/frontend/pipeline_editor/components/file-nav/branch_switcher_spec.js @@ -247,15 +247,6 @@ describe('Pipeline editor branch switcher', () => { expect(wrapper.emitted('refetchContent')).toBeUndefined(); }); - - it('emits the updateCommitSha event when selecting a different branch', async () => { - expect(wrapper.emitted('updateCommitSha')).toBeUndefined(); - - const branch = findDropdownItems().at(1); - branch.vm.$emit('click'); - - expect(wrapper.emitted('updateCommitSha')).toHaveLength(1); - }); }); describe('when searching', () => { diff --git a/spec/frontend/pipeline_editor/components/header/pipeline_status_spec.js b/spec/frontend/pipeline_editor/components/header/pipeline_status_spec.js index a95921359cc..753682d438b 100644 --- a/spec/frontend/pipeline_editor/components/header/pipeline_status_spec.js +++ b/spec/frontend/pipeline_editor/components/header/pipeline_status_spec.js @@ -27,13 +27,11 @@ describe('Pipeline Status', () => { wrapper = shallowMount(PipelineStatus, { localVue, apolloProvider: mockApollo, + propsData: { + commitSha: mockCommitSha, + }, provide: mockProvide, stubs: { GlLink, GlSprintf }, - data() { - return { - commitSha: mockCommitSha, - }; - }, }); }; diff --git a/spec/frontend/pipeline_editor/mock_data.js b/spec/frontend/pipeline_editor/mock_data.js index 4d4a8c21d78..19ad4f051ba 100644 --- a/spec/frontend/pipeline_editor/mock_data.js +++ b/spec/frontend/pipeline_editor/mock_data.js @@ -156,35 +156,33 @@ export const mergeUnwrappedCiConfig = (mergedConfig) => { }; }; -export const mockNewCommitShaResults = { +export const mockCommitShaResults = { data: { project: { pipelines: { nodes: [ { id: 'gid://gitlab/Ci::Pipeline/1', - sha: 'd0d56d363d8a3f67a8ab9fc00207d468f30032ca', + sha: mockCommitSha, path: `/${mockProjectFullPath}/-/pipelines/488`, commitPath: `/${mockProjectFullPath}/-/commit/d0d56d363d8a3f67a8ab9fc00207d468f30032ca`, }, - { - id: 'gid://gitlab/Ci::Pipeline/2', - sha: 'fcab2ece40b26f428dfa3aa288b12c3c5bdb06aa', - path: `/${mockProjectFullPath}/-/pipelines/487`, - commitPath: `/${mockProjectFullPath}/-/commit/fcab2ece40b26f428dfa3aa288b12c3c5bdb06aa`, - }, - { - id: 'gid://gitlab/Ci::Pipeline/3', - sha: '6c16b17c7f94a438ae19a96c285bb49e3c632cf4', - path: `/${mockProjectFullPath}/-/pipelines/433`, - commitPath: `/${mockProjectFullPath}/-/commit/6c16b17c7f94a438ae19a96c285bb49e3c632cf4`, - }, ], }, }, }, }; +export const mockEmptyCommitShaResults = { + data: { + project: { + pipelines: { + nodes: [], + }, + }, + }, +}; + export const mockProjectBranches = { data: { project: { diff --git a/spec/frontend/pipeline_editor/pipeline_editor_app_spec.js b/spec/frontend/pipeline_editor/pipeline_editor_app_spec.js index 0c5c08d7190..affe45f1b1d 100644 --- a/spec/frontend/pipeline_editor/pipeline_editor_app_spec.js +++ b/spec/frontend/pipeline_editor/pipeline_editor_app_spec.js @@ -26,9 +26,10 @@ import { mockBlobContentQueryResponseNoCiFile, mockCiYml, mockCommitSha, + mockCommitShaResults, mockDefaultBranch, + mockEmptyCommitShaResults, mockProjectFullPath, - mockNewCommitShaResults, } from './mock_data'; const localVue = createLocalVue(); @@ -54,7 +55,6 @@ describe('Pipeline editor app component', () => { let mockBlobContentData; let mockCiConfigData; let mockGetTemplate; - let mockUpdateCommitSha; let mockLatestCommitShaQuery; let mockPipelineQuery; @@ -71,6 +71,11 @@ describe('Pipeline editor app component', () => { SourceEditor: MockSourceEditor, PipelineEditorEmptyState, }, + data() { + return { + commitSha: '', + }; + }, mocks: { $apollo: { queries: { @@ -96,18 +101,7 @@ describe('Pipeline editor app component', () => { [getPipelineQuery, mockPipelineQuery], ]; - const resolvers = { - Query: { - commitSha() { - return mockCommitSha; - }, - }, - Mutation: { - updateCommitSha: mockUpdateCommitSha, - }, - }; - - mockApollo = createMockApollo(handlers, resolvers); + mockApollo = createMockApollo(handlers); const options = { localVue, @@ -137,7 +131,6 @@ describe('Pipeline editor app component', () => { mockBlobContentData = jest.fn(); mockCiConfigData = jest.fn(); mockGetTemplate = jest.fn(); - mockUpdateCommitSha = jest.fn(); mockLatestCommitShaQuery = jest.fn(); mockPipelineQuery = jest.fn(); }); @@ -159,11 +152,16 @@ describe('Pipeline editor app component', () => { beforeEach(() => { mockBlobContentData.mockResolvedValue(mockBlobContentQueryResponse); mockCiConfigData.mockResolvedValue(mockCiConfigQueryResponse); + mockLatestCommitShaQuery.mockResolvedValue(mockCommitShaResults); }); describe('when file exists', () => { beforeEach(async () => { await createComponentWithApollo(); + + jest + .spyOn(wrapper.vm.$apollo.queries.commitSha, 'startPolling') + .mockImplementation(jest.fn()); }); it('shows pipeline editor home component', () => { @@ -181,18 +179,32 @@ describe('Pipeline editor app component', () => { sha: mockCommitSha, }); }); + + it('does not poll for the commit sha', () => { + expect(wrapper.vm.$apollo.queries.commitSha.startPolling).toHaveBeenCalledTimes(0); + }); }); describe('when no CI config file exists', () => { - it('shows an empty state and does not show editor home component', async () => { + beforeEach(async () => { mockBlobContentData.mockResolvedValue(mockBlobContentQueryResponseNoCiFile); await createComponentWithApollo(); + jest + .spyOn(wrapper.vm.$apollo.queries.commitSha, 'startPolling') + .mockImplementation(jest.fn()); + }); + + it('shows an empty state and does not show editor home component', async () => { expect(findEmptyState().exists()).toBe(true); expect(findAlert().exists()).toBe(false); expect(findEditorHome().exists()).toBe(false); }); + it('does not poll for the commit sha', () => { + expect(wrapper.vm.$apollo.queries.commitSha.startPolling).toHaveBeenCalledTimes(0); + }); + describe('because of a fetching error', () => { it('shows a unkown error message', async () => { const loadUnknownFailureText = 'The CI configuration was not loaded, please try again.'; @@ -230,6 +242,7 @@ describe('Pipeline editor app component', () => { describe('when landing on the empty state with feature flag on', () => { it('user can click on CTA button and see an empty editor', async () => { mockBlobContentData.mockResolvedValue(mockBlobContentQueryResponseNoCiFile); + mockLatestCommitShaQuery.mockResolvedValue(mockEmptyCommitShaResults); await createComponentWithApollo({ provide: { @@ -254,9 +267,9 @@ describe('Pipeline editor app component', () => { const updateSuccessMessage = 'Your changes have been successfully committed.'; describe('and the commit mutation succeeds', () => { - beforeEach(() => { + beforeEach(async () => { window.scrollTo = jest.fn(); - createComponent(); + await createComponentWithApollo(); findEditorHome().vm.$emit('commit', { type: COMMIT_SUCCESS }); }); @@ -268,7 +281,32 @@ describe('Pipeline editor app component', () => { it('scrolls to the top of the page to bring attention to the confirmation message', () => { expect(window.scrollTo).toHaveBeenCalledWith({ top: 0, behavior: 'smooth' }); }); + + it('polls for commit sha while pipeline data is not yet available', async () => { + jest + .spyOn(wrapper.vm.$apollo.queries.commitSha, 'startPolling') + .mockImplementation(jest.fn()); + + // simulate updating current branch (which triggers commitSha refetch) + // while pipeline data is not yet available + mockLatestCommitShaQuery.mockResolvedValue(mockEmptyCommitShaResults); + await wrapper.vm.$apollo.queries.commitSha.refetch(); + + expect(wrapper.vm.$apollo.queries.commitSha.startPolling).toHaveBeenCalledTimes(1); + }); + + it('stops polling for commit sha when pipeline data is available', async () => { + jest + .spyOn(wrapper.vm.$apollo.queries.commitSha, 'stopPolling') + .mockImplementation(jest.fn()); + + mockLatestCommitShaQuery.mockResolvedValue(mockCommitShaResults); + await wrapper.vm.$apollo.queries.commitSha.refetch(); + + expect(wrapper.vm.$apollo.queries.commitSha.stopPolling).toHaveBeenCalledTimes(1); + }); }); + describe('and the commit mutation fails', () => { const commitFailedReasons = ['Commit failed']; @@ -320,6 +358,10 @@ describe('Pipeline editor app component', () => { }); describe('when refetching content', () => { + beforeEach(() => { + mockLatestCommitShaQuery.mockResolvedValue(mockCommitShaResults); + }); + it('refetches blob content', async () => { await createComponentWithApollo(); jest @@ -352,6 +394,7 @@ describe('Pipeline editor app component', () => { const originalLocation = window.location.href; beforeEach(() => { + mockLatestCommitShaQuery.mockResolvedValue(mockCommitShaResults); setWindowLocation('?template=Android'); }); @@ -371,45 +414,4 @@ describe('Pipeline editor app component', () => { expect(findTextEditor().exists()).toBe(true); }); }); - - describe('when updating commit sha', () => { - const newCommitSha = mockNewCommitShaResults.data.project.pipelines.nodes[0].sha; - - beforeEach(async () => { - mockUpdateCommitSha.mockResolvedValue(newCommitSha); - mockLatestCommitShaQuery.mockResolvedValue(mockNewCommitShaResults); - await createComponentWithApollo(); - }); - - it('fetches updated commit sha for the new branch', async () => { - expect(mockLatestCommitShaQuery).not.toHaveBeenCalled(); - - wrapper - .findComponent(PipelineEditorHome) - .vm.$emit('updateCommitSha', { newBranch: 'new-branch' }); - await waitForPromises(); - - expect(mockLatestCommitShaQuery).toHaveBeenCalledWith({ - projectPath: mockProjectFullPath, - ref: 'new-branch', - }); - }); - - it('updates commit sha with the newly fetched commit sha', async () => { - expect(mockUpdateCommitSha).not.toHaveBeenCalled(); - - wrapper - .findComponent(PipelineEditorHome) - .vm.$emit('updateCommitSha', { newBranch: 'new-branch' }); - await waitForPromises(); - - expect(mockUpdateCommitSha).toHaveBeenCalled(); - expect(mockUpdateCommitSha).toHaveBeenCalledWith( - expect.any(Object), - { commitSha: mockNewCommitShaResults.data.project.pipelines.nodes[0].sha }, - expect.any(Object), - expect.any(Object), - ); - }); - }); }); diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb index 4dac4403f70..85b572d3f68 100644 --- a/spec/helpers/projects_helper_spec.rb +++ b/spec/helpers/projects_helper_spec.rb @@ -498,7 +498,7 @@ RSpec.describe ProjectsHelper do context 'user has a configured commit email' do before do confirmed_email = create(:email, :confirmed, user: user) - user.update!(commit_email: confirmed_email) + user.update!(commit_email: confirmed_email.email) end it 'returns the commit email' do diff --git a/spec/lib/gitlab/ci/config/entry/job_spec.rb b/spec/lib/gitlab/ci/config/entry/job_spec.rb index 5b47d3a3922..4a90e765d4b 100644 --- a/spec/lib/gitlab/ci/config/entry/job_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/job_spec.rb @@ -618,6 +618,29 @@ RSpec.describe Gitlab::Ci::Config::Entry::Job do end end end + + context 'when job is using tags' do + context 'when limit is reached' do + let(:tags) { Array.new(100) { |i| "tag-#{i}" } } + let(:config) { { tags: tags, script: 'test' } } + + it 'returns error', :aggregate_failures do + expect(entry).not_to be_valid + expect(entry.errors) + .to include "tags config must be less than the limit of #{Gitlab::Ci::Config::Entry::Tags::TAGS_LIMIT} tags" + end + end + + context 'when limit is not reached' do + let(:config) { { tags: %w[tag1 tag2], script: 'test' } } + + it 'returns a valid entry', :aggregate_failures do + expect(entry).to be_valid + expect(entry.errors).to be_empty + expect(entry.tags).to eq(%w[tag1 tag2]) + end + end + end end describe '#manual_action?' do diff --git a/spec/lib/gitlab/ci/config/entry/tags_spec.rb b/spec/lib/gitlab/ci/config/entry/tags_spec.rb new file mode 100644 index 00000000000..79317de373b --- /dev/null +++ b/spec/lib/gitlab/ci/config/entry/tags_spec.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Ci::Config::Entry::Tags do + let(:entry) { described_class.new(config) } + + describe 'validation' do + context 'when tags config value is correct' do + let(:config) { %w[tag1 tag2] } + + describe '#value' do + it 'returns tags configuration' do + expect(entry.value).to eq config + end + end + + describe '#valid?' do + it 'is valid' do + expect(entry).to be_valid + end + end + end + + context 'when entry value is not correct' do + describe '#errors' do + context 'when tags config is not an array of strings' do + let(:config) { [1, 2] } + + it 'reports error' do + expect(entry.errors) + .to include 'tags config should be an array of strings' + end + end + + context 'when tags limit is reached' do + let(:config) { Array.new(50) {|i| "tag-#{i}" } } + + context 'when ci_build_tags_limit is enabled' do + before do + stub_feature_flags(ci_build_tags_limit: true) + end + + it 'reports error' do + expect(entry.errors) + .to include "tags config must be less than the limit of #{described_class::TAGS_LIMIT} tags" + end + end + + context 'when ci_build_tags_limit is disabled' do + before do + stub_feature_flags(ci_build_tags_limit: false) + end + + it 'does not report an error' do + expect(entry.errors).to be_empty + end + end + end + end + end + end +end diff --git a/spec/lib/gitlab/git/commit_spec.rb b/spec/lib/gitlab/git/commit_spec.rb index f58bab52cfa..174ed43bd40 100644 --- a/spec/lib/gitlab/git/commit_spec.rb +++ b/spec/lib/gitlab/git/commit_spec.rb @@ -370,14 +370,6 @@ RSpec.describe Gitlab::Git::Commit, :seed_helper do end it { is_expected.to contain_exactly(SeedRepo::Commit::ID) } - - context 'between_uses_list_commits FF disabled' do - before do - stub_feature_flags(between_uses_list_commits: false) - end - - it { is_expected.to contain_exactly(SeedRepo::Commit::ID) } - end end describe '.shas_with_signatures' do diff --git a/spec/models/ci/pending_build_spec.rb b/spec/models/ci/pending_build_spec.rb index 0518c9a1652..65c702941fe 100644 --- a/spec/models/ci/pending_build_spec.rb +++ b/spec/models/ci/pending_build_spec.rb @@ -113,5 +113,31 @@ RSpec.describe Ci::PendingBuild do end end end + + context 'when build has tags' do + let!(:build) { create(:ci_build, :tags) } + + subject(:ci_pending_build) { described_class.last } + + context 'when ci_pending_builds_maintain_tags_data is enabled' do + it 'sets tag_ids' do + described_class.upsert_from_build!(build) + + expect(ci_pending_build.tag_ids).to eq(build.tags_ids) + end + end + + context 'when ci_pending_builds_maintain_tags_data is disabled' do + before do + stub_feature_flags(ci_pending_builds_maintain_tags_data: false) + end + + it 'does not set tag_ids' do + described_class.upsert_from_build!(build) + + expect(ci_pending_build.tag_ids).to be_empty + end + end + end end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 17d47c838b6..494d1de681d 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -465,24 +465,19 @@ RSpec.describe User do user.commit_email = confirmed.email expect(user).to be_valid - expect(user.commit_email).to eq(confirmed.email) end it 'can not be set to an unconfirmed email' do unconfirmed = create(:email, user: user) user.commit_email = unconfirmed.email - # This should set the commit_email attribute to the primary email - expect(user).to be_valid - expect(user.commit_email).to eq(user.email) + expect(user).not_to be_valid end it 'can not be set to a non-existent email' do user.commit_email = 'non-existent-email@nonexistent.nonexistent' - # This should set the commit_email attribute to the primary email - expect(user).to be_valid - expect(user.commit_email).to eq(user.email) + expect(user).not_to be_valid end it 'can not be set to an invalid email, even if confirmed' do @@ -691,75 +686,6 @@ RSpec.describe User do end end end - - context 'owns_notification_email' do - it 'accepts temp_oauth_email emails' do - user = build(:user, email: "temp-email-for-oauth@example.com") - expect(user).to be_valid - end - - it 'does not accept not verified emails' do - email = create(:email) - user = email.user - user.notification_email = email.email - - expect(user).to be_invalid - expect(user.errors[:notification_email]).to include(_('must be an email you have verified')) - end - end - - context 'owns_public_email' do - it 'accepts verified emails' do - email = create(:email, :confirmed, email: 'test@test.com') - user = email.user - user.notification_email = email.email - - expect(user).to be_valid - end - - it 'does not accept not verified emails' do - email = create(:email) - user = email.user - user.public_email = email.email - - expect(user).to be_invalid - expect(user.errors[:public_email]).to include(_('must be an email you have verified')) - end - end - - context 'set_commit_email' do - it 'keeps commit email when private commit email is being used' do - user = create(:user, commit_email: Gitlab::PrivateCommitEmail::TOKEN) - - expect(user.read_attribute(:commit_email)).to eq(Gitlab::PrivateCommitEmail::TOKEN) - end - - it 'keeps the commit email when nil' do - user = create(:user, commit_email: nil) - - expect(user.read_attribute(:commit_email)).to be_nil - end - - it 'reverts to nil when email is not verified' do - user = create(:user, commit_email: "foo@bar.com") - - expect(user.read_attribute(:commit_email)).to be_nil - end - end - - context 'owns_commit_email' do - it 'accepts private commit email' do - user = build(:user, commit_email: Gitlab::PrivateCommitEmail::TOKEN) - - expect(user).to be_valid - end - - it 'accepts nil commit email' do - user = build(:user, commit_email: nil) - - expect(user).to be_valid - end - end end end diff --git a/spec/services/ci/create_pipeline_service/tags_spec.rb b/spec/services/ci/create_pipeline_service/tags_spec.rb new file mode 100644 index 00000000000..4c1d3e3e4a3 --- /dev/null +++ b/spec/services/ci/create_pipeline_service/tags_spec.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true +require 'spec_helper' + +RSpec.describe Ci::CreatePipelineService do + describe 'tags:' do + let_it_be(:project) { create(:project, :repository) } + let_it_be(:user) { project.owner } + + let(:ref) { 'refs/heads/master' } + let(:source) { :push } + let(:service) { described_class.new(project, user, { ref: ref }) } + let(:pipeline) { service.execute(source).payload } + + before do + stub_ci_pipeline_yaml_file(config) + end + + context 'with valid config' do + let(:config) { YAML.dump({ test: { script: 'ls', tags: %w[tag1 tag2] } }) } + + it 'creates a pipeline', :aggregate_failures do + expect(pipeline).to be_created_successfully + expect(pipeline.builds.first.tag_list).to eq(%w[tag1 tag2]) + end + end + + context 'with too many tags' do + let(:tags) { Array.new(50) {|i| "tag-#{i}" } } + let(:config) { YAML.dump({ test: { script: 'ls', tags: tags } }) } + + it 'creates a pipeline without builds', :aggregate_failures do + expect(pipeline).not_to be_created_successfully + expect(pipeline.builds).to be_empty + expect(pipeline.yaml_errors).to eq("jobs:test:tags config must be less than the limit of #{Gitlab::Ci::Config::Entry::Tags::TAGS_LIMIT} tags") + end + end + end +end diff --git a/spec/services/ci/register_job_service_spec.rb b/spec/services/ci/register_job_service_spec.rb index 2f37d0ea42d..48d25b91d8a 100644 --- a/spec/services/ci/register_job_service_spec.rb +++ b/spec/services/ci/register_job_service_spec.rb @@ -739,6 +739,22 @@ module Ci include_examples 'handles runner assignment' end + + context 'with ci_queueing_denormalize_tags_information enabled' do + before do + stub_feature_flags(ci_queueing_denormalize_tags_information: true) + end + + include_examples 'handles runner assignment' + end + + context 'with ci_queueing_denormalize_tags_information disabled' do + before do + stub_feature_flags(ci_queueing_denormalize_tags_information: false) + end + + include_examples 'handles runner assignment' + end end context 'when not using pending builds table' do diff --git a/spec/support/database/ci_tables.rb b/spec/support/database/ci_tables.rb deleted file mode 100644 index 99fc7ac2501..00000000000 --- a/spec/support/database/ci_tables.rb +++ /dev/null @@ -1,22 +0,0 @@ -# frozen_string_literal: true - -# This module stores the CI-related database tables which are -# going to be moved to a separate database. -module Database - module CiTables - def self.include?(name) - ci_tables.include?(name) - end - - def self.ci_tables - @@ci_tables ||= Set.new.tap do |tables| # rubocop:disable Style/ClassVars - tables.merge(Ci::ApplicationRecord.descendants.map(&:table_name).compact) - - # It was decided that taggings/tags are best placed with CI - # https://gitlab.com/gitlab-org/gitlab/-/issues/333413 - tables.add('taggings') - tables.add('tags') - end - end - end -end diff --git a/spec/support/database/gitlab_schema.rb b/spec/support/database/gitlab_schema.rb new file mode 100644 index 00000000000..fe05fb998e6 --- /dev/null +++ b/spec/support/database/gitlab_schema.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +# This module gathes information about table to schema mapping +# to understand table affinity +module Database + module GitlabSchema + def self.table_schemas(tables) + tables.map { |table| table_schema(table) }.to_set + end + + def self.table_schema(name) + tables_to_schema[name] || :undefined + end + + def self.tables_to_schema + @tables_to_schema ||= all_classes_with_schema.to_h do |klass| + [klass.table_name, klass.gitlab_schema] + end + end + + def self.all_classes_with_schema + ActiveRecord::Base.descendants.reject(&:abstract_class?).select(&:gitlab_schema?) # rubocop:disable Database/MultipleDatabases + end + end +end diff --git a/spec/support/database/prevent_cross_database_modification.rb b/spec/support/database/prevent_cross_database_modification.rb index 460ee99391b..587d1fec586 100644 --- a/spec/support/database/prevent_cross_database_modification.rb +++ b/spec/support/database/prevent_cross_database_modification.rb @@ -75,17 +75,17 @@ module Database return if cross_database_context[:transaction_depth_by_db].values.all?(&:zero?) tables = PgQuery.parse(sql).dml_tables - return if tables.empty? cross_database_context[:modified_tables_by_db][database].merge(tables) all_tables = cross_database_context[:modified_tables_by_db].values.map(&:to_a).flatten + schemas = Database::GitlabSchema.table_schemas(all_tables) - unless PreventCrossJoins.only_ci_or_only_main?(all_tables) + if schemas.many? raise Database::PreventCrossDatabaseModification::CrossDatabaseModificationAcrossUnsupportedTablesError, - "Cross-database data modification queries (CI and Main) were detected within " \ - "a transaction '#{all_tables.join(", ")}' discovered" + "Cross-database data modification of '#{schemas.to_a.join(", ")}' were detected within " \ + "a transaction modifying the '#{all_tables.to_a.join(", ")}'" end end end diff --git a/spec/support/database/prevent_cross_joins.rb b/spec/support/database/prevent_cross_joins.rb index 789721ccd38..22e1885ffb9 100644 --- a/spec/support/database/prevent_cross_joins.rb +++ b/spec/support/database/prevent_cross_joins.rb @@ -27,20 +27,15 @@ module Database # PgQuery might fail in some cases due to limited nesting: # https://github.com/pganalyze/pg_query/issues/209 tables = PgQuery.parse(sql).tables + schemas = Database::GitlabSchema.table_schemas(tables) - unless only_ci_or_only_main?(tables) + if schemas.many? raise CrossJoinAcrossUnsupportedTablesError, - "Unsupported cross-join across '#{tables.join(", ")}' discovered " \ + "Unsupported cross-join across '#{tables.join(", ")}' modifying '#{schemas.to_a.join(", ")}' discovered " \ "when executing query '#{sql}'" end end - # Returns true if a set includes only CI tables, or includes only non-CI tables - def self.only_ci_or_only_main?(tables) - tables.all? { |table| CiTables.include?(table) } || - tables.none? { |table| CiTables.include?(table) } - end - module SpecHelpers def with_cross_joins_prevented subscriber = ActiveSupport::Notifications.subscribe('sql.active_record') do |event| diff --git a/spec/support/shared_examples/boards/multiple_issue_boards_shared_examples.rb b/spec/support/shared_examples/boards/multiple_issue_boards_shared_examples.rb index cadc753513d..1e303197990 100644 --- a/spec/support/shared_examples/boards/multiple_issue_boards_shared_examples.rb +++ b/spec/support/shared_examples/boards/multiple_issue_boards_shared_examples.rb @@ -3,14 +3,10 @@ RSpec.shared_examples 'multiple issue boards' do context 'authorized user' do before do - stub_feature_flags(board_new_list: false) - parent.add_maintainer(user) login_as(user) - stub_feature_flags(board_new_list: false) - visit boards_path wait_for_requests end @@ -79,13 +75,13 @@ RSpec.shared_examples 'multiple issue boards' do expect(page).to have_content(board2.name) end - click_button 'Add list' + click_button 'Create list' - wait_for_requests + click_button 'Select a label' - page.within '.dropdown-menu-issues-board-new' do - click_link planning.title - end + page.choose(planning.title) + + click_button 'Add to board' wait_for_requests diff --git a/spec/support_specs/database/prevent_cross_database_modification_spec.rb b/spec/support_specs/database/prevent_cross_database_modification_spec.rb index 4fd55d59db0..5ea465356a1 100644 --- a/spec/support_specs/database/prevent_cross_database_modification_spec.rb +++ b/spec/support_specs/database/prevent_cross_database_modification_spec.rb @@ -66,7 +66,7 @@ RSpec.describe 'Database::PreventCrossDatabaseModification' do pipeline.touch end end - end.to raise_error /Cross-database data modification queries/ + end.to raise_error /Cross-database data modification/ end end @@ -84,7 +84,7 @@ RSpec.describe 'Database::PreventCrossDatabaseModification' do context 'when data modification happens in a transaction' do it 'raises error' do Project.transaction do - expect { run_queries }.to raise_error /Cross-database data modification queries/ + expect { run_queries }.to raise_error /Cross-database data modification/ end end @@ -93,7 +93,7 @@ RSpec.describe 'Database::PreventCrossDatabaseModification' do Project.transaction(requires_new: true) do project.touch Project.transaction(requires_new: true) do - expect { pipeline.touch }.to raise_error /Cross-database data modification queries/ + expect { pipeline.touch }.to raise_error /Cross-database data modification/ end end end @@ -127,7 +127,7 @@ RSpec.describe 'Database::PreventCrossDatabaseModification' do ApplicationRecord.transaction do create(:ci_pipeline) end - end.to raise_error /Cross-database data modification queries/ + end.to raise_error /Cross-database data modification/ end it 'skips raising error on factory creation' do |