diff options
| author | erdgeist <erdgeist@erdgeist.org> | 2026-07-16 02:40:03 +0200 |
|---|---|---|
| committer | erdgeist <erdgeist@erdgeist.org> | 2026-07-16 02:40:03 +0200 |
| commit | c0c9250141c2fa64fed967b8b9ad9bada3694c3d (patch) | |
| tree | 8a36020521dafc0d259d5d78461c59a4635b6ba4 | |
| parent | fb7b8233f1bbf7acb88d48f96e6f81cefe168d1c (diff) | |
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.
| -rw-r--r-- | app/controllers/nodes_controller.rb | 3 | ||||
| -rw-r--r-- | app/controllers/page_translations_controller.rb | 3 | ||||
| -rw-r--r-- | app/controllers/revisions_controller.rb | 2 | ||||
| -rw-r--r-- | app/models/node.rb | 29 | ||||
| -rw-r--r-- | app/models/node_action.rb | 132 | ||||
| -rw-r--r-- | test/controllers/nodes_controller_test.rb | 15 | ||||
| -rw-r--r-- | test/controllers/page_translations_controller_test.rb | 11 | ||||
| -rw-r--r-- | test/models/node_action_test.rb | 112 | ||||
| -rw-r--r-- | test/models/node_test.rb | 63 |
9 files changed, 348 insertions, 22 deletions
diff --git a/app/controllers/nodes_controller.rb b/app/controllers/nodes_controller.rb index ed5f8ef..b66ea49 100644 --- a/app/controllers/nodes_controller.rb +++ b/app/controllers/nodes_controller.rb | |||
| @@ -50,6 +50,9 @@ class NodesController < ApplicationController | |||
| 50 | @node.draft.save! | 50 | @node.draft.save! |
| 51 | 51 | ||
| 52 | @node.update!(default_template_name: config[:template]) if config && config[:template] | 52 | @node.update!(default_template_name: config[:template]) if config && config[:template] |
| 53 | NodeAction.record!(:node => @node, :page => @node.draft, :user => current_user, | ||
| 54 | :action => "create", | ||
| 55 | :title => params[:title], :path => @node.unique_name) | ||
| 53 | 56 | ||
| 54 | redirect_to(edit_node_path(@node)) | 57 | redirect_to(edit_node_path(@node)) |
| 55 | else | 58 | else |
diff --git a/app/controllers/page_translations_controller.rb b/app/controllers/page_translations_controller.rb index be4f488..38a7c4f 100644 --- a/app/controllers/page_translations_controller.rb +++ b/app/controllers/page_translations_controller.rb | |||
| @@ -63,9 +63,6 @@ class PageTranslationsController < ApplicationController | |||
| 63 | draft.translations.where(:locale => @locale).delete_all | 63 | draft.translations.where(:locale => @locale).delete_all |
| 64 | draft.reload | 64 | draft.reload |
| 65 | 65 | ||
| 66 | NodeAction.record!(:node => @node, :page => draft, :user => current_user, | ||
| 67 | :action => "translation_destroy", :locale => @locale) | ||
| 68 | |||
| 69 | flash[:notice] = "#{@locale.upcase} translation removed from the draft. Publish to make this permanent." | 66 | flash[:notice] = "#{@locale.upcase} translation removed from the draft. Publish to make this permanent." |
| 70 | redirect_to node_path(@node) | 67 | redirect_to node_path(@node) |
| 71 | rescue LockedByAnotherUser => e | 68 | rescue LockedByAnotherUser => e |
diff --git a/app/controllers/revisions_controller.rb b/app/controllers/revisions_controller.rb index c5932a4..c1237a9 100644 --- a/app/controllers/revisions_controller.rb +++ b/app/controllers/revisions_controller.rb | |||
| @@ -49,7 +49,7 @@ class RevisionsController < ApplicationController | |||
| 49 | 49 | ||
| 50 | def restore | 50 | def restore |
| 51 | page = Page.find(params[:id]) | 51 | page = Page.find(params[:id]) |
| 52 | page.node.restore_revision! page.revision | 52 | page.node.restore_revision! page.revision, current_user |
| 53 | flash[:notice] = "Revision #{page.revision} restored" | 53 | flash[:notice] = "Revision #{page.revision} restored" |
| 54 | redirect_to node_path(page.node) | 54 | redirect_to node_path(page.node) |
| 55 | end | 55 | end |
diff --git a/app/models/node.rb b/app/models/node.rb index b41df06..6ca607d 100644 --- a/app/models/node.rb +++ b/app/models/node.rb | |||
| @@ -194,17 +194,19 @@ class Node < ApplicationRecord | |||
| 194 | # Return nil if nothing to publish and no staged changes | 194 | # Return nil if nothing to publish and no staged changes |
| 195 | return nil unless self.draft || staged_slug || staged_parent_id | 195 | return nil unless self.draft || staged_slug || staged_parent_id |
| 196 | 196 | ||
| 197 | path_before = self.unique_name | ||
| 198 | |||
| 197 | ActiveRecord::Base.transaction do | 199 | ActiveRecord::Base.transaction do |
| 198 | if self.draft | 200 | if self.draft |
| 199 | previous_title = self.head ? Globalize.with_locale(I18n.default_locale) { self.head.title } : nil | 201 | outgoing_head = self.head |
| 200 | self.head = self.draft | 202 | self.head = self.draft |
| 201 | self.head.published_at ||= Time.now | 203 | self.head.published_at ||= Time.now |
| 202 | self.head.save! | 204 | self.head.save! |
| 203 | self.draft = nil | 205 | self.draft = nil |
| 204 | 206 | ||
| 205 | new_title = Globalize.with_locale(I18n.default_locale) { self.head.title } | ||
| 206 | NodeAction.record!(:node => self, :page => self.head, :user => current_user, | 207 | NodeAction.record!(:node => self, :page => self.head, :user => current_user, |
| 207 | :action => "publish", :from => previous_title, :to => new_title) | 208 | :action => "publish", :via => "draft", |
| 209 | **NodeAction.head_diff(outgoing_head, self.head)) | ||
| 208 | end | 210 | end |
| 209 | 211 | ||
| 210 | if staged_slug && (staged_slug != slug) | 212 | if staged_slug && (staged_slug != slug) |
| @@ -231,17 +233,28 @@ class Node < ApplicationRecord | |||
| 231 | self.reload | 233 | self.reload |
| 232 | self.update_unique_name | 234 | self.update_unique_name |
| 233 | self.send(:update_unique_names_of_children) | 235 | self.send(:update_unique_names_of_children) |
| 236 | if self.unique_name != path_before | ||
| 237 | NodeAction.record!(:node => self, :user => current_user, :action => "move", | ||
| 238 | :path => { "from" => path_before, "to" => self.unique_name }) | ||
| 239 | end | ||
| 234 | self.unlock! | 240 | self.unlock! |
| 235 | self | 241 | self |
| 236 | end | 242 | end |
| 237 | end | 243 | end |
| 238 | 244 | ||
| 239 | def restore_revision! revision | 245 | def restore_revision! revision, current_user = nil |
| 240 | if page = self.pages.find_by_revision(revision) | 246 | page = self.pages.find_by_revision(revision) |
| 247 | return nil unless page | ||
| 248 | |||
| 249 | ActiveRecord::Base.transaction do | ||
| 250 | outgoing_head = self.head | ||
| 241 | self.head = page | 251 | self.head = page |
| 242 | self.save | 252 | self.save! |
| 243 | else | 253 | |
| 244 | nil | 254 | NodeAction.record!(:node => self, :page => page, :user => current_user, |
| 255 | :action => "publish", :via => "revision", | ||
| 256 | **NodeAction.head_diff(outgoing_head, page)) | ||
| 257 | self | ||
| 245 | end | 258 | end |
| 246 | end | 259 | end |
| 247 | 260 | ||
diff --git a/app/models/node_action.rb b/app/models/node_action.rb index f32b5b7..39935c7 100644 --- a/app/models/node_action.rb +++ b/app/models/node_action.rb | |||
| @@ -6,6 +6,82 @@ class NodeAction < ApplicationRecord | |||
| 6 | validates :action, presence: true | 6 | validates :action, presence: true |
| 7 | validates :occurred_at, presence: true | 7 | validates :occurred_at, presence: true |
| 8 | 8 | ||
| 9 | # == Metadata contract == | ||
| 10 | # | ||
| 11 | # metadata is written once at creation and never updated. It is the | ||
| 12 | # single place anything that must survive deletion of the referenced | ||
| 13 | # rows lives; node_id/page_id/user_id are lookup and ordering only. | ||
| 14 | # All keys are strings. Pairs are always {"from" => x, "to" => y} -- | ||
| 15 | # never _old/_new suffixes. Optional pairs are written only when the | ||
| 16 | # two sides differ; required keys are always present. All titles and | ||
| 17 | # names are read pinned to I18n.default_locale unless inside a | ||
| 18 | # locale-keyed block. | ||
| 19 | # | ||
| 20 | # Baseline, every entry (written here, not by call sites): | ||
| 21 | # "username" -- actor's login at write time | ||
| 22 | # "human_readable_node_name" -- node title, default locale | ||
| 23 | # | ||
| 24 | # "create": | ||
| 25 | # "title" -- initial title, default locale | ||
| 26 | # "path" -- unique path at creation time. Historical value only: | ||
| 27 | # later renames/moves do NOT update old entries. Never | ||
| 28 | # use as a join key; node_id is the join key while the | ||
| 29 | # node lives. | ||
| 30 | # | ||
| 31 | # "publish" (any promotion of a page to head; the diff against the | ||
| 32 | # outgoing head is computed BEFORE head is re-pointed, over the | ||
| 33 | # union of both pages' locales, by the shared diff function also | ||
| 34 | # used by the backfill): | ||
| 35 | # "via" -- "draft" (ordinary publish) | "revision" (rollback | ||
| 36 | # via restore_revision!). Always written; absent | ||
| 37 | # means a pre-contract entry. | ||
| 38 | # "title" -- pair, always present. "from" is null on a first | ||
| 39 | # publish (no outgoing head). | ||
| 40 | # "author" -- pair, only when the byline (page.user) changed | ||
| 41 | # "tags" -- pair of arrays, only when changed | ||
| 42 | # "assets_changed", "template_changed" | ||
| 43 | # -- booleans, only when true; page_id links to the | ||
| 44 | # revision for the real diff (never stored here) | ||
| 45 | # "abstract_changed", "body_changed" | ||
| 46 | # -- booleans for the default locale, only when true | ||
| 47 | # "translation_diff" -- only when any non-default locale differs: | ||
| 48 | # { "<locale>" => { | ||
| 49 | # "status" -- "added" | "removed" | "changed" | ||
| 50 | # "title" -- pair; "from" null when added, | ||
| 51 | # "to" null when removed | ||
| 52 | # "abstract_changed", "body_changed" -- booleans, only when | ||
| 53 | # true, only for status "changed" | ||
| 54 | # } } | ||
| 55 | # | ||
| 56 | # "move" (reparenting and/or unique-path change; one entry at the | ||
| 57 | # subtree root, descendants get none -- a descendant's own zoomed | ||
| 58 | # view will not show path history): | ||
| 59 | # "path" -- pair | ||
| 60 | # | ||
| 61 | # Reserved for the Trash feature, final shape decided there: | ||
| 62 | # "demote" (via "trash" | "depublish"; carries the leaving-public- | ||
| 63 | # view snapshot: head presence, final published_at), "trash", | ||
| 64 | # "restore_from_trash", "destroy" (final path only; publish-state | ||
| 65 | # snapshot lives on the entry that removed it from public view). | ||
| 66 | # Whether trash logs one entry or a trash+demote pair is decided | ||
| 67 | # with that feature. | ||
| 68 | # | ||
| 69 | # Backfilled entries mirror this vocabulary exactly. Their diff | ||
| 70 | # content is computed, not guessed (consecutive revisions still | ||
| 71 | # exist); only actor (from page.editor) and occurred_at are | ||
| 72 | # inferred, and inferred_from names the heuristic per entry | ||
| 73 | # ("from_page_revision", "from_published_at_heuristic"). | ||
| 74 | # inferred_from null = witnessed live. | ||
| 75 | # | ||
| 76 | # The "locale" column is currently written by no verb (it belonged | ||
| 77 | # to the retired translation_destroy) and is retained for backfill | ||
| 78 | # or future draft-scoped verbs. | ||
| 79 | # | ||
| 80 | # This log records; it does not undo. Reversibility stays in | ||
| 81 | # restore_revision! and the revisions system. No IP, session, or | ||
| 82 | # user agent, ever. Success only -- rejected attempts are not | ||
| 83 | # logged. | ||
| 84 | |||
| 9 | def self.record!(node:, action:, user: nil, page: nil, locale: nil, **extra) | 85 | def self.record!(node:, action:, user: nil, page: nil, locale: nil, **extra) |
| 10 | create!( | 86 | create!( |
| 11 | :node => node, | 87 | :node => node, |
| @@ -23,6 +99,62 @@ class NodeAction < ApplicationRecord | |||
| 23 | ) | 99 | ) |
| 24 | end | 100 | end |
| 25 | 101 | ||
| 102 | # Computes the publish-entry diff between an outgoing head and the | ||
| 103 | # page replacing it, in the exact metadata shape the contract above | ||
| 104 | # specifies. Pure function of its two arguments -- shared verbatim by | ||
| 105 | # publish_draft!, restore_revision!, and the backfill task. Reads | ||
| 106 | # translation rows directly, never locale-dependent accessors, so a | ||
| 107 | # fallback value is never mistaken for real content. Returns | ||
| 108 | # symbol-keyed top level for splatting into record!; nested keys are | ||
| 109 | # strings and jsonb serialization stringifies the rest at write time. | ||
| 110 | def self.head_diff old_page, new_page | ||
| 111 | default = I18n.default_locale | ||
| 112 | |||
| 113 | title_of = ->(page) { page&.translations&.find_by(:locale => default)&.title } | ||
| 114 | diff = { :title => { "from" => title_of.call(old_page), | ||
| 115 | "to" => title_of.call(new_page) } } | ||
| 116 | return diff unless old_page | ||
| 117 | |||
| 118 | old_author, new_author = old_page.user&.login, new_page.user&.login | ||
| 119 | diff[:author] = { "from" => old_author, "to" => new_author } if old_author != new_author | ||
| 120 | |||
| 121 | old_tags, new_tags = old_page.tag_list.sort, new_page.tag_list.sort | ||
| 122 | diff[:tags] = { "from" => old_tags, "to" => new_tags } if old_tags != new_tags | ||
| 123 | |||
| 124 | diff[:template_changed] = true if old_page.template_name != new_page.template_name | ||
| 125 | diff[:assets_changed] = true if old_page.assets.map(&:id) != new_page.assets.map(&:id) | ||
| 126 | |||
| 127 | old_t = old_page.translations.find_by(:locale => default) | ||
| 128 | new_t = new_page.translations.find_by(:locale => default) | ||
| 129 | diff[:abstract_changed] = true if old_t&.abstract != new_t&.abstract | ||
| 130 | diff[:body_changed] = true if old_t&.body != new_t&.body | ||
| 131 | |||
| 132 | locales = (old_page.translated_locales | new_page.translated_locales) - [default] | ||
| 133 | translation_diff = {} | ||
| 134 | |||
| 135 | locales.sort_by(&:to_s).each do |locale| | ||
| 136 | o = old_page.translations.find_by(:locale => locale) | ||
| 137 | n = new_page.translations.find_by(:locale => locale) | ||
| 138 | |||
| 139 | if o.nil? | ||
| 140 | translation_diff[locale.to_s] = { "status" => "added", | ||
| 141 | "title" => { "from" => nil, "to" => n.title } } | ||
| 142 | elsif n.nil? | ||
| 143 | translation_diff[locale.to_s] = { "status" => "removed", | ||
| 144 | "title" => { "from" => o.title, "to" => nil } } | ||
| 145 | elsif o.title != n.title || o.abstract != n.abstract || o.body != n.body | ||
| 146 | entry = { "status" => "changed" } | ||
| 147 | entry["title"] = { "from" => o.title, "to" => n.title } if o.title != n.title | ||
| 148 | entry["abstract_changed"] = true if o.abstract != n.abstract | ||
| 149 | entry["body_changed"] = true if o.body != n.body | ||
| 150 | translation_diff[locale.to_s] = entry | ||
| 151 | end | ||
| 152 | end | ||
| 153 | |||
| 154 | diff[:translation_diff] = translation_diff if translation_diff.any? | ||
| 155 | diff | ||
| 156 | end | ||
| 157 | |||
| 26 | def actor_name | 158 | def actor_name |
| 27 | metadata["username"] || "unknown" | 159 | metadata["username"] || "unknown" |
| 28 | end | 160 | end |
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 | |||
| 647 | assert_select ".sitemap_node_actions", :text => /Create Child/ | 647 | assert_select ".sitemap_node_actions", :text => /Create Child/ |
| 648 | assert_select ".sitemap_node_actions", :text => /Revisions/, :count => 0 | 648 | assert_select ".sitemap_node_actions", :text => /Revisions/, :count => 0 |
| 649 | end | 649 | end |
| 650 | |||
| 651 | test "create logs a create NodeAction with path and title" do | ||
| 652 | login_as :quentin | ||
| 653 | |||
| 654 | assert_difference "NodeAction.count" do | ||
| 655 | post :create, params: { :kind => "generic", :title => "Brand New", :parent_id => Node.root.id } | ||
| 656 | end | ||
| 657 | |||
| 658 | action = NodeAction.last | ||
| 659 | assert_equal "create", action.action | ||
| 660 | assert_equal users(:quentin), action.user | ||
| 661 | assert_equal Node.last, action.node | ||
| 662 | assert_equal "Brand New", action.metadata["title"] | ||
| 663 | assert_equal Node.last.unique_name, action.metadata["path"] | ||
| 664 | end | ||
| 650 | end | 665 | 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 | |||
| 83 | assert_equal "in progress", Globalize.with_locale(:en) { node.autosave.title } | 83 | assert_equal "in progress", Globalize.with_locale(:en) { node.autosave.title } |
| 84 | end | 84 | end |
| 85 | 85 | ||
| 86 | test "destroy logs a translation_destroy NodeAction" do | 86 | test "destroy removes the translation, logs nothing, and redirects" do |
| 87 | login_as :quentin | 87 | login_as :quentin |
| 88 | node = Node.root.children.create!(:slug => "translation_destroy_log_test") | 88 | node = Node.root.children.create!(:slug => "translation_destroy_log_test") |
| 89 | Globalize.with_locale(:en) { node.draft.update!(:title => "English title") } | 89 | Globalize.with_locale(:en) { node.draft.update!(:title => "English title") } |
| 90 | 90 | ||
| 91 | delete :destroy, params: { :node_id => node.id, :translation_locale => "en" } | 91 | assert_no_difference "NodeAction.count" do |
| 92 | delete :destroy, params: { :node_id => node.id, :translation_locale => "en" } | ||
| 93 | end | ||
| 92 | 94 | ||
| 93 | action = NodeAction.last | ||
| 94 | assert_equal node, action.node | ||
| 95 | assert_equal "en", action.locale | ||
| 96 | assert_equal "translation_destroy", action.action | ||
| 97 | assert_equal users(:quentin), action.user | ||
| 98 | assert_redirected_to node_path(node) | 95 | assert_redirected_to node_path(node) |
| 99 | end | 96 | end |
| 100 | end | 97 | 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 @@ | |||
| 1 | require "test_helper" | ||
| 2 | |||
| 3 | class NodeActionTest < ActiveSupport::TestCase | ||
| 4 | |||
| 5 | # Standalone pages, no node -- same pattern autosave relies on. | ||
| 6 | # Default-locale attributes are written pinned, never ambient. | ||
| 7 | def build_page en: nil, **attrs | ||
| 8 | page = Globalize.with_locale(I18n.default_locale) do | ||
| 9 | Page.create!({ :title => "Titel" }.merge(attrs)) | ||
| 10 | end | ||
| 11 | page.translations.create!({ :locale => "en" }.merge(en)) if en | ||
| 12 | page.reload | ||
| 13 | end | ||
| 14 | |||
| 15 | test "first publish yields exactly the title pair, from nil" do | ||
| 16 | diff = NodeAction.head_diff(nil, build_page(:title => "Erstausgabe")) | ||
| 17 | |||
| 18 | assert_equal({ :title => { "from" => nil, "to" => "Erstausgabe" } }, diff) | ||
| 19 | end | ||
| 20 | |||
| 21 | test "title pair is always present, even when unchanged" do | ||
| 22 | diff = NodeAction.head_diff(build_page, build_page) | ||
| 23 | |||
| 24 | assert_equal "Titel", diff[:title]["from"] | ||
| 25 | assert_equal "Titel", diff[:title]["to"] | ||
| 26 | end | ||
| 27 | |||
| 28 | test "author pair present only when the byline changed" do | ||
| 29 | u1, u2 = users(:quentin), users(:aaron) | ||
| 30 | |||
| 31 | assert_nil NodeAction.head_diff(build_page(:user => u1), build_page(:user => u1))[:author] | ||
| 32 | |||
| 33 | changed = NodeAction.head_diff(build_page(:user => u1), build_page(:user => u2)) | ||
| 34 | assert_equal({ "from" => u1.login, "to" => u2.login }, changed[:author]) | ||
| 35 | end | ||
| 36 | |||
| 37 | test "tags pair present only when the tag set changed" do | ||
| 38 | old_page = build_page; old_page.tag_list = "alpha, beta"; old_page.save! | ||
| 39 | new_page = build_page; new_page.tag_list = "beta, gamma"; new_page.save! | ||
| 40 | |||
| 41 | diff = NodeAction.head_diff(old_page.reload, new_page.reload) | ||
| 42 | assert_equal %w[alpha beta], diff[:tags]["from"] | ||
| 43 | assert_equal %w[beta gamma], diff[:tags]["to"] | ||
| 44 | |||
| 45 | same = NodeAction.head_diff(old_page, old_page) | ||
| 46 | assert_nil same[:tags] | ||
| 47 | end | ||
| 48 | |||
| 49 | test "template_changed flag only when true" do | ||
| 50 | old_page = build_page(:template_name => "standard_template") | ||
| 51 | |||
| 52 | assert NodeAction.head_diff(old_page, build_page(:template_name => "update"))[:template_changed] | ||
| 53 | assert_nil NodeAction.head_diff(old_page, build_page(:template_name => "standard_template"))[:template_changed] | ||
| 54 | end | ||
| 55 | |||
| 56 | test "assets_changed flag when the attached set differs" do | ||
| 57 | asset = Asset.create!(:name => "diff probe", | ||
| 58 | :upload_file_name => "test_image.png", | ||
| 59 | :upload_content_type => "image/png", | ||
| 60 | :upload_file_size => 49854, | ||
| 61 | :upload_updated_at => Time.current) | ||
| 62 | old_page, new_page = build_page, build_page | ||
| 63 | new_page.related_assets.create!(:asset_id => asset.id, :position => 1) | ||
| 64 | |||
| 65 | |||
| 66 | diff = NodeAction.head_diff(old_page, new_page.reload) | ||
| 67 | assert diff[:assets_changed] | ||
| 68 | assert_nil NodeAction.head_diff(old_page, old_page)[:assets_changed] | ||
| 69 | end | ||
| 70 | |||
| 71 | test "default-locale abstract and body changes become flags, only when true" do | ||
| 72 | diff = NodeAction.head_diff(build_page(:abstract => "a"), build_page(:abstract => "b")) | ||
| 73 | |||
| 74 | assert diff[:abstract_changed] | ||
| 75 | assert_nil diff[:body_changed] | ||
| 76 | end | ||
| 77 | |||
| 78 | test "added locale carries status and the new title" do | ||
| 79 | diff = NodeAction.head_diff(build_page, build_page(en: { :title => "English" })) | ||
| 80 | |||
| 81 | entry = diff[:translation_diff]["en"] | ||
| 82 | assert_equal "added", entry["status"] | ||
| 83 | assert_equal({ "from" => nil, "to" => "English" }, entry["title"]) | ||
| 84 | end | ||
| 85 | |||
| 86 | test "removed locale keeps the vanished title" do | ||
| 87 | diff = NodeAction.head_diff(build_page(en: { :title => "English" }), build_page) | ||
| 88 | |||
| 89 | entry = diff[:translation_diff]["en"] | ||
| 90 | assert_equal "removed", entry["status"] | ||
| 91 | assert_equal "English", entry["title"]["from"] | ||
| 92 | assert_nil entry["title"]["to"] | ||
| 93 | end | ||
| 94 | |||
| 95 | test "changed locale carries only what actually differs" do | ||
| 96 | old_page = build_page(en: { :title => "Same", :body => "old" }) | ||
| 97 | new_page = build_page(en: { :title => "Same", :body => "new" }) | ||
| 98 | |||
| 99 | entry = NodeAction.head_diff(old_page, new_page)[:translation_diff]["en"] | ||
| 100 | assert_equal "changed", entry["status"] | ||
| 101 | assert_nil entry["title"] | ||
| 102 | assert entry["body_changed"] | ||
| 103 | assert_nil entry["abstract_changed"] | ||
| 104 | end | ||
| 105 | |||
| 106 | test "untouched locales are absent; identical pages yield no translation_diff at all" do | ||
| 107 | old_page = build_page(en: { :title => "Same" }) | ||
| 108 | new_page = build_page(en: { :title => "Same" }) | ||
| 109 | |||
| 110 | assert_nil NodeAction.head_diff(old_page, new_page)[:translation_diff] | ||
| 111 | end | ||
| 112 | 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 | |||
| 633 | assert_equal node.head, action.page | 633 | assert_equal node.head, action.page |
| 634 | assert_equal @user1, action.user | 634 | assert_equal @user1, action.user |
| 635 | assert_equal "publish", action.action | 635 | assert_equal "publish", action.action |
| 636 | assert_equal "draft", action.metadata["via"] | ||
| 636 | end | 637 | end |
| 637 | 638 | ||
| 638 | test "publish_draft! called with no user logs no actor, not a guessed one" do | 639 | test "publish_draft! called with no user logs no actor, not a guessed one" do |
| @@ -692,7 +693,7 @@ class NodeTest < ActiveSupport::TestCase | |||
| 692 | assert_equal count_before, NodeAction.count | 693 | assert_equal count_before, NodeAction.count |
| 693 | end | 694 | end |
| 694 | 695 | ||
| 695 | test "publish_draft! records the title's from/to in metadata" do | 696 | test "publish_draft! records the title diff in metadata" do |
| 696 | node = create_node_with_published_page | 697 | node = create_node_with_published_page |
| 697 | Globalize.with_locale(:de) { node.head.update!(:title => "Original Title") } | 698 | Globalize.with_locale(:de) { node.head.update!(:title => "Original Title") } |
| 698 | find_or_create_draft(node, @user1) | 699 | find_or_create_draft(node, @user1) |
| @@ -701,7 +702,63 @@ class NodeTest < ActiveSupport::TestCase | |||
| 701 | node.publish_draft!(@user1) | 702 | node.publish_draft!(@user1) |
| 702 | 703 | ||
| 703 | action = NodeAction.last | 704 | action = NodeAction.last |
| 704 | assert_equal "Original Title", action.metadata["from"] | 705 | assert_equal "draft", action.metadata["via"] |
| 705 | assert_equal "New Title", action.metadata["to"] | 706 | assert_equal "Original Title", action.metadata.dig("title", "from") |
| 707 | assert_equal "New Title", action.metadata.dig("title", "to") | ||
| 708 | end | ||
| 709 | |||
| 710 | test "publishing a staged slug change logs a move with the path pair" do | ||
| 711 | node = create_node_with_published_page | ||
| 712 | path_before = node.unique_name | ||
| 713 | node.staged_slug = "moved-#{node.slug}" | ||
| 714 | node.save! | ||
| 715 | publish_count_before = NodeAction.where(:action => "publish").count | ||
| 716 | |||
| 717 | node.publish_draft!(@user1) | ||
| 718 | |||
| 719 | node.reload | ||
| 720 | assert_not_equal path_before, node.unique_name | ||
| 721 | |||
| 722 | action = NodeAction.where(:action => "move").last | ||
| 723 | assert_equal node, action.node | ||
| 724 | assert_equal @user1, action.user | ||
| 725 | assert_equal path_before, action.metadata.dig("path", "from") | ||
| 726 | assert_equal node.unique_name, action.metadata.dig("path", "to") | ||
| 727 | |||
| 728 | # No draft was pending: path change alone must not fabricate a publish. | ||
| 729 | assert_equal publish_count_before, NodeAction.where(:action => "publish").count | ||
| 730 | end | ||
| 731 | |||
| 732 | test "publishing a draft together with a staged move logs two entries" do | ||
| 733 | node = create_node_with_published_page | ||
| 734 | find_or_create_draft(node, @user1) | ||
| 735 | node.staged_slug = "relocated-#{node.slug}" | ||
| 736 | node.save! | ||
| 737 | |||
| 738 | assert_difference "NodeAction.count", 2 do | ||
| 739 | node.publish_draft!(@user1) | ||
| 740 | end | ||
| 741 | |||
| 742 | assert_equal %w[move publish], | ||
| 743 | NodeAction.order(:id).last(2).map(&:action).sort | ||
| 744 | end | ||
| 745 | |||
| 746 | test "restore_revision! logs a publish via revision" do | ||
| 747 | node = create_node_with_published_page | ||
| 748 | Globalize.with_locale(:de) { node.head.update!(:title => "First") } | ||
| 749 | first_head = node.head | ||
| 750 | find_or_create_draft(node, @user1) | ||
| 751 | Globalize.with_locale(:de) { node.draft.update!(:title => "Second") } | ||
| 752 | node.publish_draft!(@user1) | ||
| 753 | |||
| 754 | node.restore_revision!(first_head.revision, @user1) | ||
| 755 | |||
| 756 | action = NodeAction.last | ||
| 757 | assert_equal "publish", action.action | ||
| 758 | assert_equal "revision", action.metadata["via"] | ||
| 759 | assert_equal first_head, action.page | ||
| 760 | assert_equal @user1, action.user | ||
| 761 | assert_equal "Second", action.metadata.dig("title", "from") | ||
| 762 | assert_equal "First", action.metadata.dig("title", "to") | ||
| 706 | end | 763 | end |
| 707 | end | 764 | end |
