Merge branch 'master' into i-#25814-500-error

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();
       }
     }

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');
         },
       });
     }

app/assets/javascripts/single_file_diff.js

-/* 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));
       }
     });
   };

app/assets/javascripts/vue_pipelines_index/index.js.es6

 /* global Vue, VueResource, gl */
 /*= require vue_common_component/commit */
+/*= require vue_pagination/index */
 /*= require vue-resource
 /*= require boards/vue_resource_interceptor */
 /*= require ./status.js.es6 */

app/assets/javascripts/vue_pipelines_index/stage.js.es6

 /* 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"

app/assets/javascripts/vue_pipelines_index/store.js.es6

@@ -3,14 +3,24 @@
 /*= require vue_realtime_listener/index.js */
 
 ((gl) => {
-  const pageValues = headers => ({
-    perPage: +headers['X-Per-Page'],
-    page: +headers['X-Page'],
-    total: +headers['X-Total'],
-    totalPages: +headers['X-Total-Pages'],
-    nextPage: +headers['X-Next-Page'],
-    previousPage: +headers['X-Prev-Page'],
-  });
+  const pageValues = (headers) => {
+    const normalizedHeaders = {};
+
+    Object.keys(headers).forEach((e) => {
+      normalizedHeaders[e.toUpperCase()] = headers[e];
+    });
+
+    const paginationInfo = {
+      perPage: +normalizedHeaders['X-PER-PAGE'],
+      page: +normalizedHeaders['X-PAGE'],
+      total: +normalizedHeaders['X-TOTAL'],
+      totalPages: +normalizedHeaders['X-TOTAL-PAGES'],
+      nextPage: +normalizedHeaders['X-NEXT-PAGE'],
+      previousPage: +normalizedHeaders['X-PREV-PAGE'],
+    };
+
+    return paginationInfo;
+  };
 
   gl.PipelineStore = class {
     fetchDataLoop(Vue, pageNum, url, apiScope) {

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);
+}

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 {

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;
 }

app/assets/stylesheets/framework/nav.scss

@@ -101,7 +101,7 @@
         &:hover,
         &:active,
         &:focus {
-          border-bottom: none;
+          border-color: transparent;
         }
       }
     }

app/assets/stylesheets/framework/variables.scss

@@ -102,6 +102,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
@@ -172,6 +176,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
@@ -530,4 +537,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

app/assets/stylesheets/pages/cycle_analytics.scss

@@ -20,6 +20,10 @@
 
     .fa {
       color: $cycle-analytics-light-gray;
+
+      &:hover {
+        color: $gl-text-color;
+      }
     }
 
     .stage-header {

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);
+      }
     }
   }
 

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 {

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
+}

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;

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' }

app/models/forked_project_link.rb

 class ForkedProjectLink < ActiveRecord::Base
-  belongs_to :forked_to_project, class_name: Project
-  belongs_to :forked_from_project, class_name: Project
+  belongs_to :forked_to_project, class_name: 'Project'
+  belongs_to :forked_from_project, class_name: 'Project'
 end

app/models/project.rb

@@ -122,7 +122,7 @@ class Project < ActiveRecord::Base
   # Merge Requests for target project should be removed with it
   has_many :merge_requests,     dependent: :destroy, foreign_key: 'target_project_id'
   # Merge requests from source project should be kept when source project was removed
-  has_many :fork_merge_requests, foreign_key: 'source_project_id', class_name: MergeRequest
+  has_many :fork_merge_requests, foreign_key: 'source_project_id', class_name: 'MergeRequest'
   has_many :issues,             dependent: :destroy
   has_many :labels,             dependent: :destroy, class_name: 'ProjectLabel'
   has_many :services,           dependent: :destroy
@@ -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
 

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

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|

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

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

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)

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' }

app/views/projects/diffs/_file.html.haml

 .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?

app/views/projects/empty.html.haml

@@ -52,7 +52,7 @@
               git push -u origin master
 
         %fieldset
-          %h5 Existing folder or Git repository
+          %h5 Existing folder
           %pre.light-well
             :preserve
               cd existing_folder
@@ -62,6 +62,15 @@
               git commit
               git push -u origin master
 
+        %fieldset
+          %h5 Existing Git repository
+          %pre.light-well
+            :preserve
+              cd existing_repo
+              git remote add origin #{ content_tag(:span, default_url_to_repo, class: 'clone')}
+              git push -u origin --all
+              git push -u origin --tags
+
           - if can? current_user, :remove_project, @project
             .prepend-top-20
               = link_to 'Remove project', [@project.namespace.becomes(Namespace), @project], data: { confirm: remove_project_message(@project)}, method: :delete, class: "btn btn-remove pull-right"

app/views/projects/pipelines/index.html.haml

@@ -64,5 +64,4 @@
 
       .vue-pipelines-index
 
-= page_specific_javascript_tag('vue_pagination/index.js')
 = page_specific_javascript_tag('vue_pipelines_index/index.js')

changelogs/unreleased/22111-remove-lock-icon-on-protected-tag.yml

+---
+title: Remove Lock Icon on Protected Tag
+merge_request: 8513
+author: Sergey Nikitin

changelogs/unreleased/25946-manual-pipeline-dropdown-casing.yml

+---
+title: Use original casing for build action text
+merge_request: 8387
+author:

changelogs/unreleased/26207-add-hover-animations.yml

+---
+title: Add various hover animations throughout the application
+merge_request:
+author:

changelogs/unreleased/allow_plus_sign_for_snippets.yml

+---
+title: Allow to use + symbol in filenames
+merge_request: 6644
+author: blackst0ne

changelogs/unreleased/dot-in-project-queries.yml

+---
+title: Allow API query to find projects with dots in their name
+merge_request:
+author: Bruno Melli

changelogs/unreleased/pmq20-gitlab-ce-psvr-head-cache.yml

+---
+title: Expire related caches after changing HEAD
+merge_request:
+author: Minqi Pan

changelogs/unreleased/sandish-gitlab-ce-update_ret_val.yml

+---
+title: Ensure updating project settings shows a flash message on success
+merge_request: 8579
+author: Sandish Chen

config/application.rb

@@ -111,7 +111,6 @@ module Gitlab
     config.assets.precompile << "lib/*.js"
     config.assets.precompile << "u2f.js"
     config.assets.precompile << "vue_pipelines_index/index.js"
-    config.assets.precompile << "vue_pagination/index.js"
     config.assets.precompile << "vendor/assets/fonts/*"
 
     # Version of your assets, change this if you want to expire all your assets

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

features/admin/groups.feature

-@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"

features/steps/admin/groups.rb

-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

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

lib/api/helpers.rb

@@ -294,7 +294,7 @@ module API
         header['X-Sendfile'] = path
         body
       else
-        file FileStreamer.new(path)
+        path
       end
     end
 

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
 

lib/gitlab/regex.rb

@@ -61,11 +61,11 @@ module Gitlab
     end
 
     def file_name_regex
-      @file_name_regex ||= /\A[[[:alnum:]]_\-\.\@]*\z/.freeze
+      @file_name_regex ||= /\A[[[:alnum:]]_\-\.\@\+]*\z/.freeze
     end
 
     def file_name_regex_message
-      "can contain only letters, digits, '_', '-', '@' and '.'."
+      "can contain only letters, digits, '_', '-', '@', '+' and '.'."
     end
 
     def file_path_regex

scripts/notify_slack.sh

-#!/bin/bash
+#!/bin/sh
 # Sends Slack notification ERROR_MSG to CHANNEL
 # An env. variable CI_SLACK_WEBHOOK_URL needs to be set.
 

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
 

spec/features/admin/admin_groups_spec.rb

 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]"
 

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')

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

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

spec/features/snippets/create_snippet_spec.rb

@@ -17,4 +17,18 @@ feature 'Create Snippet', feature: true do
     expect(page).to have_content('My Snippet Title')
     expect(page).to have_content('Hello World!')
   end
+
+  scenario 'Authenticated user creates a snippet with + in filename' do
+    fill_in 'personal_snippet_title', with: 'My Snippet Title'
+    page.within('.file-editor') do
+      find(:xpath, "//input[@id='personal_snippet_file_name']").set 'snippet+file+name'
+      find(:xpath, "//input[@id='personal_snippet_content']").set 'Hello World!'
+    end
+
+    click_button 'Create snippet'
+
+    expect(page).to have_content('My Snippet Title')
+    expect(page).to have_content('snippet+file+name')
+    expect(page).to have_content('Hello World!')
+  end
 end

spec/migrations/remove_dot_git_from_usernames.rb → spec/migrations/remove_dot_git_from_usernames_spec.rb

@@ -5,17 +5,11 @@ require Rails.root.join('db', 'migrate', '20161226122833_remove_dot_git_from_use
 
 describe RemoveDotGitFromUsernames do
   let(:user) { create(:user) }
+  let(:migration) { described_class.new }
 
   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)
+      update_namespace(user, 'test.git')
     end
 
     it 'renames user with .git in username' do
@@ -26,4 +20,38 @@ describe RemoveDotGitFromUsernames do
       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

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)

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

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

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

spec/services/projects/update_service_spec.rb

 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