diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-08-18 18:10:10 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-08-18 18:10:10 +0000 |
commit | 85f7fa54f404f28b0f351c2be0f7a6e9d74fe65f (patch) | |
tree | b0f4a7578f374185fb649be904641cd79baa2ca0 | |
parent | a8a9c520128bffc1157db4dc1beaa215fc731c80 (diff) | |
download | gitlab-ce-85f7fa54f404f28b0f351c2be0f7a6e9d74fe65f.tar.gz |
Add latest changes from gitlab-org/gitlab@master
108 files changed, 2634 insertions, 1175 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index cf3ef0434ea..3ea88385f9b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,11 @@ entry. ## 13.2.5 (2020-08-17) -- No changes. +### Security (2 changes) + +- Stop deploy token being mis-used as user in ProjectPolicy and GroupPolicy. +- Project access is checked during deploy token authentication. + ## 13.2.4 (2020-08-11) @@ -1072,7 +1076,11 @@ entry. ## 13.1.7 (2020-08-17) -- No changes. +### Security (2 changes) + +- Stop deploy token being mis-used as user in ProjectPolicy and GroupPolicy. +- Project access is checked during deploy token authentication. + ## 13.1.6 (2020-08-05) @@ -1631,7 +1639,11 @@ entry. ## 13.0.13 (2020-08-17) -- No changes. +### Security (2 changes) + +- Stop deploy token being mis-used as user in ProjectPolicy and GroupPolicy. +- Project access is checked during deploy token authentication. + ## 13.0.12 (2020-08-05) diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 379a56d288b..b0d59f532f4 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -8302f636d0a3f1b83cb7e5420b2720e83e564306 +397a8aa41c8b1b159a667fb262aebc644719e074 diff --git a/GITLAB_SHELL_VERSION b/GITLAB_SHELL_VERSION index d224e69099c..649e128e1b0 100644 --- a/GITLAB_SHELL_VERSION +++ b/GITLAB_SHELL_VERSION @@ -1 +1 @@ -13.5.0 +13.6.0
\ No newline at end of file @@ -406,7 +406,7 @@ group :test do gem 'rspec_profiling', '~> 0.0.5' gem 'rspec-parameterized', require: false - gem 'capybara', '~> 3.22.0' + gem 'capybara', '~> 3.33.0' gem 'capybara-screenshot', '~> 1.0.22' gem 'selenium-webdriver', '~> 3.142' diff --git a/Gemfile.lock b/Gemfile.lock index 48fdb342e55..aa786f37ca0 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -143,7 +143,7 @@ GEM bundler (>= 1.2.0, < 3) thor (~> 0.18) byebug (9.1.0) - capybara (3.22.0) + capybara (3.33.0) addressable mini_mime (>= 0.1.3) nokogiri (~> 1.8) @@ -1219,7 +1219,7 @@ DEPENDENCIES browser (~> 4.2) bullet (~> 6.0.2) bundler-audit (~> 0.6.1) - capybara (~> 3.22.0) + capybara (~> 3.33.0) capybara-screenshot (~> 1.0.22) carrierwave (~> 1.3) charlock_holmes (~> 0.7.5) diff --git a/app/assets/javascripts/blob/components/blob_edit_header.vue b/app/assets/javascripts/blob/components/blob_edit_header.vue index 5d3a1f0ccdb..2cbbbddceeb 100644 --- a/app/assets/javascripts/blob/components/blob_edit_header.vue +++ b/app/assets/javascripts/blob/components/blob_edit_header.vue @@ -15,7 +15,7 @@ export default { canDelete: { type: Boolean, required: false, - default: false, + default: true, }, showDelete: { type: Boolean, diff --git a/app/assets/javascripts/monitoring/components/dashboard_actions_menu.vue b/app/assets/javascripts/monitoring/components/dashboard_actions_menu.vue index 943cee9b504..68afa2ace01 100644 --- a/app/assets/javascripts/monitoring/components/dashboard_actions_menu.vue +++ b/app/assets/javascripts/monitoring/components/dashboard_actions_menu.vue @@ -138,6 +138,11 @@ export default { </script> <template> + <!-- + This component should be replaced with a variant developed + as part of https://gitlab.com/gitlab-org/gitlab-ui/-/issues/936 + The variant will create a dropdown with an icon, no text and no caret + --> <gl-new-dropdown v-gl-tooltip data-testid="actions-menu" diff --git a/app/assets/javascripts/monitoring/components/dashboard_panel.vue b/app/assets/javascripts/monitoring/components/dashboard_panel.vue index 9a0ac0f48ef..278858d3a94 100644 --- a/app/assets/javascripts/monitoring/components/dashboard_panel.vue +++ b/app/assets/javascripts/monitoring/components/dashboard_panel.vue @@ -394,15 +394,21 @@ export default { data-qa-selector="prometheus_graph_widgets" > <div data-testid="dropdown-wrapper" class="d-flex align-items-center"> + <!-- + This component should be replaced with a variant developed + as part of https://gitlab.com/gitlab-org/gitlab-ui/-/issues/936 + The variant will create a dropdown with an icon, no text and no caret + --> <gl-dropdown v-gl-tooltip - toggle-class="shadow-none border-0" + toggle-class="gl-px-3!" + no-caret data-qa-selector="prometheus_widgets_dropdown" right :title="__('More actions')" > - <template slot="button-content"> - <gl-icon name="ellipsis_v" class="dropdown-icon text-secondary" /> + <template #button-content> + <gl-icon class="gl-mr-0!" name="ellipsis_v" /> </template> <gl-dropdown-item v-if="expandBtnAvailable" diff --git a/app/assets/javascripts/pipelines/components/dag/dag.vue b/app/assets/javascripts/pipelines/components/dag/dag.vue index 10a9703a4c7..8487da3d621 100644 --- a/app/assets/javascripts/pipelines/components/dag/dag.vue +++ b/app/assets/javascripts/pipelines/components/dag/dag.vue @@ -1,8 +1,9 @@ <script> import { GlAlert, GlButton, GlEmptyState, GlSprintf } from '@gitlab/ui'; import { isEmpty } from 'lodash'; -import axios from '~/lib/utils/axios_utils'; import { __ } from '~/locale'; +import { fetchPolicies } from '~/lib/graphql'; +import getDagVisData from '../../graphql/queries/get_dag_vis_data.query.graphql'; import DagGraph from './dag_graph.vue'; import DagAnnotations from './dag_annotations.vue'; import { @@ -27,23 +28,58 @@ export default { GlEmptyState, GlButton, }, - props: { - graphUrl: { - type: String, - required: false, - default: '', + inject: { + dagDocPath: { + default: null, }, emptySvgPath: { - type: String, - required: true, default: '', }, - dagDocPath: { - type: String, - required: true, + pipelineIid: { + default: '', + }, + pipelineProjectPath: { default: '', }, }, + apollo: { + graphData: { + fetchPolicy: fetchPolicies.CACHE_AND_NETWORK, + query: getDagVisData, + variables() { + return { + projectPath: this.pipelineProjectPath, + iid: this.pipelineIid, + }; + }, + update(data) { + const { + stages: { nodes: stages }, + } = data.project.pipeline; + + const unwrappedGroups = stages + .map(({ name, groups: { nodes: groups } }) => { + return groups.map(group => { + return { category: name, ...group }; + }); + }) + .flat(2); + + const nodes = unwrappedGroups.map(group => { + const jobs = group.jobs.nodes.map(({ name, needs }) => { + return { name, needs: needs.nodes.map(need => need.name) }; + }); + + return { ...group, jobs }; + }); + + return nodes; + }, + error() { + this.reportFailure(LOAD_FAILURE); + }, + }, + }, data() { return { annotationsMap: {}, @@ -90,32 +126,20 @@ export default { default: return { text: this.$options.errorTexts[DEFAULT], - vatiant: 'danger', + variant: 'danger', }; } }, + processedData() { + return this.processGraphData(this.graphData); + }, shouldDisplayAnnotations() { return !isEmpty(this.annotationsMap); }, shouldDisplayGraph() { - return Boolean(!this.showFailureAlert && this.graphData); + return Boolean(!this.showFailureAlert && !this.hasNoDependentJobs && this.graphData); }, }, - mounted() { - const { processGraphData, reportFailure } = this; - - if (!this.graphUrl) { - reportFailure(); - return; - } - - axios - .get(this.graphUrl) - .then(response => { - processGraphData(response.data); - }) - .catch(() => reportFailure(LOAD_FAILURE)); - }, methods: { addAnnotationToMap({ uid, source, target }) { this.$set(this.annotationsMap, uid, { source, target }); @@ -124,25 +148,25 @@ export default { let parsed; try { - parsed = parseData(data.stages); + parsed = parseData(data); } catch { this.reportFailure(PARSE_FAILURE); - return; + return {}; } if (parsed.links.length === 1) { this.reportFailure(UNSUPPORTED_DATA); - return; + return {}; } // If there are no links, we don't report failure // as it simply means the user does not use job dependencies if (parsed.links.length === 0) { this.hasNoDependentJobs = true; - return; + return {}; } - this.graphData = parsed; + return parsed; }, hideAlert() { this.showFailureAlert = false; @@ -182,7 +206,7 @@ export default { <dag-annotations v-if="shouldDisplayAnnotations" :annotations="annotationsMap" /> <dag-graph v-if="shouldDisplayGraph" - :graph-data="graphData" + :graph-data="processedData" @onFailure="reportFailure" @update-annotation="updateAnnotation" /> @@ -209,7 +233,7 @@ export default { </p> </div> </template> - <template #actions> + <template v-if="dagDocPath" #actions> <gl-button :href="dagDocPath" target="__blank" variant="success"> {{ $options.emptyStateTexts.button }} </gl-button> diff --git a/app/assets/javascripts/pipelines/components/dag/parsing_utils.js b/app/assets/javascripts/pipelines/components/dag/parsing_utils.js index 3234f80ee91..1ed415688f2 100644 --- a/app/assets/javascripts/pipelines/components/dag/parsing_utils.js +++ b/app/assets/javascripts/pipelines/components/dag/parsing_utils.js @@ -5,14 +5,16 @@ import { uniqWith, isEqual } from 'lodash'; received from the endpoint into the format the d3 graph expects. Input is of the form: - [stages] - stages: {name, groups} - groups: [{ name, size, jobs }] - name is a group name; in the case that the group has one job, it is - also the job name - size is the number of parallel jobs - jobs: [{ name, needs}] - job name is either the same as the group name or group x/y + [nodes] + nodes: [{category, name, jobs, size}] + category is the stage name + name is a group name; in the case that the group has one job, it is + also the job name + size is the number of parallel jobs + jobs: [{ name, needs}] + job name is either the same as the group name or group x/y + needs: [job-names] + needs is an array of job-name strings Output is of the form: { nodes: [node], links: [link] } @@ -20,30 +22,17 @@ import { uniqWith, isEqual } from 'lodash'; link: { source, target, value }, with source & target being node names and value being a constant - We create nodes, create links, and then dedupe the links, so that in the case where + We create nodes in the GraphQL update function, and then here we create the node dictionary, + then create links, and then dedupe the links, so that in the case where job 4 depends on job 1 and job 2, and job 2 depends on job 1, we show only a single link from job 1 to job 2 then another from job 2 to job 4. - CREATE NODES - stage.name -> node.category - stage.group.name -> node.name (this is the group name if there are parallel jobs) - stage.group.jobs -> node.jobs - stage.group.size -> node.size - CREATE LINKS - stages.groups.name -> target - stages.groups.needs.each -> source (source is the name of the group, not the parallel job) + nodes.name -> target + nodes.name.needs.each -> source (source is the name of the group, not the parallel job) 10 -> value (constant) */ -export const createNodes = data => { - return data.flatMap(({ groups, name }) => { - return groups.map(group => { - return { ...group, category: name }; - }); - }); -}; - export const createNodeDict = nodes => { return nodes.reduce((acc, node) => { const newNode = { @@ -62,13 +51,6 @@ export const createNodeDict = nodes => { }, {}); }; -export const createNodesStructure = data => { - const nodes = createNodes(data); - const nodeDict = createNodeDict(nodes); - - return { nodes, nodeDict }; -}; - export const makeLinksFromNodes = (nodes, nodeDict) => { const constantLinkValue = 10; // all links are the same weight return nodes @@ -126,8 +108,8 @@ export const filterByAncestors = (links, nodeDict) => return !allAncestors.includes(source); }); -export const parseData = data => { - const { nodes, nodeDict } = createNodesStructure(data); +export const parseData = nodes => { + const nodeDict = createNodeDict(nodes); const allLinks = makeLinksFromNodes(nodes, nodeDict); const filteredLinks = filterByAncestors(allLinks, nodeDict); const links = uniqWith(filteredLinks, isEqual); diff --git a/app/assets/javascripts/pipelines/graphql/queries/get_dag_vis_data.query.graphql b/app/assets/javascripts/pipelines/graphql/queries/get_dag_vis_data.query.graphql new file mode 100644 index 00000000000..c73b186739e --- /dev/null +++ b/app/assets/javascripts/pipelines/graphql/queries/get_dag_vis_data.query.graphql @@ -0,0 +1,27 @@ +query getDagVisData($projectPath: ID!, $iid: ID!) { + project(fullPath: $projectPath) { + pipeline(iid: $iid) { + stages { + nodes { + name + groups { + nodes { + name + size + jobs { + nodes { + name + needs { + nodes { + name + } + } + } + } + } + } + } + } + } + } +} diff --git a/app/assets/javascripts/pipelines/pipeline_details_bundle.js b/app/assets/javascripts/pipelines/pipeline_details_bundle.js index 127d24c5473..e154c3f2422 100644 --- a/app/assets/javascripts/pipelines/pipeline_details_bundle.js +++ b/app/assets/javascripts/pipelines/pipeline_details_bundle.js @@ -4,7 +4,7 @@ import Translate from '~/vue_shared/translate'; import { __ } from '~/locale'; import { setUrlFragment, redirectTo } from '~/lib/utils/url_utility'; import pipelineGraph from './components/graph/graph_component.vue'; -import Dag from './components/dag/dag.vue'; +import createDagApp from './pipeline_details_dag'; import GraphBundleMixin from './mixins/graph_pipeline_bundle_mixin'; import PipelinesMediator from './pipeline_details_mediator'; import pipelineHeader from './components/header_component.vue'; @@ -114,32 +114,6 @@ const createTestDetails = () => { }); }; -const createDagApp = () => { - if (!window.gon?.features?.dagPipelineTab) { - return; - } - - const el = document.querySelector('#js-pipeline-dag-vue'); - const { pipelineDataPath, emptySvgPath, dagDocPath } = el?.dataset; - - // eslint-disable-next-line no-new - new Vue({ - el, - components: { - Dag, - }, - render(createElement) { - return createElement('dag', { - props: { - graphUrl: pipelineDataPath, - emptySvgPath, - dagDocPath, - }, - }); - }, - }); -}; - export default () => { const { dataset } = document.querySelector('.js-pipeline-details-vue'); const mediator = new PipelinesMediator({ endpoint: dataset.endpoint }); diff --git a/app/assets/javascripts/pipelines/pipeline_details_dag.js b/app/assets/javascripts/pipelines/pipeline_details_dag.js new file mode 100644 index 00000000000..dc03b457265 --- /dev/null +++ b/app/assets/javascripts/pipelines/pipeline_details_dag.js @@ -0,0 +1,39 @@ +import Vue from 'vue'; +import VueApollo from 'vue-apollo'; +import createDefaultClient from '~/lib/graphql'; +import Dag from './components/dag/dag.vue'; + +Vue.use(VueApollo); + +const apolloProvider = new VueApollo({ + defaultClient: createDefaultClient(), +}); + +const createDagApp = () => { + if (!window.gon?.features?.dagPipelineTab) { + return; + } + + const el = document.querySelector('#js-pipeline-dag-vue'); + const { pipelineProjectPath, pipelineIid, emptySvgPath, dagDocPath } = el?.dataset; + + // eslint-disable-next-line no-new + new Vue({ + el, + components: { + Dag, + }, + apolloProvider, + provide: { + pipelineProjectPath, + pipelineIid, + emptySvgPath, + dagDocPath, + }, + render(createElement) { + return createElement('dag', {}); + }, + }); +}; + +export default createDagApp; diff --git a/app/assets/javascripts/snippets/components/edit.vue b/app/assets/javascripts/snippets/components/edit.vue index 81be39bda04..e1b57f35134 100644 --- a/app/assets/javascripts/snippets/components/edit.vue +++ b/app/assets/javascripts/snippets/components/edit.vue @@ -14,9 +14,6 @@ import { SNIPPET_VISIBILITY_PRIVATE, SNIPPET_CREATE_MUTATION_ERROR, SNIPPET_UPDATE_MUTATION_ERROR, - SNIPPET_BLOB_ACTION_CREATE, - SNIPPET_BLOB_ACTION_UPDATE, - SNIPPET_BLOB_ACTION_MOVE, } from '../constants'; import SnippetBlobActionsEdit from './snippet_blob_actions_edit.vue'; import SnippetVisibilityEdit from './snippet_visibility_edit.vue'; @@ -56,25 +53,20 @@ export default { }, data() { return { - blobsActions: {}, isUpdating: false, newSnippet: false, + actions: [], }; }, computed: { - getActionsEntries() { - return Object.values(this.blobsActions); + hasBlobChanges() { + return this.actions.length > 0; }, - allBlobsHaveContent() { - const entries = this.getActionsEntries; - return entries.length > 0 && !entries.find(action => !action.content); - }, - allBlobChangesRegistered() { - const entries = this.getActionsEntries; - return entries.length > 0 && !entries.find(action => action.action === ''); + hasValidBlobs() { + return this.actions.every(x => x.filePath && x.content); }, updatePrevented() { - return this.snippet.title === '' || !this.allBlobsHaveContent || this.isUpdating; + return this.snippet.title === '' || !this.hasValidBlobs || this.isUpdating; }, isProjectSnippet() { return Boolean(this.projectPath); @@ -85,7 +77,7 @@ export default { title: this.snippet.title, description: this.snippet.description, visibilityLevel: this.snippet.visibilityLevel, - blobActions: this.getActionsEntries.filter(entry => entry.action !== ''), + blobActions: this.actions, }; }, saveButtonLabel() { @@ -120,48 +112,11 @@ export default { onBeforeUnload(e = {}) { const returnValue = __('Are you sure you want to lose unsaved changes?'); - if (!this.allBlobChangesRegistered || this.isUpdating) return undefined; + if (!this.hasBlobChanges || this.isUpdating) return undefined; Object.assign(e, { returnValue }); return returnValue; }, - updateBlobActions(args = {}) { - // `_constants` is the internal prop that - // should not be sent to the mutation. Hence we filter it out from - // the argsToUpdateAction that is the data-basis for the mutation. - const { _constants: blobConstants, ...argsToUpdateAction } = args; - const { previousPath, filePath, content } = argsToUpdateAction; - let actionEntry = this.blobsActions[blobConstants.id] || {}; - let tunedActions = { - action: '', - previousPath, - }; - - if (this.newSnippet) { - // new snippet, hence new blob - tunedActions = { - action: SNIPPET_BLOB_ACTION_CREATE, - previousPath: '', - }; - } else if (previousPath && filePath) { - // renaming of a blob + renaming & content update - const renamedToOriginal = filePath === blobConstants.originalPath; - tunedActions = { - action: renamedToOriginal ? SNIPPET_BLOB_ACTION_UPDATE : SNIPPET_BLOB_ACTION_MOVE, - previousPath: !renamedToOriginal ? blobConstants.originalPath : '', - }; - } else if (content !== blobConstants.originalContent) { - // content update only - tunedActions = { - action: SNIPPET_BLOB_ACTION_UPDATE, - previousPath: '', - }; - } - - actionEntry = { ...actionEntry, ...argsToUpdateAction, ...tunedActions }; - - this.$set(this.blobsActions, blobConstants.id, actionEntry); - }, flashAPIFailure(err) { const defaultErrorMsg = this.newSnippet ? SNIPPET_CREATE_MUTATION_ERROR @@ -218,7 +173,6 @@ export default { if (errors.length) { this.flashAPIFailure(errors[0]); } else { - this.originalContent = this.content; redirectTo(baseObj.snippet.webUrl); } }) @@ -226,6 +180,9 @@ export default { this.flashAPIFailure(e); }); }, + updateActions(actions) { + this.actions = actions; + }, }, newSnippetSchema: { title: '', @@ -261,7 +218,7 @@ export default { :markdown-preview-path="markdownPreviewPath" :markdown-docs-path="markdownDocsPath" /> - <snippet-blob-actions-edit :blobs="blobs" @blob-updated="updateBlobActions" /> + <snippet-blob-actions-edit :init-blobs="blobs" @actions="updateActions" /> <snippet-visibility-edit v-model="snippet.visibilityLevel" diff --git a/app/assets/javascripts/snippets/components/snippet_blob_actions_edit.vue b/app/assets/javascripts/snippets/components/snippet_blob_actions_edit.vue index fd81a5fa69c..55cd13a6930 100644 --- a/app/assets/javascripts/snippets/components/snippet_blob_actions_edit.vue +++ b/app/assets/javascripts/snippets/components/snippet_blob_actions_edit.vue @@ -1,25 +1,156 @@ <script> +import { GlButton } from '@gitlab/ui'; +import { cloneDeep } from 'lodash'; +import { s__, sprintf } from '~/locale'; +import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import SnippetBlobEdit from './snippet_blob_edit.vue'; +import { SNIPPET_MAX_BLOBS } from '../constants'; +import { createBlob, decorateBlob, diffAll } from '../utils/blob'; export default { components: { SnippetBlobEdit, + GlButton, }, + mixins: [glFeatureFlagsMixin()], props: { - blobs: { + initBlobs: { type: Array, required: true, }, }, + data() { + return { + // This is a dictionary (by .id) of the original blobs and + // is used as the baseline for calculating diffs + // (e.g., what has been deleted, changed, renamed, etc.) + blobsOrig: {}, + // This is a dictionary (by .id) of the current blobs and + // is updated as the user makes changes. + blobs: {}, + // This is a list of blob ID's in order how they should be + // presented. + blobIds: [], + }; + }, + computed: { + actions() { + return diffAll(this.blobs, this.blobsOrig); + }, + count() { + return this.blobIds.length; + }, + addLabel() { + return sprintf(s__('Snippets|Add another file %{num}/%{total}'), { + num: this.count, + total: SNIPPET_MAX_BLOBS, + }); + }, + canDelete() { + return this.count > 1; + }, + canAdd() { + return this.count < SNIPPET_MAX_BLOBS; + }, + hasMultiFilesEnabled() { + return this.glFeatures.snippetMultipleFiles; + }, + filesLabel() { + return this.hasMultiFilesEnabled ? s__('Snippets|Files') : s__('Snippets|File'); + }, + firstInputId() { + const blobId = this.blobIds[0]; + + if (!blobId) { + return ''; + } + + return `${blobId}_file_path`; + }, + }, + watch: { + actions: { + immediate: true, + handler(val) { + this.$emit('actions', val); + }, + }, + }, + created() { + const blobs = this.initBlobs.map(decorateBlob); + const blobsById = blobs.reduce((acc, x) => Object.assign(acc, { [x.id]: x }), {}); + + this.blobsOrig = blobsById; + this.blobs = cloneDeep(blobsById); + this.blobIds = blobs.map(x => x.id); + + // Show 1 empty blob if none exist + if (!this.blobIds.length) { + this.addBlob(); + } + }, + methods: { + updateBlobContent(id, content) { + const origBlob = this.blobsOrig[id]; + const blob = this.blobs[id]; + + blob.content = content; + + // If we've received content, but we haven't loaded the content before + // then this is also the original content. + if (origBlob && !origBlob.isLoaded) { + blob.isLoaded = true; + origBlob.isLoaded = true; + origBlob.content = content; + } + }, + updateBlobFilePath(id, path) { + const blob = this.blobs[id]; + + blob.path = path; + }, + addBlob() { + const blob = createBlob(); + + this.$set(this.blobs, blob.id, blob); + this.blobIds.push(blob.id); + }, + deleteBlob(id) { + this.blobIds = this.blobIds.filter(x => x !== id); + this.$delete(this.blobs, id); + }, + updateBlob(id, args) { + if ('content' in args) { + this.updateBlobContent(id, args.content); + } + if ('path' in args) { + this.updateBlobFilePath(id, args.path); + } + }, + }, }; </script> - <template> <div class="form-group file-editor"> - <label for="snippet_file_path">{{ s__('Snippets|File') }}</label> - <template v-if="blobs.length"> - <snippet-blob-edit v-for="blob in blobs" :key="blob.name" :blob="blob" v-on="$listeners" /> - </template> - <snippet-blob-edit v-else v-on="$listeners" /> + <label :for="firstInputId">{{ filesLabel }}</label> + <snippet-blob-edit + v-for="(blobId, index) in blobIds" + :key="blobId" + :class="{ 'gl-mt-3': index > 0 }" + :blob="blobs[blobId]" + :can-delete="canDelete" + :show-delete="hasMultiFilesEnabled" + @blob-updated="updateBlob(blobId, $event)" + @delete="deleteBlob(blobId)" + /> + <gl-button + v-if="hasMultiFilesEnabled" + :disabled="!canAdd" + data-testid="add_button" + class="gl-my-3" + variant="dashed" + @click="addBlob" + >{{ addLabel }}</gl-button + > </div> </template> diff --git a/app/assets/javascripts/snippets/components/snippet_blob_edit.vue b/app/assets/javascripts/snippets/components/snippet_blob_edit.vue index b349069d73a..5066b7f3ed6 100644 --- a/app/assets/javascripts/snippets/components/snippet_blob_edit.vue +++ b/app/assets/javascripts/snippets/components/snippet_blob_edit.vue @@ -8,12 +8,6 @@ import { SNIPPET_BLOB_CONTENT_FETCH_ERROR } from '~/snippets/constants'; import Flash from '~/flash'; import { sprintf } from '~/locale'; -function localId() { - return Math.floor((1 + Math.random()) * 0x10000) - .toString(16) - .substring(1); -} - export default { components: { BlobHeaderEdit, @@ -24,49 +18,35 @@ export default { props: { blob: { type: Object, + required: true, + }, + canDelete: { + type: Boolean, required: false, - default: null, - validator: ({ rawPath }) => Boolean(rawPath), + default: true, }, - }, - data() { - return { - id: localId(), - filePath: this.blob?.path || '', - previousPath: '', - originalPath: this.blob?.path || '', - content: this.blob?.content || '', - originalContent: '', - isContentLoading: this.blob, - }; - }, - watch: { - filePath(filePath, previousPath) { - this.previousPath = previousPath; - this.notifyAboutUpdates({ previousPath }); + showDelete: { + type: Boolean, + required: false, + default: false, }, - content() { - this.notifyAboutUpdates(); + }, + computed: { + inputId() { + return `${this.blob.id}_file_path`; }, }, mounted() { - if (this.blob) { + if (!this.blob.isLoaded) { this.fetchBlobContent(); } }, methods: { + onDelete() { + this.$emit('delete'); + }, notifyAboutUpdates(args = {}) { - const { filePath, previousPath } = args; - this.$emit('blob-updated', { - filePath: filePath || this.filePath, - previousPath: previousPath || this.previousPath, - content: this.content, - _constants: { - originalPath: this.originalPath, - originalContent: this.originalContent, - id: this.id, - }, - }); + this.$emit('blob-updated', args); }, fetchBlobContent() { const baseUrl = getBaseURL(); @@ -75,17 +55,12 @@ export default { axios .get(url) .then(res => { - this.originalContent = res.data; - this.content = res.data; + this.notifyAboutUpdates({ content: res.data }); }) - .catch(e => this.flashAPIFailure(e)) - .finally(() => { - this.isContentLoading = false; - }); + .catch(e => this.flashAPIFailure(e)); }, flashAPIFailure(err) { Flash(sprintf(SNIPPET_BLOB_CONTENT_FETCH_ERROR, { err })); - this.isContentLoading = false; }, }, }; @@ -93,16 +68,26 @@ export default { <template> <div class="file-holder snippet"> <blob-header-edit - id="snippet_file_path" - v-model="filePath" + :id="inputId" + :value="blob.path" data-qa-selector="file_name_field" + :can-delete="canDelete" + :show-delete="showDelete" + @input="notifyAboutUpdates({ path: $event })" + @delete="onDelete" /> <gl-loading-icon - v-if="isContentLoading" + v-if="!blob.isLoaded" :label="__('Loading snippet')" size="lg" class="loading-animation prepend-top-20 append-bottom-20" /> - <blob-content-edit v-else v-model="content" :file-global-id="id" :file-name="filePath" /> + <blob-content-edit + v-else + :value="blob.content" + :file-global-id="blob.id" + :file-name="blob.path" + @input="notifyAboutUpdates({ content: $event })" + /> </div> </template> diff --git a/app/assets/javascripts/snippets/constants.js b/app/assets/javascripts/snippets/constants.js index a59d7aa84eb..12b83525bf7 100644 --- a/app/assets/javascripts/snippets/constants.js +++ b/app/assets/javascripts/snippets/constants.js @@ -31,3 +31,5 @@ export const SNIPPET_BLOB_ACTION_CREATE = 'create'; export const SNIPPET_BLOB_ACTION_UPDATE = 'update'; export const SNIPPET_BLOB_ACTION_MOVE = 'move'; export const SNIPPET_BLOB_ACTION_DELETE = 'delete'; + +export const SNIPPET_MAX_BLOBS = 10; diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/work_in_progress.vue b/app/assets/javascripts/vue_merge_request_widget/components/states/work_in_progress.vue index 6837545c681..0db90f281cd 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/work_in_progress.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/work_in_progress.vue @@ -3,6 +3,11 @@ import $ from 'jquery'; import { GlButton } from '@gitlab/ui'; import { __ } from '~/locale'; import createFlash from '~/flash'; +import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; +import mergeRequestQueryVariablesMixin from '../../mixins/merge_request_query_variables'; +import getStateQuery from '../../queries/get_state.query.graphql'; +import workInProgressQuery from '../../queries/states/work_in_progress.query.graphql'; +import removeWipMutation from '../../queries/toggle_wip.mutation.graphql'; import StatusIcon from '../mr_widget_status_icon.vue'; import tooltip from '../../../vue_shared/directives/tooltip'; import eventHub from '../../event_hub'; @@ -16,38 +21,127 @@ export default { directives: { tooltip, }, + mixins: [glFeatureFlagMixin(), mergeRequestQueryVariablesMixin], + apollo: { + userPermissions: { + query: workInProgressQuery, + skip() { + return !this.glFeatures.mergeRequestWidgetGraphql; + }, + variables() { + return this.mergeRequestQueryVariables; + }, + update: data => data.project.mergeRequest.userPermissions, + }, + }, props: { mr: { type: Object, required: true }, service: { type: Object, required: true }, }, data() { return { + userPermissions: {}, isMakingRequest: false, }; }, + computed: { + canUpdate() { + if (this.glFeatures.mergeRequestWidgetGraphql) { + return this.userPermissions.updateMergeRequest; + } + + return Boolean(this.mr.removeWIPPath); + }, + }, methods: { - handleRemoveWIP() { + removeWipMutation() { this.isMakingRequest = true; - this.service - .removeWIP() - .then(res => res.data) - .then(data => { - eventHub.$emit('UpdateWidgetData', data); + + this.$apollo + .mutate({ + mutation: removeWipMutation, + variables: { + ...this.mergeRequestQueryVariables, + wip: false, + }, + update( + store, + { + data: { + mergeRequestSetWip: { + errors, + mergeRequest: { workInProgress, title }, + }, + }, + }, + ) { + if (errors?.length) { + createFlash(__('Something went wrong. Please try again.')); + + return; + } + + const data = store.readQuery({ + query: getStateQuery, + variables: this.mergeRequestQueryVariables, + }); + data.project.mergeRequest.workInProgress = workInProgress; + data.project.mergeRequest.title = title; + store.writeQuery({ + query: getStateQuery, + data, + variables: this.mergeRequestQueryVariables, + }); + }, + optimisticResponse: { + // eslint-disable-next-line @gitlab/require-i18n-strings + __typename: 'Mutation', + mergeRequestSetWip: { + __typename: 'MergeRequestSetWipPayload', + errors: [], + mergeRequest: { + __typename: 'MergeRequest', + title: this.mr.title, + workInProgress: false, + }, + }, + }, + }) + .then(({ data: { mergeRequestSetWip: { mergeRequest: { title } } } }) => { createFlash(__('The merge request can now be merged.'), 'notice'); - $('.merge-request .detail-page-description .title').text(this.mr.title); + $('.merge-request .detail-page-description .title').text(title); }) - .catch(() => { + .catch(() => createFlash(__('Something went wrong. Please try again.'))) + .finally(() => { this.isMakingRequest = false; - createFlash(__('Something went wrong. Please try again.')); }); }, + handleRemoveWIP() { + if (this.glFeatures.mergeRequestWidgetGraphql) { + this.removeWipMutation(); + } else { + this.isMakingRequest = true; + this.service + .removeWIP() + .then(res => res.data) + .then(data => { + eventHub.$emit('UpdateWidgetData', data); + createFlash(__('The merge request can now be merged.'), 'notice'); + $('.merge-request .detail-page-description .title').text(this.mr.title); + }) + .catch(() => { + this.isMakingRequest = false; + createFlash(__('Something went wrong. Please try again.')); + }); + } + }, }, }; </script> <template> <div class="mr-widget-body media"> - <status-icon :show-disabled-button="Boolean(mr.removeWIPPath)" status="warning" /> + <status-icon :show-disabled-button="canUpdate" status="warning" /> <div class="media-body"> <div class="gl-ml-3 float-left"> <span class="gl-font-weight-bold"> @@ -58,7 +152,7 @@ export default { }}</span> </div> <gl-button - v-if="mr.removeWIPPath" + v-if="canUpdate" size="small" :disabled="isMakingRequest" :loading="isMakingRequest" diff --git a/app/assets/javascripts/vue_merge_request_widget/mixins/merge_request_query_variables.js b/app/assets/javascripts/vue_merge_request_widget/mixins/merge_request_query_variables.js new file mode 100644 index 00000000000..3a121908f36 --- /dev/null +++ b/app/assets/javascripts/vue_merge_request_widget/mixins/merge_request_query_variables.js @@ -0,0 +1,10 @@ +export default { + computed: { + mergeRequestQueryVariables() { + return { + projectPath: this.mr.targetProjectFullPath, + iid: `${this.mr.iid}`, + }; + }, + }, +}; diff --git a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue index 34c7e1568db..cb8d9bd7791 100644 --- a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue +++ b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue @@ -8,6 +8,7 @@ import { sprintf, s__, __ } from '~/locale'; import Project from '~/pages/projects/project'; import SmartInterval from '~/smart_interval'; import createFlash from '../flash'; +import mergeRequestQueryVariablesMixin from './mixins/merge_request_query_variables'; import Loading from './components/loading.vue'; import WidgetHeader from './components/mr_widget_header.vue'; import WidgetSuggestPipeline from './components/mr_widget_suggest_pipeline.vue'; @@ -42,6 +43,7 @@ import GroupedCodequalityReportsApp from '../reports/codequality_report/grouped_ import GroupedTestReportsApp from '../reports/components/grouped_test_reports_app.vue'; import { setFaviconOverlay } from '../lib/utils/common_utils'; import GroupedAccessibilityReportsApp from '../reports/accessibility_report/grouped_accessibility_reports_app.vue'; +import getStateQuery from './queries/get_state.query.graphql'; export default { el: '#js-vue-mr-widget', @@ -83,6 +85,27 @@ export default { GroupedAccessibilityReportsApp, MrWidgetApprovals, }, + apollo: { + state: { + query: getStateQuery, + manual: true, + pollInterval: 10 * 1000, + skip() { + return !this.mr || !window.gon?.features?.mergeRequestWidgetGraphql; + }, + variables() { + return this.mergeRequestQueryVariables; + }, + result({ + data: { + project: { mergeRequest }, + }, + }) { + this.mr.setGraphqlData(mergeRequest); + }, + }, + }, + mixins: [mergeRequestQueryVariablesMixin], props: { mrData: { type: Object, diff --git a/app/assets/javascripts/vue_merge_request_widget/queries/get_state.query.graphql b/app/assets/javascripts/vue_merge_request_widget/queries/get_state.query.graphql new file mode 100644 index 00000000000..488397e7735 --- /dev/null +++ b/app/assets/javascripts/vue_merge_request_widget/queries/get_state.query.graphql @@ -0,0 +1,8 @@ +query getState($projectPath: ID!, $iid: String!) { + project(fullPath: $projectPath) { + mergeRequest(iid: $iid) { + title + workInProgress + } + } +} diff --git a/app/assets/javascripts/vue_merge_request_widget/queries/states/work_in_progress.query.graphql b/app/assets/javascripts/vue_merge_request_widget/queries/states/work_in_progress.query.graphql new file mode 100644 index 00000000000..73e205ebf2b --- /dev/null +++ b/app/assets/javascripts/vue_merge_request_widget/queries/states/work_in_progress.query.graphql @@ -0,0 +1,9 @@ +query workInProgressQuery($projectPath: ID!, $iid: String!) { + project(fullPath: $projectPath) { + mergeRequest(iid: $iid) { + userPermissions { + updateMergeRequest + } + } + } +} diff --git a/app/assets/javascripts/vue_merge_request_widget/queries/toggle_wip.mutation.graphql b/app/assets/javascripts/vue_merge_request_widget/queries/toggle_wip.mutation.graphql new file mode 100644 index 00000000000..37abe5ddf3c --- /dev/null +++ b/app/assets/javascripts/vue_merge_request_widget/queries/toggle_wip.mutation.graphql @@ -0,0 +1,9 @@ +mutation toggleWIPStatus($projectPath: ID!, $iid: String!, $wip: Boolean!) { + mergeRequestSetWip(input: { projectPath: $projectPath, iid: $iid, wip: $wip }) { + mergeRequest { + title + workInProgress + } + errors + } +} diff --git a/app/assets/javascripts/vue_merge_request_widget/stores/get_state_key.js b/app/assets/javascripts/vue_merge_request_widget/stores/get_state_key.js index 44e8167d6a3..3bd512c89bf 100644 --- a/app/assets/javascripts/vue_merge_request_widget/stores/get_state_key.js +++ b/app/assets/javascripts/vue_merge_request_widget/stores/get_state_key.js @@ -1,21 +1,21 @@ import { stateKey } from './state_maps'; -export default function deviseState(data) { - if (data.project_archived) { +export default function deviseState() { + if (this.projectArchived) { return stateKey.archived; - } else if (data.branch_missing) { + } else if (this.branchMissing) { return stateKey.missingBranch; - } else if (!data.commits_count) { + } else if (!this.commitsCount) { return stateKey.nothingToMerge; } else if (this.mergeStatus === 'unchecked' || this.mergeStatus === 'checking') { return stateKey.checking; - } else if (data.has_conflicts) { + } else if (this.hasConflicts) { return stateKey.conflicts; } else if (this.shouldBeRebased) { return stateKey.rebase; } else if (this.onlyAllowMergeIfPipelineSucceeds && this.isPipelineFailed) { return stateKey.pipelineFailed; - } else if (data.work_in_progress) { + } else if (this.workInProgress) { return stateKey.workInProgress; } else if (this.hasMergeableDiscussionsState) { return stateKey.unresolvedDiscussions; diff --git a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js index 28b41b64ff6..8c98ba1b023 100644 --- a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js +++ b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js @@ -60,6 +60,9 @@ export default class MergeRequestStore { this.rebaseInProgress = data.rebase_in_progress; this.mergeRequestDiffsPath = data.diffs_path; this.approvalsWidgetType = data.approvals_widget_type; + this.projectArchived = data.project_archived; + this.branchMissing = data.branch_missing; + this.hasConflicts = data.has_conflicts; if (data.issues_links) { const links = data.issues_links; @@ -90,7 +93,8 @@ export default class MergeRequestStore { this.ffOnlyEnabled = data.ff_only_enabled; this.shouldBeRebased = Boolean(data.should_be_rebased); this.isRemovingSourceBranch = this.isRemovingSourceBranch || false; - this.isOpen = data.state === 'opened'; + this.mergeRequestState = data.state; + this.isOpen = this.mergeRequestState === 'opened'; this.hasMergeableDiscussionsState = data.mergeable_discussions_state === false; this.isSHAMismatch = this.sha !== data.diff_head_sha; this.latestSHA = data.diff_head_sha; @@ -133,6 +137,10 @@ export default class MergeRequestStore { this.mergeCommitPath = data.merge_commit_path; this.canPushToSourceBranch = data.can_push_to_source_branch; + if (data.work_in_progress !== undefined) { + this.workInProgress = data.work_in_progress; + } + const currentUser = data.current_user; this.cherryPickInForkPath = currentUser.cherry_pick_in_fork_path; @@ -143,19 +151,25 @@ export default class MergeRequestStore { this.canCherryPickInCurrentMR = currentUser.can_cherry_pick_on_current_merge_request || false; this.canRevertInCurrentMR = currentUser.can_revert_on_current_merge_request || false; - this.setState(data); + this.setState(); + } + + setGraphqlData(data) { + this.workInProgress = data.workInProgress; + + this.setState(); } - setState(data) { + setState() { if (this.mergeOngoing) { this.state = 'merging'; return; } if (this.isOpen) { - this.state = getStateKey.call(this, data); + this.state = getStateKey.call(this); } else { - switch (data.state) { + switch (this.mergeRequestState) { case 'merged': this.state = 'merged'; break; diff --git a/app/assets/javascripts/vue_shared/components/gl_mentions.vue b/app/assets/javascripts/vue_shared/components/gl_mentions.vue index 5d97d364e94..00bc46257ed 100644 --- a/app/assets/javascripts/vue_shared/components/gl_mentions.vue +++ b/app/assets/javascripts/vue_shared/components/gl_mentions.vue @@ -6,6 +6,7 @@ import { spriteIcon } from '~/lib/utils/common_utils'; import SidebarMediator from '~/sidebar/sidebar_mediator'; const AutoComplete = { + Issues: 'issues', Labels: 'labels', Members: 'members', }; @@ -17,6 +18,14 @@ function doesCurrentLineStartWith(searchString, fullText, selectionStart) { } const autoCompleteMap = { + [AutoComplete.Issues]: { + filterValues() { + return this[AutoComplete.Issues]; + }, + menuItemTemplate({ original }) { + return `<small>${original.reference || original.iid}</small> ${escape(original.title)}`; + }, + }, [AutoComplete.Labels]: { filterValues() { const fullText = this.$slots.default?.[0]?.elm?.value; @@ -107,6 +116,13 @@ export default { this.tribute = new Tribute({ collection: [ { + trigger: '#', + lookup: value => value.iid + value.title, + menuItemTemplate: autoCompleteMap[AutoComplete.Issues].menuItemTemplate, + selectTemplate: ({ original }) => original.reference || `#${original.iid}`, + values: this.getValues(AutoComplete.Issues), + }, + { trigger: '@', fillAttr: 'username', lookup: value => value.name + value.username, diff --git a/app/assets/javascripts/vue_shared/components/markdown/field.vue b/app/assets/javascripts/vue_shared/components/markdown/field.vue index c354228ce74..8a32de0a2e4 100644 --- a/app/assets/javascripts/vue_shared/components/markdown/field.vue +++ b/app/assets/javascripts/vue_shared/components/markdown/field.vue @@ -167,7 +167,7 @@ export default { return new GLForm($(this.$refs['gl-form']), { emojis: this.enableAutocomplete, members: this.enableAutocomplete && !this.glFeatures.tributeAutocomplete, - issues: this.enableAutocomplete, + issues: this.enableAutocomplete && !this.glFeatures.tributeAutocomplete, mergeRequests: this.enableAutocomplete, epics: this.enableAutocomplete, milestones: this.enableAutocomplete, diff --git a/app/assets/stylesheets/framework/filters.scss b/app/assets/stylesheets/framework/filters.scss index 2e87724a9e7..ed4281123cd 100644 --- a/app/assets/stylesheets/framework/filters.scss +++ b/app/assets/stylesheets/framework/filters.scss @@ -307,23 +307,6 @@ color: $gl-text-color; border-color: $border-color; } - - svg { - height: 14px; - width: 14px; - vertical-align: middle; - margin-bottom: 4px; - } - - .dropdown-toggle-text { - display: inline-block; - color: inherit; - - .fa { - vertical-align: middle; - color: inherit; - } - } } .filtered-search-history-dropdown { diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index b217b366ee5..e77d2f0f5ee 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -39,6 +39,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo push_frontend_feature_flag(:auto_expand_collapsed_diffs, @project, default_enabled: true) push_frontend_feature_flag(:approvals_commented_by, @project, default_enabled: true) push_frontend_feature_flag(:hide_jump_to_next_unresolved_in_threads, default_enabled: true) + push_frontend_feature_flag(:merge_request_widget_graphql, @project) end before_action do diff --git a/app/controllers/projects/snippets_controller.rb b/app/controllers/projects/snippets_controller.rb index 49840e847f2..632e8db9796 100644 --- a/app/controllers/projects/snippets_controller.rb +++ b/app/controllers/projects/snippets_controller.rb @@ -14,6 +14,10 @@ class Projects::SnippetsController < Projects::Snippets::ApplicationController before_action :authorize_update_snippet!, only: [:edit, :update] before_action :authorize_admin_snippet!, only: [:destroy] + before_action do + push_frontend_feature_flag(:snippet_multiple_files, current_user) + end + def index @snippet_counts = ::Snippets::CountService .new(current_user, project: @project) diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 10a23c0539e..ba21fbddde1 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -44,6 +44,10 @@ class ProjectsController < Projects::ApplicationController push_frontend_feature_flag(:service_desk_custom_address, @project) end + before_action only: [:edit] do + push_frontend_feature_flag(:approval_suggestions, @project) + end + layout :determine_layout def index diff --git a/app/controllers/snippets_controller.rb b/app/controllers/snippets_controller.rb index e68b821459d..486c7f1d028 100644 --- a/app/controllers/snippets_controller.rb +++ b/app/controllers/snippets_controller.rb @@ -17,6 +17,10 @@ class SnippetsController < Snippets::ApplicationController layout 'snippets' + before_action do + push_frontend_feature_flag(:snippet_multiple_files, current_user) + end + def index if params[:username].present? @user = UserFinder.new(params[:username]).find_by_username! diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index fdc55063433..b70340a98cd 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -51,14 +51,16 @@ class MergeRequestDiff < ApplicationRecord scope :by_commit_sha, ->(sha) do joins(:merge_request_diff_commits).where(merge_request_diff_commits: { sha: sha }).reorder(nil) end - scope :has_diff_files, -> { where(id: MergeRequestDiffFile.select(:merge_request_diff_id)) } scope :by_project_id, -> (project_id) do joins(:merge_request).where(merge_requests: { target_project_id: project_id }) end scope :recent, -> { order(id: :desc).limit(100) } - scope :files_in_database, -> { where(stored_externally: [false, nil]) } + + scope :files_in_database, -> do + where(stored_externally: [false, nil]).where(arel_table[:files_count].gt(0)) + end scope :not_latest_diffs, -> do merge_requests = MergeRequest.arel_table @@ -115,29 +117,28 @@ class MergeRequestDiff < ApplicationRecord end def ids_for_external_storage_migration_strategy_always(limit:) - has_diff_files.files_in_database.limit(limit).pluck(:id) + files_in_database.limit(limit).pluck(:id) end def ids_for_external_storage_migration_strategy_outdated(limit:) # Outdated is too complex to be a single SQL query, so split into three before = EXTERNAL_DIFF_CUTOFF.ago - potentials = has_diff_files.files_in_database - ids = potentials + ids = files_in_database .old_merged_diffs(before) .limit(limit) .pluck(:id) return ids if ids.size >= limit - ids += potentials + ids += files_in_database .old_closed_diffs(before) .limit(limit - ids.size) .pluck(:id) return ids if ids.size >= limit - ids + potentials + ids + files_in_database .not_latest_diffs .limit(limit - ids.size) .pluck(:id) diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb index 42545cffc61..3cc1be9dfb7 100644 --- a/app/policies/group_policy.rb +++ b/app/policies/group_policy.rb @@ -167,6 +167,7 @@ class GroupPolicy < BasePolicy def access_level return GroupMember::NO_ACCESS if @user.nil? + return GroupMember::NO_ACCESS unless user_is_user? @access_level ||= lookup_access_level! end @@ -174,6 +175,12 @@ class GroupPolicy < BasePolicy def lookup_access_level! @subject.max_member_access_for_user(@user) end + + private + + def user_is_user? + user.is_a?(User) + end end GroupPolicy.prepend_if_ee('EE::GroupPolicy') diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index 21a73185df4..081cc018b5b 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -589,8 +589,13 @@ class ProjectPolicy < BasePolicy private + def user_is_user? + user.is_a?(User) + end + def team_member? return false if @user.nil? + return false unless user_is_user? greedy_load_subject = false @@ -618,6 +623,7 @@ class ProjectPolicy < BasePolicy # rubocop: disable CodeReuse/ActiveRecord def project_group_member? return false if @user.nil? + return false unless user_is_user? project.group && ( @@ -629,6 +635,7 @@ class ProjectPolicy < BasePolicy def team_access_level return -1 if @user.nil? + return -1 unless user_is_user? lookup_access_level! end diff --git a/app/views/projects/pipelines/_with_tabs.html.haml b/app/views/projects/pipelines/_with_tabs.html.haml index 9199fdb99d6..4ae06e1e16f 100644 --- a/app/views/projects/pipelines/_with_tabs.html.haml +++ b/app/views/projects/pipelines/_with_tabs.html.haml @@ -81,7 +81,7 @@ - if dag_pipeline_tab_enabled #js-tab-dag.tab-pane - #js-pipeline-dag-vue{ data: { pipeline_data_path: dag_project_pipeline_path(@project, @pipeline), empty_svg_path: image_path('illustrations/empty-state/empty-dag-md.svg'), dag_doc_path: help_page_path('ci/yaml/README.md', anchor: 'needs')} } + #js-pipeline-dag-vue{ data: { pipeline_project_path: @project.full_path, pipeline_iid: @pipeline.iid, empty_svg_path: image_path('illustrations/empty-state/empty-dag-md.svg'), dag_doc_path: help_page_path('ci/yaml/README.md', anchor: 'needs')} } #js-tab-tests.tab-pane #js-pipeline-tests-detail{ data: { summary_endpoint: summary_project_pipeline_tests_path(@project, @pipeline, format: :json), diff --git a/app/views/shared/issuable/_search_bar.html.haml b/app/views/shared/issuable/_search_bar.html.haml index ea3ca278426..3f3b9146e71 100644 --- a/app/views/shared/issuable/_search_bar.html.haml +++ b/app/views/shared/issuable/_search_bar.html.haml @@ -20,7 +20,8 @@ .issues-other-filters.filtered-search-wrapper.d-flex.flex-column.flex-md-row .filtered-search-box - if type != :boards_modal && type != :boards - = dropdown_tag(_('Recent searches'), + - text = tag.span(sprite_icon('history'), class: "d-md-none") + tag.span(_('Recent searches'), class: "d-none d-md-inline") + = dropdown_tag(text, options: { wrapper_class: "filtered-search-history-dropdown-wrapper", toggle_class: "btn filtered-search-history-dropdown-toggle-button", dropdown_class: "filtered-search-history-dropdown", diff --git a/changelogs/unreleased/227570-use-files-count-for-external-diff-migration.yml b/changelogs/unreleased/227570-use-files-count-for-external-diff-migration.yml new file mode 100644 index 00000000000..a13a26f32f0 --- /dev/null +++ b/changelogs/unreleased/227570-use-files-count-for-external-diff-migration.yml @@ -0,0 +1,5 @@ +--- +title: Use more-efficient indexing for the MergeRequestDiff storage migration +merge_request: 39470 +author: +type: performance diff --git a/changelogs/unreleased/235797-fix-issues-analytics-feature-name.yml b/changelogs/unreleased/233119-deprecate-repository-contributors-additions-deletions.yml index 2221ab750b6..2221ab750b6 100644 --- a/changelogs/unreleased/235797-fix-issues-analytics-feature-name.yml +++ b/changelogs/unreleased/233119-deprecate-repository-contributors-additions-deletions.yml diff --git a/changelogs/unreleased/237337-revisit-history-icon-on-recent-search.yml b/changelogs/unreleased/237337-revisit-history-icon-on-recent-search.yml new file mode 100644 index 00000000000..4dd12efa48e --- /dev/null +++ b/changelogs/unreleased/237337-revisit-history-icon-on-recent-search.yml @@ -0,0 +1,5 @@ +--- +title: Use history icon on recent search filter tab only on mobile +merge_request: 39557 +author: Takuya Noguchi +type: fixed diff --git a/changelogs/unreleased/33909-peek-api-requests.yml b/changelogs/unreleased/33909-peek-api-requests.yml new file mode 100644 index 00000000000..f6e90187175 --- /dev/null +++ b/changelogs/unreleased/33909-peek-api-requests.yml @@ -0,0 +1,5 @@ +--- +title: Support adding of API requests to the performance bar +merge_request: 39057 +author: +type: added diff --git a/changelogs/unreleased/astoicescu-fix_panel_more_actions_button.yml b/changelogs/unreleased/astoicescu-fix_panel_more_actions_button.yml new file mode 100644 index 00000000000..0324f271223 --- /dev/null +++ b/changelogs/unreleased/astoicescu-fix_panel_more_actions_button.yml @@ -0,0 +1,5 @@ +--- +title: Fix panel "more actions" button layout +merge_request: 39534 +author: +type: fixed diff --git a/changelogs/unreleased/pb-gitlab-shell-13-6-0.yml b/changelogs/unreleased/pb-gitlab-shell-13-6-0.yml new file mode 100644 index 00000000000..02c8f56fce9 --- /dev/null +++ b/changelogs/unreleased/pb-gitlab-shell-13-6-0.yml @@ -0,0 +1,5 @@ +--- +title: Update gitlab-shell to v13.6.0 +merge_request: 39675 +author: +type: added diff --git a/config/initializers/peek.rb b/config/initializers/peek.rb index deac938c80b..fa74d8620f4 100644 --- a/config/initializers/peek.rb +++ b/config/initializers/peek.rb @@ -14,3 +14,9 @@ Peek.into Peek::Views::Rugged Peek.into Peek::Views::BulletDetailed if defined?(Bullet) Peek.into Peek::Views::Tracing if Labkit::Tracing.tracing_url_enabled? + +ActiveSupport::Notifications.subscribe('endpoint_run.grape') do |_name, _start, _finish, _id, payload| + if request_id = payload[:env]['action_dispatch.request_id'] + Peek.adapter.save(request_id) + end +end diff --git a/db/migrate/20200813143304_add_new_external_diff_migration_index.rb b/db/migrate/20200813143304_add_new_external_diff_migration_index.rb new file mode 100644 index 00000000000..64611e4f50a --- /dev/null +++ b/db/migrate/20200813143304_add_new_external_diff_migration_index.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +class AddNewExternalDiffMigrationIndex < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + INDEX_NAME = 'index_merge_request_diffs_by_id_partial' + + disable_ddl_transaction! + + def up + add_concurrent_index( + :merge_request_diffs, + :id, + name: INDEX_NAME, + where: 'files_count > 0 AND ((NOT stored_externally) OR (stored_externally IS NULL))' + ) + end + + def down + remove_concurrent_index_by_name(:merge_request_diffs, INDEX_NAME) + end +end diff --git a/db/migrate/20200813143356_remove_old_external_diff_migration_index.rb b/db/migrate/20200813143356_remove_old_external_diff_migration_index.rb new file mode 100644 index 00000000000..9b466f8734f --- /dev/null +++ b/db/migrate/20200813143356_remove_old_external_diff_migration_index.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +class RemoveOldExternalDiffMigrationIndex < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + remove_concurrent_index_by_name( + :merge_request_diffs, + 'index_merge_request_diffs_on_merge_request_id_and_id_partial' + ) + end + + def down + add_concurrent_index( + :merge_request_diffs, + [:merge_request_id, :id], + where: { stored_externally: [nil, false] } + ) + end +end diff --git a/db/schema_migrations/20200813143304 b/db/schema_migrations/20200813143304 new file mode 100644 index 00000000000..7c6f8560e65 --- /dev/null +++ b/db/schema_migrations/20200813143304 @@ -0,0 +1 @@ +12bb243862adf930fc68f2f7641ee7cc6170bfbcc3aae2b98bfa262dacfeba49
\ No newline at end of file diff --git a/db/schema_migrations/20200813143356 b/db/schema_migrations/20200813143356 new file mode 100644 index 00000000000..bffd3f45ba7 --- /dev/null +++ b/db/schema_migrations/20200813143356 @@ -0,0 +1 @@ +7c4fec044a278fa51fdd23320a372528c420e8a650a26770fccf0cabe91163c5
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 8b14be0905b..78b1e88e38f 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -20003,14 +20003,14 @@ CREATE INDEX index_merge_request_diff_commits_on_sha ON public.merge_request_dif CREATE UNIQUE INDEX index_merge_request_diff_files_on_mr_diff_id_and_order ON public.merge_request_diff_files USING btree (merge_request_diff_id, relative_order); +CREATE INDEX index_merge_request_diffs_by_id_partial ON public.merge_request_diffs USING btree (id) WHERE ((files_count > 0) AND ((NOT stored_externally) OR (stored_externally IS NULL))); + CREATE INDEX index_merge_request_diffs_external_diff_store_is_null ON public.merge_request_diffs USING btree (id) WHERE (external_diff_store IS NULL); CREATE INDEX index_merge_request_diffs_on_external_diff_store ON public.merge_request_diffs USING btree (external_diff_store); CREATE INDEX index_merge_request_diffs_on_merge_request_id_and_id ON public.merge_request_diffs USING btree (merge_request_id, id); -CREATE INDEX index_merge_request_diffs_on_merge_request_id_and_id_partial ON public.merge_request_diffs USING btree (merge_request_id, id) WHERE ((NOT stored_externally) OR (stored_externally IS NULL)); - CREATE INDEX index_merge_request_metrics_on_first_deployed_to_production_at ON public.merge_request_metrics USING btree (first_deployed_to_production_at); CREATE INDEX index_merge_request_metrics_on_latest_closed_at ON public.merge_request_metrics USING btree (latest_closed_at) WHERE (latest_closed_at IS NOT NULL); diff --git a/doc/README.md b/doc/README.md index 49170b75dbd..02a3e0981eb 100644 --- a/doc/README.md +++ b/doc/README.md @@ -50,7 +50,7 @@ Have a look at some of our most popular topics: ## The entire DevOps Lifecycle GitLab is the first single application for software development, security, -and operations that enables [Concurrent DevOps](https://about.gitlab.com/concurrent-devops/), +and operations that enables [Concurrent DevOps](https://about.gitlab.com/topics/concurrent-devops/), making the software lifecycle faster and radically improving the speed of business. GitLab provides solutions for [each of the stages of the DevOps lifecycle](https://about.gitlab.com/stages-devops-lifecycle/): diff --git a/doc/administration/consul.md b/doc/administration/consul.md index faae952e2e2..ae22d8bd4d0 100644 --- a/doc/administration/consul.md +++ b/doc/administration/consul.md @@ -235,5 +235,5 @@ Shortly after that, the client agents should rejoin as well. If you have taken advantage of Consul to store other data and want to restore the failed node, follow the -[Consul guide](https://learn.hashicorp.com/consul/day-2-operations/outage) +[Consul guide](https://learn.hashicorp.com/tutorials/consul/recovery-outage) to recover a failed cluster. diff --git a/doc/administration/geo/replication/database.md b/doc/administration/geo/replication/database.md index 980a5b43501..0bc37ce6438 100644 --- a/doc/administration/geo/replication/database.md +++ b/doc/administration/geo/replication/database.md @@ -373,7 +373,7 @@ There is an [issue where support is being discussed](https://gitlab.com/gitlab-o ## postgresql['sql_user_password'] = '<md5_hash_of_your_password>' gitlab_rails['db_password'] = '<your_password_here>' - + ``` For external PostgreSQL instances, see [additional instructions](external_database.md). If you bring a former **primary** node back online to serve as a **secondary** node, then you also need to remove `roles ['geo_primary_role']` or `geo_primary_role['enable'] = true`. diff --git a/doc/administration/gitaly/reference.md b/doc/administration/gitaly/reference.md index 0429149ec2d..0c211c220d7 100644 --- a/doc/administration/gitaly/reference.md +++ b/doc/administration/gitaly/reference.md @@ -233,7 +233,7 @@ The following values configure logging in Gitaly under the `[logging]` section. | `format` | string | no | Log format: `text` or `json`. Default: `text`. | | `level` | string | no | Log level: `debug`, `info`, `warn`, `error`, `fatal`, or `panic`. Default: `info`. | | `sentry_dsn` | string | no | Sentry DSN for exception monitoring. | -| `sentry_environment` | string | no | [Sentry Environment](https://docs.sentry.io/enriching-error-data/environments/) for exception monitoring. | +| `sentry_environment` | string | no | [Sentry Environment](https://docs.sentry.io/product/sentry-basics/environments/) for exception monitoring. | | `ruby_sentry_dsn` | string | no | Sentry DSN for `gitaly-ruby` exception monitoring. | While the main Gitaly application logs go to `stdout`, there are some extra log diff --git a/doc/api/environments.md b/doc/api/environments.md index 2287ec9aad2..98391f16571 100644 --- a/doc/api/environments.md +++ b/doc/api/environments.md @@ -62,7 +62,7 @@ Example of response "id": 1, "name": "review/fix-foo", "slug": "review-fix-foo-dfjre3", - "external_url": "https://review-fix-foo-dfjre3.example.gitlab.com" + "external_url": "https://review-fix-foo-dfjre3.example.gitlab.com", "state": "available", "last_deployment": { "id": 100, @@ -78,7 +78,7 @@ Example of response "username": "root", "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", "web_url": "http://localhost:3000/root" - } + }, "deployable": { "id": 710, "status": "success", @@ -107,7 +107,7 @@ Example of response "twitter": "", "website_url": "", "organization": null - } + }, "commit": { "id": "416d8ea11849050d3d1f5104cf8cf51053e790ab", "short_id": "416d8ea1", diff --git a/doc/ci/merge_request_pipelines/index.md b/doc/ci/merge_request_pipelines/index.md index 6a6e34b4dbd..73b971c2d41 100644 --- a/doc/ci/merge_request_pipelines/index.md +++ b/doc/ci/merge_request_pipelines/index.md @@ -208,7 +208,7 @@ The variable names begin with the `CI_MERGE_REQUEST_` prefix. ### Two pipelines created when pushing to a merge request If you are experiencing duplicated pipelines when using `rules`, take a look at -the [important differences between `rules` and `only`/`except`](../yaml/README.md#differences-between-rules-and-onlyexcept), +the [important differences between `rules` and `only`/`except`](../yaml/README.md#prevent-duplicate-pipelines), which will help you get your starting configuration correct. If you are seeing two pipelines when using `only/except`, please see the caveats diff --git a/doc/ci/troubleshooting.md b/doc/ci/troubleshooting.md index 3994f3758c4..96d94a6c165 100644 --- a/doc/ci/troubleshooting.md +++ b/doc/ci/troubleshooting.md @@ -7,6 +7,24 @@ type: reference # Troubleshooting CI/CD +## Pipeline warnings + +Pipeline configuration warnings are shown when you: + +- [View pipeline details](pipelines/index.md#view-pipelines). +- [Validate configuration with the CI Lint tool](yaml/README.md#validate-the-gitlab-ciyml). +- [Manually run a pipeline](pipelines/index.md#run-a-pipeline-manually). + +### "Job may allow multiple pipelines to run for a single action" + +When you use [`rules`](yaml/README.md#rules) with a `when:` clause without +an `if:` clause, multiple pipelines may run. Usually +this occurs when you push a commit to a branch that has an open merge request associated with it. + +To [prevent duplicate pipelines](yaml/README.md#prevent-duplicate-pipelines), use +[`workflow: rules`](yaml/README.md#workflowrules) or rewrite your rules +to control which pipelines can run. + ## Merge request pipeline widget The merge request pipeline widget shows information about the pipeline status in a Merge Request. It's displayed above the [merge request ability to merge widget](#merge-request-ability-to-merge-widget). diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index 82a410129a6..c5caef39a9f 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -348,7 +348,7 @@ but does allow pipelines in **all** other cases, *including* merge request pipel As with `rules` defined in jobs, be careful not to use a configuration that allows merge request pipelines and branch pipelines to run at the same time, or you could -have [duplicate pipelines](#differences-between-rules-and-onlyexcept). +have [duplicate pipelines](#prevent-duplicate-pipelines). #### `workflow:rules` templates @@ -1218,19 +1218,22 @@ job: - In **all other cases**, the job is added to the pipeline, with `when: on_success`. CAUTION: **Caution:** -If you use `when: on_success`, `always`, or `delayed` as the final rule, two +If you use a `when:` clause as the final rule (not including `when: never`), two simultaneous pipelines may start. Both push pipelines and merge request pipelines can be triggered by the same event (a push to the source branch for an open merge request). -See the [important differences between `rules` and `only`/`except`](#differences-between-rules-and-onlyexcept) +See how to [prevent duplicate pipelines](#prevent-duplicate-pipelines) for more details. -#### Differences between `rules` and `only`/`except` +#### Prevent duplicate pipelines -Jobs defined with `only/except` do not trigger merge request pipelines by default. -You must explicitly add `only: merge_requests`. +Jobs defined with `rules` can trigger multiple pipelines with the same action. You +don't have to explicitly configure rules for each type of pipeline to trigger them +accidentally. Rules that are too loose (allowing too many types of pipelines) could +cause a second pipeline to run unexpectedly. -Jobs defined with `rules` can trigger all types of pipelines. -You do not have to explicitly configure each type. +Some configurations that have the potential to cause duplicate pipelines cause a +[pipeline warning](../troubleshooting.md#pipeline-warnings) to be displayed. +[Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/219431) in GitLab 13.3. For example: @@ -1246,21 +1249,77 @@ job: This job does not run when `$CUSTOM_VARIABLE` is false, but it *does* run in **all** other pipelines, including **both** push (branch) and merge request pipelines. With this configuration, every push to an open merge request's source branch -causes duplicated pipelines. Explicitly allowing both push and merge request pipelines -in the same job could have the same effect. +causes duplicated pipelines. -We recommend using [`workflow: rules`](#workflowrules) to limit which types of pipelines -are permitted. Allowing only merge request pipelines, or only branch pipelines, -eliminates duplicated pipelines. Alternatively, you can rewrite the rules to be -stricter, or avoid using a final `when` (`always`, `on_success` or `delayed`). +There are multiple ways to avoid this: -It is not possible to run a job for branch pipelines first, then only for merge request -pipelines after the merge request is created (skipping the duplicate branch pipeline). See -the [related issue](https://gitlab.com/gitlab-org/gitlab/-/issues/201845) for more details. +- Use [`workflow: rules`](#workflowrules) to specify which types of pipelines + can run. To eliminate duplicate pipelines, allow only merge request pipelines + or push (branch) pipelines. -Also, we don't recommend mixing `only/except` jobs with `rules` jobs in the same pipeline. -It may not cause YAML errors, but debugging the exact execution behavior can be complex -due to the different default behaviors of `only/except` and `rules`. +- Rewrite the rules to run the job only in very specific cases, + and avoid using a final `when:` rule: + + ```yaml + job: + script: "echo This does NOT create double pipelines!" + rules: + - if: '$CUSTOM_VARIABLE == "true" && $CI_PIPELINE_SOURCE == "merge_request_event"' + ``` + +You can prevent duplicate pipelines by changing the job rules to avoid either push (branch) +pipelines or merge request pipelines. However, if you use a `- when: always` rule without +`workflow: rules`, GitLab still displays a [pipeline warning](../troubleshooting.md#pipeline-warnings). + +For example, the following does not trigger double pipelines, but is not recommended +without `workflow: rules`: + +```yaml +job: + script: "echo This does NOT create double pipelines!" + rules: + - if: '$CI_PIPELINE_SOURCE == "push"' + when: never + - when: always +``` + +Do not include both push and merge request pipelines in the same job: + +```yaml +job: + script: "echo This creates double pipelines!" + rules: + - if: '$CI_PIPELINE_SOURCE == "push"' + - if: '$CI_PIPELINE_SOURCE == "merge_request_event"' +``` + +Also, do not mix `only/except` jobs with `rules` jobs in the same pipeline. +It may not cause YAML errors, but the different default behaviors of `only/except` +and `rules` can cause issues that are difficult to troubleshoot: + +```yaml +job-with-no-rules: + script: "echo This job runs in branch pipelines." + +job-with-rules: + script: "echo This job runs in merge request pipelines." + rules: + - if: '$CI_PIPELINE_SOURCE == "merge_request_event"' +``` + +For every change pushed to the branch, duplicate pipelines run. One +branch pipeline runs a single job (`job-with-no-rules`), and one merge request pipeline +runs the other job (`job-with-rules`). Jobs with no rules default +to [`except: merge_requests`](#onlyexcept-basic), so `job-with-no-rules` +runs in all cases except merge requests. + +It is not possible to define rules based on whether or not a branch has an open +merge request associated with it. You can't configure a job to be included in: + +- Only branch pipelines when the branch doesn't have a merge request associated with it. +- Only merge request pipelines when the branch has a merge request associated with it. + +See the [related issue](https://gitlab.com/gitlab-org/gitlab/-/issues/201845) for more details. #### `rules:if` diff --git a/doc/development/documentation/structure.md b/doc/development/documentation/structure.md index 44df003494e..e13b2f4d031 100644 --- a/doc/development/documentation/structure.md +++ b/doc/development/documentation/structure.md @@ -233,4 +233,4 @@ In the same code block, precede each with comments: `# Better` and `# Best`. NOTE: **Note:** While the bad-then-good approach is acceptable for the GitLab development guidelines, do not use it for user documentation. For user documentation, use "Do" and "Don't." For example, see the -[Pajamas Design System](https://design.gitlab.com/content/punctuation). +[Pajamas Design System](https://design.gitlab.com/content/punctuation/). diff --git a/doc/development/fe_guide/style/scss.md b/doc/development/fe_guide/style/scss.md index af9c56df266..dba39eeb98c 100644 --- a/doc/development/fe_guide/style/scss.md +++ b/doc/development/fe_guide/style/scss.md @@ -53,8 +53,8 @@ Examples of component classes that were created using "utility-first" include: Inspiration: -- <https://tailwindcss.com/docs/utility-first/> -- <https://tailwindcss.com/docs/extracting-components/> +- <https://tailwindcss.com/docs/utility-first> +- <https://tailwindcss.com/docs/extracting-components> ### Naming diff --git a/doc/development/fe_guide/vue.md b/doc/development/fe_guide/vue.md index 8fb24b9c22e..58a8332589d 100644 --- a/doc/development/fe_guide/vue.md +++ b/doc/development/fe_guide/vue.md @@ -285,7 +285,7 @@ describe('~/todos/app.vue', () => { ### Test the component's output The main return value of a Vue component is the rendered output. In order to test the component we -need to test the rendered output. [Vue](https://vuejs.org/v2/guide/unit-testing.html) guide's to unit test show us exactly that: +need to test the rendered output. Visit the [Vue testing guide](https://vuejs.org/v2/guide/testing.html#Unit-Testing). ### Events diff --git a/doc/development/new_fe_guide/development/performance.md b/doc/development/new_fe_guide/development/performance.md index 7b58a80576a..ad6fdc0b85c 100644 --- a/doc/development/new_fe_guide/development/performance.md +++ b/doc/development/new_fe_guide/development/performance.md @@ -10,7 +10,7 @@ Any frontend engineer can contribute to this dashboard. They can contribute by a There are 3 recommended high impact metrics to review on each page: - [First visual change](https://web.dev/first-meaningful-paint/) -- [Speed Index](https://sites.google.com/a/webpagetest.org/docs/using-webpagetest/metrics/speed-index) -- [Visual Complete 95%](https://sites.google.com/a/webpagetest.org/docs/using-webpagetest/metrics/speed-index) +- [Speed Index](https://github.com/WPO-Foundation/webpagetest-docs/blob/master/user/Metrics/SpeedIndex.md) +- [Visual Complete 95%](https://github.com/WPO-Foundation/webpagetest-docs/blob/master/user/Metrics/SpeedIndex.md) For these metrics, lower numbers are better as it means that the website is more performant. diff --git a/doc/operations/error_tracking.md b/doc/operations/error_tracking.md index 15d3272dc9e..c5699ee3d22 100644 --- a/doc/operations/error_tracking.md +++ b/doc/operations/error_tracking.md @@ -26,11 +26,13 @@ You will need at least Maintainer [permissions](../user/permissions.md) to enabl GitLab provides an easy way to connect Sentry to your project: 1. Sign up to Sentry.io or [deploy your own](#deploying-sentry) Sentry instance. -1. [Create](https://docs.sentry.io/guides/tutorials/integrate-frontend/create-new-project/) a new Sentry project. For each GitLab project that you want to integrate, we recommend that you create a new Sentry project. +1. [Create](https://docs.sentry.io/product/sentry-basics/guides/integrate-frontend/create-new-project/) a new Sentry project. For each GitLab project that you want to integrate, we recommend that you create a new Sentry project. 1. [Find or generate](https://docs.sentry.io/api/auth/) a Sentry auth token for your Sentry project. Make sure to give the token at least the following scopes: `event:read` and `project:read`. -1. Navigate to your project’s **Settings > Operations**. -1. Ensure that the **Active** checkbox is set. +1. In GitLab, navigate to your project’s **Operations > Error Tracking** page, and + click **Enable Error Tracking**. +1. Navigate to your project’s **Settings > Operations**. In the **Error Tracking** section, + ensure the **Active** checkbox is set. 1. In the **Sentry API URL** field, enter your Sentry hostname. For example, enter `https://sentry.example.com` if this is the address at which your Sentry instance is available. For the SaaS version of Sentry, the hostname will be `https://sentry.io`. 1. In the **Auth Token** field, enter the token you previously generated. 1. Click the **Connect** button to test the connection to Sentry and populate the **Project** dropdown. @@ -40,7 +42,7 @@ GitLab provides an easy way to connect Sentry to your project: ### Enabling GitLab issues links -You may also want to enable Sentry's GitLab integration by following the steps in the [Sentry documentation](https://docs.sentry.io/workflow/integrations/gitlab/) +You may also want to enable Sentry's GitLab integration by following the steps in the [Sentry documentation](https://docs.sentry.io/product/integrations/gitlab/) ## Error Tracking List @@ -59,7 +61,7 @@ From error list, users can navigate to the error details page by clicking the ti This page has: - A link to the Sentry issue. -- A link to the GitLab commit if the Sentry [release ID/version](https://docs.sentry.io/workflow/releases/?platform=javascript#configure-sdk) on the Sentry Issue's first release matches a commit SHA in your GitLab hosted project. +- A link to the GitLab commit if the Sentry [release ID/version](https://docs.sentry.io/product/releases/?platform=javascript#configure-sdk) on the Sentry Issue's first release matches a commit SHA in your GitLab hosted project. - Other details about the issue, including a full stack trace. - In [GitLab 12.7 and newer](https://gitlab.com/gitlab-org/gitlab/-/issues/36246), language and urgency are displayed. diff --git a/doc/user/clusters/applications.md b/doc/user/clusters/applications.md index 74b858a4b1e..3b04c7aac18 100644 --- a/doc/user/clusters/applications.md +++ b/doc/user/clusters/applications.md @@ -1189,7 +1189,7 @@ server: } ``` -Once you have successfully installed Vault, you will need to [initialize the Vault](https://learn.hashicorp.com/vault/getting-started/deploy#initializing-the-vault) +Once you have successfully installed Vault, you will need to [initialize the Vault](https://learn.hashicorp.com/tutorials/vault/getting-started-deploy#initializing-the-vault) and obtain the initial root token. You will need access to your Kubernetes cluster that Vault has been deployed into in order to do this. To initialize the Vault, get a shell to one of the Vault pods running inside Kubernetes (typically this is done by using the `kubectl` command line tool). Once you have a shell into the pod, run the `vault operator init` command: diff --git a/doc/user/compliance/license_compliance/index.md b/doc/user/compliance/license_compliance/index.md index 9119991e02e..47f14b93d29 100644 --- a/doc/user/compliance/license_compliance/index.md +++ b/doc/user/compliance/license_compliance/index.md @@ -75,15 +75,15 @@ which means that the reported licenses might be incomplete or inaccurate. | Language | Package managers | Scan Tool | |------------|-------------------------------------------------------------------|----------------------------------------------------------| -| JavaScript | [yarn](https://yarnpkg.com/)|[License Finder](https://github.com/pivotal/LicenseFinder)| +| JavaScript | [Yarn](https://yarnpkg.com/)|[License Finder](https://github.com/pivotal/LicenseFinder)| | Go | go get, gvt, glide, dep, trash, govendor |[License Finder](https://github.com/pivotal/LicenseFinder)| -| Erlang | [rebar](https://www.rebar3.org/) |[License Finder](https://github.com/pivotal/LicenseFinder)| +| Erlang | [Rebar](https://www.rebar3.org/) |[License Finder](https://github.com/pivotal/LicenseFinder)| | Objective-C, Swift | [CocoaPods](https://cocoapods.org/) v0.39 and below |[License Finder](https://github.com/pivotal/LicenseFinder)| -| Elixir | [mix](https://elixir-lang.org/getting-started/mix-otp/introduction-to-mix.html) |[License Finder](https://github.com/pivotal/LicenseFinder)| -| C++/C | [conan](https://conan.io/) |[License Finder](https://github.com/pivotal/LicenseFinder)| +| Elixir | [Mix](https://elixir-lang.org/getting-started/mix-otp/introduction-to-mix.html) |[License Finder](https://github.com/pivotal/LicenseFinder)| +| C++/C | [Conan](https://conan.io/) |[License Finder](https://github.com/pivotal/LicenseFinder)| | Scala | [sbt](https://www.scala-sbt.org/) |[License Finder](https://github.com/pivotal/LicenseFinder)| -| Rust | [cargo](https://crates.io) |[License Finder](https://github.com/pivotal/LicenseFinder)| -| PHP | [composer](https://getcomposer.org/) |[License Finder](https://github.com/pivotal/LicenseFinder)| +| Rust | [Cargo](https://crates.io) |[License Finder](https://github.com/pivotal/LicenseFinder)| +| PHP | [Composer](https://getcomposer.org/) |[License Finder](https://github.com/pivotal/LicenseFinder)| ## Requirements @@ -330,13 +330,13 @@ strict-ssl = false ### Configuring Yarn projects -You can configure Yarn projects by using a [`.yarnrc.yml`](https://yarnpkg.com/configuration/yarnrc) +You can configure Yarn projects by using a [`.yarnrc.yml`](https://yarnpkg.com/configuration/yarnrc/) file. #### Using private Yarn registries If you have a private Yarn registry you can use the -[`npmRegistryServer`](https://yarnpkg.com/configuration/yarnrc#npmRegistryServer) +[`npmRegistryServer`](https://yarnpkg.com/configuration/yarnrc/#npmRegistryServer) setting to specify its location. For example: diff --git a/doc/user/group/saml_sso/index.md b/doc/user/group/saml_sso/index.md index 13878e72162..ed63e8e13ef 100644 --- a/doc/user/group/saml_sso/index.md +++ b/doc/user/group/saml_sso/index.md @@ -162,7 +162,7 @@ For more information, see our [discussion on providers](#providers). Your identity provider may have relevant documentation. It may be generic SAML documentation, or specifically targeted for GitLab. Examples: -- [Auth0](https://auth0.com/docs/protocols/saml/saml-idp-generic) +- [Auth0](https://auth0.com/docs/protocols/saml-configuration-options/configure-auth0-as-saml-identity-provider) - [G Suite](https://support.google.com/a/answer/6087519?hl=en) - [JumpCloud](https://support.jumpcloud.com/support/s/article/single-sign-on-sso-with-gitlab-2019-08-21-10-36-47) - [PingOne by Ping Identity](https://docs.pingidentity.com/bundle/pingone/page/xsh1564020480660-1.html) diff --git a/doc/user/infrastructure/index.md b/doc/user/infrastructure/index.md index d6ab79e0aee..525b447dd69 100644 --- a/doc/user/infrastructure/index.md +++ b/doc/user/infrastructure/index.md @@ -427,7 +427,7 @@ apply: ### Multiple Terraform Plan reports -Starting with 13.2, you can display mutiple reports on the Merge Request page. The reports will also display the `artifact: name:`. See example below for a suggested setup. +Starting with 13.2, you can display mutiple reports on the Merge Request page. The reports will also display the `artifacts: name:`. See example below for a suggested setup. ```yaml image: diff --git a/doc/user/instance_statistics/dev_ops_score.md b/doc/user/instance_statistics/dev_ops_score.md index 73b9532015d..35dcbd01916 100644 --- a/doc/user/instance_statistics/dev_ops_score.md +++ b/doc/user/instance_statistics/dev_ops_score.md @@ -7,7 +7,7 @@ NOTE: **Note:** Your GitLab instance's [usage ping](../admin_area/settings/usage_statistics.md#usage-ping-core-only) must be activated in order to use this feature. The DevOps Score gives you an overview of your entire instance's adoption of -[Concurrent DevOps](https://about.gitlab.com/concurrent-devops/) +[Concurrent DevOps](https://about.gitlab.com/topics/concurrent-devops/) from planning to monitoring. This displays the usage of these GitLab features over diff --git a/doc/user/packages/index.md b/doc/user/packages/index.md index 9e037ef651b..92d31c31986 100644 --- a/doc/user/packages/index.md +++ b/doc/user/packages/index.md @@ -18,11 +18,11 @@ The Package Registry supports the following formats: <tr style="background:#dfdfdf"><th>Package type</th><th>GitLab version</th></tr> <tr><td><a href="https://docs.gitlab.com/ee/user/packages/composer_repository/index.html">Composer</a></td><td>13.2+</td></tr> <tr><td><a href="https://docs.gitlab.com/ee/user/packages/conan_repository/index.html">Conan</a></td><td>12.6+</td></tr> -<tr><td><a href="http://docs.gitlab.com/ee/user/packages/go_proxy/index.html">Go</a></td><td>13.1+</td></tr> -<tr><td><a href="http://docs.gitlab.com/ee/user/packages/maven_repository/index.html">Maven</a></td><td>11.3+</td></tr> +<tr><td><a href="https://docs.gitlab.com/ee/user/packages/go_proxy/index.html">Go</a></td><td>13.1+</td></tr> +<tr><td><a href="https://docs.gitlab.com/ee/user/packages/maven_repository/index.html">Maven</a></td><td>11.3+</td></tr> <tr><td><a href="https://docs.gitlab.com/ee/user/packages/npm_registry/index.html">NPM</a></td><td>11.7+</td></tr> -<tr><td><a href="http://docs.gitlab.com/ee/user/packages/nuget_repository/index.html">NuGet</a></td><td>12.8+</td></tr> -<tr><td><a href="http://docs.gitlab.com/ee/user/packages/pypi_repository/index.html">PyPI</a></td><td>12.10+</td></tr> +<tr><td><a href="https://docs.gitlab.com/ee/user/packages/nuget_repository/index.html">NuGet</a></td><td>12.8+</td></tr> +<tr><td><a href="https://docs.gitlab.com/ee/user/packages/pypi_repository/index.html">PyPI</a></td><td>12.10+</td></tr> </table> </div> </div> diff --git a/doc/user/project/merge_requests/merge_when_pipeline_succeeds.md b/doc/user/project/merge_requests/merge_when_pipeline_succeeds.md index 7d90c9f3cd7..5151b28361a 100644 --- a/doc/user/project/merge_requests/merge_when_pipeline_succeeds.md +++ b/doc/user/project/merge_requests/merge_when_pipeline_succeeds.md @@ -84,7 +84,7 @@ merge-request-pipeline-job: ``` You should avoid configuration like this, and only use branch (`push`) pipelines -or merge request pipelines, when possible. See [`rules` documentation](../../../ci/yaml/README.md#differences-between-rules-and-onlyexcept) +or merge request pipelines, when possible. See [`rules` documentation](../../../ci/yaml/README.md#prevent-duplicate-pipelines) for details on avoiding two pipelines for a single merge request. ### Skipped pipelines diff --git a/doc/user/project/pages/custom_domains_ssl_tls_certification/ssl_tls_concepts.md b/doc/user/project/pages/custom_domains_ssl_tls_certification/ssl_tls_concepts.md index 8844a4eb9aa..f30361e6669 100644 --- a/doc/user/project/pages/custom_domains_ssl_tls_certification/ssl_tls_concepts.md +++ b/doc/user/project/pages/custom_domains_ssl_tls_certification/ssl_tls_concepts.md @@ -56,7 +56,7 @@ reiterating the importance of HTTPS. ## Issuing Certificates -GitLab Pages accepts certificates provided in the [PEM](https://support.quovadisglobal.com/kb/a37/what-is-pem-format.aspx) format, issued by +GitLab Pages accepts certificates provided in the [PEM](https://knowledge.digicert.com/quovadis) format, issued by [Certificate Authorities](https://en.wikipedia.org/wiki/Certificate_authority) or as [self-signed certificates](https://en.wikipedia.org/wiki/Self-signed_certificate). Note that [self-signed certificates are typically not used](https://www.mcafee.com/blogs/other-blogs/mcafee-labs/self-signed-certificates-secure-so-why-ban/) for public websites for security reasons and to ensure that browsers trust your site's certificate. diff --git a/doc/user/project/static_site_editor/img/wysiwyg_editor_v13_0.png b/doc/user/project/static_site_editor/img/wysiwyg_editor_v13_0.png Binary files differdeleted file mode 100644 index a2f5248bf39..00000000000 --- a/doc/user/project/static_site_editor/img/wysiwyg_editor_v13_0.png +++ /dev/null diff --git a/doc/user/project/static_site_editor/img/wysiwyg_editor_v13_3.png b/doc/user/project/static_site_editor/img/wysiwyg_editor_v13_3.png Binary files differnew file mode 100644 index 00000000000..52776c6a290 --- /dev/null +++ b/doc/user/project/static_site_editor/img/wysiwyg_editor_v13_3.png diff --git a/doc/user/project/static_site_editor/index.md b/doc/user/project/static_site_editor/index.md index b64fe632732..4e401014122 100644 --- a/doc/user/project/static_site_editor/index.md +++ b/doc/user/project/static_site_editor/index.md @@ -13,6 +13,7 @@ description: "The static site editor enables users to edit content on static web > - Support for adding images through the WYSIWYG editor [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/216640) in GitLab 13.1. > - Markdown front matter hidden on the WYSIWYG editor [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/216834) in GitLab 13.1. > - Support for `*.md.erb` files [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/223171) in GitLab 13.2. +> - Non-Markdown content blocks uneditable on the WYSIWYG mode [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/216836) in GitLab 13.3. DANGER: **Danger:** In GitLab 13.0, we [introduced breaking changes](https://gitlab.com/gitlab-org/gitlab/-/issues/213282) @@ -57,7 +58,7 @@ When you click it, GitLab opens up an editor window from which the content can be directly edited. When you're ready, you can submit your changes in a click of a button: -![Static Site Editor](img/wysiwyg_editor_v13_0.png) +![Static Site Editor](img/wysiwyg_editor_v13_3.png) When an editor submits their changes, in the background, GitLab automatically creates a new branch, commits their changes, and opens a merge request. The diff --git a/lib/api/api.rb b/lib/api/api.rb index ef8c2b2cde5..2be6792af5f 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -56,6 +56,10 @@ module API ) end + before do + set_peek_enabled_for_current_request + end + # The locale is set to the current user's locale when `current_user` is loaded after { Gitlab::I18n.use_default_locale } @@ -116,6 +120,7 @@ module API # Ensure the namespace is right, otherwise we might load Grape::API::Helpers helpers ::API::Helpers helpers ::API::Helpers::CommonHelpers + helpers ::API::Helpers::PerformanceBarHelpers namespace do after do diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb index 4b87861a3de..59978962b1d 100644 --- a/lib/api/api_guard.rb +++ b/lib/api/api_guard.rb @@ -7,6 +7,7 @@ require 'rack/oauth2' module API module APIGuard extend ActiveSupport::Concern + include Gitlab::Utils::StrongMemoize included do |base| # OAuth2 Resource Server Authentication @@ -64,10 +65,12 @@ module API end def find_user_from_sources - deploy_token_from_request || - find_user_from_bearer_token || - find_user_from_job_token || - find_user_from_warden + strong_memoize(:find_user_from_sources) do + deploy_token_from_request || + find_user_from_bearer_token || + find_user_from_job_token || + find_user_from_warden + end end private diff --git a/lib/api/helpers/performance_bar_helpers.rb b/lib/api/helpers/performance_bar_helpers.rb new file mode 100644 index 00000000000..8430e889dff --- /dev/null +++ b/lib/api/helpers/performance_bar_helpers.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module API + module Helpers + module PerformanceBarHelpers + def set_peek_enabled_for_current_request + Gitlab::SafeRequestStore.fetch(:peek_enabled) { perf_bar_cookie_enabled? && perf_bar_enabled_for_user? } + end + + def perf_bar_cookie_enabled? + cookies[:perf_bar_enabled] == 'true' + end + + def perf_bar_enabled_for_user? + # We cannot use `current_user` here because that method raises an exception when the user + # is unauthorized and some API endpoints require that `current_user` is not called. + Gitlab::PerformanceBar.enabled_for_user?(find_user_from_sources) + end + end + end +end diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb index 960da54a943..332d0bc1478 100644 --- a/lib/gitlab/auth.rb +++ b/lib/gitlab/auth.rb @@ -220,6 +220,9 @@ module Gitlab return unless token && login return if login != token.username + # Registry access (with jwt) does not have access to project + return if project && !token.has_access_to?(project) + scopes = abilities_for_scopes(token.scopes) if valid_scoped_token?(token, all_available_scopes) diff --git a/lib/gitlab/ci/config/entry/processable.rb b/lib/gitlab/ci/config/entry/processable.rb index 37ccb74a009..f10c509d0cc 100644 --- a/lib/gitlab/ci/config/entry/processable.rb +++ b/lib/gitlab/ci/config/entry/processable.rb @@ -102,7 +102,7 @@ module Gitlab last_rule = rules_value.last if last_rule&.keys == [:when] && last_rule[:when] != 'never' - docs_url = 'read more: https://docs.gitlab.com/ee/ci/yaml/README.html#differences-between-rules-and-onlyexcept' + docs_url = 'read more: https://docs.gitlab.com/ee/ci/troubleshooting.html#pipeline-warnings' add_warning("may allow multiple pipelines to run for a single action due to `rules:when` clause with no `workflow:rules` - #{docs_url}") end end diff --git a/lib/gitlab/ci/runner_instructions.rb b/lib/gitlab/ci/runner_instructions.rb new file mode 100644 index 00000000000..2171637687f --- /dev/null +++ b/lib/gitlab/ci/runner_instructions.rb @@ -0,0 +1,137 @@ +# frozen_string_literal: true + +module Gitlab + module Ci + class RunnerInstructions + class ArgumentError < ::ArgumentError; end + + include Gitlab::Allowable + + OS = { + linux: { + human_readable_name: "Linux", + download_locations: { + amd64: "https://gitlab-runner-downloads.s3.amazonaws.com/latest/binaries/gitlab-runner-linux-amd64", + '386': "https://gitlab-runner-downloads.s3.amazonaws.com/latest/binaries/gitlab-runner-linux-386", + arm: "https://gitlab-runner-downloads.s3.amazonaws.com/latest/binaries/gitlab-runner-linux-arm", + arm64: "https://gitlab-runner-downloads.s3.amazonaws.com/latest/binaries/gitlab-runner-linux-arm64" + }, + install_script_template_path: "lib/gitlab/ci/runner_instructions/templates/linux/install.sh", + runner_executable: "sudo gitlab-runner" + }, + osx: { + human_readable_name: "macOS", + download_locations: { + amd64: "https://gitlab-runner-downloads.s3.amazonaws.com/latest/binaries/gitlab-runner-darwin-amd64" + }, + install_script_template_path: "lib/gitlab/ci/runner_instructions/templates/osx/install.sh", + runner_executable: "sudo gitlab-runner" + }, + windows: { + human_readable_name: "Windows", + download_locations: { + amd64: "https://gitlab-runner-downloads.s3.amazonaws.com/latest/binaries/gitlab-runner-windows-amd64.exe", + '386': "https://gitlab-runner-downloads.s3.amazonaws.com/latest/binaries/gitlab-runner-windows-386.exe" + }, + install_script_template_path: "lib/gitlab/ci/runner_instructions/templates/windows/install.ps1", + runner_executable: "./gitlab-runner.exe" + } + }.freeze + + OTHER_ENVIRONMENTS = { + docker: { + human_readable_name: "Docker", + installation_instructions_url: "https://docs.gitlab.com/runner/install/docker.html" + }, + kubernetes: { + human_readable_name: "Kubernetes", + installation_instructions_url: "https://docs.gitlab.com/runner/install/kubernetes.html" + } + }.freeze + + attr_reader :errors + + def initialize(current_user:, group: nil, project: nil, os:, arch:) + @current_user = current_user + @group = group + @project = project + @os = os + @arch = arch + @errors = [] + + validate_params + end + + def install_script + with_error_handling [Gitlab::Ci::RunnerInstructions::ArgumentError] do + raise Gitlab::Ci::RunnerInstructions::ArgumentError, s_('Architecture not found for OS') unless environment[:download_locations].key?(@arch.to_sym) + + replace_variables(get_file(environment[:install_script_template_path])) + end + end + + def register_command + with_error_handling [Gitlab::Ci::RunnerInstructions::ArgumentError, Gitlab::Access::AccessDeniedError] do + raise Gitlab::Ci::RunnerInstructions::ArgumentError, s_('No runner executable') unless environment[:runner_executable] + + server_url = Gitlab::Routing.url_helpers.root_url(only_path: false) + runner_executable = environment[:runner_executable] + + "#{runner_executable} register --url #{server_url} --registration-token #{registration_token}" + end + end + + private + + def with_error_handling(exceptions) + return if errors.present? + + yield + rescue *exceptions => e + @errors << e.message + nil + end + + def environment + @environment ||= OS[@os.to_sym] || ( raise Gitlab::Ci::RunnerInstructions::ArgumentError, s_('Invalid OS') ) + end + + def validate_params + @errors << s_('Missing OS') unless @os.present? + @errors << s_('Missing arch') unless @arch.present? + end + + def replace_variables(expression) + expression.sub('${GITLAB_CI_RUNNER_DOWNLOAD_LOCATION}', "#{environment[:download_locations][@arch.to_sym]}") + end + + def get_file(path) + File.read(path) + end + + def registration_token + project_token || group_token || instance_token + end + + def project_token + return unless @project + raise Gitlab::Access::AccessDeniedError unless can?(@current_user, :admin_pipeline, @project) + + @project.runners_token + end + + def group_token + return unless @group + raise Gitlab::Access::AccessDeniedError unless can?(@current_user, :admin_group, @group) + + @group.runners_token + end + + def instance_token + raise Gitlab::Access::AccessDeniedError unless @current_user&.admin? + + Gitlab::CurrentSettings.runners_registration_token + end + end + end +end diff --git a/lib/gitlab/ci/runner_instructions/templates/linux/install.sh b/lib/gitlab/ci/runner_instructions/templates/linux/install.sh new file mode 100644 index 00000000000..6c8a0796d23 --- /dev/null +++ b/lib/gitlab/ci/runner_instructions/templates/linux/install.sh @@ -0,0 +1,12 @@ +# Download the binary for your system +sudo curl -L --output /usr/local/bin/gitlab-runner ${GITLAB_CI_RUNNER_DOWNLOAD_LOCATION} + +# Give it permissions to execute +sudo chmod +x /usr/local/bin/gitlab-runner + +# Create a GitLab CI user +sudo useradd --comment 'GitLab Runner' --create-home gitlab-runner --shell /bin/bash + +# Install and run as service +sudo gitlab-runner install --user=gitlab-runner --working-directory=/home/gitlab-runner +sudo gitlab-runner start diff --git a/lib/gitlab/ci/runner_instructions/templates/osx/install.sh b/lib/gitlab/ci/runner_instructions/templates/osx/install.sh new file mode 100644 index 00000000000..de4ee3e52fc --- /dev/null +++ b/lib/gitlab/ci/runner_instructions/templates/osx/install.sh @@ -0,0 +1,11 @@ +# Download the binary for your system +sudo curl --output /usr/local/bin/gitlab-runner ${GITLAB_CI_RUNNER_DOWNLOAD_LOCATION} + +# Give it permissions to execute +sudo chmod +x /usr/local/bin/gitlab-runner + +# The rest of commands execute as the user who will run the Runner +# Register the Runner (steps below), then run +cd ~ +gitlab-runner install +gitlab-runner start diff --git a/lib/gitlab/ci/runner_instructions/templates/windows/install.ps1 b/lib/gitlab/ci/runner_instructions/templates/windows/install.ps1 new file mode 100644 index 00000000000..dc37f88543c --- /dev/null +++ b/lib/gitlab/ci/runner_instructions/templates/windows/install.ps1 @@ -0,0 +1,13 @@ +# Run PowerShell: https://docs.microsoft.com/en-us/powershell/scripting/windows-powershell/starting-windows-powershell?view=powershell-7#with-administrative-privileges-run-as-administrator +# Create a folder somewhere in your system ex.: C:\GitLab-Runner +New-Item -Path 'C:\GitLab-Runner' -ItemType Directory + +# Enter the folder +cd 'C:\GitLab-Runner' + +# Dowload binary +Invoke-WebRequest -Uri "${GITLAB_CI_RUNNER_DOWNLOAD_LOCATION}" -OutFile "gitlab-runner.exe" + +# Register the Runner (steps below), then run +.\gitlab-runner.exe install +.\gitlab-runner.exe start diff --git a/lib/gitlab/import_export/file_importer.rb b/lib/gitlab/import_export/file_importer.rb index 3cb1eb72ceb..081745a49f4 100644 --- a/lib/gitlab/import_export/file_importer.rb +++ b/lib/gitlab/import_export/file_importer.rb @@ -28,7 +28,9 @@ module Gitlab copy_archive wait_for_archived_file do - validate_decompressed_archive_size if Feature.enabled?(:validate_import_decompressed_archive_size, default_enabled: true) + # Disable archive validation by default + # See: https://gitlab.com/gitlab-org/gitlab/-/issues/235949 + validate_decompressed_archive_size if Feature.enabled?(:validate_import_decompressed_archive_size) decompress_archive end rescue => e diff --git a/locale/gitlab.pot b/locale/gitlab.pot index eba71cacad1..fa746ee8af8 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -297,6 +297,9 @@ msgstr[1] "" msgid "%{actionText} & %{openOrClose} %{noteable}" msgstr "" +msgid "%{address} is an invalid IP address range" +msgstr "" + msgid "%{author_link} wrote:" msgstr "" @@ -3127,6 +3130,9 @@ msgstr "" msgid "April" msgstr "" +msgid "Architecture not found for OS" +msgstr "" + msgid "Archive" msgstr "" @@ -13201,6 +13207,9 @@ msgstr "" msgid "Invalid Login or password" msgstr "" +msgid "Invalid OS" +msgstr "" + msgid "Invalid URL" msgstr "" @@ -15732,6 +15741,12 @@ msgstr "" msgid "Missing OAuth configuration for GitHub." msgstr "" +msgid "Missing OS" +msgstr "" + +msgid "Missing arch" +msgstr "" + msgid "Missing commit signatures endpoint!" msgstr "" @@ -16454,6 +16469,9 @@ msgstr "" msgid "No required pipeline" msgstr "" +msgid "No runner executable" +msgstr "" + msgid "No runners found" msgstr "" @@ -21486,9 +21504,6 @@ msgstr "" msgid "Security Dashboard" msgstr "" -msgid "Security Dashboard - Settings" -msgstr "" - msgid "Security dashboard" msgstr "" @@ -21498,6 +21513,18 @@ msgstr "" msgid "Security report is out of date. Run %{newPipelineLinkStart}a new pipeline%{newPipelineLinkEnd} for the target branch (%{targetBranchName})" msgstr "" +msgid "SecurityApprovals|License Scanning must be enabled. %{linkStart}More information%{linkEnd}" +msgstr "" + +msgid "SecurityApprovals|One or more of the security scanners must be enabled. %{linkStart}More information%{linkEnd}" +msgstr "" + +msgid "SecurityApprovals|Requires approval for vulnerabilties of Critical, High, or Unknown severity. %{linkStart}More information%{linkEnd}" +msgstr "" + +msgid "SecurityApprovals|Requires license policy rules for licenses of Allowed, or Denied. %{linkStart}More information%{linkEnd}" +msgstr "" + msgid "SecurityConfiguration|An error occurred while creating the merge request." msgstr "" @@ -22657,6 +22684,9 @@ msgstr "" msgid "SnippetsEmptyState|There are no snippets to show." msgstr "" +msgid "Snippets|Add another file %{num}/%{total}" +msgstr "" + msgid "Snippets|Delete file" msgstr "" @@ -22666,6 +22696,9 @@ msgstr "" msgid "Snippets|File" msgstr "" +msgid "Snippets|Files" +msgstr "" + msgid "Snippets|Give your file a name to add code highlighting, e.g. example.rb for Ruby" msgstr "" diff --git a/scripts/sync-stable-branch.sh b/scripts/sync-stable-branch.sh index 59ab52844fb..62538002278 100644 --- a/scripts/sync-stable-branch.sh +++ b/scripts/sync-stable-branch.sh @@ -51,7 +51,8 @@ then echo 'Commit not available, triggering a merge train' fi -curl -X POST \ +curl -f \ + -X POST \ -F token="$MERGE_TRAIN_TRIGGER_TOKEN" \ -F ref=master \ -F "variables[MERGE_FOSS]=1" \ diff --git a/spec/features/issues/filtered_search/recent_searches_spec.rb b/spec/features/issues/filtered_search/recent_searches_spec.rb index 85b7a093536..61c1e35f3c8 100644 --- a/spec/features/issues/filtered_search/recent_searches_spec.rb +++ b/spec/features/issues/filtered_search/recent_searches_spec.rb @@ -4,6 +4,7 @@ require 'spec_helper' RSpec.describe 'Recent searches', :js do include FilteredSearchHelpers + include MobileHelpers let(:project_1) { create(:project, :public) } let(:project_2) { create(:project, :public) } @@ -104,4 +105,24 @@ RSpec.describe 'Recent searches', :js do expect(find('.flash-alert')).to have_text('An error occurred while parsing recent searches') end + + context 'on tablet/mobile screen' do + it 'shows only the history icon in the dropdown' do + resize_screen_sm + visit project_issues_path(project_1) + + expect(find('.filtered-search-history-dropdown-wrapper')).to have_selector('svg', visible: true) + expect(find('.filtered-search-history-dropdown-wrapper')).to have_selector('span', text: 'Recent searches', visible: false) + end + end + + context 'on PC screen' do + it 'shows only the Recent searches text in the dropdown' do + restore_window_size + visit project_issues_path(project_1) + + expect(find('.filtered-search-history-dropdown-wrapper')).to have_selector('svg', visible: false) + expect(find('.filtered-search-history-dropdown-wrapper')).to have_selector('span', text: 'Recent searches', visible: true) + end + end end diff --git a/spec/features/issues/gfm_autocomplete_spec.rb b/spec/features/issues/gfm_autocomplete_spec.rb index 2251d8036c3..3757985f99c 100644 --- a/spec/features/issues/gfm_autocomplete_spec.rb +++ b/spec/features/issues/gfm_autocomplete_spec.rb @@ -504,6 +504,23 @@ RSpec.describe 'GFM autocomplete', :js do expect(page).to have_selector('.tribute-container', visible: true) end + it 'opens autocomplete menu for Issues when field starts with text with item escaping HTML characters' do + issue_xss_title = 'This will execute alert<img src=x onerror=alert(2)<img src=x onerror=alert(1)>' + create(:issue, project: project, title: issue_xss_title) + + page.within '.timeline-content-form' do + find('#note-body').native.send_keys('#') + end + + wait_for_requests + + expect(page).to have_selector('.tribute-container', visible: true) + + page.within '.tribute-container ul' do + expect(page.all('li').first.text).to include(issue_xss_title) + end + end + it 'opens autocomplete menu for Username when field starts with text with item escaping HTML characters' do page.within '.timeline-content-form' do find('#note-body').native.send_keys('@ev') diff --git a/spec/frontend/blob/components/blob_edit_header_spec.js b/spec/frontend/blob/components/blob_edit_header_spec.js index 280d70387d5..c71595a79cf 100644 --- a/spec/frontend/blob/components/blob_edit_header_spec.js +++ b/spec/frontend/blob/components/blob_edit_header_spec.js @@ -56,9 +56,9 @@ describe('Blob Header Editing', () => { }); describe.each` - props | expectedDisabled - ${{ showDelete: true }} | ${true} - ${{ showDelete: true, canDelete: true }} | ${false} + props | expectedDisabled + ${{ showDelete: true }} | ${false} + ${{ showDelete: true, canDelete: false }} | ${true} `('with $props', ({ props, expectedDisabled }) => { beforeEach(() => { createComponent(props); diff --git a/spec/frontend/pipelines/components/dag/dag_spec.js b/spec/frontend/pipelines/components/dag/dag_spec.js index 0416e619724..989f6c17197 100644 --- a/spec/frontend/pipelines/components/dag/dag_spec.js +++ b/spec/frontend/pipelines/components/dag/dag_spec.js @@ -1,8 +1,5 @@ import { mount, shallowMount } from '@vue/test-utils'; -import MockAdapter from 'axios-mock-adapter'; -import waitForPromises from 'helpers/wait_for_promises'; import { GlAlert, GlEmptyState } from '@gitlab/ui'; -import axios from '~/lib/utils/axios_utils'; import Dag from '~/pipelines/components/dag/dag.vue'; import DagGraph from '~/pipelines/components/dag/dag_graph.vue'; import DagAnnotations from '~/pipelines/components/dag/dag_annotations.vue'; @@ -11,13 +8,11 @@ import { ADD_NOTE, REMOVE_NOTE, REPLACE_NOTES, - DEFAULT, PARSE_FAILURE, - LOAD_FAILURE, UNSUPPORTED_DATA, } from '~/pipelines/components/dag//constants'; import { - mockBaseData, + mockParsedGraphQLNodes, tooSmallGraph, unparseableGraph, graphWithoutDependencies, @@ -27,7 +22,6 @@ import { describe('Pipeline DAG graph wrapper', () => { let wrapper; - let mock; const getAlert = () => wrapper.find(GlAlert); const getAllAlerts = () => wrapper.findAll(GlAlert); const getGraph = () => wrapper.find(DagGraph); @@ -35,45 +29,46 @@ describe('Pipeline DAG graph wrapper', () => { const getErrorText = type => wrapper.vm.$options.errorTexts[type]; const getEmptyState = () => wrapper.find(GlEmptyState); - const dataPath = '/root/test/pipelines/90/dag.json'; - - const createComponent = (propsData = {}, method = shallowMount) => { + const createComponent = ({ + graphData = mockParsedGraphQLNodes, + provideOverride = {}, + method = shallowMount, + } = {}) => { if (wrapper?.destroy) { wrapper.destroy(); } wrapper = method(Dag, { - propsData: { + provide: { + pipelineProjectPath: 'root/abc-dag', + pipelineIid: '1', emptySvgPath: '/my-svg', dagDocPath: '/my-doc', - ...propsData, + ...provideOverride, }, data() { return { + graphData, showFailureAlert: false, }; }, }); }; - beforeEach(() => { - mock = new MockAdapter(axios); - }); - afterEach(() => { - mock.restore(); wrapper.destroy(); wrapper = null; }); - describe('when there is no dataUrl', () => { + describe('when a query argument is undefined', () => { beforeEach(() => { - createComponent({ graphUrl: undefined }); + createComponent({ + provideOverride: { pipelineProjectPath: undefined }, + graphData: null, + }); }); - it('shows the DEFAULT alert and not the graph', () => { - expect(getAlert().exists()).toBe(true); - expect(getAlert().text()).toBe(getErrorText(DEFAULT)); + it('does not render the graph', async () => { expect(getGraph().exists()).toBe(false); }); @@ -82,36 +77,12 @@ describe('Pipeline DAG graph wrapper', () => { }); }); - describe('when there is a dataUrl', () => { - describe('but the data fetch fails', () => { + describe('when all query variables are defined', () => { + describe('but the parse fails', () => { beforeEach(async () => { - mock.onGet(dataPath).replyOnce(500); - createComponent({ graphUrl: dataPath }); - - await wrapper.vm.$nextTick(); - - return waitForPromises(); - }); - - it('shows the LOAD_FAILURE alert and not the graph', () => { - expect(getAlert().exists()).toBe(true); - expect(getAlert().text()).toBe(getErrorText(LOAD_FAILURE)); - expect(getGraph().exists()).toBe(false); - }); - - it('does not render the empty state', () => { - expect(getEmptyState().exists()).toBe(false); - }); - }); - - describe('the data fetch succeeds but the parse fails', () => { - beforeEach(async () => { - mock.onGet(dataPath).replyOnce(200, unparseableGraph); - createComponent({ graphUrl: dataPath }); - - await wrapper.vm.$nextTick(); - - return waitForPromises(); + createComponent({ + graphData: unparseableGraph, + }); }); it('shows the PARSE_FAILURE alert and not the graph', () => { @@ -125,14 +96,9 @@ describe('Pipeline DAG graph wrapper', () => { }); }); - describe('and the data fetch and parse succeeds', () => { + describe('parse succeeds', () => { beforeEach(async () => { - mock.onGet(dataPath).replyOnce(200, mockBaseData); - createComponent({ graphUrl: dataPath }, mount); - - await wrapper.vm.$nextTick(); - - return waitForPromises(); + createComponent({ method: mount }); }); it('shows the graph', () => { @@ -144,14 +110,11 @@ describe('Pipeline DAG graph wrapper', () => { }); }); - describe('the data fetch and parse succeeds, but the resulting graph is too small', () => { + describe('parse succeeds, but the resulting graph is too small', () => { beforeEach(async () => { - mock.onGet(dataPath).replyOnce(200, tooSmallGraph); - createComponent({ graphUrl: dataPath }); - - await wrapper.vm.$nextTick(); - - return waitForPromises(); + createComponent({ + graphData: tooSmallGraph, + }); }); it('shows the UNSUPPORTED_DATA alert and not the graph', () => { @@ -165,14 +128,12 @@ describe('Pipeline DAG graph wrapper', () => { }); }); - describe('the data fetch succeeds but the returned data is empty', () => { + describe('the returned data is empty', () => { beforeEach(async () => { - mock.onGet(dataPath).replyOnce(200, graphWithoutDependencies); - createComponent({ graphUrl: dataPath }, mount); - - await wrapper.vm.$nextTick(); - - return waitForPromises(); + createComponent({ + method: mount, + graphData: graphWithoutDependencies, + }); }); it('does not render an error alert or the graph', () => { @@ -188,12 +149,7 @@ describe('Pipeline DAG graph wrapper', () => { describe('annotations', () => { beforeEach(async () => { - mock.onGet(dataPath).replyOnce(200, mockBaseData); - createComponent({ graphUrl: dataPath }, mount); - - await wrapper.vm.$nextTick(); - - return waitForPromises(); + createComponent(); }); it('toggles on link mouseover and mouseout', async () => { diff --git a/spec/frontend/pipelines/components/dag/drawing_utils_spec.js b/spec/frontend/pipelines/components/dag/drawing_utils_spec.js index a50163411ed..37a7d07485b 100644 --- a/spec/frontend/pipelines/components/dag/drawing_utils_spec.js +++ b/spec/frontend/pipelines/components/dag/drawing_utils_spec.js @@ -1,9 +1,9 @@ import { createSankey } from '~/pipelines/components/dag/drawing_utils'; import { parseData } from '~/pipelines/components/dag/parsing_utils'; -import { mockBaseData } from './mock_data'; +import { mockParsedGraphQLNodes } from './mock_data'; describe('DAG visualization drawing utilities', () => { - const parsed = parseData(mockBaseData.stages); + const parsed = parseData(mockParsedGraphQLNodes); const layoutSettings = { width: 200, diff --git a/spec/frontend/pipelines/components/dag/mock_data.js b/spec/frontend/pipelines/components/dag/mock_data.js index 3b39b9cd21c..e7e93804195 100644 --- a/spec/frontend/pipelines/components/dag/mock_data.js +++ b/spec/frontend/pipelines/components/dag/mock_data.js @@ -1,127 +1,56 @@ -/* - It is important that the simple base include parallel jobs - as well as non-parallel jobs with spaces in the name to prevent - us relying on spaces as an indicator. -*/ -export const mockBaseData = { - stages: [ - { - name: 'test', - groups: [ - { - name: 'jest', - size: 2, - jobs: [{ name: 'jest 1/2', needs: ['frontend fixtures'] }, { name: 'jest 2/2' }], - }, - { - name: 'rspec', - size: 1, - jobs: [{ name: 'rspec', needs: ['frontend fixtures'] }], - }, - ], - }, - { - name: 'fixtures', - groups: [ - { - name: 'frontend fixtures', - size: 1, - jobs: [{ name: 'frontend fixtures' }], - }, - ], - }, - { - name: 'un-needed', - groups: [ - { - name: 'un-needed', - size: 1, - jobs: [{ name: 'un-needed' }], - }, - ], - }, - ], -}; - -export const tooSmallGraph = { - stages: [ - { - name: 'test', - groups: [ - { - name: 'jest', - size: 2, - jobs: [{ name: 'jest 1/2' }, { name: 'jest 2/2' }], - }, - { - name: 'rspec', - size: 1, - jobs: [{ name: 'rspec', needs: ['frontend fixtures'] }], - }, - ], - }, - { - name: 'fixtures', - groups: [ - { - name: 'frontend fixtures', - size: 1, - jobs: [{ name: 'frontend fixtures' }], - }, - ], - }, - { - name: 'un-needed', - groups: [ - { - name: 'un-needed', - size: 1, - jobs: [{ name: 'un-needed' }], - }, - ], - }, - ], -}; +export const tooSmallGraph = [ + { + category: 'test', + name: 'jest', + size: 2, + jobs: [{ name: 'jest 1/2' }, { name: 'jest 2/2' }], + }, + { + category: 'test', + name: 'rspec', + size: 1, + jobs: [{ name: 'rspec', needs: ['frontend fixtures'] }], + }, + { + category: 'fixtures', + name: 'frontend fixtures', + size: 1, + jobs: [{ name: 'frontend fixtures' }], + }, + { + category: 'un-needed', + name: 'un-needed', + size: 1, + jobs: [{ name: 'un-needed' }], + }, +]; -export const graphWithoutDependencies = { - stages: [ - { - name: 'test', - groups: [ - { - name: 'jest', - size: 2, - jobs: [{ name: 'jest 1/2' }, { name: 'jest 2/2' }], - }, - { - name: 'rspec', - size: 1, - jobs: [{ name: 'rspec' }], - }, - ], - }, - { - name: 'fixtures', - groups: [ - { - name: 'frontend fixtures', - size: 1, - jobs: [{ name: 'frontend fixtures' }], - }, - ], - }, - { - name: 'un-needed', - groups: [ - { - name: 'un-needed', - size: 1, - jobs: [{ name: 'un-needed' }], - }, - ], - }, - ], -}; +export const graphWithoutDependencies = [ + { + category: 'test', + name: 'jest', + size: 2, + jobs: [{ name: 'jest 1/2' }, { name: 'jest 2/2' }], + }, + { + category: 'test', + name: 'rspec', + size: 1, + jobs: [{ name: 'rspec' }], + }, + { + category: 'fixtures', + name: 'frontend fixtures', + size: 1, + jobs: [{ name: 'frontend fixtures' }], + }, + { + category: 'un-needed', + name: 'un-needed', + size: 1, + jobs: [{ name: 'un-needed' }], + }, +]; export const unparseableGraph = [ { @@ -468,3 +397,264 @@ export const multiNote = { }, }, }; + +/* + It is important that the base include parallel jobs + as well as non-parallel jobs with spaces in the name to prevent + us relying on spaces as an indicator. +*/ + +export const mockParsedGraphQLNodes = [ + { + category: 'build', + name: 'build_a', + size: 1, + jobs: [ + { + name: 'build_a', + needs: [], + }, + ], + __typename: 'CiGroup', + }, + { + category: 'build', + name: 'build_b', + size: 1, + jobs: [ + { + name: 'build_b', + needs: [], + }, + ], + __typename: 'CiGroup', + }, + { + category: 'test', + name: 'test_a', + size: 1, + jobs: [ + { + name: 'test_a', + needs: ['build_a'], + }, + ], + __typename: 'CiGroup', + }, + { + category: 'test', + name: 'test_b', + size: 1, + jobs: [ + { + name: 'test_b', + needs: [], + }, + ], + __typename: 'CiGroup', + }, + { + category: 'test', + name: 'test_c', + size: 1, + jobs: [ + { + name: 'test_c', + needs: [], + }, + ], + __typename: 'CiGroup', + }, + { + category: 'test', + name: 'test_d', + size: 1, + jobs: [ + { + name: 'test_d', + needs: [], + }, + ], + __typename: 'CiGroup', + }, + { + category: 'post-test', + name: 'post_test_a', + size: 1, + jobs: [ + { + name: 'post_test_a', + needs: [], + }, + ], + __typename: 'CiGroup', + }, + { + category: 'post-test', + name: 'post_test_b', + size: 1, + jobs: [ + { + name: 'post_test_b', + needs: [], + }, + ], + __typename: 'CiGroup', + }, + { + category: 'post-test', + name: 'post_test_c', + size: 1, + jobs: [ + { + name: 'post_test_c', + needs: ['test_b', 'test_a'], + }, + ], + __typename: 'CiGroup', + }, + { + category: 'staging', + name: 'staging_a', + size: 1, + jobs: [ + { + name: 'staging_a', + needs: ['post_test_a'], + }, + ], + __typename: 'CiGroup', + }, + { + category: 'staging', + name: 'staging_b', + size: 1, + jobs: [ + { + name: 'staging_b', + needs: ['post_test_b'], + }, + ], + __typename: 'CiGroup', + }, + { + category: 'staging', + name: 'staging_c', + size: 1, + jobs: [ + { + name: 'staging_c', + needs: [], + }, + ], + __typename: 'CiGroup', + }, + { + category: 'staging', + name: 'staging_d', + size: 1, + jobs: [ + { + name: 'staging_d', + needs: [], + }, + ], + __typename: 'CiGroup', + }, + { + category: 'staging', + name: 'staging_e', + size: 1, + jobs: [ + { + name: 'staging_e', + needs: [], + }, + ], + __typename: 'CiGroup', + }, + { + category: 'canary', + name: 'canary_a', + size: 1, + jobs: [ + { + name: 'canary_a', + needs: ['staging_b', 'staging_a'], + }, + ], + __typename: 'CiGroup', + }, + { + category: 'canary', + name: 'canary_b', + size: 1, + jobs: [ + { + name: 'canary_b', + needs: [], + }, + ], + __typename: 'CiGroup', + }, + { + category: 'canary', + name: 'canary_c', + size: 1, + jobs: [ + { + name: 'canary_c', + needs: ['staging_b'], + }, + ], + __typename: 'CiGroup', + }, + { + category: 'production', + name: 'production_a', + size: 1, + jobs: [ + { + name: 'production_a', + needs: ['canary_a'], + }, + ], + __typename: 'CiGroup', + }, + { + category: 'production', + name: 'production_b', + size: 1, + jobs: [ + { + name: 'production_b', + needs: [], + }, + ], + __typename: 'CiGroup', + }, + { + category: 'production', + name: 'production_c', + size: 1, + jobs: [ + { + name: 'production_c', + needs: [], + }, + ], + __typename: 'CiGroup', + }, + { + category: 'production', + name: 'production_d', + size: 1, + jobs: [ + { + name: 'production_d', + needs: ['canary_c'], + }, + ], + __typename: 'CiGroup', + }, +]; diff --git a/spec/frontend/pipelines/components/dag/parsing_utils_spec.js b/spec/frontend/pipelines/components/dag/parsing_utils_spec.js index d9a1296e572..e93fa8e6760 100644 --- a/spec/frontend/pipelines/components/dag/parsing_utils_spec.js +++ b/spec/frontend/pipelines/components/dag/parsing_utils_spec.js @@ -1,5 +1,5 @@ import { - createNodesStructure, + createNodeDict, makeLinksFromNodes, filterByAncestors, parseData, @@ -8,56 +8,17 @@ import { } from '~/pipelines/components/dag/parsing_utils'; import { createSankey } from '~/pipelines/components/dag/drawing_utils'; -import { mockBaseData } from './mock_data'; +import { mockParsedGraphQLNodes } from './mock_data'; describe('DAG visualization parsing utilities', () => { - const { nodes, nodeDict } = createNodesStructure(mockBaseData.stages); - const unfilteredLinks = makeLinksFromNodes(nodes, nodeDict); - const parsed = parseData(mockBaseData.stages); - - const layoutSettings = { - width: 200, - height: 200, - nodeWidth: 10, - nodePadding: 20, - paddingForLabels: 100, - }; - - const sankeyLayout = createSankey(layoutSettings)(parsed); - - describe('createNodesStructure', () => { - const parallelGroupName = 'jest'; - const parallelJobName = 'jest 1/2'; - const singleJobName = 'frontend fixtures'; - - const { name, jobs, size } = mockBaseData.stages[0].groups[0]; - - it('returns the expected node structure', () => { - expect(nodes[0]).toHaveProperty('category', mockBaseData.stages[0].name); - expect(nodes[0]).toHaveProperty('name', name); - expect(nodes[0]).toHaveProperty('jobs', jobs); - expect(nodes[0]).toHaveProperty('size', size); - }); - - it('adds needs to top level of nodeDict entries', () => { - expect(nodeDict[parallelGroupName]).toHaveProperty('needs'); - expect(nodeDict[parallelJobName]).toHaveProperty('needs'); - expect(nodeDict[singleJobName]).toHaveProperty('needs'); - }); - - it('makes entries in nodeDict for jobs and parallel jobs', () => { - const nodeNames = Object.keys(nodeDict); - - expect(nodeNames.includes(parallelGroupName)).toBe(true); - expect(nodeNames.includes(parallelJobName)).toBe(true); - expect(nodeNames.includes(singleJobName)).toBe(true); - }); - }); + const nodeDict = createNodeDict(mockParsedGraphQLNodes); + const unfilteredLinks = makeLinksFromNodes(mockParsedGraphQLNodes, nodeDict); + const parsed = parseData(mockParsedGraphQLNodes); describe('makeLinksFromNodes', () => { it('returns the expected link structure', () => { - expect(unfilteredLinks[0]).toHaveProperty('source', 'frontend fixtures'); - expect(unfilteredLinks[0]).toHaveProperty('target', 'jest'); + expect(unfilteredLinks[0]).toHaveProperty('source', 'build_a'); + expect(unfilteredLinks[0]).toHaveProperty('target', 'test_a'); expect(unfilteredLinks[0]).toHaveProperty('value', 10); }); }); @@ -107,8 +68,22 @@ describe('DAG visualization parsing utilities', () => { describe('removeOrphanNodes', () => { it('removes sankey nodes that have no needs and are not needed', () => { + const layoutSettings = { + width: 200, + height: 200, + nodeWidth: 10, + nodePadding: 20, + paddingForLabels: 100, + }; + + const sankeyLayout = createSankey(layoutSettings)(parsed); const cleanedNodes = removeOrphanNodes(sankeyLayout.nodes); - expect(cleanedNodes).toHaveLength(sankeyLayout.nodes.length - 1); + /* + These lengths are determined by the mock data. + If the data changes, the numbers may also change. + */ + expect(parsed.nodes).toHaveLength(21); + expect(cleanedNodes).toHaveLength(12); }); }); diff --git a/spec/frontend/snippets/components/__snapshots__/snippet_blob_edit_spec.js.snap b/spec/frontend/snippets/components/__snapshots__/snippet_blob_edit_spec.js.snap index a8019320f15..1cf1ee74ddf 100644 --- a/spec/frontend/snippets/components/__snapshots__/snippet_blob_edit_spec.js.snap +++ b/spec/frontend/snippets/components/__snapshots__/snippet_blob_edit_spec.js.snap @@ -1,19 +1,21 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`Snippet Blob Edit component rendering matches the snapshot 1`] = ` +exports[`Snippet Blob Edit component with loaded blob matches snapshot 1`] = ` <div class="file-holder snippet" > <blob-header-edit-stub + candelete="true" data-qa-selector="file_name_field" - id="snippet_file_path" - value="lorem.txt" + id="blob_local_7_file_path" + value="foo/bar/test.md" /> <blob-content-edit-stub - fileglobalid="0a3d" - filename="lorem.txt" - value="Lorem ipsum dolor sit amet, consectetur adipiscing elit." + fileglobalid="blob_local_7" + filename="foo/bar/test.md" + value="Lorem ipsum dolar sit amet, +consectetur adipiscing elit." /> </div> `; diff --git a/spec/frontend/snippets/components/edit_spec.js b/spec/frontend/snippets/components/edit_spec.js index af449025d6a..47a7207a7c3 100644 --- a/spec/frontend/snippets/components/edit_spec.js +++ b/spec/frontend/snippets/components/edit_spec.js @@ -1,133 +1,157 @@ -import { shallowMount } from '@vue/test-utils'; +import { ApolloMutation } from 'vue-apollo'; import { GlLoadingIcon } from '@gitlab/ui'; +import { shallowMount } from '@vue/test-utils'; import waitForPromises from 'helpers/wait_for_promises'; -import { ApolloMutation } from 'vue-apollo'; import Flash from '~/flash'; - -import { redirectTo } from '~/lib/utils/url_utility'; - +import * as urlUtils from '~/lib/utils/url_utility'; import SnippetEditApp from '~/snippets/components/edit.vue'; import SnippetDescriptionEdit from '~/snippets/components/snippet_description_edit.vue'; import SnippetVisibilityEdit from '~/snippets/components/snippet_visibility_edit.vue'; import SnippetBlobActionsEdit from '~/snippets/components/snippet_blob_actions_edit.vue'; import TitleField from '~/vue_shared/components/form/title.vue'; import FormFooterActions from '~/vue_shared/components/form/form_footer_actions.vue'; -import { SNIPPET_CREATE_MUTATION_ERROR, SNIPPET_UPDATE_MUTATION_ERROR } from '~/snippets/constants'; - +import { SNIPPET_VISIBILITY_PRIVATE } from '~/snippets/constants'; import UpdateSnippetMutation from '~/snippets/mutations/updateSnippet.mutation.graphql'; import CreateSnippetMutation from '~/snippets/mutations/createSnippet.mutation.graphql'; - -jest.mock('~/lib/utils/url_utility', () => ({ - redirectTo: jest.fn().mockName('redirectTo'), -})); +import { testEntries } from '../test_utils'; jest.mock('~/flash'); -let flashSpy; - -const rawProjectPathMock = '/project/path'; -const newlyEditedSnippetUrl = 'http://foo.bar'; -const apiError = { message: 'Ufff' }; -const mutationError = 'Bummer'; +const TEST_UPLOADED_FILES = ['foo/bar.txt', 'alpha/beta.js']; +const TEST_API_ERROR = 'Ufff'; +const TEST_MUTATION_ERROR = 'Bummer'; -const attachedFilePath1 = 'foo/bar'; -const attachedFilePath2 = 'alpha/beta'; - -const actionWithContent = { - content: 'Foo Bar', -}; -const actionWithoutContent = { - content: '', -}; - -const defaultProps = { - snippetGid: 'gid://gitlab/PersonalSnippet/42', - markdownPreviewPath: 'http://preview.foo.bar', - markdownDocsPath: 'http://docs.foo.bar', -}; -const defaultData = { - blobsActions: { - ...actionWithContent, - action: '', +const TEST_ACTIONS = { + NO_CONTENT: { + ...testEntries.created.diff, + content: '', + }, + NO_PATH: { + ...testEntries.created.diff, + filePath: '', + }, + VALID: { + ...testEntries.created.diff, }, }; +const TEST_WEB_URL = '/snippets/7'; + +const createTestSnippet = () => ({ + webUrl: TEST_WEB_URL, + id: 7, + title: 'Snippet Title', + description: 'Lorem ipsum snippet desc', + visibilityLevel: SNIPPET_VISIBILITY_PRIVATE, +}); + describe('Snippet Edit app', () => { let wrapper; - const resolveMutate = jest.fn().mockResolvedValue({ - data: { - updateSnippet: { - errors: [], - snippet: { - webUrl: newlyEditedSnippetUrl, + const mutationTypes = { + RESOLVE: jest.fn().mockResolvedValue({ + data: { + updateSnippet: { + errors: [], + snippet: createTestSnippet(), }, }, - }, - }); - - const resolveMutateWithErrors = jest.fn().mockResolvedValue({ - data: { - updateSnippet: { - errors: [mutationError], - snippet: { - webUrl: newlyEditedSnippetUrl, + }), + RESOLVE_WITH_ERRORS: jest.fn().mockResolvedValue({ + data: { + updateSnippet: { + errors: [TEST_MUTATION_ERROR], + snippet: createTestSnippet(), + }, + createSnippet: { + errors: [TEST_MUTATION_ERROR], + snippet: null, }, }, - createSnippet: { - errors: [mutationError], - snippet: null, - }, - }, - }); - - const rejectMutation = jest.fn().mockRejectedValue(apiError); - - const mutationTypes = { - RESOLVE: resolveMutate, - RESOLVE_WITH_ERRORS: resolveMutateWithErrors, - REJECT: rejectMutation, + }), + REJECT: jest.fn().mockRejectedValue(TEST_API_ERROR), }; function createComponent({ - props = defaultProps, - data = {}, + props = {}, loading = false, mutationRes = mutationTypes.RESOLVE, } = {}) { - const $apollo = { - queries: { - snippet: { - loading, - }, - }, - mutate: mutationRes, - }; + if (wrapper) { + throw new Error('wrapper already exists'); + } wrapper = shallowMount(SnippetEditApp, { - mocks: { $apollo }, + mocks: { + $apollo: { + queries: { + snippet: { loading }, + }, + mutate: mutationRes, + }, + }, stubs: { - FormFooterActions, ApolloMutation, + FormFooterActions, }, propsData: { + snippetGid: 'gid://gitlab/PersonalSnippet/42', + markdownPreviewPath: 'http://preview.foo.bar', + markdownDocsPath: 'http://docs.foo.bar', ...props, }, - data() { - return data; - }, }); - - flashSpy = jest.spyOn(wrapper.vm, 'flashAPIFailure'); } + beforeEach(() => { + jest.spyOn(urlUtils, 'redirectTo').mockImplementation(); + }); + afterEach(() => { wrapper.destroy(); + wrapper = null; }); + const findBlobActions = () => wrapper.find(SnippetBlobActionsEdit); const findSubmitButton = () => wrapper.find('[data-testid="snippet-submit-btn"]'); - const findCancellButton = () => wrapper.find('[data-testid="snippet-cancel-btn"]'); + const findCancelButton = () => wrapper.find('[data-testid="snippet-cancel-btn"]'); + const hasDisabledSubmit = () => Boolean(findSubmitButton().attributes('disabled')); + const clickSubmitBtn = () => wrapper.find('[data-testid="snippet-edit-form"]').trigger('submit'); + const triggerBlobActions = actions => findBlobActions().vm.$emit('actions', actions); + const setUploadFilesHtml = paths => { + wrapper.vm.$el.innerHTML = paths.map(path => `<input name="files[]" value="${path}">`).join(''); + }; + const getApiData = ({ + id, + title = '', + description = '', + visibilityLevel = SNIPPET_VISIBILITY_PRIVATE, + } = {}) => ({ + id, + title, + description, + visibilityLevel, + blobActions: [], + }); + + // Ideally we wouldn't call this method directly, but we don't have a way to trigger + // apollo responses yet. + const loadSnippet = (...edges) => { + if (edges.length) { + wrapper.setData({ + snippet: edges[0], + }); + } + + wrapper.vm.onSnippetFetch({ + data: { + snippets: { + edges, + }, + }, + }); + }; describe('rendering', () => { it('renders loader while the query is in flight', () => { @@ -135,302 +159,163 @@ describe('Snippet Edit app', () => { expect(wrapper.find(GlLoadingIcon).exists()).toBe(true); }); - it('renders all required components', () => { - createComponent(); - - expect(wrapper.contains(TitleField)).toBe(true); - expect(wrapper.contains(SnippetDescriptionEdit)).toBe(true); - expect(wrapper.contains(SnippetBlobActionsEdit)).toBe(true); - expect(wrapper.contains(SnippetVisibilityEdit)).toBe(true); - expect(wrapper.contains(FormFooterActions)).toBe(true); - }); - - it('does not fail if there is no snippet yet (new snippet creation)', () => { - const snippetGid = ''; - createComponent({ - props: { - ...defaultProps, - snippetGid, - }, - }); - - expect(wrapper.props('snippetGid')).toBe(snippetGid); - }); + it.each([[{}], [{ snippetGid: '' }]])( + 'should render all required components with %s', + props => { + createComponent(props); - it.each` - title | blobsActions | expectation - ${''} | ${{}} | ${true} - ${''} | ${{ actionWithContent }} | ${true} - ${''} | ${{ actionWithoutContent }} | ${true} - ${'foo'} | ${{}} | ${true} - ${'foo'} | ${{ actionWithoutContent }} | ${true} - ${'foo'} | ${{ actionWithoutContent, actionWithContent }} | ${true} - ${'foo'} | ${{ actionWithContent }} | ${false} - `( - 'disables submit button unless both title and content for all blobs are present', - ({ title, blobsActions, expectation }) => { - createComponent({ - data: { - snippet: { title }, - blobsActions, - }, - }); - const isBtnDisabled = Boolean(findSubmitButton().attributes('disabled')); - expect(isBtnDisabled).toBe(expectation); + expect(wrapper.contains(TitleField)).toBe(true); + expect(wrapper.contains(SnippetDescriptionEdit)).toBe(true); + expect(wrapper.contains(SnippetVisibilityEdit)).toBe(true); + expect(wrapper.contains(FormFooterActions)).toBe(true); + expect(findBlobActions().exists()).toBe(true); }, ); it.each` - isNew | status | expectation - ${true} | ${`new`} | ${`/-/snippets`} - ${false} | ${`existing`} | ${newlyEditedSnippetUrl} - `('sets correct href for the cancel button on a $status snippet', ({ isNew, expectation }) => { - createComponent({ - data: { - snippet: { webUrl: newlyEditedSnippetUrl }, - newSnippet: isNew, - }, - }); + title | actions | shouldDisable + ${''} | ${[]} | ${true} + ${''} | ${[TEST_ACTIONS.VALID]} | ${true} + ${'foo'} | ${[]} | ${false} + ${'foo'} | ${[TEST_ACTIONS.VALID]} | ${false} + ${'foo'} | ${[TEST_ACTIONS.VALID, TEST_ACTIONS.NO_CONTENT]} | ${true} + ${'foo'} | ${[TEST_ACTIONS.VALID, TEST_ACTIONS.NO_PATH]} | ${true} + `( + 'should handle submit disable (title=$title, actions=$actions, shouldDisable=$shouldDisable)', + async ({ title, actions, shouldDisable }) => { + createComponent(); - expect(findCancellButton().attributes('href')).toBe(expectation); - }); - }); + loadSnippet({ title }); + triggerBlobActions(actions); - describe('functionality', () => { - describe('form submission handling', () => { - it('does not submit unchanged blobs', () => { - const foo = { - action: '', - }; - const bar = { - action: 'update', - }; - createComponent({ - data: { - blobsActions: { - foo, - bar, - }, - }, - }); - clickSubmitBtn(); + await wrapper.vm.$nextTick(); - return waitForPromises().then(() => { - expect(resolveMutate).toHaveBeenCalledWith( - expect.objectContaining({ variables: { input: { blobActions: [bar] } } }), - ); - }); - }); + expect(hasDisabledSubmit()).toBe(shouldDisable); + }, + ); - it.each` - newSnippet | projectPath | mutation | mutationName - ${true} | ${rawProjectPathMock} | ${CreateSnippetMutation} | ${'CreateSnippetMutation with projectPath'} - ${true} | ${''} | ${CreateSnippetMutation} | ${'CreateSnippetMutation without projectPath'} - ${false} | ${rawProjectPathMock} | ${UpdateSnippetMutation} | ${'UpdateSnippetMutation with projectPath'} - ${false} | ${''} | ${UpdateSnippetMutation} | ${'UpdateSnippetMutation without projectPath'} - `('should submit $mutationName correctly', ({ newSnippet, projectPath, mutation }) => { + it.each` + projectPath | snippetArg | expectation + ${''} | ${[]} | ${'/-/snippets'} + ${'project/path'} | ${[]} | ${'/project/path/-/snippets'} + ${''} | ${[createTestSnippet()]} | ${TEST_WEB_URL} + ${'project/path'} | ${[createTestSnippet()]} | ${TEST_WEB_URL} + `( + 'should set cancel href when (projectPath=$projectPath, snippet=$snippetArg)', + async ({ projectPath, snippetArg, expectation }) => { createComponent({ - data: { - newSnippet, - ...defaultData, - }, - props: { - ...defaultProps, - projectPath, - }, + props: { projectPath }, }); - const mutationPayload = { - mutation, - variables: { - input: newSnippet ? expect.objectContaining({ projectPath }) : expect.any(Object), - }, - }; - - clickSubmitBtn(); + loadSnippet(...snippetArg); - expect(resolveMutate).toHaveBeenCalledWith(mutationPayload); - }); - - it('redirects to snippet view on successful mutation', () => { - createComponent(); - clickSubmitBtn(); + await wrapper.vm.$nextTick(); - return waitForPromises().then(() => { - expect(redirectTo).toHaveBeenCalledWith(newlyEditedSnippetUrl); - }); - }); + expect(findCancelButton().attributes('href')).toBe(expectation); + }, + ); + }); + describe('functionality', () => { + describe('form submission handling', () => { it.each` - newSnippet | projectPath | mutationName - ${true} | ${rawProjectPathMock} | ${'CreateSnippetMutation with projectPath'} - ${true} | ${''} | ${'CreateSnippetMutation without projectPath'} - ${false} | ${rawProjectPathMock} | ${'UpdateSnippetMutation with projectPath'} - ${false} | ${''} | ${'UpdateSnippetMutation without projectPath'} + snippetArg | projectPath | uploadedFiles | input | mutation + ${[]} | ${'project/path'} | ${[]} | ${{ ...getApiData(), projectPath: 'project/path', uploadedFiles: [] }} | ${CreateSnippetMutation} + ${[]} | ${''} | ${[]} | ${{ ...getApiData(), projectPath: '', uploadedFiles: [] }} | ${CreateSnippetMutation} + ${[]} | ${''} | ${TEST_UPLOADED_FILES} | ${{ ...getApiData(), projectPath: '', uploadedFiles: TEST_UPLOADED_FILES }} | ${CreateSnippetMutation} + ${[createTestSnippet()]} | ${'project/path'} | ${[]} | ${getApiData(createTestSnippet())} | ${UpdateSnippetMutation} + ${[createTestSnippet()]} | ${''} | ${[]} | ${getApiData(createTestSnippet())} | ${UpdateSnippetMutation} `( - 'does not redirect to snippet view if the seemingly successful' + - ' $mutationName response contains errors', - ({ newSnippet, projectPath }) => { + 'should submit mutation with (snippet=$snippetArg, projectPath=$projectPath, uploadedFiles=$uploadedFiles)', + async ({ snippetArg, projectPath, uploadedFiles, mutation, input }) => { createComponent({ - data: { - newSnippet, - }, props: { - ...defaultProps, projectPath, }, - mutationRes: mutationTypes.RESOLVE_WITH_ERRORS, }); + loadSnippet(...snippetArg); + setUploadFilesHtml(uploadedFiles); + + await wrapper.vm.$nextTick(); clickSubmitBtn(); - return waitForPromises().then(() => { - expect(redirectTo).not.toHaveBeenCalled(); - expect(flashSpy).toHaveBeenCalledWith(mutationError); + expect(mutationTypes.RESOLVE).toHaveBeenCalledWith({ + mutation, + variables: { + input, + }, }); }, ); - it('flashes an error if mutation failed', () => { - createComponent({ - mutationRes: mutationTypes.REJECT, - }); + it('should redirect to snippet view on successful mutation', async () => { + createComponent(); + loadSnippet(createTestSnippet()); clickSubmitBtn(); - return waitForPromises().then(() => { - expect(redirectTo).not.toHaveBeenCalled(); - expect(flashSpy).toHaveBeenCalledWith(apiError); - }); + await waitForPromises(); + + expect(urlUtils.redirectTo).toHaveBeenCalledWith(TEST_WEB_URL); }); it.each` - isNew | status | expectation - ${true} | ${`new`} | ${SNIPPET_CREATE_MUTATION_ERROR.replace('%{err}', '')} - ${false} | ${`existing`} | ${SNIPPET_UPDATE_MUTATION_ERROR.replace('%{err}', '')} + snippetArg | projectPath | mutationRes | expectMessage + ${[]} | ${'project/path'} | ${mutationTypes.RESOLVE_WITH_ERRORS} | ${`Can't create snippet: ${TEST_MUTATION_ERROR}`} + ${[]} | ${''} | ${mutationTypes.RESOLVE_WITH_ERRORS} | ${`Can't create snippet: ${TEST_MUTATION_ERROR}`} + ${[]} | ${''} | ${mutationTypes.REJECT} | ${`Can't create snippet: ${TEST_API_ERROR}`} + ${[createTestSnippet()]} | ${'project/path'} | ${mutationTypes.RESOLVE_WITH_ERRORS} | ${`Can't update snippet: ${TEST_MUTATION_ERROR}`} + ${[createTestSnippet()]} | ${''} | ${mutationTypes.RESOLVE_WITH_ERRORS} | ${`Can't update snippet: ${TEST_MUTATION_ERROR}`} `( - `renders the correct error message if mutation fails for $status snippet`, - ({ isNew, expectation }) => { + 'should flash error with (snippet=$snippetArg, projectPath=$projectPath)', + async ({ snippetArg, projectPath, mutationRes, expectMessage }) => { createComponent({ - data: { - newSnippet: isNew, + props: { + projectPath, }, - mutationRes: mutationTypes.REJECT, + mutationRes, }); + loadSnippet(...snippetArg); clickSubmitBtn(); - return waitForPromises().then(() => { - expect(Flash).toHaveBeenCalledWith(expect.stringContaining(expectation)); - }); + await waitForPromises(); + + expect(urlUtils.redirectTo).not.toHaveBeenCalled(); + expect(Flash).toHaveBeenCalledWith(expectMessage); }, ); }); - describe('correctly includes attached files into the mutation', () => { - const createMutationPayload = expectation => { - return expect.objectContaining({ - variables: { - input: expect.objectContaining({ uploadedFiles: expectation }), - }, - }); - }; - - const updateMutationPayload = () => { - return expect.objectContaining({ - variables: { - input: expect.not.objectContaining({ uploadedFiles: expect.anything() }), - }, - }); - }; - + describe('on before unload', () => { it.each` - paths | expectation - ${[attachedFilePath1]} | ${[attachedFilePath1]} - ${[attachedFilePath1, attachedFilePath2]} | ${[attachedFilePath1, attachedFilePath2]} - ${[]} | ${[]} - `(`correctly sends paths for $paths.length files`, ({ paths, expectation }) => { - createComponent({ - data: { - newSnippet: true, - }, - }); - - const fixtures = paths.map(path => { - return path ? `<input name="files[]" value="${path}">` : undefined; - }); - wrapper.vm.$el.innerHTML += fixtures.join(''); - - clickSubmitBtn(); - - expect(resolveMutate).toHaveBeenCalledWith(createMutationPayload(expectation)); - }); - - it(`neither fails nor sends 'uploadedFiles' to update mutation`, () => { - createComponent(); - - clickSubmitBtn(); - expect(resolveMutate).toHaveBeenCalledWith(updateMutationPayload()); - }); - }); + condition | expectPrevented | action + ${'there are no actions'} | ${false} | ${() => triggerBlobActions([])} + ${'there are actions'} | ${true} | ${() => triggerBlobActions([testEntries.updated.diff])} + ${'the snippet is being saved'} | ${false} | ${() => clickSubmitBtn()} + `( + 'handles before unload prevent when $condition (expectPrevented=$expectPrevented)', + ({ expectPrevented, action }) => { + createComponent(); + loadSnippet(); - describe('on before unload', () => { - let event; - let returnValueSetter; + action(); - const bootstrap = data => { - createComponent({ - data, - }); + const event = new Event('beforeunload'); + const returnValueSetter = jest.spyOn(event, 'returnValue', 'set'); - event = new Event('beforeunload'); - returnValueSetter = jest.spyOn(event, 'returnValue', 'set'); - }; + window.dispatchEvent(event); - const actionsWithoutAction = { - blobsActions: { - foo: { - ...actionWithContent, - action: '', - }, - }, - }; - const actionsWithUpdate = { - blobsActions: { - foo: { - ...actionWithContent, - action: 'update', - }, - }, - }; - const actionsWithUpdateWhileSaving = { - blobsActions: { - foo: { - ...actionWithContent, - action: 'update', - }, + if (expectPrevented) { + expect(returnValueSetter).toHaveBeenCalledWith( + 'Are you sure you want to lose unsaved changes?', + ); + } else { + expect(returnValueSetter).not.toHaveBeenCalled(); + } }, - isUpdating: true, - }; - - it.each` - bool | expectToBePrevented | data | condition - ${'does not prevent'} | ${false} | ${undefined} | ${'there are no blobs'} - ${'does not prevent'} | ${false} | ${actionsWithoutAction} | ${'there are no changes to the blobs content'} - ${'prevents'} | ${true} | ${actionsWithUpdate} | ${'there are changes to the blobs content'} - ${'does not prevent'} | ${false} | ${actionsWithUpdateWhileSaving} | ${'the snippet is being saved'} - `('$bool page navigation if $condition', ({ expectToBePrevented, data }) => { - bootstrap(data); - window.dispatchEvent(event); - - if (expectToBePrevented) { - expect(returnValueSetter).toHaveBeenCalledWith( - 'Are you sure you want to lose unsaved changes?', - ); - } else { - expect(returnValueSetter).not.toHaveBeenCalled(); - } - }); + ); }); }); }); diff --git a/spec/frontend/snippets/components/snippet_blob_actions_edit_spec.js b/spec/frontend/snippets/components/snippet_blob_actions_edit_spec.js index a7b4f509e50..8b2051008d7 100644 --- a/spec/frontend/snippets/components/snippet_blob_actions_edit_spec.js +++ b/spec/frontend/snippets/components/snippet_blob_actions_edit_spec.js @@ -1,34 +1,68 @@ +import { times } from 'lodash'; import { shallowMount } from '@vue/test-utils'; import SnippetBlobActionsEdit from '~/snippets/components/snippet_blob_actions_edit.vue'; import SnippetBlobEdit from '~/snippets/components/snippet_blob_edit.vue'; +import { + SNIPPET_MAX_BLOBS, + SNIPPET_BLOB_ACTION_CREATE, + SNIPPET_BLOB_ACTION_MOVE, +} from '~/snippets/constants'; +import { testEntries, createBlobFromTestEntry } from '../test_utils'; const TEST_BLOBS = [ - { name: 'foo', content: 'abc', rawPath: 'test/raw' }, - { name: 'bar', content: 'def', rawPath: 'test/raw' }, + createBlobFromTestEntry(testEntries.updated), + createBlobFromTestEntry(testEntries.deleted), ]; -const TEST_EVENT = 'blob-update'; + +const TEST_BLOBS_UNLOADED = TEST_BLOBS.map(blob => ({ ...blob, content: '', isLoaded: false })); describe('snippets/components/snippet_blob_actions_edit', () => { - let onEvent; let wrapper; - const createComponent = (props = {}) => { + const createComponent = (props = {}, snippetMultipleFiles = true) => { wrapper = shallowMount(SnippetBlobActionsEdit, { propsData: { - blobs: [], + initBlobs: TEST_BLOBS, ...props, }, - listeners: { - [TEST_EVENT]: onEvent, + provide: { + glFeatures: { + snippetMultipleFiles, + }, }, }); }; - const findBlobEdit = () => wrapper.find(SnippetBlobEdit); - const findBlobEditData = () => wrapper.findAll(SnippetBlobEdit).wrappers.map(x => x.props()); - beforeEach(() => { - onEvent = jest.fn(); - }); + const findLabel = () => wrapper.find('label'); + const findBlobEdits = () => wrapper.findAll(SnippetBlobEdit); + const findBlobsData = () => + findBlobEdits().wrappers.map(x => ({ + blob: x.props('blob'), + classes: x.classes(), + })); + const findFirstBlobEdit = () => findBlobEdits().at(0); + const findAddButton = () => wrapper.find('[data-testid="add_button"]'); + const getLastActions = () => { + const events = wrapper.emitted().actions; + + return events[events.length - 1]?.[0]; + }; + const buildBlobsDataExpectation = blobs => + blobs.map((blob, index) => ({ + blob: { + ...blob, + id: expect.stringMatching('blob_local_'), + }, + classes: index > 0 ? ['gl-mt-3'] : [], + })); + const triggerBlobDelete = idx => + findBlobEdits() + .at(idx) + .vm.$emit('delete'); + const triggerBlobUpdate = (idx, props) => + findBlobEdits() + .at(idx) + .vm.$emit('blob-updated', props); afterEach(() => { wrapper.destroy(); @@ -36,24 +70,232 @@ describe('snippets/components/snippet_blob_actions_edit', () => { }); describe.each` - props | expectedData - ${{}} | ${[{ blob: null }]} - ${{ blobs: TEST_BLOBS }} | ${TEST_BLOBS.map(blob => ({ blob }))} - `('with $props', ({ props, expectedData }) => { + featureFlag | label | showDelete | showAdd + ${true} | ${'Files'} | ${true} | ${true} + ${false} | ${'File'} | ${false} | ${false} + `('with feature flag = $featureFlag', ({ featureFlag, label, showDelete, showAdd }) => { beforeEach(() => { - createComponent(props); + createComponent({}, featureFlag); + }); + + it('renders label', () => { + expect(findLabel().text()).toBe(label); + }); + + it(`renders delete button (show=${showDelete})`, () => { + expect(findFirstBlobEdit().props()).toMatchObject({ + showDelete, + canDelete: true, + }); + }); + + it(`renders add button (show=${showAdd})`, () => { + expect(findAddButton().exists()).toBe(showAdd); + }); + }); + + describe('with default', () => { + beforeEach(() => { + createComponent(); + }); + + it('emits no actions', () => { + expect(getLastActions()).toEqual([]); }); - it('renders blob edit', () => { - expect(findBlobEditData()).toEqual(expectedData); + it('shows blobs', () => { + expect(findBlobsData()).toEqual(buildBlobsDataExpectation(TEST_BLOBS_UNLOADED)); }); - it('emits event', () => { - expect(onEvent).not.toHaveBeenCalled(); + it('shows add button', () => { + const button = findAddButton(); + + expect(button.text()).toBe(`Add another file ${TEST_BLOBS.length}/${SNIPPET_MAX_BLOBS}`); + expect(button.props('disabled')).toBe(false); + }); + + describe('when add is clicked', () => { + beforeEach(() => { + findAddButton().vm.$emit('click'); + }); + + it('adds blob with empty content', () => { + expect(findBlobsData()).toEqual( + buildBlobsDataExpectation([ + ...TEST_BLOBS_UNLOADED, + { + content: '', + isLoaded: true, + path: '', + }, + ]), + ); + }); + + it('emits action', () => { + expect(getLastActions()).toEqual([ + expect.objectContaining({ + action: SNIPPET_BLOB_ACTION_CREATE, + }), + ]); + }); + }); + + describe('when blob is deleted', () => { + beforeEach(() => { + triggerBlobDelete(1); + }); + + it('removes blob', () => { + expect(findBlobsData()).toEqual(buildBlobsDataExpectation(TEST_BLOBS_UNLOADED.slice(0, 1))); + }); + + it('emits action', () => { + expect(getLastActions()).toEqual([ + expect.objectContaining({ + ...testEntries.deleted.diff, + content: '', + }), + ]); + }); + }); + + describe('when blob changes path', () => { + beforeEach(() => { + triggerBlobUpdate(0, { path: 'new/path' }); + }); + + it('renames blob', () => { + expect(findBlobsData()[0]).toMatchObject({ + blob: { + path: 'new/path', + }, + }); + }); + + it('emits action', () => { + expect(getLastActions()).toMatchObject([ + { + action: SNIPPET_BLOB_ACTION_MOVE, + filePath: 'new/path', + previousPath: testEntries.updated.diff.filePath, + }, + ]); + }); + }); + + describe('when blob emits new content', () => { + const { content } = testEntries.updated.diff; + const originalContent = `${content}\noriginal content\n`; + + beforeEach(() => { + triggerBlobUpdate(0, { content: originalContent }); + }); - findBlobEdit().vm.$emit('blob-update', TEST_BLOBS[0]); + it('loads new content', () => { + expect(findBlobsData()[0]).toMatchObject({ + blob: { + content: originalContent, + isLoaded: true, + }, + }); + }); + + it('does not emit an action', () => { + expect(getLastActions()).toEqual([]); + }); + + it('emits an action when content changes again', async () => { + triggerBlobUpdate(0, { content }); + + await wrapper.vm.$nextTick(); + + expect(getLastActions()).toEqual([testEntries.updated.diff]); + }); + }); + }); + + describe('with 1 blob', () => { + beforeEach(() => { + createComponent({ initBlobs: [createBlobFromTestEntry(testEntries.created)] }); + }); + + it('disables delete button', () => { + expect(findBlobEdits()).toHaveLength(1); + expect( + findBlobEdits() + .at(0) + .props(), + ).toMatchObject({ + showDelete: true, + canDelete: false, + }); + }); + + describe(`when added ${SNIPPET_MAX_BLOBS} files`, () => { + let addButton; + + beforeEach(() => { + addButton = findAddButton(); + + times(SNIPPET_MAX_BLOBS - 1, () => addButton.vm.$emit('click')); + }); + + it('should have blobs', () => { + expect(findBlobsData()).toHaveLength(SNIPPET_MAX_BLOBS); + }); + + it('should disable add button', () => { + expect(addButton.props('disabled')).toBe(true); + }); + }); + }); + + describe('with 0 init blob', () => { + beforeEach(() => { + createComponent({ initBlobs: [] }); + }); + + it('shows 1 blob by default', () => { + expect(findBlobsData()).toEqual([ + expect.objectContaining({ + blob: { + id: expect.stringMatching('blob_local_'), + content: '', + path: '', + isLoaded: true, + }, + }), + ]); + }); + + it('emits create action', () => { + expect(getLastActions()).toEqual([ + { + action: SNIPPET_BLOB_ACTION_CREATE, + content: '', + filePath: '', + previousPath: '', + }, + ]); + }); + }); + + describe(`with ${SNIPPET_MAX_BLOBS} files`, () => { + beforeEach(() => { + const initBlobs = Array(SNIPPET_MAX_BLOBS) + .fill(1) + .map(() => createBlobFromTestEntry(testEntries.created)); + + createComponent({ initBlobs }); + }); + + it('should have blobs', () => { + expect(findBlobsData()).toHaveLength(SNIPPET_MAX_BLOBS); + }); - expect(onEvent).toHaveBeenCalledWith(TEST_BLOBS[0]); + it('should disable add button', () => { + expect(findAddButton().props('disabled')).toBe(true); }); }); }); diff --git a/spec/frontend/snippets/components/snippet_blob_edit_spec.js b/spec/frontend/snippets/components/snippet_blob_edit_spec.js index 683dfd35cb1..29497012ddf 100644 --- a/spec/frontend/snippets/components/snippet_blob_edit_spec.js +++ b/spec/frontend/snippets/components/snippet_blob_edit_spec.js @@ -1,6 +1,5 @@ import { GlLoadingIcon } from '@gitlab/ui'; import { shallowMount } from '@vue/test-utils'; -import { nextTick } from 'vue'; import AxiosMockAdapter from 'axios-mock-adapter'; import waitForPromises from 'helpers/wait_for_promises'; import SnippetBlobEdit from '~/snippets/components/snippet_blob_edit.vue'; @@ -8,166 +7,162 @@ import BlobHeaderEdit from '~/blob/components/blob_edit_header.vue'; import BlobContentEdit from '~/blob/components/blob_edit_content.vue'; import axios from '~/lib/utils/axios_utils'; import { joinPaths } from '~/lib/utils/url_utility'; - -jest.mock('~/blob/utils', () => jest.fn()); - -jest.mock('~/lib/utils/url_utility', () => ({ - getBaseURL: jest.fn().mockReturnValue('foo/'), - joinPaths: jest - .fn() - .mockName('joinPaths') - .mockReturnValue('contentApiURL'), -})); +import createFlash from '~/flash'; +import { TEST_HOST } from 'helpers/test_constants'; jest.mock('~/flash'); -let flashSpy; +const TEST_ID = 'blob_local_7'; +const TEST_PATH = 'foo/bar/test.md'; +const TEST_RAW_PATH = '/gitlab/raw/path/to/blob/7'; +const TEST_FULL_PATH = joinPaths(TEST_HOST, TEST_RAW_PATH); +const TEST_CONTENT = 'Lorem ipsum dolar sit amet,\nconsectetur adipiscing elit.'; + +const TEST_BLOB = { + id: TEST_ID, + rawPath: TEST_RAW_PATH, + path: TEST_PATH, + content: '', + isLoaded: false, +}; + +const TEST_BLOB_LOADED = { + ...TEST_BLOB, + content: TEST_CONTENT, + isLoaded: true, +}; describe('Snippet Blob Edit component', () => { let wrapper; let axiosMock; - const contentMock = 'Lorem ipsum dolor sit amet, consectetur adipiscing elit.'; - const pathMock = 'lorem.txt'; - const rawPathMock = 'foo/bar'; - const blob = { - path: pathMock, - content: contentMock, - rawPath: rawPathMock, - }; - const findComponent = component => wrapper.find(component); - function createComponent(props = {}, data = { isContentLoading: false }) { + const createComponent = (props = {}) => { wrapper = shallowMount(SnippetBlobEdit, { propsData: { + blob: TEST_BLOB, ...props, }, - data() { - return { - ...data, - }; - }, }); - flashSpy = jest.spyOn(wrapper.vm, 'flashAPIFailure'); - } + }; - beforeEach(() => { - // This component generates a random id. Soon this will be abstracted away, but for now let's make this deterministic. - // see https://gitlab.com/gitlab-org/gitlab/-/merge_requests/38855 - jest.spyOn(Math, 'random').mockReturnValue(0.04); + const findLoadingIcon = () => wrapper.find(GlLoadingIcon); + const findHeader = () => wrapper.find(BlobHeaderEdit); + const findContent = () => wrapper.find(BlobContentEdit); + const getLastUpdatedArgs = () => { + const event = wrapper.emitted()['blob-updated']; + + return event?.[event.length - 1][0]; + }; + beforeEach(() => { axiosMock = new AxiosMockAdapter(axios); - createComponent(); + axiosMock.onGet(TEST_FULL_PATH).reply(200, TEST_CONTENT); }); afterEach(() => { - axiosMock.restore(); wrapper.destroy(); + wrapper = null; + axiosMock.restore(); }); - describe('rendering', () => { - it('matches the snapshot', () => { - createComponent({ blob }); - expect(wrapper.element).toMatchSnapshot(); + describe('with not loaded blob', () => { + beforeEach(async () => { + createComponent(); }); - it('renders required components', () => { - expect(findComponent(BlobHeaderEdit).exists()).toBe(true); - expect(findComponent(BlobContentEdit).props()).toEqual({ - fileGlobalId: expect.any(String), - fileName: '', - value: '', + it('shows blob header', () => { + expect(findHeader().props()).toMatchObject({ + value: TEST_BLOB.path, }); + expect(findHeader().attributes('id')).toBe(`${TEST_ID}_file_path`); }); - it('renders loader if existing blob is supplied but no content is fetched yet', () => { - createComponent({ blob }, { isContentLoading: true }); - expect(wrapper.contains(GlLoadingIcon)).toBe(true); - expect(findComponent(BlobContentEdit).exists()).toBe(false); + it('emits delete when deleted', () => { + expect(wrapper.emitted().delete).toBeUndefined(); + + findHeader().vm.$emit('delete'); + + expect(wrapper.emitted().delete).toHaveLength(1); }); - it('does not render loader if when blob is not supplied', () => { - createComponent(); - expect(wrapper.contains(GlLoadingIcon)).toBe(false); - expect(findComponent(BlobContentEdit).exists()).toBe(true); + it('emits update when path changes', () => { + const newPath = 'new/path.md'; + + findHeader().vm.$emit('input', newPath); + + expect(getLastUpdatedArgs()).toEqual({ path: newPath }); }); - }); - describe('functionality', () => { - it('does not fail without blob', () => { - const spy = jest.spyOn(global.console, 'error'); - createComponent({ blob: undefined }); + it('emits update when content is loaded', async () => { + await waitForPromises(); - expect(spy).not.toHaveBeenCalled(); - expect(findComponent(BlobContentEdit).exists()).toBe(true); + expect(getLastUpdatedArgs()).toEqual({ content: TEST_CONTENT }); }); + }); - it.each` - emitter | prop - ${BlobHeaderEdit} | ${'filePath'} - ${BlobContentEdit} | ${'content'} - `('emits "blob-updated" event when the $prop gets changed', ({ emitter, prop }) => { - expect(wrapper.emitted('blob-updated')).toBeUndefined(); - const newValue = 'foo.bar'; - findComponent(emitter).vm.$emit('input', newValue); - - return nextTick().then(() => { - expect(wrapper.emitted('blob-updated')[0]).toEqual([ - expect.objectContaining({ - [prop]: newValue, - }), - ]); - }); + describe('with error', () => { + beforeEach(() => { + axiosMock.reset(); + axiosMock.onGet(TEST_FULL_PATH).replyOnce(500); + createComponent(); }); - describe('fetching blob content', () => { - const bootstrapForExistingSnippet = resp => { - createComponent({ - blob: { - ...blob, - content: '', - }, - }); + it('should call flash', async () => { + await waitForPromises(); - if (resp === 500) { - axiosMock.onGet('contentApiURL').reply(500); - } else { - axiosMock.onGet('contentApiURL').reply(200, contentMock); - } - }; + expect(createFlash).toHaveBeenCalledWith( + "Can't fetch content for the blob: Error: Request failed with status code 500", + ); + }); + }); - const bootstrapForNewSnippet = () => { - createComponent(); - }; + describe('with loaded blob', () => { + beforeEach(() => { + createComponent({ blob: TEST_BLOB_LOADED }); + }); - it('fetches blob content with the additional query', () => { - bootstrapForExistingSnippet(); + it('matches snapshot', () => { + expect(wrapper.element).toMatchSnapshot(); + }); - return waitForPromises().then(() => { - expect(joinPaths).toHaveBeenCalledWith('foo/', rawPathMock); - expect(findComponent(BlobHeaderEdit).props('value')).toBe(pathMock); - expect(findComponent(BlobContentEdit).props('value')).toBe(contentMock); - }); - }); + it('does not make API request', () => { + expect(axiosMock.history.get).toHaveLength(0); + }); + }); - it('flashes the error message if fetching content fails', () => { - bootstrapForExistingSnippet(500); + describe.each` + props | showLoading | showContent + ${{ blob: TEST_BLOB, canDelete: true, showDelete: true }} | ${true} | ${false} + ${{ blob: TEST_BLOB, canDelete: false, showDelete: false }} | ${true} | ${false} + ${{ blob: TEST_BLOB_LOADED }} | ${false} | ${true} + `('with $props', ({ props, showLoading, showContent }) => { + beforeEach(() => { + createComponent(props); + }); - return waitForPromises().then(() => { - expect(flashSpy).toHaveBeenCalled(); - expect(findComponent(BlobContentEdit).props('value')).toBe(''); - }); + it('shows blob header', () => { + const { canDelete = true, showDelete = false } = props; + + expect(findHeader().props()).toMatchObject({ + canDelete, + showDelete, }); + }); + + it(`handles loading icon (show=${showLoading})`, () => { + expect(findLoadingIcon().exists()).toBe(showLoading); + }); - it('does not fetch content for new snippet', () => { - bootstrapForNewSnippet(); + it(`handles content (show=${showContent})`, () => { + expect(findContent().exists()).toBe(showContent); - return waitForPromises().then(() => { - // we keep using waitForPromises to make sure we do not run failed test - expect(findComponent(BlobHeaderEdit).props('value')).toBe(''); - expect(findComponent(BlobContentEdit).props('value')).toBe(''); - expect(joinPaths).not.toHaveBeenCalled(); + if (showContent) { + expect(findContent().props()).toEqual({ + value: TEST_BLOB_LOADED.content, + fileGlobalId: TEST_BLOB_LOADED.id, + fileName: TEST_BLOB_LOADED.path, }); - }); + } }); }); }); diff --git a/spec/frontend/snippets/test_utils.js b/spec/frontend/snippets/test_utils.js new file mode 100644 index 00000000000..86262723157 --- /dev/null +++ b/spec/frontend/snippets/test_utils.js @@ -0,0 +1,76 @@ +import { + SNIPPET_BLOB_ACTION_CREATE, + SNIPPET_BLOB_ACTION_UPDATE, + SNIPPET_BLOB_ACTION_MOVE, + SNIPPET_BLOB_ACTION_DELETE, +} from '~/snippets/constants'; + +const CONTENT_1 = 'Lorem ipsum dolar\nSit amit\n\nGoodbye!\n'; +const CONTENT_2 = 'Lorem ipsum dolar sit amit.\n\nGoodbye!\n'; + +export const testEntries = { + created: { + id: 'blob_1', + diff: { + action: SNIPPET_BLOB_ACTION_CREATE, + filePath: '/new/file', + previousPath: '/new/file', + content: CONTENT_1, + }, + }, + deleted: { + id: 'blob_2', + diff: { + action: SNIPPET_BLOB_ACTION_DELETE, + filePath: '/src/delete/me', + previousPath: '/src/delete/me', + content: CONTENT_1, + }, + }, + updated: { + id: 'blob_3', + origContent: CONTENT_1, + diff: { + action: SNIPPET_BLOB_ACTION_UPDATE, + filePath: '/lorem.md', + previousPath: '/lorem.md', + content: CONTENT_2, + }, + }, + renamed: { + id: 'blob_4', + diff: { + action: SNIPPET_BLOB_ACTION_MOVE, + filePath: '/dolar.md', + previousPath: '/ipsum.md', + content: CONTENT_1, + }, + }, + renamedAndUpdated: { + id: 'blob_5', + origContent: CONTENT_1, + diff: { + action: SNIPPET_BLOB_ACTION_MOVE, + filePath: '/sit.md', + previousPath: '/sit/amit.md', + content: CONTENT_2, + }, + }, +}; + +export const createBlobFromTestEntry = ({ diff, origContent }, isOrig = false) => ({ + content: isOrig && origContent ? origContent : diff.content, + path: isOrig ? diff.previousPath : diff.filePath, +}); + +export const createBlobsFromTestEntries = (entries, isOrig = false) => + entries.reduce( + (acc, entry) => + Object.assign(acc, { + [entry.id]: { + id: entry.id, + ...createBlobFromTestEntry(entry, isOrig), + }, + }), + {}, + ); diff --git a/spec/frontend/snippets/utils/blob_spec.js b/spec/frontend/snippets/utils/blob_spec.js index ba39b9fec36..c20cf2e6102 100644 --- a/spec/frontend/snippets/utils/blob_spec.js +++ b/spec/frontend/snippets/utils/blob_spec.js @@ -1,19 +1,12 @@ import { cloneDeep } from 'lodash'; -import { - SNIPPET_BLOB_ACTION_CREATE, - SNIPPET_BLOB_ACTION_UPDATE, - SNIPPET_BLOB_ACTION_MOVE, - SNIPPET_BLOB_ACTION_DELETE, -} from '~/snippets/constants'; import { decorateBlob, createBlob, diffAll } from '~/snippets/utils/blob'; +import { testEntries, createBlobsFromTestEntries } from '../test_utils'; jest.mock('lodash/uniqueId', () => arg => `${arg}fakeUniqueId`); const TEST_RAW_BLOB = { rawPath: '/test/blob/7/raw', }; -const CONTENT_1 = 'Lorem ipsum dolar\nSit amit\n\nGoodbye!\n'; -const CONTENT_2 = 'Lorem ipsum dolar sit amit.\n\nGoodbye!\n'; describe('~/snippets/utils/blob', () => { describe('decorateBlob', () => { @@ -41,70 +34,6 @@ describe('~/snippets/utils/blob', () => { }); describe('diffAll', () => { - // This object contains entries that contain an expected "diff" and the `id` - // or `origContent` that should be used to generate the expected diff. - const testEntries = { - created: { - id: 'blob_1', - diff: { - action: SNIPPET_BLOB_ACTION_CREATE, - filePath: '/new/file', - previousPath: '/new/file', - content: CONTENT_1, - }, - }, - deleted: { - id: 'blob_2', - diff: { - action: SNIPPET_BLOB_ACTION_DELETE, - filePath: '/src/delete/me', - previousPath: '/src/delete/me', - content: CONTENT_1, - }, - }, - updated: { - id: 'blob_3', - origContent: CONTENT_1, - diff: { - action: SNIPPET_BLOB_ACTION_UPDATE, - filePath: '/lorem.md', - previousPath: '/lorem.md', - content: CONTENT_2, - }, - }, - renamed: { - id: 'blob_4', - diff: { - action: SNIPPET_BLOB_ACTION_MOVE, - filePath: '/dolar.md', - previousPath: '/ipsum.md', - content: CONTENT_1, - }, - }, - renamedAndUpdated: { - id: 'blob_5', - origContent: CONTENT_1, - diff: { - action: SNIPPET_BLOB_ACTION_MOVE, - filePath: '/sit.md', - previousPath: '/sit/amit.md', - content: CONTENT_2, - }, - }, - }; - const createBlobsFromTestEntries = (entries, isOrig = false) => - entries.reduce( - (acc, { id, diff, origContent }) => - Object.assign(acc, { - [id]: { - id, - content: isOrig && origContent ? origContent : diff.content, - path: isOrig ? diff.previousPath : diff.filePath, - }, - }), - {}, - ); - it('should create diff from original files', () => { const origBlobs = createBlobsFromTestEntries( [ diff --git a/spec/frontend/vue_mr_widget/stores/get_state_key_spec.js b/spec/frontend/vue_mr_widget/stores/get_state_key_spec.js index 1cb2c6c669b..128e0f39c41 100644 --- a/spec/frontend/vue_mr_widget/stores/get_state_key_spec.js +++ b/spec/frontend/vue_mr_widget/stores/get_state_key_spec.js @@ -11,15 +11,13 @@ describe('getStateKey', () => { hasMergeableDiscussionsState: false, isPipelineBlocked: false, canBeMerged: false, + projectArchived: false, + branchMissing: false, + commitsCount: 2, + hasConflicts: false, + workInProgress: false, }; - const data = { - project_archived: false, - branch_missing: false, - commits_count: 2, - has_conflicts: false, - work_in_progress: false, - }; - const bound = getStateKey.bind(context, data); + const bound = getStateKey.bind(context); expect(bound()).toEqual(null); @@ -49,7 +47,7 @@ describe('getStateKey', () => { expect(bound()).toEqual('unresolvedDiscussions'); - data.work_in_progress = true; + context.workInProgress = true; expect(bound()).toEqual('workInProgress'); @@ -62,7 +60,7 @@ describe('getStateKey', () => { expect(bound()).toEqual('rebase'); - data.has_conflicts = true; + context.hasConflicts = true; expect(bound()).toEqual('conflicts'); @@ -70,15 +68,15 @@ describe('getStateKey', () => { expect(bound()).toEqual('checking'); - data.commits_count = 0; + context.commitsCount = 0; expect(bound()).toEqual('nothingToMerge'); - data.branch_missing = true; + context.branchMissing = true; expect(bound()).toEqual('missingBranch'); - data.project_archived = true; + context.projectArchived = true; expect(bound()).toEqual('archived'); }); @@ -94,15 +92,13 @@ describe('getStateKey', () => { isPipelineBlocked: false, canBeMerged: false, shouldBeRebased: true, + projectArchived: false, + branchMissing: false, + commitsCount: 2, + hasConflicts: false, + workInProgress: false, }; - const data = { - project_archived: false, - branch_missing: false, - commits_count: 2, - has_conflicts: false, - work_in_progress: false, - }; - const bound = getStateKey.bind(context, data); + const bound = getStateKey.bind(context); expect(bound()).toEqual('rebase'); }); @@ -115,15 +111,11 @@ describe('getStateKey', () => { `( 'returns $stateKey when canMerge is $canMerge and isSHAMismatch is $isSHAMismatch', ({ canMerge, isSHAMismatch, stateKey }) => { - const bound = getStateKey.bind( - { - canMerge, - isSHAMismatch, - }, - { - commits_count: 2, - }, - ); + const bound = getStateKey.bind({ + canMerge, + isSHAMismatch, + commitsCount: 2, + }); expect(bound()).toEqual(stateKey); }, diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb index 83bba3cec04..dcaaa8d4188 100644 --- a/spec/lib/gitlab/auth_spec.rb +++ b/spec/lib/gitlab/auth_spec.rb @@ -551,7 +551,7 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do it 'fails if token is not related to project' do another_deploy_token = create(:deploy_token) - expect(gl_auth.find_for_git_client(login, another_deploy_token.token, project: project, ip: 'ip')) + expect(gl_auth.find_for_git_client(another_deploy_token.username, another_deploy_token.token, project: project, ip: 'ip')) .to eq(auth_failure) end @@ -576,6 +576,13 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do expect(subject).to eq(auth_success) end + + it 'fails if token is not related to group' do + another_deploy_token = create(:deploy_token, :group, read_repository: true) + + expect(gl_auth.find_for_git_client(another_deploy_token.username, another_deploy_token.token, project: project_with_group, ip: 'ip')) + .to eq(auth_failure) + end end context 'when the deploy token has read_registry as a scope' do diff --git a/spec/lib/gitlab/ci/runner_instructions_spec.rb b/spec/lib/gitlab/ci/runner_instructions_spec.rb new file mode 100644 index 00000000000..32ee2ceb040 --- /dev/null +++ b/spec/lib/gitlab/ci/runner_instructions_spec.rb @@ -0,0 +1,217 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Ci::RunnerInstructions do + using RSpec::Parameterized::TableSyntax + + let(:params) { {} } + let(:user) { create(:user) } + + describe 'OS' do + Gitlab::Ci::RunnerInstructions::OS.each do |name, subject| + context name do + it 'has the required fields' do + expect(subject).to have_key(:human_readable_name) + expect(subject).to have_key(:download_locations) + expect(subject).to have_key(:install_script_template_path) + expect(subject).to have_key(:runner_executable) + end + + it 'has a valid script' do + expect(File.read(subject[:install_script_template_path]).length).not_to eq(0) + end + end + end + end + + describe 'OTHER_ENVIRONMENTS' do + Gitlab::Ci::RunnerInstructions::OTHER_ENVIRONMENTS.each do |name, subject| + context name do + it 'has the required fields' do + expect(subject).to have_key(:human_readable_name) + expect(subject).to have_key(:installation_instructions_url) + end + end + end + end + + describe '#install_script' do + subject { described_class.new(current_user: user, **params) } + + context 'invalid params' do + where(:current_params, :expected_error_message) do + { os: nil, arch: nil } | 'Missing OS' + { os: 'linux', arch: nil } | 'Missing arch' + { os: nil, arch: 'amd64' } | 'Missing OS' + { os: 'non_existing_os', arch: 'amd64' } | 'Invalid OS' + { os: 'linux', arch: 'non_existing_arch' } | 'Architecture not found for OS' + { os: 'windows', arch: 'non_existing_arch' } | 'Architecture not found for OS' + end + + with_them do + let(:params) { current_params } + + it 'raises argument error' do + result = subject.install_script + + expect(result).to be_nil + expect(subject.errors).to include(expected_error_message) + end + end + end + + context 'with valid params' do + where(:os, :arch) do + 'linux' | 'amd64' + 'linux' | '386' + 'linux' | 'arm' + 'linux' | 'arm64' + 'windows' | 'amd64' + 'windows' | '386' + 'osx' | 'amd64' + end + + with_them do + let(:params) { { os: os, arch: arch } } + + it 'returns string containing correct params' do + result = subject.install_script + + expect(result).to be_a(String) + + if os == 'osx' + expect(result).to include("darwin-#{arch}") + else + expect(result).to include("#{os}-#{arch}") + end + end + end + end + end + + describe '#register_command' do + let(:params) { { os: 'linux', arch: 'foo' } } + + where(:commands) do + Gitlab::Ci::RunnerInstructions::OS.map do |name, values| + { name => values[:runner_executable] } + end + end + + context 'group' do + let(:group) { create(:group) } + + subject { described_class.new(current_user: user, group: group, **params) } + + context 'user is owner' do + before do + group.add_owner(user) + end + + with_them do + let(:params) { { os: commands.each_key.first, arch: 'foo' } } + + it 'have correct configurations' do + result = subject.register_command + + expect(result).to include("#{commands[commands.each_key.first]} register") + expect(result).to include("--registration-token #{group.runners_token}") + expect(result).to include("--url #{Gitlab::Routing.url_helpers.root_url(only_path: false)}") + end + end + end + + context 'user is not owner' do + where(:user_permission) do + [:maintainer, :developer, :reporter, :guest] + end + + with_them do + before do + create(:group_member, user_permission, group: group, user: user) + end + + it 'raises error' do + result = subject.register_command + + expect(result).to be_nil + expect(subject.errors).to include("Gitlab::Access::AccessDeniedError") + end + end + end + end + + context 'project' do + let(:project) { create(:project) } + + subject { described_class.new(current_user: user, project: project, **params) } + + context 'user is maintainer' do + before do + project.add_maintainer(user) + end + + with_them do + let(:params) { { os: commands.each_key.first, arch: 'foo' } } + + it 'have correct configurations' do + result = subject.register_command + + expect(result).to include("#{commands[commands.each_key.first]} register") + expect(result).to include("--registration-token #{project.runners_token}") + expect(result).to include("--url #{Gitlab::Routing.url_helpers.root_url(only_path: false)}") + end + end + end + + context 'user is not maintainer' do + where(:user_permission) do + [:developer, :reporter, :guest] + end + + with_them do + before do + create(:project_member, user_permission, project: project, user: user) + end + + it 'raises error' do + result = subject.register_command + + expect(result).to be_nil + expect(subject.errors).to include("Gitlab::Access::AccessDeniedError") + end + end + end + end + + context 'instance' do + subject { described_class.new(current_user: user, **params) } + + context 'user is admin' do + let(:user) { create(:user, :admin) } + + with_them do + let(:params) { { os: commands.each_key.first, arch: 'foo' } } + + it 'have correct configurations' do + result = subject.register_command + + expect(result).to include("#{commands[commands.each_key.first]} register") + expect(result).to include("--registration-token #{Gitlab::CurrentSettings.runners_registration_token}") + expect(result).to include("--url #{Gitlab::Routing.url_helpers.root_url(only_path: false)}") + end + end + end + + context 'user is not admin' do + it 'raises error' do + result = subject.register_command + + expect(result).to be_nil + expect(subject.errors).to include("Gitlab::Access::AccessDeniedError") + end + end + end + end +end diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb index 84b9464b610..e02c71a1c6f 100644 --- a/spec/models/merge_request_diff_spec.rb +++ b/spec/models/merge_request_diff_spec.rb @@ -103,6 +103,8 @@ RSpec.describe MergeRequestDiff do it 'ignores diffs with 0 files' do MergeRequestDiffFile.where(merge_request_diff_id: [closed_recently.id, merged_recently.id]).delete_all + closed_recently.update!(files_count: 0) + merged_recently.update!(files_count: 0) is_expected.to contain_exactly(outdated.id, latest.id, closed.id, merged.id) end diff --git a/spec/policies/group_policy_spec.rb b/spec/policies/group_policy_spec.rb index 4debaf59eb9..3e0ea164e3d 100644 --- a/spec/policies/group_policy_spec.rb +++ b/spec/policies/group_policy_spec.rb @@ -63,6 +63,24 @@ RSpec.describe GroupPolicy do end end + shared_examples 'deploy token does not get confused with user' do + before do + deploy_token.update!(id: user_id) + end + + let(:deploy_token) { create(:deploy_token) } + let(:current_user) { deploy_token } + + it do + expect_disallowed(*read_group_permissions) + expect_disallowed(*guest_permissions) + expect_disallowed(*reporter_permissions) + expect_disallowed(*developer_permissions) + expect_disallowed(*maintainer_permissions) + expect_disallowed(*owner_permissions) + end + end + context 'guests' do let(:current_user) { guest } @@ -74,6 +92,10 @@ RSpec.describe GroupPolicy do expect_disallowed(*maintainer_permissions) expect_disallowed(*owner_permissions) end + + it_behaves_like 'deploy token does not get confused with user' do + let(:user_id) { guest.id } + end end context 'reporter' do @@ -87,6 +109,10 @@ RSpec.describe GroupPolicy do expect_disallowed(*maintainer_permissions) expect_disallowed(*owner_permissions) end + + it_behaves_like 'deploy token does not get confused with user' do + let(:user_id) { reporter.id } + end end context 'developer' do @@ -100,6 +126,10 @@ RSpec.describe GroupPolicy do expect_disallowed(*maintainer_permissions) expect_disallowed(*owner_permissions) end + + it_behaves_like 'deploy token does not get confused with user' do + let(:user_id) { developer.id } + end end context 'maintainer' do @@ -136,6 +166,10 @@ RSpec.describe GroupPolicy do expect_disallowed(*owner_permissions) end end + + it_behaves_like 'deploy token does not get confused with user' do + let(:user_id) { maintainer.id } + end end context 'owner' do @@ -149,6 +183,10 @@ RSpec.describe GroupPolicy do expect_allowed(*maintainer_permissions) expect_allowed(*owner_permissions) end + + it_behaves_like 'deploy token does not get confused with user' do + let(:user_id) { owner.id } + end end context 'admin' do @@ -166,6 +204,14 @@ RSpec.describe GroupPolicy do context 'with admin mode', :enable_admin_mode do specify { expect_allowed(*admin_permissions) } end + + it_behaves_like 'deploy token does not get confused with user' do + let(:user_id) { admin.id } + + context 'with admin mode', :enable_admin_mode do + it { expect_disallowed(*admin_permissions) } + end + end end describe 'private nested group use the highest access level from the group and inherited permissions' do diff --git a/spec/requests/api/maven_packages_spec.rb b/spec/requests/api/maven_packages_spec.rb index 189d6a4c1a4..b9351308545 100644 --- a/spec/requests/api/maven_packages_spec.rb +++ b/spec/requests/api/maven_packages_spec.rb @@ -193,6 +193,24 @@ RSpec.describe API::MavenPackages do it_behaves_like 'downloads with a job token' it_behaves_like 'downloads with a deploy token' + + it 'does not allow download by a unauthorized deploy token with same id as a user with access' do + unauthorized_deploy_token = create(:deploy_token, read_package_registry: true, write_package_registry: true) + + another_user = create(:user) + project.add_developer(another_user) + + # We force the id of the deploy token and the user to be the same + unauthorized_deploy_token.update!(id: another_user.id) + + download_file( + package_file.file_name, + {}, + Gitlab::Auth::AuthFinders::DEPLOY_TOKEN_HEADER => unauthorized_deploy_token.token + ) + + expect(response).to have_gitlab_http_status(:forbidden) + end end context 'project name is different from a package name' do @@ -451,6 +469,20 @@ RSpec.describe API::MavenPackages do expect(response).to have_gitlab_http_status(:ok) end + it 'rejects requests by a unauthorized deploy token with same id as a user with access' do + unauthorized_deploy_token = create(:deploy_token, read_package_registry: true, write_package_registry: true) + + another_user = create(:user) + project.add_developer(another_user) + + # We force the id of the deploy token and the user to be the same + unauthorized_deploy_token.update!(id: another_user.id) + + authorize_upload({}, headers.merge(Gitlab::Auth::AuthFinders::DEPLOY_TOKEN_HEADER => unauthorized_deploy_token.token)) + + expect(response).to have_gitlab_http_status(:forbidden) + end + def authorize_upload(params = {}, request_headers = headers) put api("/projects/#{project.id}/packages/maven/com/example/my-app/#{version}/maven-metadata.xml/authorize"), params: params, headers: request_headers end @@ -538,6 +570,20 @@ RSpec.describe API::MavenPackages do expect(response).to have_gitlab_http_status(:ok) end + it 'rejects uploads by a unauthorized deploy token with same id as a user with access' do + unauthorized_deploy_token = create(:deploy_token, read_package_registry: true, write_package_registry: true) + + another_user = create(:user) + project.add_developer(another_user) + + # We force the id of the deploy token and the user to be the same + unauthorized_deploy_token.update!(id: another_user.id) + + upload_file(params, headers.merge(Gitlab::Auth::AuthFinders::DEPLOY_TOKEN_HEADER => unauthorized_deploy_token.token)) + + expect(response).to have_gitlab_http_status(:forbidden) + end + context 'version is not correct' do let(:version) { '$%123' } diff --git a/spec/requests/api/performance_bar_spec.rb b/spec/requests/api/performance_bar_spec.rb new file mode 100644 index 00000000000..a4dbb3d17b8 --- /dev/null +++ b/spec/requests/api/performance_bar_spec.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Performance Bar for API requests', :request_store, :clean_gitlab_redis_cache do + context 'with user that has access to the performance bar' do + let_it_be(:admin) { create(:admin) } + + context 'when cookie is set to true' do + before do + cookies[:perf_bar_enabled] = 'true' + end + + it 'stores performance data' do + get api("/users/#{admin.id}", admin) + + expect(Peek.adapter.get(headers['X-Request-Id'])).not_to be_empty + end + end + + context 'when cookie is missing' do + it 'does not store performance data' do + get api("/users/#{admin.id}", admin) + + expect(Peek.adapter.get(headers['X-Request-Id'])).to be_nil + end + end + end + + context 'with user that does not have access to the performance bar' do + let(:user) { create(:user) } + + it 'does not store performance data' do + cookies[:perf_bar_enabled] = 'true' + + get api("/users/#{user.id}", user) + + expect(Peek.adapter.get(headers['X-Request-Id'])).to be_nil + end + end +end diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb index 7a06fe21e60..fd4261fb50d 100644 --- a/spec/requests/lfs_http_spec.rb +++ b/spec/requests/lfs_http_spec.rb @@ -549,12 +549,6 @@ RSpec.describe 'Git LFS API and storage' do project.lfs_objects << lfs_object end - context 'when Deploy Token is valid' do - let(:deploy_token) { create(:deploy_token, projects: [project]) } - - it_behaves_like 'an authorized request', renew_authorization: false - end - context 'when Deploy Token is not valid' do let(:deploy_token) { create(:deploy_token, projects: [project], read_repository: false) } @@ -564,7 +558,14 @@ RSpec.describe 'Git LFS API and storage' do context 'when Deploy Token is not related to the project' do let(:deploy_token) { create(:deploy_token, projects: [other_project]) } - it_behaves_like 'LFS http 404 response' + it_behaves_like 'LFS http 401 response' + end + + # TODO: We should fix this test case that causes flakyness by alternating the result of the above test cases. + context 'when Deploy Token is valid' do + let(:deploy_token) { create(:deploy_token, projects: [project]) } + + it_behaves_like 'an authorized request', renew_authorization: false end end diff --git a/spec/support/helpers/filtered_search_helpers.rb b/spec/support/helpers/filtered_search_helpers.rb index 1847a8f8a06..d203ff60cc9 100644 --- a/spec/support/helpers/filtered_search_helpers.rb +++ b/spec/support/helpers/filtered_search_helpers.rb @@ -13,7 +13,7 @@ module FilteredSearchHelpers search = "#{search_term} " end - filtered_search.set(search) + filtered_search.set(search, rapid: false) if submit # Wait for the lazy author/assignee tokens that diff --git a/spec/support/shared_examples/policies/project_policy_shared_examples.rb b/spec/support/shared_examples/policies/project_policy_shared_examples.rb index c35c6cf5794..d8476f5dcc2 100644 --- a/spec/support/shared_examples/policies/project_policy_shared_examples.rb +++ b/spec/support/shared_examples/policies/project_policy_shared_examples.rb @@ -86,6 +86,28 @@ RSpec.shared_examples 'project policies as anonymous' do end end +RSpec.shared_examples 'deploy token does not get confused with user' do + before do + deploy_token.update!(id: user_id) + + # Project with public builds are available to all + project.update!(public_builds: false) + end + + let(:deploy_token) { create(:deploy_token) } + + subject { described_class.new(deploy_token, project) } + + it do + expect_disallowed(*guest_permissions) + expect_disallowed(*reporter_permissions) + expect_disallowed(*team_member_reporter_permissions) + expect_disallowed(*developer_permissions) + expect_disallowed(*maintainer_permissions) + expect_disallowed(*owner_permissions) + end +end + RSpec.shared_examples 'project policies as guest' do subject { described_class.new(guest, project) } @@ -104,6 +126,10 @@ RSpec.shared_examples 'project policies as guest' do expect_disallowed(*owner_permissions) end + it_behaves_like 'deploy token does not get confused with user' do + let(:user_id) { guest.id } + end + it_behaves_like 'archived project policies' do let(:regular_abilities) { guest_permissions } end @@ -117,7 +143,7 @@ RSpec.shared_examples 'project policies as guest' do context 'when public builds disabled' do before do - project.update(public_builds: false) + project.update!(public_builds: false) end it do @@ -128,7 +154,7 @@ RSpec.shared_examples 'project policies as guest' do context 'when builds are disabled' do before do - project.project_feature.update(builds_access_level: ProjectFeature::DISABLED) + project.project_feature.update!(builds_access_level: ProjectFeature::DISABLED) end it do @@ -154,6 +180,10 @@ RSpec.shared_examples 'project policies as reporter' do expect_disallowed(*owner_permissions) end + it_behaves_like 'deploy token does not get confused with user' do + let(:user_id) { reporter.id } + end + it_behaves_like 'archived project policies' do let(:regular_abilities) { reporter_permissions } end @@ -175,6 +205,10 @@ RSpec.shared_examples 'project policies as developer' do expect_disallowed(*owner_permissions) end + it_behaves_like 'deploy token does not get confused with user' do + let(:user_id) { developer.id } + end + it_behaves_like 'archived project policies' do let(:regular_abilities) { developer_permissions } end @@ -196,6 +230,10 @@ RSpec.shared_examples 'project policies as maintainer' do expect_disallowed(*owner_permissions) end + it_behaves_like 'deploy token does not get confused with user' do + let(:user_id) { maintainer.id } + end + it_behaves_like 'archived project policies' do let(:regular_abilities) { maintainer_permissions } end @@ -217,6 +255,10 @@ RSpec.shared_examples 'project policies as owner' do expect_allowed(*owner_permissions) end + it_behaves_like 'deploy token does not get confused with user' do + let(:user_id) { owner.id } + end + it_behaves_like 'archived project policies' do let(:regular_abilities) { owner_permissions } end @@ -238,6 +280,28 @@ RSpec.shared_examples 'project policies as admin with admin mode' do expect_allowed(*owner_permissions) end + context 'deploy token does not get confused with user' do + before do + allow(deploy_token).to receive(:id).and_return(admin.id) + + # Project with public builds are available to all + project.update!(public_builds: false) + end + + let(:deploy_token) { create(:deploy_token) } + + subject { described_class.new(deploy_token, project) } + + it do + expect_disallowed(*guest_permissions) + expect_disallowed(*reporter_permissions) + expect_disallowed(*team_member_reporter_permissions) + expect_disallowed(*developer_permissions) + expect_disallowed(*maintainer_permissions) + expect_disallowed(*owner_permissions) + end + end + it_behaves_like 'archived project policies' do let(:regular_abilities) { owner_permissions } end @@ -257,5 +321,20 @@ RSpec.shared_examples 'project policies as admin without admin mode' do subject { described_class.new(admin, project) } it { is_expected.to be_banned } + + context 'deploy token does not get confused with user' do + before do + allow(deploy_token).to receive(:id).and_return(admin.id) + + # Project with public builds are available to all + project.update!(public_builds: false) + end + + let(:deploy_token) { create(:deploy_token) } + + subject { described_class.new(deploy_token, project) } + + it { is_expected.to be_banned } + end end end |