Attempt to fix unwanted position update

[multiscan]

The function position_changed? returns true even if the position remains the same. This because the corresponding method on the positioned object (@positioned.position_changed?) returns true due to the following code in positioning.rb and, in particular the first line of the redefined method:

          redefine_method(:"#{column}=") do |position|
            send :"#{column}_will_change!"
            super(position)
          end

Probably, my change could be moved to the above method but, since I don't really understand the reason behind that will_change!, I think that this version is less risky.

[brendon]

Hi @multiscan, thanks for this :)

The reason we signal that the position column has changed even if it's value hasn't is due to scope change detection. You could imaging that if you changed the scope of an item and also sent along its desired position but the position just happened to be the same as the items position in the previous scope, then we wouldn't detect the position change and instead the item would be put at the end of the new scope.

send :"#{column}_will_change!" will only be called if you explicitly set a position as part of the parameters sent to update() so the solution might be to not send that parameter if you're not changing it.

I'm certainly open to looking at this further with you if you think I've missed something. It's one of the uglier parts of this gem for sure. I did try hard to avoid it (since we do it in acts_as_list too) but unfortunately I couldn't think of another way around it. Rails dirty checking literally won't signal that an attribute was touched if its value wasn't changed.

[multiscan]

Hello @brendon and thank you very much for your quick reply.
Your explaination makes me understand better. I dug deeper into the problem that made me start this MR and indeed found that the problem was not in the code but in my usage: in my seed files I was assigning position 0 to the first element. Then, when I was updating another element of the list, the update_position method was called. And, due to the fact that position_changed? is always true (at least for integer positions), the move_out_of_the_way method were called which were trying to temporarily assignposition=0. At that point, the database were refusing the record complaining that the value 0 for position were already taken. This again, is due to the fact that my seed data were wrongly presuming that the position could start from 0.
Now that I have fixed my seed data and, I no longer have issues with your original gem.
Therefore I think I will close my useless MR.
Sorry for the waste of time.
Best,
giova

[brendon]

Don't apologise @multiscan! I'm glad you were able to get to the root of the problem :) That was a tricky one for sure.

ActsAsList does allow you to start from 0 so I can understand the confusion. Unfortunately we can only guard against that at the Active Record layer and it sounds like you were outside of that.

All the best! And be in touch if you need any assistance in the future :)