Merge branch 'master' into docs-patch-71docs-patch-71

7 files changed, 243 insertions, 26 deletions

diff --git a/spec/controllers/concerns/confirm_email_warning_spec.rb b/spec/controllers/concerns/confirm_email_warning_spec.rb
new file mode 100644
index 00000000000..0c598a360af
--- /dev/null
+++ b/spec/controllers/concerns/confirm_email_warning_spec.rb
@@ -0,0 +1,98 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ConfirmEmailWarning do
+  before do
+    stub_feature_flags(soft_email_confirmation: true)
+    allow(User).to receive(:allow_unconfirmed_access_for).and_return 2.days
+  end
+
+  controller(ApplicationController) do
+    # `described_class` is not available in this context
+    include ConfirmEmailWarning # rubocop:disable RSpec/DescribedClass
+
+    def index
+      head :ok
+    end
+  end
+
+  RSpec::Matchers.define :set_confirm_warning_for do |email|
+    match do |response|
+      expect(response).to set_flash.now[:warning].to include("Please check your email (#{email}) to verify that you own this address.")
+    end
+  end
+
+  describe 'confirm email flash warning' do
+    context 'when not signed in' do
+      let(:user) { create(:user, confirmed_at: nil) }
+
+      before do
+        get :index
+      end
+
+      it { is_expected.not_to set_confirm_warning_for(user.email) }
+    end
+
+    context 'when signed in' do
+      before do
+        sign_in(user)
+      end
+
+      context 'with a confirmed user' do
+        let(:user) { create(:user) }
+
+        before do
+          get :index
+        end
+
+        it { is_expected.not_to set_confirm_warning_for(user.email) }
+      end
+
+      context 'with an unconfirmed user' do
+        let(:user) { create(:user, confirmed_at: nil) }
+
+        context 'when executing a peek request' do
+          before do
+            request.path = '/-/peek'
+            get :index
+          end
+
+          it { is_expected.not_to set_confirm_warning_for(user.email) }
+        end
+
+        context 'when executing a json request' do
+          before do
+            get :index, format: :json
+          end
+
+          it { is_expected.not_to set_confirm_warning_for(user.email) }
+        end
+
+        context 'when executing a post request' do
+          before do
+            post :index
+          end
+
+          it { is_expected.not_to set_confirm_warning_for(user.email) }
+        end
+
+        context 'when executing a get request' do
+          before do
+            get :index
+          end
+
+          context 'with an unconfirmed email address present' do
+            let(:user) { create(:user, confirmed_at: nil, unconfirmed_email: 'unconfirmed@gitlab.com') }
+
+            it { is_expected.to set_confirm_warning_for(user.unconfirmed_email) }
+          end
+
+          context 'without an unconfirmed email address present' do
+            it { is_expected.to set_confirm_warning_for(user.email) }
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/spec/controllers/concerns/continue_params_spec.rb b/spec/controllers/concerns/continue_params_spec.rb
index 5e47f5e9f28..b4b62cbe1e3 100644
--- a/spec/controllers/concerns/continue_params_spec.rb
+++ b/spec/controllers/concerns/continue_params_spec.rb
@@ -18,6 +18,14 @@ describe ContinueParams do
     ActionController::Parameters.new(continue: params)
   end
 
+  it 'returns an empty hash if params are not present' do
+    allow(controller).to receive(:params) do
+      ActionController::Parameters.new
+    end
+
+    expect(controller.continue_params).to eq({})
+  end
+
   it 'cleans up any params that are not allowed' do
     allow(controller).to receive(:params) do
       strong_continue_params(to: '/hello',
diff --git a/spec/controllers/concerns/group_tree_spec.rb b/spec/controllers/concerns/group_tree_spec.rb
index aa3cd690e3f..835c3d9b3af 100644
--- a/spec/controllers/concerns/group_tree_spec.rb
+++ b/spec/controllers/concerns/group_tree_spec.rb
@@ -30,7 +30,7 @@ describe GroupTree do
       expect(assigns(:groups)).to contain_exactly(other_group)
     end
 
-    context 'for subgroups', :nested_groups do
+    context 'for subgroups' do
       it 'only renders root groups when no parent was given' do
         create(:group, :public, parent: group)
 
@@ -85,7 +85,7 @@ describe GroupTree do
         expect(json_response.first['id']).to eq(group.id)
       end
 
-      context 'nested groups', :nested_groups do
+      context 'nested groups' do
         it 'expands the tree when filtering' do
           subgroup = create(:group, :public, parent: group, name: 'filter')
 
diff --git a/spec/controllers/concerns/internal_redirect_spec.rb b/spec/controllers/concerns/internal_redirect_spec.rb
index 97119438ca1..da68c8c8697 100644
--- a/spec/controllers/concerns/internal_redirect_spec.rb
+++ b/spec/controllers/concerns/internal_redirect_spec.rb
@@ -15,44 +15,71 @@ describe InternalRedirect do
   subject(:controller) { controller_class.new }
 
   describe '#safe_redirect_path' do
-    it 'is `nil` for invalid uris' do
-      expect(controller.safe_redirect_path('Hello world')).to be_nil
+    where(:input) do
+      [
+        'Hello world',
+        '//example.com/hello/world',
+        'https://example.com/hello/world'
+      ]
     end
 
-    it 'is `nil` for paths trying to include a host' do
-      expect(controller.safe_redirect_path('//example.com/hello/world')).to be_nil
+    with_them 'being invalid' do
+      it 'returns nil' do
+        expect(controller.safe_redirect_path(input)).to be_nil
+      end
     end
 
-    it 'returns the path if it is valid' do
-      expect(controller.safe_redirect_path('/hello/world')).to eq('/hello/world')
+    where(:input) do
+      [
+        '/hello/world',
+        '/-/ide/project/path'
+      ]
     end
 
-    it 'returns the path with querystring if it is valid' do
-      expect(controller.safe_redirect_path('/hello/world?hello=world#L123'))
-        .to eq('/hello/world?hello=world#L123')
+    with_them 'being valid' do
+      it 'returns the path' do
+        expect(controller.safe_redirect_path(input)).to eq(input)
+      end
+
+      it 'returns the path with querystring and fragment' do
+        expect(controller.safe_redirect_path("#{input}?hello=world#L123"))
+          .to eq("#{input}?hello=world#L123")
+      end
     end
   end
 
   describe '#safe_redirect_path_for_url' do
-    it 'is `nil` for invalid urls' do
-      expect(controller.safe_redirect_path_for_url('Hello world')).to be_nil
+    where(:input) do
+      [
+        'Hello world',
+        'http://example.com/hello/world',
+        'http://test.host:3000/hello/world'
+      ]
     end
 
-    it 'is `nil` for urls from a with a different host' do
-      expect(controller.safe_redirect_path_for_url('http://example.com/hello/world')).to be_nil
+    with_them 'being invalid' do
+      it 'returns nil' do
+        expect(controller.safe_redirect_path_for_url(input)).to be_nil
+      end
     end
 
-    it 'is `nil` for urls from a with a different port' do
-      expect(controller.safe_redirect_path_for_url('http://test.host:3000/hello/world')).to be_nil
+    where(:input) do
+      [
+        'http://test.host/hello/world'
+      ]
     end
 
-    it 'returns the path if the url is on the same host' do
-      expect(controller.safe_redirect_path_for_url('http://test.host/hello/world')).to eq('/hello/world')
-    end
+    with_them 'being on the same host' do
+      let(:path) { URI(input).path }
 
-    it 'returns the path including querystring if the url is on the same host' do
-      expect(controller.safe_redirect_path_for_url('http://test.host/hello/world?hello=world#L123'))
-        .to eq('/hello/world?hello=world#L123')
+      it 'returns the path' do
+        expect(controller.safe_redirect_path_for_url(input)).to eq(path)
+      end
+
+      it 'returns the path with querystring and fragment' do
+        expect(controller.safe_redirect_path_for_url("#{input}?hello=world#L123"))
+          .to eq("#{path}?hello=world#L123")
+      end
     end
   end
 
@@ -82,12 +109,16 @@ describe InternalRedirect do
   end
 
   describe '#host_allowed?' do
-    it 'allows uris with the same host and port' do
+    it 'allows URI with the same host and port' do
       expect(controller.host_allowed?(URI('http://test.host/test'))).to be(true)
     end
 
-    it 'rejects uris with other host and port' do
+    it 'rejects URI with other host' do
       expect(controller.host_allowed?(URI('http://example.com/test'))).to be(false)
     end
+
+    it 'rejects URI with other port' do
+      expect(controller.host_allowed?(URI('http://test.host:3000/test'))).to be(false)
+    end
   end
 end
diff --git a/spec/controllers/concerns/issuable_actions_spec.rb b/spec/controllers/concerns/issuable_actions_spec.rb
new file mode 100644
index 00000000000..7b0b4497f3f
--- /dev/null
+++ b/spec/controllers/concerns/issuable_actions_spec.rb
@@ -0,0 +1,69 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe IssuableActions do
+  let(:project) { double('project') }
+  let(:user) { double('user') }
+  let(:issuable) { double('issuable') }
+  let(:finder_params_for_issuable) { {} }
+  let(:notes_result) { double('notes_result') }
+  let(:discussion_serializer) { double('discussion_serializer') }
+
+  let(:controller) do
+    klass = Class.new do
+      attr_reader :current_user, :project, :issuable
+
+      def self.before_action(action, params = nil)
+      end
+
+      include IssuableActions
+
+      def initialize(issuable, project, user, finder_params)
+        @issuable = issuable
+        @project = project
+        @current_user = user
+        @finder_params = finder_params
+      end
+
+      def finder_params_for_issuable
+        @finder_params
+      end
+
+      def params
+        {
+          notes_filter: 1
+        }
+      end
+
+      def prepare_notes_for_rendering(notes)
+        []
+      end
+
+      def render(options)
+      end
+    end
+
+    klass.new(issuable, project, user, finder_params_for_issuable)
+  end
+
+  describe '#discussions' do
+    before do
+      allow(user).to receive(:set_notes_filter)
+      allow(user).to receive(:user_preference)
+      allow(discussion_serializer).to receive(:represent)
+    end
+
+    it 'instantiates and calls NotesFinder as expected' do
+      expect(Discussion).to receive(:build_collection).and_return([])
+      expect(DiscussionSerializer).to receive(:new).and_return(discussion_serializer)
+      expect(NotesFinder).to receive(:new).with(user, finder_params_for_issuable).and_call_original
+
+      expect_any_instance_of(NotesFinder).to receive(:execute).and_return(notes_result)
+
+      expect(notes_result).to receive_messages(inc_relations_for_view: notes_result, includes: notes_result, fresh: notes_result)
+
+      controller.discussions
+    end
+  end
+end
diff --git a/spec/controllers/concerns/issuable_collections_spec.rb b/spec/controllers/concerns/issuable_collections_spec.rb
index fb2cd5ca955..f210537aad5 100644
--- a/spec/controllers/concerns/issuable_collections_spec.rb
+++ b/spec/controllers/concerns/issuable_collections_spec.rb
@@ -180,5 +180,16 @@ describe IssuableCollections do
         is_expected.not_to include('invalid_param', 'invalid_array')
       end
     end
+
+    context 'search using an issue iid' do
+      let(:params) { { search: "#5" } }
+
+      it 'mutates the search into a filter by iid' do
+        is_expected.to include({
+            'iids' => '5',
+            'search' => nil
+        })
+      end
+    end
   end
 end
diff --git a/spec/controllers/concerns/send_file_upload_spec.rb b/spec/controllers/concerns/send_file_upload_spec.rb
index a3ce08f736c..3bf0ec799c7 100644
--- a/spec/controllers/concerns/send_file_upload_spec.rb
+++ b/spec/controllers/concerns/send_file_upload_spec.rb
@@ -115,7 +115,7 @@ describe SendFileUpload do
 
         it 'sends a file with a custom type' do
           headers = double
-          expected_headers = /response-content-disposition=attachment%3B%20filename%3D%22test.js%22%3B%20filename%2A%3DUTF-8%27%27test.js&response-content-type=application%2Fecmascript/
+          expected_headers = /response-content-disposition=attachment%3B%20filename%3D%22test.js%22%3B%20filename%2A%3DUTF-8%27%27test.js&response-content-type=application%2Fjavascript/
           expect(Gitlab::Workhorse).to receive(:send_url).with(expected_headers).and_call_original
           expect(headers).to receive(:store).with(Gitlab::Workhorse::SEND_DATA_HEADER, /^send-url:/)