From 9d32458491d79f8e06a706375030c78282f4427b Mon Sep 17 00:00:00 2001
From: erdgeist <erdgeist@erdgeist.org>
Date: Sun, 9 Aug 2026 15:45:56 +0200
Subject: Report every editorial attribute in the revision diff

diff_against gained external_url, published_at and user. The view renders
each metadata section only when that attribute changed, and says so once
when none did.

Metadata now precedes the locale pointer, which speaks only of translations:
above the metadata it read as "nothing changed" on a revision that had moved
the page.
---
 app/models/page.rb                |   6 ++
 app/views/revisions/diff.html.erb | 133 +++++++++++++++++++++++---------------
 2 files changed, 87 insertions(+), 52 deletions(-)

(limited to 'app')

diff --git a/app/models/page.rb b/app/models/page.rb
index 3240057f..c2dc227b 100644
--- a/app/models/page.rb
+++ b/app/models/page.rb
@@ -277,6 +277,12 @@ class Page < ApplicationRecord
 
     text_diffs.merge(
       address:       address_diff_against(other),
+      external_url:  { from: other.external_url, to: external_url,
+                       changed: external_url.presence != other.external_url.presence },
+      published_at:  { from: other.published_at, to: published_at,
+                       changed: published_at != other.published_at },
+      user:          { from: other.user, to: user,
+                       changed: user_id != other.user_id },
       tags:          { added: tag_list.to_a - other.tag_list.to_a, removed: other.tag_list.to_a - tag_list.to_a },
       template_name: { from: other.template_name, to: template_name, changed: template_name != other.template_name },
       assets:        { added: assets.to_a - other.assets.to_a, removed: other.assets.to_a - assets.to_a }
diff --git a/app/views/revisions/diff.html.erb b/app/views/revisions/diff.html.erb
index 0fd9ccb9..25f5ad04 100644
--- a/app/views/revisions/diff.html.erb
+++ b/app/views/revisions/diff.html.erb
@@ -48,7 +48,6 @@
   </p>
 <% end %>
 
-
 <% if @available_layer_pairs.present? %>
   <div class="node_action_bar standalone_action_bar">
     <% @available_layer_pairs.each do |pair| %>
@@ -71,21 +70,88 @@
 <% end %>
 
 <div id="diffview">
-  <% current_summary = @locale_summary.find { |s| s[:locale] == @translation_locale } %>
-  <% if current_summary && !current_summary[:changed] %>
-    <% elsewhere = @locale_summary.select { |s| s[:changed] }.map { |s|
-         link_to s[:locale].to_s.upcase, diff_node_revisions_path(@node,
-           start_revision: params[:start_revision], end_revision: params[:end_revision],
-           view: @diff_view, translation_locale: s[:locale]) } %>
-    <p class="diff_unchanged diff_locale_pointer">
-      <% if elsewhere.any? %>
-        <%= t(".unchanged_here_html", :lang => @translation_locale.to_s.upcase,
-              :others => safe_join(elsewhere, ", ")) %>
-      <% else %>
-        <%= t(".unchanged_anywhere") %>
+  <% meta_changed = @diff[:address][:changed] || @diff[:external_url][:changed] ||
+                    @diff[:template_name][:changed] ||
+                    @diff[:published_at][:changed] || @diff[:user][:changed] ||
+                    @diff[:tags][:added].any?   || @diff[:tags][:removed].any? ||
+                    @diff[:assets][:added].any? || @diff[:assets][:removed].any? %>
+
+  <div class="diff_preamble">
+    <div class="diff_meta">
+      <% unless meta_changed %>
+        <p class="diff_unchanged"><%= t(".no_metadata_change") %></p>
+      <% end %>
+
+      <% if @diff[:address][:changed] %>
+        <h3><%= t(".address") %></h3>
+        <p>
+          <del><%= @diff[:address][:from] || t(".none_marker") %></del>
+          <ins><%= @diff[:address][:to] || t(".none_marker") %></ins>
+        </p>
+      <% end %>
+
+      <% if @diff[:external_url][:changed] %>
+        <h3><%= Page.human_attribute_name(:external_url) %></h3>
+        <p>
+          <del><%= @diff[:external_url][:from].presence || t(".none_marker") %></del>
+          <ins><%= @diff[:external_url][:to].presence || t(".none_marker") %></ins>
+        </p>
+      <% end %>
+
+      <% if @diff[:published_at][:changed] %>
+        <h3><%= Page.human_attribute_name(:published_at) %></h3>
+        <p>
+          <del><%= @diff[:published_at][:from] ? admin_datetime(@diff[:published_at][:from]) : t(".none_marker") %></del>
+          <ins><%= @diff[:published_at][:to] ? admin_datetime(@diff[:published_at][:to]) : t(".none_marker") %></ins>
+        </p>
       <% end %>
-    </p>
-  <% end %>
+
+      <% if @diff[:user][:changed] %>
+        <h3><%= Page.human_attribute_name(:user) %></h3>
+        <p>
+          <del><%= @diff[:user][:from]&.login || t(".none_marker") %></del>
+          <ins><%= @diff[:user][:to]&.login || t(".none_marker") %></ins>
+        </p>
+      <% end %>
+
+      <% if @diff[:tags][:added].any? || @diff[:tags][:removed].any? %>
+        <h3><%= Page.human_attribute_name(:tag_list) %></h3>
+        <ul class="diff_set_list">
+          <% @diff[:tags][:added].each do |tag| %><li><ins><%= tag %></ins></li><% end %>
+          <% @diff[:tags][:removed].each do |tag| %><li><del><%= tag %></del></li><% end %>
+        </ul>
+      <% end %>
+
+      <% if @diff[:template_name][:changed] %>
+        <h3><%= Page.human_attribute_name(:template_name) %></h3>
+        <p><del><%= @diff[:template_name][:from] || t(".none_marker") %></del> <ins><%= @diff[:template_name][:to] || t(".none_marker") %></ins></p>
+      <% end %>
+
+      <% if @diff[:assets][:added].any? || @diff[:assets][:removed].any? %>
+        <h3><%= Page.human_attribute_name(:assets) %></h3>
+        <ul class="diff_set_list">
+          <% @diff[:assets][:added].each do |asset| %><li><ins><%= asset.upload_file_name %></ins></li><% end %>
+          <% @diff[:assets][:removed].each do |asset| %><li><del><%= asset.upload_file_name %></del></li><% end %>
+        </ul>
+      <% end %>
+    </div>
+
+    <% current_summary = @locale_summary.find { |s| s[:locale] == @translation_locale } %>
+    <% if current_summary && !current_summary[:changed] %>
+      <% elsewhere = @locale_summary.select { |s| s[:changed] }.map { |s|
+           link_to s[:locale].to_s.upcase, diff_node_revisions_path(@node,
+             start_revision: params[:start_revision], end_revision: params[:end_revision],
+             view: @diff_view, translation_locale: s[:locale]) } %>
+      <p class="diff_unchanged diff_locale_pointer">
+        <% if elsewhere.any? %>
+          <%= t(".unchanged_here_html", :lang => @translation_locale.to_s.upcase,
+                :others => safe_join(elsewhere, ", ")) %>
+        <% else %>
+          <%= t(".unchanged_anywhere") %>
+        <% end %>
+      </p>
+    <% end %>
+  </div>
 
   <% if @diff_view == :side_by_side %>
     <div class="diff_side_by_side">
@@ -114,41 +180,4 @@
     <h3><%= Page.human_attribute_name(:body) %></h3>
     <%= raw @diff[:body] %>
   <% end %>
-
-  <h3><%= t(".address") %></h3>
-  <% if @diff[:address][:changed] %>
-    <p>
-      <del><%= @diff[:address][:from] || t(".none_marker") %></del>
-      <ins><%= @diff[:address][:to] || t(".none_marker") %></ins>
-    </p>
-  <% else %>
-    <p class="diff_unchanged"><%= t(".no_change") %></p>
-  <% end %>
-
-  <h3><%= Page.human_attribute_name(:tag_list) %></h3>
-  <% if @diff[:tags][:added].empty? && @diff[:tags][:removed].empty? %>
-    <p class="diff_unchanged"><%= t(".no_change") %></p>
-  <% else %>
-    <ul class="diff_set_list">
-      <% @diff[:tags][:added].each do |tag| %><li><ins><%= tag %></ins></li><% end %>
-      <% @diff[:tags][:removed].each do |tag| %><li><del><%= tag %></del></li><% end %>
-    </ul>
-  <% end %>
-
-  <h3><%= Page.human_attribute_name(:template_name) %></h3>
-  <% if @diff[:template_name][:changed] %>
-    <p><del><%= @diff[:template_name][:from] || t(".none_marker") %></del> <ins><%= @diff[:template_name][:to] || t(".none_marker") %></ins></p>
-  <% else %>
-    <p class="diff_unchanged"><%= t(".no_change") %></p>
-  <% end %>
-
-  <h3><%= Page.human_attribute_name(:assets) %></h3>
-  <% if @diff[:assets][:added].empty? && @diff[:assets][:removed].empty? %>
-    <p class="diff_unchanged"><%= t(".no_change") %></p>
-  <% else %>
-    <ul class="diff_set_list">
-      <% @diff[:assets][:added].each do |asset| %><li><ins><%= asset.upload_file_name %></ins></li><% end %>
-      <% @diff[:assets][:removed].each do |asset| %><li><del><%= asset.upload_file_name %></del></li><% end %>
-    </ul>
-  <% end %>
 </div>
-- 
cgit v1.3