Fix #1258 and fix #1259

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
 

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)