From 71a95f4ca6460a7c2bc345e8b32dbef03d6dd985 Mon Sep 17 00:00:00 2001 From: erdgeist Date: Fri, 17 Jul 2026 22:11:44 +0200 Subject: Refuse destroying nodes that still have children NestedTree's before_destroy silently delete_all'd the whole subtree, bypassing every per-node cleanup. Nodes are never destroyed recursively; descendants must be removed individually. --- app/models/concerns/nested_tree.rb | 8 -------- app/models/node.rb | 16 +++++++++++++--- 2 files changed, 13 insertions(+), 11 deletions(-) (limited to 'app/models') diff --git a/app/models/concerns/nested_tree.rb b/app/models/concerns/nested_tree.rb index 9a2eb0e..befcdb3 100644 --- a/app/models/concerns/nested_tree.rb +++ b/app/models/concerns/nested_tree.rb @@ -9,8 +9,6 @@ module NestedTree 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 @@ -77,10 +75,4 @@ module NestedTree 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 70ed4da..6ce6591 100644 --- a/app/models/node.rb +++ b/app/models/node.rb @@ -12,9 +12,10 @@ class Node < ApplicationRecord belongs_to :lock_owner, :class_name => "User", :foreign_key => :locking_user_id, optional: true # Callbacks - after_create :initialize_empty_page - before_save :check_for_changed_slug - after_save :update_unique_names_of_children + after_create :initialize_empty_page + before_save :check_for_changed_slug + after_save :update_unique_names_of_children + before_destroy :refuse_destroy_with_children # Validations validates_length_of :slug, :within => 1..255, :unless => -> { parent_id.nil? || slug.blank? } @@ -366,6 +367,15 @@ class Node < ApplicationRecord ) end + # Nodes are never destroyed recursively + # Descendants must be removed or reparented individually first. + # The Trash feature will be the ordinary path to deletion. + def refuse_destroy_with_children + return unless children.exists? + errors.add(:base, "Cannot destroy a node that still has children") + throw :abort + end + def self.recently_changed includes(:head).where( "pages.updated_at < ? AND pages.updated_at > ? AND nodes.parent_id IS NOT NULL", -- cgit v1.3