Move the pending address from the node onto the draft

3 files changed, 117 insertions, 40 deletions

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