From 222a6b4680b19f578b42b32d908a1d3cb8e343f7 Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Tue, 11 Jul 2017 10:53:58 +0100 Subject: [PATCH] Don't reload ActiveRecord objects when building note URLs When we build a note URL, and we have the note loaded already, there are two cases: 1. The `noteable` is already loaded. In that case, this is faster as it doesn't build a new AR object from the query. 2. The `noteable` is not already loaded. In that case, this change is no worse than the previous code. --- changelogs/unreleased/fix-n-plus-one-in-url-builder.yml | 4 ++++ lib/gitlab/url_builder.rb | 8 +++----- 2 files changed, 7 insertions(+), 5 deletions(-) create mode 100644 changelogs/unreleased/fix-n-plus-one-in-url-builder.yml diff --git a/changelogs/unreleased/fix-n-plus-one-in-url-builder.yml b/changelogs/unreleased/fix-n-plus-one-in-url-builder.yml new file mode 100644 index 00000000000..5781316cfd9 --- /dev/null +++ b/changelogs/unreleased/fix-n-plus-one-in-url-builder.yml @@ -0,0 +1,4 @@ +--- +title: Improve issue rendering performance with lots of notes from other users +merge_request: +author: diff --git a/lib/gitlab/url_builder.rb b/lib/gitlab/url_builder.rb index 35792d2d67f..824e2d7251f 100644 --- a/lib/gitlab/url_builder.rb +++ b/lib/gitlab/url_builder.rb @@ -52,15 +52,13 @@ module Gitlab commit_url(id: object.commit_id, anchor: dom_id(object)) elsif object.for_issue? - issue = Issue.find(object.noteable_id) - issue_url(issue, anchor: dom_id(object)) + issue_url(object.noteable, anchor: dom_id(object)) elsif object.for_merge_request? - merge_request = MergeRequest.find(object.noteable_id) - merge_request_url(merge_request, anchor: dom_id(object)) + merge_request_url(object.noteable, anchor: dom_id(object)) elsif object.for_snippet? - snippet = Snippet.find(object.noteable_id) + snippet = object.noteable if snippet.is_a?(PersonalSnippet) snippet_url(snippet, anchor: dom_id(object))