3 files changed, 80 insertions, 39 deletions

diff --git a/test/controllers/assets_controller_test.rb b/test/controllers/assets_controller_test.rb
index 467e1a68..aa3e00e6 100644
--- a/test/controllers/assets_controller_test.rb
+++ b/test/controllers/assets_controller_test.rb
@@ -94,7 +94,7 @@ class AssetsControllerTest < ActionController::TestCase
 
   # --- create with attach ---
 
-  test "create with node_id attaches the asset to the node's draft" do
+  test "create with node_id attaches the asset to an existing draft" do
     node = Node.root.children.create!(:slug => "asset_attach_target")
 
     post :create, params: { asset: { name: 'Attach me' }, node_id: node.id }
@@ -102,7 +102,21 @@ class AssetsControllerTest < ActionController::TestCase
     assert_response :redirect
     asset = Asset.last
     assert_includes node.draft.assets.reload, asset
-    assert_equal I18n.t("flash.assets.attached", :title => node.title), flash[:notice]
+    assert_equal I18n.t("flash.assets.attached_to_draft", :title => node.title), flash[:notice]
+  end
+
+  test "create with node_id creates a draft when none is pending" do
+    node = Node.root.children.create!(:slug => "asset_attach_no_draft")
+    node.publish_draft!(users(:quentin))
+    assert_nil node.reload.draft
+
+    post :create, params: { asset: { name: 'Attach me too' }, node_id: node.id }
+
+    node.reload
+    assert_includes node.draft.assets.reload, Asset.last
+    assert_empty node.head.assets.reload
+    assert_equal users(:quentin), node.draft.editor
+    assert_equal I18n.t("flash.assets.attached_new_draft", :title => node.title), flash[:notice]
   end
 
   test "create against a foreign-locked node keeps the asset but refuses the attach" do
@@ -133,14 +147,16 @@ class AssetsControllerTest < ActionController::TestCase
     assert_equal node_path(node), flash[:headline_kept_path]
   end
 
-  test "create with node_id writes an asset_create and an asset_attach entry" do
-    node = Node.root.children.create!(:slug => "asset_log_pair")
-    assert_difference 'NodeAction.where(:action => "asset_create").count' do
-      assert_difference 'NodeAction.where(:action => "asset_attach").count' do
-        post :create, params: { asset: { name: 'Logged twice' }, node_id: node.id }
-      end
+  test "create with node_id writes only an asset_create entry" do
+    node = Node.root.children.create!(:slug => "asset_log_single")
+
+    assert_difference 'NodeAction.count', 1 do
+      post :create, params: { asset: { name: 'Logged once' }, node_id: node.id }
     end
-    assert_equal users(:quentin), NodeAction.last.user
+
+    action = NodeAction.last
+    assert_equal "asset_create", action.action
+    assert_equal users(:quentin), action.user
   end
 
   # --- edit ---
diff --git a/test/controllers/content_controller_test.rb b/test/controllers/content_controller_test.rb
index 39fe276b..482c1ddf 100644
--- a/test/controllers/content_controller_test.rb
+++ b/test/controllers/content_controller_test.rb
@@ -170,6 +170,8 @@ class ContentControllerTest < ActionController::TestCase
                           :upload_content_type => "image/png",
                           :upload_updated_at => Time.at(1_700_000_000))
     node.attach_asset!(asset, :user => @user1, :headline => true)
+    node.publish_draft!
+    node.reload
 
     # has_variant? only tests File.exist?, so touching the path is enough
     # and no ImageMagick runs in the suite. image/png takes .jpg for the
diff --git a/test/models/node_attach_asset_test.rb b/test/models/node_attach_asset_test.rb
index 2df2cfbb..9ab38158 100644
--- a/test/models/node_attach_asset_test.rb
+++ b/test/models/node_attach_asset_test.rb
@@ -9,49 +9,67 @@ class NodeAttachAssetTest < ActiveSupport::TestCase
     @image = create_image_asset
   end
 
-  test "attaches to a draft-only node" do
+  test "attaches to an existing draft" do
     result = @node.attach_asset!(@image, :user => @user)
     assert_equal 1, result[:attached]
+    assert_not result[:draft_created]
     assert_includes @node.draft.assets, @image
   end
 
-  test "attaches to head when no draft is pending" do
+  test "creates a draft when none is pending and leaves head untouched" do
     @node.publish_draft!(@user)
     result = @node.attach_asset!(@image, :user => @user)
     assert_equal 1, result[:attached]
-    assert_includes @node.head.assets, @image
+    assert result[:draft_created]
+    assert_includes @node.draft.assets, @image
+    assert_empty @node.head.assets.reload
   end
 
-  test "attaches to head and pending draft alike" do
+  test "attaches to a pending draft and leaves head untouched" do
     @node.publish_draft!(@user)
     @node.lock_for_editing!(@user)
     @node.create_new_draft(@user)
     result = @node.attach_asset!(@image, :user => @user)
-    assert_equal 2, result[:attached]
-    assert_includes @node.head.assets, @image
+    assert_equal 1, result[:attached]
+    assert_not result[:draft_created]
     assert_includes @node.draft.assets, @image
+    assert_empty @node.head.assets.reload
   end
 
-  test "attaches to all three lifecycle rows" do
+  test "refuses when an autosave exists and writes nothing" do
     @node.publish_draft!(@user)
     @node.lock_for_editing!(@user)
     @node.create_new_draft(@user)
     @node.autosave!({ :title => "wip" }, @user)
+    assert_raises(ActiveRecord::RecordInvalid) { @node.attach_asset!(@image, :user => @user) }
+    assert_empty @node.draft.assets.reload
+    assert_empty @node.autosave.assets.reload
+  end
+
+  test "reports an asset the draft already carries without duplicating it" do
+    @node.draft.related_assets.create!(:asset => @image)
     result = @node.attach_asset!(@image, :user => @user)
-    assert_equal 3, result[:attached]
-    [@node.head, @node.draft, @node.autosave].each do |row|
-      assert_includes row.assets, @image
-    end
+    assert_equal 0, result[:attached]
+    assert_equal 1, result[:already]
+    assert_not result[:draft_created]
+    assert_equal 1, @node.draft.related_assets.where(:asset_id => @image.id).count
   end
 
-  test "skips rows that already carry the asset" do
+  test "creates no draft when head already carries the asset" do
     @node.draft.related_assets.create!(:asset => @image)
     @node.publish_draft!(@user)
-    @node.lock_for_editing!(@user)
-    @node.create_new_draft(@user)
+    assert_nil @node.draft
+
+    result = @node.attach_asset!(@image, :user => @user)
+    assert_equal 0, result[:attached]
+    assert_not result[:draft_created]
+    assert_nil @node.reload.draft
+  end
+
+  test "attaching twice leaves one join row" do
+    @node.attach_asset!(@image, :user => @user)
     result = @node.attach_asset!(@image, :user => @user)
     assert_equal 0, result[:attached]
-    assert_equal 2, result[:already]
     assert_equal 1, @node.draft.related_assets.where(:asset_id => @image.id).count
   end
 
@@ -82,6 +100,17 @@ class NodeAttachAssetTest < ActiveSupport::TestCase
     assert_includes @node.draft.assets, @image
   end
 
+  test "keeps a headline the new draft inherited from head" do
+    incumbent = create_image_asset
+    @node.draft.related_assets.create!(:asset => incumbent, :headline => true)
+    @node.publish_draft!(@user)
+
+    result = @node.attach_asset!(@image, :user => @user, :headline => true)
+    assert result[:draft_created]
+    assert_equal :kept_existing, result[:headline]
+    assert_equal incumbent, @node.draft.reload.headline_asset
+  end
+
   test "declines the headline flag for ineligible asset types" do
     plain = create_plain_asset
     result = @node.attach_asset!(plain, :user => @user, :headline => true)
@@ -95,25 +124,19 @@ class NodeAttachAssetTest < ActiveSupport::TestCase
     assert_raises(ActiveRecord::RecordInvalid) { @node.attach_asset!(@image, :user => @user) }
   end
 
-  test "attaching writes an asset_attach entry with node and asset participants" do
-    result = @node.attach_asset!(@image, :user => @user, :headline => true)
-    assert_equal :set, result[:headline]
-
-    action = NodeAction.where(:action => "asset_attach").last
-    assert_equal @node, action.node
-    subjects = action.action_participants.map { |p| [p.subject_type, p.subject_id] }
-    assert_includes subjects, ["Node", @node.id]
-    assert_includes subjects, ["Asset", @image.id]
-    assert action.metadata["headline"]
-  end
-
-  test "a fully redundant attach writes no entry" do
-    @node.attach_asset!(@image, :user => @user)
-    assert_no_difference 'NodeAction.count' do
+  test "attaching writes no log entry -- publish carries the witnessing" do
+    assert_no_difference "NodeAction.count" do
       @node.attach_asset!(@image, :user => @user)
     end
   end
 
+  test "attaching under a restricted surface needs no redaktion role" do
+    updates = Node.root.children.create!(:slug => "updates")
+    node    = updates.children.create!(:slug => "gated-attachment")
+    result  = node.reload.attach_asset!(@image, :user => @user)
+    assert_equal 1, result[:attached]
+  end
+
   private
 
     def create_image_asset