Shuffling positions is halting the callback chain

[DavidVII]

Howdy, I've been having an issue where the callback chain is being halted around the update_positions callback. The issue goes away if I remove this line. The remaining callbacks fire just fine, but of course, the shuffle_positions_on_intermediate_items method doesn't execute.

Following that method, I tried various things to address the issue. I mostly tried to explicitly return true, but I wasn't able to get things work.

I'm running v0.8.2 and rails v4.2.7.1. I also ran the master version of this gem with no success. I'm happy to open a PR, but I'm struggling to see why exactly the shuffling method is halting the callback chain.

[brendon]

Hi @DavidVII, that certainly sounds strange. You'll hate me saying so, but it'd be helpful if you could try to reproduce this in a test in our test suite. Once we have a failing case seperate from your application, it'll be much easier to diagnose. I'd be surprised if you didn't discover the reason for the failure in that process anyway :)

Let me know how you get on.

The only way for a callback to halt the chain is for it to return false, so somehow that must be happening? In Rails 5 I think that's also changed to needing to specifically call some kind of halting method.

[DavidVII]

Hi @brendon, indeed this is a strange one. I did write tests, but they're within my application. I'll see if I can replicate within your test suite. If I do, I'll open up a PR and with any luck, it'll include a fix too.

It's a bit tricky cause the code in question doesn't appear to be returning false. However, whenever that line runs, the remaining callbacks do not fire. I'll keep hacking around.

[brendon]

Thanks @DavidVII, sounds good. :)

[DavidVII]

Hi @brendon, i've got an isolated test file here that replicates the issue.

The main idea here, is that my_callback should be triggered every time the position is updated. The callback simply adds one to the counter column. As you can see by running the file, the callback is only triggered once. All Status position columns get updated, but not all counter columns do.

Perhaps this has something to do with the update_all in this block of code?

Is this by design? Do we want to avoid callbacks here or should we always be firing those off? Perhaps there should be an option to allow users to choose?

[brendon]

Thanks for the test :) What would be even better would be if you brought it into our test suite proper then we can run it more easily on our end too. Create a PR with the failing test case.

I had one quick thought though. It's prone to create havoc if you kick off other database operations in after_ callbacks. In this case you're kicking off an update of a column which will in turn kick off the acts_as_list callbacks again. In your case perhaps the side effects aren't too bad, but sometimes you can end up in infinite loops etc...

If you're just testing the logic and in your real case you're not also updating the database in an after_ callback, then in your test case perhaps test that your callback method is called the expected number of times. This might help: http://stackoverflow.com/questions/10869417/how-to-assert-certain-method-is-called-with-ruby-minitest-framework

Let me know how you get on :)

[brendon]

@DavidVII, I'm closing this due to inactivity. Let me know if it's still a problem and we can reopen and investigate further. You'll also need to create a PR with a failing test case if you want us to look into it for you.