fix bug introduced by last commit and add test for it

also refactor check_for_next_todo a bit to depend less on globals

app/controllers/todos_controller.rb

@@ -134,7 +134,7 @@ class TodosController < ApplicationController
     @saved = @todo.toggle_completion!
   
     # check if this todo has a related recurring_todo. If so, create next todo
-    check_for_next_todo if @saved
+    @new_recurring_todo = check_for_next_todo(@todo) if @saved
     
     respond_to do |format|
       format.js do
@@ -279,7 +279,7 @@ class TodosController < ApplicationController
     @project_id = @todo.project_id
 
     # check if this todo has a related recurring_todo. If so, create next todo
-    check_for_next_todo
+    @new_recurring_todo = check_for_next_todo(@todo)
     
     @saved = @todo.destroy
     
@@ -771,22 +771,22 @@ class TodosController < ApplicationController
     ['rss','atom','txt','ics'].include?(req.parameters[:format])
   end
   
-  def check_for_next_todo
+  def check_for_next_todo(todo)
     # check if this todo has a related recurring_todo. If so, create next todo
-    @new_recurring_todo = nil
+    new_recurring_todo = nil
-    @recurring_todo = nil
+    recurring_todo = nil
-    if @todo.from_recurring_todo?
+    if todo.from_recurring_todo?
-      @recurring_todo = current_user.recurring_todos.find(@todo.recurring_todo_id)
+      recurring_todo = current_user.recurring_todos.find(todo.recurring_todo_id)
       
       # 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 with yesterday
       # as reference point. We pick yesterday so that new todos for today will
       # be created instead of new todos for tomorrow.
       date_to_check = Time.zone.now-1.day 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
         # the past. This is to make sure we do not get older todos for overdue
@@ -795,9 +795,10 @@ class TodosController < ApplicationController
         # date. Discard the time part in the compare
         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    
   end
   
   def get_due_id_for_calendar(due)

app/models/recurring_todo.rb

@@ -523,7 +523,7 @@ class RecurringTodo < ActiveRecord::Base
     
     case self.recurrence_selector
     when 0 # specific day of a specific month
-      if start.month > month || (start.month == month && start.day > day)
+      if start.month > month || (start.month == month && start.day >= day)
         # if there is no next month n and day m in this year, search in next year
         start = Time.zone.local(start.year+1, month, 1) 
       else

test/fixtures/todos.yml

@@ -230,3 +230,17 @@ end
   completed_at: ~
   show_from: <%= next_week %>
   user_id: 2
+
+18:
+  id: 18
+  user_id: 1
+  context_id: 1
+  project_id: 2
+  description: Call Bill Gates every day
+  notes: ~
+  state: active
+  created_at: <%= last_week %>
+  due: <%= last_week %>
+  completed_at: ~
+  show_from: ~
+  recurring_todo_id: 1

test/functional/recurring_todos_controller_test.rb

@@ -79,6 +79,11 @@ class RecurringTodosControllerTest < ActionController::TestCase
     recurring_todo_1 = RecurringTodo.find(1)
     assert recurring_todo_1.completed?
     
+    # remove remaining todo 
+    todo = Todo.find_by_recurring_todo_id(1)
+    todo.recurring_todo_id = 2
+    todo.save
+    
     todo_count = Todo.count
     
     # mark as active
@@ -119,5 +124,5 @@ class RecurringTodosControllerTest < ActionController::TestCase
     # show_from should be nil since now+4.days-10.days is in the past
     assert_equal nil, new_todo.show_from
   end
-  
+    
 end

test/functional/stats_controller_test.rb

@@ -56,7 +56,7 @@ class StatsControllerTest < Test::Unit::TestCase
     assert_equal 3, assigns['projects'].count
     assert_equal 3, assigns['projects'].count(:conditions => "state = 'active'")
     assert_equal 10, assigns['contexts'].count
-    assert_equal 15, assigns['actions'].count
+    assert_equal 16, assigns['actions'].count
     assert_equal 4, assigns['tags'].count
     assert_equal 2, assigns['unique_tags'].size
     assert_equal 2.week.ago.utc.beginning_of_day, assigns['first_action'].created_at

test/functional/todos_controller_test.rb

@@ -124,9 +124,9 @@ class TodosControllerTest < Test::Rails::TestCase
   def test_update_todo_to_deferred_is_reflected_in_badge_count
     login_as(:admin_user)
     get :index
-    assert_equal 10, assigns['count']
+    assert_equal 11, assigns['count']
     xhr :post, :update, :id => 1, :_source_view => 'todo', "context_name"=>"library", "project_name"=>"Make more money than Billy Gates", "todo"=>{"id"=>"1", "notes"=>"", "description"=>"Call Warren Buffet to find out how much he makes per day", "due"=>"30/11/2006", "show_from"=>"30/11/2030"}, "tag_list"=>"foo bar"
-    assert_equal 9, assigns['down_count']
+    assert_equal 10, assigns['down_count']
   end
   
   def test_update_todo
@@ -188,7 +188,7 @@ class TodosControllerTest < Test::Rails::TestCase
         assert_select '>description', "Actions for #{users(:admin_user).display_name}"
         assert_select 'language', 'en-us'
         assert_select 'ttl', '40'
-        assert_select 'item', 10 do
+        assert_select 'item', 11 do
           assert_select 'title', /.+/
           assert_select 'description', /.*/
           assert_select 'link', %r{http://test.host/contexts/.+}
@@ -242,7 +242,7 @@ class TodosControllerTest < Test::Rails::TestCase
     assert_xml_select 'feed[xmlns="http://www.w3.org/2005/Atom"]' do
       assert_xml_select '>title', 'Tracks Actions'
       assert_xml_select '>subtitle', "Actions for #{users(:admin_user).display_name}"
-      assert_xml_select 'entry', 10 do
+      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)})/
@@ -311,7 +311,7 @@ class TodosControllerTest < Test::Rails::TestCase
   def test_mobile_index_assigns_down_count
     login_as(:admin_user)
     get :index, { :format => "m" }
-    assert_equal 10, assigns['down_count']
+    assert_equal 11, assigns['down_count']
   end
   
   def test_mobile_create_action_creates_a_new_todo
@@ -362,38 +362,81 @@ class TodosControllerTest < Test::Rails::TestCase
     
     # link todo_1 and recurring_todo_1
     recurring_todo_1 = RecurringTodo.find(1)
-    todo_1 = Todo.find(1)
+    todo_1 = Todo.find_by_recurring_todo_id(1)
-    todo_1.recurring_todo_id = recurring_todo_1.id
-    
-    # update todo_1
-    assert todo_1.save
     
     # mark todo_1 as complete by toggle_check
-    xhr :post, :toggle_check, :id => 1, :_source_view => 'todo' 
+    xhr :post, :toggle_check, :id => todo_1.id, :_source_view => 'todo' 
     todo_1.reload
     assert todo_1.completed?
 
+    # check that there is only one active todo belonging to recurring_todo
+    count = Todo.count(:all, :conditions => {:recurring_todo_id => recurring_todo_1.id, :state => 'active'})
+    assert_equal 1, count
+    
     # check there is a new todo linked to the recurring pattern
     next_todo = Todo.find(:first, :conditions => {:recurring_todo_id => recurring_todo_1.id, :state => 'active'})
     assert_equal "Call Bill Gates every day", next_todo.description
+    # check that the new todo is not the same as todo_1
+    assert_not_equal todo_1.id, next_todo.id
     
-    # change recurrence pattern to weekly and set show_from 2 days befor due date
+    # change recurrence pattern to monthly and set show_from 2 days before due
-    # this forces the next todo to be put in the tickler
+    # date this forces the next todo to be put in the tickler
     recurring_todo_1.show_from_delta = 2
-    recurring_todo_1.recurring_period = 'weekly'
+    recurring_todo_1.recurring_period = 'monthly'
-    recurring_todo_1.every_day = 'smtwtfs'
+    recurring_todo_1.recurrence_selector = 0
+    recurring_todo_1.every_other1 = 1
+    recurring_todo_1.every_other2 = 2
+    recurring_todo_1.every_other3 = 5
     recurring_todo_1.save
-    
+
     # mark next_todo as complete by toggle_check
     xhr :post, :toggle_check, :id => next_todo.id, :_source_view => 'todo' 
     next_todo.reload
     assert next_todo.completed?
 
+    # check that there are three todos belonging to recurring_todo: two
+    # completed and one deferred
+    count = Todo.count(:all, :conditions => {:recurring_todo_id => recurring_todo_1.id})
+    assert_equal 3, count
+
     # check there is a new todo linked to the recurring pattern in the tickler
     next_todo = Todo.find(:first, :conditions => {:recurring_todo_id => recurring_todo_1.id, :state => 'deferred'})
+    assert !next_todo.nil?
     assert_equal "Call Bill Gates every day", next_todo.description
     # check that the todo is in the tickler
     assert !next_todo.show_from.nil?
   end
-   
+  
+  def test_check_for_next_todo
+    login_as :admin_user
+
+    recurring_todo_1 = RecurringTodo.find(5)
+    @todo = Todo.find_by_recurring_todo_id(1)
+    assert @todo.from_recurring_todo?
+    # rewire @todo to yearly recurring todo
+    @todo.recurring_todo_id = 5
+
+    # make todo due tomorrow and change recurring date also to tomorrow
+    @todo.due = Time.zone.now + 1.day
+    @todo.save
+    recurring_todo_1.every_other1 = @todo.due.day
+    recurring_todo_1.every_other2 = @todo.due.month    
+    recurring_todo_1.save
+    
+    # mark todo complete
+    xhr :post, :toggle_check, :id => @todo.id, :_source_view => 'todo' 
+    @todo.reload
+    assert @todo.completed?
+
+    # check that there is no active todo
+    next_todo = Todo.find(:first, :conditions => {:recurring_todo_id => recurring_todo_1.id, :state => 'active'})
+    assert next_todo.nil?
+
+    # check for new deferred todo
+    next_todo = Todo.find(:first, :conditions => {:recurring_todo_id => recurring_todo_1.id, :state => 'deferred'})
+    assert !next_todo.nil?
+    # check that the due date of the new todo is later than tomorrow
+    assert next_todo.due > @todo.due    
+  end
+    
 end

test/unit/context_test.rb

@@ -53,7 +53,7 @@ class ContextTest < Test::Rails::TestCase
   end
   
   def test_delete_context_deletes_todos_within_it
-    assert_equal 6, @agenda.todos.count
+    assert_equal 7, @agenda.todos.count
     agenda_todo_ids = @agenda.todos.collect{|t| t.id }
     @agenda.destroy
     agenda_todo_ids.each do |todo_id|
@@ -62,11 +62,11 @@ class ContextTest < Test::Rails::TestCase
   end
   
   def test_not_done_todos
-    assert_equal 5, @agenda.not_done_todos.size
+    assert_equal 6, @agenda.not_done_todos.size
     t = @agenda.not_done_todos[0]
     t.complete!
     t.save!
-    assert_equal 4, Context.find(@agenda.id).not_done_todos.size
+    assert_equal 5, Context.find(@agenda.id).not_done_todos.size
   end
     
   def test_done_todos

test/unit/project_test.rb

@@ -172,9 +172,9 @@ class ProjectTest < Test::Rails::TestCase
 
   def test_not_done_todo_count
     assert_equal 2, @timemachine.not_done_todo_count
-    assert_equal 3, @moremoney.not_done_todo_count
+    assert_equal 4, @moremoney.not_done_todo_count
     @moremoney.todos[0].complete!
-    assert_equal 2, @moremoney.not_done_todo_count
+    assert_equal 3, @moremoney.not_done_todo_count
   end
   
   def test_default_context_name