diff options
70 files changed, 1129 insertions, 309 deletions
diff --git a/.gitlab/merge_request_templates/Documentation.md b/.gitlab/merge_request_templates/Documentation.md index b059c1f68ad..fcf26d95035 100644 --- a/.gitlab/merge_request_templates/Documentation.md +++ b/.gitlab/merge_request_templates/Documentation.md @@ -15,9 +15,9 @@ ## Author's checklist (required) -- [ ] Follow the [Documentation Guidelines](https://docs.gitlab.com/ee/development/documentation/) and [Style Guide](https://docs.gitlab.com/ee/development/documentation/styleguide.html). +- [ ] Follow the [Documentation Guidelines](https://docs.gitlab.com/ee/development/documentation/) and [Style Guide](https://docs.gitlab.com/ee/development/documentation/styleguide/). - If you have **Developer** permissions or higher: - - [ ] Ensure that the [product tier badge](https://docs.gitlab.com/ee/development/documentation/styleguide.html#product-badges) is added to doc's `h1`. + - [ ] Ensure that the [product tier badge](https://docs.gitlab.com/ee/development/documentation/styleguide/index.html#product-tier-badges) is added to doc's `h1`. - [ ] Apply the ~documentation label, plus: - The corresponding DevOps stage and group labels, if applicable. - ~"development guidelines" when changing docs under `doc/development/*`, `CONTRIBUTING.md`, or `README.md`. diff --git a/app/assets/javascripts/frequent_items/components/app.vue b/app/assets/javascripts/frequent_items/components/app.vue index 61080fb5487..c4f61b839e4 100644 --- a/app/assets/javascripts/frequent_items/components/app.vue +++ b/app/assets/javascripts/frequent_items/components/app.vue @@ -3,7 +3,6 @@ import { mapState, mapActions, mapGetters } from 'vuex'; import { GlLoadingIcon } from '@gitlab/ui'; import AccessorUtilities from '~/lib/utils/accessor'; import eventHub from '../event_hub'; -import store from '../store'; import { FREQUENT_ITEMS, STORAGE_KEY } from '../constants'; import { isMobile, updateExistingFrequentItem, sanitizeItem } from '../utils'; import FrequentItemsSearchInput from './frequent_items_search_input.vue'; @@ -11,7 +10,6 @@ import FrequentItemsList from './frequent_items_list.vue'; import frequentItemsMixin from './frequent_items_mixin'; export default { - store, components: { FrequentItemsSearchInput, FrequentItemsList, diff --git a/app/assets/javascripts/frequent_items/components/frequent_items_list_item.vue b/app/assets/javascripts/frequent_items/components/frequent_items_list_item.vue index 1203f389931..3260d768fd9 100644 --- a/app/assets/javascripts/frequent_items/components/frequent_items_list_item.vue +++ b/app/assets/javascripts/frequent_items/components/frequent_items_list_item.vue @@ -1,13 +1,18 @@ <script> /* eslint-disable vue/require-default-prop, vue/no-v-html */ +import { mapState } from 'vuex'; import Identicon from '~/vue_shared/components/identicon.vue'; import highlight from '~/lib/utils/highlight'; import { truncateNamespace } from '~/lib/utils/text_utility'; +import Tracking from '~/tracking'; + +const trackingMixin = Tracking.mixin(); export default { components: { Identicon, }, + mixins: [trackingMixin], props: { matcher: { type: String, @@ -37,6 +42,7 @@ export default { }, }, computed: { + ...mapState(['dropdownType']), truncatedNamespace() { return truncateNamespace(this.namespace); }, @@ -49,7 +55,11 @@ export default { <template> <li class="frequent-items-list-item-container"> - <a :href="webUrl" class="clearfix"> + <a + :href="webUrl" + class="clearfix" + @click="track('click_link', { label: `${dropdownType}_dropdown_frequent_items_list_item` })" + > <div ref="frequentItemsItemAvatarContainer" class="frequent-items-item-avatar-container avatar-container rect-avatar s32" diff --git a/app/assets/javascripts/frequent_items/components/frequent_items_search_input.vue b/app/assets/javascripts/frequent_items/components/frequent_items_search_input.vue index 19cb09f0dcc..8042e8c7bc9 100644 --- a/app/assets/javascripts/frequent_items/components/frequent_items_search_input.vue +++ b/app/assets/javascripts/frequent_items/components/frequent_items_search_input.vue @@ -1,27 +1,34 @@ <script> import { debounce } from 'lodash'; -import { mapActions } from 'vuex'; +import { mapActions, mapState } from 'vuex'; import { GlIcon } from '@gitlab/ui'; import eventHub from '../event_hub'; import frequentItemsMixin from './frequent_items_mixin'; +import Tracking from '~/tracking'; + +const trackingMixin = Tracking.mixin(); export default { components: { GlIcon, }, - mixins: [frequentItemsMixin], + mixins: [frequentItemsMixin, trackingMixin], data() { return { searchQuery: '', }; }, computed: { + ...mapState(['dropdownType']), translations() { return this.getTranslations(['searchInputPlaceholder']); }, }, watch: { searchQuery: debounce(function debounceSearchQuery() { + this.track('type_search_query', { + label: `${this.dropdownType}_dropdown_frequent_items_search_input`, + }); this.setSearchQuery(this.searchQuery); }, 500), }, diff --git a/app/assets/javascripts/frequent_items/index.js b/app/assets/javascripts/frequent_items/index.js index 1998bf4358a..639562bf961 100644 --- a/app/assets/javascripts/frequent_items/index.js +++ b/app/assets/javascripts/frequent_items/index.js @@ -2,6 +2,7 @@ import $ from 'jquery'; import Vue from 'vue'; import Translate from '~/vue_shared/translate'; import eventHub from './event_hub'; +import { createStore } from '~/frequent_items/store'; Vue.use(Translate); @@ -28,11 +29,15 @@ export default function initFrequentItemDropdowns() { return; } + const dropdownType = namespace; + const store = createStore({ dropdownType }); + import('./components/app.vue') .then(({ default: FrequentItems }) => { // eslint-disable-next-line no-new new Vue({ el, + store, data() { const { dataset } = this.$options.el; const item = { diff --git a/app/assets/javascripts/frequent_items/store/index.js b/app/assets/javascripts/frequent_items/store/index.js index ece9e6419dd..83176d69802 100644 --- a/app/assets/javascripts/frequent_items/store/index.js +++ b/app/assets/javascripts/frequent_items/store/index.js @@ -7,10 +7,11 @@ import state from './state'; Vue.use(Vuex); -export default () => - new Vuex.Store({ +export const createStore = (initState = {}) => { + return new Vuex.Store({ actions, getters, mutations, - state: state(), + state: state(initState), }); +}; diff --git a/app/assets/javascripts/frequent_items/store/state.js b/app/assets/javascripts/frequent_items/store/state.js index 75b04febee4..c5c0b25fdf2 100644 --- a/app/assets/javascripts/frequent_items/store/state.js +++ b/app/assets/javascripts/frequent_items/store/state.js @@ -1,5 +1,6 @@ -export default () => ({ +export default ({ dropdownType = '' } = {}) => ({ namespace: '', + dropdownType, storageKey: '', searchQuery: '', isLoadingItems: false, diff --git a/app/assets/javascripts/pipelines/components/graph/graph_component.vue b/app/assets/javascripts/pipelines/components/graph/graph_component.vue index 7704f2ba0fd..2f050302db5 100644 --- a/app/assets/javascripts/pipelines/components/graph/graph_component.vue +++ b/app/assets/javascripts/pipelines/components/graph/graph_component.vue @@ -1,10 +1,14 @@ <script> +import LinkedGraphWrapper from '../graph_shared/linked_graph_wrapper.vue'; +import LinkedPipelinesColumn from './linked_pipelines_column.vue'; import StageColumnComponent from './stage_column_component.vue'; -import { MAIN } from './constants'; +import { DOWNSTREAM, MAIN, UPSTREAM } from './constants'; export default { name: 'PipelineGraph', components: { + LinkedGraphWrapper, + LinkedPipelinesColumn, StageColumnComponent, }, props: { @@ -23,10 +27,60 @@ export default { default: MAIN, }, }, + pipelineTypeConstants: { + DOWNSTREAM, + UPSTREAM, + }, + data() { + return { + hoveredJobName: '', + pipelineExpanded: { + jobName: '', + expanded: false, + }, + }; + }, computed: { + downstreamPipelines() { + return this.hasDownstreamPipelines ? this.pipeline.downstream : []; + }, graph() { return this.pipeline.stages; }, + hasDownstreamPipelines() { + return Boolean(this.pipeline?.downstream?.length > 0); + }, + hasUpstreamPipelines() { + return Boolean(this.pipeline?.upstream?.length > 0); + }, + // The two show checks prevent upstream / downstream from showing redundant linked columns + showDownstreamPipelines() { + return ( + this.hasDownstreamPipelines && this.type !== this.$options.pipelineTypeConstants.UPSTREAM + ); + }, + showUpstreamPipelines() { + return ( + this.hasUpstreamPipelines && this.type !== this.$options.pipelineTypeConstants.DOWNSTREAM + ); + }, + upstreamPipelines() { + return this.hasUpstreamPipelines ? this.pipeline.upstream : []; + }, + }, + methods: { + handleError(errorType) { + this.$emit('error', errorType); + }, + setJob(jobName) { + this.hoveredJobName = jobName; + }, + togglePipelineExpanded(jobName, expanded) { + this.pipelineExpanded = { + expanded, + jobName: expanded ? jobName : '', + }; + }, }, }; </script> @@ -36,13 +90,39 @@ export default { class="gl-pipeline-min-h gl-display-flex gl-position-relative gl-overflow-auto gl-bg-gray-10 gl-white-space-nowrap" :class="{ 'gl-py-5': !isLinkedPipeline }" > - <stage-column-component - v-for="stage in graph" - :key="stage.name" - :title="stage.name" - :groups="stage.groups" - :action="stage.status.action" - /> + <linked-graph-wrapper> + <template #upstream> + <linked-pipelines-column + v-if="showUpstreamPipelines" + :linked-pipelines="upstreamPipelines" + :column-title="__('Upstream')" + :type="$options.pipelineTypeConstants.UPSTREAM" + @error="handleError" + /> + </template> + <template #main> + <stage-column-component + v-for="stage in graph" + :key="stage.name" + :title="stage.name" + :groups="stage.groups" + :action="stage.status.action" + :job-hovered="hoveredJobName" + :pipeline-expanded="pipelineExpanded" + /> + </template> + <template #downstream> + <linked-pipelines-column + v-if="showDownstreamPipelines" + :linked-pipelines="downstreamPipelines" + :column-title="__('Downstream')" + :type="$options.pipelineTypeConstants.DOWNSTREAM" + @downstreamHovered="setJob" + @pipelineExpandToggle="togglePipelineExpanded" + @error="handleError" + /> + </template> + </linked-graph-wrapper> </div> </div> </template> diff --git a/app/assets/javascripts/pipelines/components/graph/graph_component_wrapper.vue b/app/assets/javascripts/pipelines/components/graph/graph_component_wrapper.vue index 49a2feab0fc..cb2f4d0d623 100644 --- a/app/assets/javascripts/pipelines/components/graph/graph_component_wrapper.vue +++ b/app/assets/javascripts/pipelines/components/graph/graph_component_wrapper.vue @@ -42,7 +42,7 @@ export default { }; }, update(data) { - return unwrapPipelineData(this.pipelineIid, data); + return unwrapPipelineData(this.pipelineProjectPath, data); }, error() { this.reportFailure(LOAD_FAILURE); @@ -77,13 +77,11 @@ export default { }; </script> <template> - <gl-alert v-if="showAlert" :variant="alert.variant" @dismiss="hideAlert"> - {{ alert.text }} - </gl-alert> - <gl-loading-icon - v-else-if="$apollo.queries.pipeline.loading" - class="gl-mx-auto gl-my-4" - size="lg" - /> - <pipeline-graph v-else :pipeline="pipeline" /> + <div> + <gl-alert v-if="showAlert" :variant="alert.variant" @dismiss="hideAlert"> + {{ alert.text }} + </gl-alert> + <gl-loading-icon v-if="$apollo.queries.pipeline.loading" class="gl-mx-auto gl-my-4" size="lg" /> + <pipeline-graph v-if="pipeline" :pipeline="pipeline" @error="reportFailure" /> + </div> </template> diff --git a/app/assets/javascripts/pipelines/components/graph/linked_pipeline.vue b/app/assets/javascripts/pipelines/components/graph/linked_pipeline.vue index 97e5a309215..1a179de64cd 100644 --- a/app/assets/javascripts/pipelines/components/graph/linked_pipeline.vue +++ b/app/assets/javascripts/pipelines/components/graph/linked_pipeline.vue @@ -25,23 +25,33 @@ export default { type: String, required: true, }, - pipeline: { - type: Object, + expanded: { + type: Boolean, required: true, }, - projectId: { - type: Number, + pipeline: { + type: Object, required: true, }, type: { type: String, required: true, }, - }, - data() { - return { - expanded: false, - }; + /* + The next two props will be removed or required + once the graph transition is done. + See: https://gitlab.com/gitlab-org/gitlab/-/issues/291043 + */ + isLoading: { + type: Boolean, + required: false, + default: false, + }, + projectId: { + type: Number, + required: false, + default: -1, + }, }, computed: { tooltipText() { @@ -74,6 +84,9 @@ export default { } return __('Multi-project'); }, + pipelineIsLoading() { + return Boolean(this.isLoading || this.pipeline.isLoading); + }, isDownstream() { return this.type === DOWNSTREAM; }, @@ -81,7 +94,9 @@ export default { return this.type === UPSTREAM; }, isSameProject() { - return this.projectId === this.pipeline.project.id; + return this.projectId > -1 + ? this.projectId === this.pipeline.project.id + : !this.pipeline.multiproject; }, sourceJobName() { return accessValue(this.dataMethod, 'sourceJob', this.pipeline); @@ -101,16 +116,15 @@ export default { }, methods: { onClickLinkedPipeline() { - this.$root.$emit('bv::hide::tooltip', this.buttonId); - this.expanded = !this.expanded; + this.hideTooltips(); this.$emit('pipelineClicked', this.$refs.linkedPipeline); - this.$emit('pipelineExpandToggle', this.pipeline.source_job.name, this.expanded); + this.$emit('pipelineExpandToggle', this.sourceJobName, !this.expanded); }, hideTooltips() { this.$root.$emit('bv::hide::tooltip'); }, onDownstreamHovered() { - this.$emit('downstreamHovered', this.pipeline.source_job.name); + this.$emit('downstreamHovered', this.sourceJobName); }, onDownstreamHoverLeave() { this.$emit('downstreamHovered', ''); @@ -120,10 +134,10 @@ export default { </script> <template> - <li + <div ref="linkedPipeline" v-gl-tooltip - class="linked-pipeline build" + class="linked-pipeline build gl-pipeline-job-width" :title="tooltipText" :class="{ 'downstream-pipeline': isDownstream }" data-qa-selector="child_pipeline" @@ -136,8 +150,9 @@ export default { > <div class="gl-display-flex"> <ci-status - v-if="!pipeline.isLoading" + v-if="!pipelineIsLoading" :status="pipelineStatus" + :size="24" css-classes="gl-top-0 gl-pr-2" /> <div v-else class="gl-pr-2"><gl-loading-icon inline /></div> @@ -160,10 +175,10 @@ export default { class="gl-absolute gl-top-0 gl-bottom-0 gl-shadow-none! gl-rounded-0!" :class="`js-pipeline-expand-${pipeline.id} ${expandButtonPosition}`" :icon="expandedIcon" - data-testid="expandPipelineButton" + data-testid="expand-pipeline-button" data-qa-selector="expand_pipeline_button" @click="onClickLinkedPipeline" /> </div> - </li> + </div> </template> diff --git a/app/assets/javascripts/pipelines/components/graph/linked_pipelines_column.vue b/app/assets/javascripts/pipelines/components/graph/linked_pipelines_column.vue index 2ca33e6d33e..58757a88102 100644 --- a/app/assets/javascripts/pipelines/components/graph/linked_pipelines_column.vue +++ b/app/assets/javascripts/pipelines/components/graph/linked_pipelines_column.vue @@ -1,10 +1,14 @@ <script> +import getPipelineDetails from '../../graphql/queries/get_pipeline_details.query.graphql'; import LinkedPipeline from './linked_pipeline.vue'; +import { LOAD_FAILURE } from '../../constants'; import { UPSTREAM } from './constants'; +import { unwrapPipelineData } from './utils'; export default { components: { LinkedPipeline, + PipelineGraph: () => import('./graph_component.vue'), }, props: { columnTitle: { @@ -19,11 +23,22 @@ export default { type: String, required: true, }, - projectId: { - type: Number, - required: true, - }, }, + data() { + return { + currentPipeline: null, + loadingPipelineId: null, + pipelineExpanded: false, + }; + }, + titleClasses: [ + 'gl-font-weight-bold', + 'gl-pipeline-job-width', + 'gl-text-truncate', + 'gl-line-height-36', + 'gl-pl-3', + 'gl-mb-5', + ], computed: { columnClass() { const positionValues = { @@ -35,14 +50,66 @@ export default { graphPosition() { return this.isUpstream ? 'left' : 'right'; }, - // Refactor string match when BE returns Upstream/Downstream indicators isUpstream() { return this.type === UPSTREAM; }, + computedTitleClasses() { + const positionalClasses = this.isUpstream + ? ['gl-w-full', 'gl-text-right', 'gl-linked-pipeline-padding'] + : []; + + return [...this.$options.titleClasses, ...positionalClasses]; + }, }, methods: { - onPipelineClick(downstreamNode, pipeline, index) { - this.$emit('linkedPipelineClick', pipeline, index, downstreamNode); + getPipelineData(pipeline) { + const projectPath = pipeline.project.fullPath; + + this.$apollo.addSmartQuery('currentPipeline', { + query: getPipelineDetails, + variables() { + return { + projectPath, + iid: pipeline.iid, + }; + }, + update(data) { + return unwrapPipelineData(projectPath, data); + }, + result() { + this.loadingPipelineId = null; + }, + error() { + this.$emit('error', LOAD_FAILURE); + }, + }); + }, + isExpanded(id) { + return Boolean(this.currentPipeline?.id && id === this.currentPipeline.id); + }, + isLoadingPipeline(id) { + return this.loadingPipelineId === id; + }, + onPipelineClick(pipeline) { + /* If the clicked pipeline has been expanded already, close it, clear, exit */ + if (this.currentPipeline?.id === pipeline.id) { + this.pipelineExpanded = false; + this.currentPipeline = null; + return; + } + + /* Set the loading id */ + this.loadingPipelineId = pipeline.id; + + /* + Expand the pipeline. + If this was not a toggle close action, and + it was already showing a different pipeline, then + this will be a no-op, but that doesn't matter. + */ + this.pipelineExpanded = true; + + this.getPipelineData(pipeline); }, onDownstreamHovered(jobName) { this.$emit('downstreamHovered', jobName); @@ -60,25 +127,40 @@ export default { </script> <template> - <div :class="columnClass" class="stage-column linked-pipelines-column"> - <div class="stage-name linked-pipelines-column-title">{{ columnTitle }}</div> - <div v-if="isUpstream" class="cross-project-triangle"></div> - <ul> - <linked-pipeline - v-for="(pipeline, index) in linkedPipelines" - :key="pipeline.id" - :class="{ - active: pipeline.isExpanded, - 'left-connector': pipeline.isExpanded && graphPosition === 'left', - }" - :pipeline="pipeline" - :column-title="columnTitle" - :project-id="projectId" - :type="type" - @pipelineClicked="onPipelineClick($event, pipeline, index)" - @downstreamHovered="onDownstreamHovered" - @pipelineExpandToggle="onPipelineExpandToggle" - /> - </ul> + <div class="gl-display-flex"> + <div :class="columnClass" class="linked-pipelines-column"> + <div data-testid="linked-column-title" class="stage-name" :class="computedTitleClasses"> + {{ columnTitle }} + </div> + <ul class="gl-pl-0"> + <li + v-for="pipeline in linkedPipelines" + :key="pipeline.id" + class="gl-display-flex gl-mb-4" + :class="{ 'gl-flex-direction-row-reverse': isUpstream }" + > + <linked-pipeline + class="gl-display-inline-block" + :is-loading="isLoadingPipeline(pipeline.id)" + :pipeline="pipeline" + :column-title="columnTitle" + :type="type" + :expanded="isExpanded(pipeline.id)" + @downstreamHovered="onDownstreamHovered" + @pipelineClicked="onPipelineClick(pipeline)" + @pipelineExpandToggle="onPipelineExpandToggle" + /> + <div v-if="isExpanded(pipeline.id)" class="gl-display-inline-block"> + <pipeline-graph + v-if="currentPipeline" + :type="type" + class="d-inline-block gl-mt-n2" + :pipeline="currentPipeline" + :is-linked-pipeline="true" + /> + </div> + </li> + </ul> + </div> </div> </template> diff --git a/app/assets/javascripts/pipelines/components/graph/linked_pipelines_column_legacy.vue b/app/assets/javascripts/pipelines/components/graph/linked_pipelines_column_legacy.vue index 2ca33e6d33e..7d371b33220 100644 --- a/app/assets/javascripts/pipelines/components/graph/linked_pipelines_column_legacy.vue +++ b/app/assets/javascripts/pipelines/components/graph/linked_pipelines_column_legacy.vue @@ -35,7 +35,9 @@ export default { graphPosition() { return this.isUpstream ? 'left' : 'right'; }, - // Refactor string match when BE returns Upstream/Downstream indicators + isExpanded() { + return this.pipeline?.isExpanded || false; + }, isUpstream() { return this.type === UPSTREAM; }, @@ -64,21 +66,22 @@ export default { <div class="stage-name linked-pipelines-column-title">{{ columnTitle }}</div> <div v-if="isUpstream" class="cross-project-triangle"></div> <ul> - <linked-pipeline - v-for="(pipeline, index) in linkedPipelines" - :key="pipeline.id" - :class="{ - active: pipeline.isExpanded, - 'left-connector': pipeline.isExpanded && graphPosition === 'left', - }" - :pipeline="pipeline" - :column-title="columnTitle" - :project-id="projectId" - :type="type" - @pipelineClicked="onPipelineClick($event, pipeline, index)" - @downstreamHovered="onDownstreamHovered" - @pipelineExpandToggle="onPipelineExpandToggle" - /> + <li v-for="(pipeline, index) in linkedPipelines" :key="pipeline.id"> + <linked-pipeline + :class="{ + active: pipeline.isExpanded, + 'left-connector': pipeline.isExpanded && graphPosition === 'left', + }" + :pipeline="pipeline" + :column-title="columnTitle" + :project-id="projectId" + :type="type" + :expanded="isExpanded" + @pipelineClicked="onPipelineClick($event, pipeline, index)" + @downstreamHovered="onDownstreamHovered" + @pipelineExpandToggle="onPipelineExpandToggle" + /> + </li> </ul> </div> </template> diff --git a/app/assets/javascripts/pipelines/components/graph/utils.js b/app/assets/javascripts/pipelines/components/graph/utils.js index df3615772ce..7bf44b160ef 100644 --- a/app/assets/javascripts/pipelines/components/graph/utils.js +++ b/app/assets/javascripts/pipelines/components/graph/utils.js @@ -1,28 +1,42 @@ +import { getIdFromGraphQLId } from '~/graphql_shared/utils'; import { unwrapStagesWithNeeds } from '../unwrapping_utils'; -const addMulti = (mainId, pipeline) => { - return { ...pipeline, multiproject: mainId !== pipeline.id }; +const addMulti = (mainPipelineProjectPath, linkedPipeline) => { + return { + ...linkedPipeline, + multiproject: mainPipelineProjectPath !== linkedPipeline.project.fullPath, + }; }; -const unwrapPipelineData = (mainPipelineId, data) => { +const transformId = linkedPipeline => { + return { ...linkedPipeline, id: getIdFromGraphQLId(linkedPipeline.id) }; +}; + +const unwrapPipelineData = (mainPipelineProjectPath, data) => { if (!data?.project?.pipeline) { return null; } + const { pipeline } = data.project; + const { - id, upstream, downstream, stages: { nodes: stages }, - } = data.project.pipeline; + } = pipeline; const nodes = unwrapStagesWithNeeds(stages); return { - id, + ...pipeline, + id: getIdFromGraphQLId(pipeline.id), stages: nodes, - upstream: upstream ? [upstream].map(addMulti.bind(null, mainPipelineId)) : [], - downstream: downstream ? downstream.map(addMulti.bind(null, mainPipelineId)) : [], + upstream: upstream + ? [upstream].map(addMulti.bind(null, mainPipelineProjectPath)).map(transformId) + : [], + downstream: downstream + ? downstream.nodes.map(addMulti.bind(null, mainPipelineProjectPath)).map(transformId) + : [], }; }; diff --git a/app/assets/javascripts/pipelines/components/graph_shared/linked_graph_wrapper.vue b/app/assets/javascripts/pipelines/components/graph_shared/linked_graph_wrapper.vue new file mode 100644 index 00000000000..fb2280d971a --- /dev/null +++ b/app/assets/javascripts/pipelines/components/graph_shared/linked_graph_wrapper.vue @@ -0,0 +1,7 @@ +<template> + <div class="gl-display-flex"> + <slot name="upstream"></slot> + <slot name="main"></slot> + <slot name="downstream"></slot> + </div> +</template> diff --git a/app/assets/javascripts/pipelines/components/graph_shared/main_graph_wrapper.vue b/app/assets/javascripts/pipelines/components/graph_shared/main_graph_wrapper.vue index 205ee0fb414..1c9e3236d56 100644 --- a/app/assets/javascripts/pipelines/components/graph_shared/main_graph_wrapper.vue +++ b/app/assets/javascripts/pipelines/components/graph_shared/main_graph_wrapper.vue @@ -17,7 +17,7 @@ export default { <template> <div> <div - class="gl-display-flex gl-align-items-center gl-w-full gl-px-8 gl-py-4 gl-mb-5" + class="gl-display-flex gl-align-items-center gl-w-full gl-px-8 gl-mb-5" :class="stageClasses" > <slot name="stages"> </slot> diff --git a/app/assets/javascripts/pipelines/graphql/fragments/linked_pipelines.fragment.graphql b/app/assets/javascripts/pipelines/graphql/fragments/linked_pipelines.fragment.graphql new file mode 100644 index 00000000000..3bf6d8dc9d8 --- /dev/null +++ b/app/assets/javascripts/pipelines/graphql/fragments/linked_pipelines.fragment.graphql @@ -0,0 +1,17 @@ +fragment LinkedPipelineData on Pipeline { + id + iid + path + status: detailedStatus { + group + label + icon + } + sourceJob { + name + } + project { + name + fullPath + } +} diff --git a/app/assets/javascripts/pipelines/graphql/queries/get_pipeline_details.query.graphql b/app/assets/javascripts/pipelines/graphql/queries/get_pipeline_details.query.graphql index 6d80e7b8d51..25aede49631 100644 --- a/app/assets/javascripts/pipelines/graphql/queries/get_pipeline_details.query.graphql +++ b/app/assets/javascripts/pipelines/graphql/queries/get_pipeline_details.query.graphql @@ -1,7 +1,18 @@ +#import "../fragments/linked_pipelines.fragment.graphql" + query getPipelineDetails($projectPath: ID!, $iid: ID!) { project(fullPath: $projectPath) { pipeline(iid: $iid) { - id: iid + id + iid + downstream { + nodes { + ...LinkedPipelineData + } + } + upstream { + ...LinkedPipelineData + } stages { nodes { name diff --git a/app/assets/javascripts/pipelines/graphql/queries/get_pipeline_header_data.query.graphql b/app/assets/javascripts/pipelines/graphql/queries/get_pipeline_header_data.query.graphql index 06083daeca0..1b3f80b1f18 100644 --- a/app/assets/javascripts/pipelines/graphql/queries/get_pipeline_header_data.query.graphql +++ b/app/assets/javascripts/pipelines/graphql/queries/get_pipeline_header_data.query.graphql @@ -2,6 +2,7 @@ query getPipelineHeaderData($fullPath: ID!, $iid: ID!) { project(fullPath: $fullPath) { pipeline(iid: $iid) { id + iid status retryable cancelable diff --git a/app/assets/javascripts/static_site_editor/constants.js b/app/assets/javascripts/static_site_editor/constants.js index faa4026c064..d6a54176a3b 100644 --- a/app/assets/javascripts/static_site_editor/constants.js +++ b/app/assets/javascripts/static_site_editor/constants.js @@ -21,4 +21,7 @@ export const TRACKING_ACTION_CREATE_COMMIT = 'create_commit'; export const TRACKING_ACTION_CREATE_MERGE_REQUEST = 'create_merge_request'; export const TRACKING_ACTION_INITIALIZE_EDITOR = 'initialize_editor'; +export const USAGE_PING_TRACKING_ACTION_CREATE_COMMIT = 'static_site_editor_commits'; +export const USAGE_PING_TRACKING_ACTION_CREATE_MERGE_REQUEST = 'static_site_editor_merge_requests'; + export const MR_META_LOCAL_STORAGE_KEY = 'sse-merge-request-meta-storage-key'; diff --git a/app/assets/javascripts/static_site_editor/services/submit_content_changes.js b/app/assets/javascripts/static_site_editor/services/submit_content_changes.js index 8623a671a7d..e7aeb73e88b 100644 --- a/app/assets/javascripts/static_site_editor/services/submit_content_changes.js +++ b/app/assets/javascripts/static_site_editor/services/submit_content_changes.js @@ -10,6 +10,8 @@ import { SUBMIT_CHANGES_MERGE_REQUEST_ERROR, TRACKING_ACTION_CREATE_COMMIT, TRACKING_ACTION_CREATE_MERGE_REQUEST, + USAGE_PING_TRACKING_ACTION_CREATE_COMMIT, + USAGE_PING_TRACKING_ACTION_CREATE_MERGE_REQUEST, } from '../constants'; const createBranch = (projectId, branch) => @@ -47,6 +49,7 @@ const createImageActions = (images, markdown) => { const commitContent = (projectId, message, branch, sourcePath, content, images) => { Tracking.event(document.body.dataset.page, TRACKING_ACTION_CREATE_COMMIT); + Api.trackRedisCounterEvent(USAGE_PING_TRACKING_ACTION_CREATE_COMMIT); return Api.commitMultiple( projectId, @@ -75,6 +78,7 @@ const createMergeRequest = ( targetBranch = DEFAULT_TARGET_BRANCH, ) => { Tracking.event(document.body.dataset.page, TRACKING_ACTION_CREATE_MERGE_REQUEST); + Api.trackRedisCounterEvent(USAGE_PING_TRACKING_ACTION_CREATE_MERGE_REQUEST); return Api.createProjectMergeRequest( projectId, diff --git a/app/assets/stylesheets/page_bundles/pipeline.scss b/app/assets/stylesheets/page_bundles/pipeline.scss index 7b424882ffa..6b70cda4891 100644 --- a/app/assets/stylesheets/page_bundles/pipeline.scss +++ b/app/assets/stylesheets/page_bundles/pipeline.scss @@ -139,6 +139,10 @@ width: 186px; } +.gl-linked-pipeline-padding { + padding-right: 120px; +} + .gl-build-content { @include build-content(); } diff --git a/app/models/namespace_onboarding_action.rb b/app/models/namespace_onboarding_action.rb index e1121279e2e..bf4df7de13f 100644 --- a/app/models/namespace_onboarding_action.rb +++ b/app/models/namespace_onboarding_action.rb @@ -8,6 +8,7 @@ class NamespaceOnboardingAction < ApplicationRecord ACTIONS = { subscription_created: 1, git_write: 2, + merge_request_created: 3, git_read: 4 }.freeze diff --git a/app/services/merge_requests/after_create_service.rb b/app/services/merge_requests/after_create_service.rb index f0c85ae03c9..fbb9d5fa9dc 100644 --- a/app/services/merge_requests/after_create_service.rb +++ b/app/services/merge_requests/after_create_service.rb @@ -11,6 +11,8 @@ module MergeRequests merge_request.diffs(include_stats: false).write_cache merge_request.create_cross_references!(current_user) + + NamespaceOnboardingAction.create_action(merge_request.target_project.namespace, :merge_request_created) end end end diff --git a/app/validators/json_schemas/vulnerability_finding_details.json b/app/validators/json_schemas/vulnerability_finding_details.json index 8b44ac62dfc..f2940866f4b 100644 --- a/app/validators/json_schemas/vulnerability_finding_details.json +++ b/app/validators/json_schemas/vulnerability_finding_details.json @@ -1,5 +1,182 @@ { "type": "object", "description": "The schema for vulnerability finding details", - "additionalProperties": false + "additionalProperties": false, + "patternProperties": { + "^.*$": { + "allOf": [ + { "$ref": "#/definitions/named_field" }, + { "$ref": "#/definitions/type_list" } + ] + } + }, + "definitions": { + "type_list": { + "oneOf": [ + { "$ref": "#/definitions/named_list" }, + { "$ref": "#/definitions/list" }, + { "$ref": "#/definitions/table" }, + + { "$ref": "#/definitions/text" }, + { "$ref": "#/definitions/url" }, + { "$ref": "#/definitions/code" }, + { "$ref": "#/definitions/int" }, + + { "$ref": "#/definitions/commit" }, + { "$ref": "#/definitions/file_location" }, + { "$ref": "#/definitions/module_location" } + ] + }, + "lang_text": { + "type": "object", + "required": [ "value", "lang" ], + "properties": { + "lang": { "type": "string" }, + "value": { "type": "string" } + } + }, + "lang_text_list": { + "type": "array", + "items": { "$ref": "#/definitions/lang_text" } + }, + "named_field": { + "type": "object", + "required": [ "name" ], + "properties": { + "name": { "$ref": "#/definitions/lang_text_list" }, + "description": { "$ref": "#/definitions/lang_text_list" } + } + }, + "named_list": { + "type": "object", + "description": "An object with named and typed fields", + "required": [ "type", "items" ], + "properties": { + "type": { "const": "named-list" }, + "items": { + "type": "object", + "patternProperties": { + "^.*$": { + "allOf": [ + { "$ref": "#/definitions/named_field" }, + { "$ref": "#/definitions/type_list" } + ] + } + } + } + } + }, + "list": { + "type": "object", + "description": "A list of typed fields", + "required": [ "type", "items" ], + "properties": { + "type": { "const": "list" }, + "items": { + "type": "array", + "items": { "$ref": "#/definitions/type_list" } + } + } + }, + "table": { + "type": "object", + "description": "A table of typed fields", + "required": [], + "properties": { + "type": { "const": "table" }, + "items": { + "type": "object", + "properties": { + "header": { + "type": "array", + "items": { + "$ref": "#/definitions/type_list" + } + }, + "rows": { + "type": "array", + "items": { + "type": "array", + "items": { + "$ref": "#/definitions/type_list" + } + } + } + } + } + } + }, + "text": { + "type": "object", + "description": "Raw text", + "required": [ "type", "value" ], + "properties": { + "type": { "const": "text" }, + "value": { "$ref": "#/definitions/lang_text_list" } + } + }, + "url": { + "type": "object", + "description": "A single URL", + "required": [ "type", "href" ], + "properties": { + "type": { "const": "url" }, + "text": { "$ref": "#/definitions/lang_text_list" }, + "href": { "type": "string" } + } + }, + "code": { + "type": "object", + "description": "A codeblock", + "required": [ "type", "value" ], + "properties": { + "type": { "const": "code" }, + "value": { "type": "string" }, + "lang": { "type": "string" } + } + }, + "int": { + "type": "object", + "description": "An integer", + "required": [ "type", "value" ], + "properties": { + "type": { "const": "int" }, + "value": { "type": "integer" }, + "format": { + "type": "string", + "enum": [ "default", "hex" ] + } + } + }, + "commit": { + "type": "object", + "description": "A specific commit within the project", + "required": [ "type", "value" ], + "properties": { + "type": { "const": "commit" }, + "value": { "type": "string", "description": "The commit SHA" } + } + }, + "file_location": { + "type": "object", + "description": "A location within a file in the project", + "required": [ "type", "file_name", "line_start" ], + "properties": { + "type": { "const": "file-location" }, + "file_name": { "type": "string" }, + "line_start": { "type": "integer" }, + "line_end": { "type": "integer" } + } + }, + "module_location": { + "type": "object", + "description": "A location within a binary module of the form module+relative_offset", + "required": [ "type", "module_name", "offset" ], + "properties": { + "type": { "const": "module-location" }, + "module_name": { "type": "string" }, + "offset": { "type": "integer" } + } + } + } } diff --git a/app/views/layouts/nav/groups_dropdown/_show.html.haml b/app/views/layouts/nav/groups_dropdown/_show.html.haml index 3ce1fa6bcca..d0394451a61 100644 --- a/app/views/layouts/nav/groups_dropdown/_show.html.haml +++ b/app/views/layouts/nav/groups_dropdown/_show.html.haml @@ -3,10 +3,10 @@ .frequent-items-dropdown-sidebar.qa-groups-dropdown-sidebar %ul = nav_link(path: 'dashboard/groups#index') do - = link_to dashboard_groups_path, class: 'qa-your-groups-link' do + = link_to dashboard_groups_path, class: 'qa-your-groups-link', data: { track_label: "groups_dropdown_your_groups", track_event: "click_link" } do = _('Your groups') = nav_link(path: 'groups#explore') do - = link_to explore_groups_path do + = link_to explore_groups_path, data: { track_label: "groups_dropdown_explore_groups", track_event: "click_link" } do = _('Explore groups') .frequent-items-dropdown-content #js-groups-dropdown{ data: { user_name: current_user.username, group: group_meta } } diff --git a/app/views/layouts/nav/projects_dropdown/_show.html.haml b/app/views/layouts/nav/projects_dropdown/_show.html.haml index f2170f71532..91f999a9a74 100644 --- a/app/views/layouts/nav/projects_dropdown/_show.html.haml +++ b/app/views/layouts/nav/projects_dropdown/_show.html.haml @@ -3,13 +3,13 @@ .frequent-items-dropdown-sidebar.qa-projects-dropdown-sidebar %ul = nav_link(path: 'dashboard/projects#index') do - = link_to dashboard_projects_path, class: 'qa-your-projects-link' do + = link_to dashboard_projects_path, class: 'qa-your-projects-link', data: { track_label: "projects_dropdown_your_projects", track_event: "click_link" } do = _('Your projects') = nav_link(path: 'projects#starred') do - = link_to starred_dashboard_projects_path do + = link_to starred_dashboard_projects_path, data: { track_label: "projects_dropdown_starred_projects", track_event: "click_link" } do = _('Starred projects') = nav_link(path: 'projects#trending') do - = link_to explore_root_path do + = link_to explore_root_path, data: { track_label: "projects_dropdown_explore_projects", track_event: "click_link" } do = _('Explore projects') .frequent-items-dropdown-content #js-projects-dropdown{ data: { user_name: current_user.username, project: project_meta } } diff --git a/changelogs/unreleased/233994_send_usage_data_events.yml b/changelogs/unreleased/233994_send_usage_data_events.yml new file mode 100644 index 00000000000..38707c88b94 --- /dev/null +++ b/changelogs/unreleased/233994_send_usage_data_events.yml @@ -0,0 +1,5 @@ +--- +title: Send Static Site Editor events to Usage Ping API +merge_request: 47640 +author: +type: added diff --git a/changelogs/unreleased/use-json-pretty_generate_in_usage_data_rake_tasks.yml b/changelogs/unreleased/use-json-pretty_generate_in_usage_data_rake_tasks.yml new file mode 100644 index 00000000000..05d6973aac6 --- /dev/null +++ b/changelogs/unreleased/use-json-pretty_generate_in_usage_data_rake_tasks.yml @@ -0,0 +1,5 @@ +--- +title: Add usage data rake tasks to prettify JSON output +merge_request: 49137 +author: +type: added diff --git a/doc/development/dangerbot.md b/doc/development/dangerbot.md index 3e71d5b5aa5..d739fb399bc 100644 --- a/doc/development/dangerbot.md +++ b/doc/development/dangerbot.md @@ -10,7 +10,7 @@ The GitLab CI/CD pipeline includes a `danger-review` job that uses [Danger](http to perform a variety of automated checks on the code under test. Danger is a gem that runs in the CI environment, like any other analysis tool. -What sets it apart from, e.g., RuboCop, is that it's designed to allow you to +What sets it apart from (for example, RuboCop) is that it's designed to allow you to easily write arbitrary code to test properties of your code or changes. To this end, it provides a set of common helpers and access to information about what has actually changed in your environment, then simply runs your code! @@ -32,7 +32,7 @@ from the start of the merge request. ### Disadvantages -- It's not obvious Danger will update the old comment, thus you need to +- It's not obvious Danger updates the old comment, thus you need to pay attention to it if it is updated or not. ## Run Danger locally @@ -48,13 +48,12 @@ bin/rake danger_local On startup, Danger reads a [`Dangerfile`](https://gitlab.com/gitlab-org/gitlab/blob/master/Dangerfile) from the project root. GitLab's Danger code is decomposed into a set of helpers and plugins, all within the [`danger/`](https://gitlab.com/gitlab-org/gitlab-foss/tree/master/danger/) -subdirectory, so ours just tells Danger to load it all. Danger will then run +subdirectory, so ours just tells Danger to load it all. Danger then runs each plugin against the merge request, collecting the output from each. A plugin may output notifications, warnings, or errors, all of which are copied to the -CI job's log. If an error happens, the CI job (and so the entire pipeline) will -be failed. +CI job's log. If an error happens, the CI job (and so the entire pipeline) fails. -On merge requests, Danger will also copy the output to a comment on the MR +On merge requests, Danger also copies the output to a comment on the MR itself, increasing visibility. ## Development guidelines @@ -75,17 +74,17 @@ often face similar challenges, after all. Think about how you could fulfill the same need while ensuring everyone can benefit from the work, and do that instead if you can. -If a standard tool (e.g. `rubocop`) exists for a task, it is better to use it -directly, rather than calling it via Danger. Running and debugging the results -of those tools locally is easier if Danger isn't involved, and unless you're -using some Danger-specific functionality, there's no benefit to including it in -the Danger run. +If a standard tool (for example, `rubocop`) exists for a task, it's better to +use it directly, rather than calling it by using Danger. Running and debugging +the results of those tools locally is easier if Danger isn't involved, and +unless you're using some Danger-specific functionality, there's no benefit to +including it in the Danger run. Danger is well-suited to prototyping and rapidly iterating on solutions, so if what we want to build is unclear, a solution in Danger can be thought of as a trial run to gather information about a product area. If you're doing this, make sure the problem you're trying to solve, and the outcomes of that prototyping, -are captured in an issue or epic as you go along. This will help us to address +are captured in an issue or epic as you go along. This helps us to address the need as part of the product in a future version of GitLab! ### Implementation details @@ -110,16 +109,17 @@ At present, we do this by putting the code in a module in `lib/gitlab/danger/... and including it in the matching `danger/plugins/...` file. Specs can then be added in `spec/lib/gitlab/danger/...`. -You'll only know if your `Dangerfile` works by pushing the branch that contains -it to GitLab. This can be quite frustrating, as it significantly increases the -cycle time when developing a new task, or trying to debug something in an -existing one. If you've followed the guidelines above, most of your code can -be exercised locally in RSpec, minimizing the number of cycles you need to go -through in CI. However, you can speed these cycles up somewhat by emptying the +To determine if your `Dangerfile` works, push the branch that contains it to +GitLab. This can be quite frustrating, as it significantly increases the cycle +time when developing a new task, or trying to debug something in an existing +one. If you've followed the guidelines above, most of your code can be exercised +locally in RSpec, minimizing the number of cycles you need to go through in CI. +However, you can speed these cycles up somewhat by emptying the `.gitlab/ci/rails.gitlab-ci.yml` file in your merge request. Just don't forget to revert the change before merging! -To enable the Dangerfile on another existing GitLab project, run the following extra steps, based on [this procedure](https://danger.systems/guides/getting_started.html#creating-a-bot-account-for-danger-to-use): +To enable the Dangerfile on another existing GitLab project, run the following +extra steps, based on [this procedure](https://danger.systems/guides/getting_started.html#creating-a-bot-account-for-danger-to-use): 1. Add `@gitlab-bot` to the project as a `reporter`. 1. Add the `@gitlab-bot`'s `GITLAB_API_PRIVATE_TOKEN` value as a value for a new CI/CD @@ -156,10 +156,10 @@ at GitLab so far: To work around this, you can add an [environment variable](../ci/variables/README.md) called `DANGER_GITLAB_API_TOKEN` with a personal API token to your - fork. That way the danger comments will be made from CI using that + fork. That way the danger comments are made from CI using that API token instead. Making the variable - [masked](../ci/variables/README.md#mask-a-custom-variable) will make sure + [masked](../ci/variables/README.md#mask-a-custom-variable) makes sure it doesn't show up in the job logs. The variable cannot be [protected](../ci/variables/README.md#protect-a-custom-variable), as it needs to be present for all feature branches. diff --git a/doc/development/documentation/workflow.md b/doc/development/documentation/workflow.md index 17a2a575c07..698512e149d 100644 --- a/doc/development/documentation/workflow.md +++ b/doc/development/documentation/workflow.md @@ -146,7 +146,7 @@ Remember: advance of a milestone release and for larger documentation changes. - You can request a post-merge Technical Writer review of documentation if it's important to get the code with which it ships merged as soon as possible. In this case, the author of the original MR - will address the feedback provided by the Technical Writer in a follow-up MR. + can address the feedback provided by the Technical Writer in a follow-up MR. - The Technical Writer can also help decide that documentation can be merged without Technical writer review, with the review to occur soon after merge. diff --git a/doc/development/ee_features.md b/doc/development/ee_features.md index 0ba523baba5..5be601187ca 100644 --- a/doc/development/ee_features.md +++ b/doc/development/ee_features.md @@ -143,7 +143,7 @@ There are a few gotchas with it: - you should always [`extend ::Gitlab::Utils::Override`](utilities.md#override) and use `override` to guard the "overrider" method to ensure that if the method gets renamed in - CE, the EE override won't be silently forgotten. + CE, the EE override isn't silently forgotten. - when the "overrider" would add a line in the middle of the CE implementation, you should refactor the CE method and split it in smaller methods. Or create a "hook" method that is empty in CE, @@ -284,7 +284,7 @@ wrap it in a self-descriptive method and use that method. For example, in GitLab-FOSS, the only user created by the system is `User.ghost` but in EE there are several types of bot-users that aren't really users. It would be incorrect to override the implementation of `User#ghost?`, so instead we add -a method `#internal?` to `app/models/user.rb`. The implementation will be: +a method `#internal?` to `app/models/user.rb`. The implementation: ```ruby def internal? @@ -303,13 +303,13 @@ end ### Code in `config/routes` -When we add `draw :admin` in `config/routes.rb`, the application will try to +When we add `draw :admin` in `config/routes.rb`, the application tries to load the file located in `config/routes/admin.rb`, and also try to load the file located in `ee/config/routes/admin.rb`. In EE, it should at least load one file, at most two files. If it cannot find -any files, an error will be raised. In CE, since we don't know if there will -be an EE route, it will not raise any errors even if it cannot find anything. +any files, an error is raised. In CE, since we don't know if an +an EE route exists, it doesn't raise any errors even if it cannot find anything. This means if we want to extend a particular CE route file, just add the same file located in `ee/config/routes`. If we want to add an EE only route, we @@ -467,7 +467,7 @@ end #### Using `render_if_exists` Instead of using regular `render`, we should use `render_if_exists`, which -will not render anything if it cannot find the specific partial. We use this +doesn't render anything if it cannot find the specific partial. We use this so that we could put `render_if_exists` in CE, keeping code the same between CE and EE. @@ -482,7 +482,7 @@ The disadvantage of this: ##### Caveats The `render_if_exists` view path argument must be relative to `app/views/` and `ee/app/views`. -Resolving an EE template path that is relative to the CE view path will not work. +Resolving an EE template path that is relative to the CE view path doesn't work. ```haml - # app/views/projects/index.html.haml @@ -577,7 +577,7 @@ We can define `params` and use `use` in another `params` definition to include parameters defined in EE. However, we need to define the "interface" first in CE in order for EE to override it. We don't have to do this in other places due to `prepend_if_ee`, but Grape is complex internally and we couldn't easily -do that, so we'll follow regular object-oriented practices that we define the +do that, so we follow regular object-oriented practices that we define the interface first here. For example, suppose we have a few more optional parameters for EE. We can move the @@ -738,7 +738,7 @@ end It's very hard to extend this in an EE module, and this is simply storing some meta-data for a particular route. Given that, we could simply leave the -EE `route_setting` in CE as it won't hurt and we are just not going to use +EE `route_setting` in CE as it doesn't hurt and we don't use those meta-data in CE. We could revisit this policy when we're using `route_setting` more and whether @@ -1039,7 +1039,7 @@ export default { `import MyComponent from 'ee_else_ce/path/my_component'.vue` -- this way the correct component will be included for either the ce or ee implementation +- this way the correct component is included for either the CE or EE implementation **For EE components that need different results for the same computed values, we can pass in props to the CE wrapper as seen in the example.** @@ -1053,7 +1053,7 @@ export default { For regular JS files, the approach is similar. -1. We will keep using the [`ee_else_ce`](../development/ee_features.md#javascript-code-in-assetsjavascripts) helper, this means that EE only code should be inside the `ee/` folder. +1. We keep using the [`ee_else_ce`](../development/ee_features.md#javascript-code-in-assetsjavascripts) helper, this means that EE only code should be inside the `ee/` folder. 1. An EE file should be created with the EE only code, and it should extend the CE counterpart. 1. For code inside functions that can't be extended, the code should be moved into a new file and we should use `ee_else_ce` helper: diff --git a/doc/development/file_storage.md b/doc/development/file_storage.md index f45441a3d93..1f929d64058 100644 --- a/doc/development/file_storage.md +++ b/doc/development/file_storage.md @@ -93,7 +93,7 @@ All the `GitlabUploader` derived classes should comply with this path segment sc | | | `ObjectStorage::Concern#upload_path | ``` -The `RecordsUploads::Concern` concern will create an `Upload` entry for every file stored by a `GitlabUploader` persisting the dynamic parts of the path using +The `RecordsUploads::Concern` concern creates an `Upload` entry for every file stored by a `GitlabUploader` persisting the dynamic parts of the path using `GitlabUploader#dynamic_path`. You may then use the `Upload#build_uploader` method to manipulate the file. ## Object Storage @@ -108,9 +108,9 @@ The `CarrierWave::Uploader#store_dir` is overridden to ### Using `ObjectStorage::Extension::RecordsUploads` -This concern will automatically include `RecordsUploads::Concern` if not already included. +This concern includes `RecordsUploads::Concern` if not already included. -The `ObjectStorage::Concern` uploader will search for the matching `Upload` to select the correct object store. The `Upload` is mapped using `#store_dirs + identifier` for each store (LOCAL/REMOTE). +The `ObjectStorage::Concern` uploader searches for the matching `Upload` to select the correct object store. The `Upload` is mapped using `#store_dirs + identifier` for each store (LOCAL/REMOTE). ```ruby class SongUploader < GitlabUploader @@ -130,7 +130,7 @@ end ### Using a mounted uploader -The `ObjectStorage::Concern` will query the `model.<mount>_store` attribute to select the correct object store. +The `ObjectStorage::Concern` queries the `model.<mount>_store` attribute to select the correct object store. This column must be present in the model schema. ```ruby diff --git a/doc/development/gotchas.md b/doc/development/gotchas.md index 3d89a3a6c0a..2f8ff826358 100644 --- a/doc/development/gotchas.md +++ b/doc/development/gotchas.md @@ -14,7 +14,7 @@ might encounter or should avoid during development of GitLab CE and EE. In GitLab 10.8 and later, Omnibus has [dropped the `app/assets` directory](https://gitlab.com/gitlab-org/omnibus-gitlab/-/merge_requests/2456), after asset compilation. The `ee/app/assets`, `vendor/assets` directories are dropped as well. -This means that reading files from that directory will fail in Omnibus-installed GitLab instances: +This means that reading files from that directory fails in Omnibus-installed GitLab instances: ```ruby file = Rails.root.join('app/assets/images/logo.svg') @@ -243,8 +243,8 @@ end In this case, if for any reason the top level `ApplicationController` is loaded but `Projects::ApplicationController` is not, `ApplicationController` -would be resolved to `::ApplicationController` and then the `project` method will -be undefined and we will get an error. +would be resolved to `::ApplicationController` and then the `project` method is +undefined, causing an error. #### Solution @@ -265,7 +265,7 @@ By specifying `Projects::`, we tell Rails exactly what class we are referring to and we would avoid the issue. NOTE: -This problem will disappear as soon as we upgrade to Rails 6 and use the Zeitwerk autoloader. +This problem disappears as soon as we upgrade to Rails 6 and use the Zeitwerk autoloader. ### Further reading diff --git a/doc/development/licensing.md b/doc/development/licensing.md index 494ff0bc78b..d7c2c764883 100644 --- a/doc/development/licensing.md +++ b/doc/development/licensing.md @@ -12,16 +12,16 @@ info: To determine the technical writer assigned to the Stage/Group associated w In order to comply with the terms the libraries we use are licensed under, we have to make sure to check new gems for compatible licenses whenever they're added. To automate this process, we use the [license_finder](https://github.com/pivotal/LicenseFinder) gem by Pivotal. It runs every time a new commit is pushed and verifies that all gems and node modules in the bundle use a license that doesn't conflict with the licensing of either GitLab Community Edition or GitLab Enterprise Edition. -There are some limitations with the automated testing, however. CSS, JavaScript, or Ruby libraries which are not included by way of Bundler, NPM, or Yarn (for instance those manually copied into our source tree in the `vendor` directory), must be verified manually and independently. Take care whenever one such library is used, as automated tests won't catch problematic licenses from them. +There are some limitations with the automated testing, however. CSS, JavaScript, or Ruby libraries which are not included by way of Bundler, NPM, or Yarn (for instance those manually copied into our source tree in the `vendor` directory), must be verified manually and independently. Take care whenever one such library is used, as automated tests don't catch problematic licenses from them. -Some gems may not include their license information in their `gemspec` file, and some node modules may not include their license information in their `package.json` file. These won't be detected by License Finder, and will have to be verified manually. +Some gems may not include their license information in their `gemspec` file, and some node modules may not include their license information in their `package.json` file. These aren't detected by License Finder, and must be verified manually. ### License Finder commands NOTE: License Finder currently uses GitLab misused terms of `whitelist` and `blacklist`. As a result, the commands below reference those terms. We've created an [issue on their project](https://github.com/pivotal/LicenseFinder/issues/745) to propose that they rename their commands. -There are a few basic commands License Finder provides that you'll need in order to manage license detection. +There are a few basic commands License Finder provides that you need in order to manage license detection. To verify that the checks are passing, and/or to see what dependencies are causing the checks to fail: diff --git a/doc/development/mass_insert.md b/doc/development/mass_insert.md index 0560be650ca..4b1716ff00a 100644 --- a/doc/development/mass_insert.md +++ b/doc/development/mass_insert.md @@ -7,7 +7,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w # Mass inserting Rails models Setting the environment variable [`MASS_INSERT=1`](rake_tasks.md#environment-variables) -when running [`rake setup`](rake_tasks.md) will create millions of records, but these records +when running [`rake setup`](rake_tasks.md) creates millions of records, but these records aren't visible to the `root` user by default. To make any number of the mass-inserted projects visible to the `root` user, run diff --git a/doc/development/polling.md b/doc/development/polling.md index 5caa43abb62..18f9fb954dd 100644 --- a/doc/development/polling.md +++ b/doc/development/polling.md @@ -47,7 +47,7 @@ Cache Hit: resource. 1. If the `If-None-Match` header matches the current value in Redis we know that the resource did not change so we can send 304 response immediately, - without querying the database at all. The client's browser will use the + without querying the database at all. The client's browser uses the cached response. 1. If the `If-None-Match` header does not match the current value in Redis we have to generate a new response, because the resource changed. diff --git a/doc/development/polymorphic_associations.md b/doc/development/polymorphic_associations.md index fbc09942998..cabd2e3fb41 100644 --- a/doc/development/polymorphic_associations.md +++ b/doc/development/polymorphic_associations.md @@ -16,7 +16,7 @@ target ID. For example, at the time of writing we have such a setup for - `source_type`: a string defining the model to use, can be either `Project` or `Namespace`. - `source_id`: the ID of the row to retrieve based on `source_type`. For - example, when `source_type` is `Project` then `source_id` will contain a + example, when `source_type` is `Project` then `source_id` contains a project ID. While such a setup may appear to be useful, it comes with many drawbacks; enough @@ -24,8 +24,8 @@ that you should avoid this at all costs. ## Space Wasted -Because this setup relies on string values to determine the model to use it will -end up wasting a lot of space. For example, for `Project` and `Namespace` the +Because this setup relies on string values to determine the model to use, it +wastes a lot of space. For example, for `Project` and `Namespace` the maximum size is 9 bytes, plus 1 extra byte for every string when using PostgreSQL. While this may only be 10 bytes per row, given enough tables and rows using such a setup we can end up wasting quite a bit of disk space and @@ -84,7 +84,7 @@ Let's say you have a `members` table storing both approved and pending members, for both projects and groups, and the pending state is determined by the column `requested_at` being set or not. Schema wise such a setup can lead to various columns only being set for certain rows, wasting space. It's also possible that -certain indexes will only be set for certain rows, again wasting space. Finally, +certain indexes are only set for certain rows, again wasting space. Finally, querying such a table requires less than ideal queries. For example: ```sql @@ -121,7 +121,7 @@ WHERE group_id = 4 ``` If you want to get both you can use a UNION, though you need to be explicit -about what columns you want to SELECT as otherwise the result set will use the +about what columns you want to SELECT as otherwise the result set uses the columns of the first query. For example: ```sql @@ -147,6 +147,6 @@ filter rows using the `IS NULL` condition. To summarize: using separate tables allows us to use foreign keys effectively, create indexes only where necessary, conserve space, query data more efficiently, and scale these tables more easily (e.g. by storing them on -separate disks). A nice side effect of this is that code can also become easier -as you won't end up with a single model having to handle different kinds of +separate disks). A nice side effect of this is that code can also become easier, +as a single model isn't responsible for handling different kinds of data. diff --git a/doc/development/post_deployment_migrations.md b/doc/development/post_deployment_migrations.md index 605a13c3192..6ab3620c197 100644 --- a/doc/development/post_deployment_migrations.md +++ b/doc/development/post_deployment_migrations.md @@ -8,7 +8,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w Post deployment migrations are regular Rails migrations that can optionally be executed after a deployment. By default these migrations are executed alongside -the other migrations. To skip these migrations you will have to set the +the other migrations. To skip these migrations you must set the environment variable `SKIP_POST_DEPLOYMENT_MIGRATIONS` to a non-empty value when running `rake db:migrate`. @@ -19,7 +19,7 @@ migrations: bundle exec rake db:migrate ``` -This however will skip post deployment migrations: +This however skips post deployment migrations: ```shell SKIP_POST_DEPLOYMENT_MIGRATIONS=true bundle exec rake db:migrate @@ -40,7 +40,7 @@ Once all servers have been updated you can run `chef-client` again on a single server _without_ the environment variable. The process is similar for other deployment techniques: first you would deploy -with the environment variable set, then you'll essentially re-deploy a single +with the environment variable set, then you re-deploy a single server but with the variable _unset_. ## Creating Migrations @@ -51,7 +51,7 @@ To create a post deployment migration you can use the following Rails generator: bundle exec rails g post_deployment_migration migration_name_here ``` -This will generate the migration file in `db/post_migrate`. These migrations +This generates the migration file in `db/post_migrate`. These migrations behave exactly like regular Rails migrations. ## Use Cases diff --git a/doc/development/profiling.md b/doc/development/profiling.md index 07c8d514b8f..76c89d361fc 100644 --- a/doc/development/profiling.md +++ b/doc/development/profiling.md @@ -24,7 +24,7 @@ When using the script, command-line documentation is available by passing no arguments. When using the method in an interactive console session, any changes to the -application code within that console session will be reflected in the profiler +application code within that console session is reflected in the profiler output. For example: @@ -37,14 +37,14 @@ Gitlab::Profiler.profile('/my-user') # Returns a RubyProf::Profile where 100 seconds is spent in UsersController#show ``` -For routes that require authorization you will need to provide a user to +For routes that require authorization you must provide a user to `Gitlab::Profiler`. You can do this like so: ```ruby Gitlab::Profiler.profile('/gitlab-org/gitlab-test', user: User.first) ``` -Passing a `logger:` keyword argument to `Gitlab::Profiler.profile` will send +Passing a `logger:` keyword argument to `Gitlab::Profiler.profile` sends ActiveRecord and ActionController log output to that logger. Further options are documented with the method source. @@ -123,7 +123,7 @@ starting GitLab. For example: ENABLE_BULLET=true bundle exec rails s ``` -Bullet will log query problems to both the Rails log as well as the Chrome +Bullet logs query problems to both the Rails log as well as the Chrome console. As a follow up to finding `N+1` queries with Bullet, consider writing a [QueryRecoder test](query_recorder.md) to prevent a regression. diff --git a/doc/development/reference_processing.md b/doc/development/reference_processing.md index 5b7be8ece98..23c0861081d 100644 --- a/doc/development/reference_processing.md +++ b/doc/development/reference_processing.md @@ -101,7 +101,7 @@ format the reference as: This default implementation is not very efficient, because we need to call `#find_object` for each reference, which may require issuing a DB query every -time. For this reason, most reference filter implementations will instead use an +time. For this reason, most reference filter implementations instead use an optimization included in `AbstractReferenceFilter`: > `AbstractReferenceFilter` provides a lazily initialized value @@ -140,7 +140,7 @@ We are skipping: To avoid filtering such nodes for each `ReferenceFilter`, we do it only once and store the result in the result Hash of the pipeline as `result[:reference_filter_nodes]`. -Pipeline `result` is passed to each filter for modification, so every time when `ReferenceFilter` replaces text or link tag, filtered list (`reference_filter_nodes`) will be updated for the next filter to use. +Pipeline `result` is passed to each filter for modification, so every time when `ReferenceFilter` replaces text or link tag, filtered list (`reference_filter_nodes`) are updated for the next filter to use. ## Reference parsers @@ -199,4 +199,4 @@ In practice, all reference parsers inherit from [`BaseParser`](https://gitlab.co - `#nodes_user_can_reference(user, nodes)` to filter nodes directly. A failure to implement this class for each reference type means that the -application will raise exceptions during Markdown processing. +application raises exceptions during Markdown processing. diff --git a/doc/development/scalability.md b/doc/development/scalability.md index ad58a6abe60..78922e550e9 100644 --- a/doc/development/scalability.md +++ b/doc/development/scalability.md @@ -16,7 +16,7 @@ scalability and reliability. _[diagram source - GitLab employees only](https://docs.google.com/drawings/d/1RTGtuoUrE0bDT-9smoHbFruhEMI4Ys6uNrufe5IA-VI/edit)_ The diagram above shows a GitLab reference architecture scaled up for 50,000 -users. We will discuss each component below. +users. We discuss each component below. ## Components @@ -26,11 +26,10 @@ The PostgreSQL database holds all metadata for projects, issues, merge requests, users, etc. The schema is managed by the Rails application [db/structure.sql](https://gitlab.com/gitlab-org/gitlab/blob/master/db/structure.sql). -GitLab Web/API servers and Sidekiq nodes talk directly to the database via a -Rails object relational model (ORM). Most SQL queries are accessed via this +GitLab Web/API servers and Sidekiq nodes talk directly to the database by using a +Rails object relational model (ORM). Most SQL queries are accessed by using this ORM, although some custom SQL is also written for performance or for -exploiting advanced PostgreSQL features (e.g. recursive CTEs, LATERAL JOINs, -etc.). +exploiting advanced PostgreSQL features (like recursive CTEs or LATERAL JOINs). The application has a tight coupling to the database schema. When the application starts, Rails queries the database schema, caching the tables and @@ -42,8 +41,8 @@ no-downtime changes](what_requires_downtime.md). #### Multi-tenancy A single database is used to store all customer data. Each user can belong to -many groups or projects, and the access level (e.g. guest, developer, -maintainer, etc.) to groups and projects determines what users can see and +many groups or projects, and the access level (including guest, developer, or +maintainer) to groups and projects determines what users can see and what they can access. Users with admin access can access all projects and even impersonate @@ -70,7 +69,7 @@ dates](https://gitlab.com/groups/gitlab-org/-/epics/2023). For example, the `events` and `audit_events` table are natural candidates for this kind of partitioning. -Sharding is likely more difficult and will require significant changes +Sharding is likely more difficult and requires significant changes to the schema and application. For example, if we have to store projects in many different databases, we immediately run into the question, "How can we retrieve data across different projects?" One answer to this is @@ -78,7 +77,7 @@ to abstract data access into API calls that abstract the database from the application, but this is a significant amount of work. There are solutions that may help abstract the sharding to some extent -from the application. For example, we will want to look at [Citus +from the application. For example, we want to look at [Citus Data](https://www.citusdata.com/product/community) closely. Citus Data provides a Rails plugin that adds a [tenant ID to ActiveRecord models](https://www.citusdata.com/blog/2017/01/05/easily-scale-out-multi-tenant-apps/). @@ -100,17 +99,16 @@ systems. A recent [database checkup shows a breakdown of the table sizes on GitLab.com](https://gitlab.com/gitlab-com/gl-infra/infrastructure/-/issues/8022#master-1022016101-8). -Since `merge_request_diff_files` contains over 1 TB of data, we will want to +Since `merge_request_diff_files` contains over 1 TB of data, we want to reduce/eliminate this table first. GitLab has support for [storing diffs in -object storage](../administration/merge_request_diffs.md), which we [will -want to do on +object storage](../administration/merge_request_diffs.md), which we [want to do on GitLab.com](https://gitlab.com/gitlab-com/gl-infra/infrastructure/-/issues/7356). #### High availability There are several strategies to provide high-availability and redundancy: -- Write-ahead logs (WAL) streamed to object storage (e.g. S3, Google Cloud +- Write-ahead logs (WAL) streamed to object storage (for example, S3, or Google Cloud Storage). - Read-replicas (hot backups). - Delayed replicas. @@ -126,11 +124,10 @@ the read replicas. [Omnibus ships with both repmgr and Patroni](../administratio #### Load-balancing GitLab EE has [application support for load balancing using read -replicas](../administration/database_load_balancing.md). This load -balancer does some smart things that are not traditionally available in -standard load balancers. For example, the application will only consider a -replica if its replication lag is low (e.g. WAL data behind by < 100 -megabytes). +replicas](../administration/database_load_balancing.md). This load balancer does +some actions that aren't traditionally available in standard load balancers. For +example, the application considers a replica only if its replication lag is low +(for example, WAL data behind by less than 100 MB). More [details are in a blog post](https://about.gitlab.com/blog/2017/10/02/scaling-the-gitlab-database/). @@ -140,7 +137,7 @@ post](https://about.gitlab.com/blog/2017/10/02/scaling-the-gitlab-database/). As PostgreSQL forks a backend process for each request, PostgreSQL has a finite limit of connections that it can support, typically around 300 by default. Without a connection pooler like PgBouncer, it's quite possible to -hit connection limits. Once the limits are reached, then GitLab will generate +hit connection limits. Once the limits are reached, then GitLab generates errors or slow down as it waits for a connection to be available. #### High availability @@ -151,7 +148,7 @@ background job and/or Web requests. There are two ways to address this limitation: - Run multiple PgBouncer instances. -- Use a multi-threaded connection pooler (e.g. +- Use a multi-threaded connection pooler (for example, [Odyssey](https://gitlab.com/gitlab-com/gl-infra/infrastructure/-/issues/7776). On some Linux systems, it's possible to run [multiple PgBouncer instances on @@ -192,9 +189,9 @@ connections gracefully. There are three ways Redis is used in GitLab: -- Queues. Sidekiq jobs marshal jobs into JSON payloads. -- Persistent state. Session data, exclusive leases, etc. -- Cache. Repository data (e.g. Branch and tag names), view partials, etc. +- Queues: Sidekiq jobs marshal jobs into JSON payloads. +- Persistent state: Session data and exclusive leases. +- Cache: Repository data (like Branch and tag names) and view partials. For GitLab instances running at scale, splitting Redis usage into separate Redis clusters helps for two reasons: @@ -206,8 +203,8 @@ For example, the cache instance can behave like an least-recently used (LRU) cache by setting the `maxmemory` configuration option. That option should not be set for the queues or persistent clusters because data would be evicted from memory at random times. This would cause jobs to -be dropped on the floor, which would cause many problems (e.g. merges -not running, builds not updating, etc.). +be dropped on the floor, which would cause many problems (like merges +not running or builds not updating). Sidekiq also polls its queues quite frequently, and this activity can slow down other queries. For this reason, having a dedicated Redis @@ -219,7 +216,7 @@ Redis process. Single-core: Like PgBouncer, a single Redis process can only use one core. It does not support multi-threading. -Dumb secondaries: Redis secondaries (aka replicas) don't actually +Dumb secondaries: Redis secondaries (also known as replicas) don't actually handle any load. Unlike PostgreSQL secondaries, they don't even serve read queries. They simply replicate data from the primary and take over only when the primary fails. @@ -236,7 +233,7 @@ election to determine a new leader. No leader: A Redis cluster can get into a mode where there are no primaries. For example, this can happen if Redis nodes are misconfigured to follow the wrong node. Sometimes this requires forcing one node to -become a primary via the [`REPLICAOF NO ONE` +become a primary by using the [`REPLICAOF NO ONE` command](https://redis.io/commands/replicaof). ### Sidekiq @@ -260,8 +257,8 @@ directories in the GitLab code base. As jobs are added to the Sidekiq queue, Sidekiq worker threads need to pull these jobs from the queue and finish them at a rate faster than -they are added. When an imbalance occurs (e.g. delays in the database, -slow jobs, etc.), Sidekiq queues can balloon and lead to runaway queues. +they are added. When an imbalance occurs (for example, delays in the database +or slow jobs), Sidekiq queues can balloon and lead to runaway queues. In recent months, many of these queues have ballooned due to delays in PostgreSQL, PgBouncer, and Redis. For example, PgBouncer saturation can @@ -278,11 +275,11 @@ in a timely manner: used to process each commit message in the push, but now it farms out this to `ProcessCommitWorker`. - Redistribute/gerrymander Sidekiq processes by queue - types. Long-running jobs (e.g. relating to project import) can often - squeeze out jobs that run fast (e.g. delivering e-mail). [This technique + types. Long-running jobs (for example, relating to project import) can often + squeeze out jobs that run fast (for example, delivering e-mail). [This technique was used in to optimize our existing Sidekiq deployment](https://gitlab.com/gitlab-com/gl-infra/infrastructure/-/issues/7219#note_218019483). - Optimize jobs. Eliminating unnecessary work, reducing network calls - (e.g. SQL, Gitaly, etc.), and optimizing processor time can yield significant + (including SQL and Gitaly), and optimizing processor time can yield significant benefits. From the Sidekiq logs, it's possible to see which jobs run the most diff --git a/doc/development/shell_scripting_guide/index.md b/doc/development/shell_scripting_guide/index.md index d6b72132a3f..c9e2c98716b 100644 --- a/doc/development/shell_scripting_guide/index.md +++ b/doc/development/shell_scripting_guide/index.md @@ -73,7 +73,7 @@ shell check: ``` TIP: **Tip:** -By default, ShellCheck will use the [shell detection](https://github.com/koalaman/shellcheck/wiki/SC2148#rationale) +By default, ShellCheck uses the [shell detection](https://github.com/koalaman/shellcheck/wiki/SC2148#rationale) to determine the shell dialect in use. If the shell file is out of your control and ShellCheck cannot detect the dialect, use `-s` flag to specify it: `-s sh` or `-s bash`. @@ -101,7 +101,7 @@ shfmt: ``` TIP: **Tip:** -By default, shfmt will use the [shell detection](https://github.com/mvdan/sh#shfmt) similar to one of ShellCheck +By default, shfmt uses the [shell detection](https://github.com/mvdan/sh#shfmt) similar to one of ShellCheck and ignore files starting with a period. To override this, use `-ln` flag to specify the shell dialect: `-ln posix` or `-ln bash`. diff --git a/doc/development/uploads.md b/doc/development/uploads.md index cba56eb99e7..7ffa9014240 100644 --- a/doc/development/uploads.md +++ b/doc/development/uploads.md @@ -46,7 +46,7 @@ We have three challenges here: performance, availability, and scalability. ### Performance -Rails process are expensive in terms of both CPU and memory. Ruby [global interpreter lock](https://en.wikipedia.org/wiki/Global_interpreter_lock) adds to cost too because the Ruby process will spend time on I/O operations on step 3 causing incoming requests to pile up. +Rails process are expensive in terms of both CPU and memory. Ruby [global interpreter lock](https://en.wikipedia.org/wiki/Global_interpreter_lock) adds to cost too because the Ruby process spends time on I/O operations on step 3 causing incoming requests to pile up. In order to improve this, [disk buffered upload](#disk-buffered-upload) was implemented. With this, Rails no longer deals with writing uploaded files to disk. @@ -88,7 +88,7 @@ To address this problem an HA object storage can be used and it's supported by [ Scaling NFS is outside of our support scope, and NFS is not a part of cloud native installations. -All features that require Sidekiq and do not use direct upload won't work without NFS. In Kubernetes, machine boundaries translate to PODs, and in this case the uploaded file will be written into the POD private disk. Since Sidekiq POD cannot reach into other pods, the operation will fail to read it. +All features that require Sidekiq and do not use direct upload doesn't work without NFS. In Kubernetes, machine boundaries translate to PODs, and in this case the uploaded file is written into the POD private disk. Since Sidekiq POD cannot reach into other pods, the operation fails to read it. ## How to select the proper level of acceleration? @@ -96,7 +96,7 @@ Selecting the proper acceleration is a tradeoff between speed of development and We can identify three major use-cases for an upload: -1. **storage:** if we are uploading for storing a file (i.e. artifacts, packages, discussion attachments). In this case [direct upload](#direct-upload) is the proper level as it's the less resource-intensive operation. Additional information can be found on [File Storage in GitLab](file_storage.md). +1. **storage:** if we are uploading for storing a file (like artifacts, packages, or discussion attachments). In this case [direct upload](#direct-upload) is the proper level as it's the less resource-intensive operation. Additional information can be found on [File Storage in GitLab](file_storage.md). 1. **in-controller/synchronous processing:** if we allow processing **small files** synchronously, using [disk buffered upload](#disk-buffered-upload) may speed up development. 1. **Sidekiq/asynchronous processing:** Asynchronous processing must implement [direct upload](#direct-upload), the reason being that it's the only way to support Cloud Native deployments without a shared NFS. @@ -120,7 +120,7 @@ We have three kinds of file encoding in our uploads: 1. <i class="fa fa-check-circle"></i> **multipart**: `multipart/form-data` is the most common, a file is encoded as a part of a multipart encoded request. 1. <i class="fa fa-check-circle"></i> **body**: some APIs uploads files as the whole request body. -1. <i class="fa fa-times-circle"></i> **JSON**: some JSON API uploads files as base64 encoded strings. This will require a change to GitLab Workhorse, which [is planned](https://gitlab.com/gitlab-org/gitlab-workhorse/-/issues/226). +1. <i class="fa fa-times-circle"></i> **JSON**: some JSON API uploads files as base64 encoded strings. This requires a change to GitLab Workhorse, which [is planned](https://gitlab.com/gitlab-org/gitlab-workhorse/-/issues/226). ## Uploading technologies @@ -166,7 +166,7 @@ is replaced with the path to the corresponding file before it is forwarded to Rails. To prevent abuse of this feature, Workhorse signs the modified request with a -special header, stating which entries it modified. Rails will ignore any +special header, stating which entries it modified. Rails ignores any unsigned path entries. ```mermaid @@ -220,8 +220,8 @@ In this setup, an extra Rails route must be implemented in order to handle autho and [its routes](https://gitlab.com/gitlab-org/gitlab/blob/cc723071ad337573e0360a879cbf99bc4fb7adb9/config/routes/git_http.rb#L31-32). - [API endpoints for uploading packages](packages.md#file-uploads). -This will fallback to _disk buffered upload_ when `direct_upload` is disabled inside the [object storage setting](../administration/uploads.md#object-storage-settings). -The answer to the `/authorize` call will only contain a file system path. +This falls back to _disk buffered upload_ when `direct_upload` is disabled inside the [object storage setting](../administration/uploads.md#object-storage-settings). +The answer to the `/authorize` call contains only a file system path. ```mermaid sequenceDiagram @@ -272,7 +272,7 @@ sequenceDiagram ## How to add a new upload route -In this section, we'll describe how to add a new upload route [accelerated](#uploading-technologies) by Workhorse for [body and multipart](#upload-encodings) encoded uploads. +In this section, we describe how to add a new upload route [accelerated](#uploading-technologies) by Workhorse for [body and multipart](#upload-encodings) encoded uploads. Uploads routes belong to one of these categories: diff --git a/doc/integration/elasticsearch.md b/doc/integration/elasticsearch.md index 22390ed8b68..057cb923a13 100644 --- a/doc/integration/elasticsearch.md +++ b/doc/integration/elasticsearch.md @@ -441,6 +441,22 @@ After the reindexing is completed, the original index will be scheduled to be de While the reindexing is running, you will be able to follow its progress under that same section. +### Mark the most recent reindex job as failed and unpause the indexing + +Sometimes, you might want to abandon the unfinished reindex job and unpause the indexing. You can achieve this via the following steps: + +1. Mark the most recent reindex job as failed: + + ```shell + # Omnibus installations + sudo gitlab-rake gitlab:elastic:mark_reindex_failed + + # Installations from source + bundle exec rake gitlab:elastic:mark_reindex_failed RAILS_ENV=production + ``` + +1. Uncheck the "Pause Elasticsearch indexing" checkbox in **Admin Area > Settings > General > Advanced Search**. + ## Background migrations > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/234046) in GitLab 13.6. @@ -511,7 +527,8 @@ The following are some available Rake tasks: | [`sudo gitlab-rake gitlab:elastic:recreate_index[<TARGET_NAME>]`](https://gitlab.com/gitlab-org/gitlab/blob/master/ee/lib/tasks/gitlab/elastic.rake) | Wrapper task for `gitlab:elastic:delete_index[<TARGET_NAME>]` and `gitlab:elastic:create_empty_index[<TARGET_NAME>]`. | | [`sudo gitlab-rake gitlab:elastic:index_snippets`](https://gitlab.com/gitlab-org/gitlab/blob/master/ee/lib/tasks/gitlab/elastic.rake) | Performs an Elasticsearch import that indexes the snippets data. | | [`sudo gitlab-rake gitlab:elastic:projects_not_indexed`](https://gitlab.com/gitlab-org/gitlab/blob/master/ee/lib/tasks/gitlab/elastic.rake) | Displays which projects are not indexed. | -| [`sudo gitlab-rake gitlab:elastic:reindex_cluster`](https://gitlab.com/gitlab-org/gitlab/blob/master/ee/lib/tasks/gitlab/elastic.rake) | Schedules a zero-downtime cluster reindexing task. This feature should be used with an index that was created after GitLab 13.0. | +| [`sudo gitlab-rake gitlab:elastic:reindex_cluster`](https://gitlab.com/gitlab-org/gitlab/blob/master/ee/lib/tasks/gitlab/elastic.rake) | Schedules a zero-downtime cluster reindexing task. This feature should be used with an index that was created after GitLab 13.0. | +| [`sudo gitlab-rake gitlab:elastic:mark_reindex_failed`](https://gitlab.com/gitlab-org/gitlab/blob/master/ee/lib/tasks/gitlab/elastic.rake)`] | Mark the most recent re-index job as failed. | NOTE: The `TARGET_NAME` parameter is optional and will use the default index/alias name from the current `RAILS_ENV` if not set. @@ -789,7 +806,7 @@ There are a couple of ways to achieve that: This is always correctly identifying whether the current project/namespace being searched is using Elasticsearch. -- From the admin area under **Settings > General > Elasticsearch** check that the +- From the admin area under **Settings > General > Advanced Search** check that the Advanced Search settings are checked. Those same settings there can be obtained from the Rails console if necessary: diff --git a/doc/raketasks/README.md b/doc/raketasks/README.md index 79e494496b0..a42bf2a5d91 100644 --- a/doc/raketasks/README.md +++ b/doc/raketasks/README.md @@ -42,6 +42,7 @@ The following are available Rake tasks: | [Repository storage](../administration/raketasks/storage.md) | List and migrate existing projects and attachments from legacy storage to hashed storage. | | [Uploads migrate](../administration/raketasks/uploads/migrate.md) | Migrate uploads between storage local and object storage. | | [Uploads sanitize](../administration/raketasks/uploads/sanitize.md) | Remove EXIF data from images uploaded to earlier versions of GitLab. | +| [Usage data](../administration/troubleshooting/gitlab_rails_cheat_sheet.md#generate-usage-ping) | Generate and troubleshoot [Usage Ping](../development/product_analytics/usage_ping.md).| | [User management](user_management.md) | Perform user management tasks. | | [Webhooks administration](web_hooks.md) | Maintain project Webhooks. | | [X.509 signatures](x509_signatures.md) | Update X.509 commit signatures, useful if certificate store has changed. | diff --git a/doc/security/asset_proxy.md b/doc/security/asset_proxy.md index dac18b1b921..613743143d3 100644 --- a/doc/security/asset_proxy.md +++ b/doc/security/asset_proxy.md @@ -10,7 +10,7 @@ A possible security concern when managing a public facing GitLab instance is the ability to steal a users IP address by referencing images in issues, comments, etc. For example, adding `` to -an issue description will cause the image to be loaded from the external +an issue description causes the image to be loaded from the external server in order to be displayed. However, this also allows the external server to log the IP address of the user. @@ -51,7 +51,7 @@ To install a Camo server as an asset proxy: | `asset_proxy_enabled` | Enable proxying of assets. If enabled, requires: `asset_proxy_url`). | | `asset_proxy_secret_key` | Shared secret with the asset proxy server. | | `asset_proxy_url` | URL of the asset proxy server. | - | `asset_proxy_whitelist` | Assets that match these domain(s) will NOT be proxied. Wildcards allowed. Your GitLab installation URL is automatically whitelisted. | + | `asset_proxy_whitelist` | Assets that match these domain(s) are NOT proxied. Wildcards allowed. Your GitLab installation URL is automatically whitelisted. | 1. Restart the server for the changes to take effect. Each time you change any values for the asset proxy, you need to restart the server. @@ -59,7 +59,7 @@ To install a Camo server as an asset proxy: ## Using the Camo server Once the Camo server is running and you've enabled the GitLab settings, any image, video, or audio that -references an external source will get proxied to the Camo server. +references an external source are proxied to the Camo server. For example, the following is a link to an image in Markdown: diff --git a/doc/security/rack_attack.md b/doc/security/rack_attack.md index 3b178498079..f159b4f8e21 100644 --- a/doc/security/rack_attack.md +++ b/doc/security/rack_attack.md @@ -32,10 +32,10 @@ Rack Attack disabled. ## Behavior If set up as described in the [Settings](#settings) section below, two behaviors -will be enabled: +are enabled: -- Protected paths will be throttled. -- Failed authentications for Git and container registry requests will trigger a temporary IP ban. +- Protected paths are throttled. +- Failed authentications for Git and container registry requests trigger a temporary IP ban. ### Protected paths throttle @@ -119,7 +119,7 @@ The following settings can be configured: specified time. - `findtime`: The maximum amount of time that failed requests can count against an IP before it's blacklisted (in seconds). -- `bantime`: The total amount of time that a blacklisted IP will be blocked (in +- `bantime`: The total amount of time that a blacklisted IP is blocked (in seconds). **Installations from source** @@ -142,8 +142,8 @@ taken in order to enable protection for your GitLab instance: If you want more restrictive/relaxed throttle rules, edit `config/initializers/rack_attack.rb` and change the `limit` or `period` values. -For example, more relaxed throttle rules will be if you set -`limit: 3` and `period: 1.seconds` (this will allow 3 requests per second). +For example, you can set more relaxed throttle rules with +`limit: 3` and `period: 1.seconds`, allowing 3 requests per second. You can also add other paths to the protected list by adding to `paths_to_be_protected` variable. If you change any of these settings you must restart your GitLab instance. @@ -185,10 +185,10 @@ In case you want to remove a blocked IP, follow these steps: ### Rack attack is blacklisting the load balancer Rack Attack may block your load balancer if all traffic appears to come from -the load balancer. In that case, you will need to: +the load balancer. In that case, you must: 1. [Configure `nginx[real_ip_trusted_addresses]`](https://docs.gitlab.com/omnibus/settings/nginx.html#configuring-gitlab-trusted_proxies-and-the-nginx-real_ip-module). - This will keep users' IPs from being listed as the load balancer IPs. + This keeps users' IPs from being listed as the load balancer IPs. 1. Whitelist the load balancer's IP address(es) in the Rack Attack [settings](#settings). 1. Reconfigure GitLab: diff --git a/doc/update/README.md b/doc/update/README.md index ba6707913ec..38d1077c6f1 100644 --- a/doc/update/README.md +++ b/doc/update/README.md @@ -276,7 +276,7 @@ Edition, follow the guides below based on the installation method: to a version upgrade: stop the server, get the code, update configuration files for the new functionality, install libraries and do migrations, update the init script, start the application and check its status. -- [Omnibus CE to EE](https://docs.gitlab.com/omnibus/update/README.html#updating-community-edition-to-enterprise-edition) - Follow this guide to update your Omnibus +- [Omnibus CE to EE](https://docs.gitlab.com/omnibus/update/README.html#update-community-edition-to-enterprise-edition) - Follow this guide to update your Omnibus GitLab Community Edition to the Enterprise Edition. ### Enterprise to Community Edition diff --git a/doc/user/profile/notifications.md b/doc/user/profile/notifications.md index c19961e378b..0a2a48db955 100644 --- a/doc/user/profile/notifications.md +++ b/doc/user/profile/notifications.md @@ -13,12 +13,12 @@ Notifications are sent via email. ## Receiving notifications -You will receive notifications for one of the following reasons: +You receive notifications for one of the following reasons: - You participate in an issue, merge request, epic or design. In this context, _participate_ means comment, or edit. - You enable notifications in an issue, merge request, or epic. To enable notifications, click the **Notifications** toggle in the sidebar to _on_. -While notifications are enabled, you will receive notification of actions occurring in that issue, merge request, or epic. +While notifications are enabled, you receive notification of actions occurring in that issue, merge request, or epic. NOTE: Notifications can be blocked by an admin, preventing them from being sent. @@ -50,7 +50,7 @@ These notification settings apply only to you. They do not affect the notificati Your **Global notification settings** are the default settings unless you select different values for a project or a group. - Notification email - - This is the email address your notifications will be sent to. + - This is the email address your notifications are sent to. - Global notification level - This is the default [notification level](#notification-levels) which applies to all your notifications. - Receive notifications about your own activity. @@ -138,7 +138,7 @@ For each project and group you can select one of the following levels: ## Notification events -Users will be notified of the following events: +Users are notified of the following events: | Event | Sent to | Settings level | |------------------------------|---------------------|------------------------------| @@ -158,7 +158,7 @@ Users will be notified of the following events: ## Issue / Epics / Merge request events -In most of the below cases, the notification will be sent to: +In most of the below cases, the notification is sent to: - Participants: - the author and assignee of the issue/merge request @@ -193,23 +193,23 @@ To minimize the number of notifications that do not require any action, from [Gi | New comment | The above, plus anyone mentioned by `@username` in the comment, with notification level "Mention" or higher | | Failed pipeline | The author of the pipeline | | Fixed pipeline | The author of the pipeline. Enabled by default. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/24309) in GitLab 13.1. | -| Successful pipeline | The author of the pipeline, if they have the custom notification setting for successful pipelines set. If the pipeline failed previously, a `Fixed pipeline` message will be sent for the first successful pipeline after the failure, then a `Successful pipeline` message for any further successful pipelines. | +| Successful pipeline | The author of the pipeline, if they have the custom notification setting for successful pipelines set. If the pipeline failed previously, a `Fixed pipeline` message is sent for the first successful pipeline after the failure, then a `Successful pipeline` message for any further successful pipelines. | | New epic **(ULTIMATE)** | | | Close epic **(ULTIMATE)** | | | Reopen epic **(ULTIMATE)** | | In addition, if the title or description of an Issue or Merge Request is -changed, notifications will be sent to any **new** mentions by `@username` as +changed, notifications are sent to any **new** mentions by `@username` as if they had been mentioned in the original text. -You won't receive notifications for Issues, Merge Requests or Milestones created -by yourself (except when an issue is due). You will only receive automatic +You don't receive notifications for Issues, Merge Requests or Milestones created +by yourself (except when an issue is due). You only receive automatic notifications when somebody else comments or adds changes to the ones that you've created or mentions you. -If an open merge request becomes unmergeable due to conflict, its author will be notified about the cause. +If an open merge request becomes unmergeable due to conflict, its author is notified about the cause. If a user has also set the merge request to automatically merge once pipeline succeeds, -then that user will also be notified. +then that user is also notified. ## Design email notifications @@ -252,7 +252,7 @@ The `X-GitLab-NotificationReason` header contains the reason for the notificatio - `mentioned` The reason for the notification is also included in the footer of the notification email. For example an email with the -reason `assigned` will have this sentence in the footer: +reason `assigned` has this sentence in the footer: - `You are receiving this email because you have been assigned an item on <configured GitLab hostname>.` diff --git a/doc/user/project/merge_requests/getting_started.md b/doc/user/project/merge_requests/getting_started.md index 3cb35ea765e..b4abac7ebba 100644 --- a/doc/user/project/merge_requests/getting_started.md +++ b/doc/user/project/merge_requests/getting_started.md @@ -111,7 +111,7 @@ It is also possible to manage multiple assignees: - When creating a merge request. - Using [quick actions](../quick_actions.md#quick-actions-for-issues-merge-requests-and-epics). -## Reviewer +### Reviewer > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/216054) in GitLab 13.5. > - It's [deployed behind a feature flag](../../../user/feature_flags.md), enabled by default. @@ -134,7 +134,7 @@ This makes it easy to determine the relevant roles for the users involved in the To request it, open the **Reviewers** drop-down box to search for the user you wish to get a review from. -### Enable or disable Merge Request Reviewers **(CORE ONLY)** +#### Enable or disable Merge Request Reviewers **(CORE ONLY)** Merge Request Reviewers is under development and not ready for production use. It is deployed behind a feature flag that is **disabled by default**. diff --git a/lib/api/feature_flags_user_lists.rb b/lib/api/feature_flags_user_lists.rb index 086bcbcdc89..8577da173b1 100644 --- a/lib/api/feature_flags_user_lists.rb +++ b/lib/api/feature_flags_user_lists.rb @@ -54,7 +54,7 @@ module API end params do - requires :iid, type: String, desc: 'The internal id of the user list' + requires :iid, type: String, desc: 'The internal ID of the user list' end resource 'feature_flags_user_lists/:iid' do desc 'Get a single feature flag user list belonging to a project' do diff --git a/lib/api/group_labels.rb b/lib/api/group_labels.rb index bf3ac8800b7..7fbf4445116 100644 --- a/lib/api/group_labels.rb +++ b/lib/api/group_labels.rb @@ -66,7 +66,7 @@ module API success Entities::GroupLabel end params do - optional :label_id, type: Integer, desc: 'The id of the label to be updated' + optional :label_id, type: Integer, desc: 'The ID of the label to be updated' optional :name, type: String, desc: 'The name of the label to be updated' use :group_label_update_params exactly_one_of :label_id, :name diff --git a/lib/api/labels.rb b/lib/api/labels.rb index a8fc277989e..c9f29865664 100644 --- a/lib/api/labels.rb +++ b/lib/api/labels.rb @@ -57,7 +57,7 @@ module API success Entities::ProjectLabel end params do - optional :label_id, type: Integer, desc: 'The id of the label to be updated' + optional :label_id, type: Integer, desc: 'The ID of the label to be updated' optional :name, type: String, desc: 'The name of the label to be updated' use :project_label_update_params exactly_one_of :label_id, :name @@ -71,7 +71,7 @@ module API success Entities::ProjectLabel end params do - optional :label_id, type: Integer, desc: 'The id of the label to be deleted' + optional :label_id, type: Integer, desc: 'The ID of the label to be deleted' optional :name, type: String, desc: 'The name of the label to be deleted' exactly_one_of :label_id, :name end diff --git a/lib/api/release/links.rb b/lib/api/release/links.rb index d3a185a51c8..52c73104bb4 100644 --- a/lib/api/release/links.rb +++ b/lib/api/release/links.rb @@ -57,7 +57,7 @@ module API end params do - requires :link_id, type: String, desc: 'The id of the link' + requires :link_id, type: String, desc: 'The ID of the link' end resource 'links/:link_id' do desc 'Get a link detail of a release' do diff --git a/lib/gitlab/usage_data.rb b/lib/gitlab/usage_data.rb index b182a160e29..7cfe500260a 100644 --- a/lib/gitlab/usage_data.rb +++ b/lib/gitlab/usage_data.rb @@ -769,7 +769,7 @@ module Gitlab end def report_snowplow_events? - self_monitoring_project && Feature.enabled?(:product_analytics, self_monitoring_project) + self_monitoring_project && Feature.enabled?(:product_analytics_tracking, type: :ops) end def distinct_count_service_desk_enabled_projects(time_period) diff --git a/lib/tasks/gitlab/usage_data.rake b/lib/tasks/gitlab/usage_data.rake index 140077da2b5..d6f5661d5eb 100644 --- a/lib/tasks/gitlab/usage_data.rake +++ b/lib/tasks/gitlab/usage_data.rake @@ -12,13 +12,14 @@ namespace :gitlab do desc 'GitLab | UsageData | Generate usage ping in JSON' task generate: :environment do - puts Gitlab::UsageData.to_json(force_refresh: true) + puts Gitlab::Json.pretty_generate(Gitlab::UsageData.uncached_data) end desc 'GitLab | UsageData | Generate usage ping and send it to Versions Application' task generate_and_send: :environment do result = SubmitUsagePingService.new.execute - puts result.inspect + + puts Gitlab::Json.pretty_generate(result.attributes) end end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 989dc19a589..f5113f7f441 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -27113,6 +27113,9 @@ msgstr "" msgid "The %{link_start}true-up model%{link_end} allows having more users, and additional users will incur a retroactive charge on renewal." msgstr "" +msgid "The %{plan_name} is no longer available to purchase. For more information about how this will impact you, check our %{faq_link_start}frequently asked questions%{faq_link_end}." +msgstr "" + msgid "The %{type} contains the following error:" msgid_plural "The %{type} contains the following errors:" msgstr[0] "" diff --git a/spec/frontend/frequent_items/components/app_spec.js b/spec/frontend/frequent_items/components/app_spec.js index b4f36b82385..439a410eaa1 100644 --- a/spec/frontend/frequent_items/components/app_spec.js +++ b/spec/frontend/frequent_items/components/app_spec.js @@ -6,10 +6,10 @@ import waitForPromises from 'helpers/wait_for_promises'; import axios from '~/lib/utils/axios_utils'; import appComponent from '~/frequent_items/components/app.vue'; import eventHub from '~/frequent_items/event_hub'; -import store from '~/frequent_items/store'; import { FREQUENT_ITEMS, HOUR_IN_MS } from '~/frequent_items/constants'; import { getTopFrequentItems } from '~/frequent_items/utils'; import { currentSession, mockFrequentProjects, mockSearchedProjects } from '../mock_data'; +import { createStore } from '~/frequent_items/store'; useLocalStorageSpy(); @@ -18,6 +18,7 @@ const createComponentWithStore = (namespace = 'projects') => { session = currentSession[namespace]; gon.api_version = session.apiVersion; const Component = Vue.extend(appComponent); + const store = createStore(); return mountComponentWithStore(Component, { store, diff --git a/spec/frontend/frequent_items/components/frequent_items_list_item_spec.js b/spec/frontend/frequent_items/components/frequent_items_list_item_spec.js index ab5784b8f7a..1160ed5c84b 100644 --- a/spec/frontend/frequent_items/components/frequent_items_list_item_spec.js +++ b/spec/frontend/frequent_items/components/frequent_items_list_item_spec.js @@ -1,10 +1,14 @@ import { shallowMount } from '@vue/test-utils'; +import { mockTracking, unmockTracking } from 'helpers/tracking_helper'; import { trimText } from 'helpers/text_helper'; import frequentItemsListItemComponent from '~/frequent_items/components/frequent_items_list_item.vue'; -import { mockProject } from '../mock_data'; // can also use 'mockGroup', but not useful to test here +import { createStore } from '~/frequent_items/store'; +import { mockProject } from '../mock_data'; describe('FrequentItemsListItemComponent', () => { let wrapper; + let trackingSpy; + let store = createStore(); const findTitle = () => wrapper.find({ ref: 'frequentItemsItemTitle' }); const findAvatar = () => wrapper.find({ ref: 'frequentItemsItemAvatar' }); @@ -18,6 +22,7 @@ describe('FrequentItemsListItemComponent', () => { const createComponent = (props = {}) => { wrapper = shallowMount(frequentItemsListItemComponent, { + store, propsData: { itemId: mockProject.id, itemName: mockProject.name, @@ -29,7 +34,14 @@ describe('FrequentItemsListItemComponent', () => { }); }; + beforeEach(() => { + store = createStore({ dropdownType: 'project' }); + trackingSpy = mockTracking('_category_', document, jest.spyOn); + trackingSpy.mockImplementation(() => {}); + }); + afterEach(() => { + unmockTracking(); wrapper.destroy(); wrapper = null; }); @@ -97,5 +109,18 @@ describe('FrequentItemsListItemComponent', () => { `('should render $expected $name', ({ selector, expected }) => { expect(selector()).toHaveLength(expected); }); + + it('tracks when item link is clicked', () => { + const link = wrapper.find('a'); + // NOTE: this listener is required to prevent the click from going through and causing: + // `Error: Not implemented: navigation ...` + link.element.addEventListener('click', e => { + e.preventDefault(); + }); + link.trigger('click'); + expect(trackingSpy).toHaveBeenCalledWith(undefined, 'click_link', { + label: 'project_dropdown_frequent_items_list_item', + }); + }); }); }); diff --git a/spec/frontend/frequent_items/components/frequent_items_list_spec.js b/spec/frontend/frequent_items/components/frequent_items_list_spec.js index 238fd508053..b6d2dd714ce 100644 --- a/spec/frontend/frequent_items/components/frequent_items_list_spec.js +++ b/spec/frontend/frequent_items/components/frequent_items_list_spec.js @@ -1,4 +1,5 @@ import { mount } from '@vue/test-utils'; +import { createStore } from '~/frequent_items/store'; import frequentItemsListComponent from '~/frequent_items/components/frequent_items_list.vue'; import frequentItemsListItemComponent from '~/frequent_items/components/frequent_items_list_item.vue'; import { mockFrequentProjects } from '../mock_data'; @@ -8,6 +9,7 @@ describe('FrequentItemsListComponent', () => { const createComponent = (props = {}) => { wrapper = mount(frequentItemsListComponent, { + store: createStore(), propsData: { namespace: 'projects', items: mockFrequentProjects, diff --git a/spec/frontend/frequent_items/components/frequent_items_search_input_spec.js b/spec/frontend/frequent_items/components/frequent_items_search_input_spec.js index c5155315bb9..f5e654e6bcb 100644 --- a/spec/frontend/frequent_items/components/frequent_items_search_input_spec.js +++ b/spec/frontend/frequent_items/components/frequent_items_search_input_spec.js @@ -1,23 +1,35 @@ import { shallowMount } from '@vue/test-utils'; +import { mockTracking, unmockTracking } from 'helpers/tracking_helper'; import searchComponent from '~/frequent_items/components/frequent_items_search_input.vue'; +import { createStore } from '~/frequent_items/store'; import eventHub from '~/frequent_items/event_hub'; -const createComponent = (namespace = 'projects') => - shallowMount(searchComponent, { - propsData: { namespace }, - }); - describe('FrequentItemsSearchInputComponent', () => { let wrapper; + let trackingSpy; let vm; + let store; + + const createComponent = (namespace = 'projects') => + shallowMount(searchComponent, { + store, + propsData: { namespace }, + }); beforeEach(() => { + store = createStore({ dropdownType: 'project' }); + jest.spyOn(store, 'dispatch').mockImplementation(() => {}); + + trackingSpy = mockTracking('_category_', document, jest.spyOn); + trackingSpy.mockImplementation(() => {}); + wrapper = createComponent(); ({ vm } = wrapper); }); afterEach(() => { + unmockTracking(); vm.$destroy(); }); @@ -76,4 +88,24 @@ describe('FrequentItemsSearchInputComponent', () => { ); }); }); + + describe('tracking', () => { + it('tracks when search query is entered', async () => { + expect(trackingSpy).not.toHaveBeenCalled(); + expect(store.dispatch).not.toHaveBeenCalled(); + + const value = 'my project'; + + const input = wrapper.find('input'); + input.setValue(value); + input.trigger('input'); + + await wrapper.vm.$nextTick(); + + expect(trackingSpy).toHaveBeenCalledWith(undefined, 'type_search_query', { + label: 'project_dropdown_frequent_items_search_input', + }); + expect(store.dispatch).toHaveBeenCalledWith('setSearchQuery', value); + }); + }); }); diff --git a/spec/frontend/frequent_items/mock_data.js b/spec/frontend/frequent_items/mock_data.js index 8c3c66f67ff..5e15b4b33e0 100644 --- a/spec/frontend/frequent_items/mock_data.js +++ b/spec/frontend/frequent_items/mock_data.js @@ -30,7 +30,6 @@ export const currentSession = { }; export const mockNamespace = 'projects'; - export const mockStorageKey = 'test-user/frequent-projects'; export const mockGroup = { diff --git a/spec/frontend/pipelines/graph/graph_component_legacy_spec.js b/spec/frontend/pipelines/graph/graph_component_legacy_spec.js index 4b34c260b91..3b1909b6564 100644 --- a/spec/frontend/pipelines/graph/graph_component_legacy_spec.js +++ b/spec/frontend/pipelines/graph/graph_component_legacy_spec.js @@ -15,8 +15,8 @@ describe('graph component', () => { let mediator; let wrapper; - const findExpandPipelineBtn = () => wrapper.find('[data-testid="expandPipelineButton"]'); - const findAllExpandPipelineBtns = () => wrapper.findAll('[data-testid="expandPipelineButton"]'); + const findExpandPipelineBtn = () => wrapper.find('[data-testid="expand-pipeline-button"]'); + const findAllExpandPipelineBtns = () => wrapper.findAll('[data-testid="expand-pipeline-button"]'); const findStageColumns = () => wrapper.findAll(StageColumnComponentLegacy); const findStageColumnAt = i => findStageColumns().at(i); diff --git a/spec/frontend/pipelines/graph/graph_component_spec.js b/spec/frontend/pipelines/graph/graph_component_spec.js index 32616c23be9..0ee8e21d8d7 100644 --- a/spec/frontend/pipelines/graph/graph_component_spec.js +++ b/spec/frontend/pipelines/graph/graph_component_spec.js @@ -1,9 +1,13 @@ -import { shallowMount } from '@vue/test-utils'; +import { mount, shallowMount } from '@vue/test-utils'; import PipelineGraph from '~/pipelines/components/graph/graph_component.vue'; import StageColumnComponent from '~/pipelines/components/graph/stage_column_component.vue'; import LinkedPipelinesColumn from '~/pipelines/components/graph/linked_pipelines_column.vue'; -import { unwrapPipelineData } from '~/pipelines/components/graph/utils'; -import { mockPipelineResponse } from './mock_data'; +import { GRAPHQL } from '~/pipelines/components/graph/constants'; +import { + generateResponse, + mockPipelineResponse, + pipelineWithUpstreamDownstream, +} from './mock_data'; describe('graph component', () => { let wrapper; @@ -11,10 +15,8 @@ describe('graph component', () => { const findLinkedColumns = () => wrapper.findAll(LinkedPipelinesColumn); const findStageColumns = () => wrapper.findAll(StageColumnComponent); - const generateResponse = raw => unwrapPipelineData(raw.data.project.pipeline.id, raw.data); - const defaultProps = { - pipeline: generateResponse(mockPipelineResponse), + pipeline: generateResponse(mockPipelineResponse, 'root/fungi-xoxo'), }; const createComponent = ({ mountFn = shallowMount, props = {} } = {}) => { @@ -23,6 +25,9 @@ describe('graph component', () => { ...defaultProps, ...props, }, + provide: { + dataMethod: GRAPHQL, + }, }); }; @@ -33,7 +38,7 @@ describe('graph component', () => { describe('with data', () => { beforeEach(() => { - createComponent(); + createComponent({ mountFn: mount }); }); it('renders the main columns in the graph', () => { @@ -43,11 +48,24 @@ describe('graph component', () => { describe('when linked pipelines are not present', () => { beforeEach(() => { - createComponent(); + createComponent({ mountFn: mount }); }); it('should not render a linked pipelines column', () => { expect(findLinkedColumns()).toHaveLength(0); }); }); + + describe('when linked pipelines are present', () => { + beforeEach(() => { + createComponent({ + mountFn: mount, + props: { pipeline: pipelineWithUpstreamDownstream(mockPipelineResponse) }, + }); + }); + + it('should render linked pipelines columns', () => { + expect(findLinkedColumns()).toHaveLength(2); + }); + }); }); diff --git a/spec/frontend/pipelines/graph/linked_pipeline_spec.js b/spec/frontend/pipelines/graph/linked_pipeline_spec.js index 67986ca7739..fb005d628a9 100644 --- a/spec/frontend/pipelines/graph/linked_pipeline_spec.js +++ b/spec/frontend/pipelines/graph/linked_pipeline_spec.js @@ -17,7 +17,7 @@ describe('Linked pipeline', () => { const findLinkedPipeline = () => wrapper.find({ ref: 'linkedPipeline' }); const findLoadingIcon = () => wrapper.find(GlLoadingIcon); const findPipelineLink = () => wrapper.find('[data-testid="pipelineLink"]'); - const findExpandButton = () => wrapper.find('[data-testid="expandPipelineButton"]'); + const findExpandButton = () => wrapper.find('[data-testid="expand-pipeline-button"]'); const createWrapper = (propsData, data = []) => { wrapper = mount(LinkedPipelineComponent, { @@ -40,20 +40,13 @@ describe('Linked pipeline', () => { projectId: invalidTriggeredPipelineId, columnTitle: 'Downstream', type: DOWNSTREAM, + expanded: false, }; beforeEach(() => { createWrapper(props); }); - it('should render a list item as the containing element', () => { - expect(wrapper.element.tagName).toBe('LI'); - }); - - it('should render a button', () => { - expect(findButton().exists()).toBe(true); - }); - it('should render the project name', () => { expect(wrapper.text()).toContain(props.pipeline.project.name); }); @@ -105,12 +98,14 @@ describe('Linked pipeline', () => { projectId: validTriggeredPipelineId, columnTitle: 'Downstream', type: DOWNSTREAM, + expanded: false, }; const upstreamProps = { ...downstreamProps, columnTitle: 'Upstream', type: UPSTREAM, + expanded: false, }; it('parent/child label container should exist', () => { @@ -173,7 +168,7 @@ describe('Linked pipeline', () => { `( '$pipelineType.columnTitle pipeline button icon should be $anglePosition if expanded state is $expanded', ({ pipelineType, anglePosition, expanded }) => { - createWrapper(pipelineType, { expanded }); + createWrapper({ ...pipelineType, expanded }); expect(findExpandButton().props('icon')).toBe(anglePosition); }, ); @@ -185,6 +180,7 @@ describe('Linked pipeline', () => { projectId: invalidTriggeredPipelineId, columnTitle: 'Downstream', type: DOWNSTREAM, + expanded: false, }; beforeEach(() => { @@ -202,6 +198,7 @@ describe('Linked pipeline', () => { projectId: validTriggeredPipelineId, columnTitle: 'Downstream', type: DOWNSTREAM, + expanded: false, }; beforeEach(() => { @@ -219,10 +216,7 @@ describe('Linked pipeline', () => { jest.spyOn(wrapper.vm.$root, '$emit'); findButton().trigger('click'); - expect(wrapper.vm.$root.$emit.mock.calls[0]).toEqual([ - 'bv::hide::tooltip', - 'js-linked-pipeline-34993051', - ]); + expect(wrapper.vm.$root.$emit.mock.calls[0]).toEqual(['bv::hide::tooltip']); }); it('should emit downstreamHovered with job name on mouseover', () => { diff --git a/spec/frontend/pipelines/graph/linked_pipelines_column_spec.js b/spec/frontend/pipelines/graph/linked_pipelines_column_spec.js index e6ae3154d1d..37eb5f900dd 100644 --- a/spec/frontend/pipelines/graph/linked_pipelines_column_spec.js +++ b/spec/frontend/pipelines/graph/linked_pipelines_column_spec.js @@ -1,40 +1,120 @@ -import { shallowMount } from '@vue/test-utils'; +import VueApollo from 'vue-apollo'; +import { mount, shallowMount, createLocalVue } from '@vue/test-utils'; +import createMockApollo from 'jest/helpers/mock_apollo_helper'; +import PipelineGraph from '~/pipelines/components/graph/graph_component.vue'; import LinkedPipelinesColumn from '~/pipelines/components/graph/linked_pipelines_column.vue'; import LinkedPipeline from '~/pipelines/components/graph/linked_pipeline.vue'; -import { UPSTREAM } from '~/pipelines/components/graph/constants'; -import mockData from './linked_pipelines_mock_data'; +import getPipelineDetails from '~/pipelines/graphql/queries/get_pipeline_details.query.graphql'; +import { DOWNSTREAM, GRAPHQL } from '~/pipelines/components/graph/constants'; +import { LOAD_FAILURE } from '~/pipelines/constants'; +import { + mockPipelineResponse, + pipelineWithUpstreamDownstream, + wrappedPipelineReturn, +} from './mock_data'; + +const processedPipeline = pipelineWithUpstreamDownstream(mockPipelineResponse); describe('Linked Pipelines Column', () => { - const propsData = { + const defaultProps = { columnTitle: 'Upstream', - linkedPipelines: mockData.triggered, - graphPosition: 'right', - projectId: 19, - type: UPSTREAM, + linkedPipelines: processedPipeline.downstream, + type: DOWNSTREAM, }; + let wrapper; + const findLinkedColumnTitle = () => wrapper.find('[data-testid="linked-column-title"]'); + const findLinkedPipelineElements = () => wrapper.findAll(LinkedPipeline); + const findPipelineGraph = () => wrapper.find(PipelineGraph); + const findExpandButton = () => wrapper.find('[data-testid="expand-pipeline-button"]'); - beforeEach(() => { - wrapper = shallowMount(LinkedPipelinesColumn, { propsData }); - }); + const localVue = createLocalVue(); + localVue.use(VueApollo); + + const createComponent = ({ apolloProvider, mountFn = shallowMount, props = {} } = {}) => { + wrapper = mountFn(LinkedPipelinesColumn, { + apolloProvider, + localVue, + propsData: { + ...defaultProps, + ...props, + }, + provide: { + dataMethod: GRAPHQL, + }, + }); + }; + + const createComponentWithApollo = ( + mountFn = shallowMount, + getPipelineDetailsHandler = jest.fn().mockResolvedValue(wrappedPipelineReturn), + ) => { + const requestHandlers = [[getPipelineDetails, getPipelineDetailsHandler]]; + + const apolloProvider = createMockApollo(requestHandlers); + createComponent({ apolloProvider, mountFn }); + }; afterEach(() => { wrapper.destroy(); + wrapper = null; }); - it('renders the pipeline orientation', () => { - const titleElement = wrapper.find('.linked-pipelines-column-title'); + describe('it renders correctly', () => { + beforeEach(() => { + createComponent(); + }); + + it('renders the pipeline title', () => { + expect(findLinkedColumnTitle().text()).toBe(defaultProps.columnTitle); + }); - expect(titleElement.text()).toBe(propsData.columnTitle); + it('renders the correct number of linked pipelines', () => { + expect(findLinkedPipelineElements()).toHaveLength(defaultProps.linkedPipelines.length); + }); }); - it('renders the correct number of linked pipelines', () => { - const linkedPipelineElements = wrapper.findAll(LinkedPipeline); + describe('click action', () => { + const clickExpandButton = async () => { + await findExpandButton().trigger('click'); + await wrapper.vm.$nextTick(); + }; - expect(linkedPipelineElements.length).toBe(propsData.linkedPipelines.length); - }); + const clickExpandButtonAndAwaitTimers = async () => { + await clickExpandButton(); + jest.runOnlyPendingTimers(); + await wrapper.vm.$nextTick(); + }; + + describe('when successful', () => { + beforeEach(() => { + createComponentWithApollo(mount); + }); + + it('toggles the pipeline visibility', async () => { + expect(findPipelineGraph().exists()).toBe(false); + await clickExpandButtonAndAwaitTimers(); + expect(findPipelineGraph().exists()).toBe(true); + await clickExpandButton(); + expect(findPipelineGraph().exists()).toBe(false); + }); + }); + + describe('on error', () => { + beforeEach(() => { + createComponentWithApollo(mount, jest.fn().mockRejectedValue(new Error('GraphQL error'))); + }); + + it('emits the error', async () => { + await clickExpandButton(); + expect(wrapper.emitted().error).toEqual([[LOAD_FAILURE]]); + }); - it('renders cross project triangle when column is upstream', () => { - expect(wrapper.find('.cross-project-triangle').exists()).toBe(true); + it('does not show the pipeline', async () => { + expect(findPipelineGraph().exists()).toBe(false); + await clickExpandButtonAndAwaitTimers(); + expect(findPipelineGraph().exists()).toBe(false); + }); + }); }); }); diff --git a/spec/frontend/pipelines/graph/mock_data.js b/spec/frontend/pipelines/graph/mock_data.js index b468af7ef25..d53a11eea0e 100644 --- a/spec/frontend/pipelines/graph/mock_data.js +++ b/spec/frontend/pipelines/graph/mock_data.js @@ -1,10 +1,15 @@ +import { unwrapPipelineData } from '~/pipelines/components/graph/utils'; + export const mockPipelineResponse = { data: { project: { __typename: 'Project', pipeline: { __typename: 'Pipeline', - id: '22', + id: 163, + iid: '22', + downstream: null, + upstream: null, stages: { __typename: 'CiStageConnection', nodes: [ @@ -497,3 +502,164 @@ export const mockPipelineResponse = { }, }, }; + +export const downstream = { + nodes: [ + { + id: 175, + iid: '31', + path: '/root/elemenohpee/-/pipelines/175', + status: { + group: 'success', + label: 'passed', + icon: 'status_success', + __typename: 'DetailedStatus', + }, + sourceJob: { + name: 'test_c', + __typename: 'CiJob', + }, + project: { + id: 'gid://gitlab/Project/25', + name: 'elemenohpee', + fullPath: 'root/elemenohpee', + __typename: 'Project', + }, + __typename: 'Pipeline', + multiproject: true, + }, + { + id: 181, + iid: '27', + path: '/root/abcd-dag/-/pipelines/181', + status: { + group: 'success', + label: 'passed', + icon: 'status_success', + __typename: 'DetailedStatus', + }, + sourceJob: { + name: 'test_d', + __typename: 'CiJob', + }, + project: { + id: 'gid://gitlab/Project/23', + name: 'abcd-dag', + fullPath: 'root/abcd-dag', + __typename: 'Project', + }, + __typename: 'Pipeline', + multiproject: false, + }, + ], +}; + +export const upstream = { + id: 161, + iid: '24', + path: '/root/abcd-dag/-/pipelines/161', + status: { + group: 'success', + label: 'passed', + icon: 'status_success', + __typename: 'DetailedStatus', + }, + sourceJob: null, + project: { + id: 'gid://gitlab/Project/23', + name: 'abcd-dag', + fullPath: 'root/abcd-dag', + __typename: 'Project', + }, + __typename: 'Pipeline', + multiproject: true, +}; + +export const wrappedPipelineReturn = { + data: { + project: { + pipeline: { + id: 'gid://gitlab/Ci::Pipeline/175', + iid: '38', + downstream: { + nodes: [], + }, + upstream: { + id: 'gid://gitlab/Ci::Pipeline/174', + iid: '37', + path: '/root/elemenohpee/-/pipelines/174', + status: { + group: 'success', + label: 'passed', + icon: 'status_success', + }, + sourceJob: { + name: 'test_c', + }, + project: { + id: 'gid://gitlab/Project/25', + name: 'elemenohpee', + fullPath: 'root/elemenohpee', + }, + }, + stages: { + nodes: [ + { + name: 'build', + status: { + action: null, + }, + groups: { + nodes: [ + { + status: { + label: 'passed', + group: 'success', + icon: 'status_success', + }, + name: 'build_n', + size: 1, + jobs: { + nodes: [ + { + name: 'build_n', + scheduledAt: null, + needs: { + nodes: [], + }, + status: { + icon: 'status_success', + tooltip: 'passed', + hasDetails: true, + detailsPath: '/root/elemenohpee/-/jobs/1662', + group: 'success', + action: { + buttonTitle: 'Retry this job', + icon: 'retry', + path: '/root/elemenohpee/-/jobs/1662/retry', + title: 'Retry', + }, + }, + }, + ], + }, + }, + ], + }, + }, + ], + }, + }, + }, + }, +}; + +export const generateResponse = (raw, mockPath) => unwrapPipelineData(mockPath, raw.data); + +export const pipelineWithUpstreamDownstream = base => { + const pip = { ...base }; + pip.data.project.pipeline.downstream = downstream; + pip.data.project.pipeline.upstream = upstream; + + return generateResponse(pip, 'root/abcd-dag'); +}; diff --git a/spec/frontend/static_site_editor/services/submit_content_changes_spec.js b/spec/frontend/static_site_editor/services/submit_content_changes_spec.js index 5018da7300b..b75c0b7df8c 100644 --- a/spec/frontend/static_site_editor/services/submit_content_changes_spec.js +++ b/spec/frontend/static_site_editor/services/submit_content_changes_spec.js @@ -9,6 +9,8 @@ import { SUBMIT_CHANGES_MERGE_REQUEST_ERROR, TRACKING_ACTION_CREATE_COMMIT, TRACKING_ACTION_CREATE_MERGE_REQUEST, + USAGE_PING_TRACKING_ACTION_CREATE_COMMIT, + USAGE_PING_TRACKING_ACTION_CREATE_MERGE_REQUEST, } from '~/static_site_editor/constants'; import generateBranchName from '~/static_site_editor/services/generate_branch_name'; import submitContentChanges from '~/static_site_editor/services/submit_content_changes'; @@ -201,4 +203,26 @@ describe('submitContentChanges', () => { ); }); }); + + describe('sends the correct Usage Ping tracking event', () => { + beforeEach(() => { + jest.spyOn(Api, 'trackRedisCounterEvent').mockResolvedValue({ data: '' }); + }); + + it('for commiting changes', () => { + return submitContentChanges(buildPayload()).then(() => { + expect(Api.trackRedisCounterEvent).toHaveBeenCalledWith( + USAGE_PING_TRACKING_ACTION_CREATE_COMMIT, + ); + }); + }); + + it('for creating a merge request', () => { + return submitContentChanges(buildPayload()).then(() => { + expect(Api.trackRedisCounterEvent).toHaveBeenCalledWith( + USAGE_PING_TRACKING_ACTION_CREATE_MERGE_REQUEST, + ); + }); + }); + }); }); diff --git a/spec/lib/gitlab/usage_data_spec.rb b/spec/lib/gitlab/usage_data_spec.rb index 7e1fa3280b6..49b4051716c 100644 --- a/spec/lib/gitlab/usage_data_spec.rb +++ b/spec/lib/gitlab/usage_data_spec.rb @@ -1313,7 +1313,7 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do context 'and product_analytics FF is enabled for it' do before do - stub_feature_flags(product_analytics: project) + stub_feature_flags(product_analytics_tracking: true) create(:product_analytics_event, project: project, se_category: 'epics', se_action: 'promote') create(:product_analytics_event, project: project, se_category: 'epics', se_action: 'promote', collector_tstamp: 2.days.ago) @@ -1329,7 +1329,7 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do context 'and product_analytics FF is disabled' do before do - stub_feature_flags(product_analytics: false) + stub_feature_flags(product_analytics_tracking: false) end it 'returns an empty hash' do diff --git a/spec/services/merge_requests/after_create_service_spec.rb b/spec/services/merge_requests/after_create_service_spec.rb index 840b7bc0a1c..69bab3b1ea4 100644 --- a/spec/services/merge_requests/after_create_service_spec.rb +++ b/spec/services/merge_requests/after_create_service_spec.rb @@ -18,32 +18,34 @@ RSpec.describe MergeRequests::AfterCreateService do allow(after_create_service).to receive(:notification_service).and_return(notification_service) end + subject(:execute_service) { after_create_service.execute(merge_request) } + it 'creates a merge request open event' do expect(event_service) .to receive(:open_mr).with(merge_request, merge_request.author) - after_create_service.execute(merge_request) + execute_service end it 'creates a new merge request notification' do expect(notification_service) .to receive(:new_merge_request).with(merge_request, merge_request.author) - after_create_service.execute(merge_request) + execute_service end it 'writes diffs to the cache' do expect(merge_request) .to receive_message_chain(:diffs, :write_cache) - after_create_service.execute(merge_request) + execute_service end it 'creates cross references' do expect(merge_request) .to receive(:create_cross_references!).with(merge_request.author) - after_create_service.execute(merge_request) + execute_service end it 'creates a pipeline and updates the HEAD pipeline' do @@ -51,7 +53,14 @@ RSpec.describe MergeRequests::AfterCreateService do .to receive(:create_pipeline_for).with(merge_request, merge_request.author) expect(merge_request).to receive(:update_head_pipeline) - after_create_service.execute(merge_request) + execute_service + end + + it 'records a namespace onboarding progress action' do + expect(NamespaceOnboardingAction).to receive(:create_action) + .with(merge_request.target_project.namespace, :merge_request_created).and_call_original + + expect { execute_service }.to change(NamespaceOnboardingAction, :count).by(1) end end end |