Pin bookmarks through Slim controller

4 files changed, 185 insertions, 15 deletions

diff --git a/application/front/controller/admin/ManageShaareController.php b/application/front/controller/admin/ManageShaareController.php
index ff330a99..bdfc5ca7 100644
--- a/application/front/controller/admin/ManageShaareController.php
+++ b/application/front/controller/admin/ManageShaareController.php
@@ -297,6 +297,42 @@ class ManageShaareController extends ShaarliAdminController
     }
 
     /**
+     * GET /admin/shaare/{id}/pin - Pin or unpin a bookmark.
+     */
+    public function pinBookmark(Request $request, Response $response, array $args): Response
+    {
+        $this->checkToken($request);
+
+        $id = $args['id'] ?? '';
+        try {
+            if (false === ctype_digit($id)) {
+                throw new BookmarkNotFoundException();
+            }
+            $bookmark = $this->container->bookmarkService->get((int) $id);  // Read database
+        } catch (BookmarkNotFoundException $e) {
+            $this->saveErrorMessage(sprintf(
+                t('Bookmark with identifier %s could not be found.'),
+                $id
+            ));
+
+            return $this->redirectFromReferer($request, $response, ['/pin'], ['pin']);
+        }
+
+        $formatter = $this->container->formatterFactory->getFormatter('raw');
+
+        $bookmark->setSticky(!$bookmark->isSticky());
+
+        // To preserve backward compatibility with 3rd parties, plugins still use arrays
+        $data = $formatter->format($bookmark);
+        $this->container->pluginManager->executeHooks('save_link', $data);
+        $bookmark->fromArray($data);
+
+        $this->container->bookmarkService->set($bookmark);
+
+        return $this->redirectFromReferer($request, $response, ['/pin'], ['pin']);
+    }
+
+    /**
      * Helper function used to display the shaare form whether it's a new or existing bookmark.
      *
      * @param array $link data used in template, either from parameters or from the data store
diff --git a/index.php b/index.php
index 93e5590b..c8660658 100644
--- a/index.php
+++ b/index.php
@@ -567,20 +567,8 @@ function renderPage($conf, $pluginManager, $bookmarkService, $history, $sessionM
     }
 
     if ($targetPage == Router::$PAGE_PINLINK) {
-        if (! isset($_GET['id']) || !$bookmarkService->exists($_GET['id'])) {
-            // FIXME! Use a proper error system.
-            $msg = t('Invalid link ID provided');
-            echo '<script>alert("'. $msg .'");document.location=\''. index_url($_SERVER) .'\';</script>';
-            exit;
-        }
-        if (! $sessionManager->checkToken($_GET['token'])) {
-            die('Wrong token.');
-        }
-
-        $link = $bookmarkService->get($_GET['id']);
-        $link->setSticky(! $link->isSticky());
-        $bookmarkService->set($link);
-        header('Location: '.index_url($_SERVER));
+        // This route is no longer supported in legacy mode
+        header('Location: ./');
         exit;
     }
 
@@ -1121,6 +1109,7 @@ $app->group('', function () {
     $this->post('/admin/shaare', '\Shaarli\Front\Controller\Admin\ManageShaareController:save');
     $this->get('/admin/shaare/delete', '\Shaarli\Front\Controller\Admin\ManageShaareController:deleteBookmark');
     $this->get('/admin/shaare/visibility', '\Shaarli\Front\Controller\Admin\ManageShaareController:changeVisibility');
+    $this->get('/admin/shaare/{id:[0-9]+}/pin', '\Shaarli\Front\Controller\Admin\ManageShaareController:pinBookmark');
 
     $this->get('/links-per-page', '\Shaarli\Front\Controller\Admin\SessionFilterController:linksPerPage');
     $this->get('/visibility/{visibility}', '\Shaarli\Front\Controller\Admin\SessionFilterController:visibility');
diff --git a/tests/front/controller/admin/ManageShaareControllerTest/PinBookmarkTest.php b/tests/front/controller/admin/ManageShaareControllerTest/PinBookmarkTest.php
new file mode 100644
index 00000000..1607b475
--- /dev/null
+++ b/tests/front/controller/admin/ManageShaareControllerTest/PinBookmarkTest.php
@@ -0,0 +1,145 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Shaarli\Front\Controller\Admin\ManageShaareControllerTest;
+
+use PHPUnit\Framework\TestCase;
+use Shaarli\Bookmark\Bookmark;
+use Shaarli\Bookmark\Exception\BookmarkNotFoundException;
+use Shaarli\Front\Controller\Admin\FrontAdminControllerMockHelper;
+use Shaarli\Front\Controller\Admin\ManageShaareController;
+use Shaarli\Http\HttpAccess;
+use Shaarli\Security\SessionManager;
+use Slim\Http\Request;
+use Slim\Http\Response;
+
+class PinBookmarkTest extends TestCase
+{
+    use FrontAdminControllerMockHelper;
+
+    /** @var ManageShaareController */
+    protected $controller;
+
+    public function setUp(): void
+    {
+        $this->createContainer();
+
+        $this->container->httpAccess = $this->createMock(HttpAccess::class);
+        $this->controller = new ManageShaareController($this->container);
+    }
+
+    /**
+     * Test pin bookmark - with valid input
+     *
+     * @dataProvider initialStickyValuesProvider()
+     */
+    public function testPinBookmarkIsStickyNull(?bool $sticky, bool $expectedValue): void
+    {
+        $id = 123;
+
+        $request = $this->createMock(Request::class);
+        $response = new Response();
+
+        $bookmark = (new Bookmark())
+            ->setId(123)
+            ->setUrl('http://domain.tld')
+            ->setTitle('Title 123')
+            ->setSticky($sticky)
+        ;
+
+        $this->container->bookmarkService->expects(static::once())->method('get')->with(123)->willReturn($bookmark);
+        $this->container->bookmarkService->expects(static::once())->method('set')->with($bookmark, true);
+
+        // Make sure that PluginManager hook is triggered
+        $this->container->pluginManager
+            ->expects(static::once())
+            ->method('executeHooks')
+            ->with('save_link')
+        ;
+
+        $result = $this->controller->pinBookmark($request, $response, ['id' => (string) $id]);
+
+        static::assertSame(302, $result->getStatusCode());
+        static::assertSame(['/subfolder/'], $result->getHeader('location'));
+
+        static::assertSame($expectedValue, $bookmark->isSticky());
+    }
+
+    public function initialStickyValuesProvider(): array
+    {
+        // [initialStickyState, isStickyAfterPin]
+        return [[null, true], [false, true], [true, false]];
+    }
+
+    /**
+     * Test pin bookmark - invalid bookmark ID
+     */
+    public function testDisplayEditFormInvalidId(): void
+    {
+        $id = 'invalid';
+
+        $request = $this->createMock(Request::class);
+        $response = new Response();
+
+        $this->container->sessionManager
+            ->expects(static::once())
+            ->method('setSessionParameter')
+            ->with(SessionManager::KEY_ERROR_MESSAGES, ['Bookmark with identifier invalid could not be found.'])
+        ;
+
+        $result = $this->controller->pinBookmark($request, $response, ['id' => $id]);
+
+        static::assertSame(302, $result->getStatusCode());
+        static::assertSame(['/subfolder/'], $result->getHeader('location'));
+    }
+
+    /**
+     * Test pin bookmark - Bookmark ID not provided
+     */
+    public function testDisplayEditFormIdNotProvided(): void
+    {
+        $request = $this->createMock(Request::class);
+        $response = new Response();
+
+        $this->container->sessionManager
+            ->expects(static::once())
+            ->method('setSessionParameter')
+            ->with(SessionManager::KEY_ERROR_MESSAGES, ['Bookmark with identifier  could not be found.'])
+        ;
+
+        $result = $this->controller->pinBookmark($request, $response, []);
+
+        static::assertSame(302, $result->getStatusCode());
+        static::assertSame(['/subfolder/'], $result->getHeader('location'));
+    }
+
+    /**
+     * Test pin bookmark - bookmark not found
+     */
+    public function testDisplayEditFormBookmarkNotFound(): void
+    {
+        $id = 123;
+
+        $request = $this->createMock(Request::class);
+        $response = new Response();
+
+        $this->container->bookmarkService
+            ->expects(static::once())
+            ->method('get')
+            ->with($id)
+            ->willThrowException(new BookmarkNotFoundException())
+        ;
+
+        $this->container->sessionManager
+            ->expects(static::once())
+            ->method('setSessionParameter')
+            ->with(SessionManager::KEY_ERROR_MESSAGES, ['Bookmark with identifier 123 could not be found.'])
+        ;
+
+        $result = $this->controller->pinBookmark($request, $response, ['id' => (string) $id]);
+
+        static::assertSame(302, $result->getStatusCode());
+        static::assertSame(['/subfolder/'], $result->getHeader('location'));
+    }
+}
diff --git a/tpl/default/linklist.html b/tpl/default/linklist.html
index 2e2f96d6..b0a5fdf2 100644
--- a/tpl/default/linklist.html
+++ b/tpl/default/linklist.html
@@ -207,7 +207,7 @@
                       </a>
                     </span>
                     <span class="linklist-item-infos-controls-item ctrl-pin">
-                      <a href="{$base_path}/?do=pin&amp;id={$value.id}&amp;token={$token}"
+                      <a href="{$base_path}/admin/shaare/{$value.id}/pin?token={$token}"
                          title="{$strToggleSticky}" aria-label="{$strToggleSticky}" class="pin-link {if="$value.sticky"}pinned-link{/if} pure-u-0 pure-u-lg-visible">
                         <i class="fa fa-thumb-tack" aria-hidden="true"></i>
                       </a>