Paginate has_many show view #736
Conversation
|
Woot! Awesome! Thanks @acrogenesis! |
|
Could you add a simple feature spec please? This way no one breaks it in the future. |
|
@BenMorganIO I've added the test, I'm not entirely sure if it's the best way to make it. What do you think? |
acrogenesis
force-pushed
the
paginate_has_many
branch
from
6783811
to
5e277df
Compare
spec/features/show_page_spec.rb
Outdated
| it "displays the customers orders paginated" do | ||
| customer = create(:customer) | ||
| orders = [] | ||
| 10.times do |
There was a problem hiding this comment.
Refactor suggestion:
orders = 10.times.each_with_object([]) do |_index, arr|
arr << create(:order, customer: customer)
endThere was a problem hiding this comment.
I don't think we need to do this 10 times. That could slow the tests down a bit. You could do 2 orders and set Kaminari.per_page = 2. Something like:
first_order, second_order = Array.new(2) { create :order }
visit admin_customer_path customer
expect(page).to have_content(first_order.id)
expect(page).to have_no_content(second_order.id)
click_on 'Next >'
expect(page).to have_content(second_order.id)
expect(page).to have_no_content(first_order.id)
|
Hey @BenMorganIO I've done some improvements on the tests, sorry for taking so long I've had a lot of work. |
|
I like it! Going to squash and merge. |
|
Thanks! |
|
No, thank you @acrogenesis! |
* paginate has_many show view * test paginated has_many show view * improve has_many#show tests
| @@ -34,7 +34,7 @@ def svg_tag(asset, svg_id, options = {}) | |||
| end | |||
|
|
|||
| def sanitized_order_params | |||
| params.permit(:search, :id, :order, :page, :per_page, :direction) | |||
| params.permit(:search, :id, :order, :page, :per_page, :direction, :orders) | |||
There was a problem hiding this comment.
Sorry, a bit late to the party but: was this change intended? Looks to me like it comes from "orders" being one of the examples, and it slipped through.
There was a problem hiding this comment.
🤔 I think you are right. I'll run the tests without that and report back.
There was a problem hiding this comment.
OK, don't spend too long on this though! Sorry, I was looking at something else and this confused me and thought of commenting here.
There was a problem hiding this comment.
It fails:
1) customer show page displays the customers orders paginated when the second page is specified displays the second page of results
Failure/Error: params.permit(:search, :id, :order, :page, :per_page, :direction)
ActionView::Template::Error:
found unpermitted parameter: :orders
There was a problem hiding this comment.
Thank you for looking into that 👍
After your finding, I dug a bit deeper: it's a bug! orders is only relevant in that very specific example, where a "customer" has many "orders". The orders param tells the page number for the orders has_many list.
If I try to paginate through any other relationship, it breaks because the param has a different name.
No matter, I'll fix this.
This PR paginates the has_many show and takes into account that the show view can have several has_many fields.
Here you can see how the Attendee model is on page 1 and the User model is on page 2
PS. I blurred some personal information.