Commit 05f03b39 authored by Etienne Baqué's avatar Etienne Baqué Committed by Jan Provaznik

Renamed methods to expose playable build

Removed build_manual_actions method.
Using playable? check to expose build.
Added related tests.
parent f42880c0
...@@ -5,6 +5,7 @@ class Deployment < ApplicationRecord ...@@ -5,6 +5,7 @@ class Deployment < ApplicationRecord
include IidRoutes include IidRoutes
include AfterCommitQueue include AfterCommitQueue
include UpdatedAtFilterable include UpdatedAtFilterable
include Gitlab::Utils::StrongMemoize
belongs_to :project, required: true belongs_to :project, required: true
belongs_to :environment, required: true belongs_to :environment, required: true
...@@ -126,6 +127,12 @@ class Deployment < ApplicationRecord ...@@ -126,6 +127,12 @@ class Deployment < ApplicationRecord
@scheduled_actions ||= deployable.try(:other_scheduled_actions) @scheduled_actions ||= deployable.try(:other_scheduled_actions)
end end
def playable_build
strong_memoize(:playable_build) do
deployable.try(:playable?) ? deployable : nil
end
end
def includes_commit?(commit) def includes_commit?(commit)
return false unless commit return false unless commit
......
...@@ -78,7 +78,7 @@ class EnvironmentStatus ...@@ -78,7 +78,7 @@ class EnvironmentStatus
def self.build_environments_status(mr, user, pipeline) def self.build_environments_status(mr, user, pipeline)
return [] unless pipeline return [] unless pipeline
pipeline.environments.available.map do |environment| pipeline.environments.includes(:project).available.map do |environment|
next unless Ability.allowed?(user, :read_environment, environment) next unless Ability.allowed?(user, :read_environment, environment)
EnvironmentStatus.new(pipeline.project, environment, mr, pipeline.sha) EnvironmentStatus.new(pipeline.project, environment, mr, pipeline.sha)
......
...@@ -37,6 +37,9 @@ class DeploymentEntity < Grape::Entity ...@@ -37,6 +37,9 @@ class DeploymentEntity < Grape::Entity
expose :commit, using: CommitEntity, if: -> (*) { include_details? } expose :commit, using: CommitEntity, if: -> (*) { include_details? }
expose :manual_actions, using: JobEntity, if: -> (*) { include_details? && can_create_deployment? } expose :manual_actions, using: JobEntity, if: -> (*) { include_details? && can_create_deployment? }
expose :scheduled_actions, using: JobEntity, if: -> (*) { include_details? && can_create_deployment? } expose :scheduled_actions, using: JobEntity, if: -> (*) { include_details? && can_create_deployment? }
expose :playable_build, expose_nil: false, if: -> (*) { include_details? && can_create_deployment? } do |deployment, options|
JobEntity.represent(deployment.playable_build, options.merge(only: [:play_path, :retry_path]))
end
expose :cluster, using: ClusterBasicEntity expose :cluster, using: ClusterBasicEntity
...@@ -47,7 +50,7 @@ class DeploymentEntity < Grape::Entity ...@@ -47,7 +50,7 @@ class DeploymentEntity < Grape::Entity
end end
def can_create_deployment? def can_create_deployment?
can?(request.current_user, :create_deployment, request.project) can?(request.current_user, :create_deployment, project)
end end
def can_read_deployables? def can_read_deployables?
...@@ -56,6 +59,10 @@ class DeploymentEntity < Grape::Entity ...@@ -56,6 +59,10 @@ class DeploymentEntity < Grape::Entity
# because it triggers a policy evaluation that involves multiple # because it triggers a policy evaluation that involves multiple
# Gitaly calls that might not be cached. # Gitaly calls that might not be cached.
# #
can?(request.current_user, :read_build, request.project) can?(request.current_user, :read_build, project)
end
def project
request.try(:project) || options[:project]
end end
end end
...@@ -37,6 +37,10 @@ class EnvironmentStatusEntity < Grape::Entity ...@@ -37,6 +37,10 @@ class EnvironmentStatusEntity < Grape::Entity
es.deployment.try(:formatted_deployment_time) es.deployment.try(:formatted_deployment_time)
end end
expose :deployment, as: :details do |es, options|
DeploymentEntity.represent(es.deployment, options.merge(project: es.project, only: [:playable_build]))
end
expose :changes expose :changes
private private
......
...@@ -77,6 +77,10 @@ class PipelineEntity < Grape::Entity ...@@ -77,6 +77,10 @@ class PipelineEntity < Grape::Entity
cancel_project_pipeline_path(pipeline.project, pipeline) cancel_project_pipeline_path(pipeline.project, pipeline)
end end
expose :failed_builds, if: -> (*) { can_retry? }, using: JobEntity do |pipeline|
pipeline.builds.failed
end
private private
alias_method :pipeline, :object alias_method :pipeline, :object
......
---
title: Exposed deployment build manual actions for merge request page
merge_request: 20615
author:
type: changed
...@@ -1226,9 +1226,9 @@ describe Projects::MergeRequestsController do ...@@ -1226,9 +1226,9 @@ describe Projects::MergeRequestsController do
environment2 = create(:environment, project: forked) environment2 = create(:environment, project: forked)
create(:deployment, :succeed, environment: environment2, sha: sha, ref: 'master', deployable: build) create(:deployment, :succeed, environment: environment2, sha: sha, ref: 'master', deployable: build)
# TODO address the last 5 queries # TODO address the last 3 queries
# See https://gitlab.com/gitlab-org/gitlab-foss/issues/63952 (5 queries) # See https://gitlab.com/gitlab-org/gitlab-foss/issues/63952 (3 queries)
leeway = 5 leeway = 3
expect { get_ci_environments_status }.not_to exceed_all_query_limit(control_count + leeway) expect { get_ci_environments_status }.not_to exceed_all_query_limit(control_count + leeway)
end end
end end
......
...@@ -442,4 +442,36 @@ describe Deployment do ...@@ -442,4 +442,36 @@ describe Deployment do
expect(deploy2.previous_environment_deployment).to be_nil expect(deploy2.previous_environment_deployment).to be_nil
end end
end end
describe '#playable_build' do
subject { deployment.playable_build }
context 'when there is a deployable build' do
let(:deployment) { create(:deployment, deployable: build) }
context 'when the deployable build is playable' do
let(:build) { create(:ci_build, :playable) }
it 'returns that build' do
is_expected.to eq(build)
end
end
context 'when the deployable build is not playable' do
let(:build) { create(:ci_build) }
it 'returns nil' do
is_expected.to be_nil
end
end
end
context 'when there is no deployable build' do
let(:deployment) { create(:deployment) }
it 'returns nil' do
is_expected.to be_nil
end
end
end
end end
...@@ -107,6 +107,36 @@ describe DeploymentEntity do ...@@ -107,6 +107,36 @@ describe DeploymentEntity do
end end
end end
describe 'playable_build' do
let_it_be(:project) { create(:project, :repository) }
context 'when the deployment has a playable deployable' do
context 'when this build is ready to be played' do
let(:build) { create(:ci_build, :playable, :scheduled, pipeline: pipeline) }
it 'exposes only the play_path' do
expect(subject[:playable_build].keys).to contain_exactly(:play_path)
end
end
context 'when this build has failed' do
let(:build) { create(:ci_build, :playable, :failed, pipeline: pipeline) }
it 'exposes the play_path and the retry_path' do
expect(subject[:playable_build].keys).to contain_exactly(:play_path, :retry_path)
end
end
end
context 'when the deployment does not have a playable deployable' do
let(:build) { create(:ci_build) }
it 'is not exposed' do
expect(subject[:playable_build]).to be_nil
end
end
end
context 'when deployment details serialization was disabled' do context 'when deployment details serialization was disabled' do
include Gitlab::Routing include Gitlab::Routing
......
...@@ -4,7 +4,7 @@ require 'spec_helper' ...@@ -4,7 +4,7 @@ require 'spec_helper'
describe EnvironmentStatusEntity do describe EnvironmentStatusEntity do
let(:user) { create(:user) } let(:user) { create(:user) }
let(:request) { double('request') } let(:request) { double('request', project: project) }
let(:deployment) { create(:deployment, :succeed, :review_app) } let(:deployment) { create(:deployment, :succeed, :review_app) }
let(:environment) { deployment.environment } let(:environment) { deployment.environment }
...@@ -28,6 +28,7 @@ describe EnvironmentStatusEntity do ...@@ -28,6 +28,7 @@ describe EnvironmentStatusEntity do
it { is_expected.to include(:external_url_formatted) } it { is_expected.to include(:external_url_formatted) }
it { is_expected.to include(:deployed_at) } it { is_expected.to include(:deployed_at) }
it { is_expected.to include(:deployed_at_formatted) } it { is_expected.to include(:deployed_at_formatted) }
it { is_expected.to include(:details) }
it { is_expected.to include(:changes) } it { is_expected.to include(:changes) }
it { is_expected.to include(:status) } it { is_expected.to include(:status) }
......
...@@ -218,5 +218,28 @@ describe PipelineEntity do ...@@ -218,5 +218,28 @@ describe PipelineEntity do
expect(subject[:merge_request_event_type]).to be_present expect(subject[:merge_request_event_type]).to be_present
end end
end end
context 'when pipeline has failed builds' do
let_it_be(:pipeline) { create(:ci_pipeline, project: project, user: user) }
let_it_be(:build) { create(:ci_build, :success, pipeline: pipeline) }
let_it_be(:failed_1) { create(:ci_build, :failed, pipeline: pipeline) }
let_it_be(:failed_2) { create(:ci_build, :failed, pipeline: pipeline) }
context 'when the user can retry the pipeline' do
it 'exposes these failed builds' do
allow(entity).to receive(:can_retry?).and_return(true)
expect(subject[:failed_builds].map { |b| b[:id] }).to contain_exactly(failed_1.id, failed_2.id)
end
end
context 'when the user cannot retry the pipeline' do
it 'is nil' do
allow(entity).to receive(:can_retry?).and_return(false)
expect(subject[:failed_builds]).to be_nil
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