search

brendon / acts_as_list

An ActiveRecord plugin for managing lists.
http://brendon.github.io/acts_as_list/
MIT License
2.04k stars 355 forks source link

Adding to existing model with data and methods don't work #209

Closed tinderfields closed 7 years ago

tinderfields commented 8 years ago

When I add acts_as_list to a model retroactively the position is set to NULL and the move_higher and lower methods etc don't work. I have to add something like this to the migration.

TeamMember.reset_column_information
TeamMember.all.each_with_index do |tm, i|
  tm.update_columns(position: i)
end

It would be good if there was something in the documentation to highlight this as I seem to have to figure it out every time I use acts_as_list

brendon commented 8 years ago

Hi @tinderfields, yes that's a good idea. Would you mind forking the project, updating the README and doing a pull request? Try to make you example as generic as possible to ensure it makes sense to everyone. You'll also want to set position to i + 1 since each_with_index is zero-based, and acts_as_list generally is configured one-based.

Remember also that scopes come into play here. So some model's might have positions scoped to some parent value.

@swanandp, is it worth looking into making all the methods work with an uninitialised table? We can't really auto-populate the table because we can't predict the scope values, but we can make the methods cope with NULL values.

tinderfields commented 8 years ago

Done

swanandp commented 8 years ago

@brendon That does seem slightly out of scope for us, but we should certainly add the documentation. FWIW, in my experience, acts_as_list is almost exclusively used with a scope, than without.

brendon commented 8 years ago

@swanandp, yes that's been my experience also. If @tinderfields can adjust the methods so that they cope with nil values or values with gaps between adjacent items that'd be a bonus :)

swanandp commented 8 years ago

👍 Agreed

brendon commented 8 years ago

@tinderfields, I can't find your Pull Request for this change. Can you make one or link it into here if it already exists?

If you'd also like to tweak the public methods to cope with nil values that would be a bonus (with tests of course). Let me know if you need help getting the test suite running etc...

brendon commented 7 years ago

Closing this for now. Let me know if you want to pursue it again @tinderfields.