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
DamienHarper · Apr 14, 2019 · b97a659 · b97a659
1 parent 54f364f
commit b97a659
Show file tree

Hide file tree

Showing 10 changed files with 158 additions and 28 deletions.
diff --git 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
@@ -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
@@ -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
@@ -23,7 +23,7 @@
                 <tr>
                     <td><code>{{ entity|escape }}</code></td>
                     <td>{{ table|escape }}</td>
-                    <td>{{ bootstrap.badge(reader.audits(entity)|length ~ ' operations') }}</td>
+                    <td>{{ bootstrap.badge(reader.getAuditsCount(entity) ~ ' operations') }}</td>
                     <td>
                         <a href="{{ path('dh_doctrine_audit_show_entity_history', { 'entity': entity }) }}">View audits</a>
                     </td>

diff --git a/src/DoctrineAuditBundle/Resources/views/Audit/entity_history.html.twig b/src/DoctrineAuditBundle/Resources/views/Audit/entity_history.html.twig
@@ -21,10 +21,11 @@
 
         <div class="timeline-centered">
         {% for entry in entries %}
+            {% set diffs = entry.diffs|json_decode(true) %}
             <article class="timeline-entry">
                 <div class="timeline-entry-inner">
                     <time class="timeline-time" datetime="2014-01-10T03:45">
-                        <span>{{ entry.getCreatedAt()|date('H:i:s') }}</span> <span>{{ entry.getCreatedAt()|date('l d F Y') }}</span>
+                        <span>{{ entry.created_at|date('H:i:s') }}</span> <span>{{ entry.created_at|date('l d F Y') }}</span>
                     </time>
 
                     <div class="timeline-icon bg-{{ bootstrap.label_type(entry.type) }}">
@@ -44,7 +45,7 @@
                             <th width="35%">New value</th>
                             </thead>
                             <tbody>
-                            {% for key, values in entry.diffs %}
+                            {% for key, values in diffs %}
                                 <tr>
                                     <td><code>{{ key }}</code></td>
                                     <td>
@@ -95,6 +96,12 @@
             </article>
         {% endfor %}
         </div>
+
+        {% if entries.haveToPaginate() %}
+        <div class="pagerfanta float-right">
+            {{ pagerfanta(entries, 'twitter_bootstrap4') }}
+        </div>
+        {% endif %}
     </div>
 </div>
 {% 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
@@ -16,30 +16,31 @@
 {% 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' %}
     {% elseif entry.type == 'update' %}
         {% 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 %}
 
-    <code><a href="{{ path('dh_doctrine_audit_show_entity_history', { 'entity': entity, 'id': entry.getObjectId() }) }}" class="code">{{ subject }}</a></code>
+    <code><a href="{{ path('dh_doctrine_audit_show_entity_history', { 'entity': entity, 'id': entry.object_id }) }}" class="code">{{ subject }}</a></code>
     {% if source is defined and subject != source.label %}
     <em>({{ source.label }})</em>
     {% endif %}
@@ -51,8 +52,8 @@
             (<em>{{ helper.dump(target) }}</em>)
         {% endif %}
     {% endif %}
-    by <b>{{ entry.getUsername() is null ? 'unknown user' : entry.getUsername() }}</b>
-    {% if entry.getIp() is not empty %}
-        , IP: {{ entry.getIp() }}
+    by <b>{{ entry.blame_user is null ? 'unknown user' : entry.blame_user }}</b>
+    {% 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
@@ -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
@@ -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
@@ -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
@@ -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 
 ```