Process tag list page through Slim controller

2020-05-16 14:56:22 +02:00 · 2020-05-16 14:56:22 +02:00 · 60ae241251
commit 60ae241251
parent 3772298ee7
7 changed files with 247 additions and 51 deletions
--- 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:
@ -23,27 +26,54 @@ class TagCloudController extends ShaarliController
     *   - `searchtags` query parameter: will return tags associated with filter in at least one bookmark
     */
    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()]
        );
--- 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
--- 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');

--- 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);
--- 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"}
--- 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}
--- 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>