Callbacks for Position Columns Affect All Position Columns When Only One Is Updated

What's the issue?

When updating a single position column in a model configured with multiple positioned columns, the gem triggers updates for all position columns one after the other.

Example

I have a model, RankingNode with four position columns (half_ppr_position, full_ppr_position, non_ppr_position, te_premium_position), updating one of these columns (half_ppr_position) results in updates for all four columns:

What I think is happening:

A before_update callback gets defined for each column defined by positioned:

positioning/lib/positioning.rb

Line 53 in 116aaf5

before_update { Mechanisms.new(self, column).update_position }

It calls the update method which eventually checks if the positioning_scope_changed?method to determine if it should update:

positioning/lib/positioning/mechanisms.rb

Line 27 in 116aaf5

if positioning_scope_changed? || position_changed?

This positioning_scope_changed? method is checking whether any of the positioning columns has changed (which returns true bc we changed half_ppr_position)

positioning/lib/positioning/mechanisms.rb

Line 179 in 116aaf5

def positioning_scope_changed?

I think it could be fixed with modifying the before_update call:

          before_update { Mechanisms.new(self, column).update_position if send(:"#{column}_changed?") }

Hi @johnlukeG, thanks for raising the issue. I can kind of see what you're getting at but I think this needs more exploration. Your proposed solution wouldn't work because it doesn't catch the case where the scope has changed but you haven't set an explicit position and in that case we still want to process things so that your item gets moved to the end of the new scope.

Looking at the code, the only way I can see that positioning_scope_changed? would return true for your other positionings (for want of a better word) is if they share the same scope columns or at least some of them. This is the check:

def positioning_columns
  base_class.positioning_columns[@column]
end

def positioning_scope_changed?
  positioning_columns.any? do |scope_component|
    @positioned.attribute_changed?(scope_component)
  end
end

positioning_columns are the scope columns that you declared when you set up each of your your positionings.

Correct me if I'm wrong, but right now we'll detect a scope change if you change the value of one of those columns. This will signal to us that the scope of each positioning that relies on that column has changed. We now definitely need to run the callback for each positioning affected. From here it can go two ways, and you can decide individually for each positioning affected. You can either specify the desired position for the item in its new scope, or leave it alone and we'll move that item to the end of the scope. You can mix and match here with some positions being explicit and the others just moving to the end of the list.

Your next step would probably be to post your usage of positioned in your model here so that I can see how you're using it.

If you're sure there's a bug, have a go at creating a failing test with the test suite and I'll take a deeper dive :)

@johnlukeG, do you still need help with this? If not, do you mind if I close the issue?

Closing this due to inactivity. Get back to me @johnlukeG if you want to resurrect it.