diff options
48 files changed, 599 insertions, 967 deletions
diff --git a/app/assets/javascripts/boards/components/board_card.vue b/app/assets/javascripts/boards/components/board_card.vue index 12d68256598..faf722f61af 100644 --- a/app/assets/javascripts/boards/components/board_card.vue +++ b/app/assets/javascripts/boards/components/board_card.vue @@ -42,19 +42,12 @@ export default { return { showDetail: false, detailIssue: boardsStore.detail, - multiSelect: boardsStore.multiSelect, }; }, computed: { issueDetailVisible() { return this.detailIssue.issue && this.detailIssue.issue.id === this.issue.id; }, - multiSelectVisible() { - return this.multiSelect.list.findIndex(issue => issue.id === this.issue.id) > -1; - }, - canMultiSelect() { - return gon.features && gon.features.multiSelectBoard; - }, }, methods: { mouseDown() { @@ -65,20 +58,14 @@ export default { }, showIssue(e) { if (e.target.classList.contains('js-no-trigger')) return; + if (this.showDetail) { this.showDetail = false; - // If CMD or CTRL is clicked - const isMultiSelect = this.canMultiSelect && (e.ctrlKey || e.metaKey); - if (boardsStore.detail.issue && boardsStore.detail.issue.id === this.issue.id) { - eventHub.$emit('clearDetailIssue', isMultiSelect); - - if (isMultiSelect) { - eventHub.$emit('newDetailIssue', this.issue, isMultiSelect); - } + eventHub.$emit('clearDetailIssue'); } else { - eventHub.$emit('newDetailIssue', this.issue, isMultiSelect); + eventHub.$emit('newDetailIssue', this.issue); boardsStore.setListDetail(this.list); } } @@ -90,7 +77,6 @@ export default { <template> <li :class="{ - 'multi-select': multiSelectVisible, 'user-can-drag': !disabled && issue.id, 'is-disabled': disabled || !issue.id, 'is-active': issueDetailVisible, diff --git a/app/assets/javascripts/boards/components/board_list.vue b/app/assets/javascripts/boards/components/board_list.vue index 1273fcc6a91..de41698ca04 100644 --- a/app/assets/javascripts/boards/components/board_list.vue +++ b/app/assets/javascripts/boards/components/board_list.vue @@ -1,22 +1,12 @@ <script> -import { Sortable, MultiDrag } from 'sortablejs'; +/* eslint-disable @gitlab/vue-i18n/no-bare-strings */ +import Sortable from 'sortablejs'; import { GlLoadingIcon } from '@gitlab/ui'; -import _ from 'underscore'; import boardNewIssue from './board_new_issue.vue'; import boardCard from './board_card.vue'; import eventHub from '../eventhub'; import boardsStore from '../stores/boards_store'; -import { sprintf, __ } from '~/locale'; -import createFlash from '~/flash'; -import { - getBoardSortableDefaultOptions, - sortableStart, - sortableEnd, -} from '../mixins/sortable_default_options'; - -if (gon.features && gon.features.multiSelectBoard) { - Sortable.mount(new MultiDrag()); -} +import { getBoardSortableDefaultOptions, sortableStart } from '../mixins/sortable_default_options'; export default { name: 'BoardList', @@ -64,14 +54,6 @@ export default { showIssueForm: false, }; }, - computed: { - paginatedIssueText() { - return sprintf(__('Showing %{pageSize} of %{total} issues'), { - pageSize: this.list.issues.length, - total: this.list.issuesSize, - }); - }, - }, watch: { filters: { handler() { @@ -105,20 +87,11 @@ export default { eventHub.$on(`scroll-board-list-${this.list.id}`, this.scrollToTop); }, mounted() { - const multiSelectOpts = {}; - if (gon.features && gon.features.multiSelectBoard) { - multiSelectOpts.multiDrag = true; - multiSelectOpts.selectedClass = 'js-multi-select'; - multiSelectOpts.animation = 500; - } - const options = getBoardSortableDefaultOptions({ scroll: true, disabled: this.disabled, filter: '.board-list-count, .is-disabled', dataIdAttr: 'data-issue-id', - removeCloneOnHide: false, - ...multiSelectOpts, group: { name: 'issues', /** @@ -172,66 +145,25 @@ export default { card.showDetail = false; const { list } = card; - const issue = list.findIssue(Number(e.item.dataset.issueId)); - boardsStore.startMoving(list, issue); sortableStart(); }, onAdd: e => { - const { items = [], newIndicies = [] } = e; - if (items.length) { - // Not using e.newIndex here instead taking a min of all - // the newIndicies. Basically we have to find that during - // a drop what is the index we're going to start putting - // all the dropped elements from. - const newIndex = Math.min(...newIndicies.map(obj => obj.index).filter(i => i !== -1)); - const issues = items.map(item => - boardsStore.moving.list.findIssue(Number(item.dataset.issueId)), - ); + boardsStore.moveIssueToList( + boardsStore.moving.list, + this.list, + boardsStore.moving.issue, + e.newIndex, + ); - boardsStore.moveMultipleIssuesToList({ - listFrom: boardsStore.moving.list, - listTo: this.list, - issues, - newIndex, - }); - } else { - boardsStore.moveIssueToList( - boardsStore.moving.list, - this.list, - boardsStore.moving.issue, - e.newIndex, - ); - this.$nextTick(() => { - e.item.remove(); - }); - } + this.$nextTick(() => { + e.item.remove(); + }); }, onUpdate: e => { const sortedArray = this.sortable.toArray().filter(id => id !== '-1'); - - const { items = [], newIndicies = [], oldIndicies = [] } = e; - if (items.length) { - const newIndex = Math.min(...newIndicies.map(obj => obj.index)); - const issues = items.map(item => - boardsStore.moving.list.findIssue(Number(item.dataset.issueId)), - ); - boardsStore.moveMultipleIssuesInList({ - list: this.list, - issues, - oldIndicies: oldIndicies.map(obj => obj.index), - newIndex, - idArray: sortedArray, - }); - e.items.forEach(el => { - Sortable.utils.deselect(el); - }); - boardsStore.clearMultiSelect(); - return; - } - boardsStore.moveIssueInList( this.list, boardsStore.moving.issue, @@ -240,133 +172,9 @@ export default { sortedArray, ); }, - onEnd: e => { - const { items = [], clones = [], to } = e; - - // This is not a multi select operation - if (!items.length && !clones.length) { - sortableEnd(); - return; - } - - let toList; - if (to) { - const containerEl = to.closest('.js-board-list'); - toList = boardsStore.findList('id', Number(containerEl.dataset.board)); - } - - /** - * onEnd is called irrespective if the cards were moved in the - * same list or the other list. Don't remove items if it's same list. - */ - const isSameList = toList && toList.id === this.list.id; - - if (toList && !isSameList && boardsStore.shouldRemoveIssue(this.list, toList)) { - const issues = items.map(item => this.list.findIssue(Number(item.dataset.issueId))); - - if (_.compact(issues).length && !boardsStore.issuesAreContiguous(this.list, issues)) { - const indexes = []; - const ids = this.list.issues.map(i => i.id); - issues.forEach(issue => { - const index = ids.indexOf(issue.id); - if (index > -1) { - indexes.push(index); - } - }); - - // Descending sort because splice would cause index discrepancy otherwise - const sortedIndexes = indexes.sort((a, b) => (a < b ? 1 : -1)); - - sortedIndexes.forEach(i => { - /** - * **setTimeout and splice each element one-by-one in a loop - * is intended.** - * - * The problem here is all the indexes are in the list but are - * non-contiguous. Due to that, when we splice all the indexes, - * at once, Vue -- during a re-render -- is unable to find reference - * nodes and the entire app crashes. - * - * If the indexes are contiguous, this piece of code is not - * executed. If it is, this is a possible regression. Only when - * issue indexes are far apart, this logic should ever kick in. - */ - setTimeout(() => { - this.list.issues.splice(i, 1); - }, 0); - }); - } - } - - if (!toList) { - createFlash(__('Something went wrong while performing the action.')); - } - - if (!isSameList) { - boardsStore.clearMultiSelect(); - - // Since Vue's list does not re-render the same keyed item, we'll - // remove `multi-select` class to express it's unselected - if (clones && clones.length) { - clones.forEach(el => el.classList.remove('multi-select')); - } - - // Due to some bug which I am unable to figure out - // Sortable does not deselect some pending items from the - // source list. - // We'll just do it forcefully here. - Array.from(document.querySelectorAll('.js-multi-select') || []).forEach(item => { - Sortable.utils.deselect(item); - }); - - /** - * SortableJS leaves all the moving items "as is" on the DOM. - * Vue picks up and rehydrates the DOM, but we need to explicity - * remove the "trash" items from the DOM. - * - * This is in parity to the logic on single item move from a list/in - * a list. For reference, look at the implementation of onAdd method. - */ - this.$nextTick(() => { - if (items && items.length) { - items.forEach(item => { - item.remove(); - }); - } - }); - } - sortableEnd(); - }, onMove(e) { return !e.related.classList.contains('board-list-count'); }, - onSelect(e) { - const { - item: { classList }, - } = e; - - if ( - classList && - classList.contains('js-multi-select') && - !classList.contains('multi-select') - ) { - Sortable.utils.deselect(e.item); - } - }, - onDeselect: e => { - const { - item: { dataset, classList }, - } = e; - - if ( - classList && - classList.contains('multi-select') && - !classList.contains('js-multi-select') - ) { - const issue = this.list.findIssue(Number(dataset.issueId)); - boardsStore.toggleMultiSelect(issue); - } - }, }); this.sortable = Sortable.create(this.$refs.list, options); @@ -452,7 +260,7 @@ export default { <li v-if="showCount" class="board-list-count text-center" data-issue-id="-1"> <gl-loading-icon v-show="list.loadingMore" label="Loading more issues" /> <span v-if="list.issues.length === list.issuesSize">{{ __('Showing all issues') }}</span> - <span v-else>{{ paginatedIssueText }}</span> + <span v-else> Showing {{ list.issues.length }} of {{ list.issuesSize }} issues </span> </li> </ul> </div> diff --git a/app/assets/javascripts/boards/constants.js b/app/assets/javascripts/boards/constants.js deleted file mode 100644 index 3c66c7a0660..00000000000 --- a/app/assets/javascripts/boards/constants.js +++ /dev/null @@ -1,11 +0,0 @@ -export const ListType = { - assignee: 'assignee', - milestone: 'milestone', - backlog: 'backlog', - closed: 'closed', - label: 'label', -}; - -export default { - ListType, -}; diff --git a/app/assets/javascripts/boards/index.js b/app/assets/javascripts/boards/index.js index befca70eeae..da2669e7cde 100644 --- a/app/assets/javascripts/boards/index.js +++ b/app/assets/javascripts/boards/index.js @@ -146,7 +146,7 @@ export default () => { updateTokens() { this.filterManager.updateTokens(); }, - updateDetailIssue(newIssue, multiSelect = false) { + updateDetailIssue(newIssue) { const { sidebarInfoEndpoint } = newIssue; if (sidebarInfoEndpoint && newIssue.subscribed === undefined) { newIssue.setFetchingState('subscriptions', true); @@ -185,23 +185,9 @@ export default () => { }); } - if (multiSelect) { - boardsStore.toggleMultiSelect(newIssue); - - if (boardsStore.detail.issue) { - boardsStore.clearDetailIssue(); - return; - } - - return; - } - boardsStore.setIssueDetail(newIssue); }, - clearDetailIssue(multiSelect = false) { - if (multiSelect) { - boardsStore.clearMultiSelect(); - } + clearDetailIssue() { boardsStore.clearDetailIssue(); }, toggleSubscription(id) { diff --git a/app/assets/javascripts/boards/models/list.js b/app/assets/javascripts/boards/models/list.js index 1e213c324eb..b3e56a34c28 100644 --- a/app/assets/javascripts/boards/models/list.js +++ b/app/assets/javascripts/boards/models/list.js @@ -5,7 +5,6 @@ import ListLabel from './label'; import ListAssignee from './assignee'; import ListIssue from 'ee_else_ce/boards/models/issue'; import { urlParamsToObject } from '~/lib/utils/common_utils'; -import flash from '~/flash'; import boardsStore from '../stores/boards_store'; import ListMilestone from './milestone'; @@ -177,53 +176,6 @@ class List { }); } - addMultipleIssues(issues, listFrom, newIndex) { - let moveBeforeId = null; - let moveAfterId = null; - - const listHasIssues = issues.every(issue => this.findIssue(issue.id)); - - if (!listHasIssues) { - if (newIndex !== undefined) { - if (this.issues[newIndex - 1]) { - moveBeforeId = this.issues[newIndex - 1].id; - } - - if (this.issues[newIndex]) { - moveAfterId = this.issues[newIndex].id; - } - - this.issues.splice(newIndex, 0, ...issues); - } else { - this.issues.push(...issues); - } - - if (this.label) { - issues.forEach(issue => issue.addLabel(this.label)); - } - - if (this.assignee) { - if (listFrom && listFrom.type === 'assignee') { - issues.forEach(issue => issue.removeAssignee(listFrom.assignee)); - } - issues.forEach(issue => issue.addAssignee(this.assignee)); - } - - if (IS_EE && this.milestone) { - if (listFrom && listFrom.type === 'milestone') { - issues.forEach(issue => issue.removeMilestone(listFrom.milestone)); - } - issues.forEach(issue => issue.addMilestone(this.milestone)); - } - - if (listFrom) { - this.issuesSize += issues.length; - - this.updateMultipleIssues(issues, listFrom, moveBeforeId, moveAfterId); - } - } - } - addIssue(issue, listFrom, newIndex) { let moveBeforeId = null; let moveAfterId = null; @@ -278,23 +230,6 @@ class List { }); } - moveMultipleIssues({ issues, oldIndicies, newIndex, moveBeforeId, moveAfterId }) { - oldIndicies.reverse().forEach(index => { - this.issues.splice(index, 1); - }); - this.issues.splice(newIndex, 0, ...issues); - - gl.boardService - .moveMultipleIssues({ - ids: issues.map(issue => issue.id), - fromListId: null, - toListId: null, - moveBeforeId, - moveAfterId, - }) - .catch(() => flash(__('Something went wrong while moving issues.'))); - } - updateIssueLabel(issue, listFrom, moveBeforeId, moveAfterId) { gl.boardService .moveIssue(issue.id, listFrom.id, this.id, moveBeforeId, moveAfterId) @@ -303,37 +238,10 @@ class List { }); } - updateMultipleIssues(issues, listFrom, moveBeforeId, moveAfterId) { - gl.boardService - .moveMultipleIssues({ - ids: issues.map(issue => issue.id), - fromListId: listFrom.id, - toListId: this.id, - moveBeforeId, - moveAfterId, - }) - .catch(() => flash(__('Something went wrong while moving issues.'))); - } - findIssue(id) { return this.issues.find(issue => issue.id === id); } - removeMultipleIssues(removeIssues) { - const ids = removeIssues.map(issue => issue.id); - - this.issues = this.issues.filter(issue => { - const matchesRemove = ids.includes(issue.id); - - if (matchesRemove) { - this.issuesSize -= 1; - issue.removeLabel(this.label); - } - - return !matchesRemove; - }); - } - removeIssue(removeIssue) { this.issues = this.issues.filter(issue => { const matchesRemove = removeIssue.id === issue.id; diff --git a/app/assets/javascripts/boards/services/board_service.js b/app/assets/javascripts/boards/services/board_service.js index 03369febb4a..0d11db89511 100644 --- a/app/assets/javascripts/boards/services/board_service.js +++ b/app/assets/javascripts/boards/services/board_service.js @@ -48,16 +48,6 @@ export default class BoardService { return boardsStore.moveIssue(id, fromListId, toListId, moveBeforeId, moveAfterId); } - moveMultipleIssues({ - ids, - fromListId = null, - toListId = null, - moveBeforeId = null, - moveAfterId = null, - }) { - return boardsStore.moveMultipleIssues({ ids, fromListId, toListId, moveBeforeId, moveAfterId }); - } - newIssue(id, issue) { return boardsStore.newIssue(id, issue); } diff --git a/app/assets/javascripts/boards/stores/boards_store.js b/app/assets/javascripts/boards/stores/boards_store.js index 8b737d1dab0..6da1cca9628 100644 --- a/app/assets/javascripts/boards/stores/boards_store.js +++ b/app/assets/javascripts/boards/stores/boards_store.js @@ -11,7 +11,6 @@ import { __ } from '~/locale'; import axios from '~/lib/utils/axios_utils'; import { mergeUrlParams } from '~/lib/utils/url_utility'; import eventHub from '../eventhub'; -import { ListType } from '../constants'; const boardsStore = { disabled: false, @@ -40,7 +39,6 @@ const boardsStore = { issue: {}, list: {}, }, - multiSelect: { list: [] }, setEndpoints({ boardsEndpoint, listsEndpoint, bulkUpdatePath, boardId, recentBoardsEndpoint }) { const listsEndpointGenerate = `${listsEndpoint}/generate.json`; @@ -53,6 +51,7 @@ const boardsStore = { recentBoardsEndpoint: `${recentBoardsEndpoint}.json`, }; }, + create() { this.state.lists = []; this.filter.path = getUrlParamsArray().join('&'); @@ -135,107 +134,6 @@ const boardsStore = { Object.assign(this.moving, { list, issue }); }, - moveMultipleIssuesToList({ listFrom, listTo, issues, newIndex }) { - const issueTo = issues.map(issue => listTo.findIssue(issue.id)); - const issueLists = _.flatten(issues.map(issue => issue.getLists())); - const listLabels = issueLists.map(list => list.label); - - const hasMoveableIssues = _.compact(issueTo).length > 0; - - if (!hasMoveableIssues) { - // Check if target list assignee is already present in this issue - if ( - listTo.type === ListType.assignee && - listFrom.type === ListType.assignee && - issues.some(issue => issue.findAssignee(listTo.assignee)) - ) { - const targetIssues = issues.map(issue => listTo.findIssue(issue.id)); - targetIssues.forEach(targetIssue => targetIssue.removeAssignee(listFrom.assignee)); - } else if (listTo.type === 'milestone') { - const currentMilestones = issues.map(issue => issue.milestone); - const currentLists = this.state.lists - .filter(list => list.type === 'milestone' && list.id !== listTo.id) - .filter(list => - list.issues.some(listIssue => issues.some(issue => listIssue.id === issue.id)), - ); - - issues.forEach(issue => { - currentMilestones.forEach(milestone => { - issue.removeMilestone(milestone); - }); - }); - - issues.forEach(issue => { - issue.addMilestone(listTo.milestone); - }); - - currentLists.forEach(currentList => { - issues.forEach(issue => { - currentList.removeIssue(issue); - }); - }); - - listTo.addMultipleIssues(issues, listFrom, newIndex); - } else { - // Add to new lists issues if it doesn't already exist - listTo.addMultipleIssues(issues, listFrom, newIndex); - } - } else { - listTo.updateMultipleIssues(issues, listFrom); - issues.forEach(issue => { - issue.removeLabel(listFrom.label); - }); - } - - if (listTo.type === ListType.closed && listFrom.type !== ListType.backlog) { - issueLists.forEach(list => { - issues.forEach(issue => { - list.removeIssue(issue); - }); - }); - - issues.forEach(issue => { - issue.removeLabels(listLabels); - }); - } else if (listTo.type === ListType.backlog && listFrom.type === ListType.assignee) { - issues.forEach(issue => { - issue.removeAssignee(listFrom.assignee); - }); - issueLists.forEach(list => { - issues.forEach(issue => { - list.removeIssue(issue); - }); - }); - } else if (listTo.type === ListType.backlog && listFrom.type === ListType.milestone) { - issues.forEach(issue => { - issue.removeMilestone(listFrom.milestone); - }); - issueLists.forEach(list => { - issues.forEach(issue => { - list.removeIssue(issue); - }); - }); - } else if ( - this.shouldRemoveIssue(listFrom, listTo) && - this.issuesAreContiguous(listFrom, issues) - ) { - listFrom.removeMultipleIssues(issues); - } - }, - - issuesAreContiguous(list, issues) { - // When there's only 1 issue selected, we can return early. - if (issues.length === 1) return true; - - // Create list of ids for issues involved. - const listIssueIds = list.issues.map(issue => issue.id); - const movedIssueIds = issues.map(issue => issue.id); - - // Check if moved issue IDs is sub-array - // of source list issue IDs (i.e. contiguous selection). - return listIssueIds.join('|').includes(movedIssueIds.join('|')); - }, - moveIssueToList(listFrom, listTo, issue, newIndex) { const issueTo = listTo.findIssue(issue.id); const issueLists = issue.getLists(); @@ -297,17 +195,6 @@ const boardsStore = { list.moveIssue(issue, oldIndex, newIndex, beforeId, afterId); }, - moveMultipleIssuesInList({ list, issues, oldIndicies, newIndex, idArray }) { - const beforeId = parseInt(idArray[newIndex - 1], 10) || null; - const afterId = parseInt(idArray[newIndex + issues.length], 10) || null; - list.moveMultipleIssues({ - issues, - oldIndicies, - newIndex, - moveBeforeId: beforeId, - moveAfterId: afterId, - }); - }, findList(key, val, type = 'label') { const filteredList = this.state.lists.filter(list => { const byType = type @@ -373,10 +260,6 @@ const boardsStore = { }`; }, - generateMultiDragPath(boardId) { - return `${gon.relative_url_root}/-/boards/${boardId ? `${boardId}` : ''}/issues/bulk_move`; - }, - all() { return axios.get(this.state.endpoints.listsEndpoint); }, @@ -426,16 +309,6 @@ const boardsStore = { }); }, - moveMultipleIssues({ ids, fromListId, toListId, moveBeforeId, moveAfterId }) { - return axios.put(this.generateMultiDragPath(this.state.endpoints.boardId), { - from_list_id: fromListId, - to_list_id: toListId, - move_before_id: moveBeforeId, - move_after_id: moveAfterId, - ids, - }); - }, - newIssue(id, issue) { return axios.post(this.generateIssuesPath(id), { issue, @@ -506,25 +379,6 @@ const boardsStore = { setCurrentBoard(board) { this.state.currentBoard = board; }, - - toggleMultiSelect(issue) { - const selectedIssueIds = this.multiSelect.list.map(issue => issue.id); - const index = selectedIssueIds.indexOf(issue.id); - - if (index === -1) { - this.multiSelect.list.push(issue); - return; - } - - this.multiSelect.list = [ - ...this.multiSelect.list.slice(0, index), - ...this.multiSelect.list.slice(index + 1), - ]; - }, - - clearMultiSelect() { - this.multiSelect.list = []; - }, }; BoardsStoreEE.initEESpecific(boardsStore); diff --git a/app/assets/javascripts/test_utils/index.js b/app/assets/javascripts/test_utils/index.js index 1a1f3e8d0a8..1e75ee60671 100644 --- a/app/assets/javascripts/test_utils/index.js +++ b/app/assets/javascripts/test_utils/index.js @@ -1,10 +1,8 @@ import 'core-js/es/map'; import 'core-js/es/set'; -import { Sortable } from 'sortablejs'; import simulateDrag from './simulate_drag'; import simulateInput from './simulate_input'; // Export to global space for rspec to use window.simulateDrag = simulateDrag; window.simulateInput = simulateInput; -window.Sortable = Sortable; diff --git a/app/assets/stylesheets/pages/boards.scss b/app/assets/stylesheets/pages/boards.scss index 2a7a53d8bd7..d540a347dde 100644 --- a/app/assets/stylesheets/pages/boards.scss +++ b/app/assets/stylesheets/pages/boards.scss @@ -245,7 +245,6 @@ box-shadow: 0 1px 2px $issue-boards-card-shadow; line-height: $gl-padding; list-style: none; - position: relative; &:not(:last-child) { margin-bottom: $gl-padding-8; @@ -256,11 +255,6 @@ background-color: $blue-50; } - &.multi-select { - border-color: $blue-200; - background-color: $blue-50; - } - .badge { border: 0; outline: 0; diff --git a/app/assets/stylesheets/pages/help.scss b/app/assets/stylesheets/pages/help.scss index ef872e693e0..ab281bc7f23 100644 --- a/app/assets/stylesheets/pages/help.scss +++ b/app/assets/stylesheets/pages/help.scss @@ -37,6 +37,7 @@ .documentation { padding: 7px; + font-size: $gl-font-size-large; } .card.links-card { diff --git a/app/controllers/groups/boards_controller.rb b/app/controllers/groups/boards_controller.rb index 3c86f3108ab..40b8d5ed72c 100644 --- a/app/controllers/groups/boards_controller.rb +++ b/app/controllers/groups/boards_controller.rb @@ -5,9 +5,6 @@ class Groups::BoardsController < Groups::ApplicationController include RecordUserLastActivity before_action :assign_endpoint_vars - before_action do - push_frontend_feature_flag(:multi_select_board) - end private diff --git a/app/controllers/projects/boards_controller.rb b/app/controllers/projects/boards_controller.rb index 3b335fa4af4..14b02993e6e 100644 --- a/app/controllers/projects/boards_controller.rb +++ b/app/controllers/projects/boards_controller.rb @@ -7,9 +7,6 @@ class Projects::BoardsController < Projects::ApplicationController before_action :check_issues_available! before_action :authorize_read_board!, only: [:index, :show] before_action :assign_endpoint_vars - before_action do - push_frontend_feature_flag(:multi_select_board) - end private diff --git a/app/models/concerns/atomic_internal_id.rb b/app/models/concerns/atomic_internal_id.rb index b510129b35d..7a7e485a95a 100644 --- a/app/models/concerns/atomic_internal_id.rb +++ b/app/models/concerns/atomic_internal_id.rb @@ -60,12 +60,11 @@ module AtomicInternalId iid_always_track = Feature.enabled?(:iid_always_track, default_enabled: true) return unless @internal_id_needs_tracking || iid_always_track - @internal_id_needs_tracking = false - scope_value = internal_id_read_scope(scope) - value = read_attribute(column) return unless scope_value + value = read_attribute(column) + if value.present? # The value was set externally, e.g. by the user # We update the InternalId record to keep track of the greatest value. @@ -75,6 +74,8 @@ module AtomicInternalId internal_id_scope_usage, value, init) + + @internal_id_needs_tracking = false end end diff --git a/app/services/git/process_ref_changes_service.rb b/app/services/git/process_ref_changes_service.rb new file mode 100644 index 00000000000..33925147750 --- /dev/null +++ b/app/services/git/process_ref_changes_service.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +module Git + class ProcessRefChangesService < BaseService + PIPELINE_PROCESS_LIMIT = 4 + + def execute + changes = params[:changes] + + process_changes_by_action(:branch, changes.branch_changes) + process_changes_by_action(:tag, changes.tag_changes) + end + + private + + def process_changes_by_action(ref_type, changes) + changes_by_action = group_changes_by_action(changes) + + changes_by_action.each do |_, changes| + process_changes(ref_type, changes) if changes.any? + end + end + + def group_changes_by_action(changes) + changes.group_by do |change| + change_action(change) + end + end + + def change_action(change) + return :created if Gitlab::Git.blank_ref?(change[:oldrev]) + return :removed if Gitlab::Git.blank_ref?(change[:newrev]) + + :pushed + end + + def process_changes(ref_type, changes) + push_service_class = push_service_class_for(ref_type) + + changes.each do |change| + push_service_class.new( + project, + current_user, + change: change, + push_options: params[:push_options], + create_pipelines: change[:index] < PIPELINE_PROCESS_LIMIT || Feature.enabled?(:git_push_create_all_pipelines, project) + ).execute + end + end + + def push_service_class_for(ref_type) + return Git::TagPushService if ref_type == :tag + + Git::BranchPushService + end + end +end diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb index dc3c363f650..528b1ea61b3 100644 --- a/app/services/issues/update_service.rb +++ b/app/services/issues/update_service.rb @@ -56,7 +56,7 @@ module Issues handle_milestone_change(issue) - added_mentions = issue.mentioned_users - old_mentioned_users + added_mentions = issue.mentioned_users(current_user) - old_mentioned_users if added_mentions.present? notification_service.async.new_mentions_in_issue(issue, added_mentions, current_user) diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb index 4acc3f1981a..ae678d4c036 100644 --- a/app/services/merge_requests/update_service.rb +++ b/app/services/merge_requests/update_service.rb @@ -69,7 +69,8 @@ module MergeRequests ) end - added_mentions = merge_request.mentioned_users - old_mentioned_users + added_mentions = merge_request.mentioned_users(current_user) - old_mentioned_users + if added_mentions.present? notification_service.async.new_mentions_in_merge_request( merge_request, diff --git a/app/services/notes/update_service.rb b/app/services/notes/update_service.rb index 853faed9d85..573be8fbe8b 100644 --- a/app/services/notes/update_service.rb +++ b/app/services/notes/update_service.rb @@ -5,7 +5,7 @@ module Notes def execute(note) return note unless note.editable? - old_mentioned_users = note.mentioned_users.to_a + old_mentioned_users = note.mentioned_users(current_user).to_a note.update(params.merge(updated_by: current_user)) diff --git a/app/views/help/show.html.haml b/app/views/help/show.html.haml index dce27dee9be..dace8a77736 100644 --- a/app/views/help/show.html.haml +++ b/app/views/help/show.html.haml @@ -1,3 +1,5 @@ - page_title @path.split("/").reverse.map(&:humanize) +- @content_class = "limit-container-width" unless fluid_layout + .documentation.md.prepend-top-default = markdown @markdown diff --git a/app/workers/post_receive.rb b/app/workers/post_receive.rb index a4b9ef18a3b..4f193e95faa 100644 --- a/app/workers/post_receive.rb +++ b/app/workers/post_receive.rb @@ -3,8 +3,6 @@ class PostReceive include ApplicationWorker - PIPELINE_PROCESS_LIMIT = 4 - def perform(gl_repository, identifier, changes, push_options = {}) project, repo_type = Gitlab::GlRepository.parse(gl_repository) @@ -49,8 +47,7 @@ class PostReceive expire_caches(post_received, post_received.project.repository) enqueue_repository_cache_update(post_received) - process_changes(Git::BranchPushService, project, user, push_options, changes.branch_changes) - process_changes(Git::TagPushService, project, user, push_options, changes.tag_changes) + process_ref_changes(project, user, push_options: push_options, changes: changes) update_remote_mirrors(post_received) after_project_changes_hooks(project, user, changes.refs, changes.repository_data) end @@ -75,18 +72,10 @@ class PostReceive ) end - def process_changes(service_class, project, user, push_options, changes) - return if changes.empty? - - changes.each do |change| - service_class.new( - project, - user, - change: change, - push_options: push_options, - create_pipelines: change[:index] < PIPELINE_PROCESS_LIMIT || Feature.enabled?(:git_push_create_all_pipelines, project) - ).execute - end + def process_ref_changes(project, user, params = {}) + return unless params[:changes].any? + + Git::ProcessRefChangesService.new(project, user, params).execute end def update_remote_mirrors(post_received) diff --git a/changelogs/unreleased/30112-private-internal-sub-group-email-notifications-not-sent-when-mentio.yml b/changelogs/unreleased/30112-private-internal-sub-group-email-notifications-not-sent-when-mentio.yml new file mode 100644 index 00000000000..fd20fc274e8 --- /dev/null +++ b/changelogs/unreleased/30112-private-internal-sub-group-email-notifications-not-sent-when-mentio.yml @@ -0,0 +1,5 @@ +--- +title: Fix notifications for private group mentions in Notes, Issues, and Merge Requests +merge_request: 18183 +author: +type: fixed diff --git a/changelogs/unreleased/32030-move-licence-complience-to-the-backend-FE.yml b/changelogs/unreleased/32030-move-licence-complience-to-the-backend-FE.yml new file mode 100644 index 00000000000..a37732960a4 --- /dev/null +++ b/changelogs/unreleased/32030-move-licence-complience-to-the-backend-FE.yml @@ -0,0 +1,5 @@ +--- +title: Moves the license compliance reports to the Backend +merge_request: 17905 +author: +type: other diff --git a/changelogs/unreleased/dz-improve-help-layout.yml b/changelogs/unreleased/dz-improve-help-layout.yml new file mode 100644 index 00000000000..2e4d885e2c2 --- /dev/null +++ b/changelogs/unreleased/dz-improve-help-layout.yml @@ -0,0 +1,5 @@ +--- +title: Improve UI of documentation under /help +merge_request: 18331 +author: +type: changed diff --git a/doc/README.md b/doc/README.md index 0ba2fd27b6a..61265f94004 100644 --- a/doc/README.md +++ b/doc/README.md @@ -258,6 +258,7 @@ The following documentation relates to the DevOps **Package** stage: |:----------------------------------------------------------------|:-------------------------------------------------------| | [Container Registry](user/packages/container_registry/index.md) | The GitLab Container Registry enables every project in GitLab to have its own space to store [Docker](https://www.docker.com/) images. | | [Dependency Proxy](user/packages/dependency_proxy/index.md) **(PREMIUM)** | The GitLab Dependency Proxy sets up a local proxy for frequently used upstream images/packages. | +| [Conan Repository](user/packages/conan_repository/index.md) **(PREMIUM)** | The GitLab Conan Repository enables every project in GitLab to have its own space to store [Conan](https://conan.io/) packages. | | [Maven Repository](user/packages/maven_repository/index.md) **(PREMIUM)** | The GitLab Maven Repository enables every project in GitLab to have its own space to store [Maven](https://maven.apache.org/) packages. | | [NPM Registry](user/packages/npm_registry/index.md) **(PREMIUM)** | The GitLab NPM Registry enables every project in GitLab to have its own space to store [NPM](https://www.npmjs.com/) packages. | diff --git a/doc/administration/geo/replication/index.md b/doc/administration/geo/replication/index.md index 518291e8d8c..bcad820531c 100644 --- a/doc/administration/geo/replication/index.md +++ b/doc/administration/geo/replication/index.md @@ -281,6 +281,7 @@ You can keep track of the progress to include the missing items in: | [Container Registry](../../packages/container_registry.md) | Yes | No | | [NPM Registry](../../../user/packages/npm_registry/index.md) | No | No | | [Maven Packages](../../../user/packages/maven_repository/index.md) | No | No | +| [Conan Packages](../../../user/packages/conan_repository/index.md) | No | No | | [External merge request diffs](../../merge_request_diffs.md) | No, if they are on-disk | No | | Content in object storage ([track progress](https://gitlab.com/groups/gitlab-org/-/epics/1526)) | No | No | diff --git a/doc/administration/packages/index.md b/doc/administration/packages/index.md index 99ec5811681..d4afc65577e 100644 --- a/doc/administration/packages/index.md +++ b/doc/administration/packages/index.md @@ -8,6 +8,7 @@ The Packages feature allows GitLab to act as a repository for the following: | Software repository | Description | Available in GitLab version | | ------------------- | ----------- | --------------------------- | +| [Conan Repository](../../user/packages/conan_repository/index.md) | The GitLab Conan Repository enables every project in GitLab to have its own space to store [Conan](https://conan.io/) packages. | 12.4+ | | [Maven Repository](../../user/packages/maven_repository/index.md) | The GitLab Maven Repository enables every project in GitLab to have its own space to store [Maven](https://maven.apache.org/) packages. | 11.3+ | | [NPM Registry](../../user/packages/npm_registry/index.md) | The GitLab NPM Registry enables every project in GitLab to have its own space to store [NPM](https://www.npmjs.com/) packages. | 11.7+ | diff --git a/doc/ci/introduction/index.md b/doc/ci/introduction/index.md index 4389b2ce015..a644a89eee4 100644 --- a/doc/ci/introduction/index.md +++ b/doc/ci/introduction/index.md @@ -190,6 +190,7 @@ according to each stage (Verify, Package, Release). - Store Docker images with [Container Registry](../../user/packages/container_registry/index.md). - Store NPM packages with [NPM Registry](../../user/packages/npm_registry/index.md). **(PREMIUM)** - Store Maven artifacts with [Maven Repository](../../user/packages/maven_repository/index.md). **(PREMIUM)** + - Store Conan packages with [Conan Repository](../../user/packages/conan_repository/index.md). **(PREMIUM)** 1. **Release**: - Continuous Deployment, automatically deploying your app to production. - Continuous Delivery, manually click to deploy your app to production. diff --git a/doc/development/documentation/index.md b/doc/development/documentation/index.md index a5b47b3a5c0..4ac33fddfd2 100644 --- a/doc/development/documentation/index.md +++ b/doc/development/documentation/index.md @@ -374,17 +374,29 @@ The following GitLab features are used among others: ## Testing -We treat documentation as code, and so use tests to maintain the standards and quality of the docs. -The current tests are: - -1. `docs lint`: Check that all internal (relative) links work correctly and - that all cURL examples in API docs use the full switches. It's recommended - to [check locally](#previewing-the-changes-live) before pushing to GitLab by executing the command - `bundle exec nanoc check internal_links` on your local - [`gitlab-docs`](https://gitlab.com/gitlab-org/gitlab-docs) directory. In addition, - `docs-lint` also runs [`markdownlint`](#markdownlint) to ensure the - markdown is consistent across all documentation. -1. In a full pipeline, tests for [`/help`](#gitlab-help-tests). +We treat documentation as code, and so use tests in our CI pipeline to maintain the +standards and quality of the docs. The current tests, which run in CI jobs when a +merge request with new or changed docs is submitted, are: + +- [`docs lint`](https://gitlab.com/gitlab-org/gitlab/blob/master/.gitlab/ci/docs.gitlab-ci.yml#L48): + Runs several tests on the content of the docs themselves: + - [`lint-doc.sh` script](https://gitlab.com/gitlab-org/gitlab/blob/master/scripts/lint-doc.sh) + checks that: + - All cURL examples use the long flags (ex: `--header`, not `-H`). + - The `CHANGELOG.md` does not contain duplicate versions. + - No files in `doc/` are executable. + - No new `README.md` was added. + - [`markdownlint`](#markdownlint). + - Nanoc tests, which you can [run locally](#previewing-the-changes-live) before + pushing to GitLab by executing the command `bundle exec nanoc check internal_links` + (or `internal_anchors`) on your local [`gitlab-docs`](https://gitlab.com/gitlab-org/gitlab-docs) directory: + - [`internal_links`](https://gitlab.com/gitlab-org/gitlab/blob/master/.gitlab/ci/docs.gitlab-ci.yml#L67) + checks that all internal links (ex: `[link](../index.md)`) are valid. + - [`internal_anchors`](https://gitlab.com/gitlab-org/gitlab/blob/master/.gitlab/ci/docs.gitlab-ci.yml#L69) + checks that all internal anchors (ex: `[link](../index.md#internal_anchor)`) + are valid. +- If any code or the `doc/README.md` file is changed, a full pipeline will run, which + runs tests for [`/help`](#gitlab-help-tests). ### Linting @@ -490,7 +502,10 @@ four repos that are the sources for <https://docs.gitlab.com>: - <https://gitlab.com/charts/gitlab/blob/master/.markdownlint.json> By default all rules are enabled, so the configuration file is used to disable unwanted -rules, and also to configure optional parameters for enabled rules as needed. +rules, and also to configure optional parameters for enabled rules as needed. You can +also check [the issue](https://gitlab.com/gitlab-org/gitlab-foss/issues/64352) that +tracked the changes required to implement these rules, and details which rules were +on or off when `markdownlint` was enabled on the docs. ## Danger Bot diff --git a/doc/development/feature_flags/controls.md b/doc/development/feature_flags/controls.md index c8664831736..39f8f1bfea6 100644 --- a/doc/development/feature_flags/controls.md +++ b/doc/development/feature_flags/controls.md @@ -1,9 +1,11 @@ -# Access for enabling a feature flag in production +# Feature flag controls -In order to be able to turn on/off features behind feature flags in any of the +## Access + +To be able to turn on/off features behind feature flags in any of the GitLab Inc. provided environments such as staging and production, you need to -have access to the chatops bot. Chatops bot is currently running on the ops instance, -which is different from <https://gitlab.com> or <https://dev.gitlab.org>. +have access to the [Chatops](../chatops_on_gitlabcom.md) bot. The Chatops bot +is currently running on the ops instance, which is different from <https://gitlab.com> or <https://dev.gitlab.org>. Follow the Chatops document to [request access](../chatops_on_gitlabcom.md#requesting-access). @@ -14,6 +16,19 @@ run: /chatops run feature --help ``` +## Where to run commands + +To increase visibility, we recommend that GitLab team members run feature flag +related Chatops commands within certain slack channels based on the environment +and related feature. For the [staging](https://staging.gitlab.com) +and [development](https://dev.gitlab.org) environments of GitLab.com, +the commands should run in a channel for the stage the feature is relevant too. + +For example, use the `#s_monitor` channel for features developed by the +Monitor stage, Health group. + +For all production environment Chatops commands, use the `#production` channel. + ## Rolling out changes When the changes are deployed to the environments it is time to start @@ -28,7 +43,7 @@ easier to measure the impact of both separately. GitLab's feature library (using [Flipper](https://github.com/jnunemaker/flipper), and covered in the [Feature Flags process](process.md) guide) supports rolling out changes to a percentage of -users. This in turn can be controlled using [GitLab chatops](../../ci/chatops/README.md). +users. This in turn can be controlled using [GitLab Chatops](../../ci/chatops/README.md). For an up to date list of feature flag commands please see [the source code](https://gitlab.com/gitlab-com/chatops/blob/master/lib/chatops/commands/feature.rb). @@ -37,7 +52,7 @@ Note that all the examples in that file must be preceded by If you get an error "Whoops! This action is not allowed. This incident will be reported." that means your Slack account is not allowed to -change feature flags or you do not [have access](#access-for-enabling-a-feature-flag-in-production). +change feature flags or you do not [have access](#access). ### Enabling feature for internal testing @@ -64,7 +79,7 @@ there for any exceptions while testing your feature after enabling the feature f Once you are confident enough that these environments are in a good state with your feature enabled, you can roll out the change to GitLab.com. -## Enabling feature for GitLab.com +### Enabling a feature for GitLab.com Similar to above, to enable a feature for 25% of all users, run the following in Slack: @@ -114,10 +129,17 @@ merge request has to be picked into a stable branch, make sure to also add the appropriate "Pick into X" label (e.g. "Pick into XX.X"). See [the process document](process.md#including-a-feature-behind-feature-flag-in-the-final-release) for further details. -When a feature gate has been removed from the code base, the value still exists -in the database. -This can be removed through ChatOps: +When a feature gate has been removed from the code base, the feature +record still exists in the database that the flag was deployed too. +The record can be deleted once the MR is deployed to each environment: +```sh +/chatops run feature delete some_feature --dev +/chatops run feature delete some_feature --staging ``` + +Then, you can delete it from production after the MR is deployed to prod: + +```sh /chatops run feature delete some_feature ``` diff --git a/doc/user/packages/conan_repository/img/conan_package_view.png b/doc/user/packages/conan_repository/img/conan_package_view.png Binary files differnew file mode 100644 index 00000000000..79a188d7856 --- /dev/null +++ b/doc/user/packages/conan_repository/img/conan_package_view.png diff --git a/doc/user/packages/conan_repository/index.md b/doc/user/packages/conan_repository/index.md new file mode 100644 index 00000000000..f81756f7979 --- /dev/null +++ b/doc/user/packages/conan_repository/index.md @@ -0,0 +1,135 @@ +# GitLab Conan Repository **(PREMIUM)** + +> [Introduced](https://gitlab.com/gitlab-org/gitlab-ee/issues/8248) in [GitLab Premium](https://about.gitlab.com/pricing/) 12.4. + +With the GitLab Conan Repository, every +project can have its own space to store Conan packages. + + + +## Enabling the Conan Repository + +NOTE: **Note:** +This option is available only if your GitLab administrator has +[enabled support for the Conan Repository](../../../administration/packages/index.md).**(PREMIUM ONLY)** + +After the Conan Repository is enabled, it will be available for all new projects +by default. To enable it for existing projects, or if you want to disable it: + +1. Navigate to your project's **Settings > General > Permissions**. +1. Find the Packages feature and enable or disable it. +1. Click on **Save changes** for the changes to take effect. + +You should then be able to see the **Packages** section on the left sidebar. + +Before proceeding to authenticating with the GitLab Conan Repository, you should +get familiar with the package naming convention. + +## Authenticating to the GitLab Conan Repository + +You will need to generate a [personal access token](../../../user/profile/personal_access_tokens.md) for repository authentication. + +Now you can run conan commands using your token. + +`CONAN_LOGIN_USERNAME=<gitlab-username> CONAN_PASSWORD=<personal_access_token> conan upload Hello/0.2@user/channel --remote=gitlab` +`CONAN_LOGIN_USERNAME=<gitlab-username> CONAN_PASSWORD=<personal_access_token> conan search Hello* --all --remote=gitlab` + +Alternatively, you can set the `CONAN_LOGIN_USERNAME` and `CONAN_PASSWORD` in your local conan config to be used when connecting to the `gitlab` remote. The examples here show the username and password inline. + +Next, you'll need to set your Conan remote to point to the GitLab Package Registry. + +## Setting the Conan remote to the GitLab Package Registry + +After you authenticate to the [GitLab Conan Repository](#authenticating-to-the-gitlab-conan-repository), +you can set the Conan remote: + +```sh +conan remote add gitlab https://gitlab.example.com/api/v4/packages/conan +``` + +Once the remote is set, you can use the remote when running Conan commands: + +```sh +conan search Hello* --all --remote=gitlab +``` + +## Supported CLI commands + +The GitLab Conan repository supports the following Conan CLI commands: + +- `conan upload`: Upload your recipe and package files to the GitLab Package Registry. +- `conan install`: Install a conan package from the GitLab Package Registry, this includes using the `conan.txt` file. +- `conan search`: Search the GitLab Package Registry for public packages, and private packages you have permission to view. +- `conan info`: View the info on a given package from the GitLab Package Registry. +- `conan remove`: Delete the package from the GitLab Package Registry. + +## Uploading a package + +First you need to [create your Conan package locally](https://docs.conan.io/en/latest/creating_packages/getting_started.html). In order to work with the GitLab Package Registry, a specific [naming convention](#package-recipe-naming-convention) must be followed. + +Ensure you have a project created on GitLab and that the personal access token you are using has the correct permissions for write access to the container registry by selecting the `api` [scope](../../../user/profile/personal_access_tokens.md#limiting-scopes-of-a-personal-access-token). + +You can upload your package to the GitLab Package Registry using the `conan upload` command: + +```sh +CONAN_LOGIN_USERNAME=<gitlab-username> CONAN_PASSWORD=<personal_access_token> conan upload Hello/0.1@my-group+my-project/beta --all --remote=gitlab +``` + +### Package recipe naming convention + +Standard Conan recipe convention looks like `package_name/version@username/channel`. + +**Recipe usernames must be the `+` separated project path**. The package +name may be anything, but it is preferred that the project name be used unless +it is not possible due to a naming collision. For example: + +| Project | Package | Supported | +| ---------------------------------- | ----------------------------------------------- | --------- | +| `foo/bar` | `my-package/1.0.0@foo+bar/stable` | Yes | +| `foo/bar-baz/buz` | `my-package/1.0.0@foo+bar-baz+buz/stable` | Yes | +| `gitlab-org/gitlab-ce` | `my-package/1.0.0@gitlab-org+gitlab-ce/stable` | Yes | +| `gitlab-org/gitlab-ce` | `my-package/1.0.0@foo/stable` | No | + +NOTE: **Note:** +A future iteration will extend support to [project and group level](https://gitlab.com/gitlab-org/gitlab/issues/11679) remotes which will allow for more flexible naming conventions. + +## Installing a package + +Add the conan package to the `[requires]` section of your `conan.txt` file and they will be installed when you run `conan install` within your project. + +## Removing a package + +There are two ways to remove a Conan package from the GitLab Package Registry. + +- **Using the Conan client in the command line:** + + ```sh + CONAN_LOGIN_USERNAME=<gitlab-username> CONAN_PASSWORD=<personal_access_token> conan remove Hello/0.2@user/channel -r gitlab + ``` + + NOTE: **Note:** + This command will remove all recipe and binary package files from the Package Registry. + +- **GitLab project interface**: in the packages view of your project page, you can delete packages by clicking the red trash icons. + +## Searching the GitLab Package Registry for Conan packages + +The `conan search` command can be run searching by full or partial package name, or by exact recipe. + +To search using a partial name, use the wildcard symbol `*`, which should be placed at the end of your search (e.g., `my-packa*`): + +```sh +CONAN_LOGIN_USERNAME=<gitlab-username> CONAN_PASSWORD=<personal_access_token> conan search Hello --all --remote=gitlab +CONAN_LOGIN_USERNAME=<gitlab-username> CONAN_PASSWORD=<personal_access_token> conan search He* --all --remote=gitlab +CONAN_LOGIN_USERNAME=<gitlab-username> CONAN_PASSWORD=<personal_access_token> conan search Hello/1.0.0@my-group+my-project/stable --all --remote=gitlab +``` + +The scope of your search will include all projects you have permission to access, this includes your private projects as well as all public projects. + +## Fetching Conan package info from the GitLab Package Registry + +The `conan info` command will return info about a given package: + +```sh +CONAN_LOGIN_USERNAME=<gitlab-username> CONAN_PASSWORD=<personal_access_token> conan info Hello/1.0.0@my-group+my-project/stable -r gitlab +``` diff --git a/doc/user/packages/index.md b/doc/user/packages/index.md index 506eb5703a6..9873bd80e8b 100644 --- a/doc/user/packages/index.md +++ b/doc/user/packages/index.md @@ -10,6 +10,7 @@ The Packages feature allows GitLab to act as a repository for the following: | ------------------- | ----------- | --------------------------- | | [Container Registry](container_registry/index.md) | The GitLab Container Registry enables every project in GitLab to have its own space to store [Docker](https://www.docker.com/) images. | 8.8+ | | [Dependency Proxy](dependency_proxy/index.md) **(PREMIUM)** | The GitLab Dependency Proxy sets up a local proxy for frequently used upstream images/packages. | 11.11+ | +| [Conan Repository](conan_repository/index.md) **(PREMIUM)** | The GitLab Conan Repository enables every project in GitLab to have its own space to store [Conan](https://conan.io/) packages. | 12.4+ | | [Maven Repository](maven_repository/index.md) **(PREMIUM)** | The GitLab Maven Repository enables every project in GitLab to have its own space to store [Maven](https://maven.apache.org/) packages. | 11.3+ | | [NPM Registry](npm_registry/index.md) **(PREMIUM)** | The GitLab NPM Registry enables every project in GitLab to have its own space to store [NPM](https://www.npmjs.com/) packages. | 11.7+ | diff --git a/doc/user/permissions.md b/doc/user/permissions.md index 85930365708..90874eca2eb 100644 --- a/doc/user/permissions.md +++ b/doc/user/permissions.md @@ -76,8 +76,8 @@ The following table depicts the various user permission levels in a project. | See a list of merge requests | | ✓ | ✓ | ✓ | ✓ | | View project statistics | | ✓ | ✓ | ✓ | ✓ | | View Error Tracking list | | ✓ | ✓ | ✓ | ✓ | -| Pull from [Maven repository](packages/maven_repository/index.md) or [NPM registry](packages/npm_registry/index.md) **(PREMIUM)** | | ✓ | ✓ | ✓ | ✓ | -| Publish to [Maven repository](packages/maven_repository/index.md) or [NPM registry](packages/npm_registry/index.md) **(PREMIUM)** | | | ✓ | ✓ | ✓ | +| Pull from [Conan repository](packages/conan_repository/index.md), [Maven repository](packages/maven_repository/index.md), or [NPM registry](packages/npm_registry/index.md) **(PREMIUM)** | | ✓ | ✓ | ✓ | ✓ | +| Publish to [Conan repository](packages/conan_repository/index.md), [Maven repository](packages/maven_repository/index.md), or [NPM registry](packages/npm_registry/index.md) **(PREMIUM)** | | | ✓ | ✓ | ✓ | | Upload [Design Management](project/issues/design_management.md) files **(PREMIUM)** | | | ✓ | ✓ | ✓ | | Create new branches | | | ✓ | ✓ | ✓ | | Push to non-protected branches | | | ✓ | ✓ | ✓ | diff --git a/doc/user/project/img/issue_boards_multi_select.png b/doc/user/project/img/issue_boards_multi_select.png Binary files differdeleted file mode 100644 index 34ec0c1c58e..00000000000 --- a/doc/user/project/img/issue_boards_multi_select.png +++ /dev/null diff --git a/doc/user/project/index.md b/doc/user/project/index.md index fff587027cf..ef80c8fc6a3 100644 --- a/doc/user/project/index.md +++ b/doc/user/project/index.md @@ -95,6 +95,7 @@ When you create a project in GitLab, you'll have access to a large number of - [Releases](releases/index.md): a way to track deliverables in your project as snapshot in time of the source, build output, and other metadata or artifacts associated with a released version of your code. +- [Conan packages](../packages/conan_repository/index.md): your private Conan repository in GitLab. **(PREMIUM)** - [Maven packages](../packages/maven_repository/index.md): your private Maven repository in GitLab. **(PREMIUM)** - [NPM packages](../packages/npm_registry/index.md): your private NPM package registry in GitLab. **(PREMIUM)** - [Code owners](code_owners.md): specify code owners for certain files **(STARTER)** diff --git a/doc/user/project/issue_board.md b/doc/user/project/issue_board.md index e9a7a15b630..0c0068ddd09 100644 --- a/doc/user/project/issue_board.md +++ b/doc/user/project/issue_board.md @@ -180,18 +180,6 @@ These are shortcuts to your last 4 visited boards. When you're revisiting an issue board in a project or group with multiple boards, GitLab will automatically load the last board you visited. -### Multi-select Issue Cards - -As the name suggest, multi-select issue cards allows more than one issue card -to be dragged and dropped across different lists. This becomes helpful while -moving and grooming a lot of issues at once. - -You can multi-select an issue card by pressing `CTRL` + `Left mouse click` on -Windows or `CMD` + `Left mouse click` on MacOS. Once done, start by dragging one -of the issue card you have selected and drop it in the new list you want. - - - ### Configurable Issue Boards **(STARTER)** > Introduced in [GitLab Starter Edition 10.2](https://about.gitlab.com/2017/11/22/gitlab-10-2-released/#issue-boards-configuration). diff --git a/doc/workflow/time_tracking.md b/doc/workflow/time_tracking.md index 8c57c43b7d6..3d2e1de24da 100644 --- a/doc/workflow/time_tracking.md +++ b/doc/workflow/time_tracking.md @@ -11,7 +11,7 @@ requests within GitLab. ## Overview -Time Tracking allows you: +Time Tracking allows you to: - Record the time spent working on an issue or a merge request. - Add an estimate of the amount of time needed to complete an issue or a merge diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 6716fc6fcd2..c3c1b551b50 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -14842,9 +14842,6 @@ msgid_plural "Showing %d events" msgstr[0] "" msgstr[1] "" -msgid "Showing %{pageSize} of %{total} issues" -msgstr "" - msgid "Showing Latest Version" msgstr "" @@ -15100,12 +15097,6 @@ msgstr "" msgid "Something went wrong while merging this merge request. Please try again." msgstr "" -msgid "Something went wrong while moving issues." -msgstr "" - -msgid "Something went wrong while performing the action." -msgstr "" - msgid "Something went wrong while reopening the %{issuable}. Please try again later" msgstr "" diff --git a/spec/features/boards/multi_select_spec.rb b/spec/features/boards/multi_select_spec.rb deleted file mode 100644 index ff3cb0271bd..00000000000 --- a/spec/features/boards/multi_select_spec.rb +++ /dev/null @@ -1,129 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -describe 'Multi Select Issue', :js do - include DragTo - - let(:group) { create(:group, :nested) } - let(:project) { create(:project, :public, namespace: group) } - let(:board) { create(:board, project: project) } - let(:user) { create(:user) } - - def drag(selector: '.board-list', list_from_index: 1, from_index: 0, to_index: 0, list_to_index: 1, duration: 1000) - drag_to(selector: selector, - scrollable: '#board-app', - list_from_index: list_from_index, - from_index: from_index, - to_index: to_index, - list_to_index: list_to_index, - duration: duration) - end - - def wait_for_board_cards(board_number, expected_cards) - page.within(find(".board:nth-child(#{board_number})")) do - expect(page.find('.board-header')).to have_content(expected_cards.to_s) - expect(page).to have_selector('.board-card', count: expected_cards) - end - end - - def multi_select(selector, action = 'select') - element = page.find(selector) - script = "var el = document.querySelector('#{selector}');" - script += "var mousedown = new MouseEvent('mousedown', { button: 0, bubbles: true });" - script += "var mouseup = new MouseEvent('mouseup', { ctrlKey: true, button: 0, bubbles:true });" - script += "el.dispatchEvent(mousedown); el.dispatchEvent(mouseup);" - script += "Sortable.utils.#{action}(el);" - - page.execute_script(script, element) - end - - before do - project.add_maintainer(user) - - sign_in(user) - end - - context 'with lists' do - let(:label1) { create(:label, project: project, name: 'Label 1', description: 'Test') } - let(:label2) { create(:label, project: project, name: 'Label 2', description: 'Test') } - let!(:list1) { create(:list, board: board, label: label1, position: 0) } - let!(:list2) { create(:list, board: board, label: label2, position: 1) } - let!(:issue1) { create(:labeled_issue, project: project, title: 'Issue 1', description: '', assignees: [user], labels: [label1], relative_position: 1) } - let!(:issue2) { create(:labeled_issue, project: project, title: 'Issue 2', description: '', author: user, labels: [label1], relative_position: 2) } - let!(:issue3) { create(:labeled_issue, project: project, title: 'Issue 3', description: '', labels: [label1], relative_position: 3) } - let!(:issue4) { create(:labeled_issue, project: project, title: 'Issue 4', description: '', labels: [label1], relative_position: 4) } - let!(:issue5) { create(:labeled_issue, project: project, title: 'Issue 5', description: '', labels: [label1], relative_position: 5) } - - before do - visit project_board_path(project, board) - - wait_for_requests - end - - it 'moves multiple issues to another list', :js do - multi_select('.board-card:nth-child(1)') - multi_select('.board-card:nth-child(2)') - drag(list_from_index: 1, list_to_index: 2) - - wait_for_requests - - page.within(all('.js-board-list')[2]) do - expect(find('.board-card:nth-child(1)')).to have_content(issue1.title) - expect(find('.board-card:nth-child(2)')).to have_content(issue2.title) - end - end - - it 'maintains order when moved', :js do - multi_select('.board-card:nth-child(3)') - multi_select('.board-card:nth-child(2)') - multi_select('.board-card:nth-child(1)') - - drag(list_from_index: 1, list_to_index: 2) - - wait_for_requests - - page.within(all('.js-board-list')[2]) do - expect(find('.board-card:nth-child(1)')).to have_content(issue1.title) - expect(find('.board-card:nth-child(2)')).to have_content(issue2.title) - expect(find('.board-card:nth-child(3)')).to have_content(issue3.title) - end - end - - it 'move issues in the same list', :js do - multi_select('.board-card:nth-child(3)') - multi_select('.board-card:nth-child(4)') - - drag(list_from_index: 1, list_to_index: 1, from_index: 2, to_index: 4) - - wait_for_requests - - page.within(all('.js-board-list')[1]) do - expect(find('.board-card:nth-child(1)')).to have_content(issue1.title) - expect(find('.board-card:nth-child(2)')).to have_content(issue2.title) - expect(find('.board-card:nth-child(3)')).to have_content(issue5.title) - expect(find('.board-card:nth-child(4)')).to have_content(issue3.title) - expect(find('.board-card:nth-child(5)')).to have_content(issue4.title) - end - end - - it 'adds label when issues are moved to different card', :js do - page.within(all('.js-board-list')[1]) do - expect(find('.board-card:nth-child(1)')).not_to have_content(label2.title) - expect(find('.board-card:nth-child(2)')).not_to have_content(label2.title) - end - - multi_select('.board-card:nth-child(1)') - multi_select('.board-card:nth-child(2)') - - drag(list_from_index: 1, list_to_index: 2) - - wait_for_requests - - page.within(all('.js-board-list')[2]) do - expect(find('.board-card:nth-child(1)')).to have_content(label2.title) - expect(find('.board-card:nth-child(2)')).to have_content(label2.title) - end - end - end -end diff --git a/spec/javascripts/boards/board_card_spec.js b/spec/javascripts/boards/board_card_spec.js index 9f441ca319e..13b708a03d5 100644 --- a/spec/javascripts/boards/board_card_spec.js +++ b/spec/javascripts/boards/board_card_spec.js @@ -67,16 +67,6 @@ describe('Board card', () => { expect(vm.issueDetailVisible).toBe(true); }); - it("returns false when multiSelect doesn't contain issue", () => { - expect(vm.multiSelectVisible).toBe(false); - }); - - it('returns true when multiSelect contains issue', () => { - boardsStore.multiSelect.list = [vm.issue]; - - expect(vm.multiSelectVisible).toBe(true); - }); - it('adds user-can-drag class if not disabled', () => { expect(vm.$el.classList.contains('user-can-drag')).toBe(true); }); @@ -190,7 +180,7 @@ describe('Board card', () => { triggerEvent('mousedown'); triggerEvent('mouseup'); - expect(eventHub.$emit).toHaveBeenCalledWith('newDetailIssue', vm.issue, undefined); + expect(eventHub.$emit).toHaveBeenCalledWith('newDetailIssue', vm.issue); expect(boardsStore.detail.list).toEqual(vm.list); }); @@ -213,7 +203,7 @@ describe('Board card', () => { triggerEvent('mousedown'); triggerEvent('mouseup'); - expect(eventHub.$emit).toHaveBeenCalledWith('clearDetailIssue', undefined); + expect(eventHub.$emit).toHaveBeenCalledWith('clearDetailIssue'); }); }); }); diff --git a/spec/javascripts/boards/boards_store_spec.js b/spec/javascripts/boards/boards_store_spec.js index 678fe5befa8..11352140ba4 100644 --- a/spec/javascripts/boards/boards_store_spec.js +++ b/spec/javascripts/boards/boards_store_spec.js @@ -12,7 +12,6 @@ import '~/boards/services/board_service'; import boardsStore from '~/boards/stores/boards_store'; import eventHub from '~/boards/eventhub'; import { listObj, listObjDuplicate, boardsMockInterceptor, mockBoardService } from './mock_data'; -import waitForPromises from '../../frontend/helpers/wait_for_promises'; describe('Store', () => { let mock; @@ -30,13 +29,6 @@ describe('Store', () => { }), ); - spyOn(gl.boardService, 'moveMultipleIssues').and.callFake( - () => - new Promise(resolve => { - resolve(); - }), - ); - Cookies.set('issue_board_welcome_hidden', 'false', { expires: 365 * 10, path: '', @@ -384,128 +376,4 @@ describe('Store', () => { expect(state.currentBoard).toEqual(dummyBoard); }); }); - - describe('toggleMultiSelect', () => { - let basicIssueObj; - - beforeAll(() => { - basicIssueObj = { id: 987654 }; - }); - - afterEach(() => { - boardsStore.clearMultiSelect(); - }); - - it('adds issue when not present', () => { - boardsStore.toggleMultiSelect(basicIssueObj); - - const selectedIds = boardsStore.multiSelect.list.map(x => x.id); - - expect(selectedIds.includes(basicIssueObj.id)).toEqual(true); - }); - - it('removes issue when issue is present', () => { - boardsStore.toggleMultiSelect(basicIssueObj); - let selectedIds = boardsStore.multiSelect.list.map(x => x.id); - - expect(selectedIds.includes(basicIssueObj.id)).toEqual(true); - - boardsStore.toggleMultiSelect(basicIssueObj); - selectedIds = boardsStore.multiSelect.list.map(x => x.id); - - expect(selectedIds.includes(basicIssueObj.id)).toEqual(false); - }); - }); - - describe('clearMultiSelect', () => { - it('clears all the multi selected issues', () => { - const issue1 = { id: 12345 }; - const issue2 = { id: 12346 }; - - boardsStore.toggleMultiSelect(issue1); - boardsStore.toggleMultiSelect(issue2); - - expect(boardsStore.multiSelect.list.length).toEqual(2); - - boardsStore.clearMultiSelect(); - - expect(boardsStore.multiSelect.list.length).toEqual(0); - }); - }); - - describe('moveMultipleIssuesToList', () => { - it('move issues on the new index', done => { - const listOne = boardsStore.addList(listObj); - const listTwo = boardsStore.addList(listObjDuplicate); - - expect(boardsStore.state.lists.length).toBe(2); - - setTimeout(() => { - expect(listOne.issues.length).toBe(1); - expect(listTwo.issues.length).toBe(1); - - boardsStore.moveMultipleIssuesToList({ - listFrom: listOne, - listTo: listTwo, - issues: listOne.issues, - newIndex: 0, - }); - - expect(listTwo.issues.length).toBe(1); - - done(); - }, 0); - }); - }); - - describe('moveMultipleIssuesInList', () => { - it('moves multiple issues in list', done => { - const issueObj = { - title: 'Issue #1', - id: 12345, - iid: 2, - confidential: false, - labels: [], - assignees: [], - }; - const issue1 = new ListIssue(issueObj); - const issue2 = new ListIssue({ - ...issueObj, - title: 'Issue #2', - id: 12346, - }); - - const list = boardsStore.addList(listObj); - - waitForPromises() - .then(() => { - list.addIssue(issue1); - list.addIssue(issue2); - - expect(list.issues.length).toBe(3); - expect(list.issues[0].id).not.toBe(issue2.id); - - boardsStore.moveMultipleIssuesInList({ - list, - issues: [issue1, issue2], - oldIndicies: [0], - newIndex: 1, - idArray: [1, 12345, 12346], - }); - - expect(list.issues[0].id).toBe(issue1.id); - - expect(gl.boardService.moveMultipleIssues).toHaveBeenCalledWith({ - ids: [issue1.id, issue2.id], - fromListId: null, - toListId: null, - moveBeforeId: 1, - moveAfterId: null, - }); - - done(); - }) - .catch(done.fail); - }); - }); }); diff --git a/spec/models/concerns/atomic_internal_id_spec.rb b/spec/models/concerns/atomic_internal_id_spec.rb index f9ca9660bdb..80f296d8825 100644 --- a/spec/models/concerns/atomic_internal_id_spec.rb +++ b/spec/models/concerns/atomic_internal_id_spec.rb @@ -22,7 +22,7 @@ describe AtomicInternalId do end context 'when value is set by ensure_project_iid!' do - context 'with iid_always_track true' do + context 'with iid_always_track false' do before do stub_feature_flags(iid_always_track: false) end @@ -33,6 +33,17 @@ describe AtomicInternalId do milestone.ensure_project_iid! subject end + + it 'tracks the iid for the scope that is actually present' do + milestone.iid = external_iid + + expect(InternalId).to receive(:track_greatest).once.with(milestone, scope_attrs, usage, external_iid, anything) + expect(InternalId).not_to receive(:generate_next) + + # group scope is not present here, the milestone does not have a group + milestone.track_group_iid! + subject + end end context 'with iid_always_track enabled' do diff --git a/spec/services/git/process_ref_changes_service_spec.rb b/spec/services/git/process_ref_changes_service_spec.rb new file mode 100644 index 00000000000..4d394a29867 --- /dev/null +++ b/spec/services/git/process_ref_changes_service_spec.rb @@ -0,0 +1,105 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Git::ProcessRefChangesService do + let(:project) { create(:project, :repository) } + let(:user) { project.owner } + let(:params) { { changes: git_changes } } + + subject { described_class.new(project, user, params) } + + shared_examples_for 'service for processing ref changes' do |push_service_class| + let(:service) { double(execute: true) } + let(:git_changes) { double(branch_changes: [], tag_changes: []) } + + let(:changes) do + [ + { index: 0, oldrev: Gitlab::Git::BLANK_SHA, newrev: '789012', ref: "#{ref_prefix}/create" }, + { index: 1, oldrev: '123456', newrev: '789012', ref: "#{ref_prefix}/update" }, + { index: 2, oldrev: '123456', newrev: Gitlab::Git::BLANK_SHA, ref: "#{ref_prefix}/delete" } + ] + end + + before do + allow(git_changes).to receive(changes_method).and_return(changes) + end + + it "calls #{push_service_class}" do + expect(push_service_class) + .to receive(:new) + .exactly(changes.count).times + .and_return(service) + + subject.execute + end + + context 'pipeline creation' do + context 'with valid .gitlab-ci.yml' do + before do + stub_ci_pipeline_to_return_yaml_file + + allow_any_instance_of(Project) + .to receive(:commit) + .and_return(project.commit) + + allow_any_instance_of(Repository) + .to receive(:branch_exists?) + .and_return(true) + end + + context 'when git_push_create_all_pipelines is disabled' do + before do + stub_feature_flags(git_push_create_all_pipelines: false) + end + + it 'creates pipeline for branches and tags' do + subject.execute + + expect(Ci::Pipeline.pluck(:ref)).to contain_exactly('create', 'update', 'delete') + end + + it "creates exactly #{described_class::PIPELINE_PROCESS_LIMIT} pipelines" do + stub_const("#{described_class}::PIPELINE_PROCESS_LIMIT", changes.count - 1) + + expect { subject.execute }.to change { Ci::Pipeline.count }.by(described_class::PIPELINE_PROCESS_LIMIT) + end + end + + context 'when git_push_create_all_pipelines is enabled' do + before do + stub_feature_flags(git_push_create_all_pipelines: true) + end + + it 'creates all pipelines' do + expect { subject.execute }.to change { Ci::Pipeline.count }.by(changes.count) + end + end + end + + context 'with invalid .gitlab-ci.yml' do + before do + stub_ci_pipeline_yaml_file(nil) + end + + it 'does not create a pipeline' do + expect { subject.execute }.not_to change { Ci::Pipeline.count } + end + end + end + end + + context 'branch changes' do + let(:changes_method) { :branch_changes } + let(:ref_prefix) { 'refs/heads' } + + it_behaves_like 'service for processing ref changes', Git::BranchPushService + end + + context 'tag changes' do + let(:changes_method) { :tag_changes } + let(:ref_prefix) { 'refs/tags' } + + it_behaves_like 'service for processing ref changes', Git::TagPushService + end +end diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index 55254b61ac8..154bfec0da2 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -6,7 +6,8 @@ describe Issues::UpdateService, :mailer do let(:user) { create(:user) } let(:user2) { create(:user) } let(:user3) { create(:user) } - let(:project) { create(:project) } + let(:group) { create(:group, :public) } + let(:project) { create(:project, :repository, group: group) } let(:label) { create(:label, project: project) } let(:label2) { create(:label) } @@ -667,6 +668,7 @@ describe Issues::UpdateService, :mailer do context 'updating mentions' do let(:mentionable) { issue } + include_examples 'updating mentions', described_class end diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index 9688e02d6ac..d3c4c436901 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -5,7 +5,8 @@ require 'spec_helper' describe MergeRequests::UpdateService, :mailer do include ProjectForksHelper - let(:project) { create(:project, :repository) } + let(:group) { create(:group, :public) } + let(:project) { create(:project, :repository, group: group) } let(:user) { create(:user) } let(:user2) { create(:user) } let(:user3) { create(:user) } @@ -472,6 +473,7 @@ describe MergeRequests::UpdateService, :mailer do context 'updating mentions' do let(:mentionable) { merge_request } + include_examples 'updating mentions', described_class end diff --git a/spec/services/notes/update_service_spec.rb b/spec/services/notes/update_service_spec.rb index 717eb97fa5a..73fcdd787aa 100644 --- a/spec/services/notes/update_service_spec.rb +++ b/spec/services/notes/update_service_spec.rb @@ -3,17 +3,25 @@ require 'spec_helper' describe Notes::UpdateService do - let(:project) { create(:project) } + let(:group) { create(:group, :public) } + let(:project) { create(:project, :public, group: group) } + let(:private_group) { create(:group, :private) } + let(:private_project) { create(:project, :private, group: private_group) } let(:user) { create(:user) } let(:user2) { create(:user) } let(:user3) { create(:user) } let(:issue) { create(:issue, project: project) } + let(:issue2) { create(:issue, project: private_project) } let(:note) { create(:note, project: project, noteable: issue, author: user, note: "Old note #{user2.to_reference}") } before do project.add_maintainer(user) project.add_developer(user2) project.add_developer(user3) + group.add_developer(user3) + private_group.add_developer(user) + private_group.add_developer(user2) + private_project.add_developer(user3) end describe '#execute' do @@ -46,13 +54,17 @@ describe Notes::UpdateService do end context 'todos' do - let!(:todo) { create(:todo, :assigned, user: user, project: project, target: issue, author: user2) } + shared_examples 'does not update todos' do + it 'keep todos' do + expect(todo.reload).to be_pending + end - context 'when the note change' do - before do - update_note({ note: "New note #{user2.to_reference} #{user3.to_reference}" }) + it 'does not create any new todos' do + expect(Todo.count).to eq(1) end + end + shared_examples 'creates one todo' do it 'marks todos as done' do expect(todo.reload).to be_done end @@ -62,17 +74,75 @@ describe Notes::UpdateService do end end - context 'when the note does not change' do - before do - update_note({ note: "Old note #{user2.to_reference}" }) + context 'when note includes a user mention' do + let!(:todo) { create(:todo, :assigned, user: user, project: project, target: issue, author: user2) } + + context 'when the note does not change mentions' do + before do + update_note({ note: "Old note #{user2.to_reference}" }) + end + + it_behaves_like 'does not update todos' end - it 'keep todos' do - expect(todo.reload).to be_pending + context 'when the note changes to include one more user mention' do + before do + update_note({ note: "New note #{user2.to_reference} #{user3.to_reference}" }) + end + + it_behaves_like 'creates one todo' end - it 'does not create any new todos' do - expect(Todo.count).to eq(1) + context 'when the note changes to include a group mentions' do + before do + update_note({ note: "New note #{private_group.to_reference}" }) + end + + it_behaves_like 'creates one todo' + end + end + + context 'when note includes a group mention' do + context 'when the group is public' do + let(:note) { create(:note, project: project, noteable: issue, author: user, note: "Old note #{group.to_reference}") } + let!(:todo) { create(:todo, :assigned, user: user, project: project, target: issue, author: user2) } + + context 'when the note does not change mentions' do + before do + update_note({ note: "Old note #{group.to_reference}" }) + end + + it_behaves_like 'does not update todos' + end + + context 'when the note changes mentions' do + before do + update_note({ note: "New note #{user2.to_reference} #{user3.to_reference}" }) + end + + it_behaves_like 'creates one todo' + end + end + + context 'when the group is private' do + let(:note) { create(:note, project: project, noteable: issue, author: user, note: "Old note #{private_group.to_reference}") } + let!(:todo) { create(:todo, :assigned, user: user, project: project, target: issue, author: user2) } + + context 'when the note does not change mentions' do + before do + update_note({ note: "Old note #{private_group.to_reference}" }) + end + + it_behaves_like 'does not update todos' + end + + context 'when the note changes mentions' do + before do + update_note({ note: "New note #{user2.to_reference} #{user3.to_reference}" }) + end + + it_behaves_like 'creates one todo' + end end end end diff --git a/spec/support/shared_examples/models/atomic_internal_id_shared_examples.rb b/spec/support/shared_examples/models/atomic_internal_id_shared_examples.rb index b837ca87256..44f66ff47f4 100644 --- a/spec/support/shared_examples/models/atomic_internal_id_shared_examples.rb +++ b/spec/support/shared_examples/models/atomic_internal_id_shared_examples.rb @@ -47,6 +47,10 @@ shared_examples_for 'AtomicInternalId' do |validate_presence: true| end describe 'internal id generation' do + before do + stub_feature_flags(iid_always_track: false) + end + subject { instance.save! } it 'calls InternalId.generate_next and sets internal id attribute' do diff --git a/spec/support/shared_examples/updating_mentions_shared_examples.rb b/spec/support/shared_examples/updating_mentions_shared_examples.rb index ef385f94cc2..9a8f8012762 100644 --- a/spec/support/shared_examples/updating_mentions_shared_examples.rb +++ b/spec/support/shared_examples/updating_mentions_shared_examples.rb @@ -1,11 +1,16 @@ # frozen_string_literal: true RSpec.shared_examples 'updating mentions' do |service_class| - let(:mentioned_user) { create(:user) } - let(:service_class) { service_class } + let(:service_class) { service_class } + let(:mentioned_user) { create(:user) } + let(:group_member1) { create(:user) } + let(:group_member2) { create(:user) } + let(:external_group) { create(:group, :private) } before do project.add_developer(mentioned_user) + group.add_developer(group_member1) + group.add_developer(group_member2) end def update_mentionable(opts) @@ -16,23 +21,74 @@ RSpec.shared_examples 'updating mentions' do |service_class| mentionable.reload end - context 'in title' do - before do - update_mentionable(title: mentioned_user.to_reference) + context 'when mentioning a different user' do + context 'in title' do + before do + update_mentionable(title: "For #{mentioned_user.to_reference}") + end + + it 'emails only the newly-mentioned user' do + should_only_email(mentioned_user) + end + end + + context 'in description' do + before do + update_mentionable(description: "For #{mentioned_user.to_reference}") + end + + it 'emails only the newly-mentioned user' do + should_only_email(mentioned_user) + end + end + end + + context 'when mentioning a user and a group with access to' do + shared_examples 'updating attribute with allowed mentions' do |attribute| + before do + update_mentionable( + { attribute => "For #{group.to_reference}, cc: #{mentioned_user.to_reference}" } + ) + end + + it 'emails group members' do + should_email(mentioned_user) + should_email(group_member1) + should_email(group_member2) + end + end + + context 'when group is public' do + it_behaves_like 'updating attribute with allowed mentions', :title + it_behaves_like 'updating attribute with allowed mentions', :description end - it 'emails only the newly-mentioned user' do - should_only_email(mentioned_user) + context 'when the group is private' do + before do + group.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) + end + + it_behaves_like 'updating attribute with allowed mentions', :title + it_behaves_like 'updating attribute with allowed mentions', :description end end - context 'in description' do - before do - update_mentionable(description: mentioned_user.to_reference) + context 'when mentioning a user and a group without access to' do + shared_examples 'updating attribute with not allowed mentions' do |attribute| + before do + update_mentionable( + { attribute => "For #{external_group.to_reference}, cc: #{mentioned_user.to_reference}" } + ) + end + + it 'emails mentioned user' do + should_only_email(mentioned_user) + end end - it 'emails only the newly-mentioned user' do - should_only_email(mentioned_user) + context 'when the group is private' do + it_behaves_like 'updating attribute with not allowed mentions', :title + it_behaves_like 'updating attribute with not allowed mentions', :description end end end diff --git a/spec/workers/post_receive_spec.rb b/spec/workers/post_receive_spec.rb index d034e962cee..6983fea021c 100644 --- a/spec/workers/post_receive_spec.rb +++ b/spec/workers/post_receive_spec.rb @@ -73,8 +73,7 @@ describe PostReceive do context 'empty changes' do it "does not call any PushService but runs after project hooks" do - expect(Git::BranchPushService).not_to receive(:new) - expect(Git::TagPushService).not_to receive(:new) + expect(Git::ProcessRefChangesService).not_to receive(:new) expect_next_instance_of(SystemHooksService) { |service| expect(service).to receive(:execute_hooks) } perform(changes: "") @@ -87,8 +86,7 @@ describe PostReceive do let!(:key_id) { "" } it 'returns false' do - expect(Git::BranchPushService).not_to receive(:new) - expect(Git::TagPushService).not_to receive(:new) + expect(Git::ProcessRefChangesService).not_to receive(:new) expect(perform).to be false end @@ -131,13 +129,11 @@ describe PostReceive do perform end - it 'calls Git::BranchPushService' do - expect_any_instance_of(Git::BranchPushService) do |service| + it 'calls Git::ProcessRefChangesService' do + expect_next_instance_of(Git::ProcessRefChangesService) do |service| expect(service).to receive(:execute).and_return(true) end - expect(Git::TagPushService).not_to receive(:new) - perform end @@ -174,8 +170,6 @@ describe PostReceive do 654321 210987 refs/tags/tag1 654322 210986 refs/tags/tag2 654323 210985 refs/tags/tag3 - 654324 210984 refs/tags/tag4 - 654325 210983 refs/tags/tag5 EOF end @@ -189,23 +183,19 @@ describe PostReceive do perform end - it "only invalidates tags once" do - expect(project.repository).to receive(:repository_event).exactly(5).times.with(:push_tag).and_call_original + it 'only invalidates tags once' do + expect(project.repository).to receive(:repository_event).exactly(3).times.with(:push_tag).and_call_original expect(project.repository).to receive(:expire_caches_for_tags).once.and_call_original expect(project.repository).to receive(:expire_tags_cache).once.and_call_original perform end - it "calls Git::TagPushService" do - expect(Git::BranchPushService).not_to receive(:new) - - expect_any_instance_of(Git::TagPushService) do |service| + it 'calls Git::ProcessRefChangesService' do + expect_next_instance_of(Git::ProcessRefChangesService) do |service| expect(service).to receive(:execute).and_return(true) end - expect(Git::BranchPushService).not_to receive(:new) - perform end @@ -223,8 +213,7 @@ describe PostReceive do let(:changes) { "123456 789012 refs/merge-requests/123" } it "does not call any of the services" do - expect(Git::BranchPushService).not_to receive(:new) - expect(Git::TagPushService).not_to receive(:new) + expect(Git::ProcessRefChangesService).not_to receive(:new) perform end @@ -232,72 +221,6 @@ describe PostReceive do it_behaves_like 'not updating remote mirrors' end - context "gitlab-ci.yml" do - let(:changes) do - <<-EOF.strip_heredoc - 123456 789012 refs/heads/feature - 654321 210987 refs/tags/tag - 123456 789012 refs/heads/feature2 - 123458 789013 refs/heads/feature3 - 123459 789015 refs/heads/feature4 - EOF - end - - let(:changes_count) { changes.lines.count } - - subject { perform } - - context "with valid .gitlab-ci.yml" do - before do - stub_ci_pipeline_to_return_yaml_file - - allow_any_instance_of(Project) - .to receive(:commit) - .and_return(project.commit) - - allow_any_instance_of(Repository) - .to receive(:branch_exists?) - .and_return(true) - end - - context 'when git_push_create_all_pipelines is disabled' do - before do - stub_feature_flags(git_push_create_all_pipelines: false) - end - - it "creates pipeline for branches and tags" do - subject - - expect(Ci::Pipeline.pluck(:ref)).to contain_exactly("feature", "tag", "feature2", "feature3") - end - - it "creates exactly #{described_class::PIPELINE_PROCESS_LIMIT} pipelines" do - expect(changes_count).to be > described_class::PIPELINE_PROCESS_LIMIT - - expect { subject }.to change { Ci::Pipeline.count }.by(described_class::PIPELINE_PROCESS_LIMIT) - end - end - - context 'when git_push_create_all_pipelines is enabled' do - before do - stub_feature_flags(git_push_create_all_pipelines: true) - end - - it "creates all pipelines" do - expect { subject }.to change { Ci::Pipeline.count }.by(changes_count) - end - end - end - - context "does not create a Ci::Pipeline" do - before do - stub_ci_pipeline_yaml_file(nil) - end - - it { expect { subject }.not_to change { Ci::Pipeline.count } } - end - end - context 'after project changes hooks' do let(:changes) { '123456 789012 refs/heads/tést' } let(:fake_hook_data) { Hash.new(event_name: 'repository_update') } @@ -306,7 +229,7 @@ describe PostReceive do allow_any_instance_of(Gitlab::DataBuilder::Repository).to receive(:update).and_return(fake_hook_data) # silence hooks so we can isolate allow_any_instance_of(Key).to receive(:post_create_hook).and_return(true) - expect_next_instance_of(Git::BranchPushService) do |service| + expect_next_instance_of(Git::ProcessRefChangesService) do |service| expect(service).to receive(:execute).and_return(true) end end |
