From d9e95bdb6436768c738539df74a164c885887174 Mon Sep 17 00:00:00 2001 From: erdgeist Date: Thu, 23 Jul 2026 03:28:55 +0200 Subject: Add action_participants, recording every node a trash/destroy touches --- app/models/node.rb | 16 +++++++++------- app/models/node_action.rb | 19 ++++++++++++++----- lib/tasks/node_actions.rake | 14 ++++++++++++++ test/models/node_action_test.rb | 20 +++++++++++++++++++- test/models/node_test.rb | 10 ++++++++++ 5 files changed, 66 insertions(+), 13 deletions(-) diff --git a/app/models/node.rb b/app/models/node.rb index a5a40d30..e92fa1ea 100644 --- a/app/models/node.rb +++ b/app/models/node.rb @@ -312,14 +312,14 @@ class Node < ApplicationRecord was_published = head_id.present? final_published_at = head&.published_at - demoted = 0 - ([self] + descendants.to_a).each do |node| - next unless node.head_id + subtree = [self] + descendants.to_a + demoted_nodes = subtree.select do |node| + next false unless node.head_id former = node.head node.head = nil node.draft_id = former.id if node.draft_id.nil? node.save! - demoted += 1 + true end self.reload @@ -332,9 +332,10 @@ class Node < ApplicationRecord metadata = { :path => { "from" => path_before, "to" => unique_name } } metadata[:was_published] = true if was_published metadata[:final_published_at] = final_published_at.iso8601 if final_published_at - metadata[:demoted_heads] = demoted if demoted > 0 + metadata[:demoted_heads] = demoted_nodes.size if demoted_nodes.any? - NodeAction.record!(:node => self, :user => current_user, :action => "trash", **metadata) + NodeAction.record!(:participants => subtree, :user => current_user, + :action => "trash", **metadata) self end end @@ -382,7 +383,8 @@ class Node < ApplicationRecord metadata = { :path => unique_name } metadata[:destroyed_descendants] = doomed.size - 1 if doomed.size > 1 - NodeAction.record!(:node => self, :user => current_user, :action => "destroy", **metadata) + NodeAction.record!(:participants => doomed, :user => current_user, + :action => "destroy", **metadata) doomed.each(&:destroy!) end end diff --git a/app/models/node_action.rb b/app/models/node_action.rb index 63a99ae8..8a3dd8b7 100644 --- a/app/models/node_action.rb +++ b/app/models/node_action.rb @@ -3,6 +3,8 @@ class NodeAction < ApplicationRecord belongs_to :page, optional: true belongs_to :user, optional: true + has_many :action_participants, :dependent => :destroy + validates :action, presence: true validates :occurred_at, presence: true @@ -81,10 +83,15 @@ class NodeAction < ApplicationRecord # This log records; it does not undo. No IP, session, or user # agent, ever. Success only. - def self.record!(node:, action:, user: nil, page: nil, locale: nil, - occurred_at: nil, inferred_from: nil, **extra) + def self.record!(node: nil, participants: [], action:, user: nil, page: nil, + locale: nil, occurred_at: nil, inferred_from: nil, **extra) + participants = participants.presence || [node].compact + raise ArgumentError, "NodeAction.record! needs at least one participant" if participants.empty? + + primary_node = node || (participants.first if participants.first.is_a?(Node)) + create!( - :node => node, + :node => primary_node, :page => page, :user => user, :action => action, @@ -94,10 +101,12 @@ class NodeAction < ApplicationRecord :metadata => { "username" => user&.login, "human_readable_node_name" => Globalize.with_locale(I18n.default_locale) { - node&.head&.title || node&.draft&.title + primary_node&.head&.title || primary_node&.draft&.title }, }.merge(extra.stringify_keys) - ) + ).tap do |na| + participants.each { |subject| na.action_participants.create!(:subject => subject) } + end end # Computes the publish-entry diff between an outgoing head and the diff --git a/lib/tasks/node_actions.rake b/lib/tasks/node_actions.rake index fdd286c1..c378db24 100644 --- a/lib/tasks/node_actions.rake +++ b/lib/tasks/node_actions.rake @@ -58,4 +58,18 @@ namespace :node_actions do puts "Created #{created} inferred entries" end + + desc "Backfill action_participants for existing single-subject entries. " \ + "Idempotent: entries that already have a participant row are skipped. " \ + "Multi-node trash/destroy entries predating this feature are NOT " \ + "reconstructable (only a count was ever stored) and are left as-is." + task :backfill_participants => :environment do + scope = NodeAction.where.missing(:action_participants).where.not(:node_id => nil) + created = 0 + scope.find_each do |action| + action.action_participants.create!(:subject => action.node) + created += 1 + end + puts "Created #{created} participant rows" + end end diff --git a/test/models/node_action_test.rb b/test/models/node_action_test.rb index b177ccac..849b36f4 100644 --- a/test/models/node_action_test.rb +++ b/test/models/node_action_test.rb @@ -118,11 +118,29 @@ class NodeActionTest < ActiveSupport::TestCase test "record! passes provenance and historical timestamps through" do long_ago = 2.years.ago - action = NodeAction.record!(:node => nil, :action => "publish", + node = Node.root.children.create!(:slug => "provenance_backfill_test") + action = NodeAction.record!(:node => node, :action => "publish", :occurred_at => long_ago, :inferred_from => "from_page_revision") assert_equal "from_page_revision", action.inferred_from assert_in_delta long_ago.to_f, action.occurred_at.to_f, 1.0 end + + + test "record! with participants: writes one action_participant per subject" do + n1, n2 = Node.root.children.create!(:slug => "participant_a"), Node.root.children.create!(:slug => "participant_b") + action = NodeAction.record!(:participants => [n1, n2], :action => "trash", :user => users(:quentin)) + assert_equal [n1, n2], action.action_participants.map(&:subject) + end + + test "record! with node: alone still writes exactly one participant" do + node = Node.root.children.create!(:slug => "participant_sugar") + action = NodeAction.record!(:node => node, :action => "create", :user => users(:quentin)) + assert_equal [node], action.action_participants.map(&:subject) + end + + test "record! refuses an empty participant set" do + assert_raises(ArgumentError) { NodeAction.record!(:action => "create", :user => users(:quentin)) } + end end diff --git a/test/models/node_test.rb b/test/models/node_test.rb index ba383405..0083b088 100644 --- a/test/models/node_test.rb +++ b/test/models/node_test.rb @@ -756,6 +756,16 @@ class NodeTest < ActiveSupport::TestCase assert node.valid? end + test "trash! records every subtree node as a participant, not just the root" do + parent = Node.root.children.create!(:slug => "trash_participants_parent") + child = parent.children.create!(:slug => "trash_participants_child") + + parent.trash!(users(:quentin)) + + action = parent.node_actions.where(:action => "trash").last + assert_includes action.action_participants.map(&:subject), child + end + test "destroying a node with children is refused" do parent = Node.root.children.create!(:slug => "destroy_guard_parent") parent.children.create!(:slug => "destroy_guard_child") -- cgit v1.3