babdev/pagerfanta

Pagination library for PHP applications with support for several data providers

pagerfanta's Issues

Is there a way to use query builder result cache?

Hi, i'm using fractal + pagerfanta for pagination my result in my project. I wonder how can i apply result cache in my query builder when using pagerfanta?

        $adapter = new QueryAdapter($result);
        
        $pagerfanta = new Pagerfanta($adapter);

        $pagerfanta->setMaxPerPage($limitPerPage);
        $pagerfanta->setCurrentPage($page);

        $paginatorAdapter = new PagerfantaPaginatorAdapter($pagerfanta, function(int $page) use ($request, $limitPerPage) {
            $route = $request->attributes->get('_route');
            $inputParams = $request->attributes->get('_route_params');
            $newParams = array_merge($inputParams, $request->query->all());
            $newParams['page'] = $page;
            $newParams['limit'] = $limitPerPage;
            return $this->router->generate($route, $newParams, UrlGeneratorInterface::ABSOLUTE_URL);
        });

        $resource = new Collection($pagerfanta->getCurrentPageResults(), $transformer, 'data');
        $resource->setPaginator($paginatorAdapter);

Please remove positive-int typehint from `Pagerfanta::setCurrentPage`

By adding the positive-int typehint to Pagerfanta::setCurrentPage it only becomes annoying for consumers of the library. I understand that it's nice to add this but I think it should be just asserted in the method.

Currently PHPStan reports 20 errors on our project:

Parameter #1 $currentPage of method Pagerfanta\Pagerfanta<Entity>::setCurrentPage() expects int<1, max>, int given.

I have to add this:

Assert::greaterThanEq($page, 1);

in all 20 places where I call:

$pagerfanta->setCurrentPage($page);

Would it be possible to reconsider this?

Is there any way to set the max per page to "all items"?

Hey!

I'm considering migrating to this package, as it's mostly a direct replacemente from the package I'm currently using, but this one seems a bit more maintained.

The only thing I'm not being able to do with pagerfanta (or I haven't found the way so far) is allowing to "remove" the limit. Let me elaborate.

The package I'm currently using allows you to set -1 as the max items per page. When you do this, it internally fetches the number of results from the adapter, and when calling the equivalent to getSlice, you get that value as $length, so it's basically to get all results.

I don't see a similar way to do this with pagerfanta, as Pagerfanta::setMaxPerPage seems to not allow providing anything but possitive numbers.

I agree this -1 approach is a bit hacky. Maybe supporting a nullable length would be another option, letting adapter implementations to decide how to act, but having support for this would be useful.

Any thoughts?

Of course, if this is not supported but you are open to have it, I'm willing to provide a PR.

Move Twig Integration From Bundle

I added a Twig view to the bundle to use it for templating instead of the PHP class templates, it looks like a pretty good candidate to move to the main package given Twig's use throughout the PHP ecosystem.

Deprecations from within the package

Since I've upgraded this package (along with Symfony 5.0 => 5.1), I have the following deprecations :

Since babdev/pagerfanta 2.2: The "Pagerfanta\PagerfantaInterface" interface is deprecated and will be removed in 3.0. Use the "Pagerfanta\Pagerfanta" class instead.
Since babdev/pagerfanta 2.2: The "Pagerfanta\Exception\Exception" interface is deprecated and will be removed in 3.0, exceptions should implement "Pagerfanta\Exception\PagerfantaException" instead.

However, I've noticed these two interfaces are not referenced from the project's code, but from this package itself. Any reason why it wasn't fixed when the deprecations triggers were introduced ?

Stable release ruflin/elastica 7

I saw that ruflin/elastica has a new stable release for ES7. Do you look into supporting this version? Kind regards

Name of the package

Currently, this package has been submitted to packagist as babdev/pagerfanta.

If this because the official place to keep developing pagerfanta, maybe the pagerfanta/pagerfanta name could be transferred by the original maintainers.

One of the benefits of this transfer would be that you would then have the pagerfanta/ namespace on Packagist, which would make things much better if splitting adapters to separate repos.

/cc @sampart

Auto paginator feature

Hey !

Do you are interested by an Auto Paginator Feature ?

This is a generator that can be used to navigate across all results over all pages.
When page boundaries are encountered, the next page will be fetched automatically for continued iteration.

This can be seen in Stripe SDK for example (https://stripe.com/docs/api/pagination/auto?lang=php).

This can be useful when Pagerfanta is connected with a remote API, to seamless retrieve a large collection.

I can work on a PR if you want.

Create Tailwind Template

The bundle has a Twig template for a Tailwind CSS based paginator. Let's try to keep feature parity between Twig templates and PHP templates, and add a PHP template for Tailwind.

[Question] Why is the PagerfantaInterface deprecated?

Why is the PagerfantaInterface deprecated? And why is the suggested replacement a concrete implementation (the Pagerfanta class)?

Re-evaluate DBAL single table adapter due to DBAL 4.0 removals

doctrine/dbal#6179 (based on refactoring from doctrine/dbal#3836) deprecates methods from the Doctrine DBAL's query builder that allows fetching and resetting the query parts, which the single table adapter uses to function. This adapter's functionality will need to be re-evaluated to determine if it can be implemented without the features removed from DBAL 4.0 or it'll need to be deprecated/removed.

Is it possible to create our own adapter and use it?

Hi, is it possible to create our own adapter and use it in pagerfanta?

For example, i want to use Ds\Vector instead of array but currently pagerfanta only have ArrayAdapter.

Thanks.

Static analysis improvements

Hi,

this commit added strict phpstan parameter docblocks. While I agree that return types could be helpful I would argue that the input parameter hints will only cause consumers of this library to add more stuff to the phpstan ignore file without any real benefit.

This can also be witnessed by the fact that the ignore file had to be updated for the library itself.

Cursor and offset based paginators

Just a few things to consider for v4:
Sometimes there is a need to provide an offset-limit pagination instead of human readable "page numbers". This might be useful for API endpoints. Most of the time the offset would match the ($page-1)*$perPage but there might be situations where someone requests offset: 50, limit: 100. A new Adapter (sub)interface could be introduced to address that.
Another idea would be to allow cursor-based navigation, where page ID is a cursor. An example would be Shopify API or AWS Cognito API.

Fix Doctrine MongoDB ODM Adapter Test

The Doctrine MongoDB ODM adapter tests are mostly skipped right now because the test was originally written for 1.x of that package and not updated to support 2.x which made the Doctrine\ODM\MongoDB\Query\Query class final (and therefore cannot be mocked).

The tests for this adapter should be updated to validate the adapter's behavior.

(Note, ext/mongodb and the doctrine/mongodb-odm package are installed in the GitHub Actions builds, so the test environment should be usable enough)

Pagerfanta\Exception\Exception triggers deprecation log on every request

This is a duplicate of #13 but I want to re-open it, because this is a regression in my eyes.

I am seeing

Since pagerfanta/pagerfanta 2.2: The "Pagerfanta\Exception\Exception" interface is deprecated and will be removed in 3.0, exceptions should implement "Pagerfanta\Exception\PagerfantaException" instead

Updating to 3.0 is not a solution for quite a while, because of the 7.4 requirement and going back to a release that did not include the deprecation feels wrong too.

Can we work on fixing/removing the deprecation? Probably marking the interface as internal instead?
You are triggering a deprecation without any user land implementation. I is triggered simply by using pagerfanta.
I understand the reason why you did that, but this feels wrong to me.
Now a deprecation is triggered on every page load of every project out there that uses pagerfanta 2.x.

Pagerfanta::getAdapter being deprecated without any fallback

Why is this being deprecated? There is no real replacement for this and it breaks functionality in my app.

I'm using the FOSElasticaBundle which uses this library internally. When I create pagination the FOS bundle handles creating the Pagerfanta class and assigning the adapter. There is no way for me to get access to the adapter except for using the Pagerfanta::getAdapter method.

When I query elasticsearch there is an extra field that I can only access via the adapter to get my hit count per index. Example:

        $indexHits = [];
        foreach($results->getAdapter()->getAggregations()['_index']['buckets'] as $bucket) {
            $indexHits[$bucket['key']] = $bucket['doc_count'];
        }

Why is this being removed when there is no alternative? Doing without my hit count per index just isn't an option for me (and I am sure others will find access to this object rather handy for other similar purposes).

Add Template Parameter to AdapterInterface

The QueryAdapter has the queried type annotated as template parameter,
which allows more precise annotations (e.g. @return QueryAdapter<MyEntity).

It would be nice if the implemented AdapterInterface would have a template parameter as well.

Address deprecations from DBAL 3.4

Doctrine\DBAL\Query\QueryBuilder::getType() was deprecated in doctrine/dbal#5551, this most likely means removing the safeguard in the Pagerfanta\Doctrine\DBAL\QueryAdapter constructor to prevent using non-SELECT queries

Interface the Route Generator?

In the bundle, I created a RouteGeneratorInterface to describe the callable that is used for generating paginated URLs. Would there be value in deprecating the existing callable $routeGenerator signatures in favor of the typed PHP class? Note, I wouldn't change the interface signature, so it would still fulfill the callable typehint, but if the interface is added in at some point the requirements would change to require that instead of any callable.

Allow to setCurrentPageOffsetStart() and setCurrentPageOffsetEnd()

I have paginated iterable, say split in 5 items per page.

I want to be able to setCurrentPageOffsetStart() to, 2, for example. That way when I iterate over the items, I get item 3 to 5.

Similarly, I want to setCurrentPageOffsetEnd() to 4, for example, to iterate between the currentPageOffsetStart and currentPageOffsetEnd.

PHP 8 Compatibility

Right now this is based on the 3.x branch with running PHPUnit on each package in isolation. Compatibility on the 2.x branch is going to be at a best effort basis for the adapters because the dependency tree is more complex to build against the monopackage, but the core API should be OK.

Status (as of 19 October 2020):

• Adapters:
  • Doctrine:
    • Collections: Passes
    • DBAL: Passes with DBAL 2.12 and 3.x
    • MongoDB ODM: Non-skipped test passes
    • ORM: Passes with DBAL 2.12
    • PHPCR ODM: Passes
  • Elastica: Passes
  • Solarium: Passes
• Integrations:
  • Twig: Passes
• Core: Passes

Remove non-negative-int typehint on `FixedAdapter` `nbResults` argument

Same issue as this one, but this time for the nbResults argument of FixedAdapter's constructor.

    /**
     * @param iterable<array-key, T> $results
     *
     * @phpstan-param int<0, max> $nbResults
     */
    public function __construct(int $nbResults, iterable $results)
    {
        $this->nbResults = $nbResults;
        $this->results = $results;
    }

It's type-hinted as @phpstan-param int<0, max>, which is a annoying to have to assert on our end, especially in the common use-case of passing is the result of a count query from a repository, which is never going to be negative.

Could we type-hint it as a simple int instead?

Road To 3.0

The 2.2 release here is an update from the original package's last release (2.1.3) which was mostly code style adjustments, deprecating some adapters, and a PHP minimum version update.

A 3.0 version of this package will start soon. Some changes I am thinking about include:

• Removing deprecated elements
• PHP 7.4 minimum
• Strongly typed API
• Extracting the Doctrine, Elastica, and Solarium adapters into separate packages
  • The rationale here is it should be possible to add new "officially supported" adapters, deprecate adapters, and update adapters based on their upstream changes without forcing version updates of the base Pagerfanta API
  • Yet to be determined is how to deal with class naming (keep class names as is or rename when moved to an external package (such as Pagerfanta\Solarium\SolariumAdapter or Pagerfanta\DoctrineCollections\CollectionsAdapter))

The release timeframe on this is most likely going to match up with the release timeframe for the BabDevPagerfantaBundle 3.0 release, which will come around the time Symfony 3.4 stops receiving regular bug fixes (November 2020).

The intent for this release isn't to rewrite everything for the sake of rewriting, but rather to upgrade the code structure to reflect the modern version of PHP we have now versus the version that existed when the library was first written, and to also give more flexibility to the release patterns of the adapters.

Change default branch on github

As 3.0 is released, could you please change the default branch on github?

Thanks for your work. :)

Tag new release

I see that 3.x already fixed a bunch of deprecations for Symfony 6, great 🎉
Would be great to have it tagged 🙏

In README.md link "BabDev website" brings to a 404 page

https://www.babdev.com/open-source/packages/pagerfanta/docs/3.x

Add MongoDB Adapter

The current MongoAdapter uses the deprecated ext/mongo, and as such this adapter is now deprecated.

A new adapter supporting ext/mongodb should be created.

Cannot install pagerfanta/doctrine-orm-adapter

$ composer req babdev/pagerfanta-bundle pagerfanta/doctrine-orm-adapter
Using version ^2.5 for babdev/pagerfanta-bundle
Using version ^2.4 for pagerfanta/doctrine-orm-adapter
./composer.json has been updated
Loading composer repositories with package information
Updating dependencies (including require-dev)
Package operations: 2 installs, 0 updates, 0 removals
  - Installing pagerfanta/pagerfanta (v2.4.1): Loading from cache
  - Installing babdev/pagerfanta-bundle (v2.5.0): Loading from cache

How come it's installing pagerfanta/pagerfanta instead of pagerfanta/doctrine-orm-adapter?

Pagerfanta::setCurrentPage() must be called AFTER Pagerfanta::setMaxPerPage()

Hi!

I'm been testing out the library & bundle to include in a Symfonycasts screencast - it's very nice!

I ran into one minor issue:

// works correctly - yay!
$pagerfanta = new Pagerfanta(new QueryAdapter($queryBuilder));
$pagerfanta->setMaxPerPage(5);
$pagerfanta->setCurrentPage($request->query->get('page', 1));

// does not work - boo :(
$pagerfanta = new Pagerfanta(new QueryAdapter($queryBuilder));
$pagerfanta->setCurrentPage($request->query->get('page', 1));
$pagerfanta->setMaxPerPage(5);

When I say "does not work" - here is specifically what happens. Suppose I have 20 "items". This would be 2 pages using the default 10 max per page. With the 2nd code, it correctly renders 5 items on each page. And when rendering the pagination links, it correctly renders 4 pages. However, when you click to page 3, you get an error:

I believe that Pagerfanta::resetForCurrentPageChange() also needs to have $this->nbResults = null;.

Thanks!