diff options
160 files changed, 2146 insertions, 784 deletions
diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 4e8f395fa5e..1b58cc10180 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -0.26.0 +0.27.0 diff --git a/GITLAB_SHELL_VERSION b/GITLAB_SHELL_VERSION index 8a30e8f94a3..42cdd0b540f 100644 --- a/GITLAB_SHELL_VERSION +++ b/GITLAB_SHELL_VERSION @@ -1 +1 @@ -5.4.0 +5.7.0 @@ -314,11 +314,11 @@ group :development, :test do gem 'pry-rails', '~> 0.3.4' gem 'awesome_print', '~> 1.2.0', require: false - gem 'fuubar', '~> 2.0.0' + gem 'fuubar', '~> 2.2.0' gem 'database_cleaner', '~> 1.5.0' gem 'factory_girl_rails', '~> 4.7.0' - gem 'rspec-rails', '~> 3.5.0' + gem 'rspec-rails', '~> 3.6.0' gem 'rspec-retry', '~> 0.4.5' gem 'spinach-rails', '~> 0.2.1' gem 'spinach-rerun-reporter', '~> 0.0.2' @@ -391,7 +391,7 @@ gem 'vmstat', '~> 2.3.0' gem 'sys-filesystem', '~> 1.1.6' # Gitaly GRPC client -gem 'gitaly', '~> 0.21.0' +gem 'gitaly', '~> 0.24.0' gem 'toml-rb', '~> 0.3.15', require: false diff --git a/Gemfile.lock b/Gemfile.lock index 5a327a14c4a..57cfe0a9de0 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -156,7 +156,7 @@ GEM devise (~> 4.0) railties rotp (~> 2.0) - diff-lcs (1.2.5) + diff-lcs (1.3) diffy (3.1.0) docile (1.1.5) domain_name (0.5.20161021) @@ -250,8 +250,8 @@ GEM foreman (0.78.0) thor (~> 0.19.1) formatador (0.2.5) - fuubar (2.0.0) - rspec (~> 3.0) + fuubar (2.2.0) + rspec-core (~> 3.0) ruby-progressbar (~> 1.4) gemnasium-gitlab-service (0.2.6) rugged (~> 0.21) @@ -269,7 +269,7 @@ GEM po_to_json (>= 1.0.0) rails (>= 3.2.0) gherkin-ruby (0.3.2) - gitaly (0.21.0) + gitaly (0.24.0) google-protobuf (~> 3.1) grpc (~> 1.0) github-linguist (4.7.6) @@ -393,7 +393,7 @@ GEM json (~> 1.8) multi_xml (>= 0.5.2) httpclient (2.8.2) - i18n (0.8.1) + i18n (0.8.6) ice_nine (0.11.2) influxdb (0.2.3) cause @@ -610,7 +610,7 @@ GEM pry-rails (0.3.5) pry (>= 0.9.10) pyu-ruby-sasl (0.0.3.3) - rack (1.6.5) + rack (1.6.8) rack-accept (0.4.5) rack (>= 0.4) rack-attack (4.4.1) @@ -658,7 +658,7 @@ GEM rainbow (2.2.2) rake raindrops (0.18.0) - rake (10.5.0) + rake (12.0.0) rblineprof (0.3.6) debugger-ruby_core_source (~> 1.3) rdoc (4.2.2) @@ -702,30 +702,26 @@ GEM chunky_png rqrcode-rails3 (0.1.7) rqrcode (>= 0.4.2) - rspec (3.5.0) - rspec-core (~> 3.5.0) - rspec-expectations (~> 3.5.0) - rspec-mocks (~> 3.5.0) - rspec-core (3.5.0) - rspec-support (~> 3.5.0) - rspec-expectations (3.5.0) + rspec-core (3.6.0) + rspec-support (~> 3.6.0) + rspec-expectations (3.6.0) diff-lcs (>= 1.2.0, < 2.0) - rspec-support (~> 3.5.0) - rspec-mocks (3.5.0) + rspec-support (~> 3.6.0) + rspec-mocks (3.6.0) diff-lcs (>= 1.2.0, < 2.0) - rspec-support (~> 3.5.0) - rspec-rails (3.5.0) + rspec-support (~> 3.6.0) + rspec-rails (3.6.0) actionpack (>= 3.0) activesupport (>= 3.0) railties (>= 3.0) - rspec-core (~> 3.5.0) - rspec-expectations (~> 3.5.0) - rspec-mocks (~> 3.5.0) - rspec-support (~> 3.5.0) + rspec-core (~> 3.6.0) + rspec-expectations (~> 3.6.0) + rspec-mocks (~> 3.6.0) + rspec-support (~> 3.6.0) rspec-retry (0.4.5) rspec-core rspec-set (0.1.3) - rspec-support (3.5.0) + rspec-support (3.6.0) rspec_profiling (0.0.5) activerecord pg @@ -860,7 +856,7 @@ GEM truncato (0.7.8) htmlentities (~> 4.3.1) nokogiri (~> 1.6.1) - tzinfo (1.2.2) + tzinfo (1.2.3) thread_safe (~> 0.1) u2f (0.2.1) uglifier (2.7.2) @@ -972,13 +968,13 @@ DEPENDENCIES fog-rackspace (~> 0.1.1) font-awesome-rails (~> 4.7) foreman (~> 0.78.0) - fuubar (~> 2.0.0) + fuubar (~> 2.2.0) gemnasium-gitlab-service (~> 0.2) gemojione (~> 3.0) gettext (~> 3.2.2) gettext_i18n_rails (~> 1.8.0) gettext_i18n_rails_js (~> 1.2.0) - gitaly (~> 0.21.0) + gitaly (~> 0.24.0) github-linguist (~> 4.7.0) gitlab-flowdock-git-hook (~> 1.0.1) gitlab-markup (~> 1.5.1) @@ -1076,7 +1072,7 @@ DEPENDENCIES responders (~> 2.0) rouge (~> 2.0) rqrcode-rails3 (~> 0.1.7) - rspec-rails (~> 3.5.0) + rspec-rails (~> 3.6.0) rspec-retry (~> 0.4.5) rspec-set (~> 0.1.3) rspec_profiling (~> 0.0.5) @@ -1130,4 +1126,4 @@ DEPENDENCIES wikicloth (= 0.8.1) BUNDLED WITH - 1.15.1 + 1.15.3 diff --git a/app/assets/javascripts/ajax_loading_spinner.js b/app/assets/javascripts/ajax_loading_spinner.js index 38a8317dbd7..8f5e2e545ec 100644 --- a/app/assets/javascripts/ajax_loading_spinner.js +++ b/app/assets/javascripts/ajax_loading_spinner.js @@ -10,7 +10,7 @@ class AjaxLoadingSpinner { e.target.setAttribute('disabled', ''); const iconElement = e.target.querySelector('i'); // get first fa- icon - const originalIcon = iconElement.className.match(/(fa-)([^\s]+)/g).first(); + const originalIcon = iconElement.className.match(/(fa-)([^\s]+)/g)[0]; iconElement.dataset.icon = originalIcon; AjaxLoadingSpinner.toggleLoadingIcon(iconElement); $(e.target).off('ajax:beforeSend', AjaxLoadingSpinner.ajaxBeforeSend); diff --git a/app/assets/javascripts/awards_handler.js b/app/assets/javascripts/awards_handler.js index 18cd04b176a..097f79a250a 100644 --- a/app/assets/javascripts/awards_handler.js +++ b/app/assets/javascripts/awards_handler.js @@ -1,6 +1,6 @@ /* eslint-disable class-methods-use-this */ /* global Flash */ - +import _ from 'underscore'; import Cookies from 'js-cookie'; const animationEndEventString = 'animationend webkitAnimationEnd MSAnimationEnd oAnimationEnd'; diff --git a/app/assets/javascripts/behaviors/requires_input.js b/app/assets/javascripts/behaviors/requires_input.js index b20d108aa25..035a7e5c431 100644 --- a/app/assets/javascripts/behaviors/requires_input.js +++ b/app/assets/javascripts/behaviors/requires_input.js @@ -1,3 +1,4 @@ +import _ from 'underscore'; import '../commons/bootstrap'; // Requires Input behavior @@ -48,7 +49,9 @@ function hideOrShowHelpBlock(form) { $(() => { const $form = $('form.js-requires-input'); - $form.requiresInput(); - hideOrShowHelpBlock($form); - $('.select2.js-select-namespace').change(() => hideOrShowHelpBlock($form)); + if ($form) { + $form.requiresInput(); + hideOrShowHelpBlock($form); + $('.select2.js-select-namespace').change(() => hideOrShowHelpBlock($form)); + } }); diff --git a/app/assets/javascripts/behaviors/toggler_behavior.js b/app/assets/javascripts/behaviors/toggler_behavior.js index 77e92ff8caf..b70b0a9bbf8 100644 --- a/app/assets/javascripts/behaviors/toggler_behavior.js +++ b/app/assets/javascripts/behaviors/toggler_behavior.js @@ -1,4 +1,3 @@ - // Toggle button. Show/hide content inside parent container. // Button does not change visibility. If button has icon - it changes chevron style. // diff --git a/app/assets/javascripts/boards/boards_bundle.js b/app/assets/javascripts/boards/boards_bundle.js index 88b054b76e6..89c14180149 100644 --- a/app/assets/javascripts/boards/boards_bundle.js +++ b/app/assets/javascripts/boards/boards_bundle.js @@ -2,6 +2,7 @@ /* global BoardService */ /* global Flash */ +import _ from 'underscore'; import Vue from 'vue'; import VueResource from 'vue-resource'; import FilteredSearchBoards from './filtered_search_boards'; diff --git a/app/assets/javascripts/boards/components/board_blank_state.js b/app/assets/javascripts/boards/components/board_blank_state.js index e7f16899362..edfe7c326db 100644 --- a/app/assets/javascripts/boards/components/board_blank_state.js +++ b/app/assets/javascripts/boards/components/board_blank_state.js @@ -1,5 +1,6 @@ /* global ListLabel */ +import _ from 'underscore'; import Cookies from 'js-cookie'; const Store = gl.issueBoards.BoardsStore; diff --git a/app/assets/javascripts/boards/components/modal/index.js b/app/assets/javascripts/boards/components/modal/index.js index 1d36519c75c..96af69e7a36 100644 --- a/app/assets/javascripts/boards/components/modal/index.js +++ b/app/assets/javascripts/boards/components/modal/index.js @@ -1,8 +1,8 @@ /* global ListIssue */ import Vue from 'vue'; -import queryData from '../../utils/query_data'; -import loadingIcon from '../../../vue_shared/components/loading_icon.vue'; +import queryData from '~/boards/utils/query_data'; +import loadingIcon from '~/vue_shared/components/loading_icon.vue'; import './header'; import './list'; import './footer'; diff --git a/app/assets/javascripts/boards/components/new_list_dropdown.js b/app/assets/javascripts/boards/components/new_list_dropdown.js index f29b6caa1ac..72bb9e10fbc 100644 --- a/app/assets/javascripts/boards/components/new_list_dropdown.js +++ b/app/assets/javascripts/boards/components/new_list_dropdown.js @@ -1,5 +1,6 @@ /* eslint-disable comma-dangle, func-names, no-new, space-before-function-paren, one-var, promise/catch-or-return */ +import _ from 'underscore'; window.gl = window.gl || {}; window.gl.issueBoards = window.gl.issueBoards || {}; diff --git a/app/assets/javascripts/boards/stores/boards_store.js b/app/assets/javascripts/boards/stores/boards_store.js index 1e12d4ca415..43928e602d6 100644 --- a/app/assets/javascripts/boards/stores/boards_store.js +++ b/app/assets/javascripts/boards/stores/boards_store.js @@ -1,6 +1,6 @@ /* eslint-disable comma-dangle, space-before-function-paren, one-var, no-shadow, dot-notation, max-len */ /* global List */ - +import _ from 'underscore'; import Cookies from 'js-cookie'; window.gl = window.gl || {}; diff --git a/app/assets/javascripts/commons/bootstrap.js b/app/assets/javascripts/commons/bootstrap.js index 510bedbf641..389587a2596 100644 --- a/app/assets/javascripts/commons/bootstrap.js +++ b/app/assets/javascripts/commons/bootstrap.js @@ -9,6 +9,7 @@ import 'bootstrap-sass/assets/javascripts/bootstrap/tab'; import 'bootstrap-sass/assets/javascripts/bootstrap/transition'; import 'bootstrap-sass/assets/javascripts/bootstrap/tooltip'; import 'bootstrap-sass/assets/javascripts/bootstrap/popover'; +import 'bootstrap-sass/assets/javascripts/bootstrap/button'; // custom jQuery functions $.fn.extend({ diff --git a/app/assets/javascripts/commons/index.js b/app/assets/javascripts/commons/index.js index 7063f59d446..6db8b3afbef 100644 --- a/app/assets/javascripts/commons/index.js +++ b/app/assets/javascripts/commons/index.js @@ -1,3 +1,4 @@ +import 'underscore'; import './polyfills'; import './jquery'; import './bootstrap'; diff --git a/app/assets/javascripts/copy_as_gfm.js b/app/assets/javascripts/copy_as_gfm.js index 54257531284..13ba4a57293 100644 --- a/app/assets/javascripts/copy_as_gfm.js +++ b/app/assets/javascripts/copy_as_gfm.js @@ -1,5 +1,5 @@ /* eslint-disable class-methods-use-this, object-shorthand, no-unused-vars, no-use-before-define, no-new, max-len, no-restricted-syntax, guard-for-in, no-continue */ - +import _ from 'underscore'; import './lib/utils/common_utils'; import { placeholderImage } from './lazy_loader'; diff --git a/app/assets/javascripts/cycle_analytics/cycle_analytics_bundle.js b/app/assets/javascripts/cycle_analytics/cycle_analytics_bundle.js index 44791a93936..6583e471a48 100644 --- a/app/assets/javascripts/cycle_analytics/cycle_analytics_bundle.js +++ b/app/assets/javascripts/cycle_analytics/cycle_analytics_bundle.js @@ -92,7 +92,7 @@ $(() => { }); }, selectDefaultStage() { - const stage = this.state.stages.first(); + const stage = this.state.stages[0]; this.selectStage(stage); }, selectStage(stage) { diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js index 178e72a1127..ad5ff19ec58 100644 --- a/app/assets/javascripts/dispatcher.js +++ b/app/assets/javascripts/dispatcher.js @@ -79,10 +79,6 @@ import GpgBadges from './gpg_badges'; (function() { var Dispatcher; - $(function() { - return new Dispatcher(); - }); - Dispatcher = (function() { function Dispatcher() { this.initSearch(); @@ -508,7 +504,7 @@ import GpgBadges from './gpg_badges'; new gl.DueDateSelectors(); break; } - switch (path.first()) { + switch (path[0]) { case 'sessions': case 'omniauth_callbacks': if (!gon.u2f) break; @@ -637,4 +633,8 @@ import GpgBadges from './gpg_badges'; return Dispatcher; })(); + + $(function() { + new Dispatcher(); + }); }).call(window); diff --git a/app/assets/javascripts/dropzone_input.js b/app/assets/javascripts/dropzone_input.js index 9ebbb22e807..6d19a6d9b3a 100644 --- a/app/assets/javascripts/dropzone_input.js +++ b/app/assets/javascripts/dropzone_input.js @@ -1,11 +1,10 @@ /* eslint-disable func-names, space-before-function-paren, wrap-iife, max-len, one-var, no-var, one-var-declaration-per-line, no-unused-vars, camelcase, quotes, no-useless-concat, prefer-template, quote-props, comma-dangle, object-shorthand, consistent-return, prefer-arrow-callback */ /* global Dropzone */ - +import _ from 'underscore'; import './preview_markdown'; window.DropzoneInput = (function() { function DropzoneInput(form) { - Dropzone.autoDiscover = false; const divHover = '<div class="div-dropzone-hover"></div>'; const iconPaperclip = '<i class="fa fa-paperclip div-dropzone-icon"></i>'; const $attachButton = form.find('.button-attach-file'); @@ -218,7 +217,7 @@ window.DropzoneInput = (function() { value = e.clipboardData.getData('text/plain'); } value = value.split("\r"); - return value.first(); + return value[0]; }; const showSpinner = function(e) { diff --git a/app/assets/javascripts/emoji/index.js b/app/assets/javascripts/emoji/index.js index cac35d6eed5..dc7672560ea 100644 --- a/app/assets/javascripts/emoji/index.js +++ b/app/assets/javascripts/emoji/index.js @@ -1,3 +1,4 @@ +import _ from 'underscore'; import emojiMap from 'emojis/digests.json'; import emojiAliases from 'emojis/aliases.json'; diff --git a/app/assets/javascripts/extensions/array.js b/app/assets/javascripts/extensions/array.js deleted file mode 100644 index 027222f804d..00000000000 --- a/app/assets/javascripts/extensions/array.js +++ /dev/null @@ -1,11 +0,0 @@ -// TODO: remove this - -// eslint-disable-next-line no-extend-native -Array.prototype.first = function first() { - return this[0]; -}; - -// eslint-disable-next-line no-extend-native -Array.prototype.last = function last() { - return this[this.length - 1]; -}; diff --git a/app/assets/javascripts/filterable_list.js b/app/assets/javascripts/filterable_list.js index 139206cc185..6d516a253bb 100644 --- a/app/assets/javascripts/filterable_list.js +++ b/app/assets/javascripts/filterable_list.js @@ -1,3 +1,5 @@ +import _ from 'underscore'; + /** * Makes search request for content when user types a value in the search input. * Updates the html content of the page with the received one. diff --git a/app/assets/javascripts/filtered_search/dropdown_utils.js b/app/assets/javascripts/filtered_search/dropdown_utils.js index 9c7a4d9f6ad..8d711e3213c 100644 --- a/app/assets/javascripts/filtered_search/dropdown_utils.js +++ b/app/assets/javascripts/filtered_search/dropdown_utils.js @@ -1,3 +1,4 @@ +import _ from 'underscore'; import FilteredSearchContainer from './container'; class DropdownUtils { @@ -122,11 +123,11 @@ class DropdownUtils { if (!allowMultiple && itemInExistingTokens) { updatedItem.droplab_hidden = true; - } else if (!lastKey || searchInput.split('').last() === ' ') { + } else if (!lastKey || _.last(searchInput.split('')) === ' ') { updatedItem.droplab_hidden = false; } else if (lastKey) { const split = lastKey.split(':'); - const tokenName = split[0].split(' ').last(); + const tokenName = _.last(split[0].split(' ')); const match = updatedItem.hint.indexOf(tokenName.toLowerCase()) === -1; updatedItem.droplab_hidden = tokenName ? match : false; diff --git a/app/assets/javascripts/filtered_search/filtered_search_dropdown_manager.js b/app/assets/javascripts/filtered_search/filtered_search_dropdown_manager.js index 47cecd5b5f7..dd1c067df87 100644 --- a/app/assets/javascripts/filtered_search/filtered_search_dropdown_manager.js +++ b/app/assets/javascripts/filtered_search/filtered_search_dropdown_manager.js @@ -167,7 +167,7 @@ class FilteredSearchDropdownManager { // Eg. token = 'label:' const split = lastToken.split(':'); - const dropdownName = split[0].split(' ').last(); + const dropdownName = _.last(split[0].split(' ')); this.loadDropdown(split.length > 1 ? dropdownName : ''); } else if (lastToken) { // Token has been initialized into an object because it has a value diff --git a/app/assets/javascripts/filtered_search/filtered_search_manager.js b/app/assets/javascripts/filtered_search/filtered_search_manager.js index 3ce8b8607ad..a31be2b0bc7 100644 --- a/app/assets/javascripts/filtered_search/filtered_search_manager.js +++ b/app/assets/javascripts/filtered_search/filtered_search_manager.js @@ -367,7 +367,7 @@ class FilteredSearchManager { const fragments = searchToken.split(':'); if (fragments.length > 1) { const inputValues = fragments[0].split(' '); - const tokenKey = inputValues.last(); + const tokenKey = _.last(inputValues); if (inputValues.length > 1) { inputValues.pop(); diff --git a/app/assets/javascripts/gfm_auto_complete.js b/app/assets/javascripts/gfm_auto_complete.js index 6cb9cfe1382..5c624b79d45 100644 --- a/app/assets/javascripts/gfm_auto_complete.js +++ b/app/assets/javascripts/gfm_auto_complete.js @@ -1,3 +1,4 @@ +import _ from 'underscore'; import glRegexp from './lib/utils/regexp'; import AjaxCache from './lib/utils/ajax_cache'; diff --git a/app/assets/javascripts/gl_dropdown.js b/app/assets/javascripts/gl_dropdown.js index 9475498e176..7d11cd0b6b2 100644 --- a/app/assets/javascripts/gl_dropdown.js +++ b/app/assets/javascripts/gl_dropdown.js @@ -1,5 +1,6 @@ /* eslint-disable func-names, space-before-function-paren, no-var, one-var, one-var-declaration-per-line, prefer-rest-params, max-len, vars-on-top, wrap-iife, no-unused-vars, quotes, no-shadow, no-cond-assign, prefer-arrow-callback, no-return-assign, no-else-return, camelcase, comma-dangle, no-lonely-if, guard-for-in, no-restricted-syntax, consistent-return, prefer-template, no-param-reassign, no-loop-func, no-mixed-operators */ /* global fuzzaldrinPlus */ +import _ from 'underscore'; import { isObject } from './lib/utils/type_utility'; var GitLabDropdown, GitLabDropdownFilter, GitLabDropdownRemote; @@ -114,7 +115,7 @@ GitLabDropdownFilter = (function() { } else { elements = this.options.elements(); if (search_text) { - return elements.each(function() { + elements.each(function() { var $el, matches; $el = $(this); matches = fuzzaldrinPlus.match($el.text().trim(), search_text); @@ -127,8 +128,10 @@ GitLabDropdownFilter = (function() { } }); } else { - return elements.show().removeClass('option-hidden'); + elements.show().removeClass('option-hidden'); } + + elements.parent().find('.dropdown-menu-empty-link').toggleClass('hidden', elements.is(':visible')); } }; @@ -731,9 +734,15 @@ GitLabDropdown = (function() { GitLabDropdown.prototype.focusTextInput = function(triggerFocus = false) { if (this.options.filterable) { this.dropdown.one('transitionend', () => { + const initialScrollTop = $(window).scrollTop(); + if (this.dropdown.is('.open')) { this.filterInput.focus(); } + + if ($(window).scrollTop() < initialScrollTop) { + $(window).scrollTop(initialScrollTop); + } }); if (triggerFocus) { diff --git a/app/assets/javascripts/graphs/stat_graph_contributors.js b/app/assets/javascripts/graphs/stat_graph_contributors.js index c6be4c9e8fe..cdc4fcf6573 100644 --- a/app/assets/javascripts/graphs/stat_graph_contributors.js +++ b/app/assets/javascripts/graphs/stat_graph_contributors.js @@ -1,5 +1,6 @@ /* eslint-disable func-names, space-before-function-paren, wrap-iife, no-var, one-var, camelcase, one-var-declaration-per-line, quotes, no-param-reassign, quote-props, comma-dangle, prefer-template, max-len, no-return-assign, no-shadow */ +import _ from 'underscore'; import d3 from 'd3'; import { ContributorsGraph, ContributorsAuthorGraph, ContributorsMasterGraph } from './stat_graph_contributors_graph'; import ContributorsStatGraphUtil from './stat_graph_contributors_util'; diff --git a/app/assets/javascripts/graphs/stat_graph_contributors_graph.js b/app/assets/javascripts/graphs/stat_graph_contributors_graph.js index 0deb27e522b..f64b4638485 100644 --- a/app/assets/javascripts/graphs/stat_graph_contributors_graph.js +++ b/app/assets/javascripts/graphs/stat_graph_contributors_graph.js @@ -1,5 +1,5 @@ /* eslint-disable func-names, space-before-function-paren, no-var, prefer-rest-params, max-len, no-restricted-syntax, vars-on-top, no-use-before-define, no-param-reassign, new-cap, no-underscore-dangle, wrap-iife, comma-dangle, no-return-assign, prefer-arrow-callback, quotes, prefer-template, newline-per-chained-call, no-else-return, no-shadow */ - +import _ from 'underscore'; import d3 from 'd3'; const extend = function(child, parent) { for (var key in parent) { if (hasProp.call(parent, key)) child[key] = parent[key]; } function ctor() { this.constructor = child; } ctor.prototype = parent.prototype; child.prototype = new ctor(); child.__super__ = parent.prototype; return child; }; diff --git a/app/assets/javascripts/graphs/stat_graph_contributors_util.js b/app/assets/javascripts/graphs/stat_graph_contributors_util.js index c583757f3f2..77135ad1f0e 100644 --- a/app/assets/javascripts/graphs/stat_graph_contributors_util.js +++ b/app/assets/javascripts/graphs/stat_graph_contributors_util.js @@ -1,4 +1,5 @@ /* eslint-disable func-names, space-before-function-paren, object-shorthand, no-var, one-var, camelcase, one-var-declaration-per-line, comma-dangle, no-param-reassign, no-return-assign, quotes, prefer-arrow-callback, wrap-iife, consistent-return, no-unused-vars, max-len, no-cond-assign, no-else-return, max-len */ +import _ from 'underscore'; export default { parse_log: function(log) { diff --git a/app/assets/javascripts/issuable_bulk_update_actions.js b/app/assets/javascripts/issuable_bulk_update_actions.js index e46c0e90255..c39ffdb2e0f 100644 --- a/app/assets/javascripts/issuable_bulk_update_actions.js +++ b/app/assets/javascripts/issuable_bulk_update_actions.js @@ -1,6 +1,7 @@ /* eslint-disable comma-dangle, quotes, consistent-return, func-names, array-callback-return, space-before-function-paren, prefer-arrow-callback, max-len, no-unused-expressions, no-sequences, no-underscore-dangle, no-unused-vars, no-param-reassign */ /* global IssuableIndex */ /* global Flash */ +import _ from 'underscore'; export default { init({ container, form, issues, prefixId } = {}) { diff --git a/app/assets/javascripts/issuable_index.js b/app/assets/javascripts/issuable_index.js index 5c96646def8..ece0220c927 100644 --- a/app/assets/javascripts/issuable_index.js +++ b/app/assets/javascripts/issuable_index.js @@ -1,6 +1,6 @@ /* eslint-disable no-param-reassign, func-names, no-var, camelcase, no-unused-vars, object-shorthand, space-before-function-paren, no-return-assign, comma-dangle, consistent-return, one-var, one-var-declaration-per-line, quotes, prefer-template, prefer-arrow-callback, wrap-iife, max-len */ /* global IssuableIndex */ - +import _ from 'underscore'; import IssuableBulkUpdateSidebar from './issuable_bulk_update_sidebar'; import IssuableBulkUpdateActions from './issuable_bulk_update_actions'; diff --git a/app/assets/javascripts/labels_select.js b/app/assets/javascripts/labels_select.js index f0e02ca0fb2..7d7f91227f9 100644 --- a/app/assets/javascripts/labels_select.js +++ b/app/assets/javascripts/labels_select.js @@ -1,7 +1,7 @@ /* eslint-disable no-useless-return, func-names, space-before-function-paren, wrap-iife, no-var, no-underscore-dangle, prefer-arrow-callback, max-len, one-var, no-unused-vars, one-var-declaration-per-line, prefer-template, no-new, consistent-return, object-shorthand, comma-dangle, no-shadow, no-param-reassign, brace-style, vars-on-top, quotes, no-lonely-if, no-else-return, dot-notation, no-empty, no-return-assign, camelcase, prefer-spread */ /* global Issuable */ /* global ListLabel */ - +import _ from 'underscore'; import IssuableBulkUpdateActions from './issuable_bulk_update_actions'; import DropdownUtils from './filtered_search/dropdown_utils'; diff --git a/app/assets/javascripts/lib/utils/common_utils.js b/app/assets/javascripts/lib/utils/common_utils.js index 122ec138c59..e916724b666 100644 --- a/app/assets/javascripts/lib/utils/common_utils.js +++ b/app/assets/javascripts/lib/utils/common_utils.js @@ -86,8 +86,9 @@ // This is required to handle non-unicode characters in hash hash = decodeURIComponent(hash); - var fixedTabs = document.querySelector('.js-tabs-affix'); - var fixedNav = document.querySelector('.navbar-gitlab'); + const fixedTabs = document.querySelector('.js-tabs-affix'); + const fixedDiffStats = document.querySelector('.js-diff-files-changed.is-stuck'); + const fixedNav = document.querySelector('.navbar-gitlab'); var adjustment = 0; if (fixedNav) adjustment -= fixedNav.offsetHeight; @@ -104,6 +105,11 @@ if (fixedTabs) { adjustment -= fixedTabs.offsetHeight; } + + if (fixedDiffStats) { + adjustment -= fixedDiffStats.offsetHeight; + } + window.scrollBy(0, adjustment); } }; diff --git a/app/assets/javascripts/lib/utils/pretty_time.js b/app/assets/javascripts/lib/utils/pretty_time.js index ae397212e55..716aefbfcb7 100644 --- a/app/assets/javascripts/lib/utils/pretty_time.js +++ b/app/assets/javascripts/lib/utils/pretty_time.js @@ -1,3 +1,5 @@ +import _ from 'underscore'; + (() => { /* * TODO: Make these methods more configurable (e.g. parseSeconds timePeriodContstraints, diff --git a/app/assets/javascripts/lib/utils/sticky.js b/app/assets/javascripts/lib/utils/sticky.js new file mode 100644 index 00000000000..43a808b6ab3 --- /dev/null +++ b/app/assets/javascripts/lib/utils/sticky.js @@ -0,0 +1,23 @@ +export const isSticky = (el, scrollY, stickyTop) => { + const top = el.offsetTop - scrollY; + + if (top === stickyTop) { + el.classList.add('is-stuck'); + } else { + el.classList.remove('is-stuck'); + } +}; + +export default (el) => { + if (!el) return; + + const computedStyle = window.getComputedStyle(el); + + if (!/sticky/.test(computedStyle.position)) return; + + const stickyTop = parseInt(computedStyle.top, 10); + + document.addEventListener('scroll', () => isSticky(el, window.scrollY, stickyTop), { + passive: true, + }); +}; diff --git a/app/assets/javascripts/main.js b/app/assets/javascripts/main.js index cd45091c211..42092a34c2f 100644 --- a/app/assets/javascripts/main.js +++ b/app/assets/javascripts/main.js @@ -16,9 +16,6 @@ import 'mousetrap'; import 'mousetrap/plugins/pause/mousetrap-pause'; import 'vendor/fuzzaldrin-plus'; -// extensions -import './extensions/array'; - // expose common libraries as globals (TODO: remove these) window.jQuery = jQuery; window.$ = jQuery; @@ -36,9 +33,6 @@ import './shortcuts_find_file'; import './shortcuts_issuable'; import './shortcuts_network'; -// behaviors -import './behaviors/'; - // templates import './templates/issuable_template_selector'; import './templates/issuable_template_selectors'; @@ -56,6 +50,9 @@ import './lib/utils/pretty_time'; import './lib/utils/text_utility'; import './lib/utils/url_utility'; +// behaviors +import './behaviors/'; + // u2f import './u2f/authenticate'; import './u2f/error'; @@ -86,7 +83,6 @@ import './copy_as_gfm'; import './copy_to_clipboard'; import './create_label'; import './diff'; -import './dispatcher'; import './dropzone_input'; import './due_date_select'; import './files_comment_button'; @@ -150,9 +146,13 @@ import './subscription'; import './subscription_select'; import './syntax_highlight'; +import './dispatcher'; + // eslint-disable-next-line global-require, import/no-commonjs if (process.env.NODE_ENV !== 'production') require('./test_utils/'); +Dropzone.autoDiscover = false; + document.addEventListener('beforeunload', function () { // Unbind scroll events $(document).off('scroll'); diff --git a/app/assets/javascripts/merge_request_tabs.js b/app/assets/javascripts/merge_request_tabs.js index 7840f05a8ae..4ffd71d9de5 100644 --- a/app/assets/javascripts/merge_request_tabs.js +++ b/app/assets/javascripts/merge_request_tabs.js @@ -7,6 +7,7 @@ import Cookies from 'js-cookie'; import './breakpoints'; import './flash'; import BlobForkSuggestion from './blob/blob_fork_suggestion'; +import stickyMonitor from './lib/utils/sticky'; /* eslint-disable max-len */ // MergeRequestTabs @@ -266,6 +267,10 @@ import BlobForkSuggestion from './blob/blob_fork_suggestion'; const $container = $('#diffs'); $container.html(data.html); + this.initChangesDropdown(); + + stickyMonitor(document.querySelector('.js-diff-files-changed')); + if (typeof gl.diffNotesCompileComponents !== 'undefined') { gl.diffNotesCompileComponents(); } @@ -314,6 +319,13 @@ import BlobForkSuggestion from './blob/blob_fork_suggestion'; }); } + initChangesDropdown() { + $('.js-diff-stats-dropdown').glDropdown({ + filterable: true, + remoteFilter: false, + }); + } + // Show or hide the loading spinner // // status - Boolean, true to show, false to hide diff --git a/app/assets/javascripts/milestone_select.js b/app/assets/javascripts/milestone_select.js index 6756ab0b3aa..04579058688 100644 --- a/app/assets/javascripts/milestone_select.js +++ b/app/assets/javascripts/milestone_select.js @@ -1,6 +1,7 @@ /* eslint-disable func-names, space-before-function-paren, wrap-iife, no-var, no-underscore-dangle, prefer-arrow-callback, max-len, one-var, one-var-declaration-per-line, no-unused-vars, object-shorthand, comma-dangle, no-else-return, no-self-compare, consistent-return, no-param-reassign, no-shadow */ /* global Issuable */ /* global ListMilestone */ +import _ from 'underscore'; (function() { this.MilestoneSelect = (function() { diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index dfa07a2def4..b38a6abc8d1 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -11,6 +11,7 @@ newline-per-chained-call, no-useless-escape, class-methods-use-this */ /* global mrRefreshWidgetUrl */ import $ from 'jquery'; +import _ from 'underscore'; import Cookies from 'js-cookie'; import autosize from 'vendor/autosize'; import Dropzone from 'dropzone'; diff --git a/app/assets/javascripts/pipeline_schedules/components/interval_pattern_input.vue b/app/assets/javascripts/pipeline_schedules/components/interval_pattern_input.vue index ce46b3fa3fa..b5d85299cf8 100644 --- a/app/assets/javascripts/pipeline_schedules/components/interval_pattern_input.vue +++ b/app/assets/javascripts/pipeline_schedules/components/interval_pattern_input.vue @@ -1,4 +1,6 @@ <script> + import _ from 'underscore'; + export default { props: { initialCronInterval: { diff --git a/app/assets/javascripts/pipelines/components/graph/graph_component.vue b/app/assets/javascripts/pipelines/components/graph/graph_component.vue index 77cbaeb43ef..66bc1d1979c 100644 --- a/app/assets/javascripts/pipelines/components/graph/graph_component.vue +++ b/app/assets/javascripts/pipelines/components/graph/graph_component.vue @@ -1,7 +1,7 @@ <script> + import loadingIcon from '~/vue_shared/components/loading_icon.vue'; + import '~/flash'; import stageColumnComponent from './stage_column_component.vue'; - import loadingIcon from '../../../vue_shared/components/loading_icon.vue'; - import '../../../flash'; export default { props: { diff --git a/app/assets/javascripts/profile/gl_crop.js b/app/assets/javascripts/profile/gl_crop.js index f32b2413725..291ae24aa68 100644 --- a/app/assets/javascripts/profile/gl_crop.js +++ b/app/assets/javascripts/profile/gl_crop.js @@ -1,6 +1,7 @@ /* eslint-disable no-useless-escape, max-len, quotes, no-var, no-underscore-dangle, func-names, space-before-function-paren, no-unused-vars, no-return-assign, object-shorthand, one-var, one-var-declaration-per-line, comma-dangle, consistent-return, class-methods-use-this, new-parens */ import 'cropper'; +import _ from 'underscore'; ((global) => { // Matches everything but the file name diff --git a/app/assets/javascripts/project_edit.js b/app/assets/javascripts/project_edit.js index d7d284b6c86..7572fec15e0 100644 --- a/app/assets/javascripts/project_edit.js +++ b/app/assets/javascripts/project_edit.js @@ -1,6 +1,6 @@ export default function setupProjectEdit() { const $transferForm = $('.js-project-transfer-form'); - const $selectNamespace = $transferForm.find('.select2'); + const $selectNamespace = $transferForm.find('select.select2'); $selectNamespace.on('change', () => { $transferForm.find(':submit').prop('disabled', !$selectNamespace.val()); diff --git a/app/assets/javascripts/protected_branches/protected_branch_dropdown.js b/app/assets/javascripts/protected_branches/protected_branch_dropdown.js index cc0b2ebe071..678882a8d2c 100644 --- a/app/assets/javascripts/protected_branches/protected_branch_dropdown.js +++ b/app/assets/javascripts/protected_branches/protected_branch_dropdown.js @@ -1,3 +1,5 @@ +import _ from 'underscore'; + export default class ProtectedBranchDropdown { /** * @param {Object} options containing diff --git a/app/assets/javascripts/protected_tags/protected_tag_dropdown.js b/app/assets/javascripts/protected_tags/protected_tag_dropdown.js index 9d045886262..a0224213aa0 100644 --- a/app/assets/javascripts/protected_tags/protected_tag_dropdown.js +++ b/app/assets/javascripts/protected_tags/protected_tag_dropdown.js @@ -1,3 +1,5 @@ +import _ from 'underscore'; + export default class ProtectedTagDropdown { /** * @param {Object} options containing diff --git a/app/assets/javascripts/right_sidebar.js b/app/assets/javascripts/right_sidebar.js index d8f1fe10b26..fa958d75fa4 100644 --- a/app/assets/javascripts/right_sidebar.js +++ b/app/assets/javascripts/right_sidebar.js @@ -1,5 +1,6 @@ /* eslint-disable func-names, space-before-function-paren, no-var, prefer-rest-params, wrap-iife, no-unused-vars, consistent-return, one-var, one-var-declaration-per-line, quotes, prefer-template, object-shorthand, comma-dangle, no-else-return, no-param-reassign, max-len */ +import _ from 'underscore'; import Cookies from 'js-cookie'; import SidebarHeightManager from './sidebar_height_manager'; diff --git a/app/assets/javascripts/shortcuts_issuable.js b/app/assets/javascripts/shortcuts_issuable.js index 51448252c0f..0be141eb5f9 100644 --- a/app/assets/javascripts/shortcuts_issuable.js +++ b/app/assets/javascripts/shortcuts_issuable.js @@ -3,6 +3,7 @@ /* global ShortcutsNavigation */ /* global sidebar */ +import _ from 'underscore'; import 'mousetrap'; import './shortcuts_navigation'; @@ -58,7 +59,7 @@ import './shortcuts_navigation'; }); // If replyField already has some content, add a newline before our quote separator = replyField.val().trim() !== "" && "\n\n" || ''; - replyField.val(function(_, current) { + replyField.val(function(a, current) { return current + separator + quote.join('') + "\n"; }); diff --git a/app/assets/javascripts/sidebar/components/time_tracking/sidebar_time_tracking.js b/app/assets/javascripts/sidebar/components/time_tracking/sidebar_time_tracking.js index 650e935b116..2d682215cf8 100644 --- a/app/assets/javascripts/sidebar/components/time_tracking/sidebar_time_tracking.js +++ b/app/assets/javascripts/sidebar/components/time_tracking/sidebar_time_tracking.js @@ -1,3 +1,5 @@ +import _ from 'underscore'; + import '~/smart_interval'; import timeTracker from './time_tracker'; diff --git a/app/assets/javascripts/sidebar_height_manager.js b/app/assets/javascripts/sidebar_height_manager.js index 022415f22b2..df19d7305f8 100644 --- a/app/assets/javascripts/sidebar_height_manager.js +++ b/app/assets/javascripts/sidebar_height_manager.js @@ -1,3 +1,5 @@ +import _ from 'underscore'; + export default { init() { if (!this.initialized) { @@ -30,4 +32,3 @@ export default { } }, }; - diff --git a/app/assets/javascripts/todos.js b/app/assets/javascripts/todos.js index bba8b5abbb4..a606852c22c 100644 --- a/app/assets/javascripts/todos.js +++ b/app/assets/javascripts/todos.js @@ -52,6 +52,7 @@ export default class Todos { } updateRowStateClicked(e) { + e.stopPropagation(); e.preventDefault(); const target = e.target; @@ -92,6 +93,7 @@ export default class Todos { } updateAllStateClicked(e) { + e.stopPropagation(); e.preventDefault(); const target = e.currentTarget; @@ -142,6 +144,7 @@ export default class Todos { if (gl.utils.isMetaClick(e)) { const windowTarget = '_blank'; const selected = e.target; + e.stopPropagation(); e.preventDefault(); if (selected.tagName === 'IMG') { diff --git a/app/assets/javascripts/u2f/authenticate.js b/app/assets/javascripts/u2f/authenticate.js index cd5280948fd..8821b22477f 100644 --- a/app/assets/javascripts/u2f/authenticate.js +++ b/app/assets/javascripts/u2f/authenticate.js @@ -3,6 +3,8 @@ /* global U2FError */ /* global U2FUtil */ +import _ from 'underscore'; + // Authenticate U2F (universal 2nd factor) devices for users to authenticate with. // // State Flow #1: setup -> in_progress -> authenticated -> POST to server diff --git a/app/assets/javascripts/u2f/register.js b/app/assets/javascripts/u2f/register.js index 1234d17b8fd..3a2534d553b 100644 --- a/app/assets/javascripts/u2f/register.js +++ b/app/assets/javascripts/u2f/register.js @@ -3,6 +3,8 @@ /* global U2FError */ /* global U2FUtil */ +import _ from 'underscore'; + // Register U2F (universal 2nd factor) devices for users to authenticate with. // // State Flow #1: setup -> in_progress -> registered -> POST to server diff --git a/app/assets/javascripts/username_validator.js b/app/assets/javascripts/username_validator.js index a348d69153c..bb34d5d2008 100644 --- a/app/assets/javascripts/username_validator.js +++ b/app/assets/javascripts/username_validator.js @@ -1,5 +1,7 @@ /* eslint-disable comma-dangle, consistent-return, class-methods-use-this, arrow-parens, no-param-reassign, max-len */ +import _ from 'underscore'; + const debounceTimeoutDuration = 1000; const invalidInputClass = 'gl-field-error-outline'; const successInputClass = 'gl-field-success-outline'; diff --git a/app/assets/javascripts/users/activity_calendar.js b/app/assets/javascripts/users/activity_calendar.js index f091e319f44..3dac31c2121 100644 --- a/app/assets/javascripts/users/activity_calendar.js +++ b/app/assets/javascripts/users/activity_calendar.js @@ -1,3 +1,4 @@ +import _ from 'underscore'; import d3 from 'd3'; const LOADING_HTML = ` diff --git a/app/assets/javascripts/users_select.js b/app/assets/javascripts/users_select.js index 5728afb4c59..16ebf5916dc 100644 --- a/app/assets/javascripts/users_select.js +++ b/app/assets/javascripts/users_select.js @@ -1,6 +1,7 @@ /* eslint-disable func-names, space-before-function-paren, one-var, no-var, prefer-rest-params, wrap-iife, quotes, max-len, one-var-declaration-per-line, vars-on-top, prefer-arrow-callback, consistent-return, comma-dangle, object-shorthand, no-shadow, no-unused-vars, no-else-return, no-self-compare, prefer-template, no-unused-expressions, no-lonely-if, yoda, prefer-spread, no-void, camelcase, no-param-reassign */ /* global Issuable */ /* global emitSidebarEvent */ +import _ from 'underscore'; // TODO: remove eventHub hack after code splitting refactor window.emitSidebarEvent = window.emitSidebarEvent || $.noop; diff --git a/app/assets/javascripts/vue_merge_request_widget/dependencies.js b/app/assets/javascripts/vue_merge_request_widget/dependencies.js index fe5e1bbb55c..546a3f625c7 100644 --- a/app/assets/javascripts/vue_merge_request_widget/dependencies.js +++ b/app/assets/javascripts/vue_merge_request_widget/dependencies.js @@ -1,7 +1,7 @@ /** * This file is the centerpiece of an attempt to reduce potential conflicts * between the CE and EE versions of the MR widget. EE additions to the MR widget should - * be contained in the ./vue_merge_request_widget/ee directory, and should **extend** + * be contained in the ee/vue_merge_request_widget directory, and should **extend** * rather than mutate CE MR Widget code. * * This file should be the only source of conflicts between EE and CE. EE-only components should diff --git a/app/assets/stylesheets/framework/dropdowns.scss b/app/assets/stylesheets/framework/dropdowns.scss index 572203bce34..bd4bd541c3a 100644 --- a/app/assets/stylesheets/framework/dropdowns.scss +++ b/app/assets/stylesheets/framework/dropdowns.scss @@ -574,6 +574,7 @@ .dropdown-input-field, .default-dropdown-input { + display: block; width: 100%; min-height: 30px; padding: 0 7px; @@ -725,7 +726,8 @@ // TODO: change global style and remove mixin @mixin new-style-dropdown { - .dropdown-menu { + .dropdown-menu, + .dropdown-menu-nav { li { padding: 0 1px; @@ -766,4 +768,8 @@ } } } + + .dropdown-menu-align-right { + margin-top: 2px; + } } diff --git a/app/assets/stylesheets/framework/header.scss b/app/assets/stylesheets/framework/header.scss index 1c4238bc564..555e444a062 100644 --- a/app/assets/stylesheets/framework/header.scss +++ b/app/assets/stylesheets/framework/header.scss @@ -4,6 +4,8 @@ */ header { + @include new-style-dropdown; + transition: padding $sidebar-transition-duration; &.navbar-empty { @@ -313,25 +315,6 @@ header { .impersonation i { color: $red-500; } - - // TODO: fallback to global style - .dropdown-menu, - .dropdown-menu-nav { - li { - padding: 0 1px; - - a { - border-radius: 0; - padding: 8px 16px; - - &:hover, - &:active, - &:focus { - background-color: $gray-darker; - } - } - } - } } .with-performance-bar header.navbar-gitlab { diff --git a/app/assets/stylesheets/pages/cycle_analytics.scss b/app/assets/stylesheets/pages/cycle_analytics.scss index 87b50c7687e..6753eb08285 100644 --- a/app/assets/stylesheets/pages/cycle_analytics.scss +++ b/app/assets/stylesheets/pages/cycle_analytics.scss @@ -1,4 +1,6 @@ #cycle-analytics { + @include new-style-dropdown; + max-width: 1000px; margin: 24px auto 0; position: relative; @@ -110,10 +112,6 @@ .js-ca-dropdown { top: $gl-padding-top; - - .dropdown-menu-align-right { - margin-top: 2px; - } } .content-list { @@ -446,24 +444,6 @@ margin-bottom: 20px; } } - - // TODO: fallback to global style - .dropdown-menu { - li { - padding: 0 1px; - - a { - border-radius: 0; - padding: 8px 16px; - - &:hover, - &:active, - &:focus { - background-color: $gray-darker; - } - } - } - } } .cycle-analytics-overview { diff --git a/app/assets/stylesheets/pages/diff.scss b/app/assets/stylesheets/pages/diff.scss index 398fd4444ea..da77346d8b2 100644 --- a/app/assets/stylesheets/pages/diff.scss +++ b/app/assets/stylesheets/pages/diff.scss @@ -395,12 +395,11 @@ background-color: transparent; border: 0; color: $gl-link-color; - transition: color 0.1s linear; + font-weight: 600; &:hover, &:focus { outline: none; - text-decoration: underline; color: $gl-link-hover-color; } } @@ -559,3 +558,68 @@ outline: 0; } } + +.diff-files-changed { + .commit-stat-summary { + @include new-style-dropdown; + z-index: -1; + + @media (min-width: $screen-sm-min) { + margin-left: -$gl-padding; + padding-left: $gl-padding; + background-color: $white-light; + } + } + + @media (min-width: $screen-sm-min) { + position: -webkit-sticky; + position: sticky; + top: 84px; + background-color: $white-light; + z-index: 190; + + + .files, + + .alert { + margin-top: 1px; + } + + &:not(.is-stuck) .diff-stats-additions-deletions-collapsed { + display: none; + } + + &.is-stuck { + padding-top: 0; + padding-bottom: 0; + border-bottom: 1px solid $white-dark; + transform: translateY(16px); + + .diff-stats-additions-deletions-expanded, + .inline-parallel-buttons { + display: none; + } + + + .files, + + .alert { + margin-top: 30px; + } + } + } +} + +.diff-file-changes { + width: 450px; + z-index: 150; + + @media (min-width: $screen-sm-min) { + left: $gl-padding; + } + + a { + padding-top: 8px; + padding-bottom: 8px; + } +} + +.diff-file-changes-path { + @include str-truncated(78%); +} diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss index 4693b2434c7..a4e19094508 100644 --- a/app/assets/stylesheets/pages/merge_requests.scss +++ b/app/assets/stylesheets/pages/merge_requests.scss @@ -691,8 +691,10 @@ } .mr-version-controls { + position: relative; background: $gray-light; color: $gl-text-color; + z-index: 199; .mr-version-menus-container { display: -webkit-flex; diff --git a/app/assets/stylesheets/pages/tree.scss b/app/assets/stylesheets/pages/tree.scss index e0f46172769..44ab07a4367 100644 --- a/app/assets/stylesheets/pages/tree.scss +++ b/app/assets/stylesheets/pages/tree.scss @@ -1,4 +1,5 @@ .tree-holder { + @include new-style-dropdown; .nav-block { margin: 10px 0; @@ -202,28 +203,6 @@ } } } - - // TODO: fallback to global style - .dropdown-menu:not(.dropdown-menu-selectable) { - li { - padding: 0 1px; - - &.dropdown-header { - padding: 8px 16px; - } - - a { - border-radius: 0; - padding: 8px 16px; - - &:hover, - &:active, - &:focus { - background-color: $gray-darker; - } - } - } - } } .blob-commit-info { diff --git a/app/controllers/dashboard/todos_controller.rb b/app/controllers/dashboard/todos_controller.rb index 59e5b5e4775..a8b2b93b458 100644 --- a/app/controllers/dashboard/todos_controller.rb +++ b/app/controllers/dashboard/todos_controller.rb @@ -13,7 +13,7 @@ class Dashboard::TodosController < Dashboard::ApplicationController end def destroy - TodoService.new.mark_todos_as_done_by_ids([params[:id]], current_user) + TodoService.new.mark_todos_as_done_by_ids(params[:id], current_user) respond_to do |format| format.html do @@ -37,7 +37,7 @@ class Dashboard::TodosController < Dashboard::ApplicationController end def restore - TodoService.new.mark_todos_as_pending_by_ids([params[:id]], current_user) + TodoService.new.mark_todos_as_pending_by_ids(params[:id], current_user) render json: todos_counts end diff --git a/app/finders/todos_finder.rb b/app/finders/todos_finder.rb index 3fe37c75381..b276116f0c6 100644 --- a/app/finders/todos_finder.rb +++ b/app/finders/todos_finder.rb @@ -95,9 +95,18 @@ class TodosFinder @project end + def project_ids(items) + ids = items.except(:order).select(:project_id) + if Gitlab::Database.mysql? + # To make UPDATE work on MySQL, wrap it in a SELECT with an alias + ids = Todo.except(:order).select('*').from("(#{ids.to_sql}) AS t") + end + + ids + end + def projects(items) - item_project_ids = items.reorder(nil).select(:project_id) - ProjectsFinder.new(current_user: current_user, project_ids_relation: item_project_ids).execute + ProjectsFinder.new(current_user: current_user, project_ids_relation: project_ids(items)).execute end def type? diff --git a/app/helpers/defer_script_tag_helper.rb b/app/helpers/defer_script_tag_helper.rb new file mode 100644 index 00000000000..e1567556e5e --- /dev/null +++ b/app/helpers/defer_script_tag_helper.rb @@ -0,0 +1,6 @@ +module DeferScriptTagHelper + # Override the default ActionView `javascript_include_tag` helper to support page specific deferred loading + def javascript_include_tag(*sources) + super(*sources, defer: true) + end +end diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index 91ddd73fac1..087f7f88fb5 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -148,6 +148,24 @@ module DiffHelper options end + def diff_file_changed_icon(diff_file) + if diff_file.deleted_file? || diff_file.renamed_file? + "minus" + elsif diff_file.new_file? + "plus" + else + "adjust" + end + end + + def diff_file_changed_icon_color(diff_file) + if diff_file.deleted_file? + "cred" + elsif diff_file.new_file? + "cgreen" + end + end + private def diff_btn(title, name, selected) diff --git a/app/models/concerns/referable.rb b/app/models/concerns/referable.rb index da803c7f481..10f4be72016 100644 --- a/app/models/concerns/referable.rb +++ b/app/models/concerns/referable.rb @@ -25,6 +25,18 @@ module Referable to_reference(from_project) end + def referable_inspect + if respond_to?(:id) + "#<#{self.class.name} id:#{id} #{to_reference(full: true)}>" + else + "#<#{self.class.name} #{to_reference(full: true)}>" + end + end + + def inspect + referable_inspect + end + module ClassMethods # The character that prefixes the actual reference identifier # diff --git a/app/models/key.rb b/app/models/key.rb index cb8f10f6d55..49bc26122fa 100644 --- a/app/models/key.rb +++ b/app/models/key.rb @@ -16,8 +16,6 @@ class Key < ActiveRecord::Base presence: true, length: { maximum: 5000 }, format: { with: /\A(ssh|ecdsa)-.*\Z/ } - validates :key, - format: { without: /\n|\r/, message: 'should be a single line' } validates :fingerprint, uniqueness: true, presence: { message: 'cannot be generated' } @@ -31,6 +29,7 @@ class Key < ActiveRecord::Base after_destroy :post_destroy_hook def key=(value) + value&.delete!("\n\r") value.strip! unless value.blank? write_attribute(:key, value) end diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index ec87aee9310..d9d746ccf41 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -85,11 +85,7 @@ class MergeRequestDiff < ActiveRecord::Base def raw_diffs(options = {}) if options[:ignore_whitespace_change] - @diffs_no_whitespace ||= - Gitlab::Git::Compare.new( - repository.raw_repository, - safe_start_commit_sha, - head_commit_sha).diffs(options) + @diffs_no_whitespace ||= compare.diffs(options) else @raw_diffs ||= {} @raw_diffs[options] ||= load_diffs(options) diff --git a/app/models/notification_recipient.rb b/app/models/notification_recipient.rb new file mode 100644 index 00000000000..418b42d8f1d --- /dev/null +++ b/app/models/notification_recipient.rb @@ -0,0 +1,125 @@ +class NotificationRecipient + attr_reader :user, :type + def initialize( + user, type, + custom_action: nil, + target: nil, + acting_user: nil, + project: nil + ) + @custom_action = custom_action + @acting_user = acting_user + @target = target + @project = project || @target&.project + @user = user + @type = type + end + + def notification_setting + @notification_setting ||= find_notification_setting + end + + def raw_notification_level + notification_setting&.level&.to_sym + end + + def notification_level + # custom is treated the same as watch if it's enabled - otherwise it's + # set to :custom, meaning to send exactly when our type is :participating + # or :mention. + @notification_level ||= + case raw_notification_level + when :custom + if @custom_action && notification_setting&.event_enabled?(@custom_action) + :watch + else + :custom + end + else + raw_notification_level + end + end + + def notifiable? + return false unless has_access? + return false if own_activity? + + return true if @type == :subscription + + return false if notification_level.nil? || notification_level == :disabled + + return %i[participating mention].include?(@type) if notification_level == :custom + + return false if %i[watch participating].include?(notification_level) && excluded_watcher_action? + + return false unless NotificationSetting.levels[notification_level] <= NotificationSetting.levels[@type] + + return false if unsubscribed? + + true + end + + def unsubscribed? + return false unless @target + return false unless @target.respond_to?(:subscriptions) + + subscription = @target.subscriptions.find_by_user_id(@user.id) + subscription && !subscription.subscribed + end + + def own_activity? + return false unless @acting_user + return false if @acting_user.notified_of_own_activity? + + user == @acting_user + end + + def has_access? + DeclarativePolicy.subject_scope do + return false unless user.can?(:receive_notifications) + return false if @project && !user.can?(:read_project, @project) + + return true unless read_ability + return true unless DeclarativePolicy.has_policy?(@target) + + user.can?(read_ability, @target) + end + end + + def excluded_watcher_action? + return false unless @custom_action + return false if raw_notification_level == :custom + + NotificationSetting::EXCLUDED_WATCHER_EVENTS.include?(@custom_action) + end + + private + + def read_ability + return @read_ability if instance_variable_defined?(:@read_ability) + + @read_ability = + case @target + when Issuable + :"read_#{@target.to_ability_name}" + when Ci::Pipeline + :read_build # We have build trace in pipeline emails + when ActiveRecord::Base + :"read_#{@target.class.model_name.name.underscore}" + else + nil + end + end + + def find_notification_setting + project_setting = @project && user.notification_settings_for(@project) + + return project_setting unless project_setting.nil? || project_setting.global? + + group_setting = @project&.group && user.notification_settings_for(@project.group) + + return group_setting unless group_setting.nil? || group_setting.global? + + user.global_notification_setting + end +end diff --git a/app/models/project_services/jira_service.rb b/app/models/project_services/jira_service.rb index c2414885368..9ee3a533c1e 100644 --- a/app/models/project_services/jira_service.rb +++ b/app/models/project_services/jira_service.rb @@ -104,7 +104,7 @@ class JiraService < IssueTrackerService def close_issue(entity, external_issue) issue = jira_request { client.Issue.find(external_issue.iid) } - return if issue.nil? || issue.resolution.present? || !jira_issue_transition_id.present? + return if issue.nil? || has_resolution?(issue) || !jira_issue_transition_id.present? commit_id = if entity.is_a?(Commit) entity.id @@ -118,7 +118,7 @@ class JiraService < IssueTrackerService # may or may not be allowed. Refresh the issue after transition and check # if it is closed, so we don't have one comment for every commit. issue = jira_request { client.Issue.find(issue.key) } if transition_issue(issue) - add_issue_solved_comment(issue, commit_id, commit_url) if issue.resolution + add_issue_solved_comment(issue, commit_id, commit_url) if has_resolution?(issue) end def create_cross_reference_note(mentioned, noteable, author) @@ -216,6 +216,10 @@ class JiraService < IssueTrackerService end end + def has_resolution?(issue) + issue.respond_to?(:resolution) && issue.resolution.present? + end + def comment_exists?(issue, message) comments = jira_request { issue.comments } diff --git a/app/models/repository.rb b/app/models/repository.rb index 4e9fe759fdc..2dd48290e58 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -613,17 +613,26 @@ class Repository end def last_commit_for_path(sha, path) - sha = last_commit_id_for_path(sha, path) - commit(sha) + raw_repository.gitaly_migrate(:last_commit_for_path) do |is_enabled| + if is_enabled + last_commit_for_path_by_gitaly(sha, path) + else + last_commit_for_path_by_rugged(sha, path) + end + end end - # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/383 def last_commit_id_for_path(sha, path) key = path.blank? ? "last_commit_id_for_path:#{sha}" : "last_commit_id_for_path:#{sha}:#{Digest::SHA1.hexdigest(path)}" cache.fetch(key) do - args = %W(#{Gitlab.config.git.bin_path} rev-list --max-count=1 #{sha} -- #{path}) - Gitlab::Popen.popen(args, path_to_repo).first.strip + raw_repository.gitaly_migrate(:last_commit_for_path) do |is_enabled| + if is_enabled + last_commit_for_path_by_gitaly(sha, path).id + else + last_commit_id_for_path_by_shelling_out(sha, path) + end + end end end @@ -1138,6 +1147,21 @@ class Repository Rugged::Commit.create(rugged, params) end + def last_commit_for_path_by_gitaly(sha, path) + c = raw_repository.gitaly_commit_client.last_commit_for_path(sha, path) + commit(c) + end + + def last_commit_for_path_by_rugged(sha, path) + sha = last_commit_id_for_path_by_shelling_out(sha, path) + commit(sha) + end + + def last_commit_id_for_path_by_shelling_out(sha, path) + args = %W(#{Gitlab.config.git.bin_path} rev-list --max-count=1 #{sha} -- #{path}) + Gitlab::Popen.popen(args, path_to_repo).first.strip + end + def repository_storage_path @project.repository_storage_path end diff --git a/app/models/user.rb b/app/models/user.rb index 6e66c587a1f..267eebb42ff 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -47,6 +47,11 @@ class User < ActiveRecord::Base devise :lockable, :recoverable, :rememberable, :trackable, :validatable, :omniauthable, :confirmable, :registerable + # devise overrides #inspect, so we manually use the Referable one + def inspect + referable_inspect + end + # Override Devise::Models::Trackable#update_tracked_fields! # to limit database writes to at most once every hour def update_tracked_fields!(request) diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index ea497729115..760a15e3ed0 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -288,7 +288,7 @@ class IssuableBaseService < BaseService todo_service.mark_todo(issuable, current_user) when 'done' todo = TodosFinder.new(current_user).execute.find_by(target: issuable) - todo_service.mark_todos_as_done([todo], current_user) if todo + todo_service.mark_todos_as_done_by_ids(todo, current_user) if todo end end diff --git a/app/services/notification_recipient_service.rb b/app/services/notification_recipient_service.rb index 9ac561e4bd2..21c9c314a2a 100644 --- a/app/services/notification_recipient_service.rb +++ b/app/services/notification_recipient_service.rb @@ -1,331 +1,288 @@ # # Used by NotificationService to determine who should receive notification # -class NotificationRecipientService - attr_reader :project - - def initialize(project) - @project = project +module NotificationRecipientService + def self.notifiable_users(users, *args) + users.compact.map { |u| NotificationRecipient.new(u, *args) }.select(&:notifiable?).map(&:user) end - def build_recipients(target, current_user, action:, previous_assignee: nil, skip_current_user: true) - custom_action = build_custom_key(action, target) - - recipients = participants(target, current_user) - recipients = add_project_watchers(recipients) - recipients = add_custom_notifications(recipients, custom_action) - recipients = reject_mention_users(recipients) - - # Re-assign is considered as a mention of the new assignee so we add the - # new assignee to the list of recipients after we rejected users with - # the "on mention" notification level - case custom_action - when :reassign_merge_request - recipients << previous_assignee if previous_assignee - recipients << target.assignee - when :reassign_issue - previous_assignees = Array(previous_assignee) - recipients.concat(previous_assignees) - recipients.concat(target.assignees) - end - - recipients = reject_muted_users(recipients) - recipients = add_subscribed_users(recipients, target) - - if [:new_issue, :new_merge_request].include?(custom_action) - recipients = add_labels_subscribers(recipients, target) - end - - recipients = reject_unsubscribed_users(recipients, target) - recipients = reject_users_without_access(recipients, target) + def self.notifiable?(user, *args) + NotificationRecipient.new(user, *args).notifiable? + end - recipients.delete(current_user) if skip_current_user && !current_user.notified_of_own_activity? + def self.build_recipients(*a) + Builder::Default.new(*a).recipient_users + end - recipients.uniq + def self.build_new_note_recipients(*a) + Builder::NewNote.new(*a).recipient_users end - def build_pipeline_recipients(target, current_user, action:) - return [] unless current_user + module Builder + class Base + def initialize(*) + raise 'abstract' + end - custom_action = - case action.to_s - when 'failed' - :failed_pipeline - when 'success' - :success_pipeline + def build! + raise 'abstract' end - notification_setting = notification_setting_for_user_project(current_user, target.project) + def filter! + recipients.select!(&:notifiable?) + end - return [] if notification_setting.mention? || notification_setting.disabled? + def acting_user + current_user + end - return [] if notification_setting.custom? && !notification_setting.event_enabled?(custom_action) + def target + raise 'abstract' + end - return [] if (notification_setting.watch? || notification_setting.participating?) && NotificationSetting::EXCLUDED_WATCHER_EVENTS.include?(custom_action) + # rubocop:disable Rails/Delegate + def project + target.project + end - reject_users_without_access([current_user], target) - end + def recipients + @recipients ||= [] + end - def build_relabeled_recipients(target, current_user, labels:) - recipients = add_labels_subscribers([], target, labels: labels) - recipients = reject_unsubscribed_users(recipients, target) - recipients = reject_users_without_access(recipients, target) - recipients.delete(current_user) unless current_user.notified_of_own_activity? - recipients.uniq - end + def <<(pair) + users, type = pair - def build_new_note_recipients(note) - target = note.noteable + if users.is_a?(ActiveRecord::Relation) + users = users.includes(:notification_settings) + end - ability, subject = if note.for_personal_snippet? - [:read_personal_snippet, note.noteable] - else - [:read_project, note.project] - end + users = Array(users) + users.compact! + recipients.concat(users.map { |u| make_recipient(u, type) }) + end - mentioned_users = note.mentioned_users.select { |user| user.can?(ability, subject) } + def user_scope + User.includes(:notification_settings) + end - # Add all users participating in the thread (author, assignee, comment authors) - recipients = participants(target, note.author) || mentioned_users + def make_recipient(user, type) + NotificationRecipient.new( + user, type, + project: project, + custom_action: custom_action, + target: target, + acting_user: acting_user + ) + end - unless note.for_personal_snippet? - # Merge project watchers - recipients = add_project_watchers(recipients) + def recipient_users + @recipient_users ||= + begin + build! + filter! + users = recipients.map(&:user) + users.uniq! + users.freeze + end + end - # Merge project with custom notification - recipients = add_custom_notifications(recipients, :new_note) - end + def custom_action + nil + end - # Reject users with Mention notification level, except those mentioned in _this_ note. - recipients = reject_mention_users(recipients - mentioned_users) - recipients = recipients + mentioned_users + protected - recipients = reject_muted_users(recipients) + def add_participants(user) + return unless target.respond_to?(:participants) - recipients = add_subscribed_users(recipients, note.noteable) - recipients = reject_unsubscribed_users(recipients, note.noteable) - recipients = reject_users_without_access(recipients, note.noteable) + self << [target.participants(user), :watch] + end - recipients.delete(note.author) unless note.author.notified_of_own_activity? - recipients.uniq - end + # Get project/group users with CUSTOM notification level + def add_custom_notifications + user_ids = [] - # Remove users with disabled notifications from array - # Also remove duplications and nil recipients - def reject_muted_users(users) - reject_users(users, :disabled) - end + # Users with a notification setting on group or project + user_ids += user_ids_notifiable_on(project, :custom) + user_ids += user_ids_notifiable_on(project.group, :custom) - protected + # Users with global level custom + user_ids_with_project_level_global = user_ids_notifiable_on(project, :global) + user_ids_with_group_level_global = user_ids_notifiable_on(project.group, :global) - # Ensure that if we modify this array, we aren't modifying the memoised - # participants on the target. - def participants(target, user) - return unless target.respond_to?(:participants) + global_users_ids = user_ids_with_project_level_global.concat(user_ids_with_group_level_global) + user_ids += user_ids_with_global_level_custom(global_users_ids, custom_action) - target.participants(user).dup - end + self << [user_scope.where(id: user_ids), :watch] + end - # Get project/group users with CUSTOM notification level - def add_custom_notifications(recipients, action) - user_ids = [] + def add_project_watchers + self << [project_watchers, :watch] + end - # Users with a notification setting on group or project - user_ids += user_ids_notifiable_on(project, :custom, action) - user_ids += user_ids_notifiable_on(project.group, :custom, action) + # Get project users with WATCH notification level + def project_watchers + project_members_ids = user_ids_notifiable_on(project) - # Users with global level custom - user_ids_with_project_level_global = user_ids_notifiable_on(project, :global) - user_ids_with_group_level_global = user_ids_notifiable_on(project.group, :global) + user_ids_with_project_global = user_ids_notifiable_on(project, :global) + user_ids_with_group_global = user_ids_notifiable_on(project.group, :global) - global_users_ids = user_ids_with_project_level_global.concat(user_ids_with_group_level_global) - user_ids += user_ids_with_global_level_custom(global_users_ids, action) + user_ids = user_ids_with_global_level_watch((user_ids_with_project_global + user_ids_with_group_global).uniq) - recipients.concat(User.find(user_ids)) - end + user_ids_with_project_setting = select_project_members_ids(user_ids_with_project_global, user_ids) + user_ids_with_group_setting = select_group_members_ids(project.group, project_members_ids, user_ids_with_group_global, user_ids) - def add_project_watchers(recipients) - recipients.concat(project_watchers).compact - end + user_scope.where(id: user_ids_with_project_setting.concat(user_ids_with_group_setting).uniq) + end - # Get project users with WATCH notification level - def project_watchers - project_members_ids = user_ids_notifiable_on(project) + def add_subscribed_users + return unless target.respond_to? :subscribers - user_ids_with_project_global = user_ids_notifiable_on(project, :global) - user_ids_with_group_global = user_ids_notifiable_on(project.group, :global) + self << [target.subscribers(project), :subscription] + end - user_ids = user_ids_with_global_level_watch((user_ids_with_project_global + user_ids_with_group_global).uniq) + def user_ids_notifiable_on(resource, notification_level = nil) + return [] unless resource - user_ids_with_project_setting = select_project_members_ids(project, user_ids_with_project_global, user_ids) - user_ids_with_group_setting = select_group_members_ids(project.group, project_members_ids, user_ids_with_group_global, user_ids) + scope = resource.notification_settings - User.where(id: user_ids_with_project_setting.concat(user_ids_with_group_setting).uniq).to_a - end + if notification_level + scope = scope.where(level: NotificationSetting.levels[notification_level]) + end - # Remove users with notification level 'Mentioned' - def reject_mention_users(users) - reject_users(users, :mention) - end + scope.pluck(:user_id) + end - def add_subscribed_users(recipients, target) - return recipients unless target.respond_to? :subscribers + # Build a list of user_ids based on project notification settings + def select_project_members_ids(global_setting, user_ids_global_level_watch) + user_ids = user_ids_notifiable_on(project, :watch) - recipients + target.subscribers(project) - end + # If project setting is global, add to watch list if global setting is watch + user_ids + (global_setting & user_ids_global_level_watch) + end - def user_ids_notifiable_on(resource, notification_level = nil, action = nil) - return [] unless resource + # Build a list of user_ids based on group notification settings + def select_group_members_ids(group, project_members, global_setting, user_ids_global_level_watch) + uids = user_ids_notifiable_on(group, :watch) - if notification_level - settings = resource.notification_settings.where(level: NotificationSetting.levels[notification_level]) - settings = settings.select { |setting| setting.event_enabled?(action) } if action.present? - settings.map(&:user_id) - else - resource.notification_settings.pluck(:user_id) - end - end + # Group setting is global, add to user_ids list if global setting is watch + uids + (global_setting & user_ids_global_level_watch) - project_members + end - # Build a list of user_ids based on project notification settings - def select_project_members_ids(project, global_setting, user_ids_global_level_watch) - user_ids = user_ids_notifiable_on(project, :watch) + def user_ids_with_global_level_watch(ids) + settings_with_global_level_of(:watch, ids).pluck(:user_id) + end - # If project setting is global, add to watch list if global setting is watch - global_setting.each do |user_id| - if user_ids_global_level_watch.include?(user_id) - user_ids << user_id + def user_ids_with_global_level_custom(ids, action) + settings_with_global_level_of(:custom, ids).pluck(:user_id) end - end - user_ids - end + def settings_with_global_level_of(level, ids) + NotificationSetting.where( + user_id: ids, + source_type: nil, + level: NotificationSetting.levels[level] + ) + end - # Build a list of user_ids based on group notification settings - def select_group_members_ids(group, project_members, global_setting, user_ids_global_level_watch) - uids = user_ids_notifiable_on(group, :watch) + def add_labels_subscribers(labels: nil) + return unless target.respond_to? :labels - # Group setting is watch, add to user_ids list if user is not project member - user_ids = [] - uids.each do |user_id| - if project_members.exclude?(user_id) - user_ids << user_id + (labels || target.labels).each do |label| + self << [label.subscribers(project), :subscription] + end end end - # Group setting is global, add to user_ids list if global setting is watch - global_setting.each do |user_id| - if project_members.exclude?(user_id) && user_ids_global_level_watch.include?(user_id) - user_ids << user_id + class Default < Base + attr_reader :target + attr_reader :current_user + attr_reader :action + attr_reader :previous_assignee + attr_reader :skip_current_user + def initialize(target, current_user, action:, previous_assignee: nil, skip_current_user: true) + @target = target + @current_user = current_user + @action = action + @previous_assignee = previous_assignee + @skip_current_user = skip_current_user end - end - - user_ids - end - - def user_ids_with_global_level_watch(ids) - settings_with_global_level_of(:watch, ids).pluck(:user_id) - end - - def user_ids_with_global_level_custom(ids, action) - settings = settings_with_global_level_of(:custom, ids) - settings = settings.select { |setting| setting.event_enabled?(action) } - settings.map(&:user_id) - end - def settings_with_global_level_of(level, ids) - NotificationSetting.where( - user_id: ids, - source_type: nil, - level: NotificationSetting.levels[level] - ) - end + def build! + add_participants(current_user) + add_project_watchers + add_custom_notifications + + # Re-assign is considered as a mention of the new assignee + case custom_action + when :reassign_merge_request + self << [previous_assignee, :mention] + self << [target.assignee, :mention] + when :reassign_issue + previous_assignees = Array(previous_assignee) + self << [previous_assignees, :mention] + self << [target.assignees, :mention] + end + + add_subscribed_users + + if [:new_issue, :new_merge_request].include?(custom_action) + add_labels_subscribers + end + end - # Reject users which has certain notification level - # - # Example: - # reject_users(users, :watch, project) - # - def reject_users(users, level) - level = level.to_s + def acting_user + current_user if skip_current_user + end - unless NotificationSetting.levels.keys.include?(level) - raise 'Invalid notification level' + # Build event key to search on custom notification level + # Check NotificationSetting::EMAIL_EVENTS + def custom_action + @custom_action ||= "#{action}_#{target.class.model_name.name.underscore}".to_sym + end end - users = users.to_a.compact.uniq - users = users.select { |u| u.can?(:receive_notifications) } - - users.reject do |user| - global_notification_setting = user.global_notification_setting - - next global_notification_setting.level == level unless project - - setting = user.notification_settings_for(project) - - if project.group && (setting.nil? || setting.global?) - setting = user.notification_settings_for(project.group) + class NewNote < Base + attr_reader :note + def initialize(note) + @note = note end - # reject users who globally set mention notification and has no setting per project/group - next global_notification_setting.level == level unless setting - - # reject users who set mention notification in project - next true if setting.level == level - - # reject users who have mention level in project and disabled in global settings - setting.global? && global_notification_setting.level == level - end - end + def target + note.noteable + end - def reject_unsubscribed_users(recipients, target) - return recipients unless target.respond_to? :subscriptions + # NOTE: may be nil, in the case of a PersonalSnippet + # + # (this is okay because NotificationRecipient is written + # to handle nil projects) + def project + note.project + end - recipients.reject do |user| - subscription = target.subscriptions.find_by_user_id(user.id) - subscription && !subscription.subscribed - end - end + def build! + # Add all users participating in the thread (author, assignee, comment authors) + add_participants(note.author) + self << [note.mentioned_users, :mention] - def reject_users_without_access(recipients, target) - ability = case target - when Issuable - :"read_#{target.to_ability_name}" - when Ci::Pipeline - :read_build # We have build trace in pipeline emails - end + unless note.for_personal_snippet? + # Merge project watchers + add_project_watchers - return recipients unless ability + # Merge project with custom notification + add_custom_notifications + end - recipients.select do |user| - user.can?(ability, target) - end - end + add_subscribed_users + end - def add_labels_subscribers(recipients, target, labels: nil) - return recipients unless target.respond_to? :labels + def custom_action + :new_note + end - (labels || target.labels).each do |label| - recipients += label.subscribers(project) + def acting_user + note.author + end end - - recipients - end - - # Build event key to search on custom notification level - # Check NotificationSetting::EMAIL_EVENTS - def build_custom_key(action, object) - "#{action}_#{object.class.model_name.name.underscore}".to_sym - end - - def notification_setting_for_user_project(user, project) - project_setting = user.notification_settings_for(project) - - return project_setting unless project_setting.global? - - group_setting = user.notification_settings_for(project.group) - - return group_setting unless group_setting.global? - - user.global_notification_setting end end diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index b94921d2a08..df04b1a4fe3 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -42,7 +42,7 @@ class NotificationService # * users with custom level checked with "new issue" # def new_issue(issue, current_user) - new_resource_email(issue, issue.project, :new_issue_email) + new_resource_email(issue, :new_issue_email) end # When issue text is updated, we should send an email to: @@ -52,7 +52,6 @@ class NotificationService def new_mentions_in_issue(issue, new_mentioned_users, current_user) new_mentions_in_resource_email( issue, - issue.project, new_mentioned_users, current_user, :new_mention_in_issue_email @@ -67,7 +66,7 @@ class NotificationService # * users with custom level checked with "close issue" # def close_issue(issue, current_user) - close_resource_email(issue, issue.project, current_user, :closed_issue_email) + close_resource_email(issue, current_user, :closed_issue_email) end # When we reassign an issue we should send an email to: @@ -77,7 +76,7 @@ class NotificationService # * users with custom level checked with "reassign issue" # def reassigned_issue(issue, current_user, previous_assignees = []) - recipients = NotificationRecipientService.new(issue.project).build_recipients( + recipients = NotificationRecipientService.build_recipients( issue, current_user, action: "reassign", @@ -102,7 +101,7 @@ class NotificationService # * watchers of the issue's labels # def relabeled_issue(issue, added_labels, current_user) - relabeled_resource_email(issue, issue.project, added_labels, current_user, :relabeled_issue_email) + relabeled_resource_email(issue, added_labels, current_user, :relabeled_issue_email) end # When create a merge request we should send an email to: @@ -113,7 +112,7 @@ class NotificationService # * users with custom level checked with "new merge request" # def new_merge_request(merge_request, current_user) - new_resource_email(merge_request, merge_request.target_project, :new_merge_request_email) + new_resource_email(merge_request, :new_merge_request_email) end # When merge request text is updated, we should send an email to: @@ -123,7 +122,6 @@ class NotificationService def new_mentions_in_merge_request(merge_request, new_mentioned_users, current_user) new_mentions_in_resource_email( merge_request, - merge_request.target_project, new_mentioned_users, current_user, :new_mention_in_merge_request_email @@ -137,7 +135,7 @@ class NotificationService # * users with custom level checked with "reassign merge request" # def reassigned_merge_request(merge_request, current_user) - reassign_resource_email(merge_request, merge_request.target_project, current_user, :reassigned_merge_request_email) + reassign_resource_email(merge_request, current_user, :reassigned_merge_request_email) end # When we add labels to a merge request we should send an email to: @@ -145,21 +143,20 @@ class NotificationService # * watchers of the mr's labels # def relabeled_merge_request(merge_request, added_labels, current_user) - relabeled_resource_email(merge_request, merge_request.target_project, added_labels, current_user, :relabeled_merge_request_email) + relabeled_resource_email(merge_request, added_labels, current_user, :relabeled_merge_request_email) end def close_mr(merge_request, current_user) - close_resource_email(merge_request, merge_request.target_project, current_user, :closed_merge_request_email) + close_resource_email(merge_request, current_user, :closed_merge_request_email) end def reopen_issue(issue, current_user) - reopen_resource_email(issue, issue.project, current_user, :issue_status_changed_email, 'reopened') + reopen_resource_email(issue, current_user, :issue_status_changed_email, 'reopened') end def merge_mr(merge_request, current_user) close_resource_email( merge_request, - merge_request.target_project, current_user, :merged_merge_request_email, skip_current_user: !merge_request.merge_when_pipeline_succeeds? @@ -169,7 +166,6 @@ class NotificationService def reopen_mr(merge_request, current_user) reopen_resource_email( merge_request, - merge_request.target_project, current_user, :merge_request_status_email, 'reopened' @@ -177,7 +173,7 @@ class NotificationService end def resolve_all_discussions(merge_request, current_user) - recipients = NotificationRecipientService.new(merge_request.target_project).build_recipients( + recipients = NotificationRecipientService.build_recipients( merge_request, current_user, action: "resolve_all_discussions") @@ -202,7 +198,7 @@ class NotificationService notify_method = "note_#{note.to_ability_name}_email".to_sym - recipients = NotificationRecipientService.new(note.project).build_new_note_recipients(note) + recipients = NotificationRecipientService.build_new_note_recipients(note) recipients.each do |recipient| mailer.send(notify_method, recipient.id, note.id).deliver_later end @@ -270,8 +266,7 @@ class NotificationService end def project_was_moved(project, old_path_with_namespace) - recipients = project.team.members - recipients = NotificationRecipientService.new(project).reject_muted_users(recipients) + recipients = NotificationRecipientService.notifiable_users(project.team.members, :mention, project: project) recipients.each do |recipient| mailer.project_was_moved_email( @@ -283,7 +278,7 @@ class NotificationService end def issue_moved(issue, new_issue, current_user) - recipients = NotificationRecipientService.new(issue.project).build_recipients(issue, current_user, action: 'moved') + recipients = NotificationRecipientService.build_recipients(issue, current_user, action: 'moved') recipients.map do |recipient| email = mailer.issue_moved_email(recipient, issue, new_issue, current_user) @@ -305,10 +300,10 @@ class NotificationService return unless mailer.respond_to?(email_template) - recipients ||= NotificationRecipientService.new(pipeline.project).build_pipeline_recipients( - pipeline, - pipeline.user, - action: pipeline.status + recipients ||= NotificationRecipientService.notifiable_users( + [pipeline.user], :watch, + custom_action: :"#{pipeline.status}_pipeline", + target: pipeline ).map(&:notification_email) if recipients.any? @@ -318,16 +313,16 @@ class NotificationService protected - def new_resource_email(target, project, method) - recipients = NotificationRecipientService.new(project).build_recipients(target, target.author, action: "new") + def new_resource_email(target, method) + recipients = NotificationRecipientService.build_recipients(target, target.author, action: "new") recipients.each do |recipient| mailer.send(method, recipient.id, target.id).deliver_later end end - def new_mentions_in_resource_email(target, project, new_mentioned_users, current_user, method) - recipients = NotificationRecipientService.new(project).build_recipients(target, current_user, action: "new") + def new_mentions_in_resource_email(target, new_mentioned_users, current_user, method) + recipients = NotificationRecipientService.build_recipients(target, current_user, action: "new") recipients = recipients & new_mentioned_users recipients.each do |recipient| @@ -335,10 +330,10 @@ class NotificationService end end - def close_resource_email(target, project, current_user, method, skip_current_user: true) + def close_resource_email(target, current_user, method, skip_current_user: true) action = method == :merged_merge_request_email ? "merge" : "close" - recipients = NotificationRecipientService.new(project).build_recipients( + recipients = NotificationRecipientService.build_recipients( target, current_user, action: action, @@ -350,11 +345,11 @@ class NotificationService end end - def reassign_resource_email(target, project, current_user, method) + def reassign_resource_email(target, current_user, method) previous_assignee_id = previous_record(target, 'assignee_id') previous_assignee = User.find_by(id: previous_assignee_id) if previous_assignee_id - recipients = NotificationRecipientService.new(project).build_recipients( + recipients = NotificationRecipientService.build_recipients( target, current_user, action: "reassign", @@ -372,8 +367,14 @@ class NotificationService end end - def relabeled_resource_email(target, project, labels, current_user, method) - recipients = NotificationRecipientService.new(project).build_relabeled_recipients(target, current_user, labels: labels) + def relabeled_resource_email(target, labels, current_user, method) + recipients = labels.flat_map { |l| l.subscribers(target.project) } + recipients = NotificationRecipientService.notifiable_users( + recipients, :subscription, + target: target, + acting_user: current_user + ) + label_names = labels.map(&:name) recipients.each do |recipient| @@ -381,8 +382,8 @@ class NotificationService end end - def reopen_resource_email(target, project, current_user, method, status) - recipients = NotificationRecipientService.new(project).build_recipients(target, current_user, action: "reopen") + def reopen_resource_email(target, current_user, method, status) + recipients = NotificationRecipientService.build_recipients(target, current_user, action: "reopen") recipients.each do |recipient| mailer.send(method, recipient.id, target.id, status, current_user.id).deliver_later diff --git a/app/services/todo_service.rb b/app/services/todo_service.rb index 322c6286365..6ee96d6a0f8 100644 --- a/app/services/todo_service.rb +++ b/app/services/todo_service.rb @@ -170,20 +170,22 @@ class TodoService # When user marks some todos as done def mark_todos_as_done(todos, current_user) - update_todos_state_by_ids(todos.select(&:id), current_user, :done) + update_todos_state(todos, current_user, :done) end def mark_todos_as_done_by_ids(ids, current_user) - update_todos_state_by_ids(ids, current_user, :done) + todos = todos_by_ids(ids, current_user) + mark_todos_as_done(todos, current_user) end # When user marks some todos as pending def mark_todos_as_pending(todos, current_user) - update_todos_state_by_ids(todos.select(&:id), current_user, :pending) + update_todos_state(todos, current_user, :pending) end def mark_todos_as_pending_by_ids(ids, current_user) - update_todos_state_by_ids(ids, current_user, :pending) + todos = todos_by_ids(ids, current_user) + mark_todos_as_pending(todos, current_user) end # When user marks an issue as todo @@ -198,9 +200,11 @@ class TodoService private - def update_todos_state_by_ids(ids, current_user, state) - todos = current_user.todos.where(id: ids) + def todos_by_ids(ids, current_user) + current_user.todos.where(id: Array(ids)) + end + def update_todos_state(todos, current_user, state) # Only update those that are not really on that state todos = todos.where.not(state: state) todos_ids = todos.pluck(:id) diff --git a/app/views/layouts/_bootlint.haml b/app/views/layouts/_bootlint.haml index 69280687a9d..d603a74c4e4 100644 --- a/app/views/layouts/_bootlint.haml +++ b/app/views/layouts/_bootlint.haml @@ -1,4 +1,5 @@ +-# haml-lint:disable InlineJavaScript :javascript - jQuery(document).ready(function() { - javascript:(function(){var s=document.createElement("script");s.onload=function(){bootlint.showLintReportForCurrentDocument([], {hasProblems: false, problemFree: false});};s.src="https://maxcdn.bootstrapcdn.com/bootlint/latest/bootlint.min.js";document.body.appendChild(s)})(); - }); + window.onload = function() { + var s=document.createElement("script");s.onload=function(){bootlint.showLintReportForCurrentDocument([], {hasProblems: false, problemFree: false});};s.src="https://maxcdn.bootstrapcdn.com/bootlint/latest/bootlint.min.js";document.body.appendChild(s); + } diff --git a/app/views/layouts/_init_auto_complete.html.haml b/app/views/layouts/_init_auto_complete.html.haml index 9704c9ec624..fe0ec35d003 100644 --- a/app/views/layouts/_init_auto_complete.html.haml +++ b/app/views/layouts/_init_auto_complete.html.haml @@ -4,6 +4,7 @@ - if project -# haml-lint:disable InlineJavaScript :javascript + gl = window.gl || {}; gl.GfmAutoComplete = gl.GfmAutoComplete || {}; gl.GfmAutoComplete.dataSources = { members: "#{members_project_autocomplete_sources_path(project, type: noteable_type, type_id: params[:id])}", diff --git a/app/views/layouts/nav/_group.html.haml b/app/views/layouts/nav/_group.html.haml index 8605380848d..261445ecd2b 100644 --- a/app/views/layouts/nav/_group.html.haml +++ b/app/views/layouts/nav/_group.html.haml @@ -25,7 +25,7 @@ %span Members - if current_user && can?(current_user, :admin_group, @group) - = nav_link(path: %w[groups#projects groups#edit]) do + = nav_link(path: %w[groups#projects groups#edit ci_cd#show]) do = link_to edit_group_path(@group), title: 'Settings' do %span Settings diff --git a/app/views/layouts/nav/_new_group_sidebar.html.haml b/app/views/layouts/nav/_new_group_sidebar.html.haml index fdfd7e60732..33a83866cbf 100644 --- a/app/views/layouts/nav/_new_group_sidebar.html.haml +++ b/app/views/layouts/nav/_new_group_sidebar.html.haml @@ -85,6 +85,6 @@ Projects = nav_link(controller: :ci_cd) do - = link_to group_settings_ci_cd_path(@group), title: 'Pipelines' do + = link_to group_settings_ci_cd_path(@group), title: 'CI / CD' do %span - Pipelines + CI / CD diff --git a/app/views/layouts/nav/_new_project_sidebar.html.haml b/app/views/layouts/nav/_new_project_sidebar.html.haml index 9f1cb248c4e..8e246b6e91f 100644 --- a/app/views/layouts/nav/_new_project_sidebar.html.haml +++ b/app/views/layouts/nav/_new_project_sidebar.html.haml @@ -122,9 +122,9 @@ - if project_nav_tab? :pipelines = nav_link(controller: [:pipelines, :builds, :jobs, :pipeline_schedules, :environments, :artifacts]) do - = link_to project_pipelines_path(@project), title: 'Pipelines', class: 'shortcuts-pipelines' do + = link_to project_pipelines_path(@project), title: 'CI / CD', class: 'shortcuts-pipelines' do .nav-icon-container - = custom_icon('pipeline') + = custom_icon('CI / CD') %span.nav-item-name Pipelines @@ -205,9 +205,9 @@ Repository - if @project.feature_available?(:builds, current_user) = nav_link(controller: :ci_cd) do - = link_to project_settings_ci_cd_path(@project), title: 'Pipelines' do + = link_to project_settings_ci_cd_path(@project), title: 'CI / CD' do %span - Pipelines + CI / CD - if Gitlab.config.pages.enabled = nav_link(controller: :pages) do = link_to project_pages_path(@project), title: 'Pages' do diff --git a/app/views/projects/diffs/_diffs.html.haml b/app/views/projects/diffs/_diffs.html.haml index f9385459a66..8c8aa4c78f5 100644 --- a/app/views/projects/diffs/_diffs.html.haml +++ b/app/views/projects/diffs/_diffs.html.haml @@ -3,7 +3,7 @@ - can_create_note = !@diff_notes_disabled && can?(current_user, :create_note, diffs.project) - diff_files = diffs.diff_files -.content-block.oneline-block.files-changed +.content-block.oneline-block.files-changed.diff-files-changed.js-diff-files-changed .inline-parallel-buttons - if !diffs_expanded? && diff_files.any? { |diff_file| diff_file.collapsed? } = link_to 'Expand all', url_for(params.merge(expanded: 1, format: nil)), class: 'btn btn-default' diff --git a/app/views/projects/diffs/_stats.html.haml b/app/views/projects/diffs/_stats.html.haml index e69c7f20d49..efc0ea31917 100644 --- a/app/views/projects/diffs/_stats.html.haml +++ b/app/views/projects/diffs/_stats.html.haml @@ -1,36 +1,34 @@ -.js-toggle-container - .commit-stat-summary - Showing - %button.diff-stats-summary-toggler.js-toggle-button{ type: "button" } - %strong= pluralize(diff_files.size, "changed file") +- sum_added_lines = diff_files.sum(&:added_lines) +- sum_removed_lines = diff_files.sum(&:removed_lines) +.commit-stat-summary.dropdown + Showing + %button.diff-stats-summary-toggler.js-diff-stats-dropdown{ type: "button", data: { toggle: "dropdown" } }< + = pluralize(diff_files.size, "changed file") + = icon("caret-down", class: "prepend-left-5") + %span.diff-stats-additions-deletions-expanded#diff-stats with - %strong.cgreen #{diff_files.sum(&:added_lines)} additions + %strong.cgreen #{sum_added_lines} additions and - %strong.cred #{diff_files.sum(&:removed_lines)} deletions - .file-stats.js-toggle-content.hide - %ul - - diff_files.each do |diff_file| - - file_hash = hexdigest(diff_file.file_path) - %li - - if diff_file.deleted_file? - %span.deleted-file - %a{ href: "##{file_hash}" } - %i.fa.fa-minus - = diff_file.old_path - - elsif diff_file.renamed_file? - %span.renamed-file - %a{ href: "##{file_hash}" } - %i.fa.fa-minus - = diff_file.old_path - → - = diff_file.new_path - - elsif diff_file.new_file? - %span.new-file - %a{ href: "##{file_hash}" } - %i.fa.fa-plus - = diff_file.new_path - - else - %span.edit-file - %a{ href: "##{file_hash}" } - %i.fa.fa-adjust - = diff_file.new_path + %strong.cred #{sum_removed_lines} deletions + .diff-stats-additions-deletions-collapsed.pull-right{ "aria-hidden": "true", "aria-describedby": "diff-stats" } + %strong.cgreen< + +#{sum_added_lines} + %strong.cred< + \-#{sum_removed_lines} + .dropdown-menu.diff-file-changes + = dropdown_filter("Search files") + .dropdown-content + %ul + - diff_files.each do |diff_file| + %li + %a{ href: "##{hexdigest(diff_file.file_path)}", title: diff_file.new_path } + = icon("#{diff_file_changed_icon(diff_file)} fw", class: "#{diff_file_changed_icon_color(diff_file)} append-right-5") + %span.diff-file-changes-path= diff_file.new_path + .pull-right + %span.cgreen< + +#{diff_file.added_lines} + %span.cred< + \-#{diff_file.removed_lines} + %li.dropdown-menu-empty-link.hidden + %a{ href: "#" } + No files found. diff --git a/app/views/projects/merge_requests/show.html.haml b/app/views/projects/merge_requests/show.html.haml index ea6cd16c7ad..d27e121beb4 100644 --- a/app/views/projects/merge_requests/show.html.haml +++ b/app/views/projects/merge_requests/show.html.haml @@ -17,6 +17,7 @@ -# haml-lint:disable InlineJavaScript :javascript + window.gl = window.gl || {}; window.gl.mrWidgetData = #{serialize_issuable(@merge_request)} #js-vue-mr-widget.mr-widget diff --git a/changelogs/unreleased/35659-rename-pipeline.yml b/changelogs/unreleased/35659-rename-pipeline.yml new file mode 100644 index 00000000000..0fe211868e4 --- /dev/null +++ b/changelogs/unreleased/35659-rename-pipeline.yml @@ -0,0 +1,4 @@ +--- +title: Rename Pipelines tab to CI / CD in new navigation +merge_request: +author: diff --git a/changelogs/unreleased/3686_make_tarball_download_url.yml b/changelogs/unreleased/3686_make_tarball_download_url.yml new file mode 100644 index 00000000000..4e75e52e3ac --- /dev/null +++ b/changelogs/unreleased/3686_make_tarball_download_url.yml @@ -0,0 +1,4 @@ +--- +title: repository archive download url now ends with selected file extension +merge_request: 13178 +author: haseebeqx diff --git a/changelogs/unreleased/diff-changed-files-dropdown.yml b/changelogs/unreleased/diff-changed-files-dropdown.yml new file mode 100644 index 00000000000..2d2a26ffea2 --- /dev/null +++ b/changelogs/unreleased/diff-changed-files-dropdown.yml @@ -0,0 +1,4 @@ +--- +title: Moved diff changed files into a dropdown +merge_request: +author: diff --git a/changelogs/unreleased/fix-oauth-checkboxes.yml b/changelogs/unreleased/fix-oauth-checkboxes.yml new file mode 100644 index 00000000000..2839ccc42cb --- /dev/null +++ b/changelogs/unreleased/fix-oauth-checkboxes.yml @@ -0,0 +1,4 @@ +--- +title: Fixed sign-in restrictions buttons not toggling active state +merge_request: +author: diff --git a/changelogs/unreleased/reorganise-issues-indexes-for-sorting.yml b/changelogs/unreleased/reorganise-issues-indexes-for-sorting.yml new file mode 100644 index 00000000000..5bfe55e562f --- /dev/null +++ b/changelogs/unreleased/reorganise-issues-indexes-for-sorting.yml @@ -0,0 +1,4 @@ +--- +title: Re-organise "issues" indexes for faster ordering +merge_request: +author: diff --git a/changelogs/unreleased/tc-no-todo-service-select.yml b/changelogs/unreleased/tc-no-todo-service-select.yml new file mode 100644 index 00000000000..ddcae334aa7 --- /dev/null +++ b/changelogs/unreleased/tc-no-todo-service-select.yml @@ -0,0 +1,4 @@ +--- +title: Avoid plucking Todo ids in TodoService +merge_request: 10845 +author: diff --git a/config/application.rb b/config/application.rb index 1c13cc81270..f7145566262 100644 --- a/config/application.rb +++ b/config/application.rb @@ -23,13 +23,13 @@ module Gitlab # https://github.com/rails/rails/blob/v4.2.6/railties/lib/rails/engine.rb#L687 # This is a nice reference article on autoloading/eager loading: # http://blog.arkency.com/2014/11/dont-forget-about-eager-load-when-extending-autoload - config.eager_load_paths.push(*%W(#{config.root}/lib + config.eager_load_paths.push(*%W[#{config.root}/lib #{config.root}/app/models/hooks #{config.root}/app/models/members #{config.root}/app/models/project_services #{config.root}/app/workers/concerns #{config.root}/app/services/concerns - #{config.root}/app/finders/concerns)) + #{config.root}/app/finders/concerns]) config.generators.templates.push("#{config.root}/generator_templates") diff --git a/config/routes/repository.rb b/config/routes/repository.rb index edcf3ddf57b..2ba16035ece 100644 --- a/config/routes/repository.rb +++ b/config/routes/repository.rb @@ -2,7 +2,7 @@ resource :repository, only: [:create] do member do - get 'archive', constraints: { format: Gitlab::PathRegex.archive_formats_regex } + get ':ref/archive', constraints: { format: Gitlab::PathRegex.archive_formats_regex, ref: /.+/ }, action: 'archive', as: 'archive' end end diff --git a/db/migrate/20170803130232_reorganise_issues_indexes_for_faster_sorting.rb b/db/migrate/20170803130232_reorganise_issues_indexes_for_faster_sorting.rb new file mode 100644 index 00000000000..eb7d1be1732 --- /dev/null +++ b/db/migrate/20170803130232_reorganise_issues_indexes_for_faster_sorting.rb @@ -0,0 +1,43 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class ReorganiseIssuesIndexesForFasterSorting < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + disable_ddl_transaction! + + REMOVE_INDEX_COLUMNS = %i[project_id created_at due_date updated_at].freeze + + ADD_INDEX_COLUMNS = [ + %i[project_id created_at id state], + %i[project_id due_date id state], + %i[project_id updated_at id state] + ].freeze + + TABLE = :issues + + def up + add_indexes(ADD_INDEX_COLUMNS) + remove_indexes(REMOVE_INDEX_COLUMNS) + end + + def down + add_indexes(REMOVE_INDEX_COLUMNS) + remove_indexes(ADD_INDEX_COLUMNS) + end + + def add_indexes(columns) + columns.each do |column| + add_concurrent_index(TABLE, column) unless index_exists?(TABLE, column) + end + end + + def remove_indexes(columns) + columns.each do |column| + remove_concurrent_index(TABLE, column) if index_exists?(TABLE, column) + end + end +end diff --git a/db/post_migrate/20170703130158_schedule_merge_request_diff_migrations.rb b/db/post_migrate/20170703130158_schedule_merge_request_diff_migrations.rb new file mode 100644 index 00000000000..17a9dc293f1 --- /dev/null +++ b/db/post_migrate/20170703130158_schedule_merge_request_diff_migrations.rb @@ -0,0 +1,33 @@ +class ScheduleMergeRequestDiffMigrations < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + BATCH_SIZE = 2500 + MIGRATION = 'DeserializeMergeRequestDiffsAndCommits' + + disable_ddl_transaction! + + class MergeRequestDiff < ActiveRecord::Base + self.table_name = 'merge_request_diffs' + + include ::EachBatch + end + + # Assuming that there are 5 million rows affected (which is more than on + # GitLab.com), and that each batch of 2,500 rows takes up to 5 minutes, then + # we can migrate all the rows in 7 days. + # + # On staging, plucking the IDs themselves takes 5 seconds. + def up + non_empty = 'st_commits IS NOT NULL OR st_diffs IS NOT NULL' + + MergeRequestDiff.where(non_empty).each_batch(of: BATCH_SIZE) do |relation, index| + range = relation.pluck('MIN(id)', 'MAX(id)').first + + BackgroundMigrationWorker.perform_in(index * 5.minutes, MIGRATION, range) + end + end + + def down + end +end diff --git a/db/schema.rb b/db/schema.rb index 4ba218e1e0d..f2f35acef95 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: 20170728101014) do +ActiveRecord::Schema.define(version: 20170803130232) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -641,12 +641,13 @@ ActiveRecord::Schema.define(version: 20170728101014) do 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", ["created_at"], name: "index_issues_on_created_at", using: :btree add_index "issues", ["deleted_at"], name: "index_issues_on_deleted_at", using: :btree add_index "issues", ["description"], name: "index_issues_on_description_trigram", using: :gin, opclasses: {"description"=>"gin_trgm_ops"} - add_index "issues", ["due_date"], name: "index_issues_on_due_date", using: :btree add_index "issues", ["milestone_id"], name: "index_issues_on_milestone_id", using: :btree + add_index "issues", ["project_id", "created_at", "id", "state"], name: "index_issues_on_project_id_and_created_at_and_id_and_state", using: :btree + add_index "issues", ["project_id", "due_date", "id", "state"], name: "index_issues_on_project_id_and_due_date_and_id_and_state", using: :btree add_index "issues", ["project_id", "iid"], name: "index_issues_on_project_id_and_iid", unique: true, using: :btree + add_index "issues", ["project_id", "updated_at", "id", "state"], name: "index_issues_on_project_id_and_updated_at_and_id_and_state", using: :btree add_index "issues", ["relative_position"], name: "index_issues_on_relative_position", using: :btree add_index "issues", ["state"], name: "index_issues_on_state", using: :btree add_index "issues", ["title"], name: "index_issues_on_title_trigram", using: :gin, opclasses: {"title"=>"gin_trgm_ops"} diff --git a/doc/articles/index.md b/doc/articles/index.md index 9d2e5956029..558c624fe39 100644 --- a/doc/articles/index.md +++ b/doc/articles/index.md @@ -80,7 +80,7 @@ Install, upgrade, integrate, migrate to GitLab: | :------------ | :------: | --------------: | | [Video Tutorial: Idea to Production on Google Container Engine (GKE)](https://about.gitlab.com/2017/01/23/video-tutorial-idea-to-production-on-google-container-engine-gke/) | Tutorial | 2017/01/23 | | [How to Setup a GitLab Instance on Microsoft Azure](https://about.gitlab.com/2016/07/13/how-to-setup-a-gitlab-instance-on-microsoft-azure/) | Tutorial | 2016/07/13 | -| [Get started with OpenShift Origin 3 and GitLab](https://about.gitlab.com/2016/06/28/get-started-with-openshift-origin-3-and-gitlab/) | Tutorial | 2016/06/28 | +| [Get started with OpenShift Origin 3 and GitLab](openshift_and_gitlab/index.md) | Tutorial | 2016/06/28 | | [Getting started with GitLab and DigitalOcean](https://about.gitlab.com/2016/04/27/getting-started-with-gitlab-and-digitalocean/) | Tutorial | 2016/04/27 | ## Software development diff --git a/doc/articles/openshift_and_gitlab/img/add-gitlab-to-project.png b/doc/articles/openshift_and_gitlab/img/add-gitlab-to-project.png Binary files differnew file mode 100644 index 00000000000..fcad4e59ae3 --- /dev/null +++ b/doc/articles/openshift_and_gitlab/img/add-gitlab-to-project.png diff --git a/doc/articles/openshift_and_gitlab/img/add-to-project.png b/doc/articles/openshift_and_gitlab/img/add-to-project.png Binary files differnew file mode 100644 index 00000000000..bd915a229f6 --- /dev/null +++ b/doc/articles/openshift_and_gitlab/img/add-to-project.png diff --git a/doc/articles/openshift_and_gitlab/img/create-project-ui.png b/doc/articles/openshift_and_gitlab/img/create-project-ui.png Binary files differnew file mode 100644 index 00000000000..e72866f252a --- /dev/null +++ b/doc/articles/openshift_and_gitlab/img/create-project-ui.png diff --git a/doc/articles/openshift_and_gitlab/img/gitlab-logs.png b/doc/articles/openshift_and_gitlab/img/gitlab-logs.png Binary files differnew file mode 100644 index 00000000000..1e24080c7df --- /dev/null +++ b/doc/articles/openshift_and_gitlab/img/gitlab-logs.png diff --git a/doc/articles/openshift_and_gitlab/img/gitlab-overview.png b/doc/articles/openshift_and_gitlab/img/gitlab-overview.png Binary files differnew file mode 100644 index 00000000000..3c5df0ea101 --- /dev/null +++ b/doc/articles/openshift_and_gitlab/img/gitlab-overview.png diff --git a/doc/articles/openshift_and_gitlab/img/gitlab-running.png b/doc/articles/openshift_and_gitlab/img/gitlab-running.png Binary files differnew file mode 100644 index 00000000000..c7db691cb30 --- /dev/null +++ b/doc/articles/openshift_and_gitlab/img/gitlab-running.png diff --git a/doc/articles/openshift_and_gitlab/img/gitlab-scale.png b/doc/articles/openshift_and_gitlab/img/gitlab-scale.png Binary files differnew file mode 100644 index 00000000000..4903c7d7498 --- /dev/null +++ b/doc/articles/openshift_and_gitlab/img/gitlab-scale.png diff --git a/doc/articles/openshift_and_gitlab/img/gitlab-settings.png b/doc/articles/openshift_and_gitlab/img/gitlab-settings.png Binary files differnew file mode 100644 index 00000000000..db4360ffef0 --- /dev/null +++ b/doc/articles/openshift_and_gitlab/img/gitlab-settings.png diff --git a/doc/articles/openshift_and_gitlab/img/no-resources.png b/doc/articles/openshift_and_gitlab/img/no-resources.png Binary files differnew file mode 100644 index 00000000000..480fb766468 --- /dev/null +++ b/doc/articles/openshift_and_gitlab/img/no-resources.png diff --git a/doc/articles/openshift_and_gitlab/img/openshift-infra-project.png b/doc/articles/openshift_and_gitlab/img/openshift-infra-project.png Binary files differnew file mode 100644 index 00000000000..8b9f85aa341 --- /dev/null +++ b/doc/articles/openshift_and_gitlab/img/openshift-infra-project.png diff --git a/doc/articles/openshift_and_gitlab/img/pods-overview.png b/doc/articles/openshift_and_gitlab/img/pods-overview.png Binary files differnew file mode 100644 index 00000000000..e1cf08bd217 --- /dev/null +++ b/doc/articles/openshift_and_gitlab/img/pods-overview.png diff --git a/doc/articles/openshift_and_gitlab/img/rc-name.png b/doc/articles/openshift_and_gitlab/img/rc-name.png Binary files differnew file mode 100644 index 00000000000..889e34adbec --- /dev/null +++ b/doc/articles/openshift_and_gitlab/img/rc-name.png diff --git a/doc/articles/openshift_and_gitlab/img/running-pods.png b/doc/articles/openshift_and_gitlab/img/running-pods.png Binary files differnew file mode 100644 index 00000000000..3fd4e56662f --- /dev/null +++ b/doc/articles/openshift_and_gitlab/img/running-pods.png diff --git a/doc/articles/openshift_and_gitlab/img/storage-volumes.png b/doc/articles/openshift_and_gitlab/img/storage-volumes.png Binary files differnew file mode 100644 index 00000000000..ae1e5381faa --- /dev/null +++ b/doc/articles/openshift_and_gitlab/img/storage-volumes.png diff --git a/doc/articles/openshift_and_gitlab/img/web-console.png b/doc/articles/openshift_and_gitlab/img/web-console.png Binary files differnew file mode 100644 index 00000000000..aa1425d4f94 --- /dev/null +++ b/doc/articles/openshift_and_gitlab/img/web-console.png diff --git a/doc/articles/openshift_and_gitlab/index.md b/doc/articles/openshift_and_gitlab/index.md new file mode 100644 index 00000000000..7f76e577efa --- /dev/null +++ b/doc/articles/openshift_and_gitlab/index.md @@ -0,0 +1,510 @@ +# Getting started with OpenShift Origin 3 and GitLab + +> **Article [Type](../../development/writing_documentation.html#types-of-technical-articles):** tutorial || +> **Level:** intermediary || +> **Author:** [Achilleas Pipinellis](https://gitlab.com/axil) || +> **Publication date:** 2016/06/28 + +## Introduction + +[OpenShift Origin][openshift] is an open source container application +platform created by [RedHat], based on [kubernetes] and [Docker]. That means +you can host your own PaaS for free and almost with no hassle. + +In this tutorial, we will see how to deploy GitLab in OpenShift using GitLab's +official Docker image while getting familiar with the web interface and CLI +tools that will help us achieve our goal. + +--- + +## Prerequisites + +OpenShift 3 is not yet deployed on RedHat's offered Online platform ([openshift.com]), +so in order to test it, we will use an [all-in-one Virtualbox image][vm] that is +offered by the OpenShift developers and managed by Vagrant. If you haven't done +already, go ahead and install the following components as they are essential to +test OpenShift easily: + +- [VirtualBox] +- [Vagrant] +- [OpenShift Client][oc] (`oc` for short) + +It is also important to mention that for the purposes of this tutorial, the +latest Origin release is used: + +- **oc** `v1.3.0` (must be [installed][oc-gh] locally on your computer) +- **openshift** `v1.3.0` (is pre-installed in the [VM image][vm-new]) +- **kubernetes** `v1.3.0` (is pre-installed in the [VM image][vm-new]) + +>**Note:** +If you intend to deploy GitLab on a production OpenShift cluster, there are some +limitations to bare in mind. Read on the [limitations](#current-limitations) +section for more information and follow the linked links for the relevant +discussions. + +Now that you have all batteries, let's see how easy it is to test OpenShift +on your computer. + +## Getting familiar with OpenShift Origin + +The environment we are about to use is based on CentOS 7 which comes with all +the tools needed pre-installed: Docker, kubernetes, OpenShift, etcd. + +### Test OpenShift using Vagrant + +As of this writing, the all-in-one VM is at version 1.3, and that's +what we will use in this tutorial. + +In short: + +1. Open a terminal and in a new directory run: + ```sh + vagrant init openshift/origin-all-in-one + ``` +1. This will generate a Vagrantfile based on the all-in-one VM image +1. In the same directory where you generated the Vagrantfile + enter: + + ```sh + vagrant up + ``` + +This will download the VirtualBox image and fire up the VM with some preconfigured +values as you can see in the Vagrantfile. As you may have noticed, you need +plenty of RAM (5GB in our example), so make sure you have enough. + +Now that OpenShift is setup, let's see how the web console looks like. + +### Explore the OpenShift web console + +Once Vagrant finishes its thing with the VM, you will be presented with a +message which has some important information. One of them is the IP address +of the deployed OpenShift platform and in particular <https://10.2.2.2:8443/console/>. +Open this link with your browser and accept the self-signed certificate in +order to proceed. + +Let's login as admin with username/password `admin/admin`. This is what the +landing page looks like: + + + +You can see that a number of [projects] are already created for testing purposes. + +If you head over the `openshift-infra` project, a number of services with their +respective pods are there to explore. + + + +We are not going to explore the whole interface, but if you want to learn about +the key concepts of OpenShift, read the [core concepts reference][core] in the +official documentation. + +### Explore the OpenShift CLI + +OpenShift Client (`oc`), is a powerful CLI tool that talks to the OpenShift API +and performs pretty much everything you can do from the web UI and much more. + +Assuming you have [installed][oc] it, let's explore some of its main +functionalities. + +Let's first see the version of `oc`: + +```sh +$ oc version + +oc v1.3.0 +kubernetes v1.3.0+52492b4 +``` + +With `oc help` you can see the top level arguments you can run with `oc` and +interact with your cluster, kubernetes, run applications, create projects and +much more. + +Let's login to the all-in-one VM and see how to achieve the same results like +when we visited the web console earlier. The username/password for the +administrator user is `admin/admin`. There is also a test user with username/ +password `user/user`, with limited access. Let's login as admin for the moment: + +```sh +$ oc login https://10.2.2.2:8443 + +Authentication required for https://10.2.2.2:8443 (openshift) +Username: admin +Password: +Login successful. + +You have access to the following projects and can switch between them with 'oc project <projectname>': + + * cockpit + * default (current) + * delete + * openshift + * openshift-infra + * sample + +Using project "default". +``` + +Switch to the `openshift-infra` project with: + +```sh +oc project openshift-infra +``` + +And finally, see its status: + +```sh +oc status +``` + +The last command should spit a bunch of information about the statuses of the +pods and the services, which if you look closely is what we encountered in the +second image when we explored the web console. + +You can always read more about `oc` in the [OpenShift CLI documentation][oc]. + +### Troubleshooting the all-in-one VM + +Using the all-in-one VM gives you the ability to test OpenShift whenever you +want. That means you get to play with it, shutdown the VM, and pick up where +you left off. + +Sometimes though, you may encounter some issues, like OpenShift not running +when booting up the VM. The web UI may not responding or you may see issues +when trying to login with `oc`, like: + +``` +The connection to the server 10.2.2.2:8443 was refused - did you specify the right host or port? +``` + +In that case, the OpenShift service might not be running, so in order to fix it: + +1. SSH into the VM by going to the directory where the Vagrantfile is and then + run: + + ```sh + vagrant ssh + ``` + +1. Run `systemctl` and verify by the output that the `openshift` service is not + running (it will be in red color). If that's the case start the service with: + + ```sh + sudo systemctl start openshift + ``` + +1. Verify the service is up with: + + ```sh + systemctl status openshift -l + ``` + +Now you will be able to login using `oc` (like we did before) and visit the web +console. + +## Deploy GitLab + +Now that you got a taste of what OpenShift looks like, let's deploy GitLab! + +### Create a new project + +First, we will create a new project to host our application. You can do this +either by running the CLI client: + +```bash +$ oc new-project gitlab +``` + +or by using the web interface: + + + +If you used the command line, `oc` automatically uses the new project and you +can see its status with: + +```sh +$ oc status + +In project gitlab on server https://10.2.2.2:8443 + +You have no services, deployment configs, or build configs. +Run 'oc new-app' to create an application. +``` + +If you visit the web console, you can now see `gitlab` listed in the projects list. + +The next step is to import the OpenShift template for GitLab. + +### Import the template + +The [template][templates] is basically a JSON file which describes a set of +related object definitions to be created together, as well as a set of +parameters for those objects. + +The template for GitLab resides in the Omnibus GitLab repository under the +docker directory. Let's download it locally with `wget`: + +```bash +wget https://gitlab.com/gitlab-org/omnibus-gitlab/raw/master/docker/openshift-template.json +``` + +And then let's import it in OpenShift: + +```bash +oc create -f openshift-template.json -n openshift +``` + +>**Note:** +The `-n openshift` namespace flag is a trick to make the template available to all +projects. If you recall from when we created the `gitlab` project, `oc` switched +to it automatically, and that can be verified by the `oc status` command. If +you omit the namespace flag, the application will be available only to the +current project, in our case `gitlab`. The `openshift` namespace is a global +one that the administrators should use if they want the application to be +available to all users. + +We are now ready to finally deploy GitLab! + +### Create a new application + +The next step is to use the template we previously imported. Head over to the +`gitlab` project and hit the **Add to Project** button. + + + +This will bring you to the catalog where you can find all the pre-defined +applications ready to deploy with the click of a button. Search for `gitlab` +and you will see the previously imported template: + + + +Select it, and in the following screen you will be presented with the predefined +values used with the GitLab template: + + + +Notice at the top that there are three resources to be created with this +template: + +- `gitlab-ce` +- `gitlab-ce-redis` +- `gitlab-ce-postgresql` + +While PostgreSQL and Redis are bundled in Omnibus GitLab, the template is using +separate images as you can see from [this line][line] in the template. + +The predefined values have been calculated for the purposes of testing out +GitLab in the all-in-one VM. You don't need to change anything here, hit +**Create** to start the deployment. + +If you are deploying to production you will want to change the **GitLab instance +hostname** and use greater values for the volume sizes. If you don't provide a +password for PostgreSQL, it will be created automatically. + +>**Note:** +The `gitlab.apps.10.2.2.2.xip.io` hostname that is used by default will +resolve to the host with IP `10.2.2.2` which is the IP our VM uses. It is a +trick to have distinct FQDNs pointing to services that are on our local network. +Read more on how this works in <http://xip.io>. + +Now that we configured this, let's see how to manage and scale GitLab. + +## Manage and scale GitLab + +Setting up GitLab for the first time might take a while depending on your +internet connection and the resources you have attached to the all-in-one VM. +GitLab's docker image is quite big (~500MB), so you'll have to wait until +it's downloaded and configured before you use it. + +### Watch while GitLab gets deployed + +Navigate to the `gitlab` project at **Overview**. You can notice that the +deployment is in progress by the orange color. The Docker images are being +downloaded and soon they will be up and running. + + + +Switch to the **Browse > Pods** and you will eventually see all 3 pods in a +running status. Remember the 3 resources that were to be created when we first +created the GitLab app? This is where you can see them in action. + + + +You can see GitLab being reconfigured by taking look at the logs in realtime. +Click on `gitlab-ce-2-j7ioe` (your ID will be different) and go to the **Logs** +tab. + + + +At a point you should see a _**gitlab Reconfigured!**_ message in the logs. +Navigate back to the **Overview** and hopefully all pods will be up and running. + + + +Congratulations! You can now navigate to your new shinny GitLab instance by +visiting <http://gitlab.apps.10.2.2.2.xip.io> where you will be asked to +change the root user password. Login using `root` as username and providing the +password you just set, and start using GitLab! + +### Scale GitLab with the push of a button + +If you reach to a point where your GitLab instance could benefit from a boost +of resources, you'd be happy to know that you can scale up with the push of a +button. + +In the **Overview** page just click the up arrow button in the pod where +GitLab is. The change is instant and you can see the number of [replicas] now +running scaled to 2. + + + +Upping the GitLab pods is actually like adding new application servers to your +cluster. You can see how that would work if you didn't use GitLab with +OpenShift by following the [HA documentation][ha] for the application servers. + +Bare in mind that you may need more resources (CPU, RAM, disk space) when you +scale up. If a pod is in pending state for too long, you can navigate to +**Browse > Events** and see the reason and message of the state. + + + +### Scale GitLab using the `oc` CLI + +Using `oc` is super easy to scale up the replicas of a pod. You may want to +skim through the [basic CLI operations][basic-cli] to get a taste how the CLI +commands are used. Pay extra attention to the object types as we will use some +of them and their abbreviated versions below. + +In order to scale up, we need to find out the name of the replication controller. +Let's see how to do that using the following steps. + +1. Make sure you are in the `gitlab` project: + + ```sh + oc project gitlab + ``` + +1. See what services are used for this project: + + ```sh + oc get svc + ``` + + The output will be similar to: + + ``` + NAME CLUSTER-IP EXTERNAL-IP PORT(S) AGE + gitlab-ce 172.30.243.177 <none> 22/TCP,80/TCP 5d + gitlab-ce-postgresql 172.30.116.75 <none> 5432/TCP 5d + gitlab-ce-redis 172.30.105.88 <none> 6379/TCP 5d + ``` + +1. We need to see the replication controllers of the `gitlab-ce` service. + Get a detailed view of the current ones: + + ```sh + oc describe rc gitlab-ce + ``` + + This will return a large detailed list of the current replication controllers. + Search for the name of the GitLab controller, usually `gitlab-ce-1` or if + that failed at some point and you spawned another one, it will be named + `gitlab-ce-2`. + +1. Scale GitLab using the previous information: + + ```sh + oc scale --replicas=2 replicationcontrollers gitlab-ce-2 + ``` + +1. Get the new replicas number to make sure scaling worked: + + ```sh + oc get rc gitlab-ce-2 + ``` + + which will return something like: + + ``` + NAME DESIRED CURRENT AGE + gitlab-ce-2 2 2 5d + ``` + +And that's it! We successfully scaled the replicas to 2 using the CLI. + +As always, you can find the name of the controller using the web console. Just +click on the service you are interested in and you will see the details in the +right sidebar. + + + +### Autoscaling GitLab + +In case you were wondering whether there is an option to autoscale a pod based +on the resources of your server, the answer is yes, of course there is. + +We will not expand on this matter, but feel free to read the documentation on +OpenShift's website about [autoscaling]. + +## Current limitations + +As stated in the [all-in-one VM][vm] page: + +> By default, OpenShift will not allow a container to run as root or even a +non-random container assigned userid. Most Docker images in the Dockerhub do not +follow this best practice and instead run as root. + +The all-in-one VM we are using has this security turned off so it will not +bother us. In any case, it is something to keep in mind when deploying GitLab +on a production cluster. + +In order to deploy GitLab on a production cluster, you will need to assign the +GitLab service account to the `anyuid` Security Context. + +1. Edit the Security Context: + ```sh + oc edit scc anyuid + ``` + +1. Add `system:serviceaccount:<project>:gitlab-ce-user` to the `users` section. + If you changed the Application Name from the default the user will + will be `<app-name>-user` instead of `gitlab-ce-user` + +1. Save and exit the editor + +## Conclusion + +By now, you should have an understanding of the basic OpenShift Origin concepts +and a sense of how things work using the web console or the CLI. + +GitLab was hard to install in previous versions of OpenShift, +but now that belongs to the past. Upload a template, create a project, add an +application and you are done. You are ready to login to your new GitLab instance. + +And remember that in this tutorial we just scratched the surface of what Origin +is capable of. As always, you can refer to the detailed +[documentation][openshift-docs] to learn more about deploying your own OpenShift +PaaS and managing your applications with the ease of containers. + +[RedHat]: https://www.redhat.com/en "RedHat website" +[openshift]: https://www.openshift.org "OpenShift Origin website" +[vm]: https://www.openshift.org/vm/ "OpenShift All-in-one VM" +[vm-new]: https://atlas.hashicorp.com/openshift/boxes/origin-all-in-one "Official OpenShift Vagrant box on Atlas" +[template]: https://gitlab.com/gitlab-org/omnibus-gitlab/blob/master/docker/openshift-template.json "OpenShift template for GitLab" +[openshift.com]: https://openshift.com "OpenShift Online" +[kubernetes]: http://kubernetes.io/ "Kubernetes website" +[Docker]: https://www.docker.com "Docker website" +[oc]: https://docs.openshift.org/latest/cli_reference/get_started_cli.html "Documentation - oc CLI documentation" +[VirtualBox]: https://www.virtualbox.org/wiki/Downloads "VirtualBox downloads" +[Vagrant]: https://www.vagrantup.com/downloads.html "Vagrant downloads" +[projects]: https://docs.openshift.org/latest/dev_guide/projects.html "Documentation - Projects overview" +[core]: https://docs.openshift.org/latest/architecture/core_concepts/index.html "Documentation - Core concepts of OpenShift Origin" +[templates]: https://docs.openshift.org/latest/architecture/core_concepts/templates.html "Documentation - OpenShift templates" +[old-post]: https://blog.openshift.com/deploy-gitlab-openshift/ "Old post - Deploy GitLab on OpenShift" +[line]: https://gitlab.com/gitlab-org/omnibus-gitlab/blob/658c065c8d022ce858dd63eaeeadb0b2ddc8deea/docker/openshift-template.json#L239 "GitLab - OpenShift template" +[oc-gh]: https://github.com/openshift/origin/releases/tag/v1.3.0 "Openshift 1.3.0 release on GitHub" +[ha]: http://docs.gitlab.com/ce/administration/high_availability/gitlab.html "Documentation - GitLab High Availability" +[replicas]: https://docs.openshift.org/latest/architecture/core_concepts/deployments.html#replication-controllers "Documentation - Replication controller" +[autoscaling]: https://docs.openshift.org/latest/dev_guide/pod_autoscaling.html "Documentation - Autoscale" +[basic-cli]: https://docs.openshift.org/latest/cli_reference/basic_cli_operations.html "Documentation - Basic CLI operations" +[openshift-docs]: https://docs.openshift.org "OpenShift documentation" diff --git a/doc/development/fe_guide/style_guide_js.md b/doc/development/fe_guide/style_guide_js.md index 149a0159680..6ade3231fac 100644 --- a/doc/development/fe_guide/style_guide_js.md +++ b/doc/development/fe_guide/style_guide_js.md @@ -11,7 +11,7 @@ See [our current .eslintrc][eslintrc] for specific rules and patterns. #### ESlint -1. **Never** disable eslint rules unless you have a good reason. +1. **Never** disable eslint rules unless you have a good reason. You may see a lot of legacy files with `/* eslint-disable some-rule, some-other-rule */` at the top, but legacy files are a special case. Any time you develop a new feature or refactor an existing one, you should abide by the eslint rules. @@ -100,26 +100,44 @@ followed by any global declarations, then a blank newline prior to any imports o export default Foo; ``` -1. Relative paths: Unless you are writing a test, always reference other scripts using -relative paths instead of `~` - * In **app/assets/javascripts**: +1. Relative paths: when importing a module in the same directory, a child +directory, or an immediate parent directory prefer relative paths. When +importing a module which is two or more levels up, prefer either `~/` or `ee/` +. - ```javascript - // bad - import Foo from '~/foo' +In **app/assets/javascripts/my-feature/subdir**: - // good - import Foo from '../foo'; - ``` - * In **spec/javascripts**: +``` javascript +// bad +import Foo from '~/my-feature/foo'; +import Bar from '~/my-feature/subdir/bar'; +import Bin from '~/my-feature/subdir/lib/bin'; - ```javascript - // bad - import Foo from '../../app/assets/javascripts/foo' +// good +import Foo from '../foo'; +import Bar from './bar'; +import Bin from './lib/bin'; +``` - // good - import Foo from '~/foo'; - ``` +In **spec/javascripts**: + +``` javascript +// bad +import Foo from '../../app/assets/javascripts/my-feature/foo'; + +// good +import Foo from '~/my-feature/foo'; +``` + +When referencing an **EE component**: + +``` javascript +// bad +import Foo from '../../../../../ee/app/assets/javascripts/my-feature/ee-foo'; + +// good +import Foo from 'ee/my-feature/foo'; +``` 1. Avoid using IIFE. Although we have a lot of examples of files which wrap their contents in IIFEs (immediately-invoked function expressions), @@ -465,7 +483,7 @@ A forEach will cause side effects, it will be mutating the array being iterated. #### Vue and Boostrap 1. Tooltips: Do not rely on `has-tooltip` class name for Vue components - ``` javascript + ```javascript // bad <span class="has-tooltip" diff --git a/lib/api/todos.rb b/lib/api/todos.rb index d1f7e364029..55191169dd4 100644 --- a/lib/api/todos.rb +++ b/lib/api/todos.rb @@ -59,10 +59,10 @@ module API requires :id, type: Integer, desc: 'The ID of the todo being marked as done' end post ':id/mark_as_done' do - todo = current_user.todos.find(params[:id]) - TodoService.new.mark_todos_as_done([todo], current_user) + TodoService.new.mark_todos_as_done_by_ids(params[:id], current_user) + todo = Todo.find(params[:id]) - present todo.reload, with: Entities::Todo, current_user: current_user + present todo, with: Entities::Todo, current_user: current_user end desc 'Mark all todos as done' diff --git a/lib/api/v3/todos.rb b/lib/api/v3/todos.rb index e3b311d61cd..2f2cf259987 100644 --- a/lib/api/v3/todos.rb +++ b/lib/api/v3/todos.rb @@ -11,10 +11,10 @@ module API requires :id, type: Integer, desc: 'The ID of the todo being marked as done' end delete ':id' do - todo = current_user.todos.find(params[:id]) - TodoService.new.mark_todos_as_done([todo], current_user) + TodoService.new.mark_todos_as_done_by_ids(params[:id], current_user) + todo = Todo.find(params[:id]) - present todo.reload, with: ::API::Entities::Todo, current_user: current_user + present todo, with: ::API::Entities::Todo, current_user: current_user end desc 'Mark all todos as done' diff --git a/lib/banzai/renderer.rb b/lib/banzai/renderer.rb index c7801cb5baf..ad08c0905e2 100644 --- a/lib/banzai/renderer.rb +++ b/lib/banzai/renderer.rb @@ -132,6 +132,8 @@ module Banzai end def self.cacheless_render(text, context = {}) + return text.to_s unless text.present? + Gitlab::Metrics.measure(:banzai_cacheless_render) do result = render_result(text, context) diff --git a/lib/declarative_policy.rb b/lib/declarative_policy.rb index b1eb1a6cef1..ae65653645b 100644 --- a/lib/declarative_policy.rb +++ b/lib/declarative_policy.rb @@ -28,7 +28,13 @@ module DeclarativePolicy subject = find_delegate(subject) - class_for_class(subject.class) + policy_class = class_for_class(subject.class) + raise "no policy for #{subject.class.name}" if policy_class.nil? + policy_class + end + + def has_policy?(subject) + !class_for_class(subject.class).nil? end private @@ -51,9 +57,7 @@ module DeclarativePolicy end end - policy_class = subject_class.instance_variable_get(CLASS_CACHE_IVAR) - raise "no policy for #{subject.class.name}" if policy_class.nil? - policy_class + subject_class.instance_variable_get(CLASS_CACHE_IVAR) end def compute_class_for_class(subject_class) @@ -71,6 +75,8 @@ module DeclarativePolicy nil end end + + nil end def find_delegate(subject) diff --git a/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits.rb b/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits.rb new file mode 100644 index 00000000000..0fbc6b70989 --- /dev/null +++ b/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits.rb @@ -0,0 +1,107 @@ +module Gitlab + module BackgroundMigration + class DeserializeMergeRequestDiffsAndCommits + attr_reader :diff_ids, :commit_rows, :file_rows + + class MergeRequestDiff < ActiveRecord::Base + self.table_name = 'merge_request_diffs' + end + + BUFFER_ROWS = 1000 + + def perform(start_id, stop_id) + merge_request_diffs = MergeRequestDiff + .select(:id, :st_commits, :st_diffs) + .where('st_commits IS NOT NULL OR st_diffs IS NOT NULL') + .where(id: start_id..stop_id) + + reset_buffers! + + merge_request_diffs.each do |merge_request_diff| + commits, files = single_diff_rows(merge_request_diff) + + diff_ids << merge_request_diff.id + commit_rows.concat(commits) + file_rows.concat(files) + + if diff_ids.length > BUFFER_ROWS || + commit_rows.length > BUFFER_ROWS || + file_rows.length > BUFFER_ROWS + + flush_buffers! + end + end + + flush_buffers! + end + + private + + def reset_buffers! + @diff_ids = [] + @commit_rows = [] + @file_rows = [] + end + + def flush_buffers! + if diff_ids.any? + MergeRequestDiff.transaction do + Gitlab::Database.bulk_insert('merge_request_diff_commits', commit_rows) + Gitlab::Database.bulk_insert('merge_request_diff_files', file_rows) + + MergeRequestDiff.where(id: diff_ids).update_all(st_commits: nil, st_diffs: nil) + end + end + + reset_buffers! + end + + def single_diff_rows(merge_request_diff) + sha_attribute = Gitlab::Database::ShaAttribute.new + commits = YAML.load(merge_request_diff.st_commits) rescue [] + + commit_rows = commits.map.with_index do |commit, index| + commit_hash = commit.to_hash.with_indifferent_access.except(:parent_ids) + sha = commit_hash.delete(:id) + + commit_hash.merge( + merge_request_diff_id: merge_request_diff.id, + relative_order: index, + sha: sha_attribute.type_cast_for_database(sha) + ) + end + + diffs = YAML.load(merge_request_diff.st_diffs) rescue [] + diffs = [] unless valid_raw_diffs?(diffs) + + file_rows = diffs.map.with_index do |diff, index| + diff_hash = diff.to_hash.with_indifferent_access.merge( + binary: false, + merge_request_diff_id: merge_request_diff.id, + relative_order: index + ) + + # Compatibility with old diffs created with Psych. + diff_hash.tap do |hash| + diff_text = hash[:diff] + + if diff_text.encoding == Encoding::BINARY && !diff_text.ascii_only? + hash[:binary] = true + hash[:diff] = [diff_text].pack('m0') + end + end + end + + [commit_rows, file_rows] + end + + # Unlike MergeRequestDiff#valid_raw_diff?, don't count Rugged objects as + # valid, because we don't render them usefully anyway. + def valid_raw_diffs?(diffs) + return false unless diffs.respond_to?(:each) + + diffs.all? { |diff| diff.is_a?(Hash) } + end + end + end +end diff --git a/lib/gitlab/git/blame.rb b/lib/gitlab/git/blame.rb index 0deaab01b5b..8dbe25e55f6 100644 --- a/lib/gitlab/git/blame.rb +++ b/lib/gitlab/git/blame.rb @@ -1,5 +1,3 @@ -# Gitaly note: JV: needs 1 RPC for #load_blame. - module Gitlab module Git class Blame @@ -26,15 +24,29 @@ module Gitlab private - # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/376 def load_blame - cmd = %W(#{Gitlab.config.git.bin_path} --git-dir=#{@repo.path} blame -p #{@sha} -- #{@path}) - # Read in binary mode to ensure ASCII-8BIT - raw_output = IO.popen(cmd, 'rb') {|io| io.read } + raw_output = @repo.gitaly_migrate(:blame) do |is_enabled| + if is_enabled + load_blame_by_gitaly + else + load_blame_by_shelling_out + end + end + output = encode_utf8(raw_output) process_raw_blame output end + def load_blame_by_gitaly + @repo.gitaly_commit_client.raw_blame(@sha, @path) + end + + def load_blame_by_shelling_out + cmd = %W(#{Gitlab.config.git.bin_path} --git-dir=#{@repo.path} blame -p #{@sha} -- #{@path}) + # Read in binary mode to ensure ASCII-8BIT + IO.popen(cmd, 'rb') {|io| io.read } + end + def process_raw_blame(output) lines, final = [], [] info, commits = {}, {} diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 70d793f8e4a..734aed8fbc1 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -300,17 +300,14 @@ module Gitlab raw_log(options).map { |c| Commit.decorate(c) } end - # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/382 def count_commits(options) - cmd = %W[#{Gitlab.config.git.bin_path} --git-dir=#{path} rev-list] - cmd << "--after=#{options[:after].iso8601}" if options[:after] - cmd << "--before=#{options[:before].iso8601}" if options[:before] - cmd += %W[--count #{options[:ref]}] - cmd += %W[-- #{options[:path]}] if options[:path].present? - - raw_output = IO.popen(cmd) { |io| io.read } - - raw_output.to_i + gitaly_migrate(:count_commits) do |is_enabled| + if is_enabled + count_commits_by_gitaly(options) + else + count_commits_by_shelling_out(options) + end + end end def sha_from_ref(ref) @@ -686,6 +683,14 @@ module Gitlab @gitaly_repository_client ||= Gitlab::GitalyClient::RepositoryService.new(self) end + def gitaly_migrate(method, &block) + Gitlab::GitalyClient.migrate(method, &block) + rescue GRPC::NotFound => e + raise NoRepository.new(e) + rescue GRPC::BadStatus => e + raise CommandError.new(e) + end + private # Gitaly note: JV: Trying to get rid of the 'filter' option so we can implement this with 'git'. @@ -1001,16 +1006,29 @@ module Gitlab end.sort_by(&:name) end + def last_commit_for_path_by_rugged(sha, path) + sha = last_commit_id_for_path(sha, path) + commit(sha) + end + def tags_from_gitaly gitaly_ref_client.tags end - def gitaly_migrate(method, &block) - Gitlab::GitalyClient.migrate(method, &block) - rescue GRPC::NotFound => e - raise NoRepository.new(e) - rescue GRPC::BadStatus => e - raise CommandError.new(e) + def count_commits_by_gitaly(options) + gitaly_commit_client.commit_count(options[:ref], options) + end + + def count_commits_by_shelling_out(options) + cmd = %W[#{Gitlab.config.git.bin_path} --git-dir=#{path} rev-list] + cmd << "--after=#{options[:after].iso8601}" if options[:after] + cmd << "--before=#{options[:before].iso8601}" if options[:before] + cmd += %W[--count #{options[:ref]}] + cmd += %W[-- #{options[:path]}] if options[:path].present? + + raw_output = IO.popen(cmd) { |io| io.read } + + raw_output.to_i end end end diff --git a/lib/gitlab/gitaly_client/commit_service.rb b/lib/gitlab/gitaly_client/commit_service.rb index a834781b1f1..ac6817e6d0e 100644 --- a/lib/gitlab/gitaly_client/commit_service.rb +++ b/lib/gitlab/gitaly_client/commit_service.rb @@ -85,15 +85,32 @@ module Gitlab end end - def commit_count(ref) + def commit_count(ref, options = {}) request = Gitaly::CountCommitsRequest.new( repository: @gitaly_repo, revision: ref ) + request.after = Google::Protobuf::Timestamp.new(seconds: options[:after].to_i) if options[:after].present? + request.before = Google::Protobuf::Timestamp.new(seconds: options[:before].to_i) if options[:before].present? + request.path = options[:path] if options[:path].present? GitalyClient.call(@repository.storage, :commit_service, :count_commits, request).count end + def last_commit_for_path(revision, path) + request = Gitaly::LastCommitForPathRequest.new( + repository: @gitaly_repo, + revision: revision.force_encoding(Encoding::ASCII_8BIT), + path: path.to_s.force_encoding(Encoding::ASCII_8BIT) + ) + + gitaly_commit = GitalyClient.call(@repository.storage, :commit_service, :last_commit_for_path, request).commit + return unless gitaly_commit + + commit = GitalyClient::Commit.new(@repository, gitaly_commit) + Gitlab::Git::Commit.new(commit) + end + def between(from, to) request = Gitaly::CommitsBetweenRequest.new( repository: @gitaly_repo, @@ -125,6 +142,17 @@ module Gitlab response.languages.map { |l| { value: l.share.round(2), label: l.name, color: l.color, highlight: l.color } } end + def raw_blame(revision, path) + request = Gitaly::RawBlameRequest.new( + repository: @gitaly_repo, + revision: revision, + path: path + ) + + response = GitalyClient.call(@repository.storage, :commit_service, :raw_blame, request) + response.reduce("") { |memo, msg| memo << msg.data } + end + private def commit_diff_request_params(commit, options = {}) diff --git a/spec/controllers/projects/repositories_controller_spec.rb b/spec/controllers/projects/repositories_controller_spec.rb index 9c55d159fa0..f712d1e0d63 100644 --- a/spec/controllers/projects/repositories_controller_spec.rb +++ b/spec/controllers/projects/repositories_controller_spec.rb @@ -6,7 +6,7 @@ describe Projects::RepositoriesController do describe "GET archive" do context 'as a guest' do it 'responds with redirect in correct format' do - get :archive, namespace_id: project.namespace, project_id: project, format: "zip" + get :archive, namespace_id: project.namespace, project_id: project, format: "zip", ref: 'master' expect(response.header["Content-Type"]).to start_with('text/html') expect(response).to be_redirect diff --git a/spec/features/admin/admin_settings_spec.rb b/spec/features/admin/admin_settings_spec.rb index c1eced417cf..c9591a7d854 100644 --- a/spec/features/admin/admin_settings_spec.rb +++ b/spec/features/admin/admin_settings_spec.rb @@ -69,6 +69,14 @@ feature 'Admin updates settings' do expect(find('#service_push_channel').value).to eq '#test_channel' end + context 'sign-in restrictions', :js do + it 'de-activates oauth sign-in source' do + find('.btn', text: 'GitLab.com').click + + expect(find('.btn', text: 'GitLab.com')).not_to have_css('.active') + end + end + def check_all_events page.check('Active') page.check('Push') diff --git a/spec/helpers/defer_script_tag_helper_spec.rb b/spec/helpers/defer_script_tag_helper_spec.rb new file mode 100644 index 00000000000..d10b6f134e4 --- /dev/null +++ b/spec/helpers/defer_script_tag_helper_spec.rb @@ -0,0 +1,13 @@ +# coding: utf-8 +require 'spec_helper' + +describe DeferScriptTagHelper do + describe 'script tag' do + script_url = 'test.js' + + it 'returns an script tag with defer=true' do + expect(javascript_include_tag(script_url).to_s) + .to eq "<script src=\"/javascripts/#{script_url}\" defer=\"defer\"></script>" + end + end +end diff --git a/spec/javascripts/abuse_reports_spec.js b/spec/javascripts/abuse_reports_spec.js index 069d857eab6..13cab81dd60 100644 --- a/spec/javascripts/abuse_reports_spec.js +++ b/spec/javascripts/abuse_reports_spec.js @@ -6,10 +6,10 @@ import '~/abuse_reports'; const FIXTURE = 'abuse_reports/abuse_reports_list.html.raw'; const MAX_MESSAGE_LENGTH = 500; - let messages; + let $messages; const assertMaxLength = $message => expect($message.text().length).toEqual(MAX_MESSAGE_LENGTH); - const findMessage = searchText => messages.filter( + const findMessage = searchText => $messages.filter( (index, element) => element.innerText.indexOf(searchText) > -1, ).first(); @@ -18,7 +18,7 @@ import '~/abuse_reports'; beforeEach(function () { loadFixtures(FIXTURE); this.abuseReports = new global.AbuseReports(); - messages = $('.abuse-reports .message'); + $messages = $('.abuse-reports .message'); }); it('should truncate long messages', () => { diff --git a/spec/javascripts/ajax_loading_spinner_spec.js b/spec/javascripts/ajax_loading_spinner_spec.js index 1518ae68b0d..46e072a8ebb 100644 --- a/spec/javascripts/ajax_loading_spinner_spec.js +++ b/spec/javascripts/ajax_loading_spinner_spec.js @@ -1,4 +1,3 @@ -import '~/extensions/array'; import 'jquery'; import 'jquery-ujs'; import '~/ajax_loading_spinner'; diff --git a/spec/javascripts/extensions/array_spec.js b/spec/javascripts/extensions/array_spec.js deleted file mode 100644 index b1b81b4efc2..00000000000 --- a/spec/javascripts/extensions/array_spec.js +++ /dev/null @@ -1,22 +0,0 @@ -/* eslint-disable space-before-function-paren, no-var */ - -import '~/extensions/array'; - -(function() { - describe('Array extensions', function() { - describe('first', function() { - return it('returns the first item', function() { - var arr; - arr = [0, 1, 2, 3, 4, 5]; - return expect(arr.first()).toBe(0); - }); - }); - describe('last', function() { - return it('returns the last item', function() { - var arr; - arr = [0, 1, 2, 3, 4, 5]; - return expect(arr.last()).toBe(5); - }); - }); - }); -}).call(window); diff --git a/spec/javascripts/filtered_search/dropdown_utils_spec.js b/spec/javascripts/filtered_search/dropdown_utils_spec.js index 244f170ab7a..b1b3d43f241 100644 --- a/spec/javascripts/filtered_search/dropdown_utils_spec.js +++ b/spec/javascripts/filtered_search/dropdown_utils_spec.js @@ -1,4 +1,3 @@ -import '~/extensions/array'; import '~/filtered_search/dropdown_utils'; import '~/filtered_search/filtered_search_tokenizer'; import '~/filtered_search/filtered_search_dropdown_manager'; diff --git a/spec/javascripts/filtered_search/filtered_search_dropdown_manager_spec.js b/spec/javascripts/filtered_search/filtered_search_dropdown_manager_spec.js index 9e2076dc383..5c7e9115aac 100644 --- a/spec/javascripts/filtered_search/filtered_search_dropdown_manager_spec.js +++ b/spec/javascripts/filtered_search/filtered_search_dropdown_manager_spec.js @@ -1,4 +1,3 @@ -import '~/extensions/array'; import '~/filtered_search/filtered_search_visual_tokens'; import '~/filtered_search/filtered_search_tokenizer'; import '~/filtered_search/filtered_search_dropdown_manager'; diff --git a/spec/javascripts/filtered_search/filtered_search_token_keys_spec.js b/spec/javascripts/filtered_search/filtered_search_token_keys_spec.js index 1a7631994b4..69b424c3af5 100644 --- a/spec/javascripts/filtered_search/filtered_search_token_keys_spec.js +++ b/spec/javascripts/filtered_search/filtered_search_token_keys_spec.js @@ -1,4 +1,3 @@ -import '~/extensions/array'; import '~/filtered_search/filtered_search_token_keys'; describe('Filtered Search Token Keys', () => { diff --git a/spec/javascripts/filtered_search/filtered_search_tokenizer_spec.js b/spec/javascripts/filtered_search/filtered_search_tokenizer_spec.js index e4a15c83c23..585bea9b499 100644 --- a/spec/javascripts/filtered_search/filtered_search_tokenizer_spec.js +++ b/spec/javascripts/filtered_search/filtered_search_tokenizer_spec.js @@ -1,4 +1,3 @@ -import '~/extensions/array'; import '~/filtered_search/filtered_search_token_keys'; import '~/filtered_search/filtered_search_tokenizer'; diff --git a/spec/javascripts/lib/utils/sticky_spec.js b/spec/javascripts/lib/utils/sticky_spec.js new file mode 100644 index 00000000000..c3ee3ef9825 --- /dev/null +++ b/spec/javascripts/lib/utils/sticky_spec.js @@ -0,0 +1,52 @@ +import { isSticky } from '~/lib/utils/sticky'; + +describe('sticky', () => { + const el = { + offsetTop: 0, + classList: {}, + }; + + beforeEach(() => { + el.offsetTop = 0; + el.classList.add = jasmine.createSpy('spy'); + el.classList.remove = jasmine.createSpy('spy'); + }); + + describe('classList.remove', () => { + it('does not call classList.remove when stuck', () => { + isSticky(el, 0, 0); + + expect( + el.classList.remove, + ).not.toHaveBeenCalled(); + }); + + it('calls classList.remove when not stuck', () => { + el.offsetTop = 10; + isSticky(el, 0, 0); + + expect( + el.classList.remove, + ).toHaveBeenCalledWith('is-stuck'); + }); + }); + + describe('classList.add', () => { + it('calls classList.add when stuck', () => { + isSticky(el, 0, 0); + + expect( + el.classList.add, + ).toHaveBeenCalledWith('is-stuck'); + }); + + it('does not call classList.add when not stuck', () => { + el.offsetTop = 10; + isSticky(el, 0, 0); + + expect( + el.classList.add, + ).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/spec/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits_spec.rb b/spec/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits_spec.rb new file mode 100644 index 00000000000..18843cbe992 --- /dev/null +++ b/spec/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits_spec.rb @@ -0,0 +1,188 @@ +require 'spec_helper' + +describe Gitlab::BackgroundMigration::DeserializeMergeRequestDiffsAndCommits do + describe '#perform' do + set(:merge_request) { create(:merge_request) } + set(:merge_request_diff) { merge_request.merge_request_diff } + let(:updated_merge_request_diff) { MergeRequestDiff.find(merge_request_diff.id) } + + def diffs_to_hashes(diffs) + diffs.as_json(only: Gitlab::Git::Diff::SERIALIZE_KEYS).map(&:with_indifferent_access) + end + + def quote_yaml(value) + MergeRequestDiff.connection.quote(YAML.dump(value)) + end + + def convert_to_yaml(merge_request_diff_id, commits, diffs) + MergeRequestDiff.where(id: merge_request_diff_id).update_all( + "st_commits = #{quote_yaml(commits)}, st_diffs = #{quote_yaml(diffs)}" + ) + end + + shared_examples 'updated MR diff' do + before do + convert_to_yaml(merge_request_diff.id, commits, diffs) + + MergeRequestDiffCommit.delete_all + MergeRequestDiffFile.delete_all + + subject.perform(merge_request_diff.id, merge_request_diff.id) + end + + it 'creates correct entries in the merge_request_diff_commits table' do + expect(updated_merge_request_diff.merge_request_diff_commits.count).to eq(commits.count) + expect(updated_merge_request_diff.commits.map(&:to_hash)).to eq(commits) + end + + it 'creates correct entries in the merge_request_diff_files table' do + expect(updated_merge_request_diff.merge_request_diff_files.count).to eq(expected_diffs.count) + expect(diffs_to_hashes(updated_merge_request_diff.raw_diffs)).to eq(expected_diffs) + end + + it 'sets the st_commits and st_diffs columns to nil' do + expect(updated_merge_request_diff.st_commits_before_type_cast).to be_nil + expect(updated_merge_request_diff.st_diffs_before_type_cast).to be_nil + end + end + + context 'when the diff IDs passed do not exist' do + it 'does not raise' do + expect { subject.perform(0, 0) }.not_to raise_exception + end + end + + context 'when the merge request diff has no serialised commits or diffs' do + before do + merge_request_diff.update(st_commits: nil, st_diffs: nil) + end + + it 'does not raise' do + expect { subject.perform(merge_request_diff.id, merge_request_diff.id) } + .not_to raise_exception + end + end + + context 'processing multiple merge request diffs' do + let(:start_id) { described_class::MergeRequestDiff.minimum(:id) } + let(:stop_id) { described_class::MergeRequestDiff.maximum(:id) } + + before do + merge_request.reload_diff(true) + + convert_to_yaml(start_id, merge_request_diff.commits, merge_request_diff.diffs) + convert_to_yaml(stop_id, updated_merge_request_diff.commits, updated_merge_request_diff.diffs) + + MergeRequestDiffCommit.delete_all + MergeRequestDiffFile.delete_all + end + + context 'when BUFFER_ROWS is exceeded' do + before do + stub_const("#{described_class}::BUFFER_ROWS", 1) + end + + it 'updates and continues' do + expect(described_class::MergeRequestDiff).to receive(:transaction).twice + + subject.perform(start_id, stop_id) + end + end + + context 'when BUFFER_ROWS is not exceeded' do + it 'only updates once' do + expect(described_class::MergeRequestDiff).to receive(:transaction).once + + subject.perform(start_id, stop_id) + end + end + end + + context 'when the merge request diff update fails' do + before do + allow(described_class::MergeRequestDiff) + .to receive(:update_all).and_raise(ActiveRecord::Rollback) + end + + it 'does not add any diff commits' do + expect { subject.perform(merge_request_diff.id, merge_request_diff.id) } + .not_to change { MergeRequestDiffCommit.count } + end + + it 'does not add any diff files' do + expect { subject.perform(merge_request_diff.id, merge_request_diff.id) } + .not_to change { MergeRequestDiffFile.count } + end + end + + context 'when the merge request diff has valid commits and diffs' do + let(:commits) { merge_request_diff.commits.map(&:to_hash) } + let(:diffs) { diffs_to_hashes(merge_request_diff.merge_request_diff_files) } + let(:expected_diffs) { diffs } + + include_examples 'updated MR diff' + end + + context 'when the merge request diffs have binary content' do + let(:commits) { merge_request_diff.commits.map(&:to_hash) } + let(:expected_diffs) { diffs } + + # The start of a PDF created by Illustrator + let(:binary_string) do + "\x25\x50\x44\x46\x2d\x31\x2e\x35\x0d\x25\xe2\xe3\xcf\xd3\x0d\x0a".force_encoding(Encoding::BINARY) + end + + let(:diffs) do + [ + { + 'diff' => binary_string, + 'new_path' => 'path', + 'old_path' => 'path', + 'a_mode' => '100644', + 'b_mode' => '100644', + 'new_file' => false, + 'renamed_file' => false, + 'deleted_file' => false, + 'too_large' => false + } + ] + end + + include_examples 'updated MR diff' + end + + context 'when the merge request diff has commits, but no diffs' do + let(:commits) { merge_request_diff.commits.map(&:to_hash) } + let(:diffs) { [] } + let(:expected_diffs) { diffs } + + include_examples 'updated MR diff' + end + + context 'when the merge request diffs have invalid content' do + let(:commits) { merge_request_diff.commits.map(&:to_hash) } + let(:diffs) { ['--broken-diff'] } + let(:expected_diffs) { [] } + + include_examples 'updated MR diff' + end + + context 'when the merge request diffs are Rugged::Patch instances' do + let(:commits) { merge_request_diff.commits.map(&:to_hash) } + let(:first_commit) { merge_request.project.repository.commit(merge_request_diff.head_commit_sha) } + let(:diffs) { first_commit.diff_from_parent.patches } + let(:expected_diffs) { [] } + + include_examples 'updated MR diff' + end + + context 'when the merge request diffs are Rugged::Diff::Delta instances' do + let(:commits) { merge_request_diff.commits.map(&:to_hash) } + let(:first_commit) { merge_request.project.repository.commit(merge_request_diff.head_commit_sha) } + let(:diffs) { first_commit.diff_from_parent.deltas } + let(:expected_diffs) { [] } + + include_examples 'updated MR diff' + end + end +end diff --git a/spec/lib/gitlab/git/blame_spec.rb b/spec/lib/gitlab/git/blame_spec.rb index 66c016d14b3..800c245b130 100644 --- a/spec/lib/gitlab/git/blame_spec.rb +++ b/spec/lib/gitlab/git/blame_spec.rb @@ -7,63 +7,73 @@ describe Gitlab::Git::Blame, seed_helper: true do Gitlab::Git::Blame.new(repository, SeedRepo::Commit::ID, "CONTRIBUTING.md") end - context "each count" do - it do - data = [] - blame.each do |commit, line| - data << { - commit: commit, - line: line - } - end - - expect(data.size).to eq(95) - expect(data.first[:commit]).to be_kind_of(Gitlab::Git::Commit) - expect(data.first[:line]).to eq("# Contribute to GitLab") - expect(data.first[:line]).to be_utf8 - end - end + shared_examples 'blaming a file' do + context "each count" do + it do + data = [] + blame.each do |commit, line| + data << { + commit: commit, + line: line + } + end - context "ISO-8859 encoding" do - let(:blame) do - Gitlab::Git::Blame.new(repository, SeedRepo::EncodingCommit::ID, "encoding/iso8859.txt") + expect(data.size).to eq(95) + expect(data.first[:commit]).to be_kind_of(Gitlab::Git::Commit) + expect(data.first[:line]).to eq("# Contribute to GitLab") + expect(data.first[:line]).to be_utf8 + end end - it 'converts to UTF-8' do - data = [] - blame.each do |commit, line| - data << { - commit: commit, - line: line - } + context "ISO-8859 encoding" do + let(:blame) do + Gitlab::Git::Blame.new(repository, SeedRepo::EncodingCommit::ID, "encoding/iso8859.txt") end - expect(data.size).to eq(1) - expect(data.first[:commit]).to be_kind_of(Gitlab::Git::Commit) - expect(data.first[:line]).to eq("Ä ü") - expect(data.first[:line]).to be_utf8 - end - end + it 'converts to UTF-8' do + data = [] + blame.each do |commit, line| + data << { + commit: commit, + line: line + } + end - context "unknown encoding" do - let(:blame) do - Gitlab::Git::Blame.new(repository, SeedRepo::EncodingCommit::ID, "encoding/iso8859.txt") + expect(data.size).to eq(1) + expect(data.first[:commit]).to be_kind_of(Gitlab::Git::Commit) + expect(data.first[:line]).to eq("Ä ü") + expect(data.first[:line]).to be_utf8 + end end - it 'converts to UTF-8' do - expect(CharlockHolmes::EncodingDetector).to receive(:detect).and_return(nil) - data = [] - blame.each do |commit, line| - data << { + context "unknown encoding" do + let(:blame) do + Gitlab::Git::Blame.new(repository, SeedRepo::EncodingCommit::ID, "encoding/iso8859.txt") + end + + it 'converts to UTF-8' do + expect(CharlockHolmes::EncodingDetector).to receive(:detect).and_return(nil) + data = [] + blame.each do |commit, line| + data << { commit: commit, line: line - } - end + } + end - expect(data.size).to eq(1) - expect(data.first[:commit]).to be_kind_of(Gitlab::Git::Commit) - expect(data.first[:line]).to eq(" ") - expect(data.first[:line]).to be_utf8 + expect(data.size).to eq(1) + expect(data.first[:commit]).to be_kind_of(Gitlab::Git::Commit) + expect(data.first[:line]).to eq(" ") + expect(data.first[:line]).to be_utf8 + end end end + + context 'when Gitaly blame feature is enabled' do + it_behaves_like 'blaming a file' + end + + context 'when Gitaly blame feature is disabled', skip_gitaly_mock: true do + it_behaves_like 'blaming a file' + end end diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 8e4a1f31ced..9bfad0c9bdf 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -361,20 +361,20 @@ describe Gitlab::Git::Repository, seed_helper: true do end describe '#commit_count' do - shared_examples 'counting commits' do + shared_examples 'simple commit counting' do it { expect(repository.commit_count("master")).to eq(25) } it { expect(repository.commit_count("feature")).to eq(9) } end context 'when Gitaly commit_count feature is enabled' do - it_behaves_like 'counting commits' + it_behaves_like 'simple commit counting' it_behaves_like 'wrapping gRPC errors', Gitlab::GitalyClient::CommitService, :commit_count do subject { repository.commit_count('master') } end end context 'when Gitaly commit_count feature is disabled', skip_gitaly_mock: true do - it_behaves_like 'counting commits' + it_behaves_like 'simple commit counting' end end @@ -797,29 +797,39 @@ describe Gitlab::Git::Repository, seed_helper: true do end describe '#count_commits' do - context 'with after timestamp' do - it 'returns the number of commits after timestamp' do - options = { ref: 'master', limit: nil, after: Time.iso8601('2013-03-03T20:15:01+00:00') } + shared_examples 'extended commit counting' do + context 'with after timestamp' do + it 'returns the number of commits after timestamp' do + options = { ref: 'master', limit: nil, after: Time.iso8601('2013-03-03T20:15:01+00:00') } - expect(repository.count_commits(options)).to eq(25) + expect(repository.count_commits(options)).to eq(25) + end end - end - context 'with before timestamp' do - it 'returns the number of commits after timestamp' do - options = { ref: 'feature', limit: nil, before: Time.iso8601('2015-03-03T20:15:01+00:00') } + context 'with before timestamp' do + it 'returns the number of commits before timestamp' do + options = { ref: 'feature', limit: nil, before: Time.iso8601('2015-03-03T20:15:01+00:00') } - expect(repository.count_commits(options)).to eq(9) + expect(repository.count_commits(options)).to eq(9) + end end - end - context 'with path' do - it 'returns the number of commits with path ' do - options = { ref: 'master', limit: nil, path: "encoding" } + context 'with path' do + it 'returns the number of commits with path ' do + options = { ref: 'master', limit: nil, path: "encoding" } - expect(repository.count_commits(options)).to eq(2) + expect(repository.count_commits(options)).to eq(2) + end end end + + context 'when Gitaly count_commits feature is enabled' do + it_behaves_like 'extended commit counting' + end + + context 'when Gitaly count_commits feature is disabled', skip_gitaly_mock: true do + it_behaves_like 'extended commit counting' + end end describe "branch_names_contains" do diff --git a/spec/migrations/schedule_merge_request_diff_migrations_spec.rb b/spec/migrations/schedule_merge_request_diff_migrations_spec.rb new file mode 100644 index 00000000000..f95bd6e3511 --- /dev/null +++ b/spec/migrations/schedule_merge_request_diff_migrations_spec.rb @@ -0,0 +1,59 @@ +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20170703130158_schedule_merge_request_diff_migrations') + +describe ScheduleMergeRequestDiffMigrations, :migration, :sidekiq do + matcher :be_scheduled_migration do |time, *expected| + match do |migration| + BackgroundMigrationWorker.jobs.any? do |job| + job['args'] == [migration, expected] && + job['at'].to_i == time.to_i + end + end + + failure_message do |migration| + "Migration `#{migration}` with args `#{expected.inspect}` not scheduled!" + end + end + + let(:merge_request_diffs) { table(:merge_request_diffs) } + let(:merge_requests) { table(:merge_requests) } + let(:projects) { table(:projects) } + + before do + stub_const("#{described_class.name}::BATCH_SIZE", 1) + + projects.create!(id: 1, name: 'gitlab', path: 'gitlab') + + merge_requests.create!(id: 1, target_project_id: 1, source_project_id: 1, target_branch: 'feature', source_branch: 'master') + + merge_request_diffs.create!(id: 1, merge_request_id: 1, st_commits: YAML.dump([]), st_diffs: nil) + merge_request_diffs.create!(id: 2, merge_request_id: 1, st_commits: nil, st_diffs: YAML.dump([])) + merge_request_diffs.create!(id: 3, merge_request_id: 1, st_commits: nil, st_diffs: nil) + merge_request_diffs.create!(id: 4, merge_request_id: 1, st_commits: YAML.dump([]), st_diffs: YAML.dump([])) + end + + it 'correctly schedules background migrations' do + Sidekiq::Testing.fake! do + Timecop.freeze do + migrate! + + expect(described_class::MIGRATION).to be_scheduled_migration(5.minutes.from_now, 1, 1) + expect(described_class::MIGRATION).to be_scheduled_migration(10.minutes.from_now, 2, 2) + expect(described_class::MIGRATION).to be_scheduled_migration(15.minutes.from_now, 4, 4) + expect(BackgroundMigrationWorker.jobs.size).to eq 3 + end + end + end + + it 'schedules background migrations' do + Sidekiq::Testing.inline! do + non_empty = 'st_commits IS NOT NULL OR st_diffs IS NOT NULL' + + expect(merge_request_diffs.where(non_empty).count).to eq 3 + + migrate! + + expect(merge_request_diffs.where(non_empty).count).to eq 0 + end + end +end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index f63ff19c2fc..ac75c6501ee 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -1,8 +1,6 @@ require 'spec_helper' -describe Ci::Pipeline do - include EmailHelpers - +describe Ci::Pipeline, :mailer do let(:user) { create(:user) } let(:project) { create(:project) } @@ -1248,8 +1246,6 @@ describe Ci::Pipeline do pipeline.user.global_notification_setting .update(level: 'custom', failed_pipeline: true, success_pipeline: true) - reset_delivered_emails! - perform_enqueued_jobs do pipeline.enqueue pipeline.run diff --git a/spec/models/deploy_key_spec.rb b/spec/models/deploy_key_spec.rb index 2aece75b817..3d7283e2164 100644 --- a/spec/models/deploy_key_spec.rb +++ b/spec/models/deploy_key_spec.rb @@ -1,8 +1,6 @@ require 'spec_helper' -describe DeployKey do - include EmailHelpers - +describe DeployKey, :mailer do describe "Associations" do it { is_expected.to have_many(:deploy_keys_projects) } it { is_expected.to have_many(:projects) } diff --git a/spec/models/gpg_key_spec.rb b/spec/models/gpg_key_spec.rb index 59c074199db..e48f20bf53b 100644 --- a/spec/models/gpg_key_spec.rb +++ b/spec/models/gpg_key_spec.rb @@ -114,9 +114,7 @@ describe GpgKey do end end - describe 'notification' do - include EmailHelpers - + describe 'notification', :mailer do let(:user) { create(:user) } it 'sends a notification' do diff --git a/spec/models/key_spec.rb b/spec/models/key_spec.rb index d41717d0223..0daeb337168 100644 --- a/spec/models/key_spec.rb +++ b/spec/models/key_spec.rb @@ -1,8 +1,6 @@ require 'spec_helper' -describe Key do - include EmailHelpers - +describe Key, :mailer do describe "Associations" do it { is_expected.to belong_to(:user) } end @@ -94,15 +92,17 @@ describe Key do expect(key).not_to be_valid end - it 'rejects the unfingerprintable key (not a key)' do - expect(build(:key, key: 'ssh-rsa an-invalid-key==')).not_to be_valid + it 'accepts a key with newline charecters after stripping them' do + key = build(:key) + key.key = key.key.insert(100, "\n") + key.key = key.key.insert(40, "\r\n") + expect(key).to be_valid end - it 'rejects the multiple line key' do - key = build(:key) - key.key.tr!(' ', "\n") - expect(key).not_to be_valid + it 'rejects the unfingerprintable key (not a key)' do + expect(build(:key, key: 'ssh-rsa an-invalid-key==')).not_to be_valid end + end context 'callbacks' do diff --git a/spec/models/project_services/jira_service_spec.rb b/spec/models/project_services/jira_service_spec.rb index 204a00778a7..63bf131cfc5 100644 --- a/spec/models/project_services/jira_service_spec.rb +++ b/spec/models/project_services/jira_service_spec.rb @@ -153,6 +153,15 @@ describe JiraService do expect(WebMock).not_to have_requested(:post, @remote_link_url) end + it "does not send comment or remote links to issues with unknown resolution" do + allow_any_instance_of(JIRA::Resource::Issue).to receive(:respond_to?).with(:resolution).and_return(false) + + @jira_service.close_issue(merge_request, ExternalIssue.new("JIRA-123", project)) + + expect(WebMock).not_to have_requested(:post, @comment_url) + expect(WebMock).not_to have_requested(:post, @remote_link_url) + end + it "references the GitLab commit/merge request" do stub_config_setting(base_url: custom_base_url) diff --git a/spec/models/project_services/pipelines_email_service_spec.rb b/spec/models/project_services/pipelines_email_service_spec.rb index 03932895b0e..5faab9ba38b 100644 --- a/spec/models/project_services/pipelines_email_service_spec.rb +++ b/spec/models/project_services/pipelines_email_service_spec.rb @@ -1,8 +1,6 @@ require 'spec_helper' -describe PipelinesEmailService do - include EmailHelpers - +describe PipelinesEmailService, :mailer do let(:pipeline) do create(:ci_pipeline, project: project, sha: project.commit('master').sha) end @@ -14,10 +12,6 @@ describe PipelinesEmailService do Gitlab::DataBuilder::Pipeline.build(pipeline) end - before do - reset_delivered_emails! - end - describe 'Validations' do context 'when service is active' do before do diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 764f548be45..f876baaa805 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -139,24 +139,44 @@ describe Repository do end describe '#last_commit_for_path' do - subject { repository.last_commit_for_path(sample_commit.id, '.gitignore').id } + shared_examples 'getting last commit for path' do + subject { repository.last_commit_for_path(sample_commit.id, '.gitignore').id } - it { is_expected.to eq('c1acaa58bbcbc3eafe538cb8274ba387047b69f8') } + it { is_expected.to eq('c1acaa58bbcbc3eafe538cb8274ba387047b69f8') } + end + + context 'when Gitaly feature last_commit_for_path is enabled' do + it_behaves_like 'getting last commit for path' + end + + context 'when Gitaly feature last_commit_for_path is disabled', skip_gitaly_mock: true do + it_behaves_like 'getting last commit for path' + end end describe '#last_commit_id_for_path' do - subject { repository.last_commit_id_for_path(sample_commit.id, '.gitignore') } + shared_examples 'getting last commit ID for path' do + subject { repository.last_commit_id_for_path(sample_commit.id, '.gitignore') } - it "returns last commit id for a given path" do - is_expected.to eq('c1acaa58bbcbc3eafe538cb8274ba387047b69f8') + it "returns last commit id for a given path" do + is_expected.to eq('c1acaa58bbcbc3eafe538cb8274ba387047b69f8') + end + + it "caches last commit id for a given path" do + cache = repository.send(:cache) + key = "last_commit_id_for_path:#{sample_commit.id}:#{Digest::SHA1.hexdigest('.gitignore')}" + + expect(cache).to receive(:fetch).with(key).and_return('c1acaa5') + is_expected.to eq('c1acaa5') + end end - it "caches last commit id for a given path" do - cache = repository.send(:cache) - key = "last_commit_id_for_path:#{sample_commit.id}:#{Digest::SHA1.hexdigest('.gitignore')}" + context 'when Gitaly feature last_commit_for_path is enabled' do + it_behaves_like 'getting last commit ID for path' + end - expect(cache).to receive(:fetch).with(key).and_return('c1acaa5') - is_expected.to eq('c1acaa5') + context 'when Gitaly feature last_commit_for_path is disabled', skip_gitaly_mock: true do + it_behaves_like 'getting last commit ID for path' end end diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb index 60687db9316..7d120e4a234 100644 --- a/spec/requests/api/issues_spec.rb +++ b/spec/requests/api/issues_spec.rb @@ -1,8 +1,6 @@ require 'spec_helper' -describe API::Issues do - include EmailHelpers - +describe API::Issues, :mailer do set(:user) { create(:user) } set(:project) do create(:project, :public, creator_id: user.id, namespace: user.namespace) diff --git a/spec/requests/api/v3/issues_spec.rb b/spec/requests/api/v3/issues_spec.rb index b092c863c8a..9eb538c4b09 100644 --- a/spec/requests/api/v3/issues_spec.rb +++ b/spec/requests/api/v3/issues_spec.rb @@ -1,8 +1,6 @@ require 'spec_helper' -describe API::V3::Issues do - include EmailHelpers - +describe API::V3::Issues, :mailer do let(:user) { create(:user) } let(:user2) { create(:user) } let(:non_member) { create(:user) } diff --git a/spec/routing/project_routing_spec.rb b/spec/routing/project_routing_spec.rb index c02409b2e0b..39d44245c3f 100644 --- a/spec/routing/project_routing_spec.rb +++ b/spec/routing/project_routing_spec.rb @@ -165,15 +165,19 @@ describe 'project routing' do # edit_project_repository GET /:project_id/repository/edit(.:format) projects/repositories#edit describe Projects::RepositoriesController, 'routing' do it 'to #archive' do - expect(get('/gitlab/gitlabhq/repository/archive')).to route_to('projects/repositories#archive', namespace_id: 'gitlab', project_id: 'gitlabhq') + expect(get('/gitlab/gitlabhq/repository/master/archive')).to route_to('projects/repositories#archive', namespace_id: 'gitlab', project_id: 'gitlabhq', ref: 'master') end it 'to #archive format:zip' do - expect(get('/gitlab/gitlabhq/repository/archive.zip')).to route_to('projects/repositories#archive', namespace_id: 'gitlab', project_id: 'gitlabhq', format: 'zip') + expect(get('/gitlab/gitlabhq/repository/master/archive.zip')).to route_to('projects/repositories#archive', namespace_id: 'gitlab', project_id: 'gitlabhq', format: 'zip', ref: 'master') end it 'to #archive format:tar.bz2' do - expect(get('/gitlab/gitlabhq/repository/archive.tar.bz2')).to route_to('projects/repositories#archive', namespace_id: 'gitlab', project_id: 'gitlabhq', format: 'tar.bz2') + expect(get('/gitlab/gitlabhq/repository/master/archive.tar.bz2')).to route_to('projects/repositories#archive', namespace_id: 'gitlab', project_id: 'gitlabhq', format: 'tar.bz2', ref: 'master') + end + + it 'to #archive with "/" in route' do + expect(get('/gitlab/gitlabhq/repository/improve/awesome/archive')).to route_to('projects/repositories#archive', namespace_id: 'gitlab', project_id: 'gitlabhq', ref: 'improve/awesome') end end diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index ff0d876e6da..814e2cfbed0 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -1,9 +1,7 @@ # coding: utf-8 require 'spec_helper' -describe Issues::UpdateService do - include EmailHelpers - +describe Issues::UpdateService, :mailer do let(:user) { create(:user) } let(:user2) { create(:user) } let(:user3) { create(:user) } diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index dd3ac9c4ac6..9368594bc86 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -1,8 +1,6 @@ require 'spec_helper' -describe MergeRequests::UpdateService do - include EmailHelpers - +describe MergeRequests::UpdateService, :mailer do let(:project) { create(:project, :repository) } let(:user) { create(:user) } let(:user2) { create(:user) } diff --git a/spec/services/notification_recipient_service_spec.rb b/spec/services/notification_recipient_service_spec.rb deleted file mode 100644 index 0eb0771fd29..00000000000 --- a/spec/services/notification_recipient_service_spec.rb +++ /dev/null @@ -1,34 +0,0 @@ -require 'spec_helper' - -describe NotificationRecipientService do - set(:user) { create(:user) } - set(:project) { create(:project, :public) } - set(:issue) { create(:issue, project: project) } - - set(:watcher) do - watcher = create(:user) - setting = watcher.notification_settings_for(project) - setting.level = :watch - setting.save - - watcher - end - - subject { described_class.new(project) } - - describe '#build_recipients' do - it 'does not modify the participants of the target' do - expect { subject.build_recipients(issue, user, action: :new_issue) } - .not_to change { issue.participants(user) } - end - end - - describe '#build_new_note_recipients' do - set(:note) { create(:note_on_issue, noteable: issue, project: project) } - - it 'does not modify the participants of the target' do - expect { subject.build_new_note_recipients(note) } - .not_to change { note.noteable.participants(note.author) } - end - end -end diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 882ee7751b5..6af5c79135d 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -1,8 +1,6 @@ require 'spec_helper' -describe NotificationService do - include EmailHelpers - +describe NotificationService, :mailer do let(:notification) { described_class.new } let(:assignee) { create(:user) } @@ -14,7 +12,6 @@ describe NotificationService do shared_examples 'notifications for new mentions' do def send_notifications(*new_mentions) - reset_delivered_emails! notification.send(notification_method, mentionable, new_mentions, @u_disabled) end @@ -137,12 +134,11 @@ describe NotificationService do describe '#new_note' do it do add_users_with_subscription(note.project, issue) + reset_delivered_emails! # Ensure create SentNotification by noteable = issue 6 times, not noteable = note expect(SentNotification).to receive(:record).with(issue, any_args).exactly(8).times - reset_delivered_emails! - notification.new_note(note) should_email(@u_watcher) @@ -165,9 +161,10 @@ describe NotificationService do it "emails the note author if they've opted into notifications about their activity" do add_users_with_subscription(note.project, issue) - note.author.notified_of_own_activity = true reset_delivered_emails! + note.author.notified_of_own_activity = true + notification.new_note(note) should_email(note.author) @@ -309,6 +306,11 @@ describe NotificationService do before do build_team(note.project) + + # make sure these users can read the project snippet! + project.add_guest(@u_guest_watcher) + project.add_guest(@u_guest_custom) + note.project.add_master(note.author) reset_delivered_emails! end diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb index 534d3e65be2..80d05451e09 100644 --- a/spec/services/todo_service_spec.rb +++ b/spec/services/todo_service_spec.rb @@ -336,7 +336,7 @@ describe TodoService do describe '#mark_todos_as_done' do it_behaves_like 'updating todos state', :mark_todos_as_done, :pending, :done do - let(:collection) { [first_todo, second_todo] } + let(:collection) { Todo.all } end end @@ -348,7 +348,7 @@ describe TodoService do describe '#mark_todos_as_pending' do it_behaves_like 'updating todos state', :mark_todos_as_pending, :done, :pending do - let(:collection) { [first_todo, second_todo] } + let(:collection) { Todo.all } end end @@ -880,14 +880,16 @@ describe TodoService do it 'marks an array of todos as done' do todo = create(:todo, :mentioned, user: john_doe, target: issue, project: project) - expect { described_class.new.mark_todos_as_done([todo], john_doe) } + todos = TodosFinder.new(john_doe, {}).execute + expect { described_class.new.mark_todos_as_done(todos, john_doe) } .to change { todo.reload.state }.from('pending').to('done') end it 'returns the ids of updated todos' do # Needed on API todo = create(:todo, :mentioned, user: john_doe, target: issue, project: project) - expect(described_class.new.mark_todos_as_done([todo], john_doe)).to eq([todo.id]) + todos = TodosFinder.new(john_doe, {}).execute + expect(described_class.new.mark_todos_as_done(todos, john_doe)).to eq([todo.id]) end context 'when some of the todos are done already' do @@ -907,11 +909,32 @@ describe TodoService do expect(described_class.new.mark_todos_as_done(Todo.all, john_doe)).to eq([]) end end + end + + describe '#mark_todos_as_done_by_ids' do + let(:issue) { create(:issue, project: project, author: author, assignees: [john_doe]) } + let(:another_issue) { create(:issue, project: project, author: author, assignees: [john_doe]) } + + it 'marks an array of todo ids as done' do + todo = create(:todo, :mentioned, user: john_doe, target: issue, project: project) + another_todo = create(:todo, :mentioned, user: john_doe, target: another_issue, project: project) + + expect { described_class.new.mark_todos_as_done_by_ids([todo.id, another_todo.id], john_doe) } + .to change { john_doe.todos.done.count }.from(0).to(2) + end + + it 'marks a single todo id as done' do + todo = create(:todo, :mentioned, user: john_doe, target: issue, project: project) + + expect { described_class.new.mark_todos_as_done_by_ids(todo.id, john_doe) } + .to change { todo.reload.state }.from('pending').to('done') + end it 'caches the number of todos of a user', :use_clean_rails_memory_store_caching do create(:todo, :mentioned, user: john_doe, target: issue, project: project) todo = create(:todo, :mentioned, user: john_doe, target: issue, project: project) - described_class.new.mark_todos_as_done([todo], john_doe) + + described_class.new.mark_todos_as_done_by_ids(todo, john_doe) expect_any_instance_of(TodosFinder).not_to receive(:execute) diff --git a/spec/support/api/schema_matcher.rb b/spec/support/api/schema_matcher.rb index 67599f77adb..6591d56e473 100644 --- a/spec/support/api/schema_matcher.rb +++ b/spec/support/api/schema_matcher.rb @@ -1,23 +1,25 @@ -def schema_path(schema) - schema_directory = "#{Dir.pwd}/spec/fixtures/api/schemas" - "#{schema_directory}/#{schema}.json" +module SchemaPath + def self.expand(schema, dir = '') + Rails.root.join('spec', dir, "fixtures/api/schemas/#{schema}.json").to_s + end end -RSpec::Matchers.define :match_response_schema do |schema, **options| +RSpec::Matchers.define :match_response_schema do |schema, dir: '', **options| match do |response| - @errors = JSON::Validator.fully_validate(schema_path(schema), response.body, options) + @errors = JSON::Validator.fully_validate( + SchemaPath.expand(schema, dir), response.body, options) @errors.empty? end failure_message do |response| - "didn't match the schema defined by #{schema_path(schema)}" \ + "didn't match the schema defined by #{SchemaPath.expand(schema, dir)}" \ " The validation errors were:\n#{@errors.join("\n")}" end end -RSpec::Matchers.define :match_schema do |schema, **options| +RSpec::Matchers.define :match_schema do |schema, dir: '', **options| match do |data| - JSON::Validator.validate!(schema_path(schema), data, options) + JSON::Validator.validate!(SchemaPath.expand(schema, dir), data, options) end end diff --git a/spec/support/login_helpers.rb b/spec/support/login_helpers.rb index c714d1b08a6..3e117530151 100644 --- a/spec/support/login_helpers.rb +++ b/spec/support/login_helpers.rb @@ -1,3 +1,5 @@ +require_relative 'devise_helpers' + module LoginHelpers include DeviseHelpers diff --git a/spec/support/notify_shared_examples.rb b/spec/support/notify_shared_examples.rb index d6117d604f2..136f92c6419 100644 --- a/spec/support/notify_shared_examples.rb +++ b/spec/support/notify_shared_examples.rb @@ -7,7 +7,6 @@ shared_context 'gitlab email notification' do let(:new_user_address) { 'newguy@example.com' } before do - reset_delivered_emails! email = recipient.emails.create(email: "notifications@example.com") recipient.update_attribute(:notification_email, email.email) stub_incoming_email_setting(enabled: true, address: "reply+%{key}@#{Gitlab.config.gitlab.host}") diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb index bed78928f14..c1298ed9cae 100644 --- a/spec/support/test_env.rb +++ b/spec/support/test_env.rb @@ -229,7 +229,6 @@ module TestEnv # Otherwise they'd be created by the first test, often timing out and # causing a transient test failure def eager_load_driver_server - return unless ENV['CI'] return unless defined?(Capybara) puts "Starting the Capybara driver server..." diff --git a/spec/support/updating_mentions_shared_examples.rb b/spec/support/updating_mentions_shared_examples.rb index eeec3e1d79b..565d3203e4f 100644 --- a/spec/support/updating_mentions_shared_examples.rb +++ b/spec/support/updating_mentions_shared_examples.rb @@ -7,8 +7,6 @@ RSpec.shared_examples 'updating mentions' do |service_class| end def update_mentionable(opts) - reset_delivered_emails! - perform_enqueued_jobs do service_class.new(project, user, opts).execute(mentionable) end diff --git a/spec/workers/emails_on_push_worker_spec.rb b/spec/workers/emails_on_push_worker_spec.rb index 5b6b38e0f76..318aad4bc1e 100644 --- a/spec/workers/emails_on_push_worker_spec.rb +++ b/spec/workers/emails_on_push_worker_spec.rb @@ -1,8 +1,7 @@ require 'spec_helper' -describe EmailsOnPushWorker do +describe EmailsOnPushWorker, :mailer do include RepoHelpers - include EmailHelpers include EmailSpec::Matchers let(:project) { create(:project, :repository) } @@ -90,7 +89,6 @@ describe EmailsOnPushWorker do context "when there is an SMTP error" do before do - reset_delivered_emails! allow(Notify).to receive(:repository_push_email).and_raise(Net::SMTPFatalError) allow(subject).to receive_message_chain(:logger, :info) perform @@ -114,8 +112,6 @@ describe EmailsOnPushWorker do allow_any_instance_of(Mail::TestMailer).to receive(:deliver!).and_wrap_original do |original, mail| original.call(Mail.new(mail.encoded)) end - - reset_delivered_emails! end it "sends the mail to each of the recipients" do diff --git a/spec/workers/pipeline_notification_worker_spec.rb b/spec/workers/pipeline_notification_worker_spec.rb index 139032d77bd..eb539ffd893 100644 --- a/spec/workers/pipeline_notification_worker_spec.rb +++ b/spec/workers/pipeline_notification_worker_spec.rb @@ -1,8 +1,6 @@ require 'spec_helper' -describe PipelineNotificationWorker do - include EmailHelpers - +describe PipelineNotificationWorker, :mailer do let(:pipeline) { create(:ci_pipeline) } describe '#execute' do |
