Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

deprecate PagerInterface::getNbResults() in favour of countResults() #6732

Merged
merged 10 commits into from
Jan 6, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions UPGRADE-3.x.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,17 @@
UPGRADE 3.x
===========

UPGRADE FROM 3.85 to 3.86
=========================

### Sonata\AdminBundle\Datagrid\PagerInterface

Deprecated `getNbResults()` method in favor of `countResults()`.

### Sonata\AdminBundle\Datagrid\Pager and Sonata\AdminBundle\Datagrid\SimplePager

Deprecated `$nbResults` property, `getNbResults()` and `setNbResults()` methods.

UPGRADE FROM 3.83 to 3.84
=========================

Expand Down
12 changes: 11 additions & 1 deletion src/Action/SearchAction.php
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,17 @@ public function __invoke(Request $request): Response
];
}
$page = (int) $pager->getPage();
$total = (int) $pager->getNbResults();

// NEXT_MAJOR: remove the existence check and just use $pager->countResults() without casting to int
if (method_exists($pager, 'countResults')) {
dmaicher marked this conversation as resolved.
Show resolved Hide resolved
$total = (int) $pager->countResults();
VincentLanglet marked this conversation as resolved.
Show resolved Hide resolved
} else {
@trigger_error(sprintf(
'Not implementing "%s::countResults()" is deprecated since sonata-project/admin-bundle 3.86 and will fail in 4.0.',
'Sonata\AdminBundle\Datagrid\PagerInterface'
), E_USER_DEPRECATED);
$total = (int) $pager->getNbResults();
}
}

$response = new JsonResponse([
Expand Down
38 changes: 36 additions & 2 deletions src/Datagrid/Pager.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ abstract class Pager implements \Iterator, \Countable, \Serializable, PagerInter
protected $lastPage = 1;

/**
* NEXT_MAJOR: Remove this property.
*
* @deprecated since sonata-project/admin-bundle 3.86
*
* @var int
*/
protected $nbResults = 0;
Expand Down Expand Up @@ -222,7 +226,19 @@ public function getLinks($nbLinks = null)
*/
public function haveToPaginate()
{
return $this->getMaxPerPage() && $this->getNbResults() > $this->getMaxPerPage();
// NEXT_MAJOR: remove the existence check and just use $pager->countResults() without casting to int
if (method_exists($this, 'countResults')) {
$countResults = (int) $this->countResults();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$countResults = (int) $this->countResults();
$countResults = $this->countResults();

This is not needed, isn't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

} else {
@trigger_error(sprintf(
'Not implementing "%s::countResults()" is deprecated since sonata-project/admin-bundle 3.86 and will fail in 4.0.',
'Sonata\AdminBundle\Datagrid\PagerInterface'
), E_USER_DEPRECATED);

$countResults = (int) $this->getNbResults();
}

return $this->getMaxPerPage() && $countResults > $this->getMaxPerPage();
}

/**
Expand Down Expand Up @@ -441,10 +457,19 @@ public function getLastIndice()
}

/**
* NEXT_MAJOR: remove this method.
*
* @deprecated since sonata-project/admin-bundle 3.86, use countResults instead
*
* @return int
*/
public function getNbResults()
{
@trigger_error(sprintf(
'The method "%s()" is deprecated since sonata-project/admin-bundle 3.86 and will be removed in 4.0. Use countResults() instead.',
__METHOD__
), E_USER_DEPRECATED);

return $this->nbResults;
dmaicher marked this conversation as resolved.
Show resolved Hide resolved
}

Expand Down Expand Up @@ -739,7 +764,7 @@ public function valid()
/**
* NEXT_MAJOR: Remove this method.
*
* @deprecated since sonata-project/admin-bundle 3.84, use getNbResults instead
* @deprecated since sonata-project/admin-bundle 3.84, use countResults instead
*/
public function count()
{
Expand Down Expand Up @@ -836,12 +861,21 @@ public function getQuery()
}

/**
* NEXT_MAJOR: remove this method.
*
* @param int $nb
*
* @deprecated since sonata-project/admin-bundle 3.86
*
* @return void
*/
protected function setNbResults($nb)
VincentLanglet marked this conversation as resolved.
Show resolved Hide resolved
{
@trigger_error(sprintf(
'The method "%s()" is deprecated since sonata-project/admin-bundle 3.86 and will be removed in 4.0.',
__METHOD__
), E_USER_DEPRECATED);

$this->nbResults = $nb;
}

Expand Down
3 changes: 2 additions & 1 deletion src/Datagrid/PagerInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
* @method bool isFirstPage()
* @method bool isLastPage()
* @method int getNbResults()
VincentLanglet marked this conversation as resolved.
Show resolved Hide resolved
* @method int countResults()
* @method array getLinks(?int $nbLinks = null)
* @method bool haveToPaginate()
* @method ProxyQueryInterface|null getQuery()
Expand Down Expand Up @@ -99,7 +100,7 @@ public function setQuery($query);
public function getResults();

// NEXT_MAJOR: uncomment this method in 4.0
// public function getNbResults(): int;
// public function countResults(): int;

// NEXT_MAJOR: uncomment this method 4.0
// /**
Expand Down
13 changes: 13 additions & 0 deletions src/Datagrid/SimplePager.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,20 @@ public function __construct($maxPerPage = 10, $threshold = 1)
$this->setThreshold($threshold);
}

/**
* NEXT_MAJOR: remove this method.
*/
public function getNbResults()
{
@trigger_error(sprintf(
'The method "%s()" is deprecated since sonata-project/admin-bundle 3.86 and will be removed in 4.0. Use countResults() instead.',
__METHOD__
), E_USER_DEPRECATED);

return $this->countResults();
}

public function countResults(): int
{
$n = ($this->getLastPage() - 1) * $this->getMaxPerPage();
if ($this->getLastPage() === $this->getPage()) {
Expand Down
6 changes: 4 additions & 2 deletions src/Resources/views/Block/block_search_result.html.twig
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,10 @@ file that was distributed with this source code.
</h3>

<div class="box-tools pull-right">
{% if pager and pager.getNbResults() > 0 %}
<span class="badge">{{ pager.getNbResults() }}</span>
{# NEXT_MAJOR: remove the attribute check and just use .countResults() #}
{% if pager and (attribute(pager, 'countResults') is defined ? pager.countResults() : pager.getNbResults()) > 0 %}
{# NEXT_MAJOR: remove the attribute check and just use .countResults() #}
<span class="badge">{{ (attribute(pager, 'countResults') is defined ? pager.countResults() : pager.getNbResults()) }}</span>
{% elseif admin.hasRoute('create') and admin.hasAccess('create') %}
<a href="{{ admin.generateUrl('create') }}" class="btn btn-box-tool">
<i class="fa fa-plus" aria-hidden="true"></i>
Expand Down
6 changes: 4 additions & 2 deletions src/Resources/views/Block/block_stats.html.twig
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,12 @@ file that was distributed with this source code.
<!-- small box -->
<div class="small-box {{ settings.color }}">
<div class="inner">
<h3>{{ pager.getNbResults() }}</h3>
{# NEXT_MAJOR: remove the attribute check and just use .countResults() #}
<h3>{{ attribute(pager, 'countResults') is defined ? pager.countResults() : pager.getNbResults() }}</h3>
<p>
{% if translation_domain %}
{{ settings.text|trans({'%count%': pager.getNbResults()}, translation_domain) }}
{# NEXT_MAJOR: remove the attribute check and just use .countResults() #}
{{ settings.text|trans({'%count%': attribute(pager, 'countResults') is defined ? pager.countResults() : pager.getNbResults()}, translation_domain) }}
{% else %}
{{ settings.text }}
{% endif %}
Expand Down
10 changes: 9 additions & 1 deletion tests/App/Datagrid/Pager.php
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,15 @@ public function getResults(): array
return $this->repository->all();
}

public function getNbResults()
/**
* NEXT_MAJOR: remove this method.
*/
public function getNbResults(): int
{
return $this->countResults();
}

public function countResults(): int
{
return \count($this->getResults());
}
Expand Down
46 changes: 37 additions & 9 deletions tests/Datagrid/PagerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,15 @@ class PagerTest extends TestCase

protected function setUp(): void
{
$this->pager = $this->getMockForAbstractClass(Pager::class);
$this->pager = $this->getMockForAbstractClass(
Pager::class,
[],
'',
true,
true,
true,
['countResults']
);
}

/**
Expand Down Expand Up @@ -136,11 +144,16 @@ public function testGetMaxRecordLimit(): void
$this->assertSame(99, $this->pager->getMaxRecordLimit());
}

/**
* NEXT_MAJOR: Remove this test.
*
* @group legacy
dmaicher marked this conversation as resolved.
Show resolved Hide resolved
*/
public function testGetNbResults(): void
{
$this->assertSame(0, $this->pager->getNbResults());

$this->callMethod($this->pager, 'setNbResults', [100]);
$this->setProperty($this->pager, 'nbResults', 100);

$this->assertSame(100, $this->pager->getNbResults());
}
Expand All @@ -155,7 +168,7 @@ public function testCount(): void
$this->expectDeprecation('The method "Sonata\AdminBundle\Datagrid\Pager::count()" is deprecated since sonata-project/admin-bundle 3.84 and will be removed in 4.0.');
$this->assertSame(0, $this->pager->count());

$this->callMethod($this->pager, 'setNbResults', [100]);
$this->setProperty($this->pager, 'nbResults', 100);

$this->assertSame(100, $this->pager->count());
}
Expand Down Expand Up @@ -293,7 +306,11 @@ public function testHaveToPaginate(): void
$this->pager->setMaxPerPage(10);
$this->assertFalse($this->pager->haveToPaginate());

$this->callMethod($this->pager, 'setNbResults', [100]);
$this->pager->expects($this->once())
->method('countResults')
->willReturn(100)
;

$this->assertTrue($this->pager->haveToPaginate());
}

Expand Down Expand Up @@ -417,7 +434,7 @@ public function testGetCursor(): void
$this->pager->setCursor(300);
$this->assertSame(0, $this->pager->getCursor());

$this->callMethod($this->pager, 'setNbResults', [100]);
$this->setProperty($this->pager, 'nbResults', 100);

$this->pager->setCursor(5);
$this->assertSame(5, $this->pager->getCursor());
Expand All @@ -442,7 +459,7 @@ public function testGetObjectByCursor(): void
$object3 = new \stdClass();
$object3->foo = 'bar3';

$this->callMethod($this->pager, 'setNbResults', [3]);
$this->setProperty($this->pager, 'nbResults', 3);

$query = $this->createMock(ProxyQueryInterface::class);

Expand Down Expand Up @@ -557,7 +574,7 @@ public function testGetLastIndex(): void
$this->pager->setPage(0);
$this->assertSame(0, $this->pager->getLastIndex());

$this->callMethod($this->pager, 'setNbResults', [100]);
$this->setProperty($this->pager, 'nbResults', 100);

$this->assertSame(100, $this->pager->getLastIndex());

Expand Down Expand Up @@ -590,7 +607,7 @@ public function testGetNext(): void
$object3 = new \stdClass();
$object3->foo = 'bar3';

$this->callMethod($this->pager, 'setNbResults', [3]);
$this->setProperty($this->pager, 'nbResults', 3);

$query = $this->createMock(ProxyQueryInterface::class);

Expand Down Expand Up @@ -654,7 +671,7 @@ public function testGetPrevious(): void
$object3 = new \stdClass();
$object3->foo = 'bar3';

$this->callMethod($this->pager, 'setNbResults', [3]);
$this->setProperty($this->pager, 'nbResults', 3);

$query = $this->createMock(ProxyQueryInterface::class);

Expand Down Expand Up @@ -770,4 +787,15 @@ protected function callMethod($obj, string $name, array $args = [])

return $method->invokeArgs($obj, $args);
}

/**
* NEXT_MAJOR: remove this method.
*/
private function setProperty(object $obj, string $name, $value): void
{
$class = new \ReflectionClass($obj);
dmaicher marked this conversation as resolved.
Show resolved Hide resolved
$property = $class->getProperty($name);
$property->setAccessible(true);
$property->setValue($obj, $value);
}
}
2 changes: 1 addition & 1 deletion tests/Datagrid/SimplePagerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ public function testNoPagesForNoResults(): void
$this->pager->setQuery($this->proxyQuery);
$this->pager->init();
$this->assertSame(1, $this->pager->getLastPage());
$this->assertSame(0, $this->pager->getNbResults());
$this->assertSame(0, $this->pager->countResults());
}

public function testInitNoQuery(): void
Expand Down