Commit f7d900aa authored by Rémy Coutable's avatar Rémy Coutable

Merge branch '24147-delete-env-button' into 'master'

Makes stop button visible in environment page

Closes #24147

See merge request !7379
parents a658654c 0886b0fe
...@@ -147,12 +147,12 @@ require('./environment_terminal_button'); ...@@ -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} * @returns {Boolean}
*/ */
isStoppable() { hasStopAction() {
return this.model['stoppable?']; return this.model['stop_action?'];
}, },
/** /**
...@@ -508,7 +508,7 @@ require('./environment_terminal_button'); ...@@ -508,7 +508,7 @@ require('./environment_terminal_button');
</external-url-component> </external-url-component>
</div> </div>
<div v-if="isStoppable && canCreateDeployment" <div v-if="hasStopAction && canCreateDeployment"
class="inline js-stop-component-container"> class="inline js-stop-component-container">
<stop-component <stop-component
:stop-url="model.stop_path"> :stop-url="model.stop_path">
......
...@@ -52,10 +52,15 @@ class Projects::EnvironmentsController < Projects::ApplicationController ...@@ -52,10 +52,15 @@ class Projects::EnvironmentsController < Projects::ApplicationController
end end
def stop def stop
return render_404 unless @environment.stoppable? return render_404 unless @environment.available?
new_action = @environment.stop!(current_user) stop_action = @environment.stop_with_action!(current_user)
redirect_to polymorphic_path([project.namespace.becomes(Namespace), project, new_action])
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 end
def terminal def terminal
......
...@@ -454,7 +454,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController ...@@ -454,7 +454,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
deployment = environment.first_deployment_for(@merge_request.diff_head_commit) deployment = environment.first_deployment_for(@merge_request.diff_head_commit)
stop_url = 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) stop_namespace_project_environment_path(project.namespace, project, environment)
end end
......
...@@ -91,7 +91,7 @@ class Deployment < ActiveRecord::Base ...@@ -91,7 +91,7 @@ class Deployment < ActiveRecord::Base
@stop_action ||= manual_actions.find_by(name: on_stop) @stop_action ||= manual_actions.find_by(name: on_stop)
end end
def stoppable? def stop_action?
stop_action.present? stop_action.present?
end end
......
...@@ -110,15 +110,15 @@ class Environment < ActiveRecord::Base ...@@ -110,15 +110,15 @@ class Environment < ActiveRecord::Base
external_url.gsub(/\A.*?:\/\//, '') external_url.gsub(/\A.*?:\/\//, '')
end end
def stoppable? def stop_action?
available? && stop_action.present? available? && stop_action.present?
end end
def stop!(current_user) def stop_with_action!(current_user)
return unless stoppable? return unless available?
stop stop!
stop_action.play(current_user) stop_action.play(current_user) if stop_action
end end
def actions_for(environment) def actions_for(environment)
......
...@@ -7,7 +7,7 @@ class EnvironmentEntity < Grape::Entity ...@@ -7,7 +7,7 @@ class EnvironmentEntity < Grape::Entity
expose :external_url expose :external_url
expose :environment_type expose :environment_type
expose :last_deployment, using: DeploymentEntity expose :last_deployment, using: DeploymentEntity
expose :stoppable? expose :stop_action?
expose :environment_path do |environment| expose :environment_path do |environment|
namespace_project_environment_path( namespace_project_environment_path(
......
...@@ -8,10 +8,9 @@ module Ci ...@@ -8,10 +8,9 @@ module Ci
return unless has_ref? return unless has_ref?
environments.each do |environment| environments.each do |environment|
next unless environment.stoppable?
next unless can?(current_user, :create_deployment, project) next unless can?(current_user, :create_deployment, project)
environment.stop!(current_user) environment.stop_with_action!(current_user)
end end
end end
......
- if can?(current_user, :create_deployment, environment) && environment.stoppable? - if can?(current_user, :create_deployment, environment) && environment.stop_action?
.inline .inline
= link_to stop_namespace_project_environment_path(@project.namespace, @project, environment), method: :post, = 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 class: 'btn stop-env-link', rel: 'nofollow', data: { confirm: 'Are you sure you want to stop this environment?' } do
......
...@@ -12,7 +12,7 @@ ...@@ -12,7 +12,7 @@
= render 'projects/environments/external_url', environment: @environment = render 'projects/environments/external_url', environment: @environment
- if can?(current_user, :update_environment, @environment) - if can?(current_user, :update_environment, @environment)
= link_to 'Edit', edit_namespace_project_environment_path(@project.namespace, @project, @environment), class: 'btn' = 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 = 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 .deployments-container
......
---
title: Adds back ability to stop all environments
merge_request: 7379
author:
...@@ -64,10 +64,6 @@ feature 'Environment', :feature do ...@@ -64,10 +64,6 @@ feature 'Environment', :feature do
expect(page).to have_link('Re-deploy') expect(page).to have_link('Re-deploy')
end end
scenario 'does not show stop button' do
expect(page).not_to have_link('Stop')
end
scenario 'does not show terminal button' do scenario 'does not show terminal button' do
expect(page).not_to have_terminal_button expect(page).not_to have_terminal_button
end end
...@@ -116,6 +112,7 @@ feature 'Environment', :feature do ...@@ -116,6 +112,7 @@ feature 'Environment', :feature do
end end
end end
context 'when environment is available' do
context 'with stop action' do context 'with stop action' do
given(:manual) { create(:ci_build, :manual, pipeline: pipeline, name: 'close_app') } given(:manual) { create(:ci_build, :manual, pipeline: pipeline, name: 'close_app') }
given(:deployment) { create(:deployment, environment: environment, deployable: build, on_stop: 'close_app') } given(:deployment) { create(:deployment, environment: environment, deployable: build, on_stop: 'close_app') }
...@@ -138,6 +135,21 @@ feature 'Environment', :feature do ...@@ -138,6 +135,21 @@ feature 'Environment', :feature do
end end
end 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 end
end end
end end
......
...@@ -52,6 +52,22 @@ feature 'Environments page', :feature, :js do ...@@ -52,6 +52,22 @@ feature 'Environments page', :feature, :js do
scenario 'does show no deployments' do scenario 'does show no deployments' do
expect(page).to have_content('No deployments yet') expect(page).to have_content('No deployments yet')
end 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 end
context 'with deployments' do context 'with deployments' do
......
...@@ -119,7 +119,7 @@ describe('Environment item', () => { ...@@ -119,7 +119,7 @@ describe('Environment item', () => {
}, },
], ],
}, },
'stoppable?': true, 'stop_action?': true,
environment_path: 'root/ci-folders/environments/31', environment_path: 'root/ci-folders/environments/31',
created_at: '2016-11-07T11:11:16.525Z', created_at: '2016-11-07T11:11:16.525Z',
updated_at: '2016-11-10T15:55:58.778Z', updated_at: '2016-11-10T15:55:58.778Z',
......
...@@ -50,7 +50,7 @@ const environmentsList = [ ...@@ -50,7 +50,7 @@ const environmentsList = [
}, },
manual_actions: [], manual_actions: [],
}, },
'stoppable?': true, 'stop_action?': true,
environment_path: '/root/ci-folders/environments/31', environment_path: '/root/ci-folders/environments/31',
created_at: '2016-11-07T11:11:16.525Z', created_at: '2016-11-07T11:11:16.525Z',
updated_at: '2016-11-07T11:11:16.525Z', updated_at: '2016-11-07T11:11:16.525Z',
...@@ -105,7 +105,7 @@ const environmentsList = [ ...@@ -105,7 +105,7 @@ const environmentsList = [
}, },
manual_actions: [], manual_actions: [],
}, },
'stoppable?': false, 'stop_action?': false,
environment_path: '/root/ci-folders/environments/31', environment_path: '/root/ci-folders/environments/31',
created_at: '2016-11-07T11:11:16.525Z', created_at: '2016-11-07T11:11:16.525Z',
updated_at: '2016-11-07T11:11:16.525Z', updated_at: '2016-11-07T11:11:16.525Z',
...@@ -116,7 +116,7 @@ const environmentsList = [ ...@@ -116,7 +116,7 @@ const environmentsList = [
state: 'available', state: 'available',
environment_type: 'review', environment_type: 'review',
last_deployment: null, last_deployment: null,
'stoppable?': true, 'stop_action?': true,
environment_path: '/root/ci-folders/environments/31', environment_path: '/root/ci-folders/environments/31',
created_at: '2016-11-07T11:11:16.525Z', created_at: '2016-11-07T11:11:16.525Z',
updated_at: '2016-11-07T11:11:16.525Z', updated_at: '2016-11-07T11:11:16.525Z',
...@@ -127,7 +127,7 @@ const environmentsList = [ ...@@ -127,7 +127,7 @@ const environmentsList = [
state: 'available', state: 'available',
environment_type: 'review', environment_type: 'review',
last_deployment: null, last_deployment: null,
'stoppable?': true, 'stop_action?': true,
environment_path: '/root/ci-folders/environments/31', environment_path: '/root/ci-folders/environments/31',
created_at: '2016-11-07T11:11:16.525Z', created_at: '2016-11-07T11:11:16.525Z',
updated_at: '2016-11-07T11:11:16.525Z', updated_at: '2016-11-07T11:11:16.525Z',
...@@ -143,7 +143,7 @@ const environment = { ...@@ -143,7 +143,7 @@ const environment = {
external_url: 'http://production.', external_url: 'http://production.',
environment_type: null, environment_type: null,
last_deployment: {}, last_deployment: {},
'stoppable?': false, 'stop_action?': false,
environment_path: '/root/review-app/environments/4', environment_path: '/root/review-app/environments/4',
stop_path: '/root/review-app/environments/4/stop', stop_path: '/root/review-app/environments/4/stop',
created_at: '2016-12-16T11:51:04.690Z', created_at: '2016-12-16T11:51:04.690Z',
......
...@@ -77,8 +77,8 @@ describe Deployment, models: true do ...@@ -77,8 +77,8 @@ describe Deployment, models: true do
end end
end end
describe '#stoppable?' do describe '#stop_action?' do
subject { deployment.stoppable? } subject { deployment.stop_action? }
context 'when no other actions' do context 'when no other actions' do
let(:deployment) { build(:deployment) } let(:deployment) { build(:deployment) }
......
...@@ -112,8 +112,8 @@ describe Environment, models: true do ...@@ -112,8 +112,8 @@ describe Environment, models: true do
end end
end end
describe '#stoppable?' do describe '#stop_action?' do
subject { environment.stoppable? } subject { environment.stop_action? }
context 'when no other actions' do context 'when no other actions' do
it { is_expected.to be_falsey } it { is_expected.to be_falsey }
...@@ -142,17 +142,39 @@ describe Environment, models: true do ...@@ -142,17 +142,39 @@ describe Environment, models: true do
end end
end end
describe '#stop!' do describe '#stop_with_action!' do
let(:user) { create(:user) } let(:user) { create(:user) }
subject { environment.stop!(user) } subject { environment.stop_with_action!(user) }
before do before do
expect(environment).to receive(:stoppable?).and_call_original expect(environment).to receive(:available?).and_call_original
end end
context 'when no other actions' do 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 end
context 'when matching action is defined' do context 'when matching action is defined' do
......
...@@ -42,10 +42,10 @@ describe Ci::StopEnvironmentsService, services: true do ...@@ -42,10 +42,10 @@ describe Ci::StopEnvironmentsService, services: true do
end end
end end
context 'when environment is not stoppable' do context 'when environment is not stopped' do
before do before do
allow_any_instance_of(Environment) allow_any_instance_of(Environment)
.to receive(:stoppable?).and_return(false) .to receive(:state).and_return(:stopped)
end end
it 'does not stop environment' do it 'does not stop environment' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment