From 26820d1f5d950a3430737b350b664bfdf7622ba6 Mon Sep 17 00:00:00 2001
From: Kamil Trzcinski <ayufan@ayufan.eu>
Date: Tue, 13 Jun 2017 10:17:11 +0200
Subject: [PATCH] Do not show manual actions that cannot be run

---
 app/models/ci/build.rb                           |  4 ++--
 .../disable-blocked-manual-actions.yml           |  4 ++++
 spec/models/ci/build_spec.rb                     | 16 ++++++++++++++++
 spec/models/deployment_spec.rb                   |  4 ++--
 spec/models/environment_spec.rb                  |  2 +-
 5 files changed, 25 insertions(+), 5 deletions(-)
 create mode 100644 changelogs/unreleased/disable-blocked-manual-actions.yml

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index cec1ca89a6a..58758f7ca8a 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -33,7 +33,7 @@ module Ci
     scope :with_artifacts_not_expired, ->() { with_artifacts.where('artifacts_expire_at IS NULL OR artifacts_expire_at > ?', Time.now) }
     scope :with_expired_artifacts, ->() { with_artifacts.where('artifacts_expire_at < ?', Time.now) }
     scope :last_month, ->() { where('created_at > ?', Date.today - 1.month) }
-    scope :manual_actions, ->() { where(when: :manual).relevant }
+    scope :manual_actions, ->() { where(when: :manual, status: COMPLETED_STATUSES + [:manual]) }
 
     mount_uploader :artifacts_file, ArtifactUploader
     mount_uploader :artifacts_metadata, ArtifactUploader
@@ -109,7 +109,7 @@ module Ci
     end
 
     def playable?
-      action? && manual?
+      action? && (manual? || complete?)
     end
 
     def action?
diff --git a/changelogs/unreleased/disable-blocked-manual-actions.yml b/changelogs/unreleased/disable-blocked-manual-actions.yml
new file mode 100644
index 00000000000..a640f61a7dd
--- /dev/null
+++ b/changelogs/unreleased/disable-blocked-manual-actions.yml
@@ -0,0 +1,4 @@
+---
+title: disable blocked manual actions
+merge_request:
+author:
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index b0716e04d3d..b06e77c34f6 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -21,6 +21,18 @@ describe Ci::Build, :models do
   it { is_expected.to respond_to(:has_trace?) }
   it { is_expected.to respond_to(:trace) }
 
+  describe '.manual_actions' do
+    let!(:manual_but_created) { create(:ci_build, :manual, status: :created, pipeline: pipeline) }
+    let!(:manual_but_succeeded) { create(:ci_build, :manual, status: :success, pipeline: pipeline) }
+    let!(:manual_action) { create(:ci_build, :manual, pipeline: pipeline) }
+
+    subject { described_class.manual_actions }
+
+    it { is_expected.to include(manual_action) }
+    it { is_expected.to include(manual_but_succeeded) }
+    it { is_expected.not_to include(manual_but_created) }
+  end
+
   describe '#actionize' do
     context 'when build is a created' do
       before do
@@ -926,6 +938,10 @@ describe Ci::Build, :models do
     context 'when other build is retried' do
       let!(:retried_build) { Ci::Build.retry(other_build, user) }
 
+      before do
+        retried_build.success
+      end
+
       it 'returns a retried build' do
         is_expected.to contain_exactly(retried_build)
       end
diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb
index 6f0d2db23c7..aad215d5f41 100644
--- a/spec/models/deployment_spec.rb
+++ b/spec/models/deployment_spec.rb
@@ -102,7 +102,7 @@ describe Deployment, models: true do
     end
 
     context 'with other actions' do
-      let!(:close_action) { create(:ci_build, pipeline: build.pipeline, name: 'close_app', when: :manual) }
+      let!(:close_action) { create(:ci_build, :manual, pipeline: build.pipeline, name: 'close_app') }
 
       context 'when matching action is defined' do
         let(:deployment) { FactoryGirl.build(:deployment, deployable: build, on_stop: 'close_other_app') }
@@ -130,7 +130,7 @@ describe Deployment, models: true do
     context 'when matching action is defined' do
       let(:build) { create(:ci_build) }
       let(:deployment) { FactoryGirl.build(:deployment, deployable: build, on_stop: 'close_app') }
-      let!(:close_action) { create(:ci_build, pipeline: build.pipeline, name: 'close_app', when: :manual) }
+      let!(:close_action) { create(:ci_build, :manual, pipeline: build.pipeline, name: 'close_app') }
 
       it { is_expected.to be_truthy }
     end
diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb
index fe69c8e351d..f8123cb518e 100644
--- a/spec/models/environment_spec.rb
+++ b/spec/models/environment_spec.rb
@@ -170,7 +170,7 @@ describe Environment, models: true 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, pipeline: build.pipeline, name: 'close_app', when: :manual) }
+      let!(:close_action) { create(:ci_build, :manual, pipeline: build.pipeline, name: 'close_app') }
 
       context 'when environment is available' do
         before do