From f993853db3e233f05a55de5ba2a87b77acf041aa Mon Sep 17 00:00:00 2001 From: erdgeist Date: Thu, 23 Jul 2026 17:49:18 +0200 Subject: Record asset deltas at publish, with changed assets as participants --- app/helpers/node_actions_helper.rb | 9 ++++++- app/models/node.rb | 8 ++++-- app/models/node_action.rb | 34 ++++++++++++++++++++++--- config/locales/de.yml | 3 +++ config/locales/en.yml | 3 +++ lib/tasks/node_actions.rake | 2 ++ test/models/node_action_test.rb | 9 +++---- test/models/node_test.rb | 52 ++++++++++++++++++++++++++++++++++++++ 8 files changed, 109 insertions(+), 11 deletions(-) diff --git a/app/helpers/node_actions_helper.rb b/app/helpers/node_actions_helper.rb index ed8d0407..4041ffa9 100644 --- a/app/helpers/node_actions_helper.rb +++ b/app/helpers/node_actions_helper.rb @@ -49,7 +49,7 @@ module NodeActionsHelper m = action.metadata return true if m["translation_diff"].present? return true if m["title"].is_a?(Hash) && m.dig("title", "from") != m.dig("title", "to") - %w[author tags template_changed assets_changed + %w[author tags template_changed assets assets_changed assets_reordered abstract_changed body_changed].any? { |key| m[key].present? } end @@ -69,6 +69,13 @@ module NodeActionsHelper items << t("node_actions.abstract_changed") if m["abstract_changed"] items << t("node_actions.body_changed") if m["body_changed"] items << t("node_actions.template_changed") if m["template_changed"] + if m["assets"] + items << t("node_actions.detail_assets_added", + :names => Array(m.dig("assets", "added")).join(", ")) if m.dig("assets", "added") + items << t("node_actions.detail_assets_removed", + :names => Array(m.dig("assets", "removed")).join(", ")) if m.dig("assets", "removed") + end + items << t("node_actions.assets_reordered") if m["assets_reordered"] items << t("node_actions.assets_changed") if m["assets_changed"] items end diff --git a/app/models/node.rb b/app/models/node.rb index 0a9cd2d1..274b2f94 100644 --- a/app/models/node.rb +++ b/app/models/node.rb @@ -249,7 +249,9 @@ class Node < ApplicationRecord self.head.save! self.draft = nil - NodeAction.record!(:node => self, :page => self.head, :user => current_user, + NodeAction.record!(:node => self, + :participants => [self] + NodeAction.changed_assets(outgoing_head, self.head), + :page => self.head, :user => current_user, :action => "publish", :via => "draft", **NodeAction.head_diff(outgoing_head, self.head)) end @@ -296,7 +298,9 @@ class Node < ApplicationRecord self.head = page self.save! - NodeAction.record!(:node => self, :page => page, :user => current_user, + NodeAction.record!(:node => self, + :participants => [self] + NodeAction.changed_assets(outgoing_head, page), + :page => page, :user => current_user, :action => "publish", :via => "revision", **NodeAction.head_diff(outgoing_head, page)) self diff --git a/app/models/node_action.rb b/app/models/node_action.rb index aa52f489..afa2195c 100644 --- a/app/models/node_action.rb +++ b/app/models/node_action.rb @@ -35,8 +35,16 @@ class NodeAction < ApplicationRecord # "title" -- pair, always; "from" null on first publish # "author" -- pair, when the byline changed (incl. first publish) # "tags" -- pair of arrays, when changed - # "assets_changed", "template_changed", - # "abstract_changed", "body_changed" + # "assets" -- {"added" => [asset names], "removed" => [asset names]}, + # keys only when any; a delta, not a pair. The event IS + # the delta, full sets would bloat every entry. Changed + # assets are participants of the entry. Replaces the + # legacy "assets_changed" boolean, which witnessed + # pre-contract entries still carry and the renderer keeps + # understanding. Assets destroyed since leave no trace in + # regenerated deltas, their joins died with them. + # "assets_reordered" -- boolean, set unchanged but gallery order not + # "template_changed", "abstract_changed", "body_changed" # -- the last two for the default locale; page_id links # to the revision for the real diff (never stored) # "translation_diff" -- only when a non-default locale differs: @@ -155,7 +163,17 @@ class NodeAction < ApplicationRecord diff[:tags] = { "from" => old_tags, "to" => new_tags } if old_tags != new_tags diff[:template_changed] = true if old_page.template_name != new_page.template_name - diff[:assets_changed] = true if old_page.assets.map(&:id) != new_page.assets.map(&:id) + + old_assets, new_assets = old_page.assets.to_a, new_page.assets.to_a + added, removed = new_assets - old_assets, old_assets - new_assets + if added.any? || removed.any? + assets = {} + assets["added"] = added.map { |a| a.name.presence || a.upload_file_name } if added.any? + assets["removed"] = removed.map { |a| a.name.presence || a.upload_file_name } if removed.any? + diff[:assets] = assets + elsif old_assets.map(&:id) != new_assets.map(&:id) + diff[:assets_reordered] = true + end old_t = old_page.translations.find_by(:locale => default) new_t = new_page.translations.find_by(:locale => default) @@ -188,6 +206,16 @@ class NodeAction < ApplicationRecord diff end + # The asset records added or removed between an outgoing head and its + # replacement -- the participant complement to head_diff's "assets" + # names. Empty on first publish, mirroring head_diff, which records + # no asset delta when everything is new. + def self.changed_assets old_page, new_page + return [] unless old_page + old_a, new_a = old_page.assets.to_a, new_page.assets.to_a + (new_a - old_a) | (old_a - new_a) + end + def actor_name metadata["username"] || "unknown" end diff --git a/config/locales/de.yml b/config/locales/de.yml index fff74799..b1594f19 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -122,6 +122,9 @@ de: asset_destroy: "%{actor} hat das Asset „%{asset}“ gelöscht" asset_destroy_detached: "— entfernt von %{paths}" asset_destroy_headlines: "(war Aufmacher von %{paths})" + detail_assets_added: "Anhänge hinzugefügt: %{names}" + detail_assets_removed: "Anhänge entfernt: %{names}" + assets_reordered: "Anhänge umsortiert" open_gallery: "Gallerie anzeigen" asset_licenses: diff --git a/config/locales/en.yml b/config/locales/en.yml index fc9f5d13..649c8812 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -74,6 +74,9 @@ en: asset_create: "%{actor} uploaded asset “%{asset}”" asset_attach: "%{actor} attached “%{asset}” to %{subject}" asset_attach_headline: "%{actor} attached “%{asset}” to %{subject} as its headline" + detail_assets_added: "Attachments added: %{names}" + detail_assets_removed: "Attachments removed: %{names}" + assets_reordered: "Attachments reordered" open_gallery: "Open gallery" asset_licenses: diff --git a/lib/tasks/node_actions.rake b/lib/tasks/node_actions.rake index c378db24..645315b5 100644 --- a/lib/tasks/node_actions.rake +++ b/lib/tasks/node_actions.rake @@ -13,6 +13,7 @@ namespace :node_actions do stale = NodeAction.where.not(:inferred_from => nil) stale = stale.where(:node_id => ENV["NODE_ID"]) if ENV["NODE_ID"] puts "Removing #{stale.count} previously inferred entries" + ActionParticipant.where(:node_action => stale).delete_all stale.delete_all witnessed_creates = NodeAction.where(:action => "create", :inferred_from => nil).pluck(:node_id).to_set @@ -43,6 +44,7 @@ namespace :node_actions do diff = NodeAction.head_diff(previous, page) NodeAction.record!( :node => node, :page => page, :user => page.editor, + :participants => [node] + NodeAction.changed_assets(previous, page), :action => "publish", :occurred_at => page.updated_at, :inferred_from => "from_page_revision", diff --git a/test/models/node_action_test.rb b/test/models/node_action_test.rb index 849b36f4..4672456a 100644 --- a/test/models/node_action_test.rb +++ b/test/models/node_action_test.rb @@ -59,7 +59,7 @@ class NodeActionTest < ActiveSupport::TestCase assert_nil NodeAction.head_diff(old_page, build_page(:template_name => "standard_template"))[:template_changed] end - test "assets_changed flag when the attached set differs" do + test "asset delta when the attached set differs" do asset = Asset.create!(:name => "diff probe", :upload_file_name => "test_image.png", :upload_content_type => "image/png", @@ -68,10 +68,9 @@ class NodeActionTest < ActiveSupport::TestCase old_page, new_page = build_page, build_page new_page.related_assets.create!(:asset_id => asset.id, :position => 1) - - diff = NodeAction.head_diff(old_page, new_page.reload) - assert diff[:assets_changed] - assert_nil NodeAction.head_diff(old_page, old_page)[:assets_changed] + diff = NodeAction.head_diff(old_page, new_page) + assert diff[:assets].present? + assert_nil NodeAction.head_diff(old_page, old_page)[:assets] end test "default-locale abstract and body changes become flags, only when true" do diff --git a/test/models/node_test.rb b/test/models/node_test.rb index 0083b088..8bdb90ee 100644 --- a/test/models/node_test.rb +++ b/test/models/node_test.rb @@ -728,6 +728,58 @@ class NodeTest < ActiveSupport::TestCase NodeAction.order(:id).last(2).map(&:action).sort end + test "publish records the asset delta with changed assets as participants" do + node = Node.root.children.create!(:slug => "publish_asset_delta") + kept = Asset.create!(:name => "Kept", :upload_content_type => "image/png") + added = Asset.create!(:name => "Added", :upload_content_type => "image/png") + node.draft.related_assets.create!(:asset => kept) + node.publish_draft!(@user1) + + node.lock_for_editing!(@user1) + node.create_new_draft(@user1) + node.draft.related_assets.create!(:asset => added) + node.publish_draft!(@user1) + + action = node.node_actions.where(:action => "publish").order(:id).last + assert_equal ["Added"], action.metadata.dig("assets", "added") + assert_nil action.metadata.dig("assets", "removed") + subjects = action.action_participants.map { |p| [p.subject_type, p.subject_id] } + assert_includes subjects, ["Asset", added.id] + assert_not_includes subjects, ["Asset", kept.id] + end + + test "publish without an asset change writes no assets key" do + node = Node.root.children.create!(:slug => "publish_asset_static") + node.draft.related_assets.create!(:asset => Asset.create!(:name => "Steady")) + node.publish_draft!(@user1) + + node.lock_for_editing!(@user1) + node.create_new_draft(@user1) + node.publish_draft!(@user1) + + action = node.node_actions.where(:action => "publish").order(:id).last + assert_nil action.metadata["assets"] + assert_equal [["Node", node.id]], + action.action_participants.map { |p| [p.subject_type, p.subject_id] } + end + + test "pure reordering is recorded as assets_reordered" do + node = Node.root.children.create!(:slug => "publish_asset_reorder") + a1, a2 = Asset.create!(:name => "First"), Asset.create!(:name => "Second") + node.draft.related_assets.create!(:asset => a1) + node.draft.related_assets.create!(:asset => a2) + node.publish_draft!(@user1) + + node.lock_for_editing!(@user1) + node.create_new_draft(@user1) + node.draft.related_assets.reload.first.move_to_bottom + node.publish_draft!(@user1) + + action = node.node_actions.where(:action => "publish").order(:id).last + assert action.metadata["assets_reordered"] + assert_nil action.metadata["assets"] + end + test "restore_revision! logs a publish via revision" do node = create_node_with_published_page Globalize.with_locale(:de) { node.head.update!(:title => "First") } -- cgit v1.3