Merge branch 'master' into layout-nav-es-modulelayout-nav-es-module

* master: (21 commits)
  Prevent some specs from mangling the gitlab-shell checkout
  Line up search dropdown with other nav dropdowns
  Fix onion-skin re-entering state
  Remove related links in MR widget when empty state
  Show inline edit button for issues
  Fix tags in the Activity tab not being clickable
  Fix shortcut links on help page
  Don't link LFS-objects multiple times.
  [CE->EE] Fix spec/lib/gitlab/git/gitlab_projects_spec.rb
  Tidy up the documentation of Gitlab HA/Gitlab Application
  Make sure two except won't overwrite each other
  Update axios.md
  Remove transitionend event from GL dropdown
  Preserve gem path so that we use the same gems
  Load commit in batches for pipelines#index
  BlobViewer::PackageJson - if private link to homepage
  Do not generate links for private NPM modules in blob view
  Remove block styling from search dropdown
  Fix sidebar height when performance bar enabled
  Remove all dropdown animations and set display: none if they're not open
  ...

16 files changed, 317 insertions, 41 deletions

diff --git a/spec/controllers/projects/pipelines_controller_spec.rb b/spec/controllers/projects/pipelines_controller_spec.rb
index 1604a2da485..35ac999cc65 100644
--- a/spec/controllers/projects/pipelines_controller_spec.rb
+++ b/spec/controllers/projects/pipelines_controller_spec.rb
@@ -17,13 +17,10 @@ describe Projects::PipelinesController do
 
   describe 'GET index.json' do
     before do
-      branch_head = project.commit
-      parent = branch_head.parent
-
-      create(:ci_empty_pipeline, status: 'pending', project: project, sha: branch_head.id)
-      create(:ci_empty_pipeline, status: 'running', project: project, sha: branch_head.id)
-      create(:ci_empty_pipeline, status: 'created', project: project, sha: parent.id)
-      create(:ci_empty_pipeline, status: 'success', project: project, sha: parent.id)
+      %w(pending running created success).each_with_index do |status, index|
+        sha = project.commit("HEAD~#{index}")
+        create(:ci_empty_pipeline, status: status, project: project, sha: sha)
+      end
     end
 
     subject do
@@ -46,7 +43,7 @@ describe Projects::PipelinesController do
 
     context 'when performing gitaly calls', :request_store do
       it 'limits the Gitaly requests' do
-        expect { subject }.to change { Gitlab::GitalyClient.get_request_count }.by(8)
+        expect { subject }.to change { Gitlab::GitalyClient.get_request_count }.by(3)
       end
     end
   end
diff --git a/spec/features/help_pages_spec.rb b/spec/features/help_pages_spec.rb
index ab896a310be..0d04ed612c2 100644
--- a/spec/features/help_pages_spec.rb
+++ b/spec/features/help_pages_spec.rb
@@ -32,6 +32,24 @@ describe 'Help Pages' do
 
       it_behaves_like 'help page', prefix: '/gitlab'
     end
+
+    context 'quick link shortcuts', :js do
+      before do
+        visit help_path
+      end
+
+      it 'focuses search bar' do
+        find('.js-trigger-search-bar').click
+
+        expect(page).to have_selector('#search:focus')
+      end
+
+      it 'opens shortcuts help dialog' do
+        find('.js-trigger-shortcut').click
+
+        expect(page).to have_selector('#modal-shortcuts')
+      end
+    end
   end
 
   context 'in a production environment with version check enabled', :js do
diff --git a/spec/features/issues/issue_detail_spec.rb b/spec/features/issues/issue_detail_spec.rb
index 4224a8fe5d4..babb0285590 100644
--- a/spec/features/issues/issue_detail_spec.rb
+++ b/spec/features/issues/issue_detail_spec.rb
@@ -24,7 +24,7 @@ feature 'Issue Detail', :js do
       visit project_issue_path(project, issue)
       wait_for_requests
 
-      click_link 'Edit'
+      page.find('.js-issuable-edit').click
       fill_in 'issuable-title', with: 'issue title'
       click_button 'Save'
       wait_for_requests
diff --git a/spec/features/merge_requests/image_diff_notes.rb b/spec/features/merge_requests/image_diff_notes_spec.rb
index 021c4e03428..ddc73437917 100644
--- a/spec/features/merge_requests/image_diff_notes.rb
+++ b/spec/features/merge_requests/image_diff_notes_spec.rb
@@ -10,8 +10,6 @@ feature 'image diff notes', :js do
     project.team << [user, :master]
     sign_in user
 
-    page.driver.set_cookie('sidebar_collapsed', 'true')
-
     # Stub helper to return any blob file as image from public app folder.
     # This is necessary to run this specs since we don't display repo images in capybara.
     allow_any_instance_of(DiffHelper).to receive(:diff_file_blob_raw_path).and_return('/apple-touch-icon.png')
@@ -141,13 +139,13 @@ feature 'image diff notes', :js do
         end
 
         it 'allows expanding/collapsing the discussion notes' do
-          page.all('.js-diff-notes-toggle')[0].trigger('click')
-          page.all('.js-diff-notes-toggle')[1].trigger('click')
+          page.all('.js-diff-notes-toggle')[0].click
+          page.all('.js-diff-notes-toggle')[1].click
 
           expect(page).not_to have_content('image diff test comment')
 
-          page.all('.js-diff-notes-toggle')[0].trigger('click')
-          page.all('.js-diff-notes-toggle')[1].trigger('click')
+          page.all('.js-diff-notes-toggle')[0].click
+          page.all('.js-diff-notes-toggle')[1].click
 
           expect(page).to have_content('image diff test comment')
         end
@@ -196,13 +194,31 @@ feature 'image diff notes', :js do
 
       expect(find('.onion-skin-frame')['style']).to match('width: 228px; height: 240px;')
     end
+
+    it 'resets onion skin view mode opacity when toggling between view modes' do
+      find('.view-modes-menu .onion-skin').click
+
+      # Simulate dragging onion-skin slider
+      drag_and_drop_by(find('.dragger'), -30, 0)
+
+      expect(find('.onion-skin-frame .frame.added', visible: false)['style']).not_to match('opacity: 1;')
+
+      find('.view-modes-menu .swipe').click
+      find('.view-modes-menu .onion-skin').click
+
+      expect(find('.onion-skin-frame .frame.added', visible: false)['style']).to match('opacity: 1;')
+    end
   end
-end
 
-def create_image_diff_note
-  find('.js-add-image-diff-note-button', match: :first).click
-  page.all('.js-add-image-diff-note-button')[0].trigger('click')
-  find('.diff-content .note-textarea').native.send_keys('image diff test comment')
-  click_button 'Comment'
-  wait_for_requests
+  def drag_and_drop_by(element, right_by, down_by)
+    page.driver.browser.action.drag_and_drop_by(element.native, right_by, down_by).perform
+  end
+
+  def create_image_diff_note
+    find('.js-add-image-diff-note-button', match: :first).click
+    page.all('.js-add-image-diff-note-button')[0].click
+    find('.diff-content .note-textarea').native.send_keys('image diff test comment')
+    click_button 'Comment'
+    wait_for_requests
+  end
 end
diff --git a/spec/features/projects/files/project_owner_sees_link_to_create_license_file_in_empty_project_spec.rb b/spec/features/projects/files/project_owner_sees_link_to_create_license_file_in_empty_project_spec.rb
index 6c616bf0456..8ac9821b879 100644
--- a/spec/features/projects/files/project_owner_sees_link_to_create_license_file_in_empty_project_spec.rb
+++ b/spec/features/projects/files/project_owner_sees_link_to_create_license_file_in_empty_project_spec.rb
@@ -2,15 +2,15 @@ require 'spec_helper'
 
 feature 'project owner sees a link to create a license file in empty project', :js do
   let(:project_master) { create(:user) }
-  let(:project) { create(:project) }
+  let(:project) { create(:project_empty_repo) }
+
   background do
-    project.team << [project_master, :master]
+    project.add_master(project_master)
     sign_in(project_master)
   end
 
   scenario 'project master creates a license file from a template' do
     visit project_path(project)
-    click_link 'Create empty bare repository'
     click_on 'LICENSE'
     expect(page).to have_content('New file')
 
@@ -26,8 +26,6 @@ feature 'project owner sees a link to create a license file in empty project', :
     expect(file_content).to have_content("Copyright (c) #{Time.now.year} #{project.namespace.human_name}")
 
     fill_in :commit_message, with: 'Add a LICENSE file', visible: true
-    # Remove pre-receive hook so we can push without auth
-    FileUtils.rm_f(File.join(project.repository.path, 'hooks', 'pre-receive'))
     click_button 'Commit changes'
 
     expect(current_path).to eq(
diff --git a/spec/features/projects/issuable_templates_spec.rb b/spec/features/projects/issuable_templates_spec.rb
index 0257cd157c9..4319fc2746c 100644
--- a/spec/features/projects/issuable_templates_spec.rb
+++ b/spec/features/projects/issuable_templates_spec.rb
@@ -32,9 +32,7 @@ feature 'issuable templates', :js do
         message: 'added issue template',
         branch_name: 'master')
       visit project_issue_path project, issue
-      page.within('.js-issuable-actions') do
-        click_on 'Edit'
-      end
+      page.find('.js-issuable-edit').click
       fill_in :'issuable-title', with: 'test issue title'
     end
 
@@ -77,9 +75,7 @@ feature 'issuable templates', :js do
         message: 'added issue template',
         branch_name: 'master')
       visit project_issue_path project, issue
-      page.within('.js-issuable-actions') do
-        click_on 'Edit'
-      end
+      page.find('.js-issuable-edit').click
       fill_in :'issuable-title', with: 'test issue title'
       fill_in :'issue-description', with: prior_description
     end
diff --git a/spec/features/tags/master_views_tags_spec.rb b/spec/features/tags/master_views_tags_spec.rb
index 9edc7ced163..4662367d843 100644
--- a/spec/features/tags/master_views_tags_spec.rb
+++ b/spec/features/tags/master_views_tags_spec.rb
@@ -4,18 +4,17 @@ feature 'Master views tags' do
   let(:user) { create(:user) }
 
   before do
-    project.team << [user, :master]
+    project.add_master(user)
     sign_in(user)
   end
 
   context 'when project has no tags' do
     let(:project) { create(:project_empty_repo) }
+
     before do
       visit project_path(project)
       click_on 'README'
       fill_in :commit_message, with: 'Add a README file', visible: true
-      # Remove pre-receive hook so we can push without auth
-      FileUtils.rm_f(File.join(project.repository.path, 'hooks', 'pre-receive'))
       click_button 'Commit changes'
       visit project_tags_path(project)
     end
diff --git a/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js b/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js
index 9e6d0aa472c..74b343c573e 100644
--- a/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js
+++ b/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js
@@ -2,6 +2,7 @@ import Vue from 'vue';
 import mrWidgetOptions from '~/vue_merge_request_widget/mr_widget_options';
 import eventHub from '~/vue_merge_request_widget/event_hub';
 import notify from '~/lib/utils/notify';
+import { stateKey } from '~/vue_merge_request_widget/stores/state_maps';
 import mockData from './mock_data';
 import mountComponent from '../helpers/vue_mount_component_helper';
 
@@ -344,4 +345,31 @@ describe('mrWidgetOptions', () => {
       expect(comps['mr-widget-merge-when-pipeline-succeeds']).toBeDefined();
     });
   });
+
+  describe('rendering relatedLinks', () => {
+    beforeEach((done) => {
+      vm.mr.relatedLinks = {
+        assignToMe: null,
+        closing: `
+          <a class="close-related-link" href="#'>
+            Close
+          </a>
+        `,
+        mentioned: '',
+      };
+      Vue.nextTick(done);
+    });
+
+    it('renders if there are relatedLinks', () => {
+      expect(vm.$el.querySelector('.close-related-link')).toBeDefined();
+    });
+
+    it('does not render if state is nothingToMerge', (done) => {
+      vm.mr.state = stateKey.nothingToMerge;
+      Vue.nextTick(() => {
+        expect(vm.$el.querySelector('.close-related-link')).toBeNull();
+        done();
+      });
+    });
+  });
 });
diff --git a/spec/javascripts/vue_mr_widget/stores/mr_widget_store_spec.js b/spec/javascripts/vue_mr_widget/stores/mr_widget_store_spec.js
index 8e5614b20f0..33d052aceb2 100644
--- a/spec/javascripts/vue_mr_widget/stores/mr_widget_store_spec.js
+++ b/spec/javascripts/vue_mr_widget/stores/mr_widget_store_spec.js
@@ -1,4 +1,5 @@
 import MergeRequestStore from '~/vue_merge_request_widget/stores/mr_widget_store';
+import { stateKey } from '~/vue_merge_request_widget/stores/state_maps';
 import mockData from '../mock_data';
 
 describe('MergeRequestStore', () => {
@@ -52,5 +53,17 @@ describe('MergeRequestStore', () => {
         expect(store.isPipelineSkipped).toBe(false);
       });
     });
+
+    describe('isNothingToMergeState', () => {
+      it('returns true when nothingToMerge', () => {
+        store.state = stateKey.nothingToMerge;
+        expect(store.isNothingToMergeState).toEqual(true);
+      });
+
+      it('returns false when not nothingToMerge', () => {
+        store.state = 'state';
+        expect(store.isNothingToMergeState).toEqual(false);
+      });
+    });
   });
 });
diff --git a/spec/lib/gitlab/git/gitlab_projects_spec.rb b/spec/lib/gitlab/git/gitlab_projects_spec.rb
index 18906955df6..24da9589458 100644
--- a/spec/lib/gitlab/git/gitlab_projects_spec.rb
+++ b/spec/lib/gitlab/git/gitlab_projects_spec.rb
@@ -41,7 +41,8 @@ describe Gitlab::Git::GitlabProjects do
     end
 
     it "fails if the source path doesn't exist" do
-      expect(logger).to receive(:error).with("mv-project failed: source path <#{tmp_repos_path}/bad-src.git> does not exist.")
+      expected_source_path = File.join(tmp_repos_path, 'bad-src.git')
+      expect(logger).to receive(:error).with("mv-project failed: source path <#{expected_source_path}> does not exist.")
 
       result = build_gitlab_projects(tmp_repos_path, 'bad-src.git').mv_project('repo.git')
       expect(result).to be_falsy
@@ -50,7 +51,8 @@ describe Gitlab::Git::GitlabProjects do
     it 'fails if the destination path already exists' do
       FileUtils.mkdir_p(File.join(tmp_repos_path, 'already-exists.git'))
 
-      message = "mv-project failed: destination path <#{tmp_repos_path}/already-exists.git> already exists."
+      expected_distination_path = File.join(tmp_repos_path, 'already-exists.git')
+      message = "mv-project failed: destination path <#{expected_distination_path}> already exists."
       expect(logger).to receive(:error).with(message)
 
       expect(gl_projects.mv_project('already-exists.git')).to be_falsy
diff --git a/spec/models/blob_viewer/package_json_spec.rb b/spec/models/blob_viewer/package_json_spec.rb
index 0f8330e91c1..5ed2f4400bc 100644
--- a/spec/models/blob_viewer/package_json_spec.rb
+++ b/spec/models/blob_viewer/package_json_spec.rb
@@ -22,4 +22,51 @@ describe BlobViewer::PackageJson do
       expect(subject.package_name).to eq('module-name')
     end
   end
+
+  describe '#package_url' do
+    it 'returns the package URL' do
+      expect(subject).to receive(:prepare!)
+
+      expect(subject.package_url).to eq("https://www.npmjs.com/package/#{subject.package_name}")
+    end
+  end
+
+  describe '#package_type' do
+    it 'returns "package"' do
+      expect(subject).to receive(:prepare!)
+
+      expect(subject.package_type).to eq('package')
+    end
+  end
+
+  context 'when package.json has "private": true' do
+    let(:data) do
+      <<-SPEC.strip_heredoc
+      {
+        "name": "module-name",
+        "version": "10.3.1",
+        "private": true,
+        "homepage": "myawesomepackage.com"
+      }
+      SPEC
+    end
+    let(:blob) { fake_blob(path: 'package.json', data: data) }
+    subject { described_class.new(blob) }
+
+    describe '#package_url' do
+      it 'returns homepage if any' do
+        expect(subject).to receive(:prepare!)
+
+        expect(subject.package_url).to eq('myawesomepackage.com')
+      end
+    end
+
+    describe '#package_type' do
+      it 'returns "private package"' do
+        expect(subject).to receive(:prepare!)
+
+        expect(subject.package_type).to eq('private package')
+      end
+    end
+  end
 end
diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb
index d18a5c9dfa6..cd955a5eb69 100644
--- a/spec/models/commit_spec.rb
+++ b/spec/models/commit_spec.rb
@@ -13,6 +13,45 @@ describe Commit do
     it { is_expected.to include_module(StaticModel) }
   end
 
+  describe '.lazy' do
+    set(:project) { create(:project, :repository) }
+
+    context 'when the commits are found' do
+      let(:oids) do
+        %w(
+          498214de67004b1da3d820901307bed2a68a8ef6
+          c642fe9b8b9f28f9225d7ea953fe14e74748d53b
+          6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9
+          048721d90c449b244b7b4c53a9186b04330174ec
+          281d3a76f31c812dbf48abce82ccf6860adedd81
+        )
+      end
+
+      subject { oids.map { |oid| described_class.lazy(project, oid) } }
+
+      it 'batches requests for commits' do
+        expect(project.repository).to receive(:commits_by).once.and_call_original
+
+        subject.first.title
+        subject.last.title
+      end
+
+      it 'maintains ordering' do
+        subject.each_with_index do |commit, i|
+          expect(commit.id).to eq(oids[i])
+        end
+      end
+    end
+
+    context 'when not found' do
+      it 'returns nil as commit' do
+        commit = described_class.lazy(project, 'deadbeef').__sync
+
+        expect(commit).to be_nil
+      end
+    end
+  end
+
   describe '#author' do
     it 'looks up the author in a case-insensitive way' do
       user = create(:user, email: commit.author_email.upcase)
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index bdc430c9095..1d7069feebd 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -239,6 +239,54 @@ describe Repository do
     end
   end
 
+  describe '#commits_by' do
+    set(:project) { create(:project, :repository) }
+
+    shared_examples 'batch commits fetching' do
+      let(:oids) { TestEnv::BRANCH_SHA.values }
+
+      subject { project.repository.commits_by(oids: oids) }
+
+      it 'finds each commit' do
+        expect(subject).not_to include(nil)
+        expect(subject.size).to eq(oids.size)
+      end
+
+      it 'returns only Commit instances' do
+        expect(subject).to all( be_a(Commit) )
+      end
+
+      context 'when some commits are not found ' do
+        let(:oids) do
+          ['deadbeef'] + TestEnv::BRANCH_SHA.values.first(10)
+        end
+
+        it 'returns only found commits' do
+          expect(subject).not_to include(nil)
+          expect(subject.size).to eq(10)
+        end
+      end
+
+      context 'when no oids are passed' do
+        let(:oids) { [] }
+
+        it 'does not call #batch_by_oid' do
+          expect(Gitlab::Git::Commit).not_to receive(:batch_by_oid)
+
+          subject
+        end
+      end
+    end
+
+    context 'when Gitaly list_commits_by_oid is enabled' do
+      it_behaves_like 'batch commits fetching'
+    end
+
+    context 'when Gitaly list_commits_by_oid is enabled', :disable_gitaly do
+      it_behaves_like 'batch commits fetching'
+    end
+  end
+
   describe '#find_commits_by_message' do
     shared_examples 'finding commits by message' do
       it 'returns commits with messages containing a given string' do
diff --git a/spec/serializers/pipeline_serializer_spec.rb b/spec/serializers/pipeline_serializer_spec.rb
index 88d347322a6..c38795ad1a1 100644
--- a/spec/serializers/pipeline_serializer_spec.rb
+++ b/spec/serializers/pipeline_serializer_spec.rb
@@ -1,6 +1,7 @@
 require 'spec_helper'
 
 describe PipelineSerializer do
+  set(:project) { create(:project, :repository) }
   set(:user) { create(:user) }
 
   let(:serializer) do
@@ -16,7 +17,7 @@ describe PipelineSerializer do
       end
 
       context 'when a single object is being serialized' do
-        let(:resource) { create(:ci_empty_pipeline) }
+        let(:resource) { create(:ci_empty_pipeline, project: project) }
 
         it 'serializers the pipeline object' do
           expect(subject[:id]).to eq resource.id
@@ -24,7 +25,7 @@ describe PipelineSerializer do
       end
 
       context 'when multiple objects are being serialized' do
-        let(:resource) { create_list(:ci_pipeline, 2) }
+        let(:resource) { create_list(:ci_pipeline, 2, project: project) }
 
         it 'serializers the array of pipelines' do
           expect(subject).not_to be_empty
@@ -100,7 +101,6 @@ describe PipelineSerializer do
 
     context 'number of queries' do
       let(:resource) { Ci::Pipeline.all }
-      let(:project) { create(:project) }
 
       before do
         # Since RequestStore.active? is true we have to allow the
diff --git a/spec/services/projects/unlink_fork_service_spec.rb b/spec/services/projects/unlink_fork_service_spec.rb
index 2bba71fef4f..3ec6139bfa6 100644
--- a/spec/services/projects/unlink_fork_service_spec.rb
+++ b/spec/services/projects/unlink_fork_service_spec.rb
@@ -62,6 +62,26 @@ describe Projects::UnlinkForkService do
     expect(source.forks_count).to be_zero
   end
 
+  context 'when the source has LFS objects' do
+    let(:lfs_object) { create(:lfs_object) }
+
+    before do
+      lfs_object.projects << project
+    end
+
+    it 'links the fork to the lfs object before unlinking' do
+      subject.execute
+
+      expect(lfs_object.projects).to include(forked_project)
+    end
+
+    it 'does not fail if the lfs objects were already linked' do
+      lfs_object.projects << forked_project
+
+      expect { subject.execute }.not_to raise_error
+    end
+  end
+
   context 'when the original project was deleted' do
     it 'does not fail when the original project is deleted' do
       source = forked_project.forked_from_project
diff --git a/spec/views/events/event/_push.html.haml_spec.rb b/spec/views/events/event/_push.html.haml_spec.rb
new file mode 100644
index 00000000000..f5634de4916
--- /dev/null
+++ b/spec/views/events/event/_push.html.haml_spec.rb
@@ -0,0 +1,55 @@
+require 'spec_helper'
+
+describe 'events/event/_push.html.haml' do
+  let(:event) { build_stubbed(:push_event) }
+
+  context 'with a branch' do
+    let(:payload) { build_stubbed(:push_event_payload, event: event) }
+
+    before do
+      allow(event).to receive(:push_event_payload).and_return(payload)
+    end
+
+    it 'links to the branch' do
+      allow(event.project.repository).to receive(:branch_exists?).with(event.ref_name).and_return(true)
+      link = project_commits_path(event.project, event.ref_name)
+
+      render partial: 'events/event/push', locals: { event: event }
+
+      expect(rendered).to have_link(event.ref_name, href: link)
+    end
+
+    context 'that has been deleted' do
+      it 'does not link to the branch' do
+        render partial: 'events/event/push', locals: { event: event }
+
+        expect(rendered).not_to have_link(event.ref_name)
+      end
+    end
+  end
+
+  context 'with a tag' do
+    let(:payload) { build_stubbed(:push_event_payload, event: event, ref_type: :tag, ref: 'v0.1.0') }
+
+    before do
+      allow(event).to receive(:push_event_payload).and_return(payload)
+    end
+
+    it 'links to the tag' do
+      allow(event.project.repository).to receive(:tag_exists?).with(event.ref_name).and_return(true)
+      link = project_commits_path(event.project, event.ref_name)
+
+      render partial: 'events/event/push', locals: { event: event }
+
+      expect(rendered).to have_link(event.ref_name, href: link)
+    end
+
+    context 'that has been deleted' do
+      it 'does not link to the tag' do
+        render partial: 'events/event/push', locals: { event: event }
+
+        expect(rendered).not_to have_link(event.ref_name)
+      end
+    end
+  end
+end