fix #857 and further refactoring for better done view

app/apis/todo_api.rb

@@ -15,8 +15,8 @@ class TodoApi < ActionWebService::API::Base
              :expects => [{:username => :string}, {:token => :string}],
              :returns => [[Context]]
 
- api_method :list_projects,
-            :expects => [{:username => :string}, {:token => :string}],
-            :returns => [[Project]]
+  api_method :list_projects,
+             :expects => [{:username => :string}, {:token => :string}],
+             :returns => [[Project]]
 
 end

app/controllers/contexts_controller.rb

@@ -35,7 +35,7 @@ class ContextsController < ApplicationController
   
   def show
     @contexts = current_user.contexts(true)
-    if (@context.nil?)
+    if @context.nil?
       respond_to do |format|
         format.html { render :text => 'Context not found', :status => 404 }
         format.xml  { render :xml => '<error>Context not found</error>', :status => 404 }
@@ -240,7 +240,7 @@ class ContextsController < ApplicationController
   def init_todos
     set_context_from_params
     unless @context.nil?
-      @context.todos.send :with_scope, :find => { :include => [:project, :tags] } do
+      @context.todos.send :with_scope, :find => { :include => [:project, :tags, :successors, :predecessors, :recurring_todo] } do
         @done = @context.done_todos
       end
 
@@ -250,11 +250,9 @@ class ContextsController < ApplicationController
       # search manually until I can work out a way to do the same thing using
       # not_done_todos acts_as_todo_container method Hides actions in hidden
       # projects from context.
-      @not_done_todos = @context.todos.find(
-        :all, 
-        :conditions => ['todos.state = ? AND (todos.project_id IS ? OR projects.state = ?)', 'active', nil, 'active'], 
+      @not_done_todos = @context.todos.active(
         :order => "todos.due IS NULL, todos.due ASC, todos.created_at ASC", 
-        :include => [:project, :tags])
+        :include => [:project, :tags, :successors, :predecessors, :recurring_todo])
 
       @projects = current_user.projects
 

app/controllers/todos_controller.rb

@@ -212,7 +212,7 @@ class TodosController < ApplicationController
   end
   
   def edit
-    @todo = current_user.todos.find(params['id'], :include => [:project, :context, :tags, :taggings, :predecessors])
+    @todo = current_user.todos.find(params['id'], :include => Todo::DEFAULT_INCLUDES)
     @source_view = params['_source_view'] || 'todo'
     @tag_name = params['_tag_name']
     respond_to do |format|
@@ -237,7 +237,7 @@ class TodosController < ApplicationController
   def add_predecessor
     @source_view = params['_source_view'] || 'todo'
     @predecessor = current_user.todos.find(params['predecessor'])
-    @todo = current_user.todos.find(params['successor'])
+    @todo = current_user.todos.find(params['successor'], :include => Todo::DEFAULT_INCLUDES)
     @original_state = @todo.state
     unless @predecessor.completed?
       # Add predecessor
@@ -256,9 +256,8 @@ class TodosController < ApplicationController
   end
 
   def remove_predecessor
-    puts "@@@ start remove_predecessor"
     @source_view = params['_source_view'] || 'todo'
-    @todo = current_user.todos.find(params['id'])
+    @todo = current_user.todos.find(params['id'], :include => Todo::DEFAULT_INCLUDES)
     @predecessor = current_user.todos.find(params['predecessor'])
     @successor = @todo
     @removed = @successor.remove_predecessor(@predecessor)
@@ -325,7 +324,7 @@ class TodosController < ApplicationController
   end
   
   def toggle_star
-    @todo = current_user.todos.find(params['id'], :include => [:taggings, :tags])
+    @todo = current_user.todos.find(params['id'])
     @todo.toggle_star!
     @saved = true # cannot determine error
     respond_to do |format|
@@ -409,7 +408,7 @@ class TodosController < ApplicationController
     
   def destroy
     @source_view = params['_source_view'] || 'todo'
-    @todo = current_user.todos.find(params['id'], :include => [:pending_successors, :uncompleted_predecessors, :taggings, :tags, :project, :context])
+    @todo = current_user.todos.find(params['id'])
     @original_item_due = @todo.due
     @context_id = @todo.context_id
     @project_id = @todo.project_id
@@ -477,7 +476,7 @@ class TodosController < ApplicationController
     start_of_this_week = Time.zone.now.beginning_of_week
     start_of_this_month = Time.zone.now.beginning_of_month
     start_of_previous_month = (Time.zone.now.beginning_of_month - 1.day).beginning_of_month
-    includes = {:include => [:context, :project, :tags, :taggings, :successors, :predecessors]}
+    includes = {:include => Todo::DEFAULT_INCLUDES}
 
     @done_today = completed_todos.completed_after(start_of_this_day).all(includes)
     @done_this_week = completed_todos.completed_after(start_of_this_week).completed_before(start_of_this_day).all(includes)
@@ -489,9 +488,7 @@ class TodosController < ApplicationController
     @source_view = 'done'
     @page_title = t('todos.completed_tasks_title')
 
-    includes = [:context, :project, :tags, :taggings, :successors, :predecessors]
-    
-    @done = current_user.todos.completed.paginate :page => params[:page], :per_page => 20, :order => 'completed_at DESC', :include => includes
+    @done = current_user.todos.completed.paginate :page => params[:page], :per_page => 20, :order => 'completed_at DESC', :include => Todo::DEFAULT_INCLUDES
     @count = @done.size
   end
   
@@ -501,12 +498,15 @@ class TodosController < ApplicationController
     
     @contexts_to_show = @contexts = current_user.contexts.find(:all)
     
-    @not_done_todos = current_user.todos.deferred(:include => [:tags, :taggings, :projects]) + current_user.todos.pending(:include => [:tags, :taggings, :projects])
+    includes = params[:format]=='xml' ? [:context, :project] : Todo::DEFAULT_INCLUDES
+    
+    @not_done_todos = current_user.todos.deferred(:include => includes) + current_user.todos.pending(:include => includes)
     @down_count = @count = @not_done_todos.size
     
     respond_to do |format|
       format.html
       format.m { render :action => 'mobile_list_deferred' }
+      format.xml { render :xml => @not_done_todos.to_xml( :except => :user_id ) }
     end
   end
   
@@ -594,7 +594,7 @@ class TodosController < ApplicationController
     @source_view = params['_source_view'] || 'todo'
     numdays = params['days'].to_i
 
-    @todo = current_user.todos.find(params[:id], :include => [:taggings, :tags, :uncompleted_predecessors, :pending_successors])
+    @todo = current_user.todos.find(params[:id])
     @original_item_context_id = @todo.context_id
     @todo_deferred_state_changed = true
     @new_context_created = false
@@ -635,7 +635,7 @@ class TodosController < ApplicationController
     due_this_week_date = Time.zone.now.end_of_week
     due_next_week_date = due_this_week_date + 7.days
     due_this_month_date = Time.zone.now.end_of_month
-    included_tables = [:taggings, :tags, :recurring_todo]
+    included_tables = Todo::DEFAULT_INCLUDES
         
     @due_today = current_user.todos.not_completed.find(:all,
       :include => included_tables,
@@ -848,13 +848,13 @@ class TodosController < ApplicationController
             # current_users.todos.find but that broke with_scope for :limit
 
             # Exclude hidden projects from count on home page
-            @todos = current_user.todos.find(:all, :include => [ :project, :context, :tags, :pending_successors, :recurring_todo ])
+            @todos = current_user.todos.find(:all, :include => Todo::DEFAULT_INCLUDES)
 
             # Exclude hidden projects from the home page
             @not_done_todos = current_user.todos.find(:all,
               :conditions => ['contexts.hide = ? AND (projects.state = ? OR todos.project_id IS NULL)', false, 'active'],
               :order => "todos.due IS NULL, todos.due ASC, todos.created_at ASC",
-              :include => [ :project, :context, :tags, :pending_successors, :recurring_todo ])
+              :include => Todo::DEFAULT_INCLUDES)
           end
 
         end
@@ -977,7 +977,7 @@ class TodosController < ApplicationController
       # If you've set no_completed to zero, the completed items box isn't shown
       # on the home page
       max_completed = current_user.prefs.show_number_completed
-      @done = current_user.todos.completed.find(:all, :limit => max_completed, :include => [ :context, :project, :tags ]) unless max_completed == 0
+      @done = current_user.todos.completed.find(:all, :limit => max_completed, :include => Todo::DEFAULT_INCLUDES) unless max_completed == 0
 
       # Set count badge to number of not-done, not hidden context items
       @count = current_user.todos.active.not_hidden.count(:all)

app/models/todo.rb

@@ -44,6 +44,7 @@ class Todo < ActiveRecord::Base
   named_scope :completed_before, lambda { |date| {:conditions => ["todos.completed_at < ? ", date] } }  
 
   STARRED_TAG_NAME = "starred"
+  DEFAULT_INCLUDES = [ :project, :context, :tags, :taggings, :pending_successors, :successors, :predecessors, :recurring_todo ]
 
   # regular expressions for dependencies
   RE_TODO = /[^']+/
@@ -182,7 +183,17 @@ class Todo < ActiveRecord::Base
     end
     return false
   end
+  
+  def has_pending_successors
+    #  has_many :pending_successors, :through => :predecessor_dependencies,
+    #           :source => :successor, :conditions => ['todos.state = ?', 'pending']
 
+    successors = predecessor_dependencies.all(:include => [:successor])
+    pending = successors.reject { |d| !( d.successor.state=='pending') }
+    return !pending.empty?
+    #return !pending_successors.empty?
+  end
+  
   def has_tag?(tag)
     return self.tags.select{|t| t.name==tag }.size > 0
   end

app/views/todos/_completed.rhtml

@@ -3,6 +3,7 @@
    suppress_project ||= false
 -%>
 <div class="container completed" id="completed_container">
+  <div class=add_note_link><%= link_to "Show all", done_todos_path%></div>
   <h2>
       <% if collapsible %>
       <a href="#" class="container_toggle" id="toggle_completed"><%= image_tag("collapse.png") %></a>

app/views/todos/_todo.html.erb

@@ -32,7 +32,7 @@ parameters += "&_tag_name=#{@tag_name}" if @source_view == 'tag'
       <%= deferred_due_date(todo) %>
       <%= project_and_context_links( todo, parent_container_type, :suppress_context => suppress_context, :suppress_project => suppress_project ) %>
       <%= collapsed_notes_image(todo) unless todo.notes.blank? %>
-      <%= collapsed_successors_image(todo) unless todo.pending_successors.empty? %>
+      <%= collapsed_successors_image(todo) if todo.has_pending_successors %>
     </div>
   </div>
   <div id="<%= dom_id(todo, 'edit') %>" class="edit-form" style="display:none">

config/routes.rb

@@ -28,12 +28,12 @@ ActionController::Routing::Routes.draw do |map|
 
   map.resources :todos,
     :member => {:toggle_check => :put, :toggle_star => :put},
-    :collection => {:check_deferred => :post, :filter_to_context => :post, :filter_to_project => :post, 
+    :collection => {:check_deferred => :post, :filter_to_context => :post, :filter_to_project => :post, :done => :get, :all_done => :get
   }
 
   map.with_options :controller => :todos do |todos|
     todos.home '', :action => "index"
-    todos.tickler 'tickler', :action => "list_deferred"
+    todos.tickler 'tickler.:format', :action => "list_deferred"
     todos.mobile_tickler 'tickler.m', :action => "list_deferred", :format => 'm'
     
     # This route works for tags with dots like /todos/tag/version1.5

lib/login_system.rb

@@ -188,7 +188,6 @@ module LoginSystem
   end
   
   def basic_auth_denied
-      response.headers["Status"] = "401 Unauthorized"
       response.headers["WWW-Authenticate"] = "Basic realm=\"'Tracks Login Required'\""
       render :text => t('login.unsuccessful'), :status => 401
   end

lib/tracks/todo_list.rb

@@ -25,7 +25,7 @@ module Tracks
     end
 
     def find_done_todos
-      self.todos.find_in_state(:all, :completed, :order => "todos.completed_at DESC", :limit => self.user.prefs.show_number_completed)
+      self.todos.completed.all(:order => "todos.completed_at DESC", :limit => self.user.prefs.show_number_completed)
     end
   
     def not_done_todo_count(opts={})

test/integration/todo_xml_api_test.rb

@@ -0,0 +1,37 @@
+require File.expand_path(File.dirname(__FILE__) + '/../test_helper')
+require 'todos_controller'
+
+class TodoXmlApiTest < ActionController::IntegrationTest
+  fixtures :users, :contexts, :preferences, :todos
+  
+  def setup
+    assert_test_environment_ok
+    @user = users(:other_user)
+    @password = 'sesame'
+  end
+  
+  def test_get_tickler_succeeds
+    authenticated_get_xml "/tickler", @user.login, @password, {}
+    assert_response 200
+  end
+  
+  def test_get_tickler_needs_authentication
+    get '/tickler.xml', {}, {}
+    assert_response 401
+
+    get "/tickler", {}, {'AUTHORIZATION' => "Basic " + Base64.encode64("wrong:wrong"),'ACCEPT' => 'application/xml'}
+    assert_response 401
+  end
+  
+  def test_get_tickler_returns_all_deferred_todos
+    number = @user.todos.deferred.count
+    authenticated_get_xml "/tickler", @user.login, @password, {}
+    assert_tag :tag => "todos", :children => { :count => number, :only => { :tag => "todo" } }
+  end
+  
+  def test_get_tickler_omits_user_id
+    authenticated_get_xml "/tickler", @user.login, @password, {}
+    assert_no_tag :tag => "user_id"
+  end
+  
+end