Clean up ActiveRecord code in TodoService

This refactors the TodoService class according to our code reuse
guidelines. The resulting code is a wee bit more verbose, but it allows
us to decouple the column names from the input, resulting in fewer
changes being necessary when we change the schema.

One particular noteworthy line in TodoService is the following:

    todos_ids = todos.update_state(state)

Technically this is a violation of the guidelines, because
`update_state` is a class method, which services are not supposed to use
(safe for a few allowed ones). I decided to keep this, since there is no
alternative. `update_state` doesn't produce a relation so it doesn't
belong in a Finder, and we can't move it to another Service either. As
such I opted to just use the method directly.

Cases like this may happen more frequently, at which point we should
update our documentation with some sort of recommendation. For now, I
want to refrain from doing so until we have a few more examples.

3 files changed, 87 insertions, 0 deletions

diff --git a/app/finders/pending_todos_finder.rb b/app/finders/pending_todos_finder.rb
new file mode 100644
index 00000000000..c21d90c9182
--- /dev/null
+++ b/app/finders/pending_todos_finder.rb
@@ -0,0 +1,64 @@
+# frozen_string_literal: true
+
+# Finder for retrieving the pending todos of a user, optionally filtered using
+# various fields.
+#
+# While this finder is a bit more verbose compared to use
+# `where(params.slice(...))`, it allows us to decouple the input parameters from
+# the actual column names. For example, if we ever decide to use separate
+# columns for target types (e.g. `issue_id`, `merge_request_id`, etc), we no
+# longer need to change _everything_ that uses this finder. Instead, we just
+# change the various `by_*` methods in this finder, without having to touch
+# everything that uses it.
+class PendingTodosFinder
+  attr_reader :current_user, :params
+
+  # current_user - The user to retrieve the todos for.
+  # params - A Hash containing columns and values to use for filtering todos.
+  def initialize(current_user, params = {})
+    @current_user = current_user
+    @params = params
+  end
+
+  def execute
+    todos = current_user.todos.pending
+    todos = by_project(todos)
+    todos = by_target_id(todos)
+    todos = by_target_type(todos)
+    todos = by_commit_id(todos)
+
+    todos
+  end
+
+  def by_project(todos)
+    if (id = params[:project_id])
+      todos.for_project(id)
+    else
+      todos
+    end
+  end
+
+  def by_target_id(todos)
+    if (id = params[:target_id])
+      todos.for_target(id)
+    else
+      todos
+    end
+  end
+
+  def by_target_type(todos)
+    if (type = params[:target_type])
+      todos.for_type(type)
+    else
+      todos
+    end
+  end
+
+  def by_commit_id(todos)
+    if (id = params[:commit_id])
+      todos.for_commit(id)
+    else
+      todos
+    end
+  end
+end
diff --git a/app/finders/todos_finder.rb b/app/finders/todos_finder.rb
index 0e37f946d35..d001e18fea9 100644
--- a/app/finders/todos_finder.rb
+++ b/app/finders/todos_finder.rb
@@ -47,6 +47,13 @@ class TodosFinder
     sort(items)
   end
 
+  # Returns `true` if the current user has any todos for the given target.
+  #
+  # target - The value of the `target_type` column, such as `Issue`.
+  def any_for_target?(target)
+    current_user.todos.any_for_target?(target)
+  end
+
   private
 
   def action_id?
diff --git a/app/finders/users_with_pending_todos_finder.rb b/app/finders/users_with_pending_todos_finder.rb
new file mode 100644
index 00000000000..461bd92a366
--- /dev/null
+++ b/app/finders/users_with_pending_todos_finder.rb
@@ -0,0 +1,16 @@
+# frozen_string_literal: true
+
+# Finder that given a target (e.g. an issue) finds all the users that have
+# pending todos for said target.
+class UsersWithPendingTodosFinder
+  attr_reader :target
+
+  # target - The target, such as an Issue or MergeRequest.
+  def initialize(target)
+    @target = target
+  end
+
+  def execute
+    User.for_todos(target.todos.pending)
+  end
+end