Fix position when no serial positions #208
Conversation
| @@ -256,9 +264,9 @@ def higher_items(limit=nil) | |||
| position_value = send(position_column) | |||
| acts_as_list_list. | |||
| where("#{quoted_table_name}.#{quoted_position_column} < ?", position_value). | |||
| where("#{quoted_table_name}.#{quoted_position_column} >= ?", position_value - limit). | |||
| # where("#{quoted_table_name}.#{quoted_position_column} >= ?", position_value - limit). | |||
There was a problem hiding this comment.
Could you give a quick explanation why this isn't needed anymore? :)
|
Thanks for this. It's a handy change. You need to add specific tests to test this scenario though. Also, rather than comment out code, just remove it and explain why. |
PoslinskiNet
force-pushed
the
fix-not-serial-positions
branch
2 times, most recently
from
43125b9 to
43972eb
Compare
|
@brendon I've applied your feedback. Hopefully, I will find some time to add specs later this week. |
PoslinskiNet
force-pushed
the
fix-not-serial-positions
branch
from
43972eb to
5630e62
Compare
|
Thanks @PoslinskiNet, quite a few existing tests are failing now so you'll need to address why that is. I suspect it's because you've changed some pretty core methods. You may want to run the test suite locally to ensure things aren't broken before pushing your changes to the PR. Just run |
PoslinskiNet
force-pushed
the
fix-not-serial-positions
branch
from
5630e62 to
f5806e3
Compare
|
@brendon I made some improvement, so most of the test are passing. However, 3 of them are still failing, so probably I will fix them later on. |
|
Thanks @PoslinskiNet, remember to also add those tests to test the functionality you're trying to add. :) |
|
👍 would be nice to have this fix in next release |
|
Hi @jpalumickas, thanks for the spec to test your new feature. Unfortunately we can't accept this PR until the entire test suite is green. Can you work on this when you have time? This is a great feature to support, so I look forward to seeing it merged :) |
|
@brendon @jpalumickas I've added some specs, but some other are still failing, and I don't have time to investigate it right now. If anyone sees the problem, I would be really grateful for the fix :). |
|
@PoslinskiNet this is because you changed order in |
Tasks:
[15, 25, 55].