Replace page numbers with ID based selection for diary indexes

tomhughes · Jul 27, 2023 · f29ba01 · f29ba01
1 parent b57ed0b
commit f29ba01
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 16 deletions.
diff --git a/app/controllers/diary_entries_controller.rb b/app/controllers/diary_entries_controller.rb
@@ -17,48 +17,56 @@ def index
 
       if @user
         @title = t ".user_title", :user => @user.display_name
-        @entries = @user.diary_entries
+        entries = @user.diary_entries
       else
         render_unknown_user params[:display_name]
         return
       end
     elsif params[:friends]
       if current_user
         @title = t ".title_friends"
-        @entries = DiaryEntry.where(:user_id => current_user.friends)
+        entries = DiaryEntry.where(:user_id => current_user.friends)
       else
         require_user
         return
       end
     elsif params[:nearby]
       if current_user
         @title = t ".title_nearby"
-        @entries = DiaryEntry.where(:user_id => current_user.nearby)
+        entries = DiaryEntry.where(:user_id => current_user.nearby)
       else
         require_user
         return
       end
     else
-      @entries = DiaryEntry.joins(:user).where(:users => { :status => %w[active confirmed] })
+      entries = DiaryEntry.joins(:user).where(:users => { :status => %w[active confirmed] })
 
       if params[:language]
         @title = t ".in_language_title", :language => Language.find(params[:language]).english_name
-        @entries = @entries.where(:language_code => params[:language])
+        entries = entries.where(:language_code => params[:language])
       else
         @title = t ".title"
       end
     end
 
+    entries = entries.visible unless can? :unhide, DiaryEntry
+
     @params = params.permit(:display_name, :friends, :nearby, :language)
 
-    @page = (params[:page] || 1).to_i
-    @page_size = 20
+    @entries = if params[:before]
+                 entries.where("diary_entries.id < ?", params[:before]).order(:id => :desc)
+               elsif params[:after]
+                 entries.where("diary_entries.id > ?", params[:after]).order(:id => :asc)
+               else
+                 entries.order(:id => :desc)
+               end
 
-    @entries = @entries.visible unless can? :unhide, DiaryEntry
-    @entries = @entries.order("created_at DESC")
-    @entries = @entries.offset((@page - 1) * @page_size)
-    @entries = @entries.limit(@page_size)
+    @entries = @entries.limit(20)
     @entries = @entries.includes(:user, :language)
+    @entries = @entries.sort.reverse
+
+    @newer_entries = @entries.count.positive? && entries.exists?(["diary_entries.id > ?", @entries.first.id])
+    @older_entries = @entries.count.positive? && entries.exists?(["diary_entries.id < ?", @entries.last.id])
   end
 
   def show

diff --git a/app/views/diary_entries/index.html.erb b/app/views/diary_entries/index.html.erb
@@ -38,19 +38,19 @@
 
   <nav>
     <ul class="pagination">
-      <% if @entries.size >= @page_size -%>
+      <% if @older_entries -%>
         <li class="page-item">
-          <%= link_to t(".older_entries"), @params.merge(:page => @page + 1), :class => "page-link" %>
+          <%= link_to t(".older_entries"), @params.merge(:before => @entries.last.id), :class => "page-link" %>
         </li>
       <% else -%>
         <li class="page-item disabled">
           <span class="page-link"><%= t(".older_entries") %></span>
         </li>
       <% end -%>
 
-      <% if @page > 1 -%>
+      <% if @newer_entries -%>
         <li class="page-item">
-          <%= link_to t(".newer_entries"), @params.merge(:page => @page - 1), :class => "page-link" %>
+          <%= link_to t(".newer_entries"), @params.merge(:after => @entries.first.id), :class => "page-link" %>
         </li>
       <% else -%>
         <li class="page-item disabled">

diff --git a/test/controllers/diary_entries_controller_test.rb b/test/controllers/diary_entries_controller_test.rb
@@ -564,11 +564,36 @@ def test_index_paged
     get diary_entries_path
     assert_response :success
     assert_select "div.diary_post", :count => 20
+    assert_select "li.page-item a.page-link", :text => "Older Entries", :count => 1
+    assert_select "li.page-item.disabled span.page-link", :text => "Newer Entries", :count => 1
 
     # Try and get the second page
-    get diary_entries_path(:page => 2)
+    get css_select("li.page-item a.page-link").first["href"]
     assert_response :success
     assert_select "div.diary_post", :count => 20
+    assert_select "li.page-item a.page-link", :text => "Older Entries", :count => 1
+    assert_select "li.page-item a.page-link", :text => "Newer Entries", :count => 1
+
+    # Try and get the third page
+    get css_select("li.page-item a.page-link").first["href"]
+    assert_response :success
+    assert_select "div.diary_post", :count => 10
+    assert_select "li.page-item.disabled span.page-link", :text => "Older Entries", :count => 1
+    assert_select "li.page-item a.page-link", :text => "Newer Entries", :count => 1
+
+    # Go back to the second page
+    get css_select("li.page-item a.page-link").last["href"]
+    assert_response :success
+    assert_select "div.diary_post", :count => 20
+    assert_select "li.page-item a.page-link", :text => "Older Entries", :count => 1
+    assert_select "li.page-item a.page-link", :text => "Newer Entries", :count => 1
+
+    # Go back to the first page
+    get css_select("li.page-item a.page-link").last["href"]
+    assert_response :success
+    assert_select "div.diary_post", :count => 20
+    assert_select "li.page-item a.page-link", :text => "Older Entries", :count => 1
+    assert_select "li.page-item.disabled span.page-link", :text => "Newer Entries", :count => 1
   end
 
   def test_rss