From 49f9f33ac03ce59d4dc77f9680377cf2d2b69a73 Mon Sep 17 00:00:00 2001 From: Reinier Balt Date: Fri, 21 Nov 2008 13:36:10 +0100 Subject: [PATCH 1/6] add updated_at column to todos table This timestamp was missing. All other tables with timestamps have update_at. Could use this in statistics --- db/migrate/043_add_updated_at_to_todos.rb | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 db/migrate/043_add_updated_at_to_todos.rb diff --git a/db/migrate/043_add_updated_at_to_todos.rb b/db/migrate/043_add_updated_at_to_todos.rb new file mode 100644 index 00000000..992f2a92 --- /dev/null +++ b/db/migrate/043_add_updated_at_to_todos.rb @@ -0,0 +1,8 @@ +class AddUpdatedAtToTodos < ActiveRecord::Migration + def self.up + add_column :todos, :updated_at, :timestamp + end + def self.down + remove_column :todos, :updated_at + end + end \ No newline at end of file From 764e41685beb8a42c5862b615ea8fe87a10a3869 Mon Sep 17 00:00:00 2001 From: Reinier Balt Date: Wed, 26 Nov 2008 10:53:00 +0100 Subject: [PATCH 2/6] fix migration to fill the updated_at column. The rss feeds need a filled column. Fixes #794 --- db/migrate/043_add_updated_at_to_todos.rb | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/db/migrate/043_add_updated_at_to_todos.rb b/db/migrate/043_add_updated_at_to_todos.rb index 992f2a92..ec14720a 100644 --- a/db/migrate/043_add_updated_at_to_todos.rb +++ b/db/migrate/043_add_updated_at_to_todos.rb @@ -1,8 +1,10 @@ class AddUpdatedAtToTodos < ActiveRecord::Migration def self.up add_column :todos, :updated_at, :timestamp - end - def self.down - remove_column :todos, :updated_at - end - end \ No newline at end of file + execute 'update todos set updated_at = created_at where completed_at IS NULL' + execute 'update todos set updated_at = completed_at where NOT completed_at IS NULL' + end + def self.down + remove_column :todos, :updated_at + end +end From f43447e33f404f0d8e4a44f44bcf553289c5bf6a Mon Sep 17 00:00:00 2001 From: Reinier Balt Date: Thu, 27 Nov 2008 15:48:20 +0100 Subject: [PATCH 3/6] added workaround for strange timezone behaviour --- app/models/recurring_todo.rb | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/app/models/recurring_todo.rb b/app/models/recurring_todo.rb index fdd230e7..4fb4a49c 100644 --- a/app/models/recurring_todo.rb +++ b/app/models/recurring_todo.rb @@ -29,7 +29,7 @@ class RecurringTodo < ActiveRecord::Base end # the following recurrence patterns can be stored: - # + # # daily todos - recurrence_period = 'daily' # every nth day - nth stored in every_other1 # every work day - only_work_days = true @@ -392,7 +392,7 @@ class RecurringTodo < ActiveRecord::Base # previous is the due date of the previous todo or it is the completed_at # date when the completed_at date is after due_date (i.e. you did not make # the due date in time) - # + # # assumes self.recurring_period == 'daily' # determine start @@ -481,7 +481,14 @@ class RecurringTodo < ActiveRecord::Base if the_next.nil? || the_next <= start # the nth day is already passed in this month, go to next month and try # again - the_next = the_next+n.months + + # fiddle with timezone. Looks like we hit a bug in rails here where + # 2008-12-01 +0100 plus 1.month becomes 2008-12-31 +0100. For now, just + # calculate in UTC and convert back to local timezone. + # TODO: recheck if future rails versions have this problem too + the_next = Time.utc(the_next.year, the_next.month, the_next.day)+n.months + the_next = Time.zone.local(the_next.year, the_next.month, the_next.day) + # TODO: if there is still no match, start will be set to nil. if we ever # support 5th day of the month, we need to handle this case the_next = get_xth_day_of_month(self.every_other3, self.every_count, the_next.month, the_next.year) @@ -504,8 +511,10 @@ class RecurringTodo < ActiveRecord::Base # convert back to local timezone return Time.zone.local(last_day.year, last_day.month, last_day.day) else - # 1-4th -> count upwards - start = Time.zone.local(year,month,1) + # 1-4th -> count upwards last -> count backwards. use UTC to avoid strange + # timezone oddities where last_day -= 1.day seems to shift tz+0100 to + # tz+0000 + start = Time.utc(year,month,1) n = x while n > 0 while start.wday() != weekday @@ -514,7 +523,8 @@ class RecurringTodo < ActiveRecord::Base n -= 1 start += 1.day unless n==0 end - return start + # convert back to local timezone + return Time.zone.local(start.year, start.month, start.day) end end From bd2b410c7b73f96e32f9feeb7d83bc3bfd3b19c9 Mon Sep 17 00:00:00 2001 From: Reinier Balt Date: Thu, 27 Nov 2008 16:45:47 +0100 Subject: [PATCH 4/6] change titles on calendar page to show the name of the month instead of This Month and Next Month --- app/views/todos/calendar.html.erb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/todos/calendar.html.erb b/app/views/todos/calendar.html.erb index 056c0cff..0d7e1dcb 100644 --- a/app/views/todos/calendar.html.erb +++ b/app/views/todos/calendar.html.erb @@ -31,7 +31,7 @@
-

Due in rest of this month

+

Due in rest of <%= Time.zone.now.strftime("%B") %>

> No actions due in rest of this month
@@ -41,7 +41,7 @@
-

Due next month and later

+

Due in <%= (Time.zone.now+1.month).strftime("%B") %> and later

> No actions due after this month
From 4a98ee5669cd7f8ca8269cdbad090ef1ba82d1cf Mon Sep 17 00:00:00 2001 From: Reinier Balt Date: Fri, 28 Nov 2008 16:39:50 +0100 Subject: [PATCH 5/6] 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 | 21 ++++++++--- app/controllers/todos_controller.rb | 37 +++++++++++-------- app/models/recurring_todo.rb | 5 +++ app/models/todo.rb | 5 ++- app/views/todos/toggle_check.js.rjs | 4 +- test/fixtures/todos.yml | 2 +- test/functional/projects_controller_test.rb | 6 +-- test/functional/todos_controller_test.rb | 6 +-- 8 files changed, 55 insertions(+), 31 deletions(-) diff --git a/app/controllers/recurring_todos_controller.rb b/app/controllers/recurring_todos_controller.rb index 60fb5a00..9928a4d5 100644 --- a/app/controllers/recurring_todos_controller.rb +++ b/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"]) - @completed_recurring_todos = current_user.recurring_todos.find(:all, :conditions => ["state = ?", "completed"]) + find_and_inactivate + + @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 diff --git a/app/controllers/todos_controller.rb b/app/controllers/todos_controller.rb index aead74a1..15bbcca4 100644 --- a/app/controllers/todos_controller.rb +++ b/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 - date_to_check = todo.due.nil? ? todo.show_from : todo.due + # 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 - # 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? + # 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? - 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 - # 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 - # date. Discard the time part in the compare. We pick yesterday so that - # new todos due for today will be created instead of new todos for - # tomorrow. - date = date_to_check.at_midnight >= Time.zone.now.at_midnight ? date_to_check : Time.zone.now-1.day + # 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 + # 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 + # date. Discard the time part in the compare. We pick yesterday so + # that new todos due for today will be created instead of new todos + # for tomorrow. + 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 diff --git a/app/models/recurring_todo.rb b/app/models/recurring_todo.rb index 4fb4a49c..4561ed58 100644 --- a/app/models/recurring_todo.rb +++ b/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 diff --git a/app/models/todo.rb b/app/models/todo.rb index 972b032f..f432037d 100644 --- a/app/models/todo.rb +++ b/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' diff --git a/app/views/todos/toggle_check.js.rjs b/app/views/todos/toggle_check.js.rjs index d862862c..ee9c26e1 100644 --- a/app/views/todos/toggle_check.js.rjs +++ b/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 diff --git a/test/fixtures/todos.yml b/test/fixtures/todos.yml index f4941c0c..4dcf984e 100644 --- a/test/fixtures/todos.yml +++ b/test/fixtures/todos.yml @@ -95,7 +95,7 @@ end completed_at: ~ user_id: 1 -7: +book: id: 7 context_id: 6 project_id: 3 diff --git a/test/functional/projects_controller_test.rb b/test/functional/projects_controller_test.rb index ee3df3e4..33e6ca95 100644 --- a/test/functional/projects_controller_test.rb +++ b/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 2, projects(:timemachine).position - assert_equal 3, projects(:moremoney).position + assert_equal 1, projects(:gardenclean).position + assert_equal 2, projects(:moremoney).position + assert_equal 3, projects(:timemachine).position end def test_alphabetize_sorts_active_projects_alphabetically diff --git a/test/functional/todos_controller_test.rb b/test/functional/todos_controller_test.rb index 9804b0b5..4888a7c4 100644 --- a/test/functional/todos_controller_test.rb +++ b/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 From 4a78b9f97a02b0247de6c2c05e4b0ce06631f7fc Mon Sep 17 00:00:00 2001 From: Reinier Balt Date: Fri, 28 Nov 2008 16:52:54 +0100 Subject: [PATCH 6/6] fix case where deleting a completed todo which belongs to a recurring todo resulted in multiple todos belonging to the recurring pattern or no todo at all --- app/controllers/todos_controller.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/todos_controller.rb b/app/controllers/todos_controller.rb index 15bbcca4..ad3a762a 100644 --- a/app/controllers/todos_controller.rb +++ b/app/controllers/todos_controller.rb @@ -277,11 +277,11 @@ class TodosController < ApplicationController @original_item_due = @todo.due @context_id = @todo.context_id @project_id = @todo.project_id + + @saved = @todo.destroy # check if this todo has a related recurring_todo. If so, create next todo - @new_recurring_todo = check_for_next_todo(@todo) - - @saved = @todo.destroy + @new_recurring_todo = check_for_next_todo(@todo) if @saved respond_to do |format|