Commit 1f54c921 authored by Oswaldo Ferreira's avatar Oswaldo Ferreira

Reduce method calls while evaluating Projects::MergeRequestsController#show.json

parent ccfe6860
...@@ -734,10 +734,9 @@ class MergeRequest < ActiveRecord::Base ...@@ -734,10 +734,9 @@ class MergeRequest < ActiveRecord::Base
end end
def has_ci? def has_ci?
has_ci_integration = source_project.try(:ci_service) return false if has_no_commits?
uses_gitlab_ci = all_pipelines.any?
(has_ci_integration || uses_gitlab_ci) && commits.any? !!(head_pipeline_id || all_pipelines.any? || source_project&.ci_service)
end end
def branch_missing? def branch_missing?
......
...@@ -31,7 +31,7 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated ...@@ -31,7 +31,7 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated
end end
def remove_wip_path def remove_wip_path
if can?(current_user, :update_merge_request, merge_request.project) if work_in_progress? && can?(current_user, :update_merge_request, merge_request.project)
remove_wip_project_merge_request_path(project, merge_request) remove_wip_project_merge_request_path(project, merge_request)
end end
end end
......
...@@ -18,7 +18,6 @@ class MergeRequestEntity < IssuableEntity ...@@ -18,7 +18,6 @@ class MergeRequestEntity < IssuableEntity
expose :closed_event, using: EventEntity expose :closed_event, using: EventEntity
# User entities # User entities
expose :author, using: UserEntity
expose :merge_user, using: UserEntity expose :merge_user, using: UserEntity
# Diff sha's # Diff sha's
...@@ -26,7 +25,6 @@ class MergeRequestEntity < IssuableEntity ...@@ -26,7 +25,6 @@ class MergeRequestEntity < IssuableEntity
merge_request.diff_head_sha if merge_request.diff_head_commit merge_request.diff_head_sha if merge_request.diff_head_commit
end end
expose :merge_commit_sha
expose :merge_commit_message expose :merge_commit_message
expose :head_pipeline, with: PipelineDetailsEntity, as: :pipeline expose :head_pipeline, with: PipelineDetailsEntity, as: :pipeline
......
...@@ -2,13 +2,13 @@ ...@@ -2,13 +2,13 @@
%h2.merge-requests-title %h2.merge-requests-title
= pluralize(@merge_requests.count, 'Related Merge Request') = pluralize(@merge_requests.count, 'Related Merge Request')
%ul.unstyled-list.related-merge-requests %ul.unstyled-list.related-merge-requests
- has_any_ci = @merge_requests.any?(&:head_pipeline) - has_any_head_pipeline = @merge_requests.any?(&:head_pipeline_id)
- @merge_requests.each do |merge_request| - @merge_requests.each do |merge_request|
%li %li
%span.merge-request-ci-status %span.merge-request-ci-status
- if merge_request.head_pipeline - if merge_request.head_pipeline
= render_pipeline_status(merge_request.head_pipeline) = render_pipeline_status(merge_request.head_pipeline)
- elsif has_any_ci - elsif has_any_head_pipeline
= icon('blank fw') = icon('blank fw')
%span.merge-request-id %span.merge-request-id
= merge_request.to_reference = merge_request.to_reference
......
...@@ -96,18 +96,6 @@ describe Projects::MergeRequestsController do ...@@ -96,18 +96,6 @@ describe Projects::MergeRequestsController do
expect(response).to match_response_schema('entities/merge_request') expect(response).to match_response_schema('entities/merge_request')
end end
end end
context 'number of queries', :request_store do
it 'verifies number of queries' do
# pre-create objects
merge_request
recorded = ActiveRecord::QueryRecorder.new { go(format: :json) }
expect(recorded.count).to be_within(5).of(30)
expect(recorded.cached_count).to eq(0)
end
end
end end
describe "as diff" do describe "as diff" do
......
...@@ -93,7 +93,7 @@ ...@@ -93,7 +93,7 @@
"merge_commit_message_with_description": { "type": "string" }, "merge_commit_message_with_description": { "type": "string" },
"diverged_commits_count": { "type": "integer" }, "diverged_commits_count": { "type": "integer" },
"commit_change_content_path": { "type": "string" }, "commit_change_content_path": { "type": "string" },
"remove_wip_path": { "type": "string" }, "remove_wip_path": { "type": ["string", "null"] },
"commits_count": { "type": "integer" }, "commits_count": { "type": "integer" },
"remove_source_branch": { "type": ["boolean", "null"] }, "remove_source_branch": { "type": ["boolean", "null"] },
"merge_ongoing": { "type": "boolean" } "merge_ongoing": { "type": "boolean" }
......
...@@ -791,6 +791,49 @@ describe MergeRequest do ...@@ -791,6 +791,49 @@ describe MergeRequest do
end end
end end
describe '#has_ci?' do
let(:merge_request) { build_stubbed(:merge_request) }
context 'has ci' do
it 'returns true if MR has head_pipeline_id and commits' do
allow(merge_request).to receive_message_chain(:source_project, :ci_service) { nil }
allow(merge_request).to receive(:head_pipeline_id) { double }
allow(merge_request).to receive(:has_no_commits?) { false }
expect(merge_request.has_ci?).to be(true)
end
it 'returns true if MR has any pipeline and commits' do
allow(merge_request).to receive_message_chain(:source_project, :ci_service) { nil }
allow(merge_request).to receive(:head_pipeline_id) { nil }
allow(merge_request).to receive(:has_no_commits?) { false }
allow(merge_request).to receive(:all_pipelines) { [double] }
expect(merge_request.has_ci?).to be(true)
end
it 'returns true if MR has CI service and commits' do
allow(merge_request).to receive_message_chain(:source_project, :ci_service) { double }
allow(merge_request).to receive(:head_pipeline_id) { nil }
allow(merge_request).to receive(:has_no_commits?) { false }
allow(merge_request).to receive(:all_pipelines) { [] }
expect(merge_request.has_ci?).to be(true)
end
end
context 'has no ci' do
it 'returns false if MR has no CI service nor pipeline, and no commits' do
allow(merge_request).to receive_message_chain(:source_project, :ci_service) { nil }
allow(merge_request).to receive(:head_pipeline_id) { nil }
allow(merge_request).to receive(:all_pipelines) { [] }
allow(merge_request).to receive(:has_no_commits?) { true }
expect(merge_request.has_ci?).to be(false)
end
end
end
describe '#all_pipelines' do describe '#all_pipelines' do
shared_examples 'returning pipelines with proper ordering' do shared_examples 'returning pipelines with proper ordering' do
let!(:all_pipelines) do let!(:all_pipelines) do
......
...@@ -300,6 +300,10 @@ describe MergeRequestPresenter do ...@@ -300,6 +300,10 @@ describe MergeRequestPresenter do
described_class.new(resource, current_user: user).remove_wip_path described_class.new(resource, current_user: user).remove_wip_path
end end
before do
allow(resource).to receive(:work_in_progress?).and_return(true)
end
context 'when merge request enabled and has permission' do context 'when merge request enabled and has permission' do
it 'has remove_wip_path' do it 'has remove_wip_path' do
allow(project).to receive(:merge_requests_enabled?) { true } allow(project).to receive(:merge_requests_enabled?) { true }
......
...@@ -11,16 +11,6 @@ describe MergeRequestEntity do ...@@ -11,16 +11,6 @@ describe MergeRequestEntity do
described_class.new(resource, request: request).as_json described_class.new(resource, request: request).as_json
end end
it 'includes author' do
req = double('request')
author_payload = UserEntity
.represent(resource.author, request: req)
.as_json
expect(subject[:author]).to eq(author_payload)
end
it 'includes pipeline' do it 'includes pipeline' do
req = double('request', current_user: user) req = double('request', current_user: user)
pipeline = build_stubbed(:ci_pipeline) pipeline = build_stubbed(:ci_pipeline)
......
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