Process change visibility action through Slim controller

application/front/controller/admin/ManageShaareController.php

@@ -174,7 +174,7 @@ class ManageShaareController extends ShaarliAdminController
     }
 
     /**
-     * GET /admin/shaare/delete
+     * GET /admin/shaare/delete - Delete one or multiple bookmarks (depending on `id` query parameter).
      */
     public function deleteBookmark(Request $request, Response $response): Response
     {
@@ -228,6 +228,74 @@ class ManageShaareController extends ShaarliAdminController
         return $this->redirect($response, '/');
     }
 
+    /**
+     * GET /admin/shaare/visibility
+     *
+     * Change visibility (public/private) of one or multiple bookmarks (depending on `id` query parameter).
+     */
+    public function changeVisibility(Request $request, Response $response): Response
+    {
+        $this->checkToken($request);
+
+        $ids = trim(escape($request->getParam('id') ?? ''));
+        if (empty($ids) || strpos($ids, ' ') !== false) {
+            // multiple, space-separated ids provided
+            $ids = array_values(array_filter(preg_split('/\s+/', $ids), 'ctype_digit'));
+        } else {
+            // only a single id provided
+            $ids = [$ids];
+        }
+
+        // assert at least one id is given
+        if (0 === count($ids)) {
+            $this->saveErrorMessage(t('Invalid bookmark ID provided.'));
+
+            return $this->redirectFromReferer($request, $response, [], ['change_visibility']);
+        }
+
+        // assert that the visibility is valid
+        $visibility = $request->getParam('newVisibility');
+        if (null === $visibility || false === in_array($visibility, ['public', 'private'], true)) {
+            $this->saveErrorMessage(t('Invalid visibility provided.'));
+
+            return $this->redirectFromReferer($request, $response, [], ['change_visibility']);
+        } else {
+            $isPrivate = $visibility === 'private';
+        }
+
+        $formatter = $this->container->formatterFactory->getFormatter('raw');
+        $count = 0;
+
+        foreach ($ids as $id) {
+            try {
+                $bookmark = $this->container->bookmarkService->get((int) $id);
+            } catch (BookmarkNotFoundException $e) {
+                $this->saveErrorMessage(sprintf(
+                    t('Bookmark with identifier %s could not be found.'),
+                    $id
+                ));
+
+                continue;
+            }
+
+            $bookmark->setPrivate($isPrivate);
+
+            // 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, false);
+            ++$count;
+        }
+
+        if ($count > 0) {
+            $this->container->bookmarkService->save();
+        }
+
+        return $this->redirectFromReferer($request, $response, ['/visibility'], ['change_visibility']);
+    }
+
     /**
      * Helper function used to display the shaare form whether it's a new or existing bookmark.
      *

assets/default/js/base.js

@@ -486,7 +486,7 @@ function init(description) {
 
         const ids = links.map(item => item.id);
         window.location =
-          `${basePath}/?change_visibility&token=${token.value}&newVisibility=${visibility}&ids=${ids.join('+')}`;
+          `${basePath}/admin/shaare/visibility?token=${token.value}&newVisibility=${visibility}&id=${ids.join('+')}`;
       });
     });
   }

index.php

@@ -499,6 +499,8 @@ function renderPage($conf, $pluginManager, $bookmarkService, $history, $sessionM
 
     // -------- All other functions are reserved for the registered user:
 
+    // TODO: Remove legacy admin route redirections. We'll only keep public URL.
+
     // -------- Display the Tools menu if requested (import/export/bookmarklet...)
     if ($targetPage == Router::$PAGE_TOOLS) {
         header('Location: ./admin/tools');
@@ -547,53 +549,7 @@ function renderPage($conf, $pluginManager, $bookmarkService, $history, $sessionM
 
     // -------- User clicked either "Set public" or "Set private" bulk operation
     if ($targetPage == Router::$PAGE_CHANGE_VISIBILITY) {
-        if (! $sessionManager->checkToken($_GET['token'])) {
-            die(t('Wrong token.'));
-        }
-
-        $ids = trim($_GET['ids']);
-        if (strpos($ids, ' ') !== false) {
-            // multiple, space-separated ids provided
-            $ids = array_values(array_filter(preg_split('/\s+/', escape($ids))));
-        } else {
-            // only a single id provided
-            $ids = [$ids];
-        }
-
-        // assert at least one id is given
-        if (!count($ids)) {
-            die('no id provided');
-        }
-        // assert that the visibility is valid
-        if (!isset($_GET['newVisibility']) || !in_array($_GET['newVisibility'], ['public', 'private'])) {
-            die('invalid visibility');
-        } else {
-            $private = $_GET['newVisibility'] === 'private';
-        }
-        $factory = new FormatterFactory($conf, $loginManager->isLoggedIn());
-        $formatter = $factory->getFormatter('raw');
-        foreach ($ids as $id) {
-            $id = (int) escape($id);
-            $bookmark = $bookmarkService->get($id);
-            $bookmark->setPrivate($private);
-
-            // To preserve backward compatibility with 3rd parties, plugins still use arrays
-            $data = $formatter->format($bookmark);
-            $pluginManager->executeHooks('save_link', $data);
-            $bookmark->fromArray($data);
-
-            $bookmarkService->set($bookmark);
-        }
-        $bookmarkService->save();
-
-        $location = '?';
-        if (isset($_SERVER['HTTP_REFERER'])) {
-            $location = generateLocation(
-                $_SERVER['HTTP_REFERER'],
-                $_SERVER['HTTP_HOST']
-            );
-        }
-        header('Location: ' . $location); // After deleting the link, redirect to appropriate location
+        header('Location: ./admin/shaare/visibility?id=' . $_GET['token']);
         exit;
     }
 
@@ -1164,6 +1120,7 @@ $app->group('', function () {
     $this->get('/admin/shaare/{id:[0-9]+}', '\Shaarli\Front\Controller\Admin\ManageShaareController:displayEditForm');
     $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('/links-per-page', '\Shaarli\Front\Controller\Admin\SessionFilterController:linksPerPage');
     $this->get('/visibility/{visibility}', '\Shaarli\Front\Controller\Admin\SessionFilterController:visibility');

tests/front/controller/admin/ManageShaareControllerTest/ChangeVisibilityBookmarkTest.php

@@ -0,0 +1,418 @@
+<?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\Formatter\BookmarkFormatter;
+use Shaarli\Formatter\BookmarkRawFormatter;
+use Shaarli\Formatter\FormatterFactory;
+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 ChangeVisibilityBookmarkTest 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);
+    }
+
+    /**
+     * Change bookmark visibility - Set private - Single public bookmark with valid parameters
+     */
+    public function testSetSingleBookmarkPrivate(): void
+    {
+        $parameters = ['id' => '123', 'newVisibility' => 'private'];
+
+        $request = $this->createMock(Request::class);
+        $request
+            ->method('getParam')
+            ->willReturnCallback(function (string $key) use ($parameters): ?string {
+                return $parameters[$key] ?? null;
+            })
+        ;
+        $response = new Response();
+
+        $bookmark = (new Bookmark())->setId(123)->setUrl('http://domain.tld')->setTitle('Title 123')->setPrivate(false);
+
+        static::assertFalse($bookmark->isPrivate());
+
+        $this->container->bookmarkService->expects(static::once())->method('get')->with(123)->willReturn($bookmark);
+        $this->container->bookmarkService->expects(static::once())->method('set')->with($bookmark, false);
+        $this->container->bookmarkService->expects(static::once())->method('save');
+        $this->container->formatterFactory = $this->createMock(FormatterFactory::class);
+        $this->container->formatterFactory
+            ->expects(static::once())
+            ->method('getFormatter')
+            ->with('raw')
+            ->willReturnCallback(function () use ($bookmark): BookmarkFormatter {
+                return new BookmarkRawFormatter($this->container->conf, true);
+            })
+        ;
+
+        // Make sure that PluginManager hook is triggered
+        $this->container->pluginManager
+            ->expects(static::once())
+            ->method('executeHooks')
+            ->with('save_link')
+        ;
+
+        $result = $this->controller->changeVisibility($request, $response);
+
+        static::assertTrue($bookmark->isPrivate());
+
+        static::assertSame(302, $result->getStatusCode());
+        static::assertSame(['/subfolder/'], $result->getHeader('location'));
+    }
+
+    /**
+     * Change bookmark visibility - Set public - Single private bookmark with valid parameters
+     */
+    public function testSetSingleBookmarkPublic(): void
+    {
+        $parameters = ['id' => '123', 'newVisibility' => 'public'];
+
+        $request = $this->createMock(Request::class);
+        $request
+            ->method('getParam')
+            ->willReturnCallback(function (string $key) use ($parameters): ?string {
+                return $parameters[$key] ?? null;
+            })
+        ;
+        $response = new Response();
+
+        $bookmark = (new Bookmark())->setId(123)->setUrl('http://domain.tld')->setTitle('Title 123')->setPrivate(true);
+
+        static::assertTrue($bookmark->isPrivate());
+
+        $this->container->bookmarkService->expects(static::once())->method('get')->with(123)->willReturn($bookmark);
+        $this->container->bookmarkService->expects(static::once())->method('set')->with($bookmark, false);
+        $this->container->bookmarkService->expects(static::once())->method('save');
+        $this->container->formatterFactory = $this->createMock(FormatterFactory::class);
+        $this->container->formatterFactory
+            ->expects(static::once())
+            ->method('getFormatter')
+            ->with('raw')
+            ->willReturn(new BookmarkRawFormatter($this->container->conf, true))
+        ;
+
+        // Make sure that PluginManager hook is triggered
+        $this->container->pluginManager
+            ->expects(static::once())
+            ->method('executeHooks')
+            ->with('save_link')
+        ;
+
+        $result = $this->controller->changeVisibility($request, $response);
+
+        static::assertFalse($bookmark->isPrivate());
+
+        static::assertSame(302, $result->getStatusCode());
+        static::assertSame(['/subfolder/'], $result->getHeader('location'));
+    }
+
+    /**
+     * Change bookmark visibility - Set private on single already private bookmark
+     */
+    public function testSetSinglePrivateBookmarkPrivate(): void
+    {
+        $parameters = ['id' => '123', 'newVisibility' => 'private'];
+
+        $request = $this->createMock(Request::class);
+        $request
+            ->method('getParam')
+            ->willReturnCallback(function (string $key) use ($parameters): ?string {
+                return $parameters[$key] ?? null;
+            })
+        ;
+        $response = new Response();
+
+        $bookmark = (new Bookmark())->setId(123)->setUrl('http://domain.tld')->setTitle('Title 123')->setPrivate(true);
+
+        static::assertTrue($bookmark->isPrivate());
+
+        $this->container->bookmarkService->expects(static::once())->method('get')->with(123)->willReturn($bookmark);
+        $this->container->bookmarkService->expects(static::once())->method('set')->with($bookmark, false);
+        $this->container->bookmarkService->expects(static::once())->method('save');
+        $this->container->formatterFactory = $this->createMock(FormatterFactory::class);
+        $this->container->formatterFactory
+            ->expects(static::once())
+            ->method('getFormatter')
+            ->with('raw')
+            ->willReturn(new BookmarkRawFormatter($this->container->conf, true))
+        ;
+
+        // Make sure that PluginManager hook is triggered
+        $this->container->pluginManager
+            ->expects(static::once())
+            ->method('executeHooks')
+            ->with('save_link')
+        ;
+
+        $result = $this->controller->changeVisibility($request, $response);
+
+        static::assertTrue($bookmark->isPrivate());
+
+        static::assertSame(302, $result->getStatusCode());
+        static::assertSame(['/subfolder/'], $result->getHeader('location'));
+    }
+
+    /**
+     * Change bookmark visibility - Set multiple bookmarks private
+     */
+    public function testSetMultipleBookmarksPrivate(): void
+    {
+        $parameters = ['id' => '123 456 789', 'newVisibility' => 'private'];
+
+        $request = $this->createMock(Request::class);
+        $request
+            ->method('getParam')
+            ->willReturnCallback(function (string $key) use ($parameters): ?string {
+                return $parameters[$key] ?? null;
+            })
+        ;
+        $response = new Response();
+
+        $bookmarks = [
+            (new Bookmark())->setId(123)->setUrl('http://domain.tld')->setTitle('Title 123')->setPrivate(false),
+            (new Bookmark())->setId(456)->setUrl('http://domain.tld')->setTitle('Title 456')->setPrivate(true),
+            (new Bookmark())->setId(789)->setUrl('http://domain.tld')->setTitle('Title 789')->setPrivate(false),
+        ];
+
+        $this->container->bookmarkService
+            ->expects(static::exactly(3))
+            ->method('get')
+            ->withConsecutive([123], [456], [789])
+            ->willReturnOnConsecutiveCalls(...$bookmarks)
+        ;
+        $this->container->bookmarkService
+            ->expects(static::exactly(3))
+            ->method('set')
+            ->withConsecutive(...array_map(function (Bookmark $bookmark): array {
+                return [$bookmark, false];
+            }, $bookmarks))
+        ;
+        $this->container->bookmarkService->expects(static::once())->method('save');
+        $this->container->formatterFactory = $this->createMock(FormatterFactory::class);
+        $this->container->formatterFactory
+            ->expects(static::once())
+            ->method('getFormatter')
+            ->with('raw')
+            ->willReturn(new BookmarkRawFormatter($this->container->conf, true))
+        ;
+
+        // Make sure that PluginManager hook is triggered
+        $this->container->pluginManager
+            ->expects(static::exactly(3))
+            ->method('executeHooks')
+            ->with('save_link')
+        ;
+
+        $result = $this->controller->changeVisibility($request, $response);
+
+        static::assertTrue($bookmarks[0]->isPrivate());
+        static::assertTrue($bookmarks[1]->isPrivate());
+        static::assertTrue($bookmarks[2]->isPrivate());
+
+        static::assertSame(302, $result->getStatusCode());
+        static::assertSame(['/subfolder/'], $result->getHeader('location'));
+    }
+
+    /**
+     * Change bookmark visibility - Single bookmark not found.
+     */
+    public function testChangeVisibilitySingleBookmarkNotFound(): void
+    {
+        $parameters = ['id' => '123', 'newVisibility' => 'private'];
+
+        $request = $this->createMock(Request::class);
+        $request
+            ->method('getParam')
+            ->willReturnCallback(function (string $key) use ($parameters): ?string {
+                return $parameters[$key] ?? null;
+            })
+        ;
+        $response = new Response();
+
+        $this->container->bookmarkService
+            ->expects(static::once())
+            ->method('get')
+            ->willThrowException(new BookmarkNotFoundException())
+        ;
+        $this->container->bookmarkService->expects(static::never())->method('set');
+        $this->container->bookmarkService->expects(static::never())->method('save');
+        $this->container->formatterFactory = $this->createMock(FormatterFactory::class);
+        $this->container->formatterFactory
+            ->expects(static::once())
+            ->method('getFormatter')
+            ->with('raw')
+            ->willReturn(new BookmarkRawFormatter($this->container->conf, true))
+        ;
+
+        // Make sure that PluginManager hook is not triggered
+        $this->container->pluginManager
+            ->expects(static::never())
+            ->method('executeHooks')
+            ->with('save_link')
+        ;
+
+        $result = $this->controller->changeVisibility($request, $response);
+
+        static::assertSame(302, $result->getStatusCode());
+        static::assertSame(['/subfolder/'], $result->getHeader('location'));
+    }
+
+    /**
+     * Change bookmark visibility - Multiple bookmarks with one not found.
+     */
+    public function testChangeVisibilityMultipleBookmarksOneNotFound(): void
+    {
+        $parameters = ['id' => '123 456 789', 'newVisibility' => 'public'];
+
+        $request = $this->createMock(Request::class);
+        $request
+            ->method('getParam')
+            ->willReturnCallback(function (string $key) use ($parameters): ?string {
+                return $parameters[$key] ?? null;
+            })
+        ;
+        $response = new Response();
+
+        $bookmarks = [
+            (new Bookmark())->setId(123)->setUrl('http://domain.tld')->setTitle('Title 123')->setPrivate(true),
+            (new Bookmark())->setId(789)->setUrl('http://domain.tld')->setTitle('Title 789')->setPrivate(false),
+        ];
+
+        $this->container->bookmarkService
+            ->expects(static::exactly(3))
+            ->method('get')
+            ->withConsecutive([123], [456], [789])
+            ->willReturnCallback(function (int $id) use ($bookmarks): Bookmark {
+                if ($id === 123) {
+                    return $bookmarks[0];
+                }
+                if ($id === 789) {
+                    return $bookmarks[1];
+                }
+                throw new BookmarkNotFoundException();
+            })
+        ;
+        $this->container->bookmarkService
+            ->expects(static::exactly(2))
+            ->method('set')
+            ->withConsecutive(...array_map(function (Bookmark $bookmark): array {
+                return [$bookmark, false];
+            }, $bookmarks))
+        ;
+        $this->container->bookmarkService->expects(static::once())->method('save');
+
+        // Make sure that PluginManager hook is not triggered
+        $this->container->pluginManager
+            ->expects(static::exactly(2))
+            ->method('executeHooks')
+            ->with('save_link')
+        ;
+
+        $this->container->sessionManager
+            ->expects(static::once())
+            ->method('setSessionParameter')
+            ->with(SessionManager::KEY_ERROR_MESSAGES, ['Bookmark with identifier 456 could not be found.'])
+        ;
+
+        $result = $this->controller->changeVisibility($request, $response);
+
+        static::assertSame(302, $result->getStatusCode());
+        static::assertSame(['/subfolder/'], $result->getHeader('location'));
+    }
+
+    /**
+     * Change bookmark visibility - Invalid ID
+     */
+    public function testChangeVisibilityInvalidId(): void
+    {
+        $parameters = ['id' => 'nope not an ID', 'newVisibility' => 'private'];
+
+        $request = $this->createMock(Request::class);
+        $request
+            ->method('getParam')
+            ->willReturnCallback(function (string $key) use ($parameters): ?string {
+                return $parameters[$key] ?? null;
+            })
+        ;
+        $response = new Response();
+
+        $this->container->sessionManager
+            ->expects(static::once())
+            ->method('setSessionParameter')
+            ->with(SessionManager::KEY_ERROR_MESSAGES, ['Invalid bookmark ID provided.'])
+        ;
+
+        $result = $this->controller->changeVisibility($request, $response);
+
+        static::assertSame(302, $result->getStatusCode());
+        static::assertSame(['/subfolder/'], $result->getHeader('location'));
+    }
+
+    /**
+     * Change bookmark visibility - Empty ID
+     */
+    public function testChangeVisibilityEmptyId(): void
+    {
+        $request = $this->createMock(Request::class);
+        $response = new Response();
+
+        $this->container->sessionManager
+            ->expects(static::once())
+            ->method('setSessionParameter')
+            ->with(SessionManager::KEY_ERROR_MESSAGES, ['Invalid bookmark ID provided.'])
+        ;
+
+        $result = $this->controller->changeVisibility($request, $response);
+
+        static::assertSame(302, $result->getStatusCode());
+        static::assertSame(['/subfolder/'], $result->getHeader('location'));
+    }
+
+    /**
+     * Change bookmark visibility - with invalid visibility
+     */
+    public function testChangeVisibilityWithInvalidVisibility(): void
+    {
+        $parameters = ['id' => '123', 'newVisibility' => 'invalid'];
+
+        $request = $this->createMock(Request::class);
+        $request
+            ->method('getParam')
+            ->willReturnCallback(function (string $key) use ($parameters): ?string {
+                return $parameters[$key] ?? null;
+            })
+        ;
+        $response = new Response();
+
+        $this->container->sessionManager
+            ->expects(static::once())
+            ->method('setSessionParameter')
+            ->with(SessionManager::KEY_ERROR_MESSAGES, ['Invalid visibility provided.'])
+        ;
+
+        $result = $this->controller->changeVisibility($request, $response);
+
+        static::assertSame(302, $result->getStatusCode());
+        static::assertSame(['/subfolder/'], $result->getHeader('location'));
+    }
+}