Don't use ProjectsFinder in TodosFinder

Using ProjectsFinder in TodosFinder to limit todos to the right projects leads to overly complicated and slow database queries. This commit removes the use of ProjectsFinder in favour of using Project.public_or_visible_to_current_user directly. Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/43767

Don't use ProjectsFinder in TodosFinder
f09fe848 · Yorick Peterse · de454de9 · f09fe848 · f09fe848
Unverified Commit f09fe848 authored Mar 01, 2018 by Yorick Peterse
Hide whitespace changes
Inline Side-by-side

Showing with 11 additions and 9 deletions

todos_finder.rb app/finders/todos_finder.rb +6 -9

remove-projects-finder-from-todos-finder.yml ...s/unreleased/remove-projects-finder-from-todos-finder.yml +5 -0

No files found.
--- a/app/finders/todos_finder.rb
+++ b/app/finders/todos_finder.rb
@@ -110,10 +110,6 @@ class TodosFinder
    ids
  end

-  def projects(items)
-    ProjectsFinder.new(current_user: current_user, project_ids_relation: project_ids(items)).execute
-  end
-
  def type?
    type.present? && %w(Issue MergeRequest).include?(type)
  end
@@ -152,13 +148,14 @@ class TodosFinder

  def by_project(items)
    if project?
-      items = items.where(project: project)
+      items.where(project: project)
    else
-      item_projects = projects(items)
-      items = items.merge(item_projects).joins(:project)
-    end
+      projects = Project
+        .public_or_visible_to_user(current_user)
+        .order_id_desc

-    items
+      items.joins(:project).merge(projects)
+    end
  end

  def by_state(items)

--- a/changelogs/unreleased/remove-projects-finder-from-todos-finder.yml
+++ b/changelogs/unreleased/remove-projects-finder-from-todos-finder.yml
+---
+title: Don't use ProjectsFinder in TodosFinder
+merge_request:
+author:
+type: performance