[BUG] Items are not contracted correctly when destroying multiple items

[james-reading]

Demonstrated with this failing test case:

  def test_destroying_multiple_items
    @first_list.items.order(:position).limit(2).destroy_all

    assert_equal 1, @third_item.reload.position
  end

I am destroying the first 2 items, so I expect the remaining item to be position 1, but it is position 2.

That's because both items are instantiated before either are destroyed. So when destroying the 2nd item, it's memoized position is still 2. Item >= position 3 are contacted, but by this point the third item is position 2, so it remains there.

[brendon]

Thanks for the heads up :) I had been thinking of using the uncached method to prevent this kind of issue when shuffling things around. That probably won't help here because this is the call:

def destroy_position
  contract(positioning_scope, (position + 1)..) unless destroyed_via_positioning_scope?
end

and position is just:

def position
  @positioned.send @column
end

We'd have to change that to perhaps a call like position_was: record_scope.pick(@column). It'll need some careful attention to make sure it's working properly though :) I'll put this down to work on soon :)

[brendon]

Hi @james-reading, please try the master branch to see if this fixes your problem. I've basically change the destruction callback to a before_. It sets the position of the item being destroyed to 0 while at the same time memoizing its original value. It then contracts the list before actually destroying the item.

[james-reading]

Lovely stuff, that's working nicely.

[brendon]

Thanks @james-reading, I'll release that now :)