Merge branch '11-10-stable-patch-4' into '11-10-stable'

Prepare 11.10.4 release

See merge request gitlab-org/gitlab-ce!27927

15 files changed, 272 insertions, 20 deletions

diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index d91dbe4bf49..49c56ecafd3 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -1097,6 +1097,16 @@ class MergeRequest < ApplicationRecord
     @environments[current_user]
   end
 
+  ##
+  # This method is for looking for active environments which created via pipelines for merge requests.
+  # Since deployments run on a merge request ref (e.g. `refs/merge-requests/:iid/head`),
+  # we cannot look up environments with source branch name.
+  def environments
+    return Environment.none unless actual_head_pipeline&.triggered_by_merge_request?
+
+    actual_head_pipeline.environments
+  end
+
   def state_human_name
     if merged?
       "Merged"
diff --git a/app/presenters/ci/pipeline_presenter.rb b/app/presenters/ci/pipeline_presenter.rb
index 1c1347c5a57..944895904fe 100644
--- a/app/presenters/ci/pipeline_presenter.rb
+++ b/app/presenters/ci/pipeline_presenter.rb
@@ -63,19 +63,11 @@ module Ci
     end
 
     def link_to_merge_request_source_branch
-      return unless merge_request_presenter
-
-      link_to(merge_request_presenter.source_branch,
-        merge_request_presenter.source_branch_commits_path,
-        class: 'ref-name')
+      merge_request_presenter&.source_branch_link
     end
 
     def link_to_merge_request_target_branch
-      return unless merge_request_presenter
-
-      link_to(merge_request_presenter.target_branch,
-        merge_request_presenter.target_branch_commits_path,
-        class: 'ref-name')
+      merge_request_presenter&.target_branch_link
     end
 
     private
diff --git a/app/presenters/merge_request_presenter.rb b/app/presenters/merge_request_presenter.rb
index 55103c0a95c..99ea7849f0b 100644
--- a/app/presenters/merge_request_presenter.rb
+++ b/app/presenters/merge_request_presenter.rb
@@ -213,6 +213,22 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated
     help_page_path('ci/merge_request_pipelines/index.md')
   end
 
+  def source_branch_link
+    if source_branch_exists?
+      link_to(source_branch, source_branch_commits_path, class: 'ref-name')
+    else
+      content_tag(:span, source_branch, class: 'ref-name')
+    end
+  end
+
+  def target_branch_link
+    if target_branch_exists?
+      link_to(target_branch, target_branch_commits_path, class: 'ref-name')
+    else
+      content_tag(:span, target_branch, class: 'ref-name')
+    end
+  end
+
   private
 
   def cached_can_be_reverted?
diff --git a/app/services/ci/stop_environments_service.rb b/app/services/ci/stop_environments_service.rb
index 973ae5ce5aa..d9a800791f2 100644
--- a/app/services/ci/stop_environments_service.rb
+++ b/app/services/ci/stop_environments_service.rb
@@ -9,12 +9,11 @@ module Ci
 
       return unless @ref.present?
 
-      environments.each do |environment|
-        next unless environment.stop_action_available?
-        next unless can?(current_user, :stop_environment, environment)
+      environments.each { |environment| stop(environment) }
+    end
 
-        environment.stop_with_action!(current_user)
-      end
+    def execute_for_merge_request(merge_request)
+      merge_request.environments.each { |environment| stop(environment) }
     end
 
     private
@@ -24,5 +23,12 @@ module Ci
         .new(project, current_user, ref: @ref, recently_updated: true)
         .execute
     end
+
+    def stop(environment)
+      return unless environment.stop_action_available?
+      return unless can?(current_user, :stop_environment, environment)
+
+      environment.stop_with_action!(current_user)
+    end
   end
 end
diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb
index 8a9e5ebb014..efb08b5ee1d 100644
--- a/app/services/merge_requests/base_service.rb
+++ b/app/services/merge_requests/base_service.rb
@@ -24,6 +24,11 @@ module MergeRequests
       end
     end
 
+    def cleanup_environments(merge_request)
+      Ci::StopEnvironmentsService.new(merge_request.source_project, current_user)
+                                 .execute_for_merge_request(merge_request)
+    end
+
     private
 
     def handle_wip_event(merge_request)
diff --git a/app/services/merge_requests/close_service.rb b/app/services/merge_requests/close_service.rb
index 04527bb9713..e77051bb1c9 100644
--- a/app/services/merge_requests/close_service.rb
+++ b/app/services/merge_requests/close_service.rb
@@ -17,6 +17,7 @@ module MergeRequests
         execute_hooks(merge_request, 'close')
         invalidate_cache_counts(merge_request, users: merge_request.assignees)
         merge_request.update_project_counter_caches
+        cleanup_environments(merge_request)
       end
 
       merge_request
diff --git a/app/services/merge_requests/post_merge_service.rb b/app/services/merge_requests/post_merge_service.rb
index f26e3bee06f..c13f7dd5088 100644
--- a/app/services/merge_requests/post_merge_service.rb
+++ b/app/services/merge_requests/post_merge_service.rb
@@ -18,6 +18,7 @@ module MergeRequests
       invalidate_cache_counts(merge_request, users: merge_request.assignees)
       merge_request.update_project_counter_caches
       delete_non_latest_diffs(merge_request)
+      cleanup_environments(merge_request)
     end
 
     private
diff --git a/changelogs/unreleased/fix-environment-on-stop-not-work.yml b/changelogs/unreleased/fix-environment-on-stop-not-work.yml
new file mode 100644
index 00000000000..72e58b26c4d
--- /dev/null
+++ b/changelogs/unreleased/fix-environment-on-stop-not-work.yml
@@ -0,0 +1,5 @@
+---
+title: "`on_stop` is not automatically triggered with pipelines for merge requests"
+merge_request: 27618
+author:
+type: fixed
diff --git a/changelogs/unreleased/fix-ref-text-of-mr-pipelines.yml b/changelogs/unreleased/fix-ref-text-of-mr-pipelines.yml
new file mode 100644
index 00000000000..8803f9b52a4
--- /dev/null
+++ b/changelogs/unreleased/fix-ref-text-of-mr-pipelines.yml
@@ -0,0 +1,6 @@
+---
+title: Fix pipelines for merge requests does not show pipeline page when source branch
+  is removed
+merge_request: 27803
+author:
+type: fixed
diff --git a/spec/features/projects/pipelines/pipeline_spec.rb b/spec/features/projects/pipelines/pipeline_spec.rb
index cf334e1e4da..4ec44cb05b3 100644
--- a/spec/features/projects/pipelines/pipeline_spec.rb
+++ b/spec/features/projects/pipelines/pipeline_spec.rb
@@ -331,11 +331,9 @@ describe 'Pipeline', :js do
         merge_request.all_pipelines.last
       end
 
-      before do
+      it 'shows the pipeline information' do
         visit_pipeline
-      end
 
-      it 'shows the pipeline information' do
         within '.pipeline-info' do
           expect(page).to have_content("#{pipeline.statuses.count} jobs " \
                                        "for !#{merge_request.iid} " \
@@ -347,6 +345,21 @@ describe 'Pipeline', :js do
         end
       end
 
+      context 'when source branch does not exist' do
+        before do
+          project.repository.rm_branch(user, merge_request.source_branch)
+        end
+
+        it 'does not link to the source branch commit path' do
+          visit_pipeline
+
+          within '.pipeline-info' do
+            expect(page).not_to have_link(merge_request.source_branch)
+            expect(page).to have_content(merge_request.source_branch)
+          end
+        end
+      end
+
       context 'when source project is a forked project' do
         let(:source_project) { fork_project(project, user, repository: true) }
 
@@ -386,11 +399,11 @@ describe 'Pipeline', :js do
 
       before do
         pipeline.update(user: user)
-
-        visit_pipeline
       end
 
       it 'shows the pipeline information' do
+        visit_pipeline
+
         within '.pipeline-info' do
           expect(page).to have_content("#{pipeline.statuses.count} jobs " \
                                        "for !#{merge_request.iid} " \
@@ -405,6 +418,21 @@ describe 'Pipeline', :js do
         end
       end
 
+      context 'when target branch does not exist' do
+        before do
+          project.repository.rm_branch(user, merge_request.target_branch)
+        end
+
+        it 'does not link to the target branch commit path' do
+          visit_pipeline
+
+          within '.pipeline-info' do
+            expect(page).not_to have_link(merge_request.target_branch)
+            expect(page).to have_content(merge_request.target_branch)
+          end
+        end
+      end
+
       context 'when source project is a forked project' do
         let(:source_project) { fork_project(project, user, repository: true) }
 
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index 6f34ef9c1bc..191fa688f28 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -2304,6 +2304,50 @@ describe MergeRequest do
     end
   end
 
+  describe "#environments" do
+    subject { merge_request.environments }
+
+    let(:merge_request) { create(:merge_request, source_branch: 'feature', target_branch: 'master') }
+    let(:project) { merge_request.project }
+
+    let(:pipeline) do
+      create(:ci_pipeline,
+        source: :merge_request_event,
+        merge_request: merge_request, project: project,
+        sha: merge_request.diff_head_sha,
+        merge_requests_as_head_pipeline: [merge_request])
+    end
+
+    let!(:job) { create(:ci_build, :start_review_app, pipeline: pipeline, project: project) }
+
+    it 'returns environments' do
+      is_expected.to eq(pipeline.environments)
+      expect(subject.count).to be(1)
+    end
+
+    context 'when pipeline is not associated with environments' do
+      let!(:job) { create(:ci_build, pipeline: pipeline, project: project) }
+
+      it 'returns empty array' do
+        is_expected.to be_empty
+      end
+    end
+
+    context 'when pipeline is not a pipeline for merge request' do
+      let(:pipeline) do
+        create(:ci_pipeline,
+          project: project,
+          ref: 'feature',
+          sha: merge_request.diff_head_sha,
+          merge_requests_as_head_pipeline: [merge_request])
+      end
+
+      it 'returns empty relation' do
+        is_expected.to be_empty
+      end
+    end
+  end
+
   describe "#reload_diff" do
     it 'calls MergeRequests::ReloadDiffsService#execute with correct params' do
       user = create(:user)
diff --git a/spec/presenters/merge_request_presenter_spec.rb b/spec/presenters/merge_request_presenter_spec.rb
index 4a0f91c4c7a..2d6b63cff85 100644
--- a/spec/presenters/merge_request_presenter_spec.rb
+++ b/spec/presenters/merge_request_presenter_spec.rb
@@ -439,6 +439,52 @@ describe MergeRequestPresenter do
     end
   end
 
+  describe '#source_branch_link' do
+    subject { presenter.source_branch_link }
+
+    let(:presenter) { described_class.new(resource, current_user: user) }
+
+    context 'when source branch exists' do
+      it 'returns link' do
+        allow(resource).to receive(:source_branch_exists?) { true }
+
+        is_expected
+          .to eq("<a class=\"ref-name\" href=\"#{presenter.source_branch_commits_path}\">#{presenter.source_branch}</a>")
+      end
+    end
+
+    context 'when source branch does not exist' do
+      it 'returns text' do
+        allow(resource).to receive(:source_branch_exists?) { false }
+
+        is_expected.to eq("<span class=\"ref-name\">#{presenter.source_branch}</span>")
+      end
+    end
+  end
+
+  describe '#target_branch_link' do
+    subject { presenter.target_branch_link }
+
+    let(:presenter) { described_class.new(resource, current_user: user) }
+
+    context 'when target branch exists' do
+      it 'returns link' do
+        allow(resource).to receive(:target_branch_exists?) { true }
+
+        is_expected
+          .to eq("<a class=\"ref-name\" href=\"#{presenter.target_branch_commits_path}\">#{presenter.target_branch}</a>")
+      end
+    end
+
+    context 'when target branch does not exist' do
+      it 'returns text' do
+        allow(resource).to receive(:target_branch_exists?) { false }
+
+        is_expected.to eq("<span class=\"ref-name\">#{presenter.target_branch}</span>")
+      end
+    end
+  end
+
   describe '#source_branch_with_namespace_link' do
     subject do
       described_class.new(resource, current_user: user).source_branch_with_namespace_link
diff --git a/spec/services/ci/stop_environments_service_spec.rb b/spec/services/ci/stop_environments_service_spec.rb
index cdd3d851f61..002a8f6da8e 100644
--- a/spec/services/ci/stop_environments_service_spec.rb
+++ b/spec/services/ci/stop_environments_service_spec.rb
@@ -103,6 +103,82 @@ describe Ci::StopEnvironmentsService do
     end
   end
 
+  describe '#execute_for_merge_request' do
+    subject { service.execute_for_merge_request(merge_request) }
+
+    let(:merge_request) { create(:merge_request, source_branch: 'feature', target_branch: 'master') }
+    let(:project) { merge_request.project }
+    let(:user) { create(:user) }
+
+    let(:pipeline) do
+      create(:ci_pipeline,
+        source: :merge_request_event,
+        merge_request: merge_request,
+        project: project,
+        sha: merge_request.diff_head_sha,
+        merge_requests_as_head_pipeline: [merge_request])
+    end
+
+    let!(:review_job) { create(:ci_build, :start_review_app, pipeline: pipeline, project: project) }
+    let!(:stop_review_job) { create(:ci_build, :stop_review_app, :manual, pipeline: pipeline, project: project) }
+
+    before do
+      review_job.deployment.success!
+    end
+
+    it 'has active environment at first' do
+      expect(pipeline.environments.first).to be_available
+    end
+
+    context 'when user is a developer' do
+      before do
+        project.add_developer(user)
+      end
+
+      it 'stops the active environment' do
+        subject
+
+        expect(pipeline.environments.first).to be_stopped
+      end
+    end
+
+    context 'when user is a reporter' do
+      before do
+        project.add_reporter(user)
+      end
+
+      it 'does not stop the active environment' do
+        subject
+
+        expect(pipeline.environments.first).to be_available
+      end
+    end
+
+    context 'when pipeline is not associated with environments' do
+      let!(:job) { create(:ci_build, pipeline: pipeline, project: project) }
+
+      it 'does not raise exception' do
+        expect { subject }.not_to raise_exception
+      end
+    end
+
+    context 'when pipeline is not a pipeline for merge request' do
+      let(:pipeline) do
+        create(:ci_pipeline,
+          project: project,
+          ref: 'feature',
+          sha: merge_request.diff_head_sha,
+          merge_requests_as_head_pipeline: [merge_request])
+      end
+
+      it 'does not stop the active environment' do
+        subject
+
+        expect(pipeline.environments.first).to be_available
+      end
+    end
+  end
+
   def expect_environment_stopped_on(branch)
     expect_any_instance_of(Environment)
       .to receive(:stop!)
diff --git a/spec/services/merge_requests/close_service_spec.rb b/spec/services/merge_requests/close_service_spec.rb
index 433ffbd97f0..93f5b092cab 100644
--- a/spec/services/merge_requests/close_service_spec.rb
+++ b/spec/services/merge_requests/close_service_spec.rb
@@ -72,6 +72,14 @@ describe MergeRequests::CloseService do
         .to change { project.open_merge_requests_count }.from(1).to(0)
     end
 
+    it 'clean up environments for the merge request' do
+      expect_next_instance_of(Ci::StopEnvironmentsService) do |service|
+        expect(service).to receive(:execute_for_merge_request).with(merge_request)
+      end
+
+      described_class.new(project, user).execute(merge_request)
+    end
+
     context 'current user is not authorized to close merge request' do
       before do
         perform_enqueued_jobs do
diff --git a/spec/services/merge_requests/post_merge_service_spec.rb b/spec/services/merge_requests/post_merge_service_spec.rb
index 5ad6f5528f9..0fbcd0efb86 100644
--- a/spec/services/merge_requests/post_merge_service_spec.rb
+++ b/spec/services/merge_requests/post_merge_service_spec.rb
@@ -60,5 +60,13 @@ describe MergeRequests::PostMergeService do
 
       expect(merge_request.reload).to be_merged
     end
+
+    it 'clean up environments for the merge request' do
+      expect_next_instance_of(Ci::StopEnvironmentsService) do |service|
+        expect(service).to receive(:execute_for_merge_request).with(merge_request)
+      end
+
+      described_class.new(project, user).execute(merge_request)
+    end
   end
 end