diff --git a/.gitlab/issue_templates/Feature Flag Roll Out.md b/.gitlab/issue_templates/Feature Flag Roll Out.md index 3fe72feda57..bc1a23729e2 100644 --- a/.gitlab/issue_templates/Feature Flag Roll Out.md +++ b/.gitlab/issue_templates/Feature Flag Roll Out.md @@ -48,6 +48,12 @@ Example below: ### What can we monitor to detect problems with this? +_Consider mentioning checks for 5xx errors or other anomalies like an increase in redirects +(302 HTTP response status)_ + +### What can we check for monitoring production after rollouts? + +_Consider adding links to check for Sentry errors, Production logs for 5xx, 302s, etc._ ## Rollout Steps @@ -84,7 +90,7 @@ Example below: If a different developer will be covering, or an exception is needed, please inform the oncall SRE by using the `@sre-oncall` Slack alias. - [ ] Ensure that documentation has been updated ([More info](https://docs.gitlab.com/ee/development/documentation/feature_flags.html#features-that-became-enabled-by-default)). - [ ] Announce on [the feature issue](ISSUE LINK) an estimated time this will be enabled on GitLab.com. -- [ ] If the feature might impact the user experience, notify `#support_gitlab-com` and your team channel ([more guidance when this is necessary in the dev docs](https://docs.gitlab.com/ee/development/feature_flags/controls.html#communicate-the-change)). +- [ ] Notify `#support_gitlab-com` and your team channel ([more guidance when this is necessary in the dev docs](https://docs.gitlab.com/ee/development/feature_flags/controls.html#communicate-the-change)). ### Global rollout on production diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 986b14ebec7..bc2c01b8e1f 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -a4079265eb1b35c794ca45a056984cb13a7c2c82 +8f04e23b9f004d70635aaeecd30d837a584572f8 diff --git a/app/assets/javascripts/editor/constants.js b/app/assets/javascripts/editor/constants.js index d44bfdfb966..e855e304d27 100644 --- a/app/assets/javascripts/editor/constants.js +++ b/app/assets/javascripts/editor/constants.js @@ -1,17 +1,9 @@ import { DEFAULT_DEBOUNCE_AND_THROTTLE_MS } from '~/lib/utils/constants'; import { s__ } from '~/locale'; -export const SOURCE_EDITOR_INSTANCE_ERROR_NO_EL = s__( - 'SourceEditor|"el" parameter is required for createInstance()', -); - export const URI_PREFIX = 'gitlab'; export const CONTENT_UPDATE_DEBOUNCE = DEFAULT_DEBOUNCE_AND_THROTTLE_MS; -export const ERROR_INSTANCE_REQUIRED_FOR_EXTENSION = s__( - 'SourceEditor|Source Editor instance is required to set up an extension.', -); - export const EDITOR_READY_EVENT = 'editor-ready'; export const EDITOR_TYPE_CODE = 'vs.editor.ICodeEditor'; @@ -20,9 +12,31 @@ export const EDITOR_TYPE_DIFF = 'vs.editor.IDiffEditor'; export const EDITOR_CODE_INSTANCE_FN = 'createInstance'; export const EDITOR_DIFF_INSTANCE_FN = 'createDiffInstance'; +export const SOURCE_EDITOR_INSTANCE_ERROR_NO_EL = s__( + 'SourceEditor|"el" parameter is required for createInstance()', +); +export const ERROR_INSTANCE_REQUIRED_FOR_EXTENSION = s__( + 'SourceEditor|Source Editor instance is required to set up an extension.', +); export const EDITOR_EXTENSION_DEFINITION_ERROR = s__( 'SourceEditor|Extension definition should be either a class or a function', ); +export const EDITOR_EXTENSION_NO_DEFINITION_ERROR = s__( + 'SourceEditor|`definition` property is expected on the extension.', +); +export const EDITOR_EXTENSION_DEFINITION_TYPE_ERROR = s__( + 'SourceEditor|Extension definition should be either class, function, or an Array of definitions.', +); +export const EDITOR_EXTENSION_NOT_SPECIFIED_FOR_UNUSE_ERROR = s__( + 'SourceEditor|No extension for unuse has been specified.', +); +export const EDITOR_EXTENSION_NOT_REGISTERED_ERROR = s__('SourceEditor|%{name} is not registered.'); +export const EDITOR_EXTENSION_NAMING_CONFLICT_ERROR = s__( + 'SourceEditor|Name conflict for "%{prop}()" method.', +); +export const EDITOR_EXTENSION_STORE_IS_MISSING_ERROR = s__( + 'SourceEditor|Extensions Store is required to check for an extension.', +); // // EXTENSIONS' CONSTANTS diff --git a/app/assets/javascripts/editor/extensions/source_editor_extension_base.js b/app/assets/javascripts/editor/extensions/source_editor_extension_base.js index 5fa01f03f7e..03c68fed3b1 100644 --- a/app/assets/javascripts/editor/extensions/source_editor_extension_base.js +++ b/app/assets/javascripts/editor/extensions/source_editor_extension_base.js @@ -36,12 +36,24 @@ export class SourceEditorExtension { }); } - static highlightLines(instance) { - const { hash } = window.location; - if (!hash) { - return; - } - const [start, end] = hash.replace(hashRegexp, '').split('-'); + static removeHighlights(instance) { + Object.assign(instance, { + lineDecorations: instance.deltaDecorations(instance.lineDecorations || [], []), + }); + } + + /** + * Returns a function that can only be invoked once between + * each browser screen repaint. + * @param {Object} instance - The Source Editor instance + * @param {Array} bounds - The [start, end] array with start + * and end coordinates for highlighting + */ + static highlightLines(instance, bounds = null) { + const [start, end] = + bounds && Array.isArray(bounds) + ? bounds + : window.location.hash?.replace(hashRegexp, '').split('-'); let startLine = start ? parseInt(start, 10) : null; let endLine = end ? parseInt(end, 10) : startLine; if (endLine < startLine) { @@ -51,15 +63,12 @@ export class SourceEditorExtension { window.requestAnimationFrame(() => { instance.revealLineInCenter(startLine); Object.assign(instance, { - lineDecorations: instance.deltaDecorations( - [], - [ - { - range: new Range(startLine, 1, endLine, 1), - options: { isWholeLine: true, className: 'active-line-text' }, - }, - ], - ), + lineDecorations: instance.deltaDecorations(instance.lineDecorations || [], [ + { + range: new Range(startLine, 1, endLine, 1), + options: { isWholeLine: true, className: 'active-line-text' }, + }, + ]), }); }); } diff --git a/app/assets/javascripts/editor/extensions/source_editor_yaml_ext.js b/app/assets/javascripts/editor/extensions/source_editor_yaml_ext.js new file mode 100644 index 00000000000..212e09c8724 --- /dev/null +++ b/app/assets/javascripts/editor/extensions/source_editor_yaml_ext.js @@ -0,0 +1,293 @@ +import { toPath } from 'lodash'; +import { parseDocument, Document, visit, isScalar, isCollection, isMap } from 'yaml'; +import { findPair } from 'yaml/util'; +import { SourceEditorExtension } from '~/editor/extensions/source_editor_extension_base'; + +export class YamlEditorExtension extends SourceEditorExtension { + /** + * Extends the source editor with capabilities for yaml files. + * + * @param { Instance } instance Source Editor Instance + * @param { boolean } enableComments Convert model nodes with the comment + * pattern to comments? + * @param { string } highlightPath Add a line highlight to the + * node specified by this e.g. `"foo.bar[0]"` + * @param { * } model Any JS Object that will be stringified and used as the + * editor's value. Equivalent to using `setDataModel()` + * @param options SourceEditorExtension Options + */ + constructor({ + instance, + enableComments = false, + highlightPath = null, + model = null, + ...options + } = {}) { + super({ + instance, + options: { + ...options, + enableComments, + highlightPath, + }, + }); + + if (model) { + YamlEditorExtension.initFromModel(instance, model); + } + + instance.onDidChangeModelContent(() => instance.onUpdate()); + } + + /** + * @private + */ + static initFromModel(instance, model) { + const doc = new Document(model); + if (instance.options.enableComments) { + YamlEditorExtension.transformComments(doc); + } + instance.setValue(doc.toString()); + } + + /** + * @private + * This wraps long comments to a maximum line length of 80 chars. + * + * The `yaml` package does not currently wrap comments. This function + * is a local workaround and should be deprecated if + * https://github.com/eemeli/yaml/issues/322 + * is resolved. + */ + static wrapCommentString(string, level = 0) { + if (!string) { + return null; + } + if (level < 0 || Number.isNaN(parseInt(level, 10))) { + throw Error(`Invalid value "${level}" for variable \`level\``); + } + const maxLineWidth = 80; + const indentWidth = 2; + const commentMarkerWidth = '# '.length; + const maxLength = maxLineWidth - commentMarkerWidth - level * indentWidth; + const lines = [[]]; + string.split(' ').forEach((word) => { + const currentLine = lines.length - 1; + if ([...lines[currentLine], word].join(' ').length <= maxLength) { + lines[currentLine].push(word); + } else { + lines.push([word]); + } + }); + return lines.map((line) => ` ${line.join(' ')}`).join('\n'); + } + + /** + * @private + * + * This utilizes `yaml`'s `visit` function to transform nodes with a + * comment key pattern to actual comments. + * + * In Objects, a key of '#' will be converted to a comment at the top of a + * property. Any key following the pattern `#|` will be placed + * right before ``. + * + * In Arrays, any string that starts with # (including the space), will + * be converted to a comment at the position it was in. + * + * @param { Document } doc + * @returns { Document } + */ + static transformComments(doc) { + const getLevel = (path) => { + const { length } = path.filter((x) => isCollection(x)); + return length ? length - 1 : 0; + }; + + visit(doc, { + Pair(_, pair, path) { + const key = pair.key.value; + // If the key is = '#', we add the value as a comment to the parent + // We can then remove the node. + if (key === '#') { + Object.assign(path[path.length - 1], { + commentBefore: YamlEditorExtension.wrapCommentString(pair.value.value, getLevel(path)), + }); + return visit.REMOVE; + } + // If the key starts with `#|`, we want to add a comment to the + // corresponding property. We can then remove the node. + if (key.startsWith('#|')) { + const targetProperty = key.split('|')[1]; + const target = findPair(path[path.length - 1].items, targetProperty); + if (target) { + target.key.commentBefore = YamlEditorExtension.wrapCommentString( + pair.value.value, + getLevel(path), + ); + } + return visit.REMOVE; + } + return undefined; // If the node is not a comment, do nothing with it + }, + // Sequence is basically an array + Seq(_, node, path) { + let comment = null; + const items = node.items.flatMap((child) => { + if (comment) { + Object.assign(child, { commentBefore: comment }); + comment = null; + } + if ( + isScalar(child) && + child.value && + child.value.startsWith && + child.value.startsWith('#') + ) { + const commentValue = child.value.replace(/^#\s?/, ''); + comment = YamlEditorExtension.wrapCommentString(commentValue, getLevel(path)); + return []; + } + return child; + }); + Object.assign(node, { items }); + // Adding a comment in case the last one is a comment + if (comment) { + Object.assign(node, { comment }); + } + }, + }); + return doc; + } + + /** + * Get the editor's value parsed as a `Document` as defined by the `yaml` + * package + * @returns {Document} + */ + getDoc() { + return parseDocument(this.getValue()); + } + + /** + * Accepts a `Document` as defined by the `yaml` package and + * sets the Editor's value to a stringified version of it. + * @param { Document } doc + */ + setDoc(doc) { + if (this.options.enableComments) { + YamlEditorExtension.transformComments(doc); + } + + if (!this.getValue()) { + this.setValue(doc.toString()); + } else { + this.updateValue(doc.toString()); + } + } + + /** + * Returns the parsed value of the Editor's content as JS. + * @returns {*} + */ + getDataModel() { + return this.getDoc().toJS(); + } + + /** + * Accepts any JS Object and sets the Editor's value to a stringified version + * of that value. + * + * @param value + */ + setDataModel(value) { + this.setDoc(new Document(value)); + } + + /** + * Method to be executed when the Editor's was updated + */ + onUpdate() { + if (this.options.highlightPath) { + this.highlight(this.options.highlightPath); + } + } + + /** + * Set the editors content to the input without recreating the content model. + * + * @param blob + */ + updateValue(blob) { + // Using applyEdits() instead of setValue() ensures that tokens such as + // highlighted lines aren't deleted/recreated which causes a flicker. + const model = this.getModel(); + model.applyEdits([ + { + // A nice improvement would be to replace getFullModelRange() with + // a range of the actual diff, avoiding re-formatting the document, + // but that's something for a later iteration. + range: model.getFullModelRange(), + text: blob, + }, + ]); + } + + /** + * Add a line highlight style to the node specified by the path. + * + * @param {string|null|false} path A path to a node of the Editor's value, + * e.g. `"foo.bar[0]"`. If the value is falsy, this will remove all + * highlights. + */ + highlight(path) { + if (this.options.highlightPath === path) return; + if (!path) { + SourceEditorExtension.removeHighlights(this); + } else { + const res = this.locate(path); + SourceEditorExtension.highlightLines(this, res); + } + this.options.highlightPath = path || null; + } + + /** + * Return the line numbers of a certain node identified by `path` within + * the yaml. + * + * @param {string} path A path to a node, eg. `foo.bar[0]` + * @returns {number[]} Array following the schema `[firstLine, lastLine]` + * (both inclusive) + * + * @throws {Error} Will throw if the path is not found inside the document + */ + locate(path) { + if (!path) throw Error(`No path provided.`); + const blob = this.getValue(); + const doc = parseDocument(blob); + const pathArray = toPath(path); + + if (!doc.getIn(pathArray)) { + throw Error(`The node ${path} could not be found inside the document.`); + } + + const parentNode = doc.getIn(pathArray.slice(0, pathArray.length - 1)); + let startChar; + let endChar; + if (isMap(parentNode)) { + const node = parentNode.items.find( + (item) => item.key.value === pathArray[pathArray.length - 1], + ); + [startChar] = node.key.range; + [, , endChar] = node.value.range; + } else { + const node = doc.getIn(pathArray); + [startChar, , endChar] = node.range; + } + const startSlice = blob.slice(0, startChar); + const endSlice = blob.slice(0, endChar); + const startLine = (startSlice.match(/\n/g) || []).length + 1; + const endLine = (endSlice.match(/\n/g) || []).length; + return [startLine, endLine]; + } +} diff --git a/app/assets/javascripts/editor/source_editor_extension.js b/app/assets/javascripts/editor/source_editor_extension.js index 664bcabcf45..f6bc62a1c09 100644 --- a/app/assets/javascripts/editor/source_editor_extension.js +++ b/app/assets/javascripts/editor/source_editor_extension.js @@ -12,6 +12,6 @@ export default class EditorExtension { } get api() { - return this.obj.provides(); + return this.obj.provides?.(); } } diff --git a/app/assets/javascripts/editor/source_editor_instance.js b/app/assets/javascripts/editor/source_editor_instance.js new file mode 100644 index 00000000000..e0ca4ea518b --- /dev/null +++ b/app/assets/javascripts/editor/source_editor_instance.js @@ -0,0 +1,271 @@ +/** + * @module source_editor_instance + */ + +/** + * A Source Editor Extension definition + * @typedef {Object} SourceEditorExtensionDefinition + * @property {Object} definition + * @property {Object} setupOptions + */ + +/** + * A Source Editor Extension + * @typedef {Object} SourceEditorExtension + * @property {Object} obj + * @property {string} name + * @property {Object} api + */ + +import { isEqual } from 'lodash'; +import { editor as monacoEditor } from 'monaco-editor'; +import { getBlobLanguage } from '~/editor/utils'; +import { logError } from '~/lib/logger'; +import { sprintf } from '~/locale'; +import EditorExtension from './source_editor_extension'; +import { + EDITOR_EXTENSION_DEFINITION_TYPE_ERROR, + EDITOR_EXTENSION_NAMING_CONFLICT_ERROR, + EDITOR_EXTENSION_NO_DEFINITION_ERROR, + EDITOR_EXTENSION_NOT_REGISTERED_ERROR, + EDITOR_EXTENSION_NOT_SPECIFIED_FOR_UNUSE_ERROR, + EDITOR_EXTENSION_STORE_IS_MISSING_ERROR, +} from './constants'; + +const utils = { + removeExtFromMethod: (method, extensionName, container) => { + if (!container) { + return; + } + if (Object.prototype.hasOwnProperty.call(container, method)) { + // eslint-disable-next-line no-param-reassign + delete container[method]; + } + }, + + getStoredExtension: (extensionsStore, name) => { + if (!extensionsStore) { + logError(EDITOR_EXTENSION_STORE_IS_MISSING_ERROR); + return undefined; + } + return extensionsStore.get(name); + }, +}; + +/** Class representing a Source Editor Instance */ +export default class EditorInstance { + /** + * Create a Source Editor Instance + * @param {Object} rootInstance - Monaco instance to build on top of + * @param {Map} extensionsStore - The global registry for the extension instances + * @returns {Object} - A Proxy returning props/methods from either registered extensions, or Source Editor instance, or underlying Monaco instance + */ + constructor(rootInstance = {}, extensionsStore = new Map()) { + /** The methods provided by extensions. */ + this.methods = {}; + + const seInstance = this; + const getHandler = { + get(target, prop, receiver) { + const methodExtension = + Object.prototype.hasOwnProperty.call(seInstance.methods, prop) && + seInstance.methods[prop]; + if (methodExtension) { + const extension = extensionsStore.get(methodExtension); + + return (...args) => { + return extension.api[prop].call(seInstance, ...args, receiver); + }; + } + return Reflect.get(seInstance[prop] ? seInstance : target, prop, receiver); + }, + set(target, prop, value) { + Object.assign(seInstance, { + [prop]: value, + }); + return true; + }, + }; + const instProxy = new Proxy(rootInstance, getHandler); + + /** + * Main entry point to apply an extension to the instance + * @param {SourceEditorExtensionDefinition} + */ + this.use = EditorInstance.useUnuse.bind(instProxy, extensionsStore, this.useExtension); + + /** + * Main entry point to un-use an extension and remove it from the instance + * @param {SourceEditorExtension} + */ + this.unuse = EditorInstance.useUnuse.bind(instProxy, extensionsStore, this.unuseExtension); + + return instProxy; + } + + /** + * A private dispatcher function for both `use` and `unuse` + * @param {Map} extensionsStore - The global registry for the extension instances + * @param {Function} fn - A function to route to. Either `this.useExtension` or `this.unuseExtension` + * @param {SourceEditorExtensionDefinition[]} extensions - The extensions to use/unuse. + * @returns {Function} + */ + static useUnuse(extensionsStore, fn, extensions) { + if (Array.isArray(extensions)) { + /** + * We cut short if the Array is empty and let the destination function to throw + * Otherwise, we run the destination function on every entry of the Array + */ + return extensions.length + ? extensions.map(fn.bind(this, extensionsStore)) + : fn.call(this, extensionsStore); + } + return fn.call(this, extensionsStore, extensions); + } + + // + // REGISTERING NEW EXTENSION + // + + /** + * Run all registrations when using an extension + * @param {Map} extensionsStore - The global registry for the extension instances + * @param {SourceEditorExtensionDefinition} extension - The extension definition to use. + * @returns {EditorExtension|*} + */ + useExtension(extensionsStore, extension = {}) { + const { definition } = extension; + if (!definition) { + throw new Error(EDITOR_EXTENSION_NO_DEFINITION_ERROR); + } + if (typeof definition !== 'function') { + throw new Error(EDITOR_EXTENSION_DEFINITION_TYPE_ERROR); + } + + // Existing Extension Path + const existingExt = utils.getStoredExtension(extensionsStore, definition.name); + if (existingExt) { + if (isEqual(extension.setupOptions, existingExt.setupOptions)) { + return existingExt; + } + this.unuseExtension(extensionsStore, existingExt); + } + + // New Extension Path + const extensionInstance = new EditorExtension(extension); + const { setupOptions, obj: extensionObj } = extensionInstance; + if (extensionObj.onSetup) { + extensionObj.onSetup(setupOptions, this); + } + if (extensionsStore) { + this.registerExtension(extensionInstance, extensionsStore); + } + this.registerExtensionMethods(extensionInstance); + return extensionInstance; + } + + /** + * Register extension in the global extensions store + * @param {SourceEditorExtension} extension - Instance of Source Editor extension + * @param {Map} extensionsStore - The global registry for the extension instances + */ + registerExtension(extension, extensionsStore) { + const { name } = extension; + const hasExtensionRegistered = + extensionsStore.has(name) && + isEqual(extension.setupOptions, extensionsStore.get(name).setupOptions); + if (hasExtensionRegistered) { + return; + } + extensionsStore.set(name, extension); + const { obj: extensionObj } = extension; + if (extensionObj.onUse) { + extensionObj.onUse(this); + } + } + + /** + * Register extension methods in the registry on the instance + * @param {SourceEditorExtension} extension - Instance of Source Editor extension + */ + registerExtensionMethods(extension) { + const { api, name } = extension; + + if (!api) { + return; + } + + Object.keys(api).forEach((prop) => { + if (this[prop]) { + logError(sprintf(EDITOR_EXTENSION_NAMING_CONFLICT_ERROR, { prop })); + } else { + this.methods[prop] = name; + } + }, this); + } + + // + // UNREGISTERING AN EXTENSION + // + + /** + * Unregister extension with the cleanup + * @param {Map} extensionsStore - The global registry for the extension instances + * @param {SourceEditorExtension} extension - Instance of Source Editor extension to un-use + */ + unuseExtension(extensionsStore, extension) { + if (!extension) { + throw new Error(EDITOR_EXTENSION_NOT_SPECIFIED_FOR_UNUSE_ERROR); + } + const { name } = extension; + const existingExt = utils.getStoredExtension(extensionsStore, name); + if (!existingExt) { + throw new Error(sprintf(EDITOR_EXTENSION_NOT_REGISTERED_ERROR, { name })); + } + const { obj: extensionObj } = existingExt; + if (extensionObj.onBeforeUnuse) { + extensionObj.onBeforeUnuse(this); + } + this.unregisterExtensionMethods(existingExt); + if (extensionObj.onUnuse) { + extensionObj.onUnuse(this); + } + } + + /** + * Remove all methods associated with this extension from the registry on the instance + * @param {SourceEditorExtension} extension - Instance of Source Editor extension to un-use + */ + unregisterExtensionMethods(extension) { + const { api, name } = extension; + if (!api) { + return; + } + Object.keys(api).forEach((method) => { + utils.removeExtFromMethod(method, name, this.methods); + }); + } + + /** + * PUBLIC API OF AN INSTANCE + */ + + /** + * Updates model language based on the path + * @param {String} path - blob path + */ + updateModelLanguage(path) { + const lang = getBlobLanguage(path); + const model = this.getModel(); + // return monacoEditor.setModelLanguage(model, lang); + monacoEditor.setModelLanguage(model, lang); + } + + /** + * Get the methods returned by extensions. + * @returns {Array} + */ + get extensionsAPI() { + return Object.keys(this.methods); + } +} diff --git a/db/post_migrate/20211115151704_add_index_on_projects_import_type_id.rb b/db/post_migrate/20211115151704_add_index_on_projects_import_type_id.rb new file mode 100644 index 00000000000..b54edc1cf73 --- /dev/null +++ b/db/post_migrate/20211115151704_add_index_on_projects_import_type_id.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class AddIndexOnProjectsImportTypeId < Gitlab::Database::Migration[1.0] + disable_ddl_transaction! + + INDEX_NAME = 'index_imported_projects_on_import_type_id' + + def up + add_concurrent_index(:projects, [:import_type, :id], where: 'import_type IS NOT NULL', name: INDEX_NAME) + end + + def down + remove_concurrent_index_by_name(:projects, INDEX_NAME) + end +end diff --git a/db/schema_migrations/20211115151704 b/db/schema_migrations/20211115151704 new file mode 100644 index 00000000000..03093ade126 --- /dev/null +++ b/db/schema_migrations/20211115151704 @@ -0,0 +1 @@ +3db18116febc760bcfeab597e0508d5b2835d0135068d79073770d343aa4b09c \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 5b6614be454..c5d619cb2a3 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -26064,6 +26064,8 @@ CREATE INDEX index_import_failures_on_project_id_not_null ON import_failures USI CREATE INDEX index_imported_projects_on_import_type_creator_id_created_at ON projects USING btree (import_type, creator_id, created_at) WHERE (import_type IS NOT NULL); +CREATE INDEX index_imported_projects_on_import_type_id ON projects USING btree (import_type, id) WHERE (import_type IS NOT NULL); + CREATE INDEX index_in_product_marketing_emails_on_user_id ON in_product_marketing_emails USING btree (user_id); CREATE UNIQUE INDEX index_in_product_marketing_emails_on_user_track_series ON in_product_marketing_emails USING btree (user_id, track, series); diff --git a/doc/api/todos.md b/doc/api/todos.md index 737bfb11da9..98f254f6620 100644 --- a/doc/api/todos.md +++ b/doc/api/todos.md @@ -27,7 +27,7 @@ Parameters: | `project_id` | integer | no | The ID of a project | | `group_id` | integer | no | The ID of a group | | `state` | string | no | The state of the to-do item. Can be either `pending` or `done` | -| `type` | string | no | The type of to-do item. Can be either `Issue`, `MergeRequest`, `DesignManagement::Design` or `AlertManagement::Alert` | +| `type` | string | no | The type of to-do item. Can be either `Issue`, `MergeRequest`, `Commit`, `Epic`, `DesignManagement::Design` or `AlertManagement::Alert` | ```shell curl --header "PRIVATE-TOKEN: " "https://gitlab.example.com/api/v4/todos" diff --git a/doc/development/feature_flags/controls.md b/doc/development/feature_flags/controls.md index 35dbc2703f9..abb100c659e 100644 --- a/doc/development/feature_flags/controls.md +++ b/doc/development/feature_flags/controls.md @@ -90,9 +90,9 @@ This depends on the feature and what sort of impact it might have. Guidelines: -1. If the feature meets the requirements for creating a [Change Management](https://about.gitlab.com/handbook/engineering/infrastructure/change-management/#feature-flags-and-the-change-management-process) issue, create a Change Management issue per [criticality guidelines](https://about.gitlab.com/handbook/engineering/infrastructure/change-management/#change-request-workflows). -1. For simple, low-risk, easily reverted features, proceed and [enable the feature in `#production`](#process). -1. For features that impact the user experience, consider notifying `#support_gitlab-com` beforehand. +- Consider notifying `#support_gitlab-com` beforehand. So in case if the feature has any side effects on user experience, they can mitigate and disable the feature flag to reduce some impact. +- If the feature meets the requirements for creating a [Change Management](https://about.gitlab.com/handbook/engineering/infrastructure/change-management/#feature-flags-and-the-change-management-process) issue, create a Change Management issue per [criticality guidelines](https://about.gitlab.com/handbook/engineering/infrastructure/change-management/#change-request-workflows). +- For simple, low-risk, easily reverted features, proceed and [enable the feature in `#production`](#process). #### Process diff --git a/doc/user/project/merge_requests/accessibility_testing.md b/doc/user/project/merge_requests/accessibility_testing.md index 2bc6d5bb148..8f803f9207c 100644 --- a/doc/user/project/merge_requests/accessibility_testing.md +++ b/doc/user/project/merge_requests/accessibility_testing.md @@ -21,6 +21,9 @@ measuring the accessibility of web sites, and has built a simple This job outputs accessibility violations, warnings, and notices for each page analyzed to a file called `accessibility`. +From [GitLab 14.5](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/73309), the version of `pa11y` uses +[WCAG 2.1 rules](https://www.w3.org/TR/WCAG21/#new-features-in-wcag-2-1), which may report more issues. + ## Accessibility merge request widget > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/39425) in GitLab 13.0 behind the disabled [feature flag](../../../administration/feature_flags.md) `:accessibility_report_view`. diff --git a/doc/user/project/requirements/index.md b/doc/user/project/requirements/index.md index 661bd3e0598..6ee23c91680 100644 --- a/doc/user/project/requirements/index.md +++ b/doc/user/project/requirements/index.md @@ -38,7 +38,9 @@ see [GitLab Requirements Traceability Walkthrough](https://youtu.be/VIiuTQYFVa0) A paginated list of requirements is available in each project, and there you can create a new requirement. -Users with Reporter or higher [permissions](../../permissions.md) can create requirements. +Prerequisite: + +- You must have at least the Reporter [role](../../permissions.md). To create a requirement: @@ -66,7 +68,9 @@ next to the requirement title. You can edit a requirement from the requirements list page. -Users with Reporter or higher [permissions](../../permissions.md) can edit requirements. +Prerequisite: + +- You must have at least the Reporter [role](../../permissions.md). To edit a requirement: @@ -80,7 +84,9 @@ To edit a requirement: You can archive an open requirement while you're in the **Open** tab. -Users with Reporter or higher [permissions](../../permissions.md) can archive requirements. +Prerequisite: + +- You must have at least the Reporter [role](../../permissions.md). To archive a requirement, select **Archive** (**{archive}**). @@ -90,7 +96,9 @@ As soon as a requirement is archived, it no longer appears in the **Open** tab. You can view the list of archived requirements in the **Archived** tab. -Users with Reporter or higher [permissions](../../permissions.md) can reopen archived requirements. +Prerequisite: + +- You must have at least the Reporter [role](../../permissions.md). ![archived requirements list](img/requirements_archived_list_view_v13_1.png) @@ -209,13 +217,13 @@ requirements_confirmation: > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/246857) in GitLab 13.7. +You must have at least the Reporter [role](../../permissions.md). + You can import requirements to a project by uploading a [CSV file](https://en.wikipedia.org/wiki/Comma-separated_values) with the columns `title` and `description`. After the import, the user uploading the CSV file is set as the author of the imported requirements. -Users with Reporter or higher [permissions](../../permissions.md) can import requirements. - ### Import the file Before you import your file: @@ -281,7 +289,9 @@ By exporting requirements, you and your team can import them into another tool o your customers. Exporting requirements can aid collaboration with higher-level systems, as well as audit and regulatory compliance tasks. -Users with Reporter or higher [permissions](../../permissions.md) can export requirements. +Prerequisite: + +- You must have at least the Reporter [role](../../permissions.md). To export requirements: diff --git a/doc/user/project/service_desk.md b/doc/user/project/service_desk.md index 510fcd61cb6..4f8e068ca2d 100644 --- a/doc/user/project/service_desk.md +++ b/doc/user/project/service_desk.md @@ -86,10 +86,10 @@ To improve your project's security, we recommend the following: - [Enable Akismet](../../integration/akismet.md) on your GitLab instance to add spam checking to this service. Unblocked email spam can result in many spam issues being created. -The unique internal email address is visible to project members with Maintainer (or higher) -[permission level](../permissions.md) -in your GitLab instance. However, when using an email alias externally, an end user -(issue creator) cannot see the internal email address displayed in the information note. +The unique internal email address is visible to project members at least +the Reporter [role](../permissions.md) in your GitLab instance. +An external user (issue creator) cannot see the internal email address +displayed in the information note. ### Using customized email templates diff --git a/lib/gitlab/ci/templates/Verify/Accessibility.gitlab-ci.yml b/lib/gitlab/ci/templates/Verify/Accessibility.gitlab-ci.yml index 22c40d8a8b8..4f63ff93d4d 100644 --- a/lib/gitlab/ci/templates/Verify/Accessibility.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Verify/Accessibility.gitlab-ci.yml @@ -13,7 +13,7 @@ stages: a11y: stage: accessibility - image: registry.gitlab.com/gitlab-org/ci-cd/accessibility:5.3.0-gitlab.3 + image: registry.gitlab.com/gitlab-org/ci-cd/accessibility:6.0.1 script: /gitlab-accessibility.sh $a11y_urls allow_failure: true artifacts: diff --git a/lib/gitlab/database/gitlab_schemas.yml b/lib/gitlab/database/gitlab_schemas.yml index ee5039ccab8..b1cae5e8eee 100644 --- a/lib/gitlab/database/gitlab_schemas.yml +++ b/lib/gitlab/database/gitlab_schemas.yml @@ -284,7 +284,7 @@ lfs_objects_projects: :gitlab_main licenses: :gitlab_main lists: :gitlab_main list_user_preferences: :gitlab_main -loose_foreign_keys_deleted_records: :gitlab_main +loose_foreign_keys_deleted_records: :gitlab_shared member_tasks: :gitlab_main members: :gitlab_main merge_request_assignees: :gitlab_main diff --git a/lib/gitlab/usage_data.rb b/lib/gitlab/usage_data.rb index 9c9ecef01bc..20e526aeefa 100644 --- a/lib/gitlab/usage_data.rb +++ b/lib/gitlab/usage_data.rb @@ -618,9 +618,9 @@ module Gitlab todos: distinct_count(::Todo.where(time_period), :author_id), service_desk_enabled_projects: distinct_count_service_desk_enabled_projects(time_period), service_desk_issues: count(::Issue.service_desk.where(time_period)), - projects_jira_active: distinct_count(::Project.with_active_integration(::Integrations::Jira) .where(time_period), :creator_id), - projects_jira_dvcs_cloud_active: distinct_count(::Project.with_active_integration(::Integrations::Jira) .with_jira_dvcs_cloud.where(time_period), :creator_id), - projects_jira_dvcs_server_active: distinct_count(::Project.with_active_integration(::Integrations::Jira) .with_jira_dvcs_server.where(time_period), :creator_id) + projects_jira_active: distinct_count(::Project.with_active_integration(::Integrations::Jira).where(time_period), :creator_id), + projects_jira_dvcs_cloud_active: distinct_count(::Project.with_active_integration(::Integrations::Jira).with_jira_dvcs_cloud.where(time_period), :creator_id), + projects_jira_dvcs_server_active: distinct_count(::Project.with_active_integration(::Integrations::Jira).with_jira_dvcs_server.where(time_period), :creator_id) } end # rubocop: enable CodeReuse/ActiveRecord @@ -915,7 +915,30 @@ module Gitlab end def projects_imported_count(from, time_period) - count(::Project.imported_from(from).where(time_period).where.not(import_type: nil)) # rubocop: disable CodeReuse/ActiveRecord + # rubocop:disable CodeReuse/ActiveRecord + relation = ::Project.imported_from(from).where.not(import_type: nil) # rubocop:disable UsageData/LargeTable + if time_period.empty? + count(relation) + else + @project_import_id ||= {} + start = time_period[:created_at].first + finish = time_period[:created_at].last + + # can be nil values here if no records are in our range and it is possible the same instance + # is called with different time periods since it is passed in as a variable + unless @project_import_id.key?(start) + @project_import_id[start] = ::Project.select(:id).where(Project.arel_table[:created_at].gteq(start)) # rubocop:disable UsageData/LargeTable + .order(created_at: :asc).limit(1).first&.id + end + + unless @project_import_id.key?(finish) + @project_import_id[finish] = ::Project.select(:id).where(Project.arel_table[:created_at].lteq(finish)) # rubocop:disable UsageData/LargeTable + .order(created_at: :desc).limit(1).first&.id + end + + count(relation, start: @project_import_id[start], finish: @project_import_id[finish]) + end + # rubocop:enable CodeReuse/ActiveRecord end def issue_imports(time_period) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 2ecc0c6543e..1f983b2cb1f 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -32697,12 +32697,30 @@ msgstr "" msgid "SourceEditor|\"el\" parameter is required for createInstance()" msgstr "" +msgid "SourceEditor|%{name} is not registered." +msgstr "" + msgid "SourceEditor|Extension definition should be either a class or a function" msgstr "" +msgid "SourceEditor|Extension definition should be either class, function, or an Array of definitions." +msgstr "" + +msgid "SourceEditor|Extensions Store is required to check for an extension." +msgstr "" + +msgid "SourceEditor|Name conflict for \"%{prop}()\" method." +msgstr "" + +msgid "SourceEditor|No extension for unuse has been specified." +msgstr "" + msgid "SourceEditor|Source Editor instance is required to set up an extension." msgstr "" +msgid "SourceEditor|`definition` property is expected on the extension." +msgstr "" + msgid "Sourcegraph" msgstr "" diff --git a/package.json b/package.json index b6f7725d068..a22e53b7722 100644 --- a/package.json +++ b/package.json @@ -199,7 +199,8 @@ "webpack-cli": "^3.3.12", "webpack-stats-plugin": "^0.3.1", "worker-loader": "^2.0.0", - "xterm": "3.14.5" + "xterm": "3.14.5", + "yaml": "^2.0.0-8" }, "devDependencies": { "@babel/plugin-transform-modules-commonjs": "^7.10.1", diff --git a/qa/qa/page/group/show.rb b/qa/qa/page/group/show.rb index 38d919be4db..2cd78f9f17a 100644 --- a/qa/qa/page/group/show.rb +++ b/qa/qa/page/group/show.rb @@ -9,6 +9,7 @@ module QA view 'app/views/groups/_home_panel.html.haml' do element :new_project_button element :new_subgroup_button + element :group_id_content end view 'app/assets/javascripts/groups/constants.js' do @@ -40,6 +41,10 @@ module QA click_element :new_project_button end + def group_id + find_element(:group_id_content).text.delete('Group ID: ') + end + def leave_group accept_alert do click_element :leave_group_link diff --git a/qa/qa/resource/sandbox.rb b/qa/qa/resource/sandbox.rb index 41bf5eac3b2..385e0fa4f7e 100644 --- a/qa/qa/resource/sandbox.rb +++ b/qa/qa/resource/sandbox.rb @@ -25,6 +25,8 @@ module QA alias_method :full_path, :path def fabricate! + Flow::Login.sign_in_unless_signed_in + Page::Main::Menu.perform(&:go_to_groups) Page::Dashboard::Groups.perform do |groups_page| @@ -39,6 +41,8 @@ module QA group.set_visibility('Public') group.create end + + @id = Page::Group::Show.perform(&:group_id) end end end diff --git a/spec/frontend/editor/helpers.js b/spec/frontend/editor/helpers.js new file mode 100644 index 00000000000..6f7cdf6efb3 --- /dev/null +++ b/spec/frontend/editor/helpers.js @@ -0,0 +1,53 @@ +export class MyClassExtension { + // eslint-disable-next-line class-methods-use-this + provides() { + return { + shared: () => 'extension', + classExtMethod: () => 'class own method', + }; + } +} + +export function MyFnExtension() { + return { + fnExtMethod: () => 'fn own method', + provides: () => { + return { + fnExtMethod: () => 'class own method', + }; + }, + }; +} + +export const MyConstExt = () => { + return { + provides: () => { + return { + constExtMethod: () => 'const own method', + }; + }, + }; +}; + +export const conflictingExtensions = { + WithInstanceExt: () => { + return { + provides: () => { + return { + use: () => 'A conflict with instance', + ownMethod: () => 'Non-conflicting method', + }; + }, + }; + }, + WithAnotherExt: () => { + return { + provides: () => { + return { + shared: () => 'A conflict with extension', + ownMethod: () => 'Non-conflicting method', + }; + }, + }; + }, +}; diff --git a/spec/frontend/editor/source_editor_extension_base_spec.js b/spec/frontend/editor/source_editor_extension_base_spec.js index 2c06ae03892..a0fb1178b3b 100644 --- a/spec/frontend/editor/source_editor_extension_base_spec.js +++ b/spec/frontend/editor/source_editor_extension_base_spec.js @@ -148,7 +148,10 @@ describe('The basis for an Source Editor extension', () => { revealLineInCenter: revealSpy, deltaDecorations: decorationsSpy, }; - const defaultDecorationOptions = { isWholeLine: true, className: 'active-line-text' }; + const defaultDecorationOptions = { + isWholeLine: true, + className: 'active-line-text', + }; useFakeRequestAnimationFrame(); @@ -157,18 +160,22 @@ describe('The basis for an Source Editor extension', () => { }); it.each` - desc | hash | shouldReveal | expectedRange - ${'properly decorates a single line'} | ${'#L10'} | ${true} | ${[10, 1, 10, 1]} - ${'properly decorates multiple lines'} | ${'#L7-42'} | ${true} | ${[7, 1, 42, 1]} - ${'correctly highlights if lines are reversed'} | ${'#L42-7'} | ${true} | ${[7, 1, 42, 1]} - ${'highlights one line if start/end are the same'} | ${'#L7-7'} | ${true} | ${[7, 1, 7, 1]} - ${'does not highlight if there is no hash'} | ${''} | ${false} | ${null} - ${'does not highlight if the hash is undefined'} | ${undefined} | ${false} | ${null} - ${'does not highlight if hash is incomplete 1'} | ${'#L'} | ${false} | ${null} - ${'does not highlight if hash is incomplete 2'} | ${'#L-'} | ${false} | ${null} - `('$desc', ({ hash, shouldReveal, expectedRange } = {}) => { + desc | hash | bounds | shouldReveal | expectedRange + ${'properly decorates a single line'} | ${'#L10'} | ${undefined} | ${true} | ${[10, 1, 10, 1]} + ${'properly decorates multiple lines'} | ${'#L7-42'} | ${undefined} | ${true} | ${[7, 1, 42, 1]} + ${'correctly highlights if lines are reversed'} | ${'#L42-7'} | ${undefined} | ${true} | ${[7, 1, 42, 1]} + ${'highlights one line if start/end are the same'} | ${'#L7-7'} | ${undefined} | ${true} | ${[7, 1, 7, 1]} + ${'does not highlight if there is no hash'} | ${''} | ${undefined} | ${false} | ${null} + ${'does not highlight if the hash is undefined'} | ${undefined} | ${undefined} | ${false} | ${null} + ${'does not highlight if hash is incomplete 1'} | ${'#L'} | ${undefined} | ${false} | ${null} + ${'does not highlight if hash is incomplete 2'} | ${'#L-'} | ${undefined} | ${false} | ${null} + ${'highlights lines if bounds are passed'} | ${undefined} | ${[17, 42]} | ${true} | ${[17, 1, 42, 1]} + ${'highlights one line if bounds has a single value'} | ${undefined} | ${[17]} | ${true} | ${[17, 1, 17, 1]} + ${'does not highlight if bounds is invalid'} | ${undefined} | ${[Number.NaN]} | ${false} | ${null} + ${'uses bounds if both hash and bounds exist'} | ${'#L7-42'} | ${[3, 5]} | ${true} | ${[3, 1, 5, 1]} + `('$desc', ({ hash, bounds, shouldReveal, expectedRange } = {}) => { window.location.hash = hash; - SourceEditorExtension.highlightLines(instance); + SourceEditorExtension.highlightLines(instance, bounds); if (!shouldReveal) { expect(revealSpy).not.toHaveBeenCalled(); expect(decorationsSpy).not.toHaveBeenCalled(); @@ -193,6 +200,43 @@ describe('The basis for an Source Editor extension', () => { SourceEditorExtension.highlightLines(instance); expect(instance.lineDecorations).toBe('foo'); }); + + it('replaces existing line highlights', () => { + const oldLineDecorations = [ + { + range: new Range(1, 1, 20, 1), + options: { isWholeLine: true, className: 'active-line-text' }, + }, + ]; + const newLineDecorations = [ + { + range: new Range(7, 1, 10, 1), + options: { isWholeLine: true, className: 'active-line-text' }, + }, + ]; + instance.lineDecorations = oldLineDecorations; + SourceEditorExtension.highlightLines(instance, [7, 10]); + expect(decorationsSpy).toHaveBeenCalledWith(oldLineDecorations, newLineDecorations); + }); + }); + + describe('removeHighlights', () => { + const decorationsSpy = jest.fn(); + const lineDecorations = [ + { + range: new Range(1, 1, 20, 1), + options: { isWholeLine: true, className: 'active-line-text' }, + }, + ]; + const instance = { + deltaDecorations: decorationsSpy, + lineDecorations, + }; + + it('removes all existing decorations', () => { + SourceEditorExtension.removeHighlights(instance); + expect(decorationsSpy).toHaveBeenCalledWith(lineDecorations, []); + }); }); describe('setupLineLinking', () => { diff --git a/spec/frontend/editor/source_editor_extension_spec.js b/spec/frontend/editor/source_editor_extension_spec.js index ebeeae7e42f..6f2eb07a043 100644 --- a/spec/frontend/editor/source_editor_extension_spec.js +++ b/spec/frontend/editor/source_editor_extension_spec.js @@ -1,37 +1,6 @@ import EditorExtension from '~/editor/source_editor_extension'; import { EDITOR_EXTENSION_DEFINITION_ERROR } from '~/editor/constants'; - -class MyClassExtension { - // eslint-disable-next-line class-methods-use-this - provides() { - return { - shared: () => 'extension', - classExtMethod: () => 'class own method', - }; - } -} - -function MyFnExtension() { - return { - fnExtMethod: () => 'fn own method', - provides: () => { - return { - shared: () => 'extension', - }; - }, - }; -} - -const MyConstExt = () => { - return { - provides: () => { - return { - shared: () => 'extension', - constExtMethod: () => 'const own method', - }; - }, - }; -}; +import * as helpers from './helpers'; describe('Editor Extension', () => { const dummyObj = { foo: 'bar' }; @@ -52,16 +21,16 @@ describe('Editor Extension', () => { ); it.each` - definition | setupOptions | expectedName - ${MyClassExtension} | ${undefined} | ${'MyClassExtension'} - ${MyClassExtension} | ${{}} | ${'MyClassExtension'} - ${MyClassExtension} | ${dummyObj} | ${'MyClassExtension'} - ${MyFnExtension} | ${undefined} | ${'MyFnExtension'} - ${MyFnExtension} | ${{}} | ${'MyFnExtension'} - ${MyFnExtension} | ${dummyObj} | ${'MyFnExtension'} - ${MyConstExt} | ${undefined} | ${'MyConstExt'} - ${MyConstExt} | ${{}} | ${'MyConstExt'} - ${MyConstExt} | ${dummyObj} | ${'MyConstExt'} + definition | setupOptions | expectedName + ${helpers.MyClassExtension} | ${undefined} | ${'MyClassExtension'} + ${helpers.MyClassExtension} | ${{}} | ${'MyClassExtension'} + ${helpers.MyClassExtension} | ${dummyObj} | ${'MyClassExtension'} + ${helpers.MyFnExtension} | ${undefined} | ${'MyFnExtension'} + ${helpers.MyFnExtension} | ${{}} | ${'MyFnExtension'} + ${helpers.MyFnExtension} | ${dummyObj} | ${'MyFnExtension'} + ${helpers.MyConstExt} | ${undefined} | ${'MyConstExt'} + ${helpers.MyConstExt} | ${{}} | ${'MyConstExt'} + ${helpers.MyConstExt} | ${dummyObj} | ${'MyConstExt'} `( 'correctly creates extension for definition = $definition and setupOptions = $setupOptions', ({ definition, setupOptions, expectedName }) => { @@ -81,10 +50,10 @@ describe('Editor Extension', () => { describe('api', () => { it.each` - definition | expectedKeys - ${MyClassExtension} | ${['shared', 'classExtMethod']} - ${MyFnExtension} | ${['shared']} - ${MyConstExt} | ${['shared', 'constExtMethod']} + definition | expectedKeys + ${helpers.MyClassExtension} | ${['shared', 'classExtMethod']} + ${helpers.MyFnExtension} | ${['fnExtMethod']} + ${helpers.MyConstExt} | ${['constExtMethod']} `('correctly returns API for $definition', ({ definition, expectedKeys }) => { const extension = new EditorExtension({ definition }); const expectedApi = Object.fromEntries( diff --git a/spec/frontend/editor/source_editor_instance_spec.js b/spec/frontend/editor/source_editor_instance_spec.js new file mode 100644 index 00000000000..87b20a4ba73 --- /dev/null +++ b/spec/frontend/editor/source_editor_instance_spec.js @@ -0,0 +1,387 @@ +import { editor as monacoEditor } from 'monaco-editor'; +import { + EDITOR_EXTENSION_NAMING_CONFLICT_ERROR, + EDITOR_EXTENSION_NO_DEFINITION_ERROR, + EDITOR_EXTENSION_DEFINITION_TYPE_ERROR, + EDITOR_EXTENSION_NOT_REGISTERED_ERROR, + EDITOR_EXTENSION_NOT_SPECIFIED_FOR_UNUSE_ERROR, +} from '~/editor/constants'; +import Instance from '~/editor/source_editor_instance'; +import { sprintf } from '~/locale'; +import { MyClassExtension, conflictingExtensions, MyFnExtension, MyConstExt } from './helpers'; + +describe('Source Editor Instance', () => { + let seInstance; + + const defSetupOptions = { foo: 'bar' }; + const fullExtensionsArray = [ + { definition: MyClassExtension }, + { definition: MyFnExtension }, + { definition: MyConstExt }, + ]; + const fullExtensionsArrayWithOptions = [ + { definition: MyClassExtension, setupOptions: defSetupOptions }, + { definition: MyFnExtension, setupOptions: defSetupOptions }, + { definition: MyConstExt, setupOptions: defSetupOptions }, + ]; + + const fooFn = jest.fn(); + class DummyExt { + // eslint-disable-next-line class-methods-use-this + provides() { + return { + fooFn, + }; + } + } + + afterEach(() => { + seInstance = undefined; + }); + + it('sets up the registry for the methods coming from extensions', () => { + seInstance = new Instance(); + expect(seInstance.methods).toBeDefined(); + + seInstance.use({ definition: MyClassExtension }); + expect(seInstance.methods).toEqual({ + shared: 'MyClassExtension', + classExtMethod: 'MyClassExtension', + }); + + seInstance.use({ definition: MyFnExtension }); + expect(seInstance.methods).toEqual({ + shared: 'MyClassExtension', + classExtMethod: 'MyClassExtension', + fnExtMethod: 'MyFnExtension', + }); + }); + + describe('proxy', () => { + it('returns prop from an extension if extension provides it', () => { + seInstance = new Instance(); + seInstance.use({ definition: DummyExt }); + + expect(fooFn).not.toHaveBeenCalled(); + seInstance.fooFn(); + expect(fooFn).toHaveBeenCalled(); + }); + + it('returns props from SE instance itself if no extension provides the prop', () => { + seInstance = new Instance({ + use: fooFn, + }); + jest.spyOn(seInstance, 'use').mockImplementation(() => {}); + expect(seInstance.use).not.toHaveBeenCalled(); + expect(fooFn).not.toHaveBeenCalled(); + seInstance.use(); + expect(seInstance.use).toHaveBeenCalled(); + expect(fooFn).not.toHaveBeenCalled(); + }); + + it('returns props from Monaco instance when the prop does not exist on the SE instance', () => { + seInstance = new Instance({ + fooFn, + }); + + expect(fooFn).not.toHaveBeenCalled(); + seInstance.fooFn(); + expect(fooFn).toHaveBeenCalled(); + }); + }); + + describe('public API', () => { + it.each(['use', 'unuse'], 'provides "%s" as public method by default', (method) => { + seInstance = new Instance(); + expect(seInstance[method]).toBeDefined(); + }); + + describe('use', () => { + it('extends the SE instance with methods provided by an extension', () => { + seInstance = new Instance(); + seInstance.use({ definition: DummyExt }); + + expect(fooFn).not.toHaveBeenCalled(); + seInstance.fooFn(); + expect(fooFn).toHaveBeenCalled(); + }); + + it.each` + extensions | expectedProps + ${{ definition: MyClassExtension }} | ${['shared', 'classExtMethod']} + ${{ definition: MyFnExtension }} | ${['fnExtMethod']} + ${{ definition: MyConstExt }} | ${['constExtMethod']} + ${fullExtensionsArray} | ${['shared', 'classExtMethod', 'fnExtMethod', 'constExtMethod']} + ${fullExtensionsArrayWithOptions} | ${['shared', 'classExtMethod', 'fnExtMethod', 'constExtMethod']} + `( + 'Should register $expectedProps when extension is "$extensions"', + ({ extensions, expectedProps }) => { + seInstance = new Instance(); + expect(seInstance.extensionsAPI).toHaveLength(0); + + seInstance.use(extensions); + + expect(seInstance.extensionsAPI).toEqual(expectedProps); + }, + ); + + it.each` + definition | preInstalledExtDefinition | expectedErrorProp + ${conflictingExtensions.WithInstanceExt} | ${MyClassExtension} | ${'use'} + ${conflictingExtensions.WithInstanceExt} | ${null} | ${'use'} + ${conflictingExtensions.WithAnotherExt} | ${null} | ${undefined} + ${conflictingExtensions.WithAnotherExt} | ${MyClassExtension} | ${'shared'} + ${MyClassExtension} | ${conflictingExtensions.WithAnotherExt} | ${'shared'} + `( + 'logs the naming conflict error when registering $definition', + ({ definition, preInstalledExtDefinition, expectedErrorProp }) => { + seInstance = new Instance(); + jest.spyOn(console, 'error').mockImplementation(() => {}); + + if (preInstalledExtDefinition) { + seInstance.use({ definition: preInstalledExtDefinition }); + // eslint-disable-next-line no-console + expect(console.error).not.toHaveBeenCalled(); + } + + seInstance.use({ definition }); + + if (expectedErrorProp) { + // eslint-disable-next-line no-console + expect(console.error).toHaveBeenCalledWith( + expect.any(String), + expect.stringContaining( + sprintf(EDITOR_EXTENSION_NAMING_CONFLICT_ERROR, { prop: expectedErrorProp }), + ), + ); + } else { + // eslint-disable-next-line no-console + expect(console.error).not.toHaveBeenCalled(); + } + }, + ); + + it.each` + extensions | thrownError + ${''} | ${EDITOR_EXTENSION_NO_DEFINITION_ERROR} + ${undefined} | ${EDITOR_EXTENSION_NO_DEFINITION_ERROR} + ${{}} | ${EDITOR_EXTENSION_NO_DEFINITION_ERROR} + ${{ foo: 'bar' }} | ${EDITOR_EXTENSION_NO_DEFINITION_ERROR} + ${{ definition: '' }} | ${EDITOR_EXTENSION_NO_DEFINITION_ERROR} + ${{ definition: undefined }} | ${EDITOR_EXTENSION_NO_DEFINITION_ERROR} + ${{ definition: [] }} | ${EDITOR_EXTENSION_DEFINITION_TYPE_ERROR} + ${{ definition: {} }} | ${EDITOR_EXTENSION_DEFINITION_TYPE_ERROR} + ${{ definition: { foo: 'bar' } }} | ${EDITOR_EXTENSION_DEFINITION_TYPE_ERROR} + `( + 'Should throw $thrownError when extension is "$extensions"', + ({ extensions, thrownError }) => { + seInstance = new Instance(); + const useExtension = () => { + seInstance.use(extensions); + }; + expect(useExtension).toThrowError(thrownError); + }, + ); + + describe('global extensions registry', () => { + let extensionStore; + + beforeEach(() => { + extensionStore = new Map(); + seInstance = new Instance({}, extensionStore); + }); + + it('stores _instances_ of the used extensions in a global registry', () => { + const extension = seInstance.use({ definition: MyClassExtension }); + + expect(extensionStore.size).toBe(1); + expect(extensionStore.entries().next().value).toEqual(['MyClassExtension', extension]); + }); + + it('does not duplicate entries in the registry', () => { + jest.spyOn(extensionStore, 'set'); + + const extension1 = seInstance.use({ definition: MyClassExtension }); + seInstance.use({ definition: MyClassExtension }); + + expect(extensionStore.set).toHaveBeenCalledTimes(1); + expect(extensionStore.set).toHaveBeenCalledWith('MyClassExtension', extension1); + }); + + it.each` + desc | currentSetupOptions | newSetupOptions | expectedCallTimes + ${'updates'} | ${undefined} | ${defSetupOptions} | ${2} + ${'updates'} | ${defSetupOptions} | ${undefined} | ${2} + ${'updates'} | ${{ foo: 'bar' }} | ${{ foo: 'new' }} | ${2} + ${'does not update'} | ${undefined} | ${undefined} | ${1} + ${'does not update'} | ${{}} | ${{}} | ${1} + ${'does not update'} | ${defSetupOptions} | ${defSetupOptions} | ${1} + `( + '$desc the extensions entry when setupOptions "$currentSetupOptions" get changed to "$newSetupOptions"', + ({ currentSetupOptions, newSetupOptions, expectedCallTimes }) => { + jest.spyOn(extensionStore, 'set'); + + const extension1 = seInstance.use({ + definition: MyClassExtension, + setupOptions: currentSetupOptions, + }); + const extension2 = seInstance.use({ + definition: MyClassExtension, + setupOptions: newSetupOptions, + }); + + expect(extensionStore.size).toBe(1); + expect(extensionStore.set).toHaveBeenCalledTimes(expectedCallTimes); + if (expectedCallTimes > 1) { + expect(extensionStore.set).toHaveBeenCalledWith('MyClassExtension', extension2); + } else { + expect(extensionStore.set).toHaveBeenCalledWith('MyClassExtension', extension1); + } + }, + ); + }); + }); + + describe('unuse', () => { + it.each` + unuseExtension | thrownError + ${undefined} | ${EDITOR_EXTENSION_NOT_SPECIFIED_FOR_UNUSE_ERROR} + ${''} | ${EDITOR_EXTENSION_NOT_SPECIFIED_FOR_UNUSE_ERROR} + ${{}} | ${sprintf(EDITOR_EXTENSION_NOT_REGISTERED_ERROR, { name: '' })} + ${[]} | ${EDITOR_EXTENSION_NOT_SPECIFIED_FOR_UNUSE_ERROR} + `( + `Should throw "${EDITOR_EXTENSION_NOT_SPECIFIED_FOR_UNUSE_ERROR}" when extension is "$unuseExtension"`, + ({ unuseExtension, thrownError }) => { + seInstance = new Instance(); + const unuse = () => { + seInstance.unuse(unuseExtension); + }; + expect(unuse).toThrowError(thrownError); + }, + ); + + it.each` + initExtensions | unuseExtensionIndex | remainingAPI + ${{ definition: MyClassExtension }} | ${0} | ${[]} + ${{ definition: MyFnExtension }} | ${0} | ${[]} + ${{ definition: MyConstExt }} | ${0} | ${[]} + ${fullExtensionsArray} | ${0} | ${['fnExtMethod', 'constExtMethod']} + ${fullExtensionsArray} | ${1} | ${['shared', 'classExtMethod', 'constExtMethod']} + ${fullExtensionsArray} | ${2} | ${['shared', 'classExtMethod', 'fnExtMethod']} + `( + 'un-registers properties introduced by single extension $unuseExtension', + ({ initExtensions, unuseExtensionIndex, remainingAPI }) => { + seInstance = new Instance(); + const extensions = seInstance.use(initExtensions); + + if (Array.isArray(initExtensions)) { + seInstance.unuse(extensions[unuseExtensionIndex]); + } else { + seInstance.unuse(extensions); + } + expect(seInstance.extensionsAPI).toEqual(remainingAPI); + }, + ); + + it.each` + unuseExtensionIndex | remainingAPI + ${[0, 1]} | ${['constExtMethod']} + ${[0, 2]} | ${['fnExtMethod']} + ${[1, 2]} | ${['shared', 'classExtMethod']} + `( + 'un-registers properties introduced by multiple extensions $unuseExtension', + ({ unuseExtensionIndex, remainingAPI }) => { + seInstance = new Instance(); + const extensions = seInstance.use(fullExtensionsArray); + const extensionsToUnuse = extensions.filter((ext, index) => + unuseExtensionIndex.includes(index), + ); + + seInstance.unuse(extensionsToUnuse); + expect(seInstance.extensionsAPI).toEqual(remainingAPI); + }, + ); + + it('it does not remove entry from the global registry to keep for potential future re-use', () => { + const extensionStore = new Map(); + seInstance = new Instance({}, extensionStore); + const extensions = seInstance.use(fullExtensionsArray); + const verifyExpectations = () => { + const entries = extensionStore.entries(); + const mockExtensions = ['MyClassExtension', 'MyFnExtension', 'MyConstExt']; + expect(extensionStore.size).toBe(mockExtensions.length); + mockExtensions.forEach((ext, index) => { + expect(entries.next().value).toEqual([ext, extensions[index]]); + }); + }; + + verifyExpectations(); + seInstance.unuse(extensions); + verifyExpectations(); + }); + }); + + describe('updateModelLanguage', () => { + let instanceModel; + + beforeEach(() => { + instanceModel = monacoEditor.createModel(''); + seInstance = new Instance({ + getModel: () => instanceModel, + }); + }); + + it.each` + path | expectedLanguage + ${'foo.js'} | ${'javascript'} + ${'foo.md'} | ${'markdown'} + ${'foo.rb'} | ${'ruby'} + ${''} | ${'plaintext'} + ${undefined} | ${'plaintext'} + ${'test.nonexistingext'} | ${'plaintext'} + `( + 'changes language of an attached model to "$expectedLanguage" when filepath is "$path"', + ({ path, expectedLanguage }) => { + seInstance.updateModelLanguage(path); + expect(instanceModel.getLanguageIdentifier().language).toBe(expectedLanguage); + }, + ); + }); + + describe('extensions life-cycle callbacks', () => { + const onSetup = jest.fn().mockImplementation(() => {}); + const onUse = jest.fn().mockImplementation(() => {}); + const onBeforeUnuse = jest.fn().mockImplementation(() => {}); + const onUnuse = jest.fn().mockImplementation(() => {}); + const MyFullExtWithCallbacks = () => { + return { + onSetup, + onUse, + onBeforeUnuse, + onUnuse, + }; + }; + + it('passes correct arguments to callback fns when using an extension', () => { + seInstance = new Instance(); + seInstance.use({ + definition: MyFullExtWithCallbacks, + setupOptions: defSetupOptions, + }); + expect(onSetup).toHaveBeenCalledWith(defSetupOptions, seInstance); + expect(onUse).toHaveBeenCalledWith(seInstance); + }); + + it('passes correct arguments to callback fns when un-using an extension', () => { + seInstance = new Instance(); + const extension = seInstance.use({ + definition: MyFullExtWithCallbacks, + setupOptions: defSetupOptions, + }); + seInstance.unuse(extension); + expect(onBeforeUnuse).toHaveBeenCalledWith(seInstance); + expect(onUnuse).toHaveBeenCalledWith(seInstance); + }); + }); + }); +}); diff --git a/spec/frontend/editor/source_editor_yaml_ext_spec.js b/spec/frontend/editor/source_editor_yaml_ext_spec.js new file mode 100644 index 00000000000..97d2b0b21d0 --- /dev/null +++ b/spec/frontend/editor/source_editor_yaml_ext_spec.js @@ -0,0 +1,449 @@ +import { Document } from 'yaml'; +import SourceEditor from '~/editor/source_editor'; +import { YamlEditorExtension } from '~/editor/extensions/source_editor_yaml_ext'; +import { SourceEditorExtension } from '~/editor/extensions/source_editor_extension_base'; + +const getEditorInstance = (editorInstanceOptions = {}) => { + setFixtures('
'); + return new SourceEditor().createInstance({ + el: document.getElementById('editor'), + blobPath: '.gitlab-ci.yml', + language: 'yaml', + ...editorInstanceOptions, + }); +}; + +const getEditorInstanceWithExtension = (extensionOptions = {}, editorInstanceOptions = {}) => { + setFixtures('
'); + const instance = getEditorInstance(editorInstanceOptions); + instance.use(new YamlEditorExtension({ instance, ...extensionOptions })); + + // Remove the below once + // https://gitlab.com/gitlab-org/gitlab/-/issues/325992 is resolved + if (editorInstanceOptions.value && !extensionOptions.model) { + instance.setValue(editorInstanceOptions.value); + } + + return instance; +}; + +describe('YamlCreatorExtension', () => { + describe('constructor', () => { + it('saves constructor options', () => { + const instance = getEditorInstanceWithExtension({ + highlightPath: 'foo', + enableComments: true, + }); + expect(instance).toEqual( + expect.objectContaining({ + options: expect.objectContaining({ + highlightPath: 'foo', + enableComments: true, + }), + }), + ); + }); + + it('dumps values loaded with the model constructor options', () => { + const model = { foo: 'bar' }; + const expected = 'foo: bar\n'; + const instance = getEditorInstanceWithExtension({ model }); + expect(instance.getDoc().get('foo')).toBeDefined(); + expect(instance.getValue()).toEqual(expected); + }); + + it('registers the onUpdate() function', () => { + const instance = getEditorInstance(); + const onDidChangeModelContent = jest.spyOn(instance, 'onDidChangeModelContent'); + instance.use(new YamlEditorExtension({ instance })); + expect(onDidChangeModelContent).toHaveBeenCalledWith(expect.any(Function)); + }); + + it("If not provided with a load constructor option, it will parse the editor's value", () => { + const editorValue = 'foo: bar'; + const instance = getEditorInstanceWithExtension({}, { value: editorValue }); + expect(instance.getDoc().get('foo')).toBeDefined(); + }); + + it("Prefers values loaded with the load constructor option over the editor's existing value", () => { + const editorValue = 'oldValue: this should be overriden'; + const model = { thisShould: 'be the actual value' }; + const expected = 'thisShould: be the actual value\n'; + const instance = getEditorInstanceWithExtension({ model }, { value: editorValue }); + expect(instance.getDoc().get('oldValue')).toBeUndefined(); + expect(instance.getValue()).toEqual(expected); + }); + }); + + describe('initFromModel', () => { + const model = { foo: 'bar', 1: 2, abc: ['def'] }; + const doc = new Document(model); + + it('should call transformComments if enableComments is true', () => { + const instance = getEditorInstanceWithExtension({ enableComments: true }); + const transformComments = jest.spyOn(YamlEditorExtension, 'transformComments'); + YamlEditorExtension.initFromModel(instance, model); + expect(transformComments).toHaveBeenCalled(); + }); + + it('should not call transformComments if enableComments is false', () => { + const instance = getEditorInstanceWithExtension({ enableComments: false }); + const transformComments = jest.spyOn(YamlEditorExtension, 'transformComments'); + YamlEditorExtension.initFromModel(instance, model); + expect(transformComments).not.toHaveBeenCalled(); + }); + + it('should call setValue with the stringified model', () => { + const instance = getEditorInstanceWithExtension(); + const setValue = jest.spyOn(instance, 'setValue'); + YamlEditorExtension.initFromModel(instance, model); + expect(setValue).toHaveBeenCalledWith(doc.toString()); + }); + }); + + describe('wrapCommentString', () => { + const longString = + 'Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet. Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet.'; + + it('should add spaces before each line', () => { + const result = YamlEditorExtension.wrapCommentString(longString); + const lines = result.split('\n'); + expect(lines.every((ln) => ln.startsWith(' '))).toBe(true); + }); + + it('should break long comments into lines of max. 79 chars', () => { + // 79 = 80 char width minus 1 char for the '#' at the start of each line + const result = YamlEditorExtension.wrapCommentString(longString); + const lines = result.split('\n'); + expect(lines.every((ln) => ln.length <= 79)).toBe(true); + }); + + it('should decrease the line width if passed a level by 2 chars per level', () => { + for (let i = 0; i <= 5; i += 1) { + const result = YamlEditorExtension.wrapCommentString(longString, i); + const lines = result.split('\n'); + const decreaseLineWidthBy = i * 2; + const maxLineWith = 79 - decreaseLineWidthBy; + const isValidLine = (ln) => { + if (ln.length <= maxLineWith) return true; + // The line may exceed the max line width in case the word is the + // only one in the line and thus cannot be broken further + return ln.split(' ').length <= 1; + }; + expect(lines.every(isValidLine)).toBe(true); + } + }); + + it('return null if passed an invalid string value', () => { + expect(YamlEditorExtension.wrapCommentString(null)).toBe(null); + expect(YamlEditorExtension.wrapCommentString()).toBe(null); + }); + + it('throw an error if passed an invalid level value', () => { + expect(() => YamlEditorExtension.wrapCommentString('abc', -5)).toThrow( + 'Invalid value "-5" for variable `level`', + ); + expect(() => YamlEditorExtension.wrapCommentString('abc', 'invalid')).toThrow( + 'Invalid value "invalid" for variable `level`', + ); + }); + }); + + describe('transformComments', () => { + const getInstanceWithModel = (model) => { + return getEditorInstanceWithExtension({ + model, + enableComments: true, + }); + }; + + it('converts comments inside an array', () => { + const model = ['# test comment', 'def', '# foo', 999]; + const expected = `# test comment\n- def\n# foo\n- 999\n`; + const instance = getInstanceWithModel(model); + expect(instance.getValue()).toEqual(expected); + }); + + it('converts generic comments inside an object and places them at the top', () => { + const model = { foo: 'bar', 1: 2, '#': 'test comment' }; + const expected = `# test comment\n"1": 2\nfoo: bar\n`; + const instance = getInstanceWithModel(model); + expect(instance.getValue()).toEqual(expected); + }); + + it('adds specific comments before the mentioned entry of an object', () => { + const model = { foo: 'bar', 1: 2, '#|foo': 'foo comment' }; + const expected = `"1": 2\n# foo comment\nfoo: bar\n`; + const instance = getInstanceWithModel(model); + expect(instance.getValue()).toEqual(expected); + }); + + it('limits long comments to 80 char width, including indentation', () => { + const model = { + '#|foo': + 'Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum.', + foo: { + nested1: { + nested2: { + nested3: { + '#|bar': + 'Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum.', + bar: 'baz', + }, + }, + }, + }, + }; + const expected = `# Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy +# eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam +# voluptua. At vero eos et accusam et justo duo dolores et ea rebum. +foo: + nested1: + nested2: + nested3: + # Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam + # nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, + # sed diam voluptua. At vero eos et accusam et justo duo dolores et ea + # rebum. + bar: baz +`; + const instance = getInstanceWithModel(model); + expect(instance.getValue()).toEqual(expected); + }); + }); + + describe('getDoc', () => { + it('returns a yaml `Document` Type', () => { + const instance = getEditorInstanceWithExtension(); + expect(instance.getDoc()).toBeInstanceOf(Document); + }); + }); + + describe('setDoc', () => { + const model = { foo: 'bar', 1: 2, abc: ['def'] }; + const doc = new Document(model); + + it('should call transformComments if enableComments is true', () => { + const spy = jest.spyOn(YamlEditorExtension, 'transformComments'); + const instance = getEditorInstanceWithExtension({ enableComments: true }); + instance.setDoc(doc); + expect(spy).toHaveBeenCalledWith(doc); + }); + + it('should not call transformComments if enableComments is false', () => { + const spy = jest.spyOn(YamlEditorExtension, 'transformComments'); + const instance = getEditorInstanceWithExtension({ enableComments: false }); + instance.setDoc(doc); + expect(spy).not.toHaveBeenCalled(); + }); + + it("should call setValue with the stringified doc if the editor's value is empty", () => { + const instance = getEditorInstanceWithExtension(); + const setValue = jest.spyOn(instance, 'setValue'); + const updateValue = jest.spyOn(instance, 'updateValue'); + instance.setDoc(doc); + expect(setValue).toHaveBeenCalledWith(doc.toString()); + expect(updateValue).not.toHaveBeenCalled(); + }); + + it("should call updateValue with the stringified doc if the editor's value is not empty", () => { + const instance = getEditorInstanceWithExtension({}, { value: 'asjkdhkasjdh' }); + const setValue = jest.spyOn(instance, 'setValue'); + const updateValue = jest.spyOn(instance, 'updateValue'); + instance.setDoc(doc); + expect(setValue).not.toHaveBeenCalled(); + expect(updateValue).toHaveBeenCalledWith(doc.toString()); + }); + + it('should trigger the onUpdate method', () => { + const instance = getEditorInstanceWithExtension(); + const onUpdate = jest.spyOn(instance, 'onUpdate'); + instance.setDoc(doc); + expect(onUpdate).toHaveBeenCalled(); + }); + }); + + describe('getDataModel', () => { + it('returns the model as JS', () => { + const value = 'abc: def\nfoo:\n - bar\n - baz\n'; + const expected = { abc: 'def', foo: ['bar', 'baz'] }; + const instance = getEditorInstanceWithExtension({}, { value }); + expect(instance.getDataModel()).toEqual(expected); + }); + }); + + describe('setDataModel', () => { + it('sets the value to a YAML-representation of the Doc', () => { + const model = { + abc: ['def'], + '#|foo': 'foo comment', + foo: { + '#|abc': 'abc comment', + abc: [{ def: 'ghl', lorem: 'ipsum' }, '# array comment', null], + bar: 'baz', + }, + }; + const expected = + 'abc:\n' + + ' - def\n' + + '# foo comment\n' + + 'foo:\n' + + ' # abc comment\n' + + ' abc:\n' + + ' - def: ghl\n' + + ' lorem: ipsum\n' + + ' # array comment\n' + + ' - null\n' + + ' bar: baz\n'; + + const instance = getEditorInstanceWithExtension({ enableComments: true }); + const setValue = jest.spyOn(instance, 'setValue'); + + instance.setDataModel(model); + + expect(setValue).toHaveBeenCalledWith(expected); + }); + + it('causes the editor value to be updated', () => { + const initialModel = { foo: 'this should be overriden' }; + const initialValue = 'foo: this should be overriden\n'; + const newValue = { thisShould: 'be the actual value' }; + const expected = 'thisShould: be the actual value\n'; + const instance = getEditorInstanceWithExtension({ model: initialModel }); + expect(instance.getValue()).toEqual(initialValue); + instance.setDataModel(newValue); + expect(instance.getValue()).toEqual(expected); + }); + }); + + describe('onUpdate', () => { + it('calls highlight', () => { + const highlightPath = 'foo'; + const instance = getEditorInstanceWithExtension({ highlightPath }); + instance.highlight = jest.fn(); + instance.onUpdate(); + expect(instance.highlight).toHaveBeenCalledWith(highlightPath); + }); + }); + + describe('updateValue', () => { + it("causes the editor's value to be updated", () => { + const oldValue = 'foobar'; + const newValue = 'bazboo'; + const instance = getEditorInstanceWithExtension({}, { value: oldValue }); + instance.updateValue(newValue); + expect(instance.getValue()).toEqual(newValue); + }); + }); + + describe('highlight', () => { + const highlightPathOnSetup = 'abc'; + const value = `foo: + bar: + - baz + - boo + abc: def +`; + let instance; + let highlightLinesSpy; + let removeHighlightsSpy; + + beforeEach(() => { + instance = getEditorInstanceWithExtension({ highlightPath: highlightPathOnSetup }, { value }); + highlightLinesSpy = jest.spyOn(SourceEditorExtension, 'highlightLines'); + removeHighlightsSpy = jest.spyOn(SourceEditorExtension, 'removeHighlights'); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + it('saves the highlighted path in highlightPath', () => { + const path = 'foo.bar'; + instance.highlight(path); + expect(instance.options.highlightPath).toEqual(path); + }); + + it('calls highlightLines with a number of lines', () => { + const path = 'foo.bar'; + instance.highlight(path); + expect(highlightLinesSpy).toHaveBeenCalledWith(instance, [2, 4]); + }); + + it('calls removeHighlights if path is null', () => { + instance.highlight(null); + expect(removeHighlightsSpy).toHaveBeenCalledWith(instance); + expect(highlightLinesSpy).not.toHaveBeenCalled(); + expect(instance.options.highlightPath).toBeNull(); + }); + + it('throws an error if path is invalid and does not change the highlighted path', () => { + expect(() => instance.highlight('invalidPath[0]')).toThrow( + 'The node invalidPath[0] could not be found inside the document.', + ); + expect(instance.options.highlightPath).toEqual(highlightPathOnSetup); + expect(highlightLinesSpy).not.toHaveBeenCalled(); + expect(removeHighlightsSpy).not.toHaveBeenCalled(); + }); + }); + + describe('locate', () => { + const options = { + enableComments: true, + model: { + abc: ['def'], + '#|foo': 'foo comment', + foo: { + '#|abc': 'abc comment', + abc: [{ def: 'ghl', lorem: 'ipsum' }, '# array comment', null], + bar: 'baz', + }, + }, + }; + + const value = + /* 1 */ 'abc:\n' + + /* 2 */ ' - def\n' + + /* 3 */ '# foo comment\n' + + /* 4 */ 'foo:\n' + + /* 5 */ ' # abc comment\n' + + /* 6 */ ' abc:\n' + + /* 7 */ ' - def: ghl\n' + + /* 8 */ ' lorem: ipsum\n' + + /* 9 */ ' # array comment\n' + + /* 10 */ ' - null\n' + + /* 11 */ ' bar: baz\n'; + + it('asserts that the test setup is correct', () => { + const instance = getEditorInstanceWithExtension(options); + expect(instance.getValue()).toEqual(value); + }); + + it('returns the expected line numbers for a path to an object inside the yaml', () => { + const path = 'foo.abc'; + const expected = [6, 10]; + const instance = getEditorInstanceWithExtension(options); + expect(instance.locate(path)).toEqual(expected); + }); + + it('throws an error if a path cannot be found inside the yaml', () => { + const path = 'baz[8]'; + const instance = getEditorInstanceWithExtension(options); + expect(() => instance.locate(path)).toThrow(); + }); + + it('returns the expected line numbers for a path to an array entry inside the yaml', () => { + const path = 'foo.abc[0]'; + const expected = [7, 8]; + const instance = getEditorInstanceWithExtension(options); + expect(instance.locate(path)).toEqual(expected); + }); + + it('returns the expected line numbers for a path that includes a comment inside the yaml', () => { + const path = 'foo'; + const expected = [4, 11]; + const instance = getEditorInstanceWithExtension(options); + expect(instance.locate(path)).toEqual(expected); + }); + }); +}); diff --git a/spec/support/shared_examples/features/resolving_discussions_in_issues_shared_examples.rb b/spec/support/shared_examples/features/resolving_discussions_in_issues_shared_examples.rb index 6d44a6fde85..337b3f3cbd0 100644 --- a/spec/support/shared_examples/features/resolving_discussions_in_issues_shared_examples.rb +++ b/spec/support/shared_examples/features/resolving_discussions_in_issues_shared_examples.rb @@ -1,43 +1,29 @@ # frozen_string_literal: true RSpec.shared_examples 'creating an issue for a thread' do - it 'shows an issue with the title filled in' do + it 'shows an issue creation form' do + # Title field is filled in title_field = page.find_field('issue[title]') - expect(title_field.value).to include(merge_request.title) - end - it 'has a mention of the discussion in the description' do + # Has a hidden field for the merge request + merge_request_field = find('#merge_request_to_resolve_discussions_of', visible: false) + expect(merge_request_field.value).to eq(merge_request.iid.to_s) + + # Has a mention of the discussion in the description description_field = page.find_field('issue[description]') - expect(description_field.value).to include(discussion.first_note.note) end - it 'can create a new issue for the project' do + it 'creates a new issue for the project' do + # Actually creates an issue for the project expect { click_button 'Create issue' }.to change { project.issues.reload.size }.by(1) - end - - it 'resolves the discussion in the merge request' do - click_button 'Create issue' + # Resolves the discussion in the merge request discussion.first_note.reload - expect(discussion.resolved?).to eq(true) - end - it 'shows a flash messaage after resolving a discussion' do - click_button 'Create issue' - - page.within '.flash-notice' do - # Only check for the word 'Resolved' since the spec might have resolved - # multiple discussions - expect(page).to have_content('Resolved') - end - end - - it 'has a hidden field for the merge request' do - merge_request_field = find('#merge_request_to_resolve_discussions_of', visible: false) - - expect(merge_request_field.value).to eq(merge_request.iid.to_s) + # Issue title inludes MR title + expect(page).to have_content(%Q(Follow-up from "#{merge_request.title}")) end end diff --git a/yarn.lock b/yarn.lock index a286b762a50..e7c60b9ea03 100644 --- a/yarn.lock +++ b/yarn.lock @@ -12806,6 +12806,11 @@ yaml@^1.10.0: resolved "https://registry.yarnpkg.com/yaml/-/yaml-1.10.2.tgz#2301c5ffbf12b467de8da2333a459e29e7920e4b" integrity sha512-r3vXyErRCYJ7wg28yvBY5VSoAF8ZvlcW9/BwUzEtUsjvX/DKs24dIkuwjtuprwJJHsbyUbLApepYTR1BN4uHrg== +yaml@^2.0.0-8: + version "2.0.0-8" + resolved "https://registry.yarnpkg.com/yaml/-/yaml-2.0.0-8.tgz#226365f0d804ba7fb8cc2b527a00a7a4a3d8ea5f" + integrity sha512-QaYgJZMfWD6fKN/EYMk6w1oLWPCr1xj9QaPSZW5qkDb3y8nGCXhy2Ono+AF4F+CSL/vGcqswcAT0BaS//pgD2A== + yargs-parser@^13.1.2: version "13.1.2" resolved "https://registry.yarnpkg.com/yargs-parser/-/yargs-parser-13.1.2.tgz#130f09702ebaeef2650d54ce6e3e5706f7a4fb38"