Surface redirects in nodes#show, the action log and when publishing

9 files changed, 107 insertions, 9 deletions

diff --git a/app/helpers/admin_helper.rb b/app/helpers/admin_helper.rb
index be20eb8b..b696c21c 100644
--- a/app/helpers/admin_helper.rb
+++ b/app/helpers/admin_helper.rb
@@ -38,4 +38,12 @@ module AdminHelper
       }
     }.to_json
   end
+
+  def redirect_flag_hint page
+    target = page.redirect_target
+    return t("nodes.show.redirect_flag_broken") unless target
+
+    target.internal? ? t("nodes.show.redirect_flag", :path => target.node.unique_name)
+                     : t("nodes.show.redirect_flag_url", :url => target.url)
+  end
 end
diff --git a/app/models/node.rb b/app/models/node.rb
index 52e06d2d..ef981a97 100644
--- a/app/models/node.rb
+++ b/app/models/node.rb
@@ -243,7 +243,7 @@ class Node < ApplicationRecord
       end
 
       if self.draft.redirect_node_id.present?
-        target = Node.find_by(:id => self.draft.redirect_node_id)
+        target = self.draft.redirect_node
 
         unless target
           errors.add(:base, :redirect_target_missing)
diff --git a/app/models/node_action.rb b/app/models/node_action.rb
index aab1d238..a7036833 100644
--- a/app/models/node_action.rb
+++ b/app/models/node_action.rb
@@ -200,6 +200,12 @@ class NodeAction < ApplicationRecord
     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_redirect, new_redirect = old_page.redirect, new_page.redirect
+    diff[:redirect] = { "from" => old_redirect, "to" => new_redirect } if old_redirect != new_redirect
+
+    old_target, new_target = old_page.redirect_node&.unique_name, new_page.redirect_node&.unique_name
+    diff[:redirect_target] = { "from" => old_target, "to" => new_target } if old_target != new_target
+
     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 5d3cb0aa..91b7e975 100644
--- a/app/models/page.rb
+++ b/app/models/page.rb
@@ -36,6 +36,7 @@ class Page < ApplicationRecord
   belongs_to :parent_node, :class_name => "Node", :optional => true
   belongs_to :user, optional: true
   belongs_to :editor, :class_name => "User", optional: true
+  belongs_to :redirect_node, :class_name => "Node", :optional => true
   has_many   :related_assets, :dependent => :destroy
   has_many   :assets, -> { order("position ASC") }, :through => :related_assets
 
@@ -289,7 +290,11 @@ class Page < ApplicationRecord
                        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 }
+      assets:        { added: assets.to_a - other.assets.to_a, removed: other.assets.to_a - assets.to_a },
+      redirect:      { from: other.redirect, to: redirect,
+                       changed: redirect != other.redirect },
+      redirect_node: { from: other.redirect_node, to: redirect_node,
+                       changed: redirect_node_id != other.redirect_node_id }
     )
   end
 
@@ -326,9 +331,8 @@ class Page < ApplicationRecord
     return nil if redirect.blank?
 
     if redirect_node_id.present?
-      target = Node.find_by(:id => redirect_node_id)
-      return nil unless target&.head
-      return RedirectTarget.new(target, nil)
+      return nil unless redirect_node&.head
+      return RedirectTarget.new(redirect_node, nil)
     end
 
     return RedirectTarget.new(nil, external_url) if external_url.present?
@@ -345,6 +349,11 @@ class Page < ApplicationRecord
     Node.where(:head_id => where(:redirect_node_id => node_id).select(:id))
   end
 
+  def redirect_differs_from? other
+    mine, theirs = redirect_target, other&.redirect_target
+    mine&.url != theirs&.url || mine&.node != theirs&.node
+  end
+
   # The address this page will have once published.
   def prospective_unique_name
     return nil if parent_node_id.nil?
diff --git a/app/views/nodes/edit.html.erb b/app/views/nodes/edit.html.erb
index f629985e..a40f3949 100644
--- a/app/views/nodes/edit.html.erb
+++ b/app/views/nodes/edit.html.erb
@@ -154,8 +154,7 @@
                 <div id="redirect_target_field" style="<%= "display: none;" unless @page.redirect.present? %>">
                   <div class="input_group" data-clears="page_redirect_node_id">
                     <span class="field_search_icon"><%= icon("search", library: "tabler", "aria-hidden": true) %></span>
-                    <%= text_field_tag :redirect_search_term,
-                          Node.find_by(:id => @page.redirect_node_id)&.title,
+                    <%= text_field_tag :redirect_search_term, @page.redirect_node&.title,
                           :placeholder => "—", :class => "clearable_input" %>
                     <div id="redirect_search_results" class="search_results"></div>
                     <button type="button" class="field_clear" aria-label="clear input">×</button>
diff --git a/app/views/nodes/show.html.erb b/app/views/nodes/show.html.erb
index 24ac4084..d8c08eb1 100644
--- a/app/views/nodes/show.html.erb
+++ b/app/views/nodes/show.html.erb
@@ -9,14 +9,35 @@
       <div class="info_group_items">
         <div class="info_item">
           <span class="info_label"><%= t(".head") %></span>
+          <% if @node.head&.redirect.present? %>
+            <span class="flag_stack">
+              <%= flag("arrow-ramp-right",
+                       redirect_flag_hint(@node.head),
+                       :tier => :attention) %>
+            </span>
+          <% end %>
           <%= @node.head ? t(".head_line", :title => @node.head.title, :rev => @node.head.revision, :time => @node.head.updated_at) : t(".never_published") %>
         </div>
         <div class="info_item">
           <span class="info_label"><%= t(".draft") %></span>
+          <% if @node.draft&.redirect.present? %>
+            <span class="flag_stack">
+              <%= flag("arrow-ramp-right",
+                       redirect_flag_hint(@node.draft),
+                       :tier => :attention) %>
+            </span>
+          <% end %>
           <%= @node.draft ? t(".draft_line", :title => @node.draft.title, :rev => @node.draft.revision, :time => @node.draft.updated_at) : t(".none") %>
         </div>
         <div class="info_item">
           <span class="info_label"><%= t(".autosave") %></span>
+          <% if @node.autosave&.redirect.present? %>
+            <span class="flag_stack">
+              <%= flag("arrow-ramp-right",
+                       redirect_flag_hint(@node.autosave),
+                       :tier => :attention) %>
+            </span>
+          <% end %>
           <%= @node.autosave ? t(".autosave_line", :title => @node.autosave.title, :time => @node.autosave.updated_at) : t(".none") %>
         </div>
       </div>
@@ -56,6 +77,11 @@
                   <% confirm += " " + t(".confirm_publish_moves",
                                         :path => @node.prospective_unique_name) %>
                 <% end %>
+                <% if @node.draft.redirect.present? && @node.head&.redirect.blank? %>
+                  <% confirm += " " + t(".confirm_publish_redirects") %>
+                <% elsif @node.draft.redirect.present? && @node.draft.redirect_differs_from?(@node.head) %>
+                  <% confirm += " " + t(".confirm_publish_redirect_changes") %>
+                <% end %>
                 <%= button_to t(".publish"), publish_node_path(@node), method: :put,
                       form: { data: { confirm: confirm }, class: 'button_to state_changing' } %>
               <% else %>
@@ -246,6 +272,17 @@
         </p>
       <% end %>
 
+      <% if (redirect = @node.head&.redirect_target) %>
+        <p class="pending_address">
+          <%= icon("arrow-right", library: "tabler", "aria-hidden": true) %>
+          <% if redirect.internal? %>
+            <%= t(".redirects_to", :path => redirect.node.unique_name) %>
+          <% else %>
+            <%= t(".redirects_to_url", :url => redirect.url) %>
+          <% end %>
+        </p>
+      <% end %>
+
       <div class="link_matrix link_matrix_<%= link_locales.size %>">
         <div class="link_matrix_head">
           <span></span>
diff --git a/app/views/revisions/diff.html.erb b/app/views/revisions/diff.html.erb
index 17146615..eb7955c2 100644
--- a/app/views/revisions/diff.html.erb
+++ b/app/views/revisions/diff.html.erb
@@ -74,7 +74,8 @@
                     @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? %>
+                    @diff[:assets][:added].any? || @diff[:assets][:removed].any? ||
+                    @diff[:redirect][:changed] || @diff[:redirect_node][:changed] %>
 
   <div class="diff_preamble">
     <div class="diff_meta">
@@ -98,6 +99,22 @@
         </p>
       <% end %>
 
+      <% if @diff[:redirect][:changed] %>
+        <h3><%= Page.human_attribute_name(:redirect) %></h3>
+        <p>
+          <del><%= @diff[:redirect][:from] ? t("nodes.edit.redirect_mode_#{@diff[:redirect][:from]}") : t(".none_marker") %></del>
+          <ins><%= @diff[:redirect][:to]   ? t("nodes.edit.redirect_mode_#{@diff[:redirect][:to]}")   : t(".none_marker") %></ins>
+        </p>
+      <% end %>
+
+      <% if @diff[:redirect_node][:changed] %>
+        <h3><%= Page.human_attribute_name(:redirect_node) %></h3>
+        <p>
+          <del><%= @diff[:redirect_node][:from]&.unique_name || t(".none_marker") %></del>
+          <ins><%= @diff[:redirect_node][:to]&.unique_name   || t(".none_marker") %></ins>
+        </p>
+      <% end %>
+
       <% if @diff[:published_at][:changed] %>
         <h3><%= Page.human_attribute_name(:published_at) %></h3>
         <p>
diff --git a/config/locales/de.yml b/config/locales/de.yml
index 95d6edc2..1d8d7090 100644
--- a/config/locales/de.yml
+++ b/config/locales/de.yml
@@ -107,6 +107,8 @@ de:
         template_name: "Template"
         assets: "Anhänge"
         external_url: "Externe Homepage"
+        redirect: "Weiterleitung"
+        redirect_node: "Weiterleitungsziel"
         published_at: "Veröffentlichungsdatum"
         user: "Autor"
       menu_item:
@@ -459,6 +461,8 @@ de:
       redirect_hint: "Ohne Ziel-Node wird auf die externe Homepage weitergeleitet."
       redirect_target_hint: "Bleibt erhalten, wenn die Weiterleitung abgeschaltet wird."
       redirect_clear: "Ziel entfernen"
+      redirect_mode_temporary: "vorübergehend"
+      redirect_mode_permanent: "dauerhaft"
     show:
       status: "Status"
       head: "Head"
@@ -480,6 +484,8 @@ de:
       publish: "Veröffentlichen"
       confirm_publish: "Diesen Entwurf veröffentlichen?"
       confirm_publish_moves: "Dabei wird die Seite nach %{path} verschoben."
+      confirm_publish_redirects: "Danach zeigt die Seite ihren Inhalt nicht mehr, sondern leitet weiter."
+      confirm_publish_redirect_changes: "Das Ziel der Weiterleitung ändert sich."
       destroy_draft: "Entwurf löschen"
       discard_autosave: "Autosave verwerfen"
       confirm_trash: "Diese Seite und alles darunter in den Papierkorb verschieben? Alle veröffentlichten Inhalte darin gehen offline."
@@ -516,6 +522,11 @@ de:
       links: "Links"
       public: "Öffentlich"
       pending_address: "Beim Veröffentlichen wird diese Seite nach %{path} verschoben"
+      redirects_to: "Diese Seite leitet auf %{path} weiter."
+      redirects_to_url: "Diese Seite leitet auf %{url} weiter."
+      redirect_flag: "Leitet auf %{path} weiter"
+      redirect_flag_url: "Leitet auf %{url} weiter"
+      redirect_flag_broken: "Weiterleitung ohne gültiges Ziel"
       admin_preview: "Admin-Vorschau"
       public_preview: "Öffentliche Vorschau"
       revoke: "Zurückziehen"
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 8b67f0ed..9fab0f58 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -58,6 +58,8 @@ en:
         template_name: "Template"
         assets: "Assets"
         external_url: "External homepage"
+        redirect: "Redirect"
+        redirect_node: "Redirect node"
         published_at: "Publication date"
         user: "Author"
       menu_item:
@@ -411,6 +413,8 @@ en:
       redirect_hint: "If the taget node is empty, the external url will be used."
       redirect_target_hint: "Disabling does not clear the destination"
       redirect_clear: "Clear redirect"
+      redirect_mode_temporary: "temporary"
+      redirect_mode_permanent: "permanent"
     show:
       status: "Status"
       head: "Head"
@@ -432,11 +436,16 @@ en:
       publish: "Publish"
       confirm_publish: "Publish this draft?"
       confirm_publish_moves: "This will also move the page to %{path}."
+      confirm_publish_redirects: "This will make the page redirect elsewhere."
+      confirm_publish_redirect_changes: "This will change the page's redirect target."
       destroy_draft: "Destroy Draft"
       discard_autosave: "Discard Autosave"
       confirm_trash: "Move this page and everything beneath it to the Trash? All published content in it will go offline."
       move_to_trash: "Move to Trash"
       locked_hint: "Locked — see People below to unlock before editing, publishing, or discarding."
+      redirect_flag: "Redirects to %{path}"
+      redirect_flag_url: "Redirects to %{url}"
+      redirect_flag_broken: "Broken redirect"
       visit: "Visit"
       copy_url: "Copy URL"
       trash: "Trash"
@@ -467,7 +476,9 @@ en:
       will_publish_at: "Will publish at"
       links: "Links"
       public: "Public"
-      pending_address: "Beim Veröffentlichen wird diese Seite nach %{path} verschoben"
+      pending_address: "Publishing will move this page to %{path}"
+      redirects_to: "This page redirects to %{path}"
+      redirects_to_url: "This page redirects to %{url}"
       admin_preview: "Admin Preview"
       public_preview: "Public Preview"
       revoke: "Revoke"