Extract drafts/recent query logic from controller into Node

Node.drafts_and_autosaves and Node.recently_changed replace inline
query logic in NodesController#drafts/#recent -- pure refactor, no
behavior change for either action. The real reason: the dashboard's
upcoming abridged widgets need the exact same queries the full pages
already use, just limited and (for drafts) sorted with the current
user's own locked nodes first. Better to share one method than let a
widget and a full page quietly drift onto two versions of "what counts
as a current draft."

current_user_id: is an explicit, optional argument rather than a
separate method -- ordering by lock ownership only applies when a user
is given; omitted entirely, it's pure recency, exactly today's
behavior. Needed Arel.sql wrapping a sanitized CASE expression before
.order would accept it -- sanitize_sql_array only vouches for the
values inside the string, a separate Rails safety check still refuses
any string shaped like more than a plain column reference unless it's
explicitly marked as already-vetted.

3 files changed, 42 insertions, 7 deletions

diff --git a/app/controllers/nodes_controller.rb b/app/controllers/nodes_controller.rb
index 772bf4b..c1468be 100644
--- a/app/controllers/nodes_controller.rb
+++ b/app/controllers/nodes_controller.rb
@@ -185,16 +185,11 @@ class NodesController < ApplicationController
 
   # Filter functions for admin views
   def drafts
-    base = Node.where("draft_id IS NOT NULL OR autosave_id IS NOT NULL")
-    @nodes = index_matching(base)
+    @nodes = index_matching(Node.drafts_and_autosaves)
   end
 
   def recent
-    base = Node.where(
-      "nodes.updated_at < ? AND nodes.updated_at > ? AND nodes.parent_id IS NOT NULL",
-      Time.now, Time.now - 14.days
-    )
-    @nodes = index_matching(base)
+    @nodes = index_matching(Node.recently_changed)
   end
 
   def mine
diff --git a/app/models/node.rb b/app/models/node.rb
index 24f3cd0..aa2f7f3 100644
--- a/app/models/node.rb
+++ b/app/models/node.rb
@@ -367,6 +367,22 @@ class Node < ApplicationRecord
       .distinct
   end
 
+  def self.drafts_and_autosaves(current_user_id: nil)
+    scope = where("draft_id IS NOT NULL OR autosave_id IS NOT NULL")
+    return scope.order("updated_at DESC") unless current_user_id
+
+    scope.order(
+      Arel.sql(sanitize_sql_array(["CASE WHEN locking_user_id = ? THEN 0 ELSE 1 END, updated_at DESC", current_user_id]))
+    )
+  end
+
+  def self.recently_changed
+    where(
+      "nodes.updated_at < ? AND nodes.updated_at > ? AND nodes.parent_id IS NOT NULL",
+      Time.now, Time.now - 14.days
+    )
+  end
+
   protected
     def lock_for! current_user
       self.lock_owner = current_user
diff --git a/test/models/node_test.rb b/test/models/node_test.rb
index 15e908b..de540f8 100644
--- a/test/models/node_test.rb
+++ b/test/models/node_test.rb
@@ -537,4 +537,28 @@ class NodeTest < ActiveSupport::TestCase
     assert_includes Node.editor_search("Backspace Spiegelgraben"), node
     assert_equal 0, Node.editor_search("Backspace Nonexistentstreet").count
   end
+
+  test "drafts_and_autosaves without a user sorts by recency only" do
+    older = Node.root.children.create!(:slug => "drafts_order_older")
+    older.find_or_create_draft(@user1)
+    newer = Node.root.children.create!(:slug => "drafts_order_newer")
+    newer.find_or_create_draft(@user1)
+
+    result = Node.drafts_and_autosaves.to_a
+    assert result.index(newer) < result.index(older)
+  end
+
+  test "drafts_and_autosaves with a user puts their own locked nodes first, regardless of recency" do
+    mine = Node.root.children.create!(:slug => "drafts_order_mine")
+    mine.lock_for_editing!(@user1)
+    mine.autosave!({:title => "mine"}, @user1)
+
+    someone_elses_newer = Node.root.children.create!(:slug => "drafts_order_theirs")
+    other_user = User.find_by_login("quentin")
+    someone_elses_newer.lock_for_editing!(other_user)
+    someone_elses_newer.autosave!({:title => "theirs"}, other_user)
+
+    result = Node.drafts_and_autosaves(current_user_id: @user1.id).to_a
+    assert result.index(mine) < result.index(someone_elses_newer)
+  end
 end