Uysim / pagy-cursor

Cursor pagination with pagy for Ruby On Rails
MIT License
126 stars 23 forks source link

return as ActiveRecord::Collection. Fix issue #28 #30

Closed yunanhelmy closed 2 years ago

yunanhelmy commented 2 years ago

Previously it was returned as an Array. Needs to return it as ActiveRecord::Relation.

Uysim commented 2 years ago

@yunanhelmy This is not a solution. We prevent has_more query here #19 . But we query again just to return active_record relation? I think the best solution is to revert back #19. And allow has_more requery.

yunanhelmy commented 2 years ago

@yunanhelmy This is not a solution. We prevent has_more query here #19 . But we query again just to return active_record relation?

You're right. I've just tested it will have another query when we return it items.limit(pagy.items). It's 2 queries again, even if it's different query. The point of having array is so we don't need to query it again in method pagy_cursor_has_more?.

I think the best solution is to revert back #19. And allow has_more requery.

I think this one is make sense, @Uysim.

walro commented 2 years ago

Yeah, agree with above points. Didn't look at the implementation of #pagy_cursor_has_more? (just looked at the diff here and it made sense to me) before approving 🤦.