Merge branch '29534-todos-performance' into 'master'

Todos performance: Include associations in Finder

See merge request !10076

4 files changed, 21 insertions, 8 deletions

diff --git a/app/controllers/dashboard/todos_controller.rb b/app/controllers/dashboard/todos_controller.rb
index 498690e8f11..096de8032ae 100644
--- a/app/controllers/dashboard/todos_controller.rb
+++ b/app/controllers/dashboard/todos_controller.rb
@@ -51,7 +51,7 @@ class Dashboard::TodosController < Dashboard::ApplicationController
   private
 
   def find_todos
-    @todos ||= TodosFinder.new(current_user, params).execute
+    @todos ||= TodosFinder.new(current_user, params.merge(include_associations: true)).execute
   end
 
   def todos_counts
diff --git a/app/finders/todos_finder.rb b/app/finders/todos_finder.rb
index b7f091f334d..13d33a1c31b 100644
--- a/app/finders/todos_finder.rb
+++ b/app/finders/todos_finder.rb
@@ -24,6 +24,7 @@ class TodosFinder
 
   def execute
     items = current_user.todos
+    items = include_associations(items)
     items = by_action_id(items)
     items = by_action(items)
     items = by_author(items)
@@ -38,6 +39,17 @@ class TodosFinder
 
   private
 
+  def include_associations(items)
+    return items unless params[:include_associations]
+
+    items.includes(
+      [
+        target: { project: [:route, namespace: :route] },
+        author: { namespace: :route },
+      ]
+    )
+  end
+
   def action_id?
     action_id.present? && Todo::ACTION_NAMES.has_key?(action_id.to_i)
   end
diff --git a/app/helpers/todos_helper.rb b/app/helpers/todos_helper.rb
index 4f5adf623f2..847a8fdfca6 100644
--- a/app/helpers/todos_helper.rb
+++ b/app/helpers/todos_helper.rb
@@ -39,9 +39,13 @@ module TodosHelper
       namespace_project_commit_path(todo.project.namespace.becomes(Namespace), todo.project,
                                     todo.target, anchor: anchor)
     else
-      path = [todo.project.namespace.becomes(Namespace), todo.project, todo.target]
-
-      path.unshift(:pipelines) if todo.build_failed?
+      if todo.build_failed?
+        # associated namespace and route would be loaded from the db again if todo.project was used
+        project = todo.target.project
+        path = [:pipelines, project.namespace.becomes(Namespace), project, todo.target]
+      else
+        path = [todo.target]
+      end
 
       polymorphic_path(path, anchor: anchor)
     end
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 4759829a15c..cef8ad76b07 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -7,6 +7,7 @@ class MergeRequest < ActiveRecord::Base
 
   belongs_to :target_project, class_name: "Project"
   belongs_to :source_project, class_name: "Project"
+  belongs_to :project, foreign_key: :target_project_id
   belongs_to :merge_user, class_name: "User"
 
   has_many :merge_request_diffs, dependent: :destroy
@@ -540,10 +541,6 @@ class MergeRequest < ActiveRecord::Base
     target_project != source_project
   end
 
-  def project
-    target_project
-  end
-
   # If the merge request closes any issues, save this information in the
   # `MergeRequestsClosingIssues` model. This is a performance optimization.
   # Calculating this information for a number of merge requests requires