Bug: Pagy seems to ignore #limit scope about pagy HOT 6 CLOSED

Thank you for your report.

Not sure if this is something pagy can solve:

scope = User.limit(1)
scope.limit(100) # will come out with 100 records, not 1 - and pagy not involved here at all in this example

It is a known "issue" with Active Record and is the intended behaviour....but should it be?

All pagy does is basically apply limit and offset in the background. Not sure if it's worth the effort to solve it with pagy.

Thoughts?

from pagy.

The @users_limited returns up to Pagy::DEFAULT[:items] amount of items. Intuitively, it should return up to 2
items.

Pagy just applies the limit to whichever scope you are paginating, hence it basically reset the AR limit to the :items variable.

I would say that 2 items would be "counter-intuitive" if you know how AR limit works.

Besides, with the last statement, you are basically stating that you want to paginate with the defined :items, effectively overriding the scope limit. It looks quite logic to me. Did I miss anything?

from pagy.

Added warning info to the how-to docs.

@cappe I hope this will avoid any confusion.

from pagy.

@benkoshy @ddnexus Thank you for taking time to answer, and adding the warning in the docs.

I think the AR behavior for (multiple) limits makes sense. When used by the developer themselves, it's usually somewhat visible and explicit. However, when Pagy does it, it's behind the scenes. I may know that Pagy (or pagination in general) is basically limit + offset but the next person may not (especially newcomers).

hence it basically reset the AR limit to the :items variable.

If I use limit in my code, I should be confident it to be applied, not reset behind the scenes.

from pagy.

If I use limit in my code, I should be confident it to be applied, not reset behind the scenes.

OK. So the alternative would be ignoring the :items if the collection is an ActiveRecord AND the collection has a set limit.

If you want to do it automatically, it would mean checking that 2 conditions at each request, which would be an expensive performance hit without any good reason.

If you want to avoid penalizing most requests (which are practically almost the totality), then you should explicitly pass some param to the constructor, which is equivalent to just explicitly set the :items to the limit you want, which indeed is the current behavior. 😜

from pagy.

Not too ideal either. I guess having the warning in the docs is enough. Considering use cases for using limit along with pagination isn't too common anyway. Thanks for helping sorting this out.

from pagy.