From 9147fb887cfd94d6ad50de2a4300458a215a9405 Mon Sep 17 00:00:00 2001 From: Reinier Balt Date: Sat, 29 Nov 2008 15:35:17 +0100 Subject: [PATCH 1/5] refactor conditional finds to use named_scopes also use updated_at in calendar feed --- app/controllers/contexts_controller.rb | 4 +- app/controllers/feedlist_controller.rb | 10 ++-- app/controllers/projects_controller.rb | 28 +++++----- app/controllers/recurring_todos_controller.rb | 4 +- app/controllers/stats_controller.rb | 2 +- app/controllers/todos_controller.rb | 54 +++++++++---------- app/models/context.rb | 5 +- app/models/project.rb | 4 ++ app/models/todo.rb | 1 + app/views/todos/calendar.ics.erb | 2 +- 10 files changed, 61 insertions(+), 53 deletions(-) diff --git a/app/controllers/contexts_controller.rb b/app/controllers/contexts_controller.rb index 34c11d26..d09883a4 100644 --- a/app/controllers/contexts_controller.rb +++ b/app/controllers/contexts_controller.rb @@ -134,8 +134,8 @@ class ContextsController < ApplicationController def render_contexts_mobile lambda do @page_title = "TRACKS::List Contexts" - @active_contexts = @contexts.find(:all, { :conditions => ["hide = ?", false]}) - @hidden_contexts = @contexts.find(:all, { :conditions => ["hide = ?", true]}) + @active_contexts = @contexts.active + @hidden_contexts = @contexts.hidden @down_count = @active_contexts.size + @hidden_contexts.size cookies[:mobile_url]= {:value => request.request_uri, :secure => TRACKS_COOKIES_SECURE} render :action => 'index_mobile' diff --git a/app/controllers/feedlist_controller.rb b/app/controllers/feedlist_controller.rb index 68c7118c..4c2924e9 100644 --- a/app/controllers/feedlist_controller.rb +++ b/app/controllers/feedlist_controller.rb @@ -12,12 +12,12 @@ class FeedlistController < ApplicationController @contexts = current_user.contexts end - @active_projects = @projects.select{ |p| p.active? } - @hidden_projects = @projects.select{ |p| p.hidden? } - @completed_projects = @projects.select{ |p| p.completed? } + @active_projects = current_user.projects.active + @hidden_projects = current_user.projects.hidden + @completed_projects = current_user.projects.completed - @active_contexts = @contexts.select{ |c| !c.hidden? } - @hidden_contexts = @contexts.select{ |c| c.hidden? } + @active_contexts = current_user.contexts.active + @hidden_contexts = current_user.contexts.hidden respond_to do |format| format.html { render :layout => 'standard' } diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index b2489c7d..9443168f 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -31,7 +31,7 @@ class ProjectsController < ApplicationController end def projects_and_actions - @projects = @projects.select { |p| p.active? } + @projects = @projects.active respond_to do |format| format.text { render :action => 'index_text_projects_and_actions', :layout => false, :content_type => Mime::TEXT @@ -83,7 +83,7 @@ class ProjectsController < ApplicationController @go_to_project = params['go_to_project'] @saved = @project.save @project_not_done_counts = { @project.id => 0 } - @active_projects_count = current_user.projects.count(:conditions => "state = 'active'") + @active_projects_count = current_user.projects.active.count @contexts = current_user.contexts respond_to do |format| format.js { @down_count = current_user.projects.size } @@ -124,9 +124,9 @@ class ProjectsController < ApplicationController @project_not_done_counts[@project.id] = @project.reload().not_done_todo_count(:include_project_hidden_todos => true) end @contexts = current_user.contexts - @active_projects_count = current_user.projects.count(:conditions => "state = 'active'") - @hidden_projects_count = current_user.projects.count(:conditions => "state = 'hidden'") - @completed_projects_count = current_user.projects.count(:conditions => "state = 'completed'") + @active_projects_count = current_user.projects.active.count + @hidden_projects_count = current_user.projects.hidden.count + @completed_projects_count = current_user.projects.completed.count render :template => 'projects/update.js.rjs' return elsif boolean_param('update_status') @@ -161,9 +161,9 @@ class ProjectsController < ApplicationController def destroy @project.destroy - @active_projects_count = current_user.projects.count(:conditions => "state = 'active'") - @hidden_projects_count = current_user.projects.count(:conditions => "state = 'hidden'") - @completed_projects_count = current_user.projects.count(:conditions => "state = 'completed'") + @active_projects_count = current_user.projects.active.count + @hidden_projects_count = current_user.projects.hidden.count + @completed_projects_count = current_user.projects.completed.count respond_to do |format| format.js { @down_count = current_user.projects.size } format.xml { render :text => "Deleted project #{@project.name}" } @@ -199,9 +199,9 @@ class ProjectsController < ApplicationController lambda do @page_title = "TRACKS::List Projects" @count = current_user.projects.size - @active_projects = @projects.select{ |p| p.active? } - @hidden_projects = @projects.select{ |p| p.hidden? } - @completed_projects = @projects.select{ |p| p.completed? } + @active_projects = @projects.active + @hidden_projects = @projects.hidden + @completed_projects = @projects.completed @no_projects = @projects.empty? @projects.cache_note_counts @new_project = current_user.projects.build @@ -211,9 +211,9 @@ class ProjectsController < ApplicationController def render_projects_mobile lambda do - @active_projects = @projects.select{ |p| p.active? } - @hidden_projects = @projects.select{ |p| p.hidden? } - @completed_projects = @projects.select{ |p| p.completed? } + @active_projects = @projects.active + @hidden_projects = @projects.hidden + @completed_projects = @projects.completed @down_count = @active_projects.size + @hidden_projects.size + @completed_projects.size cookies[:mobile_url]= {:value => request.request_uri, :secure => TRACKS_COOKIES_SECURE} render :action => 'index_mobile' diff --git a/app/controllers/recurring_todos_controller.rb b/app/controllers/recurring_todos_controller.rb index 9928a4d5..668e6acb 100644 --- a/app/controllers/recurring_todos_controller.rb +++ b/app/controllers/recurring_todos_controller.rb @@ -118,7 +118,7 @@ class RecurringTodosController < ApplicationController else @message += " / did not create todo" end - @count = current_user.recurring_todos.count(:all, :conditions => ["state = ?", "active"]) + @count = current_user.recurring_todos.active.count else @message = "Error saving recurring todo" end @@ -140,7 +140,7 @@ class RecurringTodosController < ApplicationController # delete the recurring todo @saved = @recurring_todo.destroy - @remaining = current_user.recurring_todos.count(:all) + @remaining = current_user.recurring_todos.count respond_to do |format| diff --git a/app/controllers/stats_controller.rb b/app/controllers/stats_controller.rb index c74811d4..2d0f95c6 100755 --- a/app/controllers/stats_controller.rb +++ b/app/controllers/stats_controller.rb @@ -8,7 +8,7 @@ class StatsController < ApplicationController @page_title = 'TRACKS::Statistics' @unique_tags = @tags.count(:all, {:group=>"tag_id"}) - @hidden_contexts = @contexts.find(:all, {:conditions => ["hide = ? ", true]}) + @hidden_contexts = @contexts.hidden @first_action = @actions.find(:first, :order => "created_at ASC") get_stats_actions diff --git a/app/controllers/todos_controller.rb b/app/controllers/todos_controller.rb index ad3a762a..6e61585e 100644 --- a/app/controllers/todos_controller.rb +++ b/app/controllers/todos_controller.rb @@ -14,7 +14,7 @@ class TodosController < ApplicationController @projects = current_user.projects.find(:all, :include => [:default_context]) @contexts = current_user.contexts.find(:all) - @contexts_to_show = @contexts.reject {|x| x.hide? } + @contexts_to_show = current_user.contexts.active respond_to do |format| format.html &render_todos_html @@ -28,7 +28,7 @@ class TodosController < ApplicationController end def new - @projects = current_user.projects.select { |p| p.active? } + @projects = current_user.projects.active @contexts = current_user.contexts.find(:all) respond_to do |format| format.m { @@ -116,7 +116,7 @@ class TodosController < ApplicationController def show respond_to do |format| format.m do - @projects = current_user.projects.select { |p| p.active? } + @projects = current_user.projects.active @contexts = current_user.contexts.find(:all) @edit_mobile = true @return_path=cookies[:mobile_url] @@ -442,33 +442,33 @@ class TodosController < ApplicationController due_next_week_date = due_this_week_date + 7.days due_this_month_date = Time.zone.now.end_of_month - @due_today = current_user.todos.find(:all, + @due_today = current_user.todos.not_completed.find(:all, :include => [:taggings, :tags], - :conditions => ['(todos.state = ? OR todos.state = ?) AND todos.due <= ?', 'active', 'deferred', due_today_date], + :conditions => ['todos.due <= ?', due_today_date], :order => "due") - @due_this_week = current_user.todos.find(:all, + @due_this_week = current_user.todos.not_completed.find(:all, :include => [:taggings, :tags], - :conditions => ['(todos.state = ? OR todos.state = ?) AND todos.due > ? AND todos.due <= ?', 'active', 'deferred', due_today_date, due_this_week_date], + :conditions => ['todos.due > ? AND todos.due <= ?', due_today_date, due_this_week_date], :order => "due") - @due_next_week = current_user.todos.find(:all, + @due_next_week = current_user.todos.not_completed.find(:all, :include => [:taggings, :tags], - :conditions => ['(todos.state = ? OR todos.state = ?) AND todos.due > ? AND todos.due <= ?', 'active', 'deferred', due_this_week_date, due_next_week_date], + :conditions => ['todos.due > ? AND todos.due <= ?', due_this_week_date, due_next_week_date], :order => "due") - @due_this_month = current_user.todos.find(:all, + @due_this_month = current_user.todos.not_completed.find(:all, :include => [:taggings, :tags], - :conditions => ['(todos.state = ? OR todos.state = ?) AND todos.due > ? AND todos.due <= ?', 'active', 'deferred', due_next_week_date, due_this_month_date], + :conditions => ['todos.due > ? AND todos.due <= ?', due_next_week_date, due_this_month_date], :order => "due") - @due_after_this_month = current_user.todos.find(:all, + @due_after_this_month = current_user.todos.not_completed.find(:all, :include => [:taggings, :tags], - :conditions => ['(todos.state = ? OR todos.state = ?) AND todos.due > ?', 'active', 'deferred', due_this_month_date], + :conditions => ['todos.due > ?', due_this_month_date], :order => "due") - + + @count = current_user.todos.not_completed.are_due.count + respond_to do |format| format.html format.ics { - @due_all = current_user.todos.find(:all, - :conditions => ['(todos.state = ? OR todos.state = ?) AND NOT todos.due IS NULL', 'active', 'deferred'], - :order => "due") + @due_all = current_user.todos.not_completed.are_due.find(:all, :order => "due") render :action => 'calendar', :layout => false, :content_type => Mime::ICS } end @@ -834,20 +834,20 @@ class TodosController < ApplicationController due_this_month_date = Time.zone.now.end_of_month case id when "due_today" - return 0 == current_user.todos.count(:all, - :conditions => ['(todos.state = ? OR todos.state = ?) AND todos.due <= ?', 'active', 'deferred', due_today_date]) + return 0 == current_user.todos.not_completed.count(:all, + :conditions => ['todos.due <= ?', due_today_date]) when "due_this_week" - return 0 == current_user.todos.count(:all, - :conditions => ['(todos.state = ? OR todos.state = ?) AND todos.due > ? AND todos.due <= ?', 'active', 'deferred', due_today_date, due_this_week_date]) + return 0 == current_user.todos.not_completed.count(:all, + :conditions => ['todos.due > ? AND todos.due <= ?', due_today_date, due_this_week_date]) when "due_next_week" - return 0 == current_user.todos.count(:all, - :conditions => ['(todos.state = ? OR todos.state = ?) AND todos.due > ? AND todos.due <= ?', 'active', 'deferred', due_this_week_date, due_next_week_date]) + return 0 == current_user.todos.not_completed.count(:all, + :conditions => ['todos.due > ? AND todos.due <= ?', due_this_week_date, due_next_week_date]) when "due_this_month" - return 0 == current_user.todos.count(:all, - :conditions => ['(todos.state = ? OR todos.state = ?) AND todos.due > ? AND todos.due <= ?', 'active', 'deferred', due_next_week_date, due_this_month_date]) + return 0 == current_user.todos.not_completed.count(:all, + :conditions => ['todos.due > ? AND todos.due <= ?', due_next_week_date, due_this_month_date]) when "due_after_this_month" - return 0 == current_user.todos.count(:all, - :conditions => ['(todos.state = ? OR todos.state = ?) AND todos.due > ?', 'active', 'deferred', due_this_month_date]) + return 0 == current_user.todos.not_completed.count(:all, + :conditions => ['todos.due > ?', due_this_month_date]) else raise Exception.new, "unknown due id for calendar: '#{id}'" end diff --git a/app/models/context.rb b/app/models/context.rb index 39cce497..41d27a53 100644 --- a/app/models/context.rb +++ b/app/models/context.rb @@ -2,7 +2,10 @@ class Context < ActiveRecord::Base has_many :todos, :dependent => :delete_all, :include => :project, :order => "todos.completed_at DESC" belongs_to :user - + + named_scope :active, :conditions => { :hide => false } + named_scope :hidden, :conditions => { :hide => true } + acts_as_list :scope => :user extend NamePartFinder include Tracks::TodoList diff --git a/app/models/project.rb b/app/models/project.rb index 396d49cc..4013e985 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -3,6 +3,10 @@ class Project < ActiveRecord::Base has_many :notes, :dependent => :delete_all, :order => "created_at DESC" belongs_to :default_context, :class_name => "Context", :foreign_key => "default_context_id" belongs_to :user + + named_scope :active, :conditions => { :state => 'active' } + named_scope :hidden, :conditions => { :state => 'hidden' } + named_scope :completed, :conditions => { :state => 'completed'} validates_presence_of :name, :message => "project must have a name" validates_length_of :name, :maximum => 255, :message => "project name must be less than 256 characters" diff --git a/app/models/todo.rb b/app/models/todo.rb index f432037d..5aed02be 100644 --- a/app/models/todo.rb +++ b/app/models/todo.rb @@ -7,6 +7,7 @@ class Todo < ActiveRecord::Base named_scope :active, :conditions => { :state => 'active' } named_scope :not_completed, :conditions => ['NOT state = ? ', 'completed'] + named_scope :are_due, :conditions => ['NOT todos.due IS NULL'] STARRED_TAG_NAME = "starred" diff --git a/app/views/todos/calendar.ics.erb b/app/views/todos/calendar.ics.erb index 46a02f60..6f3960dc 100644 --- a/app/views/todos/calendar.ics.erb +++ b/app/views/todos/calendar.ics.erb @@ -20,7 +20,7 @@ CLASS:PUBLIC CATEGORIES:Tracks CREATED:<%= todo.created_at.strftime("%Y%m%dT%H%M%SZ") %> DESCRIPTION:<%= format_ical_notes(todo.notes) %> -LAST-MODIFIED:<%= due_date.strftime("%Y%m%dT%H%M%SZ") %> +LAST-MODIFIED:<%= updated_at.strftime("%Y%m%dT%H%M%SZ") %> LOCATION: SEQUENCE:0 STATUS:CONFIRMED From 9d246f70cd515f3bea6774e72eda0c1424da2149 Mon Sep 17 00:00:00 2001 From: Reinier Balt Date: Sat, 29 Nov 2008 15:55:16 +0100 Subject: [PATCH 2/5] Fix #795. Similar hack as http://github.com/bsag/tracks/commit/f43447e33f404f0d8e4a44f44bcf553289c5bf6a --- app/controllers/todos_controller.rb | 8 ++++---- app/models/recurring_todo.rb | 17 ++++++++++++++--- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/app/controllers/todos_controller.rb b/app/controllers/todos_controller.rb index 6e61585e..265fcea3 100644 --- a/app/controllers/todos_controller.rb +++ b/app/controllers/todos_controller.rb @@ -779,8 +779,8 @@ class TodosController < ApplicationController if todo.from_recurring_todo? 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 + # 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 @@ -788,7 +788,7 @@ class TodosController < ApplicationController # 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) # shift the reference date to yesterday if date_to_check is furher in @@ -799,7 +799,7 @@ class TodosController < ApplicationController # 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) end end diff --git a/app/models/recurring_todo.rb b/app/models/recurring_todo.rb index 4561ed58..be4e2414 100644 --- a/app/models/recurring_todo.rb +++ b/app/models/recurring_todo.rb @@ -471,12 +471,23 @@ class RecurringTodo < ActiveRecord::Base start = determine_start(previous) day = self.every_other1 n = self.every_other2 - + case self.recurrence_selector when 0 # specific day of the month - if start.mday >= day + if start.mday >= day # there is no next day n in this month, search in next month - start += n.months + # + # start += n.months + # + # The above seems to not work. 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 + start = Time.utc(start.year, start.month, start.day)+n.months + start = Time.zone.local(start.year, start.month, start.day) + # go back to day end return Time.zone.local(start.year, start.month, day) From 5659b8adbb321311673611c53bf4af9de0f73c82 Mon Sep 17 00:00:00 2001 From: Reinier Balt Date: Sat, 29 Nov 2008 20:15:49 +0100 Subject: [PATCH 3/5] fix #764 where unnecessary space is removed in the recurrence pattern also fixes the case that a rec todo that has a start date but no end date is not showing the from xx-xx-xxxx on the page --- app/helpers/recurring_todos_helper.rb | 10 +++++----- app/views/recurring_todos/_recurring_todo.html.erb | 11 ++++++++++- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/app/helpers/recurring_todos_helper.rb b/app/helpers/recurring_todos_helper.rb index b346e7ea..76fcc95e 100644 --- a/app/helpers/recurring_todos_helper.rb +++ b/app/helpers/recurring_todos_helper.rb @@ -3,7 +3,7 @@ module RecurringTodosHelper def recurrence_time_span(rt) case rt.ends_on when "no_end_date" - return "" + return rt.start_from.nil? ? "" : "from " + format_date(rt.start_from) when "ends_on_number_of_times" return "for "+rt.number_of_occurences.to_s + " times" when "ends_on_end_date" @@ -11,7 +11,7 @@ module RecurringTodosHelper ends = rt.end_date.nil? ? "" : " until " + format_date(rt.end_date) return starts+ends else - raise Exception.new, "unknown recurrence time span selection (#{self.ends_on})" + raise Exception.new, "unknown recurrence time span selection (#{self.ends_on})" end end @@ -28,10 +28,10 @@ module RecurringTodosHelper def recurring_todo_tag_list tags_except_starred = @recurring_todo.tags.reject{|t| t.name == Todo::STARRED_TAG_NAME} - tag_list = tags_except_starred.collect{|t| "" + + tag_list = tags_except_starred.collect{|t| "" + # link_to(t.name, :controller => "todos", :action => "tag", :id => # t.name) + TODO: tag view for recurring_todos (yet?) - t.name + + t.name + ""}.join('') "#{tag_list}" end @@ -44,7 +44,7 @@ module RecurringTodosHelper str end - def recurring_todo_remote_star_icon + def recurring_todo_remote_star_icon str = link_to( image_tag_for_star(@recurring_todo), toggle_star_recurring_todo_path(@recurring_todo), :class => "icon star_item", :title => "star the action '#{@recurring_todo.description}'") diff --git a/app/views/recurring_todos/_recurring_todo.html.erb b/app/views/recurring_todos/_recurring_todo.html.erb index 3962b27b..ccc93cbb 100644 --- a/app/views/recurring_todos/_recurring_todo.html.erb +++ b/app/views/recurring_todos/_recurring_todo.html.erb @@ -6,7 +6,16 @@
<%= sanitize(recurring_todo.description) %> <%= recurring_todo_tag_list %> - [<%=recurrence_target(recurring_todo)%> <%= recurring_todo.recurrence_pattern %> <%= recurrence_time_span(recurring_todo) %>] + <% + rt = recurrence_target(recurring_todo) + rp = recurring_todo.recurrence_pattern + # only add space if recurrence_pattern has content + rp = " " + rp if !rp.nil? + rts = recurrence_time_span(recurring_todo) + # only add space if recurrence_time_span has content + rts = " " + rts if !(rts == "") + %> + [<%=rt%><%=rp%><%=rts%>]
From ee3057185540408b54ed016712805fe08eba4956 Mon Sep 17 00:00:00 2001 From: Reinier Balt Date: Sat, 29 Nov 2008 20:32:39 +0100 Subject: [PATCH 4/5] fix #793 where the feed title is given more information about the content also fixes project feed where the link in the feed points to the context of the todo instead of the project --- app/controllers/todos_controller.rb | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/app/controllers/todos_controller.rb b/app/controllers/todos_controller.rb index 265fcea3..6038f3f3 100644 --- a/app/controllers/todos_controller.rb +++ b/app/controllers/todos_controller.rb @@ -536,13 +536,17 @@ class TodosController < ApplicationController end def with_parent_resource_scope(&block) + @feed_title = "Actions " if (params[:context_id]) @context = current_user.contexts.find_by_params(params) + @feed_title = @feed_title + "in context '#{@context.name}'" Todo.send :with_scope, :find => {:conditions => ['todos.context_id = ?', @context.id]} do yield end elsif (params[:project_id]) @project = current_user.projects.find_by_params(params) + @feed_title = @feed_title + "in project '#{@project.name}'" + @project_feed = true Todo.send :with_scope, :find => {:conditions => ['todos.project_id = ?', @project.id]} do yield end @@ -718,7 +722,7 @@ class TodosController < ApplicationController render_rss_feed_for @todos, :feed => todo_feed_options, :item => { :title => :description, - :link => lambda { |t| context_url(t.context) }, + :link => lambda { |t| @project_feed.nil? ? context_url(t.context) : project_url(t.project) }, :guid => lambda { |t| todo_url(t) }, :description => todo_feed_content } @@ -726,7 +730,9 @@ class TodosController < ApplicationController end def todo_feed_options - Todo.feed_options(current_user) + options = Todo.feed_options(current_user) + options[:title] = @feed_title + return options end def todo_feed_content From 2980de2d8d8fb11739ad690dfcf7e6df5db9465e Mon Sep 17 00:00:00 2001 From: Reinier Balt Date: Sun, 30 Nov 2008 13:56:41 +0100 Subject: [PATCH 5/5] fix error in calendar feed --- app/views/todos/calendar.ics.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/todos/calendar.ics.erb b/app/views/todos/calendar.ics.erb index 6f3960dc..8378a096 100644 --- a/app/views/todos/calendar.ics.erb +++ b/app/views/todos/calendar.ics.erb @@ -20,7 +20,7 @@ CLASS:PUBLIC CATEGORIES:Tracks CREATED:<%= todo.created_at.strftime("%Y%m%dT%H%M%SZ") %> DESCRIPTION:<%= format_ical_notes(todo.notes) %> -LAST-MODIFIED:<%= updated_at.strftime("%Y%m%dT%H%M%SZ") %> +LAST-MODIFIED:<%= todo.updated_at.strftime("%Y%m%dT%H%M%SZ") %> LOCATION: SEQUENCE:0 STATUS:CONFIRMED