From cca0e57b21d506cd341b927984bcb68f0e461783 Mon Sep 17 00:00:00 2001 From: erdgeist Date: Tue, 14 Jul 2026 14:31:18 +0200 Subject: Retire wipe_draft! and find_or_create_draft from production code Both had already lost their reason to exist as production API: wipe_draft!'s one remaining callsite (nodes#show) was removed two sessions ago, and find_or_create_draft had zero production callers left at all -- confirmed by a fresh grep, not assumed -- every one of its ~65 call sites was test setup, unrelated to what those tests actually cover. wipe_draft! is deleted outright, along with its two tests -- the lock/draft/autosave cleanup it silently performed already has explicit, always-visible manual equivalents (Unlock, Discard Autosave, Destroy Draft), so nothing real is lost. find_or_create_draft moves to test_helper.rb as a plain method on ActiveSupport::TestCase, alongside the create_node_with_draft/ create_node_with_published_page helpers already living there -- extending the framework's own designated test-extension point rather than reopening the Node model from test code. Its three tests of real dispatch behavior (idempotency on repeat calls, and raising when a second user contends for the lock) are kept, since ~65 other tests depend on this helper actually working correctly; only the call syntax changed, from node.find_or_create_draft(user) to find_or_create_draft(node, user). --- test/models/event_test.rb | 2 +- test/models/node_test.rb | 63 ++++++++++++++--------------------------------- test/models/page_test.rb | 52 +++++++++++++++++++------------------- 3 files changed, 46 insertions(+), 71 deletions(-) (limited to 'test/models') diff --git a/test/models/event_test.rb b/test/models/event_test.rb index e98605e..2a671b7 100644 --- a/test/models/event_test.rb +++ b/test/models/event_test.rb @@ -5,7 +5,7 @@ class EventTest < ActiveSupport::TestCase def setup Page.delete_all @cal_node = Node.root.children.create! :slug => "calendar" - @draft = @cal_node.find_or_create_draft User.first + @draft = find_or_create_draft(@cal_node, User.first) @draft.title = "99C3" @draft.abstract = "The 99th Chaos Comunication Congress" @draft.body = "Its totally freakin awesome" diff --git a/test/models/node_test.rb b/test/models/node_test.rb index 91a4a6d..959387d 100644 --- a/test/models/node_test.rb +++ b/test/models/node_test.rb @@ -31,7 +31,7 @@ class NodeTest < ActiveSupport::TestCase assert_nil test_node.draft.user 3.times do - test_node.find_or_create_draft @user1 + find_or_create_draft(test_node, @user1) end assert_equal 1, test_node.pages.length @@ -40,7 +40,7 @@ class NodeTest < ActiveSupport::TestCase def test_user_gets_assigned_to_unlocked_draft assert_not_nil @first_child.draft assert_nil @first_child.draft.user - @first_child.find_or_create_draft @user1 + find_or_create_draft(@first_child, @user1) assert_equal @user1, @first_child.lock_owner end @@ -77,7 +77,7 @@ class NodeTest < ActiveSupport::TestCase @first_child.publish_draft! - draft1 = @first_child.find_or_create_draft(@user1) + draft1 = find_or_create_draft(@first_child, @user1) I18n.locale = :de assert_equal "Hallo", draft1.title @@ -109,15 +109,15 @@ class NodeTest < ActiveSupport::TestCase def test_find_or_create_draft_if_no_draft_exists node = Node.root.children.create :slug => "xyz" node.publish_draft! - assert_not_nil node.find_or_create_draft( @user1 ) + assert_not_nil find_or_create_draft(node, @user1) end def test_find_or_create_draft_if_draft_exists_and_is_owned_by_user node = Node.root.children.create :slug => "xyz" node.publish_draft! - first_call = node.find_or_create_draft @user1 - second_call = node.find_or_create_draft @user1 + first_call = find_or_create_draft(node, @user1) + second_call = find_or_create_draft(node, @user1) assert_equal first_call, second_call assert_equal 2, node.pages.count @@ -127,10 +127,10 @@ class NodeTest < ActiveSupport::TestCase def test_exception_if_draft_exists_but_locked_by_another_user node = Node.root.children.create :slug => "xyz" node.publish_draft! - node.find_or_create_draft @user1 + find_or_create_draft(node, @user1) assert_equal @user1, node.lock_owner assert_raise(LockedByAnotherUser) do - node.find_or_create_draft @user2 + find_or_create_draft(node, @user2) end end @@ -222,7 +222,7 @@ class NodeTest < ActiveSupport::TestCase assert_equal 1, test_node.pages.length assert_not_nil test_node.head assert_nil test_node.draft - test_node.find_or_create_draft @user1 + find_or_create_draft(test_node, @user1) test_node.reload assert_equal 2, test_node.pages.length assert_not_nil test_node.draft @@ -232,7 +232,7 @@ class NodeTest < ActiveSupport::TestCase test "restoring a revision" do test_node = Node.root.children.create! :slug => "test_node" create_revisions( test_node, 3 ) - test_node.find_or_create_draft @user1 + find_or_create_draft(test_node, @user1) test_node.reload assert_equal 4, test_node.pages.count @@ -250,7 +250,7 @@ class NodeTest < ActiveSupport::TestCase draft.user = users(:aaron) draft.save node.publish_draft! - new_draft = node.find_or_create_draft( users(:quentin) ) + new_draft = find_or_create_draft(node, users(:quentin)) assert_equal "aaron", new_draft.user.login end @@ -261,7 +261,7 @@ class NodeTest < ActiveSupport::TestCase draft.user = users(:aaron) draft.save! node.publish_draft! - new_draft = node.find_or_create_draft( users(:quentin) ) + new_draft = find_or_create_draft(node, users(:quentin)) new_draft.user_id = users(:quentin).id new_draft.save node.publish_draft! @@ -359,31 +359,6 @@ class NodeTest < ActiveSupport::TestCase assert_equal @user1, node.reload.lock_owner end - test "wipe_draft! leaves a fresh autosave and its lock untouched" do - node = create_node_with_published_page - node.lock_for_editing!(@user1) - node.autosave!({ :title => "still typing" }, @user1) - - node.wipe_draft! - node.reload - - assert_not_nil node.autosave - assert_equal @user1, node.lock_owner - end - - test "wipe_draft! destroys a stale, orphaned autosave and releases its lock" do - node = create_node_with_published_page - node.lock_for_editing!(@user1) - node.autosave!({ :title => "abandoned mid-session" }, @user1) - node.autosave.update_column(:updated_at, 2.days.ago) - - node.wipe_draft! - node.reload - - assert_nil node.autosave - assert_nil node.lock_owner - end - test "revert! is a safe no-op on a fresh node with only a draft" do node = create_node_with_draft node.lock_for_editing!(@user1) @@ -473,7 +448,7 @@ class NodeTest < ActiveSupport::TestCase def create_revisions node, count count.times do - node.find_or_create_draft @user1 + find_or_create_draft(node, @user1) node.publish_draft! end end @@ -514,7 +489,7 @@ class NodeTest < ActiveSupport::TestCase test "editor_search matches a partial substring, not just a whole word" do node = Node.root.children.create!(:slug => "editor_search_substring_test") - node.find_or_create_draft(@user1) + find_or_create_draft(node, @user1) node.draft.update(:title => "Biometrics Conference") node.publish_draft! @@ -530,7 +505,7 @@ class NodeTest < ActiveSupport::TestCase test "editor_search requires every word to match, but each word can match a different field" do node = Node.root.children.create!(:slug => "editor_search_multiword_test") - node.find_or_create_draft(@user1) + find_or_create_draft(node, @user1) node.draft.update(:title => "Backspace e.V. Bamberg", :abstract => "Spiegelgraben 41, 96052 Bamberg") node.publish_draft! @@ -540,9 +515,9 @@ class NodeTest < ActiveSupport::TestCase test "drafts_and_autosaves without a user sorts by recency only" do older = Node.root.children.create!(:slug => "drafts_order_older") - older.find_or_create_draft(@user1) + find_or_create_draft(older, @user1) newer = Node.root.children.create!(:slug => "drafts_order_newer") - newer.find_or_create_draft(@user1) + find_or_create_draft(newer, @user1) result = Node.drafts_and_autosaves.to_a assert result.index(newer) < result.index(older) @@ -564,7 +539,7 @@ class NodeTest < ActiveSupport::TestCase test "recently_changed includes a node whose head was recently published" do node = Node.root.children.create!(:slug => "recent_changed_published") - node.find_or_create_draft(@user1) + find_or_create_draft(node, @user1) node.autosave!({:title => "v1"}, @user1) node.save_draft!(@user1) node.publish_draft! @@ -574,7 +549,7 @@ class NodeTest < ActiveSupport::TestCase test "recently_changed excludes a node only touched by locking or unlocking after an old publish" do node = Node.root.children.create!(:slug => "recent_changed_lock_only") - node.find_or_create_draft(@user1) + find_or_create_draft(node, @user1) node.autosave!({:title => "v1"}, @user1) node.save_draft!(@user1) node.publish_draft! diff --git a/test/models/page_test.rb b/test/models/page_test.rb index 24bc7c5..edb7c37 100644 --- a/test/models/page_test.rb +++ b/test/models/page_test.rb @@ -13,15 +13,15 @@ class PageTest < ActiveSupport::TestCase n2 = Node.root.children.create! :slug => "two" # get the drafts and assign a user to it - assert_not_nil d1 = n1.find_or_create_draft( @user1 ) - assert_not_nil d3 = n2.find_or_create_draft( @user1 ) + assert_not_nil d1 = find_or_create_draft(n1, @user1) + assert_not_nil d3 = find_or_create_draft(n2, @user1) # tag and double publish so we have 4 pages tagged with "update" d1.tag_list = "update" d1.save n1.publish_draft! - d2 = n1.find_or_create_draft @user1 + d2 = find_or_create_draft(n1, @user1) n1.publish_draft! @@ -29,7 +29,7 @@ class PageTest < ActiveSupport::TestCase d3.save n2.publish_draft! - d4 = n2.find_or_create_draft @user1 + d4 = find_or_create_draft(n2, @user1) n2.publish_draft! # Set up two options hashes for the assertions @@ -49,7 +49,7 @@ class PageTest < ActiveSupport::TestCase def test_before_save_rewrite_links_in_body n = Node.root.children.create :slug => "link_test" - d = n.find_or_create_draft @user1 + d = find_or_create_draft(n, @user1) before = "

Hello World

\n" \ "Linkme" @@ -67,7 +67,7 @@ class PageTest < ActiveSupport::TestCase def test_before_save_rewrite_links_in_body_if_no_locale_prefix_present n = Node.root.children.create :slug => "link_test" - d = n.find_or_create_draft @user1 + d = find_or_create_draft(n, @user1) before = "

Hello World

\n" \ "Linkme" @@ -85,7 +85,7 @@ class PageTest < ActiveSupport::TestCase def test_before_save_rewrite_links_skips_on_external_links n = Node.root.children.create :slug => "link_test" - d = n.find_or_create_draft @user1 + d = find_or_create_draft(n, @user1) before = "

Hello World

\n" \ "Linkme" @@ -145,7 +145,7 @@ class PageTest < ActiveSupport::TestCase test "a page scheduled for future publication is not yet public even after being published" do node = Node.root.children.create!(slug: "preview-scheduled-test") - draft = node.find_or_create_draft(@user1) + draft = find_or_create_draft(node, @user1) draft.title = "Scheduled test" draft.published_at = 1.day.from_now draft.save! @@ -160,13 +160,13 @@ class PageTest < ActiveSupport::TestCase test "a superseded page is no longer the head, even though it was once published" do node = Node.root.children.create!(slug: "preview-superseded-test") - first_draft = node.find_or_create_draft(@user1) + first_draft = find_or_create_draft(node, @user1) first_draft.title = "First version" first_draft.save! first_token = first_draft.ensure_preview_token! node.publish_draft! - second_draft = node.find_or_create_draft(@user1) + second_draft = find_or_create_draft(node, @user1) second_draft.title = "Second version" second_draft.save! node.publish_draft! @@ -229,12 +229,12 @@ class PageTest < ActiveSupport::TestCase def test_diff_against_inline_keeps_tags_and_marks_only_the_changed_word n = Node.root.children.create! :slug => "diff_against_test" - d = n.find_or_create_draft @user1 + d = find_or_create_draft(n, @user1) d.title = "Old heading" d.save! n.publish_draft! - d2 = n.find_or_create_draft @user1 + d2 = find_or_create_draft(n, @user1) d2.title = "New heading" d2.save! @@ -246,12 +246,12 @@ class PageTest < ActiveSupport::TestCase def test_diff_against_side_by_side_returns_two_annotated_strings n = Node.root.children.create! :slug => "diff_against_sbs_test" - d = n.find_or_create_draft @user1 + d = find_or_create_draft(n, @user1) d.title = "Old heading" d.save! n.publish_draft! - d2 = n.find_or_create_draft @user1 + d2 = find_or_create_draft(n, @user1) d2.title = "New heading" d2.save! @@ -263,12 +263,12 @@ class PageTest < ActiveSupport::TestCase test "diff_against handles an inserted paragraph split without corrupting the document" do n = Node.root.children.create! :slug => "paragraph_split_test" - d = n.find_or_create_draft @user1 + d = find_or_create_draft(n, @user1) d.body = "

Der Vortragsraum ist ab 19 Uhr geöffnet, der Zugang erfolgt über den Hinterhof.

" d.save! n.publish_draft! - d2 = n.find_or_create_draft @user1 + d2 = find_or_create_draft(n, @user1) d2.body = "

Der Vortragsraum ist ab 19 Uhr geöffnet,

\n

der Zugang erfolgt über den Hinterhof.

" d2.save! @@ -281,13 +281,13 @@ class PageTest < ActiveSupport::TestCase test "diff_against reports tag and template changes" do n = Node.root.children.create! :slug => "field_diff_test" - d = n.find_or_create_draft @user1 + d = find_or_create_draft(n, @user1) d.tag_list = "update" d.template_name = "standard_template" d.save! n.publish_draft! - d2 = n.find_or_create_draft @user1 + d2 = find_or_create_draft(n, @user1) d2.tag_list = "update, pressemitteilung" d2.template_name = "title_only" d2.save! @@ -303,7 +303,7 @@ class PageTest < ActiveSupport::TestCase test "diff_against reports added and removed assets by filename" do n = Node.root.children.create! :slug => "asset_diff_test" - d = n.find_or_create_draft @user1 + d = find_or_create_draft(n, @user1) d.save! n.publish_draft! @@ -311,7 +311,7 @@ class PageTest < ActiveSupport::TestCase removed_asset = Asset.create!(:upload_file_name => "removed.pdf", :upload_content_type => "application/pdf", :upload_file_size => 1) n.head.update_assets([kept_asset.id, removed_asset.id]) - d2 = n.find_or_create_draft @user1 + d2 = find_or_create_draft(n, @user1) added_asset = Asset.create!(:upload_file_name => "added.png", :upload_content_type => "image/png", :upload_file_size => 1) d2.update_assets([kept_asset.id, added_asset.id]) d2.save! @@ -324,12 +324,12 @@ class PageTest < ActiveSupport::TestCase test "diff_against with an explicit locale compares that locale's own translation on each side" do n = Node.root.children.create!(:slug => "diff_locale_test") - d = n.find_or_create_draft(@user1) + d = find_or_create_draft(n, @user1) Globalize.with_locale(:en) { d.update!(:title => "Old English") } d.save! n.publish_draft! - d2 = n.find_or_create_draft(@user1) + d2 = find_or_create_draft(n, @user1) Globalize.with_locale(:en) { d2.update!(:title => "New English") } d2.save! @@ -341,11 +341,11 @@ class PageTest < ActiveSupport::TestCase test "diff_against with an explicit locale ignores content in other locales entirely" do n = Node.root.children.create!(:slug => "diff_locale_isolation_test") - d = n.find_or_create_draft(@user1) + d = find_or_create_draft(n, @user1) d.save! n.publish_draft! - d2 = n.find_or_create_draft(@user1) + d2 = find_or_create_draft(n, @user1) Globalize.with_locale(:de) { d2.update!(:title => "Nur Deutsch geändert") } d2.save! @@ -356,11 +356,11 @@ class PageTest < ActiveSupport::TestCase test "locale_diff_summary flags a locale that only exists on one side as changed" do n = Node.root.children.create!(:slug => "diff_locale_summary_test") - d = n.find_or_create_draft(@user1) + d = find_or_create_draft(n, @user1) d.save! n.publish_draft! - d2 = n.find_or_create_draft(@user1) + d2 = find_or_create_draft(n, @user1) Globalize.with_locale(:en) { d2.update!(:title => "New English translation") } d2.save! -- cgit v1.3