Merge remote-tracking branch 'upstream/pull/4625'

app/controllers/concerns/pagination_methods.rb

@@ -0,0 +1,27 @@
+module PaginationMethods
+  extend ActiveSupport::Concern
+
+  private
+
+  ##
+  # limit selected items to one page, get ids of first item before/after the page
+  def get_page_items(items, includes: [], limit: 20)
+    id_column = "#{items.table_name}.id"
+    page_items = if params[:before]
+                   items.where("#{id_column} < ?", params[:before]).order(:id => :desc)
+                 elsif params[:after]
+                   items.where("#{id_column} > ?", params[:after]).order(:id => :asc)
+                 else
+                   items.order(:id => :desc)
+                 end
+
+    page_items = page_items.limit(limit)
+    page_items = page_items.includes(includes)
+    page_items = page_items.sort.reverse
+
+    newer_items_id = page_items.first.id if page_items.count.positive? && items.exists?(["#{id_column} > ?", page_items.first.id])
+    older_items_id = page_items.last.id if page_items.count.positive? && items.exists?(["#{id_column} < ?", page_items.last.id])
+
+    [page_items, newer_items_id, older_items_id]
+  end
+end

app/controllers/diary_entries_controller.rb

@@ -1,5 +1,6 @@
 class DiaryEntriesController < ApplicationController
   include UserMethods
+  include PaginationMethods
 
   layout "site", :except => :rss
 
@@ -57,7 +58,7 @@ class DiaryEntriesController < ApplicationController
 
     @params = params.permit(:display_name, :friends, :nearby, :language)
 
-    @entries, @newer_entries_id, @older_entries_id = get_page_items(entries, [:user, :language])
+    @entries, @newer_entries_id, @older_entries_id = get_page_items(entries, :includes => [:user, :language])
   end
 
   def show
@@ -246,7 +247,7 @@ class DiaryEntriesController < ApplicationController
 
     @params = params.permit(:display_name, :before, :after)
 
-    @comments, @newer_comments_id, @older_comments_id = get_page_items(comments, [:user])
+    @comments, @newer_comments_id, @older_comments_id = get_page_items(comments, :includes => [:user])
   end
 
   private
@@ -282,24 +283,4 @@ class DiaryEntriesController < ApplicationController
       @zoom = 12
     end
   end
-
-  def get_page_items(items, includes)
-    id_column = "#{items.table_name}.id"
-    page_items = if params[:before]
-                   items.where("#{id_column} < ?", params[:before]).order(:id => :desc)
-                 elsif params[:after]
-                   items.where("#{id_column} > ?", params[:after]).order(:id => :asc)
-                 else
-                   items.order(:id => :desc)
-                 end
-
-    page_items = page_items.limit(20)
-    page_items = page_items.includes(includes)
-    page_items = page_items.sort.reverse
-
-    newer_items_id = page_items.first.id if page_items.count.positive? && items.exists?(["#{id_column} > ?", page_items.first.id])
-    older_items_id = page_items.last.id if page_items.count.positive? && items.exists?(["#{id_column} < ?", page_items.last.id])
-
-    [page_items, newer_items_id, older_items_id]
-  end
 end

app/controllers/traces_controller.rb

@@ -1,5 +1,6 @@
 class TracesController < ApplicationController
   include UserMethods
+  include PaginationMethods
 
   layout "site", :except => :georss
 
@@ -60,20 +61,7 @@ class TracesController < ApplicationController
 
     @params = params.permit(:display_name, :tag, :before, :after)
 
-    @traces = if params[:before]
+    @traces, @newer_traces_id, @older_traces_id = get_page_items(traces, :includes => [:user, :tags])
-                traces.where("gpx_files.id < ?", params[:before]).order(:id => :desc)
-              elsif params[:after]
-                traces.where("gpx_files.id > ?", params[:after]).order(:id => :asc)
-              else
-                traces.order(:id => :desc)
-              end
-
-    @traces = @traces.limit(20)
-    @traces = @traces.includes(:user, :tags)
-    @traces = @traces.sort.reverse
-
-    @newer_traces = @traces.count.positive? && traces.exists?(["gpx_files.id > ?", @traces.first.id])
-    @older_traces = @traces.count.positive? && traces.exists?(["gpx_files.id < ?", @traces.last.id])
 
     # final helper vars for view
     @target_user = target_user

app/controllers/users_controller.rb

@@ -2,6 +2,7 @@ class UsersController < ApplicationController
   include EmailMethods
   include SessionMethods
   include UserMethods
+  include PaginationMethods
 
   layout "site"
 
@@ -29,16 +30,14 @@ class UsersController < ApplicationController
 
       redirect_to url_for(:status => params[:status], :ip => params[:ip], :page => params[:page])
     else
-      @params = params.permit(:status, :ip)
+      @params = params.permit(:status, :ip, :before, :after)
 
-      conditions = {}
+      users = User.all
-      conditions[:status] = @params[:status] if @params[:status]
+      users = users.where(:status => @params[:status]) if @params[:status]
-      conditions[:creation_ip] = @params[:ip] if @params[:ip]
+      users = users.where(:creation_ip => @params[:ip]) if @params[:ip]
 
-      @user_pages, @users = paginate(:users,
+      @users_count = users.count
-                                     :conditions => conditions,
+      @users, @newer_users_id, @older_users_id = get_page_items(users, :limit => 50)
-                                     :order => :id,
-                                     :per_page => 50)
     end
   end
 

app/views/traces/index.html.erb

@@ -68,8 +68,8 @@
   <%= render "shared/pagination",
              :newer_key => "traces.trace_paging_nav.newer",
              :older_key => "traces.trace_paging_nav.older",
-             :newer_id => @newer_traces && @traces.first.id,
+             :newer_id => @newer_traces_id,
-             :older_id => @older_traces && @traces.last.id %>
+             :older_id => @older_traces_id %>
 
   <table id="trace_list" class="table table-borderless table-striped">
     <tbody>
@@ -80,8 +80,8 @@
   <%= render "shared/pagination",
              :newer_key => "traces.trace_paging_nav.newer",
              :older_key => "traces.trace_paging_nav.older",
-             :newer_id => @newer_traces && @traces.first.id,
+             :newer_id => @newer_traces_id,
-             :older_id => @older_traces && @traces.last.id %>
+             :older_id => @older_traces_id %>
 <% else %>
   <h2><%= t ".empty_title" %></h2>
   <p><%= t ".empty_upload_html", :upload_link => link_to(t(".upload_new"), new_trace_path),

app/views/users/index.html.erb

@@ -10,6 +10,19 @@
 
 <% unless @users.empty? %>
   <%= form_tag do %>
+    <div class="row">
+      <div class="col">
+        <%= render "shared/pagination",
+                   :newer_key => "users.index.newer",
+                   :older_key => "users.index.older",
+                   :newer_id => @newer_users_id,
+                   :older_id => @older_users_id %>
+      </div>
+      <div class="col col-auto">
+        <%= t ".found_users", :count => @users_count %>
+      </div>
+    <div>
+
     <%= hidden_field_tag :status, params[:status] if params[:status] %>
     <%= hidden_field_tag :ip, params[:ip] if params[:ip] %>
     <%= hidden_field_tag :page, params[:page] if params[:page] %>
@@ -17,15 +30,6 @@
       <thead>
         <tr>
           <td colspan="2">
-            <%= t ".showing",
-                  :page => @user_pages.current_page.number,
-                  :first_item => @user_pages.current_page.first_item,
-                  :last_item => @user_pages.current_page.last_item,
-                  :items => @user_pages.item_count,
-                  :count => @user_pages.current_page.last_item - @user_pages.current_page.first_item + 1 %>
-            <% if @user_pages.page_count > 1 %>
-            | <%= raw pagination_links_each(@user_pages, {}) { |n| link_to n, @params.merge(:page => n) } %>
-            <% end %>
           </td>
           <td>
             <%= check_box_tag "user_all", "1", false %>
@@ -35,6 +39,19 @@
       <%= render @users %>
     </table>
 
+    <div class="row">
+      <div class="col">
+        <%= render "shared/pagination",
+                   :newer_key => "users.index.newer",
+                   :older_key => "users.index.older",
+                   :newer_id => @newer_users_id,
+                   :older_id => @older_users_id %>
+      </div>
+      <div class="col col-auto">
+        <%= t ".found_users", :count => @users_count %>
+      </div>
+    <div>
+
     <div>
       <%= submit_tag t(".confirm"), :name => "confirm", :class => "btn btn-primary" %>
       <%= submit_tag t(".hide"), :name => "hide", :class => "btn btn-primary" %>

config/locales/en.yml

@@ -2832,9 +2832,11 @@ en:
     index:
       title: Users
       heading: Users
-      showing:
+      older: "Older Users"
-        one: Page %{page} (%{first_item} of %{items})
+      newer: "Newer Users"
-        other: Page %{page} (%{first_item}-%{last_item} of %{items})
+      found_users:
+        one: "%{count} user found"
+        other: "%{count} users found"
       summary_html: "%{name} created from %{ip_address} on %{date}"
       summary_no_ip_html: "%{name} created on %{date}"
       confirm: Confirm Selected Users

test/controllers/users_controller_test.rb

@@ -644,23 +644,44 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
 
     # 100 examples, an administrator, and a granter for the admin.
     assert_equal 102, User.count
+    next_path = users_path
 
-    get users_path
+    get next_path
     assert_response :success
     assert_template :index
     assert_select "table#user_list tbody tr", :count => 50
+    check_no_page_link "Newer Users"
+    next_path = check_page_link "Older Users"
 
-    get users_path, :params => { :page => 2 }
+    get next_path
     assert_response :success
     assert_template :index
     assert_select "table#user_list tbody tr", :count => 50
+    check_page_link "Newer Users"
+    next_path = check_page_link "Older Users"
 
-    get users_path, :params => { :page => 3 }
+    get next_path
     assert_response :success
     assert_template :index
     assert_select "table#user_list tbody tr", :count => 2
+    check_page_link "Newer Users"
+    check_no_page_link "Older Users"
   end
 
+  private
+
+  def check_no_page_link(name)
+    assert_select "a.page-link", { :text => /#{Regexp.quote(name)}/, :count => 0 }, "unexpected #{name} page link"
+  end
+
+  def check_page_link(name)
+    assert_select "a.page-link", { :text => /#{Regexp.quote(name)}/ }, "missing #{name} page link" do |buttons|
+      return buttons.first.attributes["href"].value
+    end
+  end
+
+  public
+
   def test_index_post_confirm
     inactive_user = create(:user, :pending)
     suspended_user = create(:user, :suspended)