Reparenting child leaves gap in source list in rails 5

[thefloweringash]

When moving an item from one list to another list by assigning to the list relation, the positions of the source list aren't updated. Tested under acts_as_list d605d17f6966c03d4e547fc43f6c52c774dde706 and version 0.7.2.

Test case:

#!/usr/bin/env ruby
require 'sqlite3'
require 'active_record'
require 'acts_as_list'
require 'logger'

ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Base.establish_connection adapter: 'sqlite3', database: ':memory:'

ActiveRecord::Schema.define do
  self.verbose = false
  create_table "comments", force: :cascade do |t|
    t.integer "photo_id"
    t.integer "position"
  end

  add_index "comments", ["photo_id"], name: "index_comments_on_photo_id"

  create_table "photos", force: :cascade do |t|
    t.string "title"
  end
end

class Photo < ActiveRecord::Base
  has_many :comments, -> { order(position: :asc) }
end

class Comment < ActiveRecord::Base
  belongs_to :photo, validate: false # only validate owned data
  acts_as_list scope: :photo
end

def test_list_reparenting
  p1 = Photo.new(title: "parent 1")
  p1.save!
  p1.comments = [Comment.new, Comment.new]
  p1.save!

  p1.reload

  puts "p1 list positions: " + p1.comments.pluck(:position).inspect

  puts "Reparenting"

  p2 = Photo.new(title: "parent 2")
  p2.save!
  p2.comments = [p1.comments[0]]
  p2.save!

  p1.reload
  p2.reload

  puts "p1 list positions: " + p1.comments.pluck(:position).inspect
  puts "p2 list positions: " + p2.comments.pluck(:position).inspect
end

test_list_reparenting

Under rails 4.2.6 I see

p1 list positions: [1, 2]
Reparenting
p1 list positions: [1]
p2 list positions: [1]

While under rails 5.0.0beta3 I see

p1 list positions: [1, 2]
Reparenting
p1 list positions: [2]
p2 list positions: [1]

[brendon]

Thanks for that @thefloweringash :)

At a glance, it's probably to do with the hacky way we swap the changed attributes with their original values in order to remove the item from the first list when the scope changes:

      # Temporarily swap changes attributes with current attributes
      def swap_changed_attributes
        @changed_attributes.each do |k, _|
          if self.class.column_names.include? k
            @changed_attributes[k], self[k] = self[k], @changed_attributes[k]
          end
        end
      end

      def check_scope
        if scope_changed?
          swap_changed_attributes
          send('decrement_positions_on_lower_items') if lower_item
          swap_changed_attributes
          send("add_to_list_#{add_new_at}")
        end
      end

Are you able to do some testing around that and let me know?

I don't think there'd be much harm is repeating the contents of decrement_positions_on_lower_items in check_scope and modifying it for the purpose (moving an item from one scope to the other) instead of trying to hackily reuse the existing method.

[brendon]

@thefloweringash, can you test again on master? I've rewritten that part of the code now to fix a problem with Rails 5.

[thefloweringash]

This resolves the issue for me. Tested against revision a3b1d4ec6a31c713a57d31cdf24e407407c24b5b. Thanks!

[brendon]

That's great to hear! :) You're welcome :)