diff --git a/Gemfile.lock b/Gemfile.lock index c19c6a6d..f004cd78 100644 --- a/Gemfile.lock +++ b/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) diff --git a/app/controllers/maintenance_tasks/application_controller.rb b/app/controllers/maintenance_tasks/application_controller.rb index e15c030c..56b9e02a 100644 --- a/app/controllers/maintenance_tasks/application_controller.rb +++ b/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| diff --git a/app/controllers/maintenance_tasks/tasks_controller.rb b/app/controllers/maintenance_tasks/tasks_controller.rb index 7a05c86c..b4c3ebe6 100644 --- a/app/controllers/maintenance_tasks/tasks_controller.rb +++ b/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. diff --git a/app/helpers/maintenance_tasks/application_helper.rb b/app/helpers/maintenance_tasks/application_helper.rb index 2a610dda..352e88b4 100644 --- a/app/helpers/maintenance_tasks/application_helper.rb +++ b/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. # diff --git a/app/models/maintenance_tasks/runs_page.rb b/app/models/maintenance_tasks/runs_page.rb new file mode 100644 index 00000000..18019530 --- /dev/null +++ b/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] 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] 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 diff --git a/app/views/maintenance_tasks/tasks/show.html.erb b/app/views/maintenance_tasks/tasks/show.html.erb index 3c6494ca..0893c213 100644 --- a/app/views/maintenance_tasks/tasks/show.html.erb +++ b/app/views/maintenance_tasks/tasks/show.html.erb @@ -41,12 +41,12 @@
<%= highlight_code(code) %>
<% end %> -<% if @previous_runs.present? %> +<% if @runs_page.records.present? %>

Previous Runs

- <%= 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 %> diff --git a/lib/maintenance_tasks.rb b/lib/maintenance_tasks.rb index ce9ce301..590e92b3 100644 --- a/lib/maintenance_tasks.rb +++ b/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 diff --git a/maintenance_tasks.gemspec b/maintenance_tasks.gemspec index 723975d1..e040abd8 100644 --- a/maintenance_tasks.gemspec +++ b/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 diff --git a/test/helpers/maintenance_tasks/application_helper_test.rb b/test/helpers/maintenance_tasks/application_helper_test.rb index c4bae936..60afa2f5 100644 --- a/test/helpers/maintenance_tasks/application_helper_test.rb +++ b/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) diff --git a/test/models/maintenance_tasks/runs_page_test.rb b/test/models/maintenance_tasks/runs_page_test.rb new file mode 100644 index 00000000..f4d996de --- /dev/null +++ b/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 diff --git a/test/system/maintenance_tasks/tasks_test.rb b/test/system/maintenance_tasks/tasks_test.rb index cc3426c5..29803441 100644 --- a/test/system/maintenance_tasks/tasks_test.rb +++ b/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'