From c0c9250141c2fa64fed967b8b9ad9bada3694c3d Mon Sep 17 00:00:00 2001 From: erdgeist Date: Thu, 16 Jul 2026 02:40:03 +0200 Subject: Record the full lifecycle contract in NodeAction entries A contract comment above NodeAction.record! now specifies every verb's metadata shape. NodeAction.head_diff computes the publish diff between an outgoing head and its replacement -- default-locale title pair always, author/tags pairs and template/assets/abstract/ body flags only when changed, and a per-locale translation_diff with added/removed/changed status. It is a pure function of its two pages, shared by publish, rollback, and the future backfill, and reads translation rows directly so fallbacks never masquerade as content. publish entries carry via ("draft" or "revision"); restore_revision! is now transactional, takes the acting user, and logs through the same diff. Staged slug/parent changes applied at publish log a move entry with the path pair. Node creation logs a create entry with initial title and path. The draft-scoped translation_destroy writer is retired -- locale removal is recorded by the publish diff, where it becomes public fact. --- test/controllers/nodes_controller_test.rb | 15 +++ .../page_translations_controller_test.rb | 11 +- test/models/node_action_test.rb | 112 +++++++++++++++++++++ test/models/node_test.rb | 63 +++++++++++- 4 files changed, 191 insertions(+), 10 deletions(-) create mode 100644 test/models/node_action_test.rb (limited to 'test') diff --git a/test/controllers/nodes_controller_test.rb b/test/controllers/nodes_controller_test.rb index 9da9514..5b66bd3 100644 --- a/test/controllers/nodes_controller_test.rb +++ b/test/controllers/nodes_controller_test.rb @@ -647,4 +647,19 @@ class NodesControllerTest < ActionController::TestCase assert_select ".sitemap_node_actions", :text => /Create Child/ assert_select ".sitemap_node_actions", :text => /Revisions/, :count => 0 end + + test "create logs a create NodeAction with path and title" do + login_as :quentin + + assert_difference "NodeAction.count" do + post :create, params: { :kind => "generic", :title => "Brand New", :parent_id => Node.root.id } + end + + action = NodeAction.last + assert_equal "create", action.action + assert_equal users(:quentin), action.user + assert_equal Node.last, action.node + assert_equal "Brand New", action.metadata["title"] + assert_equal Node.last.unique_name, action.metadata["path"] + end end diff --git a/test/controllers/page_translations_controller_test.rb b/test/controllers/page_translations_controller_test.rb index 82f8bce..feaacd0 100644 --- a/test/controllers/page_translations_controller_test.rb +++ b/test/controllers/page_translations_controller_test.rb @@ -83,18 +83,15 @@ class PageTranslationsControllerTest < ActionController::TestCase assert_equal "in progress", Globalize.with_locale(:en) { node.autosave.title } end - test "destroy logs a translation_destroy NodeAction" do + test "destroy removes the translation, logs nothing, and redirects" do login_as :quentin node = Node.root.children.create!(:slug => "translation_destroy_log_test") Globalize.with_locale(:en) { node.draft.update!(:title => "English title") } - delete :destroy, params: { :node_id => node.id, :translation_locale => "en" } + assert_no_difference "NodeAction.count" do + delete :destroy, params: { :node_id => node.id, :translation_locale => "en" } + end - action = NodeAction.last - assert_equal node, action.node - assert_equal "en", action.locale - assert_equal "translation_destroy", action.action - assert_equal users(:quentin), action.user assert_redirected_to node_path(node) end end diff --git a/test/models/node_action_test.rb b/test/models/node_action_test.rb new file mode 100644 index 0000000..24d8245 --- /dev/null +++ b/test/models/node_action_test.rb @@ -0,0 +1,112 @@ +require "test_helper" + +class NodeActionTest < ActiveSupport::TestCase + + # Standalone pages, no node -- same pattern autosave relies on. + # Default-locale attributes are written pinned, never ambient. + def build_page en: nil, **attrs + page = Globalize.with_locale(I18n.default_locale) do + Page.create!({ :title => "Titel" }.merge(attrs)) + end + page.translations.create!({ :locale => "en" }.merge(en)) if en + page.reload + end + + test "first publish yields exactly the title pair, from nil" do + diff = NodeAction.head_diff(nil, build_page(:title => "Erstausgabe")) + + assert_equal({ :title => { "from" => nil, "to" => "Erstausgabe" } }, diff) + end + + test "title pair is always present, even when unchanged" do + diff = NodeAction.head_diff(build_page, build_page) + + assert_equal "Titel", diff[:title]["from"] + assert_equal "Titel", diff[:title]["to"] + end + + test "author pair present only when the byline changed" do + u1, u2 = users(:quentin), users(:aaron) + + assert_nil NodeAction.head_diff(build_page(:user => u1), build_page(:user => u1))[:author] + + changed = NodeAction.head_diff(build_page(:user => u1), build_page(:user => u2)) + assert_equal({ "from" => u1.login, "to" => u2.login }, changed[:author]) + end + + test "tags pair present only when the tag set changed" do + old_page = build_page; old_page.tag_list = "alpha, beta"; old_page.save! + new_page = build_page; new_page.tag_list = "beta, gamma"; new_page.save! + + diff = NodeAction.head_diff(old_page.reload, new_page.reload) + assert_equal %w[alpha beta], diff[:tags]["from"] + assert_equal %w[beta gamma], diff[:tags]["to"] + + same = NodeAction.head_diff(old_page, old_page) + assert_nil same[:tags] + end + + test "template_changed flag only when true" do + old_page = build_page(:template_name => "standard_template") + + assert NodeAction.head_diff(old_page, build_page(:template_name => "update"))[:template_changed] + 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 + asset = Asset.create!(:name => "diff probe", + :upload_file_name => "test_image.png", + :upload_content_type => "image/png", + :upload_file_size => 49854, + :upload_updated_at => Time.current) + 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] + end + + test "default-locale abstract and body changes become flags, only when true" do + diff = NodeAction.head_diff(build_page(:abstract => "a"), build_page(:abstract => "b")) + + assert diff[:abstract_changed] + assert_nil diff[:body_changed] + end + + test "added locale carries status and the new title" do + diff = NodeAction.head_diff(build_page, build_page(en: { :title => "English" })) + + entry = diff[:translation_diff]["en"] + assert_equal "added", entry["status"] + assert_equal({ "from" => nil, "to" => "English" }, entry["title"]) + end + + test "removed locale keeps the vanished title" do + diff = NodeAction.head_diff(build_page(en: { :title => "English" }), build_page) + + entry = diff[:translation_diff]["en"] + assert_equal "removed", entry["status"] + assert_equal "English", entry["title"]["from"] + assert_nil entry["title"]["to"] + end + + test "changed locale carries only what actually differs" do + old_page = build_page(en: { :title => "Same", :body => "old" }) + new_page = build_page(en: { :title => "Same", :body => "new" }) + + entry = NodeAction.head_diff(old_page, new_page)[:translation_diff]["en"] + assert_equal "changed", entry["status"] + assert_nil entry["title"] + assert entry["body_changed"] + assert_nil entry["abstract_changed"] + end + + test "untouched locales are absent; identical pages yield no translation_diff at all" do + old_page = build_page(en: { :title => "Same" }) + new_page = build_page(en: { :title => "Same" }) + + assert_nil NodeAction.head_diff(old_page, new_page)[:translation_diff] + end +end diff --git a/test/models/node_test.rb b/test/models/node_test.rb index 022fff6..c8e49e5 100644 --- a/test/models/node_test.rb +++ b/test/models/node_test.rb @@ -633,6 +633,7 @@ class NodeTest < ActiveSupport::TestCase assert_equal node.head, action.page assert_equal @user1, action.user assert_equal "publish", action.action + assert_equal "draft", action.metadata["via"] end test "publish_draft! called with no user logs no actor, not a guessed one" do @@ -692,7 +693,7 @@ class NodeTest < ActiveSupport::TestCase assert_equal count_before, NodeAction.count end - test "publish_draft! records the title's from/to in metadata" do + test "publish_draft! records the title diff in metadata" do node = create_node_with_published_page Globalize.with_locale(:de) { node.head.update!(:title => "Original Title") } find_or_create_draft(node, @user1) @@ -701,7 +702,63 @@ class NodeTest < ActiveSupport::TestCase node.publish_draft!(@user1) action = NodeAction.last - assert_equal "Original Title", action.metadata["from"] - assert_equal "New Title", action.metadata["to"] + assert_equal "draft", action.metadata["via"] + assert_equal "Original Title", action.metadata.dig("title", "from") + assert_equal "New Title", action.metadata.dig("title", "to") + end + + test "publishing a staged slug change logs a move with the path pair" do + node = create_node_with_published_page + path_before = node.unique_name + node.staged_slug = "moved-#{node.slug}" + node.save! + publish_count_before = NodeAction.where(:action => "publish").count + + node.publish_draft!(@user1) + + node.reload + assert_not_equal path_before, node.unique_name + + action = NodeAction.where(:action => "move").last + assert_equal node, action.node + assert_equal @user1, action.user + assert_equal path_before, action.metadata.dig("path", "from") + assert_equal node.unique_name, action.metadata.dig("path", "to") + + # No draft was pending: path change alone must not fabricate a publish. + assert_equal publish_count_before, NodeAction.where(:action => "publish").count + end + + test "publishing a draft together with a staged move logs two entries" do + node = create_node_with_published_page + find_or_create_draft(node, @user1) + node.staged_slug = "relocated-#{node.slug}" + node.save! + + assert_difference "NodeAction.count", 2 do + node.publish_draft!(@user1) + end + + assert_equal %w[move publish], + NodeAction.order(:id).last(2).map(&:action).sort + 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") } + first_head = node.head + find_or_create_draft(node, @user1) + Globalize.with_locale(:de) { node.draft.update!(:title => "Second") } + node.publish_draft!(@user1) + + node.restore_revision!(first_head.revision, @user1) + + action = NodeAction.last + assert_equal "publish", action.action + assert_equal "revision", action.metadata["via"] + assert_equal first_head, action.page + assert_equal @user1, action.user + assert_equal "Second", action.metadata.dig("title", "from") + assert_equal "First", action.metadata.dig("title", "to") end end -- cgit v1.3