From bf41a2e7525cc952686623b508023c169dbdfe2d Mon Sep 17 00:00:00 2001 From: Jarka Kadlecova Date: Mon, 20 Mar 2017 09:20:46 +0100 Subject: Todos performance: Include associations in Finder --- app/controllers/dashboard/todos_controller.rb | 2 +- app/finders/todos_finder.rb | 12 ++++++++++++ app/helpers/todos_helper.rb | 10 +++++++--- app/models/merge_request.rb | 5 +---- 4 files changed, 21 insertions(+), 8 deletions(-) (limited to 'app') 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 -- cgit v1.2.3