diff options
-rw-r--r-- | app/controllers/projects/environments_controller.rb | 3 | ||||
-rw-r--r-- | app/controllers/projects/merge_requests_controller.rb | 2 | ||||
-rw-r--r-- | app/models/environment.rb | 8 | ||||
-rw-r--r-- | app/serializers/environment_entity.rb | 2 | ||||
-rw-r--r-- | app/services/ci/stop_environments_service.rb | 3 | ||||
-rw-r--r-- | app/views/projects/environments/_stop.html.haml | 2 | ||||
-rw-r--r-- | app/views/projects/environments/show.html.haml | 2 | ||||
-rw-r--r-- | spec/features/environment_spec.rb | 48 | ||||
-rw-r--r-- | spec/models/environment_spec.rb | 8 | ||||
-rw-r--r-- | spec/services/ci/stop_environments_service_spec.rb | 4 |
10 files changed, 47 insertions, 35 deletions
diff --git a/app/controllers/projects/environments_controller.rb b/app/controllers/projects/environments_controller.rb index a203efc62b8..77877cd262d 100644 --- a/app/controllers/projects/environments_controller.rb +++ b/app/controllers/projects/environments_controller.rb @@ -54,7 +54,8 @@ class Projects::EnvironmentsController < Projects::ApplicationController def stop return render_404 unless @environment.available? - stop_action = @environment.run_stop!(current_user) + stop_action = @environment.stop_with_action!(current_user) + if stop_action redirect_to polymorphic_path([project.namespace.becomes(Namespace), project, stop_action]) else diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 7b6a9427098..6eb542e4bd8 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -451,7 +451,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController deployment = environment.first_deployment_for(@merge_request.diff_head_commit) stop_url = - if environment.can_run_stop_action? && can?(current_user, :create_deployment, environment) + if environment.stoppable? && can?(current_user, :create_deployment, environment) stop_namespace_project_environment_path(project.namespace, project, environment) end diff --git a/app/models/environment.rb b/app/models/environment.rb index 066f57292b8..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 can_run_stop_action? + def stop_action? available? && stop_action.present? end - def run_stop!(current_user) + 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 7279de59aa8..5d15eb8d3d3 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 :can_run_stop_action? + expose :stoppable? 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 b78ad7ee2c7..69848123c17 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.can_run_stop_action? +- if can?(current_user, :create_deployment, environment) && environment.stoppable? .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 e5cdf8fc603..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.available? + - 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/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,27 +112,43 @@ 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 show stop button' do + expect(page).to have_link('Stop') + end - scenario 'does allow to stop environment' do - click_link('Stop') + scenario 'does allow to stop environment' do + click_link('Stop') - expect(page).to have_content('close_app') - end + expect(page).to have_content('close_app') + end - context 'for reporter' do - let(:role) { :reporter } + context 'for reporter' do + let(:role) { :reporter } - scenario 'does not show stop button' do - expect(page).not_to have_link('Stop') + 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 end diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 1ac5e0413ee..ffce6847ff3 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 '#can_run_stop_action?' do - subject { environment.can_run_stop_action? } + describe '#stoppable?' do + subject { environment.stoppable? } context 'when no other actions' do it { is_expected.to be_falsey } @@ -142,10 +142,10 @@ describe Environment, models: true do end end - describe '#run_stop!' do + describe '#stop_with_action!' do let(:user) { create(:user) } - subject { environment.run_stop!(user) } + subject { environment.stop_with_action!(user) } before do expect(environment).to receive(:available?).and_call_original 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 |