From 4e6dff782d551db66498b4522e0474faa9609a15 Mon Sep 17 00:00:00 2001 From: evazion Date: Thu, 18 Feb 2021 18:55:27 -0600 Subject: [PATCH] paginator: fix page 1 shown twice in empty searches. Fix a bug where page 1 was shown twice (once for the current page, and once for the last page) in searches that returned 0 results. --- app/components/paginator_component.rb | 4 +- test/components/paginator_component_test.rb | 67 +++++++++++++-------- 2 files changed, 44 insertions(+), 27 deletions(-) diff --git a/app/components/paginator_component.rb b/app/components/paginator_component.rb index b449a04e8..aa02bae5c 100644 --- a/app/components/paginator_component.rb +++ b/app/components/paginator_component.rb @@ -16,7 +16,7 @@ class PaginatorComponent < ApplicationComponent end def pages - last_page = total_pages + last_page = total_pages.clamp(1..) left = (current_page - window).clamp(2..) right = (current_page + window).clamp(..last_page - 1) @@ -25,7 +25,7 @@ class PaginatorComponent < ApplicationComponent ("..." unless left == 2), (left..right).to_a, ("..." unless right == last_page - 1), - last_page + (last_page unless last_page == 1) ].flatten.compact end diff --git a/test/components/paginator_component_test.rb b/test/components/paginator_component_test.rb index 5b315b88d..5752669a6 100644 --- a/test/components/paginator_component_test.rb +++ b/test/components/paginator_component_test.rb @@ -1,11 +1,11 @@ require "test_helper" class PaginatorComponentTest < ViewComponent::TestCase - def render_paginator(variant, page, limit: 3, page_limit: 100) + def render_paginator(variant, records, page: 1, limit: 3, page_limit: 100) with_variant(variant) do - tags = Tag.paginate(page, limit: limit, page_limit: page_limit) - params = ActionController::Parameters.new(controller: :tags, action: :index) - return render_inline(PaginatorComponent.new(records: tags, params: params)) + records = records.paginate(page, limit: limit, page_limit: page_limit) + params = ActionController::Parameters.new(controller: records.model_name.plural, action: :index) + return render_inline(PaginatorComponent.new(records: records, params: params)) end end @@ -18,20 +18,20 @@ class PaginatorComponentTest < ViewComponent::TestCase end context "The PaginatorComponent" do - setup do - @tags = create_list(:tag, 10) - end - context "when using sequential pagination" do + setup do + @tags = create_list(:tag, 10) + end + should "work with an aN page" do - html = render_paginator(:sequential, "a#{@tags[5].id}", limit: 3) + html = render_paginator(:sequential, Tag.all, page: "a#{@tags[5].id}", limit: 3) assert_page("a#{@tags[5+3].id}", html.css("a[rel=prev]")) assert_page("b#{@tags[5+1].id}", html.css("a[rel=next]")) end should "work with a bN page" do - html = render_paginator(:sequential, "b#{@tags[5].id}", limit: 3) + html = render_paginator(:sequential, Tag.all, page: "b#{@tags[5].id}", limit: 3) assert_page("a#{@tags[5-1].id}", html.css("a[rel=prev]")) assert_page("b#{@tags[5-3].id}", html.css("a[rel=next]")) @@ -39,25 +39,42 @@ class PaginatorComponentTest < ViewComponent::TestCase end context "when using numbered pagination" do - should "work for page 1" do - html = render_paginator(:numbered, 1, limit: 3) + context "for a search with 10 results" do + setup do + @tags = create_list(:tag, 10) + end - assert_css("span.paginator-prev") - assert_page("2", html.css("a.paginator-next")) + should "work for page 1" do + html = render_paginator(:numbered, Tag.all, page: 1, limit: 3) + + assert_css("span.paginator-prev") + assert_page("2", html.css("a.paginator-next")) + end + + should "work for page 2" do + html = render_paginator(:numbered, Tag.all, page: 2, limit: 3) + + assert_page("1", html.css("a.paginator-prev")) + assert_page("3", html.css("a.paginator-next")) + end + + should "work for page 4" do + html = render_paginator(:numbered, Tag.all, page: 4, limit: 3) + + assert_page("3", html.css("a.paginator-prev")) + assert_css("span.paginator-next") + end end - should "work for page 2" do - html = render_paginator(:numbered, 2, limit: 3) + context "for a search with zero results" do + should "work for page 1" do + html = render_paginator(:numbered, Tag.none, page: 1, limit: 3) - assert_page("1", html.css("a.paginator-prev")) - assert_page("3", html.css("a.paginator-next")) - end - - should "work for page 4" do - html = render_paginator(:numbered, 4, limit: 3) - - assert_page("3", html.css("a.paginator-prev")) - assert_css("span.paginator-next") + assert_css("span.paginator-current", text: "1") + assert_css("span.paginator-prev") + assert_css("span.paginator-next") + assert_css("span", count: 3) + end end end end