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

Merge branch 'id-reduce-pipeline-serializer-for-merge-requests' into 'master'

Serialize fewer pipeline fields for MR widget

See merge request gitlab-org/gitlab!37422
parents b413926f 2377a76f
......@@ -84,9 +84,6 @@ export default {
hasCommitInfo() {
return this.pipeline.commit && Object.keys(this.pipeline.commit).length > 0;
},
isTriggeredByMergeRequest() {
return Boolean(this.pipeline.merge_request);
},
isMergeRequestPipeline() {
return Boolean(this.pipeline.flags && this.pipeline.flags.merge_request_pipeline);
},
......
......@@ -19,9 +19,21 @@ class MergeRequestPollWidgetEntity < Grape::Entity
# User entities
expose :merge_user, using: UserEntity
expose :actual_head_pipeline, with: PipelineDetailsEntity, as: :pipeline, if: -> (mr, _) { presenter(mr).can_read_pipeline? }
expose :actual_head_pipeline, as: :pipeline, if: -> (mr, _) { presenter(mr).can_read_pipeline? } do |merge_request, options|
if Feature.enabled?(:merge_request_short_pipeline_serializer, merge_request.project)
MergeRequests::PipelineEntity.represent(merge_request.actual_head_pipeline, options)
else
PipelineDetailsEntity.represent(merge_request.actual_head_pipeline, options)
end
end
expose :merge_pipeline, with: PipelineDetailsEntity, if: ->(mr, _) { mr.merged? && can?(request.current_user, :read_pipeline, mr.target_project)}
expose :merge_pipeline, if: ->(mr, _) { mr.merged? && can?(request.current_user, :read_pipeline, mr.target_project)} do |merge_request, options|
if Feature.enabled?(:merge_request_short_pipeline_serializer, merge_request.project)
MergeRequests::PipelineEntity.represent(merge_request.merge_pipeline, options)
else
PipelineDetailsEntity.represent(merge_request.merge_pipeline, options)
end
end
expose :default_merge_commit_message
......
# frozen_string_literal: true
class MergeRequests::PipelineEntity < Grape::Entity
include RequestAwareEntity
expose :id
expose :active?, as: :active
expose :path do |pipeline|
project_pipeline_path(pipeline.project, pipeline)
end
expose :flags do
expose :merge_request_pipeline?, as: :merge_request_pipeline
end
expose :commit, using: CommitEntity
expose :details do
expose :name do |pipeline|
pipeline.present.name
end
expose :detailed_status, as: :status, with: DetailedStatusEntity do |pipeline|
pipeline.detailed_status(request.current_user)
end
expose :ordered_stages, as: :stages, using: StageEntity
end
# Coverage isn't always necessary (e.g. when displaying project pipelines in
# the UI). Instead of creating an entirely different entity we just allow the
# disabling of this specific field whenever necessary.
expose :coverage, unless: proc { options[:disable_coverage] }
expose :ref do
expose :branch?, as: :branch
end
expose :triggered_by_pipeline, as: :triggered_by, with: TriggeredPipelineEntity
expose :triggered_pipelines, as: :triggered, using: TriggeredPipelineEntity
end
......@@ -36,13 +36,28 @@ RSpec.describe MergeRequestPollWidgetEntity do
it 'returns merge_pipeline' do
pipeline.reload
pipeline_payload = PipelineDetailsEntity
.represent(pipeline, request: request)
.as_json
pipeline_payload =
MergeRequests::PipelineEntity
.represent(pipeline, request: request)
.as_json
expect(subject[:merge_pipeline]).to eq(pipeline_payload)
end
context 'when merge_request_short_pipeline_serializer is disabled' do
it 'returns detailed info about pipeline' do
stub_feature_flags(merge_request_short_pipeline_serializer: false)
pipeline.reload
pipeline_payload =
PipelineDetailsEntity
.represent(pipeline, request: request)
.as_json
expect(subject[:merge_pipeline]).to eq(pipeline_payload)
end
end
context 'when user cannot read pipelines on target project' do
before do
project.add_guest(user)
......@@ -222,13 +237,27 @@ RSpec.describe MergeRequestPollWidgetEntity do
let(:req) { double('request', current_user: user, project: project) }
it 'returns pipeline' do
pipeline_payload = PipelineDetailsEntity
.represent(pipeline, request: req)
.as_json
pipeline_payload =
MergeRequests::PipelineEntity
.represent(pipeline, request: req)
.as_json
expect(subject[:pipeline]).to eq(pipeline_payload)
end
context 'when merge_request_short_pipeline_serializer is disabled' do
it 'returns detailed info about pipeline' do
stub_feature_flags(merge_request_short_pipeline_serializer: false)
pipeline_payload =
PipelineDetailsEntity
.represent(pipeline, request: req)
.as_json
expect(subject[:pipeline]).to eq(pipeline_payload)
end
end
it 'returns ci_status' do
expect(subject[:ci_status]).to eq('pending')
end
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe MergeRequests::PipelineEntity do
let_it_be(:project) { create(:project, :repository) }
let_it_be(:user) { create(:user) }
let_it_be(:pipeline) { create(:ci_pipeline, project: project) }
let(:request) { double('request') }
before do
stub_not_protect_default_branch
allow(request).to receive(:current_user).and_return(user)
allow(request).to receive(:project).and_return(project)
end
let(:entity) do
described_class.represent(pipeline, request: request)
end
subject { entity.as_json }
describe '#as_json' do
it 'contains required fields' do
is_expected.to include(
:id, :path, :active, :coverage, :ref, :commit, :details,
:flags, :triggered, :triggered_by
)
expect(subject[:commit]).to include(:short_id, :commit_path)
expect(subject[:ref]).to include(:branch)
expect(subject[:details]).to include(:name, :status, :stages)
expect(subject[:details][:status]).to include(:icon, :favicon, :text, :label, :tooltip)
expect(subject[:flags]).to include(:merge_request_pipeline)
end
it 'excludes coverage data when disabled' do
entity = described_class
.represent(pipeline, request: request, disable_coverage: true)
expect(entity.as_json).not_to include(:coverage)
end
end
end
......@@ -7,7 +7,6 @@ RSpec.describe PipelineEntity do
let_it_be(:project) { create(:project) }
let_it_be(:user) { create(:user) }
let_it_be(:project) { create(:project) }
let(:request) { double('request') }
before do
......
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