handle SonataAdminBundle Pager::nbresults deprecation

[dmaicher]

Subject

I am targeting this branch, because its BC.

Changelog

### Added
- implemented `Sonata\AdminBundle\Datagrid\PagerInterface::countResults()`

### Deprecated
- `Sonata\DoctrineORMAdminBundle\Datagrid\Pager::computeNbResult()`
- `Sonata\DoctrineORMAdminBundle\Datagrid\Pager::getNbResults()`
- `Sonata\DoctrineORMAdminBundle\Datagrid\Pager::setNbResults()`

[dmaicher]

@VincentLanglet before I continue: The Pager here is marked final via annotation. Do we keep BC then for people who extended this class? What's the policy there?

https://github.com/sonata-project/SonataDoctrineORMAdminBundle/blob/3.x/src/Datagrid/Pager.php#L25

[VincentLanglet]

Yes, I think we still keep BC for method with @final

Be careful, your implementation of countResults is wrong.
getResults is returning the results of the current page !

private $resultsCount;

public function setResultsCount($count) {
    $this->resultsCount = $count;
    // NEXT_MAJOR: Remove this line.
    $this->setNbResults($count, 'sonata_deprecation_mute'); // You need to override this method or make a new PR to sonataAdmin to allow to avoid the deprecation.
}

public function countResults() {
    $deprecatedCount = $this->getNbResults('sonata_deprecation_mute'); // You need to override this method or make a new PR to sonataAdmin to allow to avoid the deprecation.

    if ($deprecatedCount !== $this->resultsCount) {
        // trigger deprecation

        return $deprecatedCount;
    }

    return $this->resultsCount;
}

Maybe we can do something like this ?

I think it would be good to deprecate getResults in favor of getPageResults in SonataAdmin too.
cc @sonata-project/contributors

And if we're changing nbResult, we should rename computeNbResult.
But after rereading, the computeNbResult method should be private. So you should just

• Add a comment NEXT_MAJOR: restrict visibility to private and rename to computeResultsCount`
• Add a deprecation in this method.
• Making the call in init with sonata_deprecation_mute.

[dmaicher]

Be careful, your implementation of countResults is wrong.
getResults is returning the results of the current page !

I don't follow. Currently I just call getNbResults (which will trigger a deprecation though).

Will look into it then with keeping BC.

[VincentLanglet]

Be careful, your implementation of countResults is wrong.
getResults is returning the results of the current page !

I don't follow. Currently I just call getNbResults (which will trigger a deprecation though).

Will look into it then with keeping BC.

My bad, i'm tired, I read

count($this->getResults())

But I just discovered that

count($this->getResults()) !== $this->countResults()

[dmaicher]

Once SonataAdminBundle 3.86 was released we need to bump the minimum in composer.json here before we release 3.27

[OskarStark]

Can you bump it in this PR?

[VincentLanglet]

Can you bump it in this PR?

Need sonata-project/SonataAdminBundle#6756

[VincentLanglet]

3.86 is released :)

[OskarStark]

@phansys I merged this, because your suggestion was merged 👍🏻

[phansys]

@phansys I merged this, because your suggestion was merged 👍🏻

Thank you!