Process manage tags page through Slim controller

application/front/controller/admin/ConfigureController.php

@@ -12,7 +12,7 @@
 use Throwable;
 
 /**
- * Class PasswordController
+ * Class ConfigureController
  *
  * Slim controller used to handle Shaarli configuration page (display + save new config).
  */

application/front/controller/admin/ManageTagController.php

@@ -0,0 +1,87 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Shaarli\Front\Controller\Admin;
+
+use Shaarli\Bookmark\BookmarkFilter;
+use Slim\Http\Request;
+use Slim\Http\Response;
+
+/**
+ * Class ManageTagController
+ *
+ * Slim controller used to handle Shaarli manage tags page (rename and delete tags).
+ */
+class ManageTagController extends ShaarliAdminController
+{
+    /**
+     * GET /manage-tags - Displays the manage tags page
+     */
+    public function index(Request $request, Response $response): Response
+    {
+        $fromTag = $request->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);
+    }
+}

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();

assets/default/scss/shaarli.scss

@@ -490,6 +490,10 @@ body,
   }
 }
 
+.header-alert-message {
+  text-align: center;
+}
+
 // CONTENT - GENERAL
 .container {
   position: relative;

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 '<script>alert("'. $alert .'");document.location=\'?'. $redirect .'\';</script>';
+        header('./manage-tags');
         exit;
     }
 
@@ -1380,6 +1349,8 @@ function install($conf, $sessionManager, $loginManager)
     $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')

tests/front/controller/admin/ManageTagControllerTest.php

@@ -0,0 +1,272 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Shaarli\Front\Controller\Admin;
+
+use PHPUnit\Framework\TestCase;
+use Shaarli\Bookmark\Bookmark;
+use Shaarli\Bookmark\BookmarkFilter;
+use Shaarli\Front\Exception\WrongTokenException;
+use Shaarli\Security\SessionManager;
+use Slim\Http\Request;
+use Slim\Http\Response;
+
+class ManageTagControllerTest extends TestCase
+{
+    use FrontAdminControllerMockHelper;
+
+    /** @var ManageTagController */
+    protected $controller;
+
+    public function setUp(): void
+    {
+        $this->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]);
+    }
+}

tpl/default/page.header.html

@@ -185,7 +185,7 @@
 {/if}
 
 {if="!empty($global_errors) && $is_logged_in"}
-  <div class="pure-g new-version-message pure-alert pure-alert-error pure-alert-closable" id="shaarli-errors-alert">
+  <div class="pure-g header-alert-message pure-alert pure-alert-error pure-alert-closable" id="shaarli-errors-alert">
   <div class="pure-u-2-24"></div>
     <div class="pure-u-20-24">
       {loop="$global_errors"}
@@ -199,7 +199,7 @@
 {/if}
 
 {if="!empty($global_warnings) && $is_logged_in"}
-  <div class="pure-g pure-alert pure-alert-warning pure-alert-closable" id="shaarli-warnings-alert">
+  <div class="pure-g header-alert-message pure-alert pure-alert-warning pure-alert-closable" id="shaarli-warnings-alert">
     <div class="pure-u-2-24"></div>
     <div class="pure-u-20-24">
       {loop="global_warnings"}
@@ -213,7 +213,7 @@
 {/if}
 
 {if="!empty($global_successes) && $is_logged_in"}
-  <div class="pure-g new-version-message pure-alert pure-alert-success pure-alert-closable" id="shaarli-success-alert">
+  <div class="pure-g header-alert-message new-version-message pure-alert pure-alert-success pure-alert-closable" id="shaarli-success-alert">
     <div class="pure-u-2-24"></div>
     <div class="pure-u-20-24">
       {loop="$global_successes"}

tpl/default/tag.list.html

@@ -51,7 +51,7 @@ <h2 class="window-title">{'Tag list'|t} - {$countTags} {'tags'|t}</h2>
           <div class="pure-u-1">
             {if="$is_logged_in===true"}
               <a href="#" class="delete-tag" aria-label="{'Delete'|t}"><i class="fa fa-trash" aria-hidden="true"></i></a>&nbsp;&nbsp;
-              <a href="./?do=changetag&fromtag={$key|urlencode}" class="rename-tag" aria-label="{'Rename tag'|t}">
+              <a href="./manage-tags?fromtag={$key|urlencode}" class="rename-tag" aria-label="{'Rename tag'|t}">
                 <i class="fa fa-pencil-square-o {$key}" aria-hidden="true"></i>
               </a>
             {/if}

tpl/default/tools.html

@@ -28,7 +28,7 @@ <h2 class="window-title">{'Settings'|t}</h2>
       </div>
     {/if}
     <div class="tools-item">
-      <a href="./?do=changetag" title="{'Rename or delete a tag in all links'|t}">
+      <a href="./manage-tags" title="{'Rename or delete a tag in all links'|t}">
         <span class="pure-button pure-u-lg-2-3 pure-u-3-4">{'Manage tags'|t}</span>
       </a>
     </div>

tpl/vintage/tools.html

@@ -11,7 +11,7 @@
     <br><br>
 		{if="!$openshaarli"}<a href="?do=changepasswd"><b>Change password</b><span>: Change your password.</span></a>
     <br><br>{/if}
-		<a href="./?do=changetag"><b>Rename/delete tags</b><span>: Rename or delete a tag in all links</span></a>
+		<a href="./manage-tags"><b>Rename/delete tags</b><span>: Rename or delete a tag in all links</span></a>
     <br><br>
 		<a href="./?do=import"><b>Import</b><span>: Import Netscape html bookmarks (as exported from Firefox, Chrome, Opera, delicious...)</span></a>
     <br><br>