Commit 2377a76f authored by Igor Drozdov's avatar Igor Drozdov

Serialize fewer pipeline fields for MR widget

Frontend doesn't need all the information currently provided
This commit reduces the number of serialized fields to the
necessary ones
parent f4b8fb60
......@@ -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