Add latest changes from gitlab-org/gitlab@master

3 files changed, 107 insertions, 9 deletions

diff --git a/spec/services/metrics/dashboard/update_dashboard_service_spec.rb b/spec/services/metrics/dashboard/update_dashboard_service_spec.rb
index 66622524e9c..227041344d7 100644
--- a/spec/services/metrics/dashboard/update_dashboard_service_spec.rb
+++ b/spec/services/metrics/dashboard/update_dashboard_service_spec.rb
@@ -27,7 +27,14 @@ describe Metrics::Dashboard::UpdateDashboardService, :use_clean_rails_memory_sto
     end
 
     context 'user does not have push right to repository' do
-      it_behaves_like 'misconfigured dashboard service response', :forbidden, "You are not allowed to push into this branch. Create another branch or open a merge request."
+      it 'returns an appropriate message and status code', :aggregate_failures do
+        result = service_call
+
+        expect(result.keys).to contain_exactly(:message, :http_status, :status, :last_step)
+        expect(result[:status]).to eq(:error)
+        expect(result[:http_status]).to eq(:forbidden)
+        expect(result[:message]).to eq("You are not allowed to push into this branch. Create another branch or open a merge request.")
+      end
     end
 
     context 'with rights to push to the repository' do
@@ -39,13 +46,27 @@ describe Metrics::Dashboard::UpdateDashboardService, :use_clean_rails_memory_sto
         context 'with a yml extension' do
           let(:file_name) { 'config/prometheus/../database.yml' }
 
-          it_behaves_like 'misconfigured dashboard service response', :bad_request, "A file with this name doesn't exist"
+          it 'returns an appropriate message and status code', :aggregate_failures do
+            result = service_call
+
+            expect(result.keys).to contain_exactly(:message, :http_status, :status, :last_step)
+            expect(result[:status]).to eq(:error)
+            expect(result[:http_status]).to eq(:bad_request)
+            expect(result[:message]).to eq("A file with this name doesn't exist")
+          end
         end
 
         context 'without a yml extension' do
           let(:file_name) { '../../..../etc/passwd' }
 
-          it_behaves_like 'misconfigured dashboard service response', :bad_request, "The file name should have a .yml extension"
+          it 'returns an appropriate message and status code', :aggregate_failures do
+            result = service_call
+
+            expect(result.keys).to contain_exactly(:message, :http_status, :status, :last_step)
+            expect(result[:status]).to eq(:error)
+            expect(result[:http_status]).to eq(:bad_request)
+            expect(result[:message]).to eq("The file name should have a .yml extension")
+          end
         end
       end
 
@@ -60,7 +81,14 @@ describe Metrics::Dashboard::UpdateDashboardService, :use_clean_rails_memory_sto
           project.repository.add_branch(user, branch, 'master')
         end
 
-        it_behaves_like 'misconfigured dashboard service response', :bad_request, "There was an error updating the dashboard, branch named: existing_branch already exists."
+        it 'returns an appropriate message and status code', :aggregate_failures do
+          result = service_call
+
+          expect(result.keys).to contain_exactly(:message, :http_status, :status, :last_step)
+          expect(result[:status]).to eq(:error)
+          expect(result[:http_status]).to eq(:bad_request)
+          expect(result[:message]).to eq("There was an error updating the dashboard, branch named: existing_branch already exists.")
+        end
       end
 
       context 'Files::UpdateService success' do
diff --git a/spec/services/projects/import_service_spec.rb b/spec/services/projects/import_service_spec.rb
index d9f9ede8ecd..1e9ac40128a 100644
--- a/spec/services/projects/import_service_spec.rb
+++ b/spec/services/projects/import_service_spec.rb
@@ -122,7 +122,7 @@ describe Projects::ImportService do
         end
 
         it 'succeeds if repository import is successful' do
-          expect_any_instance_of(Gitlab::Shell).to receive(:import_repository).and_return(true)
+          expect(project.repository).to receive(:import_repository).and_return(true)
           expect_any_instance_of(Gitlab::BitbucketImport::Importer).to receive(:execute).and_return(true)
           expect_any_instance_of(Projects::LfsPointers::LfsImportService).to receive(:execute).and_return(status: :success)
 
@@ -132,7 +132,9 @@ describe Projects::ImportService do
         end
 
         it 'fails if repository import fails' do
-          expect_any_instance_of(Gitlab::Shell).to receive(:import_repository).and_raise(Gitlab::Shell::Error.new('Failed to import the repository /a/b/c'))
+          expect(project.repository)
+            .to receive(:import_repository)
+            .and_raise(Gitlab::Git::CommandError, 'Failed to import the repository /a/b/c')
 
           result = subject.execute
 
@@ -144,7 +146,7 @@ describe Projects::ImportService do
           it 'logs the error' do
             error_message = 'error message'
 
-            expect_any_instance_of(Gitlab::Shell).to receive(:import_repository).and_return(true)
+            expect(project.repository).to receive(:import_repository).and_return(true)
             expect_any_instance_of(Gitlab::BitbucketImport::Importer).to receive(:execute).and_return(true)
             expect_any_instance_of(Projects::LfsPointers::LfsImportService).to receive(:execute).and_return(status: :error, message: error_message)
             expect(Gitlab::AppLogger).to receive(:error).with("The Lfs import process failed. #{error_message}")
@@ -155,7 +157,7 @@ describe Projects::ImportService do
 
         context 'when repository import scheduled' do
           before do
-            allow_any_instance_of(Gitlab::Shell).to receive(:import_repository).and_return(true)
+            expect(project.repository).to receive(:import_repository).and_return(true)
             allow(subject).to receive(:import_data)
           end
 
diff --git a/spec/services/search_service_spec.rb b/spec/services/search_service_spec.rb
index 48065bf596a..10dafaebe85 100644
--- a/spec/services/search_service_spec.rb
+++ b/spec/services/search_service_spec.rb
@@ -10,13 +10,16 @@ describe SearchService do
   let!(:group_member) { create(:group_member, group: accessible_group, user: user) }
 
   let!(:accessible_project) { create(:project, :private, name: 'accessible_project') }
-  let!(:inaccessible_project) { create(:project, :private, name: 'inaccessible_project') }
   let(:note) { create(:note_on_issue, project: accessible_project) }
 
+  let!(:inaccessible_project) { create(:project, :private, name: 'inaccessible_project') }
+
   let(:snippet) { create(:snippet, author: user) }
   let(:group_project) { create(:project, group: accessible_group, name: 'group_project') }
   let(:public_project) { create(:project, :public, name: 'public_project') }
 
+  subject(:search_service) { described_class.new(user, search: search, scope: scope, page: 1) }
+
   before do
     accessible_project.add_maintainer(user)
   end
@@ -293,5 +296,70 @@ describe SearchService do
         expect(search_objects.first).to eq public_project
       end
     end
+
+    context 'redacting search results' do
+      shared_examples 'it redacts incorrect results' do
+        before do
+          allow(Ability).to receive(:allowed?).and_return(allowed)
+        end
+
+        context 'when allowed' do
+          let(:allowed) { true }
+
+          it 'does nothing' do
+            expect(results).not_to be_empty
+            expect(results).to all(be_an(model_class))
+          end
+        end
+
+        context 'when disallowed' do
+          let(:allowed) { false }
+
+          it 'does nothing' do
+            expect(results).to be_empty
+          end
+        end
+      end
+
+      context 'issues' do
+        let(:issue) { create(:issue, project: accessible_project) }
+        let(:scope) { 'issues' }
+        let(:model_class) { Issue }
+        let(:ability) { :read_issue }
+        let(:search) { issue.title }
+        let(:results) { subject.search_objects }
+
+        it_behaves_like 'it redacts incorrect results'
+      end
+
+      context 'notes' do
+        let(:note) { create(:note_on_commit, project: accessible_project) }
+        let(:scope) { 'notes' }
+        let(:model_class) { Note }
+        let(:ability) { :read_note }
+        let(:search) { note.note }
+        let(:results) do
+          described_class.new(
+            user,
+            project_id: accessible_project.id,
+            scope: scope,
+            search: note.note
+          ).search_objects
+        end
+
+        it_behaves_like 'it redacts incorrect results'
+      end
+
+      context 'merge_requests' do
+        let(:scope) { 'merge_requests' }
+        let(:model_class) { MergeRequest }
+        let(:ability) { :read_merge_request }
+        let(:merge_request) { create(:merge_request, source_project: accessible_project, author: user) }
+        let(:search) { merge_request.title }
+        let(:results) { subject.search_objects }
+
+        it_behaves_like 'it redacts incorrect results'
+      end
+    end
   end
 end