Give node lists a flag column and a shared action bar

13 files changed, 104 insertions, 124 deletions

diff --git a/app/controllers/nodes_controller.rb b/app/controllers/nodes_controller.rb
index ca30976f..447d596d 100644
--- a/app/controllers/nodes_controller.rb
+++ b/app/controllers/nodes_controller.rb
@@ -20,9 +20,7 @@ class NodesController < ApplicationController
                             ]
 
   def index
-    @nodes = Node.root.descendants.includes(:head, :draft)
-      .order('id DESC')
-      .paginate(:page => params[:page], :per_page => 25)
+    @nodes = index_matching(Node.root.descendants)
   end
 
   def new
diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index b06d11fd..20355aa8 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -140,7 +140,7 @@ class UsersController < ApplicationController
 
     def verify_status
       @user ||= User.new
-      unless @user.id == current_user.id || current_user.admin
+      unless @user.id == current_user.id || current_user.is_admin?
         deny_user_access
       end
     end
diff --git a/app/models/node.rb b/app/models/node.rb
index d6fe32ff..f8c29b48 100644
--- a/app/models/node.rb
+++ b/app/models/node.rb
@@ -648,6 +648,10 @@ class Node < ApplicationRecord
     )
   end
 
+  def embargoed?
+    head&.published_at.present? && head.published_at > Time.zone.now
+  end
+
   # Nodes are never destroyed recursively
   # Descendants must be removed or reparented individually first.
   # The Trash feature will be the ordinary path to deletion.
diff --git a/app/models/user.rb b/app/models/user.rb
index 786f8d14..95706f86 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -70,24 +70,6 @@ class User < ApplicationRecord
     roles.include?("admin")
   end
 
-  # Compatibility shims for the users form, which posts user[admin] as a
-  # checkbox. Goes away when that form learns about roles.
-  def admin
-    is_admin?
-  end
-
-  def admin?
-    is_admin?
-  end
-
-  def admin=(value)
-    if ActiveModel::Type::Boolean.new.cast(value)
-      self.roles = (roles | ["admin"])
-    else
-      self.roles = (roles - ["admin"])
-    end
-  end
-
   def redaktion?
     roles.include?("redaktion")
   end
diff --git a/app/views/events/index.html.erb b/app/views/events/index.html.erb
index a2710c2a..cc3f163d 100644
--- a/app/views/events/index.html.erb
+++ b/app/views/events/index.html.erb
@@ -22,10 +22,12 @@
 <% @events.each do |event| %>
   <tr>
     <td>
-      <span class="flag_stack">
-        <%= flag("clock-off", Event.human_attribute_name(:allday)) if event.allday %>
-      </span>
-      <%= link_to event.display_title, event %>
+      <div class="title_with_flags">
+        <span class="flag_stack">
+          <%= flag("clock-off", Event.human_attribute_name(:allday)) if event.allday %>
+        </span>
+        <div class="title_body"><%= link_to event.display_title, event %></div>
+      </div>
     </td>
     <td><%= admin_datetime(event.start_time) %></td>
     <td><%= admin_datetime(event.end_time) %></td>
diff --git a/app/views/nodes/_node_list.html.erb b/app/views/nodes/_node_list.html.erb
index 0dc3c5a0..458ab1ae 100644
--- a/app/views/nodes/_node_list.html.erb
+++ b/app/views/nodes/_node_list.html.erb
@@ -15,26 +15,46 @@
 <%= will_paginate @nodes %>
 <table class="node_table">
   <tr class="header">
-    <th class="node_id"><%= t("admin.columns.id") %></th>
     <th class="title"><%= t("admin.columns.title") %></th>
     <th class="actions"><%= t("admin.columns.actions") %></th>
-    <th class="editor"><%= t("admin.columns.locked_by") %></th>
     <th class="revision"><%= t("admin.columns.rev") %></th>
   </tr>
   <% @nodes.each do |node| %>
   <tr class="<%= cycle("even", "odd") %>">
-    <td class="node_id"><%= node.id %></td>
     <td class="title">
-      <h4><%= link_to title_for_node(node), node_path(node) %></h4>
-      <p><%= link_to_path(node.unique_name, node.unique_name) %></p>
+      <div class="title_with_flags">
+        <span class="flag_stack">
+          <%= flag("lock", t(".flag_locked", :login => node.lock_owner.login),
+                   :tier => :attention) if node.lock_owner %>
+          <%= flag("calendar-clock",
+                   t(".flag_embargo", :date => admin_datetime(node.head.published_at)),
+                   :tier => :attention) if node.embargoed? %>
+          <%= flag("file-pencil", t(".flag_draft")) if node.draft %>
+          <%= flag("file-off", t(".flag_no_head")) unless node.head %>
+        </span>
+        <div class="title_body">
+          <div class="node_title"><%= link_to title_for_node(node), node_path(node) %></div>
+          <div class="node_path"><%= link_to_path(node.unique_name, node.unique_name) %></div>
+        </div>
+      </div>
     </td>
     <td class="actions">
-      <%= link_to 'show',      node_path(node) %>
-      <%= link_to 'edit',      edit_node_path(node) %>
-      <%= link_to 'revisions', node_revisions_path(node) %>
+      <div class="action_grid">
+        <span class="action_item">
+          <%= link_to node_path(node), "aria-label" => t("admin.common.show"),
+                title: t("admin.common.show") do %>
+            <%= icon("eye", library: "tabler", "aria-hidden": true) %>
+          <% end %>
+        </span>
+        <span class="action_item">
+          <%= link_to node_revisions_path(node), "aria-label" => t("admin.common.revisions"),
+                title: t("admin.common.revisions") do %>
+            <%= icon("history", library: "tabler", "aria-hidden": true) %>
+          <% end %>
+        </span>
+      </div>
     </td>
-    <td><%= node.lock_owner.login if node.lock_owner %></td>
-    <td><%= node.draft ? node.draft.revision : (node.head ? node.head.revision : t(".no_revision")) %></td>
+    <td><%= node.head ? node.head.revision : t(".no_revision") %></td>
   </tr>
   <% end %>
 </table>
diff --git a/app/views/nodes/index.html.erb b/app/views/nodes/index.html.erb
index 7ab5679e..357c938b 100644
--- a/app/views/nodes/index.html.erb
+++ b/app/views/nodes/index.html.erb
@@ -7,35 +7,4 @@
   <span class="field_hint"><%= t(".create_hint") %></span>
 </div>
 
-<%= will_paginate @nodes %>
-  <table class="node_table">
-    <tr class="header">
-      <th class="node_id"><%= t("admin.columns.id") %></th>
-      <th class="title"><%= t("admin.columns.title") %></th>
-      <th class="actions"><%= t("admin.columns.actions") %></th>
-      <th class="editor"><%= t("admin.columns.locked_by") %></th>
-      <th class="revision"><%= t("admin.columns.rev") %></th>
-    </tr>
-    <% @nodes.each do |node| %>
-    <tr class="<%= cycle("even", "odd") %>">
-      <td class="node_id"><%= node.id %></td>
-      <td class="title">
-        <h4><%= link_to title_for_node(node), node_path(node) %></h4>
-        <p><%= link_to_path(node.unique_name, node.unique_name) %></p>
-      </td>
-      <td class="actions">
-        <%= link_to 'show',      node_path(node) %>
-        <%= link_to 'edit',      edit_node_path(node) %>
-        <%= link_to 'revisions', node_revisions_path(node) %>
-      </td>
-      <td>
-        <%= node.lock_owner.login if node.lock_owner %>
-      </td>
-      <td>
-        <%= node.draft ? node.draft.revision : (node.head ? node.head.revision : "EMPTY") %>
-      </td>
-    </tr>
-    <% end %>
-  </table>
-<%= will_paginate @nodes %>
-
+<%= render 'node_list' %>
diff --git a/app/views/users/_user.html.erb b/app/views/users/_user.html.erb
index be92ad1d..028ee02f 100644
--- a/app/views/users/_user.html.erb
+++ b/app/views/users/_user.html.erb
@@ -27,17 +27,17 @@
       <span class="field_hint"><%= t(".no_roles") %></span>
     <% end %>
   </td>
-  <td class="user_actions">
-    <div class="user_action_grid">
-      <span class="user_action">
+  <td class="actions">
+    <div class="action_grid">
+      <span class="action_item">
         <%= link_to user_path(user), "aria-label" => t("admin.common.show"),
               title: t("admin.common.show") do %>
           <%= icon("eye", library: "tabler", "aria-hidden": true) %>
         <% end %>
       </span>
 
-      <span class="user_action">
-        <% if current_user.admin? || current_user == user %>
+      <span class="action_item">
+        <% if current_user.is_admin? || current_user == user %>
           <%= link_to edit_user_path(user), "aria-label" => t("admin.common.edit"),
                 title: t("admin.common.edit") do %>
             <%= icon("edit", library: "tabler", "aria-hidden": true) %>
@@ -45,15 +45,15 @@
         <% end %>
       </span>
 
-      <span class="user_action">
+      <span class="action_item">
         <%= link_to admin_log_path(:user_id => user.id),
               "aria-label" => t(".action_log"), title: t(".action_log") do %>
           <%= icon("history", library: "tabler", "aria-hidden": true) %>
         <% end %>
       </span>
 
-      <span class="user_action">
-        <% if current_user.admin? && current_user != user %>
+      <span class="action_item">
+        <% if current_user.is_admin? && current_user != user %>
           <% if user.alumni? %>
             <%= button_to reactivate_user_path(user), method: :put,
                   form: { class: 'button_to state_changing' },
@@ -71,7 +71,7 @@
         <% end %>
       </span>
 
-      <span class="user_action">
+      <span class="action_item">
         <% if current_user.redaktion? && !user.alumni? %>
           <% if user.redaktion? %>
             <% unless user == current_user %>
diff --git a/app/views/users/edit.html.erb b/app/views/users/edit.html.erb
index ad2e0891..1763394d 100644
--- a/app/views/users/edit.html.erb
+++ b/app/views/users/edit.html.erb
@@ -20,7 +20,7 @@
     <div class="layout_row_label"><%= t("users.labels.confirm") %></div>
     <div class="layout_row_content"><%= f.password_field :password_confirmation, :autocomplete => "new-password" %></div>
 
-    <% if current_user.admin? %>
+    <% if current_user.is_admin? %>
       <div class="layout_row_label"><%= t("users.labels.roles") %></div>
       <div class="layout_row_content">
         <% User::ROLES.each do |role| %>
@@ -62,7 +62,7 @@
         <% end %>
       <% end %>
     </div>
-  <% elsif current_user.admin? && @user.otp_enrolled? %>
+  <% elsif current_user.is_admin? && @user.otp_enrolled? %>
     <div class="layout_row_label"><%= t(".second_factor") %></div>
     <div class="layout_row_content">
       <%= t(".enabled") %>
diff --git a/config/locales/de.yml b/config/locales/de.yml
index a1b29199..6f8dd24a 100644
--- a/config/locales/de.yml
+++ b/config/locales/de.yml
@@ -373,6 +373,7 @@ de:
       unlock_confirm: "Wirklich entsperren?\nGesperrt von %{login}\nZuletzt geändert %{time}"
       toggle_preview: "Live-Vorschau umschalten"
       force_render: "Vorschau neu rendern"
+      revisions: "Revisionen"
       "yes": "ja"
       "no": "nein"
     columns:
diff --git a/config/locales/en.yml b/config/locales/en.yml
index fc011437..c7a728ab 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -321,6 +321,7 @@ en:
       unlock_confirm: "Are you sure you want to unlock?\nLocked by %{login}\nLast modified %{time}"
       toggle_preview: "Toggle live preview"
       force_render: "Force preview render"
+      revisions: "revisions"
       "yes": "yes"
       "no": "no"
     columns:
diff --git a/public/stylesheets/admin.css b/public/stylesheets/admin.css
index 1c74bbf5..8b976599 100644
--- a/public/stylesheets/admin.css
+++ b/public/stylesheets/admin.css
@@ -644,24 +644,20 @@ table tr.header {
   text-align: left;
 }
 
-table.node_table ,
+table.node_table,
 table.assets_table,
 table.user_table,
 table.events_table {
   border-collapse: collapse;
 }
 
-table.node_table tr {
-  border-bottom: 1px solid var(--text);
-}
-
+table.node_table tr,
 table.assets_table tr,
 table.user_table tr,
 table.events_table tr {
   border-bottom: 1px solid var(--hairline);
 }
 
-table.node_table th.node_id,
 table.node_table th.revision {
   min-width: 4rem;
 }
@@ -682,14 +678,7 @@ table.events_table tr.header {
   text-align: left;
 }
 
-table.node_table td {
-  padding-top: 0px;
-  padding-bottom: 0px;
-  padding-right: 25px;
-  padding-left: 0px;
-  min-width: 2rem;
-}
-
+table.node_table td,
 table.assets_table td,
 table.user_table td,
 table.events_table td {
@@ -700,15 +689,6 @@ table.events_table td {
   min-width: 2rem;
 }
 
-table.node_table .node_id {
-  padding-left: 10px;
-  padding-right: 15px;
-}
-
-table.node_table .actions {
-  text-transform: lowercase;
-}
-
 table.node_table tr.header:hover,
 table.assets_table tr.header:hover,
 table.user_table tr.header:hover,
@@ -723,10 +703,6 @@ table.events_table tr:hover {
   background-color: var(--surface-tint);
 }
 
-#admin_sitemap_table .node_id:before {
-  content: "• ";
-}
-
 table.revisions_table {
   border-collapse: collapse;
 }
@@ -844,37 +820,47 @@ table.revisions_table tr:hover {
   margin-bottom: 1.5rem;
 }
 
-.user_table td.user_actions {
+.user_table td.actions,
+.node_table td.actions {
   width: 1px;
   white-space: nowrap;
+}
+
+/* Only where the action column is last. In the node lists rev. follows it,
+   and the 25px gutter is a real gap between columns there. */
+.user_table td.actions {
   padding-right: 0;
 }
 
-.user_table .user_action_grid {
+.action_grid {
   display: grid;
-  grid-template-columns: repeat(3, auto);
   gap: 0.4rem;
   justify-content: end;
 }
 
-.user_table .user_action {
+.user_table .action_grid { grid-template-columns: repeat(3, auto); }
+.node_table .action_grid { grid-template-columns: repeat(2, auto); }
+
+/* min-height keeps a row the same height whether or not the viewer may see
+   the lower controls. */
+.action_item {
   display: inline-flex;
   min-height: 2rem;
 }
 
-.user_table .user_action svg {
+.action_item svg {
   width: 1.25rem;
   height: 1.25rem;
 }
 
-.user_table .user_action form.button_to {
+.action_item form.button_to {
   display: inline-flex;
   margin: 0;
 }
 
-.user_table .user_action a,
-.user_table .user_action button[type="submit"],
-.user_table .user_action .disabled_action {
+.action_item a,
+.action_item form.button_to button[type="submit"],
+.action_item .disabled_action {
   display: inline-flex;
   align-items: center;
   justify-content: center;
@@ -886,22 +872,22 @@ table.revisions_table tr:hover {
   text-decoration: none;
 }
 
-.user_table .user_action a:hover {
+.action_item a:hover {
   color: var(--surface);
   background-color: var(--text);
 }
 
-.user_table .user_action form.button_to.destructive button:hover {
+.action_item form.button_to.destructive button:hover {
   color: var(--on-solid);
   background-color: var(--danger-solid);
 }
 
-.user_table .user_action form.button_to.state_changing button:hover {
+.action_item form.button_to.state_changing button:hover {
   color: var(--on-solid);
   background-color: var(--action-solid);
 }
 
-.user_table .user_action .disabled_action {
+.action_item .disabled_action {
   color: var(--text-muted);
   background-color: transparent;
   border-color: var(--border);
@@ -915,22 +901,39 @@ table.revisions_table tr:hover {
 .flag_stack {
   display: inline-flex;
   flex-direction: column;
-  align-items: center;
-  min-width: 1rem;
+  align-items: flex-start;
   gap: 0.25rem;
   vertical-align: -0.15em;
   margin-right: 0.5rem;
+  width: 1.5rem;
 }
 
 .flag_stack svg {
-  width: 1rem;
-  height: 1rem;
+  width: 1.25rem;
+  height: 1.25rem;
   color: var(--text-muted);
 }
 
 .flag_stack .flag_attention svg { color: var(--accent); }
 .flag_stack .flag_alert svg     { color: var(--danger); }
 
+.title_with_flags {
+  display: flex;
+  align-items: flex-start;
+}
+
+.title_body {
+  min-width: 0;
+}
+
+.node_table .node_title {
+  font-weight: bold;
+}
+
+.node_table .node_path {
+  margin-top: 0.15rem;
+}
+
 .user_group_heading {
   margin-top: 1.5rem;
 }
diff --git a/test/controllers/users_controller_test.rb b/test/controllers/users_controller_test.rb
index aeff9bc7..d06b9298 100644
--- a/test/controllers/users_controller_test.rb
+++ b/test/controllers/users_controller_test.rb
@@ -50,7 +50,7 @@ class UsersControllerTest < ActionController::TestCase
     end
     
     assert_redirected_to user_path(User.last)
-    assert !User.last.admin
+    assert_not User.last.is_admin?
   end
   
   test "creating a Redaktion account" do