From 9fe1b32f9773fc6bd041e80be290f3bd1ce1bfb8 Mon Sep 17 00:00:00 2001
From: Allen Cook <acook@gitlab.com>
Date: Fri, 20 May 2022 14:48:16 +0000
Subject: [PATCH] Resolve "Environments View - Deploy button tied to wrong job"
---
app/models/ci/build.rb | 8 ++++
app/models/deployment.rb | 20 ++++++++-
app/models/environment.rb | 6 +--
app/serializers/environment_serializer.rb | 4 +-
.../deployment_environment_manual_actions.yml | 8 ++++
.../environments/environments_spec.rb | 2 +-
.../user_sees_deployment_widget_spec.rb | 5 ++-
.../projects/environments/environment_spec.rb | 5 ++-
spec/lib/gitlab/slash_commands/deploy_spec.rb | 2 +-
spec/models/deployment_spec.rb | 44 ++++++++++++++-----
spec/serializers/deployment_entity_spec.rb | 12 +++--
.../environment_serializer_spec.rb | 7 ++-
12 files changed, 94 insertions(+), 29 deletions(-)
create mode 100644 config/feature_flags/development/deployment_environment_manual_actions.yml
diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index fd264b71b9251..261230189474e 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -425,10 +425,18 @@ def other_manual_actions
pipeline.manual_actions.reject { |action| action.name == self.name }
end
+ def environment_manual_actions
+ pipeline.manual_actions.filter { |action| action.expanded_environment_name == self.expanded_environment_name }
+ end
+
def other_scheduled_actions
pipeline.scheduled_actions.reject { |action| action.name == self.name }
end
+ def environment_scheduled_actions
+ pipeline.scheduled_actions.filter { |action| action.expanded_environment_name == self.expanded_environment_name }
+ end
+
def pages_generator?
Gitlab.config.pages.enabled &&
self.name == 'pages'
diff --git a/app/models/deployment.rb b/app/models/deployment.rb
index 3a96f4e386771..863e8bcfaa8e4 100644
--- a/app/models/deployment.rb
+++ b/app/models/deployment.rb
@@ -256,11 +256,27 @@ def invalidate_cache
end
def manual_actions
- @manual_actions ||= deployable.try(:other_manual_actions)
+ Feature.enabled?(:deployment_environment_manual_actions) ? environment_manual_actions : other_manual_actions
+ end
+
+ def other_manual_actions
+ @other_manual_actions ||= deployable.try(:other_manual_actions)
+ end
+
+ def environment_manual_actions
+ @environment_manual_actions ||= deployable.try(:environment_manual_actions)
end
def scheduled_actions
- @scheduled_actions ||= deployable.try(:other_scheduled_actions)
+ Feature.enabled?(:deployment_environment_manual_actions) ? environment_scheduled_actions : other_scheduled_actions
+ end
+
+ def environment_scheduled_actions
+ @environment_scheduled_actions ||= deployable.try(:environment_scheduled_actions)
+ end
+
+ def other_scheduled_actions
+ @other_scheduled_actions ||= deployable.try(:other_scheduled_actions)
end
def playable_build
diff --git a/app/models/environment.rb b/app/models/environment.rb
index 865f5c68af172..919998abaf229 100644
--- a/app/models/environment.rb
+++ b/app/models/environment.rb
@@ -59,7 +59,7 @@ class Environment < ApplicationRecord
allow_nil: true,
addressable_url: true
- delegate :manual_actions, to: :last_deployment, allow_nil: true
+ delegate :manual_actions, :other_manual_actions, to: :last_deployment, allow_nil: true
delegate :auto_rollback_enabled?, to: :project
scope :available, -> { with_state(:available) }
@@ -325,9 +325,9 @@ def reset_auto_stop
end
def actions_for(environment)
- return [] unless manual_actions
+ return [] unless other_manual_actions
- manual_actions.select do |action|
+ other_manual_actions.select do |action|
action.expanded_environment_name == environment
end
end
diff --git a/app/serializers/environment_serializer.rb b/app/serializers/environment_serializer.rb
index b13140efea730..3cb60a59f513b 100644
--- a/app/serializers/environment_serializer.rb
+++ b/app/serializers/environment_serializer.rb
@@ -89,8 +89,8 @@ def deployment_associations
user: [],
metadata: [],
pipeline: {
- manual_actions: [],
- scheduled_actions: []
+ manual_actions: [:metadata],
+ scheduled_actions: [:metadata]
},
project: project_associations
}
diff --git a/config/feature_flags/development/deployment_environment_manual_actions.yml b/config/feature_flags/development/deployment_environment_manual_actions.yml
new file mode 100644
index 0000000000000..47ab1b257f984
--- /dev/null
+++ b/config/feature_flags/development/deployment_environment_manual_actions.yml
@@ -0,0 +1,8 @@
+---
+name: deployment_environment_manual_actions
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/87138
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/362824
+milestone: '15.1'
+type: development
+group: group::release
+default_enabled: false
diff --git a/ee/spec/features/projects/environments/environments_spec.rb b/ee/spec/features/projects/environments/environments_spec.rb
index 2296a645fa81a..5e267bae09035 100644
--- a/ee/spec/features/projects/environments/environments_spec.rb
+++ b/ee/spec/features/projects/environments/environments_spec.rb
@@ -33,7 +33,7 @@ def actions_button_selector
context 'when environment has manual actions' do
let!(:pipeline) { create(:ci_pipeline, project: project) }
- let!(:build) { create(:ci_build, pipeline: pipeline) }
+ let!(:build) { create(:ci_build, pipeline: pipeline, environment: 'production') }
let!(:deployment) do
create(:deployment,
diff --git a/spec/features/merge_request/user_sees_deployment_widget_spec.rb b/spec/features/merge_request/user_sees_deployment_widget_spec.rb
index 01cc58777ba75..9f1f0d97b0954 100644
--- a/spec/features/merge_request/user_sees_deployment_widget_spec.rb
+++ b/spec/features/merge_request/user_sees_deployment_widget_spec.rb
@@ -109,7 +109,10 @@
end
context 'with stop action' do
- let(:manual) { create(:ci_build, :manual, pipeline: pipeline, name: 'close_app') }
+ let(:manual) do
+ create(:ci_build, :manual, pipeline: pipeline,
+ name: 'close_app', environment: environment.name)
+ end
before do
stub_feature_flags(bootstrap_confirmation_modals: false)
diff --git a/spec/features/projects/environments/environment_spec.rb b/spec/features/projects/environments/environment_spec.rb
index bfd54b9c6daea..124fc1e20866d 100644
--- a/spec/features/projects/environments/environment_spec.rb
+++ b/spec/features/projects/environments/environment_spec.rb
@@ -157,7 +157,7 @@ def auto_stop_button_selector
context 'with related deployable present' do
let(:pipeline) { create(:ci_pipeline, project: project) }
- let(:build) { create(:ci_build, pipeline: pipeline) }
+ let(:build) { create(:ci_build, pipeline: pipeline, environment: environment.name) }
let(:deployment) do
create(:deployment, :success, environment: environment, deployable: build)
@@ -263,7 +263,8 @@ def auto_stop_button_selector
context 'with stop action' do
let(:action) do
create(:ci_build, :manual, pipeline: pipeline,
- name: 'close_app')
+ name: 'close_app',
+ environment: environment.name)
end
let(:deployment) do
diff --git a/spec/lib/gitlab/slash_commands/deploy_spec.rb b/spec/lib/gitlab/slash_commands/deploy_spec.rb
index 71fca1e1fc80b..5167523ff58e4 100644
--- a/spec/lib/gitlab/slash_commands/deploy_spec.rb
+++ b/spec/lib/gitlab/slash_commands/deploy_spec.rb
@@ -32,7 +32,7 @@
context 'with environment' do
let!(:staging) { create(:environment, name: 'staging', project: project) }
let!(:pipeline) { create(:ci_pipeline, project: project) }
- let!(:build) { create(:ci_build, pipeline: pipeline) }
+ let!(:build) { create(:ci_build, pipeline: pipeline, environment: 'production') }
let!(:deployment) { create(:deployment, :success, environment: staging, deployable: build) }
context 'without actions' do
diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb
index 8096604b96585..ce6b59a5f957a 100644
--- a/spec/models/deployment_spec.rb
+++ b/spec/models/deployment_spec.rb
@@ -17,7 +17,6 @@
it { is_expected.to delegate_method(:name).to(:environment).with_prefix }
it { is_expected.to delegate_method(:commit).to(:project) }
it { is_expected.to delegate_method(:commit_title).to(:commit).as(:try) }
- it { is_expected.to delegate_method(:manual_actions).to(:deployable).as(:try) }
it { is_expected.to delegate_method(:kubernetes_namespace).to(:deployment_cluster).as(:kubernetes_namespace) }
it { is_expected.to validate_presence_of(:ref) }
@@ -25,20 +24,43 @@
it_behaves_like 'having unique enum values'
+ describe '#manual_actions' do
+ let(:deployment) { create(:deployment) }
+
+ it 'delegates to environment_manual_actions when deployment_environment_manual_actions ff is enabled' do
+ stub_feature_flags(deployment_environment_manual_actions: true)
+
+ expect(deployment.deployable).to receive(:environment_manual_actions).and_call_original
+
+ deployment.manual_actions
+ end
+
+ it 'delegates to other_manual_actions when deployment_environment_manual_actions ff is disabled' do
+ stub_feature_flags(deployment_environment_manual_actions: false)
+
+ expect(deployment.deployable).to receive(:other_manual_actions).and_call_original
+
+ deployment.manual_actions
+ end
+ end
+
describe '#scheduled_actions' do
- subject { deployment.scheduled_actions }
+ let(:deployment) { create(:deployment) }
- let(:project) { create(:project, :repository) }
- let(:pipeline) { create(:ci_pipeline, project: project) }
- let(:build) { create(:ci_build, :success, pipeline: pipeline) }
- let(:deployment) { create(:deployment, deployable: build) }
+ it 'delegates to environment_scheduled_actions when deployment_environment_manual_actions ff is enabled' do
+ stub_feature_flags(deployment_environment_manual_actions: true)
- it 'delegates to other_scheduled_actions' do
- expect_next_instance_of(Ci::Build) do |instance|
- expect(instance).to receive(:other_scheduled_actions)
- end
+ expect(deployment.deployable).to receive(:environment_scheduled_actions).and_call_original
- subject
+ deployment.scheduled_actions
+ end
+
+ it 'delegates to other_scheduled_actions when deployment_environment_manual_actions ff is disabled' do
+ stub_feature_flags(deployment_environment_manual_actions: false)
+
+ expect(deployment.deployable).to receive(:other_scheduled_actions).and_call_original
+
+ deployment.scheduled_actions
end
end
diff --git a/spec/serializers/deployment_entity_spec.rb b/spec/serializers/deployment_entity_spec.rb
index 500d5718bf195..4452ccbcf07f3 100644
--- a/spec/serializers/deployment_entity_spec.rb
+++ b/spec/serializers/deployment_entity_spec.rb
@@ -60,12 +60,16 @@
end
context 'when the pipeline has another manual action' do
- let(:other_build) { create(:ci_build, :manual, name: 'another deploy', pipeline: pipeline) }
- let!(:other_deployment) { create(:deployment, deployable: other_build) }
+ let!(:other_build) do
+ create(:ci_build, :manual, name: 'another deploy',
+ pipeline: pipeline, environment: build.environment)
+ end
+
+ let!(:other_deployment) { create(:deployment, deployable: build) }
it 'returns another manual action' do
- expect(subject[:manual_actions].count).to eq(1)
- expect(subject[:manual_actions].first[:name]).to eq('another deploy')
+ expect(subject[:manual_actions].count).to eq(2)
+ expect(subject[:manual_actions].second[:name]).to eq('another deploy')
end
context 'when user is a reporter' do
diff --git a/spec/serializers/environment_serializer_spec.rb b/spec/serializers/environment_serializer_spec.rb
index fe6278084f93c..47d2c8b379479 100644
--- a/spec/serializers/environment_serializer_spec.rb
+++ b/spec/serializers/environment_serializer_spec.rb
@@ -227,8 +227,11 @@
def create_environment_with_associations(project)
create(:environment, project: project).tap do |environment|
- create(:deployment, :success, environment: environment, project: project)
- create(:deployment, :running, environment: environment, project: project)
+ pipeline = create(:ci_pipeline, project: project)
+ manual_build = create(:ci_build, :manual, project: project, pipeline: pipeline, environment: environment.name)
+ scheduled_build = create(:ci_build, :scheduled, project: project, pipeline: pipeline, environment: environment.name)
+ create(:deployment, :success, environment: environment, project: project, deployable: manual_build)
+ create(:deployment, :running, environment: environment, project: project, deployable: scheduled_build)
end
end
end
--
GitLab