Update the pager object with fresh dataset before returning #403
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Addresses #366
In the issue @flash-gordon identified root cause as eager evaluation of the pager, but my investigation showed that the problem is different. See what calling
users.page(2)
does:It calculates the new dataset and creates a new
Pager
holding this new dataset - and return newRelation
with new dataset and pager object. Now, if something else is called after it, for exampleusers.page(2).where(active: true)
, a newRelation
is constructed with a new dataset containing anactive=true
condition, but the pager still keep reference to old dataset, without this condition. As a result, when calculating#total_pages
etc., it uses stale dataset, without all the conditions - and this results in wrong numbers being returned.Now, I'm not very good at naming, so I'm sure some things might be improved here, but I believe this is the simplest (or easiest?) way to fix the bug.
As an alternative approach, I thought about creating additional private API class, say,
PagerConfig
, which would only holdcurrent_page
andper_page
and only create "real"Pager
object (with methods liketotal_pages
etc.) from the newly introducedpager
method onRelation
. But I'm not sure if this is not an overkill.