Commit bf41a2e7 authored by Jarka Kadlecova's avatar Jarka Kadlecova

Todos performance: Include associations in Finder

parent 691402fb
......@@ -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
......
......@@ -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
......
......@@ -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
......
......@@ -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
......
......@@ -4,6 +4,7 @@ FactoryGirl.define do
author
association :source_project, :repository, factory: :project
target_project { source_project }
project { target_project }
# $ git log --pretty=oneline feature..master
# 5937ac0a7beb003549fc5fd26fc247adbce4a52e Add submodule from gitlab.com
......
require "spec_helper"
describe TodosHelper do
include GitlabRoutingHelper
describe '#todo_target_path' do
let(:project) { create(:project) }
let(:merge_request) { create(:merge_request, target_project: project, source_project: project) }
let(:issue) { create(:issue, project: project) }
let(:note) { create(:note_on_issue, noteable: issue, project: project) }
let(:mr_todo) { build(:todo, project: project, target: merge_request) }
let(:issue_todo) { build(:todo, project: project, target: issue) }
let(:note_todo) { build(:todo, project: project, target: issue, note: note) }
let(:build_failed_todo) { build(:todo, :build_failed, project: project, target: merge_request) }
it 'returns correct path to the todo MR' do
expect(todo_target_path(mr_todo)).
to eq("/#{project.full_path}/merge_requests/#{merge_request.iid}")
end
it 'returns correct path to the todo issue' do
expect(todo_target_path(issue_todo)).
to eq("/#{project.full_path}/issues/#{issue.iid}")
end
it 'returns correct path to the todo note' do
expect(todo_target_path(note_todo)).
to eq("/#{project.full_path}/issues/#{issue.iid}#note_#{note.id}")
end
it 'returns correct path to build_todo MR when pipeline failed' do
expect(todo_target_path(build_failed_todo)).
to eq("/#{project.full_path}/merge_requests/#{merge_request.iid}/pipelines")
end
end
describe '#todo_projects_options' do
let(:projects) { create_list(:empty_project, 3) }
let(:user) { create(:user) }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment