From 8eac2e54882d8adae8cbb45386dca1b465242632 Mon Sep 17 00:00:00 2001 From: ArthurHoaro Date: Sat, 30 May 2020 15:51:14 +0200 Subject: Process manage tags page through Slim controller --- .../front/controller/admin/ConfigureController.php | 2 +- .../front/controller/admin/ManageTagController.php | 87 +++++++ assets/default/js/base.js | 6 +- assets/default/scss/shaarli.scss | 4 + index.php | 35 +-- .../controller/admin/ManageTagControllerTest.php | 272 +++++++++++++++++++++ tpl/default/page.header.html | 6 +- tpl/default/tag.list.html | 2 +- tpl/default/tools.html | 2 +- tpl/vintage/tools.html | 2 +- 10 files changed, 376 insertions(+), 42 deletions(-) create mode 100644 application/front/controller/admin/ManageTagController.php create mode 100644 tests/front/controller/admin/ManageTagControllerTest.php diff --git a/application/front/controller/admin/ConfigureController.php b/application/front/controller/admin/ConfigureController.php index b1d32270..5a482d8e 100644 --- a/application/front/controller/admin/ConfigureController.php +++ b/application/front/controller/admin/ConfigureController.php @@ -12,7 +12,7 @@ use Slim\Http\Response; use Throwable; /** - * Class PasswordController + * Class ConfigureController * * Slim controller used to handle Shaarli configuration page (display + save new config). */ diff --git a/application/front/controller/admin/ManageTagController.php b/application/front/controller/admin/ManageTagController.php new file mode 100644 index 00000000..e015e613 --- /dev/null +++ b/application/front/controller/admin/ManageTagController.php @@ -0,0 +1,87 @@ +getParam('fromtag') ?? ''; + + $this->assignView('fromtag', escape($fromTag)); + $this->assignView( + 'pagetitle', + t('Manage tags') .' - '. $this->container->conf->get('general.title', 'Shaarli') + ); + + return $response->write($this->render('changetag')); + } + + /** + * POST /manage-tags - Update or delete provided tag + */ + public function save(Request $request, Response $response): Response + { + $this->checkToken($request); + + $isDelete = null !== $request->getParam('deletetag') && null === $request->getParam('renametag'); + + $fromTag = escape(trim($request->getParam('fromtag') ?? '')); + $toTag = escape(trim($request->getParam('totag') ?? '')); + + if (0 === strlen($fromTag) || false === $isDelete && 0 === strlen($toTag)) { + $this->saveWarningMessage(t('Invalid tags provided.')); + + return $response->withRedirect('./manage-tags'); + } + + // TODO: move this to bookmark service + $count = 0; + $bookmarks = $this->container->bookmarkService->search(['searchtags' => $fromTag], BookmarkFilter::$ALL, true); + foreach ($bookmarks as $bookmark) { + if (false === $isDelete) { + $bookmark->renameTag($fromTag, $toTag); + } else { + $bookmark->deleteTag($fromTag); + } + + $this->container->bookmarkService->set($bookmark, false); + $this->container->history->updateLink($bookmark); + $count++; + } + + $this->container->bookmarkService->save(); + + if (true === $isDelete) { + $alert = sprintf( + t('The tag was removed from %d bookmark.', 'The tag was removed from %d bookmarks.', $count), + $count + ); + } else { + $alert = sprintf( + t('The tag was renamed in %d bookmark.', 'The tag was renamed in %d bookmarks.', $count), + $count + ); + } + + $this->saveSuccessMessage($alert); + + $redirect = true === $isDelete ? './manage-tags' : './?searchtags='. urlencode($toTag); + + return $response->withRedirect($redirect); + } +} diff --git a/assets/default/js/base.js b/assets/default/js/base.js index f61cfa92..8cc7eed5 100644 --- a/assets/default/js/base.js +++ b/assets/default/js/base.js @@ -546,7 +546,7 @@ function init(description) { const refreshedToken = document.getElementById('token').value; const fromtag = block.getAttribute('data-tag'); const xhr = new XMLHttpRequest(); - xhr.open('POST', './?do=changetag'); + xhr.open('POST', './manage-tags'); xhr.setRequestHeader('Content-Type', 'application/x-www-form-urlencoded'); xhr.onload = () => { if (xhr.status !== 200) { @@ -559,7 +559,7 @@ function init(description) { findParent(input, 'div', { class: 'rename-tag-form' }).style.display = 'none'; block.querySelector('a.tag-link').innerHTML = htmlEntities(totag); block.querySelector('a.tag-link').setAttribute('href', `./?searchtags=${encodeURIComponent(totag)}`); - block.querySelector('a.rename-tag').setAttribute('href', `./?do=changetag&fromtag=${encodeURIComponent(totag)}`); + block.querySelector('a.rename-tag').setAttribute('href', `./manage-tags?fromtag=${encodeURIComponent(totag)}`); // Refresh awesomplete values existingTags = existingTags.map(tag => (tag === fromtag ? totag : tag)); @@ -593,7 +593,7 @@ function init(description) { if (confirm(`Are you sure you want to delete the tag "${tag}"?`)) { const xhr = new XMLHttpRequest(); - xhr.open('POST', './?do=changetag'); + xhr.open('POST', './manage-tags'); xhr.setRequestHeader('Content-Type', 'application/x-www-form-urlencoded'); xhr.onload = () => { block.remove(); diff --git a/assets/default/scss/shaarli.scss b/assets/default/scss/shaarli.scss index 243ab1b2..759dff29 100644 --- a/assets/default/scss/shaarli.scss +++ b/assets/default/scss/shaarli.scss @@ -490,6 +490,10 @@ body, } } +.header-alert-message { + text-align: center; +} + // CONTENT - GENERAL .container { position: relative; diff --git a/index.php b/index.php index 50c0634a..00e4a40b 100644 --- a/index.php +++ b/index.php @@ -519,38 +519,7 @@ function renderPage($conf, $pluginManager, $bookmarkService, $history, $sessionM // -------- User wants to rename a tag or delete it if ($targetPage == Router::$PAGE_CHANGETAG) { - if (empty($_POST['fromtag']) || (empty($_POST['totag']) && isset($_POST['renametag']))) { - $PAGE->assign('fromtag', ! empty($_GET['fromtag']) ? escape($_GET['fromtag']) : ''); - $PAGE->assign('pagetitle', t('Manage tags') .' - '. $conf->get('general.title', 'Shaarli')); - $PAGE->renderPage('changetag'); - exit; - } - - if (!$sessionManager->checkToken($_POST['token'])) { - die(t('Wrong token.')); - } - - $toTag = isset($_POST['totag']) ? escape($_POST['totag']) : null; - $fromTag = escape($_POST['fromtag']); - $count = 0; - $bookmarks = $bookmarkService->search(['searchtags' => $fromTag], BookmarkFilter::$ALL, true); - foreach ($bookmarks as $bookmark) { - if ($toTag) { - $bookmark->renameTag($fromTag, $toTag); - } else { - $bookmark->deleteTag($fromTag); - } - $bookmarkService->set($bookmark, false); - $history->updateLink($bookmark); - $count++; - } - $bookmarkService->save(); - $delete = empty($_POST['totag']); - $redirect = $delete ? './do=changetag' : 'searchtags='. urlencode(escape($_POST['totag'])); - $alert = $delete - ? sprintf(t('The tag was removed from %d link.', 'The tag was removed from %d bookmarks.', $count), $count) - : sprintf(t('The tag was renamed in %d link.', 'The tag was renamed in %d bookmarks.', $count), $count); - echo ''; + header('./manage-tags'); exit; } @@ -1380,6 +1349,8 @@ $app->group('', function () { $this->post('/password', '\Shaarli\Front\Controller\Admin\PasswordController:change')->setName('changePassword'); $this->get('/configure', '\Shaarli\Front\Controller\Admin\ConfigureController:index')->setName('configure'); $this->post('/configure', '\Shaarli\Front\Controller\Admin\ConfigureController:save')->setName('saveConfigure'); + $this->get('/manage-tags', '\Shaarli\Front\Controller\Admin\ManageTagController:index')->setName('manageTag'); + $this->post('/manage-tags', '\Shaarli\Front\Controller\Admin\ManageTagController:save')->setName('saveManageTag'); $this ->get('/links-per-page', '\Shaarli\Front\Controller\Admin\SessionFilterController:linksPerPage') diff --git a/tests/front/controller/admin/ManageTagControllerTest.php b/tests/front/controller/admin/ManageTagControllerTest.php new file mode 100644 index 00000000..eed99231 --- /dev/null +++ b/tests/front/controller/admin/ManageTagControllerTest.php @@ -0,0 +1,272 @@ +createContainer(); + + $this->controller = new ManageTagController($this->container); + } + + /** + * Test displaying manage tag page + */ + public function testIndex(): void + { + $assignedVariables = []; + $this->assignTemplateVars($assignedVariables); + + $request = $this->createMock(Request::class); + $request->method('getParam')->with('fromtag')->willReturn('fromtag'); + $response = new Response(); + + $result = $this->controller->index($request, $response); + + static::assertSame(200, $result->getStatusCode()); + static::assertSame('changetag', (string) $result->getBody()); + + static::assertSame('fromtag', $assignedVariables['fromtag']); + static::assertSame('Manage tags - Shaarli', $assignedVariables['pagetitle']); + } + + /** + * Test posting a tag update - rename tag - valid info provided. + */ + public function testSaveRenameTagValid(): void + { + $session = []; + $this->assignSessionVars($session); + + $requestParameters = [ + 'renametag' => 'rename', + 'fromtag' => 'old-tag', + 'totag' => 'new-tag', + ]; + $request = $this->createMock(Request::class); + $request + ->expects(static::atLeastOnce()) + ->method('getParam') + ->willReturnCallback(function (string $key) use ($requestParameters): ?string { + return $requestParameters[$key] ?? null; + }) + ; + $response = new Response(); + + $bookmark1 = $this->createMock(Bookmark::class); + $bookmark2 = $this->createMock(Bookmark::class); + $this->container->bookmarkService + ->expects(static::once()) + ->method('search') + ->with(['searchtags' => 'old-tag'], BookmarkFilter::$ALL, true) + ->willReturnCallback(function () use ($bookmark1, $bookmark2): array { + $bookmark1->expects(static::once())->method('renameTag')->with('old-tag', 'new-tag'); + $bookmark2->expects(static::once())->method('renameTag')->with('old-tag', 'new-tag'); + + return [$bookmark1, $bookmark2]; + }) + ; + $this->container->bookmarkService + ->expects(static::exactly(2)) + ->method('set') + ->withConsecutive([$bookmark1, false], [$bookmark2, false]) + ; + $this->container->bookmarkService->expects(static::once())->method('save'); + + $result = $this->controller->save($request, $response); + + static::assertSame(302, $result->getStatusCode()); + static::assertSame(['./?searchtags=new-tag'], $result->getHeader('location')); + + static::assertArrayNotHasKey(SessionManager::KEY_ERROR_MESSAGES, $session); + static::assertArrayNotHasKey(SessionManager::KEY_WARNING_MESSAGES, $session); + static::assertArrayHasKey(SessionManager::KEY_SUCCESS_MESSAGES, $session); + static::assertSame(['The tag was renamed in 2 bookmarks.'], $session[SessionManager::KEY_SUCCESS_MESSAGES]); + } + + /** + * Test posting a tag update - delete tag - valid info provided. + */ + public function testSaveDeleteTagValid(): void + { + $session = []; + $this->assignSessionVars($session); + + $requestParameters = [ + 'deletetag' => 'delete', + 'fromtag' => 'old-tag', + ]; + $request = $this->createMock(Request::class); + $request + ->expects(static::atLeastOnce()) + ->method('getParam') + ->willReturnCallback(function (string $key) use ($requestParameters): ?string { + return $requestParameters[$key] ?? null; + }) + ; + $response = new Response(); + + $bookmark1 = $this->createMock(Bookmark::class); + $bookmark2 = $this->createMock(Bookmark::class); + $this->container->bookmarkService + ->expects(static::once()) + ->method('search') + ->with(['searchtags' => 'old-tag'], BookmarkFilter::$ALL, true) + ->willReturnCallback(function () use ($bookmark1, $bookmark2): array { + $bookmark1->expects(static::once())->method('deleteTag')->with('old-tag'); + $bookmark2->expects(static::once())->method('deleteTag')->with('old-tag'); + + return [$bookmark1, $bookmark2]; + }) + ; + $this->container->bookmarkService + ->expects(static::exactly(2)) + ->method('set') + ->withConsecutive([$bookmark1, false], [$bookmark2, false]) + ; + $this->container->bookmarkService->expects(static::once())->method('save'); + + $result = $this->controller->save($request, $response); + + static::assertSame(302, $result->getStatusCode()); + static::assertSame(['./manage-tags'], $result->getHeader('location')); + + static::assertArrayNotHasKey(SessionManager::KEY_ERROR_MESSAGES, $session); + static::assertArrayNotHasKey(SessionManager::KEY_WARNING_MESSAGES, $session); + static::assertArrayHasKey(SessionManager::KEY_SUCCESS_MESSAGES, $session); + static::assertSame(['The tag was removed from 2 bookmarks.'], $session[SessionManager::KEY_SUCCESS_MESSAGES]); + } + + /** + * Test posting a tag update - wrong token. + */ + public function testSaveWrongToken(): void + { + $this->container->sessionManager = $this->createMock(SessionManager::class); + $this->container->sessionManager->method('checkToken')->willReturn(false); + + $this->container->conf->expects(static::never())->method('set'); + $this->container->conf->expects(static::never())->method('write'); + + $request = $this->createMock(Request::class); + $response = new Response(); + + $this->expectException(WrongTokenException::class); + + $this->controller->save($request, $response); + } + + /** + * Test posting a tag update - rename tag - missing "FROM" tag. + */ + public function testSaveRenameTagMissingFrom(): void + { + $session = []; + $this->assignSessionVars($session); + + $requestParameters = [ + 'renametag' => 'rename', + ]; + $request = $this->createMock(Request::class); + $request + ->expects(static::atLeastOnce()) + ->method('getParam') + ->willReturnCallback(function (string $key) use ($requestParameters): ?string { + return $requestParameters[$key] ?? null; + }) + ; + $response = new Response(); + + $result = $this->controller->save($request, $response); + + static::assertSame(302, $result->getStatusCode()); + static::assertSame(['./manage-tags'], $result->getHeader('location')); + + static::assertArrayNotHasKey(SessionManager::KEY_ERROR_MESSAGES, $session); + static::assertArrayHasKey(SessionManager::KEY_WARNING_MESSAGES, $session); + static::assertArrayNotHasKey(SessionManager::KEY_SUCCESS_MESSAGES, $session); + static::assertSame(['Invalid tags provided.'], $session[SessionManager::KEY_WARNING_MESSAGES]); + } + + /** + * Test posting a tag update - delete tag - missing "FROM" tag. + */ + public function testSaveDeleteTagMissingFrom(): void + { + $session = []; + $this->assignSessionVars($session); + + $requestParameters = [ + 'deletetag' => 'delete', + ]; + $request = $this->createMock(Request::class); + $request + ->expects(static::atLeastOnce()) + ->method('getParam') + ->willReturnCallback(function (string $key) use ($requestParameters): ?string { + return $requestParameters[$key] ?? null; + }) + ; + $response = new Response(); + + $result = $this->controller->save($request, $response); + + static::assertSame(302, $result->getStatusCode()); + static::assertSame(['./manage-tags'], $result->getHeader('location')); + + static::assertArrayNotHasKey(SessionManager::KEY_ERROR_MESSAGES, $session); + static::assertArrayHasKey(SessionManager::KEY_WARNING_MESSAGES, $session); + static::assertArrayNotHasKey(SessionManager::KEY_SUCCESS_MESSAGES, $session); + static::assertSame(['Invalid tags provided.'], $session[SessionManager::KEY_WARNING_MESSAGES]); + } + + /** + * Test posting a tag update - rename tag - missing "TO" tag. + */ + public function testSaveRenameTagMissingTo(): void + { + $session = []; + $this->assignSessionVars($session); + + $requestParameters = [ + 'renametag' => 'rename', + 'fromtag' => 'old-tag' + ]; + $request = $this->createMock(Request::class); + $request + ->expects(static::atLeastOnce()) + ->method('getParam') + ->willReturnCallback(function (string $key) use ($requestParameters): ?string { + return $requestParameters[$key] ?? null; + }) + ; + $response = new Response(); + + $result = $this->controller->save($request, $response); + + static::assertSame(302, $result->getStatusCode()); + static::assertSame(['./manage-tags'], $result->getHeader('location')); + + static::assertArrayNotHasKey(SessionManager::KEY_ERROR_MESSAGES, $session); + static::assertArrayHasKey(SessionManager::KEY_WARNING_MESSAGES, $session); + static::assertArrayNotHasKey(SessionManager::KEY_SUCCESS_MESSAGES, $session); + static::assertSame(['Invalid tags provided.'], $session[SessionManager::KEY_WARNING_MESSAGES]); + } +} diff --git a/tpl/default/page.header.html b/tpl/default/page.header.html index 4afcca73..bde5036d 100644 --- a/tpl/default/page.header.html +++ b/tpl/default/page.header.html @@ -185,7 +185,7 @@ {/if} {if="!empty($global_errors) && $is_logged_in"} -
+
{loop="$global_errors"} @@ -199,7 +199,7 @@ {/if} {if="!empty($global_warnings) && $is_logged_in"} -
+
{loop="global_warnings"} @@ -213,7 +213,7 @@ {/if} {if="!empty($global_successes) && $is_logged_in"} -
+
{loop="$global_successes"} diff --git a/tpl/default/tag.list.html b/tpl/default/tag.list.html index 3e498f89..3adcfd1f 100644 --- a/tpl/default/tag.list.html +++ b/tpl/default/tag.list.html @@ -51,7 +51,7 @@
{if="$is_logged_in===true"}    - + {/if} diff --git a/tpl/default/tools.html b/tpl/default/tools.html index 0135c480..6e432e00 100644 --- a/tpl/default/tools.html +++ b/tpl/default/tools.html @@ -28,7 +28,7 @@
{/if}
- + {'Manage tags'|t}
diff --git a/tpl/vintage/tools.html b/tpl/vintage/tools.html index 0d8fcdec..8f606efb 100644 --- a/tpl/vintage/tools.html +++ b/tpl/vintage/tools.html @@ -11,7 +11,7 @@

{if="!$openshaarli"}Change password: Change your password.

{/if} - Rename/delete tags: Rename or delete a tag in all links + Rename/delete tags: Rename or delete a tag in all links

Import: Import Netscape html bookmarks (as exported from Firefox, Chrome, Opera, delicious...)

-- cgit v1.2.3