fix #1271. Don't block an already blocked todo. Create tests for this case

app/controllers/todos_controller.rb

@@ -286,7 +286,7 @@ class TodosController < ApplicationController
     @original_state = @todo.state
     unless @predecessor.completed?
       @todo.add_predecessor(@predecessor)
-      @todo.block!
+      @todo.block! unless @todo.pending?
       @saved = @todo.save
 
       @status_message = t('todos.added_dependency', :dependency => @predecessor.description)

app/models/todo.rb

@@ -164,10 +164,14 @@ class Todo < ActiveRecord::Base
     return @removed_predecessors
   end
 
+  # remove predecessor and activate myself if it was the last predecessor
   def remove_predecessor(predecessor)
-    # remove predecessor and activate myself
     self.predecessors.delete(predecessor)
-    self.activate!
+    if self.predecessors.empty?
+      self.activate!
+    else
+      save!
+    end
   end
 
   # Returns true if t is equal to self or a successor of self

lib/tracks/source_view.rb

@@ -56,7 +56,7 @@ module Tracks
       end
     
       def source_view_is_one_of( *s )
-        s.include?(params[:_source_view].to_sym)
+        s.include?((params[:_source_view] || @source_view).to_sym)
       end
 
     end

test/functional/todos_controller_test.rb

@@ -722,5 +722,111 @@ class TodosControllerTest < ActionController::TestCase
     get :tag, {:name => "foo", :format => "txt" }
     assert_response 401
   end
+    
+  def test_make_todo_dependent
+    login_as(:admin_user)
 
+    predecessor = todos(:call_bill)
+    successor = todos(:call_dino_ext)
+    
+    # no predecessors yet
+    assert_equal 0, successor.predecessors.size
+    
+    # add predecessor
+    put :add_predecessor, :predecessor=>predecessor.id, :successor=>successor.id
+    
+    assert_equal 1, successor.predecessors.count
+    assert_equal predecessor.id, successor.predecessors.first.id
+  end
+
+  def test_make_todo_with_dependencies_dependent
+    login_as(:admin_user)
+
+    predecessor = todos(:call_bill)
+    successor = todos(:call_dino_ext)
+    other_todo = todos(:phone_grandfather)
+              
+    # predecessor -> successor
+    put :add_predecessor, :predecessor=>predecessor.id, :successor=>successor.id
+    
+    # other_todo -> predecessor -> successor
+    put :add_predecessor, :predecessor=>other_todo.id, :successor=>predecessor.id
+        
+    assert_equal 1, successor.predecessors(true).count
+    assert_equal 0, other_todo.predecessors(true).count
+    assert_equal 1, predecessor.predecessors(true).count
+    assert_equal predecessor.id, successor.predecessors.first.id
+    assert_equal other_todo.id, predecessor.predecessors.first.id
+  end
+
+  def test_mingle_dependent_todos_leave
+    # based on #1271
+    login_as(:admin_user)
+
+    t1 = todos(:call_bill)
+    t2 = todos(:call_dino_ext)
+    t3 = todos(:phone_grandfather)
+    t4 = todos(:construct_dilation_device)
+              
+    # t1 -> t2
+    put :add_predecessor, :predecessor=>t1.id, :successor=>t2.id
+    # t3 -> t4
+    put :add_predecessor, :predecessor=>t3.id, :successor=>t4.id
+    
+    # t2 -> t4
+    put :add_predecessor, :predecessor=>t2.id, :successor=>t4.id
+    
+    # should be: t1 -> t2 -> t4 and t3 -> t4
+    assert t4.predecessors.map(&:id).include?(t2.id)
+    assert t4.predecessors.map(&:id).include?(t3.id)
+    assert t2.predecessors.map(&:id).include?(t1.id)
+  end
+
+  def test_mingle_dependent_todos_root
+    # based on #1271
+    login_as(:admin_user)
+
+    t1 = todos(:call_bill)
+    t2 = todos(:call_dino_ext)
+    t3 = todos(:phone_grandfather)
+    t4 = todos(:construct_dilation_device)
+              
+    # t1 -> t2
+    put :add_predecessor, :predecessor=>t1.id, :successor=>t2.id
+    # t3 -> t4
+    put :add_predecessor, :predecessor=>t3.id, :successor=>t4.id
+    
+    # t3 -> t2
+    put :add_predecessor, :predecessor=>t3.id, :successor=>t2.id
+    
+    # should be: t1 -> t2 and t3 -> t4 & t2
+    assert t3.successors.map(&:id).include?(t4.id)
+    assert t3.successors.map(&:id).include?(t2.id)
+    assert t2.predecessors.map(&:id).include?(t1.id)
+    assert t2.predecessors.map(&:id).include?(t3.id)
+  end
+  
+  def test_unmingle_dependent_todos
+    # based on #1271
+    login_as(:admin_user)
+
+    t1 = todos(:call_bill)
+    t2 = todos(:call_dino_ext)
+    t3 = todos(:phone_grandfather)
+    t4 = todos(:construct_dilation_device)
+              
+    # create same dependency tree as previous test
+    # should be: t1 -> t2 -> t4 and t3 -> t4
+    put :add_predecessor, :predecessor=>t1.id, :successor=>t2.id
+    put :add_predecessor, :predecessor=>t3.id, :successor=>t4.id
+    put :add_predecessor, :predecessor=>t2.id, :successor=>t4.id
+
+    # removing t4 as successor of t2 should leave t4 blocked with t3 as predecessor
+    put :remove_predecessor, :predecessor=>t2.id, :id=>t4.id
+    
+    t4.reload
+    assert t4.pending?, "t4 should remain pending"
+    assert t4.predecessors.map(&:id).include?(t3.id)
+  end
+  
 end

test/unit/todo_test.rb

@@ -342,5 +342,49 @@ class TodoTest < ActiveSupport::TestCase
     assert 2, todos_with_aORc_and_b.count
   end
 
+  # test named_scopes
+  def test_find_completed
+    # Given 2 completed todos, one completed now and one completed 2 months ago
+    @not_completed1.toggle_completion!
+    @completed.completed_at = 2.months.ago
+    @completed.save!
 
+    completed_old = @completed
+    completed_now = @not_completed1
+
+    # When I use the finders
+    recent_completed_todos = Todo.completed_after(1.month.ago).find(:all)
+    older_completed_todos = Todo.completed_before(1.month.ago).find(:all)
+
+    # Then completed1 should be before and completed2 should be after a month ago
+    assert older_completed_todos.include?(completed_old)
+    assert recent_completed_todos.include?(completed_now)
+
+    # And completed1 should not be after and completed2 should not be before a month ago
+    assert !older_completed_todos.include?(completed_now)
+    assert !recent_completed_todos.include?(completed_old)
+  end
+
+  def test_find_created
+    # Given 2 created todos, one created now and one created 2 months ago
+    user = @completed.user
+    todo_old = user.todos.create!({:description => "created long long ago", :context => @completed.context})
+    todo_old.created_at = 2.months.ago
+    todo_old.save!
+    todo_now = user.todos.create!({:description => "just created", :context => @completed.context})
+
+    # When I use the finders
+    recent_created_todos = Todo.created_after(1.month.ago).find(:all)
+    older_created_todos = Todo.created_before(1.month.ago).find(:all)
+
+    # Then todo1 should be before and todo2 should be after a month ago
+    assert older_created_todos.include?(todo_old)
+    assert recent_created_todos.include?(todo_now)
+
+    # And todo1 should not be after and todo2 should not be before a month ago
+    assert !older_created_todos.include?(todo_now)
+    assert !recent_created_todos.include?(todo_old)
+  end
+
+  
 end

test/unit/todo_test2.rb

@@ -1,57 +0,0 @@
-require File.expand_path(File.dirname(__FILE__) + '/../test_helper')
-require 'date'
-
-class TodoTest < ActiveSupport::TestCase
-  fixtures :todos, :recurring_todos, :users, :contexts, :preferences, :tags, :taggings, :projects
-
-  def setup
-    @not_completed1 = Todo.find(1).reload
-    @not_completed2 = Todo.find(2).reload
-    @completed = Todo.find(8).reload
-  end
-
-  # test named_scopes
-  def test_find_completed
-    # Given 2 completed todos, one completed now and one completed 2 months ago
-    @not_completed1.toggle_completion!
-    @completed.completed_at = 2.months.ago
-    @completed.save!
-
-    completed_old = @completed
-    completed_now = @not_completed1
-
-    # When I use the finders
-    recent_completed_todos = Todo.completed_after(1.month.ago).find(:all)
-    older_completed_todos = Todo.completed_before(1.month.ago).find(:all)
-
-    # Then completed1 should be before and completed2 should be after a month ago
-    assert older_completed_todos.include?(completed_old)
-    assert recent_completed_todos.include?(completed_now)
-
-    # And completed1 should not be after and completed2 should not be before a month ago
-    assert !older_completed_todos.include?(completed_now)
-    assert !recent_completed_todos.include?(completed_old)
-  end
-
-  def test_find_created
-    # Given 2 created todos, one created now and one created 2 months ago
-    user = @completed.user
-    todo_old = user.todos.create!({:description => "created long long ago", :context => @completed.context})
-    todo_old.created_at = 2.months.ago
-    todo_old.save!
-    todo_now = user.todos.create!({:description => "just created", :context => @completed.context})
-
-    # When I use the finders
-    recent_created_todos = Todo.created_after(1.month.ago).find(:all)
-    older_created_todos = Todo.created_before(1.month.ago).find(:all)
-
-    # Then todo1 should be before and todo2 should be after a month ago
-    assert older_created_todos.include?(todo_old)
-    assert recent_created_todos.include?(todo_now)
-
-    # And todo1 should not be after and todo2 should not be before a month ago
-    assert !older_created_todos.include?(todo_now)
-    assert !recent_created_todos.include?(todo_old)
-  end
-
-end