Commit 50776d2d authored by Shinya Maeda's avatar Shinya Maeda

Expose merge request entity for pipelines

Add preload

Fix

ok

Write tests

test only postgresql

ok

add more test
;

Improve wording

Add changelog

Fix
parent c9e5ce8d
...@@ -98,6 +98,12 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated ...@@ -98,6 +98,12 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated
end end
end end
def target_branch_path
if target_branch_exists?
project_branch_path(project, target_branch)
end
end
def source_branch_path def source_branch_path
if source_branch_exists? if source_branch_exists?
project_branch_path(source_project, source_branch) project_branch_path(source_project, source_branch)
......
# frozen_string_literal: true
class MergeRequestForPipelineEntity < Grape::Entity
include RequestAwareEntity
expose :iid
expose :path do |merge_request|
project_merge_request_path(merge_request.project, merge_request)
end
expose :title
expose :source_branch
expose :source_branch_path
expose :target_branch
expose :target_branch_path
end
...@@ -28,6 +28,7 @@ class PipelineEntity < Grape::Entity ...@@ -28,6 +28,7 @@ class PipelineEntity < Grape::Entity
expose :can_retry?, as: :retryable expose :can_retry?, as: :retryable
expose :can_cancel?, as: :cancelable expose :can_cancel?, as: :cancelable
expose :failure_reason?, as: :failure_reason expose :failure_reason?, as: :failure_reason
expose :detached_merge_request_pipeline?, as: :detached
end end
expose :details do expose :details do
...@@ -36,6 +37,10 @@ class PipelineEntity < Grape::Entity ...@@ -36,6 +37,10 @@ class PipelineEntity < Grape::Entity
expose :finished_at expose :finished_at
end end
expose :merge_request, if: -> (*) { has_presentable_merge_request? }, with: MergeRequestForPipelineEntity do |pipeline|
pipeline.merge_request.present(current_user: request.current_user)
end
expose :ref do expose :ref do
expose :name do |pipeline| expose :name do |pipeline|
pipeline.ref pipeline.ref
...@@ -81,6 +86,11 @@ class PipelineEntity < Grape::Entity ...@@ -81,6 +86,11 @@ class PipelineEntity < Grape::Entity
pipeline.cancelable? pipeline.cancelable?
end end
def has_presentable_merge_request?
pipeline.triggered_by_merge_request? &&
can?(request.current_user, :read_merge_request, pipeline.merge_request)
end
def detailed_status def detailed_status
pipeline.detailed_status(request.current_user) pipeline.detailed_status(request.current_user)
end end
......
...@@ -15,6 +15,7 @@ class PipelineSerializer < BaseSerializer ...@@ -15,6 +15,7 @@ class PipelineSerializer < BaseSerializer
:manual_actions, :manual_actions,
:scheduled_actions, :scheduled_actions,
:artifacts, :artifacts,
:merge_request,
{ {
pending_builds: :project, pending_builds: :project,
project: [:route, { namespace: :route }], project: [:route, { namespace: :route }],
......
---
title: Expose merge request entity for pipelines
merge_request: 25679
author:
type: added
...@@ -392,6 +392,29 @@ describe MergeRequestPresenter do ...@@ -392,6 +392,29 @@ describe MergeRequestPresenter do
end end
end end
describe '#target_branch_path' do
subject do
described_class.new(resource, current_user: user).target_branch_path
end
context 'when target branch exists' do
it 'returns path' do
allow(resource).to receive(:target_branch_exists?) { true }
is_expected
.to eq("/#{resource.source_project.full_path}/branches/#{resource.target_branch}")
end
end
context 'when target branch does not exist' do
it 'returns nil' do
allow(resource).to receive(:target_branch_exists?) { false }
is_expected.to be_nil
end
end
end
describe '#source_branch_with_namespace_link' do describe '#source_branch_with_namespace_link' do
subject do subject do
described_class.new(resource, current_user: user).source_branch_with_namespace_link described_class.new(resource, current_user: user).source_branch_with_namespace_link
......
require 'spec_helper'
describe MergeRequestForPipelineEntity do
let(:project) { create(:project, :repository) }
let(:user) { create(:user) }
let(:request) { EntityRequest.new(project: project) }
let(:merge_request) { create(:merge_request, target_project: project, source_project: project) }
let(:presenter) { MergeRequestPresenter.new(merge_request, current_user: user) }
let(:entity) do
described_class.new(presenter, request: request)
end
before do
project.add_developer(user)
end
context 'as json' do
subject { entity.as_json }
it 'exposes needed attributes' do
expect(subject).to include(
:iid, :path, :title,
:source_branch, :source_branch_path,
:target_branch, :target_branch_path
)
end
end
end
require 'spec_helper' require 'spec_helper'
describe PipelineEntity do describe PipelineEntity do
include Gitlab::Routing
set(:user) { create(:user) } set(:user) { create(:user) }
let(:request) { double('request') } let(:request) { double('request') }
...@@ -128,5 +130,48 @@ describe PipelineEntity do ...@@ -128,5 +130,48 @@ describe PipelineEntity do
.to eq 'CI/CD YAML configuration error!' .to eq 'CI/CD YAML configuration error!'
end end
end end
context 'when pipeline is detached merge request pipeline' do
let(:merge_request) { create(:merge_request, :with_merge_request_pipeline) }
let(:project) { merge_request.target_project }
let(:pipeline) { merge_request.merge_request_pipelines.first }
it 'makes detached flag true' do
expect(subject[:flags][:detached]).to be_truthy
end
context 'when user is a developer' do
before do
project.add_developer(user)
end
it 'has merge request information' do
expect(subject[:merge_request][:iid]).to eq(merge_request.iid)
expect(project_merge_request_path(project, merge_request))
.to include(subject[:merge_request][:path])
expect(subject[:merge_request][:title]).to eq(merge_request.title)
expect(subject[:merge_request][:source_branch])
.to eq(merge_request.source_branch)
expect(project_branch_path(project, merge_request.source_branch))
.to include(subject[:merge_request][:source_branch_path])
expect(subject[:merge_request][:target_branch])
.to eq(merge_request.target_branch)
expect(project_branch_path(project, merge_request.target_branch))
.to include(subject[:merge_request][:target_branch_path])
end
end
context 'when user is an external user' do
it 'has no merge request information' do
expect(subject[:merge_request]).to be_nil
end
end
end
end end
end end
...@@ -97,6 +97,44 @@ describe PipelineSerializer do ...@@ -97,6 +97,44 @@ describe PipelineSerializer do
end end
end end
context 'when there are pipelines for merge requests' do
let(:resource) { Ci::Pipeline.all }
let!(:merge_request_1) do
create(:merge_request,
:with_merge_request_pipeline,
target_project: project,
target_branch: 'master',
source_project: project,
source_branch: 'feature-1')
end
let!(:merge_request_2) do
create(:merge_request,
:with_merge_request_pipeline,
target_project: project,
target_branch: 'master',
source_project: project,
source_branch: 'feature-2')
end
before do
project.add_developer(user)
end
it 'includes merge requests information' do
expect(subject.all? { |entry| entry[:merge_request].present? }).to be_truthy
end
it 'preloads related merge requests', :postgresql do
recorded = ActiveRecord::QueryRecorder.new { subject }
expect(recorded.log)
.to include("SELECT \"merge_requests\".* FROM \"merge_requests\" " \
"WHERE \"merge_requests\".\"id\" IN (#{merge_request_1.id}, #{merge_request_2.id})")
end
end
describe 'number of queries when preloaded' do describe 'number of queries when preloaded' do
subject { serializer.represent(resource, preload: true) } subject { serializer.represent(resource, preload: true) }
let(:resource) { Ci::Pipeline.all } let(:resource) { Ci::Pipeline.all }
......
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