small refactorings

fix passing params in use of _collection

app/helpers/todos_helper.rb

@@ -23,7 +23,11 @@ module TodosHelper
 
   def show_grouped_todos
     collection = (@group_view_by == 'context') ? @contexts_to_show : @projects_to_show
-    render(:partial => collection, :locals => { :settings => {:collapsible => true, :show_empty_containers => @show_empty_containers }})
+    render(:partial => collection, :locals => { :settings => {
+      :collapsible => true, 
+      :show_empty_containers => @show_empty_containers,
+      :parent_container_type => @group_view_by
+    }})
   end
 
   def default_collection_settings
@@ -295,22 +299,34 @@ module TodosHelper
   def project_and_context_links(todo, parent_container_type, opts = {})
     str = ''
     if todo.completed?
-      str += todo.context.name unless opts[:suppress_context]
+      links = []
-      should_suppress_project = opts[:suppress_project] || todo.project.nil?
+      links << todo.context.name unless opts[:suppress_context]
-      str += ", " unless str.blank? || should_suppress_project
+      links << todo.project.name unless opts[:suppress_project] || todo.project.nil?
-      str += todo.project.name unless should_suppress_project
+      str = "(#{links.join(", ")})" unless links.empty?
-      str = "(#{str})" unless str.blank?
     else
-      if (['project', 'tag', 'stats', 'search'].include?(parent_container_type))
+      str << item_link_to_context( todo ) if include_context_link(todo, parent_container_type)
-        str << item_link_to_context( todo )
+      str << item_link_to_project( todo ) if include_project_link(todo, parent_container_type)
-      end
-      if (['context', 'tickler', 'tag', 'stats', 'search'].include?(parent_container_type)) && !todo.project_id.nil? && !todo.project.is_a?(NullProject)
-        str << item_link_to_project( todo )
-      end
     end
     return str.html_safe
   end
 
+  def include_context_link(todo, parent_container_type)
+    return true if (['stats', 'search'].include?(parent_container_type))
+    # TODO: remove next line if 'project' supports group_view_by
+    return true if parent_container_type == 'project'
+    return true if @group_view_by == 'project'
+    return false
+  end
+
+  def include_project_link(todo, parent_container_type)
+    return false unless todo.has_project?
+    return true if (['stats', 'search'].include?(parent_container_type))
+    # TODO: remove next line if 'context' supports group_view_by
+    return true if parent_container_type == 'context'
+    return true if @group_view_by == 'context'
+    return false
+  end
+
   # Uses the 'staleness_starts' value from settings.yml (in days) to colour the
   # background of the action appropriately according to the age of the creation
   # date:

app/models/recurring_todo.rb

@@ -527,19 +527,15 @@ class RecurringTodo < ActiveRecord::Base
       end
     end
 
-    # check if there are any days left this week for the next todo
+    day = find_first_day_in_this_week(start)
-    start.wday().upto 6 do |i|
+    return day unless day == -1
-      return start + (i-start.wday()).days unless self.every_day[i,1] == ' '
-    end
 
     # we did not find anything this week, so check the nth next, starting from
     # sunday
     start = start + self.every_other1.week - (start.wday()).days
 
-    # check if there are any days left this week for the next todo
+    start = find_first_day_in_this_week(start)
-    start.wday().upto 6 do |i|
+    return start unless start == -1
-      return start + (i-start.wday()).days unless self.every_day[i,1] == ' '
-    end
 
     raise Exception.new, "unable to find next weekly date (#{self.every_day})"
   end
@@ -729,4 +725,12 @@ class RecurringTodo < ActiveRecord::Base
     return start
   end
 
+  def find_first_day_in_this_week(start)
+    # check if there are any days left this week for the next todo
+    start.wday().upto 6 do |i|
+      return start + (i-start.wday()).days unless self.every_day[i,1] == ' '
+    end
+    return -1
+  end
+
 end

app/models/todo.rb

@@ -367,6 +367,10 @@ class Todo < ActiveRecord::Base
     end
   end
 
+  def has_project?
+    return ! (project_id.nil? || project.is_a?(NullProject))
+  end
+
   # used by the REST API. <tags> will also work, this is renamed to add_tags in TodosController::TodoCreateParamsHelper::initialize
   def add_tags=(params)
     unless params[:tag].nil?

app/views/contexts/_context.html.erb

@@ -4,7 +4,6 @@
 # rendering of "due in x days" that change without touching updated at of the todo
 cache [context, @source_view, current_user.date.strftime("%Y%m%d"), @tag_name] do
 %>
-
   <%= 
     render :partial => 'todos/collection', 
       :object => @not_done, 
@@ -13,8 +12,8 @@ cache [context, @source_view, current_user.date.strftime("%Y%m%d"), @tag_name] d
         :collapsible => settings[:collapsible], 
         :container_name => 'context', 
         :title => show_context_name(context),
-        :show_empty_containers => settings[:show_empty_containers]
+        :show_empty_containers => settings[:show_empty_containers],
+        :parent_container_type => settings[:parent_container_type]
       }} 
   %>
-
+<% end %>
-<% end %>

app/views/contexts/show.html.erb

@@ -4,7 +4,7 @@
   done_todo_options = {:append_descriptor => suffix_completed, :suppress_context => true, :parent_container_type => 'context'}
 -%>
 <div id="display_box">
-	<%= render :partial => @context, :locals => { :settings => {:collapsible => false, :show_empty_containers => true }} %>
+	<%= render :partial => @context, :locals => { :settings => {:collapsible => false, :show_empty_containers => true, :parent_container_type => 'context' }} %>
 
 	<%= show_deferred_pending_todos(@deferred_todos, @pending_todos, deferred_pending_options) %>
 

app/views/projects/_project.html.erb

@@ -10,14 +10,15 @@ cache [project, @source_view, current_user.date.strftime("%Y%m%d")] do
   <%= 
     title = source_view_is(:project) ? t('projects.actions_in_project_title') : show_project_name(project)
 
-    render :partial => 'todos/collection', 
+    render(:partial => 'todos/collection', 
       :object => @not_done, 
       :locals => { :settings => {
         :id => "p#{project.id}", 
         :collapsible => settings[:collapsible], 
         :title => title, 
         :container_name => 'project',
-        :show_empty_containers => settings[:show_empty_containers]
+        :show_empty_containers => settings[:show_empty_containers],
-      }} 
+        :parent_container_type => settings[:parent_container_type]
+      }})
   %>
 <% end -%>

app/views/projects/show.html.erb

@@ -9,7 +9,7 @@
 <div id="display_box">
   <%= project_next_prev %>
 
-  <%= render :partial => @project, :locals => {:settings => {:collapsible => false, :show_empty_containers => true }} %>
+  <%= render :partial => @project, :locals => {:settings => {:collapsible => false, :show_empty_containers => true, :parent_container_type => 'project' }} %>
 
   <%= show_deferred_pending_todos(@deferred_todos, @pending_todos, deferred_pending_options) %>
 

app/views/todos/index.html.erb

@@ -7,7 +7,7 @@
     <%= show_todos_without_project(@todos_without_project) -%>
   <% end -%>
 
-  <%= show_done_todos(@done, {:parent_container_type => 'home', :collapsible => true}) unless @done.nil? %>
+  <%= show_done_todos(@done, {:parent_container_type => @group_view_by, :collapsible => true}) unless @done.nil? %>
 </div>
 
 <div id="input_box">