several fixes to recurring todos and adds some named_scopes

fixes case where unmarking a complete todo which belongs to a recurring pattern results in two todos that both keep on recurring. With this fix a new todo will only be created when there are no active todos left belonging to that recurring pattern

fixes tests that failed because of previous commits

adds some named_scopes, inspired by http://railscasts.com/episodes/108

app/controllers/recurring_todos_controller.rb

@@ -6,8 +6,10 @@ class RecurringTodosController < ApplicationController
   append_before_filter :get_recurring_todo_from_param, :only => [:destroy, :toggle_check, :toggle_star, :edit, :update]
 
   def index
-    @recurring_todos = current_user.recurring_todos.find(:all, :conditions => ["state = ?", "active"])
+    find_and_inactivate
-    @completed_recurring_todos = current_user.recurring_todos.find(:all, :conditions => ["state = ?", "completed"])
+
+    @recurring_todos = current_user.recurring_todos.active
+    @completed_recurring_todos = current_user.recurring_todos.completed
     @no_recurring_todos = @recurring_todos.size == 0
     @no_completed_recurring_todos = @completed_recurring_todos.size == 0
     @count = @recurring_todos.size 
@@ -129,7 +131,7 @@ class RecurringTodosController < ApplicationController
   def destroy
     
     # remove all references to this recurring todo
-    @todos = current_user.todos.find(:all, {:conditions => ["recurring_todo_id = ?", params[:id]]})
+    @todos = @recurring_todo.todos
     @number_of_todos = @todos.size
     @todos.each do |t|
       t.recurring_todo_id = nil
@@ -161,14 +163,15 @@ class RecurringTodosController < ApplicationController
   def toggle_check
     @saved = @recurring_todo.toggle_completion!
 
-    @count = current_user.recurring_todos.count(:all, :conditions => ["state = ?", "active"])
+    @count = current_user.recurring_todos.active.count
     @remaining = @count
 
     if @recurring_todo.active?
-      @remaining = current_user.recurring_todos.count(:all, :conditions => ["state = ?", 'completed']) 
+      @remaining = current_user.recurring_todos.completed.count
       
       # from completed back to active -> check if there is an active todo
-      @active_todos = current_user.todos.count(:all, {:conditions => ["state = ? AND recurring_todo_id = ?", 'active',params[:id]]})
+      # current_user.todos.count(:all, {:conditions => ["state = ? AND recurring_todo_id = ?", 'active',params[:id]]})
+      @active_todos = @recurring_todo.todos.active.count
       # create todo if there is no active todo belonging to the activated
       # recurring_todo
       @new_recurring_todo = create_todo_from_recurring_todo(@recurring_todo) if @active_todos == 0
@@ -255,5 +258,11 @@ class RecurringTodosController < ApplicationController
   def get_recurring_todo_from_param
     @recurring_todo = current_user.recurring_todos.find(params[:id])
   end
+
+  def find_and_inactivate
+    # find active recurring todos without active todos and inactivate them
+    recurring_todos = current_user.recurring_todos.active
+    recurring_todos.each { |rt| rt.toggle_completion! if rt.todos.not_completed.count == 0}
+  end
   
 end

app/controllers/todos_controller.rb

@@ -127,7 +127,7 @@ class TodosController < ApplicationController
   end
 
   # Toggles the 'done' status of the action
-  # 
+  #
   def toggle_check
     @source_view = params['_source_view'] || 'todo'
     @original_item_due = @todo.due
@@ -777,26 +777,31 @@ class TodosController < ApplicationController
     new_recurring_todo = nil
     recurring_todo = nil
     if todo.from_recurring_todo?
-      recurring_todo = current_user.recurring_todos.find(todo.recurring_todo_id)
+      recurring_todo = todo.recurring_todo
+
+      # check if there are active todos belonging to this recurring todo.
+      # only add new one if all active todos are completed
+      if recurring_todo.todos.active.count == 0
       
-      # check for next todo either from the due date or the show_from date
+        # check for next todo either from the due date or the show_from date
-      date_to_check = todo.due.nil? ? todo.show_from : todo.due
+        date_to_check = todo.due.nil? ? todo.show_from : todo.due
       
-      # if both due and show_from are nil, check for a next todo from now
+        # if both due and show_from are nil, check for a next todo from now
-      date_to_check = Time.zone.now if date_to_check.nil?
+        date_to_check = Time.zone.now if date_to_check.nil?
       
-      if recurring_todo.active? && recurring_todo.has_next_todo(date_to_check)
+        if recurring_todo.active? && recurring_todo.has_next_todo(date_to_check)
         
-        # shift the reference date to yesterday if date_to_check is furher in
+          # shift the reference date to yesterday if date_to_check is furher in
-        # the past. This is to make sure we do not get older todos for overdue
+          # the past. This is to make sure we do not get older todos for overdue
-        # todos. I.e. checking a daily todo that is overdue with 5 days will
+          # todos. I.e. checking a daily todo that is overdue with 5 days will
-        # create a new todo which is overdue by 4 days if we don't shift the
+          # create a new todo which is overdue by 4 days if we don't shift the
-        # date. Discard the time part in the compare. We pick yesterday so that
+          # date. Discard the time part in the compare. We pick yesterday so
-        # new todos due for today will be created instead of new todos for
+          # that new todos due for today will be created instead of new todos
-        # tomorrow.
+          # for tomorrow.
-        date = date_to_check.at_midnight >= Time.zone.now.at_midnight ? date_to_check : Time.zone.now-1.day
+          date = date_to_check.at_midnight >= Time.zone.now.at_midnight ? date_to_check : Time.zone.now-1.day
         
-        new_recurring_todo = create_todo_from_recurring_todo(recurring_todo, date) 
+          new_recurring_todo = create_todo_from_recurring_todo(recurring_todo, date)
+        end
       end
     end
     return new_recurring_todo    

app/models/recurring_todo.rb

@@ -4,6 +4,8 @@ class RecurringTodo < ActiveRecord::Base
   belongs_to :project
   belongs_to :user
 
+  has_many :todos
+
   attr_protected :user
 
   acts_as_state_machine :initial => :active, :column => 'state'
@@ -20,6 +22,9 @@ class RecurringTodo < ActiveRecord::Base
 
   validates_presence_of :context
 
+  named_scope :active, :conditions => { :state => 'active'}
+  named_scope :completed, :conditions => { :state => 'completed'}
+
   event :complete do
     transitions :to => :completed, :from => [:active]
   end

app/models/todo.rb

@@ -4,7 +4,10 @@ class Todo < ActiveRecord::Base
   belongs_to :project
   belongs_to :user
   belongs_to :recurring_todo
-  
+
+  named_scope :active, :conditions => { :state => 'active' }
+  named_scope :not_completed, :conditions =>  ['NOT state = ? ', 'completed']
+
   STARRED_TAG_NAME = "starred"
   
   acts_as_state_machine :initial => :active, :column => 'state'

app/views/todos/toggle_check.js.rjs

@@ -24,7 +24,9 @@ if @saved
         page.insert_html :bottom, item_container_id(@new_recurring_todo), :partial => 'todos/todo', :locals => { :todo => @new_recurring_todo, :parent_container_type => parent_container_type }
         page.visual_effect :highlight, dom_id(@new_recurring_todo, 'line'), {'startcolor' => "'#99ff99'"}      
       else
-        page.notify :notice, "There is no next action after the recurring action you just finished. The recurrence is completed", 6.0 if @new_recurring_todo.nil?
+        if @todo.recurring_todo.todos.active.count == 0
+          page.notify :notice, "There is no next action after the recurring action you just finished. The recurrence is completed", 6.0 if @new_recurring_todo.nil?
+        end
       end
     end
         

test/fixtures/todos.yml

@@ -95,7 +95,7 @@ end
   completed_at: ~
   user_id: 1
 
-7:
+book:
   id: 7
   context_id: 6
   project_id: 3

test/functional/projects_controller_test.rb

@@ -223,9 +223,9 @@ class ProjectsControllerTest < TodoContainerControllerTestBase
     login_as :admin_user
     u = users(:admin_user)
     post :actionize, :state => "active", :format => 'js'
-    assert_equal 1, projects(:gardenclean).position 
+    assert_equal 1, projects(:gardenclean).position
-    assert_equal 2, projects(:timemachine).position 
+    assert_equal 2, projects(:moremoney).position
-    assert_equal 3, projects(:moremoney).position
+    assert_equal 3, projects(:timemachine).position
   end
   
   def test_alphabetize_sorts_active_projects_alphabetically

test/functional/todos_controller_test.rb

@@ -180,7 +180,7 @@ class TodosControllerTest < Test::Rails::TestCase
     login_as(:admin_user)
     get :index, { :format => "rss" }
     assert_equal 'application/rss+xml', @response.content_type
-    # #puts @response.body
+    # puts @response.body
 
     assert_xml_select 'rss[version="2.0"]' do
       assert_select 'channel' do
@@ -193,7 +193,7 @@ class TodosControllerTest < Test::Rails::TestCase
           assert_select 'description', /.*/
           assert_select 'link', %r{http://test.host/contexts/.+}
           assert_select 'guid', %r{http://test.host/todos/.+}
-          assert_select 'pubDate', projects(:timemachine).updated_at.to_s(:rfc822)
+          assert_select 'pubDate', todos(:book).updated_at.to_s(:rfc822)
         end
       end
     end
@@ -245,7 +245,7 @@ class TodosControllerTest < Test::Rails::TestCase
       assert_xml_select 'entry', 11 do
         assert_xml_select 'title', /.+/
         assert_xml_select 'content[type="html"]', /.*/
-        assert_xml_select 'published', /(#{Regexp.escape(projects(:timemachine).updated_at.xmlschema)}|#{Regexp.escape(projects(:moremoney).updated_at.xmlschema)})/
+        assert_xml_select 'published', /(#{Regexp.escape(todos(:book).updated_at.xmlschema)}|#{Regexp.escape(projects(:moremoney).updated_at.xmlschema)})/
       end
     end
   end