Add pagination to Issues page

openstreetmap · Aug 13, 2024 · 3814fe2 · 3814fe2
1 parent 1677587
commit 3814fe2
Show file tree

Hide file tree

Showing 5 changed files with 134 additions and 79 deletions.
diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb
@@ -1,4 +1,6 @@
 class IssuesController < ApplicationController
+  include PaginationMethods
+
   layout "site"
 
   before_action :authorize_web
@@ -11,14 +13,17 @@ class IssuesController < ApplicationController
   before_action :check_database_writable, :only => [:resolve, :ignore, :reopen]
 
   def index
+    @params = params.permit(:before, :after, :limit, :status, :search_by_user, :issue_type, :last_updated_by)
+    @params[:limit] ||= 50
     @title = t ".title"
+    @user_not_found = false
 
     @issue_types = []
     @issue_types.push("Note", "User") if current_user.moderator?
     @issue_types.push("DiaryEntry", "DiaryComment", "User") if current_user.administrator?
 
     @users = User.joins(:roles).where(:user_roles => { :role => current_user.roles.map(&:role) }).distinct
-    @issues = Issue.visible_to(current_user).order(:updated_at => :desc)
+    @issues = Issue.visible_to(current_user)
 
     # If search
     if params[:search_by_user].present?
@@ -27,7 +32,7 @@ def index
         @issues = @issues.where(:reported_user => @find_user)
       else
         @issues = @issues.none
-        flash.now[:warning] = t(".user_not_found")
+        @user_not_found = true
       end
     end
 
@@ -39,6 +44,10 @@ def index
       last_updated_by = params[:last_updated_by].to_s == "nil" ? nil : params[:last_updated_by].to_i
       @issues = @issues.where(:updated_by => last_updated_by)
     end
+
+    @issues, @newer_issues_id, @older_issues_id = get_page_items(@issues, :limit => @params[:limit])
+
+    render :partial => "page" if turbo_frame_request_id == "pagination" || turbo_frame_request_id == "search"
   end
 
   def show

diff --git a/app/views/issues/_page.html.erb b/app/views/issues/_page.html.erb
@@ -0,0 +1,40 @@
+<turbo-frame id="pagination" data-turbo-action="advance">
+  <% if @issues.length == 0 %>
+    <p><%= t(@user_not_found ? ".user_not_found" : ".issues_not_found") %></p>
+  <% else %>
+    <table class="table table-sm">
+      <thead>
+        <tr>
+          <th><%= t ".status" %></th>
+          <th><%= t ".reports" %></th>
+          <th><%= t ".reported_item" %></th>
+          <th><%= t ".reported_user" %></th>
+          <th><%= t ".last_updated" %></th>
+        </tr>
+      </thead>
+      <tbody>
+        <% @issues.each do |issue| %>
+          <tr>
+            <td><%= t ".states.#{issue.status}" %></td>
+            <td class="text-nowrap"><%= link_to t(".reports_count", :count => issue.reports_count), issue, "data-turbo-frame" => "_top" %></td>
+            <td><%= link_to reportable_title(issue.reportable), reportable_url(issue.reportable), "data-turbo-frame" => "_top" %></td>
+            <td><%= link_to issue.reported_user.display_name, issue.reported_user, "data-turbo-frame" => "_top" if issue.reported_user %></td>
+            <td>
+              <% if issue.user_updated %>
+                <%= t ".last_updated_time_ago_user_html", :user => link_to(issue.user_updated.display_name, issue.user_updated, "data-turbo-frame" => "_top"),
+                                                          :time_ago => friendly_date_ago(issue.updated_at) %>
+              <% else %>
+                <%= friendly_date_ago(issue.updated_at) %>
+              <% end %>
+            </td>
+          </tr>
+        <% end %>
+      </tbody>
+    </table>
+    <%= render "shared/pagination",
+               :newer_key => "issues.page.newer_issues",
+               :older_key => "issues.page.older_issues",
+               :newer_id => @newer_issues_id,
+               :older_id => @older_issues_id %>
+  <% end %>
+</turbo-frame>
diff --git a/app/views/issues/index.html.erb b/app/views/issues/index.html.erb
@@ -4,72 +4,42 @@
 
 <p><%= t ".search_guidance" %></p>
 
-<%= form_tag(issues_path, :method => :get) do %>
-  <div class="row gx-1">
-    <div class="mb-3 col-md-auto">
-      <%= select_tag :status,
-                     options_for_select(Issue.aasm.states.map(&:name).map { |state| [t(".states.#{state}"), state] }, params[:status]),
-                     :include_blank => t(".select_status"),
-                     :data => { :behavior => "category_dropdown" },
-                     :class => "form-select" %>
+<turbo-frame id="search">
+  <%= form_tag(issues_path, :method => :get, :data => { :turbo_frame => "pagination" }) do %>
+    <div class="row gx-1">
+      <div class="mb-3 col-md-auto">
+        <%= select_tag :status,
+                       options_for_select(Issue.aasm.states.map(&:name).map { |state| [t(".states.#{state}"), state] }, params[:status]),
+                       :include_blank => t(".select_status"),
+                       :data => { :behavior => "category_dropdown" },
+                       :class => "form-select" %>
+      </div>
+      <div class="mb-3 col-md-auto">
+        <%= select_tag :issue_type,
+                       options_for_select(@issue_types, params[:issue_type]),
+                       :include_blank => t(".select_type"),
+                       :data => { :behavior => "category_dropdown" },
+                       :class => "form-select" %>
+      </div>
+      <div class="mb-3 col-md">
+        <%= text_field_tag :search_by_user,
+                           params[:search_by_user],
+                           :placeholder => t(".reported_user"),
+                           :autocomplete => "on",
+                           :class => "form-control" %>
+      </div>
+      <div class="mb-3 col-md-auto">
+        <%= select_tag :last_updated_by,
+                       options_for_select(@users.all.collect { |f| [f.display_name, f.id] } << [t(".not_updated"), "nil"], params[:last_updated_by]),
+                       :include_blank => t(".select_last_updated_by"),
+                       :data => { :behavior => "category_dropdown" },
+                       :class => "form-select" %>
+      </div>
+      <div class="mb-3 col-md-auto">
+        <%= submit_tag t(".search"), :name => nil, :class => "btn btn-primary" %>
+      </div>
     </div>
-    <div class="mb-3 col-md-auto">
-      <%= select_tag :issue_type,
-                     options_for_select(@issue_types, params[:issue_type]),
-                     :include_blank => t(".select_type"),
-                     :data => { :behavior => "category_dropdown" },
-                     :class => "form-select" %>
-    </div>
-    <div class="mb-3 col-md">
-      <%= text_field_tag :search_by_user,
-                         params[:search_by_user],
-                         :placeholder => t(".reported_user"),
-                         :autocomplete => "on",
-                         :class => "form-control" %>
-    </div>
-    <div class="mb-3 col-md-auto">
-      <%= select_tag :last_updated_by,
-                     options_for_select(@users.all.collect { |f| [f.display_name, f.id] } << [t(".not_updated"), "nil"], params[:last_updated_by]),
-                     :include_blank => t(".select_last_updated_by"),
-                     :data => { :behavior => "category_dropdown" },
-                     :class => "form-select" %>
-    </div>
-    <div class="mb-3 col-md-auto">
-      <%= submit_tag t(".search"), :name => nil, :class => "btn btn-primary" %>
-    </div>
-  </div>
-<% end %>
+  <% end %>
+</turbo-frame>
 
-<% if @issues.length == 0 %>
-  <p><%= t ".issues_not_found" %></p>
-<% else %>
-  <table class="table table-sm">
-    <thead>
-      <tr>
-        <th><%= t ".status" %></th>
-        <th><%= t ".reports" %></th>
-        <th><%= t ".reported_item" %></th>
-        <th><%= t ".reported_user" %></th>
-        <th><%= t ".last_updated" %></th>
-      </tr>
-    </thead>
-    <tbody>
-      <% @issues.each do |issue| %>
-        <tr>
-          <td><%= t ".states.#{issue.status}" %></td>
-          <td class="text-nowrap"><%= link_to t(".reports_count", :count => issue.reports_count), issue %></td>
-          <td><%= link_to reportable_title(issue.reportable), reportable_url(issue.reportable) %></td>
-          <td><%= link_to issue.reported_user.display_name, issue.reported_user if issue.reported_user %></td>
-          <td>
-            <% if issue.user_updated %>
-              <%= t ".last_updated_time_ago_user_html", :user => link_to(issue.user_updated.display_name, issue.user_updated),
-                                                        :time_ago => friendly_date_ago(issue.updated_at) %>
-            <% else %>
-              <%= friendly_date_ago(issue.updated_at) %>
-            <% end %>
-          </td>
-        </tr>
-      <% end %>
-    </tbody>
-  </table>
-<% end %>
+<%= render :partial => "page" %>
diff --git a/config/locales/en.yml b/config/locales/en.yml
@@ -1465,13 +1465,19 @@ en:
       not_updated: Not Updated
       search: Search
       search_guidance: "Search Issues:"
+      link_to_reports: View Reports
+      states:
+        ignored: Ignored
+        open: Open
+        resolved: Resolved
+    page:
+      reported_user: Reported User
       user_not_found: User does not exist
       issues_not_found: No such issues found
       status: Status
       reports: Reports
       last_updated: Last Updated
       last_updated_time_ago_user_html: "%{time_ago} by %{user}"
-      link_to_reports: View Reports
       reports_count:
         one: "%{count} Report"
         other: "%{count} Reports"
@@ -1480,6 +1486,8 @@ en:
         ignored: Ignored
         open: Open
         resolved: Resolved
+      older_issues: Older Issues
+      newer_issues: Newer Issues
     show:
       title: "%{status} Issue #%{issue_id}"
       reports:

diff --git a/test/system/issues_test.rb b/test/system/issues_test.rb
@@ -19,7 +19,7 @@ def test_view_no_issues
     sign_in_as(create(:moderator_user))
 
     visit issues_path
-    assert_content I18n.t("issues.index.issues_not_found")
+    assert_content I18n.t("issues.page.issues_not_found")
   end
 
   def test_view_issues
@@ -81,22 +81,22 @@ def test_search_issues_by_user
     visit issues_path
     fill_in "search_by_user", :with => good_user.display_name
     click_on "Search"
-    assert_no_content I18n.t("issues.index.user_not_found")
-    assert_content I18n.t("issues.index.issues_not_found")
+    assert_no_content I18n.t("issues.page.user_not_found")
+    assert_content I18n.t("issues.page.issues_not_found")
 
     # User doesn't exist
     visit issues_path
     fill_in "search_by_user", :with => "Nonexistent User"
     click_on "Search"
-    assert_content I18n.t("issues.index.user_not_found")
-    assert_content I18n.t("issues.index.issues_not_found")
+    assert_content I18n.t("issues.page.user_not_found")
+    assert_no_content I18n.t("issues.page.issues_not_found")
 
     # Find Issue against bad_user
     visit issues_path
     fill_in "search_by_user", :with => bad_user.display_name
     click_on "Search"
-    assert_no_content I18n.t("issues.index.user_not_found")
-    assert_no_content I18n.t("issues.index.issues_not_found")
+    assert_no_content I18n.t("issues.page.user_not_found")
+    assert_no_content I18n.t("issues.page.issues_not_found")
   end
 
   def test_commenting
@@ -158,7 +158,35 @@ def test_issue_index_with_multiple_roles
 
     visit issues_path
 
-    assert_link I18n.t("issues.index.reports_count", :count => issue1.reports_count), :href => issue_path(issue1)
-    assert_link I18n.t("issues.index.reports_count", :count => issue2.reports_count), :href => issue_path(issue2)
+    assert_link I18n.t("issues.page.reports_count", :count => issue1.reports_count), :href => issue_path(issue1)
+    assert_link I18n.t("issues.page.reports_count", :count => issue2.reports_count), :href => issue_path(issue2)
+  end
+
+  def test_issues_pagination
+    1.upto(80).each do
+      user = create(:user)
+      create(:issue, :reportable => user, :reported_user => user, :assigned_role => "administrator")
+    end
+
+    sign_in_as(create(:administrator_user))
+
+    visit issues_path
+
+    # First Page
+    assert_no_content I18n.t("issues.page.user_not_found")
+    assert_no_content I18n.t("issues.page.issues_not_found")
+    assert_css "tr", :count => 51
+
+    # Second Page
+    click_on I18n.t("issues.page.older_issues")
+    assert_no_content I18n.t("issues.page.user_not_found")
+    assert_no_content I18n.t("issues.page.issues_not_found")
+    assert_css "tr", :count => 31, :wait => 1
+
+    # Back to First Page
+    click_on I18n.t("issues.page.newer_issues")
+    assert_no_content I18n.t("issues.page.user_not_found")
+    assert_no_content I18n.t("issues.page.issues_not_found")
+    assert_css "tr", :count => 51, :wait => 1
   end
 end