diff options
| author | GitLab Bot <gitlab-bot@gitlab.com> | 2022-05-17 12:08:06 +0000 |
|---|---|---|
| committer | GitLab Bot <gitlab-bot@gitlab.com> | 2022-05-17 12:08:06 +0000 |
| commit | f0a387b4a5f08d6739894455664b4d3cb1509cc6 (patch) | |
| tree | 3c5b144fbb894189dfb9a61e47c53c54af4415f4 | |
| parent | 91c2554bcf93c3c41aa830da4dd7a2d4b7483e2d (diff) | |
| download | gitlab-ce-f0a387b4a5f08d6739894455664b4d3cb1509cc6.tar.gz | |
Add latest changes from gitlab-org/gitlab@master
41 files changed, 930 insertions, 241 deletions
diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 235e115267d..a149f31c093 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -42fab8fc526215f9426bc9f459f9e6da0951c574 +6b31501b13eae70aea5061edc8273c551ba4c349 diff --git a/app/assets/javascripts/issues/show/components/app.vue b/app/assets/javascripts/issues/show/components/app.vue index e7051cec956..5c4aae45ef2 100644 --- a/app/assets/javascripts/issues/show/components/app.vue +++ b/app/assets/javascripts/issues/show/components/app.vue @@ -342,20 +342,24 @@ export default { }); }, - updateFormState(state) { + setFormState(state) { this.store.setFormState(state); }, - updateAndShowForm(templates = {}) { + updateFormState(templates = {}) { + this.setFormState({ + title: this.state.titleText, + description: this.state.descriptionText, + lock_version: this.state.lock_version, + lockedWarningVisible: false, + updateLoading: false, + issuableTemplates: templates, + }); + }, + + updateAndShowForm(templates) { if (!this.showForm) { - this.store.setFormState({ - title: this.state.titleText, - description: this.state.descriptionText, - lock_version: this.state.lock_version, - lockedWarningVisible: false, - updateLoading: false, - issuableTemplates: templates, - }); + this.updateFormState(templates); this.showForm = true; } }, @@ -388,9 +392,7 @@ export default { }, updateIssuable() { - this.store.setFormState({ - updateLoading: true, - }); + this.setFormState({ updateLoading: true }); const { store: { formState }, @@ -428,10 +430,6 @@ export default { .catch((error = {}) => { const { message, response = {} } = error; - this.store.setFormState({ - updateLoading: false, - }); - let errMsg = this.defaultErrorMessage; if (response.data && response.data.errors) { @@ -443,6 +441,9 @@ export default { this.flashContainer = createFlash({ message: errMsg, }); + }) + .finally(() => { + this.setFormState({ updateLoading: false }); }); }, @@ -461,6 +462,12 @@ export default { } }, + handleListItemReorder(description) { + this.updateFormState(); + this.setFormState({ description }); + this.updateIssuable(); + }, + taskListUpdateStarted() { this.poll.stop(); }, @@ -498,7 +505,7 @@ export default { :can-attach-file="canAttachFile" :enable-autocomplete="enableAutocomplete" :issuable-type="issuableType" - @updateForm="updateFormState" + @updateForm="setFormState" /> </div> <div v-else> @@ -573,6 +580,8 @@ export default { :issuable-type="issuableType" :update-url="updateEndpoint" :lock-version="state.lock_version" + :is-updating="formState.updateLoading" + @listItemReorder="handleListItemReorder" @taskListUpdateStarted="taskListUpdateStarted" @taskListUpdateSucceeded="taskListUpdateSucceeded" @taskListUpdateFailed="taskListUpdateFailed" diff --git a/app/assets/javascripts/issues/show/components/description.vue b/app/assets/javascripts/issues/show/components/description.vue index 831cef66836..4f97458dcd1 100644 --- a/app/assets/javascripts/issues/show/components/description.vue +++ b/app/assets/javascripts/issues/show/components/description.vue @@ -7,6 +7,7 @@ import { GlModalDirective, } from '@gitlab/ui'; import $ from 'jquery'; +import Sortable from 'sortablejs'; import Vue from 'vue'; import { getIdFromGraphQLId, convertToGraphQLId } from '~/graphql_shared/utils'; import { TYPE_WORK_ITEM } from '~/graphql_shared/constants'; @@ -14,6 +15,7 @@ import createFlash from '~/flash'; import { isPositiveInteger } from '~/lib/utils/number_utils'; import { getParameterByName, setUrlParams, updateHistory } from '~/lib/utils/url_utility'; import { __, s__, sprintf } from '~/locale'; +import { getSortableDefaultOptions, isDragging } from '~/sortable/utils'; import TaskList from '~/task_list'; import Tracking from '~/tracking'; import workItemQuery from '~/work_items/graphql/work_item.query.graphql'; @@ -22,9 +24,14 @@ import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import WorkItemDetailModal from '~/work_items/components/work_item_detail_modal.vue'; import CreateWorkItem from '~/work_items/pages/create_work_item.vue'; import animateMixin from '../mixins/animate'; +import { convertDescriptionWithNewSort } from '../utils'; Vue.use(GlToast); +const workItemTypes = { + TASK: 'task', +}; + export default { directives: { SafeHtml, @@ -76,6 +83,11 @@ export default { required: false, default: null, }, + isUpdating: { + type: Boolean, + required: false, + default: false, + }, }, data() { const workItemId = getParameterByName('work_item_id'); @@ -146,6 +158,9 @@ export default { this.openWorkItemDetailModal(taskLink); } }, + beforeDestroy() { + this.removeAllPointerEventListeners(); + }, methods: { renderGFM() { $(this.$refs['gfm-content']).renderGFM(); @@ -161,9 +176,67 @@ export default { onSuccess: this.taskListUpdateSuccess.bind(this), onError: this.taskListUpdateError.bind(this), }); + + this.renderSortableLists(); } }, + renderSortableLists() { + this.removeAllPointerEventListeners(); + + const lists = document.querySelectorAll('.description ul, .description ol'); + lists.forEach((list) => { + Array.from(list.children).forEach((listItem) => { + listItem.prepend(this.createDragIconElement()); + this.addPointerEventListeners(listItem); + }); + + Sortable.create( + list, + getSortableDefaultOptions({ + handle: '.drag-icon', + onUpdate: (event) => { + const description = convertDescriptionWithNewSort(this.descriptionText, event.to); + this.$emit('listItemReorder', description); + }, + }), + ); + }); + }, + createDragIconElement() { + const container = document.createElement('div'); + container.innerHTML = `<svg class="drag-icon s14 gl-icon gl-cursor-grab gl-visibility-hidden" role="img" aria-hidden="true"> + <use href="${gon.sprite_icons}#drag-vertical"></use> + </svg>`; + return container.firstChild; + }, + addPointerEventListeners(listItem) { + const pointeroverListener = (event) => { + if (isDragging() || this.isUpdating) { + return; + } + event.target.closest('li').querySelector('.drag-icon').style.visibility = 'visible'; // eslint-disable-line no-param-reassign + }; + const pointeroutListener = (event) => { + event.target.closest('li').querySelector('.drag-icon').style.visibility = 'hidden'; // eslint-disable-line no-param-reassign + }; + + // We use pointerover/pointerout instead of CSS so that when we hover over a + // list item with children, the drag icons of its children do not become visible. + listItem.addEventListener('pointerover', pointeroverListener); + listItem.addEventListener('pointerout', pointeroutListener); + this.pointerEventListeners = this.pointerEventListeners || new Map(); + this.pointerEventListeners.set(listItem, [ + { type: 'pointerover', listener: pointeroverListener }, + { type: 'pointerout', listener: pointeroutListener }, + ]); + }, + removeAllPointerEventListeners() { + this.pointerEventListeners?.forEach((events, listItem) => { + events.forEach((event) => listItem.removeEventListener(event.type, event.listener)); + this.pointerEventListeners.delete(listItem); + }); + }, taskListUpdateStarted() { this.$emit('taskListUpdateStarted'); }, @@ -214,7 +287,10 @@ export default { taskListFields.forEach((item, index) => { const taskLink = item.querySelector('.gfm-issue'); if (taskLink) { - const { issue, referenceType } = taskLink.dataset; + const { issue, referenceType, issueType } = taskLink.dataset; + if (issueType !== workItemTypes.TASK) { + return; + } const workItemId = convertToGraphQLId(TYPE_WORK_ITEM, issue); this.addHoverListeners(taskLink, workItemId); taskLink.addEventListener('click', (e) => { @@ -237,10 +313,9 @@ export default { 'btn-md', 'gl-button', 'btn-default-tertiary', - 'gl-left-0', 'gl-p-0!', - 'gl-top-2', - 'gl-absolute', + 'gl-mt-n1', + 'gl-ml-3', 'js-add-task', ); button.id = `js-task-button-${index}`; @@ -252,7 +327,7 @@ export default { `; button.setAttribute('aria-label', s__('WorkItem|Convert to work item')); button.addEventListener('click', () => this.openCreateTaskModal(button)); - item.prepend(button); + item.append(button); }); }, addHoverListeners(taskLink, id) { diff --git a/app/assets/javascripts/issues/show/utils.js b/app/assets/javascripts/issues/show/utils.js new file mode 100644 index 00000000000..60e66f59f92 --- /dev/null +++ b/app/assets/javascripts/issues/show/utils.js @@ -0,0 +1,99 @@ +import { COLON, HYPHEN, NEWLINE } from '~/lib/utils/text_utility'; + +/** + * Get the index from sourcepos that represents the line of + * the description when the description is split by newline. + * + * @param {String} sourcepos Source position in format `23:3-23:14` + * @returns {Number} Index of description split by newline + */ +const getDescriptionIndex = (sourcepos) => { + const [startRange] = sourcepos.split(HYPHEN); + const [startRow] = startRange.split(COLON); + return startRow - 1; +}; + +/** + * Given a `ul` or `ol` element containing a new sort order, this function performs + * a depth-first search to get the new sort order in the form of sourcepos indices. + * + * @param {HTMLElement} list A `ul` or `ol` element containing a new sort order + * @returns {Array<Number>} An array representing the new order of the list + */ +const getNewSourcePositions = (list) => { + const newSourcePositions = []; + + function pushPositionOfChildListItems(el) { + if (!el) { + return; + } + if (el.tagName === 'LI') { + newSourcePositions.push(getDescriptionIndex(el.dataset.sourcepos)); + } + Array.from(el.children).forEach(pushPositionOfChildListItems); + } + + pushPositionOfChildListItems(list); + + return newSourcePositions; +}; + +/** + * Converts a description to one with a new list sort order. + * + * Given a description like: + * + * <pre> + * 1. I am text + * 2. + * 3. - Item 1 + * 4. - Item 2 + * 5. - Item 3 + * 6. - Item 4 + * 7. - Item 5 + * </pre> + * + * And a reordered list (due to dragging Item 2 into Item 1's position) like: + * + * <pre> + * <ul data-sourcepos="3:1-8:0"> + * <li data-sourcepos="4:1-4:8"> + * Item 2 + * <ul data-sourcepos="5:1-6:10"> + * <li data-sourcepos="5:1-5:10">Item 3</li> + * <li data-sourcepos="6:1-6:10">Item 4</li> + * </ul> + * </li> + * <li data-sourcepos="3:1-3:8">Item 1</li> + * <li data-sourcepos="7:1-8:0">Item 5</li> + * <ul> + * </pre> + * + * This function returns: + * + * <pre> + * 1. I am text + * 2. + * 3. - Item 2 + * 4. - Item 3 + * 5. - Item 4 + * 6. - Item 1 + * 7. - Item 5 + * </pre> + * + * @param {String} description Description in markdown format + * @param {HTMLElement} list A `ul` or `ol` element containing a new sort order + * @returns {String} Markdown with a new list sort order + */ +export const convertDescriptionWithNewSort = (description, list) => { + const descriptionLines = description.split(NEWLINE); + const startIndexOfList = getDescriptionIndex(list.dataset.sourcepos); + + getNewSourcePositions(list) + .map((lineIndex) => descriptionLines[lineIndex]) + .forEach((line, index) => { + descriptionLines[startIndexOfList + index] = line; + }); + + return descriptionLines.join(NEWLINE); +}; diff --git a/app/assets/javascripts/jobs/components/job_app.vue b/app/assets/javascripts/jobs/components/job_app.vue index 85fe5ed7e26..396b015ad83 100644 --- a/app/assets/javascripts/jobs/components/job_app.vue +++ b/app/assets/javascripts/jobs/components/job_app.vue @@ -94,7 +94,7 @@ export default { 'emptyStateIllustration', 'isScrollingDown', 'emptyStateAction', - 'hasRunnersForProject', + 'hasOfflineRunnersForProject', ]), shouldRenderContent() { @@ -220,7 +220,7 @@ export default { <!-- Body Section --> <stuck-block v-if="job.stuck" - :has-no-runners-for-project="hasRunnersForProject" + :has-offline-runners-for-project="hasOfflineRunnersForProject" :tags="job.tags" :runners-path="runnerSettingsUrl" /> diff --git a/app/assets/javascripts/jobs/components/stuck_block.vue b/app/assets/javascripts/jobs/components/stuck_block.vue index abd0c13702a..f9cde61e917 100644 --- a/app/assets/javascripts/jobs/components/stuck_block.vue +++ b/app/assets/javascripts/jobs/components/stuck_block.vue @@ -11,7 +11,7 @@ export default { GlLink, }, props: { - hasNoRunnersForProject: { + hasOfflineRunnersForProject: { type: Boolean, required: true, }, @@ -37,7 +37,7 @@ export default { dataTestId: 'job-stuck-with-tags', showTags: true, }; - } else if (this.hasNoRunnersForProject) { + } else if (this.hasOfflineRunnersForProject) { return { text: s__(`Job|This job is stuck because the project doesn't have any runners online assigned to it.`), diff --git a/app/assets/javascripts/jobs/store/getters.js b/app/assets/javascripts/jobs/store/getters.js index 5849a31e9e6..a0f9db7409d 100644 --- a/app/assets/javascripts/jobs/store/getters.js +++ b/app/assets/javascripts/jobs/store/getters.js @@ -46,5 +46,5 @@ export const shouldRenderSharedRunnerLimitWarning = (state) => export const isScrollingDown = (state) => isScrolledToBottom() && !state.isJobLogComplete; -export const hasRunnersForProject = (state) => +export const hasOfflineRunnersForProject = (state) => state?.job?.runners?.available && !state?.job?.runners?.online; diff --git a/app/assets/javascripts/lib/utils/text_utility.js b/app/assets/javascripts/lib/utils/text_utility.js index 419afa0a0a9..dad9cbcb6f6 100644 --- a/app/assets/javascripts/lib/utils/text_utility.js +++ b/app/assets/javascripts/lib/utils/text_utility.js @@ -6,6 +6,10 @@ import { } from '~/lib/utils/constants'; import { allSingleQuotes } from '~/lib/utils/regexp'; +export const COLON = ':'; +export const HYPHEN = '-'; +export const NEWLINE = '\n'; + /** * Adds a , to a string composed by numbers, at every 3 chars. * diff --git a/app/assets/javascripts/sortable/constants.js b/app/assets/javascripts/sortable/constants.js index 7fddac00ab2..f5bb0a3b11f 100644 --- a/app/assets/javascripts/sortable/constants.js +++ b/app/assets/javascripts/sortable/constants.js @@ -1,3 +1,5 @@ +export const DRAG_CLASS = 'is-dragging'; + /** * Default config options for sortablejs. * @type {object} @@ -12,7 +14,7 @@ export const defaultSortableOptions = { animation: 200, forceFallback: true, - fallbackClass: 'is-dragging', + fallbackClass: DRAG_CLASS, fallbackOnBody: true, ghostClass: 'is-ghost', fallbackTolerance: 1, diff --git a/app/assets/javascripts/sortable/utils.js b/app/assets/javascripts/sortable/utils.js index c2c8fb03b58..88ac1295a39 100644 --- a/app/assets/javascripts/sortable/utils.js +++ b/app/assets/javascripts/sortable/utils.js @@ -1,13 +1,17 @@ /* global DocumentTouch */ -import { defaultSortableOptions } from './constants'; +import { defaultSortableOptions, DRAG_CLASS } from './constants'; export function sortableStart() { - document.body.classList.add('is-dragging'); + document.body.classList.add(DRAG_CLASS); } export function sortableEnd() { - document.body.classList.remove('is-dragging'); + document.body.classList.remove(DRAG_CLASS); +} + +export function isDragging() { + return document.body.classList.contains(DRAG_CLASS); } export function getSortableDefaultOptions(options) { diff --git a/app/assets/javascripts/vue_shared/components/markdown/toolbar.vue b/app/assets/javascripts/vue_shared/components/markdown/toolbar.vue index 1d83a61f2ae..6c99a749edc 100644 --- a/app/assets/javascripts/vue_shared/components/markdown/toolbar.vue +++ b/app/assets/javascripts/vue_shared/components/markdown/toolbar.vue @@ -42,21 +42,30 @@ export default { <div v-if="showCommentToolBar" class="comment-toolbar clearfix"> <div class="toolbar-text"> <template v-if="!hasQuickActionsDocsPath && markdownDocsPath"> - <gl-link :href="markdownDocsPath" target="_blank"> - {{ __('Markdown is supported') }} - </gl-link> + <gl-sprintf + :message=" + s__('MarkdownToolbar|Supports %{markdownDocsLinkStart}Markdown%{markdownDocsLinkEnd}') + " + > + <template #markdownDocsLink="{ content }"> + <gl-link :href="markdownDocsPath" target="_blank">{{ content }}</gl-link> + </template> + </gl-sprintf> </template> <template v-if="hasQuickActionsDocsPath && markdownDocsPath"> <gl-sprintf :message=" - __( - '%{markdownDocsLinkStart}Markdown%{markdownDocsLinkEnd} and %{quickActionsDocsLinkStart}quick actions%{quickActionsDocsLinkEnd} are supported', + s__( + 'NoteToolbar|Supports %{markdownDocsLinkStart}Markdown%{markdownDocsLinkEnd}. For %{quickActionsDocsLinkStart}quick actions%{quickActionsDocsLinkEnd}, type %{keyboardStart}/%{keyboardEnd}.', ) " > <template #markdownDocsLink="{ content }"> <gl-link :href="markdownDocsPath" target="_blank">{{ content }}</gl-link> </template> + <template #keyboard="{ content }"> + <kbd>{{ content }}</kbd> + </template> <template #quickActionsDocsLink="{ content }"> <gl-link :href="quickActionsDocsPath" target="_blank">{{ content }}</gl-link> </template> diff --git a/app/assets/stylesheets/errors.scss b/app/assets/stylesheets/errors.scss index dc08c816d7d..00cc3409fa7 100644 --- a/app/assets/stylesheets/errors.scss +++ b/app/assets/stylesheets/errors.scss @@ -118,3 +118,11 @@ a { } } } + +.tanuki-logo { + width: 210px; + height: 210px; + max-width: 40vw; + display: block; + margin: map-get($spacing-scale, 4) auto; +} diff --git a/app/assets/stylesheets/page_bundles/issues_show.scss b/app/assets/stylesheets/page_bundles/issues_show.scss index 1642e6a39f3..9873a0121c0 100644 --- a/app/assets/stylesheets/page_bundles/issues_show.scss +++ b/app/assets/stylesheets/page_bundles/issues_show.scss @@ -1,10 +1,46 @@ @import 'mixins_and_variables_and_functions'; -.description.work-items-enabled { +.description { + ul, + ol { + /* We're changing list-style-position to inside because the default of outside + * doesn't move the negative margin to the left of the bullet. */ + list-style-position: inside; + } + + li { + position: relative; + /* In the browser, the li element comes after (to the right of) the bullet point, so hovering + * over the left of the bullet point doesn't trigger a row hover. To trigger hovering on the + * left, we're applying negative margin here to shift the li element left. */ + margin-inline-start: -1rem; + padding-inline-start: 2.5rem; + + .drag-icon { + position: absolute; + inset-block-start: 0.3rem; + inset-inline-start: 1rem; + } + } + ul.task-list { > li.task-list-item { + /* We're using !important to override the same selector in typography.scss */ + margin-inline-start: -1rem !important; padding-inline-start: 2.5rem; + > input.task-list-item-checkbox { + position: static; + vertical-align: middle; + margin-block-start: -2px; + } + } + } +} + +.description.work-items-enabled { + ul.task-list { + > li.task-list-item { .js-add-task { svg { visibility: hidden; @@ -15,10 +51,6 @@ } } - > input.task-list-item-checkbox { - left: 1.25rem; - } - &:hover, &:focus-within { .js-add-task svg { @@ -28,3 +60,8 @@ } } } + +.is-ghost { + opacity: 0.3; + pointer-events: none; +} diff --git a/app/controllers/pwa_controller.rb b/app/controllers/pwa_controller.rb new file mode 100644 index 00000000000..ea14dfb27b3 --- /dev/null +++ b/app/controllers/pwa_controller.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +class PwaController < ApplicationController # rubocop:disable Gitlab/NamespacedClass + layout 'errors' + + feature_category :navigation + + skip_before_action :authenticate_user! + + def offline + end +end diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb index a2730c53ea2..e1c9e7d3896 100644 --- a/app/helpers/merge_requests_helper.rb +++ b/app/helpers/merge_requests_helper.rb @@ -246,13 +246,13 @@ module MergeRequestsHelper '' end - link_to branch, branch_path, title: branch, class: 'gl-link gl-font-monospace gl-bg-blue-50 gl-rounded-base gl-font-sm gl-p-2 gl-display-inline-block gl-text-truncate gl-max-w-26 gl-mb-n3' + link_to branch, branch_path, title: branch, class: 'gl-link gl-font-monospace gl-bg-blue-50 gl-rounded-base gl-font-sm gl-px-2 gl-display-inline-block gl-text-truncate gl-max-w-26 gl-mb-n2' end def merge_request_header(project, merge_request) link_to_author = link_to_member(project, merge_request.author, size: 24, extra_class: 'gl-font-weight-bold', avatar: false) copy_button = clipboard_button(text: merge_request.source_branch, title: _('Copy branch name'), class: 'btn btn-default btn-sm gl-button btn-default-tertiary btn-icon gl-display-none! gl-md-display-inline-block! js-source-branch-copy') - target_branch = link_to merge_request.target_branch, project_tree_path(merge_request.target_project, merge_request.target_branch), title: merge_request.target_branch, class: 'gl-link gl-font-monospace gl-bg-blue-50 gl-rounded-base gl-font-sm gl-p-2 gl-display-inline-block gl-text-truncate gl-max-w-26 gl-mb-n3' + target_branch = link_to merge_request.target_branch, project_tree_path(merge_request.target_project, merge_request.target_branch), title: merge_request.target_branch, class: 'gl-link gl-font-monospace gl-bg-blue-50 gl-rounded-base gl-font-sm gl-px-2 gl-display-inline-block gl-text-truncate gl-max-w-26 gl-mb-n2' _('%{author} requested to merge %{source_branch} %{copy_button} into %{target_branch} %{created_at}').html_safe % { author: link_to_author.html_safe, source_branch: merge_request_source_branch(merge_request).html_safe, copy_button: copy_button.html_safe, target_branch: target_branch.html_safe, created_at: time_ago_with_tooltip(merge_request.created_at, html_class: 'gl-display-inline-block').html_safe } end diff --git a/app/views/pwa/offline.html.haml b/app/views/pwa/offline.html.haml new file mode 100644 index 00000000000..5eae546bea9 --- /dev/null +++ b/app/views/pwa/offline.html.haml @@ -0,0 +1,31 @@ += link_to root_path do + = render 'shared/logo.svg' +%h1= _('Offline') +.container + %h3= _('You are currently offline, or the GitLab instance is not reachable.') + %p= _("In the background, we're attempting to connect you again.") + -# haml-lint:disable InlineJavaScript + :javascript + window.addEventListener('online', () => { + window.location.reload(); + }); + + async function checkNetworkAndReload() { + try { + const response = await fetch('.'); + // Verify we get a valid response from the server + if (response.status >= 200 && response.status < 500) { + window.location.reload(); + return; + } + } catch { + // Unable to connect to the server, ignore. + } + window.setTimeout(checkNetworkAndReload, 2500); + } + + if (window.location.pathname.endsWith('/-/offline')) { + return; + } + + checkNetworkAndReload(); diff --git a/app/views/shared/notes/_hints.html.haml b/app/views/shared/notes/_hints.html.haml index 8a79a17b166..c845d4df7df 100644 --- a/app/views/shared/notes/_hints.html.haml +++ b/app/views/shared/notes/_hints.html.haml @@ -2,15 +2,12 @@ - supports_file_upload = local_assigns.fetch(:supports_file_upload, true) .comment-toolbar.clearfix .toolbar-text - = link_to _('Markdown'), help_page_path('user/markdown'), target: '_blank', rel: 'noopener noreferrer' + - markdownLinkStart = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: help_page_path('user/markdown') } + - quickActionsLinkStart = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: help_page_path('user/project/quick_actions') } - if supports_quick_actions - and - = link_to _('quick actions'), help_page_path('user/project/quick_actions'), target: '_blank', rel: 'noopener noreferrer' - are + = html_escape(s_('NoteToolbar|Supports %{markdownDocsLinkStart}Markdown%{markdownDocsLinkEnd}. For %{quickActionsDocsLinkStart}quick actions%{quickActionsDocsLinkEnd}, type %{keyboardStart}/%{keyboardEnd}.')) % { markdownDocsLinkStart: markdownLinkStart, markdownDocsLinkEnd: '</a>'.html_safe, quickActionsDocsLinkStart: quickActionsLinkStart, quickActionsDocsLinkEnd: '</a>'.html_safe, keyboardStart: '<kbd>'.html_safe, keyboardEnd: '</kbd>'.html_safe } - else - is - supported - + = html_escape(s_('MarkdownToolbar|Supports %{markdownDocsLinkStart}Markdown%{markdownDocsLinkEnd}')) % { markdownDocsLinkStart: markdownLinkStart, markdownDocsLinkEnd: '</a>'.html_safe } - if supports_file_upload %span.uploading-container %span.uploading-progress-container.hide diff --git a/config/feature_flags/development/automated_email_provision.yml b/config/feature_flags/development/automated_email_provision.yml index 2b3fee208ab..2734413dd8f 100644 --- a/config/feature_flags/development/automated_email_provision.yml +++ b/config/feature_flags/development/automated_email_provision.yml @@ -5,4 +5,4 @@ rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/348317 milestone: '14.6' type: development group: group::license -default_enabled: false +default_enabled: true diff --git a/config/routes.rb b/config/routes.rb index 4154c0270c6..b2b4bece68a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -116,6 +116,8 @@ Rails.application.routes.draw do get '/whats_new' => 'whats_new#index' + get 'offline' => "pwa#offline" + # '/-/health' implemented by BasicHealthCheck middleware get 'liveness' => 'health#liveness' get 'readiness' => 'health#readiness' diff --git a/data/removals/15_0/15-0-secret-detection-configurations.yml b/data/removals/15_0/15-0-secret-detection-configurations.yml new file mode 100644 index 00000000000..0f759fae11c --- /dev/null +++ b/data/removals/15_0/15-0-secret-detection-configurations.yml @@ -0,0 +1,30 @@ +- name: "Secret Detection configuration variables" + announcement_milestone: "14.8" + announcement_date: "2022-02-22" + removal_milestone: "15.0" + removal_date: "2022-05-22" + breaking_change: false + reporter: connorgilbert + stage: Secure + issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/352565 + body: | # (required) Do not modify this line, instead modify the lines below. + To make it simpler and more reliable to [customize GitLab Secret Detection](https://docs.gitlab.com/ee/user/application_security/secret_detection/#customizing-settings), we've removed some of the variables that you could previously set in your CI/CD configuration. + + The following variables previously allowed you to customize the options for historical scanning, but interacted poorly with the [GitLab-managed CI/CD template](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/ci/templates/Security/Secret-Detection.gitlab-ci.yml) and are now removed: + + - `SECRET_DETECTION_COMMIT_FROM` + - `SECRET_DETECTION_COMMIT_TO` + - `SECRET_DETECTION_COMMITS` + - `SECRET_DETECTION_COMMITS_FILE` + + The `SECRET_DETECTION_ENTROPY_LEVEL` previously allowed you to configure rules that only considered the entropy level of strings in your codebase, and is now removed. + This type of entropy-only rule created an unacceptable number of incorrect results (false positives). + + You can still customize the behavior of the Secret Detection analyzer using the [available CI/CD variables](https://docs.gitlab.com/ee/user/application_security/secret_detection/#available-cicd-variables). + + For further details, see [the deprecation issue for this change](https://gitlab.com/gitlab-org/gitlab/-/issues/352565). +# The following items are not published on the docs page, but may be used in the future. + tiers: [Free, Silver, Gold, Core, Premium, Ultimate] + documentation_url: https://docs.gitlab.com/ee/user/application_security/secret_detection/#available-cicd-variables # (optional) This is a link to the current documentation page + image_url: # (optional) This is a link to a thumbnail image depicting the feature + video_url: # (optional) Use the youtube thumbnail URL with the structure of https://img.youtube.com/vi/UNIQUEID/hqdefault.jpg diff --git a/doc/development/snowplow/troubleshooting.md b/doc/development/snowplow/troubleshooting.md index 597e0436c67..2a6db80a6f2 100644 --- a/doc/development/snowplow/troubleshooting.md +++ b/doc/development/snowplow/troubleshooting.md @@ -48,3 +48,27 @@ Already conducted investigations: ### Troubleshooting data warehouse layer Reach out to [Data team](https://about.gitlab.com/handbook/business-technology/data-team/) to ask about current state of data warehouse. On their handbook page there is a [section with contact details](https://about.gitlab.com/handbook/business-technology/data-team/#how-to-connect-with-us) + +## Delay in Snowplow Enrichers + +If there is an alert for **Snowplow Raw Good Stream Backing Up**, we receive an email notification. This sometimes happens because Snowplow Enrichers don't scale well enough for the amount of Snowplow events. + +If the delay goes over 48 hours, we lose data. + +### Contact SRE on-call + +Send a message in the [#infrastructure_lounge](https://gitlab.slack.com/archives/CB3LSMEJV) Slack channel using the following template: + +```markdown +Hello team! + +We received an alert for [Snowplow Raw Good Stream Backing Up](https://us-east-1.console.aws.amazon.com/cloudwatch/home?region=us-east-1#alarmsV2:alarm/SnowPlow+Raw+Good+Stream+Backing+Up?). + +Enrichers are not scalling well for the amount of events we receive. + +See the [dashboard](https://us-east-1.console.aws.amazon.com/cloudwatch/home?region=us-east-1#dashboards:name=SnowPlow). + +Could we get assistance in order to fix the delay? + +Thank you! +``` diff --git a/doc/update/removals.md b/doc/update/removals.md index cbb86f721a6..79e03f5b5eb 100644 --- a/doc/update/removals.md +++ b/doc/update/removals.md @@ -512,6 +512,24 @@ changes to your code, settings, or workflow. Long term service and support (LTSS) for SUSE Linux Enterprise Server (SLES) 12 SP2 [ended on March 31, 2021](https://www.suse.com/lifecycle/). The CA certificates on SP2 include the expired DST root certificate, and it's not getting new CA certificate package updates. We have implemented some [workarounds](https://gitlab.com/gitlab-org/gitlab-omnibus-builder/-/merge_requests/191), but we will not be able to continue to keep the build running properly. +### Secret Detection configuration variables + +To make it simpler and more reliable to [customize GitLab Secret Detection](https://docs.gitlab.com/ee/user/application_security/secret_detection/#customizing-settings), we've removed some of the variables that you could previously set in your CI/CD configuration. + +The following variables previously allowed you to customize the options for historical scanning, but interacted poorly with the [GitLab-managed CI/CD template](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/ci/templates/Security/Secret-Detection.gitlab-ci.yml) and are now removed: + +- `SECRET_DETECTION_COMMIT_FROM` +- `SECRET_DETECTION_COMMIT_TO` +- `SECRET_DETECTION_COMMITS` +- `SECRET_DETECTION_COMMITS_FILE` + +The `SECRET_DETECTION_ENTROPY_LEVEL` previously allowed you to configure rules that only considered the entropy level of strings in your codebase, and is now removed. +This type of entropy-only rule created an unacceptable number of incorrect results (false positives). + +You can still customize the behavior of the Secret Detection analyzer using the [available CI/CD variables](https://docs.gitlab.com/ee/user/application_security/secret_detection/#available-cicd-variables). + +For further details, see [the deprecation issue for this change](https://gitlab.com/gitlab-org/gitlab/-/issues/352565). + ### Sidekiq configuration for metrics and health checks WARNING: diff --git a/doc/user/application_security/container_scanning/index.md b/doc/user/application_security/container_scanning/index.md index f50042975b3..d0487dc57ba 100644 --- a/doc/user/application_security/container_scanning/index.md +++ b/doc/user/application_security/container_scanning/index.md @@ -184,8 +184,8 @@ include: The `CS_DISABLE_DEPENDENCY_LIST` CI/CD variable controls whether the scan creates a [Dependency List](../dependency_list/) -report. The variable's default setting of `false` causes the scan to create the report. To disable -the report, set the variable to `true`: +report. This variable is currently only supported when the `trivy` analyzer is used. The variable's default setting of `"false"` causes the scan to create the report. To disable +the report, set the variable to `"true"`: For example: diff --git a/doc/user/application_security/secret_detection/index.md b/doc/user/application_security/secret_detection/index.md index 0a18e7d5f45..3937cbd77b6 100644 --- a/doc/user/application_security/secret_detection/index.md +++ b/doc/user/application_security/secret_detection/index.md @@ -195,13 +195,18 @@ Secret Detection can be customized by defining available CI/CD variables: | CI/CD variable | Default value | Description | |-----------------------------------|---------------|-------------| -| `SECRET_DETECTION_COMMIT_FROM` | - | The commit a Gitleaks scan starts at. [Removed](https://gitlab.com/gitlab-org/gitlab/-/issues/243564) in GitLab 13.5. Replaced with `SECRET_DETECTION_COMMITS`. | -| `SECRET_DETECTION_COMMIT_TO` | - | The commit a Gitleaks scan ends at. [Removed](https://gitlab.com/gitlab-org/gitlab/-/issues/243564) in GitLab 13.5. Replaced with `SECRET_DETECTION_COMMITS`. | -| `SECRET_DETECTION_COMMITS` | - | The list of commits that Gitleaks should scan. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/243564) in GitLab 13.5. | | `SECRET_DETECTION_EXCLUDED_PATHS` | "" | Exclude vulnerabilities from output based on the paths. This is a comma-separated list of patterns. Patterns can be globs, or file or folder paths (for example, `doc,spec` ). Parent directories also match patterns. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/225273) in GitLab 13.3. | | `SECRET_DETECTION_HISTORIC_SCAN` | false | Flag to enable a historic Gitleaks scan. | | `SECRET_DETECTION_IMAGE_SUFFIX` | Suffix added to the image name. If set to `-fips`, `FIPS-enabled` images are used for scan. See [FIPS-enabled images](#fips-enabled-images) for more details. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/355519) in GitLab 14.10. | +In previous GitLab versions, the following variables were also available: + +| CI/CD variable | Default value | Description | +|-----------------------------------|---------------|-------------| +| `SECRET_DETECTION_COMMIT_FROM` | - | The commit a Gitleaks scan starts at. [Removed](https://gitlab.com/gitlab-org/gitlab/-/issues/243564) in GitLab 13.5. Replaced with `SECRET_DETECTION_COMMITS`. | +| `SECRET_DETECTION_COMMIT_TO` | - | The commit a Gitleaks scan ends at. [Removed](https://gitlab.com/gitlab-org/gitlab/-/issues/243564) in GitLab 13.5. Replaced with `SECRET_DETECTION_COMMITS`. | +| `SECRET_DETECTION_COMMITS` | - | The list of commits that Gitleaks should scan. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/243564) in GitLab 13.5. [Removed](https://gitlab.com/gitlab-org/gitlab/-/issues/352565) in GitLab 15.0. | + ### Custom rulesets **(ULTIMATE)** > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/211387) in GitLab 13.5. diff --git a/doc/user/free_user_limit.md b/doc/user/free_user_limit.md new file mode 100644 index 00000000000..2340ef254f6 --- /dev/null +++ b/doc/user/free_user_limit.md @@ -0,0 +1,60 @@ +--- +stage: Growth +group: Conversion +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments +--- + +# Free user limit **(FREE SAAS)** + +In GitLab 15.1 (June 22, 2022) and later, namespaces in GitLab.com on the Free tier +will be limited to five (5) members per [namespace](group/index.md#namespaces). +This limit applies to top-level groups and personal namespaces. + +In a personal namespace, the limit applies across all projects in your personal +namespace. + +On the transition date, if your namespace has six or more unique members: + +- Five members will keep a status of `Active`. +- Remaining members will get a status of `Over limit` and lose access to the + group. +- Members invited through a group or project invitation outside of the namespace + will be removed. You can add these members back by inviting them through their + username or email address on the **Members** page for your group or project. + +## How active members are determined + +On the transition date, we'll automatically select the members who keep their `Active` status +in the following order, until we reach a total of five: + +1. Members with the Owner or Maintainer role. +1. The most recently active members. + +## Manage members in your namespace + +To help manage your free user limit, +you can view and manage the total number of members across all projects and groups +in your namespace. + +Prerequisite: + +- You must have the Owner role for the group. + +1. On the top bar, select **Menu > Groups** and find your group. +1. On the left sidebar, select **Settings > Usage Quotas**. +1. To view all members, select the **Seats** tab. +1. To remove a member, select **Remove user**. + +NOTE: +The **Usage Quotas** page is not available for personal namespaces. You can +view and [remove members](project/members/index.md#remove-a-member-from-a-project) +in each project instead. The five user limit includes all +unique members across all projects in your personal namespace. + +If you need more time to manage your members, or to try GitLab features +with a team of more than five members, you can [start a trial](https://about.gitlab.com/free-trial/). +A trial lasts for 30 days and includes an unlimited number of members. + +## Related topics + +- [GitLab SaaS Free tier frequently asked questions](https://about.gitlab.com/pricing/faq-efficient-free-tier/) diff --git a/doc/user/project/issues/managing_issues.md b/doc/user/project/issues/managing_issues.md index 4508ef30ac5..7db66dd013b 100644 --- a/doc/user/project/issues/managing_issues.md +++ b/doc/user/project/issues/managing_issues.md @@ -225,6 +225,8 @@ When you're creating a new issue, you can complete the following fields: ## Edit an issue +> Reordering list items [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/15260) in GitLab 15.0. + You can edit an issue's title and description. Prerequisites: @@ -237,6 +239,14 @@ To edit an issue: 1. Edit the available fields. 1. Select **Save changes**. +You can also reorder list items, which include bullet, numerical, and task list items. +To reorder list items: + +1. Hover over the list item row to make the drag icon visible. +1. Click and hold the drag icon. +1. Drag the row to the new position in the list. +1. Release the drag icon. + ### Bulk edit issues from a project > - Assigning epic [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/210470) in GitLab 13.2. diff --git a/lib/gitlab/ci/parsers/security/validators/schema_validator.rb b/lib/gitlab/ci/parsers/security/validators/schema_validator.rb index d299fa871b7..4460843545e 100644 --- a/lib/gitlab/ci/parsers/security/validators/schema_validator.rb +++ b/lib/gitlab/ci/parsers/security/validators/schema_validator.rb @@ -154,7 +154,6 @@ module Gitlab def handle_unsupported_report_version(treat_as:) if report_version.nil? message = "Report version not provided, #{report_type} report type supports versions: #{supported_schema_versions}" - add_message_as(level: treat_as, message: message) else message = "Version #{report_version} for report type #{report_type} is unsupported, supported versions for this report type are: #{supported_schema_versions}" end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index a260318cd6e..81255bed4c6 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -772,9 +772,6 @@ msgstr "" msgid "%{lock_path} is locked by GitLab User %{lock_user_id}" msgstr "" -msgid "%{markdownDocsLinkStart}Markdown%{markdownDocsLinkEnd} and %{quickActionsDocsLinkStart}quick actions%{quickActionsDocsLinkEnd} are supported" -msgstr "" - msgid "%{mergeLength}/%{usersLength} can merge" msgstr "" @@ -19357,6 +19354,9 @@ msgstr "" msgid "In progress" msgstr "" +msgid "In the background, we're attempting to connect you again." +msgstr "" + msgid "In this page you will find information about the settings that are used in your current instance." msgstr "" @@ -23282,18 +23282,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 "Markdown supported." msgstr "" @@ -23321,6 +23315,9 @@ msgstr "" msgid "MarkdownEditor|Add strikethrough text (%{modifier_key}⇧X)" msgstr "" +msgid "MarkdownToolbar|Supports %{markdownDocsLinkStart}Markdown%{markdownDocsLinkEnd}" +msgstr "" + msgid "Marked For Deletion At - %{deletion_time}" msgstr "" @@ -25789,6 +25786,9 @@ msgstr "" msgid "NoteForm|Note" msgstr "" +msgid "NoteToolbar|Supports %{markdownDocsLinkStart}Markdown%{markdownDocsLinkEnd}. For %{quickActionsDocsLinkStart}quick actions%{quickActionsDocsLinkEnd}, type %{keyboardStart}/%{keyboardEnd}." +msgstr "" + msgid "Notes" msgstr "" @@ -26058,6 +26058,9 @@ msgstr "" msgid "Off" msgstr "" +msgid "Offline" +msgstr "" + msgid "Oh no!" msgstr "" @@ -42992,6 +42995,9 @@ msgstr "" msgid "You are connected to the Prometheus server, but there is currently no data to display." msgstr "" +msgid "You are currently offline, or the GitLab instance is not reachable." +msgstr "" + msgid "You are going to delete %{project_full_name}. Deleted projects CANNOT be restored! Are you ABSOLUTELY sure?" msgstr "" @@ -45566,9 +45572,6 @@ msgstr "" msgid "projects" msgstr "" -msgid "quick actions" -msgstr "" - msgid "reCAPTCHA" msgstr "" diff --git a/qa/qa/page/component/blob_content.rb b/qa/qa/page/component/blob_content.rb index ce743b24dda..c2a1687ccfc 100644 --- a/qa/qa/page/component/blob_content.rb +++ b/qa/qa/page/component/blob_content.rb @@ -63,6 +63,17 @@ module QA end end + def has_normalized_ws_text?(text, wait: Capybara.default_max_wait_time) + if has_element?(:blob_viewer_file_content, wait: 1) + # The blob viewer renders line numbers and whitespace in a way that doesn't match the source file + # This isn't a visual validation test, so we ignore line numbers and whitespace + find_element(:blob_viewer_file_content, wait: 0).text.gsub(/^\d+\s|\s*/, '') + .start_with?(text.gsub(/\s*/, '')) + else + has_text?(text.gsub(/\s+/, " "), wait: wait) + end + end + def click_copy_file_contents(file_number = nil) within_file_by_number(:default_actions_container, file_number) { click_element(:copy_contents_button) } end diff --git a/qa/qa/page/file/form.rb b/qa/qa/page/file/form.rb index a6251f185f9..bb8934db498 100644 --- a/qa/qa/page/file/form.rb +++ b/qa/qa/page/file/form.rb @@ -4,8 +4,9 @@ module QA module Page module File class Form < Page::Base - include Shared::CommitMessage include Page::Component::DropdownFilter + include Page::Component::BlobContent + include Shared::CommitMessage include Shared::CommitButton include Shared::Editor diff --git a/qa/qa/page/main/login.rb b/qa/qa/page/main/login.rb index 30d1895781a..f3ee627c41e 100644 --- a/qa/qa/page/main/login.rb +++ b/qa/qa/page/main/login.rb @@ -133,6 +133,14 @@ module QA has_css?('[name="arkose_labs_token"][value]', visible: false) end + def has_accept_all_cookies_button? + has_button?('Accept All Cookies') + end + + def click_accept_all_cookies + click_button('Accept All Cookies') + end + def switch_to_sign_in_tab click_element :sign_in_tab end @@ -179,6 +187,7 @@ module QA fill_element :password_field, user.password if Runtime::Env.running_on_dot_com? + click_accept_all_cookies if has_accept_all_cookies_button? # Arkose only appears in staging.gitlab.com, gitlab.com, etc... # Wait until the ArkoseLabs challenge has initialized diff --git a/spec/frontend/issues/show/components/app_spec.js b/spec/frontend/issues/show/components/app_spec.js index c4c1ee6347e..4b800a4ccc9 100644 --- a/spec/frontend/issues/show/components/app_spec.js +++ b/spec/frontend/issues/show/components/app_spec.js @@ -4,6 +4,7 @@ import { nextTick } from 'vue'; import { setHTMLFixture, resetHTMLFixture } from 'helpers/fixtures'; import { createMockDirective, getBinding } from 'helpers/vue_mock_directive'; import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; +import waitForPromises from 'helpers/wait_for_promises'; import '~/behaviors/markdown/render_gfm'; import { IssuableStatus, IssuableStatusText, IssuableType } from '~/issues/constants'; import IssuableApp from '~/issues/show/components/app.vue'; @@ -647,4 +648,14 @@ describe('Issuable output', () => { expect(wrapper.vm.updateStoreState).toHaveBeenCalled(); }); }); + + describe('listItemReorder event', () => { + it('makes request to update issue', async () => { + const description = 'I have been updated!'; + findDescription().vm.$emit('listItemReorder', description); + await waitForPromises(); + + expect(mock.history.put[0].data).toContain(description); + }); + }); }); diff --git a/spec/frontend/issues/show/mock_data/mock_data.js b/spec/frontend/issues/show/mock_data/mock_data.js index 7b0b8ca686a..909789b7a0f 100644 --- a/spec/frontend/issues/show/mock_data/mock_data.js +++ b/spec/frontend/issues/show/mock_data/mock_data.js @@ -77,7 +77,22 @@ export const descriptionHtmlWithTask = ` <ul data-sourcepos="1:1-3:7" class="task-list" dir="auto"> <li data-sourcepos="1:1-1:10" class="task-list-item"> <input type="checkbox" class="task-list-item-checkbox" disabled> - <a href="/gitlab-org/gitlab-test/-/issues/48" data-original="#48+" data-link="false" data-link-reference="false" data-project="1" data-issue="2" data-reference-format="+" data-reference-type="task" data-container="body" data-placement="top" title="1" class="gfm gfm-issue has-tooltip">1 (#48)</a> + <a href="/gitlab-org/gitlab-test/-/issues/48" data-original="#48+" data-link="false" data-link-reference="false" data-project="1" data-issue="2" data-reference-format="+" data-reference-type="task" data-container="body" data-placement="top" title="1" class="gfm gfm-issue has-tooltip" data-issue-type="task">1 (#48)</a> + </li> + <li data-sourcepos="2:1-2:7" class="task-list-item"> + <input type="checkbox" class="task-list-item-checkbox" disabled> 2 + </li> + <li data-sourcepos="3:1-3:7" class="task-list-item"> + <input type="checkbox" class="task-list-item-checkbox" disabled> 3 + </li> + </ul> +`; + +export const descriptionHtmlWithIssue = ` + <ul data-sourcepos="1:1-3:7" class="task-list" dir="auto"> + <li data-sourcepos="1:1-1:10" class="task-list-item"> + <input type="checkbox" class="task-list-item-checkbox" disabled> + <a href="/gitlab-org/gitlab-test/-/issues/48" data-original="#48+" data-link="false" data-link-reference="false" data-project="1" data-issue="2" data-reference-format="+" data-reference-type="task" data-container="body" data-placement="top" title="1" class="gfm gfm-issue has-tooltip" data-issue-type="issue">1 (#48)</a> </li> <li data-sourcepos="2:1-2:7" class="task-list-item"> <input type="checkbox" class="task-list-item-checkbox" disabled> 2 diff --git a/spec/frontend/issues/show/utils_spec.js b/spec/frontend/issues/show/utils_spec.js new file mode 100644 index 00000000000..e5f14cfc01a --- /dev/null +++ b/spec/frontend/issues/show/utils_spec.js @@ -0,0 +1,40 @@ +import { convertDescriptionWithNewSort } from '~/issues/show/utils'; + +describe('app/assets/javascripts/issues/show/utils.js', () => { + describe('convertDescriptionWithNewSort', () => { + it('converts markdown description with new list sort order', () => { + const description = `I am text + +- Item 1 +- Item 2 + - Item 3 + - Item 4 +- Item 5`; + + // Drag Item 2 + children to Item 1's position + const html = `<ul data-sourcepos="3:1-8:0"> + <li data-sourcepos="4:1-4:8"> + Item 2 + <ul data-sourcepos="5:1-6:10"> + <li data-sourcepos="5:1-5:10">Item 3</li> + <li data-sourcepos="6:1-6:10">Item 4</li> + </ul> + </li> + <li data-sourcepos="3:1-3:8">Item 1</li> + <li data-sourcepos="7:1-8:0">Item 5</li> + <ul>`; + const list = document.createElement('div'); + list.innerHTML = html; + + const expected = `I am text + +- Item 2 + - Item 3 + - Item 4 +- Item 1 +- Item 5`; + + expect(convertDescriptionWithNewSort(description, list.firstChild)).toBe(expected); + }); + }); +}); diff --git a/spec/frontend/jobs/components/stuck_block_spec.js b/spec/frontend/jobs/components/stuck_block_spec.js index 4db73eaaaec..1580ed45e46 100644 --- a/spec/frontend/jobs/components/stuck_block_spec.js +++ b/spec/frontend/jobs/components/stuck_block_spec.js @@ -32,7 +32,7 @@ describe('Stuck Block Job component', () => { describe('with no runners for project', () => { beforeEach(() => { createWrapper({ - hasNoRunnersForProject: true, + hasOfflineRunnersForProject: true, runnersPath: '/root/project/runners#js-runners-settings', }); }); @@ -53,7 +53,7 @@ describe('Stuck Block Job component', () => { describe('with tags', () => { beforeEach(() => { createWrapper({ - hasNoRunnersForProject: false, + hasOfflineRunnersForProject: false, tags, runnersPath: '/root/project/runners#js-runners-settings', }); @@ -81,7 +81,7 @@ describe('Stuck Block Job component', () => { describe('without active runners', () => { beforeEach(() => { createWrapper({ - hasNoRunnersForProject: false, + hasOfflineRunnersForProject: false, runnersPath: '/root/project/runners#js-runners-settings', }); }); diff --git a/spec/frontend/jobs/store/getters_spec.js b/spec/frontend/jobs/store/getters_spec.js index 7a0fe2b8202..c13b051c672 100644 --- a/spec/frontend/jobs/store/getters_spec.js +++ b/spec/frontend/jobs/store/getters_spec.js @@ -208,7 +208,7 @@ describe('Job Store Getters', () => { }); }); - describe('hasRunnersForProject', () => { + describe('hasOfflineRunnersForProject', () => { describe('with available and offline runners', () => { it('returns true', () => { localState.job.runners = { @@ -216,7 +216,7 @@ describe('Job Store Getters', () => { online: false, }; - expect(getters.hasRunnersForProject(localState)).toEqual(true); + expect(getters.hasOfflineRunnersForProject(localState)).toEqual(true); }); }); @@ -227,7 +227,7 @@ describe('Job Store Getters', () => { online: false, }; - expect(getters.hasRunnersForProject(localState)).toEqual(false); + expect(getters.hasOfflineRunnersForProject(localState)).toEqual(false); }); }); @@ -238,7 +238,7 @@ describe('Job Store Getters', () => { online: true, }; - expect(getters.hasRunnersForProject(localState)).toEqual(false); + expect(getters.hasOfflineRunnersForProject(localState)).toEqual(false); }); }); }); diff --git a/spec/lib/gitlab/ci/parsers/security/validators/schema_validator_spec.rb b/spec/lib/gitlab/ci/parsers/security/validators/schema_validator_spec.rb index fae09c32c20..d06077d69b6 100644 --- a/spec/lib/gitlab/ci/parsers/security/validators/schema_validator_spec.rb +++ b/spec/lib/gitlab/ci/parsers/security/validators/schema_validator_spec.rb @@ -5,6 +5,8 @@ require 'spec_helper' RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do let_it_be(:project) { create(:project) } + let(:supported_dast_versions) { described_class::SUPPORTED_VERSIONS[:dast].join(', ') } + let(:scanner) do { 'id' => 'gemnasium', @@ -22,7 +24,7 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do expect(described_class::SUPPORTED_VERSIONS.keys).to eq(described_class::DEPRECATED_VERSIONS.keys) end - context 'files under schema path are explicitly listed' do + context 'when a schema JSON file exists for a particular report type version' do # We only care about the part that comes before report-format.json # https://rubular.com/r/N8Juz7r8hYDYgD filename_regex = /(?<report_type>[-\w]*)\-report-format.json/ @@ -36,14 +38,14 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do matches = filename_regex.match(file) report_type = matches[:report_type].tr("-", "_").to_sym - it "#{report_type} #{version}" do + it "#{report_type} #{version} is in the constant" do expect(described_class::SUPPORTED_VERSIONS[report_type]).to include(version) end end end end - context 'every SUPPORTED_VERSION has a corresponding JSON file' do + context 'when every SUPPORTED_VERSION has a corresponding JSON file' do described_class::SUPPORTED_VERSIONS.each_key do |report_type| # api_fuzzing is covered by DAST schema next if report_type == :api_fuzzing @@ -66,7 +68,7 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do let(:report_type) { :dast } let(:report_version) { described_class::SUPPORTED_VERSIONS[report_type].last } - context 'and the report is valid' do + context 'when the report is valid' do let(:report_data) do { 'version' => report_version, @@ -77,7 +79,7 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do it { is_expected.to be_truthy } end - context 'and the report is invalid' do + context 'when the report is invalid' do let(:report_data) do { 'version' => report_version @@ -116,7 +118,7 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do stub_const("#{described_class}::DEPRECATED_VERSIONS", deprecations_hash) end - context 'and the report passes schema validation' do + context 'when the report passes schema validation' do let(:report_data) do { 'version' => '10.0.0', @@ -141,8 +143,8 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do end end - context 'and the report does not pass schema validation' do - context 'and enforce_security_report_validation is enabled' do + context 'when the report does not pass schema validation' do + context 'when enforce_security_report_validation is enabled' do before do stub_feature_flags(enforce_security_report_validation: true) end @@ -156,7 +158,7 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do it { is_expected.to be_falsey } end - context 'and enforce_security_report_validation is disabled' do + context 'when enforce_security_report_validation is disabled' do before do stub_feature_flags(enforce_security_report_validation: false) end @@ -176,12 +178,12 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do let(:report_type) { :dast } let(:report_version) { "12.37.0" } - context 'if enforce_security_report_validation is enabled' do + context 'when enforce_security_report_validation is enabled' do before do stub_feature_flags(enforce_security_report_validation: true) end - context 'and the report is valid' do + context 'when the report is valid' do let(:report_data) do { 'version' => report_version, @@ -206,14 +208,14 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do end end - context 'and the report is invalid' do + context 'when the report is invalid' do let(:report_data) do { 'version' => report_version } end - context 'and scanner information is empty' do + context 'when scanner information is empty' do let(:scanner) { {} } it 'logs related information' do @@ -245,12 +247,12 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do end end - context 'if enforce_security_report_validation is disabled' do + context 'when enforce_security_report_validation is disabled' do before do stub_feature_flags(enforce_security_report_validation: false) end - context 'and the report is valid' do + context 'when the report is valid' do let(:report_data) do { 'version' => report_version, @@ -261,7 +263,7 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do it { is_expected.to be_truthy } end - context 'and the report is invalid' do + context 'when the report is invalid' do let(:report_data) do { 'version' => report_version @@ -272,6 +274,30 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do end end end + + context 'when not given a schema version' do + let(:report_type) { :dast } + let(:report_version) { nil } + let(:report_data) do + { + 'vulnerabilities' => [] + } + end + + before do + stub_feature_flags(enforce_security_report_validation: true) + end + + it { is_expected.to be_falsey } + + context 'when enforce_security_report_validation is disabled' do + before do + stub_feature_flags(enforce_security_report_validation: false) + end + + it { is_expected.to be_truthy } + end + end end describe '#errors' do @@ -281,7 +307,7 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do let(:report_type) { :dast } let(:report_version) { described_class::SUPPORTED_VERSIONS[report_type].last } - context 'and the report is valid' do + context 'when the report is valid' do let(:report_data) do { 'version' => report_version, @@ -289,19 +315,17 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do } end - let(:expected_errors) { [] } - - it { is_expected.to match_array(expected_errors) } + it { is_expected.to be_empty } end - context 'and the report is invalid' do + context 'when the report is invalid' do let(:report_data) do { 'version' => report_version } end - context 'if enforce_security_report_validation is enabled' do + context 'when enforce_security_report_validation is enabled' do before do stub_feature_flags(enforce_security_report_validation: project) end @@ -315,14 +339,12 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do it { is_expected.to match_array(expected_errors) } end - context 'if enforce_security_report_validation is disabled' do + context 'when enforce_security_report_validation is disabled' do before do stub_feature_flags(enforce_security_report_validation: false) end - let(:expected_errors) { [] } - - it { is_expected.to match_array(expected_errors) } + it { is_expected.to be_empty } end end end @@ -341,7 +363,7 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do stub_const("#{described_class}::DEPRECATED_VERSIONS", deprecations_hash) end - context 'and the report passes schema validation' do + context 'when the report passes schema validation' do let(:report_data) do { 'version' => '10.0.0', @@ -349,13 +371,11 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do } end - let(:expected_errors) { [] } - - it { is_expected.to match_array(expected_errors) } + it { is_expected.to be_empty } end - context 'and the report does not pass schema validation' do - context 'and enforce_security_report_validation is enabled' do + context 'when the report does not pass schema validation' do + context 'when enforce_security_report_validation is enabled' do before do stub_feature_flags(enforce_security_report_validation: true) end @@ -376,7 +396,7 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do it { is_expected.to match_array(expected_errors) } end - context 'and enforce_security_report_validation is disabled' do + context 'when enforce_security_report_validation is disabled' do before do stub_feature_flags(enforce_security_report_validation: false) end @@ -387,9 +407,7 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do } end - let(:expected_errors) { [] } - - it { is_expected.to match_array(expected_errors) } + it { is_expected.to be_empty } end end end @@ -398,12 +416,12 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do let(:report_type) { :dast } let(:report_version) { "12.37.0" } - context 'if enforce_security_report_validation is enabled' do + context 'when enforce_security_report_validation is enabled' do before do stub_feature_flags(enforce_security_report_validation: true) end - context 'and the report is valid' do + context 'when the report is valid' do let(:report_data) do { 'version' => report_version, @@ -413,14 +431,14 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do let(:expected_errors) do [ - "Version 12.37.0 for report type dast is unsupported, supported versions for this report type are: 14.0.0, 14.0.1, 14.0.2, 14.0.3, 14.0.4, 14.0.5, 14.0.6, 14.1.0, 14.1.1, 14.1.2" + "Version 12.37.0 for report type dast is unsupported, supported versions for this report type are: #{supported_dast_versions}" ] end it { is_expected.to match_array(expected_errors) } end - context 'and the report is invalid' do + context 'when the report is invalid' do let(:report_data) do { 'version' => report_version @@ -429,7 +447,7 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do let(:expected_errors) do [ - "Version 12.37.0 for report type dast is unsupported, supported versions for this report type are: 14.0.0, 14.0.1, 14.0.2, 14.0.3, 14.0.4, 14.0.5, 14.0.6, 14.1.0, 14.1.1, 14.1.2", + "Version 12.37.0 for report type dast is unsupported, supported versions for this report type are: #{supported_dast_versions}", "root is missing required keys: vulnerabilities" ] end @@ -438,12 +456,12 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do end end - context 'if enforce_security_report_validation is disabled' do + context 'when enforce_security_report_validation is disabled' do before do stub_feature_flags(enforce_security_report_validation: false) end - context 'and the report is valid' do + context 'when the report is valid' do let(:report_data) do { 'version' => report_version, @@ -451,22 +469,45 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do } end - let(:expected_errors) { [] } - - it { is_expected.to match_array(expected_errors) } + it { is_expected.to be_empty } end - context 'and the report is invalid' do + context 'when the report is invalid' do let(:report_data) do { 'version' => report_version } end - let(:expected_errors) { [] } + it { is_expected.to be_empty } + end + end + end + + context 'when not given a schema version' do + let(:report_type) { :dast } + let(:report_version) { nil } + let(:report_data) do + { + 'vulnerabilities' => [] + } + end - it { is_expected.to match_array(expected_errors) } + let(:expected_errors) do + [ + "root is missing required keys: version", + "Report version not provided, dast report type supports versions: #{supported_dast_versions}" + ] + end + + it { is_expected.to match_array(expected_errors) } + + context 'when enforce_security_report_validation is disabled' do + before do + stub_feature_flags(enforce_security_report_validation: false) end + + it { is_expected.to be_empty } end end end @@ -478,9 +519,7 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do let(:report_type) { :dast } let(:report_version) { described_class::SUPPORTED_VERSIONS[report_type].last } - let(:expected_deprecation_warnings) { [] } - - context 'and the report is valid' do + context 'when the report is valid' do let(:report_data) do { 'version' => report_version, @@ -488,17 +527,17 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do } end - it { is_expected.to match_array(expected_deprecation_warnings) } + it { is_expected.to be_empty } end - context 'and the report is invalid' do + context 'when the report is invalid' do let(:report_data) do { 'version' => report_version } end - it { is_expected.to match_array(expected_deprecation_warnings) } + it { is_expected.to be_empty } end end @@ -513,7 +552,7 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do let(:report_version) { described_class::DEPRECATED_VERSIONS[report_type].last } let(:expected_deprecation_warnings) do [ - "Version V2.7.0 for report type dast has been deprecated, supported versions for this report type are: 14.0.0, 14.0.1, 14.0.2, 14.0.3, 14.0.4, 14.0.5, 14.0.6, 14.1.0, 14.1.1, 14.1.2" + "Version V2.7.0 for report type dast has been deprecated, supported versions for this report type are: #{supported_dast_versions}" ] end @@ -521,7 +560,7 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do stub_const("#{described_class}::DEPRECATED_VERSIONS", deprecations_hash) end - context 'and the report passes schema validation' do + context 'when the report passes schema validation' do let(:report_data) do { 'version' => report_version, @@ -532,7 +571,7 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do it { is_expected.to match_array(expected_deprecation_warnings) } end - context 'and the report does not pass schema validation' do + context 'when the report does not pass schema validation' do let(:report_data) do { 'version' => 'V2.7.0' @@ -565,7 +604,7 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do let(:report_type) { :dast } let(:report_version) { described_class::SUPPORTED_VERSIONS[report_type].last } - context 'and the report is valid' do + context 'when the report is valid' do let(:report_data) do { 'version' => report_version, @@ -573,29 +612,25 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do } end - let(:expected_warnings) { [] } - - it { is_expected.to match_array(expected_warnings) } + it { is_expected.to be_empty } end - context 'and the report is invalid' do + context 'when the report is invalid' do let(:report_data) do { 'version' => report_version } end - context 'if enforce_security_report_validation is enabled' do + context 'when enforce_security_report_validation is enabled' do before do stub_feature_flags(enforce_security_report_validation: project) end - let(:expected_warnings) { [] } - - it { is_expected.to match_array(expected_warnings) } + it { is_expected.to be_empty } end - context 'if enforce_security_report_validation is disabled' do + context 'when enforce_security_report_validation is disabled' do before do stub_feature_flags(enforce_security_report_validation: false) end @@ -625,36 +660,32 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do stub_const("#{described_class}::DEPRECATED_VERSIONS", deprecations_hash) end - context 'and the report passes schema validation' do + context 'when the report passes schema validation' do let(:report_data) do { 'vulnerabilities' => [] } end - let(:expected_warnings) { [] } - - it { is_expected.to match_array(expected_warnings) } + it { is_expected.to be_empty } end - context 'and the report does not pass schema validation' do + context 'when the report does not pass schema validation' do let(:report_data) do { 'version' => 'V2.7.0' } end - context 'and enforce_security_report_validation is enabled' do + context 'when enforce_security_report_validation is enabled' do before do stub_feature_flags(enforce_security_report_validation: true) end - let(:expected_warnings) { [] } - - it { is_expected.to match_array(expected_warnings) } + it { is_expected.to be_empty } end - context 'and enforce_security_report_validation is disabled' do + context 'when enforce_security_report_validation is disabled' do before do stub_feature_flags(enforce_security_report_validation: false) end @@ -675,12 +706,12 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do let(:report_type) { :dast } let(:report_version) { "12.37.0" } - context 'if enforce_security_report_validation is enabled' do + context 'when enforce_security_report_validation is enabled' do before do stub_feature_flags(enforce_security_report_validation: true) end - context 'and the report is valid' do + context 'when the report is valid' do let(:report_data) do { 'version' => report_version, @@ -688,30 +719,26 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do } end - let(:expected_warnings) { [] } - - it { is_expected.to match_array(expected_warnings) } + it { is_expected.to be_empty } end - context 'and the report is invalid' do + context 'when the report is invalid' do let(:report_data) do { 'version' => report_version } end - let(:expected_warnings) { [] } - - it { is_expected.to match_array(expected_warnings) } + it { is_expected.to be_empty } end end - context 'if enforce_security_report_validation is disabled' do + context 'when enforce_security_report_validation is disabled' do before do stub_feature_flags(enforce_security_report_validation: false) end - context 'and the report is valid' do + context 'when the report is valid' do let(:report_data) do { 'version' => report_version, @@ -721,14 +748,14 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do let(:expected_warnings) do [ - "Version 12.37.0 for report type dast is unsupported, supported versions for this report type are: 14.0.0, 14.0.1, 14.0.2, 14.0.3, 14.0.4, 14.0.5, 14.0.6, 14.1.0, 14.1.1, 14.1.2" + "Version 12.37.0 for report type dast is unsupported, supported versions for this report type are: #{supported_dast_versions}" ] end it { is_expected.to match_array(expected_warnings) } end - context 'and the report is invalid' do + context 'when the report is invalid' do let(:report_data) do { 'version' => report_version @@ -737,7 +764,7 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do let(:expected_warnings) do [ - "Version 12.37.0 for report type dast is unsupported, supported versions for this report type are: 14.0.0, 14.0.1, 14.0.2, 14.0.3, 14.0.4, 14.0.5, 14.0.6, 14.1.0, 14.1.1, 14.1.2", + "Version 12.37.0 for report type dast is unsupported, supported versions for this report type are: #{supported_dast_versions}", "root is missing required keys: vulnerabilities" ] end @@ -746,5 +773,32 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator do end end end + + context 'when not given a schema version' do + let(:report_type) { :dast } + let(:report_version) { nil } + let(:report_data) do + { + 'vulnerabilities' => [] + } + end + + it { is_expected.to be_empty } + + context 'when enforce_security_report_validation is disabled' do + before do + stub_feature_flags(enforce_security_report_validation: false) + end + + let(:expected_warnings) do + [ + "root is missing required keys: version", + "Report version not provided, dast report type supports versions: #{supported_dast_versions}" + ] + end + + it { is_expected.to match_array(expected_warnings) } + end + end end end diff --git a/spec/requests/pwa_controller_spec.rb b/spec/requests/pwa_controller_spec.rb new file mode 100644 index 00000000000..f74f37ea9d0 --- /dev/null +++ b/spec/requests/pwa_controller_spec.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe PwaController do + describe 'GET #offline' do + it 'responds with static HTML page' do + get offline_path + + expect(response.body).to include('You are currently offline') + expect(response).to have_gitlab_http_status(:success) + end + end +end diff --git a/spec/views/shared/notes/_form.html.haml_spec.rb b/spec/views/shared/notes/_form.html.haml_spec.rb index b7bad4c5d78..ccf1e08b7e7 100644 --- a/spec/views/shared/notes/_form.html.haml_spec.rb +++ b/spec/views/shared/notes/_form.html.haml_spec.rb @@ -23,7 +23,7 @@ RSpec.describe 'shared/notes/_form' do let(:note) { build(:"note_on_#{noteable}", project: project) } it 'says that markdown and quick actions are supported' do - expect(rendered).to have_content('Markdown and quick actions are supported') + expect(rendered).to have_content('Supports Markdown. For quick actions, type /.') end end end diff --git a/workhorse/internal/redis/keywatcher.go b/workhorse/internal/redis/keywatcher.go index 13e9fc3f051..82cb082f5f0 100644 --- a/workhorse/internal/redis/keywatcher.go +++ b/workhorse/internal/redis/keywatcher.go @@ -1,6 +1,7 @@ package redis import ( + "errors" "fmt" "strings" "sync" @@ -189,7 +190,9 @@ func WatchKey(key, value string, timeout time.Duration) (WatchKeyStatus, error) defer delKeyChan(kw) currentValue, err := GetString(key) - if err != nil { + if errors.Is(err, redis.ErrNil) { + currentValue = "" + } else if err != nil { return WatchKeyStatusNoChange, fmt.Errorf("keywatcher: redis GET: %v", err) } if currentValue != value { diff --git a/workhorse/internal/redis/keywatcher_test.go b/workhorse/internal/redis/keywatcher_test.go index 7ff5f8204c0..a2f2b73898f 100644 --- a/workhorse/internal/redis/keywatcher_test.go +++ b/workhorse/internal/redis/keywatcher_test.go @@ -1,10 +1,12 @@ package redis import ( + "errors" "sync" "testing" "time" + "github.com/gomodule/redigo/redis" "github.com/rafaeljusto/redigomock" "github.com/stretchr/testify/require" ) @@ -65,100 +67,191 @@ func processMessages(numWatchers int, value string) { processInner(psc) } -func TestWatchKeySeenChange(t *testing.T) { +type keyChangeTestCase struct { + desc string + returnValue string + isKeyMissing bool + watchValue string + processedValue string + expectedStatus WatchKeyStatus + timeout time.Duration +} + +func TestKeyChangesBubblesUpError(t *testing.T) { conn, td := setupMockPool() defer td() - conn.Command("GET", runnerKey).Expect("something") - - wg := &sync.WaitGroup{} - wg.Add(1) + conn.Command("GET", runnerKey).ExpectError(errors.New("test error")) - go func() { - val, err := WatchKey(runnerKey, "something", time.Second) - require.NoError(t, err, "Expected no error") - require.Equal(t, WatchKeyStatusSeenChange, val, "Expected value to change") - wg.Done() - }() + _, err := WatchKey(runnerKey, "something", time.Second) + require.Error(t, err, "Expected error") - processMessages(1, "somethingelse") - wg.Wait() + deleteWatchers(runnerKey) } -func TestWatchKeyNoChange(t *testing.T) { - conn, td := setupMockPool() - defer td() +func TestKeyChangesInstantReturn(t *testing.T) { + testCases := []keyChangeTestCase{ + // WatchKeyStatusAlreadyChanged + { + desc: "sees change with key existing and changed", + returnValue: "somethingelse", + watchValue: "something", + expectedStatus: WatchKeyStatusAlreadyChanged, + timeout: time.Second, + }, + { + desc: "sees change with key non-existing", + isKeyMissing: true, + watchValue: "something", + processedValue: "somethingelse", + expectedStatus: WatchKeyStatusAlreadyChanged, + timeout: time.Second, + }, + // WatchKeyStatusTimeout + { + desc: "sees timeout with key existing and unchanged", + returnValue: "something", + watchValue: "something", + expectedStatus: WatchKeyStatusTimeout, + timeout: time.Millisecond, + }, + { + desc: "sees timeout with key non-existing and unchanged", + isKeyMissing: true, + watchValue: "", + expectedStatus: WatchKeyStatusTimeout, + timeout: time.Millisecond, + }, + } - conn.Command("GET", runnerKey).Expect("something") + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + conn, td := setupMockPool() + defer td() - wg := &sync.WaitGroup{} - wg.Add(1) + if tc.isKeyMissing { + conn.Command("GET", runnerKey).ExpectError(redis.ErrNil) + } else { + conn.Command("GET", runnerKey).Expect(tc.returnValue) + } - go func() { - val, err := WatchKey(runnerKey, "something", time.Second) - require.NoError(t, err, "Expected no error") - require.Equal(t, WatchKeyStatusNoChange, val, "Expected notification without change to value") - wg.Done() - }() + val, err := WatchKey(runnerKey, tc.watchValue, tc.timeout) - processMessages(1, "something") - wg.Wait() -} + require.NoError(t, err, "Expected no error") + require.Equal(t, tc.expectedStatus, val, "Expected value") -func TestWatchKeyTimeout(t *testing.T) { - conn, td := setupMockPool() - defer td() + deleteWatchers(runnerKey) + }) + } +} - conn.Command("GET", runnerKey).Expect("something") +func TestKeyChangesWhenWatching(t *testing.T) { + testCases := []keyChangeTestCase{ + // WatchKeyStatusSeenChange + { + desc: "sees change with key existing", + returnValue: "something", + watchValue: "something", + processedValue: "somethingelse", + expectedStatus: WatchKeyStatusSeenChange, + }, + { + desc: "sees change with key non-existing, when watching empty value", + isKeyMissing: true, + watchValue: "", + processedValue: "something", + expectedStatus: WatchKeyStatusSeenChange, + }, + // WatchKeyStatusNoChange + { + desc: "sees no change with key existing", + returnValue: "something", + watchValue: "something", + processedValue: "something", + expectedStatus: WatchKeyStatusNoChange, + }, + } - val, err := WatchKey(runnerKey, "something", time.Millisecond) - require.NoError(t, err, "Expected no error") - require.Equal(t, WatchKeyStatusTimeout, val, "Expected value to not change") + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + conn, td := setupMockPool() + defer td() - // Clean up watchers since Process isn't doing that for us (not running) - deleteWatchers(runnerKey) -} + if tc.isKeyMissing { + conn.Command("GET", runnerKey).ExpectError(redis.ErrNil) + } else { + conn.Command("GET", runnerKey).Expect(tc.returnValue) + } -func TestWatchKeyAlreadyChanged(t *testing.T) { - conn, td := setupMockPool() - defer td() + wg := &sync.WaitGroup{} + wg.Add(1) - conn.Command("GET", runnerKey).Expect("somethingelse") + go func() { + defer wg.Done() + val, err := WatchKey(runnerKey, tc.watchValue, time.Second) - val, err := WatchKey(runnerKey, "something", time.Second) - require.NoError(t, err, "Expected no error") - require.Equal(t, WatchKeyStatusAlreadyChanged, val, "Expected value to have already changed") + require.NoError(t, err, "Expected no error") + require.Equal(t, tc.expectedStatus, val, "Expected value") + }() - // Clean up watchers since Process isn't doing that for us (not running) - deleteWatchers(runnerKey) + processMessages(1, tc.processedValue) + wg.Wait() + }) + } } -func TestWatchKeyMassivelyParallel(t *testing.T) { - runTimes := 100 // 100 parallel watchers +func TestKeyChangesParallel(t *testing.T) { + testCases := []keyChangeTestCase{ + { + desc: "massively parallel, sees change with key existing", + returnValue: "something", + watchValue: "something", + processedValue: "somethingelse", + expectedStatus: WatchKeyStatusSeenChange, + }, + { + desc: "massively parallel, sees change with key existing, watching missing keys", + isKeyMissing: true, + watchValue: "", + processedValue: "somethingelse", + expectedStatus: WatchKeyStatusSeenChange, + }, + } - conn, td := setupMockPool() - defer td() + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + runTimes := 100 - wg := &sync.WaitGroup{} - wg.Add(runTimes) + conn, td := setupMockPool() + defer td() - getCmd := conn.Command("GET", runnerKey) + getCmd := conn.Command("GET", runnerKey) - for i := 0; i < runTimes; i++ { - getCmd = getCmd.Expect("something") - } + for i := 0; i < runTimes; i++ { + if tc.isKeyMissing { + getCmd = getCmd.ExpectError(redis.ErrNil) + } else { + getCmd = getCmd.Expect(tc.returnValue) + } + } - for i := 0; i < runTimes; i++ { - go func() { - val, err := WatchKey(runnerKey, "something", time.Second) - require.NoError(t, err, "Expected no error") - require.Equal(t, WatchKeyStatusSeenChange, val, "Expected value to change") - wg.Done() - }() - } + wg := &sync.WaitGroup{} + wg.Add(runTimes) - processMessages(runTimes, "somethingelse") - wg.Wait() + for i := 0; i < runTimes; i++ { + go func() { + defer wg.Done() + val, err := WatchKey(runnerKey, tc.watchValue, time.Second) + + require.NoError(t, err, "Expected no error") + require.Equal(t, tc.expectedStatus, val, "Expected value") + }() + } + + processMessages(runTimes, tc.processedValue) + wg.Wait() + }) + } } func TestShutdown(t *testing.T) { |