diff --git a/app/controllers/todos_controller.rb b/app/controllers/todos_controller.rb index eb590cf5..00a3eed2 100644 --- a/app/controllers/todos_controller.rb +++ b/app/controllers/todos_controller.rb @@ -177,30 +177,44 @@ class TodosController < ApplicationController @sequential = !params[:todos_sequential].blank? && params[:todos_sequential]=='true' - @todos = [] + @todos_init = [] @predecessor = nil + validates = true + + # first build all todos and check if they would validate on save params[:todo][:multiple_todos].split("\n").map do |line| unless line.blank? @todo = current_user.todos.build( :description => line) @todo.project_id = @project_id @todo.context_id = @context_id - @saved = @todo.save + validates = validates && !@todo.invalid? + @todos_init << @todo + end + end + + # if all todos validate, then save them and add predecessors and tags + @todos = [] + if validates + @todos_init.each do |todo| + @saved = todo.save if @predecessor && @saved && @sequential - @todo.add_predecessor(@predecessor) - @todo.block! + todo.add_predecessor(@predecessor) + todo.block! end unless (@saved == false) || tag_list.blank? - @todo.tag_with(tag_list) - @todo.tags.reload + todo.tag_with(tag_list) + todo.tags.reload end - @todos << @todo - @not_done_todos << @todo if @new_context_created - @predecessor = @todo + @todos << todo + @not_done_todos << todo if @new_context_created + @predecessor = todo end + else + @saved = false end respond_to do |format| @@ -212,11 +226,11 @@ class TodosController < ApplicationController @initial_context_name = params['default_context_name'] @initial_project_name = params['default_project_name'] @initial_tags = params['initial_tag_list'] - if @todos.size > 0 + if @saved && @todos.size > 0 @default_tags = @todos[0].project.default_tags unless @todos[0].project.nil? else @multiple_error = t('todos.next_action_needed') - @saved = false; + @saved = false @default_tags = current_user.projects.find_by_name(@initial_project_name).default_tags unless @initial_project_name.blank? end diff --git a/test/functional/todos_controller_test.rb b/test/functional/todos_controller_test.rb index 147255b7..9ee46863 100644 --- a/test/functional/todos_controller_test.rb +++ b/test/functional/todos_controller_test.rb @@ -222,9 +222,20 @@ class TodosControllerTest < ActionController::TestCase put :create, :_source_view => 'todo', "context_name"=>"library", "project_name"=>"Build a working time machine", "todo"=>{ :multiple_todos=>"a\nb\nmuch \"ado\" about \'nothing\'"} - assert_equal start_count+2, Todo.count, "two todos should have been added" + assert_equal start_count+3, Todo.count, "two todos should have been added" end + + def test_add_multiple_todos_with_validation_error + login_as(:admin_user) + long_string = "a" * 500 + + start_count = Todo.count + put :create, :_source_view => 'todo', "context_name"=>"library", "project_name"=>"Build a working time machine", "todo"=>{ + :multiple_todos=>"a\nb\nmuch \"ado\" about \'nothing\'\n#{long_string}"} + + assert_equal start_count, Todo.count, "no todos should have been added" + end def test_add_multiple_dependent_todos login_as(:admin_user)