Merge pull request Shopify#371 from Shopify/remove-pagy

Remove Pagy in favour of cursor-based pagination

Gemfile.lock

@@ -6,7 +6,6 @@ PATH
       activejob (>= 6.0)
       activerecord (>= 6.0)
       job-iteration (~> 1.1)
-      pagy (~> 3.9)
       railties (>= 6.0)
 
 GEM
@@ -112,7 +111,6 @@ GEM
     nokogiri (1.11.1)
       mini_portile2 (~> 2.5.0)
       racc (~> 1.4)
-    pagy (3.11.0)
     parallel (1.20.1)
     parser (2.7.2.0)
       ast (~> 2.4.1)

app/controllers/maintenance_tasks/application_controller.rb

@@ -5,8 +5,6 @@ module MaintenanceTasks
   #
   # Can be extended to add different authentication and authorization code.
   class ApplicationController < ActionController::Base
-    include Pagy::Backend
-
     BULMA_CDN = 'https://cdn.jsdelivr.net'
 
     content_security_policy do |policy|

app/controllers/maintenance_tasks/tasks_controller.rb

@@ -20,7 +20,7 @@ def index
     def show
       @task = TaskData.find(params.fetch(:id))
       set_refresh if @task.last_run&.active?
-      @pagy, @previous_runs = pagy(@task.previous_runs)
+      @runs_page = RunsPage.new(@task.previous_runs, params[:cursor])
     end
 
     # Runs a given Task and redirects to the Task page.

app/helpers/maintenance_tasks/application_helper.rb

@@ -4,17 +4,6 @@ module MaintenanceTasks
   #
   # @api private
   module ApplicationHelper
-    include Pagy::Frontend
-
-    # Renders pagination for the page, if there is more than one page present.
-    #
-    # @param pagy [Pagy] the pagy instance containing pagination details,
-    #   including the number of pages the results are spread across.
-    # @return [String] the HTML to render for pagination.
-    def pagination(pagy)
-      raw(pagy_bulma_nav(pagy)) if pagy.pages > 1
-    end
-
     # Renders a time element with the given datetime, worded as relative to the
     # current time.
     #

app/models/maintenance_tasks/runs_page.rb

@@ -0,0 +1,55 @@
+# frozen_string_literal: true
+module MaintenanceTasks
+  # This class is responsible for handling cursor-based pagination for Run
+  # records.
+  #
+  # @api private
+  class RunsPage
+    # The number of Runs to show on a single Task page.
+    RUNS_PER_PAGE = 20
+
+    # Initializes a Runs Page with a Runs relation and a cursor. This page is
+    # used by the views to render a set of Runs.
+    # @param runs [ActiveRecord::Relation<MaintenanceTasks::Run>] the relation
+    #   of Run records to be paginated.
+    # @param cursor [String, nil] the id that serves as the cursor when
+    #   querying the Runs dataset to produce a page of Runs. If nil, the first
+    #   Runs in the relation are used.
+    def initialize(runs, cursor)
+      @runs = runs
+      @cursor = cursor
+    end
+
+    # Returns the records for a Page, taking into account the cursor if one is
+    # present. Limits the number of records to 20.
+    #
+    # @return [ActiveRecord::Relation<MaintenanceTasks::Run>] a limited amount
+    #  of Run records.
+    def records
+      @records ||= begin
+        runs_after_cursor = if @cursor.present?
+          @runs.where('id < ?', @cursor)
+        else
+          @runs
+        end
+        runs_after_cursor.limit(RUNS_PER_PAGE)
+      end
+    end
+
+    # Returns the cursor to use for the next Page of Runs. It is the id of the
+    # last record on the current Page.
+    #
+    # @return [Integer] the id of the last record for the Page.
+    def next_cursor
+      records.last.id
+    end
+
+    # Returns whether this Page is the last one.
+    #
+    # @return [Boolean] whether this Page contains the last Run record in the
+    #   Runs dataset that is being paginated.
+    def last?
+      @runs.unscope(:includes).pluck(:id).last == next_cursor
+    end
+  end
+end

app/views/maintenance_tasks/tasks/show.html.erb

@@ -41,12 +41,12 @@
   <pre><code><%= highlight_code(code) %></code></pre>
 <% end %>
 
-<% if @previous_runs.present? %>
+<% if @runs_page.records.present? %>
   <hr/>
 
   <h4 class="title is-4">Previous Runs</h4>
 
-  <%= render @previous_runs %>
+  <%= render @runs_page.records %>
 
-  <%= pagination(@pagy) %>
+  <%= link_to "Next page", task_path(@task, cursor: @runs_page.next_cursor) unless @runs_page.last? %>
 <% end %>

lib/maintenance_tasks.rb

@@ -6,8 +6,6 @@
 
 require 'job-iteration'
 require 'maintenance_tasks/engine'
-require 'pagy'
-require 'pagy/extras/bulma'
 
 # Force the TaskJob class to load so we can verify upstream compatibility with
 # the JobIteration gem

maintenance_tasks.gemspec

@@ -26,6 +26,5 @@ Gem::Specification.new do |spec|
   spec.add_dependency('activejob', '>= 6.0')
   spec.add_dependency('activerecord', '>= 6.0')
   spec.add_dependency('job-iteration', '~> 1.1')
-  spec.add_dependency('pagy', '~> 3.9')
   spec.add_dependency('railties', '>= 6.0')
 end

test/helpers/maintenance_tasks/application_helper_test.rb

@@ -3,20 +3,6 @@
 
 module MaintenanceTasks
   class ApplicationHelperTest < ActionView::TestCase
-    setup { @pagy = mock }
-
-    test '#pagination returns nil if pages is less than or equal to 1' do
-      @pagy.expects(pages: 1)
-      expects(:pagy_bulma_nav).never
-      assert_nil pagination(@pagy)
-    end
-
-    test '#pagination returns pagination element if pages is greater than 1' do
-      @pagy.expects(pages: 2)
-      expects(:pagy_bulma_nav).with(@pagy).returns('pagination')
-      assert_equal 'pagination', pagination(@pagy)
-    end
-
     test '#time_ago returns a time element with the given datetime worded as relative to now and ISO 8601 UTC time in title attribute' do
       travel_to Time.zone.local(2020, 1, 9, 9, 41, 44)
       time = Time.zone.local(2020, 01, 01, 01, 00, 00)

test/models/maintenance_tasks/runs_page_test.rb

@@ -0,0 +1,48 @@
+# frozen_string_literal: true
+
+require 'test_helper'
+
+module MaintenanceTasks
+  class RunsPageTest < ActiveSupport::TestCase
+    setup do
+      @task_name = 'Maintenance::TestTask'
+      21.times do
+        Run.create!(
+          task_name: @task_name,
+          started_at: Time.now,
+          tick_count: 10,
+          tick_total: 10,
+          status: :succeeded,
+          ended_at: Time.now
+        )
+      end
+      @runs = Run.where(task_name: @task_name).order(created_at: :desc)
+    end
+
+    test '#records returns the most recent 20 runs when there is no cursor' do
+      runs_page = RunsPage.new(@runs, nil)
+      assert_equal @runs.first(20), runs_page.records
+    end
+
+    test '#records returns 20 runs after cursor if one is present' do
+      runs_page = RunsPage.new(@runs, @runs.first.id)
+      assert_equal @runs.last(20), runs_page.records
+    end
+
+    test '#next_cursor returns the id of the last run in the record set' do
+      last_id = @runs.last.id
+      runs_page = RunsPage.new(@runs, @runs.first.id)
+      assert_equal last_id, runs_page.next_cursor
+    end
+
+    test '#last? returns true if the last run in the record set is the last run for the relation' do
+      runs_page = RunsPage.new(@runs, @runs.first.id)
+      assert_predicate runs_page, :last?
+    end
+
+    test '#last? returns false if the last run in the record set is not the last run for the relation' do
+      runs_page = RunsPage.new(@runs, nil)
+      refute_predicate runs_page, :last?
+    end
+  end
+end

test/system/maintenance_tasks/tasks_test.rb

@@ -40,6 +40,38 @@ class TasksTest < ApplicationSystemTestCase
       assert_text 'Ran for less than 5 seconds, finished 8 days ago.'
     end
 
+    test 'view a Task with multiple pages of Runs' do
+      Run.create!(
+        task_name: 'Maintenance::TestTask',
+        created_at: 1.hour.ago,
+        started_at: 1.hour.ago,
+        tick_count: 2,
+        tick_total: 10,
+        status: :errored,
+        ended_at: 1.hour.ago
+      )
+      21.times do |i|
+        Run.create!(
+          task_name: 'Maintenance::TestTask',
+          created_at: i.minutes.ago,
+          started_at: i.minutes.ago,
+          tick_count: 10,
+          tick_total: 10,
+          status: :succeeded,
+          ended_at: i.minutes.ago
+        )
+      end
+
+      visit maintenance_tasks_path
+
+      click_on('Maintenance::TestTask')
+      assert_no_text 'Errored'
+
+      click_on('Next page')
+      assert_text 'Errored'
+      assert_no_link 'Next page'
+    end
+
     test 'show a deleted Task' do
       visit maintenance_tasks_path + '/tasks/Maintenance::DeletedTask'