diff options
210 files changed, 2502 insertions, 1253 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 6088a1b3515..adf097b52f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,17 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 10.2.4 (2017-12-07) + +### Security (5 changes) + +- Fix e-mail address disclosure through member search fields +- Prevent creating issues through API when user does not have permissions +- Prevent an information disclosure in the Groups API +- Fix user without access to private Wiki being able to see it on the project page +- Fix Cross-Site Scripting (XSS) vulnerability while editing a comment + + ## 10.2.3 (2017-11-30) ### Fixed (7 changes) @@ -237,6 +248,17 @@ entry. - Add Gitaly metrics to the performance bar. +## 10.1.5 (2017-12-07) + +### Security (5 changes) + +- Fix e-mail address disclosure through member search fields +- Prevent creating issues through API when user does not have permissions +- Prevent an information disclosure in the Groups API +- Fix user without access to private Wiki being able to see it on the project page +- Fix Cross-Site Scripting (XSS) vulnerability while editing a comment + + ## 10.1.4 (2017-11-14) ### Fixed (4 changes) @@ -485,6 +507,17 @@ entry. - creation of keys moved to services. !13331 (haseebeqx) - Add username as GL_USERNAME in hooks. +## 10.0.7 (2017-12-07) + +### Security (5 changes) + +- Fix e-mail address disclosure through member search fields +- Prevent creating issues through API when user does not have permissions +- Prevent an information disclosure in the Groups API +- Fix user without access to private Wiki being able to see it on the project page +- Fix Cross-Site Scripting (XSS) vulnerability while editing a comment + + ## 10.0.5 (2017-11-03) - [FIXED] Fix incorrect X-axis labels in Prometheus graphs. !14258 diff --git a/GITLAB_SHELL_VERSION b/GITLAB_SHELL_VERSION index 4e32c7b1caf..269fb5dfe2c 100644 --- a/GITLAB_SHELL_VERSION +++ b/GITLAB_SHELL_VERSION @@ -1 +1 @@ -5.10.1 +5.10.2 @@ -283,7 +283,7 @@ group :metrics do gem 'influxdb', '~> 0.2', require: false # Prometheus - gem 'prometheus-client-mmap', '~> 0.7.0.beta39' + gem 'prometheus-client-mmap', '~> 0.7.0.beta43' gem 'raindrops', '~> 0.18' end @@ -411,3 +411,6 @@ gem 'flipper-active_record', '~> 0.10.2' # Structured logging gem 'lograge', '~> 0.5' gem 'grape_logging', '~> 1.7' + +# Asset synchronization +gem 'asset_sync', '~> 2.2.0' diff --git a/Gemfile.lock b/Gemfile.lock index 6213167ae0b..efae71efdb7 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -58,6 +58,11 @@ GEM asciidoctor (1.5.3) asciidoctor-plantuml (0.0.7) asciidoctor (~> 1.5) + asset_sync (2.2.0) + activemodel (>= 4.1.0) + fog-core + mime-types (>= 2.99) + unf ast (2.3.0) atomic (1.1.99) attr_encrypted (3.0.3) @@ -975,6 +980,7 @@ DEPENDENCIES asana (~> 0.6.0) asciidoctor (~> 1.5.2) asciidoctor-plantuml (= 0.0.7) + asset_sync (~> 2.2.0) attr_encrypted (~> 3.0.0) awesome_print (~> 1.2.0) babosa (~> 1.0.2) @@ -1107,7 +1113,7 @@ DEPENDENCIES peek-sidekiq (~> 1.0.3) pg (~> 0.18.2) premailer-rails (~> 1.9.7) - prometheus-client-mmap (~> 0.7.0.beta39) + prometheus-client-mmap (~> 0.7.0.beta43) pry-byebug (~> 3.4.1) pry-rails (~> 0.3.4) rack-attack (~> 4.4.1) diff --git a/PROCESS.md b/PROCESS.md index 7c8db689256..3fcf676b302 100644 --- a/PROCESS.md +++ b/PROCESS.md @@ -130,7 +130,8 @@ freeze date (the 7th) should have a corresponding Enterprise Edition merge request, even if there are no conflicts. This is to reduce the size of the subsequent EE merge, as we often merge a lot to CE on the release date. For more information, see -[limit conflicts with EE when developing on CE][limit_ee_conflicts]. +[Automatic CE->EE merge][automatic_ce_ee_merge] and +[Guidelines for implementing Enterprise Edition features][ee_features]. ### After the 7th @@ -281,4 +282,5 @@ still an issue I encourage you to open it on the [GitLab.com issue tracker](http ["Implement design & UI elements" guidelines]: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#implement-design-ui-elements [Thoughtbot code review guide]: https://github.com/thoughtbot/guides/tree/master/code-review [done]: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#definition-of-done -[limit_ee_conflicts]: https://docs.gitlab.com/ce/development/limit_ee_conflicts.html +[automatic_ce_ee_merge]: https://docs.gitlab.com/ce/development/automatic_ce_ee_merge.html +[ee_features]: https://docs.gitlab.com/ce/development/ee_features.html diff --git a/app/assets/javascripts/admin.js b/app/assets/javascripts/admin.js index 34669dd13d6..b0b72c40f25 100644 --- a/app/assets/javascripts/admin.js +++ b/app/assets/javascripts/admin.js @@ -1,4 +1,5 @@ /* eslint-disable func-names, space-before-function-paren, wrap-iife, one-var, no-var, one-var-declaration-per-line, no-unused-vars, no-else-return, prefer-arrow-callback, camelcase, quotes, comma-dangle, max-len */ +import { refreshCurrentPage } from './lib/utils/url_utility'; window.Admin = (function() { function Admin() { @@ -40,10 +41,10 @@ window.Admin = (function() { return $('.change-owner-link').show(); }); $('li.project_member').bind('ajax:success', function() { - return gl.utils.refreshCurrentPage(); + return refreshCurrentPage(); }); $('li.group_member').bind('ajax:success', function() { - return gl.utils.refreshCurrentPage(); + return refreshCurrentPage(); }); showBlacklistType = function() { if ($("input[name='blacklist_type']:checked").val() === 'file') { diff --git a/app/assets/javascripts/behaviors/toggler_behavior.js b/app/assets/javascripts/behaviors/toggler_behavior.js index b70b0a9bbf8..417ac31fc86 100644 --- a/app/assets/javascripts/behaviors/toggler_behavior.js +++ b/app/assets/javascripts/behaviors/toggler_behavior.js @@ -5,6 +5,7 @@ // %button.js-toggle-button // %div.js-toggle-content // +import { getLocationHash } from '../lib/utils/url_utility'; $(() => { function toggleContainer(container, toggleState) { @@ -32,7 +33,7 @@ $(() => { // If we're accessing a permalink, ensure it is not inside a // closed js-toggle-container! - const hash = window.gl.utils.getLocationHash(); + const hash = getLocationHash(); const anchor = hash && document.getElementById(hash); const container = anchor && $(anchor).closest('.js-toggle-container'); diff --git a/app/assets/javascripts/blob/blob_file_dropzone.js b/app/assets/javascripts/blob/blob_file_dropzone.js index 0d590a9dbc4..f7ae6f1cd12 100644 --- a/app/assets/javascripts/blob/blob_file_dropzone.js +++ b/app/assets/javascripts/blob/blob_file_dropzone.js @@ -1,6 +1,6 @@ /* eslint-disable func-names, object-shorthand, prefer-arrow-callback */ import Dropzone from 'dropzone'; -import '../lib/utils/url_utility'; +import { visitUrl } from '../lib/utils/url_utility'; import { HIDDEN_CLASS } from '../lib/utils/constants'; import csrf from '../lib/utils/csrf'; @@ -49,7 +49,7 @@ export default class BlobFileDropzone { }); this.on('success', function (header, response) { $('#modal-upload-blob').modal('hide'); - window.gl.utils.visitUrl(response.filePath); + visitUrl(response.filePath); }); this.on('maxfilesexceeded', function (file) { dropzoneMessage.addClass(HIDDEN_CLASS); diff --git a/app/assets/javascripts/blob/blob_line_permalink_updater.js b/app/assets/javascripts/blob/blob_line_permalink_updater.js index c8f68860fbd..d36d9f0de2d 100644 --- a/app/assets/javascripts/blob/blob_line_permalink_updater.js +++ b/app/assets/javascripts/blob/blob_line_permalink_updater.js @@ -1,7 +1,9 @@ +import { getLocationHash } from '../lib/utils/url_utility'; + const lineNumberRe = /^L[0-9]+/; const updateLineNumbersOnBlobPermalinks = (linksToUpdate) => { - const hash = gl.utils.getLocationHash(); + const hash = getLocationHash(); if (hash && lineNumberRe.test(hash)) { const hashUrlString = `#${hash}`; diff --git a/app/assets/javascripts/commit/image_file.js b/app/assets/javascripts/commit/image_file.js index 50d0cb5c86d..5662802525e 100644 --- a/app/assets/javascripts/commit/image_file.js +++ b/app/assets/javascripts/commit/image_file.js @@ -121,7 +121,7 @@ export default class ImageFile { return $('.swipe.view', this.file).each((function(_this) { return function(index, view) { var $swipeWrap, $swipeBar, $swipeFrame, wrapPadding, ref; - ref = this.prepareFrames(view), maxWidth = ref[0], maxHeight = ref[1]; + ref = _this.prepareFrames(view), maxWidth = ref[0], maxHeight = ref[1]; $swipeFrame = $('.swipe-frame', view); $swipeWrap = $('.swipe-wrap', view); $swipeBar = $('.swipe-bar', view); @@ -158,7 +158,7 @@ export default class ImageFile { return $('.onion-skin.view', this.file).each((function(_this) { return function(index, view) { var $frame, $track, $dragger, $frameAdded, framePadding, ref, dragging = false; - ref = this.prepareFrames(view), maxWidth = ref[0], maxHeight = ref[1]; + ref = _this.prepareFrames(view), maxWidth = ref[0], maxHeight = ref[1]; $frame = $('.onion-skin-frame', view); $frameAdded = $('.frame.added', view); $track = $('.drag-track', view); diff --git a/app/assets/javascripts/compare.js b/app/assets/javascripts/compare.js index 9e5dbd64a7e..0ce467a3bd4 100644 --- a/app/assets/javascripts/compare.js +++ b/app/assets/javascripts/compare.js @@ -1,7 +1,7 @@ /* eslint-disable func-names, space-before-function-paren, wrap-iife, quotes, no-var, object-shorthand, consistent-return, no-unused-vars, comma-dangle, vars-on-top, prefer-template, max-len */ -window.Compare = (function() { - function Compare(opts) { +export default class Compare { + constructor(opts) { this.opts = opts; this.source_loading = $(".js-source-loading"); this.target_loading = $(".js-target-loading"); @@ -34,12 +34,12 @@ window.Compare = (function() { this.initialState(); } - Compare.prototype.initialState = function() { + initialState() { this.getSourceHtml(); - return this.getTargetHtml(); - }; + this.getTargetHtml(); + } - Compare.prototype.getTargetProject = function() { + getTargetProject() { return $.ajax({ url: this.opts.targetProjectUrl, data: { @@ -52,22 +52,22 @@ window.Compare = (function() { return $('.js-target-branch-dropdown .dropdown-content').html(html); } }); - }; + } - Compare.prototype.getSourceHtml = function() { - return this.sendAjax(this.opts.sourceBranchUrl, this.source_loading, '.mr_source_commit', { + getSourceHtml() { + return this.constructor.sendAjax(this.opts.sourceBranchUrl, this.source_loading, '.mr_source_commit', { ref: $("input[name='merge_request[source_branch]']").val() }); - }; + } - Compare.prototype.getTargetHtml = function() { - return this.sendAjax(this.opts.targetBranchUrl, this.target_loading, '.mr_target_commit', { + getTargetHtml() { + return this.constructor.sendAjax(this.opts.targetBranchUrl, this.target_loading, '.mr_target_commit', { target_project_id: $("input[name='merge_request[target_project_id]']").val(), ref: $("input[name='merge_request[target_branch]']").val() }); - }; + } - Compare.prototype.sendAjax = function(url, loading, target, data) { + static sendAjax(url, loading, target, data) { var $target; $target = $(target); return $.ajax({ @@ -84,7 +84,5 @@ window.Compare = (function() { gl.utils.localTimeAgo($('.js-timeago', className)); } }); - }; - - return Compare; -})(); + } +} diff --git a/app/assets/javascripts/compare_autocomplete.js b/app/assets/javascripts/compare_autocomplete.js index 72c0d98d47c..e633ef8a29e 100644 --- a/app/assets/javascripts/compare_autocomplete.js +++ b/app/assets/javascripts/compare_autocomplete.js @@ -1,68 +1,60 @@ /* eslint-disable func-names, space-before-function-paren, one-var, no-var, one-var-declaration-per-line, object-shorthand, comma-dangle, prefer-arrow-callback, no-else-return, newline-per-chained-call, wrap-iife, max-len */ -window.CompareAutocomplete = (function() { - function CompareAutocomplete() { - this.initDropdown(); - } - - CompareAutocomplete.prototype.initDropdown = function() { - return $('.js-compare-dropdown').each(function() { - var $dropdown, selected; - $dropdown = $(this); - selected = $dropdown.data('selected'); - const $dropdownContainer = $dropdown.closest('.dropdown'); - const $fieldInput = $(`input[name="${$dropdown.data('field-name')}"]`, $dropdownContainer); - const $filterInput = $('input[type="search"]', $dropdownContainer); - $dropdown.glDropdown({ - data: function(term, callback) { - return $.ajax({ - url: $dropdown.data('refs-url'), - data: { - ref: $dropdown.data('ref'), - search: term, - } - }).done(function(refs) { - return callback(refs); - }); - }, - selectable: true, - filterable: true, - filterRemote: true, - fieldName: $dropdown.data('field-name'), - filterInput: 'input[type="search"]', - renderRow: function(ref) { - var link; - if (ref.header != null) { - return $('<li />').addClass('dropdown-header').text(ref.header); - } else { - link = $('<a />').attr('href', '#').addClass(ref === selected ? 'is-active' : '').text(ref).attr('data-ref', escape(ref)); - return $('<li />').append(link); +export default function initCompareAutocomplete() { + $('.js-compare-dropdown').each(function() { + var $dropdown, selected; + $dropdown = $(this); + selected = $dropdown.data('selected'); + const $dropdownContainer = $dropdown.closest('.dropdown'); + const $fieldInput = $(`input[name="${$dropdown.data('field-name')}"]`, $dropdownContainer); + const $filterInput = $('input[type="search"]', $dropdownContainer); + $dropdown.glDropdown({ + data: function(term, callback) { + return $.ajax({ + url: $dropdown.data('refs-url'), + data: { + ref: $dropdown.data('ref'), + search: term, } - }, - id: function(obj, $el) { - return $el.attr('data-ref'); - }, - toggleLabel: function(obj, $el) { - return $el.text().trim(); - } - }); - $filterInput.on('keyup', (e) => { - const keyCode = e.keyCode || e.which; - if (keyCode !== 13) return; - const text = $filterInput.val(); - $fieldInput.val(text); - $('.dropdown-toggle-text', $dropdown).text(text); - $dropdownContainer.removeClass('open'); - }); - - $dropdownContainer.on('click', '.dropdown-content a', (e) => { - $dropdown.prop('title', e.target.text.replace(/_+?/g, '-')); - if ($dropdown.hasClass('has-tooltip')) { - $dropdown.tooltip('fixTitle'); + }).done(function(refs) { + return callback(refs); + }); + }, + selectable: true, + filterable: true, + filterRemote: true, + fieldName: $dropdown.data('field-name'), + filterInput: 'input[type="search"]', + renderRow: function(ref) { + var link; + if (ref.header != null) { + return $('<li />').addClass('dropdown-header').text(ref.header); + } else { + link = $('<a />').attr('href', '#').addClass(ref === selected ? 'is-active' : '').text(ref).attr('data-ref', escape(ref)); + return $('<li />').append(link); } - }); + }, + id: function(obj, $el) { + return $el.attr('data-ref'); + }, + toggleLabel: function(obj, $el) { + return $el.text().trim(); + } + }); + $filterInput.on('keyup', (e) => { + const keyCode = e.keyCode || e.which; + if (keyCode !== 13) return; + const text = $filterInput.val(); + $fieldInput.val(text); + $('.dropdown-toggle-text', $dropdown).text(text); + $dropdownContainer.removeClass('open'); }); - }; - return CompareAutocomplete; -})(); + $dropdownContainer.on('click', '.dropdown-content a', (e) => { + $dropdown.prop('title', e.target.text.replace(/_+?/g, '-')); + if ($dropdown.hasClass('has-tooltip')) { + $dropdown.tooltip('fixTitle'); + } + }); + }); +} diff --git a/app/assets/javascripts/contextual_sidebar.js b/app/assets/javascripts/contextual_sidebar.js index 46b68ebe158..cd20dde2951 100644 --- a/app/assets/javascripts/contextual_sidebar.js +++ b/app/assets/javascripts/contextual_sidebar.js @@ -28,7 +28,7 @@ export default class ContextualSidebar { this.$closeSidebar.on('click', () => this.toggleSidebarNav(false)); this.$overlay.on('click', () => this.toggleSidebarNav(false)); this.$sidebarToggle.on('click', () => { - const value = !this.$sidebar.hasClass('sidebar-icons-only'); + const value = !this.$sidebar.hasClass('sidebar-collapsed-desktop'); this.toggleCollapsedSidebar(value); }); @@ -43,16 +43,16 @@ export default class ContextualSidebar { } toggleSidebarNav(show) { - this.$sidebar.toggleClass('nav-sidebar-expanded', show); + this.$sidebar.toggleClass('sidebar-expanded-mobile', show); this.$overlay.toggleClass('mobile-nav-open', show); - this.$sidebar.removeClass('sidebar-icons-only'); + this.$sidebar.removeClass('sidebar-collapsed-desktop'); } toggleCollapsedSidebar(collapsed) { const breakpoint = bp.getBreakpointSize(); if (this.$sidebar.length) { - this.$sidebar.toggleClass('sidebar-icons-only', collapsed); + this.$sidebar.toggleClass('sidebar-collapsed-desktop', collapsed); this.$page.toggleClass('page-with-icon-sidebar', breakpoint === 'sm' ? true : collapsed); } ContextualSidebar.setCollapsedCookie(collapsed); diff --git a/app/assets/javascripts/diff.js b/app/assets/javascripts/diff.js index c8874e48c09..a162424b3cf 100644 --- a/app/assets/javascripts/diff.js +++ b/app/assets/javascripts/diff.js @@ -1,4 +1,4 @@ -import './lib/utils/url_utility'; +import { getLocationHash } from './lib/utils/url_utility'; import FilesCommentButton from './files_comment_button'; import SingleFileDiff from './single_file_diff'; import imageDiffHelper from './image_diff/helpers/index'; @@ -31,7 +31,7 @@ export default class Diff { isBound = true; } - if (gl.utils.getLocationHash()) { + if (getLocationHash()) { this.highlightSelectedLine(); } @@ -73,7 +73,7 @@ export default class Diff { } openAnchoredDiff(cb) { - const locationHash = gl.utils.getLocationHash(); + const locationHash = getLocationHash(); const anchoredDiff = locationHash && locationHash.split('_')[0]; if (!anchoredDiff) return; @@ -128,7 +128,7 @@ export default class Diff { } // eslint-disable-next-line class-methods-use-this highlightSelectedLine() { - const hash = gl.utils.getLocationHash(); + const hash = getLocationHash(); const $diffFiles = $('.diff-file'); $diffFiles.find('.hll').removeClass('hll'); diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js index 678af8f7b7a..299e43a4e90 100644 --- a/app/assets/javascripts/dispatcher.js +++ b/app/assets/javascripts/dispatcher.js @@ -22,8 +22,8 @@ import NewCommitForm from './new_commit_form'; import Project from './project'; import projectAvatar from './project_avatar'; /* global MergeRequest */ -/* global Compare */ -/* global CompareAutocomplete */ +import Compare from './compare'; +import initCompareAutocomplete from './compare_autocomplete'; /* global ProjectFindFile */ import ProjectNew from './project_new'; import projectImport from './project_import'; @@ -622,7 +622,7 @@ import ProjectVariables from './project_variables'; projectAvatar(); switch (path[1]) { case 'compare': - new CompareAutocomplete(); + initCompareAutocomplete(); break; case 'edit': shortcut_handler = new ShortcutsNavigation(); diff --git a/app/assets/javascripts/filtered_search/filtered_search_manager.js b/app/assets/javascripts/filtered_search/filtered_search_manager.js index 69c57f923b6..2ba85c7da97 100644 --- a/app/assets/javascripts/filtered_search/filtered_search_manager.js +++ b/app/assets/javascripts/filtered_search/filtered_search_manager.js @@ -1,3 +1,4 @@ +import { visitUrl } from '../lib/utils/url_utility'; import Flash from '../flash'; import FilteredSearchContainer from './container'; import RecentSearchesRoot from './recent_searches_root'; @@ -566,7 +567,7 @@ class FilteredSearchManager { if (this.updateObject) { this.updateObject(parameterizedUrl); } else { - gl.utils.visitUrl(parameterizedUrl); + visitUrl(parameterizedUrl); } } diff --git a/app/assets/javascripts/fly_out_nav.js b/app/assets/javascripts/fly_out_nav.js index 98837c3b2a0..6110d961609 100644 --- a/app/assets/javascripts/fly_out_nav.js +++ b/app/assets/javascripts/fly_out_nav.js @@ -21,7 +21,7 @@ let headerHeight = 50; export const getHeaderHeight = () => headerHeight; -export const isSidebarCollapsed = () => sidebar && sidebar.classList.contains('sidebar-icons-only'); +export const isSidebarCollapsed = () => sidebar && sidebar.classList.contains('sidebar-collapsed-desktop'); export const canShowActiveSubItems = (el) => { if (el.classList.contains('active') && !isSidebarCollapsed()) { diff --git a/app/assets/javascripts/gl_dropdown.js b/app/assets/javascripts/gl_dropdown.js index 7ca783d3af6..cf4a70e321e 100644 --- a/app/assets/javascripts/gl_dropdown.js +++ b/app/assets/javascripts/gl_dropdown.js @@ -2,6 +2,7 @@ /* global fuzzaldrinPlus */ import _ from 'underscore'; import fuzzaldrinPlus from 'fuzzaldrin-plus'; +import { visitUrl } from './lib/utils/url_utility'; import { isObject } from './lib/utils/type_utility'; var GitLabDropdown, GitLabDropdownFilter, GitLabDropdownRemote, GitLabDropdownInput; @@ -852,7 +853,7 @@ GitLabDropdown = (function() { if ($el.length) { var href = $el.attr('href'); if (href && href !== '#') { - gl.utils.visitUrl(href); + visitUrl(href); } else { $el.trigger('click'); } diff --git a/app/assets/javascripts/groups/components/app.vue b/app/assets/javascripts/groups/components/app.vue index 2c0b6ab4ea8..241e026b84c 100644 --- a/app/assets/javascripts/groups/components/app.vue +++ b/app/assets/javascripts/groups/components/app.vue @@ -5,7 +5,7 @@ import eventHub from '../event_hub'; import { getParameterByName } from '../../lib/utils/common_utils'; import loadingIcon from '../../vue_shared/components/loading_icon.vue'; import { COMMON_STR } from '../constants'; - +import { mergeUrlParams } from '../../lib/utils/url_utility'; import groupsComponent from './groups.vue'; export default { @@ -93,7 +93,7 @@ export default { this.isLoading = false; $.scrollTo(0); - const currentPath = gl.utils.mergeUrlParams({ page }, window.location.href); + const currentPath = mergeUrlParams({ page }, window.location.href); window.history.replaceState({ page: currentPath, }, document.title, currentPath); diff --git a/app/assets/javascripts/groups/components/group_item.vue b/app/assets/javascripts/groups/components/group_item.vue index c76ce762b54..6421547bbde 100644 --- a/app/assets/javascripts/groups/components/group_item.vue +++ b/app/assets/javascripts/groups/components/group_item.vue @@ -1,4 +1,5 @@ <script> +import { visitUrl } from '../../lib/utils/url_utility'; import tooltip from '../../vue_shared/directives/tooltip'; import identicon from '../../vue_shared/components/identicon.vue'; import eventHub from '../event_hub'; @@ -60,7 +61,7 @@ export default { if (this.hasChildren) { eventHub.$emit('toggleChildren', this.group); } else { - gl.utils.visitUrl(this.group.relativePath); + visitUrl(this.group.relativePath); } } }, diff --git a/app/assets/javascripts/groups/new_group_child.js b/app/assets/javascripts/groups/new_group_child.js index 8e273579aae..a120d501e35 100644 --- a/app/assets/javascripts/groups/new_group_child.js +++ b/app/assets/javascripts/groups/new_group_child.js @@ -1,3 +1,4 @@ +import { visitUrl } from '../lib/utils/url_utility'; import DropLab from '../droplab/drop_lab'; import ISetter from '../droplab/plugins/input_setter'; @@ -54,9 +55,9 @@ export default class NewGroupChild { onClickNewGroupChildButton(e) { if (e.target.dataset.action === NEW_PROJECT) { - gl.utils.visitUrl(this.newGroupPath); + visitUrl(this.newGroupPath); } else if (e.target.dataset.action === NEW_SUBGROUP) { - gl.utils.visitUrl(this.subgroupPath); + visitUrl(this.subgroupPath); } } } diff --git a/app/assets/javascripts/issue_show/components/app.vue b/app/assets/javascripts/issue_show/components/app.vue index c7ce16bb623..fd1a50dd533 100644 --- a/app/assets/javascripts/issue_show/components/app.vue +++ b/app/assets/javascripts/issue_show/components/app.vue @@ -1,5 +1,6 @@ <script> import Visibility from 'visibilityjs'; +import { visitUrl } from '../../lib/utils/url_utility'; import Poll from '../../lib/utils/poll'; import eventHub from '../event_hub'; import Service from '../services/index'; @@ -8,7 +9,6 @@ import titleComponent from './title.vue'; import descriptionComponent from './description.vue'; import editedComponent from './edited.vue'; import formComponent from './form.vue'; -import '../../lib/utils/url_utility'; import RecaptchaDialogImplementor from '../../vue_shared/mixins/recaptcha_dialog_implementor'; export default { @@ -177,7 +177,7 @@ export default { .then(data => this.checkForSpam(data)) .then((data) => { if (location.pathname !== data.web_url) { - gl.utils.visitUrl(data.web_url); + visitUrl(data.web_url); } return this.service.getData(); @@ -212,7 +212,7 @@ export default { // Stop the poll so we don't get 404's with the issuable not existing this.poll.stop(); - gl.utils.visitUrl(data.web_url); + visitUrl(data.web_url); }) .catch(() => { eventHub.$emit('close.form'); diff --git a/app/assets/javascripts/job.js b/app/assets/javascripts/job.js index 85ea6330ee9..06b0e02a870 100644 --- a/app/assets/javascripts/job.js +++ b/app/assets/javascripts/job.js @@ -1,4 +1,5 @@ import _ from 'underscore'; +import { visitUrl } from './lib/utils/url_utility'; import bp from './breakpoints'; import { bytesToKiB } from './lib/utils/number_utils'; import { setCiStatusFavicon } from './lib/utils/common_utils'; @@ -209,7 +210,7 @@ export default class Job { } if (log.status !== this.buildStatus) { - gl.utils.visitUrl(this.pagePath); + visitUrl(this.pagePath); } }) .fail(() => { diff --git a/app/assets/javascripts/lib/utils/common_utils.js b/app/assets/javascripts/lib/utils/common_utils.js index 33cc807912c..b5328c77b25 100644 --- a/app/assets/javascripts/lib/utils/common_utils.js +++ b/app/assets/javascripts/lib/utils/common_utils.js @@ -1,3 +1,4 @@ +import { getLocationHash } from './url_utility'; export const getPagePath = (index = 0) => $('body').attr('data-page').split(':')[index]; @@ -65,7 +66,7 @@ export const disableButtonIfEmptyField = (fieldSelector, buttonSelector, eventNa // automatically adjust scroll position for hash urls taking the height of the navbar into account // https://github.com/twitter/bootstrap/issues/1768 export const handleLocationHash = () => { - let hash = window.gl.utils.getLocationHash(); + let hash = getLocationHash(); if (!hash) return; // This is required to handle non-unicode characters in hash diff --git a/app/assets/javascripts/lib/utils/url_utility.js b/app/assets/javascripts/lib/utils/url_utility.js index 17236c91490..f1ee9c8f2e5 100644 --- a/app/assets/javascripts/lib/utils/url_utility.js +++ b/app/assets/javascripts/lib/utils/url_utility.js @@ -1,93 +1,69 @@ -/* eslint-disable func-names, space-before-function-paren, wrap-iife, no-var, no-param-reassign, no-cond-assign, one-var, one-var-declaration-per-line, no-void, guard-for-in, no-restricted-syntax, prefer-template, quotes, max-len */ - -var base; -var w = window; -if (w.gl == null) { - w.gl = {}; -} -if ((base = w.gl).utils == null) { - base.utils = {}; -} // Returns an array containing the value(s) of the // of the key passed as an argument -w.gl.utils.getParameterValues = function(sParam) { - var i, sPageURL, sParameterName, sURLVariables, values; - sPageURL = decodeURIComponent(window.location.search.substring(1)); - sURLVariables = sPageURL.split('&'); - sParameterName = void 0; - values = []; - i = 0; - while (i < sURLVariables.length) { - sParameterName = sURLVariables[i].split('='); +export function getParameterValues(sParam) { + const sPageURL = decodeURIComponent(window.location.search.substring(1)); + + return sPageURL.split('&').reduce((acc, urlParam) => { + const sParameterName = urlParam.split('='); + if (sParameterName[0] === sParam) { - values.push(sParameterName[1].replace(/\+/g, ' ')); + acc.push(sParameterName[1].replace(/\+/g, ' ')); } - i += 1; - } - return values; -}; + + return acc; + }, []); +} + // @param {Object} params - url keys and value to merge // @param {String} url -w.gl.utils.mergeUrlParams = function(params, url) { - var lastChar, newUrl, paramName, paramValue, pattern; - newUrl = decodeURIComponent(url); - for (paramName in params) { - paramValue = params[paramName]; - pattern = new RegExp("\\b(" + paramName + "=).*?(&|$)"); - if (paramValue == null) { - newUrl = newUrl.replace(pattern, ''); +export function mergeUrlParams(params, url) { + let newUrl = Object.keys(params).reduce((acc, paramName) => { + const paramValue = params[paramName]; + const pattern = new RegExp(`\\b(${paramName}=).*?(&|$)`); + + if (paramValue === null) { + return acc.replace(pattern, ''); } else if (url.search(pattern) !== -1) { - newUrl = newUrl.replace(pattern, "$1" + paramValue + "$2"); - } else { - newUrl = "" + newUrl + (newUrl.indexOf('?') > 0 ? '&' : '?') + paramName + "=" + paramValue; + return acc.replace(pattern, `$1${paramValue}$2`); } - } + + return `${acc}${acc.indexOf('?') > 0 ? '&' : '?'}${paramName}=${paramValue}`; + }, decodeURIComponent(url)); + // Remove a trailing ampersand - lastChar = newUrl[newUrl.length - 1]; + const lastChar = newUrl[newUrl.length - 1]; + if (lastChar === '&') { newUrl = newUrl.slice(0, -1); } + return newUrl; -}; -// removes parameter query string from url. returns the modified url -w.gl.utils.removeParamQueryString = function(url, param) { - var urlVariables, variables; - url = decodeURIComponent(url); - urlVariables = url.split('&'); - return ((function() { - var j, len, results; - results = []; - for (j = 0, len = urlVariables.length; j < len; j += 1) { - variables = urlVariables[j]; - if (variables.indexOf(param) === -1) { - results.push(variables); - } - } - return results; - })()).join('&'); -}; -w.gl.utils.removeParams = (params) => { +} + +export function removeParamQueryString(url, param) { + const decodedUrl = decodeURIComponent(url); + const urlVariables = decodedUrl.split('&'); + + return urlVariables.filter(variable => variable.indexOf(param) === -1).join('&'); +} + +export function removeParams(params) { const url = document.createElement('a'); url.href = window.location.href; + params.forEach((param) => { - url.search = w.gl.utils.removeParamQueryString(url.search, param); + url.search = removeParamQueryString(url.search, param); }); + return url.href; -}; -w.gl.utils.getLocationHash = function(url) { - var hashIndex; - if (typeof url === 'undefined') { - // Note: We can't use window.location.hash here because it's - // not consistent across browsers - Firefox will pre-decode it - url = window.location.href; - } - hashIndex = url.indexOf('#'); - return hashIndex === -1 ? null : url.substring(hashIndex + 1); -}; +} -w.gl.utils.refreshCurrentPage = () => gl.utils.visitUrl(window.location.href); +export function getLocationHash(url = window.location.href) { + const hashIndex = url.indexOf('#'); + + return hashIndex === -1 ? null : url.substring(hashIndex + 1); +} -// eslint-disable-next-line import/prefer-default-export export function visitUrl(url, external = false) { if (external) { // Simulate `target="blank" rel="noopener noreferrer"` @@ -100,12 +76,10 @@ export function visitUrl(url, external = false) { } } +export function refreshCurrentPage() { + visitUrl(window.location.href); +} + export function redirectTo(url) { return window.location.assign(url); } - -window.gl = window.gl || {}; -window.gl.utils = { - ...(window.gl.utils || {}), - visitUrl, -}; diff --git a/app/assets/javascripts/main.js b/app/assets/javascripts/main.js index dcc0fa63b63..9e4047b6840 100644 --- a/app/assets/javascripts/main.js +++ b/app/assets/javascripts/main.js @@ -29,7 +29,7 @@ import './commit/image_file'; // lib/utils import { handleLocationHash } from './lib/utils/common_utils'; import './lib/utils/datetime_utility'; -import './lib/utils/url_utility'; +import { getLocationHash, visitUrl } from './lib/utils/url_utility'; // behaviors import './behaviors/'; @@ -40,9 +40,6 @@ import './admin'; import './aside'; import loadAwardsHandler from './awards_handler'; import bp from './breakpoints'; -import './commits'; -import './compare'; -import './compare_autocomplete'; import './confirm_danger_modal'; import Flash, { removeFlashClickListener } from './flash'; import './gl_dropdown'; @@ -122,7 +119,7 @@ $(function () { // `hashchange` is not triggered when link target is already in window.location $body.on('click', 'a[href^="#"]', function() { var href = this.getAttribute('href'); - if (href.substr(1) === gl.utils.getLocationHash()) { + if (href.substr(1) === getLocationHash()) { setTimeout(handleLocationHash, 1); } }); @@ -294,7 +291,7 @@ $(function () { const action = `${this.action}${link.search === '' ? '?' : '&'}`; event.preventDefault(); - gl.utils.visitUrl(`${action}${$(this).serialize()}`); + visitUrl(`${action}${$(this).serialize()}`); }); const flashContainer = document.querySelector('.flash-container'); diff --git a/app/assets/javascripts/merge_request_tabs.js b/app/assets/javascripts/merge_request_tabs.js index 54c1b7a268e..589e65647ac 100644 --- a/app/assets/javascripts/merge_request_tabs.js +++ b/app/assets/javascripts/merge_request_tabs.js @@ -11,6 +11,7 @@ import { handleLocationHash, isMetaClick, } from './lib/utils/common_utils'; +import { getLocationHash } from './lib/utils/url_utility'; import initDiscussionTab from './image_diff/init_discussion_tab'; import Diff from './diff'; @@ -317,7 +318,7 @@ import Diff from './diff'; // Scroll any linked note into view // Similar to `toggler_behavior` in the discussion tab - const hash = window.gl.utils.getLocationHash(); + const hash = getLocationHash(); const anchor = hash && $container.find(`.note[id="${hash}"]`); if (anchor && anchor.length > 0) { const notesContent = anchor.closest('.notes_content'); diff --git a/app/assets/javascripts/monitoring/components/dashboard.vue b/app/assets/javascripts/monitoring/components/dashboard.vue index cbe24c0915b..8da723ced03 100644 --- a/app/assets/javascripts/monitoring/components/dashboard.vue +++ b/app/assets/javascripts/monitoring/components/dashboard.vue @@ -21,6 +21,8 @@ hasMetrics: convertPermissionToBoolean(metricsData.hasMetrics), documentationPath: metricsData.documentationPath, settingsPath: metricsData.settingsPath, + tagsPath: metricsData.tagsPath, + projectPath: metricsData.projectPath, metricsEndpoint: metricsData.additionalMetrics, deploymentEndpoint: metricsData.deploymentEndpoint, emptyGettingStartedSvgPath: metricsData.emptyGettingStartedSvgPath, @@ -112,6 +114,8 @@ :hover-data="hoverData" :update-aspect-ratio="updateAspectRatio" :deployment-data="store.deploymentData" + :project-path="projectPath" + :tags-path="tagsPath" /> </graph-group> </div> diff --git a/app/assets/javascripts/monitoring/components/graph.vue b/app/assets/javascripts/monitoring/components/graph.vue index f8782fde927..cdae287658b 100644 --- a/app/assets/javascripts/monitoring/components/graph.vue +++ b/app/assets/javascripts/monitoring/components/graph.vue @@ -30,6 +30,14 @@ required: false, default: () => ({}), }, + projectPath: { + type: String, + required: true, + }, + tagsPath: { + type: String, + required: true, + }, }, mixins: [MonitoringMixin], @@ -251,6 +259,14 @@ :line-color="path.lineColor" :area-color="path.areaColor" /> + <rect + class="prometheus-graph-overlay" + :width="(graphWidth - 70)" + :height="(graphHeight - 100)" + transform="translate(-5, 20)" + ref="graphOverlay" + @mousemove="handleMouseOverGraph($event)"> + </rect> <graph-deployment :show-deploy-info="showDeployInfo" :deployment-data="reducedDeploymentData" @@ -267,14 +283,6 @@ :graph-height-offset="graphHeightOffset" :show-flag-content="showFlagContent" /> - <rect - class="prometheus-graph-overlay" - :width="(graphWidth - 70)" - :height="(graphHeight - 100)" - transform="translate(-5, 20)" - ref="graphOverlay" - @mousemove="handleMouseOverGraph($event)"> - </rect> </svg> </svg> </div> diff --git a/app/assets/javascripts/monitoring/components/graph/deployment.vue b/app/assets/javascripts/monitoring/components/graph/deployment.vue index e3b8be0c7fb..026e2fd0c49 100644 --- a/app/assets/javascripts/monitoring/components/graph/deployment.vue +++ b/app/assets/javascripts/monitoring/components/graph/deployment.vue @@ -1,5 +1,6 @@ <script> - import { dateFormat, timeFormat } from '../../utils/date_time_formatters'; + import { dateFormatWithName, timeFormat } from '../../utils/date_time_formatters'; + import Icon from '../../../vue_shared/components/icon.vue'; export default { props: { @@ -25,6 +26,10 @@ }, }, + components: { + Icon, + }, + computed: { calculatedHeight() { return this.graphHeight - this.graphHeightOffset; @@ -33,7 +38,7 @@ methods: { refText(d) { - return d.tag ? d.ref : d.sha.slice(0, 6); + return d.tag ? d.ref : d.sha.slice(0, 8); }, formatTime(deploymentTime) { @@ -41,7 +46,7 @@ }, formatDate(deploymentTime) { - return dateFormat(deploymentTime); + return dateFormatWithName(deploymentTime); }, nameDeploymentClass(deployment) { @@ -54,11 +59,19 @@ positionFlag(deployment) { let xPosition = 3; - if (deployment.xPos > (this.graphWidth - 200)) { - xPosition = -97; + if (deployment.xPos > (this.graphWidth - 225)) { + xPosition = -142; } return xPosition; }, + + svgContainerHeight(tag) { + let svgHeight = 80; + if (!tag) { + svgHeight -= 20; + } + return svgHeight; + }, }, }; </script> @@ -91,35 +104,75 @@ class="js-deploy-info-box" :x="positionFlag(deployment)" y="0" - width="92" - height="60"> + width="134" + :height="svgContainerHeight(deployment.tag)"> <rect class="rect-text-metric deploy-info-rect rect-metric" x="1" y="1" rx="2" - width="90" - height="58"> + width="132" + :height="svgContainerHeight(deployment.tag) - 2"> </rect> - <g - transform="translate(5, 2)"> - <text - class="deploy-info-text text-metric-bold"> - {{refText(deployment)}} - </text> - </g> - <text - class="deploy-info-text" - y="18" - transform="translate(5, 2)"> - {{formatDate(deployment.time)}} - </text> <text class="deploy-info-text text-metric-bold" - y="38" transform="translate(5, 2)"> - {{formatTime(deployment.time)}} + Deployed </text> + <!--The date info--> + <g transform="translate(5, 20)"> + <text class="deploy-info-text"> + {{formatDate(deployment.time)}} + </text> + <text + class="deploy-info-text text-metric-bold" + x="62"> + {{formatTime(deployment.time)}} + </text> + </g> + <line + class="divider-line" + x1="0" + y1="38" + x2="132" + :y2="38" + stroke="#000"> + </line> + <!--Commit information--> + <g transform="translate(5, 40)"> + <icon + name="commit" + :width="12" + :height="12" + :y="3"> + </icon> + <a :xlink:href="deployment.commitUrl"> + <text + class="deploy-info-text deploy-info-text-link" + transform="translate(20, 2)"> + {{refText(deployment)}} + </text> + </a> + </g> + <!--Tag information--> + <g + transform="translate(5, 55)" + v-if="deployment.tag"> + <icon + name="label" + :width="12" + :height="12" + :y="5"> + </icon> + <a :xlink:href="deployment.tagUrl"> + <text + class="deploy-info-text deploy-info-text-link" + transform="translate(20, 2)" + y="2"> + {{deployment.tag}} + </text> + </a> + </g> </svg> </g> <svg diff --git a/app/assets/javascripts/monitoring/mixins/monitoring_mixins.js b/app/assets/javascripts/monitoring/mixins/monitoring_mixins.js index 31f38aca5d6..cbca14ede02 100644 --- a/app/assets/javascripts/monitoring/mixins/monitoring_mixins.js +++ b/app/assets/javascripts/monitoring/mixins/monitoring_mixins.js @@ -33,7 +33,9 @@ const mixins = { id: deployment.id, time, sha: deployment.sha, + commitUrl: `${this.projectPath}/commit/${deployment.sha}`, tag: deployment.tag, + tagUrl: `${this.tagsPath}/${deployment.tag}`, ref: deployment.ref.name, xPos, showDeploymentFlag: false, diff --git a/app/assets/javascripts/monitoring/utils/date_time_formatters.js b/app/assets/javascripts/monitoring/utils/date_time_formatters.js index c4c6b1ac1f5..ad07a8465e2 100644 --- a/app/assets/javascripts/monitoring/utils/date_time_formatters.js +++ b/app/assets/javascripts/monitoring/utils/date_time_formatters.js @@ -1,6 +1,7 @@ import d3 from 'd3'; export const dateFormat = d3.time.format('%b %-d, %Y'); +export const dateFormatWithName = d3.time.format('%a, %b %-d'); export const timeFormat = d3.time.format('%-I:%M%p'); export const bisectDate = d3.bisector(d => d.time).left; diff --git a/app/assets/javascripts/namespace_select.js b/app/assets/javascripts/namespace_select.js index 1d496c64e53..aa377327107 100644 --- a/app/assets/javascripts/namespace_select.js +++ b/app/assets/javascripts/namespace_select.js @@ -1,6 +1,6 @@ /* eslint-disable func-names, space-before-function-paren, no-var, comma-dangle, object-shorthand, no-else-return, prefer-template, quotes, prefer-arrow-callback, max-len */ import Api from './api'; -import './lib/utils/url_utility'; +import { mergeUrlParams } from './lib/utils/url_utility'; export default class NamespaceSelect { constructor(opts) { @@ -50,7 +50,7 @@ export default class NamespaceSelect { } }, url(namespace) { - return gl.utils.mergeUrlParams({ [fieldName]: namespace.id }, window.location.href); + return mergeUrlParams({ [fieldName]: namespace.id }, window.location.href); }, }); } diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index e1ab28978e8..2a570ac705e 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -16,6 +16,7 @@ import Autosize from 'autosize'; import 'vendor/jquery.caret'; // required by jquery.atwho import 'vendor/jquery.atwho'; import AjaxCache from '~/lib/utils/ajax_cache'; +import { getLocationHash } from './lib/utils/url_utility'; import Flash from './flash'; import CommentTypeToggle from './comment_type_toggle'; import GLForm from './gl_form'; @@ -330,7 +331,7 @@ export default class Notes { } static updateNoteTargetSelector($note) { - const hash = gl.utils.getLocationHash(); + const hash = getLocationHash(); // Needs to be an explicit true/false for the jQuery `toggleClass(force)` const addTargetClass = Boolean(hash && $note.filter(`#${hash}`).length > 0); $note.toggleClass('target', addTargetClass); diff --git a/app/assets/javascripts/notes/components/noteable_note.vue b/app/assets/javascripts/notes/components/noteable_note.vue index 32e0332b711..63d48f9de69 100644 --- a/app/assets/javascripts/notes/components/noteable_note.vue +++ b/app/assets/javascripts/notes/components/noteable_note.vue @@ -1,5 +1,6 @@ <script> import { mapGetters, mapActions } from 'vuex'; + import { escape } from 'underscore'; import Flash from '../../flash'; import userAvatarLink from '../../vue_shared/components/user_avatar/user_avatar_link.vue'; import noteHeader from './note_header.vue'; @@ -85,7 +86,7 @@ }; this.isRequesting = true; this.oldContent = this.note.note_html; - this.note.note_html = noteText; + this.note.note_html = escape(noteText); this.updateNote(data) .then(() => { diff --git a/app/assets/javascripts/notes/components/notes_app.vue b/app/assets/javascripts/notes/components/notes_app.vue index e526c2491f5..c4cae4b3b6f 100644 --- a/app/assets/javascripts/notes/components/notes_app.vue +++ b/app/assets/javascripts/notes/components/notes_app.vue @@ -1,5 +1,6 @@ <script> import { mapGetters, mapActions } from 'vuex'; + import { getLocationHash } from '../../lib/utils/url_utility'; import Flash from '../../flash'; import store from '../stores/'; import * as constants from '../constants'; @@ -95,7 +96,7 @@ this.poll(); }, checkLocationHash() { - const hash = gl.utils.getLocationHash(); + const hash = getLocationHash(); const element = document.getElementById(hash); if (hash && element) { diff --git a/app/assets/javascripts/pager.js b/app/assets/javascripts/pager.js index e3fc1e2fc2f..6792b984cc5 100644 --- a/app/assets/javascripts/pager.js +++ b/app/assets/javascripts/pager.js @@ -1,5 +1,5 @@ import { getParameterByName } from '~/lib/utils/common_utils'; -import '~/lib/utils/url_utility'; +import { removeParams } from './lib/utils/url_utility'; (() => { const ENDLESS_SCROLL_BOTTOM_PX = 400; @@ -7,7 +7,7 @@ import '~/lib/utils/url_utility'; const Pager = { init(limit = 0, preload = false, disable = false, prepareData = $.noop, callback = $.noop) { - this.url = $('.content_list').data('href') || gl.utils.removeParams(['limit', 'offset']); + this.url = $('.content_list').data('href') || removeParams(['limit', 'offset']); this.limit = limit; this.offset = parseInt(getParameterByName('offset'), 10) || this.limit; this.disable = disable; diff --git a/app/assets/javascripts/performance_bar.js b/app/assets/javascripts/performance_bar.js index 9bbdf7f513c..0562a681c4b 100644 --- a/app/assets/javascripts/performance_bar.js +++ b/app/assets/javascripts/performance_bar.js @@ -1,5 +1,6 @@ import 'vendor/peek'; import 'vendor/peek.performance_bar'; +import { getParameterValues } from './lib/utils/url_utility'; export default class PerformanceBar { constructor(opts) { @@ -39,7 +40,7 @@ export default class PerformanceBar { } handleLineProfileLink(e) { - const lineProfilerParameter = gl.utils.getParameterValues('lineprofiler'); + const lineProfilerParameter = getParameterValues('lineprofiler'); const lineProfilerParameterRegex = new RegExp(`lineprofiler=${lineProfilerParameter[0]}`); const shouldToggleModal = lineProfilerParameter.length > 0 && lineProfilerParameterRegex.test(e.currentTarget.href); diff --git a/app/assets/javascripts/project.js b/app/assets/javascripts/project.js index 3131e71d9d6..d4f26b81f30 100644 --- a/app/assets/javascripts/project.js +++ b/app/assets/javascripts/project.js @@ -1,6 +1,7 @@ /* eslint-disable func-names, space-before-function-paren, no-var, consistent-return, no-new, prefer-arrow-callback, no-return-assign, one-var, one-var-declaration-per-line, object-shorthand, no-else-return, newline-per-chained-call, no-shadow, vars-on-top, prefer-template, max-len */ import Cookies from 'js-cookie'; +import { visitUrl } from './lib/utils/url_utility'; import projectSelect from './project_select'; export default class Project { @@ -122,7 +123,7 @@ export default class Project { var action = $form.attr('action'); var divider = action.indexOf('?') === -1 ? '?' : '&'; if (shouldVisit) { - gl.utils.visitUrl(`${action}${divider}${$form.serialize()}`); + visitUrl(`${action}${divider}${$form.serialize()}`); } } }, diff --git a/app/assets/javascripts/projects/project_import_gitlab_project.js b/app/assets/javascripts/projects/project_import_gitlab_project.js index c34927499fc..cec6f0dd5a3 100644 --- a/app/assets/javascripts/projects/project_import_gitlab_project.js +++ b/app/assets/javascripts/projects/project_import_gitlab_project.js @@ -1,7 +1,7 @@ -import '../lib/utils/url_utility'; +import { getParameterValues } from '../lib/utils/url_utility'; const bindEvents = () => { - const path = gl.utils.getParameterValues('path')[0]; + const path = getParameterValues('path')[0]; // get the path url and append it in the inputS $('.js-path-name').val(path); diff --git a/app/assets/javascripts/repo/stores/actions.js b/app/assets/javascripts/repo/stores/actions.js index 120ce96f44d..af5dcf054ef 100644 --- a/app/assets/javascripts/repo/stores/actions.js +++ b/app/assets/javascripts/repo/stores/actions.js @@ -1,9 +1,10 @@ import Vue from 'vue'; +import { visitUrl } from '../../lib/utils/url_utility'; import flash from '../../flash'; import service from '../services'; import * as types from './mutation_types'; -export const redirectToUrl = (_, url) => gl.utils.visitUrl(url); +export const redirectToUrl = (_, url) => visitUrl(url); export const setInitialData = ({ commit }, data) => commit(types.SET_INITIAL_DATA, data); diff --git a/app/assets/javascripts/repo/stores/actions/tree.js b/app/assets/javascripts/repo/stores/actions/tree.js index aa830e946a2..7c251e26bed 100644 --- a/app/assets/javascripts/repo/stores/actions/tree.js +++ b/app/assets/javascripts/repo/stores/actions/tree.js @@ -1,3 +1,4 @@ +import { visitUrl } from '../../../lib/utils/url_utility'; import { normalizeHeaders } from '../../../lib/utils/common_utils'; import flash from '../../../flash'; import service from '../../services'; @@ -73,7 +74,7 @@ export const clickedTreeRow = ({ commit, dispatch }, row) => { } else if (row.type === 'submodule') { commit(types.TOGGLE_LOADING, row); - gl.utils.visitUrl(row.url); + visitUrl(row.url); } else if (row.type === 'blob' && row.opened) { dispatch('setFileActive', row); } else { diff --git a/app/assets/javascripts/shortcuts.js b/app/assets/javascripts/shortcuts.js index ebe7a99ffae..130730b1700 100644 --- a/app/assets/javascripts/shortcuts.js +++ b/app/assets/javascripts/shortcuts.js @@ -1,5 +1,6 @@ import Cookies from 'js-cookie'; import Mousetrap from 'mousetrap'; +import { refreshCurrentPage, visitUrl } from './lib/utils/url_utility'; import findAndFollowLink from './shortcuts_dashboard_navigation'; const defaultStopCallback = Mousetrap.stopCallback; @@ -38,7 +39,7 @@ export default class Shortcuts { if (typeof findFileURL !== 'undefined' && findFileURL !== null) { Mousetrap.bind('t', () => { - gl.utils.visitUrl(findFileURL); + visitUrl(findFileURL); }); } @@ -62,7 +63,7 @@ export default class Shortcuts { } else { Cookies.set(performanceBarCookieName, 'true', { path: '/' }); } - gl.utils.refreshCurrentPage(); + refreshCurrentPage(); } static toggleMarkdownPreview(e) { diff --git a/app/assets/javascripts/shortcuts_blob.js b/app/assets/javascripts/shortcuts_blob.js index fbc57bb4304..cf309be4f6f 100644 --- a/app/assets/javascripts/shortcuts_blob.js +++ b/app/assets/javascripts/shortcuts_blob.js @@ -1,5 +1,5 @@ /* global Mousetrap */ - +import { getLocationHash, visitUrl } from './lib/utils/url_utility'; import Shortcuts from './shortcuts'; const defaults = { @@ -18,9 +18,9 @@ export default class ShortcutsBlob extends Shortcuts { moveToFilePermalink() { if (this.options.fileBlobPermalinkUrl) { - const hash = gl.utils.getLocationHash(); + const hash = getLocationHash(); const hashUrlString = hash ? `#${hash}` : ''; - gl.utils.visitUrl(`${this.options.fileBlobPermalinkUrl}${hashUrlString}`); + visitUrl(`${this.options.fileBlobPermalinkUrl}${hashUrlString}`); } } } diff --git a/app/assets/javascripts/sidebar/components/assignees/sidebar_assignees.js b/app/assets/javascripts/sidebar/components/assignees/sidebar_assignees.js index 74c17bc14a2..9e47039d920 100644 --- a/app/assets/javascripts/sidebar/components/assignees/sidebar_assignees.js +++ b/app/assets/javascripts/sidebar/components/assignees/sidebar_assignees.js @@ -1,22 +1,32 @@ import Flash from '../../../flash'; import AssigneeTitle from './assignee_title'; import Assignees from './assignees'; - import Store from '../../stores/sidebar_store'; -import Mediator from '../../sidebar_mediator'; - import eventHub from '../../event_hub'; export default { name: 'SidebarAssignees', data() { return { - mediator: new Mediator(), store: new Store(), loading: false, - field: '', }; }, + props: { + mediator: { + type: Object, + required: true, + }, + field: { + type: String, + required: true, + }, + signedIn: { + type: Boolean, + required: false, + default: false, + }, + }, components: { 'assignee-title': AssigneeTitle, assignees: Assignees, @@ -61,10 +71,6 @@ export default { eventHub.$off('sidebar.removeAllAssignees', this.removeAllAssignees); eventHub.$off('sidebar.saveAssignees', this.saveAssignees); }, - beforeMount() { - this.field = this.$el.dataset.field; - this.signedIn = typeof this.$el.dataset.signedIn !== 'undefined'; - }, template: ` <div> <assignee-title diff --git a/app/assets/javascripts/sidebar/components/participants/sidebar_participants.vue b/app/assets/javascripts/sidebar/components/participants/sidebar_participants.vue index c1296b28db7..6fcd2f95309 100644 --- a/app/assets/javascripts/sidebar/components/participants/sidebar_participants.vue +++ b/app/assets/javascripts/sidebar/components/participants/sidebar_participants.vue @@ -1,15 +1,19 @@ <script> import Store from '../../stores/sidebar_store'; -import Mediator from '../../sidebar_mediator'; import participants from './participants.vue'; export default { data() { return { - mediator: new Mediator(), store: new Store(), }; }, + props: { + mediator: { + type: Object, + required: true, + }, + }, components: { participants, }, @@ -21,6 +25,7 @@ export default { <participants :loading="store.isFetching.participants" :participants="store.participants" - :number-of-less-participants="7" /> + :number-of-less-participants="7" + /> </div> </template> diff --git a/app/assets/javascripts/sidebar/components/subscriptions/sidebar_subscriptions.vue b/app/assets/javascripts/sidebar/components/subscriptions/sidebar_subscriptions.vue index 25acc099699..f4bae1d3dd5 100644 --- a/app/assets/javascripts/sidebar/components/subscriptions/sidebar_subscriptions.vue +++ b/app/assets/javascripts/sidebar/components/subscriptions/sidebar_subscriptions.vue @@ -1,6 +1,5 @@ <script> import Store from '../../stores/sidebar_store'; -import Mediator from '../../sidebar_mediator'; import eventHub from '../../event_hub'; import Flash from '../../../flash'; import { __ } from '../../../locale'; @@ -9,11 +8,15 @@ import subscriptions from './subscriptions.vue'; export default { data() { return { - mediator: new Mediator(), store: new Store(), }; }, - + props: { + mediator: { + type: Object, + required: true, + }, + }, components: { subscriptions, }, diff --git a/app/assets/javascripts/sidebar/mount_sidebar.js b/app/assets/javascripts/sidebar/mount_sidebar.js index 4032f156b15..56cc78ca0ca 100644 --- a/app/assets/javascripts/sidebar/mount_sidebar.js +++ b/app/assets/javascripts/sidebar/mount_sidebar.js @@ -10,6 +10,27 @@ import Translate from '../vue_shared/translate'; Vue.use(Translate); +function mountAssigneesComponent(mediator) { + const el = document.getElementById('js-vue-sidebar-assignees'); + + if (!el) return; + + // eslint-disable-next-line no-new + new Vue({ + el, + components: { + SidebarAssignees, + }, + render: createElement => createElement('sidebar-assignees', { + props: { + mediator, + field: el.dataset.field, + signedIn: el.hasAttribute('data-signed-in'), + }, + }), + }); +} + function mountConfidentialComponent(mediator) { const el = document.getElementById('js-confidential-entry-point'); @@ -49,9 +70,10 @@ function mountLockComponent(mediator) { }).$mount(el); } -function mountParticipantsComponent() { +function mountParticipantsComponent(mediator) { const el = document.querySelector('.js-sidebar-participants-entry-point'); + // eslint-disable-next-line no-new if (!el) return; // eslint-disable-next-line no-new @@ -60,11 +82,15 @@ function mountParticipantsComponent() { components: { sidebarParticipants, }, - render: createElement => createElement('sidebar-participants', {}), + render: createElement => createElement('sidebar-participants', { + props: { + mediator, + }, + }), }); } -function mountSubscriptionsComponent() { +function mountSubscriptionsComponent(mediator) { const el = document.querySelector('.js-sidebar-subscriptions-entry-point'); if (!el) return; @@ -75,22 +101,35 @@ function mountSubscriptionsComponent() { components: { sidebarSubscriptions, }, - render: createElement => createElement('sidebar-subscriptions', {}), + render: createElement => createElement('sidebar-subscriptions', { + props: { + mediator, + }, + }), }); } -function mount(mediator) { - const sidebarAssigneesEl = document.getElementById('js-vue-sidebar-assignees'); - // Only create the sidebarAssignees vue app if it is found in the DOM - // We currently do not use sidebarAssignees for the MR page - if (sidebarAssigneesEl) { - new Vue(SidebarAssignees).$mount(sidebarAssigneesEl); - } +function mountTimeTrackingComponent() { + const el = document.getElementById('issuable-time-tracker'); + if (!el) return; + + // eslint-disable-next-line no-new + new Vue({ + el, + components: { + SidebarTimeTracking, + }, + render: createElement => createElement('sidebar-time-tracking', {}), + }); +} + +export function mountSidebar(mediator) { + mountAssigneesComponent(mediator); mountConfidentialComponent(mediator); mountLockComponent(mediator); - mountParticipantsComponent(); - mountSubscriptionsComponent(); + mountParticipantsComponent(mediator); + mountSubscriptionsComponent(mediator); new SidebarMoveIssue( mediator, @@ -98,7 +137,9 @@ function mount(mediator) { $('.js-move-issue-confirmation-button'), ).init(); - new Vue(SidebarTimeTracking).$mount('#issuable-time-tracker'); + mountTimeTrackingComponent(); } -export default mount; +export function getSidebarOptions() { + return JSON.parse(document.querySelector('.js-sidebar-options').innerHTML); +} diff --git a/app/assets/javascripts/sidebar/sidebar_bundle.js b/app/assets/javascripts/sidebar/sidebar_bundle.js index f78287e504b..04c39d7b6b5 100644 --- a/app/assets/javascripts/sidebar/sidebar_bundle.js +++ b/app/assets/javascripts/sidebar/sidebar_bundle.js @@ -1,9 +1,8 @@ import Mediator from './sidebar_mediator'; -import mountSidebar from './mount_sidebar'; +import { mountSidebar, getSidebarOptions } from './mount_sidebar'; function domContentLoaded() { - const sidebarOptions = JSON.parse(document.querySelector('.js-sidebar-options').innerHTML); - const mediator = new Mediator(sidebarOptions); + const mediator = new Mediator(getSidebarOptions()); mediator.fetch(); mountSidebar(mediator); diff --git a/app/assets/javascripts/sidebar/sidebar_mediator.js b/app/assets/javascripts/sidebar/sidebar_mediator.js index d4c07a188b3..d86557e870a 100644 --- a/app/assets/javascripts/sidebar/sidebar_mediator.js +++ b/app/assets/javascripts/sidebar/sidebar_mediator.js @@ -1,3 +1,4 @@ +import { visitUrl } from '../lib/utils/url_utility'; import Flash from '../flash'; import Service from './services/sidebar_service'; import Store from './stores/sidebar_store'; @@ -7,7 +8,6 @@ export default class SidebarMediator { if (!SidebarMediator.singleton) { this.initSingleton(options); } - return SidebarMediator.singleton; } @@ -81,7 +81,7 @@ export default class SidebarMediator { .then(response => response.json()) .then((data) => { if (location.pathname !== data.web_url) { - gl.utils.visitUrl(data.web_url); + visitUrl(data.web_url); } }); } diff --git a/app/assets/javascripts/sidebar/stores/sidebar_store.js b/app/assets/javascripts/sidebar/stores/sidebar_store.js index 73eb25e2333..f20cc6d8cca 100644 --- a/app/assets/javascripts/sidebar/stores/sidebar_store.js +++ b/app/assets/javascripts/sidebar/stores/sidebar_store.js @@ -1,33 +1,37 @@ export default class SidebarStore { - constructor(store) { + constructor(options) { if (!SidebarStore.singleton) { - const { currentUser, rootPath, editable } = store; - this.currentUser = currentUser; - this.rootPath = rootPath; - this.editable = editable; - this.timeEstimate = 0; - this.totalTimeSpent = 0; - this.humanTimeEstimate = ''; - this.humanTimeSpent = ''; - this.assignees = []; - this.isFetching = { - assignees: true, - participants: true, - subscriptions: true, - }; - this.isLoading = {}; - this.autocompleteProjects = []; - this.moveToProjectId = 0; - this.isLockDialogOpen = false; - this.participants = []; - this.subscribed = null; - - SidebarStore.singleton = this; + this.initSingleton(options); } return SidebarStore.singleton; } + initSingleton(options) { + const { currentUser, rootPath, editable } = options; + this.currentUser = currentUser; + this.rootPath = rootPath; + this.editable = editable; + this.timeEstimate = 0; + this.totalTimeSpent = 0; + this.humanTimeEstimate = ''; + this.humanTimeSpent = ''; + this.assignees = []; + this.isFetching = { + assignees: true, + participants: true, + subscriptions: true, + }; + this.isLoading = {}; + this.autocompleteProjects = []; + this.moveToProjectId = 0; + this.isLockDialogOpen = false; + this.participants = []; + this.subscribed = null; + + SidebarStore.singleton = this; + } + setAssigneeData(data) { this.isFetching.assignees = false; if (data.assignees) { diff --git a/app/assets/javascripts/todos.js b/app/assets/javascripts/todos.js index 2fffe09c74e..748caecf153 100644 --- a/app/assets/javascripts/todos.js +++ b/app/assets/javascripts/todos.js @@ -1,5 +1,5 @@ /* eslint-disable class-methods-use-this, no-unneeded-ternary, quote-props */ - +import { visitUrl } from './lib/utils/url_utility'; import UsersSelect from './users_select'; import { isMetaClick } from './lib/utils/common_utils'; @@ -150,7 +150,7 @@ export default class Todos { window.open(todoLink, windowTarget); } else { - gl.utils.visitUrl(todoLink); + visitUrl(todoLink); } } } diff --git a/app/assets/javascripts/tree.js b/app/assets/javascripts/tree.js index 7777ed1c3dc..1a0b2c0415b 100644 --- a/app/assets/javascripts/tree.js +++ b/app/assets/javascripts/tree.js @@ -1,4 +1,5 @@ /* eslint-disable func-names, space-before-function-paren, wrap-iife, max-len, quotes, consistent-return, no-var, one-var, one-var-declaration-per-line, no-else-return, prefer-arrow-callback, class-methods-use-this */ +import { visitUrl } from './lib/utils/url_utility'; export default class TreeView { constructor() { @@ -14,7 +15,7 @@ export default class TreeView { e.preventDefault(); return window.open(path, '_blank'); } else { - return gl.utils.visitUrl(path); + return visitUrl(path); } } }); @@ -56,7 +57,7 @@ export default class TreeView { } else if (e.which === 13) { path = $('.tree-item.selected .tree-item-file-name a').attr('href'); if (path) { - return gl.utils.visitUrl(path); + return visitUrl(path); } } }); diff --git a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_deployment.js b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_deployment.js index e86a0f7e749..32028a4a609 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_deployment.js +++ b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_deployment.js @@ -1,4 +1,5 @@ import '~/lib/utils/datetime_utility'; +import { visitUrl } from '../../lib/utils/url_utility'; import Flash from '../../flash'; import MemoryUsage from './mr_widget_memory_usage'; import StatusIcon from './mr_widget_status_icon'; @@ -36,7 +37,7 @@ export default { .then(res => res.json()) .then((res) => { if (res.redirect_url) { - gl.utils.visitUrl(res.redirect_url); + visitUrl(res.redirect_url); } }) .catch(() => { diff --git a/app/assets/javascripts/vue_shared/components/icon.vue b/app/assets/javascripts/vue_shared/components/icon.vue index 4216660da8c..365229ea274 100644 --- a/app/assets/javascripts/vue_shared/components/icon.vue +++ b/app/assets/javascripts/vue_shared/components/icon.vue @@ -36,6 +36,30 @@ required: false, default: '', }, + + width: { + type: Number, + required: false, + default: null, + }, + + height: { + type: Number, + required: false, + default: null, + }, + + y: { + type: Number, + required: false, + default: null, + }, + + x: { + type: Number, + required: false, + default: null, + }, }, computed: { @@ -51,7 +75,11 @@ <template> <svg - :class="[iconSizeClass, cssClasses]"> + :class="[iconSizeClass, cssClasses]" + :width="width" + :height="height" + :x="x" + :y="y"> <use v-bind="{'xlink:href':spriteHref}"/> </svg> diff --git a/app/assets/stylesheets/framework/contextual-sidebar.scss b/app/assets/stylesheets/framework/contextual-sidebar.scss index b73932eb7e1..26a2db99e0a 100644 --- a/app/assets/stylesheets/framework/contextual-sidebar.scss +++ b/app/assets/stylesheets/framework/contextual-sidebar.scss @@ -1,4 +1,6 @@ .page-with-contextual-sidebar { + transition: padding-left $sidebar-transition-duration; + @media (min-width: $screen-md-min) { padding-left: $contextual-sidebar-collapsed-width; } @@ -27,8 +29,10 @@ .context-header { position: relative; margin-right: 2px; + width: $contextual-sidebar-width; a { + transition: padding $sidebar-transition-duration; font-weight: $gl-font-weight-bold; display: flex; align-items: center; @@ -63,10 +67,10 @@ } .nav-sidebar { + transition: width $sidebar-transition-duration, left $sidebar-transition-duration; position: fixed; z-index: 400; width: $contextual-sidebar-width; - transition: left $sidebar-transition-duration; top: $header-height; bottom: 0; left: 0; @@ -74,16 +78,15 @@ box-shadow: inset -2px 0 0 $border-color; transform: translate3d(0, 0, 0); - &:not(.sidebar-icons-only) { + &:not(.sidebar-collapsed-desktop) { @media (min-width: $screen-sm-min) and (max-width: $screen-md-max) { box-shadow: inset -2px 0 0 $border-color, 2px 1px 3px $dropdown-shadow-color; } } - &.sidebar-icons-only { - width: auto; - min-width: $contextual-sidebar-collapsed-width; + &.sidebar-collapsed-desktop { + width: $contextual-sidebar-collapsed-width; .nav-sidebar-inner-scroll { overflow-x: hidden; @@ -108,12 +111,11 @@ } } - &.nav-sidebar-expanded { + &.sidebar-expanded-mobile { left: 0; } a { - transition: none; text-decoration: none; } @@ -126,9 +128,10 @@ white-space: nowrap; a { + transition: padding $sidebar-transition-duration; display: flex; align-items: center; - padding: 12px 16px; + padding: 12px 15px; color: $gl-text-color-secondary; } @@ -288,7 +291,8 @@ > a { margin-left: 4px; - padding-left: 12px; + // Subtract width of left border on active element + padding-left: 11px; } .badge { @@ -313,6 +317,7 @@ .toggle-sidebar-button, .close-nav-button { width: $contextual-sidebar-width - 2px; + transition: width $sidebar-transition-duration; position: fixed; bottom: 0; padding: 16px; @@ -343,20 +348,21 @@ } } +.collapse-text { + white-space: nowrap; + overflow: hidden; +} -.sidebar-icons-only { +.sidebar-collapsed-desktop { .context-header { - height: 61px; + height: 60px; + width: $contextual-sidebar-collapsed-width; a { padding: 10px 4px; } } - li a { - padding: 12px 15px; - } - .sidebar-top-level-items > li { &.active a { padding-left: 12px; @@ -374,8 +380,8 @@ } .toggle-sidebar-button { - width: $contextual-sidebar-collapsed-width - 2px; padding: 16px; + width: $contextual-sidebar-collapsed-width - 2px; .collapse-text, .icon-angle-double-left { diff --git a/app/assets/stylesheets/framework/sidebar.scss b/app/assets/stylesheets/framework/sidebar.scss index 792981fdc48..3ebba4f9efb 100644 --- a/app/assets/stylesheets/framework/sidebar.scss +++ b/app/assets/stylesheets/framework/sidebar.scss @@ -50,6 +50,11 @@ &:not(.disabled) { cursor: pointer; } + + svg { + width: $gl-padding; + height: $gl-padding; + } } } diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index 4f99c27eff1..a6fa96d834c 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -5,10 +5,9 @@ $grid-size: 8px; $gutter_collapsed_width: 62px; $gutter_width: 290px; $gutter_inner_width: 250px; -$sidebar-transition-duration: .15s; +$sidebar-transition-duration: .3s; $sidebar-breakpoint: 1024px; $default-transition-duration: .15s; -$right-sidebar-transition-duration: .3s; $contextual-sidebar-width: 220px; $contextual-sidebar-collapsed-width: 50px; diff --git a/app/assets/stylesheets/pages/boards.scss b/app/assets/stylesheets/pages/boards.scss index 3683afa07de..862ea379cbc 100644 --- a/app/assets/stylesheets/pages/boards.scss +++ b/app/assets/stylesheets/pages/boards.scss @@ -57,7 +57,7 @@ position: relative; @media (min-width: $screen-sm-min) { - transition: width $right-sidebar-transition-duration; + transition: width $sidebar-transition-duration; width: 100%; &.is-compact { @@ -453,8 +453,8 @@ .right-sidebar.right-sidebar-expanded { &.boards-sidebar-slide-enter-active, &.boards-sidebar-slide-leave-active { - transition: width $right-sidebar-transition-duration, - padding $right-sidebar-transition-duration; + transition: width $sidebar-transition-duration, + padding $sidebar-transition-duration; } &.boards-sidebar-slide-enter, diff --git a/app/assets/stylesheets/pages/environments.scss b/app/assets/stylesheets/pages/environments.scss index b0795353ec1..a5a6b7461a3 100644 --- a/app/assets/stylesheets/pages/environments.scss +++ b/app/assets/stylesheets/pages/environments.scss @@ -201,8 +201,9 @@ stroke-width: 1; } -.deploy-info-text { - dominant-baseline: text-before-edge; +.divider-line { + stroke-width: 1; + stroke: $gray-darkest; } .prometheus-state { @@ -312,6 +313,20 @@ stroke: $gray-darker; } + .deploy-info-text { + dominant-baseline: text-before-edge; + font-size: 12px; + } + + .deploy-info-text-link { + font-family: $monospace_font; + fill: $gl-link-color; + + &:hover { + fill: $gl-link-hover-color; + } + } + @media (max-width: $screen-sm-max) { .label-axis-text, .text-metric-usage, diff --git a/app/assets/stylesheets/pages/issuable.scss b/app/assets/stylesheets/pages/issuable.scss index 11ee1232bfe..2e26f2b5bbd 100644 --- a/app/assets/stylesheets/pages/issuable.scss +++ b/app/assets/stylesheets/pages/issuable.scss @@ -126,7 +126,7 @@ top: $header-height; bottom: 0; right: 0; - transition: width $right-sidebar-transition-duration; + transition: width $sidebar-transition-duration; background: $gray-light; z-index: 200; overflow: hidden; @@ -470,7 +470,8 @@ } } - .milestone-title span { + .milestone-title span, + .collapse-truncated-title { @include str-truncated(100%); display: block; margin: 0 4px; diff --git a/app/controllers/concerns/issuable_actions.rb b/app/controllers/concerns/issuable_actions.rb index ecac9be0360..281756af57a 100644 --- a/app/controllers/concerns/issuable_actions.rb +++ b/app/controllers/concerns/issuable_actions.rb @@ -21,11 +21,11 @@ module IssuableActions respond_to do |format| format.html do - recaptcha_check_with_fallback { render :edit } + recaptcha_check_if_spammable { render :edit } end format.json do - recaptcha_check_with_fallback(false) { render_entity_json } + recaptcha_check_if_spammable(false) { render_entity_json } end end @@ -80,6 +80,12 @@ module IssuableActions private + def recaptcha_check_if_spammable(should_redirect = true, &block) + return yield unless @issuable.is_a? Spammable + + recaptcha_check_with_fallback(should_redirect, &block) + end + def render_conflict_response respond_to do |format| format.html do diff --git a/app/controllers/groups/group_members_controller.rb b/app/controllers/groups/group_members_controller.rb index 8fc234a62b1..5919bf54468 100644 --- a/app/controllers/groups/group_members_controller.rb +++ b/app/controllers/groups/group_members_controller.rb @@ -22,7 +22,7 @@ class Groups::GroupMembersController < Groups::ApplicationController end def update - @group_member = @group.group_members.find(params[:id]) + @group_member = @group.members_and_requesters.find(params[:id]) return render_403 unless can?(current_user, :update_group_member, @group_member) diff --git a/app/controllers/projects/project_members_controller.rb b/app/controllers/projects/project_members_controller.rb index d925dcd21ff..5a01a59481b 100644 --- a/app/controllers/projects/project_members_controller.rb +++ b/app/controllers/projects/project_members_controller.rb @@ -26,7 +26,7 @@ class Projects::ProjectMembersController < Projects::ApplicationController end def update - @project_member = @project.project_members.find(params[:id]) + @project_member = @project.members_and_requesters.find(params[:id]) return render_403 unless can?(current_user, :update_project_member, @project_member) diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 3882fa4791d..8e9d6766d80 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -272,7 +272,7 @@ class ProjectsController < Projects::ApplicationController render 'projects/empty' if @project.empty_repo? else - if @project.wiki_enabled? + if can?(current_user, :read_wiki, @project) @project_wiki = @project.wiki @wiki_home = @project_wiki.find_page('home', params[:version_id]) elsif @project.feature_available?(:issues, current_user) diff --git a/app/helpers/preferences_helper.rb b/app/helpers/preferences_helper.rb index 8e822ed0ea2..aaee6eaeedd 100644 --- a/app/helpers/preferences_helper.rb +++ b/app/helpers/preferences_helper.rb @@ -58,7 +58,7 @@ module PreferencesHelper user_view elsif user_view == "activity" "activity" - elsif @project.wiki_enabled? + elsif can?(current_user, :read_wiki, @project) "wiki" elsif @project.feature_available?(:issues, current_user) "projects/issues/issues" diff --git a/app/models/event.rb b/app/models/event.rb index 0997b056c6a..6053594fab5 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -72,7 +72,7 @@ class Event < ActiveRecord::Base # We're using preload for "push_event_payload" as otherwise the association # is not always available (depending on the query being built). includes(:author, :project, project: :namespace) - .preload(:target, :push_event_payload) + .preload(:push_event_payload, target: :author) end scope :for_milestone_id, ->(milestone_id) { where(target_type: "Milestone", target_id: milestone_id) } diff --git a/app/models/issue.rb b/app/models/issue.rb index 33db197e612..bbda848c39d 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -10,6 +10,9 @@ class Issue < ActiveRecord::Base include RelativePositioning include TimeTrackable include ThrottledTouch + include IgnorableColumn + + ignore_column :assignee_id DueDateStruct = Struct.new(:title, :name).freeze NoDueDate = DueDateStruct.new('No Due Date', '0').freeze diff --git a/app/models/namespace.rb b/app/models/namespace.rb index 901dbf2ba69..0ff169d4531 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -40,6 +40,7 @@ class Namespace < ActiveRecord::Base namespace_path: true validate :nesting_level_allowed + validate :allowed_path_by_redirects delegate :name, to: :owner, allow_nil: true, prefix: true @@ -257,4 +258,14 @@ class Namespace < ActiveRecord::Base Namespace.where(id: descendants.select(:id)) .update_all(share_with_group_lock: true) end + + def allowed_path_by_redirects + return if path.nil? + + errors.add(:path, "#{path} has been taken before. Please use another one") if namespace_previously_created_with_same_path? + end + + def namespace_previously_created_with_same_path? + RedirectRoute.permanent.exists?(path: path) + end end diff --git a/app/models/redirect_route.rb b/app/models/redirect_route.rb index 31de204d824..20532527346 100644 --- a/app/models/redirect_route.rb +++ b/app/models/redirect_route.rb @@ -17,4 +17,32 @@ class RedirectRoute < ActiveRecord::Base where(wheres, path, "#{sanitize_sql_like(path)}/%") end + + scope :permanent, -> do + if column_permanent_exists? + where(permanent: true) + else + none + end + end + + scope :temporary, -> do + if column_permanent_exists? + where(permanent: [false, nil]) + else + all + end + end + + default_value_for :permanent, false + + def permanent=(value) + if self.class.column_permanent_exists? + super + end + end + + def self.column_permanent_exists? + ActiveRecord::Base.connection.column_exists?(:redirect_routes, :permanent) + end end diff --git a/app/models/repository.rb b/app/models/repository.rb index 751306188a0..d1ae3304e4a 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -971,8 +971,7 @@ class Repository tmp_remote_name = true end - add_remote(remote_name, url) - set_remote_as_mirror(remote_name, refmap: refmap) + add_remote(remote_name, url, mirror_refmap: refmap) fetch_remote(remote_name, forced: forced) ensure remove_remote(remote_name) if tmp_remote_name diff --git a/app/models/route.rb b/app/models/route.rb index 97e8a6ad9e9..7ba3ec06041 100644 --- a/app/models/route.rb +++ b/app/models/route.rb @@ -8,6 +8,8 @@ class Route < ActiveRecord::Base presence: true, uniqueness: { case_sensitive: false } + validate :ensure_permanent_paths + after_create :delete_conflicting_redirects after_update :delete_conflicting_redirects, if: :path_changed? after_update :create_redirect_for_old_path @@ -40,7 +42,7 @@ class Route < ActiveRecord::Base # We are not calling route.delete_conflicting_redirects here, in hopes # of avoiding deadlocks. The parent (self, in this method) already # called it, which deletes conflicts for all descendants. - route.create_redirect(old_path) if attributes[:path] + route.create_redirect(old_path, permanent: permanent_redirect?) if attributes[:path] end end end @@ -50,16 +52,30 @@ class Route < ActiveRecord::Base end def conflicting_redirects - RedirectRoute.matching_path_and_descendants(path) + RedirectRoute.temporary.matching_path_and_descendants(path) end - def create_redirect(path) - RedirectRoute.create(source: source, path: path) + def create_redirect(path, permanent: false) + RedirectRoute.create(source: source, path: path, permanent: permanent) end private def create_redirect_for_old_path - create_redirect(path_was) if path_changed? + create_redirect(path_was, permanent: permanent_redirect?) if path_changed? + end + + def permanent_redirect? + source_type != "Project" + end + + def ensure_permanent_paths + return if path.nil? + + errors.add(:path, "#{path} has been taken before. Please use another one") if conflicting_redirect_exists? + end + + def conflicting_redirect_exists? + RedirectRoute.permanent.matching_path_and_descendants(path).exists? end end diff --git a/app/models/user.rb b/app/models/user.rb index af1c36d9c93..92b461ce3ed 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -315,6 +315,8 @@ class User < ActiveRecord::Base # # Returns an ActiveRecord::Relation. def search(query) + query = query.downcase + order = <<~SQL CASE WHEN users.name = %{query} THEN 0 @@ -324,8 +326,11 @@ class User < ActiveRecord::Base END SQL - fuzzy_search(query, [:name, :email, :username]) - .reorder(order % { query: ActiveRecord::Base.connection.quote(query) }, :name) + where( + fuzzy_arel_match(:name, query) + .or(fuzzy_arel_match(:username, query)) + .or(arel_table[:email].eq(query)) + ).reorder(order % { query: ActiveRecord::Base.connection.quote(query) }, :name) end # searches user by given pattern @@ -333,15 +338,17 @@ class User < ActiveRecord::Base # This method uses ILIKE on PostgreSQL and LIKE on MySQL. def search_with_secondary_emails(query) + query = query.downcase + email_table = Email.arel_table matched_by_emails_user_ids = email_table .project(email_table[:user_id]) - .where(Email.fuzzy_arel_match(:email, query)) + .where(email_table[:email].eq(query)) where( fuzzy_arel_match(:name, query) - .or(fuzzy_arel_match(:email, query)) .or(fuzzy_arel_match(:username, query)) + .or(arel_table[:email].eq(query)) .or(arel_table[:id].in(matched_by_emails_user_ids)) ) end @@ -1054,13 +1061,13 @@ class User < ActiveRecord::Base end def todos_done_count(force: false) - Rails.cache.fetch(['users', id, 'todos_done_count'], force: force) do + Rails.cache.fetch(['users', id, 'todos_done_count'], force: force, expires_in: 20.minutes) do TodosFinder.new(self, state: :done).execute.count end end def todos_pending_count(force: false) - Rails.cache.fetch(['users', id, 'todos_pending_count'], force: force) do + Rails.cache.fetch(['users', id, 'todos_pending_count'], force: force, expires_in: 20.minutes) do TodosFinder.new(self, state: :pending).execute.count end end diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index 1e5f2ed4dd2..85db2760e23 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -12,18 +12,19 @@ module Ci def execute(source, ignore_skip_ci: false, save_on_errors: true, trigger_request: nil, schedule: nil, &block) @pipeline = Ci::Pipeline.new - command = OpenStruct.new(source: source, - origin_ref: params[:ref], - checkout_sha: params[:checkout_sha], - after_sha: params[:after], - before_sha: params[:before], - trigger_request: trigger_request, - schedule: schedule, - ignore_skip_ci: ignore_skip_ci, - save_incompleted: save_on_errors, - seeds_block: block, - project: project, - current_user: current_user) + command = Gitlab::Ci::Pipeline::Chain::Command.new( + source: source, + origin_ref: params[:ref], + checkout_sha: params[:checkout_sha], + after_sha: params[:after], + before_sha: params[:before], + trigger_request: trigger_request, + schedule: schedule, + ignore_skip_ci: ignore_skip_ci, + save_incompleted: save_on_errors, + seeds_block: block, + project: project, + current_user: current_user) sequence = Gitlab::Ci::Pipeline::Chain::Sequence .new(pipeline, command, SEQUENCE) diff --git a/app/views/layouts/nav/sidebar/_admin.html.haml b/app/views/layouts/nav/sidebar/_admin.html.haml index 0ec07605631..cb8db306b56 100644 --- a/app/views/layouts/nav/sidebar/_admin.html.haml +++ b/app/views/layouts/nav/sidebar/_admin.html.haml @@ -1,4 +1,4 @@ -.nav-sidebar{ class: ("sidebar-icons-only" if collapsed_sidebar?) } +.nav-sidebar{ class: ("sidebar-collapsed-desktop" if collapsed_sidebar?) } .nav-sidebar-inner-scroll .context-header = link_to admin_root_path, title: 'Admin Overview' do diff --git a/app/views/layouts/nav/sidebar/_group.html.haml b/app/views/layouts/nav/sidebar/_group.html.haml index 0bf318b0b66..0c27b09f7b1 100644 --- a/app/views/layouts/nav/sidebar/_group.html.haml +++ b/app/views/layouts/nav/sidebar/_group.html.haml @@ -1,7 +1,7 @@ - issues = IssuesFinder.new(current_user, group_id: @group.id, state: 'opened').execute - merge_requests = MergeRequestsFinder.new(current_user, group_id: @group.id, state: 'opened', non_archived: true).execute -.nav-sidebar{ class: ("sidebar-icons-only" if collapsed_sidebar?) } +.nav-sidebar{ class: ("sidebar-collapsed-desktop" if collapsed_sidebar?) } .nav-sidebar-inner-scroll .context-header = link_to group_path(@group), title: @group.name do diff --git a/app/views/layouts/nav/sidebar/_profile.html.haml b/app/views/layouts/nav/sidebar/_profile.html.haml index 7e23f9c1f05..a5a62a0695f 100644 --- a/app/views/layouts/nav/sidebar/_profile.html.haml +++ b/app/views/layouts/nav/sidebar/_profile.html.haml @@ -1,4 +1,4 @@ -.nav-sidebar{ class: ("sidebar-icons-only" if collapsed_sidebar?) } +.nav-sidebar{ class: ("sidebar-collapsed-desktop" if collapsed_sidebar?) } .nav-sidebar-inner-scroll .context-header = link_to profile_path, title: 'Profile Settings' do diff --git a/app/views/layouts/nav/sidebar/_project.html.haml b/app/views/layouts/nav/sidebar/_project.html.haml index 53a9162b703..be39f577ba7 100644 --- a/app/views/layouts/nav/sidebar/_project.html.haml +++ b/app/views/layouts/nav/sidebar/_project.html.haml @@ -1,4 +1,4 @@ -.nav-sidebar{ class: ("sidebar-icons-only" if collapsed_sidebar?) } +.nav-sidebar{ class: ("sidebar-collapsed-desktop" if collapsed_sidebar?) } .nav-sidebar-inner-scroll - can_edit = can?(current_user, :admin_project, @project) .context-header diff --git a/app/views/projects/environments/metrics.html.haml b/app/views/projects/environments/metrics.html.haml index e0aedcac5e1..56cf16c3778 100644 --- a/app/views/projects/environments/metrics.html.haml +++ b/app/views/projects/environments/metrics.html.haml @@ -19,4 +19,6 @@ "empty-loading-svg-path": image_path('illustrations/monitoring/loading'), "empty-unable-to-connect-svg-path": image_path('illustrations/monitoring/unable_to_connect'), "additional-metrics": additional_metrics_project_environment_path(@project, @environment, format: :json), + "project-path": project_path(@project), + "tags-path": project_tags_path(@project), "has-metrics": "#{@environment.has_metrics?}", deployment_endpoint: project_environment_deployments_path(@project, @environment, format: :json) } } diff --git a/changelogs/unreleased/15832-fix-access-level-update-for-requesters.yml b/changelogs/unreleased/15832-fix-access-level-update-for-requesters.yml new file mode 100644 index 00000000000..9d6c958cb3e --- /dev/null +++ b/changelogs/unreleased/15832-fix-access-level-update-for-requesters.yml @@ -0,0 +1,5 @@ +--- +title: Fix error that was preventing users to change the access level of access requests for Groups or Projects +merge_request: 15832 +author: +type: fixed diff --git a/changelogs/unreleased/35385-allow-git-pull-push-on-project-redirects.yml b/changelogs/unreleased/35385-allow-git-pull-push-on-project-redirects.yml new file mode 100644 index 00000000000..31450287caf --- /dev/null +++ b/changelogs/unreleased/35385-allow-git-pull-push-on-project-redirects.yml @@ -0,0 +1,5 @@ +--- +title: Allow git pull/push on group/user/project redirects +merge_request: 15670 +author: +type: added diff --git a/changelogs/unreleased/35724-animate-sidebar.yml b/changelogs/unreleased/35724-animate-sidebar.yml new file mode 100644 index 00000000000..5d0b46a23c8 --- /dev/null +++ b/changelogs/unreleased/35724-animate-sidebar.yml @@ -0,0 +1,5 @@ +--- +title: Animate contextual sidebar on collapse/expand +merge_request: +author: +type: changed diff --git a/changelogs/unreleased/38032-deploy-markers-should-be-more-verbose.yml b/changelogs/unreleased/38032-deploy-markers-should-be-more-verbose.yml new file mode 100644 index 00000000000..a1f28b3ba0f --- /dev/null +++ b/changelogs/unreleased/38032-deploy-markers-should-be-more-verbose.yml @@ -0,0 +1,5 @@ +--- +title: Changed the deploy markers on the prometheus dashboard to be more verbose +merge_request: 38032 +author: +type: changed diff --git a/changelogs/unreleased/40031-include-assset_sync-gem.yml b/changelogs/unreleased/40031-include-assset_sync-gem.yml new file mode 100644 index 00000000000..93ce565b32c --- /dev/null +++ b/changelogs/unreleased/40031-include-assset_sync-gem.yml @@ -0,0 +1,5 @@ +--- +title: Add assets_sync gem to Gemfile +merge_request: 15734 +author: +type: added diff --git a/changelogs/unreleased/anchor-issue-references.yml b/changelogs/unreleased/anchor-issue-references.yml new file mode 100644 index 00000000000..78896427417 --- /dev/null +++ b/changelogs/unreleased/anchor-issue-references.yml @@ -0,0 +1,6 @@ +--- +title: Fix false positive issue references in merge requests caused by header anchor + links. +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/fix-event-target-author-preloading.yml b/changelogs/unreleased/fix-event-target-author-preloading.yml new file mode 100644 index 00000000000..c6154cc0835 --- /dev/null +++ b/changelogs/unreleased/fix-event-target-author-preloading.yml @@ -0,0 +1,5 @@ +--- +title: Fix N+1 query when displaying events +merge_request: +author: +type: performance diff --git a/changelogs/unreleased/mk-fix-schema-dump-of-untracked-files-for-uploads.yml b/changelogs/unreleased/mk-fix-schema-dump-of-untracked-files-for-uploads.yml new file mode 100644 index 00000000000..2691e85320c --- /dev/null +++ b/changelogs/unreleased/mk-fix-schema-dump-of-untracked-files-for-uploads.yml @@ -0,0 +1,5 @@ +--- +title: Fix error during schema dump. +merge_request: 15866 +author: +type: fixed diff --git a/changelogs/unreleased/sh-fix-import-rake-task.yml b/changelogs/unreleased/sh-fix-import-rake-task.yml new file mode 100644 index 00000000000..9cd6d7e4a72 --- /dev/null +++ b/changelogs/unreleased/sh-fix-import-rake-task.yml @@ -0,0 +1,5 @@ +--- +title: Fix gitlab:import:repos Rake task moving repositories into the wrong location +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/sh-remove-allocation-tracking-influxdb.yml b/changelogs/unreleased/sh-remove-allocation-tracking-influxdb.yml new file mode 100644 index 00000000000..b98573df303 --- /dev/null +++ b/changelogs/unreleased/sh-remove-allocation-tracking-influxdb.yml @@ -0,0 +1,5 @@ +--- +title: Remove allocation tracking code from InfluxDB sampler for performance +merge_request: +author: +type: performance diff --git a/config/initializers/active_record_schema_ignore_tables.rb b/config/initializers/active_record_schema_ignore_tables.rb new file mode 100644 index 00000000000..661135f8ade --- /dev/null +++ b/config/initializers/active_record_schema_ignore_tables.rb @@ -0,0 +1,2 @@ +# Ignore table used temporarily in background migration +ActiveRecord::SchemaDumper.ignore_tables = ["untracked_files_for_uploads"] diff --git a/config/initializers/asset_sync.rb b/config/initializers/asset_sync.rb new file mode 100644 index 00000000000..db8500f6231 --- /dev/null +++ b/config/initializers/asset_sync.rb @@ -0,0 +1,31 @@ +AssetSync.configure do |config| + # Disable the asset_sync gem by default. If it is enabled, but not configured, + # asset_sync will cause the build to fail. + config.enabled = if ENV.has_key?('ASSET_SYNC_ENABLED') + ENV['ASSET_SYNC_ENABLED'] == 'true' + else + false + end + + # Pulled from https://github.com/AssetSync/asset_sync/blob/v2.2.0/lib/asset_sync/engine.rb#L15-L40 + # This allows us to disable asset_sync by default and configure through environment variables + # Updates to asset_sync gem should be checked + config.fog_provider = ENV['FOG_PROVIDER'] if ENV.has_key?('FOG_PROVIDER') + config.fog_directory = ENV['FOG_DIRECTORY'] if ENV.has_key?('FOG_DIRECTORY') + config.fog_region = ENV['FOG_REGION'] if ENV.has_key?('FOG_REGION') + + config.aws_access_key_id = ENV['AWS_ACCESS_KEY_ID'] if ENV.has_key?('AWS_ACCESS_KEY_ID') + config.aws_secret_access_key = ENV['AWS_SECRET_ACCESS_KEY'] if ENV.has_key?('AWS_SECRET_ACCESS_KEY') + config.aws_reduced_redundancy = ENV['AWS_REDUCED_REDUNDANCY'] == true if ENV.has_key?('AWS_REDUCED_REDUNDANCY') + + config.rackspace_username = ENV['RACKSPACE_USERNAME'] if ENV.has_key?('RACKSPACE_USERNAME') + config.rackspace_api_key = ENV['RACKSPACE_API_KEY'] if ENV.has_key?('RACKSPACE_API_KEY') + + config.google_storage_access_key_id = ENV['GOOGLE_STORAGE_ACCESS_KEY_ID'] if ENV.has_key?('GOOGLE_STORAGE_ACCESS_KEY_ID') + config.google_storage_secret_access_key = ENV['GOOGLE_STORAGE_SECRET_ACCESS_KEY'] if ENV.has_key?('GOOGLE_STORAGE_SECRET_ACCESS_KEY') + + config.existing_remote_files = ENV['ASSET_SYNC_EXISTING_REMOTE_FILES'] || "keep" + + config.gzip_compression = (ENV['ASSET_SYNC_GZIP_COMPRESSION'] == 'true') if ENV.has_key?('ASSET_SYNC_GZIP_COMPRESSION') + config.manifest = (ENV['ASSET_SYNC_MANIFEST'] == 'true') if ENV.has_key?('ASSET_SYNC_MANIFEST') +end diff --git a/db/migrate/20160610194713_remove_deprecated_issues_tracker_columns_from_projects.rb b/db/migrate/20160610194713_remove_deprecated_issues_tracker_columns_from_projects.rb index 477b2106dea..21b367711c3 100644 --- a/db/migrate/20160610194713_remove_deprecated_issues_tracker_columns_from_projects.rb +++ b/db/migrate/20160610194713_remove_deprecated_issues_tracker_columns_from_projects.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/RemoveColumn class RemoveDeprecatedIssuesTrackerColumnsFromProjects < ActiveRecord::Migration def change remove_column :projects, :issues_tracker, :string, default: 'gitlab', null: false diff --git a/db/migrate/20160610301627_remove_notification_level_from_users.rb b/db/migrate/20160610301627_remove_notification_level_from_users.rb index 8afb14df2cf..356e53b4b23 100644 --- a/db/migrate/20160610301627_remove_notification_level_from_users.rb +++ b/db/migrate/20160610301627_remove_notification_level_from_users.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/RemoveColumn class RemoveNotificationLevelFromUsers < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20160705055809_remove_developers_can_push_from_protected_branches.rb b/db/migrate/20160705055809_remove_developers_can_push_from_protected_branches.rb index 52a9819c628..058bd539e65 100644 --- a/db/migrate/20160705055809_remove_developers_can_push_from_protected_branches.rb +++ b/db/migrate/20160705055809_remove_developers_can_push_from_protected_branches.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/RemoveColumn # See http://doc.gitlab.com/ce/development/migration_style_guide.html # for more information on how to write migrations for GitLab. diff --git a/db/migrate/20160705055813_remove_developers_can_merge_from_protected_branches.rb b/db/migrate/20160705055813_remove_developers_can_merge_from_protected_branches.rb index 4a7bde7f9f3..d0e5da4d28b 100644 --- a/db/migrate/20160705055813_remove_developers_can_merge_from_protected_branches.rb +++ b/db/migrate/20160705055813_remove_developers_can_merge_from_protected_branches.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/RemoveColumn # See http://doc.gitlab.com/ce/development/migration_style_guide.html # for more information on how to write migrations for GitLab. diff --git a/db/migrate/20160729173930_remove_project_id_from_spam_logs.rb b/db/migrate/20160729173930_remove_project_id_from_spam_logs.rb index e28ab31d629..baf254c3bcc 100644 --- a/db/migrate/20160729173930_remove_project_id_from_spam_logs.rb +++ b/db/migrate/20160729173930_remove_project_id_from_spam_logs.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/RemoveColumn # See http://doc.gitlab.com/ce/development/migration_style_guide.html # for more information on how to write migrations for GitLab. diff --git a/db/migrate/20160831223750_remove_features_enabled_from_projects.rb b/db/migrate/20160831223750_remove_features_enabled_from_projects.rb index aec709aaf59..9eafd8b9477 100644 --- a/db/migrate/20160831223750_remove_features_enabled_from_projects.rb +++ b/db/migrate/20160831223750_remove_features_enabled_from_projects.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/RemoveColumn # See http://doc.gitlab.com/ce/development/migration_style_guide.html # for more information on how to write migrations for GitLab. diff --git a/db/migrate/20160913162434_remove_projects_pushes_since_gc.rb b/db/migrate/20160913162434_remove_projects_pushes_since_gc.rb index df7d922b816..f32167037e0 100644 --- a/db/migrate/20160913162434_remove_projects_pushes_since_gc.rb +++ b/db/migrate/20160913162434_remove_projects_pushes_since_gc.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/RemoveColumn # See http://doc.gitlab.com/ce/development/migration_style_guide.html # for more information on how to write migrations for GitLab. diff --git a/db/migrate/20161018024550_remove_priority_from_labels.rb b/db/migrate/20161018024550_remove_priority_from_labels.rb index b7416cca664..bc25a43526c 100644 --- a/db/migrate/20161018024550_remove_priority_from_labels.rb +++ b/db/migrate/20161018024550_remove_priority_from_labels.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/RemoveColumn class RemovePriorityFromLabels < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20161201160452_migrate_project_statistics.rb b/db/migrate/20161201160452_migrate_project_statistics.rb index 82fbdf02444..a547409aaa5 100644 --- a/db/migrate/20161201160452_migrate_project_statistics.rb +++ b/db/migrate/20161201160452_migrate_project_statistics.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/RemoveColumn class MigrateProjectStatistics < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20170222143500_remove_old_project_id_columns.rb b/db/migrate/20170222143500_remove_old_project_id_columns.rb index 268144a2552..9bed38a3444 100644 --- a/db/migrate/20170222143500_remove_old_project_id_columns.rb +++ b/db/migrate/20170222143500_remove_old_project_id_columns.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/RemoveColumn # rubocop:disable RemoveIndex class RemoveOldProjectIdColumns < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20170301205639_remove_unused_ci_tables_and_columns.rb b/db/migrate/20170301205639_remove_unused_ci_tables_and_columns.rb index 1a77d5934a3..0535c2ddaf2 100644 --- a/db/migrate/20170301205639_remove_unused_ci_tables_and_columns.rb +++ b/db/migrate/20170301205639_remove_unused_ci_tables_and_columns.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/RemoveColumn # rubocop:disable Migration/Datetime class RemoveUnusedCiTablesAndColumns < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20170315174634_revert_add_notified_of_own_activity_to_users.rb b/db/migrate/20170315174634_revert_add_notified_of_own_activity_to_users.rb index 807dfcb385d..9b9098d115d 100644 --- a/db/migrate/20170315174634_revert_add_notified_of_own_activity_to_users.rb +++ b/db/migrate/20170315174634_revert_add_notified_of_own_activity_to_users.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/RemoveColumn # rubocop:disable Migration/UpdateLargeTable class RevertAddNotifiedOfOwnActivityToUsers < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20171106154015_remove_issues_branch_name.rb b/db/migrate/20171106154015_remove_issues_branch_name.rb index 3d08225c96d..162b6bafab4 100644 --- a/db/migrate/20171106154015_remove_issues_branch_name.rb +++ b/db/migrate/20171106154015_remove_issues_branch_name.rb @@ -1,3 +1,4 @@ +# rubocop:disable Migration/RemoveColumn # See http://doc.gitlab.com/ce/development/migration_style_guide.html # for more information on how to write migrations for GitLab. diff --git a/db/migrate/20171204204233_add_permanent_to_redirect_route.rb b/db/migrate/20171204204233_add_permanent_to_redirect_route.rb new file mode 100644 index 00000000000..f3ae471201e --- /dev/null +++ b/db/migrate/20171204204233_add_permanent_to_redirect_route.rb @@ -0,0 +1,18 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddPermanentToRedirectRoute < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + disable_ddl_transaction! + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + def up + add_column(:redirect_routes, :permanent, :boolean) + end + + def down + remove_column(:redirect_routes, :permanent) + end +end diff --git a/db/migrate/20171206221519_add_permanent_index_to_redirect_route.rb b/db/migrate/20171206221519_add_permanent_index_to_redirect_route.rb new file mode 100644 index 00000000000..33ce7e1aa68 --- /dev/null +++ b/db/migrate/20171206221519_add_permanent_index_to_redirect_route.rb @@ -0,0 +1,19 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddPermanentIndexToRedirectRoute < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_concurrent_index(:redirect_routes, :permanent) + end + + def down + remove_concurrent_index(:redirect_routes, :permanent) if index_exists?(:redirect_routes, :permanent) + end +end diff --git a/db/post_migrate/20170523073948_remove_assignee_id_from_issue.rb b/db/post_migrate/20170523073948_remove_assignee_id_from_issue.rb new file mode 100644 index 00000000000..006d17b4d62 --- /dev/null +++ b/db/post_migrate/20170523073948_remove_assignee_id_from_issue.rb @@ -0,0 +1,48 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class RemoveAssigneeIdFromIssue < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + # When a migration requires downtime you **must** uncomment the following + # constant and define a short and easy to understand explanation as to why the + # migration requires downtime. + # DOWNTIME_REASON = '' + + # When using the methods "add_concurrent_index", "remove_concurrent_index" or + # "add_column_with_default" you must disable the use of transactions + # as these methods can not run in an existing transaction. + # When using "add_concurrent_index" or "remove_concurrent_index" methods make sure + # that either of them is the _only_ method called in the migration, + # any other changes should go in a separate migration. + # This ensures that upon failure _only_ the index creation or removing fails + # and can be retried or reverted easily. + # + # To disable transactions uncomment the following line and remove these + # comments: + disable_ddl_transaction! + + class Issue < ActiveRecord::Base + self.table_name = 'issues' + + include ::EachBatch + end + + def up + remove_column :issues, :assignee_id + end + + def down + add_column :issues, :assignee_id, :integer + add_concurrent_index :issues, :assignee_id + + update_value = Arel.sql('(SELECT user_id FROM issue_assignees WHERE issue_assignees.issue_id = issues.id LIMIT 1)') + + # This is only used in the down step, so we can ignore the RuboCop warning + # about large tables, as this is very unlikely to be run on GitLab.com + update_column_in_batches(:issues, :assignee_id, update_value) # rubocop:disable Migration/UpdateLargeTable + end +end diff --git a/db/schema.rb b/db/schema.rb index 4c697a4a384..f0b1da16d53 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20171205190711) do +ActiveRecord::Schema.define(version: 20171206221519) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -841,7 +841,6 @@ ActiveRecord::Schema.define(version: 20171205190711) do create_table "issues", force: :cascade do |t| t.string "title" - t.integer "assignee_id" t.integer "author_id" t.integer "project_id" t.datetime "created_at" @@ -867,7 +866,6 @@ ActiveRecord::Schema.define(version: 20171205190711) do t.datetime_with_timezone "closed_at" end - add_index "issues", ["assignee_id"], name: "index_issues_on_assignee_id", using: :btree add_index "issues", ["author_id"], name: "index_issues_on_author_id", using: :btree add_index "issues", ["confidential"], name: "index_issues_on_confidential", using: :btree add_index "issues", ["deleted_at"], name: "index_issues_on_deleted_at", using: :btree @@ -1526,10 +1524,12 @@ ActiveRecord::Schema.define(version: 20171205190711) do t.string "path", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.boolean "permanent" end add_index "redirect_routes", ["path"], name: "index_redirect_routes_on_path", unique: true, using: :btree add_index "redirect_routes", ["path"], name: "index_redirect_routes_on_path_text_pattern_ops", using: :btree, opclasses: {"path"=>"varchar_pattern_ops"} + add_index "redirect_routes", ["permanent"], name: "index_redirect_routes_on_permanent", using: :btree add_index "redirect_routes", ["source_type", "source_id"], name: "index_redirect_routes_on_source_type_and_source_id", using: :btree create_table "releases", force: :cascade do |t| diff --git a/doc/administration/index.md b/doc/administration/index.md index c8d28d8485a..e6986a2b07f 100644 --- a/doc/administration/index.md +++ b/doc/administration/index.md @@ -35,7 +35,7 @@ Learn how to install, configure, update, and maintain your GitLab instance. - [Raketasks](../raketasks/README.md): Perform various tasks for maintenance, backups, automatic webhooks setup, etc. - [Backup and restore](../raketasks/backup_restore.md): Backup and restore your GitLab instance. -- [Operations](operations.md): Keeping GitLab up and running (clean up Redis sessions, moving repositories, Sidekiq Job throttling, Sidekiq MemoryKiller, Unicorn). +- [Operations](operations/index.md): Keeping GitLab up and running (clean up Redis sessions, moving repositories, Sidekiq Job throttling, Sidekiq MemoryKiller, Unicorn). - [Restart GitLab](restart_gitlab.md): Learn how to restart GitLab and its components. #### Updating GitLab diff --git a/doc/administration/monitoring/prometheus/gitlab_metrics.md b/doc/administration/monitoring/prometheus/gitlab_metrics.md index 11d5e077a36..f495990d9a4 100644 --- a/doc/administration/monitoring/prometheus/gitlab_metrics.md +++ b/doc/administration/monitoring/prometheus/gitlab_metrics.md @@ -45,8 +45,9 @@ In this experimental phase, only a few metrics are available: | redis_ping_success | Gauge | 9.4 | Whether or not the last redis ping succeeded | | redis_ping_latency_seconds | Gauge | 9.4 | Round trip time of the redis ping | | user_session_logins_total | Counter | 9.4 | Counter of how many users have logged in | -| filesystem_circuitbreaker_latency_seconds | Histogram | 9.5 | Latency of the stat check the circuitbreaker uses to probe a shard | +| filesystem_circuitbreaker_latency_seconds | Gauge | 9.5 | Time spent validating if a storage is accessible | | filesystem_circuitbreaker | Gauge | 9.5 | Wether or not the circuit for a certain shard is broken or not | +| circuitbreaker_storage_check_duration_seconds | Histogram | 10.3 | Time a single storage probe took | ## Metrics shared directory diff --git a/doc/administration/operations.md b/doc/administration/operations.md index 0daceb98d99..4797d2a3206 100644 --- a/doc/administration/operations.md +++ b/doc/administration/operations.md @@ -1,7 +1 @@ -# GitLab operations - -- [Sidekiq MemoryKiller](operations/sidekiq_memory_killer.md) -- [Sidekiq Job throttling](operations/sidekiq_job_throttling.md) -- [Cleaning up Redis sessions](operations/cleaning_up_redis_sessions.md) -- [Understanding Unicorn and unicorn-worker-killer](operations/unicorn.md) -- [Moving repositories to a new location](operations/moving_repositories.md) +This document was moved to [another location](operations/index.md). diff --git a/doc/administration/operations/index.md b/doc/administration/operations/index.md new file mode 100644 index 00000000000..320d71a9527 --- /dev/null +++ b/doc/administration/operations/index.md @@ -0,0 +1,16 @@ +# Performing Operations in GitLab + +Keep your GitLab instance up and running smoothly. + +- [Clean up Redis sessions](cleaning_up_redis_sessions.md): Prior to GitLab 7.3, +user sessions did not automatically expire from Redis. If +you have been running a large GitLab server (thousands of users) since before +GitLab 7.3 we recommend cleaning up stale sessions to compact the Redis +database after you upgrade to GitLab 7.3. +- [Moving repositories](moving_repositories.md): Moving all repositories managed +by GitLab to another file system or another server. +- [Sidekiq job throttling](sidekiq_job_throttling.md): Throttle Sidekiq queues +that to prioritize important jobs. +- [Sidekiq MemoryKiller](sidekiq_memory_killer.md): Configure Sidekiq MemoryKiller +to restart Sidekiq. +- [Unicorn](unicorn.md): Understand Unicorn and unicorn-worker-killer.
\ No newline at end of file diff --git a/doc/development/README.md b/doc/development/README.md index 6892838be7f..7e4c767692a 100644 --- a/doc/development/README.md +++ b/doc/development/README.md @@ -16,7 +16,8 @@ comments: false - [GitLab core team & GitLab Inc. contribution process](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/PROCESS.md) - [Generate a changelog entry with `bin/changelog`](changelog.md) - [Code review guidelines](code_review.md) for reviewing code and having code reviewed. -- [Limit conflicts with EE when developing on CE](limit_ee_conflicts.md) +- [Automatic CE->EE merge](automatic_ce_ee_merge.md) +- [Guidelines for implementing Enterprise Edition features](ee_features.md) ## UX and frontend guides diff --git a/doc/development/automatic_ce_ee_merge.md b/doc/development/automatic_ce_ee_merge.md new file mode 100644 index 00000000000..4b9791c95bc --- /dev/null +++ b/doc/development/automatic_ce_ee_merge.md @@ -0,0 +1,93 @@ +# Automatic CE->EE merge + +GitLab Community Edition is merged automatically every 3 hours into the +Enterprise Edition (look for the [`CE Upstream` merge requests]). + +This merge is done automatically in a +[scheduled pipeline](https://gitlab.com/gitlab-org/release-tools/-/jobs/43201679). +If a merge is already in progress, the job [doesn't create a new one](https://gitlab.com/gitlab-org/release-tools/-/jobs/43157687). + +**If you are pinged in a `CE Upstream` merge request to resolve a conflict, +please resolve the conflict as soon as possible or ask someone else to do it!** + +>**Note:** +It's ok to resolve more conflicts than the one that you are asked to resolve. In +that case, it's a good habit to ask for a double-check on your resolution by +someone who is familiar with the code you touched. + +[`CE Upstream` merge requests]: https://gitlab.com/gitlab-org/gitlab-ee/merge_requests?label_name%5B%5D=CE+upstream + +## Always merge EE merge requests before their CE counterparts + +**In order to avoid conflicts in the CE->EE merge, you should always merge the +EE version of your CE merge request first, if present.** + +The rationale for this is that as CE->EE merges are done automatically every few +hours, it can happen that: + +1. A CE merge request that needs EE-specific changes is merged +1. The automatic CE->EE merge happens +1. Conflicts due to the CE merge request occur since its EE merge request isn't + merged yet +1. The automatic merge bot will ping someone to resolve the conflict **that are + already resolved in the EE merge request that isn't merged yet** + +That's a waste of time, and that's why you should merge EE merge request before +their CE counterpart. + +## Avoiding CE->EE merge conflicts beforehand + +To avoid the conflicts beforehand, check out the +[Guidelines for implementing Enterprise Edition features](ee_features.md). + +In any case, the CI `ee_compat_check` job will tell you if you need to open an +EE version of your CE merge request. + +### Conflicts detection in CE merge requests + +For each commit (except on `master`), the `ee_compat_check` CI job tries to +detect if the current branch's changes will conflict during the CE->EE merge. + +The job reports what files are conflicting and how to setup a merge request +against EE. + +#### How the job works + +1. Generates the diff between your branch and current CE `master` +1. Tries to apply it to current EE `master` +1. If it applies cleanly, the job succeeds, otherwise... +1. Detects a branch with the `ee-` prefix or `-ee` suffix in EE +1. If it exists, generate the diff between this branch and current EE `master` +1. Tries to apply it to current EE `master` +1. If it applies cleanly, the job succeeds + +In the case where the job fails, it means you should create a `ee-<ce_branch>` +or `<ce_branch>-ee` branch, push it to EE and open a merge request against EE +`master`. +At this point if you retry the failing job in your CE merge request, it should +now pass. + +Notes: + +- This task is not a silver-bullet, its current goal is to bring awareness to + developers that their work needs to be ported to EE. +- Community contributors shouldn't be required to submit merge requests against + EE, but reviewers should take actions by either creating such EE merge request + or asking a GitLab developer to do it **before the merge request is merged**. +- If you branch is too far behind `master`, the job will fail. In that case you + should rebase your branch upon latest `master`. +- Code reviews for merge requests often consist of multiple iterations of + feedback and fixes. There is no need to update your EE MR after each + iteration. Instead, create an EE MR as soon as you see the + `ee_compat_check` job failing. After you receive the final approval + from a Maintainer (but **before the CE MR is merged**) update the EE MR. + This helps to identify significant conflicts sooner, but also reduces the + number of times you have to resolve conflicts. +- Please remember to + [always have your EE merge request merged before the CE version](#always-merge-ee-merge-requests-before-their-ce-counterparts). +- You can use [`git rerere`](https://git-scm.com/blog/2010/03/08/rerere.html) + to avoid resolving the same conflicts multiple times. + +--- + +[Return to Development documentation](README.md) diff --git a/doc/development/ee_features.md b/doc/development/ee_features.md index 932a44f65e4..1af839a27e1 100644 --- a/doc/development/ee_features.md +++ b/doc/development/ee_features.md @@ -1,4 +1,4 @@ -# Guidelines for implementing Enterprise Edition feature +# Guidelines for implementing Enterprise Edition features - **Write the code and the tests.**: As with any code, EE features should have good test coverage to prevent regressions. @@ -380,3 +380,9 @@ to avoid conflicts during CE to EE merge. } } ``` + +## gitlab-svgs + +Conflicts in `app/assets/images/icons.json` or `app/assets/images/icons.svg` can +be resolved simply by regenerating those assets with +[`yarn run svg`](https://gitlab.com/gitlab-org/gitlab-svgs). diff --git a/doc/development/fe_guide/style_guide_js.md b/doc/development/fe_guide/style_guide_js.md index 10f4c5a0902..1cd66f27492 100644 --- a/doc/development/fe_guide/style_guide_js.md +++ b/doc/development/fe_guide/style_guide_js.md @@ -86,34 +86,34 @@ followed by any global declarations, then a blank newline prior to any imports o #### Modules, Imports, and Exports 1. Use ES module syntax to import modules - ```javascript - // bad - const SomeClass = require('some_class'); + ```javascript + // bad + const SomeClass = require('some_class'); - // good - import SomeClass from 'some_class'; + // good + import SomeClass from 'some_class'; - // bad - module.exports = SomeClass; + // bad + module.exports = SomeClass; - // good - export default SomeClass; - ``` - - Import statements are following usual naming guidelines, for example object literals use camel case: - - ```javascript - // some_object file - export default { - key: 'value', - }; - - // bad - import ObjectLiteral from 'some_object'; + // good + export default SomeClass; + ``` + + Import statements are following usual naming guidelines, for example object literals use camel case: - // good - import objectLiteral from 'some_object'; - ``` + ```javascript + // some_object file + export default { + key: 'value', + }; + + // bad + import ObjectLiteral from 'some_object'; + + // good + import objectLiteral from 'some_object'; + ``` 1. Relative paths: when importing a module in the same directory, a child directory, or an immediate parent directory prefer relative paths. When @@ -334,33 +334,33 @@ A forEach will cause side effects, it will be mutating the array being iterated. #### Alignment 1. Follow these alignment styles for the template method: 1. With more than one attribute, all attributes should be on a new line: - ```javascript - // bad - <component v-if="bar" - param="baz" /> + ```javascript + // bad + <component v-if="bar" + param="baz" /> - <button class="btn">Click me</button> + <button class="btn">Click me</button> - // good - <component - v-if="bar" - param="baz" - /> + // good + <component + v-if="bar" + param="baz" + /> - <button class="btn"> - Click me - </button> - ``` + <button class="btn"> + Click me + </button> + ``` 1. The tag can be inline if there is only one attribute: - ```javascript - // good - <component bar="bar" /> + ```javascript + // good + <component bar="bar" /> - // good - <component - bar="bar" - /> - ``` + // good + <component + bar="bar" + /> + ``` #### Quotes 1. Always use double quotes `"` inside templates and single quotes `'` for all other JS. @@ -414,7 +414,6 @@ A forEach will cause side effects, it will be mutating the array being iterated. 1. Default key should be provided if the prop is not required. _Note:_ There are some scenarios where we need to check for the existence of the property. On those a default key should not be provided. - ```javascript // good props: { @@ -494,21 +493,20 @@ On those a default key should not be provided. #### Ordering 1. Tag order in `.vue` file - - ``` - <script> - // ... - </script> - - <template> - // ... - </template> - - // We don't use scoped styles but there are few instances of this - <style> - // ... - </style> - ``` + ``` + <script> + // ... + </script> + + <template> + // ... + </template> + + // We don't use scoped styles but there are few instances of this + <style> + // ... + </style> + ``` 1. Properties in a Vue Component: 1. `name` diff --git a/doc/development/limit_ee_conflicts.md b/doc/development/limit_ee_conflicts.md deleted file mode 100644 index ba82babb38a..00000000000 --- a/doc/development/limit_ee_conflicts.md +++ /dev/null @@ -1,347 +0,0 @@ -# Limit conflicts with EE when developing on CE - -This guide contains best-practices for avoiding conflicts between CE and EE. - -## Daily CE Upstream merge - -GitLab Community Edition is merged daily into the Enterprise Edition (look for -the [`CE Upstream` merge requests]). The daily merge is currently done manually -by four individuals. - -**If a developer pings you in a `CE Upstream` merge request for help with -resolving conflicts, please help them because it means that you didn't do your -job to reduce the conflicts nor to ease their resolution in the first place!** - -To avoid the conflicts beforehand when working on CE, there are a few tools and -techniques that can help you: - -- know what are the usual types of conflicts and how to prevent them -- the CI `rake ee_compat_check` job tells you if you need to open an EE-version - of your CE merge request - -[`CE Upstream` merge requests]: https://gitlab.com/gitlab-org/gitlab-ee/merge_requests?label_name%5B%5D=CE+upstream - -## Check the status of the CI `rake ee_compat_check` job - -For each commit (except on `master`), the `rake ee_compat_check` CI job tries to -detect if the current branch's changes will conflict during the CE->EE merge. - -The job reports what files are conflicting and how to setup a merge request -against EE. Here is roughly how it works: - -1. Generates the diff between your branch and current CE `master` -1. Tries to apply it to current EE `master` -1. If it applies cleanly, the job succeeds, otherwise... -1. Detects a branch with the `-ee` suffix in EE -1. If it exists, generate the diff between this branch and current EE `master` -1. Tries to apply it to current EE `master` -1. If it applies cleanly, the job succeeds - -In the case where the job fails, it means you should create a `<ce_branch>-ee` -branch, push it to EE and open a merge request against EE `master`. At this -point if you retry the failing job in your CE merge request, it should now pass. - -Notes: - -- This task is not a silver-bullet, its current goal is to bring awareness to - developers that their work needs to be ported to EE. -- Community contributors shouldn't submit merge requests against EE, but - reviewers should take actions by either creating such EE merge request or - asking a GitLab developer to do it once the merge request is merged. -- If you branch is more than 500 commits behind `master`, the job will fail and - you should rebase your branch upon latest `master`. -- Code reviews for merge requests often consist of multiple iterations of - feedback and fixes. There is no need to update your EE MR after each - iteration. Instead, create an EE MR as soon as you see the - `rake ee_compat_check` job failing. After you receive the final acceptance - from a Maintainer (but before the CE MR is merged) update the EE MR. - This helps to identify significant conflicts sooner, but also reduces the - number of times you have to resolve conflicts. -- You can use [`git rerere`](https://git-scm.com/blog/2010/03/08/rerere.html) - to avoid resolving the same conflicts multiple times. - -## Possible type of conflicts - -### Controllers - -#### List or arrays are augmented in EE - -In controllers, the most common type of conflict is with `before_action` that -has a list of actions in CE but EE adds some actions to that list. - -The same problem often occurs for `params.require` / `params.permit` calls. - -##### Mitigations - -Separate CE and EE actions/keywords. For instance for `params.require` in -`ProjectsController`: - -```ruby -def project_params - params.require(:project).permit(project_params_ce) - # On EE, this is always: - # params.require(:project).permit(project_params_ce << project_params_ee) -end - -# Always returns an array of symbols, created however best fits the use case. -# It _should_ be sorted alphabetically. -def project_params_ce - %i[ - description - name - path - ] -end - -# (On EE) -def project_params_ee - %i[ - approvals_before_merge - approver_group_ids - approver_ids - ... - ] -end -``` - -#### Additional condition(s) in EE - -For instance for LDAP: - -```diff - def destroy - @key = current_user.keys.find(params[:id]) - - @key.destroy - + @key.destroy unless @key.is_a? LDAPKey - - respond_to do |format| -``` - -Or for Geo: - -```diff -def after_sign_out_path_for(resource) -- current_application_settings.after_sign_out_path.presence || new_user_session_path -+ if Gitlab::Geo.secondary? -+ Gitlab::Geo.primary_node.oauth_logout_url(@geo_logout_state) -+ else -+ current_application_settings.after_sign_out_path.presence || new_user_session_path -+ end -end -``` - -Or even for audit log: - -```diff -def approve_access_request -- Members::ApproveAccessRequestService.new(membershipable, current_user, params).execute -+ member = Members::ApproveAccessRequestService.new(membershipable, current_user, params).execute -+ -+ log_audit_event(member, action: :create) - - redirect_to polymorphic_url([membershipable, :members]) -end -``` - -### Views - -#### Additional view code in EE - -A block of code added in CE conflicts because there is already another block -at the same place in EE - -##### Mitigations - -Blocks of code that are EE-specific should be moved to partials as much as -possible to avoid conflicts with big chunks of HAML code that that are not fun -to resolve when you add the indentation to the equation. - -For instance this kind of thing: - -```haml -.form-group.detail-page-description - = form.label :description, 'Description', class: 'control-label' - .col-sm-10 - = render layout: 'projects/md_preview', locals: { preview_class: "md-preview", referenced_users: true } do - = render 'projects/zen', f: form, attr: :description, - classes: 'note-textarea', - placeholder: "Write a comment or drag your files here...", - supports_quick_actions: !issuable.persisted? - = render 'projects/notes/hints', supports_quick_actions: !issuable.persisted? - .clearfix - .error-alert -- if issuable.is_a?(Issue) - .form-group - .col-sm-offset-2.col-sm-10 - .checkbox - = form.label :confidential do - = form.check_box :confidential - This issue is confidential and should only be visible to team members with at least Reporter access. -- if can?(current_user, :"admin_#{issuable.to_ability_name}", issuable.project) - - has_due_date = issuable.has_attribute?(:due_date) - %hr - .row - %div{ class: (has_due_date ? "col-lg-6" : "col-sm-12") } - .form-group.issue-assignee - = form.label :assignee_id, "Assignee", class: "control-label #{"col-lg-4" if has_due_date}" - .col-sm-10{ class: ("col-lg-8" if has_due_date) } - .issuable-form-select-holder - - if issuable.assignee_id - = form.hidden_field :assignee_id - = dropdown_tag(user_dropdown_label(issuable.assignee_id, "Assignee"), options: { toggle_class: "js-dropdown-keep-input js-user-search js-issuable-form-dropdown js-assignee-search", title: "Select assignee", filter: true, dropdown_class: "dropdown-menu-user dropdown-menu-selectable dropdown-menu-assignee js-filter-submit", - placeholder: "Search assignee", data: { first_user: current_user.try(:username), null_user: true, current_user: true, project_id: project.try(:id), selected: issuable.assignee_id, field_name: "#{issuable.class.model_name.param_key}[assignee_id]", default_label: "Assignee"} }) - .form-group.issue-milestone - = form.label :milestone_id, "Milestone", class: "control-label #{"col-lg-4" if has_due_date}" - .col-sm-10{ class: ("col-lg-8" if has_due_date) } - .issuable-form-select-holder - = render "shared/issuable/milestone_dropdown", selected: issuable.milestone, name: "#{issuable.class.model_name.param_key}[milestone_id]", show_any: false, show_upcoming: false, extra_class: "js-issuable-form-dropdown js-dropdown-keep-input", dropdown_title: "Select milestone" - .form-group - - has_labels = @labels && @labels.any? - = form.label :label_ids, "Labels", class: "control-label #{"col-lg-4" if has_due_date}" - = form.hidden_field :label_ids, multiple: true, value: '' - .col-sm-10{ class: "#{"col-lg-8" if has_due_date} #{'issuable-form-padding-top' if !has_labels}" } - .issuable-form-select-holder - = render "shared/issuable/label_dropdown", classes: ["js-issuable-form-dropdown"], selected: issuable.labels, data_options: { field_name: "#{issuable.class.model_name.param_key}[label_ids][]", show_any: false }, dropdown_title: "Select label" - - if issuable.respond_to?(:weight) - - weight_options = Issue.weight_options - - weight_options.delete(Issue::WEIGHT_ALL) - - weight_options.delete(Issue::WEIGHT_ANY) - .form-group - = form.label :label_ids, class: "control-label #{"col-lg-4" if has_due_date}" do - Weight - .col-sm-10{ class: ("col-lg-8" if has_due_date) } - .issuable-form-select-holder - - if issuable.weight - = form.hidden_field :weight - = dropdown_tag(issuable.weight || "Weight", options: { title: "Select weight", toggle_class: 'js-weight-select js-issuable-form-weight', dropdown_class: "dropdown-menu-selectable dropdown-menu-weight", - placeholder: "Search weight", data: { field_name: "#{issuable.class.model_name.param_key}[weight]" , default_label: "Weight" } }) do - %ul - - weight_options.each do |weight| - %li - %a{href: "#", data: { id: weight, none: weight === Issue::WEIGHT_NONE }, class: ("is-active" if issuable.weight == weight)} - = weight - - if has_due_date - .col-lg-6 - .form-group - = form.label :due_date, "Due date", class: "control-label" - .col-sm-10 - .issuable-form-select-holder - = form.text_field :due_date, id: "issuable-due-date", class: "datepicker form-control", placeholder: "Select due date" -``` - -could be simplified by using partials: - -```haml -= render 'shared/issuable/form/description', issuable: issuable, form: form - -- if issuable.respond_to?(:confidential) - .form-group - .col-sm-offset-2.col-sm-10 - .checkbox - = form.label :confidential do - = form.check_box :confidential - This issue is confidential and should only be visible to team members with at least Reporter access. - -= render 'shared/issuable/form/metadata', issuable: issuable, form: form -``` - -and then the `app/views/shared/issuable/form/_metadata.html.haml` could be as follows: - -```haml -- issuable = local_assigns.fetch(:issuable) - -- return unless can?(current_user, :"admin_#{issuable.to_ability_name}", issuable.project) - -- has_due_date = issuable.has_attribute?(:due_date) -- has_labels = @labels && @labels.any? -- form = local_assigns.fetch(:form) - -%hr -.row - %div{ class: (has_due_date ? "col-lg-6" : "col-sm-12") } - .form-group.issue-assignee - = form.label :assignee_id, "Assignee", class: "control-label #{"col-lg-4" if has_due_date}" - .col-sm-10{ class: ("col-lg-8" if has_due_date) } - .issuable-form-select-holder - - if issuable.assignee_id - = form.hidden_field :assignee_id - = dropdown_tag(user_dropdown_label(issuable.assignee_id, "Assignee"), options: { toggle_class: "js-dropdown-keep-input js-user-search js-issuable-form-dropdown js-assignee-search", title: "Select assignee", filter: true, dropdown_class: "dropdown-menu-user dropdown-menu-selectable dropdown-menu-assignee js-filter-submit", - placeholder: "Search assignee", data: { first_user: current_user.try(:username), null_user: true, current_user: true, project_id: issuable.project.try(:id), selected: issuable.assignee_id, field_name: "#{issuable.class.model_name.param_key}[assignee_id]", default_label: "Assignee"} }) - .form-group.issue-milestone - = form.label :milestone_id, "Milestone", class: "control-label #{"col-lg-4" if has_due_date}" - .col-sm-10{ class: ("col-lg-8" if has_due_date) } - .issuable-form-select-holder - = render "shared/issuable/milestone_dropdown", selected: issuable.milestone, name: "#{issuable.class.model_name.param_key}[milestone_id]", show_any: false, show_upcoming: false, extra_class: "js-issuable-form-dropdown js-dropdown-keep-input", dropdown_title: "Select milestone" - .form-group - - has_labels = @labels && @labels.any? - = form.label :label_ids, "Labels", class: "control-label #{"col-lg-4" if has_due_date}" - = form.hidden_field :label_ids, multiple: true, value: '' - .col-sm-10{ class: "#{"col-lg-8" if has_due_date} #{'issuable-form-padding-top' if !has_labels}" } - .issuable-form-select-holder - = render "shared/issuable/label_dropdown", classes: ["js-issuable-form-dropdown"], selected: issuable.labels, data_options: { field_name: "#{issuable.class.model_name.param_key}[label_ids][]", show_any: false }, dropdown_title: "Select label" - - = render "shared/issuable/form/weight", issuable: issuable, form: form - - - if has_due_date - .col-lg-6 - .form-group - = form.label :due_date, "Due date", class: "control-label" - .col-sm-10 - .issuable-form-select-holder - = form.text_field :due_date, id: "issuable-due-date", class: "datepicker form-control", placeholder: "Select due date" -``` - -and then the `app/views/shared/issuable/form/_weight.html.haml` could be as follows: - -```haml -- issuable = local_assigns.fetch(:issuable) - -- return unless issuable.respond_to?(:weight) - -- has_due_date = issuable.has_attribute?(:due_date) -- form = local_assigns.fetch(:form) - -.form-group - = form.label :label_ids, class: "control-label #{"col-lg-4" if has_due_date}" do - Weight - .col-sm-10{ class: ("col-lg-8" if has_due_date) } - .issuable-form-select-holder - - if issuable.weight - = form.hidden_field :weight - - = weight_dropdown_tag(issuable, toggle_class: 'js-issuable-form-weight') do - %ul - - Issue.weight_options.each do |weight| - %li - %a{ href: '#', data: { id: weight, none: weight === Issue::WEIGHT_NONE }, class: ("is-active" if issuable.weight == weight) } - = weight -``` - -Note: - -- The safeguards at the top allow to get rid of an unneccessary indentation level -- Here we only moved the 'Weight' code to a partial since this is the only - EE-specific code in that view, so it's the most likely to conflict, but you - are encouraged to use partials even for code that's in CE to logically split - big views into several smaller files. - -#### Indentation issue - -Sometimes a code block is indented more or less in EE because there's an -additional condition. - -##### Mitigations - -Blocks of code that are EE-specific should be moved to partials as much as -possible to avoid conflicts with big chunks of HAML code that that are not fun -to resolve when you add the indentation in the equation. - -### Assets - -#### gitlab-svgs - -Conflicts in `app/assets/images/icons.json` or `app/assets/images/icons.svg` can be resolved simply by regenerating those assets with [`yarn run svg`](https://gitlab.com/gitlab-org/gitlab-svgs). - ---- - -[Return to Development documentation](README.md) diff --git a/doc/development/writing_documentation.md b/doc/development/writing_documentation.md index b6def7ef541..48e04a40050 100644 --- a/doc/development/writing_documentation.md +++ b/doc/development/writing_documentation.md @@ -142,7 +142,7 @@ tests. If it doesn't, the whole test suite will run (including docs). --- When you submit a merge request to GitLab Community Edition (CE), there is an -additional job called `rake ee_compat_check` that runs against Enterprise +additional job called `ee_compat_check` that runs against Enterprise Edition (EE) and checks if your changes can apply cleanly to the EE codebase. If that job fails, read the instructions in the job log for what to do next. Contributors do not need to submit their changes to EE, GitLab Inc. employees diff --git a/doc/operations/README.md b/doc/operations/README.md index 58f16aff7bd..d7a83948b87 100644 --- a/doc/operations/README.md +++ b/doc/operations/README.md @@ -1 +1 @@ -This document was moved to [administration/operations](../administration/operations.md). +This document was moved to [another location](../administration/operations/index.md). diff --git a/features/steps/groups.rb b/features/steps/groups.rb index a2d9a0332e0..753694a5392 100644 --- a/features/steps/groups.rb +++ b/features/steps/groups.rb @@ -138,7 +138,7 @@ class Spinach::Features::Groups < Spinach::FeatureSteps private def assigned_to_me(key) - project.send(key).where(assignee_id: current_user.id) + project.send(key).assigned_to(current_user) end def project diff --git a/lib/api/entities.rb b/lib/api/entities.rb index d96e7f2770f..928706dfda7 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -248,8 +248,21 @@ module API end class GroupDetail < Group - expose :projects, using: Entities::Project - expose :shared_projects, using: Entities::Project + expose :projects, using: Entities::Project do |group, options| + GroupProjectsFinder.new( + group: group, + current_user: options[:current_user], + options: { only_owned: true } + ).execute + end + + expose :shared_projects, using: Entities::Project do |group, options| + GroupProjectsFinder.new( + group: group, + current_user: options[:current_user], + options: { only_shared: true } + ).execute + end end class Commit < Grape::Entity diff --git a/lib/api/internal.rb b/lib/api/internal.rb index 451121a4cea..ccaaeca10d4 100644 --- a/lib/api/internal.rb +++ b/lib/api/internal.rb @@ -4,6 +4,7 @@ module API before { authenticate_by_gitlab_shell_token! } helpers ::API::Helpers::InternalHelpers + helpers ::Gitlab::Identifier namespace 'internal' do # Check if git command is allowed to project @@ -176,17 +177,25 @@ module API post '/post_receive' do status 200 - PostReceive.perform_async(params[:gl_repository], params[:identifier], params[:changes]) broadcast_message = BroadcastMessage.current&.last&.message reference_counter_decreased = Gitlab::ReferenceCounter.new(params[:gl_repository]).decrease - { + output = { merge_request_urls: merge_request_urls, broadcast_message: broadcast_message, reference_counter_decreased: reference_counter_decreased } + + project = Gitlab::GlRepository.parse(params[:gl_repository]).first + user = identify(params[:identifier]) + redirect_message = Gitlab::Checks::ProjectMoved.fetch_redirect_message(user.id, project.id) + if redirect_message + output[:redirected_message] = redirect_message + end + + output end end end diff --git a/lib/api/issues.rb b/lib/api/issues.rb index e60e00d7956..5f943ba27d1 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -161,6 +161,8 @@ module API use :issue_params end post ':id/issues' do + authorize! :create_issue, user_project + # Setting created_at time only allowed for admins and project owners unless current_user.admin? || user_project.owner == current_user params.delete(:created_at) diff --git a/lib/banzai/filter/table_of_contents_filter.rb b/lib/banzai/filter/table_of_contents_filter.rb index 47151626208..97244159985 100644 --- a/lib/banzai/filter/table_of_contents_filter.rb +++ b/lib/banzai/filter/table_of_contents_filter.rb @@ -32,6 +32,7 @@ module Banzai .gsub(PUNCTUATION_REGEXP, '') # remove punctuation .tr(' ', '-') # replace spaces with dash .squeeze('-') # replace multiple dashes with one + .gsub(/\A(\d+)\z/, 'anchor-\1') # digits-only hrefs conflict with issue refs uniq = headers[id] > 0 ? "-#{headers[id]}" : '' headers[id] += 1 diff --git a/lib/gitlab/bare_repository_import/importer.rb b/lib/gitlab/bare_repository_import/importer.rb index 196de667805..298409d8b5a 100644 --- a/lib/gitlab/bare_repository_import/importer.rb +++ b/lib/gitlab/bare_repository_import/importer.rb @@ -55,6 +55,7 @@ module Gitlab name: project_name, path: project_name, skip_disk_validation: true, + import_type: 'gitlab_project', namespace_id: group&.id).execute if project.persisted? && mv_repo(project) diff --git a/lib/gitlab/bare_repository_import/repository.rb b/lib/gitlab/bare_repository_import/repository.rb index 8574ac6eb30..fa7891c8dcc 100644 --- a/lib/gitlab/bare_repository_import/repository.rb +++ b/lib/gitlab/bare_repository_import/repository.rb @@ -7,6 +7,8 @@ module Gitlab @root_path = root_path @repo_path = repo_path + @root_path << '/' unless root_path.ends_with?('/') + # Split path into 'all/the/namespaces' and 'project_name' @group_path, _, @project_name = repo_relative_path.rpartition('/') end diff --git a/lib/gitlab/checks/project_moved.rb b/lib/gitlab/checks/project_moved.rb new file mode 100644 index 00000000000..3a1c0a3455e --- /dev/null +++ b/lib/gitlab/checks/project_moved.rb @@ -0,0 +1,65 @@ +module Gitlab + module Checks + class ProjectMoved + REDIRECT_NAMESPACE = "redirect_namespace".freeze + + def initialize(project, user, redirected_path, protocol) + @project = project + @user = user + @redirected_path = redirected_path + @protocol = protocol + end + + def self.fetch_redirect_message(user_id, project_id) + redirect_key = redirect_message_key(user_id, project_id) + + Gitlab::Redis::SharedState.with do |redis| + message = redis.get(redirect_key) + redis.del(redirect_key) + message + end + end + + def add_redirect_message + Gitlab::Redis::SharedState.with do |redis| + key = self.class.redirect_message_key(user.id, project.id) + redis.setex(key, 5.minutes, redirect_message) + end + end + + def redirect_message(rejected: false) + <<~MESSAGE.strip_heredoc + Project '#{redirected_path}' was moved to '#{project.full_path}'. + + Please update your Git remote: + + #{remote_url_message(rejected)} + MESSAGE + end + + def permanent_redirect? + RedirectRoute.permanent.exists?(path: redirected_path) + end + + private + + attr_reader :project, :redirected_path, :protocol, :user + + def self.redirect_message_key(user_id, project_id) + "#{REDIRECT_NAMESPACE}:#{user_id}:#{project_id}" + end + + def remote_url_message(rejected) + if rejected + "git remote set-url origin #{url} and try again." + else + "git remote set-url origin #{url}" + end + end + + def url + protocol == 'ssh' ? project.ssh_url_to_repo : project.http_url_to_repo + end + end + end +end diff --git a/lib/gitlab/ci/pipeline/chain/base.rb b/lib/gitlab/ci/pipeline/chain/base.rb index 8d82e1b288d..efed19da21c 100644 --- a/lib/gitlab/ci/pipeline/chain/base.rb +++ b/lib/gitlab/ci/pipeline/chain/base.rb @@ -3,14 +3,13 @@ module Gitlab module Pipeline module Chain class Base - attr_reader :pipeline, :project, :current_user + attr_reader :pipeline, :command + + delegate :project, :current_user, to: :command def initialize(pipeline, command) @pipeline = pipeline @command = command - - @project = command.project - @current_user = command.current_user end def perform! diff --git a/lib/gitlab/ci/pipeline/chain/build.rb b/lib/gitlab/ci/pipeline/chain/build.rb index a126dded1ae..70732d26bbd 100644 --- a/lib/gitlab/ci/pipeline/chain/build.rb +++ b/lib/gitlab/ci/pipeline/chain/build.rb @@ -3,20 +3,18 @@ module Gitlab module Pipeline module Chain class Build < Chain::Base - include Chain::Helpers - def perform! @pipeline.assign_attributes( source: @command.source, - project: @project, - ref: ref, - sha: sha, - before_sha: before_sha, - tag: tag_exists?, + project: @command.project, + ref: @command.ref, + sha: @command.sha, + before_sha: @command.before_sha, + tag: @command.tag_exists?, trigger_requests: Array(@command.trigger_request), - user: @current_user, + user: @command.current_user, pipeline_schedule: @command.schedule, - protected: protected_ref? + protected: @command.protected_ref? ) @pipeline.set_config_source @@ -25,32 +23,6 @@ module Gitlab def break? false end - - private - - def ref - @ref ||= Gitlab::Git.ref_name(origin_ref) - end - - def sha - @project.commit(origin_sha || origin_ref).try(:id) - end - - def origin_ref - @command.origin_ref - end - - def origin_sha - @command.checkout_sha || @command.after_sha - end - - def before_sha - @command.checkout_sha || @command.before_sha || Gitlab::Git::BLANK_SHA - end - - def protected_ref? - @project.protected_for?(ref) - end end end end diff --git a/lib/gitlab/ci/pipeline/chain/command.rb b/lib/gitlab/ci/pipeline/chain/command.rb new file mode 100644 index 00000000000..7b19b10e05b --- /dev/null +++ b/lib/gitlab/ci/pipeline/chain/command.rb @@ -0,0 +1,61 @@ +module Gitlab + module Ci + module Pipeline + module Chain + Command = Struct.new( + :source, :project, :current_user, + :origin_ref, :checkout_sha, :after_sha, :before_sha, + :trigger_request, :schedule, + :ignore_skip_ci, :save_incompleted, + :seeds_block + ) do + include Gitlab::Utils::StrongMemoize + + def initialize(**params) + params.each do |key, value| + self[key] = value + end + end + + def branch_exists? + strong_memoize(:is_branch) do + project.repository.branch_exists?(ref) + end + end + + def tag_exists? + strong_memoize(:is_tag) do + project.repository.tag_exists?(ref) + end + end + + def ref + strong_memoize(:ref) do + Gitlab::Git.ref_name(origin_ref) + end + end + + def sha + strong_memoize(:sha) do + project.commit(origin_sha || origin_ref).try(:id) + end + end + + def origin_sha + checkout_sha || after_sha + end + + def before_sha + self[:before_sha] || checkout_sha || Gitlab::Git::BLANK_SHA + end + + def protected_ref? + strong_memoize(:protected_ref) do + project.protected_for?(ref) + end + end + end + end + end + end +end diff --git a/lib/gitlab/ci/pipeline/chain/helpers.rb b/lib/gitlab/ci/pipeline/chain/helpers.rb index 02d81286f21..bf1380a1da9 100644 --- a/lib/gitlab/ci/pipeline/chain/helpers.rb +++ b/lib/gitlab/ci/pipeline/chain/helpers.rb @@ -3,18 +3,6 @@ module Gitlab module Pipeline module Chain module Helpers - def branch_exists? - return @is_branch if defined?(@is_branch) - - @is_branch = project.repository.branch_exists?(pipeline.ref) - end - - def tag_exists? - return @is_tag if defined?(@is_tag) - - @is_tag = project.repository.tag_exists?(pipeline.ref) - end - def error(message) pipeline.errors.add(:base, message) end diff --git a/lib/gitlab/ci/pipeline/chain/validate/abilities.rb b/lib/gitlab/ci/pipeline/chain/validate/abilities.rb index 4913a604079..13c6fedd831 100644 --- a/lib/gitlab/ci/pipeline/chain/validate/abilities.rb +++ b/lib/gitlab/ci/pipeline/chain/validate/abilities.rb @@ -14,7 +14,7 @@ module Gitlab unless allowed_to_trigger_pipeline? if can?(current_user, :create_pipeline, project) - return error("Insufficient permissions for protected ref '#{pipeline.ref}'") + return error("Insufficient permissions for protected ref '#{command.ref}'") else return error('Insufficient permissions to create a new pipeline') end @@ -29,7 +29,7 @@ module Gitlab if current_user allowed_to_create? else # legacy triggers don't have a corresponding user - !project.protected_for?(@pipeline.ref) + !@command.protected_ref? end end @@ -38,10 +38,10 @@ module Gitlab access = Gitlab::UserAccess.new(current_user, project: project) - if branch_exists? - access.can_update_branch?(@pipeline.ref) - elsif tag_exists? - access.can_create_tag?(@pipeline.ref) + if @command.branch_exists? + access.can_update_branch?(@command.ref) + elsif @command.tag_exists? + access.can_create_tag?(@command.ref) else true # Allow it for now and we'll reject when we check ref existence end diff --git a/lib/gitlab/ci/pipeline/chain/validate/repository.rb b/lib/gitlab/ci/pipeline/chain/validate/repository.rb index 70a4cfdbdea..9699c24e5b6 100644 --- a/lib/gitlab/ci/pipeline/chain/validate/repository.rb +++ b/lib/gitlab/ci/pipeline/chain/validate/repository.rb @@ -7,14 +7,11 @@ module Gitlab include Chain::Helpers def perform! - unless branch_exists? || tag_exists? + unless @command.branch_exists? || @command.tag_exists? return error('Reference not found') end - ## TODO, we check commit in the service, that is why - # there is no repository access here. - # - unless pipeline.sha + unless @command.sha return error('Commit not found') end end diff --git a/lib/gitlab/ee_compat_check.rb b/lib/gitlab/ee_compat_check.rb index 4a9d3e52fae..37face8e7d0 100644 --- a/lib/gitlab/ee_compat_check.rb +++ b/lib/gitlab/ee_compat_check.rb @@ -280,7 +280,7 @@ module Gitlab The `#{branch}` branch applies cleanly to EE/master! Much ❤️! For more information, see - https://docs.gitlab.com/ce/development/limit_ee_conflicts.html#check-the-rake-ee_compat_check-in-your-merge-requests + https://docs.gitlab.com/ce/development/automatic_ce_ee_merge.html #{THANKS_FOR_READING_BANNER} } end @@ -357,7 +357,7 @@ module Gitlab Once this is done, you can retry this failed build, and it should pass. Stay 💪 ! For more information, see - https://docs.gitlab.com/ce/development/limit_ee_conflicts.html#check-the-rake-ee_compat_check-in-your-merge-requests + https://docs.gitlab.com/ce/development/automatic_ce_ee_merge.html #{THANKS_FOR_READING_BANNER} } end @@ -378,7 +378,7 @@ module Gitlab retry this build. Stay 💪 ! For more information, see - https://docs.gitlab.com/ce/development/limit_ee_conflicts.html#check-the-rake-ee_compat_check-in-your-merge-requests + https://docs.gitlab.com/ce/development/automatic_ce_ee_merge.html #{THANKS_FOR_READING_BANNER} } end diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 73889328f36..fbdd646ea13 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -20,6 +20,7 @@ module Gitlab SEARCH_CONTEXT_LINES = 3 REBASE_WORKTREE_PREFIX = 'rebase'.freeze SQUASH_WORKTREE_PREFIX = 'squash'.freeze + GITALY_INTERNAL_URL = 'ssh://gitaly/internal.git'.freeze NoRepository = Class.new(StandardError) InvalidBlobName = Class.new(StandardError) @@ -887,8 +888,11 @@ module Gitlab end end - def add_remote(remote_name, url) + # If `mirror_refmap` is present the remote is set as mirror with that mapping + def add_remote(remote_name, url, mirror_refmap: nil) rugged.remotes.create(remote_name, url) + + set_remote_as_mirror(remote_name, refmap: mirror_refmap) if mirror_refmap rescue Rugged::ConfigError remote_update(remote_name, url: url) end @@ -1128,12 +1132,24 @@ module Gitlab !has_visible_content? end - # Like all public `Gitlab::Git::Repository` methods, this method is part - # of `Repository`'s interface through `method_missing`. - # `Repository` has its own `fetch_remote` which uses `gitlab-shell` and - # takes some extra attributes, so we qualify this method name to prevent confusion. - def fetch_remote_without_shell(remote = 'origin') - run_git(['fetch', remote]).last.zero? + def fetch_repository_as_mirror(repository) + remote_name = "tmp-#{SecureRandom.hex}" + + # Notice that this feature flag is not for `fetch_repository_as_mirror` + # as a whole but for the fetching mechanism (file path or gitaly-ssh). + url, env = gitaly_migrate(:fetch_internal) do |is_enabled| + if is_enabled + repository = RemoteRepository.new(repository) unless repository.is_a?(RemoteRepository) + [GITALY_INTERNAL_URL, repository.fetch_env] + else + [repository.path, nil] + end + end + + add_remote(remote_name, url, mirror_refmap: :all_refs) + fetch_remote(remote_name, env: env) + ensure + remove_remote(remote_name) end def blob_at(sha, path) @@ -1851,7 +1867,7 @@ module Gitlab end def gitaly_fetch_ref(source_repository, source_ref:, target_ref:) - args = %W(fetch --no-tags -f ssh://gitaly/internal.git #{source_ref}:#{target_ref}) + args = %W(fetch --no-tags -f #{GITALY_INTERNAL_URL} #{source_ref}:#{target_ref}) run_git(args, env: source_repository.fetch_env) end @@ -1871,6 +1887,10 @@ module Gitlab rescue Rugged::ReferenceError raise ArgumentError, 'Invalid merge source' end + + def fetch_remote(remote_name = 'origin', env: nil) + run_git(['fetch', remote_name], env: env).last.zero? + end end end end diff --git a/lib/gitlab/git/repository_mirroring.rb b/lib/gitlab/git/repository_mirroring.rb index 392bef69e80..effb1f0ca19 100644 --- a/lib/gitlab/git/repository_mirroring.rb +++ b/lib/gitlab/git/repository_mirroring.rb @@ -17,33 +17,6 @@ module Gitlab rugged.config["remote.#{remote_name}.prune"] = true end - def set_remote_refmap(remote_name, refmap) - Array(refmap).each_with_index do |refspec, i| - refspec = REFMAPS[refspec] || refspec - - # We need multiple `fetch` entries, but Rugged only allows replacing a config, not adding to it. - # To make sure we start from scratch, we set the first using rugged, and use `git` for any others - if i == 0 - rugged.config["remote.#{remote_name}.fetch"] = refspec - else - run_git(%W[config --add remote.#{remote_name}.fetch #{refspec}]) - end - end - end - - # Like all_refs public `Gitlab::Git::Repository` methods, this method is part - # of `Repository`'s interface through `method_missing`. - # `Repository` has its own `fetch_as_mirror` which uses `gitlab-shell` and - # takes some extra attributes, so we qualify this method name to prevent confusion. - def fetch_as_mirror_without_shell(url) - remote_name = "tmp-#{SecureRandom.hex}" - add_remote(remote_name, url) - set_remote_as_mirror(remote_name) - fetch_remote_without_shell(remote_name) - ensure - remove_remote(remote_name) if remote_name - end - def remote_tags(remote) # Each line has this format: "dc872e9fa6963f8f03da6c8f6f264d0845d6b092\trefs/tags/v1.10.0\n" # We want to convert it to: [{ 'v1.10.0' => 'dc872e9fa6963f8f03da6c8f6f264d0845d6b092' }, ...] @@ -85,6 +58,20 @@ module Gitlab private + def set_remote_refmap(remote_name, refmap) + Array(refmap).each_with_index do |refspec, i| + refspec = REFMAPS[refspec] || refspec + + # We need multiple `fetch` entries, but Rugged only allows replacing a config, not adding to it. + # To make sure we start from scratch, we set the first using rugged, and use `git` for any others + if i == 0 + rugged.config["remote.#{remote_name}.fetch"] = refspec + else + run_git(%W[config --add remote.#{remote_name}.fetch #{refspec}]) + end + end + end + def list_remote_tags(remote) tag_list, exit_code, error = nil cmd = %W(#{Gitlab.config.git.bin_path} --git-dir=#{path} ls-remote --tags #{remote}) diff --git a/lib/gitlab/git/storage/checker.rb b/lib/gitlab/git/storage/checker.rb index de63cb4b40c..d3c37f82101 100644 --- a/lib/gitlab/git/storage/checker.rb +++ b/lib/gitlab/git/storage/checker.rb @@ -5,6 +5,8 @@ module Gitlab include CircuitBreakerSettings attr_reader :storage_path, :storage, :hostname, :logger + METRICS_MUTEX = Mutex.new + STORAGE_TIMING_BUCKETS = [0.1, 0.15, 0.25, 0.33, 0.5, 1, 1.5, 2.5, 5, 10, 15].freeze def self.check_all(logger = Rails.logger) threads = Gitlab.config.repositories.storages.keys.map do |storage_name| @@ -19,6 +21,17 @@ module Gitlab end end + def self.check_histogram + @check_histogram ||= + METRICS_MUTEX.synchronize do + @check_histogram || Gitlab::Metrics.histogram(:circuitbreaker_storage_check_duration_seconds, + 'Storage check time in seconds', + {}, + STORAGE_TIMING_BUCKETS + ) + end + end + def initialize(storage, logger = Rails.logger) @storage = storage config = Gitlab.config.repositories.storages[@storage] @@ -45,7 +58,7 @@ module Gitlab end def check - if Gitlab::Git::Storage::ForkedStorageCheck.storage_available?(storage_path, storage_timeout, access_retries) + if perform_access_check track_storage_accessible true else @@ -57,6 +70,15 @@ module Gitlab private + def perform_access_check + start_time = Gitlab::Metrics::System.monotonic_time + + Gitlab::Git::Storage::ForkedStorageCheck.storage_available?(storage_path, storage_timeout, access_retries) + ensure + execution_time = Gitlab::Metrics::System.monotonic_time - start_time + self.class.check_histogram.observe({ storage: storage }, execution_time) + end + def track_storage_inaccessible first_failure = current_failure_info.first_failure || Time.now last_failure = Time.now diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb index 9d7d921bb9c..56f6febe86d 100644 --- a/lib/gitlab/git_access.rb +++ b/lib/gitlab/git_access.rb @@ -102,18 +102,15 @@ module Gitlab end def check_project_moved! - return unless redirected_path + return if redirected_path.nil? - url = protocol == 'ssh' ? project.ssh_url_to_repo : project.http_url_to_repo - message = <<-MESSAGE.strip_heredoc - Project '#{redirected_path}' was moved to '#{project.full_path}'. + project_moved = Checks::ProjectMoved.new(project, user, redirected_path, protocol) - Please update your Git remote and try again: - - git remote set-url origin #{url} - MESSAGE - - raise ProjectMovedError, message + if project_moved.permanent_redirect? + project_moved.add_redirect_message + else + raise ProjectMovedError, project_moved.redirect_message(rejected: true) + end end def check_command_disabled!(cmd) diff --git a/lib/gitlab/identifier.rb b/lib/gitlab/identifier.rb index 94678b6ec40..3f3f10596c5 100644 --- a/lib/gitlab/identifier.rb +++ b/lib/gitlab/identifier.rb @@ -2,9 +2,8 @@ # key-13 or user-36 or last commit module Gitlab module Identifier - def identify(identifier, project, newrev) + def identify(identifier, project = nil, newrev = nil) if identifier.blank? - # Local push from gitlab identify_using_commit(project, newrev) elsif identifier =~ /\Auser-\d+\Z/ # git push over http @@ -17,6 +16,8 @@ module Gitlab # Tries to identify a user based on a commit SHA. def identify_using_commit(project, ref) + return if project.nil? && ref.nil? + commit = project.commit(ref) return if !commit || !commit.author_email diff --git a/lib/gitlab/metrics/samplers/influx_sampler.rb b/lib/gitlab/metrics/samplers/influx_sampler.rb index f4f9b5ca792..5a0f7f28fc8 100644 --- a/lib/gitlab/metrics/samplers/influx_sampler.rb +++ b/lib/gitlab/metrics/samplers/influx_sampler.rb @@ -27,7 +27,6 @@ module Gitlab def sample sample_memory_usage sample_file_descriptors - sample_objects sample_gc flush @@ -48,29 +47,6 @@ module Gitlab add_metric('file_descriptors', value: System.file_descriptor_count) end - if Metrics.mri? - def sample_objects - sample = Allocations.to_hash - counts = sample.each_with_object({}) do |(klass, count), hash| - name = klass.name - - next unless name - - hash[name] = count - end - - # Symbols aren't allocated so we'll need to add those manually. - counts['Symbol'] = Symbol.all_symbols.length - - counts.each do |name, count| - add_metric('object_counts', { count: count }, type: name) - end - end - else - def sample_objects - end - end - def sample_gc time = GC::Profiler.total_time * 1000.0 stats = GC.stat.merge(total_time: time) diff --git a/lib/gitlab/metrics/samplers/ruby_sampler.rb b/lib/gitlab/metrics/samplers/ruby_sampler.rb index f4901be9581..b68800417a2 100644 --- a/lib/gitlab/metrics/samplers/ruby_sampler.rb +++ b/lib/gitlab/metrics/samplers/ruby_sampler.rb @@ -48,7 +48,6 @@ module Gitlab def sample start_time = System.monotonic_time sample_gc - sample_objects metrics[:memory_usage].set(labels, System.memory_usage) metrics[:file_descriptors].set(labels, System.file_descriptor_count) @@ -68,32 +67,6 @@ module Gitlab end end - def sample_objects - list_objects.each do |name, count| - metrics[:objects_total].set(labels.merge(class: name), count) - end - end - - if Metrics.mri? - def list_objects - sample = Allocations.to_hash - counts = sample.each_with_object({}) do |(klass, count), hash| - name = klass.name - - next unless name - - hash[name] = count - end - - # Symbols aren't allocated so we'll need to add those manually. - counts['Symbol'] = Symbol.all_symbols.length - counts - end - else - def list_objects - end - end - def worker_label return {} unless defined?(Unicorn::Worker) @@ -46,6 +46,10 @@ module QA autoload :Create, 'qa/scenario/gitlab/project/create' end + module Repository + autoload :Push, 'qa/scenario/gitlab/repository/push' + end + module Sandbox autoload :Prepare, 'qa/scenario/gitlab/sandbox/prepare' end diff --git a/qa/qa/scenario/gitlab/repository/push.rb b/qa/qa/scenario/gitlab/repository/push.rb new file mode 100644 index 00000000000..b00ab0c313a --- /dev/null +++ b/qa/qa/scenario/gitlab/repository/push.rb @@ -0,0 +1,47 @@ +require "pry-byebug" + +module QA + module Scenario + module Gitlab + module Repository + class Push < Scenario::Template + PAGE_REGEX_CHECK = + %r{\/#{Runtime::Namespace.sandbox_name}\/qa-test[^\/]+\/{1}[^\/]+\z}.freeze + + attr_writer :file_name, + :file_content, + :commit_message, + :branch_name + + def initialize + @file_name = 'file.txt' + @file_content = '# This is test project' + @commit_message = "Add #{@file_name}" + @branch_name = 'master' + end + + def perform + Git::Repository.perform do |repository| + repository.location = Page::Project::Show.act do + unless PAGE_REGEX_CHECK.match(current_path) + raise "To perform this scenario the current page should be project show." + end + + choose_repository_clone_http + repository_location + end + + repository.use_default_credentials + repository.clone + repository.configure_identity('GitLab QA', 'root@gitlab.com') + + repository.add_file(@file_name, @file_content) + repository.commit(@commit_message) + repository.push_changes(@branch_name) + end + end + end + end + end + end +end diff --git a/qa/qa/specs/features/repository/push_spec.rb b/qa/qa/specs/features/repository/push_spec.rb index 30935dc1e13..5b930b9818a 100644 --- a/qa/qa/specs/features/repository/push_spec.rb +++ b/qa/qa/specs/features/repository/push_spec.rb @@ -10,21 +10,10 @@ module QA scenario.description = 'project with repository' end - Git::Repository.perform do |repository| - repository.location = Page::Project::Show.act do - choose_repository_clone_http - repository_location - end - - repository.use_default_credentials - - repository.act do - clone - configure_identity('GitLab QA', 'root@gitlab.com') - add_file('README.md', '# This is test project') - commit('Add README.md') - push_changes - end + Scenario::Gitlab::Repository::Push.perform do |scenario| + scenario.file_name = 'README.md' + scenario.file_content = '# This is test project' + scenario.commit_message = 'Add README.md' end Page::Project::Show.act do diff --git a/rubocop/cop/migration/remove_column.rb b/rubocop/cop/migration/remove_column.rb new file mode 100644 index 00000000000..e53eb2e07b2 --- /dev/null +++ b/rubocop/cop/migration/remove_column.rb @@ -0,0 +1,30 @@ +require_relative '../../migration_helpers' + +module RuboCop + module Cop + module Migration + # Cop that checks if remove_column is used in a regular (not + # post-deployment) migration. + class RemoveColumn < RuboCop::Cop::Cop + include MigrationHelpers + + MSG = '`remove_column` must only be used in post-deployment migrations'.freeze + + def on_def(node) + def_method = node.children[0] + + return unless in_migration?(node) && !in_post_deployment_migration?(node) + return unless def_method == :change || def_method == :up + + node.each_descendant(:send) do |send_node| + send_method = send_node.children[1] + + if send_method == :remove_column + add_offense(send_node, :selector) + end + end + end + end + end + end +end diff --git a/rubocop/migration_helpers.rb b/rubocop/migration_helpers.rb index c3473771178..c066d424437 100644 --- a/rubocop/migration_helpers.rb +++ b/rubocop/migration_helpers.rb @@ -7,5 +7,11 @@ module RuboCop dirname.end_with?('db/migrate', 'db/post_migrate') end + + def in_post_deployment_migration?(node) + dirname = File.dirname(node.location.expression.source_buffer.name) + + dirname.end_with?('db/post_migrate') + end end end diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb index 7621ea50da9..eb52be3d731 100644 --- a/rubocop/rubocop.rb +++ b/rubocop/rubocop.rb @@ -14,6 +14,7 @@ require_relative 'cop/migration/add_index' require_relative 'cop/migration/add_timestamps' require_relative 'cop/migration/datetime' require_relative 'cop/migration/hash_index' +require_relative 'cop/migration/remove_column' require_relative 'cop/migration/remove_concurrent_index' require_relative 'cop/migration/remove_index' require_relative 'cop/migration/reversible_add_column_with_default' diff --git a/scripts/trigger-build-omnibus b/scripts/trigger-build-omnibus index 3c5c22c9372..4ff0e8e10b7 100755 --- a/scripts/trigger-build-omnibus +++ b/scripts/trigger-build-omnibus @@ -66,7 +66,7 @@ module Omnibus raise 'Pipeline timeout!' if timeout? case status - when :pending, :running + when :created, :pending, :running puts "Waiting another #{INTERVAL} seconds ..." sleep INTERVAL when :success diff --git a/spec/controllers/groups/group_members_controller_spec.rb b/spec/controllers/groups/group_members_controller_spec.rb index 9c6d584f59b..362d5cc4514 100644 --- a/spec/controllers/groups/group_members_controller_spec.rb +++ b/spec/controllers/groups/group_members_controller_spec.rb @@ -62,6 +62,25 @@ describe Groups::GroupMembersController do end end + describe 'PUT update' do + let(:requester) { create(:group_member, :access_request, group: group) } + + before do + group.add_owner(user) + sign_in(user) + end + + Gitlab::Access.options.each do |label, value| + it "can change the access level to #{label}" do + xhr :put, :update, group_member: { access_level: value }, + group_id: group, + id: requester + + expect(requester.reload.human_access).to eq(label) + end + end + end + describe 'DELETE destroy' do let(:member) { create(:group_member, :developer, group: group) } diff --git a/spec/controllers/projects/project_members_controller_spec.rb b/spec/controllers/projects/project_members_controller_spec.rb index a34dc27a5ed..290dba0610a 100644 --- a/spec/controllers/projects/project_members_controller_spec.rb +++ b/spec/controllers/projects/project_members_controller_spec.rb @@ -66,6 +66,26 @@ describe Projects::ProjectMembersController do end end + describe 'PUT update' do + let(:requester) { create(:project_member, :access_request, project: project) } + + before do + project.add_master(user) + sign_in(user) + end + + Gitlab::Access.options.each do |label, value| + it "can change the access level to #{label}" do + xhr :put, :update, project_member: { access_level: value }, + namespace_id: project.namespace, + project_id: project, + id: requester + + expect(requester.reload.human_access).to eq(label) + end + end + end + describe 'DELETE destroy' do let(:member) { create(:project_member, :developer, project: project) } diff --git a/spec/factories/users.rb b/spec/factories/users.rb index 4000cd085b7..8ace424f8af 100644 --- a/spec/factories/users.rb +++ b/spec/factories/users.rb @@ -58,6 +58,10 @@ FactoryGirl.define do end end + trait :readme do + project_view :readme + end + factory :omniauth_user do transient do extern_uid '123456' diff --git a/spec/features/groups/members/manage_members.rb b/spec/features/groups/members/manage_members.rb index da1e17225db..21f7b4999ad 100644 --- a/spec/features/groups/members/manage_members.rb +++ b/spec/features/groups/members/manage_members.rb @@ -38,6 +38,27 @@ feature 'Groups > Members > Manage members' do end end + scenario 'do not disclose email addresses', :js do + group.add_owner(user1) + create(:user, email: 'undisclosed_email@gitlab.com', name: "Jane 'invisible' Doe") + + visit group_group_members_path(group) + + find('.select2-container').click + select_input = find('.select2-input') + + select_input.send_keys('@gitlab.com') + wait_for_requests + + expect(page).to have_content('No matches found') + + select_input.native.clear + select_input.send_keys('undisclosed_email@gitlab.com') + wait_for_requests + + expect(page).to have_content("Jane 'invisible' Doe") + end + scenario 'remove user from group', :js do group.add_owner(user1) group.add_developer(user2) diff --git a/spec/features/merge_requests/image_diff_notes.rb b/spec/features/merge_requests/image_diff_notes.rb index 3c53b51e330..021c4e03428 100644 --- a/spec/features/merge_requests/image_diff_notes.rb +++ b/spec/features/merge_requests/image_diff_notes.rb @@ -185,6 +185,18 @@ feature 'image diff notes', :js do expect(page).to have_content(diff_note.note) end end + + describe 'image view modes' do + before do + visit project_commit_path(project, '2f63565e7aac07bcdadb654e253078b727143ec4') + end + + it 'resizes image in onion skin view mode' do + find('.view-modes-menu .onion-skin').click + + expect(find('.onion-skin-frame')['style']).to match('width: 228px; height: 240px;') + end + end end def create_image_diff_note diff --git a/spec/helpers/preferences_helper_spec.rb b/spec/helpers/preferences_helper_spec.rb index 8b8080563d3..749aa25e632 100644 --- a/spec/helpers/preferences_helper_spec.rb +++ b/spec/helpers/preferences_helper_spec.rb @@ -77,15 +77,6 @@ describe PreferencesHelper do end end - def stub_user(messages = {}) - if messages.empty? - allow(helper).to receive(:current_user).and_return(nil) - else - allow(helper).to receive(:current_user) - .and_return(double('user', messages)) - end - end - describe '#default_project_view' do context 'user not signed in' do before do @@ -125,5 +116,70 @@ describe PreferencesHelper do end end end + + context 'user signed in' do + let(:user) { create(:user, :readme) } + let(:project) { create(:project, :public, :repository) } + + before do + helper.instance_variable_set(:@project, project) + allow(helper).to receive(:current_user).and_return(user) + end + + context 'when the user is allowed to see the code' do + it 'returns the project view' do + allow(helper).to receive(:can?).with(user, :download_code, project).and_return(true) + + expect(helper.default_project_view).to eq('readme') + end + end + + context 'with wikis enabled and the right policy for the user' do + before do + project.project_feature.update_attribute(:issues_access_level, 0) + allow(helper).to receive(:can?).with(user, :download_code, project).and_return(false) + end + + it 'returns wiki if the user has the right policy' do + allow(helper).to receive(:can?).with(user, :read_wiki, project).and_return(true) + + expect(helper.default_project_view).to eq('wiki') + end + + it 'returns customize_workflow if the user does not have the right policy' do + allow(helper).to receive(:can?).with(user, :read_wiki, project).and_return(false) + + expect(helper.default_project_view).to eq('customize_workflow') + end + end + + context 'with issues as a feature available' do + it 'return issues' do + allow(helper).to receive(:can?).with(user, :download_code, project).and_return(false) + allow(helper).to receive(:can?).with(user, :read_wiki, project).and_return(false) + + expect(helper.default_project_view).to eq('projects/issues/issues') + end + end + + context 'with no activity, no wikies and no issues' do + it 'returns customize_workflow as default' do + project.project_feature.update_attribute(:issues_access_level, 0) + allow(helper).to receive(:can?).with(user, :download_code, project).and_return(false) + allow(helper).to receive(:can?).with(user, :read_wiki, project).and_return(false) + + expect(helper.default_project_view).to eq('customize_workflow') + end + end + end + end + + def stub_user(messages = {}) + if messages.empty? + allow(helper).to receive(:current_user).and_return(nil) + else + allow(helper).to receive(:current_user) + .and_return(double('user', messages)) + end end end diff --git a/spec/javascripts/boards/boards_store_spec.js b/spec/javascripts/boards/boards_store_spec.js index 9e5b0bd3efe..0e656858182 100644 --- a/spec/javascripts/boards/boards_store_spec.js +++ b/spec/javascripts/boards/boards_store_spec.js @@ -9,7 +9,6 @@ import Vue from 'vue'; import Cookies from 'js-cookie'; -import '~/lib/utils/url_utility'; import '~/boards/models/issue'; import '~/boards/models/label'; import '~/boards/models/list'; diff --git a/spec/javascripts/boards/issue_spec.js b/spec/javascripts/boards/issue_spec.js index 10b88878c2a..41dcb19df3c 100644 --- a/spec/javascripts/boards/issue_spec.js +++ b/spec/javascripts/boards/issue_spec.js @@ -4,7 +4,6 @@ /* global mockBoardService */ import Vue from 'vue'; -import '~/lib/utils/url_utility'; import '~/boards/models/issue'; import '~/boards/models/label'; import '~/boards/models/list'; diff --git a/spec/javascripts/boards/list_spec.js b/spec/javascripts/boards/list_spec.js index d4627223a12..eead396ca7e 100644 --- a/spec/javascripts/boards/list_spec.js +++ b/spec/javascripts/boards/list_spec.js @@ -9,7 +9,6 @@ import Vue from 'vue'; -import '~/lib/utils/url_utility'; import '~/boards/models/issue'; import '~/boards/models/label'; import '~/boards/models/list'; diff --git a/spec/javascripts/filtered_search/filtered_search_manager_spec.js b/spec/javascripts/filtered_search/filtered_search_manager_spec.js index 230c15e5de6..5111632d681 100644 --- a/spec/javascripts/filtered_search/filtered_search_manager_spec.js +++ b/spec/javascripts/filtered_search/filtered_search_manager_spec.js @@ -1,8 +1,8 @@ +import * as urlUtils from '~/lib/utils/url_utility'; import * as recentSearchesStoreSrc from '~/filtered_search/stores/recent_searches_store'; import RecentSearchesService from '~/filtered_search/services/recent_searches_service'; import RecentSearchesServiceError from '~/filtered_search/services/recent_searches_service_error'; import RecentSearchesRoot from '~/filtered_search/recent_searches_root'; -import '~/lib/utils/url_utility'; import '~/lib/utils/common_utils'; import '~/filtered_search/filtered_search_token_keys'; import '~/filtered_search/filtered_search_tokenizer'; @@ -162,7 +162,7 @@ describe('Filtered Search Manager', () => { it('should search with a single word', (done) => { input.value = 'searchTerm'; - spyOn(gl.utils, 'visitUrl').and.callFake((url) => { + spyOn(urlUtils, 'visitUrl').and.callFake((url) => { expect(url).toEqual(`${defaultParams}&search=searchTerm`); done(); }); @@ -173,7 +173,7 @@ describe('Filtered Search Manager', () => { it('should search with multiple words', (done) => { input.value = 'awesome search terms'; - spyOn(gl.utils, 'visitUrl').and.callFake((url) => { + spyOn(urlUtils, 'visitUrl').and.callFake((url) => { expect(url).toEqual(`${defaultParams}&search=awesome+search+terms`); done(); }); @@ -184,7 +184,7 @@ describe('Filtered Search Manager', () => { it('should search with special characters', (done) => { input.value = '~!@#$%^&*()_+{}:<>,.?/'; - spyOn(gl.utils, 'visitUrl').and.callFake((url) => { + spyOn(urlUtils, 'visitUrl').and.callFake((url) => { expect(url).toEqual(`${defaultParams}&search=~!%40%23%24%25%5E%26*()_%2B%7B%7D%3A%3C%3E%2C.%3F%2F`); done(); }); @@ -198,7 +198,7 @@ describe('Filtered Search Manager', () => { ${FilteredSearchSpecHelper.createFilterVisualTokenHTML('label', '~bug')} `); - spyOn(gl.utils, 'visitUrl').and.callFake((url) => { + spyOn(urlUtils, 'visitUrl').and.callFake((url) => { expect(url).toEqual(`${defaultParams}&label_name[]=bug`); done(); }); diff --git a/spec/javascripts/fly_out_nav_spec.js b/spec/javascripts/fly_out_nav_spec.js index 4f20e31f511..a3fa07d5bc2 100644 --- a/spec/javascripts/fly_out_nav_spec.js +++ b/spec/javascripts/fly_out_nav_spec.js @@ -253,7 +253,7 @@ describe('Fly out sidebar navigation', () => { it('shows collapsed only sub-items if icon only sidebar', () => { const subItems = el.querySelector('.sidebar-sub-level-items'); const sidebar = document.createElement('div'); - sidebar.classList.add('sidebar-icons-only'); + sidebar.classList.add('sidebar-collapsed-desktop'); subItems.classList.add('is-fly-out-only'); setSidebar(sidebar); @@ -343,7 +343,7 @@ describe('Fly out sidebar navigation', () => { it('returns true when active & collapsed sidebar', () => { const sidebar = document.createElement('div'); - sidebar.classList.add('sidebar-icons-only'); + sidebar.classList.add('sidebar-collapsed-desktop'); el.classList.add('active'); setSidebar(sidebar); diff --git a/spec/javascripts/gl_dropdown_spec.js b/spec/javascripts/gl_dropdown_spec.js index ca048123bf7..b13d1bf8dff 100644 --- a/spec/javascripts/gl_dropdown_spec.js +++ b/spec/javascripts/gl_dropdown_spec.js @@ -2,7 +2,7 @@ import '~/gl_dropdown'; import '~/lib/utils/common_utils'; -import '~/lib/utils/url_utility'; +import * as urlUtils from '~/lib/utils/url_utility'; describe('glDropdown', function describeDropdown() { preloadFixtures('static/gl_dropdown.html.raw'); @@ -137,13 +137,13 @@ describe('glDropdown', function describeDropdown() { expect(this.dropdownContainerElement).toHaveClass('open'); const randomIndex = Math.floor(Math.random() * (this.projectsData.length - 1)) + 0; navigateWithKeys('down', randomIndex, () => { - spyOn(gl.utils, 'visitUrl').and.stub(); + spyOn(urlUtils, 'visitUrl').and.stub(); navigateWithKeys('enter', null, () => { expect(this.dropdownContainerElement).not.toHaveClass('open'); const link = $(`${ITEM_SELECTOR}:eq(${randomIndex}) a`, this.$dropdownMenuElement); expect(link).toHaveClass('is-active'); const linkedLocation = link.attr('href'); - if (linkedLocation && linkedLocation !== '#') expect(gl.utils.visitUrl).toHaveBeenCalledWith(linkedLocation); + if (linkedLocation && linkedLocation !== '#') expect(urlUtils.visitUrl).toHaveBeenCalledWith(linkedLocation); }); }); }); diff --git a/spec/javascripts/groups/components/app_spec.js b/spec/javascripts/groups/components/app_spec.js index 59d4f7c45c6..97e39f6411b 100644 --- a/spec/javascripts/groups/components/app_spec.js +++ b/spec/javascripts/groups/components/app_spec.js @@ -1,9 +1,9 @@ import Vue from 'vue'; +import * as utils from '~/lib/utils/url_utility'; import appComponent from '~/groups/components/app.vue'; import groupFolderComponent from '~/groups/components/group_folder.vue'; import groupItemComponent from '~/groups/components/group_item.vue'; - import eventHub from '~/groups/event_hub'; import GroupsStore from '~/groups/store/groups_store'; import GroupsService from '~/groups/service/groups_service'; @@ -176,7 +176,7 @@ describe('AppComponent', () => { it('should fetch groups for provided page details and update window state', (done) => { spyOn(vm, 'fetchGroups').and.returnValue(returnServicePromise(mockGroups)); spyOn(vm, 'updateGroups').and.callThrough(); - spyOn(gl.utils, 'mergeUrlParams').and.callThrough(); + spyOn(utils, 'mergeUrlParams').and.callThrough(); spyOn(window.history, 'replaceState'); spyOn($, 'scrollTo'); @@ -192,7 +192,7 @@ describe('AppComponent', () => { setTimeout(() => { expect(vm.isLoading).toBeFalsy(); expect($.scrollTo).toHaveBeenCalledWith(0); - expect(gl.utils.mergeUrlParams).toHaveBeenCalledWith({ page: 2 }, jasmine.any(String)); + expect(utils.mergeUrlParams).toHaveBeenCalledWith({ page: 2 }, jasmine.any(String)); expect(window.history.replaceState).toHaveBeenCalledWith({ page: jasmine.any(String), }, jasmine.any(String), jasmine.any(String)); diff --git a/spec/javascripts/groups/components/group_item_spec.js b/spec/javascripts/groups/components/group_item_spec.js index 0f4fbdae445..618d0022e4f 100644 --- a/spec/javascripts/groups/components/group_item_spec.js +++ b/spec/javascripts/groups/components/group_item_spec.js @@ -1,5 +1,5 @@ import Vue from 'vue'; - +import * as urlUtils from '~/lib/utils/url_utility'; import groupItemComponent from '~/groups/components/group_item.vue'; import groupFolderComponent from '~/groups/components/group_folder.vue'; import eventHub from '~/groups/event_hub'; @@ -136,13 +136,13 @@ describe('GroupItemComponent', () => { const group = Object.assign({}, mockParentGroupItem); group.childrenCount = 0; const newVm = createComponent(group); - spyOn(gl.utils, 'visitUrl').and.stub(); + spyOn(urlUtils, 'visitUrl').and.stub(); spyOn(eventHub, '$emit'); newVm.onClickRowGroup(event); setTimeout(() => { expect(eventHub.$emit).not.toHaveBeenCalled(); - expect(gl.utils.visitUrl).toHaveBeenCalledWith(newVm.group.relativePath); + expect(urlUtils.visitUrl).toHaveBeenCalledWith(newVm.group.relativePath); done(); }, 0); }); diff --git a/spec/javascripts/issue_show/components/app_spec.js b/spec/javascripts/issue_show/components/app_spec.js index 53b8a368d28..729c3c29f22 100644 --- a/spec/javascripts/issue_show/components/app_spec.js +++ b/spec/javascripts/issue_show/components/app_spec.js @@ -1,6 +1,7 @@ import Vue from 'vue'; import '~/render_math'; import '~/render_gfm'; +import * as urlUtils from '~/lib/utils/url_utility'; import issuableApp from '~/issue_show/components/app.vue'; import eventHub from '~/issue_show/event_hub'; import issueShowData from '../mock_data'; @@ -180,7 +181,7 @@ describe('Issuable output', () => { }); it('does not redirect if issue has not moved', (done) => { - spyOn(gl.utils, 'visitUrl'); + spyOn(urlUtils, 'visitUrl'); spyOn(vm.service, 'updateIssuable').and.callFake(() => new Promise((resolve) => { resolve({ json() { @@ -196,7 +197,7 @@ describe('Issuable output', () => { setTimeout(() => { expect( - gl.utils.visitUrl, + urlUtils.visitUrl, ).not.toHaveBeenCalled(); done(); @@ -204,7 +205,7 @@ describe('Issuable output', () => { }); it('redirects if returned web_url has changed', (done) => { - spyOn(gl.utils, 'visitUrl'); + spyOn(urlUtils, 'visitUrl'); spyOn(vm.service, 'updateIssuable').and.callFake(() => new Promise((resolve) => { resolve({ json() { @@ -220,7 +221,7 @@ describe('Issuable output', () => { setTimeout(() => { expect( - gl.utils.visitUrl, + urlUtils.visitUrl, ).toHaveBeenCalledWith('/testing-issue-move'); done(); @@ -319,7 +320,7 @@ describe('Issuable output', () => { describe('deleteIssuable', () => { it('changes URL when deleted', (done) => { - spyOn(gl.utils, 'visitUrl'); + spyOn(urlUtils, 'visitUrl'); spyOn(vm.service, 'deleteIssuable').and.callFake(() => new Promise((resolve) => { resolve({ json() { @@ -332,7 +333,7 @@ describe('Issuable output', () => { setTimeout(() => { expect( - gl.utils.visitUrl, + urlUtils.visitUrl, ).toHaveBeenCalledWith('/test'); done(); @@ -340,7 +341,7 @@ describe('Issuable output', () => { }); it('stops polling when deleting', (done) => { - spyOn(gl.utils, 'visitUrl'); + spyOn(urlUtils, 'visitUrl'); spyOn(vm.poll, 'stop').and.callThrough(); spyOn(vm.service, 'deleteIssuable').and.callFake(() => new Promise((resolve) => { resolve({ diff --git a/spec/javascripts/job_spec.js b/spec/javascripts/job_spec.js index 20c4caa865d..4f06237deb5 100644 --- a/spec/javascripts/job_spec.js +++ b/spec/javascripts/job_spec.js @@ -1,6 +1,6 @@ import { bytesToKiB } from '~/lib/utils/number_utils'; +import * as urlUtils from '~/lib/utils/url_utility'; import '~/lib/utils/datetime_utility'; -import '~/lib/utils/url_utility'; import Job from '~/job'; import '~/breakpoints'; @@ -65,7 +65,7 @@ describe('Job', () => { const deferred2 = $.Deferred(); const deferred3 = $.Deferred(); spyOn($, 'ajax').and.returnValues(deferred1.promise(), deferred2.promise(), deferred3.promise()); - spyOn(gl.utils, 'visitUrl'); + spyOn(urlUtils, 'visitUrl'); deferred1.resolve({ html: '<span>Update<span>', @@ -103,7 +103,7 @@ describe('Job', () => { spyOn($, 'ajax').and.returnValues(deferred1.promise(), deferred2.promise(), deferred3.promise()); - spyOn(gl.utils, 'visitUrl'); + spyOn(urlUtils, 'visitUrl'); deferred1.resolve({ html: '<span>Update<span>', @@ -134,7 +134,7 @@ describe('Job', () => { describe('truncated information', () => { describe('when size is less than total', () => { it('shows information about truncated log', () => { - spyOn(gl.utils, 'visitUrl'); + spyOn(urlUtils, 'visitUrl'); const deferred = $.Deferred(); spyOn($, 'ajax').and.returnValue(deferred.promise()); @@ -153,7 +153,7 @@ describe('Job', () => { it('shows the size in KiB', () => { const size = 50; - spyOn(gl.utils, 'visitUrl'); + spyOn(urlUtils, 'visitUrl'); const deferred = $.Deferred(); spyOn($, 'ajax').and.returnValue(deferred.promise()); @@ -179,7 +179,7 @@ describe('Job', () => { spyOn($, 'ajax').and.returnValues(deferred1.promise(), deferred2.promise(), deferred3.promise()); - spyOn(gl.utils, 'visitUrl'); + spyOn(urlUtils, 'visitUrl'); deferred1.resolve({ html: '<span>Update</span>', @@ -214,7 +214,7 @@ describe('Job', () => { it('renders the raw link', () => { const deferred = $.Deferred(); - spyOn(gl.utils, 'visitUrl'); + spyOn(urlUtils, 'visitUrl'); spyOn($, 'ajax').and.returnValue(deferred.promise()); deferred.resolve({ @@ -236,7 +236,7 @@ describe('Job', () => { describe('when size is equal than total', () => { it('does not show the trunctated information', () => { const deferred = $.Deferred(); - spyOn(gl.utils, 'visitUrl'); + spyOn(urlUtils, 'visitUrl'); spyOn($, 'ajax').and.returnValue(deferred.promise()); deferred.resolve({ @@ -257,7 +257,7 @@ describe('Job', () => { describe('output trace', () => { beforeEach(() => { const deferred = $.Deferred(); - spyOn(gl.utils, 'visitUrl'); + spyOn(urlUtils, 'visitUrl'); spyOn($, 'ajax').and.returnValue(deferred.promise()); deferred.resolve({ diff --git a/spec/javascripts/merge_request_tabs_spec.js b/spec/javascripts/merge_request_tabs_spec.js index e441d1153ed..5076435e7a8 100644 --- a/spec/javascripts/merge_request_tabs_spec.js +++ b/spec/javascripts/merge_request_tabs_spec.js @@ -1,6 +1,7 @@ /* eslint-disable no-var, comma-dangle, object-shorthand */ /* global Notes */ +import * as urlUtils from '~/lib/utils/url_utility'; import '~/merge_request_tabs'; import '~/commit/pipelines/pipelines_bundle'; import '~/breakpoints'; @@ -333,7 +334,7 @@ import 'vendor/jquery.scrollTo'; describe('with note fragment hash', () => { it('should expand and scroll to linked fragment hash #note_xxx', function () { - spyOn(window.gl.utils, 'getLocationHash').and.returnValue(noteId); + spyOn(urlUtils, 'getLocationHash').and.returnValue(noteId); this.class.loadDiff('/foo/bar/merge_requests/1/diffs'); expect(noteId.length).toBeGreaterThan(0); @@ -345,7 +346,7 @@ import 'vendor/jquery.scrollTo'; }); it('should gracefully ignore non-existant fragment hash', function () { - spyOn(window.gl.utils, 'getLocationHash').and.returnValue('note_something-that-does-not-exist'); + spyOn(urlUtils, 'getLocationHash').and.returnValue('note_something-that-does-not-exist'); this.class.loadDiff('/foo/bar/merge_requests/1/diffs'); expect(window.notes.toggleDiffNote).not.toHaveBeenCalled(); @@ -354,7 +355,7 @@ import 'vendor/jquery.scrollTo'; describe('with line number fragment hash', () => { it('should gracefully ignore line number fragment hash', function () { - spyOn(window.gl.utils, 'getLocationHash').and.returnValue(noteLineNumId); + spyOn(urlUtils, 'getLocationHash').and.returnValue(noteLineNumId); this.class.loadDiff('/foo/bar/merge_requests/1/diffs'); expect(noteLineNumId.length).toBeGreaterThan(0); @@ -387,7 +388,7 @@ import 'vendor/jquery.scrollTo'; describe('with note fragment hash', () => { it('should expand and scroll to linked fragment hash #note_xxx', function () { - spyOn(window.gl.utils, 'getLocationHash').and.returnValue(noteId); + spyOn(urlUtils, 'getLocationHash').and.returnValue(noteId); this.class.loadDiff('/foo/bar/merge_requests/1/diffs'); @@ -400,7 +401,7 @@ import 'vendor/jquery.scrollTo'; }); it('should gracefully ignore non-existant fragment hash', function () { - spyOn(window.gl.utils, 'getLocationHash').and.returnValue('note_something-that-does-not-exist'); + spyOn(urlUtils, 'getLocationHash').and.returnValue('note_something-that-does-not-exist'); this.class.loadDiff('/foo/bar/merge_requests/1/diffs'); expect(window.notes.toggleDiffNote).not.toHaveBeenCalled(); @@ -409,7 +410,7 @@ import 'vendor/jquery.scrollTo'; describe('with line number fragment hash', () => { it('should gracefully ignore line number fragment hash', function () { - spyOn(window.gl.utils, 'getLocationHash').and.returnValue(noteLineNumId); + spyOn(urlUtils, 'getLocationHash').and.returnValue(noteLineNumId); this.class.loadDiff('/foo/bar/merge_requests/1/diffs'); expect(noteLineNumId.length).toBeGreaterThan(0); diff --git a/spec/javascripts/monitoring/graph/deployment_spec.js b/spec/javascripts/monitoring/graph/deployment_spec.js index dea42d755d4..bf6ada8185e 100644 --- a/spec/javascripts/monitoring/graph/deployment_spec.js +++ b/spec/javascripts/monitoring/graph/deployment_spec.js @@ -118,7 +118,7 @@ describe('MonitoringDeployment', () => { ).not.toEqual('display: none;'); }); - it('shows the refText inside a text element with the deploy-info-text class', () => { + it('contains date, refs and the "deployed" text', () => { reducedDeploymentData[0].showDeploymentFlag = true; const component = createComponent({ showDeployInfo: true, @@ -129,8 +129,31 @@ describe('MonitoringDeployment', () => { }); expect( - component.$el.querySelector('.deploy-info-text').firstChild.nodeValue.trim(), - ).toEqual(component.refText(reducedDeploymentData[0])); + component.$el.querySelectorAll('.deploy-info-text'), + ).toContainText('Deployed'); + + expect( + component.$el.querySelectorAll('.deploy-info-text'), + ).toContainText('Wed, May 31'); + + expect( + component.$el.querySelectorAll('.deploy-info-text'), + ).toContainText(component.refText(reducedDeploymentData[0])); + }); + + it('contains a link to the commit contents', () => { + reducedDeploymentData[0].showDeploymentFlag = true; + const component = createComponent({ + showDeployInfo: true, + deploymentData: reducedDeploymentData, + graphHeight: 300, + graphWidth: 440, + graphHeightOffset: 120, + }); + + expect( + component.$el.querySelectorAll('.deploy-info-text-link')[0].parentElement.getAttribute('xlink:href'), + ).not.toEqual(''); }); it('should contain a hidden gradient', () => { diff --git a/spec/javascripts/monitoring/graph_spec.js b/spec/javascripts/monitoring/graph_spec.js index fd79abe241a..b1d69752bad 100644 --- a/spec/javascripts/monitoring/graph_spec.js +++ b/spec/javascripts/monitoring/graph_spec.js @@ -4,6 +4,8 @@ import MonitoringMixins from '~/monitoring/mixins/monitoring_mixins'; import eventHub from '~/monitoring/event_hub'; import { deploymentData, convertDatesMultipleSeries, singleRowMetricsMultipleSeries } from './mock_data'; +const tagsPath = 'http://test.host/frontend-fixtures/environments-project/tags'; +const projectPath = 'http://test.host/frontend-fixtures/environments-project'; const createComponent = (propsData) => { const Component = Vue.extend(Graph); @@ -25,6 +27,8 @@ describe('Graph', () => { classType: 'col-md-6', updateAspectRatio: false, deploymentData, + tagsPath, + projectPath, }); expect(component.$el.querySelector('.text-center').innerText.trim()).toBe(component.graphData.title); @@ -37,6 +41,8 @@ describe('Graph', () => { classType: 'col-md-6', updateAspectRatio: false, deploymentData, + tagsPath, + projectPath, }); const transformedHeight = `${component.graphHeight - 100}`; @@ -50,6 +56,8 @@ describe('Graph', () => { classType: 'col-md-6', updateAspectRatio: false, deploymentData, + tagsPath, + projectPath, }); const viewBoxArray = component.outerViewBox.split(' '); @@ -65,6 +73,8 @@ describe('Graph', () => { classType: 'col-md-6', updateAspectRatio: false, deploymentData, + tagsPath, + projectPath, }); spyOn(eventHub, '$emit'); @@ -81,6 +91,8 @@ describe('Graph', () => { classType: 'col-md-6', updateAspectRatio: false, deploymentData, + tagsPath, + projectPath, }); expect(component.yAxisLabel).toEqual(component.graphData.y_label); @@ -98,6 +110,8 @@ describe('Graph', () => { hoveredDate: new Date('Sun Aug 27 2017 06:11:51 GMT-0500 (CDT)'), currentDeployXPos: null, }, + tagsPath, + projectPath, }); component.positionFlag(); diff --git a/spec/javascripts/monitoring/mock_data.js b/spec/javascripts/monitoring/mock_data.js index 6b34855b8b2..1f4e858e731 100644 --- a/spec/javascripts/monitoring/mock_data.js +++ b/spec/javascripts/monitoring/mock_data.js @@ -2430,33 +2430,39 @@ export const deploymentData = [ id: 111, iid: 3, sha: 'f5bcd1d9dac6fa4137e2510b9ccd134ef2e84187', + commitUrl: 'http://test.host/frontend-fixtures/environments-project/commit/f5bcd1d9dac6fa4137e2510b9ccd134ef2e84187', ref: { name: 'master' }, created_at: '2017-05-31T21:23:37.881Z', tag: false, + tagUrl: 'http://test.host/frontend-fixtures/environments-project/tags/false', 'last?': true }, { id: 110, iid: 2, sha: 'f5bcd1d9dac6fa4137e2510b9ccd134ef2e84187', + commitUrl: 'http://test.host/frontend-fixtures/environments-project/commit/f5bcd1d9dac6fa4137e2510b9ccd134ef2e84187', ref: { name: 'master' }, created_at: '2017-05-30T20:08:04.629Z', tag: false, + tagUrl: 'http://test.host/frontend-fixtures/environments-project/tags/false', 'last?': false }, { id: 109, iid: 1, sha: '6511e58faafaa7ad2228990ec57f19d66f7db7c2', + commitUrl: 'http://test.host/frontend-fixtures/environments-project/commit/6511e58faafaa7ad2228990ec57f19d66f7db7c2', ref: { name: 'update2-readme' }, created_at: '2017-05-30T17:42:38.409Z', tag: false, + tagUrl: 'http://test.host/frontend-fixtures/environments-project/tags/false', 'last?': false } ]; diff --git a/spec/javascripts/notes/components/noteable_note_spec.js b/spec/javascripts/notes/components/noteable_note_spec.js index 623a2d6d9d7..c8a6cb7e612 100644 --- a/spec/javascripts/notes/components/noteable_note_spec.js +++ b/spec/javascripts/notes/components/noteable_note_spec.js @@ -41,4 +41,19 @@ describe('issue_note', () => { it('should render issue body', () => { expect(vm.$el.querySelector('.note-text').innerHTML).toEqual(note.note_html); }); + + it('prevents note preview xss', (done) => { + const imgSrc = 'data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7'; + const noteBody = `<img src="${imgSrc}" onload="alert(1)" />`; + const alertSpy = spyOn(window, 'alert'); + vm.updateNote = () => new Promise($.noop); + + vm.formUpdateHandler(noteBody, null, $.noop); + + setTimeout(() => { + expect(alertSpy).not.toHaveBeenCalled(); + expect(vm.note.note_html).toEqual(_.escape(noteBody)); + done(); + }, 0); + }); }); diff --git a/spec/javascripts/notes_spec.js b/spec/javascripts/notes_spec.js index 677a389b88f..2612c5fd7bc 100644 --- a/spec/javascripts/notes_spec.js +++ b/spec/javascripts/notes_spec.js @@ -1,6 +1,7 @@ /* eslint-disable space-before-function-paren, no-unused-expressions, no-var, object-shorthand, comma-dangle, max-len */ /* global Notes */ +import * as urlUtils from '~/lib/utils/url_utility'; import 'autosize'; import '~/gl_form'; import '~/lib/utils/text_utility'; @@ -168,8 +169,7 @@ import '~/notes'; }); it('sets target when hash matches', () => { - spyOn(gl.utils, 'getLocationHash'); - gl.utils.getLocationHash.and.returnValue(hash); + spyOn(urlUtils, 'getLocationHash').and.returnValue(hash); Notes.updateNoteTargetSelector($note); @@ -178,8 +178,7 @@ import '~/notes'; }); it('unsets target when hash does not match', () => { - spyOn(gl.utils, 'getLocationHash'); - gl.utils.getLocationHash.and.returnValue('note_doesnotexist'); + spyOn(urlUtils, 'getLocationHash').and.returnValue('note_doesnotexist'); Notes.updateNoteTargetSelector($note); @@ -187,8 +186,7 @@ import '~/notes'; }); it('unsets target when there is not a hash fragment anymore', () => { - spyOn(gl.utils, 'getLocationHash'); - gl.utils.getLocationHash.and.returnValue(null); + spyOn(urlUtils, 'getLocationHash').and.returnValue(null); Notes.updateNoteTargetSelector($note); diff --git a/spec/javascripts/pager_spec.js b/spec/javascripts/pager_spec.js index 1d3e1263371..fe3ea996eac 100644 --- a/spec/javascripts/pager_spec.js +++ b/spec/javascripts/pager_spec.js @@ -1,5 +1,6 @@ /* global fixture */ +import * as utils from '~/lib/utils/url_utility'; import '~/pager'; describe('pager', () => { @@ -30,7 +31,7 @@ describe('pager', () => { it('should use current url if data-href attribute not provided', () => { const href = `${gl.TEST_HOST}/some_list`; - spyOn(gl.utils, 'removeParams').and.returnValue(href); + spyOn(utils, 'removeParams').and.returnValue(href); Pager.init(); expect(Pager.url).toBe(href); }); @@ -44,9 +45,9 @@ describe('pager', () => { it('keeps extra query parameters from url', () => { window.history.replaceState({}, null, '?filter=test&offset=100'); const href = `${gl.TEST_HOST}/some_list?filter=test`; - spyOn(gl.utils, 'removeParams').and.returnValue(href); + spyOn(utils, 'removeParams').and.returnValue(href); Pager.init(); - expect(gl.utils.removeParams).toHaveBeenCalledWith(['limit', 'offset']); + expect(utils.removeParams).toHaveBeenCalledWith(['limit', 'offset']); expect(Pager.url).toEqual(href); }); }); diff --git a/spec/javascripts/repo/components/repo_commit_section_spec.js b/spec/javascripts/repo/components/repo_commit_section_spec.js index 1c794123095..72712e058e5 100644 --- a/spec/javascripts/repo/components/repo_commit_section_spec.js +++ b/spec/javascripts/repo/components/repo_commit_section_spec.js @@ -1,4 +1,5 @@ import Vue from 'vue'; +import * as urlUtils from '~/lib/utils/url_utility'; import store from '~/repo/stores'; import service from '~/repo/services'; import repoCommitSection from '~/repo/components/repo_commit_section.vue'; @@ -97,7 +98,7 @@ describe('RepoCommitSection', () => { }); it('redirects to MR creation page if start new MR checkbox checked', (done) => { - spyOn(gl.utils, 'visitUrl'); + spyOn(urlUtils, 'visitUrl'); vm.startNewMR = true; vm.makeCommit(); @@ -105,7 +106,7 @@ describe('RepoCommitSection', () => { getSetTimeoutPromise() .then(() => Vue.nextTick()) .then(() => { - expect(gl.utils.visitUrl).toHaveBeenCalled(); + expect(urlUtils.visitUrl).toHaveBeenCalled(); }) .then(done) .catch(done.fail); diff --git a/spec/javascripts/repo/stores/actions/tree_spec.js b/spec/javascripts/repo/stores/actions/tree_spec.js index 393a797c6a3..2bbc49d5a9f 100644 --- a/spec/javascripts/repo/stores/actions/tree_spec.js +++ b/spec/javascripts/repo/stores/actions/tree_spec.js @@ -1,4 +1,5 @@ import Vue from 'vue'; +import * as urlUtils from '~/lib/utils/url_utility'; import store from '~/repo/stores'; import service from '~/repo/services'; import { file, resetStore } from '../../helpers'; @@ -255,7 +256,7 @@ describe('Multi-file store tree actions', () => { let row; beforeEach(() => { - spyOn(gl.utils, 'visitUrl'); + spyOn(urlUtils, 'visitUrl'); row = { url: 'submoduleurl', @@ -276,7 +277,7 @@ describe('Multi-file store tree actions', () => { it('opens submodule URL', (done) => { store.dispatch('clickedTreeRow', row) .then(() => { - expect(gl.utils.visitUrl).toHaveBeenCalledWith('submoduleurl'); + expect(urlUtils.visitUrl).toHaveBeenCalledWith('submoduleurl'); done(); }).catch(done.fail); diff --git a/spec/javascripts/repo/stores/actions_spec.js b/spec/javascripts/repo/stores/actions_spec.js index f2a7a698912..21d87e46216 100644 --- a/spec/javascripts/repo/stores/actions_spec.js +++ b/spec/javascripts/repo/stores/actions_spec.js @@ -1,4 +1,5 @@ import Vue from 'vue'; +import * as urlUtils from '~/lib/utils/url_utility'; import store from '~/repo/stores'; import service from '~/repo/services'; import { resetStore, file } from '../helpers'; @@ -10,11 +11,11 @@ describe('Multi-file store actions', () => { describe('redirectToUrl', () => { it('calls visitUrl', (done) => { - spyOn(gl.utils, 'visitUrl'); + spyOn(urlUtils, 'visitUrl'); store.dispatch('redirectToUrl', 'test') .then(() => { - expect(gl.utils.visitUrl).toHaveBeenCalledWith('test'); + expect(urlUtils.visitUrl).toHaveBeenCalledWith('test'); done(); }) @@ -326,13 +327,13 @@ describe('Multi-file store actions', () => { }); it('redirects to new merge request page', (done) => { - spyOn(gl.utils, 'visitUrl'); + spyOn(urlUtils, 'visitUrl'); store.state.endpoints.newMergeRequestUrl = 'newMergeRequestUrl?branch='; store.dispatch('commitChanges', { payload, newMr: true }) .then(() => { - expect(gl.utils.visitUrl).toHaveBeenCalledWith('newMergeRequestUrl?branch=master'); + expect(urlUtils.visitUrl).toHaveBeenCalledWith('newMergeRequestUrl?branch=master'); done(); }).catch(done.fail); diff --git a/spec/javascripts/search_autocomplete_spec.js b/spec/javascripts/search_autocomplete_spec.js index fdfc59a6f12..1e4c2c9faad 100644 --- a/spec/javascripts/search_autocomplete_spec.js +++ b/spec/javascripts/search_autocomplete_spec.js @@ -3,6 +3,7 @@ import '~/gl_dropdown'; import '~/search_autocomplete'; import '~/lib/utils/common_utils'; +import * as urlUtils from '~/lib/utils/url_utility'; (function() { var assertLinks, dashboardIssuesPath, dashboardMRsPath, groupIssuesPath, groupMRsPath, groupName, mockDashboardOptions, mockGroupOptions, mockProjectOptions, projectIssuesPath, projectMRsPath, projectName, userId, widget; @@ -121,7 +122,7 @@ import '~/lib/utils/common_utils'; loadFixtures('static/search_autocomplete.html.raw'); // Prevent turbolinks from triggering within gl_dropdown - spyOn(window.gl.utils, 'visitUrl').and.returnValue(true); + spyOn(urlUtils, 'visitUrl').and.returnValue(true); window.gon = {}; window.gon.current_user_id = userId; diff --git a/spec/javascripts/sidebar/sidebar_assignees_spec.js b/spec/javascripts/sidebar/sidebar_assignees_spec.js index 929ba75e67d..b97e24d9dcf 100644 --- a/spec/javascripts/sidebar/sidebar_assignees_spec.js +++ b/spec/javascripts/sidebar/sidebar_assignees_spec.js @@ -4,20 +4,29 @@ import SidebarMediator from '~/sidebar/sidebar_mediator'; import SidebarService from '~/sidebar/services/sidebar_service'; import SidebarStore from '~/sidebar/stores/sidebar_store'; import Mock from './mock_data'; +import mountComponent from '../helpers/vue_mount_component_helper'; describe('sidebar assignees', () => { - let component; - let SidebarAssigneeComponent; + let vm; + let mediator; + let sidebarAssigneesEl; preloadFixtures('issues/open-issue.html.raw'); beforeEach(() => { Vue.http.interceptors.push(Mock.sidebarMockInterceptor); - SidebarAssigneeComponent = Vue.extend(SidebarAssignees); - spyOn(SidebarMediator.prototype, 'saveAssignees').and.callThrough(); - spyOn(SidebarMediator.prototype, 'assignYourself').and.callThrough(); - this.mediator = new SidebarMediator(Mock.mediator); + loadFixtures('issues/open-issue.html.raw'); - this.sidebarAssigneesEl = document.querySelector('#js-vue-sidebar-assignees'); + + mediator = new SidebarMediator(Mock.mediator); + spyOn(mediator, 'saveAssignees').and.callThrough(); + spyOn(mediator, 'assignYourself').and.callThrough(); + + const SidebarAssigneeComponent = Vue.extend(SidebarAssignees); + sidebarAssigneesEl = document.querySelector('#js-vue-sidebar-assignees'); + vm = mountComponent(SidebarAssigneeComponent, { + mediator, + field: sidebarAssigneesEl.dataset.field, + }, sidebarAssigneesEl); }); afterEach(() => { @@ -28,30 +37,24 @@ describe('sidebar assignees', () => { }); it('calls the mediator when saves the assignees', () => { - component = new SidebarAssigneeComponent() - .$mount(this.sidebarAssigneesEl); - component.saveAssignees(); - - expect(SidebarMediator.prototype.saveAssignees).toHaveBeenCalled(); + vm.saveAssignees(); + expect(mediator.saveAssignees).toHaveBeenCalled(); }); it('calls the mediator when "assignSelf" method is called', () => { - component = new SidebarAssigneeComponent() - .$mount(this.sidebarAssigneesEl); - component.assignSelf(); + vm.assignSelf(); - expect(SidebarMediator.prototype.assignYourself).toHaveBeenCalled(); - expect(this.mediator.store.assignees.length).toEqual(1); + expect(mediator.assignYourself).toHaveBeenCalled(); + expect(mediator.store.assignees.length).toEqual(1); }); it('hides assignees until fetched', (done) => { - component = new SidebarAssigneeComponent().$mount(this.sidebarAssigneesEl); - const currentAssignee = this.sidebarAssigneesEl.querySelector('.value'); + const currentAssignee = sidebarAssigneesEl.querySelector('.value'); expect(currentAssignee).toBe(null); - component.store.isFetching.assignees = false; + vm.store.isFetching.assignees = false; Vue.nextTick(() => { - expect(component.$el.querySelector('.value')).toBeVisible(); + expect(vm.$el.querySelector('.value')).toBeVisible(); done(); }); }); diff --git a/spec/javascripts/sidebar/sidebar_mediator_spec.js b/spec/javascripts/sidebar/sidebar_mediator_spec.js index 14c34d5a78c..9efd109b996 100644 --- a/spec/javascripts/sidebar/sidebar_mediator_spec.js +++ b/spec/javascripts/sidebar/sidebar_mediator_spec.js @@ -1,4 +1,5 @@ import Vue from 'vue'; +import * as urlUtils from '~/lib/utils/url_utility'; import SidebarMediator from '~/sidebar/sidebar_mediator'; import SidebarStore from '~/sidebar/stores/sidebar_store'; import SidebarService from '~/sidebar/services/sidebar_service'; @@ -85,12 +86,12 @@ describe('Sidebar mediator', () => { const moveToProjectId = 7; this.mediator.store.setMoveToProjectId(moveToProjectId); spyOn(this.mediator.service, 'moveIssue').and.callThrough(); - spyOn(gl.utils, 'visitUrl'); + spyOn(urlUtils, 'visitUrl'); this.mediator.moveIssue() .then(() => { expect(this.mediator.service.moveIssue).toHaveBeenCalledWith(moveToProjectId); - expect(gl.utils.visitUrl).toHaveBeenCalledWith('/root/some-project/issues/5'); + expect(urlUtils.visitUrl).toHaveBeenCalledWith('/root/some-project/issues/5'); }) .then(done) .catch(done.fail); diff --git a/spec/javascripts/sidebar/sidebar_subscriptions_spec.js b/spec/javascripts/sidebar/sidebar_subscriptions_spec.js index 7adf22b0f1f..a6113cb0bae 100644 --- a/spec/javascripts/sidebar/sidebar_subscriptions_spec.js +++ b/spec/javascripts/sidebar/sidebar_subscriptions_spec.js @@ -26,11 +26,14 @@ describe('Sidebar Subscriptions', function () { }); it('calls the mediator toggleSubscription on event', () => { - spyOn(SidebarMediator.prototype, 'toggleSubscription').and.returnValue(Promise.resolve()); - vm = mountComponent(SidebarSubscriptions, {}); + const mediator = new SidebarMediator(); + spyOn(mediator, 'toggleSubscription').and.returnValue(Promise.resolve()); + vm = mountComponent(SidebarSubscriptions, { + mediator, + }); eventHub.$emit('toggleSubscription'); - expect(SidebarMediator.prototype.toggleSubscription).toHaveBeenCalled(); + expect(mediator.toggleSubscription).toHaveBeenCalled(); }); }); diff --git a/spec/javascripts/todos_spec.js b/spec/javascripts/todos_spec.js index 7d3c9319a11..59e16f0786e 100644 --- a/spec/javascripts/todos_spec.js +++ b/spec/javascripts/todos_spec.js @@ -1,3 +1,4 @@ +import * as urlUtils from '~/lib/utils/url_utility'; import Todos from '~/todos'; import '~/lib/utils/common_utils'; @@ -16,7 +17,7 @@ describe('Todos', () => { it('opens the todo url', (done) => { const todoLink = todoItem.dataset.url; - spyOn(gl.utils, 'visitUrl').and.callFake((url) => { + spyOn(urlUtils, 'visitUrl').and.callFake((url) => { expect(url).toEqual(todoLink); done(); }); @@ -31,7 +32,7 @@ describe('Todos', () => { beforeEach(() => { metakeyEvent = $.Event('click', { keyCode: 91, ctrlKey: true }); - visitUrlSpy = spyOn(gl.utils, 'visitUrl').and.callFake(() => {}); + visitUrlSpy = spyOn(urlUtils, 'visitUrl').and.callFake(() => {}); windowOpenSpy = spyOn(window, 'open').and.callFake(() => {}); }); diff --git a/spec/javascripts/vue_mr_widget/components/mr_widget_deployment_spec.js b/spec/javascripts/vue_mr_widget/components/mr_widget_deployment_spec.js index 7ee998c8fce..d5b8947c86f 100644 --- a/spec/javascripts/vue_mr_widget/components/mr_widget_deployment_spec.js +++ b/spec/javascripts/vue_mr_widget/components/mr_widget_deployment_spec.js @@ -1,4 +1,5 @@ import Vue from 'vue'; +import * as urlUtils from '~/lib/utils/url_utility'; import deploymentComponent from '~/vue_merge_request_widget/components/mr_widget_deployment'; import MRWidgetService from '~/vue_merge_request_widget/services/mr_widget_service'; @@ -108,13 +109,13 @@ describe('MRWidgetDeployment', () => { it('should show a confirm dialog and call service.stopEnvironment when confirmed', (done) => { spyOn(window, 'confirm').and.returnValue(true); spyOn(MRWidgetService, 'stopEnvironment').and.returnValue(returnPromise(true)); - spyOn(gl.utils, 'visitUrl').and.returnValue(true); + spyOn(urlUtils, 'visitUrl').and.returnValue(true); vm = mockStopEnvironment(); expect(window.confirm).toHaveBeenCalled(); expect(MRWidgetService.stopEnvironment).toHaveBeenCalledWith(deploymentMockData.stop_url); setTimeout(() => { - expect(gl.utils.visitUrl).toHaveBeenCalledWith(url); + expect(urlUtils.visitUrl).toHaveBeenCalledWith(url); done(); }, 333); }); diff --git a/spec/lib/banzai/filter/table_of_contents_filter_spec.rb b/spec/lib/banzai/filter/table_of_contents_filter_spec.rb index 85eddde732e..0cfef4ff5bf 100644 --- a/spec/lib/banzai/filter/table_of_contents_filter_spec.rb +++ b/spec/lib/banzai/filter/table_of_contents_filter_spec.rb @@ -65,6 +65,13 @@ describe Banzai::Filter::TableOfContentsFilter do expect(doc.css('h2 a').first.attr('href')).to eq '#one-1' end + it 'prepends a prefix to digits-only ids' do + doc = filter(header(1, "123") + header(2, "1.0")) + + expect(doc.css('h1 a').first.attr('href')).to eq '#anchor-123' + expect(doc.css('h2 a').first.attr('href')).to eq '#anchor-10' + end + it 'supports Unicode' do doc = filter(header(1, '한글')) expect(doc.css('h1 a').first.attr('id')).to eq 'user-content-한글' diff --git a/spec/lib/gitlab/bare_repository_import/importer_spec.rb b/spec/lib/gitlab/bare_repository_import/importer_spec.rb index 7f3bf5fc41c..8a83e446935 100644 --- a/spec/lib/gitlab/bare_repository_import/importer_spec.rb +++ b/spec/lib/gitlab/bare_repository_import/importer_spec.rb @@ -132,6 +132,23 @@ describe Gitlab::BareRepositoryImport::Importer, repository: true do expect(File).to exist(File.join(project.repository_storage_path, project.disk_path + '.git')) end + + it 'moves an existing project to the correct path' do + # This is a quick way to get a valid repository instead of copying an + # existing one. Since it's not persisted, the importer will try to + # create the project. + project = build(:project, :repository) + original_commit_count = project.repository.commit_count + + bare_repo = Gitlab::BareRepositoryImport::Repository.new(project.repository_storage_path, project.repository.path) + gitlab_importer = described_class.new(admin, bare_repo) + + expect(gitlab_importer).to receive(:create_project).and_call_original + + new_project = gitlab_importer.create_project_if_needed + + expect(new_project.repository.commit_count).to eq(original_commit_count) + end end context 'with Wiki' do diff --git a/spec/lib/gitlab/bare_repository_import/repository_spec.rb b/spec/lib/gitlab/bare_repository_import/repository_spec.rb index 2db737f5fb6..61b73abcba4 100644 --- a/spec/lib/gitlab/bare_repository_import/repository_spec.rb +++ b/spec/lib/gitlab/bare_repository_import/repository_spec.rb @@ -46,6 +46,13 @@ describe ::Gitlab::BareRepositoryImport::Repository do describe '#project_full_path' do it 'returns the project full path' do expect(project_repo_path.repo_path).to eq('/full/path/to/repo.git') + expect(project_repo_path.project_full_path).to eq('to/repo') + end + + it 'with no trailing slash in the root path' do + repo_path = described_class.new('/full/path', '/full/path/to/repo.git') + + expect(repo_path.project_full_path).to eq('to/repo') end end end diff --git a/spec/lib/gitlab/checks/project_moved_spec.rb b/spec/lib/gitlab/checks/project_moved_spec.rb new file mode 100644 index 00000000000..fa1575e2177 --- /dev/null +++ b/spec/lib/gitlab/checks/project_moved_spec.rb @@ -0,0 +1,81 @@ +require 'rails_helper' + +describe Gitlab::Checks::ProjectMoved, :clean_gitlab_redis_shared_state do + let(:user) { create(:user) } + let(:project) { create(:project) } + + describe '.fetch_redirct_message' do + context 'with a redirect message queue' do + it 'should return the redirect message' do + project_moved = described_class.new(project, user, 'foo/bar', 'http') + project_moved.add_redirect_message + + expect(described_class.fetch_redirect_message(user.id, project.id)).to eq(project_moved.redirect_message) + end + + it 'should delete the redirect message from redis' do + project_moved = described_class.new(project, user, 'foo/bar', 'http') + project_moved.add_redirect_message + + expect(Gitlab::Redis::SharedState.with { |redis| redis.get("redirect_namespace:#{user.id}:#{project.id}") }).not_to be_nil + described_class.fetch_redirect_message(user.id, project.id) + expect(Gitlab::Redis::SharedState.with { |redis| redis.get("redirect_namespace:#{user.id}:#{project.id}") }).to be_nil + end + end + + context 'with no redirect message queue' do + it 'should return nil' do + expect(described_class.fetch_redirect_message(1, 2)).to be_nil + end + end + end + + describe '#add_redirect_message' do + it 'should queue a redirect message' do + project_moved = described_class.new(project, user, 'foo/bar', 'http') + expect(project_moved.add_redirect_message).to eq("OK") + end + end + + describe '#redirect_message' do + context 'when the push is rejected' do + it 'should return a redirect message telling the user to try again' do + project_moved = described_class.new(project, user, 'foo/bar', 'http') + message = "Project 'foo/bar' was moved to '#{project.full_path}'." + + "\n\nPlease update your Git remote:" + + "\n\n git remote set-url origin #{project.http_url_to_repo} and try again.\n" + + expect(project_moved.redirect_message(rejected: true)).to eq(message) + end + end + + context 'when the push is not rejected' do + it 'should return a redirect message' do + project_moved = described_class.new(project, user, 'foo/bar', 'http') + message = "Project 'foo/bar' was moved to '#{project.full_path}'." + + "\n\nPlease update your Git remote:" + + "\n\n git remote set-url origin #{project.http_url_to_repo}\n" + + expect(project_moved.redirect_message).to eq(message) + end + end + end + + describe '#permanent_redirect?' do + context 'with a permanent RedirectRoute' do + it 'should return true' do + project.route.create_redirect('foo/bar', permanent: true) + project_moved = described_class.new(project, user, 'foo/bar', 'http') + expect(project_moved.permanent_redirect?).to be_truthy + end + end + + context 'without a permanent RedirectRoute' do + it 'should return false' do + project.route.create_redirect('foo/bar') + project_moved = described_class.new(project, user, 'foo/bar', 'http') + expect(project_moved.permanent_redirect?).to be_falsy + end + end + end +end diff --git a/spec/lib/gitlab/ci/pipeline/chain/build_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/build_spec.rb index 0f1d72080c5..3ae7053a995 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/build_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/build_spec.rb @@ -6,46 +6,81 @@ describe Gitlab::Ci::Pipeline::Chain::Build do let(:pipeline) { Ci::Pipeline.new } let(:command) do - double('command', source: :push, - origin_ref: 'master', - checkout_sha: project.commit.id, - after_sha: nil, - before_sha: nil, - trigger_request: nil, - schedule: nil, - project: project, - current_user: user) + Gitlab::Ci::Pipeline::Chain::Command.new( + source: :push, + origin_ref: 'master', + checkout_sha: project.commit.id, + after_sha: nil, + before_sha: nil, + trigger_request: nil, + schedule: nil, + project: project, + current_user: user) end let(:step) { described_class.new(pipeline, command) } before do stub_repository_ci_yaml_file(sha: anything) - - step.perform! end it 'never breaks the chain' do + step.perform! + expect(step.break?).to be false end it 'fills pipeline object with data' do + step.perform! + expect(pipeline.sha).not_to be_empty expect(pipeline.sha).to eq project.commit.id expect(pipeline.ref).to eq 'master' + expect(pipeline.tag).to be false expect(pipeline.user).to eq user expect(pipeline.project).to eq project end it 'sets a valid config source' do + step.perform! + expect(pipeline.repository_source?).to be true end it 'returns a valid pipeline' do + step.perform! + expect(pipeline).to be_valid end it 'does not persist a pipeline' do + step.perform! + expect(pipeline).not_to be_persisted end + + context 'when pipeline is running for a tag' do + let(:command) do + Gitlab::Ci::Pipeline::Chain::Command.new( + source: :push, + origin_ref: 'mytag', + checkout_sha: project.commit.id, + after_sha: nil, + before_sha: nil, + trigger_request: nil, + schedule: nil, + project: project, + current_user: user) + end + + before do + allow_any_instance_of(Repository).to receive(:tag_exists?).with('mytag').and_return(true) + + step.perform! + end + + it 'correctly indicated that this is a tagged pipeline' do + expect(pipeline).to be_tag + end + end end diff --git a/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb new file mode 100644 index 00000000000..75a177d2d1f --- /dev/null +++ b/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb @@ -0,0 +1,185 @@ +require 'spec_helper' + +describe Gitlab::Ci::Pipeline::Chain::Command do + set(:project) { create(:project, :repository) } + + describe '#initialize' do + subject do + described_class.new(origin_ref: 'master') + end + + it 'properly initialises object from hash' do + expect(subject.origin_ref).to eq('master') + end + end + + context 'handling of origin_ref' do + let(:command) { described_class.new(project: project, origin_ref: origin_ref) } + + describe '#branch_exists?' do + subject { command.branch_exists? } + + context 'for existing branch' do + let(:origin_ref) { 'master' } + + it { is_expected.to eq(true) } + end + + context 'for invalid branch' do + let(:origin_ref) { 'something' } + + it { is_expected.to eq(false) } + end + end + + describe '#tag_exists?' do + subject { command.tag_exists? } + + context 'for existing ref' do + let(:origin_ref) { 'v1.0.0' } + + it { is_expected.to eq(true) } + end + + context 'for invalid ref' do + let(:origin_ref) { 'something' } + + it { is_expected.to eq(false) } + end + end + + describe '#ref' do + subject { command.ref } + + context 'for regular ref' do + let(:origin_ref) { 'master' } + + it { is_expected.to eq('master') } + end + + context 'for branch ref' do + let(:origin_ref) { 'refs/heads/master' } + + it { is_expected.to eq('master') } + end + + context 'for tag ref' do + let(:origin_ref) { 'refs/tags/1.0.0' } + + it { is_expected.to eq('1.0.0') } + end + + context 'for other refs' do + let(:origin_ref) { 'refs/merge-requests/11/head' } + + it { is_expected.to eq('refs/merge-requests/11/head') } + end + end + end + + describe '#sha' do + subject { command.sha } + + context 'when invalid checkout_sha is specified' do + let(:command) { described_class.new(project: project, checkout_sha: 'aaa') } + + it 'returns empty value' do + is_expected.to be_nil + end + end + + context 'when a valid checkout_sha is specified' do + let(:command) { described_class.new(project: project, checkout_sha: project.commit.id) } + + it 'returns checkout_sha' do + is_expected.to eq(project.commit.id) + end + end + + context 'when a valid after_sha is specified' do + let(:command) { described_class.new(project: project, after_sha: project.commit.id) } + + it 'returns after_sha' do + is_expected.to eq(project.commit.id) + end + end + + context 'when a valid origin_ref is specified' do + let(:command) { described_class.new(project: project, origin_ref: 'HEAD') } + + it 'returns SHA for given ref' do + is_expected.to eq(project.commit.id) + end + end + end + + describe '#origin_sha' do + subject { command.origin_sha } + + context 'when using checkout_sha and after_sha' do + let(:command) { described_class.new(project: project, checkout_sha: 'aaa', after_sha: 'bbb') } + + it 'uses checkout_sha' do + is_expected.to eq('aaa') + end + end + + context 'when using after_sha only' do + let(:command) { described_class.new(project: project, after_sha: 'bbb') } + + it 'uses after_sha' do + is_expected.to eq('bbb') + end + end + end + + describe '#before_sha' do + subject { command.before_sha } + + context 'when using checkout_sha and before_sha' do + let(:command) { described_class.new(project: project, checkout_sha: 'aaa', before_sha: 'bbb') } + + it 'uses before_sha' do + is_expected.to eq('bbb') + end + end + + context 'when using checkout_sha only' do + let(:command) { described_class.new(project: project, checkout_sha: 'aaa') } + + it 'uses checkout_sha' do + is_expected.to eq('aaa') + end + end + + context 'when checkout_sha and before_sha are empty' do + let(:command) { described_class.new(project: project) } + + it 'uses BLANK_SHA' do + is_expected.to eq(Gitlab::Git::BLANK_SHA) + end + end + end + + describe '#protected_ref?' do + let(:command) { described_class.new(project: project, origin_ref: 'my-branch') } + + subject { command.protected_ref? } + + context 'when a ref is protected' do + before do + expect_any_instance_of(Project).to receive(:protected_for?).with('my-branch').and_return(true) + end + + it { is_expected.to eq(true) } + end + + context 'when a ref is unprotected' do + before do + expect_any_instance_of(Project).to receive(:protected_for?).with('my-branch').and_return(false) + end + + it { is_expected.to eq(false) } + end + end +end diff --git a/spec/lib/gitlab/ci/pipeline/chain/create_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/create_spec.rb index f54e2326b06..1b03227d67b 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/create_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/create_spec.rb @@ -10,9 +10,9 @@ describe Gitlab::Ci::Pipeline::Chain::Create do end let(:command) do - double('command', project: project, - current_user: user, - seeds_block: nil) + Gitlab::Ci::Pipeline::Chain::Command.new( + project: project, + current_user: user, seeds_block: nil) end let(:step) { described_class.new(pipeline, command) } diff --git a/spec/lib/gitlab/ci/pipeline/chain/sequence_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/sequence_spec.rb index e165e0fac2a..eca23694a2b 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/sequence_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/sequence_spec.rb @@ -5,7 +5,7 @@ describe Gitlab::Ci::Pipeline::Chain::Sequence do set(:user) { create(:user) } let(:pipeline) { build_stubbed(:ci_pipeline) } - let(:command) { double('command' ) } + let(:command) { Gitlab::Ci::Pipeline::Chain::Command.new } let(:first_step) { spy('first step') } let(:second_step) { spy('second step') } let(:sequence) { [first_step, second_step] } diff --git a/spec/lib/gitlab/ci/pipeline/chain/skip_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/skip_spec.rb index 32bd5de829b..dc13cae961c 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/skip_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/skip_spec.rb @@ -6,10 +6,11 @@ describe Gitlab::Ci::Pipeline::Chain::Skip do set(:pipeline) { create(:ci_pipeline, project: project) } let(:command) do - double('command', project: project, - current_user: user, - ignore_skip_ci: false, - save_incompleted: true) + Gitlab::Ci::Pipeline::Chain::Command.new( + project: project, + current_user: user, + ignore_skip_ci: false, + save_incompleted: true) end let(:step) { described_class.new(pipeline, command) } diff --git a/spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb index 0bbdd23f4d6..a973ccda8de 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb @@ -5,11 +5,12 @@ describe Gitlab::Ci::Pipeline::Chain::Validate::Abilities do set(:user) { create(:user) } let(:pipeline) do - build_stubbed(:ci_pipeline, ref: ref, project: project) + build_stubbed(:ci_pipeline, project: project) end let(:command) do - double('command', project: project, current_user: user) + Gitlab::Ci::Pipeline::Chain::Command.new( + project: project, current_user: user, origin_ref: ref) end let(:step) { described_class.new(pipeline, command) } diff --git a/spec/lib/gitlab/ci/pipeline/chain/validate/config_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/validate/config_spec.rb index 8357af38f92..5c12c6e6392 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/validate/config_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/validate/config_spec.rb @@ -5,9 +5,10 @@ describe Gitlab::Ci::Pipeline::Chain::Validate::Config do set(:user) { create(:user) } let(:command) do - double('command', project: project, - current_user: user, - save_incompleted: true) + Gitlab::Ci::Pipeline::Chain::Command.new( + project: project, + current_user: user, + save_incompleted: true) end let!(:step) { described_class.new(pipeline, command) } diff --git a/spec/lib/gitlab/ci/pipeline/chain/validate/repository_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/validate/repository_spec.rb index bb356efe9ad..fb1b53fc55c 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/validate/repository_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/validate/repository_spec.rb @@ -3,10 +3,7 @@ require 'spec_helper' describe Gitlab::Ci::Pipeline::Chain::Validate::Repository do set(:project) { create(:project, :repository) } set(:user) { create(:user) } - - let(:command) do - double('command', project: project, current_user: user) - end + let(:pipeline) { build_stubbed(:ci_pipeline) } let!(:step) { described_class.new(pipeline, command) } @@ -14,9 +11,10 @@ describe Gitlab::Ci::Pipeline::Chain::Validate::Repository do step.perform! end - context 'when pipeline ref and sha exists' do - let(:pipeline) do - build_stubbed(:ci_pipeline, ref: 'master', sha: '123', project: project) + context 'when ref and sha exists' do + let(:command) do + Gitlab::Ci::Pipeline::Chain::Command.new( + project: project, current_user: user, origin_ref: 'master', checkout_sha: project.commit.id) end it 'does not break the chain' do @@ -28,9 +26,10 @@ describe Gitlab::Ci::Pipeline::Chain::Validate::Repository do end end - context 'when pipeline ref does not exist' do - let(:pipeline) do - build_stubbed(:ci_pipeline, ref: 'something', project: project) + context 'when ref does not exist' do + let(:command) do + Gitlab::Ci::Pipeline::Chain::Command.new( + project: project, current_user: user, origin_ref: 'something') end it 'breaks the chain' do @@ -43,9 +42,10 @@ describe Gitlab::Ci::Pipeline::Chain::Validate::Repository do end end - context 'when pipeline does not have SHA set' do - let(:pipeline) do - build_stubbed(:ci_pipeline, ref: 'master', sha: nil, project: project) + context 'when does not have existing SHA set' do + let(:command) do + Gitlab::Ci::Pipeline::Chain::Command.new( + project: project, current_user: user, origin_ref: 'master', checkout_sha: 'something') end it 'breaks the chain' do diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index f19b65a5f71..e6845420f7d 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -588,12 +588,12 @@ describe Gitlab::Git::Repository, seed_helper: true do end end - describe '#fetch_as_mirror_without_shell' do + describe '#fetch_repository_as_mirror' do let(:new_repository) do Gitlab::Git::Repository.new('default', 'my_project.git', '') end - subject { new_repository.fetch_as_mirror_without_shell(repository.path) } + subject { new_repository.fetch_repository_as_mirror(repository) } before do Gitlab::Shell.new.add_repository('default', 'my_project') @@ -603,7 +603,7 @@ describe Gitlab::Git::Repository, seed_helper: true do Gitlab::Shell.new.remove_repository(TestEnv.repos_path, 'my_project') end - it 'fetches a url as a mirror remote' do + it 'fetches a repository as a mirror remote' do subject expect(refs(new_repository.path)).to eq(refs(repository.path)) @@ -1662,21 +1662,6 @@ describe Gitlab::Git::Repository, seed_helper: true do end end - describe '#fetch_remote_without_shell' do - let(:git_path) { Gitlab.config.git.bin_path } - let(:remote_name) { 'my_remote' } - - subject { repository.fetch_remote_without_shell(remote_name) } - - it 'fetches the remote and returns true if the command was successful' do - expect(repository).to receive(:popen) - .with(%W(#{git_path} fetch #{remote_name}), repository.path, {}) - .and_return(['', 0]) - - expect(subject).to be(true) - end - end - describe '#merge' do let(:repository) do Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb index c9643c5da47..2db560c2cec 100644 --- a/spec/lib/gitlab/git_access_spec.rb +++ b/spec/lib/gitlab/git_access_spec.rb @@ -193,7 +193,15 @@ describe Gitlab::GitAccess do let(:actor) { build(:rsa_deploy_key_2048, user: user) } end - describe '#check_project_moved!' do + shared_examples 'check_project_moved' do + it 'enqueues a redirected message' do + push_access_check + + expect(Gitlab::Checks::ProjectMoved.fetch_redirect_message(user.id, project.id)).not_to be_nil + end + end + + describe '#check_project_moved!', :clean_gitlab_redis_shared_state do before do project.add_master(user) end @@ -207,7 +215,40 @@ describe Gitlab::GitAccess do end end - context 'when a redirect was followed to find the project' do + context 'when a permanent redirect and ssh protocol' do + let(:redirected_path) { 'some/other-path' } + + before do + allow_any_instance_of(Gitlab::Checks::ProjectMoved).to receive(:permanent_redirect?).and_return(true) + end + + it 'allows push and pull access' do + aggregate_failures do + expect { push_access_check }.not_to raise_error + end + end + + it_behaves_like 'check_project_moved' + end + + context 'with a permanent redirect and http protocol' do + let(:redirected_path) { 'some/other-path' } + let(:protocol) { 'http' } + + before do + allow_any_instance_of(Gitlab::Checks::ProjectMoved).to receive(:permanent_redirect?).and_return(true) + end + + it 'allows_push and pull access' do + aggregate_failures do + expect { push_access_check }.not_to raise_error + end + end + + it_behaves_like 'check_project_moved' + end + + context 'with a temporal redirect and ssh protocol' do let(:redirected_path) { 'some/other-path' } it 'blocks push and pull access' do @@ -219,16 +260,15 @@ describe Gitlab::GitAccess do expect { pull_access_check }.to raise_error(described_class::ProjectMovedError, /git remote set-url origin #{project.ssh_url_to_repo}/) end end + end - context 'http protocol' do - let(:protocol) { 'http' } + context 'with a temporal redirect and http protocol' do + let(:redirected_path) { 'some/other-path' } + let(:protocol) { 'http' } - it 'includes the path to the project using HTTP' do - aggregate_failures do - expect { push_access_check }.to raise_error(described_class::ProjectMovedError, /git remote set-url origin #{project.http_url_to_repo}/) - expect { pull_access_check }.to raise_error(described_class::ProjectMovedError, /git remote set-url origin #{project.http_url_to_repo}/) - end - end + it 'does not allow to push and pull access' do + expect { push_access_check }.to raise_error(described_class::ProjectMovedError, /git remote set-url origin #{project.http_url_to_repo}/) + expect { pull_access_check }.to raise_error(described_class::ProjectMovedError, /git remote set-url origin #{project.http_url_to_repo}/) end end end diff --git a/spec/lib/gitlab/identifier_spec.rb b/spec/lib/gitlab/identifier_spec.rb index cfaeb1f0d4f..0385dd762c2 100644 --- a/spec/lib/gitlab/identifier_spec.rb +++ b/spec/lib/gitlab/identifier_spec.rb @@ -70,6 +70,10 @@ describe Gitlab::Identifier do expect(identifier.identify_using_commit(project, '123')).to eq(user) end end + + it 'returns nil if the project & ref are not present' do + expect(identifier.identify_using_commit(nil, nil)).to be_nil + end end describe '#identify_using_user' do diff --git a/spec/lib/gitlab/metrics/samplers/influx_sampler_spec.rb b/spec/lib/gitlab/metrics/samplers/influx_sampler_spec.rb index 667e4747897..f66451c5188 100644 --- a/spec/lib/gitlab/metrics/samplers/influx_sampler_spec.rb +++ b/spec/lib/gitlab/metrics/samplers/influx_sampler_spec.rb @@ -21,7 +21,6 @@ describe Gitlab::Metrics::Samplers::InfluxSampler do it 'samples various statistics' do expect(sampler).to receive(:sample_memory_usage) expect(sampler).to receive(:sample_file_descriptors) - expect(sampler).to receive(:sample_objects) expect(sampler).to receive(:sample_gc) expect(sampler).to receive(:flush) @@ -72,28 +71,6 @@ describe Gitlab::Metrics::Samplers::InfluxSampler do end end - if Gitlab::Metrics.mri? - describe '#sample_objects' do - it 'adds a metric containing the amount of allocated objects' do - expect(sampler).to receive(:add_metric) - .with(/object_counts/, an_instance_of(Hash), an_instance_of(Hash)) - .at_least(:once) - .and_call_original - - sampler.sample_objects - end - - it 'ignores classes without a name' do - expect(Allocations).to receive(:to_hash).and_return({ Class.new => 4 }) - - expect(sampler).not_to receive(:add_metric) - .with('object_counts', an_instance_of(Hash), type: nil) - - sampler.sample_objects - end - end - end - describe '#sample_gc' do it 'adds a metric containing garbage collection statistics' do expect(GC::Profiler).to receive(:total_time).and_return(0.24) diff --git a/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb b/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb index 53699327da1..375cbf8a9ca 100644 --- a/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb +++ b/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb @@ -11,7 +11,6 @@ describe Gitlab::Metrics::Samplers::RubySampler do it 'samples various statistics' do expect(Gitlab::Metrics::System).to receive(:memory_usage) expect(Gitlab::Metrics::System).to receive(:file_descriptor_count) - expect(sampler).to receive(:sample_objects) expect(sampler).to receive(:sample_gc) sampler.sample @@ -65,26 +64,4 @@ describe Gitlab::Metrics::Samplers::RubySampler do sampler.sample end end - - if Gitlab::Metrics.mri? - describe '#sample_objects' do - it 'adds a metric containing the amount of allocated objects' do - expect(sampler.metrics[:objects_total]).to receive(:set) - .with(include(class: anything), be > 0) - .at_least(:once) - .and_call_original - - sampler.sample - end - - it 'ignores classes without a name' do - expect(Allocations).to receive(:to_hash).and_return({ Class.new => 4 }) - - expect(sampler.metrics[:objects_total]).not_to receive(:set) - .with(include(class: 'object_counts'), anything) - - sampler.sample - end - end - end end diff --git a/spec/lib/gitlab/reference_extractor_spec.rb b/spec/lib/gitlab/reference_extractor_spec.rb index ef874368077..8ec3f55e6de 100644 --- a/spec/lib/gitlab/reference_extractor_spec.rb +++ b/spec/lib/gitlab/reference_extractor_spec.rb @@ -115,6 +115,15 @@ describe Gitlab::ReferenceExtractor do end end + it 'does not include anchors from table of contents in issue references' do + issue1 = create(:issue, project: project) + issue2 = create(:issue, project: project) + + subject.analyze("not real issue <h4>#{issue1.iid}</h4>, real issue #{issue2.to_reference}") + + expect(subject.issues).to match_array([issue2]) + end + it 'accesses valid issue objects' do @i0 = create(:issue, project: project) @i1 = create(:issue, project: project) diff --git a/spec/migrations/remove_assignee_id_from_issue_spec.rb b/spec/migrations/remove_assignee_id_from_issue_spec.rb new file mode 100644 index 00000000000..2c6f992d3ae --- /dev/null +++ b/spec/migrations/remove_assignee_id_from_issue_spec.rb @@ -0,0 +1,37 @@ +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20170523073948_remove_assignee_id_from_issue.rb') + +describe RemoveAssigneeIdFromIssue, :migration do + let(:issues) { table(:issues) } + let(:issue_assignees) { table(:issue_assignees) } + let(:users) { table(:users) } + + let!(:user_1) { users.create(email: 'email1@example.com') } + let!(:user_2) { users.create(email: 'email2@example.com') } + let!(:user_3) { users.create(email: 'email3@example.com') } + + def create_issue(assignees:) + issues.create.tap do |issue| + assignees.each do |assignee| + issue_assignees.create(issue_id: issue.id, user_id: assignee.id) + end + end + end + + let!(:issue_single_assignee) { create_issue(assignees: [user_1]) } + let!(:issue_no_assignee) { create_issue(assignees: []) } + let!(:issue_multiple_assignees) { create_issue(assignees: [user_2, user_3]) } + + describe '#down' do + it 'sets the assignee_id to a random matching assignee from the assignees table' do + migrate! + disable_migrations_output { described_class.new.down } + + expect(issue_single_assignee.reload.assignee_id).to eq(user_1.id) + expect(issue_no_assignee.reload.assignee_id).to be_nil + expect(issue_multiple_assignees.reload.assignee_id).to eq(user_2.id).or(user_3.id) + + disable_migrations_output { described_class.new.up } + end + end +end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 3817f20bfe7..b7c6286fd83 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -559,4 +559,34 @@ describe Namespace do end end end + + describe "#allowed_path_by_redirects" do + let(:namespace1) { create(:namespace, path: 'foo') } + + context "when the path has been taken before" do + before do + namespace1.path = 'bar' + namespace1.save! + end + + it 'should be invalid' do + namespace2 = build(:group, path: 'foo') + expect(namespace2).to be_invalid + end + + it 'should return an error on path' do + namespace2 = build(:group, path: 'foo') + namespace2.valid? + expect(namespace2.errors.messages[:path].first).to eq('foo has been taken before. Please use another one') + end + end + + context "when the path has not been taken before" do + it 'should be valid' do + expect(RedirectRoute.count).to eq(0) + namespace = build(:namespace) + expect(namespace).to be_valid + end + end + end end diff --git a/spec/models/route_spec.rb b/spec/models/route_spec.rb index fece370c03f..ddad6862a63 100644 --- a/spec/models/route_spec.rb +++ b/spec/models/route_spec.rb @@ -87,6 +87,7 @@ describe Route do end context 'when conflicting redirects exist' do + let(:route) { create(:project).route } let!(:conflicting_redirect1) { route.create_redirect('bar/test') } let!(:conflicting_redirect2) { route.create_redirect('bar/test/foo') } let!(:conflicting_redirect3) { route.create_redirect('gitlab-org') } @@ -141,11 +142,50 @@ describe Route do expect(redirect_route.source).to eq(route.source) expect(redirect_route.path).to eq('foo') end + + context 'when the source is a Project' do + it 'creates a temporal RedirectRoute' do + project = create(:project) + route = project.route + redirect_route = route.create_redirect('foo') + expect(redirect_route.permanent?).to be_falsy + end + end + + context 'when the source is not a project' do + it 'creates a permanent RedirectRoute' do + redirect_route = route.create_redirect('foo', permanent: true) + expect(redirect_route.permanent?).to be_truthy + end + end end describe '#delete_conflicting_redirects' do + context 'with permanent redirect' do + it 'does not delete the redirect' do + route.create_redirect("#{route.path}/foo", permanent: true) + + expect do + route.delete_conflicting_redirects + end.not_to change { RedirectRoute.count } + end + end + + context 'with temporal redirect' do + let(:route) { create(:project).route } + + it 'deletes the redirect' do + route.create_redirect("#{route.path}/foo") + + expect do + route.delete_conflicting_redirects + end.to change { RedirectRoute.count }.by(-1) + end + end + context 'when a redirect route with the same path exists' do context 'when the redirect route has matching case' do + let(:route) { create(:project).route } let!(:redirect1) { route.create_redirect(route.path) } it 'deletes the redirect' do @@ -169,6 +209,7 @@ describe Route do end context 'when the redirect route is differently cased' do + let(:route) { create(:project).route } let!(:redirect1) { route.create_redirect(route.path.upcase) } it 'deletes the redirect' do @@ -185,7 +226,32 @@ describe Route do expect(route.conflicting_redirects).to be_an(ActiveRecord::Relation) end + context 'with permanent redirects' do + it 'does not return anything' do + route.create_redirect("#{route.path}/foo", permanent: true) + route.create_redirect("#{route.path}/foo/bar", permanent: true) + route.create_redirect("#{route.path}/baz/quz", permanent: true) + + expect(route.conflicting_redirects).to be_empty + end + end + + context 'with temporal redirects' do + let(:route) { create(:project).route } + + it 'returns the redirect routes' do + route = create(:project).route + redirect1 = route.create_redirect("#{route.path}/foo") + redirect2 = route.create_redirect("#{route.path}/foo/bar") + redirect3 = route.create_redirect("#{route.path}/baz/quz") + + expect(route.conflicting_redirects).to match_array([redirect1, redirect2, redirect3]) + end + end + context 'when a redirect route with the same path exists' do + let(:route) { create(:project).route } + context 'when the redirect route has matching case' do let!(:redirect1) { route.create_redirect(route.path) } @@ -214,4 +280,42 @@ describe Route do end end end + + describe "#conflicting_redirect_exists?" do + context 'when a conflicting redirect exists' do + let(:group1) { create(:group, path: 'foo') } + let(:group2) { create(:group, path: 'baz') } + + it 'should not be saved' do + group1.path = 'bar' + group1.save + + group2.path = 'foo' + + expect(group2.save).to be_falsy + end + + it 'should return an error on path' do + group1.path = 'bar' + group1.save + + group2.path = 'foo' + group2.valid? + expect(group2.errors["route.path"].first).to eq('foo has been taken before. Please use another one') + end + end + + context 'when a conflicting redirect does not exist' do + let(:project1) { create(:project, path: 'foo') } + let(:project2) { create(:project, path: 'baz') } + + it 'should be saved' do + project1.path = 'bar' + project1.save + + project2.path = 'foo' + expect(project2.save).to be_truthy + end + end + end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 03c96a8f5aa..4687d9dfa00 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -913,11 +913,11 @@ describe User do describe 'email matching' do it 'returns users with a matching Email' do - expect(described_class.search(user.email)).to eq([user, user2]) + expect(described_class.search(user.email)).to eq([user]) end - it 'returns users with a partially matching Email' do - expect(described_class.search(user.email[0..2])).to eq([user, user2]) + it 'does not return users with a partially matching Email' do + expect(described_class.search(user.email[0..2])).not_to include(user, user2) end it 'returns users with a matching Email regardless of the casing' do @@ -973,8 +973,8 @@ describe User do expect(search_with_secondary_emails(user.email)).to eq([user]) end - it 'returns users with a partially matching email' do - expect(search_with_secondary_emails(user.email[0..2])).to eq([user]) + it 'does not return users with a partially matching email' do + expect(search_with_secondary_emails(user.email[0..2])).not_to include([user]) end it 'returns users with a matching email regardless of the casing' do @@ -997,29 +997,8 @@ describe User do expect(search_with_secondary_emails(email.email)).to eq([email.user]) end - it 'returns users with a matching part of secondary email' do - expect(search_with_secondary_emails(email.email[1..4])).to eq([email.user]) - end - - it 'return users with a matching part of secondary email regardless of case' do - expect(search_with_secondary_emails(email.email[1..4].upcase)).to eq([email.user]) - expect(search_with_secondary_emails(email.email[1..4].downcase)).to eq([email.user]) - expect(search_with_secondary_emails(email.email[1..4].capitalize)).to eq([email.user]) - end - - it 'returns multiple users with matching secondary emails' do - email1 = create(:email, email: '1_testemail@example.com') - email2 = create(:email, email: '2_testemail@example.com') - email3 = create(:email, email: 'other@email.com') - email3.user.update_attributes!(email: 'another@mail.com') - - expect( - search_with_secondary_emails('testemail@example.com').map(&:id) - ).to include(email1.user.id, email2.user.id) - - expect( - search_with_secondary_emails('testemail@example.com').map(&:id) - ).not_to include(email3.user.id) + it 'does not return users with a matching part of secondary email' do + expect(search_with_secondary_emails(email.email[1..4])).not_to include([email.user]) end end @@ -2592,4 +2571,28 @@ describe User do include_examples 'max member access for groups' end end + + describe "#username_previously_taken?" do + let(:user1) { create(:user, username: 'foo') } + + context 'when the username has been taken before' do + before do + user1.username = 'bar' + user1.save! + end + + it 'should raise an ActiveRecord::RecordInvalid exception' do + user2 = build(:user, username: 'foo') + expect { user2.save! }.to raise_error(ActiveRecord::RecordInvalid, /Path foo has been taken before/) + end + end + + context 'when the username has not been taken before' do + it 'should be valid' do + expect(RedirectRoute.count).to eq(0) + user2 = build(:user, username: 'baz') + expect(user2).to be_valid + end + end + end end diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index 554723d6b1e..6330c140246 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -173,6 +173,28 @@ describe API::Groups do end describe "GET /groups/:id" do + # Given a group, create one project for each visibility level + # + # group - Group to add projects to + # share_with - If provided, each project will be shared with this Group + # + # Returns a Hash of visibility_level => Project pairs + def add_projects_to_group(group, share_with: nil) + projects = { + public: create(:project, :public, namespace: group), + internal: create(:project, :internal, namespace: group), + private: create(:project, :private, namespace: group) + } + + if share_with + create(:project_group_link, project: projects[:public], group: share_with) + create(:project_group_link, project: projects[:internal], group: share_with) + create(:project_group_link, project: projects[:private], group: share_with) + end + + projects + end + context 'when unauthenticated' do it 'returns 404 for a private group' do get api("/groups/#{group2.id}") @@ -183,6 +205,26 @@ describe API::Groups do get api("/groups/#{group1.id}") expect(response).to have_gitlab_http_status(200) end + + it 'returns only public projects in the group' do + public_group = create(:group, :public) + projects = add_projects_to_group(public_group) + + get api("/groups/#{public_group.id}") + + expect(json_response['projects'].map { |p| p['id'].to_i }) + .to contain_exactly(projects[:public].id) + end + + it 'returns only public projects shared with the group' do + public_group = create(:group, :public) + projects = add_projects_to_group(public_group, share_with: group1) + + get api("/groups/#{group1.id}") + + expect(json_response['shared_projects'].map { |p| p['id'].to_i }) + .to contain_exactly(projects[:public].id) + end end context "when authenticated as user" do @@ -222,6 +264,26 @@ describe API::Groups do expect(response).to have_gitlab_http_status(404) end + + it 'returns only public and internal projects in the group' do + public_group = create(:group, :public) + projects = add_projects_to_group(public_group) + + get api("/groups/#{public_group.id}", user2) + + expect(json_response['projects'].map { |p| p['id'].to_i }) + .to contain_exactly(projects[:public].id, projects[:internal].id) + end + + it 'returns only public and internal projects shared with the group' do + public_group = create(:group, :public) + projects = add_projects_to_group(public_group, share_with: group1) + + get api("/groups/#{group1.id}", user2) + + expect(json_response['shared_projects'].map { |p| p['id'].to_i }) + .to contain_exactly(projects[:public].id, projects[:internal].id) + end end context "when authenticated as admin" do diff --git a/spec/requests/api/internal_spec.rb b/spec/requests/api/internal_spec.rb index 67e1539cbc3..3c31980b273 100644 --- a/spec/requests/api/internal_spec.rb +++ b/spec/requests/api/internal_spec.rb @@ -537,16 +537,7 @@ describe API::Internal do context 'the project path was changed' do let!(:old_path_to_repo) { project.repository.path_to_repo } - let!(:old_full_path) { project.full_path } - let(:project_moved_message) do - <<-MSG.strip_heredoc - Project '#{old_full_path}' was moved to '#{project.full_path}'. - - Please update your Git remote and try again: - - git remote set-url origin #{project.ssh_url_to_repo} - MSG - end + let!(:repository) { project.repository } before do project.team << [user, :developer] @@ -555,19 +546,17 @@ describe API::Internal do end it 'rejects the push' do - push_with_path(key, old_path_to_repo) + push(key, project) expect(response).to have_gitlab_http_status(200) - expect(json_response['status']).to be_falsey - expect(json_response['message']).to eq(project_moved_message) + expect(json_response['status']).to be_falsy end it 'rejects the SSH pull' do - pull_with_path(key, old_path_to_repo) + pull(key, project) expect(response).to have_gitlab_http_status(200) - expect(json_response['status']).to be_falsey - expect(json_response['message']).to eq(project_moved_message) + expect(json_response['status']).to be_falsy end end end @@ -695,7 +684,7 @@ describe API::Internal do # end # end - describe 'POST /internal/post_receive' do + describe 'POST /internal/post_receive', :clean_gitlab_redis_shared_state do let(:identifier) { 'key-123' } let(:valid_params) do @@ -713,6 +702,8 @@ describe API::Internal do before do project.team << [user, :developer] + allow(described_class).to receive(:identify).and_return(user) + allow_any_instance_of(Gitlab::Identifier).to receive(:identify).and_return(user) end it 'enqueues a PostReceive worker job' do @@ -780,6 +771,19 @@ describe API::Internal do expect(json_response['broadcast_message']).to eq(nil) end end + + context 'with a redirected data' do + it 'returns redirected message on the response' do + project_moved = Gitlab::Checks::ProjectMoved.new(project, user, 'foo/baz', 'http') + project_moved.add_redirect_message + + post api("/internal/post_receive"), valid_params + + expect(response).to have_gitlab_http_status(200) + expect(json_response["redirected_message"]).to be_present + expect(json_response["redirected_message"]).to eq(project_moved.redirect_message) + end + end end describe 'POST /internal/pre_receive' do diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb index 99525cd0a6a..3f5070a1fd2 100644 --- a/spec/requests/api/issues_spec.rb +++ b/spec/requests/api/issues_spec.rb @@ -860,6 +860,20 @@ describe API::Issues, :mailer do end end + context 'user does not have permissions to create issue' do + let(:not_member) { create(:user) } + + before do + project.project_feature.update(issues_access_level: ProjectFeature::PRIVATE) + end + + it 'renders 403' do + post api("/projects/#{project.id}/issues", not_member), title: 'new issue' + + expect(response).to have_gitlab_http_status(403) + end + end + it 'creates a new project issue' do post api("/projects/#{project.id}/issues", user), title: 'new issue', labels: 'label, label2', weight: 3, diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb index a16f98bec36..fa02fffc82a 100644 --- a/spec/requests/git_http_spec.rb +++ b/spec/requests/git_http_spec.rb @@ -324,9 +324,9 @@ describe 'Git HTTP requests' do <<-MSG.strip_heredoc Project '#{redirect.path}' was moved to '#{project.full_path}'. - Please update your Git remote and try again: + Please update your Git remote: - git remote set-url origin #{project.http_url_to_repo} + git remote set-url origin #{project.http_url_to_repo} and try again. MSG end @@ -533,9 +533,9 @@ describe 'Git HTTP requests' do <<-MSG.strip_heredoc Project '#{redirect.path}' was moved to '#{project.full_path}'. - Please update your Git remote and try again: + Please update your Git remote: - git remote set-url origin #{project.http_url_to_repo} + git remote set-url origin #{project.http_url_to_repo} and try again. MSG end diff --git a/spec/rubocop/cop/migration/remove_column_spec.rb b/spec/rubocop/cop/migration/remove_column_spec.rb new file mode 100644 index 00000000000..89112f01723 --- /dev/null +++ b/spec/rubocop/cop/migration/remove_column_spec.rb @@ -0,0 +1,68 @@ +require 'spec_helper' + +require 'rubocop' +require 'rubocop/rspec/support' + +require_relative '../../../../rubocop/cop/migration/remove_column' + +describe RuboCop::Cop::Migration::RemoveColumn do + include CopHelper + + subject(:cop) { described_class.new } + + def source(meth = 'change') + "def #{meth}; remove_column :table, :column; end" + end + + context 'in a regular migration' do + before do + allow(cop).to receive(:in_migration?).and_return(true) + allow(cop).to receive(:in_post_deployment_migration?).and_return(false) + end + + it 'registers an offense when remove_column is used in the change method' do + inspect_source(cop, source('change')) + + aggregate_failures do + expect(cop.offenses.size).to eq(1) + expect(cop.offenses.map(&:line)).to eq([1]) + end + end + + it 'registers an offense when remove_column is used in the up method' do + inspect_source(cop, source('up')) + + aggregate_failures do + expect(cop.offenses.size).to eq(1) + expect(cop.offenses.map(&:line)).to eq([1]) + end + end + + it 'registers no offense when remove_column is used in the down method' do + inspect_source(cop, source('down')) + + expect(cop.offenses.size).to eq(0) + end + end + + context 'in a post-deployment migration' do + before do + allow(cop).to receive(:in_migration?).and_return(true) + allow(cop).to receive(:in_post_deployment_migration?).and_return(true) + end + + it 'registers no offense' do + inspect_source(cop, source) + + expect(cop.offenses.size).to eq(0) + end + end + + context 'outside of a migration' do + it 'registers no offense' do + inspect_source(cop, source) + + expect(cop.offenses.size).to eq(0) + end + end +end diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index b0de8d447a2..41ce81e0651 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -518,5 +518,20 @@ describe Ci::CreatePipelineService do end end end + + context 'when pipeline is running for a tag' do + before do + config = YAML.dump(test: { script: 'test', only: ['branches'] }, + deploy: { script: 'deploy', only: ['tags'] }) + + stub_ci_pipeline_yaml_file(config) + end + + it 'creates a tagged pipeline' do + pipeline = execute_service(ref: 'v1.0.0') + + expect(pipeline.tag?).to be true + end + end end end diff --git a/spec/services/members/authorized_destroy_service_spec.rb b/spec/services/members/authorized_destroy_service_spec.rb index 2d04d824180..d4ef31c0c74 100644 --- a/spec/services/members/authorized_destroy_service_spec.rb +++ b/spec/services/members/authorized_destroy_service_spec.rb @@ -45,7 +45,7 @@ describe Members::AuthorizedDestroyService do expect { described_class.new(member, member_user).execute } .to change { number_of_assigned_issuables(member_user) }.from(4).to(2) - expect(issue.reload.assignee_id).to be_nil + expect(issue.reload.assignee_ids).to be_empty expect(merge_request.reload.assignee_id).to be_nil end end diff --git a/spec/services/users/keys_count_service_spec.rb b/spec/services/users/keys_count_service_spec.rb index a188cf86772..bee8380e8b7 100644 --- a/spec/services/users/keys_count_service_spec.rb +++ b/spec/services/users/keys_count_service_spec.rb @@ -15,14 +15,12 @@ describe Users::KeysCountService, :use_clean_rails_memory_store_caching do expect(service.count).to eq(1) end - it 'caches the number of keys in Redis' do + it 'caches the number of keys in Redis', :request_store do + service.delete_cache + control_count = ActiveRecord::QueryRecorder.new { service.count }.count service.delete_cache - recorder = ActiveRecord::QueryRecorder.new do - 2.times { service.count } - end - - expect(recorder.count).to eq(1) + expect { 2.times { service.count } }.not_to exceed_query_limit(control_count) end end |