diff --git a/app/assets/javascripts/environments/components/environment_item.js.es6 b/app/assets/javascripts/environments/components/environment_item.js.es6
index 6a3d996f69c..33a99231315 100644
--- a/app/assets/javascripts/environments/components/environment_item.js.es6
+++ b/app/assets/javascripts/environments/components/environment_item.js.es6
@@ -147,12 +147,12 @@ require('./environment_terminal_button');
},
/**
- * Returns the value of the `stoppable?` key provided in the response.
+ * Returns the value of the `stop_action?` key provided in the response.
*
* @returns {Boolean}
*/
- isStoppable() {
- return this.model['stoppable?'];
+ hasStopAction() {
+ return this.model['stop_action?'];
},
/**
@@ -508,7 +508,7 @@ require('./environment_terminal_button');
-
diff --git a/app/controllers/projects/environments_controller.rb b/app/controllers/projects/environments_controller.rb
index 87cc36253f1..77877cd262d 100644
--- a/app/controllers/projects/environments_controller.rb
+++ b/app/controllers/projects/environments_controller.rb
@@ -52,10 +52,15 @@ class Projects::EnvironmentsController < Projects::ApplicationController
end
def stop
- return render_404 unless @environment.stoppable?
+ return render_404 unless @environment.available?
- new_action = @environment.stop!(current_user)
- redirect_to polymorphic_path([project.namespace.becomes(Namespace), project, new_action])
+ stop_action = @environment.stop_with_action!(current_user)
+
+ if stop_action
+ redirect_to polymorphic_path([project.namespace.becomes(Namespace), project, stop_action])
+ else
+ redirect_to namespace_project_environment_path(project.namespace, project, @environment)
+ end
end
def terminal
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index 38a1946a71e..368bd27c91a 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -454,7 +454,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
deployment = environment.first_deployment_for(@merge_request.diff_head_commit)
stop_url =
- if environment.stoppable? && can?(current_user, :create_deployment, environment)
+ if environment.stop_action? && can?(current_user, :create_deployment, environment)
stop_namespace_project_environment_path(project.namespace, project, environment)
end
diff --git a/app/models/deployment.rb b/app/models/deployment.rb
index 91d85c2279b..afad001d50f 100644
--- a/app/models/deployment.rb
+++ b/app/models/deployment.rb
@@ -91,7 +91,7 @@ class Deployment < ActiveRecord::Base
@stop_action ||= manual_actions.find_by(name: on_stop)
end
- def stoppable?
+ def stop_action?
stop_action.present?
end
diff --git a/app/models/environment.rb b/app/models/environment.rb
index 577367f1eed..13c4630c565 100644
--- a/app/models/environment.rb
+++ b/app/models/environment.rb
@@ -110,15 +110,15 @@ class Environment < ActiveRecord::Base
external_url.gsub(/\A.*?:\/\//, '')
end
- def stoppable?
+ def stop_action?
available? && stop_action.present?
end
- def stop!(current_user)
- return unless stoppable?
+ def stop_with_action!(current_user)
+ return unless available?
- stop
- stop_action.play(current_user)
+ stop!
+ stop_action.play(current_user) if stop_action
end
def actions_for(environment)
diff --git a/app/serializers/environment_entity.rb b/app/serializers/environment_entity.rb
index 5d15eb8d3d3..4c017960628 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 :stoppable?
+ expose :stop_action?
expose :environment_path do |environment|
namespace_project_environment_path(
diff --git a/app/services/ci/stop_environments_service.rb b/app/services/ci/stop_environments_service.rb
index cf590459cb2..a51310c3967 100644
--- a/app/services/ci/stop_environments_service.rb
+++ b/app/services/ci/stop_environments_service.rb
@@ -8,10 +8,9 @@ module Ci
return unless has_ref?
environments.each do |environment|
- next unless environment.stoppable?
next unless can?(current_user, :create_deployment, project)
- environment.stop!(current_user)
+ environment.stop_with_action!(current_user)
end
end
diff --git a/app/views/projects/environments/_stop.html.haml b/app/views/projects/environments/_stop.html.haml
index 69848123c17..14a2d627203 100644
--- a/app/views/projects/environments/_stop.html.haml
+++ b/app/views/projects/environments/_stop.html.haml
@@ -1,4 +1,4 @@
-- if can?(current_user, :create_deployment, environment) && environment.stoppable?
+- if can?(current_user, :create_deployment, environment) && environment.stop_action?
.inline
= link_to stop_namespace_project_environment_path(@project.namespace, @project, environment), method: :post,
class: 'btn stop-env-link', rel: 'nofollow', data: { confirm: 'Are you sure you want to stop this environment?' } do
diff --git a/app/views/projects/environments/show.html.haml b/app/views/projects/environments/show.html.haml
index 7800d6ac382..7036325fff8 100644
--- a/app/views/projects/environments/show.html.haml
+++ b/app/views/projects/environments/show.html.haml
@@ -12,7 +12,7 @@
= render 'projects/environments/external_url', environment: @environment
- if can?(current_user, :update_environment, @environment)
= link_to 'Edit', edit_namespace_project_environment_path(@project.namespace, @project, @environment), class: 'btn'
- - if can?(current_user, :create_deployment, @environment) && @environment.stoppable?
+ - if can?(current_user, :create_deployment, @environment) && @environment.can_stop?
= link_to 'Stop', stop_namespace_project_environment_path(@project.namespace, @project, @environment), data: { confirm: 'Are you sure you want to stop this environment?' }, class: 'btn btn-danger', method: :post
.deployments-container
diff --git a/changelogs/unreleased/24147-delete-env-button.yml b/changelogs/unreleased/24147-delete-env-button.yml
new file mode 100644
index 00000000000..14e80cacbfb
--- /dev/null
+++ b/changelogs/unreleased/24147-delete-env-button.yml
@@ -0,0 +1,4 @@
+---
+title: Adds back ability to stop all environments
+merge_request: 7379
+author:
diff --git a/spec/features/environment_spec.rb b/spec/features/environment_spec.rb
index 511c95b758f..2f49e89b4e4 100644
--- a/spec/features/environment_spec.rb
+++ b/spec/features/environment_spec.rb
@@ -64,10 +64,6 @@ feature 'Environment', :feature do
expect(page).to have_link('Re-deploy')
end
- scenario 'does not show stop button' do
- expect(page).not_to have_link('Stop')
- end
-
scenario 'does not show terminal button' do
expect(page).not_to have_terminal_button
end
@@ -116,26 +112,42 @@ feature 'Environment', :feature do
end
end
- context 'with stop action' do
- given(:manual) { create(:ci_build, :manual, pipeline: pipeline, name: 'close_app') }
- given(:deployment) { create(:deployment, environment: environment, deployable: build, on_stop: 'close_app') }
+ context 'when environment is available' do
+ context 'with stop action' do
+ given(:manual) { create(:ci_build, :manual, pipeline: pipeline, name: 'close_app') }
+ given(:deployment) { create(:deployment, environment: environment, deployable: build, on_stop: 'close_app') }
- scenario 'does show stop button' do
- expect(page).to have_link('Stop')
- end
-
- scenario 'does allow to stop environment' do
- click_link('Stop')
-
- expect(page).to have_content('close_app')
- end
-
- context 'for reporter' do
- let(:role) { :reporter }
-
- scenario 'does not show stop button' do
- expect(page).not_to have_link('Stop')
+ scenario 'does show stop button' do
+ expect(page).to have_link('Stop')
end
+
+ scenario 'does allow to stop environment' do
+ click_link('Stop')
+
+ expect(page).to have_content('close_app')
+ end
+
+ context 'for reporter' do
+ let(:role) { :reporter }
+
+ scenario 'does not show stop button' do
+ expect(page).not_to have_link('Stop')
+ end
+ end
+ end
+
+ context 'without stop action' do
+ scenario 'does allow to stop environment' do
+ click_link('Stop')
+ end
+ end
+ end
+
+ context 'when environment is stopped' do
+ given(:environment) { create(:environment, project: project, state: :stopped) }
+
+ scenario 'does not show stop button' do
+ expect(page).not_to have_link('Stop')
end
end
end
diff --git a/spec/features/environments_spec.rb b/spec/features/environments_spec.rb
index c033b693213..78be7d36f47 100644
--- a/spec/features/environments_spec.rb
+++ b/spec/features/environments_spec.rb
@@ -52,6 +52,22 @@ feature 'Environments page', :feature, :js do
scenario 'does show no deployments' do
expect(page).to have_content('No deployments yet')
end
+
+ context 'for available environment' do
+ given(:environment) { create(:environment, project: project, state: :available) }
+
+ scenario 'does not shows stop button' do
+ expect(page).not_to have_selector('.stop-env-link')
+ end
+ end
+
+ context 'for stopped environment' do
+ given(:environment) { create(:environment, project: project, state: :stopped) }
+
+ scenario 'does not shows stop button' do
+ expect(page).not_to have_selector('.stop-env-link')
+ end
+ end
end
context 'with deployments' do
diff --git a/spec/javascripts/environments/environment_item_spec.js.es6 b/spec/javascripts/environments/environment_item_spec.js.es6
index 9858f346c83..d87cc0996c9 100644
--- a/spec/javascripts/environments/environment_item_spec.js.es6
+++ b/spec/javascripts/environments/environment_item_spec.js.es6
@@ -119,7 +119,7 @@ describe('Environment item', () => {
},
],
},
- 'stoppable?': true,
+ '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.es6 b/spec/javascripts/environments/mock_data.js.es6
index 58f6fb96afb..80e1cbc6f4d 100644
--- a/spec/javascripts/environments/mock_data.js.es6
+++ b/spec/javascripts/environments/mock_data.js.es6
@@ -50,7 +50,7 @@ const environmentsList = [
},
manual_actions: [],
},
- 'stoppable?': true,
+ 'stop_action?': true,
environment_path: '/root/ci-folders/environments/31',
created_at: '2016-11-07T11:11:16.525Z',
updated_at: '2016-11-07T11:11:16.525Z',
@@ -105,7 +105,7 @@ const environmentsList = [
},
manual_actions: [],
},
- 'stoppable?': false,
+ 'stop_action?': false,
environment_path: '/root/ci-folders/environments/31',
created_at: '2016-11-07T11:11:16.525Z',
updated_at: '2016-11-07T11:11:16.525Z',
@@ -116,7 +116,7 @@ const environmentsList = [
state: 'available',
environment_type: 'review',
last_deployment: null,
- 'stoppable?': true,
+ 'stop_action?': true,
environment_path: '/root/ci-folders/environments/31',
created_at: '2016-11-07T11:11:16.525Z',
updated_at: '2016-11-07T11:11:16.525Z',
@@ -127,7 +127,7 @@ const environmentsList = [
state: 'available',
environment_type: 'review',
last_deployment: null,
- 'stoppable?': true,
+ 'stop_action?': true,
environment_path: '/root/ci-folders/environments/31',
created_at: '2016-11-07T11:11:16.525Z',
updated_at: '2016-11-07T11:11:16.525Z',
@@ -143,7 +143,7 @@ const environment = {
external_url: 'http://production.',
environment_type: null,
last_deployment: {},
- 'stoppable?': false,
+ 'stop_action?': false,
environment_path: '/root/review-app/environments/4',
stop_path: '/root/review-app/environments/4/stop',
created_at: '2016-12-16T11:51:04.690Z',
diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb
index fc4435a2f64..080ff2f3f43 100644
--- a/spec/models/deployment_spec.rb
+++ b/spec/models/deployment_spec.rb
@@ -77,8 +77,8 @@ describe Deployment, models: true do
end
end
- describe '#stoppable?' do
- subject { deployment.stoppable? }
+ describe '#stop_action?' do
+ subject { deployment.stop_action? }
context 'when no other actions' do
let(:deployment) { build(:deployment) }
diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb
index eba392044bf..8b57d8600fe 100644
--- a/spec/models/environment_spec.rb
+++ b/spec/models/environment_spec.rb
@@ -112,8 +112,8 @@ describe Environment, models: true do
end
end
- describe '#stoppable?' do
- subject { environment.stoppable? }
+ describe '#stop_action?' do
+ subject { environment.stop_action? }
context 'when no other actions' do
it { is_expected.to be_falsey }
@@ -142,17 +142,39 @@ describe Environment, models: true do
end
end
- describe '#stop!' do
+ describe '#stop_with_action!' do
let(:user) { create(:user) }
- subject { environment.stop!(user) }
+ subject { environment.stop_with_action!(user) }
before do
- expect(environment).to receive(:stoppable?).and_call_original
+ expect(environment).to receive(:available?).and_call_original
end
context 'when no other actions' do
- it { is_expected.to be_nil }
+ context 'environment is available' do
+ before do
+ environment.update(state: :available)
+ end
+
+ it do
+ subject
+
+ expect(environment).to be_stopped
+ end
+ end
+
+ context 'environment is already stopped' do
+ before do
+ environment.update(state: :stopped)
+ end
+
+ it do
+ subject
+
+ expect(environment).to be_stopped
+ end
+ end
end
context 'when matching action is defined' do
diff --git a/spec/services/ci/stop_environments_service_spec.rb b/spec/services/ci/stop_environments_service_spec.rb
index 6f7d1a5d28d..560f83d94f7 100644
--- a/spec/services/ci/stop_environments_service_spec.rb
+++ b/spec/services/ci/stop_environments_service_spec.rb
@@ -42,10 +42,10 @@ describe Ci::StopEnvironmentsService, services: true do
end
end
- context 'when environment is not stoppable' do
+ context 'when environment is not stopped' do
before do
allow_any_instance_of(Environment)
- .to receive(:stoppable?).and_return(false)
+ .to receive(:state).and_return(:stopped)
end
it 'does not stop environment' do