diff options
104 files changed, 2868 insertions, 263 deletions
diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js index 0c420c12345..51cc8c085b2 100644 --- a/app/assets/javascripts/dispatcher.js +++ b/app/assets/javascripts/dispatcher.js @@ -160,9 +160,6 @@ import initSettingsPanels from './settings_panels'; case 'admin:projects:index': new ProjectsList(); break; - case 'dashboard:groups:index': - new GroupsList(); - break; case 'explore:groups:index': new GroupsList(); @@ -374,9 +371,11 @@ import initSettingsPanels from './settings_panels'; new ProjectFork(); break; case 'projects:artifacts:browse': + new ShortcutsNavigation(); new BuildArtifacts(); break; case 'projects:artifacts:file': + new ShortcutsNavigation(); new BlobViewer(); break; case 'help:index': diff --git a/app/assets/javascripts/filterable_list.js b/app/assets/javascripts/filterable_list.js index aaaeb9bddb1..139206cc185 100644 --- a/app/assets/javascripts/filterable_list.js +++ b/app/assets/javascripts/filterable_list.js @@ -8,39 +8,87 @@ export default class FilterableList { this.filterForm = form; this.listFilterElement = filter; this.listHolderElement = holder; + this.isBusy = false; + } + + getFilterEndpoint() { + return `${this.filterForm.getAttribute('action')}?${$(this.filterForm).serialize()}`; + } + + getPagePath() { + return this.getFilterEndpoint(); } initSearch() { - this.debounceFilter = _.debounce(this.filterResults.bind(this), 500); + // Wrap to prevent passing event arguments to .filterResults; + this.debounceFilter = _.debounce(this.onFilterInput.bind(this), 500); - this.listFilterElement.removeEventListener('input', this.debounceFilter); + this.unbindEvents(); + this.bindEvents(); + } + + onFilterInput() { + const $form = $(this.filterForm); + const queryData = {}; + const filterGroupsParam = $form.find('[name="filter_groups"]').val(); + + if (filterGroupsParam) { + queryData.filter_groups = filterGroupsParam; + } + + this.filterResults(queryData); + + if (this.setDefaultFilterOption) { + this.setDefaultFilterOption(); + } + } + + bindEvents() { this.listFilterElement.addEventListener('input', this.debounceFilter); } - filterResults() { - const form = this.filterForm; - const filterUrl = `${form.getAttribute('action')}?${$(form).serialize()}`; + unbindEvents() { + this.listFilterElement.removeEventListener('input', this.debounceFilter); + } + + filterResults(queryData) { + if (this.isBusy) { + return false; + } $(this.listHolderElement).fadeTo(250, 0.5); return $.ajax({ - url: form.getAttribute('action'), - data: $(form).serialize(), + url: this.getFilterEndpoint(), + data: queryData, type: 'GET', dataType: 'json', context: this, - complete() { - $(this.listHolderElement).fadeTo(250, 1); + complete: this.onFilterComplete, + beforeSend: () => { + this.isBusy = true; }, - success(data) { - this.listHolderElement.innerHTML = data.html; - - // Change url so if user reload a page - search results are saved - return window.history.replaceState({ - page: filterUrl, - - }, document.title, filterUrl); + success: (response, textStatus, xhr) => { + this.onFilterSuccess(response, xhr, queryData); }, }); } + + onFilterSuccess(response, xhr, queryData) { + if (response.html) { + this.listHolderElement.innerHTML = response.html; + } + + // Change url so if user reload a page - search results are saved + const currentPath = this.getPagePath(queryData); + + return window.history.replaceState({ + page: currentPath, + }, document.title, currentPath); + } + + onFilterComplete() { + this.isBusy = false; + $(this.listHolderElement).fadeTo(250, 1); + } } diff --git a/app/assets/javascripts/gl_dropdown.js b/app/assets/javascripts/gl_dropdown.js index d34561e5512..3babe273100 100644 --- a/app/assets/javascripts/gl_dropdown.js +++ b/app/assets/javascripts/gl_dropdown.js @@ -248,7 +248,7 @@ GitLabDropdown = (function() { return function(data) { _this.fullData = data; _this.parseData(_this.fullData); - _this.focusTextInput(); + _this.focusTextInput(true); if (_this.options.filterable && _this.filter && _this.filter.input && _this.filter.input.val() && _this.filter.input.val().trim() !== '') { return _this.filter.input.trigger('input'); } @@ -728,8 +728,20 @@ GitLabDropdown = (function() { return [selectedObject, isMarking]; }; - GitLabDropdown.prototype.focusTextInput = function() { - if (this.options.filterable) { this.filterInput.focus(); } + GitLabDropdown.prototype.focusTextInput = function(triggerFocus = false) { + if (this.options.filterable) { + $(':focus').blur(); + + this.dropdown.one('transitionend', () => { + this.filterInput.focus(); + }); + + if (triggerFocus) { + // This triggers after a ajax request + // in case of slow requests, the dropdown transition could already be finished + this.dropdown.trigger('transitionend'); + } + } }; GitLabDropdown.prototype.addInput = function(fieldName, value, selectedObject) { diff --git a/app/assets/javascripts/groups/components/group_folder.vue b/app/assets/javascripts/groups/components/group_folder.vue new file mode 100644 index 00000000000..7cc6c4b0359 --- /dev/null +++ b/app/assets/javascripts/groups/components/group_folder.vue @@ -0,0 +1,27 @@ +<script> +export default { + props: { + groups: { + type: Object, + required: true, + }, + baseGroup: { + type: Object, + required: false, + default: () => ({}), + }, + }, +}; +</script> + +<template> + <ul class="content-list group-list-tree"> + <group-item + v-for="(group, index) in groups" + :key="index" + :group="group" + :base-group="baseGroup" + :collection="groups" + /> + </ul> +</template> diff --git a/app/assets/javascripts/groups/components/group_item.vue b/app/assets/javascripts/groups/components/group_item.vue new file mode 100644 index 00000000000..32815b9f73e --- /dev/null +++ b/app/assets/javascripts/groups/components/group_item.vue @@ -0,0 +1,220 @@ +<script> +import eventHub from '../event_hub'; + +export default { + props: { + group: { + type: Object, + required: true, + }, + baseGroup: { + type: Object, + required: false, + default: () => ({}), + }, + collection: { + type: Object, + required: false, + default: () => ({}), + }, + }, + methods: { + onClickRowGroup(e) { + e.stopPropagation(); + + // Skip for buttons + if (!(e.target.tagName === 'A') && !(e.target.tagName === 'I' && e.target.parentElement.tagName === 'A')) { + if (this.group.hasSubgroups) { + eventHub.$emit('toggleSubGroups', this.group); + } else { + window.location.href = this.group.webUrl; + } + } + }, + onLeaveGroup(e) { + e.preventDefault(); + + // eslint-disable-next-line no-alert + if (confirm(`Are you sure you want to leave the "${this.group.fullName}" group?`)) { + this.leaveGroup(); + } + }, + leaveGroup() { + eventHub.$emit('leaveGroup', this.group, this.collection); + }, + }, + computed: { + groupDomId() { + return `group-${this.group.id}`; + }, + rowClass() { + return { + 'group-row': true, + 'is-open': this.group.isOpen, + 'has-subgroups': this.group.hasSubgroups, + 'no-description': !this.group.description, + }; + }, + visibilityIcon() { + return { + fa: true, + 'fa-globe': this.group.visibility === 'public', + 'fa-shield': this.group.visibility === 'internal', + 'fa-lock': this.group.visibility === 'private', + }; + }, + fullPath() { + let fullPath = ''; + + if (this.group.isOrphan) { + // check if current group is baseGroup + if (Object.keys(this.baseGroup).length > 0 && this.baseGroup !== this.group) { + // Remove baseGroup prefix from our current group.fullName. e.g: + // baseGroup.fullName: `level1` + // group.fullName: `level1 / level2 / level3` + // Result: `level2 / level3` + const gfn = this.group.fullName; + const bfn = this.baseGroup.fullName; + const length = bfn.length; + const start = gfn.indexOf(bfn); + const extraPrefixChars = 3; + + fullPath = gfn.substr(start + length + extraPrefixChars); + } else { + fullPath = this.group.fullName; + } + } else { + fullPath = this.group.name; + } + + return fullPath; + }, + hasGroups() { + return Object.keys(this.group.subGroups).length > 0; + }, + }, +}; +</script> + +<template> + <li + @click.stop="onClickRowGroup" + :id="groupDomId" + :class="rowClass" + > + <div + class="group-row-contents"> + <div + class="controls"> + <a + v-if="group.canEdit" + class="edit-group btn" + :href="group.editPath"> + <i + class="fa fa-cogs" + aria-hidden="true" + > + </i> + </a> + <a + @click="onLeaveGroup" + :href="group.leavePath" + class="leave-group btn" + title="Leave this group"> + <i + class="fa fa-sign-out" + aria-hidden="true" + > + </i> + </a> + </div> + <div + class="stats"> + <span + class="number-projects"> + <i + class="fa fa-bookmark" + aria-hidden="true" + > + </i> + {{group.numberProjects}} + </span> + <span + class="number-users"> + <i + class="fa fa-users" + aria-hidden="true" + > + </i> + {{group.numberUsers}} + </span> + <span + class="group-visibility"> + <i + :class="visibilityIcon" + aria-hidden="true" + > + </i> + </span> + </div> + <div + class="folder-toggle-wrap"> + <span + class="folder-caret" + v-if="group.hasSubgroups"> + <i + v-if="group.isOpen" + class="fa fa-caret-down" + aria-hidden="true" + > + </i> + <i + v-if="!group.isOpen" + class="fa fa-caret-right" + aria-hidden="true" + > + </i> + </span> + <span class="folder-icon"> + <i + v-if="group.isOpen" + class="fa fa-folder-open" + aria-hidden="true" + > + </i> + <i + v-if="!group.isOpen" + class="fa fa-folder" + aria-hidden="true"> + </i> + </span> + </div> + <div + class="avatar-container s40 hidden-xs"> + <a + :href="group.webUrl"> + <img + class="avatar s40" + :src="group.avatarUrl" + /> + </a> + </div> + <div + class="title"> + <a + :href="group.webUrl">{{fullPath}}</a> + <template v-if="group.permissions.humanGroupAccess"> + as + <span class="access-type">{{group.permissions.humanGroupAccess}}</span> + </template> + </div> + <div + class="description">{{group.description}}</div> + </div> + <group-folder + v-if="group.isOpen && hasGroups" + :groups="group.subGroups" + :baseGroup="group" + /> + </li> +</template> diff --git a/app/assets/javascripts/groups/components/groups.vue b/app/assets/javascripts/groups/components/groups.vue new file mode 100644 index 00000000000..36a04d4202f --- /dev/null +++ b/app/assets/javascripts/groups/components/groups.vue @@ -0,0 +1,39 @@ +<script> +import tablePagination from '~/vue_shared/components/table_pagination.vue'; +import eventHub from '../event_hub'; + +export default { + props: { + groups: { + type: Object, + required: true, + }, + pageInfo: { + type: Object, + required: true, + }, + }, + components: { + tablePagination, + }, + methods: { + change(page) { + const filterGroupsParam = gl.utils.getParameterByName('filter_groups'); + const sortParam = gl.utils.getParameterByName('sort'); + eventHub.$emit('fetchPage', page, filterGroupsParam, sortParam); + }, + }, +}; +</script> + +<template> + <div class="groups-list-tree-container"> + <group-folder + :groups="groups" + /> + <table-pagination + :change="change" + :pageInfo="pageInfo" + /> + </div> +</template> diff --git a/app/assets/javascripts/groups/event_hub.js b/app/assets/javascripts/groups/event_hub.js new file mode 100644 index 00000000000..0948c2e5352 --- /dev/null +++ b/app/assets/javascripts/groups/event_hub.js @@ -0,0 +1,3 @@ +import Vue from 'vue'; + +export default new Vue(); diff --git a/app/assets/javascripts/groups/groups_filterable_list.js b/app/assets/javascripts/groups/groups_filterable_list.js new file mode 100644 index 00000000000..439a931ddad --- /dev/null +++ b/app/assets/javascripts/groups/groups_filterable_list.js @@ -0,0 +1,87 @@ +import FilterableList from '~/filterable_list'; +import eventHub from './event_hub'; + +export default class GroupFilterableList extends FilterableList { + constructor({ form, filter, holder, filterEndpoint, pagePath }) { + super(form, filter, holder); + this.form = form; + this.filterEndpoint = filterEndpoint; + this.pagePath = pagePath; + this.$dropdown = $('.js-group-filter-dropdown-wrap'); + } + + getFilterEndpoint() { + return this.filterEndpoint; + } + + getPagePath(queryData) { + const params = queryData ? $.param(queryData) : ''; + const queryString = params ? `?${params}` : ''; + return `${this.pagePath}${queryString}`; + } + + bindEvents() { + super.bindEvents(); + + this.onFormSubmitWrapper = this.onFormSubmit.bind(this); + this.onFilterOptionClikWrapper = this.onOptionClick.bind(this); + + this.filterForm.addEventListener('submit', this.onFormSubmitWrapper); + this.$dropdown.on('click', 'a', this.onFilterOptionClikWrapper); + } + + onFormSubmit(e) { + e.preventDefault(); + + const $form = $(this.form); + const filterGroupsParam = $form.find('[name="filter_groups"]').val(); + const queryData = {}; + + if (filterGroupsParam) { + queryData.filter_groups = filterGroupsParam; + } + + this.filterResults(queryData); + this.setDefaultFilterOption(); + } + + setDefaultFilterOption() { + const defaultOption = $.trim(this.$dropdown.find('.dropdown-menu a:first-child').text()); + this.$dropdown.find('.dropdown-label').text(defaultOption); + } + + onOptionClick(e) { + e.preventDefault(); + + const queryData = {}; + const sortParam = gl.utils.getParameterByName('sort', e.currentTarget.href); + + if (sortParam) { + queryData.sort = sortParam; + } + + this.filterResults(queryData); + + // Active selected option + this.$dropdown.find('.dropdown-label').text($.trim(e.currentTarget.text)); + + // Clear current value on search form + this.form.querySelector('[name="filter_groups"]').value = ''; + } + + onFilterSuccess(data, xhr, queryData) { + super.onFilterSuccess(data, xhr, queryData); + + const paginationData = { + 'X-Per-Page': xhr.getResponseHeader('X-Per-Page'), + 'X-Page': xhr.getResponseHeader('X-Page'), + 'X-Total': xhr.getResponseHeader('X-Total'), + 'X-Total-Pages': xhr.getResponseHeader('X-Total-Pages'), + 'X-Next-Page': xhr.getResponseHeader('X-Next-Page'), + 'X-Prev-Page': xhr.getResponseHeader('X-Prev-Page'), + }; + + eventHub.$emit('updateGroups', data); + eventHub.$emit('updatePagination', paginationData); + } +} diff --git a/app/assets/javascripts/groups/index.js b/app/assets/javascripts/groups/index.js new file mode 100644 index 00000000000..ff601db2aa6 --- /dev/null +++ b/app/assets/javascripts/groups/index.js @@ -0,0 +1,190 @@ +/* global Flash */ + +import Vue from 'vue'; +import GroupFilterableList from './groups_filterable_list'; +import GroupsComponent from './components/groups.vue'; +import GroupFolder from './components/group_folder.vue'; +import GroupItem from './components/group_item.vue'; +import GroupsStore from './stores/groups_store'; +import GroupsService from './services/groups_service'; +import eventHub from './event_hub'; + +document.addEventListener('DOMContentLoaded', () => { + const el = document.getElementById('dashboard-group-app'); + + // Don't do anything if element doesn't exist (No groups) + // This is for when the user enters directly to the page via URL + if (!el) { + return; + } + + Vue.component('groups-component', GroupsComponent); + Vue.component('group-folder', GroupFolder); + Vue.component('group-item', GroupItem); + + // eslint-disable-next-line no-new + new Vue({ + el, + data() { + this.store = new GroupsStore(); + this.service = new GroupsService(el.dataset.endpoint); + + return { + store: this.store, + isLoading: true, + state: this.store.state, + loading: true, + }; + }, + computed: { + isEmpty() { + return Object.keys(this.state.groups).length === 0; + }, + }, + methods: { + fetchGroups(parentGroup) { + let parentId = null; + let getGroups = null; + let page = null; + let sort = null; + let pageParam = null; + let sortParam = null; + let filterGroups = null; + let filterGroupsParam = null; + + if (parentGroup) { + parentId = parentGroup.id; + } else { + this.isLoading = true; + } + + pageParam = gl.utils.getParameterByName('page'); + if (pageParam) { + page = pageParam; + } + + filterGroupsParam = gl.utils.getParameterByName('filter_groups'); + if (filterGroupsParam) { + filterGroups = filterGroupsParam; + } + + sortParam = gl.utils.getParameterByName('sort'); + if (sortParam) { + sort = sortParam; + } + + getGroups = this.service.getGroups(parentId, page, filterGroups, sort); + getGroups + .then(response => response.json()) + .then((response) => { + this.isLoading = false; + + this.updateGroups(response, parentGroup); + }) + .catch(this.handleErrorResponse); + + return getGroups; + }, + fetchPage(page, filterGroups, sort) { + this.isLoading = true; + + return this.service + .getGroups(null, page, filterGroups, sort) + .then((response) => { + this.isLoading = false; + $.scrollTo(0); + + const currentPath = gl.utils.mergeUrlParams({ page }, window.location.href); + window.history.replaceState({ + page: currentPath, + }, document.title, currentPath); + + this.updateGroups(response.json()); + this.updatePagination(response.headers); + }) + .catch(this.handleErrorResponse); + }, + toggleSubGroups(parentGroup = null) { + if (!parentGroup.isOpen) { + this.store.resetGroups(parentGroup); + this.fetchGroups(parentGroup); + } + + this.store.toggleSubGroups(parentGroup); + }, + leaveGroup(group, collection) { + this.service.leaveGroup(group.leavePath) + .then((response) => { + $.scrollTo(0); + + this.store.removeGroup(group, collection); + + // eslint-disable-next-line no-new + new Flash(response.json().notice, 'notice'); + }) + .catch((response) => { + let message = 'An error occurred. Please try again.'; + + if (response.status === 403) { + message = 'Failed to leave the group. Please make sure you are not the only owner'; + } + + // eslint-disable-next-line no-new + new Flash(message); + }); + }, + updateGroups(groups, parentGroup) { + this.store.setGroups(groups, parentGroup); + }, + updatePagination(headers) { + this.store.storePagination(headers); + }, + handleErrorResponse() { + this.isLoading = false; + $.scrollTo(0); + + // eslint-disable-next-line no-new + new Flash('An error occurred. Please try again.'); + }, + }, + created() { + eventHub.$on('fetchPage', this.fetchPage); + eventHub.$on('toggleSubGroups', this.toggleSubGroups); + eventHub.$on('leaveGroup', this.leaveGroup); + eventHub.$on('updateGroups', this.updateGroups); + eventHub.$on('updatePagination', this.updatePagination); + }, + beforeMount() { + let groupFilterList = null; + const form = document.querySelector('form#group-filter-form'); + const filter = document.querySelector('.js-groups-list-filter'); + const holder = document.querySelector('.js-groups-list-holder'); + + const opts = { + form, + filter, + holder, + filterEndpoint: el.dataset.endpoint, + pagePath: el.dataset.path, + }; + + groupFilterList = new GroupFilterableList(opts); + groupFilterList.initSearch(); + }, + mounted() { + this.fetchGroups() + .then((response) => { + this.updatePagination(response.headers); + this.isLoading = false; + }) + .catch(this.handleErrorResponse); + }, + beforeDestroy() { + eventHub.$off('fetchPage', this.fetchPage); + eventHub.$off('toggleSubGroups', this.toggleSubGroups); + eventHub.$off('leaveGroup', this.leaveGroup); + eventHub.$off('updateGroups', this.updateGroups); + eventHub.$off('updatePagination', this.updatePagination); + }, + }); +}); diff --git a/app/assets/javascripts/groups/services/groups_service.js b/app/assets/javascripts/groups/services/groups_service.js new file mode 100644 index 00000000000..97e02fcb76d --- /dev/null +++ b/app/assets/javascripts/groups/services/groups_service.js @@ -0,0 +1,38 @@ +import Vue from 'vue'; +import VueResource from 'vue-resource'; + +Vue.use(VueResource); + +export default class GroupsService { + constructor(endpoint) { + this.groups = Vue.resource(endpoint); + } + + getGroups(parentId, page, filterGroups, sort) { + const data = {}; + + if (parentId) { + data.parent_id = parentId; + } else { + // Do not send the following param for sub groups + if (page) { + data.page = page; + } + + if (filterGroups) { + data.filter_groups = filterGroups; + } + + if (sort) { + data.sort = sort; + } + } + + return this.groups.get(data); + } + + // eslint-disable-next-line class-methods-use-this + leaveGroup(endpoint) { + return Vue.http.delete(endpoint); + } +} diff --git a/app/assets/javascripts/groups/stores/groups_store.js b/app/assets/javascripts/groups/stores/groups_store.js new file mode 100644 index 00000000000..67ee7d140ce --- /dev/null +++ b/app/assets/javascripts/groups/stores/groups_store.js @@ -0,0 +1,151 @@ +import Vue from 'vue'; + +export default class GroupsStore { + constructor() { + this.state = {}; + this.state.groups = {}; + this.state.pageInfo = {}; + } + + setGroups(rawGroups, parent) { + const parentGroup = parent; + const tree = this.buildTree(rawGroups, parentGroup); + + if (parentGroup) { + parentGroup.subGroups = tree; + } else { + this.state.groups = tree; + } + + return tree; + } + + // eslint-disable-next-line class-methods-use-this + resetGroups(parent) { + const parentGroup = parent; + parentGroup.subGroups = {}; + } + + storePagination(pagination = {}) { + let paginationInfo; + + if (Object.keys(pagination).length) { + const normalizedHeaders = gl.utils.normalizeHeaders(pagination); + paginationInfo = gl.utils.parseIntPagination(normalizedHeaders); + } else { + paginationInfo = pagination; + } + + this.state.pageInfo = paginationInfo; + } + + buildTree(rawGroups, parentGroup) { + const groups = this.decorateGroups(rawGroups); + const tree = {}; + const mappedGroups = {}; + const orphans = []; + + // Map groups to an object + groups.map((group) => { + mappedGroups[group.id] = group; + mappedGroups[group.id].subGroups = {}; + return group; + }); + + Object.keys(mappedGroups).map((key) => { + const currentGroup = mappedGroups[key]; + if (currentGroup.parentId) { + // If the group is not at the root level, add it to its parent array of subGroups. + const findParentGroup = mappedGroups[currentGroup.parentId]; + if (findParentGroup) { + mappedGroups[currentGroup.parentId].subGroups[currentGroup.id] = currentGroup; + mappedGroups[currentGroup.parentId].isOpen = true; // Expand group if it has subgroups + } else if (parentGroup && parentGroup.id === currentGroup.parentId) { + tree[currentGroup.id] = currentGroup; + } else { + // Means the groups hast no direct parent. + // Save for later processing, we will add them to its corresponding base group + orphans.push(currentGroup); + } + } else { + // If the group is at the root level, add it to first level elements array. + tree[currentGroup.id] = currentGroup; + } + + return key; + }); + + // Hopefully this array will be empty for most cases + if (orphans.length) { + orphans.map((orphan) => { + let found = false; + const currentOrphan = orphan; + + Object.keys(tree).map((key) => { + const group = tree[key]; + if (currentOrphan.fullPath.lastIndexOf(group.fullPath) === 0) { + group.subGroups[currentOrphan.id] = currentOrphan; + group.isOpen = true; + currentOrphan.isOrphan = true; + found = true; + } + + return key; + }); + + if (!found) { + currentOrphan.isOrphan = true; + tree[currentOrphan.id] = currentOrphan; + } + + return orphan; + }); + } + + return tree; + } + + decorateGroups(rawGroups) { + this.groups = rawGroups.map(this.decorateGroup); + return this.groups; + } + + // eslint-disable-next-line class-methods-use-this + decorateGroup(rawGroup) { + return { + id: rawGroup.id, + fullName: rawGroup.full_name, + fullPath: rawGroup.full_path, + avatarUrl: rawGroup.avatar_url, + name: rawGroup.name, + hasSubgroups: rawGroup.has_subgroups, + canEdit: rawGroup.can_edit, + description: rawGroup.description, + webUrl: rawGroup.web_url, + parentId: rawGroup.parent_id, + visibility: rawGroup.visibility, + leavePath: rawGroup.leave_path, + editPath: rawGroup.edit_path, + isOpen: false, + isOrphan: false, + numberProjects: rawGroup.number_projects_with_delimiter, + numberUsers: rawGroup.number_users_with_delimiter, + permissions: { + humanGroupAccess: rawGroup.permissions.human_group_access, + }, + subGroups: {}, + }; + } + + // eslint-disable-next-line class-methods-use-this + removeGroup(group, collection) { + Vue.delete(collection, group.id); + } + + // eslint-disable-next-line class-methods-use-this + toggleSubGroups(toggleGroup) { + const group = toggleGroup; + group.isOpen = !group.isOpen; + return group; + } +} diff --git a/app/assets/javascripts/lib/utils/common_utils.js b/app/assets/javascripts/lib/utils/common_utils.js index a537267643e..2aca86189fd 100644 --- a/app/assets/javascripts/lib/utils/common_utils.js +++ b/app/assets/javascripts/lib/utils/common_utils.js @@ -167,8 +167,8 @@ if the name does not exist this function will return `null` otherwise it will return the value of the param key provided */ - w.gl.utils.getParameterByName = (name) => { - const url = window.location.href; + w.gl.utils.getParameterByName = (name, parseUrl) => { + const url = parseUrl || window.location.href; name = name.replace(/[[\]]/g, '\\$&'); const regex = new RegExp(`[?&]${name}(=([^&#]*)|&|#|$)`); const results = regex.exec(url); diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index 929965de5c1..b0143b12cfe 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -1478,7 +1478,7 @@ const normalizeNewlines = function(str) { const cachedNoteBodyText = $noteBodyText.html(); // Show updated comment content temporarily - $noteBodyText.html(formContent); + $noteBodyText.html(_.escape(formContent)); $editingNote.removeClass('is-editing fade-in-full').addClass('being-posted fade-in-half'); $editingNote.find('.note-headline-meta a').html('<i class="fa fa-spinner fa-spin" aria-label="Comment is being updated" aria-hidden="true"></i>'); @@ -1491,7 +1491,7 @@ const normalizeNewlines = function(str) { }) .fail(() => { // Submission failed, revert back to original note - $noteBodyText.html(cachedNoteBodyText); + $noteBodyText.html(_.escape(cachedNoteBodyText)); $editingNote.removeClass('being-posted fade-in'); $editingNote.find('.fa.fa-spinner').remove(); diff --git a/app/assets/javascripts/pipelines/pipelines.js b/app/assets/javascripts/pipelines/pipelines.js index 9f247af1dec..23b967b4b32 100644 --- a/app/assets/javascripts/pipelines/pipelines.js +++ b/app/assets/javascripts/pipelines/pipelines.js @@ -284,9 +284,7 @@ export default { <table-pagination v-if="shouldRenderPagination" - :pagenum="pagenum" :change="change" - :count="state.count.all" :pageInfo="state.pageInfo" /> </div> diff --git a/app/assets/stylesheets/framework/lists.scss b/app/assets/stylesheets/framework/lists.scss index 49163653548..38727e15c6f 100644 --- a/app/assets/stylesheets/framework/lists.scss +++ b/app/assets/stylesheets/framework/lists.scss @@ -264,3 +264,103 @@ ul.controls { ul.indent-list { padding: 10px 0 0 30px; } + + +// Specific styles for tree list +.group-list-tree { + .folder-toggle-wrap { + float: left; + line-height: $list-text-height; + font-size: 0; + + span { + font-size: $gl-font-size; + } + } + + .folder-caret, + .folder-icon { + display: inline-block; + } + + .folder-caret { + width: 15px; + } + + .folder-icon { + width: 20px; + } + + > .group-row:not(.has-subgroups) { + .folder-caret .fa { + opacity: 0; + } + } + + .content-list li:last-child { + padding-bottom: 0; + } + + .group-list-tree { + margin-bottom: 0; + margin-left: 30px; + position: relative; + + &::before { + content: ''; + display: block; + width: 0; + position: absolute; + top: 5px; + bottom: 0; + left: -16px; + border-left: 2px solid $border-white-normal; + } + + .group-row { + position: relative; + + &::before { + content: ""; + display: block; + width: 10px; + height: 0; + border-top: 2px solid $border-white-normal; + position: absolute; + top: 30px; + left: -16px; + } + + &:last-child::before { + background: $white-light; + height: auto; + top: 30px; + bottom: 0; + } + } + } + + .group-row { + padding: 0; + border: none; + } + + .group-row-contents { + padding: 10px 10px 8px; + border-top: solid 1px transparent; + border-bottom: solid 1px $white-normal; + + &:hover { + border-color: $row-hover-border; + background-color: $row-hover; + cursor: pointer; + } + } +} + +.js-groups-list-holder { + .groups-list-loading { + font-size: 34px; + text-align: center; + } +} diff --git a/app/assets/stylesheets/pages/members.scss b/app/assets/stylesheets/pages/members.scss index 971d54e7472..4be0e133b69 100644 --- a/app/assets/stylesheets/pages/members.scss +++ b/app/assets/stylesheets/pages/members.scss @@ -3,6 +3,41 @@ border-bottom: 1px solid $border-color; } +.project-member-tabs { + background: $gray-light; + border: 1px solid $border-color; + + li { + width: 50%; + + &.active { + background: $white-light; + } + + &:first-child { + border-right: 1px solid $border-color; + } + + a { + width: 100%; + text-align: center; + } + } +} + +.users-project-form { + .btn-create { + margin-right: 10px; + } +} + +.project-member-tab-content { + padding: $gl-padding; + border: 1px solid $border-color; + border-top: 0; + margin-bottom: $gl-padding; +} + .member { .list-item-name { @media (min-width: $screen-sm-min) { diff --git a/app/controllers/autocomplete_controller.rb b/app/controllers/autocomplete_controller.rb index 907717dcb96..fe331a883c1 100644 --- a/app/controllers/autocomplete_controller.rb +++ b/app/controllers/autocomplete_controller.rb @@ -21,7 +21,7 @@ class AutocompleteController < ApplicationController @users = [current_user, *@users].uniq end - if params[:author_id].present? + if params[:author_id].present? && current_user author = User.find_by_id(params[:author_id]) @users = [author, *@users].uniq if author end diff --git a/app/controllers/concerns/membership_actions.rb b/app/controllers/concerns/membership_actions.rb index cefb9b4e766..8d07780f6c2 100644 --- a/app/controllers/concerns/membership_actions.rb +++ b/app/controllers/concerns/membership_actions.rb @@ -52,9 +52,14 @@ module MembershipActions "You left the \"#{membershipable.human_name}\" #{source_type}." end - redirect_path = member.request? ? member.source : [:dashboard, membershipable.class.to_s.tableize] + respond_to do |format| + format.html do + redirect_path = member.request? ? member.source : [:dashboard, membershipable.class.to_s.tableize] + redirect_to redirect_path, notice: notice + end - redirect_to redirect_path, notice: notice + format.json { render json: { notice: notice } } + end end protected diff --git a/app/controllers/dashboard/groups_controller.rb b/app/controllers/dashboard/groups_controller.rb index d03265e9f20..742157d113d 100644 --- a/app/controllers/dashboard/groups_controller.rb +++ b/app/controllers/dashboard/groups_controller.rb @@ -1,16 +1,30 @@ class Dashboard::GroupsController < Dashboard::ApplicationController def index - @group_members = current_user.group_members.includes(source: :route).joins(:group) - @group_members = @group_members.merge(Group.search(params[:filter_groups])) if params[:filter_groups].present? - @group_members = @group_members.merge(Group.sort(@sort = params[:sort])) - @group_members = @group_members.page(params[:page]) + @groups = + if params[:parent_id] && Group.supports_nested_groups? + parent = Group.find_by(id: params[:parent_id]) + + if can?(current_user, :read_group, parent) + GroupsFinder.new(current_user, parent: parent).execute + else + Group.none + end + else + current_user.groups + end + + @groups = @groups.search(params[:filter_groups]) if params[:filter_groups].present? + @groups = @groups.includes(:route) + @groups = @groups.sort(@sort = params[:sort]) + @groups = @groups.page(params[:page]) respond_to do |format| format.html format.json do - render json: { - html: view_to_html_string("dashboard/groups/_groups", locals: { group_members: @group_members }) - } + render json: GroupSerializer + .new(current_user: @current_user) + .with_pagination(request, response) + .represent(@groups) end end end diff --git a/app/helpers/groups_helper.rb b/app/helpers/groups_helper.rb index a6014088e92..c003b01e226 100644 --- a/app/helpers/groups_helper.rb +++ b/app/helpers/groups_helper.rb @@ -8,7 +8,7 @@ module GroupsHelper group = Group.find_by_full_path(group) end - group.try(:avatar_url) || image_path('no_group_avatar.png') + group.try(:avatar_url) || ActionController::Base.helpers.image_path('no_group_avatar.png') end def group_title(group, name = nil, url = nil) diff --git a/app/policies/project_snippet_policy.rb b/app/policies/project_snippet_policy.rb index cf8ff92617f..bc5c4f32f79 100644 --- a/app/policies/project_snippet_policy.rb +++ b/app/policies/project_snippet_policy.rb @@ -1,5 +1,10 @@ class ProjectSnippetPolicy < BasePolicy def rules + # We have to check both project feature visibility and a snippet visibility and take the stricter one + # This will be simplified - check https://gitlab.com/gitlab-org/gitlab-ce/issues/27573 + return unless @subject.project.feature_available?(:snippets, @user) + return unless Ability.allowed?(@user, :read_project, @subject.project) + can! :read_project_snippet if @subject.public? return unless @user diff --git a/app/serializers/group_entity.rb b/app/serializers/group_entity.rb new file mode 100644 index 00000000000..4f506f7e745 --- /dev/null +++ b/app/serializers/group_entity.rb @@ -0,0 +1,46 @@ +class GroupEntity < Grape::Entity + include ActionView::Helpers::NumberHelper + include RequestAwareEntity + include MembersHelper + include GroupsHelper + + expose :id, :name, :path, :description, :visibility + expose :web_url + expose :full_name, :full_path + expose :parent_id + expose :created_at, :updated_at + + expose :permissions do + expose :human_group_access do |group, options| + group.group_members.find_by(user_id: request.current_user)&.human_access + end + end + + expose :edit_path do |group| + edit_group_path(group) + end + + expose :leave_path do |group| + leave_group_group_members_path(group) + end + + expose :can_edit do |group| + can?(request.current_user, :admin_group, group) + end + + expose :has_subgroups do |group| + GroupsFinder.new(request.current_user, parent: group).execute.any? + end + + expose :number_projects_with_delimiter do |group| + number_with_delimiter(GroupProjectsFinder.new(group: group, current_user: request.current_user).execute.count) + end + + expose :number_users_with_delimiter do |group| + number_with_delimiter(group.users.count) + end + + expose :avatar_url do |group| + group_icon(group) + end +end diff --git a/app/serializers/group_serializer.rb b/app/serializers/group_serializer.rb new file mode 100644 index 00000000000..26e8566828b --- /dev/null +++ b/app/serializers/group_serializer.rb @@ -0,0 +1,19 @@ +class GroupSerializer < BaseSerializer + entity GroupEntity + + def with_pagination(request, response) + tap { @paginator = Gitlab::Serializer::Pagination.new(request, response) } + end + + def paginated? + @paginator.present? + end + + def represent(resource, opts = {}) + if paginated? + super(@paginator.paginate(resource), opts) + else + super(resource, opts) + end + end +end diff --git a/app/uploaders/file_uploader.rb b/app/uploaders/file_uploader.rb index 7e94218c23d..652277e3b78 100644 --- a/app/uploaders/file_uploader.rb +++ b/app/uploaders/file_uploader.rb @@ -13,6 +13,13 @@ class FileUploader < GitlabUploader ) end + # Not using `GitlabUploader.base_dir` because all project namespaces are in + # the `public/uploads` dir. + # + def self.base_dir + root_dir + end + # Returns the part of `store_dir` that can change based on the model's current # path # diff --git a/app/uploaders/gitlab_uploader.rb b/app/uploaders/gitlab_uploader.rb index 02afddb8c6a..e4e6d6f46b1 100644 --- a/app/uploaders/gitlab_uploader.rb +++ b/app/uploaders/gitlab_uploader.rb @@ -3,16 +3,28 @@ class GitlabUploader < CarrierWave::Uploader::Base File.join(CarrierWave.root, upload_record.path) end - def self.base_dir + def self.root_dir 'uploads' end - delegate :base_dir, to: :class + # When object storage is used, keep the `root_dir` as `base_dir`. + # The files aren't really in folders there, they just have a name. + # The files that contain user input in their name, also contain a hash, so + # the names are still unique + # + # This method is overridden in the `FileUploader` + def self.base_dir + return root_dir unless file_storage? + + File.join(root_dir, 'system') + end - def file_storage? - storage.is_a?(CarrierWave::Storage::File) + def self.file_storage? + self.storage == CarrierWave::Storage::File end + delegate :base_dir, :file_storage?, to: :class + def file_cache_storage? cache_storage.is_a?(CarrierWave::Storage::File) end diff --git a/app/views/dashboard/groups/_groups.html.haml b/app/views/dashboard/groups/_groups.html.haml index 6c3bf1a2b3b..168e6272d8e 100644 --- a/app/views/dashboard/groups/_groups.html.haml +++ b/app/views/dashboard/groups/_groups.html.haml @@ -1,6 +1,9 @@ .js-groups-list-holder - %ul.content-list - - @group_members.each do |group_member| - = render 'shared/groups/group', group: group_member.group, group_member: group_member - - = paginate @group_members, theme: 'gitlab' + #dashboard-group-app{ data: { endpoint: dashboard_groups_path(format: :json), path: dashboard_groups_path } } + .groups-list-loading + = icon('spinner spin', 'v-show' => 'isLoading') + %template{ 'v-if' => '!isLoading && isEmpty' } + %div{ 'v-cloak' => true } + = render 'empty_state' + %template{ 'v-else-if' => '!isLoading && !isEmpty' } + %groups-component{ ':groups' => 'state.groups', ':page-info' => 'state.pageInfo' } diff --git a/app/views/dashboard/groups/index.html.haml b/app/views/dashboard/groups/index.html.haml index 73ab2c95ff9..f9b45a539a1 100644 --- a/app/views/dashboard/groups/index.html.haml +++ b/app/views/dashboard/groups/index.html.haml @@ -2,7 +2,10 @@ - header_title "Groups", dashboard_groups_path = render 'dashboard/groups_head' -- if @group_members.empty? += webpack_bundle_tag 'common_vue' += webpack_bundle_tag 'groups' + +- if @groups.empty? = render 'empty_state' - else = render 'groups' diff --git a/app/views/projects/group_links/_index.html.haml b/app/views/projects/group_links/_index.html.haml deleted file mode 100644 index debb0214d06..00000000000 --- a/app/views/projects/group_links/_index.html.haml +++ /dev/null @@ -1,53 +0,0 @@ -- page_title "Groups" -.row.prepend-top-default - .col-lg-3.settings-sidebar - %h4.prepend-top-0 - Share project with other groups - %p - Projects can be stored in only one group at once. However you can share a project with other groups here. - .col-lg-9 - = form_tag namespace_project_group_links_path(@project.namespace, @project), class: 'js-requires-input', method: :post do - .form-group - = label_tag :link_group_id, "Select a group to share with", class: "label-light" - = groups_select_tag(:link_group_id, data: { skip_groups: @skip_groups }, required: true) - .form-group - = label_tag :link_group_access, "Max access level", class: "label-light" - .select-wrapper - = select_tag :link_group_access, options_for_select(ProjectGroupLink.access_options, ProjectGroupLink.default_access), class: "form-control select-control" - = icon('caret-down') - .form-group - = label_tag :expires_at, 'Access expiration date', class: 'label-light' - .clearable-input - = text_field_tag :expires_at, nil, class: 'form-control js-access-expiration-date-groups', placeholder: 'Select access expiration date', id: 'expires_at_groups' - %i.clear-icon.js-clear-input - .help-block - On this date, all members in the group will automatically lose access to this project. - = submit_tag "Share", class: "btn btn-create" - .col-lg-9.col-lg-offset-3 - %hr - .col-lg-9.col-lg-offset-3.append-bottom-default.enabled-groups - %h5.prepend-top-0 - Groups you share with (#{@group_links.size}) - - if @group_links.present? - %ul.well-list - - @group_links.each do |group_link| - - group = group_link.group - %li - .pull-left.append-right-10.hidden-xs - = icon("folder-open-o", class: "settings-list-icon") - .pull-left - = link_to group do - = group.full_name - %br - up to #{group_link.human_access} - - if group_link.expires? - · - %span{ class: ('text-warning' if group_link.expires_soon?) } - expires in #{distance_of_time_in_words_to_now(group_link.expires_at)} - .pull-right - = link_to namespace_project_group_link_path(@project.namespace, @project, group_link), method: :delete, class: "btn btn-transparent" do - %span.sr-only disable sharing - = icon("trash") - - else - .settings-message.text-center - There are no groups with access to your project, add one in the form above diff --git a/app/views/projects/pipelines/_head.html.haml b/app/views/projects/pipelines/_head.html.haml index a33da149c62..d2f0cb0806f 100644 --- a/app/views/projects/pipelines/_head.html.haml +++ b/app/views/projects/pipelines/_head.html.haml @@ -10,7 +10,7 @@ Pipelines - if project_nav_tab? :builds - = nav_link(controller: [:builds, :artifacts]) do + = nav_link(controller: [:jobs, :artifacts]) do = link_to project_jobs_path(@project), title: 'Jobs', class: 'shortcuts-builds' do %span Jobs diff --git a/app/views/projects/project_members/_index.html.haml b/app/views/projects/project_members/_index.html.haml index d080b6c83d4..cfae371e169 100644 --- a/app/views/projects/project_members/_index.html.haml +++ b/app/views/projects/project_members/_index.html.haml @@ -1,11 +1,12 @@ .row.prepend-top-default .col-lg-3.settings-sidebar %h4.prepend-top-0 - Members + Project members - if can?(current_user, :admin_project_member, @project) %p - Add a new member to + You can add a new member to %strong= @project.name + or share it with another group. - else %p Members can be added by project @@ -13,9 +14,20 @@ or %i Owners .col-lg-9 - .light.prepend-top-default + .light - if can?(current_user, :admin_project_member, @project) - = render "projects/project_members/new_project_member" + %ul.nav-links.project-member-tabs{ role: 'tablist' } + %li.active{ role: 'presentation' } + %a{ href: '#add-member-pane', id: 'add-member-tab', data: { toggle: 'tab' }, role: 'tab' } Add member + - if @project.allowed_to_share_with_group? + %li{ role: 'presentation' } + %a{ href: '#share-with-group-pane', id: 'share-with-group-tab', data: { toggle: 'tab' }, role: 'tab' } Share with group + + .tab-content.project-member-tab-content + .tab-pane.active{ id: 'add-member-pane', role: 'tabpanel' } + = render 'projects/project_members/new_project_member', tab_title: 'Add member' + .tab-pane{ id: 'share-with-group-pane', role: 'tabpanel' } + = render 'projects/project_members/new_shared_group', tab_title: 'Share with group' = render 'shared/members/requests', membership_source: @project, requesters: @requesters .clearfix diff --git a/app/views/projects/project_members/_new_project_member.html.haml b/app/views/projects/project_members/_new_project_member.html.haml index 2b1c23f7dda..247c4bdbe2d 100644 --- a/app/views/projects/project_members/_new_project_member.html.haml +++ b/app/views/projects/project_members/_new_project_member.html.haml @@ -1,18 +1,19 @@ -= form_for @project_member, as: :project_member, url: namespace_project_project_members_path(@project.namespace, @project), html: { class: 'users-project-form' } do |f| - .form-group - = users_select_tag(:user_ids, multiple: true, class: "input-clamp", scope: :all, email_user: true, placeholder: "Search for members to update or invite") - .help-block.append-bottom-10 - Search for members by name, username, or email, or invite new ones using their email address. - .form-group - = select_tag :access_level, options_for_select(ProjectMember.access_level_roles, @project_member.access_level), class: "form-control project-access-select" - .help-block.append-bottom-10 - = link_to "Read more", help_page_path("user/permissions"), class: "vlink" - about role permissions - .form-group - .clearable-input - = text_field_tag :expires_at, nil, class: 'form-control js-access-expiration-date', placeholder: 'Expiration date' - %i.clear-icon.js-clear-input - .help-block.append-bottom-10 - On this date, the member(s) will automatically lose access to this project. - = f.submit "Add to project", class: "btn btn-create" - = link_to "Import", import_namespace_project_project_members_path(@project.namespace, @project), class: "btn btn-default", title: "Import members from another project" +.row + .col-sm-12 + = form_for @project_member, as: :project_member, url: namespace_project_project_members_path(@project.namespace, @project), html: { class: 'users-project-form' } do |f| + .form-group + = label_tag :user_ids, "Select members to invite", class: "label-light" + = users_select_tag(:user_ids, multiple: true, class: "input-clamp", scope: :all, email_user: true, placeholder: "Search for members to update or invite") + .form-group + = label_tag :access_level, "Choose a role permission", class: "label-light" + = select_tag :access_level, options_for_select(ProjectMember.access_level_roles, @project_member.access_level), class: "form-control project-access-select" + .help-block.append-bottom-10 + = link_to "Read more", help_page_path("user/permissions"), class: "vlink" + about role permissions + .form-group + .clearable-input + = label_tag :expires_at, 'Access expiration date', class: 'label-light' + = text_field_tag :expires_at, nil, class: 'form-control js-access-expiration-date', placeholder: 'Expiration date' + %i.clear-icon.js-clear-input + = f.submit "Add to project", class: "btn btn-create" + = link_to "Import", import_namespace_project_project_members_path(@project.namespace, @project), class: "btn btn-default", title: "Import members from another project" diff --git a/app/views/projects/project_members/_new_shared_group.html.haml b/app/views/projects/project_members/_new_shared_group.html.haml new file mode 100644 index 00000000000..b7cc8dd7062 --- /dev/null +++ b/app/views/projects/project_members/_new_shared_group.html.haml @@ -0,0 +1,20 @@ +.row + .col-sm-12 + = form_tag namespace_project_group_links_path(@project.namespace, @project), class: 'js-requires-input', method: :post do + .form-group + = label_tag :link_group_id, "Select a group to share with", class: "label-light" + = groups_select_tag(:link_group_id, data: { skip_groups: @skip_groups }, class: "input-clamp", required: true) + .form-group + = label_tag :link_group_access, "Max access level", class: "label-light" + .select-wrapper + = select_tag :link_group_access, options_for_select(ProjectGroupLink.access_options, ProjectGroupLink.default_access), class: "form-control select-control" + = icon('caret-down') + .help-block.append-bottom-10 + = link_to "Read more", help_page_path("user/permissions"), class: "vlink" + about role permissions + .form-group + = label_tag :expires_at, 'Access expiration date', class: 'label-light' + .clearable-input + = text_field_tag :expires_at, nil, class: 'form-control js-access-expiration-date-groups', placeholder: 'Expiration date', id: 'expires_at_groups' + %i.clear-icon.js-clear-input + = submit_tag "Share", class: "btn btn-create" diff --git a/app/views/projects/settings/members/show.html.haml b/app/views/projects/settings/members/show.html.haml index 20e1ad68244..343807b87cd 100644 --- a/app/views/projects/settings/members/show.html.haml +++ b/app/views/projects/settings/members/show.html.haml @@ -2,6 +2,3 @@ = render "projects/settings/head" = render "projects/project_members/index" -- if can?(current_user, :admin_project, @project) - - if @project.allowed_to_share_with_group? - = render "projects/group_links/index" diff --git a/app/views/shared/groups/_dropdown.html.haml b/app/views/shared/groups/_dropdown.html.haml index 37589b634fa..760370a6984 100644 --- a/app/views/shared/groups/_dropdown.html.haml +++ b/app/views/shared/groups/_dropdown.html.haml @@ -1,10 +1,10 @@ -.dropdown.inline +.dropdown.inline.js-group-filter-dropdown-wrap %button.dropdown-toggle{ type: 'button', 'data-toggle' => 'dropdown' } - %span.light - - if @sort.present? - = sort_options_hash[@sort] - - else - = sort_title_recently_created + %span.dropdown-label + - if @sort.present? + = sort_options_hash[@sort] + - else + = sort_title_recently_created = icon('chevron-down') %ul.dropdown-menu.dropdown-menu-align-right %li diff --git a/app/workers/post_receive.rb b/app/workers/post_receive.rb index c29571d3c62..89286595ca6 100644 --- a/app/workers/post_receive.rb +++ b/app/workers/post_receive.rb @@ -17,14 +17,15 @@ class PostReceive post_received = Gitlab::GitPostReceive.new(project, identifier, changes) if is_wiki - # Nothing defined here yet. + process_wiki_changes(post_received) else process_project_changes(post_received) - process_repository_update(post_received) end end - def process_repository_update(post_received) + private + + def process_project_changes(post_received) changes = [] refs = Set.new @@ -36,32 +37,27 @@ class PostReceive return false end - changes << Gitlab::DataBuilder::Repository.single_change(oldrev, newrev, ref) - refs << ref - end - - hook_data = Gitlab::DataBuilder::Repository.update(post_received.project, @user, changes, refs.to_a) - SystemHooksService.new.execute_hooks(hook_data, :repository_update_hooks) - end - - def process_project_changes(post_received) - post_received.changes_refs do |oldrev, newrev, ref| - @user ||= post_received.identify(newrev) - - unless @user - log("Triggered hook for non-existing user \"#{post_received.identifier}\"") - return false - end - if Gitlab::Git.tag_ref?(ref) GitTagPushService.new(post_received.project, @user, oldrev: oldrev, newrev: newrev, ref: ref).execute elsif Gitlab::Git.branch_ref?(ref) GitPushService.new(post_received.project, @user, oldrev: oldrev, newrev: newrev, ref: ref).execute end + + changes << Gitlab::DataBuilder::Repository.single_change(oldrev, newrev, ref) + refs << ref end + + after_project_changes_hooks(post_received, @user, refs.to_a, changes) end - private + def after_project_changes_hooks(post_received, user, refs, changes) + hook_data = Gitlab::DataBuilder::Repository.update(post_received.project, user, changes, refs) + SystemHooksService.new.execute_hooks(hook_data, :repository_update_hooks) + end + + def process_wiki_changes(post_received) + # Nothing defined here yet. + end # To maintain backwards compatibility, we accept both gl_repository or # repository paths as project identifiers. Our plan is to migrate to diff --git a/changelogs/unreleased/25426-group-dashboard-ui.yml b/changelogs/unreleased/25426-group-dashboard-ui.yml new file mode 100644 index 00000000000..cc2bf62d07b --- /dev/null +++ b/changelogs/unreleased/25426-group-dashboard-ui.yml @@ -0,0 +1,4 @@ +--- +title: Update Dashboard Groups UI with better support for subgroups +merge_request: +author: diff --git a/changelogs/unreleased/artifacts-keyboard-shortcuts.yml b/changelogs/unreleased/artifacts-keyboard-shortcuts.yml new file mode 100644 index 00000000000..69569504c4f --- /dev/null +++ b/changelogs/unreleased/artifacts-keyboard-shortcuts.yml @@ -0,0 +1,4 @@ +--- +title: Enabled keyboard shortcuts on artifacts pages +merge_request: +author: diff --git a/changelogs/unreleased/ce-31853-projects-shared-groups.yml b/changelogs/unreleased/ce-31853-projects-shared-groups.yml new file mode 100644 index 00000000000..ffa3aed682d --- /dev/null +++ b/changelogs/unreleased/ce-31853-projects-shared-groups.yml @@ -0,0 +1,4 @@ +--- +title: Remove duplication for sharing projects with groups in project settings +merge_request: +author: diff --git a/changelogs/unreleased/issuable-sidebar-edit-button-field-focus.yml b/changelogs/unreleased/issuable-sidebar-edit-button-field-focus.yml new file mode 100644 index 00000000000..05d52fcad0f --- /dev/null +++ b/changelogs/unreleased/issuable-sidebar-edit-button-field-focus.yml @@ -0,0 +1,4 @@ +--- +title: Fixed dropdown filter input not focusing after transition +merge_request: +author: diff --git a/config/routes/uploads.rb b/config/routes/uploads.rb index ae8747d766d..a49e244af1a 100644 --- a/config/routes/uploads.rb +++ b/config/routes/uploads.rb @@ -1,6 +1,6 @@ scope path: :uploads do # Note attachments and User/Group/Project avatars - get ":model/:mounted_as/:id/:filename", + get "system/:model/:mounted_as/:id/:filename", to: "uploads#show", constraints: { model: /note|user|group|project/, mounted_as: /avatar|attachment/, filename: /[^\/]+/ } @@ -15,7 +15,7 @@ scope path: :uploads do constraints: { filename: /[^\/]+/ } # Appearance - get ":model/:mounted_as/:id/:filename", + get "system/:model/:mounted_as/:id/:filename", to: "uploads#show", constraints: { model: /appearance/, mounted_as: /logo|header_logo/, filename: /.+/ } diff --git a/config/webpack.config.js b/config/webpack.config.js index cbcf5ce996d..7501acb7633 100644 --- a/config/webpack.config.js +++ b/config/webpack.config.js @@ -40,6 +40,7 @@ var config = { filtered_search: './filtered_search/filtered_search_bundle.js', graphs: './graphs/graphs_bundle.js', group: './group.js', + groups: './groups/index.js', groups_list: './groups_list.js', issue_show: './issue_show/index.js', integrations: './integrations', @@ -155,6 +156,7 @@ var config = { 'environments', 'environments_folder', 'filtered_search', + 'groups', 'issue_show', 'merge_conflicts', 'notebook_viewer', diff --git a/db/migrate/20170316163800_rename_system_namespaces.rb b/db/migrate/20170316163800_rename_system_namespaces.rb new file mode 100644 index 00000000000..b5408fbf112 --- /dev/null +++ b/db/migrate/20170316163800_rename_system_namespaces.rb @@ -0,0 +1,231 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. +class RenameSystemNamespaces < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + include Gitlab::ShellAdapter + disable_ddl_transaction! + + class User < ActiveRecord::Base + self.table_name = 'users' + end + + class Namespace < ActiveRecord::Base + self.table_name = 'namespaces' + belongs_to :parent, class_name: 'RenameSystemNamespaces::Namespace' + has_one :route, as: :source + has_many :children, class_name: 'RenameSystemNamespaces::Namespace', foreign_key: :parent_id + belongs_to :owner, class_name: 'RenameSystemNamespaces::User' + + # Overridden to have the correct `source_type` for the `route` relation + def self.name + 'Namespace' + end + + def full_path + if route && route.path.present? + @full_path ||= route.path + else + update_route if persisted? + + build_full_path + end + end + + def build_full_path + if parent && path + parent.full_path + '/' + path + else + path + end + end + + def update_route + prepare_route + route.save + end + + def prepare_route + route || build_route(source: self) + route.path = build_full_path + route.name = build_full_name + @full_path = nil + @full_name = nil + end + + def build_full_name + if parent && name + parent.human_name + ' / ' + name + else + name + end + end + + def human_name + owner&.name + end + end + + class Route < ActiveRecord::Base + self.table_name = 'routes' + belongs_to :source, polymorphic: true + end + + class Project < ActiveRecord::Base + self.table_name = 'projects' + + def repository_storage_path + Gitlab.config.repositories.storages[repository_storage]['path'] + end + end + + DOWNTIME = false + + def up + return unless system_namespace + + old_path = system_namespace.path + old_full_path = system_namespace.full_path + # Only remove the last occurrence of the path name to get the parent namespace path + namespace_path = remove_last_occurrence(old_full_path, old_path) + new_path = rename_path(namespace_path, old_path) + new_full_path = join_namespace_path(namespace_path, new_path) + + Namespace.where(id: system_namespace).update_all(path: new_path) # skips callbacks & validations + + replace_statement = replace_sql(Route.arel_table[:path], old_full_path, new_full_path) + route_matches = [old_full_path, "#{old_full_path}/%"] + + update_column_in_batches(:routes, :path, replace_statement) do |table, query| + query.where(Route.arel_table[:path].matches_any(route_matches)) + end + + clear_cache_for_namespace(system_namespace) + + # tasks here are based on `Namespace#move_dir` + move_repositories(system_namespace, old_full_path, new_full_path) + move_namespace_folders(uploads_dir, old_full_path, new_full_path) if file_storage? + move_namespace_folders(pages_dir, old_full_path, new_full_path) + end + + def down + # nothing to do + end + + def remove_last_occurrence(string, pattern) + string.reverse.sub(pattern.reverse, "").reverse + end + + def move_namespace_folders(directory, old_relative_path, new_relative_path) + old_path = File.join(directory, old_relative_path) + return unless File.directory?(old_path) + + new_path = File.join(directory, new_relative_path) + FileUtils.mv(old_path, new_path) + end + + def move_repositories(namespace, old_full_path, new_full_path) + repo_paths_for_namespace(namespace).each do |repository_storage_path| + # Ensure old directory exists before moving it + gitlab_shell.add_namespace(repository_storage_path, old_full_path) + + unless gitlab_shell.mv_namespace(repository_storage_path, old_full_path, new_full_path) + say "Exception moving path #{repository_storage_path} from #{old_full_path} to #{new_full_path}" + end + end + end + + def rename_path(namespace_path, path_was) + counter = 0 + path = "#{path_was}#{counter}" + + while route_exists?(join_namespace_path(namespace_path, path)) + counter += 1 + path = "#{path_was}#{counter}" + end + + path + end + + def route_exists?(full_path) + Route.where(Route.arel_table[:path].matches(full_path)).any? + end + + def join_namespace_path(namespace_path, path) + if namespace_path.present? + File.join(namespace_path, path) + else + path + end + end + + def system_namespace + @system_namespace ||= Namespace.where(parent_id: nil). + where(arel_table[:path].matches(system_namespace_path)). + first + end + + def system_namespace_path + "system" + end + + def clear_cache_for_namespace(namespace) + project_ids = projects_for_namespace(namespace).pluck(:id) + + update_column_in_batches(:projects, :description_html, nil) do |table, query| + query.where(table[:id].in(project_ids)) + end + + update_column_in_batches(:issues, :description_html, nil) do |table, query| + query.where(table[:project_id].in(project_ids)) + end + + update_column_in_batches(:merge_requests, :description_html, nil) do |table, query| + query.where(table[:target_project_id].in(project_ids)) + end + + update_column_in_batches(:notes, :note_html, nil) do |table, query| + query.where(table[:project_id].in(project_ids)) + end + + update_column_in_batches(:milestones, :description_html, nil) do |table, query| + query.where(table[:project_id].in(project_ids)) + end + end + + def projects_for_namespace(namespace) + namespace_ids = child_ids_for_parent(namespace, ids: [namespace.id]) + namespace_or_children = Project.arel_table[:namespace_id].in(namespace_ids) + Project.unscoped.where(namespace_or_children) + end + + # This won't scale to huge trees, but it should do for a handful of namespaces + # called `system`. + def child_ids_for_parent(namespace, ids: []) + namespace.children.each do |child| + ids << child.id + child_ids_for_parent(child, ids: ids) if child.children.any? + end + ids + end + + def repo_paths_for_namespace(namespace) + projects_for_namespace(namespace).distinct. + select(:repository_storage).map(&:repository_storage_path) + end + + def uploads_dir + File.join(Rails.root, "public", "uploads") + end + + def pages_dir + Settings.pages.path + end + + def file_storage? + CarrierWave::Uploader::Base.storage == CarrierWave::Storage::File + end + + def arel_table + Namespace.arel_table + end +end diff --git a/db/migrate/20170316163845_move_uploads_to_system_dir.rb b/db/migrate/20170316163845_move_uploads_to_system_dir.rb new file mode 100644 index 00000000000..564ee10b5ab --- /dev/null +++ b/db/migrate/20170316163845_move_uploads_to_system_dir.rb @@ -0,0 +1,59 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class MoveUploadsToSystemDir < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + disable_ddl_transaction! + + DOWNTIME = false + DIRECTORIES_TO_MOVE = %w(user project note group appearance).freeze + + def up + return unless file_storage? + + FileUtils.mkdir_p(new_upload_dir) + + DIRECTORIES_TO_MOVE.each do |dir| + source = File.join(old_upload_dir, dir) + destination = File.join(new_upload_dir, dir) + next unless File.directory?(source) + next if File.directory?(destination) + + say "Moving #{source} -> #{destination}" + FileUtils.mv(source, destination) + FileUtils.ln_s(destination, source) + end + end + + def down + return unless file_storage? + return unless File.directory?(new_upload_dir) + + DIRECTORIES_TO_MOVE.each do |dir| + source = File.join(new_upload_dir, dir) + destination = File.join(old_upload_dir, dir) + next unless File.directory?(source) + next if File.directory?(destination) && !File.symlink?(destination) + + say "Moving #{source} -> #{destination}" + FileUtils.rm(destination) if File.symlink?(destination) + FileUtils.mv(source, destination) + end + end + + def file_storage? + CarrierWave::Uploader::Base.storage == CarrierWave::Storage::File + end + + def base_directory + Rails.root + end + + def old_upload_dir + File.join(base_directory, "public", "uploads") + end + + def new_upload_dir + File.join(base_directory, "public", "uploads", "system") + end +end diff --git a/db/post_migrate/20170317162059_update_upload_paths_to_system.rb b/db/post_migrate/20170317162059_update_upload_paths_to_system.rb new file mode 100644 index 00000000000..9a77b0bbdfb --- /dev/null +++ b/db/post_migrate/20170317162059_update_upload_paths_to_system.rb @@ -0,0 +1,55 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class UpdateUploadPathsToSystem < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + AFFECTED_MODELS = %w(User Project Note Namespace Appearance) + + def up + update_column_in_batches(:uploads, :path, replace_sql(arel_table[:path], base_directory, new_upload_dir)) do |_table, query| + query.where(uploads_to_switch_to_new_path) + end + end + + def down + update_column_in_batches(:uploads, :path, replace_sql(arel_table[:path], new_upload_dir, base_directory)) do |_table, query| + query.where(uploads_to_switch_to_old_path) + end + end + + # "SELECT \"uploads\".* FROM \"uploads\" WHERE \"uploads\".\"model_type\" IN ('User', 'Project', 'Note', 'Namespace', 'Appearance') AND (\"uploads\".\"path\" ILIKE 'uploads/%' AND NOT (\"uploads\".\"path\" ILIKE 'uploads/system/%'))" + def uploads_to_switch_to_new_path + affected_uploads.and(starting_with_base_directory).and(starting_with_new_upload_directory.not) + end + + # "SELECT \"uploads\".* FROM \"uploads\" WHERE \"uploads\".\"model_type\" IN ('User', 'Project', 'Note', 'Namespace', 'Appearance') AND (\"uploads\".\"path\" ILIKE 'uploads/%' AND \"uploads\".\"path\" ILIKE 'uploads/system/%')" + def uploads_to_switch_to_old_path + affected_uploads.and(starting_with_new_upload_directory) + end + + def starting_with_base_directory + arel_table[:path].matches("#{base_directory}/%") + end + + def starting_with_new_upload_directory + arel_table[:path].matches("#{new_upload_dir}/%") + end + + def affected_uploads + arel_table[:model_type].in(AFFECTED_MODELS) + end + + def base_directory + "uploads" + end + + def new_upload_dir + File.join(base_directory, "system") + end + + def arel_table + Arel::Table.new(:uploads) + end +end diff --git a/db/post_migrate/20170406111121_clean_upload_symlinks.rb b/db/post_migrate/20170406111121_clean_upload_symlinks.rb new file mode 100644 index 00000000000..3ac9a6c10bc --- /dev/null +++ b/db/post_migrate/20170406111121_clean_upload_symlinks.rb @@ -0,0 +1,52 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class CleanUploadSymlinks < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + disable_ddl_transaction! + + DOWNTIME = false + DIRECTORIES_TO_MOVE = %w(user project note group appeareance) + + def up + return unless file_storage? + + DIRECTORIES_TO_MOVE.each do |dir| + symlink_location = File.join(old_upload_dir, dir) + next unless File.symlink?(symlink_location) + say "removing symlink: #{symlink_location}" + FileUtils.rm(symlink_location) + end + end + + def down + return unless file_storage? + + DIRECTORIES_TO_MOVE.each do |dir| + symlink = File.join(old_upload_dir, dir) + destination = File.join(new_upload_dir, dir) + + next if File.directory?(symlink) + next unless File.directory?(destination) + + say "Creating symlink #{symlink} -> #{destination}" + FileUtils.ln_s(destination, symlink) + end + end + + def file_storage? + CarrierWave::Uploader::Base.storage == CarrierWave::Storage::File + end + + def base_directory + Rails.root + end + + def old_upload_dir + File.join(base_directory, "public", "uploads") + end + + def new_upload_dir + File.join(base_directory, "public", "uploads", "system") + end +end diff --git a/db/post_migrate/20170606202615_move_appearance_to_system_dir.rb b/db/post_migrate/20170606202615_move_appearance_to_system_dir.rb new file mode 100644 index 00000000000..561de59ec69 --- /dev/null +++ b/db/post_migrate/20170606202615_move_appearance_to_system_dir.rb @@ -0,0 +1,57 @@ +class MoveAppearanceToSystemDir < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + disable_ddl_transaction! + + DOWNTIME = false + DIRECTORY_TO_MOVE = 'appearance'.freeze + + def up + source = File.join(old_upload_dir, DIRECTORY_TO_MOVE) + destination = File.join(new_upload_dir, DIRECTORY_TO_MOVE) + + move_directory(source, destination) + end + + def down + source = File.join(new_upload_dir, DIRECTORY_TO_MOVE) + destination = File.join(old_upload_dir, DIRECTORY_TO_MOVE) + + move_directory(source, destination) + end + + def move_directory(source, destination) + unless file_storage? + say 'Not using file storage, skipping' + return + end + + unless File.directory?(source) + say "#{source} did not exist, skipping" + return + end + + if File.directory?(destination) + say "#{destination} already existed, skipping" + return + end + + say "Moving #{source} -> #{destination}" + FileUtils.mv(source, destination) + end + + def file_storage? + CarrierWave::Uploader::Base.storage == CarrierWave::Storage::File + end + + def base_directory + Rails.root + end + + def old_upload_dir + File.join(base_directory, "public", "uploads") + end + + def new_upload_dir + File.join(base_directory, "public", "uploads", "system") + end +end diff --git a/db/schema.rb b/db/schema.rb index 83172a92b49..b93630a410d 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,8 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20170603200744) do - +ActiveRecord::Schema.define(version: 20170606202615) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" enable_extension "pg_trgm" diff --git a/doc/api/README.md b/doc/api/README.md index 2175b305e02..1241801a81c 100644 --- a/doc/api/README.md +++ b/doc/api/README.md @@ -57,12 +57,16 @@ following locations: ## Road to GraphQL -API v4 will be the last REST API that we support. Going forward, we will start -on moving to GraphQL and deprecate the use of controller-specific -endpoints. GraphQL has a number of benefits: +Going forward, we will start on moving to +[GraphQL](http://graphql.org/learn/best-practices/) and deprecate the use of +controller-specific endpoints. GraphQL has a number of benefits: 1. We avoid having to maintain two different APIs. 2. Callers of the API can request only what they need. +3. It is versioned by default. + +It will co-exist with the current V4 REST API. If we have a V5 API, this should be +compatability layer on top of GraphQL. ### Internal CI API diff --git a/doc/install/requirements.md b/doc/install/requirements.md index 5338ccb9d3a..197a92905c8 100644 --- a/doc/install/requirements.md +++ b/doc/install/requirements.md @@ -129,7 +129,8 @@ We _highly_ recommend the use of PostgreSQL instead of MySQL/MariaDB as not all features of GitLab may work with MySQL/MariaDB. For example, MySQL does not have the right features to support nested groups in an efficient manner; see <https://gitlab.com/gitlab-org/gitlab-ce/issues/30472> for more information -about this. Existing users using GitLab with MySQL/MariaDB are advised to +about this. GitLab Geo also does [not support MySQL](https://docs.gitlab.com/ee/gitlab-geo/database.html#mysql-replication). +Existing users using GitLab with MySQL/MariaDB are advised to migrate to PostgreSQL instead. The server running the database should have _at least_ 5-10 GB of storage diff --git a/features/steps/groups.rb b/features/steps/groups.rb index 83d8abbab1f..25bb374b868 100644 --- a/features/steps/groups.rb +++ b/features/steps/groups.rb @@ -81,7 +81,7 @@ class Spinach::Features::Groups < Spinach::FeatureSteps step 'I should see new group "Owned" avatar' do expect(owned_group.avatar).to be_instance_of AvatarUploader - expect(owned_group.avatar.url).to eq "/uploads/group/avatar/#{Group.find_by(name: "Owned").id}/banana_sample.gif" + expect(owned_group.avatar.url).to eq "/uploads/system/group/avatar/#{Group.find_by(name: "Owned").id}/banana_sample.gif" end step 'I should see the "Remove avatar" button' do diff --git a/features/steps/profile/profile.rb b/features/steps/profile/profile.rb index 24cfbaad7fe..254c26bb6af 100644 --- a/features/steps/profile/profile.rb +++ b/features/steps/profile/profile.rb @@ -36,7 +36,7 @@ class Spinach::Features::Profile < Spinach::FeatureSteps step 'I should see new avatar' do expect(@user.avatar).to be_instance_of AvatarUploader - expect(@user.avatar.url).to eq "/uploads/user/avatar/#{@user.id}/banana_sample.gif" + expect(@user.avatar.url).to eq "/uploads/system/user/avatar/#{@user.id}/banana_sample.gif" end step 'I should see the "Remove avatar" button' do diff --git a/features/steps/project/project.rb b/features/steps/project/project.rb index de32c9afcca..7d34331db46 100644 --- a/features/steps/project/project.rb +++ b/features/steps/project/project.rb @@ -38,7 +38,7 @@ class Spinach::Features::Project < Spinach::FeatureSteps step 'I should see new project avatar' do expect(@project.avatar).to be_instance_of AvatarUploader url = @project.avatar.url - expect(url).to eq "/uploads/project/avatar/#{@project.id}/banana_sample.gif" + expect(url).to eq "/uploads/system/project/avatar/#{@project.id}/banana_sample.gif" end step 'I should see the "Remove avatar" button' do diff --git a/features/steps/project/project_group_links.rb b/features/steps/project/project_group_links.rb index 739a85e5fa4..5280a38ce81 100644 --- a/features/steps/project/project_group_links.rb +++ b/features/steps/project/project_group_links.rb @@ -5,18 +5,19 @@ class Spinach::Features::ProjectGroupLinks < Spinach::FeatureSteps include Select2Helper step 'I should see project already shared with group "Ops"' do - page.within '.enabled-groups' do + page.within '.project-members-groups' do expect(page).to have_content "Ops" end end step 'I should see project is not shared with group "Market"' do - page.within '.enabled-groups' do + page.within '.project-members-groups' do expect(page).not_to have_content "Market" end end step 'I select group "Market" for share' do + click_link 'Share with group' group = Group.find_by(path: 'market') select2(group.id, from: "#link_group_id") select "Master", from: 'link_group_access' @@ -24,7 +25,7 @@ class Spinach::Features::ProjectGroupLinks < Spinach::FeatureSteps end step 'I should see project is shared with group "Market"' do - page.within '.enabled-groups' do + page.within '.project-members-groups' do expect(page).to have_content "Market" end end diff --git a/lib/api/api.rb b/lib/api/api.rb index 88f91c07194..d767af36e8e 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -45,6 +45,7 @@ module API end before { allow_access_with_scope :api } + before { header['X-Frame-Options'] = 'SAMEORIGIN' } before { Gitlab::I18n.locale = current_user&.preferred_language } after { Gitlab::I18n.use_default_locale } diff --git a/lib/banzai/reference_parser/base_parser.rb b/lib/banzai/reference_parser/base_parser.rb index d99a3bfa625..1e2536231d8 100644 --- a/lib/banzai/reference_parser/base_parser.rb +++ b/lib/banzai/reference_parser/base_parser.rb @@ -62,7 +62,7 @@ module Banzai nodes.select do |node| if node.has_attribute?(project_attr) - can_read_reference?(user, projects[node]) + can_read_reference?(user, projects[node], node) else true end @@ -231,7 +231,7 @@ module Banzai # see reference comments. # Override this method on subclasses # to check if user can read resource - def can_read_reference?(user, ref_project) + def can_read_reference?(user, ref_project, node) raise NotImplementedError end diff --git a/lib/banzai/reference_parser/commit_parser.rb b/lib/banzai/reference_parser/commit_parser.rb index 8c54a041cb8..30dc87248b4 100644 --- a/lib/banzai/reference_parser/commit_parser.rb +++ b/lib/banzai/reference_parser/commit_parser.rb @@ -32,7 +32,7 @@ module Banzai private - def can_read_reference?(user, ref_project) + def can_read_reference?(user, ref_project, node) can?(user, :download_code, ref_project) end end diff --git a/lib/banzai/reference_parser/commit_range_parser.rb b/lib/banzai/reference_parser/commit_range_parser.rb index 0878b6afba3..a50e6f8ef8f 100644 --- a/lib/banzai/reference_parser/commit_range_parser.rb +++ b/lib/banzai/reference_parser/commit_range_parser.rb @@ -36,7 +36,7 @@ module Banzai private - def can_read_reference?(user, ref_project) + def can_read_reference?(user, ref_project, node) can?(user, :download_code, ref_project) end end diff --git a/lib/banzai/reference_parser/external_issue_parser.rb b/lib/banzai/reference_parser/external_issue_parser.rb index 6e7b7669578..6307c1b571a 100644 --- a/lib/banzai/reference_parser/external_issue_parser.rb +++ b/lib/banzai/reference_parser/external_issue_parser.rb @@ -23,7 +23,7 @@ module Banzai private - def can_read_reference?(user, ref_project) + def can_read_reference?(user, ref_project, node) can?(user, :read_issue, ref_project) end end diff --git a/lib/banzai/reference_parser/label_parser.rb b/lib/banzai/reference_parser/label_parser.rb index aa76c64ac5f..30e2a012f09 100644 --- a/lib/banzai/reference_parser/label_parser.rb +++ b/lib/banzai/reference_parser/label_parser.rb @@ -9,7 +9,7 @@ module Banzai private - def can_read_reference?(user, ref_project) + def can_read_reference?(user, ref_project, node) can?(user, :read_label, ref_project) end end diff --git a/lib/banzai/reference_parser/merge_request_parser.rb b/lib/banzai/reference_parser/merge_request_parser.rb index 8b0662749fd..75cbc7fdac4 100644 --- a/lib/banzai/reference_parser/merge_request_parser.rb +++ b/lib/banzai/reference_parser/merge_request_parser.rb @@ -40,6 +40,10 @@ module Banzai self.class.data_attribute ) end + + def can_read_reference?(user, ref_project, node) + can?(user, :read_merge_request, ref_project) + end end end end diff --git a/lib/banzai/reference_parser/milestone_parser.rb b/lib/banzai/reference_parser/milestone_parser.rb index d3968d6b229..68675abe22a 100644 --- a/lib/banzai/reference_parser/milestone_parser.rb +++ b/lib/banzai/reference_parser/milestone_parser.rb @@ -9,7 +9,7 @@ module Banzai private - def can_read_reference?(user, ref_project) + def can_read_reference?(user, ref_project, node) can?(user, :read_milestone, ref_project) end end diff --git a/lib/banzai/reference_parser/snippet_parser.rb b/lib/banzai/reference_parser/snippet_parser.rb index 63b592137bb..3ade168b566 100644 --- a/lib/banzai/reference_parser/snippet_parser.rb +++ b/lib/banzai/reference_parser/snippet_parser.rb @@ -9,8 +9,8 @@ module Banzai private - def can_read_reference?(user, ref_project) - can?(user, :read_project_snippet, ref_project) + def can_read_reference?(user, ref_project, node) + can?(user, :read_project_snippet, referenced_by([node]).first) end end end diff --git a/lib/banzai/reference_parser/user_parser.rb b/lib/banzai/reference_parser/user_parser.rb index 09b66cbd8fb..3efbd2fd631 100644 --- a/lib/banzai/reference_parser/user_parser.rb +++ b/lib/banzai/reference_parser/user_parser.rb @@ -103,7 +103,7 @@ module Banzai flat_map { |p| p.team.members.to_a } end - def can_read_reference?(user, ref_project) + def can_read_reference?(user, ref_project, node) can?(user, :read_project, ref_project) end end diff --git a/lib/gitlab/path_regex.rb b/lib/gitlab/path_regex.rb index 9ff6829cd49..10eb99fb461 100644 --- a/lib/gitlab/path_regex.rb +++ b/lib/gitlab/path_regex.rb @@ -49,6 +49,7 @@ module Gitlab sent_notifications services snippets + system teams u unicorn_test diff --git a/lib/gitlab/uploads_transfer.rb b/lib/gitlab/uploads_transfer.rb index 7d0c47c5361..b5f41240529 100644 --- a/lib/gitlab/uploads_transfer.rb +++ b/lib/gitlab/uploads_transfer.rb @@ -1,7 +1,7 @@ module Gitlab class UploadsTransfer < ProjectTransfer def root_dir - File.join(CarrierWave.root, GitlabUploader.base_dir) + File.join(CarrierWave.root, FileUploader.base_dir) end end end diff --git a/spec/controllers/autocomplete_controller_spec.rb b/spec/controllers/autocomplete_controller_spec.rb index 2c9d1ffc9c2..4c3a5ec49ef 100644 --- a/spec/controllers/autocomplete_controller_spec.rb +++ b/spec/controllers/autocomplete_controller_spec.rb @@ -170,22 +170,32 @@ describe AutocompleteController do end context 'author of issuable included' do - before do - sign_in(user) - end - let(:body) { JSON.parse(response.body) } - it 'includes the author' do - get(:users, author_id: non_member.id) + context 'authenticated' do + before do + sign_in(user) + end + + it 'includes the author' do + get(:users, author_id: non_member.id) + + expect(body.first["username"]).to eq non_member.username + end + + it 'rejects non existent user ids' do + get(:users, author_id: 99999) - expect(body.first["username"]).to eq non_member.username + expect(body.collect { |u| u['id'] }).not_to include(99999) + end end - it 'rejects non existent user ids' do - get(:users, author_id: 99999) + context 'without authenticating' do + it 'returns empty result' do + get(:users, author_id: non_member.id) - expect(body.collect { |u| u['id'] }).not_to include(99999) + expect(body).to be_empty + end end end diff --git a/spec/controllers/groups/group_members_controller_spec.rb b/spec/controllers/groups/group_members_controller_spec.rb index 60db0192dfd..ed4ad7b600e 100644 --- a/spec/controllers/groups/group_members_controller_spec.rb +++ b/spec/controllers/groups/group_members_controller_spec.rb @@ -124,6 +124,13 @@ describe Groups::GroupMembersController do expect(response).to redirect_to(dashboard_groups_path) expect(group.users).not_to include user end + + it 'supports json request' do + delete :leave, group_id: group, format: :json + + expect(response).to have_http_status(200) + expect(json_response['notice']).to eq "You left the \"#{group.name}\" group." + end end context 'and is an owner' do diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index 08024a2148b..0a64fe2beda 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -121,13 +121,18 @@ describe Projects::MergeRequestsController do context 'number of queries' do it 'verifies number of queries' do + RequestStore.begin! + # pre-create objects merge_request recorded = ActiveRecord::QueryRecorder.new { go(format: :json) } - expect(recorded.count).to be_within(5).of(50) + expect(recorded.count).to be_within(5).of(30) expect(recorded.cached_count).to eq(0) + + RequestStore.end! + RequestStore.clear! end end end diff --git a/spec/factories/uploads.rb b/spec/factories/uploads.rb new file mode 100644 index 00000000000..1383420fb44 --- /dev/null +++ b/spec/factories/uploads.rb @@ -0,0 +1,8 @@ +FactoryGirl.define do + factory :upload do + model { build(:project) } + path { "uploads/system/project/avatar/avatar.jpg" } + size 100.kilobytes + uploader "AvatarUploader" + end +end diff --git a/spec/features/admin/admin_appearance_spec.rb b/spec/features/admin/admin_appearance_spec.rb index 96d715ef383..595366ce352 100644 --- a/spec/features/admin/admin_appearance_spec.rb +++ b/spec/features/admin/admin_appearance_spec.rb @@ -63,11 +63,11 @@ feature 'Admin Appearance', feature: true do end def logo_selector - '//img[@src^="/uploads/appearance/logo"]' + '//img[@src^="/uploads/system/appearance/logo"]' end def header_logo_selector - '//img[@src^="/uploads/appearance/header_logo"]' + '//img[@src^="/uploads/system/appearance/header_logo"]' end def logo_fixture diff --git a/spec/features/dashboard/groups_list_spec.rb b/spec/features/dashboard/groups_list_spec.rb index b0e2953dda2..7eb254f8451 100644 --- a/spec/features/dashboard/groups_list_spec.rb +++ b/spec/features/dashboard/groups_list_spec.rb @@ -6,40 +6,124 @@ describe 'Dashboard Groups page', js: true, feature: true do let!(:nested_group) { create(:group, :nested) } let!(:another_group) { create(:group) } - before do + it 'shows groups user is member of' do group.add_owner(user) nested_group.add_owner(user) login_as(user) - visit dashboard_groups_path - end - it 'shows groups user is member of' do expect(page).to have_content(group.full_name) expect(page).to have_content(nested_group.full_name) expect(page).not_to have_content(another_group.full_name) end - it 'filters groups' do - fill_in 'filter_groups', with: group.name - wait_for_requests + describe 'when filtering groups' do + before do + group.add_owner(user) + nested_group.add_owner(user) - expect(page).to have_content(group.full_name) - expect(page).not_to have_content(nested_group.full_name) - expect(page).not_to have_content(another_group.full_name) + login_as(user) + + visit dashboard_groups_path + end + + it 'filters groups' do + fill_in 'filter_groups', with: group.name + wait_for_requests + + expect(page).to have_content(group.full_name) + expect(page).not_to have_content(nested_group.full_name) + expect(page).not_to have_content(another_group.full_name) + end + + it 'resets search when user cleans the input' do + fill_in 'filter_groups', with: group.name + wait_for_requests + + fill_in 'filter_groups', with: "" + wait_for_requests + + expect(page).to have_content(group.full_name) + expect(page).to have_content(nested_group.full_name) + expect(page).not_to have_content(another_group.full_name) + expect(page.all('.js-groups-list-holder .content-list li').length).to eq 2 + end end - it 'resets search when user cleans the input' do - fill_in 'filter_groups', with: group.name - wait_for_requests + describe 'group with subgroups' do + let!(:subgroup) { create(:group, :public, parent: group) } - fill_in 'filter_groups', with: "" - wait_for_requests + before do + group.add_owner(user) + subgroup.add_owner(user) - expect(page).to have_content(group.full_name) - expect(page).to have_content(nested_group.full_name) - expect(page).not_to have_content(another_group.full_name) - expect(page.all('.js-groups-list-holder .content-list li').length).to eq 2 + login_as(user) + + visit dashboard_groups_path + end + + it 'shows subgroups inside of its parent group' do + expect(page).to have_selector('.groups-list-tree-container .group-list-tree', count: 2) + expect(page).to have_selector(".groups-list-tree-container #group-#{group.id} #group-#{subgroup.id}", count: 1) + end + + it 'can toggle parent group' do + # Expanded by default + expect(page).to have_selector("#group-#{group.id} .fa-caret-down", count: 1) + expect(page).not_to have_selector("#group-#{group.id} .fa-caret-right") + + # Collapse + find("#group-#{group.id}").trigger('click') + + expect(page).not_to have_selector("#group-#{group.id} .fa-caret-down") + expect(page).to have_selector("#group-#{group.id} .fa-caret-right", count: 1) + expect(page).not_to have_selector("#group-#{group.id} #group-#{subgroup.id}") + + # Expand + find("#group-#{group.id}").trigger('click') + + expect(page).to have_selector("#group-#{group.id} .fa-caret-down", count: 1) + expect(page).not_to have_selector("#group-#{group.id} .fa-caret-right") + expect(page).to have_selector("#group-#{group.id} #group-#{subgroup.id}") + end + end + + describe 'when using pagination' do + let(:group2) { create(:group) } + + before do + group.add_owner(user) + group2.add_owner(user) + + allow(Kaminari.config).to receive(:default_per_page).and_return(1) + + login_as(user) + visit dashboard_groups_path + end + + it 'shows pagination' do + expect(page).to have_selector('.gl-pagination') + expect(page).to have_selector('.gl-pagination .page', count: 2) + end + + it 'loads results for next page' do + # Check first page + expect(page).to have_content(group2.full_name) + expect(page).to have_selector("#group-#{group2.id}") + expect(page).not_to have_content(group.full_name) + expect(page).not_to have_selector("#group-#{group.id}") + + # Go to next page + find(".gl-pagination .page:not(.active) a").trigger('click') + + wait_for_requests + + # Check second page + expect(page).to have_content(group.full_name) + expect(page).to have_selector("#group-#{group.id}") + expect(page).not_to have_content(group2.full_name) + expect(page).not_to have_selector("#group-#{group2.id}") + end end end diff --git a/spec/features/projects/group_links_spec.rb b/spec/features/projects/group_links_spec.rb index 4e5682c8636..1b680a56492 100644 --- a/spec/features/projects/group_links_spec.rb +++ b/spec/features/projects/group_links_spec.rb @@ -16,15 +16,17 @@ feature 'Project group links', :feature, :js do before do visit namespace_project_settings_members_path(project.namespace, project) + click_on 'share-with-group-tab' + select2 group.id, from: '#link_group_id' fill_in 'expires_at_groups', with: (Time.current + 4.5.days).strftime('%Y-%m-%d') page.find('body').click - click_on 'Share' + find('.btn-create').trigger('click') end it 'shows the expiration time with a warning class' do - page.within('.enabled-groups') do - expect(page).to have_content('expires in 4 days') + page.within('.project-members-groups') do + expect(page).to have_content('Expires in 4 days') expect(page).to have_selector('.text-warning') end end @@ -43,6 +45,7 @@ feature 'Project group links', :feature, :js do it 'does not show ancestors', :nested_groups do visit namespace_project_settings_members_path(project.namespace, project) + click_on 'share-with-group-tab' click_link 'Search for a group' page.within '.select2-drop' do diff --git a/spec/features/uploads/user_uploads_avatar_to_group_spec.rb b/spec/features/uploads/user_uploads_avatar_to_group_spec.rb index f88a515f7fc..d9d6f2e2382 100644 --- a/spec/features/uploads/user_uploads_avatar_to_group_spec.rb +++ b/spec/features/uploads/user_uploads_avatar_to_group_spec.rb @@ -18,7 +18,7 @@ feature 'User uploads avatar to group', feature: true do visit group_path(group) - expect(page).to have_selector(%Q(img[src$="/uploads/group/avatar/#{group.id}/dk.png"])) + expect(page).to have_selector(%Q(img[src$="/uploads/system/group/avatar/#{group.id}/dk.png"])) # Cheating here to verify something that isn't user-facing, but is important expect(group.reload.avatar.file).to exist diff --git a/spec/features/uploads/user_uploads_avatar_to_profile_spec.rb b/spec/features/uploads/user_uploads_avatar_to_profile_spec.rb index 0dfd29045e5..eb8dbd76aab 100644 --- a/spec/features/uploads/user_uploads_avatar_to_profile_spec.rb +++ b/spec/features/uploads/user_uploads_avatar_to_profile_spec.rb @@ -16,7 +16,7 @@ feature 'User uploads avatar to profile', feature: true do visit user_path(user) - expect(page).to have_selector(%Q(img[src$="/uploads/user/avatar/#{user.id}/dk.png"])) + expect(page).to have_selector(%Q(img[src$="/uploads/system/user/avatar/#{user.id}/dk.png"])) # Cheating here to verify something that isn't user-facing, but is important expect(user.reload.avatar.file).to exist diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index 785fb724132..49df91b236f 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -1,3 +1,4 @@ +# coding: utf-8 require 'spec_helper' describe ApplicationHelper do @@ -58,13 +59,13 @@ describe ApplicationHelper do describe 'project_icon' do it 'returns an url for the avatar' do project = create(:empty_project, avatar: File.open(uploaded_image_temp_path)) - avatar_url = "/uploads/project/avatar/#{project.id}/banana_sample.gif" + avatar_url = "/uploads/system/project/avatar/#{project.id}/banana_sample.gif" expect(helper.project_icon(project.full_path).to_s). to eq "<img src=\"#{avatar_url}\" alt=\"Banana sample\" />" allow(ActionController::Base).to receive(:asset_host).and_return(gitlab_host) - avatar_url = "#{gitlab_host}/uploads/project/avatar/#{project.id}/banana_sample.gif" + avatar_url = "#{gitlab_host}/uploads/system/project/avatar/#{project.id}/banana_sample.gif" expect(helper.project_icon(project.full_path).to_s). to eq "<img src=\"#{avatar_url}\" alt=\"Banana sample\" />" @@ -84,12 +85,12 @@ describe ApplicationHelper do it 'returns an url for the avatar' do user = create(:user, avatar: File.open(uploaded_image_temp_path)) - avatar_url = "/uploads/user/avatar/#{user.id}/banana_sample.gif" + avatar_url = "/uploads/system/user/avatar/#{user.id}/banana_sample.gif" expect(helper.avatar_icon(user.email).to_s).to match(avatar_url) allow(ActionController::Base).to receive(:asset_host).and_return(gitlab_host) - avatar_url = "#{gitlab_host}/uploads/user/avatar/#{user.id}/banana_sample.gif" + avatar_url = "#{gitlab_host}/uploads/system/user/avatar/#{user.id}/banana_sample.gif" expect(helper.avatar_icon(user.email).to_s).to match(avatar_url) end @@ -102,7 +103,7 @@ describe ApplicationHelper do user = create(:user, avatar: File.open(uploaded_image_temp_path)) expect(helper.avatar_icon(user.email).to_s). - to match("/gitlab/uploads/user/avatar/#{user.id}/banana_sample.gif") + to match("/gitlab/uploads/system/user/avatar/#{user.id}/banana_sample.gif") end it 'calls gravatar_icon when no User exists with the given email' do @@ -116,7 +117,7 @@ describe ApplicationHelper do user = create(:user, avatar: File.open(uploaded_image_temp_path)) expect(helper.avatar_icon(user).to_s). - to match("/uploads/user/avatar/#{user.id}/banana_sample.gif") + to match("/uploads/system/user/avatar/#{user.id}/banana_sample.gif") end end end diff --git a/spec/helpers/emails_helper_spec.rb b/spec/helpers/emails_helper_spec.rb index cd112dbb2fb..c68e4f56b05 100644 --- a/spec/helpers/emails_helper_spec.rb +++ b/spec/helpers/emails_helper_spec.rb @@ -52,7 +52,7 @@ describe EmailsHelper do ) expect(header_logo).to eq( - %{<img style="height: 50px" src="/uploads/appearance/header_logo/#{appearance.id}/dk.png" alt="Dk" />} + %{<img style="height: 50px" src="/uploads/system/appearance/header_logo/#{appearance.id}/dk.png" alt="Dk" />} ) end end diff --git a/spec/helpers/groups_helper_spec.rb b/spec/helpers/groups_helper_spec.rb index c8b0d86425f..0337afa4452 100644 --- a/spec/helpers/groups_helper_spec.rb +++ b/spec/helpers/groups_helper_spec.rb @@ -9,7 +9,7 @@ describe GroupsHelper do group.avatar = fixture_file_upload(avatar_file_path) group.save! expect(group_icon(group.path).to_s). - to match("/uploads/group/avatar/#{group.id}/banana_sample.gif") + to match("/uploads/system/group/avatar/#{group.id}/banana_sample.gif") end it 'gives default avatar_icon when no avatar is present' do diff --git a/spec/helpers/page_layout_helper_spec.rb b/spec/helpers/page_layout_helper_spec.rb index 2cc0b40b2d0..dff2784f21f 100644 --- a/spec/helpers/page_layout_helper_spec.rb +++ b/spec/helpers/page_layout_helper_spec.rb @@ -60,7 +60,7 @@ describe PageLayoutHelper do %w(project user group).each do |type| context "with @#{type} assigned" do it "uses #{type.titlecase} avatar if available" do - object = double(avatar_url: 'http://example.com/uploads/avatar.png') + object = double(avatar_url: 'http://example.com/uploads/system/avatar.png') assign(type, object) expect(helper.page_image).to eq object.avatar_url diff --git a/spec/javascripts/gl_dropdown_spec.js b/spec/javascripts/gl_dropdown_spec.js index 3292590b9ed..10fcc590c89 100644 --- a/spec/javascripts/gl_dropdown_spec.js +++ b/spec/javascripts/gl_dropdown_spec.js @@ -185,7 +185,7 @@ import '~/lib/utils/url_utility'; expect($(document.activeElement)).toEqual($(SEARCH_INPUT_SELECTOR)); }); - it('should focus on input when opening for the second time', () => { + it('should focus on input when opening for the second time after transition', () => { remoteCallback(); this.dropdownContainerElement.trigger({ type: 'keyup', @@ -193,6 +193,7 @@ import '~/lib/utils/url_utility'; keyCode: ARROW_KEYS.ESC }); this.dropdownButtonElement.click(); + this.dropdownContainerElement.trigger('transitionend'); expect($(document.activeElement)).toEqual($(SEARCH_INPUT_SELECTOR)); }); }); @@ -201,6 +202,7 @@ import '~/lib/utils/url_utility'; it('should focus input when passing array data to drop down', () => { initDropDown.call(this, false, true); this.dropdownButtonElement.click(); + this.dropdownContainerElement.trigger('transitionend'); expect($(document.activeElement)).toEqual($(SEARCH_INPUT_SELECTOR)); }); }); diff --git a/spec/javascripts/groups/group_item_spec.js b/spec/javascripts/groups/group_item_spec.js new file mode 100644 index 00000000000..25e10552d95 --- /dev/null +++ b/spec/javascripts/groups/group_item_spec.js @@ -0,0 +1,102 @@ +import Vue from 'vue'; +import groupItemComponent from '~/groups/components/group_item.vue'; +import GroupsStore from '~/groups/stores/groups_store'; +import { group1 } from './mock_data'; + +describe('Groups Component', () => { + let GroupItemComponent; + let component; + let store; + let group; + + describe('group with default data', () => { + beforeEach((done) => { + GroupItemComponent = Vue.extend(groupItemComponent); + store = new GroupsStore(); + group = store.decorateGroup(group1); + + component = new GroupItemComponent({ + propsData: { + group, + }, + }).$mount(); + + Vue.nextTick(() => { + done(); + }); + }); + + afterEach(() => { + component.$destroy(); + }); + + it('should render the group item correctly', () => { + expect(component.$el.classList.contains('group-row')).toBe(true); + expect(component.$el.classList.contains('.no-description')).toBe(false); + expect(component.$el.querySelector('.number-projects').textContent).toContain(group.numberProjects); + expect(component.$el.querySelector('.number-users').textContent).toContain(group.numberUsers); + expect(component.$el.querySelector('.group-visibility')).toBeDefined(); + expect(component.$el.querySelector('.avatar-container')).toBeDefined(); + expect(component.$el.querySelector('.title').textContent).toContain(group.name); + expect(component.$el.querySelector('.access-type').textContent).toContain(group.permissions.humanGroupAccess); + expect(component.$el.querySelector('.description').textContent).toContain(group.description); + expect(component.$el.querySelector('.edit-group')).toBeDefined(); + expect(component.$el.querySelector('.leave-group')).toBeDefined(); + }); + }); + + describe('group without description', () => { + beforeEach((done) => { + GroupItemComponent = Vue.extend(groupItemComponent); + store = new GroupsStore(); + group1.description = ''; + group = store.decorateGroup(group1); + + component = new GroupItemComponent({ + propsData: { + group, + }, + }).$mount(); + + Vue.nextTick(() => { + done(); + }); + }); + + afterEach(() => { + component.$destroy(); + }); + + it('should render group item correctly', () => { + expect(component.$el.querySelector('.description').textContent).toBe(''); + expect(component.$el.classList.contains('.no-description')).toBe(false); + }); + }); + + describe('user has not access to group', () => { + beforeEach((done) => { + GroupItemComponent = Vue.extend(groupItemComponent); + store = new GroupsStore(); + group1.permissions.human_group_access = null; + group = store.decorateGroup(group1); + + component = new GroupItemComponent({ + propsData: { + group, + }, + }).$mount(); + + Vue.nextTick(() => { + done(); + }); + }); + + afterEach(() => { + component.$destroy(); + }); + + it('should not display access type', () => { + expect(component.$el.querySelector('.access-type')).toBeNull(); + }); + }); +}); diff --git a/spec/javascripts/groups/groups_spec.js b/spec/javascripts/groups/groups_spec.js new file mode 100644 index 00000000000..2a77f7259da --- /dev/null +++ b/spec/javascripts/groups/groups_spec.js @@ -0,0 +1,64 @@ +import Vue from 'vue'; +import groupFolderComponent from '~/groups/components/group_folder.vue'; +import groupItemComponent from '~/groups/components/group_item.vue'; +import groupsComponent from '~/groups/components/groups.vue'; +import GroupsStore from '~/groups/stores/groups_store'; +import { groupsData } from './mock_data'; + +describe('Groups Component', () => { + let GroupsComponent; + let store; + let component; + let groups; + + beforeEach((done) => { + Vue.component('group-folder', groupFolderComponent); + Vue.component('group-item', groupItemComponent); + + store = new GroupsStore(); + groups = store.setGroups(groupsData.groups); + + store.storePagination(groupsData.pagination); + + GroupsComponent = Vue.extend(groupsComponent); + + component = new GroupsComponent({ + propsData: { + groups: store.state.groups, + pageInfo: store.state.pageInfo, + }, + }).$mount(); + + Vue.nextTick(() => { + done(); + }); + }); + + afterEach(() => { + component.$destroy(); + }); + + describe('with data', () => { + it('should render a list of groups', () => { + expect(component.$el.classList.contains('groups-list-tree-container')).toBe(true); + expect(component.$el.querySelector('#group-12')).toBeDefined(); + expect(component.$el.querySelector('#group-1119')).toBeDefined(); + expect(component.$el.querySelector('#group-1120')).toBeDefined(); + }); + + it('should render group and its subgroup', () => { + const lists = component.$el.querySelectorAll('.group-list-tree'); + + expect(lists.length).toBe(3); // one parent and two subgroups + + expect(lists[0].querySelector('#group-1119').classList.contains('is-open')).toBe(true); + expect(lists[0].querySelector('#group-1119').classList.contains('has-subgroups')).toBe(true); + + expect(lists[2].querySelector('#group-1120').textContent).toContain(groups[1119].subGroups[1120].name); + }); + + it('should remove prefix of parent group', () => { + expect(component.$el.querySelector('#group-12 #group-1128 .title').textContent).toContain('level2 / level3 / level4'); + }); + }); +}); diff --git a/spec/javascripts/groups/mock_data.js b/spec/javascripts/groups/mock_data.js new file mode 100644 index 00000000000..1c0ec7a97d0 --- /dev/null +++ b/spec/javascripts/groups/mock_data.js @@ -0,0 +1,110 @@ +const group1 = { + id: '12', + name: 'level1', + path: 'level1', + description: 'foo', + visibility: 'public', + avatar_url: null, + web_url: 'http://localhost:3000/groups/level1', + full_name: 'level1', + full_path: 'level1', + parent_id: null, + created_at: '2017-05-15T19:01:23.670Z', + updated_at: '2017-05-15T19:01:23.670Z', + number_projects_with_delimiter: '1', + number_users_with_delimiter: '1', + has_subgroups: true, + permissions: { + human_group_access: 'Master', + }, +}; + +// This group has no direct parent, should be placed as subgroup of group1 +const group14 = { + id: 1128, + name: 'level4', + path: 'level4', + description: 'foo', + visibility: 'public', + avatar_url: null, + web_url: 'http://localhost:3000/groups/level1/level2/level3/level4', + full_name: 'level1 / level2 / level3 / level4', + full_path: 'level1/level2/level3/level4', + parent_id: 1127, + created_at: '2017-05-15T19:02:01.645Z', + updated_at: '2017-05-15T19:02:01.645Z', + number_projects_with_delimiter: '1', + number_users_with_delimiter: '1', + has_subgroups: true, + permissions: { + human_group_access: 'Master', + }, +}; + +const group2 = { + id: 1119, + name: 'devops', + path: 'devops', + description: 'foo', + visibility: 'public', + avatar_url: null, + web_url: 'http://localhost:3000/groups/devops', + full_name: 'devops', + full_path: 'devops', + parent_id: null, + created_at: '2017-05-11T19:35:09.635Z', + updated_at: '2017-05-11T19:35:09.635Z', + number_projects_with_delimiter: '1', + number_users_with_delimiter: '1', + has_subgroups: true, + permissions: { + human_group_access: 'Master', + }, +}; + +const group21 = { + id: 1120, + name: 'chef', + path: 'chef', + description: 'foo', + visibility: 'public', + avatar_url: null, + web_url: 'http://localhost:3000/groups/devops/chef', + full_name: 'devops / chef', + full_path: 'devops/chef', + parent_id: 1119, + created_at: '2017-05-11T19:51:04.060Z', + updated_at: '2017-05-11T19:51:04.060Z', + number_projects_with_delimiter: '1', + number_users_with_delimiter: '1', + has_subgroups: true, + permissions: { + human_group_access: 'Master', + }, +}; + +const groupsData = { + groups: [group1, group14, group2, group21], + pagination: { + Date: 'Mon, 22 May 2017 22:31:52 GMT', + 'X-Prev-Page': '1', + 'X-Content-Type-Options': 'nosniff', + 'X-Total': '31', + 'Transfer-Encoding': 'chunked', + 'X-Runtime': '0.611144', + 'X-Xss-Protection': '1; mode=block', + 'X-Request-Id': 'f5db8368-3ce5-4aa4-89d2-a125d9dead09', + 'X-Ua-Compatible': 'IE=edge', + 'X-Per-Page': '20', + Link: '<http://localhost:3000/dashboard/groups.json?page=1&per_page=20>; rel="prev", <http://localhost:3000/dashboard/groups.json?page=1&per_page=20>; rel="first", <http://localhost:3000/dashboard/groups.json?page=2&per_page=20>; rel="last"', + 'X-Next-Page': '', + Etag: 'W/"a82f846947136271cdb7d55d19ef33d2"', + 'X-Frame-Options': 'DENY', + 'Content-Type': 'application/json; charset=utf-8', + 'Cache-Control': 'max-age=0, private, must-revalidate', + 'X-Total-Pages': '2', + 'X-Page': '2', + }, +}; + +export { groupsData, group1 }; diff --git a/spec/javascripts/lib/utils/common_utils_spec.js b/spec/javascripts/lib/utils/common_utils_spec.js index e3938a77680..52cf217c25f 100644 --- a/spec/javascripts/lib/utils/common_utils_spec.js +++ b/spec/javascripts/lib/utils/common_utils_spec.js @@ -150,6 +150,14 @@ import '~/lib/utils/common_utils'; const value = gl.utils.getParameterByName('fakeParameter'); expect(value).toBe(null); }); + + it('should return valid paramentes if URL is provided', () => { + let value = gl.utils.getParameterByName('foo', 'http://cocteau.twins/?foo=bar'); + expect(value).toBe('bar'); + + value = gl.utils.getParameterByName('manan', 'http://cocteau.twins/?foo=bar&manan=canchu'); + expect(value).toBe('canchu'); + }); }); describe('gl.utils.normalizedHeaders', () => { diff --git a/spec/javascripts/notes_spec.js b/spec/javascripts/notes_spec.js index 24335614e09..bfd8b8648a6 100644 --- a/spec/javascripts/notes_spec.js +++ b/spec/javascripts/notes_spec.js @@ -461,6 +461,45 @@ import '~/notes'; }); }); + describe('update comment with script tags', () => { + const sampleComment = '<script></script>'; + const updatedComment = '<script></script>'; + const note = { + id: 1234, + html: `<li class="note note-row-1234 timeline-entry" id="note_1234"> + <div class="note-text">${sampleComment}</div> + </li>`, + note: sampleComment, + valid: true + }; + let $form; + let $notesContainer; + + beforeEach(() => { + this.notes = new Notes('', []); + window.gon.current_username = 'root'; + window.gon.current_user_fullname = 'Administrator'; + $form = $('form.js-main-target-form'); + $notesContainer = $('ul.main-notes-list'); + $form.find('textarea.js-note-text').html(sampleComment); + }); + + it('should not render a script tag', () => { + const deferred = $.Deferred(); + spyOn($, 'ajax').and.returnValue(deferred.promise()); + $('.js-comment-button').click(); + + deferred.resolve(note); + const $noteEl = $notesContainer.find(`#note_${note.id}`); + $noteEl.find('.js-note-edit').click(); + $noteEl.find('textarea.js-note-text').html(updatedComment); + $noteEl.find('.js-comment-save-button').click(); + + const $updatedNoteEl = $notesContainer.find(`#note_${note.id}`).find('.js-task-list-container'); + expect($updatedNoteEl.find('.note-text').text().trim()).toEqual(''); + }); + }); + describe('getFormData', () => { let $form; let sampleComment; diff --git a/spec/javascripts/test_bundle.js b/spec/javascripts/test_bundle.js index 13827a26571..2c34402576b 100644 --- a/spec/javascripts/test_bundle.js +++ b/spec/javascripts/test_bundle.js @@ -51,7 +51,6 @@ if (process.env.BABEL_ENV === 'coverage') { './environments/environments_bundle.js', './filtered_search/filtered_search_bundle.js', './graphs/graphs_bundle.js', - './issuable/issuable_bundle.js', './issuable/time_tracking/time_tracking_bundle.js', './main.js', './merge_conflicts/merge_conflicts_bundle.js', diff --git a/spec/javascripts/vue_shared/components/commit_spec.js b/spec/javascripts/vue_shared/components/commit_spec.js index 050170a54e9..540245fe71e 100644 --- a/spec/javascripts/vue_shared/components/commit_spec.js +++ b/spec/javascripts/vue_shared/components/commit_spec.js @@ -22,7 +22,7 @@ describe('Commit component', () => { shortSha: 'b7836edd', title: 'Commit message', author: { - avatar_url: 'https://gitlab.com/uploads/user/avatar/300478/avatar.png', + avatar_url: 'https://gitlab.com/uploads/system/user/avatar/300478/avatar.png', web_url: 'https://gitlab.com/jschatz1', path: '/jschatz1', username: 'jschatz1', @@ -45,7 +45,7 @@ describe('Commit component', () => { shortSha: 'b7836edd', title: 'Commit message', author: { - avatar_url: 'https://gitlab.com/uploads/user/avatar/300478/avatar.png', + avatar_url: 'https://gitlab.com/uploads/system/user/avatar/300478/avatar.png', web_url: 'https://gitlab.com/jschatz1', path: '/jschatz1', username: 'jschatz1', diff --git a/spec/lib/banzai/reference_parser/base_parser_spec.rb b/spec/lib/banzai/reference_parser/base_parser_spec.rb index d5746107ee1..f4f42bfc3ed 100644 --- a/spec/lib/banzai/reference_parser/base_parser_spec.rb +++ b/spec/lib/banzai/reference_parser/base_parser_spec.rb @@ -30,7 +30,7 @@ describe Banzai::ReferenceParser::BaseParser, lib: true do it 'checks if user can read the resource' do link['data-project'] = project.id.to_s - expect(subject).to receive(:can_read_reference?).with(user, project) + expect(subject).to receive(:can_read_reference?).with(user, project, link) subject.nodes_visible_to_user(user, [link]) end diff --git a/spec/lib/banzai/reference_parser/snippet_parser_spec.rb b/spec/lib/banzai/reference_parser/snippet_parser_spec.rb index d217a775802..620875ece20 100644 --- a/spec/lib/banzai/reference_parser/snippet_parser_spec.rb +++ b/spec/lib/banzai/reference_parser/snippet_parser_spec.rb @@ -4,20 +4,199 @@ describe Banzai::ReferenceParser::SnippetParser, lib: true do include ReferenceParserHelpers let(:project) { create(:empty_project, :public) } + let(:user) { create(:user) } - let(:snippet) { create(:snippet, project: project) } + let(:external_user) { create(:user, :external) } + let(:project_member) { create(:user) } + subject { described_class.new(project, user) } let(:link) { empty_html_link } + def visible_references(snippet_visibility, user = nil) + snippet = create(:project_snippet, snippet_visibility, project: project) + link['data-project'] = project.id.to_s + link['data-snippet'] = snippet.id.to_s + + subject.nodes_visible_to_user(user, [link]) + end + + before do + project.add_user(project_member, :developer) + end + describe '#nodes_visible_to_user' do - context 'when the link has a data-issue attribute' do - before { link['data-snippet'] = snippet.id.to_s } + context 'when a project is public and the snippets feature is enabled for everyone' do + before do + project.project_feature.update_attribute(:snippets_access_level, ProjectFeature::ENABLED) + end + + it 'creates a reference for guest for a public snippet' do + expect(visible_references(:public)).to eq([link]) + end + + it 'creates a reference for a regular user for a public snippet' do + expect(visible_references(:public, user)).to eq([link]) + end + + it 'creates a reference for a regular user for an internal snippet' do + expect(visible_references(:internal, user)).to eq([link]) + end + + it 'does not create a reference for an external user for an internal snippet' do + expect(visible_references(:internal, external_user)).to be_empty + end + + it 'creates a reference for a project member for a private snippet' do + expect(visible_references(:private, project_member)).to eq([link]) + end + + it 'does not create a reference for a regular user for a private snippet' do + expect(visible_references(:private, user)).to be_empty + end + end + + context 'when a project is public and the snippets feature is enabled for project team members' do + before do + project.project_feature.update_attribute(:snippets_access_level, ProjectFeature::PRIVATE) + end + + it 'creates a reference for a project member for a public snippet' do + expect(visible_references(:public, project_member)).to eq([link]) + end + + it 'does not create a reference for guest for a public snippet' do + expect(visible_references(:public, nil)).to be_empty + end + + it 'does not create a reference for a regular user for a public snippet' do + expect(visible_references(:public, user)).to be_empty + end + + it 'creates a reference for a project member for an internal snippet' do + expect(visible_references(:internal, project_member)).to eq([link]) + end + + it 'does not create a reference for a regular user for an internal snippet' do + expect(visible_references(:internal, user)).to be_empty + end + + it 'creates a reference for a project member for a private snippet' do + expect(visible_references(:private, project_member)).to eq([link]) + end + + it 'does not create a reference for a regular user for a private snippet' do + expect(visible_references(:private, user)).to be_empty + end + end + + context 'when a project is internal and the snippets feature is enabled for everyone' do + before do + project.update_attribute(:visibility, Gitlab::VisibilityLevel::INTERNAL) + project.project_feature.update_attribute(:snippets_access_level, ProjectFeature::ENABLED) + end + + it 'does not create a reference for guest for a public snippet' do + expect(visible_references(:public)).to be_empty + end + + it 'does not create a reference for an external user for a public snippet' do + expect(visible_references(:public, external_user)).to be_empty + end - it_behaves_like "referenced feature visibility", "snippets" + it 'creates a reference for a regular user for a public snippet' do + expect(visible_references(:public, user)).to eq([link]) + end + + it 'creates a reference for a regular user for an internal snippet' do + expect(visible_references(:internal, user)).to eq([link]) + end + + it 'does not create a reference for an external user for an internal snippet' do + expect(visible_references(:internal, external_user)).to be_empty + end + + it 'creates a reference for a project member for a private snippet' do + expect(visible_references(:private, project_member)).to eq([link]) + end + + it 'does not create a reference for a regular user for a private snippet' do + expect(visible_references(:private, user)).to be_empty + end + end + + context 'when a project is internal and the snippets feature is enabled for project team members' do + before do + project.update_attribute(:visibility, Gitlab::VisibilityLevel::INTERNAL) + project.project_feature.update_attribute(:snippets_access_level, ProjectFeature::PRIVATE) + end + + it 'creates a reference for a project member for a public snippet' do + expect(visible_references(:public, project_member)).to eq([link]) + end + + it 'does not create a reference for guest for a public snippet' do + expect(visible_references(:public, nil)).to be_empty + end + + it 'does not create reference for a regular user for a public snippet' do + expect(visible_references(:public, user)).to be_empty + end + + it 'creates a reference for a project member for an internal snippet' do + expect(visible_references(:internal, project_member)).to eq([link]) + end + + it 'does not create a reference for a regular user for an internal snippet' do + expect(visible_references(:internal, user)).to be_empty + end + + it 'creates a reference for a project member for a private snippet' do + expect(visible_references(:private, project_member)).to eq([link]) + end + + it 'does not create reference for a regular user for a private snippet' do + expect(visible_references(:private, user)).to be_empty + end + end + + context 'when a project is private and the snippets feature is enabled for project team members' do + before do + project.update_attribute(:visibility, Gitlab::VisibilityLevel::PRIVATE) + project.project_feature.update_attribute(:snippets_access_level, ProjectFeature::PRIVATE) + end + + it 'creates a reference for a project member for a public snippet' do + expect(visible_references(:public, project_member)).to eq([link]) + end + + it 'does not create a reference for guest for a public snippet' do + expect(visible_references(:public, nil)).to be_empty + end + + it 'does not create a reference for a regular user for a public snippet' do + expect(visible_references(:public, user)).to be_empty + end + + it 'creates a reference for a project member for an internal snippet' do + expect(visible_references(:internal, project_member)).to eq([link]) + end + + it 'does not create a reference for a regular user for an internal snippet' do + expect(visible_references(:internal, user)).to be_empty + end + + it 'creates a reference for a project member for a private snippet' do + expect(visible_references(:private, project_member)).to eq([link]) + end + + it 'does not create a reference for a regular user for a private snippet' do + expect(visible_references(:private, user)).to be_empty + end end end describe '#referenced_by' do + let(:snippet) { create(:snippet, project: project) } describe 'when the link has a data-snippet attribute' do context 'using an existing snippet ID' do it 'returns an Array of snippets' do @@ -31,7 +210,7 @@ describe Banzai::ReferenceParser::SnippetParser, lib: true do it 'returns an empty Array' do link['data-snippet'] = '' - expect(subject.referenced_by([link])).to eq([]) + expect(subject.referenced_by([link])).to be_empty end end end diff --git a/spec/lib/gitlab/uploads_transfer_spec.rb b/spec/lib/gitlab/uploads_transfer_spec.rb new file mode 100644 index 00000000000..109559bb01c --- /dev/null +++ b/spec/lib/gitlab/uploads_transfer_spec.rb @@ -0,0 +1,11 @@ +require 'spec_helper' + +describe Gitlab::UploadsTransfer do + it 'leaves avatar uploads where they are' do + project_with_avatar = create(:empty_project, :with_avatar) + + described_class.new.rename_namespace('project', 'project-renamed') + + expect(File.exist?(project_with_avatar.avatar.path)).to be_truthy + end +end diff --git a/spec/migrations/clean_upload_symlinks_spec.rb b/spec/migrations/clean_upload_symlinks_spec.rb new file mode 100644 index 00000000000..cecb3ddac53 --- /dev/null +++ b/spec/migrations/clean_upload_symlinks_spec.rb @@ -0,0 +1,46 @@ +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20170406111121_clean_upload_symlinks.rb') + +describe CleanUploadSymlinks do + let(:migration) { described_class.new } + let(:test_dir) { File.join(Rails.root, "tmp", "tests", "move_uploads_test") } + let(:uploads_dir) { File.join(test_dir, "public", "uploads") } + let(:new_uploads_dir) { File.join(uploads_dir, "system") } + let(:original_path) { File.join(new_uploads_dir, 'user') } + let(:symlink_path) { File.join(uploads_dir, 'user') } + + before do + FileUtils.remove_dir(test_dir) if File.directory?(test_dir) + FileUtils.mkdir_p(uploads_dir) + allow(migration).to receive(:base_directory).and_return(test_dir) + allow(migration).to receive(:say) + end + + describe "#up" do + before do + FileUtils.mkdir_p(original_path) + FileUtils.ln_s(original_path, symlink_path) + end + + it 'removes the symlink' do + migration.up + + expect(File.symlink?(symlink_path)).to be(false) + end + end + + describe '#down' do + before do + FileUtils.mkdir_p(File.join(original_path)) + FileUtils.touch(File.join(original_path, 'dummy.file')) + end + + it 'creates a symlink' do + expected_path = File.join(symlink_path, "dummy.file") + migration.down + + expect(File.exist?(expected_path)).to be(true) + expect(File.symlink?(symlink_path)).to be(true) + end + end +end diff --git a/spec/migrations/move_uploads_to_system_dir_spec.rb b/spec/migrations/move_uploads_to_system_dir_spec.rb new file mode 100644 index 00000000000..37d66452447 --- /dev/null +++ b/spec/migrations/move_uploads_to_system_dir_spec.rb @@ -0,0 +1,68 @@ +require "spec_helper" +require Rails.root.join("db", "migrate", "20170316163845_move_uploads_to_system_dir.rb") + +describe MoveUploadsToSystemDir do + let(:migration) { described_class.new } + let(:test_dir) { File.join(Rails.root, "tmp", "move_uploads_test") } + let(:uploads_dir) { File.join(test_dir, "public", "uploads") } + let(:new_uploads_dir) { File.join(uploads_dir, "system") } + + before do + FileUtils.remove_dir(test_dir) if File.directory?(test_dir) + FileUtils.mkdir_p(uploads_dir) + allow(migration).to receive(:base_directory).and_return(test_dir) + allow(migration).to receive(:say) + end + + describe "#up" do + before do + FileUtils.mkdir_p(File.join(uploads_dir, 'user')) + FileUtils.touch(File.join(uploads_dir, 'user', 'dummy.file')) + end + + it 'moves the directory to the new path' do + expected_path = File.join(new_uploads_dir, 'user', 'dummy.file') + + migration.up + + expect(File.exist?(expected_path)).to be(true) + end + + it 'creates a symlink in the old location' do + symlink_path = File.join(uploads_dir, 'user') + expected_path = File.join(symlink_path, 'dummy.file') + + migration.up + + expect(File.exist?(expected_path)).to be(true) + expect(File.symlink?(symlink_path)).to be(true) + end + end + + describe "#down" do + before do + FileUtils.mkdir_p(File.join(new_uploads_dir, 'user')) + FileUtils.touch(File.join(new_uploads_dir, 'user', 'dummy.file')) + end + + it 'moves the directory to the old path' do + expected_path = File.join(uploads_dir, 'user', 'dummy.file') + + migration.down + + expect(File.exist?(expected_path)).to be(true) + end + + it 'removes the symlink if it existed' do + FileUtils.ln_s(File.join(new_uploads_dir, 'user'), File.join(uploads_dir, 'user')) + + directory = File.join(uploads_dir, 'user') + expected_path = File.join(directory, 'dummy.file') + + migration.down + + expect(File.exist?(expected_path)).to be(true) + expect(File.symlink?(directory)).to be(false) + end + end +end diff --git a/spec/migrations/rename_system_namespaces_spec.rb b/spec/migrations/rename_system_namespaces_spec.rb new file mode 100644 index 00000000000..626a6005838 --- /dev/null +++ b/spec/migrations/rename_system_namespaces_spec.rb @@ -0,0 +1,254 @@ +require "spec_helper" +require Rails.root.join("db", "migrate", "20170316163800_rename_system_namespaces.rb") + +describe RenameSystemNamespaces, truncate: true do + let(:migration) { described_class.new } + let(:test_dir) { File.join(Rails.root, "tmp", "tests", "rename_namespaces_test") } + let(:uploads_dir) { File.join(test_dir, "public", "uploads") } + let(:system_namespace) do + namespace = build(:namespace, path: "system") + namespace.save(validate: false) + namespace + end + + def save_invalid_routable(routable) + routable.__send__(:prepare_route) + routable.save(validate: false) + end + + before do + FileUtils.remove_dir(test_dir) if File.directory?(test_dir) + FileUtils.mkdir_p(uploads_dir) + FileUtils.remove_dir(TestEnv.repos_path) if File.directory?(TestEnv.repos_path) + allow(migration).to receive(:say) + allow(migration).to receive(:uploads_dir).and_return(uploads_dir) + end + + describe "#system_namespace" do + it "only root namespaces called with path `system`" do + system_namespace + system_namespace_with_parent = build(:namespace, path: 'system', parent: create(:namespace)) + system_namespace_with_parent.save(validate: false) + + expect(migration.system_namespace.id).to eq(system_namespace.id) + end + end + + describe "#up" do + before do + system_namespace + end + + it "doesn't break if there are no namespaces called system" do + Namespace.delete_all + + migration.up + end + + it "renames namespaces called system" do + migration.up + + expect(system_namespace.reload.path).to eq("system0") + end + + it "renames the route to the namespace" do + migration.up + + expect(system_namespace.reload.full_path).to eq("system0") + end + + it "renames the route for projects of the namespace" do + project = build(:project, path: "project-path", namespace: system_namespace) + save_invalid_routable(project) + + migration.up + + expect(project.route.reload.path).to eq("system0/project-path") + end + + it "doesn't touch routes of namespaces that look like system" do + namespace = create(:group, path: 'systemlookalike') + project = create(:project, namespace: namespace, path: 'the-project') + + migration.up + + expect(project.route.reload.path).to eq('systemlookalike/the-project') + expect(namespace.route.reload.path).to eq('systemlookalike') + end + + it "moves the the repository for a project in the namespace" do + project = build(:project, namespace: system_namespace, path: "system-project") + save_invalid_routable(project) + TestEnv.copy_repo(project, + bare_repo: TestEnv.factory_repo_path_bare, + refs: TestEnv::BRANCH_SHA) + expected_repo = File.join(TestEnv.repos_path, "system0", "system-project.git") + + migration.up + + expect(File.directory?(expected_repo)).to be(true) + end + + it "moves the uploads for the namespace" do + allow(migration).to receive(:move_namespace_folders).with(Settings.pages.path, "system", "system0") + expect(migration).to receive(:move_namespace_folders).with(uploads_dir, "system", "system0") + + migration.up + end + + it "moves the pages for the namespace" do + allow(migration).to receive(:move_namespace_folders).with(uploads_dir, "system", "system0") + expect(migration).to receive(:move_namespace_folders).with(Settings.pages.path, "system", "system0") + + migration.up + end + + describe "clears the markdown cache for projects in the system namespace" do + let!(:project) do + project = build(:project, namespace: system_namespace) + save_invalid_routable(project) + project + end + + it 'removes description_html from projects' do + migration.up + + expect(project.reload.description_html).to be_nil + end + + it 'removes issue descriptions' do + issue = create(:issue, project: project, description_html: 'Issue description') + + migration.up + + expect(issue.reload.description_html).to be_nil + end + + it 'removes merge request descriptions' do + merge_request = create(:merge_request, + source_project: project, + target_project: project, + description_html: 'MergeRequest description') + + migration.up + + expect(merge_request.reload.description_html).to be_nil + end + + it 'removes note html' do + note = create(:note, + project: project, + noteable: create(:issue, project: project), + note_html: 'note description') + + migration.up + + expect(note.reload.note_html).to be_nil + end + + it 'removes milestone description' do + milestone = create(:milestone, + project: project, + description_html: 'milestone description') + + migration.up + + expect(milestone.reload.description_html).to be_nil + end + end + + context "system namespace -> subgroup -> system0 project" do + it "updates the route of the project correctly" do + subgroup = build(:group, path: "subgroup", parent: system_namespace) + save_invalid_routable(subgroup) + project = build(:project, path: "system0", namespace: subgroup) + save_invalid_routable(project) + + migration.up + + expect(project.route.reload.path).to eq("system0/subgroup/system0") + end + end + end + + describe "#move_repositories" do + let(:namespace) { create(:group, name: "hello-group") } + it "moves a project for a namespace" do + create(:project, namespace: namespace, path: "hello-project") + expected_path = File.join(TestEnv.repos_path, "bye-group", "hello-project.git") + + migration.move_repositories(namespace, "hello-group", "bye-group") + + expect(File.directory?(expected_path)).to be(true) + end + + it "moves a namespace in a subdirectory correctly" do + child_namespace = create(:group, name: "sub-group", parent: namespace) + create(:project, namespace: child_namespace, path: "hello-project") + + expected_path = File.join(TestEnv.repos_path, "hello-group", "renamed-sub-group", "hello-project.git") + + migration.move_repositories(child_namespace, "hello-group/sub-group", "hello-group/renamed-sub-group") + + expect(File.directory?(expected_path)).to be(true) + end + + it "moves a parent namespace with subdirectories" do + child_namespace = create(:group, name: "sub-group", parent: namespace) + create(:project, namespace: child_namespace, path: "hello-project") + expected_path = File.join(TestEnv.repos_path, "renamed-group", "sub-group", "hello-project.git") + + migration.move_repositories(child_namespace, "hello-group", "renamed-group") + + expect(File.directory?(expected_path)).to be(true) + end + end + + describe "#move_namespace_folders" do + it "moves a namespace with files" do + source = File.join(uploads_dir, "parent-group", "sub-group") + FileUtils.mkdir_p(source) + destination = File.join(uploads_dir, "parent-group", "moved-group") + FileUtils.touch(File.join(source, "test.txt")) + expected_file = File.join(destination, "test.txt") + + migration.move_namespace_folders(uploads_dir, File.join("parent-group", "sub-group"), File.join("parent-group", "moved-group")) + + expect(File.exist?(expected_file)).to be(true) + end + + it "moves a parent namespace uploads" do + source = File.join(uploads_dir, "parent-group", "sub-group") + FileUtils.mkdir_p(source) + destination = File.join(uploads_dir, "moved-parent", "sub-group") + FileUtils.touch(File.join(source, "test.txt")) + expected_file = File.join(destination, "test.txt") + + migration.move_namespace_folders(uploads_dir, "parent-group", "moved-parent") + + expect(File.exist?(expected_file)).to be(true) + end + end + + describe "#child_ids_for_parent" do + it "collects child ids for all levels" do + parent = create(:group) + first_child = create(:group, parent: parent) + second_child = create(:group, parent: parent) + third_child = create(:group, parent: second_child) + all_ids = [parent.id, first_child.id, second_child.id, third_child.id] + + collected_ids = migration.child_ids_for_parent(parent, ids: [parent.id]) + + expect(collected_ids).to contain_exactly(*all_ids) + end + end + + describe "#remove_last_ocurrence" do + it "removes only the last occurance of a string" do + input = "this/is/system/namespace/with/system" + + expect(migration.remove_last_occurrence(input, "system")).to eq("this/is/system/namespace/with/") + end + end +end diff --git a/spec/migrations/update_upload_paths_to_system_spec.rb b/spec/migrations/update_upload_paths_to_system_spec.rb new file mode 100644 index 00000000000..7df44515424 --- /dev/null +++ b/spec/migrations/update_upload_paths_to_system_spec.rb @@ -0,0 +1,53 @@ +require "spec_helper" +require Rails.root.join("db", "post_migrate", "20170317162059_update_upload_paths_to_system.rb") + +describe UpdateUploadPathsToSystem do + let(:migration) { described_class.new } + + before do + allow(migration).to receive(:say) + end + + describe "#uploads_to_switch_to_new_path" do + it "contains only uploads with the old path for the correct models" do + _upload_for_other_type = create(:upload, model: create(:ci_pipeline), path: "uploads/ci_pipeline/avatar.jpg") + _upload_with_system_path = create(:upload, model: create(:empty_project), path: "uploads/system/project/avatar.jpg") + _upload_with_other_path = create(:upload, model: create(:empty_project), path: "thelongsecretforafileupload/avatar.jpg") + old_upload = create(:upload, model: create(:empty_project), path: "uploads/project/avatar.jpg") + group_upload = create(:upload, model: create(:group), path: "uploads/group/avatar.jpg") + + expect(Upload.where(migration.uploads_to_switch_to_new_path)).to contain_exactly(old_upload, group_upload) + end + end + + describe "#uploads_to_switch_to_old_path" do + it "contains only uploads with the new path for the correct models" do + _upload_for_other_type = create(:upload, model: create(:ci_pipeline), path: "uploads/ci_pipeline/avatar.jpg") + upload_with_system_path = create(:upload, model: create(:empty_project), path: "uploads/system/project/avatar.jpg") + _upload_with_other_path = create(:upload, model: create(:empty_project), path: "thelongsecretforafileupload/avatar.jpg") + _old_upload = create(:upload, model: create(:empty_project), path: "uploads/project/avatar.jpg") + + expect(Upload.where(migration.uploads_to_switch_to_old_path)).to contain_exactly(upload_with_system_path) + end + end + + describe "#up", truncate: true do + it "updates old upload records to the new path" do + old_upload = create(:upload, model: create(:empty_project), path: "uploads/project/avatar.jpg") + + migration.up + + expect(old_upload.reload.path).to eq("uploads/system/project/avatar.jpg") + end + end + + describe "#down", truncate: true do + it "updates the new system patsh to the old paths" do + new_upload = create(:upload, model: create(:empty_project), path: "uploads/system/project/avatar.jpg") + + migration.down + + expect(new_upload.reload.path).to eq("uploads/project/avatar.jpg") + end + end +end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 316bf153660..3d437ca0fcc 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -179,7 +179,7 @@ describe Group, models: true do let!(:group) { create(:group, :access_requestable, :with_avatar) } let(:user) { create(:user) } let(:gitlab_host) { "http://#{Gitlab.config.gitlab.host}" } - let(:avatar_path) { "/uploads/group/avatar/#{group.id}/dk.png" } + let(:avatar_path) { "/uploads/system/group/avatar/#{group.id}/dk.png" } context 'when avatar file is uploaded' do before { group.add_master(user) } diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 0e74f1ab1bd..145c7ad5770 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -43,6 +43,12 @@ describe Namespace, models: true do end end + context "is case insensitive" do + let(:group) { build(:group, path: "System") } + + it { expect(group).not_to be_valid } + end + context 'top-level group' do let(:group) { build(:group, path: 'tree') } @@ -178,8 +184,8 @@ describe Namespace, models: true do let(:parent) { create(:group, name: 'parent', path: 'parent') } let(:child) { create(:group, name: 'child', path: 'child', parent: parent) } let!(:project) { create(:project_empty_repo, path: 'the-project', namespace: child) } - let(:uploads_dir) { File.join(CarrierWave.root, 'uploads') } - let(:pages_dir) { TestEnv.pages_path } + let(:uploads_dir) { File.join(CarrierWave.root, FileUploader.base_dir) } + let(:pages_dir) { File.join(TestEnv.pages_path) } before do FileUtils.mkdir_p(File.join(uploads_dir, 'parent', 'child', 'the-project')) diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 3ed52d42f86..454eeb58ecd 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -812,7 +812,7 @@ describe Project, models: true do context 'when avatar file is uploaded' do let(:project) { create(:empty_project, :with_avatar) } - let(:avatar_path) { "/uploads/project/avatar/#{project.id}/dk.png" } + let(:avatar_path) { "/uploads/system/project/avatar/#{project.id}/dk.png" } let(:gitlab_host) { "http://#{Gitlab.config.gitlab.host}" } it 'shows correct url' do diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index a83726b48a0..d5bd9946ab6 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -987,7 +987,7 @@ describe User, models: true do context 'when avatar file is uploaded' do let(:gitlab_host) { "http://#{Gitlab.config.gitlab.host}" } - let(:avatar_path) { "/uploads/user/avatar/#{user.id}/dk.png" } + let(:avatar_path) { "/uploads/system/user/avatar/#{user.id}/dk.png" } it 'shows correct avatar url' do expect(user.avatar_url).to eq(avatar_path) diff --git a/spec/policies/project_snippet_policy_spec.rb b/spec/policies/project_snippet_policy_spec.rb index e1771b636b8..ddbed5f781e 100644 --- a/spec/policies/project_snippet_policy_spec.rb +++ b/spec/policies/project_snippet_policy_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe ProjectSnippetPolicy, models: true do let(:regular_user) { create(:user) } let(:external_user) { create(:user, :external) } - let(:project) { create(:empty_project) } + let(:project) { create(:empty_project, :public) } let(:author_permissions) do [ @@ -107,7 +107,7 @@ describe ProjectSnippetPolicy, models: true do end context 'snippet author' do - let(:snippet) { create(:project_snippet, :private, author: regular_user) } + let(:snippet) { create(:project_snippet, :private, author: regular_user, project: project) } subject { described_class.abilities(regular_user, snippet).to_set } diff --git a/spec/requests/openid_connect_spec.rb b/spec/requests/openid_connect_spec.rb index 05176c3beaa..6d1f0b24196 100644 --- a/spec/requests/openid_connect_spec.rb +++ b/spec/requests/openid_connect_spec.rb @@ -79,7 +79,7 @@ describe 'OpenID Connect requests' do 'email_verified' => true, 'website' => 'https://example.com', 'profile' => 'http://localhost/alice', - 'picture' => "http://localhost/uploads/user/avatar/#{user.id}/dk.png" + 'picture' => "http://localhost/uploads/system/user/avatar/#{user.id}/dk.png" }) end end diff --git a/spec/services/projects/participants_service_spec.rb b/spec/services/projects/participants_service_spec.rb index 0657b7e93fe..d75851134ee 100644 --- a/spec/services/projects/participants_service_spec.rb +++ b/spec/services/projects/participants_service_spec.rb @@ -13,7 +13,7 @@ describe Projects::ParticipantsService, services: true do groups = participants.groups expect(groups.size).to eq 1 - expect(groups.first[:avatar_url]).to eq("/uploads/group/avatar/#{group.id}/dk.png") + expect(groups.first[:avatar_url]).to eq("/uploads/system/group/avatar/#{group.id}/dk.png") end it 'should return an url for the avatar with relative url' do @@ -24,7 +24,7 @@ describe Projects::ParticipantsService, services: true do groups = participants.groups expect(groups.size).to eq 1 - expect(groups.first[:avatar_url]).to eq("/gitlab/uploads/group/avatar/#{group.id}/dk.png") + expect(groups.first[:avatar_url]).to eq("/gitlab/uploads/system/group/avatar/#{group.id}/dk.png") end end end diff --git a/spec/uploaders/attachment_uploader_spec.rb b/spec/uploaders/attachment_uploader_spec.rb index ea714fb08f0..d82dbe871d5 100644 --- a/spec/uploaders/attachment_uploader_spec.rb +++ b/spec/uploaders/attachment_uploader_spec.rb @@ -3,6 +3,17 @@ require 'spec_helper' describe AttachmentUploader do let(:uploader) { described_class.new(build_stubbed(:user)) } + describe "#store_dir" do + it "stores in the system dir" do + expect(uploader.store_dir).to start_with("uploads/system/user") + end + + it "uses the old path when using object storage" do + expect(described_class).to receive(:file_storage?).and_return(false) + expect(uploader.store_dir).to start_with("uploads/user") + end + end + describe '#move_to_cache' do it 'is true' do expect(uploader.move_to_cache).to eq(true) diff --git a/spec/uploaders/avatar_uploader_spec.rb b/spec/uploaders/avatar_uploader_spec.rb index c4d558805ab..201fe6949aa 100644 --- a/spec/uploaders/avatar_uploader_spec.rb +++ b/spec/uploaders/avatar_uploader_spec.rb @@ -3,6 +3,17 @@ require 'spec_helper' describe AvatarUploader do let(:uploader) { described_class.new(build_stubbed(:user)) } + describe "#store_dir" do + it "stores in the system dir" do + expect(uploader.store_dir).to start_with("uploads/system/user") + end + + it "uses the old path when using object storage" do + expect(described_class).to receive(:file_storage?).and_return(false) + expect(uploader.store_dir).to start_with("uploads/user") + end + end + describe '#move_to_cache' do it 'is false' do expect(uploader.move_to_cache).to eq(false) diff --git a/spec/uploaders/file_uploader_spec.rb b/spec/uploaders/file_uploader_spec.rb index d9113ef4095..47e9365e13d 100644 --- a/spec/uploaders/file_uploader_spec.rb +++ b/spec/uploaders/file_uploader_spec.rb @@ -15,6 +15,16 @@ describe FileUploader do end end + describe "#store_dir" do + it "stores in the namespace path" do + project = build_stubbed(:empty_project) + uploader = described_class.new(project) + + expect(uploader.store_dir).to include(project.path_with_namespace) + expect(uploader.store_dir).not_to include("system") + end + end + describe 'initialize' do it 'generates a secret if none is provided' do expect(SecureRandom).to receive(:hex).and_return('secret') diff --git a/spec/workers/post_receive_spec.rb b/spec/workers/post_receive_spec.rb index f4bc63bcc6a..44163c735ba 100644 --- a/spec/workers/post_receive_spec.rb +++ b/spec/workers/post_receive_spec.rb @@ -94,26 +94,23 @@ describe PostReceive do it { expect{ subject }.not_to change{ Ci::Pipeline.count } } end end - end - describe '#process_repository_update' do - let(:changes) {'123456 789012 refs/heads/tést'} - let(:fake_hook_data) do - { event_name: 'repository_update' } - 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') } - before do - allow_any_instance_of(Gitlab::GitPostReceive).to receive(:identify).and_return(project.owner) - 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) - allow(subject).to receive(:process_project_changes).and_return(true) - end + before 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) + allow_any_instance_of(GitPushService).to receive(:execute).and_return(true) + end - it 'calls SystemHooksService' do - expect_any_instance_of(SystemHooksService).to receive(:execute_hooks).with(fake_hook_data, :repository_update_hooks).and_return(true) + it 'calls SystemHooksService' do + expect_any_instance_of(SystemHooksService).to receive(:execute_hooks).with(fake_hook_data, :repository_update_hooks).and_return(true) - subject.perform(pwd(project), key_id, base64_changes) + described_class.new.perform(project_identifier, key_id, base64_changes) + end end end |