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: ''.html_safe, emphasis_end: ''.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