From b97a659883030905fc8310c7391face44e126e19 Mon Sep 17 00:00:00 2001 From: Damien Harper Date: Mon, 15 Apr 2019 01:22:14 +0200 Subject: [PATCH] Pagination (#60) * Performance improvement * Code coverage section in the tests README * Pagerfanta requirement * Expose json_decode as a twig filter * Do not address properties using getters because `entry` is not an `AuditEntry` anymore (plain array coming from Pagerfanta) * Include Pagerfanta at the bottom of the timeline * Use of Pagerfanta * Cast page query parameter as int * 50 items per page by default * Pagerfanta related unit-tests * json_decode twig filter unit-test * Safety check * Fixed issue with PostgreSQL --- composer.json | 4 +- .../Controller/AuditController.php | 12 ++- .../Reader/AuditReader.php | 86 +++++++++++++++++-- .../Resources/views/Audit/audits.html.twig | 2 +- .../views/Audit/entity_history.html.twig | 11 ++- .../Resources/views/Audit/helper.html.twig | 21 ++--- .../Twig/Extension/TwigExtension.php | 8 ++ .../Reader/AuditReaderTest.php | 22 +++++ .../Twig/Extension/TwigExtensionTest.php | 14 ++- tests/README.md | 6 +- 10 files changed, 158 insertions(+), 28 deletions(-) diff --git a/composer.json b/composer.json index 7cf3d93c..37ef46c1 100644 --- a/composer.json +++ b/composer.json @@ -13,7 +13,9 @@ "require": { "php": ">=7.1", "symfony/orm-pack": "^1.0", - "symfony/security-bundle": "^3.4 || ^4.0" + "symfony/security-bundle": "^3.4 || ^4.0", + "pagerfanta/pagerfanta": "^2.1", + "white-october/pagerfanta-bundle": "^1.2" }, "autoload": { "psr-4": { diff --git a/src/DoctrineAuditBundle/Controller/AuditController.php b/src/DoctrineAuditBundle/Controller/AuditController.php index 989dae9a..c599a0aa 100644 --- a/src/DoctrineAuditBundle/Controller/AuditController.php +++ b/src/DoctrineAuditBundle/Controller/AuditController.php @@ -2,7 +2,9 @@ namespace DH\DoctrineAuditBundle\Controller; +use DH\DoctrineAuditBundle\Reader\AuditReader; use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; +use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\Routing\Annotation\Route; @@ -26,15 +28,17 @@ public function listAuditsAction(): Response * * @param string $entity * @param int|string $id - * @param null|int $page - * @param null|int $pageSize + * @param int $page + * @param int $pageSize * * @return Response */ - public function showEntityHistoryAction(string $entity, $id = null, ?int $page = null, ?int $pageSize = null): Response + public function showEntityHistoryAction(Request $request, string $entity, $id = null): Response { + $page = (int) $request->query->get('page', 1); + $reader = $this->container->get('dh_doctrine_audit.reader'); - $entries = $reader->getAudits($entity, $id, $page, $pageSize); + $entries = $reader->getAuditsPager($entity, $id, $page, AuditReader::PAGE_SIZE); return $this->render('@DHDoctrineAudit/Audit/entity_history.html.twig', [ 'id' => $id, diff --git a/src/DoctrineAuditBundle/Reader/AuditReader.php b/src/DoctrineAuditBundle/Reader/AuditReader.php index 6a02a182..d9458fd3 100644 --- a/src/DoctrineAuditBundle/Reader/AuditReader.php +++ b/src/DoctrineAuditBundle/Reader/AuditReader.php @@ -3,8 +3,11 @@ namespace DH\DoctrineAuditBundle\Reader; use DH\DoctrineAuditBundle\AuditConfiguration; +use Doctrine\DBAL\Query\QueryBuilder; use Doctrine\DBAL\Statement; use Doctrine\ORM\EntityManagerInterface; +use Pagerfanta\Adapter\DoctrineDbalSingleTableAdapter; +use Pagerfanta\Pagerfanta; class AuditReader { @@ -14,6 +17,8 @@ class AuditReader const INSERT = 'insert'; const REMOVE = 'remove'; + const PAGE_SIZE = 50; + /** * @var AuditConfiguration */ @@ -113,6 +118,77 @@ public function getEntities(): array * @return array */ public function getAudits($entity, $id = null, ?int $page = null, ?int $pageSize = null): array + { + $queryBuilder = $this->getAuditsQueryBuilder($entity, $id, $page, $pageSize); + + /** @var Statement $statement */ + $statement = $queryBuilder->execute(); + $statement->setFetchMode(\PDO::FETCH_CLASS, AuditEntry::class); + + return $statement->fetchAll(); + } + + /** + * Returns an array of audited entries/operations. + * + * @param object|string $entity + * @param int|string $id + * @param null|int $page + * @param null|int $pageSize + * + * @return Pagerfanta + */ + public function getAuditsPager($entity, $id = null, int $page = 1, int $pageSize = self::PAGE_SIZE): Pagerfanta + { + $queryBuilder = $this->getAuditsQueryBuilder($entity, $id); + + $adapter = new DoctrineDbalSingleTableAdapter($queryBuilder, 'at.id'); + + $pagerfanta = new Pagerfanta($adapter); + $pagerfanta + ->setMaxPerPage($pageSize) + ->setCurrentPage($page) + ; + + return $pagerfanta; + } + + /** + * Returns the amount of audited entries/operations. + * + * @param object|string $entity + * @param int|string $id + * + * @throws \Doctrine\ORM\NonUniqueResultException + * + * @return int + */ + public function getAuditsCount($entity, $id = null): int + { + $queryBuilder = $this->getAuditsQueryBuilder($entity, $id); + + $result = $queryBuilder + ->resetQueryPart('select') + ->resetQueryPart('orderBy') + ->select('COUNT(id)') + ->execute() + ->fetchColumn(0) + ; + + return false === $result ? 0 : $result; + } + + /** + * Returns an array of audited entries/operations. + * + * @param object|string $entity + * @param int|string $id + * @param int $page + * @param int $pageSize + * + * @return QueryBuilder + */ + private function getAuditsQueryBuilder($entity, $id = null, ?int $page = null, ?int $pageSize = null): QueryBuilder { if (null !== $page && $page < 1) { throw new \InvalidArgumentException('$page must be greater or equal than 1.'); @@ -135,7 +211,7 @@ public function getAudits($entity, $id = null, ?int $page = null, ?int $pageSize $queryBuilder = $connection->createQueryBuilder(); $queryBuilder ->select('*') - ->from($auditTable) + ->from($auditTable, 'at') ->orderBy('created_at', 'DESC') ->addOrderBy('id', 'DESC') ; @@ -149,7 +225,7 @@ public function getAudits($entity, $id = null, ?int $page = null, ?int $pageSize if (null !== $id) { $queryBuilder - ->where('object_id = :object_id') + ->andWhere('object_id = :object_id') ->setParameter('object_id', $id); } @@ -159,11 +235,7 @@ public function getAudits($entity, $id = null, ?int $page = null, ?int $pageSize ->setParameter('filter', $this->filter); } - /** @var Statement $statement */ - $statement = $queryBuilder->execute(); - $statement->setFetchMode(\PDO::FETCH_CLASS, AuditEntry::class); - - return $statement->fetchAll(); + return $queryBuilder; } /** diff --git a/src/DoctrineAuditBundle/Resources/views/Audit/audits.html.twig b/src/DoctrineAuditBundle/Resources/views/Audit/audits.html.twig index ca3c1778..58980cbc 100644 --- a/src/DoctrineAuditBundle/Resources/views/Audit/audits.html.twig +++ b/src/DoctrineAuditBundle/Resources/views/Audit/audits.html.twig @@ -23,7 +23,7 @@ {{ entity|escape }} {{ table|escape }} - {{ bootstrap.badge(reader.audits(entity)|length ~ ' operations') }} + {{ bootstrap.badge(reader.getAuditsCount(entity) ~ ' operations') }} View audits diff --git a/src/DoctrineAuditBundle/Resources/views/Audit/entity_history.html.twig b/src/DoctrineAuditBundle/Resources/views/Audit/entity_history.html.twig index 9f7dd7ea..8006f7dc 100644 --- a/src/DoctrineAuditBundle/Resources/views/Audit/entity_history.html.twig +++ b/src/DoctrineAuditBundle/Resources/views/Audit/entity_history.html.twig @@ -21,10 +21,11 @@
{% for entry in entries %} + {% set diffs = entry.diffs|json_decode(true) %}
@@ -44,7 +45,7 @@ New value - {% for key, values in entry.diffs %} + {% for key, values in diffs %} {{ key }} @@ -95,6 +96,12 @@
{% endfor %}
+ + {% if entries.haveToPaginate() %} +
+ {{ pagerfanta(entries, 'twitter_bootstrap4') }} +
+ {% endif %} {% endblock dh_doctrine_audit_content %} diff --git a/src/DoctrineAuditBundle/Resources/views/Audit/helper.html.twig b/src/DoctrineAuditBundle/Resources/views/Audit/helper.html.twig index 88d9a7d3..fd3f88af 100644 --- a/src/DoctrineAuditBundle/Resources/views/Audit/helper.html.twig +++ b/src/DoctrineAuditBundle/Resources/views/Audit/helper.html.twig @@ -16,7 +16,8 @@ {% macro humanize(entity, entry) %} {% import _self as helper %} - {% set subject = entity~'#'~entry.getObjectId() %} + {% set diffs = entry.diffs|json_decode(true) %} + {% set subject = entity~'#'~entry.object_id %} {% if entry.type == 'insert' %} {% set action = 'inserted' %} @@ -24,22 +25,22 @@ {% set action = 'updated' %} {% elseif entry.type == 'remove' %} {% set action = 'deleted' %} - {% set source = entry.diffs %} + {% set source = diffs %} {% elseif entry.type == 'associate' %} {% set action = 'associated' %} - {% set source = entry.diffs.source %} - {% set target = entry.diffs.target %} + {% set source = diffs.source %} + {% set target = diffs.target %} {% set direction = 'to' %} {% elseif entry.type == 'dissociate' %} {% set action = 'dissociated' %} - {% set source = entry.diffs.source %} - {% set target = entry.diffs.target %} + {% set source = diffs.source %} + {% set target = diffs.target %} {% set direction = 'from' %} {% else %} {% set action = '???' %} {% endif %} - {{ subject }} + {{ subject }} {% if source is defined and subject != source.label %} ({{ source.label }}) {% endif %} @@ -51,8 +52,8 @@ ({{ helper.dump(target) }}) {% endif %} {% endif %} - by {{ entry.getUsername() is null ? 'unknown user' : entry.getUsername() }} - {% if entry.getIp() is not empty %} - , IP: {{ entry.getIp() }} + by {{ entry.blame_user is null ? 'unknown user' : entry.blame_user }} + {% if entry.ip is not empty %} + , IP: {{ entry.ip }} {% endif %} {% endmacro %} diff --git a/src/DoctrineAuditBundle/Twig/Extension/TwigExtension.php b/src/DoctrineAuditBundle/Twig/Extension/TwigExtension.php index c940c2dc..3010accf 100644 --- a/src/DoctrineAuditBundle/Twig/Extension/TwigExtension.php +++ b/src/DoctrineAuditBundle/Twig/Extension/TwigExtension.php @@ -4,6 +4,7 @@ use Symfony\Bridge\Doctrine\RegistryInterface; use Twig\Extension\AbstractExtension; +use Twig\TwigFilter; use Twig\TwigFunction; class TwigExtension extends AbstractExtension @@ -19,6 +20,13 @@ public function getFunctions(): array ]; } + public function getFilters(): array + { + return [ + new TwigFilter('json_decode', 'json_decode'), + ]; + } + public function __construct(RegistryInterface $doctrine) { $this->doctrine = $doctrine; diff --git a/tests/DoctrineAuditBundle/Reader/AuditReaderTest.php b/tests/DoctrineAuditBundle/Reader/AuditReaderTest.php index f3a4b77d..a56ee340 100644 --- a/tests/DoctrineAuditBundle/Reader/AuditReaderTest.php +++ b/tests/DoctrineAuditBundle/Reader/AuditReaderTest.php @@ -10,6 +10,7 @@ use DH\DoctrineAuditBundle\Tests\Fixtures\Core\Comment; use DH\DoctrineAuditBundle\Tests\Fixtures\Core\Post; use DH\DoctrineAuditBundle\Tests\Fixtures\Core\Tag; +use Pagerfanta\Pagerfanta; /** * @covers \DH\DoctrineAuditBundle\AuditConfiguration @@ -179,6 +180,27 @@ public function testGetAudits(): void $reader->getAudits(Post::class, null, -1, 50); } + public function testGetAuditsPager(): void + { + $reader = $this->getReader($this->getAuditConfiguration()); + + /** @var AuditEntry[] $audits */ + $pager = $reader->getAuditsPager(Author::class, null, 1, 3); + + $this->assertInstanceOf(Pagerfanta::class, $pager, 'pager is a Pagerfanta instance.'); + $this->assertTrue($pager->haveToPaginate(), 'pager has to paginate.'); + } + + public function testGetAuditsCount(): void + { + $reader = $this->getReader($this->getAuditConfiguration()); + + /** @var AuditEntry[] $audits */ + $count = $reader->getAuditsCount(Author::class, null); + + $this->assertSame(5, $count, 'count is ok.'); + } + /** * @depends testGetAudits */ diff --git a/tests/DoctrineAuditBundle/Twig/Extension/TwigExtensionTest.php b/tests/DoctrineAuditBundle/Twig/Extension/TwigExtensionTest.php index fe2932cc..996dc098 100644 --- a/tests/DoctrineAuditBundle/Twig/Extension/TwigExtensionTest.php +++ b/tests/DoctrineAuditBundle/Twig/Extension/TwigExtensionTest.php @@ -61,7 +61,19 @@ public function testGetFunctions(): void $this->assertNotEmpty($functions, 'extension has at least 1 function.'); foreach ($functions as $function) { - $this->assertInstanceOf('Twig_SimpleFunction', $function, 'function instanceof Twig_SimpleFunction'); + $this->assertInstanceOf('Twig\TwigFunction', $function, 'function instanceof Twig\TwigFunction'); + } + } + + public function testGetFilters(): void + { + $extension = new TwigExtension($this->container->get('doctrine')); + $filters = $extension->getFilters(); + + $this->assertNotEmpty($filters, 'extension has at least 1 filter.'); + + foreach ($filters as $filter) { + $this->assertInstanceOf('Twig\TwigFilter', $filter, 'filter instanceof Twig\TwigFilter'); } } diff --git a/tests/README.md b/tests/README.md index ec7356d7..c361e0ae 100644 --- a/tests/README.md +++ b/tests/README.md @@ -11,12 +11,14 @@ Then you can run the test suite with different configuration (SQLite, MySQL or P ### Default configuration (SQLite) -This configuration uses an in memory sqlite database, it's the fastest configuration. +This configuration uses an in memory SQLite database and generates code coverage report in `tests/coverage` folder (requires [Xdebug extension](https://xdebug.org/docs/install#configure-php)). ```bash ./vendor/bin/phpunit ``` -or + +You can also run tests using an in memory SQLite database and without generating code coverage report, it's the fastest configuration. + ```bash ./vendor/bin/phpunit -c tests/travis/sqlite.travis.xml ```