Sorting in admin sortable index miscalculates indices when using acts_as_list plus ancestry

[armchairdj]

I'm using Acts as List + Ancestry to create a sortable tree for my model, and I'm using this gem to generate a drag-and-drop sortable index.

Relevant code here:

# model
has_ancestry cache_depth: true, orphan_strategy: :rootify
acts_as_list scope: [:ancestry], top_of_list: 0

# active_admin
sortable tree: true, max_levels: 2

index :as => :sortable do
  label :name

  actions
end

Based on that combination, I expect that when I create a bunch of records, indices will be calculated starting at 0 in each subnode of my tree structure. So if I start with this:

{
  { id: 0, ancestry: nil, position: 0, name: "Beatles"} => {
    { id: 1, ancestry: "0", position: 0, name: "John Lennon"     } => {},
    { id: 2, ancestry: "0", position: 1, name: "Paul McCartney"  } => {},
    { id: 3, ancestry: "0", position: 2, name: "Ringo Star"      } => {},
    { id: 4, ancestry: "0", position: 3, name: "George Harrison" } => {}
  },
  { id: 5, ancestry: nil, position: 1, name: "Kate Bush" } => {
    # Empty
  },
  { id: 6, ancestry: nil, position: 2, name: "Pink Floyd" } => {
    { id: 7, ancestry: "6", position: 0, name: "Syd Barett"      } => {},
    { id: 8, ancestry: "6", position: 1, name: "Roger Waters"    } => {},
    { id: 9, ancestry: "6", position: 2, name: "David Gilmour"   } => {}
  }
}

I can then call this:

Node.find(0).move_to_bottom
Node.find(9).move_to_top

And end up with this:

{
  { id: 5, ancestry: nil, position: 1, name: "Kate Bush" } => {
    # Empty
  },
  { id: 6, ancestry: nil, position: 2, name: "Pink Floyd" } => {
    { id: 9, ancestry: "6", position: 2, name: "David Gilmour"   } => {},
    { id: 7, ancestry: "6", position: 0, name: "Syd Barett"      } => {},
    { id: 8, ancestry: "6", position: 1, name: "Roger Waters"    } => {}
  },
  { id: 0, ancestry: nil, position: 0, name: "Beatles" } => {
    { id: 1, ancestry: "0", position: 0, name: "John Lennon"     } => {},
    { id: 2, ancestry: "0", position: 1, name: "Paul McCartney"  } => {},
    { id: 3, ancestry: "0", position: 2, name: "Ringo Star"      } => {},
    { id: 4, ancestry: "0", position: 3, name: "George Harrison" } => {}
  }
}

The acts_as_list methods are correctly executing the move commands within the context of the scope, AND the positions within each level of the tree are calculated correctly, starting at 0 within each ancestry depth.

However, when I revert to the original ordering and perform the same operations within my admin panel, I end up with this:

{
  { id: 5, ancestry: nil, position: 0, name: "Kate Bush" } => {
    # Empty
  },
  { id: 6, ancestry: nil, position: 1, name: "Pink Floyd" } => {
    { id: 9, ancestry: "6", position: 2, name: "David Gilmour"   } => {},
    { id: 7, ancestry: "6", position: 3, name: "Syd Barett"      } => {},
    { id: 8, ancestry: "6", position: 4, name: "Roger Waters"    } => {}
  },
  { id: 0, ancestry: nil, position: 5, name: "Beatles" } => {
    { id: 1, ancestry: "0", position: 6, name: "John Lennon"     } => {},
    { id: 2, ancestry: "0", position: 7, name: "Paul McCartney"  } => {},
    { id: 3, ancestry: "0", position: 8, name: "Ringo Star"      } => {},
    { id: 4, ancestry: "0", position: 9, name: "George Harrison" } => {}
  }
}

As you can see, active_admin-sortable_tree is generating positions without regard to scope. This hoses the position sequence of my entire model because when I use acts_as_list methods directly in the future, it assumes the indices will be relative to scope: :ancestry.

Am I doing something wrong in configuring my admin panel? Or is this a bug? Please advise.

[zorab47]

Hmm.. I've yet to use ActsAsList. Do you have a sample app to illustrate? I do know that the indexes are only calculated from zero at the top most node then it iterates from top to bottom incrementing the indexes to be saved to the DB.

Are you managing the list in two separate locations with only portions of the overall tree (a subtree)?

[armchairdj]

I have a sample app, but unfortunately it's private. I could give you read-only access if it would help.

I'm not using a subtree.

You're right that, by default, acts_as_list calculates indices from the top of the list. But you can give it a scope - meaning another field within the same model - and then it will calculate an index for each unique scope value.

When using with the ancestry gem, the scope is the ancestry field, which is ancestry's field for storing the ancestors of a given node as a colon-delimited list.

Here's the model code:

has_ancestry cache_depth: true, orphan_strategy: :rootify
acts_as_list scope: [:ancestry], top_of_list: 0

My examples above demonstrate expected and actual position columns when using acts_as_list methods to resort vs. using active_admin-sortable_tree.

That's because active_admin-sortable_tree assumes there will be a single index sequence for position, so it doesn't really work with this combination. I had to fork active_admin-sortable_tree and completely rewrite the internals of the sort method to work correctly for this use case. My hacky code isn't suitable for a PR - I pretty much destroyed active_admin-sortable_tree's ability to work in any other use case - but I could see a couple of ways of extending the gem to keep it agnostic but also support this use case:

1. Add an option to take the array of arrays that the JavaScript sends to the gem and just pass that up to a model method in the app that is using active_admin-sortable_tree. That way the calling model could implement its own position generation. That would allow active_admin-sortable_tree to work with a variety of corner cases where the built-in sorting doesn't work while still allowing people w/ those corner cases to leverage all of the other stuff in the gem - the JavaScript, the views, the active_admin customization, etc.
2. Add a special case within the gem for this use case, on the basis that ancestry and acts_as_list are both extremely popular gems. This could be triggered by detecting the ancestry/acts_as_list combination automatically or just offering an option to trigger the special case.

If either of those appeals, I'd be happy to submit a PR.

Either way, the documentation for active_admin-sortable_tree specifically mentions both acts_as_list and ancestry, so the documentation could probably be updated to call out incompatibility with this specific combination.

[armchairdj]

By the way, here's my hacky code:

    def sortable(options = {})
      options.reverse_merge! :sorting_attribute => :position,
                             :parent_method => :parent,
                             :children_method => :children,
                             :roots_method => :roots,
                             :tree => false,
                             :max_levels => 0,
                             :protect_root => false,
                             :collapsible => false, #hides +/- buttons
                             :start_collapsed => false,
                             :sortable => true

      # BAD BAD BAD FIXME: don't pollute original class
      @sortable_options = options

      # disable pagination
      config.paginate = false

      collection_action :sort, :method => :post do
        resource_name = ActiveAdmin::SortableTree::Compatibility.normalized_resource_name(active_admin_config.resource_name)

        records = params[resource_name].each_pair.map do |resource, parent_resource|
          record        = resource_class.find(resource)
          parent_record = resource_class.find(parent_resource) rescue nil

          [record, parent_record]
        end

        success = true

        ActiveRecord::Base.transaction do
          begin
            # Here's where I just call my own model method
            resource_class.resort(records)
          rescue => err
            success = false
          end
        end

        if success
          head 200
        else
          render json: {}, status: 422
        end
      end
    end

And here's the #resort method I call above, using the raw records array (two-dimensional array of model instances and their parents):

    # Re-sort the entire collection based on data from
    # active_admin-sortable_tree
    def resort(nested_array)
      # I'm going to remap the 2D array into a hash of scopes so each can have their own index.
      # Keys will be nil (for roots) or the model instance of the parent
      scopes = {}

      nested_array.each do |(record, parent_record)|
        scopes[parent_record] ||= []

        scopes[parent_record] << record
      end

      self.acts_as_list_no_update do
        scopes.each_pair do |parent_record, records|
          records.each.with_index(0) do |record, index|
            record.parent   = parent_record
            record.position = index

            record.save!
          end
        end
      end
    end

[zorab47]

Great detail, thank you. I'll read up more on ActsAsList to be more informed before I can decide on the best action to take.