Ticket #19 (closed defect: fixed)

Opened 2 years ago

Last modified 2 years ago

"detect impossible move" raises exceptions, where it doesn't

Reported by: rails Assigned to: jcm
Priority: major Milestone: 0.1 with tests
Component: plugin Version: trunk
Keywords: move_to Cc:

Description

When I want a 'first' node becoming a child of a 'second' node, and both are sibblings with the same parent_id, the following code raises a exception while executing the move_to method. 

Line 342

 # detect impossible move
          if ((cur_left <= target_left) && (target_left <= cur_right)) or ((cur_left <= target_right) && (target_right <= cur_right))
            raise ActiveRecord::ActiveRecordError, "Impossible move, target node cannot be inside moved tree."
          end

I didn't have examined the if condition very deeply, but when I remove the condition, everything works fine, because it is not an "impossible move", right?

Change History

11/08/06 23:06:17 changed by jcm

  • status changed from new to assigned.
  • milestone set to 0.1 with tests.

Let's build a test case for such a move, and then we'll fix this. Probably some <= condition to be changed to <.

11/15/06 18:45:22 changed by Krishna

  • status changed from assigned to closed.
  • resolution set to fixed.

I've updated the "detect impossible move" condition, and the following test passes. Note the use of reload: without it, stale objects could corrupt the indexing and cause problems. 

  def test_ticket_19
    root = Category.create
    first = Category.create
    second = Category.create    
    first.move_to_child_of(root)
    second.move_to_child_of(root) 
    first.reload ## needed because first is stale
    
    # now we should have the situation described in the ticket
    assert_nothing_raised {first.move_to_child_of(second)}
    assert_raise(ActiveRecord::ActiveRecordError) {second.move_to_child_of(first)} # try illegal move
    first.move_to_child_of(root) # move it back
    second.reload ## needed because second is stale
    assert_nothing_raised {first.move_to_child_of(second)} # try it the other way-- first is now on the other side of second
  end