diff options
112 files changed, 1392 insertions, 566 deletions
diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 6d4d7170fe8..d581610162f 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -62,7 +62,7 @@ Lint/UnusedMethodArgument: # Offense count: 93 # Configuration parameters: CountComments. Metrics/BlockLength: - Max: 288 + Enabled: false # Offense count: 3 # Cop supports --auto-correct. @@ -125,7 +125,7 @@ RSpec/MessageSpies: # Offense count: 3036 RSpec/MultipleExpectations: - Max: 37 + Enabled: false # Offense count: 2133 RSpec/NamedSubject: @@ -219,7 +219,7 @@ gem 'oj', '~> 2.17.4' gem 'chronic', '~> 0.10.2' gem 'chronic_duration', '~> 0.10.6' -gem 'sass-rails', '~> 5.0.6' +gem 'sassc-rails', '~> 1.3.0' gem 'coffee-rails', '~> 4.1.0' gem 'uglifier', '~> 2.7.2' gem 'gitlab-turbolinks-classic', '~> 2.5', '>= 2.5.6' @@ -255,7 +255,6 @@ group :development do gem 'brakeman', '~> 3.3.0', require: false gem 'letter_opener_web', '~> 1.3.0' - gem 'rerun', '~> 0.11.0' gem 'bullet', '~> 5.2.0', require: false gem 'rblineprof', '~> 0.3.6', platform: :mri, require: false gem 'web-console', '~> 2.0' @@ -286,7 +285,7 @@ group :development, :test do gem 'minitest', '~> 5.7.0' # Generate Fake data - gem 'ffaker', '~> 2.0.0' + gem 'ffaker', '~> 2.4' gem 'capybara', '~> 2.6.2' gem 'capybara-screenshot', '~> 1.0.0' diff --git a/Gemfile.lock b/Gemfile.lock index e2d7f94e571..4d025683b20 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -198,7 +198,7 @@ GEM faraday_middleware-multi_json (0.0.6) faraday_middleware multi_json - ffaker (2.0.0) + ffaker (2.4.0) ffi (1.9.10) flay (2.6.1) ruby_parser (~> 3.0) @@ -407,9 +407,6 @@ GEM xml-simple licensee (8.0.0) rugged (>= 0.24b) - listen (3.0.5) - rb-fsevent (>= 0.9.3) - rb-inotify (>= 0.9) little-plugger (1.1.4) logging (2.1.0) little-plugger (~> 1.1) @@ -580,9 +577,6 @@ GEM rainbow (2.1.0) raindrops (0.17.0) rake (10.5.0) - rb-fsevent (0.9.6) - rb-inotify (0.9.5) - ffi (>= 0.5.0) rblineprof (0.3.6) debugger-ruby_core_source (~> 1.3) rdoc (4.2.2) @@ -611,8 +605,6 @@ GEM redis-store (1.2.0) redis (>= 2.2) request_store (1.3.1) - rerun (0.11.0) - listen (~> 3.0) responders (2.3.0) railties (>= 4.2.0, < 5.1) rest-client (2.0.0) @@ -675,12 +667,17 @@ GEM sanitize (2.1.0) nokogiri (>= 1.4.4) sass (3.4.22) - sass-rails (5.0.6) - railties (>= 4.0.0, < 6) - sass (~> 3.1) - sprockets (>= 2.8, < 4.0) - sprockets-rails (>= 2.0, < 4.0) - tilt (>= 1.1, < 3) + sassc (1.11.1) + bundler + ffi (~> 1.9.6) + sass (>= 3.3.0) + sassc-rails (1.3.0) + railties (>= 4.0.0) + sass + sassc (~> 1.9) + sprockets (> 2.11) + sprockets-rails + tilt sawyer (0.8.1) addressable (>= 2.3.5, < 2.6) faraday (~> 0.8, < 1.0) @@ -877,7 +874,7 @@ DEPENDENCIES email_reply_trimmer (~> 0.1) email_spec (~> 1.6.0) factory_girl_rails (~> 4.7.0) - ffaker (~> 2.0.0) + ffaker (~> 2.4) flay (~> 2.6.1) fog-aws (~> 0.9) fog-core (~> 1.40) @@ -968,7 +965,6 @@ DEPENDENCIES redis-namespace (~> 1.5.2) redis-rails (~> 5.0.1) request_store (~> 1.3) - rerun (~> 0.11.0) responders (~> 2.0) rouge (~> 2.0) rqrcode-rails3 (~> 0.1.7) @@ -980,7 +976,7 @@ DEPENDENCIES ruby-prof (~> 0.16.2) rugged (~> 0.24.0) sanitize (~> 2.0) - sass-rails (~> 5.0.6) + sassc-rails (~> 1.3.0) scss_lint (~> 0.47.0) seed-fu (~> 2.3.5) select2-rails (~> 3.5.9) diff --git a/app/assets/javascripts/diff.js.es6 b/app/assets/javascripts/diff.js.es6 index 9cf33e62958..5e1a4c948aa 100644 --- a/app/assets/javascripts/diff.js.es6 +++ b/app/assets/javascripts/diff.js.es6 @@ -20,7 +20,7 @@ .on('click', '.js-unfold', this.handleClickUnfold.bind(this)) .on('click', '.diff-line-num a', this.handleClickLineNum.bind(this)); - this.highlighSelectedLine(); + this.openAnchoredDiff(); } handleClickUnfold(e) { @@ -61,13 +61,22 @@ $.get(link, params, response => $target.parent().replaceWith(response)); } - openAnchoredDiff(anchoredDiff, cb) { - const diffTitle = $(`#file-path-${anchoredDiff}`); + openAnchoredDiff(cb) { + const locationHash = gl.utils.getLocationHash(); + const anchoredDiff = locationHash && locationHash.split('_')[0]; + + if (!anchoredDiff) return; + + const diffTitle = $(`#${anchoredDiff}`); const diffFile = diffTitle.closest('.diff-file'); const nothingHereBlock = $('.nothing-here-block:visible', diffFile); if (nothingHereBlock.length) { - diffFile.singleFileDiff(true, cb); - } else { + const clickTarget = $('.file-title, .click-to-expand', diffFile); + diffFile.data('singleFileDiff').toggleDiff(clickTarget, () => { + this.highlighSelectedLine(); + if (cb) cb(); + }); + } else if (cb) { cb(); } } diff --git a/app/assets/javascripts/gl_dropdown.js b/app/assets/javascripts/gl_dropdown.js index bb516b3d2df..00859728c30 100644 --- a/app/assets/javascripts/gl_dropdown.js +++ b/app/assets/javascripts/gl_dropdown.js @@ -476,7 +476,7 @@ this.removeArrayKeyEvent(); $input = this.dropdown.find(".dropdown-input-field"); if (this.options.filterable) { - $input.blur().val(""); + $input.blur(); } if (this.dropdown.find(".dropdown-toggle-page").length) { $('.dropdown-menu', this.dropdown).removeClass(PAGE_TWO_CLASS); diff --git a/app/assets/javascripts/merge_request_tabs.js.es6 b/app/assets/javascripts/merge_request_tabs.js.es6 index 860e7e066a0..4c8c28af755 100644 --- a/app/assets/javascripts/merge_request_tabs.js.es6 +++ b/app/assets/javascripts/merge_request_tabs.js.es6 @@ -237,13 +237,8 @@ } this.diffsLoaded = true; - const diffPage = new gl.Diff(); - - const locationHash = gl.utils.getLocationHash(); - const anchoredDiff = locationHash && locationHash.split('_')[0]; - if (anchoredDiff) { - diffPage.openAnchoredDiff(anchoredDiff, () => this.scrollToElement('#diffs')); - } + new gl.Diff(); + this.scrollToElement('#diffs'); }, }); } diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index 603db88567d..fac21f8cd32 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -3,6 +3,7 @@ /* global GLForm */ /* global Autosave */ /* global ResolveService */ +/* global mrRefreshWidgetUrl */ /*= require autosave */ /*= require autosize */ @@ -244,6 +245,16 @@ }; + Notes.prototype.handleCreateChanges = function(note) { + if (typeof note === 'undefined') { + return; + } + + if (note.commands_changes && note.commands_changes.indexOf('merge') !== -1) { + $.get(mrRefreshWidgetUrl); + } + }; + /* Render note in main comments area. @@ -429,6 +440,7 @@ */ Notes.prototype.addNote = function(xhr, note, status) { + this.handleCreateChanges(note); return this.renderNote(note); }; diff --git a/app/assets/javascripts/search.js b/app/assets/javascripts/search.js index 4b6ebadeac7..5945cab4cf0 100644 --- a/app/assets/javascripts/search.js +++ b/app/assets/javascripts/search.js @@ -12,6 +12,9 @@ selectable: true, filterable: true, fieldName: 'group_id', + search: { + fields: ['name'] + }, data: function(term, callback) { return Api.groups(term, {}, function(data) { data.unshift({ @@ -40,6 +43,9 @@ selectable: true, filterable: true, fieldName: 'project_id', + search: { + fields: ['name'] + }, data: function(term, callback) { return Api.projects(term, 'id', function(data) { data.unshift({ diff --git a/app/assets/javascripts/single_file_diff.js b/app/assets/javascripts/single_file_diff.js index ac8603ccd10..9602526063e 100644 --- a/app/assets/javascripts/single_file_diff.js +++ b/app/assets/javascripts/single_file_diff.js @@ -1,4 +1,4 @@ -/* eslint-disable func-names, space-before-function-paren, no-var, space-before-blocks, prefer-rest-params, wrap-iife, one-var, one-var-declaration-per-line, consistent-return, no-param-reassign, padded-blocks, max-len */ +/* eslint-disable func-names, prefer-arrow-callback, space-before-function-paren, no-var, space-before-blocks, prefer-rest-params, wrap-iife, one-var, one-var-declaration-per-line, consistent-return, no-param-reassign, padded-blocks, max-len */ (function() { var bind = function(fn, me){ return function(){ return fn.apply(me, arguments); }; }; @@ -14,8 +14,7 @@ COLLAPSED_HTML = '<div class="nothing-here-block diff-collapsed">This diff is collapsed. <a class="click-to-expand">Click to expand it.</a></div>'; - function SingleFileDiff(file, forceLoad, cb) { - var clickTarget; + function SingleFileDiff(file) { this.file = file; this.toggleDiff = bind(this.toggleDiff, this); this.content = $('.diff-content', this.file); @@ -33,14 +32,13 @@ this.content.after(this.collapsedContent); this.$toggleIcon.addClass('fa-caret-down'); } - clickTarget = $('.file-title, .click-to-expand', this.file).on('click', this.toggleDiff); - if (forceLoad) { - this.toggleDiff({ target: clickTarget }, cb); - } + + $('.file-title, .click-to-expand', this.file).on('click', (function (e) { + this.toggleDiff($(e.target)); + }).bind(this)); } - SingleFileDiff.prototype.toggleDiff = function(e, cb) { - var $target = $(e.target); + SingleFileDiff.prototype.toggleDiff = function($target, cb) { if (!$target.hasClass('file-title') && !$target.hasClass('click-to-expand') && !$target.hasClass('diff-toggle-caret')) return; this.isOpen = !this.isOpen; if (!this.isOpen && !this.hasError) { @@ -91,10 +89,10 @@ })(); - $.fn.singleFileDiff = function(forceLoad, cb) { + $.fn.singleFileDiff = function() { return this.each(function() { - if (!$.data(this, 'singleFileDiff') || forceLoad) { - return $.data(this, 'singleFileDiff', new window.SingleFileDiff(this, forceLoad, cb)); + if (!$.data(this, 'singleFileDiff')) { + return $.data(this, 'singleFileDiff', new window.SingleFileDiff(this)); } }); }; diff --git a/app/assets/javascripts/vue_pipelines_index/stage.js.es6 b/app/assets/javascripts/vue_pipelines_index/stage.js.es6 index 74a79dcedae..f075a995846 100644 --- a/app/assets/javascripts/vue_pipelines_index/stage.js.es6 +++ b/app/assets/javascripts/vue_pipelines_index/stage.js.es6 @@ -1,11 +1,11 @@ /* global Vue, Flash, gl */ -/* eslint-disable no-param-reassign */ +/* eslint-disable no-param-reassign, no-bitwise */ ((gl) => { gl.VueStage = Vue.extend({ data() { return { - request: false, + count: 0, builds: '', spinner: '<span class="fa fa-spinner fa-spin"></span>', }; @@ -13,29 +13,23 @@ props: ['stage', 'svgs', 'match'], methods: { fetchBuilds() { - if (this.request) return this.clearBuilds(); - + if (this.count > 0) return null; return this.$http.get(this.stage.dropdown_path) .then((response) => { - this.request = true; + this.count += 1; this.builds = JSON.parse(response.body).html; }, () => { const flash = new Flash('Something went wrong on our end.'); - this.request = false; return flash; }); }, - clearBuilds() { - this.builds = ''; - this.request = false; - }, }, computed: { buildsOrSpinner() { - return this.request ? this.builds : this.spinner; + return this.builds ? this.builds : this.spinner; }, dropdownClass() { - if (this.request) return 'js-builds-dropdown-container'; + if (this.builds) return 'js-builds-dropdown-container'; return 'js-builds-dropdown-loading builds-dropdown-loading'; }, buildStatus() { @@ -57,7 +51,6 @@ <div> <button @click='fetchBuilds' - @blur='fetchBuilds' :class="triggerButtonClass" :title='stage.title' data-placement="top" diff --git a/app/assets/stylesheets/framework/animations.scss b/app/assets/stylesheets/framework/animations.scss index f1d36efb3de..8d38fc78a19 100644 --- a/app/assets/stylesheets/framework/animations.scss +++ b/app/assets/stylesheets/framework/animations.scss @@ -50,3 +50,77 @@ .pulse { @include webkit-prefix(animation-name, pulse); } + +/* +* General hover animations +*/ + + +// Sass multiple transitions mixin | https://gist.github.com/tobiasahlin/7a421fb9306a4f518aab +// Usage: @include transition(width, height 0.3s ease-in-out); +// Output: -webkit-transition(width 0.2s, height 0.3s ease-in-out); +// transition(width 0.2s, height 0.3s ease-in-out); +// +// Pass in any number of transitions +@mixin transition($transitions...) { + $unfoldedTransitions: (); + @each $transition in $transitions { + $unfoldedTransitions: append($unfoldedTransitions, unfoldTransition($transition), comma); + } + + transition: $unfoldedTransitions; +} + +@function unfoldTransition ($transition) { + // Default values + $property: all; + $duration: $general-hover-transition-duration; + $easing: $general-hover-transition-curve; // Browser default is ease, which is what we want + $delay: null; // Browser default is 0, which is what we want + $defaultProperties: ($property, $duration, $easing, $delay); + + // Grab transition properties if they exist + $unfoldedTransition: (); + @for $i from 1 through length($defaultProperties) { + $p: null; + @if $i <= length($transition) { + $p: nth($transition, $i); + } @else { + $p: nth($defaultProperties, $i); + } + $unfoldedTransition: append($unfoldedTransition, $p); + } + + @return $unfoldedTransition; +} + +.btn, +.side-nav-toggle { + @include transition(background-color, border-color, color, box-shadow); +} + +.dropdown-menu-toggle, +.avatar-circle, +.header-user-avatar { + @include transition(border-color); +} + +.note-action-button .link-highlight, +.toolbar-btn, +.dropdown-toggle-caret, +.fa:not(.fa-bell) { + @include transition(color); +} + +a { + @include transition(background-color, color, border); +} + +.tree-table td, +.well-list > li { + @include transition(background-color, border-color); +} + +.stage-nav-item { + @include transition(background-color, box-shadow); +} diff --git a/app/assets/stylesheets/framework/avatar.scss b/app/assets/stylesheets/framework/avatar.scss index 48827578d94..8392b98f0a7 100644 --- a/app/assets/stylesheets/framework/avatar.scss +++ b/app/assets/stylesheets/framework/avatar.scss @@ -52,6 +52,10 @@ border-radius: 0; border: none; } + + &:not([href]):hover { + border-color: rgba($avatar-border, .2); + } } .identicon { diff --git a/app/assets/stylesheets/framework/header.scss b/app/assets/stylesheets/framework/header.scss index 72b3fe2016c..24a1ce2b84d 100644 --- a/app/assets/stylesheets/framework/header.scss +++ b/app/assets/stylesheets/framework/header.scss @@ -57,6 +57,14 @@ header { &.header-user-dropdown-toggle { margin-left: 14px; + + &:hover, + &:focus, + &:active { + .header-user-avatar { + border-color: rgba($avatar-border, .2); + } + } } &:hover, @@ -104,6 +112,7 @@ header { &:hover { background-color: $white-normal; + color: $gl-header-nav-hover-color; } } } @@ -180,6 +189,7 @@ header { &:hover { text-decoration: underline; + color: $gl-header-nav-hover-color; } } @@ -198,7 +208,7 @@ header { cursor: pointer; &:hover { - color: darken($color: $gl-text-color, $amount: 30%); + color: $gl-header-nav-hover-color; } } @@ -271,4 +281,5 @@ header { float: left; margin-right: 5px; border-radius: 50%; + border: 1px solid $avatar-border; } diff --git a/app/assets/stylesheets/framework/nav.scss b/app/assets/stylesheets/framework/nav.scss index a292e7686f9..401c2d0f6ee 100644 --- a/app/assets/stylesheets/framework/nav.scss +++ b/app/assets/stylesheets/framework/nav.scss @@ -101,7 +101,7 @@ &:hover, &:active, &:focus { - border-bottom: none; + border-color: transparent; } } } diff --git a/app/assets/stylesheets/framework/sidebar.scss b/app/assets/stylesheets/framework/sidebar.scss index 838f5442fff..f0b03710c79 100644 --- a/app/assets/stylesheets/framework/sidebar.scss +++ b/app/assets/stylesheets/framework/sidebar.scss @@ -236,9 +236,13 @@ header.header-sidebar-pinned { @media (min-width: $screen-md-min) { padding-right: $gutter_width; - .merge-request-tabs-holder.affix { + &:not(.with-overlay) .merge-request-tabs-holder.affix { right: $gutter_width; } + + &.with-overlay .merge-request-tabs-holder.affix { + right: $sidebar_collapsed_width; + } } &.with-overlay { diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index 3f95846522b..07cb669a46e 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -104,6 +104,10 @@ $gl-text-red: #d12f19; $gl-text-orange: #d90; $gl-link-color: #3777b0; $gl-grayish-blue: #7f8fa4; +$gl-gray: $gl-text-color; +$gl-gray-dark: #313236; +$gl-header-color: #4c4e54; +$gl-header-nav-hover-color: #434343; /* * Lists @@ -174,6 +178,9 @@ $count-arrow-border: #dce0e5; $save-project-loader-color: #555; $divergence-graph-bar-bg: #ccc; $divergence-graph-separator-bg: #ccc; +$general-hover-transition-duration: 150ms; +$general-hover-transition-curve: linear; + /* * Common component specific colors @@ -534,4 +541,4 @@ Pipeline Graph */ $stage-hover-bg: #eaf3fc; $stage-hover-border: #d1e7fc; -$action-icon-color: #d6d6d6; +$action-icon-color: #d6d6d6;
\ No newline at end of file diff --git a/app/assets/stylesheets/pages/cycle_analytics.scss b/app/assets/stylesheets/pages/cycle_analytics.scss index 6566f27ea2d..cda069e6c0e 100644 --- a/app/assets/stylesheets/pages/cycle_analytics.scss +++ b/app/assets/stylesheets/pages/cycle_analytics.scss @@ -20,6 +20,10 @@ .fa { color: $cycle-analytics-light-gray; + + &:hover { + color: $gl-text-color; + } } .stage-header { diff --git a/app/assets/stylesheets/pages/issuable.scss b/app/assets/stylesheets/pages/issuable.scss index 1825c44e090..324c6cec96a 100644 --- a/app/assets/stylesheets/pages/issuable.scss +++ b/app/assets/stylesheets/pages/issuable.scss @@ -154,8 +154,8 @@ .edit-link { color: $gl-text-color; - &:hover { - color: $md-link-color; + &:not([href]):hover { + color: rgba($avatar-border, .2); } } } @@ -332,6 +332,10 @@ &:hover { color: $md-link-color; text-decoration: none; + + .avatar { + border-color: rgba($avatar-border, .2); + } } } diff --git a/app/assets/stylesheets/pages/labels.scss b/app/assets/stylesheets/pages/labels.scss index 78683c7d574..21d9b4c54ea 100644 --- a/app/assets/stylesheets/pages/labels.scss +++ b/app/assets/stylesheets/pages/labels.scss @@ -203,6 +203,10 @@ z-index: 3; border-radius: $label-border-radius; padding: 6px 10px 6px 9px; + + &:hover { + box-shadow: inset 0 0 0 80px $label-remove-border; + } } .btn { diff --git a/app/assets/stylesheets/pages/profile.scss b/app/assets/stylesheets/pages/profile.scss index 8b1976bd925..722b3006f7c 100644 --- a/app/assets/stylesheets/pages/profile.scss +++ b/app/assets/stylesheets/pages/profile.scss @@ -216,8 +216,8 @@ } } -.user-profile { +.user-profile { .cover-controls a { margin-left: 5px; } @@ -231,8 +231,11 @@ } } - @media (max-width: $screen-xs-max) { + .user-profile-nav { + font-size: 0; + } + @media (max-width: $screen-xs-max) { .cover-block { padding-top: 20px; } @@ -253,6 +256,12 @@ } } } + + .user-profile-nav { + a { + margin-right: 0; + } + } } } @@ -271,4 +280,4 @@ table.u2f-registrations { .scopes-list { padding-left: 18px; } -}
\ No newline at end of file +} diff --git a/app/assets/stylesheets/pages/search.scss b/app/assets/stylesheets/pages/search.scss index cedd4cb2987..12bff32bbf3 100644 --- a/app/assets/stylesheets/pages/search.scss +++ b/app/assets/stylesheets/pages/search.scss @@ -14,6 +14,20 @@ } } +.search form:hover, +.file-finder-input:hover, +.issuable-search-form:hover, +.search-text-input:hover, +textarea:hover, +.form-control:hover { + border-color: lighten($dropdown-input-focus-border, 20%); + box-shadow: 0 0 4px lighten($search-input-focus-shadow-color, 20%); +} + +input[type="checkbox"]:hover { + box-shadow: 0 0 2px 2px lighten($search-input-focus-shadow-color, 20%), 0 0 0 1px lighten($search-input-focus-shadow-color, 20%); +} + .search { margin-right: 10px; margin-left: 10px; diff --git a/app/controllers/projects/compare_controller.rb b/app/controllers/projects/compare_controller.rb index ec02fc15d35..d32966645c8 100644 --- a/app/controllers/projects/compare_controller.rb +++ b/app/controllers/projects/compare_controller.rb @@ -25,8 +25,17 @@ class Projects::CompareController < Projects::ApplicationController end def create - redirect_to namespace_project_compare_path(@project.namespace, @project, + if params[:from].blank? || params[:to].blank? + flash[:alert] = "You must select from and to branches" + from_to_vars = { + from: params[:from].presence, + to: params[:to].presence + } + redirect_to namespace_project_compare_index_path(@project.namespace, @project, from_to_vars) + else + redirect_to namespace_project_compare_path(@project.namespace, @project, params[:from], params[:to]) + end end private diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index aaebd4efa00..9ac5bf4b9f8 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -347,6 +347,16 @@ class Projects::MergeRequestsController < Projects::ApplicationController end end + def merge_widget_refresh + if merge_request.in_progress_merge_commit_sha || merge_request.state == 'merged' + @status = :success + elsif merge_request.merge_when_build_succeeds + @status = :merge_when_build_succeeds + end + + render 'merge' + end + def branch_from # This is always source @source_project = @merge_request.nil? ? @project : @merge_request.source_project diff --git a/app/controllers/projects/notes_controller.rb b/app/controllers/projects/notes_controller.rb index b71509f2c9b..c5d93ce25bc 100644 --- a/app/controllers/projects/notes_controller.rb +++ b/app/controllers/projects/notes_controller.rb @@ -23,7 +23,8 @@ class Projects::NotesController < Projects::ApplicationController end def create - @note = Notes::CreateService.new(project, current_user, note_params).execute + create_params = note_params.merge(merge_request_diff_head_sha: params[:merge_request_diff_head_sha]) + @note = Notes::CreateService.new(project, current_user, create_params).execute if @note.is_a?(Note) Banzai::NoteRenderer.render([@note], @project, current_user) diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index d5ee503c44c..444ff837bb3 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -42,19 +42,16 @@ class ProjectsController < Projects::ApplicationController end def update - status = ::Projects::UpdateService.new(@project, current_user, project_params).execute + result = ::Projects::UpdateService.new(@project, current_user, project_params).execute # Refresh the repo in case anything changed - @repository = project.repository + @repository = @project.repository respond_to do |format| - if status + if result[:status] == :success flash[:notice] = "Project '#{@project.name}' was successfully updated." format.html do - redirect_to( - edit_project_path(@project), - notice: "Project '#{@project.name}' was successfully updated." - ) + redirect_to(edit_project_path(@project)) end else format.html { render 'edit' } diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index c35d6611ab0..aed1d7c839f 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -165,4 +165,10 @@ module DiffHelper link_to "#{hide_whitespace? ? 'Show' : 'Hide'} whitespace changes", url, class: options[:class] end + + def render_overflow_warning?(diff_files) + diffs = @merge_request_diff.presence || diff_files + + diffs.overflow? + end end diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb index 20218775659..8c2c4e8833b 100644 --- a/app/helpers/merge_requests_helper.rb +++ b/app/helpers/merge_requests_helper.rb @@ -19,6 +19,14 @@ module MergeRequestsHelper } end + def mr_widget_refresh_url(mr) + if mr && mr.source_project + merge_widget_refresh_namespace_project_merge_request_url(mr.source_project.namespace, mr.source_project, mr) + else + '' + end + end + def mr_css_classes(mr) classes = "merge-request" classes << " closed" if mr.closed? diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 27042798741..48ffe40abc6 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -507,6 +507,10 @@ module Ci end end + def has_expiring_artifacts? + artifacts_expire_at.present? + end + def keep_artifacts! self.update(artifacts_expire_at: nil) end diff --git a/app/models/commit.rb b/app/models/commit.rb index 0b924b063a4..3365f4ffdbf 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -318,6 +318,14 @@ class Commit Gitlab::Diff::FileCollection::Commit.new(self, diff_options: diff_options) end + def persisted? + true + end + + def touch + # no-op but needs to be defined since #persisted? is defined + end + private def commit_reference(from_project, referable_commit_id, full: false) diff --git a/app/models/concerns/milestoneish.rb b/app/models/concerns/milestoneish.rb index fcc8feddb39..e9450dd0c26 100644 --- a/app/models/concerns/milestoneish.rb +++ b/app/models/concerns/milestoneish.rb @@ -7,11 +7,14 @@ module Milestoneish def total_items_count(user) memoize_per_user(user, :total_items_count) do - issues_count = count_issues_by_state(user).values.sum - issues_count + merge_requests.size + total_issues_count(user) + merge_requests.size end end + def total_issues_count(user) + count_issues_by_state(user).values.sum + end + def complete?(user) total_items_count(user) > 0 && total_items_count(user) == closed_items_count(user) end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 70005a87f4b..10251302db8 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -898,10 +898,22 @@ class MergeRequest < ActiveRecord::Base end def has_commits? - commits_count > 0 + merge_request_diff && commits_count > 0 end def has_no_commits? !has_commits? end + + def mergeable_with_slash_command?(current_user, autocomplete_precheck: false, last_diff_sha: nil) + return false unless can_be_merged_by?(current_user) + + return true if autocomplete_precheck + + return false unless mergeable?(skip_ci_check: true) + return false if head_pipeline && !(head_pipeline.success? || head_pipeline.active?) + return false if last_diff_sha != diff_head_sha + + true + end end diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index 64dd586c9e0..dadb81f9b6e 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -234,28 +234,28 @@ class MergeRequestDiff < ActiveRecord::Base # and save it as array of hashes in st_diffs db field def save_diffs new_attributes = {} - new_diffs = [] if commits.size.zero? new_attributes[:state] = :empty else diff_collection = compare.diffs(Commit.max_diff_options) - - if diff_collection.overflow? - # Set our state to 'overflow' to make the #empty? and #collected? - # methods (generated by StateMachine) return false. - new_attributes[:state] = :overflow - end - - new_attributes[:real_size] = diff_collection.real_size + new_attributes[:real_size] = compare.diffs.real_size if diff_collection.any? new_diffs = dump_diffs(diff_collection) new_attributes[:state] = :collected end + + new_attributes[:st_diffs] = new_diffs || [] + + # Set our state to 'overflow' to make the #empty? and #collected? + # methods (generated by StateMachine) return false. + # + # This attribution has to come at the end of the method so 'overflow' + # state does not get overridden by 'collected'. + new_attributes[:state] = :overflow if diff_collection.overflow? end - new_attributes[:st_diffs] = new_diffs update_columns_serialized(new_attributes) end diff --git a/app/models/project.rb b/app/models/project.rb index e85d3d3bc6c..1630975b0d3 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1032,7 +1032,7 @@ class Project < ActiveRecord::Base "refs/heads/#{branch}", force: true) repository.copy_gitattributes(branch) - repository.expire_avatar_cache + repository.after_change_head reload_default_branch end diff --git a/app/models/repository.rb b/app/models/repository.rb index 3266e9c75f0..43dba86e5ed 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -439,6 +439,11 @@ class Repository expire_content_cache end + # Runs code after the HEAD of a repository is changed. + def after_change_head + expire_method_caches(METHOD_CACHES_FOR_FILE_TYPES.keys) + end + # Runs code after a repository has been forked/imported. def after_import expire_content_cache diff --git a/app/serializers/build_action_entity.rb b/app/serializers/build_action_entity.rb index 3e72892d584..184f5fd4b52 100644 --- a/app/serializers/build_action_entity.rb +++ b/app/serializers/build_action_entity.rb @@ -2,7 +2,7 @@ class BuildActionEntity < Grape::Entity include RequestAwareEntity expose :name do |build| - build.name.humanize + build.name end expose :path do |build| diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb index ad16ef8c70f..3cb9aae83f6 100644 --- a/app/services/merge_requests/update_service.rb +++ b/app/services/merge_requests/update_service.rb @@ -7,6 +7,8 @@ module MergeRequests params.except!(:target_project_id) params.except!(:source_branch) + merge_from_slash_command(merge_request) if params[:merge] + if merge_request.closed_without_fork? params.except!(:target_branch, :force_remove_source_branch) end @@ -69,6 +71,19 @@ module MergeRequests end end + def merge_from_slash_command(merge_request) + last_diff_sha = params.delete(:merge) + return unless merge_request.mergeable_with_slash_command?(current_user, last_diff_sha: last_diff_sha) + + merge_request.update(merge_error: nil) + + if merge_request.head_pipeline && merge_request.head_pipeline.active? + MergeRequests::MergeWhenPipelineSucceedsService.new(project, current_user).execute(merge_request) + else + MergeWorker.perform_async(merge_request.id, current_user.id, {}) + end + end + def reopen_service MergeRequests::ReopenService end diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb index 1beca9f4109..cdd765c85eb 100644 --- a/app/services/notes/create_service.rb +++ b/app/services/notes/create_service.rb @@ -1,6 +1,8 @@ module Notes class CreateService < BaseService def execute + merge_request_diff_head_sha = params.delete(:merge_request_diff_head_sha) + note = project.notes.new(params) note.author = current_user note.system = false @@ -19,7 +21,8 @@ module Notes slash_commands_service = SlashCommandsService.new(project, current_user) if slash_commands_service.supported?(note) - content, command_params = slash_commands_service.extract_commands(note) + options = { merge_request_diff_head_sha: merge_request_diff_head_sha } + content, command_params = slash_commands_service.extract_commands(note, options) only_commands = content.empty? diff --git a/app/services/notes/slash_commands_service.rb b/app/services/notes/slash_commands_service.rb index 2edbd39a9e7..aaea9717fc4 100644 --- a/app/services/notes/slash_commands_service.rb +++ b/app/services/notes/slash_commands_service.rb @@ -19,10 +19,10 @@ module Notes self.class.supported?(note, current_user) end - def extract_commands(note) + def extract_commands(note, options = {}) return [note.note, {}] unless supported?(note) - SlashCommands::InterpretService.new(project, current_user). + SlashCommands::InterpretService.new(project, current_user, options). execute(note.note, note.noteable) end diff --git a/app/services/projects/update_service.rb b/app/services/projects/update_service.rb index 8a6af8d8ada..842e23eb6b6 100644 --- a/app/services/projects/update_service.rb +++ b/app/services/projects/update_service.rb @@ -9,7 +9,7 @@ module Projects Gitlab::VisibilityLevel.allowed_for?(current_user, new_visibility) deny_visibility_level(project, new_visibility) - return project + return error('Visibility level unallowed') end end @@ -23,6 +23,10 @@ module Projects if project.previous_changes.include?('path') project.rename_repo end + + success + else + error('Project could not be updated') end end end diff --git a/app/services/slash_commands/interpret_service.rb b/app/services/slash_commands/interpret_service.rb index ea00415ae1f..a6e35d340e9 100644 --- a/app/services/slash_commands/interpret_service.rb +++ b/app/services/slash_commands/interpret_service.rb @@ -2,7 +2,7 @@ module SlashCommands class InterpretService < BaseService include Gitlab::SlashCommands::Dsl - attr_reader :issuable + attr_reader :issuable, :options # Takes a text and interprets the commands that are extracted from it. # Returns the content without commands, and hash of changes to be applied to a record. @@ -13,7 +13,8 @@ module SlashCommands opts = { issuable: issuable, current_user: current_user, - project: project + project: project, + params: params } content, commands = extractor.extract_commands(content, opts) @@ -58,6 +59,17 @@ module SlashCommands @updates[:state_event] = 'reopen' end + desc 'Merge (when build succeeds)' + condition do + last_diff_sha = params && params[:merge_request_diff_head_sha] + issuable.is_a?(MergeRequest) && + issuable.persisted? && + issuable.mergeable_with_slash_command?(current_user, autocomplete_precheck: !last_diff_sha, last_diff_sha: last_diff_sha) + end + command :merge do + @updates[:merge] = params[:merge_request_diff_head_sha] + end + desc 'Change title' params '<New title>' condition do diff --git a/app/services/users/refresh_authorized_projects_service.rb b/app/services/users/refresh_authorized_projects_service.rb index 21ec1bd9e65..2d211d5ebbe 100644 --- a/app/services/users/refresh_authorized_projects_service.rb +++ b/app/services/users/refresh_authorized_projects_service.rb @@ -26,8 +26,26 @@ module Users user.reload end - # This method returns the updated User object. def execute + lease_key = "refresh_authorized_projects:#{user.id}" + lease = Gitlab::ExclusiveLease.new(lease_key, timeout: LEASE_TIMEOUT) + + until uuid = lease.try_obtain + # Keep trying until we obtain the lease. If we don't do so we may end up + # not updating the list of authorized projects properly. To prevent + # hammering Redis too much we'll wait for a bit between retries. + sleep(1) + end + + begin + execute_without_lease + ensure + Gitlab::ExclusiveLease.cancel(lease_key, uuid) + end + end + + # This method returns the updated User object. + def execute_without_lease current = current_authorizations_per_project fresh = fresh_access_levels_per_project @@ -47,26 +65,7 @@ module Users end end - update_with_lease(remove, add) - end - - # Updates the list of authorizations using an exclusive lease. - def update_with_lease(remove = [], add = []) - lease_key = "refresh_authorized_projects:#{user.id}" - lease = Gitlab::ExclusiveLease.new(lease_key, timeout: LEASE_TIMEOUT) - - until uuid = lease.try_obtain - # Keep trying until we obtain the lease. If we don't do so we may end up - # not updating the list of authorized projects properly. To prevent - # hammering Redis too much we'll wait for a bit between retries. - sleep(1) - end - - begin - update_authorizations(remove, add) - ensure - Gitlab::ExclusiveLease.cancel(lease_key, uuid) - end + update_authorizations(remove, add) end # Updates the list of authorizations for the current user. diff --git a/app/views/profiles/show.html.haml b/app/views/profiles/show.html.haml index 2385a90401e..c0c82cde2f6 100644 --- a/app/views/profiles/show.html.haml +++ b/app/views/profiles/show.html.haml @@ -18,7 +18,8 @@ or change it at #{link_to Gitlab.config.gravatar.host, "http://" + Gitlab.config.gravatar.host} .col-lg-9 .clearfix.avatar-image.append-bottom-default - = image_tag avatar_icon(@user, 160), alt: '', class: 'avatar s160' + = link_to avatar_icon(@user, 400), target: '_blank' do + = image_tag avatar_icon(@user, 160), alt: '', class: 'avatar s160' %h5.prepend-top-0 Upload new avatar .prepend-top-5.append-bottom-10 diff --git a/app/views/projects/branches/_branch.html.haml b/app/views/projects/branches/_branch.html.haml index 2eb49685f08..04efc2e996c 100644 --- a/app/views/projects/branches/_branch.html.haml +++ b/app/views/projects/branches/_branch.html.haml @@ -17,7 +17,6 @@ - if @project.protected_branch? branch.name %span.label.label-success - %i.fa.fa-lock protected .controls.hidden-xs - if merge_project && create_mr_button?(@repository.root_ref, branch.name) diff --git a/app/views/projects/builds/_sidebar.html.haml b/app/views/projects/builds/_sidebar.html.haml index 0b3adcbe121..37bf085130a 100644 --- a/app/views/projects/builds/_sidebar.html.haml +++ b/app/views/projects/builds/_sidebar.html.haml @@ -22,14 +22,14 @@ %p.build-detail-row The artifacts were removed #{time_ago_with_tooltip(@build.artifacts_expire_at)} - - elsif @build.artifacts_expire_at + - elsif @build.has_expiring_artifacts? %p.build-detail-row The artifacts will be removed in %span.js-artifacts-remove= @build.artifacts_expire_at - if @build.artifacts? .btn-group.btn-group-justified{ role: :group } - - if @build.artifacts_expire_at + - if @build.has_expiring_artifacts? && can?(current_user, :update_build, @build) = link_to keep_namespace_project_build_artifacts_path(@project.namespace, @project, @build), class: 'btn btn-sm btn-default', method: :post do Keep diff --git a/app/views/projects/ci/pipelines/_pipeline.html.haml b/app/views/projects/ci/pipelines/_pipeline.html.haml index 6ce586cc8f6..990bfbcf951 100644 --- a/app/views/projects/ci/pipelines/_pipeline.html.haml +++ b/app/views/projects/ci/pipelines/_pipeline.html.haml @@ -86,7 +86,7 @@ %li = link_to play_namespace_project_build_path(pipeline.project.namespace, pipeline.project, build), method: :post, rel: 'nofollow' do = custom_icon('icon_play') - %span= build.name.humanize + %span= build.name - if artifacts.present? .btn-group %button.dropdown-toggle.btn.btn-default.build-artifacts.js-pipeline-dropdown-download{ type: 'button', 'data-toggle' => 'dropdown' } diff --git a/app/views/projects/commit/_change.html.haml b/app/views/projects/commit/_change.html.haml index 12e4280d344..990908211de 100644 --- a/app/views/projects/commit/_change.html.haml +++ b/app/views/projects/commit/_change.html.haml @@ -13,7 +13,7 @@ %a.close{ href: "#", "data-dismiss" => "modal" } × %h3.page-title== #{label} this #{commit.change_type_title(current_user)} .modal-body - = form_tag send("#{type.underscore}_namespace_project_commit_path", @project.namespace, @project, commit.id), method: :post, remote: false, class: "form-horizontal js-#{type}-form js-requires-input" do + = form_tag [type.underscore, @project.namespace, @project, commit], method: :post, remote: false, class: "form-horizontal js-#{type}-form js-requires-input" do .form-group.branch = label_tag 'target_branch', target_label, class: 'control-label' .col-sm-10 diff --git a/app/views/projects/commit/_commit_box.html.haml b/app/views/projects/commit/_commit_box.html.haml index a9ee9230076..08eb0c57f66 100644 --- a/app/views/projects/commit/_commit_box.html.haml +++ b/app/views/projects/commit/_commit_box.html.haml @@ -1,7 +1,7 @@ .page-content-header .header-main-content %strong - = clipboard_button(clipboard_text: @commit.id) + = clipboard_button(clipboard_text: @commit.id, title: "Copy commit SHA to clipboard") = @commit.short_id %span.hidden-xs authored #{time_ago_with_tooltip(@commit.authored_date)} diff --git a/app/views/projects/commits/_commit.html.haml b/app/views/projects/commits/_commit.html.haml index 3c6c50dce3c..002e3d345dc 100644 --- a/app/views/projects/commits/_commit.html.haml +++ b/app/views/projects/commits/_commit.html.haml @@ -36,6 +36,6 @@ .table-list-cell.commit-actions.hidden-xs - if commit.status(ref) = render_commit_status(commit, ref: ref) - = clipboard_button(clipboard_text: commit.id) + = clipboard_button(clipboard_text: commit.id, title: "Copy commit SHA to clipboard") = link_to commit.short_id, namespace_project_commit_path(project.namespace, project, commit), class: "commit-short-id btn btn-transparent" = link_to_browse_code(project, commit) diff --git a/app/views/projects/diffs/_diffs.html.haml b/app/views/projects/diffs/_diffs.html.haml index ab4a2dc36e5..58c20e225c6 100644 --- a/app/views/projects/diffs/_diffs.html.haml +++ b/app/views/projects/diffs/_diffs.html.haml @@ -18,8 +18,8 @@ = parallel_diff_btn = render 'projects/diffs/stats', diff_files: diff_files -- if diff_files.overflow? - = render 'projects/diffs/warning', diff_files: diff_files +- if render_overflow_warning?(diff_files) + = render 'projects/diffs/warning', diff_files: diffs .files{ data: { can_create_note: can_create_note } } - diff_files.each_with_index do |diff_file| diff --git a/app/views/projects/diffs/_file.html.haml b/app/views/projects/diffs/_file.html.haml index 15df2edefc7..c37a33bbcd5 100644 --- a/app/views/projects/diffs/_file.html.haml +++ b/app/views/projects/diffs/_file.html.haml @@ -1,5 +1,5 @@ .diff-file.file-holder{ id: file_hash, data: diff_file_html_data(project, diff_file.file_path, diff_commit.id) } - .file-title{ id: "file-path-#{hexdigest(diff_file.file_path)}" } + .file-title = render "projects/diffs/file_header", diff_file: diff_file, blob: blob, diff_commit: diff_commit, project: project, url: "##{file_hash}" - unless diff_file.submodule? diff --git a/app/views/projects/merge_requests/_show.html.haml b/app/views/projects/merge_requests/_show.html.haml index 0516801cdf3..2a7cd3a19d0 100644 --- a/app/views/projects/merge_requests/_show.html.haml +++ b/app/views/projects/merge_requests/_show.html.haml @@ -113,3 +113,6 @@ merge_request = new MergeRequest({ action: "#{controller.action_name}" }); + + var mrRefreshWidgetUrl = "#{mr_widget_refresh_url(@merge_request)}"; + diff --git a/app/views/projects/merge_requests/show/_diffs.html.haml b/app/views/projects/merge_requests/show/_diffs.html.haml index 99c71e1454a..5f048d04b27 100644 --- a/app/views/projects/merge_requests/show/_diffs.html.haml +++ b/app/views/projects/merge_requests/show/_diffs.html.haml @@ -1,13 +1,5 @@ -- if @merge_request_diff.collected? +- if @merge_request_diff.collected? || @merge_request_diff.overflow? = render 'projects/merge_requests/show/versions' = render "projects/diffs/diffs", diffs: @diffs - elsif @merge_request_diff.empty? .nothing-here-block Nothing to merge from #{@merge_request.source_branch} into #{@merge_request.target_branch} -- else - .alert.alert-warning - %h4 - Changes view for this comparison is extremely large. - %p - You can - = link_to "download it", merge_request_path(@merge_request, format: :diff), class: "vlink" - instead. diff --git a/app/views/projects/notes/_form.html.haml b/app/views/projects/notes/_form.html.haml index 39731668a61..b561052e721 100644 --- a/app/views/projects/notes/_form.html.haml +++ b/app/views/projects/notes/_form.html.haml @@ -3,6 +3,7 @@ = form_for [@project.namespace.becomes(Namespace), @project, @note], remote: true, html: { :'data-type' => 'json', multipart: true, id: nil, class: "new-note js-new-note-form js-quick-submit common-note-form", "data-noteable-iid" => @note.noteable.try(:iid), }, authenticity_token: true do |f| = hidden_field_tag :view, diff_view = hidden_field_tag :line_type + = hidden_field_tag :merge_request_diff_head_sha, @note.noteable.try(:diff_head_sha) = note_target_fields(@note) = f.hidden_field :commit_id = f.hidden_field :line_code diff --git a/app/views/projects/tree/_tree_content.html.haml b/app/views/projects/tree/_tree_content.html.haml index 038a960bd0c..2c08221565b 100644 --- a/app/views/projects/tree/_tree_content.html.haml +++ b/app/views/projects/tree/_tree_content.html.haml @@ -8,7 +8,7 @@ .pull-left Last commit .last-commit.hidden-sm.pull-left %small.light - = clipboard_button(clipboard_text: @commit.id) + = clipboard_button(clipboard_text: @commit.id, title: "Copy commit SHA to clipboard") = link_to @commit.short_id, namespace_project_commit_path(@project.namespace, @project, @commit), class: "monospace" = time_ago_with_tooltip(@commit.committed_date) = @commit.full_title diff --git a/app/views/shared/milestones/_milestone.html.haml b/app/views/shared/milestones/_milestone.html.haml index 3200aacf542..9e6a76e1ddb 100644 --- a/app/views/shared/milestones/_milestone.html.haml +++ b/app/views/shared/milestones/_milestone.html.haml @@ -9,7 +9,7 @@ .pull-right.light #{milestone.percent_complete(current_user)}% complete .row .col-sm-6 - = link_to pluralize(milestone.issues_visible_to_user(current_user).size, 'Issue'), issues_path + = link_to pluralize(milestone.total_issues_count(current_user), 'Issue'), issues_path · = link_to pluralize(milestone.merge_requests.size, 'Merge Request'), merge_requests_path .col-sm-6= milestone_progress_bar(milestone) diff --git a/changelogs/unreleased/22111-remove-lock-icon-on-protected-tag.yml b/changelogs/unreleased/22111-remove-lock-icon-on-protected-tag.yml new file mode 100644 index 00000000000..e4f7c1b7762 --- /dev/null +++ b/changelogs/unreleased/22111-remove-lock-icon-on-protected-tag.yml @@ -0,0 +1,4 @@ +--- +title: Remove Lock Icon on Protected Tag +merge_request: 8513 +author: Sergey Nikitin diff --git a/changelogs/unreleased/24915_merge_slash_command.yml b/changelogs/unreleased/24915_merge_slash_command.yml new file mode 100644 index 00000000000..eb8ced8ab01 --- /dev/null +++ b/changelogs/unreleased/24915_merge_slash_command.yml @@ -0,0 +1,4 @@ +--- +title: Support slash comand `/merge` for merging merge requests. +merge_request: 7746 +author: Jarka Kadlecova diff --git a/changelogs/unreleased/25946-manual-pipeline-dropdown-casing.yml b/changelogs/unreleased/25946-manual-pipeline-dropdown-casing.yml new file mode 100644 index 00000000000..b753c823348 --- /dev/null +++ b/changelogs/unreleased/25946-manual-pipeline-dropdown-casing.yml @@ -0,0 +1,4 @@ +--- +title: Use original casing for build action text +merge_request: 8387 +author: diff --git a/changelogs/unreleased/26207-add-hover-animations.yml b/changelogs/unreleased/26207-add-hover-animations.yml new file mode 100644 index 00000000000..12a69d04717 --- /dev/null +++ b/changelogs/unreleased/26207-add-hover-animations.yml @@ -0,0 +1,4 @@ +--- +title: Add various hover animations throughout the application +merge_request: +author: diff --git a/changelogs/unreleased/26616-fix-search-group-project-filters.yml b/changelogs/unreleased/26616-fix-search-group-project-filters.yml new file mode 100644 index 00000000000..0fd0dbbfc24 --- /dev/null +++ b/changelogs/unreleased/26616-fix-search-group-project-filters.yml @@ -0,0 +1,4 @@ +--- +title: Fix search group/project filtering to show results +merge_request: +author: diff --git a/changelogs/unreleased/clipboard-button-commit-sha.yml b/changelogs/unreleased/clipboard-button-commit-sha.yml new file mode 100644 index 00000000000..6aa4a5664e7 --- /dev/null +++ b/changelogs/unreleased/clipboard-button-commit-sha.yml @@ -0,0 +1,3 @@ +--- +title: 'Copy commit SHA to clipboard' +merge_request: 8547 diff --git a/changelogs/unreleased/dot-in-project-queries.yml b/changelogs/unreleased/dot-in-project-queries.yml new file mode 100644 index 00000000000..fc48dc7b74d --- /dev/null +++ b/changelogs/unreleased/dot-in-project-queries.yml @@ -0,0 +1,4 @@ +--- +title: Allow API query to find projects with dots in their name +merge_request: +author: Bruno Melli diff --git a/changelogs/unreleased/fix-keep-artifacts-button-visibility.yml b/changelogs/unreleased/fix-keep-artifacts-button-visibility.yml new file mode 100644 index 00000000000..3d8cf1c74a2 --- /dev/null +++ b/changelogs/unreleased/fix-keep-artifacts-button-visibility.yml @@ -0,0 +1,4 @@ +--- +title: Hide build artifacts keep button if operation is not allowed +merge_request: 8501 +author: diff --git a/changelogs/unreleased/i--25814-500-error.yml b/changelogs/unreleased/i--25814-500-error.yml new file mode 100644 index 00000000000..cd55ede84c8 --- /dev/null +++ b/changelogs/unreleased/i--25814-500-error.yml @@ -0,0 +1,4 @@ +--- +title: Fix Compare page throws 500 error when any branch/reference is not selected +merge_request: 8492 +author: Martin Cabrera diff --git a/changelogs/unreleased/issue_25017.yml b/changelogs/unreleased/issue_25017.yml new file mode 100644 index 00000000000..09126ae81bc --- /dev/null +++ b/changelogs/unreleased/issue_25017.yml @@ -0,0 +1,4 @@ +--- +title: Show 'too many changes' message for created merge requests when they are too large +merge_request: +author: diff --git a/changelogs/unreleased/mr-tabs-alignment-sidebar-open.yml b/changelogs/unreleased/mr-tabs-alignment-sidebar-open.yml new file mode 100644 index 00000000000..b8c7b78cf0d --- /dev/null +++ b/changelogs/unreleased/mr-tabs-alignment-sidebar-open.yml @@ -0,0 +1,4 @@ +--- +title: Fixed merge request tabs dont move when opening collapsed sidebar +merge_request: +author: diff --git a/changelogs/unreleased/pmq20-gitlab-ce-psvr-head-cache.yml b/changelogs/unreleased/pmq20-gitlab-ce-psvr-head-cache.yml new file mode 100644 index 00000000000..23230128dc9 --- /dev/null +++ b/changelogs/unreleased/pmq20-gitlab-ce-psvr-head-cache.yml @@ -0,0 +1,4 @@ +--- +title: Expire related caches after changing HEAD +merge_request: +author: Minqi Pan diff --git a/changelogs/unreleased/reduce-queries-milestone-index.yml b/changelogs/unreleased/reduce-queries-milestone-index.yml new file mode 100644 index 00000000000..a779b58c973 --- /dev/null +++ b/changelogs/unreleased/reduce-queries-milestone-index.yml @@ -0,0 +1,4 @@ +--- +title: Use cached values to compute total issues count in milestone index pages +merge_request: 8518 +author: diff --git a/changelogs/unreleased/refresh-authorizations-tighter-lease.yml b/changelogs/unreleased/refresh-authorizations-tighter-lease.yml new file mode 100644 index 00000000000..ab42b2eb72d --- /dev/null +++ b/changelogs/unreleased/refresh-authorizations-tighter-lease.yml @@ -0,0 +1,4 @@ +--- +title: Synchronize all project authorization refreshing work to prevent race conditions +merge_request: +author: diff --git a/changelogs/unreleased/sandish-gitlab-ce-update_ret_val.yml b/changelogs/unreleased/sandish-gitlab-ce-update_ret_val.yml new file mode 100644 index 00000000000..7107ddfd982 --- /dev/null +++ b/changelogs/unreleased/sandish-gitlab-ce-update_ret_val.yml @@ -0,0 +1,4 @@ +--- +title: Ensure updating project settings shows a flash message on success +merge_request: 8579 +author: Sandish Chen diff --git a/changelogs/unreleased/switch-to-sassc.yml b/changelogs/unreleased/switch-to-sassc.yml new file mode 100644 index 00000000000..3e6c4baf6d9 --- /dev/null +++ b/changelogs/unreleased/switch-to-sassc.yml @@ -0,0 +1,4 @@ +--- +title: Switch to sassc-rails for faster stylesheet compilation +merge_request: 8556 +author: Richard Macklin diff --git a/config/routes/project.rb b/config/routes/project.rb index 26e2dc9e6e7..1fc6ed28c74 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -94,6 +94,7 @@ constraints(ProjectUrlConstrainer.new) do get :pipelines get :merge_check post :merge + get :merge_widget_refresh post :cancel_merge_when_build_succeeds get :ci_status get :ci_environments_status diff --git a/db/migrate/20161226122833_remove_dot_git_from_usernames.rb b/db/migrate/20161226122833_remove_dot_git_from_usernames.rb index 7d97339581f..a0ce927161f 100644 --- a/db/migrate/20161226122833_remove_dot_git_from_usernames.rb +++ b/db/migrate/20161226122833_remove_dot_git_from_usernames.rb @@ -14,16 +14,25 @@ class RemoveDotGitFromUsernames < ActiveRecord::Migration namespace_id = user['namespace_id'] path_was = user['username'] path_was_wildcard = quote_string("#{path_was}/%") + path = quote_string(new_path(path_was)) - path = move_namespace(namespace_id, path_was, path) + move_namespace(namespace_id, path_was, path) - execute "UPDATE routes SET path = '#{path}' WHERE source_type = 'Namespace' AND source_id = #{namespace_id}" - execute "UPDATE namespaces SET path = '#{path}' WHERE id = #{namespace_id}" - execute "UPDATE users SET username = '#{path}' WHERE id = #{id}" + begin + execute "UPDATE routes SET path = '#{path}' WHERE source_type = 'Namespace' AND source_id = #{namespace_id}" + execute "UPDATE namespaces SET path = '#{path}' WHERE id = #{namespace_id}" + execute "UPDATE users SET username = '#{path}' WHERE id = #{id}" - select_all("SELECT id, path FROM routes WHERE path LIKE '#{path_was_wildcard}'").each do |route| - new_path = "#{path}/#{route['path'].split('/').last}" - execute "UPDATE routes SET path = '#{new_path}' WHERE id = #{route['id']}" + select_all("SELECT id, path FROM routes WHERE path LIKE '#{path_was_wildcard}'").each do |route| + new_path = "#{path}/#{route['path'].split('/').last}" + execute "UPDATE routes SET path = '#{new_path}' WHERE id = #{route['id']}" + end + rescue => e + say("Couldn't update routes for path #{path_was} to #{path}") + # Move namespace back + move_namespace(namespace_id, path, path_was) + + raise e end end end @@ -44,23 +53,30 @@ class RemoveDotGitFromUsernames < ActiveRecord::Migration select_all("SELECT id, path FROM routes WHERE path = '#{quote_string(path)}'").present? end - def path_exists?(repository_storage_path, path) - gitlab_shell.exists?(repository_storage_path, path) + def path_exists?(path, repository_storage_path) + repository_storage_path && gitlab_shell.exists?(repository_storage_path, path) end # Accepts invalid path like test.git and returns test_git or # test_git1 if test_git already taken - def rename_path(repository_storage_path, path) + def new_path(path) # To stay closer with original name and reduce risk of duplicates # we rename suffix instead of removing it path = path.sub(/\.git\z/, '_git') - counter = 0 - base = path + check_routes(path.dup, 0, path) + end + + def check_routes(base, counter, path) + route_exists = route_exists?(path) - while route_exists?(path) || path_exists?(repository_storage_path, path) - counter += 1 - path = "#{base}#{counter}" + Gitlab.config.repositories.storages.each_value do |storage| + if route_exists || path_exists?(path, storage) + counter += 1 + path = "#{base}#{counter}" + + return check_routes(base, counter, path) + end end path @@ -76,8 +92,6 @@ class RemoveDotGitFromUsernames < ActiveRecord::Migration # Ensure old directory exists before moving it gitlab_shell.add_namespace(repository_storage_path, path_was) - path = quote_string(rename_path(repository_storage_path, path_was)) - unless gitlab_shell.mv_namespace(repository_storage_path, path_was, path) Rails.logger.error "Exception moving path #{repository_storage_path} from #{path_was} to #{path}" @@ -87,8 +101,14 @@ class RemoveDotGitFromUsernames < ActiveRecord::Migration end end - Gitlab::UploadsTransfer.new.rename_namespace(path_was, path) - - path + begin + Gitlab::UploadsTransfer.new.rename_namespace(path_was, path) + rescue => e + if path.nil? + say("Couldn't find a storage path for #{namespace_id}, #{path_was} -- skipping") + else + raise e + end + end end end diff --git a/doc/user/project/slash_commands.md b/doc/user/project/slash_commands.md index 87c9756ea5d..a6546cffce2 100644 --- a/doc/user/project/slash_commands.md +++ b/doc/user/project/slash_commands.md @@ -14,6 +14,7 @@ do. |:---------------------------|:-------------| | `/close` | Close the issue or merge request | | `/reopen` | Reopen the issue or merge request | +| `/merge` | Merge (when build succeeds) | | `/title <New title>` | Change title | | `/assign @username` | Assign | | `/unassign` | Remove assignee | diff --git a/doc/workflow/importing/migrating_from_svn.md b/doc/workflow/importing/migrating_from_svn.md index 423b095e69e..7a3628a39d7 100644 --- a/doc/workflow/importing/migrating_from_svn.md +++ b/doc/workflow/importing/migrating_from_svn.md @@ -79,7 +79,7 @@ Now that SubGit has configured the Git/SVN repos, run `subgit` to perform the initial translation of existing SVN revisions into the Git repository: ``` -subgit install $GIT_REPOS_PATH +subgit install $GIT_REPO_PATH ``` After the initial translation is completed, the Git repository and the SVN diff --git a/features/admin/groups.feature b/features/admin/groups.feature deleted file mode 100644 index 657e847cf4a..00000000000 --- a/features/admin/groups.feature +++ /dev/null @@ -1,49 +0,0 @@ -@admin -Feature: Admin Groups - Background: - Given I sign in as an admin - And I have group with projects - And User "John Doe" exists - And I visit admin groups page - - Scenario: See group list - Then I should be all groups - - Scenario: Create a group - When I click new group link - And submit form with new group info - Then I should be redirected to group page - And I should see newly created group - - @javascript - Scenario: Add user into projects in group - When I visit admin group page - When I select user "John Doe" from user list as "Reporter" - Then I should see "John Doe" in team list in every project as "Reporter" - - Scenario: Shared projects - Given group has shared projects - When I visit group page - Then I should see project shared with group - - @javascript - Scenario: Invite user to a group by e-mail - When I visit admin group page - When I select user "johndoe@gitlab.com" from user list as "Reporter" - Then I should see "johndoe@gitlab.com" in team list in every project as "Reporter" - - @javascript - Scenario: Signed in admin should be able to add himself to a group - Given "John Doe" is owner of group "Owned" - When I visit group "Owned" members page - When I select current user as "Developer" - Then I should see current user as "Developer" - - @javascript - Scenario: Signed in admin should be able to remove himself from group - Given current user is developer of group "Owned" - When I visit group "Owned" members page - Then I should see current user as "Developer" - When I click on the "Remove User From Group" button for current user - When I visit group "Owned" members page - Then I should not see current user as "Developer" diff --git a/features/steps/admin/groups.rb b/features/steps/admin/groups.rb deleted file mode 100644 index 9396a76f0a2..00000000000 --- a/features/steps/admin/groups.rb +++ /dev/null @@ -1,143 +0,0 @@ -class Spinach::Features::AdminGroups < Spinach::FeatureSteps - include SharedAuthentication - include SharedGroup - include SharedPaths - include SharedUser - include SharedActiveTab - include Select2Helper - - When 'I visit admin group page' do - visit admin_group_path(current_group) - end - - When 'I click new group link' do - click_link "New Group" - end - - step 'I have group with projects' do - @group = create(:group) - @project = create(:project, group: @group) - @event = create(:closed_issue_event, project: @project) - - @project.team << [current_user, :master] - end - - step 'submit form with new group info' do - fill_in 'group_path', with: 'gitlab' - fill_in 'group_description', with: 'Group description' - click_button "Create group" - end - - step 'I should see newly created group' do - expect(page).to have_content "Group: gitlab" - expect(page).to have_content "Group description" - end - - step 'I should be redirected to group page' do - expect(current_path).to eq admin_group_path(Group.find_by(path: 'gitlab')) - end - - When 'I select user "John Doe" from user list as "Reporter"' do - select2(user_john.id, from: "#user_ids", multiple: true) - page.within "#new_project_member" do - select "Reporter", from: "access_level" - end - click_button "Add users to group" - end - - When 'I select user "johndoe@gitlab.com" from user list as "Reporter"' do - select2('johndoe@gitlab.com', from: "#user_ids", multiple: true) - page.within "#new_project_member" do - select "Reporter", from: "access_level" - end - click_button "Add users to group" - end - - step 'I should see "John Doe" in team list in every project as "Reporter"' do - page.within ".group-users-list" do - expect(page).to have_content "John Doe" - expect(page).to have_content "Reporter" - end - end - - step 'I should see "johndoe@gitlab.com" in team list in every project as "Reporter"' do - page.within ".group-users-list" do - expect(page).to have_content "johndoe@gitlab.com" - expect(page).to have_content "Invited by" - expect(page).to have_content "Reporter" - end - end - - step 'I should be all groups' do - Group.all.each do |group| - expect(page).to have_content group.name - end - end - - step 'group has shared projects' do - share_link = shared_project.project_group_links.new(group_access: Gitlab::Access::MASTER) - share_link.group_id = current_group.id - share_link.save! - end - - step 'I visit group page' do - visit admin_group_path(current_group) - end - - step 'I should see project shared with group' do - expect(page).to have_content(shared_project.name_with_namespace) - expect(page).to have_content "Projects shared with" - end - - step 'we have user "John Doe" in group' do - current_group.add_reporter(user_john) - end - - step 'I should not see "John Doe" in team list' do - page.within ".group-users-list" do - expect(page).not_to have_content "John Doe" - end - end - - step 'I select current user as "Developer"' do - page.within ".users-group-form" do - select2(current_user.id, from: "#user_ids", multiple: true) - select "Developer", from: "access_level" - end - - click_button "Add to group" - end - - step 'I should see current user as "Developer"' do - page.within '.content-list' do - expect(page).to have_content(current_user.name) - expect(page).to have_content('Developer') - end - end - - step 'I click on the "Remove User From Group" button for current user' do - find(:css, 'li', text: current_user.name).find(:css, 'a.btn-remove').click - # poltergeist always confirms popups. - end - - step 'I should not see current user as "Developer"' do - page.within '.content-list' do - expect(page).not_to have_content(current_user.name) - expect(page).not_to have_content('Developer') - end - end - - protected - - def current_group - @group ||= Group.first - end - - def shared_project - @shared_project ||= create(:empty_project) - end - - def user_john - @user_john ||= User.find_by(name: "John Doe") - end -end diff --git a/features/steps/shared/paths.rb b/features/steps/shared/paths.rb index 15b81fa529b..670e6ca49a3 100644 --- a/features/steps/shared/paths.rb +++ b/features/steps/shared/paths.rb @@ -191,10 +191,6 @@ module SharedPaths visit admin_background_jobs_path end - step 'I visit admin groups page' do - visit admin_groups_path - end - step 'I visit admin teams page' do visit admin_teams_path end diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 20b5bc1502a..eb2d370c68e 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -294,7 +294,7 @@ module API header['X-Sendfile'] = path body else - file FileStreamer.new(path) + path end end diff --git a/lib/api/projects.rb b/lib/api/projects.rb index 3be14e8eb76..941f47114a4 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -159,7 +159,7 @@ module API use :sort_params use :pagination end - get "/search/:query" do + get "/search/:query", requirements: { query: /[^\/]+/ } do search_service = Search::GlobalService.new(current_user, search: params[:query]).execute projects = search_service.objects('projects', params[:page]) projects = projects.reorder(params[:order_by] => params[:sort]) @@ -295,13 +295,13 @@ module API authorize! :rename_project, user_project if attrs[:name].present? authorize! :change_visibility_level, user_project if attrs[:visibility_level].present? - ::Projects::UpdateService.new(user_project, current_user, attrs).execute + result = ::Projects::UpdateService.new(user_project, current_user, attrs).execute - if user_project.errors.any? - render_validation_error!(user_project) - else + if result[:status] == :success present user_project, with: Entities::Project, user_can_admin_project: can?(current_user, :admin_project, user_project) + else + render_validation_error!(user_project) end end diff --git a/lib/gitlab/git/blame.rb b/lib/gitlab/git/blame.rb index 2913230e979..58193391926 100644 --- a/lib/gitlab/git/blame.rb +++ b/lib/gitlab/git/blame.rb @@ -1,5 +1,3 @@ -require_relative 'encoding_helper' - module Gitlab module Git class Blame diff --git a/lib/gitlab/git/blob.rb b/lib/gitlab/git/blob.rb index 4a623311c14..b742d9e1e4b 100644 --- a/lib/gitlab/git/blob.rb +++ b/lib/gitlab/git/blob.rb @@ -1,6 +1,3 @@ -require_relative 'encoding_helper' -require_relative 'path_helper' - module Gitlab module Git class Blob diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 79b23d59b3a..7068e68a855 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -1,6 +1,4 @@ # Gitlab::Git::Repository is a wrapper around native Rugged::Repository object -require_relative 'encoding_helper' -require_relative 'path_helper' require 'forwardable' require 'tempfile' require 'forwardable' diff --git a/lib/tasks/dev.rake b/lib/tasks/dev.rake index 6f27972c4e4..5e94fba97bf 100644 --- a/lib/tasks/dev.rake +++ b/lib/tasks/dev.rake @@ -7,9 +7,4 @@ namespace :dev do Rake::Task["gitlab:setup"].invoke Rake::Task["gitlab:shell:setup"].invoke end - - desc 'GitLab | Start/restart foreman and watch for changes' - task :foreman => :environment do - sh 'rerun --dir app,config,lib -- foreman start' - end end diff --git a/spec/controllers/projects/compare_controller_spec.rb b/spec/controllers/projects/compare_controller_spec.rb index 7a57801c437..b03c4b52de6 100644 --- a/spec/controllers/projects/compare_controller_spec.rb +++ b/spec/controllers/projects/compare_controller_spec.rb @@ -64,6 +64,36 @@ describe Projects::CompareController do expect(assigns(:diffs)).to eq(nil) expect(assigns(:commits)).to eq(nil) end + + it 'redirects back to index when params[:from] is empty and preserves params[:to]' do + post(:create, + namespace_id: project.namespace.to_param, + project_id: project.to_param, + from: '', + to: 'master') + + expect(response).to redirect_to(namespace_project_compare_index_path(project.namespace, project, to: 'master')) + end + + it 'redirects back to index when params[:to] is empty and preserves params[:from]' do + post(:create, + namespace_id: project.namespace.to_param, + project_id: project.to_param, + from: 'master', + to: '') + + expect(response).to redirect_to(namespace_project_compare_index_path(project.namespace, project, from: 'master')) + end + + it 'redirects back to index when params[:from] and params[:to] are empty' do + post(:create, + namespace_id: project.namespace.to_param, + project_id: project.to_param, + from: '', + to: '') + + expect(response).to redirect_to(namespace_project_compare_index_path) + end end describe 'GET diff_for_path' do diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index 2a411d78395..7ea3ea4f376 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -1048,4 +1048,72 @@ describe Projects::MergeRequestsController do end end end + + describe 'GET merge_widget_refresh' do + let(:params) do + { + namespace_id: project.namespace.path, + project_id: project.path, + id: merge_request.iid, + format: :raw + } + end + + before do + project.team << [user, :developer] + xhr :get, :merge_widget_refresh, params + end + + context 'when merge in progress' do + let(:merge_request) { create(:merge_request, source_project: project, in_progress_merge_commit_sha: 'sha') } + + it 'returns an OK response' do + expect(response).to have_http_status(:ok) + end + + it 'sets status to :success' do + expect(assigns(:status)).to eq(:success) + expect(response).to render_template('merge') + end + end + + context 'when merge request was merged already' do + let(:merge_request) { create(:merge_request, source_project: project, state: :merged) } + + it 'returns an OK response' do + expect(response).to have_http_status(:ok) + end + + it 'sets status to :success' do + expect(assigns(:status)).to eq(:success) + expect(response).to render_template('merge') + end + end + + context 'when waiting for build' do + let(:merge_request) { create(:merge_request, source_project: project, merge_when_build_succeeds: true, merge_user: user) } + + it 'returns an OK response' do + expect(response).to have_http_status(:ok) + end + + it 'sets status to :merge_when_build_succeeds' do + expect(assigns(:status)).to eq(:merge_when_build_succeeds) + expect(response).to render_template('merge') + end + end + + context 'when no special status for MR' do + let(:merge_request) { create(:merge_request, source_project: project) } + + it 'returns an OK response' do + expect(response).to have_http_status(:ok) + end + + it 'sets status to nil' do + expect(assigns(:status)).to be_nil + expect(response).to render_template('merge') + end + end + end end diff --git a/spec/controllers/projects/notes_controller_spec.rb b/spec/controllers/projects/notes_controller_spec.rb index 92e38b02615..9f6d4ec6537 100644 --- a/spec/controllers/projects/notes_controller_spec.rb +++ b/spec/controllers/projects/notes_controller_spec.rb @@ -14,6 +14,54 @@ describe Projects::NotesController do } end + describe 'POST create' do + let(:merge_request) { create(:merge_request) } + let(:request_params) do + { + note: { note: 'some note', noteable_id: merge_request.id, noteable_type: 'MergeRequest' }, + namespace_id: project.namespace, + project_id: project, + merge_request_diff_head_sha: 'sha' + } + end + + before do + sign_in(user) + project.team << [user, :developer] + end + + it "returns status 302 for html" do + post :create, request_params + + expect(response).to have_http_status(302) + end + + it "returns status 200 for json" do + post :create, request_params.merge(format: :json) + + expect(response).to have_http_status(200) + end + + context 'when merge_request_diff_head_sha present' do + before do + service_params = { + note: 'some note', + noteable_id: merge_request.id.to_s, + noteable_type: 'MergeRequest', + merge_request_diff_head_sha: 'sha' + } + + expect(Notes::CreateService).to receive(:new).with(project, user, service_params).and_return(double(execute: true)) + end + + it "returns status 302 for html" do + post :create, request_params + + expect(response).to have_http_status(302) + end + end + end + describe 'POST toggle_award_emoji' do before do sign_in(user) diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 5ddcaa60dc6..d0a63aa9403 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -245,7 +245,7 @@ describe ProjectsController do expect(project.repository.path).to include(new_path) expect(assigns(:repository).path).to eq(project.repository.path) - expect(response).to have_http_status(200) + expect(response).to have_http_status(302) end end diff --git a/spec/features/admin/admin_groups_spec.rb b/spec/features/admin/admin_groups_spec.rb index 9c19db6b420..a871e370ba2 100644 --- a/spec/features/admin/admin_groups_spec.rb +++ b/spec/features/admin/admin_groups_spec.rb @@ -1,15 +1,39 @@ require 'spec_helper' feature 'Admin Groups', feature: true do + include Select2Helper + let(:internal) { Gitlab::VisibilityLevel::INTERNAL } + let(:user) { create :user } + let!(:group) { create :group } + let!(:current_user) { login_as :admin } before do - login_as(:admin) - stub_application_setting(default_group_visibility: internal) end + describe 'list' do + it 'renders groups' do + visit admin_groups_path + + expect(page).to have_content(group.name) + end + end + describe 'create a group' do + it 'creates new group' do + visit admin_groups_path + + click_link "New Group" + fill_in 'group_path', with: 'gitlab' + fill_in 'group_description', with: 'Group description' + click_button "Create group" + + expect(current_path).to eq admin_group_path(Group.find_by(path: 'gitlab')) + expect(page).to have_content('Group: gitlab') + expect(page).to have_content('Group description') + end + scenario 'shows the visibility level radio populated with the default value' do visit new_admin_group_path @@ -37,6 +61,91 @@ feature 'Admin Groups', feature: true do end end + describe 'add user into a group', js: true do + shared_context 'adds user into a group' do + it do + visit admin_group_path(group) + + select2(user_selector, from: '#user_ids', multiple: true) + page.within '#new_project_member' do + select2(Gitlab::Access::REPORTER, from: '#access_level') + end + click_button "Add users to group" + page.within ".group-users-list" do + expect(page).to have_content(user.name) + expect(page).to have_content('Reporter') + end + end + end + + it_behaves_like 'adds user into a group' do + let(:user_selector) { user.id } + end + + it_behaves_like 'adds user into a group' do + let(:user_selector) { user.email } + end + end + + describe 'add admin himself to a group' do + before do + group.add_user(:user, Gitlab::Access::OWNER) + end + + it 'adds admin a to a group as developer', js: true do + visit group_group_members_path(group) + + page.within '.users-group-form' do + select2(current_user.id, from: '#user_ids', multiple: true) + select 'Developer', from: 'access_level' + end + + click_button 'Add to group' + + page.within '.content-list' do + expect(page).to have_content(current_user.name) + expect(page).to have_content('Developer') + end + end + end + + describe 'admin remove himself from a group', js: true do + it 'removes admin from the group' do + group.add_user(current_user, Gitlab::Access::DEVELOPER) + + visit group_group_members_path(group) + + page.within '.content-list' do + expect(page).to have_content(current_user.name) + expect(page).to have_content('Developer') + end + + find(:css, 'li', text: current_user.name).find(:css, 'a.btn-remove').click + + visit group_group_members_path(group) + + page.within '.content-list' do + expect(page).not_to have_content(current_user.name) + expect(page).not_to have_content('Developer') + end + end + end + + describe 'shared projects' do + it 'renders shared project' do + empty_project = create(:empty_project) + empty_project.project_group_links.create!( + group_access: Gitlab::Access::MASTER, + group: group + ) + + visit admin_group_path(group) + + expect(page).to have_content(empty_project.name_with_namespace) + expect(page).to have_content('Projects shared with') + end + end + def expect_selected_visibility(level) selector = "#group_visibility_level_#{level}[checked=checked]" diff --git a/spec/features/boards/sidebar_spec.rb b/spec/features/boards/sidebar_spec.rb index c16aafa1470..188d33e8ef4 100644 --- a/spec/features/boards/sidebar_spec.rb +++ b/spec/features/boards/sidebar_spec.rb @@ -125,7 +125,9 @@ describe 'Issue Boards', feature: true, js: true do first('.card').click end - page.within('.assignee') do + page.within(find('.assignee')) do + expect(page).to have_content('No assignee') + click_link 'assign yourself' wait_for_vue_resource diff --git a/spec/features/expand_collapse_diffs_spec.rb b/spec/features/expand_collapse_diffs_spec.rb index 3934c936f20..8b3e2fa93a2 100644 --- a/spec/features/expand_collapse_diffs_spec.rb +++ b/spec/features/expand_collapse_diffs_spec.rb @@ -4,10 +4,10 @@ feature 'Expand and collapse diffs', js: true, feature: true do include WaitForAjax let(:branch) { 'expand-collapse-diffs' } + let(:project) { create(:project) } before do login_as :admin - project = create(:project) # Ensure that undiffable.md is in .gitattributes project.repository.copy_gitattributes(branch) @@ -31,6 +31,33 @@ feature 'Expand and collapse diffs', js: true, feature: true do define_method(file.split('.').first) { file_container(file) } end + it 'should show the diff content with a highlighted line when linking to line' do + expect(large_diff).not_to have_selector('.code') + expect(large_diff).to have_selector('.nothing-here-block') + + visit namespace_project_commit_path(project.namespace, project, project.commit(branch), anchor: "#{large_diff[:id]}_0_1") + execute_script('window.location.reload()') + + wait_for_ajax + + expect(large_diff).to have_selector('.code') + expect(large_diff).not_to have_selector('.nothing-here-block') + expect(large_diff).to have_selector('.hll') + end + + it 'should show the diff content when linking to file' do + expect(large_diff).not_to have_selector('.code') + expect(large_diff).to have_selector('.nothing-here-block') + + visit namespace_project_commit_path(project.namespace, project, project.commit(branch), anchor: large_diff[:id]) + execute_script('window.location.reload()') + + wait_for_ajax + + expect(large_diff).to have_selector('.code') + expect(large_diff).not_to have_selector('.nothing-here-block') + end + context 'visiting a commit with collapsed diffs' do it 'shows small diffs immediately' do expect(small_diff).to have_selector('.code') diff --git a/spec/features/merge_requests/diffs_spec.rb b/spec/features/merge_requests/diffs_spec.rb index c9a0059645d..4a6c76a5caf 100644 --- a/spec/features/merge_requests/diffs_spec.rb +++ b/spec/features/merge_requests/diffs_spec.rb @@ -22,4 +22,18 @@ feature 'Diffs URL', js: true, feature: true do expect(page).to have_css('.diffs.tab-pane.active') end end + + context 'when merge request has overflow' do + it 'displays warning' do + allow_any_instance_of(MergeRequestDiff).to receive(:overflow?).and_return(true) + allow(Commit).to receive(:max_diff_options).and_return(max_files: 20, max_lines: 20) + + visit diffs_namespace_project_merge_request_path(@project.namespace, @project, @merge_request) + + page.within('.alert') do + expect(page).to have_text("Too many changes to show. Plain diff Email patch To preserve + performance only 3 of 3+ files are displayed.") + end + end + end end diff --git a/spec/features/merge_requests/user_uses_slash_commands_spec.rb b/spec/features/merge_requests/user_uses_slash_commands_spec.rb index b1b3a47a1ce..b13674b4db9 100644 --- a/spec/features/merge_requests/user_uses_slash_commands_spec.rb +++ b/spec/features/merge_requests/user_uses_slash_commands_spec.rb @@ -68,6 +68,51 @@ feature 'Merge Requests > User uses slash commands', feature: true, js: true do end end + describe 'merging the MR from the note' do + context 'when the current user can merge the MR' do + it 'merges the MR' do + write_note("/merge") + + expect(page).to have_content 'Commands applied' + + expect(merge_request.reload).to be_merged + end + end + + context 'when the head diff changes in the meanwhile' do + before do + merge_request.source_branch = 'another_branch' + merge_request.save + end + + it 'does not merge the MR' do + write_note("/merge") + + expect(page).not_to have_content 'Your commands have been executed!' + + expect(merge_request.reload).not_to be_merged + end + end + + context 'when the current user cannot merge the MR' do + let(:guest) { create(:user) } + before do + project.team << [guest, :guest] + logout + login_with(guest) + visit namespace_project_merge_request_path(project.namespace, project, merge_request) + end + + it 'does not merge the MR' do + write_note("/merge") + + expect(page).not_to have_content 'Your commands have been executed!' + + expect(merge_request.reload).not_to be_merged + end + end + end + describe 'adding a due date from note' do it 'does not recognize the command nor create a note' do write_note('/due 2016-08-28') diff --git a/spec/features/projects/builds_spec.rb b/spec/features/projects/builds_spec.rb index 8c4d4320dc5..11d27feab0b 100644 --- a/spec/features/projects/builds_spec.rb +++ b/spec/features/projects/builds_spec.rb @@ -3,6 +3,7 @@ require 'tempfile' feature 'Builds', :feature do let(:user) { create(:user) } + let(:user_access_level) { :developer } let(:project) { create(:project) } let(:pipeline) { create(:ci_pipeline, project: project) } @@ -14,7 +15,7 @@ feature 'Builds', :feature do end before do - project.team << [user, :developer] + project.team << [user, user_access_level] login_as(user) end @@ -131,7 +132,9 @@ feature 'Builds', :feature do context 'Artifacts expire date' do before do - build.update_attributes(artifacts_file: artifacts_file, artifacts_expire_at: expire_at) + build.update_attributes(artifacts_file: artifacts_file, + artifacts_expire_at: expire_at) + visit namespace_project_build_path(project.namespace, project, build) end @@ -146,12 +149,23 @@ feature 'Builds', :feature do context 'when expire date is defined' do let(:expire_at) { Time.now + 7.days } - it 'keeps artifacts when Keep button is clicked' do - expect(page).to have_content 'The artifacts will be removed' - click_link 'Keep' + context 'when user has ability to update build' do + it 'keeps artifacts when keep button is clicked' do + expect(page).to have_content 'The artifacts will be removed' - expect(page).not_to have_link 'Keep' - expect(page).not_to have_content 'The artifacts will be removed' + click_link 'Keep' + + expect(page).to have_no_link 'Keep' + expect(page).to have_no_content 'The artifacts will be removed' + end + end + + context 'when user does not have ability to update build' do + let(:user_access_level) { :guest } + + it 'does not have keep button' do + expect(page).to have_no_link 'Keep' + end end end diff --git a/spec/features/projects/pipelines/pipelines_spec.rb b/spec/features/projects/pipelines/pipelines_spec.rb index 3ba996e2e10..ca18ac073d8 100644 --- a/spec/features/projects/pipelines/pipelines_spec.rb +++ b/spec/features/projects/pipelines/pipelines_spec.rb @@ -128,13 +128,13 @@ describe 'Pipelines', :feature, :js do it 'has link to the manual action' do find('.js-pipeline-dropdown-manual-actions').click - expect(page).to have_link('Manual build') + expect(page).to have_link('manual build') end context 'when manual action was played' do before do find('.js-pipeline-dropdown-manual-actions').click - click_link('Manual build') + click_link('manual build') end it 'enqueues manual action job' do diff --git a/spec/features/projects/project_settings_spec.rb b/spec/features/projects/project_settings_spec.rb index bf60cca4ea4..55d5d082c6e 100644 --- a/spec/features/projects/project_settings_spec.rb +++ b/spec/features/projects/project_settings_spec.rb @@ -21,6 +21,16 @@ describe 'Edit Project Settings', feature: true do expect(page).to have_content "Name can contain only letters, digits, emojis, '_', '.', dash, space. It must start with letter, digit, emoji or '_'." expect(page).to have_button 'Save changes' end + + scenario 'shows a successful notice when the project is updated' do + visit edit_namespace_project_path(project.namespace, project) + + fill_in 'project_name_edit', with: 'hello world' + + click_button 'Save changes' + + expect(page).to have_content "Project 'hello world' was successfully updated." + end end describe 'Rename repository' do diff --git a/spec/helpers/merge_requests_helper_spec.rb b/spec/helpers/merge_requests_helper_spec.rb index 903224589dd..1f221487393 100644 --- a/spec/helpers/merge_requests_helper_spec.rb +++ b/spec/helpers/merge_requests_helper_spec.rb @@ -62,4 +62,19 @@ describe MergeRequestsHelper do it { is_expected.to eq([source_title, target_title]) } end end + + describe 'mr_widget_refresh_url' do + let(:merge_request) { create(:merge_request, source_project: project) } + let(:project) { create(:project) } + + it 'returns correct url for MR' do + expected_url = "#{project.path_with_namespace}/merge_requests/#{merge_request.iid}/merge_widget_refresh" + + expect(mr_widget_refresh_url(merge_request)).to end_with(expected_url) + end + + it 'returns empty string for nil' do + expect(mr_widget_refresh_url(nil)).to end_with('') + end + end end diff --git a/spec/javascripts/gl_dropdown_spec.js.es6 b/spec/javascripts/gl_dropdown_spec.js.es6 index ce96571bd52..d11b1182d9a 100644 --- a/spec/javascripts/gl_dropdown_spec.js.es6 +++ b/spec/javascripts/gl_dropdown_spec.js.es6 @@ -50,6 +50,9 @@ selectable: true, filterable: isFilterable, data: hasRemote ? remoteMock.bind({}, this.projectsData) : this.projectsData, + search: { + fields: ['name'] + }, text: (project) => { (project.name_with_namespace || project.name); }, @@ -167,5 +170,21 @@ expect($(document.activeElement)).toEqual($(SEARCH_INPUT_SELECTOR)); }); }); + + + it('should still have input value on close and restore', () => { + let $searchInput = $(SEARCH_INPUT_SELECTOR); + initDropDown.call(this, false, true); + $searchInput + .trigger('focus') + .val('g') + .trigger('input'); + expect($searchInput.val()).toEqual('g'); + this.dropdownButtonElement.trigger('hidden.bs.dropdown'); + $searchInput + .trigger('blur') + .trigger('focus'); + expect($searchInput.val()).toEqual('g'); + }); }); })(); diff --git a/spec/migrations/remove_dot_git_from_usernames.rb b/spec/migrations/remove_dot_git_from_usernames.rb deleted file mode 100644 index 1b1d2adc463..00000000000 --- a/spec/migrations/remove_dot_git_from_usernames.rb +++ /dev/null @@ -1,29 +0,0 @@ -# encoding: utf-8 - -require 'spec_helper' -require Rails.root.join('db', 'migrate', '20161226122833_remove_dot_git_from_usernames.rb') - -describe RemoveDotGitFromUsernames do - let(:user) { create(:user) } - - describe '#up' do - let(:migration) { described_class.new } - - before do - namespace = user.namespace - namespace.path = 'test.git' - namespace.save!(validate: false) - - user.username = 'test.git' - user.save!(validate: false) - end - - it 'renames user with .git in username' do - migration.up - - expect(user.reload.username).to eq('test_git') - expect(user.namespace.reload.path).to eq('test_git') - expect(user.namespace.route.path).to eq('test_git') - end - end -end diff --git a/spec/migrations/remove_dot_git_from_usernames_spec.rb b/spec/migrations/remove_dot_git_from_usernames_spec.rb new file mode 100644 index 00000000000..8737e00eaeb --- /dev/null +++ b/spec/migrations/remove_dot_git_from_usernames_spec.rb @@ -0,0 +1,57 @@ +# encoding: utf-8 + +require 'spec_helper' +require Rails.root.join('db', 'migrate', '20161226122833_remove_dot_git_from_usernames.rb') + +describe RemoveDotGitFromUsernames do + let(:user) { create(:user) } + let(:migration) { described_class.new } + + describe '#up' do + before do + update_namespace(user, 'test.git') + end + + it 'renames user with .git in username' do + migration.up + + expect(user.reload.username).to eq('test_git') + expect(user.namespace.reload.path).to eq('test_git') + expect(user.namespace.route.path).to eq('test_git') + end + end + + context 'when new path exists already' do + describe '#up' do + let(:user2) { create(:user) } + + before do + update_namespace(user, 'test.git') + update_namespace(user2, 'test_git') + + storages = { 'default' => 'tmp/tests/custom_repositories' } + + allow(Gitlab.config.repositories).to receive(:storages).and_return(storages) + allow(migration).to receive(:route_exists?).with('test_git').and_return(true) + allow(migration).to receive(:route_exists?).with('test_git1').and_return(false) + end + + it 'renames user with .git in username' do + migration.up + + expect(user.reload.username).to eq('test_git1') + expect(user.namespace.reload.path).to eq('test_git1') + expect(user.namespace.route.path).to eq('test_git1') + end + end + end + + def update_namespace(user, path) + namespace = user.namespace + namespace.path = path + namespace.save!(validate: false) + + user.username = path + user.save!(validate: false) + end +end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index af0f6a31eda..3309a7fff9f 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -1013,6 +1013,24 @@ describe Ci::Build, :models do end end + describe '#has_expiring_artifacts?' do + context 'when artifacts have expiration date set' do + before { build.update(artifacts_expire_at: 1.day.from_now) } + + it 'has expiring artifacts' do + expect(build).to have_expiring_artifacts + end + end + + context 'when artifacts do not have expiration date set' do + before { build.update(artifacts_expire_at: nil) } + + it 'does not have expiring artifacts' do + expect(build).not_to have_expiring_artifacts + end + end + end + describe '#has_trace_file?' do context 'when there is no trace' do it { expect(build.has_trace_file?).to be_falsey } diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb index eb876d105da..6d599e148a2 100644 --- a/spec/models/merge_request_diff_spec.rb +++ b/spec/models/merge_request_diff_spec.rb @@ -76,6 +76,32 @@ describe MergeRequestDiff, models: true do end end + describe '#save_diffs' do + it 'saves collected state' do + mr_diff = create(:merge_request).merge_request_diff + + expect(mr_diff.collected?).to be_truthy + end + + it 'saves overflow state' do + allow(Commit).to receive(:max_diff_options) + .and_return(max_lines: 0, max_files: 0) + + mr_diff = create(:merge_request).merge_request_diff + + expect(mr_diff.overflow?).to be_truthy + end + + it 'saves empty state' do + allow_any_instance_of(MergeRequestDiff).to receive(:commits) + .and_return([]) + + mr_diff = create(:merge_request).merge_request_diff + + expect(mr_diff.empty?).to be_truthy + end + end + describe '#commits_sha' do it 'returns all commits SHA using serialized commits' do subject.st_commits = [ diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 8d1385016fd..861426acbc3 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -1514,6 +1514,108 @@ describe MergeRequest, models: true do end end + describe '#mergeable_with_slash_command?' do + def create_pipeline(status) + create(:ci_pipeline_with_one_job, + project: project, + ref: merge_request.source_branch, + sha: merge_request.diff_head_sha, + status: status) + end + + let(:project) { create(:project, :public, only_allow_merge_if_build_succeeds: true) } + let(:developer) { create(:user) } + let(:user) { create(:user) } + let(:merge_request) { create(:merge_request, source_project: project) } + let(:mr_sha) { merge_request.diff_head_sha } + + before do + project.team << [developer, :developer] + end + + context 'when autocomplete_precheck is set to true' do + it 'is mergeable by developer' do + expect(merge_request.mergeable_with_slash_command?(developer, autocomplete_precheck: true)).to be_truthy + end + + it 'is not mergeable by normal user' do + expect(merge_request.mergeable_with_slash_command?(user, autocomplete_precheck: true)).to be_falsey + end + end + + context 'when autocomplete_precheck is set to false' do + it 'is mergeable by developer' do + expect(merge_request.mergeable_with_slash_command?(developer, last_diff_sha: mr_sha)).to be_truthy + end + + it 'is not mergeable by normal user' do + expect(merge_request.mergeable_with_slash_command?(user, last_diff_sha: mr_sha)).to be_falsey + end + + context 'closed MR' do + before do + merge_request.update_attribute(:state, :closed) + end + + it 'is not mergeable' do + expect(merge_request.mergeable_with_slash_command?(developer, last_diff_sha: mr_sha)).to be_falsey + end + end + + context 'MR with WIP' do + before do + merge_request.update_attribute(:title, 'WIP: some MR') + end + + it 'is not mergeable' do + expect(merge_request.mergeable_with_slash_command?(developer, last_diff_sha: mr_sha)).to be_falsey + end + end + + context 'sha differs from the MR diff_head_sha' do + it 'is not mergeable' do + expect(merge_request.mergeable_with_slash_command?(developer, last_diff_sha: 'some other sha')).to be_falsey + end + end + + context 'sha is not provided' do + it 'is not mergeable' do + expect(merge_request.mergeable_with_slash_command?(developer)).to be_falsey + end + end + + context 'with pipeline ok' do + before do + create_pipeline(:success) + end + + it 'is mergeable' do + expect(merge_request.mergeable_with_slash_command?(developer, last_diff_sha: mr_sha)).to be_truthy + end + end + + context 'with failing pipeline' do + before do + create_pipeline(:failed) + end + + it 'is not mergeable' do + expect(merge_request.mergeable_with_slash_command?(developer, last_diff_sha: mr_sha)).to be_falsey + end + end + + context 'with running pipeline' do + before do + create_pipeline(:running) + end + + it 'is mergeable' do + expect(merge_request.mergeable_with_slash_command?(developer, last_diff_sha: mr_sha)).to be_truthy + end + end + end + end + describe '#has_commits?' do before do allow(subject.merge_request_diff).to receive(:commits_count). diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 32779eb92ef..e93a4e62244 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1545,11 +1545,13 @@ describe Project, models: true do end end - describe 'change_head' do + describe '#change_head' do let(:project) { create(:project) } - it 'calls the before_change_head method' do + it 'calls the before_change_head and after_change_head methods' do expect(project.repository).to receive(:before_change_head) + expect(project.repository).to receive(:after_change_head) + project.change_head(project.default_branch) end @@ -1565,11 +1567,6 @@ describe Project, models: true do project.change_head(project.default_branch) end - it 'expires the avatar cache' do - expect(project.repository).to receive(:expire_avatar_cache) - project.change_head(project.default_branch) - end - it 'reloads the default branch' do expect(project).to receive(:reload_default_branch) project.change_head(project.default_branch) diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index af7e89eae05..99ca53938c8 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -1150,6 +1150,24 @@ describe Repository, models: true do end end + describe '#after_change_head' do + it 'flushes the readme cache' do + expect(repository).to receive(:expire_method_caches).with([ + :readme, + :changelog, + :license, + :contributing, + :version, + :gitignore, + :koding, + :gitlab_ci, + :avatar + ]) + + repository.after_change_head + end + end + describe '#before_push_tag' do it 'flushes the cache' do expect(repository).to receive(:expire_statistics_caches) @@ -1513,14 +1531,6 @@ describe Repository, models: true do end end - describe '#expire_avatar_cache' do - it 'expires the cache' do - expect(repository).to receive(:expire_method_caches).with(%i(avatar)) - - repository.expire_avatar_cache - end - end - describe '#file_on_head' do context 'with a non-existing repository' do it 'returns nil' do diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index f5788d15f93..cdb16b4c46b 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -1085,7 +1085,7 @@ describe API::Projects, api: true do end describe 'GET /projects/search/:query' do - let!(:query) { 'query'} + let!(:query) { 'query'} let!(:search) { create(:empty_project, name: query, creator_id: user.id, namespace: user.namespace) } let!(:pre) { create(:empty_project, name: "pre_#{query}", creator_id: user.id, namespace: user.namespace) } let!(:post) { create(:empty_project, name: "#{query}_post", creator_id: user.id, namespace: user.namespace) } @@ -1095,32 +1095,37 @@ describe API::Projects, api: true do let!(:unfound_internal) { create(:empty_project, :internal, name: 'unfound internal') } let!(:public) { create(:empty_project, :public, name: "public #{query}") } let!(:unfound_public) { create(:empty_project, :public, name: 'unfound public') } + let!(:one_dot_two) { create(:empty_project, :public, name: "one.dot.two") } shared_examples_for 'project search response' do |args = {}| it 'returns project search responses' do - get api("/projects/search/#{query}", current_user) + get api("/projects/search/#{args[:query]}", current_user) expect(response).to have_http_status(200) expect(json_response).to be_an Array expect(json_response.size).to eq(args[:results]) - json_response.each { |project| expect(project['name']).to match(args[:match_regex] || /.*query.*/) } + json_response.each { |project| expect(project['name']).to match(args[:match_regex] || /.*#{args[:query]}.*/) } end end context 'when unauthenticated' do - it_behaves_like 'project search response', results: 1 do + it_behaves_like 'project search response', query: 'query', results: 1 do let(:current_user) { nil } end end context 'when authenticated' do - it_behaves_like 'project search response', results: 6 do + it_behaves_like 'project search response', query: 'query', results: 6 do let(:current_user) { user } end + it_behaves_like 'project search response', query: 'one.dot.two', results: 1 do + let(:current_user) { user } + end + end context 'when authenticated as a different user' do - it_behaves_like 'project search response', results: 2, match_regex: /(internal|public) query/ do + it_behaves_like 'project search response', query: 'query', results: 2, match_regex: /(internal|public) query/ do let(:current_user) { user2 } end end diff --git a/spec/serializers/build_action_entity_spec.rb b/spec/serializers/build_action_entity_spec.rb index 383704572b1..0f7be8b2c39 100644 --- a/spec/serializers/build_action_entity_spec.rb +++ b/spec/serializers/build_action_entity_spec.rb @@ -10,8 +10,8 @@ describe BuildActionEntity do describe '#as_json' do subject { entity.as_json } - it 'contains humanized build name' do - expect(subject[:name]).to eq 'Test build' + it 'contains original build name' do + expect(subject[:name]).to eq 'test_build' end it 'contains path to the action play' do diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index 88c786947d3..7d73c0ea5d0 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -121,6 +121,99 @@ describe MergeRequests::UpdateService, services: true do end end + context 'merge' do + let(:opts) do + { + merge: merge_request.diff_head_sha + } + end + + let(:service) { MergeRequests::UpdateService.new(project, user, opts) } + + context 'without pipeline' do + before do + merge_request.merge_error = 'Error' + + perform_enqueued_jobs do + service.execute(merge_request) + @merge_request = MergeRequest.find(merge_request.id) + end + end + + it { expect(@merge_request).to be_valid } + it { expect(@merge_request.state).to eq('merged') } + it { expect(@merge_request.merge_error).to be_nil } + end + + context 'with finished pipeline' do + before do + create(:ci_pipeline_with_one_job, + project: project, + ref: merge_request.source_branch, + sha: merge_request.diff_head_sha, + status: :success) + + perform_enqueued_jobs do + @merge_request = service.execute(merge_request) + @merge_request = MergeRequest.find(merge_request.id) + end + end + + it { expect(@merge_request).to be_valid } + it { expect(@merge_request.state).to eq('merged') } + end + + context 'with active pipeline' do + before do + service_mock = double + create(:ci_pipeline_with_one_job, + project: project, + ref: merge_request.source_branch, + sha: merge_request.diff_head_sha) + + expect(MergeRequests::MergeWhenPipelineSucceedsService).to receive(:new).with(project, user). + and_return(service_mock) + expect(service_mock).to receive(:execute).with(merge_request) + end + + it { service.execute(merge_request) } + end + + context 'with a non-authorised user' do + let(:visitor) { create(:user) } + let(:service) { MergeRequests::UpdateService.new(project, visitor, opts) } + + before do + merge_request.update_attribute(:merge_error, 'Error') + + perform_enqueued_jobs do + @merge_request = service.execute(merge_request) + @merge_request = MergeRequest.find(merge_request.id) + end + end + + it { expect(@merge_request.state).to eq('opened') } + it { expect(@merge_request.merge_error).not_to be_nil } + end + + context 'MR can not be merged when note sha != MR sha' do + let(:opts) do + { + merge: 'other_commit' + } + end + + before do + perform_enqueued_jobs do + @merge_request = service.execute(merge_request) + @merge_request = MergeRequest.find(merge_request.id) + end + end + + it { expect(@merge_request.state).to eq('opened') } + end + end + context 'todos' do let!(:pending_todo) { create(:todo, :assigned, user: user, project: project, target: merge_request, author: user2) } diff --git a/spec/services/notes/create_service_spec.rb b/spec/services/notes/create_service_spec.rb index 25804696d2e..b0cc3ce5f5a 100644 --- a/spec/services/notes/create_service_spec.rb +++ b/spec/services/notes/create_service_spec.rb @@ -63,6 +63,17 @@ describe Notes::CreateService, services: true do expect(note.note).to eq "HELLO\nWORLD" end end + + describe '/merge with sha option' do + let(:note_text) { %(HELLO\n/merge\nWORLD) } + let(:params) { opts.merge(note: note_text, merge_request_diff_head_sha: 'sha') } + + it 'saves the note and exectues merge command' do + note = described_class.new(project, user, params).execute + + expect(note.note).to eq "HELLO\nWORLD" + end + end end end diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index e139be19140..caa23962519 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/spec/services/projects/update_service_spec.rb @@ -1,145 +1,101 @@ require 'spec_helper' describe Projects::UpdateService, services: true do - describe :update_by_user do - before do - @user = create :user - @admin = create :user, admin: true - @project = create :project, creator_id: @user.id, namespace: @user.namespace - @opts = {} - end + let(:user) { create(:user) } + let(:admin) { create(:admin) } + let(:project) { create(:project, creator_id: user.id, namespace: user.namespace) } - context 'is private when updated to private' do - before do - @created_private = @project.private? + describe 'update_by_user' do + context 'when visibility_level is INTERNAL' do + it 'updates the project to internal' do + result = update_project(project, user, visibility_level: Gitlab::VisibilityLevel::INTERNAL) - @opts.merge!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) - update_project(@project, @user, @opts) + expect(result).to eq({ status: :success }) + expect(project).to be_internal end - - it { expect(@created_private).to be_truthy } - it { expect(@project.private?).to be_truthy } end - context 'is internal when updated to internal' do - before do - @created_private = @project.private? - - @opts.merge!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) - update_project(@project, @user, @opts) + context 'when visibility_level is PUBLIC' do + it 'updates the project to public' do + result = update_project(project, user, visibility_level: Gitlab::VisibilityLevel::PUBLIC) + expect(result).to eq({ status: :success }) + expect(project).to be_public end - - it { expect(@created_private).to be_truthy } - it { expect(@project.internal?).to be_truthy } end - context 'is public when updated to public' do + context 'when visibility levels are restricted to PUBLIC only' do before do - @created_private = @project.private? - - @opts.merge!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) - update_project(@project, @user, @opts) - end - - it { expect(@created_private).to be_truthy } - it { expect(@project.public?).to be_truthy } - end - - context 'respect configured visibility restrictions setting' do - before(:each) do stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC]) end - context 'is private when updated to private' do - before do - @created_private = @project.private? - - @opts.merge!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) - update_project(@project, @user, @opts) + context 'when visibility_level is INTERNAL' do + it 'updates the project to internal' do + result = update_project(project, user, visibility_level: Gitlab::VisibilityLevel::INTERNAL) + expect(result).to eq({ status: :success }) + expect(project).to be_internal end - - it { expect(@created_private).to be_truthy } - it { expect(@project.private?).to be_truthy } end - context 'is internal when updated to internal' do - before do - @created_private = @project.private? + context 'when visibility_level is PUBLIC' do + it 'does not update the project to public' do + result = update_project(project, user, visibility_level: Gitlab::VisibilityLevel::PUBLIC) - @opts.merge!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) - update_project(@project, @user, @opts) + expect(result).to eq({ status: :error, message: 'Visibility level unallowed' }) + expect(project).to be_private end - it { expect(@created_private).to be_truthy } - it { expect(@project.internal?).to be_truthy } - end - - context 'is private when updated to public' do - before do - @created_private = @project.private? - - @opts.merge!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) - update_project(@project, @user, @opts) + context 'when updated by an admin' do + it 'updates the project to public' do + result = update_project(project, admin, visibility_level: Gitlab::VisibilityLevel::PUBLIC) + expect(result).to eq({ status: :success }) + expect(project).to be_public + end end - - it { expect(@created_private).to be_truthy } - it { expect(@project.private?).to be_truthy } - end - - context 'is public when updated to public by admin' do - before do - @created_private = @project.private? - - @opts.merge!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) - update_project(@project, @admin, @opts) - end - - it { expect(@created_private).to be_truthy } - it { expect(@project.public?).to be_truthy } end end end - describe :visibility_level do - let(:user) { create :user, admin: true } + describe 'visibility_level' do let(:project) { create(:project, :internal) } let(:forked_project) { create(:forked_project_with_submodules, :internal) } - let(:opts) { {} } before do forked_project.build_forked_project_link(forked_to_project_id: forked_project.id, forked_from_project_id: project.id) forked_project.save - - @created_internal = project.internal? - @fork_created_internal = forked_project.internal? end - context 'updates forks visibility level when parent set to more restrictive' do - before do - opts.merge!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) - update_project(project, user, opts).inspect - end + it 'updates forks visibility level when parent set to more restrictive' do + opts = { visibility_level: Gitlab::VisibilityLevel::PRIVATE } + + expect(project).to be_internal + expect(forked_project).to be_internal - it { expect(@created_internal).to be_truthy } - it { expect(@fork_created_internal).to be_truthy } - it { expect(project.private?).to be_truthy } - it { expect(project.forks.first.private?).to be_truthy } + expect(update_project(project, admin, opts)).to eq({ status: :success }) + + expect(project).to be_private + expect(forked_project.reload).to be_private end - context 'does not update forks visibility level when parent set to less restrictive' do - before do - opts.merge!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) - update_project(project, user, opts).inspect - end + it 'does not update forks visibility level when parent set to less restrictive' do + opts = { visibility_level: Gitlab::VisibilityLevel::PUBLIC } + + expect(project).to be_internal + expect(forked_project).to be_internal - it { expect(@created_internal).to be_truthy } - it { expect(@fork_created_internal).to be_truthy } - it { expect(project.public?).to be_truthy } - it { expect(project.forks.first.internal?).to be_truthy } + expect(update_project(project, admin, opts)).to eq({ status: :success }) + + expect(project).to be_public + expect(forked_project.reload).to be_internal end end + it 'returns an error result when record cannot be updated' do + result = update_project(project, admin, { name: 'foo&bar' }) + + expect(result).to eq({ status: :error, message: 'Project could not be updated' }) + end + def update_project(project, user, opts) - Projects::UpdateService.new(project, user, opts).execute + described_class.new(project, user, opts).execute end end diff --git a/spec/services/slash_commands/interpret_service_spec.rb b/spec/services/slash_commands/interpret_service_spec.rb index e1358acd7c1..99dd9be3218 100644 --- a/spec/services/slash_commands/interpret_service_spec.rb +++ b/spec/services/slash_commands/interpret_service_spec.rb @@ -1,12 +1,13 @@ require 'spec_helper' describe SlashCommands::InterpretService, services: true do - let(:project) { create(:empty_project, :public) } + let(:project) { create(:project, :public) } let(:developer) { create(:user) } let(:issue) { create(:issue, project: project) } let(:milestone) { create(:milestone, project: project, title: '9.10') } let(:inprogress) { create(:label, project: project, title: 'In Progress') } let(:bug) { create(:label, project: project, title: 'Bug') } + let(:note) { build(:note, commit_id: merge_request.diff_head_sha) } before do project.team << [developer, :developer] @@ -258,6 +259,14 @@ describe SlashCommands::InterpretService, services: true do end end + shared_examples 'merge command' do + it 'runs merge command if content contains /merge' do + _, updates = service.execute(content, issuable) + + expect(updates).to eq(merge: merge_request.diff_head_sha) + end + end + it_behaves_like 'reopen command' do let(:content) { '/reopen' } let(:issuable) { issue } @@ -278,6 +287,64 @@ describe SlashCommands::InterpretService, services: true do let(:issuable) { merge_request } end + context 'merge command' do + let(:service) { described_class.new(project, developer, { merge_request_diff_head_sha: merge_request.diff_head_sha }) } + + it_behaves_like 'merge command' do + let(:content) { '/merge' } + let(:issuable) { merge_request } + end + + context 'can not be merged when logged user does not have permissions' do + let(:service) { described_class.new(project, create(:user)) } + + it_behaves_like 'empty command' do + let(:content) { "/merge" } + let(:issuable) { merge_request } + end + end + + context 'can not be merged when sha does not match' do + let(:service) { described_class.new(project, developer, { merge_request_diff_head_sha: 'othersha' }) } + + it_behaves_like 'empty command' do + let(:content) { "/merge" } + let(:issuable) { merge_request } + end + end + + context 'when sha is missing' do + let(:service) { described_class.new(project, developer, {}) } + + it 'precheck passes and returns merge command' do + _, updates = service.execute('/merge', merge_request) + + expect(updates).to eq(merge: nil) + end + end + + context 'issue can not be merged' do + it_behaves_like 'empty command' do + let(:content) { "/merge" } + let(:issuable) { issue } + end + end + + context 'non persisted merge request cant be merged' do + it_behaves_like 'empty command' do + let(:content) { "/merge" } + let(:issuable) { build(:merge_request) } + end + end + + context 'not persisted merge request can not be merged' do + it_behaves_like 'empty command' do + let(:content) { "/merge" } + let(:issuable) { build(:merge_request, source_project: project) } + end + end + end + it_behaves_like 'title command' do let(:content) { '/title A brand new title' } let(:issuable) { issue } diff --git a/spec/services/users/refresh_authorized_projects_service_spec.rb b/spec/services/users/refresh_authorized_projects_service_spec.rb index 9fbb61565e3..690fe979492 100644 --- a/spec/services/users/refresh_authorized_projects_service_spec.rb +++ b/spec/services/users/refresh_authorized_projects_service_spec.rb @@ -10,7 +10,21 @@ describe Users::RefreshAuthorizedProjectsService do create!(project: project, user: user, access_level: access_level) end - describe '#execute' do + describe '#execute', :redis do + it 'refreshes the authorizations using a lease' do + expect_any_instance_of(Gitlab::ExclusiveLease).to receive(:try_obtain). + and_return('foo') + + expect(Gitlab::ExclusiveLease).to receive(:cancel). + with(an_instance_of(String), 'foo') + + expect(service).to receive(:execute_without_lease) + + service.execute + end + end + + describe '#execute_without_lease' do before do user.project_authorizations.delete_all end @@ -19,37 +33,23 @@ describe Users::RefreshAuthorizedProjectsService do project2 = create(:empty_project) to_remove = create_authorization(project2, user) - expect(service).to receive(:update_with_lease). + expect(service).to receive(:update_authorizations). with([to_remove.project_id], [[user.id, project.id, Gitlab::Access::MASTER]]) - service.execute + service.execute_without_lease end it 'sets the access level of a project to the highest available level' do to_remove = create_authorization(project, user, Gitlab::Access::DEVELOPER) - expect(service).to receive(:update_with_lease). + expect(service).to receive(:update_authorizations). with([to_remove.project_id], [[user.id, project.id, Gitlab::Access::MASTER]]) - service.execute + service.execute_without_lease end it 'returns a User' do - expect(service.execute).to be_an_instance_of(User) - end - end - - describe '#update_with_lease', :redis do - it 'refreshes the authorizations using a lease' do - expect_any_instance_of(Gitlab::ExclusiveLease).to receive(:try_obtain). - and_return('foo') - - expect(Gitlab::ExclusiveLease).to receive(:cancel). - with(an_instance_of(String), 'foo') - - expect(service).to receive(:update_authorizations).with([1], []) - - service.update_with_lease([1]) + expect(service.execute_without_lease).to be_an_instance_of(User) end end |