diff options
| -rw-r--r-- | app/controllers/dashboard/todos_controller.rb | 2 | ||||
| -rw-r--r-- | app/finders/todos_finder.rb | 12 | ||||
| -rw-r--r-- | app/helpers/todos_helper.rb | 10 | ||||
| -rw-r--r-- | app/models/merge_request.rb | 5 | ||||
| -rw-r--r-- | spec/factories/merge_requests.rb | 1 | ||||
| -rw-r--r-- | spec/helpers/todos_helper_spec.rb | 34 |
6 files changed, 8 insertions, 56 deletions
diff --git a/app/controllers/dashboard/todos_controller.rb b/app/controllers/dashboard/todos_controller.rb index 096de8032ae..498690e8f11 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.merge(include_associations: true)).execute + @todos ||= TodosFinder.new(current_user, params).execute end def todos_counts diff --git a/app/finders/todos_finder.rb b/app/finders/todos_finder.rb index 13d33a1c31b..b7f091f334d 100644 --- a/app/finders/todos_finder.rb +++ b/app/finders/todos_finder.rb @@ -24,7 +24,6 @@ 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) @@ -39,17 +38,6 @@ 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 847a8fdfca6..4f5adf623f2 100644 --- a/app/helpers/todos_helper.rb +++ b/app/helpers/todos_helper.rb @@ -39,13 +39,9 @@ module TodosHelper namespace_project_commit_path(todo.project.namespace.becomes(Namespace), todo.project, todo.target, anchor: anchor) else - 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 + path = [todo.project.namespace.becomes(Namespace), todo.project, todo.target] + + path.unshift(:pipelines) if todo.build_failed? polymorphic_path(path, anchor: anchor) end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index cef8ad76b07..4759829a15c 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -7,7 +7,6 @@ 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 @@ -541,6 +540,10 @@ 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 diff --git a/spec/factories/merge_requests.rb b/spec/factories/merge_requests.rb index 21487541507..ae0bbbd6aeb 100644 --- a/spec/factories/merge_requests.rb +++ b/spec/factories/merge_requests.rb @@ -4,7 +4,6 @@ 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 diff --git a/spec/helpers/todos_helper_spec.rb b/spec/helpers/todos_helper_spec.rb index 21e0e74e008..50060a0925d 100644 --- a/spec/helpers/todos_helper_spec.rb +++ b/spec/helpers/todos_helper_spec.rb @@ -1,40 +1,6 @@ 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) } |