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

Migrate from Kaminari to pagy #4922

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ gem "good_job", "~> 3.99"
gem "gravtastic", "~> 3.2"
gem "honeybadger", "~> 5.5.1", require: false # see https://github.com/rubygems/rubygems.org/pull/4598
gem "http_accept_language", "~> 2.1"
gem "kaminari", "~> 1.2"
gem "launchdarkly-server-sdk", "~> 8.7"
gem "mail", "~> 2.8"
gem "octokit", "~> 9.1"
Expand Down
17 changes: 0 additions & 17 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -326,18 +326,6 @@ GEM
faraday (~> 2.0)
faraday-follow_redirects
jwt (2.7.1)
kaminari (1.2.2)
activesupport (>= 4.1.0)
kaminari-actionview (= 1.2.2)
kaminari-activerecord (= 1.2.2)
kaminari-core (= 1.2.2)
kaminari-actionview (1.2.2)
actionview
kaminari-core (= 1.2.2)
kaminari-activerecord (1.2.2)
activerecord
kaminari-core (= 1.2.2)
kaminari-core (1.2.2)
language_server-protocol (3.17.0.3)
launchdarkly-server-sdk (8.7.0)
concurrent-ruby (~> 1.1)
Expand Down Expand Up @@ -836,7 +824,6 @@ DEPENDENCIES
honeybadger (~> 5.5.1)
http_accept_language (~> 2.1)
importmap-rails (~> 2.0)
kaminari (~> 1.2)
launchdarkly-server-sdk (~> 8.7)
launchy (~> 3.0)
letter_opener (~> 1.10)
Expand Down Expand Up @@ -1035,10 +1022,6 @@ CHECKSUMS
json (2.7.2) sha256=1898b5cbc81cd36c0fd4d0b7ad2682c39fb07c5ff682fc6265f678f550d4982c
json-jwt (1.16.6) sha256=ab451f9cd8743cecc4137f4170806046c1d8a6d4ee6e8570e0b5c958409b266c
jwt (2.7.1) sha256=07357cd2f180739b2f8184eda969e252d850ac996ed0a23f616e8ff0a90ae19b
kaminari (1.2.2) sha256=c4076ff9adccc6109408333f87b5c4abbda5e39dc464bd4c66d06d9f73442a3e
kaminari-actionview (1.2.2) sha256=1330f6fc8b59a4a4ef6a549ff8a224797289ebf7a3a503e8c1652535287cc909
kaminari-activerecord (1.2.2) sha256=0dd3a67bab356a356f36b3b7236bcb81cef313095365befe8e98057dd2472430
kaminari-core (1.2.2) sha256=3bd26fec7370645af40ca73b9426a448d09b8a8ba7afa9ba3c3e0d39cdbb83ff
language_server-protocol (3.17.0.3) sha256=3d5c58c02f44a20d972957a9febe386d7e7468ab3900ce6bd2b563dd910c6b3f
launchdarkly-server-sdk (8.7.0) sha256=062e7dc42c2884ed9deed42c49937917faa911128a659386e4bd45d1ad1506eb
launchy (3.0.1) sha256=b7fa60bda0197cf57614e271a250a8ca1f6a34ab889a3c73f67ec5d57c8a7f2c
Expand Down
21 changes: 8 additions & 13 deletions app/assets/stylesheets/modules/nav/nav--paginated.css
Original file line number Diff line number Diff line change
Expand Up @@ -46,32 +46,27 @@
margin-right: 8px;
margin-left: 8px; }

.pagination {
nav.pagy {
margin-top: 90px;
position: relative;
width: 100%;
overflow: auto;
text-align: center; }
.pagination a, .pagination em {
nav.pagy a, nav.pagy em {
margin-right: 8px;
margin-left: 8px;
font-style: normal; }
.pagination a {
nav.pagy a {
color: #9da2ab;
transition-duration: 0.25s;
transition-property: color;
outline: none; }
.pagination a:hover, .pagination a:focus, .pagination a:active {
nav.pagy a:hover, nav.pagy a:focus, nav.pagy a:active {
color: #141c22; }
.pagination .previous_page.disabled, .pagination .next_page.disabled {
nav.pagy a:first-child:not([href]), nav.pagy a:last-child:not([href]) {
color: #9da2ab; }
.pagination .previous_page.disabled:hover, .pagination .previous_page.disabled:focus, .pagination .previous_page.disabled:active, .pagination .next_page.disabled:hover, .pagination .next_page.disabled:focus, .pagination .next_page.disabled:active {
nav.pagy a:first-child:not([href]):hover, nav.pagy a:first-child:not([href]):focus, nav.pagy a:first-child:not([href]):active, nav.pagy nav.pagy a:last-child:not([href]):hover, nav.pagy nav.pagy a:last-child:not([href]):focus, nav.pagy nav.pagy a:last-child:not([href]):active {
color: #9da2ab; }
.pagination .previous_page {
position: absolute;
left: 0; }
.pagination .next_page {
position: absolute;
right: 0; }
.pagination .current {
nav.pagy .current {
color: #000;
margin: 0 8px; }
7 changes: 3 additions & 4 deletions app/controllers/api/v1/timeframe_versions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@ class InvalidTimeframeParameterError < StandardError; end
MAXIMUM_TIMEFRAME_QUERY_IN_DAYS = 7

def index
render_rubygems(
Version.created_between(from_time, to_time).page(@page)
)
render_rubygems(*pagy(Version.created_between(from_time, to_time)))
end

private
Expand All @@ -35,12 +33,13 @@ def to_time
raise InvalidTimeframeParameterError, 'the "to" parameter must be iso8601 formatted'
end

def render_rubygems(versions)
def render_rubygems(pagy, versions)
rubygems = versions.includes(:dependencies, :gem_download, rubygem: %i[linkset gem_download]).map do |version|
payload = version.rubygem.payload(version)
payload.merge(version.as_json)
end

pagy_headers_merge(pagy)
respond_to do |format|
format.json { render json: rubygems }
format.yaml { render yaml: rubygems }
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/api_keys_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ class ApiKeysController < ApplicationController
verify_session_before

def index
@api_key = session.delete(:api_key)
@api_keys = current_user.api_keys.unexpired.not_oidc.preload(ownership: :rubygem).page(@page)
@api_key = session.delete(:api_key)
@api_keys_pagy, @api_keys = pagy(current_user.api_keys.unexpired.not_oidc.preload(ownership: :rubygem))
redirect_to new_profile_api_key_path if @api_keys.empty?
end

Expand Down
1 change: 1 addition & 0 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ class ApplicationController < ActionController::Base
include Pundit::Authorization
include ApplicationMultifactorMethods
include TraceTagger
include Pagy::Backend

helper ActiveSupport::NumberHelper

Expand Down
17 changes: 9 additions & 8 deletions app/controllers/news_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,20 @@ class NewsController < ApplicationController
before_action -> { set_page Gemcutter::NEWS_MAX_PAGES }

def show
@rubygems = Rubygem.preload(:latest_version, :gem_download)
.news(Gemcutter::NEWS_DAYS_LIMIT)
.page(@page)
.per(Gemcutter::NEWS_PER_PAGE)
@rubygems_pagy, @rubygems = pagy(
Rubygem.preload(:latest_version, :gem_download)
.news(Gemcutter::NEWS_DAYS_LIMIT),
limit: Gemcutter::NEWS_MAX_PAGES
)

limit_total_count
end

def popular
@title = t(".title")
@rubygems = Rubygem.preload(:latest_version, :gem_download)
.popular(Gemcutter::POPULAR_DAYS_LIMIT)
.page(@page)
.per(Gemcutter::NEWS_PER_PAGE)
@rubygems_pagy, @rubygems = pagy(Rubygem.preload(:latest_version, :gem_download)
.popular(Gemcutter::POPULAR_DAYS_LIMIT),
limit: Gemcutter::NEWS_MAX_PAGES)
limit_total_count

render :show
Expand Down
8 changes: 2 additions & 6 deletions app/controllers/oidc/api_key_roles_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,11 @@ class OIDC::ApiKeyRolesController < ApplicationController
before_action :set_page, only: :index

def index
@api_key_roles = current_user.oidc_api_key_roles.active.includes(:provider)
.page(@page)
.strict_loading
@api_key_roles_pagy, @api_key_roles = pagy(current_user.oidc_api_key_roles.active.includes(:provider).strict_loading)
end

def show
@id_tokens = @api_key_role.id_tokens.order(id: :desc).includes(:api_key)
.page(0).per(10)
.strict_loading
@id_tokens_pagy, @id_tokens = pagy(@api_key_role.id_tokens.order(id: :desc).includes(:api_key).strict_loading, limit: 10)
respond_to do |format|
format.json do
render json: @api_key_role
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/oidc/providers_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ class OIDC::ProvidersController < ApplicationController
before_action :set_page, only: :index

def index
providers = OIDC::Provider.strict_loading.page(@page)
render OIDC::Providers::IndexView.new(providers:)
providers_pagy, providers = pagy(OIDC::Provider.strict_loading)
render OIDC::Providers::IndexView.new(providers_pagy:, providers:)
end

def show
Expand Down
7 changes: 4 additions & 3 deletions app/controllers/ownership_calls_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@ class OwnershipCallsController < ApplicationController

def index
set_page
@ownership_calls = OwnershipCall.opened.includes(:user, rubygem: %i[latest_version gem_download]).order(created_at: :desc)
.page(@page)
.per(Gemcutter::OWNERSHIP_CALLS_PER_PAGE)
@ownership_calls_pagy, @ownership_calls = pagy(
OwnershipCall.opened.includes(:user, rubygem: %i[latest_version gem_download]).order(created_at: :desc),
limit: Gemcutter::OWNERSHIP_CALLS_PER_PAGE
)
end

def create
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/profiles_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ def adoptions
end

def security_events
@security_events = current_user.events.order(id: :desc).page(params[:page]).per(50)
render Profiles::SecurityEventsView.new(security_events: @security_events)
@security_events_pagy, @security_events = pagy(current_user.events.order(id: :desc), limit: 50)
render Profiles::SecurityEventsView.new(security_events_pagy: @security_events_pagy, security_events: @security_events)
end

private
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/reverse_dependencies_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,6 @@ def index
.preload(:gem_download, :latest_version)

@reverse_dependencies = @reverse_dependencies.legacy_search(params[:rdeps_query]) if params[:rdeps_query].is_a?(String)
@reverse_dependencies = @reverse_dependencies.page(@page).without_count
@reverse_dependencies_pagy, @reverse_dependencies = pagy_countless(@reverse_dependencies)
end
end
6 changes: 3 additions & 3 deletions app/controllers/rubygems_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ def index
respond_to do |format|
format.html do
@letter = Rubygem.letterize(gem_params[:letter])
@gems = Rubygem.letter(@letter).includes(:latest_version, :gem_download).page(@page)
@gems_pagy, @gems = pagy(Rubygem.letter(@letter).includes(:latest_version, :gem_download), limit: 2)
end
format.atom do
@versions = Version.published.limit(Gemcutter::DEFAULT_PAGINATION)
Expand All @@ -32,8 +32,8 @@ def show

def security_events
authorize @rubygem, :show_events?
@security_events = @rubygem.events.order(id: :desc).page(params[:page]).per(50)
render Rubygems::SecurityEventsView.new(rubygem: @rubygem, security_events: @security_events)
@security_events_pagy, @security_events = pagy(@rubygem.events.order(id: :desc), limit: 50)
render Rubygems::SecurityEventsView.new(rubygem: @rubygem, security_events_pagy: @security_events_pagy, security_events: @security_events)
end

private
Expand Down
3 changes: 2 additions & 1 deletion app/controllers/searches_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ def show
@error_msg, @gems = ElasticSearcher.new(params[:query], page: @page).search

return unless @gems
set_total_pages if @gems.total_count > Gemcutter::SEARCH_MAX_PAGES * Rubygem.default_per_page
@gems_pagy = Pagy.new_from_searchkick(@gems)
set_total_pages if @gems.total_count > 10_000
exact_match = Rubygem.name_is(params[:query]).first
@yanked_gem = exact_match unless exact_match&.indexed_versions?
@yanked_filter = true if params[:yanked] == "true"
Expand Down
3 changes: 2 additions & 1 deletion app/controllers/stats_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ def index
@number_of_gems = Rubygem.total_count
@number_of_users = User.count
@number_of_downloads = GemDownload.total_count
@most_downloaded = Rubygem.by_downloads.includes(:gem_download).page(@page).per(Gemcutter::STATS_PER_PAGE)
@most_downloaded_pagy,
@most_downloaded = pagy(Rubygem.by_downloads.includes(:gem_download), limit: Gemcutter::STATS_PER_PAGE)
@most_downloaded_count = GemDownload.most_downloaded_gem_count
limit_total_count
end
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/versions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ class VersionsController < ApplicationController

def index
set_page
@versions = @rubygem.versions.by_position.page(@page).per(Gemcutter::VERSIONS_PER_PAGE)
@versions_pagy, @versions = pagy(@rubygem.versions.by_position, limit: Gemcutter::VERSIONS_PER_PAGE)
end

def show
Expand Down
7 changes: 1 addition & 6 deletions app/helpers/application_helper.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
module ApplicationHelper
include BetterHtml::Helpers
include Pagy::Frontend

def page_title
combo = "#{t :title} | #{t :subtitle}"
Expand Down Expand Up @@ -59,12 +60,6 @@ def active?(path)
"is-active" if request.path_info == path
end

# replacement for Kaminari::ActionViewExtension#paginate
# only shows `next` and `prev` links and not page numbers, saving a COUNT(DISTINCT ..) query
def plain_paginate(items)
render "layouts/plain_paginate", items: items
end

def content_for_title(title, title_url)
return title unless title_url
link_to title, title_url, class: "t-link--black"
Expand Down
4 changes: 2 additions & 2 deletions app/views/api_keys/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

<header class="gems__header">
<p class="gems__meter">
<%= page_entries_info @api_keys, entry_name: 'API keys' %>
<%== pagy_info @api_keys_pagy, item_name: 'API keys' %>
</p>
</header>

Expand Down Expand Up @@ -121,7 +121,7 @@
</tr>
</table>

<%= paginate @api_keys %>
<%== pagy_nav @api_keys_pagy if @api_keys_pagy.pages > 1 %>

<p><%= button_to t(".new_key"), new_profile_api_key_path, method: "get", class: "form__submit" %></p>
<% if current_user.oidc_api_key_roles.any? %>
Expand Down
9 changes: 5 additions & 4 deletions app/views/components/events/table_component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,17 @@ class Events::TableComponent < ApplicationComponent
extend Phlex::Rails::HelperMacros

register_value_helper :current_user
register_value_helper :page_entries_info
register_value_helper :paginate
register_value_helper :pagy_info
register_value_helper :pagy_nav

extend Dry::Initializer

option :security_events_pagy
option :security_events

def view_template
header(class: "gems__header push--s") do
p(class: "gems__meter l-mb-0") { plain page_entries_info(security_events) }
p(class: "gems__meter l-mb-0") { plain pagy_info(security_events_pagy, item_name: "security events") }
end

if security_events.any?
Expand All @@ -39,7 +40,7 @@ def view_template
end
end

plain paginate(security_events)
plain pagy_nav(security_events_pagy) if security_events_pagy.pages > 1
end

private
Expand Down
4 changes: 2 additions & 2 deletions app/views/layouts/_plain_paginate.html.erb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<nav class="pagination">
<%= link_to_prev_page items, '‹ Prev' %>
<%= link_to_next_page items, 'Next ›' %>
<%== pagy_prev_a items, '‹ Prev' %>
<%== pagy_next_a items, 'Next ›' %>
</nav>
4 changes: 2 additions & 2 deletions app/views/news/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
</div>

<header class="gems__header">
<p class="gems__meter"><%= page_entries_info @rubygems %></p>
<p class="gems__meter"><%== pagy_info @rubygems_pagy, item_name: 'gems' %></p>
</header>

<%= render partial: "news/rubygem", collection: @rubygems %>
<%= paginate @rubygems %>
<%== pagy_nav @rubygems_pagy if @rubygems_pagy.pages > 1 %>
4 changes: 2 additions & 2 deletions app/views/oidc/api_key_roles/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
<%= button_to(t(".new_role"), new_profile_oidc_api_key_role_path, method: "get", class: "form__submit") %>
</p>
<header class="gems__header push--s">
<p class="gems__meter l-mb-0"><%= page_entries_info(@api_key_roles) %></p>
<p class="gems__meter l-mb-0"><%== pagy_info(@api_key_roles_pagy) %></p>
</header>
<table class="owners__table">
<thead>
Expand Down Expand Up @@ -36,5 +36,5 @@
<% end %>
</tbody>
</table>
<%= paginate @api_key_roles %>
<%== pagy_nav @api_key_roles_pagy if @api_key_roles_pagy.pages > 1 %>
</div>
2 changes: 1 addition & 1 deletion app/views/oidc/api_key_roles/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
<h3 class="t-list__heading"><%= OIDC::ApiKeyRole.human_attribute_name(:id_tokens) %></h3>
<div class="push--s">
<header class="gems__header push--s">
<p class="gems__meter l-mb-0"><%= page_entries_info(@id_tokens) %></p>
<p class="gems__meter l-mb-0"><%== pagy_info(@id_tokens_pagy, item_name: "ID Tokens") %></p>
</header>
<% if @id_tokens.present? %>
<%= render OIDC::IdToken::TableComponent.new(id_tokens: @id_tokens) %>
Expand Down
Loading
Loading