From f4ddfff03ca9f25d50f39a1971877362d85eb9cb Mon Sep 17 00:00:00 2001 From: erdgeist Date: Fri, 7 Aug 2026 06:15:14 +0200 Subject: Move the pending address from the node onto the draft --- app/controllers/nodes_controller.rb | 19 ++--- app/models/node.rb | 51 ++++++------ app/models/page.rb | 28 +++++++ app/views/nodes/edit.html.erb | 12 +-- app/views/nodes/show.html.erb | 2 +- config/locales/de.yml | 8 +- config/locales/en.yml | 8 +- db/migrate/20260807010722_add_address_to_pages.rb | 6 ++ lib/tasks/pages.rake | 33 ++++++++ public/javascripts/admin_search.js | 2 +- public/stylesheets/admin.css | 4 +- test/controllers/nodes_controller_test.rb | 51 +++++++----- test/models/node_test.rb | 96 ++++++++++++++++++----- test/models/node_trash_test.rb | 10 ++- 14 files changed, 233 insertions(+), 97 deletions(-) create mode 100644 db/migrate/20260807010722_add_address_to_pages.rb create mode 100644 lib/tasks/pages.rake diff --git a/app/controllers/nodes_controller.rb b/app/controllers/nodes_controller.rb index 22606674..3009e25a 100644 --- a/app/controllers/nodes_controller.rb +++ b/app/controllers/nodes_controller.rb @@ -90,7 +90,6 @@ class NodesController < ApplicationController end def update - @node.update(node_update_params) @node.autosave!( page_params.merge(:tag_list => params[:tag_list]), current_user ) @node.save_draft!(current_user) @@ -123,7 +122,6 @@ class NodesController < ApplicationController end def autosave - @node.update(node_update_params) @node.autosave!( page_params.merge(:tag_list => params[:tag_list]), current_user ) head :ok rescue LockedByAnotherUser => e @@ -167,13 +165,13 @@ class NodesController < ApplicationController end def restore_from_trash - parent = Node.find(params[:parent_id]) - @node.restore_from_trash!(parent, current_user) + if params[:parent_id].present? && @node.draft + @node.draft.update!(:parent_node_id => params[:parent_id]) + end + + @node.restore_from_trash!(current_user) flash[:notice] = t("flash.nodes.restored") redirect_to node_path(@node) - rescue ActiveRecord::RecordNotFound - flash[:error] = t("flash.nodes.restore_target_missing") - redirect_to node_path(@node) rescue ActiveRecord::RecordInvalid => e flash[:error] = e.record.errors.full_messages.to_sentence redirect_to node_path(@node) @@ -281,12 +279,9 @@ class NodesController < ApplicationController params.fetch(:node, {}).permit(:slug, :parent_id) end - def node_update_params - params.fetch(:node, {}).permit(:staged_slug, :staged_parent_id) - end - def page_params - params.fetch(:page, {}).permit(:title, :abstract, :body, :template_name, :published_at, :user_id) + params.fetch(:page, {}).permit(:title, :abstract, :body, :template_name, + :published_at, :user_id, :slug, :parent_node_id) end def find_node diff --git a/app/models/node.rb b/app/models/node.rb index 4b7c9772..5c28a786 100644 --- a/app/models/node.rb +++ b/app/models/node.rb @@ -128,6 +128,7 @@ class Node < ApplicationRecord def autosave! attributes, current_user ensure_autosave!(current_user) + attributes = attributes.except(:slug, "slug") if attributes[:slug].blank? && attributes["slug"].blank? self.autosave.assign_attributes(attributes) self.autosave.save! self.autosave @@ -227,17 +228,9 @@ class Node < ApplicationRecord self.reload end - def staged_slug=(value) - if head.blank? - self.slug = value - else - super - end - end - def publish_draft! current_user = nil - # Return nil if nothing to publish and no staged changes - return nil unless self.draft || staged_slug || staged_parent_id + # Return nil if nothing to publish + return nil unless self.draft guard_live_change!(current_user, :target_path => prospective_unique_name) @@ -263,26 +256,28 @@ class Node < ApplicationRecord **NodeAction.head_diff(outgoing_head, self.head)) end - if staged_slug && (staged_slug != slug) - self.slug = staged_slug - self.staged_slug = nil + if self.head.slug.present? && self.head.slug != slug + self.slug = self.head.slug end - if staged_parent_id && (staged_parent_id != parent_id) - new_parent = Node.find(staged_parent_id) + if self.head.parent_node_id && self.head.parent_node_id != parent_id + new_parent = Node.find_by(:id => self.head.parent_node_id) - if new_parent == self || self.descendants.include?(new_parent) + unless new_parent + errors.add(:base, :move_target_missing) + raise ActiveRecord::RecordInvalid.new(self) + end + + if new_parent == self || self.descendants.include?(new_parent) || + new_parent.trash_node? || new_parent.in_trash? errors.add(:base, :move_under_self) raise ActiveRecord::RecordInvalid.new(self) end - self.staged_parent_id = nil self.save! self.move_to_child_of(new_parent) else - unless self.save - raise ActiveRecord::RecordInvalid.new(self) - end + raise ActiveRecord::RecordInvalid.new(self) unless self.save end self.reload @@ -369,9 +364,12 @@ class Node < ApplicationRecord # subtree comes back exactly as it sits in the Trash: all drafts, # nothing published. Republication is a separate, witnessed act # per node. - def restore_from_trash! new_parent, current_user = nil + def restore_from_trash! current_user = nil return nil unless in_trash? + target = (draft || head)&.parent_node_id + new_parent = target ? Node.find_by(:id => target) : nil + if new_parent.nil? || new_parent == self || descendants.include?(new_parent) || new_parent.trash_node? || new_parent.in_trash? errors.add(:base, :restore_target_invalid) @@ -593,11 +591,9 @@ class Node < ApplicationRecord root? || self.class.restricted_path?(unique_name) end + # Falls back to the live address when the draft records none. def prospective_unique_name - target_parent = staged_parent_id ? Node.find_by(:id => staged_parent_id) : parent - return nil unless target_parent - - [target_parent.unique_name.presence, staged_slug.presence || slug].compact.join("/") + (draft || head)&.prospective_unique_name || unique_name end # Returns immutable node id for all new nodes so that the atom feed entry ids @@ -702,7 +698,7 @@ class Node < ApplicationRecord # that draft and publishes it. def initialize_empty_page if self.pages.empty? - self.draft = self.pages.create! + self.draft = self.pages.create!(:slug => self.slug, :parent_node_id => self.parent_id) self.save end end @@ -744,14 +740,11 @@ class Node < ApplicationRecord def reserved_slug_stays_reserved if parent&.root? && !trash_node_already_me? errors.add(:slug, :reserved_for_trash) if slug == CccConventions::TRASH_SLUG - errors.add(:staged_slug, :reserved_for_trash) if staged_slug == CccConventions::TRASH_SLUG end if persisted? && slug_was == CccConventions::TRASH_SLUG && Node.find(id).trash_node? errors.add(:slug, :trash_immutable) if slug_changed? errors.add(:parent_id, :trash_immutable) if parent_id_changed? - errors.add(:staged_slug, :trash_must_be_empty) if staged_slug.present? - errors.add(:staged_parent_id, :trash_must_be_empty) if staged_parent_id.present? end end diff --git a/app/models/page.rb b/app/models/page.rb index 8313b1d4..4635f1b4 100644 --- a/app/models/page.rb +++ b/app/models/page.rb @@ -20,9 +20,13 @@ class Page < ApplicationRecord :inclusion => { :in => ->(_) { Page.custom_templates } }, :allow_blank => true, :if => :template_name_changed? + validates_format_of :slug, :with => /\A[A-Za-z0-9][A-Za-z0-9_-]*\z/, + :unless => -> { slug.blank? } + validate :page_slug_not_reserved # Associations belongs_to :node, optional: true + belongs_to :parent_node, :class_name => "Node", :optional => true belongs_to :user, optional: true belongs_to :editor, :class_name => "User", optional: true has_many :related_assets, :dependent => :destroy @@ -202,6 +206,8 @@ class Page < ApplicationRecord page.translations.reload # Clone untranslated attributes + self.slug = page.slug + self.parent_node_id = page.parent_node_id self.tag_list = page.tag_list self.template_name ||= page.template_name self.published_at = page.published_at @@ -292,6 +298,20 @@ class Page < ApplicationRecord published_at.nil? ? true : published_at < Time.now end + # The address this page will have once published. + def prospective_unique_name + return nil if parent_node_id.nil? + + parent = Node.find_by(:id => parent_node_id) + return nil unless parent + + [parent.unique_name.presence, slug].compact.join("/") + end + + def parent_node_missing? + parent_node_id.present? && !Node.exists?(:id => parent_node_id) + end + def effective_lang if translated_locales.empty? return 'de' @@ -333,6 +353,14 @@ class Page < ApplicationRecord end end + def page_slug_not_reserved + return unless slug == CccConventions::TRASH_SLUG + return if node&.trash_node? + return unless parent_node_id && Node.find_by(:id => parent_node_id)&.root? + + errors.add(:slug, :reserved_for_trash) + end + # Installs (or re-installs) the trigger that keeps page_translations' # search_vector in sync. Idempotent, safe to call on every boot. # search_vector is populated by a raw Postgres trigger, not anything diff --git a/app/views/nodes/edit.html.erb b/app/views/nodes/edit.html.erb index c153c241..ae206578 100644 --- a/app/views/nodes/edit.html.erb +++ b/app/views/nodes/edit.html.erb @@ -96,20 +96,20 @@
<%= t(".slug") %>
- <%= f.text_field( - :staged_slug, :value => @node.staged_slug || @node.slug + <%= d.text_field( + :slug, :value => @page.slug || @node.slug ) %>
<%= t(".parent") %>
- <%= text_field_tag :move_to_search_term, @node.parent.title rescue "" %> + <%= text_field_tag :move_to_search_term, (Node.find_by(:id => @page.parent_node_id) || @node.parent)&.title %>

<%= t(".parent_hint") %>

- <%= f.hidden_field( - :staged_parent_id, - :value => @node.staged_parent_id || @node.parent_id + <%= d.hidden_field( + :parent_node_id, + :value => @page.parent_node_id || @node.parent_id ) %>
diff --git a/app/views/nodes/show.html.erb b/app/views/nodes/show.html.erb index 105e37ea..3b2d28d9 100644 --- a/app/views/nodes/show.html.erb +++ b/app/views/nodes/show.html.erb @@ -105,7 +105,7 @@ <% end %>
<%= t(".restore_to") %> - <% suggestion = @node.suggested_restore_parent %> + <% suggestion = @node.draft&.parent_node || @node.suggested_restore_parent %> <%= form_tag restore_from_trash_node_path(@node), :method => :put, :class => "aligned_action_row" do %>
<%= text_field_tag :restore_search_term, diff --git a/config/locales/de.yml b/config/locales/de.yml index 85cc91c5..6068d4c4 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -112,9 +112,7 @@ de: title: "Titel" node: slug: "Slug" - staged_slug: "Vorgemerkter Slug" parent_id: "Eltern-Node" - staged_parent_id: "Vorgemerkter Eltern-Node" head_id: "Head" related_asset: headline: "Aufmacher" @@ -137,11 +135,16 @@ de: trash_undeletable: "Der Papierkorb-Node kann nicht gelöscht werden" publish_in_trash: "Ein Node im Papierkorb kann nicht veröffentlicht werden" move_under_self: "Ein Node kann nicht unter sich selbst oder einen seiner Nachfahren verschoben werden" + move_target_missing: "Die Seite, unter die verschoben werden sollte, gibt es nicht mehr." trash_the_trash: "Der Papierkorb-Node selbst kann nicht in den Papierkorb verschoben werden" restore_target_invalid: "Das Wiederherstellungsziel muss ein lebender Node sein" destroy_outside_trash: "Nodes können nur aus dem Papierkorb gelöscht werden" attach_in_trash: "An einen Node im Papierkorb können keine Assets angehängt werden" not_permitted: "In diesem Bereich dürfen nur Mitglieder der Redaktion veröffentlichte Inhalte ändern" + page: + attributes: + slug: + reserved_for_trash: "ist für den Papierkorb reserviert" user: attributes: roles: @@ -676,7 +679,6 @@ de: trashed: "Seite wurde in den Papierkorb verschoben" already_trashed: "Seite ist bereits im Papierkorb" restored: "Seite wurde aus dem Papierkorb wiederhergestellt" - restore_target_missing: "Wiederherstellungsziel nicht gefunden" deleted: "Seite wurde endgültig gelöscht" published: "Entwurf wurde veröffentlicht" unlocked: "Node entsperrt" diff --git a/config/locales/en.yml b/config/locales/en.yml index 6cff43f0..524d07d0 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -63,9 +63,7 @@ en: title: "Title" node: slug: "Slug" - staged_slug: "Staged slug" parent_id: "Parent node" - staged_parent_id: "Staged parent node" head_id: "Head" related_asset: headline: "Headline" @@ -83,11 +81,16 @@ en: trash_undeletable: "The Trash node cannot be destroyed" publish_in_trash: "Cannot publish a node in the Trash" move_under_self: "Cannot move a node under itself or one of its own descendants" + move_target_missing: "The page this was to be moved under no longer exists." trash_the_trash: "The Trash node itself cannot be trashed" restore_target_invalid: "Restore target must be a living node" destroy_outside_trash: "Nodes are only destroyed from the Trash" attach_in_trash: "Cannot attach assets to a node in the Trash" not_permitted: "Only Redaktion members may change published content in this section" + page: + attributes: + slug: + reserved_for_trash: "is reserved for the Trash" user: attributes: roles: @@ -639,7 +642,6 @@ en: trashed: "Page has been moved to the Trash" already_trashed: "Page is already in the Trash" restored: "Page has been restored from the Trash" - restore_target_missing: "Restore target not found" deleted: "Page has been permanently deleted" published: "Draft has been published" unlocked: "Node unlocked" diff --git a/db/migrate/20260807010722_add_address_to_pages.rb b/db/migrate/20260807010722_add_address_to_pages.rb new file mode 100644 index 00000000..12cf89bd --- /dev/null +++ b/db/migrate/20260807010722_add_address_to_pages.rb @@ -0,0 +1,6 @@ +class AddAddressToPages < ActiveRecord::Migration[8.1] + def change + add_column :pages, :slug, :string + add_column :pages, :parent_node_id, :integer + end +end diff --git a/lib/tasks/pages.rake b/lib/tasks/pages.rake new file mode 100644 index 00000000..0cfbc5dc --- /dev/null +++ b/lib/tasks/pages.rake @@ -0,0 +1,33 @@ +namespace :pages do + desc "Backfill pages.slug and pages.parent_node_id from each page's " \ + "node. Historical accuracy is not attempted. Every revision gets " \ + "the node's current address, which is right for head and draft and " \ + "harmless for older revisions, and avoids nil checks everywhere. " \ + "Dry run unless WRITE=1." + task :backfill_address => :environment do + write = ENV["WRITE"] == "1" + puts "DRY RUN -- nothing written. Re-run with WRITE=1." unless write + + touched = 0 + Node.find_each do |node| + scope = node.pages.where("slug IS DISTINCT FROM :s OR parent_node_id IS DISTINCT FROM :p", + :s => node.slug, :p => node.parent_id) + count = scope.count + next if count.zero? + + scope.update_all(:slug => node.slug, :parent_node_id => node.parent_id) if write + touched += count + end + + # Autosaves carry no node_id -- has_many :pages does not cover them. + Node.where.not(:autosave_id => nil).includes(:autosave).find_each do |node| + a = node.autosave + next if a.slug == node.slug && a.parent_node_id == node.parent_id + + a.update_columns(:slug => node.slug, :parent_node_id => node.parent_id) if write + touched += 1 + end + + puts "#{write ? "updated" : "would update"} #{touched} pages" + end +end diff --git a/public/javascripts/admin_search.js b/public/javascripts/admin_search.js index 37b696e4..852f5339 100644 --- a/public/javascripts/admin_search.js +++ b/public/javascripts/admin_search.js @@ -257,7 +257,7 @@ move_to_search = { showRestricted: true, onSelect: function(node) { $("#move_to_search_term").val(node.title); - $("#node_staged_parent_id").val(node.node_id); + $("#page_parent_node_id").val(node.node_id); } }); } diff --git a/public/stylesheets/admin.css b/public/stylesheets/admin.css index 08a19a79..889a5729 100644 --- a/public/stylesheets/admin.css +++ b/public/stylesheets/admin.css @@ -1566,7 +1566,7 @@ form.button_to button[type="submit"] { } input[type=text]#tag_list, -input[type=text]#node_staged_slug, +input[type=text]#page_slug, input#move_to_search_term { padding: 5px; } @@ -1644,7 +1644,7 @@ input[type=text]#page_title { } input#tag_list, -input#node_staged_slug, +input#page_slug, input#move_to_search_term { box-sizing: border-box; width: 100%; diff --git a/test/controllers/nodes_controller_test.rb b/test/controllers/nodes_controller_test.rb index 745ae4e0..c7caeed1 100644 --- a/test/controllers/nodes_controller_test.rb +++ b/test/controllers/nodes_controller_test.rb @@ -193,7 +193,8 @@ class NodesControllerTest < ActionController::TestCase test "publish draft with staged_slug unqueal slug" do login_as :quentin - test_node = Node.root.children.create! :slug => "test_node", :staged_slug => "peter_pan" + test_node = Node.root.children.create!(:slug => "test_node") + test_node.draft.update!(:slug => "peter_pan") put :publish, params: { :id => test_node.id } @@ -205,7 +206,8 @@ class NodesControllerTest < ActionController::TestCase test "publish draft with staged_slug with more levels of nodes" do login_as :quentin - test_node = Node.root.children.create! :slug => "test_node", :staged_slug => "peter_pan" + test_node = Node.root.children.create!(:slug => "test_node") + test_node.draft.update!(:slug => "peter_pan") test_node2 = test_node.children.create! :slug => "test_node2" put :publish, params: { :id => test_node.id } @@ -215,12 +217,13 @@ class NodesControllerTest < ActionController::TestCase assert_equal "peter_pan", test_node.unique_name end - test "publish draft with staged_parent_id" do + test "publish draft with a moved parent" do login_as :quentin - parent = Node.root.children.create! :slug => "parent" - test_node = Node.root.children.create! :slug => "test_node", :staged_parent_id => parent.id - test_node2 = test_node.children.create! :slug => "test_node2" + parent = Node.root.children.create!(:slug => "parent") + test_node = Node.root.children.create!(:slug => "test_node") + test_node.draft.update!(:parent_node_id => parent.id) + test_node2 = test_node.children.create!(:slug => "test_node2") put :publish, params: { :id => test_node.id } @@ -229,18 +232,13 @@ class NodesControllerTest < ActionController::TestCase assert_equal "parent/test_node/test_node2", test_node2.unique_name end - test "publish draft with staged_parent_id and staged_slug" do + test "publish draft with a moved parent and a renamed slug" do login_as :quentin - parent = Node.root.children.create! :slug => "parent" - - test_node = Node.root.children.create!( - :slug => "test_node", - :staged_parent_id => parent.id, - :staged_slug => "peter_pan" - ) - - test_node2 = test_node.children.create! :slug => "test_node2" + parent = Node.root.children.create!(:slug => "parent") + test_node = Node.root.children.create!(:slug => "test_node") + test_node.draft.update!(:parent_node_id => parent.id, :slug => "peter_pan") + test_node2 = test_node.children.create!(:slug => "test_node2") put :publish, params: { :id => test_node.id } @@ -293,7 +291,7 @@ class NodesControllerTest < ActionController::TestCase other_node = Node.root.children.create( :slug => "other" ) - node.staged_parent_id = other_node.id + node.draft.update!(:parent_node_id => other_node.id) node.publish_draft! assert Node.valid? @@ -711,18 +709,33 @@ class NodesControllerTest < ActionController::TestCase assert flash[:error].present? end - test "restore_from_trash reparents to the given parent" do + test "restore_from_trash reparents to the 'old' parent" do login_as :quentin node = Node.root.children.create!(:slug => "restore_me") node.trash!(users(:quentin)) target = Node.root.children.create!(:slug => "restore_home") - put :restore_from_trash, params: { :id => node.id, :parent_id => target.id } + node.reload.draft.update!(:parent_node_id => target.id) + put :restore_from_trash, params: { :locale => "de", :id => node.id } assert_redirected_to node_path(node) assert_equal target, node.reload.parent end + test "restore_from_trash follows an explicitly chosen parent" do + login_as :quentin + node = Node.root.children.create!(:slug => "restore_pick") + node.trash!(users(:quentin)) + chosen = Node.root.children.create!(:slug => "chosen_home") + + put :restore_from_trash, params: { :locale => "de", :id => node.id, + :parent_id => chosen.id } + + assert_equal chosen, node.reload.parent + assert_equal chosen.id, node.draft.parent_node_id, + "the choice is recorded on the draft, not applied behind its back" + end + test "destroy refuses a node outside the Trash" do login_as :quentin node = Node.root.children.create!(:slug => "not_deletable_here") diff --git a/test/models/node_test.rb b/test/models/node_test.rb index c7fee58e..df1e96cb 100644 --- a/test/models/node_test.rb +++ b/test/models/node_test.rb @@ -488,7 +488,7 @@ class NodeTest < ActiveSupport::TestCase a = Node.root.children.create!(:slug => "cycle_guard_a") b = a.children.create!(:slug => "cycle_guard_b") - a.staged_parent_id = b.id + a.draft.update!(:parent_node_id => b.id) assert_raises(ActiveRecord::RecordInvalid) { a.publish_draft! } @@ -702,16 +702,15 @@ class NodeTest < ActiveSupport::TestCase assert_equal "New Title", action.metadata.dig("title", "to") end - test "publishing a staged slug change logs a move with the path pair" do + test "publishing a 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 + find_or_create_draft(node, @user1) + node.draft.update!(:slug => "moved-#{node.slug}") node.publish_draft!(@user1) - node.reload + assert_not_equal path_before, node.unique_name action = NodeAction.where(:action => "move").last @@ -719,16 +718,12 @@ class NodeTest < ActiveSupport::TestCase 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 + test "publishing a draft together with a 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! + node.draft.update!(:slug => "relocated-#{node.slug}") assert_difference "NodeAction.count", 2 do node.publish_draft!(@user1) @@ -890,7 +885,8 @@ class NodeTest < ActiveSupport::TestCase Node.trash assert_not Node.root.children.build(:slug => CccConventions::TRASH_SLUG).valid? - assert_not Node.root.children.build(:slug => "fine", :staged_slug => CccConventions::TRASH_SLUG).valid? + page = Page.new(:slug => CccConventions::TRASH_SLUG, :parent_node_id => Node.root.id) + assert_not page.valid? assert Node.trash.children.create!(:slug => "sub").children.build(:slug => CccConventions::TRASH_SLUG).valid? end @@ -1012,7 +1008,7 @@ class NodeTest < ActiveSupport::TestCase year = updates.children.create!(:slug => "2026") node = Node.root.children.create!(:slug => "outside-post") node.reload.draft.update!(:title => "Entwurf") - node.update!(:staged_parent_id => year.id) + node.draft.update!(:parent_node_id => year.id) assert_raises(ActiveRecord::RecordInvalid) { node.publish_draft!(editor) } assert_nil node.reload.head @@ -1025,7 +1021,7 @@ class NodeTest < ActiveSupport::TestCase club = Node.root.children.create!(:slug => "club") node = Node.root.children.create!(:slug => "movable-post") node.reload.draft.update!(:title => "Entwurf") - node.update!(:staged_parent_id => club.id) + node.draft.update!(:parent_node_id => club.id) node.publish_draft!(editor) assert_equal club.id, node.reload.parent_id @@ -1036,7 +1032,7 @@ class NodeTest < ActiveSupport::TestCase :password => "secret", :password_confirmation => "secret") node = Node.root.children.create!(:slug => "harmless") node.reload.draft.update!(:title => "Entwurf") - node.update!(:staged_slug => "updates") + node.draft.update!(:slug => "updates") assert_raises(ActiveRecord::RecordInvalid) { node.publish_draft!(editor) } assert_nil node.reload.head @@ -1057,7 +1053,9 @@ class NodeTest < ActiveSupport::TestCase node = Node.root.children.create!(:slug => "restorable") node.reload.trash! - assert_raises(ActiveRecord::RecordInvalid) { node.restore_from_trash!(updates, editor) } + node.reload.draft.update!(:parent_node_id => updates.id) + + assert_raises(ActiveRecord::RecordInvalid) { node.restore_from_trash!(editor) } assert node.reload.in_trash? end @@ -1068,7 +1066,8 @@ class NodeTest < ActiveSupport::TestCase node = Node.root.children.create!(:slug => "restorable_free") node.reload.trash! - node.restore_from_trash!(club, editor) + node.reload.draft.update!(:parent_node_id => club.id) + node.restore_from_trash!(editor) assert_not node.reload.in_trash? assert_equal club.id, node.parent_id end @@ -1101,4 +1100,65 @@ class NodeTest < ActiveSupport::TestCase node.update_external_url!("https://example.org", users(:quentin)) end end + + test "a new node's draft carries the node's address" do + parent = Node.root.children.create!(:slug => "addr_parent") + node = parent.children.create!(:slug => "addr_child") + + assert_equal "addr_child", node.draft.slug + assert_equal parent.id, node.draft.parent_node_id + end + + test "an autosave inherits the draft's address" do + node = Node.root.children.create!(:slug => "addr_autosave") + node.draft.update!(:slug => "renamed") + node.lock_for_editing!(users(:quentin)) + node.autosave!({ :title => "x" }, users(:quentin)) + + assert_equal "renamed", node.autosave.slug + end + + test "a blank slug in an autosave leaves the address unchanged" do + node = Node.root.children.create!(:slug => "addr_blank") + node.lock_for_editing!(users(:quentin)) + node.autosave!({ :slug => "", :title => "x" }, users(:quentin)) + + assert_equal "addr_blank", node.autosave.slug + end + + test "a node with no draft has nothing to publish" do + node = create_node_with_published_page + assert_nil node.draft + + assert_nil node.publish_draft!(@user1) + end + + test "prospective_unique_name reports the draft's intended address" do + parent = Node.root.children.create!(:slug => "prospective_parent") + node = Node.root.children.create!(:slug => "prospective_child") + node.draft.update!(:parent_node_id => parent.id) + + assert_equal "prospective_parent/prospective_child", + node.prospective_unique_name + end + + test "prospective_unique_name falls back to the live address when the parent is gone" do + parent = Node.root.children.create!(:slug => "doomed_parent") + node = Node.root.children.create!(:slug => "orphan_child") + node.draft.update!(:parent_node_id => parent.id) + parent.destroy! + + assert node.reload.draft.parent_node_missing? + assert_equal "orphan_child", node.prospective_unique_name + end + + test "publishing a draft whose parent is gone is refused" do + parent = Node.root.children.create!(:slug => "doomed_parent_two") + node = Node.root.children.create!(:slug => "orphan_child_two") + node.draft.update!(:parent_node_id => parent.id) + parent.destroy! + + assert_raises(ActiveRecord::RecordInvalid) { node.reload.publish_draft! } + assert_nil node.reload.head + end end diff --git a/test/models/node_trash_test.rb b/test/models/node_trash_test.rb index 3947f20e..a4774c58 100644 --- a/test/models/node_trash_test.rb +++ b/test/models/node_trash_test.rb @@ -76,7 +76,8 @@ class NodeTrashTest < ActiveSupport::TestCase node.trash!(@user1) target = Node.root.children.create!(:slug => "restore_target") - node.reload.restore_from_trash!(target, @user1) + node.reload.draft.update!(:parent_node_id => target.id) + node.reload.restore_from_trash!(@user1) node.reload assert_equal target, node.parent @@ -91,8 +92,11 @@ class NodeTrashTest < ActiveSupport::TestCase node.trash!(@user1) other_trashed = Node.trash.children.create!(:slug => "also_trashed") - assert_raises(ActiveRecord::RecordInvalid) { node.reload.restore_from_trash!(Node.trash, @user1) } - assert_raises(ActiveRecord::RecordInvalid) { node.reload.restore_from_trash!(other_trashed, @user1) } + node.reload.draft.update!(:parent_node_id => Node.trash.id) + assert_raises(ActiveRecord::RecordInvalid) { node.reload.restore_from_trash!(@user1) } + + node.reload.draft.update!(:parent_node_id => other_trashed.id) + assert_raises(ActiveRecord::RecordInvalid) { node.reload.restore_from_trash!(@user1) } end test "destroy_from_trash! refuses nodes outside the Trash" do -- cgit v1.3