summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorerdgeist <erdgeist@erdgeist.org>2026-07-10 02:03:19 +0200
committererdgeist <erdgeist@erdgeist.org>2026-07-10 02:03:19 +0200
commit205e6216fc7850fe717122c189e5003d1f9e8afe (patch)
tree8627354f3577f0a974949e759b0e93662840ade7
parent81769557804ce59ce3e4603e8f47e731b61b180d (diff)
Add head/draft/autosave layer comparison at three UI entry points
Node#resolve_page_reference and #available_layer_pairs let Page#diff_against compare named layers (head/draft/autosave), not just numbered revisions -- autosave was never part of Node#pages, so this was the missing piece. Wired into nodes#show's Status section, nodes#edit right after an autosave gets resurrected ("What changed?"), and the admin wizard's current-drafts table, which now also lists autosave-only nodes it previously never showed. revisions#diff hides the numbered-revision picker when comparing named layers (it can't represent them), shows a plain label instead, and offers buttons to switch between whichever other pairs make sense for the node's current state. Destroying the topmost layer is available directly from the diff view, reusing the existing revert! path. "Discard changes" is renamed "Discard Autosave" everywhere it appears, to match "Destroy Draft".
-rw-r--r--app/controllers/admin_controller.rb4
-rw-r--r--app/controllers/nodes_controller.rb2
-rw-r--r--app/controllers/revisions_controller.rb16
-rw-r--r--app/helpers/revisions_helper.rb5
-rw-r--r--app/models/node.rb21
-rw-r--r--app/views/admin/index.html.erb13
-rw-r--r--app/views/nodes/edit.html.erb9
-rw-r--r--app/views/nodes/show.html.erb12
-rw-r--r--app/views/revisions/diff.html.erb40
-rw-r--r--config/routes.rb1
-rw-r--r--test/controllers/admin_controller_test.rb15
-rw-r--r--test/controllers/revisions_controller_test.rb39
-rw-r--r--test/models/node_test.rb22
13 files changed, 179 insertions, 20 deletions
diff --git a/app/controllers/admin_controller.rb b/app/controllers/admin_controller.rb
index 3fa0519..6ab2135 100644
--- a/app/controllers/admin_controller.rb
+++ b/app/controllers/admin_controller.rb
@@ -5,10 +5,10 @@ class AdminController < ApplicationController
5 before_action :login_required 5 before_action :login_required
6 6
7 def index 7 def index
8 @drafts = Node.where("draft_id IS NOT NULL") 8 @drafts = Node.where("draft_id IS NOT NULL OR autosave_id IS NOT NULL")
9 .limit(50).order("updated_at desc") 9 .limit(50).order("updated_at desc")
10 10
11 @drafts_count = Node.where("draft_id IS NOT NULL").count 11 @drafts_count = Node.where("draft_id IS NOT NULL OR autosave_id IS NOT NULL").count
12 12
13 @recent_changes = Node.where( 13 @recent_changes = Node.where(
14 "updated_at < ? AND updated_at > ? AND parent_id IS NOT NULL", 14 "updated_at < ? AND updated_at > ? AND parent_id IS NOT NULL",
diff --git a/app/controllers/nodes_controller.rb b/app/controllers/nodes_controller.rb
index 38d42d9..d1538e1 100644
--- a/app/controllers/nodes_controller.rb
+++ b/app/controllers/nodes_controller.rb
@@ -72,7 +72,7 @@ class NodesController < ApplicationController
72 if @node.autosave 72 if @node.autosave
73 flash.now[:notice] = 73 flash.now[:notice] =
74 "This page has unsaved changes from a previous session, shown below. " \ 74 "This page has unsaved changes from a previous session, shown below. " \
75 "Save to keep them, or use \"Discard changes\" below to go back to the last saved version." 75 "Save to keep them, or use \"Discard Autosave\" below to go back to the last saved version."
76 elsif freshly_locked 76 elsif freshly_locked
77 flash.now[:notice] = "Node locked and ready to edit" 77 flash.now[:notice] = "Node locked and ready to edit"
78 end 78 end
diff --git a/app/controllers/revisions_controller.rb b/app/controllers/revisions_controller.rb
index 9acb26f..4b0c549 100644
--- a/app/controllers/revisions_controller.rb
+++ b/app/controllers/revisions_controller.rb
@@ -21,10 +21,18 @@ class RevisionsController < ApplicationController
21 params[:start_revision], params[:end_revision] = 1, 1 21 params[:start_revision], params[:end_revision] = 1, 1
22 end 22 end
23 23
24 @start = @node.pages.find_by_revision( params[:start_revision] ) 24 @start = @node.resolve_page_reference(params[:start_revision])
25 @end = @node.pages.find_by_revision( params[:end_revision] ) 25 @end = @node.resolve_page_reference(params[:end_revision])
26 @diff_view = params[:view] == "side_by_side" ? :side_by_side : :inline 26
27 @diff = @end.diff_against( @start, view: @diff_view ) 27 if @start.nil? || @end.nil?
28 flash[:error] = "That comparison is no longer available."
29 redirect_to(node_path(@node)) and return
30 end
31
32 @diff_view = params[:view] == "side_by_side" ? :side_by_side : :inline
33 @diff = @end.diff_against(@start, view: @diff_view)
34 @available_layer_pairs = @node.available_layer_pairs
35 @locked_by_other = @node.locked? && @node.lock_owner != current_user
28 end 36 end
29 37
30 def show 38 def show
diff --git a/app/helpers/revisions_helper.rb b/app/helpers/revisions_helper.rb
index fdb51f8..a629013 100644
--- a/app/helpers/revisions_helper.rb
+++ b/app/helpers/revisions_helper.rb
@@ -1,2 +1,7 @@
1module RevisionsHelper 1module RevisionsHelper
2 # Human-readable label for a diff endpoint -- "head"/"draft"/"autosave"
3 # get their name; anything else is a revision number.
4 def describe_page_reference(ref)
5 %w[head draft autosave].include?(ref.to_s) ? ref.to_s.capitalize : "revision #{ref}"
6 end
2end 7end
diff --git a/app/models/node.rb b/app/models/node.rb
index 7675ab6..82d9954 100644
--- a/app/models/node.rb
+++ b/app/models/node.rb
@@ -123,6 +123,27 @@ class Node < ApplicationRecord
123 self.draft.reload 123 self.draft.reload
124 end 124 end
125 125
126 def resolve_page_reference ref
127 case ref.to_s
128 when "head" then head
129 when "draft" then draft
130 when "autosave" then autosave
131 else pages.find_by_revision(ref)
132 end
133 end
134
135 # Which layer-pairs are meaningful to compare right now, given this
136 # node's actual state. Head vs autosave only shows up when no draft
137 # sits between them -- with a draft present, autosave is compared
138 # against the draft, never past it straight to head.
139 def available_layer_pairs
140 pairs = []
141 pairs << [:head, :draft] if head && draft
142 pairs << [:draft, :autosave] if draft && autosave
143 pairs << [:head, :autosave] if head && autosave && !draft
144 pairs
145 end
146
126 def find_or_create_draft current_user 147 def find_or_create_draft current_user
127 self.wipe_draft! 148 self.wipe_draft!
128 if draft && self.lock_owner == current_user 149 if draft && self.lock_owner == current_user
diff --git a/app/views/admin/index.html.erb b/app/views/admin/index.html.erb
index 77b45f4..c67ccb3 100644
--- a/app/views/admin/index.html.erb
+++ b/app/views/admin/index.html.erb
@@ -82,9 +82,9 @@
82</div> 82</div>
83 83
84<div id="current_drafts_table"> 84<div id="current_drafts_table">
85 85
86 <h2>Current Drafts (<%= @drafts_count %>)</h2> 86 <h2>Current Drafts & Autosaves (<%= @drafts_count %>)</h2>
87 87
88 <table class="node_table"> 88 <table class="node_table">
89 <tr class="header"> 89 <tr class="header">
90 <th class="node_id">ID</th> 90 <th class="node_id">ID</th>
@@ -92,6 +92,7 @@
92 <th class="actions">Actions</th> 92 <th class="actions">Actions</th>
93 <th class="editor">Locked by</th> 93 <th class="editor">Locked by</th>
94 <th class="revision">Rev.</th> 94 <th class="revision">Rev.</th>
95 <th class="autosave">Autosave</th>
95 </tr> 96 </tr>
96 <% @drafts.each do |node| %> 97 <% @drafts.each do |node| %>
97 <tr class="<%= cycle("even", "odd") %>"> 98 <tr class="<%= cycle("even", "odd") %>">
@@ -103,6 +104,9 @@
103 <td class="actions"> 104 <td class="actions">
104 <%= link_to 'show', node_path(node) %> 105 <%= link_to 'show', node_path(node) %>
105 <%= link_to 'Revisions', node_revisions_path(node) %> 106 <%= link_to 'Revisions', node_revisions_path(node) %>
107 <% if pair = node.available_layer_pairs.last %>
108 <%= link_to 'diff', diff_node_revisions_path(node, start_revision: pair.first, end_revision: pair.last) %>
109 <% end %>
106 </td> 110 </td>
107 <td> 111 <td>
108 <%= node.lock_owner.login if node.lock_owner %> 112 <%= node.lock_owner.login if node.lock_owner %>
@@ -110,6 +114,9 @@
110 <td> 114 <td>
111 <%= link_to ( node.draft ? node.draft.revision : (node.head ? node.head.revision : "EMPTY" ) ), node_revisions_path(node) %> 115 <%= link_to ( node.draft ? node.draft.revision : (node.head ? node.head.revision : "EMPTY" ) ), node_revisions_path(node) %>
112 </td> 116 </td>
117 <td class="autosave">
118 <%= node.autosave ? "unsaved changes" : "" %>
119 </td>
113 </tr> 120 </tr>
114 <% end %> 121 <% end %>
115 </table> 122 </table>
diff --git a/app/views/nodes/edit.html.erb b/app/views/nodes/edit.html.erb
index cdc9b36..1c19410 100644
--- a/app/views/nodes/edit.html.erb
+++ b/app/views/nodes/edit.html.erb
@@ -6,9 +6,16 @@
6 disabled: @node.autosave.present? %> 6 disabled: @node.autosave.present? %>
7 7
8 <% if @node.autosave || (@node.draft && @node.head) %> 8 <% if @node.autosave || (@node.draft && @node.head) %>
9 <%= button_to (@node.draft && !@node.autosave ? 'Destroy Draft' : 'Discard changes'), 9 <%= button_to (@node.draft && !@node.autosave ? 'Destroy Draft' : 'Discard Autosave'),
10 revert_node_path(@node), method: :put, 10 revert_node_path(@node), method: :put,
11 form: { data: { confirm: "This cannot be undone. Continue?" }, class: 'button_to destructive' } %> 11 form: { data: { confirm: "This cannot be undone. Continue?" }, class: 'button_to destructive' } %>
12 <% if pair = @node.available_layer_pairs.find { |p| p.include?(:autosave) } %>
13 <%= button_to 'What changed?',
14 diff_node_revisions_path(@node),
15 method: :get,
16 params: { start_revision: pair.first, end_revision: pair.last },
17 form: { class: 'button_to computation' } %>
18 <% end %>
12 <% end %> 19 <% end %>
13 20
14 <%= submit_tag "Save Draft", form: dom_id(@node, :edit) %> 21 <%= submit_tag "Save Draft", form: dom_id(@node, :edit) %>
diff --git a/app/views/nodes/show.html.erb b/app/views/nodes/show.html.erb
index 036caf2..2469310 100644
--- a/app/views/nodes/show.html.erb
+++ b/app/views/nodes/show.html.erb
@@ -32,6 +32,16 @@
32 <% end %> 32 <% end %>
33 </div> 33 </div>
34 34
35 <% @node.available_layer_pairs.each do |pair| %>
36 <div class="node_info_item">
37 <%= button_to "Diff #{pair.first.to_s.capitalize} vs. #{pair.last.to_s.capitalize}",
38 diff_node_revisions_path(@node),
39 method: :get,
40 params: { start_revision: pair.first, end_revision: pair.last },
41 form: { class: 'button_to computation' } %>
42 </div>
43 <% end %>
44
35 <% unless locked_by_other %> 45 <% unless locked_by_other %>
36 <% if @node.draft && !@node.autosave %> 46 <% if @node.draft && !@node.autosave %>
37 <div class="node_info_item"> 47 <div class="node_info_item">
@@ -41,7 +51,7 @@
41 <% end %> 51 <% end %>
42 <% if @node.draft || @node.autosave %> 52 <% if @node.draft || @node.autosave %>
43 <div class="node_info_item"> 53 <div class="node_info_item">
44 <%= button_to (@node.draft && !@node.autosave ? 'Destroy Draft' : 'Discard changes'), 54 <%= button_to (@node.draft && !@node.autosave ? 'Destroy Draft' : 'Discard Autosave'),
45 revert_node_path(@node), method: :put, 55 revert_node_path(@node), method: :put,
46 form: { data: { confirm: "This cannot be undone. Continue?" }, class: 'button_to destructive' } %> 56 form: { data: { confirm: "This cannot be undone. Continue?" }, class: 'button_to destructive' } %>
47 </div> 57 </div>
diff --git a/app/views/revisions/diff.html.erb b/app/views/revisions/diff.html.erb
index d7bb528..3157dca 100644
--- a/app/views/revisions/diff.html.erb
+++ b/app/views/revisions/diff.html.erb
@@ -4,11 +4,41 @@
4 <%= link_to 'Revisions', node_revisions_path(@node) %> 4 <%= link_to 'Revisions', node_revisions_path(@node) %>
5</p> 5</p>
6 6
7<%= form_tag diff_node_revisions_path do %> 7<p class="diff_comparison_label">
8 <%= select_tag :start_revision, options_for_select(@node.pages.map{|x| x.revision}, params[:start_revision].to_i) %> 8 Comparing <strong><%= describe_page_reference(params[:start_revision]) %></strong>
9 <%= select_tag :end_revision, options_for_select(@node.pages.map{|x| x.revision}, params[:end_revision].to_i) %> 9 against <strong><%= describe_page_reference(params[:end_revision]) %></strong>
10 <%= select_tag :view, options_for_select([['Inline', 'inline'], ['Side by side', 'side_by_side']], @diff_view) %> 10</p>
11 <%= submit_tag 'Diff' %> 11
12<% numeric_comparison = params[:start_revision].to_s =~ /\A\d+\z/ && params[:end_revision].to_s =~ /\A\d+\z/ %>
13
14<% if numeric_comparison %>
15 <%= form_tag diff_node_revisions_path do %>
16 <%= select_tag :start_revision, options_for_select(@node.pages.map{|x| x.revision}, params[:start_revision].to_i) %>
17 <%= select_tag :end_revision, options_for_select(@node.pages.map{|x| x.revision}, params[:end_revision].to_i) %>
18 <%= select_tag :view, options_for_select([['Inline', 'inline'], ['Side by side', 'side_by_side']], @diff_view) %>
19 <%= submit_tag 'Diff' %>
20 <% end %>
21<% else %>
22 <p class="node_action_bar standalone_action_bar"><%= link_to 'Compare two numbered revisions instead', node_revisions_path(@node) %></p>
23<% end %>
24
25<% if @available_layer_pairs.present? %>
26 <p class="node_action_bar standalone_action_bar">
27 <% @available_layer_pairs.each do |pair| %>
28 <% next if [params[:start_revision].to_s, params[:end_revision].to_s].sort == pair.map(&:to_s).sort %>
29 <%= button_to "Diff #{pair.first.to_s.capitalize} vs. #{pair.last.to_s.capitalize}",
30 diff_node_revisions_path(@node),
31 method: :get,
32 params: { start_revision: pair.first, end_revision: pair.last, view: @diff_view },
33 form: { class: 'button_to computation' } %>
34 <% end %>
35
36 <% if !@locked_by_other && (@node.autosave || @node.draft) %>
37 <%= button_to (@node.draft && !@node.autosave ? 'Destroy Draft' : 'Discard Autosave'),
38 revert_node_path(@node), method: :put,
39 form: { data: { confirm: "This cannot be undone. Continue?" }, class: 'button_to destructive' } %>
40 <% end %>
41 </p>
12<% end %> 42<% end %>
13 43
14<div id="diffview"> 44<div id="diffview">
diff --git a/config/routes.rb b/config/routes.rb
index c0aef2f..da6b626 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -47,6 +47,7 @@ Cccms::Application.routes.draw do
47 resources :revisions do 47 resources :revisions do
48 collection do 48 collection do
49 post :diff 49 post :diff
50 get :diff
50 end 51 end
51 member do 52 member do
52 put :restore 53 put :restore
diff --git a/test/controllers/admin_controller_test.rb b/test/controllers/admin_controller_test.rb
index 9bbf29b..d6005ba 100644
--- a/test/controllers/admin_controller_test.rb
+++ b/test/controllers/admin_controller_test.rb
@@ -1,8 +1,17 @@
1require 'test_helper' 1require 'test_helper'
2 2
3class AdminControllerTest < ActionController::TestCase 3class AdminControllerTest < ActionController::TestCase
4 # Replace this with your real tests. 4 test "current drafts includes nodes with only an autosave" do
5 test "the truth" do 5 node = Node.root.children.create!(:slug => "admin_autosave_only")
6 assert true 6 node.lock_for_editing!(User.find_by_login("aaron"))
7 node.autosave!({title: "in progress"}, User.find_by_login("aaron"))
8 node.save_draft!(User.find_by_login("aaron"))
9 node.publish_draft!
10 node.lock_for_editing!(User.find_by_login("aaron"))
11 node.autosave!({title: "editing again"}, User.find_by_login("aaron"))
12
13 login_as :quentin
14 get :index
15 assert_includes assigns(:drafts), node
7 end 16 end
8end 17end
diff --git a/test/controllers/revisions_controller_test.rb b/test/controllers/revisions_controller_test.rb
index caca6bf..162e6f1 100644
--- a/test/controllers/revisions_controller_test.rb
+++ b/test/controllers/revisions_controller_test.rb
@@ -100,4 +100,43 @@ class RevisionsControllerTest < ActionController::TestCase
100 assert_response :success 100 assert_response :success
101 assert_select ".diff_column", 2 101 assert_select ".diff_column", 2
102 end 102 end
103
104 test "diffing head against draft by name" do
105 login_as :quentin
106 @node.find_or_create_draft(@user)
107 @node.draft.update(:body => "draft body")
108
109 post(:diff, params: { :node_id => @node.id, :start_revision => "head", :end_revision => "draft" })
110 assert_response :success
111 end
112
113 test "diffing a layer pair that no longer exists redirects with a flash" do
114 login_as :quentin
115 post(:diff, params: { :node_id => @node.id, :start_revision => "draft", :end_revision => "autosave" })
116 assert_redirected_to node_path(@node)
117 assert flash[:error].present?
118 end
119
120 test "diffing by name shows a clear comparison label instead of a misleading revision picker" do
121 login_as :quentin
122 @node.find_or_create_draft(@user)
123
124 post(:diff, params: { :node_id => @node.id, :start_revision => "head", :end_revision => "draft" })
125 assert_response :success
126 assert_select "strong", "Head"
127 assert_select "strong", "Draft"
128 assert_select "select[name=?]", "start_revision", :count => 0
129 end
130
131 test "pair-switcher buttons carry their params as real hidden fields, not a query string" do
132 login_as :quentin
133 @node.find_or_create_draft(@user)
134 @node.lock_for_editing!(@user)
135 @node.autosave!({ :body => "unsaved" }, @user)
136
137 post(:diff, params: { :node_id => @node.id, :start_revision => "head", :end_revision => "draft" })
138 assert_response :success
139 assert_select "form.computation input[type=hidden][name=start_revision]"
140 assert_select "form.computation input[type=hidden][name=end_revision]"
141 end
103end 142end
diff --git a/test/models/node_test.rb b/test/models/node_test.rb
index 2138c19..9e71dec 100644
--- a/test/models/node_test.rb
+++ b/test/models/node_test.rb
@@ -473,4 +473,26 @@ class NodeTest < ActiveSupport::TestCase
473 node.publish_draft! 473 node.publish_draft!
474 end 474 end
475 end 475 end
476
477 test "available_layer_pairs matches the six-state table" do
478 node = Node.root.children.create!(:slug => "layer_pairs_test")
479 user = @user1 || User.find_by_login("aaron")
480
481 assert_equal [[:draft, :autosave]], (node.lock_for_editing!(user); node.autosave!({title: "v1"}, user); node.available_layer_pairs) # state F
482
483 node.save_draft!(user)
484 node.publish_draft!
485 assert_equal [], node.available_layer_pairs # state A
486
487 node.lock_for_editing!(user)
488 node.autosave!({title: "v2"}, user)
489 assert_equal [[:head, :autosave]], node.available_layer_pairs # state B
490
491 node.save_draft!(user)
492 assert_equal [[:head, :draft]], node.available_layer_pairs # state C
493
494 node.lock_for_editing!(user)
495 node.autosave!({title: "v3"}, user)
496 assert_equal [[:head, :draft], [:draft, :autosave]], node.available_layer_pairs # state D
497 end
476end 498end