Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
tinderfields opened this issue Jun 21, 2016 · 7 comments
Closed

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

tinderfields opened this issue Jun 21, 2016 · 7 comments

Comments

@tinderfields
Copy link

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
Copy link
Owner

brendon commented Jun 21, 2016

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.

paulodeon added a commit to tinderfields/acts_as_list that referenced this issue Jun 22, 2016
@tinderfields
Copy link
Author

Done

@swanandp
Copy link
Collaborator

@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
Copy link
Owner

brendon commented Jun 22, 2016

@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
Copy link
Collaborator

👍 Agreed

paulodeon added a commit to tinderfields/acts_as_list that referenced this issue Jun 23, 2016
@brendon
Copy link
Owner

brendon commented Jul 15, 2016

@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
Copy link
Owner

brendon commented Nov 27, 2016

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

@brendon brendon closed this as completed Nov 27, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants