Asset picker: attach/detach/reorder UI, read-only view, autosave fix

Replaces nodes#edit's old Images section -- a hidden panel dumping
every image asset in the system unfiltered (#image_browser) plus a
raw drag-and-drop box (#image_box) -- with a small search-and-click
picker built on the endpoint from the last two commits. Attaching
posts immediately and appends the new thumbnail via a cloned
<template> -- icons only render correctly through the Rails helper
server-side, so the template holds real, pre-rendered markup for JS
to clone rather than duplicating raw SVG in a JS string. Reordering is
jQuery UI sortable on the small attached list only, with a dedicated
drag handle rather than the whole thumbnail.

Two bugs caught while click-testing, fixed here rather than shipped
and patched after: the search panel never closed after attaching an
image, since the success handler re-triggered focus to keep it open
for attaching several in a row -- which meant it just re-populated
itself forever instead of signaling "done." Fixed to close explicitly;
a click-outside-closes handler was added alongside it, matching the
affordance the top-bar search already has.

A real, independent, pre-existing data bug surfaced during the same
testing: Node#autosave!'s first-time-creation branch never carried
related assets forward from whatever page was previously current --
attach an image, let autosave fire once, and it silently landed on a
fresh, assetless Page row. Long-dormant, not introduced by this work,
just finally exercised by something that made it visible. Fixed inside
the `unless self.autosave` guard specifically -- running this on every
call, not just creation, would overwrite anything attached directly to
an existing autosave in between, a worse bug than the one being fixed.

nodes#show gains a read-only Images section, rendered only when a page
actually has attached images, so an attachment can be confirmed
present without entering the edit/lock cycle -- useful on its own, and
specifically useful the next time an asset bug needs investigating.
Its thumbnail CSS is shared with the edit view's picker via a class
(.thumbnail_list) rather than duplicated under a second name.

10 files changed, 230 insertions, 15 deletions

diff --git a/app/controllers/related_assets_controller.rb b/app/controllers/related_assets_controller.rb
index ae37d2f..479ebec 100644
--- a/app/controllers/related_assets_controller.rb
+++ b/app/controllers/related_assets_controller.rb
@@ -26,7 +26,8 @@ class RelatedAssetsController < ApplicationController
       id: related.id,
       asset_id: asset.id,
       name: asset.name,
-      thumb_url: asset.upload.url(:thumb)
+      thumb_url: asset.upload.url(:thumb),
+      url: node_related_asset_path(@node, related)
     }
   end
 
diff --git a/app/models/node.rb b/app/models/node.rb
index 0361c1e..6f435ee 100644
--- a/app/models/node.rb
+++ b/app/models/node.rb
@@ -80,6 +80,7 @@ class Node < ApplicationRecord
 
     unless self.autosave
       self.autosave = Page.create!(:editor => current_user)
+      self.autosave.assets = (self.draft || self.head).assets if self.draft || self.head
       self.save!
     end
     self.autosave.assign_attributes(attributes)
diff --git a/app/views/layouts/admin.html.erb b/app/views/layouts/admin.html.erb
index 5ae363f..8bbc0d5 100644
--- a/app/views/layouts/admin.html.erb
+++ b/app/views/layouts/admin.html.erb
@@ -13,6 +13,7 @@
     <link rel="stylesheet" href="/stylesheets/admin.css">
     <script src="/javascripts/admin_search.js"></script>
     <script src="/javascripts/admin_interface.js"></script>
+    <script src="/javascripts/related_assets.js"></script>
     <script>
       var ADMIN_SEARCH_URL = "<%= admin_search_path %>";
       var ADMIN_MENU_SEARCH_URL = "<%= admin_menu_search_path %>";
diff --git a/app/views/nodes/edit.html.erb b/app/views/nodes/edit.html.erb
index 1ac05e0..e0e51d8 100644
--- a/app/views/nodes/edit.html.erb
+++ b/app/views/nodes/edit.html.erb
@@ -76,24 +76,35 @@
 
       <div class="node_description">Images</div>
       <div class="node_content">
-        <ul id="image_box" rel="<%= @page.id %>">
-          <% @page.assets.images.each do |image| %>
-            <li rel="images_<%= image.id %>">
-              <%= image_tag(image.upload.url(:thumb)) %>
-            </li>
-          <% end %>
-        </ul>
-        <div class="clear_left right">
-          <a id="image_browser_toggle" class="unselected" href="#">image browser</a>
-        </div>
-        <div id="image_browser">
-          <ul>
-            <% Asset.images.each do |image| %>
-            <li rel="images_<%= image.id %>"><%= image_tag(image.upload.url(:thumb)) %></li>
+        <div id="related_assets"
+             data-search-url="<%= search_node_related_assets_path(@node) %>"
+             data-create-url="<%= node_related_assets_path(@node) %>">
+          <ul id="related_asset_list" class="thumbnail_list">
+            <% @page.related_assets.includes(:asset).each do |related| %>
+              <li data-url="<%= node_related_asset_path(@node, related) %>">
+                <span class="related_asset_handle"><%= icon("grip-vertical", library: "tabler", "aria-hidden": true) %></span>
+                <%= image_tag related.asset.upload.url(:thumb) %>
+                <button type="button" class="related_asset_remove" aria-label="Remove image">
+                  <%= icon("x", library: "tabler", "aria-hidden": true) %>
+                </button>
+              </li>
             <% end %>
           </ul>
+          <%= text_field_tag nil, nil, id: "related_asset_search_term", placeholder: "Search images to attach…", autocomplete: "off" %>
+          <div id="related_asset_search_results" class="search_results"></div>
         </div>
       </div>
+
+      <template id="related_asset_template">
+        <li>
+          <span class="related_asset_handle"><%= icon("grip-vertical", library: "tabler", "aria-hidden": true) %></span>
+          <img src="">
+          <button type="button" class="related_asset_remove" aria-label="Remove image">
+            <%= icon("x", library: "tabler", "aria-hidden": true) %>
+          </button>
+        </li>
+      </template>
+
     </div>
   </details>
 
diff --git a/app/views/nodes/show.html.erb b/app/views/nodes/show.html.erb
index 280e492..8ea03cd 100644
--- a/app/views/nodes/show.html.erb
+++ b/app/views/nodes/show.html.erb
@@ -152,6 +152,17 @@
     <div class="node_description">Tags</div>
     <div class="node_content node_info_group"><%= @page.tag_list %></div>
 
+    <% if @page.assets.images.any? %>
+      <div class="node_description">Images</div>
+      <div class="node_content node_info_group">
+        <ul class="thumbnail_list">
+          <% @page.assets.images.each do |asset| %>
+            <li><%= link_to image_tag(asset.upload.url(:thumb)), asset_path(asset) %></li>
+          <% end %>
+        </ul>
+      </div>
+    <% end %>
+
     <div class="node_description">Events</div>
     <div class="node_content node_info_group">
       <ul>
diff --git a/public/javascripts/admin_interface.js b/public/javascripts/admin_interface.js
index 3f6a0a9..619933d 100644
--- a/public/javascripts/admin_interface.js
+++ b/public/javascripts/admin_interface.js
@@ -59,6 +59,10 @@ $(document).ready(function () {
     event_search.initialize_search();
   }
 
+  if ($("#related_asset_search_term").length != 0) {
+    related_assets.initialize();
+  }
+
   if ($("#rrule_builder").length != 0) {
     rrule_builder.initialize();
   }
diff --git a/public/javascripts/related_assets.js b/public/javascripts/related_assets.js
new file mode 100644
index 0000000..2955cbf
--- /dev/null
+++ b/public/javascripts/related_assets.js
@@ -0,0 +1,80 @@
+related_assets = {
+  initialize: function() {
+    var container = $("#related_assets");
+    var searchUrl = container.data("search-url");
+    var createUrl = container.data("create-url");
+    var list = $("#related_asset_list");
+
+    initSearchPicker({
+      inputSelector: "#related_asset_search_term",
+      resultsSelector: "#related_asset_search_results",
+      url: searchUrl,
+      loadOnFocus: true,
+      renderResults: function(data, results) {
+        var found = false;
+        data.forEach(function(asset) {
+          var item = $(
+            "<a href='#' class='related_asset_result'>" +
+              "<img src='" + asset.thumb_url + "' alt=''>" +
+              "<span>" + asset.name + "</span>" +
+            "</a>"
+          );
+          item.bind("click", function() {
+            related_assets.attach(asset.id, createUrl, list);
+            return false;
+          });
+          results.append(item);
+          found = true;
+        });
+        return found;
+      }
+    });
+
+    $(document).on("click", function(e) {
+      if (!$(e.target).closest("#related_assets").length) {
+        $("#related_asset_search_results").slideUp().empty();
+      }
+    });
+
+    list.sortable({
+      handle: ".related_asset_handle",
+      update: function(event, ui) {
+        var newPosition = ui.item.index() + 1; // acts_as_list is 1-based; jQuery UI's index() is 0-based
+        $.ajax({
+          type: "PATCH",
+          url: ui.item.data("url"),
+          data: "position=" + newPosition
+        });
+      }
+    });
+
+    list.on("click", ".related_asset_remove", function() {
+      var item = $(this).closest("li");
+      $.ajax({
+        type: "DELETE",
+        url: item.data("url"),
+        success: function() {
+          item.remove();
+        }
+      });
+    });
+  },
+
+  attach: function(assetId, createUrl, list) {
+    $.ajax({
+      type: "POST",
+      url: createUrl,
+      data: "asset_id=" + assetId,
+      dataType: "json",
+      success: function(related) {
+        var item = $($("#related_asset_template").html().trim());
+        item.attr("data-url", related.url);
+        item.find("img").attr("src", related.thumb_url);
+        list.append(item);
+        list.sortable("refresh");
+        $("#related_asset_search_term").val("");
+        $("#related_asset_search_results").slideUp().empty();
+      }
+    });
+  }
+};
diff --git a/public/stylesheets/admin.css b/public/stylesheets/admin.css
index a58a061..c3c0cb4 100644
--- a/public/stylesheets/admin.css
+++ b/public/stylesheets/admin.css
@@ -1210,3 +1210,78 @@ div#image_browser {
 div#image_browser ul li {
   list-style-type: none;
 }
+
+.thumbnail_list {
+  display: flex;
+  flex-wrap: wrap;
+  gap: 0.75rem;
+  list-style: none;
+  margin: 0 0 0.75rem;
+  padding: 0;
+}
+
+.thumbnail_list li {
+  position: relative;
+  display: flex;
+  align-items: center;
+  gap: 0.25rem;
+  border: 1px solid #e8e8e8;
+  border-radius: 6px;
+  padding: 4px;
+}
+
+.thumbnail_list img {
+  display: block;
+  width: 60px;
+  height: 60px;
+  object-fit: cover;
+  border-radius: 4px;
+}
+
+.related_asset_handle {
+  display: flex;
+  color: #969696;
+  cursor: grab;
+}
+
+.related_asset_handle svg {
+  width: 1rem;
+  height: 1rem;
+}
+
+.related_asset_remove {
+  display: flex;
+  align-items: center;
+  justify-content: center;
+  background: none;
+  border: none;
+  padding: 2px;
+  cursor: pointer;
+  color: #969696;
+}
+
+.related_asset_remove svg {
+  width: 0.9rem;
+  height: 0.9rem;
+}
+
+.related_asset_remove:hover {
+  color: #cc0000;
+}
+
+.related_asset_result {
+  display: flex;
+  align-items: center;
+  gap: 0.5rem;
+  padding: 4px;
+  border-bottom: 1px solid #e8e8e8;
+  text-decoration: none;
+}
+
+.related_asset_result img {
+  width: 40px;
+  height: 40px;
+  object-fit: cover;
+  border-radius: 4px;
+}
+
diff --git a/test/controllers/related_assets_controller_test.rb b/test/controllers/related_assets_controller_test.rb
index 7f6d487..9d20cbb 100644
--- a/test/controllers/related_assets_controller_test.rb
+++ b/test/controllers/related_assets_controller_test.rb
@@ -44,6 +44,8 @@ class RelatedAssetsControllerTest < ActionController::TestCase
 
     assert_response :success
     assert_includes node.draft.reload.related_assets.map(&:asset_id), asset.id
+    json = JSON.parse(response.body)
+    assert json["url"].present?
   end
 
   test "create does not duplicate an already-attached asset" do
diff --git a/test/models/node_test.rb b/test/models/node_test.rb
index 0bce222..a1da82c 100644
--- a/test/models/node_test.rb
+++ b/test/models/node_test.rb
@@ -585,4 +585,33 @@ class NodeTest < ActiveSupport::TestCase
 
     assert_not_includes Node.recently_changed, node
   end
+
+  test "autosave! carries over the current related assets to the newly created autosave row" do
+    node = Node.root.children.create!(:slug => "autosave_asset_carryover_test")
+    user = User.find_by_login("quentin")
+    asset = Asset.create!(:name => "carryover-photo", :upload_content_type => "image/png")
+    node.draft.assets << asset
+
+    node.lock_for_editing!(user)
+    node.autosave!({:title => "v1"}, user)
+
+    assert_includes node.autosave.reload.assets, asset
+  end
+
+  test "autosave! does not reset assets already attached directly to an existing autosave" do
+    node = Node.root.children.create!(:slug => "autosave_asset_no_reset_test")
+    user = User.find_by_login("quentin")
+    original = Asset.create!(:name => "original-photo", :upload_content_type => "image/png")
+    extra = Asset.create!(:name => "extra-photo", :upload_content_type => "image/png")
+    node.draft.assets << original
+
+    node.lock_for_editing!(user)
+    node.autosave!({:title => "v1"}, user)
+    node.autosave.assets << extra
+
+    node.autosave!({:title => "v2"}, user)
+
+    assert_includes node.autosave.reload.assets, original
+    assert_includes node.autosave.reload.assets, extra
+  end
 end