diff --git a/app/assets/stylesheets/application.scss b/app/assets/stylesheets/application.scss index 7616d273a..a6ef809c1 100644 --- a/app/assets/stylesheets/application.scss +++ b/app/assets/stylesheets/application.scss @@ -20,6 +20,7 @@ $govuk-page-width: 1140px; @import 'govuk_publishing_components/components/layout-header'; @import 'govuk_publishing_components/components/lead-paragraph'; @import 'govuk_publishing_components/components/notice'; +@import 'govuk_publishing_components/components/previous-and-next-navigation'; @import 'govuk_publishing_components/components/search'; @import 'govuk_publishing_components/components/skip-link'; @import 'govuk_publishing_components/components/success-alert'; diff --git a/app/controllers/root_controller.rb b/app/controllers/root_controller.rb index 2021abc18..156b65c86 100644 --- a/app/controllers/root_controller.rb +++ b/app/controllers/root_controller.rb @@ -27,12 +27,13 @@ def index assigned_to_filter: assignee_filter, format_filter:, title_filter:, + page: filter_params_hash[:page], ) end private def filter_params - params.permit(:assignee_filter, :format_filter, :title_filter, states_filter: []) + params.permit(:page, :assignee_filter, :format_filter, :title_filter, states_filter: []) end end diff --git a/app/helpers/pagination_helper.rb b/app/helpers/pagination_helper.rb new file mode 100644 index 000000000..d8f018733 --- /dev/null +++ b/app/helpers/pagination_helper.rb @@ -0,0 +1,158 @@ +module PaginationHelper + class << self + LARGE_NUMBER_OF_PAGES = 6 + + def pagination_hash(current_page:, total_pages:, path:) + return if total_pages == 1 + + self.current_page = current_page + self.total_pages = total_pages + self.path = normalise_path(path, current_page) + + previous_page = current_page - 1 if current_page >= 2 + next_page = current_page + 1 if current_page < total_pages + + previous_page_href = build_path_for(current_page - 1) + self.previous_href = previous_page.present? ? previous_page_href : nil + + next_page_href = build_path_for(current_page + 1) if next_page.present? + self.next_href = next_page_href.presence + + if total_pages >= LARGE_NUMBER_OF_PAGES + large_page_number_hash + else + small_page_number_hash + end + end + + private + + attr_accessor :path, :previous_href, :next_href, :current_page, :total_pages + + def normalise_path(path, current_page) + if url_has_page_param?(path) + path + elsif url_has_a_query_string?(path) && !url_has_an_anchor?(path) + path + "&page=#{current_page}" + elsif url_has_a_query_string?(path) && url_has_an_anchor?(path) + path.gsub("#", "&page=#{current_page}#") + elsif url_has_an_anchor?(path) + path.gsub("#", "?page=#{current_page}#") + else + path + "?page=#{current_page}" + end + end + + def url_has_page_param?(path) + path.include?("page=#{current_page}") + end + + def url_has_a_query_string?(path) + path.include?("?") + end + + def url_has_an_anchor?(path) + path.include?("#") + end + + def build_path_for(page) + path.gsub("page=#{current_page}", "page=#{page}") + end + + def small_page_number_hash + items = [] + + [*1..total_pages].map do |page| + items << { + href: build_path_for(page), + current: page == current_page, + } + end + + { + previous_href:, + next_href:, + items:, + } + end + + def large_page_number_hash + items = [ + first_page_hash, + first_elipsis_hash, + middle_pages_array, + second_elipsis_hash, + last_page_hash, + ] + .flatten + .compact + + { + previous_href:, + next_href:, + items:, + } + end + + def first_page_hash + { + href: build_path_for(1), + label: "1", + current: current_page == 1, + aria_label: "Page 1", + } + end + + def first_elipsis_hash + return unless current_page >= 4 + + { ellipses: true } + end + + def middle_pages_array + get_page_numbers.map do |page| + { + href: build_path_for(page), + label: page.to_s, + current: current_page == page, + aria_label: "Page #{page}", + } + end + end + + def get_page_numbers + first_page = 1 + second_page = 2 + penultimate_page = total_pages - 1 + last_page = total_pages + + case current_page + when first_page + [first_page + 1, first_page + 2, first_page + 3] + when second_page + [second_page, second_page + 1, second_page + 2] + when last_page + [last_page - 3, last_page - 2, last_page - 1] + when penultimate_page + [penultimate_page - 2, penultimate_page - 1, penultimate_page] + else + [current_page - 1, current_page, current_page + 1] + end + end + + def second_elipsis_hash + return unless total_pages - current_page >= 3 + + { ellipses: true } + end + + def last_page_hash + { + href: build_path_for(total_pages), + label: total_pages.to_s, + current: current_page == total_pages, + aria_label: "Page #{total_pages}", + } + end + end +end diff --git a/app/presenters/filtered_editions_presenter.rb b/app/presenters/filtered_editions_presenter.rb index c8540e9a8..b0dc99245 100644 --- a/app/presenters/filtered_editions_presenter.rb +++ b/app/presenters/filtered_editions_presenter.rb @@ -3,11 +3,12 @@ class FilteredEditionsPresenter ITEMS_PER_PAGE = 20 - def initialize(states_filter: [], assigned_to_filter: nil, format_filter: nil, title_filter: nil) + def initialize(states_filter: [], assigned_to_filter: nil, format_filter: nil, title_filter: nil, page: nil) @states_filter = states_filter || [] @assigned_to_filter = assigned_to_filter @format_filter = format_filter @title_filter = title_filter + @page = page end def available_users @@ -20,9 +21,7 @@ def editions result = apply_assigned_to_filter(result) result = apply_title_filter(result) result = result.where.not(_type: "PopularLinksEdition") - # Sets a temporary limit of one page and twenty items - # Pagination to follow - result.page(1).per(ITEMS_PER_PAGE) + result.order_by(%w[updated_at desc]).page(@page).per(ITEMS_PER_PAGE) end private @@ -61,5 +60,5 @@ def apply_title_filter(editions) editions.title_contains(title_filter) end - attr_reader :states_filter, :assigned_to_filter, :format_filter, :title_filter + attr_reader :states_filter, :assigned_to_filter, :format_filter, :title_filter, :page end diff --git a/app/views/components/_pagination.html.erb b/app/views/components/_pagination.html.erb new file mode 100644 index 000000000..fb37b40cc --- /dev/null +++ b/app/views/components/_pagination.html.erb @@ -0,0 +1,60 @@ +<% + id ||= "pagination-#{SecureRandom.hex(4)}" + aria_label ||= "Pagination" + items ||= [] + previous_href ||= false + next_href ||= false +%> + + diff --git a/app/views/kaminari/_paginator.html.erb b/app/views/kaminari/_paginator.html.erb new file mode 100644 index 000000000..b5a6e2a0e --- /dev/null +++ b/app/views/kaminari/_paginator.html.erb @@ -0,0 +1,11 @@ +<% anchor = anchor.presence || "" %> + +<% if total_pages > 1 %> + <% PaginationHelper.pagination_hash(current_page: current_page.to_i, total_pages:, path: request.url + anchor).tap do |hash| %> + <%= render "components/pagination", { + previous_href: hash[:previous_href], + next_href: hash[:next_href], + items: hash[:items], + } %> + <% end %> +<% end %> diff --git a/app/views/root/index.html.erb b/app/views/root/index.html.erb index b5d2488a5..f14aa23a7 100644 --- a/app/views/root/index.html.erb +++ b/app/views/root/index.html.erb @@ -21,4 +21,5 @@
<%= render :partial => "table" %> + <%= paginate @presenter.editions %>
diff --git a/test/functional/root_controller_test.rb b/test/functional/root_controller_test.rb index cb20d6fb5..52a0a8eb4 100644 --- a/test/functional/root_controller_test.rb +++ b/test/functional/root_controller_test.rb @@ -56,10 +56,30 @@ class RootControllerTest < ActionController::TestCase should "ignore unrecognised filter states" do FilteredEditionsPresenter .expects(:new) - .with(states_filter: %w[draft], assigned_to_filter: anything, format_filter: anything, title_filter: anything) - .returns(stub(editions: [], available_users: [])) + .with(has_entry(:states_filter, %w[draft])) + .returns(stub(editions: Kaminari.paginate_array([]).page(1), available_users: [])) get :index, params: { states_filter: %w[draft not_a_real_state] } end + + should "show the first page of publications when no page is specified" do + FactoryBot.create_list(:guide_edition, FilteredEditionsPresenter::ITEMS_PER_PAGE + 1) + + get :index + + assert_response :ok + assert_select "p.publications-table__heading", "#{FilteredEditionsPresenter::ITEMS_PER_PAGE + 1} document(s)" + assert_select ".govuk-table__row .title", FilteredEditionsPresenter::ITEMS_PER_PAGE + end + + should "show the specified page of publications" do + FactoryBot.create_list(:guide_edition, FilteredEditionsPresenter::ITEMS_PER_PAGE + 1) + + get :index, params: { page: 2 } + + assert_response :ok + assert_select "p.publications-table__heading", "#{FilteredEditionsPresenter::ITEMS_PER_PAGE + 1} document(s)" + assert_select ".govuk-table__row .title", 1 + end end end diff --git a/test/integration/delete_edition_test.rb b/test/integration/delete_edition_test.rb index 971224a94..e2a3108b9 100644 --- a/test/integration/delete_edition_test.rb +++ b/test/integration/delete_edition_test.rb @@ -5,6 +5,9 @@ class DeleteEditionTest < LegacyIntegrationTest setup_users stub_linkables stub_holidays_used_by_fact_check + + test_strategy = Flipflop::FeatureSet.current.test! + test_strategy.switch!(:design_system_publications_filter, false) end teardown do diff --git a/test/integration/edition_scheduled_publishing_test.rb b/test/integration/edition_scheduled_publishing_test.rb index 761d808cc..c01747798 100644 --- a/test/integration/edition_scheduled_publishing_test.rb +++ b/test/integration/edition_scheduled_publishing_test.rb @@ -7,6 +7,9 @@ class EditionScheduledPublishingTest < LegacyJavascriptIntegrationTest stub_holidays_used_by_fact_check # queue up the edition, don't perform inline Sidekiq::Testing.fake! + + test_strategy = Flipflop::FeatureSet.current.test! + test_strategy.switch!(:design_system_publications_filter, false) end teardown do diff --git a/test/integration/edition_workflow_test.rb b/test/integration/edition_workflow_test.rb index ba198f9bc..07ea5d30e 100644 --- a/test/integration/edition_workflow_test.rb +++ b/test/integration/edition_workflow_test.rb @@ -15,6 +15,9 @@ class EditionWorkflowTest < LegacyJavascriptIntegrationTest @guide = FactoryBot.create(:guide_edition) login_as "Alice" + + test_strategy = Flipflop::FeatureSet.current.test! + test_strategy.switch!(:design_system_publications_filter, false) end teardown do diff --git a/test/integration/mark_edition_in_beta_test.rb b/test/integration/mark_edition_in_beta_test.rb index 396ef9187..c8352ccc1 100644 --- a/test/integration/mark_edition_in_beta_test.rb +++ b/test/integration/mark_edition_in_beta_test.rb @@ -5,6 +5,9 @@ class MarkEditionInBetaTest < LegacyJavascriptIntegrationTest setup_users stub_linkables stub_holidays_used_by_fact_check + + test_strategy = Flipflop::FeatureSet.current.test! + test_strategy.switch!(:design_system_publications_filter, false) end with_and_without_javascript do diff --git a/test/integration/root_overview_test.rb b/test/integration/root_overview_test.rb new file mode 100644 index 000000000..60c93b162 --- /dev/null +++ b/test/integration/root_overview_test.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +require_relative "../integration_test_helper" + +class RootOverviewTest < IntegrationTest + setup do + test_strategy = Flipflop::FeatureSet.current.test! + test_strategy.switch!(:design_system_publications_filter, true) + end + + should "be able to view different pages of results" do + FactoryBot.create(:user, :govuk_editor, name: "Alice", uid: "alice") + FactoryBot.create(:guide_edition, title: "Guides and Gals") + FactoryBot.create_list(:guide_edition, FilteredEditionsPresenter::ITEMS_PER_PAGE) + + visit "/" + assert_content("21 document(s)") + assert_no_content("Guides and Gals") + + click_on "Next" + assert_content("21 document(s)") + assert_content("Guides and Gals") + + click_on "Prev" + assert_content("21 document(s)") + assert_no_content("Guides and Gals") + end +end diff --git a/test/integration/unpublish_test.rb b/test/integration/unpublish_test.rb index b0db7b7b4..e4ec007d8 100644 --- a/test/integration/unpublish_test.rb +++ b/test/integration/unpublish_test.rb @@ -18,6 +18,9 @@ class UnpublishTest < LegacyIntegrationTest setup_users stub_linkables stub_holidays_used_by_fact_check + + test_strategy = Flipflop::FeatureSet.current.test! + test_strategy.switch!(:design_system_publications_filter, false) end should "unpublishing an artefact archives all editions" do diff --git a/test/integration_test_helper.rb b/test/integration_test_helper.rb index 50377453e..aa2aca3ee 100644 --- a/test/integration_test_helper.rb +++ b/test/integration_test_helper.rb @@ -1,10 +1,12 @@ require "test_helper" +require "capybara/minitest" require "capybara/rails" require "capybara-select-2" require "support/govuk_test" class IntegrationTest < ActionDispatch::IntegrationTest include Capybara::DSL + include Capybara::Minitest::Assertions include CapybaraSelect2 include CapybaraSelect2::Helpers include Warden::Test::Helpers diff --git a/test/unit/presenters/filtered_editions_presenter_test.rb b/test/unit/presenters/filtered_editions_presenter_test.rb index 5eefb21e5..f68d658fc 100644 --- a/test/unit/presenters/filtered_editions_presenter_test.rb +++ b/test/unit/presenters/filtered_editions_presenter_test.rb @@ -11,8 +11,8 @@ class FilteredEditionsPresenterTest < ActiveSupport::TestCase filtered_editions = FilteredEditionsPresenter.new.editions assert_equal(2, filtered_editions.count) - assert_equal(draft_guide, filtered_editions[0]) - assert_equal(published_guide, filtered_editions[1]) + assert_includes(filtered_editions, draft_guide) + assert_includes(filtered_editions, published_guide) end should "filter by state" do @@ -30,9 +30,9 @@ class FilteredEditionsPresenterTest < ActiveSupport::TestCase assigned_to_anna = FactoryBot.create(:guide_edition, assigned_to: anna.id) FactoryBot.create(:guide_edition) - filtered_editions = FilteredEditionsPresenter.new(assigned_to_filter: anna.id).editions.to_a + filtered_editions = FilteredEditionsPresenter.new(assigned_to_filter: anna.id).editions - assert_equal([assigned_to_anna], filtered_editions) + assert_equal([assigned_to_anna], filtered_editions.to_a) end should "filter by 'not assigned'" do @@ -40,9 +40,9 @@ class FilteredEditionsPresenterTest < ActiveSupport::TestCase FactoryBot.create(:guide_edition, assigned_to: anna.id) not_assigned = FactoryBot.create(:guide_edition) - filtered_editions = FilteredEditionsPresenter.new(assigned_to_filter: "nobody").editions.to_a + filtered_editions = FilteredEditionsPresenter.new(assigned_to_filter: "nobody").editions - assert_equal([not_assigned], filtered_editions) + assert_equal([not_assigned], filtered_editions.to_a) end should "ignore invalid 'assigned to'" do @@ -60,9 +60,9 @@ class FilteredEditionsPresenterTest < ActiveSupport::TestCase guide = FactoryBot.create(:guide_edition) FactoryBot.create(:completed_transaction_edition) - filtered_editions = FilteredEditionsPresenter.new(format_filter: "guide").editions.to_a + filtered_editions = FilteredEditionsPresenter.new(format_filter: "guide").editions - assert_equal([guide], filtered_editions) + assert_equal([guide], filtered_editions.to_a) end should "return all formats when specified by the format filter" do @@ -78,18 +78,48 @@ class FilteredEditionsPresenterTest < ActiveSupport::TestCase guide_fawkes = FactoryBot.create(:guide_edition, title: "Guide Fawkes") FactoryBot.create(:guide_edition, title: "Hitchhiker's Guide") - filtered_editions = FilteredEditionsPresenter.new(title_filter: "Fawkes").editions.to_a + filtered_editions = FilteredEditionsPresenter.new(title_filter: "Fawkes").editions - assert_equal([guide_fawkes], filtered_editions) + assert_equal([guide_fawkes], filtered_editions.to_a) end should "not return popular links" do guide_fawkes = FactoryBot.create(:guide_edition) FactoryBot.create(:popular_links) - filtered_editions = FilteredEditionsPresenter.new.editions.to_a + filtered_editions = FilteredEditionsPresenter.new.editions + + assert_equal([guide_fawkes], filtered_editions.to_a) + end + + should "return a single 'page' of results when no page number is specified" do + FactoryBot.create_list(:guide_edition, FilteredEditionsPresenter::ITEMS_PER_PAGE + 1) + + filtered_editions = FilteredEditionsPresenter.new.editions + + assert_equal(FilteredEditionsPresenter::ITEMS_PER_PAGE + 1, filtered_editions.count) + assert_equal(FilteredEditionsPresenter::ITEMS_PER_PAGE, filtered_editions.to_a.count) + end + + should "return the specified 'page' of results" do + FactoryBot.create_list(:guide_edition, FilteredEditionsPresenter::ITEMS_PER_PAGE + 1) + + filtered_editions = FilteredEditionsPresenter.new(page: 2).editions + + assert_equal(1, filtered_editions.to_a.count) + end + + should "order the returned results by 'updated_at' in descending order" do + oldest = FactoryBot.create(:guide_edition, updated_at: Time.utc(2022, 1)) + newest = FactoryBot.create(:guide_edition, updated_at: Time.utc(2024, 1)) + middle = FactoryBot.create(:guide_edition, updated_at: Time.utc(2023, 1)) + + filtered_editions = FilteredEditionsPresenter.new.editions - assert_equal([guide_fawkes], filtered_editions) + assert_equal(3, filtered_editions.count) + assert_equal(newest, filtered_editions[0]) + assert_equal(middle, filtered_editions[1]) + assert_equal(oldest, filtered_editions[2]) end end