diff options
80 files changed, 1951 insertions, 572 deletions
diff --git a/app/assets/javascripts/commons/polyfills.js b/app/assets/javascripts/commons/polyfills.js index 7a6ad3dc771..daa941a63cd 100644 --- a/app/assets/javascripts/commons/polyfills.js +++ b/app/assets/javascripts/commons/polyfills.js @@ -12,6 +12,7 @@ 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 a66555838ba..b98fe9f6ce2 100644 --- a/app/assets/javascripts/gl_form.js +++ b/app/assets/javascripts/gl_form.js @@ -3,9 +3,16 @@ 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); @@ -16,6 +23,10 @@ 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 @@ -85,9 +96,84 @@ 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) @@ -99,5 +185,6 @@ 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 new file mode 100644 index 00000000000..a8815fac04e --- /dev/null +++ b/app/assets/javascripts/helpers/indent_helper.js @@ -0,0 +1,182 @@ +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/lib/utils/common_utils.js b/app/assets/javascripts/lib/utils/common_utils.js index 5e90893b684..1a94aee2398 100644 --- a/app/assets/javascripts/lib/utils/common_utils.js +++ b/app/assets/javascripts/lib/utils/common_utils.js @@ -203,6 +203,71 @@ 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 5e0f9b612a2..e24fcf47d71 100644 --- a/app/assets/javascripts/lib/utils/keycodes.js +++ b/app/assets/javascripts/lib/utils/keycodes.js @@ -1,4 +1,10 @@ -export const UP_KEY_CODE = 38; -export const DOWN_KEY_CODE = 40; +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; diff --git a/app/assets/javascripts/lib/utils/undo_stack.js b/app/assets/javascripts/lib/utils/undo_stack.js new file mode 100644 index 00000000000..6cfdc2a0a0f --- /dev/null +++ b/app/assets/javascripts/lib/utils/undo_stack.js @@ -0,0 +1,105 @@ +/** + * 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 8ce5b615795..21c44b59520 100644 --- a/app/assets/javascripts/vue_shared/components/markdown/toolbar.vue +++ b/app/assets/javascripts/vue_shared/components/markdown/toolbar.vue @@ -1,5 +1,6 @@ <script> import { GlLink } from '@gitlab/ui'; +import { s__, sprintf } from '~/locale'; export default { components: { @@ -22,8 +23,28 @@ export default { }, }, computed: { - hasQuickActionsDocsPath() { - return this.quickActionsDocsPath !== ''; + 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; }, }, }; @@ -32,21 +53,7 @@ export default { <template> <div class="comment-toolbar clearfix"> <div class="toolbar-text"> - <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> + <span v-html="toolbarHelpHtml"></span> </div> <span v-if="canAttachFile" class="uploading-container"> <span class="uploading-progress-container hide"> diff --git a/app/controllers/concerns/authenticates_with_two_factor.rb b/app/controllers/concerns/authenticates_with_two_factor.rb index 4926062f9ca..8c8f0b3a22e 100644 --- a/app/controllers/concerns/authenticates_with_two_factor.rb +++ b/app/controllers/concerns/authenticates_with_two_factor.rb @@ -55,7 +55,7 @@ module AuthenticatesWithTwoFactor remember_me(user) if user_params[:remember_me] == '1' user.save! - sign_in(user, message: :two_factor_authenticated) + sign_in(user, message: :two_factor_authenticated, event: :authentication) else user.increment_failed_attempts! Gitlab::AppLogger.info("Failed Login: user=#{user.username} ip=#{request.remote_ip} method=OTP") @@ -72,7 +72,7 @@ module AuthenticatesWithTwoFactor session.delete(:challenge) remember_me(user) if user_params[:remember_me] == '1' - sign_in(user, message: :two_factor_authenticated) + sign_in(user, message: :two_factor_authenticated, event: :authentication) else user.increment_failed_attempts! Gitlab::AppLogger.info("Failed Login: user=#{user.username} ip=#{request.remote_ip} method=U2F") diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index 2a8dd997d04..b1efa767154 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -139,7 +139,7 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController if user.two_factor_enabled? && !auth_user.bypass_two_factor? prompt_for_two_factor(user) else - sign_in_and_redirect(user) + sign_in_and_redirect(user, event: :authentication) end else fail_login(user) diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 7604b31467a..1880bead3ee 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -26,6 +26,17 @@ class SessionsController < Devise::SessionsController after_action :log_failed_login, if: -> { action_name == 'new' && failed_login? } helper_method :captcha_enabled? + # protect_from_forgery is already prepended in ApplicationController but + # authenticate_with_two_factor which signs in the user is prepended before + # that here. + # We need to make sure CSRF token is verified before authenticating the user + # because Devise.clean_up_csrf_token_on_authentication is set to true by + # default to avoid CSRF token fixation attacks. Authenticating the user first + # would cause the CSRF token to be cleared and then + # RequestForgeryProtection#verify_authenticity_token would fail because of + # token mismatch. + protect_from_forgery with: :exception, prepend: true + CAPTCHA_HEADER = 'X-GitLab-Show-Login-Captcha'.freeze def new diff --git a/app/mailers/emails/members.rb b/app/mailers/emails/members.rb index 2bfa59774d7..76fa7236ab1 100644 --- a/app/mailers/emails/members.rb +++ b/app/mailers/emails/members.rb @@ -9,11 +9,11 @@ module Emails helper_method :member_source, :member end - def member_access_requested_email(member_source_type, member_id, recipient_notification_email) + def member_access_requested_email(member_source_type, member_id, recipient_id) @member_source_type = member_source_type @member_id = member_id - mail(to: recipient_notification_email, + mail(to: recipient(recipient_id, notification_group), subject: subject("Request to join the #{member_source.human_name} #{member_source.model_name.singular}")) end @@ -21,16 +21,15 @@ module Emails @member_source_type = member_source_type @member_id = member_id - mail(to: member.user.notification_email, + mail(to: recipient(member.user, notification_group), subject: subject("Access to the #{member_source.human_name} #{member_source.model_name.singular} was granted")) end def member_access_denied_email(member_source_type, source_id, user_id) @member_source_type = member_source_type @member_source = member_source_class.find(source_id) - requester = User.find(user_id) - mail(to: requester.notification_email, + mail(to: recipient(user_id, notification_group), subject: subject("Access to the #{member_source.human_name} #{member_source.model_name.singular} was denied")) end @@ -48,7 +47,7 @@ module Emails @member_id = member_id return unless member.created_by - mail(to: member.created_by.notification_email, + mail(to: recipient(member.created_by, notification_group), subject: subject('Invitation accepted')) end @@ -59,7 +58,7 @@ module Emails @member_source = member_source_class.find(source_id) @invite_email = invite_email - mail(to: recipient(created_by_id, member_source_type == 'Project' ? @member_source.group : @member_source), + mail(to: recipient(created_by_id, notification_group), subject: subject('Invitation declined')) end @@ -71,6 +70,10 @@ module Emails @member_source ||= member.source end + def notification_group + @member_source_type.casecmp?('project') ? member_source.group : member_source + end + private def member_source_class diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb index 8ef20a03541..5d292094a05 100644 --- a/app/mailers/notify.rb +++ b/app/mailers/notify.rb @@ -71,14 +71,18 @@ class Notify < BaseMailer address.format end - # Look up a User by their ID and return their email address + # Look up a User's notification email for a particular context. + # Can look up by their ID or can accept a User object. # - # recipient_id - User ID + # recipient - User object OR a User ID # notification_group - The parent group of the notification # # Returns a String containing the User's email address. - def recipient(recipient_id, notification_group = nil) - User.find(recipient_id).notification_email_for(notification_group) + def recipient(recipient, notification_group = nil) + user = recipient if recipient.is_a?(User) + user ||= User.find(recipient) + + user.notification_email_for(notification_group) end # Formats arguments into a String suitable for use as an email subject diff --git a/app/mailers/previews/notify_preview.rb b/app/mailers/previews/notify_preview.rb index 80e0a17c312..b3fab930922 100644 --- a/app/mailers/previews/notify_preview.rb +++ b/app/mailers/previews/notify_preview.rb @@ -105,11 +105,11 @@ class NotifyPreview < ActionMailer::Preview end def member_access_granted_email - Notify.member_access_granted_email('project', user.id).message + Notify.member_access_granted_email(member.source_type, member.id).message end def member_access_requested_email - Notify.member_access_requested_email('group', user.id, 'some@example.com').message + Notify.member_access_requested_email('group', user.id, user.id).message end def member_invite_accepted_email @@ -183,6 +183,10 @@ class NotifyPreview < ActionMailer::Preview @user ||= User.last end + def member + @member ||= Member.last + end + def create_note(params) Notes::CreateService.new(project, user, params).execute end diff --git a/app/models/concerns/protected_ref.rb b/app/models/concerns/protected_ref.rb index af387c99f3d..0648b4a78e1 100644 --- a/app/models/concerns/protected_ref.rb +++ b/app/models/concerns/protected_ref.rb @@ -47,7 +47,7 @@ module ProtectedRef def access_levels_for_ref(ref, action:, protected_refs: nil) self.matching(ref, protected_refs: protected_refs) - .map(&:"#{action}_access_levels").flatten + .flat_map(&:"#{action}_access_levels") end # Returns all protected refs that match the given ref name. diff --git a/app/models/group.rb b/app/models/group.rb index 26ce2957e9b..65a6705b6c0 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -388,7 +388,7 @@ class Group < Namespace variables = Ci::GroupVariable.where(group: list_of_ids) variables = variables.unprotected unless project.protected_for?(ref) variables = variables.group_by(&:group_id) - list_of_ids.reverse.map { |group| variables[group.id] }.compact.flatten + list_of_ids.reverse.flat_map { |group| variables[group.id] }.compact end def group_member(user) diff --git a/app/serializers/stage_entity.rb b/app/serializers/stage_entity.rb index 029dd3d0684..0b0454c5282 100644 --- a/app/serializers/stage_entity.rb +++ b/app/serializers/stage_entity.rb @@ -59,14 +59,14 @@ class StageEntity < Grape::Entity end def latest_statuses - HasStatus::ORDERED_STATUSES.map do |ordered_status| + HasStatus::ORDERED_STATUSES.flat_map do |ordered_status| grouped_statuses.fetch(ordered_status, []) - end.flatten + end end def retried_statuses - HasStatus::ORDERED_STATUSES.map do |ordered_status| + HasStatus::ORDERED_STATUSES.flat_map do |ordered_status| grouped_retried_statuses.fetch(ordered_status, []) - end.flatten + end end end diff --git a/app/services/ci/process_pipeline_service.rb b/app/services/ci/process_pipeline_service.rb index aaf56048b5c..207cc5017d0 100644 --- a/app/services/ci/process_pipeline_service.rb +++ b/app/services/ci/process_pipeline_service.rb @@ -10,13 +10,13 @@ module Ci update_retried new_builds = - stage_indexes_of_created_processables.map do |index| + stage_indexes_of_created_processables.flat_map do |index| process_stage(index) end @pipeline.update_status - new_builds.flatten.any? + new_builds.any? end private diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index a55771ed538..21fab22e0d4 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -595,7 +595,7 @@ class NotificationService end def deliver_access_request_email(recipient, member) - mailer.member_access_requested_email(member.real_source_type, member.id, recipient.user.notification_email).deliver_later + mailer.member_access_requested_email(member.real_source_type, member.id, recipient.user.id).deliver_later end def fallback_to_group_owners_maintainers?(recipients, member) diff --git a/app/views/shared/notes/_hints.html.haml b/app/views/shared/notes/_hints.html.haml index fae7d6526e8..72ede50dd8c 100644 --- a/app/views/shared/notes/_hints.html.haml +++ b/app/views/shared/notes/_hints.html.haml @@ -1,14 +1,13 @@ - supports_quick_actions = local_assigns.fetch(:supports_quick_actions, false) .comment-toolbar.clearfix .toolbar-text - = link_to _('Markdown'), help_page_path('user/markdown'), target: '_blank', tabindex: -1 + - 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 - if supports_quick_actions - and - = link_to _('quick actions'), help_page_path('user/project/quick_actions'), target: '_blank', tabindex: -1 - are + = 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 } - else - is - supported + = s_('Editor|%{mdLinkStart}Markdown is supported%{mdLinkEnd}').html_safe % { mdLinkStart: md_link_start, mdLinkEnd: link_end } %span.uploading-container %span.uploading-progress-container.hide diff --git a/app/views/shared/projects/_project.html.haml b/app/views/shared/projects/_project.html.haml index c4d1bdad2c4..f40a9cffb29 100644 --- a/app/views/shared/projects/_project.html.haml +++ b/app/views/shared/projects/_project.html.haml @@ -89,4 +89,6 @@ %span.icon-wrapper.pipeline-status = render 'ci/status/icon', status: project.commit.last_pipeline.detailed_status(current_user), type: 'commit', tooltip_placement: 'top', path: pipeline_path .updated-note - %span #{_('Updated')} #{updated_tooltip} + %span + = _('Updated') + = updated_tooltip diff --git a/changelogs/unreleased/63568-access-email-notifications-custom-email.yml b/changelogs/unreleased/63568-access-email-notifications-custom-email.yml new file mode 100644 index 00000000000..ece6442d7cf --- /dev/null +++ b/changelogs/unreleased/63568-access-email-notifications-custom-email.yml @@ -0,0 +1,5 @@ +--- +title: Respect group notification email when sending group access notifications +merge_request: 31089 +author: +type: fixed diff --git a/changelogs/unreleased/64257-warden_set_user_fix.yml b/changelogs/unreleased/64257-warden_set_user_fix.yml new file mode 100644 index 00000000000..7b6818876fb --- /dev/null +++ b/changelogs/unreleased/64257-warden_set_user_fix.yml @@ -0,0 +1,5 @@ +--- +title: Ensure Warden triggers after_authentication callback +merge_request: 31138 +author: +type: fixed diff --git a/changelogs/unreleased/65088-incorrect-message-interpolation-on-project-listing.yml b/changelogs/unreleased/65088-incorrect-message-interpolation-on-project-listing.yml new file mode 100644 index 00000000000..dd74b8443bc --- /dev/null +++ b/changelogs/unreleased/65088-incorrect-message-interpolation-on-project-listing.yml @@ -0,0 +1,5 @@ +--- +title: Fix incorrect use of message interpolation +merge_request: 31121 +author: +type: fixed diff --git a/changelogs/unreleased/delete-designs-v2.yml b/changelogs/unreleased/delete-designs-v2.yml new file mode 100644 index 00000000000..a678e4f93b9 --- /dev/null +++ b/changelogs/unreleased/delete-designs-v2.yml @@ -0,0 +1,4 @@ +--- +title: Adds event enum column to DesignsVersions join table +merge_request: 30745 +type: added diff --git a/changelogs/unreleased/extract_auto_deploy_into_base_image.yml b/changelogs/unreleased/extract_auto_deploy_into_base_image.yml new file mode 100644 index 00000000000..ff0d1f3bd71 --- /dev/null +++ b/changelogs/unreleased/extract_auto_deploy_into_base_image.yml @@ -0,0 +1,5 @@ +--- +title: Extract Auto DevOps deploy functions into a base image +merge_request: 30404 +author: +type: changed diff --git a/changelogs/unreleased/mh-editor-indents.yml b/changelogs/unreleased/mh-editor-indents.yml new file mode 100644 index 00000000000..a282c0f505d --- /dev/null +++ b/changelogs/unreleased/mh-editor-indents.yml @@ -0,0 +1,5 @@ +--- +title: Markdown editors now have indentation shortcuts and auto-indentation +merge_request: 28914 +author: +type: added diff --git a/db/migrate/20190715140740_add_event_type_to_design_management_designs_versions.rb b/db/migrate/20190715140740_add_event_type_to_design_management_designs_versions.rb new file mode 100644 index 00000000000..81a8b0a3271 --- /dev/null +++ b/db/migrate/20190715140740_add_event_type_to_design_management_designs_versions.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +# This migration sets up a event enum on the DesignsVersions join table +class AddEventTypeToDesignManagementDesignsVersions < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + # We disable these cops here because adding this column is safe. The table does not + # have any data in it. + # rubocop: disable Migration/AddIndex + # rubocop: disable Migration/AddColumn + def up + add_column(:design_management_designs_versions, :event, :integer, + limit: 2, + null: false, + default: 0) + add_index(:design_management_designs_versions, :event) + end + + # rubocop: disable Migration/RemoveIndex + def down + remove_index(:design_management_designs_versions, :event) + remove_column(:design_management_designs_versions, :event) + end +end diff --git a/db/schema.rb b/db/schema.rb index dbfc5959d9d..67479937b47 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1097,8 +1097,10 @@ ActiveRecord::Schema.define(version: 2019_07_25_012225) do create_table "design_management_designs_versions", id: false, force: :cascade do |t| t.bigint "design_id", null: false t.bigint "version_id", null: false + t.integer "event", limit: 2, default: 0, null: false t.index ["design_id", "version_id"], name: "design_management_designs_versions_uniqueness", unique: true t.index ["design_id"], name: "index_design_management_designs_versions_on_design_id" + t.index ["event"], name: "index_design_management_designs_versions_on_event" t.index ["version_id"], name: "index_design_management_designs_versions_on_version_id" end diff --git a/doc/ci/README.md b/doc/ci/README.md index 7048ceaac41..6923d07bb1d 100644 --- a/doc/ci/README.md +++ b/doc/ci/README.md @@ -13,6 +13,11 @@ through the [continuous methodologies](introduction/index.md#introduction-to-cic - Continuous Delivery (CD) - Continuous Deployment (CD) +NOTE: **Out-of-the-box management systems can decrease hours spent on maintaining toolchains by 10% or more.** +Watch our +["Mastering continuous software development"](https://about.gitlab.com/webcast/mastering-ci-cd/) +webcast to learn about continuous methods and how GitLab’s built-in CI can help you simplify and scale software development. + ## Overview Continuous Integration works by pushing small code chunks to your @@ -155,6 +160,7 @@ for your CI/CD infrastructure: - [Why we chose GitLab CI for our CI/CD solution](https://about.gitlab.com/2016/10/17/gitlab-ci-oohlala/) - [Building our web-app on GitLab CI](https://about.gitlab.com/2016/07/22/building-our-web-app-on-gitlab-ci/) +- [5 Teams that made the switch to GitLab CI/CD](https://about.gitlab.com/2019/04/25/5-teams-that-made-the-switch-to-gitlab-ci-cd/) See also the [Why CI/CD?](https://docs.google.com/presentation/d/1OGgk2Tcxbpl7DJaIOzCX4Vqg3dlwfELC3u2jEeCBbDk) presentation. diff --git a/doc/ci/introduction/index.md b/doc/ci/introduction/index.md index 6401ff90a0a..366aca3442e 100644 --- a/doc/ci/introduction/index.md +++ b/doc/ci/introduction/index.md @@ -9,6 +9,11 @@ In this document we'll present an overview of the concepts of Continuous Integra Continuous Delivery, and Continuous Deployment, as well as an introduction to GitLab CI/CD. +NOTE: **Out-of-the-box management systems can decrease hours spent on maintaining toolchains by 10% or more.** +Watch our +["Mastering continuous software development"](https://about.gitlab.com/webcast/mastering-ci-cd/) +webcast to learn about continuous methods and how GitLab’s built-in CI can help you simplify and scale software development. + ## Introduction to CI/CD methodologies The continuous methodologies of software development are based on diff --git a/doc/ci/merge_request_pipelines/pipelines_for_merged_results/index.md b/doc/ci/merge_request_pipelines/pipelines_for_merged_results/index.md index f63b17a9e5a..ad07c662965 100644 --- a/doc/ci/merge_request_pipelines/pipelines_for_merged_results/index.md +++ b/doc/ci/merge_request_pipelines/pipelines_for_merged_results/index.md @@ -62,18 +62,32 @@ CAUTION: **Warning:** Make sure your `gitlab-ci.yml` file is [configured properly for pipelines for merge requests](../index.md#configuring-pipelines-for-merge-requests), otherwise pipelines for merged results won't run and your merge requests will be stuck in an unresolved state. -## Merge Trains **(PREMIUM)** +## Troubleshooting -Read the [documentation on Merge Trains](merge_trains/index.md). +### Pipelines for merged results not created even with new change pushed to merge request -<!-- ## Troubleshooting +Can be caused by some disabled feature flags. Please make sure that +the following feature flags are enabled on your GitLab instance: -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. +- `:ci_use_merge_request_ref` +- `:merge_ref_auto_sync` -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. --> +To check these feature flag values, please ask administrator to execute the following commands: + +```shell +> sudo gitlab-rails console # Login to Rails console of GitLab instance. +> Feature.enabled?(:ci_use_merge_request_ref) # Check if it's enabled or not. +> Feature.enable(:ci_use_merge_request_ref) # Enable the feature flag. +``` + +## Using Merge Trains **(PREMIUM)** + +By enabling [Pipelines for merged results](#pipelines-for-merged-results-premium), +GitLab will [automatically display](merge_trains/index.md#how-to-add-a-merge-request-to-a-merge-train) +a **Start/Add Merge Train button** as the most recommended merge strategy. + +Generally, this is a safer option than merging merge requests immediately as your +merge request will be evaluated with an expected post-merge result before the actual +merge happens. + +For more information, read the [documentation on Merge Trains](merge_trains/index.md). diff --git a/doc/ci/merge_request_pipelines/pipelines_for_merged_results/merge_trains/img/merge_train_failure.png b/doc/ci/merge_request_pipelines/pipelines_for_merged_results/merge_trains/img/merge_train_failure.png Binary files differnew file mode 100644 index 00000000000..a8916e5721c --- /dev/null +++ b/doc/ci/merge_request_pipelines/pipelines_for_merged_results/merge_trains/img/merge_train_failure.png diff --git a/doc/ci/merge_request_pipelines/pipelines_for_merged_results/merge_trains/img/merge_train_immediate_merge.png b/doc/ci/merge_request_pipelines/pipelines_for_merged_results/merge_trains/img/merge_train_immediate_merge.png Binary files differnew file mode 100644 index 00000000000..65ff7e3d674 --- /dev/null +++ b/doc/ci/merge_request_pipelines/pipelines_for_merged_results/merge_trains/img/merge_train_immediate_merge.png diff --git a/doc/ci/merge_request_pipelines/pipelines_for_merged_results/merge_trains/index.md b/doc/ci/merge_request_pipelines/pipelines_for_merged_results/merge_trains/index.md index 44cbcde264c..80a1c264bc4 100644 --- a/doc/ci/merge_request_pipelines/pipelines_for_merged_results/merge_trains/index.md +++ b/doc/ci/merge_request_pipelines/pipelines_for_merged_results/merge_trains/index.md @@ -80,14 +80,65 @@ button while the latest pipeline is running.  -<!-- ## Troubleshooting +## Immediately merge a merge request with a merge train -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. +In case, you have a high-priority merge request (e.g. critical patch) to be merged urgently, +you can use **Merge Immediately** option for bypassing the merge train. +This is the fastest option to get the change merged into the target branch. -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. --> + + +However, every time you merge a merge request immediately, it could affect the +existing merge train to be reconstructed, specifically, it regenerates expected +merge commits and pipelines. This means, merging immediately essentially wastes +CI resources. + +## Troubleshooting + +### Merge request dropped from the merge train immediately + +If a merge request is not mergeable (for example, it's WIP, there is a merge +conflict, etc), your merge request will be dropped from the merge train automatically. + +In these cases, the reason for dropping the merge request is in the **system notes**. + +To check the reason: + +1. Open the merge request that was dropped from the merge train. +1. Open the **Discussion** tab. +1. Find a system note that includes either: + - The text **... removed this merge request from the merge train because ...** + - **... aborted this merge request from the merge train because ...** + The reason is given in the text after the **because ...** phrase. + + + +### Merge When Pipeline Succeeds cannot be chosen + +[Merge When Pipeline Succeeds](../../../../user/project/merge_requests/merge_when_pipeline_succeeds.md) +is unavailable when +[Pipelines for Merged Results is enabled](../index.md#enabling-pipelines-for-merged-results). + +Follow [this issue](https://gitlab.com/gitlab-org/gitlab-ee/issues/12267) to +track progress on this issue. + +### Merge Train disturbs your workflow + +First of all, please check if [merge immediately](#immediately-merge-a-merge-request-with-a-merge-train) +is available as a workaround in your workflow. This is the most recommended +workaround you'd be able to take immediately. If it's not available or acceptable, +please read through this section. + +Merge train is enabled by default when you enable [Pipelines for merged results](../index.md), +however, you can forcibly disable this feature by disabling the feature flag `:merge_trains_enabled`. +After you disabled this feature, all the existing merge trains will be aborted and +you will no longer see the **Start/Add Merge Train** button in merge requests. + +To check if the feature flag is enabled on your GitLab instance, +please ask administrator to execute the following commands: + +```shell +> sudo gitlab-rails console # Login to Rails console of GitLab instance. +> Feature.enabled?(:merge_trains_enabled) # Check if it's enabled or not. +> Feature.disable(:merge_trains_enabled) # Disable the feature flag. +```
\ No newline at end of file diff --git a/doc/ci/pipelines.md b/doc/ci/pipelines.md index 98f30350968..be8f66c741f 100644 --- a/doc/ci/pipelines.md +++ b/doc/ci/pipelines.md @@ -6,6 +6,11 @@ type: reference > Introduced in GitLab 8.8. +NOTE: **Tip:** +Watch our +["Mastering continuous software development"](https://about.gitlab.com/webcast/mastering-ci-cd/) +webcast to see a comprehensive demo of GitLab CI/CD pipeline. + ## Introduction Pipelines are the top-level component of continuous integration, delivery, and deployment. diff --git a/doc/development/README.md b/doc/development/README.md index ea5d9e10e2c..99c88146be5 100644 --- a/doc/development/README.md +++ b/doc/development/README.md @@ -113,6 +113,10 @@ description: 'Learn how to contribute to GitLab.' - [Database helper modules](database_helpers.md) - [Code comments](code_comments.md) +## Case studies + +- [Database case study: Filtering by label](filtering_by_label.md) + ## Integration guides - [Jira Connect app](integrations/jira_connect.md) diff --git a/doc/development/database_review.md b/doc/development/database_review.md index 1413c2f69fb..3d10a0c84e5 100644 --- a/doc/development/database_review.md +++ b/doc/development/database_review.md @@ -68,6 +68,17 @@ make sure you have applied the ~database label and rerun the `danger-review` CI job, or pick someone from the [`@gl-database` team](https://gitlab.com/groups/gl-database/-/group_members). +### How to prepare for speedy database reviews + +In order to make reviewing easier and therefore faster, please consider preparing a comment +and details for a database reviewer: + +- Provide queries in SQL form rather than ActiveRecord. +- Format any queries with a SQL query formatter, for example with [sqlformat.darold.net](http://sqlformat.darold.net). +- Consider providing query plans via a link to [explain.depesz.com](https://explain.depesz.com) or another tool instead of textual form. +- For query changes, it is best to provide the SQL query along with a plan *before* and *after* the change. This helps to spot differences quickly. +- When providing query plans, make sure to use good parameter values, so that the query executed is a good example and also hits enough data. Usually, the `gitlab-org` namespace (`namespace_id = 9970`) and the `gitlab-org/gitlab-ce` project (`project_id = 13083`) provides enough data to serve as a good example. + ### How to review for database - Check migrations diff --git a/doc/development/documentation/styleguide.md b/doc/development/documentation/styleguide.md index 36ffc02644e..dd798777c12 100644 --- a/doc/development/documentation/styleguide.md +++ b/doc/development/documentation/styleguide.md @@ -1092,6 +1092,6 @@ curl --request PUT --header "PRIVATE-TOKEN: <your_access_token>" --data "domain_ [cURL]: http://curl.haxx.se/ "cURL website" [single spaces]: http://www.slate.com/articles/technology/technology/2011/01/space_invaders.html -[gfm]: https://docs.gitlab.com/ce/user/markdown.html#newlines "GitLab flavored markdown documentation" +[gfm]: ../../user/markdown.md#newlines "GitLab flavored markdown documentation" [ce-1242]: https://gitlab.com/gitlab-org/gitlab-ce/issues/1242 [doc-restart]: ../../administration/restart_gitlab.md "GitLab restart documentation" diff --git a/doc/development/filtering_by_label.md b/doc/development/filtering_by_label.md new file mode 100644 index 00000000000..6e6b71b1787 --- /dev/null +++ b/doc/development/filtering_by_label.md @@ -0,0 +1,166 @@ +# Filtering by label + +## Introduction + +GitLab has [labels](../user/project/labels.md) that can be assigned to issues, +merge requests, and epics. Labels on those objects are a many-to-many relation +through the polymorphic `label_links` table. + +To filter these objects by multiple labels - for instance, 'all open +issues with the label ~Plan and the label ~backend' - we generate a +query containing a `GROUP BY` clause. In a simple form, this looks like: + +```sql +SELECT + issues.* +FROM + issues + INNER JOIN label_links ON label_links.target_id = issues.id + AND label_links.target_type = 'Issue' + INNER JOIN labels ON labels.id = label_links.label_id +WHERE + issues.project_id = 13083 + AND (issues.state IN ('opened')) + AND labels.title IN ('Plan', + 'backend') +GROUP BY + issues.id +HAVING (COUNT(DISTINCT labels.title) = 2) +ORDER BY + issues.updated_at DESC, + issues.id DESC +LIMIT 20 OFFSET 0 +``` + +In particular, note that: + +1. We `GROUP BY issues.id` so that we can ... +2. Use the `HAVING (COUNT(DISTINCT labels.title) = 2)` condition to ensure that + all matched issues have both labels. + +This is more complicated than is ideal. It makes the query construction more +prone to errors (such as +[gitlab-org/gitlab-ce#15557](https://gitlab.com/gitlab-org/gitlab-ce/issues/15557)). + +## Attempt A: WHERE EXISTS + +### Attempt A1: use multiple subqueries with WHERE EXISTS + +In +[gitlab-org/gitlab-ce#37137](https://gitlab.com/gitlab-org/gitlab-ce/issues/37137) +and its associated merge request +[gitlab-org/gitlab-ce!14022](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/14022), +we tried to replace the `GROUP BY` with multiple uses of `WHERE EXISTS`. For the +example above, this would give: + +```sql +WHERE (EXISTS ( + SELECT + TRUE + FROM + label_links + INNER JOIN labels ON labels.id = label_links.label_id + WHERE + labels.title = 'Plan' + AND target_type = 'Issue' + AND target_id = issues.id)) +AND (EXISTS ( + SELECT + TRUE + FROM + label_links + INNER JOIN labels ON labels.id = label_links.label_id + WHERE + labels.title = 'backend' + AND target_type = 'Issue' + AND target_id = issues.id)) +``` + +While this worked without schema changes, and did improve readability somewhat, +it did not improve query performance. + +## Attempt B: Denormalize using an array column + +Having [removed MySQL support in GitLab +12.1](https://about.gitlab.com/2019/06/27/removing-mysql-support/), using +[Postgres's arrays](https://www.postgresql.org/docs/9.6/arrays.html) became more +tractable as we didn't have to support two databases. We discussed denormalizing +the `label_links` table for querying in +[gitlab-org/gitlab-ce#49651](https://gitlab.com/gitlab-org/gitlab-ce/issues/49651), +with two options: label IDs and titles. + +We can think of both of those as array columns on `issues`, `merge_requests`, +and `epics`: `issues.label_ids` would be an array column of label IDs, and +`issues.label_titles` would be an array of label titles. + +These array columns can be complemented with [GIN +indexes](https://www.postgresql.org/docs/9.6/gin-intro.html) to improve +matching. + +### Attempt B1: store label IDs for each object + +This has some strong advantages over titles: + +1. Unless a label is deleted, or a project is moved, we never need to + bulk-update the denormalized column. +2. It uses less storage than the titles. + +Unfortunately, our application design makes this hard. If we were able to query +just by label ID easily, we wouldn't need the `INNER JOIN labels` in the initial +query at the start of this document. GitLab allows users to filter by label +title across projects and even across groups, so a filter by the label ~Plan may +include labels with multiple distinct IDs. + +We do not want users to have to know about the different IDs, which means that +given this data set: + +| Project | ~Plan label ID | ~backend label ID | +| --- | --- | --- | +| A | 11 | 12 | +| B | 21 | 22 | +| C | 31 | 32 | + +We would need something like: + +```sql +WHERE + label_ids @> ARRAY[11, 12] + OR label_ids @> ARRAY[21, 22] + OR label_ids @> ARRAY[31, 32] +``` + +This can get even more complicated when we consider that in some cases, there +might be two ~backend labels - with different IDs - that could apply to the same +object, so the number of combinations would balloon further. + +### Attempt B2: store label titles for each object + +From the perspective of updating the labelable object, this is the worst +option. We have to bulk update the objects when: + +1. The objects are moved from one project to another. +1. The project is moved from one group to another. +1. The label is renamed. +1. The label is deleted. + +It also uses much more storage. Querying is simple, though: + +```sql +WHERE + label_titles @> ARRAY['Plan', 'backend'] +``` + +And our [tests in +gitlab-org/gitlab-ce#49651](https://gitlab.com/gitlab-org/gitlab-ce/issues/49651#note_188777346) +showed that this could be fast. + +However, at present, the disadvantages outweigh the advantages. + +## Conclusion + +We have yet to find a method that is demonstratably better than the current +method, when considering: + +1. Query performance. +1. Readability. +1. Ease of maintaining schema consistency. diff --git a/doc/development/testing_guide/frontend_testing.md b/doc/development/testing_guide/frontend_testing.md index ff28c2ea5e2..2985278cc92 100644 --- a/doc/development/testing_guide/frontend_testing.md +++ b/doc/development/testing_guide/frontend_testing.md @@ -588,7 +588,7 @@ end [jasmine-focus]: https://jasmine.github.io/2.5/focused_specs.html [karma]: http://karma-runner.github.io/ -[vue-test]: https://docs.gitlab.com/ce/development/fe_guide/vue.html#testing-vue-components +[vue-test]: ../fe_guide/vue.md#testing-vue-components [rspec]: https://github.com/rspec/rspec-rails#feature-specs [capybara]: https://github.com/teamcapybara/capybara [jasmine]: https://jasmine.github.io/ diff --git a/doc/install/azure/index.md b/doc/install/azure/index.md index c0e1b0ebbc8..543a222bd25 100644 --- a/doc/install/azure/index.md +++ b/doc/install/azure/index.md @@ -70,7 +70,7 @@ The first items we need to configure are the basic settings of the underlying vi > **Note:** if you're unsure which authentication type to use, select **Password** 1. If you chose **SSH public key** - enter your `SSH public key` into the field provided - _(read the [SSH documentation][GitLab-Docs-SSH] to learn more about how to set up SSH + _(read the [SSH documentation](../../ssh/README.md) to learn more about how to set up SSH public keys)_ 1. If you chose **Password** - enter the password you wish to use _(this is the password that you will use later in this tutorial to [SSH] into the VM, so make sure it's a strong password/passphrase)_ @@ -407,7 +407,7 @@ on any cloud service you choose. ## Where to next? -Check out our other [Technical Articles][GitLab-Technical-Articles] or browse the [GitLab Documentation][GitLab-Docs] to learn more about GitLab. +Check out our other [Technical Articles](../../articles/index.md) or browse the [GitLab Documentation][GitLab-Docs](../../README.md) to learn more about GitLab. ### Useful links @@ -423,9 +423,6 @@ Check out our other [Technical Articles][GitLab-Technical-Articles] or browse th - [SSH], [PuTTY] and [Using SSH in PuTTY][Using-SSH-In-Putty] [Original-Blog-Post]: https://about.gitlab.com/2016/07/13/how-to-setup-a-gitlab-instance-on-microsoft-azure/ "How to Set up a GitLab Instance on Microsoft Azure" -[GitLab-Docs]: https://docs.gitlab.com/ce/README.html "GitLab Documentation" -[GitLab-Technical-Articles]: https://docs.gitlab.com/ce/articles/index.html "GitLab Technical Articles" -[GitLab-Docs-SSH]: https://docs.gitlab.com/ce/ssh/README.html "GitLab Documentation: SSH" [CE]: https://about.gitlab.com/features/ [EE]: https://about.gitlab.com/features/#ee-starter diff --git a/doc/integration/README.md b/doc/integration/README.md index 135952a1b08..55f9666e3a3 100644 --- a/doc/integration/README.md +++ b/doc/integration/README.md @@ -27,7 +27,7 @@ See the documentation below for details on how to configure these services. - [SAML](saml.md) Configure GitLab as a SAML 2.0 Service Provider - [Trello](trello_power_up.md) Integrate Trello with GitLab -> GitLab Enterprise Edition contains [advanced Jenkins support][jenkins]. +> GitLab Enterprise Edition contains [advanced Jenkins support](jenkins.md). ## Project services @@ -70,5 +70,3 @@ After that restart GitLab with: ```bash sudo gitlab-ctl restart ``` - -[jenkins]: https://docs.gitlab.com/ee/integration/jenkins.html diff --git a/doc/topics/autodevops/index.md b/doc/topics/autodevops/index.md index 296ab63166f..c6219126b30 100644 --- a/doc/topics/autodevops/index.md +++ b/doc/topics/autodevops/index.md @@ -192,10 +192,10 @@ The following table is an example of how the three different clusters would be configured. | Cluster name | Cluster environment scope | `KUBE_INGRESS_BASE_DOMAIN` variable value | Variable environment scope | Notes | -| ------------ | -------------- | ----------------------------- | ------------- | ------ | -| review | `review/*` | `review.example.com` | `review/*` | The review cluster which will run all [Review Apps](../../ci/review_apps/index.md). `*` is a wildcard, which means it will be used by every environment name starting with `review/`. | -| staging | `staging` | `staging.example.com` | `staging` | (Optional) The staging cluster which will run the deployments of the staging environments. You need to [enable it first](#deploy-policy-for-staging-and-production-environments). | -| production | `production` | `example.com` | `production` | The production cluster which will run the deployments of the production environment. You can use [incremental rollouts](#incremental-rollout-to-production-premium). | +|--------------|---------------------------|-------------------------------------------|----------------------------|---| +| review | `review/*` | `review.example.com` | `review/*` | The review cluster which will run all [Review Apps](../../ci/review_apps/index.md). `*` is a wildcard, which means it will be used by every environment name starting with `review/`. | +| staging | `staging` | `staging.example.com` | `staging` | (Optional) The staging cluster which will run the deployments of the staging environments. You need to [enable it first](#deploy-policy-for-staging-and-production-environments). | +| production | `production` | `example.com` | `production` | The production cluster which will run the deployments of the production environment. You can use [incremental rollouts](#incremental-rollout-to-production-premium). | To add a different cluster for each environment: @@ -721,47 +721,47 @@ The following variables can be used for setting up the Auto DevOps domain, providing a custom Helm chart, or scaling your application. PostgreSQL can also be customized, and you can easily use a [custom buildpack](#custom-buildpacks). -| **Variable** | **Description** | -| ------------ | --------------- | -| `AUTO_DEVOPS_CHART` | The Helm Chart used to deploy your apps; defaults to the one [provided by GitLab](https://gitlab.com/gitlab-org/charts/auto-deploy-app). | -| `AUTO_DEVOPS_CHART_REPOSITORY` | The Helm Chart repository used to search for charts; defaults to `https://charts.gitlab.io`. | -| `AUTO_DEVOPS_CHART_REPOSITORY_NAME` | From Gitlab 11.11, this variable can be used to set the name of the helm repository; defaults to "gitlab" | +| **Variable** | **Description** | +|-----------------------------------------|------------------------------------| +| `AUTO_DEVOPS_CHART` | The Helm Chart used to deploy your apps; defaults to the one [provided by GitLab](https://gitlab.com/gitlab-org/charts/auto-deploy-app). | +| `AUTO_DEVOPS_CHART_REPOSITORY` | The Helm Chart repository used to search for charts; defaults to `https://charts.gitlab.io`. | +| `AUTO_DEVOPS_CHART_REPOSITORY_NAME` | From Gitlab 11.11, this variable can be used to set the name of the helm repository; defaults to "gitlab" | | `AUTO_DEVOPS_CHART_REPOSITORY_USERNAME` | From Gitlab 11.11, this variable can be used to set a username to connect to the helm repository. Defaults to no credentials. (Also set AUTO_DEVOPS_CHART_REPOSITORY_PASSWORD) | | `AUTO_DEVOPS_CHART_REPOSITORY_PASSWORD` | From Gitlab 11.11, this variable can be used to set a password to connect to the helm repository. Defaults to no credentials. (Also set AUTO_DEVOPS_CHART_REPOSITORY_USERNAME) | -| `REPLICAS` | The number of replicas to deploy; defaults to 1. | -| `PRODUCTION_REPLICAS` | The number of replicas to deploy in the production environment. Takes precedence over `REPLICAS` and defaults to 1. For zero downtime upgrades, set to 2 or greater. | -| `CANARY_REPLICAS` | The number of canary replicas to deploy for [Canary Deployments](../../user/project/canary_deployments.md); defaults to 1 | -| `CANARY_PRODUCTION_REPLICAS` | The number of canary replicas to deploy for [Canary Deployments](../../user/project/canary_deployments.md) in the production environment. This takes precedence over `CANARY_REPLICAS`; defaults to 1 | -| `ADDITIONAL_HOSTS` | Fully qualified domain names specified as a comma-separated list that are added to the ingress hosts. | -| `<ENVIRONMENT>_ADDITIONAL_HOSTS` | For a specific environment, the fully qualified domain names specified as a comma-separated list that are added to the ingress hosts. This takes precedence over `ADDITIONAL_HOSTS`. | -| `POSTGRES_ENABLED` | Whether PostgreSQL is enabled; defaults to `"true"`. Set to `false` to disable the automatic deployment of PostgreSQL. | -| `POSTGRES_USER` | The PostgreSQL user; defaults to `user`. Set it to use a custom username. | -| `POSTGRES_PASSWORD` | The PostgreSQL password; defaults to `testing-password`. Set it to use a custom password. | -| `POSTGRES_DB` | The PostgreSQL database name; defaults to the value of [`$CI_ENVIRONMENT_SLUG`](../../ci/variables/README.md#predefined-environment-variables). Set it to use a custom database name. | -| `POSTGRES_VERSION` | Tag for the [`postgres` Docker image](https://hub.docker.com/_/postgres) to use. Defaults to `9.6.2`. | -| `BUILDPACK_URL` | The buildpack's full URL. It can point to either Git repositories or a tarball URL. For Git repositories, it is possible to point to a specific `ref`, for example `https://github.com/heroku/heroku-buildpack-ruby.git#v142` | -| `SAST_CONFIDENCE_LEVEL` | The minimum confidence level of security issues you want to be reported; `1` for Low, `2` for Medium, `3` for High; defaults to `3`.| -| `DEP_SCAN_DISABLE_REMOTE_CHECKS` | Whether remote Dependency Scanning checks are disabled; defaults to `"false"`. Set to `"true"` to disable checks that send data to GitLab central servers. [Read more about remote checks](https://gitlab.com/gitlab-org/security-products/dependency-scanning#remote-checks).| -| `DB_INITIALIZE` | From GitLab 11.4, this variable can be used to specify the command to run to initialize the application's PostgreSQL database. It runs inside the application pod. | -| `DB_MIGRATE` | From GitLab 11.4, this variable can be used to specify the command to run to migrate the application's PostgreSQL database. It runs inside the application pod. | -| `STAGING_ENABLED` | From GitLab 10.8, this variable can be used to define a [deploy policy for staging and production environments](#deploy-policy-for-staging-and-production-environments). | -| `CANARY_ENABLED` | From GitLab 11.0, this variable can be used to define a [deploy policy for canary environments](#deploy-policy-for-canary-environments-premium). | -| `INCREMENTAL_ROLLOUT_MODE`| From GitLab 11.4, this variable, if present, can be used to enable an [incremental rollout](#incremental-rollout-to-production-premium) of your application for the production environment. Set to `manual` for manual deployment jobs or `timed` for automatic rollout deployments with a 5 minute delay each one. | -| `TEST_DISABLED` | From GitLab 11.0, this variable can be used to disable the `test` job. If the variable is present, the job will not be created. | -| `CODE_QUALITY_DISABLED` | From GitLab 11.0, this variable can be used to disable the `codequality` job. If the variable is present, the job will not be created. | -| `LICENSE_MANAGEMENT_DISABLED` | From GitLab 11.0, this variable can be used to disable the `license_management` job. If the variable is present, the job will not be created. | -| `SAST_DISABLED` | From GitLab 11.0, this variable can be used to disable the `sast` job. If the variable is present, the job will not be created. | -| `DEPENDENCY_SCANNING_DISABLED` | From GitLab 11.0, this variable can be used to disable the `dependency_scanning` job. If the variable is present, the job will not be created. | -| `CONTAINER_SCANNING_DISABLED` | From GitLab 11.0, this variable can be used to disable the `sast:container` job. If the variable is present, the job will not be created. | -| `REVIEW_DISABLED` | From GitLab 11.0, this variable can be used to disable the `review` and the manual `review:stop` job. If the variable is present, these jobs will not be created. | -| `DAST_DISABLED` | From GitLab 11.0, this variable can be used to disable the `dast` job. If the variable is present, the job will not be created. | -| `PERFORMANCE_DISABLED` | From GitLab 11.0, this variable can be used to disable the `performance` job. If the variable is present, the job will not be created. | -| `K8S_SECRET_*` | From GitLab 11.7, any variable prefixed with [`K8S_SECRET_`](#application-secret-variables) will be made available by Auto DevOps as environment variables to the deployed application. | -| `KUBE_INGRESS_BASE_DOMAIN` | From GitLab 11.8, this variable can be used to set a domain per cluster. See [cluster domains](../../user/project/clusters/index.md#base-domain) for more information. | -| `ROLLOUT_RESOURCE_TYPE` | From GitLab 11.9, this variable allows specification of the resource type being deployed when using a custom helm chart. Default value is `deployment`. | -| `ROLLOUT_STATUS_DISABLED` | From GitLab 12.0, this variable allows to disable rollout status check because it doesn't support all resource types, for example, `cronjob`. | -| `HELM_UPGRADE_EXTRA_ARGS` | From GitLab 11.11, this variable allows extra arguments in `helm` commands when deploying the application. Note that using quotes will not prevent word splitting. **Tip:** you can use this variable to [customize the Auto Deploy helm chart](https://docs.gitlab.com/ee/topics/autodevops/index.html#custom-helm-chart) by applying custom override values with `--values my-values.yaml`. | -| `HELM_RELEASE_NAME` | From GitLab 12.1, this variable allows the `helm` release name to be overridden, this can be used to assign unique release names when deploying multiple projects to a single namespace | +| `REPLICAS` | The number of replicas to deploy; defaults to 1. | +| `PRODUCTION_REPLICAS` | The number of replicas to deploy in the production environment. Takes precedence over `REPLICAS` and defaults to 1. For zero downtime upgrades, set to 2 or greater. | +| `CANARY_REPLICAS` | The number of canary replicas to deploy for [Canary Deployments](../../user/project/canary_deployments.md); defaults to 1. | +| `CANARY_PRODUCTION_REPLICAS` | The number of canary replicas to deploy for [Canary Deployments](../../user/project/canary_deployments.md) in the production environment. This takes precedence over `CANARY_REPLICAS`; defaults to 1. | +| `ADDITIONAL_HOSTS` | Fully qualified domain names specified as a comma-separated list that are added to the ingress hosts. | +| `<ENVIRONMENT>_ADDITIONAL_HOSTS` | For a specific environment, the fully qualified domain names specified as a comma-separated list that are added to the ingress hosts. This takes precedence over `ADDITIONAL_HOSTS`. | +| `POSTGRES_ENABLED` | Whether PostgreSQL is enabled; defaults to `"true"`. Set to `false` to disable the automatic deployment of PostgreSQL. | +| `POSTGRES_USER` | The PostgreSQL user; defaults to `user`. Set it to use a custom username. | +| `POSTGRES_PASSWORD` | The PostgreSQL password; defaults to `testing-password`. Set it to use a custom password. | +| `POSTGRES_DB` | The PostgreSQL database name; defaults to the value of [`$CI_ENVIRONMENT_SLUG`](../../ci/variables/README.md#predefined-environment-variables). Set it to use a custom database name. | +| `POSTGRES_VERSION` | Tag for the [`postgres` Docker image](https://hub.docker.com/_/postgres) to use. Defaults to `9.6.2`. | +| `BUILDPACK_URL` | The buildpack's full URL. It can point to either Git repositories or a tarball URL. For Git repositories, it is possible to point to a specific `ref`, for example `https://github.com/heroku/heroku-buildpack-ruby.git#v142`. | +| `SAST_CONFIDENCE_LEVEL` | The minimum confidence level of security issues you want to be reported; `1` for Low, `2` for Medium, `3` for High; defaults to `3`. | +| `DS_DISABLE_REMOTE_CHECKS` | Whether remote Dependency Scanning checks are disabled; defaults to `"false"`. Set to `"true"` to disable checks that send data to GitLab central servers. [Read more about remote checks](../../user/application_security/dependency_scanning/index.md#remote-checks). | +| `DB_INITIALIZE` | From GitLab 11.4, this variable can be used to specify the command to run to initialize the application's PostgreSQL database. It runs inside the application pod. | +| `DB_MIGRATE` | From GitLab 11.4, this variable can be used to specify the command to run to migrate the application's PostgreSQL database. It runs inside the application pod. | +| `STAGING_ENABLED` | From GitLab 10.8, this variable can be used to define a [deploy policy for staging and production environments](#deploy-policy-for-staging-and-production-environments). | +| `CANARY_ENABLED` | From GitLab 11.0, this variable can be used to define a [deploy policy for canary environments](#deploy-policy-for-canary-environments-premium). | +| `INCREMENTAL_ROLLOUT_MODE` | From GitLab 11.4, this variable, if present, can be used to enable an [incremental rollout](#incremental-rollout-to-production-premium) of your application for the production environment. Set to `manual` for manual deployment jobs or `timed` for automatic rollout deployments with a 5 minute delay each one. | +| `TEST_DISABLED` | From GitLab 11.0, this variable can be used to disable the `test` job. If the variable is present, the job will not be created. | +| `CODE_QUALITY_DISABLED` | From GitLab 11.0, this variable can be used to disable the `codequality` job. If the variable is present, the job will not be created. | +| `LICENSE_MANAGEMENT_DISABLED` | From GitLab 11.0, this variable can be used to disable the `license_management` job. If the variable is present, the job will not be created. | +| `SAST_DISABLED` | From GitLab 11.0, this variable can be used to disable the `sast` job. If the variable is present, the job will not be created. | +| `DEPENDENCY_SCANNING_DISABLED` | From GitLab 11.0, this variable can be used to disable the `dependency_scanning` job. If the variable is present, the job will not be created. | +| `CONTAINER_SCANNING_DISABLED` | From GitLab 11.0, this variable can be used to disable the `sast:container` job. If the variable is present, the job will not be created. | +| `REVIEW_DISABLED` | From GitLab 11.0, this variable can be used to disable the `review` and the manual `review:stop` job. If the variable is present, these jobs will not be created. | +| `DAST_DISABLED` | From GitLab 11.0, this variable can be used to disable the `dast` job. If the variable is present, the job will not be created. | +| `PERFORMANCE_DISABLED` | From GitLab 11.0, this variable can be used to disable the `performance` job. If the variable is present, the job will not be created. | +| `K8S_SECRET_*` | From GitLab 11.7, any variable prefixed with [`K8S_SECRET_`](#application-secret-variables) will be made available by Auto DevOps as environment variables to the deployed application. | +| `KUBE_INGRESS_BASE_DOMAIN` | From GitLab 11.8, this variable can be used to set a domain per cluster. See [cluster domains](../../user/project/clusters/index.md#base-domain) for more information. | +| `ROLLOUT_RESOURCE_TYPE` | From GitLab 11.9, this variable allows specification of the resource type being deployed when using a custom helm chart. Default value is `deployment`. | +| `ROLLOUT_STATUS_DISABLED` | From GitLab 12.0, this variable allows to disable rollout status check because it doesn't support all resource types, for example, `cronjob`. | +| `HELM_UPGRADE_EXTRA_ARGS` | From GitLab 11.11, this variable allows extra arguments in `helm` commands when deploying the application. Note that using quotes will not prevent word splitting. **Tip:** you can use this variable to [customize the Auto Deploy helm chart](#custom-helm-chart) by applying custom override values with `--values my-values.yaml`. | +| `HELM_RELEASE_NAME` | From GitLab 12.1, this variable allows the `helm` release name to be overridden, this can be used to assign unique release names when deploying multiple projects to a single namespace. | TIP: **Tip:** Set up the replica variables using a diff --git a/doc/user/project/integrations/prometheus.md b/doc/user/project/integrations/prometheus.md index ba982c9cf6f..e609fe43507 100644 --- a/doc/user/project/integrations/prometheus.md +++ b/doc/user/project/integrations/prometheus.md @@ -323,8 +323,9 @@ Once enabled, an issue will be opened automatically when an alert is triggered w - `starts_at`: Alert start time via `startsAt` - `full_query`: Alert query extracted from `generatorURL` - Optional list of attached annotations extracted from `annotations/*` +- Alert [GFM](../../markdown.md): GitLab Flavored Markdown from `annotations/gitlab_incident_markdown` -To further customize the issue, you can add labels, mentions, or any other supported [quick action](../quick_actions.md) in the selected issue template. +To further customize the issue, you can add labels, mentions, or any other supported [quick action](../quick_actions.md) in the selected issue template, which will apply to all incidents. To limit quick actions or other information to only specific types of alerts, use the `annotations/gitlab_incident_markdown` field. Since [version 12.2](https://gitlab.com/gitlab-org/gitlab-ce/issues/63373), GitLab will tag each incident issue with the `incident` label automatically. If the label does not yet exist, it will be created automatically as well. diff --git a/doc/user/project/protected_branches.md b/doc/user/project/protected_branches.md index 20a03dff2da..9a3c02e1f50 100644 --- a/doc/user/project/protected_branches.md +++ b/doc/user/project/protected_branches.md @@ -180,6 +180,5 @@ for details about the pipelines security model. [ce-4892]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/4892 "Allow developers to merge into a protected branch without having push access" [ce-5081]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5081 "Allow creating protected branches that can't be pushed to" [ce-21393]: https://gitlab.com/gitlab-org/gitlab-ce/issues/21393 -[ee-restrict]: https://docs.gitlab.com/ee/user/project/protected_branches.html#restricting-push-and-merge-access-to-certain-users [perm]: ../permissions.md [ee]: https://about.gitlab.com/pricing/ diff --git a/lib/api/validations/types/labels_list.rb b/lib/api/validations/types/labels_list.rb index 47cd83c29cf..60277b99106 100644 --- a/lib/api/validations/types/labels_list.rb +++ b/lib/api/validations/types/labels_list.rb @@ -10,7 +10,7 @@ module API when String value.split(',').map(&:strip) when Array - value.map { |v| v.to_s.split(',').map(&:strip) }.flatten + value.flat_map { |v| v.to_s.split(',').map(&:strip) } when LabelsList value else diff --git a/lib/banzai/reference_redactor.rb b/lib/banzai/reference_redactor.rb index eb5c35da375..936436982e7 100644 --- a/lib/banzai/reference_redactor.rb +++ b/lib/banzai/reference_redactor.rb @@ -33,7 +33,7 @@ module Banzai # # data - An Array of a Hashes mapping an HTML document to nodes to redact. def redact_document_nodes(all_document_nodes) - all_nodes = all_document_nodes.map { |x| x[:nodes] }.flatten + all_nodes = all_document_nodes.flat_map { |x| x[:nodes] } visible = nodes_visible_to_user(all_nodes) metadata = [] diff --git a/lib/gitlab/auth/activity.rb b/lib/gitlab/auth/activity.rb index 558628b5422..988ff196193 100644 --- a/lib/gitlab/auth/activity.rb +++ b/lib/gitlab/auth/activity.rb @@ -37,14 +37,17 @@ module Gitlab def user_authenticated! self.class.user_authenticated_counter_increment! + + case @opts[:message] + when :two_factor_authenticated + self.class.user_two_factor_authenticated_counter_increment! + end end def user_session_override! self.class.user_session_override_counter_increment! case @opts[:message] - when :two_factor_authenticated - self.class.user_two_factor_authenticated_counter_increment! when :sessionless_sign_in self.class.user_sessionless_authentication_counter_increment! end diff --git a/lib/gitlab/background_migration/backfill_project_repositories.rb b/lib/gitlab/background_migration/backfill_project_repositories.rb index c8d83cc1803..1d9aa050041 100644 --- a/lib/gitlab/background_migration/backfill_project_repositories.rb +++ b/lib/gitlab/background_migration/backfill_project_repositories.rb @@ -40,7 +40,7 @@ module Gitlab end def reload! - @shards = Hash[*Shard.all.map { |shard| [shard.name, shard.id] }.flatten] + @shards = Hash[*Shard.all.flat_map { |shard| [shard.name, shard.id] }] end end diff --git a/lib/gitlab/ci/config/normalizer.rb b/lib/gitlab/ci/config/normalizer.rb index 191f5d09645..99356226ef9 100644 --- a/lib/gitlab/ci/config/normalizer.rb +++ b/lib/gitlab/ci/config/normalizer.rb @@ -46,7 +46,7 @@ module Gitlab 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.map { |dep| @parallelized_jobs[dep.to_sym].map(&:first) }.flatten + 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) else diff --git a/lib/gitlab/ci/status/factory.rb b/lib/gitlab/ci/status/factory.rb index 3446644eff8..2a0bf060c9b 100644 --- a/lib/gitlab/ci/status/factory.rb +++ b/lib/gitlab/ci/status/factory.rb @@ -34,11 +34,9 @@ module Gitlab def extended_statuses return @extended_statuses if defined?(@extended_statuses) - groups = self.class.extended_statuses.map do |group| + @extended_statuses = self.class.extended_statuses.flat_map do |group| Array(group).find { |status| status.matches?(@subject, @user) } - end - - @extended_statuses = groups.flatten.compact + end.compact end def self.extended_statuses diff --git a/lib/gitlab/ci/templates/Auto-DevOps.gitlab-ci.yml b/lib/gitlab/ci/templates/Auto-DevOps.gitlab-ci.yml index 7b9a169a91f..5c1c0c142e5 100644 --- a/lib/gitlab/ci/templates/Auto-DevOps.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Auto-DevOps.gitlab-ci.yml @@ -50,9 +50,6 @@ variables: POSTGRES_DB: $CI_ENVIRONMENT_SLUG POSTGRES_VERSION: 9.6.2 - KUBERNETES_VERSION: 1.11.10 - HELM_VERSION: 2.14.0 - DOCKER_DRIVER: overlay2 ROLLOUT_RESOURCE_TYPE: deployment diff --git a/lib/gitlab/ci/templates/Jobs/Deploy.gitlab-ci.yml b/lib/gitlab/ci/templates/Jobs/Deploy.gitlab-ci.yml index 6ead127e7b6..a8ec2d4781d 100644 --- a/lib/gitlab/ci/templates/Jobs/Deploy.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Jobs/Deploy.gitlab-ci.yml @@ -1,14 +1,17 @@ +.auto-deploy: + image: "registry.gitlab.com/gitlab-org/cluster-integration/auto-deploy-image:v0.1.0" + review: + extends: .auto-deploy stage: review script: - - check_kube_domain - - install_dependencies - - download_chart - - ensure_namespace - - initialize_tiller - - create_secret - - deploy - - persist_environment_url + - auto-deploy check_kube_domain + - auto-deploy download_chart + - auto-deploy ensure_namespace + - auto-deploy initialize_tiller + - auto-deploy create_secret + - auto-deploy deploy + - auto-deploy persist_environment_url environment: name: review/$CI_COMMIT_REF_NAME url: http://$CI_PROJECT_ID-$CI_ENVIRONMENT_SLUG.$KUBE_INGRESS_BASE_DOMAIN @@ -27,13 +30,13 @@ review: - $REVIEW_DISABLED stop_review: + extends: .auto-deploy stage: cleanup variables: GIT_STRATEGY: none script: - - install_dependencies - - initialize_tiller - - delete + - auto-deploy initialize_tiller + - auto-deploy delete environment: name: review/$CI_COMMIT_REF_NAME action: stop @@ -57,15 +60,15 @@ stop_review: # STAGING_ENABLED. staging: + extends: .auto-deploy stage: staging script: - - check_kube_domain - - install_dependencies - - download_chart - - ensure_namespace - - initialize_tiller - - create_secret - - deploy + - auto-deploy check_kube_domain + - auto-deploy download_chart + - auto-deploy ensure_namespace + - auto-deploy initialize_tiller + - auto-deploy create_secret + - auto-deploy deploy environment: name: staging url: http://$CI_PROJECT_PATH_SLUG-staging.$KUBE_INGRESS_BASE_DOMAIN @@ -81,15 +84,15 @@ staging: # CANARY_ENABLED. canary: + extends: .auto-deploy stage: canary script: - - check_kube_domain - - install_dependencies - - download_chart - - ensure_namespace - - initialize_tiller - - create_secret - - deploy canary + - auto-deploy check_kube_domain + - auto-deploy download_chart + - auto-deploy ensure_namespace + - auto-deploy initialize_tiller + - auto-deploy create_secret + - auto-deploy deploy canary environment: name: production url: http://$CI_PROJECT_PATH_SLUG.$KUBE_INGRESS_BASE_DOMAIN @@ -102,18 +105,18 @@ canary: - $CANARY_ENABLED .production: &production_template + extends: .auto-deploy stage: production script: - - check_kube_domain - - install_dependencies - - download_chart - - ensure_namespace - - initialize_tiller - - create_secret - - deploy - - delete canary - - delete rollout - - persist_environment_url + - auto-deploy check_kube_domain + - auto-deploy download_chart + - auto-deploy ensure_namespace + - auto-deploy initialize_tiller + - auto-deploy create_secret + - auto-deploy deploy + - auto-deploy delete canary + - auto-deploy delete rollout + - auto-deploy persist_environment_url environment: name: production url: http://$CI_PROJECT_PATH_SLUG.$KUBE_INGRESS_BASE_DOMAIN @@ -152,17 +155,17 @@ production_manual: # This job implements incremental rollout on for every push to `master`. .rollout: &rollout_template + extends: .auto-deploy script: - - check_kube_domain - - install_dependencies - - download_chart - - ensure_namespace - - initialize_tiller - - create_secret - - deploy rollout $ROLLOUT_PERCENTAGE - - scale stable $((100-ROLLOUT_PERCENTAGE)) - - delete canary - - persist_environment_url + - auto-deploy check_kube_domain + - auto-deploy download_chart + - auto-deploy ensure_namespace + - auto-deploy initialize_tiller + - auto-deploy create_secret + - auto-deploy deploy rollout $ROLLOUT_PERCENTAGE + - auto-deploy scale stable $((100-ROLLOUT_PERCENTAGE)) + - auto-deploy delete canary + - auto-deploy persist_environment_url environment: name: production url: http://$CI_PROJECT_PATH_SLUG.$KUBE_INGRESS_BASE_DOMAIN @@ -240,331 +243,3 @@ rollout 100%: <<: *manual_rollout_template <<: *production_template allow_failure: false - -.deploy_helpers: &deploy_helpers | - [[ "$TRACE" ]] && set -x - export RELEASE_NAME=${HELM_RELEASE_NAME:-$CI_ENVIRONMENT_SLUG} - auto_database_url=postgres://${POSTGRES_USER}:${POSTGRES_PASSWORD}@${RELEASE_NAME}-postgres:5432/${POSTGRES_DB} - export DATABASE_URL=${DATABASE_URL-$auto_database_url} - export TILLER_NAMESPACE=$KUBE_NAMESPACE - - function get_replicas() { - track="${1:-stable}" - percentage="${2:-100}" - - env_track=$( echo $track | tr -s '[:lower:]' '[:upper:]' ) - env_slug=$( echo ${CI_ENVIRONMENT_SLUG//-/_} | tr -s '[:lower:]' '[:upper:]' ) - - if [[ "$track" == "stable" ]] || [[ "$track" == "rollout" ]]; then - # for stable track get number of replicas from `PRODUCTION_REPLICAS` - eval new_replicas=\$${env_slug}_REPLICAS - if [[ -z "$new_replicas" ]]; then - new_replicas=$REPLICAS - fi - else - # for all tracks get number of replicas from `CANARY_PRODUCTION_REPLICAS` - eval new_replicas=\$${env_track}_${env_slug}_REPLICAS - if [[ -z "$new_replicas" ]]; then - eval new_replicas=\${env_track}_REPLICAS - fi - fi - - replicas="${new_replicas:-1}" - replicas="$(($replicas * $percentage / 100))" - - # always return at least one replicas - if [[ $replicas -gt 0 ]]; then - echo "$replicas" - else - echo 1 - fi - } - - # Extracts variables prefixed with K8S_SECRET_ - # and creates a Kubernetes secret. - # - # e.g. If we have the following environment variables: - # K8S_SECRET_A=value1 - # K8S_SECRET_B=multi\ word\ value - # - # Then we will create a secret with the following key-value pairs: - # data: - # A: dmFsdWUxCg== - # B: bXVsdGkgd29yZCB2YWx1ZQo= - function create_application_secret() { - track="${1-stable}" - export APPLICATION_SECRET_NAME=$(application_secret_name "$track") - - env | sed -n "s/^K8S_SECRET_\(.*\)$/\1/p" > k8s_prefixed_variables - - kubectl create secret \ - -n "$KUBE_NAMESPACE" generic "$APPLICATION_SECRET_NAME" \ - --from-env-file k8s_prefixed_variables -o yaml --dry-run | - kubectl replace -n "$KUBE_NAMESPACE" --force -f - - - export APPLICATION_SECRET_CHECKSUM=$(cat k8s_prefixed_variables | sha256sum | cut -d ' ' -f 1) - - rm k8s_prefixed_variables - } - - function deploy_name() { - name="$RELEASE_NAME" - track="${1-stable}" - - if [[ "$track" != "stable" ]]; then - name="$name-$track" - fi - - echo $name - } - - function application_secret_name() { - track="${1-stable}" - name=$(deploy_name "$track") - - echo "${name}-secret" - } - - function deploy() { - track="${1-stable}" - percentage="${2:-100}" - name=$(deploy_name "$track") - - if [[ -z "$CI_COMMIT_TAG" ]]; then - image_repository=${CI_APPLICATION_REPOSITORY:-$CI_REGISTRY_IMAGE/$CI_COMMIT_REF_SLUG} - image_tag=${CI_APPLICATION_TAG:-$CI_COMMIT_SHA} - else - image_repository=${CI_APPLICATION_REPOSITORY:-$CI_REGISTRY_IMAGE} - image_tag=${CI_APPLICATION_TAG:-$CI_COMMIT_TAG} - fi - - service_enabled="true" - postgres_enabled="$POSTGRES_ENABLED" - - # if track is different than stable, - # re-use all attached resources - if [[ "$track" != "stable" ]]; then - service_enabled="false" - postgres_enabled="false" - fi - - replicas=$(get_replicas "$track" "$percentage") - - if [[ "$CI_PROJECT_VISIBILITY" != "public" ]]; then - secret_name='gitlab-registry' - else - secret_name='' - fi - - create_application_secret "$track" - - env_slug=$(echo ${CI_ENVIRONMENT_SLUG//-/_} | tr -s '[:lower:]' '[:upper:]') - eval env_ADDITIONAL_HOSTS=\$${env_slug}_ADDITIONAL_HOSTS - if [ -n "$env_ADDITIONAL_HOSTS" ]; then - additional_hosts="{$env_ADDITIONAL_HOSTS}" - elif [ -n "$ADDITIONAL_HOSTS" ]; then - additional_hosts="{$ADDITIONAL_HOSTS}" - fi - - if [[ -n "$DB_INITIALIZE" && -z "$(helm ls -q "^$name$")" ]]; then - echo "Deploying first release with database initialization..." - helm upgrade --install \ - --wait \ - --set service.enabled="$service_enabled" \ - --set gitlab.app="$CI_PROJECT_PATH_SLUG" \ - --set gitlab.env="$CI_ENVIRONMENT_SLUG" \ - --set releaseOverride="$RELEASE_NAME" \ - --set image.repository="$image_repository" \ - --set image.tag="$image_tag" \ - --set image.pullPolicy=IfNotPresent \ - --set image.secrets[0].name="$secret_name" \ - --set application.track="$track" \ - --set application.database_url="$DATABASE_URL" \ - --set application.secretName="$APPLICATION_SECRET_NAME" \ - --set application.secretChecksum="$APPLICATION_SECRET_CHECKSUM" \ - --set service.commonName="le-$CI_PROJECT_ID.$KUBE_INGRESS_BASE_DOMAIN" \ - --set service.url="$CI_ENVIRONMENT_URL" \ - --set service.additionalHosts="$additional_hosts" \ - --set replicaCount="$replicas" \ - --set postgresql.enabled="$postgres_enabled" \ - --set postgresql.nameOverride="postgres" \ - --set postgresql.postgresUser="$POSTGRES_USER" \ - --set postgresql.postgresPassword="$POSTGRES_PASSWORD" \ - --set postgresql.postgresDatabase="$POSTGRES_DB" \ - --set postgresql.imageTag="$POSTGRES_VERSION" \ - --set application.initializeCommand="$DB_INITIALIZE" \ - $HELM_UPGRADE_EXTRA_ARGS \ - --namespace="$KUBE_NAMESPACE" \ - "$name" \ - chart/ - - echo "Deploying second release..." - helm upgrade --reuse-values \ - --wait \ - --set application.initializeCommand="" \ - --set application.migrateCommand="$DB_MIGRATE" \ - $HELM_UPGRADE_EXTRA_ARGS \ - --namespace="$KUBE_NAMESPACE" \ - "$name" \ - chart/ - else - echo "Deploying new release..." - helm upgrade --install \ - --wait \ - --set service.enabled="$service_enabled" \ - --set gitlab.app="$CI_PROJECT_PATH_SLUG" \ - --set gitlab.env="$CI_ENVIRONMENT_SLUG" \ - --set releaseOverride="$RELEASE_NAME" \ - --set image.repository="$image_repository" \ - --set image.tag="$image_tag" \ - --set image.pullPolicy=IfNotPresent \ - --set image.secrets[0].name="$secret_name" \ - --set application.track="$track" \ - --set application.database_url="$DATABASE_URL" \ - --set application.secretName="$APPLICATION_SECRET_NAME" \ - --set application.secretChecksum="$APPLICATION_SECRET_CHECKSUM" \ - --set service.commonName="le-$CI_PROJECT_ID.$KUBE_INGRESS_BASE_DOMAIN" \ - --set service.url="$CI_ENVIRONMENT_URL" \ - --set service.additionalHosts="$additional_hosts" \ - --set replicaCount="$replicas" \ - --set postgresql.enabled="$postgres_enabled" \ - --set postgresql.nameOverride="postgres" \ - --set postgresql.postgresUser="$POSTGRES_USER" \ - --set postgresql.postgresPassword="$POSTGRES_PASSWORD" \ - --set postgresql.postgresDatabase="$POSTGRES_DB" \ - --set postgresql.imageTag="$POSTGRES_VERSION" \ - --set application.migrateCommand="$DB_MIGRATE" \ - $HELM_UPGRADE_EXTRA_ARGS \ - --namespace="$KUBE_NAMESPACE" \ - "$name" \ - chart/ - fi - - if [[ -z "$ROLLOUT_STATUS_DISABLED" ]]; then - kubectl rollout status -n "$KUBE_NAMESPACE" -w "$ROLLOUT_RESOURCE_TYPE/$name" - fi - } - - function scale() { - track="${1-stable}" - percentage="${2-100}" - name=$(deploy_name "$track") - - replicas=$(get_replicas "$track" "$percentage") - - if [[ -n "$(helm ls -q "^$name$")" ]]; then - helm upgrade --reuse-values \ - --wait \ - --set replicaCount="$replicas" \ - --namespace="$KUBE_NAMESPACE" \ - "$name" \ - chart/ - fi - } - - function install_dependencies() { - apk add -U openssl curl tar gzip bash ca-certificates git - curl -sSL -o /etc/apk/keys/sgerrand.rsa.pub https://alpine-pkgs.sgerrand.com/sgerrand.rsa.pub - curl -sSL -O https://github.com/sgerrand/alpine-pkg-glibc/releases/download/2.28-r0/glibc-2.28-r0.apk - apk add glibc-2.28-r0.apk - rm glibc-2.28-r0.apk - - curl -sS "https://kubernetes-helm.storage.googleapis.com/helm-v${HELM_VERSION}-linux-amd64.tar.gz" | tar zx - mv linux-amd64/helm /usr/bin/ - mv linux-amd64/tiller /usr/bin/ - helm version --client - tiller -version - - curl -sSL -o /usr/bin/kubectl "https://storage.googleapis.com/kubernetes-release/release/v${KUBERNETES_VERSION}/bin/linux/amd64/kubectl" - chmod +x /usr/bin/kubectl - kubectl version --client - } - - function download_chart() { - if [[ ! -d chart ]]; then - auto_chart=${AUTO_DEVOPS_CHART:-gitlab/auto-deploy-app} - auto_chart_name=$(basename $auto_chart) - auto_chart_name=${auto_chart_name%.tgz} - auto_chart_name=${auto_chart_name%.tar.gz} - else - auto_chart="chart" - auto_chart_name="chart" - fi - - helm init --client-only - helm repo add ${AUTO_DEVOPS_CHART_REPOSITORY_NAME:-gitlab} ${AUTO_DEVOPS_CHART_REPOSITORY:-https://charts.gitlab.io} ${AUTO_DEVOPS_CHART_REPOSITORY_USERNAME:+"--username" "$AUTO_DEVOPS_CHART_REPOSITORY_USERNAME"} ${AUTO_DEVOPS_CHART_REPOSITORY_PASSWORD:+"--password" "$AUTO_DEVOPS_CHART_REPOSITORY_PASSWORD"} - if [[ ! -d "$auto_chart" ]]; then - helm fetch ${auto_chart} --untar - fi - if [ "$auto_chart_name" != "chart" ]; then - mv ${auto_chart_name} chart - fi - - helm dependency update chart/ - helm dependency build chart/ - } - - function ensure_namespace() { - kubectl get namespace "$KUBE_NAMESPACE" || kubectl create namespace "$KUBE_NAMESPACE" - } - - function check_kube_domain() { - if [[ -z "$KUBE_INGRESS_BASE_DOMAIN" ]]; then - echo "In order to deploy or use Review Apps," - echo "KUBE_INGRESS_BASE_DOMAIN variables must be set" - echo "From 11.8, you can set KUBE_INGRESS_BASE_DOMAIN in cluster settings" - echo "or by defining a variable at group or project level." - echo "You can also manually add it in .gitlab-ci.yml" - false - else - true - fi - } - - function initialize_tiller() { - echo "Checking Tiller..." - - export HELM_HOST="localhost:44134" - tiller -listen ${HELM_HOST} -alsologtostderr > /dev/null 2>&1 & - echo "Tiller is listening on ${HELM_HOST}" - - if ! helm version --debug; then - echo "Failed to init Tiller." - return 1 - fi - echo "" - } - - function create_secret() { - echo "Create secret..." - if [[ "$CI_PROJECT_VISIBILITY" == "public" ]]; then - return - fi - - kubectl create secret -n "$KUBE_NAMESPACE" \ - docker-registry gitlab-registry \ - --docker-server="$CI_REGISTRY" \ - --docker-username="${CI_DEPLOY_USER:-$CI_REGISTRY_USER}" \ - --docker-password="${CI_DEPLOY_PASSWORD:-$CI_REGISTRY_PASSWORD}" \ - --docker-email="$GITLAB_USER_EMAIL" \ - -o yaml --dry-run | kubectl replace -n "$KUBE_NAMESPACE" --force -f - - } - - function persist_environment_url() { - echo $CI_ENVIRONMENT_URL > environment_url.txt - } - - function delete() { - track="${1-stable}" - name=$(deploy_name "$track") - - if [[ -n "$(helm ls -q "^$name$")" ]]; then - helm delete --purge "$name" - fi - - secret_name=$(application_secret_name "$track") - kubectl delete secret --ignore-not-found -n "$KUBE_NAMESPACE" "$secret_name" - } - -before_script: - - *deploy_helpers diff --git a/lib/gitlab/sidekiq_middleware/memory_killer.rb b/lib/gitlab/sidekiq_middleware/memory_killer.rb index 671d795ec33..4b10f921ed8 100644 --- a/lib/gitlab/sidekiq_middleware/memory_killer.rb +++ b/lib/gitlab/sidekiq_middleware/memory_killer.rb @@ -14,9 +14,12 @@ module Gitlab # shut Sidekiq down MUTEX = Mutex.new + attr_reader :worker + def call(worker, job, queue) yield + @worker = worker current_rss = get_rss return unless MAX_RSS > 0 && current_rss > MAX_RSS @@ -25,9 +28,11 @@ module Gitlab # Return if another thread is already waiting to shut Sidekiq down next unless MUTEX.try_lock - Sidekiq.logger.warn "Sidekiq worker PID-#{pid} current RSS #{current_rss}"\ - " exceeds maximum RSS #{MAX_RSS} after finishing job #{worker.class} JID-#{job['jid']}" - Sidekiq.logger.warn "Sidekiq worker PID-#{pid} will stop fetching new jobs in #{GRACE_TIME} seconds, and will be shut down #{SHUTDOWN_WAIT} seconds later" + warn("Sidekiq worker PID-#{pid} current RSS #{current_rss}"\ + " exceeds maximum RSS #{MAX_RSS} after finishing job #{worker.class} JID-#{job['jid']}") + + warn("Sidekiq worker PID-#{pid} will stop fetching new jobs"\ + " in #{GRACE_TIME} seconds, and will be shut down #{SHUTDOWN_WAIT} seconds later") # Wait `GRACE_TIME` to give the memory intensive job time to finish. # Then, tell Sidekiq to stop fetching new jobs. @@ -59,24 +64,28 @@ module Gitlab def wait_and_signal_pgroup(time, signal, explanation) return wait_and_signal(time, signal, explanation) unless Process.getpgrp == pid - Sidekiq.logger.warn "waiting #{time} seconds before sending Sidekiq worker PGRP-#{pid} #{signal} (#{explanation})" + warn("waiting #{time} seconds before sending Sidekiq worker PGRP-#{pid} #{signal} (#{explanation})", signal: signal) sleep(time) - Sidekiq.logger.warn "sending Sidekiq worker PGRP-#{pid} #{signal} (#{explanation})" + warn("sending Sidekiq worker PGRP-#{pid} #{signal} (#{explanation})", signal: signal) Process.kill(signal, 0) end def wait_and_signal(time, signal, explanation) - Sidekiq.logger.warn "waiting #{time} seconds before sending Sidekiq worker PID-#{pid} #{signal} (#{explanation})" + warn("waiting #{time} seconds before sending Sidekiq worker PID-#{pid} #{signal} (#{explanation})", signal: signal) sleep(time) - Sidekiq.logger.warn "sending Sidekiq worker PID-#{pid} #{signal} (#{explanation})" + warn("sending Sidekiq worker PID-#{pid} #{signal} (#{explanation})", signal: signal) Process.kill(signal, pid) end def pid Process.pid end + + def warn(message, signal: nil) + Sidekiq.logger.warn(class: worker.class, pid: pid, signal: signal, message: message) + end end end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index ed71a4b42d9..114d245b688 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -3941,6 +3941,12 @@ 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 "" @@ -6382,18 +6388,12 @@ 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 "Marks this issue as a duplicate of %{duplicate_reference}." msgstr "" @@ -13314,9 +13314,6 @@ msgstr "" msgid "project avatar" msgstr "" -msgid "quick actions" -msgstr "" - msgid "register" msgstr "" diff --git a/package.json b/package.json index 4264064c93d..773918524f9 100644 --- a/package.json +++ b/package.json @@ -38,7 +38,7 @@ "@babel/preset-env": "^7.4.4", "@gitlab/csslab": "^1.9.0", "@gitlab/svgs": "^1.67.0", - "@gitlab/ui": "^5.7.1", + "@gitlab/ui": "^5.9.0", "apollo-cache-inmemory": "^1.5.1", "apollo-client": "^2.5.1", "apollo-link": "^1.2.11", @@ -99,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/page/project/issue/show.rb b/qa/qa/page/project/issue/show.rb index b59540d0377..507dccb52d0 100644 --- a/qa/qa/page/project/issue/show.rb +++ b/qa/qa/page/project/issue/show.rb @@ -70,7 +70,10 @@ module QA end def select_labels_and_refresh(labels) - click_element(:edit_link_labels) + Support::Retrier.retry_until do + click_element(:edit_link_labels) + has_element?(:dropdown_menu_labels, text: labels.first) + end labels.each do |label| within_element(:dropdown_menu_labels, text: label) do diff --git a/spec/features/markdown/metrics_spec.rb b/spec/features/markdown/metrics_spec.rb new file mode 100644 index 00000000000..aa53ac50c78 --- /dev/null +++ b/spec/features/markdown/metrics_spec.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Metrics rendering', :js, :use_clean_rails_memory_store_caching do + include PrometheusHelpers + + let(:user) { create(:user) } + let(:project) { create(:prometheus_project) } + let(:environment) { create(:environment, project: project) } + let(:issue) { create(:issue, project: project, description: description) } + let(:description) { "See [metrics dashboard](#{metrics_url}) for info." } + let(:metrics_url) { metrics_project_environment_url(project, environment) } + + before do + configure_host + import_common_metrics + stub_any_prometheus_request_with_response + + project.add_developer(user) + + sign_in(user) + end + + after do + restore_host + end + + context 'with deployments and related deployable present' do + it 'shows embedded metrics' do + visit project_issue_path(project, issue) + + expect(page).to have_css('div.prometheus-graph') + expect(page).to have_text('Memory Usage (Total)') + expect(page).to have_text('Core Usage (Total)') + end + end + + def import_common_metrics + ::Gitlab::DatabaseImporters::CommonMetrics::Importer.new.execute + end + + def configure_host + @original_default_host = default_url_options[:host] + @original_gitlab_url = Gitlab.config.gitlab[:url] + + # Ensure we create a metrics url with the right host. + # Configure host for route helpers in specs (also updates root_url): + default_url_options[:host] = Capybara.server_host + + # Ensure we identify urls with the appropriate host. + # Configure host to include port in app: + Gitlab.config.gitlab[:url] = root_url.chomp('/') + end + + def restore_host + default_url_options[:host] = @original_default_host + Gitlab.config.gitlab[:url] = @original_gitlab_url + end +end diff --git a/spec/features/oauth_login_spec.rb b/spec/features/oauth_login_spec.rb index 86331728f88..54705cd51b0 100644 --- a/spec/features/oauth_login_spec.rb +++ b/spec/features/oauth_login_spec.rb @@ -34,6 +34,7 @@ describe 'OAuth Login', :js, :allow_forgery_protection do before do stub_omniauth_config(provider) + expect(ActiveSession).to receive(:cleanup).with(user).at_least(:once).and_call_original end context 'when two-factor authentication is disabled' do 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 aac095bfa6b..80741ace5d6 100644 --- a/spec/features/projects/wiki/user_creates_wiki_page_spec.rb +++ b/spec/features/projects/wiki/user_creates_wiki_page_spec.rb @@ -132,9 +132,15 @@ describe "User creates wiki page" do fill_in(:wiki_content, with: ascii_content) - page.within(".wiki-form") do - click_button("Create page") - end + # 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('.qa-create-page-button').click()") page.within ".md" do expect(page).to have_selector(".katex", count: 3).and have_content("2+2 is 4") diff --git a/spec/features/users/login_spec.rb b/spec/features/users/login_spec.rb index efba303033b..1ea88010c89 100644 --- a/spec/features/users/login_spec.rb +++ b/spec/features/users/login_spec.rb @@ -132,7 +132,6 @@ describe 'Login' do it 'does not show a "You are already signed in." error message' do expect(authentication_metrics) .to increment(:user_authenticated_counter) - .and increment(:user_session_override_counter) .and increment(:user_two_factor_authenticated_counter) enter_code(user.current_otp) @@ -144,7 +143,6 @@ describe 'Login' do it 'allows login with valid code' do expect(authentication_metrics) .to increment(:user_authenticated_counter) - .and increment(:user_session_override_counter) .and increment(:user_two_factor_authenticated_counter) enter_code(user.current_otp) @@ -170,7 +168,6 @@ describe 'Login' do it 'allows login with invalid code, then valid code' do expect(authentication_metrics) .to increment(:user_authenticated_counter) - .and increment(:user_session_override_counter) .and increment(:user_two_factor_authenticated_counter) enter_code('foo') @@ -179,6 +176,15 @@ describe 'Login' do enter_code(user.current_otp) expect(current_path).to eq root_path end + + it 'triggers ActiveSession.cleanup for the user' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + .and increment(:user_two_factor_authenticated_counter) + expect(ActiveSession).to receive(:cleanup).with(user).once.and_call_original + + enter_code(user.current_otp) + end end context 'using backup code' do @@ -195,7 +201,6 @@ describe 'Login' do it 'allows login' do expect(authentication_metrics) .to increment(:user_authenticated_counter) - .and increment(:user_session_override_counter) .and increment(:user_two_factor_authenticated_counter) enter_code(codes.sample) @@ -206,7 +211,6 @@ describe 'Login' do it 'invalidates the used code' do expect(authentication_metrics) .to increment(:user_authenticated_counter) - .and increment(:user_session_override_counter) .and increment(:user_two_factor_authenticated_counter) expect { enter_code(codes.sample) } @@ -216,7 +220,6 @@ describe 'Login' do it 'invalidates backup codes twice in a row' do expect(authentication_metrics) .to increment(:user_authenticated_counter).twice - .and increment(:user_session_override_counter).twice .and increment(:user_two_factor_authenticated_counter).twice .and increment(:user_session_destroyed_counter) @@ -230,6 +233,15 @@ describe 'Login' do expect { enter_code(codes.sample) } .to change { user.reload.otp_backup_codes.size }.by(-1) end + + it 'triggers ActiveSession.cleanup for the user' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + .and increment(:user_two_factor_authenticated_counter) + expect(ActiveSession).to receive(:cleanup).with(user).once.and_call_original + + enter_code(codes.sample) + end end context 'with invalid code' do @@ -274,7 +286,7 @@ describe 'Login' do expect(authentication_metrics) .to increment(:user_authenticated_counter) - .and increment(:user_session_override_counter) + expect(ActiveSession).to receive(:cleanup).with(user).once.and_call_original sign_in_using_saml! @@ -287,8 +299,8 @@ describe 'Login' do it 'shows 2FA prompt after OAuth login' do expect(authentication_metrics) .to increment(:user_authenticated_counter) - .and increment(:user_session_override_counter) .and increment(:user_two_factor_authenticated_counter) + expect(ActiveSession).to receive(:cleanup).with(user).once.and_call_original sign_in_using_saml! @@ -329,6 +341,14 @@ describe 'Login' do expect(page).not_to have_content(I18n.t('devise.failure.already_authenticated')) end + + it 'triggers ActiveSession.cleanup for the user' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + expect(ActiveSession).to receive(:cleanup).with(user).once.and_call_original + + gitlab_sign_in(user) + end end context 'with invalid username and password' do @@ -649,7 +669,6 @@ describe 'Login' do it 'asks the user to accept the terms' do expect(authentication_metrics) .to increment(:user_authenticated_counter) - .and increment(:user_session_override_counter) .and increment(:user_two_factor_authenticated_counter) visit new_user_session_path @@ -708,7 +727,6 @@ describe 'Login' do it 'asks the user to accept the terms before setting an email' do expect(authentication_metrics) .to increment(:user_authenticated_counter) - .and increment(:user_session_override_counter) gitlab_sign_in_via('saml', user, 'my-uid') diff --git a/spec/features/users/user_browses_projects_on_user_page_spec.rb b/spec/features/users/user_browses_projects_on_user_page_spec.rb index 6a9b281fb4c..5768f42c888 100644 --- a/spec/features/users/user_browses_projects_on_user_page_spec.rb +++ b/spec/features/users/user_browses_projects_on_user_page_spec.rb @@ -53,6 +53,19 @@ describe 'Users > User browses projects on user page', :js do expect(page).to have_content(project2.name) end + it 'does not have incorrectly interpolated message', :js do + project = create(:project, namespace: user.namespace, updated_at: 2.minutes.since) + + sign_in(user) + visit user_path(user) + click_nav_link('Personal projects') + + wait_for_requests + + expect(page).to have_content(project.name) + expect(page).not_to have_content("_('Updated')") + end + context 'when not signed in' do it 'renders user public project' do visit user_path(user) diff --git a/spec/frontend/helpers/indent_helper_spec.js b/spec/frontend/helpers/indent_helper_spec.js new file mode 100644 index 00000000000..fca12f0d1ef --- /dev/null +++ b/spec/frontend/helpers/indent_helper_spec.js @@ -0,0 +1,371 @@ +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 new file mode 100644 index 00000000000..e3d3b82d2f3 --- /dev/null +++ b/spec/frontend/lib/utils/common_utils_spec.js @@ -0,0 +1,180 @@ +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 new file mode 100644 index 00000000000..31ad0e77d6f --- /dev/null +++ b/spec/frontend/lib/utils/undo_stack_spec.js @@ -0,0 +1,237 @@ +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/badges/components/badge_list_spec.js b/spec/javascripts/badges/components/badge_list_spec.js index 2f72c9ed89d..2fa807657de 100644 --- a/spec/javascripts/badges/components/badge_list_spec.js +++ b/spec/javascripts/badges/components/badge_list_spec.js @@ -60,7 +60,7 @@ describe('BadgeList component', () => { Vue.nextTick() .then(() => { - const loadingIcon = vm.$el.querySelector('.spinner'); + const loadingIcon = vm.$el.querySelector('.gl-spinner'); expect(loadingIcon).toBeVisible(); }) diff --git a/spec/javascripts/badges/components/badge_spec.js b/spec/javascripts/badges/components/badge_spec.js index 4e4d1ae2e99..c82a03a628a 100644 --- a/spec/javascripts/badges/components/badge_spec.js +++ b/spec/javascripts/badges/components/badge_spec.js @@ -15,7 +15,7 @@ describe('Badge component', () => { const buttons = vm.$el.querySelectorAll('button'); return { badgeImage: vm.$el.querySelector('img.project-badge'), - loadingIcon: vm.$el.querySelector('.spinner'), + loadingIcon: vm.$el.querySelector('.gl-spinner'), reloadButton: buttons[buttons.length - 1], }; }; diff --git a/spec/javascripts/boards/board_list_spec.js b/spec/javascripts/boards/board_list_spec.js index 9c9b435d7fd..6774a46ed58 100644 --- a/spec/javascripts/boards/board_list_spec.js +++ b/spec/javascripts/boards/board_list_spec.js @@ -148,7 +148,7 @@ describe('Board list component', () => { component.list.loadingMore = true; Vue.nextTick(() => { - expect(component.$el.querySelector('.board-list-count .spinner')).not.toBeNull(); + expect(component.$el.querySelector('.board-list-count .gl-spinner')).not.toBeNull(); done(); }); diff --git a/spec/javascripts/registry/components/app_spec.js b/spec/javascripts/registry/components/app_spec.js index 7b9b8d2b039..e7675669f7a 100644 --- a/spec/javascripts/registry/components/app_spec.js +++ b/spec/javascripts/registry/components/app_spec.js @@ -106,7 +106,7 @@ describe('Registry List', () => { it('should render a loading spinner', done => { Vue.nextTick(() => { - expect(vm.$el.querySelector('.spinner')).not.toBe(null); + expect(vm.$el.querySelector('.gl-spinner')).not.toBe(null); done(); }); }); diff --git a/spec/javascripts/reports/components/grouped_test_reports_app_spec.js b/spec/javascripts/reports/components/grouped_test_reports_app_spec.js index a17494966a3..1f1e626ed33 100644 --- a/spec/javascripts/reports/components/grouped_test_reports_app_spec.js +++ b/spec/javascripts/reports/components/grouped_test_reports_app_spec.js @@ -34,7 +34,7 @@ describe('Grouped Test Reports App', () => { it('renders success summary text', done => { setTimeout(() => { - expect(vm.$el.querySelector('.fa-spinner')).toBeNull(); + expect(vm.$el.querySelector('.gl-spinner')).toBeNull(); expect(vm.$el.querySelector('.js-code-text').textContent.trim()).toEqual( 'Test summary contained no changed test results out of 11 total tests', ); @@ -61,7 +61,7 @@ describe('Grouped Test Reports App', () => { it('renders success summary text', done => { setTimeout(() => { - expect(vm.$el.querySelector('.spinner')).not.toBeNull(); + expect(vm.$el.querySelector('.gl-spinner')).not.toBeNull(); expect(vm.$el.querySelector('.js-code-text').textContent.trim()).toEqual( 'Test summary results are being parsed', ); @@ -81,7 +81,7 @@ describe('Grouped Test Reports App', () => { it('renders failed summary text + new badge', done => { setTimeout(() => { - expect(vm.$el.querySelector('.spinner')).toBeNull(); + expect(vm.$el.querySelector('.gl-spinner')).toBeNull(); expect(vm.$el.querySelector('.js-code-text').textContent.trim()).toEqual( 'Test summary contained 2 failed test results out of 11 total tests', ); @@ -109,7 +109,7 @@ describe('Grouped Test Reports App', () => { it('renders summary text', done => { setTimeout(() => { - expect(vm.$el.querySelector('.spinner')).toBeNull(); + expect(vm.$el.querySelector('.gl-spinner')).toBeNull(); expect(vm.$el.querySelector('.js-code-text').textContent.trim()).toEqual( 'Test summary contained 2 failed test results and 2 fixed test results out of 11 total tests', ); @@ -137,7 +137,7 @@ describe('Grouped Test Reports App', () => { it('renders summary text', done => { setTimeout(() => { - expect(vm.$el.querySelector('.spinner')).toBeNull(); + expect(vm.$el.querySelector('.gl-spinner')).toBeNull(); expect(vm.$el.querySelector('.js-code-text').textContent.trim()).toEqual( 'Test summary contained 2 fixed test results out of 11 total tests', ); @@ -190,7 +190,7 @@ describe('Grouped Test Reports App', () => { }); it('renders loading summary text with loading icon', done => { - expect(vm.$el.querySelector('.spinner')).not.toBeNull(); + expect(vm.$el.querySelector('.gl-spinner')).not.toBeNull(); expect(vm.$el.querySelector('.js-code-text').textContent.trim()).toEqual( 'Test summary results are being parsed', ); diff --git a/spec/javascripts/vue_mr_widget/components/mr_widget_status_icon_spec.js b/spec/javascripts/vue_mr_widget/components/mr_widget_status_icon_spec.js index f622f52a7b9..5aac37d28df 100644 --- a/spec/javascripts/vue_mr_widget/components/mr_widget_status_icon_spec.js +++ b/spec/javascripts/vue_mr_widget/components/mr_widget_status_icon_spec.js @@ -18,7 +18,7 @@ describe('MR widget status icon component', () => { it('renders loading icon', () => { vm = mountComponent(Component, { status: 'loading' }); - expect(vm.$el.querySelector('.mr-widget-icon span').classList).toContain('spinner'); + expect(vm.$el.querySelector('.mr-widget-icon span').classList).toContain('gl-spinner'); }); }); diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_auto_merge_failed_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_auto_merge_failed_spec.js index d93badf8cd3..55a11a72551 100644 --- a/spec/javascripts/vue_mr_widget/components/states/mr_widget_auto_merge_failed_spec.js +++ b/spec/javascripts/vue_mr_widget/components/states/mr_widget_auto_merge_failed_spec.js @@ -38,7 +38,9 @@ describe('MRWidgetAutoMergeFailed', () => { Vue.nextTick(() => { expect(vm.$el.querySelector('button').getAttribute('disabled')).toEqual('disabled'); - expect(vm.$el.querySelector('button .loading-container span').classList).toContain('spinner'); + expect(vm.$el.querySelector('button .loading-container span').classList).toContain( + 'gl-spinner', + ); done(); }); }); diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_checking_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_checking_spec.js index 96e512d222a..70c70eca746 100644 --- a/spec/javascripts/vue_mr_widget/components/states/mr_widget_checking_spec.js +++ b/spec/javascripts/vue_mr_widget/components/states/mr_widget_checking_spec.js @@ -20,7 +20,7 @@ describe('MRWidgetChecking', () => { }); it('renders loading icon', () => { - expect(vm.$el.querySelector('.mr-widget-icon span').classList).toContain('spinner'); + expect(vm.$el.querySelector('.mr-widget-icon span').classList).toContain('gl-spinner'); }); it('renders information about merging', () => { diff --git a/spec/javascripts/vue_shared/components/file_icon_spec.js b/spec/javascripts/vue_shared/components/file_icon_spec.js index 5bea8c43da3..1f61e19fa84 100644 --- a/spec/javascripts/vue_shared/components/file_icon_spec.js +++ b/spec/javascripts/vue_shared/components/file_icon_spec.js @@ -72,7 +72,7 @@ describe('File Icon component', () => { const { classList } = vm.$el.querySelector('.loading-container span'); - expect(classList.contains('spinner')).toEqual(true); + expect(classList.contains('gl-spinner')).toEqual(true); }); it('should add a special class and a size class', () => { diff --git a/spec/javascripts/vue_shared/components/header_ci_component_spec.js b/spec/javascripts/vue_shared/components/header_ci_component_spec.js index a9c1a67b39b..2b059e5e9f4 100644 --- a/spec/javascripts/vue_shared/components/header_ci_component_spec.js +++ b/spec/javascripts/vue_shared/components/header_ci_component_spec.js @@ -88,7 +88,7 @@ describe('Header CI Component', () => { vm.actions[0].isLoading = true; Vue.nextTick(() => { - expect(vm.$el.querySelector('.btn .spinner').getAttribute('style')).toBeFalsy(); + expect(vm.$el.querySelector('.btn .gl-spinner').getAttribute('style')).toBeFalsy(); done(); }); }); diff --git a/spec/lib/gitlab/sidekiq_middleware/memory_killer_spec.rb b/spec/lib/gitlab/sidekiq_middleware/memory_killer_spec.rb index 1a5a38b5d99..b451844f06c 100644 --- a/spec/lib/gitlab/sidekiq_middleware/memory_killer_spec.rb +++ b/spec/lib/gitlab/sidekiq_middleware/memory_killer_spec.rb @@ -45,6 +45,9 @@ describe Gitlab::SidekiqMiddleware::MemoryKiller do expect(subject).to receive(:sleep).with(10).ordered expect(Process).to receive(:kill).with('SIGKILL', pid).ordered + expect(Sidekiq.logger) + .to receive(:warn).with(class: 'TestWorker', message: anything, pid: pid, signal: anything).at_least(:once) + run end diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index 56bbcc4c306..dcc4b70a382 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -640,7 +640,7 @@ describe Notify do project.request_access(user) project.requesters.find_by(user_id: user.id) end - subject { described_class.member_access_requested_email('project', project_member.id, recipient.notification_email) } + subject { described_class.member_access_requested_email('project', project_member.id, recipient.id) } it_behaves_like 'an email sent from GitLab' it_behaves_like 'it should not have Gmail Actions links' @@ -737,9 +737,9 @@ describe Notify do describe 'project invitation accepted' do let(:invited_user) { create(:user, name: 'invited user') } - let(:maintainer) { create(:user).tap { |u| project.add_maintainer(u) } } + let(:recipient) { create(:user).tap { |u| project.add_maintainer(u) } } let(:project_member) do - invitee = invite_to_project(project, inviter: maintainer) + invitee = invite_to_project(project, inviter: recipient) invitee.accept_invite!(invited_user) invitee end @@ -747,6 +747,7 @@ describe Notify do subject { described_class.member_invite_accepted_email('project', project_member.id) } it_behaves_like 'an email sent from GitLab' + it_behaves_like 'an email sent to a user' it_behaves_like 'it should not have Gmail Actions links' it_behaves_like "a user cannot unsubscribe through footer link" it_behaves_like 'appearance header and footer enabled' @@ -762,16 +763,17 @@ describe Notify do end describe 'project invitation declined' do - let(:maintainer) { create(:user).tap { |u| project.add_maintainer(u) } } + let(:recipient) { create(:user).tap { |u| project.add_maintainer(u) } } let(:project_member) do - invitee = invite_to_project(project, inviter: maintainer) + invitee = invite_to_project(project, inviter: recipient) invitee.decline_invite! invitee end - subject { described_class.member_invite_declined_email('Project', project.id, project_member.invite_email, maintainer.id) } + subject { described_class.member_invite_declined_email('Project', project.id, project_member.invite_email, recipient.id) } it_behaves_like 'an email sent from GitLab' + it_behaves_like 'an email sent to a user' it_behaves_like 'it should not have Gmail Actions links' it_behaves_like "a user cannot unsubscribe through footer link" it_behaves_like 'appearance header and footer enabled' @@ -1087,9 +1089,10 @@ describe Notify do group.request_access(user) group.requesters.find_by(user_id: user.id) end - subject { described_class.member_access_requested_email('group', group_member.id, recipient.notification_email) } + subject { described_class.member_access_requested_email('group', group_member.id, recipient.id) } it_behaves_like 'an email sent from GitLab' + it_behaves_like 'an email sent to a user' it_behaves_like 'it should not have Gmail Actions links' it_behaves_like "a user cannot unsubscribe through footer link" it_behaves_like 'appearance header and footer enabled' @@ -1111,9 +1114,11 @@ describe Notify do group.request_access(user) group.requesters.find_by(user_id: user.id) end + let(:recipient) { user } subject { described_class.member_access_denied_email('group', group.id, user.id) } it_behaves_like 'an email sent from GitLab' + it_behaves_like 'an email sent to a user' it_behaves_like 'it should not have Gmail Actions links' it_behaves_like "a user cannot unsubscribe through footer link" it_behaves_like 'appearance header and footer enabled' @@ -1128,10 +1133,12 @@ describe Notify do describe 'group access changed' do let(:group_member) { create(:group_member, group: group, user: user) } + let(:recipient) { user } subject { described_class.member_access_granted_email('group', group_member.id) } it_behaves_like 'an email sent from GitLab' + it_behaves_like 'an email sent to a user' it_behaves_like 'it should not have Gmail Actions links' it_behaves_like "a user cannot unsubscribe through footer link" it_behaves_like 'appearance header and footer enabled' diff --git a/spec/support/helpers/prometheus_helpers.rb b/spec/support/helpers/prometheus_helpers.rb index 4d39d9df494..7c03746a395 100644 --- a/spec/support/helpers/prometheus_helpers.rb +++ b/spec/support/helpers/prometheus_helpers.rb @@ -76,6 +76,14 @@ module PrometheusHelpers WebMock.stub_request(:any, /prometheus.example.com/) end + def stub_any_prometheus_request_with_response(status: 200, headers: {}, body: nil) + stub_any_prometheus_request.to_return({ + status: status, + headers: { 'Content-Type' => 'application/json' }.merge(headers), + body: body || prometheus_values_body.to_json + }) + end + def stub_all_prometheus_requests(environment_slug, body: nil, status: 200) stub_prometheus_request( prometheus_query_with_time_url(prometheus_memory_query(environment_slug), Time.now.utc), diff --git a/spec/support/shared_examples/notify_shared_examples.rb b/spec/support/shared_examples/notify_shared_examples.rb index a537fab4bcd..ca031df000e 100644 --- a/spec/support/shared_examples/notify_shared_examples.rb +++ b/spec/support/shared_examples/notify_shared_examples.rb @@ -52,7 +52,7 @@ shared_examples 'an email sent to a user' do it 'is sent to user\'s group notification email' do group_notification_email = 'user+group@example.com' - create(:notification_setting, user: recipient, source: project.group, notification_email: group_notification_email) + create(:notification_setting, user: recipient, source: group, notification_email: group_notification_email) expect(subject).to deliver_to(group_notification_email) end diff --git a/yarn.lock b/yarn.lock index 4fa7665b000..7095ff706c2 100644 --- a/yarn.lock +++ b/yarn.lock @@ -996,10 +996,10 @@ resolved "https://registry.yarnpkg.com/@gitlab/svgs/-/svgs-1.67.0.tgz#c7b94eca13b99fd3aaa737fb6dcc0abc41d3c579" integrity sha512-hJOmWEs6RkjzyKkb1vc9wwKGZIBIP0coHkxu/KgOoxhBVudpGk4CH7xJ6UuB2TKpb0SEh5CC1CzRZfBYaFhsaA== -"@gitlab/ui@^5.7.1": - version "5.7.1" - resolved "https://registry.yarnpkg.com/@gitlab/ui/-/ui-5.7.1.tgz#e55d04052dd6e50ed1e90676aacc64290d62c0b6" - integrity sha512-F06/6z6/69LbKIK0PYRDTB/teSPUnF7LijHl4JiuYHXn7Y2/iVoLsAMikhT89RVR84orHPGnw16vtCPjSjBDrA== +"@gitlab/ui@^5.9.0": + version "5.9.0" + resolved "https://registry.yarnpkg.com/@gitlab/ui/-/ui-5.9.0.tgz#a38b1b57c365608b100b95969ae7a57ce1707542" + integrity sha512-cgvEPWVerYZNLqkHjg5dd0VhEDBWj8aNoISZCaGOWI9K4yVtpMPVRUv19o/xYm4vUexfFsG9vg9lBgd+4ZU6Yw== dependencies: "@babel/standalone" "^7.0.0" "@gitlab/vue-toasted" "^1.2.1" @@ -8340,7 +8340,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= |
