diff options
143 files changed, 2268 insertions, 475 deletions
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 66fbf3fce58..86bdb7a4643 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -618,7 +618,8 @@ karma: codequality: <<: *dedicated-no-docs-no-db-pull-cache-job - image: docker:latest + image: docker:stable + allow_failure: true # gitlab-org runners set `privileged: false` but we need to have it set to true # since we're using Docker in Docker tags: [] @@ -628,14 +629,15 @@ codequality: variables: SETUP_DB: "false" DOCKER_DRIVER: overlay2 - CODECLIMATE_FORMAT: json cache: {} dependencies: [] script: - - apk update && apk add jq - - ./scripts/codequality analyze -f json > raw_codeclimate.json || true - # The following line keeps only the fields used in the MR widget, reducing the JSON artifact size - - jq -c 'map({check_name,description,fingerprint,location})' raw_codeclimate.json > codeclimate.json + # Get the custom rubocop codeclimate image (https://gitlab.com/gitlab-org/codeclimate-rubocop/wikis/home) + - docker pull dev.gitlab.org:5005/gitlab/gitlab-build-images:gitlab-codeclimate-rubocop-0-52-1 + - docker tag dev.gitlab.org:5005/gitlab/gitlab-build-images:gitlab-codeclimate-rubocop-0-52-1 codeclimate/codeclimate-rubocop:gitlab-codeclimate-rubocop-0-52-1 + # Extract "MAJOR.MINOR" from CI_SERVER_VERSION and generate "MAJOR-MINOR-stable" for Security Products + - export SP_VERSION=$(echo "$CI_SERVER_VERSION" | sed 's/^\([0-9]*\)\.\([0-9]*\).*/\1-\2-stable/') + - docker run --env SOURCE_CODE="$PWD" --volume "$PWD":/code --volume /var/run/docker.sock:/var/run/docker.sock "registry.gitlab.com/gitlab-org/security-products/codequality:$SP_VERSION" /code artifacts: paths: [codeclimate.json] expire_in: 1 week diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a90a7fcdc2..6491905a1ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -191,7 +191,6 @@ entry. - Enable privileged mode for GitLab Runner. !17528 - Expose GITLAB_FEATURES as CI/CD variable (fixes #40994). - Upgrade GitLab Workhorse to 4.0.0. -- Allow CI/CD Jobs being grouped on version strings. - Add discussions API for Issues and Snippets. - Add one group board to Libre. - Add support for filtering by source and target branch to merge requests API. diff --git a/Gemfile.rails5.lock b/Gemfile.rails5.lock index ca86861255b..08ae3fb514c 100644 --- a/Gemfile.rails5.lock +++ b/Gemfile.rails5.lock @@ -97,7 +97,7 @@ GEM autoprefixer-rails (>= 5.2.1) sass (>= 3.3.4) bootstrap_form (2.7.0) - brakeman (3.6.2) + brakeman (4.2.1) browser (2.5.3) builder (3.2.3) bullet (5.5.1) @@ -400,7 +400,7 @@ GEM hipchat (1.5.4) httparty mimemagic - html-pipeline (2.6.0) + html-pipeline (2.7.1) activesupport (>= 2) nokogiri (>= 1.4) html2text (0.2.1) @@ -1013,7 +1013,7 @@ DEPENDENCIES binding_of_caller (~> 0.7.2) bootstrap-sass (~> 3.3.0) bootstrap_form (~> 2.7.0) - brakeman (~> 3.6.0) + brakeman (~> 4.2) browser (~> 2.2) bullet (~> 5.5.0) bundler-audit (~> 0.5.0) @@ -1084,7 +1084,7 @@ DEPENDENCIES hashie-forbidden_attributes health_check (~> 2.6.0) hipchat (~> 1.5.0) - html-pipeline (~> 2.6.0) + html-pipeline (~> 2.7.1) html2text httparty (~> 0.13.3) influxdb (~> 0.2) diff --git a/app/assets/javascripts/filtered_search/filtered_search_dropdown_manager.js b/app/assets/javascripts/filtered_search/filtered_search_dropdown_manager.js index e6390f0855b..d7e1de18d09 100644 --- a/app/assets/javascripts/filtered_search/filtered_search_dropdown_manager.js +++ b/app/assets/javascripts/filtered_search/filtered_search_dropdown_manager.js @@ -26,8 +26,8 @@ export default class FilteredSearchDropdownManager { this.filteredSearchInput = this.container.querySelector('.filtered-search'); this.page = page; this.groupsOnly = isGroup; - this.groupAncestor = isGroupAncestor; - this.isGroupDecendent = isGroupDecendent; + this.includeAncestorGroups = isGroupAncestor; + this.includeDescendantGroups = isGroupDecendent; this.setupMapping(); @@ -108,7 +108,19 @@ export default class FilteredSearchDropdownManager { } getLabelsEndpoint() { - const endpoint = `${this.baseEndpoint}/labels.json`; + let endpoint = `${this.baseEndpoint}/labels.json?`; + + if (this.groupsOnly) { + endpoint = `${endpoint}only_group_labels=true&`; + } + + if (this.includeAncestorGroups) { + endpoint = `${endpoint}include_ancestor_groups=true&`; + } + + if (this.includeDescendantGroups) { + endpoint = `${endpoint}include_descendant_groups=true`; + } return endpoint; } diff --git a/app/assets/javascripts/filtered_search/filtered_search_manager.js b/app/assets/javascripts/filtered_search/filtered_search_manager.js index 71b7e80335b..cf5ba1e1771 100644 --- a/app/assets/javascripts/filtered_search/filtered_search_manager.js +++ b/app/assets/javascripts/filtered_search/filtered_search_manager.js @@ -21,7 +21,7 @@ export default class FilteredSearchManager { constructor({ page, isGroup = false, - isGroupAncestor = false, + isGroupAncestor = true, isGroupDecendent = false, filteredSearchTokenKeys = FilteredSearchTokenKeys, stateFiltersSelector = '.issues-state-filters', @@ -86,6 +86,7 @@ export default class FilteredSearchManager { page: this.page, isGroup: this.isGroup, isGroupAncestor: this.isGroupAncestor, + isGroupDecendent: this.isGroupDecendent, filteredSearchTokenKeys: this.filteredSearchTokenKeys, }); diff --git a/app/assets/javascripts/ide/components/commit_sidebar/list_item.vue b/app/assets/javascripts/ide/components/commit_sidebar/list_item.vue index 18934af004a..560cdd941cd 100644 --- a/app/assets/javascripts/ide/components/commit_sidebar/list_item.vue +++ b/app/assets/javascripts/ide/components/commit_sidebar/list_item.vue @@ -1,38 +1,36 @@ <script> - import { mapActions } from 'vuex'; - import icon from '~/vue_shared/components/icon.vue'; - import router from '../../ide_router'; +import { mapActions } from 'vuex'; +import Icon from '~/vue_shared/components/icon.vue'; - export default { - components: { - icon, +export default { + components: { + Icon, + }, + props: { + file: { + type: Object, + required: true, }, - props: { - file: { - type: Object, - required: true, - }, + }, + computed: { + iconName() { + return this.file.tempFile ? 'file-addition' : 'file-modified'; }, - computed: { - iconName() { - return this.file.tempFile ? 'file-addition' : 'file-modified'; - }, - iconClass() { - return `multi-file-${this.file.tempFile ? 'addition' : 'modified'} append-right-8`; - }, + iconClass() { + return `multi-file-${this.file.tempFile ? 'addition' : 'modified'} append-right-8`; }, - methods: { - ...mapActions([ - 'discardFileChanges', - 'updateViewer', - ]), - openFileInEditor(file) { - this.updateViewer('diff'); - - router.push(`/project${file.url}`); - }, + }, + methods: { + ...mapActions(['discardFileChanges', 'updateViewer', 'openPendingTab']), + openFileInEditor(file) { + return this.openPendingTab(file).then(changeViewer => { + if (changeViewer) { + this.updateViewer('diff'); + } + }); }, - }; + }, +}; </script> <template> diff --git a/app/assets/javascripts/ide/components/ide.vue b/app/assets/javascripts/ide/components/ide.vue index 5e44af01241..d22869466c9 100644 --- a/app/assets/javascripts/ide/components/ide.vue +++ b/app/assets/javascripts/ide/components/ide.vue @@ -60,6 +60,7 @@ export default { v-if="activeFile" > <repo-tabs + :active-file="activeFile" :files="openFiles" :viewer="viewer" :has-changes="hasChanges" diff --git a/app/assets/javascripts/ide/components/repo_editor.vue b/app/assets/javascripts/ide/components/repo_editor.vue index b6f8f8a1c99..b1a16350c19 100644 --- a/app/assets/javascripts/ide/components/repo_editor.vue +++ b/app/assets/javascripts/ide/components/repo_editor.vue @@ -21,7 +21,8 @@ export default { }, watch: { file(oldVal, newVal) { - if (newVal.path !== this.file.path) { + // Compare key to allow for files opened in review mode to be cached differently + if (newVal.key !== this.file.key) { this.initMonaco(); } }, @@ -70,7 +71,7 @@ export default { }) .then(() => { const viewerPromise = this.delayViewerUpdated - ? this.updateViewer('editor') + ? this.updateViewer(this.file.pending ? 'diff' : 'editor') : Promise.resolve(); return viewerPromise; diff --git a/app/assets/javascripts/ide/components/repo_file.vue b/app/assets/javascripts/ide/components/repo_file.vue index 1935ee1a4bb..3b5068d4910 100644 --- a/app/assets/javascripts/ide/components/repo_file.vue +++ b/app/assets/javascripts/ide/components/repo_file.vue @@ -62,11 +62,7 @@ export default { this.toggleTreeOpen(this.file.path); } - const delayPromise = this.file.changed - ? Promise.resolve() - : this.updateDelayViewerUpdated(true); - - return delayPromise.then(() => { + return this.updateDelayViewerUpdated(true).then(() => { router.push(`/project${this.file.url}`); }); }, diff --git a/app/assets/javascripts/ide/components/repo_tab.vue b/app/assets/javascripts/ide/components/repo_tab.vue index c337bc813e6..304a73ed1ad 100644 --- a/app/assets/javascripts/ide/components/repo_tab.vue +++ b/app/assets/javascripts/ide/components/repo_tab.vue @@ -1,60 +1,64 @@ <script> - import { mapActions } from 'vuex'; +import { mapActions } from 'vuex'; - import fileIcon from '~/vue_shared/components/file_icon.vue'; - import icon from '~/vue_shared/components/icon.vue'; - import fileStatusIcon from './repo_file_status_icon.vue'; - import changedFileIcon from './changed_file_icon.vue'; +import FileIcon from '~/vue_shared/components/file_icon.vue'; +import Icon from '~/vue_shared/components/icon.vue'; +import FileStatusIcon from './repo_file_status_icon.vue'; +import ChangedFileIcon from './changed_file_icon.vue'; - export default { - components: { - fileStatusIcon, - fileIcon, - icon, - changedFileIcon, +export default { + components: { + FileStatusIcon, + FileIcon, + Icon, + ChangedFileIcon, + }, + props: { + tab: { + type: Object, + required: true, }, - props: { - tab: { - type: Object, - required: true, - }, + }, + data() { + return { + tabMouseOver: false, + }; + }, + computed: { + closeLabel() { + if (this.tab.changed || this.tab.tempFile) { + return `${this.tab.name} changed`; + } + return `Close ${this.tab.name}`; }, - data() { - return { - tabMouseOver: false, - }; - }, - computed: { - closeLabel() { - if (this.tab.changed || this.tab.tempFile) { - return `${this.tab.name} changed`; - } - return `Close ${this.tab.name}`; - }, - showChangedIcon() { - return this.tab.changed ? !this.tabMouseOver : false; - }, + showChangedIcon() { + return this.tab.changed ? !this.tabMouseOver : false; }, + }, + + methods: { + ...mapActions(['closeFile', 'updateDelayViewerUpdated', 'openPendingTab']), + clickFile(tab) { + this.updateDelayViewerUpdated(true); - methods: { - ...mapActions([ - 'closeFile', - ]), - clickFile(tab) { + if (tab.pending) { + this.openPendingTab(tab); + } else { this.$router.push(`/project${tab.url}`); - }, - mouseOverTab() { - if (this.tab.changed) { - this.tabMouseOver = true; - } - }, - mouseOutTab() { - if (this.tab.changed) { - this.tabMouseOver = false; - } - }, + } + }, + mouseOverTab() { + if (this.tab.changed) { + this.tabMouseOver = true; + } + }, + mouseOutTab() { + if (this.tab.changed) { + this.tabMouseOver = false; + } }, - }; + }, +}; </script> <template> @@ -66,7 +70,7 @@ <button type="button" class="multi-file-tab-close" - @click.stop.prevent="closeFile(tab.path)" + @click.stop.prevent="closeFile(tab)" :aria-label="closeLabel" > <icon @@ -82,7 +86,9 @@ <div class="multi-file-tab" - :class="{active : tab.active }" + :class="{ + active: tab.active + }" :title="tab.url" > <file-icon diff --git a/app/assets/javascripts/ide/components/repo_tabs.vue b/app/assets/javascripts/ide/components/repo_tabs.vue index a44e418b2eb..7bd646ba9b0 100644 --- a/app/assets/javascripts/ide/components/repo_tabs.vue +++ b/app/assets/javascripts/ide/components/repo_tabs.vue @@ -2,6 +2,7 @@ import { mapActions } from 'vuex'; import RepoTab from './repo_tab.vue'; import EditorMode from './editor_mode_dropdown.vue'; +import router from '../ide_router'; export default { components: { @@ -9,6 +10,10 @@ export default { EditorMode, }, props: { + activeFile: { + type: Object, + required: true, + }, files: { type: Array, required: true, @@ -38,7 +43,18 @@ export default { this.showShadow = this.$refs.tabsScroller.scrollWidth > this.$refs.tabsScroller.offsetWidth; }, methods: { - ...mapActions(['updateViewer']), + ...mapActions(['updateViewer', 'removePendingTab']), + openFileViewer(viewer) { + this.updateViewer(viewer); + + if (this.activeFile.pending) { + return this.removePendingTab(this.activeFile).then(() => { + router.push(`/project${this.activeFile.url}`); + }); + } + + return null; + }, }, }; </script> @@ -60,7 +76,7 @@ export default { :show-shadow="showShadow" :has-changes="hasChanges" :merge-request-id="mergeRequestId" - @click="updateViewer" + @click="openFileViewer" /> </div> </template> diff --git a/app/assets/javascripts/ide/ide_router.js b/app/assets/javascripts/ide/ide_router.js index be2c12c0487..20983666b4a 100644 --- a/app/assets/javascripts/ide/ide_router.js +++ b/app/assets/javascripts/ide/ide_router.js @@ -77,7 +77,11 @@ router.beforeEach((to, from, next) => { if (to.params[0]) { const path = to.params[0].slice(-1) === '/' ? to.params[0].slice(0, -1) : to.params[0]; - const treeEntry = store.state.entries[path]; + const treeEntryKey = Object.keys(store.state.entries).find( + key => key === path && !store.state.entries[key].pending, + ); + const treeEntry = store.state.entries[treeEntryKey]; + if (treeEntry) { store.dispatch('handleTreeEntryAction', treeEntry); } diff --git a/app/assets/javascripts/ide/lib/common/model.js b/app/assets/javascripts/ide/lib/common/model.js index d372c2aaad8..e47adae99ed 100644 --- a/app/assets/javascripts/ide/lib/common/model.js +++ b/app/assets/javascripts/ide/lib/common/model.js @@ -13,12 +13,12 @@ export default class Model { (this.originalModel = this.monaco.editor.createModel( this.file.raw, undefined, - new this.monaco.Uri(null, null, `original/${this.file.path}`), + new this.monaco.Uri(null, null, `original/${this.file.key}`), )), (this.model = this.monaco.editor.createModel( this.content, undefined, - new this.monaco.Uri(null, null, this.file.path), + new this.monaco.Uri(null, null, this.file.key), )), ); if (this.file.mrChange) { @@ -36,7 +36,7 @@ export default class Model { this.updateContent = this.updateContent.bind(this); this.dispose = this.dispose.bind(this); - eventHub.$on(`editor.update.model.dispose.${this.file.path}`, this.dispose); + eventHub.$on(`editor.update.model.dispose.${this.file.key}`, this.dispose); eventHub.$on(`editor.update.model.content.${this.file.path}`, this.updateContent); } @@ -53,7 +53,7 @@ export default class Model { } get path() { - return this.file.path; + return this.file.key; } getModel() { @@ -88,7 +88,7 @@ export default class Model { this.disposable.dispose(); this.events.clear(); - eventHub.$off(`editor.update.model.dispose.${this.file.path}`, this.dispose); + eventHub.$off(`editor.update.model.dispose.${this.file.key}`, this.dispose); eventHub.$off(`editor.update.model.content.${this.file.path}`, this.updateContent); } } diff --git a/app/assets/javascripts/ide/lib/common/model_manager.js b/app/assets/javascripts/ide/lib/common/model_manager.js index 57d5e59a88b..0e7b563b5d6 100644 --- a/app/assets/javascripts/ide/lib/common/model_manager.js +++ b/app/assets/javascripts/ide/lib/common/model_manager.js @@ -9,17 +9,17 @@ export default class ModelManager { this.models = new Map(); } - hasCachedModel(path) { - return this.models.has(path); + hasCachedModel(key) { + return this.models.has(key); } - getModel(path) { - return this.models.get(path); + getModel(key) { + return this.models.get(key); } addModel(file) { - if (this.hasCachedModel(file.path)) { - return this.getModel(file.path); + if (this.hasCachedModel(file.key)) { + return this.getModel(file.key); } const model = new Model(this.monaco, file); @@ -27,7 +27,7 @@ export default class ModelManager { this.disposable.add(model); eventHub.$on( - `editor.update.model.dispose.${file.path}`, + `editor.update.model.dispose.${file.key}`, this.removeCachedModel.bind(this, file), ); @@ -35,12 +35,9 @@ export default class ModelManager { } removeCachedModel(file) { - this.models.delete(file.path); + this.models.delete(file.key); - eventHub.$off( - `editor.update.model.dispose.${file.path}`, - this.removeCachedModel, - ); + eventHub.$off(`editor.update.model.dispose.${file.key}`, this.removeCachedModel); } dispose() { diff --git a/app/assets/javascripts/ide/stores/actions.js b/app/assets/javascripts/ide/stores/actions.js index 0a74f4f8925..c6ba679d99c 100644 --- a/app/assets/javascripts/ide/stores/actions.js +++ b/app/assets/javascripts/ide/stores/actions.js @@ -21,7 +21,7 @@ export const discardAllChanges = ({ state, commit, dispatch }) => { }; export const closeAllFiles = ({ state, dispatch }) => { - state.openFiles.forEach(file => dispatch('closeFile', file.path)); + state.openFiles.forEach(file => dispatch('closeFile', file)); }; export const setPanelCollapsedStatus = ({ commit }, { side, collapsed }) => { diff --git a/app/assets/javascripts/ide/stores/actions/file.js b/app/assets/javascripts/ide/stores/actions/file.js index c21c1a3f5d4..6b034ea1e82 100644 --- a/app/assets/javascripts/ide/stores/actions/file.js +++ b/app/assets/javascripts/ide/stores/actions/file.js @@ -6,24 +6,34 @@ import * as types from '../mutation_types'; import router from '../../ide_router'; import { setPageTitle } from '../utils'; -export const closeFile = ({ commit, state, getters, dispatch }, path) => { - const indexOfClosedFile = state.openFiles.findIndex(f => f.path === path); - const file = state.entries[path]; +export const closeFile = ({ commit, state, dispatch }, file) => { + const path = file.path; + const indexOfClosedFile = state.openFiles.findIndex(f => f.key === file.key); const fileWasActive = file.active; - commit(types.TOGGLE_FILE_OPEN, path); - commit(types.SET_FILE_ACTIVE, { path, active: false }); + if (file.pending) { + commit(types.REMOVE_PENDING_TAB, file); + } else { + commit(types.TOGGLE_FILE_OPEN, path); + commit(types.SET_FILE_ACTIVE, { path, active: false }); + } if (state.openFiles.length > 0 && fileWasActive) { const nextIndexToOpen = indexOfClosedFile === 0 ? 0 : indexOfClosedFile - 1; - const nextFileToOpen = state.entries[state.openFiles[nextIndexToOpen].path]; - - router.push(`/project${nextFileToOpen.url}`); + const nextFileToOpen = state.openFiles[nextIndexToOpen]; + + if (nextFileToOpen.pending) { + dispatch('updateViewer', 'diff'); + dispatch('openPendingTab', nextFileToOpen); + } else { + dispatch('updateDelayViewerUpdated', true); + router.push(`/project${nextFileToOpen.url}`); + } } else if (!state.openFiles.length) { router.push(`/project/${file.projectId}/tree/${file.branchId}/`); } - eventHub.$emit(`editor.update.model.dispose.${file.path}`); + eventHub.$emit(`editor.update.model.dispose.${file.key}`); }; export const setFileActive = ({ commit, state, getters, dispatch }, path) => { @@ -151,3 +161,23 @@ export const discardFileChanges = ({ state, commit }, path) => { eventHub.$emit(`editor.update.model.content.${file.path}`, file.raw); }; + +export const openPendingTab = ({ commit, getters, dispatch, state }, file) => { + if (getters.activeFile && getters.activeFile.path === file.path && state.viewer === 'diff') { + return false; + } + + commit(types.ADD_PENDING_TAB, { file }); + + dispatch('scrollToTab'); + + router.push(`/project/${file.projectId}/tree/${state.currentBranchId}/`); + + return true; +}; + +export const removePendingTab = ({ commit }, file) => { + commit(types.REMOVE_PENDING_TAB, file); + + eventHub.$emit(`editor.update.model.dispose.${file.key}`); +}; diff --git a/app/assets/javascripts/ide/stores/mutation_types.js b/app/assets/javascripts/ide/stores/mutation_types.js index e06be0a3fe9..ee759bff516 100644 --- a/app/assets/javascripts/ide/stores/mutation_types.js +++ b/app/assets/javascripts/ide/stores/mutation_types.js @@ -49,3 +49,6 @@ export const CREATE_TMP_ENTRY = 'CREATE_TMP_ENTRY'; export const SET_FILE_MERGE_REQUEST_CHANGE = 'SET_FILE_MERGE_REQUEST_CHANGE'; export const UPDATE_VIEWER = 'UPDATE_VIEWER'; export const UPDATE_DELAY_VIEWER_CHANGE = 'UPDATE_DELAY_VIEWER_CHANGE'; + +export const ADD_PENDING_TAB = 'ADD_PENDING_TAB'; +export const REMOVE_PENDING_TAB = 'REMOVE_PENDING_TAB'; diff --git a/app/assets/javascripts/ide/stores/mutations/file.js b/app/assets/javascripts/ide/stores/mutations/file.js index 692fe39b38e..926b6f66d78 100644 --- a/app/assets/javascripts/ide/stores/mutations/file.js +++ b/app/assets/javascripts/ide/stores/mutations/file.js @@ -5,6 +5,14 @@ export default { Object.assign(state.entries[path], { active, }); + + if (active && !state.entries[path].pending) { + Object.assign(state, { + openFiles: state.openFiles.map(f => + Object.assign(f, { active: f.pending ? false : f.active }), + ), + }); + } }, [types.TOGGLE_FILE_OPEN](state, path) { Object.assign(state.entries[path], { @@ -12,10 +20,14 @@ export default { }); if (state.entries[path].opened) { - state.openFiles.push(state.entries[path]); + Object.assign(state, { + openFiles: state.openFiles.filter(f => f.path !== path).concat(state.entries[path]), + }); } else { + const file = state.entries[path]; + Object.assign(state, { - openFiles: state.openFiles.filter(f => f.path !== path), + openFiles: state.openFiles.filter(f => f.key !== file.key), }); } }, @@ -92,4 +104,37 @@ export default { changed, }); }, + [types.ADD_PENDING_TAB](state, { file, keyPrefix = 'pending' }) { + const pendingTab = state.openFiles.find(f => f.path === file.path && f.pending); + let openFiles = state.openFiles.map(f => + Object.assign(f, { active: f.path === file.path, opened: false }), + ); + + if (!pendingTab) { + const openFile = openFiles.find(f => f.path === file.path); + + openFiles = openFiles.concat(openFile ? null : file).reduce((acc, f) => { + if (!f) return acc; + + if (f.path === file.path) { + return acc.concat({ + ...f, + active: true, + pending: true, + opened: true, + key: `${keyPrefix}-${f.key}`, + }); + } + + return acc.concat(f); + }, []); + } + + Object.assign(state, { openFiles }); + }, + [types.REMOVE_PENDING_TAB](state, file) { + Object.assign(state, { + openFiles: state.openFiles.filter(f => f.key !== file.key), + }); + }, }; diff --git a/app/assets/javascripts/ide/stores/utils.js b/app/assets/javascripts/ide/stores/utils.js index 3389eeeaa2e..63e4de3b17d 100644 --- a/app/assets/javascripts/ide/stores/utils.js +++ b/app/assets/javascripts/ide/stores/utils.js @@ -1,5 +1,7 @@ export const dataStructure = () => ({ id: '', + // Key will contain a mixture of ID and path + // it can also contain a prefix `pending-` for files opened in review mode key: '', type: '', projectId: '', diff --git a/app/assets/javascripts/pages/groups/issues/index.js b/app/assets/javascripts/pages/groups/issues/index.js index d149b307e7f..914f804fdd3 100644 --- a/app/assets/javascripts/pages/groups/issues/index.js +++ b/app/assets/javascripts/pages/groups/issues/index.js @@ -5,6 +5,7 @@ import { FILTERED_SEARCH } from '~/pages/constants'; document.addEventListener('DOMContentLoaded', () => { initFilteredSearch({ page: FILTERED_SEARCH.ISSUES, + isGroupDecendent: true, }); projectSelect(); }); diff --git a/app/assets/javascripts/pages/groups/merge_requests/index.js b/app/assets/javascripts/pages/groups/merge_requests/index.js index a5cc1f34b63..1600faa3611 100644 --- a/app/assets/javascripts/pages/groups/merge_requests/index.js +++ b/app/assets/javascripts/pages/groups/merge_requests/index.js @@ -5,6 +5,7 @@ import { FILTERED_SEARCH } from '~/pages/constants'; document.addEventListener('DOMContentLoaded', () => { initFilteredSearch({ page: FILTERED_SEARCH.MERGE_REQUESTS, + isGroupDecendent: true, }); projectSelect(); }); diff --git a/app/assets/stylesheets/pages/pipelines.scss b/app/assets/stylesheets/pages/pipelines.scss index 42772f13155..ce2f1482456 100644 --- a/app/assets/stylesheets/pages/pipelines.scss +++ b/app/assets/stylesheets/pages/pipelines.scss @@ -706,8 +706,8 @@ button.mini-pipeline-graph-dropdown-toggle { // dropdown content for big and mini pipeline .big-pipeline-graph-dropdown-menu, .mini-pipeline-graph-dropdown-menu { - width: 195px; - max-width: 195px; + width: 240px; + max-width: 240px; .scrollable-menu { padding: 0; @@ -750,7 +750,7 @@ button.mini-pipeline-graph-dropdown-toggle { height: #{$ci-action-icon-size - 6}; left: -3px; position: relative; - top: -2px; + top: -1px; &.icon-action-stop, &.icon-action-cancel { @@ -931,13 +931,11 @@ button.mini-pipeline-graph-dropdown-toggle { */ &.dropdown-menu { transform: translate(-80%, 0); - min-width: 150px; @media(min-width: $screen-md-min) { transform: translate(-50%, 0); right: auto; left: 50%; - min-width: 240px; } } } diff --git a/app/controllers/admin/groups_controller.rb b/app/controllers/admin/groups_controller.rb index cc38608eda5..001f6520093 100644 --- a/app/controllers/admin/groups_controller.rb +++ b/app/controllers/admin/groups_controller.rb @@ -5,7 +5,7 @@ class Admin::GroupsController < Admin::ApplicationController def index @groups = Group.with_statistics.with_route - @groups = @groups.sort(@sort = params[:sort]) + @groups = @groups.sort_by_attribute(@sort = params[:sort]) @groups = @groups.search(params[:name]) if params[:name].present? @groups = @groups.page(params[:page]) end diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index 156a8e2c515..bfeb5a2d097 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -4,7 +4,7 @@ class Admin::UsersController < Admin::ApplicationController def index @users = User.order_name_asc.filter(params[:filter]) @users = @users.search_with_secondary_emails(params[:search_query]) if params[:search_query].present? - @users = @users.sort(@sort = params[:sort]) + @users = @users.sort_by_attribute(@sort = params[:sort]) @users = @users.page(params[:page]) end diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 7f83bd10e93..24651dd392c 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -229,10 +229,6 @@ class ApplicationController < ActionController::Base @event_filter ||= EventFilter.new(filters) end - def gitlab_ldap_access(&block) - Gitlab::Auth::LDAP::Access.open { |access| yield(access) } - end - # JSON for infinite scroll via Pager object def pager_json(partial, count, locals = {}) html = render_to_string( diff --git a/app/controllers/concerns/group_tree.rb b/app/controllers/concerns/group_tree.rb index fafb10090ca..56770a17406 100644 --- a/app/controllers/concerns/group_tree.rb +++ b/app/controllers/concerns/group_tree.rb @@ -14,7 +14,7 @@ module GroupTree end @groups = @groups.with_selects_for_list(archived: params[:archived]) - .sort(@sort = params[:sort]) + .sort_by_attribute(@sort = params[:sort]) .page(params[:page]) respond_to do |format| diff --git a/app/controllers/groups/group_members_controller.rb b/app/controllers/groups/group_members_controller.rb index f210434b2d7..134b0dfc0db 100644 --- a/app/controllers/groups/group_members_controller.rb +++ b/app/controllers/groups/group_members_controller.rb @@ -17,7 +17,7 @@ class Groups::GroupMembersController < Groups::ApplicationController @members = GroupMembersFinder.new(@group).execute @members = @members.non_invite unless can?(current_user, :admin_group, @group) @members = @members.search(params[:search]) if params[:search].present? - @members = @members.sort(@sort) + @members = @members.sort_by_attribute(@sort) @members = @members.page(params[:page]).per(50) @members = present_members(@members.includes(:user)) diff --git a/app/controllers/profiles_controller.rb b/app/controllers/profiles_controller.rb index dbf61a17724..3d27ae18b17 100644 --- a/app/controllers/profiles_controller.rb +++ b/app/controllers/profiles_controller.rb @@ -51,7 +51,7 @@ class ProfilesController < Profiles::ApplicationController end def update_username - result = Users::UpdateService.new(current_user, user: @user, username: user_params[:username]).execute + result = Users::UpdateService.new(current_user, user: @user, username: username_param).execute options = if result[:status] == :success { notice: "Username successfully changed" } @@ -72,6 +72,10 @@ class ProfilesController < Profiles::ApplicationController return render_404 unless @user.can_change_username? end + def username_param + @username_param ||= user_params.require(:username) + end + def user_params @user_params ||= params.require(:user).permit( :avatar, diff --git a/app/controllers/projects/labels_controller.rb b/app/controllers/projects/labels_controller.rb index 516198b1b8a..91016f6494e 100644 --- a/app/controllers/projects/labels_controller.rb +++ b/app/controllers/projects/labels_controller.rb @@ -150,7 +150,8 @@ class Projects::LabelsController < Projects::ApplicationController end def find_labels - @available_labels ||= LabelsFinder.new(current_user, project_id: @project.id).execute + @available_labels ||= + LabelsFinder.new(current_user, project_id: @project.id, include_ancestor_groups: params[:include_ancestor_groups]).execute end def authorize_admin_labels! diff --git a/app/controllers/projects/milestones_controller.rb b/app/controllers/projects/milestones_controller.rb index e898136d203..c5a044541f1 100644 --- a/app/controllers/projects/milestones_controller.rb +++ b/app/controllers/projects/milestones_controller.rb @@ -14,7 +14,7 @@ class Projects::MilestonesController < Projects::ApplicationController def index @sort = params[:sort] || 'due_date_asc' - @milestones = milestones.sort(@sort) + @milestones = milestones.sort_by_attribute(@sort) respond_to do |format| format.html do diff --git a/app/controllers/projects/project_members_controller.rb b/app/controllers/projects/project_members_controller.rb index e9b4679f94c..cfa5e72af64 100644 --- a/app/controllers/projects/project_members_controller.rb +++ b/app/controllers/projects/project_members_controller.rb @@ -21,7 +21,7 @@ class Projects::ProjectMembersController < Projects::ApplicationController @group_links = @group_links.where(group_id: @project.invited_groups.search(params[:search]).select(:id)) end - @project_members = present_members(@project_members.sort(@sort).page(params[:page])) + @project_members = present_members(@project_members.sort_by_attribute(@sort).page(params[:page])) @requesters = present_members(AccessRequestsFinder.new(@project).execute(current_user)) @project_member = @project.project_members.new end diff --git a/app/controllers/projects/settings/repository_controller.rb b/app/controllers/projects/settings/repository_controller.rb index d06d18c498b..dd9e4a2af3e 100644 --- a/app/controllers/projects/settings/repository_controller.rb +++ b/app/controllers/projects/settings/repository_controller.rb @@ -16,6 +16,10 @@ module Projects @protected_tags = @project.protected_tags.order(:name).page(params[:page]) @protected_branch = @project.protected_branches.new @protected_tag = @project.protected_tags.new + + @protected_branches_count = @protected_branches.reduce(0) { |sum, branch| sum + branch.matching(@project.repository.branches).size } + @protected_tags_count = @protected_tags.reduce(0) { |sum, tag| sum + tag.matching(@project.repository.tags).size } + load_gon_index end diff --git a/app/finders/admin/projects_finder.rb b/app/finders/admin/projects_finder.rb index 2c8f21c2400..53b77f5fed9 100644 --- a/app/finders/admin/projects_finder.rb +++ b/app/finders/admin/projects_finder.rb @@ -62,6 +62,6 @@ class Admin::ProjectsFinder def sort(items) sort = params.fetch(:sort) { 'latest_activity_desc' } - items.sort(sort) + items.sort_by_attribute(sort) end end diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb index b2d4f9938ff..61c72aa22a8 100644 --- a/app/finders/issuable_finder.rb +++ b/app/finders/issuable_finder.rb @@ -337,7 +337,7 @@ class IssuableFinder def sort(items) # Ensure we always have an explicit sort order (instead of inheriting # multiple orders when combining ActiveRecord::Relation objects). - params[:sort] ? items.sort(params[:sort], excluded_labels: label_names) : items.reorder(id: :desc) + params[:sort] ? items.sort_by_attribute(params[:sort], excluded_labels: label_names) : items.reorder(id: :desc) end def by_assignee(items) diff --git a/app/finders/labels_finder.rb b/app/finders/labels_finder.rb index 780c0fdb03e..afd1f824b32 100644 --- a/app/finders/labels_finder.rb +++ b/app/finders/labels_finder.rb @@ -28,9 +28,10 @@ class LabelsFinder < UnionFinder if project if project.group.present? labels_table = Label.arel_table + group_ids = group_ids_for(project.group) label_ids << Label.where( - labels_table[:type].eq('GroupLabel').and(labels_table[:group_id].eq(project.group.id)).or( + labels_table[:type].eq('GroupLabel').and(labels_table[:group_id].in(group_ids)).or( labels_table[:type].eq('ProjectLabel').and(labels_table[:project_id].eq(project.id)) ) ) @@ -38,11 +39,14 @@ class LabelsFinder < UnionFinder label_ids << project.labels end end - elsif only_group_labels? - label_ids << Label.where(group_id: group_ids) else + if group? + group = Group.find(params[:group_id]) + label_ids << Label.where(group_id: group_ids_for(group)) + end + label_ids << Label.where(group_id: projects.group_ids) - label_ids << Label.where(project_id: projects.select(:id)) + label_ids << Label.where(project_id: projects.select(:id)) unless only_group_labels? end label_ids @@ -59,22 +63,33 @@ class LabelsFinder < UnionFinder items.where(title: title) end - def group_ids + # Gets redacted array of group ids + # which can include the ancestors and descendants of the requested group. + def group_ids_for(group) strong_memoize(:group_ids) do - groups_user_can_read_labels(groups_to_include).map(&:id) + groups = groups_to_include(group) + + groups_user_can_read_labels(groups).map(&:id) end end - def groups_to_include - group = Group.find(params[:group_id]) + def groups_to_include(group) groups = [group] - groups += group.ancestors if params[:include_ancestor_groups].present? - groups += group.descendants if params[:include_descendant_groups].present? + groups += group.ancestors if include_ancestor_groups? + groups += group.descendants if include_descendant_groups? groups end + def include_ancestor_groups? + params[:include_ancestor_groups] + end + + def include_descendant_groups? + params[:include_descendant_groups] + end + def group? params[:group_id].present? end diff --git a/app/finders/projects_finder.rb b/app/finders/projects_finder.rb index 005612ededc..c7d6bc6cfdc 100644 --- a/app/finders/projects_finder.rb +++ b/app/finders/projects_finder.rb @@ -124,7 +124,7 @@ class ProjectsFinder < UnionFinder end def sort(items) - params[:sort].present? ? items.sort(params[:sort]) : items.order_id_desc + params[:sort].present? ? items.sort_by_attribute(params[:sort]) : items.order_id_desc end def by_archived(projects) diff --git a/app/finders/todos_finder.rb b/app/finders/todos_finder.rb index 150f4c7688b..09e2c586f2a 100644 --- a/app/finders/todos_finder.rb +++ b/app/finders/todos_finder.rb @@ -119,7 +119,7 @@ class TodosFinder end def sort(items) - params[:sort] ? items.sort(params[:sort]) : items.order_id_desc + params[:sort] ? items.sort_by_attribute(params[:sort]) : items.order_id_desc end def by_action(items) diff --git a/app/helpers/boards_helper.rb b/app/helpers/boards_helper.rb index 275e892b2e6..af878bcf9a0 100644 --- a/app/helpers/boards_helper.rb +++ b/app/helpers/boards_helper.rb @@ -53,10 +53,12 @@ module BoardsHelper end def board_list_data + include_descendant_groups = @group&.present? + { toggle: "dropdown", - list_labels_path: labels_filter_path(true), - labels: labels_filter_path(true), + list_labels_path: labels_filter_path(true, include_ancestor_groups: true), + labels: labels_filter_path(true, include_descendant_groups: include_descendant_groups), labels_endpoint: @labels_endpoint, namespace_path: @namespace_path, project_path: @project&.path, diff --git a/app/helpers/labels_helper.rb b/app/helpers/labels_helper.rb index 87ff607dc3f..c4a6a1e4bb3 100644 --- a/app/helpers/labels_helper.rb +++ b/app/helpers/labels_helper.rb @@ -129,13 +129,17 @@ module LabelsHelper end end - def labels_filter_path(only_group_labels = false) + def labels_filter_path(only_group_labels = false, include_ancestor_groups: true, include_descendant_groups: false) project = @target_project || @project + options = {} + options[:include_ancestor_groups] = include_ancestor_groups if include_ancestor_groups + options[:include_descendant_groups] = include_descendant_groups if include_descendant_groups + if project - project_labels_path(project, :json) + project_labels_path(project, :json, options) elsif @group - options = { only_group_labels: only_group_labels } if only_group_labels + options[:only_group_labels] = only_group_labels if only_group_labels group_labels_path(@group, :json, options) else dashboard_labels_path(:json) diff --git a/app/models/commit.rb b/app/models/commit.rb index b64462fb768..3f7f36e83c0 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -32,7 +32,8 @@ class Commit COMMIT_SHA_PATTERN = /\h{#{MIN_SHA_LENGTH},40}/.freeze def banzai_render_context(field) - context = { pipeline: :single_line, project: self.project } + pipeline = field == :description ? :commit_description : :single_line + context = { pipeline: pipeline, project: self.project } context[:author] = self.author if self.author context diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index 9fb5b7efec6..3469d5d795c 100644 --- a/app/models/commit_status.rb +++ b/app/models/commit_status.rb @@ -141,7 +141,7 @@ class CommitStatus < ActiveRecord::Base end def group_name - name.to_s.gsub(%r{\d+[\.\s:/\\]+\d+\s*}, '').strip + name.to_s.gsub(%r{\d+[\s:/\\]+\d+\s*}, '').strip end def failed_but_allowed? diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 5a566f3ac02..b45395343cc 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -137,7 +137,7 @@ module Issuable fuzzy_search(query, [:title, :description]) end - def sort(method, excluded_labels: []) + def sort_by_attribute(method, excluded_labels: []) sorted = case method.to_s when 'downvotes_desc' then order_downvotes_desc diff --git a/app/models/concerns/milestoneish.rb b/app/models/concerns/milestoneish.rb index caf8afa97f9..5130ecec472 100644 --- a/app/models/concerns/milestoneish.rb +++ b/app/models/concerns/milestoneish.rb @@ -45,11 +45,11 @@ module Milestoneish end def sorted_issues(user) - issues_visible_to_user(user).preload_associations.sort('label_priority') + issues_visible_to_user(user).preload_associations.sort_by_attribute('label_priority') end def sorted_merge_requests - merge_requests.sort('label_priority') + merge_requests.sort_by_attribute('label_priority') end def upcoming? diff --git a/app/models/group.rb b/app/models/group.rb index d99af79b5fe..3cfe21ac93b 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -53,7 +53,7 @@ class Group < Namespace Gitlab::Database.postgresql? end - def sort(method) + def sort_by_attribute(method) if method == 'storage_size_desc' # storage_size is a virtual column so we need to # pass a string to avoid AR adding the table name diff --git a/app/models/issue.rb b/app/models/issue.rb index 6a94d60c828..13abc6c1a0d 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -116,7 +116,7 @@ class Issue < ActiveRecord::Base 'project_id' end - def self.sort(method, excluded_labels: []) + def self.sort_by_attribute(method, excluded_labels: []) case method.to_s when 'due_date' then order_due_date_asc when 'due_date_asc' then order_due_date_asc diff --git a/app/models/member.rb b/app/models/member.rb index e1a32148538..eac4a22a03f 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -96,7 +96,7 @@ class Member < ActiveRecord::Base joins(:user).merge(User.search(query)) end - def sort(method) + def sort_by_attribute(method) case method.to_s when 'access_level_asc' then reorder(access_level: :asc) when 'access_level_desc' then reorder(access_level: :desc) diff --git a/app/models/milestone.rb b/app/models/milestone.rb index e7d397f40f5..dafae58d121 100644 --- a/app/models/milestone.rb +++ b/app/models/milestone.rb @@ -138,7 +138,7 @@ class Milestone < ActiveRecord::Base User.joins(assigned_issues: :milestone).where("milestones.id = ?", id).uniq end - def self.sort(method) + def self.sort_by_attribute(method) case method.to_s when 'due_date_asc' reorder(Gitlab::Database.nulls_last_order('due_date', 'ASC')) diff --git a/app/models/project.rb b/app/models/project.rb index b343786d2c9..714a15ade9c 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -436,7 +436,7 @@ class Project < ActiveRecord::Base Gitlab::VisibilityLevel.options end - def sort(method) + def sort_by_attribute(method) case method.to_s when 'storage_size_desc' # storage_size is a joined column so we need to @@ -566,9 +566,7 @@ class Project < ActiveRecord::Base def add_import_job job_id = if forked? - RepositoryForkWorker.perform_async(id, - forked_from_project.repository_storage_path, - forked_from_project.disk_path) + RepositoryForkWorker.perform_async(id) elsif gitlab_project_import? # Do not retry on Import/Export until https://gitlab.com/gitlab-org/gitlab-ce/issues/26189 is solved. RepositoryImportWorker.set(retry: false).perform_async(self.id) diff --git a/app/models/todo.rb b/app/models/todo.rb index 8afacd188e0..a2ab405fdbe 100644 --- a/app/models/todo.rb +++ b/app/models/todo.rb @@ -50,7 +50,7 @@ class Todo < ActiveRecord::Base # Priority sorting isn't displayed in the dropdown, because we don't show # milestones, but still show something if the user has a URL with that # selected. - def sort(method) + def sort_by_attribute(method) sorted = case method.to_s when 'priority', 'label_priority' then order_by_labels_priority diff --git a/app/models/user.rb b/app/models/user.rb index f934b654225..ba51595e6a3 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -256,7 +256,7 @@ class User < ActiveRecord::Base end end - def sort(method) + def sort_by_attribute(method) order_method = method || 'id_desc' case order_method.to_s diff --git a/app/services/boards/lists/create_service.rb b/app/services/boards/lists/create_service.rb index bebc90c7a8d..02f1c709374 100644 --- a/app/services/boards/lists/create_service.rb +++ b/app/services/boards/lists/create_service.rb @@ -12,11 +12,15 @@ module Boards private def available_labels_for(board) + options = { include_ancestor_groups: true } + if board.group_board? - parent.labels + options.merge!(group_id: parent.id, only_group_labels: true) else - LabelsFinder.new(current_user, project_id: parent.id).execute + options[:project_id] = parent.id end + + LabelsFinder.new(current_user, options).execute end def next_position(board) diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index 02fb48108fb..91ec702fbc6 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -106,7 +106,7 @@ class IssuableBaseService < BaseService end def available_labels - @available_labels ||= LabelsFinder.new(current_user, project_id: @project.id).execute + @available_labels ||= LabelsFinder.new(current_user, project_id: @project.id, include_ancestor_groups: true).execute end def handle_quick_actions_on_create(issuable) diff --git a/app/services/projects/autocomplete_service.rb b/app/services/projects/autocomplete_service.rb index e61ecb696d0..346971138b1 100644 --- a/app/services/projects/autocomplete_service.rb +++ b/app/services/projects/autocomplete_service.rb @@ -21,7 +21,8 @@ module Projects end def labels(target = nil) - labels = LabelsFinder.new(current_user, project_id: project.id).execute.select([:color, :title]) + labels = LabelsFinder.new(current_user, project_id: project.id, include_ancestor_groups: true) + .execute.select([:color, :title]) return labels unless target&.respond_to?(:labels) diff --git a/app/services/projects/import_service.rb b/app/services/projects/import_service.rb index a3828acc50b..bdd9598f85a 100644 --- a/app/services/projects/import_service.rb +++ b/app/services/projects/import_service.rb @@ -61,7 +61,7 @@ module Projects project.ensure_repository project.repository.fetch_as_mirror(project.import_url, refmap: refmap) else - gitlab_shell.import_repository(project.repository_storage_path, project.disk_path, project.import_url) + gitlab_shell.import_repository(project.repository_storage, project.disk_path, project.import_url) end rescue Gitlab::Shell::Error, Gitlab::Git::RepositoryMirroring::RemoteError => e # Expire cache to prevent scenarios such as: diff --git a/app/services/quick_actions/interpret_service.rb b/app/services/quick_actions/interpret_service.rb index cba49faac31..6cc51b6ee1b 100644 --- a/app/services/quick_actions/interpret_service.rb +++ b/app/services/quick_actions/interpret_service.rb @@ -200,7 +200,7 @@ module QuickActions end params '~label1 ~"label 2"' condition do - available_labels = LabelsFinder.new(current_user, project_id: project.id).execute + available_labels = LabelsFinder.new(current_user, project_id: project.id, include_ancestor_groups: true).execute current_user.can?(:"admin_#{issuable.to_ability_name}", project) && available_labels.any? @@ -562,7 +562,7 @@ module QuickActions def find_labels(labels_param) extract_references(labels_param, :label) | - LabelsFinder.new(current_user, project_id: project.id, name: labels_param.split).execute + LabelsFinder.new(current_user, project_id: project.id, name: labels_param.split, include_ancestor_groups: true).execute end def find_label_references(labels_param) @@ -593,6 +593,7 @@ module QuickActions def extract_references(arg, type) ext = Gitlab::ReferenceExtractor.new(project, current_user) + ext.analyze(arg, author: current_user) ext.references(type) diff --git a/app/views/projects/commit/_commit_box.html.haml b/app/views/projects/commit/_commit_box.html.haml index 461129a3e0e..74c5317428c 100644 --- a/app/views/projects/commit/_commit_box.html.haml +++ b/app/views/projects/commit/_commit_box.html.haml @@ -49,10 +49,10 @@ .commit-box{ data: { project_path: project_path(@project) } } %h3.commit-title - = markdown(@commit.title, pipeline: :single_line, author: @commit.author) + = markdown_field(@commit, :title) - if @commit.description.present? %pre.commit-description - = preserve(markdown(@commit.description, pipeline: :single_line, author: @commit.author)) + = preserve(markdown_field(@commit, :description)) .info-well .well-segment.branch-info diff --git a/app/views/projects/commits/_commit.atom.builder b/app/views/projects/commits/_commit.atom.builder index 50f7e7a3a33..640b5ecf99e 100644 --- a/app/views/projects/commits/_commit.atom.builder +++ b/app/views/projects/commits/_commit.atom.builder @@ -10,5 +10,5 @@ xml.entry do xml.email commit.author_email end - xml.summary markdown(commit.description, pipeline: :single_line), type: 'html' + xml.summary markdown_field(commit, :description), type: 'html' end diff --git a/app/views/projects/protected_branches/shared/_branches_list.html.haml b/app/views/projects/protected_branches/shared/_branches_list.html.haml index 2a0704bc7af..a09c13176c3 100644 --- a/app/views/projects/protected_branches/shared/_branches_list.html.haml +++ b/app/views/projects/protected_branches/shared/_branches_list.html.haml @@ -2,7 +2,7 @@ - if @protected_branches.empty? .panel-heading %h3.panel-title - Protected branch (#{@protected_branches.size}) + Protected branch (#{@protected_branches_count}) %p.settings-message.text-center There are currently no protected branches, protect a branch with the form above. - else @@ -16,7 +16,7 @@ %col %thead %tr - %th Protected branch (#{@protected_branches.size}) + %th Protected branch (#{@protected_branches_count}) %th Last commit %th Allowed to merge %th Allowed to push diff --git a/app/views/projects/protected_tags/shared/_tags_list.html.haml b/app/views/projects/protected_tags/shared/_tags_list.html.haml index 3f42ae58438..02908e16dc5 100644 --- a/app/views/projects/protected_tags/shared/_tags_list.html.haml +++ b/app/views/projects/protected_tags/shared/_tags_list.html.haml @@ -2,7 +2,7 @@ - if @protected_tags.empty? .panel-heading %h3.panel-title - Protected tag (#{@protected_tags.size}) + Protected tag (#{@protected_tags_count}) %p.settings-message.text-center There are currently no protected tags, protect a tag with the form above. - else @@ -17,7 +17,7 @@ %col %thead %tr - %th Protected tag (#{@protected_tags.size}) + %th Protected tag (#{@protected_tags_count}) %th Last commit %th Allowed to create - if can_admin_project diff --git a/app/views/shared/issuable/_sidebar.html.haml b/app/views/shared/issuable/_sidebar.html.haml index 6afcd447f28..975b9cb4729 100644 --- a/app/views/shared/issuable/_sidebar.html.haml +++ b/app/views/shared/issuable/_sidebar.html.haml @@ -107,7 +107,7 @@ - selected_labels.each do |label| = hidden_field_tag "#{issuable.to_ability_name}[label_names][]", label.id, id: nil .dropdown - %button.dropdown-menu-toggle.js-label-select.js-multiselect.js-label-sidebar-dropdown{ type: "button", data: {toggle: "dropdown", default_label: "Labels", field_name: "#{issuable.to_ability_name}[label_names][]", ability_name: issuable.to_ability_name, show_no: "true", show_any: "true", namespace_path: @project.try(:namespace).try(:full_path), project_path: @project.try(:path), issue_update: issuable_json_path(issuable), labels: (project_labels_path(@project, :json) if @project) } } + %button.dropdown-menu-toggle.js-label-select.js-multiselect.js-label-sidebar-dropdown{ type: "button", data: {toggle: "dropdown", default_label: "Labels", field_name: "#{issuable.to_ability_name}[label_names][]", ability_name: issuable.to_ability_name, show_no: "true", show_any: "true", namespace_path: @project.try(:namespace).try(:full_path), project_path: @project.try(:path), issue_update: issuable_json_path(issuable), labels: (labels_filter_path(false) if @project) } } %span.dropdown-toggle-text{ class: ("is-default" if selected_labels.empty?) } = multi_label_name(selected_labels, "Labels") = icon('chevron-down', 'aria-hidden': 'true') diff --git a/app/workers/repository_fork_worker.rb b/app/workers/repository_fork_worker.rb index 712a63af532..51fad4faf36 100644 --- a/app/workers/repository_fork_worker.rb +++ b/app/workers/repository_fork_worker.rb @@ -1,28 +1,50 @@ -# Gitaly issue: https://gitlab.com/gitlab-org/gitaly/issues/1110 class RepositoryForkWorker include ApplicationWorker include Gitlab::ShellAdapter include ProjectStartImport include ProjectImportOptions - def perform(project_id, forked_from_repository_storage_path, source_disk_path) - project = Project.find(project_id) + def perform(*args) + target_project_id = args.shift + target_project = Project.find(target_project_id) - return unless start_fork(project) + # By v10.8, we should've drained the queue of all jobs using the old arguments. + # We can remove the else clause if we're no longer logging the message in that clause. + # See https://gitlab.com/gitlab-org/gitaly/issues/1110 + if args.empty? + source_project = target_project.forked_from_project + return target_project.mark_import_as_failed('Source project cannot be found.') unless source_project - Gitlab::Metrics.add_event(:fork_repository, - source_path: source_disk_path, - target_path: project.disk_path) + fork_repository(target_project, source_project.repository_storage, source_project.disk_path) + else + Rails.logger.info("Project #{target_project.id} is being forked using old-style arguments.") + + source_repository_storage_path, source_disk_path = *args - result = gitlab_shell.fork_repository(forked_from_repository_storage_path, source_disk_path, - project.repository_storage_path, project.disk_path) - raise "Unable to fork project #{project_id} for repository #{source_disk_path} -> #{project.disk_path}" unless result + source_repository_storage_name = Gitlab.config.repositories.storages.find do |_, info| + info.legacy_disk_path == source_repository_storage_path + end&.first || raise("no shard found for path '#{source_repository_storage_path}'") - project.after_import + fork_repository(target_project, source_repository_storage_name, source_disk_path) + end end private + def fork_repository(target_project, source_repository_storage_name, source_disk_path) + return unless start_fork(target_project) + + Gitlab::Metrics.add_event(:fork_repository, + source_path: source_disk_path, + target_path: target_project.disk_path) + + result = gitlab_shell.fork_repository(source_repository_storage_name, source_disk_path, + target_project.repository_storage, target_project.disk_path) + raise "Unable to fork project #{target_project.id} for repository #{source_disk_path} -> #{target_project.disk_path}" unless result + + target_project.after_import + end + def start_fork(project) return true if start(project) diff --git a/changelogs/unreleased/20394-protected-branches-wildcard.yml b/changelogs/unreleased/20394-protected-branches-wildcard.yml new file mode 100644 index 00000000000..3fa8ee4f69f --- /dev/null +++ b/changelogs/unreleased/20394-protected-branches-wildcard.yml @@ -0,0 +1,5 @@ +--- +title: Include matching branches and tags in protected branches / tags count +merge_request: +author: Jan Beckmann +type: fixed diff --git a/changelogs/unreleased/39880-merge-method-api.yml b/changelogs/unreleased/39880-merge-method-api.yml new file mode 100644 index 00000000000..dd44a752c4f --- /dev/null +++ b/changelogs/unreleased/39880-merge-method-api.yml @@ -0,0 +1,5 @@ +--- +title: 'API: Add parameter merge_method to projects' +merge_request: 18031 +author: Jan Beckmann +type: added diff --git a/changelogs/unreleased/41224-pipeline-icons.yml b/changelogs/unreleased/41224-pipeline-icons.yml new file mode 100644 index 00000000000..3fe05448d1c --- /dev/null +++ b/changelogs/unreleased/41224-pipeline-icons.yml @@ -0,0 +1,5 @@ +--- +title: Increase dropdown width in pipeline graph & center action icon +merge_request: 18089 +author: +type: fixed diff --git a/changelogs/unreleased/feature_detect_co_authored_commits.yml b/changelogs/unreleased/feature_detect_co_authored_commits.yml new file mode 100644 index 00000000000..7b1269ed982 --- /dev/null +++ b/changelogs/unreleased/feature_detect_co_authored_commits.yml @@ -0,0 +1,6 @@ +--- +title: Detect commit message trailers and link users properly to their accounts + on Gitlab +merge_request: 17919 +author: cousine +type: added diff --git a/changelogs/unreleased/fj-174-better-ldap-connection-handling.yml b/changelogs/unreleased/fj-174-better-ldap-connection-handling.yml new file mode 100644 index 00000000000..be0b83505fb --- /dev/null +++ b/changelogs/unreleased/fj-174-better-ldap-connection-handling.yml @@ -0,0 +1,5 @@ +--- +title: Add better LDAP connection handling +merge_request: 18039 +author: +type: fixed diff --git a/changelogs/unreleased/issue_40915.yml b/changelogs/unreleased/issue_40915.yml new file mode 100644 index 00000000000..2b6d98e69a6 --- /dev/null +++ b/changelogs/unreleased/issue_40915.yml @@ -0,0 +1,5 @@ +--- +title: Allow assigning and filtering issuables by ancestor group labels +merge_request: +author: +type: added diff --git a/changelogs/unreleased/osw-41401-render-mr-commit-sha-instead-diffs.yml b/changelogs/unreleased/osw-41401-render-mr-commit-sha-instead-diffs.yml new file mode 100644 index 00000000000..44973641325 --- /dev/null +++ b/changelogs/unreleased/osw-41401-render-mr-commit-sha-instead-diffs.yml @@ -0,0 +1,5 @@ +--- +title: Render MR commit SHA instead "diffs" when viable +merge_request: +author: +type: added diff --git a/changelogs/unreleased/sh-gitlab-sidekiq-logger.yml b/changelogs/unreleased/sh-gitlab-sidekiq-logger.yml new file mode 100644 index 00000000000..f68d45d2f38 --- /dev/null +++ b/changelogs/unreleased/sh-gitlab-sidekiq-logger.yml @@ -0,0 +1,5 @@ +--- +title: Add support for Sidekiq JSON logging +merge_request: +author: +type: added diff --git a/changelogs/unreleased/zj-feature-gate-remove-http-api.yml b/changelogs/unreleased/zj-feature-gate-remove-http-api.yml new file mode 100644 index 00000000000..2095f60146c --- /dev/null +++ b/changelogs/unreleased/zj-feature-gate-remove-http-api.yml @@ -0,0 +1,5 @@ +--- +title: Allow feature gates to be removed through the API +merge_request: +author: +type: added diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index 8db66037d61..126a9b8b803 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -226,6 +226,10 @@ production: &base # plain_url: "http://..." # default: https://www.gravatar.com/avatar/%{hash}?s=%{size}&d=identicon # ssl_url: "https://..." # default: https://secure.gravatar.com/avatar/%{hash}?s=%{size}&d=identicon + ## Sidekiq + sidekiq: + log_format: default # (json is also supported) + ## Auxiliary jobs # Periodically executed jobs, to self-heal GitLab, do external synchronizations, etc. # Please read here for more information: https://github.com/ondrejbartas/sidekiq-cron#adding-cron-job diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 69b59b26d8c..187e70868ea 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -454,6 +454,12 @@ Settings.cron_jobs['pages_domain_verification_cron_worker']['cron'] ||= '*/15 * Settings.cron_jobs['pages_domain_verification_cron_worker']['job_class'] = 'PagesDomainVerificationCronWorker' # +# Sidekiq +# +Settings['sidekiq'] ||= Settingslogic.new({}) +Settings['sidekiq']['log_format'] ||= 'default' + +# # GitLab Shell # Settings['gitlab_shell'] ||= Settingslogic.new({}) diff --git a/config/initializers/sidekiq.rb b/config/initializers/sidekiq.rb index 161fb185c9b..f6803eb0b5a 100644 --- a/config/initializers/sidekiq.rb +++ b/config/initializers/sidekiq.rb @@ -5,16 +5,23 @@ queues_config_hash[:namespace] = Gitlab::Redis::Queues::SIDEKIQ_NAMESPACE # Default is to retry 25 times with exponential backoff. That's too much. Sidekiq.default_worker_options = { retry: 3 } +enable_json_logs = Gitlab.config.sidekiq.log_format == 'json' + Sidekiq.configure_server do |config| config.redis = queues_config_hash config.server_middleware do |chain| - chain.add Gitlab::SidekiqMiddleware::ArgumentsLogger if ENV['SIDEKIQ_LOG_ARGUMENTS'] + chain.add Gitlab::SidekiqMiddleware::ArgumentsLogger if ENV['SIDEKIQ_LOG_ARGUMENTS'] && !enable_json_logs chain.add Gitlab::SidekiqMiddleware::Shutdown chain.add Gitlab::SidekiqMiddleware::RequestStoreMiddleware unless ENV['SIDEKIQ_REQUEST_STORE'] == '0' chain.add Gitlab::SidekiqStatus::ServerMiddleware end + if enable_json_logs + Sidekiq.logger.formatter = Gitlab::SidekiqLogging::JSONFormatter.new + config.options[:job_logger] = Gitlab::SidekiqLogging::StructuredLogger + end + config.client_middleware do |chain| chain.add Gitlab::SidekiqStatus::ClientMiddleware end diff --git a/doc/administration/logs.md b/doc/administration/logs.md index cd107a5b39c..c8a3ef80e8f 100644 --- a/doc/administration/logs.md +++ b/doc/administration/logs.md @@ -146,6 +146,28 @@ this file. For example: 2014-06-10T18:18:26Z 14299 TID-55uqo INFO: Booting Sidekiq 3.0.0 with redis options {:url=>"redis://localhost:6379/0", :namespace=>"sidekiq"} ``` +Instead of the format above, you can opt to generate JSON logs for +Sidekiq. For example: + +```json +{"severity":"INFO","time":"2018-04-03T22:57:22.071Z","queue":"cronjob:update_all_mirrors","args":[],"class":"UpdateAllMirrorsWorker","retry":false,"queue_namespace":"cronjob","jid":"06aeaa3b0aadacf9981f368e","created_at":"2018-04-03T22:57:21.930Z","enqueued_at":"2018-04-03T22:57:21.931Z","pid":10077,"message":"UpdateAllMirrorsWorker JID-06aeaa3b0aadacf9981f368e: done: 0.139 sec","job_status":"done","duration":0.139,"completed_at":"2018-04-03T22:57:22.071Z"} +``` + +For Omnibus GitLab installations, add the configuration option: + +```ruby +sidekiq['log_format'] = 'json' +``` + +For source installations, edit the `gitlab.yml` and set the Sidekiq +`log_format` configuration option: + +```yaml + ## Sidekiq + sidekiq: + log_format: json +``` + ## `gitlab-shell.log` This file lives in `/var/log/gitlab/gitlab-shell/gitlab-shell.log` for diff --git a/doc/api/features.md b/doc/api/features.md index 6861dbf00a2..6ee1c36ef5b 100644 --- a/doc/api/features.md +++ b/doc/api/features.md @@ -86,3 +86,11 @@ Example response: ] } ``` + +## Delete a feature + +Removes a feature gate. Response is equal when the gate exists, or doesn't. + +``` +DELETE /features/:name +``` diff --git a/doc/api/projects.md b/doc/api/projects.md index f388fae42a9..a0cb5aa0820 100644 --- a/doc/api/projects.md +++ b/doc/api/projects.md @@ -16,6 +16,21 @@ Values for the project visibility level are: * `public`: The project can be cloned without any authentication. +## Project merge method + +There are currently three options for `merge_method` to choose from: + +* `merge`: + A merge commit is created for every merge, and merging is allowed as long as there are no conflicts. + +* `rebase_merge`: + A merge commit is created for every merge, but merging is only allowed if fast-forward merge is possible. + This way you could make sure that if this merge request would build, after merging to target branch it would also build. + +* `ff`: + No merge commits are created and all merges are fast-forwarded, which means that merging is only allowed if the branch could be fast-forwarded. + + ## List all projects Get a list of all visible projects across GitLab for the authenticated user. @@ -94,6 +109,7 @@ GET /projects "only_allow_merge_if_pipeline_succeeds": false, "only_allow_merge_if_all_discussions_are_resolved": false, "request_access_enabled": false, + "merge_method": "merge", "statistics": { "commit_count": 37, "storage_size": 1038090, @@ -173,6 +189,7 @@ GET /projects "only_allow_merge_if_pipeline_succeeds": false, "only_allow_merge_if_all_discussions_are_resolved": false, "request_access_enabled": false, + "merge_method": "merge", "statistics": { "commit_count": 12, "storage_size": 2066080, @@ -278,6 +295,7 @@ GET /users/:user_id/projects "only_allow_merge_if_pipeline_succeeds": false, "only_allow_merge_if_all_discussions_are_resolved": false, "request_access_enabled": false, + "merge_method": "merge", "statistics": { "commit_count": 37, "storage_size": 1038090, @@ -357,6 +375,7 @@ GET /users/:user_id/projects "only_allow_merge_if_pipeline_succeeds": false, "only_allow_merge_if_all_discussions_are_resolved": false, "request_access_enabled": false, + "merge_method": "merge", "statistics": { "commit_count": 12, "storage_size": 2066080, @@ -467,6 +486,7 @@ GET /projects/:id "only_allow_merge_if_all_discussions_are_resolved": false, "printing_merge_requests_link_enabled": true, "request_access_enabled": false, + "merge_method": "merge", "statistics": { "commit_count": 37, "storage_size": 1038090, @@ -550,6 +570,7 @@ POST /projects | `public_jobs` | boolean | no | If `true`, jobs can be viewed by non-project-members | | `only_allow_merge_if_pipeline_succeeds` | boolean | no | Set whether merge requests can only be merged with successful jobs | | `only_allow_merge_if_all_discussions_are_resolved` | boolean | no | Set whether merge requests can only be merged when all the discussions are resolved | +| `merge_method` | string | no | Set the merge method used | | `lfs_enabled` | boolean | no | Enable LFS | | `request_access_enabled` | boolean | no | Allow users to request member access | | `tag_list` | array | no | The list of tags for a project; put array of tags, that should be finally assigned to a project | @@ -586,6 +607,7 @@ POST /projects/user/:user_id | `public_jobs` | boolean | no | If `true`, jobs can be viewed by non-project-members | | `only_allow_merge_if_pipeline_succeeds` | boolean | no | Set whether merge requests can only be merged with successful jobs | | `only_allow_merge_if_all_discussions_are_resolved` | boolean | no | Set whether merge requests can only be merged when all the discussions are resolved | +| `merge_method` | string | no | Set the merge method used | | `lfs_enabled` | boolean | no | Enable LFS | | `request_access_enabled` | boolean | no | Allow users to request member access | | `tag_list` | array | no | The list of tags for a project; put array of tags, that should be finally assigned to a project | @@ -621,6 +643,7 @@ PUT /projects/:id | `public_jobs` | boolean | no | If `true`, jobs can be viewed by non-project-members | | `only_allow_merge_if_pipeline_succeeds` | boolean | no | Set whether merge requests can only be merged with successful jobs | | `only_allow_merge_if_all_discussions_are_resolved` | boolean | no | Set whether merge requests can only be merged when all the discussions are resolved | +| `merge_method` | string | no | Set the merge method used | | `lfs_enabled` | boolean | no | Enable LFS | | `request_access_enabled` | boolean | no | Allow users to request member access | | `tag_list` | array | no | The list of tags for a project; put array of tags, that should be finally assigned to a project | @@ -724,6 +747,7 @@ Example responses: "only_allow_merge_if_pipeline_succeeds": false, "only_allow_merge_if_all_discussions_are_resolved": false, "request_access_enabled": false, + "merge_method": "merge", "_links": { "self": "http://example.com/api/v4/projects", "issues": "http://example.com/api/v4/projects/1/issues", @@ -801,6 +825,7 @@ Example response: "only_allow_merge_if_pipeline_succeeds": false, "only_allow_merge_if_all_discussions_are_resolved": false, "request_access_enabled": false, + "merge_method": "merge", "_links": { "self": "http://example.com/api/v4/projects", "issues": "http://example.com/api/v4/projects/1/issues", @@ -877,6 +902,7 @@ Example response: "only_allow_merge_if_pipeline_succeeds": false, "only_allow_merge_if_all_discussions_are_resolved": false, "request_access_enabled": false, + "merge_method": "merge", "_links": { "self": "http://example.com/api/v4/projects", "issues": "http://example.com/api/v4/projects/1/issues", @@ -971,6 +997,7 @@ Example response: "only_allow_merge_if_pipeline_succeeds": false, "only_allow_merge_if_all_discussions_are_resolved": false, "request_access_enabled": false, + "merge_method": "merge", "_links": { "self": "http://example.com/api/v4/projects", "issues": "http://example.com/api/v4/projects/1/issues", @@ -1065,6 +1092,7 @@ Example response: "only_allow_merge_if_pipeline_succeeds": false, "only_allow_merge_if_all_discussions_are_resolved": false, "request_access_enabled": false, + "merge_method": "merge", "_links": { "self": "http://example.com/api/v4/projects", "issues": "http://example.com/api/v4/projects/1/issues", diff --git a/doc/ci/examples/laravel_with_gitlab_and_envoy/index.md b/doc/ci/examples/laravel_with_gitlab_and_envoy/index.md index b62874ef029..1f9b9d53fc1 100644 --- a/doc/ci/examples/laravel_with_gitlab_and_envoy/index.md +++ b/doc/ci/examples/laravel_with_gitlab_and_envoy/index.md @@ -190,7 +190,7 @@ To start, we create an `Envoy.blade.php` in the root of our app with a simple ta ```php @servers(['web' => 'remote_username@remote_host']) -@task('list', [on => 'web']) +@task('list', ['on' => 'web']) ls -l @endtask ``` diff --git a/doc/user/project/labels.md b/doc/user/project/labels.md index dabffaec5fa..a89a1206170 100644 --- a/doc/user/project/labels.md +++ b/doc/user/project/labels.md @@ -9,7 +9,7 @@ Labels allow you to categorize issues or merge requests using descriptive titles In GitLab, you can create project and group labels: - **Project labels** can be assigned to issues or merge requests in that project only. -- **Group labels** can be assigned to any issue or merge request of any project in that group. +- **Group labels** can be assigned to any issue or merge request of any project in that group or subgroup. - In the [future](https://gitlab.com/gitlab-org/gitlab-ce/issues/40915), you will be able to assign group labels to issues and merge reqeusts of projects in [subgroups](../group/subgroups/index.md). ## Creating labels @@ -74,9 +74,9 @@ Every issue and merge request can be assigned any number of labels. The labels a ### Filtering in list pages -From the project issue list page and the project merge request list page, you can [filter](../search/index.md#issues-and-merge-requests) by both group labels and project labels. +From the project issue list page and the project merge request list page, you can [filter](../search/index.md#issues-and-merge-requests) by both group (including subgroup ancestors) labels and project labels. -From the group issue list page and the group merge request list page, you can [filter](../search/index.md#issues-and-merge-requests) by both group labels and project labels. +From the group issue list page and the group merge request list page, you can [filter](../search/index.md#issues-and-merge-requests) by both group labels (including subgroup ancestors and subgroup descendants) and project labels.  diff --git a/lib/api/entities.rb b/lib/api/entities.rb index b7a390696c7..e5ecd37e473 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -206,6 +206,7 @@ module API expose :request_access_enabled expose :only_allow_merge_if_all_discussions_are_resolved expose :printing_merge_request_link_enabled + expose :merge_method expose :statistics, using: 'API::Entities::ProjectStatistics', if: :statistics diff --git a/lib/api/features.rb b/lib/api/features.rb index 9385c6ca174..11d848584d9 100644 --- a/lib/api/features.rb +++ b/lib/api/features.rb @@ -65,6 +65,13 @@ module API present feature, with: Entities::Feature, current_user: current_user end + + desc 'Remove the gate value for the given feature' + delete ':name' do + Feature.get(params[:name]).remove + + status 204 + end end end end diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index e59e8a45908..61c138a7dec 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -83,12 +83,13 @@ module API end def available_labels_for(label_parent) - search_params = - if label_parent.is_a?(Project) - { project_id: label_parent.id } - else - { group_id: label_parent.id, only_group_labels: true } - end + search_params = { include_ancestor_groups: true } + + if label_parent.is_a?(Project) + search_params[:project_id] = label_parent.id + else + search_params.merge!(group_id: label_parent.id, only_group_labels: true) + end LabelsFinder.new(current_user, search_params).execute end diff --git a/lib/api/projects.rb b/lib/api/projects.rb index 467bc78dad8..3d5b3c5a535 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -28,6 +28,7 @@ module API optional :tag_list, type: Array[String], desc: 'The list of tags for a project' optional :avatar, type: File, desc: 'Avatar image for project' optional :printing_merge_request_link_enabled, type: Boolean, desc: 'Show link to create/view merge request when pushing from the command line' + optional :merge_method, type: String, values: %w(ff rebase_merge merge), desc: 'The merge method used when merging merge requests' end params :optional_params do @@ -274,6 +275,7 @@ module API :issues_enabled, :lfs_enabled, :merge_requests_enabled, + :merge_method, :name, :only_allow_merge_if_all_discussions_are_resolved, :only_allow_merge_if_pipeline_succeeds, diff --git a/lib/banzai/filter/abstract_reference_filter.rb b/lib/banzai/filter/abstract_reference_filter.rb index c9e3f8ce42b..c3a03f13306 100644 --- a/lib/banzai/filter/abstract_reference_filter.rb +++ b/lib/banzai/filter/abstract_reference_filter.rb @@ -171,7 +171,7 @@ module Banzai end if object - title = object_link_title(object) + title = object_link_title(object, matches) klass = reference_class(object_sym) data = data_attributes_for(link_content || match, parent, object, @@ -216,7 +216,7 @@ module Banzai extras end - def object_link_title(object) + def object_link_title(object, matches) object.title end diff --git a/lib/banzai/filter/commit_range_reference_filter.rb b/lib/banzai/filter/commit_range_reference_filter.rb index 21bcb1c5ca8..99fa2d9d8fb 100644 --- a/lib/banzai/filter/commit_range_reference_filter.rb +++ b/lib/banzai/filter/commit_range_reference_filter.rb @@ -34,7 +34,7 @@ module Banzai range.to_param.merge(only_path: context[:only_path])) end - def object_link_title(range) + def object_link_title(range, matches) nil end end diff --git a/lib/banzai/filter/commit_trailers_filter.rb b/lib/banzai/filter/commit_trailers_filter.rb new file mode 100644 index 00000000000..ef16df1f3ae --- /dev/null +++ b/lib/banzai/filter/commit_trailers_filter.rb @@ -0,0 +1,152 @@ +module Banzai + module Filter + # HTML filter that replaces users' names and emails in commit trailers + # with links to their GitLab accounts or mailto links to their mentioned + # emails. + # + # Commit trailers are special labels in the form of `*-by:` and fall on a + # single line, ex: + # + # Reported-By: John S. Doe <john.doe@foo.bar> + # + # More info about this can be found here: + # * https://git.wiki.kernel.org/index.php/CommitMessageConventions + class CommitTrailersFilter < HTML::Pipeline::Filter + include ActionView::Helpers::TagHelper + include ApplicationHelper + include AvatarsHelper + + TRAILER_REGEXP = /(?<label>[[:alpha:]-]+-by:)/i.freeze + AUTHOR_REGEXP = /(?<author_name>.+)/.freeze + # Devise.email_regexp wouldn't work here since its designed to match + # against strings that only contains email addresses; the \A and \z + # around the expression will only match if the string being matched + # contains just the email nothing else. + MAIL_REGEXP = /<(?<author_email>[^@\s]+@[^@\s]+)>/.freeze + FILTER_REGEXP = /(?<trailer>^\s*#{TRAILER_REGEXP}\s*#{AUTHOR_REGEXP}\s+#{MAIL_REGEXP}$)/mi.freeze + + def call + doc.xpath('descendant-or-self::text()').each do |node| + content = node.to_html + + next unless content.match(FILTER_REGEXP) + + html = trailer_filter(content) + + next if html == content + + node.replace(html) + end + + doc + end + + private + + # Replace trailer lines with links to GitLab users or mailto links to + # non GitLab users. + # + # text - String text to replace trailers in. + # + # Returns a String with all trailer lines replaced with links to GitLab + # users and mailto links to non GitLab users. All links have `data-trailer` + # and `data-user` attributes attached. + def trailer_filter(text) + text.gsub(FILTER_REGEXP) do |author_match| + label = $~[:label] + "#{label} #{parse_user($~[:author_name], $~[:author_email], label)}" + end + end + + # Find a GitLab user using the supplied email and generate + # a valid link to them, otherwise, generate a mailto link. + # + # name - String name used in the commit message for the user + # email - String email used in the commit message for the user + # trailer - String trailer used in the commit message + # + # Returns a String with a link to the user. + def parse_user(name, email, trailer) + link_to_user User.find_by_any_email(email), + name: name, + email: email, + trailer: trailer + end + + def urls + Gitlab::Routing.url_helpers + end + + def link_to_user(user, name:, email:, trailer:) + wrapper = link_wrapper(data: { + trailer: trailer, + user: user.try(:id) + }) + + avatar = user_avatar_without_link( + user: user, + user_email: email, + css_class: 'avatar-inline', + has_tooltip: false + ) + + link_href = user.nil? ? "mailto:#{email}" : urls.user_url(user) + + avatar_link = link_tag( + link_href, + content: avatar, + title: email + ) + + name_link = link_tag( + link_href, + content: name, + title: email + ) + + email_link = link_tag( + "mailto:#{email}", + content: email, + title: email + ) + + wrapper << "#{avatar_link}#{name_link} <#{email_link}>" + end + + def link_wrapper(data: {}) + data_attributes = data_attributes_from_hash(data) + + doc.document.create_element( + 'span', + data_attributes + ) + end + + def link_tag(url, title: "", content: "", data: {}) + data_attributes = data_attributes_from_hash(data) + + attributes = data_attributes.merge( + href: url, + title: title + ) + + link = doc.document.create_element('a', attributes) + + if content.html_safe? + link << content + else + link.content = content # make sure we escape content using nokogiri's #content= + end + + link + end + + def data_attributes_from_hash(data = {}) + data.reject! {|_, value| value.nil?} + data.map do |key, value| + [%(data-#{key.to_s.dasherize}), value] + end.to_h + end + end + end +end diff --git a/lib/banzai/filter/label_reference_filter.rb b/lib/banzai/filter/label_reference_filter.rb index d5360ad8f68..faa5b344e6f 100644 --- a/lib/banzai/filter/label_reference_filter.rb +++ b/lib/banzai/filter/label_reference_filter.rb @@ -41,7 +41,7 @@ module Banzai end def find_labels(project) - LabelsFinder.new(nil, project_id: project.id).execute(skip_authorization: true) + LabelsFinder.new(nil, project_id: project.id, include_ancestor_groups: true).execute(skip_authorization: true) end # Parameters to pass to `Label.find_by` based on the given arguments @@ -77,7 +77,7 @@ module Banzai CGI.unescapeHTML(text.to_s) end - def object_link_title(object) + def object_link_title(object, matches) # use title of wrapped element instead nil end diff --git a/lib/banzai/filter/merge_request_reference_filter.rb b/lib/banzai/filter/merge_request_reference_filter.rb index b3cfa97d0e0..5cbdb01c130 100644 --- a/lib/banzai/filter/merge_request_reference_filter.rb +++ b/lib/banzai/filter/merge_request_reference_filter.rb @@ -17,10 +17,19 @@ module Banzai only_path: context[:only_path]) end + def object_link_title(object, matches) + object_link_commit_title(object, matches) || super + end + def object_link_text_extras(object, matches) extras = super + if commit_ref = object_link_commit_ref(object, matches) + return extras.unshift(commit_ref) + end + path = matches[:path] if matches.names.include?("path") + case path when '/diffs' extras.unshift "diffs" @@ -38,6 +47,36 @@ module Banzai .where(iid: ids.to_a) .includes(target_project: :namespace) end + + private + + def object_link_commit_title(object, matches) + object_link_commit(object, matches)&.title + end + + def object_link_commit_ref(object, matches) + object_link_commit(object, matches)&.short_id + end + + def object_link_commit(object, matches) + return unless matches.names.include?('query') && query = matches[:query] + + # Removes leading "?". CGI.parse expects "arg1&arg2&arg3" + params = CGI.parse(query.sub(/^\?/, '')) + + return unless commit_sha = params['commit_id']&.first + + if commit = find_commit_by_sha(object, commit_sha) + Commit.from_hash(commit.to_hash, object.project) + end + end + + def find_commit_by_sha(object, commit_sha) + @all_commits ||= {} + @all_commits[object.id] ||= object.all_commits + + @all_commits[object.id].find { |commit| commit.sha == commit_sha } + end end end end diff --git a/lib/banzai/filter/milestone_reference_filter.rb b/lib/banzai/filter/milestone_reference_filter.rb index 8ec696ce5fc..1a1d7dbeb3d 100644 --- a/lib/banzai/filter/milestone_reference_filter.rb +++ b/lib/banzai/filter/milestone_reference_filter.rb @@ -84,7 +84,7 @@ module Banzai end end - def object_link_title(object) + def object_link_title(object, matches) nil end end diff --git a/lib/banzai/pipeline/commit_description_pipeline.rb b/lib/banzai/pipeline/commit_description_pipeline.rb new file mode 100644 index 00000000000..607c2731ed3 --- /dev/null +++ b/lib/banzai/pipeline/commit_description_pipeline.rb @@ -0,0 +1,11 @@ +module Banzai + module Pipeline + class CommitDescriptionPipeline < SingleLinePipeline + def self.filters + @filters ||= super.concat FilterArray[ + Filter::CommitTrailersFilter, + ] + end + end + end +end diff --git a/lib/gitlab/auth/ldap/access.rb b/lib/gitlab/auth/ldap/access.rb index 77c0ddc2d48..34286900e72 100644 --- a/lib/gitlab/auth/ldap/access.rb +++ b/lib/gitlab/auth/ldap/access.rb @@ -52,6 +52,8 @@ module Gitlab block_user(user, 'does not exist anymore') false end + rescue LDAPConnectionError + false end def adapter diff --git a/lib/gitlab/auth/ldap/adapter.rb b/lib/gitlab/auth/ldap/adapter.rb index caf2d18c668..82ff1e77e5c 100644 --- a/lib/gitlab/auth/ldap/adapter.rb +++ b/lib/gitlab/auth/ldap/adapter.rb @@ -2,6 +2,9 @@ module Gitlab module Auth module LDAP class Adapter + SEARCH_RETRY_FACTOR = [1, 1, 2, 3].freeze + MAX_SEARCH_RETRIES = Rails.env.test? ? 1 : SEARCH_RETRY_FACTOR.size.freeze + attr_reader :provider, :ldap def self.open(provider, &block) @@ -16,7 +19,7 @@ module Gitlab def initialize(provider, ldap = nil) @provider = provider - @ldap = ldap || Net::LDAP.new(config.adapter_options) + @ldap = ldap || renew_connection_adapter end def config @@ -47,8 +50,10 @@ module Gitlab end def ldap_search(*args) + retries ||= 0 + # Net::LDAP's `time` argument doesn't work. Use Ruby `Timeout` instead. - Timeout.timeout(config.timeout) do + Timeout.timeout(timeout_time(retries)) do results = ldap.search(*args) if results.nil? @@ -63,16 +68,26 @@ module Gitlab results end end - rescue Net::LDAP::Error => error - Rails.logger.warn("LDAP search raised exception #{error.class}: #{error.message}") - [] - rescue Timeout::Error - Rails.logger.warn("LDAP search timed out after #{config.timeout} seconds") - [] + rescue Net::LDAP::Error, Timeout::Error => error + retries += 1 + error_message = connection_error_message(error) + + Rails.logger.warn(error_message) + + if retries < MAX_SEARCH_RETRIES + renew_connection_adapter + retry + else + raise LDAPConnectionError, error_message + end end private + def timeout_time(retry_number) + SEARCH_RETRY_FACTOR[retry_number] * config.timeout + end + def user_options(fields, value, limit) options = { attributes: Gitlab::Auth::LDAP::Person.ldap_attributes(config), @@ -104,6 +119,18 @@ module Gitlab filter end end + + def connection_error_message(exception) + if exception.is_a?(Timeout::Error) + "LDAP search timed out after #{config.timeout} seconds" + else + "LDAP search raised exception #{exception.class}: #{exception.message}" + end + end + + def renew_connection_adapter + @ldap = Net::LDAP.new(config.adapter_options) + end end end end diff --git a/lib/gitlab/auth/ldap/ldap_connection_error.rb b/lib/gitlab/auth/ldap/ldap_connection_error.rb new file mode 100644 index 00000000000..ef0a695742b --- /dev/null +++ b/lib/gitlab/auth/ldap/ldap_connection_error.rb @@ -0,0 +1,7 @@ +module Gitlab + module Auth + module LDAP + LDAPConnectionError = Class.new(StandardError) + end + end +end diff --git a/lib/gitlab/auth/o_auth/user.rb b/lib/gitlab/auth/o_auth/user.rb index b6a96081278..d0c6b0386ba 100644 --- a/lib/gitlab/auth/o_auth/user.rb +++ b/lib/gitlab/auth/o_auth/user.rb @@ -124,6 +124,9 @@ module Gitlab Gitlab::Auth::LDAP::Person.find_by_uid(auth_hash.uid, adapter) || Gitlab::Auth::LDAP::Person.find_by_email(auth_hash.uid, adapter) || Gitlab::Auth::LDAP::Person.find_by_dn(auth_hash.uid, adapter) + + rescue Gitlab::Auth::LDAP::LDAPConnectionError + nil end def ldap_config diff --git a/lib/gitlab/bitbucket_import/importer.rb b/lib/gitlab/bitbucket_import/importer.rb index bffbcb86137..f3999e690fa 100644 --- a/lib/gitlab/bitbucket_import/importer.rb +++ b/lib/gitlab/bitbucket_import/importer.rb @@ -63,7 +63,7 @@ module Gitlab disk_path = project.wiki.disk_path import_url = project.import_url.sub(/\.git\z/, ".git/wiki") - gitlab_shell.import_repository(project.repository_storage_path, disk_path, import_url) + gitlab_shell.import_repository(project.repository_storage, disk_path, import_url) rescue StandardError => e errors << { type: :wiki, errors: e.message } end diff --git a/lib/gitlab/git/gitlab_projects.rb b/lib/gitlab/git/gitlab_projects.rb index dc0bc8518bc..099709620b3 100644 --- a/lib/gitlab/git/gitlab_projects.rb +++ b/lib/gitlab/git/gitlab_projects.rb @@ -4,20 +4,14 @@ module Gitlab include Gitlab::Git::Popen include Gitlab::Utils::StrongMemoize - ShardNameNotFoundError = Class.new(StandardError) - - # Absolute path to directory where repositories are stored. - # Example: /home/git/repositories - attr_reader :shard_path + # Name of shard where repositories are stored. + # Example: nfs-file06 + attr_reader :shard_name # Relative path is a directory name for repository with .git at the end. # Example: gitlab-org/gitlab-test.git attr_reader :repository_relative_path - # Absolute path to the repository. - # Example: /home/git/repositorities/gitlab-org/gitlab-test.git - attr_reader :repository_absolute_path - # This is the path at which the gitlab-shell hooks directory can be found. # It's essential for integration between git and GitLab proper. All new # repositories should have their hooks directory symlinked here. @@ -25,13 +19,12 @@ module Gitlab attr_reader :logger - def initialize(shard_path, repository_relative_path, global_hooks_path:, logger:) - @shard_path = shard_path + def initialize(shard_name, repository_relative_path, global_hooks_path:, logger:) + @shard_name = shard_name @repository_relative_path = repository_relative_path @logger = logger @global_hooks_path = global_hooks_path - @repository_absolute_path = File.join(shard_path, repository_relative_path) @output = StringIO.new end @@ -41,6 +34,22 @@ module Gitlab io.read end + # Absolute path to the repository. + # Example: /home/git/repositorities/gitlab-org/gitlab-test.git + # Probably will be removed when we fully migrate to Gitaly, part of + # https://gitlab.com/gitlab-org/gitaly/issues/1124. + def repository_absolute_path + strong_memoize(:repository_absolute_path) do + File.join(shard_path, repository_relative_path) + end + end + + def shard_path + strong_memoize(:shard_path) do + Gitlab.config.repositories.storages.fetch(shard_name).legacy_disk_path + end + end + # Import project via git clone --bare # URL must be publicly cloneable def import_project(source, timeout) @@ -53,12 +62,12 @@ module Gitlab end end - def fork_repository(new_shard_path, new_repository_relative_path) + def fork_repository(new_shard_name, new_repository_relative_path) Gitlab::GitalyClient.migrate(:fork_repository) do |is_enabled| if is_enabled - gitaly_fork_repository(new_shard_path, new_repository_relative_path) + gitaly_fork_repository(new_shard_name, new_repository_relative_path) else - git_fork_repository(new_shard_path, new_repository_relative_path) + git_fork_repository(new_shard_name, new_repository_relative_path) end end end @@ -205,17 +214,6 @@ module Gitlab private - def shard_name - strong_memoize(:shard_name) do - shard_name_from_shard_path(shard_path) - end - end - - def shard_name_from_shard_path(shard_path) - Gitlab.config.repositories.storages.find { |_, info| info.legacy_disk_path == shard_path }&.first || - raise(ShardNameNotFoundError, "no shard found for path '#{shard_path}'") - end - def git_import_repository(source, timeout) # Skip import if repo already exists return false if File.exist?(repository_absolute_path) @@ -252,8 +250,9 @@ module Gitlab false end - def git_fork_repository(new_shard_path, new_repository_relative_path) + def git_fork_repository(new_shard_name, new_repository_relative_path) from_path = repository_absolute_path + new_shard_path = Gitlab.config.repositories.storages.fetch(new_shard_name).legacy_disk_path to_path = File.join(new_shard_path, new_repository_relative_path) # The repository cannot already exist @@ -271,8 +270,8 @@ module Gitlab run(cmd, nil) && Gitlab::Git::Repository.create_hooks(to_path, global_hooks_path) end - def gitaly_fork_repository(new_shard_path, new_repository_relative_path) - target_repository = Gitlab::Git::Repository.new(shard_name_from_shard_path(new_shard_path), new_repository_relative_path, nil) + def gitaly_fork_repository(new_shard_name, new_repository_relative_path) + target_repository = Gitlab::Git::Repository.new(new_shard_name, new_repository_relative_path, nil) raw_repository = Gitlab::Git::Repository.new(shard_name, repository_relative_path, nil) Gitlab::GitalyClient::RepositoryService.new(target_repository).fork_repository(raw_repository) diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index e692c9ce342..d16a096ffb9 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -96,7 +96,7 @@ module Gitlab storage_path = Gitlab.config.repositories.storages[@storage].legacy_disk_path @gitlab_projects = Gitlab::Git::GitlabProjects.new( - storage_path, + storage, relative_path, global_hooks_path: Gitlab.config.gitlab_shell.hooks_path, logger: Rails.logger diff --git a/lib/gitlab/github_import/importer/repository_importer.rb b/lib/gitlab/github_import/importer/repository_importer.rb index b1b283e98b5..01168abde6c 100644 --- a/lib/gitlab/github_import/importer/repository_importer.rb +++ b/lib/gitlab/github_import/importer/repository_importer.rb @@ -56,9 +56,8 @@ module Gitlab def import_wiki_repository wiki_path = "#{project.disk_path}.wiki" - storage_path = project.repository_storage_path - gitlab_shell.import_repository(storage_path, wiki_path, wiki_url) + gitlab_shell.import_repository(project.repository_storage, wiki_path, wiki_url) true rescue Gitlab::Shell::Error => e diff --git a/lib/gitlab/legacy_github_import/importer.rb b/lib/gitlab/legacy_github_import/importer.rb index 0526ef9eb13..7edd0ad2033 100644 --- a/lib/gitlab/legacy_github_import/importer.rb +++ b/lib/gitlab/legacy_github_import/importer.rb @@ -259,7 +259,7 @@ module Gitlab def import_wiki unless project.wiki.repository_exists? wiki = WikiFormatter.new(project) - gitlab_shell.import_repository(project.repository_storage_path, wiki.disk_path, wiki.import_url) + gitlab_shell.import_repository(project.repository_storage, wiki.disk_path, wiki.import_url) end rescue Gitlab::Shell::Error => e # GitHub error message when the wiki repo has not been created, diff --git a/lib/gitlab/shell.rb b/lib/gitlab/shell.rb index c8c15b9684a..67407b651a5 100644 --- a/lib/gitlab/shell.rb +++ b/lib/gitlab/shell.rb @@ -93,12 +93,12 @@ module Gitlab # Import repository # - # storage - project's storage path + # storage - project's storage name # name - project disk path # url - URL to import from # # Ex. - # import_repository("/path/to/storage", "gitlab/gitlab-ci", "https://gitlab.com/gitlab-org/gitlab-test.git") + # import_repository("nfs-file06", "gitlab/gitlab-ci", "https://gitlab.com/gitlab-org/gitlab-test.git") # # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/874 def import_repository(storage, name, url) @@ -131,8 +131,7 @@ module Gitlab if is_enabled repository.gitaly_repository_client.fetch_remote(remote, ssh_auth: ssh_auth, forced: forced, no_tags: no_tags, timeout: git_timeout, prune: prune) else - storage_path = Gitlab.config.repositories.storages[repository.storage].legacy_disk_path - local_fetch_remote(storage_path, repository.relative_path, remote, ssh_auth: ssh_auth, forced: forced, no_tags: no_tags, prune: prune) + local_fetch_remote(repository.storage, repository.relative_path, remote, ssh_auth: ssh_auth, forced: forced, no_tags: no_tags, prune: prune) end end end @@ -156,13 +155,13 @@ module Gitlab end # Fork repository to new path - # forked_from_storage - forked-from project's storage path - # forked_from_disk_path - project disk path - # forked_to_storage - forked-to project's storage path - # forked_to_disk_path - forked project disk path + # forked_from_storage - forked-from project's storage name + # forked_from_disk_path - project disk relative path + # forked_to_storage - forked-to project's storage name + # forked_to_disk_path - forked project disk relative path # # Ex. - # fork_repository("/path/to/forked_from/storage", "gitlab/gitlab-ci", "/path/to/forked_to/storage", "new-namespace/gitlab-ci") + # fork_repository("nfs-file06", "gitlab/gitlab-ci", "nfs-file07", "new-namespace/gitlab-ci") # # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/817 def fork_repository(forked_from_storage, forked_from_disk_path, forked_to_storage, forked_to_disk_path) @@ -420,16 +419,16 @@ module Gitlab private - def gitlab_projects(shard_path, disk_path) + def gitlab_projects(shard_name, disk_path) Gitlab::Git::GitlabProjects.new( - shard_path, + shard_name, disk_path, global_hooks_path: Gitlab.config.gitlab_shell.hooks_path, logger: Rails.logger ) end - def local_fetch_remote(storage_path, repository_relative_path, remote, ssh_auth: nil, forced: false, no_tags: false, prune: true) + def local_fetch_remote(storage_name, repository_relative_path, remote, ssh_auth: nil, forced: false, no_tags: false, prune: true) vars = { force: forced, tags: !no_tags, prune: prune } if ssh_auth&.ssh_import? @@ -442,7 +441,7 @@ module Gitlab end end - cmd = gitlab_projects(storage_path, repository_relative_path) + cmd = gitlab_projects(storage_name, repository_relative_path) success = cmd.fetch_remote(remote, git_timeout, vars) diff --git a/lib/gitlab/sidekiq_logging/json_formatter.rb b/lib/gitlab/sidekiq_logging/json_formatter.rb new file mode 100644 index 00000000000..98f8222fd03 --- /dev/null +++ b/lib/gitlab/sidekiq_logging/json_formatter.rb @@ -0,0 +1,21 @@ +module Gitlab + module SidekiqLogging + class JSONFormatter + def call(severity, timestamp, progname, data) + output = { + severity: severity, + time: timestamp.utc.iso8601(3) + } + + case data + when String + output[:message] = data + when Hash + output.merge!(data) + end + + output.to_json + "\n" + end + end + end +end diff --git a/lib/gitlab/sidekiq_logging/structured_logger.rb b/lib/gitlab/sidekiq_logging/structured_logger.rb new file mode 100644 index 00000000000..9a89ae70b98 --- /dev/null +++ b/lib/gitlab/sidekiq_logging/structured_logger.rb @@ -0,0 +1,96 @@ +module Gitlab + module SidekiqLogging + class StructuredLogger + START_TIMESTAMP_FIELDS = %w[created_at enqueued_at].freeze + DONE_TIMESTAMP_FIELDS = %w[started_at retried_at failed_at completed_at].freeze + + def call(job, queue) + started_at = current_time + base_payload = parse_job(job) + + Sidekiq.logger.info log_job_start(started_at, base_payload) + + yield + + Sidekiq.logger.info log_job_done(started_at, base_payload) + rescue => job_exception + Sidekiq.logger.warn log_job_done(started_at, base_payload, job_exception) + + raise + end + + private + + def base_message(payload) + "#{payload['class']} JID-#{payload['jid']}" + end + + def log_job_start(started_at, payload) + payload['message'] = "#{base_message(payload)}: start" + payload['job_status'] = 'start' + + payload + end + + def log_job_done(started_at, payload, job_exception = nil) + payload = payload.dup + payload['duration'] = elapsed(started_at) + payload['completed_at'] = Time.now.utc + + message = base_message(payload) + + if job_exception + payload['message'] = "#{message}: fail: #{payload['duration']} sec" + payload['job_status'] = 'fail' + payload['error_message'] = job_exception.message + payload['error'] = job_exception.class + payload['error_backtrace'] = backtrace_cleaner.clean(job_exception.backtrace) + else + payload['message'] = "#{message}: done: #{payload['duration']} sec" + payload['job_status'] = 'done' + end + + convert_to_iso8601(payload, DONE_TIMESTAMP_FIELDS) + + payload + end + + def parse_job(job) + job = job.dup + + # Add process id params + job['pid'] = ::Process.pid + + job.delete('args') unless ENV['SIDEKIQ_LOG_ARGUMENTS'] + + convert_to_iso8601(job, START_TIMESTAMP_FIELDS) + + job + end + + def convert_to_iso8601(payload, keys) + keys.each do |key| + payload[key] = format_time(payload[key]) if payload[key] + end + end + + def elapsed(start) + (current_time - start).round(3) + end + + def current_time + Gitlab::Metrics::System.monotonic_time + end + + def backtrace_cleaner + @backtrace_cleaner ||= ActiveSupport::BacktraceCleaner.new + end + + def format_time(timestamp) + return timestamp if timestamp.is_a?(String) + + Time.at(timestamp).utc.iso8601(3) + end + end + end +end diff --git a/scripts/codequality b/scripts/codequality deleted file mode 100755 index 2f3ccef7d2d..00000000000 --- a/scripts/codequality +++ /dev/null @@ -1,19 +0,0 @@ -#!/bin/sh - -set -eo pipefail - -code_path=$(pwd) - -# docker run --tty will merge stderr and stdout, we don't need this on CI or -# it will break codequality json file -[ "$CI" != "" ] || docker_tty="--tty" - -# The codebase and instructions for the following image can be found at https://gitlab.com/gitlab-org/codeclimate-rubocop/wikis/home -docker pull dev.gitlab.org:5005/gitlab/gitlab-build-images:gitlab-codeclimate-rubocop-0-52-1 > /dev/null -docker tag dev.gitlab.org:5005/gitlab/gitlab-build-images:gitlab-codeclimate-rubocop-0-52-1 codeclimate/codeclimate-rubocop:gitlab-codeclimate-rubocop-0-52-1 > /dev/null - -exec docker run --rm $docker_tty --env CODECLIMATE_CODE="$code_path" \ - --volume "$code_path":/code \ - --volume /var/run/docker.sock:/var/run/docker.sock \ - --volume /tmp/cc:/tmp/cc \ - "codeclimate/codeclimate:${CODECLIMATE_VERSION:-0.71.1}" "$@" diff --git a/spec/controllers/profiles_controller_spec.rb b/spec/controllers/profiles_controller_spec.rb index 03cbbb21e62..891485406c6 100644 --- a/spec/controllers/profiles_controller_spec.rb +++ b/spec/controllers/profiles_controller_spec.rb @@ -84,6 +84,13 @@ describe ProfilesController, :request_store do expect(user.username).to eq(new_username) end + it 'raises a correct error when the username is missing' do + sign_in(user) + + expect { put :update_username, user: { gandalf: 'you shall not pass' } } + .to raise_error(ActionController::ParameterMissing) + end + context 'with legacy storage' do it 'moves dependent projects to new namespace' do project = create(:project_empty_repo, :legacy_storage, namespace: namespace) diff --git a/spec/features/issues/filtered_search/filter_issues_spec.rb b/spec/features/issues/filtered_search/filter_issues_spec.rb index b3c50964810..08ba91a2682 100644 --- a/spec/features/issues/filtered_search/filter_issues_spec.rb +++ b/spec/features/issues/filtered_search/filter_issues_spec.rb @@ -22,15 +22,6 @@ describe 'Filter issues', :js do end end - def expect_issues_list_count(open_count, closed_count = 0) - all_count = open_count + closed_count - - expect(page).to have_issuable_counts(open: open_count, closed: closed_count, all: all_count) - page.within '.issues-list' do - expect(page).to have_selector('.issue', count: open_count) - end - end - before do project.add_master(user) diff --git a/spec/features/labels_hierarchy_spec.rb b/spec/features/labels_hierarchy_spec.rb new file mode 100644 index 00000000000..99e1fb30d5b --- /dev/null +++ b/spec/features/labels_hierarchy_spec.rb @@ -0,0 +1,305 @@ +require 'spec_helper' + +feature 'Labels Hierarchy', :js, :nested_groups do + include FilteredSearchHelpers + + let!(:user) { create(:user) } + let!(:grandparent) { create(:group) } + let!(:parent) { create(:group, parent: grandparent) } + let!(:child) { create(:group, parent: parent) } + let!(:project_1) { create(:project, namespace: parent) } + + let!(:grandparent_group_label) { create(:group_label, group: grandparent, title: 'Label_1') } + let!(:parent_group_label) { create(:group_label, group: parent, title: 'Label_2') } + let!(:child_group_label) { create(:group_label, group: child, title: 'Label_3') } + let!(:project_label_1) { create(:label, project: project_1, title: 'Label_4') } + + before do + grandparent.add_owner(user) + + sign_in(user) + end + + shared_examples 'assigning labels from sidebar' do + it 'can assign all ancestors labels' do + [grandparent_group_label, parent_group_label, project_label_1].each do |label| + page.within('.block.labels') do + find('.edit-link').click + end + + wait_for_requests + + find('a.label-item', text: label.title).click + find('.dropdown-menu-close-icon').click + + wait_for_requests + + expect(page).to have_selector('span.label', text: label.title) + end + end + + it 'does not find child group labels on dropdown' do + page.within('.block.labels') do + find('.edit-link').click + end + + wait_for_requests + + expect(page).not_to have_selector('span.label', text: child_group_label.title) + end + end + + shared_examples 'filtering by ancestor labels for projects' do |board = false| + it 'filters by ancestor labels' do + [grandparent_group_label, parent_group_label, project_label_1].each do |label| + select_label_on_dropdown(label.title) + + wait_for_requests + + if board + expect(page).to have_selector('.card-title') do |card| + expect(card).to have_selector('a', text: labeled_issue.title) + end + else + expect_issues_list_count(1) + expect(page).to have_selector('span.issue-title-text', text: labeled_issue.title) + end + end + end + + it 'does not filter by descendant group labels' do + filtered_search.set("label:") + + wait_for_requests + + expect(page).not_to have_selector('.btn-link', text: child_group_label.title) + end + end + + shared_examples 'filtering by ancestor labels for groups' do |board = false| + let(:project_2) { create(:project, namespace: parent) } + let!(:project_label_2) { create(:label, project: project_2, title: 'Label_4') } + + let(:project_3) { create(:project, namespace: child) } + let!(:group_label_3) { create(:group_label, group: child, title: 'Label_5') } + let!(:project_label_3) { create(:label, project: project_3, title: 'Label_6') } + + let!(:labeled_issue_2) { create(:labeled_issue, project: project_2, labels: [grandparent_group_label, parent_group_label, project_label_2]) } + let!(:labeled_issue_3) { create(:labeled_issue, project: project_3, labels: [grandparent_group_label, parent_group_label, group_label_3]) } + + let!(:issue_2) { create(:issue, project: project_2) } + + it 'filters by ancestors and current group labels' do + [grandparent_group_label, parent_group_label].each do |label| + select_label_on_dropdown(label.title) + + wait_for_requests + + if board + expect(page).to have_selector('.card-title') do |card| + expect(card).to have_selector('a', text: labeled_issue.title) + end + + expect(page).to have_selector('.card-title') do |card| + expect(card).to have_selector('a', text: labeled_issue_2.title) + end + else + expect_issues_list_count(3) + expect(page).to have_selector('span.issue-title-text', text: labeled_issue.title) + expect(page).to have_selector('span.issue-title-text', text: labeled_issue_2.title) + expect(page).to have_selector('span.issue-title-text', text: labeled_issue_3.title) + end + end + end + + it 'filters by descendant group labels' do + wait_for_requests + + if board + pending("Waiting for https://gitlab.com/gitlab-org/gitlab-ce/issues/44270") + + select_label_on_dropdown(group_label_3.title) + + expect(page).to have_selector('.card-title') do |card| + expect(card).to have_selector('a', text: labeled_issue_3.title) + end + else + select_label_on_dropdown(group_label_3.title) + + expect_issues_list_count(1) + expect(page).to have_selector('span.issue-title-text', text: labeled_issue_3.title) + end + end + + it 'does not filter by descendant group project labels' do + filtered_search.set("label:") + + wait_for_requests + + expect(page).not_to have_selector('.btn-link', text: project_label_3.title) + end + end + + context 'when creating new issuable' do + before do + visit new_project_issue_path(project_1) + end + + it 'should be able to assign ancestor group labels' do + fill_in 'issue_title', with: 'new created issue' + fill_in 'issue_description', with: 'new issue description' + + find(".js-label-select").click + wait_for_requests + + find('a.label-item', text: grandparent_group_label.title).click + find('a.label-item', text: parent_group_label.title).click + find('a.label-item', text: project_label_1.title).click + + find('.btn-create').click + + expect(page.find('.issue-details h2.title')).to have_content('new created issue') + expect(page).to have_selector('span.label', text: grandparent_group_label.title) + expect(page).to have_selector('span.label', text: parent_group_label.title) + expect(page).to have_selector('span.label', text: project_label_1.title) + end + end + + context 'issuable sidebar' do + let!(:issue) { create(:issue, project: project_1) } + + context 'on issue sidebar' do + before do + visit project_issue_path(project_1, issue) + end + + it_behaves_like 'assigning labels from sidebar' + end + + context 'on project board issue sidebar' do + let(:board) { create(:board, project: project_1) } + + before do + visit project_board_path(project_1, board) + + wait_for_requests + + find('.card').click + end + + it_behaves_like 'assigning labels from sidebar' + end + + context 'on group board issue sidebar' do + let(:board) { create(:board, group: parent) } + + before do + visit group_board_path(parent, board) + + wait_for_requests + + find('.card').click + end + + it_behaves_like 'assigning labels from sidebar' + end + end + + context 'issuable filtering' do + let!(:labeled_issue) { create(:labeled_issue, project: project_1, labels: [grandparent_group_label, parent_group_label, project_label_1]) } + let!(:issue) { create(:issue, project: project_1) } + + context 'on project issuable list' do + before do + visit project_issues_path(project_1) + end + + it_behaves_like 'filtering by ancestor labels for projects' + + it 'does not filter by descendant group labels' do + filtered_search.set("label:") + + wait_for_requests + + expect(page).not_to have_selector('.btn-link', text: child_group_label.title) + end + end + + context 'on group issuable list' do + before do + visit issues_group_path(parent) + end + + it_behaves_like 'filtering by ancestor labels for groups' + end + + context 'on project boards filter' do + let(:board) { create(:board, project: project_1) } + + before do + visit project_board_path(project_1, board) + end + + it_behaves_like 'filtering by ancestor labels for projects', true + end + + context 'on group boards filter' do + let(:board) { create(:board, group: parent) } + + before do + visit group_board_path(parent, board) + end + + it_behaves_like 'filtering by ancestor labels for groups', true + end + end + + context 'creating boards lists' do + context 'on project boards' do + let(:board) { create(:board, project: project_1) } + + before do + visit project_board_path(project_1, board) + find('.js-new-board-list').click + wait_for_requests + end + + it 'creates lists from all ancestor labels' do + [grandparent_group_label, parent_group_label, project_label_1].each do |label| + find('a', text: label.title).click + end + + wait_for_requests + + expect(page).to have_selector('.board-title-text', text: grandparent_group_label.title) + expect(page).to have_selector('.board-title-text', text: parent_group_label.title) + expect(page).to have_selector('.board-title-text', text: project_label_1.title) + end + end + + context 'on group boards' do + let(:board) { create(:board, group: parent) } + + before do + visit group_board_path(parent, board) + find('.js-new-board-list').click + wait_for_requests + end + + it 'creates lists from all ancestor group labels' do + [grandparent_group_label, parent_group_label].each do |label| + find('a', text: label.title).click + end + + wait_for_requests + + expect(page).to have_selector('.board-title-text', text: grandparent_group_label.title) + expect(page).to have_selector('.board-title-text', text: parent_group_label.title) + end + + it 'does not create lists from descendant groups' do + expect(page).not_to have_selector('a', text: child_group_label.title) + end + end + end +end diff --git a/spec/features/projects/issues/user_sorts_issues_spec.rb b/spec/features/projects/issues/user_sorts_issues_spec.rb index 34148ae0116..c3d63000dac 100644 --- a/spec/features/projects/issues/user_sorts_issues_spec.rb +++ b/spec/features/projects/issues/user_sorts_issues_spec.rb @@ -25,17 +25,14 @@ describe "User sorts issues" do page.within(".issues-list") do page.within("li.issue:nth-child(1)") do expect(page).to have_content(issue1.title) - expect(page).to have_content("2 1") end page.within("li.issue:nth-child(2)") do expect(page).to have_content(issue2.title) - expect(page).to have_content("1 2") end page.within("li.issue:nth-child(3)") do expect(page).to have_content(issue3.title) - expect(page).not_to have_content("0 0") end end end diff --git a/spec/features/protected_branches_spec.rb b/spec/features/protected_branches_spec.rb index a4084818284..43cabd3b9f2 100644 --- a/spec/features/protected_branches_spec.rb +++ b/spec/features/protected_branches_spec.rb @@ -142,7 +142,10 @@ feature 'Protected Branches', :js do set_protected_branch_name('*-stable') click_on "Protect" - within(".protected-branches-list") { expect(page).to have_content("2 matching branches") } + within(".protected-branches-list") do + expect(page).to have_content("Protected branch (2)") + expect(page).to have_content("2 matching branches") + end end it "displays all the branches matching the wildcard" do diff --git a/spec/features/protected_tags_spec.rb b/spec/features/protected_tags_spec.rb index 8cc6f17b8d9..efccaeaff6c 100644 --- a/spec/features/protected_tags_spec.rb +++ b/spec/features/protected_tags_spec.rb @@ -65,7 +65,10 @@ feature 'Protected Tags', :js do set_protected_tag_name('*-stable') click_on "Protect" - within(".protected-tags-list") { expect(page).to have_content("2 matching tags") } + within(".protected-tags-list") do + expect(page).to have_content("Protected tag (2)") + expect(page).to have_content("2 matching tags") + end end it "displays all the tags matching the wildcard" do diff --git a/spec/finders/labels_finder_spec.rb b/spec/finders/labels_finder_spec.rb index d434c501110..899d0d22819 100644 --- a/spec/finders/labels_finder_spec.rb +++ b/spec/finders/labels_finder_spec.rb @@ -71,6 +71,24 @@ describe LabelsFinder do end end + context 'when group has no projects' do + let(:empty_group) { create(:group) } + let!(:empty_group_label_1) { create(:group_label, group: empty_group, title: 'Label 1 (empty group)') } + let!(:empty_group_label_2) { create(:group_label, group: empty_group, title: 'Label 2 (empty group)') } + + before do + empty_group.add_developer(user) + end + + context 'when only group labels is false' do + it 'returns group labels' do + finder = described_class.new(user, group_id: empty_group.id) + + expect(finder.execute).to eq [empty_group_label_1, empty_group_label_2] + end + end + end + context 'when including labels from group ancestors', :nested_groups do it 'returns labels from group and its ancestors' do private_group_1.add_developer(user) @@ -110,7 +128,21 @@ describe LabelsFinder do end end - context 'filtering by project_id' do + context 'filtering by project_id', :nested_groups do + context 'when include_ancestor_groups is true' do + let!(:sub_project) { create(:project, namespace: private_subgroup_1 ) } + let!(:project_label) { create(:label, project: sub_project, title: 'Label 5') } + let(:finder) { described_class.new(user, project_id: sub_project.id, include_ancestor_groups: true) } + + before do + private_group_1.add_developer(user) + end + + it 'returns all ancestor labels' do + expect(finder.execute).to match_array([private_subgroup_label_1, private_group_label_1, project_label]) + end + end + it 'returns labels available for the project' do finder = described_class.new(user, project_id: project_1.id) diff --git a/spec/javascripts/ide/components/commit_sidebar/list_item_spec.js b/spec/javascripts/ide/components/commit_sidebar/list_item_spec.js index 15b66952d99..509434e4300 100644 --- a/spec/javascripts/ide/components/commit_sidebar/list_item_spec.js +++ b/spec/javascripts/ide/components/commit_sidebar/list_item_spec.js @@ -1,8 +1,9 @@ import Vue from 'vue'; import listItem from '~/ide/components/commit_sidebar/list_item.vue'; import router from '~/ide/ide_router'; -import mountComponent from 'spec/helpers/vue_mount_component_helper'; -import { file } from '../../helpers'; +import store from '~/ide/stores'; +import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper'; +import { file, resetStore } from '../../helpers'; describe('Multi-file editor commit sidebar list item', () => { let vm; @@ -13,19 +14,21 @@ describe('Multi-file editor commit sidebar list item', () => { f = file('test-file'); - vm = mountComponent(Component, { + store.state.entries[f.path] = f; + + vm = createComponentWithStore(Component, store, { file: f, - }); + }).$mount(); }); afterEach(() => { vm.$destroy(); + + resetStore(store); }); it('renders file path', () => { - expect( - vm.$el.querySelector('.multi-file-commit-list-path').textContent.trim(), - ).toBe(f.path); + expect(vm.$el.querySelector('.multi-file-commit-list-path').textContent.trim()).toBe(f.path); }); it('calls discardFileChanges when clicking discard button', () => { @@ -36,25 +39,32 @@ describe('Multi-file editor commit sidebar list item', () => { expect(vm.discardFileChanges).toHaveBeenCalled(); }); - it('opens a closed file in the editor when clicking the file path', () => { + it('opens a closed file in the editor when clicking the file path', done => { spyOn(vm, 'openFileInEditor').and.callThrough(); - spyOn(vm, 'updateViewer'); spyOn(router, 'push'); vm.$el.querySelector('.multi-file-commit-list-path').click(); - expect(vm.openFileInEditor).toHaveBeenCalled(); - expect(router.push).toHaveBeenCalled(); + setTimeout(() => { + expect(vm.openFileInEditor).toHaveBeenCalled(); + expect(router.push).toHaveBeenCalled(); + + done(); + }); }); - it('calls updateViewer with diff when clicking file', () => { + it('calls updateViewer with diff when clicking file', done => { spyOn(vm, 'openFileInEditor').and.callThrough(); - spyOn(vm, 'updateViewer'); + spyOn(vm, 'updateViewer').and.callThrough(); spyOn(router, 'push'); vm.$el.querySelector('.multi-file-commit-list-path').click(); - expect(vm.updateViewer).toHaveBeenCalledWith('diff'); + setTimeout(() => { + expect(vm.updateViewer).toHaveBeenCalledWith('diff'); + + done(); + }); }); describe('computed', () => { diff --git a/spec/javascripts/ide/components/repo_tab_spec.js b/spec/javascripts/ide/components/repo_tab_spec.js index ddb5204e3a7..8cabc6e8935 100644 --- a/spec/javascripts/ide/components/repo_tab_spec.js +++ b/spec/javascripts/ide/components/repo_tab_spec.js @@ -59,7 +59,7 @@ describe('RepoTab', () => { vm.$el.querySelector('.multi-file-tab-close').click(); - expect(vm.closeFile).toHaveBeenCalledWith(vm.tab.path); + expect(vm.closeFile).toHaveBeenCalledWith(vm.tab); }); it('changes icon on hover', done => { diff --git a/spec/javascripts/ide/components/repo_tabs_spec.js b/spec/javascripts/ide/components/repo_tabs_spec.js index 73ea7960485..cb785ba2cd3 100644 --- a/spec/javascripts/ide/components/repo_tabs_spec.js +++ b/spec/javascripts/ide/components/repo_tabs_spec.js @@ -17,6 +17,7 @@ describe('RepoTabs', () => { files: openedFiles, viewer: 'editor', hasChanges: false, + activeFile: file('activeFile'), hasMergeRequest: false, }); openedFiles[0].active = true; @@ -57,6 +58,7 @@ describe('RepoTabs', () => { files: [], viewer: 'editor', hasChanges: false, + activeFile: file('activeFile'), hasMergeRequest: false, }, '#test-app', diff --git a/spec/javascripts/ide/lib/common/model_manager_spec.js b/spec/javascripts/ide/lib/common/model_manager_spec.js index 4381f6fcfd0..c00d590c580 100644 --- a/spec/javascripts/ide/lib/common/model_manager_spec.js +++ b/spec/javascripts/ide/lib/common/model_manager_spec.js @@ -27,9 +27,10 @@ describe('Multi-file editor library model manager', () => { }); it('caches model by file path', () => { - instance.addModel(file('path-name')); + const f = file('path-name'); + instance.addModel(f); - expect(instance.models.keys().next().value).toBe('path-name'); + expect(instance.models.keys().next().value).toBe(f.key); }); it('adds model into disposable', () => { @@ -56,7 +57,7 @@ describe('Multi-file editor library model manager', () => { instance.addModel(f); expect(eventHub.$on).toHaveBeenCalledWith( - `editor.update.model.dispose.${f.path}`, + `editor.update.model.dispose.${f.key}`, jasmine.anything(), ); }); @@ -68,9 +69,11 @@ describe('Multi-file editor library model manager', () => { }); it('returns true when model exists', () => { - instance.addModel(file('path-name')); + const f = file('path-name'); + + instance.addModel(f); - expect(instance.hasCachedModel('path-name')).toBeTruthy(); + expect(instance.hasCachedModel(f.key)).toBeTruthy(); }); }); @@ -103,7 +106,7 @@ describe('Multi-file editor library model manager', () => { instance.removeCachedModel(f); expect(eventHub.$off).toHaveBeenCalledWith( - `editor.update.model.dispose.${f.path}`, + `editor.update.model.dispose.${f.key}`, jasmine.anything(), ); }); diff --git a/spec/javascripts/ide/lib/common/model_spec.js b/spec/javascripts/ide/lib/common/model_spec.js index 7cd990adb53..8fc2fccb64c 100644 --- a/spec/javascripts/ide/lib/common/model_spec.js +++ b/spec/javascripts/ide/lib/common/model_spec.js @@ -32,14 +32,14 @@ describe('Multi-file editor library model', () => { it('adds eventHub listener', () => { expect(eventHub.$on).toHaveBeenCalledWith( - `editor.update.model.dispose.${model.file.path}`, + `editor.update.model.dispose.${model.file.key}`, jasmine.anything(), ); }); describe('path', () => { it('returns file path', () => { - expect(model.path).toBe('path'); + expect(model.path).toBe(model.file.key); }); }); @@ -74,7 +74,7 @@ describe('Multi-file editor library model', () => { model.onChange(() => {}); expect(model.events.size).toBe(1); - expect(model.events.keys().next().value).toBe('path'); + expect(model.events.keys().next().value).toBe(model.file.key); }); it('calls callback on change', done => { @@ -115,7 +115,7 @@ describe('Multi-file editor library model', () => { model.dispose(); expect(eventHub.$off).toHaveBeenCalledWith( - `editor.update.model.dispose.${model.file.path}`, + `editor.update.model.dispose.${model.file.key}`, jasmine.anything(), ); }); diff --git a/spec/javascripts/ide/lib/decorations/controller_spec.js b/spec/javascripts/ide/lib/decorations/controller_spec.js index 092170d086a..aec325e26a9 100644 --- a/spec/javascripts/ide/lib/decorations/controller_spec.js +++ b/spec/javascripts/ide/lib/decorations/controller_spec.js @@ -36,9 +36,7 @@ describe('Multi-file editor library decorations controller', () => { }); it('returns decorations by model URL', () => { - controller.addDecorations(model, 'key', [ - { decoration: 'decorationValue' }, - ]); + controller.addDecorations(model, 'key', [{ decoration: 'decorationValue' }]); const decorations = controller.getAllDecorationsForModel(model); @@ -48,39 +46,29 @@ describe('Multi-file editor library decorations controller', () => { describe('addDecorations', () => { it('caches decorations in a new map', () => { - controller.addDecorations(model, 'key', [ - { decoration: 'decorationValue' }, - ]); + controller.addDecorations(model, 'key', [{ decoration: 'decorationValue' }]); expect(controller.decorations.size).toBe(1); }); it('does not create new cache model', () => { - controller.addDecorations(model, 'key', [ - { decoration: 'decorationValue' }, - ]); - controller.addDecorations(model, 'key', [ - { decoration: 'decorationValue2' }, - ]); + controller.addDecorations(model, 'key', [{ decoration: 'decorationValue' }]); + controller.addDecorations(model, 'key', [{ decoration: 'decorationValue2' }]); expect(controller.decorations.size).toBe(1); }); it('caches decorations by model URL', () => { - controller.addDecorations(model, 'key', [ - { decoration: 'decorationValue' }, - ]); + controller.addDecorations(model, 'key', [{ decoration: 'decorationValue' }]); expect(controller.decorations.size).toBe(1); - expect(controller.decorations.keys().next().value).toBe('path'); + expect(controller.decorations.keys().next().value).toBe('path--path'); }); it('calls decorate method', () => { spyOn(controller, 'decorate'); - controller.addDecorations(model, 'key', [ - { decoration: 'decorationValue' }, - ]); + controller.addDecorations(model, 'key', [{ decoration: 'decorationValue' }]); expect(controller.decorate).toHaveBeenCalled(); }); @@ -92,10 +80,7 @@ describe('Multi-file editor library decorations controller', () => { controller.decorate(model); - expect(controller.editor.instance.deltaDecorations).toHaveBeenCalledWith( - [], - [], - ); + expect(controller.editor.instance.deltaDecorations).toHaveBeenCalledWith([], []); }); it('caches decorations', () => { @@ -111,15 +96,13 @@ describe('Multi-file editor library decorations controller', () => { controller.decorate(model); - expect(controller.editorDecorations.keys().next().value).toBe('path'); + expect(controller.editorDecorations.keys().next().value).toBe('path--path'); }); }); describe('dispose', () => { it('clears cached decorations', () => { - controller.addDecorations(model, 'key', [ - { decoration: 'decorationValue' }, - ]); + controller.addDecorations(model, 'key', [{ decoration: 'decorationValue' }]); controller.dispose(); @@ -127,9 +110,7 @@ describe('Multi-file editor library decorations controller', () => { }); it('clears cached editorDecorations', () => { - controller.addDecorations(model, 'key', [ - { decoration: 'decorationValue' }, - ]); + controller.addDecorations(model, 'key', [{ decoration: 'decorationValue' }]); controller.dispose(); diff --git a/spec/javascripts/ide/lib/diff/controller_spec.js b/spec/javascripts/ide/lib/diff/controller_spec.js index c8f3e9f4830..ff73240734e 100644 --- a/spec/javascripts/ide/lib/diff/controller_spec.js +++ b/spec/javascripts/ide/lib/diff/controller_spec.js @@ -131,7 +131,7 @@ describe('Multi-file editor library dirty diff controller', () => { it('adds decorations into decorations controller', () => { spyOn(controller.decorationsController, 'addDecorations'); - controller.decorate({ data: { changes: [], path: 'path' } }); + controller.decorate({ data: { changes: [], path: model.path } }); expect( controller.decorationsController.addDecorations, @@ -145,7 +145,7 @@ describe('Multi-file editor library dirty diff controller', () => { ); controller.decorate({ - data: { changes: computeDiff('123', '1234'), path: 'path' }, + data: { changes: computeDiff('123', '1234'), path: model.path }, }); expect(spy).toHaveBeenCalledWith( diff --git a/spec/javascripts/ide/stores/actions/file_spec.js b/spec/javascripts/ide/stores/actions/file_spec.js index 2f4516377cf..479ed7ce49e 100644 --- a/spec/javascripts/ide/stores/actions/file_spec.js +++ b/spec/javascripts/ide/stores/actions/file_spec.js @@ -29,7 +29,7 @@ describe('IDE store file actions', () => { it('closes open files', done => { store - .dispatch('closeFile', localFile.path) + .dispatch('closeFile', localFile) .then(() => { expect(localFile.opened).toBeFalsy(); expect(localFile.active).toBeFalsy(); @@ -44,7 +44,7 @@ describe('IDE store file actions', () => { store.state.changedFiles.push(localFile); store - .dispatch('closeFile', localFile.path) + .dispatch('closeFile', localFile) .then(Vue.nextTick) .then(() => { expect(store.state.openFiles.length).toBe(0); @@ -65,7 +65,7 @@ describe('IDE store file actions', () => { store.state.entries[f.path] = f; store - .dispatch('closeFile', localFile.path) + .dispatch('closeFile', localFile) .then(Vue.nextTick) .then(() => { expect(router.push).toHaveBeenCalledWith(`/project${f.url}`); @@ -74,6 +74,22 @@ describe('IDE store file actions', () => { }) .catch(done.fail); }); + + it('removes file if it pending', done => { + store.state.openFiles.push({ + ...localFile, + pending: true, + }); + + store + .dispatch('closeFile', localFile) + .then(() => { + expect(store.state.openFiles.length).toBe(0); + + done(); + }) + .catch(done.fail); + }); }); describe('setFileActive', () => { @@ -445,4 +461,113 @@ describe('IDE store file actions', () => { .catch(done.fail); }); }); + + describe('openPendingTab', () => { + let f; + + beforeEach(() => { + f = { + ...file(), + projectId: '123', + }; + + store.state.entries[f.path] = f; + }); + + it('makes file pending in openFiles', done => { + store + .dispatch('openPendingTab', f) + .then(() => { + expect(store.state.openFiles[0].pending).toBe(true); + }) + .then(done) + .catch(done.fail); + }); + + it('returns true when opened', done => { + store + .dispatch('openPendingTab', f) + .then(added => { + expect(added).toBe(true); + }) + .then(done) + .catch(done.fail); + }); + + it('pushes router URL when added', done => { + store.state.currentBranchId = 'master'; + + store + .dispatch('openPendingTab', f) + .then(() => { + expect(router.push).toHaveBeenCalledWith('/project/123/tree/master/'); + }) + .then(done) + .catch(done.fail); + }); + + it('calls scrollToTab', done => { + const scrollToTabSpy = jasmine.createSpy('scrollToTab'); + const oldScrollToTab = store._actions.scrollToTab; // eslint-disable-line + store._actions.scrollToTab = [scrollToTabSpy]; // eslint-disable-line + + store + .dispatch('openPendingTab', f) + .then(() => { + expect(scrollToTabSpy).toHaveBeenCalled(); + store._actions.scrollToTab = oldScrollToTab; // eslint-disable-line + }) + .then(done) + .catch(done.fail); + }); + + it('returns false when passed in file is active & viewer is diff', done => { + f.active = true; + store.state.openFiles.push(f); + store.state.viewer = 'diff'; + + store + .dispatch('openPendingTab', f) + .then(added => { + expect(added).toBe(false); + }) + .then(done) + .catch(done.fail); + }); + }); + + describe('removePendingTab', () => { + let f; + + beforeEach(() => { + spyOn(eventHub, '$emit'); + + f = { + ...file('pendingFile'), + pending: true, + }; + }); + + it('removes pending file from open files', done => { + store.state.openFiles.push(f); + + store + .dispatch('removePendingTab', f) + .then(() => { + expect(store.state.openFiles.length).toBe(0); + }) + .then(done) + .catch(done.fail); + }); + + it('emits event to dispose model', done => { + store + .dispatch('removePendingTab', f) + .then(() => { + expect(eventHub.$emit).toHaveBeenCalledWith(`editor.update.model.dispose.${f.key}`); + }) + .then(done) + .catch(done.fail); + }); + }); }); diff --git a/spec/javascripts/ide/stores/mutations/file_spec.js b/spec/javascripts/ide/stores/mutations/file_spec.js index 8fec94e882a..88285ee409f 100644 --- a/spec/javascripts/ide/stores/mutations/file_spec.js +++ b/spec/javascripts/ide/stores/mutations/file_spec.js @@ -22,6 +22,21 @@ describe('IDE store file mutations', () => { expect(localFile.active).toBeTruthy(); }); + + it('sets pending tab as not active', () => { + localState.openFiles.push({ + ...localFile, + pending: true, + active: true, + }); + + mutations.SET_FILE_ACTIVE(localState, { + path: localFile.path, + active: true, + }); + + expect(localState.openFiles[0].active).toBe(false); + }); }); describe('TOGGLE_FILE_OPEN', () => { @@ -178,4 +193,69 @@ describe('IDE store file mutations', () => { expect(localFile.changed).toBeTruthy(); }); }); + + describe('ADD_PENDING_TAB', () => { + beforeEach(() => { + const f = { + ...file('openFile'), + path: 'openFile', + active: true, + opened: true, + }; + + localState.entries[f.path] = f; + localState.openFiles.push(f); + }); + + it('adds file into openFiles as pending', () => { + mutations.ADD_PENDING_TAB(localState, { file: localFile }); + + expect(localState.openFiles.length).toBe(2); + expect(localState.openFiles[1].pending).toBe(true); + expect(localState.openFiles[1].key).toBe(`pending-${localFile.key}`); + }); + + it('updates open file to pending', () => { + mutations.ADD_PENDING_TAB(localState, { file: localState.openFiles[0] }); + + expect(localState.openFiles.length).toBe(1); + }); + + it('updates pending open file to active', () => { + localState.openFiles.push({ + ...localFile, + pending: true, + }); + + mutations.ADD_PENDING_TAB(localState, { file: localFile }); + + expect(localState.openFiles[1].pending).toBe(true); + expect(localState.openFiles[1].active).toBe(true); + }); + + it('sets all openFiles to not active', () => { + mutations.ADD_PENDING_TAB(localState, { file: localFile }); + + expect(localState.openFiles.length).toBe(2); + + localState.openFiles.forEach(f => { + if (f.pending) { + expect(f.active).toBe(true); + } else { + expect(f.active).toBe(false); + } + }); + }); + }); + + describe('REMOVE_PENDING_TAB', () => { + it('removes pending tab from openFiles', () => { + localFile.key = 'testing'; + localState.openFiles.push(localFile); + + mutations.REMOVE_PENDING_TAB(localState, localFile); + + expect(localState.openFiles.length).toBe(0); + }); + }); }); diff --git a/spec/lib/banzai/filter/commit_trailers_filter_spec.rb b/spec/lib/banzai/filter/commit_trailers_filter_spec.rb new file mode 100644 index 00000000000..1fd145116df --- /dev/null +++ b/spec/lib/banzai/filter/commit_trailers_filter_spec.rb @@ -0,0 +1,171 @@ +require 'spec_helper' +require 'ffaker' + +describe Banzai::Filter::CommitTrailersFilter do + include FilterSpecHelper + include CommitTrailersSpecHelper + + let(:secondary_email) { create(:email, :confirmed) } + let(:user) { create(:user) } + + let(:trailer) { "#{FFaker::Lorem.word}-by:"} + + let(:commit_message) { trailer_line(trailer, user.name, user.email) } + let(:commit_message_html) { commit_html(commit_message) } + + context 'detects' do + let(:email) { FFaker::Internet.email } + + it 'trailers in the form of *-by and replace users with links' do + doc = filter(commit_message_html) + + expect_to_have_user_link_with_avatar(doc, user: user, trailer: trailer) + end + + it 'trailers prefixed with whitespaces' do + message_html = commit_html("\n\r #{commit_message}") + + doc = filter(message_html) + + expect_to_have_user_link_with_avatar(doc, user: user, trailer: trailer) + end + + it 'GitLab users via a secondary email' do + _, message_html = build_commit_message( + trailer: trailer, + name: secondary_email.user.name, + email: secondary_email.email + ) + + doc = filter(message_html) + + expect_to_have_user_link_with_avatar( + doc, + user: secondary_email.user, + trailer: trailer, + email: secondary_email.email + ) + end + + it 'non GitLab users and replaces them with mailto links' do + _, message_html = build_commit_message( + trailer: trailer, + name: FFaker::Name.name, + email: email + ) + + doc = filter(message_html) + + expect_to_have_mailto_link(doc, email: email, trailer: trailer) + end + + it 'multiple trailers in the same message' do + different_trailer = "#{FFaker::Lorem.word}-by:" + message = commit_html %( + #{commit_message} + #{trailer_line(different_trailer, FFaker::Name.name, email)} + ) + + doc = filter(message) + + expect_to_have_user_link_with_avatar(doc, user: user, trailer: trailer) + expect_to_have_mailto_link(doc, email: email, trailer: different_trailer) + end + + context 'special names' do + where(:name) do + [ + 'John S. Doe', + 'L33t H@x0r' + ] + end + + with_them do + it do + message, message_html = build_commit_message( + trailer: trailer, + name: name, + email: email + ) + + doc = filter(message_html) + + expect_to_have_mailto_link(doc, email: email, trailer: trailer) + expect(doc.text).to match Regexp.escape(message) + end + end + end + end + + context "ignores" do + it 'commit messages without trailers' do + exp = message = commit_html(FFaker::Lorem.sentence) + doc = filter(message) + + expect(doc.to_html).to match Regexp.escape(exp) + end + + it 'trailers that are inline the commit message body' do + message = commit_html %( + #{FFaker::Lorem.sentence} #{commit_message} #{FFaker::Lorem.sentence} + ) + + doc = filter(message) + + expect(doc.css('a').size).to eq 0 + end + end + + context "structure" do + it 'preserves the commit trailer structure' do + doc = filter(commit_message_html) + + expect_to_have_user_link_with_avatar(doc, user: user, trailer: trailer) + expect(doc.text).to match Regexp.escape(commit_message) + end + + it 'preserves the original name used in the commit message' do + message, message_html = build_commit_message( + trailer: trailer, + name: FFaker::Name.name, + email: user.email + ) + + doc = filter(message_html) + + expect_to_have_user_link_with_avatar(doc, user: user, trailer: trailer) + expect(doc.text).to match Regexp.escape(message) + end + + it 'preserves the original email used in the commit message' do + message, message_html = build_commit_message( + trailer: trailer, + name: secondary_email.user.name, + email: secondary_email.email + ) + + doc = filter(message_html) + + expect_to_have_user_link_with_avatar( + doc, + user: secondary_email.user, + trailer: trailer, + email: secondary_email.email + ) + expect(doc.text).to match Regexp.escape(message) + end + + it 'only replaces trailer lines not the full commit message' do + commit_body = FFaker::Lorem.paragraph + message = commit_html %( + #{commit_body} + #{commit_message} + ) + + doc = filter(message) + + expect_to_have_user_link_with_avatar(doc, user: user, trailer: trailer) + expect(doc.text).to include(commit_body) + end + end +end diff --git a/spec/lib/banzai/filter/merge_request_reference_filter_spec.rb b/spec/lib/banzai/filter/merge_request_reference_filter_spec.rb index eeb82822f68..a1dd72c498f 100644 --- a/spec/lib/banzai/filter/merge_request_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/merge_request_reference_filter_spec.rb @@ -196,6 +196,41 @@ describe Banzai::Filter::MergeRequestReferenceFilter do end end + context 'URL reference for a commit' do + let(:mr) { create(:merge_request, :with_diffs) } + let(:reference) do + urls.project_merge_request_url(mr.project, mr) + "/diffs?commit_id=#{mr.diff_head_sha}" + end + let(:commit) { mr.commits.find { |commit| commit.sha == mr.diff_head_sha } } + + it 'links to a valid reference' do + doc = reference_filter("See #{reference}") + + expect(doc.css('a').first.attr('href')) + .to eq reference + end + + it 'has valid text' do + doc = reference_filter("See #{reference}") + + expect(doc.text).to eq("See #{mr.to_reference(full: true)} (#{commit.short_id})") + end + + it 'has valid title attribute' do + doc = reference_filter("See #{reference}") + + expect(doc.css('a').first.attr('title')).to eq(commit.title) + end + + it 'ignores invalid commit short_ids on link text' do + invalidate_commit_reference = + urls.project_merge_request_url(mr.project, mr) + "/diffs?commit_id=12345678" + doc = reference_filter("See #{invalidate_commit_reference}") + + expect(doc.text).to eq("See #{mr.to_reference(full: true)} (diffs)") + end + end + context 'cross-project URL reference' do let(:namespace) { create(:namespace, name: 'cross-reference') } let(:project2) { create(:project, :public, namespace: namespace) } diff --git a/spec/lib/gitlab/auth/ldap/access_spec.rb b/spec/lib/gitlab/auth/ldap/access_spec.rb index 9b3916bf9e3..6b251d824f7 100644 --- a/spec/lib/gitlab/auth/ldap/access_spec.rb +++ b/spec/lib/gitlab/auth/ldap/access_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe Gitlab::Auth::LDAP::Access do + include LdapHelpers + let(:access) { described_class.new user } let(:user) { create(:omniauth_user) } @@ -32,8 +34,10 @@ describe Gitlab::Auth::LDAP::Access do end context 'when the user is found' do + let(:ldap_user) { Gitlab::Auth::LDAP::Person.new(Net::LDAP::Entry.new, 'ldapmain') } + before do - allow(Gitlab::Auth::LDAP::Person).to receive(:find_by_dn).and_return(:ldap_user) + allow(Gitlab::Auth::LDAP::Person).to receive(:find_by_dn).and_return(ldap_user) end context 'and the user is disabled via active directory' do @@ -120,6 +124,22 @@ describe Gitlab::Auth::LDAP::Access do end end end + + context 'when the connection fails' do + before do + raise_ldap_connection_error + end + + it 'does not block the user' do + access.allowed? + + expect(user.ldap_blocked?).to be_falsey + end + + it 'denies access' do + expect(access.allowed?).to be_falsey + end + end end describe '#block_user' do diff --git a/spec/lib/gitlab/auth/ldap/adapter_spec.rb b/spec/lib/gitlab/auth/ldap/adapter_spec.rb index 10c60d792bd..3eeaf3862f6 100644 --- a/spec/lib/gitlab/auth/ldap/adapter_spec.rb +++ b/spec/lib/gitlab/auth/ldap/adapter_spec.rb @@ -124,16 +124,36 @@ describe Gitlab::Auth::LDAP::Adapter do context "when the search raises an LDAP exception" do before do + allow(adapter).to receive(:renew_connection_adapter).and_return(ldap) allow(ldap).to receive(:search) { raise Net::LDAP::Error, "some error" } allow(Rails.logger).to receive(:warn) end - it { is_expected.to eq [] } + context 'retries the operation' do + before do + stub_const("#{described_class}::MAX_SEARCH_RETRIES", 3) + end + + it 'as many times as MAX_SEARCH_RETRIES' do + expect(ldap).to receive(:search).exactly(3).times + expect { subject }.to raise_error(Gitlab::Auth::LDAP::LDAPConnectionError) + end + + context 'when no more retries' do + before do + stub_const("#{described_class}::MAX_SEARCH_RETRIES", 1) + end - it 'logs the error' do - subject - expect(Rails.logger).to have_received(:warn).with( - "LDAP search raised exception Net::LDAP::Error: some error") + it 'raises the exception' do + expect { subject }.to raise_error(Gitlab::Auth::LDAP::LDAPConnectionError) + end + + it 'logs the error' do + expect { subject }.to raise_error(Gitlab::Auth::LDAP::LDAPConnectionError) + expect(Rails.logger).to have_received(:warn).with( + "LDAP search raised exception Net::LDAP::Error: some error") + end + end end end end diff --git a/spec/lib/gitlab/auth/o_auth/user_spec.rb b/spec/lib/gitlab/auth/o_auth/user_spec.rb index 0c71f1d8ca6..64f3d09a25b 100644 --- a/spec/lib/gitlab/auth/o_auth/user_spec.rb +++ b/spec/lib/gitlab/auth/o_auth/user_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe Gitlab::Auth::OAuth::User do + include LdapHelpers + let(:oauth_user) { described_class.new(auth_hash) } let(:gl_user) { oauth_user.gl_user } let(:uid) { 'my-uid' } @@ -38,10 +40,6 @@ describe Gitlab::Auth::OAuth::User do end describe '#save' do - def stub_ldap_config(messages) - allow(Gitlab::Auth::LDAP::Config).to receive_messages(messages) - end - let(:provider) { 'twitter' } describe 'when account exists on server' do @@ -269,20 +267,47 @@ describe Gitlab::Auth::OAuth::User do end context 'when an LDAP person is not found by uid' do - it 'tries to find an LDAP person by DN and adds the omniauth identity to the user' do + it 'tries to find an LDAP person by email and adds the omniauth identity to the user' do allow(Gitlab::Auth::LDAP::Person).to receive(:find_by_uid).and_return(nil) - allow(Gitlab::Auth::LDAP::Person).to receive(:find_by_dn).and_return(ldap_user) + allow(Gitlab::Auth::LDAP::Person).to receive(:find_by_email).and_return(ldap_user) + + oauth_user.save + + identities_as_hash = gl_user.identities.map { |id| { provider: id.provider, extern_uid: id.extern_uid } } + expect(identities_as_hash).to match_array(result_identities(dn, uid)) + end + + context 'when also not found by email' do + it 'tries to find an LDAP person by DN and adds the omniauth identity to the user' do + allow(Gitlab::Auth::LDAP::Person).to receive(:find_by_uid).and_return(nil) + allow(Gitlab::Auth::LDAP::Person).to receive(:find_by_email).and_return(nil) + allow(Gitlab::Auth::LDAP::Person).to receive(:find_by_dn).and_return(ldap_user) + + oauth_user.save + + identities_as_hash = gl_user.identities.map { |id| { provider: id.provider, extern_uid: id.extern_uid } } + expect(identities_as_hash).to match_array(result_identities(dn, uid)) + end + end + end + def result_identities(dn, uid) + [ + { provider: 'ldapmain', extern_uid: dn }, + { provider: 'twitter', extern_uid: uid } + ] + end + + context 'when there is an LDAP connection error' do + before do + raise_ldap_connection_error + end + + it 'does not save the identity' do oauth_user.save identities_as_hash = gl_user.identities.map { |id| { provider: id.provider, extern_uid: id.extern_uid } } - expect(identities_as_hash) - .to match_array( - [ - { provider: 'ldapmain', extern_uid: dn }, - { provider: 'twitter', extern_uid: uid } - ] - ) + expect(identities_as_hash).to match_array([{ provider: 'twitter', extern_uid: uid }]) end end end @@ -739,4 +764,19 @@ describe Gitlab::Auth::OAuth::User do expect(oauth_user.find_user).to eql gl_user end end + + describe '#find_ldap_person' do + context 'when LDAP connection fails' do + before do + raise_ldap_connection_error + end + + it 'returns nil' do + adapter = Gitlab::Auth::LDAP::Adapter.new('ldapmain') + hash = OmniAuth::AuthHash.new(uid: 'whatever', provider: 'ldapmain') + + expect(oauth_user.send(:find_ldap_person, hash, adapter)).to be_nil + end + end + end end diff --git a/spec/lib/gitlab/bitbucket_import/importer_spec.rb b/spec/lib/gitlab/bitbucket_import/importer_spec.rb index a6a1d9e619f..c63120b0b29 100644 --- a/spec/lib/gitlab/bitbucket_import/importer_spec.rb +++ b/spec/lib/gitlab/bitbucket_import/importer_spec.rb @@ -137,7 +137,7 @@ describe Gitlab::BitbucketImport::Importer do it 'imports to the project disk_path' do expect(project.wiki).to receive(:repository_exists?) { false } expect(importer.gitlab_shell).to receive(:import_repository).with( - project.repository_storage_path, + project.repository_storage, project.wiki.disk_path, project.import_url + '/wiki' ) diff --git a/spec/lib/gitlab/git/gitlab_projects_spec.rb b/spec/lib/gitlab/git/gitlab_projects_spec.rb index dfccc15a4f3..8b715d717c1 100644 --- a/spec/lib/gitlab/git/gitlab_projects_spec.rb +++ b/spec/lib/gitlab/git/gitlab_projects_spec.rb @@ -16,7 +16,7 @@ describe Gitlab::Git::GitlabProjects do let(:tmp_repos_path) { TestEnv.repos_path } let(:repo_name) { project.disk_path + '.git' } let(:tmp_repo_path) { File.join(tmp_repos_path, repo_name) } - let(:gl_projects) { build_gitlab_projects(tmp_repos_path, repo_name) } + let(:gl_projects) { build_gitlab_projects(TestEnv::REPOS_STORAGE, repo_name) } describe '#initialize' do it { expect(gl_projects.shard_path).to eq(tmp_repos_path) } @@ -223,11 +223,12 @@ describe Gitlab::Git::GitlabProjects do end describe '#fork_repository' do + let(:dest_repos) { TestEnv::REPOS_STORAGE } let(:dest_repos_path) { tmp_repos_path } let(:dest_repo_name) { File.join('@hashed', 'aa', 'bb', 'xyz.git') } let(:dest_repo) { File.join(dest_repos_path, dest_repo_name) } - subject { gl_projects.fork_repository(dest_repos_path, dest_repo_name) } + subject { gl_projects.fork_repository(dest_repos, dest_repo_name) } before do FileUtils.mkdir_p(dest_repos_path) @@ -268,7 +269,12 @@ describe Gitlab::Git::GitlabProjects do # that is not very straight-forward so I'm leaving this test here for now till # https://gitlab.com/gitlab-org/gitlab-ce/issues/41393 is fixed. context 'different storages' do - let(:dest_repos_path) { File.join(File.dirname(tmp_repos_path), 'alternative') } + let(:dest_repos) { 'alternative' } + let(:dest_repos_path) { File.join(File.dirname(tmp_repos_path), dest_repos) } + + before do + stub_storage_settings(dest_repos => { 'path' => dest_repos_path }) + end it 'forks the repo' do is_expected.to be_truthy diff --git a/spec/lib/gitlab/github_import/importer/repository_importer_spec.rb b/spec/lib/gitlab/github_import/importer/repository_importer_spec.rb index 1f0f1fdd7da..879b1d9fb0f 100644 --- a/spec/lib/gitlab/github_import/importer/repository_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/repository_importer_spec.rb @@ -9,7 +9,7 @@ describe Gitlab::GithubImport::Importer::RepositoryImporter do :project, import_url: 'foo.git', import_source: 'foo/bar', - repository_storage_path: 'foo', + repository_storage: 'foo', disk_path: 'foo', repository: repository, create_wiki: true diff --git a/spec/lib/gitlab/shell_spec.rb b/spec/lib/gitlab/shell_spec.rb index ea5ce58e34b..7ff2c0639ec 100644 --- a/spec/lib/gitlab/shell_spec.rb +++ b/spec/lib/gitlab/shell_spec.rb @@ -14,7 +14,7 @@ describe Gitlab::Shell do allow(Project).to receive(:find).and_return(project) allow(gitlab_shell).to receive(:gitlab_projects) - .with(project.repository_storage_path, project.disk_path + '.git') + .with(project.repository_storage, project.disk_path + '.git') .and_return(gitlab_projects) end @@ -487,21 +487,21 @@ describe Gitlab::Shell do describe '#fork_repository' do subject do gitlab_shell.fork_repository( - project.repository_storage_path, + project.repository_storage, project.disk_path, - 'new/storage', + 'nfs-file05', 'fork/path' ) end it 'returns true when the command succeeds' do - expect(gitlab_projects).to receive(:fork_repository).with('new/storage', 'fork/path.git') { true } + expect(gitlab_projects).to receive(:fork_repository).with('nfs-file05', 'fork/path.git') { true } is_expected.to be_truthy end it 'return false when the command fails' do - expect(gitlab_projects).to receive(:fork_repository).with('new/storage', 'fork/path.git') { false } + expect(gitlab_projects).to receive(:fork_repository).with('nfs-file05', 'fork/path.git') { false } is_expected.to be_falsy end @@ -661,7 +661,7 @@ describe Gitlab::Shell do it 'returns true when the command succeeds' do expect(gitlab_projects).to receive(:import_project).with(import_url, timeout) { true } - result = gitlab_shell.import_repository(project.repository_storage_path, project.disk_path, import_url) + result = gitlab_shell.import_repository(project.repository_storage, project.disk_path, import_url) expect(result).to be_truthy end @@ -671,7 +671,7 @@ describe Gitlab::Shell do expect(gitlab_projects).to receive(:import_project) { false } expect do - gitlab_shell.import_repository(project.repository_storage_path, project.disk_path, import_url) + gitlab_shell.import_repository(project.repository_storage, project.disk_path, import_url) end.to raise_error(Gitlab::Shell::Error, "error") end end diff --git a/spec/lib/gitlab/sidekiq_logging/json_formatter_spec.rb b/spec/lib/gitlab/sidekiq_logging/json_formatter_spec.rb new file mode 100644 index 00000000000..fed9aeba30c --- /dev/null +++ b/spec/lib/gitlab/sidekiq_logging/json_formatter_spec.rb @@ -0,0 +1,31 @@ +require 'spec_helper' + +describe Gitlab::SidekiqLogging::JSONFormatter do + let(:hash_input) { { foo: 1, bar: 'test' } } + let(:message) { 'This is a test' } + let(:timestamp) { Time.now } + + it 'wraps a Hash' do + result = subject.call('INFO', timestamp, 'my program', hash_input) + + data = JSON.parse(result) + expected_output = hash_input.stringify_keys + expected_output['severity'] = 'INFO' + expected_output['time'] = timestamp.utc.iso8601(3) + + expect(data).to eq(expected_output) + end + + it 'wraps a String' do + result = subject.call('DEBUG', timestamp, 'my string', message) + + data = JSON.parse(result) + expected_output = { + severity: 'DEBUG', + time: timestamp.utc.iso8601(3), + message: message + } + + expect(data).to eq(expected_output.stringify_keys) + end +end diff --git a/spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb b/spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb new file mode 100644 index 00000000000..2421b1e5a1a --- /dev/null +++ b/spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb @@ -0,0 +1,101 @@ +require 'spec_helper' + +describe Gitlab::SidekiqLogging::StructuredLogger do + describe '#call' do + let(:timestamp) { Time.new('2018-01-01 12:00:00').utc } + let(:job) do + { + "class" => "TestWorker", + "args" => [1234, 'hello'], + "retry" => false, + "queue" => "cronjob:test_queue", + "queue_namespace" => "cronjob", + "jid" => "da883554ee4fe414012f5f42", + "created_at" => timestamp.to_f, + "enqueued_at" => timestamp.to_f + } + end + let(:logger) { double() } + let(:start_payload) do + job.merge( + 'message' => 'TestWorker JID-da883554ee4fe414012f5f42: start', + 'job_status' => 'start', + 'pid' => Process.pid, + 'created_at' => timestamp.iso8601(3), + 'enqueued_at' => timestamp.iso8601(3) + ) + end + let(:end_payload) do + start_payload.merge( + 'message' => 'TestWorker JID-da883554ee4fe414012f5f42: done: 0.0 sec', + 'job_status' => 'done', + 'duration' => 0.0, + "completed_at" => timestamp.iso8601(3) + ) + end + let(:exception_payload) do + end_payload.merge( + 'message' => 'TestWorker JID-da883554ee4fe414012f5f42: fail: 0.0 sec', + 'job_status' => 'fail', + 'error' => ArgumentError, + 'error_message' => 'some exception' + ) + end + + before do + allow(Sidekiq).to receive(:logger).and_return(logger) + + allow(subject).to receive(:current_time).and_return(timestamp.to_f) + end + + subject { described_class.new } + + context 'with SIDEKIQ_LOG_ARGUMENTS enabled' do + before do + stub_env('SIDEKIQ_LOG_ARGUMENTS', '1') + end + + it 'logs start and end of job' do + Timecop.freeze(timestamp) do + expect(logger).to receive(:info).with(start_payload).ordered + expect(logger).to receive(:info).with(end_payload).ordered + expect(subject).to receive(:log_job_start).and_call_original + expect(subject).to receive(:log_job_done).and_call_original + + subject.call(job, 'test_queue') { } + end + end + + it 'logs an exception in job' do + Timecop.freeze(timestamp) do + expect(logger).to receive(:info).with(start_payload) + # This excludes the exception_backtrace + expect(logger).to receive(:warn).with(hash_including(exception_payload)) + expect(subject).to receive(:log_job_start).and_call_original + expect(subject).to receive(:log_job_done).and_call_original + + expect do + subject.call(job, 'test_queue') do + raise ArgumentError, 'some exception' + end + end.to raise_error(ArgumentError) + end + end + end + + context 'with SIDEKIQ_LOG_ARGUMENTS disabled' do + it 'logs start and end of job' do + Timecop.freeze(timestamp) do + start_payload.delete('args') + + expect(logger).to receive(:info).with(start_payload).ordered + expect(logger).to receive(:info).with(end_payload).ordered + expect(subject).to receive(:log_job_start).and_call_original + expect(subject).to receive(:log_job_done).and_call_original + + subject.call(job, 'test_queue') { } + end + end + end + end +end diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index b7ed8be69fc..c536dab2681 100644 --- a/spec/models/commit_status_spec.rb +++ b/spec/models/commit_status_spec.rb @@ -368,9 +368,7 @@ describe CommitStatus do 'rspec:windows 0 : / 1' => 'rspec:windows', 'rspec:windows 0 : / 1 name' => 'rspec:windows name', '0 1 name ruby' => 'name ruby', - '0 :/ 1 name ruby' => 'name ruby', - 'golang test 1.8' => 'golang test', - '1.9 golang test' => 'golang test' + '0 :/ 1 name ruby' => 'name ruby' } tests.each do |name, group_name| diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index f8874d14e3f..05693f067e1 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -176,7 +176,7 @@ describe Issuable do end end - describe "#sort" do + describe "#sort_by_attribute" do let(:project) { create(:project) } context "by milestone due date" do @@ -193,12 +193,12 @@ describe Issuable do let!(:issue3) { create(:issue, project: project) } it "sorts desc" do - issues = project.issues.sort('milestone_due_desc') + issues = project.issues.sort_by_attribute('milestone_due_desc') expect(issues).to match_array([issue2, issue1, issue, issue3]) end it "sorts asc" do - issues = project.issues.sort('milestone_due_asc') + issues = project.issues.sort_by_attribute('milestone_due_asc') expect(issues).to match_array([issue1, issue2, issue, issue3]) end end @@ -210,7 +210,7 @@ describe Issuable do it 'has no duplicates across pages' do sorted_issue_ids = 1.upto(10).map do |i| - project.issues.sort('milestone_due_desc').page(i).per(1).first.id + project.issues.sort_by_attribute('milestone_due_desc').page(i).per(1).first.id end expect(sorted_issue_ids).to eq(sorted_issue_ids.uniq) diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index fef868ac0f2..0e560be9eaa 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1645,7 +1645,7 @@ describe Project do before do allow_any_instance_of(Gitlab::Shell).to receive(:import_repository) - .with(project.repository_storage_path, project.disk_path, project.import_url) + .with(project.repository_storage, project.disk_path, project.import_url) .and_return(true) expect_any_instance_of(Repository).to receive(:after_import) @@ -1798,10 +1798,7 @@ describe Project do let(:project) { forked_project_link.forked_to_project } it 'schedules a RepositoryForkWorker job' do - expect(RepositoryForkWorker).to receive(:perform_async).with( - project.id, - forked_from_project.repository_storage_path, - forked_from_project.disk_path).and_return(import_jid) + expect(RepositoryForkWorker).to receive(:perform_async).with(project.id).and_return(import_jid) expect(project.add_import_job).to eq(import_jid) end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 100418da804..4027c420e47 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1451,7 +1451,7 @@ describe User do end end - describe '#sort' do + describe '#sort_by_attribute' do before do described_class.delete_all @user = create :user, created_at: Date.today, current_sign_in_at: Date.today, name: 'Alpha' @@ -1460,7 +1460,7 @@ describe User do end context 'when sort by recent_sign_in' do - let(:users) { described_class.sort('recent_sign_in') } + let(:users) { described_class.sort_by_attribute('recent_sign_in') } it 'sorts users by recent sign-in time' do expect(users.first).to eq(@user) @@ -1473,7 +1473,7 @@ describe User do end context 'when sort by oldest_sign_in' do - let(:users) { described_class.sort('oldest_sign_in') } + let(:users) { described_class.sort_by_attribute('oldest_sign_in') } it 'sorts users by the oldest sign-in time' do expect(users.first).to eq(@user1) @@ -1486,15 +1486,15 @@ describe User do end it 'sorts users in descending order by their creation time' do - expect(described_class.sort('created_desc').first).to eq(@user) + expect(described_class.sort_by_attribute('created_desc').first).to eq(@user) end it 'sorts users in ascending order by their creation time' do - expect(described_class.sort('created_asc').first).to eq(@user2) + expect(described_class.sort_by_attribute('created_asc').first).to eq(@user2) end it 'sorts users by id in descending order when nil is passed' do - expect(described_class.sort(nil).first).to eq(@user2) + expect(described_class.sort_by_attribute(nil).first).to eq(@user2) end end diff --git a/spec/requests/api/boards_spec.rb b/spec/requests/api/boards_spec.rb index c6c10025f7f..92b614b087e 100644 --- a/spec/requests/api/boards_spec.rb +++ b/spec/requests/api/boards_spec.rb @@ -48,5 +48,36 @@ describe API::Boards do expect(json_response['label']['name']).to eq(group_label.title) expect(json_response['position']).to eq(3) end + + it 'creates a new board list for ancestor group labels' do + group = create(:group) + sub_group = create(:group, parent: group) + group_label = create(:group_label, group: group) + board_parent.update(group: sub_group) + group.add_developer(user) + sub_group.add_developer(user) + + post api(url, user), label_id: group_label.id + + expect(response).to have_gitlab_http_status(201) + expect(json_response['label']['name']).to eq(group_label.title) + end + end + + describe "POST /groups/:id/boards/lists", :nested_groups do + set(:group) { create(:group) } + set(:board_parent) { create(:group, parent: group ) } + let(:url) { "/groups/#{board_parent.id}/boards/#{board.id}/lists" } + set(:board) { create(:board, group: board_parent) } + + it 'creates a new board list for ancestor group labels' do + group.add_developer(user) + group_label = create(:group_label, group: group) + + post api(url, user), label_id: group_label.id + + expect(response).to have_gitlab_http_status(201) + expect(json_response['label']['name']).to eq(group_label.title) + end end end diff --git a/spec/requests/api/features_spec.rb b/spec/requests/api/features_spec.rb index 267058d98ee..c5354c2d639 100644 --- a/spec/requests/api/features_spec.rb +++ b/spec/requests/api/features_spec.rb @@ -1,8 +1,8 @@ require 'spec_helper' describe API::Features do - let(:user) { create(:user) } - let(:admin) { create(:admin) } + set(:user) { create(:user) } + set(:admin) { create(:admin) } before do Flipper.unregister_groups @@ -249,4 +249,43 @@ describe API::Features do end end end + + describe 'DELETE /feature/:name' do + let(:feature_name) { 'my_feature' } + + context 'when the user has no access' do + it 'returns a 401 for anonymous users' do + delete api("/features/#{feature_name}") + + expect(response).to have_gitlab_http_status(401) + end + + it 'returns a 403 for users' do + delete api("/features/#{feature_name}", user) + + expect(response).to have_gitlab_http_status(403) + end + end + + context 'when the user has access' do + it 'returns 204 when the value is not set' do + delete api("/features/#{feature_name}", admin) + + expect(response).to have_gitlab_http_status(204) + end + + context 'when the gate value was set' do + before do + Feature.get(feature_name).enable + end + + it 'deletes an enabled feature' do + delete api("/features/#{feature_name}", admin) + + expect(response).to have_gitlab_http_status(204) + expect(Feature.get(feature_name)).not_to be_enabled + end + end + end + end end diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index d73a42f48ad..2ec29a79e93 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -452,7 +452,8 @@ describe API::Projects do only_allow_merge_if_pipeline_succeeds: false, request_access_enabled: true, only_allow_merge_if_all_discussions_are_resolved: false, - ci_config_path: 'a/custom/path' + ci_config_path: 'a/custom/path', + merge_method: 'ff' }) post api('/projects', user), project @@ -569,6 +570,22 @@ describe API::Projects do expect(json_response['only_allow_merge_if_all_discussions_are_resolved']).to be_truthy end + it 'sets the merge method of a project to rebase merge' do + project = attributes_for(:project, merge_method: 'rebase_merge') + + post api('/projects', user), project + + expect(json_response['merge_method']).to eq('rebase_merge') + end + + it 'rejects invalid values for merge_method' do + project = attributes_for(:project, merge_method: 'totally_not_valid_method') + + post api('/projects', user), project + + expect(response).to have_gitlab_http_status(400) + end + it 'ignores import_url when it is nil' do project = attributes_for(:project, import_url: nil) @@ -823,6 +840,7 @@ describe API::Projects do expect(json_response['shared_with_groups'][0]['group_access_level']).to eq(link.group_access) expect(json_response['only_allow_merge_if_pipeline_succeeds']).to eq(project.only_allow_merge_if_pipeline_succeeds) expect(json_response['only_allow_merge_if_all_discussions_are_resolved']).to eq(project.only_allow_merge_if_all_discussions_are_resolved) + expect(json_response['merge_method']).to eq(project.merge_method.to_s) end it 'returns a project by path name' do @@ -1474,6 +1492,26 @@ describe API::Projects do expect(json_response[k.to_s]).to eq(v) end end + + it 'updates merge_method' do + project_param = { merge_method: 'ff' } + + put api("/projects/#{project3.id}", user), project_param + + expect(response).to have_gitlab_http_status(200) + + project_param.each_pair do |k, v| + expect(json_response[k.to_s]).to eq(v) + end + end + + it 'rejects to update merge_method when merge_method is invalid' do + project_param = { merge_method: 'invalid' } + + put api("/projects/#{project3.id}", user), project_param + + expect(response).to have_gitlab_http_status(400) + end end context 'when authenticated as project master' do @@ -1491,6 +1529,7 @@ describe API::Projects do wiki_enabled: true, snippets_enabled: true, merge_requests_enabled: true, + merge_method: 'ff', description: 'new description' } put api("/projects/#{project3.id}", user4), project_param diff --git a/spec/requests/projects/cycle_analytics_events_spec.rb b/spec/requests/projects/cycle_analytics_events_spec.rb index eef860821e5..bcc3e3a2678 100644 --- a/spec/requests/projects/cycle_analytics_events_spec.rb +++ b/spec/requests/projects/cycle_analytics_events_spec.rb @@ -23,7 +23,7 @@ describe 'cycle analytics events' do it 'lists the issue events' do get project_cycle_analytics_issue_path(project, format: :json) - first_issue_iid = project.issues.sort(:created_desc).pluck(:iid).first.to_s + first_issue_iid = project.issues.sort_by_attribute(:created_desc).pluck(:iid).first.to_s expect(json_response['events']).not_to be_empty expect(json_response['events'].first['iid']).to eq(first_issue_iid) @@ -32,7 +32,7 @@ describe 'cycle analytics events' do it 'lists the plan events' do get project_cycle_analytics_plan_path(project, format: :json) - first_mr_short_sha = project.merge_requests.sort(:created_asc).first.commits.first.short_id + first_mr_short_sha = project.merge_requests.sort_by_attribute(:created_asc).first.commits.first.short_id expect(json_response['events']).not_to be_empty expect(json_response['events'].first['short_sha']).to eq(first_mr_short_sha) @@ -43,7 +43,7 @@ describe 'cycle analytics events' do expect(json_response['events']).not_to be_empty - first_mr_iid = project.merge_requests.sort(:created_desc).pluck(:iid).first.to_s + first_mr_iid = project.merge_requests.sort_by_attribute(:created_desc).pluck(:iid).first.to_s expect(json_response['events'].first['iid']).to eq(first_mr_iid) end @@ -58,7 +58,7 @@ describe 'cycle analytics events' do it 'lists the review events' do get project_cycle_analytics_review_path(project, format: :json) - first_mr_iid = project.merge_requests.sort(:created_desc).pluck(:iid).first.to_s + first_mr_iid = project.merge_requests.sort_by_attribute(:created_desc).pluck(:iid).first.to_s expect(json_response['events']).not_to be_empty expect(json_response['events'].first['iid']).to eq(first_mr_iid) @@ -74,7 +74,7 @@ describe 'cycle analytics events' do it 'lists the production events' do get project_cycle_analytics_production_path(project, format: :json) - first_issue_iid = project.issues.sort(:created_desc).pluck(:iid).first.to_s + first_issue_iid = project.issues.sort_by_attribute(:created_desc).pluck(:iid).first.to_s expect(json_response['events']).not_to be_empty expect(json_response['events'].first['iid']).to eq(first_issue_iid) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index e8cecf361ff..beabba99cf5 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -108,7 +108,8 @@ RSpec.configure do |config| allow_any_instance_of(Gitlab::Git::GitlabProjects).to receive(:fork_repository).and_wrap_original do |m, *args| m.call(*args) - shard_path, repository_relative_path = args + shard_name, repository_relative_path = args + shard_path = Gitlab.config.repositories.storages.fetch(shard_name).legacy_disk_path # We can't leave the hooks in place after a fork, as those would fail in tests # The "internal" API is not available FileUtils.rm_rf(File.join(shard_path, repository_relative_path, 'hooks')) diff --git a/spec/support/commit_trailers_spec_helper.rb b/spec/support/commit_trailers_spec_helper.rb new file mode 100644 index 00000000000..add359946db --- /dev/null +++ b/spec/support/commit_trailers_spec_helper.rb @@ -0,0 +1,41 @@ +module CommitTrailersSpecHelper + extend ActiveSupport::Concern + + def expect_to_have_user_link_with_avatar(doc, user:, trailer:, email: nil) + wrapper = find_user_wrapper(doc, trailer) + + expect_to_have_links_with_url_and_avatar(wrapper, urls.user_url(user), email || user.email) + expect(wrapper.attribute('data-user').value).to eq user.id.to_s + end + + def expect_to_have_mailto_link(doc, email:, trailer:) + wrapper = find_user_wrapper(doc, trailer) + + expect_to_have_links_with_url_and_avatar(wrapper, "mailto:#{CGI.escape_html(email)}", email) + end + + def expect_to_have_links_with_url_and_avatar(doc, url, email) + expect(doc).not_to be_nil + expect(doc.xpath("a[position()<3 and @href='#{url}']").size).to eq 2 + expect(doc.xpath("a[position()=3 and @href='mailto:#{CGI.escape_html(email)}']").size).to eq 1 + expect(doc.css('img').size).to eq 1 + end + + def find_user_wrapper(doc, trailer) + doc.xpath("descendant-or-self::node()[@data-trailer='#{trailer}']").first + end + + def build_commit_message(trailer:, name:, email:) + message = trailer_line(trailer, name, email) + + [message, commit_html(message)] + end + + def trailer_line(trailer, name, email) + "#{trailer} #{name} <#{email}>" + end + + def commit_html(message) + "<pre>#{CGI.escape_html(message)}</pre>" + end +end diff --git a/spec/support/filtered_search_helpers.rb b/spec/support/filtered_search_helpers.rb index f3f96bd1f0a..5f42ff77fb2 100644 --- a/spec/support/filtered_search_helpers.rb +++ b/spec/support/filtered_search_helpers.rb @@ -21,6 +21,29 @@ module FilteredSearchHelpers end end + # Select a label clicking in the search dropdown instead + # of entering label names on the input. + def select_label_on_dropdown(label_title) + input_filtered_search("label:", submit: false) + + within('#js-dropdown-label') do + wait_for_requests + + find('li', text: label_title).click + end + + filtered_search.send_keys(:enter) + end + + def expect_issues_list_count(open_count, closed_count = 0) + all_count = open_count + closed_count + + expect(page).to have_issuable_counts(open: open_count, closed: closed_count, all: all_count) + page.within '.issues-list' do + expect(page).to have_selector('.issue', count: open_count) + end + end + # Enables input to be added character by character def input_filtered_search_keys(search_term) # Add an extra space to engage visual tokens diff --git a/spec/support/ldap_helpers.rb b/spec/support/ldap_helpers.rb index 081ce0ad7b7..0e87b3d359d 100644 --- a/spec/support/ldap_helpers.rb +++ b/spec/support/ldap_helpers.rb @@ -41,4 +41,9 @@ module LdapHelpers entry end + + def raise_ldap_connection_error + allow_any_instance_of(Gitlab::Auth::LDAP::Adapter) + .to receive(:ldap_search).and_raise(Gitlab::Auth::LDAP::LDAPConnectionError) + end end diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb index f14e69b1041..d87f265cdf0 100644 --- a/spec/support/test_env.rb +++ b/spec/support/test_env.rb @@ -62,6 +62,7 @@ module TestEnv }.freeze TMP_TEST_PATH = Rails.root.join('tmp', 'tests', '**') + REPOS_STORAGE = 'default'.freeze # Test environment # @@ -225,7 +226,7 @@ module TestEnv end def repos_path - Gitlab.config.repositories.storages.default.legacy_disk_path + Gitlab.config.repositories.storages[REPOS_STORAGE].legacy_disk_path end def backup_path diff --git a/spec/workers/repository_fork_worker_spec.rb b/spec/workers/repository_fork_worker_spec.rb index 6c66658d8c3..4b3c1736ea0 100644 --- a/spec/workers/repository_fork_worker_spec.rb +++ b/spec/workers/repository_fork_worker_spec.rb @@ -9,70 +9,91 @@ describe RepositoryForkWorker do describe "#perform" do let(:project) { create(:project, :repository) } - let(:fork_project) { create(:project, :repository, :import_scheduled, forked_from_project: project) } let(:shell) { Gitlab::Shell.new } + let(:fork_project) { create(:project, :repository, :import_scheduled, forked_from_project: project) } - before do - allow(subject).to receive(:gitlab_shell).and_return(shell) - end + shared_examples 'RepositoryForkWorker performing' do + before do + allow(subject).to receive(:gitlab_shell).and_return(shell) + end - def perform! - subject.perform(fork_project.id, '/test/path', project.disk_path) - end + def expect_fork_repository + expect(shell).to receive(:fork_repository).with( + 'default', + project.disk_path, + fork_project.repository_storage, + fork_project.disk_path + ) + end - def expect_fork_repository - expect(shell).to receive(:fork_repository).with( - '/test/path', - project.disk_path, - fork_project.repository_storage_path, - fork_project.disk_path - ) - end + describe 'when a worker was reset without cleanup' do + let(:jid) { '12345678' } - describe 'when a worker was reset without cleanup' do - let(:jid) { '12345678' } + it 'creates a new repository from a fork' do + allow(subject).to receive(:jid).and_return(jid) - it 'creates a new repository from a fork' do - allow(subject).to receive(:jid).and_return(jid) + expect_fork_repository.and_return(true) + perform! + end + end + + it "creates a new repository from a fork" do expect_fork_repository.and_return(true) perform! end - end - it "creates a new repository from a fork" do - expect_fork_repository.and_return(true) + it 'protects the default branch' do + expect_fork_repository.and_return(true) - perform! - end + perform! + + expect(fork_project.protected_branches.first.name).to eq(fork_project.default_branch) + end + + it 'flushes various caches' do + expect_fork_repository.and_return(true) - it 'protects the default branch' do - expect_fork_repository.and_return(true) + expect_any_instance_of(Repository).to receive(:expire_emptiness_caches) + .and_call_original - perform! + expect_any_instance_of(Repository).to receive(:expire_exists_cache) + .and_call_original - expect(fork_project.protected_branches.first.name).to eq(fork_project.default_branch) - end + perform! + end + + it "handles bad fork" do + error_message = "Unable to fork project #{fork_project.id} for repository #{project.disk_path} -> #{fork_project.disk_path}" - it 'flushes various caches' do - expect_fork_repository.and_return(true) + expect_fork_repository.and_return(false) - expect_any_instance_of(Repository).to receive(:expire_emptiness_caches) - .and_call_original + expect { perform! }.to raise_error(StandardError, error_message) + end + end - expect_any_instance_of(Repository).to receive(:expire_exists_cache) - .and_call_original + context 'only project ID passed' do + def perform! + subject.perform(fork_project.id) + end - perform! + it_behaves_like 'RepositoryForkWorker performing' end - it "handles bad fork" do - error_message = "Unable to fork project #{fork_project.id} for repository #{project.disk_path} -> #{fork_project.disk_path}" + context 'project ID, storage and repo paths passed' do + def perform! + subject.perform(fork_project.id, TestEnv.repos_path, project.disk_path) + end - expect_fork_repository.and_return(false) + it_behaves_like 'RepositoryForkWorker performing' - expect { perform! }.to raise_error(StandardError, error_message) + it 'logs a message about forking with old-style arguments' do + allow(Rails.logger).to receive(:info).with(anything) # To compensate for other logs + expect(Rails.logger).to receive(:info).with("Project #{fork_project.id} is being forked using old-style arguments.") + + perform! + end end end end |