Commit 4bd00e53 authored by Shinya Maeda's avatar Shinya Maeda

Squashed commit of the following:

commit 04b06a2293fa12660a9c213a9db27fe90b83248b
Merge: d580841d4ed a445aa0a
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 2ee8c40f
    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 b55aeca2
    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 fbbe5ccd
    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 8a581d53
    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
parent a445aa0a
...@@ -26,6 +26,8 @@ module Ci ...@@ -26,6 +26,8 @@ module Ci
has_many :builds, foreign_key: :commit_id, inverse_of: :pipeline 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 :trigger_requests, dependent: :destroy, foreign_key: :commit_id # rubocop:disable Cop/ActiveRecordDependent
has_many :variables, class_name: 'Ci::PipelineVariable' 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 # Merge requests for which the current pipeline is running against
# the merge request's latest commit. # the merge request's latest commit.
...@@ -523,10 +525,6 @@ module Ci ...@@ -523,10 +525,6 @@ module Ci
yaml_errors.present? yaml_errors.present?
end end
def environments
builds.where.not(environment: nil).success.pluck(:environment).uniq
end
# Manually set the notes for a Ci::Pipeline # Manually set the notes for a Ci::Pipeline
# There is no ActiveRecord relation between Ci::Pipeline and notes # There is no ActiveRecord relation between Ci::Pipeline and notes
# as they are related to a commit sha. This method helps importing # as they are related to a commit sha. This method helps importing
......
...@@ -12,13 +12,13 @@ class EnvironmentStatus ...@@ -12,13 +12,13 @@ class EnvironmentStatus
delegate :deployed_at, to: :deployment, allow_nil: true delegate :deployed_at, to: :deployment, allow_nil: true
def self.for_merge_request(mr, user) 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 end
def self.after_merge_request(mr, user) def self.after_merge_request(mr, user)
return [] unless mr.merged? return [] unless mr.merged?
build_environments_status(mr, user, mr.merge_commit_sha) build_environments_status(mr, user, mr.merge_pipeline)
end end
def initialize(environment, merge_request, sha) def initialize(environment, merge_request, sha)
...@@ -61,13 +61,13 @@ class EnvironmentStatus ...@@ -61,13 +61,13 @@ class EnvironmentStatus
} }
end end
def self.build_environments_status(mr, user, sha) def self.build_environments_status(mr, user, pipeline)
Environment.where(project_id: [mr.source_project_id, mr.target_project_id]) return [] unless pipeline
.available
.with_deployment(sha).map do |environment| pipeline.environments.available.map do |environment|
next unless Ability.allowed?(user, :read_environment, environment) next unless Ability.allowed?(user, :read_environment, environment)
EnvironmentStatus.new(environment, mr, sha) EnvironmentStatus.new(environment, mr, pipeline.sha)
end.compact end.compact
end end
private_class_method :build_environments_status private_class_method :build_environments_status
......
---
title: Fix unrelated deployment status in MR widget
merge_request: 23175
author:
type: fixed
...@@ -29,6 +29,22 @@ describe 'Merge request > User sees deployment widget', :js do ...@@ -29,6 +29,22 @@ describe 'Merge request > User sees deployment widget', :js do
expect(page).to have_content("Deployed to #{environment.name}") 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)) expect(find('.js-deploy-time')['data-original-title']).to eq(deployment.created_at.to_time.in_time_zone.to_s(:medium))
end 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 end
context 'when deployment failed' do context 'when deployment failed' do
......
...@@ -121,6 +121,8 @@ pipelines: ...@@ -121,6 +121,8 @@ pipelines:
- artifacts - artifacts
- pipeline_schedule - pipeline_schedule
- merge_requests - merge_requests
- deployments
- environments
pipeline_variables: pipeline_variables:
- pipeline - pipeline
stages: stages:
......
...@@ -92,16 +92,12 @@ describe EnvironmentStatus do ...@@ -92,16 +92,12 @@ describe EnvironmentStatus do
end end
describe '.build_environments_status' do 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!(:build) { create(:ci_build, :deploy_to_production, pipeline: pipeline) }
let(:environment) { build.deployment.environment } let(:environment) { build.deployment.environment }
let(:user) { project.owner } let(:user) { project.owner }
before do
build.deployment&.update!(sha: sha)
end
context 'when environment is created on a forked project' do context 'when environment is created on a forked project' do
let(:project) { create(:project, :repository) } let(:project) { create(:project, :repository) }
let(:forked) { fork_project(project, user, repository: true) } let(:forked) { fork_project(project, user, repository: true) }
...@@ -160,6 +156,39 @@ describe EnvironmentStatus do ...@@ -160,6 +156,39 @@ describe EnvironmentStatus do
expect(subject.count).to eq(0) expect(subject.count).to eq(0)
end end
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 end
end end
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