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

Replace kaminari with pagy #681

Merged
merged 5 commits into from
Oct 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 2 additions & 3 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,17 @@ gem 'country_select'
gem 'devise', '~> 4.8.1'
gem 'geocoder', '~> 1.6.1'
gem 'grape', '~> 1.6.2'
gem 'grape-pagy'
gem 'grape-swagger'
gem 'grape-kaminari', '~> 0.4.3'
gem 'haml'
gem 'high_voltage', '~> 3.0.0'
gem 'http_accept_language'
gem 'jbuilder', '~> 2.5'
gem 'kaminari', '~> 1.2.2'
gem 'kaminari-grape', '~> 1.0'
gem 'mail_form', '>= 1.7.0'
gem 'net-smtp', require: false
gem 'net-imap', require: false
gem 'net-pop', require: false
gem 'pagy'
gem 'pg'
gem 'pg_search'
gem 'puma'
Expand Down
15 changes: 6 additions & 9 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -169,9 +169,9 @@ GEM
mustermann-grape (~> 1.0.0)
rack (>= 1.3.0)
rack-accept
grape-kaminari (0.4.3)
grape (>= 1.6.1)
kaminari-grape
grape-pagy (0.6.0)
grape (>= 1.5)
pagy (>= 6.0)
grape-swagger (1.5.0)
grape (~> 1.3)
haml (6.0.4)
Expand Down Expand Up @@ -214,9 +214,6 @@ GEM
activerecord
kaminari-core (= 1.2.2)
kaminari-core (1.2.2)
kaminari-grape (1.0.1)
grape
kaminari-core (~> 1.0)
loofah (2.20.0)
crass (~> 1.0.2)
nokogiri (>= 1.5.9)
Expand Down Expand Up @@ -255,6 +252,7 @@ GEM
nokogiri (1.14.3-x86_64-linux)
racc (~> 1.4)
orm_adapter (0.5.0)
pagy (6.0.4)
parallel (1.22.1)
parser (3.1.2.1)
ast (~> 2.4.1)
Expand Down Expand Up @@ -440,19 +438,18 @@ DEPENDENCIES
factory_bot_rails (~> 4.8.2)
geocoder (~> 1.6.1)
grape (~> 1.6.2)
grape-kaminari (~> 0.4.3)
grape-pagy
grape-swagger
haml
high_voltage (~> 3.0.0)
http_accept_language
i18n-debug
jbuilder (~> 2.5)
kaminari (~> 1.2.2)
kaminari-grape (~> 1.0)
mail_form (>= 1.7.0)
net-imap
net-pop
net-smtp
pagy
pg
pg_search
poltergeist
Expand Down
17 changes: 11 additions & 6 deletions app/controllers/api/v1/restrooms.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
module API
module V1
class Restrooms < Grape::API
include Grape::Kaminari
paginate per_page: 10, max_per_page: 100
PAGY_OPTIONS = { items_param: :per_page, items: 10, max_items: 100 }.freeze

helpers Grape::Pagy::Helpers

version 'v1'
format :json
Expand All @@ -11,6 +12,7 @@ class Restrooms < Grape::API
resource :restrooms do
desc "Get all restroom records ordered by date descending."
params do
use :pagy, **PAGY_OPTIONS
optional :ada, type: Boolean, desc: "Only return restrooms that are ADA accessible."
optional :unisex, type: Boolean, desc: "Only return restrooms that are unisex."
end
Expand All @@ -20,11 +22,12 @@ class Restrooms < Grape::API
r = r.accessible if params[:ada].present?
r = r.unisex if params[:unisex].present?

paginate(r.order(created_at: :desc))
pagy(r.order(created_at: :desc))
end

desc "Perform full-text search of restroom records."
params do
use :pagy, **PAGY_OPTIONS
optional :ada, type: Boolean, desc: "Only return restrooms that are ADA accessible."
optional :unisex, type: Boolean, desc: "Only return restrooms that are unisex."
requires :query, type: String, desc: "Your search query."
Expand All @@ -35,11 +38,12 @@ class Restrooms < Grape::API
r = r.accessible if params[:ada].present?
r = r.unisex if params[:unisex].present?

paginate(r.text_search(params[:query]))
pagy(r.text_search(params[:query]))
end

desc "Search by location."
params do
use :pagy, **PAGY_OPTIONS
optional :ada, type: Boolean, desc: "Only return restrooms that are ADA accessible."
optional :unisex, type: Boolean, desc: "Only return restrooms that are unisex."
requires :lat, type: Float, desc: "latitude"
Expand All @@ -50,11 +54,12 @@ class Restrooms < Grape::API
r = r.current
r = r.accessible if params[:ada].present?
r = r.unisex if params[:unisex]
paginate(r.near([params[:lat], params[:lng]], 20, order: 'distance'))
pagy(r.near([params[:lat], params[:lng]], 20, order: 'distance'))
end

desc "Search for restroom records updated or created on or after a given date"
params do
use :pagy, **PAGY_OPTIONS
optional :ada, type: Boolean, desc: "Only return restrooms that are ADA accessible."
optional :unisex, type: Boolean, desc: "Only return restrooms that are unisex."
optional(
Expand All @@ -77,7 +82,7 @@ class Restrooms < Grape::API
end
r = r.accessible if params[:ada].present?
r = r.unisex if params[:unisex].present?
paginate(r.order(created_at: :desc))
pagy(r.order(created_at: :desc))
end
end
# rubocop:enable Metrics/BlockLength
Expand Down
2 changes: 2 additions & 0 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
class ApplicationController < ActionController::Base
# Prevent CSRF attacks by raising an exception.
# For APIs, you may want to use :null_session instead.
include Pagy::Backend

protect_from_forgery with: :exception

around_action :switch_locale
Expand Down
5 changes: 2 additions & 3 deletions app/controllers/restrooms_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -104,15 +104,14 @@ def restrooms_filters

# rubocop:disable Metrics/AbcSize
def list_restrooms
@restrooms = Restroom.current.where(@filters).page(params[:page])
@restrooms = Restroom.current.where(@filters)
@restrooms =
if params[:search].present? || params[:map] == "1"
@restrooms.near([params[:lat], params[:long]], 20, order: 'distance')
else
@restrooms.reverse_order
end

@restrooms = @restrooms.out_of_range? ? @restrooms.page(1) : @restrooms
@pagy, @restrooms = pagy(@restrooms)[0].overflow? ? pagy(@restrooms, page: 1) : pagy(@restrooms)
end
# rubocop:enable Metrics/AbcSize

Expand Down
3 changes: 3 additions & 0 deletions app/helpers/pagy_helper.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module PagyHelper
include Pagy::Frontend
end
11 changes: 0 additions & 11 deletions app/views/kaminari/_first_page.html.erb

This file was deleted.

8 changes: 0 additions & 8 deletions app/views/kaminari/_gap.html.erb

This file was deleted.

11 changes: 0 additions & 11 deletions app/views/kaminari/_last_page.html.erb

This file was deleted.

11 changes: 0 additions & 11 deletions app/views/kaminari/_next_page.html.erb

This file was deleted.

12 changes: 0 additions & 12 deletions app/views/kaminari/_page.html.erb

This file was deleted.

23 changes: 0 additions & 23 deletions app/views/kaminari/_paginator.html.erb

This file was deleted.

11 changes: 0 additions & 11 deletions app/views/kaminari/_prev_page.html.erb

This file was deleted.

6 changes: 3 additions & 3 deletions app/views/restrooms/index.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -26,20 +26,20 @@
.col-sm-12.headroom
%ul{:class => "restrooms-list", :id => "list"}
= render @restrooms
= paginate @restrooms
!= pagy_nav @pagy

Choose a reason for hiding this comment

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

Is there a comparison here we could make to keep the = instead of !=?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes! It can be rewritten in this way to maintain the = that was there previously. This is functionally the same as using !=.

Suggested change
!= pagy_nav @pagy
= pagy_nav(@pagy).html_safe

Originally, the != evaluates the Ruby without ever sanitizing the HTML. Rewritten, the = is maintained but it's assumed to be safe by marking it with html_safe. It has to be done in this format because pagy is an "agnostic pagination gem it does not use the specific html_safe rails helper on its output".

/ .row
/ .col-xs-12.headroom
/ .restrooms-index-content
/ = content_tag 'div', id: 'map-container', data: { latitude: params[:lat], longitude: params[:long] } do
/ #map-content
/ .restrooms-list
/ = render @restrooms
/ = paginate @restrooms
/ != pagy_nav @pagy


/ #results.headroom
/ = content_tag 'div', id: 'mapContainer', data: { latitude: params[:lat], longitude: params[:long] } do
/ #mapArea
/ #list.restrooms-list
/ = render @restrooms
/ = paginate @restrooms
/ != pagy_nav @pagy