Replace awesome_nested_set with a plain parent_id-based NestedTree concern

Root-caused this session: appending a child to any node never widened
that parent's own rgt boundary, on the pinned revision (Gemfile tracked
main directly, chasing a too-conservative gemspec constraint - not, as
first assumed, a deliberate pin to avoid a known bug). Reproduced
cleanly on a single ordinary create with no concurrency and no bulk
operation involved, confirmed via the gem's own SetValidator, then
confirmed as the root cause of nodes_controller_test.rb's 3 long-standing
"pre-existing" failures - not three separate mysteries, one bug.

admin_controller's sitemap needed its own real conversion, not just a
drop-in: awesome_nested_set's lft column implicitly provided correct
depth-first tree order for free, which the old code combined with a
separate class-level each_with_level iterator. Both replaced by one
method, self_and_descendants_ordered_with_level, computing an ordered
[node, level] list in a single query-then-walk pass - checked against
the actual view template first (admin/index.html.erb) rather than
assumed, since it relies on list order alone to render correct visual
nesting.

lft/rgt/depth columns intentionally left in schema, unused - dropping
them is a separate, deliberately deferred migration once this is proven
running for a while, not bundled with the behavior change.

3 files changed, 95 insertions, 24 deletions

diff --git a/app/controllers/admin_controller.rb b/app/controllers/admin_controller.rb
index bfc6cd8..3fa0519 100644
--- a/app/controllers/admin_controller.rb
+++ b/app/controllers/admin_controller.rb
@@ -15,12 +15,10 @@ class AdminController < ApplicationController
       Time.now, Time.now - 14.days
     ).limit(50).order("updated_at desc")
 
-    all_nodes = Node.root.self_and_descendants
+    ordered_with_level = Node.root.self_and_descendants_ordered_with_level
     @sitemap_depth = {}
-    Node.each_with_level(all_nodes) do |node, level|
-      @sitemap_depth[node.id] = level
-    end
-    @sitemap = all_nodes.to_a.sort! { |node1,node2| node1.lft <=> node2.lft }.delete_if { |node| node.update? }
+    ordered_with_level.each { |node, level| @sitemap_depth[node.id] = level }
+    @sitemap = ordered_with_level.map(&:first).reject(&:update?)
 
     @mypages = Page.where("user_id = ? or editor_id = ?", @current_user, @current_user)
 
diff --git a/app/models/concerns/nested_tree.rb b/app/models/concerns/nested_tree.rb
new file mode 100644
index 0000000..9a2eb0e
--- /dev/null
+++ b/app/models/concerns/nested_tree.rb
@@ -0,0 +1,86 @@
+# app/models/concerns/nested_tree.rb
+#
+# Minimal parent_id-based replacement for the tree-traversal subset of
+# awesome_nested_set actually used by this app (descendants, ancestors,
+# level, root, move_to_child_of)
+module NestedTree
+  extend ActiveSupport::Concern
+
+  included do
+    belongs_to :parent, class_name: name, foreign_key: :parent_id, optional: true, inverse_of: :children
+    has_many :children, -> { order(:id) }, class_name: name, foreign_key: :parent_id, inverse_of: :parent
+
+    before_destroy :delete_descendants
+  end
+
+  class_methods do
+    def root
+      roots.first
+    end
+
+    def roots
+      where(parent_id: nil)
+    end
+
+    def valid?
+      # Every non-root row's parent_id must point at a real, existing row.
+      where.not(parent_id: nil).where.missing(:parent).none?
+    end
+  end
+
+  def root?
+    parent_id.nil?
+  end
+
+  def ancestors
+    result = []
+    current = parent
+    while current
+      result << current
+      current = current.parent
+    end
+    result
+  end
+
+  def level
+    ancestors.size
+  end
+
+  def descendants
+    ids = []
+    queue = self.class.where(parent_id: id).pluck(:id)
+    until queue.empty?
+      ids.concat(queue)
+      queue = self.class.where(parent_id: queue).pluck(:id)
+    end
+    self.class.where(id: ids)
+  end
+
+  def self_and_descendants
+    self.class.where(id: [id] + descendants.pluck(:id))
+  end
+
+  def self_and_descendants_ordered_with_level
+    nodes = [self] + descendants.to_a
+    children_by_parent = nodes.group_by(&:parent_id)
+    children_by_parent.each_value { |list| list.sort_by!(&:id) }
+
+    result = []
+    visit = ->(node, level) do
+      result << [node, level]
+      (children_by_parent[node.id] || []).each { |child| visit.call(child, level + 1) }
+    end
+    visit.call(self, 0)
+    result
+  end
+
+  def move_to_child_of(new_parent)
+    update!(parent_id: new_parent.id)
+  end
+
+  private
+
+  def delete_descendants
+    self.class.where(id: descendants.pluck(:id)).delete_all
+  end
+end
diff --git a/app/models/node.rb b/app/models/node.rb
index 2177f15..fc23dc1 100644
--- a/app/models/node.rb
+++ b/app/models/node.rb
@@ -1,6 +1,6 @@
 class Node < ApplicationRecord
   # Mixins and Plugins
-  acts_as_nested_set
+  include NestedTree
 
   # Associations
   has_many    :pages, -> { order("revision ASC") }, :dependent => :destroy
@@ -21,16 +21,6 @@ class Node < ApplicationRecord
   validates_uniqueness_of :slug, :scope => :parent_id, :unless => -> { parent_id.nil? }
   validates_presence_of   :parent_id,                  :unless => -> { Node.root.nil? }
 
-  validate :borders       # This should never ever happen.
-
-  # Index for Fulltext Search
-  # define_index do
-  #   indexes head.translations.title
-  #   indexes slug
-  #   indexes unique_name
-  #   indexes head.translations.body
-  # end
-
   # Class methods
 
   # Returns a page for a given node. If no revision is supplied, it returns
@@ -254,10 +244,13 @@ class Node < ApplicationRecord
     # Watch out recursion ahead! update_unique_name itself triggers this
     # after_save callback which invokes update_unique_name on its children.
     # Hopefully until no childrens occur
+    #
+    # Queries parent_id directly rather than the NestedTree#children
+    # association out of habit from the old awesome_nested_set-avoidance
+    # workaround - no longer strictly necessary now that children is
+    # equally safe, but left as-is since it already works correctly.
     def update_unique_names_of_children
       unless root?
-        # Use parent_id-based traversal instead of lft/rgt descendants
-        # due to awesome_nested_set not refreshing parent lft/rgt in memory
         Node.where(:parent_id => self.id).each do |child|
           child.reload
           child.update_unique_name
@@ -265,10 +258,4 @@ class Node < ApplicationRecord
         end
       end
     end
-
-    def borders
-      if lft && rgt && (lft > rgt)
-        errors.add("Fuck!. lft should never be smaller than rgt")
-      end
-    end
 end