diff --git a/app/models/todo.rb b/app/models/todo.rb index 2794d4eb..13ae14b5 100644 --- a/app/models/todo.rb +++ b/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 - 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) diff --git a/features/dependencies.feature b/features/dependencies.feature index b6f7db84..88cc32ef 100644 --- a/features/dependencies.feature +++ b/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" \ No newline at end of file + 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 \ No newline at end of file diff --git a/features/step_definitions/project_steps.rb b/features/step_definitions/project_steps.rb index 51112c4b..d1733bfa 100644 --- a/features/step_definitions/project_steps.rb +++ b/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| diff --git a/features/step_definitions/todo_steps.rb b/features/step_definitions/todo_steps.rb index b36a15f8..55b1d3d8 100644 --- a/features/step_definitions/todo_steps.rb +++ b/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 diff --git a/test/unit/todo_test.rb b/test/unit/todo_test.rb index 470c5f60..d4025233 100644 --- a/test/unit/todo_test.rb +++ b/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