diff options
| author | erdgeist <erdgeist@erdgeist.org> | 2026-07-08 17:32:17 +0200 |
|---|---|---|
| committer | erdgeist <erdgeist@erdgeist.org> | 2026-07-08 17:32:17 +0200 |
| commit | bc03601ee5c7acd4ef012ec4a404bd7b76bceaa0 (patch) | |
| tree | 9b160ea6194ffe27b6d8471b92a84bd0bdad4e2b | |
| parent | 6ad96c44d04df01e6abde097c681e824dd5fe745 (diff) | |
Add revert!/discard and rebuild nodes#edit around the new hierarchy
revert! discards exactly the topmost non-empty layer -- autosave if
present, else draft -- and reveals whatever's beneath it, releasing the
lock only once nothing is left to protect. Guards against destroying a
brand-new, never-published node's only draft, which would violate the
(head | draft) invariant every other method here assumes holds; the
view's Destroy/Discard button is gated the same way.
nodes#edit now calls lock_for_editing! instead of find_or_create_draft,
and always displays autosave || draft || head, resurrecting an
abandoned session's unsaved content by default with an explicit flash
explaining what's shown and how to get back to the last saved version.
The view drops content_for :subnavigation entirely: Show becomes
"Unlock + Back", Preview stays a plain link, metadata's own <details>
already replaced the old toggle, and Publish moves off this page for
good, per the earlier decision to manage the publish lifecycle
entirely from nodes#show. Save Draft and Save + Unlock + Exit appear
both above and below the form, given the body field alone runs 600px
on desktop.
| -rw-r--r-- | app/controllers/nodes_controller.rb | 34 | ||||
| -rw-r--r-- | config/routes.rb | 1 | ||||
| -rw-r--r-- | test/models/node_test.rb | 87 |
3 files changed, 112 insertions, 10 deletions
diff --git a/app/controllers/nodes_controller.rb b/app/controllers/nodes_controller.rb index 042963b..6fd000b 100644 --- a/app/controllers/nodes_controller.rb +++ b/app/controllers/nodes_controller.rb | |||
| @@ -12,7 +12,8 @@ class NodesController < ApplicationController | |||
| 12 | :destroy, | 12 | :destroy, |
| 13 | :publish, | 13 | :publish, |
| 14 | :unlock, | 14 | :unlock, |
| 15 | :autosave | 15 | :autosave, |
| 16 | :revert | ||
| 16 | ] | 17 | ] |
| 17 | 18 | ||
| 18 | def index | 19 | def index |
| @@ -59,16 +60,17 @@ class NodesController < ApplicationController | |||
| 59 | end | 60 | end |
| 60 | 61 | ||
| 61 | def edit | 62 | def edit |
| 62 | begin | 63 | @node.lock_for_editing!( current_user ) |
| 63 | @draft = @node.find_or_create_draft( current_user ) | 64 | @page = @node.autosave || @node.draft || @node.head |
| 64 | rescue LockedByAnotherUser => e | 65 | |
| 65 | flash[:error] = e.message | 66 | if @node.autosave |
| 66 | if request.referer | 67 | flash.now[:notice] = |
| 67 | redirect_to request.referer || node_path(@node) | 68 | "This page has unsaved changes from a previous session, shown below. " \ |
| 68 | else | 69 | "Save to keep them, or use \"Discard changes\" below to go back to the last saved version." |
| 69 | redirect_to node_path(@node) | ||
| 70 | end | ||
| 71 | end | 70 | end |
| 71 | rescue LockedByAnotherUser => e | ||
| 72 | flash[:error] = e.message | ||
| 73 | redirect_to(request.referer || node_path(@node)) | ||
| 72 | end | 74 | end |
| 73 | 75 | ||
| 74 | def update | 76 | def update |
| @@ -104,6 +106,18 @@ class NodesController < ApplicationController | |||
| 104 | render plain: "Autosave failed", status: :internal_server_error | 106 | render plain: "Autosave failed", status: :internal_server_error |
| 105 | end | 107 | end |
| 106 | 108 | ||
| 109 | def revert | ||
| 110 | @node.revert!(current_user) | ||
| 111 | if @node.draft | ||
| 112 | redirect_to edit_node_path(@node) | ||
| 113 | else | ||
| 114 | redirect_to node_path(@node) | ||
| 115 | end | ||
| 116 | rescue LockedByAnotherUser => e | ||
| 117 | flash[:error] = e.message | ||
| 118 | redirect_to node_path(@node) | ||
| 119 | end | ||
| 120 | |||
| 107 | def destroy | 121 | def destroy |
| 108 | @node.destroy | 122 | @node.destroy |
| 109 | end | 123 | end |
diff --git a/config/routes.rb b/config/routes.rb index 1569a15..da46e5c 100644 --- a/config/routes.rb +++ b/config/routes.rb | |||
| @@ -41,6 +41,7 @@ Cccms::Application.routes.draw do | |||
| 41 | put :generate_shared_preview | 41 | put :generate_shared_preview |
| 42 | put :revoke_shared_preview | 42 | put :revoke_shared_preview |
| 43 | put :autosave | 43 | put :autosave |
| 44 | put :revert | ||
| 44 | end | 45 | end |
| 45 | 46 | ||
| 46 | resources :revisions do | 47 | resources :revisions do |
diff --git a/test/models/node_test.rb b/test/models/node_test.rb index bdf556d..2138c19 100644 --- a/test/models/node_test.rb +++ b/test/models/node_test.rb | |||
| @@ -379,6 +379,93 @@ class NodeTest < ActiveSupport::TestCase | |||
| 379 | assert_nil node.autosave | 379 | assert_nil node.autosave |
| 380 | assert_nil node.lock_owner | 380 | assert_nil node.lock_owner |
| 381 | end | 381 | end |
| 382 | |||
| 383 | test "revert! is a safe no-op on a fresh node with only a draft" do | ||
| 384 | node = create_node_with_draft | ||
| 385 | node.lock_for_editing!(@user1) | ||
| 386 | |||
| 387 | node.revert!(@user1) | ||
| 388 | node.reload | ||
| 389 | |||
| 390 | assert_not_nil node.draft | ||
| 391 | assert_equal @user1, node.lock_owner | ||
| 392 | end | ||
| 393 | |||
| 394 | test "revert! discards an autosave on a fresh node without touching its only draft" do | ||
| 395 | node = create_node_with_draft | ||
| 396 | node.lock_for_editing!(@user1) | ||
| 397 | node.autosave!({ :title => "typing" }, @user1) | ||
| 398 | |||
| 399 | node.revert!(@user1) | ||
| 400 | node.reload | ||
| 401 | |||
| 402 | assert_nil node.autosave | ||
| 403 | assert_not_nil node.draft | ||
| 404 | assert_equal @user1, node.lock_owner | ||
| 405 | end | ||
| 406 | |||
| 407 | test "revert! does nothing when a published node has no draft or autosave" do | ||
| 408 | node = create_node_with_published_page | ||
| 409 | node.lock_for_editing!(@user1) | ||
| 410 | |||
| 411 | node.revert!(@user1) | ||
| 412 | node.reload | ||
| 413 | |||
| 414 | assert_not_nil node.head | ||
| 415 | assert_nil node.draft | ||
| 416 | end | ||
| 417 | |||
| 418 | test "revert! discards a fresh autosave and releases the lock when no draft exists" do | ||
| 419 | node = create_node_with_published_page | ||
| 420 | node.lock_for_editing!(@user1) | ||
| 421 | node.autosave!({ :title => "in progress" }, @user1) | ||
| 422 | |||
| 423 | node.revert!(@user1) | ||
| 424 | node.reload | ||
| 425 | |||
| 426 | assert_nil node.autosave | ||
| 427 | assert_nil node.draft | ||
| 428 | assert_nil node.lock_owner | ||
| 429 | end | ||
| 430 | |||
| 431 | test "revert! destroys an existing draft and releases the lock" do | ||
| 432 | node = create_node_with_published_page | ||
| 433 | head_title = node.head.title | ||
| 434 | node.lock_for_editing!(@user1) | ||
| 435 | node.autosave!({ :title => "second version" }, @user1) | ||
| 436 | node.save_draft!(@user1) | ||
| 437 | |||
| 438 | node.revert!(@user1) | ||
| 439 | node.reload | ||
| 440 | |||
| 441 | assert_nil node.draft | ||
| 442 | assert_equal head_title, node.head.title | ||
| 443 | assert_nil node.lock_owner | ||
| 444 | end | ||
| 445 | |||
| 446 | test "revert! discards only the autosave when a draft survives beneath it" do | ||
| 447 | node = create_node_with_published_page | ||
| 448 | node.lock_for_editing!(@user1) | ||
| 449 | node.autosave!({ :title => "second version" }, @user1) | ||
| 450 | node.save_draft!(@user1) | ||
| 451 | node.autosave!({ :title => "third version, still typing" }, @user1) | ||
| 452 | |||
| 453 | node.revert!(@user1) | ||
| 454 | node.reload | ||
| 455 | |||
| 456 | assert_nil node.autosave | ||
| 457 | assert_not_nil node.draft | ||
| 458 | assert_equal "second version", node.draft.title | ||
| 459 | assert_equal @user1, node.lock_owner | ||
| 460 | end | ||
| 461 | |||
| 462 | test "revert! raises LockedByAnotherUser for a non-owner" do | ||
| 463 | node = create_node_with_published_page | ||
| 464 | node.lock_for_editing!(@user1) | ||
| 465 | |||
| 466 | assert_raise(LockedByAnotherUser) { node.revert!(@user2) } | ||
| 467 | assert_equal @user1, node.reload.lock_owner | ||
| 468 | end | ||
| 382 | 469 | ||
| 383 | def create_revisions node, count | 470 | def create_revisions node, count |
| 384 | count.times do | 471 | count.times do |
