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

Add class method acts_as_list_top as reader for configured top_of_list #213

Closed krzysiek1507 closed 8 years ago

brendon commented 8 years ago

Hi @krzysiek1507, could you give a bit of a use case?

Also, this change seems to be randomly breaking tests. The test suite is run in a random order each time so your additional class method is probably interfering in some way.

krzysiek1507 commented 8 years ago

Hi @brendon I need access to top_of_list when I want to reposition items in loop using set_list_position. I can get this from an instance, but I have to create a new object so this is not the best way.

Maybe we should add top_of_list to this new_position? There is no sens to set position to 0 if top_of_list is set to 100. Am I correct?

No, something like:

new_position = new_position < acts_as_list_top ? new_position + acts_as_list_top : new_position
krzysiek1507 commented 8 years ago

@brendon what do you think?

brendon commented 8 years ago

Hi @krzysiek1507, I'm not keen to change the way that method works. That'd be an unexpected change for a lot of people who may be relying on the fact that it just assigns a position without question.

In regards to your modification, I'm at a loss as to why the test cases are now all passing. What did you change to fix it? I just want to make sure it's not just a fluke run that worked.

krzysiek1507 commented 8 years ago

@brendon I've added previously missed setup method.

brendon commented 8 years ago

Thanks @krzysiek1507,

@swanandp and @fabn, are you ok with this addition?

fabn commented 8 years ago

For me it can be useful 👍

brendon commented 8 years ago

Done :)

krzysiek1507 commented 8 years ago

Thanks!