diff options
139 files changed, 2378 insertions, 895 deletions
diff --git a/.eslintignore b/.eslintignore index d6ce39636bd..1623b996213 100644 --- a/.eslintignore +++ b/.eslintignore @@ -8,4 +8,4 @@ karma.config.js webpack.config.js svg.config.js -/app/assets/javascripts/locale/**/*.js +/app/assets/javascripts/locale/**/app.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c671f8d53a..d08e42b3b65 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,13 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 10.0.2 (2017-09-27) + +- [FIXED] Notes will not show an empty bubble when the author isn't a member. !14450 +- [FIXED] Some checks in `rake gitlab:check` were failling with 'undefined method `run_command`'. !14469 +- [FIXED] Make locked setting of Runner to not affect jobs scheduling. !14483 +- [FIXED] Re-allow `name` attribute on user-provided anchor HTML. + ## 10.0.1 (2017-09-23) - [FIXED] Fix duplicate key errors in PostDeployMigrateUserExternalMailData migration. diff --git a/app/assets/javascripts/boards/boards_bundle.js b/app/assets/javascripts/boards/boards_bundle.js index ea00efe4b46..815248f38ee 100644 --- a/app/assets/javascripts/boards/boards_bundle.js +++ b/app/assets/javascripts/boards/boards_bundle.js @@ -77,9 +77,6 @@ $(() => { }); Store.rootPath = this.boardsEndpoint; - this.filterManager = new FilteredSearchBoards(Store.filter, true); - this.filterManager.setup(); - // Listen for updateTokens event eventHub.$on('updateTokens', this.updateTokens); }, @@ -87,6 +84,9 @@ $(() => { eventHub.$off('updateTokens', this.updateTokens); }, mounted () { + this.filterManager = new FilteredSearchBoards(Store.filter, true); + this.filterManager.setup(); + Store.disabled = this.disabled; gl.boardService.all() .then(response => response.json()) diff --git a/app/assets/javascripts/files_comment_button.js b/app/assets/javascripts/files_comment_button.js index d02e4cd5876..a00d29a845a 100644 --- a/app/assets/javascripts/files_comment_button.js +++ b/app/assets/javascripts/files_comment_button.js @@ -7,6 +7,8 @@ * causes reflows, visit https://gist.github.com/paulirish/5d52fb081b3570c81e3a */ +import Cookies from 'js-cookie'; + const LINE_NUMBER_CLASS = 'diff-line-num'; const UNFOLDABLE_LINE_CLASS = 'js-unfold'; const NO_COMMENT_CLASS = 'no-comment-btn'; @@ -27,9 +29,7 @@ export default { this.userCanCreateNote = $diffFile.closest(DIFF_CONTAINER_SELECTOR).data('can-create-note') === ''; } - if (typeof notes !== 'undefined' && !this.isParallelView) { - this.isParallelView = notes.isParallelView && notes.isParallelView(); - } + this.isParallelView = Cookies.get('diff_view') === 'parallel'; if (this.userCanCreateNote) { $diffFile.on('mouseover', LINE_COLUMN_CLASSES, e => this.showButton(this.isParallelView, e)) diff --git a/app/assets/javascripts/line_highlighter.js b/app/assets/javascripts/line_highlighter.js index 7400c22543f..a16d00b5cef 100644 --- a/app/assets/javascripts/line_highlighter.js +++ b/app/assets/javascripts/line_highlighter.js @@ -28,148 +28,149 @@ // </div> // </div> // -(function() { - this.LineHighlighter = (function() { - // CSS class applied to highlighted lines - LineHighlighter.prototype.highlightClass = 'hll'; - - // Internal copy of location.hash so we're not dependent on `location` in tests - LineHighlighter.prototype._hash = ''; - - function LineHighlighter(hash) { - if (hash == null) { - // Initialize a LineHighlighter object - // - // hash - String URL hash for dependency injection in tests - hash = location.hash; - } - this.setHash = this.setHash.bind(this); - this.highlightLine = this.highlightLine.bind(this); - this.clickHandler = this.clickHandler.bind(this); - this.highlightHash = this.highlightHash.bind(this); - this._hash = hash; - this.bindEvents(); - this.highlightHash(); - } - LineHighlighter.prototype.bindEvents = function() { - const $fileHolder = $('.file-holder'); - $fileHolder.on('click', 'a[data-line-number]', this.clickHandler); - $fileHolder.on('highlight:line', this.highlightHash); - }; - - LineHighlighter.prototype.highlightHash = function() { - var range; - if (this._hash !== '') { - range = this.hashToRange(this._hash); - if (range[0]) { - this.highlightRange(range); - $.scrollTo("#L" + range[0], { - // Scroll to the first highlighted line on initial load - // Offset -50 for the sticky top bar, and another -100 for some context - offset: -150 - }); - } - } - }; - - LineHighlighter.prototype.clickHandler = function(event) { - var current, lineNumber, range; - event.preventDefault(); - this.clearHighlight(); - lineNumber = $(event.target).closest('a').data('line-number'); - current = this.hashToRange(this._hash); - if (!(current[0] && event.shiftKey)) { - // If there's no current selection, or there is but Shift wasn't held, - // treat this like a single-line selection. - this.setHash(lineNumber); - return this.highlightLine(lineNumber); - } else if (event.shiftKey) { - if (lineNumber < current[0]) { - range = [lineNumber, current[0]]; - } else { - range = [current[0], lineNumber]; - } - this.setHash(range[0], range[1]); - return this.highlightRange(range); - } - }; - - LineHighlighter.prototype.clearHighlight = function() { - return $("." + this.highlightClass).removeClass(this.highlightClass); - // Unhighlight previously highlighted lines - }; - - // Convert a URL hash String into line numbers - // - // hash - Hash String - // - // Examples: - // - // hashToRange('#L5') # => [5, null] - // hashToRange('#L5-15') # => [5, 15] - // hashToRange('#foo') # => [null, null] - // - // Returns an Array - LineHighlighter.prototype.hashToRange = function(hash) { - var first, last, matches; - // ?L(\d+)(?:-(\d+))?$/) - matches = hash.match(/^#?L(\d+)(?:-(\d+))?$/); - if (matches && matches.length) { - first = parseInt(matches[1], 10); - last = matches[2] ? parseInt(matches[2], 10) : null; - return [first, last]; - } else { - return [null, null]; - } - }; - - // Highlight a single line - // - // lineNumber - Line number to highlight - LineHighlighter.prototype.highlightLine = function(lineNumber) { - return $("#LC" + lineNumber).addClass(this.highlightClass); - }; - - // Highlight all lines within a range - // - // range - Array containing the starting and ending line numbers - LineHighlighter.prototype.highlightRange = function(range) { - var i, lineNumber, ref, ref1, results; - if (range[1]) { - results = []; - for (lineNumber = i = ref = range[0], ref1 = range[1]; ref <= ref1 ? i <= ref1 : i >= ref1; lineNumber = ref <= ref1 ? (i += 1) : (i -= 1)) { - results.push(this.highlightLine(lineNumber)); - } - return results; - } else { - return this.highlightLine(range[0]); - } - }; +const LineHighlighter = function(options = {}) { + options.highlightLineClass = options.highlightLineClass || 'hll'; + options.fileHolderSelector = options.fileHolderSelector || '.file-holder'; + options.scrollFileHolder = options.scrollFileHolder || false; + options.hash = options.hash || location.hash; - // Set the URL hash string - LineHighlighter.prototype.setHash = function(firstLineNumber, lastLineNumber) { - var hash; - if (lastLineNumber) { - hash = "#L" + firstLineNumber + "-" + lastLineNumber; + this.options = options; + this._hash = options.hash; + this.highlightLineClass = options.highlightLineClass; + this.setHash = this.setHash.bind(this); + this.highlightLine = this.highlightLine.bind(this); + this.clickHandler = this.clickHandler.bind(this); + this.highlightHash = this.highlightHash.bind(this); + + this.bindEvents(); + this.highlightHash(); +}; + +LineHighlighter.prototype.bindEvents = function() { + const $fileHolder = $(this.options.fileHolderSelector); + + $fileHolder.on('click', 'a[data-line-number]', this.clickHandler); + $fileHolder.on('highlight:line', this.highlightHash); +}; + +LineHighlighter.prototype.highlightHash = function() { + var range; + + if (this._hash !== '') { + range = this.hashToRange(this._hash); + + if (range[0]) { + this.highlightRange(range); + const lineSelector = `#L${range[0]}`; + const scrollOptions = { + // Scroll to the first highlighted line on initial load + // Offset -50 for the sticky top bar, and another -100 for some context + offset: -150 + }; + if (this.options.scrollFileHolder) { + $(this.options.fileHolderSelector).scrollTo(lineSelector, scrollOptions); } else { - hash = "#L" + firstLineNumber; + $.scrollTo(lineSelector, scrollOptions); } - this._hash = hash; - return this.__setLocationHash__(hash); - }; - - // Make the actual hash change in the browser - // - // This method is stubbed in tests. - LineHighlighter.prototype.__setLocationHash__ = function(value) { - return history.pushState({ - url: value - // We're using pushState instead of assigning location.hash directly to - // prevent the page from scrolling on the hashchange event - }, document.title, value); - }; - - return LineHighlighter; - })(); -}).call(window); + } + } +}; + +LineHighlighter.prototype.clickHandler = function(event) { + var current, lineNumber, range; + event.preventDefault(); + this.clearHighlight(); + lineNumber = $(event.target).closest('a').data('line-number'); + current = this.hashToRange(this._hash); + if (!(current[0] && event.shiftKey)) { + // If there's no current selection, or there is but Shift wasn't held, + // treat this like a single-line selection. + this.setHash(lineNumber); + return this.highlightLine(lineNumber); + } else if (event.shiftKey) { + if (lineNumber < current[0]) { + range = [lineNumber, current[0]]; + } else { + range = [current[0], lineNumber]; + } + this.setHash(range[0], range[1]); + return this.highlightRange(range); + } +}; + +LineHighlighter.prototype.clearHighlight = function() { + return $("." + this.highlightLineClass).removeClass(this.highlightLineClass); +}; + +// Convert a URL hash String into line numbers +// +// hash - Hash String +// +// Examples: +// +// hashToRange('#L5') # => [5, null] +// hashToRange('#L5-15') # => [5, 15] +// hashToRange('#foo') # => [null, null] +// +// Returns an Array +LineHighlighter.prototype.hashToRange = function(hash) { + var first, last, matches; + // ?L(\d+)(?:-(\d+))?$/) + matches = hash.match(/^#?L(\d+)(?:-(\d+))?$/); + if (matches && matches.length) { + first = parseInt(matches[1], 10); + last = matches[2] ? parseInt(matches[2], 10) : null; + return [first, last]; + } else { + return [null, null]; + } +}; + +// Highlight a single line +// +// lineNumber - Line number to highlight +LineHighlighter.prototype.highlightLine = function(lineNumber) { + return $("#LC" + lineNumber).addClass(this.highlightLineClass); +}; + +// Highlight all lines within a range +// +// range - Array containing the starting and ending line numbers +LineHighlighter.prototype.highlightRange = function(range) { + var i, lineNumber, ref, ref1, results; + if (range[1]) { + results = []; + for (lineNumber = i = ref = range[0], ref1 = range[1]; ref <= ref1 ? i <= ref1 : i >= ref1; lineNumber = ref <= ref1 ? (i += 1) : (i -= 1)) { + results.push(this.highlightLine(lineNumber)); + } + return results; + } else { + return this.highlightLine(range[0]); + } +}; + +// Set the URL hash string +LineHighlighter.prototype.setHash = function(firstLineNumber, lastLineNumber) { + var hash; + if (lastLineNumber) { + hash = "#L" + firstLineNumber + "-" + lastLineNumber; + } else { + hash = "#L" + firstLineNumber; + } + this._hash = hash; + return this.__setLocationHash__(hash); +}; + +// Make the actual hash change in the browser +// +// This method is stubbed in tests. +LineHighlighter.prototype.__setLocationHash__ = function(value) { + return history.pushState({ + url: value + // We're using pushState instead of assigning location.hash directly to + // prevent the page from scrolling on the hashchange event + }, document.title, value); +}; + +window.LineHighlighter = LineHighlighter; diff --git a/app/assets/javascripts/locale/index.js b/app/assets/javascripts/locale/index.js index 7ba676d6d20..6a5084efeb8 100644 --- a/app/assets/javascripts/locale/index.js +++ b/app/assets/javascripts/locale/index.js @@ -16,9 +16,8 @@ const locales = allLocales.reduce((d, obj) => { return data; }, {}); -let lang = document.querySelector('html').getAttribute('lang') || 'en'; -lang = lang.replace(/-/g, '_'); - +const langAttribute = document.querySelector('html').getAttribute('lang'); +const lang = (langAttribute || 'en').replace(/-/g, '_'); const locale = new Jed(locales[lang]); /** diff --git a/app/assets/javascripts/main.js b/app/assets/javascripts/main.js index ec001b9b31c..3b8e2c5b2f3 100644 --- a/app/assets/javascripts/main.js +++ b/app/assets/javascripts/main.js @@ -302,7 +302,10 @@ $(function () { return $container.remove(); // Commit show suppressed diff }); - $('.navbar-toggle').on('click', () => $('.header-content').toggleClass('menu-expanded')); + $('.navbar-toggle').on('click', () => { + $('.header-content').toggleClass('menu-expanded'); + gl.lazyLoader.loadCheck(); + }); // Show/hide comments on diff $body.on('click', '.js-toggle-diff-comments', function (e) { var $this = $(this); diff --git a/app/assets/javascripts/merge_request_tabs.js b/app/assets/javascripts/merge_request_tabs.js index 8ae127776e8..d3299c15720 100644 --- a/app/assets/javascripts/merge_request_tabs.js +++ b/app/assets/javascripts/merge_request_tabs.js @@ -352,7 +352,7 @@ import { } expandViewContainer() { - const $wrapper = $('.content-wrapper .container-fluid'); + const $wrapper = $('.content-wrapper .container-fluid').not('.breadcrumbs'); if (this.fixedLayoutPref === null) { this.fixedLayoutPref = $wrapper.hasClass('container-limited'); } diff --git a/app/assets/javascripts/profile/gl_crop.js b/app/assets/javascripts/profile/gl_crop.js index 291ae24aa68..4bdda611cfc 100644 --- a/app/assets/javascripts/profile/gl_crop.js +++ b/app/assets/javascripts/profile/gl_crop.js @@ -73,7 +73,8 @@ import _ from 'underscore'; aspectRatio: 1, modal: true, scalable: false, - rotatable: false, + rotatable: true, + checkOrientation: true, zoomable: true, dragMode: 'move', guides: false, diff --git a/app/assets/javascripts/repo/components/repo_preview.vue b/app/assets/javascripts/repo/components/repo_preview.vue index 4705f4004cd..2fe369a4693 100644 --- a/app/assets/javascripts/repo/components/repo_preview.vue +++ b/app/assets/javascripts/repo/components/repo_preview.vue @@ -1,23 +1,27 @@ <script> +/* global LineHighlighter */ + import Store from '../stores/repo_store'; export default { data: () => Store, - mounted() { - this.highlightFile(); - }, computed: { html() { return this.activeFile.html; }, }, - methods: { highlightFile() { $(this.$el).find('.file-content').syntaxHighlight(); }, }, - + mounted() { + this.highlightFile(); + this.lineHighlighter = new LineHighlighter({ + fileHolderSelector: '.blob-viewer-container', + scrollFileHolder: true, + }); + }, watch: { html() { this.$nextTick(() => { diff --git a/app/assets/javascripts/right_sidebar.js b/app/assets/javascripts/right_sidebar.js index a4eae135403..0c1ec276baf 100644 --- a/app/assets/javascripts/right_sidebar.js +++ b/app/assets/javascripts/right_sidebar.js @@ -43,6 +43,8 @@ import Cookies from 'js-cookie'; $allGutterToggleIcons.removeClass('fa-angle-double-left').addClass('fa-angle-double-right'); $('aside.right-sidebar').removeClass('right-sidebar-collapsed').addClass('right-sidebar-expanded'); $('.page-with-sidebar').removeClass('right-sidebar-collapsed').addClass('right-sidebar-expanded'); + + if (gl.lazyLoader) gl.lazyLoader.loadCheck(); } if (!triggered) { return Cookies.set("collapsed_gutter", $('.right-sidebar').hasClass('right-sidebar-collapsed')); diff --git a/app/assets/stylesheets/framework/new-nav.scss b/app/assets/stylesheets/framework/new-nav.scss index c3badec59c9..81022d4af2a 100644 --- a/app/assets/stylesheets/framework/new-nav.scss +++ b/app/assets/stylesheets/framework/new-nav.scss @@ -306,6 +306,8 @@ header.navbar-gitlab-new { display: flex; width: 100%; position: relative; + padding-top: $gl-padding / 2; + padding-bottom: $gl-padding / 2; align-items: center; border-bottom: 1px solid $border-color; } @@ -317,11 +319,6 @@ header.navbar-gitlab-new { align-self: center; color: $gl-text-color-secondary; - @media (max-width: $screen-xs-max) { - padding-left: 17px; - border-left: 1px solid $gl-text-color-quaternary; - } - .avatar-tile { margin-right: 4px; border: 1px solid $border-color; @@ -351,6 +348,7 @@ header.navbar-gitlab-new { display: flex; align-items: center; position: relative; + padding: 2px 0; &:not(:last-child) { margin-right: 20px; @@ -386,7 +384,7 @@ header.navbar-gitlab-new { margin: 0; font-size: 12px; font-weight: 600; - line-height: 1; + line-height: 16px; a { color: $gl-text-color; diff --git a/app/assets/stylesheets/framework/new-sidebar.scss b/app/assets/stylesheets/framework/new-sidebar.scss index 9c404b7e542..3f1cb97aebc 100644 --- a/app/assets/stylesheets/framework/new-sidebar.scss +++ b/app/assets/stylesheets/framework/new-sidebar.scss @@ -461,6 +461,13 @@ $new-sidebar-collapsed-width: 50px; font-size: 18px; } } + + @media (max-width: $screen-xs-max) { + + .breadcrumbs-links { + padding-left: 17px; + border-left: 1px solid $gl-text-color-quaternary; + } + } } @media (max-width: $screen-xs-max) { diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index ca514add9cd..e8bb42f4a8c 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -7,6 +7,7 @@ $gutter_inner_width: 250px; $sidebar-transition-duration: .15s; $sidebar-breakpoint: 1024px; $default-transition-duration: .15s; +$right-sidebar-transition-duration: .3s; /* * Color schema diff --git a/app/assets/stylesheets/pages/boards.scss b/app/assets/stylesheets/pages/boards.scss index 700be173039..3305a482a0d 100644 --- a/app/assets/stylesheets/pages/boards.scss +++ b/app/assets/stylesheets/pages/boards.scss @@ -55,6 +55,15 @@ .boards-app { position: relative; + + @media (min-width: $screen-sm-min) { + transition: width $right-sidebar-transition-duration; + width: 100%; + + &.is-compact { + width: calc(100% - #{$gutter_width}); + } + } } .boards-app-loading { @@ -78,11 +87,6 @@ height: calc(100vh - 222px); // scss-lint:enable DuplicateProperty min-height: 475px; - transition: width .2s; - - &.is-compact { - width: calc(100% - 290px); - } } } @@ -412,14 +416,6 @@ .page-with-layout-nav.page-with-sub-nav .issue-boards-sidebar, .page-with-new-sidebar.page-with-sidebar .issue-boards-sidebar { - position: absolute; - - &.right-sidebar { - top: 0; - bottom: 0; - height: 100%; - } - .issuable-sidebar-header { position: relative; } @@ -457,8 +453,8 @@ .right-sidebar.right-sidebar-expanded { &.boards-sidebar-slide-enter-active, &.boards-sidebar-slide-leave-active { - transition: width .2s, - padding .2s; + transition: width $right-sidebar-transition-duration, + padding $right-sidebar-transition-duration; } &.boards-sidebar-slide-enter, diff --git a/app/assets/stylesheets/pages/diff.scss b/app/assets/stylesheets/pages/diff.scss index ed9d5e98467..e4bd783c8bc 100644 --- a/app/assets/stylesheets/pages/diff.scss +++ b/app/assets/stylesheets/pages/diff.scss @@ -535,7 +535,6 @@ } .diff-notes-collapse { - position: relative; width: 19px; height: 19px; padding: 0; @@ -543,11 +542,7 @@ z-index: 100; svg { - position: absolute; - left: 50%; - top: 50%; - margin-left: -5.5px; - margin-top: -5.5px; + vertical-align: text-top; } path { diff --git a/app/assets/stylesheets/pages/issuable.scss b/app/assets/stylesheets/pages/issuable.scss index 3ddf3d8ea7a..7eb28354e6d 100644 --- a/app/assets/stylesheets/pages/issuable.scss +++ b/app/assets/stylesheets/pages/issuable.scss @@ -223,14 +223,14 @@ top: $new-navbar-height; bottom: 0; right: 0; - transition: width .3s; + transition: width $right-sidebar-transition-duration; background: $gray-light; z-index: 200; overflow: hidden; .issuable-sidebar { width: calc(100% + 100px); - height: calc(100% - #{$new-navbar-height}); + height: 100%; overflow-y: scroll; overflow-x: hidden; -webkit-overflow-scrolling: touch; diff --git a/app/assets/stylesheets/pages/repo.scss b/app/assets/stylesheets/pages/repo.scss index 4d4d92f9494..c36fe25f74d 100644 --- a/app/assets/stylesheets/pages/repo.scss +++ b/app/assets/stylesheets/pages/repo.scss @@ -54,6 +54,10 @@ border-radius: $border-radius-default; color: $almost-black; + .code.white pre .hll { + background-color: $well-light-border !important; + } + .tree-content-holder { display: flex; min-height: 300px; diff --git a/app/controllers/admin/applications_controller.rb b/app/controllers/admin/applications_controller.rb index 16590e66d61..fb6d8c0bb81 100644 --- a/app/controllers/admin/applications_controller.rb +++ b/app/controllers/admin/applications_controller.rb @@ -22,8 +22,7 @@ class Admin::ApplicationsController < Admin::ApplicationController @application = Doorkeeper::Application.new(application_params) if @application.save - flash[:notice] = I18n.t(:notice, scope: [:doorkeeper, :flash, :applications, :create]) - redirect_to admin_application_url(@application) + redirect_to_admin_page else render :new end @@ -42,6 +41,13 @@ class Admin::ApplicationsController < Admin::ApplicationController redirect_to admin_applications_url, status: 302, notice: 'Application was successfully destroyed.' end + protected + + def redirect_to_admin_page + flash[:notice] = I18n.t(:notice, scope: [:doorkeeper, :flash, :applications, :create]) + redirect_to admin_application_url(@application) + end + private def set_application diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index cbcef70e957..676a7203c7d 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -128,7 +128,7 @@ class Admin::UsersController < Admin::ApplicationController end respond_to do |format| - result = Users::UpdateService.new(user, user_params_with_pass).execute do |user| + result = Users::UpdateService.new(current_user, user_params_with_pass.merge(user: user)).execute do |user| user.skip_reconfirmation! end @@ -155,7 +155,7 @@ class Admin::UsersController < Admin::ApplicationController def remove_email email = user.emails.find(params[:email_id]) - success = Emails::DestroyService.new(user, email: email.email).execute + success = Emails::DestroyService.new(current_user, user: user, email: email.email).execute respond_to do |format| if success @@ -219,7 +219,7 @@ class Admin::UsersController < Admin::ApplicationController end def update_user(&block) - result = Users::UpdateService.new(user).execute(&block) + result = Users::UpdateService.new(current_user, user: user).execute(&block) result[:status] == :success end diff --git a/app/controllers/concerns/issuable_collections.rb b/app/controllers/concerns/issuable_collections.rb index 8921d55c3d0..3181f517087 100644 --- a/app/controllers/concerns/issuable_collections.rb +++ b/app/controllers/concerns/issuable_collections.rb @@ -106,7 +106,7 @@ module IssuableCollections # @filter_params[:authorized_only] = true end - @filter_params + @filter_params.permit(IssuableFinder::VALID_PARAMS) end def set_default_state diff --git a/app/controllers/confirmations_controller.rb b/app/controllers/confirmations_controller.rb index 306afb65f10..10d2665c06a 100644 --- a/app/controllers/confirmations_controller.rb +++ b/app/controllers/confirmations_controller.rb @@ -12,10 +12,14 @@ class ConfirmationsController < Devise::ConfirmationsController def after_confirmation_path_for(resource_name, resource) if signed_in?(resource_name) - after_sign_in_path_for(resource) + after_sign_in(resource) else flash[:notice] += " Please sign in." new_session_path(resource_name) end end + + def after_sign_in(resource) + after_sign_in_path_for(resource) + end end diff --git a/app/controllers/oauth/applications_controller.rb b/app/controllers/oauth/applications_controller.rb index 2ae4785b12c..b02e64a132b 100644 --- a/app/controllers/oauth/applications_controller.rb +++ b/app/controllers/oauth/applications_controller.rb @@ -21,14 +21,20 @@ class Oauth::ApplicationsController < Doorkeeper::ApplicationsController @application.owner = current_user if @application.save - flash[:notice] = I18n.t(:notice, scope: [:doorkeeper, :flash, :applications, :create]) - redirect_to oauth_application_url(@application) + redirect_to_oauth_application_page else set_index_vars render :index end end + protected + + def redirect_to_oauth_application_page + flash[:notice] = I18n.t(:notice, scope: [:doorkeeper, :flash, :applications, :create]) + redirect_to oauth_application_url(@application) + end + private def verify_user_oauth_applications_enabled diff --git a/app/controllers/profiles/avatars_controller.rb b/app/controllers/profiles/avatars_controller.rb index 408650aac54..39b9f8a84d1 100644 --- a/app/controllers/profiles/avatars_controller.rb +++ b/app/controllers/profiles/avatars_controller.rb @@ -2,7 +2,7 @@ class Profiles::AvatarsController < Profiles::ApplicationController def destroy @user = current_user - Users::UpdateService.new(@user).execute { |user| user.remove_avatar! } + Users::UpdateService.new(current_user, user: @user).execute { |user| user.remove_avatar! } redirect_to profile_path, status: 302 end diff --git a/app/controllers/profiles/emails_controller.rb b/app/controllers/profiles/emails_controller.rb index ddb67d1c4d1..97db84b92d4 100644 --- a/app/controllers/profiles/emails_controller.rb +++ b/app/controllers/profiles/emails_controller.rb @@ -5,7 +5,7 @@ class Profiles::EmailsController < Profiles::ApplicationController end def create - @email = Emails::CreateService.new(current_user, email_params).execute + @email = Emails::CreateService.new(current_user, email_params.merge(user: current_user)).execute if @email.errors.blank? NotificationService.new.new_email(@email) @@ -19,7 +19,7 @@ class Profiles::EmailsController < Profiles::ApplicationController def destroy @email = current_user.emails.find(params[:id]) - Emails::DestroyService.new(current_user, email: @email.email).execute + Emails::DestroyService.new(current_user, user: current_user, email: @email.email).execute respond_to do |format| format.html { redirect_to profile_emails_url, status: 302 } diff --git a/app/controllers/profiles/keys_controller.rb b/app/controllers/profiles/keys_controller.rb index 89d6d7f1b52..069e6a810f2 100644 --- a/app/controllers/profiles/keys_controller.rb +++ b/app/controllers/profiles/keys_controller.rb @@ -14,7 +14,7 @@ class Profiles::KeysController < Profiles::ApplicationController @key = Keys::CreateService.new(current_user, key_params).execute if @key.persisted? - redirect_to profile_key_path(@key) + redirect_to_profile_key_path else @keys = current_user.keys.select(&:persisted?) render :index @@ -50,6 +50,12 @@ class Profiles::KeysController < Profiles::ApplicationController end end + protected + + def redirect_to_profile_key_path + redirect_to profile_key_path(@key) + end + private def key_params diff --git a/app/controllers/profiles/notifications_controller.rb b/app/controllers/profiles/notifications_controller.rb index 960b7512602..8a38ba65d4c 100644 --- a/app/controllers/profiles/notifications_controller.rb +++ b/app/controllers/profiles/notifications_controller.rb @@ -7,7 +7,7 @@ class Profiles::NotificationsController < Profiles::ApplicationController end def update - result = Users::UpdateService.new(current_user, user_params).execute + result = Users::UpdateService.new(current_user, user_params.merge(user: current_user)).execute if result[:status] == :success flash[:notice] = "Notification settings saved" diff --git a/app/controllers/profiles/passwords_controller.rb b/app/controllers/profiles/passwords_controller.rb index 7beb52dd8e8..dcfcb855ab5 100644 --- a/app/controllers/profiles/passwords_controller.rb +++ b/app/controllers/profiles/passwords_controller.rb @@ -21,10 +21,10 @@ class Profiles::PasswordsController < Profiles::ApplicationController password_automatically_set: false } - result = Users::UpdateService.new(@user, password_attributes).execute + result = Users::UpdateService.new(current_user, password_attributes.merge(user: @user)).execute if result[:status] == :success - Users::UpdateService.new(@user, password_expires_at: nil).execute + Users::UpdateService.new(current_user, user: @user, password_expires_at: nil).execute redirect_to root_path, notice: 'Password successfully changed' else @@ -46,7 +46,7 @@ class Profiles::PasswordsController < Profiles::ApplicationController return end - result = Users::UpdateService.new(@user, password_attributes).execute + result = Users::UpdateService.new(current_user, password_attributes.merge(user: @user)).execute if result[:status] == :success flash[:notice] = "Password was successfully updated. Please login with it" diff --git a/app/controllers/profiles/preferences_controller.rb b/app/controllers/profiles/preferences_controller.rb index cce2a847b53..ed0f98179eb 100644 --- a/app/controllers/profiles/preferences_controller.rb +++ b/app/controllers/profiles/preferences_controller.rb @@ -6,7 +6,7 @@ class Profiles::PreferencesController < Profiles::ApplicationController def update begin - result = Users::UpdateService.new(user, preferences_params).execute + result = Users::UpdateService.new(current_user, preferences_params.merge(user: user)).execute if result[:status] == :success flash[:notice] = 'Preferences saved.' diff --git a/app/controllers/profiles/two_factor_auths_controller.rb b/app/controllers/profiles/two_factor_auths_controller.rb index 1a4f77639e7..aa9789f8a0f 100644 --- a/app/controllers/profiles/two_factor_auths_controller.rb +++ b/app/controllers/profiles/two_factor_auths_controller.rb @@ -10,7 +10,7 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController current_user.otp_grace_period_started_at = Time.current end - Users::UpdateService.new(current_user).execute! + Users::UpdateService.new(current_user, user: current_user).execute! if two_factor_authentication_required? && !current_user.two_factor_enabled? two_factor_authentication_reason( @@ -41,7 +41,7 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController def create if current_user.validate_and_consume_otp!(params[:pin_code]) - Users::UpdateService.new(current_user, otp_required_for_login: true).execute! do |user| + Users::UpdateService.new(current_user, user: current_user, otp_required_for_login: true).execute! do |user| @codes = user.generate_otp_backup_codes! end @@ -70,7 +70,7 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController end def codes - Users::UpdateService.new(current_user).execute! do |user| + Users::UpdateService.new(current_user, user: current_user).execute! do |user| @codes = user.generate_otp_backup_codes! end end diff --git a/app/controllers/profiles_controller.rb b/app/controllers/profiles_controller.rb index d83824fef06..5d87037f012 100644 --- a/app/controllers/profiles_controller.rb +++ b/app/controllers/profiles_controller.rb @@ -10,7 +10,7 @@ class ProfilesController < Profiles::ApplicationController def update respond_to do |format| - result = Users::UpdateService.new(@user, user_params).execute + result = Users::UpdateService.new(current_user, user_params.merge(user: @user)).execute if result[:status] == :success message = "Profile was successfully updated" @@ -25,7 +25,7 @@ class ProfilesController < Profiles::ApplicationController end def reset_private_token - Users::UpdateService.new(@user).execute! do |user| + Users::UpdateService.new(current_user, user: @user).execute! do |user| user.reset_authentication_token! end @@ -35,7 +35,7 @@ class ProfilesController < Profiles::ApplicationController end def reset_incoming_email_token - Users::UpdateService.new(@user).execute! do |user| + Users::UpdateService.new(current_user, user: @user).execute! do |user| user.reset_incoming_email_token! end @@ -45,7 +45,7 @@ class ProfilesController < Profiles::ApplicationController end def reset_rss_token - Users::UpdateService.new(@user).execute! do |user| + Users::UpdateService.new(current_user, user: @user).execute! do |user| user.reset_rss_token! end @@ -61,7 +61,7 @@ class ProfilesController < Profiles::ApplicationController end def update_username - result = Users::UpdateService.new(@user, username: user_params[:username]).execute + result = Users::UpdateService.new(current_user, user: @user, username: user_params[:username]).execute options = if result[:status] == :success { notice: "Username successfully changed" } diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index be6491d042c..fe3bb117410 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -55,7 +55,7 @@ class SessionsController < Devise::SessionsController return unless user && user.require_password_creation? - Users::UpdateService.new(user).execute do |user| + Users::UpdateService.new(current_user, user: user).execute do |user| @token = user.generate_reset_token end diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb index 0a2e3c709d9..24c07f3dc70 100644 --- a/app/finders/issuable_finder.rb +++ b/app/finders/issuable_finder.rb @@ -25,6 +25,28 @@ class IssuableFinder NONE = '0'.freeze + SCALAR_PARAMS = %i[ + assignee_id + assignee_username + author_id + author_username + authorized_only + due_date + group_id + iids + label_name + milestone_title + non_archived + project_id + scope + search + sort + state + ].freeze + ARRAY_PARAMS = { label_name: [], iids: [], assignee_username: [] }.freeze + + VALID_PARAMS = (SCALAR_PARAMS + [ARRAY_PARAMS]).freeze + attr_accessor :current_user, :params def initialize(current_user, params = {}) diff --git a/app/helpers/boards_helper.rb b/app/helpers/boards_helper.rb index 62ac208f16a..7112c6ee470 100644 --- a/app/helpers/boards_helper.rb +++ b/app/helpers/boards_helper.rb @@ -79,6 +79,6 @@ module BoardsHelper end def boards_link_text - _("Board") + s_("IssueBoards|Board") end end diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index acaa028eaa2..3d5acc00f8f 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -434,7 +434,7 @@ module Ci def update_duration return unless started_at - self.duration = Gitlab::Ci::PipelineDuration.from_pipeline(self) + self.duration = Gitlab::Ci::Pipeline::Duration.from_pipeline(self) end def execute_hooks diff --git a/app/models/commit.rb b/app/models/commit.rb index 2ae8890c1b3..6dba154a6ea 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -25,8 +25,8 @@ class Commit DIFF_HARD_LIMIT_FILES = 1000 DIFF_HARD_LIMIT_LINES = 50000 - # The SHA can be between 7 and 40 hex characters. - COMMIT_SHA_PATTERN = '\h{7,40}'.freeze + MIN_SHA_LENGTH = 7 + COMMIT_SHA_PATTERN = /\h{#{MIN_SHA_LENGTH},40}/.freeze def banzai_render_context(field) context = { pipeline: :single_line, project: self.project } @@ -53,7 +53,7 @@ class Commit # Truncate sha to 8 characters def truncate_sha(sha) - sha[0..7] + sha[0..MIN_SHA_LENGTH] end def max_diff_options @@ -100,7 +100,7 @@ class Commit def self.reference_pattern @reference_pattern ||= %r{ (?:#{Project.reference_pattern}#{reference_prefix})? - (?<commit>\h{7,40}) + (?<commit>#{COMMIT_SHA_PATTERN}) }x end @@ -216,9 +216,8 @@ class Commit @raw.respond_to?(method, include_private) || super end - # Truncate sha to 8 characters def short_id - @raw.short_id(7) + @raw.short_id(MIN_SHA_LENGTH) end def diff_refs diff --git a/app/models/repository.rb b/app/models/repository.rb index 90cede9d3d4..f0de2697dfc 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -489,13 +489,7 @@ class Repository def exists? return false unless full_path - Gitlab::GitalyClient.migrate(:repository_exists) do |enabled| - if enabled - raw_repository.exists? - else - refs_directory_exists? - end - end + raw_repository.exists? end cache_method :exists? @@ -534,8 +528,11 @@ class Repository cache_method :tag_count, fallback: 0 def avatar - if tree = file_on_head(:avatar) - tree.path + # n+1: https://gitlab.com/gitlab-org/gitlab-ce/issues/38327 + Gitlab::GitalyClient.allow_n_plus_1_calls do + if tree = file_on_head(:avatar) + tree.path + end end end cache_method :avatar @@ -1060,12 +1057,6 @@ class Repository blob.data end - def refs_directory_exists? - circuit_breaker.perform do - File.exist?(File.join(path_to_repo, 'refs')) - end - end - def cache # TODO: should we use UUIDs here? We could move repositories without clearing this cache @cache ||= RepositoryCache.new(full_path, @project.id) @@ -1117,10 +1108,6 @@ class Repository Gitlab::Git::Repository.new(project.repository_storage, disk_path + '.git', Gitlab::GlRepository.gl_repository(project, false)) end - def circuit_breaker - @circuit_breaker ||= Gitlab::Git::Storage::CircuitBreaker.for_storage(project.repository_storage) - end - def find_commits_by_message_by_shelling_out(query, ref, path, limit, offset) ref ||= root_ref diff --git a/app/models/user.rb b/app/models/user.rb index 09c9b3250eb..103ac78783f 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -60,7 +60,7 @@ class User < ActiveRecord::Base lease = Gitlab::ExclusiveLease.new("user_update_tracked_fields:#{id}", timeout: 1.hour.to_i) return unless lease.try_obtain - Users::UpdateService.new(self).execute(validate: false) + Users::UpdateService.new(self, user: self).execute(validate: false) end attr_accessor :force_random_password @@ -526,8 +526,8 @@ class User < ActiveRecord::Base def update_emails_with_primary_email primary_email_record = emails.find_by(email: email) if primary_email_record - Emails::DestroyService.new(self, email: email).execute - Emails::CreateService.new(self, email: email_was).execute + Emails::DestroyService.new(self, user: self, email: email).execute + Emails::CreateService.new(self, user: self, email: email_was).execute end end @@ -1000,7 +1000,7 @@ class User < ActiveRecord::Base if attempts_exceeded? lock_access! unless access_locked? else - Users::UpdateService.new(self).execute(validate: false) + Users::UpdateService.new(self, user: self).execute(validate: false) end end @@ -1186,7 +1186,7 @@ class User < ActiveRecord::Base &creation_block ) - Users::UpdateService.new(user).execute(validate: false) + Users::UpdateService.new(user, user: user).execute(validate: false) user ensure Gitlab::ExclusiveLease.cancel(lease_key, uuid) diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index d20de9b16a4..31a712ccc1b 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -2,110 +2,55 @@ module Ci class CreatePipelineService < BaseService attr_reader :pipeline - def execute(source, ignore_skip_ci: false, save_on_errors: true, trigger_request: nil, schedule: nil) + SEQUENCE = [Gitlab::Ci::Pipeline::Chain::Validate::Abilities, + Gitlab::Ci::Pipeline::Chain::Validate::Repository, + Gitlab::Ci::Pipeline::Chain::Validate::Config, + Gitlab::Ci::Pipeline::Chain::Skip, + Gitlab::Ci::Pipeline::Chain::Create].freeze + + def execute(source, ignore_skip_ci: false, save_on_errors: true, trigger_request: nil, schedule: nil, &block) @pipeline = Ci::Pipeline.new( source: source, project: project, ref: ref, sha: sha, before_sha: before_sha, - tag: tag?, + tag: tag_exists?, trigger_requests: Array(trigger_request), user: current_user, pipeline_schedule: schedule, protected: project.protected_for?(ref) ) - result = validate_project_and_git_items || - validate_pipeline(ignore_skip_ci: ignore_skip_ci, - save_on_errors: save_on_errors) + command = OpenStruct.new(ignore_skip_ci: ignore_skip_ci, + save_incompleted: save_on_errors, + seeds_block: block, + project: project, + current_user: current_user) - return result if result + sequence = Gitlab::Ci::Pipeline::Chain::Sequence + .new(pipeline, command, SEQUENCE) - begin - Ci::Pipeline.transaction do - pipeline.save! + sequence.build! do |pipeline, sequence| + update_merge_requests_head_pipeline if pipeline.persisted? - yield(pipeline) if block_given? + if sequence.complete? + cancel_pending_pipelines if project.auto_cancel_pending_pipelines? + pipeline_created_counter.increment(source: source) - Ci::CreatePipelineStagesService - .new(project, current_user) - .execute(pipeline) + pipeline.process! end - rescue ActiveRecord::RecordInvalid => e - return error("Failed to persist the pipeline: #{e}") end - - update_merge_requests_head_pipeline - - cancel_pending_pipelines if project.auto_cancel_pending_pipelines? - - pipeline_created_counter.increment(source: source) - - pipeline.tap(&:process!) end private - def validate_project_and_git_items - unless project.builds_enabled? - return error('Pipeline is disabled') - end - - unless allowed_to_trigger_pipeline? - if can?(current_user, :create_pipeline, project) - return error("Insufficient permissions for protected ref '#{ref}'") - else - return error('Insufficient permissions to create a new pipeline') - end - end - - unless branch? || tag? - return error('Reference not found') - end - - unless commit - return error('Commit not found') - end - end - - def validate_pipeline(ignore_skip_ci:, save_on_errors:) - unless pipeline.config_processor - unless pipeline.ci_yaml_file - return error("Missing #{pipeline.ci_yaml_file_path} file") - end - return error(pipeline.yaml_errors, save: save_on_errors) - end - - if !ignore_skip_ci && skip_ci? - pipeline.skip if save_on_errors - return pipeline - end - - unless pipeline.has_stage_seeds? - return error('No stages / jobs for this pipeline.') - end - end - - def allowed_to_trigger_pipeline? - if current_user - allowed_to_create? - else # legacy triggers don't have a corresponding user - !project.protected_for?(ref) - end + def commit + @commit ||= project.commit(origin_sha || origin_ref) end - def allowed_to_create? - return unless can?(current_user, :create_pipeline, project) - - access = Gitlab::UserAccess.new(current_user, project: project) - if branch? - access.can_update_branch?(ref) - elsif tag? - access.can_create_tag?(ref) - else - true # Allow it for now and we'll reject when we check ref existence - end + def sha + commit.try(:id) end def update_merge_requests_head_pipeline @@ -115,11 +60,6 @@ module Ci .update_all(head_pipeline_id: @pipeline.id) end - def skip_ci? - return false unless pipeline.git_commit_message - pipeline.git_commit_message =~ /\[(ci[ _-]skip|skip[ _-]ci)\]/i - end - def cancel_pending_pipelines Gitlab::OptimisticLocking.retry_lock(auto_cancelable_pipelines) do |cancelables| cancelables.find_each do |cancelable| @@ -136,14 +76,6 @@ module Ci .created_or_pending end - def commit - @commit ||= project.commit(origin_sha || origin_ref) - end - - def sha - commit.try(:id) - end - def before_sha params[:checkout_sha] || params[:before] || Gitlab::Git::BLANK_SHA end @@ -156,41 +88,17 @@ module Ci params[:ref] end - def branch? - return @is_branch if defined?(@is_branch) - - @is_branch = - project.repository.ref_exists?(Gitlab::Git::BRANCH_REF_PREFIX + ref) - end - - def tag? - return @is_tag if defined?(@is_tag) - - @is_tag = - project.repository.ref_exists?(Gitlab::Git::TAG_REF_PREFIX + ref) + def tag_exists? + project.repository.tag_exists?(ref) end def ref @ref ||= Gitlab::Git.ref_name(origin_ref) end - def valid_sha? - origin_sha && origin_sha != Gitlab::Git::BLANK_SHA - end - - def error(message, save: false) - pipeline.tap do - pipeline.errors.add(:base, message) - - if save - pipeline.drop - update_merge_requests_head_pipeline - end - end - end - def pipeline_created_counter - @pipeline_created_counter ||= Gitlab::Metrics.counter(:pipelines_created_total, "Counter of pipelines created") + @pipeline_created_counter ||= Gitlab::Metrics + .counter(:pipelines_created_total, "Counter of pipelines created") end end end diff --git a/app/services/emails/base_service.rb b/app/services/emails/base_service.rb index ace49889097..7f591c89411 100644 --- a/app/services/emails/base_service.rb +++ b/app/services/emails/base_service.rb @@ -1,7 +1,8 @@ module Emails class BaseService - def initialize(user, opts) - @user = user + def initialize(current_user, opts) + @current_user = current_user + @user = opts.delete(:user) @email = opts[:email] end end diff --git a/app/services/emails/destroy_service.rb b/app/services/emails/destroy_service.rb index d586b9dfe0c..44011cc36c8 100644 --- a/app/services/emails/destroy_service.rb +++ b/app/services/emails/destroy_service.rb @@ -1,13 +1,13 @@ module Emails class DestroyService < ::Emails::BaseService def execute - Email.find_by_email!(@email).destroy && update_secondary_emails! + update_secondary_emails! if Email.find_by_email!(@email).destroy end private def update_secondary_emails! - result = ::Users::UpdateService.new(@user).execute do |user| + result = ::Users::UpdateService.new(@current_user, user: @user).execute do |user| user.update_secondary_emails! end diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb index 07cbd8f92a9..bf26859dd6d 100644 --- a/app/services/merge_requests/merge_service.rb +++ b/app/services/merge_requests/merge_service.rb @@ -14,13 +14,13 @@ module MergeRequests @merge_request = merge_request unless @merge_request.mergeable? - return log_merge_error('Merge request is not mergeable', save_message_on_model: true) + return handle_merge_error(log_message: 'Merge request is not mergeable', save_message_on_model: true) end @source = find_merge_source unless @source - return log_merge_error('No source for merge', save_message_on_model: true) + return handle_merge_error(log_message: 'No source for merge', save_message_on_model: true) end merge_request.in_locked_state do @@ -31,8 +31,7 @@ module MergeRequests end end rescue MergeError => e - clean_merge_jid - log_merge_error(e.message, save_message_on_model: true) + handle_merge_error(log_message: e.message, save_message_on_model: true) end private @@ -74,10 +73,16 @@ module MergeRequests @merge_request.force_remove_source_branch? ? @merge_request.author : current_user end - def log_merge_error(message, save_message_on_model: false) - Rails.logger.error("MergeService ERROR: #{merge_request_info} - #{message}") + # Logs merge error message and cleans `MergeRequest#merge_jid`. + # + def handle_merge_error(log_message:, save_message_on_model: false) + Rails.logger.error("MergeService ERROR: #{merge_request_info} - #{log_message}") - @merge_request.update(merge_error: message) if save_message_on_model + if save_message_on_model + @merge_request.update(merge_error: log_message, merge_jid: nil) + else + clean_merge_jid + end end def merge_request_info diff --git a/app/services/users/update_service.rb b/app/services/users/update_service.rb index 6188b8a4349..15ca1a55a5b 100644 --- a/app/services/users/update_service.rb +++ b/app/services/users/update_service.rb @@ -2,22 +2,21 @@ module Users class UpdateService < BaseService include NewUserNotifier - def initialize(user, params = {}) - @user = user + def initialize(current_user, params = {}) + @current_user = current_user + @user = params.delete(:user) @params = params.dup end def execute(validate: true, &block) yield(@user) if block_given? - assign_attributes(&block) - user_exists = @user.persisted? - if @user.save(validate: validate) - notify_new_user(@user, nil) unless user_exists + assign_attributes(&block) - success + if @user.save(validate: validate) + notify_success(user_exists) else error(@user.errors.full_messages.uniq.join('. ')) end @@ -33,6 +32,12 @@ module Users private + def notify_success(user_exists) + notify_new_user(@user, nil) unless user_exists + + success + end + def assign_attributes(&block) if @user.user_synced_attributes_metadata params.except!(*@user.user_synced_attributes_metadata.read_only_attributes) diff --git a/app/views/projects/artifacts/browse.html.haml b/app/views/projects/artifacts/browse.html.haml index 05857573297..fe02cbcbf95 100644 --- a/app/views/projects/artifacts/browse.html.haml +++ b/app/views/projects/artifacts/browse.html.haml @@ -3,7 +3,7 @@ = render "projects/jobs/header", show_controls: false -- add_to_breadcrumbs(_('Jobs'), project_jobs_path(@project)) +- add_to_breadcrumbs(s_('CICD|Jobs'), project_jobs_path(@project)) - add_to_breadcrumbs("##{@build.id}", project_jobs_path(@project)) .tree-holder diff --git a/app/views/shared/_auto_devops_callout.html.haml b/app/views/shared/_auto_devops_callout.html.haml index 2f09c2fec87..7c633175a06 100644 --- a/app/views/shared/_auto_devops_callout.html.haml +++ b/app/views/shared/_auto_devops_callout.html.haml @@ -6,10 +6,10 @@ .svg-container = custom_icon('icon_autodevops') .user-callout-copy - %h4= _('Auto DevOps (Beta)') - %p= _('Auto DevOps can be activated for this project. It will automatically build, test, and deploy your application based on a predefined CI/CD configuration.') + %h4= s_('AutoDevOps|Auto DevOps (Beta)') + %p= s_('AutoDevOps|Auto DevOps can be activated for this project. It will automatically build, test, and deploy your application based on a predefined CI/CD configuration.') %p - #{s_('AutoDevOps|Learn more in the')} - = link_to _('Auto DevOps documentation'), help_page_path('topics/autodevops/index.md'), target: '_blank', rel: 'noopener noreferrer' + - link = link_to(s_('AutoDevOps|Auto DevOps documentation'), help_page_path('topics/autodevops/index.md'), target: '_blank', rel: 'noopener noreferrer') + = s_('AutoDevOps|Learn more in the %{link_to_documentation}').html_safe % { link_to_documentation: link } - = link_to _('Enable in settings'), project_settings_ci_cd_path(@project, anchor: 'js-general-pipeline-settings'), class: 'btn btn-primary js-close-callout' + = link_to s_('AutoDevOps|Enable in settings'), project_settings_ci_cd_path(@project, anchor: 'js-general-pipeline-settings'), class: 'btn btn-primary js-close-callout' diff --git a/app/views/shared/boards/_show.html.haml b/app/views/shared/boards/_show.html.haml index 7e22c678e81..ee8ad8e3999 100644 --- a/app/views/shared/boards/_show.html.haml +++ b/app/views/shared/boards/_show.html.haml @@ -12,11 +12,11 @@ %script#js-board-template{ type: "text/x-template" }= render "shared/boards/components/board" %script#js-board-modal-filter{ type: "text/x-template" }= render "shared/issuable/search_bar", type: :boards_modal -.hidden-xs.hidden-sm - = render 'shared/issuable/search_bar', type: :boards +#board-app.boards-app{ "v-cloak" => true, data: board_data, ":class" => "{ 'is-compact': detailIssueVisible }" } + .hidden-xs.hidden-sm + = render 'shared/issuable/search_bar', type: :boards -#board-app.boards-app{ "v-cloak" => true, data: board_data } - .boards-list{ ":class" => "{ 'is-compact': detailIssueVisible }" } + .boards-list .boards-app-loading.text-center{ "v-if" => "loading" } = icon("spinner spin") %board{ "v-cloak" => true, diff --git a/changelogs/unreleased/3523-i18n-autodevops.yml b/changelogs/unreleased/3523-i18n-autodevops.yml new file mode 100644 index 00000000000..10cb22b42a0 --- /dev/null +++ b/changelogs/unreleased/3523-i18n-autodevops.yml @@ -0,0 +1,5 @@ +--- +title: Improves i18n for Auto Devops callout +merge_request: +author: +type: other diff --git a/changelogs/unreleased/37467-helper-method-from-users-endpoint-overrides-api-helper-method.yml b/changelogs/unreleased/37467-helper-method-from-users-endpoint-overrides-api-helper-method.yml new file mode 100644 index 00000000000..1984ec6e81c --- /dev/null +++ b/changelogs/unreleased/37467-helper-method-from-users-endpoint-overrides-api-helper-method.yml @@ -0,0 +1,5 @@ +--- +title: find_user Users helper method no longer overrides find_user API helper method. +merge_request: 14418 +author: +type: fixed diff --git a/changelogs/unreleased/37912-fix-dash-in-note-access-role.yml b/changelogs/unreleased/37912-fix-dash-in-note-access-role.yml deleted file mode 100644 index f9f4120479c..00000000000 --- a/changelogs/unreleased/37912-fix-dash-in-note-access-role.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Notes will not show an empty bubble when the author isn't a member. -merge_request: 14450 -author: -type: fixed diff --git a/changelogs/unreleased/38280-undefined-run_command-when-running-rake-gitlab-check.yml b/changelogs/unreleased/38280-undefined-run_command-when-running-rake-gitlab-check.yml deleted file mode 100644 index 7d3fb7d43cc..00000000000 --- a/changelogs/unreleased/38280-undefined-run_command-when-running-rake-gitlab-check.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Some checks in `rake gitlab:check` were failling with 'undefined method `run_command`' -merge_request: 14469 -author: -type: fixed diff --git a/changelogs/unreleased/38319-nomethoderror-undefined-method-sha-for-nil-nilclass.yml b/changelogs/unreleased/38319-nomethoderror-undefined-method-sha-for-nil-nilclass.yml new file mode 100644 index 00000000000..f3c39827590 --- /dev/null +++ b/changelogs/unreleased/38319-nomethoderror-undefined-method-sha-for-nil-nilclass.yml @@ -0,0 +1,5 @@ +--- +title: Fix 500 error on merged merge requests when GitLab is restored from a backup +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/38476-improve-merge-jid-cleanup-on-merge-process.yml b/changelogs/unreleased/38476-improve-merge-jid-cleanup-on-merge-process.yml new file mode 100644 index 00000000000..43dec51029b --- /dev/null +++ b/changelogs/unreleased/38476-improve-merge-jid-cleanup-on-merge-process.yml @@ -0,0 +1,5 @@ +--- +title: Adjust MRs being stuck on "process of being merged" for more than 2 hours +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/breadcrumbs-line-height-padding.yml b/changelogs/unreleased/breadcrumbs-line-height-padding.yml new file mode 100644 index 00000000000..3ac56c8b593 --- /dev/null +++ b/changelogs/unreleased/breadcrumbs-line-height-padding.yml @@ -0,0 +1,5 @@ +--- +title: breadcrumbs receives padding when double lined +merge_request: +author: +type: changed diff --git a/changelogs/unreleased/commit-side-by-side-comment.yml b/changelogs/unreleased/commit-side-by-side-comment.yml new file mode 100644 index 00000000000..f9bea285a77 --- /dev/null +++ b/changelogs/unreleased/commit-side-by-side-comment.yml @@ -0,0 +1,5 @@ +--- +title: Fixed commenting on side-by-side commit diff +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/dm-api-unauthorized.yml b/changelogs/unreleased/dm-api-unauthorized.yml new file mode 100644 index 00000000000..26b45bd4c40 --- /dev/null +++ b/changelogs/unreleased/dm-api-unauthorized.yml @@ -0,0 +1,5 @@ +--- +title: Make sure API responds with 401 when invalid authentication info is provided +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/dm-bitbucket-import-truncated-shas.yml b/changelogs/unreleased/dm-bitbucket-import-truncated-shas.yml new file mode 100644 index 00000000000..057407b78d9 --- /dev/null +++ b/changelogs/unreleased/dm-bitbucket-import-truncated-shas.yml @@ -0,0 +1,6 @@ +--- +title: Fix bug that caused merge requests with diff notes imported from Bitbucket + to raise errors +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/dm-simple-project-avatar-url.yml b/changelogs/unreleased/dm-simple-project-avatar-url.yml new file mode 100644 index 00000000000..e517345f5d2 --- /dev/null +++ b/changelogs/unreleased/dm-simple-project-avatar-url.yml @@ -0,0 +1,5 @@ +--- +title: Expose avatar_url when requesting list of projects from API with simple=true +merge_request: +author: +type: added diff --git a/changelogs/unreleased/docs-28814-clarify-artifacts-ref.yml b/changelogs/unreleased/docs-28814-clarify-artifacts-ref.yml new file mode 100644 index 00000000000..3cdcff8caaf --- /dev/null +++ b/changelogs/unreleased/docs-28814-clarify-artifacts-ref.yml @@ -0,0 +1,5 @@ +--- +title: Clarify artifact download via the API only accepts branch or tag name for ref +merge_request: +author: +type: other diff --git a/changelogs/unreleased/docs-38152-bump-recommended-mysql-version.yml b/changelogs/unreleased/docs-38152-bump-recommended-mysql-version.yml new file mode 100644 index 00000000000..eea679d0814 --- /dev/null +++ b/changelogs/unreleased/docs-38152-bump-recommended-mysql-version.yml @@ -0,0 +1,5 @@ +--- +title: Change recommended MySQL version to 5.6 +merge_request: +author: +type: other diff --git a/changelogs/unreleased/expose-last-pipeline-details-in-api-for-single-commit.yml b/changelogs/unreleased/expose-last-pipeline-details-in-api-for-single-commit.yml new file mode 100644 index 00000000000..d16e052cd92 --- /dev/null +++ b/changelogs/unreleased/expose-last-pipeline-details-in-api-for-single-commit.yml @@ -0,0 +1,5 @@ +--- +title: Expose last pipeline details in API response when getting a single commit +merge_request: 13521 +author: Mehdi Lahmam (@mehlah) +type: added diff --git a/changelogs/unreleased/fix-locked-shared-runners-problem.yml b/changelogs/unreleased/fix-locked-shared-runners-problem.yml deleted file mode 100644 index 3e3cccf79eb..00000000000 --- a/changelogs/unreleased/fix-locked-shared-runners-problem.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Make locked setting of Runner to not affect jobs scheduling -merge_request: 14483 -author: -type: fixed diff --git a/changelogs/unreleased/mr-side-by-side-breadcrumbs-container.yml b/changelogs/unreleased/mr-side-by-side-breadcrumbs-container.yml new file mode 100644 index 00000000000..39b636bdfda --- /dev/null +++ b/changelogs/unreleased/mr-side-by-side-breadcrumbs-container.yml @@ -0,0 +1,5 @@ +--- +title: Fixed breadcrumbs container expanding in side-by-side diff view +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/remove-temporary-ci-index.yml b/changelogs/unreleased/remove-temporary-ci-index.yml new file mode 100644 index 00000000000..a319f7fff7f --- /dev/null +++ b/changelogs/unreleased/remove-temporary-ci-index.yml @@ -0,0 +1,5 @@ +--- +title: Remove an index on ci_builds meant to be only temporary +merge_request: +author: +type: other diff --git a/changelogs/unreleased/rotated_profile_image.yml b/changelogs/unreleased/rotated_profile_image.yml new file mode 100644 index 00000000000..1e221e47379 --- /dev/null +++ b/changelogs/unreleased/rotated_profile_image.yml @@ -0,0 +1,5 @@ +--- +title: Fix profile image orientation based on EXIF data gvieira37 +merge_request: 14461 +author: gvieira37 +type: fixed diff --git a/changelogs/unreleased/rs-allow-name-on-anchors.yml b/changelogs/unreleased/rs-allow-name-on-anchors.yml deleted file mode 100644 index 59e95ed8a0e..00000000000 --- a/changelogs/unreleased/rs-allow-name-on-anchors.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Re-allow `name` attribute on user-provided anchor HTML -merge_request: -author: -type: fixed diff --git a/changelogs/unreleased/zj-repo-gitaly.yml b/changelogs/unreleased/zj-repo-gitaly.yml new file mode 100644 index 00000000000..634f6ba1b8b --- /dev/null +++ b/changelogs/unreleased/zj-repo-gitaly.yml @@ -0,0 +1,5 @@ +--- +title: Gitaly RepositoryExists remains opt-in for all method calls +merge_request: +author: +type: fixed diff --git a/db/migrate/20170919211300_remove_temporary_ci_builds_index.rb b/db/migrate/20170919211300_remove_temporary_ci_builds_index.rb new file mode 100644 index 00000000000..b2009b282e9 --- /dev/null +++ b/db/migrate/20170919211300_remove_temporary_ci_builds_index.rb @@ -0,0 +1,27 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class RemoveTemporaryCiBuildsIndex < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + # To use create/remove index concurrently + disable_ddl_transaction! + + def up + return unless index_exists?(:ci_builds, :id, name: 'index_for_ci_builds_retried_migration') + remove_concurrent_index(:ci_builds, :id, name: "index_for_ci_builds_retried_migration") + end + + def down + # this was a temporary index for a migration that was never + # present previously so this probably shouldn't be here but it's + # easier to test the drop if we have a way to create it. + add_concurrent_index("ci_builds", ["id"], + name: "index_for_ci_builds_retried_migration", + where: "(retried IS NULL)", + using: :btree) + end +end diff --git a/db/schema.rb b/db/schema.rb index 80ef91ec95d..330336e8e61 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -256,7 +256,6 @@ ActiveRecord::Schema.define(version: 20170921115009) do add_index "ci_builds", ["commit_id", "status", "type"], name: "index_ci_builds_on_commit_id_and_status_and_type", using: :btree add_index "ci_builds", ["commit_id", "type", "name", "ref"], name: "index_ci_builds_on_commit_id_and_type_and_name_and_ref", using: :btree add_index "ci_builds", ["commit_id", "type", "ref"], name: "index_ci_builds_on_commit_id_and_type_and_ref", using: :btree - add_index "ci_builds", ["id"], name: "index_for_ci_builds_retried_migration", where: "(retried IS NULL)", using: :btree add_index "ci_builds", ["project_id"], name: "index_ci_builds_on_project_id", using: :btree add_index "ci_builds", ["protected"], name: "index_ci_builds_on_protected", using: :btree add_index "ci_builds", ["runner_id"], name: "index_ci_builds_on_runner_id", using: :btree diff --git a/doc/api/commits.md b/doc/api/commits.md index 2a78553782f..5a4a8d888b3 100644 --- a/doc/api/commits.md +++ b/doc/api/commits.md @@ -181,6 +181,12 @@ Example response: "parent_ids": [ "ae1d9fb46aa2b07ee9836d49862ec4e2c46fbbba" ], + "last_pipeline" : { + "id": 8, + "ref": "master", + "sha": "2dc6aa325a317eda67812f05600bdf0fcdc70ab0" + "status": "created" + } "stats": { "additions": 15, "deletions": 10, diff --git a/doc/api/jobs.md b/doc/api/jobs.md index d60c7c12881..e7060e154f4 100644 --- a/doc/api/jobs.md +++ b/doc/api/jobs.md @@ -336,7 +336,7 @@ Parameters | Attribute | Type | Required | Description | |-------------|---------|----------|-------------------------- | | `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user | -| `ref_name` | string | yes | The ref from a repository | +| `ref_name` | string | yes | The ref from a repository (can only be branch or tag name, not HEAD or SHA) | | `job` | string | yes | The name of the job | Example request: diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index aad81843299..38bd0450a09 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -1570,6 +1570,11 @@ Read more on [GitLab Pages user documentation](../../user/project/pages/index.md Each instance of GitLab CI has an embedded debug tool called Lint. You can find the link under `/ci/lint` of your gitlab instance. +## Using reserved keywords + +If you get validation error when using specific values (e.g., `true` or `false`), +try to quote them, or change them to a different form (e.g., `/bin/true`). + ## Skipping jobs If your commit message contains `[ci skip]` or `[skip ci]`, using any diff --git a/doc/development/README.md b/doc/development/README.md index 3096d9f25f0..1448a4c0414 100644 --- a/doc/development/README.md +++ b/doc/development/README.md @@ -44,6 +44,7 @@ - [Building a package for testing purposes](build_test_package.md) - [Manage feature flags](feature_flags.md) - [View sent emails or preview mailers](emails.md) +- [Working with Gitaly](gitaly.md) ## Databases diff --git a/doc/development/fe_guide/index.md b/doc/development/fe_guide/index.md index d84801f91d4..031b12a8e91 100644 --- a/doc/development/fe_guide/index.md +++ b/doc/development/fe_guide/index.md @@ -29,34 +29,6 @@ For our currently-supported browsers, see our [requirements][requirements]. ## Development Process -When you are assigned an issue please follow the next steps: - -### Divide a big feature into small Merge Requests -1. Big Merge Request are painful to review. In order to make this process easier we -must break a big feature into smaller ones and create a Merge Request for each step. -1. First step is to create a branch from `master`, let's call it `new-feature`. This branch -will be the recipient of all the smaller Merge Requests. Only this one will be merged to master. -1. Don't do any work on this one, let's keep it synced with master. -1. Create a new branch from `new-feature`, let's call it `new-feature-step-1`. We advise you -to clearly identify which step the branch represents. -1. Do the first part of the modifications in this branch. The target branch of this Merge Request -should be `new-feature`. -1. Once `new-feature-step-1` gets merged into `new-feature` we can continue our work. Create a new -branch from `new-feature`, let's call it `new-feature-step-2` and repeat the process done before. - -```shell -master -└─ new-feature - ├─ new-feature-step-1 - ├─ new-feature-step-2 - └─ new-feature-step-3 -``` - -**Tips** -- Make sure `new-feature` branch is always synced with `master`: merge master frequently. -- Do the same for the feature branch you have opened. This can be accomplished by merging `master` into `new-feature` and `new-feature` into `new-feature-step-*` -- Avoid rewriting history. - ### Share your work early 1. Before writing code guarantee your vision of the architecture is aligned with GitLab's architecture. diff --git a/doc/development/gitaly.md b/doc/development/gitaly.md new file mode 100644 index 00000000000..f0be3a6b141 --- /dev/null +++ b/doc/development/gitaly.md @@ -0,0 +1,54 @@ +# GitLab Developers Guide to Working with Gitaly + +[Gitaly](https://gitlab.com/gitlab-org/gitaly) is a high-level Git RPC service used by GitLab CE/EE, +Workhorse and GitLab-Shell. All Rugged operations in GitLab CE/EE are currently being phased out to +be replaced by Gitaly API calls. + +Visit the [Gitaly Migration Board](https://gitlab.com/gitlab-org/gitaly/boards/331341) for current +status of the migration. + +## Feature Flags + +Gitaly makes heavy use of [feature flags](feature_flags.md). + +Each Rugged-to-Gitaly migration goes through a [series of phases](https://gitlab.com/gitlab-org/gitaly/blob/master/doc/MIGRATION_PROCESS.md): +* **Opt-In**: by default the Rugged implementation is used. + * Production instances can choose to enable the Gitaly endpoint by enabling the feature flag. + * For testing purposes, you may wish to enable all feature flags by default. This can be done by exporting the following + environment variable: `GITALY_FEATURE_DEFAULT_ON=1`. + * On developer instances (ie, when `Rails.env.development?` is true), the Gitaly endpoint + is enabled by default, but can be _disabled_ using feature flags. +* **Opt-Out**: by default, the Gitaly endpoint is used, but the feature can be explicitly disabled using the feature flag. +* **Madatory**: The migration is complete and cannot be disabled. The old codepath is removed. + +### Enabling and Disabling Feature + +In the Rails console, type: + +```ruby +Feature.enable(:gitaly_feature_name) +Feature.disable(:gitaly_feature_name) +``` + +Where `gitaly_feature_name` is the name of the Gitaly feature. This can be determined by finding the appropriate +`gitaly_migrate` code block, for example: + +```ruby +gitaly_migrate(:tag_names) do +... +end +``` + +Since Gitaly features are always prefixed with `gitaly_`, the name of the feature flag in this case would be `gitaly_tag_names`. + +## Gitaly-Related Test Failures + +If your test-suite is failing with Gitaly issues, as a first step, try running: + +```shell +rm -rf tmp/tests/gitaly +``` + +--- + +[Return to Development documentation](README.md) diff --git a/doc/development/licensing.md b/doc/development/licensing.md index 9a5811d8474..a75cdf22f40 100644 --- a/doc/development/licensing.md +++ b/doc/development/licensing.md @@ -65,6 +65,7 @@ Libraries with the following licenses are unacceptable for use: - [GNU AGPLv3][AGPLv3]: AGPL-licensed libraries cannot be linked to from non-GPL projects. - [Open Software License (OSL)][OSL]: is a copyleft license. In addition, the FSF [recommend against its use][OSL-GNU]. - [Facebook BSD + PATENTS][Facebook]: is a 3-clause BSD license with a patent grant that has been deemed [Category X][x-list] by the Apache foundation. +- [WTFPL][WTFPL]: is a public domain dedication [rejected by the OSI (3.2)][WTFPL-OSI]. Also has a strong language which is not in accordance with our diversity policy. ## Requesting Approval for Licenses @@ -108,3 +109,5 @@ Gems which are included only in the "development" or "test" groups by Bundler ar [x-list]: https://www.apache.org/legal/resolved.html#category-x [Acceptable-Licenses]: #acceptable-licenses [Unacceptable-Licenses]: #unacceptable-licenses +[WTFPL]: https://wtfpl.net +[WTFPL-OSI]: https://opensource.org/minutes20090304 diff --git a/doc/development/testing.md b/doc/development/testing.md index 83269303005..d856b003353 100644 --- a/doc/development/testing.md +++ b/doc/development/testing.md @@ -150,6 +150,16 @@ always in-sync with the codebase. [GitLab QA]: https://gitlab.com/gitlab-org/gitlab-qa [part of GitLab Rails]: https://gitlab.com/gitlab-org/gitlab-ce/tree/master/qa +## Test for what should not be there + +This is particularly important for permission calls and might be called a +negative assertion: make sure only the bare minimum is returned and nothing else. + +See an issue about [leaking tokens] as an example of a vulnerability that is +captured by such a test. + +[leaking tokens]: https://gitlab.com/gitlab-org/gitlab-ce/issues/37948 + ## How to test at the correct level? As many things in life, deciding what to test at each level of testing is a @@ -493,24 +503,24 @@ Here are some things to keep in mind regarding test performance: Our current CI parallelization setup is as follows: -1. The `knapsack` job in the prepare stage that is supposed to ensure we have a - `knapsack/${CI_PROJECT_NAME}/rspec_report-master.json` file: +1. The `retrieve-tests-metadata` job in the `prepare` stage ensures that we have + a `knapsack/${CI_PROJECT_NAME}/rspec_report-master.json` file: - The `knapsack/${CI_PROJECT_NAME}/rspec_report-master.json` file is fetched from S3, if it's not here we initialize the file with `{}`. -1. Each `rspec x y` job are run with `knapsack rspec` and should have an evenly - distributed share of tests: +1. Each `rspec-pg x y`/`rspec-mysql x y` job is run with `knapsack rspec` and + should have an evenly distributed share of tests: - It works because the jobs have access to the `knapsack/${CI_PROJECT_NAME}/rspec_report-master.json` since the "artifacts from all previous stages are passed by default". [^1] - - the jobs set their own report path to + - The jobs set their own report path to `KNAPSACK_REPORT_PATH=knapsack/${CI_PROJECT_NAME}/${JOB_NAME[0]}_node_${CI_NODE_INDEX}_${CI_NODE_TOTAL}_report.json`. - - if knapsack is doing its job, test files that are run should be listed under + - If knapsack is doing its job, test files that are run should be listed under `Report specs`, not under `Leftover specs`. -1. The `update-knapsack` job takes all the +1. The `update-tests-metadata` job takes all the `knapsack/${CI_PROJECT_NAME}/${JOB_NAME[0]}_node_${CI_NODE_INDEX}_${CI_NODE_TOTAL}_report.json` - files from the `rspec x y` jobs and merge them all together into a single - `knapsack/${CI_PROJECT_NAME}/rspec_report-master.json` file that is then - uploaded to S3. + files from the `rspec-pg x y`/`rspec-mysql x y`jobs and merge them all together + into a single `knapsack/${CI_PROJECT_NAME}/rspec_report-master.json` file that + is then uploaded to S3. After that, the next pipeline will use the up-to-date `knapsack/${CI_PROJECT_NAME}/rspec_report-master.json` file. The same strategy diff --git a/doc/install/database_mysql.md b/doc/install/database_mysql.md index 5c128f54a76..f9ba1508705 100644 --- a/doc/install/database_mysql.md +++ b/doc/install/database_mysql.md @@ -1,11 +1,12 @@ # Database MySQL >**Note:** -We do not recommend using MySQL due to various issues. For example, case +- We do not recommend using MySQL due to various issues. For example, case [(in)sensitivity](https://dev.mysql.com/doc/refman/5.0/en/case-sensitivity.html) and [problems](https://bugs.mysql.com/bug.php?id=65830) that [suggested](https://bugs.mysql.com/bug.php?id=50909) [fixes](https://bugs.mysql.com/bug.php?id=65830) [have](https://bugs.mysql.com/bug.php?id=63164). +- We recommend using MySQL version 5.6 or later. Please see the following [issue][ce-38152]. ## Initial database setup @@ -13,7 +14,7 @@ and [problems](https://bugs.mysql.com/bug.php?id=65830) that # Install the database packages sudo apt-get install -y mysql-server mysql-client libmysqlclient-dev -# Ensure you have MySQL version 5.5.14 or later +# Ensure you have MySQL version 5.6 or later mysql --version # Pick a MySQL root password (can be anything), type it and press enter @@ -293,3 +294,4 @@ Details can be found in the [PostgreSQL][postgres-text-type] and [postgres-text-type]: http://www.postgresql.org/docs/9.2/static/datatype-character.html [mysql-text-types]: http://dev.mysql.com/doc/refman/5.7/en/string-type-overview.html +[ce-38152]: https://gitlab.com/gitlab-org/gitlab-ce/issues/38152 diff --git a/doc/user/project/integrations/img/kubernetes_configuration.png b/doc/user/project/integrations/img/kubernetes_configuration.png Binary files differindex 349a2dc8456..e535e2b8d46 100644 --- a/doc/user/project/integrations/img/kubernetes_configuration.png +++ b/doc/user/project/integrations/img/kubernetes_configuration.png diff --git a/doc/user/project/integrations/kubernetes.md b/doc/user/project/integrations/kubernetes.md index f4000523938..e9738b683f9 100644 --- a/doc/user/project/integrations/kubernetes.md +++ b/doc/user/project/integrations/kubernetes.md @@ -1,3 +1,7 @@ +--- +last_updated: 2017-09-25 +--- + # GitLab Kubernetes / OpenShift integration GitLab can be configured to interact with Kubernetes, or other systems using the @@ -6,62 +10,114 @@ Kubernetes API (such as OpenShift). Each project can be configured to connect to a different Kubernetes cluster, see the [configuration](#configuration) section. -If you have a single cluster that you want to use for all your projects, -you can pre-fill the settings page with a default template. To configure the -template, see the [Services Templates](services_templates.md) document. - ## Configuration Navigate to the [Integrations page](project_services.md#accessing-the-project-services) -of your project and select the **Kubernetes** service to configure it. +of your project and select the **Kubernetes** service to configure it. Fill in +all the needed parameters, check the "Active" checkbox and hit **Save changes** +for the changes to take effect.  -The Kubernetes service takes the following arguments: - -1. API URL -1. Custom CA bundle -1. Kubernetes namespace -1. Service token - -The API URL is the URL that GitLab uses to access the Kubernetes API. Kubernetes -exposes several APIs - we want the "base" URL that is common to all of them, -e.g., `https://kubernetes.example.com` rather than `https://kubernetes.example.com/api/v1`. - -GitLab authenticates against Kubernetes using service tokens, which are -scoped to a particular `namespace`. If you don't have a service token yet, -you can follow the -[Kubernetes documentation](http://kubernetes.io/docs/user-guide/service-accounts/) -to create one. You can also view or create service tokens in the -[Kubernetes dashboard](http://kubernetes.io/docs/user-guide/ui/) - visit -`Config -> Secrets`. - -Fill in the service token and namespace according to the values you just got. -If the API is using a self-signed TLS certificate, you'll also need to include -the `ca.crt` contents as the `Custom CA bundle`. +The Kubernetes service takes the following parameters: + +- **API URL** - + It's the URL that GitLab uses to access the Kubernetes API. Kubernetes + exposes several APIs, we want the "base" URL that is common to all of them, + e.g., `https://kubernetes.example.com` rather than `https://kubernetes.example.com/api/v1`. +- **CA certificate** (optional) - + If the API is using a self-signed TLS certificate, you'll also need to include + the `ca.crt` contents here. +- **Project namespace** (optional) - The following apply: + - By default you don't have to fill it in; by leaving it blank, GitLab will + create one for you. + - Each project should have a unique namespace. + - The project namespace is not necessarily the namespace of the secret, if + you're using a secret with broader permissions, like the secret from `default`. + - You should **not** use `default` as the project namespace. + - If you or someone created a secret specifically for the project, usually + with limited permissions, the secret's namespace and project namespace may + be the same. +- **Token** - + GitLab authenticates against Kubernetes using service tokens, which are + scoped to a particular `namespace`. If you don't have a service token yet, + you can follow the + [Kubernetes documentation](https://kubernetes.io/docs/tasks/configure-pod-container/configure-service-account/) + to create one. You can also view or create service tokens in the + [Kubernetes dashboard](https://kubernetes.io/docs/tasks/access-application-cluster/web-ui-dashboard/#config) + (under **Config > Secrets**). + +TIP: **Tip:** +If you have a single cluster that you want to use for all your projects, +you can pre-fill the settings page with a default template. To configure the +template, see [Services Templates](services_templates.md). ## Deployment variables -The Kubernetes service exposes following +The Kubernetes service exposes the following [deployment variables](../../../ci/variables/README.md#deployment-variables) in the -GitLab CI build environment: +GitLab CI/CD build environment: -- `KUBE_URL` - equal to the API URL -- `KUBE_TOKEN` +- `KUBE_URL` - Equal to the API URL. +- `KUBE_TOKEN` - The Kubernetes token. - `KUBE_NAMESPACE` - The Kubernetes namespace is auto-generated if not specified. The default value is `<project_name>-<project_id>`. You can overwrite it to use different one if needed, otherwise the `KUBE_NAMESPACE` variable will receive the default value. -- `KUBE_CA_PEM_FILE` - only present if a custom CA bundle was specified. Path +- `KUBE_CA_PEM_FILE` - Only present if a custom CA bundle was specified. Path to a file containing PEM data. -- `KUBE_CA_PEM` (deprecated)- only if a custom CA bundle was specified. Raw PEM data. -- `KUBECONFIG` - Path to a file containing kubeconfig for this deployment. CA bundle would be embedded if specified. +- `KUBE_CA_PEM` (deprecated) - Only if a custom CA bundle was specified. Raw PEM data. +- `KUBECONFIG` - Path to a file containing `kubeconfig` for this deployment. + CA bundle would be embedded if specified. + +## What you can get with the Kubernetes integration + +Here's what you can do with GitLab if you enable the Kubernetes integration. + +### Deploy Boards (EEP) + +> Available in [GitLab Enterprise Edition Premium][ee]. -## Web terminals +GitLab's Deploy Boards offer a consolidated view of the current health and +status of each CI [environment](../../../ci/environments.md) running on Kubernetes, +displaying the status of the pods in the deployment. Developers and other +teammates can view the progress and status of a rollout, pod by pod, in the +workflow they already use without any need to access Kubernetes. ->**NOTE:** -Added in GitLab 8.15. You must be the project owner or have `master` permissions -to use terminals. Support is currently limited to the first container in the +[> Read more about Deploy Boards](https://docs.gitlab.com/ee/user/project/deploy_boards.html) + +### Canary Deployments (EEP) + +> Available in [GitLab Enterprise Edition Premium][ee]. + +Leverage [Kubernetes' Canary deployments](https://kubernetes.io/docs/concepts/cluster-administration/manage-deployment/#canary-deployments) +and visualize your canary deployments right inside the Deploy Board, without +the need to leave GitLab. + +[> Read more about Canary Deployments](https://docs.gitlab.com/ee/user/project/canary_deployments.html) + +### Kubernetes monitoring + +Automatically detect and monitor Kubernetes metrics. Automatic monitoring of +[NGINX ingress](./prometheus_library/nginx.md) is also supported. + +[> Read more about Kubernetes monitoring](prometheus_library/kubernetes.md) + +### Auto DevOps + +Auto DevOps automatically detects, builds, tests, deploys, and monitors your +applications. + +To make full use of Auto DevOps(Auto Deploy, Auto Review Apps, and Auto Monitoring) +you will need the Kubernetes project integration enabled. + +[> Read more about Auto DevOps](../../../topics/autodevops/index.md) + +### Web terminals + +NOTE: **Note:** +Introduced in GitLab 8.15. You must be the project owner or have `master` permissions +to use terminals. Support is limited to the first container in the first pod of your environment. When enabled, the Kubernetes service adds [web terminal](../../../ci/environments.md#web-terminals) @@ -70,3 +126,5 @@ Docker and Kubernetes, so you get a new shell session within your existing containers. To use this integration, you should deploy to Kubernetes using the deployment variables above, ensuring any pods you create are labelled with `app=$CI_ENVIRONMENT_SLUG`. GitLab will do the rest! + +[ee]: https://about.gitlab.com/gitlab-ee/ diff --git a/doc/user/project/integrations/prometheus_library/cloudwatch.md b/doc/user/project/integrations/prometheus_library/cloudwatch.md index cc5cee36d28..34a0b97a171 100644 --- a/doc/user/project/integrations/prometheus_library/cloudwatch.md +++ b/doc/user/project/integrations/prometheus_library/cloudwatch.md @@ -1,8 +1,13 @@ # Monitoring AWS Resources + > [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/12621) in GitLab 9.4 GitLab has support for automatically detecting and monitoring AWS resources, starting with the [Elastic Load Balancer](https://aws.amazon.com/elasticloadbalancing/). This is provided by leveraging the official [Cloudwatch exporter](https://github.com/prometheus/cloudwatch_exporter), which translates [Cloudwatch metrics](https://aws.amazon.com/cloudwatch/) into a Prometheus readable form. +## Requirements + +The [Prometheus service](../prometheus/index.md) must be enabled. + ## Metrics supported | Name | Query | diff --git a/doc/user/project/integrations/prometheus_library/haproxy.md b/doc/user/project/integrations/prometheus_library/haproxy.md index d4b5911a91c..518018e5839 100644 --- a/doc/user/project/integrations/prometheus_library/haproxy.md +++ b/doc/user/project/integrations/prometheus_library/haproxy.md @@ -3,6 +3,10 @@ GitLab has support for automatically detecting and monitoring HAProxy. This is provided by leveraging the [HAProxy Exporter](https://github.com/prometheus/haproxy_exporter), which translates HAProxy statistics into a Prometheus readable form. +## Requirements + +The [Prometheus service](../prometheus/index.md) must be enabled. + ## Metrics supported | Name | Query | diff --git a/doc/user/project/integrations/prometheus_library/kubernetes.md b/doc/user/project/integrations/prometheus_library/kubernetes.md index 4d39ae0c4fa..518683965e8 100644 --- a/doc/user/project/integrations/prometheus_library/kubernetes.md +++ b/doc/user/project/integrations/prometheus_library/kubernetes.md @@ -1,7 +1,13 @@ # Monitoring Kubernetes + > [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/8935) in GitLab 9.0 -GitLab has support for automatically detecting and monitoring Kubernetes metrics. Kubernetes exposes Node level metrics out of the box via the built-in [Prometheus metrics support in cAdvisor](https://github.com/google/cadvisor). No additional services or exporters are needed. +GitLab has support for automatically detecting and monitoring Kubernetes metrics. + +## Requirements + +The [Prometheus](../prometheus.md) and [Kubernetes](../kubernetes.md) +integration services must be enabled. ## Metrics supported @@ -23,4 +29,4 @@ Prometheus server up and running. You have two options here: In order to isolate and only display relevant metrics for a given environment however, GitLab needs a method to detect which labels are associated. To do this, GitLab will [look for an `environment` label](metrics.md#identifying-environments). -If you are using [GitLab Auto-Deploy][../../../ci/autodeploy/index.md] and one of the two [provided Kubernetes monitoring solutions](../prometheus.md#getting-started-with-prometheus-monitoring), the `environment` label will be automatically added. +If you are using [GitLab Auto-Deploy](../../../../ci/autodeploy/index.md) and one of the two [provided Kubernetes monitoring solutions](../prometheus.md#getting-started-with-prometheus-monitoring), the `environment` label will be automatically added. diff --git a/doc/user/project/integrations/prometheus_library/nginx.md b/doc/user/project/integrations/prometheus_library/nginx.md index bab22f9a384..7fb8369d3c1 100644 --- a/doc/user/project/integrations/prometheus_library/nginx.md +++ b/doc/user/project/integrations/prometheus_library/nginx.md @@ -1,8 +1,13 @@ # Monitoring NGINX + > [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/12621) in GitLab 9.4 GitLab has support for automatically detecting and monitoring NGINX. This is provided by leveraging the [NGINX VTS exporter](https://github.com/hnlq715/nginx-vts-exporter), which translates [VTS statistics](https://github.com/vozlt/nginx-module-vts) into a Prometheus readable form. +## Requirements + +The [Prometheus service](../prometheus/index.md) must be enabled. + ## Metrics supported | Name | Query | diff --git a/doc/user/project/integrations/prometheus_library/nginx_ingress.md b/doc/user/project/integrations/prometheus_library/nginx_ingress.md index 17a47cfa646..e6f13d0630b 100644 --- a/doc/user/project/integrations/prometheus_library/nginx_ingress.md +++ b/doc/user/project/integrations/prometheus_library/nginx_ingress.md @@ -1,8 +1,13 @@ # Monitoring NGINX Ingress Controller + > [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/13438) in GitLab 9.5 GitLab has support for automatically detecting and monitoring the Kubernetes NGINX ingress controller. This is provided by leveraging the built in Prometheus metrics included in [version 0.9.0](https://github.com/kubernetes/ingress/blob/master/controllers/nginx/Changelog.md#09-beta1) of the ingress. +## Requirements + +The [Prometheus service](../prometheus/index.md) must be enabled. + ## Metrics supported | Name | Query | diff --git a/doc/user/project/repository/gpg_signed_commits/index.md b/doc/user/project/repository/gpg_signed_commits/index.md index dfe43c6b691..29e04a0ccf0 100644 --- a/doc/user/project/repository/gpg_signed_commits/index.md +++ b/doc/user/project/repository/gpg_signed_commits/index.md @@ -113,25 +113,25 @@ started: 1. Use the following command to list the private GPG key you just created: ``` - gpg --list-secret-keys --keyid-format 0xLONG mr@robot.sh + gpg --list-secret-keys --keyid-format LONG mr@robot.sh ``` Replace `mr@robot.sh` with the email address you entered above. 1. Copy the GPG key ID that starts with `sec`. In the following example, that's - `0x30F2B65B9246B6CA`: + `30F2B65B9246B6CA`: ``` - sec rsa4096/0x30F2B65B9246B6CA 2017-08-18 [SC] + sec rsa4096/30F2B65B9246B6CA 2017-08-18 [SC] D5E4F29F3275DC0CDA8FFC8730F2B65B9246B6CA uid [ultimate] Mr. Robot <mr@robot.sh> - ssb rsa4096/0xB7ABC0813E4028C0 2017-08-18 [E] + ssb rsa4096/B7ABC0813E4028C0 2017-08-18 [E] ``` 1. Export the public key of that ID (replace your key ID from the previous step): ``` - gpg --armor --export 0x30F2B65B9246B6CA + gpg --armor --export 30F2B65B9246B6CA ``` 1. Finally, copy the public key and [add it in your profile settings](#adding-a-gpg-key-to-your-account) @@ -167,28 +167,28 @@ key to use. 1. Use the following command to list the private GPG key you just created: ``` - gpg --list-secret-keys --keyid-format 0xLONG mr@robot.sh + gpg --list-secret-keys --keyid-format LONG mr@robot.sh ``` Replace `mr@robot.sh` with the email address you entered above. 1. Copy the GPG key ID that starts with `sec`. In the following example, that's - `0x30F2B65B9246B6CA`: + `30F2B65B9246B6CA`: ``` - sec rsa4096/0x30F2B65B9246B6CA 2017-08-18 [SC] + sec rsa4096/30F2B65B9246B6CA 2017-08-18 [SC] D5E4F29F3275DC0CDA8FFC8730F2B65B9246B6CA uid [ultimate] Mr. Robot <mr@robot.sh> - ssb rsa4096/0xB7ABC0813E4028C0 2017-08-18 [E] + ssb rsa4096/B7ABC0813E4028C0 2017-08-18 [E] ``` 1. Tell Git to use that key to sign the commits: ``` - git config --global user.signingkey 0x30F2B65B9246B6CA + git config --global user.signingkey 30F2B65B9246B6CA ``` - Replace `0x30F2B65B9246B6CA` with your GPG key ID. + Replace `30F2B65B9246B6CA` with your GPG key ID. ## Signing commits diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb index c4c0fdda665..e79f988f549 100644 --- a/lib/api/api_guard.rb +++ b/lib/api/api_guard.rb @@ -75,7 +75,7 @@ module API raise RevokedError when AccessTokenValidationService::VALID - @current_user = User.find(access_token.resource_owner_id) + User.find(access_token.resource_owner_id) end end @@ -84,11 +84,13 @@ module API return nil unless token_string.present? - find_user_by_authentication_token(token_string) || find_user_by_personal_access_token(token_string, scopes) - end + user = + find_user_by_authentication_token(token_string) || + find_user_by_personal_access_token(token_string, scopes) + + raise UnauthorizedError unless user - def current_user - @current_user + user end private @@ -107,7 +109,16 @@ module API end def find_access_token - @access_token ||= Doorkeeper.authenticate(doorkeeper_request, Doorkeeper.configuration.access_token_methods) + return @access_token if defined?(@access_token) + + token = Doorkeeper::OAuth::Token.from_request(doorkeeper_request, *Doorkeeper.configuration.access_token_methods) + return @access_token = nil unless token + + @access_token = Doorkeeper::AccessToken.by_token(token) + raise UnauthorizedError unless @access_token + + @access_token.revoke_previous_refresh_token! + @access_token end def doorkeeper_request @@ -169,6 +180,7 @@ module API TokenNotFoundError = Class.new(StandardError) ExpiredError = Class.new(StandardError) RevokedError = Class.new(StandardError) + UnauthorizedError = Class.new(StandardError) class InsufficientScopeError < StandardError attr_reader :scopes diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 71253f72533..7f4736a08cb 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -89,6 +89,9 @@ module API expose :ssh_url_to_repo, :http_url_to_repo, :web_url expose :name, :name_with_namespace expose :path, :path_with_namespace + expose :avatar_url do |project, options| + project.avatar_url(only_path: false) + end expose :star_count, :forks_count expose :created_at, :last_activity_at end @@ -146,9 +149,7 @@ module API expose :forked_from_project, using: Entities::BasicProjectDetails, if: lambda { |project, options| project.forked? } expose :import_status expose :import_error, if: lambda { |_project, options| options[:user_can_admin_project] } - expose :avatar_url do |user, options| - user.avatar_url(only_path: false) - end + expose :open_issues_count, if: lambda { |project, options| project.feature_available?(:issues, options[:current_user]) } expose :runners_token, if: lambda { |_project, options| options[:user_can_admin_project] } expose :public_builds, as: :public_jobs @@ -193,8 +194,8 @@ module API class Group < Grape::Entity expose :id, :name, :path, :description, :visibility expose :lfs_enabled?, as: :lfs_enabled - expose :avatar_url do |user, options| - user.avatar_url(only_path: false) + expose :avatar_url do |group, options| + group.avatar_url(only_path: false) end expose :web_url expose :request_access_enabled @@ -234,6 +235,7 @@ module API class RepoCommitDetail < RepoCommit expose :stats, using: Entities::RepoCommitStats expose :status + expose :last_pipeline, using: 'API::Entities::PipelineBasic' end class RepoBranch < Grape::Entity diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 00dbc2aee7a..1e8475ba3ec 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -3,6 +3,8 @@ module API include Gitlab::Utils include Helpers::Pagination + UnauthorizedError = Class.new(StandardError) + SUDO_HEADER = "HTTP_SUDO".freeze SUDO_PARAM = :sudo @@ -139,7 +141,7 @@ module API end def authenticate! - unauthorized! unless current_user && can?(initial_current_user, :access_api) + unauthorized! unless current_user end def authenticate_non_get! @@ -397,19 +399,27 @@ module API def initial_current_user return @initial_current_user if defined?(@initial_current_user) - Gitlab::Auth::UniqueIpsLimiter.limit_user! do - @initial_current_user ||= find_user_by_private_token(scopes: scopes_registered_for_endpoint) - @initial_current_user ||= doorkeeper_guard(scopes: scopes_registered_for_endpoint) - @initial_current_user ||= find_user_from_warden - - unless @initial_current_user && Gitlab::UserAccess.new(@initial_current_user).allowed? - @initial_current_user = nil - end - @initial_current_user + begin + @initial_current_user = Gitlab::Auth::UniqueIpsLimiter.limit_user! { find_current_user } + rescue APIGuard::UnauthorizedError, UnauthorizedError + unauthorized! end end + def find_current_user + user = + find_user_by_private_token(scopes: scopes_registered_for_endpoint) || + doorkeeper_guard(scopes: scopes_registered_for_endpoint) || + find_user_from_warden + + return nil unless user + + raise UnauthorizedError unless Gitlab::UserAccess.new(user).allowed? && user.can?(:access_api) + + user + end + def sudo! return unless sudo_identifier return unless initial_current_user diff --git a/lib/api/internal.rb b/lib/api/internal.rb index c0fef56378f..a0557a609ca 100644 --- a/lib/api/internal.rb +++ b/lib/api/internal.rb @@ -136,7 +136,7 @@ module API codes = nil - ::Users::UpdateService.new(user).execute! do |user| + ::Users::UpdateService.new(current_user, user: user).execute! do |user| codes = user.generate_otp_backup_codes! end diff --git a/lib/api/notification_settings.rb b/lib/api/notification_settings.rb index bcc0833aa5c..0266bf2f717 100644 --- a/lib/api/notification_settings.rb +++ b/lib/api/notification_settings.rb @@ -35,7 +35,7 @@ module API new_notification_email = params.delete(:notification_email) if new_notification_email - ::Users::UpdateService.new(current_user, notification_email: new_notification_email).execute + ::Users::UpdateService.new(current_user, user: current_user, notification_email: new_notification_email).execute end notification_setting.update(declared_params(include_missing: false)) diff --git a/lib/api/users.rb b/lib/api/users.rb index bdebda58d3f..99024d1f0ad 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -11,7 +11,7 @@ module API end helpers do - def find_user(params) + def find_user_by_id(params) id = params[:user_id] || params[:id] User.find_by(id: id) || not_found!('User') end @@ -166,7 +166,7 @@ module API user_params[:password_expires_at] = Time.now if user_params[:password].present? - result = ::Users::UpdateService.new(user, user_params.except(:extern_uid, :provider)).execute + result = ::Users::UpdateService.new(current_user, user_params.except(:extern_uid, :provider).merge(user: user)).execute if result[:status] == :success present user, with: Entities::UserPublic @@ -326,7 +326,7 @@ module API user = User.find_by(id: params.delete(:id)) not_found!('User') unless user - email = Emails::CreateService.new(user, declared_params(include_missing: false)).execute + email = Emails::CreateService.new(current_user, declared_params(include_missing: false).merge(user: user)).execute if email.errors.blank? NotificationService.new.new_email(email) @@ -367,7 +367,7 @@ module API not_found!('Email') unless email destroy_conditionally!(email) do |email| - Emails::DestroyService.new(current_user, email: email.email).execute + Emails::DestroyService.new(current_user, user: user, email: email.email).execute end user.update_secondary_emails! @@ -430,7 +430,7 @@ module API resource :impersonation_tokens do helpers do def finder(options = {}) - user = find_user(params) + user = find_user_by_id(params) PersonalAccessTokensFinder.new({ user: user, impersonation: true }.merge(options)) end @@ -672,7 +672,7 @@ module API requires :email, type: String, desc: 'The new email' end post "emails" do - email = Emails::CreateService.new(current_user, declared_params).execute + email = Emails::CreateService.new(current_user, declared_params.merge(user: current_user)).execute if email.errors.blank? NotificationService.new.new_email(email) @@ -691,7 +691,7 @@ module API not_found!('Email') unless email destroy_conditionally!(email) do |email| - Emails::DestroyService.new(current_user, email: email.email).execute + Emails::DestroyService.new(current_user, user: current_user, email: email.email).execute end current_user.update_secondary_emails! diff --git a/lib/github/client.rb b/lib/github/client.rb index 9c476df7d46..29bd9c1f39e 100644 --- a/lib/github/client.rb +++ b/lib/github/client.rb @@ -1,6 +1,7 @@ module Github class Client TIMEOUT = 60 + DEFAULT_PER_PAGE = 100 attr_reader :connection, :rate_limit @@ -20,7 +21,7 @@ module Github exceed, reset_in = rate_limit.get sleep reset_in if exceed - Github::Response.new(connection.get(url, query)) + Github::Response.new(connection.get(url, { per_page: DEFAULT_PER_PAGE }.merge(query))) end private diff --git a/lib/github/import.rb b/lib/github/import.rb index 9354e142d3d..f5f62dc8b6f 100644 --- a/lib/github/import.rb +++ b/lib/github/import.rb @@ -202,13 +202,8 @@ module Github merge_request.save!(validate: false) merge_request.merge_request_diffs.create - # Fetch review comments review_comments_url = "/repos/#{repo}/pulls/#{pull_request.iid}/comments" fetch_comments(merge_request, :review_comment, review_comments_url, LegacyDiffNote) - - # Fetch comments - comments_url = "/repos/#{repo}/issues/#{pull_request.iid}/comments" - fetch_comments(merge_request, :comment, comments_url) rescue => e error(:pull_request, pull_request.url, e.message) ensure @@ -241,12 +236,17 @@ module Github # for both features, like manipulating assignees, labels # and milestones, are provided within the Issues API. if representation.pull_request? - return unless representation.has_labels? + return unless representation.has_labels? || representation.has_comments? merge_request = MergeRequest.find_by!(target_project_id: project.id, iid: representation.iid) - merge_request.update_attribute(:label_ids, label_ids(representation.labels)) + + if representation.has_labels? + merge_request.update_attribute(:label_ids, label_ids(representation.labels)) + end + + fetch_comments_conditionally(merge_request, representation) else - return if Issue.where(iid: representation.iid, project_id: project.id).exists? + return if Issue.exists?(iid: representation.iid, project_id: project.id) author_id = user_id(representation.author, project.creator_id) issue = Issue.new @@ -263,17 +263,20 @@ module Github issue.updated_at = representation.updated_at issue.save!(validate: false) - # Fetch comments - if representation.has_comments? - comments_url = "/repos/#{repo}/issues/#{issue.iid}/comments" - fetch_comments(issue, :comment, comments_url) - end + fetch_comments_conditionally(issue, representation) end rescue => e error(:issue, representation.url, e.message) end end + def fetch_comments_conditionally(issuable, representation) + if representation.has_comments? + comments_url = "/repos/#{repo}/issues/#{issuable.iid}/comments" + fetch_comments(issuable, :comment, comments_url) + end + end + def fetch_comments(noteable, type, url, klass = Note) while url comments = Github::Client.new(options).get(url) diff --git a/lib/gitlab/bitbucket_import/importer.rb b/lib/gitlab/bitbucket_import/importer.rb index 28bbf3b384e..d1979bb7ed3 100644 --- a/lib/gitlab/bitbucket_import/importer.rb +++ b/lib/gitlab/bitbucket_import/importer.rb @@ -149,16 +149,21 @@ module Gitlab description += @formatter.author_line(pull_request.author) unless find_user_id(pull_request.author) description += pull_request.description + source_branch_sha = pull_request.source_branch_sha + target_branch_sha = pull_request.target_branch_sha + source_branch_sha = project.repository.commit(source_branch_sha)&.sha || source_branch_sha + target_branch_sha = project.repository.commit(target_branch_sha)&.sha || target_branch_sha + merge_request = project.merge_requests.create!( iid: pull_request.iid, title: pull_request.title, description: description, source_project: project, source_branch: pull_request.source_branch_name, - source_branch_sha: pull_request.source_branch_sha, + source_branch_sha: source_branch_sha, target_project: project, target_branch: pull_request.target_branch_name, - target_branch_sha: pull_request.target_branch_sha, + target_branch_sha: target_branch_sha, state: pull_request.state, author_id: gitlab_user_id(project, pull_request.author), assignee_id: nil, diff --git a/lib/gitlab/ci/pipeline/chain/base.rb b/lib/gitlab/ci/pipeline/chain/base.rb new file mode 100644 index 00000000000..8d82e1b288d --- /dev/null +++ b/lib/gitlab/ci/pipeline/chain/base.rb @@ -0,0 +1,27 @@ +module Gitlab + module Ci + module Pipeline + module Chain + class Base + attr_reader :pipeline, :project, :current_user + + def initialize(pipeline, command) + @pipeline = pipeline + @command = command + + @project = command.project + @current_user = command.current_user + end + + def perform! + raise NotImplementedError + end + + def break? + raise NotImplementedError + end + end + end + end + end +end diff --git a/lib/gitlab/ci/pipeline/chain/create.rb b/lib/gitlab/ci/pipeline/chain/create.rb new file mode 100644 index 00000000000..d5e17a123df --- /dev/null +++ b/lib/gitlab/ci/pipeline/chain/create.rb @@ -0,0 +1,29 @@ +module Gitlab + module Ci + module Pipeline + module Chain + class Create < Chain::Base + include Chain::Helpers + + def perform! + ::Ci::Pipeline.transaction do + pipeline.save! + + @command.seeds_block&.call(pipeline) + + ::Ci::CreatePipelineStagesService + .new(project, current_user) + .execute(pipeline) + end + rescue ActiveRecord::RecordInvalid => e + error("Failed to persist the pipeline: #{e}") + end + + def break? + !pipeline.persisted? + end + end + end + end + end +end diff --git a/lib/gitlab/ci/pipeline/chain/helpers.rb b/lib/gitlab/ci/pipeline/chain/helpers.rb new file mode 100644 index 00000000000..02d81286f21 --- /dev/null +++ b/lib/gitlab/ci/pipeline/chain/helpers.rb @@ -0,0 +1,25 @@ +module Gitlab + module Ci + module Pipeline + module Chain + module Helpers + def branch_exists? + return @is_branch if defined?(@is_branch) + + @is_branch = project.repository.branch_exists?(pipeline.ref) + end + + def tag_exists? + return @is_tag if defined?(@is_tag) + + @is_tag = project.repository.tag_exists?(pipeline.ref) + end + + def error(message) + pipeline.errors.add(:base, message) + end + end + end + end + end +end diff --git a/lib/gitlab/ci/pipeline/chain/sequence.rb b/lib/gitlab/ci/pipeline/chain/sequence.rb new file mode 100644 index 00000000000..015f2988327 --- /dev/null +++ b/lib/gitlab/ci/pipeline/chain/sequence.rb @@ -0,0 +1,36 @@ +module Gitlab + module Ci + module Pipeline + module Chain + class Sequence + def initialize(pipeline, command, sequence) + @pipeline = pipeline + @completed = [] + + @sequence = sequence.map do |chain| + chain.new(pipeline, command) + end + end + + def build! + @sequence.each do |step| + step.perform! + + break if step.break? + + @completed << step + end + + @pipeline.tap do + yield @pipeline, self if block_given? + end + end + + def complete? + @completed.size == @sequence.size + end + end + end + end + end +end diff --git a/lib/gitlab/ci/pipeline/chain/skip.rb b/lib/gitlab/ci/pipeline/chain/skip.rb new file mode 100644 index 00000000000..9a72de87bab --- /dev/null +++ b/lib/gitlab/ci/pipeline/chain/skip.rb @@ -0,0 +1,33 @@ +module Gitlab + module Ci + module Pipeline + module Chain + class Skip < Chain::Base + SKIP_PATTERN = /\[(ci[ _-]skip|skip[ _-]ci)\]/i + + def perform! + if skipped? + @pipeline.skip if @command.save_incompleted + end + end + + def skipped? + !@command.ignore_skip_ci && commit_message_skips_ci? + end + + def break? + skipped? + end + + private + + def commit_message_skips_ci? + return false unless @pipeline.git_commit_message + + @skipped ||= !!(@pipeline.git_commit_message =~ SKIP_PATTERN) + end + end + end + end + end +end diff --git a/lib/gitlab/ci/pipeline/chain/validate/abilities.rb b/lib/gitlab/ci/pipeline/chain/validate/abilities.rb new file mode 100644 index 00000000000..4913a604079 --- /dev/null +++ b/lib/gitlab/ci/pipeline/chain/validate/abilities.rb @@ -0,0 +1,54 @@ +module Gitlab + module Ci + module Pipeline + module Chain + module Validate + class Abilities < Chain::Base + include Gitlab::Allowable + include Chain::Helpers + + def perform! + unless project.builds_enabled? + return error('Pipelines are disabled!') + end + + unless allowed_to_trigger_pipeline? + if can?(current_user, :create_pipeline, project) + return error("Insufficient permissions for protected ref '#{pipeline.ref}'") + else + return error('Insufficient permissions to create a new pipeline') + end + end + end + + def break? + @pipeline.errors.any? + end + + def allowed_to_trigger_pipeline? + if current_user + allowed_to_create? + else # legacy triggers don't have a corresponding user + !project.protected_for?(@pipeline.ref) + end + end + + def allowed_to_create? + return unless can?(current_user, :create_pipeline, project) + + access = Gitlab::UserAccess.new(current_user, project: project) + + if branch_exists? + access.can_update_branch?(@pipeline.ref) + elsif tag_exists? + access.can_create_tag?(@pipeline.ref) + else + true # Allow it for now and we'll reject when we check ref existence + end + end + end + end + end + end + end +end diff --git a/lib/gitlab/ci/pipeline/chain/validate/config.rb b/lib/gitlab/ci/pipeline/chain/validate/config.rb new file mode 100644 index 00000000000..489bcd79655 --- /dev/null +++ b/lib/gitlab/ci/pipeline/chain/validate/config.rb @@ -0,0 +1,35 @@ +module Gitlab + module Ci + module Pipeline + module Chain + module Validate + class Config < Chain::Base + include Chain::Helpers + + def perform! + unless @pipeline.config_processor + unless @pipeline.ci_yaml_file + return error("Missing #{@pipeline.ci_yaml_file_path} file") + end + + if @command.save_incompleted && @pipeline.has_yaml_errors? + @pipeline.drop + end + + return error(@pipeline.yaml_errors) + end + + unless @pipeline.has_stage_seeds? + return error('No stages / jobs for this pipeline.') + end + end + + def break? + @pipeline.errors.any? || @pipeline.persisted? + end + end + end + end + end + end +end diff --git a/lib/gitlab/ci/pipeline/chain/validate/repository.rb b/lib/gitlab/ci/pipeline/chain/validate/repository.rb new file mode 100644 index 00000000000..70a4cfdbdea --- /dev/null +++ b/lib/gitlab/ci/pipeline/chain/validate/repository.rb @@ -0,0 +1,30 @@ +module Gitlab + module Ci + module Pipeline + module Chain + module Validate + class Repository < Chain::Base + include Chain::Helpers + + def perform! + unless branch_exists? || tag_exists? + return error('Reference not found') + end + + ## TODO, we check commit in the service, that is why + # there is no repository access here. + # + unless pipeline.sha + return error('Commit not found') + end + end + + def break? + @pipeline.errors.any? + end + end + end + end + end + end +end diff --git a/lib/gitlab/ci/pipeline/duration.rb b/lib/gitlab/ci/pipeline/duration.rb new file mode 100644 index 00000000000..469fc094cc8 --- /dev/null +++ b/lib/gitlab/ci/pipeline/duration.rb @@ -0,0 +1,143 @@ +module Gitlab + module Ci + module Pipeline + # # Introduction - total running time + # + # The problem this module is trying to solve is finding the total running + # time amongst all the jobs, excluding retries and pending (queue) time. + # We could reduce this problem down to finding the union of periods. + # + # So each job would be represented as a `Period`, which consists of + # `Period#first` as when the job started and `Period#last` as when the + # job was finished. A simple example here would be: + # + # * A (1, 3) + # * B (2, 4) + # * C (6, 7) + # + # Here A begins from 1, and ends to 3. B begins from 2, and ends to 4. + # C begins from 6, and ends to 7. Visually it could be viewed as: + # + # 0 1 2 3 4 5 6 7 + # AAAAAAA + # BBBBBBB + # CCCC + # + # The union of A, B, and C would be (1, 4) and (6, 7), therefore the + # total running time should be: + # + # (4 - 1) + (7 - 6) => 4 + # + # # The Algorithm + # + # The algorithm used here for union would be described as follow. + # First we make sure that all periods are sorted by `Period#first`. + # Then we try to merge periods by iterating through the first period + # to the last period. The goal would be merging all overlapped periods + # so that in the end all the periods are discrete. When all periods + # are discrete, we're free to just sum all the periods to get real + # running time. + # + # Here we begin from A, and compare it to B. We could find that + # before A ends, B already started. That is `B.first <= A.last` + # that is `2 <= 3` which means A and B are overlapping! + # + # When we found that two periods are overlapping, we would need to merge + # them into a new period and disregard the old periods. To make a new + # period, we take `A.first` as the new first because remember? we sorted + # them, so `A.first` must be smaller or equal to `B.first`. And we take + # `[A.last, B.last].max` as the new last because we want whoever ended + # later. This could be broken into two cases: + # + # 0 1 2 3 4 + # AAAAAAA + # BBBBBBB + # + # Or: + # + # 0 1 2 3 4 + # AAAAAAAAAA + # BBBB + # + # So that we need to take whoever ends later. Back to our example, + # after merging and discard A and B it could be visually viewed as: + # + # 0 1 2 3 4 5 6 7 + # DDDDDDDDDD + # CCCC + # + # Now we could go on and compare the newly created D and the old C. + # We could figure out that D and C are not overlapping by checking + # `C.first <= D.last` is `false`. Therefore we need to keep both C + # and D. The example would end here because there are no more jobs. + # + # After having the union of all periods, we just need to sum the length + # of all periods to get total time. + # + # (4 - 1) + (7 - 6) => 4 + # + # That is 4 is the answer in the example. + module Duration + extend self + + Period = Struct.new(:first, :last) do + def duration + last - first + end + end + + def from_pipeline(pipeline) + status = %w[success failed running canceled] + builds = pipeline.builds.latest + .where(status: status).where.not(started_at: nil).order(:started_at) + + from_builds(builds) + end + + def from_builds(builds) + now = Time.now + + periods = builds.map do |b| + Period.new(b.started_at, b.finished_at || now) + end + + from_periods(periods) + end + + # periods should be sorted by `first` + def from_periods(periods) + process_duration(process_periods(periods)) + end + + private + + def process_periods(periods) + return periods if periods.empty? + + periods.drop(1).inject([periods.first]) do |result, current| + previous = result.last + + if overlap?(previous, current) + result[-1] = merge(previous, current) + result + else + result << current + end + end + end + + def overlap?(previous, current) + current.first <= previous.last + end + + def merge(previous, current) + Period.new(previous.first, [previous.last, current.last].max) + end + + def process_duration(periods) + periods.sum(&:duration) + end + end + end + end +end diff --git a/lib/gitlab/ci/pipeline_duration.rb b/lib/gitlab/ci/pipeline_duration.rb deleted file mode 100644 index 3208cc2bef6..00000000000 --- a/lib/gitlab/ci/pipeline_duration.rb +++ /dev/null @@ -1,141 +0,0 @@ -module Gitlab - module Ci - # # Introduction - total running time - # - # The problem this module is trying to solve is finding the total running - # time amongst all the jobs, excluding retries and pending (queue) time. - # We could reduce this problem down to finding the union of periods. - # - # So each job would be represented as a `Period`, which consists of - # `Period#first` as when the job started and `Period#last` as when the - # job was finished. A simple example here would be: - # - # * A (1, 3) - # * B (2, 4) - # * C (6, 7) - # - # Here A begins from 1, and ends to 3. B begins from 2, and ends to 4. - # C begins from 6, and ends to 7. Visually it could be viewed as: - # - # 0 1 2 3 4 5 6 7 - # AAAAAAA - # BBBBBBB - # CCCC - # - # The union of A, B, and C would be (1, 4) and (6, 7), therefore the - # total running time should be: - # - # (4 - 1) + (7 - 6) => 4 - # - # # The Algorithm - # - # The algorithm used here for union would be described as follow. - # First we make sure that all periods are sorted by `Period#first`. - # Then we try to merge periods by iterating through the first period - # to the last period. The goal would be merging all overlapped periods - # so that in the end all the periods are discrete. When all periods - # are discrete, we're free to just sum all the periods to get real - # running time. - # - # Here we begin from A, and compare it to B. We could find that - # before A ends, B already started. That is `B.first <= A.last` - # that is `2 <= 3` which means A and B are overlapping! - # - # When we found that two periods are overlapping, we would need to merge - # them into a new period and disregard the old periods. To make a new - # period, we take `A.first` as the new first because remember? we sorted - # them, so `A.first` must be smaller or equal to `B.first`. And we take - # `[A.last, B.last].max` as the new last because we want whoever ended - # later. This could be broken into two cases: - # - # 0 1 2 3 4 - # AAAAAAA - # BBBBBBB - # - # Or: - # - # 0 1 2 3 4 - # AAAAAAAAAA - # BBBB - # - # So that we need to take whoever ends later. Back to our example, - # after merging and discard A and B it could be visually viewed as: - # - # 0 1 2 3 4 5 6 7 - # DDDDDDDDDD - # CCCC - # - # Now we could go on and compare the newly created D and the old C. - # We could figure out that D and C are not overlapping by checking - # `C.first <= D.last` is `false`. Therefore we need to keep both C - # and D. The example would end here because there are no more jobs. - # - # After having the union of all periods, we just need to sum the length - # of all periods to get total time. - # - # (4 - 1) + (7 - 6) => 4 - # - # That is 4 is the answer in the example. - module PipelineDuration - extend self - - Period = Struct.new(:first, :last) do - def duration - last - first - end - end - - def from_pipeline(pipeline) - status = %w[success failed running canceled] - builds = pipeline.builds.latest - .where(status: status).where.not(started_at: nil).order(:started_at) - - from_builds(builds) - end - - def from_builds(builds) - now = Time.now - - periods = builds.map do |b| - Period.new(b.started_at, b.finished_at || now) - end - - from_periods(periods) - end - - # periods should be sorted by `first` - def from_periods(periods) - process_duration(process_periods(periods)) - end - - private - - def process_periods(periods) - return periods if periods.empty? - - periods.drop(1).inject([periods.first]) do |result, current| - previous = result.last - - if overlap?(previous, current) - result[-1] = merge(previous, current) - result - else - result << current - end - end - end - - def overlap?(previous, current) - current.first <= previous.last - end - - def merge(previous, current) - Period.new(previous.first, [previous.last, current.last].max) - end - - def process_duration(periods) - periods.sum(&:duration) - end - end - end -end diff --git a/lib/gitlab/diff/diff_refs.rb b/lib/gitlab/diff/diff_refs.rb index 371cbe04b9b..c98eefbce25 100644 --- a/lib/gitlab/diff/diff_refs.rb +++ b/lib/gitlab/diff/diff_refs.rb @@ -13,9 +13,9 @@ module Gitlab def ==(other) other.is_a?(self.class) && - base_sha == other.base_sha && - start_sha == other.start_sha && - head_sha == other.head_sha + shas_equal?(base_sha, other.base_sha) && + shas_equal?(start_sha, other.start_sha) && + shas_equal?(head_sha, other.head_sha) end alias_method :eql?, :== @@ -47,6 +47,22 @@ module Gitlab CompareService.new(project, head_sha).execute(project, start_sha, straight: straight) end end + + private + + def shas_equal?(sha1, sha2) + return true if sha1 == sha2 + return false if sha1.nil? || sha2.nil? + return false unless sha1.class == sha2.class + + length = [sha1.length, sha2.length].min + + # If either of the shas is below the minimum length, we cannot be sure + # that they actually refer to the same commit because of hash collision. + return false if length < Commit::MIN_SHA_LENGTH + + sha1[0, length] == sha2[0, length] + end end end end diff --git a/lib/gitlab/diff/position.rb b/lib/gitlab/diff/position.rb index f80afb20f0c..b8db3adef0a 100644 --- a/lib/gitlab/diff/position.rb +++ b/lib/gitlab/diff/position.rb @@ -49,12 +49,13 @@ module Gitlab coder['attributes'] = self.to_h end - def key - @key ||= [base_sha, start_sha, head_sha, Digest::SHA1.hexdigest(old_path || ""), Digest::SHA1.hexdigest(new_path || ""), old_line, new_line] - end - def ==(other) - other.is_a?(self.class) && key == other.key + other.is_a?(self.class) && + other.diff_refs == diff_refs && + other.old_path == old_path && + other.new_path == new_path && + other.old_line == old_line && + other.new_line == new_line end def to_h diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 616b075c087..6baff362dad 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -73,8 +73,6 @@ module Gitlab delegate :empty?, to: :rugged - delegate :exists?, to: :gitaly_repository_client - def ==(other) path == other.path end @@ -102,6 +100,18 @@ module Gitlab @circuit_breaker ||= Gitlab::Git::Storage::CircuitBreaker.for_storage(storage) end + def exists? + Gitlab::GitalyClient.migrate(:repository_exists) do |enabled| + if enabled + gitaly_repository_client.exists? + else + circuit_breaker.perform do + File.exist?(File.join(@path, 'refs')) + end + end + end + end + # Returns an Array of branch names # sorted by name ASC def branch_names @@ -937,7 +947,11 @@ module Gitlab if start_repository == self yield commit(start_branch_name) else - sha = start_repository.commit(start_branch_name).sha + start_commit = start_repository.commit(start_branch_name) + + return yield nil unless start_commit + + sha = start_commit.sha if branch_commit = commit(sha) yield branch_commit @@ -966,8 +980,9 @@ module Gitlab with_repo_branch_commit(source_repository, source_branch) do |commit| if commit write_ref(local_ref, commit.sha) + true else - raise Rugged::ReferenceError, 'source repository is empty' + false end end end diff --git a/lib/gitlab/ldap/access.rb b/lib/gitlab/ldap/access.rb index fb68627dedf..e60ceba27c8 100644 --- a/lib/gitlab/ldap/access.rb +++ b/lib/gitlab/ldap/access.rb @@ -16,7 +16,7 @@ module Gitlab def self.allowed?(user) self.open(user) do |access| if access.allowed? - Users::UpdateService.new(user, last_credential_check_at: Time.now).execute + Users::UpdateService.new(user, user: user, last_credential_check_at: Time.now).execute true else diff --git a/lib/gitlab/o_auth/user.rb b/lib/gitlab/o_auth/user.rb index 7704bf715e4..e06d4dc45f7 100644 --- a/lib/gitlab/o_auth/user.rb +++ b/lib/gitlab/o_auth/user.rb @@ -32,7 +32,7 @@ module Gitlab block_after_save = needs_blocking? - Users::UpdateService.new(gl_user).execute! + Users::UpdateService.new(gl_user, user: gl_user).execute! gl_user.block if block_after_save diff --git a/locale/gitlab.pot b/locale/gitlab.pot index bc476a706cb..600432be3bf 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -8,8 +8,8 @@ msgid "" msgstr "" "Project-Id-Version: gitlab 1.0.0\n" "Report-Msgid-Bugs-To: \n" -"POT-Creation-Date: 2017-09-21 14:20+0530\n" -"PO-Revision-Date: 2017-09-21 14:20+0530\n" +"POT-Creation-Date: 2017-09-27 15:43+0100\n" +"PO-Revision-Date: 2017-09-27 15:43+0100\n" "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n" "Language-Team: LANGUAGE <LL@li.org>\n" "Language: \n" @@ -31,6 +31,9 @@ msgstr[1] "" msgid "%{commit_author_link} committed %{commit_timeago}" msgstr "" +msgid "%{number_commits_behind} commits behind %{default_branch}, %{number_commits_ahead} commits ahead" +msgstr "" + msgid "%{number_of_failures} of %{maximum_failures} failures. GitLab will allow access on the next attempt." msgstr "" @@ -131,28 +134,28 @@ msgstr "" msgid "Authentication Log" msgstr "" -msgid "Auto DevOps (Beta)" +msgid "Auto Review Apps and Auto Deploy need a domain name and the %{kubernetes} to work correctly." msgstr "" -msgid "Auto DevOps can be activated for this project. It will automatically build, test, and deploy your application based on a predefined CI/CD configuration." +msgid "Auto Review Apps and Auto Deploy need a domain name to work correctly." msgstr "" -msgid "Auto DevOps documentation" +msgid "Auto Review Apps and Auto Deploy need the %{kubernetes} to work correctly." msgstr "" -msgid "Auto Review Apps and Auto Deploy need a domain name and the %{kubernetes} to work correctly." +msgid "AutoDevOps|Auto DevOps (Beta)" msgstr "" -msgid "Auto Review Apps and Auto Deploy need a domain name to work correctly." +msgid "AutoDevOps|Auto DevOps can be activated for this project. It will automatically build, test, and deploy your application based on a predefined CI/CD configuration." msgstr "" -msgid "Auto Review Apps and Auto Deploy need the %{kubernetes} to work correctly." +msgid "AutoDevOps|Auto DevOps documentation" msgstr "" -msgid "AutoDevOps|Learn more in the" +msgid "AutoDevOps|Enable in settings" msgstr "" -msgid "Board" +msgid "AutoDevOps|Learn more in the %{link_to_documentation}" msgstr "" msgid "Branch" @@ -172,12 +175,81 @@ msgstr "" msgid "Branches" msgstr "" +msgid "Branches|Cant find HEAD commit for this branch" +msgstr "" + +msgid "Branches|Compare" +msgstr "" + +msgid "Branches|Delete all branches that are merged into '%{default_branch}'" +msgstr "" + +msgid "Branches|Delete branch" +msgstr "" + +msgid "Branches|Delete merged branches" +msgstr "" + +msgid "Branches|Delete protected branch" +msgstr "" + +msgid "Branches|Delete protected branch '%{branch_name}'?" +msgstr "" + +msgid "Branches|Deleting the '%{branch_name}' branch cannot be undone. Are you sure?" +msgstr "" + +msgid "Branches|Deleting the merged branches cannot be undone. Are you sure?" +msgstr "" + +msgid "Branches|Filter by branch name" +msgstr "" + +msgid "Branches|Merged into %{default_branch}" +msgstr "" + +msgid "Branches|New branch" +msgstr "" + +msgid "Branches|No branches to show" +msgstr "" + +msgid "Branches|Once you confirm and press %{delete_protected_branch}, it cannot be undone or recovered." +msgstr "" + +msgid "Branches|Only a project master or owner can delete a protected branch" +msgstr "" + +msgid "Branches|Protected branches can be managed in %{project_settings_link}" +msgstr "" + +msgid "Branches|Sort by" +msgstr "" + +msgid "Branches|The default branch cannot be deleted" +msgstr "" + msgid "Branches|This branch hasn’t been merged into %{default_branch}." msgstr "" msgid "Branches|To avoid data loss, consider merging this branch before deleting it." msgstr "" +msgid "Branches|To confirm, type %{branch_name_confirmation}:" +msgstr "" + +msgid "Branches|You’re about to permanently delete the protected branch %{branch_name}." +msgstr "" + +msgid "Branches|merged" +msgstr "" + +msgid "Branches|project settings" +msgstr "" + +msgid "Branches|protected" +msgstr "" + msgid "Browse Directory" msgstr "" @@ -199,6 +271,9 @@ msgstr "" msgid "CI configuration" msgstr "" +msgid "CICD|Jobs" +msgstr "" + msgid "Cancel" msgstr "" @@ -402,6 +477,12 @@ msgstr "" msgid "CycleAnalyticsStage|Test" msgstr "" +msgid "DashboardProjects|All" +msgstr "" + +msgid "DashboardProjects|Personal" +msgstr "" + msgid "Define a custom pattern with cron syntax" msgstr "" @@ -467,9 +548,6 @@ msgstr "" msgid "Emails" msgstr "" -msgid "Enable in settings" -msgstr "" - msgid "EventFilterBy|Filter by all" msgstr "" @@ -568,7 +646,7 @@ msgstr "" msgid "GroupSettings|This setting is applied on %{ancestor_group}. You can override the setting or %{remove_ancestor_share_with_group_lock}." msgstr "" -msgid "GroupSettings|This setting will be applied to all subgroups unless overridden by a group owner." +msgid "GroupSettings|This setting will be applied to all subgroups unless overridden by a group owner. Groups that already have access to the project will continue to have access unless removed manually." msgstr "" msgid "GroupSettings|cannot be disabled when the parent group \"Share with group lock\" is enabled, except by the owner of the parent group" @@ -595,9 +673,6 @@ msgstr "" msgid "HealthCheck|Unhealthy" msgstr "" -msgid "Home" -msgstr "" - msgid "Housekeeping successfully started" msgstr "" @@ -616,10 +691,10 @@ msgstr "" msgid "Issue events" msgstr "" -msgid "Issues" +msgid "IssueBoards|Board" msgstr "" -msgid "Jobs" +msgid "Issues" msgstr "" msgid "LFSStatus|Disabled" @@ -680,6 +755,9 @@ msgstr "" msgid "Merge events" msgstr "" +msgid "Merge request" +msgstr "" + msgid "Messages" msgstr "" @@ -955,9 +1033,6 @@ msgstr "" msgid "Project export started. A download link will be sent by email." msgstr "" -msgid "Project home" -msgstr "" - msgid "ProjectActivityRSS|Subscribe" msgstr "" @@ -1128,6 +1203,93 @@ msgstr[1] "" msgid "Snippets" msgstr "" +msgid "SortOptions|Access level, ascending" +msgstr "" + +msgid "SortOptions|Access level, descending" +msgstr "" + +msgid "SortOptions|Created date" +msgstr "" + +msgid "SortOptions|Due date" +msgstr "" + +msgid "SortOptions|Due later" +msgstr "" + +msgid "SortOptions|Due soon" +msgstr "" + +msgid "SortOptions|Label priority" +msgstr "" + +msgid "SortOptions|Largest group" +msgstr "" + +msgid "SortOptions|Largest repository" +msgstr "" + +msgid "SortOptions|Last created" +msgstr "" + +msgid "SortOptions|Last joined" +msgstr "" + +msgid "SortOptions|Last updated" +msgstr "" + +msgid "SortOptions|Least popular" +msgstr "" + +msgid "SortOptions|Milestone" +msgstr "" + +msgid "SortOptions|Milestone due later" +msgstr "" + +msgid "SortOptions|Milestone due soon" +msgstr "" + +msgid "SortOptions|Most popular" +msgstr "" + +msgid "SortOptions|Name" +msgstr "" + +msgid "SortOptions|Name, ascending" +msgstr "" + +msgid "SortOptions|Name, descending" +msgstr "" + +msgid "SortOptions|Oldest created" +msgstr "" + +msgid "SortOptions|Oldest joined" +msgstr "" + +msgid "SortOptions|Oldest sign in" +msgstr "" + +msgid "SortOptions|Oldest updated" +msgstr "" + +msgid "SortOptions|Popularity" +msgstr "" + +msgid "SortOptions|Priority" +msgstr "" + +msgid "SortOptions|Recent sign in" +msgstr "" + +msgid "SortOptions|Start later" +msgstr "" + +msgid "SortOptions|Start soon" +msgstr "" + msgid "Source code" msgstr "" @@ -1398,9 +1560,15 @@ msgstr "" msgid "Use your global notification setting" msgstr "" +msgid "View file @ " +msgstr "" + msgid "View open merge request" msgstr "" +msgid "View replaced file @ " +msgstr "" + msgid "VisibilityLevel|Internal" msgstr "" diff --git a/spec/features/projects/commit/diff_notes_spec.rb b/spec/features/projects/commit/diff_notes_spec.rb new file mode 100644 index 00000000000..f0fe4e00acc --- /dev/null +++ b/spec/features/projects/commit/diff_notes_spec.rb @@ -0,0 +1,36 @@ +require 'spec_helper' + +feature 'Commit diff', :js do + include RepoHelpers + + let(:user) { create(:user) } + let(:project) { create(:project, :public, :repository) } + + before do + project.add_master(user) + sign_in user + end + + %w(inline parallel).each do |view| + context "#{view} view" do + before do + visit project_commit_path(project, sample_commit.id, view: view) + end + + it "adds comment to diff" do + diff_line_num = first('.diff-line-num.new') + + diff_line_num.trigger('mouseover') + diff_line_num.find('.js-add-diff-note-button').trigger('click') + + page.within(first('.diff-viewer')) do + find('.js-note-text').set 'test comment' + + click_button 'Comment' + + expect(page).to have_content('test comment') + end + end + end + end +end diff --git a/spec/fixtures/api/schemas/public_api/v4/commit/detail.json b/spec/fixtures/api/schemas/public_api/v4/commit/detail.json index b7b2535c204..88a3cad62f6 100644 --- a/spec/fixtures/api/schemas/public_api/v4/commit/detail.json +++ b/spec/fixtures/api/schemas/public_api/v4/commit/detail.json @@ -5,11 +5,18 @@ { "required" : [ "stats", - "status" + "status", + "last_pipeline" ], "properties": { "stats": { "$ref": "../commit_stats.json" }, - "status": { "type": ["string", "null"] } + "status": { "type": ["string", "null"] }, + "last_pipeline": { + "oneOf": [ + { "type": "null" }, + { "$ref": "../pipeline/basic.json" } + ] + } } } ] diff --git a/spec/fixtures/api/schemas/public_api/v4/pipeline/basic.json b/spec/fixtures/api/schemas/public_api/v4/pipeline/basic.json new file mode 100644 index 00000000000..0d127dc5297 --- /dev/null +++ b/spec/fixtures/api/schemas/public_api/v4/pipeline/basic.json @@ -0,0 +1,16 @@ +{ + "type": "object", + "required" : [ + "id", + "sha", + "ref", + "status" + ], + "properties" : { + "id": { "type": "integer" }, + "sha": { "type": "string" }, + "ref": { "type": "string" }, + "status": { "type": "string" } + }, + "additionalProperties": false +} diff --git a/spec/javascripts/line_highlighter_spec.js b/spec/javascripts/line_highlighter_spec.js index aee274641e8..645664a5219 100644 --- a/spec/javascripts/line_highlighter_spec.js +++ b/spec/javascripts/line_highlighter_spec.js @@ -18,19 +18,25 @@ import '~/line_highlighter'; beforeEach(function() { loadFixtures('static/line_highlighter.html.raw'); this["class"] = new LineHighlighter(); - this.css = this["class"].highlightClass; + this.css = this["class"].highlightLineClass; return this.spies = { __setLocationHash__: spyOn(this["class"], '__setLocationHash__').and.callFake(function() {}) }; }); describe('behavior', function() { it('highlights one line given in the URL hash', function() { - new LineHighlighter('#L13'); + new LineHighlighter({ hash: '#L13' }); return expect($('#LC13')).toHaveClass(this.css); }); + it('highlights one line given in the URL hash with given CSS class name', function() { + const hiliter = new LineHighlighter({ hash: '#L13', highlightLineClass: 'hilite' }); + expect(hiliter.highlightLineClass).toBe('hilite'); + expect($('#LC13')).toHaveClass('hilite'); + expect($('#LC13')).not.toHaveClass('hll'); + }); it('highlights a range of lines given in the URL hash', function() { var line, results; - new LineHighlighter('#L5-25'); + new LineHighlighter({ hash: '#L5-25' }); expect($("." + this.css).length).toBe(21); results = []; for (line = 5; line <= 25; line += 1) { @@ -41,7 +47,7 @@ import '~/line_highlighter'; it('scrolls to the first highlighted line on initial load', function() { var spy; spy = spyOn($, 'scrollTo'); - new LineHighlighter('#L5-25'); + new LineHighlighter({ hash: '#L5-25' }); return expect(spy).toHaveBeenCalledWith('#L5', jasmine.anything()); }); it('discards click events', function() { @@ -50,10 +56,10 @@ import '~/line_highlighter'; clickLine(13); return expect(spy).toHaveBeenPrevented(); }); - return it('handles garbage input from the hash', function() { + it('handles garbage input from the hash', function() { var func; func = function() { - return new LineHighlighter('#blob-content-holder'); + return new LineHighlighter({ fileHolderSelector: '#blob-content-holder' }); }; return expect(func).not.toThrow(); }); diff --git a/spec/javascripts/merge_request_tabs_spec.js b/spec/javascripts/merge_request_tabs_spec.js index eadab738376..ccdbfcba692 100644 --- a/spec/javascripts/merge_request_tabs_spec.js +++ b/spec/javascripts/merge_request_tabs_spec.js @@ -416,5 +416,28 @@ import 'vendor/jquery.scrollTo'; }); }); }); + + describe('expandViewContainer', function () { + beforeEach(() => { + $('body').append('<div class="content-wrapper"><div class="container-fluid container-limited"></div></div>'); + }); + + afterEach(() => { + $('.content-wrapper').remove(); + }); + + it('removes container-limited from containers', function () { + this.class.expandViewContainer(); + + expect($('.content-wrapper')).not.toContainElement('.container-limited'); + }); + + it('does remove container-limited from breadcrumbs', function () { + $('.container-limited').addClass('breadcrumbs'); + this.class.expandViewContainer(); + + expect($('.content-wrapper')).toContainElement('.container-limited'); + }); + }); }); }).call(window); diff --git a/spec/lib/github/client_spec.rb b/spec/lib/github/client_spec.rb new file mode 100644 index 00000000000..b846096fe25 --- /dev/null +++ b/spec/lib/github/client_spec.rb @@ -0,0 +1,34 @@ +require 'spec_helper' + +describe Github::Client do + let(:connection) { spy } + let(:rate_limit) { double(get: [false, 1]) } + let(:client) { described_class.new({}) } + let(:results) { double } + let(:response) { double } + + before do + allow(Faraday).to receive(:new).and_return(connection) + allow(Github::RateLimit).to receive(:new).with(connection).and_return(rate_limit) + end + + describe '#get' do + before do + allow(Github::Response).to receive(:new).with(results).and_return(response) + end + + it 'uses a default per_page param' do + expect(connection).to receive(:get).with('/foo', per_page: 100).and_return(results) + + expect(client.get('/foo')).to eq(response) + end + + context 'with per_page given' do + it 'overwrites the default per_page' do + expect(connection).to receive(:get).with('/foo', per_page: 30).and_return(results) + + expect(client.get('/foo', per_page: 30)).to eq(response) + end + end + end +end diff --git a/spec/lib/gitlab/ci/pipeline/chain/create_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/create_spec.rb new file mode 100644 index 00000000000..f54e2326b06 --- /dev/null +++ b/spec/lib/gitlab/ci/pipeline/chain/create_spec.rb @@ -0,0 +1,66 @@ +require 'spec_helper' + +describe Gitlab::Ci::Pipeline::Chain::Create do + set(:project) { create(:project) } + set(:user) { create(:user) } + + let(:pipeline) do + build(:ci_pipeline_with_one_job, project: project, + ref: 'master') + end + + let(:command) do + double('command', project: project, + current_user: user, + seeds_block: nil) + end + + let(:step) { described_class.new(pipeline, command) } + + before do + step.perform! + end + + context 'when pipeline is ready to be saved' do + it 'saves a pipeline' do + expect(pipeline).to be_persisted + end + + it 'does not break the chain' do + expect(step.break?).to be false + end + + it 'creates stages' do + expect(pipeline.reload.stages).to be_one + end + end + + context 'when pipeline has validation errors' do + let(:pipeline) do + build(:ci_pipeline, project: project, ref: nil) + end + + it 'breaks the chain' do + expect(step.break?).to be true + end + + it 'appends validation error' do + expect(pipeline.errors.to_a) + .to include /Failed to persist the pipeline/ + end + end + + context 'when there is a seed block present' do + let(:seeds) { spy('pipeline seeds') } + + let(:command) do + double('command', project: project, + current_user: user, + seeds_block: seeds) + end + + it 'executes the block' do + expect(seeds).to have_received(:call).with(pipeline) + end + end +end diff --git a/spec/lib/gitlab/ci/pipeline/chain/sequence_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/sequence_spec.rb new file mode 100644 index 00000000000..e165e0fac2a --- /dev/null +++ b/spec/lib/gitlab/ci/pipeline/chain/sequence_spec.rb @@ -0,0 +1,55 @@ +require 'spec_helper' + +describe Gitlab::Ci::Pipeline::Chain::Sequence do + set(:project) { create(:project) } + set(:user) { create(:user) } + + let(:pipeline) { build_stubbed(:ci_pipeline) } + let(:command) { double('command' ) } + let(:first_step) { spy('first step') } + let(:second_step) { spy('second step') } + let(:sequence) { [first_step, second_step] } + + subject do + described_class.new(pipeline, command, sequence) + end + + context 'when one of steps breaks the chain' do + before do + allow(first_step).to receive(:break?).and_return(true) + end + + it 'does not process the second step' do + subject.build! do |pipeline, sequence| + expect(sequence).not_to be_complete + end + + expect(second_step).not_to have_received(:perform!) + end + + it 'returns a pipeline object' do + expect(subject.build!).to eq pipeline + end + end + + context 'when all chains are executed correctly' do + before do + sequence.each do |step| + allow(step).to receive(:break?).and_return(false) + end + end + + it 'iterates through entire sequence' do + subject.build! do |pipeline, sequence| + expect(sequence).to be_complete + end + + expect(first_step).to have_received(:perform!) + expect(second_step).to have_received(:perform!) + end + + it 'returns a pipeline object' do + expect(subject.build!).to eq pipeline + end + end +end diff --git a/spec/lib/gitlab/ci/pipeline/chain/skip_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/skip_spec.rb new file mode 100644 index 00000000000..32bd5de829b --- /dev/null +++ b/spec/lib/gitlab/ci/pipeline/chain/skip_spec.rb @@ -0,0 +1,85 @@ +require 'spec_helper' + +describe Gitlab::Ci::Pipeline::Chain::Skip do + set(:project) { create(:project) } + set(:user) { create(:user) } + set(:pipeline) { create(:ci_pipeline, project: project) } + + let(:command) do + double('command', project: project, + current_user: user, + ignore_skip_ci: false, + save_incompleted: true) + end + + let(:step) { described_class.new(pipeline, command) } + + context 'when pipeline has been skipped by a user' do + before do + allow(pipeline).to receive(:git_commit_message) + .and_return('commit message [ci skip]') + + step.perform! + end + + it 'should break the chain' do + expect(step.break?).to be true + end + + it 'skips the pipeline' do + expect(pipeline.reload).to be_skipped + end + end + + context 'when pipeline has not been skipped' do + before do + step.perform! + end + + it 'should not break the chain' do + expect(step.break?).to be false + end + + it 'should not skip a pipeline chain' do + expect(pipeline.reload).not_to be_skipped + end + end + + context 'when [ci skip] should be ignored' do + let(:command) do + double('command', project: project, + current_user: user, + ignore_skip_ci: true) + end + + it 'does not break the chain' do + step.perform! + + expect(step.break?).to be false + end + end + + context 'when pipeline should be skipped but not persisted' do + let(:command) do + double('command', project: project, + current_user: user, + ignore_skip_ci: false, + save_incompleted: false) + end + + before do + allow(pipeline).to receive(:git_commit_message) + .and_return('commit message [ci skip]') + + step.perform! + end + + it 'breaks the chain' do + expect(step.break?).to be true + end + + it 'does not skip pipeline' do + expect(pipeline.reload).not_to be_skipped + end + end +end diff --git a/spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb new file mode 100644 index 00000000000..0bbdd23f4d6 --- /dev/null +++ b/spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb @@ -0,0 +1,142 @@ +require 'spec_helper' + +describe Gitlab::Ci::Pipeline::Chain::Validate::Abilities do + set(:project) { create(:project, :repository) } + set(:user) { create(:user) } + + let(:pipeline) do + build_stubbed(:ci_pipeline, ref: ref, project: project) + end + + let(:command) do + double('command', project: project, current_user: user) + end + + let(:step) { described_class.new(pipeline, command) } + + let(:ref) { 'master' } + + context 'when users has no ability to run a pipeline' do + before do + step.perform! + end + + it 'adds an error about insufficient permissions' do + expect(pipeline.errors.to_a) + .to include /Insufficient permissions/ + end + + it 'breaks the pipeline builder chain' do + expect(step.break?).to eq true + end + end + + context 'when user has ability to create a pipeline' do + before do + project.add_developer(user) + + step.perform! + end + + it 'does not invalidate the pipeline' do + expect(pipeline).to be_valid + end + + it 'does not break the chain' do + expect(step.break?).to eq false + end + end + + describe '#allowed_to_create?' do + subject { step.allowed_to_create? } + + context 'when user is a developer' do + before do + project.add_developer(user) + end + + it { is_expected.to be_truthy } + + context 'when the branch is protected' do + let!(:protected_branch) do + create(:protected_branch, project: project, name: ref) + end + + it { is_expected.to be_falsey } + + context 'when developers are allowed to merge' do + let!(:protected_branch) do + create(:protected_branch, + :developers_can_merge, + project: project, + name: ref) + end + + it { is_expected.to be_truthy } + end + end + + context 'when the tag is protected' do + let(:ref) { 'v1.0.0' } + + let!(:protected_tag) do + create(:protected_tag, project: project, name: ref) + end + + it { is_expected.to be_falsey } + + context 'when developers are allowed to create the tag' do + let!(:protected_tag) do + create(:protected_tag, + :developers_can_create, + project: project, + name: ref) + end + + it { is_expected.to be_truthy } + end + end + end + + context 'when user is a master' do + before do + project.add_master(user) + end + + it { is_expected.to be_truthy } + + context 'when the branch is protected' do + let!(:protected_branch) do + create(:protected_branch, project: project, name: ref) + end + + it { is_expected.to be_truthy } + end + + context 'when the tag is protected' do + let(:ref) { 'v1.0.0' } + + let!(:protected_tag) do + create(:protected_tag, project: project, name: ref) + end + + it { is_expected.to be_truthy } + + context 'when no one can create the tag' do + let!(:protected_tag) do + create(:protected_tag, + :no_one_can_create, + project: project, + name: ref) + end + + it { is_expected.to be_falsey } + end + end + end + + context 'when owner cannot create pipeline' do + it { is_expected.to be_falsey } + end + end +end diff --git a/spec/lib/gitlab/ci/pipeline/chain/validate/config_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/validate/config_spec.rb new file mode 100644 index 00000000000..3740df88f42 --- /dev/null +++ b/spec/lib/gitlab/ci/pipeline/chain/validate/config_spec.rb @@ -0,0 +1,126 @@ +require 'spec_helper' + +describe Gitlab::Ci::Pipeline::Chain::Validate::Config do + set(:project) { create(:project) } + set(:user) { create(:user) } + + let(:command) do + double('command', project: project, + current_user: user, + save_incompleted: true) + end + + let!(:step) { described_class.new(pipeline, command) } + + before do + step.perform! + end + + context 'when pipeline has no YAML configuration' do + let(:pipeline) do + build_stubbed(:ci_pipeline, project: project) + end + + it 'appends errors about missing configuration' do + expect(pipeline.errors.to_a) + .to include 'Missing .gitlab-ci.yml file' + end + + it 'breaks the chain' do + expect(step.break?).to be true + end + end + + context 'when YAML configuration contains errors' do + let(:pipeline) do + build(:ci_pipeline, project: project, config: 'invalid YAML') + end + + it 'appends errors about YAML errors' do + expect(pipeline.errors.to_a) + .to include 'Invalid configuration format' + end + + it 'breaks the chain' do + expect(step.break?).to be true + end + + context 'when saving incomplete pipeline is allowed' do + let(:command) do + double('command', project: project, + current_user: user, + save_incompleted: true) + end + + it 'fails the pipeline' do + expect(pipeline.reload).to be_failed + end + end + + context 'when saving incomplete pipeline is not allowed' do + let(:command) do + double('command', project: project, + current_user: user, + save_incompleted: false) + end + + it 'does not drop pipeline' do + expect(pipeline).not_to be_failed + expect(pipeline).not_to be_persisted + end + end + end + + context 'when pipeline has no stages / jobs' do + let(:config) do + { rspec: { + script: 'ls', + only: ['something'] + } } + end + + let(:pipeline) do + build(:ci_pipeline, project: project, config: config) + end + + it 'appends an error about missing stages' do + expect(pipeline.errors.to_a) + .to include 'No stages / jobs for this pipeline.' + end + + it 'breaks the chain' do + expect(step.break?).to be true + end + end + + context 'when pipeline contains configuration validation errors' do + let(:config) { { rspec: {} } } + + let(:pipeline) do + build(:ci_pipeline, project: project, config: config) + end + + it 'appends configuration validation errors to pipeline errors' do + expect(pipeline.errors.to_a) + .to include "jobs:rspec config can't be blank" + end + + it 'breaks the chain' do + expect(step.break?).to be true + end + end + + context 'when pipeline is correct and complete' do + let(:pipeline) do + build(:ci_pipeline_with_one_job, project: project) + end + + it 'does not invalidate the pipeline' do + expect(pipeline).to be_valid + end + + it 'does not break the chain' do + expect(step.break?).to be false + end + end +end diff --git a/spec/lib/gitlab/ci/pipeline/chain/validate/repository_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/validate/repository_spec.rb new file mode 100644 index 00000000000..bb356efe9ad --- /dev/null +++ b/spec/lib/gitlab/ci/pipeline/chain/validate/repository_spec.rb @@ -0,0 +1,60 @@ +require 'spec_helper' + +describe Gitlab::Ci::Pipeline::Chain::Validate::Repository do + set(:project) { create(:project, :repository) } + set(:user) { create(:user) } + + let(:command) do + double('command', project: project, current_user: user) + end + + let!(:step) { described_class.new(pipeline, command) } + + before do + step.perform! + end + + context 'when pipeline ref and sha exists' do + let(:pipeline) do + build_stubbed(:ci_pipeline, ref: 'master', sha: '123', project: project) + end + + it 'does not break the chain' do + expect(step.break?).to be false + end + + it 'does not append pipeline errors' do + expect(pipeline.errors).to be_empty + end + end + + context 'when pipeline ref does not exist' do + let(:pipeline) do + build_stubbed(:ci_pipeline, ref: 'something', project: project) + end + + it 'breaks the chain' do + expect(step.break?).to be true + end + + it 'adds an error about missing ref' do + expect(pipeline.errors.to_a) + .to include 'Reference not found' + end + end + + context 'when pipeline does not have SHA set' do + let(:pipeline) do + build_stubbed(:ci_pipeline, ref: 'master', sha: nil, project: project) + end + + it 'breaks the chain' do + expect(step.break?).to be true + end + + it 'adds an error about missing SHA' do + expect(pipeline.errors.to_a) + .to include 'Commit not found' + end + end +end diff --git a/spec/lib/gitlab/ci/pipeline_duration_spec.rb b/spec/lib/gitlab/ci/pipeline/duration_spec.rb index b26728a843c..7c9836e2da6 100644 --- a/spec/lib/gitlab/ci/pipeline_duration_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/duration_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::Ci::PipelineDuration do +describe Gitlab::Ci::Pipeline::Duration do let(:calculated_duration) { calculate(data) } shared_examples 'calculating duration' do @@ -107,9 +107,9 @@ describe Gitlab::Ci::PipelineDuration do def calculate(data) periods = data.shuffle.map do |(first, last)| - Gitlab::Ci::PipelineDuration::Period.new(first, last) + described_class::Period.new(first, last) end - Gitlab::Ci::PipelineDuration.from_periods(periods.sort_by(&:first)) + described_class.from_periods(periods.sort_by(&:first)) end end diff --git a/spec/lib/gitlab/diff/diff_refs_spec.rb b/spec/lib/gitlab/diff/diff_refs_spec.rb index c73708d90a8..f9bfb4c469e 100644 --- a/spec/lib/gitlab/diff/diff_refs_spec.rb +++ b/spec/lib/gitlab/diff/diff_refs_spec.rb @@ -3,6 +3,61 @@ require 'spec_helper' describe Gitlab::Diff::DiffRefs do let(:project) { create(:project, :repository) } + describe '#==' do + let(:commit) { project.commit('1a0b36b3cdad1d2ee32457c102a8c0b7056fa863') } + subject { commit.diff_refs } + + context 'when shas are missing' do + let(:other) { described_class.new(base_sha: subject.base_sha, start_sha: subject.start_sha, head_sha: nil) } + + it 'returns false' do + expect(subject).not_to eq(other) + end + end + + context 'when shas are equal' do + let(:other) { described_class.new(base_sha: subject.base_sha, start_sha: subject.start_sha, head_sha: subject.head_sha) } + + it 'returns true' do + expect(subject).to eq(other) + end + end + + context 'when shas are unequal' do + let(:other) { described_class.new(base_sha: subject.base_sha, start_sha: subject.start_sha, head_sha: subject.head_sha.reverse) } + + it 'returns false' do + expect(subject).not_to eq(other) + end + end + + context 'when shas are truncated' do + context 'when sha prefixes are too short' do + let(:other) { described_class.new(base_sha: subject.base_sha[0, 4], start_sha: subject.start_sha[0, 4], head_sha: subject.head_sha[0, 4]) } + + it 'returns false' do + expect(subject).not_to eq(other) + end + end + + context 'when sha prefixes are equal' do + let(:other) { described_class.new(base_sha: subject.base_sha[0, 10], start_sha: subject.start_sha[0, 10], head_sha: subject.head_sha[0, 10]) } + + it 'returns true' do + expect(subject).to eq(other) + end + end + + context 'when sha prefixes are unequal' do + let(:other) { described_class.new(base_sha: subject.base_sha[0, 10], start_sha: subject.start_sha[0, 10], head_sha: subject.head_sha[0, 10].reverse) } + + it 'returns false' do + expect(subject).not_to eq(other) + end + end + end + end + describe '#compare_in' do context 'with diff refs for the initial commit' do let(:commit) { project.commit('1a0b36b3cdad1d2ee32457c102a8c0b7056fa863') } diff --git a/spec/lib/gitlab/diff/position_spec.rb b/spec/lib/gitlab/diff/position_spec.rb index d4a2a852c12..7798736a4dc 100644 --- a/spec/lib/gitlab/diff/position_spec.rb +++ b/spec/lib/gitlab/diff/position_spec.rb @@ -429,6 +429,44 @@ describe Gitlab::Diff::Position do end end + describe '#==' do + let(:commit) { project.commit("570e7b2abdd848b95f2f578043fc23bd6f6fd24d") } + + subject do + described_class.new( + old_path: "files/ruby/popen.rb", + new_path: "files/ruby/popen.rb", + old_line: nil, + new_line: 14, + diff_refs: commit.diff_refs + ) + end + + context 'when positions are equal' do + let(:other) { described_class.new(subject.to_h) } + + it 'returns true' do + expect(subject).to eq(other) + end + end + + context 'when positions are equal, except for truncated shas' do + let(:other) { described_class.new(subject.to_h.merge(start_sha: subject.start_sha[0, 10])) } + + it 'returns true' do + expect(subject).to eq(other) + end + end + + context 'when positions are unequal' do + let(:other) { described_class.new(subject.to_h.merge(start_sha: subject.start_sha.reverse)) } + + it 'returns false' do + expect(subject).not_to eq(other) + end + end + end + describe "#to_json" do let(:hash) do { diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 4fc26c625a5..5effaf2b043 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -1332,6 +1332,84 @@ describe Gitlab::Git::Repository, seed_helper: true do end end + describe '#with_repo_branch_commit' do + context 'when comparing with the same repository' do + let(:start_repository) { repository } + + context 'when the branch exists' do + let(:start_branch_name) { 'master' } + + it 'yields the commit' do + expect { |b| repository.with_repo_branch_commit(start_repository, start_branch_name, &b) } + .to yield_with_args(an_instance_of(Gitlab::Git::Commit)) + end + end + + context 'when the branch does not exist' do + let(:start_branch_name) { 'definitely-not-master' } + + it 'yields nil' do + expect { |b| repository.with_repo_branch_commit(start_repository, start_branch_name, &b) } + .to yield_with_args(nil) + end + end + end + + context 'when comparing with another repository' do + let(:start_repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') } + + context 'when the branch exists' do + let(:start_branch_name) { 'master' } + + it 'yields the commit' do + expect { |b| repository.with_repo_branch_commit(start_repository, start_branch_name, &b) } + .to yield_with_args(an_instance_of(Gitlab::Git::Commit)) + end + end + + context 'when the branch does not exist' do + let(:start_branch_name) { 'definitely-not-master' } + + it 'yields nil' do + expect { |b| repository.with_repo_branch_commit(start_repository, start_branch_name, &b) } + .to yield_with_args(nil) + end + end + end + end + + describe '#fetch_source_branch' do + let(:local_ref) { 'refs/merge-requests/1/head' } + + context 'when the branch exists' do + let(:source_branch) { 'master' } + + it 'writes the ref' do + expect(repository).to receive(:write_ref).with(local_ref, /\h{40}/) + + repository.fetch_source_branch(repository, source_branch, local_ref) + end + + it 'returns true' do + expect(repository.fetch_source_branch(repository, source_branch, local_ref)).to eq(true) + end + end + + context 'when the branch does not exist' do + let(:source_branch) { 'definitely-not-master' } + + it 'does not write the ref' do + expect(repository).not_to receive(:write_ref) + + repository.fetch_source_branch(repository, source_branch, local_ref) + end + + it 'returns false' do + expect(repository.fetch_source_branch(repository, source_branch, local_ref)).to eq(false) + end + end + end + def create_remote_branch(repository, remote_name, branch_name, source_branch_name) source_branch = repository.branches.find { |branch| branch.name == source_branch_name } rugged = repository.rugged diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb index f663719d28c..94462b4572d 100644 --- a/spec/requests/api/commits_spec.rb +++ b/spec/requests/api/commits_spec.rb @@ -491,6 +491,7 @@ describe API::Commits do expect(json_response['stats']['deletions']).to eq(commit.stats.deletions) expect(json_response['stats']['total']).to eq(commit.stats.total) expect(json_response['status']).to be_nil + expect(json_response['last_pipeline']).to be_nil end context 'when ref does not exist' do @@ -573,6 +574,10 @@ describe API::Commits do expect(response).to have_http_status(200) expect(response).to match_response_schema('public_api/v4/commit/detail') expect(json_response['status']).to eq('created') + expect(json_response['last_pipeline']['id']).to eq(pipeline.id) + expect(json_response['last_pipeline']['ref']).to eq(pipeline.ref) + expect(json_response['last_pipeline']['sha']).to eq(pipeline.sha) + expect(json_response['last_pipeline']['status']).to eq(pipeline.status) end context 'when pipeline succeeds' do diff --git a/spec/requests/api/environments_spec.rb b/spec/requests/api/environments_spec.rb index 2361809e0e1..f8cd529a06c 100644 --- a/spec/requests/api/environments_spec.rb +++ b/spec/requests/api/environments_spec.rb @@ -20,6 +20,7 @@ describe API::Environments do path path_with_namespace star_count forks_count created_at last_activity_at + avatar_url ) get api("/projects/#{project.id}/environments", user) diff --git a/spec/requests/api/helpers_spec.rb b/spec/requests/api/helpers_spec.rb index d4006fe71a2..98c49d3364c 100644 --- a/spec/requests/api/helpers_spec.rb +++ b/spec/requests/api/helpers_spec.rb @@ -159,18 +159,25 @@ describe API::Helpers do end describe "when authenticating using a user's private token" do - it "returns nil for an invalid token" do + it "returns a 401 response for an invalid token" do env[API::APIGuard::PRIVATE_TOKEN_HEADER] = 'invalid token' allow_any_instance_of(self.class).to receive(:doorkeeper_guard) { false } - expect(current_user).to be_nil + expect { current_user }.to raise_error /401/ end - it "returns nil for a user without access" do + it "returns a 401 response for a user without access" do env[API::APIGuard::PRIVATE_TOKEN_HEADER] = user.private_token allow_any_instance_of(Gitlab::UserAccess).to receive(:allowed?).and_return(false) - expect(current_user).to be_nil + expect { current_user }.to raise_error /401/ + end + + it 'returns a 401 response for a user who is blocked' do + user.block! + env[API::APIGuard::PRIVATE_TOKEN_HEADER] = user.private_token + + expect { current_user }.to raise_error /401/ end it "leaves user as is when sudo not specified" do @@ -193,24 +200,31 @@ describe API::Helpers do allow_any_instance_of(self.class).to receive(:doorkeeper_guard) { false } end - it "returns nil for an invalid token" do + it "returns a 401 response for an invalid token" do env[API::APIGuard::PRIVATE_TOKEN_HEADER] = 'invalid token' - expect(current_user).to be_nil + expect { current_user }.to raise_error /401/ end - it "returns nil for a user without access" do + it "returns a 401 response for a user without access" do env[API::APIGuard::PRIVATE_TOKEN_HEADER] = personal_access_token.token allow_any_instance_of(Gitlab::UserAccess).to receive(:allowed?).and_return(false) - expect(current_user).to be_nil + expect { current_user }.to raise_error /401/ + end + + it 'returns a 401 response for a user who is blocked' do + user.block! + env[API::APIGuard::PRIVATE_TOKEN_HEADER] = personal_access_token.token + + expect { current_user }.to raise_error /401/ end - it "returns nil for a token without the appropriate scope" do + it "returns a 401 response for a token without the appropriate scope" do personal_access_token = create(:personal_access_token, user: user, scopes: ['read_user']) env[API::APIGuard::PRIVATE_TOKEN_HEADER] = personal_access_token.token - expect(current_user).to be_nil + expect { current_user }.to raise_error /401/ end it "leaves user as is when sudo not specified" do @@ -226,14 +240,14 @@ describe API::Helpers do personal_access_token.revoke! env[API::APIGuard::PRIVATE_TOKEN_HEADER] = personal_access_token.token - expect(current_user).to be_nil + expect { current_user }.to raise_error /401/ end it 'does not allow expired tokens' do personal_access_token.update_attributes!(expires_at: 1.day.ago) env[API::APIGuard::PRIVATE_TOKEN_HEADER] = personal_access_token.token - expect(current_user).to be_nil + expect { current_user }.to raise_error /401/ end end @@ -351,6 +365,18 @@ describe API::Helpers do end end end + + context 'when user is blocked' do + before do + user.block! + end + + it 'changes current_user to sudo' do + set_env(admin, user.id) + + expect(current_user).to eq(user) + end + end end context 'with regular user' do @@ -490,11 +516,10 @@ describe API::Helpers do context 'current_user is nil' do before do expect_any_instance_of(self.class).to receive(:current_user).and_return(nil) - allow_any_instance_of(self.class).to receive(:initial_current_user).and_return(nil) end it 'returns a 401 response' do - expect { authenticate! }.to raise_error '401 - {"message"=>"401 Unauthorized"}' + expect { authenticate! }.to raise_error /401/ end end @@ -502,35 +527,12 @@ describe API::Helpers do let(:user) { build(:user) } before do - expect_any_instance_of(self.class).to receive(:current_user).at_least(:once).and_return(user) - expect_any_instance_of(self.class).to receive(:initial_current_user).and_return(user) + expect_any_instance_of(self.class).to receive(:current_user).and_return(user) end it 'does not raise an error' do expect { authenticate! }.not_to raise_error end end - - context 'current_user is blocked' do - let(:user) { build(:user, :blocked) } - - before do - expect_any_instance_of(self.class).to receive(:current_user).at_least(:once).and_return(user) - end - - it 'raises an error' do - expect_any_instance_of(self.class).to receive(:initial_current_user).and_return(user) - - expect { authenticate! }.to raise_error '401 - {"message"=>"401 Unauthorized"}' - end - - it "doesn't raise an error if an admin user is impersonating a blocked user (via sudo)" do - admin_user = build(:user, :admin) - - expect_any_instance_of(self.class).to receive(:initial_current_user).and_return(admin_user) - - expect { authenticate! }.not_to raise_error - end - end end end diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 508df990952..18f6f7df1fa 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -193,6 +193,7 @@ describe API::Projects do path path_with_namespace star_count forks_count created_at last_activity_at + avatar_url ) get api('/projects?simple=true', user) diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 5b306ec6cbf..c30188b1b41 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -125,6 +125,15 @@ describe API::Users do end context "when admin" do + context 'when sudo is defined' do + it 'does not return 500' do + admin_personal_access_token = create(:personal_access_token, user: admin).token + get api("/users?private_token=#{admin_personal_access_token}&sudo=#{user.id}", admin) + + expect(response).to have_http_status(:success) + end + end + it "returns an array of users" do get api("/users", admin) diff --git a/spec/requests/api/v3/projects_spec.rb b/spec/requests/api/v3/projects_spec.rb index cae2c3118da..e5282c3311f 100644 --- a/spec/requests/api/v3/projects_spec.rb +++ b/spec/requests/api/v3/projects_spec.rb @@ -89,6 +89,7 @@ describe API::V3::Projects do path path_with_namespace star_count forks_count created_at last_activity_at + avatar_url ) get v3_api('/projects?simple=true', user) diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index 4c2ff08039c..eb6e683cc23 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -133,6 +133,26 @@ describe Ci::CreatePipelineService do expect(merge_request.reload.head_pipeline).to eq head_pipeline end end + + context 'when pipeline has been skipped' do + before do + allow_any_instance_of(Ci::Pipeline) + .to receive(:git_commit_message) + .and_return('some commit [ci skip]') + end + + it 'updates merge request head pipeline' do + merge_request = create(:merge_request, source_branch: 'master', + target_branch: 'feature', + source_project: project) + + head_pipeline = execute_service + + expect(head_pipeline).to be_skipped + expect(head_pipeline).to be_persisted + expect(merge_request.reload.head_pipeline).to eq head_pipeline + end + end end context 'auto-cancel enabled' do @@ -481,104 +501,4 @@ describe Ci::CreatePipelineService do end end end - - describe '#allowed_to_create?' do - let(:user) { create(:user) } - let(:project) { create(:project, :repository) } - let(:ref) { 'master' } - - subject do - described_class.new(project, user, ref: ref) - .send(:allowed_to_create?) - end - - context 'when user is a developer' do - before do - project.add_developer(user) - end - - it { is_expected.to be_truthy } - - context 'when the branch is protected' do - let!(:protected_branch) do - create(:protected_branch, project: project, name: ref) - end - - it { is_expected.to be_falsey } - - context 'when developers are allowed to merge' do - let!(:protected_branch) do - create(:protected_branch, - :developers_can_merge, - project: project, - name: ref) - end - - it { is_expected.to be_truthy } - end - end - - context 'when the tag is protected' do - let(:ref) { 'v1.0.0' } - - let!(:protected_tag) do - create(:protected_tag, project: project, name: ref) - end - - it { is_expected.to be_falsey } - - context 'when developers are allowed to create the tag' do - let!(:protected_tag) do - create(:protected_tag, - :developers_can_create, - project: project, - name: ref) - end - - it { is_expected.to be_truthy } - end - end - end - - context 'when user is a master' do - before do - project.add_master(user) - end - - it { is_expected.to be_truthy } - - context 'when the branch is protected' do - let!(:protected_branch) do - create(:protected_branch, project: project, name: ref) - end - - it { is_expected.to be_truthy } - end - - context 'when the tag is protected' do - let(:ref) { 'v1.0.0' } - - let!(:protected_tag) do - create(:protected_tag, project: project, name: ref) - end - - it { is_expected.to be_truthy } - - context 'when no one can create the tag' do - let!(:protected_tag) do - create(:protected_tag, - :no_one_can_create, - project: project, - name: ref) - end - - it { is_expected.to be_falsey } - end - end - end - - context 'when owner cannot create pipeline' do - it { is_expected.to be_falsey } - end - end end diff --git a/spec/services/emails/create_service_spec.rb b/spec/services/emails/create_service_spec.rb index 641d5538de8..75812c17309 100644 --- a/spec/services/emails/create_service_spec.rb +++ b/spec/services/emails/create_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe Emails::CreateService do let(:user) { create(:user) } - let(:opts) { { email: 'new@email.com' } } + let(:opts) { { email: 'new@email.com', user: user } } subject(:service) { described_class.new(user, opts) } diff --git a/spec/services/emails/destroy_service_spec.rb b/spec/services/emails/destroy_service_spec.rb index 1f4294dd905..7726fc0ef81 100644 --- a/spec/services/emails/destroy_service_spec.rb +++ b/spec/services/emails/destroy_service_spec.rb @@ -4,7 +4,7 @@ describe Emails::DestroyService do let!(:user) { create(:user) } let!(:email) { create(:email, user: user) } - subject(:service) { described_class.new(user, email: email.email) } + subject(:service) { described_class.new(user, user: user, email: email.email) } describe '#execute' do it 'removes an email' do diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb index b60136064b7..80213d093f1 100644 --- a/spec/services/merge_requests/merge_service_spec.rb +++ b/spec/services/merge_requests/merge_service_spec.rb @@ -13,20 +13,21 @@ describe MergeRequests::MergeService do describe '#execute' do context 'MergeRequest#merge_jid' do + let(:service) do + described_class.new(project, user, commit_message: 'Awesome message') + end + before do merge_request.update_column(:merge_jid, 'hash-123') end it 'is cleaned when no error is raised' do - service = described_class.new(project, user, commit_message: 'Awesome message') - service.execute(merge_request) expect(merge_request.reload.merge_jid).to be_nil end it 'is cleaned when expected error is raised' do - service = described_class.new(project, user, commit_message: 'Awesome message') allow(service).to receive(:commit).and_raise(described_class::MergeError) service.execute(merge_request) @@ -34,6 +35,22 @@ describe MergeRequests::MergeService do expect(merge_request.reload.merge_jid).to be_nil end + it 'is cleaned when merge request is not mergeable' do + allow(merge_request).to receive(:mergeable?).and_return(false) + + service.execute(merge_request) + + expect(merge_request.reload.merge_jid).to be_nil + end + + it 'is cleaned when no source is found' do + allow(merge_request).to receive(:diff_head_sha).and_return(nil) + + service.execute(merge_request) + + expect(merge_request.reload.merge_jid).to be_nil + end + it 'is not cleaned when unexpected error is raised' do service = described_class.new(project, user, commit_message: 'Awesome message') allow(service).to receive(:commit).and_raise(StandardError) diff --git a/spec/services/users/update_service_spec.rb b/spec/services/users/update_service_spec.rb index 6ee35a33b2d..f8d4a47b212 100644 --- a/spec/services/users/update_service_spec.rb +++ b/spec/services/users/update_service_spec.rb @@ -31,13 +31,13 @@ describe Users::UpdateService do end def update_user(user, opts) - described_class.new(user, opts).execute + described_class.new(user, opts.merge(user: user)).execute end end describe '#execute!' do it 'updates the name' do - service = described_class.new(user, name: 'New Name') + service = described_class.new(user, user: user, name: 'New Name') expect(service).not_to receive(:notify_new_user) result = service.execute! @@ -55,7 +55,7 @@ describe Users::UpdateService do it 'fires system hooks when a new user is saved' do system_hook_service = spy(:system_hook_service) user = build(:user) - service = described_class.new(user, name: 'John Doe') + service = described_class.new(user, user: user, name: 'John Doe') expect(service).to receive(:notify_new_user).and_call_original expect(service).to receive(:system_hook_service).and_return(system_hook_service) @@ -65,7 +65,7 @@ describe Users::UpdateService do end def update_user(user, opts) - described_class.new(user, opts).execute! + described_class.new(user, opts.merge(user: user)).execute! end end end diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb index 6e5b9700b54..b4e8b5ea67b 100644 --- a/spec/support/test_env.rb +++ b/spec/support/test_env.rb @@ -306,6 +306,9 @@ module TestEnv ensure_component_dir_name_is_correct!(component, install_dir) + # On CI, once installed, components never need update + return if File.exist?(install_dir) && ENV['CI'] + if component_needs_update?(install_dir, version) # Cleanup the component entirely to ensure we start fresh FileUtils.rm_rf(install_dir) diff --git a/spec/workers/post_receive_spec.rb b/spec/workers/post_receive_spec.rb index d3707a3cc11..05eecf5f0bb 100644 --- a/spec/workers/post_receive_spec.rb +++ b/spec/workers/post_receive_spec.rb @@ -70,12 +70,15 @@ describe PostReceive do context "creates a Ci::Pipeline for every change" do before do - allow_any_instance_of(Ci::CreatePipelineService).to receive(:commit) do - OpenStruct.new(id: '123456') - end - allow_any_instance_of(Ci::CreatePipelineService).to receive(:branch?).and_return(true) - allow_any_instance_of(Repository).to receive(:ref_exists?).and_return(true) stub_ci_pipeline_to_return_yaml_file + + # TODO, don't stub private methods + # + allow_any_instance_of(Ci::CreatePipelineService) + .to receive(:commit).and_return(OpenStruct.new(id: '123456')) + + allow_any_instance_of(Repository) + .to receive(:branch_exists?).and_return(true) end it { expect { subject }.to change { Ci::Pipeline.count }.by(2) } |
