Process tag list page through Slim controller

7 files changed, 247 insertions, 51 deletions

diff --git a/application/front/controllers/TagCloudController.php b/application/front/controllers/TagCloudController.php
index 93e3ae27..1ff7c2e6 100644
--- a/application/front/controllers/TagCloudController.php
+++ b/application/front/controllers/TagCloudController.php
@@ -10,12 +10,15 @@ use Slim\Http\Response;
 /**
  * Class TagCloud
  *
- * Slim controller used to render the tag cloud page.
+ * Slim controller used to render the tag cloud and tag list pages.
  *
  * @package Front\Controller
  */
 class TagCloudController extends ShaarliController
 {
+    protected const TYPE_CLOUD = 'cloud';
+    protected const TYPE_LIST = 'list';
+
     /**
      * Display the tag cloud through the template engine.
      * This controller a few filters:
@@ -24,26 +27,53 @@ class TagCloudController extends ShaarliController
      */
     public function cloud(Request $request, Response $response): Response
     {
+        return $this->processRequest(static::TYPE_CLOUD, $request, $response);
+    }
+
+    /**
+     * Display the tag list through the template engine.
+     * This controller a few filters:
+     *   - Visibility stored in the session for logged in users
+     *   - `searchtags` query parameter: will return tags associated with filter in at least one bookmark
+     *   - `sort` query parameters:
+     *       + `usage` (default): most used tags first
+     *       + `alpha`: alphabetical order
+     */
+    public function list(Request $request, Response $response): Response
+    {
+        return $this->processRequest(static::TYPE_LIST, $request, $response);
+    }
+
+    /**
+     * Process the request for both tag cloud and tag list endpoints.
+     */
+    protected function processRequest(string $type, Request $request, Response $response): Response
+    {
         if ($this->container->loginManager->isLoggedIn() === true) {
             $visibility = $this->container->sessionManager->getSessionParameter('visibility');
         }
 
+        $sort = $request->getQueryParam('sort');
         $searchTags = $request->getQueryParam('searchtags');
         $filteringTags = $searchTags !== null ? explode(' ', $searchTags) : [];
 
         $tags = $this->container->bookmarkService->bookmarksCountPerTag($filteringTags, $visibility ?? null);
 
-        // TODO: the sorting should be handled by bookmarkService instead of the controller
-        alphabetical_sort($tags, false, true);
+        if (static::TYPE_CLOUD === $type || 'alpha' === $sort) {
+            // TODO: the sorting should be handled by bookmarkService instead of the controller
+            alphabetical_sort($tags, false, true);
+        }
 
-        $tagList = $this->formatTagsForCloud($tags);
+        if (static::TYPE_CLOUD === $type) {
+            $tags = $this->formatTagsForCloud($tags);
+        }
 
         $searchTags = implode(' ', escape($filteringTags));
         $data = [
             'search_tags' => $searchTags,
-            'tags' => $tagList,
+            'tags' => $tags,
         ];
-        $data = $this->executeHooks($data);
+        $data = $this->executeHooks('tag' . $type, $data);
         foreach ($data as $key => $value) {
             $this->assignView($key, $value);
         }
@@ -51,12 +81,19 @@ class TagCloudController extends ShaarliController
         $searchTags = !empty($searchTags) ? $searchTags .' - ' : '';
         $this->assignView(
             'pagetitle',
-            $searchTags . t('Tag cloud') .' - '. $this->container->conf->get('general.title', 'Shaarli')
+            $searchTags . t('Tag '. $type) .' - '. $this->container->conf->get('general.title', 'Shaarli')
         );
 
-        return $response->write($this->render('tag.cloud'));
+        return $response->write($this->render('tag.'. $type));
     }
 
+    /**
+     * Format the tags array for the tag cloud template.
+     *
+     * @param array<string, int> $tags List of tags as key with count as value
+     *
+     * @return mixed[] List of tags as key, with count and expected font size in a subarray
+     */
     protected function formatTagsForCloud(array $tags): array
     {
         // We sort tags alphabetically, then choose a font size according to count.
@@ -81,12 +118,12 @@ class TagCloudController extends ShaarliController
     /**
      * @param mixed[] $data Template data
      *
-     * @return mixed[] Template data after active plugins render_picwall hook execution.
+     * @return mixed[] Template data after active plugins hook execution.
      */
-    protected function executeHooks(array $data): array
+    protected function executeHooks(string $template, array $data): array
     {
         $this->container->pluginManager->executeHooks(
-            'render_tagcloud',
+            'render_'. $template,
             $data,
             ['loggedin' => $this->container->loginManager->isLoggedIn()]
         );
diff --git a/doc/md/Translations.md b/doc/md/Translations.md
index b8b7053f..c1a2885d 100644
--- a/doc/md/Translations.md
+++ b/doc/md/Translations.md
@@ -45,7 +45,7 @@ http://<replace_domain>/login
 http://<replace_domain>/picture-wall
 http://<replace_domain>/?do=pluginadmin
 http://<replace_domain>/tag-cloud
-http://<replace_domain>/?do=taglist
+http://<replace_domain>/tag-list
 ```
 
 #### Improve existing translation
diff --git a/index.php b/index.php
index 6ecb9a67..89a1e581 100644
--- a/index.php
+++ b/index.php
@@ -622,28 +622,7 @@ function renderPage($conf, $pluginManager, $bookmarkService, $history, $sessionM
 
     // -------- Tag list
     if ($targetPage == Router::$PAGE_TAGLIST) {
-        $visibility = ! empty($_SESSION['visibility']) ? $_SESSION['visibility'] : '';
-        $filteringTags = isset($_GET['searchtags']) ? explode(' ', $_GET['searchtags']) : [];
-        $tags = $bookmarkService->bookmarksCountPerTag($filteringTags, $visibility);
-
-        if (! empty($_GET['sort']) && $_GET['sort'] === 'alpha') {
-            alphabetical_sort($tags, false, true);
-        }
-
-        $searchTags = implode(' ', escape($filteringTags));
-        $data = [
-            'search_tags' => $searchTags,
-            'tags' => $tags,
-        ];
-        $pluginManager->executeHooks('render_taglist', $data, ['loggedin' => $loginManager->isLoggedIn()]);
-
-        foreach ($data as $key => $value) {
-            $PAGE->assign($key, $value);
-        }
-
-        $searchTags = ! empty($searchTags) ? $searchTags .' - ' : '';
-        $PAGE->assign('pagetitle', $searchTags . t('Tag list') .' - '. $conf->get('general.title', 'Shaarli'));
-        $PAGE->renderPage('tag.list');
+        header('Location: ./tag-list');
         exit;
     }
 
@@ -1870,6 +1849,7 @@ $app->group('', function () {
     $this->get('/logout', '\Shaarli\Front\Controller\LogoutController:index')->setName('logout');
     $this->get('/picture-wall', '\Shaarli\Front\Controller\PictureWallController:index')->setName('picwall');
     $this->get('/tag-cloud', '\Shaarli\Front\Controller\TagCloudController:cloud')->setName('tagcloud');
+    $this->get('/tag-list', '\Shaarli\Front\Controller\TagCloudController:list')->setName('taglist');
     $this->get('/add-tag/{newTag}', '\Shaarli\Front\Controller\TagController:addTag')->setName('add-tag');
 })->add('\Shaarli\Front\ShaarliMiddleware');
 
diff --git a/tests/front/controller/TagCloudControllerTest.php b/tests/front/controller/TagCloudControllerTest.php
index 352bdee2..719610d7 100644
--- a/tests/front/controller/TagCloudControllerTest.php
+++ b/tests/front/controller/TagCloudControllerTest.php
@@ -30,6 +30,9 @@ class TagCloudControllerTest extends TestCase
         $this->controller = new TagCloudController($this->container);
     }
 
+    /**
+     * Tag Cloud - default parameters
+     */
     public function testValidCloudControllerInvokeDefault(): void
     {
         $this->createValidContainerMockSet();
@@ -42,7 +45,6 @@ class TagCloudControllerTest extends TestCase
         $expectedOrder = ['abc', 'def', 'ghi'];
 
         $request = $this->createMock(Request::class);
-        $request->expects(static::once())->method('getQueryParam')->with('searchtags')->willReturn(null);
         $response = new Response();
 
         // Save RainTPL assigned variables
@@ -92,7 +94,7 @@ class TagCloudControllerTest extends TestCase
     }
 
     /**
-     * Additional parameters:
+     * Tag Cloud - Additional parameters:
      *   - logged in
      *   - visibility private
      *   - search tags: `ghi` and `def` (note that filtered tags are not displayed anymore)
@@ -101,18 +103,17 @@ class TagCloudControllerTest extends TestCase
     {
         $this->createValidContainerMockSet();
 
-        $allTags = [
-            'ghi' => 1,
-            'abc' => 3,
-            'def' => 12,
-        ];
-
         $request = $this->createMock(Request::class);
         $request
-            ->expects(static::once())
             ->method('getQueryParam')
-            ->with('searchtags')
-            ->willReturn('ghi def')
+            ->with()
+            ->willReturnCallback(function (string $key): ?string {
+                if ('searchtags' === $key) {
+                    return 'ghi def';
+                }
+
+                return null;
+            })
         ;
         $response = new Response();
 
@@ -162,12 +163,14 @@ class TagCloudControllerTest extends TestCase
         static::assertLessThan(5, $assignedVariables['tags']['abc']['size']);
     }
 
+    /**
+     * Tag Cloud - empty
+     */
     public function testEmptyCloud(): void
     {
         $this->createValidContainerMockSet();
 
         $request = $this->createMock(Request::class);
-        $request->expects(static::once())->method('getQueryParam')->with('searchtags')->willReturn(null);
         $response = new Response();
 
         // Save RainTPL assigned variables
@@ -208,6 +211,182 @@ class TagCloudControllerTest extends TestCase
         static::assertCount(0, $assignedVariables['tags']);
     }
 
+    /**
+     * Tag List - Default sort is by usage DESC
+     */
+    public function testValidListControllerInvokeDefault(): void
+    {
+        $this->createValidContainerMockSet();
+
+        $allTags = [
+            'def' => 12,
+            'abc' => 3,
+            'ghi' => 1,
+        ];
+
+        $request = $this->createMock(Request::class);
+        $response = new Response();
+
+        // Save RainTPL assigned variables
+        $assignedVariables = [];
+        $this->assignTemplateVars($assignedVariables);
+
+        $this->container->bookmarkService
+            ->expects(static::once())
+            ->method('bookmarksCountPerTag')
+            ->with([], null)
+            ->willReturnCallback(function () use ($allTags): array {
+                return $allTags;
+            })
+        ;
+
+        // Make sure that PluginManager hook is triggered
+        $this->container->pluginManager
+            ->expects(static::at(0))
+            ->method('executeHooks')
+            ->willReturnCallback(function (string $hook, array $data, array $param): array {
+                static::assertSame('render_taglist', $hook);
+                static::assertSame('', $data['search_tags']);
+                static::assertCount(3, $data['tags']);
+
+                static::assertArrayHasKey('loggedin', $param);
+
+                return $data;
+            })
+        ;
+
+        $result = $this->controller->list($request, $response);
+
+        static::assertSame(200, $result->getStatusCode());
+        static::assertSame('tag.list', (string) $result->getBody());
+        static::assertSame('Tag list - Shaarli', $assignedVariables['pagetitle']);
+
+        static::assertSame('', $assignedVariables['search_tags']);
+        static::assertCount(3, $assignedVariables['tags']);
+
+        foreach ($allTags as $tag => $count) {
+            static::assertSame($count, $assignedVariables['tags'][$tag]);
+        }
+    }
+
+    /**
+     * Tag List - Additional parameters:
+     *   - logged in
+     *   - visibility private
+     *   - search tags: `ghi` and `def` (note that filtered tags are not displayed anymore)
+     *   - sort alphabetically
+     */
+    public function testValidListControllerInvokeWithParameters(): void
+    {
+        $this->createValidContainerMockSet();
+
+        $request = $this->createMock(Request::class);
+        $request
+            ->method('getQueryParam')
+            ->with()
+            ->willReturnCallback(function (string $key): ?string {
+                if ('searchtags' === $key) {
+                    return 'ghi def';
+                } elseif ('sort' === $key) {
+                    return 'alpha';
+                }
+
+                return null;
+            })
+        ;
+        $response = new Response();
+
+        // Save RainTPL assigned variables
+        $assignedVariables = [];
+        $this->assignTemplateVars($assignedVariables);
+
+        $this->container->loginManager->method('isLoggedin')->willReturn(true);
+        $this->container->sessionManager->expects(static::once())->method('getSessionParameter')->willReturn('private');
+
+        $this->container->bookmarkService
+            ->expects(static::once())
+            ->method('bookmarksCountPerTag')
+            ->with(['ghi', 'def'], BookmarkFilter::$PRIVATE)
+            ->willReturnCallback(function (): array {
+                return ['abc' => 3];
+            })
+        ;
+
+        // Make sure that PluginManager hook is triggered
+        $this->container->pluginManager
+            ->expects(static::at(0))
+            ->method('executeHooks')
+            ->willReturnCallback(function (string $hook, array $data, array $param): array {
+                static::assertSame('render_taglist', $hook);
+                static::assertSame('ghi def', $data['search_tags']);
+                static::assertCount(1, $data['tags']);
+
+                static::assertArrayHasKey('loggedin', $param);
+
+                return $data;
+            })
+        ;
+
+        $result = $this->controller->list($request, $response);
+
+        static::assertSame(200, $result->getStatusCode());
+        static::assertSame('tag.list', (string) $result->getBody());
+        static::assertSame('ghi def - Tag list - Shaarli', $assignedVariables['pagetitle']);
+
+        static::assertSame('ghi def', $assignedVariables['search_tags']);
+        static::assertCount(1, $assignedVariables['tags']);
+        static::assertSame(3, $assignedVariables['tags']['abc']);
+    }
+
+    /**
+     * Tag List - empty
+     */
+    public function testEmptyList(): void
+    {
+        $this->createValidContainerMockSet();
+
+        $request = $this->createMock(Request::class);
+        $response = new Response();
+
+        // Save RainTPL assigned variables
+        $assignedVariables = [];
+        $this->assignTemplateVars($assignedVariables);
+
+        $this->container->bookmarkService
+            ->expects(static::once())
+            ->method('bookmarksCountPerTag')
+            ->with([], null)
+            ->willReturnCallback(function (array $parameters, ?string $visibility): array {
+                return [];
+            })
+        ;
+
+        // Make sure that PluginManager hook is triggered
+        $this->container->pluginManager
+            ->expects(static::at(0))
+            ->method('executeHooks')
+            ->willReturnCallback(function (string $hook, array $data, array $param): array {
+                static::assertSame('render_taglist', $hook);
+                static::assertSame('', $data['search_tags']);
+                static::assertCount(0, $data['tags']);
+
+                static::assertArrayHasKey('loggedin', $param);
+
+                return $data;
+            })
+        ;
+
+        $result = $this->controller->list($request, $response);
+
+        static::assertSame(200, $result->getStatusCode());
+        static::assertSame('tag.list', (string) $result->getBody());
+        static::assertSame('Tag list - Shaarli', $assignedVariables['pagetitle']);
+
+        static::assertSame('', $assignedVariables['search_tags']);
+        static::assertCount(0, $assignedVariables['tags']);
+    }
+
+
     protected function createValidContainerMockSet(): void
     {
         $loginManager = $this->createMock(LoginManager::class);
diff --git a/tpl/default/changetag.html b/tpl/default/changetag.html
index cc74f786..a1a572ca 100644
--- a/tpl/default/changetag.html
+++ b/tpl/default/changetag.html
@@ -32,7 +32,7 @@
       </div>
     </form>
 
-    <p>{'You can also edit tags in the'|t} <a href="./?do=taglist&sort=usage">{'tag list'|t}</a>.</p>
+    <p>{'You can also edit tags in the'|t} <a href="./tag-list?sort=usage">{'tag list'|t}</a>.</p>
   </div>
 </div>
 {include="page.footer"}
diff --git a/tpl/default/tag.list.html b/tpl/default/tag.list.html
index 01b7a642..3e498f89 100644
--- a/tpl/default/tag.list.html
+++ b/tpl/default/tag.list.html
@@ -15,7 +15,7 @@
     <h2 class="window-title">{'Tag list'|t} - {$countTags} {'tags'|t}</h2>
     {if="!empty($search_tags)"}
       <p class="center">
-        <a href="?searchtags={$search_tags|urlencode}" class="pure-button pure-button-shaarli">
+        <a href="./?searchtags={$search_tags|urlencode}" class="pure-button pure-button-shaarli">
           {'List all links with those tags'|t}
         </a>
       </p>
@@ -57,7 +57,7 @@
             {/if}
 
             <a href="./add-tag/{$key|urlencode}" title="{'Filter by tag'|t}" class="count">{$value}</a>
-            <a href="?searchtags={$key|urlencode} {$search_tags|urlencode}" class="tag-link">{$key}</a>
+            <a href="./?searchtags={$key|urlencode} {$search_tags|urlencode}" class="tag-link">{$key}</a>
 
             {loop="$value.tag_plugin"}
               {$value}
diff --git a/tpl/default/tag.sort.html b/tpl/default/tag.sort.html
index b7aa7d80..f467e34a 100644
--- a/tpl/default/tag.sort.html
+++ b/tpl/default/tag.sort.html
@@ -2,7 +2,7 @@
   <div class="pure-u-1 pure-alert pure-alert-success tag-sort">
     {'Sort by:'|t}
     <a href="./tag-cloud">{'Cloud'|t}</a> &middot;
-    <a href="./?do=taglist&sort=usage">{'Most used'|t}</a> &middot;
-    <a href="./?do=taglist&sort=alpha">{'Alphabetical'|t}</a>
+    <a href="./tag-list?sort=usage">{'Most used'|t}</a> &middot;
+    <a href="./tag-list?sort=alpha">{'Alphabetical'|t}</a>
   </div>
 </div>