diff options
118 files changed, 926 insertions, 1084 deletions
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index ee9eaeae723..b93a79de994 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -386,20 +386,25 @@ flaky-examples-check: - scripts/merge-reports ${NEW_FLAKY_SPECS_REPORT} rspec_flaky/new_*_*.json - scripts/detect-new-flaky-examples $NEW_FLAKY_SPECS_REPORT +.assets-compile-cache: &assets-compile-cache + cache: + key: "assets-compile:vendor_ruby:.yarn-cache:tmp_cache_assets_sprockets:v3" + paths: + - vendor/ruby/ + - .yarn-cache/ + - tmp/cache/assets/sprockets + compile-assets: <<: *dedicated-runner <<: *except-docs <<: *use-pg stage: prepare - cache: - <<: *default-cache script: - node --version - - date - yarn install --frozen-lockfile --cache-folder .yarn-cache - - date - free -m - bundle exec rake gitlab:assets:compile + - scripts/clean-old-cached-assets variables: # we override the max_old_space_size to prevent OOM errors NODE_OPTIONS: --max_old_space_size=3584 @@ -408,6 +413,7 @@ compile-assets: paths: - node_modules - public/assets + <<: *assets-compile-cache setup-test-env: <<: *dedicated-runner @@ -628,7 +634,9 @@ gitlab:setup-mysql: gitlab:assets:compile: <<: *dedicated-no-docs-pull-cache-job image: dev.gitlab.org:5005/gitlab/gitlab-build-images:ruby-2.5.3-git-2.18-chrome-71.0-node-8.x-yarn-1.12-graphicsmagick-1.3.29-docker-18.06.1 - dependencies: [] + dependencies: + - setup-test-env + - compile-assets services: - docker:stable-dind variables: @@ -642,18 +650,19 @@ gitlab:assets:compile: DOCKER_DRIVER: overlay2 DOCKER_HOST: tcp://docker:2375 script: - - date + - node --version - yarn install --frozen-lockfile --production --cache-folder .yarn-cache - - date - free -m - bundle exec rake gitlab:assets:compile - - scripts/build_assets_image + - time scripts/build_assets_image + - scripts/clean-old-cached-assets artifacts: name: webpack-report expire_in: 31d paths: - webpack-report/ - public/assets/ + <<: *assets-compile-cache only: - //@gitlab-org/gitlab-ce - //@gitlab-org/gitlab-ee diff --git a/.gitlab/merge_request_templates/Security Release.md b/.gitlab/merge_request_templates/Security Release.md index d72b4eb1cb6..9a0979f27a7 100644 --- a/.gitlab/merge_request_templates/Security Release.md +++ b/.gitlab/merge_request_templates/Security Release.md @@ -9,7 +9,7 @@ See [the general developer security release guidelines](https://gitlab.com/gitla <!-- Mention the issue(s) this MR is related to --> -## Author's checklist +## Developer checklist - [ ] Link to the developer security workflow issue on `dev.gitlab.org` - [ ] MR targets `master` or `security-X-Y` for backports @@ -20,7 +20,7 @@ See [the general developer security release guidelines](https://gitlab.com/gitla - [ ] Add a link to an EE MR if required - [ ] Assign to a reviewer -## Reviewers checklist +## Reviewer checklist - [ ] Correct milestone is applied and the title is matching across all backports - [ ] Assigned to `@gitlab-release-tools-bot` with passing CI pipelines @@ -116,7 +116,6 @@ gem 'html-pipeline', '~> 2.8' gem 'deckar01-task_list', '2.2.0' gem 'gitlab-markup', '~> 1.6.5' gem 'github-markup', '~> 1.7.0', require: 'github/markup' -gem 'redcarpet', '~> 3.4' gem 'commonmarker', '~> 0.17' gem 'RedCloth', '~> 4.3.2' gem 'rdoc', '~> 6.0' diff --git a/Gemfile.lock b/Gemfile.lock index e6b563b5cb8..a9244fd853c 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -682,7 +682,6 @@ GEM recaptcha (3.0.0) json recursive-open-struct (1.1.0) - redcarpet (3.4.0) redis (3.3.5) redis-actionpack (5.0.2) actionpack (>= 4.0, < 6) @@ -1118,7 +1117,6 @@ DEPENDENCIES rdoc (~> 6.0) re2 (~> 1.1.1) recaptcha (~> 3.0) - redcarpet (~> 3.4) redis (~> 3.2) redis-namespace (~> 1.6.0) redis-rails (~> 5.0.2) diff --git a/app/assets/javascripts/behaviors/preview_markdown.js b/app/assets/javascripts/behaviors/preview_markdown.js index 35f1bb6b080..7adccbb062f 100644 --- a/app/assets/javascripts/behaviors/preview_markdown.js +++ b/app/assets/javascripts/behaviors/preview_markdown.js @@ -28,16 +28,13 @@ MarkdownPreview.prototype.ajaxCache = {}; MarkdownPreview.prototype.showPreview = function($form) { var mdText; - var markdownVersion; - var url; var preview = $form.find('.js-md-preview'); + var url = preview.data('url'); if (preview.hasClass('md-preview-loading')) { return; } mdText = $form.find('textarea.markdown-area').val(); - markdownVersion = $form.attr('data-markdown-version'); - url = this.versionedPreviewPath(preview.data('url'), markdownVersion); if (mdText.trim().length === 0) { preview.text(this.emptyMessage); @@ -67,16 +64,6 @@ MarkdownPreview.prototype.showPreview = function($form) { } }; -MarkdownPreview.prototype.versionedPreviewPath = function(markdownPreviewPath, markdownVersion) { - if (typeof markdownVersion === 'undefined') { - return markdownPreviewPath; - } - - return `${markdownPreviewPath}${ - markdownPreviewPath.indexOf('?') === -1 ? '?' : '&' - }markdown_version=${markdownVersion}`; -}; - MarkdownPreview.prototype.fetchMarkdownPreview = function(text, url, success) { if (!url) { return; diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue index d4c1b07093d..f0ce2579ee7 100644 --- a/app/assets/javascripts/diffs/components/app.vue +++ b/app/assets/javascripts/diffs/components/app.vue @@ -129,6 +129,10 @@ export default { created() { this.adjustView(); eventHub.$once('fetchedNotesData', this.setDiscussions); + eventHub.$once('fetchDiffData', this.fetchData); + }, + beforeDestroy() { + eventHub.$off('fetchDiffData', this.fetchData); }, methods: { ...mapActions(['startTaskList']), diff --git a/app/assets/javascripts/diffs/components/tree_list.vue b/app/assets/javascripts/diffs/components/tree_list.vue index 0b3def3d29d..a0f09932593 100644 --- a/app/assets/javascripts/diffs/components/tree_list.vue +++ b/app/assets/javascripts/diffs/components/tree_list.vue @@ -13,39 +13,17 @@ export default { Icon, FileRow, }, - data() { - return { - search: '', - }; - }, computed: { ...mapState('diffs', ['tree', 'addedLines', 'removedLines', 'renderTreeList']), ...mapGetters('diffs', ['allBlobs', 'diffFilesLength']), filteredTreeList() { - const search = this.search.toLowerCase().trim(); - - if (search === '') return this.renderTreeList ? this.tree : this.allBlobs; - - return this.allBlobs.reduce((acc, folder) => { - const tree = folder.tree.filter(f => f.path.toLowerCase().indexOf(search) >= 0); - - if (tree.length) { - return acc.concat({ - ...folder, - tree, - }); - } - - return acc; - }, []); + return this.renderTreeList ? this.tree : this.allBlobs; }, }, methods: { - ...mapActions('diffs', ['toggleTreeOpen', 'scrollToFile']), - clearSearch() { - this.search = ''; - }, + ...mapActions('diffs', ['toggleTreeOpen', 'scrollToFile', 'toggleFileFinder']), }, + shortcutKeyCharacter: `${/Mac/i.test(navigator.userAgent) ? '⌘' : 'Ctrl'}+P`, FileRowStats, }; </script> @@ -55,21 +33,17 @@ export default { <div class="append-bottom-8 position-relative tree-list-search d-flex"> <div class="flex-fill d-flex"> <icon name="search" class="position-absolute tree-list-icon" /> - <input - v-model="search" - :placeholder="s__('MergeRequest|Filter files')" - type="search" - class="form-control" - /> <button - v-show="search" - :aria-label="__('Clear search')" type="button" - class="position-absolute bg-transparent tree-list-icon tree-list-clear-icon border-0 p-0" - @click="clearSearch" + class="form-control text-left text-secondary" + @click="toggleFileFinder(true)" > - <icon name="close" /> + {{ s__('MergeRequest|Search files') }} </button> + <span + class="position-absolute text-secondary diff-tree-search-shortcut" + v-html="$options.shortcutKeyCharacter" + ></span> </div> </div> <div :class="{ 'pt-0 tree-list-blobs': !renderTreeList }" class="tree-list-scroll"> @@ -104,4 +78,15 @@ export default { .tree-list-blobs .file-row-name { margin-left: 12px; } + +.diff-tree-search-shortcut { + top: 50%; + right: 10px; + transform: translateY(-50%); + pointer-events: none; +} + +.tree-list-icon { + pointer-events: none; +} </style> diff --git a/app/assets/javascripts/diffs/index.js b/app/assets/javascripts/diffs/index.js index 094e5cdea9c..63954d9d412 100644 --- a/app/assets/javascripts/diffs/index.js +++ b/app/assets/javascripts/diffs/index.js @@ -1,11 +1,60 @@ import Vue from 'vue'; -import { mapActions, mapState } from 'vuex'; +import { mapActions, mapState, mapGetters } from 'vuex'; import { parseBoolean } from '~/lib/utils/common_utils'; import { getParameterValues } from '~/lib/utils/url_utility'; +import FindFile from '~/vue_shared/components/file_finder/index.vue'; +import eventHub from '../notes/event_hub'; import diffsApp from './components/app.vue'; import { TREE_LIST_STORAGE_KEY } from './constants'; export default function initDiffsApp(store) { + const fileFinderEl = document.getElementById('js-diff-file-finder'); + + if (fileFinderEl) { + // eslint-disable-next-line no-new + new Vue({ + el: fileFinderEl, + store, + computed: { + ...mapState('diffs', ['fileFinderVisible', 'isLoading']), + ...mapGetters('diffs', ['flatBlobsList']), + }, + watch: { + fileFinderVisible(newVal, oldVal) { + if (newVal && !oldVal && !this.flatBlobsList.length) { + eventHub.$emit('fetchDiffData'); + } + }, + }, + methods: { + ...mapActions('diffs', ['toggleFileFinder', 'scrollToFile']), + openFile(file) { + window.mrTabs.tabShown('diffs'); + this.scrollToFile(file.path); + }, + }, + render(createElement) { + return createElement(FindFile, { + props: { + files: this.flatBlobsList, + visible: this.fileFinderVisible, + loading: this.isLoading, + showDiffStats: true, + clearSearchOnClose: false, + }, + on: { + toggle: this.toggleFileFinder, + click: this.openFile, + }, + class: ['diff-file-finder'], + style: { + display: this.fileFinderVisible ? '' : 'none', + }, + }); + }, + }); + } + return new Vue({ el: '#js-diffs-app', name: 'MergeRequestDiffs', diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js index 2c5019fb652..7fb66ce433b 100644 --- a/app/assets/javascripts/diffs/store/actions.js +++ b/app/assets/javascripts/diffs/store/actions.js @@ -296,5 +296,9 @@ export const setShowWhitespace = ({ commit }, { showWhitespace, pushState = fals } }; +export const toggleFileFinder = ({ commit }, visible) => { + commit(types.TOGGLE_FILE_FINDER_VISIBLE, visible); +}; + // prevent babel-plugin-rewire from generating an invalid default during karma tests export default () => {}; diff --git a/app/assets/javascripts/diffs/store/getters.js b/app/assets/javascripts/diffs/store/getters.js index 86c0c7190f9..0e1ad654a2b 100644 --- a/app/assets/javascripts/diffs/store/getters.js +++ b/app/assets/javascripts/diffs/store/getters.js @@ -74,24 +74,25 @@ export const getDiffFileDiscussions = (state, getters, rootState, rootGetters) = export const getDiffFileByHash = state => fileHash => state.diffFiles.find(file => file.file_hash === fileHash); -export const allBlobs = state => - Object.values(state.treeEntries) - .filter(f => f.type === 'blob') - .reduce((acc, file) => { - const { parentPath } = file; - - if (parentPath && !acc.some(f => f.path === parentPath)) { - acc.push({ - path: parentPath, - isHeader: true, - tree: [], - }); - } - - acc.find(f => f.path === parentPath).tree.push(file); - - return acc; - }, []); +export const flatBlobsList = state => + Object.values(state.treeEntries).filter(f => f.type === 'blob'); + +export const allBlobs = (state, getters) => + getters.flatBlobsList.reduce((acc, file) => { + const { parentPath } = file; + + if (parentPath && !acc.some(f => f.path === parentPath)) { + acc.push({ + path: parentPath, + isHeader: true, + tree: [], + }); + } + + acc.find(f => f.path === parentPath).tree.push(file); + + return acc; + }, []); export const diffFilesLength = state => state.diffFiles.length; diff --git a/app/assets/javascripts/diffs/store/modules/diff_state.js b/app/assets/javascripts/diffs/store/modules/diff_state.js index 05b4c552f6e..6ee33d9fc6d 100644 --- a/app/assets/javascripts/diffs/store/modules/diff_state.js +++ b/app/assets/javascripts/diffs/store/modules/diff_state.js @@ -29,4 +29,5 @@ export default () => ({ highlightedRow: null, renderTreeList: true, showWhitespace: true, + fileFinderVisible: false, }); diff --git a/app/assets/javascripts/diffs/store/mutation_types.js b/app/assets/javascripts/diffs/store/mutation_types.js index e760b4d1079..71ad108ce88 100644 --- a/app/assets/javascripts/diffs/store/mutation_types.js +++ b/app/assets/javascripts/diffs/store/mutation_types.js @@ -22,3 +22,4 @@ export const SET_HIGHLIGHTED_ROW = 'SET_HIGHLIGHTED_ROW'; export const SET_TREE_DATA = 'SET_TREE_DATA'; export const SET_RENDER_TREE_LIST = 'SET_RENDER_TREE_LIST'; export const SET_SHOW_WHITESPACE = 'SET_SHOW_WHITESPACE'; +export const TOGGLE_FILE_FINDER_VISIBLE = 'TOGGLE_FILE_FINDER_VISIBLE'; diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js index 4aeb393b29b..7bbafe66199 100644 --- a/app/assets/javascripts/diffs/store/mutations.js +++ b/app/assets/javascripts/diffs/store/mutations.js @@ -244,4 +244,7 @@ export default { [types.SET_SHOW_WHITESPACE](state, showWhitespace) { state.showWhitespace = showWhitespace; }, + [types.TOGGLE_FILE_FINDER_VISIBLE](state, visible) { + state.fileFinderVisible = visible; + }, }; diff --git a/app/assets/javascripts/ide/components/ide.vue b/app/assets/javascripts/ide/components/ide.vue index caec8779cac..9894ebb0624 100644 --- a/app/assets/javascripts/ide/components/ide.vue +++ b/app/assets/javascripts/ide/components/ide.vue @@ -1,20 +1,17 @@ <script> import Vue from 'vue'; -import Mousetrap from 'mousetrap'; import { mapActions, mapState, mapGetters } from 'vuex'; import { __ } from '~/locale'; +import FindFile from '~/vue_shared/components/file_finder/index.vue'; import NewModal from './new_dropdown/modal.vue'; import IdeSidebar from './ide_side_bar.vue'; import RepoTabs from './repo_tabs.vue'; import IdeStatusBar from './ide_status_bar.vue'; import RepoEditor from './repo_editor.vue'; -import FindFile from './file_finder/index.vue'; import RightPane from './panes/right.vue'; import ErrorMessage from './error_message.vue'; import CommitEditorHeader from './commit_sidebar/editor_header.vue'; -const originalStopCallback = Mousetrap.stopCallback; - export default { components: { NewModal, @@ -42,21 +39,18 @@ export default { 'emptyStateSvgPath', 'currentProjectId', 'errorMessage', + 'loading', + ]), + ...mapGetters([ + 'activeFile', + 'hasChanges', + 'someUncommittedChanges', + 'isCommitModeActive', + 'allBlobs', ]), - ...mapGetters(['activeFile', 'hasChanges', 'someUncommittedChanges', 'isCommitModeActive']), }, mounted() { window.onbeforeunload = e => this.onBeforeUnload(e); - - Mousetrap.bind(['t', 'command+p', 'ctrl+p'], e => { - if (e.preventDefault) { - e.preventDefault(); - } - - this.toggleFileFinder(!this.fileFindVisible); - }); - - Mousetrap.stopCallback = (e, el, combo) => this.mousetrapStopCallback(e, el, combo); }, methods: { ...mapActions(['toggleFileFinder']), @@ -70,17 +64,8 @@ export default { }); return returnValue; }, - mousetrapStopCallback(e, el, combo) { - if ( - (combo === 't' && el.classList.contains('dropdown-input-field')) || - el.classList.contains('inputarea') - ) { - return true; - } else if (combo === 'command+p' || combo === 'ctrl+p') { - return false; - } - - return originalStopCallback(e, el, combo); + openFile(file) { + this.$router.push(`/project${file.url}`); }, }, }; @@ -90,7 +75,14 @@ export default { <article class="ide position-relative d-flex flex-column align-items-stretch"> <error-message v-if="errorMessage" :message="errorMessage" /> <div class="ide-view flex-grow d-flex"> - <find-file v-show="fileFindVisible" /> + <find-file + v-show="fileFindVisible" + :files="allBlobs" + :visible="fileFindVisible" + :loading="loading" + @toggle="toggleFileFinder" + @click="openFile" + /> <ide-sidebar /> <div class="multi-file-edit-pane"> <template v-if="activeFile"> diff --git a/app/assets/javascripts/ide/constants.js b/app/assets/javascripts/ide/constants.js index 09245ed0296..804ebae4555 100644 --- a/app/assets/javascripts/ide/constants.js +++ b/app/assets/javascripts/ide/constants.js @@ -1,8 +1,3 @@ -// Fuzzy file finder -export const MAX_FILE_FINDER_RESULTS = 40; -export const FILE_FINDER_ROW_HEIGHT = 55; -export const FILE_FINDER_EMPTY_ROW_HEIGHT = 33; - export const MAX_WINDOW_HEIGHT_COMPACT = 750; // Commit message textarea diff --git a/app/assets/javascripts/issue_show/components/app.vue b/app/assets/javascripts/issue_show/components/app.vue index fea7f0d77a5..bd757a76ee7 100644 --- a/app/assets/javascripts/issue_show/components/app.vue +++ b/app/assets/javascripts/issue_show/components/app.vue @@ -108,11 +108,6 @@ export default { type: String, required: true, }, - markdownVersion: { - type: Number, - required: false, - default: 0, - }, projectPath: { type: String, required: true, @@ -313,7 +308,6 @@ export default { :issuable-templates="issuableTemplates" :markdown-docs-path="markdownDocsPath" :markdown-preview-path="markdownPreviewPath" - :markdown-version="markdownVersion" :project-path="projectPath" :project-namespace="projectNamespace" :show-delete-button="showDeleteButton" diff --git a/app/assets/javascripts/issue_show/components/fields/description.vue b/app/assets/javascripts/issue_show/components/fields/description.vue index 90258c0e044..299130e56ae 100644 --- a/app/assets/javascripts/issue_show/components/fields/description.vue +++ b/app/assets/javascripts/issue_show/components/fields/description.vue @@ -20,11 +20,6 @@ export default { type: String, required: true, }, - markdownVersion: { - type: Number, - required: false, - default: 0, - }, canAttachFile: { type: Boolean, required: false, @@ -48,7 +43,6 @@ export default { <markdown-field :markdown-preview-path="markdownPreviewPath" :markdown-docs-path="markdownDocsPath" - :markdown-version="markdownVersion" :can-attach-file="canAttachFile" :enable-autocomplete="enableAutocomplete" > diff --git a/app/assets/javascripts/issue_show/components/form.vue b/app/assets/javascripts/issue_show/components/form.vue index 45b60bc3392..eade31f1d14 100644 --- a/app/assets/javascripts/issue_show/components/form.vue +++ b/app/assets/javascripts/issue_show/components/form.vue @@ -39,11 +39,6 @@ export default { type: String, required: true, }, - markdownVersion: { - type: Number, - required: false, - default: 0, - }, projectPath: { type: String, required: true, @@ -101,7 +96,6 @@ export default { :form-state="formState" :markdown-preview-path="markdownPreviewPath" :markdown-docs-path="markdownDocsPath" - :markdown-version="markdownVersion" :can-attach-file="canAttachFile" :enable-autocomplete="enableAutocomplete" /> diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index c3443c300e3..c9c01354333 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -1239,15 +1239,13 @@ export default class Notes { var postUrl = $originalContentEl.data('postUrl'); var targetId = $originalContentEl.data('targetId'); var targetType = $originalContentEl.data('targetType'); - var markdownVersion = $originalContentEl.data('markdownVersion'); this.glForm = new GLForm($editForm.find('form'), this.enableGFM); $editForm .find('form') .attr('action', `${postUrl}?html=true`) - .attr('data-remote', 'true') - .attr('data-markdown-version', markdownVersion); + .attr('data-remote', 'true'); $editForm.find('.js-form-target-id').val(targetId); $editForm.find('.js-form-target-type').val(targetType); $editForm diff --git a/app/assets/javascripts/notes/components/comment_form.vue b/app/assets/javascripts/notes/components/comment_form.vue index d669ba5a8fa..1d6cb9485f7 100644 --- a/app/assets/javascripts/notes/components/comment_form.vue +++ b/app/assets/javascripts/notes/components/comment_form.vue @@ -39,11 +39,6 @@ export default { type: String, required: true, }, - markdownVersion: { - type: Number, - required: false, - default: 0, - }, }, data() { return { @@ -342,7 +337,6 @@ Please check your network connection and try again.`; :markdown-preview-path="markdownPreviewPath" :markdown-docs-path="markdownDocsPath" :quick-actions-docs-path="quickActionsDocsPath" - :markdown-version="markdownVersion" :add-spacing-classes="false" > <textarea diff --git a/app/assets/javascripts/notes/components/note_body.vue b/app/assets/javascripts/notes/components/note_body.vue index bcf5d334da4..ff303d0f55a 100644 --- a/app/assets/javascripts/notes/components/note_body.vue +++ b/app/assets/javascripts/notes/components/note_body.vue @@ -111,7 +111,6 @@ export default { :line="line" :note="note" :help-page-path="helpPagePath" - :markdown-version="note.cached_markdown_version" @handleFormUpdate="handleFormUpdate" @cancelForm="formCancelHandler" /> diff --git a/app/assets/javascripts/notes/components/note_form.vue b/app/assets/javascripts/notes/components/note_form.vue index 269b4a4b117..92258a25438 100644 --- a/app/assets/javascripts/notes/components/note_form.vue +++ b/app/assets/javascripts/notes/components/note_form.vue @@ -26,11 +26,6 @@ export default { required: false, default: '', }, - markdownVersion: { - type: Number, - required: false, - default: 0, - }, saveButtonTitle: { type: String, required: false, @@ -202,7 +197,6 @@ export default { <markdown-field :markdown-preview-path="markdownPreviewPath" :markdown-docs-path="markdownDocsPath" - :markdown-version="markdownVersion" :quick-actions-docs-path="quickActionsDocsPath" :line="line" :note="discussionNote" diff --git a/app/assets/javascripts/notes/components/notes_app.vue b/app/assets/javascripts/notes/components/notes_app.vue index f3fcfdfda05..5edceea043c 100644 --- a/app/assets/javascripts/notes/components/notes_app.vue +++ b/app/assets/javascripts/notes/components/notes_app.vue @@ -44,11 +44,6 @@ export default { required: false, default: true, }, - markdownVersion: { - type: Number, - required: false, - default: 0, - }, helpPagePath: { type: String, required: false, @@ -216,10 +211,6 @@ export default { </template> </ul> - <comment-form - v-if="!commentsDisabled" - :noteable-type="noteableType" - :markdown-version="markdownVersion" - /> + <comment-form v-if="!commentsDisabled" :noteable-type="noteableType" /> </div> </template> diff --git a/app/assets/javascripts/notes/index.js b/app/assets/javascripts/notes/index.js index 2f715c85fa6..4883266dae5 100644 --- a/app/assets/javascripts/notes/index.js +++ b/app/assets/javascripts/notes/index.js @@ -18,7 +18,6 @@ document.addEventListener('DOMContentLoaded', () => { const notesDataset = document.getElementById('js-vue-notes').dataset; const parsedUserData = JSON.parse(notesDataset.currentUserData); const noteableData = JSON.parse(notesDataset.noteableData); - const markdownVersion = parseInt(notesDataset.markdownVersion, 10); let currentUserData = {}; noteableData.noteableType = notesDataset.noteableType; @@ -37,7 +36,6 @@ document.addEventListener('DOMContentLoaded', () => { return { noteableData, currentUserData, - markdownVersion, notesData: JSON.parse(notesDataset.notesData), }; }, @@ -47,7 +45,6 @@ document.addEventListener('DOMContentLoaded', () => { noteableData: this.noteableData, notesData: this.notesData, userData: this.currentUserData, - markdownVersion: this.markdownVersion, }, }); }, diff --git a/app/assets/javascripts/ide/components/file_finder/index.vue b/app/assets/javascripts/vue_shared/components/file_finder/index.vue index 0b0cd7b75eb..b57455adaad 100644 --- a/app/assets/javascripts/ide/components/file_finder/index.vue +++ b/app/assets/javascripts/vue_shared/components/file_finder/index.vue @@ -1,45 +1,62 @@ <script> -import { mapActions, mapGetters, mapState } from 'vuex'; import fuzzaldrinPlus from 'fuzzaldrin-plus'; +import Mousetrap from 'mousetrap'; import VirtualList from 'vue-virtual-scroll-list'; import Item from './item.vue'; -import router from '../../ide_router'; -import { - MAX_FILE_FINDER_RESULTS, - FILE_FINDER_ROW_HEIGHT, - FILE_FINDER_EMPTY_ROW_HEIGHT, -} from '../../constants'; -import { - UP_KEY_CODE, - DOWN_KEY_CODE, - ENTER_KEY_CODE, - ESC_KEY_CODE, -} from '../../../lib/utils/keycodes'; +import { UP_KEY_CODE, DOWN_KEY_CODE, ENTER_KEY_CODE, ESC_KEY_CODE } from '~/lib/utils/keycodes'; + +export const MAX_FILE_FINDER_RESULTS = 40; +export const FILE_FINDER_ROW_HEIGHT = 55; +export const FILE_FINDER_EMPTY_ROW_HEIGHT = 33; + +const originalStopCallback = Mousetrap.stopCallback; export default { components: { Item, VirtualList, }, + props: { + files: { + type: Array, + required: true, + }, + visible: { + type: Boolean, + required: true, + }, + loading: { + type: Boolean, + required: true, + }, + showDiffStats: { + type: Boolean, + required: false, + default: false, + }, + clearSearchOnClose: { + type: Boolean, + required: false, + default: true, + }, + }, data() { return { - focusedIndex: 0, + focusedIndex: -1, searchText: '', mouseOver: false, cancelMouseOver: false, }; }, computed: { - ...mapGetters(['allBlobs']), - ...mapState(['fileFindVisible', 'loading']), filteredBlobs() { const searchText = this.searchText.trim(); if (searchText === '') { - return this.allBlobs.slice(0, MAX_FILE_FINDER_RESULTS); + return this.files.slice(0, MAX_FILE_FINDER_RESULTS); } - return fuzzaldrinPlus.filter(this.allBlobs, searchText, { + return fuzzaldrinPlus.filter(this.files, searchText, { key: 'path', maxResults: MAX_FILE_FINDER_RESULTS, }); @@ -58,10 +75,12 @@ export default { }, }, watch: { - fileFindVisible() { + visible() { this.$nextTick(() => { - if (!this.fileFindVisible) { - this.searchText = ''; + if (!this.visible) { + if (this.clearSearchOnClose) { + this.searchText = ''; + } } else { this.focusedIndex = 0; @@ -72,7 +91,11 @@ export default { }); }, searchText() { - this.focusedIndex = 0; + this.focusedIndex = -1; + + this.$nextTick(() => { + this.focusedIndex = 0; + }); }, focusedIndex() { if (!this.mouseOver) { @@ -98,8 +121,25 @@ export default { } }, }, + mounted() { + if (this.files.length) { + this.focusedIndex = 0; + } + + Mousetrap.bind(['t', 'command+p', 'ctrl+p'], e => { + if (e.preventDefault) { + e.preventDefault(); + } + + this.toggle(!this.visible); + }); + + Mousetrap.stopCallback = (e, el, combo) => this.mousetrapStopCallback(e, el, combo); + }, methods: { - ...mapActions(['toggleFileFinder']), + toggle(visible) { + this.$emit('toggle', visible); + }, clearSearchInput() { this.searchText = ''; @@ -139,15 +179,15 @@ export default { this.openFile(this.filteredBlobs[this.focusedIndex]); break; case ESC_KEY_CODE: - this.toggleFileFinder(false); + this.toggle(false); break; default: break; } }, openFile(file) { - this.toggleFileFinder(false); - router.push(`/project${file.url}`); + this.toggle(false); + this.$emit('click', file); }, onMouseOver(index) { if (!this.cancelMouseOver) { @@ -159,14 +199,26 @@ export default { this.cancelMouseOver = false; this.onMouseOver(index); }, + mousetrapStopCallback(e, el, combo) { + if ( + (combo === 't' && el.classList.contains('dropdown-input-field')) || + el.classList.contains('inputarea') + ) { + return true; + } else if (combo === 'command+p' || combo === 'ctrl+p') { + return false; + } + + return originalStopCallback(e, el, combo); + }, }, }; </script> <template> - <div class="ide-file-finder-overlay" @mousedown.self="toggleFileFinder(false)"> - <div class="dropdown-menu diff-file-changes ide-file-finder show"> - <div class="dropdown-input"> + <div class="file-finder-overlay" @mousedown.self="toggle(false)"> + <div class="dropdown-menu diff-file-changes file-finder show"> + <div :class="{ 'has-value': showClearInputButton }" class="dropdown-input"> <input ref="searchInput" v-model="searchText" @@ -186,9 +238,6 @@ export default { ></i> <i :aria-label="__('Clear search input')" - :class="{ - show: showClearInputButton, - }" role="button" class="fa fa-times dropdown-input-clear" @click="clearSearchInput" @@ -203,6 +252,7 @@ export default { :search-text="searchText" :focused="index === focusedIndex" :index="index" + :show-diff-stats="showDiffStats" class="disable-hover" @click="openFile" @mouseover="onMouseOver" @@ -225,3 +275,25 @@ export default { </div> </div> </template> + +<style scoped> +.file-finder-overlay { + position: absolute; + top: 0; + right: 0; + bottom: 0; + left: 0; + z-index: 200; +} + +.file-finder { + top: 10px; + left: 50%; + transform: translateX(-50%); +} + +.diff-file-changes { + top: 50px; + max-height: 327px; +} +</style> diff --git a/app/assets/javascripts/ide/components/file_finder/item.vue b/app/assets/javascripts/vue_shared/components/file_finder/item.vue index 83e80d50aff..73511879ff2 100644 --- a/app/assets/javascripts/ide/components/file_finder/item.vue +++ b/app/assets/javascripts/vue_shared/components/file_finder/item.vue @@ -1,5 +1,6 @@ <script> import fuzzaldrinPlus from 'fuzzaldrin-plus'; +import Icon from '~/vue_shared/components/icon.vue'; import FileIcon from '../../../vue_shared/components/file_icon.vue'; import ChangedFileIcon from '../../../vue_shared/components/changed_file_icon.vue'; @@ -7,6 +8,7 @@ const MAX_PATH_LENGTH = 60; export default { components: { + Icon, ChangedFileIcon, FileIcon, }, @@ -27,6 +29,11 @@ export default { type: Number, required: true, }, + showDiffStats: { + type: Boolean, + required: false, + default: false, + }, }, computed: { pathWithEllipsis() { @@ -97,8 +104,23 @@ export default { </span> </span> </span> - <span v-if="file.changed || file.tempFile" class="diff-changed-stats"> - <changed-file-icon :file="file" /> + <span v-if="file.changed || file.tempFile" v-once class="diff-changed-stats"> + <span v-if="showDiffStats"> + <span class="cgreen bold"> + <icon name="file-addition" class="align-text-top" /> {{ file.addedLines }} + </span> + <span class="cred bold ml-1"> + <icon name="file-deletion" class="align-text-top" /> {{ file.removedLines }} + </span> + </span> + <changed-file-icon v-else :file="file" /> </span> </button> </template> + +<style scoped> +.highlighted { + color: #1f78d1; + font-weight: 600; +} +</style> diff --git a/app/assets/javascripts/vue_shared/components/markdown/field.vue b/app/assets/javascripts/vue_shared/components/markdown/field.vue index cc07ef46064..3f607aa2a0a 100644 --- a/app/assets/javascripts/vue_shared/components/markdown/field.vue +++ b/app/assets/javascripts/vue_shared/components/markdown/field.vue @@ -27,11 +27,6 @@ export default { type: String, required: true, }, - markdownVersion: { - type: Number, - required: false, - default: 0, - }, addSpacingClasses: { type: Boolean, required: false, @@ -158,7 +153,7 @@ export default { this.markdownPreviewLoading = true; this.markdownPreview = __('Loading…'); this.$http - .post(this.versionedPreviewPath(), { text }) + .post(this.markdownPreviewPath, { text }) .then(resp => resp.json()) .then(data => this.renderMarkdown(data)) .catch(() => new Flash(__('Error loading markdown preview'))); @@ -186,13 +181,6 @@ export default { .then(() => $(this.$refs['markdown-preview']).renderGFM()) .catch(() => new Flash(__('Error rendering markdown preview'))); }, - - versionedPreviewPath() { - const { markdownPreviewPath, markdownVersion } = this; - return `${markdownPreviewPath}${ - markdownPreviewPath.indexOf('?') === -1 ? '?' : '&' - }markdown_version=${markdownVersion}`; - }, }, }; </script> diff --git a/app/assets/stylesheets/page_bundles/ide.scss b/app/assets/stylesheets/page_bundles/ide.scss index 1f24b8dfa9e..2ac98b5d18f 100644 --- a/app/assets/stylesheets/page_bundles/ide.scss +++ b/app/assets/stylesheets/page_bundles/ide.scss @@ -816,26 +816,6 @@ $ide-commit-header-height: 48px; z-index: 1; } -.ide-file-finder-overlay { - position: absolute; - top: 0; - right: 0; - bottom: 0; - left: 0; - z-index: 100; -} - -.ide-file-finder { - top: 10px; - left: 50%; - transform: translateX(-50%); - - .highlighted { - color: $blue-500; - font-weight: $gl-font-weight-bold; - } -} - .ide-commit-message-field { height: 200px; background-color: $white-light; diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss index 53afb182b54..38a7e199c6a 100644 --- a/app/assets/stylesheets/pages/merge_requests.scss +++ b/app/assets/stylesheets/pages/merge_requests.scss @@ -986,3 +986,9 @@ width: $ci-action-icon-size-lg; } } + +.merge-request-details .file-finder-overlay.diff-file-finder { + position: fixed; + z-index: 99999; + background: $black-transparent; +} diff --git a/app/controllers/concerns/preview_markdown.rb b/app/controllers/concerns/preview_markdown.rb index 4b0f0b8255c..f72d25fc54c 100644 --- a/app/controllers/concerns/preview_markdown.rb +++ b/app/controllers/concerns/preview_markdown.rb @@ -16,8 +16,6 @@ module PreviewMarkdown else {} end - markdown_params[:markdown_engine] = result[:markdown_engine] - render json: { body: view_context.markdown(result[:text], markdown_params), references: { diff --git a/app/controllers/concerns/send_file_upload.rb b/app/controllers/concerns/send_file_upload.rb index 515a9eede8e..9ca54c5519b 100644 --- a/app/controllers/concerns/send_file_upload.rb +++ b/app/controllers/concerns/send_file_upload.rb @@ -3,16 +3,19 @@ module SendFileUpload def send_upload(file_upload, send_params: {}, redirect_params: {}, attachment: nil, proxy: false, disposition: 'attachment') if attachment + response_disposition = ::Gitlab::ContentDisposition.format(disposition: 'attachment', filename: attachment) + # Response-Content-Type will not override an existing Content-Type in # Google Cloud Storage, so the metadata needs to be cleared on GCS for # this to work. However, this override works with AWS. - redirect_params[:query] = { "response-content-disposition" => "#{disposition};filename=#{attachment.inspect}", + redirect_params[:query] = { "response-content-disposition" => response_disposition, "response-content-type" => guess_content_type(attachment) } # By default, Rails will send uploads with an extension of .js with a # content-type of text/javascript, which will trigger Rails' # cross-origin JavaScript protection. send_params[:content_type] = 'text/plain' if File.extname(attachment) == '.js' - send_params.merge!(filename: attachment, disposition: disposition) + + send_params.merge!(filename: attachment, disposition: utf8_encoded_disposition(disposition, attachment)) end if file_upload.file_storage? @@ -25,6 +28,18 @@ module SendFileUpload end end + # Since Rails 5 doesn't properly support support non-ASCII filenames, + # we have to add our own to ensure RFC 5987 compliance. However, Rails + # 5 automatically appends `filename#{filename}` here: + # https://github.com/rails/rails/blob/v5.0.7/actionpack/lib/action_controller/metal/data_streaming.rb#L137 + # Rails 6 will have https://github.com/rails/rails/pull/33829, so we + # can get rid of this special case handling when we upgrade. + def utf8_encoded_disposition(disposition, filename) + content = ::Gitlab::ContentDisposition.new(disposition: disposition, filename: filename) + + "#{disposition}; #{content.utf8_filename}" + end + def guess_content_type(filename) types = MIME::Types.type_for(filename) diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index f8e482937d5..97120273d6b 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -4,7 +4,7 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController include AuthenticatesWithTwoFactor include Devise::Controllers::Rememberable - protect_from_forgery except: [:kerberos, :saml, :cas3], prepend: true + protect_from_forgery except: [:kerberos, :saml, :cas3, :failure], with: :exception, prepend: true def handle_omniauth omniauth_flow(Gitlab::Auth::OAuth) diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb index 0fee29bf7c7..1a471034972 100644 --- a/app/helpers/issuables_helper.rb +++ b/app/helpers/issuables_helper.rb @@ -268,7 +268,6 @@ module IssuablesHelper issuableRef: issuable.to_reference, markdownPreviewPath: preview_markdown_path(parent), markdownDocsPath: help_page_path('user/markdown'), - markdownVersion: issuable.cached_markdown_version, lockVersion: issuable.lock_version, issuableTemplates: issuable_templates(issuable), initialTitleHtml: markdown_field(issuable, :title), diff --git a/app/helpers/markup_helper.rb b/app/helpers/markup_helper.rb index 0d638b850b4..66f4b7b3f30 100644 --- a/app/helpers/markup_helper.rb +++ b/app/helpers/markup_helper.rb @@ -116,7 +116,6 @@ module MarkupHelper def markup(file_name, text, context = {}) context[:project] ||= @project - context[:markdown_engine] ||= :redcarpet unless commonmark_for_repositories_enabled? html = context.delete(:rendered) || markup_unsafe(file_name, text, context) prepare_for_rendering(html, context) end @@ -132,7 +131,6 @@ module MarkupHelper page_slug: wiki_page.slug, issuable_state_filter_enabled: true ) - context[:markdown_engine] ||= :redcarpet unless commonmark_for_repositories_enabled? html = case wiki_page.format @@ -187,10 +185,6 @@ module MarkupHelper end end - def commonmark_for_repositories_enabled? - Feature.enabled?(:commonmark_for_repositories, default_enabled: true) - end - private # Return +text+, truncated to +max_chars+ characters, excluding any HTML diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb index 293dd20ad49..aaf38cbfe70 100644 --- a/app/helpers/notes_helper.rb +++ b/app/helpers/notes_helper.rb @@ -171,7 +171,6 @@ module NotesHelper registerPath: new_session_path(:user, redirect_to_referer: 'yes', anchor: 'register-pane'), newSessionPath: new_session_path(:user, redirect_to_referer: 'yes'), markdownDocsPath: help_page_path('user/markdown'), - markdownVersion: issuable.cached_markdown_version, quickActionsDocsPath: help_page_path('user/project/quick_actions'), closePath: close_issuable_path(issuable), reopenPath: reopen_issuable_path(issuable), diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index 4408cb5145a..c400302cda3 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -265,10 +265,6 @@ module ProjectsHelper link_to 'BFG', 'https://rtyley.github.io/bfg-repo-cleaner/', target: '_blank', rel: 'noopener noreferrer' end - def legacy_render_context(params) - params[:legacy_render] ? { markdown_engine: :redcarpet } : {} - end - def explore_projects_tab? current_page?(explore_projects_path) || current_page?(trending_explore_projects_path) || diff --git a/app/models/application_record.rb b/app/models/application_record.rb index 29696ab276f..c4e310e638d 100644 --- a/app/models/application_record.rb +++ b/app/models/application_record.rb @@ -6,4 +6,12 @@ class ApplicationRecord < ActiveRecord::Base def self.id_in(ids) where(id: ids) end + + def self.safe_find_or_create_by(*args) + transaction(requires_new: true) do + find_or_create_by(*args) + end + rescue ActiveRecord::RecordNotUnique + retry + end end diff --git a/app/models/concerns/cache_markdown_field.rb b/app/models/concerns/cache_markdown_field.rb index 588204c7470..5fa6f79bdaa 100644 --- a/app/models/concerns/cache_markdown_field.rb +++ b/app/models/concerns/cache_markdown_field.rb @@ -13,7 +13,6 @@ module CacheMarkdownField extend ActiveSupport::Concern # Increment this number every time the renderer changes its output - CACHE_REDCARPET_VERSION = 3 CACHE_COMMONMARK_VERSION_START = 10 CACHE_COMMONMARK_VERSION = 14 @@ -42,18 +41,6 @@ module CacheMarkdownField end end - class MarkdownEngine - def self.from_version(version = nil) - return :common_mark if version.nil? || version == 0 - - if version < CacheMarkdownField::CACHE_COMMONMARK_VERSION_START - :redcarpet - else - :common_mark - end - end - end - def skip_project_check? false end @@ -71,7 +58,7 @@ module CacheMarkdownField # Banzai is less strict about authors, so don't always have an author key context[:author] = self.author if self.respond_to?(:author) - context[:markdown_engine] = MarkdownEngine.from_version(latest_cached_markdown_version) + context[:markdown_engine] = :common_mark context end @@ -128,17 +115,7 @@ module CacheMarkdownField end def latest_cached_markdown_version - return CacheMarkdownField::CACHE_COMMONMARK_VERSION unless cached_markdown_version - - if legacy_markdown? - CacheMarkdownField::CACHE_REDCARPET_VERSION - else - CacheMarkdownField::CACHE_COMMONMARK_VERSION - end - end - - def legacy_markdown? - cached_markdown_version && cached_markdown_version.between?(1, CacheMarkdownField::CACHE_COMMONMARK_VERSION_START - 1) + CacheMarkdownField::CACHE_COMMONMARK_VERSION end included do diff --git a/app/models/repository.rb b/app/models/repository.rb index e6ab3b484a2..9ae13fbaa80 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -614,7 +614,6 @@ class Repository return unless readme context = { project: project } - context[:markdown_engine] = :redcarpet unless MarkupHelper.commonmark_for_repositories_enabled? MarkupHelper.markup_unsafe(readme.name, readme.data, context) end diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index f8d8ef04001..699b3e8555e 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -7,14 +7,17 @@ module Ci CreateError = Class.new(StandardError) SEQUENCE = [Gitlab::Ci::Pipeline::Chain::Build, + Gitlab::Ci::Pipeline::Chain::RemoveUnwantedChatJobs, Gitlab::Ci::Pipeline::Chain::Validate::Abilities, Gitlab::Ci::Pipeline::Chain::Validate::Repository, Gitlab::Ci::Pipeline::Chain::Validate::Config, Gitlab::Ci::Pipeline::Chain::Skip, + Gitlab::Ci::Pipeline::Chain::Limit::Size, Gitlab::Ci::Pipeline::Chain::Populate, - Gitlab::Ci::Pipeline::Chain::Create].freeze + Gitlab::Ci::Pipeline::Chain::Create, + Gitlab::Ci::Pipeline::Chain::Limit::Activity].freeze - def execute(source, ignore_skip_ci: false, save_on_errors: true, trigger_request: nil, schedule: nil, merge_request: nil, &block) + def execute(source, ignore_skip_ci: false, save_on_errors: true, trigger_request: nil, schedule: nil, merge_request: nil, **options, &block) @pipeline = Ci::Pipeline.new command = Gitlab::Ci::Pipeline::Chain::Command.new( @@ -32,7 +35,8 @@ module Ci variables_attributes: params[:variables_attributes], project: project, current_user: current_user, - push_options: params[:push_options]) + push_options: params[:push_options], + **extra_options(**options)) sequence = Gitlab::Ci::Pipeline::Chain::Sequence .new(pipeline, command, SEQUENCE) @@ -103,5 +107,9 @@ module Ci pipeline.project.source_of_merge_requests.opened.where(source_branch: pipeline.ref) end # rubocop: enable CodeReuse/ActiveRecord + + def extra_options + {} # overriden in EE + end end end diff --git a/app/services/groups/update_service.rb b/app/services/groups/update_service.rb index de78a3f7b27..9ff1da270e2 100644 --- a/app/services/groups/update_service.rb +++ b/app/services/groups/update_service.rb @@ -11,6 +11,8 @@ module Groups return false unless valid_share_with_group_lock_change? + before_assignment_hook(group, params) + group.assign_attributes(params) begin @@ -28,6 +30,10 @@ module Groups private + def before_assignment_hook(group, params) + # overriden in EE + end + def after_update if group.previous_changes.include?(:visibility_level) && group.private? # don't enqueue immediately to prevent todos removal in case of a mistake diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index 842d59d26a0..ef991eaf234 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -270,9 +270,7 @@ class IssuableBaseService < BaseService tasklist_toggler = TaskListToggleService.new(issuable.description, issuable.description_html, line_source: update_task_params[:line_source], line_number: update_task_params[:line_number].to_i, - toggle_as_checked: update_task_params[:checked], - index: update_task_params[:index].to_i, - sourcepos: !issuable.legacy_markdown?) + toggle_as_checked: update_task_params[:checked]) unless tasklist_toggler.execute # if we make it here, the data is much newer than we thought it was - fail fast diff --git a/app/services/preview_markdown_service.rb b/app/services/preview_markdown_service.rb index a449a5dc3e9..c1655c38095 100644 --- a/app/services/preview_markdown_service.rb +++ b/app/services/preview_markdown_service.rb @@ -10,8 +10,7 @@ class PreviewMarkdownService < BaseService text: text, users: users, suggestions: suggestions, - commands: commands.join(' '), - markdown_engine: markdown_engine + commands: commands.join(' ') ) end @@ -49,12 +48,4 @@ class PreviewMarkdownService < BaseService def commands_target_id params[:quick_actions_target_id] end - - def markdown_engine - if params[:legacy_render] - :redcarpet - else - CacheMarkdownField::MarkdownEngine.from_version(params[:markdown_version].to_i) - end - end end diff --git a/app/services/search/global_service.rb b/app/services/search/global_service.rb index cb1bf0a03a5..d6af26d949d 100644 --- a/app/services/search/global_service.rb +++ b/app/services/search/global_service.rb @@ -2,6 +2,8 @@ module Search class GlobalService + include Gitlab::Utils::StrongMemoize + attr_accessor :current_user, :params attr_reader :default_project_filter @@ -19,11 +21,15 @@ module Search @projects ||= ProjectsFinder.new(current_user: current_user).execute end - def scope - @scope ||= begin - allowed_scopes = %w[issues merge_requests milestones] + def allowed_scopes + strong_memoize(:allowed_scopes) do + %w[issues merge_requests milestones] + end + end - allowed_scopes.delete(params[:scope]) { 'projects' } + def scope + strong_memoize(:scope) do + allowed_scopes.include?(params[:scope]) ? params[:scope] : 'projects' end end end diff --git a/app/services/task_list_toggle_service.rb b/app/services/task_list_toggle_service.rb index 2717fc9035a..b5c4cd3235d 100644 --- a/app/services/task_list_toggle_service.rb +++ b/app/services/task_list_toggle_service.rb @@ -5,17 +5,13 @@ # We don't care if the text has changed above or below the specific checkbox, as long # the checkbox still exists at exactly the same line number and the text is equal. # If successful, new values are available in `updated_markdown` and `updated_markdown_html` -# -# Note: once we've removed RedCarpet support, we can remove the `index` and `sourcepos` -# parameters class TaskListToggleService attr_reader :updated_markdown, :updated_markdown_html - def initialize(markdown, markdown_html, line_source:, line_number:, toggle_as_checked:, index:, sourcepos: true) + def initialize(markdown, markdown_html, line_source:, line_number:, toggle_as_checked:) @markdown, @markdown_html = markdown, markdown_html @line_source, @line_number = line_source, line_number @toggle_as_checked = toggle_as_checked - @index, @use_sourcepos = index, sourcepos @updated_markdown, @updated_markdown_html = nil end @@ -28,8 +24,8 @@ class TaskListToggleService private - attr_reader :markdown, :markdown_html, :index, :toggle_as_checked - attr_reader :line_source, :line_number, :use_sourcepos + attr_reader :markdown, :markdown_html, :toggle_as_checked + attr_reader :line_source, :line_number def toggle_markdown source_lines = markdown.split("\n") @@ -68,17 +64,8 @@ class TaskListToggleService end # When using CommonMark, we should be able to use the embedded `sourcepos` attribute to - # target the exact line in the DOM. For RedCarpet, we need to use the index of the checkbox - # that was checked and match it with what we think is the same checkbox. - # The reason `sourcepos` is slightly more reliable is the case where a line of text is - # changed from a regular line into a checkbox (or vice versa). Then the checked index - # in the UI will be off from the list of checkboxes we've calculated locally. - # It's a rare circumstance, but since we can account for it, we do. + # target the exact line in the DOM. def get_html_checkbox(html) - if use_sourcepos - html.css(".task-list-item[data-sourcepos^='#{line_number}:'] > input.task-list-item-checkbox").first - else - html.css('.task-list-item-checkbox')[index - 1] - end + html.css(".task-list-item[data-sourcepos^='#{line_number}:'] > input.task-list-item-checkbox").first end end diff --git a/app/views/projects/_wiki.html.haml b/app/views/projects/_wiki.html.haml index 45e1d32980c..de4653dad2c 100644 --- a/app/views/projects/_wiki.html.haml +++ b/app/views/projects/_wiki.html.haml @@ -2,7 +2,7 @@ %div{ class: container_class } .prepend-top-default.append-bottom-default .wiki - = render_wiki_content(@wiki_home, legacy_render_context(params)) + = render_wiki_content(@wiki_home) - else - can_create_wiki = can?(current_user, :create_wiki, @project) .landing{ class: [('row-content-block row p-0 align-items-center' if can_create_wiki), ('content-block' unless can_create_wiki)] } diff --git a/app/views/projects/blob/edit.html.haml b/app/views/projects/blob/edit.html.haml index 3f2d96b70e5..4520cca8cf5 100644 --- a/app/views/projects/blob/edit.html.haml +++ b/app/views/projects/blob/edit.html.haml @@ -21,7 +21,7 @@ Write %li - = link_to '#preview', 'data-preview-url' => project_preview_blob_path(@project, @id, legacy_render: params[:legacy_render]) do + = link_to '#preview', 'data-preview-url' => project_preview_blob_path(@project, @id) do = editing_preview_title(@blob.name) = form_tag(project_update_blob_path(@project, @id), method: :put, class: 'js-quick-submit js-requires-input js-edit-blob-form', data: blob_editor_paths(@project)) do diff --git a/app/views/projects/blob/preview.html.haml b/app/views/projects/blob/preview.html.haml index ff460a3831c..66687f087ff 100644 --- a/app/views/projects/blob/preview.html.haml +++ b/app/views/projects/blob/preview.html.haml @@ -2,7 +2,7 @@ .diff-content - if markup?(@blob.name) .file-content.wiki.md{ class: ('use-csslab' if Feature.enabled?(:csslab)) } - = markup(@blob.name, @content, legacy_render_context(params)) + = markup(@blob.name, @content) - else .file-content.code.js-syntax-highlight - unless @diff_lines.empty? diff --git a/app/views/projects/blob/viewers/_markup.html.haml b/app/views/projects/blob/viewers/_markup.html.haml index 6edbfd91b21..1a77eb078be 100644 --- a/app/views/projects/blob/viewers/_markup.html.haml +++ b/app/views/projects/blob/viewers/_markup.html.haml @@ -1,6 +1,4 @@ - blob = viewer.blob -- context = legacy_render_context(params) -- unless context[:markdown_engine] == :redcarpet - - context[:rendered] = blob.rendered_markup if blob.respond_to?(:rendered_markup) +- context = blob.respond_to?(:rendered_markup) ? { rendered: blob.rendered_markup } : {} .file-content.wiki.md{ class: ('use-csslab' if Feature.enabled?(:csslab)) } = markup(blob.name, blob.data, context) diff --git a/app/views/projects/issues/_form.html.haml b/app/views/projects/issues/_form.html.haml index 1e4e9450ffa..1be1087b36f 100644 --- a/app/views/projects/issues/_form.html.haml +++ b/app/views/projects/issues/_form.html.haml @@ -1,4 +1,3 @@ = form_for [@project.namespace.becomes(Namespace), @project, @issue], - html: { class: 'issue-form common-note-form js-quick-submit js-requires-input' }, - data: { markdown_version: @issue.cached_markdown_version } do |f| + html: { class: 'issue-form common-note-form js-quick-submit js-requires-input' } do |f| = render 'shared/issuable/form', f: f, issuable: @issue diff --git a/app/views/projects/merge_requests/_form.html.haml b/app/views/projects/merge_requests/_form.html.haml index 13b967beba1..a7c9e54506d 100644 --- a/app/views/projects/merge_requests/_form.html.haml +++ b/app/views/projects/merge_requests/_form.html.haml @@ -1,4 +1,3 @@ = form_for [@project.namespace.becomes(Namespace), @project, @merge_request], - html: { class: 'merge-request-form common-note-form js-requires-input js-quick-submit' }, - data: { markdown_version: @merge_request.cached_markdown_version } do |f| + html: { class: 'merge-request-form common-note-form js-requires-input js-quick-submit' } do |f| = render 'shared/issuable/form', f: f, issuable: @merge_request, presenter: @mr_presenter diff --git a/app/views/projects/merge_requests/show.html.haml b/app/views/projects/merge_requests/show.html.haml index 0b720e5d542..5111c9fab8d 100644 --- a/app/views/projects/merge_requests/show.html.haml +++ b/app/views/projects/merge_requests/show.html.haml @@ -59,6 +59,7 @@ #js-vue-discussion-counter .tab-content#diff-notes-app + #js-diff-file-finder #notes.notes.tab-pane.voting_notes .row %section.col-md-12 diff --git a/app/views/projects/milestones/_form.html.haml b/app/views/projects/milestones/_form.html.haml index 19f5bba75c4..5cc6b5a173b 100644 --- a/app/views/projects/milestones/_form.html.haml +++ b/app/views/projects/milestones/_form.html.haml @@ -1,6 +1,5 @@ = form_for [@project.namespace.becomes(Namespace), @project, @milestone], - html: { class: 'milestone-form common-note-form js-quick-submit js-requires-input' }, - data: { markdown_version: @milestone.cached_markdown_version } do |f| + html: { class: 'milestone-form common-note-form js-quick-submit js-requires-input' } do |f| = form_errors(@milestone) .row .col-md-6 diff --git a/app/views/projects/tags/releases/edit.html.haml b/app/views/projects/tags/releases/edit.html.haml index 52c6c7ec424..e4efeed04f0 100644 --- a/app/views/projects/tags/releases/edit.html.haml +++ b/app/views/projects/tags/releases/edit.html.haml @@ -12,8 +12,7 @@ = form_for(@release, method: :put, url: project_tag_release_path(@project, @tag.name), - html: { class: 'common-note-form release-form js-quick-submit' }, - data: { markdown_version: @release.cached_markdown_version }) do |f| + html: { class: 'common-note-form release-form js-quick-submit' }) do |f| = render layout: 'projects/md_preview', locals: { url: preview_markdown_path(@project), referenced_users: true } do = render 'projects/zen', f: f, attr: :description, classes: 'note-textarea', placeholder: "Write your release notes or drag files here…" = render 'shared/notes/hints' diff --git a/app/views/projects/wikis/_form.html.haml b/app/views/projects/wikis/_form.html.haml index d1556dbd077..5bb69563b51 100644 --- a/app/views/projects/wikis/_form.html.haml +++ b/app/views/projects/wikis/_form.html.haml @@ -1,13 +1,9 @@ - commit_message = @page.persisted? ? s_("WikiPageEdit|Update %{page_title}") : s_("WikiPageCreate|Create %{page_title}") - commit_message = commit_message % { page_title: @page.title } -- if params[:legacy_render] || !commonmark_for_repositories_enabled? - - markdown_version = CacheMarkdownField::CACHE_REDCARPET_VERSION -- else - - markdown_version = 0 = form_for [@project.namespace.becomes(Namespace), @project, @page], method: @page.persisted? ? :put : :post, html: { class: 'wiki-form common-note-form prepend-top-default js-quick-submit' }, - data: { markdown_version: markdown_version, uploads_path: uploads_path } do |f| + data: { uploads_path: uploads_path } do |f| = form_errors(@page) - if @page.persisted? diff --git a/app/views/projects/wikis/_sidebar.html.haml b/app/views/projects/wikis/_sidebar.html.haml index 02c5a6ea55c..28353927135 100644 --- a/app/views/projects/wikis/_sidebar.html.haml +++ b/app/views/projects/wikis/_sidebar.html.haml @@ -12,7 +12,7 @@ .blocks-container .block.block-first - if @sidebar_page - = render_wiki_content(@sidebar_page, legacy_render_context(params)) + = render_wiki_content(@sidebar_page) - else %ul.wiki-pages = render @sidebar_wiki_entries, context: 'sidebar' diff --git a/app/views/projects/wikis/show.html.haml b/app/views/projects/wikis/show.html.haml index 8b348bb4e4f..8e1c054b50c 100644 --- a/app/views/projects/wikis/show.html.haml +++ b/app/views/projects/wikis/show.html.haml @@ -27,6 +27,6 @@ .prepend-top-default.append-bottom-default .wiki.md{ class: ('use-csslab' if Feature.enabled?(:csslab)) } - = render_wiki_content(@page, legacy_render_context(params)) + = render_wiki_content(@page) = render 'sidebar' diff --git a/app/views/search/results/_snippet_blob.html.haml b/app/views/search/results/_snippet_blob.html.haml index e0130f9a4b5..a60a4501557 100644 --- a/app/views/search/results/_snippet_blob.html.haml +++ b/app/views/search/results/_snippet_blob.html.haml @@ -21,7 +21,7 @@ .file-content.wiki - snippet_chunks.each do |chunk| - unless chunk[:data].empty? - = markup(snippet.file_name, chunk[:data], legacy_render_context(params)) + = markup(snippet.file_name, chunk[:data]) - else .file-content.code .nothing-here-block= _("Empty file") diff --git a/app/views/shared/notes/_note.html.haml b/app/views/shared/notes/_note.html.haml index cb5c64fb91d..41d6ae79c81 100644 --- a/app/views/shared/notes/_note.html.haml +++ b/app/views/shared/notes/_note.html.haml @@ -54,7 +54,7 @@ .note-text.md = markdown_field(note, :note) = edited_time_ago_with_tooltip(note, placement: 'bottom', html_class: 'note_edited_ago') - .original-note-content.hidden{ data: { post_url: note_url(note), target_id: note.noteable.id, target_type: note.noteable.class.name.underscore, markdown_version: note.cached_markdown_version } } + .original-note-content.hidden{ data: { post_url: note_url(note), target_id: note.noteable.id, target_type: note.noteable.class.name.underscore } } #{note.note} - if note_editable = render 'shared/notes/edit', note: note diff --git a/app/views/shared/snippets/_form.html.haml b/app/views/shared/snippets/_form.html.haml index cf9c3055499..3007da0c189 100644 --- a/app/views/shared/snippets/_form.html.haml +++ b/app/views/shared/snippets/_form.html.haml @@ -3,8 +3,7 @@ .snippet-form-holder = form_for @snippet, url: url, - html: { class: "snippet-form js-requires-input js-quick-submit common-note-form" }, - data: { markdown_version: @snippet.cached_markdown_version } do |f| + html: { class: "snippet-form js-requires-input js-quick-submit common-note-form" } do |f| = form_errors(@snippet) .form-group.row diff --git a/changelogs/unreleased/diff-file-finder.yml b/changelogs/unreleased/diff-file-finder.yml new file mode 100644 index 00000000000..3160e9fc91b --- /dev/null +++ b/changelogs/unreleased/diff-file-finder.yml @@ -0,0 +1,5 @@ +--- +title: Added fuzzy file finder to merge requests +merge_request: +author: +type: changed diff --git a/changelogs/unreleased/jej-avoid-csrf-check-on-saml-failure.yml b/changelogs/unreleased/jej-avoid-csrf-check-on-saml-failure.yml new file mode 100644 index 00000000000..18cced2906a --- /dev/null +++ b/changelogs/unreleased/jej-avoid-csrf-check-on-saml-failure.yml @@ -0,0 +1,5 @@ +--- +title: Display SAML failure messages instead of expecting CSRF token +merge_request: 24509 +author: +type: fixed diff --git a/changelogs/unreleased/jprovazn-remove-redcarpet.yml b/changelogs/unreleased/jprovazn-remove-redcarpet.yml new file mode 100644 index 00000000000..4e12de2d19b --- /dev/null +++ b/changelogs/unreleased/jprovazn-remove-redcarpet.yml @@ -0,0 +1,5 @@ +--- +title: Removed deprecated Redcarpet markdown engine. +merge_request: +author: +type: removed diff --git a/changelogs/unreleased/sh-encode-content-disposition.yml b/changelogs/unreleased/sh-encode-content-disposition.yml new file mode 100644 index 00000000000..b40ee6a85a8 --- /dev/null +++ b/changelogs/unreleased/sh-encode-content-disposition.yml @@ -0,0 +1,5 @@ +--- +title: Encode Content-Disposition filenames +merge_request: 24919 +author: +type: fixed diff --git a/config/initializers/sprockets_base_file_digest_key.rb b/config/initializers/sprockets_base_file_digest_key.rb new file mode 100644 index 00000000000..81ff3812091 --- /dev/null +++ b/config/initializers/sprockets_base_file_digest_key.rb @@ -0,0 +1,3 @@ +# frozen_string_literal: true + +Sprockets::Base.prepend(Gitlab::Patch::SprocketsBaseFileDigestKey) diff --git a/doc/development/sql.md b/doc/development/sql.md index 06005a0a6f8..47519d39e74 100644 --- a/doc/development/sql.md +++ b/doc/development/sql.md @@ -256,32 +256,12 @@ violation, for example. Using transactions does not solve this problem. -The following pattern should be used to avoid the problem: +To solve this we've added the `ApplicationRecord.safe_find_or_create_by`. -```ruby -Project.transaction do - begin - User.find_or_create_by(username: "foo") - rescue ActiveRecord::RecordNotUnique - retry - end -end -``` - -If the above block is run inside a transaction and hits the race -condition, the transaction is aborted and we cannot simply retry (any -further queries inside the aborted transaction are going to fail). We -can employ [nested transactions](http://api.rubyonrails.org/classes/ActiveRecord/Transactions/ClassMethods.html#module-ActiveRecord::Transactions::ClassMethods-label-Nested+transactions) -here to only rollback the "inner transaction". Note that `requires_new: true` is required here. +This method can be used just as you would the normal +`find_or_create_by` but it wraps the call in a *new* transaction and +retries if it were to fail because of an +`ActiveRecord::RecordNotUnique` error. -```ruby -Project.transaction do - begin - User.transaction(requires_new: true) do - User.find_or_create_by(username: "foo") - end - rescue ActiveRecord::RecordNotUnique - retry - end -end -``` +To be able to use this method, make sure the model you want to use +this on inherits from `ApplicationRecord`. diff --git a/doc/user/markdown.md b/doc/user/markdown.md index f2448f240ca..9a01625f3ff 100644 --- a/doc/user/markdown.md +++ b/doc/user/markdown.md @@ -31,8 +31,10 @@ dependency to do so. Please see the [`github-markup` gem readme](https://github. > As of 11.1, GitLab uses the [CommonMark Ruby Library][commonmarker] for Markdown processing of all new issues, merge requests, comments, and other Markdown content in the GitLab system. As of 11.3, wiki pages and Markdown files (`.md`) in the -repositories are also processed with CommonMark. Older content in issues/comments -are still processed using the [Redcarpet Ruby library][redcarpet]. +repositories are also processed with CommonMark. As of 11.8, the [Redcarpet +Ruby library][redcarpet] has been removed and all issues/comments, including +those from pre-11.1, are now processed using [CommonMark Ruby +Library][commonmarker]. > > The documentation website had its [markdown engine migrated from Redcarpet to Kramdown](https://gitlab.com/gitlab-com/gitlab-docs/merge_requests/108) in October 2018. @@ -41,11 +43,11 @@ in October 2018. ### Transitioning to CommonMark -You may have Markdown documents in your repository that were written using some -of the nuances of RedCarpet's version of Markdown. Since CommonMark uses a -slightly stricter syntax, these documents may now display a little strangely -since we've transitioned to CommonMark. Numbered lists with nested lists in -particular can be displayed incorrectly. +You may have older issues/merge requests or Markdown documents in your +repository that were written using some of the nuances of RedCarpet's version +of Markdown. Since CommonMark uses a slightly stricter syntax, these documents +may now display a little strangely since we've transitioned to CommonMark. +Numbered lists with nested lists in particular can be displayed incorrectly. It is usually quite easy to fix. In the case of a nested list such as this: @@ -65,11 +67,6 @@ simply add a space to each nested item: In the documentation below, we try to highlight some of the differences. -If you have a need to view a document using RedCarpet, you can add the token -`legacy_render=1` to the end of the url, like this: - -https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/user/markdown.md?legacy_render=1 - If you have a large volume of Markdown files, it can be tedious to determine if they will be displayed correctly or not. You can use the [diff_redcarpet_cmark](https://gitlab.com/digitalmoksha/diff_redcarpet_cmark) @@ -677,7 +674,7 @@ Becomes: + Or pluses If a list item contains multiple paragraphs, -each subsequent paragraph should be indented to the same level as the start of the list item text (_Redcarpet: paragraph should be indented with four spaces._) +each subsequent paragraph should be indented to the same level as the start of the list item text Example: @@ -841,7 +838,7 @@ These details <em>will</em> remain <strong>hidden</strong> until expanded. </details> </p> -**Note:** Markdown inside these tags is supported, as long as you have a blank line after the `</summary>` tag and before the `</details>` tag, as shown in the example. _Redcarpet does not support Markdown inside these tags. You can work around this by using HTML, for example you can use `<pre><code>` tags instead of [code fences](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/user/markdown.md#code-and-syntax-highlighting)._ +**Note:** Markdown inside these tags is supported, as long as you have a blank line after the `</summary>` tag and before the `</details>` tag, as shown in the example. ```html <details> diff --git a/doc/user/project/operations/index.md b/doc/user/project/operations/index.md new file mode 100644 index 00000000000..b0f9936be5c --- /dev/null +++ b/doc/user/project/operations/index.md @@ -0,0 +1,11 @@ +# Project operations + +GitLab provides a variety of tools to help operate and maintain +your applications: + +- Collect [Prometheus metrics](../integrations/prometheus_library/index.md). +- Deploy to different [environments](../../../ci/environments.md). +- Connect your project to a [Kubernetes cluster](../clusters/index.md). +- Discover and view errors generated by your applications with [Error Tracking](error_tracking.md). +- Create, toggle, and remove [Feature Flags](https://docs.gitlab.com/ee/user/project/operations/feature_flags.html). **[PREMIUM]** +- [Trace](https://docs.gitlab.com/ee/user/project/operations/tracing.html) the performance and health of a deployed application. **[ULTIMATE]** diff --git a/doc/user/project/pages/getting_started_part_two.md b/doc/user/project/pages/getting_started_part_two.md index b0560c2f44c..8dbbdd051f1 100644 --- a/doc/user/project/pages/getting_started_part_two.md +++ b/doc/user/project/pages/getting_started_part_two.md @@ -31,12 +31,26 @@ The optional settings, custom domain, DNS records, and SSL/TLS certificates, are ## Project Your GitLab Pages project is a regular project created the -same way you do for the other ones. To get started with GitLab Pages, you have two ways: +same way you do for the other ones. To get started with GitLab Pages, you have three ways: +- Use one of the popular templates already in the app, - Fork one of the templates from Page Examples, or - Create a new project from scratch -Let's go over both options. +Let's go over each option. + +### Use one of the popular Pages templates bundled with GitLab + +> [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/issues/47857) +in GitLab 11.8. + +The simplest way to create a GitLab Pages site is to use one of the most +popular templates, which come already bundled and ready to go. To use one +of these templates: + +1. From the top navigation, click the **+** button and select **New project** +1. Select **Create from Template** +1. Choose one of the templates starting with **Pages** ### Fork a project to get started from diff --git a/doc/user/project/pages/index.md b/doc/user/project/pages/index.md index ce4fccdaff3..11f6165fcb4 100644 --- a/doc/user/project/pages/index.md +++ b/doc/user/project/pages/index.md @@ -91,8 +91,8 @@ site under the HTTPS protocol. ## Getting started -To get started with GitLab Pages, you can either [create a project from scratch](getting_started_part_two.md#create-a-project-from-scratch) -or quickly start from copying an existing example project, as follows: +To get started with GitLab Pages, you can either [create a project from scratch](getting_started_part_two.md#create-a-project-from-scratch), +use a [bundled template](getting_started_part_two.md#use-one-of-the-popular-pages-templates-bundled-with-gitlab), or copy any of our existing example projects: 1. Choose an [example project](https://gitlab.com/pages) to [fork](../../../gitlab-basics/fork-project.md#how-to-fork-a-project): by forking a project, you create a copy of the codebase you're forking from to start from a template instead of starting from scratch. diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index fa6c9777824..e3d0b981065 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -422,7 +422,7 @@ module API def present_disk_file!(path, filename, content_type = 'application/octet-stream') filename ||= File.basename(path) - header['Content-Disposition'] = "attachment; filename=#{filename}" + header['Content-Disposition'] = ::Gitlab::ContentDisposition.format(disposition: 'attachment', filename: filename) header['Content-Transfer-Encoding'] = 'binary' content_type content_type @@ -496,7 +496,7 @@ module API def send_git_blob(repository, blob) env['api.format'] = :txt content_type 'text/plain' - header['Content-Disposition'] = content_disposition('inline', blob.name) + header['Content-Disposition'] = ::Gitlab::ContentDisposition.format(disposition: 'inline', filename: blob.name) # Let Workhorse examine the content and determine the better content disposition header[Gitlab::Workhorse::DETECT_HEADER] = "true" @@ -533,11 +533,5 @@ module API params[:archived] end - - def content_disposition(disposition, filename) - disposition += %(; filename=#{filename.inspect}) if filename.present? - - disposition - end end end diff --git a/lib/banzai/filter/markdown_engines/redcarpet.rb b/lib/banzai/filter/markdown_engines/redcarpet.rb deleted file mode 100644 index 5b3f75096b1..00000000000 --- a/lib/banzai/filter/markdown_engines/redcarpet.rb +++ /dev/null @@ -1,34 +0,0 @@ -# frozen_string_literal: true - -# `Redcarpet` markdown engine for GitLab's Banzai markdown filter. -# This module is used in Banzai::Filter::MarkdownFilter. -# Used gem is `redcarpet` which is a ruby library for markdown processing. -# Homepage: https://github.com/vmg/redcarpet - -module Banzai - module Filter - module MarkdownEngines - class Redcarpet - OPTIONS = { - fenced_code_blocks: true, - footnotes: true, - lax_spacing: true, - no_intra_emphasis: true, - space_after_headers: true, - strikethrough: true, - superscript: true, - tables: true - }.freeze - - def initialize(context = nil) - html_renderer = Banzai::Renderer::Redcarpet::HTML.new - @renderer = ::Redcarpet::Markdown.new(html_renderer, OPTIONS) - end - - def render(text) - @renderer.render(text) - end - end - end - end -end diff --git a/lib/banzai/filter/spaced_link_filter.rb b/lib/banzai/filter/spaced_link_filter.rb index 00dbf2d3130..50bf823929c 100644 --- a/lib/banzai/filter/spaced_link_filter.rb +++ b/lib/banzai/filter/spaced_link_filter.rb @@ -45,8 +45,6 @@ module Banzai ]).freeze def call - return doc if context[:markdown_engine] == :redcarpet - doc.xpath(TEXT_QUERY).each do |node| content = node.to_html diff --git a/lib/banzai/filter/syntax_highlight_filter.rb b/lib/banzai/filter/syntax_highlight_filter.rb index bcf77861f10..9ffde52b5f2 100644 --- a/lib/banzai/filter/syntax_highlight_filter.rb +++ b/lib/banzai/filter/syntax_highlight_filter.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require 'rouge/plugins/common_mark' -require 'rouge/plugins/redcarpet' # Generated HTML is transformed back to GFM by app/assets/javascripts/behaviors/markdown/nodes/code_block.js module Banzai diff --git a/lib/banzai/renderer/redcarpet/html.rb b/lib/banzai/renderer/redcarpet/html.rb deleted file mode 100644 index 84931fdc784..00000000000 --- a/lib/banzai/renderer/redcarpet/html.rb +++ /dev/null @@ -1,17 +0,0 @@ -# frozen_string_literal: true - -module Banzai - module Renderer - module Redcarpet - class HTML < ::Redcarpet::Render::HTML - def block_code(code, lang) - lang_attr = lang ? %Q{ lang="#{lang}"} : '' - - "\n<pre>" \ - "<code#{lang_attr}>#{ERB::Util.html_escape(code)}</code>" \ - "</pre>" - end - end - end - end -end diff --git a/lib/gitlab/ci/pipeline/chain/limit/activity.rb b/lib/gitlab/ci/pipeline/chain/limit/activity.rb new file mode 100644 index 00000000000..fe7c8738cc0 --- /dev/null +++ b/lib/gitlab/ci/pipeline/chain/limit/activity.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module Gitlab + module Ci + module Pipeline + module Chain + module Limit + class Activity < Chain::Base + def perform! + # to be overriden in EE + end + + def break? + false # to be overriden in EE + end + end + end + end + end + end +end diff --git a/lib/gitlab/ci/pipeline/chain/limit/size.rb b/lib/gitlab/ci/pipeline/chain/limit/size.rb new file mode 100644 index 00000000000..b4d51437cd6 --- /dev/null +++ b/lib/gitlab/ci/pipeline/chain/limit/size.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module Gitlab + module Ci + module Pipeline + module Chain + module Limit + class Size < Chain::Base + def perform! + # to be overriden in EE + end + + def break? + false # to be overriden in EE + end + end + end + end + end + end +end diff --git a/lib/gitlab/ci/pipeline/chain/remove_unwanted_chat_jobs.rb b/lib/gitlab/ci/pipeline/chain/remove_unwanted_chat_jobs.rb new file mode 100644 index 00000000000..0f687a4ce9b --- /dev/null +++ b/lib/gitlab/ci/pipeline/chain/remove_unwanted_chat_jobs.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module Gitlab + module Ci + module Pipeline + module Chain + class RemoveUnwantedChatJobs < Chain::Base + def perform! + # to be overriden in EE + end + + def break? + false + end + end + end + end + end +end diff --git a/lib/gitlab/content_disposition.rb b/lib/gitlab/content_disposition.rb new file mode 100644 index 00000000000..32207514ce5 --- /dev/null +++ b/lib/gitlab/content_disposition.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true +# This ports ActionDispatch::Http::ContentDisposition (https://github.com/rails/rails/pull/33829, +# which will be available in Rails 6. +module Gitlab + class ContentDisposition # :nodoc: + # Make sure we remove this patch starting with Rails 6.0. + if Rails.version.start_with?('6.0') + raise <<~MSG + Please remove this file and use `ActionDispatch::Http::ContentDisposition` instead. + MSG + end + + def self.format(disposition:, filename:) + new(disposition: disposition, filename: filename).to_s + end + + attr_reader :disposition, :filename + + def initialize(disposition:, filename:) + @disposition = disposition + @filename = filename + end + + # rubocop:disable Style/VariableInterpolation + TRADITIONAL_ESCAPED_CHAR = /[^ A-Za-z0-9!#$+.^_`|~-]/ + + def ascii_filename + 'filename="' + percent_escape(::I18n.transliterate(filename), TRADITIONAL_ESCAPED_CHAR) + '"' + end + + RFC_5987_ESCAPED_CHAR = /[^A-Za-z0-9!#$&+.^_`|~-]/ + # rubocop:enable Style/VariableInterpolation + + def utf8_filename + "filename*=UTF-8''" + percent_escape(filename, RFC_5987_ESCAPED_CHAR) + end + + def to_s + if filename + "#{disposition}; #{ascii_filename}; #{utf8_filename}" + else + "#{disposition}" + end + end + + private + + def percent_escape(string, pattern) + string.gsub(pattern) do |char| + char.bytes.map { |byte| "%%%02X" % byte }.join + end + end + end +end diff --git a/lib/gitlab/patch/sprockets_base_file_digest_key.rb b/lib/gitlab/patch/sprockets_base_file_digest_key.rb new file mode 100644 index 00000000000..3925cdbbada --- /dev/null +++ b/lib/gitlab/patch/sprockets_base_file_digest_key.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +# This monkey patch prevent cache ballooning when caching tmp/cache/assets/sprockets +# on the CI. See https://github.com/rails/sprockets/issues/563 and +# https://github.com/rails/sprockets/compare/3.x...jmreid:no-mtime-for-digest-key. +module Gitlab + module Patch + module SprocketsBaseFileDigestKey + def file_digest(path) + if stat = self.stat(path) + digest = self.stat_digest(path, stat) + integrity_uri = self.hexdigest_integrity_uri(digest) + + key = Sprockets::UnloadedAsset.new(path, self).file_digest_key(integrity_uri) + cache.fetch(key) do + digest + end + end + end + end + end +end diff --git a/lib/gitlab/task_helpers.rb b/lib/gitlab/task_helpers.rb index 224bb648d8f..8532845f3cb 100644 --- a/lib/gitlab/task_helpers.rb +++ b/lib/gitlab/task_helpers.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require 'rainbow/ext/string' -require 'gitlab/utils/strong_memoize' +require_dependency 'gitlab/utils/strong_memoize' # rubocop:disable Rails/Output module Gitlab @@ -13,6 +13,12 @@ module Gitlab extend self + def invoke_and_time_task(task) + start = Time.now + Rake::Task[task].invoke + puts "`#{task}` finished in #{Time.now - start} seconds" + end + # Ask if the user wants to continue # # Returns "yes" the user chose to continue diff --git a/lib/gitlab/utils/merge_hash.rb b/lib/gitlab/utils/merge_hash.rb index fc237861e2f..48ba13b8561 100644 --- a/lib/gitlab/utils/merge_hash.rb +++ b/lib/gitlab/utils/merge_hash.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require_dependency 'gitlab/utils' + module Gitlab module Utils module MergeHash diff --git a/lib/gitlab/utils/override.rb b/lib/gitlab/utils/override.rb index c87e97d0213..f5299439fce 100644 --- a/lib/gitlab/utils/override.rb +++ b/lib/gitlab/utils/override.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require_dependency 'gitlab/utils' + module Gitlab module Utils module Override diff --git a/lib/gitlab/utils/strong_memoize.rb b/lib/gitlab/utils/strong_memoize.rb index aa1f8e2fdda..3021a91dd83 100644 --- a/lib/gitlab/utils/strong_memoize.rb +++ b/lib/gitlab/utils/strong_memoize.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require_dependency 'gitlab/utils' + module Gitlab module Utils module StrongMemoize diff --git a/lib/tasks/gitlab/assets.rake b/lib/tasks/gitlab/assets.rake index a42f02a84fd..7a42e4e92a0 100644 --- a/lib/tasks/gitlab/assets.rake +++ b/lib/tasks/gitlab/assets.rake @@ -1,13 +1,17 @@ namespace :gitlab do namespace :assets do desc 'GitLab | Assets | Compile all frontend assets' - task compile: [ - 'yarn:check', - 'gettext:po_to_json', - 'rake:assets:precompile', - 'webpack:compile', - 'fix_urls' - ] + task :compile do + require_dependency 'gitlab/task_helpers' + + %w[ + yarn:check + gettext:po_to_json + rake:assets:precompile + webpack:compile + gitlab:assets:fix_urls + ].each(&Gitlab::TaskHelpers.method(:invoke_and_time_task)) + end desc 'GitLab | Assets | Clean up old compiled frontend assets' task clean: ['rake:assets:clean'] diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 6b5b4e93e70..24ca8744414 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -4438,10 +4438,10 @@ msgstr "" msgid "MergeRequest| %{paragraphStart}changed the description %{descriptionChangedTimes} times %{timeDifferenceMinutes}%{paragraphEnd}" msgstr "" -msgid "MergeRequest|Filter files" +msgid "MergeRequest|No files found" msgstr "" -msgid "MergeRequest|No files found" +msgid "MergeRequest|Search files" msgstr "" msgid "Merged" diff --git a/scripts/clean-old-cached-assets b/scripts/clean-old-cached-assets new file mode 100755 index 00000000000..7a3a62a477a --- /dev/null +++ b/scripts/clean-old-cached-assets @@ -0,0 +1,6 @@ +#!/bin/bash + +# Clean up cached files that are older than 1 week +find tmp/cache/assets/sprockets/ -type f -mtime +7 -execdir rm -- "{}" \; + +du -d 0 -h tmp/cache/assets/sprockets | cut -f1 | xargs -I % echo "tmp/cache/assets/sprockets/ is currently %" diff --git a/spec/controllers/concerns/send_file_upload_spec.rb b/spec/controllers/concerns/send_file_upload_spec.rb index 379b2d6b935..a07113a6156 100644 --- a/spec/controllers/concerns/send_file_upload_spec.rb +++ b/spec/controllers/concerns/send_file_upload_spec.rb @@ -53,19 +53,38 @@ describe SendFileUpload do end context 'with attachment' do - let(:params) { { attachment: 'test.js' } } + let(:filename) { 'test.js' } + let(:params) { { attachment: filename } } it 'sends a file with content-type of text/plain' do + # Notice the filename= is omitted from the disposition; this is because + # Rails 5 will append this header in send_file expected_params = { content_type: 'text/plain', filename: 'test.js', - disposition: 'attachment' + disposition: "attachment; filename*=UTF-8''test.js" } expect(controller).to receive(:send_file).with(uploader.path, expected_params) subject end + context 'with non-ASCII encoded filename' do + let(:filename) { 'テスト.txt' } + + # Notice the filename= is omitted from the disposition; this is because + # Rails 5 will append this header in send_file + it 'sends content-disposition for non-ASCII encoded filenames' do + expected_params = { + filename: filename, + disposition: "attachment; filename*=UTF-8''%E3%83%86%E3%82%B9%E3%83%88.txt" + } + expect(controller).to receive(:send_file).with(uploader.path, expected_params) + + subject + end + end + context 'with a proxied file in object storage' do before do stub_uploads_object_storage(uploader: uploader_class) @@ -76,7 +95,7 @@ describe SendFileUpload do it 'sends a file with a custom type' do headers = double - expected_headers = %r(response-content-disposition=attachment%3Bfilename%3D%22test.js%22&response-content-type=application/ecmascript) + expected_headers = %r(response-content-disposition=attachment%3B%20filename%3D%22test.js%22%3B%20filename%2A%3DUTF-8%27%27test.js&response-content-type=application/ecmascript) expect(Gitlab::Workhorse).to receive(:send_url).with(expected_headers).and_call_original expect(headers).to receive(:store).with(Gitlab::Workhorse::SEND_DATA_HEADER, /^send-url:/) diff --git a/spec/controllers/omniauth_callbacks_controller_spec.rb b/spec/controllers/omniauth_callbacks_controller_spec.rb index 59463462e5a..232a5e2793b 100644 --- a/spec/controllers/omniauth_callbacks_controller_spec.rb +++ b/spec/controllers/omniauth_callbacks_controller_spec.rb @@ -45,6 +45,29 @@ describe OmniauthCallbacksController, type: :controller do end end + context 'when sign in fails' do + include RoutesHelpers + + let(:extern_uid) { 'my-uid' } + let(:provider) { :saml } + + def stub_route_as(path) + allow(@routes).to receive(:generate_extras) { [path, []] } + end + + it 'it calls through to the failure handler' do + request.env['omniauth.error'] = OneLogin::RubySaml::ValidationError.new("Fingerprint mismatch") + request.env['omniauth.error.strategy'] = OmniAuth::Strategies::SAML.new(nil) + stub_route_as('/users/auth/saml/callback') + + ForgeryProtection.with_forgery_protection do + post :failure + end + + expect(flash[:alert]).to match(/Fingerprint mismatch/) + end + end + context 'when a redirect fragment is provided' do let(:provider) { :jwt } let(:extern_uid) { 'my-uid' } diff --git a/spec/controllers/projects/artifacts_controller_spec.rb b/spec/controllers/projects/artifacts_controller_spec.rb index bd10de45b67..29df00e6bb0 100644 --- a/spec/controllers/projects/artifacts_controller_spec.rb +++ b/spec/controllers/projects/artifacts_controller_spec.rb @@ -26,8 +26,15 @@ describe Projects::ArtifactsController do end context 'when no file type is supplied' do + let(:filename) { job.artifacts_file.filename } + it 'sends the artifacts file' do - expect(controller).to receive(:send_file).with(job.artifacts_file.path, hash_including(disposition: 'attachment')).and_call_original + # Notice the filename= is omitted from the disposition; this is because + # Rails 5 will append this header in send_file + expect(controller).to receive(:send_file) + .with( + job.artifacts_file.file.path, + hash_including(disposition: %Q(attachment; filename*=UTF-8''#{filename}))).and_call_original download_artifact end @@ -46,6 +53,7 @@ describe Projects::ArtifactsController do context 'when codequality file type is supplied' do let(:file_type) { 'codequality' } + let(:filename) { job.job_artifacts_codequality.filename } context 'when file is stored locally' do before do @@ -53,7 +61,11 @@ describe Projects::ArtifactsController do end it 'sends the codequality report' do - expect(controller).to receive(:send_file).with(job.job_artifacts_codequality.file.path, hash_including(disposition: 'attachment')).and_call_original + # Notice the filename= is omitted from the disposition; this is because + # Rails 5 will append this header in send_file + expect(controller).to receive(:send_file) + .with(job.job_artifacts_codequality.file.path, + hash_including(disposition: %Q(attachment; filename*=UTF-8''#{filename}))).and_call_original download_artifact(file_type: file_type) end diff --git a/spec/features/markdown/markdown_spec.rb b/spec/features/markdown/markdown_spec.rb index 3b37ede8579..8815643ca96 100644 --- a/spec/features/markdown/markdown_spec.rb +++ b/spec/features/markdown/markdown_spec.rb @@ -13,7 +13,7 @@ require 'erb' # # Raw Markdown # -> `markdown` helper -# -> Redcarpet::Render::GitlabHTML converts Markdown to HTML +# -> CommonMark::Render::GitlabHTML converts Markdown to HTML # -> Post-process HTML # -> `gfm` helper # -> HTML::Pipeline @@ -324,31 +324,6 @@ describe 'GitLab Markdown', :aggregate_failures do end end - context 'Redcarpet documents' do - before do - allow_any_instance_of(Banzai::Filter::MarkdownFilter).to receive(:engine).and_return('Redcarpet') - @html = markdown(@feat.raw_markdown) - end - - it 'processes certain elements differently' do - aggregate_failures 'parses superscript' do - expect(doc).to have_selector('sup', count: 3) - end - - aggregate_failures 'permits style attribute in th elements' do - expect(doc.at_css('th:contains("Header")')['style']).to eq 'text-align: center' - expect(doc.at_css('th:contains("Row")')['style']).to eq 'text-align: right' - expect(doc.at_css('th:contains("Example")')['style']).to eq 'text-align: left' - end - - aggregate_failures 'permits style attribute in td elements' do - expect(doc.at_css('td:contains("Foo")')['style']).to eq 'text-align: center' - expect(doc.at_css('td:contains("Bar")')['style']).to eq 'text-align: right' - expect(doc.at_css('td:contains("Baz")')['style']).to eq 'text-align: left' - end - end - end - # Fake a `current_user` helper def current_user @feat.user diff --git a/spec/features/projects/artifacts/user_downloads_artifacts_spec.rb b/spec/features/projects/artifacts/user_downloads_artifacts_spec.rb index 554f0b49052..5cb015e80be 100644 --- a/spec/features/projects/artifacts/user_downloads_artifacts_spec.rb +++ b/spec/features/projects/artifacts/user_downloads_artifacts_spec.rb @@ -7,7 +7,7 @@ describe "User downloads artifacts" do shared_examples "downloading" do it "downloads the zip" do - expect(page.response_headers["Content-Disposition"]).to eq(%Q{attachment; filename="#{job.artifacts_file.filename}"}) + expect(page.response_headers["Content-Disposition"]).to eq(%Q{attachment; filename*=UTF-8''#{job.artifacts_file.filename}; filename="#{job.artifacts_file.filename}"}) expect(page.response_headers['Content-Transfer-Encoding']).to eq("binary") expect(page.response_headers['Content-Type']).to eq("application/zip") expect(page.source.b).to eq(job.artifacts_file.file.read.b) diff --git a/spec/features/projects/blobs/blob_show_spec.rb b/spec/features/projects/blobs/blob_show_spec.rb index e2f9e7e9cc5..3edcc7ac2cd 100644 --- a/spec/features/projects/blobs/blob_show_spec.rb +++ b/spec/features/projects/blobs/blob_show_spec.rb @@ -5,8 +5,8 @@ describe 'File blob', :js do let(:project) { create(:project, :public, :repository) } - def visit_blob(path, anchor: nil, ref: 'master', legacy_render: nil) - visit project_blob_path(project, File.join(ref, path), anchor: anchor, legacy_render: legacy_render) + def visit_blob(path, anchor: nil, ref: 'master') + visit project_blob_path(project, File.join(ref, path), anchor: anchor) wait_for_requests end @@ -171,21 +171,6 @@ describe 'File blob', :js do end end end - - context 'when rendering legacy markdown' do - before do - visit_blob('files/commonmark/file.md', legacy_render: 1) - - wait_for_requests - end - - it 'renders using RedCarpet' do - aggregate_failures do - expect(page).to have_content("sublist") - expect(page).to have_xpath("//ol//li//ul") - end - end - end end context 'Markdown file (stored in LFS)' do diff --git a/spec/features/projects/blobs/edit_spec.rb b/spec/features/projects/blobs/edit_spec.rb index d5b20605860..6e6c299ee2e 100644 --- a/spec/features/projects/blobs/edit_spec.rb +++ b/spec/features/projects/blobs/edit_spec.rb @@ -81,17 +81,6 @@ describe 'Editing file blob', :js do expect(page).to have_content("sublist") expect(page).not_to have_xpath("//ol//li//ul") end - - it 'renders content with RedCarpet when legacy_render is set' do - visit project_edit_blob_path(project, tree_join(branch, readme_file_path), legacy_render: 1) - fill_editor(content: "1. one\\n - sublist\\n") - click_link 'Preview' - wait_for_requests - - # the above generates a sublist list in RedCarpet - expect(page).to have_content("sublist") - expect(page).to have_xpath("//ol//li//ul") - end end end diff --git a/spec/features/projects/jobs_spec.rb b/spec/features/projects/jobs_spec.rb index 24830b2bd3e..65ce872363f 100644 --- a/spec/features/projects/jobs_spec.rb +++ b/spec/features/projects/jobs_spec.rb @@ -220,7 +220,7 @@ describe 'Jobs', :clean_gitlab_redis_shared_state do artifact_request = requests.find { |req| req.url.match(%r{artifacts/download}) } - expect(artifact_request.response_headers["Content-Disposition"]).to eq(%Q{attachment; filename="#{job.artifacts_file.filename}"}) + expect(artifact_request.response_headers["Content-Disposition"]).to eq(%Q{attachment; filename*=UTF-8''#{job.artifacts_file.filename}; filename="#{job.artifacts_file.filename}"}) expect(artifact_request.response_headers['Content-Transfer-Encoding']).to eq("binary") expect(artifact_request.response_headers['Content-Type']).to eq("image/gif") expect(artifact_request.body).to eq(job.artifacts_file.file.read.b) diff --git a/spec/features/projects/wiki/markdown_preview_spec.rb b/spec/features/projects/wiki/markdown_preview_spec.rb index f505023d1d0..3b469fee867 100644 --- a/spec/features/projects/wiki/markdown_preview_spec.rb +++ b/spec/features/projects/wiki/markdown_preview_spec.rb @@ -175,20 +175,6 @@ describe 'Projects > Wiki > User previews markdown changes', :js do expect(page).to have_content("sublist") expect(page).not_to have_xpath("//ol//li//ul") end - - it 'renders content with RedCarpet when legacy_render is set' do - wiki_page = create(:wiki_page, - wiki: project.wiki, - attrs: { title: 'home', content: "Empty content" }) - visit(project_wiki_edit_path(project, wiki_page, legacy_render: 1)) - - fill_in :wiki_content, with: "1. one\n - sublist\n" - click_on "Preview" - - # the above generates a sublist list in RedCarpet - expect(page).to have_content("sublist") - expect(page).to have_xpath("//ol//li//ul") - end end end diff --git a/spec/features/snippets/show_spec.rb b/spec/features/snippets/show_spec.rb index 367a479f62a..eb974c7c7fd 100644 --- a/spec/features/snippets/show_spec.rb +++ b/spec/features/snippets/show_spec.rb @@ -80,19 +80,6 @@ describe 'Snippet', :js do end end - context 'when rendering legacy markdown' do - before do - visit snippet_path(snippet, legacy_render: 1) - - wait_for_requests - end - - it 'renders using RedCarpet' do - expect(page).to have_content("sublist") - expect(page).to have_xpath("//ol//li//ul") - end - end - context 'with cached CommonMark html' do let(:snippet) { create(:personal_snippet, :public, file_name: file_name, content: content, cached_markdown_version: CacheMarkdownField::CACHE_COMMONMARK_VERSION) } @@ -100,14 +87,6 @@ describe 'Snippet', :js do expect(page).not_to have_xpath("//ol//li//ul") end end - - context 'with cached Redcarpet html' do - let(:snippet) { create(:personal_snippet, :public, file_name: file_name, content: content, cached_markdown_version: CacheMarkdownField::CACHE_REDCARPET_VERSION) } - - it 'renders correctly' do - expect(page).to have_xpath("//ol//li//ul") - end - end end context 'switching to the simple viewer' do diff --git a/spec/features/task_lists_spec.rb b/spec/features/task_lists_spec.rb index b549f2b5c62..6fe840dccf6 100644 --- a/spec/features/task_lists_spec.rb +++ b/spec/features/task_lists_spec.rb @@ -36,19 +36,6 @@ describe 'Task Lists' do MARKDOWN end - let(:nested_tasks_markdown_redcarpet) do - <<-EOT.strip_heredoc - - [ ] Task a - - [x] Task a.1 - - [ ] Task a.2 - - [ ] Task b - - 1. [ ] Task 1 - 1. [ ] Task 1.1 - 1. [x] Task 1.2 - EOT - end - let(:nested_tasks_markdown) do <<-EOT.strip_heredoc - [ ] Task a @@ -153,61 +140,6 @@ describe 'Task Lists' do expect(page).to have_content("1 of 1 task completed") end end - - shared_examples 'shared nested tasks' do - before do - allow(Banzai::Filter::MarkdownFilter).to receive(:engine).and_return('Redcarpet') - visit_issue(project, issue) - end - it 'renders' do - expect(page).to have_selector('ul.task-list', count: 2) - expect(page).to have_selector('li.task-list-item', count: 7) - expect(page).to have_selector('ul input[checked]', count: 1) - expect(page).to have_selector('ol input[checked]', count: 1) - end - - it 'solves tasks' do - expect(page).to have_content("2 of 7 tasks completed") - - page.find('li.task-list-item', text: 'Task b').find('input').click - wait_for_requests - page.find('li.task-list-item ul li.task-list-item', text: 'Task a.2').find('input').click - wait_for_requests - page.find('li.task-list-item ol li.task-list-item', text: 'Task 1.1').find('input').click - wait_for_requests - - expect(page).to have_content("5 of 7 tasks completed") - - visit_issue(project, issue) # reload to see new system notes - - expect(page).to have_content('marked the task Task b as complete') - expect(page).to have_content('marked the task Task a.2 as complete') - expect(page).to have_content('marked the task Task 1.1 as complete') - end - end - - describe 'nested tasks', :js do - let(:cache_version) { CacheMarkdownField::CACHE_COMMONMARK_VERSION } - let!(:issue) do - create(:issue, description: nested_tasks_markdown, author: user, project: project, - cached_markdown_version: cache_version) - end - - before do - visit_issue(project, issue) - end - - context 'with Redcarpet' do - let(:cache_version) { CacheMarkdownField::CACHE_REDCARPET_VERSION } - let(:nested_tasks_markdown) { nested_tasks_markdown_redcarpet } - - it_behaves_like 'shared nested tasks' - end - - context 'with CommonMark' do - it_behaves_like 'shared nested tasks' - end - end end describe 'for Notes' do diff --git a/spec/fixtures/markdown.md.erb b/spec/fixtures/markdown.md.erb index e5d01c3bd03..bbeacf1707b 100644 --- a/spec/fixtures/markdown.md.erb +++ b/spec/fixtures/markdown.md.erb @@ -6,7 +6,7 @@ started. ## Markdown -GitLab uses [Redcarpet](http://git.io/ld_NVQ) to parse all Markdown into +GitLab uses [Commonmark](https://git.io/fhDag) to parse all Markdown into HTML. It has some special features. Let's try 'em out! diff --git a/spec/helpers/issuables_helper_spec.rb b/spec/helpers/issuables_helper_spec.rb index af319e5ebfe..8b82dea2524 100644 --- a/spec/helpers/issuables_helper_spec.rb +++ b/spec/helpers/issuables_helper_spec.rb @@ -188,7 +188,6 @@ describe IssuablesHelper do issuableRef: "##{issue.iid}", markdownPreviewPath: "/#{@project.full_path}/preview_markdown", markdownDocsPath: '/help/user/markdown', - markdownVersion: CacheMarkdownField::CACHE_COMMONMARK_VERSION, issuableTemplates: [], lockVersion: issue.lock_version, projectPath: @project.path, diff --git a/spec/helpers/markup_helper_spec.rb b/spec/helpers/markup_helper_spec.rb index a0c0af94fa5..c3956ba08fd 100644 --- a/spec/helpers/markup_helper_spec.rb +++ b/spec/helpers/markup_helper_spec.rb @@ -212,17 +212,6 @@ describe MarkupHelper do helper.render_wiki_content(@wiki) end - it 'uses Wiki pipeline for markdown files with RedCarpet if feature disabled' do - stub_feature_flags(commonmark_for_repositories: false) - allow(@wiki).to receive(:format).and_return(:markdown) - - expect(helper).to receive(:markdown_unsafe).with('wiki content', - pipeline: :wiki, project: project, project_wiki: @wiki, page_slug: "nested/page", - issuable_state_filter_enabled: true, markdown_engine: :redcarpet) - - helper.render_wiki_content(@wiki) - end - it "uses Asciidoctor for asciidoc files" do allow(@wiki).to receive(:format).and_return(:asciidoc) @@ -273,16 +262,6 @@ describe MarkupHelper do it 'defaults to CommonMark' do expect(helper.markup('foo.md', 'x^2')).to include('x^2') end - - it 'honors markdown_engine for RedCarpet' do - expect(helper.markup('foo.md', 'x^2', { markdown_engine: :redcarpet })).to include('x<sup>2</sup>') - end - - it 'uses RedCarpet if feature disabled' do - stub_feature_flags(commonmark_for_repositories: false) - - expect(helper.markup('foo.md', 'x^2', { markdown_engine: :redcarpet })).to include('x<sup>2</sup>') - end end describe '#first_line_in_markdown' do diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb index 990750f0b2f..49895b0680b 100644 --- a/spec/helpers/projects_helper_spec.rb +++ b/spec/helpers/projects_helper_spec.rb @@ -535,18 +535,6 @@ describe ProjectsHelper do end end - describe '#legacy_render_context' do - it 'returns the redcarpet engine' do - params = { legacy_render: '1' } - - expect(helper.legacy_render_context(params)).to include(markdown_engine: :redcarpet) - end - - it 'returns nothing' do - expect(helper.legacy_render_context({})).to be_empty - end - end - describe '#explore_projects_tab?' do subject { helper.explore_projects_tab? } diff --git a/spec/javascripts/diffs/components/tree_list_spec.js b/spec/javascripts/diffs/components/tree_list_spec.js index 08b0b4f9e45..c5ef48a81e9 100644 --- a/spec/javascripts/diffs/components/tree_list_spec.js +++ b/spec/javascripts/diffs/components/tree_list_spec.js @@ -83,17 +83,6 @@ describe('Diffs tree list component', () => { expect(vm.$el.querySelectorAll('.file-row')[1].textContent).toContain('app'); }); - it('filters tree list to blobs matching search', done => { - vm.search = 'app/index'; - - vm.$nextTick(() => { - expect(vm.$el.querySelectorAll('.file-row').length).toBe(1); - expect(vm.$el.querySelectorAll('.file-row')[0].textContent).toContain('index.js'); - - done(); - }); - }); - it('calls toggleTreeOpen when clicking folder', () => { spyOn(vm.$store, 'dispatch').and.stub(); @@ -130,14 +119,4 @@ describe('Diffs tree list component', () => { }); }); }); - - describe('clearSearch', () => { - it('resets search', () => { - vm.search = 'test'; - - vm.$el.querySelector('.tree-list-clear-icon').click(); - - expect(vm.search).toBe(''); - }); - }); }); diff --git a/spec/javascripts/diffs/store/getters_spec.js b/spec/javascripts/diffs/store/getters_spec.js index 190ca1230ca..4f69dc92ab8 100644 --- a/spec/javascripts/diffs/store/getters_spec.js +++ b/spec/javascripts/diffs/store/getters_spec.js @@ -242,7 +242,11 @@ describe('Diffs Module Getters', () => { }, }; - expect(getters.allBlobs(localState)).toEqual([ + expect( + getters.allBlobs(localState, { + flatBlobsList: getters.flatBlobsList(localState), + }), + ).toEqual([ { isHeader: true, path: '/', diff --git a/spec/javascripts/ide/components/ide_spec.js b/spec/javascripts/ide/components/ide_spec.js index 55f40be0e4e..dc5790f6562 100644 --- a/spec/javascripts/ide/components/ide_spec.js +++ b/spec/javascripts/ide/components/ide_spec.js @@ -1,5 +1,4 @@ import Vue from 'vue'; -import Mousetrap from 'mousetrap'; import store from '~/ide/stores'; import ide from '~/ide/components/ide.vue'; import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper'; @@ -72,73 +71,6 @@ describe('ide component', () => { }); }); - describe('file finder', () => { - beforeEach(done => { - spyOn(vm, 'toggleFileFinder'); - - vm.$store.state.fileFindVisible = true; - - vm.$nextTick(done); - }); - - it('calls toggleFileFinder on `t` key press', done => { - Mousetrap.trigger('t'); - - vm.$nextTick() - .then(() => { - expect(vm.toggleFileFinder).toHaveBeenCalled(); - }) - .then(done) - .catch(done.fail); - }); - - it('calls toggleFileFinder on `command+p` key press', done => { - Mousetrap.trigger('command+p'); - - vm.$nextTick() - .then(() => { - expect(vm.toggleFileFinder).toHaveBeenCalled(); - }) - .then(done) - .catch(done.fail); - }); - - it('calls toggleFileFinder on `ctrl+p` key press', done => { - Mousetrap.trigger('ctrl+p'); - - vm.$nextTick() - .then(() => { - expect(vm.toggleFileFinder).toHaveBeenCalled(); - }) - .then(done) - .catch(done.fail); - }); - - it('always allows `command+p` to trigger toggleFileFinder', () => { - expect( - vm.mousetrapStopCallback(null, vm.$el.querySelector('.dropdown-input-field'), 'command+p'), - ).toBe(false); - }); - - it('always allows `ctrl+p` to trigger toggleFileFinder', () => { - expect( - vm.mousetrapStopCallback(null, vm.$el.querySelector('.dropdown-input-field'), 'ctrl+p'), - ).toBe(false); - }); - - it('onlys handles `t` when focused in input-field', () => { - expect( - vm.mousetrapStopCallback(null, vm.$el.querySelector('.dropdown-input-field'), 't'), - ).toBe(true); - }); - - it('stops callback in monaco editor', () => { - setFixtures('<div class="inputarea"></div>'); - - expect(vm.mousetrapStopCallback(null, document.querySelector('.inputarea'), 't')).toBe(true); - }); - }); - it('shows error message when set', done => { expect(vm.$el.querySelector('.flash-container')).toBe(null); diff --git a/spec/javascripts/ide/components/file_finder/index_spec.js b/spec/javascripts/vue_shared/components/file_finder/index_spec.js index 15ef8c31f91..bae4741f652 100644 --- a/spec/javascripts/ide/components/file_finder/index_spec.js +++ b/spec/javascripts/vue_shared/components/file_finder/index_spec.js @@ -1,54 +1,51 @@ import Vue from 'vue'; -import store from '~/ide/stores'; -import FindFileComponent from '~/ide/components/file_finder/index.vue'; +import Mousetrap from 'mousetrap'; +import FindFileComponent from '~/vue_shared/components/file_finder/index.vue'; import { UP_KEY_CODE, DOWN_KEY_CODE, ENTER_KEY_CODE, ESC_KEY_CODE } from '~/lib/utils/keycodes'; -import router from '~/ide/ide_router'; -import { file, resetStore } from '../../helpers'; -import { mountComponentWithStore } from '../../../helpers/vue_mount_component_helper'; +import { file } from 'spec/ide/helpers'; +import timeoutPromise from 'spec/helpers/set_timeout_promise_helper'; -describe('IDE File finder item spec', () => { +describe('File finder item spec', () => { const Component = Vue.extend(FindFileComponent); let vm; - beforeEach(done => { - setFixtures('<div id="app"></div>'); - - vm = mountComponentWithStore(Component, { - store, - el: '#app', - props: { - index: 0, + function createComponent(props) { + vm = new Component({ + propsData: { + files: [], + visible: true, + loading: false, + ...props, }, }); - setTimeout(done); + vm.$mount('#app'); + } + + beforeEach(() => { + setFixtures('<div id="app"></div>'); }); afterEach(() => { vm.$destroy(); - - resetStore(vm.$store); }); describe('with entries', () => { beforeEach(done => { - Vue.set(vm.$store.state.entries, 'folder', { - ...file('folder'), - path: 'folder', - type: 'folder', - }); - - Vue.set(vm.$store.state.entries, 'index.js', { - ...file('index.js'), - path: 'index.js', - type: 'blob', - url: '/index.jsurl', - }); - - Vue.set(vm.$store.state.entries, 'component.js', { - ...file('component.js'), - path: 'component.js', - type: 'blob', + createComponent({ + files: [ + { + ...file('index.js'), + path: 'index.js', + type: 'blob', + url: '/index.jsurl', + }, + { + ...file('component.js'), + path: 'component.js', + type: 'blob', + }, + ], }); setTimeout(done); @@ -56,13 +53,14 @@ describe('IDE File finder item spec', () => { it('renders list of blobs', () => { expect(vm.$el.textContent).toContain('index.js'); + expect(vm.$el.textContent).toContain('component.js'); expect(vm.$el.textContent).not.toContain('folder'); }); it('filters entries', done => { vm.searchText = 'index'; - vm.$nextTick(() => { + setTimeout(() => { expect(vm.$el.textContent).toContain('index.js'); expect(vm.$el.textContent).not.toContain('component.js'); @@ -73,8 +71,8 @@ describe('IDE File finder item spec', () => { it('shows clear button when searchText is not empty', done => { vm.searchText = 'index'; - vm.$nextTick(() => { - expect(vm.$el.querySelector('.dropdown-input-clear').classList).toContain('show'); + setTimeout(() => { + expect(vm.$el.querySelector('.dropdown-input').classList).toContain('has-value'); expect(vm.$el.querySelector('.dropdown-input-search').classList).toContain('hidden'); done(); @@ -84,11 +82,11 @@ describe('IDE File finder item spec', () => { it('clear button resets searchText', done => { vm.searchText = 'index'; - vm.$nextTick() + timeoutPromise() .then(() => { vm.$el.querySelector('.dropdown-input-clear').click(); }) - .then(vm.$nextTick) + .then(timeoutPromise) .then(() => { expect(vm.searchText).toBe(''); }) @@ -100,11 +98,11 @@ describe('IDE File finder item spec', () => { spyOn(vm.$refs.searchInput, 'focus'); vm.searchText = 'index'; - vm.$nextTick() + timeoutPromise() .then(() => { vm.$el.querySelector('.dropdown-input-clear').click(); }) - .then(vm.$nextTick) + .then(timeoutPromise) .then(() => { expect(vm.$refs.searchInput.focus).toHaveBeenCalled(); }) @@ -116,7 +114,7 @@ describe('IDE File finder item spec', () => { it('returns 1 when no filtered entries exist', done => { vm.searchText = 'testing 123'; - vm.$nextTick(() => { + setTimeout(() => { expect(vm.listShowCount).toBe(1); done(); @@ -136,7 +134,7 @@ describe('IDE File finder item spec', () => { it('returns 33 when entries dont exist', done => { vm.searchText = 'testing 123'; - vm.$nextTick(() => { + setTimeout(() => { expect(vm.listHeight).toBe(33); done(); @@ -148,7 +146,7 @@ describe('IDE File finder item spec', () => { it('returns length of filtered blobs', done => { vm.searchText = 'index'; - vm.$nextTick(() => { + setTimeout(() => { expect(vm.filteredBlobsLength).toBe(1); done(); @@ -162,7 +160,7 @@ describe('IDE File finder item spec', () => { vm.focusedIndex = 1; vm.searchText = 'test'; - vm.$nextTick(() => { + setTimeout(() => { expect(vm.focusedIndex).toBe(0); done(); @@ -170,16 +168,16 @@ describe('IDE File finder item spec', () => { }); }); - describe('fileFindVisible', () => { + describe('visible', () => { it('returns searchText when false', done => { vm.searchText = 'test'; - vm.$store.state.fileFindVisible = true; + vm.visible = true; - vm.$nextTick() + timeoutPromise() .then(() => { - vm.$store.state.fileFindVisible = false; + vm.visible = false; }) - .then(vm.$nextTick) + .then(timeoutPromise) .then(() => { expect(vm.searchText).toBe(''); }) @@ -191,20 +189,19 @@ describe('IDE File finder item spec', () => { describe('openFile', () => { beforeEach(() => { - spyOn(router, 'push'); - spyOn(vm, 'toggleFileFinder'); + spyOn(vm, '$emit'); }); it('closes file finder', () => { - vm.openFile(vm.$store.state.entries['index.js']); + vm.openFile(vm.files[0]); - expect(vm.toggleFileFinder).toHaveBeenCalled(); + expect(vm.$emit).toHaveBeenCalledWith('toggle', false); }); it('pushes to router', () => { - vm.openFile(vm.$store.state.entries['index.js']); + vm.openFile(vm.files[0]); - expect(router.push).toHaveBeenCalledWith('/project/index.jsurl'); + expect(vm.$emit).toHaveBeenCalledWith('click', vm.files[0]); }); }); @@ -217,8 +214,8 @@ describe('IDE File finder item spec', () => { vm.$refs.searchInput.dispatchEvent(event); - vm.$nextTick(() => { - expect(vm.openFile).toHaveBeenCalledWith(vm.$store.state.entries['index.js']); + setTimeout(() => { + expect(vm.openFile).toHaveBeenCalledWith(vm.files[0]); done(); }); @@ -228,12 +225,12 @@ describe('IDE File finder item spec', () => { const event = new CustomEvent('keyup'); event.keyCode = ESC_KEY_CODE; - spyOn(vm, 'toggleFileFinder'); + spyOn(vm, '$emit'); vm.$refs.searchInput.dispatchEvent(event); - vm.$nextTick(() => { - expect(vm.toggleFileFinder).toHaveBeenCalled(); + setTimeout(() => { + expect(vm.$emit).toHaveBeenCalledWith('toggle', false); done(); }); @@ -287,18 +284,85 @@ describe('IDE File finder item spec', () => { }); describe('without entries', () => { - it('renders loading text when loading', done => { - store.state.loading = true; - - vm.$nextTick(() => { - expect(vm.$el.textContent).toContain('Loading...'); - - done(); + it('renders loading text when loading', () => { + createComponent({ + loading: true, }); + + expect(vm.$el.textContent).toContain('Loading...'); }); it('renders no files text', () => { + createComponent(); + expect(vm.$el.textContent).toContain('No files found.'); }); }); + + describe('keyboard shortcuts', () => { + beforeEach(done => { + createComponent(); + + spyOn(vm, 'toggle'); + + vm.$nextTick(done); + }); + + it('calls toggle on `t` key press', done => { + Mousetrap.trigger('t'); + + vm.$nextTick() + .then(() => { + expect(vm.toggle).toHaveBeenCalled(); + }) + .then(done) + .catch(done.fail); + }); + + it('calls toggle on `command+p` key press', done => { + Mousetrap.trigger('command+p'); + + vm.$nextTick() + .then(() => { + expect(vm.toggle).toHaveBeenCalled(); + }) + .then(done) + .catch(done.fail); + }); + + it('calls toggle on `ctrl+p` key press', done => { + Mousetrap.trigger('ctrl+p'); + + vm.$nextTick() + .then(() => { + expect(vm.toggle).toHaveBeenCalled(); + }) + .then(done) + .catch(done.fail); + }); + + it('always allows `command+p` to trigger toggle', () => { + expect( + vm.mousetrapStopCallback(null, vm.$el.querySelector('.dropdown-input-field'), 'command+p'), + ).toBe(false); + }); + + it('always allows `ctrl+p` to trigger toggle', () => { + expect( + vm.mousetrapStopCallback(null, vm.$el.querySelector('.dropdown-input-field'), 'ctrl+p'), + ).toBe(false); + }); + + it('onlys handles `t` when focused in input-field', () => { + expect( + vm.mousetrapStopCallback(null, vm.$el.querySelector('.dropdown-input-field'), 't'), + ).toBe(true); + }); + + it('stops callback in monaco editor', () => { + setFixtures('<div class="inputarea"></div>'); + + expect(vm.mousetrapStopCallback(null, document.querySelector('.inputarea'), 't')).toBe(true); + }); + }); }); diff --git a/spec/javascripts/ide/components/file_finder/item_spec.js b/spec/javascripts/vue_shared/components/file_finder/item_spec.js index 0f1116c6912..c1511643a9d 100644 --- a/spec/javascripts/ide/components/file_finder/item_spec.js +++ b/spec/javascripts/vue_shared/components/file_finder/item_spec.js @@ -1,9 +1,9 @@ import Vue from 'vue'; -import ItemComponent from '~/ide/components/file_finder/item.vue'; -import { file } from '../../helpers'; +import ItemComponent from '~/vue_shared/components/file_finder/item.vue'; +import { file } from 'spec/ide/helpers'; import createComponent from '../../../helpers/vue_mount_component_helper'; -describe('IDE File finder item spec', () => { +describe('File finder item spec', () => { const Component = Vue.extend(ItemComponent); let vm; let localFile; diff --git a/spec/lib/api/helpers_spec.rb b/spec/lib/api/helpers_spec.rb index e1aea82653d..08165f147bb 100644 --- a/spec/lib/api/helpers_spec.rb +++ b/spec/lib/api/helpers_spec.rb @@ -179,7 +179,7 @@ describe API::Helpers do context 'when blob name is not null' do it 'returns disposition with the blob name' do - expect(send_git_blob['Content-Disposition']).to eq 'inline; filename="foobar"' + expect(send_git_blob['Content-Disposition']).to eq %q(inline; filename="foobar"; filename*=UTF-8''foobar) end end end diff --git a/spec/lib/banzai/filter/markdown_filter_spec.rb b/spec/lib/banzai/filter/markdown_filter_spec.rb index 4c4e821deab..83fcda29680 100644 --- a/spec/lib/banzai/filter/markdown_filter_spec.rb +++ b/spec/lib/banzai/filter/markdown_filter_spec.rb @@ -10,12 +10,6 @@ describe Banzai::Filter::MarkdownFilter do filter('test') end - it 'uses Redcarpet' do - expect_any_instance_of(Banzai::Filter::MarkdownEngines::Redcarpet).to receive(:render).and_return('test') - - filter('test', { markdown_engine: :redcarpet }) - end - it 'uses CommonMark' do expect_any_instance_of(Banzai::Filter::MarkdownEngines::CommonMark).to receive(:render).and_return('test') @@ -47,24 +41,6 @@ describe Banzai::Filter::MarkdownFilter do expect(result).to start_with('<pre><code lang="日">') end end - - context 'using Redcarpet' do - before do - stub_const('Banzai::Filter::MarkdownFilter::DEFAULT_ENGINE', :redcarpet) - end - - it 'adds language to lang attribute when specified' do - result = filter("```html\nsome code\n```") - - expect(result).to start_with("\n<pre><code lang=\"html\">") - end - - it 'does not add language to lang attribute when not specified' do - result = filter("```\nsome code\n```") - - expect(result).to start_with("\n<pre><code>") - end - end end describe 'source line position' do @@ -85,18 +61,6 @@ describe Banzai::Filter::MarkdownFilter do expect(result).to eq '<p>test</p>' end end - - context 'using Redcarpet' do - before do - stub_const('Banzai::Filter::MarkdownFilter::DEFAULT_ENGINE', :redcarpet) - end - - it 'does not support data-sourcepos' do - result = filter('test') - - expect(result).to eq '<p>test</p>' - end - end end describe 'footnotes in tables' do diff --git a/spec/lib/banzai/filter/spaced_link_filter_spec.rb b/spec/lib/banzai/filter/spaced_link_filter_spec.rb index 1ad7f3ff567..76d7644d76c 100644 --- a/spec/lib/banzai/filter/spaced_link_filter_spec.rb +++ b/spec/lib/banzai/filter/spaced_link_filter_spec.rb @@ -26,11 +26,6 @@ describe Banzai::Filter::SpacedLinkFilter do expect(doc.at_css('p')).to be_nil end - it 'does nothing when markdown_engine is redcarpet' do - exp = act = link - expect(filter(act, markdown_engine: :redcarpet).to_html).to eq exp - end - it 'does nothing with empty text' do link = '[](page slug)' doc = filter("See #{link}") diff --git a/spec/models/application_record_spec.rb b/spec/models/application_record_spec.rb index 68aed387bfc..ca23f581fdc 100644 --- a/spec/models/application_record_spec.rb +++ b/spec/models/application_record_spec.rb @@ -10,4 +10,14 @@ describe ApplicationRecord do expect(User.id_in(records.last(2).map(&:id))).to eq(records.last(2)) end end + + describe '#safe_find_or_create_by' do + it 'creates the user avoiding race conditions' do + expect(Suggestion).to receive(:find_or_create_by).and_raise(ActiveRecord::RecordNotUnique) + allow(Suggestion).to receive(:find_or_create_by).and_call_original + + expect { Suggestion.safe_find_or_create_by(build(:suggestion).attributes) } + .to change { Suggestion.count }.by(1) + end + end end diff --git a/spec/models/concerns/cache_markdown_field_spec.rb b/spec/models/concerns/cache_markdown_field_spec.rb index 925e2ab0955..29197ef372e 100644 --- a/spec/models/concerns/cache_markdown_field_spec.rb +++ b/spec/models/concerns/cache_markdown_field_spec.rb @@ -73,6 +73,7 @@ describe CacheMarkdownField do let(:updated_html) { '<p dir="auto"><code>Bar</code></p>' } let(:thing) { ThingWithMarkdownFields.new(foo: markdown, foo_html: html, cached_markdown_version: CacheMarkdownField::CACHE_COMMONMARK_VERSION) } + let(:cache_version) { CacheMarkdownField::CACHE_COMMONMARK_VERSION } before do stub_commonmark_sourcepos_disabled @@ -97,20 +98,15 @@ describe CacheMarkdownField do end context 'a changed markdown field' do - shared_examples 'with cache version' do |cache_version| - let(:thing) { ThingWithMarkdownFields.new(foo: markdown, foo_html: html, cached_markdown_version: cache_version) } + let(:thing) { ThingWithMarkdownFields.new(foo: markdown, foo_html: html, cached_markdown_version: cache_version) } - before do - thing.foo = updated_markdown - thing.save - end - - it { expect(thing.foo_html).to eq(updated_html) } - it { expect(thing.cached_markdown_version).to eq(cache_version) } + before do + thing.foo = updated_markdown + thing.save end - it_behaves_like 'with cache version', CacheMarkdownField::CACHE_REDCARPET_VERSION - it_behaves_like 'with cache version', CacheMarkdownField::CACHE_COMMONMARK_VERSION + it { expect(thing.foo_html).to eq(updated_html) } + it { expect(thing.cached_markdown_version).to eq(cache_version) } end context 'when a markdown field is set repeatedly to an empty string' do @@ -143,22 +139,17 @@ describe CacheMarkdownField do end context 'a non-markdown field changed' do - shared_examples 'with cache version' do |cache_version| - let(:thing) { ThingWithMarkdownFields.new(foo: markdown, foo_html: html, cached_markdown_version: cache_version) } - - before do - thing.bar = 'OK' - thing.save - end + let(:thing) { ThingWithMarkdownFields.new(foo: markdown, foo_html: html, cached_markdown_version: cache_version) } - it { expect(thing.bar).to eq('OK') } - it { expect(thing.foo).to eq(markdown) } - it { expect(thing.foo_html).to eq(html) } - it { expect(thing.cached_markdown_version).to eq(cache_version) } + before do + thing.bar = 'OK' + thing.save end - it_behaves_like 'with cache version', CacheMarkdownField::CACHE_REDCARPET_VERSION - it_behaves_like 'with cache version', CacheMarkdownField::CACHE_COMMONMARK_VERSION + it { expect(thing.bar).to eq('OK') } + it { expect(thing.foo).to eq(markdown) } + it { expect(thing.foo_html).to eq(html) } + it { expect(thing.cached_markdown_version).to eq(cache_version) } end context 'version is out of date' do @@ -173,73 +164,63 @@ describe CacheMarkdownField do end describe '#cached_html_up_to_date?' do - shared_examples 'with cache version' do |cache_version| - let(:thing) { ThingWithMarkdownFields.new(foo: markdown, foo_html: html, cached_markdown_version: cache_version) } + let(:thing) { ThingWithMarkdownFields.new(foo: markdown, foo_html: html, cached_markdown_version: cache_version) } - subject { thing.cached_html_up_to_date?(:foo) } + subject { thing.cached_html_up_to_date?(:foo) } - it 'returns false when the version is absent' do - thing.cached_markdown_version = nil + it 'returns false when the version is absent' do + thing.cached_markdown_version = nil - is_expected.to be_falsy - end + is_expected.to be_falsy + end - it 'returns false when the version is too early' do - thing.cached_markdown_version -= 1 + it 'returns false when the version is too early' do + thing.cached_markdown_version -= 1 - is_expected.to be_falsy - end + is_expected.to be_falsy + end - it 'returns false when the version is too late' do - thing.cached_markdown_version += 1 + it 'returns false when the version is too late' do + thing.cached_markdown_version += 1 - is_expected.to be_falsy - end + is_expected.to be_falsy + end - it 'returns true when the version is just right' do - thing.cached_markdown_version = cache_version + it 'returns true when the version is just right' do + thing.cached_markdown_version = cache_version - is_expected.to be_truthy - end + is_expected.to be_truthy + end - it 'returns false if markdown has been changed but html has not' do - thing.foo = updated_html + it 'returns false if markdown has been changed but html has not' do + thing.foo = updated_html - is_expected.to be_falsy - end + is_expected.to be_falsy + end - it 'returns true if markdown has not been changed but html has' do - thing.foo_html = updated_html + it 'returns true if markdown has not been changed but html has' do + thing.foo_html = updated_html - is_expected.to be_truthy - end + is_expected.to be_truthy + end - it 'returns true if markdown and html have both been changed' do - thing.foo = updated_markdown - thing.foo_html = updated_html + it 'returns true if markdown and html have both been changed' do + thing.foo = updated_markdown + thing.foo_html = updated_html - is_expected.to be_truthy - end + is_expected.to be_truthy + end - it 'returns false if the markdown field is set but the html is not' do - thing.foo_html = nil + it 'returns false if the markdown field is set but the html is not' do + thing.foo_html = nil - is_expected.to be_falsy - end + is_expected.to be_falsy end - - it_behaves_like 'with cache version', CacheMarkdownField::CACHE_REDCARPET_VERSION - it_behaves_like 'with cache version', CacheMarkdownField::CACHE_COMMONMARK_VERSION end describe '#latest_cached_markdown_version' do subject { thing.latest_cached_markdown_version } - it 'returns redcarpet version' do - thing.cached_markdown_version = CacheMarkdownField::CACHE_COMMONMARK_VERSION_START - 1 - is_expected.to eq(CacheMarkdownField::CACHE_REDCARPET_VERSION) - end - it 'returns commonmark version' do thing.cached_markdown_version = CacheMarkdownField::CACHE_COMMONMARK_VERSION_START + 1 is_expected.to eq(CacheMarkdownField::CACHE_COMMONMARK_VERSION) @@ -251,30 +232,6 @@ describe CacheMarkdownField do end end - describe '#legacy_markdown?' do - subject { thing.legacy_markdown? } - - it 'returns true for redcarpet versions' do - thing.cached_markdown_version = CacheMarkdownField::CACHE_COMMONMARK_VERSION_START - 1 - is_expected.to be_truthy - end - - it 'returns false for commonmark versions' do - thing.cached_markdown_version = CacheMarkdownField::CACHE_COMMONMARK_VERSION_START - is_expected.to be_falsey - end - - it 'returns false if nil' do - thing.cached_markdown_version = nil - is_expected.to be_falsey - end - - it 'returns false if 0' do - thing.cached_markdown_version = 0 - is_expected.to be_falsey - end - end - describe '#refresh_markdown_cache' do before do thing.foo = updated_markdown @@ -303,39 +260,34 @@ describe CacheMarkdownField do end describe '#refresh_markdown_cache!' do - shared_examples 'with cache version' do |cache_version| - let(:thing) { ThingWithMarkdownFields.new(foo: markdown, foo_html: html, cached_markdown_version: cache_version) } + let(:thing) { ThingWithMarkdownFields.new(foo: markdown, foo_html: html, cached_markdown_version: cache_version) } - before do - thing.foo = updated_markdown - end + before do + thing.foo = updated_markdown + end - it 'fills all html fields' do - thing.refresh_markdown_cache! + it 'fills all html fields' do + thing.refresh_markdown_cache! - expect(thing.foo_html).to eq(updated_html) - expect(thing.foo_html_changed?).to be_truthy - expect(thing.baz_html_changed?).to be_truthy - end + expect(thing.foo_html).to eq(updated_html) + expect(thing.foo_html_changed?).to be_truthy + expect(thing.baz_html_changed?).to be_truthy + end - it 'skips saving if not persisted' do - expect(thing).to receive(:persisted?).and_return(false) - expect(thing).not_to receive(:update_columns) + it 'skips saving if not persisted' do + expect(thing).to receive(:persisted?).and_return(false) + expect(thing).not_to receive(:update_columns) - thing.refresh_markdown_cache! - end + thing.refresh_markdown_cache! + end - it 'saves the changes using #update_columns' do - expect(thing).to receive(:persisted?).and_return(true) - expect(thing).to receive(:update_columns) - .with("foo_html" => updated_html, "baz_html" => "", "cached_markdown_version" => cache_version) + it 'saves the changes using #update_columns' do + expect(thing).to receive(:persisted?).and_return(true) + expect(thing).to receive(:update_columns) + .with("foo_html" => updated_html, "baz_html" => "", "cached_markdown_version" => cache_version) - thing.refresh_markdown_cache! - end + thing.refresh_markdown_cache! end - - it_behaves_like 'with cache version', CacheMarkdownField::CACHE_REDCARPET_VERSION - it_behaves_like 'with cache version', CacheMarkdownField::CACHE_COMMONMARK_VERSION end describe '#banzai_render_context' do @@ -408,20 +360,4 @@ describe CacheMarkdownField do end end end - - describe CacheMarkdownField::MarkdownEngine do - subject { lambda { |version| CacheMarkdownField::MarkdownEngine.from_version(version) } } - - it 'returns :common_mark as a default' do - expect(subject.call(nil)).to eq :common_mark - end - - it 'returns :common_mark' do - expect(subject.call(CacheMarkdownField::CACHE_COMMONMARK_VERSION)).to eq :common_mark - end - - it 'returns :redcarpet' do - expect(subject.call(CacheMarkdownField::CACHE_REDCARPET_VERSION)).to eq :redcarpet - end - end end diff --git a/spec/requests/api/files_spec.rb b/spec/requests/api/files_spec.rb index 9b32dc78274..1ad536258ba 100644 --- a/spec/requests/api/files_spec.rb +++ b/spec/requests/api/files_spec.rb @@ -191,7 +191,7 @@ describe API::Files do get api(url, current_user), params: params - expect(headers['Content-Disposition']).to eq('inline; filename="popen.rb"') + expect(headers['Content-Disposition']).to eq(%q(inline; filename="popen.rb"; filename*=UTF-8''popen.rb)) end context 'when mandatory params are not given' do diff --git a/spec/requests/api/jobs_spec.rb b/spec/requests/api/jobs_spec.rb index 97aa71bf231..3defe8bbf51 100644 --- a/spec/requests/api/jobs_spec.rb +++ b/spec/requests/api/jobs_spec.rb @@ -403,7 +403,7 @@ describe API::Jobs do shared_examples 'downloads artifact' do let(:download_headers) do { 'Content-Transfer-Encoding' => 'binary', - 'Content-Disposition' => 'attachment; filename=ci_build_artifacts.zip' } + 'Content-Disposition' => %q(attachment; filename="ci_build_artifacts.zip"; filename*=UTF-8''ci_build_artifacts.zip) } end it 'returns specific job artifacts' do @@ -555,7 +555,7 @@ describe API::Jobs do let(:download_headers) do { 'Content-Transfer-Encoding' => 'binary', 'Content-Disposition' => - "attachment; filename=#{job.artifacts_file.filename}" } + %Q(attachment; filename="#{job.artifacts_file.filename}"; filename*=UTF-8''#{job.artifacts_file.filename}) } end it { expect(response).to have_http_status(:ok) } diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb index ed0108c846a..d7ddd97e8c8 100644 --- a/spec/requests/api/runner_spec.rb +++ b/spec/requests/api/runner_spec.rb @@ -1584,7 +1584,7 @@ describe API::Runner, :clean_gitlab_redis_shared_state do context 'when artifacts are stored locally' do let(:download_headers) do { 'Content-Transfer-Encoding' => 'binary', - 'Content-Disposition' => 'attachment; filename=ci_build_artifacts.zip' } + 'Content-Disposition' => %q(attachment; filename="ci_build_artifacts.zip"; filename*=UTF-8''ci_build_artifacts.zip) } end before do diff --git a/spec/services/preview_markdown_service_spec.rb b/spec/services/preview_markdown_service_spec.rb index 458cb8f1f31..85515d548a7 100644 --- a/spec/services/preview_markdown_service_spec.rb +++ b/spec/services/preview_markdown_service_spec.rb @@ -114,23 +114,4 @@ describe PreviewMarkdownService do expect(result[:commands]).to eq 'Tags this commit to v1.2.3 with "Stable release".' end end - - it 'sets correct markdown engine' do - service = described_class.new(project, user, { markdown_version: CacheMarkdownField::CACHE_REDCARPET_VERSION }) - result = service.execute - - expect(result[:markdown_engine]).to eq :redcarpet - - service = described_class.new(project, user, { markdown_version: CacheMarkdownField::CACHE_COMMONMARK_VERSION }) - result = service.execute - - expect(result[:markdown_engine]).to eq :common_mark - end - - it 'honors the legacy_render parameter' do - service = described_class.new(project, user, { legacy_render: '1' }) - result = service.execute - - expect(result[:markdown_engine]).to eq :redcarpet - end end diff --git a/spec/services/task_list_toggle_service_spec.rb b/spec/services/task_list_toggle_service_spec.rb index 750ac4c40ba..cc64dd25085 100644 --- a/spec/services/task_list_toggle_service_spec.rb +++ b/spec/services/task_list_toggle_service_spec.rb @@ -3,7 +3,6 @@ require 'spec_helper' describe TaskListToggleService do - let(:sourcepos) { true } let(:markdown) do <<-EOT.strip_heredoc * [ ] Task 1 @@ -40,87 +39,47 @@ describe TaskListToggleService do EOT end - shared_examples 'task lists' do - it 'checks Task 1' do - toggler = described_class.new(markdown, markdown_html, - index: 1, toggle_as_checked: true, - line_source: '* [ ] Task 1', line_number: 1, - sourcepos: sourcepos) + it 'checks Task 1' do + toggler = described_class.new(markdown, markdown_html, + toggle_as_checked: true, + line_source: '* [ ] Task 1', line_number: 1) - expect(toggler.execute).to be_truthy - expect(toggler.updated_markdown.lines[0]).to eq "* [x] Task 1\n" - expect(toggler.updated_markdown_html).to include('disabled checked> Task 1') - end - - it 'unchecks Item 1' do - toggler = described_class.new(markdown, markdown_html, - index: 3, toggle_as_checked: false, - line_source: '1. [X] Item 1', line_number: 6, - sourcepos: sourcepos) - - expect(toggler.execute).to be_truthy - expect(toggler.updated_markdown.lines[5]).to eq "1. [ ] Item 1\n" - expect(toggler.updated_markdown_html).to include('disabled> Item 1') - end - - it 'returns false if line_source does not match the text' do - toggler = described_class.new(markdown, markdown_html, - index: 2, toggle_as_checked: false, - line_source: '* [x] Task Added', line_number: 2, - sourcepos: sourcepos) - - expect(toggler.execute).to be_falsey - end + expect(toggler.execute).to be_truthy + expect(toggler.updated_markdown.lines[0]).to eq "* [x] Task 1\n" + expect(toggler.updated_markdown_html).to include('disabled checked> Task 1') + end - it 'returns false if markdown is nil' do - toggler = described_class.new(nil, markdown_html, - index: 2, toggle_as_checked: false, - line_source: '* [x] Task Added', line_number: 2, - sourcepos: sourcepos) + it 'unchecks Item 1' do + toggler = described_class.new(markdown, markdown_html, + toggle_as_checked: false, + line_source: '1. [X] Item 1', line_number: 6) - expect(toggler.execute).to be_falsey - end + expect(toggler.execute).to be_truthy + expect(toggler.updated_markdown.lines[5]).to eq "1. [ ] Item 1\n" + expect(toggler.updated_markdown_html).to include('disabled> Item 1') + end - it 'returns false if markdown_html is nil' do - toggler = described_class.new(markdown, nil, - index: 2, toggle_as_checked: false, - line_source: '* [x] Task Added', line_number: 2, - sourcepos: sourcepos) + it 'returns false if line_source does not match the text' do + toggler = described_class.new(markdown, markdown_html, + toggle_as_checked: false, + line_source: '* [x] Task Added', line_number: 2) - expect(toggler.execute).to be_falsey - end + expect(toggler.execute).to be_falsey end - context 'when using sourcepos' do - it_behaves_like 'task lists' + it 'returns false if markdown is nil' do + toggler = described_class.new(nil, markdown_html, + toggle_as_checked: false, + line_source: '* [x] Task Added', line_number: 2) + + expect(toggler.execute).to be_falsey end - context 'when using checkbox indexing' do - let(:sourcepos) { false } - let(:markdown_html) do - <<-EOT.strip_heredoc - <ul class="task-list" dir="auto"> - <li class="task-list-item"> - <input type="checkbox" class="task-list-item-checkbox" disabled> Task 1 - </li> - <li class="task-list-item"> - <input type="checkbox" class="task-list-item-checkbox" disabled checked> Task 2 - </li> - </ul> - <p dir="auto">A paragraph</p> - <ol class="task-list" dir="auto"> - <li class="task-list-item"> - <input type="checkbox" class="task-list-item-checkbox" disabled checked> Item 1 - <ul class="task-list"> - <li class="task-list-item"> - <input type="checkbox" class="task-list-item-checkbox" disabled> Sub-item 1 - </li> - </ul> - </li> - </ol> - EOT - end + it 'returns false if markdown_html is nil' do + toggler = described_class.new(markdown, nil, + toggle_as_checked: false, + line_source: '* [x] Task Added', line_number: 2) - it_behaves_like 'task lists' + expect(toggler.execute).to be_falsey end end diff --git a/spec/support/shared_examples/controllers/repository_lfs_file_load_examples.rb b/spec/support/shared_examples/controllers/repository_lfs_file_load_examples.rb index a3d31e26498..982e0317f7f 100644 --- a/spec/support/shared_examples/controllers/repository_lfs_file_load_examples.rb +++ b/spec/support/shared_examples/controllers/repository_lfs_file_load_examples.rb @@ -28,7 +28,13 @@ shared_examples 'repository lfs file load' do end it 'serves the file' do - expect(controller).to receive(:send_file).with("#{LfsObjectUploader.root}/91/ef/f75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897", filename: filename, disposition: 'attachment') + # Notice the filename= is omitted from the disposition; this is because + # Rails 5 will append this header in send_file + expect(controller).to receive(:send_file) + .with( + "#{LfsObjectUploader.root}/91/ef/f75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897", + filename: filename, + disposition: %Q(attachment; filename*=UTF-8''#{filename})) subject @@ -56,7 +62,7 @@ shared_examples 'repository lfs file load' do file_uri = URI.parse(response.location) params = CGI.parse(file_uri.query) - expect(params["response-content-disposition"].first).to eq "attachment;filename=\"#{filename}\"" + expect(params["response-content-disposition"].first).to eq(%q(attachment; filename="lfs_object.iso"; filename*=UTF-8''lfs_object.iso)) end end end |