Only temporarily move item if using sequential updates

[tjwallace]

Otherwise we can assume there is no unique index or the constraint is deferred, meaning the temporary placement has no use. This decreases the number of UPDATE statements required.

[brendon]

Hi @tjwallace, you raise a good point. I like the optimisation.

Perhaps just as a courtesy, contact the person who introduced this feature and see if they're using sequential_updates? That'll be a good test as to whether people rely on this particular feature without enabling sequential_updates.

It'll still be a breaking change I think.

[tjwallace]

@zharikovpro would mind taking a look at this PR? You originally introduced sequential_updates in #246 to avoid problems with unique constraints.

[zharikovpro]

@brendon @tjwallace thanks for asking guys!

Perhaps just as a courtesy, contact the person who introduced this feature and see if they're using sequential_updates?

I'm not using it right now. Still it was merged to the gem and looks like it'll be good to keep this logic to avoid breaking changes? Other devs may use it as well.

Otherwise we can assume there is no unique index or the constraint is deferred, meaning the temporary placement has no use.

I believe this is the correct use of sequential_updates?. If it's false, when we could avoid additional updates, that's for sure :ok_hand:

This decreases the number of UPDATE statements required.

With that kind of optimization IMO it would be great to add the test to check the number of updates.

[brendon]

Thanks @zharikovpro. Indeed @tjwallace, we should probably have a test for this.

I think it's an ok change, but we'll have to bump a minor version to signal the change.

There is potentially some work around refactoring all of this logic and making the shuffling logic work with constraints at the database level: https://github.com/brendon/acts_as_list/pull/372

[brendon]

@tjwallace, I must apologise for this languishing. Is it still a concern for you? I'm still happy to pursue this but just require a test to ensure the case is covered. I'll close it for now since it's so old but we can reopen it at any point.

Check out my new gem that works around this issue in another way: https://github.com/brendon/positioning though we always assume a uniqueness constraint. We invert the position number to move them out of the way and then invert them again with the increment to make a gap. This requires we allow for 0 and negative positions.

[tjwallace]

Hi @brendon thanks for following up. I've switched jobs and don't use this gem anymore, so it is no longer a concern.

[brendon]

Thanks @tjwallace :D All the best!