Merge branch 'master' into go-go-gadget-webpack

15 files changed, 343 insertions, 180 deletions

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/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/javascripts/gl_dropdown_spec.js.es6 b/spec/javascripts/gl_dropdown_spec.js.es6
index c60fbd73c63..83e1638cdc6 100644
--- a/spec/javascripts/gl_dropdown_spec.js.es6
+++ b/spec/javascripts/gl_dropdown_spec.js.es6
@@ -49,6 +49,9 @@ require('~/lib/utils/type_utility');
         selectable: true,
         filterable: isFilterable,
         data: hasRemote ? remoteMock.bind({}, this.projectsData) : this.projectsData,
+        search: {
+          fields: ['name']
+        },
         text: (project) => {
           (project.name_with_namespace || project.name);
         },
@@ -166,5 +169,21 @@ require('~/lib/utils/type_utility');
         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/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/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/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