summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorerdgeist <erdgeist@erdgeist.org>2026-07-18 02:15:29 +0200
committererdgeist <erdgeist@erdgeist.org>2026-07-18 02:15:29 +0200
commitd41ee504caedbe858e24ab2a23c7a804454c64c8 (patch)
treeeb3ec7c8f44d15fea40292d0a9e7358e41640edc
parent5dced8b4b624aabf4215ba21b13957080345c326 (diff)
Add Trash affordances: cockpit, listing, dashboard entry
nodes#show gains a Trash section on trashed nodes: provenance from the trash entry, a restore form whose parent picker pre-fills the old parent while it still lives, and permanent deletion. A Move-to-Trash button joins the status actions on living nodes. nodes#trashed lists trashed subtree roots with weight, provenance, and deletion; the dashboard housekeeping row links to it, and trash/destroy redirect there. Deletion from Trash now removes the whole subtree, deepest first, each node through a real destroy! so every per-node cascade runs -- amending the never-recursive rule for this one sanctioned path (both confirms state the count; the root entry carries destroyed_descendants). Bare Node#destroy still refuses children.
-rw-r--r--app/controllers/nodes_controller.rb10
-rw-r--r--app/models/node.rb43
-rw-r--r--app/models/node_action.rb2
-rw-r--r--app/views/admin/index.html.erb4
-rw-r--r--app/views/nodes/show.html.erb53
-rw-r--r--app/views/nodes/trashed.html.erb42
-rw-r--r--config/routes.rb1
-rw-r--r--public/javascripts/admin_interface.js4
-rw-r--r--public/javascripts/admin_search.js13
-rw-r--r--public/stylesheets/admin.css35
-rw-r--r--test/controllers/nodes_controller_test.rb31
-rw-r--r--test/models/node_trash_test.rb41
12 files changed, 260 insertions, 19 deletions
diff --git a/app/controllers/nodes_controller.rb b/app/controllers/nodes_controller.rb
index 9ea66ad..021f8ff 100644
--- a/app/controllers/nodes_controller.rb
+++ b/app/controllers/nodes_controller.rb
@@ -144,7 +144,7 @@ class NodesController < ApplicationController
144 def trash 144 def trash
145 if @node.trash!(current_user) 145 if @node.trash!(current_user)
146 flash[:notice] = "Page has been moved to the Trash" 146 flash[:notice] = "Page has been moved to the Trash"
147 redirect_to node_path(Node.trash) 147 redirect_to trashed_nodes_path
148 else 148 else
149 flash[:notice] = "Page is already in the Trash" 149 flash[:notice] = "Page is already in the Trash"
150 redirect_to node_path(@node) 150 redirect_to node_path(@node)
@@ -170,7 +170,8 @@ class NodesController < ApplicationController
170 def destroy 170 def destroy
171 @node.destroy_from_trash!(current_user) 171 @node.destroy_from_trash!(current_user)
172 flash[:notice] = "Page has been permanently deleted" 172 flash[:notice] = "Page has been permanently deleted"
173 redirect_to node_path(Node.trash) 173 redirect_to trashed_nodes_path
174
174 rescue ActiveRecord::RecordInvalid, ActiveRecord::RecordNotDestroyed => e 175 rescue ActiveRecord::RecordInvalid, ActiveRecord::RecordNotDestroyed => e
175 flash[:error] = e.message 176 flash[:error] = e.message
176 redirect_to node_path(@node) 177 redirect_to node_path(@node)
@@ -247,6 +248,11 @@ class NodesController < ApplicationController
247 @sitemap_descendant_counts = descendant_counts_for(@sitemap) 248 @sitemap_descendant_counts = descendant_counts_for(@sitemap)
248 end 249 end
249 250
251 def trashed
252 @nodes = Node.trash.children.order(:slug)
253 .paginate(:page => params[:page], :per_page => 50)
254 end
255
250 private 256 private
251 257
252 def slug_for(title) 258 def slug_for(title)
diff --git a/app/models/node.rb b/app/models/node.rb
index 4dae287..8d0756a 100644
--- a/app/models/node.rb
+++ b/app/models/node.rb
@@ -4,6 +4,7 @@ class Node < ApplicationRecord
4 4
5 # Associations 5 # Associations
6 has_many :pages, -> { order("revision ASC") }, :dependent => :destroy 6 has_many :pages, -> { order("revision ASC") }, :dependent => :destroy
7 has_many :node_actions, :dependent => :nullify
7 belongs_to :head, :class_name => "Page", :foreign_key => :head_id, optional: true 8 belongs_to :head, :class_name => "Page", :foreign_key => :head_id, optional: true
8 belongs_to :draft, :class_name => "Page", :foreign_key => :draft_id, optional: true 9 belongs_to :draft, :class_name => "Page", :foreign_key => :draft_id, optional: true
9 # Autosave pages carry no node_id, so has_many :pages does not cover 10 # Autosave pages carry no node_id, so has_many :pages does not cover
@@ -351,19 +352,49 @@ class Node < ApplicationRecord
351 end 352 end
352 end 353 end
353 354
354 # Final deletion. Only from inside the Trash, never with children. 355 # Final deletion -- only from inside the Trash. Removes the whole
355 # The log entry is written first, in the same transaction; node_id 356 # subtree, deepest first, each node through a real destroy! so every
356 # nullifies when the row dies, and the metadata carries what remains. 357 # per-node cascade runs (the categorical difference from the old
358 # delete_all nuke). refuse_destroy_with_children on bare destroy is
359 # untouched
360 # One log entry at the root, per the subtree rule, written before the
361 # rows die.
357 def destroy_from_trash! current_user = nil 362 def destroy_from_trash! current_user = nil
358 raise ActiveRecord::RecordInvalid.new(self), "Nodes are only destroyed from the Trash" unless in_trash? 363 raise ActiveRecord::RecordInvalid.new(self), "Nodes are only destroyed from the Trash" unless in_trash?
359 364
360 ActiveRecord::Base.transaction do 365 ActiveRecord::Base.transaction do
361 NodeAction.record!(:node => self, :user => current_user, :action => "destroy", 366 doomed = self_and_descendants_ordered_with_level
362 :path => unique_name) 367 .sort_by { |_, level| -level }
363 destroy! 368 .map(&:first)
369
370 metadata = { :path => unique_name }
371 metadata[:destroyed_descendants] = doomed.size - 1 if doomed.size > 1
372
373 NodeAction.record!(:node => self, :user => current_user, :action => "destroy", **metadata)
374 doomed.each(&:destroy!)
364 end 375 end
365 end 376 end
366 377
378 # The most recent trash entry. Its path.from is the restore hint.
379 def last_trash_entry
380 node_actions.where(:action => "trash").order(:occurred_at => :desc, :id => :desc).first
381 end
382
383 # The node's pre-trash parent, if a living node still answers to
384 # that path. Nil when the parent was itself trashed (its unique_name
385 # changed) or deleted; a different node that has since taken over
386 # the path is a legitimate suggestion.
387 def suggested_restore_parent
388 from = last_trash_entry&.metadata&.dig("path", "from")
389 return nil unless from
390
391 parent_path = from.rpartition("/").first
392 return Node.root if parent_path.empty?
393
394 candidate = Node.find_by(:unique_name => parent_path)
395 candidate unless candidate.nil? || candidate.in_trash? || candidate.trash_node?
396 end
397
367 # returns an array with all parts of a unique_name rather than a string 398 # returns an array with all parts of a unique_name rather than a string
368 def unique_path 399 def unique_path
369 unique_name.to_s.split("/") 400 unique_name.to_s.split("/")
diff --git a/app/models/node_action.rb b/app/models/node_action.rb
index 6e09b5b..13bd5ba 100644
--- a/app/models/node_action.rb
+++ b/app/models/node_action.rb
@@ -65,6 +65,8 @@ class NodeAction < ApplicationRecord
65 # "destroy" (only from inside the Trash, never with children; the 65 # "destroy" (only from inside the Trash, never with children; the
66 # entry is written in the same transaction before the row dies): 66 # entry is written in the same transaction before the row dies):
67 # "path" -- final path, flat string (create-symmetric) 67 # "path" -- final path, flat string (create-symmetric)
68 # "destroyed_descendants" -- integer, only when positive; one entry
69 # at the root, per the subtree rule.
68 # 70 #
69 # Reserved: "demote" (via "trash" | "depublish") for an explicit 71 # Reserved: "demote" (via "trash" | "depublish") for an explicit
70 # depublish workflow, if ever built. 72 # depublish workflow, if ever built.
diff --git a/app/views/admin/index.html.erb b/app/views/admin/index.html.erb
index 7933cba..319530d 100644
--- a/app/views/admin/index.html.erb
+++ b/app/views/admin/index.html.erb
@@ -68,5 +68,9 @@
68 <%= link_to menu_items_path, class: "action_button" do %> 68 <%= link_to menu_items_path, class: "action_button" do %>
69 <%= icon("menu-2", library: "tabler", "aria-hidden": true) %> Navigation 69 <%= icon("menu-2", library: "tabler", "aria-hidden": true) %> Navigation
70 <% end %> 70 <% end %>
71 <% trash_count = Node.trash.children.count %>
72 <%= link_to trashed_nodes_path, class: "action_button" do %>
73 <%= icon("trash", library: "tabler", "aria-hidden": true) %> Trash<%= " (#{trash_count})" if trash_count > 0 %>
74 <% end %>
71 </div> 75 </div>
72</div> 76</div>
diff --git a/app/views/nodes/show.html.erb b/app/views/nodes/show.html.erb
index ae25571..4d4f6d5 100644
--- a/app/views/nodes/show.html.erb
+++ b/app/views/nodes/show.html.erb
@@ -46,7 +46,7 @@
46 <% end %> 46 <% end %>
47 47
48 <% unless locked_by_other %> 48 <% unless locked_by_other %>
49 <% if @node.draft && !@node.autosave %> 49 <% if @node.draft && !@node.autosave && !@node.in_trash? && !@node.trash_node? %>
50 <div class="node_info_item"> 50 <div class="node_info_item">
51 <%= button_to 'Publish', publish_node_path(@node), method: :put, 51 <%= button_to 'Publish', publish_node_path(@node), method: :put,
52 form: { data: { confirm: "Publish this draft?" }, class: 'button_to state_changing' } %> 52 form: { data: { confirm: "Publish this draft?" }, class: 'button_to state_changing' } %>
@@ -62,6 +62,15 @@
62 <% end %> 62 <% end %>
63 </div> 63 </div>
64 <% end %> 64 <% end %>
65 <% unless @node.trash_node? || @node.in_trash? || @node.root? %>
66 <div class="node_info_item">
67 <%= button_to trash_node_path(@node), method: :put,
68 form: { data: { confirm: "Move this page and everything beneath it to the Trash? All published content in it will go offline." }, class: 'button_to destructive' } do %>
69 <%= icon("trash", library: "tabler", "aria-hidden": true) %>
70 Move to Trash
71 <% end %>
72 </div>
73 <% end %>
65 <% end %> 74 <% end %>
66 </div> 75 </div>
67 76
@@ -70,6 +79,48 @@
70 <% end %> 79 <% end %>
71 </div> 80 </div>
72 81
82 <% if @node.in_trash? %>
83 <div class="node_description">Trash</div>
84 <div class="node_content node_info_group">
85 <div class="node_info_group_items">
86 <% if (entry = @node.last_trash_entry) && entry.metadata.dig("path", "from") %>
87 <div class="node_info_item">
88 <span class="node_info_label">Was at</span>
89 <%= entry.metadata.dig("path", "from") %>
90 </div>
91 <% end %>
92 <div class="node_info_item">
93 <span class="node_info_label">Restore to</span>
94 <% suggestion = @node.suggested_restore_parent %>
95 <%= form_tag restore_from_trash_node_path(@node), :method => :put, :class => "aligned_action_row" do %>
96 <div class="restore_picker">
97 <%= text_field_tag :restore_search_term,
98 suggestion && (suggestion.head&.title || suggestion.draft&.title || suggestion.slug),
99 :placeholder => "Search for a new parent…" %>
100 <div id="restore_search_results" class="search_results"></div>
101 </div>
102 <%= hidden_field_tag :parent_id, suggestion&.id %>
103 <%= submit_tag "Restore here", :class => "action_button action_button_no_margin_top" %>
104 <% end %>
105 <span class="field_hint">Restored pages come back unpublished. Republish each page deliberately.</span>
106 </div>
107 </div>
108 <div class="node_info_group_items">
109 <div class="node_info_item">
110 <% doomed_below = @node.descendants.count %>
111 <%= button_to node_path(@node), method: :delete,
112 form: { data: { confirm: doomed_below > 0 ?
113 "Delete this page and the #{doomed_below} pages beneath it permanently? This cannot be undone." :
114 "Delete this page permanently? This cannot be undone." },
115 class: 'button_to destructive' } do %>
116 <%= icon("trash-x", library: "tabler", "aria-hidden": true) %>
117 Delete permanently<%= " (including #{doomed_below} beneath)" if doomed_below > 0 %>
118 <% end %>
119 </div>
120 </div>
121 </div>
122 <% end %>
123
73 <div class="node_description">Translations</div> 124 <div class="node_description">Translations</div>
74 <div class="node_content node_info_group"> 125 <div class="node_content node_info_group">
75 <div class="node_info_group_items"> 126 <div class="node_info_group_items">
diff --git a/app/views/nodes/trashed.html.erb b/app/views/nodes/trashed.html.erb
new file mode 100644
index 0000000..9a4808e
--- /dev/null
+++ b/app/views/nodes/trashed.html.erb
@@ -0,0 +1,42 @@
1<h1>Trash</h1>
2
3<% if @nodes.empty? %>
4 <p class="field_hint">The Trash is empty.</p>
5<% else %>
6 <%= will_paginate @nodes %>
7 <table id="trashed_node_list">
8 <tr>
9 <th>Title</th>
10 <th>Was at</th>
11 <th>Trashed</th>
12 <th>Actions</th>
13 </tr>
14 <% @nodes.each do |node| %>
15 <% entry = node.last_trash_entry %>
16 <tr>
17 <% doomed_below = node.descendants.count %>
18 <td>
19 <%= link_to (node.draft&.title || node.slug), node_path(node) %><%= " (+ #{doomed_below} beneath)" if doomed_below > 0 %>
20 </td>
21 <td><%= entry&.metadata&.dig("path", "from") %></td>
22 <td>
23 <% if entry %>
24 <%= entry.occurred_at.strftime("%Y-%m-%d %H:%M") %> by <%= entry.actor_name %>
25 <% end %>
26 </td>
27 <td>
28 <%= button_to node_path(node), method: :delete,
29 form: { data: { confirm: doomed_below > 0 ?
30 "Delete \"#{node.draft&.title || node.slug}\" and the #{doomed_below} pages beneath it permanently? This cannot be undone." :
31 "Delete \"#{node.draft&.title || node.slug}\" permanently? This cannot be undone." },
32 class: 'button_to destructive' } do %>
33 <%= icon("trash-x", library: "tabler", "aria-hidden": true) %>
34 Delete permanently
35 <% end %>
36 </td>
37 </tr>
38 <% end %>
39 </table>
40 <%= will_paginate @nodes %>
41 <p class="field_hint">To restore a page, open it and pick a new parent in its Trash section.</p>
42<% end %>
diff --git a/config/routes.rb b/config/routes.rb
index 16145c5..f389326 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -44,6 +44,7 @@ Cccms::Application.routes.draw do
44 get :mine 44 get :mine
45 get :chapters 45 get :chapters
46 get :sitemap 46 get :sitemap
47 get :trashed
47 end 48 end
48 49
49 member do 50 member do
diff --git a/public/javascripts/admin_interface.js b/public/javascripts/admin_interface.js
index 3988016..c0e2d9c 100644
--- a/public/javascripts/admin_interface.js
+++ b/public/javascripts/admin_interface.js
@@ -57,6 +57,10 @@ $(document).ready(function () {
57 move_to_search.initialize_search(); 57 move_to_search.initialize_search();
58 } 58 }
59 59
60 if ($("#restore_search_term").length != 0) {
61 restore_search.initialize_search();
62 }
63
60 if ($("#event_node_search_term").length != 0) { 64 if ($("#event_node_search_term").length != 0) {
61 event_search.initialize_search(); 65 event_search.initialize_search();
62 } 66 }
diff --git a/public/javascripts/admin_search.js b/public/javascripts/admin_search.js
index d135503..b53c931 100644
--- a/public/javascripts/admin_search.js
+++ b/public/javascripts/admin_search.js
@@ -262,6 +262,19 @@ move_to_search = {
262 } 262 }
263}; 263};
264 264
265restore_search = {
266 initialize_search : function() {
267 initSearchPicker({
268 inputSelector: "#restore_search_term",
269 resultsSelector: "#restore_search_results",
270 onSelect: function(node) {
271 $("#restore_search_term").val(node.title);
272 $("#parent_id").val(node.node_id);
273 }
274 });
275 }
276};
277
265event_search = { 278event_search = {
266 initialize_search : function() { 279 initialize_search : function() {
267 initSearchPicker({ 280 initSearchPicker({
diff --git a/public/stylesheets/admin.css b/public/stylesheets/admin.css
index 82963dd..88fd84b 100644
--- a/public/stylesheets/admin.css
+++ b/public/stylesheets/admin.css
@@ -884,6 +884,10 @@ form.button_to button[type="submit"] {
884 margin-top: -5px; 884 margin-top: -5px;
885} 885}
886 886
887.aligned_action_row .action_button.action_button_no_margin_top {
888 margin-top: 0;
889}
890
887/* ============================================================ 891/* ============================================================
888 Page editor / metadata forms 892 Page editor / metadata forms
889 ============================================================ */ 893 ============================================================ */
@@ -1539,3 +1543,34 @@ div#image_browser ul li {
1539 font-size: smaller; 1543 font-size: smaller;
1540 color: #777; 1544 color: #777;
1541} 1545}
1546
1547/* ============================================================
1548 Trash section
1549 ============================================================ */
1550
1551.restore_form {
1552 display: flex;
1553 align-items: center;
1554 gap: 8px;
1555}
1556
1557.restore_picker {
1558 position: relative;
1559}
1560
1561.restore_picker input[type=text] {
1562 width: 22em;
1563}
1564
1565.restore_picker .search_results {
1566 position: absolute;
1567 top: 100%;
1568 left: 0;
1569 right: 0;
1570 z-index: 10;
1571 background: #fff;
1572 border: 1px solid #989898;
1573 border-top: none;
1574 max-height: 20em;
1575 overflow-y: auto;
1576}
diff --git a/test/controllers/nodes_controller_test.rb b/test/controllers/nodes_controller_test.rb
index 7e5a990..6e2ddb3 100644
--- a/test/controllers/nodes_controller_test.rb
+++ b/test/controllers/nodes_controller_test.rb
@@ -427,11 +427,11 @@ class NodesControllerTest < ActionController::TestCase
427 test "show never renders a destroy link for events" do 427 test "show never renders a destroy link for events" do
428 login_as :quentin 428 login_as :quentin
429 node = create_node_with_published_page 429 node = create_node_with_published_page
430 Event.create!(node_id: node.id, start_time: Time.now, end_time: Time.now + 1.hour) 430 event = Event.create!(node_id: node.id, start_time: Time.now, end_time: Time.now + 1.hour)
431 431
432 get :show, params: { id: node.id } 432 get :show, params: { id: node.id }
433 assert_response :success 433 assert_response :success
434 assert_select "form.button_to.destructive", count: 0 434 assert_select "form[action=?]", event_path(event), count: 0
435 end 435 end
436 436
437 test "reverting from nodes#show returns to the show page, not the editor, even if a draft remains" do 437 test "reverting from nodes#show returns to the show page, not the editor, even if a draft remains" do
@@ -479,7 +479,8 @@ class NodesControllerTest < ActionController::TestCase
479 login_as :quentin 479 login_as :quentin
480 get :show, params: { :id => node.id } 480 get :show, params: { :id => node.id }
481 assert_response :success 481 assert_response :success
482 assert_select "form.destructive", :count => 0 482 assert_select "form[action=?]", revert_node_path(node), count: 0
483 assert_select "form[action=?]", trash_node_path(node), count: 1
483 end 484 end
484 485
485 test "drafts includes a never-published node with only a draft" do 486 test "drafts includes a never-published node with only a draft" do
@@ -679,7 +680,7 @@ class NodesControllerTest < ActionController::TestCase
679 680
680 put :trash, params: { :id => node.id } 681 put :trash, params: { :id => node.id }
681 682
682 assert_redirected_to node_path(Node.trash) 683 assert_redirected_to trashed_nodes_path
683 assert node.reload.in_trash? 684 assert node.reload.in_trash?
684 end 685 end
685 686
@@ -722,6 +723,26 @@ class NodesControllerTest < ActionController::TestCase
722 delete :destroy, params: { :id => node.id } 723 delete :destroy, params: { :id => node.id }
723 724
724 assert_not Node.exists?(node.id) 725 assert_not Node.exists?(node.id)
725 assert_redirected_to node_path(Node.trash) 726 assert_redirected_to trashed_nodes_path
727 end
728
729 test "trash lists trashed subtree roots" do
730 login_as :quentin
731 node = Node.root.children.create!(:slug => "listed_in_trash")
732 node.trash!(users(:quentin))
733
734 get :trashed
735 assert_response :success
736 assert_select "a[href=?]", node_path(node)
737 end
738
739 test "trashed rows carry provenance and a delete for childless roots" do
740 login_as :quentin
741 node = Node.root.children.create!(:slug => "provenance_test")
742 node.trash!(users(:quentin))
743
744 get :trashed
745 assert_select "td", /quentin/
746 assert_select "form[action=?]", node_path(node), count: 1
726 end 747 end
727end 748end
diff --git a/test/models/node_trash_test.rb b/test/models/node_trash_test.rb
index b72770b..07e5bc6 100644
--- a/test/models/node_trash_test.rb
+++ b/test/models/node_trash_test.rb
@@ -102,12 +102,25 @@ class NodeTrashTest < ActiveSupport::TestCase
102 assert Node.exists?(node.id) 102 assert Node.exists?(node.id)
103 end 103 end
104 104
105 test "destroy_from_trash! refuses nodes with children" do 105 test "destroy_from_trash! removes the whole subtree bottom-up with one entry" do
106 node = create_node_with_published_page 106 parent = create_node_with_published_page
107 node.children.create!(:slug => "still_here") 107 child = parent.children.create!(:slug => "doomed_child")
108 node.trash!(@user1) 108 grandchild = child.children.create!(:slug => "doomed_grandchild")
109 page_ids = [parent, child, grandchild].flat_map { |n| n.pages.pluck(:id) }
110 parent.trash!(@user1)
111
112 assert_difference "NodeAction.count", 1 do
113 parent.reload.destroy_from_trash!(@user1)
114 end
115
116 assert_not Node.exists?(parent.id)
117 assert_not Node.exists?(child.id)
118 assert_not Node.exists?(grandchild.id)
119 assert_equal 0, Page.where(:id => page_ids).count
109 120
110 assert_raises(ActiveRecord::RecordNotDestroyed) { node.reload.destroy_from_trash!(@user1) } 121 action = NodeAction.where(:action => "destroy").last
122 assert_equal 2, action.metadata["destroyed_descendants"]
123 assert_nil action.node_id
111 end 124 end
112 125
113 test "destroy_from_trash! logs an entry that survives the node" do 126 test "destroy_from_trash! logs an entry that survives the node" do
@@ -124,4 +137,22 @@ class NodeTrashTest < ActiveSupport::TestCase
124 assert_equal "Doomed", action.subject_name 137 assert_equal "Doomed", action.subject_name
125 assert_equal @user1, action.user 138 assert_equal @user1, action.user
126 end 139 end
140
141 test "suggested_restore_parent finds the old parent while it lives" do
142 parent = Node.root.children.create!(:slug => "old_home")
143 node = parent.children.create!(:slug => "comes_back")
144 node.trash!(@user1)
145
146 assert_equal parent, node.reload.suggested_restore_parent
147
148 parent.trash!(@user1)
149 assert_nil node.reload.suggested_restore_parent
150 end
151
152 test "suggested_restore_parent is root for trashed top-level nodes" do
153 node = Node.root.children.create!(:slug => "was_top_level")
154 node.trash!(@user1)
155
156 assert_equal Node.root, node.reload.suggested_restore_parent
157 end
127end 158end