From 60ae241251b753fc052e50ebd95277dfcb074cb0 Mon Sep 17 00:00:00 2001 From: ArthurHoaro Date: Sat, 16 May 2020 14:56:22 +0200 Subject: [PATCH] Process tag list page through Slim controller --- .../front/controllers/TagCloudController.php | 59 ++++- doc/md/Translations.md | 2 +- index.php | 24 +-- .../controller/TagCloudControllerTest.php | 203 ++++++++++++++++-- tpl/default/changetag.html | 2 +- tpl/default/tag.list.html | 4 +- tpl/default/tag.sort.html | 4 +- 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: @@ -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 $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:///login http:///picture-wall http:///?do=pluginadmin http:///tag-cloud -http:///?do=taglist +http:///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 @@ -

{'You can also edit tags in the'|t} {'tag list'|t}.

+

{'You can also edit tags in the'|t} {'tag list'|t}.

{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 @@

{'Tag list'|t} - {$countTags} {'tags'|t}

{if="!empty($search_tags)"}

- + {'List all links with those tags'|t}

@@ -57,7 +57,7 @@ {/if} {$value} - {$key} + {$key} {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 @@
{'Sort by:'|t} {'Cloud'|t} · - {'Most used'|t} · - {'Alphabetical'|t} + {'Most used'|t} · + {'Alphabetical'|t}
-- 2.41.0