search

amrnn90 / laravel-cursor-paginator

Cursor pagination for Laravel
MIT License
18 stars 6 forks source link

Previous page url always populates, even on first page #3

Closed trevorgehman closed 4 years ago

trevorgehman commented 4 years ago

The documentation states that prev_page_url will return null, but it seems that it always is returning a url, even if I'm on the first page of results. I'm trying to populate a hasPrev boolean, but it looks like this method is always returning false:

https://github.com/amrnn90/laravel-cursor-paginator/blob/bd28394f1b1ed6b6214287cf0e84c7ff7bb81ce9/src/CursorPaginator.php#L66-L69

trevorgehman commented 4 years ago

Actually I just noticed this method seems to always return true. Am I doing something wrong?

https://github.com/amrnn90/laravel-cursor-paginator/blob/bd28394f1b1ed6b6214287cf0e84c7ff7bb81ce9/src/CursorPaginator.php#L56-L59

amrnn90 commented 4 years ago

Hi, @trevorgehman This shouldn't be the case I believe. If you can provide a simple repo where I can reproduce this I would appreciate it.

trevorgehman commented 4 years ago

https://github.com/amrnn90/laravel-cursor-paginator/blob/95e0276e28091609c28afb4fa676d870b77a13c5/src/Query/QueryMeta.php#L61

When you're doing this comparison, it will always return a cursor when you are paginating a collection of Laravel models. When $meta->first and $itemsFirst both point to the same item, $meta->first != $itemsFirst is still true because they are different object instances. I think you need to use something like $meta->first->is($itemsFirst).

It's the same thing here:

https://github.com/amrnn90/laravel-cursor-paginator/blob/95e0276e28091609c28afb4fa676d870b77a13c5/src/Query/QueryMeta.php#L69

I've just resorted to returning this:


'meta' => [
          'next_cursor' => $this->meta['next']->target,
          'prev_cursor' => $this->meta['previous']->target,
          'has_more' => $this->resource->meta['next_item'] !== null,
]

Honestly I'm a little torn on whether the cursor should still be supplied even if there aren't any more results. It seems like we could provide the next_cursor and prev_cursor values, and then also expose has_next and has_previous booleans so we can choose whether to show them. I've see it done both ways with cursor paginated APIs.

amrnn90 commented 4 years ago

Thanks @trevorgehman, Please try out the new update and let me know if there are any more issues.

trevorgehman commented 4 years ago

Yes, the latest version seems to fix it!