Merge pull request #1621 from ArthurHoaro/feature/tag-separators

7 files changed, 157 insertions, 17 deletions

diff --git a/tests/front/controller/admin/ManageTagControllerTest.php b/tests/front/controller/admin/ManageTagControllerTest.php
index 8a0ff7a9..af6f273f 100644
--- a/tests/front/controller/admin/ManageTagControllerTest.php
+++ b/tests/front/controller/admin/ManageTagControllerTest.php
@@ -6,6 +6,7 @@ namespace Shaarli\Front\Controller\Admin;
 
 use Shaarli\Bookmark\Bookmark;
 use Shaarli\Bookmark\BookmarkFilter;
+use Shaarli\Config\ConfigManager;
 use Shaarli\Front\Exception\WrongTokenException;
 use Shaarli\Security\SessionManager;
 use Shaarli\TestCase;
@@ -44,10 +45,33 @@ class ManageTagControllerTest extends TestCase
         static::assertSame('changetag', (string) $result->getBody());
 
         static::assertSame('fromtag', $assignedVariables['fromtag']);
+        static::assertSame('@', $assignedVariables['tags_separator']);
         static::assertSame('Manage tags - Shaarli', $assignedVariables['pagetitle']);
     }
 
     /**
+     * Test displaying manage tag page
+     */
+    public function testIndexWhitespaceSeparator(): void
+    {
+        $assignedVariables = [];
+        $this->assignTemplateVars($assignedVariables);
+
+        $this->container->conf = $this->createMock(ConfigManager::class);
+        $this->container->conf->method('get')->willReturnCallback(function (string $key) {
+            return $key === 'general.tags_separator' ? ' ' : $key;
+        });
+
+        $request = $this->createMock(Request::class);
+        $response = new Response();
+
+        $this->controller->index($request, $response);
+
+        static::assertSame(' ', $assignedVariables['tags_separator']);
+        static::assertSame('whitespace', $assignedVariables['tags_separator_desc']);
+    }
+
+    /**
      * Test posting a tag update - rename tag - valid info provided.
      */
     public function testSaveRenameTagValid(): void
@@ -269,4 +293,116 @@ class ManageTagControllerTest extends TestCase
         static::assertArrayNotHasKey(SessionManager::KEY_SUCCESS_MESSAGES, $session);
         static::assertSame(['Invalid tags provided.'], $session[SessionManager::KEY_WARNING_MESSAGES]);
     }
+
+    /**
+     * Test changeSeparator to '#': redirection + success message.
+     */
+    public function testChangeSeparatorValid(): void
+    {
+        $toSeparator = '#';
+
+        $session = [];
+        $this->assignSessionVars($session);
+
+        $request = $this->createMock(Request::class);
+        $request
+            ->expects(static::atLeastOnce())
+            ->method('getParam')
+            ->willReturnCallback(function (string $key) use ($toSeparator): ?string {
+                return $key === 'separator' ? $toSeparator : $key;
+            })
+        ;
+        $response = new Response();
+
+        $this->container->conf
+            ->expects(static::once())
+            ->method('set')
+            ->with('general.tags_separator', $toSeparator, true, true)
+        ;
+
+        $result = $this->controller->changeSeparator($request, $response);
+
+        static::assertSame(302, $result->getStatusCode());
+        static::assertSame(['/subfolder/admin/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(
+            ['Your tags separator setting has been updated!'],
+            $session[SessionManager::KEY_SUCCESS_MESSAGES]
+        );
+    }
+
+    /**
+     * Test changeSeparator to '#@' (too long): redirection + error message.
+     */
+    public function testChangeSeparatorInvalidTooLong(): void
+    {
+        $toSeparator = '#@';
+
+        $session = [];
+        $this->assignSessionVars($session);
+
+        $request = $this->createMock(Request::class);
+        $request
+            ->expects(static::atLeastOnce())
+            ->method('getParam')
+            ->willReturnCallback(function (string $key) use ($toSeparator): ?string {
+                return $key === 'separator' ? $toSeparator : $key;
+            })
+        ;
+        $response = new Response();
+
+        $this->container->conf->expects(static::never())->method('set');
+
+        $result = $this->controller->changeSeparator($request, $response);
+
+        static::assertSame(302, $result->getStatusCode());
+        static::assertSame(['/subfolder/admin/tags'], $result->getHeader('location'));
+
+        static::assertArrayNotHasKey(SessionManager::KEY_SUCCESS_MESSAGES, $session);
+        static::assertArrayNotHasKey(SessionManager::KEY_WARNING_MESSAGES, $session);
+        static::assertArrayHasKey(SessionManager::KEY_ERROR_MESSAGES, $session);
+        static::assertSame(
+            ['Tags separator must be a single character.'],
+            $session[SessionManager::KEY_ERROR_MESSAGES]
+        );
+    }
+
+    /**
+     * Test changeSeparator to '#@' (too long): redirection + error message.
+     */
+    public function testChangeSeparatorInvalidReservedCharacter(): void
+    {
+        $toSeparator = '*';
+
+        $session = [];
+        $this->assignSessionVars($session);
+
+        $request = $this->createMock(Request::class);
+        $request
+            ->expects(static::atLeastOnce())
+            ->method('getParam')
+            ->willReturnCallback(function (string $key) use ($toSeparator): ?string {
+                return $key === 'separator' ? $toSeparator : $key;
+            })
+        ;
+        $response = new Response();
+
+        $this->container->conf->expects(static::never())->method('set');
+
+        $result = $this->controller->changeSeparator($request, $response);
+
+        static::assertSame(302, $result->getStatusCode());
+        static::assertSame(['/subfolder/admin/tags'], $result->getHeader('location'));
+
+        static::assertArrayNotHasKey(SessionManager::KEY_SUCCESS_MESSAGES, $session);
+        static::assertArrayNotHasKey(SessionManager::KEY_WARNING_MESSAGES, $session);
+        static::assertArrayHasKey(SessionManager::KEY_ERROR_MESSAGES, $session);
+        static::assertStringStartsWith(
+            'These characters are reserved and can\'t be used as tags separator',
+            $session[SessionManager::KEY_ERROR_MESSAGES][0]
+        );
+    }
 }
diff --git a/tests/front/controller/admin/ShaarePublishControllerTest/DisplayCreateFormTest.php b/tests/front/controller/admin/ShaarePublishControllerTest/DisplayCreateFormTest.php
index f20b1def..964773da 100644
--- a/tests/front/controller/admin/ShaarePublishControllerTest/DisplayCreateFormTest.php
+++ b/tests/front/controller/admin/ShaarePublishControllerTest/DisplayCreateFormTest.php
@@ -101,7 +101,7 @@ class DisplayCreateFormTest extends TestCase
         static::assertSame($expectedUrl, $assignedVariables['link']['url']);
         static::assertSame($remoteTitle, $assignedVariables['link']['title']);
         static::assertSame($remoteDesc, $assignedVariables['link']['description']);
-        static::assertSame($remoteTags, $assignedVariables['link']['tags']);
+        static::assertSame($remoteTags . ' ', $assignedVariables['link']['tags']);
         static::assertFalse($assignedVariables['link']['private']);
 
         static::assertTrue($assignedVariables['link_is_new']);
@@ -192,7 +192,7 @@ class DisplayCreateFormTest extends TestCase
             'post' => 'http://url.tld/other?part=3&utm_ad=pay#hash',
             'title' => 'Provided Title',
             'description' => 'Provided description.',
-            'tags' => 'abc def',
+            'tags' => 'abc@def',
             'private' => '1',
             'source' => 'apps',
         ];
@@ -216,7 +216,7 @@ class DisplayCreateFormTest extends TestCase
         static::assertSame($expectedUrl, $assignedVariables['link']['url']);
         static::assertSame($parameters['title'], $assignedVariables['link']['title']);
         static::assertSame($parameters['description'], $assignedVariables['link']['description']);
-        static::assertSame($parameters['tags'], $assignedVariables['link']['tags']);
+        static::assertSame($parameters['tags'] . '@', $assignedVariables['link']['tags']);
         static::assertTrue($assignedVariables['link']['private']);
         static::assertTrue($assignedVariables['link_is_new']);
         static::assertSame($parameters['source'], $assignedVariables['source']);
@@ -360,7 +360,7 @@ class DisplayCreateFormTest extends TestCase
         static::assertSame($expectedUrl, $assignedVariables['link']['url']);
         static::assertSame($title, $assignedVariables['link']['title']);
         static::assertSame($description, $assignedVariables['link']['description']);
-        static::assertSame(implode(' ', $tags), $assignedVariables['link']['tags']);
+        static::assertSame(implode('@', $tags) . '@', $assignedVariables['link']['tags']);
         static::assertTrue($assignedVariables['link']['private']);
         static::assertSame($createdAt, $assignedVariables['link']['created']);
     }
diff --git a/tests/front/controller/admin/ShaarePublishControllerTest/DisplayEditFormTest.php b/tests/front/controller/admin/ShaarePublishControllerTest/DisplayEditFormTest.php
index da393e49..738cea12 100644
--- a/tests/front/controller/admin/ShaarePublishControllerTest/DisplayEditFormTest.php
+++ b/tests/front/controller/admin/ShaarePublishControllerTest/DisplayEditFormTest.php
@@ -74,7 +74,7 @@ class DisplayEditFormTest extends TestCase
         static::assertSame($url, $assignedVariables['link']['url']);
         static::assertSame($title, $assignedVariables['link']['title']);
         static::assertSame($description, $assignedVariables['link']['description']);
-        static::assertSame(implode(' ', $tags), $assignedVariables['link']['tags']);
+        static::assertSame(implode('@', $tags) . '@', $assignedVariables['link']['tags']);
         static::assertTrue($assignedVariables['link']['private']);
         static::assertSame($createdAt, $assignedVariables['link']['created']);
     }
diff --git a/tests/front/controller/visitor/BookmarkListControllerTest.php b/tests/front/controller/visitor/BookmarkListControllerTest.php
index 5cbc8c73..dec938f2 100644
--- a/tests/front/controller/visitor/BookmarkListControllerTest.php
+++ b/tests/front/controller/visitor/BookmarkListControllerTest.php
@@ -173,7 +173,7 @@ class BookmarkListControllerTest extends TestCase
         $request = $this->createMock(Request::class);
         $request->method('getParam')->willReturnCallback(function (string $key) {
             if ('searchtags' === $key) {
-                return 'abc def';
+                return 'abc@def';
             }
             if ('searchterm' === $key) {
                 return 'ghi jkl';
@@ -204,7 +204,7 @@ class BookmarkListControllerTest extends TestCase
             ->expects(static::once())
             ->method('search')
             ->with(
-                ['searchtags' => 'abc def', 'searchterm' => 'ghi jkl'],
+                ['searchtags' => 'abc@def', 'searchterm' => 'ghi jkl'],
                 'private',
                 false,
                 true
@@ -222,7 +222,7 @@ class BookmarkListControllerTest extends TestCase
         static::assertSame('linklist', (string) $result->getBody());
 
         static::assertSame('Search: ghi jkl [abc] [def] - Shaarli', $assignedVariables['pagetitle']);
-        static::assertSame('?page=2&searchterm=ghi+jkl&searchtags=abc+def', $assignedVariables['previous_page_url']);
+        static::assertSame('?page=2&searchterm=ghi+jkl&searchtags=abc%40def', $assignedVariables['previous_page_url']);
     }
 
     /**
diff --git a/tests/front/controller/visitor/FrontControllerMockHelper.php b/tests/front/controller/visitor/FrontControllerMockHelper.php
index fc0bb7d1..02229f68 100644
--- a/tests/front/controller/visitor/FrontControllerMockHelper.php
+++ b/tests/front/controller/visitor/FrontControllerMockHelper.php
@@ -41,6 +41,10 @@ trait FrontControllerMockHelper
         // Config
         $this->container->conf = $this->createMock(ConfigManager::class);
         $this->container->conf->method('get')->willReturnCallback(function (string $parameter, $default) {
+            if ($parameter === 'general.tags_separator') {
+                return '@';
+            }
+
             return $default === null ? $parameter : $default;
         });
 
diff --git a/tests/front/controller/visitor/TagCloudControllerTest.php b/tests/front/controller/visitor/TagCloudControllerTest.php
index 9305612e..4915573d 100644
--- a/tests/front/controller/visitor/TagCloudControllerTest.php
+++ b/tests/front/controller/visitor/TagCloudControllerTest.php
@@ -100,7 +100,7 @@ class TagCloudControllerTest extends TestCase
             ->with()
             ->willReturnCallback(function (string $key): ?string {
                 if ('searchtags' === $key) {
-                    return 'ghi def';
+                    return 'ghi@def';
                 }
 
                 return null;
@@ -131,7 +131,7 @@ class TagCloudControllerTest extends TestCase
             ->withConsecutive(['render_tagcloud'])
             ->willReturnCallback(function (string $hook, array $data, array $param): array {
                if ('render_tagcloud' === $hook) {
-                   static::assertSame('ghi def', $data['search_tags']);
+                   static::assertSame('ghi@def@', $data['search_tags']);
                    static::assertCount(1, $data['tags']);
 
                    static::assertArrayHasKey('loggedin', $param);
@@ -147,7 +147,7 @@ class TagCloudControllerTest extends TestCase
         static::assertSame('tag.cloud', (string) $result->getBody());
         static::assertSame('ghi def - Tag cloud - Shaarli', $assignedVariables['pagetitle']);
 
-        static::assertSame('ghi def', $assignedVariables['search_tags']);
+        static::assertSame('ghi@def@', $assignedVariables['search_tags']);
         static::assertCount(1, $assignedVariables['tags']);
 
         static::assertArrayHasKey('abc', $assignedVariables['tags']);
@@ -277,7 +277,7 @@ class TagCloudControllerTest extends TestCase
             ->with()
             ->willReturnCallback(function (string $key): ?string {
                 if ('searchtags' === $key) {
-                    return 'ghi def';
+                    return 'ghi@def';
                 } elseif ('sort' === $key) {
                     return 'alpha';
                 }
@@ -310,7 +310,7 @@ class TagCloudControllerTest extends TestCase
             ->withConsecutive(['render_taglist'])
             ->willReturnCallback(function (string $hook, array $data, array $param): array {
                 if ('render_taglist' === $hook) {
-                    static::assertSame('ghi def', $data['search_tags']);
+                    static::assertSame('ghi@def@', $data['search_tags']);
                     static::assertCount(1, $data['tags']);
 
                     static::assertArrayHasKey('loggedin', $param);
@@ -326,7 +326,7 @@ class TagCloudControllerTest extends TestCase
         static::assertSame('tag.list', (string) $result->getBody());
         static::assertSame('ghi def - Tag list - Shaarli', $assignedVariables['pagetitle']);
 
-        static::assertSame('ghi def', $assignedVariables['search_tags']);
+        static::assertSame('ghi@def@', $assignedVariables['search_tags']);
         static::assertCount(1, $assignedVariables['tags']);
         static::assertSame(3, $assignedVariables['tags']['abc']);
     }
diff --git a/tests/front/controller/visitor/TagControllerTest.php b/tests/front/controller/visitor/TagControllerTest.php
index 750ea02d..5a556c6d 100644
--- a/tests/front/controller/visitor/TagControllerTest.php
+++ b/tests/front/controller/visitor/TagControllerTest.php
@@ -50,7 +50,7 @@ class TagControllerTest extends TestCase
 
         static::assertInstanceOf(Response::class, $result);
         static::assertSame(302, $result->getStatusCode());
-        static::assertSame(['/controller/?searchtags=def+abc'], $result->getHeader('location'));
+        static::assertSame(['/controller/?searchtags=def%40abc'], $result->getHeader('location'));
     }
 
     public function testAddTagWithoutRefererAndExistingSearch(): void
@@ -80,7 +80,7 @@ class TagControllerTest extends TestCase
 
         static::assertInstanceOf(Response::class, $result);
         static::assertSame(302, $result->getStatusCode());
-        static::assertSame(['/controller/?searchtags=def+abc'], $result->getHeader('location'));
+        static::assertSame(['/controller/?searchtags=def%40abc'], $result->getHeader('location'));
     }
 
     public function testAddTagResetPagination(): void
@@ -96,7 +96,7 @@ class TagControllerTest extends TestCase
 
         static::assertInstanceOf(Response::class, $result);
         static::assertSame(302, $result->getStatusCode());
-        static::assertSame(['/controller/?searchtags=def+abc'], $result->getHeader('location'));
+        static::assertSame(['/controller/?searchtags=def%40abc'], $result->getHeader('location'));
     }
 
     public function testAddTagWithRefererAndEmptySearch(): void