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.

Don't reload ActiveRecord objects when building note URLs
222a6b46 · Sean McGivern · c17b1d5f · 222a6b46 · 222a6b46
Commit 222a6b46 authored Jul 11, 2017 by Sean McGivern
Hide whitespace changes
Inline Side-by-side

Showing with 7 additions and 5 deletions

fix-n-plus-one-in-url-builder.yml changelogs/unreleased/fix-n-plus-one-in-url-builder.yml +4 -0

url_builder.rb lib/gitlab/url_builder.rb +3 -5

No files found.
--- a/changelogs/unreleased/fix-n-plus-one-in-url-builder.yml
+++ b/changelogs/unreleased/fix-n-plus-one-in-url-builder.yml
+---
+title: Improve issue rendering performance with lots of notes from other users
+merge_request:
+author:
--- 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(object.noteable, anchor: dom_id(object))
-        issue_url(issue, anchor: dom_id(object))
      elsif object.for_merge_request?
-        merge_request = MergeRequest.find(object.noteable_id)
+        merge_request_url(object.noteable, anchor: dom_id(object))
-        merge_request_url(merge_request, 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))