diff --git a/app/controllers/todos_controller.rb b/app/controllers/todos_controller.rb index b92dd733..2487afc4 100644 --- a/app/controllers/todos_controller.rb +++ b/app/controllers/todos_controller.rb @@ -257,9 +257,13 @@ class TodosController < ApplicationController @todo = current_user.todos.includes(Todo::DEFAULT_INCLUDES).find(params['successor']) @original_state = @todo.state unless @predecessor.completed? - @todo.add_predecessor(@predecessor) - @todo.block! unless @todo.pending? - @saved = @todo.save + begin + @todo.add_predecessor(@predecessor) + @todo.block! unless @todo.pending? + @saved = @todo.save + rescue ActiveRecord::RecordInvalid + @saved = false + end @status_message = t('todos.added_dependency', :dependency => @predecessor.description) @status_message += t('todos.set_to_pending', :task => @todo.description) unless @original_state == 'pending' diff --git a/app/models/dependency.rb b/app/models/dependency.rb index e9fd8fcc..22e144b7 100644 --- a/app/models/dependency.rb +++ b/app/models/dependency.rb @@ -5,5 +5,13 @@ class Dependency < ActiveRecord::Base belongs_to :predecessor, :foreign_key => 'predecessor_id', :class_name => 'Todo', :touch => true belongs_to :successor, :foreign_key => 'successor_id', :class_name => 'Todo', :touch => true + validate :check_circular_dependencies + + def check_circular_dependencies + unless predecessor.nil? or successor.nil? + errors.add("Depends on:", "Adding '#{successor.specification}' would create a circular dependency") if successor.is_successor?(predecessor) + end + end + end diff --git a/app/models/todo.rb b/app/models/todo.rb index 88178891..45b0ae81 100644 --- a/app/models/todo.rb +++ b/app/models/todo.rb @@ -115,7 +115,6 @@ class Todo < ActiveRecord::Base validates_presence_of :show_from, :if => :deferred? validates_presence_of :context validate :check_show_from_in_future - validate :check_circular_dependencies def check_show_from_in_future if show_from_changed? # only check on change of show_from @@ -125,14 +124,6 @@ class Todo < ActiveRecord::Base end end - def check_circular_dependencies - unless @predecessor_array.nil? # Only validate predecessors if they changed - @predecessor_array.each do |todo| - errors.add("Depends on:", "Adding '#{todo.specification}' would create a circular dependency") if is_successor?(todo) - end - end - end - def initialize(*args) super(*args) @predecessor_array = nil # Used for deferred save of predecessors diff --git a/test/models/todo_test.rb b/test/models/todo_test.rb index 4662a283..b79bf26b 100644 --- a/test/models/todo_test.rb +++ b/test/models/todo_test.rb @@ -93,10 +93,9 @@ class TodoTest < ActiveSupport::TestCase assert_equal 1, @not_completed3.successors.count # 1 -> 3 -> 2 -> 1 == circle - @not_completed3.add_predecessor(@not_completed1) - assert !@not_completed3.valid? - error_msg = "Adding ''Call Bill Gates to find out how much he makes per day' <'agenda'; 'Make more money than Billy Gates'>' would create a circular dependency" - assert_equal error_msg, @not_completed3.errors["Depends on:"][0] + assert_raises ActiveRecord::RecordInvalid do + @not_completed3.add_predecessor(@not_completed1) + end end def test_defer_an_existing_todo