Omit issues links in merge request entity API response
The merge request widget has a section that includes which issues may be closed or mentioned based on the merge request description. The problem is that rendering and redacting Markdown can be expensive, especially since the browser polls for the data every 10 seconds. Since these links don't change much and are just nice to have, we only load them on first page load. The frontend will use the existing data if the data doesn't appear on subsequent requests. This saves about 30% of the rendering time of this endpoint, which adds up to significant savings considering that `MergeRequestsController#show.json` is called over a million times a day on GitLab.com. Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/63546
This commit is contained in:
parent
61a2ed5d0e
commit
1b7ab11f94
5 changed files with 25 additions and 4 deletions
|
@ -74,7 +74,7 @@ module IssuablesHelper
|
|||
end
|
||||
end
|
||||
|
||||
def serialize_issuable(issuable, serializer: nil)
|
||||
def serialize_issuable(issuable, opts = {})
|
||||
serializer_klass = case issuable
|
||||
when Issue
|
||||
IssueSerializer
|
||||
|
@ -84,7 +84,7 @@ module IssuablesHelper
|
|||
|
||||
serializer_klass
|
||||
.new(current_user: current_user, project: issuable.project)
|
||||
.represent(issuable, serializer: serializer)
|
||||
.represent(issuable, opts)
|
||||
.to_json
|
||||
end
|
||||
|
||||
|
|
|
@ -114,7 +114,10 @@ class MergeRequestWidgetEntity < IssuableEntity
|
|||
presenter(merge_request).ci_status
|
||||
end
|
||||
|
||||
expose :issues_links do
|
||||
# Rendering and redacting Markdown can be expensive. These links are
|
||||
# just nice to have in the merge request widget, so only
|
||||
# include them if they are explicitly requested on first load.
|
||||
expose :issues_links, if: -> (_, opts) { opts[:issues_links] } do
|
||||
expose :assign_to_closing do |merge_request|
|
||||
presenter(merge_request).assign_to_closing_issues_link
|
||||
end
|
||||
|
|
|
@ -19,7 +19,7 @@
|
|||
-# haml-lint:disable InlineJavaScript
|
||||
:javascript
|
||||
window.gl = window.gl || {};
|
||||
window.gl.mrWidgetData = #{serialize_issuable(@merge_request, serializer: 'widget')}
|
||||
window.gl.mrWidgetData = #{serialize_issuable(@merge_request, serializer: 'widget', issues_links: true)}
|
||||
|
||||
window.gl.mrWidgetData.squash_before_merge_help_path = '#{help_page_path("user/project/merge_requests/squash_and_merge")}';
|
||||
window.gl.mrWidgetData.troubleshooting_docs_path = '#{help_page_path('user/project/merge_requests/index.md', anchor: 'troubleshooting')}';
|
||||
|
|
5
changelogs/unreleased/sh-omit-issues-links-on-poll.yml
Normal file
5
changelogs/unreleased/sh-omit-issues-links-on-poll.yml
Normal file
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
title: Omit issues links in merge request entity API response
|
||||
merge_request: 29917
|
||||
author:
|
||||
type: performance
|
|
@ -32,6 +32,19 @@ describe MergeRequestWidgetEntity do
|
|||
end
|
||||
end
|
||||
|
||||
describe 'issues links' do
|
||||
it 'includes issues links when requested' do
|
||||
data = described_class.new(resource, request: request, issues_links: true).as_json
|
||||
|
||||
expect(data).to include(:issues_links)
|
||||
expect(data[:issues_links]).to include(:assign_to_closing, :closing, :mentioned_but_not_closing)
|
||||
end
|
||||
|
||||
it 'omits issue links by default' do
|
||||
expect(subject).not_to include(:issues_links)
|
||||
end
|
||||
end
|
||||
|
||||
describe 'pipeline' do
|
||||
let(:pipeline) { create(:ci_empty_pipeline, project: project, ref: resource.source_branch, sha: resource.source_branch_sha, head_pipeline_of: resource) }
|
||||
|
||||
|
|
Loading…
Reference in a new issue