summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--app/helpers/node_actions_helper.rb9
-rw-r--r--app/models/node.rb8
-rw-r--r--app/models/node_action.rb34
-rw-r--r--config/locales/de.yml3
-rw-r--r--config/locales/en.yml3
-rw-r--r--lib/tasks/node_actions.rake2
-rw-r--r--test/models/node_action_test.rb9
-rw-r--r--test/models/node_test.rb52
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
49 m = action.metadata 49 m = action.metadata
50 return true if m["translation_diff"].present? 50 return true if m["translation_diff"].present?
51 return true if m["title"].is_a?(Hash) && m.dig("title", "from") != m.dig("title", "to") 51 return true if m["title"].is_a?(Hash) && m.dig("title", "from") != m.dig("title", "to")
52 %w[author tags template_changed assets_changed 52 %w[author tags template_changed assets assets_changed assets_reordered
53 abstract_changed body_changed].any? { |key| m[key].present? } 53 abstract_changed body_changed].any? { |key| m[key].present? }
54 end 54 end
55 55
@@ -69,6 +69,13 @@ module NodeActionsHelper
69 items << t("node_actions.abstract_changed") if m["abstract_changed"] 69 items << t("node_actions.abstract_changed") if m["abstract_changed"]
70 items << t("node_actions.body_changed") if m["body_changed"] 70 items << t("node_actions.body_changed") if m["body_changed"]
71 items << t("node_actions.template_changed") if m["template_changed"] 71 items << t("node_actions.template_changed") if m["template_changed"]
72 if m["assets"]
73 items << t("node_actions.detail_assets_added",
74 :names => Array(m.dig("assets", "added")).join(", ")) if m.dig("assets", "added")
75 items << t("node_actions.detail_assets_removed",
76 :names => Array(m.dig("assets", "removed")).join(", ")) if m.dig("assets", "removed")
77 end
78 items << t("node_actions.assets_reordered") if m["assets_reordered"]
72 items << t("node_actions.assets_changed") if m["assets_changed"] 79 items << t("node_actions.assets_changed") if m["assets_changed"]
73 items 80 items
74 end 81 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
249 self.head.save! 249 self.head.save!
250 self.draft = nil 250 self.draft = nil
251 251
252 NodeAction.record!(:node => self, :page => self.head, :user => current_user, 252 NodeAction.record!(:node => self,
253 :participants => [self] + NodeAction.changed_assets(outgoing_head, self.head),
254 :page => self.head, :user => current_user,
253 :action => "publish", :via => "draft", 255 :action => "publish", :via => "draft",
254 **NodeAction.head_diff(outgoing_head, self.head)) 256 **NodeAction.head_diff(outgoing_head, self.head))
255 end 257 end
@@ -296,7 +298,9 @@ class Node < ApplicationRecord
296 self.head = page 298 self.head = page
297 self.save! 299 self.save!
298 300
299 NodeAction.record!(:node => self, :page => page, :user => current_user, 301 NodeAction.record!(:node => self,
302 :participants => [self] + NodeAction.changed_assets(outgoing_head, page),
303 :page => page, :user => current_user,
300 :action => "publish", :via => "revision", 304 :action => "publish", :via => "revision",
301 **NodeAction.head_diff(outgoing_head, page)) 305 **NodeAction.head_diff(outgoing_head, page))
302 self 306 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
35 # "title" -- pair, always; "from" null on first publish 35 # "title" -- pair, always; "from" null on first publish
36 # "author" -- pair, when the byline changed (incl. first publish) 36 # "author" -- pair, when the byline changed (incl. first publish)
37 # "tags" -- pair of arrays, when changed 37 # "tags" -- pair of arrays, when changed
38 # "assets_changed", "template_changed", 38 # "assets" -- {"added" => [asset names], "removed" => [asset names]},
39 # "abstract_changed", "body_changed" 39 # keys only when any; a delta, not a pair. The event IS
40 # the delta, full sets would bloat every entry. Changed
41 # assets are participants of the entry. Replaces the
42 # legacy "assets_changed" boolean, which witnessed
43 # pre-contract entries still carry and the renderer keeps
44 # understanding. Assets destroyed since leave no trace in
45 # regenerated deltas, their joins died with them.
46 # "assets_reordered" -- boolean, set unchanged but gallery order not
47 # "template_changed", "abstract_changed", "body_changed"
40 # -- the last two for the default locale; page_id links 48 # -- the last two for the default locale; page_id links
41 # to the revision for the real diff (never stored) 49 # to the revision for the real diff (never stored)
42 # "translation_diff" -- only when a non-default locale differs: 50 # "translation_diff" -- only when a non-default locale differs:
@@ -155,7 +163,17 @@ class NodeAction < ApplicationRecord
155 diff[:tags] = { "from" => old_tags, "to" => new_tags } if old_tags != new_tags 163 diff[:tags] = { "from" => old_tags, "to" => new_tags } if old_tags != new_tags
156 164
157 diff[:template_changed] = true if old_page.template_name != new_page.template_name 165 diff[:template_changed] = true if old_page.template_name != new_page.template_name
158 diff[:assets_changed] = true if old_page.assets.map(&:id) != new_page.assets.map(&:id) 166
167 old_assets, new_assets = old_page.assets.to_a, new_page.assets.to_a
168 added, removed = new_assets - old_assets, old_assets - new_assets
169 if added.any? || removed.any?
170 assets = {}
171 assets["added"] = added.map { |a| a.name.presence || a.upload_file_name } if added.any?
172 assets["removed"] = removed.map { |a| a.name.presence || a.upload_file_name } if removed.any?
173 diff[:assets] = assets
174 elsif old_assets.map(&:id) != new_assets.map(&:id)
175 diff[:assets_reordered] = true
176 end
159 177
160 old_t = old_page.translations.find_by(:locale => default) 178 old_t = old_page.translations.find_by(:locale => default)
161 new_t = new_page.translations.find_by(:locale => default) 179 new_t = new_page.translations.find_by(:locale => default)
@@ -188,6 +206,16 @@ class NodeAction < ApplicationRecord
188 diff 206 diff
189 end 207 end
190 208
209 # The asset records added or removed between an outgoing head and its
210 # replacement -- the participant complement to head_diff's "assets"
211 # names. Empty on first publish, mirroring head_diff, which records
212 # no asset delta when everything is new.
213 def self.changed_assets old_page, new_page
214 return [] unless old_page
215 old_a, new_a = old_page.assets.to_a, new_page.assets.to_a
216 (new_a - old_a) | (old_a - new_a)
217 end
218
191 def actor_name 219 def actor_name
192 metadata["username"] || "unknown" 220 metadata["username"] || "unknown"
193 end 221 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:
122 asset_destroy: "%{actor} hat das Asset „%{asset}“ gelöscht" 122 asset_destroy: "%{actor} hat das Asset „%{asset}“ gelöscht"
123 asset_destroy_detached: "— entfernt von %{paths}" 123 asset_destroy_detached: "— entfernt von %{paths}"
124 asset_destroy_headlines: "(war Aufmacher von %{paths})" 124 asset_destroy_headlines: "(war Aufmacher von %{paths})"
125 detail_assets_added: "Anhänge hinzugefügt: %{names}"
126 detail_assets_removed: "Anhänge entfernt: %{names}"
127 assets_reordered: "Anhänge umsortiert"
125 128
126 open_gallery: "Gallerie anzeigen" 129 open_gallery: "Gallerie anzeigen"
127 asset_licenses: 130 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:
74 asset_create: "%{actor} uploaded asset “%{asset}”" 74 asset_create: "%{actor} uploaded asset “%{asset}”"
75 asset_attach: "%{actor} attached “%{asset}” to %{subject}" 75 asset_attach: "%{actor} attached “%{asset}” to %{subject}"
76 asset_attach_headline: "%{actor} attached “%{asset}” to %{subject} as its headline" 76 asset_attach_headline: "%{actor} attached “%{asset}” to %{subject} as its headline"
77 detail_assets_added: "Attachments added: %{names}"
78 detail_assets_removed: "Attachments removed: %{names}"
79 assets_reordered: "Attachments reordered"
77 80
78 open_gallery: "Open gallery" 81 open_gallery: "Open gallery"
79 asset_licenses: 82 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
13 stale = NodeAction.where.not(:inferred_from => nil) 13 stale = NodeAction.where.not(:inferred_from => nil)
14 stale = stale.where(:node_id => ENV["NODE_ID"]) if ENV["NODE_ID"] 14 stale = stale.where(:node_id => ENV["NODE_ID"]) if ENV["NODE_ID"]
15 puts "Removing #{stale.count} previously inferred entries" 15 puts "Removing #{stale.count} previously inferred entries"
16 ActionParticipant.where(:node_action => stale).delete_all
16 stale.delete_all 17 stale.delete_all
17 18
18 witnessed_creates = NodeAction.where(:action => "create", :inferred_from => nil).pluck(:node_id).to_set 19 witnessed_creates = NodeAction.where(:action => "create", :inferred_from => nil).pluck(:node_id).to_set
@@ -43,6 +44,7 @@ namespace :node_actions do
43 diff = NodeAction.head_diff(previous, page) 44 diff = NodeAction.head_diff(previous, page)
44 NodeAction.record!( 45 NodeAction.record!(
45 :node => node, :page => page, :user => page.editor, 46 :node => node, :page => page, :user => page.editor,
47 :participants => [node] + NodeAction.changed_assets(previous, page),
46 :action => "publish", 48 :action => "publish",
47 :occurred_at => page.updated_at, 49 :occurred_at => page.updated_at,
48 :inferred_from => "from_page_revision", 50 :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
59 assert_nil NodeAction.head_diff(old_page, build_page(:template_name => "standard_template"))[:template_changed] 59 assert_nil NodeAction.head_diff(old_page, build_page(:template_name => "standard_template"))[:template_changed]
60 end 60 end
61 61
62 test "assets_changed flag when the attached set differs" do 62 test "asset delta when the attached set differs" do
63 asset = Asset.create!(:name => "diff probe", 63 asset = Asset.create!(:name => "diff probe",
64 :upload_file_name => "test_image.png", 64 :upload_file_name => "test_image.png",
65 :upload_content_type => "image/png", 65 :upload_content_type => "image/png",
@@ -68,10 +68,9 @@ class NodeActionTest < ActiveSupport::TestCase
68 old_page, new_page = build_page, build_page 68 old_page, new_page = build_page, build_page
69 new_page.related_assets.create!(:asset_id => asset.id, :position => 1) 69 new_page.related_assets.create!(:asset_id => asset.id, :position => 1)
70 70
71 71 diff = NodeAction.head_diff(old_page, new_page)
72 diff = NodeAction.head_diff(old_page, new_page.reload) 72 assert diff[:assets].present?
73 assert diff[:assets_changed] 73 assert_nil NodeAction.head_diff(old_page, old_page)[:assets]
74 assert_nil NodeAction.head_diff(old_page, old_page)[:assets_changed]
75 end 74 end
76 75
77 test "default-locale abstract and body changes become flags, only when true" do 76 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
728 NodeAction.order(:id).last(2).map(&:action).sort 728 NodeAction.order(:id).last(2).map(&:action).sort
729 end 729 end
730 730
731 test "publish records the asset delta with changed assets as participants" do
732 node = Node.root.children.create!(:slug => "publish_asset_delta")
733 kept = Asset.create!(:name => "Kept", :upload_content_type => "image/png")
734 added = Asset.create!(:name => "Added", :upload_content_type => "image/png")
735 node.draft.related_assets.create!(:asset => kept)
736 node.publish_draft!(@user1)
737
738 node.lock_for_editing!(@user1)
739 node.create_new_draft(@user1)
740 node.draft.related_assets.create!(:asset => added)
741 node.publish_draft!(@user1)
742
743 action = node.node_actions.where(:action => "publish").order(:id).last
744 assert_equal ["Added"], action.metadata.dig("assets", "added")
745 assert_nil action.metadata.dig("assets", "removed")
746 subjects = action.action_participants.map { |p| [p.subject_type, p.subject_id] }
747 assert_includes subjects, ["Asset", added.id]
748 assert_not_includes subjects, ["Asset", kept.id]
749 end
750
751 test "publish without an asset change writes no assets key" do
752 node = Node.root.children.create!(:slug => "publish_asset_static")
753 node.draft.related_assets.create!(:asset => Asset.create!(:name => "Steady"))
754 node.publish_draft!(@user1)
755
756 node.lock_for_editing!(@user1)
757 node.create_new_draft(@user1)
758 node.publish_draft!(@user1)
759
760 action = node.node_actions.where(:action => "publish").order(:id).last
761 assert_nil action.metadata["assets"]
762 assert_equal [["Node", node.id]],
763 action.action_participants.map { |p| [p.subject_type, p.subject_id] }
764 end
765
766 test "pure reordering is recorded as assets_reordered" do
767 node = Node.root.children.create!(:slug => "publish_asset_reorder")
768 a1, a2 = Asset.create!(:name => "First"), Asset.create!(:name => "Second")
769 node.draft.related_assets.create!(:asset => a1)
770 node.draft.related_assets.create!(:asset => a2)
771 node.publish_draft!(@user1)
772
773 node.lock_for_editing!(@user1)
774 node.create_new_draft(@user1)
775 node.draft.related_assets.reload.first.move_to_bottom
776 node.publish_draft!(@user1)
777
778 action = node.node_actions.where(:action => "publish").order(:id).last
779 assert action.metadata["assets_reordered"]
780 assert_nil action.metadata["assets"]
781 end
782
731 test "restore_revision! logs a publish via revision" do 783 test "restore_revision! logs a publish via revision" do
732 node = create_node_with_published_page 784 node = create_node_with_published_page
733 Globalize.with_locale(:de) { node.head.update!(:title => "First") } 785 Globalize.with_locale(:de) { node.head.update!(:title => "First") }