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 will_paginate with kaminari #1807

Closed
wants to merge 3 commits into from
Closed
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
2 changes: 1 addition & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ gem 'highline'
gem 'honeybadger'
gem 'http_accept_language'
gem 'jquery-rails'
gem 'kaminari'
gem 'mail', '2.6.6'
gem 'newrelic_rpm'
gem 'paul_revere', '~> 3.0.0'
Expand All @@ -37,7 +38,6 @@ gem 'statsd-instrument', '~> 2.3.0'
gem 'uglifier', '>= 1.0.3'
gem 'unicorn'
gem 'validates_formatting_of'
gem 'will_paginate'
gem 'elasticsearch-model', '~> 5.0.0'
gem 'elasticsearch-rails', '~> 5.0.0'
gem 'elasticsearch-dsl', '~> 0.1.2'
Expand Down
15 changes: 13 additions & 2 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,18 @@ GEM
railties (>= 4.2.0)
thor (>= 0.14, < 2.0)
jwt (2.1.0)
kaminari (1.1.1)
activesupport (>= 4.1.0)
kaminari-actionview (= 1.1.1)
kaminari-activerecord (= 1.1.1)
kaminari-core (= 1.1.1)
kaminari-actionview (1.1.1)
actionview
kaminari-core (= 1.1.1)
kaminari-activerecord (1.1.1)
activerecord
kaminari-core (= 1.1.1)
kaminari-core (1.1.1)
kgio (2.11.0)
kubeclient (3.1.2)
http (~> 2.2.2)
Expand Down Expand Up @@ -376,7 +388,6 @@ GEM
websocket-driver (0.7.0)
websocket-extensions (>= 0.1.0)
websocket-extensions (0.1.3)
will_paginate (3.1.6)
xml-simple (1.1.5)
xpath (3.1.0)
nokogiri (~> 1.8)
Expand Down Expand Up @@ -411,6 +422,7 @@ DEPENDENCIES
honeybadger
http_accept_language
jquery-rails
kaminari
kubernetes-deploy (= 0.20.6)
launchy
listen
Expand Down Expand Up @@ -446,7 +458,6 @@ DEPENDENCIES
uglifier (>= 1.0.3)
unicorn
validates_formatting_of
will_paginate
xml-simple

BUNDLED WITH
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/api/v1/searches_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ class Api::V1::SearchesController < Api::BaseController
before_action :set_page, only: :show

def show
@rubygems = Rubygem.legacy_search(params.require(:query)).with_versions.paginate(page: @page)
@rubygems = Rubygem.legacy_search(params.require(:query)).with_versions.page(@page)
respond_to do |format|
format.json { render json: @rubygems }
format.yaml { render yaml: @rubygems }
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/api/v1/timeframe_versions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ class InvalidTimeframeParameterError < StandardError; end

def index
render_rubygems(
Version.created_between(from_time, to_time).paginate(page: @page)
Version.created_between(from_time, to_time).page(@page)
)
end

Expand Down
8 changes: 2 additions & 6 deletions app/controllers/news_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,12 @@ class NewsController < ApplicationController
before_action :set_page

def show
@rubygems = Rubygem.news(7.days)
.paginate(page: @page, per_page: 10, total_entries: 100)
@rubygems = Rubygem.news(7.days).page(@page).per(10).limit(100)
end

def popular
@title = t(".title")

@rubygems = Rubygem.by_downloads
.news(70.days)
.paginate(page: @page, per_page: 10, total_entries: 100)
@rubygems = Rubygem.by_downloads.news(70.days).page(@page).per(10).limit(100)

render :show
end
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/reverse_dependencies_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ class ReverseDependenciesController < ApplicationController
def index
@reverse_dependencies = @rubygem.reverse_dependencies
.by_downloads
.includes(:latest_version, :gem_download)
.preload(:gem_download, :latest_version)

_, @reverse_dependencies = @reverse_dependencies.search(params[:rdeps_query]) if params[:rdeps_query]&.is_a?(String)
@reverse_dependencies = @reverse_dependencies.paginate(page: @page)
@reverse_dependencies = @reverse_dependencies.page(@page).without_count
end
end
2 changes: 1 addition & 1 deletion app/controllers/rubygems_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ def index
respond_to do |format|
format.html do
@letter = Rubygem.letterize(params[:letter])
@gems = Rubygem.letter(@letter).includes(:latest_version, :gem_download).paginate(page: @page)
@gems = Rubygem.letter(@letter).includes(:latest_version, :gem_download).page(@page)
end
format.atom do
@versions = Version.published(Gemcutter::DEFAULT_PAGINATION)
Expand Down
8 changes: 4 additions & 4 deletions app/controllers/searches_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ class SearchesController < ApplicationController
def show
return unless params[:query]&.is_a?(String)
@error_msg, @gems = Rubygem.search(params[:query], elasticsearch: es_enabled?, page: @page)
limit_total_entries if @gems.total_entries > MAX_PAGE * Rubygem.per_page
limit_total_count if @gems.total_count > MAX_PAGE * Rubygem.default_per_page

@exact_match = Rubygem.name_is(params[:query]).with_versions.first
redirect_to rubygem_path(@exact_match) if @exact_match && @gems.size == 1
Expand All @@ -21,10 +21,10 @@ def limit_page
render_404 if @page > MAX_PAGE
end

def limit_total_entries
def limit_total_count
class << @gems
def total_entries
MAX_PAGE * Rubygem.per_page
def total_count
MAX_PAGE * Rubygem.default_per_page
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/stats_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ def index
@number_of_downloads = GemDownload.total_count
@most_downloaded = Rubygem.by_downloads
.includes(:gem_download)
.paginate(page: @page, per_page: 10, total_entries: 100)
.page(@page).per(10).limit(100)
@most_downloaded_count = GemDownload.most_downloaded_gem_count
end
end
6 changes: 6 additions & 0 deletions app/helpers/application_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,4 +51,10 @@ def search_form_class
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
end
4 changes: 4 additions & 0 deletions app/views/layouts/_plain_paginate.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<nav class="pagination">
<%= link_to_prev_page items, '‹ Prev' %>
<%= link_to_next_page 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, model: 'gem' %></p>
<p class="gems__meter"><%= page_entries_info @rubygems %></p>
</header>

<%= render partial: "news/rubygem", collection: @rubygems %>
<%= will_paginate @rubygems %>
<%= paginate @rubygems %>
3 changes: 1 addition & 2 deletions app/views/reverse_dependencies/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@
<div class="reverse__dependencies">
<%= render @reverse_dependencies %>
</div>

<%= will_paginate @reverse_dependencies %>
<%= plain_paginate @reverse_dependencies %>
</div>
<%= render 'rubygems/aside' %>
</div>
4 changes: 2 additions & 2 deletions app/views/rubygems/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@

<header class="gems__header">
<p class="gems__meter">
<%= page_entries_info @gems, :entry_name => 'gem', :model => 'gem' %>
<%= page_entries_info @gems, :entry_name => 'gem'%>
</p>
</header>

<%= render @gems %>

<%= will_paginate @gems %>
<%= paginate @gems %>
4 changes: 2 additions & 2 deletions app/views/searches/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
<% end %>

<header class="gems__header push">
<p class="gems__meter"><%= page_entries_info(@gems, :entry_name => 'gem', :model => 'gem').html_safe %></p>
<p class="gems__meter"><%= page_entries_info(@gems, :entry_name => 'gem').html_safe %></p>
</header>

<%= render partial: 'aggregations', locals: { gems: @gems } %>
Expand All @@ -38,6 +38,6 @@

<% if @gems.present? %>
<%= render partial: 'rubygems/rubygem', collection: @gems %>
<%= will_paginate @gems %>
<%= paginate @gems %>
<% end %>
<% end %>
4 changes: 2 additions & 2 deletions app/views/stats/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
</h2>

<header class="gems__header">
<p class="gems__meter"><%= page_entries_info @most_downloaded, model: 'gem' %></p>
<p class="gems__meter"><%= page_entries_info @most_downloaded, entry_name: 'gem' %></p>
</header>

<% @most_downloaded.each do |gem| %>
Expand All @@ -33,4 +33,4 @@
</div>
</div>
<% end %>
<%= will_paginate @most_downloaded %>
<%= paginate @most_downloaded %>
13 changes: 13 additions & 0 deletions config/initializers/kaminari_config.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# frozen_string_literal: true

Kaminari.configure do |config|
config.default_per_page = 30
# config.max_per_page = nil
# config.window = 4
# config.outer_window = 0
# config.left = 0
# config.right = 0
# config.page_method_name = :page
# config.param_name = :page
# config.params_on_first_page = false
end
4 changes: 2 additions & 2 deletions test/integration/search_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,14 +56,14 @@ class SearchTest < SystemTest
end

test "params has non white listed keys" do
Rubygem.per_page = 1
Kaminari.configure { |c| c.default_per_page = 1 }
create(:rubygem, name: "ruby-ruby", number: '1.0.0')
create(:rubygem, name: "ruby-gems", number: '1.0.0')
import_and_refresh

visit '/search?query=ruby&script_name=javascript:alert(1)//'
assert page.has_content? "ruby-ruby"
assert page.has_link?("Next", href: "/search?page=2&query=ruby")
Rubygem.per_page = 30
Kaminari.configure { |c| c.default_per_page = 30 }
end
end