Merge branch 'fix-environment-status-in-merge-request-widget' into 'master'

Fix environment status in merge request widget

Closes #51120 and #25140

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

6 files changed, 211 insertions, 18 deletions

diff --git a/spec/features/merge_request/user_sees_deployment_widget_spec.rb b/spec/features/merge_request/user_sees_deployment_widget_spec.rb
index 0e439c8cb2d..74290c0fff9 100644
--- a/spec/features/merge_request/user_sees_deployment_widget_spec.rb
+++ b/spec/features/merge_request/user_sees_deployment_widget_spec.rb
@@ -1,7 +1,7 @@
 require 'rails_helper'
 
 describe 'Merge request > User sees deployment widget', :js do
-  describe 'when deployed to an environment' do
+  describe 'when merge request has associated environments' do
     let(:user) { create(:user) }
     let(:project) { create(:project, :repository) }
     let(:merge_request) { create(:merge_request, :merged, source_project: project) }
@@ -10,30 +10,74 @@ describe 'Merge request > User sees deployment widget', :js do
     let(:ref) { merge_request.target_branch }
     let(:sha) { project.commit(ref).id }
     let(:pipeline) { create(:ci_pipeline_without_jobs, sha: sha, project: project, ref: ref) }
-    let(:build)    { create(:ci_build, :success, pipeline: pipeline) }
-    let!(:deployment) { create(:deployment, :succeed, environment: environment, sha: sha, ref: ref, deployable: build) }
     let!(:manual) { }
 
     before do
       merge_request.update!(merge_commit_sha: sha)
       project.add_user(user, role)
       sign_in(user)
-      visit project_merge_request_path(project, merge_request)
-      wait_for_requests
     end
 
-    it 'displays that the environment is deployed' do
-      wait_for_requests
+    context 'when deployment succeeded' do
+      let(:build) { create(:ci_build, :success, pipeline: pipeline) }
+      let!(:deployment) { create(:deployment, :succeed, environment: environment, sha: sha, ref: ref, deployable: build) }
 
-      expect(page).to have_content("Deployed to #{environment.name}")
-      expect(find('.js-deploy-time')['data-original-title']).to eq(deployment.created_at.to_time.in_time_zone.to_s(:medium))
+      it 'displays that the environment is deployed' do
+        visit project_merge_request_path(project, merge_request)
+        wait_for_requests
+
+        expect(page).to have_content("Deployed to #{environment.name}")
+        expect(find('.js-deploy-time')['data-original-title']).to eq(deployment.created_at.to_time.in_time_zone.to_s(:medium))
+      end
+    end
+
+    context 'when deployment failed' do
+      let(:build) { create(:ci_build, :failed, pipeline: pipeline) }
+      let!(:deployment) { create(:deployment, :failed, environment: environment, sha: sha, ref: ref, deployable: build) }
+
+      it 'displays that the deployment failed' do
+        visit project_merge_request_path(project, merge_request)
+        wait_for_requests
+
+        expect(page).to have_content("Failed to deploy to #{environment.name}")
+        expect(page).not_to have_css('.js-deploy-time')
+      end
+    end
+
+    context 'when deployment running' do
+      let(:build) { create(:ci_build, :running, pipeline: pipeline) }
+      let!(:deployment) { create(:deployment, :running, environment: environment, sha: sha, ref: ref, deployable: build) }
+
+      it 'displays that the running deployment' do
+        visit project_merge_request_path(project, merge_request)
+        wait_for_requests
+
+        expect(page).to have_content("Deploying to #{environment.name}")
+        expect(page).not_to have_css('.js-deploy-time')
+      end
+    end
+
+    context 'when deployment will happen' do
+      let(:build) { create(:ci_build, :created, pipeline: pipeline) }
+      let!(:deployment) { create(:deployment, environment: environment, sha: sha, ref: ref, deployable: build) }
+
+      it 'displays that the environment name' do
+        visit project_merge_request_path(project, merge_request)
+        wait_for_requests
+
+        expect(page).to have_content("Deploying to #{environment.name}")
+        expect(page).not_to have_css('.js-deploy-time')
+      end
     end
 
     context 'with stop action' do
+      let(:build) { create(:ci_build, :success, pipeline: pipeline) }
+      let!(:deployment) { create(:deployment, :succeed, environment: environment, sha: sha, ref: ref, deployable: build) }
       let(:manual) { create(:ci_build, :manual, pipeline: pipeline, name: 'close_app') }
 
       before do
         deployment.update!(on_stop: manual.name)
+        visit project_merge_request_path(project, merge_request)
         wait_for_requests
       end
 
diff --git a/spec/features/projects/environments/environment_spec.rb b/spec/features/projects/environments/environment_spec.rb
index 056f4ee2e22..9772a7bacac 100644
--- a/spec/features/projects/environments/environment_spec.rb
+++ b/spec/features/projects/environments/environment_spec.rb
@@ -25,7 +25,7 @@ describe 'Environment' do
     end
 
     context 'without deployments' do
-      it 'does show no deployments' do
+      it 'does not show deployments' do
         expect(page).to have_content('You don\'t have any deployments right now.')
       end
     end
@@ -43,6 +43,45 @@ describe 'Environment' do
         end
       end
 
+      context 'when there is a successful deployment' do
+        let(:pipeline) { create(:ci_pipeline, project: project) }
+        let(:build) { create(:ci_build, :success, pipeline: pipeline) }
+
+        let(:deployment) do
+          create(:deployment, :success, environment: environment, deployable: build)
+        end
+
+        it 'does show deployments' do
+          expect(page).to have_link("#{build.name} (##{build.id})")
+        end
+      end
+
+      context 'when there is a running deployment' do
+        let(:pipeline) { create(:ci_pipeline, project: project) }
+        let(:build) { create(:ci_build, pipeline: pipeline) }
+
+        let(:deployment) do
+          create(:deployment, :running, environment: environment, deployable: build)
+        end
+
+        it 'does not show deployments' do
+          expect(page).to have_content('You don\'t have any deployments right now.')
+        end
+      end
+
+      context 'when there is a failed deployment' do
+        let(:pipeline) { create(:ci_pipeline, project: project) }
+        let(:build) { create(:ci_build, pipeline: pipeline) }
+
+        let(:deployment) do
+          create(:deployment, :failed, environment: environment, deployable: build)
+        end
+
+        it 'does not show deployments' do
+          expect(page).to have_content('You don\'t have any deployments right now.')
+        end
+      end
+
       context 'with related deployable present' do
         let(:pipeline) { create(:ci_pipeline, project: project) }
         let(:build) { create(:ci_build, pipeline: pipeline) }
diff --git a/spec/features/projects/environments/environments_spec.rb b/spec/features/projects/environments/environments_spec.rb
index d0ddf69d574..89954d35f91 100644
--- a/spec/features/projects/environments/environments_spec.rb
+++ b/spec/features/projects/environments/environments_spec.rb
@@ -128,7 +128,7 @@ describe 'Environments page', :js do
       end
     end
 
-    context 'when there are deployments' do
+    context 'when there are successful deployments' do
       let(:project) { create(:project, :repository) }
 
       let!(:deployment) do
@@ -328,6 +328,22 @@ describe 'Environments page', :js do
         end
       end
     end
+
+    context 'when there is a failed deployment' do
+      let(:project) { create(:project, :repository) }
+
+      let!(:deployment) do
+        create(:deployment, :failed,
+                            environment: environment,
+                            sha: project.commit.id)
+      end
+
+      it 'does not show deployments' do
+        visit_environments(project)
+
+        expect(page).to have_content('No deployments yet')
+      end
+    end
   end
 
   it 'does have a new environment button' do
diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb
index e121369f6ac..9a3f1f1c5a1 100644
--- a/spec/models/environment_spec.rb
+++ b/spec/models/environment_spec.rb
@@ -53,6 +53,25 @@ describe Environment do
     end
   end
 
+  describe '.with_deployment' do
+    subject { described_class.with_deployment(sha) }
+
+    let(:environment) { create(:environment) }
+    let(:sha) { RepoHelpers.sample_commit.id }
+
+    context 'when deployment has the specified sha' do
+      let!(:deployment) { create(:deployment, environment: environment, sha: sha) }
+
+      it { is_expected.to eq([environment]) }
+    end
+
+    context 'when deployment does not have the specified sha' do
+      let!(:deployment) { create(:deployment, environment: environment, sha: 'abc') }
+
+      it { is_expected.to be_empty }
+    end
+  end
+
   describe '#folder_name' do
     context 'when it is inside a folder' do
       subject(:environment) do
diff --git a/spec/models/environment_status_spec.rb b/spec/models/environment_status_spec.rb
index 52b98552184..90f7e4a4590 100644
--- a/spec/models/environment_status_spec.rb
+++ b/spec/models/environment_status_spec.rb
@@ -1,8 +1,10 @@
 require 'spec_helper'
 
 describe EnvironmentStatus do
+  include ProjectForksHelper
+
   let(:deployment)    { create(:deployment, :succeed, :review_app) }
-  let(:environment)   { deployment.environment}
+  let(:environment)   { deployment.environment }
   let(:project)       { deployment.project }
   let(:merge_request) { create(:merge_request, :deployed_review_app, deployment: deployment) }
   let(:sha)           { deployment.sha }
@@ -65,9 +67,9 @@ describe EnvironmentStatus do
     let(:admin)    { create(:admin) }
     let(:pipeline) { create(:ci_pipeline, sha: sha) }
 
-    it 'is based on merge_request.head_pipeline' do
-      expect(merge_request).to receive(:head_pipeline).and_return(pipeline)
-      expect(merge_request).not_to receive(:merge_pipeline)
+    it 'is based on merge_request.diff_head_sha' do
+      expect(merge_request).to receive(:diff_head_sha)
+      expect(merge_request).not_to receive(:merge_commit_sha)
 
       described_class.for_merge_request(merge_request, admin)
     end
@@ -81,11 +83,83 @@ describe EnvironmentStatus do
       merge_request.mark_as_merged!
     end
 
-    it 'is based on merge_request.merge_pipeline' do
-      expect(merge_request).to receive(:merge_pipeline).and_return(pipeline)
-      expect(merge_request).not_to receive(:head_pipeline)
+    it 'is based on merge_request.merge_commit_sha' do
+      expect(merge_request).to receive(:merge_commit_sha)
+      expect(merge_request).not_to receive(:diff_head_sha)
 
       described_class.after_merge_request(merge_request, admin)
     end
   end
+
+  describe '.build_environments_status' do
+    subject { described_class.send(:build_environments_status, merge_request, user, sha) }
+
+    let!(:build) { create(:ci_build, :deploy_to_production, pipeline: pipeline) }
+    let(:environment) { build.deployment.environment }
+    let(:user) { project.owner }
+
+    before do
+      build.deployment&.update!(sha: sha)
+    end
+
+    context 'when environment is created on a forked project' do
+      let(:project) { create(:project, :repository) }
+      let(:forked) { fork_project(project, user, repository: true) }
+      let(:sha) { forked.commit.sha }
+      let(:pipeline) { create(:ci_pipeline, sha: sha, project: forked) }
+
+      let(:merge_request) do
+        create(:merge_request,
+               source_project: forked,
+               target_project: project,
+               target_branch: 'master',
+               head_pipeline: pipeline)
+      end
+
+      it 'returns environment status' do
+        expect(subject.count).to eq(1)
+        expect(subject[0].environment).to eq(environment)
+        expect(subject[0].merge_request).to eq(merge_request)
+        expect(subject[0].sha).to eq(sha)
+      end
+    end
+
+    context 'when environment is created on a target project' do
+      let(:project) { create(:project, :repository) }
+      let(:sha) { project.commit.sha }
+      let(:pipeline) { create(:ci_pipeline, sha: sha, project: project) }
+
+      let(:merge_request) do
+        create(:merge_request,
+               source_project: project,
+               source_branch: 'feature',
+               target_project: project,
+               target_branch: 'master',
+               head_pipeline: pipeline)
+      end
+
+      it 'returns environment status' do
+        expect(subject.count).to eq(1)
+        expect(subject[0].environment).to eq(environment)
+        expect(subject[0].merge_request).to eq(merge_request)
+        expect(subject[0].sha).to eq(sha)
+      end
+
+      context 'when the build stops an environment' do
+        let!(:build) { create(:ci_build, :stop_review_app, pipeline: pipeline) }
+
+        it 'does not return environment status' do
+          expect(subject.count).to eq(0)
+        end
+      end
+
+      context 'when user does not have a permission to see the environment' do
+        let(:user) { create(:user) }
+
+        it 'does not return environment status' do
+          expect(subject.count).to eq(0)
+        end
+      end
+    end
+  end
 end
diff --git a/spec/serializers/environment_status_entity_spec.rb b/spec/serializers/environment_status_entity_spec.rb
index 962ec919092..52bd40ecb5e 100644
--- a/spec/serializers/environment_status_entity_spec.rb
+++ b/spec/serializers/environment_status_entity_spec.rb
@@ -15,6 +15,7 @@ describe EnvironmentStatusEntity do
   subject { entity.as_json }
 
   before do
+    deployment.update(sha: merge_request.diff_head_sha)
     allow(request).to receive(:current_user).and_return(user)
   end