3 files changed, 55 insertions, 19 deletions

diff --git a/app/controllers/related_assets_controller.rb b/app/controllers/related_assets_controller.rb
index ca894f2f..3aa6d3db 100644
--- a/app/controllers/related_assets_controller.rb
+++ b/app/controllers/related_assets_controller.rb
@@ -2,6 +2,10 @@ class RelatedAssetsController < ApplicationController
   before_action :login_required
   before_action :find_node
 
+  rescue_from LockedByAnotherUser do
+    head :locked
+  end
+
   def search
     term = params[:search_term].to_s.strip
     attached_ids = @node.editable_page.related_assets.pluck(:asset_id)
@@ -19,8 +23,9 @@ class RelatedAssetsController < ApplicationController
   end
 
   def create
+    page  = @node.ensure_autosave!(current_user)
     asset = Asset.find(params[:asset_id])
-    related = @node.editable_page.related_assets.find_or_create_by!(asset: asset)
+    related = page.related_assets.find_or_create_by!(asset: asset)
 
     render json: {
       id: related.id,
@@ -35,22 +40,25 @@ class RelatedAssetsController < ApplicationController
   end
 
   def destroy
-    @node.editable_page.related_assets.find(params[:id]).destroy
+    page   = @node.ensure_autosave!(current_user)
+    source = RelatedAsset.find(params[:id])
+    page.related_assets.find_by!(:asset_id => source.asset_id).destroy
     head :ok
   end
 
   def update
-    related = @node.editable_page.related_assets.find(params[:id])
+    page    = @node.ensure_autosave!(current_user)
+    source  = RelatedAsset.find(params[:id])
+    related = page.related_assets.find_by!(:asset_id => source.asset_id)
 
     if params.key?(:headline)
       RelatedAsset.transaction do
-        @node.editable_page.related_assets.update_all(headline: false)
+        page.related_assets.update_all(headline: false)
         related.update!(headline: true) if params[:headline] == "true"
       end
     else
       related.insert_at(params[:position].to_i)
     end
-
     head :ok
   end
 
diff --git a/app/models/node.rb b/app/models/node.rb
index 274b2f94..02502f6c 100644
--- a/app/models/node.rb
+++ b/app/models/node.rb
@@ -112,18 +112,18 @@ class Node < ApplicationRecord
     end
   end
 
-  # Creates or updates the autosave buffer from the given attributes.
-  # Autosave rows are never associated to the node via node_id -- they
-  # must never appear in self.pages / the revisions list, which is the
-  # whole reason autosave exists as a separate, unversioned layer.
-  def autosave! attributes, current_user
+  def ensure_autosave! current_user
     assert_locked_by! current_user
-
     unless self.autosave
       self.autosave = Page.create!(:editor => current_user)
       self.autosave.clone_attributes_from(self.draft || self.head) if self.draft || self.head
       self.save!
     end
+    self.autosave
+  end
+
+  def autosave! attributes, current_user
+    ensure_autosave!(current_user)
     self.autosave.assign_attributes(attributes)
     self.autosave.save!
     self.autosave
diff --git a/test/controllers/related_assets_controller_test.rb b/test/controllers/related_assets_controller_test.rb
index ced4b74d..fd30dddd 100644
--- a/test/controllers/related_assets_controller_test.rb
+++ b/test/controllers/related_assets_controller_test.rb
@@ -38,19 +38,24 @@ class RelatedAssetsControllerTest < ActionController::TestCase
   test "create attaches an asset to the node's editable page" do
     login_as :quentin
     node = Node.root.children.create!(:slug => "related_assets_create_test")
+    node.lock_for_editing!(users(:quentin))
     asset = Asset.create!(:name => "erfa-photo", :upload_content_type => "image/png")
 
     post :create, params: { :node_id => node.id, :asset_id => asset.id }
 
     assert_response :success
-    assert_includes node.draft.reload.related_assets.map(&:asset_id), asset.id
+    current = node.reload.editable_page
+    assert_equal node.autosave, current, "mutation should have created an autosave layer"
+    assert_includes current.related_assets.map(&:asset_id), asset.id
     json = JSON.parse(response.body)
     assert json["url"].present?
+    assert_empty node.draft.reload.related_assets, "the draft must stay untouched"
   end
 
   test "create does not duplicate an already-attached asset" do
     login_as :quentin
     node = Node.root.children.create!(:slug => "related_assets_dup_test")
+    node.lock_for_editing!(users(:quentin))
     asset = Asset.create!(:name => "erfa-photo-2", :upload_content_type => "image/png")
     node.draft.assets << asset
 
@@ -63,19 +68,22 @@ class RelatedAssetsControllerTest < ActionController::TestCase
   test "destroy removes the attached asset" do
     login_as :quentin
     node = Node.root.children.create!(:slug => "related_assets_destroy_test")
+    node.lock_for_editing!(users(:quentin))
     asset = Asset.create!(:name => "old-photo", :upload_content_type => "image/png")
     node.draft.assets << asset
-    related = node.draft.related_assets.first
 
+    related = node.draft.related_assets.first
     delete :destroy, params: { :node_id => node.id, :id => related.id }
 
     assert_response :success
-    assert_equal 0, node.draft.reload.related_assets.count
+    assert_equal 0, node.reload.editable_page.related_assets.count
+    assert_equal 1, node.draft.reload.related_assets.count, "the draft must stay untouched"
   end
 
   test "update reorders the attached assets" do
     login_as :quentin
     node = Node.root.children.create!(:slug => "related_assets_reorder_test")
+    node.lock_for_editing!(users(:quentin))
     first  = Asset.create!(:name => "first-photo", :upload_content_type => "image/png")
     second = Asset.create!(:name => "second-photo", :upload_content_type => "image/png")
     node.draft.assets << first
@@ -85,13 +93,15 @@ class RelatedAssetsControllerTest < ActionController::TestCase
     patch :update, params: { :node_id => node.id, :id => second_related.id, :position => 1 }
 
     assert_response :success
-    ordered_asset_ids = node.draft.reload.related_assets.map(&:asset_id)
+    ordered_asset_ids = node.reload.editable_page.related_assets.order(:position).map(&:asset_id)
+    # XXXX ordered_asset_ids = node.draft.reload.related_assets.map(&:asset_id)
     assert_equal [second.id, first.id], ordered_asset_ids
   end
 
   test "update sets the headline flag" do
     login_as :quentin
     node = Node.root.children.create!(:slug => "related_assets_headline_test")
+    node.lock_for_editing!(users(:quentin))
     asset = Asset.create!(:name => "headline-photo", :upload_content_type => "image/png")
     node.draft.assets << asset
     related = node.draft.related_assets.find_by(:asset_id => asset.id)
@@ -99,12 +109,14 @@ class RelatedAssetsControllerTest < ActionController::TestCase
     patch :update, params: { :node_id => node.id, :id => related.id, :headline => "true" }
 
     assert_response :success
-    assert related.reload.headline?
+    assert node.reload.editable_page.related_assets.find_by!(:asset_id => asset.id).headline?
+    assert_not related.reload.headline?, "the draft must stay untouched"
   end
 
   test "update with headline=true clears any previous headline on the same page" do
     login_as :quentin
     node = Node.root.children.create!(:slug => "related_assets_headline_swap_test")
+    node.lock_for_editing!(users(:quentin))
     first  = Asset.create!(:name => "first-headline", :upload_content_type => "image/png")
     second = Asset.create!(:name => "second-headline", :upload_content_type => "image/png")
     node.draft.assets << first
@@ -117,13 +129,16 @@ class RelatedAssetsControllerTest < ActionController::TestCase
     patch :update, params: { :node_id => node.id, :id => second_related.id, :headline => "true" }
 
     assert_response :success
-    assert_not first_related.reload.headline?
-    assert second_related.reload.headline?
+    current = node.reload.editable_page
+    assert_not current.related_assets.find_by!(:asset_id => first.id).headline?
+    assert current.related_assets.find_by!(:asset_id => second.id).headline?
+    assert first_related.reload.headline?, "the draft must stay untouched"
   end
 
   test "update with headline=false clears the headline" do
     login_as :quentin
     node = Node.root.children.create!(:slug => "related_assets_headline_unset_test")
+    node.lock_for_editing!(users(:quentin))
     asset = Asset.create!(:name => "unset-headline", :upload_content_type => "image/png")
     node.draft.assets << asset
     related = node.draft.related_assets.find_by(:asset_id => asset.id)
@@ -132,7 +147,8 @@ class RelatedAssetsControllerTest < ActionController::TestCase
     patch :update, params: { :node_id => node.id, :id => related.id, :headline => "false" }
 
     assert_response :success
-    assert_not related.reload.headline?
+    assert_not node.reload.editable_page.related_assets.find_by!(:asset_id => asset.id).headline?
+    assert related.reload.headline?, "the draft must stay untouched"
   end
 
   test "search includes PDF assets as headline-eligible candidates" do
@@ -159,4 +175,16 @@ class RelatedAssetsControllerTest < ActionController::TestCase
     ids = JSON.parse(response.body).map { |r| r["id"] }
     assert_includes ids, asset.id
   end
+
+  test "curation without holding the lock is refused with 423" do
+    login_as :quentin
+    node  = Node.root.children.create!(:slug => "curation_lock_test")
+    asset = Asset.create!(:name => "Untouchable", :upload_content_type => "image/png")
+    node.lock_for_editing!(users(:aaron))
+
+    post :create, params: { :node_id => node.id, :asset_id => asset.id }
+
+    assert_response :locked
+    assert_empty node.draft.assets.reload
+  end
 end