diff options
| -rw-r--r-- | app/controllers/nodes_controller.rb | 2 | ||||
| -rw-r--r-- | app/controllers/page_translations_controller.rb | 3 | ||||
| -rw-r--r-- | app/models/node.rb | 65 | ||||
| -rw-r--r-- | app/models/node_action.rb | 33 | ||||
| -rw-r--r-- | db/migrate/20260714233414_create_node_actions.rb | 19 | ||||
| -rw-r--r-- | test/controllers/page_translations_controller_test.rb | 15 | ||||
| -rw-r--r-- | test/models/node_test.rb | 83 |
7 files changed, 191 insertions, 29 deletions
diff --git a/app/controllers/nodes_controller.rb b/app/controllers/nodes_controller.rb index bff1733..9c5d228 100644 --- a/app/controllers/nodes_controller.rb +++ b/app/controllers/nodes_controller.rb | |||
| @@ -147,7 +147,7 @@ class NodesController < ApplicationController | |||
| 147 | end | 147 | end |
| 148 | 148 | ||
| 149 | def publish | 149 | def publish |
| 150 | @node.publish_draft! | 150 | @node.publish_draft!(current_user) |
| 151 | flash[:notice] = "Draft has been published" | 151 | flash[:notice] = "Draft has been published" |
| 152 | redirect_to node_path(@node) | 152 | redirect_to node_path(@node) |
| 153 | end | 153 | end |
diff --git a/app/controllers/page_translations_controller.rb b/app/controllers/page_translations_controller.rb index 38a7c4f..be4f488 100644 --- a/app/controllers/page_translations_controller.rb +++ b/app/controllers/page_translations_controller.rb | |||
| @@ -63,6 +63,9 @@ 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 | |||
| 66 | flash[:notice] = "#{@locale.upcase} translation removed from the draft. Publish to make this permanent." | 69 | flash[:notice] = "#{@locale.upcase} translation removed from the draft. Publish to make this permanent." |
| 67 | redirect_to node_path(@node) | 70 | redirect_to node_path(@node) |
| 68 | rescue LockedByAnotherUser => e | 71 | rescue LockedByAnotherUser => e |
diff --git a/app/models/node.rb b/app/models/node.rb index 311c5c0..b41df06 100644 --- a/app/models/node.rb +++ b/app/models/node.rb | |||
| @@ -170,10 +170,12 @@ class Node < ApplicationRecord | |||
| 170 | self.autosave.destroy | 170 | self.autosave.destroy |
| 171 | self.autosave_id = nil | 171 | self.autosave_id = nil |
| 172 | self.save! | 172 | self.save! |
| 173 | NodeAction.record!(:node => self, :user => current_user, :action => "discard_autosave") | ||
| 173 | elsif self.draft && self.head | 174 | elsif self.draft && self.head |
| 174 | self.draft.destroy | 175 | self.draft.destroy |
| 175 | self.draft_id = nil | 176 | self.draft_id = nil |
| 176 | self.save! | 177 | self.save! |
| 178 | NodeAction.record!(:node => self, :user => current_user, :action => "destroy_draft") | ||
| 177 | end | 179 | end |
| 178 | 180 | ||
| 179 | self.unlock! unless self.draft | 181 | self.unlock! unless self.draft |
| @@ -188,43 +190,50 @@ class Node < ApplicationRecord | |||
| 188 | end | 190 | end |
| 189 | end | 191 | end |
| 190 | 192 | ||
| 191 | def publish_draft! | 193 | def publish_draft! current_user = nil |
| 192 | # Return nil if nothing to publish and no staged changes | 194 | # Return nil if nothing to publish and no staged changes |
| 193 | return nil unless self.draft || staged_slug || staged_parent_id | 195 | return nil unless self.draft || staged_slug || staged_parent_id |
| 194 | 196 | ||
| 195 | if self.draft | 197 | ActiveRecord::Base.transaction do |
| 196 | self.head = self.draft | 198 | if self.draft |
| 197 | self.head.published_at ||= Time.now | 199 | previous_title = self.head ? Globalize.with_locale(I18n.default_locale) { self.head.title } : nil |
| 198 | self.head.save! | 200 | self.head = self.draft |
| 199 | self.draft = nil | 201 | self.head.published_at ||= Time.now |
| 200 | end | 202 | self.head.save! |
| 201 | 203 | self.draft = nil | |
| 202 | if staged_slug && (staged_slug != slug) | ||
| 203 | self.slug = staged_slug | ||
| 204 | self.staged_slug = nil | ||
| 205 | end | ||
| 206 | 204 | ||
| 207 | if staged_parent_id && (staged_parent_id != parent_id) | 205 | new_title = Globalize.with_locale(I18n.default_locale) { self.head.title } |
| 208 | new_parent = Node.find(staged_parent_id) | 206 | NodeAction.record!(:node => self, :page => self.head, :user => current_user, |
| 207 | :action => "publish", :from => previous_title, :to => new_title) | ||
| 208 | end | ||
| 209 | 209 | ||
| 210 | if new_parent == self || self.descendants.include?(new_parent) | 210 | if staged_slug && (staged_slug != slug) |
| 211 | raise ActiveRecord::RecordInvalid.new(self), "Cannot move a node under itself or one of its own descendants" | 211 | self.slug = staged_slug |
| 212 | self.staged_slug = nil | ||
| 212 | end | 213 | end |
| 213 | 214 | ||
| 214 | self.staged_parent_id = nil | 215 | if staged_parent_id && (staged_parent_id != parent_id) |
| 215 | self.save! | 216 | new_parent = Node.find(staged_parent_id) |
| 216 | self.move_to_child_of(new_parent) | 217 | |
| 217 | else | 218 | if new_parent == self || self.descendants.include?(new_parent) |
| 218 | unless self.save | 219 | raise ActiveRecord::RecordInvalid.new(self), "Cannot move a node under itself or one of its own descendants" |
| 219 | raise ActiveRecord::RecordInvalid.new(self) | 220 | end |
| 221 | |||
| 222 | self.staged_parent_id = nil | ||
| 223 | self.save! | ||
| 224 | self.move_to_child_of(new_parent) | ||
| 225 | else | ||
| 226 | unless self.save | ||
| 227 | raise ActiveRecord::RecordInvalid.new(self) | ||
| 228 | end | ||
| 220 | end | 229 | end |
| 221 | end | ||
| 222 | 230 | ||
| 223 | self.reload | 231 | self.reload |
| 224 | self.update_unique_name | 232 | self.update_unique_name |
| 225 | self.send(:update_unique_names_of_children) | 233 | self.send(:update_unique_names_of_children) |
| 226 | self.unlock! | 234 | self.unlock! |
| 227 | self | 235 | self |
| 236 | end | ||
| 228 | end | 237 | end |
| 229 | 238 | ||
| 230 | def restore_revision! revision | 239 | def restore_revision! revision |
diff --git a/app/models/node_action.rb b/app/models/node_action.rb new file mode 100644 index 0000000..f32b5b7 --- /dev/null +++ b/app/models/node_action.rb | |||
| @@ -0,0 +1,33 @@ | |||
| 1 | class NodeAction < ApplicationRecord | ||
| 2 | belongs_to :node, optional: true | ||
| 3 | belongs_to :page, optional: true | ||
| 4 | belongs_to :user, optional: true | ||
| 5 | |||
| 6 | validates :action, presence: true | ||
| 7 | validates :occurred_at, presence: true | ||
| 8 | |||
| 9 | def self.record!(node:, action:, user: nil, page: nil, locale: nil, **extra) | ||
| 10 | create!( | ||
| 11 | :node => node, | ||
| 12 | :page => page, | ||
| 13 | :user => user, | ||
| 14 | :action => action, | ||
| 15 | :locale => locale, | ||
| 16 | :occurred_at => Time.now, | ||
| 17 | :metadata => { | ||
| 18 | "username" => user&.login, | ||
| 19 | "human_readable_node_name" => Globalize.with_locale(I18n.default_locale) { | ||
| 20 | node&.head&.title || node&.draft&.title | ||
| 21 | }, | ||
| 22 | }.merge(extra.stringify_keys) | ||
| 23 | ) | ||
| 24 | end | ||
| 25 | |||
| 26 | def actor_name | ||
| 27 | metadata["username"] || "unknown" | ||
| 28 | end | ||
| 29 | |||
| 30 | def subject_name | ||
| 31 | metadata["human_readable_node_name"] || node&.unique_name || "deleted node" | ||
| 32 | end | ||
| 33 | end | ||
diff --git a/db/migrate/20260714233414_create_node_actions.rb b/db/migrate/20260714233414_create_node_actions.rb new file mode 100644 index 0000000..6c18285 --- /dev/null +++ b/db/migrate/20260714233414_create_node_actions.rb | |||
| @@ -0,0 +1,19 @@ | |||
| 1 | class CreateNodeActions < ActiveRecord::Migration[8.1] | ||
| 2 | def change | ||
| 3 | create_table :node_actions do |t| | ||
| 4 | t.references :node, foreign_key: { on_delete: :nullify } | ||
| 5 | t.references :page, foreign_key: { on_delete: :nullify } | ||
| 6 | t.references :user, foreign_key: { on_delete: :nullify } | ||
| 7 | t.string :action, null: false | ||
| 8 | t.string :locale | ||
| 9 | t.string :inferred_from | ||
| 10 | t.jsonb :metadata, null: false | ||
| 11 | t.datetime :occurred_at, null: false | ||
| 12 | |||
| 13 | t.timestamps | ||
| 14 | end | ||
| 15 | |||
| 16 | add_index :node_actions, [:node_id, :occurred_at] | ||
| 17 | add_index :node_actions, :action | ||
| 18 | end | ||
| 19 | end | ||
diff --git a/test/controllers/page_translations_controller_test.rb b/test/controllers/page_translations_controller_test.rb index 8fa732f..82f8bce 100644 --- a/test/controllers/page_translations_controller_test.rb +++ b/test/controllers/page_translations_controller_test.rb | |||
| @@ -82,4 +82,19 @@ class PageTranslationsControllerTest < ActionController::TestCase | |||
| 82 | assert_equal page_count_before, node.pages.count | 82 | assert_equal page_count_before, node.pages.count |
| 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 | |||
| 86 | test "destroy logs a translation_destroy NodeAction" do | ||
| 87 | login_as :quentin | ||
| 88 | node = Node.root.children.create!(:slug => "translation_destroy_log_test") | ||
| 89 | Globalize.with_locale(:en) { node.draft.update!(:title => "English title") } | ||
| 90 | |||
| 91 | delete :destroy, params: { :node_id => node.id, :translation_locale => "en" } | ||
| 92 | |||
| 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) | ||
| 99 | end | ||
| 85 | end | 100 | end |
diff --git a/test/models/node_test.rb b/test/models/node_test.rb index 959387d..022fff6 100644 --- a/test/models/node_test.rb +++ b/test/models/node_test.rb | |||
| @@ -621,4 +621,87 @@ class NodeTest < ActiveSupport::TestCase | |||
| 621 | assert_equal "Edited directly on autosave", Globalize.with_locale(:en) { autosave.title } | 621 | assert_equal "Edited directly on autosave", Globalize.with_locale(:en) { autosave.title } |
| 622 | assert_equal "v2", Globalize.with_locale(:de) { autosave.title } | 622 | assert_equal "v2", Globalize.with_locale(:de) { autosave.title } |
| 623 | end | 623 | end |
| 624 | |||
| 625 | test "publish_draft! logs a NodeAction crediting the actual publisher" do | ||
| 626 | node = Node.root.children.create!(:slug => "publish_log_test") | ||
| 627 | node.draft.update!(:title => "Version one") | ||
| 628 | |||
| 629 | node.publish_draft!(@user1) | ||
| 630 | |||
| 631 | action = NodeAction.last | ||
| 632 | assert_equal node, action.node | ||
| 633 | assert_equal node.head, action.page | ||
| 634 | assert_equal @user1, action.user | ||
| 635 | assert_equal "publish", action.action | ||
| 636 | end | ||
| 637 | |||
| 638 | test "publish_draft! called with no user logs no actor, not a guessed one" do | ||
| 639 | node = Node.root.children.create!(:slug => "publish_log_no_user_test") | ||
| 640 | node.draft.update!(:title => "Version one") | ||
| 641 | |||
| 642 | node.publish_draft! | ||
| 643 | |||
| 644 | action = NodeAction.last | ||
| 645 | assert_nil action.user | ||
| 646 | assert_nil action.metadata["username"] | ||
| 647 | end | ||
| 648 | |||
| 649 | test "publish_draft! with nothing pending creates no NodeAction" do | ||
| 650 | node = Node.root.children.create!(:slug => "publish_log_noop_test") | ||
| 651 | node.publish_draft! | ||
| 652 | count_before = NodeAction.count | ||
| 653 | |||
| 654 | result = node.publish_draft! | ||
| 655 | |||
| 656 | assert_nil result | ||
| 657 | assert_equal count_before, NodeAction.count | ||
| 658 | end | ||
| 659 | |||
| 660 | test "revert! logs discard_autosave for an in-progress autosave" do | ||
| 661 | node = create_node_with_published_page | ||
| 662 | node.lock_for_editing!(@user1) | ||
| 663 | node.autosave!({:title => "in progress"}, @user1) | ||
| 664 | |||
| 665 | node.revert!(@user1) | ||
| 666 | |||
| 667 | action = NodeAction.last | ||
| 668 | assert_equal node, action.node | ||
| 669 | assert_equal @user1, action.user | ||
| 670 | assert_equal "discard_autosave", action.action | ||
| 671 | end | ||
| 672 | |||
| 673 | test "revert! logs destroy_draft for a draft with a head behind it" do | ||
| 674 | node = create_node_with_published_page | ||
| 675 | find_or_create_draft(node, @user1) | ||
| 676 | |||
| 677 | node.revert!(@user1) | ||
| 678 | |||
| 679 | action = NodeAction.last | ||
| 680 | assert_equal node, action.node | ||
| 681 | assert_equal @user1, action.user | ||
| 682 | assert_equal "destroy_draft", action.action | ||
| 683 | end | ||
| 684 | |||
| 685 | test "revert! with nothing to revert logs nothing" do | ||
| 686 | node = create_node_with_published_page | ||
| 687 | node.lock_for_editing!(@user1) | ||
| 688 | count_before = NodeAction.count | ||
| 689 | |||
| 690 | node.revert!(@user1) | ||
| 691 | |||
| 692 | assert_equal count_before, NodeAction.count | ||
| 693 | end | ||
| 694 | |||
| 695 | test "publish_draft! records the title's from/to in metadata" do | ||
| 696 | node = create_node_with_published_page | ||
| 697 | Globalize.with_locale(:de) { node.head.update!(:title => "Original Title") } | ||
| 698 | find_or_create_draft(node, @user1) | ||
| 699 | Globalize.with_locale(:de) { node.draft.update!(:title => "New Title") } | ||
| 700 | |||
| 701 | node.publish_draft!(@user1) | ||
| 702 | |||
| 703 | action = NodeAction.last | ||
| 704 | assert_equal "Original Title", action.metadata["from"] | ||
| 705 | assert_equal "New Title", action.metadata["to"] | ||
| 706 | end | ||
| 624 | end | 707 | end |
