Uysim / pagy-cursor

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

Order is flipped when after is specified #20

Closed cschilbe closed 3 years ago

cschilbe commented 4 years ago

https://github.com/Uysim/pagy-cursor/blob/d196f1501b1b80d731c7ba33061566feae747553/lib/pagy_cursor/pagy/cursor.rb#L30

The default sort for uuid cursor is created_at desc, for the first set of results this is used. When specifying start, the order is changed to created_at asc, making the results different.

A temporary work-around, I have found, is to pass order: {created_at: :asc} to pagy_uuid_cursor. Unfortunately, I don't have a workaround for making it work with created_at: :desc yet.

cschilbe commented 4 years ago

I think the tests make this very apparent:

When there is no position or sort provided, the results are returned in descending order from post100 - post81 https://github.com/Uysim/pagy-cursor/blob/master/spec/pagy_cursor/uuid_cursor_spec.rb#L25

When after is provided, the results are returned from post31 - post50; in ascending order. https://github.com/Uysim/pagy-cursor/blob/master/spec/pagy_cursor/uuid_cursor_spec.rb#L51

Likewise, when before is provided, the results are in descending order but I would expect it to be the full previous page, ending before the item that was sent. https://github.com/Uysim/pagy-cursor/blob/master/spec/pagy_cursor/uuid_cursor_spec.rb#L43

Am I thinking about keyset pagination incorrectly or is this not doing at all what I would expect?

From what I can tell, after and before behavior need to be swapped and another reverse sort is required to make before function the way it should.

yunanhelmy commented 4 years ago

Hello @cschilbe,

From what I understood, the The default sort for uuid cursor is created_at desc. If you wish to go to the next page then you need to specify before pointer.

Let's say your case is list ordered by created_at asc. You would need to :

I also did that within my project. I had to switch between before and after for the next page.

Maybe @Uysim understood better.

Uysim commented 4 years ago

Hey @cschilbe So what @yunanhelmy mentioned is correct. Before mean getting the records before that cursor has created. The order will be descending. After mean getting the records after that cursor has created. The order will be ascending.

From this spec https://github.com/Uysim/pagy-cursor/blob/master/spec/pagy_cursor/uuid_cursor_spec.rb#L43 We get item before post5 which should be post4, post3, post2, post1

cschilbe commented 4 years ago

I would have expected the before and after params to return me a single page before or after the id I have provided in a consistent sorted order. Otherwise, the order of the results would change as you are paging forward or back, producing unexpected results. I ended up going with https://github.com/glebm/order_query which does it in this manner, it would be nice to have this behaviour as a pagy extension though.

Uysim commented 4 years ago

@cschilbe I am sorry. But from what I see that the gem already work with the way you wanted. So I am not sure the real problem. Which part did work yet? or do you have any feature to request?

To me, pagination should return multiple pages that is why it call paginate, right? I don't get why you this kind of pagination to return only 1 page?

chloerei commented 1 year ago

I add a little information why this causes confuse.

When I specify order: {created_at: :desc} it is assumed that the sequence is returned (actually uuid, but simplified with numbers)

10, 9, 8….

I wanted to get the content after 8, so I passed in after: 8, but a duplicate of 10, 9, 8 was returned.

The correct approach is to use before: 8, because the timestamp of the content after 8 is before 8 's timestamp.

The difference here is whether before/after is relative to the collection content or the sorting field, both of which make sense. But the documentation was not written. I didn’t know until I actually tested it.