Merge pull request #2191 from TracksApp/refactor-not-done-todos-query

Refactor out an object for querying not done todos

app/controllers/todos_controller.rb

@@ -1316,39 +1316,7 @@ end
   end
 
   def get_not_done_todos
-    if params[:done]
-      not_done_todos = current_user.todos.completed.completed_after(Time.zone.now - params[:done].to_i.days)
-    else
-      not_done_todos = current_user.todos.active.not_hidden
-    end
-
-    not_done_todos = not_done_todos.
-      reorder(Arel.sql("todos.due IS NULL, todos.due ASC, todos.created_at ASC")).
-      includes(Todo::DEFAULT_INCLUDES)
-
-    not_done_todos = not_done_todos.limit(sanitize(params[:limit])) if params[:limit]
-
-    if params[:due]
-      due_within_when = Time.zone.now + params['due'].to_i.days
-      not_done_todos = not_done_todos.where('todos.due <= ?', due_within_when)
-    end
-
-    if params[:tag]
-      tag = Tag.where(:name => params['tag']).first
-      not_done_todos = not_done_todos.where('taggings.tag_id = ?', tag.id)
-    end
-
-    if params[:context_id]
-      context = current_user.contexts.find(params[:context_id])
-      not_done_todos = not_done_todos.where('context_id' => context.id)
-    end
-
-    if params[:project_id]
-      project = current_user.projects.find(params[:project_id])
-      not_done_todos = not_done_todos.where('project_id' => project)
-    end
-
-    return not_done_todos
+    Todos::UndoneTodosQuery.new(current_user).query(params)
   end
 
   def onsite_redirect_to(destination)

app/models/todos/undone_todos_query.rb

@@ -0,0 +1,46 @@
+module Todos
+  class UndoneTodosQuery
+    include ActionView::Helpers::SanitizeHelper
+
+    attr_reader :current_user
+    def initialize(current_user)
+      @current_user = current_user
+    end
+
+    def query(params)
+      if params[:done]
+        not_done_todos = current_user.todos.completed.completed_after(Time.zone.now - params[:done].to_i.days)
+      else
+        not_done_todos = current_user.todos.active.not_hidden
+      end
+
+      not_done_todos = not_done_todos.
+        reorder(Arel.sql("todos.due IS NULL, todos.due ASC, todos.created_at ASC")).
+        includes(Todo::DEFAULT_INCLUDES)
+
+      not_done_todos = not_done_todos.limit(sanitize(params[:limit])) if params[:limit]
+
+      if params[:due]
+        due_within_when = Time.zone.now + params[:due].to_i.days
+        not_done_todos = not_done_todos.where('todos.due <= ?', due_within_when)
+      end
+
+      if params[:tag]
+        tag = Tag.where(:name => params[:tag]).first
+        not_done_todos = not_done_todos.joins(:taggings).where('taggings.tag_id = ?', tag.id)
+      end
+
+      if params[:context_id]
+        context = current_user.contexts.find(params[:context_id])
+        not_done_todos = not_done_todos.where('context_id' => context.id)
+      end
+
+      if params[:project_id]
+        project = current_user.projects.find(params[:project_id])
+        not_done_todos = not_done_todos.where('project_id' => project)
+      end
+
+      return not_done_todos
+    end
+  end
+end

test/fixtures/projects.yml

@@ -56,3 +56,13 @@ attendrailsconf:
   user_id: 2
   created_at: <%= today %>
   updated_at: <%= today %>
+
+attendgophercon:
+  id: 5
+  name: Attend Gophercon
+  description: 'Because those little gopher drawing are cute'
+  position: 2
+  state: 'active'
+  user_id: 2
+  created_at: <%= today %>
+  updated_at: <%= today %>

test/fixtures/taggings.yml

@@ -22,4 +22,16 @@ foo2:
   id: 4
   tag_id: 1
   taggable_id: 3 # Buy milk - completed
-  taggable_type: Todo
+  taggable_type: Todo
+
+bar1:
+  id: 5
+  tag_id: 2
+  taggable_id: 16
+  taggable_type: Todo
+
+bar2:
+  tag_id: 2
+  taggable_id: 20
+  taggable_type: Todo
+

test/fixtures/todos.yml

@@ -4,10 +4,17 @@
 # rails does automatically in models or controllers! Convert to utc manually!
 
 <%
+def yesterday
+  Time.zone.now.utc.beginning_of_day - 1.day
+end
 def today
   Time.zone.now.utc.beginning_of_day.to_s(:db)
 end
 
+def tomorrow
+  (Time.zone.now.utc.beginning_of_day + 1.day).to_s(:db)
+end
+
 def next_week
   1.week.from_now.utc.beginning_of_day.to_s(:db)
 end
@@ -255,3 +262,29 @@ email_broker:
   description: Ask about better stocks
   notes: ~
   state: pending
+
+package_delivered:
+  id: 20
+  context_id: 11
+  project_id: 5
+  description: Package delivery date
+  notes: ~
+  state: active
+  created_at: <%= two_weeks_ago %>
+  due: <%= today %>
+  completed_at: ~
+  show_from: ~
+  user_id: 2
+
+assemble_furniture:
+  id: 21
+  context_id: 11
+  project_id: 5
+  description: Put together the furniture we bought
+  notes: ~
+  state: completed
+  created_at: <%= two_weeks_ago %>
+  due: <%= today %>
+  completed_at: <%= yesterday %>
+  show_from: ~
+  user_id: 2

test/models/todos/undone_todos_query_test.rb

@@ -0,0 +1,90 @@
+require 'test_helper'
+
+module Todos
+  class UndoneTodosQueryTest < ActiveSupport::TestCase
+    def test_requires_a_user
+      assert_raises(ArgumentError) { UndoneTodosQuery.new }
+    end
+
+    def test_default_query_is_all_active_not_hidden_todos
+      user = users(:other_user)
+      undone_todos = UndoneTodosQuery.new(user).query({})
+      expected = [todos(:package_delivered),
+                  todos(:buy_tix),
+                  todos(:pal_confirmation)]
+      assert_equal expected, undone_todos.to_a
+    end
+
+    def test_filtering_by_done
+      user = users(:other_user)
+      # This gets everything done from a week ago until now
+      undone_todos = UndoneTodosQuery.new(user).query(done: '7')
+      expected = [todos(:assemble_furniture)]
+      assert_equal expected, undone_todos.to_a
+    end
+
+    def test_limiting_results
+      user = users(:other_user)
+      undone_todos = UndoneTodosQuery.new(user).query(limit: '1')
+      expected = [todos(:package_delivered)]
+      assert_equal expected, undone_todos.to_a
+    end
+
+    def test_filtering_by_due_date
+      user = users(:other_user)
+      # Only gets todos that are due today or are past their due date.
+      undone_todos = UndoneTodosQuery.new(user).query(due: '0')
+      expected = [todos(:package_delivered)]
+      assert_equal expected, undone_todos.to_a
+    end
+
+    def test_filtering_by_tag
+      user = users(:other_user)
+      undone_todos = UndoneTodosQuery.new(user).query(tag: 'bar')
+      expected = [todos(:package_delivered),
+                  todos(:buy_tix)]
+      assert_equal expected, undone_todos.to_a
+    end
+
+    def test_filtering_by_context
+      user = users(:other_user)
+      undone_todos = UndoneTodosQuery.new(user).query(context_id: '11')
+      expected = [todos(:package_delivered),
+                  todos(:pal_confirmation)]
+      assert_equal expected, undone_todos.to_a
+    end
+
+    def test_using_a_non_existant_context_raises_an_exception
+      user = users(:other_user)
+      assert_raises(ActiveRecord::RecordNotFound) do
+        undone_todos = UndoneTodosQuery.new(user).query(context_id: '110')
+      end
+    end
+
+    def test_filtering_by_project
+      user = users(:other_user)
+      undone_todos = UndoneTodosQuery.new(user).query(project_id: '5')
+      expected = [todos(:package_delivered)]
+      assert_equal expected, undone_todos.to_a
+    end
+
+    def test_using_a_non_existant_project_raises_an_exception
+      user = users(:other_user)
+      assert_raises(ActiveRecord::RecordNotFound) do
+        undone_todos = UndoneTodosQuery.new(user).query(project_id: '110')
+      end
+    end
+
+    def test_combination_of_all_params
+      user = users(:other_user)
+      undone_todos = UndoneTodosQuery.new(user).query({
+        limit: "1",
+        project_id: "5",
+        context_id: "11",
+        tag: "bar",
+        due: "0"})
+      expected = [todos(:package_delivered)]
+      assert_equal expected, undone_todos.to_a
+    end
+  end
+end