Closed seanabrahams closed 6 years ago
Hi Sean, do you think insert_at should maybe raise an error rather than return false if used on a new object? I suppose the way you've done it means that it behaves similarly to setting the position property and then saving.
Good question. I think we can add an insert_at!
method that can raise an error. This way we follow Rails convention. What do you think?
Yes good call on that :) Then that gives users the choice. Once that's done I'll merge this. :)
Thanks @seanabrahams, that looks ok to me. We will have to drop the keyword arguments though since the test suite still supports Ruby 1.9. There also seem to be some other intermittent failures in there.
@swanandp, what's your view on Ruby 1.9 support now? I guess it depends on if we want to support Rails 3.2 anymore.
No worries if it needs to be refactored to not use keyword arguments. Passing a method argument down the call chain isn't the most elegant implementation but it looked like significant re-engineering would be required otherwise. Happy to discuss alternatives.
Thanks @seanabrahams, I think yes remove the keyword arguments for now. That's something to look at in a more major refactor. Then lets see how the test suite goes. :)
Sorry @seanabrahams, I didn't see your update. I think you have to comment for me to be notified again :) This looks good and it looks like the test suite is fixed too. I'll merge this now.
Closes #311
Released as 0.9.14
Initial fix for https://github.com/swanandp/acts_as_list/issues/311