8 files changed, 25 insertions, 36 deletions

diff --git a/app/controllers/nodes_controller.rb b/app/controllers/nodes_controller.rb
index 3009e25a..ef47f258 100644
--- a/app/controllers/nodes_controller.rb
+++ b/app/controllers/nodes_controller.rb
@@ -281,7 +281,8 @@ class NodesController < ApplicationController
 
     def page_params
       params.fetch(:page, {}).permit(:title, :abstract, :body, :template_name,
-                                 :published_at, :user_id, :slug, :parent_node_id)
+                                 :published_at, :user_id, :slug, :parent_node_id,
+                                 :external_url)
     end
 
     def find_node
diff --git a/app/helpers/node_actions_helper.rb b/app/helpers/node_actions_helper.rb
index 19385543..f21fee61 100644
--- a/app/helpers/node_actions_helper.rb
+++ b/app/helpers/node_actions_helper.rb
@@ -28,8 +28,7 @@ module NodeActionsHelper
     "admin_revoke"       => "shield-minus",
     "event_create"       => "calendar-plus",
     "event_update"       => "calendar-event",
-    "event_destroy"      => "calendar-x",
-    "node_external_url"  => "world"
+    "event_destroy"      => "calendar-x"
   }.freeze
 
   def verb_icon action
@@ -67,7 +66,7 @@ module NodeActionsHelper
     return true if m["title"].is_a?(Hash) && m.dig("title", "from") != m.dig("title", "to")
     return true if m["external_url"].present?
     %w[author tags template_changed assets assets_changed assets_reordered
-       abstract_changed body_changed].any? { |key| m[key].present? }
+       abstract_changed body_changed external_url].any? { |key| m[key].present? }
   end
 
   def default_locale_changes action
@@ -95,6 +94,11 @@ module NodeActionsHelper
                     :names => linked_asset_names(action, names)).html_safe
       end
     end
+    if m["external_url"]
+      items << t("node_actions.detail_external_url",
+                  :from => m.dig("external_url", "from").presence || t("node_actions.event_none"),
+                  :to   => m.dig("external_url", "to").presence   || t("node_actions.event_none"))
+    end
     items << t("node_actions.assets_reordered") if m["assets_reordered"]
     items << t("node_actions.assets_changed")   if m["assets_changed"]
     items << t("node_actions.detail_external_url",
@@ -405,11 +409,6 @@ module NodeActionsHelper
     event_sentence(action, "event_destroy")
   end
 
-  def summarize_node_external_url action
-    t("node_actions.node_external_url", :actor => actor_ref(action),
-       :subject => subject_ref(action)).html_safe
-  end
-
   def event_sentence action, key
     if action.node
       t("node_actions.#{key}_on", :actor => actor_ref(action),
diff --git a/app/models/node.rb b/app/models/node.rb
index 5c28a786..f5b76b45 100644
--- a/app/models/node.rb
+++ b/app/models/node.rb
@@ -42,9 +42,6 @@ class Node < ApplicationRecord
             :inclusion   => { :in => ->(_) { Page.custom_templates } },
             :allow_blank => true,
             :if          => :default_template_name_changed?
-  validates :external_url, :format => { :with => %r{\Ahttps?://}i,
-                                        :allow_blank => true,
-                                        :message => :must_be_http }
 
   # Everything outside the Trash subtree, the Trash node included.
   # Relies on unique_name being authoritative for tree position --
@@ -260,6 +257,8 @@ class Node < ApplicationRecord
         self.slug = self.head.slug
       end
 
+      self.external_url = self.head.external_url
+
       if self.head.parent_node_id && self.head.parent_node_id != parent_id
         new_parent = Node.find_by(:id => self.head.parent_node_id)
 
@@ -604,23 +603,6 @@ class Node < ApplicationRecord
     self.created_at < new_id_format_date ? unique_path : id
   end
 
-  def update_external_url!(url, current_user = nil)
-    normalised = url.presence
-    return false if normalised == external_url
-
-    guard_live_change!(current_user)
-    previous = external_url
-
-    transaction do
-      update!(:external_url => normalised)
-      NodeAction.record!(:node => self, :user => current_user,
-                         :action => "node_external_url",
-                         :path => unique_name,
-                         :external_url => { "from" => previous, "to" => normalised })
-    end
-    true
-  end
-
   # Full-text search across all locale translations using PostgreSQL tsvector.
   # Uses 'simple' dictionary (no stemming, no stopwords) so queries work
   # across German and English content without language detection.
diff --git a/app/models/node_action.rb b/app/models/node_action.rb
index f1e4eaea..1e64861e 100644
--- a/app/models/node_action.rb
+++ b/app/models/node_action.rb
@@ -35,6 +35,7 @@ class NodeAction < ApplicationRecord
   #   "title"  -- pair, always; "from" null on first publish
   #   "author" -- pair, when the byline changed (incl. first publish)
   #   "tags"   -- pair of arrays, when changed
+  #   "external_url" -- pair, when a chapter's homepage changed
   #   "assets" -- {"added" => [asset names], "removed" => [asset names]},
   #               keys only when any; a delta, not a pair. The event IS
   #               the delta, full sets would bloat every entry. Changed
@@ -134,7 +135,6 @@ class NodeAction < ApplicationRecord
   #                    from the node verbs' "tags", which is a pair,
   #                    so one renderer cannot mistake the other.
   #   "path"        -- the node's unique_name, when it has a node
-  #   "external_url" -- pair
   #
   # On "event_update" only, and only when something changed -- an
   # update that changes nothing records no entry at all:
@@ -199,12 +199,16 @@ class NodeAction < ApplicationRecord
                          "to"   => title_of.call(new_page) } }
     unless old_page
       diff[:author] = { "from" => nil, "to" => new_page.user&.login } if new_page.user
+      diff[:external_url] = { "from" => nil, "to" => new_page.external_url } if new_page.external_url.present?
       return diff
     end
 
     old_author, new_author = old_page.user&.login, new_page.user&.login
     diff[:author] = { "from" => old_author, "to" => new_author } if old_author != new_author
 
+    old_url, new_url = old_page.external_url, new_page.external_url
+    diff[:external_url] = { "from" => old_url, "to" => new_url } if old_url != new_url
+
     old_tags, new_tags = old_page.tag_list.sort, new_page.tag_list.sort
     diff[:tags] = { "from" => old_tags, "to" => new_tags } if old_tags != new_tags
 
diff --git a/app/models/page.rb b/app/models/page.rb
index bba7e6bc..3240057f 100644
--- a/app/models/page.rb
+++ b/app/models/page.rb
@@ -20,6 +20,9 @@ class Page < ApplicationRecord
             :inclusion   => { :in => ->(_) { Page.custom_templates } },
             :allow_blank => true,
             :if          => :template_name_changed?
+  validates :external_url, :format => { :with => %r{\Ahttps?://}i,
+                                      :allow_blank => true,
+                                      :message => :must_be_http }
   validates_format_of   :slug, :with => /\A[A-Za-z0-9][A-Za-z0-9_-]*\z/,
                         :unless => -> { slug.blank? }
   validate :page_slug_not_reserved
@@ -208,6 +211,7 @@ class Page < ApplicationRecord
     # Clone untranslated attributes
     self.slug             = page.slug
     self.parent_node_id   = page.parent_node_id
+    self.external_url     = page.external_url
     self.tag_list         = page.tag_list
     self.template_name  ||= page.template_name
     self.published_at     = page.published_at
diff --git a/app/views/custom/page_templates/public/chapter_detail.html.erb b/app/views/custom/page_templates/public/chapter_detail.html.erb
index 794be823..105ec21a 100644
--- a/app/views/custom/page_templates/public/chapter_detail.html.erb
+++ b/app/views/custom/page_templates/public/chapter_detail.html.erb
@@ -1,8 +1,8 @@
 <div class="article" lang="<%= @page.effective_lang %>">
   <h1 class="headline"><%= @page.title %></h1>
   <%= headline_image %>
-  <% if @page.node.external_url.present? %>
-    <div class="chapter_url"><%= link_to @page.node.external_url, @page.node.external_url, target: '_blank', rel: 'noopener' %></div>
+  <% if @page.external_url.present? %>
+    <div class="chapter_url"><%= link_to @page.external_url, @page.external_url, target: '_blank', rel: 'noopener' %></div>
   <% end %>
   <p><%= sanitize(@page.abstract) %></p>
   <%= aggregate?(@page.body) %>
diff --git a/app/views/custom/partials/_chapter.html.erb b/app/views/custom/partials/_chapter.html.erb
index 8a92b46f..2065560a 100644
--- a/app/views/custom/partials/_chapter.html.erb
+++ b/app/views/custom/partials/_chapter.html.erb
@@ -7,8 +7,8 @@
     <% if page.abstract.present? %>
       <div class="chapter_location"><%= page.abstract %></div>
     <% end %>
-    <% if page.node.external_url.present? %>
-      <div class="chapter_url"><%= link_to page.node.external_url, page.node.external_url, target: '_blank', rel: 'noopener' %></div>
+    <% if page.external_url.present? %>
+      <div class="chapter_url"><%= link_to page.external_url, page.external_url, target: '_blank', rel: 'noopener' %></div>
     <% end %>
     <% open_days = page.node.events.tagged_with('open-day').order(:start_time) %>
     <% if open_days.any? %>
diff --git a/app/views/nodes/edit.html.erb b/app/views/nodes/edit.html.erb
index ae206578..6b5a1234 100644
--- a/app/views/nodes/edit.html.erb
+++ b/app/views/nodes/edit.html.erb
@@ -116,8 +116,7 @@
 
         <div class="layout_row_label"><%= t(".external_url") %></div>
         <div class="layout_row_content">
-          <%= f.text_field :external_url %>
-          <p class="field_hint"><%= t(".external_url_hint") %></p>
+          <%= d.text_field :external_url %>
         </div>
 
         <div class="layout_row_label"><%= t(".tags") %></div>