summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorShinya Maeda <shinya@gitlab.com>2018-12-03 15:46:45 +0900
committerShinya Maeda <shinya@gitlab.com>2018-12-03 15:47:09 +0900
commit4bd00e5378736231225394f80b87b7f89077cb76 (patch)
tree72929630dceb9651c2c51afdffb5d60f2e6ff3dc
parenta445aa0a9267f191fab1c46d438b42fbec549da5 (diff)
downloadgitlab-ce-4bd00e5378736231225394f80b87b7f89077cb76.tar.gz
Squashed commit of the following:
commit 04b06a2293fa12660a9c213a9db27fe90b83248b Merge: d580841d4ed a445aa0a926 Author: Shinya Maeda <shinya@gitlab.com> Date: Mon Dec 3 10:51:55 2018 +0900 Merge branch 'master-ce' into fix-mr-widget-unrelated-deployment-status commit d580841d4ed944f01e6417fa77842826843b6a04 Author: Shinya Maeda <shinya@gitlab.com> Date: Fri Nov 30 18:11:04 2018 +0900 Add environment to all_models.yml commit 689fbe2699a3adf10804312e680fa336e4560eaf Author: Shinya Maeda <shinya@gitlab.com> Date: Fri Nov 30 17:00:35 2018 +0900 Proper way to get uniq relationship commit c0733c6ecd535a6a1b6243080a2226836890f479 Author: Shinya Maeda <shinya@gitlab.com> Date: Fri Nov 30 16:20:40 2018 +0900 Revert build change commit 19dc55a8fe6e0fa575858d51144516b7fb0120de Author: Shinya Maeda <shinya@gitlab.com> Date: Fri Nov 30 16:19:18 2018 +0900 Add uniq option commit 0a6995f311c09b453fd0aecff2f6052de38efc27 Author: Shinya Maeda <shinya@gitlab.com> Date: Fri Nov 30 16:14:30 2018 +0900 Drop persisted_environment commit 3f68fc783b0ee0d66e03de6d979616c4c4892118 Author: Shinya Maeda <shinya@gitlab.com> Date: Wed Nov 28 13:59:04 2018 +0900 Return empty array if pipeline is nil commit 73801c5c3d06339e38dce7461a71285bcdbb8f45 Author: Shinya Maeda <shinya@gitlab.com> Date: Tue Nov 27 16:34:47 2018 +0900 Add changelog commit d461699abf2835cc51949a5138e829628209dd6d Author: Shinya Maeda <shinya@gitlab.com> Date: Tue Nov 27 16:32:20 2018 +0900 Squashed commit of the following: commit 77ab5259605e217a39b04d2cea6204277e42d2b5 Merge: 7ac1ed50612 2ee8c40fc16 Author: Shinya Maeda <shinya@gitlab.com> Date: Tue Nov 27 16:31:07 2018 +0900 Merge branch 'master-ce' into fix-mr-widget-unrelated-deployment-status commit 7ac1ed50612620df6408220b0a7cfcb626a04c48 Merge: 49ba5c5aeff b55aeca25e7 Author: Shinya Maeda <shinya@gitlab.com> Date: Mon Nov 26 20:01:43 2018 +0900 Merge branch 'master-ce' into fix-mr-widget-unrelated-deployment-status commit 49ba5c5aeff3efee7b7498d443372021c3f4f8b5 Merge: aa3fd0ff9e8 fbbe5ccd1be Author: Shinya Maeda <shinya@gitlab.com> Date: Mon Nov 26 15:27:36 2018 +0900 Merge branch 'master-ce' into fix-mr-widget-unrelated-deployment-status commit aa3fd0ff9e8a418a233ebaa60b38c081cab50099 Author: Shinya Maeda <shinya@gitlab.com> Date: Tue Nov 20 18:28:53 2018 +0900 Fix static analysis commit 7afe5f37003869a73dbb297229f8533f78b82684 Merge: e65b9580ff4 8a581d531ba Author: Shinya Maeda <shinya@gitlab.com> Date: Tue Nov 20 18:27:33 2018 +0900 Merge branch 'master-ce' into fix-mr-widget-unrelated-deployment-status commit e65b9580ff422359113e1a4e37c212f7b13aba4d Author: Shinya Maeda <shinya@gitlab.com> Date: Mon Nov 19 17:59:48 2018 +0900 Ignore deployments from project import/export commit 9eb4ddab8415c1ef61a3c646bdc4602bcf4ebe24 Author: Shinya Maeda <shinya@gitlab.com> Date: Mon Nov 19 16:26:00 2018 +0900 Add memoization commit 57f0bea3aaaa07b75d18e52068c532277350cda0 Author: Shinya Maeda <shinya@gitlab.com> Date: Mon Nov 19 16:21:39 2018 +0900 Fix unrelated deployment status in MR widget
-rw-r--r--app/models/ci/pipeline.rb6
-rw-r--r--app/models/environment_status.rb14
-rw-r--r--changelogs/unreleased/fix-mr-widget-unrelated-deployment-status.yml5
-rw-r--r--spec/features/merge_request/user_sees_deployment_widget_spec.rb16
-rw-r--r--spec/lib/gitlab/import_export/all_models.yml2
-rw-r--r--spec/models/environment_status_spec.rb39
6 files changed, 66 insertions, 16 deletions
diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 9512ba42f67..1487b9d3bca 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -26,6 +26,8 @@ module Ci
has_many :builds, foreign_key: :commit_id, inverse_of: :pipeline
has_many :trigger_requests, dependent: :destroy, foreign_key: :commit_id # rubocop:disable Cop/ActiveRecordDependent
has_many :variables, class_name: 'Ci::PipelineVariable'
+ has_many :deployments, through: :builds
+ has_many :environments, -> { distinct }, through: :deployments
# Merge requests for which the current pipeline is running against
# the merge request's latest commit.
@@ -523,10 +525,6 @@ module Ci
yaml_errors.present?
end
- def environments
- builds.where.not(environment: nil).success.pluck(:environment).uniq
- end
-
# Manually set the notes for a Ci::Pipeline
# There is no ActiveRecord relation between Ci::Pipeline and notes
# as they are related to a commit sha. This method helps importing
diff --git a/app/models/environment_status.rb b/app/models/environment_status.rb
index 4a128dde5cd..2fb6cadc8cd 100644
--- a/app/models/environment_status.rb
+++ b/app/models/environment_status.rb
@@ -12,13 +12,13 @@ class EnvironmentStatus
delegate :deployed_at, to: :deployment, allow_nil: true
def self.for_merge_request(mr, user)
- build_environments_status(mr, user, mr.diff_head_sha)
+ build_environments_status(mr, user, mr.actual_head_pipeline)
end
def self.after_merge_request(mr, user)
return [] unless mr.merged?
- build_environments_status(mr, user, mr.merge_commit_sha)
+ build_environments_status(mr, user, mr.merge_pipeline)
end
def initialize(environment, merge_request, sha)
@@ -61,13 +61,13 @@ class EnvironmentStatus
}
end
- def self.build_environments_status(mr, user, sha)
- Environment.where(project_id: [mr.source_project_id, mr.target_project_id])
- .available
- .with_deployment(sha).map do |environment|
+ def self.build_environments_status(mr, user, pipeline)
+ return [] unless pipeline
+
+ pipeline.environments.available.map do |environment|
next unless Ability.allowed?(user, :read_environment, environment)
- EnvironmentStatus.new(environment, mr, sha)
+ EnvironmentStatus.new(environment, mr, pipeline.sha)
end.compact
end
private_class_method :build_environments_status
diff --git a/changelogs/unreleased/fix-mr-widget-unrelated-deployment-status.yml b/changelogs/unreleased/fix-mr-widget-unrelated-deployment-status.yml
new file mode 100644
index 00000000000..ab926fbd43b
--- /dev/null
+++ b/changelogs/unreleased/fix-mr-widget-unrelated-deployment-status.yml
@@ -0,0 +1,5 @@
+---
+title: Fix unrelated deployment status in MR widget
+merge_request: 23175
+author:
+type: fixed
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 3e40179ad9a..fe8e0b07d39 100644
--- a/spec/features/merge_request/user_sees_deployment_widget_spec.rb
+++ b/spec/features/merge_request/user_sees_deployment_widget_spec.rb
@@ -29,6 +29,22 @@ describe 'Merge request > User sees deployment widget', :js do
expect(page).to have_content("Deployed to #{environment.name}")
expect(find('.js-deploy-time')['data-original-title']).to eq(deployment.created_at.to_time.in_time_zone.to_s(:medium))
end
+
+ context 'when a user created a new merge request with the same SHA' do
+ let(:pipeline2) { create(:ci_pipeline_without_jobs, sha: sha, project: project, ref: 'new-patch-1') }
+ let(:build2) { create(:ci_build, :success, pipeline: pipeline2) }
+ let(:environment2) { create(:environment, project: project) }
+ let!(:deployment2) { create(:deployment, environment: environment2, sha: sha, ref: 'new-patch-1', deployable: build2) }
+
+ it 'displays one environment which is related to the pipeline' do
+ visit project_merge_request_path(project, merge_request)
+ wait_for_requests
+
+ expect(page).to have_selector('.js-deployment-info', count: 1)
+ expect(page).to have_content("#{environment.name}")
+ expect(page).not_to have_content("#{environment2.name}")
+ end
+ end
end
context 'when deployment failed' do
diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml
index 8d2f60d7a8b..31ab11bbf8d 100644
--- a/spec/lib/gitlab/import_export/all_models.yml
+++ b/spec/lib/gitlab/import_export/all_models.yml
@@ -121,6 +121,8 @@ pipelines:
- artifacts
- pipeline_schedule
- merge_requests
+- deployments
+- environments
pipeline_variables:
- pipeline
stages:
diff --git a/spec/models/environment_status_spec.rb b/spec/models/environment_status_spec.rb
index 90f7e4a4590..9da16dea929 100644
--- a/spec/models/environment_status_spec.rb
+++ b/spec/models/environment_status_spec.rb
@@ -92,16 +92,12 @@ describe EnvironmentStatus do
end
describe '.build_environments_status' do
- subject { described_class.send(:build_environments_status, merge_request, user, sha) }
+ subject { described_class.send(:build_environments_status, merge_request, user, pipeline) }
let!(:build) { create(:ci_build, :deploy_to_production, pipeline: pipeline) }
let(:environment) { build.deployment.environment }
let(:user) { project.owner }
- before do
- build.deployment&.update!(sha: sha)
- end
-
context 'when environment is created on a forked project' do
let(:project) { create(:project, :repository) }
let(:forked) { fork_project(project, user, repository: true) }
@@ -160,6 +156,39 @@ describe EnvironmentStatus do
expect(subject.count).to eq(0)
end
end
+
+ context 'when multiple deployments with the same SHA in different environments' do
+ let(:pipeline2) { create(:ci_pipeline, sha: sha, project: project) }
+ let!(:build2) { create(:ci_build, :start_review_app, pipeline: pipeline2) }
+
+ it 'returns deployments related to the head pipeline' do
+ expect(subject.count).to eq(1)
+ expect(subject[0].environment).to eq(environment)
+ expect(subject[0].merge_request).to eq(merge_request)
+ expect(subject[0].sha).to eq(sha)
+ end
+ end
+
+ context 'when multiple deployments in the same pipeline for the same environments' do
+ let!(:build2) { create(:ci_build, :deploy_to_production, pipeline: pipeline) }
+
+ it 'returns unique entries' do
+ expect(subject.count).to eq(1)
+ expect(subject[0].environment).to eq(environment)
+ expect(subject[0].merge_request).to eq(merge_request)
+ expect(subject[0].sha).to eq(sha)
+ end
+ end
+
+ context 'when environment is stopped' do
+ before do
+ environment.stop!
+ end
+
+ it 'does not return environment status' do
+ expect(subject.count).to eq(0)
+ end
+ end
end
end
end