-
-
Notifications
You must be signed in to change notification settings - Fork 357
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
Shuffling positions is halting the callback chain #234
Comments
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. |
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. |
Thanks @DavidVII, sounds good. :) |
Hi @brendon, i've got an isolated test file here that replicates the issue. The main idea here, is that Perhaps this has something to do with the 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? |
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 If you're just testing the logic and in your real case you're not also updating the database in an Let me know how you get on :) |
@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. |
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, theshuffle_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.
The text was updated successfully, but these errors were encountered: