Replace will_paginate with kaminari #1807
Conversation
96c6276 to
7123995
Compare
| </div> | ||
|
|
||
| <%= will_paginate @reverse_dependencies %> | ||
| <nav class="pagination"> |
There was a problem hiding this comment.
What about to move this to partial/helper (or combination of both) ?
|
🤣 I guess this is super approved since I tried to approve during the GitHub outage... |
We need option to avoid COUNT(DISTINCT ..) used to calculate total_entries or total pages. will_paginate doesn't support it.
COUNT(DISTINCT ..) used in pagination for page number links is very expensive in this case. This change will only show `next` and `prev` page link. change of includes to preload: kaminari overrides load method to load one extra record, which is used to check if there is a need for next page. `includes` was returning only per_page (30) entries instead of expected per_page + 1. In hindsight, preload also takes load off main query which finds all dependant gems and should have been used from start. response time of /gems/rails/reverse_dependencies (tested locally) before: 0m1.564s after: 0m0.708s
7123995 to
0e019e0
Compare
|
@simi updated! |
app/helpers/application_helper.rb
Outdated
| "is-active" if request.path_info == path | ||
| end | ||
|
|
||
| def without_count_paginate(items) |
There was a problem hiding this comment.
I think paginate_without_count looks better. But both describes actual implementation detail and not the expected output of this helper (having pagination missing numbered steps -> prev/next links only). What about some more generic name? Maybe simple_paginate (and leave a comment about performance in here)?
0e019e0 to
761eded
Compare
Used plain_paginate and added comment.
|
|
Merged locally, had to update vendor. |
|
Great @sonalkr132! |
COUNT(DISTINCT ..) used in pagination for page number links is very expensive
in this case. This change will only show
nextandprevpage link.response time of /gems/rails/reverse_dependencies (tested locally)
before: 0m1.564s
after: 0m0.708s
Related: #1798