Merge remote-tracking branch 'origin/master' into 38759-fetch-available-parameters-directly-from-gke-when-creating-a-cluster

11 files changed, 174 insertions, 127 deletions

diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb
index 9a0b6efd8a9..2b88fcc9a96 100644
--- a/spec/services/ci/create_pipeline_service_spec.rb
+++ b/spec/services/ci/create_pipeline_service_spec.rb
@@ -395,7 +395,27 @@ describe Ci::CreatePipelineService do
         result = execute_service
 
         expect(result).to be_persisted
-        expect(Environment.find_by(name: "review/master")).not_to be_nil
+        expect(Environment.find_by(name: "review/master")).to be_present
+      end
+    end
+
+    context 'with environment name including persisted variables' do
+      before do
+        config = YAML.dump(
+          deploy: {
+            environment: { name: "review/id1$CI_PIPELINE_ID/id2$CI_BUILD_ID" },
+            script: 'ls'
+          }
+        )
+
+        stub_ci_pipeline_yaml_file(config)
+      end
+
+      it 'skipps persisted variables in environment name' do
+        result = execute_service
+
+        expect(result).to be_persisted
+        expect(Environment.find_by(name: "review/id1/id2")).to be_present
       end
     end
 
diff --git a/spec/services/ci/retry_pipeline_service_spec.rb b/spec/services/ci/retry_pipeline_service_spec.rb
index f1acfc48468..a73bd7a0268 100644
--- a/spec/services/ci/retry_pipeline_service_spec.rb
+++ b/spec/services/ci/retry_pipeline_service_spec.rb
@@ -1,6 +1,8 @@
 require 'spec_helper'
 
 describe Ci::RetryPipelineService, '#execute' do
+  include ProjectForksHelper
+
   let(:user) { create(:user) }
   let(:project) { create(:project) }
   let(:pipeline) { create(:ci_pipeline, project: project) }
@@ -266,6 +268,33 @@ describe Ci::RetryPipelineService, '#execute' do
     end
   end
 
+  context 'when maintainer is allowed to push to forked project' do
+    let(:user) { create(:user) }
+    let(:project) { create(:project, :public) }
+    let(:forked_project) { fork_project(project) }
+    let(:pipeline) { create(:ci_pipeline, project: forked_project, ref: 'fixes') }
+
+    before do
+      project.add_master(user)
+      create(:merge_request,
+        source_project: forked_project,
+        target_project: project,
+        source_branch: 'fixes',
+        allow_maintainer_to_push: true)
+      create_build('rspec 1', :failed, 1)
+    end
+
+    it 'allows to retry failed pipeline' do
+      allow_any_instance_of(Project).to receive(:fetch_branch_allows_maintainer_push?).and_return(true)
+      allow_any_instance_of(Project).to receive(:empty_repo?).and_return(false)
+
+      service.execute(pipeline)
+
+      expect(build('rspec 1')).to be_pending
+      expect(pipeline.reload).to be_running
+    end
+  end
+
   def statuses
     pipeline.reload.statuses
   end
diff --git a/spec/services/clusters/applications/schedule_installation_service_spec.rb b/spec/services/clusters/applications/schedule_installation_service_spec.rb
index 473dbcbb692..bca1e71bef2 100644
--- a/spec/services/clusters/applications/schedule_installation_service_spec.rb
+++ b/spec/services/clusters/applications/schedule_installation_service_spec.rb
@@ -2,7 +2,7 @@ require 'spec_helper'
 
 describe Clusters::Applications::ScheduleInstallationService do
   def count_scheduled
-    application_class&.with_status(:scheduled)&.count || 0
+    application&.class&.with_status(:scheduled)&.count || 0
   end
 
   shared_examples 'a failing service' do
@@ -10,45 +10,42 @@ describe Clusters::Applications::ScheduleInstallationService do
       expect(ClusterInstallAppWorker).not_to receive(:perform_async)
       count_before = count_scheduled
 
-      expect { service.execute }.to raise_error(StandardError)
+      expect { service.execute(application) }.to raise_error(StandardError)
       expect(count_scheduled).to eq(count_before)
     end
   end
 
   describe '#execute' do
-    let(:application_class) { Clusters::Applications::Helm }
-    let(:cluster) { create(:cluster, :project, :provided_by_gcp) }
-    let(:project) { cluster.project }
-    let(:service) { described_class.new(project, nil, cluster: cluster, application_class: application_class) }
+    let(:project) { double(:project) }
+    let(:service) { described_class.new(project, nil) }
 
-    it 'creates a new application' do
-      allow(ClusterInstallAppWorker).to receive(:perform_async)
+    context 'when application is installable' do
+      let(:application) { create(:clusters_applications_helm, :installable) }
 
-      expect { service.execute }.to change { application_class.count }.by(1)
-    end
-
-    it 'make the application scheduled' do
-      expect(ClusterInstallAppWorker).to receive(:perform_async).with(application_class.application_name, kind_of(Numeric)).once
+      it 'make the application scheduled' do
+        expect(ClusterInstallAppWorker).to receive(:perform_async).with(application.name, kind_of(Numeric)).once
 
-      expect { service.execute }.to change { application_class.with_status(:scheduled).count }.by(1)
+        expect { service.execute(application) }.to change { application.class.with_status(:scheduled).count }.by(1)
+      end
     end
 
     context 'when installation is already in progress' do
       let(:application) { create(:clusters_applications_helm, :installing) }
-      let(:cluster) { application.cluster }
 
       it_behaves_like 'a failing service'
     end
 
-    context 'when application_class is nil' do
-      let(:application_class) { nil }
+    context 'when application is nil' do
+      let(:application) { nil }
 
       it_behaves_like 'a failing service'
     end
 
     context 'when application cannot be persisted' do
+      let(:application) { create(:clusters_applications_helm) }
+
       before do
-        expect_any_instance_of(application_class).to receive(:make_scheduled!).once.and_raise(ActiveRecord::RecordInvalid)
+        expect(application).to receive(:make_scheduled!).once.and_raise(ActiveRecord::RecordInvalid)
       end
 
       it_behaves_like 'a failing service'
diff --git a/spec/services/clusters/create_service_spec.rb b/spec/services/clusters/create_service_spec.rb
index 1c2f9c5cf43..1685dc748bd 100644
--- a/spec/services/clusters/create_service_spec.rb
+++ b/spec/services/clusters/create_service_spec.rb
@@ -8,80 +8,22 @@ describe Clusters::CreateService do
   subject { described_class.new(project, user, params).execute(access_token) }
 
   context 'when provider is gcp' do
-    shared_context 'valid params' do
-      let(:params) do
-        {
-          name: 'test-cluster',
-          provider_type: :gcp,
-          provider_gcp_attributes: {
-            gcp_project_id: 'gcp-project',
-            zone: 'us-central1-a',
-            num_nodes: 1,
-            machine_type: 'machine_type-a'
-          }
-        }
-      end
-    end
-
-    shared_context 'invalid params' do
-      let(:params) do
-        {
-          name: 'test-cluster',
-          provider_type: :gcp,
-          provider_gcp_attributes: {
-            gcp_project_id: '!!!!!!!',
-            zone: 'us-central1-a',
-            num_nodes: 1,
-            machine_type: 'machine_type-a'
-          }
-        }
-      end
-    end
-
-    shared_examples 'create cluster' do
-      it 'creates a cluster object and performs a worker' do
-        expect(ClusterProvisionWorker).to receive(:perform_async)
-
-        expect { subject }
-          .to change { Clusters::Cluster.count }.by(1)
-          .and change { Clusters::Providers::Gcp.count }.by(1)
-
-        expect(subject.name).to eq('test-cluster')
-        expect(subject.user).to eq(user)
-        expect(subject.project).to eq(project)
-        expect(subject.provider.gcp_project_id).to eq('gcp-project')
-        expect(subject.provider.zone).to eq('us-central1-a')
-        expect(subject.provider.num_nodes).to eq(1)
-        expect(subject.provider.machine_type).to eq('machine_type-a')
-        expect(subject.provider.access_token).to eq(access_token)
-        expect(subject.platform).to be_nil
-      end
-    end
-
-    shared_examples 'error' do
-      it 'returns an error' do
-        expect(ClusterProvisionWorker).not_to receive(:perform_async)
-        expect { subject }.to change { Clusters::Cluster.count }.by(0)
-        expect(subject.errors[:"provider_gcp.gcp_project_id"]).to be_present
-      end
-    end
-
     context 'when project has no clusters' do
       context 'when correct params' do
-        include_context 'valid params'
+        include_context 'valid cluster create params'
 
-        include_examples 'create cluster'
+        include_examples 'create cluster service success'
       end
 
       context 'when invalid params' do
-        include_context 'invalid params'
+        include_context 'invalid cluster create params'
 
-        include_examples 'error'
+        include_examples 'create cluster service error'
       end
     end
 
     context 'when project has a cluster' do
-      include_context 'valid params'
+      include_context 'valid cluster create params'
       let!(:cluster) { create(:cluster, :provided_by_gcp, :production_environment, projects: [project]) }
 
       it 'does not create a cluster' do
diff --git a/spec/services/keys/destroy_service_spec.rb b/spec/services/keys/destroy_service_spec.rb
new file mode 100644
index 00000000000..28ac72ddd42
--- /dev/null
+++ b/spec/services/keys/destroy_service_spec.rb
@@ -0,0 +1,13 @@
+require 'spec_helper'
+
+describe Keys::DestroyService do
+  let(:user) { create(:user) }
+
+  subject { described_class.new(user) }
+
+  it 'destroys a key' do
+    key = create(:key)
+
+    expect { subject.execute(key) }.to change(Key, :count).by(-1)
+  end
+end
diff --git a/spec/services/merge_requests/conflicts/list_service_spec.rb b/spec/services/merge_requests/conflicts/list_service_spec.rb
index 837b8a56d12..d57852615d9 100644
--- a/spec/services/merge_requests/conflicts/list_service_spec.rb
+++ b/spec/services/merge_requests/conflicts/list_service_spec.rb
@@ -3,7 +3,7 @@ require 'spec_helper'
 describe MergeRequests::Conflicts::ListService do
   describe '#can_be_resolved_in_ui?' do
     def create_merge_request(source_branch)
-      create(:merge_request, source_branch: source_branch, target_branch: 'conflict-start') do |mr|
+      create(:merge_request, source_branch: source_branch, target_branch: 'conflict-start', merge_status: :unchecked) do |mr|
         mr.mark_as_unmergeable
       end
     end
diff --git a/spec/services/merge_requests/merge_when_pipeline_succeeds_service_spec.rb b/spec/services/merge_requests/merge_when_pipeline_succeeds_service_spec.rb
index 240aa638f79..8838742a637 100644
--- a/spec/services/merge_requests/merge_when_pipeline_succeeds_service_spec.rb
+++ b/spec/services/merge_requests/merge_when_pipeline_succeeds_service_spec.rb
@@ -112,32 +112,6 @@ describe MergeRequests::MergeWhenPipelineSucceedsService do
         service.trigger(unrelated_pipeline)
       end
     end
-
-    context 'when the merge request is not mergeable' do
-      let(:mr_conflict) do
-        create(:merge_request, merge_when_pipeline_succeeds: true, merge_user: user,
-                               source_branch: 'master', target_branch: 'feature-conflict',
-                               source_project: project, target_project: project)
-      end
-
-      let(:conflict_pipeline) do
-        create(:ci_pipeline, project: project, ref: mr_conflict.source_branch,
-                             sha: mr_conflict.diff_head_sha, status: 'success',
-                             head_pipeline_of: mr_conflict)
-      end
-
-      it 'does not merge the merge request' do
-        expect(MergeWorker).not_to receive(:perform_async)
-
-        service.trigger(conflict_pipeline)
-      end
-
-      it 'creates todos for unmergeability' do
-        expect_any_instance_of(TodoService).to receive(:merge_request_became_unmergeable).with(mr_conflict)
-
-        service.trigger(conflict_pipeline)
-      end
-    end
   end
 
   describe "#cancel" do
diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index 5f28bc123f3..831678b949d 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/services/notification_service_spec.rb
@@ -1224,6 +1224,32 @@ describe NotificationService, :mailer do
       end
     end
 
+    describe '#merge_request_unmergeable' do
+      it "sends email to merge request author" do
+        notification.merge_request_unmergeable(merge_request)
+
+        should_email(merge_request.author)
+        expect(email_recipients.size).to eq(1)
+      end
+
+      describe 'when merge_when_pipeline_succeeds is true' do
+        before do
+          merge_request.update_attributes(
+            merge_when_pipeline_succeeds: true,
+            merge_user: create(:user)
+          )
+        end
+
+        it "sends email to merge request author and merge_user" do
+          notification.merge_request_unmergeable(merge_request)
+
+          should_email(merge_request.author)
+          should_email(merge_request.merge_user)
+          expect(email_recipients.size).to eq(2)
+        end
+      end
+    end
+
     describe '#closed_merge_request' do
       before do
         update_custom_notification(:close_merge_request, @u_guest_custom, resource: project)
diff --git a/spec/services/projects/open_issues_count_service_spec.rb b/spec/services/projects/open_issues_count_service_spec.rb
index f964f9972cd..06b470849b3 100644
--- a/spec/services/projects/open_issues_count_service_spec.rb
+++ b/spec/services/projects/open_issues_count_service_spec.rb
@@ -2,20 +2,53 @@ require 'spec_helper'
 
 describe Projects::OpenIssuesCountService do
   describe '#count' do
-    it 'returns the number of open issues' do
-      project = create(:project)
-      create(:issue, :opened, project: project)
+    let(:project) { create(:project) }
 
-      expect(described_class.new(project).count).to eq(1)
+    context 'when user is nil' do
+      it 'does not include confidential issues in the issue count' do
+        create(:issue, :opened, project: project)
+        create(:issue, :opened, confidential: true, project: project)
+
+        expect(described_class.new(project).count).to eq(1)
+      end
     end
 
-    it 'does not include confidential issues in the issue count' do
-      project = create(:project)
+    context 'when user is provided' do
+      let(:user) { create(:user) }
+
+      context 'when user can read confidential issues' do
+        before do
+          project.add_reporter(user)
+        end
+
+        it 'returns the right count with confidential issues' do
+          create(:issue, :opened, project: project)
+          create(:issue, :opened, confidential: true, project: project)
+
+          expect(described_class.new(project, user).count).to eq(2)
+        end
+
+        it 'uses total_open_issues_count cache key' do
+          expect(described_class.new(project, user).cache_key_name).to eq('total_open_issues_count')
+        end
+      end
+
+      context 'when user cannot read confidential issues' do
+        before do
+          project.add_guest(user)
+        end
+
+        it 'does not include confidential issues' do
+          create(:issue, :opened, project: project)
+          create(:issue, :opened, confidential: true, project: project)
 
-      create(:issue, :opened, project: project)
-      create(:issue, :opened, confidential: true, project: project)
+          expect(described_class.new(project, user).count).to eq(1)
+        end
 
-      expect(described_class.new(project).count).to eq(1)
+        it 'uses public_open_issues_count cache key' do
+          expect(described_class.new(project, user).cache_key_name).to eq('public_open_issues_count')
+        end
+      end
     end
   end
 end
diff --git a/spec/services/projects/update_remote_mirror_service_spec.rb b/spec/services/projects/update_remote_mirror_service_spec.rb
index be09afd9f36..723cb374c37 100644
--- a/spec/services/projects/update_remote_mirror_service_spec.rb
+++ b/spec/services/projects/update_remote_mirror_service_spec.rb
@@ -343,7 +343,11 @@ describe Projects::UpdateRemoteMirrorService do
       tag = repository.find_tag(name)
       target = tag.try(:target)
       target_commit = tag.try(:dereferenced_target)
-      tags << Gitlab::Git::Tag.new(repository.raw_repository, name, target, target_commit)
+      tags << Gitlab::Git::Tag.new(repository.raw_repository, {
+        name: name,
+        target: target,
+        target_commit: target_commit
+      })
     end
   end
 
diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb
index 562b89e6767..9a51c873b30 100644
--- a/spec/services/todo_service_spec.rb
+++ b/spec/services/todo_service_spec.rb
@@ -721,17 +721,18 @@ describe TodoService do
     end
 
     describe '#merge_request_build_failed' do
-      it 'creates a pending todo for the merge request author' do
-        service.merge_request_build_failed(mr_unassigned)
+      let(:merge_participants) { [mr_unassigned.author, admin] }
 
-        should_create_todo(user: author, target: mr_unassigned, action: Todo::BUILD_FAILED)
+      before do
+        allow(mr_unassigned).to receive(:merge_participants).and_return(merge_participants)
       end
 
-      it 'creates a pending todo for merge_user' do
-        mr_unassigned.update(merge_when_pipeline_succeeds: true, merge_user: admin)
+      it 'creates a pending todo for each merge_participant' do
         service.merge_request_build_failed(mr_unassigned)
 
-        should_create_todo(user: admin, author: admin, target: mr_unassigned, action: Todo::BUILD_FAILED)
+        merge_participants.each do |participant|
+          should_create_todo(user: participant, author: participant, target: mr_unassigned, action: Todo::BUILD_FAILED)
+        end
       end
     end
 
@@ -747,11 +748,19 @@ describe TodoService do
     end
 
     describe '#merge_request_became_unmergeable' do
-      it 'creates a pending todo for a merge_user' do
+      let(:merge_participants) { [admin, create(:user)] }
+
+      before do
+        allow(mr_unassigned).to receive(:merge_participants).and_return(merge_participants)
+      end
+
+      it 'creates a pending todo for each merge_participant' do
         mr_unassigned.update(merge_when_pipeline_succeeds: true, merge_user: admin)
         service.merge_request_became_unmergeable(mr_unassigned)
 
-        should_create_todo(user: admin, author: admin, target: mr_unassigned, action: Todo::UNMERGEABLE)
+        merge_participants.each do |participant|
+          should_create_todo(user: participant, author: participant, target: mr_unassigned, action: Todo::UNMERGEABLE)
+        end
       end
     end