From fd4c9763a94fdf3e696635b2a4706679d8e40f2e Mon Sep 17 00:00:00 2001
From: Anton Khorev <tony29@yandex.ru>
Date: Fri, 18 Jul 2025 11:37:01 +0300
Subject: [PATCH] Reorder paginated items

Sometimes items passed to get_page_items are already ordered. That order needs to be replaced by cursor_column instead of adding cursor_column after the existing order.
---
 .../concerns/pagination_methods.rb            |  6 +-
 config/routes.rb                              |  2 +-
 .../diary_entries_controller_test.rb          | 80 ++++++++++---------
 3 files changed, 48 insertions(+), 40 deletions(-)

diff --git a/app/controllers/concerns/pagination_methods.rb b/app/controllers/concerns/pagination_methods.rb
index ec4556779..28a1bf7b9 100644
--- a/app/controllers/concerns/pagination_methods.rb
+++ b/app/controllers/concerns/pagination_methods.rb
@@ -11,11 +11,11 @@ module PaginationMethods
 
     qualified_cursor_column = "#{items.table_name}.#{cursor_column}"
     page_items = if params[:before]
-                   items.where("#{qualified_cursor_column} < ?", params[:before]).order(cursor_column => :desc)
+                   items.where("#{qualified_cursor_column} < ?", params[:before]).reorder(cursor_column => :desc)
                  elsif params[:after]
-                   items.where("#{qualified_cursor_column} > ?", params[:after]).order(cursor_column => :asc)
+                   items.where("#{qualified_cursor_column} > ?", params[:after]).reorder(cursor_column => :asc)
                  else
-                   items.order(cursor_column => :desc)
+                   items.reorder(cursor_column => :desc)
                  end
 
     page_items = page_items.limit(limit)
diff --git a/config/routes.rb b/config/routes.rb
index b58d71244..e84e77d31 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -274,7 +274,7 @@ OpenStreetMap::Application.routes.draw do
   get "/user/:display_name/diary/rss" => "diary_entries#rss", :defaults => { :format => :rss }
   get "/diary/:language/rss" => "diary_entries#rss", :defaults => { :format => :rss }
   get "/diary/rss" => "diary_entries#rss", :defaults => { :format => :rss }
-  get "/user/:display_name/diary" => "diary_entries#index"
+  get "/user/:display_name/diary" => "diary_entries#index", :as => :user_diary_entries
   get "/diary/:language" => "diary_entries#index"
   scope "/user/:display_name" do
     resources :diary_entries, :path => "diary", :only => [:edit, :update, :show], :id => /\d+/ do
diff --git a/test/controllers/diary_entries_controller_test.rb b/test/controllers/diary_entries_controller_test.rb
index 2051751c2..2ab5e9257 100644
--- a/test/controllers/diary_entries_controller_test.rb
+++ b/test/controllers/diary_entries_controller_test.rb
@@ -468,44 +468,15 @@ class DiaryEntriesControllerTest < ActionDispatch::IntegrationTest
   end
 
   def test_index_paged
-    # Create several pages worth of diary entries
     create_list(:diary_entry, 50)
-    next_path = diary_entries_path
-
-    # Try and get the index
-    get next_path
-    assert_response :success
-    assert_select "article.diary_post", :count => 20
-    check_no_page_link "Newer Entries"
-    next_path = check_page_link "Older Entries"
-
-    # Try and get the second page
-    get next_path
-    assert_response :success
-    assert_select "article.diary_post", :count => 20
-    check_page_link "Newer Entries"
-    next_path = check_page_link "Older Entries"
-
-    # Try and get the third page
-    get next_path
-    assert_response :success
-    assert_select "article.diary_post", :count => 10
-    next_path = check_page_link "Newer Entries"
-    check_no_page_link "Older Entries"
-
-    # Go back to the second page
-    get next_path
-    assert_response :success
-    assert_select "article.diary_post", :count => 20
-    next_path = check_page_link "Newer Entries"
-    check_page_link "Older Entries"
+    check_pagination_of_50_entries diary_entries_path
+  end
 
-    # Go back to the first page
-    get next_path
-    assert_response :success
-    assert_select "article.diary_post", :count => 20
-    check_no_page_link "Newer Entries"
-    check_page_link "Older Entries"
+  def test_index_user_paged
+    user = create(:user)
+    create_list(:diary_entry, 50, :user => user)
+    user.confirm!
+    check_pagination_of_50_entries user_diary_entries_path(user)
   end
 
   def test_index_invalid_paged
@@ -996,6 +967,43 @@ class DiaryEntriesControllerTest < ActionDispatch::IntegrationTest
     end
   end
 
+  def check_pagination_of_50_entries(path)
+    # Try and get the index
+    get path
+    assert_response :success
+    assert_select "article.diary_post", :count => 20
+    check_no_page_link "Newer Entries"
+    path = check_page_link "Older Entries"
+
+    # Try and get the second page
+    get path
+    assert_response :success
+    assert_select "article.diary_post", :count => 20
+    check_page_link "Newer Entries"
+    path = check_page_link "Older Entries"
+
+    # Try and get the third page
+    get path
+    assert_response :success
+    assert_select "article.diary_post", :count => 10
+    path = check_page_link "Newer Entries"
+    check_no_page_link "Older Entries"
+
+    # Go back to the second page
+    get path
+    assert_response :success
+    assert_select "article.diary_post", :count => 20
+    path = check_page_link "Newer Entries"
+    check_page_link "Older Entries"
+
+    # Go back to the first page
+    get path
+    assert_response :success
+    assert_select "article.diary_post", :count => 20
+    check_no_page_link "Newer Entries"
+    check_page_link "Older Entries"
+  end
+
   def check_no_page_link(name)
     assert_select "a.page-link", { :text => /#{Regexp.quote(name)}/, :count => 0 }, "unexpected #{name} page link"
   end
-- 
2.39.5