From 1a037ac6fab34c5f65ed0f37ed603b88e7b02fd1 Mon Sep 17 00:00:00 2001 From: erdgeist Date: Sat, 8 Aug 2026 02:46:06 +0200 Subject: Move the external homepage onto the draft --- app/controllers/nodes_controller.rb | 3 ++- app/helpers/node_actions_helper.rb | 15 ++++++------ app/models/node.rb | 22 ++--------------- app/models/node_action.rb | 6 ++++- app/models/page.rb | 4 ++++ .../page_templates/public/chapter_detail.html.erb | 4 ++-- app/views/custom/partials/_chapter.html.erb | 4 ++-- app/views/nodes/edit.html.erb | 3 +-- config/locales/de.yml | 11 +++------ config/locales/en.yml | 4 ++-- lib/tasks/pages.rake | 28 ++++++++++++++++++++++ test/models/node_test.rb | 25 +++++++++---------- 12 files changed, 69 insertions(+), 60 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 @@

<%= @page.title %>

<%= headline_image %> - <% if @page.node.external_url.present? %> -
<%= link_to @page.node.external_url, @page.node.external_url, target: '_blank', rel: 'noopener' %>
+ <% if @page.external_url.present? %> +
<%= link_to @page.external_url, @page.external_url, target: '_blank', rel: 'noopener' %>
<% end %>

<%= sanitize(@page.abstract) %>

<%= 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? %>
<%= page.abstract %>
<% end %> - <% if page.node.external_url.present? %> -
<%= link_to page.node.external_url, page.node.external_url, target: '_blank', rel: 'noopener' %>
+ <% if page.external_url.present? %> +
<%= link_to page.external_url, page.external_url, target: '_blank', rel: 'noopener' %>
<% 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 @@
<%= t(".external_url") %>
- <%= f.text_field :external_url %> -

<%= t(".external_url_hint") %>

+ <%= d.text_field :external_url %>
<%= t(".tags") %>
diff --git a/config/locales/de.yml b/config/locales/de.yml index 032d7647..c6dc17b7 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -120,17 +120,12 @@ de: errors: models: node: - # Attribute messages: full_messages prefixes the human attribute - # name, so these read as fragments. Defined at model level rather - # than per attribute, because several attributes share a message. reserved_for_trash: "ist für den Papierkorb reserviert" trash_immutable: "des Papierkorb-Nodes kann nicht geändert werden" trash_must_be_empty: "muss auf dem Papierkorb-Node leer bleiben" inside_trash: "kann im Papierkorb nicht existieren" attributes: base: - # :base messages stand alone -- no attribute name is prefixed, - # so these are whole sentences. has_children: "Ein Node mit Kindern kann nicht gelöscht werden" trash_undeletable: "Der Papierkorb-Node kann nicht gelöscht werden" publish_in_trash: "Ein Node im Papierkorb kann nicht veröffentlicht werden" @@ -145,6 +140,8 @@ de: attributes: slug: reserved_for_trash: "ist für den Papierkorb reserviert" + external_url: + must_be_http: "muss mit http:// oder https:// beginnen" user: attributes: roles: @@ -163,7 +160,7 @@ de: event: attributes: url: - must_be_http: "must start with http:// or https://" + must_be_http: "muss mit http:// oder https:// beginnen" tags: index: @@ -253,7 +250,6 @@ de: detail_event_allday: "ganztägig %{from} → %{to}" detail_event_end: "Ende %{from} → %{to}" event_none: "—" - node_external_url: "%{actor} hat die externe Homepage von %{subject} geändert" unknown_event: "ein Termin" open_gallery: "Gallerie anzeigen" @@ -456,7 +452,6 @@ de: set_headline_title: "Dieses Foto als Aufmacherbild der Seite verwenden" remove_image_label: "Bild entfernen" external_url: "Externe Homepage" - external_url_hint: "Die eigene Website einer Dezentrale, wird auf dessen Seite angezeigt. Vorsicht, Änderung wird ohne Draft sofort veröffentlicht." show: status: "Status" head: "Head" diff --git a/config/locales/en.yml b/config/locales/en.yml index e75000bc..bd339cc9 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -91,6 +91,8 @@ en: attributes: slug: reserved_for_trash: "is reserved for the Trash" + external_url: + must_be_http: "must start with http:// or https://" user: attributes: roles: @@ -199,7 +201,6 @@ en: detail_event_coordinates: "coordinates changed" detail_event_allday: "all-day %{from} → %{to}" detail_event_end: "end %{from} → %{to}" - node_external_url: "%{actor} changed the external homepage of %{subject}" event_none: "—" unknown_event: "an event" @@ -403,7 +404,6 @@ en: set_headline_title: "Use this photo as the page's headline image" remove_image_label: "Remove image" external_url: "External homepage" - external_url_hint: "A chapter's own website, shown on its page. N.B.: Changing takes effect immediately." show: status: "Status" head: "Head" diff --git a/lib/tasks/pages.rake b/lib/tasks/pages.rake index 0cfbc5dc..ea0776b7 100644 --- a/lib/tasks/pages.rake +++ b/lib/tasks/pages.rake @@ -30,4 +30,32 @@ namespace :pages do puts "#{write ? "updated" : "would update"} #{touched} pages" end + + desc "Backfill pages.external_url from each page's node." + task :backfill_external_url => :environment do + write = ENV["WRITE"] == "1" + puts "DRY RUN -- nothing written. Re-run with WRITE=1." unless write + + touched = 0 + Node.where.not(:external_url => [nil, ""]).find_each do |node| + scope = node.pages.where("external_url IS DISTINCT FROM :u", :u => node.external_url) + count = scope.count + next if count.zero? + + scope.update_all(:external_url => node.external_url) if write + touched += count + end + + # Autosaves carry no node_id, so has_many :pages does not cover them. + Node.where.not(:autosave_id => nil).where.not(:external_url => [nil, ""]) + .includes(:autosave).find_each do |node| + a = node.autosave + next if a.nil? || a.external_url == node.external_url + + a.update_columns(:external_url => node.external_url) if write + touched += 1 + end + + puts "#{write ? "updated" : "would update"} #{touched} pages" + end end diff --git a/test/models/node_test.rb b/test/models/node_test.rb index df1e96cb..735d2abe 100644 --- a/test/models/node_test.rb +++ b/test/models/node_test.rb @@ -1072,32 +1072,29 @@ class NodeTest < ActiveSupport::TestCase assert_equal club.id, node.parent_id end - test "setting an external url is gated and witnessed" do + test "an external url is carried by the draft and applied on publish" do node = Node.root.children.create!(:slug => "chapter_url_test") - - assert_difference -> { NodeAction.where(:action => "node_external_url").count }, 1 do - assert node.update_external_url!("https://example.org", users(:aaron)) - end + node.draft.update!(:external_url => "https://example.org") + node.publish_draft!(users(:aaron)) assert_equal "https://example.org", node.reload.external_url - entry = NodeAction.where(:action => "node_external_url").last - assert_equal({ "from" => nil, "to" => "https://example.org" }, - entry.metadata["external_url"]) + assert_equal "https://example.org", node.head.external_url end - test "a javascript url is refused" do + test "a javascript url is refused on the draft" do node = Node.root.children.create!(:slug => "chapter_url_reject") assert_raises(ActiveRecord::RecordInvalid) do - node.update_external_url!("javascript:alert(1)", users(:aaron)) + node.draft.update!(:external_url => "javascript:alert(1)") end end - test "setting an external url on a restricted node needs redaktion" do - node = Node.find_by(:unique_name => "updates") || - Node.root.children.create!(:slug => "updates") + test "publishing an external url change on a restricted node needs redaktion" do + updates = Node.root.children.create!(:slug => "updates") + node = updates.children.create!(:slug => "chapter_url_gated") + node.draft.update!(:external_url => "https://example.org") assert_raises(ActiveRecord::RecordInvalid) do - node.update_external_url!("https://example.org", users(:quentin)) + node.publish_draft!(users(:quentin)) end end -- cgit v1.3