rom-sql
rom-sql copied to clipboard
Pagination plugin returns incorect number of pages
Describe the bug
If you paginate over a filtered relation (say, users.active) then rel.total will return incorrect results since it calls dataset.unfiltered which discard all the filtering.
To Reproduce
user.active.pager.total returns the total number of users
Expected behavior
user.active.pager.total returns the number of active users
Your environment
- Affects my production application: NO (or maybe? 😅)
@flash-gordon it calls unlimited, not unfiltered
@solnic 🤦♂ ok
I'm still getting invalid results from .pager.dataset.unlimited.count, Sequel issues queries for the default dataset which is not expected. I'll take a deeper look
@flash-gordon oh wow that is puzzling
Got it, https://github.com/rom-rb/rom-sql/blob/bde6843526a4f5e2508932e9050e59a55dcf800e/lib/rom/sql/plugin/pagination.rb#L115-L121
It happens because pager is eagerly evaluated and passed around with options when you chain relation calls. It means it always sticks to the default dataset. This is 💯 a bug, we should use memoization instead
So, I just was bitten by this. And I discovered a funny thing, maybe it'd help someone:
relation.where { ... }.per_page(per_page).page(page) # this works correctly
relation.per_page(per_page).page(page).where { ... } # this does not
Anyway, I thought I could try to fix it and make a PR, but it seems that tests are currently not passing on master, so I'm not sure how to proceed.
@katafrakt please target release-3.5 branch and then I could cherry-pick your fix into master