Fix prev/next links on public profile page (#6497)

* Fix prev/next links on public profile page

* Don't make pagination urls if no available statuses

* Fix empty check method

* Put left chevron before prev page link

* Add scope for pagination "starting at" a given id

* Status pagination try 2:

s/prev/older and s/next/newer
"older" on left, "newer" on right
Use new scope for "newer" link
Extract magic 20 page size to constant
Remove max_id from feed pagination as it's not respected

* Reinstate max_id for accounts atom stream

* normalize

diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb
index 69fd20e27f4a55e0c5d6795046bffd65ecdb17a9..7bf35825f9b93a7560c6bf09b76756603a8b7e01 100644 (file)
--- a/app/controllers/accounts_controller.rb
+++ b/app/controllers/accounts_controller.rb
@@ -1,6 +1,8 @@
 # frozen_string_literal: true
 
 class AccountsController < ApplicationController
+  PAGE_SIZE = 20
+
   include AccountControllerConcern
 
   before_action :set_cache_headers
@@ -16,13 +18,16 @@ class AccountsController < ApplicationController
         end
 
         @pinned_statuses = cache_collection(@account.pinned_statuses, Status) if show_pinned_statuses?
-        @statuses        = filtered_statuses.paginate_by_max_id(20, params[:max_id], params[:since_id])
+        @statuses        = filtered_status_page(params)
         @statuses        = cache_collection(@statuses, Status)
-        @next_url        = next_url unless @statuses.empty?
+        unless @statuses.empty?
+          @older_url        = older_url if @statuses.last.id > filtered_statuses.last.id
+          @newer_url        = newer_url if @statuses.first.id < filtered_statuses.first.id
+        end
       end
 
       format.atom do
-        @entries = @account.stream_entries.where(hidden: false).with_includes.paginate_by_max_id(20, params[:max_id], params[:since_id])
+        @entries = @account.stream_entries.where(hidden: false).with_includes.paginate_by_max_id(PAGE_SIZE, params[:max_id], params[:since_id])
         render xml: OStatus::AtomSerializer.render(OStatus::AtomSerializer.new.feed(@account, @entries.reject { |entry| entry.status.nil? }))
       end
 
@@ -69,13 +74,22 @@ class AccountsController < ApplicationController
     @account = Account.find_local!(params[:username])
   end
 
-  def next_url
+  def older_url
+    ::Rails.logger.info("older: max_id #{@statuses.last.id}, url #{pagination_url(max_id: @statuses.last.id)}")
+    pagination_url(max_id: @statuses.last.id)
+  end
+
+  def newer_url
+    pagination_url(min_id: @statuses.first.id)
+  end
+
+  def pagination_url(max_id: nil, min_id: nil)
     if media_requested?
-      short_account_media_url(@account, max_id: @statuses.last.id)
+      short_account_media_url(@account, max_id: max_id, min_id: min_id)
     elsif replies_requested?
-      short_account_with_replies_url(@account, max_id: @statuses.last.id)
+      short_account_with_replies_url(@account, max_id: max_id, min_id: min_id)
     else
-      short_account_url(@account, max_id: @statuses.last.id)
+      short_account_url(@account, max_id: max_id, min_id: min_id)
     end
   end
 
@@ -86,4 +100,12 @@ class AccountsController < ApplicationController
   def replies_requested?
     request.path.ends_with?('/with_replies')
   end
+
+  def filtered_status_page(params)
+    if params[:min_id].present?
+      filtered_statuses.paginate_by_min_id(PAGE_SIZE, params[:min_id]).reverse
+    else
+      filtered_statuses.paginate_by_max_id(PAGE_SIZE, params[:max_id], params[:since_id]).to_a
+    end
+  end
 end

diff --git a/app/javascript/styles/mastodon/accounts.scss b/app/javascript/styles/mastodon/accounts.scss
index 9015d04cb390549c8fae0cf8c61b648a7065d6c9..c812766a1065d558162f2f829955d13732b3eabc 100644 (file)
--- a/app/javascript/styles/mastodon/accounts.scss
+++ b/app/javascript/styles/mastodon/accounts.scss
@@ -233,8 +233,8 @@
 
   a,
   .current,
-  .next,
-  .prev,
+  .newer,
+  .older,
   .page,
   .gap {
     font-size: 14px;
@@ -257,13 +257,13 @@
     cursor: default;
   }
 
-  .prev,
-  .next {
+  .older,
+  .newer {
     text-transform: uppercase;
     color: $ui-secondary-color;
   }
 
-  .prev {
+  .older {
     float: left;
     padding-left: 0;
 
@@ -273,7 +273,7 @@
     }
   }
 
-  .next {
+  .newer {
     float: right;
     padding-right: 0;
 
@@ -295,8 +295,8 @@
       display: none;
     }
 
-    .next,
-    .prev {
+    .newer,
+    .older {
       display: inline-block;
     }
   }

diff --git a/app/models/concerns/paginable.rb b/app/models/concerns/paginable.rb
index 6061bf9bdf2f06930741bcaa6034e5ee931cbcf5..66695677e9af6990bca0382c2b76d45bc2990a45 100644 (file)
--- a/app/models/concerns/paginable.rb
+++ b/app/models/concerns/paginable.rb
@@ -10,5 +10,14 @@ module Paginable
       query = query.where(arel_table[:id].gt(since_id)) if since_id.present?
       query
     }
+
+    # Differs from :paginate_by_max_id in that it gives the results immediately following min_id,
+    # whereas since_id gives the items with largest id, but with since_id as a cutoff.
+    # Results will be in ascending order by id.
+    scope :paginate_by_min_id, ->(limit, min_id = nil) {
+      query = reorder(arel_table[:id]).limit(limit)
+      query = query.where(arel_table[:id].gt(min_id)) if min_id.present?
+      query
+    }
   end
 end

diff --git a/app/views/accounts/show.html.haml b/app/views/accounts/show.html.haml
index accad5f781fc98936fb7bf743147fb6fb2e27140..21c585daba6e5e01353f3061ce59ab43d324067b 100644 (file)
--- a/app/views/accounts/show.html.haml
+++ b/app/views/accounts/show.html.haml
@@ -39,6 +39,9 @@
 
       = render partial: 'stream_entries/status', collection: @statuses, as: :status
 
-  - if @statuses.size == 20
+  - if @newer_url || @older_url
     .pagination
-      = link_to safe_join([t('pagination.next'), fa_icon('chevron-right')], ' '), @next_url, class: 'next', rel: 'next'
+      - if @older_url
+        = link_to safe_join([fa_icon('chevron-left'), t('pagination.older')], ' '), @older_url, class: 'older', rel: 'older'
+      - if @newer_url
+        = link_to safe_join([t('pagination.newer'), fa_icon('chevron-right')], ' '), @newer_url, class: 'newer', rel: 'newer'

diff --git a/config/locales/en.yml b/config/locales/en.yml
index 071c412909b677d40d7b025ea72025782b5df4cd..026426c84db5e6fb84d2191eeaed483237c7ab23 100644 (file)
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -545,7 +545,9 @@ en:
           trillion: T
           unit: ''
   pagination:
+    newer: Newer
     next: Next
+    older: Older
     prev: Prev
     truncate: "…"
   preferences: