4 files changed, 108 insertions, 46 deletions

diff --git a/app/helpers/node_actions_helper.rb b/app/helpers/node_actions_helper.rb
index 55ca982..996f98d 100644
--- a/app/helpers/node_actions_helper.rb
+++ b/app/helpers/node_actions_helper.rb
@@ -16,8 +16,57 @@ module NodeActionsHelper
        :action => h(action.action), :subject => subject_ref(action)).html_safe
   end
 
+
+  def action_details? action
+    m = action.metadata
+    return true if m["translation_diff"].present?
+    return true if m["title"].is_a?(Hash) && m.dig("title", "from") != m.dig("title", "to")
+    %w[author tags template_changed assets_changed
+       abstract_changed body_changed].any? { |key| m[key].present? }
+  end
+
+  # Plain strings by design -- safe_join in the template escapes them.
+  def default_locale_changes action
+    m = action.metadata
+    items = []
+    if m["title"].is_a?(Hash) && m.dig("title", "from") && m.dig("title", "from") != m.dig("title", "to")
+      items << t("node_actions.detail_title",
+                  :from => m.dig("title", "from"), :to => m.dig("title", "to"))
+    end
+    items << t("node_actions.detail_author",
+                :from => m.dig("author", "from"), :to => m.dig("author", "to")) if m["author"]
+    items << t("node_actions.detail_tags",
+                :from => Array(m.dig("tags", "from")).join(", "),
+                :to   => Array(m.dig("tags", "to")).join(", ")) if m["tags"]
+    items << t("node_actions.abstract_changed") if m["abstract_changed"]
+    items << t("node_actions.body_changed")     if m["body_changed"]
+    items << t("node_actions.template_changed") if m["template_changed"]
+    items << t("node_actions.assets_changed")   if m["assets_changed"]
+    items
+  end
+
+  def translation_changes diff
+    case diff["status"]
+    when "added"   then [t("node_actions.locale_added",   :title => diff.dig("title", "to"))]
+    when "removed" then [t("node_actions.locale_removed", :title => diff.dig("title", "from"))]
+    else
+      items = []
+      items << t("node_actions.detail_title",
+                  :from => diff.dig("title", "from"), :to => diff.dig("title", "to")) if diff["title"]
+      items << t("node_actions.abstract_changed") if diff["abstract_changed"]
+      items << t("node_actions.body_changed")     if diff["body_changed"]
+      items
+    end
+  end
+
   private
 
+  def revision_ref action, key
+    label = t(key)
+    return label unless action.node && action.page
+    link_to(label, node_revision_path(action.node, action.page))
+  end
+
   def actor_ref action
     action.user ? link_to(h(action.actor_name), admin_log_path(:user_id => action.user_id))
                 : h(action.actor_name)
@@ -29,17 +78,20 @@ module NodeActionsHelper
   end
 
   def summarize_publish action
-    key = if action.metadata["via"] == "revision"
-            "node_actions.publish_rollback"
-          elsif action.metadata.dig("title", "from").nil?
-            "node_actions.publish_first"
-          else
-            "node_actions.publish"
-          end
-
-    t(key, :actor => actor_ref(action), :subject => subject_ref(action),
-            :from => h(action.metadata.dig("title", "from")),
-            :to   => h(action.metadata.dig("title", "to"))).html_safe
+    if action.metadata["via"] == "revision"
+      t("node_actions.publish_rollback",
+         :actor => actor_ref(action), :subject => subject_ref(action),
+         :revision => revision_ref(action, "node_actions.revision_earlier")).html_safe
+    elsif action.metadata.dig("title", "from").nil?
+      author = action.metadata.dig("author", "to")
+      key = author ? "node_actions.publish_first_with_author" : "node_actions.publish_first"
+      t(key, :actor => actor_ref(action), :subject => subject_ref(action),
+              :author => h(author)).html_safe
+    else
+      t("node_actions.publish",
+         :actor => actor_ref(action), :subject => subject_ref(action),
+         :revision => revision_ref(action, "node_actions.revision_new")).html_safe
+    end
   end
 
   def summarize_move action
diff --git a/app/models/node_action.rb b/app/models/node_action.rb
index 39935c7..792ae1e 100644
--- a/app/models/node_action.rb
+++ b/app/models/node_action.rb
@@ -82,21 +82,23 @@ class NodeAction < ApplicationRecord
   # user agent, ever. Success only -- rejected attempts are not
   # logged.
 
-  def self.record!(node:, action:, user: nil, page: nil, locale: nil, **extra)
+  def self.record!(node:, action:, user: nil, page: nil, locale: nil,
+                    occurred_at: nil, inferred_from: nil, **extra)
     create!(
-      :node        => node,
-      :page        => page,
-      :user        => user,
-      :action      => action,
-      :locale      => locale,
-      :occurred_at => Time.now,
-      :metadata    => {
+      :node          => node,
+      :page          => page,
+      :user          => user,
+      :action        => action,
+      :locale        => locale,
+      :occurred_at   => occurred_at || Time.now,
+      :inferred_from => inferred_from,
+      :metadata      => {
         "username"                 => user&.login,
         "human_readable_node_name" => Globalize.with_locale(I18n.default_locale) {
                                         node&.head&.title || node&.draft&.title
                                       },
       }.merge(extra.stringify_keys)
-   )
+    )
   end
 
   # Computes the publish-entry diff between an outgoing head and the
@@ -113,7 +115,10 @@ class NodeAction < ApplicationRecord
     title_of = ->(page) { page&.translations&.find_by(:locale => default)&.title }
     diff = { :title => { "from" => title_of.call(old_page),
                          "to"   => title_of.call(new_page) } }
-    return diff unless old_page
+    unless old_page
+      diff[:author] = { "from" => nil, "to" => new_page.user&.login } if new_page.user
+      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
diff --git a/app/views/node_actions/_change_details.html.erb b/app/views/node_actions/_change_details.html.erb
index 383f53a..2583e8b 100644
--- a/app/views/node_actions/_change_details.html.erb
+++ b/app/views/node_actions/_change_details.html.erb
@@ -1,27 +1,27 @@
 <details class="node_action_details">
   <summary><%= t("node_actions.show_changes") %></summary>
   <table>
-    <% action_entry.metadata["translation_diff"].each do |locale, diff| %>
+    <% if (default_items = default_locale_changes(action_entry)).any? %>
+      <tr>
+        <th><%= I18n.default_locale.to_s.upcase %></th>
+        <td>
+          <%= safe_join(default_items, ", ") %>
+          <% if action_entry.page && action_entry.node %>
+            <%= link_to t("node_actions.view_revision"), node_revision_path(action_entry.node, action_entry.page) %>
+          <% end %>
+        </td>
+      </tr>
+    <% end %>
+    <% (action_entry.metadata["translation_diff"] || {}).each do |locale, diff| %>
       <tr>
         <th><%= locale.upcase %></th>
         <td>
-          <% case diff["status"] %>
-          <% when "added" %>
-            <%= t("node_actions.locale_added", :title => diff.dig("title", "to")) %>
-          <% when "removed" %>
-            <%= t("node_actions.locale_removed", :title => diff.dig("title", "from")) %>
-          <% else %>
-            <% if diff["title"] %>
-              <%= diff.dig("title", "from") %> → <%= diff.dig("title", "to") %>
-            <% end %>
-            <%= t("node_actions.abstract_changed") if diff["abstract_changed"] %>
-            <%= t("node_actions.body_changed") if diff["body_changed"] %>
+          <%= safe_join(translation_changes(diff), ", ") %>
+          <% if action_entry.page && action_entry.node %>
+            <%= link_to t("node_actions.view_revision"), node_revision_path(action_entry.node, action_entry.page, :locale => locale) %>
           <% end %>
         </td>
       </tr>
     <% end %>
   </table>
-  <% if action_entry.page && action_entry.node %>
-    <%= link_to t("node_actions.view_revision"), node_revision_path(action_entry.node, action_entry.page) %>
-  <% end %>
 </details>
diff --git a/app/views/node_actions/index.html.erb b/app/views/node_actions/index.html.erb
index 92740cd..ee06213 100644
--- a/app/views/node_actions/index.html.erb
+++ b/app/views/node_actions/index.html.erb
@@ -6,17 +6,22 @@
 
 <%= will_paginate @actions %>
 
-<ul id="node_action_list">
+<table id="node_action_list">
   <% @actions.each do |action| %>
-    <li class="node_action node_action--<%= action.action %>">
-      <span class="node_action_time"><%= l(action.occurred_at, :format => :ccc) %></span>
-      <% if action.inferred_from %>
-        <span class="node_action_inferred" title="<%= action.inferred_from %>"><%= t("node_actions.backfilled") %></span>
-      <% end %>
-      <span class="node_action_summary"><%= action_summary(action) %></span>
-      <%= render "change_details", :action_entry => action if action.metadata["translation_diff"].present? %>
-    </li>
+    <tr class="node_action node_action--<%= action.action %>">
+      <td class="node_action_time"><%= action.occurred_at.strftime("%Y-%m-%d %H:%M") %></td>
+      <td class="node_action_body">
+        <%= action_summary(action) %>
+        <% if action.node_id && params[:node_id].blank? %>
+          <%= link_to t("node_actions.node_history"), admin_log_path(:node_id => action.node_id), :class => "node_action_zoom" %>
+        <% end %>
+        <% if action.inferred_from %>
+          <span class="node_action_inferred" title="<%= action.inferred_from %>"><%= t("node_actions.backfilled") %></span>
+        <% end %>
+        <%= render "change_details", :action_entry => action if action_details?(action) %>
+      </td>
+    </tr>
   <% end %>
-</ul>
+</table>
 
 <%= will_paginate @actions %>