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

Fix pagination of "Page" models #1725

Merged
merged 3 commits into from
Jul 31, 2020
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 7 additions & 9 deletions app/controllers/administrate/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ def index
search_term).run
resources = apply_collection_includes(resources)
resources = order.apply(resources)
resources = resources.page(params[:page]).per(records_per_page)
resources = resources.page(params[:_page]).per(records_per_page)
page = Administrate::Page::Collection.new(dashboard, order: order)

render locals: {
Expand Down Expand Up @@ -105,27 +105,25 @@ def order
end

def sorting_attribute
params.fetch(resource_name, {}).fetch(
:order,
default_sorting_attribute,
)
sorting_params.fetch(:order) { default_sorting_attribute }
end

def default_sorting_attribute
nil
end

def sorting_direction
params.fetch(resource_name, {}).fetch(
:direction,
default_sorting_direction,
)
sorting_params.fetch(:direction) { default_sorting_direction }
end

def default_sorting_direction
nil
end

def sorting_params
Hash.try_convert(request.query_parameters[resource_name]) || {}
end

def dashboard
@dashboard ||= dashboard_class.new
end
Expand Down
4 changes: 2 additions & 2 deletions app/helpers/administrate/application_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,11 @@ def sanitized_order_params(page, current_field_name)
association_params = collection_names.map do |assoc_name|
{ assoc_name => %i[order direction page per_page] }
end
params.permit(:search, :id, :page, :per_page, association_params)
params.permit(:search, :id, :_page, :per_page, association_params)
end

def clear_search_params
params.except(:search, :page).permit(
params.except(:search, :_page).permit(
:per_page, resource_name => %i[order direction]
)
end
Expand Down
2 changes: 1 addition & 1 deletion app/views/administrate/application/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -62,5 +62,5 @@ It renders the `_table` partial to display details about the resources.
table_title: "page-title"
) %>

<%= paginate resources %>
<%= paginate resources, param_name: '_page' %>
</section>
4 changes: 4 additions & 0 deletions spec/example_app/app/controllers/admin/pages_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
module Admin
class PagesController < Admin::ApplicationController
end
end
34 changes: 34 additions & 0 deletions spec/example_app/app/dashboards/page_dashboard.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
require "administrate/base_dashboard"

class PageDashboard < Administrate::BaseDashboard
ATTRIBUTE_TYPES = {
product: Field::BelongsTo,
id: Field::Number,
title: Field::String,
body: Field::Text,
created_at: Field::DateTime,
updated_at: Field::DateTime,
}.freeze

COLLECTION_ATTRIBUTES = %i[
id
title
].freeze

SHOW_PAGE_ATTRIBUTES = %i[
product
id
title
body
created_at
updated_at
].freeze

FORM_ATTRIBUTES = %i[
product
title
body
].freeze

COLLECTION_FILTERS = {}.freeze
end
2 changes: 2 additions & 0 deletions spec/example_app/app/dashboards/product_dashboard.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
class ProductDashboard < Administrate::BaseDashboard
ATTRIBUTES = [
:name,
:pages,
:price,
:description,
:image_url,
Expand All @@ -16,6 +17,7 @@ class ProductDashboard < Administrate::BaseDashboard
description: Field::Text,
image_url: Field::Url,
name: Field::String,
pages: Field::HasMany,
price: Field::Number.with_options(prefix: "$", decimals: 2),
product_meta_tag: Field::HasOne,
release_year: Field::Select.with_options(
Expand Down
3 changes: 3 additions & 0 deletions spec/example_app/app/models/page.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
class Page < ApplicationRecord
belongs_to :product
end
1 change: 1 addition & 0 deletions spec/example_app/app/models/product.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ def self.policy_class
end

has_many :line_items, dependent: :destroy
has_many :pages, dependent: :destroy
has_one :product_meta_tag, dependent: :destroy

validates :description, presence: true
Expand Down
2 changes: 2 additions & 0 deletions spec/example_app/app/policies/page_policy.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
class PagePolicy < ApplicationPolicy
end
1 change: 1 addition & 0 deletions spec/example_app/config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
resources :line_items
resources :log_entries
resources :orders
resources :pages
resources :products
resources :product_meta_tags, except: [:index]
resources :payments, only: [:index, :show]
Expand Down
11 changes: 11 additions & 0 deletions spec/example_app/db/migrate/20200714081950_create_pages.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
class CreatePages < ActiveRecord::Migration[6.0]
def change
create_table :pages do |t|
t.string :title
t.text :body
t.belongs_to :product, foreign_key: true

t.timestamps
end
end
end
12 changes: 11 additions & 1 deletion spec/example_app/db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema.define(version: 2020_03_26_202615) do
ActiveRecord::Schema.define(version: 2020_07_14_081950) do

# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
Expand Down Expand Up @@ -77,6 +77,15 @@
t.index ["customer_id"], name: "index_orders_on_customer_id"
end

create_table "pages", force: :cascade do |t|
t.string "title"
t.text "body"
t.bigint "product_id"
t.datetime "created_at", precision: 6, null: false
t.datetime "updated_at", precision: 6, null: false
t.index ["product_id"], name: "index_pages_on_product_id"
end

create_table "payments", id: :serial, force: :cascade do |t|
t.integer "order_id"
t.datetime "created_at", null: false
Expand Down Expand Up @@ -111,5 +120,6 @@
add_foreign_key "line_items", "orders"
add_foreign_key "line_items", "products"
add_foreign_key "orders", "customers"
add_foreign_key "pages", "products"
add_foreign_key "payments", "orders"
end
18 changes: 18 additions & 0 deletions spec/example_app/db/seeds.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,24 @@
Product.create! attributes.merge(price: 20 + rand(50))
end

Product.find_each do |p|
Page.create!(
title: "Something about #{p.name}",
body: Faker::Lorem.paragraph,
product: p,
)
Page.create!(
title: "The secrets of the game #{p.name}",
body: Faker::Lorem.paragraph,
product: p,
)
Page.create!(
title: "If you liked #{p.name}, you will love these games",
body: Faker::Lorem.paragraph,
product: p,
)
end

customers.each do |customer|
(1..3).to_a.sample.times do
order = Order.create!(
Expand Down
88 changes: 88 additions & 0 deletions spec/example_app/spec/features/pagination_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
require "rails_helper"

search_input_selector = ".search__input"

RSpec.feature "Pagination", type: :feature do
def expect_to_appear_in_order(*elements)
positions = elements.map { |e| page.body.index(e) }
expect(positions).to eq(positions.sort)
end

it "paginates records based on a query param" do
customers = create_list(:customer, 2)

visit admin_customers_path(per_page: 1)

expect(page).not_to have_content(customers.last.name)
click_on "Next"
expect(page).to have_content(customers.last.name)
end

describe "sorting" do
it "allows sorting by columns" do
create(:customer, name: "unique name two")
create(:customer, name: "unique name one")

visit admin_customers_path
click_on "Name"

expect_to_appear_in_order("unique name one", "unique name two")
end

it "allows clicking through after sorting", :js do
customer = create(:customer)
create(:order, customer: customer)

visit admin_customers_path
click_on "Name"
find("[data-url]").click
expect(page).to have_header("Show #{customer.name}")
end

it "allows reverse sorting" do
create(:customer, name: "unique name one")
create(:customer, name: "unique name two")

visit admin_customers_path
2.times { click_on "Name" }

expect_to_appear_in_order("unique name two", "unique name one")
end

it "toggles the order" do
create(:customer, name: "unique name one")
create(:customer, name: "unique name two")

visit admin_customers_path
3.times { click_on "Name" }
Copy link

Choose a reason for hiding this comment

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

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.


expect_to_appear_in_order("unique name one", "unique name two")
Copy link

Choose a reason for hiding this comment

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

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.

end

it "preserves search" do
Copy link

Choose a reason for hiding this comment

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

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.

query = "bar@baz.com"
Copy link

Choose a reason for hiding this comment

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

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.


visit admin_customers_path(search: query)
click_on "Name"
Copy link

Choose a reason for hiding this comment

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

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.


expect(find(search_input_selector).value).to eq(query)
end
end

context "with resources of type Page" do
Copy link

Choose a reason for hiding this comment

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

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.

it "can paginate and sort" do
Copy link

Choose a reason for hiding this comment

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

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.

FactoryBot.create(:page, title: "Page 2")
Copy link

Choose a reason for hiding this comment

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

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.

FactoryBot.create(:page, title: "Page 4")
Copy link

Choose a reason for hiding this comment

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

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.

FactoryBot.create(:page, title: "Page 1")
Copy link

Choose a reason for hiding this comment

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

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.

FactoryBot.create(:page, title: "Page 5")
Copy link

Choose a reason for hiding this comment

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

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.

FactoryBot.create(:page, title: "Page 3")
Copy link

Choose a reason for hiding this comment

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

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.


visit admin_pages_path(per_page: 3)
click_on "Title"
Copy link

Choose a reason for hiding this comment

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

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.

expect_to_appear_in_order("Page 1", "Page 2", "Page 3")
Copy link

Choose a reason for hiding this comment

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

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.


click_on "Next"
Copy link

Choose a reason for hiding this comment

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

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.

expect_to_appear_in_order("Page 4", "Page 5")
Copy link

Choose a reason for hiding this comment

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

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.

end
end
end
2 changes: 2 additions & 0 deletions spec/factories.rb
Original file line number Diff line number Diff line change
Expand Up @@ -69,4 +69,6 @@
sequence(:name) { |n| "Country #{n}" }
sequence(:code) { |n| "C#{n}" }
end

factory :page
end
66 changes: 0 additions & 66 deletions spec/features/index_page_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,72 +68,6 @@
expect(page).to have_table_header(custom_label)
end
end

it "paginates records based on a constant" do
customers = create_list(:customer, 2)

visit admin_customers_path(per_page: 1)

expect(page).not_to have_content(customers.last.name)
click_on "Next"
expect(page).to have_content(customers.last.name)
end

describe "sorting" do
def expect_to_appear_in_order(*elements)
positions = elements.map { |e| page.body.index(e) }
expect(positions).to eq(positions.sort)
end

it "allows sorting by columns" do
create(:customer, name: "unique name two")
create(:customer, name: "unique name one")

visit admin_customers_path
click_on "Name"

expect_to_appear_in_order("unique name one", "unique name two")
end

it "allows clicking through after sorting", :js do
customer = create(:customer)
create(:order, customer: customer)

visit admin_customers_path
click_on "Name"
find("[data-url]").click
expect(page).to have_header("Show #{customer.name}")
end

it "allows reverse sorting" do
create(:customer, name: "unique name one")
create(:customer, name: "unique name two")

visit admin_customers_path
2.times { click_on "Name" }

expect_to_appear_in_order("unique name two", "unique name one")
end

it "toggles the order" do
create(:customer, name: "unique name one")
create(:customer, name: "unique name two")

visit admin_customers_path
3.times { click_on "Name" }

expect_to_appear_in_order("unique name one", "unique name two")
end

it "preserves search" do
query = "bar@baz.com"

visit admin_customers_path(search: query)
click_on "Name"

expect(find(search_input_selector).value).to eq(query)
end
end
end

describe "search input" do
Expand Down