diff options
62 files changed, 1907 insertions, 1249 deletions
diff --git a/.gitlab/ci/review-apps/main.gitlab-ci.yml b/.gitlab/ci/review-apps/main.gitlab-ci.yml index dee0240679d..16ef3c82cda 100644 --- a/.gitlab/ci/review-apps/main.gitlab-ci.yml +++ b/.gitlab/ci/review-apps/main.gitlab-ci.yml @@ -105,4 +105,4 @@ review-stop: stage: deploy needs: [] script: - - delete_k8s_release_namespace + - delete_namespace diff --git a/.gitlab/ci/review.gitlab-ci.yml b/.gitlab/ci/review.gitlab-ci.yml index b2dc835da37..b90af076a56 100644 --- a/.gitlab/ci/review.gitlab-ci.yml +++ b/.gitlab/ci/review.gitlab-ci.yml @@ -9,12 +9,13 @@ review-cleanup: action: stop before_script: - source scripts/utils.sh + - source scripts/review_apps/review-apps.sh - source scripts/review_apps/gcp_cleanup.sh - install_gitlab_gem - setup_gcp_dependencies script: - delete_release - - delete_k8s_release_namespace + - delete_namespace - ruby -rrubygems scripts/review_apps/automated_cleanup.rb - gcp_cleanup diff --git a/app/assets/javascripts/blob_edit/edit_blob.js b/app/assets/javascripts/blob_edit/edit_blob.js index 118cef59d5a..ee2f6cfb46c 100644 --- a/app/assets/javascripts/blob_edit/edit_blob.js +++ b/app/assets/javascripts/blob_edit/edit_blob.js @@ -1,4 +1,5 @@ import $ from 'jquery'; +import { SourceEditorExtension } from '~/editor/extensions/source_editor_extension_base'; import { FileTemplateExtension } from '~/editor/extensions/source_editor_file_template_ext'; import SourceEditor from '~/editor/source_editor'; import { getBlobLanguage } from '~/editor/utils'; @@ -26,23 +27,29 @@ export default class EditBlob { this.editor.focus(); } - fetchMarkdownExtension() { - import('~/editor/extensions/source_editor_markdown_ext') - .then(({ EditorMarkdownExtension: MarkdownExtension } = {}) => { - this.editor.use( - new MarkdownExtension({ - instance: this.editor, - previewMarkdownPath: this.options.previewMarkdownPath, - }), - ); - this.hasMarkdownExtension = true; - addEditorMarkdownListeners(this.editor); - }) - .catch((e) => - createFlash({ - message: `${BLOB_EDITOR_ERROR}: ${e}`, - }), - ); + async fetchMarkdownExtension() { + try { + const [ + { EditorMarkdownExtension: MarkdownExtension }, + { EditorMarkdownPreviewExtension: MarkdownLivePreview }, + ] = await Promise.all([ + import('~/editor/extensions/source_editor_markdown_ext'), + import('~/editor/extensions/source_editor_markdown_livepreview_ext'), + ]); + this.editor.use([ + { definition: MarkdownExtension }, + { + definition: MarkdownLivePreview, + setupOptions: { previewMarkdownPath: this.options.previewMarkdownPath }, + }, + ]); + } catch (e) { + createFlash({ + message: `${BLOB_EDITOR_ERROR}: ${e}`, + }); + } + this.hasMarkdownExtension = true; + addEditorMarkdownListeners(this.editor); } configureMonacoEditor() { @@ -60,7 +67,7 @@ export default class EditBlob { blobPath: fileNameEl.value, blobContent: editorEl.innerText, }); - this.editor.use(new FileTemplateExtension({ instance: this.editor })); + this.editor.use([{ definition: SourceEditorExtension }, { definition: FileTemplateExtension }]); fileNameEl.addEventListener('change', () => { this.editor.updateModelLanguage(fileNameEl.value); diff --git a/app/assets/javascripts/editor/constants.js b/app/assets/javascripts/editor/constants.js index e855e304d27..2ae9c377683 100644 --- a/app/assets/javascripts/editor/constants.js +++ b/app/assets/javascripts/editor/constants.js @@ -42,6 +42,10 @@ export const EDITOR_EXTENSION_STORE_IS_MISSING_ERROR = s__( // EXTENSIONS' CONSTANTS // +// Source Editor Base Extension +export const EXTENSION_BASE_LINE_LINK_ANCHOR_CLASS = 'link-anchor'; +export const EXTENSION_BASE_LINE_NUMBERS_CLASS = 'line-numbers'; + // For CI config schemas the filename must match // '*.gitlab-ci.yml' regardless of project configuration. // https://gitlab.com/gitlab-org/gitlab/-/issues/293641 diff --git a/app/assets/javascripts/editor/extensions/example_source_editor_extension.js b/app/assets/javascripts/editor/extensions/example_source_editor_extension.js index 33be6cf9e5d..52e2bb0b5ff 100644 --- a/app/assets/javascripts/editor/extensions/example_source_editor_extension.js +++ b/app/assets/javascripts/editor/extensions/example_source_editor_extension.js @@ -7,6 +7,16 @@ export class MyFancyExtension { /** + * A required getter returning the extension's name + * We have to provide it for every extension instead of relying on the built-in + * `name` prop because the prop does not survive the webpack's minification + * and the name mangling. + * @returns {string} + */ + static get extensionName() { + return 'MyFancyExtension'; + } + /** * THE LIFE-CYCLE CALLBACKS */ diff --git a/app/assets/javascripts/editor/extensions/source_editor_ci_schema_ext.js b/app/assets/javascripts/editor/extensions/source_editor_ci_schema_ext.js index 7069568275d..0290bb84b5f 100644 --- a/app/assets/javascripts/editor/extensions/source_editor_ci_schema_ext.js +++ b/app/assets/javascripts/editor/extensions/source_editor_ci_schema_ext.js @@ -1,32 +1,27 @@ import ciSchemaPath from '~/editor/schema/ci.json'; import { registerSchema } from '~/ide/utils'; -import { SourceEditorExtension } from './source_editor_extension_base'; -export class CiSchemaExtension extends SourceEditorExtension { - /** - * Registers a syntax schema to the editor based on project - * identifier and commit. - * - * The schema is added to the file that is currently edited - * in the editor. - * - * @param {Object} opts - * @param {String} opts.projectNamespace - * @param {String} opts.projectPath - * @param {String?} opts.ref - Current ref. Defaults to main - */ - registerCiSchema() { - // In order for workers loaded from `data://` as the - // ones loaded by monaco editor, we use absolute URLs - // to fetch schema files, hence the `gon.gitlab_url` - // reference. This prevents error: - // "Failed to execute 'fetch' on 'WorkerGlobalScope'" - const absoluteSchemaUrl = gon.gitlab_url + ciSchemaPath; - const modelFileName = this.getModel().uri.path.split('/').pop(); +export class CiSchemaExtension { + static get extensionName() { + return 'CiSchema'; + } + // eslint-disable-next-line class-methods-use-this + provides() { + return { + registerCiSchema: (instance) => { + // In order for workers loaded from `data://` as the + // ones loaded by monaco editor, we use absolute URLs + // to fetch schema files, hence the `gon.gitlab_url` + // reference. This prevents error: + // "Failed to execute 'fetch' on 'WorkerGlobalScope'" + const absoluteSchemaUrl = gon.gitlab_url + ciSchemaPath; + const modelFileName = instance.getModel().uri.path.split('/').pop(); - registerSchema({ - uri: absoluteSchemaUrl, - fileMatch: [modelFileName], - }); + registerSchema({ + uri: absoluteSchemaUrl, + fileMatch: [modelFileName], + }); + }, + }; } } 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 03c68fed3b1..3aa19df964c 100644 --- a/app/assets/javascripts/editor/extensions/source_editor_extension_base.js +++ b/app/assets/javascripts/editor/extensions/source_editor_extension_base.js @@ -1,13 +1,16 @@ import { Range } from 'monaco-editor'; -import { waitForCSSLoaded } from '~/helpers/startup_css_helper'; -import { ERROR_INSTANCE_REQUIRED_FOR_EXTENSION, EDITOR_TYPE_CODE } from '../constants'; +import { + EDITOR_TYPE_CODE, + EXTENSION_BASE_LINE_LINK_ANCHOR_CLASS, + EXTENSION_BASE_LINE_NUMBERS_CLASS, +} from '../constants'; const hashRegexp = new RegExp('#?L', 'g'); const createAnchor = (href) => { const fragment = new DocumentFragment(); const el = document.createElement('a'); - el.classList.add('link-anchor'); + el.classList.add(EXTENSION_BASE_LINE_LINK_ANCHOR_CLASS); el.href = href; fragment.appendChild(el); el.addEventListener('contextmenu', (e) => { @@ -17,38 +20,46 @@ const createAnchor = (href) => { }; export class SourceEditorExtension { - constructor({ instance, ...options } = {}) { - if (instance) { - Object.assign(instance, options); - SourceEditorExtension.highlightLines(instance); - if (instance.getEditorType && instance.getEditorType() === EDITOR_TYPE_CODE) { - SourceEditorExtension.setupLineLinking(instance); - } - SourceEditorExtension.deferRerender(instance); - } else if (Object.entries(options).length) { - throw new Error(ERROR_INSTANCE_REQUIRED_FOR_EXTENSION); + static get extensionName() { + return 'BaseExtension'; + } + + // eslint-disable-next-line class-methods-use-this + onUse(instance) { + SourceEditorExtension.highlightLines(instance); + if (instance.getEditorType && instance.getEditorType() === EDITOR_TYPE_CODE) { + SourceEditorExtension.setupLineLinking(instance); } } - static deferRerender(instance) { - waitForCSSLoaded(() => { - instance.layout(); - }); + static onMouseMoveHandler(e) { + const target = e.target.element; + if (target.classList.contains(EXTENSION_BASE_LINE_NUMBERS_CLASS)) { + const lineNum = e.target.position.lineNumber; + const hrefAttr = `#L${lineNum}`; + let lineLink = target.querySelector('a'); + if (!lineLink) { + lineLink = createAnchor(hrefAttr); + target.appendChild(lineLink); + } + } } - static removeHighlights(instance) { - Object.assign(instance, { - lineDecorations: instance.deltaDecorations(instance.lineDecorations || [], []), + static setupLineLinking(instance) { + instance.onMouseMove(SourceEditorExtension.onMouseMoveHandler); + instance.onMouseDown((e) => { + const isCorrectAnchor = e.target.element.classList.contains( + EXTENSION_BASE_LINE_LINK_ANCHOR_CLASS, + ); + if (!isCorrectAnchor) { + return; + } + if (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) @@ -74,29 +85,29 @@ export class SourceEditorExtension { } } - static onMouseMoveHandler(e) { - const target = e.target.element; - if (target.classList.contains('line-numbers')) { - const lineNum = e.target.position.lineNumber; - const hrefAttr = `#L${lineNum}`; - let el = target.querySelector('a'); - if (!el) { - el = createAnchor(hrefAttr); - target.appendChild(el); - } - } - } + // eslint-disable-next-line class-methods-use-this + provides() { + return { + /** + * Removes existing line decorations and updates the reference on the instance + * @param {module:source_editor_instance~EditorInstance} instance - The Source Editor instance + */ + removeHighlights: (instance) => { + Object.assign(instance, { + lineDecorations: instance.deltaDecorations(instance.lineDecorations || [], []), + }); + }, - static setupLineLinking(instance) { - instance.onMouseMove(SourceEditorExtension.onMouseMoveHandler); - instance.onMouseDown((e) => { - const isCorrectAnchor = e.target.element.classList.contains('link-anchor'); - if (!isCorrectAnchor) { - return; - } - if (instance.lineDecorations) { - instance.deltaDecorations(instance.lineDecorations, []); - } - }); + /** + * Returns a function that can only be invoked once between + * each browser screen repaint. + * @param {Array} bounds - The [start, end] array with start + * @param {module:source_editor_instance~EditorInstance} instance - The Source Editor instance + * and end coordinates for highlighting + */ + highlightLines(instance, bounds = null) { + SourceEditorExtension.highlightLines(instance, bounds); + }, + }; } } diff --git a/app/assets/javascripts/editor/extensions/source_editor_file_template_ext.js b/app/assets/javascripts/editor/extensions/source_editor_file_template_ext.js index 397e090ed30..ba4980896e5 100644 --- a/app/assets/javascripts/editor/extensions/source_editor_file_template_ext.js +++ b/app/assets/javascripts/editor/extensions/source_editor_file_template_ext.js @@ -1,8 +1,16 @@ import { Position } from 'monaco-editor'; -import { SourceEditorExtension } from './source_editor_extension_base'; -export class FileTemplateExtension extends SourceEditorExtension { - navigateFileStart() { - this.setPosition(new Position(1, 1)); +export class FileTemplateExtension { + static get extensionName() { + return 'FileTemplate'; + } + + // eslint-disable-next-line class-methods-use-this + provides() { + return { + navigateFileStart: (instance) => { + instance.setPosition(new Position(1, 1)); + }, + }; } } diff --git a/app/assets/javascripts/editor/extensions/source_editor_markdown_ext.js b/app/assets/javascripts/editor/extensions/source_editor_markdown_ext.js index bec7fe7e25f..a16fe93026e 100644 --- a/app/assets/javascripts/editor/extensions/source_editor_markdown_ext.js +++ b/app/assets/javascripts/editor/extensions/source_editor_markdown_ext.js @@ -1,97 +1,102 @@ -import { EditorMarkdownPreviewExtension } from '~/editor/extensions/source_editor_markdown_livepreview_ext'; - -export class EditorMarkdownExtension extends EditorMarkdownPreviewExtension { - getSelectedText(selection = this.getSelection()) { - const { startLineNumber, endLineNumber, startColumn, endColumn } = selection; - const valArray = this.getValue().split('\n'); - let text = ''; - if (startLineNumber === endLineNumber) { - text = valArray[startLineNumber - 1].slice(startColumn - 1, endColumn - 1); - } else { - const startLineText = valArray[startLineNumber - 1].slice(startColumn - 1); - const endLineText = valArray[endLineNumber - 1].slice(0, endColumn - 1); - - for (let i = startLineNumber, k = endLineNumber - 1; i < k; i += 1) { - text += `${valArray[i]}`; - if (i !== k - 1) text += `\n`; - } - text = text - ? [startLineText, text, endLineText].join('\n') - : [startLineText, endLineText].join('\n'); - } - return text; +export class EditorMarkdownExtension { + static get extensionName() { + return 'EditorMarkdown'; } - replaceSelectedText(text, select = undefined) { - const forceMoveMarkers = !select; - this.executeEdits('', [{ range: this.getSelection(), text, forceMoveMarkers }]); - } - - moveCursor(dx = 0, dy = 0) { - const pos = this.getPosition(); - pos.column += dx; - pos.lineNumber += dy; - this.setPosition(pos); - } + // eslint-disable-next-line class-methods-use-this + provides() { + return { + getSelectedText: (instance, selection = instance.getSelection()) => { + const { startLineNumber, endLineNumber, startColumn, endColumn } = selection; + const valArray = instance.getValue().split('\n'); + let text = ''; + if (startLineNumber === endLineNumber) { + text = valArray[startLineNumber - 1].slice(startColumn - 1, endColumn - 1); + } else { + const startLineText = valArray[startLineNumber - 1].slice(startColumn - 1); + const endLineText = valArray[endLineNumber - 1].slice(0, endColumn - 1); - /** - * Adjust existing selection to select text within the original selection. - * - If `selectedText` is not supplied, we fetch selected text with - * - * ALGORITHM: - * - * MULTI-LINE SELECTION - * 1. Find line that contains `toSelect` text. - * 2. Using the index of this line and the position of `toSelect` text in it, - * construct: - * * newStartLineNumber - * * newStartColumn - * - * SINGLE-LINE SELECTION - * 1. Use `startLineNumber` from the current selection as `newStartLineNumber` - * 2. Find the position of `toSelect` text in it to get `newStartColumn` - * - * 3. `newEndLineNumber` — Since this method is supposed to be used with - * markdown decorators that are pretty short, the `newEndLineNumber` is - * suggested to be assumed the same as the startLine. - * 4. `newEndColumn` — pretty obvious - * 5. Adjust the start and end positions of the current selection - * 6. Re-set selection on the instance - * - * @param {string} toSelect - New text to select within current selection. - * @param {string} selectedText - Currently selected text. It's just a - * shortcut: If it's not supplied, we fetch selected text from the instance - */ - selectWithinSelection(toSelect, selectedText) { - const currentSelection = this.getSelection(); - if (currentSelection.isEmpty() || !toSelect) { - return; - } - const text = selectedText || this.getSelectedText(currentSelection); - let lineShift; - let newStartLineNumber; - let newStartColumn; + for (let i = startLineNumber, k = endLineNumber - 1; i < k; i += 1) { + text += `${valArray[i]}`; + if (i !== k - 1) text += `\n`; + } + text = text + ? [startLineText, text, endLineText].join('\n') + : [startLineText, endLineText].join('\n'); + } + return text; + }, + replaceSelectedText: (instance, text, select) => { + const forceMoveMarkers = !select; + instance.executeEdits('', [{ range: instance.getSelection(), text, forceMoveMarkers }]); + }, + moveCursor: (instance, dx = 0, dy = 0) => { + const pos = instance.getPosition(); + pos.column += dx; + pos.lineNumber += dy; + instance.setPosition(pos); + }, + /** + * Adjust existing selection to select text within the original selection. + * - If `selectedText` is not supplied, we fetch selected text with + * + * ALGORITHM: + * + * MULTI-LINE SELECTION + * 1. Find line that contains `toSelect` text. + * 2. Using the index of this line and the position of `toSelect` text in it, + * construct: + * * newStartLineNumber + * * newStartColumn + * + * SINGLE-LINE SELECTION + * 1. Use `startLineNumber` from the current selection as `newStartLineNumber` + * 2. Find the position of `toSelect` text in it to get `newStartColumn` + * + * 3. `newEndLineNumber` — Since this method is supposed to be used with + * markdown decorators that are pretty short, the `newEndLineNumber` is + * suggested to be assumed the same as the startLine. + * 4. `newEndColumn` — pretty obvious + * 5. Adjust the start and end positions of the current selection + * 6. Re-set selection on the instance + * + * @param {module:source_editor_instance~EditorInstance} instance - The Source Editor instance. Is passed automatically. + * @param {string} toSelect - New text to select within current selection. + * @param {string} selectedText - Currently selected text. It's just a + * shortcut: If it's not supplied, we fetch selected text from the instance + */ + selectWithinSelection: (instance, toSelect, selectedText) => { + const currentSelection = instance.getSelection(); + if (currentSelection.isEmpty() || !toSelect) { + return; + } + const text = selectedText || instance.getSelectedText(currentSelection); + let lineShift; + let newStartLineNumber; + let newStartColumn; - const textLines = text.split('\n'); + const textLines = text.split('\n'); - if (textLines.length > 1) { - // Multi-line selection - lineShift = textLines.findIndex((line) => line.indexOf(toSelect) !== -1); - newStartLineNumber = currentSelection.startLineNumber + lineShift; - newStartColumn = textLines[lineShift].indexOf(toSelect) + 1; - } else { - // Single-line selection - newStartLineNumber = currentSelection.startLineNumber; - newStartColumn = currentSelection.startColumn + text.indexOf(toSelect); - } + if (textLines.length > 1) { + // Multi-line selection + lineShift = textLines.findIndex((line) => line.indexOf(toSelect) !== -1); + newStartLineNumber = currentSelection.startLineNumber + lineShift; + newStartColumn = textLines[lineShift].indexOf(toSelect) + 1; + } else { + // Single-line selection + newStartLineNumber = currentSelection.startLineNumber; + newStartColumn = currentSelection.startColumn + text.indexOf(toSelect); + } - const newEndLineNumber = newStartLineNumber; - const newEndColumn = newStartColumn + toSelect.length; + const newEndLineNumber = newStartLineNumber; + const newEndColumn = newStartColumn + toSelect.length; - const newSelection = currentSelection - .setStartPosition(newStartLineNumber, newStartColumn) - .setEndPosition(newEndLineNumber, newEndColumn); + const newSelection = currentSelection + .setStartPosition(newStartLineNumber, newStartColumn) + .setEndPosition(newEndLineNumber, newEndColumn); - this.setSelection(newSelection); + instance.setSelection(newSelection); + }, + }; } } diff --git a/app/assets/javascripts/editor/extensions/source_editor_markdown_livepreview_ext.js b/app/assets/javascripts/editor/extensions/source_editor_markdown_livepreview_ext.js index 526de7f8932..9d53268c340 100644 --- a/app/assets/javascripts/editor/extensions/source_editor_markdown_livepreview_ext.js +++ b/app/assets/javascripts/editor/extensions/source_editor_markdown_livepreview_ext.js @@ -12,9 +12,8 @@ import { EXTENSION_MARKDOWN_PREVIEW_PANEL_PARENT_CLASS, EXTENSION_MARKDOWN_PREVIEW_UPDATE_DELAY, } from '../constants'; -import { SourceEditorExtension } from './source_editor_extension_base'; -const getPreview = (text, previewMarkdownPath) => { +const fetchPreview = (text, previewMarkdownPath) => { return axios .post(previewMarkdownPath, { text, @@ -34,19 +33,20 @@ const setupDomElement = ({ injectToEl = null } = {}) => { return previewEl; }; -export class EditorMarkdownPreviewExtension extends SourceEditorExtension { - constructor({ instance, previewMarkdownPath, ...args } = {}) { - super({ instance, ...args }); - Object.assign(instance, { - previewMarkdownPath, - preview: { - el: undefined, - action: undefined, - shown: false, - modelChangeListener: undefined, - }, - }); - this.setupPreviewAction.call(instance); +export class EditorMarkdownPreviewExtension { + static get extensionName() { + return 'EditorMarkdownPreview'; + } + + onSetup(instance, setupOptions) { + this.preview = { + el: undefined, + action: undefined, + shown: false, + modelChangeListener: undefined, + path: setupOptions.previewMarkdownPath, + }; + this.setupPreviewAction(instance); instance.getModel().onDidChangeLanguage(({ newLanguage, oldLanguage } = {}) => { if (newLanguage === 'markdown' && oldLanguage !== newLanguage) { @@ -68,43 +68,31 @@ export class EditorMarkdownPreviewExtension extends SourceEditorExtension { }); } - static togglePreviewLayout() { - const { width, height } = this.getLayoutInfo(); + togglePreviewLayout(instance) { + const { width, height } = instance.getLayoutInfo(); const newWidth = this.preview.shown ? width / EXTENSION_MARKDOWN_PREVIEW_PANEL_WIDTH : width * EXTENSION_MARKDOWN_PREVIEW_PANEL_WIDTH; - this.layout({ width: newWidth, height }); + instance.layout({ width: newWidth, height }); } - static togglePreviewPanel() { - const parentEl = this.getDomNode().parentElement; + togglePreviewPanel(instance) { + const parentEl = instance.getDomNode().parentElement; const { el: previewEl } = this.preview; parentEl.classList.toggle(EXTENSION_MARKDOWN_PREVIEW_PANEL_PARENT_CLASS); if (previewEl.style.display === 'none') { // Show the preview panel - this.fetchPreview(); + this.fetchPreview(instance); } else { // Hide the preview panel previewEl.style.display = 'none'; } } - cleanup() { - if (this.preview.modelChangeListener) { - this.preview.modelChangeListener.dispose(); - } - this.preview.action.dispose(); - if (this.preview.shown) { - EditorMarkdownPreviewExtension.togglePreviewPanel.call(this); - EditorMarkdownPreviewExtension.togglePreviewLayout.call(this); - } - this.preview.shown = false; - } - - fetchPreview() { + fetchPreview(instance) { const { el: previewEl } = this.preview; - getPreview(this.getValue(), this.previewMarkdownPath) + fetchPreview(instance.getValue(), this.preview.path) .then((data) => { previewEl.innerHTML = sanitize(data); syntaxHighlight(previewEl.querySelectorAll('.js-syntax-highlight')); @@ -113,10 +101,10 @@ export class EditorMarkdownPreviewExtension extends SourceEditorExtension { .catch(() => createFlash(BLOB_PREVIEW_ERROR)); } - setupPreviewAction() { - if (this.getAction(EXTENSION_MARKDOWN_PREVIEW_ACTION_ID)) return; + setupPreviewAction(instance) { + if (instance.getAction(EXTENSION_MARKDOWN_PREVIEW_ACTION_ID)) return; - this.preview.action = this.addAction({ + this.preview.action = instance.addAction({ id: EXTENSION_MARKDOWN_PREVIEW_ACTION_ID, label: __('Preview Markdown'), keybindings: [ @@ -128,27 +116,52 @@ export class EditorMarkdownPreviewExtension extends SourceEditorExtension { // Method that will be executed when the action is triggered. // @param ed The editor instance is passed in as a convenience - run(instance) { - instance.togglePreview(); + run(inst) { + inst.togglePreview(); }, }); } - togglePreview() { - if (!this.preview?.el) { - this.preview.el = setupDomElement({ injectToEl: this.getDomNode().parentElement }); - } - EditorMarkdownPreviewExtension.togglePreviewLayout.call(this); - EditorMarkdownPreviewExtension.togglePreviewPanel.call(this); + provides() { + return { + markdownPreview: this.preview, - if (!this.preview?.shown) { - this.preview.modelChangeListener = this.onDidChangeModelContent( - debounce(this.fetchPreview.bind(this), EXTENSION_MARKDOWN_PREVIEW_UPDATE_DELAY), - ); - } else { - this.preview.modelChangeListener.dispose(); - } + cleanup: (instance) => { + if (this.preview.modelChangeListener) { + this.preview.modelChangeListener.dispose(); + } + this.preview.action.dispose(); + if (this.preview.shown) { + this.togglePreviewPanel(instance); + this.togglePreviewLayout(instance); + } + this.preview.shown = false; + }, + + fetchPreview: (instance) => this.fetchPreview(instance), - this.preview.shown = !this.preview?.shown; + setupPreviewAction: (instance) => this.setupPreviewAction(instance), + + togglePreview: (instance) => { + if (!this.preview?.el) { + this.preview.el = setupDomElement({ injectToEl: instance.getDomNode().parentElement }); + } + this.togglePreviewLayout(instance); + this.togglePreviewPanel(instance); + + if (!this.preview?.shown) { + this.preview.modelChangeListener = instance.onDidChangeModelContent( + debounce( + this.fetchPreview.bind(this, instance), + EXTENSION_MARKDOWN_PREVIEW_UPDATE_DELAY, + ), + ); + } else { + this.preview.modelChangeListener.dispose(); + } + + this.preview.shown = !this.preview?.shown; + }, + }; } } diff --git a/app/assets/javascripts/editor/extensions/source_editor_webide_ext.js b/app/assets/javascripts/editor/extensions/source_editor_webide_ext.js index 98e05489c1c..4e8c11bac54 100644 --- a/app/assets/javascripts/editor/extensions/source_editor_webide_ext.js +++ b/app/assets/javascripts/editor/extensions/source_editor_webide_ext.js @@ -1,7 +1,15 @@ +/** + * A WebIDE Extension options for Source Editor + * @typedef {Object} WebIDEExtensionOptions + * @property {Object} modelManager The root manager for WebIDE models + * @property {Object} store The state store for communication + * @property {Object} file + * @property {Object} options The Monaco editor options + */ + import { debounce } from 'lodash'; import { KeyCode, KeyMod, Range } from 'monaco-editor'; import { EDITOR_TYPE_DIFF } from '~/editor/constants'; -import { SourceEditorExtension } from '~/editor/extensions/source_editor_extension_base'; import Disposable from '~/ide/lib/common/disposable'; import { editorOptions } from '~/ide/lib/editor_options'; import keymap from '~/ide/lib/keymap.json'; @@ -11,154 +19,168 @@ const isDiffEditorType = (instance) => { }; export const UPDATE_DIMENSIONS_DELAY = 200; +const defaultOptions = { + modelManager: undefined, + store: undefined, + file: undefined, + options: {}, +}; -export class EditorWebIdeExtension extends SourceEditorExtension { - constructor({ instance, modelManager, ...options } = {}) { - super({ - instance, - ...options, - modelManager, - disposable: new Disposable(), - debouncedUpdate: debounce(() => { - instance.updateDimensions(); - }, UPDATE_DIMENSIONS_DELAY), - }); - - window.addEventListener('resize', instance.debouncedUpdate, false); - - instance.onDidDispose(() => { - window.removeEventListener('resize', instance.debouncedUpdate); - - // catch any potential errors with disposing the error - // this is mainly for tests caused by elements not existing - try { - instance.disposable.dispose(); - } catch (e) { - if (process.env.NODE_ENV !== 'test') { - // eslint-disable-next-line no-console - console.error(e); - } - } - }); +const addActions = (instance, store) => { + const getKeyCode = (key) => { + const monacoKeyMod = key.indexOf('KEY_') === 0; - EditorWebIdeExtension.addActions(instance); - } + return monacoKeyMod ? KeyCode[key] : KeyMod[key]; + }; - static addActions(instance) { - const { store } = instance; - const getKeyCode = (key) => { - const monacoKeyMod = key.indexOf('KEY_') === 0; + keymap.forEach((command) => { + const { bindings, id, label, action } = command; - return monacoKeyMod ? KeyCode[key] : KeyMod[key]; - }; + const keybindings = bindings.map((binding) => { + const keys = binding.split('+'); - keymap.forEach((command) => { - const { bindings, id, label, action } = command; - - const keybindings = bindings.map((binding) => { - const keys = binding.split('+'); - - // eslint-disable-next-line no-bitwise - return keys.length > 1 ? getKeyCode(keys[0]) | getKeyCode(keys[1]) : getKeyCode(keys[0]); - }); - - instance.addAction({ - id, - label, - keybindings, - run() { - store.dispatch(action.name, action.params); - return null; - }, - }); + // eslint-disable-next-line no-bitwise + return keys.length > 1 ? getKeyCode(keys[0]) | getKeyCode(keys[1]) : getKeyCode(keys[0]); }); - } - - createModel(file, head = null) { - return this.modelManager.addModel(file, head); - } - - attachModel(model) { - if (isDiffEditorType(this)) { - this.setModel({ - original: model.getOriginalModel(), - modified: model.getModel(), - }); - return; - } - - this.setModel(model.getModel()); + instance.addAction({ + id, + label, + keybindings, + run() { + store.dispatch(action.name, action.params); + return null; + }, + }); + }); +}; - this.updateOptions( - editorOptions.reduce((acc, obj) => { - Object.keys(obj).forEach((key) => { - Object.assign(acc, { - [key]: obj[key](model), - }); - }); - return acc; - }, {}), - ); - } +const renderSideBySide = (domElement) => { + return domElement.offsetWidth >= 700; +}; - attachMergeRequestModel(model) { - this.setModel({ - original: model.getBaseModel(), - modified: model.getModel(), +const updateInstanceDimensions = (instance) => { + instance.layout(); + if (isDiffEditorType(instance)) { + instance.updateOptions({ + renderSideBySide: renderSideBySide(instance.getDomNode()), }); } +}; - updateDimensions() { - this.layout(); - this.updateDiffView(); +export class EditorWebIdeExtension { + static get extensionName() { + return 'EditorWebIde'; } - setPos({ lineNumber, column }) { - this.revealPositionInCenter({ - lineNumber, - column, - }); - this.setPosition({ - lineNumber, - column, - }); + /** + * Set up the WebIDE extension for Source Editor + * @param {module:source_editor_instance~EditorInstance} instance - The Source Editor instance + * @param {WebIDEExtensionOptions} setupOptions + */ + onSetup(instance, setupOptions = defaultOptions) { + this.modelManager = setupOptions.modelManager; + this.store = setupOptions.store; + this.file = setupOptions.file; + this.options = setupOptions.options; + + this.disposable = new Disposable(); + this.debouncedUpdate = debounce(() => { + updateInstanceDimensions(instance); + }, UPDATE_DIMENSIONS_DELAY); + + addActions(instance, setupOptions.store); } - onPositionChange(cb) { - if (!this.onDidChangeCursorPosition) { - return; - } + onUse(instance) { + window.addEventListener('resize', this.debouncedUpdate, false); - this.disposable.add(this.onDidChangeCursorPosition((e) => cb(this, e))); + instance.onDidDispose(() => { + this.onUnuse(); + }); } - updateDiffView() { - if (!isDiffEditorType(this)) { - return; + onUnuse() { + window.removeEventListener('resize', this.debouncedUpdate); + + // catch any potential errors with disposing the error + // this is mainly for tests caused by elements not existing + try { + this.disposable.dispose(); + } catch (e) { + if (process.env.NODE_ENV !== 'test') { + // eslint-disable-next-line no-console + console.error(e); + } } - - this.updateOptions({ - renderSideBySide: EditorWebIdeExtension.renderSideBySide(this.getDomNode()), - }); } - replaceSelectedText(text) { - let selection = this.getSelection(); - const range = new Range( - selection.startLineNumber, - selection.startColumn, - selection.endLineNumber, - selection.endColumn, - ); + provides() { + return { + createModel: (instance, file, head = null) => { + return this.modelManager.addModel(file, head); + }, + attachModel: (instance, model) => { + if (isDiffEditorType(instance)) { + instance.setModel({ + original: model.getOriginalModel(), + modified: model.getModel(), + }); - this.executeEdits('', [{ range, text }]); + return; + } - selection = this.getSelection(); - this.setPosition({ lineNumber: selection.endLineNumber, column: selection.endColumn }); - } + instance.setModel(model.getModel()); + + instance.updateOptions( + editorOptions.reduce((acc, obj) => { + Object.keys(obj).forEach((key) => { + Object.assign(acc, { + [key]: obj[key](model), + }); + }); + return acc; + }, {}), + ); + }, + attachMergeRequestModel: (instance, model) => { + instance.setModel({ + original: model.getBaseModel(), + modified: model.getModel(), + }); + }, + updateDimensions: (instance) => updateInstanceDimensions(instance), + setPos: (instance, { lineNumber, column }) => { + instance.revealPositionInCenter({ + lineNumber, + column, + }); + instance.setPosition({ + lineNumber, + column, + }); + }, + onPositionChange: (instance, cb) => { + if (typeof instance.onDidChangeCursorPosition !== 'function') { + return; + } - static renderSideBySide(domElement) { - return domElement.offsetWidth >= 700; + this.disposable.add(instance.onDidChangeCursorPosition((e) => cb(instance, e))); + }, + replaceSelectedText: (instance, text) => { + let selection = instance.getSelection(); + const range = new Range( + selection.startLineNumber, + selection.startColumn, + selection.endLineNumber, + selection.endColumn, + ); + + instance.executeEdits('', [{ range, text }]); + + selection = instance.getSelection(); + instance.setPosition({ lineNumber: selection.endLineNumber, column: selection.endColumn }); + }, + }; } } diff --git a/app/assets/javascripts/editor/extensions/source_editor_yaml_ext.js b/app/assets/javascripts/editor/extensions/source_editor_yaml_ext.js index 212e09c8724..05ce617ca7c 100644 --- a/app/assets/javascripts/editor/extensions/source_editor_yaml_ext.js +++ b/app/assets/javascripts/editor/extensions/source_editor_yaml_ext.js @@ -1,50 +1,46 @@ +/** + * A Yaml Editor Extension options for Source Editor + * @typedef {Object} YamlEditorExtensionOptions + * @property { boolean } enableComments Convert model nodes with the comment + * pattern to comments? + * @property { string } highlightPath Add a line highlight to the + * node specified by this e.g. `"foo.bar[0]"` + * @property { * } model Any JS Object that will be stringified and used as the + * editor's value. Equivalent to using `setDataModel()` + * @property options SourceEditorExtension Options + */ + 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 { +export class YamlEditorExtension { + static get extensionName() { + return 'YamlEditor'; + } + /** * 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 + * @param {module:source_editor_instance~EditorInstance} instance - The Source Editor instance + * @param {YamlEditorExtensionOptions} setupOptions */ - constructor({ - instance, - enableComments = false, - highlightPath = null, - model = null, - ...options - } = {}) { - super({ - instance, - options: { - ...options, - enableComments, - highlightPath, - }, - }); + onSetup(instance, setupOptions = {}) { + const { enableComments = false, highlightPath = null, model = null } = setupOptions; + this.enableComments = enableComments; + this.highlightPath = highlightPath; + this.model = model; if (model) { - YamlEditorExtension.initFromModel(instance, model); + this.initFromModel(instance, model); } instance.onDidChangeModelContent(() => instance.onUpdate()); } - /** - * @private - */ - static initFromModel(instance, model) { + initFromModel(instance, model) { const doc = new Document(model); - if (instance.options.enableComments) { + if (this.enableComments) { YamlEditorExtension.transformComments(doc); } instance.setValue(doc.toString()); @@ -160,110 +156,13 @@ export class YamlEditorExtension extends SourceEditorExtension { 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 <TextModel> 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; + static getDoc(instance) { + return parseDocument(instance.getValue()); } - /** - * 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) { + static locate(instance, path) { if (!path) throw Error(`No path provided.`); - const blob = this.getValue(); + const blob = instance.getValue(); const doc = parseDocument(blob); const pathArray = toPath(path); @@ -290,4 +189,120 @@ export class YamlEditorExtension extends SourceEditorExtension { const endLine = (endSlice.match(/\n/g) || []).length; return [startLine, endLine]; } + + setDoc(instance, doc) { + if (this.enableComments) { + YamlEditorExtension.transformComments(doc); + } + + if (!instance.getValue()) { + instance.setValue(doc.toString()); + } else { + instance.updateValue(doc.toString()); + } + } + + highlight(instance, path) { + // IMPORTANT + // removeHighlight and highlightLines both come from + // SourceEditorExtension. So it has to be installed prior to this extension + if (this.highlightPath === path) return; + if (!path) { + instance.removeHighlights(); + } else { + const res = YamlEditorExtension.locate(instance, path); + instance.highlightLines(res); + } + this.highlightPath = path || null; + } + + provides() { + return { + /** + * Get the editor's value parsed as a `Document` as defined by the `yaml` + * package + * @param {module:source_editor_instance~EditorInstance} instance - The Source Editor instance + * @returns {Document} + */ + getDoc: (instance) => YamlEditorExtension.getDoc(instance), + + /** + * Accepts a `Document` as defined by the `yaml` package and + * sets the Editor's value to a stringified version of it. + * @param {module:source_editor_instance~EditorInstance} instance - The Source Editor instance + * @param { Document } doc + */ + setDoc: (instance, doc) => this.setDoc(instance, doc), + + /** + * Returns the parsed value of the Editor's content as JS. + * @returns {*} + */ + getDataModel: (instance) => YamlEditorExtension.getDoc(instance).toJS(), + + /** + * Accepts any JS Object and sets the Editor's value to a stringified version + * of that value. + * + * @param {module:source_editor_instance~EditorInstance} instance - The Source Editor instance + * @param value + */ + setDataModel: (instance, value) => this.setDoc(instance, new Document(value)), + + /** + * Method to be executed when the Editor's <TextModel> was updated + */ + onUpdate: (instance) => { + if (this.highlightPath) { + this.highlight(instance, this.highlightPath); + } + }, + + /** + * Set the editors content to the input without recreating the content model. + * + * @param {module:source_editor_instance~EditorInstance} instance - The Source Editor instance + * @param blob + */ + updateValue: (instance, blob) => { + // Using applyEdits() instead of setValue() ensures that tokens such as + // highlighted lines aren't deleted/recreated which causes a flicker. + const model = instance.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 {module:source_editor_instance~EditorInstance} instance - The Source Editor instance + * @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: (instance, path) => this.highlight(instance, path), + + /** + * Return the line numbers of a certain node identified by `path` within + * the yaml. + * + * @param {module:source_editor_instance~EditorInstance} instance - The Source Editor instance + * @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: (instance, path) => YamlEditorExtension.locate(instance, path), + + initFromModel: (instance, model) => this.initFromModel(instance, model), + }; + } } diff --git a/app/assets/javascripts/editor/source_editor.js b/app/assets/javascripts/editor/source_editor.js index 26fbd1f4d8a..57e2b0da565 100644 --- a/app/assets/javascripts/editor/source_editor.js +++ b/app/assets/javascripts/editor/source_editor.js @@ -1,4 +1,5 @@ import { editor as monacoEditor, Uri } from 'monaco-editor'; +import { waitForCSSLoaded } from '~/helpers/startup_css_helper'; import { defaultEditorOptions } from '~/ide/lib/editor_options'; import languages from '~/ide/lib/languages'; import { registerLanguages } from '~/ide/utils'; @@ -11,10 +12,39 @@ import { EDITOR_TYPE_DIFF, } from './constants'; import { clearDomElement, setupEditorTheme, getBlobLanguage } from './utils'; +import EditorInstance from './source_editor_instance'; + +const instanceRemoveFromRegistry = (editor, instance) => { + const index = editor.instances.findIndex((inst) => inst === instance); + editor.instances.splice(index, 1); +}; + +const instanceDisposeModels = (editor, instance, model) => { + const instanceModel = instance.getModel() || model; + if (!instanceModel) { + return; + } + if (instance.getEditorType() === EDITOR_TYPE_DIFF) { + const { original, modified } = instanceModel; + if (original) { + original.dispose(); + } + if (modified) { + modified.dispose(); + } + } else { + instanceModel.dispose(); + } +}; export default class SourceEditor { + /** + * Constructs a global editor. + * @param {Object} options - Monaco config options used to create the editor + */ constructor(options = {}) { this.instances = []; + this.extensionsStore = new Map(); this.options = { extraEditorClassName: 'gl-source-editor', ...defaultEditorOptions, @@ -26,19 +56,6 @@ export default class SourceEditor { registerLanguages(...languages); } - static mixIntoInstance(source, inst) { - if (!inst) { - return; - } - const isClassInstance = source.constructor.prototype !== Object.prototype; - const sanitizedSource = isClassInstance ? source.constructor.prototype : source; - Object.getOwnPropertyNames(sanitizedSource).forEach((prop) => { - if (prop !== 'constructor') { - Object.assign(inst, { [prop]: source[prop] }); - } - }); - } - static prepareInstance(el) { if (!el) { throw new Error(SOURCE_EDITOR_INSTANCE_ERROR_NO_EL); @@ -78,71 +95,17 @@ export default class SourceEditor { return diffModel; } - static convertMonacoToELInstance = (inst) => { - const sourceEditorInstanceAPI = { - updateModelLanguage: (path) => { - return SourceEditor.instanceUpdateLanguage(inst, path); - }, - use: (exts = []) => { - return SourceEditor.instanceApplyExtension(inst, exts); - }, - }; - const handler = { - get(target, prop, receiver) { - if (Reflect.has(sourceEditorInstanceAPI, prop)) { - return sourceEditorInstanceAPI[prop]; - } - return Reflect.get(target, prop, receiver); - }, - }; - return new Proxy(inst, handler); - }; - - static instanceUpdateLanguage(inst, path) { - const lang = getBlobLanguage(path); - const model = inst.getModel(); - return monacoEditor.setModelLanguage(model, lang); - } - - static instanceApplyExtension(inst, exts = []) { - const extensions = [].concat(exts); - extensions.forEach((extension) => { - SourceEditor.mixIntoInstance(extension, inst); - }); - return inst; - } - - static instanceRemoveFromRegistry(editor, instance) { - const index = editor.instances.findIndex((inst) => inst === instance); - editor.instances.splice(index, 1); - } - - static instanceDisposeModels(editor, instance, model) { - const instanceModel = instance.getModel() || model; - if (!instanceModel) { - return; - } - if (instance.getEditorType() === EDITOR_TYPE_DIFF) { - const { original, modified } = instanceModel; - if (original) { - original.dispose(); - } - if (modified) { - modified.dispose(); - } - } else { - instanceModel.dispose(); - } - } - /** - * Creates a monaco instance with the given options. - * - * @param {Object} options Options used to initialize monaco. - * @param {Element} options.el The element which will be used to create the monacoEditor. + * Creates a Source Editor Instance with the given options. + * @param {Object} options Options used to initialize the instance. + * @param {Element} options.el The element to attach the instance for. * @param {string} options.blobPath The path used as the URI of the model. Monaco uses the extension of this path to determine the language. * @param {string} options.blobContent The content to initialize the monacoEditor. + * @param {string} options.blobOriginalContent The original blob's content. Is used when creating a Diff Instance. * @param {string} options.blobGlobalId This is used to help globally identify monaco instances that are created with the same blobPath. + * @param {Boolean} options.isDiff Flag to enable creation of a Diff Instance? + * @param {...*} options.instanceOptions Configuration options used to instantiate an instance. + * @returns {EditorInstance} */ createInstance({ el = undefined, @@ -156,13 +119,18 @@ export default class SourceEditor { SourceEditor.prepareInstance(el); const createEditorFn = isDiff ? 'createDiffEditor' : 'create'; - const instance = SourceEditor.convertMonacoToELInstance( + const instance = new EditorInstance( monacoEditor[createEditorFn].call(this, el, { ...this.options, ...instanceOptions, }), + this.extensionsStore, ); + waitForCSSLoaded(() => { + instance.layout(); + }); + let model; if (instanceOptions.model !== null) { model = SourceEditor.createEditorModel({ @@ -176,8 +144,8 @@ export default class SourceEditor { } instance.onDidDispose(() => { - SourceEditor.instanceRemoveFromRegistry(this, instance); - SourceEditor.instanceDisposeModels(this, instance, model); + instanceRemoveFromRegistry(this, instance); + instanceDisposeModels(this, instance, model); }); this.instances.push(instance); @@ -185,6 +153,11 @@ export default class SourceEditor { return instance; } + /** + * Create a Diff Instance + * @param {Object} args Options to be passed further down to createInstance() with the same signature + * @returns {EditorInstance} + */ createDiffInstance(args) { return this.createInstance({ ...args, @@ -192,6 +165,10 @@ export default class SourceEditor { }); } + /** + * Dispose global editor + * Automatically disposes all the instances registered for this editor + */ dispose() { this.instances.forEach((instance) => instance.dispose()); } diff --git a/app/assets/javascripts/editor/source_editor_extension.js b/app/assets/javascripts/editor/source_editor_extension.js index f6bc62a1c09..6d47e1e2248 100644 --- a/app/assets/javascripts/editor/source_editor_extension.js +++ b/app/assets/javascripts/editor/source_editor_extension.js @@ -5,10 +5,10 @@ export default class EditorExtension { if (typeof definition !== 'function') { throw new Error(EDITOR_EXTENSION_DEFINITION_ERROR); } - this.name = definition.name; // both class- and fn-based extensions have a name this.setupOptions = setupOptions; // eslint-disable-next-line new-cap this.obj = new definition(); + this.extensionName = definition.extensionName || this.obj.extensionName; // both class- and fn-based extensions have a name } get api() { diff --git a/app/assets/javascripts/editor/source_editor_instance.js b/app/assets/javascripts/editor/source_editor_instance.js index fcffdc587be..8372a59964b 100644 --- a/app/assets/javascripts/editor/source_editor_instance.js +++ b/app/assets/javascripts/editor/source_editor_instance.js @@ -13,7 +13,7 @@ * A Source Editor Extension * @typedef {Object} SourceEditorExtension * @property {Object} obj - * @property {string} name + * @property {string} extensionName * @property {Object} api */ @@ -43,12 +43,12 @@ const utils = { } }, - getStoredExtension: (extensionsStore, name) => { + getStoredExtension: (extensionsStore, extensionName) => { if (!extensionsStore) { logError(EDITOR_EXTENSION_STORE_IS_MISSING_ERROR); return undefined; } - return extensionsStore.get(name); + return extensionsStore.get(extensionName); }, }; @@ -73,30 +73,18 @@ export default class EditorInstance { if (methodExtension) { const extension = extensionsStore.get(methodExtension); - return (...args) => extension.api[prop].call(seInstance, receiver, ...args); + if (typeof extension.api[prop] === 'function') { + return extension.api[prop].bind(extension.obj, receiver); + } + + return extension.api[prop]; } 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); + this.dispatchExtAction = EditorInstance.useUnuse.bind(instProxy, extensionsStore); return instProxy; } @@ -141,7 +129,7 @@ export default class EditorInstance { } // Existing Extension Path - const existingExt = utils.getStoredExtension(extensionsStore, definition.name); + const existingExt = utils.getStoredExtension(extensionsStore, definition.extensionName); if (existingExt) { if (isEqual(extension.setupOptions, existingExt.setupOptions)) { return existingExt; @@ -168,14 +156,14 @@ export default class EditorInstance { * @param {Map} extensionsStore - The global registry for the extension instances */ registerExtension(extension, extensionsStore) { - const { name } = extension; + const { extensionName } = extension; const hasExtensionRegistered = - extensionsStore.has(name) && - isEqual(extension.setupOptions, extensionsStore.get(name).setupOptions); + extensionsStore.has(extensionName) && + isEqual(extension.setupOptions, extensionsStore.get(extensionName).setupOptions); if (hasExtensionRegistered) { return; } - extensionsStore.set(name, extension); + extensionsStore.set(extensionName, extension); const { obj: extensionObj } = extension; if (extensionObj.onUse) { extensionObj.onUse(this); @@ -187,7 +175,7 @@ export default class EditorInstance { * @param {SourceEditorExtension} extension - Instance of Source Editor extension */ registerExtensionMethods(extension) { - const { api, name } = extension; + const { api, extensionName } = extension; if (!api) { return; @@ -197,7 +185,7 @@ export default class EditorInstance { if (this[prop]) { logError(sprintf(EDITOR_EXTENSION_NAMING_CONFLICT_ERROR, { prop })); } else { - this.methods[prop] = name; + this.methods[prop] = extensionName; } }, this); } @@ -215,10 +203,10 @@ export default class EditorInstance { if (!extension) { throw new Error(EDITOR_EXTENSION_NOT_SPECIFIED_FOR_UNUSE_ERROR); } - const { name } = extension; - const existingExt = utils.getStoredExtension(extensionsStore, name); + const { extensionName } = extension; + const existingExt = utils.getStoredExtension(extensionsStore, extensionName); if (!existingExt) { - throw new Error(sprintf(EDITOR_EXTENSION_NOT_REGISTERED_ERROR, { name })); + throw new Error(sprintf(EDITOR_EXTENSION_NOT_REGISTERED_ERROR, { extensionName })); } const { obj: extensionObj } = existingExt; if (extensionObj.onBeforeUnuse) { @@ -235,12 +223,12 @@ export default class EditorInstance { * @param {SourceEditorExtension} extension - Instance of Source Editor extension to un-use */ unregisterExtensionMethods(extension) { - const { api, name } = extension; + const { api, extensionName } = extension; if (!api) { return; } Object.keys(api).forEach((method) => { - utils.removeExtFromMethod(method, name, this.methods); + utils.removeExtFromMethod(method, extensionName, this.methods); }); } @@ -260,6 +248,24 @@ export default class EditorInstance { } /** + * Main entry point to apply an extension to the instance + * @param {SourceEditorExtensionDefinition[]|SourceEditorExtensionDefinition} extDefs - The extension(s) to use + * @returns {EditorExtension|*} + */ + use(extDefs) { + return this.dispatchExtAction(this.useExtension, extDefs); + } + + /** + * Main entry point to remove an extension to the instance + * @param {SourceEditorExtension[]|SourceEditorExtension} exts - + * @returns {*} + */ + unuse(exts) { + return this.dispatchExtAction(this.unuseExtension, exts); + } + + /** * Get the methods returned by extensions. * @returns {Array} */ diff --git a/app/assets/javascripts/ide/components/repo_editor.vue b/app/assets/javascripts/ide/components/repo_editor.vue index 2bf99550bf2..05493db1dff 100644 --- a/app/assets/javascripts/ide/components/repo_editor.vue +++ b/app/assets/javascripts/ide/components/repo_editor.vue @@ -7,6 +7,7 @@ import { EDITOR_CODE_INSTANCE_FN, EDITOR_DIFF_INSTANCE_FN, } from '~/editor/constants'; +import { SourceEditorExtension } from '~/editor/extensions/source_editor_extension_base'; import { EditorWebIdeExtension } from '~/editor/extensions/source_editor_webide_ext'; import SourceEditor from '~/editor/source_editor'; import createFlash from '~/flash'; @@ -302,30 +303,32 @@ export default { ...instanceOptions, ...this.editorOptions, }); - - this.editor.use( - new EditorWebIdeExtension({ - instance: this.editor, - modelManager: this.modelManager, - store: this.$store, - file: this.file, - options: this.editorOptions, - }), - ); + this.editor.use([ + { + definition: SourceEditorExtension, + }, + { + definition: EditorWebIdeExtension, + setupOptions: { + modelManager: this.modelManager, + store: this.$store, + file: this.file, + options: this.editorOptions, + }, + }, + ]); if ( this.fileType === MARKDOWN_FILE_TYPE && this.editor?.getEditorType() === EDITOR_TYPE_CODE && this.previewMarkdownPath ) { - import('~/editor/extensions/source_editor_markdown_ext') - .then(({ EditorMarkdownExtension: MarkdownExtension } = {}) => { - this.editor.use( - new MarkdownExtension({ - instance: this.editor, - previewMarkdownPath: this.previewMarkdownPath, - }), - ); + import('~/editor/extensions/source_editor_markdown_livepreview_ext') + .then(({ EditorMarkdownPreviewExtension: MarkdownLivePreview }) => { + this.editor.use({ + definition: MarkdownLivePreview, + setupOptions: { previewMarkdownPath: this.previewMarkdownPath }, + }); }) .catch((e) => createFlash({ diff --git a/app/assets/javascripts/import_entities/import_groups/components/import_table.vue b/app/assets/javascripts/import_entities/import_groups/components/import_table.vue index f2837a7bb44..9125b8a2f44 100644 --- a/app/assets/javascripts/import_entities/import_groups/components/import_table.vue +++ b/app/assets/javascripts/import_entities/import_groups/components/import_table.vue @@ -2,8 +2,6 @@ import { GlButton, GlEmptyState, - GlDropdown, - GlDropdownItem, GlIcon, GlLink, GlLoadingIcon, @@ -15,7 +13,7 @@ import { import { debounce } from 'lodash'; import createFlash from '~/flash'; import { s__, __, n__ } from '~/locale'; -import PaginationLinks from '~/vue_shared/components/pagination_links.vue'; +import PaginationBar from '~/vue_shared/components/pagination_bar/pagination_bar.vue'; import { getGroupPathAvailability } from '~/rest_api'; import axios from '~/lib/utils/axios_utils'; import { DEFAULT_DEBOUNCE_AND_THROTTLE_MS } from '~/lib/utils/constants'; @@ -44,8 +42,6 @@ export default { components: { GlButton, GlEmptyState, - GlDropdown, - GlDropdownItem, GlIcon, GlLink, GlLoadingIcon, @@ -57,7 +53,7 @@ export default { ImportTargetCell, ImportStatusCell, ImportActionsCell, - PaginationLinks, + PaginationBar, }, props: { @@ -600,49 +596,13 @@ export default { /> </template> </gl-table> - <div v-if="hasGroups" class="gl-display-flex gl-mt-3 gl-align-items-center"> - <pagination-links - :change="setPage" - :page-info="bulkImportSourceGroups.pageInfo" - class="gl-m-0" - /> - <gl-dropdown category="tertiary" :aria-label="__('Page size')" class="gl-ml-auto"> - <template #button-content> - <span class="font-weight-bold"> - <gl-sprintf :message="__('%{count} items per page')"> - <template #count> - {{ perPage }} - </template> - </gl-sprintf> - </span> - <gl-icon class="gl-button-icon dropdown-chevron" name="chevron-down" /> - </template> - <gl-dropdown-item - v-for="size in $options.PAGE_SIZES" - :key="size" - @click="setPageSize(size)" - > - <gl-sprintf :message="__('%{count} items per page')"> - <template #count> - {{ size }} - </template> - </gl-sprintf> - </gl-dropdown-item> - </gl-dropdown> - <div class="gl-ml-2"> - <gl-sprintf :message="s__('BulkImport|Showing %{start}-%{end} of %{total}')"> - <template #start> - {{ paginationInfo.start }} - </template> - <template #end> - {{ paginationInfo.end }} - </template> - <template #total> - {{ humanizedTotal }} - </template> - </gl-sprintf> - </div> - </div> + <pagination-bar + v-if="hasGroups" + :page-info="bulkImportSourceGroups.pageInfo" + class="gl-mt-3" + @set-page="setPage" + @set-page-size="setPageSize" + /> </template> </template> </div> diff --git a/app/assets/javascripts/pages/import/bulk_imports/history/components/bulk_imports_history_app.vue b/app/assets/javascripts/pages/import/bulk_imports/history/components/bulk_imports_history_app.vue index ec3cf4a8a92..0ec382983a5 100644 --- a/app/assets/javascripts/pages/import/bulk_imports/history/components/bulk_imports_history_app.vue +++ b/app/assets/javascripts/pages/import/bulk_imports/history/components/bulk_imports_history_app.vue @@ -7,7 +7,7 @@ import { parseIntPagination, normalizeHeaders } from '~/lib/utils/common_utils'; import { joinPaths } from '~/lib/utils/url_utility'; import { getBulkImportsHistory } from '~/rest_api'; import ImportStatus from '~/import_entities/components/import_status.vue'; -import PaginationBar from '~/import_entities/components/pagination_bar.vue'; +import PaginationBar from '~/vue_shared/components/pagination_bar/pagination_bar.vue'; import TimeAgo from '~/vue_shared/components/time_ago_tooltip.vue'; import { DEFAULT_ERROR } from '../utils/error_messages'; @@ -166,7 +166,6 @@ export default { </gl-table> <pagination-bar :page-info="pageInfo" - :items-count="historyItems.length" class="gl-m-0 gl-mt-3" @set-page="paginationConfig.page = $event" @set-page-size="paginationConfig.perPage = $event" diff --git a/app/assets/javascripts/pipeline_editor/components/editor/text_editor.vue b/app/assets/javascripts/pipeline_editor/components/editor/text_editor.vue index 7b8e97b573e..92fa411d5af 100644 --- a/app/assets/javascripts/pipeline_editor/components/editor/text_editor.vue +++ b/app/assets/javascripts/pipeline_editor/components/editor/text_editor.vue @@ -19,7 +19,7 @@ export default { if (this.glFeatures.schemaLinting) { const editorInstance = this.$refs.editor.getEditor(); - editorInstance.use(new CiSchemaExtension({ instance: editorInstance })); + editorInstance.use({ definition: CiSchemaExtension }); editorInstance.registerCiSchema(); } }, diff --git a/app/assets/javascripts/vue_shared/components/pagination_bar/pagination_bar.stories.js b/app/assets/javascripts/vue_shared/components/pagination_bar/pagination_bar.stories.js new file mode 100644 index 00000000000..e31446f4bb8 --- /dev/null +++ b/app/assets/javascripts/vue_shared/components/pagination_bar/pagination_bar.stories.js @@ -0,0 +1,40 @@ +/* eslint-disable @gitlab/require-i18n-strings */ +import PaginationBar from './pagination_bar.vue'; + +export default { + component: PaginationBar, + title: 'vue_shared/components/pagination_bar/pagination_bar', +}; + +const Template = (args, { argTypes }) => ({ + components: { PaginationBar }, + props: Object.keys(argTypes), + template: `<pagination-bar v-bind="$props" v-on="{ 'set-page-size': setPageSize, 'set-page': setPage }" />`, +}); + +export const Default = Template.bind({}); + +Default.args = { + pageInfo: { + perPage: 20, + page: 2, + total: 83, + totalPages: 5, + }, + pageSizes: [20, 50, 100], +}; + +Default.argTypes = { + pageInfo: { + description: 'Page info object', + control: { type: 'object' }, + }, + pageSizes: { + description: 'Array of possible page sizes', + control: { type: 'array' }, + }, + + // events + setPageSize: { action: 'set-page-size' }, + setPage: { action: 'set-page' }, +}; diff --git a/app/assets/javascripts/import_entities/components/pagination_bar.vue b/app/assets/javascripts/vue_shared/components/pagination_bar/pagination_bar.vue index 33bd3e08bb1..b4d565991f5 100644 --- a/app/assets/javascripts/import_entities/components/pagination_bar.vue +++ b/app/assets/javascripts/vue_shared/components/pagination_bar/pagination_bar.vue @@ -23,10 +23,6 @@ export default { type: Array, default: () => DEFAULT_PAGE_SIZES, }, - itemsCount: { - required: true, - type: Number, - }, }, computed: { @@ -35,9 +31,10 @@ export default { }, paginationInfo() { - const { page, perPage } = this.pageInfo; + const { page, perPage, totalPages, total } = this.pageInfo; + const itemsCount = page === totalPages ? total - (page - 1) * perPage : perPage; const start = (page - 1) * perPage + 1; - const end = start + this.itemsCount - 1; + const end = start + itemsCount - 1; return { start, end }; }, @@ -45,8 +42,24 @@ export default { methods: { setPage(page) { + // eslint-disable-next-line spaced-comment + /** + * Emitted when selected page is updated + * + * @event set-page + **/ this.$emit('set-page', page); }, + + setPageSize(pageSize) { + // eslint-disable-next-line spaced-comment + /** + * Emitted when page size is updated + * + * @event set-page-size + **/ + this.$emit('set-page-size', pageSize); + }, }, }; </script> @@ -54,7 +67,7 @@ export default { <template> <div class="gl-display-flex gl-align-items-center"> <pagination-links :change="setPage" :page-info="pageInfo" class="gl-m-0" /> - <gl-dropdown category="tertiary" class="gl-ml-auto"> + <gl-dropdown category="tertiary" class="gl-ml-auto" data-testid="page-size"> <template #button-content> <span class="gl-font-weight-bold"> <gl-sprintf :message="__('%{count} items per page')"> @@ -65,7 +78,7 @@ export default { </span> <gl-icon class="gl-button-icon dropdown-chevron" name="chevron-down" /> </template> - <gl-dropdown-item v-for="size in pageSizes" :key="size" @click="$emit('set-page-size', size)"> + <gl-dropdown-item v-for="size in pageSizes" :key="size" @click="setPageSize(size)"> <gl-sprintf :message="__('%{count} items per page')"> <template #count> {{ size }} diff --git a/app/helpers/avatars_helper.rb b/app/helpers/avatars_helper.rb index dd852a68682..af9b8a01248 100644 --- a/app/helpers/avatars_helper.rb +++ b/app/helpers/avatars_helper.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true module AvatarsHelper + DEFAULT_AVATAR_PATH = 'no_avatar.png' + def project_icon(project, options = {}) source_icon(project, options) end @@ -34,11 +36,11 @@ module AvatarsHelper end def avatar_icon_for_user(user = nil, size = nil, scale = 2, only_path: true) - if user - user.avatar_url(size: size, only_path: only_path) || default_avatar - else - gravatar_icon(nil, size, scale) - end + return gravatar_icon(nil, size, scale) unless user + return default_avatar if blocked_or_unconfirmed?(user) && !can_admin?(current_user) + + user_avatar = user.avatar_url(size: size, only_path: only_path) + user_avatar || default_avatar end def gravatar_icon(user_email = '', size = nil, scale = 2) @@ -47,7 +49,7 @@ module AvatarsHelper end def default_avatar - ActionController::Base.helpers.image_path('no_avatar.png') + ActionController::Base.helpers.image_path(DEFAULT_AVATAR_PATH) end def author_avatar(commit_or_event, options = {}) @@ -157,4 +159,14 @@ module AvatarsHelper source.name[0, 1].upcase end end + + def blocked_or_unconfirmed?(user) + user.blocked? || !user.confirmed? + end + + def can_admin?(user) + return false unless user + + user.can_admin_all_resources? + end end diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index d32298819b9..3ada60be485 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -61,6 +61,11 @@ module Issuable # We check first if we're loaded to not load unnecessarily. loaded? && to_a.all? { |note| note.association(:award_emoji).loaded? } end + + def projects_loaded? + # We check first if we're loaded to not load unnecessarily. + loaded? && to_a.all? { |note| note.association(:project).loaded? } + end end has_many :note_authors, -> { distinct }, through: :notes, source: :author @@ -524,6 +529,7 @@ module Issuable includes = [] includes << :author unless notes.authors_loaded? includes << :award_emoji unless notes.award_emojis_loaded? + includes << :project unless notes.projects_loaded? if includes.any? notes.includes(includes) diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb index 3b8ba4728e0..73abe006889 100644 --- a/app/policies/group_policy.rb +++ b/app/policies/group_policy.rb @@ -157,6 +157,7 @@ class GroupPolicy < Namespaces::GroupProjectNamespaceSharedPolicy enable :destroy_package enable :create_projects enable :admin_pipeline + enable :admin_group_runners enable :admin_build enable :read_cluster enable :add_cluster diff --git a/app/presenters/projects/security/configuration_presenter.rb b/app/presenters/projects/security/configuration_presenter.rb new file mode 100644 index 00000000000..89fca1a451a --- /dev/null +++ b/app/presenters/projects/security/configuration_presenter.rb @@ -0,0 +1,98 @@ +# frozen_string_literal: true + +module Projects + module Security + class ConfigurationPresenter < Gitlab::View::Presenter::Delegated + include AutoDevopsHelper + include ::Security::LatestPipelineInformation + + presents ::Project, as: :project + + def to_h + { + auto_devops_enabled: auto_devops_source?, + auto_devops_help_page_path: help_page_path('topics/autodevops/index'), + auto_devops_path: auto_devops_settings_path(project), + can_enable_auto_devops: can_enable_auto_devops?, + features: features, + help_page_path: help_page_path('user/application_security/index'), + latest_pipeline_path: latest_pipeline_path, + # TODO: gitlab_ci_present will incorrectly report `false` if the CI/CD configuration file name + # has been customized and a file with the given custom name exists in the repo. This edge case + # will be addressed in https://gitlab.com/gitlab-org/gitlab/-/issues/342465 + gitlab_ci_present: project.repository.gitlab_ci_yml.present?, + gitlab_ci_history_path: gitlab_ci_history_path, + auto_fix_enabled: autofix_enabled, + can_toggle_auto_fix_settings: can_toggle_autofix, + auto_fix_user_path: auto_fix_user_path + } + end + + def to_html_data_attribute + data = to_h + data[:features] = data[:features].to_json + data[:auto_fix_enabled] = data[:auto_fix_enabled].to_json + + data + end + + private + + def autofix_enabled; end + + def auto_fix_user_path; end + + def can_enable_auto_devops? + feature_available?(:builds, current_user) && + can?(current_user, :admin_project, self) && + !archived? + end + + def can_toggle_autofix; end + + def gitlab_ci_history_path + return '' if project.empty_repo? + + gitlab_ci = ::Gitlab::FileDetector::PATTERNS[:gitlab_ci] + ::Gitlab::Routing.url_helpers.project_blame_path(project, File.join(project.default_branch_or_main, gitlab_ci)) + end + + def features + scans = scan_types.map do |scan_type| + scan(scan_type, configured: scanner_enabled?(scan_type)) + end + + # These scans are "fake" (non job) entries. Add them manually. + scans << scan(:corpus_management, configured: true) + scans << scan(:dast_profiles, configured: true) + end + + def latest_pipeline_path + return help_page_path('ci/pipelines') unless latest_default_branch_pipeline + + project_pipeline_path(self, latest_default_branch_pipeline) + end + + def scan(type, configured: false) + scan = ::Gitlab::Security::ScanConfiguration.new(project: project, type: type, configured: configured) + + { + type: scan.type, + configured: scan.configured?, + configuration_path: scan.configuration_path, + available: scan.available? + } + end + + def scan_types + ::Security::SecurityJobsFinder.allowed_job_types + ::Security::LicenseComplianceJobsFinder.allowed_job_types + end + + def project_settings + project.security_setting + end + end + end +end + +Projects::Security::ConfigurationPresenter.prepend_mod_with('Projects::Security::ConfigurationPresenter') diff --git a/app/views/projects/protected_tags/shared/_protected_tag.html.haml b/app/views/projects/protected_tags/shared/_protected_tag.html.haml index 972c96dc882..70357f39e44 100644 --- a/app/views/projects/protected_tags/shared/_protected_tag.html.haml +++ b/app/views/projects/protected_tags/shared/_protected_tag.html.haml @@ -3,7 +3,7 @@ %span.ref-name= protected_tag.name - if @project.root_ref?(protected_tag.name) - %span.badge.badge-info.gl-ml-2 default + = gl_badge_tag s_('ProtectedTags|default'), variant: :info, class: 'gl-ml-2' %td - if protected_tag.wildcard? - matching_tags = protected_tag.matching(repository.tags) diff --git a/app/views/projects/runners/_group_runners.html.haml b/app/views/projects/runners/_group_runners.html.haml index 5d737bb3901..183e747afdd 100644 --- a/app/views/projects/runners/_group_runners.html.haml +++ b/app/views/projects/runners/_group_runners.html.haml @@ -27,7 +27,7 @@ - elsif @group_runners.empty? = _('This group does not have any group runners yet.') - - if can?(current_user, :admin_pipeline, @project.group) + - if can?(current_user, :admin_group_runners, @project.group) - group_link = link_to _("group's CI/CD settings."), group_settings_ci_cd_path(@project.group) = _('Group maintainers can register group runners in the %{link}').html_safe % { link: group_link } - else diff --git a/danger/product_intelligence/Dangerfile b/danger/product_intelligence/Dangerfile index fb621cdbadb..01a2f9b6feb 100644 --- a/danger/product_intelligence/Dangerfile +++ b/danger/product_intelligence/Dangerfile @@ -15,7 +15,7 @@ MSG product_intelligence_paths_to_review = helper.changes_by_category[:product_intelligence] labels_to_add = product_intelligence.missing_labels -return if product_intelligence_paths_to_review.empty? +return if product_intelligence_paths_to_review.empty? || product_intelligence.skip_review? warn format(CHANGED_FILES_MESSAGE, changed_files: helper.markdown_list(product_intelligence_paths_to_review)) unless product_intelligence.has_approved_label? diff --git a/danger/roulette/Dangerfile b/danger/roulette/Dangerfile index cdea635e808..8d1b14d3b9a 100644 --- a/danger/roulette/Dangerfile +++ b/danger/roulette/Dangerfile @@ -111,7 +111,7 @@ categories << :ux if (["UX", "Community contribution"] - helper.mr_labels).empty categories << :product_intelligence if helper.mr_labels.include?("product intelligence::review pending") # Skip Product intelligence reviews for growth experiment MRs -categories.delete(:product_intelligence) unless helper.mr_labels.include?("growth experiment") +categories.delete(:product_intelligence) if helper.mr_labels.include?("growth experiment") if changes.any? random_roulette_spins = roulette.spin(nil, categories, timezone_experiment: false) diff --git a/doc/development/snowplow/implementation.md b/doc/development/snowplow/implementation.md index a7992c44da0..6da4896c7e7 100644 --- a/doc/development/snowplow/implementation.md +++ b/doc/development/snowplow/implementation.md @@ -421,6 +421,13 @@ Snowplow Inspector Chrome Extension is a browser extension for testing frontend Docker-based solution for testing backend and frontend in a local development environment. Snowplow Micro records the same events as the full Snowplow pipeline. To query events, use the Snowplow Micro API. +It can be set up automatically using [GitLab Development Kit (GDK)](https://gitlab.com/gitlab-org/gitlab-development-kit). +See the [how-to docs](https://gitlab.com/gitlab-org/gitlab-development-kit/-/blob/main/doc/howto/snowplow_micro.md) for more details. + +Optionally, you can set it up manually, using the following instructions. + +#### Set up Snowplow Micro manually + To install and run Snowplow Micro, complete these steps to modify the [GitLab Development Kit (GDK)](https://gitlab.com/gitlab-org/gitlab-development-kit): diff --git a/doc/user/group/epics/index.md b/doc/user/group/epics/index.md index c7d793e0970..3889398e2f8 100644 --- a/doc/user/group/epics/index.md +++ b/doc/user/group/epics/index.md @@ -1,5 +1,4 @@ --- -type: reference, howto stage: Plan group: Product Planning 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 @@ -7,8 +6,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w # Epics **(PREMIUM)** -> - Introduced in GitLab 10.2. -> - Single-level epics were [moved](https://gitlab.com/gitlab-org/gitlab/-/issues/37081) from GitLab Ultimate to GitLab Premium in 12.8. +> Single-level epics were [moved](https://gitlab.com/gitlab-org/gitlab/-/issues/37081) from GitLab Ultimate to GitLab Premium in 12.8. INFO: Check out [multi-level child epics](manage_epics.md#multi-level-child-epics) with a @@ -45,8 +43,6 @@ Also, read more about possible [planning hierarchies](../planning_hierarchy/inde ## Roadmap in epics **(ULTIMATE)** -> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/7327) in GitLab 11.10. - If your epic contains one or more [child epics](manage_epics.md#multi-level-child-epics) that have a start or due date, a visual [roadmap](../roadmap/index.md) of the child epics is listed under the parent epic. diff --git a/doc/user/infrastructure/iac/index.md b/doc/user/infrastructure/iac/index.md index 89df9c1d18f..15a680e2193 100644 --- a/doc/user/infrastructure/iac/index.md +++ b/doc/user/infrastructure/iac/index.md @@ -96,10 +96,17 @@ owned by GitLab, where everyone can contribute. The [documentation of the provider](https://registry.terraform.io/providers/gitlabhq/gitlab/latest/docs) is available as part of the official Terraform provider documentations. -## Create a new cluster through IaC +## Create a new cluster through IaC (DEPRECATED) Learn how to [create a new cluster on Google Kubernetes Engine (GKE)](../clusters/connect/new_gke_cluster.md). +NOTE: +The linked tutorial connects the cluster to GitLab through cluster certificates, +and this method was [deprecated](https://gitlab.com/groups/gitlab-org/configure/-/epics/8) +in GitLab 14.5. You can still create a cluster through IaC and then connect it to GitLab +through the [Agent](../../clusters/agent/index.md), the default and fully supported +method to connect clusters to GitLab. + ## Troubleshooting ### `gitlab_group_share_group` resources not detected when subgroup state is refreshed diff --git a/doc/user/project/clusters/add_eks_clusters.md b/doc/user/project/clusters/add_eks_clusters.md index 3b74238ca0f..a5473629d4f 100644 --- a/doc/user/project/clusters/add_eks_clusters.md +++ b/doc/user/project/clusters/add_eks_clusters.md @@ -10,7 +10,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w > - [Deprecated](https://gitlab.com/groups/gitlab-org/configure/-/epics/8) in GitLab 14.5. WARNING: -This feature was deprecated in GitLab 14.5. Use [Infrastructure as Code](../../infrastructure/iac/index.md#create-a-new-cluster-through-iac) +This feature was deprecated in GitLab 14.5. Use [Infrastructure as Code](../../infrastructure/iac/index.md#create-a-new-cluster-through-iac-deprecated) to create new clusters. Through GitLab, you can create new clusters and add existing clusters hosted on Amazon Elastic @@ -23,7 +23,7 @@ use the [GitLab Agent](../../clusters/agent/index.md). ## Create a new EKS cluster -To create a new cluster from GitLab, use [Infrastructure as Code](../../infrastructure/iac/index.md#create-a-new-cluster-through-iac). +To create a new cluster from GitLab, use [Infrastructure as Code](../../infrastructure/iac/index.md#create-a-new-cluster-through-iac-deprecated). ### How to create a new cluster on EKS through cluster certificates (DEPRECATED) diff --git a/doc/user/project/clusters/add_remove_clusters.md b/doc/user/project/clusters/add_remove_clusters.md index 6c0f319de67..a0fca517f2e 100644 --- a/doc/user/project/clusters/add_remove_clusters.md +++ b/doc/user/project/clusters/add_remove_clusters.md @@ -10,7 +10,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w WARNING: This feature was [deprecated](https://gitlab.com/gitlab-org/gitlab/-/issues/327908) in GitLab 14.0. -To create a new cluster use [Infrastructure as Code](../../infrastructure/iac/index.md#create-a-new-cluster-through-iac). +To create a new cluster use [Infrastructure as Code](../../infrastructure/iac/index.md#create-a-new-cluster-through-iac-deprecated). NOTE: Every new Google Cloud Platform (GCP) account receives @@ -29,7 +29,7 @@ in a few clicks. > [Deprecated](https://gitlab.com/gitlab-org/gitlab/-/issues/327908) in GitLab 14.0. -As of GitLab 14.0, use [Infrastructure as Code](../../infrastructure/iac/index.md#create-a-new-cluster-through-iac) +As of GitLab 14.0, use [Infrastructure as Code](../../infrastructure/iac/index.md#create-a-new-cluster-through-iac-deprecated) to **safely create new clusters from GitLab**. Creating clusters from GitLab using cluster certificates is still available on the diff --git a/lib/gitlab/security/scan_configuration.rb b/lib/gitlab/security/scan_configuration.rb new file mode 100644 index 00000000000..eaccbb3be7e --- /dev/null +++ b/lib/gitlab/security/scan_configuration.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +module Gitlab + module Security + class ScanConfiguration + include ::Gitlab::Utils::StrongMemoize + include Gitlab::Routing.url_helpers + + attr_reader :type + + def initialize(project:, type:, configured: false) + @project = project + @type = type + @configured = configured + end + + def available? + # SAST and Secret Detection are always available, but this isn't + # reflected by our license model yet. + # TODO: https://gitlab.com/gitlab-org/gitlab/-/issues/333113 + %i[sast secret_detection].include?(type) + end + + def configured? + configured + end + + def configuration_path + configurable_scans[type] + end + + private + + attr_reader :project, :configured + + def configurable_scans + strong_memoize(:configurable_scans) do + { + sast: project_security_configuration_sast_path(project) + } + end + end + end + end +end + +Gitlab::Security::ScanConfiguration.prepend_mod_with('Gitlab::Security::ScanConfiguration') diff --git a/locale/gitlab.pot b/locale/gitlab.pot index f9dab340bb1..180c3e85499 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -25258,9 +25258,6 @@ msgstr "" msgid "Page settings" msgstr "" -msgid "Page size" -msgstr "" - msgid "PagerDutySettings|Active" msgstr "" @@ -28558,6 +28555,9 @@ msgstr "" msgid "ProtectedEnvironment|Your environment has been unprotected" msgstr "" +msgid "ProtectedTags|default" +msgstr "" + msgid "ProtectedTag|By default, protected branches restrict who can modify the tag." msgstr "" diff --git a/rubocop/cop/qa/testcase_link_format.rb b/rubocop/cop/qa/testcase_link_format.rb new file mode 100644 index 00000000000..683098e6eec --- /dev/null +++ b/rubocop/cop/qa/testcase_link_format.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +require_relative '../../qa_helpers' + +module RuboCop + module Cop + module QA + # This cop checks for correct format of testcase links across e2e specs + # + # @example + # + # # bad + # it 'some test', testcase: 'https://gitlab.com/gitlab-org/quality/testcases/-/issues/557' + # it 'another test, testcase: 'https://gitlab.com/gitlab-org/quality/testcases/-/quality/test_cases/2455' + # + # # good + # it 'some test', testcase: 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/348312' + # it 'another test, testcase: 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/348236' + class TestcaseLinkFormat < RuboCop::Cop::Cop + include QAHelpers + + TESTCASE_FORMAT = %r{https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/\d+}.freeze + MESSAGE = "Testcase link format incorrect. Please link a test case from the GitLab project. See: https://docs.gitlab.com/ee/development/testing_guide/end_to_end/best_practices.html#link-a-test-to-its-test-case." + + def_node_matcher :testcase_link_format, <<~PATTERN + (block + (send nil? ... + ... + (hash + (pair + (sym :testcase) + (str $_))...)...)...) + PATTERN + + def on_block(node) + return unless in_qa_file?(node) + + testcase_link_format(node) do |link| + add_offense(node, message: MESSAGE % link) unless TESTCASE_FORMAT =~ link + end + end + end + end + end +end diff --git a/scripts/review_apps/review-apps.sh b/scripts/review_apps/review-apps.sh index 81ff1db9478..695de95b8fc 100755 --- a/scripts/review_apps/review-apps.sh +++ b/scripts/review_apps/review-apps.sh @@ -1,5 +1,23 @@ [[ "$TRACE" ]] && set -x +function namespace_exists() { + local namespace="${1}" + local namespace_exists + + echoinfo "Checking if ${namespace} exists..." true + + kubectl describe namespace "${namespace}" >/dev/null 2>&1 + namespace_exists=$? + + if [ $namespace_exists -eq 0 ]; then + echoinfo "Namespace ${namespace} found." + else + echoerr "Namespace ${namespace} NOT found." + fi + + return $namespace_exists +} + function deploy_exists() { local namespace="${1}" local release="${2}" @@ -73,17 +91,20 @@ function delete_failed_release() { # Cleanup and previous installs, as FAILED and PENDING_UPGRADE will cause errors with `upgrade` if previous_deploy_failed "${namespace}" "${release}" ; then echoinfo "Review App deployment in bad state, cleaning up namespace ${release}" - delete_k8s_release_namespace + delete_namespace else echoinfo "Review App deployment in good state" fi fi } -function delete_k8s_release_namespace() { +function delete_namespace() { local namespace="${CI_ENVIRONMENT_SLUG}" - kubectl delete namespace "${namespace}" --wait + if namespace_exists "${namespace}"; then + echoinfo "Deleting namespace ${namespace}..." true + kubectl delete namespace "${namespace}" --wait + fi } function get_pod() { @@ -170,9 +191,10 @@ function check_kube_domain() { function ensure_namespace() { local namespace="${1}" - echoinfo "Ensuring the ${namespace} namespace exists..." true - - kubectl describe namespace "${namespace}" || kubectl create namespace "${namespace}" + if ! namespace_exists "${namespace}"; then + echoinfo "Creating namespace ${namespace}..." true + kubectl create namespace "${namespace}" + fi } function label_namespace() { diff --git a/spec/features/users/show_spec.rb b/spec/features/users/show_spec.rb index 8edbf639c81..4d984e15b41 100644 --- a/spec/features/users/show_spec.rb +++ b/spec/features/users/show_spec.rb @@ -243,6 +243,10 @@ RSpec.describe 'User page' do expect(page).to have_content("@#{user.username}") end + it 'shows default avatar' do + expect(page).to have_css('//img[data-src^="/assets/no_avatar"]') + end + it_behaves_like 'default brand title page meta description' end @@ -286,6 +290,10 @@ RSpec.describe 'User page' do expect(page).to have_content("This user has a private profile") end + it 'shows default avatar' do + expect(page).to have_css('//img[data-src^="/assets/no_avatar"]') + end + it_behaves_like 'default brand title page meta description' end diff --git a/spec/frontend/blob_edit/edit_blob_spec.js b/spec/frontend/blob_edit/edit_blob_spec.js index ebef0656750..9c974e79e6e 100644 --- a/spec/frontend/blob_edit/edit_blob_spec.js +++ b/spec/frontend/blob_edit/edit_blob_spec.js @@ -1,14 +1,29 @@ import waitForPromises from 'helpers/wait_for_promises'; import EditBlob from '~/blob_edit/edit_blob'; +import { SourceEditorExtension } from '~/editor/extensions/source_editor_extension_base'; import { FileTemplateExtension } from '~/editor/extensions/source_editor_file_template_ext'; import { EditorMarkdownExtension } from '~/editor/extensions/source_editor_markdown_ext'; +import { EditorMarkdownPreviewExtension } from '~/editor/extensions/source_editor_markdown_livepreview_ext'; import SourceEditor from '~/editor/source_editor'; jest.mock('~/editor/source_editor'); -jest.mock('~/editor/extensions/source_editor_markdown_ext'); +jest.mock('~/editor/extensions/source_editor_extension_base'); jest.mock('~/editor/extensions/source_editor_file_template_ext'); +jest.mock('~/editor/extensions/source_editor_markdown_ext'); +jest.mock('~/editor/extensions/source_editor_markdown_livepreview_ext'); const PREVIEW_MARKDOWN_PATH = '/foo/bar/preview_markdown'; +const defaultExtensions = [ + { definition: SourceEditorExtension }, + { definition: FileTemplateExtension }, +]; +const markdownExtensions = [ + { definition: EditorMarkdownExtension }, + { + definition: EditorMarkdownPreviewExtension, + setupOptions: { previewMarkdownPath: PREVIEW_MARKDOWN_PATH }, + }, +]; describe('Blob Editing', () => { const useMock = jest.fn(); @@ -29,7 +44,9 @@ describe('Blob Editing', () => { jest.spyOn(SourceEditor.prototype, 'createInstance').mockReturnValue(mockInstance); }); afterEach(() => { + SourceEditorExtension.mockClear(); EditorMarkdownExtension.mockClear(); + EditorMarkdownPreviewExtension.mockClear(); FileTemplateExtension.mockClear(); }); @@ -45,26 +62,22 @@ describe('Blob Editing', () => { await waitForPromises(); }; - it('loads FileTemplateExtension by default', async () => { + it('loads SourceEditorExtension and FileTemplateExtension by default', async () => { await initEditor(); - expect(useMock).toHaveBeenCalledWith(expect.any(FileTemplateExtension)); - expect(FileTemplateExtension).toHaveBeenCalledTimes(1); + expect(useMock).toHaveBeenCalledWith(defaultExtensions); }); describe('Markdown', () => { - it('does not load MarkdownExtension by default', async () => { + it('does not load MarkdownExtensions by default', async () => { await initEditor(); expect(EditorMarkdownExtension).not.toHaveBeenCalled(); + expect(EditorMarkdownPreviewExtension).not.toHaveBeenCalled(); }); it('loads MarkdownExtension only for the markdown files', async () => { await initEditor(true); - expect(useMock).toHaveBeenCalledWith(expect.any(EditorMarkdownExtension)); - expect(EditorMarkdownExtension).toHaveBeenCalledTimes(1); - expect(EditorMarkdownExtension).toHaveBeenCalledWith({ - instance: mockInstance, - previewMarkdownPath: PREVIEW_MARKDOWN_PATH, - }); + expect(useMock).toHaveBeenCalledTimes(2); + expect(useMock.mock.calls[1]).toEqual([markdownExtensions]); }); }); diff --git a/spec/frontend/editor/helpers.js b/spec/frontend/editor/helpers.js index e4942c36f6c..252d783ad6d 100644 --- a/spec/frontend/editor/helpers.js +++ b/spec/frontend/editor/helpers.js @@ -1,4 +1,22 @@ +/* eslint-disable max-classes-per-file */ + +// Helpers +export const spyOnApi = (extension, spiesObj = {}) => { + const origApi = extension.api; + if (extension?.obj) { + jest.spyOn(extension.obj, 'provides').mockReturnValue({ + ...origApi, + ...spiesObj, + }); + } +}; + +// Dummy Extensions export class SEClassExtension { + static get extensionName() { + return 'SEClassExtension'; + } + // eslint-disable-next-line class-methods-use-this provides() { return { @@ -10,6 +28,7 @@ export class SEClassExtension { export function SEFnExtension() { return { + extensionName: 'SEFnExtension', fnExtMethod: () => 'fn own method', provides: () => { return { @@ -21,6 +40,7 @@ export function SEFnExtension() { export const SEConstExt = () => { return { + extensionName: 'SEConstExt', provides: () => { return { constExtMethod: () => 'const own method', @@ -29,36 +49,39 @@ export const SEConstExt = () => { }; }; -export function SEWithSetupExt() { - return { - onSetup: (instance, setupOptions = {}) => { - if (setupOptions && !Array.isArray(setupOptions)) { - Object.entries(setupOptions).forEach(([key, value]) => { - Object.assign(instance, { - [key]: value, - }); +export class SEWithSetupExt { + static get extensionName() { + return 'SEWithSetupExt'; + } + // eslint-disable-next-line class-methods-use-this + onSetup(instance, setupOptions = {}) { + if (setupOptions && !Array.isArray(setupOptions)) { + Object.entries(setupOptions).forEach(([key, value]) => { + Object.assign(instance, { + [key]: value, }); - } - }, - provides: () => { - return { - returnInstanceAndProps: (instance, stringProp, objProp = {}) => { - return [stringProp, objProp, instance]; - }, - returnInstance: (instance) => { - return instance; - }, - giveMeContext: () => { - return this; - }, - }; - }, - }; + }); + } + } + provides() { + return { + returnInstanceAndProps: (instance, stringProp, objProp = {}) => { + return [stringProp, objProp, instance]; + }, + returnInstance: (instance) => { + return instance; + }, + giveMeContext: () => { + return this; + }, + }; + } } export const conflictingExtensions = { WithInstanceExt: () => { return { + extensionName: 'WithInstanceExt', provides: () => { return { use: () => 'A conflict with instance', @@ -69,6 +92,7 @@ export const conflictingExtensions = { }, WithAnotherExt: () => { return { + extensionName: 'WithAnotherExt', provides: () => { return { shared: () => 'A conflict with extension', diff --git a/spec/frontend/editor/source_editor_ci_schema_ext_spec.js b/spec/frontend/editor/source_editor_ci_schema_ext_spec.js index 8a0d1ecf1af..5eaac9e9ef9 100644 --- a/spec/frontend/editor/source_editor_ci_schema_ext_spec.js +++ b/spec/frontend/editor/source_editor_ci_schema_ext_spec.js @@ -23,7 +23,7 @@ describe('~/editor/editor_ci_config_ext', () => { blobPath, blobContent: '', }); - instance.use(new CiSchemaExtension()); + instance.use({ definition: CiSchemaExtension }); }; beforeAll(() => { diff --git a/spec/frontend/editor/source_editor_extension_base_spec.js b/spec/frontend/editor/source_editor_extension_base_spec.js index a0fb1178b3b..6606557fd1f 100644 --- a/spec/frontend/editor/source_editor_extension_base_spec.js +++ b/spec/frontend/editor/source_editor_extension_base_spec.js @@ -2,40 +2,25 @@ import { Range } from 'monaco-editor'; import { useFakeRequestAnimationFrame } from 'helpers/fake_request_animation_frame'; import setWindowLocation from 'helpers/set_window_location_helper'; import { - ERROR_INSTANCE_REQUIRED_FOR_EXTENSION, EDITOR_TYPE_CODE, EDITOR_TYPE_DIFF, + EXTENSION_BASE_LINE_LINK_ANCHOR_CLASS, + EXTENSION_BASE_LINE_NUMBERS_CLASS, } from '~/editor/constants'; import { SourceEditorExtension } from '~/editor/extensions/source_editor_extension_base'; - -jest.mock('~/helpers/startup_css_helper', () => { - return { - waitForCSSLoaded: jest.fn().mockImplementation((cb) => { - // We have to artificially put the callback's execution - // to the end of the current call stack to be able to - // test that the callback is called after waitForCSSLoaded. - // setTimeout with 0 delay does exactly that. - // Otherwise we might end up with false positive results - setTimeout(() => { - cb.apply(); - }, 0); - }), - }; -}); +import EditorInstance from '~/editor/source_editor_instance'; describe('The basis for an Source Editor extension', () => { const defaultLine = 3; - let ext; let event; - const defaultOptions = { foo: 'bar' }; const findLine = (num) => { - return document.querySelector(`.line-numbers:nth-child(${num})`); + return document.querySelector(`.${EXTENSION_BASE_LINE_NUMBERS_CLASS}:nth-child(${num})`); }; const generateLines = () => { let res = ''; for (let line = 1, lines = 5; line <= lines; line += 1) { - res += `<div class="line-numbers">${line}</div>`; + res += `<div class="${EXTENSION_BASE_LINE_NUMBERS_CLASS}">${line}</div>`; } return res; }; @@ -49,6 +34,9 @@ describe('The basis for an Source Editor extension', () => { }, }; }; + const createInstance = (baseInstance = {}) => { + return new EditorInstance(baseInstance); + }; beforeEach(() => { setFixtures(generateLines()); @@ -59,95 +47,47 @@ describe('The basis for an Source Editor extension', () => { jest.clearAllMocks(); }); - describe('constructor', () => { - it('resets the layout in waitForCSSLoaded callback', async () => { - const instance = { - layout: jest.fn(), - }; - ext = new SourceEditorExtension({ instance }); - expect(instance.layout).not.toHaveBeenCalled(); - - // We're waiting for the waitForCSSLoaded mock to kick in - await jest.runOnlyPendingTimers(); + describe('onUse callback', () => { + it('initializes the line highlighting', () => { + const instance = createInstance(); + const spy = jest.spyOn(SourceEditorExtension, 'highlightLines'); - expect(instance.layout).toHaveBeenCalled(); + instance.use({ definition: SourceEditorExtension }); + expect(spy).toHaveBeenCalled(); }); it.each` - description | instance | options - ${'accepts configuration options and instance'} | ${{}} | ${defaultOptions} - ${'leaves instance intact if no options are passed'} | ${{}} | ${undefined} - ${'does not fail if both instance and the options are omitted'} | ${undefined} | ${undefined} - ${'throws if only options are passed'} | ${undefined} | ${defaultOptions} - `('$description', ({ instance, options } = {}) => { - SourceEditorExtension.deferRerender = jest.fn(); - const originalInstance = { ...instance }; - - if (instance) { - if (options) { - Object.entries(options).forEach((prop) => { - expect(instance[prop]).toBeUndefined(); - }); - // Both instance and options are passed - ext = new SourceEditorExtension({ instance, ...options }); - Object.entries(options).forEach(([prop, value]) => { - expect(ext[prop]).toBeUndefined(); - expect(instance[prop]).toBe(value); - }); + description | instanceType | shouldBeCalled + ${'Sets up'} | ${EDITOR_TYPE_CODE} | ${true} + ${'Does not set up'} | ${EDITOR_TYPE_DIFF} | ${false} + `( + '$description the line linking for $instanceType instance', + ({ instanceType, shouldBeCalled }) => { + const instance = createInstance({ + getEditorType: jest.fn().mockReturnValue(instanceType), + onMouseMove: jest.fn(), + onMouseDown: jest.fn(), + }); + const spy = jest.spyOn(SourceEditorExtension, 'setupLineLinking'); + + instance.use({ definition: SourceEditorExtension }); + if (shouldBeCalled) { + expect(spy).toHaveBeenCalledWith(instance); } else { - ext = new SourceEditorExtension({ instance }); - expect(instance).toEqual(originalInstance); + expect(spy).not.toHaveBeenCalled(); } - } else if (options) { - // Options are passed without instance - expect(() => { - ext = new SourceEditorExtension({ ...options }); - }).toThrow(ERROR_INSTANCE_REQUIRED_FOR_EXTENSION); - } else { - // Neither options nor instance are passed - expect(() => { - ext = new SourceEditorExtension(); - }).not.toThrow(); - } - }); - - it('initializes the line highlighting', () => { - SourceEditorExtension.deferRerender = jest.fn(); - const spy = jest.spyOn(SourceEditorExtension, 'highlightLines'); - ext = new SourceEditorExtension({ instance: {} }); - expect(spy).toHaveBeenCalled(); - }); - - it('sets up the line linking for code instance', () => { - SourceEditorExtension.deferRerender = jest.fn(); - const spy = jest.spyOn(SourceEditorExtension, 'setupLineLinking'); - const instance = { - getEditorType: jest.fn().mockReturnValue(EDITOR_TYPE_CODE), - onMouseMove: jest.fn(), - onMouseDown: jest.fn(), - }; - ext = new SourceEditorExtension({ instance }); - expect(spy).toHaveBeenCalledWith(instance); - }); - - it('does not set up the line linking for diff instance', () => { - SourceEditorExtension.deferRerender = jest.fn(); - const spy = jest.spyOn(SourceEditorExtension, 'setupLineLinking'); - const instance = { - getEditorType: jest.fn().mockReturnValue(EDITOR_TYPE_DIFF), - }; - ext = new SourceEditorExtension({ instance }); - expect(spy).not.toHaveBeenCalled(); - }); + }, + ); }); describe('highlightLines', () => { const revealSpy = jest.fn(); const decorationsSpy = jest.fn(); - const instance = { + const instance = createInstance({ revealLineInCenter: revealSpy, deltaDecorations: decorationsSpy, - }; + }); + instance.use({ definition: SourceEditorExtension }); const defaultDecorationOptions = { isWholeLine: true, className: 'active-line-text', @@ -175,7 +115,7 @@ describe('The basis for an Source Editor extension', () => { ${'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, bounds); + instance.highlightLines(bounds); if (!shouldReveal) { expect(revealSpy).not.toHaveBeenCalled(); expect(decorationsSpy).not.toHaveBeenCalled(); @@ -193,11 +133,11 @@ describe('The basis for an Source Editor extension', () => { } }); - it('stores the line decorations on the instance', () => { + it('stores the line decorations on the instance', () => { decorationsSpy.mockReturnValue('foo'); window.location.hash = '#L10'; expect(instance.lineDecorations).toBeUndefined(); - SourceEditorExtension.highlightLines(instance); + instance.highlightLines(); expect(instance.lineDecorations).toBe('foo'); }); @@ -215,7 +155,7 @@ describe('The basis for an Source Editor extension', () => { }, ]; instance.lineDecorations = oldLineDecorations; - SourceEditorExtension.highlightLines(instance, [7, 10]); + instance.highlightLines([7, 10]); expect(decorationsSpy).toHaveBeenCalledWith(oldLineDecorations, newLineDecorations); }); }); @@ -228,13 +168,18 @@ describe('The basis for an Source Editor extension', () => { options: { isWholeLine: true, className: 'active-line-text' }, }, ]; - const instance = { - deltaDecorations: decorationsSpy, - lineDecorations, - }; + let instance; + + beforeEach(() => { + instance = createInstance({ + deltaDecorations: decorationsSpy, + lineDecorations, + }); + instance.use({ definition: SourceEditorExtension }); + }); it('removes all existing decorations', () => { - SourceEditorExtension.removeHighlights(instance); + instance.removeHighlights(); expect(decorationsSpy).toHaveBeenCalledWith(lineDecorations, []); }); }); @@ -261,9 +206,9 @@ describe('The basis for an Source Editor extension', () => { }); it.each` - desc | eventTrigger | shouldRemove - ${'does not remove the line decorations if the event is triggered on a wrong node'} | ${null} | ${false} - ${'removes existing line decorations when clicking a line number'} | ${'.link-anchor'} | ${true} + desc | eventTrigger | shouldRemove + ${'does not remove the line decorations if the event is triggered on a wrong node'} | ${null} | ${false} + ${'removes existing line decorations when clicking a line number'} | ${`.${EXTENSION_BASE_LINE_LINK_ANCHOR_CLASS}`} | ${true} `('$desc', ({ eventTrigger, shouldRemove } = {}) => { event = generateEventMock({ el: eventTrigger ? document.querySelector(eventTrigger) : null }); instance.onMouseDown.mockImplementation((fn) => { diff --git a/spec/frontend/editor/source_editor_extension_spec.js b/spec/frontend/editor/source_editor_extension_spec.js index de3f9da0aed..c5fa795f3b7 100644 --- a/spec/frontend/editor/source_editor_extension_spec.js +++ b/spec/frontend/editor/source_editor_extension_spec.js @@ -40,7 +40,7 @@ describe('Editor Extension', () => { expect(extension).toEqual( expect.objectContaining({ - name: expectedName, + extensionName: expectedName, setupOptions, }), ); diff --git a/spec/frontend/editor/source_editor_instance_spec.js b/spec/frontend/editor/source_editor_instance_spec.js index 38844b6cafe..f9518743ef8 100644 --- a/spec/frontend/editor/source_editor_instance_spec.js +++ b/spec/frontend/editor/source_editor_instance_spec.js @@ -32,11 +32,17 @@ describe('Source Editor Instance', () => { ]; const fooFn = jest.fn(); + const fooProp = 'foo'; class DummyExt { // eslint-disable-next-line class-methods-use-this + get extensionName() { + return 'DummyExt'; + } + // eslint-disable-next-line class-methods-use-this provides() { return { fooFn, + fooProp, }; } } @@ -64,7 +70,7 @@ describe('Source Editor Instance', () => { }); describe('proxy', () => { - it('returns prop from an extension if extension provides it', () => { + it('returns a method from an extension if extension provides it', () => { seInstance = new SourceEditorInstance(); seInstance.use({ definition: DummyExt }); @@ -73,6 +79,13 @@ describe('Source Editor Instance', () => { expect(fooFn).toHaveBeenCalled(); }); + it('returns a prop from an extension if extension provides it', () => { + seInstance = new SourceEditorInstance(); + seInstance.use({ definition: DummyExt }); + + expect(seInstance.fooProp).toBe('foo'); + }); + it.each` stringPropToPass | objPropToPass | setupOptions ${undefined} | ${undefined} | ${undefined} @@ -118,20 +131,20 @@ describe('Source Editor Instance', () => { it("correctly sets the context of the 'this' keyword for the extension's methods", () => { seInstance = new SourceEditorInstance(); - seInstance.use({ definition: SEWithSetupExt }); + const extension = seInstance.use({ definition: SEWithSetupExt }); - expect(seInstance.giveMeContext().constructor).toEqual(SEWithSetupExt); + expect(seInstance.giveMeContext()).toEqual(extension.obj); }); it('returns props from SE instance itself if no extension provides the prop', () => { seInstance = new SourceEditorInstance({ use: fooFn, }); - jest.spyOn(seInstance, 'use').mockImplementation(() => {}); - expect(seInstance.use).not.toHaveBeenCalled(); + const spy = jest.spyOn(seInstance.constructor.prototype, 'use').mockImplementation(() => {}); + expect(spy).not.toHaveBeenCalled(); expect(fooFn).not.toHaveBeenCalled(); seInstance.use(); - expect(seInstance.use).toHaveBeenCalled(); + expect(spy).toHaveBeenCalled(); expect(fooFn).not.toHaveBeenCalled(); }); diff --git a/spec/frontend/editor/source_editor_markdown_ext_spec.js b/spec/frontend/editor/source_editor_markdown_ext_spec.js index 4a50d801296..eecd23bff6e 100644 --- a/spec/frontend/editor/source_editor_markdown_ext_spec.js +++ b/spec/frontend/editor/source_editor_markdown_ext_spec.js @@ -9,7 +9,6 @@ describe('Markdown Extension for Source Editor', () => { let instance; let editorEl; let mockAxios; - const previewMarkdownPath = '/gitlab/fooGroup/barProj/preview_markdown'; const firstLine = 'This is a'; const secondLine = 'multiline'; const thirdLine = 'string with some **markup**'; @@ -36,7 +35,7 @@ describe('Markdown Extension for Source Editor', () => { blobPath: markdownPath, blobContent: text, }); - instance.use(new EditorMarkdownExtension({ instance, previewMarkdownPath })); + instance.use({ definition: EditorMarkdownExtension }); }); afterEach(() => { @@ -164,13 +163,11 @@ describe('Markdown Extension for Source Editor', () => { }); it('does not fail when only `toSelect` is supplied and fetches the text from selection', () => { - jest.spyOn(instance, 'getSelectedText'); const toSelect = 'string'; selectSecondAndThirdLines(); instance.selectWithinSelection(toSelect); - expect(instance.getSelectedText).toHaveBeenCalled(); expect(selectionToString()).toBe(`[3,1 -> 3,${toSelect.length + 1}]`); }); diff --git a/spec/frontend/editor/source_editor_markdown_livepreview_ext_spec.js b/spec/frontend/editor/source_editor_markdown_livepreview_ext_spec.js index 3d797073c05..c8d016e10ac 100644 --- a/spec/frontend/editor/source_editor_markdown_livepreview_ext_spec.js +++ b/spec/frontend/editor/source_editor_markdown_livepreview_ext_spec.js @@ -13,6 +13,7 @@ import SourceEditor from '~/editor/source_editor'; import createFlash from '~/flash'; import axios from '~/lib/utils/axios_utils'; import syntaxHighlight from '~/syntax_highlight'; +import { spyOnApi } from './helpers'; jest.mock('~/syntax_highlight'); jest.mock('~/flash'); @@ -23,6 +24,7 @@ describe('Markdown Live Preview Extension for Source Editor', () => { let editorEl; let panelSpy; let mockAxios; + let extension; const previewMarkdownPath = '/gitlab/fooGroup/barProj/preview_markdown'; const firstLine = 'This is a'; const secondLine = 'multiline'; @@ -47,8 +49,11 @@ describe('Markdown Live Preview Extension for Source Editor', () => { blobPath: markdownPath, blobContent: text, }); - instance.use(new EditorMarkdownPreviewExtension({ instance, previewMarkdownPath })); - panelSpy = jest.spyOn(EditorMarkdownPreviewExtension, 'togglePreviewPanel'); + extension = instance.use({ + definition: EditorMarkdownPreviewExtension, + setupOptions: { previewMarkdownPath }, + }); + panelSpy = jest.spyOn(extension.obj.constructor.prototype, 'togglePreviewPanel'); }); afterEach(() => { @@ -57,14 +62,14 @@ describe('Markdown Live Preview Extension for Source Editor', () => { mockAxios.restore(); }); - it('sets up the instance', () => { - expect(instance.preview).toEqual({ + it('sets up the preview on the instance', () => { + expect(instance.markdownPreview).toEqual({ el: undefined, action: expect.any(Object), shown: false, modelChangeListener: undefined, + path: previewMarkdownPath, }); - expect(instance.previewMarkdownPath).toBe(previewMarkdownPath); }); describe('model language changes listener', () => { @@ -72,14 +77,22 @@ describe('Markdown Live Preview Extension for Source Editor', () => { let actionSpy; beforeEach(async () => { - cleanupSpy = jest.spyOn(instance, 'cleanup'); - actionSpy = jest.spyOn(instance, 'setupPreviewAction'); + cleanupSpy = jest.fn(); + actionSpy = jest.fn(); + spyOnApi(extension, { + cleanup: cleanupSpy, + setupPreviewAction: actionSpy, + }); await togglePreview(); }); + afterEach(() => { + jest.clearAllMocks(); + }); + it('cleans up when switching away from markdown', () => { - expect(instance.cleanup).not.toHaveBeenCalled(); - expect(instance.setupPreviewAction).not.toHaveBeenCalled(); + expect(cleanupSpy).not.toHaveBeenCalled(); + expect(actionSpy).not.toHaveBeenCalled(); instance.updateModelLanguage(plaintextPath); @@ -110,8 +123,12 @@ describe('Markdown Live Preview Extension for Source Editor', () => { let actionSpy; beforeEach(() => { - cleanupSpy = jest.spyOn(instance, 'cleanup'); - actionSpy = jest.spyOn(instance, 'setupPreviewAction'); + cleanupSpy = jest.fn(); + actionSpy = jest.fn(); + spyOnApi(extension, { + cleanup: cleanupSpy, + setupPreviewAction: actionSpy, + }); instance.togglePreview(); }); @@ -153,14 +170,17 @@ describe('Markdown Live Preview Extension for Source Editor', () => { }); it('disposes the modelChange listener and does not fetch preview on content changes', () => { - expect(instance.preview.modelChangeListener).toBeDefined(); - jest.spyOn(instance, 'fetchPreview'); + expect(instance.markdownPreview.modelChangeListener).toBeDefined(); + const fetchPreviewSpy = jest.fn(); + spyOnApi(extension, { + fetchPreview: fetchPreviewSpy, + }); instance.cleanup(); instance.setValue('Foo Bar'); jest.advanceTimersByTime(EXTENSION_MARKDOWN_PREVIEW_UPDATE_DELAY); - expect(instance.fetchPreview).not.toHaveBeenCalled(); + expect(fetchPreviewSpy).not.toHaveBeenCalled(); }); it('removes the contextual menu action', () => { @@ -172,13 +192,13 @@ describe('Markdown Live Preview Extension for Source Editor', () => { }); it('toggles the `shown` flag', () => { - expect(instance.preview.shown).toBe(true); + expect(instance.markdownPreview.shown).toBe(true); instance.cleanup(); - expect(instance.preview.shown).toBe(false); + expect(instance.markdownPreview.shown).toBe(false); }); it('toggles the panel only if the preview is visible', () => { - const { el: previewEl } = instance.preview; + const { el: previewEl } = instance.markdownPreview; const parentEl = previewEl.parentElement; expect(previewEl).toBeVisible(); @@ -200,7 +220,7 @@ describe('Markdown Live Preview Extension for Source Editor', () => { it('toggles the layout only if the preview is visible', () => { const { width } = instance.getLayoutInfo(); - expect(instance.preview.shown).toBe(true); + expect(instance.markdownPreview.shown).toBe(true); instance.cleanup(); @@ -234,13 +254,13 @@ describe('Markdown Live Preview Extension for Source Editor', () => { }); it('puts the fetched content into the preview DOM element', async () => { - instance.preview.el = editorEl.parentElement; + instance.markdownPreview.el = editorEl.parentElement; await fetchPreview(); - expect(instance.preview.el.innerHTML).toEqual(responseData); + expect(instance.markdownPreview.el.innerHTML).toEqual(responseData); }); it('applies syntax highlighting to the preview content', async () => { - instance.preview.el = editorEl.parentElement; + instance.markdownPreview.el = editorEl.parentElement; await fetchPreview(); expect(syntaxHighlight).toHaveBeenCalled(); }); @@ -266,14 +286,17 @@ describe('Markdown Live Preview Extension for Source Editor', () => { }); it('toggles preview when the action is triggered', () => { - jest.spyOn(instance, 'togglePreview').mockImplementation(); + const togglePreviewSpy = jest.fn(); + spyOnApi(extension, { + togglePreview: togglePreviewSpy, + }); - expect(instance.togglePreview).not.toHaveBeenCalled(); + expect(togglePreviewSpy).not.toHaveBeenCalled(); const action = instance.getAction(EXTENSION_MARKDOWN_PREVIEW_ACTION_ID); action.run(); - expect(instance.togglePreview).toHaveBeenCalled(); + expect(togglePreviewSpy).toHaveBeenCalled(); }); }); @@ -283,39 +306,39 @@ describe('Markdown Live Preview Extension for Source Editor', () => { }); it('toggles preview flag on instance', () => { - expect(instance.preview.shown).toBe(false); + expect(instance.markdownPreview.shown).toBe(false); instance.togglePreview(); - expect(instance.preview.shown).toBe(true); + expect(instance.markdownPreview.shown).toBe(true); instance.togglePreview(); - expect(instance.preview.shown).toBe(false); + expect(instance.markdownPreview.shown).toBe(false); }); describe('panel DOM element set up', () => { it('sets up an element to contain the preview and stores it on instance', () => { - expect(instance.preview.el).toBeUndefined(); + expect(instance.markdownPreview.el).toBeUndefined(); instance.togglePreview(); - expect(instance.preview.el).toBeDefined(); - expect(instance.preview.el.classList.contains(EXTENSION_MARKDOWN_PREVIEW_PANEL_CLASS)).toBe( - true, - ); + expect(instance.markdownPreview.el).toBeDefined(); + expect( + instance.markdownPreview.el.classList.contains(EXTENSION_MARKDOWN_PREVIEW_PANEL_CLASS), + ).toBe(true); }); it('re-uses existing preview DOM element on repeated calls', () => { instance.togglePreview(); - const origPreviewEl = instance.preview.el; + const origPreviewEl = instance.markdownPreview.el; instance.togglePreview(); - expect(instance.preview.el).toBe(origPreviewEl); + expect(instance.markdownPreview.el).toBe(origPreviewEl); }); it('hides the preview DOM element by default', () => { panelSpy.mockImplementation(); instance.togglePreview(); - expect(instance.preview.el.style.display).toBe('none'); + expect(instance.markdownPreview.el.style.display).toBe('none'); }); }); @@ -350,9 +373,9 @@ describe('Markdown Live Preview Extension for Source Editor', () => { it('toggles visibility of the preview DOM element', async () => { await togglePreview(); - expect(instance.preview.el.style.display).toBe('block'); + expect(instance.markdownPreview.el.style.display).toBe('block'); await togglePreview(); - expect(instance.preview.el.style.display).toBe('none'); + expect(instance.markdownPreview.el.style.display).toBe('none'); }); describe('hidden preview DOM element', () => { @@ -367,9 +390,9 @@ describe('Markdown Live Preview Extension for Source Editor', () => { }); it('stores disposable listener for model changes', async () => { - expect(instance.preview.modelChangeListener).toBeUndefined(); + expect(instance.markdownPreview.modelChangeListener).toBeUndefined(); await togglePreview(); - expect(instance.preview.modelChangeListener).toBeDefined(); + expect(instance.markdownPreview.modelChangeListener).toBeDefined(); }); }); @@ -386,7 +409,7 @@ describe('Markdown Live Preview Extension for Source Editor', () => { it('disposes the model change event listener', () => { const disposeSpy = jest.fn(); - instance.preview.modelChangeListener = { + instance.markdownPreview.modelChangeListener = { dispose: disposeSpy, }; instance.togglePreview(); diff --git a/spec/frontend/editor/source_editor_spec.js b/spec/frontend/editor/source_editor_spec.js index f1b887b2dc1..bc53202c919 100644 --- a/spec/frontend/editor/source_editor_spec.js +++ b/spec/frontend/editor/source_editor_spec.js @@ -1,15 +1,28 @@ -/* eslint-disable max-classes-per-file */ import { editor as monacoEditor, languages as monacoLanguages } from 'monaco-editor'; import { SOURCE_EDITOR_INSTANCE_ERROR_NO_EL, URI_PREFIX, EDITOR_READY_EVENT, } from '~/editor/constants'; -import { SourceEditorExtension } from '~/editor/extensions/source_editor_extension_base'; import SourceEditor from '~/editor/source_editor'; import { DEFAULT_THEME, themes } from '~/ide/lib/themes'; import { joinPaths } from '~/lib/utils/url_utility'; +jest.mock('~/helpers/startup_css_helper', () => { + return { + waitForCSSLoaded: jest.fn().mockImplementation((cb) => { + // We have to artificially put the callback's execution + // to the end of the current call stack to be able to + // test that the callback is called after waitForCSSLoaded. + // setTimeout with 0 delay does exactly that. + // Otherwise we might end up with false positive results + setTimeout(() => { + cb.apply(); + }, 0); + }), + }; +}); + describe('Base editor', () => { let editorEl; let editor; @@ -18,7 +31,6 @@ describe('Base editor', () => { const blobContent = 'Foo Bar'; const blobPath = 'test.md'; const blobGlobalId = 'snippet_777'; - const fakeModel = { foo: 'bar', dispose: jest.fn() }; beforeEach(() => { setFixtures('<div id="editor" data-editor-loading></div>'); @@ -51,16 +63,6 @@ describe('Base editor', () => { describe('instance of the Source Editor', () => { let modelSpy; let instanceSpy; - const setModel = jest.fn(); - const dispose = jest.fn(); - const mockModelReturn = (res = fakeModel) => { - modelSpy = jest.spyOn(monacoEditor, 'createModel').mockImplementation(() => res); - }; - const mockDecorateInstance = (decorations = {}) => { - jest.spyOn(SourceEditor, 'convertMonacoToELInstance').mockImplementation((inst) => { - return Object.assign(inst, decorations); - }); - }; beforeEach(() => { modelSpy = jest.spyOn(monacoEditor, 'createModel'); @@ -72,46 +74,38 @@ describe('Base editor', () => { }); it('throws an error if no dom element is supplied', () => { - mockDecorateInstance(); - expect(() => { + const create = () => { editor.createInstance(); - }).toThrow(SOURCE_EDITOR_INSTANCE_ERROR_NO_EL); + }; + expect(create).toThrow(SOURCE_EDITOR_INSTANCE_ERROR_NO_EL); expect(modelSpy).not.toHaveBeenCalled(); expect(instanceSpy).not.toHaveBeenCalled(); - expect(SourceEditor.convertMonacoToELInstance).not.toHaveBeenCalled(); }); - it('creates model to be supplied to Monaco editor', () => { - mockModelReturn(); - mockDecorateInstance({ - setModel, - }); - editor.createInstance(defaultArguments); + it('creates model and attaches it to the instance', () => { + jest.spyOn(monacoEditor, 'createModel'); + const instance = editor.createInstance(defaultArguments); - expect(modelSpy).toHaveBeenCalledWith( + expect(monacoEditor.createModel).toHaveBeenCalledWith( blobContent, undefined, expect.objectContaining({ path: uriFilePath, }), ); - expect(setModel).toHaveBeenCalledWith(fakeModel); + expect(instance.getModel().getValue()).toEqual(defaultArguments.blobContent); }); it('does not create a model automatically if model is passed as `null`', () => { - mockDecorateInstance({ - setModel, - }); - editor.createInstance({ ...defaultArguments, model: null }); - expect(modelSpy).not.toHaveBeenCalled(); - expect(setModel).not.toHaveBeenCalled(); + const instance = editor.createInstance({ ...defaultArguments, model: null }); + expect(instance.getModel()).toBeNull(); }); it('initializes the instance on a supplied DOM node', () => { editor.createInstance({ el: editorEl }); - expect(editor.editorEl).not.toBe(null); + expect(editor.editorEl).not.toBeNull(); expect(instanceSpy).toHaveBeenCalledWith(editorEl, expect.anything()); }); @@ -142,32 +136,43 @@ describe('Base editor', () => { }); it('disposes instance when the global editor is disposed', () => { - mockDecorateInstance({ - dispose, - }); - editor.createInstance(defaultArguments); + const instance = editor.createInstance(defaultArguments); + instance.dispose = jest.fn(); - expect(dispose).not.toHaveBeenCalled(); + expect(instance.dispose).not.toHaveBeenCalled(); editor.dispose(); - expect(dispose).toHaveBeenCalled(); + expect(instance.dispose).toHaveBeenCalled(); }); it("removes the disposed instance from the global editor's storage and disposes the associated model", () => { - mockModelReturn(); - mockDecorateInstance({ - setModel, - }); const instance = editor.createInstance(defaultArguments); expect(editor.instances).toHaveLength(1); - expect(fakeModel.dispose).not.toHaveBeenCalled(); + expect(instance.getModel()).not.toBeNull(); instance.dispose(); expect(editor.instances).toHaveLength(0); - expect(fakeModel.dispose).toHaveBeenCalled(); + expect(instance.getModel()).toBeNull(); + }); + + it('resets the layout in waitForCSSLoaded callback', async () => { + const layoutSpy = jest.fn(); + jest.spyOn(monacoEditor, 'create').mockReturnValue({ + layout: layoutSpy, + setModel: jest.fn(), + onDidDispose: jest.fn(), + dispose: jest.fn(), + }); + editor.createInstance(defaultArguments); + expect(layoutSpy).not.toHaveBeenCalled(); + + // We're waiting for the waitForCSSLoaded mock to kick in + await jest.runOnlyPendingTimers(); + + expect(layoutSpy).toHaveBeenCalled(); }); }); @@ -213,26 +218,17 @@ describe('Base editor', () => { }); it('correctly disposes the diff editor model', () => { - const modifiedModel = fakeModel; - const originalModel = { ...fakeModel }; - mockDecorateInstance({ - getModel: jest.fn().mockReturnValue({ - original: originalModel, - modified: modifiedModel, - }), - }); - const instance = editor.createDiffInstance({ ...defaultArguments, blobOriginalContent }); expect(editor.instances).toHaveLength(1); - expect(originalModel.dispose).not.toHaveBeenCalled(); - expect(modifiedModel.dispose).not.toHaveBeenCalled(); + expect(instance.getOriginalEditor().getModel()).not.toBeNull(); + expect(instance.getModifiedEditor().getModel()).not.toBeNull(); instance.dispose(); expect(editor.instances).toHaveLength(0); - expect(originalModel.dispose).toHaveBeenCalled(); - expect(modifiedModel.dispose).toHaveBeenCalled(); + expect(instance.getOriginalEditor().getModel()).toBeNull(); + expect(instance.getModifiedEditor().getModel()).toBeNull(); }); }); }); @@ -354,196 +350,19 @@ describe('Base editor', () => { expect(instance.getValue()).toBe(blobContent); }); - it('is capable of changing the language of the model', () => { - // ignore warnings and errors Monaco posts during setup - // (due to being called from Jest/Node.js environment) - jest.spyOn(console, 'warn').mockImplementation(() => {}); - jest.spyOn(console, 'error').mockImplementation(() => {}); - - const blobRenamedPath = 'test.js'; - - expect(instance.getModel().getLanguageIdentifier().language).toBe('markdown'); - instance.updateModelLanguage(blobRenamedPath); - - expect(instance.getModel().getLanguageIdentifier().language).toBe('javascript'); - }); - - it('falls back to plaintext if there is no language associated with an extension', () => { - const blobRenamedPath = 'test.myext'; - const spy = jest.spyOn(console, 'error').mockImplementation(() => {}); - - instance.updateModelLanguage(blobRenamedPath); - - expect(spy).not.toHaveBeenCalled(); - expect(instance.getModel().getLanguageIdentifier().language).toBe('plaintext'); - }); - }); - - describe('extensions', () => { - let instance; - const alphaRes = jest.fn(); - const betaRes = jest.fn(); - const fooRes = jest.fn(); - const barRes = jest.fn(); - class AlphaClass { - constructor() { - this.res = alphaRes; - } - alpha() { - return this?.nonExistentProp || alphaRes; - } - } - class BetaClass { - beta() { - return this?.nonExistentProp || betaRes; - } - } - class WithStaticMethod { - constructor({ instance: inst, ...options } = {}) { - Object.assign(inst, options); - } - static computeBoo(a) { - return a + 1; - } - boo() { - return WithStaticMethod.computeBoo(this.base); - } - } - class WithStaticMethodExtended extends SourceEditorExtension { - static computeBoo(a) { - return a + 1; - } - boo() { - return WithStaticMethodExtended.computeBoo(this.base); - } - } - const AlphaExt = new AlphaClass(); - const BetaExt = new BetaClass(); - const FooObjExt = { - foo() { - return fooRes; - }, - }; - const BarObjExt = { - bar() { - return barRes; - }, - }; - - describe('basic functionality', () => { - beforeEach(() => { - instance = editor.createInstance({ el: editorEl, blobPath, blobContent }); - }); - - it('does not fail if no extensions supplied', () => { - const spy = jest.spyOn(global.console, 'error'); - instance.use(); - - expect(spy).not.toHaveBeenCalled(); - }); - - it("does not extend instance with extension's constructor", () => { - expect(instance.constructor).toBeDefined(); - const { constructor } = instance; - - expect(AlphaExt.constructor).toBeDefined(); - expect(AlphaExt.constructor).not.toEqual(constructor); - - instance.use(AlphaExt); - expect(instance.constructor).toBe(constructor); - }); - - it.each` - type | extensions | methods | expectations - ${'ES6 classes'} | ${AlphaExt} | ${['alpha']} | ${[alphaRes]} - ${'multiple ES6 classes'} | ${[AlphaExt, BetaExt]} | ${['alpha', 'beta']} | ${[alphaRes, betaRes]} - ${'simple objects'} | ${FooObjExt} | ${['foo']} | ${[fooRes]} - ${'multiple simple objects'} | ${[FooObjExt, BarObjExt]} | ${['foo', 'bar']} | ${[fooRes, barRes]} - ${'combination of ES6 classes and objects'} | ${[AlphaExt, BarObjExt]} | ${['alpha', 'bar']} | ${[alphaRes, barRes]} - `('is extensible with $type', ({ extensions, methods, expectations } = {}) => { - methods.forEach((method) => { - expect(instance[method]).toBeUndefined(); - }); - - instance.use(extensions); - - methods.forEach((method) => { - expect(instance[method]).toBeDefined(); - }); - - expectations.forEach((expectation, i) => { - expect(instance[methods[i]].call()).toEqual(expectation); - }); - }); - - it('does not extend instance with private data of an extension', () => { - const ext = new WithStaticMethod({ instance }); - ext.staticMethod = () => { - return 'foo'; + it('emits the EDITOR_READY_EVENT event after setting up the instance', () => { + jest.spyOn(monacoEditor, 'create').mockImplementation(() => { + return { + setModel: jest.fn(), + onDidDispose: jest.fn(), + layout: jest.fn(), }; - ext.staticProp = 'bar'; - - expect(instance.boo).toBeUndefined(); - expect(instance.staticMethod).toBeUndefined(); - expect(instance.staticProp).toBeUndefined(); - - instance.use(ext); - - expect(instance.boo).toBeDefined(); - expect(instance.staticMethod).toBeUndefined(); - expect(instance.staticProp).toBeUndefined(); - }); - - it.each([WithStaticMethod, WithStaticMethodExtended])( - 'properly resolves data for an extension with private data', - (ExtClass) => { - const base = 1; - expect(instance.base).toBeUndefined(); - expect(instance.boo).toBeUndefined(); - - const ext = new ExtClass({ instance, base }); - - instance.use(ext); - expect(instance.base).toBe(1); - expect(instance.boo()).toBe(2); - }, - ); - - it('uses the last definition of a method in case of an overlap', () => { - const FooObjExt2 = { foo: 'foo2' }; - instance.use([FooObjExt, BarObjExt, FooObjExt2]); - expect(instance).toMatchObject({ - foo: 'foo2', - ...BarObjExt, - }); - }); - - it('correctly resolves references withing extensions', () => { - const FunctionExt = { - inst() { - return this; - }, - mod() { - return this.getModel(); - }, - }; - instance.use(FunctionExt); - expect(instance.inst()).toEqual(editor.instances[0]); - }); - - it('emits the EDITOR_READY_EVENT event after setting up the instance', () => { - jest.spyOn(monacoEditor, 'create').mockImplementation(() => { - return { - setModel: jest.fn(), - onDidDispose: jest.fn(), - }; - }); - const eventSpy = jest.fn(); - editorEl.addEventListener(EDITOR_READY_EVENT, eventSpy); - expect(eventSpy).not.toHaveBeenCalled(); - instance = editor.createInstance({ el: editorEl }); - expect(eventSpy).toHaveBeenCalled(); }); + const eventSpy = jest.fn(); + editorEl.addEventListener(EDITOR_READY_EVENT, eventSpy); + expect(eventSpy).not.toHaveBeenCalled(); + editor.createInstance({ el: editorEl }); + expect(eventSpy).toHaveBeenCalled(); }); }); diff --git a/spec/frontend/editor/source_editor_yaml_ext_spec.js b/spec/frontend/editor/source_editor_yaml_ext_spec.js index 97d2b0b21d0..a861d9c7a45 100644 --- a/spec/frontend/editor/source_editor_yaml_ext_spec.js +++ b/spec/frontend/editor/source_editor_yaml_ext_spec.js @@ -2,6 +2,10 @@ 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'; +import { spyOnApi } from 'jest/editor/helpers'; + +let baseExtension; +let yamlExtension; const getEditorInstance = (editorInstanceOptions = {}) => { setFixtures('<div id="editor"></div>'); @@ -16,7 +20,10 @@ const getEditorInstance = (editorInstanceOptions = {}) => { const getEditorInstanceWithExtension = (extensionOptions = {}, editorInstanceOptions = {}) => { setFixtures('<div id="editor"></div>'); const instance = getEditorInstance(editorInstanceOptions); - instance.use(new YamlEditorExtension({ instance, ...extensionOptions })); + [baseExtension, yamlExtension] = instance.use([ + { definition: SourceEditorExtension }, + { definition: YamlEditorExtension, setupOptions: extensionOptions }, + ]); // Remove the below once // https://gitlab.com/gitlab-org/gitlab/-/issues/325992 is resolved @@ -29,19 +36,16 @@ const getEditorInstanceWithExtension = (extensionOptions = {}, editorInstanceOpt describe('YamlCreatorExtension', () => { describe('constructor', () => { - it('saves constructor options', () => { + it('saves setupOptions options on the extension, but does not expose those to instance', () => { + const highlightPath = 'foo'; const instance = getEditorInstanceWithExtension({ - highlightPath: 'foo', + highlightPath, enableComments: true, }); - expect(instance).toEqual( - expect.objectContaining({ - options: expect.objectContaining({ - highlightPath: 'foo', - enableComments: true, - }), - }), - ); + expect(yamlExtension.obj.highlightPath).toBe(highlightPath); + expect(yamlExtension.obj.enableComments).toBe(true); + expect(instance.highlightPath).toBeUndefined(); + expect(instance.enableComments).toBeUndefined(); }); it('dumps values loaded with the model constructor options', () => { @@ -55,7 +59,7 @@ describe('YamlCreatorExtension', () => { it('registers the onUpdate() function', () => { const instance = getEditorInstance(); const onDidChangeModelContent = jest.spyOn(instance, 'onDidChangeModelContent'); - instance.use(new YamlEditorExtension({ instance })); + instance.use({ definition: YamlEditorExtension }); expect(onDidChangeModelContent).toHaveBeenCalledWith(expect.any(Function)); }); @@ -82,21 +86,21 @@ describe('YamlCreatorExtension', () => { it('should call transformComments if enableComments is true', () => { const instance = getEditorInstanceWithExtension({ enableComments: true }); const transformComments = jest.spyOn(YamlEditorExtension, 'transformComments'); - YamlEditorExtension.initFromModel(instance, model); + instance.initFromModel(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); + instance.initFromModel(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); + instance.initFromModel(model); expect(setValue).toHaveBeenCalledWith(doc.toString()); }); }); @@ -240,26 +244,35 @@ foo: 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'); + const updateValueSpy = jest.fn(); + spyOnApi(yamlExtension, { + updateValue: updateValueSpy, + }); instance.setDoc(doc); expect(setValue).toHaveBeenCalledWith(doc.toString()); - expect(updateValue).not.toHaveBeenCalled(); + expect(updateValueSpy).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'); + const updateValueSpy = jest.fn(); + spyOnApi(yamlExtension, { + updateValue: updateValueSpy, + }); instance.setDoc(doc); expect(setValue).not.toHaveBeenCalled(); - expect(updateValue).toHaveBeenCalledWith(doc.toString()); + expect(updateValueSpy).toHaveBeenCalledWith(instance, doc.toString()); }); it('should trigger the onUpdate method', () => { const instance = getEditorInstanceWithExtension(); - const onUpdate = jest.spyOn(instance, 'onUpdate'); + const onUpdateSpy = jest.fn(); + spyOnApi(yamlExtension, { + onUpdate: onUpdateSpy, + }); instance.setDoc(doc); - expect(onUpdate).toHaveBeenCalled(); + expect(onUpdateSpy).toHaveBeenCalled(); }); }); @@ -320,9 +333,12 @@ foo: it('calls highlight', () => { const highlightPath = 'foo'; const instance = getEditorInstanceWithExtension({ highlightPath }); - instance.highlight = jest.fn(); + // Here we do not spy on the public API method of the extension, but rather + // the public method of the extension's instance. + // This is required based on how `onUpdate` works + const highlightSpy = jest.spyOn(yamlExtension.obj, 'highlight'); instance.onUpdate(); - expect(instance.highlight).toHaveBeenCalledWith(highlightPath); + expect(highlightSpy).toHaveBeenCalledWith(instance, highlightPath); }); }); @@ -350,8 +366,12 @@ foo: beforeEach(() => { instance = getEditorInstanceWithExtension({ highlightPath: highlightPathOnSetup }, { value }); - highlightLinesSpy = jest.spyOn(SourceEditorExtension, 'highlightLines'); - removeHighlightsSpy = jest.spyOn(SourceEditorExtension, 'removeHighlights'); + highlightLinesSpy = jest.fn(); + removeHighlightsSpy = jest.fn(); + spyOnApi(baseExtension, { + highlightLines: highlightLinesSpy, + removeHighlights: removeHighlightsSpy, + }); }); afterEach(() => { @@ -361,7 +381,7 @@ foo: it('saves the highlighted path in highlightPath', () => { const path = 'foo.bar'; instance.highlight(path); - expect(instance.options.highlightPath).toEqual(path); + expect(yamlExtension.obj.highlightPath).toEqual(path); }); it('calls highlightLines with a number of lines', () => { @@ -374,14 +394,14 @@ foo: instance.highlight(null); expect(removeHighlightsSpy).toHaveBeenCalledWith(instance); expect(highlightLinesSpy).not.toHaveBeenCalled(); - expect(instance.options.highlightPath).toBeNull(); + expect(yamlExtension.obj.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(yamlExtension.obj.highlightPath).toEqual(highlightPathOnSetup); expect(highlightLinesSpy).not.toHaveBeenCalled(); expect(removeHighlightsSpy).not.toHaveBeenCalled(); }); diff --git a/spec/frontend/ide/components/repo_editor_spec.js b/spec/frontend/ide/components/repo_editor_spec.js index c2212eea849..c957c64aa10 100644 --- a/spec/frontend/ide/components/repo_editor_spec.js +++ b/spec/frontend/ide/components/repo_editor_spec.js @@ -9,7 +9,7 @@ import waitUsingRealTimer from 'helpers/wait_using_real_timer'; import { exampleConfigs, exampleFiles } from 'jest/ide/lib/editorconfig/mock_data'; import { EDITOR_CODE_INSTANCE_FN, EDITOR_DIFF_INSTANCE_FN } from '~/editor/constants'; import { EditorMarkdownExtension } from '~/editor/extensions/source_editor_markdown_ext'; -import { EditorWebIdeExtension } from '~/editor/extensions/source_editor_webide_ext'; +import { EditorMarkdownPreviewExtension } from '~/editor/extensions/source_editor_markdown_livepreview_ext'; import SourceEditor from '~/editor/source_editor'; import RepoEditor from '~/ide/components/repo_editor.vue'; import { @@ -23,6 +23,8 @@ import service from '~/ide/services'; import { createStoreOptions } from '~/ide/stores'; import axios from '~/lib/utils/axios_utils'; import ContentViewer from '~/vue_shared/components/content_viewer/content_viewer.vue'; +import SourceEditorInstance from '~/editor/source_editor_instance'; +import { spyOnApi } from 'jest/editor/helpers'; import { file } from '../helpers'; const PREVIEW_MARKDOWN_PATH = '/foo/bar/preview_markdown'; @@ -101,6 +103,7 @@ describe('RepoEditor', () => { let createDiffInstanceSpy; let createModelSpy; let applyExtensionSpy; + let extensionsStore; const waitForEditorSetup = () => new Promise((resolve) => { @@ -120,6 +123,7 @@ describe('RepoEditor', () => { }); await waitForPromises(); vm = wrapper.vm; + extensionsStore = wrapper.vm.globalEditor.extensionsStore; jest.spyOn(vm, 'getFileData').mockResolvedValue(); jest.spyOn(vm, 'getRawFileData').mockResolvedValue(); }; @@ -127,28 +131,12 @@ describe('RepoEditor', () => { const findEditor = () => wrapper.find('[data-testid="editor-container"]'); const findTabs = () => wrapper.findAll('.ide-mode-tabs .nav-links li'); const findPreviewTab = () => wrapper.find('[data-testid="preview-tab"]'); - const expectEditorMarkdownExtension = (shouldHaveExtension) => { - if (shouldHaveExtension) { - expect(applyExtensionSpy).toHaveBeenCalledWith( - wrapper.vm.editor, - expect.any(EditorMarkdownExtension), - ); - // TODO: spying on extensions causes Jest to blow up, so we have to assert on - // the public property the extension adds, as opposed to the args passed to the ctor - expect(wrapper.vm.editor.previewMarkdownPath).toBe(PREVIEW_MARKDOWN_PATH); - } else { - expect(applyExtensionSpy).not.toHaveBeenCalledWith( - wrapper.vm.editor, - expect.any(EditorMarkdownExtension), - ); - } - }; beforeEach(() => { createInstanceSpy = jest.spyOn(SourceEditor.prototype, EDITOR_CODE_INSTANCE_FN); createDiffInstanceSpy = jest.spyOn(SourceEditor.prototype, EDITOR_DIFF_INSTANCE_FN); createModelSpy = jest.spyOn(monacoEditor, 'createModel'); - applyExtensionSpy = jest.spyOn(SourceEditor, 'instanceApplyExtension'); + applyExtensionSpy = jest.spyOn(SourceEditorInstance.prototype, 'use'); jest.spyOn(service, 'getFileData').mockResolvedValue(); jest.spyOn(service, 'getRawFileData').mockResolvedValue(); }); @@ -275,14 +263,13 @@ describe('RepoEditor', () => { ); it('installs the WebIDE extension', async () => { - const extensionSpy = jest.spyOn(SourceEditor, 'instanceApplyExtension'); await createComponent(); - expect(extensionSpy).toHaveBeenCalled(); - Reflect.ownKeys(EditorWebIdeExtension.prototype) - .filter((fn) => fn !== 'constructor') - .forEach((fn) => { - expect(vm.editor[fn]).toBe(EditorWebIdeExtension.prototype[fn]); - }); + expect(applyExtensionSpy).toHaveBeenCalled(); + const ideExtensionApi = extensionsStore.get('EditorWebIde').api; + Reflect.ownKeys(ideExtensionApi).forEach((fn) => { + expect(vm.editor[fn]).toBeDefined(); + expect(vm.editor.methods[fn]).toBe('EditorWebIde'); + }); }); it.each` @@ -301,7 +288,20 @@ describe('RepoEditor', () => { async ({ activeFile, viewer, shouldHaveMarkdownExtension } = {}) => { await createComponent({ state: { viewer }, activeFile }); - expectEditorMarkdownExtension(shouldHaveMarkdownExtension); + if (shouldHaveMarkdownExtension) { + expect(applyExtensionSpy).toHaveBeenCalledWith({ + definition: EditorMarkdownPreviewExtension, + setupOptions: { previewMarkdownPath: PREVIEW_MARKDOWN_PATH }, + }); + // TODO: spying on extensions causes Jest to blow up, so we have to assert on + // the public property the extension adds, as opposed to the args passed to the ctor + expect(wrapper.vm.editor.markdownPreview.path).toBe(PREVIEW_MARKDOWN_PATH); + } else { + expect(applyExtensionSpy).not.toHaveBeenCalledWith( + wrapper.vm.editor, + expect.any(EditorMarkdownExtension), + ); + } }, ); }); @@ -329,18 +329,6 @@ describe('RepoEditor', () => { expect(vm.model).toBe(existingModel); }); - it('adds callback methods', () => { - jest.spyOn(vm.editor, 'onPositionChange'); - jest.spyOn(vm.model, 'onChange'); - jest.spyOn(vm.model, 'updateOptions'); - - vm.setupEditor(); - - expect(vm.editor.onPositionChange).toHaveBeenCalledTimes(1); - expect(vm.model.onChange).toHaveBeenCalledTimes(1); - expect(vm.model.updateOptions).toHaveBeenCalledWith(vm.rules); - }); - it('updates state with the value of the model', () => { const newContent = 'As Gregor Samsa\n awoke one morning\n'; vm.model.setValue(newContent); @@ -366,53 +354,48 @@ describe('RepoEditor', () => { describe('editor updateDimensions', () => { let updateDimensionsSpy; - let updateDiffViewSpy; beforeEach(async () => { await createComponent(); - updateDimensionsSpy = jest.spyOn(vm.editor, 'updateDimensions'); - updateDiffViewSpy = jest.spyOn(vm.editor, 'updateDiffView').mockImplementation(); + const ext = extensionsStore.get('EditorWebIde'); + updateDimensionsSpy = jest.fn(); + spyOnApi(ext, { + updateDimensions: updateDimensionsSpy, + }); }); it('calls updateDimensions only when panelResizing is false', async () => { expect(updateDimensionsSpy).not.toHaveBeenCalled(); - expect(updateDiffViewSpy).not.toHaveBeenCalled(); expect(vm.$store.state.panelResizing).toBe(false); // default value vm.$store.state.panelResizing = true; await vm.$nextTick(); expect(updateDimensionsSpy).not.toHaveBeenCalled(); - expect(updateDiffViewSpy).not.toHaveBeenCalled(); vm.$store.state.panelResizing = false; await vm.$nextTick(); expect(updateDimensionsSpy).toHaveBeenCalledTimes(1); - expect(updateDiffViewSpy).toHaveBeenCalledTimes(1); vm.$store.state.panelResizing = true; await vm.$nextTick(); expect(updateDimensionsSpy).toHaveBeenCalledTimes(1); - expect(updateDiffViewSpy).toHaveBeenCalledTimes(1); }); it('calls updateDimensions when rightPane is toggled', async () => { expect(updateDimensionsSpy).not.toHaveBeenCalled(); - expect(updateDiffViewSpy).not.toHaveBeenCalled(); expect(vm.$store.state.rightPane.isOpen).toBe(false); // default value vm.$store.state.rightPane.isOpen = true; await vm.$nextTick(); expect(updateDimensionsSpy).toHaveBeenCalledTimes(1); - expect(updateDiffViewSpy).toHaveBeenCalledTimes(1); vm.$store.state.rightPane.isOpen = false; await vm.$nextTick(); expect(updateDimensionsSpy).toHaveBeenCalledTimes(2); - expect(updateDiffViewSpy).toHaveBeenCalledTimes(2); }); }); @@ -447,7 +430,11 @@ describe('RepoEditor', () => { activeFile: dummyFile.markdown, }); - updateDimensionsSpy = jest.spyOn(vm.editor, 'updateDimensions'); + const ext = extensionsStore.get('EditorWebIde'); + updateDimensionsSpy = jest.fn(); + spyOnApi(ext, { + updateDimensions: updateDimensionsSpy, + }); changeViewMode(FILE_VIEW_MODE_PREVIEW); await vm.$nextTick(); diff --git a/spec/frontend/import_entities/import_groups/components/import_table_spec.js b/spec/frontend/import_entities/import_groups/components/import_table_spec.js index c0ca3dd4109..607b8c24152 100644 --- a/spec/frontend/import_entities/import_groups/components/import_table_spec.js +++ b/spec/frontend/import_entities/import_groups/components/import_table_spec.js @@ -38,7 +38,7 @@ describe('import table', () => { wrapper.findAll('button').wrappers.find((w) => w.text() === 'Import selected'); const findImportButtons = () => wrapper.findAll('button').wrappers.filter((w) => w.text() === 'Import'); - const findPaginationDropdown = () => wrapper.find('[aria-label="Page size"]'); + const findPaginationDropdown = () => wrapper.find('[data-testid="page-size"]'); const findPaginationDropdownText = () => findPaginationDropdown().find('button').text(); const findSelectionCount = () => wrapper.find('[data-test-id="selection-count"]'); @@ -209,7 +209,12 @@ describe('import table', () => { const otherOption = findPaginationDropdown().findAll('li p').at(1); expect(otherOption.text()).toMatchInterpolatedText('50 items per page'); + bulkImportSourceGroupsQueryMock.mockResolvedValue({ + nodes: [FAKE_GROUP], + pageInfo: { ...FAKE_PAGE_INFO, perPage: 50 }, + }); await otherOption.trigger('click'); + await waitForPromises(); expect(findPaginationDropdownText()).toMatchInterpolatedText('50 items per page'); diff --git a/spec/frontend/pages/import/bulk_imports/history/components/bulk_imports_history_app_spec.js b/spec/frontend/pages/import/bulk_imports/history/components/bulk_imports_history_app_spec.js index d6b394a42c6..6fb03fa28fe 100644 --- a/spec/frontend/pages/import/bulk_imports/history/components/bulk_imports_history_app_spec.js +++ b/spec/frontend/pages/import/bulk_imports/history/components/bulk_imports_history_app_spec.js @@ -2,7 +2,7 @@ import { GlEmptyState, GlLoadingIcon, GlTable } from '@gitlab/ui'; import { mount, shallowMount } from '@vue/test-utils'; import MockAdapter from 'axios-mock-adapter'; import axios from '~/lib/utils/axios_utils'; -import PaginationBar from '~/import_entities/components/pagination_bar.vue'; +import PaginationBar from '~/vue_shared/components/pagination_bar/pagination_bar.vue'; import BulkImportsHistoryApp from '~/pages/import/bulk_imports/history/components/bulk_imports_history_app.vue'; import { extendedWrapper } from 'helpers/vue_test_utils_helper'; diff --git a/spec/frontend/pipeline_editor/components/editor/text_editor_spec.js b/spec/frontend/pipeline_editor/components/editor/text_editor_spec.js index a43da4b0f19..cab4810cbf1 100644 --- a/spec/frontend/pipeline_editor/components/editor/text_editor_spec.js +++ b/spec/frontend/pipeline_editor/components/editor/text_editor_spec.js @@ -1,7 +1,6 @@ import { shallowMount } from '@vue/test-utils'; import { EDITOR_READY_EVENT } from '~/editor/constants'; -import { SourceEditorExtension } from '~/editor/extensions/source_editor_extension_base'; import TextEditor from '~/pipeline_editor/components/editor/text_editor.vue'; import { mockCiConfigPath, @@ -59,10 +58,6 @@ describe('Pipeline Editor | Text editor component', () => { const findEditor = () => wrapper.findComponent(MockSourceEditor); - beforeEach(() => { - SourceEditorExtension.deferRerender = jest.fn(); - }); - afterEach(() => { wrapper.destroy(); diff --git a/spec/frontend/import_entities/components/pagination_bar_spec.js b/spec/frontend/vue_shared/components/pagination_bar/pagination_bar_spec.js index 163ce11a8db..08119dee8af 100644 --- a/spec/frontend/import_entities/components/pagination_bar_spec.js +++ b/spec/frontend/vue_shared/components/pagination_bar/pagination_bar_spec.js @@ -1,16 +1,16 @@ import { GlPagination, GlDropdown, GlDropdownItem } from '@gitlab/ui'; import { mount } from '@vue/test-utils'; -import PaginationBar from '~/import_entities/components/pagination_bar.vue'; +import PaginationBar from '~/vue_shared/components/pagination_bar/pagination_bar.vue'; import PaginationLinks from '~/vue_shared/components/pagination_links.vue'; describe('Pagination bar', () => { const DEFAULT_PROPS = { pageInfo: { total: 50, - page: 1, + totalPages: 3, + page: 3, perPage: 20, }, - itemsCount: 17, }; let wrapper; @@ -73,7 +73,7 @@ describe('Pagination bar', () => { createComponent(); expect(wrapper.find('[data-testid="information"]').text()).toMatchInterpolatedText( - 'Showing 1 - 17 of 50', + 'Showing 41 - 50 of 50', ); }); @@ -82,11 +82,12 @@ describe('Pagination bar', () => { pageInfo: { ...DEFAULT_PROPS.pageInfo, total: 1200, + page: 2, }, }); expect(wrapper.find('[data-testid="information"]').text()).toMatchInterpolatedText( - 'Showing 1 - 17 of 1000+', + 'Showing 21 - 40 of 1000+', ); }); }); diff --git a/spec/graphql/resolvers/users/participants_resolver_spec.rb b/spec/graphql/resolvers/users/participants_resolver_spec.rb index c857eb9f63d..77e3b6d5912 100644 --- a/spec/graphql/resolvers/users/participants_resolver_spec.rb +++ b/spec/graphql/resolvers/users/participants_resolver_spec.rb @@ -49,6 +49,19 @@ RSpec.describe Resolvers::Users::ParticipantsResolver do it 'returns all participants for this user' do is_expected.to match_array([issue.author, note.author]) end + + it 'does not execute N+1 for project relation' do + query = -> { resolve(described_class, args: {}, ctx: { current_user: current_user }, obj: issue)&.items } + + # warm-up + query.call + + control_count = ActiveRecord::QueryRecorder.new { query.call } + + create(:note, :confidential, project: project, noteable: issue, author: create(:user)) + + expect { query.call }.not_to exceed_query_limit(control_count) + end end end end diff --git a/spec/helpers/avatars_helper_spec.rb b/spec/helpers/avatars_helper_spec.rb index 7190f2fcd4a..f1d9e13c076 100644 --- a/spec/helpers/avatars_helper_spec.rb +++ b/spec/helpers/avatars_helper_spec.rb @@ -4,6 +4,7 @@ require 'spec_helper' RSpec.describe AvatarsHelper do include UploadHelpers + include Devise::Test::ControllerHelpers let_it_be(:user) { create(:user) } @@ -145,12 +146,49 @@ RSpec.describe AvatarsHelper do describe '#avatar_icon_for_user' do let(:user) { create(:user, avatar: File.open(uploaded_image_temp_path)) } + let(:helper_args) { [user] } + + shared_examples 'blocked or unconfirmed user with avatar' do + it 'returns the default avatar' do + expect(helper.avatar_icon_for_user(user).to_s) + .to match_asset_path(described_class::DEFAULT_AVATAR_PATH) + end + + context 'when the current user is an admin', :enable_admin_mode do + let(:current_user) { create(:user, :admin) } + + before do + allow(helper).to receive(:current_user).and_return(current_user) + end + + it 'returns the user avatar' do + expect(helper.avatar_icon_for_user(user).to_s) + .to eq(user.avatar.url) + end + end + end context 'with a user object passed' do it 'returns a relative URL for the avatar' do expect(helper.avatar_icon_for_user(user).to_s) .to eq(user.avatar.url) end + + context 'when the user is blocked' do + before do + user.block! + end + + it_behaves_like 'blocked or unconfirmed user with avatar' + end + + context 'when the user is unconfirmed' do + before do + user.update!(confirmed_at: nil) + end + + it_behaves_like 'blocked or unconfirmed user with avatar' + end end context 'without a user object passed' do @@ -171,7 +209,7 @@ RSpec.describe AvatarsHelper do end it 'returns a generic avatar' do - expect(helper.gravatar_icon(user_email)).to match_asset_path('no_avatar.png') + expect(helper.gravatar_icon(user_email)).to match_asset_path(described_class::DEFAULT_AVATAR_PATH) end end @@ -181,7 +219,7 @@ RSpec.describe AvatarsHelper do end it 'returns a generic avatar when email is blank' do - expect(helper.gravatar_icon('')).to match_asset_path('no_avatar.png') + expect(helper.gravatar_icon('')).to match_asset_path(described_class::DEFAULT_AVATAR_PATH) end it 'returns a valid Gravatar URL' do diff --git a/spec/lib/gitlab/security/scan_configuration_spec.rb b/spec/lib/gitlab/security/scan_configuration_spec.rb new file mode 100644 index 00000000000..0af029968e8 --- /dev/null +++ b/spec/lib/gitlab/security/scan_configuration_spec.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ::Gitlab::Security::ScanConfiguration do + let_it_be(:project) { create(:project, :repository) } + + let(:scan) { described_class.new(project: project, type: type, configured: configured) } + + describe '#available?' do + subject { scan.available? } + + let(:configured) { true } + + context 'with a core scanner' do + let(:type) { :sast } + + it { is_expected.to be_truthy } + end + + context 'with custom scanner' do + let(:type) { :my_scanner } + + it { is_expected.to be_falsey } + end + end + + describe '#configured?' do + subject { scan.configured? } + + let(:type) { :sast } + let(:configured) { false } + + it { is_expected.to be_falsey } + end + + describe '#configuration_path' do + subject { scan.configuration_path } + + let(:configured) { true } + + context 'with a non configurable scanner' do + let(:type) { :secret_detection } + + it { is_expected.to be_nil } + end + + context 'with licensed scanner for FOSS environment' do + let(:type) { :dast } + + before do + stub_env('FOSS_ONLY', '1') + end + + it { is_expected.to be_nil } + end + + context 'with custom scanner' do + let(:type) { :my_scanner } + + it { is_expected.to be_nil } + end + end +end diff --git a/spec/presenters/projects/security/configuration_presenter_spec.rb b/spec/presenters/projects/security/configuration_presenter_spec.rb new file mode 100644 index 00000000000..836753d0483 --- /dev/null +++ b/spec/presenters/projects/security/configuration_presenter_spec.rb @@ -0,0 +1,301 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Projects::Security::ConfigurationPresenter do + include Gitlab::Routing.url_helpers + using RSpec::Parameterized::TableSyntax + + let(:project_with_repo) { create(:project, :repository) } + let(:project_with_no_repo) { create(:project) } + let(:current_user) { create(:user) } + let(:presenter) { described_class.new(project, current_user: current_user) } + + before do + stub_licensed_features(licensed_scan_types.to_h { |type| [type, true] }) + + stub_feature_flags(corpus_management: false) + end + + describe '#to_html_data_attribute' do + subject(:html_data) { presenter.to_html_data_attribute } + + context 'when latest default branch pipeline`s source is not auto devops' do + let(:project) { project_with_repo } + + let(:pipeline) do + create( + :ci_pipeline, + project: project, + ref: project.default_branch, + sha: project.commit.sha + ) + end + + let!(:build_sast) { create(:ci_build, :sast, pipeline: pipeline) } + let!(:build_dast) { create(:ci_build, :dast, pipeline: pipeline) } + let!(:build_license_scanning) { create(:ci_build, :license_scanning, pipeline: pipeline) } + + it 'includes links to auto devops and secure product docs' do + expect(html_data[:auto_devops_help_page_path]).to eq(help_page_path('topics/autodevops/index')) + expect(html_data[:help_page_path]).to eq(help_page_path('user/application_security/index')) + end + + it 'returns info that Auto DevOps is not enabled' do + expect(html_data[:auto_devops_enabled]).to eq(false) + expect(html_data[:auto_devops_path]).to eq(project_settings_ci_cd_path(project, anchor: 'autodevops-settings')) + end + + it 'includes a link to the latest pipeline' do + expect(html_data[:latest_pipeline_path]).to eq(project_pipeline_path(project, pipeline)) + end + + it 'has stubs for autofix' do + expect(html_data.keys).to include(:can_toggle_auto_fix_settings, :auto_fix_enabled, :auto_fix_user_path) + end + + context "while retrieving information about user's ability to enable auto_devops" do + where(:is_admin, :archived, :feature_available, :result) do + true | true | true | false + false | true | true | false + true | false | true | true + false | false | true | false + true | true | false | false + false | true | false | false + true | false | false | false + false | false | false | false + end + + with_them do + before do + allow_next_instance_of(described_class) do |presenter| + allow(presenter).to receive(:can?).and_return(is_admin) + allow(presenter).to receive(:archived?).and_return(archived) + allow(presenter).to receive(:feature_available?).and_return(feature_available) + end + end + + it 'includes can_enable_auto_devops' do + expect(html_data[:can_enable_auto_devops]).to eq(result) + end + end + end + + it 'includes feature information' do + feature = Gitlab::Json.parse(html_data[:features]).find { |scan| scan['type'] == 'sast' } + + expect(feature['type']).to eq('sast') + expect(feature['configured']).to eq(true) + expect(feature['configuration_path']).to eq(project_security_configuration_sast_path(project)) + expect(feature['available']).to eq(true) + end + + context 'when checking features configured status' do + let(:features) { Gitlab::Json.parse(html_data[:features]) } + + where(:type, :configured) do + :dast | true + :dast_profiles | true + :sast | true + :sast_iac | false + :container_scanning | false + :cluster_image_scanning | false + :dependency_scanning | false + :license_scanning | true + :secret_detection | false + :coverage_fuzzing | false + :api_fuzzing | false + :corpus_management | true + end + + with_them do + it 'returns proper configuration status' do + feature = features.find { |scan| scan['type'] == type.to_s } + + expect(feature['configured']).to eq(configured) + end + end + end + + context 'when the job has more than one report' do + let(:features) { Gitlab::Json.parse(html_data[:features]) } + + let!(:artifacts) do + { artifacts: { reports: { other_job: ['gl-other-report.json'], sast: ['gl-sast-report.json'] } } } + end + + let!(:complicated_job) { build_stubbed(:ci_build, options: artifacts) } + + before do + allow_next_instance_of(::Security::SecurityJobsFinder) do |finder| + allow(finder).to receive(:execute).and_return([complicated_job]) + end + end + + where(:type, :configured) do + :dast | false + :dast_profiles | true + :sast | true + :sast_iac | false + :container_scanning | false + :cluster_image_scanning | false + :dependency_scanning | false + :license_scanning | true + :secret_detection | false + :coverage_fuzzing | false + :api_fuzzing | false + :corpus_management | true + end + + with_them do + it 'properly detects security jobs' do + feature = features.find { |scan| scan['type'] == type.to_s } + + expect(feature['configured']).to eq(configured) + end + end + end + + it 'includes a link to the latest pipeline' do + expect(subject[:latest_pipeline_path]).to eq(project_pipeline_path(project, pipeline)) + end + + context "while retrieving information about gitlab ci file" do + context 'when a .gitlab-ci.yml file exists' do + let!(:ci_config) do + project.repository.create_file( + project.creator, + Gitlab::FileDetector::PATTERNS[:gitlab_ci], + 'contents go here', + message: 'test', + branch_name: 'master') + end + + it 'expects gitlab_ci_present to be true' do + expect(html_data[:gitlab_ci_present]).to eq(true) + end + end + + context 'when a .gitlab-ci.yml file does not exist' do + it 'expects gitlab_ci_present to be false if the file is not present' do + expect(html_data[:gitlab_ci_present]).to eq(false) + end + end + end + + it 'includes the path to gitlab_ci history' do + expect(subject[:gitlab_ci_history_path]).to eq(project_blame_path(project, 'master/.gitlab-ci.yml')) + end + end + + context 'when the project is empty' do + let(:project) { project_with_no_repo } + + it 'includes a blank gitlab_ci history path' do + expect(html_data[:gitlab_ci_history_path]).to eq('') + end + end + + context 'when the project has no default branch set' do + let(:project) { project_with_repo } + + it 'includes the path to gitlab_ci history' do + allow(project).to receive(:default_branch).and_return(nil) + + expect(html_data[:gitlab_ci_history_path]).to eq(project_blame_path(project, 'master/.gitlab-ci.yml')) + end + end + + context "when the latest default branch pipeline's source is auto devops" do + let(:project) { project_with_repo } + + let(:pipeline) do + create( + :ci_pipeline, + :auto_devops_source, + project: project, + ref: project.default_branch, + sha: project.commit.sha + ) + end + + let!(:build_sast) { create(:ci_build, :sast, pipeline: pipeline, status: 'success') } + let!(:build_dast) { create(:ci_build, :dast, pipeline: pipeline, status: 'success') } + let!(:ci_build) { create(:ci_build, :secret_detection, pipeline: pipeline, status: 'pending') } + + it 'reports that auto devops is enabled' do + expect(html_data[:auto_devops_enabled]).to be_truthy + end + + context 'when gathering feature data' do + let(:features) { Gitlab::Json.parse(html_data[:features]) } + + where(:type, :configured) do + :dast | true + :dast_profiles | true + :sast | true + :sast_iac | false + :container_scanning | false + :cluster_image_scanning | false + :dependency_scanning | false + :license_scanning | false + :secret_detection | true + :coverage_fuzzing | false + :api_fuzzing | false + :corpus_management | true + end + + with_them do + it 'reports that all scanners are configured for which latest pipeline has builds' do + feature = features.find { |scan| scan['type'] == type.to_s } + + expect(feature['configured']).to eq(configured) + end + end + end + end + + context 'when the project has no default branch pipeline' do + let(:project) { project_with_repo } + + it 'reports that auto devops is disabled' do + expect(html_data[:auto_devops_enabled]).to be_falsy + end + + it 'includes a link to CI pipeline docs' do + expect(html_data[:latest_pipeline_path]).to eq(help_page_path('ci/pipelines')) + end + + context 'when gathering feature data' do + let(:features) { Gitlab::Json.parse(html_data[:features]) } + + where(:type, :configured) do + :dast | false + :dast_profiles | true + :sast | false + :sast_iac | false + :container_scanning | false + :cluster_image_scanning | false + :dependency_scanning | false + :license_scanning | false + :secret_detection | false + :coverage_fuzzing | false + :api_fuzzing | false + :corpus_management | true + end + + with_them do + it 'reports all security jobs as unconfigured with exception of "fake" jobs' do + feature = features.find { |scan| scan['type'] == type.to_s } + + expect(feature['configured']).to eq(configured) + end + end + end + end + + def licensed_scan_types + ::Security::SecurityJobsFinder.allowed_job_types + ::Security::LicenseComplianceJobsFinder.allowed_job_types - [:cluster_image_scanning] + end + end +end diff --git a/spec/rubocop/cop/qa/testcase_link_format_spec.rb b/spec/rubocop/cop/qa/testcase_link_format_spec.rb new file mode 100644 index 00000000000..f9b43f2a293 --- /dev/null +++ b/spec/rubocop/cop/qa/testcase_link_format_spec.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' + +require_relative '../../../../rubocop/cop/qa/testcase_link_format' + +RSpec.describe RuboCop::Cop::QA::TestcaseLinkFormat do + let(:source_file) { 'qa/page.rb' } + let(:msg) { 'Testcase link format incorrect. Please link a test case from the GitLab project. See: https://docs.gitlab.com/ee/development/testing_guide/end_to_end/best_practices.html#link-a-test-to-its-test-case.' } + + subject(:cop) { described_class.new } + + context 'in a QA file' do + before do + allow(cop).to receive(:in_qa_file?).and_return(true) + end + + it "registers an offense for a testcase link for an issue" do + node = "it 'another test', testcase: 'https://gitlab.com/gitlab-org/gitlab/-/issues/557' do" + + expect_offense(<<-RUBY, node: node, msg: msg) + %{node} + ^{node} %{msg} + end + RUBY + end + + it "registers an offense for a testcase link for the wrong project" do + node = "it 'another test', testcase: 'https://gitlab.com/gitlab-org/quality/testcases/-/quality/test_cases/2455' do" + + expect_offense(<<-RUBY, node: node, msg: msg) + %{node} + ^{node} %{msg} + end + RUBY + end + + it "doesnt offend if testcase link is correct" do + expect_no_offenses(<<-RUBY) + it 'some test', testcase: 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/348312' do + end + RUBY + end + end +end diff --git a/spec/support/shared_contexts/policies/group_policy_shared_context.rb b/spec/support/shared_contexts/policies/group_policy_shared_context.rb index b432aa24bb8..ad6462dc367 100644 --- a/spec/support/shared_contexts/policies/group_policy_shared_context.rb +++ b/spec/support/shared_contexts/policies/group_policy_shared_context.rb @@ -48,6 +48,7 @@ RSpec.shared_context 'GroupPolicy context' do destroy_package create_projects read_cluster create_cluster update_cluster admin_cluster add_cluster + admin_group_runners ] end diff --git a/spec/tooling/danger/product_intelligence_spec.rb b/spec/tooling/danger/product_intelligence_spec.rb index c090dbb4de4..d0d4b8d4df4 100644 --- a/spec/tooling/danger/product_intelligence_spec.rb +++ b/spec/tooling/danger/product_intelligence_spec.rb @@ -65,13 +65,25 @@ RSpec.describe Tooling::Danger::ProductIntelligence do it { is_expected.to be_empty } end end + end + + describe '#skip_review' do + subject { product_intelligence.skip_review? } context 'with growth experiment label' do before do allow(fake_helper).to receive(:mr_has_labels?).with('growth experiment').and_return(true) end - it { is_expected.to be_empty } + it { is_expected.to be true } + end + + context 'without growth experiment label' do + before do + allow(fake_helper).to receive(:mr_has_labels?).with('growth experiment').and_return(false) + end + + it { is_expected.to be false } end end end diff --git a/tooling/danger/product_intelligence.rb b/tooling/danger/product_intelligence.rb index 0c8fafac5e7..6185b2f0d08 100644 --- a/tooling/danger/product_intelligence.rb +++ b/tooling/danger/product_intelligence.rb @@ -13,7 +13,7 @@ module Tooling ].freeze def missing_labels - return [] if !helper.ci? || helper.mr_has_labels?('growth experiment') + return [] unless helper.ci? labels = [] labels << 'product intelligence' unless helper.mr_has_labels?('product intelligence') @@ -26,6 +26,10 @@ module Tooling helper.mr_labels.include?(APPROVED_LABEL) end + def skip_review? + helper.mr_has_labels?('growth experiment') + end + private def has_workflow_labels? |