Commit 94e5f04a authored by Yorick Peterse's avatar Yorick Peterse

Use explicitly tracked deployments for MR widgets

When displaying the deployments of a merge request in the MR widget, we
used to rely on the latest CI pipeline to determine what the deployment
was. In this commit we add the ability to use the table
"deployment_merge_requests" to display the deployments of a merge
request. This way the data displayed remains the same, regardless of the
state of any CI pipelines.

These changes are currently hidden behind the
"deployment_merge_requests_widget" project feature flag. We do not
reuse the existing deployments feature flag, as we still want to record
merge requests even if the UI is not able to display the data properly.
parent 2754fa28
......@@ -218,11 +218,16 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
end
def ci_environments_status
environments = if ci_environments_status_on_merge_result?
EnvironmentStatus.after_merge_request(@merge_request, current_user)
else
EnvironmentStatus.for_merge_request(@merge_request, current_user)
end
environments =
if ci_environments_status_on_merge_result?
if Feature.enabled?(:deployment_merge_requests_widget, @project)
EnvironmentStatus.for_deployed_merge_request(@merge_request, current_user)
else
EnvironmentStatus.after_merge_request(@merge_request, current_user)
end
else
EnvironmentStatus.for_merge_request(@merge_request, current_user)
end
render json: EnvironmentStatusSerializer.new(current_user: current_user).represent(environments)
end
......
......@@ -20,6 +20,28 @@ class EnvironmentStatus
build_environments_status(mr, user, mr.merge_pipeline)
end
def self.for_deployed_merge_request(mr, user)
statuses = []
mr.recent_visible_deployments.each do |deploy|
env = deploy.environment
next unless Ability.allowed?(user, :read_environment, env)
statuses <<
EnvironmentStatus.new(deploy.project, env, mr, deploy.sha)
end
# Existing projects that used deployments prior to the introduction of
# explicitly linked merge requests won't have any data using this new
# approach, so we fall back to retrieving deployments based on CI pipelines.
if statuses.any?
statuses
else
after_merge_request(mr, user)
end
end
def initialize(project, environment, merge_request, sha)
@project = project
@environment = environment
......
......@@ -73,6 +73,14 @@ class MergeRequest < ApplicationRecord
has_many :merge_request_assignees
has_many :assignees, class_name: "User", through: :merge_request_assignees
has_many :deployment_merge_requests
# These are deployments created after the merge request has been merged, and
# the merge request was tracked explicitly (instead of implicitly using a CI
# build).
has_many :deployments,
through: :deployment_merge_requests
KNOWN_MERGE_PARAMS = [
:auto_merge_strategy,
:should_remove_source_branch,
......@@ -1472,6 +1480,10 @@ class MergeRequest < ApplicationRecord
true
end
def recent_visible_deployments
deployments.visible.includes(:environment).order(id: :desc).limit(10)
end
private
def with_rebase_lock
......
......@@ -1280,6 +1280,28 @@ describe Projects::MergeRequestsController do
end
end
it 'uses the explicitly linked deployments' do
expect(EnvironmentStatus)
.to receive(:for_deployed_merge_request)
.with(merge_request, user)
.and_call_original
get_ci_environments_status(environment_target: 'merge_commit')
end
context 'when the deployment_merge_requests_widget feature flag is disabled' do
it 'uses the deployments retrieved using CI builds' do
stub_feature_flags(deployment_merge_requests_widget: false)
expect(EnvironmentStatus)
.to receive(:after_merge_request)
.with(merge_request, user)
.and_call_original
get_ci_environments_status(environment_target: 'merge_commit')
end
end
def get_ci_environments_status(extra_params = {})
params = {
namespace_id: merge_request.project.namespace.to_param,
......
......@@ -139,6 +139,8 @@ merge_requests:
- blocking_merge_requests
- blocked_merge_requests
- description_versions
- deployment_merge_requests
- deployments
external_pull_requests:
- project
merge_request_diff:
......
......@@ -92,6 +92,84 @@ describe EnvironmentStatus do
end
end
describe '.for_deployed_merge_request' do
context 'when a merge request has no explicitly linked deployments' do
it 'returns the statuses based on the CI pipelines' do
mr = create(:merge_request, :merged)
expect(described_class)
.to receive(:after_merge_request)
.with(mr, mr.author)
.and_return([])
statuses = described_class.for_deployed_merge_request(mr, mr.author)
expect(statuses).to eq([])
end
end
context 'when a merge request has explicitly linked deployments' do
let(:merge_request) { create(:merge_request, :merged) }
let(:environment) do
create(:environment, project: merge_request.target_project)
end
it 'returns the statuses based on the linked deployments' do
deploy = create(
:deployment,
:success,
project: merge_request.target_project,
environment: environment,
deployable: nil
)
deploy.link_merge_requests(merge_request.target_project.merge_requests)
statuses = described_class
.for_deployed_merge_request(merge_request, merge_request.author)
expect(statuses.length).to eq(1)
expect(statuses[0].environment).to eq(environment)
expect(statuses[0].merge_request).to eq(merge_request)
end
it 'excludes environments the user can not see' do
deploy = create(
:deployment,
:success,
project: merge_request.target_project,
environment: environment,
deployable: nil
)
deploy.link_merge_requests(merge_request.target_project.merge_requests)
statuses = described_class
.for_deployed_merge_request(merge_request, create(:user))
expect(statuses).to be_empty
end
it 'excludes deployments that have the status "created"' do
deploy = create(
:deployment,
:created,
project: merge_request.target_project,
environment: environment,
deployable: nil
)
deploy.link_merge_requests(merge_request.target_project.merge_requests)
statuses = described_class
.for_deployed_merge_request(merge_request, merge_request.author)
expect(statuses).to be_empty
end
end
end
describe '.build_environments_status' do
subject { described_class.send(:build_environments_status, merge_request, user, pipeline) }
......
......@@ -3376,4 +3376,56 @@ describe MergeRequest do
])
end
end
describe '#recent_visible_deployments' do
let(:merge_request) { create(:merge_request) }
let(:environment) do
create(:environment, project: merge_request.target_project)
end
it 'returns visible deployments' do
created = create(
:deployment,
:created,
project: merge_request.target_project,
environment: environment
)
success = create(
:deployment,
:success,
project: merge_request.target_project,
environment: environment
)
failed = create(
:deployment,
:failed,
project: merge_request.target_project,
environment: environment
)
merge_request.deployment_merge_requests.create!(deployment: created)
merge_request.deployment_merge_requests.create!(deployment: success)
merge_request.deployment_merge_requests.create!(deployment: failed)
expect(merge_request.recent_visible_deployments).to eq([failed, success])
end
it 'only returns a limited number of deployments' do
20.times do
deploy = create(
:deployment,
:success,
project: merge_request.target_project,
environment: environment
)
merge_request.deployment_merge_requests.create!(deployment: deploy)
end
expect(merge_request.recent_visible_deployments.count).to eq(10)
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