Merge pull request #1254 from basecamp/pagination-improvements

Pagination improvements

app/assets/stylesheets/card-columns.css

@@ -80,7 +80,7 @@
       min-inline-size: 0;
 
       .blank-slate,
-      .pagination-link,
+      .pagination-link.pagination-link--active-when-observed,
       .card {
         display: none;
       }

app/controllers/searches_controller.rb

@@ -11,7 +11,7 @@ class SearchesController < ApplicationController
 
   private
     def perform_search
-      @search_results = Current.user.search(@query_terms).limit(50)
+      set_page_and_extract_portion_from Current.user.search(@query_terms)
       @recent_search_queries = Current.user.search_queries.order(updated_at: :desc).limit(10)
     end
 end

app/helpers/pagination_helper.rb

@@ -3,14 +3,14 @@ module PaginationHelper
     turbo_frame_tag pagination_frame_id_for(namespace, page.number), data: { timeline_target: "frame", **data }, role: "presentation", **attributes, &
   end
 
-  def link_to_next_page(namespace, page, activate_when_observed: false, label: "Load more...", data: {}, **attributes)
+  def link_to_next_page(namespace, page, activate_when_observed: false, label: default_pagination_label(activate_when_observed), data: {}, **attributes)
     if page.before_last? && !params[:previous]
       pagination_link(namespace, page.number + 1, label: label, activate_when_observed: activate_when_observed, data: data, class: "btn txt-small center-block center", **attributes)
     end
   end
 
-  def pagination_link(namespace, page_number, label: spinner_tag, activate_when_observed: false, url_params: {}, data: {}, **attributes)
-    link_to label, url_for(page: page_number, **url_params),
+  def pagination_link(namespace, page_number, activate_when_observed: false, label: default_pagination_label(activate_when_observed), url_params: {}, data: {}, **attributes)
+    link_to label, url_for(params.permit!.to_h.merge(page: page_number, **url_params)),
       "aria-label": "Load page #{page_number}",
       class: class_names(attributes.delete(:class), "pagination-link", { "pagination-link--active-when-observed" => activate_when_observed }),
       data: {
@@ -55,7 +55,7 @@ module PaginationHelper
   def day_timeline_pagination_link(day_timeline, filter)
     if day_timeline.next_day
       link_to "Load more…", events_days_path(day: day_timeline.next_day.strftime("%Y-%m-%d"), **filter.as_params),
-      class: "txt-reversed", data: { frame: day_timeline_pagination_frame_id_for(day_timeline.next_day), pagination_target: "paginationLink" }
+        class: "txt-reversed", data: { frame: day_timeline_pagination_frame_id_for(day_timeline.next_day), pagination_target: "paginationLink" }
     end
   end
 
@@ -67,4 +67,8 @@ module PaginationHelper
         data: { controller: "pagination", pagination_paginate_on_intersection_value: paginate_on_scroll },
         &block
     end
+
+    def default_pagination_label(activate_when_observed)
+      "Load more…"
+    end
 end

app/views/collections/columns/closeds/show.html.erb

@@ -20,7 +20,7 @@
 <section class="cards cards--grid">
   <%= turbo_frame_tag :closed_column do %>
     <% if @page.used? %>
-      <%= with_manual_pagination :closed_column, @page do %>
+      <%= with_automatic_pagination :closed_column, @page do %>
         <%= render "collections/columns/list", cards: @page.records, draggable: true %>
       <% end %>
     <% else %>

app/views/collections/columns/not_nows/show.html.erb

@@ -20,7 +20,7 @@
 <section class="cards cards--grid">
   <%= turbo_frame_tag :not_now_column do %>
     <% if @page.used? %>
-      <%= with_manual_pagination :not_now_column, @page do %>
+      <%= with_automatic_pagination :not_now_column, @page do %>
         <%= render "collections/columns/list", cards: @page.records, draggable: true %>
       <% end %>
     <% else %>

app/views/collections/columns/show.html.erb

@@ -20,7 +20,7 @@
 <section class="cards cards--grid">
   <%= turbo_frame_tag dom_id(@column, :cards) do %>
     <% if @page.used? %>
-      <%= with_manual_pagination dom_id(@column, :cards), @page do %>
+      <%= with_automatic_pagination dom_id(@column, :cards), @page do %>
         <%= render "collections/columns/list", cards: @page.records, draggable: true %>
       <% end %>
     <% else %>

app/views/collections/columns/streams/show.html.erb

@@ -20,7 +20,7 @@
 <section class="cards cards--grid">
   <%= turbo_frame_tag :stream_column do %>
     <% if @page.used? %>
-      <%= with_manual_pagination :stream_column, @page do %>
+      <%= with_automatic_pagination :stream_column, @page do %>
         <%= render "collections/columns/list", cards: @page.records, draggable: true %>
       <% end %>
     <% else %>
@@ -29,4 +29,4 @@
       </div>
     <% end %>
   <% end %>
-</section>
+</section>

app/views/collections/show/_filtered_cards.html.erb

@@ -1,3 +1,5 @@
 <section class="cards cards--grid">
-  <%= render partial: "cards/display/preview", collection: page.records, as: :card, locals: { draggable: true }, cached: ->(card) { cacheable_preview_parts_for(card) } %>
-</section>
+  <%= with_automatic_pagination :filtered_cards_paginated_container, page do %>
+    <%= render partial: "cards/display/preview", collection: page.records, as: :card, locals: { draggable: true }, cached: ->(card) { cacheable_preview_parts_for(card) } %>
+  <% end %>
+</section>

app/views/collections/show/_stream.html.erb

@@ -16,7 +16,7 @@
     <%= render "columns/show/add_card_button", collection: collection %>
 
     <% if page.used? %>
-      <%= with_manual_pagination "the-stream", @page do %>
+      <%= with_automatic_pagination "the-stream", @page do %>
         <%= render "collections/columns/list", cards: page.records, draggable: true %>
       <% end %>
     <% end %>

app/views/public/collections/columns/closeds/show.html.erb

@@ -1,6 +1,6 @@
 <%= turbo_frame_tag :closed_column do %>
   <% if @page.used? %>
-    <%= with_manual_pagination :closed_column, @page do %>
+    <%= with_automatic_pagination :closed_column, @page do %>
       <%= render "public/collections/columns/list", cards: @page.records %>
     <% end %>
   <% else %>
@@ -8,4 +8,4 @@
       <p class="txt-align-center txt-normal">No cards here</p>
     </div>
   <% end %>
-<% end %>
+<% end %>

app/views/public/collections/columns/not_nows/show.html.erb

@@ -1,6 +1,6 @@
 <%= turbo_frame_tag :not_now_column do %>
   <% if @page.used? %>
-    <%= with_manual_pagination :not_now_column, @page do %>
+    <%= with_automatic_pagination :not_now_column, @page do %>
       <%= render "public/collections/columns/list", cards: @page.records %>
     <% end %>
   <% else %>
@@ -8,4 +8,4 @@
       <p class="txt-align-center txt-normal">No cards here</p>
     </div>
   <% end %>
-<% end %>
+<% end %>

app/views/public/collections/columns/show.html.erb

@@ -1,6 +1,6 @@
 <%= turbo_frame_tag dom_id(@column, :cards) do %>
   <% if @page.used? %>
-    <%= with_manual_pagination dom_id(@column, :cards), @page do %>
+    <%= with_automatic_pagination dom_id(@column, :cards), @page do %>
       <%= render "public/collections/columns/list", cards: @page.records %>
     <% end %>
   <% else %>
@@ -8,4 +8,4 @@
       <p class="txt-align-center txt-normal">No cards here</p>
     </div>
   <% end %>
-<% end %>
+<% end %>

app/views/public/collections/columns/streams/show.html.erb

@@ -1,6 +1,6 @@
 <%= turbo_frame_tag :stream_column do %>
   <% if @page.used? %>
-    <%= with_manual_pagination :stream_column, @page do %>
+    <%= with_automatic_pagination :stream_column, @page do %>
       <%= render "public/collections/columns/list", cards: @page.records %>
     <% end %>
   <% else %>
@@ -8,4 +8,4 @@
       <p class="txt-align-center txt-normal">No cards here</p>
     </div>
   <% end %>
-<% end %>
+<% end %>

app/views/public/collections/show/_stream.html.erb

@@ -3,7 +3,7 @@
   <div class="cards__decoration"></div>
 
   <% if page.used? %>
-    <%= with_manual_pagination "the-stream", @page do %>
+    <%= with_automatic_pagination "the-stream", @page do %>
       <%= render "public/collections/columns/list", cards: page.records %>
     <% end %>
   <% end %>

app/views/searches/_results.html.erb

@@ -7,14 +7,16 @@
 
 <section class="search">
   <div class="search__results">
-    <% if search_results.none? %>
+    <% unless page.used? %>
       <div class="search__empty">
         No matches
       </div>
     <% end %>
 
     <ul class="search__list">
-      <%= render partial: "searches/result", collection: search_results %>
+      <%= with_automatic_pagination :filtered_search_results, page do %>
+        <%= render partial: "searches/result", collection: page.records %>
+      <% end %>
     </ul>
   </div>
 </section>

app/views/searches/show.html.erb

@@ -2,6 +2,6 @@
   <% if @card %>
     <%= auto_submit_form_with url: card_path(@card), method: :get, data: { turbo_action: "advance", turbo_frame: "_top", search_redirect: true } %>
   <% else %>
-    <%= render "results", search_results: @search_results %>
+    <%= render "results", page: @page %>
   <% end %>
 <% end %>

config/brakeman.ignore

@@ -7,7 +7,7 @@
       "check_name": "SQL",
       "message": "Possible SQL injection",
       "file": "app/models/concerns/searchable.rb",
-      "line": 28,
+      "line": 43,
       "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/",
       "code": "joins(\"join #{using} idx on #{table_name}.id = idx.rowid\")",
       "render_path": null,
@@ -30,7 +30,7 @@
       "check_name": "SQL",
       "message": "Possible SQL injection",
       "file": "app/models/concerns/searchable.rb",
-      "line": 31,
+      "line": 46,
       "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/",
       "code": "joins(\"join #{using} idx on #{table_name}.id = idx.rowid\").where(\"#{using} match ?\", Search::Query.wrap(query).to_s)",
       "render_path": null,
@@ -46,6 +46,29 @@
       ],
       "note": ""
     },
+    {
+      "warning_type": "Mass Assignment",
+      "warning_code": 70,
+      "fingerprint": "ac0fa1970dea215e2f47a5676ffd63d8728951e361da12a53dd424bf6dc46f2a",
+      "check_name": "MassAssignment",
+      "message": "Specify exact keys allowed for mass assignment instead of using `permit!` which allows any keys",
+      "file": "app/helpers/pagination_helper.rb",
+      "line": 13,
+      "link": "https://brakemanscanner.org/docs/warning_types/mass_assignment/",
+      "code": "params.permit!",
+      "render_path": null,
+      "location": {
+        "type": "method",
+        "class": "PaginationHelper",
+        "method": "pagination_link"
+      },
+      "user_input": null,
+      "confidence": "Medium",
+      "cwe_id": [
+        915
+      ],
+      "note": ""
+    },
     {
       "warning_type": "Remote Code Execution",
       "warning_code": 24,
@@ -70,5 +93,5 @@
       "note": ""
     }
   ],
-  "brakeman_version": "7.0.2"
+  "brakeman_version": "7.1.0"
 }