diff options
144 files changed, 1888 insertions, 1702 deletions
diff --git a/.gitlab/ci/docs.gitlab-ci.yml b/.gitlab/ci/docs.gitlab-ci.yml index de1110f39fa..5a3940bdac2 100644 --- a/.gitlab/ci/docs.gitlab-ci.yml +++ b/.gitlab/ci/docs.gitlab-ci.yml @@ -33,8 +33,8 @@ review-docs-deploy: - gem install gitlab --no-document - ./$SCRIPT_NAME deploy only: - - /(^docs[\/-].*|.*-docs$)/@gitlab-org/gitlab-ce - - /(^docs[\/-].*|.*-docs$)/@gitlab-org/gitlab-ee + - /(^docs[\/-].+|.+-docs$)/@gitlab-org/gitlab-ce + - /(^docs[\/-].+|.+-docs$)/@gitlab-org/gitlab-ee except: - /(^qa[\/-].*|.*-qa$)/ diff --git a/.gitlab/ci/frontend.gitlab-ci.yml b/.gitlab/ci/frontend.gitlab-ci.yml index fe369ffec13..2bdde9fedaf 100644 --- a/.gitlab/ci/frontend.gitlab-ci.yml +++ b/.gitlab/ci/frontend.gitlab-ci.yml @@ -73,7 +73,7 @@ gitlab:assets:compile pull-cache: refs: - master@gitlab-org/gitlab-ce - master@gitlab-org/gitlab-ee - - /(^docs[\/-].*|.*-docs$)/ + - /(^docs[\/-].+|.+-docs$)/ .compile-assets-metadata: extends: .dedicated-runner @@ -111,7 +111,7 @@ compile-assets pull-cache: refs: - master@gitlab-org/gitlab-ce - master@gitlab-org/gitlab-ee - - /(^docs[\/-].*|.*-docs$)/ + - /(^docs[\/-].+|.+-docs$)/ karma: extends: .dedicated-no-docs-pull-cache-job diff --git a/.gitlab/ci/global.gitlab-ci.yml b/.gitlab/ci/global.gitlab-ci.yml index 4da7f404767..78ef346d417 100644 --- a/.gitlab/ci/global.gitlab-ci.yml +++ b/.gitlab/ci/global.gitlab-ci.yml @@ -31,12 +31,12 @@ .no-docs: except: refs: - - /(^docs[\/-].*|.*-docs$)/ + - /(^docs[\/-].+|.+-docs$)/ .no-docs-and-no-qa: except: refs: - - /(^docs[\/-].*|.*-docs$)/ + - /(^docs[\/-].+|.+-docs$)/ - /(^qa[\/-].*|.*-qa$)/ .dedicated-no-docs-pull-cache-job: diff --git a/.gitlab/ci/rails.gitlab-ci.yml b/.gitlab/ci/rails.gitlab-ci.yml index 1392768127b..24b4eb3a4c1 100644 --- a/.gitlab/ci/rails.gitlab-ci.yml +++ b/.gitlab/ci/rails.gitlab-ci.yml @@ -207,7 +207,7 @@ downtime_check: - master - tags - /^[\d-]+-stable(-ee)?$/ - - /(^docs[\/-].*|.*-docs$)/ + - /(^docs[\/-].+|.+-docs$)/ - /(^qa[\/-].*|.*-qa$)/ dependencies: - setup-test-env @@ -223,6 +223,7 @@ ee_compat_check: - /^security-/ - branches@gitlab-org/gitlab-ee - branches@gitlab/gitlab-ee + - /(^docs[\/-].+|.+-docs$)/ retry: 0 artifacts: name: "${CI_JOB_NAME}_${CI_COMIT_REF_NAME}_${CI_COMMIT_SHA}" diff --git a/.gitlab/ci/review.gitlab-ci.yml b/.gitlab/ci/review.gitlab-ci.yml index aa722f6ef33..076858e6c8a 100644 --- a/.gitlab/ci/review.gitlab-ci.yml +++ b/.gitlab/ci/review.gitlab-ci.yml @@ -7,7 +7,7 @@ except: refs: - master - - /(^docs[\/-].*|.*-docs$)/ + - /(^docs[\/-].+|.+-docs$)/ .review-schedules-only: &review-schedules-only only: @@ -20,7 +20,7 @@ except: refs: - tags - - /(^docs[\/-].*|.*-docs$)/ + - /(^docs[\/-].+|.+-docs$)/ .review-base: &review-base extends: .dedicated-runner @@ -116,17 +116,21 @@ schedule:review-deploy: <<: *review-schedules-only review-stop: - <<: *review-base + <<: *review-only + extends: .single-script-job-dedicated-runner + image: registry.gitlab.com/gitlab-org/gitlab-build-images:gitlab-charts-build-base stage: review when: manual allow_failure: true variables: - GIT_DEPTH: "1" + SCRIPT_NAME: review_apps/review-apps.sh environment: <<: *review-environment action: stop script: - - source scripts/review_apps/review-apps.sh + - wget $CI_PROJECT_URL/raw/$CI_COMMIT_SHA/scripts/utils.sh + - source utils.sh + - source $(basename $SCRIPT_NAME) - delete .review-qa-base: &review-qa-base diff --git a/.gitlab/ci/test-metadata.gitlab-ci.yml b/.gitlab/ci/test-metadata.gitlab-ci.yml index 3a5735a2be9..4c97a4feb18 100644 --- a/.gitlab/ci/test-metadata.gitlab-ci.yml +++ b/.gitlab/ci/test-metadata.gitlab-ci.yml @@ -71,7 +71,7 @@ flaky-examples-check: except: refs: - master - - /(^docs[\/-].*|.*-docs$)/ + - /(^docs[\/-].+|.+-docs$)/ - /(^qa[\/-].*|.*-qa$)/ artifacts: expire_in: 30d diff --git a/app/assets/javascripts/clusters/stores/clusters_store.js b/app/assets/javascripts/clusters/stores/clusters_store.js index f64f0ca616f..ada5a49e246 100644 --- a/app/assets/javascripts/clusters/stores/clusters_store.js +++ b/app/assets/javascripts/clusters/stores/clusters_store.js @@ -171,6 +171,7 @@ export default class ClusterStore { this.state.applications.cert_manager.email || serverAppEntry.email; } else if (appId === JUPYTER) { this.state.applications.jupyter.hostname = + this.state.applications.jupyter.hostname || serverAppEntry.hostname || (this.state.applications.ingress.externalIp ? `jupyter.${this.state.applications.ingress.externalIp}.nip.io` diff --git a/app/assets/javascripts/commons/polyfills.js b/app/assets/javascripts/commons/polyfills.js index daa941a63cd..7a6ad3dc771 100644 --- a/app/assets/javascripts/commons/polyfills.js +++ b/app/assets/javascripts/commons/polyfills.js @@ -12,7 +12,6 @@ import 'core-js/es/promise/finally'; import 'core-js/es/string/code-point-at'; import 'core-js/es/string/from-code-point'; import 'core-js/es/string/includes'; -import 'core-js/es/string/repeat'; import 'core-js/es/string/starts-with'; import 'core-js/es/string/ends-with'; import 'core-js/es/symbol'; diff --git a/app/assets/javascripts/gl_form.js b/app/assets/javascripts/gl_form.js index b98fe9f6ce2..a66555838ba 100644 --- a/app/assets/javascripts/gl_form.js +++ b/app/assets/javascripts/gl_form.js @@ -3,16 +3,9 @@ import autosize from 'autosize'; import GfmAutoComplete, { defaultAutocompleteConfig } from 'ee_else_ce/gfm_auto_complete'; import dropzoneInput from './dropzone_input'; import { addMarkdownListeners, removeMarkdownListeners } from './lib/utils/text_markdown'; -import IndentHelper from './helpers/indent_helper'; -import { keystroke } from './lib/utils/common_utils'; -import * as keys from './lib/utils/keycodes'; -import UndoStack from './lib/utils/undo_stack'; export default class GLForm { constructor(form, enableGFM = {}) { - this.handleKeyShortcuts = this.handleKeyShortcuts.bind(this); - this.setState = this.setState.bind(this); - this.form = form; this.textarea = this.form.find('textarea.js-gfm-input'); this.enableGFM = Object.assign({}, defaultAutocompleteConfig, enableGFM); @@ -23,10 +16,6 @@ export default class GLForm { this.enableGFM[item] = Boolean(dataSources[item]); } }); - - this.undoStack = new UndoStack(); - this.indentHelper = new IndentHelper(this.textarea[0]); - // Before we start, we should clean up any previous data for this form this.destroy(); // Set up the form @@ -96,84 +85,9 @@ export default class GLForm { clearEventListeners() { this.textarea.off('focus'); this.textarea.off('blur'); - this.textarea.off('keydown'); removeMarkdownListeners(this.form); } - setState(state) { - const selection = [this.textarea[0].selectionStart, this.textarea[0].selectionEnd]; - this.textarea.val(state); - this.textarea[0].setSelectionRange(selection[0], selection[1]); - } - - /* - Handle keypresses for a custom undo/redo stack. - We need this because the toolbar buttons and indentation helpers mess with the browser's - native undo/redo capability. - */ - handleUndo(event) { - const content = this.textarea.val(); - const { selectionStart, selectionEnd } = this.textarea[0]; - const stack = this.undoStack; - - if (stack.isEmpty()) { - // ==== Save initial state in undo history ==== - stack.save(content); - } - - if (keystroke(event, keys.Z_KEY_CODE, 'l')) { - // ==== Undo ==== - event.preventDefault(); - stack.save(content); - if (stack.canUndo()) { - this.setState(stack.undo()); - } - } else if (keystroke(event, keys.Z_KEY_CODE, 'ls') || keystroke(event, keys.Y_KEY_CODE, 'l')) { - // ==== Redo ==== - event.preventDefault(); - if (stack.canRedo()) { - this.setState(stack.redo()); - } - } else if ( - keystroke(event, keys.SPACE_KEY_CODE) || - keystroke(event, keys.ENTER_KEY_CODE) || - selectionStart !== selectionEnd - ) { - // ==== Save after finishing a word or before deleting a large selection ==== - stack.save(content); - } else if (content === '') { - // ==== Save after deleting everything ==== - stack.save(''); - } else { - // ==== Save after 1 second of inactivity ==== - stack.scheduleSave(content); - } - } - - handleIndent(event) { - if (keystroke(event, keys.LEFT_BRACKET_KEY_CODE, 'l')) { - // ==== Unindent selected lines ==== - event.preventDefault(); - this.indentHelper.unindent(); - } else if (keystroke(event, keys.RIGHT_BRACKET_KEY_CODE, 'l')) { - // ==== Indent selected lines ==== - event.preventDefault(); - this.indentHelper.indent(); - } else if (keystroke(event, keys.ENTER_KEY_CODE)) { - // ==== Auto-indent new lines ==== - event.preventDefault(); - this.indentHelper.newline(); - } else if (keystroke(event, keys.BACKSPACE_KEY_CODE)) { - // ==== Auto-delete indents at the beginning of the line ==== - this.indentHelper.backspace(event); - } - } - - handleKeyShortcuts(event) { - this.handleIndent(event); - this.handleUndo(event); - } - addEventListeners() { this.textarea.on('focus', function focusTextArea() { $(this) @@ -185,6 +99,5 @@ export default class GLForm { .closest('.md-area') .removeClass('is-focused'); }); - this.textarea.on('keydown', e => this.handleKeyShortcuts(e.originalEvent)); } } diff --git a/app/assets/javascripts/helpers/indent_helper.js b/app/assets/javascripts/helpers/indent_helper.js deleted file mode 100644 index a8815fac04e..00000000000 --- a/app/assets/javascripts/helpers/indent_helper.js +++ /dev/null @@ -1,182 +0,0 @@ -const INDENT_SEQUENCE = ' '; - -function countLeftSpaces(text) { - const i = text.split('').findIndex(c => c !== ' '); - return i === -1 ? text.length : i; -} - -/** - * IndentHelper provides methods that allow manual and smart indentation in - * textareas. It supports line indent/unindent, selection indent/unindent, - * auto indentation on newlines, and smart deletion of indents with backspace. - */ -export default class IndentHelper { - /** - * Creates a new IndentHelper and binds it to the given `textarea`. You can provide a custom indent sequence in the second parameter, but the `newline` and `backspace` operations may work funny if the indent sequence isn't spaces only. - */ - constructor(textarea, indentSequence = INDENT_SEQUENCE) { - this.element = textarea; - this.seq = indentSequence; - } - - getSelection() { - return { start: this.element.selectionStart, end: this.element.selectionEnd }; - } - - isRangeSelection() { - return this.element.selectionStart !== this.element.selectionEnd; - } - - /** - * Re-implementation of textarea's setRangeText method, because IE/Edge don't support it. - * - * @see https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#dom-textarea%2Finput-setrangetext - */ - setRangeText(replacement, start, end, selectMode) { - // Disable eslint to remain as faithful as possible to the above linked spec - /* eslint-disable no-param-reassign, no-case-declarations */ - const text = this.element.value; - - if (start > end) { - throw new RangeError('setRangeText: start index must be less than or equal to end index'); - } - - // Clamp to [0, len] - start = Math.max(0, Math.min(start, text.length)); - end = Math.max(0, Math.min(end, text.length)); - - let selection = { start: this.element.selectionStart, end: this.element.selectionEnd }; - - this.element.value = text.slice(0, start) + replacement + text.slice(end); - - const newLength = replacement.length; - const newEnd = start + newLength; - - switch (selectMode) { - case 'select': - selection = { start, newEnd }; - break; - case 'start': - selection = { start, end: start }; - break; - case 'end': - selection = { start: newEnd, end: newEnd }; - break; - case 'preserve': - default: - const oldLength = end - start; - const delta = newLength - oldLength; - if (selection.start > end) { - selection.start += delta; - } else if (selection.start > start) { - selection.start = start; - } - if (selection.end > end) { - selection.end += delta; - } else if (selection.end > start) { - selection.end = newEnd; - } - } - - this.element.setSelectionRange(selection.start, selection.end); - - /* eslint-enable no-param-reassign, no-case-declarations */ - } - - /** - * Returns an array of lines in the textarea, with information about their - * start/end offsets and whether they are included in the current selection. - */ - splitLines() { - const { start, end } = this.getSelection(); - - const lines = this.element.value.split('\n'); - let textStart = 0; - const lineObjects = []; - lines.forEach(line => { - const lineObj = { - text: line, - start: textStart, - end: textStart + line.length, - }; - lineObj.inSelection = lineObj.start <= end && lineObj.end >= start; - lineObjects.push(lineObj); - textStart += line.length + 1; - }); - return lineObjects; - } - - /** - * Indents selected lines by one level. - */ - indent() { - const { start } = this.getSelection(); - - const selectedLines = this.splitLines().filter(line => line.inSelection); - if (!this.isRangeSelection() && start === selectedLines[0].start) { - // Special case: if cursor is at the beginning of the line, move it one - // indent right. - const line = selectedLines[0]; - this.setRangeText(this.seq, line.start, line.start, 'end'); - } else { - selectedLines.reverse(); - selectedLines.forEach(line => { - this.setRangeText(INDENT_SEQUENCE, line.start, line.start, 'preserve'); - }); - } - } - - /** - * Unindents selected lines by one level. - */ - unindent() { - const lines = this.splitLines().filter(line => line.inSelection); - lines.reverse(); - lines - .filter(line => line.text.startsWith(this.seq)) - .forEach(line => { - this.setRangeText('', line.start, line.start + this.seq.length, 'preserve'); - }); - } - - /** - * Emulates a newline keypress, automatically indenting the new line. - */ - newline() { - const { start, end } = this.getSelection(); - - if (this.isRangeSelection()) { - // Manually kill the selection before calculating the indent - this.setRangeText('', start, end, 'start'); - } - - // Auto-indent the next line - const currentLine = this.splitLines().find(line => line.end >= start); - const spaces = countLeftSpaces(currentLine.text); - this.setRangeText(`\n${' '.repeat(spaces)}`, start, start, 'end'); - } - - /** - * If the cursor is positioned at the end of a line's leading indents, - * emulates a backspace keypress by deleting a single level of indents. - * @param event The DOM KeyboardEvent that triggers this action, or null. - */ - backspace(event) { - const { start } = this.getSelection(); - - // If the cursor is at the end of leading indents, delete an indent. - if (!this.isRangeSelection()) { - const currentLine = this.splitLines().find(line => line.end >= start); - const cursorPosition = start - currentLine.start; - if (countLeftSpaces(currentLine.text) === cursorPosition && cursorPosition > 0) { - if (event) event.preventDefault(); - - let spacesToDelete = cursorPosition % this.seq.length; - if (spacesToDelete === 0) { - spacesToDelete = this.seq.length; - } - this.setRangeText('', start - spacesToDelete, start, 'start'); - } - } - } -} diff --git a/app/assets/javascripts/jobs/components/sidebar.vue b/app/assets/javascripts/jobs/components/sidebar.vue index e9704584c9f..06477477aad 100644 --- a/app/assets/javascripts/jobs/components/sidebar.vue +++ b/app/assets/javascripts/jobs/components/sidebar.vue @@ -73,15 +73,14 @@ export default { }, renderBlock() { return ( - this.job.merge_request || this.job.duration || - this.job.finished_data || + this.job.finished_at || this.job.erased_at || this.job.queued || + this.hasTimeout || this.job.runner || this.job.coverage || - this.job.tags.length || - this.job.cancel_path + this.job.tags.length ); }, hasArtifact() { @@ -160,7 +159,7 @@ export default { </gl-link> </div> - <div :class="{ block: renderBlock }"> + <div v-if="renderBlock" class="block"> <detail-row v-if="job.duration" :value="duration" diff --git a/app/assets/javascripts/lib/utils/common_utils.js b/app/assets/javascripts/lib/utils/common_utils.js index 1a94aee2398..5e90893b684 100644 --- a/app/assets/javascripts/lib/utils/common_utils.js +++ b/app/assets/javascripts/lib/utils/common_utils.js @@ -203,71 +203,6 @@ export const isMetaKey = e => e.metaKey || e.ctrlKey || e.altKey || e.shiftKey; // 3) Middle-click or Mouse Wheel Click (e.which is 2) export const isMetaClick = e => e.metaKey || e.ctrlKey || e.which === 2; -export const getPlatformLeaderKey = () => { - // eslint-disable-next-line @gitlab/i18n/no-non-i18n-strings - if (navigator && navigator.platform && navigator.platform.startsWith('Mac')) { - return 'meta'; - } - return 'ctrl'; -}; - -export const getPlatformLeaderKeyHTML = () => { - if (getPlatformLeaderKey() === 'meta') { - return '⌘'; - } - // eslint-disable-next-line @gitlab/i18n/no-non-i18n-strings - return 'Ctrl'; -}; - -export const isPlatformLeaderKey = e => { - if (getPlatformLeaderKey() === 'meta') { - return Boolean(e.metaKey); - } - return Boolean(e.ctrlKey); -}; - -/** - * Tests if a KeyboardEvent corresponds exactly to a keystroke. - * - * This function avoids hacking around an old version of Mousetrap, which we ship at the moment. It should be removed after we upgrade to the newest Mousetrap. See: - * - https://gitlab.com/gitlab-org/gitlab-ce/issues/63182 - * - https://gitlab.com/gitlab-org/gitlab-ce/issues/64246 - * - * @example - * // Matches the enter key with exactly zero modifiers - * keystroke(event, 13) - * - * @example - * // Matches Control-Shift-Z - * keystroke(event, 90, 'cs') - * - * @param e The KeyboardEvent to test. - * @param keyCode The key code of the key to test. Why keycodes? IE/Edge don't support the more convenient `key` and `code` properties. - * @param modifiers A string of modifiers keys. Each modifier key is represented by one character. The set of pressed modifier keys must match the given string exactly. Available options are 'a' for Alt/Option, 'c' for Control, 'm' for Meta/Command, 's' for Shift, and 'l' for the leader key (Meta on MacOS and Control otherwise). - * @returns {boolean} True if the KeyboardEvent corresponds to the given keystroke. - */ -export const keystroke = (e, keyCode, modifiers = '') => { - if (!e || !keyCode) { - return false; - } - - const leader = getPlatformLeaderKey(); - const mods = modifiers.toLowerCase().replace('l', leader.charAt(0)); - - // Match depressed modifier keys - if ( - e.altKey !== mods.includes('a') || - e.ctrlKey !== mods.includes('c') || - e.metaKey !== mods.includes('m') || - e.shiftKey !== mods.includes('s') - ) { - return false; - } - - // Match the depressed key - return keyCode === (e.keyCode || e.which); -}; - export const contentTop = () => { const perfBar = $('#js-peek').outerHeight() || 0; const mrTabsHeight = $('.merge-request-tabs').outerHeight() || 0; diff --git a/app/assets/javascripts/lib/utils/keycodes.js b/app/assets/javascripts/lib/utils/keycodes.js index e24fcf47d71..5e0f9b612a2 100644 --- a/app/assets/javascripts/lib/utils/keycodes.js +++ b/app/assets/javascripts/lib/utils/keycodes.js @@ -1,10 +1,4 @@ -export const BACKSPACE_KEY_CODE = 8; -export const ENTER_KEY_CODE = 13; -export const ESC_KEY_CODE = 27; -export const SPACE_KEY_CODE = 32; export const UP_KEY_CODE = 38; export const DOWN_KEY_CODE = 40; -export const Y_KEY_CODE = 89; -export const Z_KEY_CODE = 90; -export const LEFT_BRACKET_KEY_CODE = 219; -export const RIGHT_BRACKET_KEY_CODE = 221; +export const ENTER_KEY_CODE = 13; +export const ESC_KEY_CODE = 27; diff --git a/app/assets/javascripts/lib/utils/undo_stack.js b/app/assets/javascripts/lib/utils/undo_stack.js deleted file mode 100644 index 6cfdc2a0a0f..00000000000 --- a/app/assets/javascripts/lib/utils/undo_stack.js +++ /dev/null @@ -1,105 +0,0 @@ -/** - * UndoStack provides a custom implementation of an undo/redo engine. It was originally written for GitLab's Markdown editor (`gl_form.js`), whose rich text editing capabilities broke native browser undo/redo behaviour. - * - * UndoStack supports predictable undos/redos, debounced saves, maximum history length, and duplicate detection. - * - * Usage: - * - `stack = new UndoStack();` - * - Saves a state to the stack with `stack.save(state)`. - * - Get the current state with `stack.current()`. - * - Revert to the previous state with `stack.undo()`. - * - Redo a previous undo with `stack.redo()`; - * - Queue a future save with `stack.scheduleSave(state, delay)`. Useful for text editors. - * - See the full undo history in `stack.history`. - */ -export default class UndoStack { - constructor(maxLength = 1000) { - this.clear(); - this.maxLength = maxLength; - - // If you're storing reference-types in the undo stack, you might want to - // reassign this property to some deep-equals function. - this.comparator = (a, b) => a === b; - } - - current() { - if (this.cursor === -1) { - return undefined; - } - return this.history[this.cursor]; - } - - isEmpty() { - return this.history.length === 0; - } - - clear() { - this.clearPending(); - this.history = []; - this.cursor = -1; - } - - save(state) { - this.clearPending(); - if (this.comparator(state, this.current())) { - // Don't save state if it's the same as the current state - return; - } - - this.history.length = this.cursor + 1; - this.history.push(state); - this.cursor += 1; - - if (this.history.length > this.maxLength) { - this.history.shift(); - this.cursor -= 1; - } - } - - scheduleSave(state, delay = 1000) { - this.clearPending(); - this.pendingState = state; - this.timeout = setTimeout(this.saveNow.bind(this), delay); - } - - saveNow() { - // Persists scheduled saves immediately - this.save(this.pendingState); - this.clearPending(); - } - - clearPending() { - // Cancels any scheduled saves - if (this.timeout) { - clearTimeout(this.timeout); - delete this.timeout; - delete this.pendingState; - } - } - - canUndo() { - return this.cursor > 0; - } - - undo() { - this.clearPending(); - if (!this.canUndo()) { - return undefined; - } - this.cursor -= 1; - return this.history[this.cursor]; - } - - canRedo() { - return this.cursor >= 0 && this.cursor < this.history.length - 1; - } - - redo() { - this.clearPending(); - if (!this.canRedo()) { - return undefined; - } - this.cursor += 1; - return this.history[this.cursor]; - } -} diff --git a/app/assets/javascripts/vue_shared/components/markdown/toolbar.vue b/app/assets/javascripts/vue_shared/components/markdown/toolbar.vue index 21c44b59520..8ce5b615795 100644 --- a/app/assets/javascripts/vue_shared/components/markdown/toolbar.vue +++ b/app/assets/javascripts/vue_shared/components/markdown/toolbar.vue @@ -1,6 +1,5 @@ <script> import { GlLink } from '@gitlab/ui'; -import { s__, sprintf } from '~/locale'; export default { components: { @@ -23,28 +22,8 @@ export default { }, }, computed: { - toolbarHelpHtml() { - const mdLinkStart = `<a href="${this.markdownDocsPath}" target="_blank" rel="noopener noreferrer" tabindex="-1">`; - const actionsLinkStart = `<a href="${this.quickActionsDocsPath}" target="_blank" rel="noopener noreferrer" tabindex="-1">`; - const linkEnd = '</a>'; - - if (this.markdownDocsPath && !this.quickActionsDocsPath) { - return sprintf( - s__('Editor|%{mdLinkStart}Markdown is supported%{mdLinkEnd}'), - { mdLinkStart, mdLinkEnd: linkEnd }, - false, - ); - } else if (this.markdownDocsPath && this.quickActionsDocsPath) { - return sprintf( - s__( - 'Editor|%{mdLinkStart}Markdown%{mdLinkEnd} and %{actionsLinkStart}quick actions%{actionsLinkEnd} are supported', - ), - { mdLinkStart, mdLinkEnd: linkEnd, actionsLinkStart, actionsLinkEnd: linkEnd }, - false, - ); - } - - return null; + hasQuickActionsDocsPath() { + return this.quickActionsDocsPath !== ''; }, }, }; @@ -53,7 +32,21 @@ export default { <template> <div class="comment-toolbar clearfix"> <div class="toolbar-text"> - <span v-html="toolbarHelpHtml"></span> + <template v-if="!hasQuickActionsDocsPath && markdownDocsPath"> + <gl-link :href="markdownDocsPath" target="_blank" tabindex="-1">{{ + __('Markdown is supported') + }}</gl-link> + </template> + <template v-if="hasQuickActionsDocsPath && markdownDocsPath"> + <gl-link :href="markdownDocsPath" target="_blank" tabindex="-1">{{ + __('Markdown') + }}</gl-link> + and + <gl-link :href="quickActionsDocsPath" target="_blank" tabindex="-1">{{ + __('quick actions') + }}</gl-link> + are supported + </template> </div> <span v-if="canAttachFile" class="uploading-container"> <span class="uploading-progress-container hide"> diff --git a/app/controllers/autocomplete_controller.rb b/app/controllers/autocomplete_controller.rb index 091327931c2..f111c7ca8cc 100644 --- a/app/controllers/autocomplete_controller.rb +++ b/app/controllers/autocomplete_controller.rb @@ -16,7 +16,7 @@ class AutocompleteController < ApplicationController .new(params: params, current_user: current_user, project: project, group: group) .execute - render json: UserSerializer.new.represent(users) + render json: UserSerializer.new(params).represent(users, project: project) end def user diff --git a/app/controllers/boards/issues_controller.rb b/app/controllers/boards/issues_controller.rb index 90528f75ffd..1d1a72d21f1 100644 --- a/app/controllers/boards/issues_controller.rb +++ b/app/controllers/boards/issues_controller.rb @@ -26,7 +26,7 @@ module Boards list_service = Boards::Issues::ListService.new(board_parent, current_user, filter_params) issues = list_service.execute issues = issues.page(params[:page]).per(params[:per] || 20).without_count - Issue.move_to_end(issues) if Gitlab::Database.read_write? + Issue.move_nulls_to_end(issues) if Gitlab::Database.read_write? issues = issues.preload(:milestone, :assignees, project: [ diff --git a/app/controllers/concerns/issuable_actions.rb b/app/controllers/concerns/issuable_actions.rb index 6fa2f75be33..398cb728e05 100644 --- a/app/controllers/concerns/issuable_actions.rb +++ b/app/controllers/concerns/issuable_actions.rb @@ -98,13 +98,12 @@ module IssuableActions render json: { notice: "#{quantity} #{resource_name.pluralize(quantity)} updated" } end - # rubocop: disable CodeReuse/ActiveRecord + # rubocop:disable CodeReuse/ActiveRecord def discussions - notes = issuable.discussion_notes - .inc_relations_for_view - .with_notes_filter(notes_filter) - .includes(:noteable) - .fresh + notes = NotesFinder.new(current_user, finder_params_for_issuable).execute + .inc_relations_for_view + .includes(:noteable) + .fresh if notes_filter != UserPreference::NOTES_FILTERS[:only_comments] notes = ResourceEvents::MergeIntoNotesService.new(issuable, current_user).execute(notes) @@ -117,7 +116,7 @@ module IssuableActions render json: discussion_serializer.represent(discussions, context: self) end - # rubocop: enable CodeReuse/ActiveRecord + # rubocop:enable CodeReuse/ActiveRecord private @@ -222,4 +221,13 @@ module IssuableActions def parent @project || @group # rubocop:disable Gitlab/ModuleWithInstanceVariables end + + # rubocop:disable Gitlab/ModuleWithInstanceVariables + def finder_params_for_issuable + { + target: @issuable, + notes_filter: notes_filter + }.tap { |new_params| new_params[:project] = project if respond_to?(:project, true) } + end + # rubocop:enable Gitlab/ModuleWithInstanceVariables end diff --git a/app/controllers/concerns/notes_actions.rb b/app/controllers/concerns/notes_actions.rb index 0098c4cdf4c..d2a961efff7 100644 --- a/app/controllers/concerns/notes_actions.rb +++ b/app/controllers/concerns/notes_actions.rb @@ -243,7 +243,7 @@ module NotesActions end def notes_finder - @notes_finder ||= NotesFinder.new(project, current_user, finder_params) + @notes_finder ||= NotesFinder.new(current_user, finder_params) end def note_serializer diff --git a/app/controllers/projects/notes_controller.rb b/app/controllers/projects/notes_controller.rb index 3152a38fd8e..65d9b074eee 100644 --- a/app/controllers/projects/notes_controller.rb +++ b/app/controllers/projects/notes_controller.rb @@ -68,7 +68,7 @@ class Projects::NotesController < Projects::ApplicationController alias_method :awardable, :note def finder_params - params.merge(last_fetched_at: last_fetched_at, notes_filter: notes_filter) + params.merge(project: project, last_fetched_at: last_fetched_at, notes_filter: notes_filter) end def authorize_admin_note! diff --git a/app/controllers/snippets/notes_controller.rb b/app/controllers/snippets/notes_controller.rb index 612897f27e6..551b37cb3d3 100644 --- a/app/controllers/snippets/notes_controller.rb +++ b/app/controllers/snippets/notes_controller.rb @@ -27,7 +27,9 @@ class Snippets::NotesController < ApplicationController alias_method :noteable, :snippet def finder_params - params.merge(last_fetched_at: last_fetched_at, target_id: snippet.id, target_type: 'personal_snippet') + params.merge(last_fetched_at: last_fetched_at, target_id: snippet.id, target_type: 'personal_snippet').tap do |merged_params| + merged_params[:project] = project if respond_to?(:project) + end end def authorize_read_snippet! diff --git a/app/finders/notes_finder.rb b/app/finders/notes_finder.rb index 8f610d7dddb..f7d9100bb78 100644 --- a/app/finders/notes_finder.rb +++ b/app/finders/notes_finder.rb @@ -3,6 +3,8 @@ class NotesFinder FETCH_OVERLAP = 5.seconds + attr_reader :target_type + # Used to filter Notes # When used with target_type and target_id this returns notes specifically for the controller # @@ -10,15 +12,17 @@ class NotesFinder # current_user - which user check authorizations with # project - which project to look for notes on # params: + # target: noteable # target_type: string # target_id: integer # last_fetched_at: time # search: string # - def initialize(project, current_user, params = {}) - @project = project + def initialize(current_user, params = {}) + @project = params[:project] @current_user = current_user - @params = params + @params = params.dup + @target_type = @params[:target_type] end def execute @@ -32,7 +36,27 @@ class NotesFinder def target return @target if defined?(@target) - target_type = @params[:target_type] + if target_given? + use_explicit_target + else + find_target_by_type_and_ids + end + end + + private + + def target_given? + @params.key?(:target) + end + + def use_explicit_target + @target = @params[:target] + @target_type = @target.class.name.underscore + + @target + end + + def find_target_by_type_and_ids target_id = @params[:target_id] target_iid = @params[:target_iid] @@ -45,13 +69,11 @@ class NotesFinder @project.commit(target_id) end else - noteables_for_type_by_id(target_type, target_id, target_iid) + noteable_for_type_by_id(target_type, target_id, target_iid) end end - private - - def noteables_for_type_by_id(type, id, iid) + def noteable_for_type_by_id(type, id, iid) query = if id { id: id } else @@ -77,10 +99,6 @@ class NotesFinder search(notes) end - def target_type - @params[:target_type] - end - # rubocop: disable CodeReuse/ActiveRecord def notes_of_any_type types = %w(commit issue merge_request snippet) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 07813e03f3a..dd2bfc42af9 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -38,6 +38,7 @@ module Ci has_one :deployment, as: :deployable, class_name: 'Deployment' has_many :trace_sections, class_name: 'Ci::BuildTraceSection' has_many :trace_chunks, class_name: 'Ci::BuildTraceChunk', foreign_key: :build_id + has_many :needs, class_name: 'Ci::BuildNeed', foreign_key: :build_id, inverse_of: :build has_many :job_artifacts, class_name: 'Ci::JobArtifact', foreign_key: :job_id, dependent: :destroy, inverse_of: :job # rubocop:disable Cop/ActiveRecordDependent has_many :job_variables, class_name: 'Ci::JobVariable', foreign_key: :job_id @@ -50,6 +51,7 @@ module Ci accepts_nested_attributes_for :runner_session accepts_nested_attributes_for :job_variables + accepts_nested_attributes_for :needs delegate :url, to: :runner_session, prefix: true, allow_nil: true delegate :terminal_specification, to: :runner_session, allow_nil: true @@ -713,11 +715,21 @@ module Ci depended_jobs = depends_on_builds - return depended_jobs unless options[:dependencies].present? + # find all jobs that are dependent on + if options[:dependencies].present? + depended_jobs = depended_jobs.select do |job| + options[:dependencies].include?(job.name) + end + end - depended_jobs.select do |job| - options[:dependencies].include?(job.name) + # find all jobs that are needed by this one + if options[:needs].present? + depended_jobs = depended_jobs.select do |job| + options[:needs].include?(job.name) + end end + + depended_jobs end def empty_dependencies? diff --git a/app/models/ci/build_need.rb b/app/models/ci/build_need.rb new file mode 100644 index 00000000000..6531dfd332f --- /dev/null +++ b/app/models/ci/build_need.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +module Ci + class BuildNeed < ApplicationRecord + extend Gitlab::Ci::Model + + belongs_to :build, class_name: "Ci::Build", foreign_key: :build_id, inverse_of: :needs + + validates :build, presence: true + validates :name, presence: true, length: { maximum: 128 } + + scope :scoped_build, -> { where('ci_builds.id=ci_build_needs.build_id') } + end +end diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 1c76f401690..3515f0b83ee 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -611,8 +611,8 @@ module Ci end # rubocop: disable CodeReuse/ServiceClass - def process! - Ci::ProcessPipelineService.new(project, user).execute(self) + def process!(trigger_build_name = nil) + Ci::ProcessPipelineService.new(project, user).execute(self, trigger_build_name) end # rubocop: enable CodeReuse/ServiceClass diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index be6f3e9c5b0..d7eb78db5b8 100644 --- a/app/models/commit_status.rb +++ b/app/models/commit_status.rb @@ -43,6 +43,12 @@ class CommitStatus < ApplicationRecord scope :after_stage, -> (index) { where('stage_idx > ?', index) } scope :processables, -> { where(type: %w[Ci::Build Ci::Bridge]) } + scope :with_needs, -> (names = nil) do + needs = Ci::BuildNeed.scoped_build.select(1) + needs = needs.where(name: names) if names + where('EXISTS (?)', needs).preload(:needs) + end + # We use `CommitStatusEnums.failure_reasons` here so that EE can more easily # extend this `Hash` with new values. enum_with_nil failure_reason: ::CommitStatusEnums.failure_reasons @@ -116,7 +122,7 @@ class CommitStatus < ApplicationRecord commit_status.run_after_commit do if pipeline_id if complete? || manual? - PipelineProcessWorker.perform_async(pipeline_id) + BuildProcessWorker.perform_async(id) else PipelineUpdateWorker.perform_async(pipeline_id) end diff --git a/app/models/concerns/ci/metadatable.rb b/app/models/concerns/ci/metadatable.rb index 9eed9492b37..304cc71e9dc 100644 --- a/app/models/concerns/ci/metadatable.rb +++ b/app/models/concerns/ci/metadatable.rb @@ -29,6 +29,7 @@ module Ci def degenerate! self.class.transaction do self.update!(options: nil, yaml_variables: nil) + self.needs.all.delete_all self.metadata&.destroy end end diff --git a/app/models/concerns/diff_positionable_note.rb b/app/models/concerns/diff_positionable_note.rb index 2d09eff0111..195d9e107c5 100644 --- a/app/models/concerns/diff_positionable_note.rb +++ b/app/models/concerns/diff_positionable_note.rb @@ -10,6 +10,8 @@ module DiffPositionableNote serialize :original_position, Gitlab::Diff::Position # rubocop:disable Cop/ActiveRecordSerialize serialize :position, Gitlab::Diff::Position # rubocop:disable Cop/ActiveRecordSerialize serialize :change_position, Gitlab::Diff::Position # rubocop:disable Cop/ActiveRecordSerialize + + validate :diff_refs_match_commit, if: :for_commit? end %i(original_position position change_position).each do |meth| @@ -71,4 +73,10 @@ module DiffPositionableNote self.position = result[:position] end end + + def diff_refs_match_commit + return if self.original_position.diff_refs == commit&.diff_refs + + errors.add(:commit_id, 'does not match the diff refs') + end end diff --git a/app/models/concerns/relative_positioning.rb b/app/models/concerns/relative_positioning.rb index 9cd7b8d6258..4a1441805fc 100644 --- a/app/models/concerns/relative_positioning.rb +++ b/app/models/concerns/relative_positioning.rb @@ -34,7 +34,7 @@ module RelativePositioning end class_methods do - def move_to_end(objects) + def move_nulls_to_end(objects) objects = objects.reject(&:relative_position) return if objects.empty? @@ -43,7 +43,7 @@ module RelativePositioning self.transaction do objects.each do |object| - relative_position = position_between(max_relative_position, MAX_POSITION) + relative_position = position_between(max_relative_position || START_POSITION, MAX_POSITION) object.relative_position = relative_position max_relative_position = relative_position object.save(touch: false) diff --git a/app/models/container_repository.rb b/app/models/container_repository.rb index facd81dde80..2a5ae7930e6 100644 --- a/app/models/container_repository.rb +++ b/app/models/container_repository.rb @@ -70,10 +70,14 @@ class ContainerRepository < ApplicationRecord digests = tags.map { |tag| tag.digest }.to_set digests.all? do |digest| - client.delete_repository_tag(self.path, digest) + delete_tag_by_digest(digest) end end + def delete_tag_by_digest(digest) + client.delete_repository_tag(self.path, digest) + end + def self.build_from_path(path) self.new(project: path.repository_project, name: path.repository_name) diff --git a/app/models/diff_note.rb b/app/models/diff_note.rb index f75c32633b1..861185dc222 100644 --- a/app/models/diff_note.rb +++ b/app/models/diff_note.rb @@ -20,7 +20,6 @@ class DiffNote < Note validates :noteable_type, inclusion: { in: -> (_note) { noteable_types } } validate :positions_complete validate :verify_supported - validate :diff_refs_match_commit, if: :for_commit? before_validation :set_line_code, if: :on_text? after_save :keep_around_commits @@ -154,12 +153,6 @@ class DiffNote < Note errors.add(:position, "is invalid") end - def diff_refs_match_commit - return if self.original_position.diff_refs == self.commit.diff_refs - - errors.add(:commit_id, 'does not match the diff refs') - end - def keep_around_commits shas = [ self.original_position.base_sha, diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 8ade91933a4..5e8a6a7d5e5 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -752,7 +752,7 @@ class MergeRequest < ApplicationRecord end def check_mergeability - MergeRequests::MergeabilityCheckService.new(self).execute + MergeRequests::MergeabilityCheckService.new(self).execute(retry_lease: false) end # rubocop: enable CodeReuse/ServiceClass diff --git a/app/models/namespace/aggregation_schedule.rb b/app/models/namespace/aggregation_schedule.rb index 0bef352cf24..61a7eb4b576 100644 --- a/app/models/namespace/aggregation_schedule.rb +++ b/app/models/namespace/aggregation_schedule.rb @@ -6,21 +6,13 @@ class Namespace::AggregationSchedule < ApplicationRecord self.primary_key = :namespace_id - DEFAULT_LEASE_TIMEOUT = 3.hours + DEFAULT_LEASE_TIMEOUT = 1.5.hours.to_i REDIS_SHARED_KEY = 'gitlab:update_namespace_statistics_delay'.freeze belongs_to :namespace after_create :schedule_root_storage_statistics - def self.delay_timeout - redis_timeout = Gitlab::Redis::SharedState.with do |redis| - redis.get(REDIS_SHARED_KEY) - end - - redis_timeout.nil? ? DEFAULT_LEASE_TIMEOUT : redis_timeout.to_i - end - def schedule_root_storage_statistics run_after_commit_or_now do try_obtain_lease do @@ -28,7 +20,7 @@ class Namespace::AggregationSchedule < ApplicationRecord .perform_async(namespace_id) Namespaces::RootStatisticsWorker - .perform_in(self.class.delay_timeout, namespace_id) + .perform_in(DEFAULT_LEASE_TIMEOUT, namespace_id) end end end @@ -37,7 +29,7 @@ class Namespace::AggregationSchedule < ApplicationRecord # Used by ExclusiveLeaseGuard def lease_timeout - self.class.delay_timeout + DEFAULT_LEASE_TIMEOUT end # Used by ExclusiveLeaseGuard diff --git a/app/serializers/user_serializer.rb b/app/serializers/user_serializer.rb index 2111e1b5667..d988caea92d 100644 --- a/app/serializers/user_serializer.rb +++ b/app/serializers/user_serializer.rb @@ -2,4 +2,21 @@ class UserSerializer < BaseSerializer entity UserEntity + + def represent(resource, opts = {}, entity = nil) + if params[:merge_request_iid] + merge_request = opts[:project].merge_requests.find_by_iid!(params[:merge_request_iid]) + preload_max_member_access(merge_request.project, Array(resource)) + + super(resource, opts.merge(merge_request: merge_request), MergeRequestAssigneeEntity) + else + super + end + end + + private + + def preload_max_member_access(project, users) + project.team.max_member_access_for_user_ids(users.map(&:id)) + end end diff --git a/app/services/auth/container_registry_authentication_service.rb b/app/services/auth/container_registry_authentication_service.rb index 707caee482c..0a069320936 100644 --- a/app/services/auth/container_registry_authentication_service.rb +++ b/app/services/auth/container_registry_authentication_service.rb @@ -17,6 +17,14 @@ module Auth end def self.full_access_token(*names) + access_token(%w(*), names) + end + + def self.pull_access_token(*names) + access_token(['pull'], names) + end + + def self.access_token(actions, names) names = names.flatten registry = Gitlab.config.registry token = JSONWebToken::RSAToken.new(registry.key) @@ -25,7 +33,7 @@ module Auth token.expire_time = token_expire_at token[:access] = names.map do |name| - { type: 'repository', name: name, actions: %w(*) } + { type: 'repository', name: name, actions: actions } end token.encoded diff --git a/app/services/ci/process_pipeline_service.rb b/app/services/ci/process_pipeline_service.rb index 207cc5017d0..e46615bcf75 100644 --- a/app/services/ci/process_pipeline_service.rb +++ b/app/services/ci/process_pipeline_service.rb @@ -4,19 +4,23 @@ module Ci class ProcessPipelineService < BaseService attr_reader :pipeline - def execute(pipeline) + def execute(pipeline, trigger_build_name = nil) @pipeline = pipeline update_retried - new_builds = + success = stage_indexes_of_created_processables.flat_map do |index| process_stage(index) - end + end.any? + + # we evaluate dependent needs, + # only when the another job has finished + success = process_builds_with_needs(trigger_build_name) || success @pipeline.update_status - new_builds.any? + success end private @@ -36,6 +40,28 @@ module Ci end end + def process_builds_with_needs(trigger_build_name) + return false unless trigger_build_name + return false unless Feature.enabled?(:ci_dag_support, project) + + created_processables + .with_needs(trigger_build_name) + .find_each + .map(&method(:process_build_with_needs)) + .any? + end + + def process_build_with_needs(build) + current_status = status_for_build_needs(build.needs.map(&:name)) + + return unless HasStatus::COMPLETED_STATUSES.include?(current_status) + + Gitlab::OptimisticLocking.retry_lock(build) do |subject| + Ci::ProcessBuildService.new(project, @user) + .execute(subject, current_status) + end + end + # rubocop: disable CodeReuse/ActiveRecord def status_for_prior_stages(index) pipeline.builds.where('stage_idx < ?', index).latest.status || 'success' @@ -43,6 +69,12 @@ module Ci # rubocop: enable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord + def status_for_build_needs(needs) + pipeline.builds.where(name: needs).latest.status || 'success' + end + # rubocop: enable CodeReuse/ActiveRecord + + # rubocop: disable CodeReuse/ActiveRecord def stage_indexes_of_created_processables created_processables.order(:stage_idx).pluck(Arel.sql('DISTINCT stage_idx')) end diff --git a/app/services/ci/retry_build_service.rb b/app/services/ci/retry_build_service.rb index fab8a179843..338495ba030 100644 --- a/app/services/ci/retry_build_service.rb +++ b/app/services/ci/retry_build_service.rb @@ -5,7 +5,7 @@ module Ci CLONE_ACCESSORS = %i[pipeline project ref tag options name allow_failure stage stage_id stage_idx trigger_request yaml_variables when environment coverage_regex - description tag_list protected].freeze + description tag_list protected needs].freeze def execute(build) reprocess!(build).tap do |new_build| diff --git a/app/services/merge_requests/mergeability_check_service.rb b/app/services/merge_requests/mergeability_check_service.rb index 9fa50c9448f..962e2327b3e 100644 --- a/app/services/merge_requests/mergeability_check_service.rb +++ b/app/services/merge_requests/mergeability_check_service.rb @@ -3,6 +3,7 @@ module MergeRequests class MergeabilityCheckService < ::BaseService include Gitlab::Utils::StrongMemoize + include Gitlab::ExclusiveLeaseHelpers delegate :project, to: :@merge_request delegate :repository, to: :project @@ -21,13 +22,35 @@ module MergeRequests # where we need the current state of the merge ref in repository, the `recheck` # argument is required. # + # retry_lease - Concurrent calls wait for at least 10 seconds until the + # lease is granted (other process finishes running). Returns an error + # ServiceResponse if the lease is not granted during this time. + # # Returns a ServiceResponse indicating merge_status is/became can_be_merged # and the merge-ref is synced. Success in case of being/becoming mergeable, # error otherwise. - def execute(recheck: false) + def execute(recheck: false, retry_lease: true) return ServiceResponse.error(message: 'Invalid argument') unless merge_request return ServiceResponse.error(message: 'Unsupported operation') if Gitlab::Database.read_only? + return check_mergeability(recheck) unless merge_ref_auto_sync_lock_enabled? + + in_write_lock(retry_lease: retry_lease) do |retried| + # When multiple calls are waiting for the same lock (retry_lease), + # it's possible that when granted, the MR status was already updated for + # that object, therefore we reset if there was a lease retry. + merge_request.reset if retried + + check_mergeability(recheck) + end + rescue FailedToObtainLockError => error + ServiceResponse.error(message: error.message) + end + + private + + attr_reader :merge_request + def check_mergeability(recheck) recheck! if recheck update_merge_status @@ -46,9 +69,21 @@ module MergeRequests ServiceResponse.success(payload: payload) end - private + # It's possible for this service to send concurrent requests to Gitaly in order + # to "git update-ref" the same ref. Therefore we handle a light exclusive + # lease here. + # + def in_write_lock(retry_lease:, &block) + lease_key = "mergeability_check:#{merge_request.id}" - attr_reader :merge_request + lease_opts = { + ttl: 1.minute, + retries: retry_lease ? 10 : 0, + sleep_sec: retry_lease ? 1.second : 0 + } + + in_lock(lease_key, lease_opts, &block) + end def payload strong_memoize(:payload) do @@ -116,5 +151,9 @@ module MergeRequests def merge_ref_auto_sync_enabled? Feature.enabled?(:merge_ref_auto_sync, project, default_enabled: true) end + + def merge_ref_auto_sync_lock_enabled? + Feature.enabled?(:merge_ref_auto_sync_lock, project, default_enabled: true) + end end end diff --git a/app/views/explore/projects/_projects.html.haml b/app/views/explore/projects/_projects.html.haml index 67f2f897137..35b32662b8a 100644 --- a/app/views/explore/projects/_projects.html.haml +++ b/app/views/explore/projects/_projects.html.haml @@ -1 +1,2 @@ -= render 'shared/projects/list', projects: projects, user: current_user +- is_explore_page = defined?(explore_page) && explore_page += render 'shared/projects/list', projects: projects, user: current_user, explore_page: is_explore_page diff --git a/app/views/explore/projects/trending.html.haml b/app/views/explore/projects/trending.html.haml index ed508fa2506..153c90e534e 100644 --- a/app/views/explore/projects/trending.html.haml +++ b/app/views/explore/projects/trending.html.haml @@ -10,4 +10,4 @@ = render 'explore/head' = render 'explore/projects/nav' unless Feature.enabled?(:project_list_filter_bar) && current_user -= render 'projects', projects: @projects += render 'projects', projects: @projects, explore_page: true diff --git a/app/views/shared/milestones/_top.html.haml b/app/views/shared/milestones/_top.html.haml index 43503e1d08a..fd3317341f6 100644 --- a/app/views/shared/milestones/_top.html.haml +++ b/app/views/shared/milestones/_top.html.haml @@ -53,7 +53,7 @@ - close_msg = group ? 'You may close the milestone now.' : 'Navigate to the project to close the milestone.' %span All issues for this milestone are closed. #{close_msg} -= render_if_exists 'shared/milestones/burndown', milestone: @milestone, project: @project += render_if_exists 'shared/milestones/burndown', milestone: milestone, project: @project - if is_dynamic_milestone .table-holder diff --git a/app/views/shared/notes/_hints.html.haml b/app/views/shared/notes/_hints.html.haml index 72ede50dd8c..fae7d6526e8 100644 --- a/app/views/shared/notes/_hints.html.haml +++ b/app/views/shared/notes/_hints.html.haml @@ -1,13 +1,14 @@ - supports_quick_actions = local_assigns.fetch(:supports_quick_actions, false) .comment-toolbar.clearfix .toolbar-text - - md_link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer" tabindex="-1">'.html_safe % { url: help_page_path('user/markdown') } - - actions_link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer" tabindex="-1">'.html_safe % { url: help_page_path('user/project/quick_actions') } - - link_end = '</a>'.html_safe + = link_to _('Markdown'), help_page_path('user/markdown'), target: '_blank', tabindex: -1 - if supports_quick_actions - = s_('Editor|%{mdLinkStart}Markdown%{mdLinkEnd} and %{actionsLinkStart}quick actions%{actionsLinkEnd} are supported').html_safe % { mdLinkStart: md_link_start, mdLinkEnd: link_end, actionsLinkStart: actions_link_start, actionsLinkEnd: link_end } + and + = link_to _('quick actions'), help_page_path('user/project/quick_actions'), target: '_blank', tabindex: -1 + are - else - = s_('Editor|%{mdLinkStart}Markdown is supported%{mdLinkEnd}').html_safe % { mdLinkStart: md_link_start, mdLinkEnd: link_end } + is + supported %span.uploading-container %span.uploading-progress-container.hide diff --git a/app/views/shared/projects/_list.html.haml b/app/views/shared/projects/_list.html.haml index 576ec3e1782..67cb1aa549c 100644 --- a/app/views/shared/projects/_list.html.haml +++ b/app/views/shared/projects/_list.html.haml @@ -21,6 +21,7 @@ - own_projects_current_user_empty_message_header = s_('UserProfile|You haven\'t created any personal projects.') - own_projects_current_user_empty_message_description = s_('UserProfile|Your projects can be available publicly, internally, or privately, at your choice.') - own_projects_visitor_empty_message = s_('UserProfile|This user doesn\'t have any personal projects') +- explore_page_empty_message = s_('UserProfile|Explore public groups to find projects to contribute to.') - primary_button_label = _('New project') - primary_button_link = new_project_path - secondary_button_label = _('Explore groups') @@ -58,4 +59,4 @@ current_user_empty_message_description: own_projects_current_user_empty_message_description, primary_button_label: primary_button_label, primary_button_link: primary_button_link, - visitor_empty_message: own_projects_visitor_empty_message } + visitor_empty_message: defined?(explore_page) && explore_page ? explore_page_empty_message : own_projects_visitor_empty_message } diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index 991a177018e..400becdd023 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -88,6 +88,7 @@ - pipeline_processing:ci_build_prepare - pipeline_processing:build_queue - pipeline_processing:build_success +- pipeline_processing:build_process - pipeline_processing:pipeline_process - pipeline_processing:pipeline_success - pipeline_processing:pipeline_update diff --git a/app/workers/build_process_worker.rb b/app/workers/build_process_worker.rb new file mode 100644 index 00000000000..19e590ee1d7 --- /dev/null +++ b/app/workers/build_process_worker.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +class BuildProcessWorker + include ApplicationWorker + include PipelineQueue + + queue_namespace :pipeline_processing + + # rubocop: disable CodeReuse/ActiveRecord + def perform(build_id) + CommitStatus.find_by(id: build_id).try do |build| + build.pipeline.process!(build.name) + end + end + # rubocop: enable CodeReuse/ActiveRecord +end diff --git a/changelogs/unreleased/58256-incorrect-empty-state-message-displayed-on-explore-projects-tab.yml b/changelogs/unreleased/58256-incorrect-empty-state-message-displayed-on-explore-projects-tab.yml new file mode 100644 index 00000000000..f719338b9cb --- /dev/null +++ b/changelogs/unreleased/58256-incorrect-empty-state-message-displayed-on-explore-projects-tab.yml @@ -0,0 +1,5 @@ +--- +title: Resolve Incorrect empty state message on Explore projects +merge_request: 25578 +author: +type: fixed diff --git a/changelogs/unreleased/59521-job-sidebar-has-a-blank-block.yml b/changelogs/unreleased/59521-job-sidebar-has-a-blank-block.yml new file mode 100644 index 00000000000..4c93a108f2b --- /dev/null +++ b/changelogs/unreleased/59521-job-sidebar-has-a-blank-block.yml @@ -0,0 +1,5 @@ +--- +title: Remove blank block from job sidebar +merge_request: 30754 +author: +type: fixed diff --git a/changelogs/unreleased/implement-dag.yml b/changelogs/unreleased/implement-dag.yml new file mode 100644 index 00000000000..72f3f9a510c --- /dev/null +++ b/changelogs/unreleased/implement-dag.yml @@ -0,0 +1,5 @@ +--- +title: "Support creating DAGs in CI config through the `needs` key" +merge_request: 31328 +author: +type: added diff --git a/changelogs/unreleased/jupyter-fixes-v1.yml b/changelogs/unreleased/jupyter-fixes-v1.yml new file mode 100644 index 00000000000..7a34f273c90 --- /dev/null +++ b/changelogs/unreleased/jupyter-fixes-v1.yml @@ -0,0 +1,5 @@ +--- +title: Jupyter fixes +merge_request: 31332 +author: Amit Rathi +type: fixed diff --git a/changelogs/unreleased/mh-editor-indents.yml b/changelogs/unreleased/mh-editor-indents.yml deleted file mode 100644 index a282c0f505d..00000000000 --- a/changelogs/unreleased/mh-editor-indents.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Markdown editors now have indentation shortcuts and auto-indentation -merge_request: 28914 -author: -type: added diff --git a/changelogs/unreleased/osw-avoid-errors-due-to-concurrent-calls.yml b/changelogs/unreleased/osw-avoid-errors-due-to-concurrent-calls.yml new file mode 100644 index 00000000000..17ff1b012cf --- /dev/null +++ b/changelogs/unreleased/osw-avoid-errors-due-to-concurrent-calls.yml @@ -0,0 +1,5 @@ +--- +title: Add exclusive lease to mergeability check process +merge_request: 31082 +author: +type: fixed diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index dd53127ac2c..39b719a5978 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -427,6 +427,11 @@ production: &base # If it is blank, it defaults to external_url. node_name: '' + registry_replication: + # enabled: true + # primary_api_url: http://localhost:5000/ # internal address to the primary registry, will be used by GitLab to directly communicate with primary registry API + + # # 2. GitLab CI settings # ========================== diff --git a/config/initializers/0_inflections.rb b/config/initializers/0_inflections.rb index 4d1f4917275..d317825c1b8 100644 --- a/config/initializers/0_inflections.rb +++ b/config/initializers/0_inflections.rb @@ -19,6 +19,7 @@ ActiveSupport::Inflector.inflections do |inflect| project_registry file_registry job_artifact_registry + container_repository_registry vulnerability_feedback vulnerabilities_feedback group_view diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 32fec7c3d22..659801f787d 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -296,6 +296,12 @@ Gitlab.ee do Settings['geo'] ||= Settingslogic.new({}) # For backwards compatibility, default to gitlab_url and if so, ensure it ends with "/" Settings.geo['node_name'] = Settings.geo['node_name'].presence || Settings.gitlab['url'].chomp('/').concat('/') + + # + # Registry replication + # + Settings.geo['registry_replication'] ||= Settingslogic.new({}) + Settings.geo.registry_replication['enabled'] ||= false end # @@ -473,6 +479,9 @@ Gitlab.ee do Settings.cron_jobs['geo_repository_verification_secondary_scheduler_worker'] ||= Settingslogic.new({}) Settings.cron_jobs['geo_repository_verification_secondary_scheduler_worker']['cron'] ||= '*/1 * * * *' Settings.cron_jobs['geo_repository_verification_secondary_scheduler_worker']['job_class'] ||= 'Geo::RepositoryVerification::Secondary::SchedulerWorker' + Settings.cron_jobs['geo_container_repository_sync_worker'] ||= Settingslogic.new({}) + Settings.cron_jobs['geo_container_repository_sync_worker']['cron'] ||= '*/1 * * * *' + Settings.cron_jobs['geo_container_repository_sync_worker']['job_class'] ||= 'Geo::ContainerRepositorySyncDispatchWorker' Settings.cron_jobs['historical_data_worker'] ||= Settingslogic.new({}) Settings.cron_jobs['historical_data_worker']['cron'] ||= '0 12 * * *' Settings.cron_jobs['historical_data_worker']['job_class'] = 'HistoricalDataWorker' diff --git a/db/migrate/20190612111404_add_geo_container_sync_capacity.rb b/db/migrate/20190612111404_add_geo_container_sync_capacity.rb new file mode 100644 index 00000000000..d4cd569f460 --- /dev/null +++ b/db/migrate/20190612111404_add_geo_container_sync_capacity.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class AddGeoContainerSyncCapacity < ActiveRecord::Migration[5.1] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def change + change_table :geo_nodes do |t| + t.column :container_repositories_max_capacity, :integer, default: 10, null: false + end + end +end diff --git a/db/migrate/20190703043358_add_commit_id_to_draft_notes.rb b/db/migrate/20190703043358_add_commit_id_to_draft_notes.rb new file mode 100644 index 00000000000..022400ce585 --- /dev/null +++ b/db/migrate/20190703043358_add_commit_id_to_draft_notes.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class AddCommitIdToDraftNotes < ActiveRecord::Migration[5.1] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def change + add_column :draft_notes, :commit_id, :binary + end +end diff --git a/db/migrate/20190731084415_add_build_need.rb b/db/migrate/20190731084415_add_build_need.rb new file mode 100644 index 00000000000..45b8abb480d --- /dev/null +++ b/db/migrate/20190731084415_add_build_need.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddBuildNeed < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def change + create_table :ci_build_needs, id: :serial do |t| + t.integer :build_id, null: false + t.text :name, null: false + + t.index [:build_id, :name], unique: true + t.foreign_key :ci_builds, column: :build_id, on_delete: :cascade + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 6f5fc6c65eb..709f9ce2541 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2019_07_29_090456) do +ActiveRecord::Schema.define(version: 2019_07_31_084415) do # These are extensions that must be enabled in order to support this database enable_extension "pg_trgm" @@ -454,6 +454,12 @@ ActiveRecord::Schema.define(version: 2019_07_29_090456) do t.index ["namespace_id"], name: "index_chat_teams_on_namespace_id", unique: true end + create_table "ci_build_needs", id: :serial, force: :cascade do |t| + t.integer "build_id", null: false + t.text "name", null: false + t.index ["build_id", "name"], name: "index_ci_build_needs_on_build_id_and_name", unique: true + end + create_table "ci_build_trace_chunks", force: :cascade do |t| t.integer "build_id", null: false t.integer "chunk_index", null: false @@ -1130,6 +1136,7 @@ ActiveRecord::Schema.define(version: 2019_07_29_090456) do t.text "position" t.text "original_position" t.text "change_position" + t.binary "commit_id" t.index ["author_id"], name: "index_draft_notes_on_author_id" t.index ["discussion_id"], name: "index_draft_notes_on_discussion_id" t.index ["merge_request_id"], name: "index_draft_notes_on_merge_request_id" @@ -1435,6 +1442,7 @@ ActiveRecord::Schema.define(version: 2019_07_29_090456) do t.integer "minimum_reverification_interval", default: 7, null: false t.string "internal_url" t.string "name", null: false + t.integer "container_repositories_max_capacity", default: 10, null: false t.index ["access_key"], name: "index_geo_nodes_on_access_key" t.index ["name"], name: "index_geo_nodes_on_name", unique: true t.index ["primary"], name: "index_geo_nodes_on_primary" @@ -3635,6 +3643,7 @@ ActiveRecord::Schema.define(version: 2019_07_29_090456) do add_foreign_key "boards", "namespaces", column: "group_id", on_delete: :cascade add_foreign_key "boards", "projects", name: "fk_f15266b5f9", on_delete: :cascade add_foreign_key "chat_teams", "namespaces", on_delete: :cascade + add_foreign_key "ci_build_needs", "ci_builds", column: "build_id", on_delete: :cascade add_foreign_key "ci_build_trace_chunks", "ci_builds", column: "build_id", on_delete: :cascade add_foreign_key "ci_build_trace_section_names", "projects", on_delete: :cascade add_foreign_key "ci_build_trace_sections", "ci_build_trace_section_names", column: "section_name_id", name: "fk_264e112c66", on_delete: :cascade diff --git a/doc/administration/auth/ldap.md b/doc/administration/auth/ldap.md index beacaa99d60..186bf4c4825 100644 --- a/doc/administration/auth/ldap.md +++ b/doc/administration/auth/ldap.md @@ -33,15 +33,18 @@ information services over an Internet Protocol (IP) network. ## Security -GitLab assumes that LDAP users are not able to change their LDAP 'mail', 'email' -or 'userPrincipalName' attribute. An LDAP user who is allowed to change their -email on the LDAP server can potentially -[take over any account](#enabling-ldap-sign-in-for-existing-gitlab-users) -on your GitLab server. +GitLab assumes that LDAP users: + +- Are not able to change their LDAP `mail`, `email`, or `userPrincipalName` attribute. + An LDAP user who is allowed to change their email on the LDAP server can potentially + [take over any account](#enabling-ldap-sign-in-for-existing-gitlab-users) + on your GitLab server. +- Have unique email addresses, otherwise it is possible for LDAP users with the same + email address to share the same GitLab account. We recommend against using LDAP integration if your LDAP users are -allowed to change their 'mail', 'email' or 'userPrincipalName' attribute on -the LDAP server. +allowed to change their 'mail', 'email' or 'userPrincipalName' attribute on +the LDAP server or share email addresses. ### User deletion diff --git a/doc/api/README.md b/doc/api/README.md index 70540420544..6cd89e34921 100644 --- a/doc/api/README.md +++ b/doc/api/README.md @@ -695,6 +695,13 @@ The correct encoding for the query parameter would be: There are many unofficial GitLab API Clients for most of the popular programming languages. Visit the [GitLab website] for a complete list. +## Rate limits + +For administrator documentation on rate limit settings, check out +[Rate limits](../security/rate_limits.md). To find the settings that are +specifically used by GitLab.com, see +[GitLab.com-specific rate limits](../user/gitlab_com/index.md). + [GitLab website]: https://about.gitlab.com/applications/#api-clients "Clients using the GitLab API" [lib-api-url]: https://gitlab.com/gitlab-org/gitlab-ce/tree/master/lib/api/api.rb [ce-3749]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/3749 diff --git a/doc/development/testing_guide/end_to_end/index.md b/doc/development/testing_guide/end_to_end/index.md index 2dc06ba10a5..882e2230636 100644 --- a/doc/development/testing_guide/end_to_end/index.md +++ b/doc/development/testing_guide/end_to_end/index.md @@ -65,28 +65,24 @@ Below you can read more about how to use it and how does it work. Currently, we are using _multi-project pipeline_-like approach to run QA pipelines. - - -<details> -<summary>Show mermaid source</summary> -<pre> +```mermaid graph LR A1 -.->|1. Triggers an omnibus-gitlab pipeline and wait for it to be done| A2 - B2[<b>`Trigger-qa` stage</b><br />`Trigger:qa-test` job] -.->|2. Triggers a gitlab-qa pipeline and wait for it to be done| A3 + B2[`Trigger-qa` stage<br>`Trigger:qa-test` job] -.->|2. Triggers a gitlab-qa pipeline and wait for it to be done| A3 -subgraph gitlab-ce/ee pipeline - A1[<b>`test` stage</b><br />`package-and-qa` job] +subgraph "gitlab-ce/ee pipeline" + A1[`test` stage<br>`package-and-qa` job] end -subgraph omnibus-gitlab pipeline - A2[<b>`Trigger-docker` stage</b><br />`Trigger:gitlab-docker` job] -->|once done| B2 +subgraph "omnibus-gitlab pipeline" + A2[`Trigger-docker` stage<br>`Trigger:gitlab-docker` job] -->|once done| B2 end -subgraph gitlab-qa pipeline - A3>QA jobs run] -.->|3. Reports back the pipeline result to the `package-and-qa` job<br />and post the result on the original commit tested| A1 +subgraph "gitlab-qa pipeline" + A3>QA jobs run] -.->|3. Reports back the pipeline result to the `package-and-qa` job<br>and post the result on the original commit tested| A1 end -</pre> -</details> +``` + 1. Developer triggers a manual action, that can be found in CE / EE merge requests. This starts a chain of pipelines in multiple projects. diff --git a/doc/development/testing_guide/img/qa_on_merge_requests_cicd_architecture.png b/doc/development/testing_guide/img/qa_on_merge_requests_cicd_architecture.png Binary files differdeleted file mode 100644 index 5b93a05db96..00000000000 --- a/doc/development/testing_guide/img/qa_on_merge_requests_cicd_architecture.png +++ /dev/null diff --git a/doc/development/testing_guide/img/review_apps_cicd_architecture.png b/doc/development/testing_guide/img/review_apps_cicd_architecture.png Binary files differdeleted file mode 100644 index 1ee28d3db91..00000000000 --- a/doc/development/testing_guide/img/review_apps_cicd_architecture.png +++ /dev/null diff --git a/doc/development/testing_guide/review_apps.md b/doc/development/testing_guide/review_apps.md index 96761622cfe..7843fc4c874 100644 --- a/doc/development/testing_guide/review_apps.md +++ b/doc/development/testing_guide/review_apps.md @@ -8,38 +8,33 @@ Review Apps are automatically deployed by each pipeline, both in ### CI/CD architecture diagram - - -<details> -<summary>Show mermaid source</summary> -<pre> +```mermaid graph TD build-qa-image -.->|once the `prepare` stage is done| gitlab:assets:compile review-build-cng -->|triggers a CNG-mirror pipeline and wait for it to be done| CNG-mirror review-build-cng -.->|once the `test` stage is done| review-deploy review-deploy -.->|once the `review` stage is done| review-qa-smoke -subgraph 1. gitlab-ce/ee `prepare` stage +subgraph "1. gitlab-ce/ee `prepare` stage" build-qa-image end -subgraph 2. gitlab-ce/ee `test` stage +subgraph "2. gitlab-ce/ee `test` stage" gitlab:assets:compile -->|plays dependent job once done| review-build-cng end -subgraph 3. gitlab-ce/ee `review` stage - review-deploy["review-deploy<br /><br />Helm deploys the Review App using the Cloud<br/>Native images built by the CNG-mirror pipeline.<br /><br />Cloud Native images are deployed to the `review-apps-ce` or `review-apps-ee`<br />Kubernetes (GKE) cluster, in the GCP `gitlab-review-apps` project."] +subgraph "3. gitlab-ce/ee `review` stage" + review-deploy["review-deploy<br><br>Helm deploys the Review App using the Cloud<br/>Native images built by the CNG-mirror pipeline.<br><br>Cloud Native images are deployed to the `review-apps-ce` or `review-apps-ee`<br>Kubernetes (GKE) cluster, in the GCP `gitlab-review-apps` project."] end -subgraph 4. gitlab-ce/ee `qa` stage - review-qa-smoke[review-qa-smoke<br /><br />gitlab-qa runs the smoke suite against the Review App.] +subgraph "4. gitlab-ce/ee `qa` stage" + review-qa-smoke[review-qa-smoke<br><br>gitlab-qa runs the smoke suite against the Review App.] end -subgraph CNG-mirror pipeline +subgraph "CNG-mirror pipeline" CNG-mirror>Cloud Native images are built]; end -</pre> -</details> +``` ### Detailed explanation @@ -115,6 +110,28 @@ On every [pipeline][gitlab-pipeline] in the `qa` stage, the browser performance testing using a [Sitespeed.io Container](../../user/project/merge_requests/browser_performance_testing.md). +## Cluster configuration + +### Node pools + +Both `review-apps-ce` and `review-apps-ee` clusters are currently set up with +two node pools: + +- a node pool of non-preemptible `n1-standard-2` (2 vCPU, 7.5 GB memory) nodes + dedicated to the `tiller` deployment (see below) with a single node. +- a node pool of preemptible `n1-standard-2` (2 vCPU, 7.5 GB memory) nodes, + with a minimum of 1 node and a maximum of 250 nodes. + +### Helm/Tiller + +The `tiller` deployment (the Helm server) is deployed to a dedicated node pool +that has the `app=helm` label and a specific +[taint](https://kubernetes.io/docs/concepts/configuration/taint-and-toleration/) +to prevent other pods from being scheduled on this node pool. + +This is to ensure Tiller isn't affected by "noisy" neighbors that could put +their node under pressure. + ## How to: ### Log into my Review App @@ -241,15 +258,6 @@ thousands of unused Docker images.** CNG-mirror project to store these Docker images so that we can just wipe out the registry at some point, and use a new fresh, empty one. -**How big are the Kubernetes clusters (`review-apps-ce` and `review-apps-ee`)?** - - > The clusters are currently set up with a single pool of preemptible nodes, - with a minimum of 1 node and a maximum of 500 nodes. - -**What are the machine running on the cluster?** - - > We're currently using `n1-standard-1` (1 vCPU, 3.75 GB memory) machines. - **How do we secure this from abuse? Apps are open to the world so we need to find a way to limit it to only us.** diff --git a/doc/gitlab-basics/create-project.md b/doc/gitlab-basics/create-project.md index 1833b27a292..8bbaf5d1927 100644 --- a/doc/gitlab-basics/create-project.md +++ b/doc/gitlab-basics/create-project.md @@ -53,7 +53,7 @@ There are two types of project templates: - [Built-in templates](#built-in-templates), sourced from the following groups: - [`project-templates`](https://gitlab.com/gitlab-org/project-templates) - [`pages`](https://gitlab.com/pages) -- [Custom project templates](#custom-project-templates-premium-only), for custom templates +- [Custom project templates](#custom-project-templates-premium), for custom templates configured by GitLab administrators and users. #### Built-in templates @@ -78,7 +78,7 @@ You can improve the existing built-in templates or contribute new ones in the [`project-templates`](https://gitlab.com/gitlab-org/project-templates) and [`pages`](https://gitlab.com/pages) groups. -#### Custom project templates **(PREMIUM ONLY)** +#### Custom project templates **(PREMIUM)** > [Introduced](https://gitlab.com/gitlab-org/gitlab-ee/issues/6860) in [GitLab Premium](https://about.gitlab.com/pricing) 11.2. diff --git a/doc/security/README.md b/doc/security/README.md index c48d5bc2065..5d498ac7602 100644 --- a/doc/security/README.md +++ b/doc/security/README.md @@ -7,7 +7,7 @@ type: index - [Password length limits](password_length_limits.md) - [Restrict SSH key technologies and minimum length](ssh_keys_restrictions.md) -- [Rack attack](rack_attack.md) +- [Rate limits](rate_limits.md) - [Webhooks and insecure internal web services](webhooks.md) - [Information exclusivity](information_exclusivity.md) - [Reset your root password](reset_root_password.md) diff --git a/doc/security/rack_attack.md b/doc/security/rack_attack.md index 1e5678ec47c..c772f783f71 100644 --- a/doc/security/rack_attack.md +++ b/doc/security/rack_attack.md @@ -2,7 +2,9 @@ type: reference, howto --- -# Rack Attack +# Rack Attack initializer + +## Overview [Rack Attack](https://github.com/kickstarter/rack-attack), also known as Rack::Attack, is a Ruby gem that is meant to protect GitLab with the ability to customize throttling and @@ -14,19 +16,72 @@ If you find throttling is not enough to protect you against abusive clients, Rack Attack offers IP whitelisting, blacklisting, Fail2ban style filtering, and tracking. -**Note:** Starting with 11.2, Rack Attack is disabled by default. To continue -using Rack Attack, please enable it by [configuring `gitlab.rb` as described in Settings](#settings). +For more information on how to use these options see the [Rack Attack README](https://github.com/kickstarter/rack-attack/blob/master/README.md). + +NOTE: **Note:** See +[User and IP rate limits](../user/admin_area/settings/user_and_ip_rate_limits.md) +for simpler throttles that are configured in UI. + +NOTE: **Note:** Starting with 11.2, Rack Attack is disabled by default. If your +instance is not exposed to the public internet, it is recommended that you leave +Rack Attack disabled. + +## Behavior + +If set up as described in the [Settings](#settings) section below, two behaviors +will be enabled: + +- Protected paths will be throttled +- Failed authentications for Git and container registry requests will trigger a temporary IP ban + +### Protected paths throttle + +GitLab responds with HTTP status code 429 to POST requests at protected paths +over 10 requests per minute per IP address. + +By default, protected paths are: + +```ruby +default['gitlab']['gitlab-rails']['rack_attack_protected_paths'] = [ + '/users/password', + '/users/sign_in', + '/api/#{API::API.version}/session.json', + '/api/#{API::API.version}/session', + '/users', + '/users/confirmation', + '/unsubscribes/', + '/import/github/personal_access_token' +] +``` + +This header is included in responses to blocked requests: + +``` +Retry-After: 60 +``` + +For example, the following are limited to a maximum 10 requests per minute: + +- user sign-in +- user sign-up (if enabled) +- user password reset + +After trying for 10 times, the client will +have to wait a minute before to be able to try again. + +### Git and container registry failed authentication ban + +GitLab responds with HTTP status code 403 for 1 hour, if 30 failed +authentication requests were received in a 3-minute period from a single IP address. -By default, user sign-in, user sign-up (if enabled), and user password reset is -limited to 6 requests per minute. After trying for 6 times, the client will -have to wait for the next minute to be able to try again. +This applies only to Git requests and container registry (`/jwt/auth`) requests +(combined). -If you installed or upgraded GitLab by following the [official guides](../install/README.md), -Rack Attack should be disabled by default. If your instance is not exposed to any incoming -connections, it is recommended that you leave Rack Attack disabled. +This limit is reset by requests that authenticate successfully. For example, 29 +failed authentication requests followed by 1 successful request, followed by 29 +more failed authentication requests would not trigger a ban. -For more information on how to use these options check out -[rack-attack README](https://github.com/kickstarter/rack-attack/blob/master/README.md). +No response headers are provided. ## Settings diff --git a/doc/security/rate_limits.md b/doc/security/rate_limits.md new file mode 100644 index 00000000000..0e5bdcd9c79 --- /dev/null +++ b/doc/security/rate_limits.md @@ -0,0 +1,32 @@ +--- +type: reference, howto +--- + +# Rate limits + +NOTE: **Note:** +For GitLab.com, please see +[GitLab.com-specific rate limits](../user/gitlab_com/index.md#gitlabcom-specific-rate-limits). + +Rate limiting is a common technique used to improve the security and durability +of a web application. + +For example, a simple script can make thousands of web requests per second. +Whether malicious, apathetic, or just a bug, your application and infrastructure +may not be able to cope with the load. For more details, see +[Denial-of-service attack](https://en.wikipedia.org/wiki/Denial-of-service_attack). +Most cases can be mitigated by limiting the rate of requests from a single IP address. + +Most [brute-force attacks](https://en.wikipedia.org/wiki/Brute-force_attack) are +similarly mitigated by a rate limit. + +## Admin Area settings + +See +[User and IP rate limits](../user/admin_area/settings/user_and_ip_rate_limits.md). + +## Rack Attack initializer + +This method of rate limiting is cumbersome, but has some advantages. It allows +throttling of specific paths, and is also integrated into Git and container +registry requests. See [Rack Attack initializer](rack_attack.md). diff --git a/doc/user/admin_area/settings/img/user_and_ip_rate_limits.png b/doc/user/admin_area/settings/img/user_and_ip_rate_limits.png Binary files differnew file mode 100644 index 00000000000..2bd85a2fd96 --- /dev/null +++ b/doc/user/admin_area/settings/img/user_and_ip_rate_limits.png diff --git a/doc/user/admin_area/settings/index.md b/doc/user/admin_area/settings/index.md index 5427d04cd7d..2a12614e325 100644 --- a/doc/user/admin_area/settings/index.md +++ b/doc/user/admin_area/settings/index.md @@ -18,6 +18,7 @@ include: - [Third party offers](third_party_offers.md) - [Usage statistics](usage_statistics.md) - [Visibility and access controls](visibility_and_access_controls.md) +- [User and IP rate limits](user_and_ip_rate_limits.md) - [Custom templates repository](instance_template_repository.md) **(PREMIUM)** NOTE: **Note:** diff --git a/doc/user/admin_area/settings/user_and_ip_rate_limits.md b/doc/user/admin_area/settings/user_and_ip_rate_limits.md new file mode 100644 index 00000000000..e3a495750f2 --- /dev/null +++ b/doc/user/admin_area/settings/user_and_ip_rate_limits.md @@ -0,0 +1,32 @@ +--- +type: reference +--- + +# User and IP rate limits + +Rate limiting is a common technique used to improve the security and durability +of a web application. For more details, see +[Rate limits](../../../security/rate_limits.md). + +The following limits can be enforced in **Admin Area > Network > User and +IP rate limits**: + +- Unauthenticated requests +- Authenticated API requests +- Authenticated web requests + +These limits are disabled by default. + + + +<!-- ## Troubleshooting + +Include any troubleshooting steps that you can foresee. If you know beforehand what issues +one might have when setting this up, or when something is changed, or on upgrading, it's +important to describe those, too. Think of things that may go wrong and include them here. +This is important to minimize requests for support, and to avoid doc comments with +questions that you know someone might ask. + +Each scenario can be a third-level heading, e.g. `### Getting error message X`. +If you have none to add when creating a doc, leave this section in place +but commented out to help encourage others to add to it in the future. --> diff --git a/doc/user/application_security/index.md b/doc/user/application_security/index.md index 31f0b5a050c..4dcb416c110 100644 --- a/doc/user/application_security/index.md +++ b/doc/user/application_security/index.md @@ -148,6 +148,38 @@ Clicking on this button will create a merge request to apply the solution onto t  +## Security approvals in merge requests **(ULTIMATE)** + +> [Introduced](https://gitlab.com/gitlab-org/gitlab-ee/issues/9928) in [GitLab Ultimate](https://about.gitlab.com/pricing) 12.2. + +Merge Request Approvals can be configured to require approval from a member +of your security team when a vulnerability would be introduced by a merge request. + +This threshold is defined as `high`, `critical`, or `unknown` +severity. When any vulnerabilities are present within a merge request, an +approval will be required from the `Vulnerability-Check` approver group. + +### Enabling Security Approvals within a project + +To enable Security Approvals, a [project approval rule](../project/merge_requests/merge_request_approvals.md#multiple-approval-rules-premium) +must be created with the case-sensitive name `Vulnerability-Check`. This approval +group must be set with an "Approvals required" count greater than zero. + +Once this group has been added to your project, the approval rule will be enabled +for all Merge Requests. + +Any code changes made will cause the count of approvals required to reset. + +An approval will be required when a security report: + +- Contains a new vulnerability of `high`, `critical`, or `unknown` severity. +- Is not generated during pipeline execution. + +An approval will be optional when a security report: + +- Contains no new vulnerabilities. +- Contains only new vulnerabilities of `low` or `medium` severity. + <!-- ## Troubleshooting Include any troubleshooting steps that you can foresee. If you know beforehand what issues diff --git a/doc/user/gitlab_com/index.md b/doc/user/gitlab_com/index.md index 7858c419e04..e6c27c33654 100644 --- a/doc/user/gitlab_com/index.md +++ b/doc/user/gitlab_com/index.md @@ -298,6 +298,79 @@ Web front-ends: - `memory_limit_min` = 1024MiB - `memory_limit_max` = 1280MiB +## GitLab.com-specific rate limits + +NOTE: **Note:** +See [Rate limits](../../security/rate_limits.md) for administrator +documentation. + +IP blocks usually happen when GitLab.com receives unusual traffic from a single +IP address that the system views as potentially malicious based on rate limit +settings. After the unusual traffic ceases, the IP address will be automatically +released depending on the type of block, as described below. + +If you receive a `403 Forbidden` error for all requests to GitLab.com, please +check for any automated processes that may be triggering a block. For +assistance, contact [GitLab Support](https://support.gitlab.com) +with details, such as the affected IP address. + +### HAProxy API throttle + +GitLab.com responds with HTTP status code 429 to API requests over 10 requests +per second per IP address. + +The following example headers are included for all API requests: + +``` +RateLimit-Limit: 600 +RateLimit-Observed: 6 +RateLimit-Remaining: 594 +RateLimit-Reset: 1563325137 +RateLimit-ResetTime: Wed, 17 Jul 2019 00:58:57 GMT +``` + +Source: + +- Search for `rate_limit_http_rate_per_minute` and `rate_limit_sessions_per_second` in [GitLab.com's current HAProxy settings](https://gitlab.com/gitlab-cookbooks/gitlab-haproxy/blob/master/attributes/default.rb). + +### Rack Attack initializer + +#### Protected paths throttle + +GitLab.com responds with HTTP status code 429 to POST requests at protected +paths over 10 requests per **minute** per IP address. + +See the source below for which paths are protected. This includes user creation, +user confirmation, user sign in, and password reset. + +This header is included in responses to blocked requests: + +``` +Retry-After: 60 +``` + +Source: + +- Search for `rate_limit_requests_per_period`, `rate_limit_period`, and `rack_attack_protected_paths` in [GitLab.com's current Rails app settings](https://gitlab.com/gitlab-org/omnibus-gitlab/blob/master/files/gitlab-cookbooks/gitlab/attributes/default.rb). + +#### Git and container registry failed authentication ban + +GitLab.com responds with HTTP status code 403 for 1 hour, if 30 failed +authentication requests were received in a 3-minute period from a single IP address. + +This applies only to Git requests and container registry (`/jwt/auth`) requests +(combined). + +This limit is reset by requests that authenticate successfully. For example, 29 +failed authentication requests followed by 1 successful request, followed by 29 +more failed authentication requests would not trigger a ban. + +No response headers are provided. + +### Admin Area settings + +GitLab.com does not currently use these settings. + ## GitLab.com at scale In addition to the GitLab Enterprise Edition Omnibus install, GitLab.com uses diff --git a/doc/user/markdown.md b/doc/user/markdown.md index 9380dcf2a32..edb1e904f2b 100644 --- a/doc/user/markdown.md +++ b/doc/user/markdown.md @@ -185,6 +185,49 @@ graph TD; C-->D; ``` +#### Subgraphs + +NOTE: **Note:** GitLab 12.1 and up now [requires quotes around subgraph +titles that contain multiple words](https://github.com/knsv/mermaid/pull/845). + +Subgraphs can also be included: + +~~~ +```mermaid +graph TB + + SubGraph1 --> SubGraph1Flow + subgraph "SubGraph 1 Flow" + SubGraph1Flow(SubNode 1) + SubGraph1Flow -- Choice1 --> DoChoice1 + SubGraph1Flow -- Choice2 --> DoChoice2 + end + + subgraph "Main Graph" + Node1[Node 1] --> Node2[Node 2] + Node2 --> SubGraph1[Jump to SubGraph1] + SubGraph1 --> FinalThing[Final Thing] +end +``` +~~~ + +```mermaid +graph TB + + SubGraph1 --> SubGraph1Flow + subgraph "SubGraph 1 Flow" + SubGraph1Flow(SubNode 1) + SubGraph1Flow -- Choice1 --> DoChoice1 + SubGraph1Flow -- Choice2 --> DoChoice2 + end + + subgraph "Main Graph" + Node1[Node 1] --> Node2[Node 2] + Node2 --> SubGraph1[Jump to SubGraph1] + SubGraph1 --> FinalThing[Final Thing] +end +``` + ### Emoji > If this is not rendered correctly, [view it in GitLab itself](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/user/markdown.md#emoji). diff --git a/doc/user/project/integrations/jira_server_configuration.md b/doc/user/project/integrations/jira_server_configuration.md index 5116cbfe9ac..1efd0ff3944 100644 --- a/doc/user/project/integrations/jira_server_configuration.md +++ b/doc/user/project/integrations/jira_server_configuration.md @@ -3,8 +3,8 @@ We need to create a user in Jira which will have access to all projects that need to integrate with GitLab. -As an example, we'll create a user named `gitlab` and add it to the `Jira-developers` -group. +As an example, we'll create a user named `gitlab` and add it to a new group +named `gitlab-developers`. NOTE: **Note** It is important that the Jira user created for the integration is given 'write' diff --git a/doc/user/project/merge_requests/merge_request_approvals.md b/doc/user/project/merge_requests/merge_request_approvals.md index 220795d6f15..656459b3b03 100644 --- a/doc/user/project/merge_requests/merge_request_approvals.md +++ b/doc/user/project/merge_requests/merge_request_approvals.md @@ -331,6 +331,16 @@ the dropdown) `approver` and select the user.  +## Security approvals in merge requests **(ULTIMATE)** + +> Introduced in [GitLab Ultimate](https://about.gitlab.com/pricing) 12.2. + +Merge Request Approvals can be configured to require approval from a member +of your security team when a vulnerability would be introduced by a merge request. + +For more information, see +[Security approvals in merge requests](../../application_security/index.md#security-approvals-in-merge-requests-ultimate). + <!-- ## Troubleshooting Include any troubleshooting steps that you can foresee. If you know beforehand what issues diff --git a/doc/workflow/shortcuts.md b/doc/workflow/shortcuts.md index fd67ea8ce87..d61d7eafd18 100644 --- a/doc/workflow/shortcuts.md +++ b/doc/workflow/shortcuts.md @@ -35,11 +35,11 @@ You can see GitLab's keyboard shortcuts by using <kbd>shift</kbd> + <kbd>?</kbd> | Keyboard Shortcut | Description | | ----------------- | ----------- | -| <kbd>g</kbd> + <kbd>a</kbd> | Go to the activity feed | -| <kbd>g</kbd> + <kbd>p</kbd> | Go to projects | -| <kbd>g</kbd> + <kbd>i</kbd> | Go to issues | -| <kbd>g</kbd> + <kbd>m</kbd> | Go to merge requests | -| <kbd>g</kbd> + <kbd>t</kbd> | Go to todos | +| <kbd>Shift</kbd> + <kbd>a</kbd> | Go to the activity feed | +| <kbd>Shift</kbd> + <kbd>p</kbd> | Go to projects | +| <kbd>Shift</kbd> + <kbd>i</kbd> | Go to issues | +| <kbd>Shift</kbd> + <kbd>m</kbd> | Go to merge requests | +| <kbd>Shift</kbd> + <kbd>t</kbd> | Go to todos | ## Project diff --git a/lib/api/helpers/notes_helpers.rb b/lib/api/helpers/notes_helpers.rb index b03ac7deb71..7124ac0c5c3 100644 --- a/lib/api/helpers/notes_helpers.rb +++ b/lib/api/helpers/notes_helpers.rb @@ -76,7 +76,7 @@ module API def find_noteable(parent_type, parent_id, noteable_type, noteable_id) params = params_by_noteable_type_and_id(noteable_type, noteable_id) - noteable = NotesFinder.new(user_project, current_user, params).target + noteable = NotesFinder.new(current_user, params.merge(project: user_project)).target noteable = nil unless can?(current_user, noteable_read_ability_name(noteable), noteable) noteable || not_found!(noteable_type) end diff --git a/lib/gitlab/ci/config/entry/job.rb b/lib/gitlab/ci/config/entry/job.rb index 5ab795359b8..2fd76bc3690 100644 --- a/lib/gitlab/ci/config/entry/job.rb +++ b/lib/gitlab/ci/config/entry/job.rb @@ -13,7 +13,7 @@ module Gitlab ALLOWED_KEYS = %i[tags script only except type image services allow_failure type stage when start_in artifacts cache - dependencies before_script after_script variables + dependencies needs before_script after_script variables environment coverage retry parallel extends].freeze validations do @@ -34,11 +34,22 @@ module Gitlab message: 'should be on_success, on_failure, ' \ 'always, manual or delayed' } validates :dependencies, array_of_strings: true + validates :needs, array_of_strings: true validates :extends, array_of_strings_or_string: true end validates :start_in, duration: { limit: '1 day' }, if: :delayed? validates :start_in, absence: true, unless: :delayed? + + validate do + next unless dependencies.present? + next unless needs.present? + + missing_needs = dependencies - needs + if missing_needs.any? + errors.add(:dependencies, "the #{missing_needs.join(", ")} should be part of needs") + end + end end entry :before_script, Entry::Script, @@ -95,10 +106,10 @@ module Gitlab helpers :before_script, :script, :stage, :type, :after_script, :cache, :image, :services, :only, :except, :variables, :artifacts, :environment, :coverage, :retry, - :parallel + :parallel, :needs attributes :script, :tags, :allow_failure, :when, :dependencies, - :retry, :parallel, :extends, :start_in + :needs, :retry, :parallel, :extends, :start_in def self.matching?(name, config) !name.to_s.start_with?('.') && @@ -178,7 +189,8 @@ module Gitlab parallel: parallel_defined? ? parallel_value.to_i : nil, artifacts: artifacts_value, after_script: after_script_value, - ignore: ignored? } + ignore: ignored?, + needs: needs_defined? ? needs_value : nil } end end end diff --git a/lib/gitlab/ci/config/normalizer.rb b/lib/gitlab/ci/config/normalizer.rb index 99356226ef9..09f9bf5f69f 100644 --- a/lib/gitlab/ci/config/normalizer.rb +++ b/lib/gitlab/ci/config/normalizer.rb @@ -4,61 +4,63 @@ module Gitlab module Ci class Config class Normalizer + include Gitlab::Utils::StrongMemoize + def initialize(jobs_config) @jobs_config = jobs_config end def normalize_jobs - extract_parallelized_jobs! - return @jobs_config if @parallelized_jobs.empty? + return @jobs_config if parallelized_jobs.empty? + + expand_parallelize_jobs do |job_name, config| + if config[:dependencies] + config[:dependencies] = expand_names(config[:dependencies]) + end - parallelized_config = parallelize_jobs - parallelize_dependencies(parallelized_config) + if config[:needs] + config[:needs] = expand_names(config[:needs]) + end + + config + end end private - def extract_parallelized_jobs! - @parallelized_jobs = {} + def expand_names(job_names) + return unless job_names - @jobs_config.each do |job_name, config| - if config[:parallel] - @parallelized_jobs[job_name] = self.class.parallelize_job_names(job_name, config[:parallel]) - end + job_names.flat_map do |job_name| + parallelized_jobs[job_name.to_sym] || job_name end - - @parallelized_jobs end - def parallelize_jobs - @jobs_config.each_with_object({}) do |(job_name, config), hash| - if @parallelized_jobs.key?(job_name) - @parallelized_jobs[job_name].each { |name, index| hash[name.to_sym] = config.merge(name: name, instance: index) } - else - hash[job_name] = config - end + def parallelized_jobs + strong_memoize(:parallelized_jobs) do + @jobs_config.each_with_object({}) do |(job_name, config), hash| + next unless config[:parallel] - hash + hash[job_name] = self.class.parallelize_job_names(job_name, config[:parallel]) + end end end - def parallelize_dependencies(parallelized_config) - parallelized_job_names = @parallelized_jobs.keys.map(&:to_s) - parallelized_config.each_with_object({}) do |(job_name, config), hash| - if config[:dependencies] && (intersection = config[:dependencies] & parallelized_job_names).any? - parallelized_deps = intersection.flat_map { |dep| @parallelized_jobs[dep.to_sym].map(&:first) } - deps = config[:dependencies] - intersection + parallelized_deps - hash[job_name] = config.merge(dependencies: deps) + def expand_parallelize_jobs + @jobs_config.each_with_object({}) do |(job_name, config), hash| + if parallelized_jobs.key?(job_name) + parallelized_jobs[job_name].each_with_index do |name, index| + hash[name.to_sym] = + yield(name, config.merge(name: name, instance: index + 1)) + end else - hash[job_name] = config + hash[job_name] = yield(job_name, config) end - - hash end end def self.parallelize_job_names(name, total) - Array.new(total) { |index| ["#{name} #{index + 1}/#{total}", index + 1] } + Array.new(total) { |index| "#{name} #{index + 1}/#{total}" } end end end diff --git a/lib/gitlab/ci/yaml_processor.rb b/lib/gitlab/ci/yaml_processor.rb index a0bbf3c23a2..998130e5bd0 100644 --- a/lib/gitlab/ci/yaml_processor.rb +++ b/lib/gitlab/ci/yaml_processor.rb @@ -40,6 +40,7 @@ module Gitlab environment: job[:environment_name], coverage_regex: job[:coverage], yaml_variables: yaml_variables(name), + needs_attributes: job[:needs]&.map { |need| { name: need } }, options: { image: job[:image], services: job[:services], @@ -108,6 +109,7 @@ module Gitlab validate_job_stage!(name, job) validate_job_dependencies!(name, job) + validate_job_needs!(name, job) validate_job_environment!(name, job) end end @@ -152,6 +154,22 @@ module Gitlab end end + def validate_job_needs!(name, job) + return unless job[:needs] + + stage_index = @stages.index(job[:stage]) + + job[:needs].each do |need| + raise ValidationError, "#{name} job: undefined need: #{need}" unless @jobs[need.to_sym] + + needs_stage_index = @stages.index(@jobs[need.to_sym][:stage]) + + unless needs_stage_index.present? && needs_stage_index < stage_index + raise ValidationError, "#{name} job: need #{need} is not defined in prior stages" + end + end + end + def validate_job_environment!(name, job) return unless job[:environment] return unless job[:environment].is_a?(Hash) diff --git a/lib/gitlab/ee_compat_check.rb b/lib/gitlab/ee_compat_check.rb index 01fd261404b..86e532766b1 100644 --- a/lib/gitlab/ee_compat_check.rb +++ b/lib/gitlab/ee_compat_check.rb @@ -7,7 +7,7 @@ module Gitlab CANONICAL_CE_PROJECT_URL = 'https://gitlab.com/gitlab-org/gitlab-ce'.freeze CANONICAL_EE_REPO_URL = 'https://gitlab.com/gitlab-org/gitlab-ee.git'.freeze CHECK_DIR = Rails.root.join('ee_compat_check') - IGNORED_FILES_REGEX = /VERSION|CHANGELOG\.md/i.freeze + IGNORED_FILES_REGEX = /VERSION|CHANGELOG\.md|doc\/.+/i.freeze PLEASE_READ_THIS_BANNER = %Q{ ============================================================ ===================== PLEASE READ THIS ===================== diff --git a/lib/gitlab/exclusive_lease_helpers.rb b/lib/gitlab/exclusive_lease_helpers.rb index 7961d4bbd6e..61eb030563d 100644 --- a/lib/gitlab/exclusive_lease_helpers.rb +++ b/lib/gitlab/exclusive_lease_helpers.rb @@ -15,17 +15,18 @@ module Gitlab raise ArgumentError, 'Key needs to be specified' unless key lease = Gitlab::ExclusiveLease.new(key, timeout: ttl) + retried = false until uuid = lease.try_obtain # Keep trying until we obtain the lease. To prevent hammering Redis too # much we'll wait for a bit. sleep(sleep_sec) - break if (retries -= 1) < 0 + (retries -= 1) < 0 ? break : retried ||= true end raise FailedToObtainLockError, 'Failed to obtain a lock' unless uuid - yield + yield(retried) ensure Gitlab::ExclusiveLease.cancel(key, uuid) end diff --git a/lib/gitlab/project_search_results.rb b/lib/gitlab/project_search_results.rb index 0f3b97e2317..827f4f77f36 100644 --- a/lib/gitlab/project_search_results.rb +++ b/lib/gitlab/project_search_results.rb @@ -108,7 +108,7 @@ module Gitlab # rubocop: disable CodeReuse/ActiveRecord def notes_finder(type) - NotesFinder.new(project, @current_user, search: query, target_type: type).execute.user.order('updated_at DESC') + NotesFinder.new(@current_user, search: query, target_type: type, project: project).execute.user.order('updated_at DESC') end # rubocop: enable CodeReuse/ActiveRecord diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 4921b3c835b..5e9e371a5fc 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -4052,12 +4052,6 @@ msgstr "" msgid "Edit public deploy key" msgstr "" -msgid "Editor|%{mdLinkStart}Markdown is supported%{mdLinkEnd}" -msgstr "" - -msgid "Editor|%{mdLinkStart}Markdown%{mdLinkEnd} and %{actionsLinkStart}quick actions%{actionsLinkEnd} are supported" -msgstr "" - msgid "Email" msgstr "" @@ -6508,12 +6502,18 @@ msgstr "" msgid "Mark to do as done" msgstr "" +msgid "Markdown" +msgstr "" + msgid "Markdown Help" msgstr "" msgid "Markdown enabled" msgstr "" +msgid "Markdown is supported" +msgstr "" + msgid "Marked this %{noun} as Work In Progress." msgstr "" @@ -13566,6 +13566,9 @@ msgstr "" msgid "project avatar" msgstr "" +msgid "quick actions" +msgstr "" + msgid "register" msgstr "" diff --git a/package.json b/package.json index c97046b0e84..f0a7f3e47af 100644 --- a/package.json +++ b/package.json @@ -76,7 +76,6 @@ "diff": "^3.4.0", "document-register-element": "1.13.1", "dropzone": "^4.2.0", - "echarts": "^4.2.0-rc.2", "emoji-regex": "^7.0.3", "emoji-unicode-version": "^0.2.1", "exports-loader": "^0.7.0", @@ -100,7 +99,7 @@ "mermaid": "^8.2.3", "monaco-editor": "^0.15.6", "monaco-editor-webpack-plugin": "^1.7.0", - "mousetrap": "1.4.6", + "mousetrap": "^1.4.6", "pdfjs-dist": "^2.0.943", "pikaday": "^1.6.1", "popper.js": "^1.14.7", diff --git a/qa/qa/specs/features/browser_ui/3_create/repository/push_over_http_file_size_spec.rb b/qa/qa/specs/features/browser_ui/3_create/repository/push_over_http_file_size_spec.rb index 11fd570d131..2027a3c16aa 100644 --- a/qa/qa/specs/features/browser_ui/3_create/repository/push_over_http_file_size_spec.rb +++ b/qa/qa/specs/features/browser_ui/3_create/repository/push_over_http_file_size_spec.rb @@ -1,8 +1,7 @@ # frozen_string_literal: true module QA - # Failure issue: https://gitlab.com/gitlab-org/quality/nightly/issues/113 - context 'Create', :requires_admin, :quarantine do + context 'Create', :requires_admin do describe 'push after setting the file size limit via admin/application_settings' do before(:context) do @project = Resource::Project.fabricate_via_api! do |p| @@ -21,14 +20,21 @@ module QA it 'push successful when the file size is under the limit' do set_file_size_limit(5) - push = push_new_file('oversize_file_1.bin', wait_for_push: true) - expect(push.output).not_to have_content 'remote: fatal: pack exceeds maximum allowed size' + + retry_on_fail do + push = push_new_file('oversize_file_1.bin', wait_for_push: true) + + expect(push.output).not_to have_content 'remote: fatal: pack exceeds maximum allowed size' + end end it 'push fails when the file size is above the limit' do set_file_size_limit(1) - expect { push_new_file('oversize_file_2.bin', wait_for_push: false) } - .to raise_error(QA::Git::Repository::RepositoryCommandError, /remote: fatal: pack exceeds maximum allowed size/) + + retry_on_fail do + expect { push_new_file('oversize_file_2.bin', wait_for_push: false) } + .to raise_error(QA::Git::Repository::RepositoryCommandError, /remote: fatal: pack exceeds maximum allowed size/) + end end def set_file_size_limit(limit) @@ -54,6 +60,22 @@ module QA output end + + # Application settings are cached for up to a minute. So when we change + # the `receive_max_input_size` setting, the setting might not be applied + # for minute. This caused the tests to intermittently fail. + # See https://gitlab.com/gitlab-org/quality/nightly/issues/113 + # + # Instead of waiting a minute after changing the setting, we retry the + # attempt to push if it fails. Most of the time the setting is updated in + # under a minute, i.e., in fewer than 6 attempts with a 10 second sleep + # between attempts. + # See https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/30233#note_188616863 + def retry_on_fail + Support::Retrier.retry_on_exception(max_attempts: 6, reload_page: nil, sleep_interval: 10) do + yield + end + end end end end diff --git a/rubocop/cop/rspec/top_level_describe_path.rb b/rubocop/cop/rspec/top_level_describe_path.rb new file mode 100644 index 00000000000..61796e23af0 --- /dev/null +++ b/rubocop/cop/rspec/top_level_describe_path.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +require 'rubocop/rspec/top_level_describe' + +module RuboCop + module Cop + module RSpec + class TopLevelDescribePath < RuboCop::Cop::Cop + include RuboCop::RSpec::TopLevelDescribe + + MESSAGE = 'A file with a top-level `describe` must end in _spec.rb.' + SHARED_EXAMPLES = %i[shared_examples shared_examples_for].freeze + + def on_top_level_describe(node, args) + return if acceptable_file_path?(processed_source.buffer.name) + return if shared_example?(node) + + add_offense(node, message: MESSAGE) + end + + private + + def acceptable_file_path?(path) + File.fnmatch?('*_spec.rb', path) || File.fnmatch?('*/frontend/fixtures/*', path) + end + + def shared_example?(node) + node.ancestors.any? do |node| + node.respond_to?(:method_name) && SHARED_EXAMPLES.include?(node.method_name) + end + end + end + end + end +end diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb index ba61a634d97..58a7ead6f13 100644 --- a/rubocop/rubocop.rb +++ b/rubocop/rubocop.rb @@ -32,6 +32,7 @@ require_relative 'cop/migration/update_large_table' require_relative 'cop/project_path_helper' require_relative 'cop/rspec/env_assignment' require_relative 'cop/rspec/factories_in_migration_specs' +require_relative 'cop/rspec/top_level_describe_path' require_relative 'cop/qa/element_with_pattern' require_relative 'cop/sidekiq_options_queue' require_relative 'cop/destroy_all' diff --git a/spec/controllers/autocomplete_controller_spec.rb b/spec/controllers/autocomplete_controller_spec.rb index eaa5d6cd073..6cdd61e7abd 100644 --- a/spec/controllers/autocomplete_controller_spec.rb +++ b/spec/controllers/autocomplete_controller_spec.rb @@ -222,6 +222,20 @@ describe AutocompleteController do expect(response_user_ids).to contain_exactly(non_member.id) end end + + context 'merge_request_iid parameter included' do + before do + sign_in(user) + end + + it 'includes can_merge option to users' do + merge_request = create(:merge_request, source_project: project) + + get(:users, params: { merge_request_iid: merge_request.iid, project_id: project.id }) + + expect(json_response.first).to have_key('can_merge') + end + end end context 'GET projects' do diff --git a/spec/controllers/concerns/issuable_actions_spec.rb b/spec/controllers/concerns/issuable_actions_spec.rb new file mode 100644 index 00000000000..7b0b4497f3f --- /dev/null +++ b/spec/controllers/concerns/issuable_actions_spec.rb @@ -0,0 +1,69 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe IssuableActions do + let(:project) { double('project') } + let(:user) { double('user') } + let(:issuable) { double('issuable') } + let(:finder_params_for_issuable) { {} } + let(:notes_result) { double('notes_result') } + let(:discussion_serializer) { double('discussion_serializer') } + + let(:controller) do + klass = Class.new do + attr_reader :current_user, :project, :issuable + + def self.before_action(action, params = nil) + end + + include IssuableActions + + def initialize(issuable, project, user, finder_params) + @issuable = issuable + @project = project + @current_user = user + @finder_params = finder_params + end + + def finder_params_for_issuable + @finder_params + end + + def params + { + notes_filter: 1 + } + end + + def prepare_notes_for_rendering(notes) + [] + end + + def render(options) + end + end + + klass.new(issuable, project, user, finder_params_for_issuable) + end + + describe '#discussions' do + before do + allow(user).to receive(:set_notes_filter) + allow(user).to receive(:user_preference) + allow(discussion_serializer).to receive(:represent) + end + + it 'instantiates and calls NotesFinder as expected' do + expect(Discussion).to receive(:build_collection).and_return([]) + expect(DiscussionSerializer).to receive(:new).and_return(discussion_serializer) + expect(NotesFinder).to receive(:new).with(user, finder_params_for_issuable).and_call_original + + expect_any_instance_of(NotesFinder).to receive(:execute).and_return(notes_result) + + expect(notes_result).to receive_messages(inc_relations_for_view: notes_result, includes: notes_result, fresh: notes_result) + + controller.discussions + end + end +end diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index 32d14dce936..0f885d776e1 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -1260,6 +1260,28 @@ describe Projects::IssuesController do sign_in(user) end + context do + it_behaves_like 'discussions provider' do + let!(:author) { create(:user) } + let!(:project) { create(:project) } + + let!(:issue) { create(:issue, project: project, author: user) } + + let!(:note_on_issue1) { create(:discussion_note_on_issue, noteable: issue, project: issue.project, author: create(:user)) } + let!(:note_on_issue2) { create(:discussion_note_on_issue, noteable: issue, project: issue.project, author: create(:user)) } + + let(:requested_iid) { issue.iid } + let(:expected_discussion_count) { 3 } + let(:expected_discussion_ids) do + [ + issue.notes.first.discussion_id, + note_on_issue1.discussion_id, + note_on_issue2.discussion_id + ] + end + end + end + it 'returns discussion json' do get :discussions, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index 57a432de3f5..b1dc6a65dd4 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -1210,6 +1210,22 @@ describe Projects::MergeRequestsController do end end end + + context do + it_behaves_like 'discussions provider' do + let!(:author) { create(:user) } + let!(:project) { create(:project) } + + let!(:merge_request) { create(:merge_request, source_project: project) } + + let!(:mr_note1) { create(:discussion_note_on_merge_request, noteable: merge_request, project: project) } + let!(:mr_note2) { create(:discussion_note_on_merge_request, noteable: merge_request, project: project) } + + let(:requested_iid) { merge_request.iid } + let(:expected_discussion_count) { 2 } + let(:expected_discussion_ids) { [mr_note1.discussion_id, mr_note2.discussion_id] } + end + end end describe 'GET edit' do diff --git a/spec/controllers/projects/notes_controller_spec.rb b/spec/controllers/projects/notes_controller_spec.rb index 98aea9056dc..9ab565dc2e8 100644 --- a/spec/controllers/projects/notes_controller_spec.rb +++ b/spec/controllers/projects/notes_controller_spec.rb @@ -43,7 +43,7 @@ describe Projects::NotesController do request.headers['X-Last-Fetched-At'] = last_fetched_at expect(NotesFinder).to receive(:new) - .with(anything, anything, hash_including(last_fetched_at: last_fetched_at)) + .with(anything, hash_including(last_fetched_at: last_fetched_at)) .and_call_original get :index, params: request_params diff --git a/spec/db/schema_spec.rb b/spec/db/schema_spec.rb index 13fad1b6dc9..232890b1bba 100644 --- a/spec/db/schema_spec.rb +++ b/spec/db/schema_spec.rb @@ -27,7 +27,7 @@ describe 'Database schema' do cluster_providers_gcp: %w[gcp_project_id operation_id], deploy_keys_projects: %w[deploy_key_id], deployments: %w[deployable_id environment_id user_id], - draft_notes: %w[discussion_id], + draft_notes: %w[discussion_id commit_id], emails: %w[user_id], events: %w[target_id], epics: %w[updated_by_id last_edited_by_id start_date_sourcing_milestone_id due_date_sourcing_milestone_id], diff --git a/spec/factories/ci/build_need.rb b/spec/factories/ci/build_need.rb new file mode 100644 index 00000000000..568aff45a91 --- /dev/null +++ b/spec/factories/ci/build_need.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :ci_build_need, class: Ci::BuildNeed do + build factory: :ci_build + sequence(:name) { |n| "build_#{n}" } + end +end diff --git a/spec/factories/container_repositories.rb b/spec/factories/container_repositories.rb index a9771200d6e..0b756220d68 100644 --- a/spec/factories/container_repositories.rb +++ b/spec/factories/container_repositories.rb @@ -2,7 +2,7 @@ FactoryBot.define do factory :container_repository do - name 'test_image' + sequence(:name) { |n| "test_image_#{n}" } project transient do diff --git a/spec/features/dashboard/shortcuts_spec.rb b/spec/features/dashboard/shortcuts_spec.rb index 659bec177ab..3a47475da2b 100644 --- a/spec/features/dashboard/shortcuts_spec.rb +++ b/spec/features/dashboard/shortcuts_spec.rb @@ -51,7 +51,7 @@ describe 'Dashboard shortcuts', :js do find('body').send_keys([:shift, 'P']) find('.nothing-here-block') - expect(page).to have_content('This user doesn\'t have any personal projects') + expect(page).to have_content('Explore public groups to find projects to contribute to.') end end diff --git a/spec/features/groups/labels/user_sees_links_to_issuables.rb b/spec/features/groups/labels/user_sees_links_to_issuables_spec.rb index e636f625b31..6199b566ebc 100644 --- a/spec/features/groups/labels/user_sees_links_to_issuables.rb +++ b/spec/features/groups/labels/user_sees_links_to_issuables_spec.rb @@ -11,7 +11,9 @@ describe 'Groups > Labels > User sees links to issuables' do end it 'shows links to MRs and issues' do - expect(page).to have_link('view merge requests') - expect(page).to have_link('view open issues') + page.within('.labels-container') do + expect(page).to have_link('Merge requests') + expect(page).to have_link('Issues') + end end end diff --git a/spec/features/instance_statistics/instance_statistics.rb b/spec/features/instance_statistics/instance_statistics_spec.rb index 40d0f1db207..40d0f1db207 100644 --- a/spec/features/instance_statistics/instance_statistics.rb +++ b/spec/features/instance_statistics/instance_statistics_spec.rb diff --git a/spec/features/projects/files/user_browses_a_tree_with_a_folder_containing_only_a_folder.rb b/spec/features/projects/files/user_browses_a_tree_with_a_folder_containing_only_a_folder_spec.rb index 934de2fde8f..c19e46da913 100644 --- a/spec/features/projects/files/user_browses_a_tree_with_a_folder_containing_only_a_folder.rb +++ b/spec/features/projects/files/user_browses_a_tree_with_a_folder_containing_only_a_folder_spec.rb @@ -3,7 +3,8 @@ require 'spec_helper' # This is a regression test for https://gitlab.com/gitlab-org/gitlab-ce/issues/37569 -describe 'Projects > Files > User browses a tree with a folder containing only a folder' do +# Quarantine: https://gitlab.com/gitlab-org/gitlab-ce/issues/65329 +describe 'Projects > Files > User browses a tree with a folder containing only a folder', :quarantine do let(:project) { create(:project, :empty_repo) } let(:user) { project.owner } diff --git a/spec/features/projects/labels/user_sees_links_to_issuables.rb b/spec/features/projects/labels/user_sees_links_to_issuables_spec.rb index fd2151a1f8e..7a9b9e6eac2 100644 --- a/spec/features/projects/labels/user_sees_links_to_issuables.rb +++ b/spec/features/projects/labels/user_sees_links_to_issuables_spec.rb @@ -19,8 +19,10 @@ describe 'Projects > Labels > User sees links to issuables' do let(:project) { create(:project, :public) } it 'shows links to MRs and issues' do - expect(page).to have_link('view merge requests') - expect(page).to have_link('view open issues') + page.within('.labels-container') do + expect(page).to have_link('Merge requests') + expect(page).to have_link('Issues') + end end end @@ -28,8 +30,10 @@ describe 'Projects > Labels > User sees links to issuables' do let(:project) { create(:project, :public, issues_access_level: ProjectFeature::DISABLED) } it 'shows links to MRs but not to issues' do - expect(page).to have_link('view merge requests') - expect(page).not_to have_link('view open issues') + page.within('.labels-container') do + expect(page).to have_link('Merge requests') + expect(page).not_to have_link('Issues') + end end end @@ -37,8 +41,10 @@ describe 'Projects > Labels > User sees links to issuables' do let(:project) { create(:project, :public, merge_requests_access_level: ProjectFeature::DISABLED) } it 'shows links to issues but not to MRs' do - expect(page).not_to have_link('view merge requests') - expect(page).to have_link('view open issues') + page.within('.labels-container') do + expect(page).not_to have_link('Merge requests') + expect(page).to have_link('Issues') + end end end end @@ -51,8 +57,10 @@ describe 'Projects > Labels > User sees links to issuables' do let(:project) { create(:project, :public, namespace: group) } it 'shows links to MRs and issues' do - expect(page).to have_link('view merge requests') - expect(page).to have_link('view open issues') + page.within('.labels-container') do + expect(page).to have_link('Merge requests') + expect(page).to have_link('Issues') + end end end @@ -60,8 +68,10 @@ describe 'Projects > Labels > User sees links to issuables' do let(:project) { create(:project, :public, namespace: group, issues_access_level: ProjectFeature::DISABLED) } it 'shows links to MRs and issues' do - expect(page).to have_link('view merge requests') - expect(page).to have_link('view open issues') + page.within('.labels-container') do + expect(page).to have_link('Merge requests') + expect(page).to have_link('Issues') + end end end @@ -69,8 +79,10 @@ describe 'Projects > Labels > User sees links to issuables' do let(:project) { create(:project, :public, namespace: group, merge_requests_access_level: ProjectFeature::DISABLED) } it 'shows links to MRs and issues' do - expect(page).to have_link('view merge requests') - expect(page).to have_link('view open issues') + page.within('.labels-container') do + expect(page).to have_link('Merge requests') + expect(page).to have_link('Issues') + end end end end diff --git a/spec/features/projects/wiki/user_creates_wiki_page_spec.rb b/spec/features/projects/wiki/user_creates_wiki_page_spec.rb index 1080976f7ce..cc6dbaa6eb8 100644 --- a/spec/features/projects/wiki/user_creates_wiki_page_spec.rb +++ b/spec/features/projects/wiki/user_creates_wiki_page_spec.rb @@ -134,15 +134,9 @@ describe "User creates wiki page" do fill_in(:wiki_content, with: ascii_content) - # This is the dumbest bug in the world: - # When the #wiki_content textarea is filled in, JS captures the `Enter` keydown event in order to do - # auto-indentation and manually inserts a newline. However, for whatever reason, when you try to click on the - # submit button in Capybara, it will not trigger the `click` event if a \n or \r character has been manually - # added to the textarea. It will, however, trigger ALL OTHER EVENTS, including `mouseover`/down/up, focus, and - # blur. Just not `click`. But only when you manually insert \n or \r - if you manually insert any other sequence - # then `click` is fired normally. And it's only Capybara. Browsers and JSDOM don't have this issue. - # So that's why the next line performs the click via JS. - page.execute_script("document.querySelector('.rspec-create-page-button').click()") + page.within(".wiki-form") do + click_button("Create page") + end page.within ".md" do expect(page).to have_selector(".katex", count: 3).and have_content("2+2 is 4") diff --git a/spec/features/snippets/user_sees_breadcrumb_links.rb b/spec/features/snippets/user_sees_breadcrumb_links.rb deleted file mode 100644 index 5b10984ce1d..00000000000 --- a/spec/features/snippets/user_sees_breadcrumb_links.rb +++ /dev/null @@ -1,19 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -describe 'New user snippet breadcrumbs' do - let(:user) { create(:user) } - - before do - sign_in(user) - visit new_snippet_path - end - - it 'display a link to user snippets and new user snippet pages' do - page.within '.breadcrumbs' do - expect(find_link('Snippets')[:href]).to end_with(dashboard_snippets_path) - expect(find_link('New')[:href]).to end_with(new_snippet_path) - end - end -end diff --git a/spec/features/user_opens_link_to_comment.rb b/spec/features/user_opens_link_to_comment_spec.rb index f1e07e55799..f1e07e55799 100644 --- a/spec/features/user_opens_link_to_comment.rb +++ b/spec/features/user_opens_link_to_comment_spec.rb diff --git a/spec/features/users/add_email_to_existing_account.rb b/spec/features/users/add_email_to_existing_account_spec.rb index 42e352399a8..42e352399a8 100644 --- a/spec/features/users/add_email_to_existing_account.rb +++ b/spec/features/users/add_email_to_existing_account_spec.rb diff --git a/spec/finders/notes_finder_spec.rb b/spec/finders/notes_finder_spec.rb index 87bde4ca2f6..88906adfeeb 100644 --- a/spec/finders/notes_finder_spec.rb +++ b/spec/finders/notes_finder_spec.rb @@ -14,7 +14,7 @@ describe NotesFinder do let!(:system_note) { create(:note_on_issue, project: project, system: true) } it 'returns only user notes when using only_comments filter' do - finder = described_class.new(project, user, notes_filter: UserPreference::NOTES_FILTERS[:only_comments]) + finder = described_class.new(user, project: project, notes_filter: UserPreference::NOTES_FILTERS[:only_comments]) notes = finder.execute @@ -22,7 +22,7 @@ describe NotesFinder do end it 'returns only system notes when using only_activity filters' do - finder = described_class.new(project, user, notes_filter: UserPreference::NOTES_FILTERS[:only_activity]) + finder = described_class.new(user, project: project, notes_filter: UserPreference::NOTES_FILTERS[:only_activity]) notes = finder.execute @@ -30,7 +30,7 @@ describe NotesFinder do end it 'gets all notes' do - finder = described_class.new(project, user, notes_filter: UserPreference::NOTES_FILTERS[:all_activity]) + finder = described_class.new(user, project: project, notes_filter: UserPreference::NOTES_FILTERS[:all_activity]) notes = finder.execute @@ -41,7 +41,7 @@ describe NotesFinder do it 'finds notes on merge requests' do create(:note_on_merge_request, project: project) - notes = described_class.new(project, user).execute + notes = described_class.new(user, project: project).execute expect(notes.count).to eq(1) end @@ -49,7 +49,7 @@ describe NotesFinder do it 'finds notes on snippets' do create(:note_on_project_snippet, project: project) - notes = described_class.new(project, user).execute + notes = described_class.new(user, project: project).execute expect(notes.count).to eq(1) end @@ -59,13 +59,13 @@ describe NotesFinder do note = create(:note_on_commit, project: project) params = { target_type: 'commit', target_id: note.noteable.id } - notes = described_class.new(project, create(:user), params).execute + notes = described_class.new(create(:user), params).execute expect(notes.count).to eq(0) end it 'succeeds when no notes found' do - notes = described_class.new(project, create(:user)).execute + notes = described_class.new(create(:user), project: project).execute expect(notes.count).to eq(0) end @@ -82,7 +82,7 @@ describe NotesFinder do it 'publicly excludes notes on merge requests' do create(:note_on_merge_request, project: project) - notes = described_class.new(project, create(:user)).execute + notes = described_class.new(create(:user), project: project).execute expect(notes.count).to eq(0) end @@ -90,7 +90,7 @@ describe NotesFinder do it 'publicly excludes notes on issues' do create(:note_on_issue, project: project) - notes = described_class.new(project, create(:user)).execute + notes = described_class.new(create(:user), project: project).execute expect(notes.count).to eq(0) end @@ -98,7 +98,7 @@ describe NotesFinder do it 'publicly excludes notes on snippets' do create(:note_on_project_snippet, project: project) - notes = described_class.new(project, create(:user)).execute + notes = described_class.new(create(:user), project: project).execute expect(notes.count).to eq(0) end @@ -110,7 +110,7 @@ describe NotesFinder do let!(:note2) { create :note_on_commit, project: project } it 'finds only notes for the selected type' do - notes = described_class.new(project, user, target_type: 'issue').execute + notes = described_class.new(user, project: project, target_type: 'issue').execute expect(notes).to eq([note1]) end @@ -118,56 +118,51 @@ describe NotesFinder do context 'for target' do let(:project) { create(:project, :repository) } - let(:note1) { create :note_on_commit, project: project } - let(:note2) { create :note_on_commit, project: project } + let!(:note1) { create :note_on_commit, project: project } + let!(:note2) { create :note_on_commit, project: project } let(:commit) { note1.noteable } - let(:params) { { target_id: commit.id, target_type: 'commit', last_fetched_at: 1.hour.ago.to_i } } - - before do - note1 - note2 - end + let(:params) { { project: project, target_id: commit.id, target_type: 'commit', last_fetched_at: 1.hour.ago.to_i } } it 'finds all notes' do - notes = described_class.new(project, user, params).execute + notes = described_class.new(user, params).execute expect(notes.size).to eq(2) end it 'finds notes on merge requests' do note = create(:note_on_merge_request, project: project) - params = { target_type: 'merge_request', target_id: note.noteable.id } + params = { project: project, target_type: 'merge_request', target_id: note.noteable.id } - notes = described_class.new(project, user, params).execute + notes = described_class.new(user, params).execute expect(notes).to include(note) end it 'finds notes on snippets' do note = create(:note_on_project_snippet, project: project) - params = { target_type: 'snippet', target_id: note.noteable.id } + params = { project: project, target_type: 'snippet', target_id: note.noteable.id } - notes = described_class.new(project, user, params).execute + notes = described_class.new(user, params).execute expect(notes.count).to eq(1) end it 'finds notes on personal snippets' do note = create(:note_on_personal_snippet) - params = { target_type: 'personal_snippet', target_id: note.noteable_id } + params = { project: project, target_type: 'personal_snippet', target_id: note.noteable_id } - notes = described_class.new(project, user, params).execute + notes = described_class.new(user, params).execute expect(notes.count).to eq(1) end it 'raises an exception for an invalid target_type' do params[:target_type] = 'invalid' - expect { described_class.new(project, user, params).execute }.to raise_error("invalid target_type '#{params[:target_type]}'") + expect { described_class.new(user, params).execute }.to raise_error("invalid target_type '#{params[:target_type]}'") end it 'filters out old notes' do note2.update_attribute(:updated_at, 2.hours.ago) - notes = described_class.new(project, user, params).execute + notes = described_class.new(user, params).execute expect(notes).to eq([note1]) end @@ -175,25 +170,47 @@ describe NotesFinder do let(:confidential_issue) { create(:issue, :confidential, project: project, author: user) } let!(:confidential_note) { create(:note, noteable: confidential_issue, project: confidential_issue.project) } - let(:params) { { target_id: confidential_issue.id, target_type: 'issue', last_fetched_at: 1.hour.ago.to_i } } + let(:params) { { project: confidential_issue.project, target_id: confidential_issue.id, target_type: 'issue', last_fetched_at: 1.hour.ago.to_i } } it 'returns notes if user can see the issue' do - expect(described_class.new(project, user, params).execute).to eq([confidential_note]) + expect(described_class.new(user, params).execute).to eq([confidential_note]) end it 'raises an error if user can not see the issue' do user = create(:user) - expect { described_class.new(project, user, params).execute }.to raise_error(ActiveRecord::RecordNotFound) + expect { described_class.new(user, params).execute }.to raise_error(ActiveRecord::RecordNotFound) end it 'raises an error for project members with guest role' do user = create(:user) project.add_guest(user) - expect { described_class.new(project, user, params).execute }.to raise_error(ActiveRecord::RecordNotFound) + expect { described_class.new(user, params).execute }.to raise_error(ActiveRecord::RecordNotFound) end end end + + context 'for explicit target' do + let(:project) { create(:project, :repository) } + let!(:note1) { create :note_on_commit, project: project, created_at: 1.day.ago, updated_at: 2.hours.ago } + let!(:note2) { create :note_on_commit, project: project } + let(:commit) { note1.noteable } + let(:params) { { project: project, target: commit } } + + it 'returns the expected notes' do + expect(described_class.new(user, params).execute).to eq([note1, note2]) + end + + it 'returns the expected notes when last_fetched_at is given' do + params = { project: project, target: commit, last_fetched_at: 1.hour.ago.to_i } + expect(described_class.new(user, params).execute).to eq([note2]) + end + + it 'fails when nil is provided' do + params = { project: project, target: nil } + expect { described_class.new(user, params).execute }.to raise_error(RuntimeError) + end + end end describe '.search' do @@ -201,17 +218,17 @@ describe NotesFinder do let(:note) { create(:note_on_issue, note: 'WoW', project: project) } it 'returns notes with matching content' do - expect(described_class.new(note.project, nil, search: note.note).execute).to eq([note]) + expect(described_class.new(nil, project: note.project, search: note.note).execute).to eq([note]) end it 'returns notes with matching content regardless of the casing' do - expect(described_class.new(note.project, nil, search: 'WOW').execute).to eq([note]) + expect(described_class.new(nil, project: note.project, search: 'WOW').execute).to eq([note]) end it 'returns commit notes user can access' do note = create(:note_on_commit, project: project) - expect(described_class.new(note.project, create(:user), search: note.note).execute).to eq([note]) + expect(described_class.new(create(:user), project: note.project, search: note.note).execute).to eq([note]) end context "confidential issues" do @@ -220,27 +237,27 @@ describe NotesFinder do let(:confidential_note) { create(:note, note: "Random", noteable: confidential_issue, project: confidential_issue.project) } it "returns notes with matching content if user can see the issue" do - expect(described_class.new(confidential_note.project, user, search: confidential_note.note).execute).to eq([confidential_note]) + expect(described_class.new(user, project: confidential_note.project, search: confidential_note.note).execute).to eq([confidential_note]) end it "does not return notes with matching content if user can not see the issue" do user = create(:user) - expect(described_class.new(confidential_note.project, user, search: confidential_note.note).execute).to be_empty + expect(described_class.new(user, project: confidential_note.project, search: confidential_note.note).execute).to be_empty end it "does not return notes with matching content for project members with guest role" do user = create(:user) project.add_guest(user) - expect(described_class.new(confidential_note.project, user, search: confidential_note.note).execute).to be_empty + expect(described_class.new(user, project: confidential_note.project, search: confidential_note.note).execute).to be_empty end it "does not return notes with matching content for unauthenticated users" do - expect(described_class.new(confidential_note.project, nil, search: confidential_note.note).execute).to be_empty + expect(described_class.new(nil, project: confidential_note.project, search: confidential_note.note).execute).to be_empty end end context 'inlines SQL filters on subqueries for performance' do - let(:sql) { described_class.new(note.project, nil, search: note.note).execute.to_sql } + let(:sql) { described_class.new(nil, project: note.project, search: note.note).execute.to_sql } let(:number_of_noteable_types) { 4 } specify 'project_id check' do @@ -254,11 +271,11 @@ describe NotesFinder do end describe '#target' do - subject { described_class.new(project, user, params) } + subject { described_class.new(user, params) } context 'for a issue target' do let(:issue) { create(:issue, project: project) } - let(:params) { { target_type: 'issue', target_id: issue.id } } + let(:params) { { project: project, target_type: 'issue', target_id: issue.id } } it 'returns the issue' do expect(subject.target).to eq(issue) @@ -267,7 +284,7 @@ describe NotesFinder do context 'for a merge request target' do let(:merge_request) { create(:merge_request, source_project: project) } - let(:params) { { target_type: 'merge_request', target_id: merge_request.id } } + let(:params) { { project: project, target_type: 'merge_request', target_id: merge_request.id } } it 'returns the merge_request' do expect(subject.target).to eq(merge_request) @@ -276,7 +293,7 @@ describe NotesFinder do context 'for a snippet target' do let(:snippet) { create(:project_snippet, project: project) } - let(:params) { { target_type: 'snippet', target_id: snippet.id } } + let(:params) { { project: project, target_type: 'snippet', target_id: snippet.id } } it 'returns the snippet' do expect(subject.target).to eq(snippet) @@ -286,7 +303,7 @@ describe NotesFinder do context 'for a commit target' do let(:project) { create(:project, :repository) } let(:commit) { project.commit } - let(:params) { { target_type: 'commit', target_id: commit.id } } + let(:params) { { project: project, target_type: 'commit', target_id: commit.id } } it 'returns the commit' do expect(subject.target).to eq(commit) @@ -298,24 +315,24 @@ describe NotesFinder do let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } it 'finds issues by iid' do - iid_params = { target_type: 'issue', target_iid: issue.iid } - expect(described_class.new(project, user, iid_params).target).to eq(issue) + iid_params = { project: project, target_type: 'issue', target_iid: issue.iid } + expect(described_class.new(user, iid_params).target).to eq(issue) end it 'finds merge requests by iid' do - iid_params = { target_type: 'merge_request', target_iid: merge_request.iid } - expect(described_class.new(project, user, iid_params).target).to eq(merge_request) + iid_params = { project: project, target_type: 'merge_request', target_iid: merge_request.iid } + expect(described_class.new(user, iid_params).target).to eq(merge_request) end it 'returns nil if both target_id and target_iid are not given' do - params_without_any_id = { target_type: 'issue' } - expect(described_class.new(project, user, params_without_any_id).target).to be_nil + params_without_any_id = { project: project, target_type: 'issue' } + expect(described_class.new(user, params_without_any_id).target).to be_nil end it 'prioritizes target_id over target_iid' do issue2 = create(:issue, project: project) - iid_params = { target_type: 'issue', target_id: issue2.id, target_iid: issue.iid } - expect(described_class.new(project, user, iid_params).target).to eq(issue2) + iid_params = { project: project, target_type: 'issue', target_id: issue2.id, target_iid: issue.iid } + expect(described_class.new(user, iid_params).target).to eq(issue2) end end end diff --git a/spec/fixtures/api/schemas/entities/discussion.json b/spec/fixtures/api/schemas/entities/discussion.json new file mode 100644 index 00000000000..bcc1db79e83 --- /dev/null +++ b/spec/fixtures/api/schemas/entities/discussion.json @@ -0,0 +1,67 @@ +{ + "type": "object", + "required" : [ + "id", + "notes", + "individual_note" + ], + "properties" : { + "id": { "type": "string" }, + "individual_note": { "type": "boolean" }, + "notes": { + "type": "array", + "items": { + "type": "object", + "properties" : { + "id": { "type": "string" }, + "type": { "type": ["string", "null"] }, + "body": { "type": "string" }, + "attachment": { "type": ["string", "null"]}, + "award_emoji": { "type": "array" }, + "author": { + "type": "object", + "properties": { + "name": { "type": "string" }, + "username": { "type": "string" }, + "id": { "type": "integer" }, + "state": { "type": "string" }, + "avatar_url": { "type": "uri" }, + "web_url": { "type": "uri" }, + "status_tooltip_html": { "type": ["string", "null"] }, + "path": { "type": "string" } + }, + "additionalProperties": false + }, + "created_at": { "type": "date" }, + "updated_at": { "type": "date" }, + "system": { "type": "boolean" }, + "noteable_id": { "type": "integer" }, + "noteable_iid": { "type": "integer" }, + "noteable_type": { "type": "string" }, + "resolved": { "type": "boolean" }, + "resolvable": { "type": "boolean" }, + "resolved_by": { "type": ["string", "null"] }, + "note": { "type": "string" }, + "note_html": { "type": "string" }, + "current_user": { "type": "object" }, + "suggestions": { "type": "array" }, + "discussion_id": { "type": "string" }, + "emoji_awardable": { "type": "boolean" }, + "report_abuse_path": { "type": "string" }, + "noteable_note_url": { "type": "string" }, + "resolve_path": { "type": "string" }, + "resolve_with_issue_path": { "type": "string" }, + "cached_markdown_version": { "type": "integer" }, + "human_access": { "type": ["string", "null"] }, + "toggle_award_path": { "type": "string" }, + "path": { "type": "string" } + }, + "required": [ + "id", "attachment", "author", "created_at", "updated_at", + "system", "noteable_id", "noteable_type" + ], + "additionalProperties": false + } + } + } +} diff --git a/spec/fixtures/api/schemas/entities/discussions.json b/spec/fixtures/api/schemas/entities/discussions.json new file mode 100644 index 00000000000..5a837429776 --- /dev/null +++ b/spec/fixtures/api/schemas/entities/discussions.json @@ -0,0 +1,4 @@ +{ + "type": "array", + "items": { "$ref": "discussion.json" } +} diff --git a/spec/frontend/helpers/indent_helper_spec.js b/spec/frontend/helpers/indent_helper_spec.js deleted file mode 100644 index fca12f0d1ef..00000000000 --- a/spec/frontend/helpers/indent_helper_spec.js +++ /dev/null @@ -1,371 +0,0 @@ -import IndentHelper from '~/helpers/indent_helper'; - -function createMockTextarea() { - const el = document.createElement('textarea'); - el.setCursor = pos => el.setSelectionRange(pos, pos); - el.setCursorToEnd = () => el.setCursor(el.value.length); - el.selection = () => [el.selectionStart, el.selectionEnd]; - el.cursor = () => { - const [start, end] = el.selection(); - return start === end ? start : undefined; - }; - return el; -} - -describe('indent_helper', () => { - let element; - let ih; - - beforeEach(() => { - element = createMockTextarea(); - ih = new IndentHelper(element); - }); - - describe('indents', () => { - describe('a single line', () => { - it('when on an empty line; and cursor follows', () => { - element.value = ''; - ih.indent(); - expect(element.value).toBe(' '); - expect(element.cursor()).toBe(4); - ih.indent(); - expect(element.value).toBe(' '); - expect(element.cursor()).toBe(8); - }); - - it('when at the start of a line; and cursor stays at start', () => { - element.value = 'foobar'; - element.setCursor(0); - ih.indent(); - expect(element.value).toBe(' foobar'); - expect(element.cursor()).toBe(4); - }); - - it('when the cursor is in the middle; and cursor follows', () => { - element.value = 'foobar'; - element.setCursor(3); - ih.indent(); - expect(element.value).toBe(' foobar'); - expect(element.cursor()).toBe(7); - }); - }); - - describe('several lines', () => { - it('when everything is selected; and everything remains selected', () => { - element.value = 'foo\nbar\nbaz'; - element.setSelectionRange(0, 11); - ih.indent(); - expect(element.value).toBe(' foo\n bar\n baz'); - expect(element.selection()).toEqual([0, 23]); - }); - - it('when all lines are partially selected; and the selection adapts', () => { - element.value = 'foo\nbar\nbaz'; - element.setSelectionRange(2, 9); - ih.indent(); - expect(element.value).toBe(' foo\n bar\n baz'); - expect(element.selection()).toEqual([6, 21]); - }); - - it('when some lines are entirely selected; and entire lines remain selected', () => { - element.value = 'foo\nbar\nbaz'; - element.setSelectionRange(4, 11); - ih.indent(); - expect(element.value).toBe('foo\n bar\n baz'); - expect(element.selection()).toEqual([4, 19]); - }); - - it('when some lines are partially selected; and the selection adapts', () => { - element.value = 'foo\nbar\nbaz'; - element.setSelectionRange(5, 9); - ih.indent(); - expect(element.value).toBe('foo\n bar\n baz'); - expect(element.selection()).toEqual([5 + 4, 9 + 2 * 4]); - }); - - it('having different indentation when some lines are entirely selected; and entire lines remain selected', () => { - element.value = ' foo\nbar\n baz'; - element.setSelectionRange(8, 19); - ih.indent(); - expect(element.value).toBe(' foo\n bar\n baz'); - expect(element.selection()).toEqual([8, 27]); - }); - - it('having different indentation when some lines are partially selected; and the selection adapts', () => { - element.value = ' foo\nbar\n baz'; - element.setSelectionRange(9, 14); - ih.indent(); - expect(element.value).toBe(' foo\n bar\n baz'); - expect(element.selection()).toEqual([13, 22]); - }); - }); - }); - - describe('unindents', () => { - describe('a single line', () => { - it('but does nothing if there is not indent', () => { - element.value = 'foobar'; - element.setCursor(2); - ih.unindent(); - expect(element.value).toBe('foobar'); - expect(element.cursor()).toBe(2); - }); - - it('but does nothing if there is a partial indent', () => { - element.value = ' foobar'; - element.setCursor(1); - ih.unindent(); - expect(element.value).toBe(' foobar'); - expect(element.cursor()).toBe(1); - }); - - it('when the cursor is in the line text; cursor follows', () => { - element.value = ' foobar'; - element.setCursor(6); - ih.unindent(); - expect(element.value).toBe('foobar'); - expect(element.cursor()).toBe(2); - }); - - it('when the cursor is in the indent; and cursor goes to start', () => { - element.value = ' foobar'; - element.setCursor(2); - ih.unindent(); - expect(element.value).toBe('foobar'); - expect(element.cursor()).toBe(0); - }); - - it('when the cursor is at line start; and cursor stays at start', () => { - element.value = ' foobar'; - element.setCursor(0); - ih.unindent(); - expect(element.value).toBe('foobar'); - expect(element.cursor()).toBe(0); - }); - - it('when a selection includes part of the indent and text', () => { - element.value = ' foobar'; - element.setSelectionRange(2, 8); - ih.unindent(); - expect(element.value).toBe('foobar'); - expect(element.selection()).toEqual([0, 4]); - }); - - it('when a selection includes part of the indent only', () => { - element.value = ' foobar'; - element.setSelectionRange(0, 4); - ih.unindent(); - expect(element.value).toBe('foobar'); - expect(element.cursor()).toBe(0); - - element.value = ' foobar'; - element.setSelectionRange(1, 3); - ih.unindent(); - expect(element.value).toBe('foobar'); - expect(element.cursor()).toBe(0); - }); - }); - - describe('several lines', () => { - it('when everything is selected', () => { - element.value = ' foo\n bar\n baz'; - element.setSelectionRange(0, 27); - ih.unindent(); - expect(element.value).toBe('foo\n bar\nbaz'); - expect(element.selection()).toEqual([0, 15]); - }); - - it('when all lines are partially selected', () => { - element.value = ' foo\n bar\n baz'; - element.setSelectionRange(5, 26); - ih.unindent(); - expect(element.value).toBe('foo\n bar\nbaz'); - expect(element.selection()).toEqual([1, 14]); - }); - - it('when all lines are entirely selected', () => { - element.value = ' foo\n bar\n baz'; - element.setSelectionRange(8, 27); - ih.unindent(); - expect(element.value).toBe(' foo\n bar\nbaz'); - expect(element.selection()).toEqual([8, 19]); - }); - - it('when some lines are entirely selected', () => { - element.value = ' foo\n bar\n baz'; - element.setSelectionRange(8, 27); - ih.unindent(); - expect(element.value).toBe(' foo\n bar\nbaz'); - expect(element.selection()).toEqual([8, 19]); - }); - - it('when some lines are partially selected', () => { - element.value = ' foo\n bar\n baz'; - element.setSelectionRange(17, 26); - ih.unindent(); - expect(element.value).toBe(' foo\n bar\nbaz'); - expect(element.selection()).toEqual([13, 18]); - }); - - it('when some lines are partially selected within their indents', () => { - element.value = ' foo\n bar\n baz'; - element.setSelectionRange(10, 22); - ih.unindent(); - expect(element.value).toBe(' foo\n bar\nbaz'); - expect(element.selection()).toEqual([8, 16]); - }); - }); - }); - - describe('newline', () => { - describe('on a single line', () => { - it('auto-indents the new line', () => { - element.value = 'foo\n bar\n baz\n qux'; - - element.setCursor(3); - ih.newline(); - expect(element.value).toBe('foo\n\n bar\n baz\n qux'); - expect(element.cursor()).toBe(4); - - element.setCursor(9); - ih.newline(); - expect(element.value).toBe('foo\n\n bar\n \n baz\n qux'); - expect(element.cursor()).toBe(11); - - element.setCursor(19); - ih.newline(); - expect(element.value).toBe('foo\n\n bar\n \n baz\n \n qux'); - expect(element.cursor()).toBe(24); - - element.setCursor(36); - ih.newline(); - expect(element.value).toBe('foo\n\n bar\n \n baz\n \n qux\n '); - expect(element.cursor()).toBe(45); - }); - - it('splits a line and auto-indents', () => { - element.value = ' foobar'; - element.setCursor(7); - ih.newline(); - expect(element.value).toBe(' foo\n bar'); - expect(element.cursor()).toBe(12); - }); - - it('replaces selection with an indented newline', () => { - element.value = ' foobarbaz'; - element.setSelectionRange(7, 10); - ih.newline(); - expect(element.value).toBe(' foo\n baz'); - expect(element.cursor()).toBe(12); - }); - }); - - it('on several lines.replaces selection with indented newline', () => { - element.value = ' foo\n bar\n baz'; - element.setSelectionRange(4, 17); - ih.newline(); - expect(element.value).toBe(' fo\n az'); - expect(element.cursor()).toBe(7); - }); - }); - - describe('backspace', () => { - let event; - - // This suite tests only the special indent-removing behaviour of the - // backspace() method, since non-special cases are handled natively as a - // backspace keypress. - - beforeEach(() => { - event = { preventDefault: jest.fn() }; - }); - - describe('on a single line', () => { - it('does nothing special if in the line text', () => { - element.value = ' foobar'; - element.setCursor(7); - ih.backspace(event); - expect(event.preventDefault).not.toHaveBeenCalled(); - }); - - it('does nothing special if after a non-leading indent', () => { - element.value = ' foo bar'; - element.setCursor(11); - ih.backspace(event); - expect(event.preventDefault).not.toHaveBeenCalled(); - }); - - it('deletes one leading indent', () => { - element.value = ' foo'; - element.setCursor(8); - ih.backspace(event); - expect(event.preventDefault).toHaveBeenCalled(); - expect(element.value).toBe(' foo'); - expect(element.cursor()).toBe(4); - }); - - it('does nothing if cursor is inside the leading indent', () => { - element.value = ' foo'; - element.setCursor(4); - ih.backspace(event); - expect(event.preventDefault).not.toHaveBeenCalled(); - }); - - it('does nothing if cursor is at the start of the line', () => { - element.value = ' foo'; - element.setCursor(0); - ih.backspace(event); - expect(event.preventDefault).not.toHaveBeenCalled(); - }); - - it('deletes one partial indent', () => { - element.value = ' foo'; - element.setCursor(6); - ih.backspace(event); - expect(event.preventDefault).toHaveBeenCalled(); - expect(element.value).toBe(' foo'); - expect(element.cursor()).toBe(4); - }); - - it('deletes indents sequentially', () => { - element.value = ' foo'; - element.setCursor(10); - ih.backspace(event); - ih.backspace(event); - ih.backspace(event); - expect(event.preventDefault).toHaveBeenCalled(); - expect(element.value).toBe('foo'); - expect(element.cursor()).toBe(0); - }); - }); - - describe('on several lines', () => { - it('deletes indent only on its own line', () => { - element.value = ' foo\n bar\n baz'; - element.setCursor(16); - ih.backspace(event); - expect(event.preventDefault).toHaveBeenCalled(); - expect(element.value).toBe(' foo\n bar\n baz'); - expect(element.cursor()).toBe(12); - }); - - it('has no special behaviour with any range selection', () => { - const text = ' foo\n bar\n baz'; - for (let start = 0; start < text.length; start += 1) { - for (let end = start + 1; end < text.length; end += 1) { - element.value = text; - element.setSelectionRange(start, end); - ih.backspace(event); - expect(event.preventDefault).not.toHaveBeenCalled(); - - // Ensure that the backspace() method doesn't change state - // In reality, these two statements won't hold because the browser - // will natively process the backspace event. - expect(element.value).toBe(text); - expect(element.selection()).toEqual([start, end]); - } - } - }); - }); - }); -}); diff --git a/spec/frontend/lib/utils/common_utils_spec.js b/spec/frontend/lib/utils/common_utils_spec.js deleted file mode 100644 index e3d3b82d2f3..00000000000 --- a/spec/frontend/lib/utils/common_utils_spec.js +++ /dev/null @@ -1,180 +0,0 @@ -import * as cu from '~/lib/utils/common_utils'; - -const CMD_ENTITY = '⌘'; - -// Redefine `navigator.platform` because it's unsettable by default in JSDOM. -let platform; -Object.defineProperty(navigator, 'platform', { - configurable: true, - get: () => platform, - set: val => { - platform = val; - }, -}); - -describe('common_utils', () => { - describe('platform leader key helpers', () => { - const CTRL_EVENT = { ctrlKey: true }; - const META_EVENT = { metaKey: true }; - const BOTH_EVENT = { ctrlKey: true, metaKey: true }; - - it('should return "ctrl" if navigator.platform is unset', () => { - expect(cu.getPlatformLeaderKey()).toBe('ctrl'); - expect(cu.getPlatformLeaderKeyHTML()).toBe('Ctrl'); - expect(cu.isPlatformLeaderKey(CTRL_EVENT)).toBe(true); - expect(cu.isPlatformLeaderKey(META_EVENT)).toBe(false); - expect(cu.isPlatformLeaderKey(BOTH_EVENT)).toBe(true); - }); - - it('should return "meta" on MacOS', () => { - navigator.platform = 'MacIntel'; - expect(cu.getPlatformLeaderKey()).toBe('meta'); - expect(cu.getPlatformLeaderKeyHTML()).toBe(CMD_ENTITY); - expect(cu.isPlatformLeaderKey(CTRL_EVENT)).toBe(false); - expect(cu.isPlatformLeaderKey(META_EVENT)).toBe(true); - expect(cu.isPlatformLeaderKey(BOTH_EVENT)).toBe(true); - }); - - it('should return "ctrl" on Linux', () => { - navigator.platform = 'Linux is great'; - expect(cu.getPlatformLeaderKey()).toBe('ctrl'); - expect(cu.getPlatformLeaderKeyHTML()).toBe('Ctrl'); - expect(cu.isPlatformLeaderKey(CTRL_EVENT)).toBe(true); - expect(cu.isPlatformLeaderKey(META_EVENT)).toBe(false); - expect(cu.isPlatformLeaderKey(BOTH_EVENT)).toBe(true); - }); - - it('should return "ctrl" on Windows', () => { - navigator.platform = 'Win32'; - expect(cu.getPlatformLeaderKey()).toBe('ctrl'); - expect(cu.getPlatformLeaderKeyHTML()).toBe('Ctrl'); - expect(cu.isPlatformLeaderKey(CTRL_EVENT)).toBe(true); - expect(cu.isPlatformLeaderKey(META_EVENT)).toBe(false); - expect(cu.isPlatformLeaderKey(BOTH_EVENT)).toBe(true); - }); - }); - - describe('keystroke', () => { - const CODE_BACKSPACE = 8; - const CODE_TAB = 9; - const CODE_ENTER = 13; - const CODE_SPACE = 32; - const CODE_4 = 52; - const CODE_F = 70; - const CODE_Z = 90; - - // Helper function that quickly creates KeyboardEvents - const k = (code, modifiers = '') => ({ - keyCode: code, - which: code, - altKey: modifiers.includes('a'), - ctrlKey: modifiers.includes('c'), - metaKey: modifiers.includes('m'), - shiftKey: modifiers.includes('s'), - }); - - const EV_F = k(CODE_F); - const EV_ALT_F = k(CODE_F, 'a'); - const EV_CONTROL_F = k(CODE_F, 'c'); - const EV_META_F = k(CODE_F, 'm'); - const EV_SHIFT_F = k(CODE_F, 's'); - const EV_CONTROL_SHIFT_F = k(CODE_F, 'cs'); - const EV_ALL_F = k(CODE_F, 'scma'); - const EV_ENTER = k(CODE_ENTER); - const EV_TAB = k(CODE_TAB); - const EV_SPACE = k(CODE_SPACE); - const EV_BACKSPACE = k(CODE_BACKSPACE); - const EV_4 = k(CODE_4); - const EV_$ = k(CODE_4, 's'); - - const { keystroke } = cu; - - it('short-circuits with bad arguments', () => { - expect(keystroke()).toBe(false); - expect(keystroke({})).toBe(false); - }); - - it('handles keystrokes using key codes', () => { - // Test a letter key with modifiers - expect(keystroke(EV_F, CODE_F)).toBe(true); - expect(keystroke(EV_F, CODE_F, '')).toBe(true); - expect(keystroke(EV_ALT_F, CODE_F, 'a')).toBe(true); - expect(keystroke(EV_CONTROL_F, CODE_F, 'c')).toBe(true); - expect(keystroke(EV_META_F, CODE_F, 'm')).toBe(true); - expect(keystroke(EV_SHIFT_F, CODE_F, 's')).toBe(true); - expect(keystroke(EV_CONTROL_SHIFT_F, CODE_F, 'cs')).toBe(true); - expect(keystroke(EV_ALL_F, CODE_F, 'acms')).toBe(true); - - // Test non-letter keys - expect(keystroke(EV_TAB, CODE_TAB)).toBe(true); - expect(keystroke(EV_ENTER, CODE_ENTER)).toBe(true); - expect(keystroke(EV_SPACE, CODE_SPACE)).toBe(true); - expect(keystroke(EV_BACKSPACE, CODE_BACKSPACE)).toBe(true); - - // Test a number/symbol key - expect(keystroke(EV_4, CODE_4)).toBe(true); - expect(keystroke(EV_$, CODE_4, 's')).toBe(true); - - // Test wrong input - expect(keystroke(EV_F, CODE_Z)).toBe(false); - expect(keystroke(EV_SHIFT_F, CODE_F)).toBe(false); - expect(keystroke(EV_SHIFT_F, CODE_F, 'c')).toBe(false); - }); - - it('is case-insensitive', () => { - expect(keystroke(EV_ALL_F, CODE_F, 'ACMS')).toBe(true); - }); - - it('handles bogus inputs', () => { - expect(keystroke(EV_F, 'not a keystroke')).toBe(false); - expect(keystroke(EV_F, null)).toBe(false); - }); - - it('handles exact modifier keys, in any order', () => { - // Test permutations of modifiers - expect(keystroke(EV_ALL_F, CODE_F, 'acms')).toBe(true); - expect(keystroke(EV_ALL_F, CODE_F, 'smca')).toBe(true); - expect(keystroke(EV_ALL_F, CODE_F, 'csma')).toBe(true); - expect(keystroke(EV_CONTROL_SHIFT_F, CODE_F, 'cs')).toBe(true); - expect(keystroke(EV_CONTROL_SHIFT_F, CODE_F, 'sc')).toBe(true); - - // Test wrong modifiers - expect(keystroke(EV_ALL_F, CODE_F, 'smca')).toBe(true); - expect(keystroke(EV_ALL_F, CODE_F)).toBe(false); - expect(keystroke(EV_ALL_F, CODE_F, '')).toBe(false); - expect(keystroke(EV_ALL_F, CODE_F, 'c')).toBe(false); - expect(keystroke(EV_ALL_F, CODE_F, 'ca')).toBe(false); - expect(keystroke(EV_ALL_F, CODE_F, 'ms')).toBe(false); - expect(keystroke(EV_CONTROL_SHIFT_F, CODE_F, 'cs')).toBe(true); - expect(keystroke(EV_CONTROL_SHIFT_F, CODE_F, 'c')).toBe(false); - expect(keystroke(EV_CONTROL_SHIFT_F, CODE_F, 's')).toBe(false); - expect(keystroke(EV_CONTROL_SHIFT_F, CODE_F, 'csa')).toBe(false); - expect(keystroke(EV_CONTROL_SHIFT_F, CODE_F, 'm')).toBe(false); - expect(keystroke(EV_SHIFT_F, CODE_F, 's')).toBe(true); - expect(keystroke(EV_SHIFT_F, CODE_F, 'c')).toBe(false); - expect(keystroke(EV_SHIFT_F, CODE_F, 'csm')).toBe(false); - }); - - it('handles the platform-dependent leader key', () => { - navigator.platform = 'Win32'; - let EV_UNDO = k(CODE_Z, 'c'); - let EV_REDO = k(CODE_Z, 'cs'); - expect(keystroke(EV_UNDO, CODE_Z, 'l')).toBe(true); - expect(keystroke(EV_UNDO, CODE_Z, 'c')).toBe(true); - expect(keystroke(EV_UNDO, CODE_Z, 'm')).toBe(false); - expect(keystroke(EV_REDO, CODE_Z, 'sl')).toBe(true); - expect(keystroke(EV_REDO, CODE_Z, 'sc')).toBe(true); - expect(keystroke(EV_REDO, CODE_Z, 'sm')).toBe(false); - - navigator.platform = 'MacIntel'; - EV_UNDO = k(CODE_Z, 'm'); - EV_REDO = k(CODE_Z, 'ms'); - expect(keystroke(EV_UNDO, CODE_Z, 'l')).toBe(true); - expect(keystroke(EV_UNDO, CODE_Z, 'c')).toBe(false); - expect(keystroke(EV_UNDO, CODE_Z, 'm')).toBe(true); - expect(keystroke(EV_REDO, CODE_Z, 'sl')).toBe(true); - expect(keystroke(EV_REDO, CODE_Z, 'sc')).toBe(false); - expect(keystroke(EV_REDO, CODE_Z, 'sm')).toBe(true); - }); - }); -}); diff --git a/spec/frontend/lib/utils/undo_stack_spec.js b/spec/frontend/lib/utils/undo_stack_spec.js deleted file mode 100644 index 31ad0e77d6f..00000000000 --- a/spec/frontend/lib/utils/undo_stack_spec.js +++ /dev/null @@ -1,237 +0,0 @@ -import UndoStack from '~/lib/utils/undo_stack'; - -import { isEqual } from 'underscore'; - -describe('UndoStack', () => { - let stack; - - beforeEach(() => { - stack = new UndoStack(); - }); - - afterEach(() => { - // Make sure there's not pending saves - const history = Array.from(stack.history); - jest.runAllTimers(); - expect(stack.history).toEqual(history); - }); - - it('is blank on construction', () => { - expect(stack.isEmpty()).toBe(true); - expect(stack.history).toEqual([]); - expect(stack.cursor).toBe(-1); - expect(stack.canUndo()).toBe(false); - expect(stack.canRedo()).toBe(false); - }); - - it('handles simple undo/redo behaviour', () => { - stack.save(10); - stack.save(11); - stack.save(12); - - expect(stack.history).toEqual([10, 11, 12]); - expect(stack.cursor).toBe(2); - expect(stack.current()).toBe(12); - expect(stack.isEmpty()).toBe(false); - expect(stack.canUndo()).toBe(true); - expect(stack.canRedo()).toBe(false); - - stack.undo(); - expect(stack.history).toEqual([10, 11, 12]); - expect(stack.current()).toBe(11); - expect(stack.canUndo()).toBe(true); - expect(stack.canRedo()).toBe(true); - - stack.undo(); - expect(stack.current()).toBe(10); - expect(stack.canUndo()).toBe(false); - expect(stack.canRedo()).toBe(true); - - stack.redo(); - expect(stack.current()).toBe(11); - - stack.redo(); - expect(stack.current()).toBe(12); - expect(stack.isEmpty()).toBe(false); - expect(stack.canUndo()).toBe(true); - expect(stack.canRedo()).toBe(false); - - // Saving should clear the redo stack - stack.undo(); - stack.save(13); - expect(stack.history).toEqual([10, 11, 13]); - expect(stack.current()).toBe(13); - }); - - it('clear() should clear the undo history', () => { - stack.save(0); - stack.save(1); - stack.save(2); - stack.clear(); - expect(stack.history).toEqual([]); - expect(stack.current()).toBeUndefined(); - }); - - it('undo and redo are no-ops if unavailable', () => { - stack.save(10); - expect(stack.canRedo()).toBe(false); - expect(stack.canUndo()).toBe(false); - - stack.save(11); - expect(stack.canRedo()).toBe(false); - expect(stack.canUndo()).toBe(true); - - expect(stack.redo()).toBeUndefined(); - expect(stack.history).toEqual([10, 11]); - expect(stack.current()).toBe(11); - expect(stack.canRedo()).toBe(false); - expect(stack.canUndo()).toBe(true); - - expect(stack.undo()).toBe(10); - expect(stack.undo()).toBeUndefined(); - expect(stack.history).toEqual([10, 11]); - expect(stack.current()).toBe(10); - expect(stack.canRedo()).toBe(true); - expect(stack.canUndo()).toBe(false); - }); - - it('should not save a duplicate state', () => { - stack.save(10); - stack.save(11); - stack.save(11); - stack.save(10); - stack.save(10); - - expect(stack.history).toEqual([10, 11, 10]); - }); - - it('uses the === operator to detect duplicates', () => { - stack.save(10); - stack.save(10); - expect(stack.history).toEqual([10]); - - // eslint-disable-next-line eqeqeq - expect(2 == '2' && '2' == 2).toBe(true); - stack.clear(); - stack.save(2); - stack.save(2); - stack.save('2'); - stack.save('2'); - stack.save(2); - expect(stack.history).toEqual([2, '2', 2]); - - const obj = {}; - stack.clear(); - stack.save(obj); - stack.save(obj); - stack.save({}); - stack.save({}); - expect(stack.history).toEqual([{}, {}, {}]); - }); - - it('should allow custom comparators', () => { - stack.comparator = isEqual; - const obj = {}; - stack.clear(); - stack.save(obj); - stack.save(obj); - stack.save({}); - stack.save({}); - expect(stack.history).toEqual([{}]); - }); - - it('should enforce a max number of undo states', () => { - // Try 2000 saves. Only the last 1000 should be preserved. - const sequence = Array(2000) - .fill(0) - .map((el, i) => i); - sequence.forEach(stack.save.bind(stack)); - expect(stack.history.length).toBe(1000); - expect(stack.history).toEqual(sequence.slice(1000)); - expect(stack.current()).toBe(1999); - expect(stack.canUndo()).toBe(true); - expect(stack.canRedo()).toBe(false); - - // Saving drops the oldest elements from the stack - stack.save('end'); - expect(stack.history.length).toBe(1000); - expect(stack.current()).toBe('end'); - expect(stack.history).toEqual([...sequence.slice(1001), 'end']); - - // If states were undone but the history is full, can still add. - stack.undo(); - stack.undo(); - expect(stack.current()).toBe(1998); - stack.save(3000); - expect(stack.history.length).toBe(999); - // should be [1001, 1002, ..., 1998, 3000] - expect(stack.history).toEqual([...sequence.slice(1001, 1999), 3000]); - - // Try a different max length - stack = new UndoStack(2); - stack.save(0); - expect(stack.history).toEqual([0]); - stack.save(1); - expect(stack.history).toEqual([0, 1]); - stack.save(2); - expect(stack.history).toEqual([1, 2]); - }); - - describe('scheduled saves', () => { - it('should work', () => { - // Schedules 1000 ms ahead by default - stack.save(0); - stack.scheduleSave(1); - expect(stack.history).toEqual([0]); - jest.advanceTimersByTime(999); - expect(stack.history).toEqual([0]); - jest.advanceTimersByTime(1); - expect(stack.history).toEqual([0, 1]); - }); - - it('should have an adjustable delay', () => { - stack.scheduleSave(2, 100); - jest.advanceTimersByTime(100); - expect(stack.history).toEqual([2]); - }); - - it('should cancel previous scheduled saves', () => { - stack.scheduleSave(3); - jest.advanceTimersByTime(100); - stack.scheduleSave(4); - jest.runAllTimers(); - expect(stack.history).toEqual([4]); - }); - - it('should be canceled by explicit saves', () => { - stack.scheduleSave(5); - stack.save(6); - jest.runAllTimers(); - expect(stack.history).toEqual([6]); - }); - - it('should be canceled by undos and redos', () => { - stack.save(1); - stack.save(2); - stack.scheduleSave(3); - stack.undo(); - jest.runAllTimers(); - expect(stack.history).toEqual([1, 2]); - expect(stack.current()).toBe(1); - - stack.scheduleSave(4); - stack.redo(); - jest.runAllTimers(); - expect(stack.history).toEqual([1, 2]); - expect(stack.current()).toBe(2); - }); - - it('should be persisted immediately with saveNow()', () => { - stack.scheduleSave(7); - stack.scheduleSave(8); - stack.saveNow(); - jest.runAllTimers(); - expect(stack.history).toEqual([8]); - }); - }); -}); diff --git a/spec/javascripts/diffs/mock_data/diff_file.js b/spec/javascripts/diffs/mock_data/diff_file.js index 27428197c1c..531686efff1 100644 --- a/spec/javascripts/diffs/mock_data/diff_file.js +++ b/spec/javascripts/diffs/mock_data/diff_file.js @@ -1,3 +1,5 @@ +// Copied to ee/spec/frontend/diffs/mock_data/diff_file.js + export default { submodule: false, submodule_link: null, diff --git a/spec/javascripts/jobs/components/job_app_spec.js b/spec/javascripts/jobs/components/job_app_spec.js index c58d59b4b16..40decbb5643 100644 --- a/spec/javascripts/jobs/components/job_app_spec.js +++ b/spec/javascripts/jobs/components/job_app_spec.js @@ -486,6 +486,31 @@ describe('Job App ', () => { }); }); }); + + describe('sidebar', () => { + it('has no blank blocks', done => { + mock.onGet(props.endpoint).replyOnce( + 200, + Object.assign({}, job, { + duration: null, + finished_at: null, + erased_at: null, + queued: null, + runner: null, + coverage: null, + tags: [], + cancel_path: null, + }), + ); + + vm.$nextTick(() => { + vm.$el.querySelectorAll('.blocks-container > *').forEach(block => { + expect(block.textContent.trim()).not.toBe(''); + }); + done(); + }); + }); + }); }); describe('archived job', () => { diff --git a/spec/javascripts/notes/mock_data.js b/spec/javascripts/notes/mock_data.js index 1df5cf9ef68..5f81a168498 100644 --- a/spec/javascripts/notes/mock_data.js +++ b/spec/javascripts/notes/mock_data.js @@ -1,3 +1,5 @@ +// Copied to ee/spec/frontend/notes/mock_data.js + export const notesDataMock = { discussionsPath: '/gitlab-org/gitlab-ce/issues/26/discussions.json', lastFetchedAt: 1501862675, diff --git a/spec/lib/gitlab/ci/config/entry/job_spec.rb b/spec/lib/gitlab/ci/config/entry/job_spec.rb index d5861d5dd07..800ef122203 100644 --- a/spec/lib/gitlab/ci/config/entry/job_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/job_spec.rb @@ -86,6 +86,22 @@ describe Gitlab::Ci::Config::Entry::Job do it { expect(entry).to be_valid } end end + + context 'when has needs' do + let(:config) do + { script: 'echo', needs: ['another-job'] } + end + + it { expect(entry).to be_valid } + + context 'when has dependencies' do + let(:config) do + { script: 'echo', dependencies: ['another-job'], needs: ['another-job'] } + end + + it { expect(entry).to be_valid } + end + end end context 'when entry value is not correct' do @@ -223,6 +239,43 @@ describe Gitlab::Ci::Config::Entry::Job do expect(entry.errors).to include 'job start in must be blank' end end + + context 'when has dependencies' do + context 'that are not a array of strings' do + let(:config) do + { script: 'echo', dependencies: 'build-job' } + end + + it 'returns error about invalid type' do + expect(entry).not_to be_valid + expect(entry.errors).to include 'job dependencies should be an array of strings' + end + end + end + + context 'when has needs' do + context 'that are not a array of strings' do + let(:config) do + { script: 'echo', needs: 'build-job' } + end + + it 'returns error about invalid type' do + expect(entry).not_to be_valid + expect(entry.errors).to include 'job needs should be an array of strings' + end + end + + context 'when have dependencies that are not subset of needs' do + let(:config) do + { script: 'echo', dependencies: ['another-job'], needs: ['build-job'] } + end + + it 'returns error about invalid data' do + expect(entry).not_to be_valid + expect(entry.errors).to include 'job dependencies the another-job should be part of needs' + end + end + end end end diff --git a/spec/lib/gitlab/ci/config/normalizer_spec.rb b/spec/lib/gitlab/ci/config/normalizer_spec.rb index cd880177170..6b766cc37bf 100644 --- a/spec/lib/gitlab/ci/config/normalizer_spec.rb +++ b/spec/lib/gitlab/ci/config/normalizer_spec.rb @@ -49,37 +49,44 @@ describe Gitlab::Ci::Config::Normalizer do end end - context 'when jobs depend on parallelized jobs' do - let(:config) { { job_name => job_config, other_job: { script: 'echo 1', dependencies: [job_name.to_s] } } } - - it 'parallelizes dependencies' do - job_names = ["rspec 1/5", "rspec 2/5", "rspec 3/5", "rspec 4/5", "rspec 5/5"] - - expect(subject[:other_job][:dependencies]).to include(*job_names) + %i[dependencies needs].each do |context| + context "when job has #{context} on parallelized jobs" do + let(:config) do + { + job_name => job_config, + other_job: { script: 'echo 1', context => [job_name.to_s] } + } + end + + it "parallelizes #{context}" do + job_names = ["rspec 1/5", "rspec 2/5", "rspec 3/5", "rspec 4/5", "rspec 5/5"] + + expect(subject[:other_job][context]).to include(*job_names) + end + + it "does not include original job name in #{context}" do + expect(subject[:other_job][context]).not_to include(job_name) + end end - it 'does not include original job name in dependencies' do - expect(subject[:other_job][:dependencies]).not_to include(job_name) - end - end + context "when there are #{context} which are both parallelized and not" do + let(:config) do + { + job_name => job_config, + other_job: { script: 'echo 1' }, + final_job: { script: 'echo 1', context => [job_name.to_s, "other_job"] } + } + end - context 'when there are dependencies which are both parallelized and not' do - let(:config) do - { - job_name => job_config, - other_job: { script: 'echo 1' }, - final_job: { script: 'echo 1', dependencies: [job_name.to_s, "other_job"] } - } - end - - it 'parallelizes dependencies' do - job_names = ["rspec 1/5", "rspec 2/5", "rspec 3/5", "rspec 4/5", "rspec 5/5"] + it "parallelizes #{context}" do + job_names = ["rspec 1/5", "rspec 2/5", "rspec 3/5", "rspec 4/5", "rspec 5/5"] - expect(subject[:final_job][:dependencies]).to include(*job_names) - end + expect(subject[:final_job][context]).to include(*job_names) + end - it 'includes the regular job in dependencies' do - expect(subject[:final_job][:dependencies]).to include('other_job') + it "includes the regular job in #{context}" do + expect(subject[:final_job][context]).to include('other_job') + end end end end diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb index d3676ee03bf..4ffa1fc9fd8 100644 --- a/spec/lib/gitlab/ci/yaml_processor_spec.rb +++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb @@ -1112,6 +1112,86 @@ module Gitlab end end + describe "Needs" do + let(:needs) { } + let(:dependencies) { } + + let(:config) do + { + build1: { stage: 'build', script: 'test' }, + build2: { stage: 'build', script: 'test' }, + test1: { stage: 'test', script: 'test', needs: needs, dependencies: dependencies }, + test2: { stage: 'test', script: 'test' }, + deploy: { stage: 'test', script: 'test' } + } + end + + subject { Gitlab::Ci::YamlProcessor.new(YAML.dump(config)) } + + context 'no needs' do + it { expect { subject }.not_to raise_error } + end + + context 'needs to builds' do + let(:needs) { %w(build1 build2) } + + it "does create jobs with valid specification" do + expect(subject.builds.size).to eq(5) + expect(subject.builds[0]).to eq( + stage: "build", + stage_idx: 0, + name: "build1", + options: { + script: ["test"] + }, + when: "on_success", + allow_failure: false, + yaml_variables: [] + ) + expect(subject.builds[2]).to eq( + stage: "test", + stage_idx: 1, + name: "test1", + options: { + script: ["test"] + }, + needs_attributes: [ + { name: "build1" }, + { name: "build2" } + ], + when: "on_success", + allow_failure: false, + yaml_variables: [] + ) + end + end + + context 'needs to builds defined as symbols' do + let(:needs) { [:build1, :build2] } + + it { expect { subject }.not_to raise_error } + end + + context 'undefined need' do + let(:needs) { ['undefined'] } + + it { expect { subject }.to raise_error(Gitlab::Ci::YamlProcessor::ValidationError, 'test1 job: undefined need: undefined') } + end + + context 'needs to deploy' do + let(:needs) { ['deploy'] } + + it { expect { subject }.to raise_error(Gitlab::Ci::YamlProcessor::ValidationError, 'test1 job: need deploy is not defined in prior stages') } + end + + context 'needs and dependencies that are mismatching' do + let(:needs) { %w(build1) } + let(:dependencies) { %w(build2) } + + it { expect { subject }.to raise_error(Gitlab::Ci::YamlProcessor::ValidationError, 'jobs:test1 dependencies the build2 should be part of needs') } + end + end + describe "Hidden jobs" do let(:config_processor) { Gitlab::Ci::YamlProcessor.new(config) } subject { config_processor.stage_builds_attributes("test") } diff --git a/spec/lib/gitlab/exclusive_lease_helpers_spec.rb b/spec/lib/gitlab/exclusive_lease_helpers_spec.rb index 5107e1efbbd..c3b706fc538 100644 --- a/spec/lib/gitlab/exclusive_lease_helpers_spec.rb +++ b/spec/lib/gitlab/exclusive_lease_helpers_spec.rb @@ -25,13 +25,13 @@ describe Gitlab::ExclusiveLeaseHelpers, :clean_gitlab_redis_shared_state do end it 'calls the given block' do - expect { |b| class_instance.in_lock(unique_key, &b) }.to yield_control.once + expect { |b| class_instance.in_lock(unique_key, &b) }.to yield_with_args(false) end it 'calls the given block continuously' do - expect { |b| class_instance.in_lock(unique_key, &b) }.to yield_control.once - expect { |b| class_instance.in_lock(unique_key, &b) }.to yield_control.once - expect { |b| class_instance.in_lock(unique_key, &b) }.to yield_control.once + expect { |b| class_instance.in_lock(unique_key, &b) }.to yield_with_args(false) + expect { |b| class_instance.in_lock(unique_key, &b) }.to yield_with_args(false) + expect { |b| class_instance.in_lock(unique_key, &b) }.to yield_with_args(false) end it 'cancels the exclusive lease after the block' do @@ -68,6 +68,15 @@ describe Gitlab::ExclusiveLeaseHelpers, :clean_gitlab_redis_shared_state do expect { subject }.to raise_error('Failed to obtain a lock') end + + context 'when lease is granted after retry' do + it 'yields block with true' do + expect(lease).to receive(:try_obtain).exactly(3).times { nil } + expect(lease).to receive(:try_obtain).once { unique_key } + + expect { |b| class_instance.in_lock(unique_key, &b) }.to yield_with_args(true) + end + end end context 'when sleep second is specified' do diff --git a/spec/models/ci/build_need_spec.rb b/spec/models/ci/build_need_spec.rb new file mode 100644 index 00000000000..450dd550a8f --- /dev/null +++ b/spec/models/ci/build_need_spec.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Ci::BuildNeed, model: true do + let(:build_need) { build(:ci_build_need) } + + it { is_expected.to belong_to(:build) } + + it { is_expected.to validate_presence_of(:build) } + it { is_expected.to validate_presence_of(:name) } + it { is_expected.to validate_length_of(:name).is_at_most(128) } +end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 17c7c05324a..9369f393b5e 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -19,7 +19,8 @@ describe Ci::Build do it { is_expected.to belong_to(:runner) } it { is_expected.to belong_to(:trigger_request) } it { is_expected.to belong_to(:erased_by) } - it { is_expected.to have_many(:trace_sections)} + it { is_expected.to have_many(:trace_sections) } + it { is_expected.to have_many(:needs) } it { is_expected.to have_one(:deployment) } it { is_expected.to have_one(:runner_session) } it { is_expected.to have_many(:job_variables) } @@ -182,6 +183,30 @@ describe Ci::Build do end end + describe '.with_needs' do + let!(:build) { create(:ci_build) } + let!(:build_need_a) { create(:ci_build_need, build: build) } + let!(:build_need_b) { create(:ci_build_need, build: build) } + + context 'when passing build name' do + subject { described_class.with_needs(build_need_a.name) } + + it { is_expected.to contain_exactly(build) } + end + + context 'when not passing any build name' do + subject { described_class.with_needs } + + it { is_expected.to contain_exactly(build) } + end + + context 'when not matching build name' do + subject { described_class.with_needs('undefined') } + + it { is_expected.to be_empty } + end + end + describe '#enqueue' do let(:build) { create(:ci_build, :created) } @@ -595,6 +620,46 @@ describe Ci::Build do expect(staging.depends_on_builds.map(&:id)) .to contain_exactly(build.id, retried_rspec.id, rubocop_test.id) end + + describe '#dependencies' do + let(:dependencies) { } + let(:needs) { } + + let!(:final) do + create(:ci_build, + pipeline: pipeline, name: 'final', + stage_idx: 3, stage: 'deploy', options: { + dependencies: dependencies, + needs: needs + } + ) + end + + subject { final.dependencies } + + context 'when depedencies are defined' do + let(:dependencies) { %w(rspec staging) } + + it { is_expected.to contain_exactly(rspec_test, staging) } + end + + context 'when needs are defined' do + let(:needs) { %w(build rspec staging) } + + it { is_expected.to contain_exactly(build, rspec_test, staging) } + end + + context 'when needs and dependencies are defined' do + let(:dependencies) { %w(rspec staging) } + let(:needs) { %w(build rspec staging) } + + it { is_expected.to contain_exactly(rspec_test, staging) } + end + + context 'when nor dependencies or needs are defined' do + it { is_expected.to contain_exactly(build, rspec_test, rubocop_test, staging) } + end + end end describe '#triggered_by?' do @@ -3614,6 +3679,7 @@ describe Ci::Build do before do build.ensure_metadata + build.needs.create!(name: 'another-job') end it 'drops metadata' do @@ -3621,6 +3687,7 @@ describe Ci::Build do expect(build.reload).to be_degenerated expect(build.metadata).to be_nil + expect(build.needs).to be_empty end end diff --git a/spec/models/diff_note_spec.rb b/spec/models/diff_note_spec.rb index d9e1fe4b165..8d7dafc523d 100644 --- a/spec/models/diff_note_spec.rb +++ b/spec/models/diff_note_spec.rb @@ -34,6 +34,10 @@ describe DiffNote do subject { create(:diff_note_on_merge_request, project: project, position: position, noteable: merge_request) } + describe 'validations' do + it_behaves_like 'a valid diff positionable note', :diff_note_on_commit + end + describe "#position=" do context "when provided a string" do it "sets the position" do diff --git a/spec/models/namespace/aggregation_schedule_spec.rb b/spec/models/namespace/aggregation_schedule_spec.rb index 0f1283717e0..38bf8089411 100644 --- a/spec/models/namespace/aggregation_schedule_spec.rb +++ b/spec/models/namespace/aggregation_schedule_spec.rb @@ -7,24 +7,6 @@ RSpec.describe Namespace::AggregationSchedule, :clean_gitlab_redis_shared_state, it { is_expected.to belong_to :namespace } - describe '.delay_timeout' do - context 'when timeout is set on redis' do - it 'uses personalized timeout' do - Gitlab::Redis::SharedState.with do |redis| - redis.set(described_class::REDIS_SHARED_KEY, 1.hour) - end - - expect(described_class.delay_timeout).to eq(1.hour) - end - end - - context 'when timeout is not set on redis' do - it 'uses default timeout' do - expect(described_class.delay_timeout).to eq(3.hours) - end - end - end - describe '#schedule_root_storage_statistics' do let(:namespace) { create(:namespace) } let(:aggregation_schedule) { namespace.build_aggregation_schedule } @@ -87,21 +69,5 @@ RSpec.describe Namespace::AggregationSchedule, :clean_gitlab_redis_shared_state, aggregation_schedule.schedule_root_storage_statistics end end - - context 'with a personalized lease timeout' do - before do - Gitlab::Redis::SharedState.with do |redis| - redis.set(described_class::REDIS_SHARED_KEY, 1.hour) - end - end - - it 'uses a personalized time' do - expect(Namespaces::RootStatisticsWorker) - .to receive(:perform_in) - .with(1.hour, aggregation_schedule.namespace_id) - - aggregation_schedule.save! - end - end end end diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 7a6f1cd548c..15d6db42760 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -1571,7 +1571,7 @@ describe API::MergeRequests do end end - describe "GET /projects/:id/merge_requests/:merge_request_iid/merge_ref" do + describe "GET /projects/:id/merge_requests/:merge_request_iid/merge_ref", :clean_gitlab_redis_shared_state do before do merge_request.mark_as_unchecked! end diff --git a/spec/rubocop/cop/rspec/top_level_describe_path_spec.rb b/spec/rubocop/cop/rspec/top_level_describe_path_spec.rb new file mode 100644 index 00000000000..258144d4000 --- /dev/null +++ b/spec/rubocop/cop/rspec/top_level_describe_path_spec.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' + +require 'rubocop' +require 'rubocop/rspec/support' + +require_relative '../../../../rubocop/cop/rspec/top_level_describe_path' + +describe RuboCop::Cop::RSpec::TopLevelDescribePath do + include RuboCop::RSpec::ExpectOffense + include CopHelper + + subject(:cop) { described_class.new } + + context 'when the file ends in _spec.rb' do + it 'registers no offenses' do + expect_no_offenses(<<~SOURCE.strip_indent, 'spec/foo_spec.rb') + describe 'Foo' do + end + SOURCE + end + end + + context 'when the file is a frontend fixture' do + it 'registers no offenses' do + expect_no_offenses(<<~SOURCE.strip_indent, 'spec/frontend/fixtures/foo.rb') + describe 'Foo' do + end + SOURCE + end + end + + context 'when the describe is in a shared example' do + context 'with shared_examples' do + it 'registers no offenses' do + expect_no_offenses(<<~SOURCE.strip_indent, 'spec/foo.rb') + shared_examples 'Foo' do + describe '#bar' do + end + end + SOURCE + end + end + + context 'with shared_examples_for' do + it 'registers no offenses' do + expect_no_offenses(<<~SOURCE.strip_indent, 'spec/foo.rb') + shared_examples_for 'Foo' do + describe '#bar' do + end + end + SOURCE + end + end + end + + context 'when the describe is at the top level' do + it 'marks the describe as offending' do + expect_offense(<<~SOURCE.strip_indent, 'spec/foo.rb') + describe 'Foo' do + ^^^^^^^^^^^^^^ #{described_class::MESSAGE} + end + SOURCE + end + end +end diff --git a/spec/serializers/user_serializer_spec.rb b/spec/serializers/user_serializer_spec.rb new file mode 100644 index 00000000000..2e4a8c644fe --- /dev/null +++ b/spec/serializers/user_serializer_spec.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe UserSerializer do + let(:user1) { create(:user) } + let(:user2) { create(:user) } + + context 'serializer with merge request context' do + let(:merge_request) { create(:merge_request) } + let(:project) { merge_request.project } + let(:serializer) { described_class.new(merge_request_iid: merge_request.iid) } + + before do + allow(project).to( + receive_message_chain(:merge_requests, :find_by_iid!) + .with(merge_request.iid).and_return(merge_request) + ) + + project.add_maintainer(user1) + end + + it 'returns a user with can_merge option' do + serialized_user1, serialized_user2 = serializer.represent([user1, user2], project: project).as_json + + expect(serialized_user1).to include("id" => user1.id, "can_merge" => true) + expect(serialized_user2).to include("id" => user2.id, "can_merge" => false) + end + end +end diff --git a/spec/services/auth/container_registry_authentication_service_spec.rb b/spec/services/auth/container_registry_authentication_service_spec.rb index 4f4776bbb27..3ca389ba25b 100644 --- a/spec/services/auth/container_registry_authentication_service_spec.rb +++ b/spec/services/auth/container_registry_authentication_service_spec.rb @@ -145,6 +145,19 @@ describe Auth::ContainerRegistryAuthenticationService do it_behaves_like 'not a container repository factory' end + describe '#pull_access_token' do + let(:project) { create(:project) } + let(:token) { described_class.pull_access_token(project.full_path) } + + subject { { token: token } } + + it_behaves_like 'an accessible' do + let(:actions) { ['pull'] } + end + + it_behaves_like 'not a container repository factory' + end + context 'user authorization' do let(:current_user) { create(:user) } diff --git a/spec/services/ci/process_pipeline_service_spec.rb b/spec/services/ci/process_pipeline_service_spec.rb index cadb519ccee..77f108b6ab8 100644 --- a/spec/services/ci/process_pipeline_service_spec.rb +++ b/spec/services/ci/process_pipeline_service_spec.rb @@ -700,6 +700,94 @@ describe Ci::ProcessPipelineService, '#execute' do end end + context 'when pipeline with needs is created' do + let!(:linux_build) { create_build('linux:build', stage: 'build', stage_idx: 0) } + let!(:mac_build) { create_build('mac:build', stage: 'build', stage_idx: 0) } + let!(:linux_rspec) { create_build('linux:rspec', stage: 'test', stage_idx: 1) } + let!(:linux_rubocop) { create_build('linux:rubocop', stage: 'test', stage_idx: 1) } + let!(:mac_rspec) { create_build('mac:rspec', stage: 'test', stage_idx: 1) } + let!(:mac_rubocop) { create_build('mac:rubocop', stage: 'test', stage_idx: 1) } + let!(:deploy) { create_build('deploy', stage: 'deploy', stage_idx: 2) } + + let!(:linux_rspec_on_build) { create(:ci_build_need, build: linux_rspec, name: 'linux:build') } + let!(:linux_rubocop_on_build) { create(:ci_build_need, build: linux_rubocop, name: 'linux:build') } + + let!(:mac_rspec_on_build) { create(:ci_build_need, build: mac_rspec, name: 'mac:build') } + let!(:mac_rubocop_on_build) { create(:ci_build_need, build: mac_rubocop, name: 'mac:build') } + + it 'when linux:* finishes first it runs it out of order' do + expect(process_pipeline).to be_truthy + + expect(stages).to eq(%w(pending created created)) + expect(builds.pending).to contain_exactly(linux_build, mac_build) + + # we follow the single path of linux + linux_build.reset.success! + + expect(stages).to eq(%w(running pending created)) + expect(builds.success).to contain_exactly(linux_build) + expect(builds.pending).to contain_exactly(mac_build, linux_rspec, linux_rubocop) + + linux_rspec.reset.success! + + expect(stages).to eq(%w(running running created)) + expect(builds.success).to contain_exactly(linux_build, linux_rspec) + expect(builds.pending).to contain_exactly(mac_build, linux_rubocop) + + linux_rubocop.reset.success! + + expect(stages).to eq(%w(running running created)) + expect(builds.success).to contain_exactly(linux_build, linux_rspec, linux_rubocop) + expect(builds.pending).to contain_exactly(mac_build) + + mac_build.reset.success! + mac_rspec.reset.success! + mac_rubocop.reset.success! + + expect(stages).to eq(%w(success success pending)) + expect(builds.success).to contain_exactly( + linux_build, linux_rspec, linux_rubocop, mac_build, mac_rspec, mac_rubocop) + expect(builds.pending).to contain_exactly(deploy) + end + + context 'when feature ci_dag_support is disabled' do + before do + stub_feature_flags(ci_dag_support: false) + end + + it 'when linux:build finishes first it follows stages' do + expect(process_pipeline).to be_truthy + + expect(stages).to eq(%w(pending created created)) + expect(builds.pending).to contain_exactly(linux_build, mac_build) + + # we follow the single path of linux + linux_build.reset.success! + + expect(stages).to eq(%w(running created created)) + expect(builds.success).to contain_exactly(linux_build) + expect(builds.pending).to contain_exactly(mac_build) + + mac_build.reset.success! + + expect(stages).to eq(%w(success pending created)) + expect(builds.success).to contain_exactly(linux_build, mac_build) + expect(builds.pending).to contain_exactly( + linux_rspec, linux_rubocop, mac_rspec, mac_rubocop) + + linux_rspec.reset.success! + linux_rubocop.reset.success! + mac_rspec.reset.success! + mac_rubocop.reset.success! + + expect(stages).to eq(%w(success success pending)) + expect(builds.success).to contain_exactly( + linux_build, linux_rspec, linux_rubocop, mac_build, mac_rspec, mac_rubocop) + expect(builds.pending).to contain_exactly(deploy) + end + end + end + def process_pipeline described_class.new(pipeline.project, user).execute(pipeline) end @@ -712,6 +800,10 @@ describe Ci::ProcessPipelineService, '#execute' do all_builds.where.not(status: [:created, :skipped]) end + def stages + pipeline.reset.stages.map(&:status) + end + def builds_names builds.pluck(:name) end diff --git a/spec/services/ci/retry_build_service_spec.rb b/spec/services/ci/retry_build_service_spec.rb index 915288cd916..fe7c6fe4700 100644 --- a/spec/services/ci/retry_build_service_spec.rb +++ b/spec/services/ci/retry_build_service_spec.rb @@ -67,6 +67,7 @@ describe Ci::RetryBuildService do end create(:ci_job_variable, job: build) + create(:ci_build_need, build: build) build.reload end diff --git a/spec/services/merge_requests/mergeability_check_service_spec.rb b/spec/services/merge_requests/mergeability_check_service_spec.rb index 6e827f2ea5b..a864da0a6fb 100644 --- a/spec/services/merge_requests/mergeability_check_service_spec.rb +++ b/spec/services/merge_requests/mergeability_check_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe MergeRequests::MergeabilityCheckService do +describe MergeRequests::MergeabilityCheckService, :clean_gitlab_redis_shared_state do shared_examples_for 'unmergeable merge request' do it 'updates or keeps merge status as cannot_be_merged' do subject @@ -60,24 +60,69 @@ describe MergeRequests::MergeabilityCheckService do subject { described_class.new(merge_request).execute } + def execute_within_threads(amount:, retry_lease: true) + threads = [] + + amount.times do + # Let's use a different object for each thread to get closer + # to a real world scenario. + mr = MergeRequest.find(merge_request.id) + + threads << Thread.new do + described_class.new(mr).execute(retry_lease: retry_lease) + end + end + + threads.each(&:join) + threads + end + before do project.add_developer(merge_request.author) end it_behaves_like 'mergeable merge request' - context 'when multiple calls to the service' do - it 'returns success' do - subject - result = subject + context 'when lock is disabled' do + before do + stub_feature_flags(merge_ref_auto_sync_lock: false) + end - expect(result).to be_a(ServiceResponse) - expect(result.success?).to be(true) + it_behaves_like 'mergeable merge request' + end + + context 'when concurrent calls' do + it 'waits first lock and returns "cached" result in subsequent calls' do + threads = execute_within_threads(amount: 3) + results = threads.map { |t| t.value.status } + + expect(results).to contain_exactly(:success, :success, :success) + end + + it 'writes the merge-ref once' do + service = instance_double(MergeRequests::MergeToRefService) + + expect(MergeRequests::MergeToRefService).to receive(:new).once { service } + expect(service).to receive(:execute).once.and_return(success: true) + + execute_within_threads(amount: 3) end - it 'second call does not change the merge-ref' do - expect { subject }.to change(merge_request, :merge_ref_head).from(nil) - expect { subject }.not_to change(merge_request.merge_ref_head, :id) + it 'resets one merge request upon execution' do + expect_any_instance_of(MergeRequest).to receive(:reset).once + + execute_within_threads(amount: 2) + end + + context 'when retry_lease flag is false' do + it 'the first call succeeds, subsequent concurrent calls get a lock error response' do + threads = execute_within_threads(amount: 3, retry_lease: false) + results = threads.map { |t| [t.value.status, t.value.message] } + + expect(results).to contain_exactly([:error, 'Failed to obtain a lock'], + [:error, 'Failed to obtain a lock'], + [:success, nil]) + end end end @@ -102,8 +147,7 @@ describe MergeRequests::MergeabilityCheckService do context 'when broken' do before do - allow(merge_request).to receive(:broken?) { true } - allow(project.repository).to receive(:can_be_merged?) { false } + expect(merge_request).to receive(:broken?) { true } end it_behaves_like 'unmergeable merge request' @@ -117,10 +161,13 @@ describe MergeRequests::MergeabilityCheckService do end end - context 'when it has conflicts' do - before do - allow(merge_request).to receive(:broken?) { false } - allow(project.repository).to receive(:can_be_merged?) { false } + context 'when it cannot be merged on git' do + let(:merge_request) do + create(:merge_request, + merge_status: :unchecked, + source_branch: 'conflict-resolvable', + source_project: project, + target_branch: 'conflict-start') end it_behaves_like 'unmergeable merge request' diff --git a/spec/support/shared_examples/discussions_provider_shared_examples.rb b/spec/support/shared_examples/discussions_provider_shared_examples.rb new file mode 100644 index 00000000000..77cf1ac3f51 --- /dev/null +++ b/spec/support/shared_examples/discussions_provider_shared_examples.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +require 'spec_helper' + +shared_examples 'discussions provider' do + it 'returns the expected discussions' do + get :discussions, params: { namespace_id: project.namespace, project_id: project, id: requested_iid } + + expect(response).to have_gitlab_http_status(200) + expect(response).to match_response_schema('entities/discussions') + + expect(json_response.size).to eq(expected_discussion_count) + expect(json_response.pluck('id')).to eq(expected_discussion_ids) + end +end diff --git a/spec/support/shared_examples/models/diff_positionable_note_shared_examples.rb b/spec/support/shared_examples/models/diff_positionable_note_shared_examples.rb new file mode 100644 index 00000000000..8b298c5c974 --- /dev/null +++ b/spec/support/shared_examples/models/diff_positionable_note_shared_examples.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +shared_examples_for 'a valid diff positionable note' do |factory_on_commit| + context 'for commit' do + let(:project) { create(:project, :repository) } + let(:commit) { project.commit(sample_commit.id) } + let(:commit_id) { commit.id } + let(:diff_refs) { commit.diff_refs } + + let(:position) do + Gitlab::Diff::Position.new( + old_path: "files/ruby/popen.rb", + new_path: "files/ruby/popen.rb", + old_line: nil, + new_line: 14, + diff_refs: diff_refs + ) + end + + subject { build(factory_on_commit, commit_id: commit_id, position: position) } + + context 'position diff refs matches commit diff refs' do + it 'is valid' do + expect(subject).to be_valid + expect(subject.errors).not_to have_key(:commit_id) + end + end + + context 'position diff refs does not match commit diff refs' do + let(:diff_refs) do + Gitlab::Diff::DiffRefs.new( + base_sha: "not_existing_sha", + head_sha: "existing_sha" + ) + end + + it 'is invalid' do + expect(subject).to be_invalid + expect(subject.errors).to have_key(:commit_id) + end + end + + context 'commit does not exist' do + let(:commit_id) { 'non-existing' } + + it 'is invalid' do + expect(subject).to be_invalid + expect(subject.errors).to have_key(:commit_id) + end + end + end +end diff --git a/spec/support/shared_examples/relative_positioning_shared_examples.rb b/spec/support/shared_examples/relative_positioning_shared_examples.rb index 5ee62644c54..1c53e2602eb 100644 --- a/spec/support/shared_examples/relative_positioning_shared_examples.rb +++ b/spec/support/shared_examples/relative_positioning_shared_examples.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -RSpec.shared_examples "a class that supports relative positioning" do +RSpec.shared_examples 'a class that supports relative positioning' do let(:item1) { create(factory, default_params) } let(:item2) { create(factory, default_params) } let(:new_item) { create(factory, default_params) } @@ -9,24 +9,32 @@ RSpec.shared_examples "a class that supports relative positioning" do create(factory, params.merge(default_params)) end - describe '.move_to_end' do - it 'moves the object to the end' do - item1.update(relative_position: 5) - item2.update(relative_position: 15) + describe '.move_nulls_to_end' do + it 'moves items with null relative_position to the end' do + skip("#{item1} has a default relative position") if item1.relative_position + skip("#{item2} has a default relative position") if item2.relative_position - described_class.move_to_end([item1, item2]) + described_class.move_nulls_to_end([item1, item2]) expect(item2.prev_relative_position).to eq item1.relative_position expect(item1.prev_relative_position).to eq nil expect(item2.next_relative_position).to eq nil end + it 'moves the item near the start position when there are no existing positions' do + skip("#{item1} has a default relative position") if item1.relative_position + + described_class.move_nulls_to_end([item1]) + + expect(item1.relative_position).to eq(described_class::START_POSITION + described_class::IDEAL_DISTANCE) + end + it 'does not perform any moves if all items have their relative_position set' do item1.update!(relative_position: 1) expect(item1).not_to receive(:save) - described_class.move_to_end([item1]) + described_class.move_nulls_to_end([item1]) end end diff --git a/spec/tasks/gitlab/mail_google_schema_whitelisting.rb b/spec/tasks/gitlab/mail_google_schema_whitelisting.rb deleted file mode 100644 index 8d1cff7a261..00000000000 --- a/spec/tasks/gitlab/mail_google_schema_whitelisting.rb +++ /dev/null @@ -1,27 +0,0 @@ -require 'spec_helper' -require 'rake' - -describe 'gitlab:mail_google_schema_whitelisting rake task' do - before :all do - Rake.application.rake_require "tasks/gitlab/helpers" - Rake.application.rake_require "tasks/gitlab/mail_google_schema_whitelisting" - # empty task as env is already loaded - Rake::Task.define_task :environment - end - - describe 'call' do - before do - # avoid writing task output to spec progress - allow($stdout).to receive :write - end - - let :run_rake_task do - Rake::Task["gitlab:mail_google_schema_whitelisting"].reenable - Rake.application.invoke_task "gitlab:mail_google_schema_whitelisting" - end - - it 'runs the task without errors' do - expect { run_rake_task }.not_to raise_error - end - end -end diff --git a/spec/views/dashboard/projects/_blank_state_admin_welcome.haml.rb b/spec/views/dashboard/projects/_blank_state_admin_welcome.haml_spec.rb index 2f58eec86dc..2f58eec86dc 100644 --- a/spec/views/dashboard/projects/_blank_state_admin_welcome.haml.rb +++ b/spec/views/dashboard/projects/_blank_state_admin_welcome.haml_spec.rb diff --git a/spec/views/dashboard/projects/_nav.html.haml.rb b/spec/views/dashboard/projects/_nav.html.haml_spec.rb index f6a8ca13040..cbdd3c0acc3 100644 --- a/spec/views/dashboard/projects/_nav.html.haml.rb +++ b/spec/views/dashboard/projects/_nav.html.haml_spec.rb @@ -4,7 +4,7 @@ describe 'dashboard/projects/_nav.html.haml' do it 'highlights All tab by default' do render - expect(rendered).to have_css('li.active a', text: 'All') + expect(rendered).to have_css('a.active', text: 'All') end it 'highlights Personal tab personal param is present' do @@ -12,6 +12,6 @@ describe 'dashboard/projects/_nav.html.haml' do render - expect(rendered).to have_css('li.active a', text: 'Personal') + expect(rendered).to have_css('a.active', text: 'Personal') end end diff --git a/spec/views/projects/deployments/_confirm_rollback_modal_spec.html.rb b/spec/views/projects/deployments/_confirm_rollback_modal_spec.html_spec.rb index 54ec4f32856..54ec4f32856 100644 --- a/spec/views/projects/deployments/_confirm_rollback_modal_spec.html.rb +++ b/spec/views/projects/deployments/_confirm_rollback_modal_spec.html_spec.rb diff --git a/spec/views/shared/_label_row.html.haml.rb b/spec/views/shared/_label_row.html.haml_spec.rb index a58d5efc1e3..4cce13aa37c 100644 --- a/spec/views/shared/_label_row.html.haml.rb +++ b/spec/views/shared/_label_row.html.haml_spec.rb @@ -7,9 +7,20 @@ describe 'shared/_label_row.html.haml' do } label_types.each do |label_type, label_factory| - let!(:label) { create(label_factory) } + let!(:label) do + label_record = create(label_factory) + label_record.present(issuable_subject: label_record.subject) + end context "for a #{label_type}" do + before do + if label.project_label? + @project = label.project + else + @group = label.group + end + end + it 'has a non-linked label title' do render 'shared/label_row', label: label diff --git a/spec/views/shared/milestones/_issuable.html.haml.rb b/spec/views/shared/milestones/_issuable.html.haml_spec.rb index 0a3f877cae0..0a3f877cae0 100644 --- a/spec/views/shared/milestones/_issuable.html.haml.rb +++ b/spec/views/shared/milestones/_issuable.html.haml_spec.rb diff --git a/spec/views/shared/milestones/_issuables.html.haml.rb b/spec/views/shared/milestones/_issuables.html.haml_spec.rb index cbbb984935f..24b55338db3 100644 --- a/spec/views/shared/milestones/_issuables.html.haml.rb +++ b/spec/views/shared/milestones/_issuables.html.haml_spec.rb @@ -6,7 +6,7 @@ describe 'shared/milestones/_issuables.html.haml' do before do allow(view).to receive_messages(title: nil, id: nil, show_project_name: nil, show_full_project_name: nil, dom_class: '', - issuables: double(size: issuables_size).as_null_object) + issuables: double(length: issuables_size).as_null_object) stub_template 'shared/milestones/_issuable.html.haml' => '' end diff --git a/spec/views/shared/milestones/_top.html.haml.rb b/spec/views/shared/milestones/_top.html.haml_spec.rb index 516d81c87ac..f2ee8be5857 100644 --- a/spec/views/shared/milestones/_top.html.haml.rb +++ b/spec/views/shared/milestones/_top.html.haml_spec.rb @@ -7,6 +7,7 @@ describe 'shared/milestones/_top.html.haml' do before do allow(milestone).to receive(:milestones) { [] } + allow(milestone).to receive(:milestone) { milestone } end it 'renders a deprecation message for a legacy milestone' do diff --git a/spec/workers/build_process_worker_spec.rb b/spec/workers/build_process_worker_spec.rb new file mode 100644 index 00000000000..cceca40717c --- /dev/null +++ b/spec/workers/build_process_worker_spec.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe BuildProcessWorker do + describe '#perform' do + context 'when build exists' do + let(:pipeline) { create(:ci_pipeline) } + let(:build) { create(:ci_build, pipeline: pipeline) } + + it 'processes build' do + expect_any_instance_of(Ci::Pipeline).to receive(:process!) + .with(build.name) + + described_class.new.perform(build.id) + end + end + + context 'when build does not exist' do + it 'does not raise exception' do + expect { described_class.new.perform(123) } + .not_to raise_error + end + end + end +end diff --git a/vendor/jupyter/values.yaml b/vendor/jupyter/values.yaml index 2aadd3dbe1e..852c6c3f2b0 100644 --- a/vendor/jupyter/values.yaml +++ b/vendor/jupyter/values.yaml @@ -57,3 +57,7 @@ ingress: annotations: kubernetes.io/ingress.class: "nginx" kubernetes.io/tls-acme: "true" + +proxy: + service: + type: ClusterIP diff --git a/yarn.lock b/yarn.lock index 6643b011dab..221ffa27f6c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4310,11 +4310,11 @@ ecc-jsbn@~0.1.1: safer-buffer "^2.1.0" echarts@^4.2.0-rc.2: - version "4.2.0-rc.2" - resolved "https://registry.yarnpkg.com/echarts/-/echarts-4.2.0-rc.2.tgz#6a98397aafa81b65cbf0bc15d9afdbfb244df91e" - integrity sha512-5Y4Kyi4eNsRM9Cnl7Q8C6PFVjznBJv1VIiMm/VSQ9zyqeo+ce1695GqUd9v4zfVx+Ow1gnwMJX67h0FNvarScw== + version "4.2.1" + resolved "https://registry.yarnpkg.com/echarts/-/echarts-4.2.1.tgz#9a8ea3b03354f86f824d97625c334cf16965ef03" + integrity sha512-pw4xScRPsLegD/cqEcoXRKeA2SD4+s+Kyo0Na166NamOWhzNl2yI5RZ2rE97tBlAopNmhyMeBVpAeD5qb+ee1A== dependencies: - zrender "4.0.5" + zrender "4.0.7" editions@^1.3.3: version "1.3.4" @@ -8371,7 +8371,7 @@ monaco-editor@^0.15.6: resolved "https://registry.yarnpkg.com/monaco-editor/-/monaco-editor-0.15.6.tgz#d63b3b06f86f803464f003b252627c3eb4a09483" integrity sha512-JoU9V9k6KqT9R9Tiw1RTU8ohZ+Xnf9DMg6Ktqqw5hILumwmq7xqa/KLXw513uTUsWbhtnHoSJYYR++u3pkyxJg== -mousetrap@1.4.6: +mousetrap@^1.4.6: version "1.4.6" resolved "https://registry.yarnpkg.com/mousetrap/-/mousetrap-1.4.6.tgz#eaca72e22e56d5b769b7555873b688c3332e390a" integrity sha1-6spy4i5W1bdpt1VYc7aIwzMuOQo= @@ -13217,7 +13217,7 @@ zen-observable@^0.8.0: resolved "https://registry.yarnpkg.com/zen-observable/-/zen-observable-0.8.11.tgz#d3415885eeeb42ee5abb9821c95bb518fcd6d199" integrity sha512-N3xXQVr4L61rZvGMpWe8XoCGX8vhU35dPyQ4fm5CY/KDlG0F75un14hjbckPXTDuKUY6V0dqR2giT6xN8Y4GEQ== -zrender@4.0.5: - version "4.0.5" - resolved "https://registry.yarnpkg.com/zrender/-/zrender-4.0.5.tgz#6e8f738971ce2cd624aac82b2156729b1c0e5a82" - integrity sha512-SintgipGEJPT9Sz2ABRoE4ZD7Yzy7oR7j7KP6H+C9FlbHWnLUfGVK7E8UV27pGwlxAMB0EsnrqhXx5XjAfv/KA== +zrender@4.0.7: + version "4.0.7" + resolved "https://registry.yarnpkg.com/zrender/-/zrender-4.0.7.tgz#15ae960822f5efed410995d37e5107fe3de10e6d" + integrity sha512-TNloHe0ums6zxbHfnaCryM61J4IWDajZwNq6dHk9vfWhhysO/OeFvvR0drBs/nbXha2YxSzfQj2FiCd6RVBe+Q== |
