Merge branch 'fix/gb/rename-environments-stop-actions' into 'master'

Make it more explicit what environment stop actions are

Closes #48672

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

10 files changed, 27 insertions, 39 deletions

diff --git a/app/models/deployment.rb b/app/models/deployment.rb
index ac86e9e8de0..687246b47b2 100644
--- a/app/models/deployment.rb
+++ b/app/models/deployment.rb
@@ -92,10 +92,6 @@ class Deployment < ActiveRecord::Base
     @stop_action ||= manual_actions.find_by(name: on_stop)
   end
 
-  def stop_action?
-    stop_action.present?
-  end
-
   def formatted_deployment_time
     created_at.to_time.in_time_zone.to_s(:medium)
   end
diff --git a/app/models/environment.rb b/app/models/environment.rb
index 8d523dae324..4856d313318 100644
--- a/app/models/environment.rb
+++ b/app/models/environment.rb
@@ -117,7 +117,7 @@ class Environment < ActiveRecord::Base
     external_url.gsub(%r{\A.*?://}, '')
   end
 
-  def stop_action?
+  def stop_action_available?
     available? && stop_action.present?
   end
 
diff --git a/app/policies/environment_policy.rb b/app/policies/environment_policy.rb
index 978dc3a7c81..2d07311db72 100644
--- a/app/policies/environment_policy.rb
+++ b/app/policies/environment_policy.rb
@@ -2,11 +2,12 @@ class EnvironmentPolicy < BasePolicy
   delegate { @subject.project }
 
   condition(:stop_with_deployment_allowed) do
-    @subject.stop_action? && can?(:create_deployment) && can?(:update_build, @subject.stop_action)
+    @subject.stop_action_available? &&
+      can?(:create_deployment) && can?(:update_build, @subject.stop_action)
   end
 
   condition(:stop_with_update_allowed) do
-    !@subject.stop_action? && can?(:update_environment, @subject)
+    !@subject.stop_action_available? && can?(:update_environment, @subject)
   end
 
   rule { stop_with_deployment_allowed | stop_with_update_allowed }.enable :stop_environment
diff --git a/app/serializers/environment_entity.rb b/app/serializers/environment_entity.rb
index 0fc3f92b151..83558fc6659 100644
--- a/app/serializers/environment_entity.rb
+++ b/app/serializers/environment_entity.rb
@@ -7,7 +7,7 @@ class EnvironmentEntity < Grape::Entity
   expose :external_url
   expose :environment_type
   expose :last_deployment, using: DeploymentEntity
-  expose :stop_action?, as: :has_stop_action
+  expose :stop_action_available?, as: :has_stop_action
 
   expose :metrics_path, if: -> (environment, _) { environment.has_metrics? } do |environment|
     metrics_project_environment_path(environment.project, environment)
diff --git a/app/services/ci/stop_environments_service.rb b/app/services/ci/stop_environments_service.rb
index 43c9a065fcf..439746e82bd 100644
--- a/app/services/ci/stop_environments_service.rb
+++ b/app/services/ci/stop_environments_service.rb
@@ -8,7 +8,7 @@ module Ci
       return unless @ref.present?
 
       environments.each do |environment|
-        next unless environment.stop_action?
+        next unless environment.stop_action_available?
         next unless can?(current_user, :stop_environment, environment)
 
         environment.stop_with_action!(current_user)
diff --git a/app/views/projects/environments/show.html.haml b/app/views/projects/environments/show.html.haml
index a33bc9d4ce6..c7890b37381 100644
--- a/app/views/projects/environments/show.html.haml
+++ b/app/views/projects/environments/show.html.haml
@@ -16,7 +16,7 @@
               ?
           .modal-body
             %p= s_('Environments|Are you sure you want to stop this environment?')
-            - unless @environment.stop_action?
+            - unless @environment.stop_action_available?
               .warning_message
                 %p= s_('Environments|Note that this action will stop the environment, but it will %{emphasis_start}not%{emphasis_end} have an effect on any existing deployment due to no “stop environment action” being defined in the %{ci_config_link_start}.gitlab-ci.yml%{ci_config_link_end} file.').html_safe % { emphasis_start: '<strong>'.html_safe,
                   emphasis_end: '</strong>'.html_safe,
diff --git a/spec/javascripts/environments/environment_item_spec.js b/spec/javascripts/environments/environment_item_spec.js
index 7a34126eef7..0b933dda431 100644
--- a/spec/javascripts/environments/environment_item_spec.js
+++ b/spec/javascripts/environments/environment_item_spec.js
@@ -104,7 +104,7 @@ describe('Environment item', () => {
             },
           ],
         },
-        'stop_action?': true,
+        has_stop_action: true,
         environment_path: 'root/ci-folders/environments/31',
         created_at: '2016-11-07T11:11:16.525Z',
         updated_at: '2016-11-10T15:55:58.778Z',
diff --git a/spec/javascripts/environments/mock_data.js b/spec/javascripts/environments/mock_data.js
index 8a1e26935d9..7bb57f938b8 100644
--- a/spec/javascripts/environments/mock_data.js
+++ b/spec/javascripts/environments/mock_data.js
@@ -7,7 +7,7 @@ export const environmentsList = [
     external_url: null,
     environment_type: null,
     last_deployment: null,
-    'stop_action?': false,
+    has_stop_action: false,
     environment_path: '/root/review-app/environments/7',
     stop_path: '/root/review-app/environments/7/stop',
     created_at: '2017-01-31T10:53:46.894Z',
@@ -22,7 +22,7 @@ export const environmentsList = [
     external_url: null,
     environment_type: 'build',
     last_deployment: null,
-    'stop_action?': false,
+    has_stop_action: false,
     environment_path: '/root/review-app/environments/12',
     stop_path: '/root/review-app/environments/12/stop',
     created_at: '2017-02-01T19:42:18.400Z',
@@ -41,7 +41,7 @@ export const serverData = [
       external_url: null,
       environment_type: null,
       last_deployment: null,
-      'stop_action?': false,
+      has_stop_action: false,
       environment_path: '/root/review-app/environments/7',
       stop_path: '/root/review-app/environments/7/stop',
       created_at: '2017-01-31T10:53:46.894Z',
@@ -58,7 +58,7 @@ export const serverData = [
       external_url: null,
       environment_type: 'build',
       last_deployment: null,
-      'stop_action?': false,
+      has_stop_action: false,
       environment_path: '/root/review-app/environments/12',
       stop_path: '/root/review-app/environments/12/stop',
       created_at: '2017-02-01T19:42:18.400Z',
@@ -77,7 +77,7 @@ export const environment = {
     external_url: null,
     environment_type: null,
     last_deployment: null,
-    'stop_action?': false,
+    has_stop_action: false,
     environment_path: '/root/review-app/environments/7',
     stop_path: '/root/review-app/environments/7/stop',
     created_at: '2017-01-31T10:53:46.894Z',
@@ -95,7 +95,7 @@ export const folder = {
   external_url: null,
   environment_type: 'build',
   last_deployment: null,
-  'stop_action?': false,
+  has_stop_action: false,
   environment_path: '/root/review-app/environments/12',
   stop_path: '/root/review-app/environments/12/stop',
   created_at: '2017-02-01T19:42:18.400Z',
diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb
index e01906f4b6c..b335e0fbeb3 100644
--- a/spec/models/deployment_spec.rb
+++ b/spec/models/deployment_spec.rb
@@ -157,22 +157,4 @@ describe Deployment do
       end
     end
   end
-
-  describe '#stop_action?' do
-    subject { deployment.stop_action? }
-
-    context 'when no other actions' do
-      let(:deployment) { build(:deployment) }
-
-      it { is_expected.to be_falsey }
-    end
-
-    context 'when matching action is defined' do
-      let(:build) { create(:ci_build) }
-      let(:deployment) { FactoryBot.build(:deployment, deployable: build, on_stop: 'close_app') }
-      let!(:close_action) { create(:ci_build, :manual, pipeline: build.pipeline, name: 'close_app') }
-
-      it { is_expected.to be_truthy }
-    end
-  end
 end
diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb
index 4bded9efe91..c65e0b81451 100644
--- a/spec/models/environment_spec.rb
+++ b/spec/models/environment_spec.rb
@@ -170,8 +170,8 @@ describe Environment do
     end
   end
 
-  describe '#stop_action?' do
-    subject { environment.stop_action? }
+  describe '#stop_action_available?' do
+    subject { environment.stop_action_available? }
 
     context 'when no other actions' do
       it { is_expected.to be_falsey }
@@ -179,8 +179,17 @@ describe Environment do
 
     context 'when matching action is defined' do
       let(:build) { create(:ci_build) }
-      let!(:deployment) { create(:deployment, environment: environment, deployable: build, on_stop: 'close_app') }
-      let!(:close_action) { create(:ci_build, :manual, pipeline: build.pipeline, name: 'close_app') }
+
+      let!(:deployment) do
+        create(:deployment, environment: environment,
+                            deployable: build,
+                            on_stop: 'close_app')
+      end
+
+      let!(:close_action) do
+        create(:ci_build, :manual, pipeline: build.pipeline,
+                                   name: 'close_app')
+      end
 
       context 'when environment is available' do
         before do