Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AO3-6880 Use pagy gem for pagination on select pages #5054

Merged
merged 16 commits into from
Feb 12, 2025
1 change: 1 addition & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
gem "lograge" # https://github.com/roidrage/lograge

gem 'will_paginate', '>=3.0.2'
gem 'pagy', '~> 9.3'

Check warning on line 40 in Gemfile

View workflow job for this annotation

GitHub Actions / Rubocop

[rubocop] reported by reviewdog 🐶 Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping. Raw Output: Gemfile:40:5: C: Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

Check warning on line 40 in Gemfile

View workflow job for this annotation

GitHub Actions / Rubocop

[rubocop] reported by reviewdog 🐶 Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping. Raw Output: Gemfile:40:13: C: Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
gem 'acts_as_list', '~> 0.9.7'
gem 'akismetor'

Expand Down
2 changes: 2 additions & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,7 @@ GEM
mini_portile2 (~> 2.8.2)
racc (~> 1.4)
orm_adapter (0.5.0)
pagy (9.3.3)
parallel (1.25.1)
parser (3.3.0.5)
ast (~> 2.4.1)
Expand Down Expand Up @@ -701,6 +702,7 @@ DEPENDENCIES
mysql2
n_plus_one_control
nokogiri (>= 1.8.5)
pagy (~> 9.3)
permit_yo
phraseapp-in-context-editor-ruby (>= 1.0.6)
pickle
Expand Down
26 changes: 26 additions & 0 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,32 @@
end
end

include Pagy::Backend
def pagy(collection, **vars)
pagy_overflow_handler do
super
end
end

def pagy_query_result(query_result, **vars)
pagy_overflow_handler do
Pagy.new(
count: query_result.total_entries,
page: query_result.current_page,
limit: query_result.per_page,
**vars
)
end
end

def pagy_overflow_handler(&block)

Check warning on line 63 in app/controllers/application_controller.rb

View workflow job for this annotation

GitHub Actions / Rubocop

[rubocop] reported by reviewdog 🐶 Unused method argument - `block`. If it's necessary, use `_` or `_block` as an argument name to indicate that it won't be used. You can also write as `pagy_overflow_handler(*)` if you want the method to accept any arguments but don't care about them. Raw Output: app/controllers/application_controller.rb:63:30: W: Lint/UnusedMethodArgument: Unused method argument - `block`. If it's necessary, use `_` or `_block` as an argument name to indicate that it won't be used. You can also write as `pagy_overflow_handler(*)` if you want the method to accept any arguments but don't care about them.
begin

Check warning on line 64 in app/controllers/application_controller.rb

View workflow job for this annotation

GitHub Actions / Rubocop

[rubocop] reported by reviewdog 🐶 Redundant `begin` block detected. Raw Output: app/controllers/application_controller.rb:64:5: C: Style/RedundantBegin: Redundant `begin` block detected.
yield
rescue Pagy::OverflowError
nil
end
end
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keeping the same behavior as now by not showing the pagination in case of overflow ("page 66 of 10").

pagy's alternative options for reference.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we're not setting Pagy::DEFAULT[:overflow], the default is :empty_page instead of :exception, do we need to wrap calls in pagy_overflow_handler?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without the require 'pagy/extras/overflow' line, Pagy always raises an exception in case of overflow (source)


def display_auth_error
respond_to do |format|
format.html do
Expand Down
9 changes: 9 additions & 0 deletions app/controllers/bookmarks_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,15 @@
end
end
set_own_bookmarks

@pagy =
if @bookmarks.respond_to?(:total_pages)
pagy_query_result(@bookmarks)
elsif @bookmarkable_items.respond_to?(:total_pages)
pagy_query_result(@bookmarkable_items)
else

Check warning on line 154 in app/controllers/bookmarks_controller.rb

View workflow job for this annotation

GitHub Actions / Rubocop

[rubocop] reported by reviewdog 🐶 Redundant `else`-clause. Raw Output: app/controllers/bookmarks_controller.rb:154:7: C: Style/EmptyElse: Redundant `else`-clause.
nil
end
end

# GET /:locale/bookmark/:id
Expand Down
3 changes: 2 additions & 1 deletion app/controllers/readings_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ def index
@readings = @readings.where(toread: true)
@page_subtitle = ts("Marked For Later")
end
@readings = @readings.order("last_viewed DESC").page(params[:page])
@readings = @readings.order("last_viewed DESC")
@pagy, @readings = pagy(@readings)
end

def destroy
Expand Down
4 changes: 4 additions & 0 deletions app/controllers/works_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,10 @@
@works = Work.latest.for_blurb.to_a
end
set_own_works

if @works.respond_to?(:total_pages)

Check warning on line 135 in app/controllers/works_controller.rb

View workflow job for this annotation

GitHub Actions / Rubocop

[rubocop] reported by reviewdog 🐶 Use a guard clause (`return unless @works.respond_to?(:total_pages)`) instead of wrapping the code inside a conditional expression. Raw Output: app/controllers/works_controller.rb:135:5: C: Style/GuardClause: Use a guard clause (`return unless @works.respond_to?(:total_pages)`) instead of wrapping the code inside a conditional expression.

Check warning on line 135 in app/controllers/works_controller.rb

View workflow job for this annotation

GitHub Actions / Rubocop

[rubocop] reported by reviewdog 🐶 Favor modifier `if` usage when having a single-line body. Another good alternative is the usage of control flow `&&`/`||`. Raw Output: app/controllers/works_controller.rb:135:5: C: Style/IfUnlessModifier: Favor modifier `if` usage when having a single-line body. Another good alternative is the usage of control flow `&&`/`||`.
@pagy = pagy_query_result(@works)
end
end

def collected
Expand Down
51 changes: 51 additions & 0 deletions app/helpers/application_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -528,6 +528,57 @@
super(*[collection_or_options, options].compact)
end

include Pagy::Frontend
ceithir marked this conversation as resolved.
Show resolved Hide resolved
# Cf https://github.com/ddnexus/pagy/blob/master/gem/lib/pagy/frontend.rb
def pagy_nav(pagy, id: nil, aria_label: nil, **vars)

Check warning on line 533 in app/helpers/application_helper.rb

View workflow job for this annotation

GitHub Actions / Rubocop

[rubocop] reported by reviewdog 🐶 Unused method argument - `aria_label`. Raw Output: app/helpers/application_helper.rb:533:31: W: Lint/UnusedMethodArgument: Unused method argument - `aria_label`.
return nil unless pagy

# Keep will_paginate behavior of showing nothing if only one page
return nil if pagy.series.length <= 1

id = %( id="#{id}") if id
a = pagy_anchor(pagy, **vars)

html = %(<h4 class="landmark heading">#{t("pagination.title")}</h4>)

Check warning on line 542 in app/helpers/application_helper.rb

View workflow job for this annotation

GitHub Actions / Rubocop

[rubocop] reported by reviewdog 🐶 Prefer single-quoted strings inside interpolations. Raw Output: app/helpers/application_helper.rb:542:47: C: Style/StringLiteralsInInterpolation: Prefer single-quoted strings inside interpolations.

html << %(<ol#{id} class="pagination actions pagy" role="navigation" aria-label="#{t("pagination.aria-label")}">)

Check warning on line 544 in app/helpers/application_helper.rb

View workflow job for this annotation

GitHub Actions / Rubocop

[rubocop] reported by reviewdog 🐶 Prefer single-quoted strings inside interpolations. Raw Output: app/helpers/application_helper.rb:544:90: C: Style/StringLiteralsInInterpolation: Prefer single-quoted strings inside interpolations.

prev_text = t("pagination.previous")
redsummernight marked this conversation as resolved.
Show resolved Hide resolved
prev_a =
if (p_prev = pagy.prev)
a.(p_prev, prev_text)
else
%(<span class="disabled">#{prev_text}</span>)
end
html << %(<li class="previous">#{prev_a}</li>)

pagy.series(**vars).each do |item| # series example: [1, :gap, 7, 8, "9", 10, 11, :gap, 36]
html << %(<li>)
html << case item
when Integer
a.(item)
when String
%(<a role="link" aria-disabled="true" aria-current="page" class="current">#{pagy.label_for(item)}</a>)
when :gap
%(<span class="gap">#{pagy_t('pagy.gap')}</span>)
else
raise InternalError, "expected item types in series to be Integer, String or :gap; got #{item.inspect}"
end
html << %(</li>)
end

next_text = t("pagination.next")
next_a =
if (p_next = pagy.next)
a.(p_next, next_text)
else
%(<span class="disabled">#{next_text}</span>)
end
html << %(<li class="next">#{next_a}</li>)

html << %(</ol>)
end

# spans for nesting a checkbox or radio button inside its label to make custom
# checkbox or radio designs
def label_indicator_and_text(text)
Expand Down
12 changes: 2 additions & 10 deletions app/views/bookmarks/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,7 @@
<p><%= ts("These are some of the latest bookmarks created on the Archive. To find more bookmarks, #{link_to 'choose a fandom', media_path} or #{link_to 'try our advanced search', search_bookmarks_path}.").html_safe %>
<% end %>

<% if @bookmarks.respond_to?(:total_pages) %>
<%= will_paginate @bookmarks %>
<% elsif @bookmarkable_items.respond_to?(:total_pages) %>
<%= will_paginate @bookmarkable_items %>
<% end %>
<%== pagy_nav @pagy %>
Fixed Show fixed Hide fixed

<!--main content-->
<h3 class="landmark heading"><%= ts("List of Bookmarks") %></h3>
Expand All @@ -71,9 +67,5 @@
<div id="dynamic-bookmark" class="dynamic hidden"></div>

<!--subnav-->
<% if @bookmarks.respond_to?(:total_pages) %>
<%= will_paginate @bookmarks %>
<% elsif @bookmarkable_items.respond_to?(:total_pages) %>
<%= will_paginate @bookmarkable_items %>
<% end %>
<%== pagy_nav @pagy %>
Fixed Show fixed Hide fixed
<!-- /subnav-->
2 changes: 1 addition & 1 deletion app/views/readings/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -30,5 +30,5 @@
<!--/content-->

<!--subnav-->
<%= will_paginate @readings %>
<%== pagy_nav @pagy %>
<!--/subnav-->
8 changes: 2 additions & 6 deletions app/views/works/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,7 @@
<p><%= ts("These are some of the latest works posted to the Archive. To find more works, #{link_to 'choose a fandom', media_path} or #{link_to 'try our advanced search', search_works_path}.").html_safe %>
<% end %>

<% if @works.respond_to?(:total_pages) %>
<%= will_paginate @works %>
<% end %>
<%== pagy_nav @pagy %>
Fixed Show fixed Hide fixed

<!--main content-->
<h3 class="landmark heading"><%= ts("Listing Works") %></h3>
Expand All @@ -64,6 +62,4 @@
<% end %>
<!---/subnav-->

<% if @works.respond_to?(:total_pages) %>
<%= will_paginate @works %>
<% end %>
<%== pagy_nav @pagy %>
Fixed Show fixed Hide fixed
7 changes: 7 additions & 0 deletions config/initializers/gem-plugin_config/pagy.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# frozen_string_literal: true

# See https://ddnexus.github.io/pagy/docs/api/pagy#variables
Pagy::DEFAULT[:limit] = ArchiveConfig.ITEMS_PER_PAGE
Pagy::DEFAULT[:size] = 9

Pagy::DEFAULT.freeze
5 changes: 5 additions & 0 deletions config/locales/views/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1565,6 +1565,11 @@ en:
orphan_user:
orphaning_bylines_only_message_html: Orphaning will automatically remove your personal information from <strong>bylines only</strong>. It will <strong>not</strong> automatically remove your personal information from anywhere else in the work(s). Social media accounts, contact information, email addresses, names, and any other personal information posted in the title, summary, notes, tags, work body, or comment text will <strong>not</strong> be automatically removed. Comments posted by other users will also <strong>not</strong> be affected by Orphaning. If you want information removed from these places, you should edit or delete it prior to Orphaning. <strong>Once you Orphan the work(s), you will no longer be able to delete information in these places yourself</strong>.
orphaning_works_message_html: 'Orphaning will <strong>permanently</strong> remove your username and/or pseud from the bylines of: the following work(s), their chapters, associated series, and any comments you may have left on them while logged into this account.'
pagination:
aria-label: Pagination
next: Next →
previous: "← Previous"
title: Pages Navigation
preferences:
index:
allow_collection_invitation: Allow others to invite my works to collections.
Expand Down
Loading