Added tests for #975. This helped to narrow the real cause of #975 but doest not solve it.

app/models/todo.rb

@@ -24,9 +24,13 @@ class Todo < ActiveRecord::Base
   named_scope :blocked, :conditions => ['todos.state = ?', 'pending']
 
   STARRED_TAG_NAME = "starred"
+
+  # regular expressions for dependencies
   RE_TODO = /[^"]+/
   RE_CONTEXT = /[^"]+/
   RE_PROJECT = /[^"]+/
+  RE_PARTS = /"(#{RE_TODO})"\s<"(#{RE_CONTEXT})";\s"(#{RE_PROJECT})">/ # results in array
+  RE_SPEC = /"#{RE_TODO}"\s<"#{RE_CONTEXT}";\s"#{RE_PROJECT}">/ # results in string
   
   acts_as_state_machine :initial => :active, :column => 'state'
   
@@ -97,27 +101,36 @@ class Todo < ActiveRecord::Base
   
   def todo_from_specification(specification)
     # Split specification into parts: description <context, project>
-    re_parts = /"(#{RE_TODO})"\s<"(#{RE_CONTEXT})";\s"(#{RE_PROJECT})">/
-    parts = specification.scan(re_parts)
+    parts = specification.scan(RE_PARTS)
     return nil unless parts.length == 1
     return nil unless parts[0].length == 3
     todo_description = parts[0][0]
     context_name = parts[0][1]
+    project_name = parts[0][2]
+    
+    # find the project
+    project_id = nil;
+    unless project_name == "(none)"
+      project = Project.first(:conditions => {
+        :user_id => self.user.id,
+        :name => project_name
+      })
+      project_id = project.id unless project.nil?
+    end
+
     todos = Todo.all(
       :joins => :context,
       :conditions => {
         :description => todo_description,
-        :contexts => {:name => context_name}
+        :user_id => self.user.id,
+        :contexts => {:name => context_name},
+        :project_id => project_id
       }
     )
     return nil if todos.empty?
-    # todos now contains all todos with matching description and context
-    # TODO: Is this possible to do with a single query?
-    todos.each do |todo|
-      project_name = todo.project.is_a?(NullProject) ? "(none)" : todo.project.name
-      return todo if project_name == parts[0][2]
-    end
-    return nil
+
+    # TODO: what todo if there are more than one todo that fit the specification
+    return todos[0]
   end
   
   def validate
@@ -270,9 +283,8 @@ class Todo < ActiveRecord::Base
 
   def add_predecessor_list(predecessor_list)
     return unless predecessor_list.kind_of? String
-    # Split into list
-    re_specification = /"#{RE_TODO}"\s<"#{RE_CONTEXT}";\s"#{RE_PROJECT}">/
-    @predecessor_array = predecessor_list.scan(re_specification)
+    @predecessor_array = predecessor_list.scan(RE_SPEC)
+    return @predecessor_array
   end
   
   def add_predecessor(t)

features/dependencies.feature

@@ -22,4 +22,17 @@ Feature: dependencies
   Then I should see "Todo 2" within the dependencies of "Todo 1"
   And I should see "Todo 3" within the dependencies of "Todo 1"
   When I expand the dependencies of "Todo 2"
-  Then I should see "Todo 3" within the dependencies of "Todo 2"
+  Then I should see "Todo 3" within the dependencies of "Todo 2"
+
+  @selenium, @wip
+  Scenario: Adding dependency with comma to todo   # for #975
+  Given I have a context called "@pc"
+  And I have a project "dependencies" that has the following todos
+    | description | context |
+    | test,1, 2,3 | @pc     |
+    | test me     | @pc     |
+  When I visit the "dependencies" project
+  And I drag "test me" to "test,1, 2,3"
+  Then the dependencies of "test me" should include "test,1, 2,3"
+  When I edit the dependency of "test me" to '"test,1, 2,3" <"@pc"; "dependencies">,"test,1, 2,3" <"@pc"; "dependencies">'
+  Then there should not be an error 

features/step_definitions/project_steps.rb

@@ -12,7 +12,7 @@ end
 Given /^there exists a project "([^\"]*)" for user "([^\"]*)"$/ do |project_name, user_name|
   user = User.find_by_login(user_name)
   user.should_not be_nil
-  user.projects.create!(:name => project_name)
+  @project = user.projects.create!(:name => project_name)
 end
 
 Given /^there exists a project called "([^"]*)" for user "([^"]*)"$/ do |project_name, login|

features/step_definitions/todo_steps.rb

@@ -33,6 +33,19 @@ Given /^"(.*)" depends on "(.*)"$/ do |successor_name, predecessor_name|
   successor.save!
 end
 
+Given /^I have a project "([^"]*)" that has the following todos$/ do |project_name, todos|
+  Given "I have a project called \"#{project_name}\""
+  @project.should_not be_nil
+  todos.hashes.each do |todo|
+    context_id = @current_user.contexts.find_by_name(todo[:context])
+    context_id.should_not be_nil
+    @current_user.todos.create!(
+      :description => todo[:description],
+      :context_id => context_id,
+      :project_id=>@project.id)
+  end
+end
+
 When /^I drag "(.*)" to "(.*)"$/ do |dragged, target|
   drag_id = Todo.find_by_description(dragged).id
   drop_id = Todo.find_by_description(target).id
@@ -93,6 +106,23 @@ When /^I submit the new multiple actions form with$/ do |multi_line_descriptions
   submit_multiple_next_action_form
 end
 
+When /^I edit the dependency of "([^"]*)" to '([^'']*)'$/ do |todo_name, deps|
+  todo = @dep_todo = @current_user.todos.find_by_description(todo_name)
+  todo.should_not be_nil
+  # click edit
+  selenium.click("//div[@id='line_todo_#{todo.id}']//img[@id='edit_icon_todo_#{todo.id}']", :wait_for => :ajax, :javascript_framework => :jquery)
+  fill_in "predecessor_list_todo_#{todo.id}", :with => deps
+  # submit form
+  selenium.click("//div[@id='edit_todo_#{todo.id}']//button[@id='submit_todo_#{todo.id}']", :wait_for => :ajax, :javascript_framework => :jquery)
+  
+end
+
+Then /^there should not be an error$/ do
+  # form should be gone and thus not errors visible
+  selenium.is_visible("edit_todo_#{@dep_todo.id}").should == false
+end
+
+
 Then /^the dependencies of "(.*)" should include "(.*)"$/ do |child_name, parent_name|
   parent = @current_user.todos.find_by_description(parent_name)
   parent.should_not be_nil

test/unit/todo_test.rb

@@ -2,7 +2,7 @@ require File.dirname(__FILE__) + '/../test_helper'
 require 'date'
 
 class TodoTest < ActiveSupport::TestCase
-  fixtures :todos, :recurring_todos, :users, :contexts, :preferences, :tags, :taggings
+  fixtures :todos, :recurring_todos, :users, :contexts, :preferences, :tags, :taggings, :projects
 
   def setup
     @not_completed1 = Todo.find(1).reload
@@ -191,5 +191,74 @@ class TodoTest < ActiveSupport::TestCase
     @not_completed1.toggle_star!
     assert !@not_completed1.starred?
   end
-  
+
+  def test_todo_specification_handles_null_project
+    # @not_completed1 has a project
+    todo_desc = @not_completed1.description
+    assert_equal "\"#{todo_desc}\" <\"agenda\"; \"Make more money than Billy Gates\">", @not_completed1.specification
+
+    # now check on null
+    @not_completed1.project = nil
+    @not_completed1.save
+    assert_equal "\"#{todo_desc}\" <\"agenda\"; \"(none)\">", @not_completed1.specification
+  end
+
+  def test_todo_from_specification_should_return_todo
+    todo = Todo.new
+
+    assert_equal @not_completed1, todo.todo_from_specification(@not_completed1.specification)
+
+    # should handle comma's in description (#975)
+    @not_completed1.description = "test,1,2,3"
+    @not_completed1.save
+    assert_equal @not_completed1, todo.todo_from_specification(@not_completed1.specification)
+  end
+
+  def test_todo_from_specification_should_return_nil_on_invalid_input
+    todo = Todo.new
+    todo_desc = @not_completed1.description
+    project_name = @not_completed1.project.name
+    context_name = @not_completed1.context.name
+
+    assert todo.todo_from_specification("").nil?
+    assert todo.todo_from_specification("bla, bla , bla").nil?
+    assert todo.todo_from_specification("#{todo_desc} <#{context_name}; #{project_name}>").nil?  # missing \"
+  end
+
+  def test_add_predecessor_list
+    todo = Todo.new
+
+    single = @not_completed1.specification
+    multi = single + "," + @not_completed2.specification
+
+    @predecessor_array = todo.add_predecessor_list(single)
+    assert_not_nil @predecessor_array
+    assert_equal 1, @predecessor_array.size
+
+    @predecessor_array = todo.add_predecessor_list(multi)
+    assert_not_nil @predecessor_array
+    assert_equal 2, @predecessor_array.size
+  end
+
+  def test_add_predecessor_list_with_comma
+    # test for #975
+    todo = Todo.new
+
+    @not_completed1.description = "test,1,2,3"
+    @not_completed1.save
+    @not_completed2.description = "test,4,5,6"
+    @not_completed2.save
+
+    single = @not_completed1.specification
+    multi = single + "," + @not_completed2.specification
+
+    @predecessor_array = todo.add_predecessor_list(single)
+    assert_not_nil @predecessor_array
+    assert_equal 1, @predecessor_array.size
+
+    @predecessor_array = todo.add_predecessor_list(multi)
+    assert_not_nil @predecessor_array
+    assert_equal 2, @predecessor_array.size
+  end
+
 end